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

.github/groom/README.md

@@ -185,12 +185,19 @@ API key — while still letting it edit its worktree and reach Anthropic.
 
   ```bash
   agent-sandbox.sh --clone <path> --clone-mode ro|rw-git-ro --out-dir <path> \
-      [--ro-file <path> ...] [--env KEY=VALUE ...] -- <command...>
+      [--ro-file <path> ...] [--env KEY=VALUE ...] [--uds <host-socket-path>] \
+      -- <command...>
   ```
 
 - **[`broker.mjs`](broker.mjs)** — a ~50-line node-stdlib reverse proxy
-  (`node broker.mjs <port>`) that holds the real key on the host and forwards the
-  jail's requests to it.
+  (`node broker.mjs <port|socket-path>`) that holds the real key on the host and
+  forwards the jail's requests to it. In socket mode it listens on a unix-domain
+  socket (bind-mounted into the jail via `--uds`); the legacy TCP port mode is
+  retained for the test plumbing and back-compat.
+- **[`jail-shim.mjs`](jail-shim.mjs)** — a ~20-line node-stdlib TCP→UDS forwarder
+  (`node jail-shim.mjs <port> /run/broker.sock`) that runs **inside** the jail so
+  agent tooling speaking HTTP to a `127.0.0.1:<port>` base URL reaches the broker's
+  bind-mounted socket (the isolated netns has no way to dial a host TCP port).
 
 ### The sandbox contract (what the agent can and cannot see)
 
@@ -204,7 +211,7 @@ API key — while still letting it edit its worktree and reach Anthropic.
 | host `$HOME` / `$RUNNER_TEMP` / `$GITHUB_WORKSPACE` / other repos | **invisible** |
 | host process table | **invisible** (own pid namespace) |
 | environment | **cleared** — only `HOME`, `PATH`, `TERM`, and each `--env KEY=VALUE`; nothing inherited from the host |
-| network | shared (so the agent can reach the broker on loopback) |
+| network | **isolated network namespace** (loopback only) — host network, host loopback, and cloud metadata are all **unreachable**; the broker is reached via a unix socket bind-mounted at `/run/broker.sock` plus the in-jail `jail-shim.mjs` TCP forwarder |
 
 The `rw-git-ro` worktree write is exactly how the builder's patch is captured: the
 agent edits tracked files, the wrapper's caller reads them back on the host
@@ -215,8 +222,21 @@ afterward, but the agent can never rewrite git history or `.git/config`.
 `x-api-key` / `authorization` header, injects the real key, and forwards only
 `/v1/*` paths to `api.anthropic.com` — streaming the response through unbuffered
 so SSE works. `GET /healthz` answers locally; anything not under `/v1/` is `404`.
-It listens on `127.0.0.1` only, refuses to start with an empty key, and logs one
-line per request — method + path + status, never headers or body.
+It listens on a unix-domain socket (`--uds`, the phase-2 default) or `127.0.0.1`
+(legacy TCP mode), refuses to start with an empty key or a relative socket path,
+and logs one line per request — method + path + status, never headers or body. The
+request-handling contract is identical on both transports.
+
+**No network egress (BE-4421).** The jail runs in an isolated network namespace
+with only loopback up, so the broker — reached over the unix socket bind-mounted
+at `/run/broker.sock` via the in-jail `jail-shim.mjs` TCP→UDS forwarder — is the
+*only* thing the agent can talk to. Host network, host loopback services, and
+cloud metadata (`169.254.169.254` / `168.63.129.16`) are all unreachable. Two
+consequences for callers: set `CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC=1` in the
+agent env so the agent doesn't stall on telemetry endpoints that can never be
+reached; and because there is no egress, in-jail `git fetch` / `npm install`
+cannot work — anything the agent needs must already be in the clone before it is
+sandboxed.
 
 ### The loud-preflight guarantee
 
