feat(apps): managed app reconciliation, end to end - #457
Open
Alexgodoroja wants to merge 4 commits into
Open
Conversation
Nodes converge their installed apps toward a desired-set document the authority writes into the fleet state mirror. The document arrives as an ordinary signed, revision-fenced state mutation, so managed app install adds no new command kind, endpoint, or inbound channel. The two-root design is the grant boundary. Grants exist only inside a signed bundle, never in the catalogue, so a node installs an unreviewed app into a staging root the supervisor does not scan: the binary is present, its manifest can be read and reported, and it cannot run. Promotion into the live install root happens only once the desired document accepts every grant the manifest declares -- and a catalogue republish that widens them demotes the app rather than silently keeping the wider capability set. Installation itself shells out to the pilotctl beside the daemon. pilotctl owns the only implementation of the catalogue trust chain, and a second verifier for a security boundary is the one thing that must not exist; AppInstaller keeps that swappable if the path is later extracted. The inventory is republished only when it actually changes: it lives in the state mirror, so a ticking timestamp would churn the revision the console fences its mutations on and invalidate in-flight installs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two halves of the same gap, both found preparing the App Store e2e. Adoption never wrote an apps block, so HasAppReconcile() was false on every managed node and the reconciler this repo ships could not run. pilotctl adopt now emits one when the authority grants the option, gated on fleet state because the desired document and the observed report both travel that way. The install and staging roots are left to the runtime defaults so the sibling rule -- staging must sit outside the root the supervisor scans, or an app could be started before its grants are accepted -- stays enforced in one place. The reconciler also took a version argument it never used: it ran `pilotctl appstore install <id> --force`, which installs whatever the catalogue currently carries. The console pins the version the operator approved, so once the catalogue moved ahead the node would install software nobody approved, and reconcileOneApp's version comparison would never converge -- re-installing on every tick, every 30 seconds, indefinitely. install now takes --version and fails closed with version_unavailable when the catalogue cannot satisfy the pin, because the catalogue carries only each app's current release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Reconcile validated the install and staging roots but never created them. The supervisor creates the install root only when it starts with apps already present, and nothing creates the staging root at all, so a freshly adopted node failed on every tick: managed app reconcile failed: app staging root: stat ~/.pilot/apps-pending: no such file or directory No app could ever be installed on a new node. Found by running the real end-to-end install rather than by any unit test, because every existing test supplied roots that already existed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Alexgodoroja
changed the title
Alexgodoroja
enabled auto-merge (squash)
August 27, 2026 21:43
Three gosec findings inside the changed lines, and one pre-existing toolchain advisory that blocks the same gate. The two G204 subprocess findings are annotated rather than restructured, because the inputs are already constrained where it matters: every argument originates in an authority-signed desired document that readDesiredApps runs through FleetAppsDocument.Validate before the reconciler sees it, which bounds the app id to fleetAppIDPattern and the version to fleetAppVersionPattern. Neither pattern admits a shell metacharacter or a leading dash, the binary is the pilotctl shipped beside the daemon, and CommandContext invokes no shell, so neither argument injection nor command substitution is reachable. The G602 finding was a real if narrow readability problem: the bound was checked against i+1 and the value then read from args[i] after an increment. The value is now read from the index the check actually covers. govulncheck fails on five standard-library advisories in go1.25.12 -- none introduced here, all fixed in go1.25.13, and the platform repo already moved to that patch. Bumping the pin clears the gate without touching a dependency. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| fatalHint("invalid_argument", "usage: --version <exact-catalogue-version>", "--version needs a value") | ||
| return | ||
| } | ||
| wantVersion = args[i+1] |
Collaborator
Author
|
CI is green — all 14 checks pass. Ready for review. What was failing and how it was addressedgosec — 3 findings inside the changed lines (matching the "2 errors, 1 warning" the check reported):
govulncheck — 5 standard-library advisories in Retested after the fixes
Nothing in the functional behaviour changed: the end-to-end install, grant-hold, approve, start and remove path proven earlier still stands. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #456 — this branch is built on #456's commit, so it contains that work in full plus the three gaps that stopped it functioning. Merging this alone delivers the whole node half; #456 can be closed.
Proven end to end against the deployed console: a dev-built node adopted into a real workspace installed
io.pilot.aegis, held it unstarted pending grant review, started it on approval (pid=15504), and removed it cleanly.What #456 contributes
The reconciler itself: desired-set convergence, staged install outside the supervisor's scan root, manifest-declared grants as the only grant truth, demote on grant widening.
What this PR adds
1. Nothing could enable the feature.
HasAppReconcile()needs anappsblock in the managed control document.pilotctl enterprise adoptbuilds that document from the enrollment claim's options, which had exactly three fields — none for apps. Neither repo emitted an apps block anywhere outside a demo fixture, soappsEnabledwas false on every node ever adopted and #456's reconciler could never run.Adopt now emits the block when the authority grants the option, gated on fleet state (the desired document and observed report both travel that way). Install and staging roots are left to the runtime defaults so the sibling rule — staging must sit outside the scanned root, or an app could start before its grants are accepted — stays enforced in one place. Pairs with platform#66, merged.
2. The approved version was silently discarded.
Installtook aversionargument and never used it;pilotctl appstore installhad no version flag at all. So a node installed whatever the catalogue currently held — software its operator never approved — and once the catalogue moved past a pin,staged.AppVersion != spec.Versionstayed true and the node re-installed every 30 seconds indefinitely.installnow takes--versionand fails closed withversion_unavailable, because the catalogue carries only each app's current release and quietly serving a different build is the wrong answer for something that gates capability grants.3. The app roots were never created.
secureDirectoryvalidates but does not create. The supervisor creates the install root only when it starts with apps already present, and nothing created the staging root, so a freshly adopted node failed on every tick:No app could ever be installed on a new node. Found only by running the real install — every existing test supplied roots that already existed.
Tests
HasAppReconcileacross all five combinations, including nil-runtime37/37 packages.
🤖 Generated with Claude Code