feat(groom): isolated netns sandbox — drop --share-net, broker over a bind-mounted unix socket (BE-4421)

[mattmillerai]

STACKED — merging lands on matt/be-4302-groom-sandbox (owned by @mattmillerai, PR #66), NOT main. This is phase 2 on top of the still-open BE-4302 sandbox harness. Do not merge until #66 lands; GitHub auto-retargets this to main when #66 merges. Not "ready to merge" in the unqualified sense.

ELI-5

The groom auto-builder runs an untrusted agent in a locked box. Until now the box still had a working network cable, so a misbehaving agent could in principle phone home, hit the runner's own services, or read cloud metadata. This change cuts the cable entirely: the box gets its own empty network with only a loopback wire. The one thing the agent still needs — talking to Anthropic through the key-broker — comes in through a tiny door (a unix socket) we mount into the box, with a little in-box translator (jail-shim.mjs) so ordinary HTTP tooling can use it. Everything else off-box is now unreachable by construction, and we prove it with tests that show cloud metadata, an arbitrary internet IP, and the host's own loopback all fail to connect from inside.

What & why

Phase 2 of the groom sandbox hardening (design from spike BE-4369: namespace isolation by construction — option b; slirp4netns/nftables/broker-in-jail alternatives were rejected). Removes --share-net so the jail runs in an isolated network namespace with only lo up — no external hosts, no host-loopback services, no cloud metadata (169.254.169.254 / 168.63.129.16) — while keeping the key-broker reachable over a unix-domain socket bind-mounted into the jail.

• broker.mjs — now accepts a unix-socket path (absolute) as argv[2] in addition to the legacy numeric TCP port. UDS mode clears a stale socket then server.listen(path); a relative path is refused loud. The request-handling contract (strip/inject key, /v1/* only, /healthz, SSE, crash-proofing) is transport-independent and unchanged.
• jail-shim.mjs (new, ~20 lines, node stdlib) — a TCP→UDS forwarder run inside the jail (node jail-shim.mjs <port> /run/broker.sock) so agent tooling speaking HTTP to a 127.0.0.1:<port> base URL reaches the bind-mounted socket (an isolated netns can't dial a host TCP port).
• agent-sandbox.sh — drops --share-net from both bwrap invocations (selftest + bwrap_args); adds --uds <host-socket-path> (at most once, absolute, must already be a listening socket) that rw-binds the socket file at the fixed in-jail path /run/broker.sock (connect(2) needs write on the inode; --ro-bind would EROFS). Omit --uds for a fully-offline jail.
• tests/sandbox-tests.sh — section 5 reworked onto the UDS+shim path (broker listens on a socket; in-jail shim bridges curl to it; all existing assertions unchanged). New section 7 = the BE-4369 acceptance proofs: host loopback, cloud metadata, and an arbitrary external IP (1.1.1.1) are all unreachable from the jail, plus --uds fail-loud on a nonexistent socket.
• README.md — isolated-netns contract row, UDS mode + shim, and the offline consequences (set CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC=1; in-jail git fetch/npm install cannot work — everything must be in the clone before sandboxing).

Testing

• shellcheck -x .github/groom/agent-sandbox.sh .github/groom/tests/sandbox-tests.sh — clean.
• The diff contains no remaining --share-net anywhere under .github/groom/.
• python3 -m unittest (groom ledger, untouched) — 48 passing (sanity).
• The full bash sandbox-tests.sh requires Linux + unprivileged user namespaces (bubblewrap), so — exactly like the phase-1 BE-4302 suite — it runs on the sandbox-tests CI job on ubuntu-latest, not on the macOS dev host. To de-risk the transport-independent JS on the host I ran a smoke test proving the real composition end-to-end: broker on a UDS + jail-shim.mjs bridging TCP→UDS → key injected, caller dummy stripped, /healthz 200, non-/v1 404, SSE frames + [DONE] stream through; plus TCP back-compat, relative-path refusal, and jail-shim arg validation all behave. The netns egress assertions rest on the spike-verified facts (bubblewrap brings up lo in the fresh netns; empty-netns connects fail immediate ENETUNREACH).

Negative-claim falsification (per the coder self-review)

This change's user-facing outcome denies a capability (network egress from the jail) — the trigger for the falsification check. It is not the #581 anti-pattern: cutting egress is the deliberate BE-4369 security goal, and the one capability the agent legitimately needs — reaching Anthropic — is preserved and empirically proven working via the sanctioned UDS+shim path (see the host smoke above: real key forwarded, SSE intact). The "unreachable" assertions target the security boundary (metadata / external IP / host loopback), which is deterministic by netns construction, not a mistaken claim that a needed feature is unavailable.

Out of scope

The groom caller workflow (epic BE-3870) that composes broker + shim + agent in production doesn't exist yet; this PR defines and test-proves the composition pattern the caller will copy.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0280f977-07ab-484e-ab62-21348c77af63

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-4421-groom-sandbox-netns

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-4421-groom-sandbox-netns

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 8 finding(s).

Severity	Count
🟡 Medium	3
🟢 Low	4
⚪ Nit	1

Panel: 8/8 reviewers contributed findings.

[mattmillerai]

⚠️ STACKED — merging lands on matt/be-4302-groom-sandbox (PR #66, @mattmillerai), NOT main. Do not merge onto the default branch.

Addressed all 8 cursor-review panel findings in dc20444 (2 medium chmod/stdout, 1 medium rmSync, 5 low/nit):

• broker.mjs — stat-first unlink (fail loud on non-socket, no EISDIR crash / silent delete); chmod 0600 the UDS after bind (owner-only, umask-independent); server.on('error') handler.
• jail-shim.mjs — startup banner → stderr (stdout is captured as agent JSON); server.on('error') handler.
• agent-sandbox.sh — reject empty --uds; /healthz probe (curl-gated) to confirm a live broker, not just a socket inode; bind the broker socket --ro-bind (connect(2) works read-only — sb_permission EROFS excludes sockets — while dropping the agent's chmod on the shared inode).
• sandbox-tests.sh — assert curl exists in the jail before the 7a/7b/7c egress checks.

The --ro-bind change is verified in CI by section 5 of sandbox-tests.sh, which exercises the exact in-jail connect over the read-only socket bind. No deferrals.