@@ -237,8 +257,11 @@ in [`test-groom-scripts.yml`](../workflows/test-groom-scripts.yml)) asserts ever
 row of the contract above with `bash -c` as the sandboxed command — env scrub, FS
 confinement + tmpfs shadowing, both clone modes, pid isolation — and points the
 broker at a local fake upstream ([`tests/fake-upstream.mjs`](tests/fake-upstream.mjs))
-to prove key injection/stripping, the `/healthz` + non-`/v1` behavior, and SSE
-pass-through. No `claude`, no API key, no spend.
+*over the bind-mounted unix socket + in-jail `jail-shim.mjs`* to prove key
+injection/stripping, the `/healthz` + non-`/v1` behavior, and SSE pass-through. It
+also proves the BE-4369 egress isolation: host loopback, cloud metadata, and an
+arbitrary external IP are all unreachable from the jail. No `claude`, no API key,
+no spend.
 
 ```bash
 shellcheck -x .github/groom/agent-sandbox.sh .github/groom/tests/sandbox-tests.sh

.github/groom/agent-sandbox.sh

@@ -3,17 +3,28 @@
 # agent-sandbox.sh — run an arbitrary command inside a bubblewrap (bwrap) jail.
 #
 # This is the confinement harness for the groom auto-builder's agent step
-# (BE-4302, phase 1). It gives an untrusted agent a network-connected shell that
-# can ONLY see: a read-only /usr + /etc, ephemeral /tmp + $HOME, the target clone
-# (read-only, or read-write worktree with a read-only .git), an explicit set of
-# read-only files, and one writable out-dir. Everything else on the host — other
-# repos, the runner's secrets, $HOME, $RUNNER_TEMP, $GITHUB_WORKSPACE, the host
-# process table — is invisible. The real API key never enters the jail; the agent
-# reaches Anthropic only through the broker (broker.mjs) on host loopback.
+# (BE-4302 phase 1; isolated-netns hardening BE-4421 phase 2). It gives an
+# untrusted agent a shell in an ISOLATED network namespace — an empty netns with
+# only loopback up (no host network, no host loopback services, no cloud metadata
+# at 169.254.169.254 / 168.63.129.16) — that can ONLY see: a read-only /usr + /etc,
+# ephemeral /tmp + $HOME, the target clone (read-only, or read-write worktree with
+# a read-only .git), an explicit set of read-only files, and one writable out-dir.
+# Everything else on the host — other repos, the runner's secrets, $HOME,
+# $RUNNER_TEMP, $GITHUB_WORKSPACE, the host process table — is invisible. The real
+# API key never enters the jail; the agent reaches Anthropic only through the
+# broker (broker.mjs), bind-mounted into the jail as a unix-domain socket at
+# /run/broker.sock (--uds). There is NO network egress — nothing else off-host is
+# reachable, so in-jail `git fetch` / `npm install` cannot work.
 #
 # Usage:
 #   agent-sandbox.sh --clone <path> --clone-mode ro|rw-git-ro --out-dir <path> \
-#       [--ro-file <path> ...] [--env KEY=VALUE ...] -- <command...>
+#       [--ro-file <path> ...] [--env KEY=VALUE ...] [--uds <host-socket-path>] \
+#       -- <command...>
+#
+#   --uds bind-mounts a host-side listening unix socket (the broker) to the fixed
+#   in-jail path /run/broker.sock (read-only: connect(2) to a socket works under a
+#   read-only bind, but the jail can't chmod/replace the shared inode).
+#   Omit it for a fully offline jail.
 #
 # The preflight FAILS LOUD: if a working bwrap sandbox cannot be established on
 # this runner image, the script exits non-zero and the command is NEVER run. It
@@ -30,7 +41,7 @@ die() {
 # minus the caller-supplied clone/ro-file/out-dir/env. `true` runs as the probe.
 selftest() {
 	bwrap \
-		--unshare-all --share-net \
+		--unshare-all \
 		--ro-bind /usr /usr \
 		--symlink usr/bin /bin \
 		--symlink usr/lib /lib \
@@ -86,7 +97,7 @@ PROFILE
 }
 
 main() {
-	local clone="" clone_mode="" out_dir=""
+	local clone="" clone_mode="" out_dir="" uds=""
 	local ro_files=() envs=() cmd=()
 
 	while [[ $# -gt 0 ]]; do
@@ -96,6 +107,7 @@ main() {
 			--out-dir) [[ $# -ge 2 ]] || die "--out-dir needs a value"; out_dir="$2"; shift 2 ;;
 			--ro-file) [[ $# -ge 2 ]] || die "--ro-file needs a value"; ro_files+=("$2"); shift 2 ;;
 			--env) [[ $# -ge 2 ]] || die "--env needs a value"; envs+=("$2"); shift 2 ;;
+			--uds) [[ $# -ge 2 ]] || die "--uds needs a value"; [[ -n "$2" ]] || die "--uds needs a non-empty value"; [[ -z "$uds" ]] || die "--uds may be given at most once"; uds="$2"; shift 2 ;;
 			--) shift; cmd=("$@"); break ;;
 			*) die "unknown argument: $1" ;;
 		esac
@@ -113,6 +125,22 @@ main() {
 		*) die "--clone-mode must be 'ro' or 'rw-git-ro' (got '${clone_mode:-}')" ;;
 	esac
 	[[ -d "$clone" ]] || die "clone path is not a directory: $clone"
+	# The broker socket is bind-mounted at its real path, so require absolute; and
+	# require it to already be a listening unix socket — a rw --bind of a missing or
+	# non-socket path would just give the jail a useless mountpoint. Fail loud.
+	if [[ -n "$uds" ]]; then
+		[[ "$uds" = /* ]] || die "--uds must be an absolute path (got '$uds')"
+		[[ -S "$uds" ]] || die "--uds path is not a listening unix socket (start the broker first): $uds"
+		# -S only proves the inode is a socket, not that a broker is actually
+		# listening — a stale socket from a crashed broker would pass -S yet the
+		# in-jail connect() then fails at runtime, breaking the fail-loud-before-
+		# running guarantee. Probe /healthz over the socket to confirm a live
+		# listener (best-effort: only when curl is present, matching the tests).
+		if command -v curl >/dev/null 2>&1; then
+			curl -fsS --max-time 5 --unix-socket "$uds" http://broker/healthz >/dev/null 2>&1 \
+				|| die "--uds socket has no live broker listening (healthz probe failed): $uds"
+		fi
+	fi
 
 	# out-dir must exist on the host before it can be bound rw into the jail; create
 	# it here so we can canonicalize it for the overlap check below.
@@ -134,7 +162,7 @@ main() {
 	preflight
 
 	local bwrap_args=(
-		--unshare-all --share-net --die-with-parent --new-session --clearenv
+		--unshare-all --die-with-parent --new-session --clearenv
 		--ro-bind /usr /usr
 		--symlink usr/bin /bin
 		--symlink usr/lib /lib
@@ -189,6 +217,19 @@ main() {
 		done
 	fi
 
+	# Bind the broker's unix socket into the isolated netns at a fixed path, READ-
+	# ONLY. connect(2) to a socket still works under a read-only bind — the kernel's
+	# read-only-fs EROFS check (sb_permission) fires only for regular files, dirs and
+	# symlinks, never for a socket inode — so the jail can still reach the broker,
+	# while --ro-bind additionally strips the agent's ability to chmod the shared
+	# socket inode (e.g. 000 to DoS itself, 0777 to widen host-side access). As a
+	# mountpoint the socket also can't be unlinked or replaced from inside the jail.
+	# bwrap auto-creates the dest. (Section 5 of sandbox-tests.sh exercises this exact
+	# in-jail connect over --ro-bind, so a regression here fails CI loudly.)
+	if [[ -n "$uds" ]]; then
+		bwrap_args+=(--ro-bind "$uds" /run/broker.sock)
+	fi
+
 	bwrap_args+=(--bind "$out_dir" "$out_dir" --chdir "$clone")
 
 	# stdout/stderr pass through to the host shell; the caller redirects stdout

.github/groom/broker.mjs

@@ -1,28 +1,69 @@
 // broker.mjs — a zero-dependency reverse proxy that holds the real Anthropic key
-// so a sandboxed agent never sees it (BE-4302, phase 1).
+// so a sandboxed agent never sees it (BE-4302 phase 1; UDS transport BE-4421 phase 2).
 //
-//   node broker.mjs <port>
+//   node broker.mjs <port|socket-path>
 //
-// The agent inside the bwrap jail (agent-sandbox.sh) talks to this broker on host
-// loopback with NO key of its own; the broker strips any inbound credential,
-// injects the real ANTHROPIC_API_KEY (read from its own env, never the jail's),
-// and forwards to api.anthropic.com. The key lives only in the host process; the
-// jail can spend against it but can never read it.
+// The agent inside the bwrap jail (agent-sandbox.sh) talks to this broker with NO
+// key of its own; the broker strips any inbound credential, injects the real
+// ANTHROPIC_API_KEY (read from its own env, never the jail's), and forwards to
+// api.anthropic.com. The key lives only in the host process; the jail can spend
+// against it but can never read it.
 //
-// Contract: listens on 127.0.0.1 only; forwards only /v1/* paths; deletes inbound
-// x-api-key / authorization before adding the real one; streams responses through
-// unbuffered so SSE works; logs method + path + status ONLY (never headers/body).
+// Transport: a numeric argv[2] keeps the legacy TCP mode (127.0.0.1:<port>,
+// retained for the fake-upstream test plumbing and back-compat); any other value
+// is treated as an absolute unix-domain-socket path (the phase-2 default — the
+// socket is bind-mounted into the isolated-netns jail at /run/broker.sock, since
+// the jail has no network egress). A relative path is refused.
+//
+// Contract: listens on 127.0.0.1 (TCP mode) or a unix socket only; forwards only
+// /v1/* paths; deletes inbound x-api-key / authorization before adding the real
+// one; streams responses through unbuffered so SSE works; logs method + path +
+// status ONLY (never headers/body). The request-handling contract is
+// transport-independent — TCP and UDS behave identically.
 //
 // BROKER_UPSTREAM_HOST / BROKER_UPSTREAM_PORT override the upstream target for
 // tests only; production leaves them unset and pins api.anthropic.com:443.
 
 import http from 'node:http';
 import https from 'node:https';
+import fs from 'node:fs';
 
-const port = Number(process.argv[2]);
-if (!Number.isInteger(port) || port <= 0 || port > 65535) {
-  console.error('broker: usage: node broker.mjs <port>');
-  process.exit(1);
+const arg = process.argv[2];
+let listenArgs; // spread into server.listen(): [port, host] for TCP or [path] for UDS
+let addr; // human label for the listen log line
+let uds; // the socket path in UDS mode (undefined in TCP mode); chmod'd after bind
+if (arg !== undefined && /^\d+$/.test(arg)) {
+  const port = Number(arg);
+  if (!Number.isInteger(port) || port <= 0 || port > 65535) {
+    console.error('broker: usage: node broker.mjs <port|socket-path>');
+    process.exit(1);
+  }
+  listenArgs = [port, '127.0.0.1'];
+  addr = `127.0.0.1:${port}`;
+} else {
+  const sockPath = arg;
+  if (!sockPath || sockPath[0] !== '/') {
+    console.error('broker: usage: node broker.mjs <port|socket-path> (socket path must be absolute)');
+    process.exit(1);
+  }
+  // Clear a stale socket left by a crashed prior run so listen() doesn't EADDRINUSE.
+  // Only ever unlink an actual SOCKET: an unconditional rmSync(force) would crash on
+  // a directory (EISDIR is not suppressed by force), silently delete a regular file
+  // at a mistyped path, and unlink a live socket. Stat first and fail loud on
+  // anything that isn't a socket. lstat so a symlink is treated as a non-socket.
+  try {
+    const st = fs.lstatSync(sockPath);
+    if (!st.isSocket()) {
+      console.error(`broker: refusing to start — ${sockPath} exists and is not a socket`);
+      process.exit(1);
+    }
+    fs.unlinkSync(sockPath);
+  } catch (e) {
+    if (e.code !== 'ENOENT') throw e; // absent path is the normal case
+  }
+  uds = sockPath;
+  listenArgs = [sockPath];
+  addr = sockPath;
 }
 
 const KEY = process.env.ANTHROPIC_API_KEY;
@@ -108,6 +149,17 @@ const server = http.createServer((req, res) => {
   req.pipe(upstream); // forward the request body streaming too
 });
 
-server.listen(port, '127.0.0.1', () => {
-  console.log(`broker listening on 127.0.0.1:${port} -> ${UPSTREAM_HOST}:${UPSTREAM_PORT}`);
+// A listen failure (EADDRINUSE, EACCES, a bad UDS dir) would otherwise surface as
+// an uncaught 'error' event and a raw stack trace; log it and exit non-zero.
+server.on('error', (e) => {
+  console.error(`broker: listen failed on ${addr}: ${e.message}`);
+  process.exit(1);
+});
+
+server.listen(...listenArgs, () => {
+  // Lock the UDS to owner-only. server.listen() creates it under the process umask,
+  // so a permissive umask could let another local user connect(2) to this
+  // credential-injecting proxy and spend the real key (they still can't read it).
+  if (uds !== undefined) fs.chmodSync(uds, 0o600);
+  console.log(`broker listening on ${addr} -> ${UPSTREAM_HOST}:${UPSTREAM_PORT}`);
 });

.github/groom/jail-shim.mjs

@@ -0,0 +1,44 @@
+// jail-shim.mjs — an in-jail TCP→UDS forwarder for the groom agent sandbox
+// (BE-4421, phase 2).
+//
+//   node jail-shim.mjs <port> <socket-path>
+//
+// The sandbox (agent-sandbox.sh) runs the agent in an ISOLATED network namespace
+// with only loopback up and no egress; the key-broker (broker.mjs) is reachable
+// only as a unix-domain socket bind-mounted into the jail at /run/broker.sock.
+// Agent tooling that speaks HTTP to a host:port (e.g. an Anthropic base URL) can't
+// dial a unix socket, so this shim listens on jail-local loopback and forwards
+// every connection to the socket. It runs INSIDE the jail as part of the
+// sandboxed command (background it, then exec the agent); an agent that kills it
+// only breaks its own API access.
+
+import net from 'node:net';
+
+const port = Number(process.argv[2]);
+const sockPath = process.argv[3];
+if (!Number.isInteger(port) || port <= 0 || port > 65535 || !sockPath) {
+  console.error('jail-shim: usage: node jail-shim.mjs <port> <socket-path>');
+  process.exit(1);
+}
+
+const server = net.createServer((c) => {
+  const u = net.connect(sockPath);
+  c.pipe(u);
+  u.pipe(c);
+  c.on('error', () => u.destroy());
+  u.on('error', () => c.destroy());
+});
+
+// The shim is backgrounded inside the jail while the caller captures the command's
+// stdout as the agent's JSON output; a listen failure surfacing as an uncaught
+// 'error' would kill the shim with a raw stack trace, so handle it and exit clean.
+server.on('error', (e) => {
+  console.error(`jail-shim: listen failed on 127.0.0.1:${port}: ${e.message}`);
+  process.exit(1);
+});
+
+server.listen(port, '127.0.0.1', () => {
+  // stderr, NOT stdout: the shim runs backgrounded alongside the agent whose stdout
+  // the caller captures as JSON — a banner on stdout would corrupt that capture.
+  console.error(`shim listening on 127.0.0.1:${port} -> ${sockPath}`);
+});