From 5b903be9fe198b7446c1f2ff584180b330af41e5 Mon Sep 17 00:00:00 2001 From: SUaDtL Date: Sat, 25 Jul 2026 23:18:11 -0400 Subject: [PATCH] fix(sandbox): --with-claude never runs NET_ADMIN without its firewall `resolveClaudeNetworkArgs` called `applyNetworkPolicy("egress-allowlist", ...)` and took only `.runArgs`, discarding `.firewallScript`. Its own comment said "The firewall script must still be applied INSIDE the box by the caller" - and no such caller exists. Nothing in production referenced `firewallScript` at all; only tests did. So selecting `anthropic-only` produced a container with `--cap-add NET_ADMIN --cap-add NET_RAW` on a custom bridge and NO iptables rules: wide-open egress PLUS the capability to manipulate its own networking, around a live OAuth token. That is strictly worse than plain default networking, and it is the posture an operator picks precisely when they want the box locked DOWN. THE FIX, in two halves that have to go together: * The flags and the rules now travel as one plan. Returning only the argv is what let the script be dropped on the floor, so the resolver hands back both, and `anthropic-only` throws rather than proceeding if network.ts ever produces no script - an unenforced allowlist under NET_ADMIN is worse than no allowlist. * `runClaudeInside` installs the rules inside the box after start, and DESTROYS the box if it cannot. Returning the id would tell the caller it is locked down when it is not, which is the failure that actually matters. The teardown is best-effort and deliberately not conditional on its own success: if it also fails there is nothing further this process can do, and the thrown error names the container so an operator can finish the job. `offline` still execs nothing. It has no interface, so there is nothing to firewall and an extra exec would be pure attack surface. The script it installs is the one network.ts produced, asserted directly against `applyNetworkPolicy(...).firewallScript` rather than re-derived - a second derivation is a second thing that can drift from the allowlist. WHY THIS WAS SAFE UNTIL NOW, AND WHY IT IS FIXED FIRST. `runClaudeInside` has zero callers, and esbuild tree-shakes it out entirely: `grep -c runClaudeInside sandbox.js` is 0, so the documented `--with-claude` feature is not merely unwired, it is ABSENT from the shipped artifact. #377 is the issue to give it a caller. Fixing the posture before it becomes reachable is the whole point, and it is why sandbox.js is unchanged by this commit. 5 new tests. Verified red first: 3 failed against the old implementation, with the box started and no exec performed. Then mutation-tested after: skipping the firewall exec -> 3 failed, dropping the teardown -> 1 failed, discarding the script again -> 3 failed. Every half has a test that notices its absence. Full ca-sandbox suite 263 passed across 20 files; typecheck clean. Refs #377 - this is the security half. The gated `claude-inside.js` entrypoint, which is what gives this code a caller, follows separately. --- plugins/ca-sandbox/CHANGELOG.md | 1 + .../ca-sandbox/tools/claude-inside.test.ts | 110 ++++++++++++++++++ plugins/ca-sandbox/tools/claude-inside.ts | 77 ++++++++++-- 3 files changed, 180 insertions(+), 8 deletions(-) diff --git a/plugins/ca-sandbox/CHANGELOG.md b/plugins/ca-sandbox/CHANGELOG.md index ac4e66d5..9a49465a 100644 --- a/plugins/ca-sandbox/CHANGELOG.md +++ b/plugins/ca-sandbox/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to the **ca-sandbox** plugin are recorded here. Format follo ## [0.1.5] — 2026-07-24 — Teardown failures are surfaced and fail ### Fixed +- **`--with-claude`'s `anthropic-only` posture ran NET_ADMIN with no firewall (#377).** `resolveClaudeNetworkArgs` called `applyNetworkPolicy("egress-allowlist", ...)` and took only `.runArgs`, discarding `.firewallScript` — with a comment saying the caller would apply it, and no such caller anywhere in the tree. The result was a container with `--cap-add NET_ADMIN --cap-add NET_RAW` on a custom bridge and no iptables rules: wide-open egress *plus* elevated network capabilities around a live OAuth token, which is strictly worse than plain default networking and the opposite of what an operator selecting that posture is asking for. The flags and the rules now travel together, `runClaudeInside` installs them inside the box after start, and a box whose firewall cannot be applied is **destroyed rather than returned** — handing back the id would tell the caller it is locked down when it is not. `offline`, the guaranteed posture, still execs nothing, because it has no interface to firewall. Unexploitable before this fix only because `runClaudeInside` has no callers yet, which is exactly why it is fixed before it gets one. - **One unreachable daemon reads as one failure (#433).** Discovery listed containers and volumes, then the post-sweep verification re-listed the same two scopes against the same dead daemon — `DOCKER_HOST=tcp://127.0.0.1:1 node sandbox.js prune` reported `failureCount: 4` for one fact. Listing failures now dedupe on the failure's *shape* rather than its text, because a real daemon returns the same connection error carrying different URLs; two listings failing *differently* still read as two. Removal failures dedupe on the whole identity, so two containers refusing for their own reasons are never collapsed. - **`prune`'s verification is scoped to what it targeted (#433).** It re-listed the global `ca.sandbox=1` scope, so a sandbox another process created *after* discovery landed in `remainingContainers` and produced "These objects may be running UNTRUSTED code" for something prune never touched. It failed safe, but a scary false positive teaches operators to ignore the signal — which defeats the point of #393. An object prune did target and could not remove is still reported. - **A deliberately kept volume is never named as a leak (#433).** Under `--keep-volume` no volume is targeted for removal, so none can be remaining. Filtering against `keptVolumes` was not enough: when the *discovery* listing failed that list was empty while the *verification* listing still succeeded, so the volume the operator explicitly asked to keep was reported as remaining — at exactly the wrong moment. diff --git a/plugins/ca-sandbox/tools/claude-inside.test.ts b/plugins/ca-sandbox/tools/claude-inside.test.ts index 2b5ac309..0f9716a0 100644 --- a/plugins/ca-sandbox/tools/claude-inside.test.ts +++ b/plugins/ca-sandbox/tools/claude-inside.test.ts @@ -31,8 +31,11 @@ import { TOKEN_ENV_VAR, buildClaudeImageDockerfile, buildClaudeRunArgs, + runClaudeInside, + ANTHROPIC_ALLOW_HOSTS, TokenCoMountRejectedError, } from "./claude-inside.ts"; +import { applyNetworkPolicy } from "./network.ts"; import { SANDBOX_USER, hardeningFlags } from "./run.ts"; // -------------------------------------------------------------------------- @@ -360,3 +363,110 @@ d("claude-inside [docker] — env-token auth + named-volume persistence (AC-12)" expect(r.status).not.toBe(0); }, 180_000); }); + +// --------------------------------------------------------------------------- +// #377 - `anthropic-only` was NET_ADMIN with no firewall. +// +// `resolveClaudeNetworkArgs` called applyNetworkPolicy("egress-allowlist", ...) +// and took ONLY `.runArgs`, discarding `.firewallScript`. Its own comment says +// "The firewall script must still be applied INSIDE the box by the caller" - +// and there is no such caller. Nothing in production references +// `firewallScript` at all; only tests do. +// +// So the `anthropic-only` posture produced a container with `--cap-add +// NET_ADMIN --cap-add NET_RAW` on a custom bridge and NO iptables rules: wide +// open egress PLUS elevated network capabilities, around a live OAuth token. +// That is strictly worse than plain default networking, and it is the posture a +// user selects when they want the box locked down. +// +// It is unexploitable today only because `runClaudeInside` has zero callers +// (#377 is the issue to wire it up). Fixing it BEFORE it becomes reachable is +// the entire point. +// --------------------------------------------------------------------------- +describe("#377 - a token-bearing box never runs NET_ADMIN without its firewall", () => { + const baseOpts = { + image: "ca-sbx-claude:test", + token: "DUMMY-NOT-A-REAL-TOKEN", + homeVolume: "ca-sbx-claude-home-test", + }; + + it("applies the firewall inside the box for anthropic-only", () => { + const calls: string[][] = []; + const dockerRun = (args: string[]) => { + calls.push(args); + return { code: 0, stdout: args[0] === "run" ? "container-id-1\n" : "", stderr: "" }; + }; + const id = runClaudeInside({ ...baseOpts, netPolicy: "anthropic-only" }, dockerRun); + expect(id).toBe("container-id-1"); + + // The run itself still carries the capabilities... + expect(calls[0]!.join(" ")).toContain("NET_ADMIN"); + // ...and something must then install the rules INSIDE the box. + const execs = calls.slice(1).filter((a) => a[0] === "exec"); + expect(execs.length, "no firewall was applied after start").toBeGreaterThan(0); + const applied = execs.map((a) => a.join(" ")).join("\n"); + expect(applied).toContain("container-id-1"); + expect(applied).toMatch(/iptables/); + }); + + it("destroys the box when the firewall cannot be applied", () => { + // Fail closed. A started, token-bearing container with NET_ADMIN and no + // rules must never be handed back to a caller - returning the id would be + // worse than never starting it, because the caller believes it is locked + // down. + const calls: string[][] = []; + const dockerRun = (args: string[]) => { + calls.push(args); + if (args[0] === "run") return { code: 0, stdout: "container-id-2\n", stderr: "" }; + if (args[0] === "exec") return { code: 1, stdout: "", stderr: "iptables: Permission denied" }; + return { code: 0, stdout: "", stderr: "" }; + }; + expect(() => runClaudeInside({ ...baseOpts, netPolicy: "anthropic-only" }, dockerRun)) + .toThrow(/firewall/i); + const teardown = calls.filter((a) => (a[0] === "rm" || a[0] === "kill") && a.includes("container-id-2")); + expect(teardown.length, "the unprotected box was left running").toBeGreaterThan(0); + }); + + it("does not exec anything extra for offline, which needs no rules", () => { + // `offline` gets no interface at all, so there is nothing to firewall and + // an extra exec would be pure attack surface. + const calls: string[][] = []; + const dockerRun = (args: string[]) => { + calls.push(args); + return { code: 0, stdout: args[0] === "run" ? "container-id-3\n" : "", stderr: "" }; + }; + const id = runClaudeInside({ ...baseOpts, netPolicy: "offline" }, dockerRun); + expect(id).toBe("container-id-3"); + expect(calls.filter((a) => a[0] === "exec")).toHaveLength(0); + }); + + it("defaults to offline, so the guaranteed posture needs no opt-in", () => { + const calls: string[][] = []; + const dockerRun = (args: string[]) => { + calls.push(args); + return { code: 0, stdout: args[0] === "run" ? "container-id-4\n" : "", stderr: "" }; + }; + runClaudeInside({ ...baseOpts }, dockerRun); + expect(calls[0]!.join(" ")).toContain("--network"); + expect(calls.filter((a) => a[0] === "exec")).toHaveLength(0); + }); + + it("the firewall script it applies is the one network.ts produced", () => { + // Not a re-implementation: the rules must come from the single owner, so a + // change to the allowlist cannot silently diverge from what is installed. + const plan = applyNetworkPolicy("egress-allowlist", { + allowHosts: [...ANTHROPIC_ALLOW_HOSTS], + networkName: "ca-sbx-claude-egress", + }); + expect(plan.firewallScript).toBeTruthy(); + + const calls: string[][] = []; + const dockerRun = (args: string[]) => { + calls.push(args); + return { code: 0, stdout: args[0] === "run" ? "cid\n" : "", stderr: "" }; + }; + runClaudeInside({ ...baseOpts, netPolicy: "anthropic-only" }, dockerRun); + const execArgv = calls.find((a) => a[0] === "exec")!; + expect(execArgv[execArgv.length - 1]).toBe(plan.firewallScript); + }); +}); diff --git a/plugins/ca-sandbox/tools/claude-inside.ts b/plugins/ca-sandbox/tools/claude-inside.ts index 8c9ddb7f..5b85bf30 100644 --- a/plugins/ca-sandbox/tools/claude-inside.ts +++ b/plugins/ca-sandbox/tools/claude-inside.ts @@ -187,19 +187,41 @@ export type ClaudeRunOptions = { * the Anthropic domains (custom bridge + NET_ADMIN/NET_RAW caps). Anything else is * a hard error — a token-bearing box must never get wide-open egress. */ -function resolveClaudeNetworkArgs(policy: ClaudeNetPolicy): string[] { +/** Run-time flags plus the rules that must be installed inside the box. #377: + * these travel TOGETHER. Handing back only the flags is what produced a + * NET_ADMIN container with no firewall. */ +type ClaudeNetworkPlan = { runArgs: string[]; firewallScript?: string }; + +function resolveClaudeNetworkPlan(policy: ClaudeNetPolicy): ClaudeNetworkPlan { switch (policy) { case "offline": - return applyNetworkPolicy("offline").runArgs; + // No interface at all, so there is nothing to firewall. An extra exec + // here would be pure attack surface. + return { runArgs: applyNetworkPolicy("offline").runArgs }; case "anthropic-only": { // The Anthropic-domains allowlist. The allowlist machinery is EXPERIMENTAL // (Spike C); offline is the only GUARANTEED posture for a token-bearing box. - // The firewall script must still be applied INSIDE the box by the caller - // (network.ts owns it); here we only contribute the run-time flags. - return applyNetworkPolicy("egress-allowlist", { + // + // #377: this used to return `.runArgs` ONLY, discarding `.firewallScript`, + // with a comment saying the caller would apply it - and no caller existed. + // The result was `--cap-add NET_ADMIN --cap-add NET_RAW` on a custom bridge + // with NO iptables rules: wide-open egress PLUS elevated network + // capabilities around a live OAuth token, which is strictly worse than + // plain default networking, and is the posture a user picks when they want + // the box locked DOWN. The script now travels with the flags so it cannot + // be dropped on the floor again. + const plan = applyNetworkPolicy("egress-allowlist", { allowHosts: [...ANTHROPIC_ALLOW_HOSTS], networkName: "ca-sbx-claude-egress", - }).runArgs; + }); + if (!plan.firewallScript) { + throw new Error( + "ca-sandbox: --with-claude refuses 'anthropic-only' without a firewall script - " + + "the posture grants NET_ADMIN/NET_RAW, so an unenforced allowlist is worse " + + "than no allowlist. Use 'offline', the guaranteed posture.", + ); + } + return { runArgs: plan.runArgs, firewallScript: plan.firewallScript }; } default: { // Exhaustiveness: a non-hardened policy is rejected, never passed through. @@ -248,7 +270,7 @@ export function buildClaudeRunArgs(opts: ClaudeRunOptions): string[] { } const netPolicy: ClaudeNetPolicy = opts.netPolicy ?? "offline"; - const networkArgs = resolveClaudeNetworkArgs(netPolicy); + const networkArgs = resolveClaudeNetworkPlan(netPolicy).runArgs; // Mounts go through the ONE chokepoint (mounts.ts): the home named volume at // HOME (so .claude persists) and a tmpfs /tmp for a read-only root. No bind can @@ -298,6 +320,18 @@ export function buildClaudeRunArgs(opts: ClaudeRunOptions): string[] { ]; } +/** + * The rules a `--with-claude` box must have installed after it starts, for + * `opts`. `undefined` when the posture needs none (offline has no interface). + * + * Exported so the entrypoint and the tests read the SAME answer the runner + * acts on, rather than re-deriving it - a second derivation is a second thing + * that can drift from network.ts's allowlist. + */ +export function claudeFirewallScript(opts: ClaudeRunOptions): string | undefined { + return resolveClaudeNetworkPlan(opts.netPolicy ?? "offline").firewallScript; +} + /** Kept as a distinct exported name for existing importers — one underlying * shape (docker.ts's RunResult), never a second parallel definition. */ export type ClaudeRunResult = RunResult; @@ -321,5 +355,32 @@ export function runClaudeInside( `${(r.stderr || r.stdout).slice(-2000)}`, ); } - return r.stdout.trim(); + const id = r.stdout.trim(); + + // #377: install the allowlist INSIDE the box, or destroy it. + // + // The run flags for `anthropic-only` grant NET_ADMIN and NET_RAW on a custom + // bridge. Until the rules land, that container has wide-open egress AND the + // capability to manipulate its own networking, while holding a live OAuth + // token - strictly worse than the default posture, and the opposite of what + // the operator asked for. So a box whose firewall cannot be applied is torn + // down rather than returned: handing back the id would tell the caller it is + // locked down when it is not, which is the failure that matters. + const firewallScript = claudeFirewallScript(opts); + if (firewallScript === undefined) return id; + + const applied = dockerRun(["exec", "--user", "root", id, "sh", "-c", firewallScript]); + if (applied.code !== 0) { + // Best effort, and deliberately not conditional on its own success: if the + // teardown ALSO fails there is nothing further this process can do, and the + // thrown error names the container so an operator can finish the job. + dockerRun(["rm", "-f", id]); + throw new Error( + `ca-sandbox: --with-claude could not apply the egress firewall to ${id} ` + + `(exit ${applied.code}); the container has been destroyed rather than left ` + + `running with NET_ADMIN and no rules around a live token.\n` + + `${(applied.stderr || applied.stdout).slice(-2000)}`, + ); + } + return id; }