Repository navigation
feat(kits): run oci-engine root startup scripts from the read-only host mount - #543
Conversation
…st mount Mark the oci-engine grant script and storage-driver helper readonly: true and pass them to the root startup command as whole argv tokens, so a consumer that mounts kit code read-only (Quickstart msb) can run them from that mount instead of an agent-writable copy. Each token is validated independently (accepted only under /var/lib/acq/host/kit-files/, no ".." and present), otherwise the command falls back to the root-owned copies the install step publishes, which is the previous behavior for consumers that ignore the flag. The grant script re-validates the helper it sources as root. Register readonly in the kit schema and make validate-kits reject any value other than the literal true/false and flow-style command lists, matching the consumer's line-based reader. Loud warnings and an updated verify script cover the case where neither the mount copy nor a published copy exists. zscaler-ca-certificate and opendesign are intentionally NOT marked: the zscaler cert is staged at start with no create-time root-owned copy to fall back to, and opendesign's only root command is an inline mkdir/chown. Tests run the real startup body, grant script and install script with shims. Co-authored-by: OpenCode Agent <bret.mogilefsky@gsa.gov>
validate-kits now rejects flow-style `command: [a, b]` lists because the acq consumer's line-based reader drops them. Two fixtures in test_validate_kits_ports.py used that form, so a kit meant to be valid failed. Rewrite them as block lists; the rule is unchanged. Co-authored-by: OpenCode Agent <bret.mogilefsky@gsa.gov>
wz-gsa
left a comment
wz-gsa
left a comment
There was a problem hiding this comment.
Fixed-head review of ccad8a7840e5b0185f48d3032665597146285dbd (static inspection only; three applicable required checks passed).
The kit marks the root startup grant script and sourced storage helper readonly: true, passes both as whole argv tokens, and refuses the agent-home token when selecting code for root execution. The offline fixture covers the two-path and missing-path cases. I have not verified an actual MSB read-only mount or rootless podman behavior; the PR correctly calls for a Patterns release, Quickstart pin bump, and live verification.
Integration gate: GSA-TTS/agentic-coding-quickstart#637 is the stated host-recorded read-only mount dependency and currently has an unresolved requested change: on staging failure, its MSB consumer can still fall back to a guest copy. Please resolve that path and verify the paired release/pin on a live MSB backend before treating this change as a complete tamper-resistance improvement. The fallback-for-older-consumers behavior here is deliberate but is not evidence of an enforced read-only mount.
This is a review comment, not approval or a merge authorization. AI-assisted static review (OpenCode); revisit after the dependency and live evidence are available.
wz-gsa
left a comment
wz-gsa
left a comment
There was a problem hiding this comment.
Fixed-head review of ccad8a7840e5b0185f48d3032665597146285dbd (static only; applicable required checks green). The earlier comment identified the required release/pin and live MSB verification. There is also a security-relevant fail-open path to correct in this PR:
integrations/isolation/acq-kits/oci-engine/spec.yaml:193-205 and files/home/oci-engine-grant-devs.sh:68-79: When the backend has selected /var/lib/acq/host/kit-files/... for root startup code but that path is missing, pick() falls back to /usr/local/lib/acq/oci-engine; the root grant script likewise falls back to its root-owned storage helper if the selected mount helper is unavailable. Those copies originated from /home/agent during create and are not host-authoritative across restarts. In a sandbox with a recorded read-only selection, missing/malformed mount files must fail closed (no root startup execution); preserve the root-owned fallback only for an explicitly recognized legacy sandbox without read-only selection. Add offline fixtures for both missing selected mount tokens and legacy no-selection behavior. This also avoids masking a failure of the paired Quickstart read-only staging path (GSA-TTS/agentic-coding-quickstart#637).
The actual mount enforcement and device/storage behavior still require the documented live MSB verification after the Patterns release and Quickstart pin bump; none was performed in this review. AI-assisted static review (OpenCode); no reviewed-repository code executed.
The oci-engine root startup fell back to the root-owned /usr/local/lib/acq/oci-engine copy when the backend had selected a path on the read-only mount but the file was missing. Those copies came from /home/agent at create time, so they are not host-authoritative across restarts. The grant script did the same for its storage helper. - Startup now classifies the two tokens together. Both on the mount: run them, or refuse (stderr error, exit 1) if either file is missing. Both /home/agent: the explicit legacy case, root-owned copy as before. Anything else, including a mixed pair or `..`: refuse. The earlier mounted-path-missing fallback is removed. - oci-engine-grant-devs.sh refuses a missing selected mount helper and any path outside the mount or the root-owned directory; no silent fallback. - scripts/verify fails when readonly was honored but a mount file is missing, or when only one file is on the mount. - Docs: table of token cases, why a non-zero exit is safe (Quickstart reports a failing startup command as a warning), the pairing with Quickstart#637, and the mount-enforcement assumption. - Tests: offline fixtures for each case, with mutation checks. Co-authored-by: OpenCode Agent <bret.mogilefsky@gsa.gov>
|
Thanks for the review. The fail-open path is fixed in 3743330 and the follow-ups, pushed on this branch:
Not done and not claimed: any live MSB run (tracked in GSA-TTS/agentic-coding-quickstart#639), and the release plus pin bump. The PR description is updated to match. AI-assisted (OpenCode). |
wz-gsa
left a comment
wz-gsa
left a comment
There was a problem hiding this comment.
Rereview of head 374333080902bfe524de69aaf09cc97416b3840c (static inspection; required checks pass). The root startup command now refuses missing selected read-only mount paths, mixed/unrecognized tokens and traversal; the sourced helper independently rejects a missing selected mount helper. Explicit legacy tokens retain the prior root-owned-copy behavior, and offline fixtures cover these branches. This resolves my requested fail-closed correction in this kit. Approval is for the reviewed Patterns change, not a claim that the mount is enforced on a live backend or authorization to merge: the paired GSA-TTS/agentic-coding-quickstart#637 path, Patterns release/Quickstart pin update, and live MSB verification described in the PR remain integration gates. AI-assisted static rereview (OpenCode); no reviewed-repository code executed.
…ker and legacy-sandbox coverage (#637) ## Summary Stacked on [Quickstart PR #611](#611); it cannot merge before #611. Continues the host-authoritative lifecycle migration under #503 and #636. - **SBX built-in kit heal trusts host state.** The Zscaler CA, USAi, and playbook kits were skipped when a guest `sbx exec` probe reported their files present, which a sudo guest can forge. A host config `builtin-kits` record (`kit ref`) is now seeded at create and updated on heal success; unrecorded or stale entries are re-applied, and `ACQ_FORCE_KIT_REAPPLY` is unchanged. The probe is a debug-only diagnostic. - **MSB forged-marker tests** (test-only): forged guest values for agent, workspace, SSH socket, kit env, install gate, and agent-user-ready do not change behavior. - **MSB read-only host-config decision is host-recorded.** `ro-host-config` is written only after a create that passed the `:ro` volume flag and became exec-ready; a missing record takes the existing guest-copy path with its warning, and the guest probe is diagnostic only. The record does not prove the guest mount works, so when it disagrees with the probe the heal fails closed (documented in ADR-0035). - **Accepted residual risks and decisions, documented not coded:** the MSB passwd shell and the SBX startup barrier (known-failure-modes sections 46 and 47), SBX trusted-startup parity (ADR-0036, proposed), and the session-state acceptance mapping (ADR-0035). - **Legacy-sandbox docs and tests**, and a fix to the stale `verify-backends` OCI marker name (`oci-engine-ready`, as written by the pinned kit). ## Plan / Context From a read-only source audit of the post-#611 lifecycle (gaps G1, G2, G4, G5, G6, G8). The Patterns half of the root-startup-script finding is [Patterns PR #543](GSA-TTS/agentic-coding-patterns#543); after its release a pin-bump commit lands here. ## Verification - `bash -n` on changed scripts: pass. - Focused bats (83, 79, 90, 85, 95, 30, 140, 72, 81, 82, 78, 115, 145; 291 tests): 0 failures, run in a scratch copy with vendored bats. - Mutation checks: reverting to guest-probe authority, dropping the create-time seed, dropping the `builtin-kits` clear, and seeding USAi regardless of provider selection each fail a test. - Independent adversarial reviews (two rounds on the later slices): no blockers or majors remaining; findings fixed. Not run locally: shellcheck, markdownlint, two env-dependent tests; CI to confirm. ## Rollback Revert this PR; the heal returns to guest-probe decisions. ## Security impact A sudo guest can no longer suppress or fake the built-in kit setup gates. Sandboxes created before the record exists re-apply on next heal; on sbx 0.38+ that is refused, so they show the recreate notice until recreated. AI-assisted (OpenCode). Human review required. Co-authored-by: OpenCode Agent <bret.mogilefsky@gsa.gov> --------- Co-authored-by: OpenCode Agent <agent@gsa.gov> Co-authored-by: OpenCode [claude_4_6_sonnet] <bret.amogilefsky@gsa.gov>
🤖 I have created a release *beep* *boop* --- ## [1.18.0](v1.17.0...v1.18.0) (2026-10-28) ### Features * **kits:** run oci-engine root startup scripts from the read-only host mount ([#543](#543)) ([94f807a](94f807a)) * **opencode:** carry reasoning and input-modality metadata in the neutral catalog ([#542](#542)) ([918e46d](918e46d)) ### Bug Fixes * **devenv-image:** take devenv 2.4.0 from a pinned nixos-unstable rev ([#550](#550)) ([ae0a921](ae0a921)) * **opencode:** allow five-minute stream idle gaps ([#533](#533)) ([9fe6440](9fe6440)) * **paseo-kit:** improve startup readiness and CLI discovery ([#535](#535)) ([3fcab2d](3fcab2d)) * **paseo:** bump pinned CLI to 0.13.0 ([#536](#536)) ([a290d5d](a290d5d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
Lets a Quickstart MSB consumer run the oci-engine kit's root startup scripts from a host-authoritative read-only mount instead of agent-writable copies (the Patterns side of audit finding G3 under GSA-TTS/agentic-coding-quickstart#503). The oci-engine grant script and storage-driver helper are marked
readonly: trueand passed to the root startup command as whole argv tokens.Context
A sudo guest can edit the guest-resident files that root re-runs on every MSB heal. Quickstart's read-only mount mechanism (
_acq_msb_stage_readonly_file, present from v4.0.2) rewrites exact argv tokens to/var/lib/acq/host/kit-files/...for kits that opt in, but no shipped kit did. Pairs with the MSB host-recorded:rodecision in Quickstart PR #637, which removes the guest probe from that decision. If the host fails to stage one of the two files, the pair is mixed and is refused for that boot.Changes
oci-engine:readonly: trueon the grant script and storage helper. The root startup command classifies its two tokens together. Both under/var/lib/acq/host/kit-files/(no..): run those files, or refuse (stderr error, exit 1) if either is missing; it never falls back to the root-owned copy. Both/home/agent/*: the legacy case for a consumer that ignores the flag; it uses the root-owned copy published at create time, as before. Anything else, including a mixed pair: refuse. The grant script re-validates the helper it sources as root and refuses a missing selected mount helper. A refusal costs only that boot's device grant and storage re-evaluation; Quickstart reports a failing startup command as a warning and continues.readonlyregistered inschemas/kit-hybrid-v1.schema.json;validate-kits.pyrejects any value but the literaltrue/falseand flow-stylecommand:lists (the consumer's line-based reader ignores or drops them).zscaler-ca-certificateandopendesignare intentionally not marked (reasons in their READMEs): the zscaler cert is staged at start with no create-time root-owned copy to fall back to, and opendesign's only root command is an inlinemkdir/chown.scripts/verifycover "neither the mount copy nor a published copy exists"; the install phase's trust in/home/agentat create time is documented.Verification
scripts/tests(715 passed),validate-kits --strict,make validate,make generate-check, markdownlint on the changed docs...check, a refusal that exits 0, the grant script accepting/home/agent) each fail a test.make ci, shellcheck, any live msb/sbx sandbox.Rollout
Needs a Patterns release and a
PATTERNS_KIT_REFpin bump in Quickstart. Older Quickstart releases (v4.0.0 to v4.0.2) and sbx ignore the flag and use the root-owned fallback, so nothing regresses there. Live MSB check after the bump: argv carries/var/lib/acq/host/kit-files/..., no scripts remain in/home/agent, device grants and storage driver still apply, a write to the mount fails (EROFS), and an older sandbox with no:romount takes the fallback with the warning. Also confirm storage is configured before first use now that install no longer seedsstorage.confwhen the flag is honored.Rollback
Revert this PR.
Security impact
Removes a path where root could execute a guest-writable or create-time copy after the backend had selected the read-only mount. Residual: the install step still trusts
/home/agentat create time, the root-owned legacy copies are guest-tamperable undersudo(so the legacy path is not tamper-resistant, unchanged from before), and the read-only property depends on the backend's mount enforcement. AI-assisted (OpenCode); human review required.Co-authored-by: OpenCode Agent bret.mogilefsky@gsa.gov