fix(msb): route ssh-agent forward through a managed symlink

[basiliskus]

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)

• feat(acq): acq restore re-derives the msb --vsock route (SSH signing survives reboot) #501 can be narrowed to the acq restore path only.
• docs(adr): add ADR-0035 host-authoritative sandbox configuration #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
  fix(acq): piping acq create/start stderr hangs until the sandbox stops #568.

AI-assisted.

[mogul]

omg great solution!

[wz-gsa]

AI-assisted review (OpenCode), advisory — needs a human to confirm before it drives a change. Reviewed head de6b9b33.

Checked the claims that a live run wouldn't catch, by compiling/executing rather than reading. One finding, about the #504 interaction. The mechanism itself verifies clean.

Verified

The sun_path budget is right, and it's the claim most likely to be wrong by one. Compiled a probe rather than trusting the comment:

struct sockaddr_un u; printf("%zu", sizeof(u.sun_path));  // -> 104

ACQ_MSB_SUN_PATH_MAX=103 is therefore correct (104 bytes including the NUL ⇒ 103 usable), and budgeting for macOS over Linux's 108 is the right direction. Also confirmed the length check doesn't over-count: printf '%s' "$p" | wc -c equals ${#p} exactly, so no phantom newline byte.

The atomic swap is actually atomic. Built a real symlink, pointed a .tmp link at a second target, mv -f'd it over — the link flips with no intermediate missing state, i.e. rename(2) as the comment claims.

The name charset guard holds. Fuzzed 13 inputs through _acq_msb_ssh_agent_link_path; it refuses ../../etc/x, a/b, -dash, a b, a;id, a$(id), ., .., empty, and even a.b. Only [A-Za-z0-9_-] reaches a path, which matters because the name interpolates into ln/rm -f arguments.

The cksum fallback doesn't collide in practice. Generated 4000 realistic long sandbox names (the only shape that can reach the hashed branch, since short names never overflow) and hashed each: 0 collisions. A collision would mean two sandboxes sharing one route link, so worth checking rather than assuming.

acq rm can't unlink the agent socket. _acq_msb_ssh_agent_link_remove:4098 is [ -L "$link" ] && rm -f "$link" — the -L test means it only ever removes a symlink, never a socket, even if something replaced the path.

Tests and static analysis. test/bats/115-ssh-agent-forward.bats: 34/34 pass (the PR body says 33 — there are 34). shellcheck --severity=warning acq.backends/msb.sh: clean, exit 0. bash -n: OK.

Three defensive details worth noting because they're the kind that get dropped in review: the [ -L "$tmp" ] check after ln -s guards hosts where ln -s copies instead of linking (a copied socket would be a dead route); the "link currently resolves to a directory" case is dropped before mv so it can't descend into it; and _acq_msb_ssh_agent_link_point refuses to clobber a non-symlink at the link path.

Finding: the #504 interaction is a conflict, not a one-line merge

Your follow-up says:

If #504 lands first: one-line merge in acq_backend_terminate (keep both cleanup calls) and seed the host config record in tests 10c24/10c25.

The resolution is right, but "one-line merge" undersells it. Both PRs edit acq_backend_terminate — this one adds _acq_msb_ssh_agent_link_remove "$1" at msb.sh:4806, and #504 adds host-config cleanup in the same function — so whichever lands second gets a real git conflict there rather than an automatic merge.

The test side is the larger half: #504 moves ssh-auth-sock from a guest marker to a host-config key (msb.sh:4145 on that branch), and your 10c24/10c25 both drive the bridge-restart path. You've already identified that those two need the host-config record seeded, so the dependency is understood — I'm flagging the framing because whoever picks up the rebase (possibly not you) will hit a stop, not a fast-forward.

Worth deciding the order deliberately rather than by merge time. #504 is the larger and more structural of the two.

---

Method: compiled a sizeof(sockaddr_un.sun_path) probe; executed a live symlink-swap test; transcribed and fuzzed _acq_msb_ssh_agent_link_path over 13 adversarial names; hashed 4000 long names for cksum collisions; ran the new bats file and shellcheck/bash -n; read the acq_backend_terminate and ssh-auth-sock call sites on both this branch and #504's.

UNVERIFIED: the live macOS/HVF reboot scenario — route persisted as the link path, re-pointing the link changing guest ssh-add -l without a restart, and the kill-agent/new-socket/acq start recovery. No sandbox-capable host here, so those are taken on your reported run. Also unverified: your "same 8 failures on a clean main export" claim about the full offline suite; I ran only 115-ssh-agent-forward.bats.

[basiliskus]

Thanks, confirmed by trial-merging this branch onto docs/adr-host-authoritative-config (256d069): one conflict hunk in acq_backend_terminate (resolution: keep both cleanup calls), and with it resolved 3 of the new tests (10c24, 10c25, 10c30) fail until each bridge-restart block seeds the host-config record with seed_host_config msb <name> || true. With those 8 one-line additions the file passes 34/34 on the merged tree. I'd prefer #504 land first; I'll rebase this one onto it and carry those edits. Also fixed the test count in the description (34, not 33).

[basiliskus]

@wz-gsa can I get another review and verdict for this PR?

[wz-gsa]

Approving. Head is unchanged at de6b9b33 since my 2026-10-07 review, and you closed the one finding I raised more thoroughly than it asked for.

My finding was that "one-line merge in acq_backend_terminate" undersold the #504 interaction. Your reply did the actual work: trial-merged onto docs/adr-host-authoritative-config (256d069), found exactly one conflict hunk with a stated resolution (keep both cleanup calls), identified the three tests that then fail (10c24, 10c25, 10c30), confirmed 34/34 on the merged tree with 8 one-line seed_host_config msb <name> || true additions, and named the ordering you prefer. That is the measured answer, and it also fixed the 33-vs-34 count I had flagged separately.

So the finding is resolved as an understood sequencing dependency, not a defect in this PR. Nothing here blocks on it: the code on this branch stands on its own, and the follow-up is a rebase you have already scoped.

Carrying forward from my earlier review, for the record — the parts I verified by execution rather than reading, all still valid at this head:

• ACQ_MSB_SUN_PATH_MAX=103 is correct. Compiled a probe: sizeof(sockaddr_un.sun_path) is 104 on macOS, so 103 usable, and budgeting for macOS over Linux's 108 is the right direction.
• mv -f over the link really is rename(2)-atomic — tested on a live symlink, no missing intermediate state.
• _acq_msb_ssh_agent_link_path's charset guard holds across 13 adversarial names (../../etc/x, a/b, -dash, a;id, a$(id), ., .., a.b), which matters because the name reaches ln/rm -f as an argument.
• The cksum fallback does not collide: 4000 realistic long names, 0 collisions.
• acq rm cannot unlink the agent socket — the [ -L "$link" ] test at msb.sh:4098 means it only ever removes a symlink.
• 34/34 in 115-ssh-agent-forward.bats; shellcheck --severity=warning and bash -n clean on msb.sh.

Filing a tracking issue for the #504 rebase so the 8 seed additions and the acq_backend_terminate resolution do not get rediscovered by whoever lands the second of the two.

Still unverified, unchanged from before: the live macOS/HVF reboot scenario — route persisted as the link path, re-pointing the link changing guest ssh-add -l without a restart, and the kill-agent/new-socket/acq start recovery. No sandbox-capable host here, so those remain on your reported run. Likewise your "same 8 failures on a clean main export" claim about the full offline suite; I ran only the ssh-agent file.

[wz-gsa]

Filed #567 to track the #504 rebase — your measured delta (one acq_backend_terminate conflict hunk, keep both cleanup calls; 8 seed_host_config additions; 34/34 after) is recorded there with your stated preference that #504 lands first, so whoever lands the second of the two does not have to rediscover it.

Checked for an existing issue first; #501 covers the reboot case this PR fixes and #533 covers ADR-0030 merge semantics, but neither tracked this specific interaction.

Also noted in #567: seed_host_config needing || true is itself a bug you found (bare [ -n ... ] && ... as the function's last line returns non-zero when STUB_RECORDED_KIT_ENV is unset), and return 0 would remove the need for the guard at each call site.

[wz-gsa]

#504 is merged. I recreated the measured rebase twice and confirmed your recipe exactly: one conflict in acq_backend_terminate (keep both cleanup calls); seed host config in the 10c24/10c25/10c30 bridge-restart blocks; focused suite 34/34, Bash syntax and ShellCheck clean.

I did not push the merge because the repository's hook-triggered full test suite exposed a separate destructive test-isolation bug: nested fixture git init inherits hook-local GIT_* state, can reinitialize the parent gitdir / set shared core.worktree, and a later kit fetch can check out FETCH_HEAD into the active repo. Reproduced twice, repaired locally, no push. Filed #569 with the reflog evidence, root cause, and acceptance criteria.

Your branch remains approved, but is now genuinely conflicted against main. Please apply the already-verified #567 merge recipe after #569 (or in a clean environment with the hook/test fix), then the refreshed CI should be the final gate. I am not bypassing hooks for a time-critical PR after they found a real repository-corruption defect.

[wz-gsa]

Approving updated head f0364df6 after merging current main (including #504).

The known integration recipe is present and verified:

• acq_backend_terminate retains both _acq_msb_ssh_agent_link_remove "$1" and acq_host_config_remove msb "$1" || true.
• Host config is seeded in each affected 10c24/10c25/10c30 bridge-restart block.
• test/bats/115-ssh-agent-forward.bats: 34/34 pass on the integrated tree.
• bash -n and ShellCheck are clean for the touched shell/test files.
• The original symlink implementation checks remain valid: macOS 103-byte sun_path budget, atomic link swap, strict sandbox-name charset, safe link-only cleanup, and fallback behavior were exercised in the earlier review.

quickstart#569 tracks a separate local hook/test-isolation defect discovered while attempting the same merge locally. It affects developer hook execution, not this branch's remote content or CI. I did not bypass hooks or push a local merge; the author's branch update now carries the measured integration.

Merge only after the refreshed Bash 3.2 build/check completes green.

[mogul]

We should probably cut a release that includes this fix.