Skip to content

docs(adr): add ADR-0035 host-authoritative sandbox configuration - #504

Merged
wz-gsa merged 15 commits into
mainfrom
docs/adr-host-authoritative-config
Oct 9, 2026
Merged

wz-gsa merged 15 commits into
mainfrom
docs/adr-host-authoritative-config

Conversation

@mogul

@mogul mogul commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Context

Adds ADR-0035 (Host-Authoritative Sandbox Configuration). This is a docs-only PR (the ADR itself); the acq-side implementation is tracked in #503.

acq deliberately gives the in-sandbox agent passwordless sudo (the sandbox is the security boundary, per ADR-0005/0011). A consequence not previously stated as a principle: inside the guest, root-ownership is not a trust boundary against the agent — a prompt-injected agent can sudo to root and rewrite any guest file, including markers under the root-owned /var/lib/acq/.

acq currently reads four content-bearing /var/lib/acq markers back and acts on them to configure the sandbox (agent, workspace, ssh-auth-sock, kit-env), and re-runs guest-resident kit startup code across restarts. Both are guest-tamperable under sudo.

This surfaced concretely in review of GSA-TTS/agentic-coding-patterns#423 (neutral model-provider discovery), whose provider-facts channel had originally claimed a root-owned guest path was "not agent-writable."

What this ADR records

  • Principle: acq must not rely on guest-generated or guest-tamperable state to configure the sandbox. Trusted inputs (data and code) are host-resident + mounted read-only, or host-held + injected at exec time — never read back from the guest and treated as authoritative.
  • Mechanism 1: per-sandbox host config dir mounted read-only into the guest.
  • Mechanism 2: trusted startup execution invoked from the read-only mount (three options considered; option 1 selected).
  • Migration: stage the existing /var/lib/acq markers off guest-writable paths (tracked in acq: host-authoritative sandbox config — migrate guest-writable state off /var/lib/acq (ADR-0030) #503).
  • Two Mermaid sequence diagrams (GitHub/Host/Guest boxes) contrasting the rejected guest-write path with the chosen host-materialized read-only path.

Verification

  • markdownlint-cli2 (repo config, v0.22.1): 0 issues.
  • Both Mermaid diagrams parse cleanly (mermaid v11).
  • Referenced ADRs (0005, 0011, 0017, 0021, 0023, 0027, 0029) all exist.

Rollback

Docs-only. Revert the single commit / delete docs/adr/0035-host-authoritative-sandbox-config.md. No code paths change.

Security impact

Documentation of a security-hardening direction; no runtime change in this PR. The implementing change (#503) narrows what acq trusts; it does not alter the agent's in-sandbox privileges.


AI-assisted (OpenCode). See the commit's Co-authored-by trailer.

Records the principle that acq must not rely on guest-generated or
guest-tamperable state to configure the sandbox: because the in-sandbox agent
has passwordless sudo, guest root-ownership is not a trust boundary against a
prompt-injected agent. The only enforceable boundary is host<->guest.

acq-trusted inputs (data AND code) must be either host-resident and mounted
read-only into the guest, or held on the host and injected at exec time --
never read back from the guest and treated as authoritative.

Introduces a per-sandbox host config directory mounted read-only, and trusted
startup execution invoked from that read-only mount (three options considered;
option 1 -- mount kit code read-only and invoke from the :ro path -- selected).
Documents the staged migration of the existing /var/lib/acq markers (agent,
workspace, ssh-auth-sock, kit-env, and the presence gates) off guest-writable
paths. First consumer: patterns ADR 0003 (neutral model-provider discovery).

Includes two Mermaid sequence diagrams (GitHub/Host/Guest boxes) contrasting
the rejected guest-write path with the chosen host-materialized read-only path.

Co-authored-by: OpenCode [claude_4_8_opus] <bret.mogilefsky@gsa.gov>
…t (ADR-0030)

Introduce a host-side per-sandbox config store (acq_host_config_* in common.sh)
keyed like the provenance/cli-kits records, and mount it into the msb guest
READ-ONLY at ACQ_HOST_CONFIG_GUEST_DIR (default /var/lib/acq/host) via
--volume host:guest:ro. This is the mechanism ADR-0030 defines: acq's trusted
per-sandbox config lives on the host, and where the guest must read it, it sees
a read-only mount the VMM enforces — a passwordless-sudo agent can read but
cannot forge it.

This commit adds only the store + mount plumbing and terminate-time cleanup on
both backends; subsequent commits migrate the individual /var/lib/acq markers
(agent, workspace, ssh-auth-sock, kit-env, and the presence gates) off the
guest-writable paths onto this store.

Refs: #503

Co-authored-by: OpenCode [claude_4_8_opus] <bret.mogilefsky@gsa.gov>
…ig store (ADR-0030)

Move the four content-bearing /var/lib/acq guest markers onto the
host-authoritative config store so a passwordless-sudo agent can no longer
tamper the values acq reads back to configure the sandbox:

- agent      -> which binary a name-only re-attach launches
- workspace  -> the session cwd (-w)
- ssh-auth-sock -> the injected SSH_AUTH_SOCK (git/ssh signing)
- kit-env    -> kit environment[] replayed as -e NAME=value each session

acq now writes these to acq_host_config_* on the host at provision/apply and
reads them back from the host on every session verb (run/attach/shell) — no
guest `msb exec cat` happens for them. The kit-env NAME re-validation and the
agent safe-token guard are preserved for defense-in-depth. Session verbs still
survive an absent value under set -euo pipefail (the host read returns empty,
not a failing pipeline).

Tests: the offline stub library gains seed_host_config (seeds the host store
from the STUB_RECORDED_* knobs, exported into subshells); the guest-cat stub
arms for these markers are removed; provision assertions read the host store.

Refs: #503

Co-authored-by: OpenCode [claude_4_8_opus] <bret.mogilefsky@gsa.gov>
Move the four presence-only idempotency gates off guest /var/lib/acq markers
onto the host-authoritative config store, so a passwordless-sudo agent can no
longer pre-create a marker to SUPPRESS a provisioning step:

- install-<cksum>        (per kit install command, run-once)
- agent-installed-<agent> (agent npm install, run-once)
- agent-user-ready       (agent user + sudoers setup)
- oci-ready              (podman/OCI engine install)

Each gate is now acq_host_config_has/_write on the host instead of a guest
`test -f`/`touch`. Behavior is otherwise unchanged (still fail-soft, still
re-runs the cheap idempotent device-node grant / shell-sync outside the gate).

Tests: seed_host_config_gates seeds agent-user-ready/oci-ready from
STUB_AGENT_USER_READY/STUB_OCI_READY; the guest test -f arms for these markers
are removed from the stub; gate assertions read the host store.

With this, no /var/lib/acq guest marker is written or read by acq anymore.

Refs: #503

Co-authored-by: OpenCode [claude_4_8_opus] <bret.mogilefsky@gsa.gov>
…030)

Implements ADR-0030 Mechanism 2. A kit files[] entry marked `readonly: true`
carries trusted CODE that acq must run from the host-authoritative read-only
mount, not a guest-writable copy a passwordless-sudo agent could tamper between
restarts.

- kit_spec_files surfaces a 5th tab field (readonly), default empty — 4-field
  consumers and unflagged files are unaffected.
- _acq_msb_apply_kit_dir stages a readonly file onto the per-sandbox host config
  dir (kit-files/<path-slug>.<crc>, cp 0555) instead of `msb copy`ing it into the
  guest, and records a guest-path -> :ro-path rewrite.
- _acq_msb_run_commands and the staged --script-path body
  (_acq_msb_startup_body_into) rewrite any startup argv token that EXACTLY equals
  a readonly file guest path to its :ro path (whole-token match; a --flag data
  path is left alone). So both the exec heal and a native replay run the :ro copy.
- Identification is declared (readonly: true), never inferred from argv, so a
  data file the command also reads (opencode.jsonc) stays a normal guest drop.

Backward compatible: no readonly files -> empty rewrite table -> unchanged
behavior. Adoption of the field by the patterns-repo kits (usai-provider merge
script, ADR 0003 provider normalizers) is additive and independently deployable.

New suite 79-msb-readonly-kit-files.bats covers field surfacing, host-staging vs
guest-copy, argv rewrite on both the exec and staged-body paths, whole-token
matching, and the :ro mount.

Refs: #503

Co-authored-by: OpenCode [claude_4_8_opus] <bret.mogilefsky@gsa.gov>
…ration (ADR-0030)

Update the design-note and doc references that named the guest /var/lib/acq
markers (agent, workspace, ssh-auth-sock, kit-env, install-*, oci-ready) to
reflect that ADR-0030 moved them to acq's host-authoritative per-sandbox config
store. Behavior is unchanged; only the storage location and its trust boundary
moved. No bare tracker refs added.

Co-authored-by: OpenCode [claude_4_8_opus] <bret.mogilefsky@gsa.gov>
@mogul

mogul commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Update — implementation of the migration (#503) added to this PR

This PR now carries both ADR-0030 and its implementation on the msb backend, per the request to land the ADR with the migration. The ADR is no longer docs-only.

Commits (one per change, as planned in #503)

  1. docs(adr): add ADR-0030 (the principle, threat model, considered options, migration plan).
  2. feat(acq): host config store + :ro mount plumbing — new acq_host_config_* helpers in common.sh (per-sandbox host dir keyed like the provenance/cli-kits records), mounted into the msb guest read-only at /var/lib/acq/host via --volume host:guest:ro; terminate-time cleanup on both backends; Windows path forms (ADR-0029).
  3. feat(acq): migrate agent / workspace / ssh-auth-sock / kit-env off the guest /var/lib/acq markers onto the host store — acq writes them host-side and reads them back from the host on every session verb; no msb exec cat for them anymore. Charset/token guards preserved; session verbs still survive an absent value under set -euo pipefail.
  4. feat(acq): migrate the run-once gates (install-<cksum>, agent-installed-<agent>, agent-user-ready, oci-ready) to the host store, so a sudo guest can no longer forge a marker to suppress a provisioning step. After this, no /var/lib/acq guest marker is written or read by acq.
  5. feat(acq): trusted startup execution (Mechanism 2) — a kit files[] entry marked readonly: true is staged onto the host config dir (kit-files/<slug>.<crc>, not copied into the guest) and the invoking startup argv token is rewritten to its :ro-mount path (whole-token match; both the exec heal and the staged --script-path body). Identification is declared, not inferred, so a data file the command also reads (opencode.jsonc) stays a normal guest drop.
  6. docs: reconcile ADR-0011 / ADR-0021 / KNOWN_FAILURE_MODES references with the new storage location.

Verification

  • Offline bats suite (bats-core, stubbed sbx/msb/ssh): all 31 files pass, 0 failures. New suite 79-msb-readonly-kit-files.bats (4 tests) covers the readonly field surfacing, host-staging vs guest-copy, argv rewrite on both paths, whole-token matching, and the :ro mount. Existing msb suites (70–78, 115) updated for the host-store storage and green.
  • bash -n clean on acq + all acq.backends/*.sh + the stub library.
  • markdownlint-cli2 (repo config): 0 issues on the touched docs.
  • Not run locally (unavailable on this host): shellcheck (no binary; .shellcheckrc batching documented in AGENTS.md) and the live verify-backends (needs a sandbox-capable host — cannot run inside a sandbox). These remain for CI / a live host.

Cross-repo dependency (tracked)

The readonly: true files[] field is additive to the neutral hybrid/v1 kit spec and must be adopted by the GSA-TTS/agentic-coding-patterns kits for their trusted code (usai-provider's merge-global-config.mjs; ADR 0003's provider normalizers). The two sides are independently deployable: acq shipping first is inert until a kit opts in; a kit shipping the field first is ignored by older acq (the file still copies into the guest). This is captured in #503 and noted in ADR-0030.

Rollback

Revert the 5 feat/docs commits (leaving the ADR) or the whole branch. The changes are additive and default-off for any kit that doesn't set readonly:; a sandbox provisioned by an older acq (guest markers) is handled by the absent-value fallbacks.

Security impact

Removes acq's reliance on guest-writable state for sandbox configuration under passwordless sudo (AC-6, SI-7, SI-17). Passwordless sudo itself is unchanged — this narrows what acq trusts, not what the agent can do in its sandbox.

AI-assisted (OpenCode). See the commits' Co-authored-by trailers.

CI pre-commit shellcheck flagged two errors introduced by the ADR-0030 work:

- SC2316 (acq.backends/msb.sh): `local ... readonly ...` applies `local` to a
  variable named `readonly`, which shellcheck reads as the readonly builtin.
  Rename the loop variable to `_readonly`.
- SC2144 (test/bats/72, 78): `[ ! -f "$DIR"/msb/<name>.*.config/<key> ]` uses a
  glob with -f, which does not work as intended. Replace with a globbed `ls`
  under `run bash -c` asserting empty output (the key is absent).

Behavior unchanged; offline bats suite still green (72/78/79/73/115).

Co-authored-by: OpenCode [claude_4_8_opus] <bret.mogilefsky@gsa.gov>
@wz-gsa

wz-gsa commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

AI-assisted review (OpenCode), advisory — needs a human to confirm before it drives a change. Reviewed at head 4343c4e, main at 8e6160c. (I first read a699af38; the two commits since are a shellcheck SC2316/SC2144 cleanup — readonly renamed to _readonly, and two [ ! -f glob ] tests converted to run ls + assert_output ''. Every finding below was re-verified at 4343c4e and every line cite re-checked.)

This is the right principle and the migration is careful. I went looking for the ways a host-authoritative mechanism usually fails and mostly did not find them — details in the verified list below, because "I checked X and it holds" is worth as much here as a finding. One substantive issue, and one documentation gap where I was wrong and the live test says so.

The :ro claim holds — I tested it

The ADR rests its whole security property on the VMM honouring :ro, and msb create --help on 0.7.2 does not list ro among --volume's OPTIONS (only uid=/gid= and quota=). So I built the exact flag shape this PR emits and attacked it:

$ msb create --name rotest --volume "<host>/cfg:/var/lib/acq/host:ro" \
      docker.io/docker/sandbox-templates:shell-docker

# in-guest:
/proc/mounts   -> var_lib_acq_… /var/lib/acq/host virtiofs ro,relatime 0 0
write as agent -> refused
write via sudo -> "cannot create …: Read-only file system"
sudo mount -o remount,rw /var/lib/acq/host
               -> command SUCCEEDS, /proc/mounts then shows rw
               -> writes STILL fail "Read-only file system"
host-side file -> unchanged

The remount is the interesting part: a sudo-capable agent can flip the guest-visible mount flag to rw, and writes still fail, because virtiofs is exported read-only on the host side. The guest-visible flag is cosmetic. Control: the same mount without :ro let the guest write and the host file changed — so :ro is what did the work, not luck.

So this is a docs nit, not a mechanism problem: the ADR's Negative/risks section (docs/adr/0030-host-authoritative-sandbox-config.md:293-302) should name the dependency and pin the msb version where :ro on --volume is contractual, since the CLI help does not advertise it. Cheap insurance against a future msb that parses options differently.

Should-fix: stale gate markers now survive destroy-and-recreate

Moving the presence-only gates host-side fixes forgery but introduces a failure mode the guest markers could not have — they died with the guest filesystem, and host state does not.

acq_host_config_dir (acq.backends/common.sh:1546-1554) keys on (backend, sanitized name, cksum of raw name) and nothing else. I transcribed it and confirmed two successive calls for the same sandbox name return an identical path — no instance id, boot id, or creation timestamp.

The gates living there are install-<cksum>, agent-installed-<agent>, agent-user-ready, oci-ready, read at acq.backends/msb.sh:1989, :3567, :3622, :4244. If a sandbox is removed outside acq — msb rm directly, a path msb.sh:4715's own comment acknowledges — or if acq_host_config_remove fails, then a new sandbox created with the same name inherits the old markers and skips provisioning steps that genuinely need to run. The install gate is keyed by the command's cksum, so an identical kit produces identical marker names and hits every time.

Failure shape: a fresh sandbox that silently never ran its install commands, with acq_debug reporting "already done (marker hit)". That is the degraded-path rule inverted — the gate reports work complete that never happened in this sandbox.

Suggested fix: bind the config dir (or just the gate keys) to a sandbox instance identity, or clear the gate keys at the start of provisioning. Either makes the marker mean "this sandbox did this" rather than "some sandbox with this name once did this". rg -i 'stale|recreat|same name|outside acq' over the ADR returns nothing, so the risk list should pick it up too.

Nit: two "world-unreadable" claims are stronger than the code

common.sh:1567 says the store is "world-unreadable on the host", and msb.sh:3187-3188 does mkdir -p … || true then chmod 700 … || true with the create proceeding regardless. The failure direction is safe — a missing dir makes acq_host_config_has read absent (re-provision) and acq_host_config_read return empty (documented fallbacks) — so this is not the usual silent-success defect. But a failed chmod leaves the dir at the ambient umask while the comment promises otherwise.

Verified as holding (evidence, not vibes)

  • The guest-read path is genuinely gone. rg -c 'var/lib/acq' acq.backends/msb.sh → 1, and that hit is a comment (:1625). The only remaining inbound msb exec read is :3883 command -v socat, a capability probe, not config. All four content markers now read from the host store: :4017 ssh-auth-sock, :4070 kit-env, :4464 workspace, :4616 agent.
  • Charset guards were kept, not dropped on a "the host store is trusted now" theory. msb.sh:4617 still runs _acq_msb_safe_agent_token before the value reaches sh -c "command -v '$agent'", and the comment says why. kit-env keeps its awk key filter ^[A-Za-z_][A-Za-z0-9_]*$ (:4079). That matters because kit-derived values are not operator-authored — a kit can be fetched from a remote git URL.
  • The kit-env eval is safe. eval "$_arrn+=(-e \"\$_line\")" (:4075) is exactly where this would break, so I transcribed it and fed it hostile kit-derived values: $(touch …), backticks, and x";touch …;#. All three landed as literal array elements; no marker file was created.
  • Mechanism 2 leaves no guest-writable copy. msb.sh:1596-1612 is an if/elif, so readonly: true takes the staging branch and skips _acq_msb_copy_file_verified entirely. Test 79:99-100 asserts the staged file exists before :102 refutes the msb copy — a real positive precondition, not a vacuous refute.
  • rm -rf guard is adequate. acq_host_config_remove (common.sh:1662-1665) matches *.config only, after excluding ""////*/.
  • Cleanup runs on both backends (msb.sh:4731, sbx.sh:634), and the early return $_rc above it fires only when acq_backend_exists is still true — so it is not a leak path.
  • The migration tests assert positive host-store observables, not merely the absence of the old guest command: 115:149,:320 ssh-auth-sock, 78:43 kit-env, 72:98 agent, 72:222,:266 oci-ready, 71b:288,:293 workspace. A pure negative would not have proven the migration works, and this avoids that.
  • No vestigial marker helpers left behind; no dead guest-path fallback in the migrated readers. bin/injection-scan.sh clean.

QA gap

No test covers the new store's degraded paths: host config dir creation failing, or a stale gate marker from a previous sandbox of the same name (the should-fix above). The absent-key direction is covered (78-msb-kit-env.bats:76-77, 72-msb-agent-user.bats:283-284), which is the harder half to remember — and 4343c4e just made both of those assertions stricter, replacing [ ! -f <glob> ] (which SC2144 flags, and which a multi-match glob makes unreliable) with run ls + assert_output ''. Good catch by whoever ran shellcheck.


Method: static analysis at a699af38; the :ro result is from a live msb create on msb 0.7.2 with a throwaway fixture, both probe sandboxes removed afterwards (msb remove --force). No PR code was executed — the eval and acq_host_config_dir results are from my own transcriptions of those functions. UNVERIFIED: whether sbx will need the same mechanism; the PR's own comment at sbx.sh:631-634 calls its cleanup "a defensive no-op unless a future sbx path writes host config".

Address adversarial review findings on PR #504:

- Make the host-config mount root traversable by the guest agent (0711) while
  keeping flat acq-internal keys private (0600). readonly kit payloads under
  kit-files/ are now in a traversable directory and staged 0555, so startup code
  rewritten to /var/lib/acq/host/kit-files/... can actually be read/executed by
  user=agent through the :ro mount.
- Detect existing msb sandboxes that predate the ADR-0030 /var/lib/acq/host:ro
  mount. If the mount is absent or not traversable, do not rewrite readonly
  startup argv to a missing path; warn and fall back to the legacy guest copy so
  old sandboxes keep working until recreated.

Tests: extend 79-msb-readonly-kit-files for traversability modes and legacy
fallback. Targeted bats suites 79/72/78/73/115 pass locally.

Refs: #503

Co-authored-by: OpenCode [claude_4_8_opus] <bret.mogilefsky@gsa.gov>
@wz-gsa

wz-gsa commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Follow-up at head b61c3f9 (I reviewed 4343c4e yesterday). AI-assisted, advisory. The new commit is fix(acq): make readonly host-config startup code usable and legacy-safe — I re-read it specifically for whether the fix introduced new surface, and for whether my earlier should-fix survived.

The legacy fallback is done right

_acq_msb_host_config_mount_available (msb.sh:1675-1682) probes as the agent user, not root, and requires -d + -r + -x — so it tests the thing that actually matters (can the consumer traverse it) rather than that the directory exists. The fallback then warns per file, names the consequence, and says how to recover:

sandbox '<name>' has no readable ADR-0030 host-config mount; copying readonly kit file '<path>' into the guest for compatibility. Recreate the sandbox to get tamper-resistant readonly startup code.

That is the degraded-path rule applied correctly: the weaker mode is reachable, loud, and distinguishable from the strong one. The staging-failure branch now also falls back rather than leaving a startup argv pointed at a path that doesn't exist, which was the real bug.

New: 0711 on the host dir widens host-side exposure, and the ADR says only half of it

msb.sh:1705 and :3211 change the per-sandbox host-config dir from 0700 to 0711, with kit-files/ at 0755. The ADR's new line explains the guest side accurately:

The mount root is traversable but not listable (0711) so the guest agent user can open/execute read-only kit payloads below kit-files/, while flat acq-internal keys stay host-private (0600).

The guest reasoning is sound and I verified the flat keys really are 0600 (common.sh:1477, :1593, :1644). But this directory lives on the host, under ACQ_STATE_DIR, and 0711 + 0755 means any local host user can now traverse into kit-files/ and read the 0555 payloads (msb.sh:1709). At 0700 they could not.

Kit payloads are public repo content, so the confidentiality loss is ~zero today. What concerns me is the precedent: kit-files/ is the directory whose whole purpose is holding trusted code that the guest must not tamper, and it is now the one part of the acq state tree that isn't owner-only. If a future kit stages anything sensitive under readonly: true, the mode is already permissive and the ADR's note ("host-private (0600)") reads as though the whole tree still is.

Two things worth doing: say in the ADR that 0711/0755 is a host-visibility relaxation and not just a guest-traversal enabler, and consider whether the guest-side need can be met with 0711 on kit-files/ too (traversable, not listable) rather than 0755 — the guest opens known paths, so listability isn't required.

Still open from my earlier review: stale gate markers

Unchanged at this head. acq_host_config_dir still keys on (backend, name, cksum-of-name) with no instance identity (common.sh:1294, and the same derivation at :1441/:1557), and the four gate reads are still msb.sh:2012, :3590, :3645, :4267. A sandbox removed outside acq, or a failed cleanup, still lets a same-named successor inherit install-<cksum> / agent-*-ready / oci-ready and skip provisioning that genuinely needs to run. rg -i 'stale|recreat|same name|outside acq' over the ADR still returns nothing.

Not a blocker on its own — the failure is a sandbox that silently never ran its install steps, which is recoverable — but it is the one finding from yesterday that no commit has touched.

Cross-PR note

Patterns #437 (the rootless-podman OCI-engine kit) is the first consumer that would want Mechanism 2, and it currently declares no readonly: true file while having two root startup phases source a helper from /home/agent/oci-engine-config/. I've raised that there; flagging here because #504's Mechanism 2 is the mechanism that fixes it, so the two land better together than apart.


Method: diffed 4343c4e..b61c3f9 and re-read the changed regions plus my earlier findings' line cites at the new head; all re-verified. No PR code executed.

Clear stale host-side instance state before fresh msb provisioning, verify the host-config mount refuses guest-root writes before trusting readonly staged kit code, and tighten staged kit-file directory listability.

Co-authored-by: OpenCode [gpt_5_5_default_v2] <bret.mogilefsky@gsa.gov>
@mogul

mogul commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Update: addressed the review feedback in e9c6183.

Changes since the last review:

  • Documented the msb ":ro" dependency in ADR-0030 and noted that acq now probes guest-root write refusal before trusting the host-config mount for readonly startup code.
  • Hardened the readonly mount availability check: it now requires agent traversal/read access and verifies guest root cannot create a probe file. If that fails, readonly kit files fall back to the legacy guest-copy path with a warning instead of being treated as tamper-resistant.
  • Changed staged readonly kit-files directories from 0755 to 0711, so known paths remain executable/readable while directory listing is not allowed.
  • Cleared stale host-side instance state at fresh msb provision, covering run-once gates plus conditionally-written workspace and ssh-auth-sock values from same-named sandboxes removed outside acq.
  • Tightened host-config flat key creation with umask 077 so new 0600 keys do not depend solely on a later chmod.
  • Added regression coverage for stale instance-state clearing, writable-mount fallback, and the 0711 kit-files mode.

Verification:

  • bash -n acq acq.backends/common.sh acq.backends/msb.sh acq.backends/sbx.sh acq.backends/kit-translate.sh acq.backends/secret-store.sh scripts/test-acq-lib.sh
  • shellcheck --severity=warning batch over acq, backend scripts, test scripts, and bats files
  • ./scripts/test-acq-bats test/bats/72-msb-agent-user.bats test/bats/79-msb-readonly-kit-files.bats: 45 tests passed
  • ./scripts/test-acq-bats: 518 tests passed
  • npm run lint:md -- docs/adr/0030-host-authoritative-sandbox-config.md: 0 issues
  • gitleaks detect --no-git -v --source .: reports existing false positive BASH_SIGNING_KEY_FPR in .github/workflows/bash32-compat.yml, outside this diff
  • diff-only gitleaks scan of the current patch: no leaks found

Please re-review when you have a chance.

The fresh-provision stale-state regression must prove a stale ssh-auth-sock value is cleared, but it should not require the CI host to expose SSH_AUTH_SOCK and create a replacement value.

Co-authored-by: OpenCode [gpt_5_5_default_v2] <bret.mogilefsky@gsa.gov>
@mogul

mogul commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Heads-up: ADR numbering contention with the msb version-pin work

While investigating why acq users end up on an unsupported msb, we found that GSA-TTS/agentic-coding-quickstart#510 needs an ADR of its own, and this PR has already claimed docs/adr/0030-host-authoritative-sandbox-config.md. To stay out of the way, the version-pin work will take ADR-0031 (pinned msb distribution and the blocked-version window) rather than renumbering anything here. Flagging it so nobody independently grabs 0031 in the meantime — if this PR's number shifts before merge, #510 will follow rather than the reverse.

For context on what #510 turned into: the original "block msb 0.7.0-0.7.2" guard was correct about the hazard but could not actually deliver a supported version. The per-tag install.sh release assets are byte-identical and resolve the version at runtime from releases/latest, so downloading the v0.6.18 asset installs 0.7.2 — which acq_backend_prepare then refuses. The superradcompany Homebrew tap ships a single always-latest microsandbox.rb, and Formula/acq.rb depends on it, so brew install GSA-TTS/tap/acq currently produces an acq that refuses to run. msb self update is hardcoded to releases/latest. So all three documented install paths landed users on a version we reject.

Two findings there touch this PR's neighbourhood. First, acq_backend_prepare is only invoked from create, run, and github-scope, so acq ls and the other state-touching verbs reach msb with no version check and surface raw upstream errors (database schema is newer than this msb binary) instead of ours; #510 is adding a cheap version-only guard to those verbs. Second, once a host has run 0.7.x, replacing the binary is not sufficient — the catalog migration is one-way, and recovery requires msb self downgrade executed by the 0.7.x binary, since that binary owns the rollback metadata. If ADR-0030's host-authoritative state work ends up relocating any sandbox state that the msb catalog also tracks, it is worth checking that the two do not disagree about who owns migration-era state.

wz-gsa added a commit to GSA-TTS/agentic-coding-patterns that referenced this pull request Sep 24, 2026
Corrections found by a 5-role pr_review panel plus independent
verification (acq internals, cross-repo PR status), then confirmed via
a 7-role higher_order consensus vote (85.7% approve) on the fix
approach itself:

- Consequences/Positive claimed "one shared config-merge/bounded-fetch
  library replaces the per-kit copies" — false against the ADR's own
  Decision. Layer 3 mandates each harness kit render its own native
  config; only the bounded-fetch helper is actually shared. Corrected
  the claim and stated explicitly that no shared config-merge library
  exists.
- Added a security invariant, in three places (Layer 2 output
  description, Layer 3 read-list, Security model bullet list): the
  guest-writable aggregate catalog.json carries only model metadata,
  never routing/credential fields (host/baseUrl/modelsUrl/keyEnv).
  Those fields are read exclusively from the Layer 1 read-only facts
  mount. Without this, a sudo-capable in-guest agent could rewrite the
  catalog to pair one provider's keyEnv with another provider's
  allowlisted host, bypassing the Layer 1 env-var-ownership and SSRF
  checks entirely within caps.network.allow.
- "quickstart ADR-0030" and "goose-server ADR 0004" were cited as
  settled sibling ADRs. Both are, as of this commit, open and unmerged
  (GSA-TTS/agentic-coding-quickstart#504,
  #415 — freshly reverified via `gh pr
  view` before this commit). Added "(pending — org/repo#N)" qualifiers
  per the repo's fully-qualified cross-repo reference convention.

Consensus panel also flagged the Layer 1 artifact-location schema gap
(no field locates a provider's normalizer or vendored snapshot) as a
design change, not a textual fix, and recommended landing it in its
own commit for separate review. That follows in the next commit.

markdownlint: 0 issues.

Co-authored-by: OpenCode Agent <agent@gsa.gov>
mogul added 2 commits October 2, 2026 00:53
ADR-0030 was double-claimed. #474 keeps ADR-0030 because patterns main already cites it for the devenv ADR.

Co-authored-by: OpenCode [gpt-5.5] <bret.mogilefsky@gsa.gov>
@mogul mogul changed the title docs(adr): add ADR-0030 host-authoritative sandbox configuration docs(adr): add ADR-0035 host-authoritative sandbox configuration Oct 2, 2026
@mogul

mogul commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

The aforementioned ADR numbering contention has been resolved.

…tative-config-1

# Conflicts:
#	acq.backends/msb.sh
#	docs/adr/0011-msb-backend-and-neutral-kits.md
#	test/bats/78-msb-kit-env.bats
@wz-gsa

wz-gsa commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

AI-assisted note (OpenCode), advisory. Checked for a rebase/merge-conflict assessment before doing a content review of the current head.

mergeStateStatus is genuinely CONFLICTING against current main, not a stale-cache false alarm (I checked for that specifically, since I hit one on #541 earlier this week) — I merged main into a local copy of this branch to see exactly what collides.

Three real conflicts, one of which is a substantive design collision rather than simple churn:

  1. acq.backends/msb.sh — guard-source blocks (trivial). Both branches independently added a guard-source stanza near the top of the file (yours for acq_host_config_*, main's for _acq_msb_select_kits). Just needs both kept.

  2. acq.backends/msb.sh — OCI auto-install vs. feat(kits)!: make oci-engine an opt-in catalog kit and retire the adapter auto-install #541's retirement (real collision, needs your call). This branch still carries the full OCI auto-install block (_acq_msb_grant_oci_devs, _acq_msb_ensure_oci, the podman provisioning/storage-driver/registries logic) because it predates feat(kits)!: make oci-engine an opt-in catalog kit and retire the adapter auto-install #541 ("make oci-engine an opt-in catalog kit and retire the adapter auto-install," merged 2026-10-05, a breaking change). Naively resolving toward this branch's side would silently resurrect code feat(kits)!: make oci-engine an opt-in catalog kit and retire the adapter auto-install #541 deliberately removed. I did not resolve this myself — it's your call whether the right merge is "drop the OCI block, take main's retirement" (my guess, but I don't have full context on whether ADR-0035's host-config migration needs to touch the OCI kit's own markers) or something else.

  3. acq.backends/msb.sh + scripts/test-acq-lib.sh — agent-name read path + matching test stub (should resolve toward this branch). This branch reads the attach-time agent name via acq_host_config_read (the ADR-0035 migration this PR is about); main's side still reads the pre-migration guest file /var/lib/acq/agent directly, because main doesn't have this migration yet. The test stub conflict is the same thing one level down — this branch's stub comment explicitly says the old guest-side test -f .../agent-user-ready stub was intentionally removed since the marker moved to the host config store. This side should resolve toward this branch, not main.

Not blocking anything new on my end — flagging so you have the collision context before resolving the conflict yourself, rather than me guessing at #2's correct resolution with less context than you have. I'll hold off on a substantive content/security review of this PR until it's green against main again, then do a full pass (I have review history here — two prior advisory rounds in September — so I'll check whether that feedback still holds at the new head too, not just review it fresh).

@mogul

mogul commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Resolved current-main conflict review in commit 256d069. Kept ADR-0035 host-authoritative config and main OCI retirement: no adapter Podman provisioning or oci-ready marker; opt-in oci-engine kit remains. Restored read-only host-config mount and trusted startup staging, using 0711 traversal plus a guest-root write refusal probe. MSB cleanup now removes host config only after confirmed absence and retains it on an indeterminate list probe. Verification passed: bash -n, ShellCheck, git diff --check, and the full ./scripts/test-acq-bats suite. Two adversarial reviews were run; their findings were fixed and regression-tested. Markdownlint/gitleaks were unavailable locally; live backend validation requires a sandbox-capable host. AI-assisted (OpenCode); human review remains required before merge.

@wz-gsa

wz-gsa commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

AI-assisted re-review (OpenCode), advisory — needs a human to confirm before it drives a change. Reviewed head 256d069, main at 16795300.

The conflict resolution is correct, and it's the resolution I was hoping for rather than the naive one. I verified the specific risk I flagged: zero matches for _acq_msb_grant_oci_devs, _acq_msb_ensure_oci, oci-ready, or ACQ_MSB_ENSURE_OCI anywhere in msb.sh — #541's OCI retirement survived intact, the opt-in kit path remains, and ADR-0035's host-config mechanism landed alongside it. mergeStateStatus is BLOCKED (review-gate) rather than DIRTY now.

One finding, reproduced and fix-verified. Everything else I probed holds.

Should-fix: stat -c is GNU-only, so the new test fails on macOS

test/bats/79-msb-readonly-kit-files.bats:107-109 uses bare stat -c '%a'. On macOS:

not ok 2 msb ro: a readonly code file is staged on the host-config dir, NOT copied into the guest
#   `assert_equal "$(stat -c '%a' "$cfg")" "711"' failed
# stat: illegal option -- c
# expected : 711
# actual   :

This repo already solved this — five other test files use the BSD/GNU fallback, and these three lines are the only bare stat -c calls in the tree:

# 116-clone-option.bats:381, 135-configure.bats:131-132,
# 45-secret-store-unit.bats:85, 71-msb-provision-secrets.bats:154
stat -c '%a' "$dir" 2>/dev/null || stat -f '%Lp' "$dir" 2>/dev/null || echo '?'

I applied that idiom to the three lines locally and re-ran: 6/6 pass (was 5/6). Reverted afterwards so this review doesn't carry a diff.

Worth fixing rather than leaving because of what the failing assertions check: they're the 0711 dir-mode and 0555 staged-file-mode checks — i.e. the permission design this ADR's Mechanism 2 rests on. A macOS developer currently gets a red suite whose only failing assertions are the security-relevant ones, with nothing indicating it's environmental. It does fail loudly (assert_equal catches the empty string), so it is not a silent pass.

Verified correct — the parts I went after

Write-refusal probe polarity (msb.sh:1823-1830). This is the right shape. It doesn't infer read-only-ness from mount flags; it attempts an actual write as root (-u 0) and treats write success as disqualifying:

_tmp="$_dir/.acq-ro-probe.$$"; if : > "$_tmp" 2>/dev/null; then rm -f "$_tmp"; exit 1; fi; exit 0

A writable mount therefore degrades to the legacy guest copy with an explicit warning rather than silently trusting a path a sudo-capable agent could forge. Three bats tests cover exactly the degraded paths that matter — no mount, writable mount, and the create-time --volume …:/var/lib/acq/host:ro assertion. The writable-mount case is the one I would have asked for.

Key-charset guard (_acq_host_config_key_ok). Fuzzed 17 inputs: refuses ../escape, a/b, .., ., empty, a$(whoami), a;rm -rf /, a b. Accepts ..., which I checked separately rather than assuming — it's an ordinary filename, normpath('cfg/...') stays inside cfg/, and it isn't a directory, so it is not a traversal token. No escape from the config dir.

cksum collision surface. The scheme is pre-existing (provenance records and the cli-kits cache already key this way, and _acq_provenance_file:1988-1989 documents the rationale), so this PR inherits rather than introduces it. I tested the only dangerous case — distinct raw names that sanitize identically, which is the precondition for aliasing — across four pairs (my box/my/box, a b/a:b, x@y/x#y, foo bar/foo|bar): all four produced different cksums. Fail-open to 0 when cksum is unavailable, consistent with the existing convention.

Fail-soft write semantics on the run-once gates. Traced it: if acq_host_config_write fails (msb.sh:2219, || true), the marker is absent, so acq_host_config_has (line 2208) returns false and the install command re-runs next apply. That's the safe direction — no half-migrated state that reads as "already done". The codebase states the install/startup idempotency contract in six places, so a re-run is the designed behaviour.

The 0711 tradeoff is documented, not accidental (common.sh:2299-2304). Guest must traverse the mount root to exec staged kit-files/… code but cannot list acq-internal key names; content-bearing keys stay 0600. The comment explicitly warns that traversable dirs make known kit-file paths host-readable, and that only non-sensitive trusted code may be staged there. Stating the limitation beside the mechanism is the right call.

Static analysis. bash -n clean on common.sh, msb.sh, sbx.sh, kit-translate.sh. shellcheck --severity=warning on all four: clean, exit 0.


Method: static review of the 21-file diff, plus execution — bash -n, shellcheck, the ADR-0035 bats file (6 tests, before and after a local fix of the stat idiom), transcription-and-fuzz of _acq_host_config_key_ok, and a cksum-aliasing probe over sanitizer-colliding name pairs.

UNVERIFIED, flagged rather than glossed: the full ./scripts/test-acq-bats suite exceeded a 15-minute timeout on my machine, so I ran 79-msb-readonly-kit-files.bats directly instead of the whole suite. I'm taking your reported full-suite pass on your evidence, not re-confirming it. Also unverified: anything needing a live sandbox-capable host — the real :ro mount behaviour under msb, and the guest-root write refusal against an actual guest rather than the stubbed probe.

@basiliskus

Copy link
Copy Markdown
Contributor

Question about sandboxes created before this lands. I read the diff but haven't run it.

The readers now go to the host config store only, and I don't see a step that copies the existing guest /var/lib/acq/* records into it. For a sandbox provisioned on 4.0.1:

  • kit-env looks fine: the kit heal calls _acq_msb_reset_kit_env and re-applies every kit, which repopulates the host key. Is that heal guaranteed to run on the first run/start after upgrading, or only when a kit is stale?
  • agent is written only in provision. _acq_msb_attach reads only acq_host_config_read msb "$name" agent (unlike acq_backend_recorded_agent, it has no provenance fallback), so a bare acq run <name> would open a plain shell instead of the agent.
  • workspace is also provision-only, so _acq_msb_workspace_for falls back to /home/agent instead of the repo.
  • The install markers (install-<cksum>, agent-user-ready) are missing from the host store, so the first heal re-runs every install-phase command and the agent-user setup. That's probably harmless, but it's slow for kits with heavy installs.

Is this intended, with recreate as the answer for existing sandboxes, or should there be a one-time import of the legacy guest records (read once, charset-guarded as before, written to the host store, then never read from the guest again)? The import would keep the trust change for new state while avoiding a silent behavior change on upgrade. Either way, a release note or KNOWN_FAILURE_MODES entry would help. Our team runs long-lived msb sandboxes with Paseo, which relies on the replayed kit env (ADR-0033), so we'd test that path before rolling the upgrade out.

@basiliskus

Copy link
Copy Markdown
Contributor

Two small test-side issues I hit running the suites locally on macOS at 256d069:

  • test/bats/79-msb-readonly-kit-files.bats test 2 ("a readonly code file is staged on the host-config dir, NOT copied into the guest") fails on macOS: stat -c '%a' is GNU-only, so BSD stat errors (stat: illegal option -- c) and the assert_equal ... "711" sees an empty string. A portable form is stat -c '%a' "$cfg" 2>/dev/null || stat -f '%Lp' "$cfg". CI on Linux won't catch it, but it breaks the suite for Mac contributors.
  • seed_host_config in scripts/test-acq-lib.sh returns non-zero whenever STUB_RECORDED_KIT_ENV is unset, because its last line is a bare [ -n ... ] && .... Called from a run bash -c 'set -euo pipefail; ...' body, that exits the test shell. Ending the function with return 0 would make it safe to call anywhere without || true.

@wz-gsa

wz-gsa commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Confirming @basiliskus's migration-gap finding with line references — it reproduces on inspection, and the fix shape already exists in this file.

The asymmetry is real. Two readers of the same agent key behave differently:

acq_backend_recorded_agent (msb.sh:4421-4430) is layered correctly:

agent=$(acq_provenance_field msb "$name" agent)      # provenance first
if [ -z "$agent" ]; then
  agent=$(acq_host_config_read msb "$name" agent …)  # host-config fallback
fi

_acq_msb_attach (msb.sh:4612) reads host-config only:

agent=$(acq_host_config_read msb "$name" agent | tr -d '[:space:]')
if [ -z "$agent" ] || ! _acq_msb_safe_agent_token "$agent"; then
  agent="shell"
fi

For a sandbox provisioned on 4.0.1 the host-config key does not exist, acq_host_config_read returns empty (it returns 0 on a missing key by design), and attach falls to shell — while the provenance record still holds the correct agent and the sibling function would find it. Since agent is written only at provision, this hits every pre-existing sandbox on its first acq run after upgrade, not an edge case.

Same shape in _acq_msb_workspace_for. Its own comment (msb.sh:4432-4437) already names the case — "fall back to the agent home if nothing was recorded (older sandbox)" — but the fallback target is /home/agent rather than the provenance record, so an upgraded sandbox opens outside the repo instead of in it.

On my own review: I checked that failed writes degrade safely (failed write → absent marker → install re-runs, which is the safe direction) and reported that as clean. I did not check the absent-key-on-upgrade read path, which is the more consequential case because it applies to every sandbox created before the migration rather than only to a write failure. basiliskus's catch is the better one.

Two things that follow from it, for whoever takes the fix:

  • The gate markers (install-<cksum>) have the inverse problem: absent host-config key means acq_host_config_has returns false and the install command re-runs. That is safe for idempotent installs (and the codebase documents them as such in six places), but it means an upgraded sandbox re-runs every install step once. Worth deciding whether that is acceptable or whether the markers also want a provenance read, since a re-run is observable behaviour even when it is harmless.
  • ssh-auth-sock (msb.sh:4145) is written when the ssh-agent bridge starts rather than only at provision, and its own comment says "Only written when forwarding is active" — so it is probably fine, but worth confirming the bridge actually restarts (and rewrites the key) on an upgraded sandbox rather than being skipped because a guest-side marker already exists.

Not re-reviewing the rest — my 256d069 review stands otherwise.

@mogul

mogul commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review feedback in 79743d5 (fix(msb): preserve legacy session context) and pushed it to this PR branch.

@wz-gsa: adopted the GNU/BSD stat fallback in 79-msb-readonly-kit-files.bats, so the 0711/0555 permission assertions now run on macOS as well as Linux. I also fixed the seed_host_config helper to return success when optional fixture values are unset, with a set -euo pipefail regression test.

@basiliskus and @wz-gsa: the pre-ADR-0035 migration gap was valid. I did not import /var/lib/acq markers, because guest state remains tamperable under passwordless sudo. Instead, missing host-config session values now fall back only to existing trusted host provenance:

  • agent: host-config first, then provenance for a legacy sandbox.
  • workspace: host-config first, then the provenance workspace record.
  • The one-time missing gate behavior remains intentionally safe: the normal heal reruns idempotent install/agent-user work once, rebuilds kit environment, and re-establishes SSH-agent forwarding when configured.

Regression coverage now proves legacy agent/workspace recovery, host-config precedence over stale provenance, the helper unset-fixture path, and portable readonly file-mode assertions. docs/KNOWN_FAILURE_MODES.md documents the upgrade behavior and recovery path.

Verification transcript:

  • PASS: PATH="/usr/bin:$PATH" ./scripts/test-acq-bats test/bats/72-msb-agent-user.bats test/bats/78-msb-kit-env.bats test/bats/79-msb-readonly-kit-files.bats (51 tests)
  • PASS: PATH="/usr/bin:$PATH" ./scripts/test-acq-bats (full offline suite)
  • PASS: bash -n acq.backends/msb.sh scripts/test-acq-lib.sh
  • PASS: git diff --check
  • PASS: independent adversarial review; its provenance-precedence and documentation-numbering findings were fixed and regression-tested before this push.
  • BLOCKED: ShellCheck and markdownlint are not installed in this checkout; no dependency install was performed.
  • BLOCKED: live backend validation requires a sandbox-capable host.

Could @wz-gsa and @basiliskus please re-review the updated head, particularly the provenance-only legacy fallback and upgrade behavior? AI-assisted (OpenCode); human review remains required before merge.

@basiliskus

basiliskus commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Verdict: comment — 79743d5ea251996264b9c0394f0014309a596ec4
Must fix before merge: none

Prior finding: resolved at 79743d5: agent and workspace now fall back to trusted host provenance, with host config taking precedence (acq.backends/msb.sh:4421-4457).

# Sev Where Problem → failure scenario Evidence
1 minor docs/adr/0035-host-authoritative-sandbox-config.md:101-129,186-206,302-312 The ADR overstates what the read-only mount guarantees. It protects the host-staged bytes, but cannot guarantee which bytes a root-controlled guest executes: guest root can alter the execution path or plausibly mask the mount. This is not a sandbox escape because startup output is not trusted back on the host and the attacker already has guest root. Narrow claims such as “guest sudo cannot override it,” “always executes” the pinned copy, and “tamper-resistant startup code” to byte integrity at the host/volume boundary. confirmed: execution remains entirely inside the guest (acq.backends/msb.sh:1823-1830,2228-2253), and startup failures/output do not configure host state.
2 minor test/bats/72-msb-agent-user.bats:146-156 Precedence is tested for agent only. The workspace fallback test (:241-251) has no conflicting host-config value, so reversing workspace order to stale provenance would stay green. Add a conflicting host-config/provenance assertion for _acq_msb_workspace_for. confirmed: traced both tests and the production ordering at acq.backends/msb.sh:4450-4456.

Nits: acq.backends/msb.sh:4614-4616 says provenance is preferred before host config, opposite to the implementation; reverse the comment order.
Not verified: offline Bats blocked by uninitialized vendored submodules; markdownlint blocked by missing local install; real Bash 3.2 and live msb/sbx validation unavailable. ShellCheck and Bash syntax checks passed.


The legacy-session fix is correctly ordered: host config wins, with trusted host provenance used only when the new key is absent. The agent order is tested, but workspace precedence still needs a conflicting-value regression test.

ADR-0035 overstates the read-only mount's guarantee at docs/adr/0035-host-authoritative-sandbox-config.md:101-129,186-206,302-312. The mount protects host-staged bytes, but cannot guarantee which bytes a root-controlled guest executes: guest root can alter the execution path or plausibly mask the mount. This does not cross the sandbox boundary because startup output is not trusted back on the host and the attacker already has guest root. Please narrow “guest sudo cannot override it,” “always executes” the pinned copy, and “tamper-resistant startup code” to the byte-integrity guarantee the host volume actually provides.

Also add a workspace precedence test with conflicting host-config and provenance values; the current legacy workspace test covers fallback, not ordering.

Assisted-by: OpenCode [gpt-5.6-sol-latest-guardrails-defaultv2] skill:change-review

@wz-gsa wz-gsa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the current head 79743d5ea, specifically the legacy-sandbox migration gap we confirmed at 256d06958. The fix now gives the new host-config store precedence, then falls back to trusted host provenance for pre-ADR-0035 sandboxes; it never imports guest markers. _acq_msb_attach now consumes acq_backend_recorded_agent, and _acq_msb_workspace_for falls back through acq_workspace_record_read, canonicalizing when available before /home/agent. The tests pin all three contracts: a legacy sandbox resumes with its recorded agent/workspace, host-config overrides stale provenance, and acq exec uses the legacy workspace. The test-helper return 0 fixes the set -e degraded path rather than hiding it, with its own regression test. All CI gates pass, including the full bats build job under bash 3.2. No remaining findings from me. AI-assisted review (OpenCode), formally approved with the maintainer’s explicit authorization; a human owns the merge.

@wz-gsa
wz-gsa merged commit 0e4bccb into main Oct 9, 2026
7 checks passed
@wz-gsa
wz-gsa deleted the docs/adr-host-authoritative-config branch October 9, 2026 20:41
wz-gsa added a commit that referenced this pull request Oct 10, 2026
Fixes the reboot case of #501 (follows
#413).

## Problem

The automatic ssh-agent `--vsock` route pins the create-time
`SSH_AUTH_SOCK` path. On macOS the
launchd agent socket path changes on every login, so after a host reboot
every existing msb sandbox
has a dead route: `ssh-add -l` in the guest reports "communication with
agent failed" and SSH commit
signing fails. Until now the only remedy was `acq rm` + recreate.

## Change

msb dials the route's `host_socket` per guest connection and follows
symlinks (verified on msb 0.7.7).
acq now emits a per-sandbox symlink,
`${ACQ_STATE_DIR}/msb/ssh-agent/<sandbox>.sock`, as the route's
host path and atomically re-points it at the current `SSH_AUTH_SOCK` on
every bridge (re)start:
provision, `acq start`, and the running re-attach re-drive. `acq rm`
removes the link.

- Custom `ACQ_FORWARD_HOST_SOCKETS` entries keep today's canonicalized
behavior.
- Sandboxes created before this keep their raw route (acq cannot change
a persisted route). The
unreachable-agent warning now says "recreate once" for them, and points
at the host agent for a
  managed route.
- A cksum-derived basename keeps the link inside `sun_path`; an unsafe
sandbox name or a failed
  symlink falls back to the raw path (today's behavior).
- sbx is unchanged.

Docs: ADR-0021 amendment, `docs/KNOWN_FAILURE_MODES.md` §35,
`docs/BACKEND_GUIDE.md`.

## Verification

- `test/bats/115-ssh-agent-forward.bats`: new cases 10c22–10c30; 34/34
pass.
- Full offline suite (after merging `main` at `0e4bccb`): 732 ok, 9 not
ok. The same 9 fail on a clean
export of `main` in the same environment (TMPDIR/volume-path and
attach-line assertions), so they are
  pre-existing.
- shellcheck (`--severity=warning`) and the pinned markdownlint pass.
- Live on macOS/HVF, msb 0.7.7, throwaway agent and key: route persisted
as the link path; re-pointing
the link to an empty agent and back changed guest `ssh-add -l` with no
restart; after `acq stop`,
killing the agent, starting a new agent at a different socket path and
`acq start`, the guest listed
the key and `git commit` produced a good signature with no recreate;
`acq rm` removed the link.

## Follow-ups (not in this PR)

- #501 can be narrowed to the `acq
restore` path only.
- #504 has landed; `main` is merged
into this branch (`cf8c02e`). The
`acq_backend_terminate` conflict keeps both cleanup calls, and the
bridge-restart blocks of 10c24, 10c25
and 10c30 seed the host-config record. `115-ssh-agent-forward.bats`
passes 34/34.
- Piping `acq create`/`acq start` stderr hangs because the msb VM
process inherits fd 9; tracked in
  #568.

AI-assisted.

---------

Co-authored-by: William Zujkowski <153217227+wz-gsa@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants