Bounded network egress for sandboxed kernels - #280
Draft
KB (KB-syntheticsciences) wants to merge 33 commits into
Draft
Bounded network egress for sandboxed kernels#280KB (KB-syntheticsciences) wants to merge 33 commits into
KB (KB-syntheticsciences) wants to merge 33 commits into
Conversation
Binary network is the wrong granularity: deny locks kernels out of PyPI and the scientific APIs, allow is unrestricted egress. A spike established an enforceable middle — --unshare-net blocks every host including the host's own loopback, while a bind-mounted unix socket still crosses the namespace, so the socket is the only route out and the proxy decides what is reachable. Recorded before implementation because it is a breaking change to a documented config key, and because it deletes the separate install sandbox rather than adding a component.
The mechanism paragraph quotes two measured values from the spike; three other statements were the ADR's own forward decisions but read in the same flat register, so a reader couldn't tell which was which. Rewords those three to say plainly that they are decisions this record is taking, and marks the seatbelt warning behaviour as unverified rather than stating it as settled fact. No content removed — the brief's ten required points all still appear.
Ported from the spike on proto/sandbox-allowlist-proxy. The matcher is pure and separable so the allowlist is testable without sockets. Both measured fixes carried across: the shim buffers writes arriving before the upstream unix connection resolves, and the proxy rewrites absolute-form to origin-form for plain HTTP.
allowlist keeps --unshare-net and binds a unix socket as the only route out, so the proxy on the far end is enforcement rather than advice. The builder refuses allowlist without a socket path instead of silently producing an open sandbox. Seatbelt has no namespace equivalent, so it reads allowlist as deny: falling back to allow would grant unrestricted egress to a user who asked for a bounded one.
…ments buildPolicy now filters `egress` through the same tooBroadToConfine gate as writable/unreadable. An over-broad egress (e.g. $HOME, "/") was reaching bubblewrapArgs unfiltered and becoming a read-write --bind, defeating write containment entirely. A rejected path is dropped, which leaves "allowlist" without an egress socket, so bubblewrapArgs' existing missing-egress check still throws — fails closed either way. Also corrects two comments that asserted things that weren't true: the --bind comment overstated the socket bind as the access-control mechanism (unshare-net is; the bind only makes the path reachable), and plan()'s doc comment no longer mentioned the allowlist-without-egress throw it gained in the previous change.
…heck The round-1 fix checked tooBroadToConfine against the raw egress string, so a trailing slash, a double slash, or an unresolved ".." bypassed the gate entirely (strict string equality never matched) while resolving to the exact same over-broad path on disk. writable/unreadable were never vulnerable to this because they already went through dedupe()'s path.resolve() before the same check. Route egress through the same dedupe() call instead of hand-rolling separate normalization, so the two paths cannot drift apart again. Test coverage widened from the one literal string to the class of lexical variants (trailing slash, double slash, unresolved ..), plus a control asserting a legitimate non-broad socket still gets bound.
pip, requests and curl take a host:port proxy and none speak unix sockets, so a loopback listener inside the namespace bridges to the bind-mounted socket. It runs from the OpenScience binary, which is already visible under --ro-bind / /, so nothing extra ships. Script composition is a pure function with quoting tests, because a path with a space or a quote in agent-authored code would otherwise split the command.
…turally Two live-verified breaks in the composed egress shim: The shim went through yargs' global middleware before reaching its handler, which opens a log file (EROFS under the sandbox's read-only root) and fetches over HTTP (hangs under --unshare-net). shimScript redirected the shim's own output to /dev/null, so this failed completely silently: network "allowlist" behaved exactly like deny. Handle __egress-shim as a raw argv check before any yargs construction so it can never reach the middleware, regardless of what middleware grows there later. The dev-mode launcher resolved its target script from Bun.main, which is whatever launched the current process. Under bun test that's the test file, not the CLI entry - the exact context Task 6's live test runs under. Resolve the entry from sandbox.ts's own location instead, a structural relationship unaffected by what invoked the process. Fixing that surfaced a second issue: the launcher's content-addressed cache location under Global.Path.state resolved under the OS tmp dir during bun test (test isolation redirects every XDG dir there), and bubblewrapArgs unconditionally mounts a fresh tmpfs over /tmp inside the sandbox - so the launcher silently didn't exist from inside and the shim never started. Anchored the launcher to the repo checkout itself instead, which neither test nor a real user's env can redirect. Also added a bounded marker-file readiness wait before exec, since the real command was exec'd immediately and anything doing network I/O in the shim's ~600ms startup window got connection-refused.
…location Location-based fixes for --tmpfs /tmp masking (Global.Path.state, then the launcher's own checkout directory) both re-anchored the same bug instead of removing it: a real checkout under /tmp (git worktree add /tmp/..., a CI mktemp -d clone, a container build) masks the launcher exactly like an XDG dir redirected under os.tmpdir() during bun test does. There is no location immune to both. Bind the launcher back in explicitly after --tmpfs /tmp instead, the same way the egress socket already is (--ro-bind-try, read-only: it's executed, never written to, from inside). Policy gains readBind for this; the launcher lives in Global.Path.bin again, matching ensureAtlasBinDir, purely for tidiness now that its location no longer needs to be "safe." Split the dev-mode launcher's target off of src/index.ts into a new minimal entry (egress-shim-entry.ts) that imports only Egress.serveShim. index.ts's full graph pulls in Global's unguarded top-level cache-version write (EROFS under a read-only tree) and a live models.dev fetch, both reachable before any argv check could skip them - live-reproduced with a stale/read-only cache dir. A compiled binary has no separate entry to redirect to, so it still evaluates that graph; documented as a residual in the Task 4 report rather than restructuring index.ts's ~30 command imports into dynamic ones. Also: the readiness wait now sleeps in whole seconds, since fractional sleep is a coreutils extension busybox doesn't reliably support and a rejected sleep would skip the wait entirely instead of slowing it down; and the live sandbox-execution test resolves bash from the same Bun.which() gate it skips on, instead of hardcoding /usr/bin/bash, so it skips rather than fails on Alpine/non-usrmerge Debian.
…verlaps Two more instances of the /tmp-masking class, both live-reproduced: The dev bind list covered the launcher and the package root but not the interpreter the launcher's exec line names (process.execPath) - it can itself live under /tmp (a portable bun install, $HOME under /tmp), independent of where the launcher or checkout live. Reproduced with a bun binary staged under /tmp: byte-identical failure signature to the original bug. Now bound alongside the other two. bubblewrapArgs emits readBind after the writable --bind-try loop, so a later read-only mount at (or inside) an already-writable path shadows it. With a workspace that is or contains this package's own checkout - the self-hosting case Task 5 will dogfood - binding the package root read-only turned src/sandbox back read-only despite being nominally writable. Fixed by excluding any readBind path already covered by an actually-bound writable root before emitting it. "/tmp" itself needed special handling: it is always nominally writable (tempDirs() adds it unconditionally) but is deliberately never bound - the fresh tmpfs already provides it - so treating it as "covers everything under it" reintroduced the original masking bug for a launcher that resolves under /tmp during bun test. Caught by the existing live test before this ever left the working tree. Only one containment direction is guarded (a readBind path inside a writable root); the reverse has no realistic trigger given today's three readBind candidates and would need reordering these mounts to handle, risking the same shadowing bug in the other direction. Consolidated the readiness-marker literal, previously duplicated across three files and kept in sync by comment only, into one exported constant all three import. Added regression coverage for both: the live test parametrized over an interpreter staged under /tmp (stages a real bun copy, drives a standalone script through it - bun:test itself isn't the thing under test), and a write-probe under a workspace that overlaps the package root. Both have negative controls confirming they fail without their respective fix.
…disk The shim's launcher exec'd bun against egress-shim-entry.ts, so every path bun touched resolving that source had to be bound back past --tmpfs /tmp. Four revisions bound the paths their author thought of and each missed one. The last: in this bun workspace an npm import resolves through backend/cli/node_modules/<pkg>, a symlink into the monorepo-root store one level above the package root, so binding the package root bound the link and not its target. Reproduced against a /tmp-relocated checkout with a real hoisted store — one added import turned the shim into ENOENT, connection refused, silent. Build the entry into a self-contained bundle instead and exec that: at run time bun opens one file, so the bound set is closed by construction — the launcher, the bundle, the interpreter — rather than a list that has to keep pace with an import graph. bun build costs 3ms, once per process, on the allowlist path only, and fails loudly at wrapArgv time. The artifacts are content-addressed and renamed into place, so a rebuild cannot overwrite one another process is executing and a stale launcher/bundle pair cannot form. That drops the package root from readBind, which also removes the shadowing hazard the writable-overlap exclusion guards: every remaining path is a regular file, so nothing can nest inside one. The old justification for guarding only that direction claimed no workspace would sit under those locations, which is not true — write roots also come from session grants and allowWrite — so it now gives the reason that holds. Two tests, neither needing the relocated-checkout fixture: the generated bundle carries no import specifier but builtins, and the shim still bridges with its own source entry masked to /dev/null.
…ltin list bun build strips the node: prefix, so a bundled `import "node:net"` reaches the guard as `from "net"` and the prefix filter flagged it — a false alarm on the next honest edit, aimed at the one check that closes the npm-import class. The same filter excused any package named bun-something, and its regex never matched a bare side-effect `import "x"` at all. Ask builtinModules instead, after stripping node:; it already carries bun's own entries. Measured on real bundles: net passes, diff / bun-pty / fuzzysort are all flagged. Two doc corrections alongside it. The bundle digest also varies with the process cwd, since bun build writes cwd-relative module banners — that is the dimension that actually varies per invocation. And the residual list was missing a fourth entry: a dependency loading a native binding bundles cleanly and still dlopens a .so at run time, which is not an import specifier and so is invisible to both the reasoning and the guard.
One proxy per process, held in a lazily-created singleton with a disposer (not Instance.state: a global config write disposes every open instance, and the proxy must outlive that or every kernel bound to its socket loses its only route out). Socket lives under the state directory. Rules are read per connection rather than captured at start: refreshed on every ensure() and reactively on global config changes via GlobalBus, so editing the allowlist takes effect without tearing down kernels - which is why proxy policy stays out of the generation hash. sandbox.network defaults to allowlist. Both cases of the proxy variables reach kernels: curl reads lowercase http_proxy and ignores the uppercase form for HTTP. Widened three downstream network enums (execution authority, sandbox settings route, persisted job records) plus a fourth (KernelEnvironment) surfaced by typecheck. Exported Sandbox.SHIM_PORT so EgressRuntime.ensure() returns the same port sandbox.ts already uses, instead of a second constant that could drift.
network defaulting to allowlist exposed that no caller of Sandbox.wrapArgv supplied the egress socket bubblewrapArgs requires for that policy, so every sandboxed kernel, terminal, and compute job spawn threw instead of running. Sandbox.plan() (the bash tool's path) never composed a shim at all, so pip/curl/uv - the feature's motivating case - had zero network under allowlist regardless. EgressRuntime.egressFor(policy) is the single decision point: it starts the proxy only when it would actually be used (bubblewrap backend, network "allowlist"), so a terminal on macOS or under network "deny" never pays for a proxy it has no way to reach. Every wrapArgv caller (notebook/rkernel/biology kernels, compute jobs x4, pty terminals) and bash.ts's plan() call now route through it and merge the returned Wrapped/Plan.env into the spawned process's environment, which had been an unconsumed seam since Task 4. Sandbox.plan() now composes the same loopback shim wrapArgv does, for a shell -c command instead of a file/args pair, by feeding shimScript the shell invocation as its argv - one composition, not two. Widened two test files' stale "deny"-only assertions to allow the new allowlist default, widened five notebook-test/command-runtime polling budgets that were tuned for pre-shim startup latency (the shim's own wait can take up to ~3s), and made one shell.test.ts assertion filter by pid instead of assuming absolute call order on a process-wide process.kill mock - a race that was always latent but only became probable once real sandboxed spawns became this common in the suite. Full suite: 74 failures -> 1 (an npm-pack environment flake confirmed present on the pre-Task-5 baseline too, unrelated to this branch).
…wn latency Three live-verified Criticals on the egress path. Backpressure. Both bridges called Socket.write and discarded the byte count it returns, with no drain handler on either side, so every transfer past one kernel send buffer lost its tail: 40 MB arrived as 2.8 MB through the proxy and 11.6 MB through shim + proxy, and pypi.org/simple/ (44,841,256 bytes) came back as 4.7/10.8/5.6 MB with "bad record mac" from curl. A pump per direction now queues what the destination refused, flushes from that destination's drain, pauses the source while a backlog exists, and defers end() until the queue has gone out. All three sizes now arrive byte-exact. Latched start. EgressRuntime cached the start promise unconditionally, so one transient failure was replayed to every later caller for the process lifetime, and stop() re-raised it instead of clearing it. Under the allowlist default that is every bash command, terminal, kernel and compute job broken until restart. A rejected start now un-caches itself, stop() is safe after one, and the bind failure carries a message naming what depends on it. Callers still throw rather than degrade: silently downgrading to no-proxy is the failure this feature keeps producing. Spawn latency. shimScript polled for readiness at whole-second granularity while the bundled shim binds in ~12 ms, so every sandboxed spawn paid a flat second whether or not it touched the network -- n=8, 1006 ms against 3 ms for network "deny". The interval is now settled once by a single fractional sleep whose stderr is discarded, falling back to whole seconds where busybox rejects it; same 3s cap either way. Same measurement: 26 ms. Regression tests move real volume (8 MB, both directions, byte-exact), drive a real start failure, and time the composed script through a real /bin/sh. Each fails against the code it fixes.
Both bridges dial from inside an async handler, and Bun does not serialize those handlers -- a second chunk, or a client's FIN, re-enters while the first call is parked on await Bun.connect. Two defects lived in that window. An aborted connection stranded a socket at both ends. A client whose FIN lands before Bun.connect(unix) resolves has its close handler run while there is no link yet, so it tears down nothing, and the socket the dial then produces is owned by nobody -- which also pins the host proxy's accepted connection. Measured across three processes, 300 connect-then-close connections leaked 0.897 fd/conn in the shim and 0.897 in the host proxy; now 0.000 in both, with completed connections unchanged at 0.000 throughout. Bounded by the sandbox's lifetime, which for a kernel or terminal is hours. One client produced several upstream dials. A body arriving after its head re-entered data, found no link, re-parsed the same buffered head and dialled again: 2 upstream connections against a local origin, 4 against a real remote one, both carrying a duplicate of a non-idempotent request with the body split between them. Reachable in practice -- wrapArgv sets HTTP_PROXY too, and "a request with a body" is the shape NCBI E-utilities recommends for large id lists, against a host in DEFAULT_RULES. Both close on one four-state phase claimed synchronously before the await. The body needs no second queue: everything before the link stays in the one buffer already there, and rest is sliced after the dial rather than before, so bytes that arrive during it go upstream in order by construction. Three regression tests count sockets opened against sockets closed at a stand-in upstream, so they measure the invariant without /proc. All three fail against the code they fix, three runs out of three.
Both pre-link phases of the host proxy buffered without limit, and the
proxy runs in the CLI's own process — so a single sandboxed process could
exhaust, and then kill, its own supervisor.
head no dial is ever attempted on this path, so nothing bounded it.
93 MiB of never-terminated head took the host process from
36.0 MB to 1344.9 MB of RSS in 8 s, still climbing.
dialing a complete CONNECT to an allowlisted host that black-holes SYNs.
2048.6 MiB blasted in 8 s took it from 36.0 MB to 2120.2 MB and
then killed it with RangeError: Out of memory, dial still in
flight and ~2 minutes of kernel SYN retries left to go.
The head gets a 64 KiB cap (Squid's request_header_max_size default, the
most generous of the conventional caps) and a 431. There is no
backpressure to apply there: the terminator is what the parse waits for,
so declining to read would deadlock rather than end the connection.
The dial window gets real backpressure instead of a cap — client.pause()
for as long as the dial is in flight, so the bytes stay in the client's
own buffer and there is no limit to tune. The previous round declined
this on the grounds that pausing would suppress the FIN that reports the
client leaving. Measured, it does not: with delivery demonstrably stopped
(0.21 MiB through a paused socket against 256 MiB unpaused), the peer's
end() still produced close while the pause was in force, for FIN and RST
alike. serveShim's identical window is paused for the same reason.
Dials also now time out at 30 s rather than riding the kernel's ~130 s
SYN-retry budget, answering 504 instead of hanging undiagnosably.
After, same measurements: head 36.0 -> 38.3 MB (+2.3), dial 36.0 -> 37.5
MB (+1.5) with the process alive and the client's own writes stalled.
Three regression tests, each failing against the parent commit: no
response at all for the head, 2,147,690,880 bytes accepted for the dial,
and no answer within 20 s for the timeout. Re-verified unchanged: 8 MB
byte-exact both directions through both bridges, flat RSS under a stalled
reader (origin stopped at 6.1 MiB, proxy +2.9 MB over 10 s), 0.000 fd/conn
across 600 aborts, one upstream dial per client, and a real bwrap fetch of
pypi.org/simple/ at 44,841,256 bytes matching the host's sha256 3/3 with
the deny control still returning 403.
…ss defaults Four held-back findings from the task-5 review, fixed together: - allowHosts was inert: the settings route PatchSchema silently dropped it (zod strips unknown keys) and the CLI never exposed it. Both now accept it, reaching the already-working reactive proxy reload. - cli/cmd/sandbox.ts's `network` choices still only listed "allow"/"deny", so `--network allowlist` was rejected and a user on "deny" had no CLI path back to the new default; the `as "allow" | "deny"` cast that hid this from tsgo is gone along with it. Status/help text updated to match. - egressFor() and decide()/buildPolicy() answered "what does a missing enabled/network mean" differently in both directions, invisible from today's five fully-resolved production callers but live for any other. Sandbox.resolved() is now the one place both read from, with a regression test pinning each direction. - ExecutionAuthority.Decision.sandbox.network is a second copy of the persisted enum Job.sandbox.network carries (via Job.authority) — recorded where both schemas live, including that ComputeJobs.read() fails the whole job history file on one unparseable record, not just that record. Verified past the type system: PUT to the settings route and `sandbox enable --network allowlist --allow-host` each drive a real sandboxed curl through to an allowed host and a 403 off a disallowed one, and each is shown to have been impossible before this change (silently dropped field / rejected CLI choice) in the same isolated run.
Asserts allowlisted hosts reach 200, denied hosts do not, and — the load-bearing pair — that direct egress with the proxy unset fails and getent resolves nothing. Without those two the test would prove the proxy works, not that it is the only way out. Skipped where bubblewrap or curl is absent rather than failing.
curl's own -m budget on the volume test matched the outer bun:test timeout exactly, with bwrap spawn and shim-readiness overhead layered on top before curl even starts. A slow-but-working download would hit the outer timeout first, trading curl's own diagnostic for a generic one and deferring the proxy's cleanup until the abandoned promise resolves. Also strips ALL_PROXY/NO_PROXY (and lowercase) alongside the existing HTTP(S)_PROXY vars in the direct-egress subshell, so a host exporting ALL_PROXY can't route that check through an unrelated proxy.
…chmod it shut The egress socket was bind-mounted read-write, and the bind shares the host inode: a sandboxed process could discover the path via /proc/self/mountinfo and `chmod 000` it, which persists on the host and disables egress for every kernel/terminal/job sharing this one process-lifetime socket. --ro-bind blocks chmod (EROFS) while still permitting connect() — verified live: a real bwrap run shows chmod failing with "Read-only file system" while a plain client still gets a reply over the same bind, and (for contrast) the same run with --bind reproduces the original vulnerability end to end (chmod succeeds, the host-side connect then fails with EPERM). Added a live regression test proving both properties together.
…s editor The panel's network type, options, and default fallback only knew allow|deny. Since allowlist is now the shipped default, every user landed on a dropdown showing no current selection and offering only Allow/Deny — picking Allow silently replaced the default bounded policy with unrestricted egress, with no way back through the GUI. Widens the type, the option list (labelled to make bounded-vs-unrestricted explicit), and every allowlist|deny fallback to match the backend/CLI contract, and adds an "Extra allowed hosts" editor (mirrors the existing writable-paths pattern) since the backend and CLI both already accept allowHosts. Verified with a real render: a Vite SSR-load + happy-dom harness mounts the panel through the app's actual context stack (PlatformProvider, ServerProvider, GlobalSDKProvider) against a real in-process HTTP server implementing the GET/PUT /settings/sandbox contract, confirming an allowlist config renders as "Allowlist" (not blank) and that opening the dropdown and picking Allow round-trips a real PATCH and re-renders the new selection.
…rontend types - ADR-0002 said macOS "has no namespace equivalent, so this enforcement argument does not transfer," implying the bounded-egress outcome itself is unreachable. Seatbelt reaches the same outcome via a different mechanism — (allow network-outbound (remote tcp "localhost:PORT")), which anthropic-experimental/sandbox-runtime ships — so it's achievable but unimplemented here, not impossible. Rewrote the paragraph accordingly. - frontend/docs/.../sandbox.mdx never mentioned allowlist: claimed network is "allowed by default", documented only allow|deny for --network and the config key, and showed a "network": "deny" example. All contradicted the shipped allowlist default; updated the quick-start prose, the flag table (added --allow-host), and the config example/field list. - frontend/workspace/src/notebook/runtime.ts labelled anything not === "deny" as "Network allowed", so an allowlist kernel read as fully open. kernelNetworkLabel/kernelNetworkTone now distinguish all three states (allowlist gets its own "Network bounded" label and a middle tone; unrestricted "allow" escalates to the danger tone). Also widened the stale KernelEnvironment.sandbox.network type to match the backend's three-state contract (science/kernel/types.ts). - frontend/workspace/src/atlas/execution-authority.ts still typed sandbox.network as allow|deny, mismatched with the backend Decision type.
Seatbelt has no network namespace to sever the way bubblewrap's --unshare-net does, so the host-side proxy listens on a loopback TCP port instead of a unix socket, and seatbeltProfile narrows the profile to exactly that port: (deny network*) always precedes a single (allow network-outbound (remote ip "localhost:PORT")), and a missing or invalid port throws rather than silently falling back to a plain deny (which would read as network:"deny", not "allowlist") or, worse, an unfiltered allow. backend()/decide()/plan()/wrapArgv() and EgressRuntime.ensure() take an injectable platform (default process.platform) since no Mac exists on this project to run sandbox-exec on — the darwin branches are only exercisable from Linux by overriding it. EgressRuntime starts an Egress.serveShim bridge (TCP loopback -> the existing unix socket) when the resolved backend is seatbelt, and egressFor returns that bridged port, stringified, instead of the socket path. No shim, launcher, or bundle is composed on darwin — the sandboxed process dials the loopback proxy directly, so none of bubblewrap's namespace-bridging machinery applies.
… the unix socket, and must require Proxy-Authorization The prior commit on this branch (macOS seatbelt support) implemented Task 7's egress path in a way that violates both design decisions the brief marked as already made: 1. It added a host-side `Egress.serveShim` bridge (TCP loopback -> the existing unix socket) running in the CLI's own process for seatbelt, instead of having `serveProxy` listen on TCP directly. The brief is explicit that this extra hop must not exist. 2. It shipped no authentication at all on that loopback TCP port. A unix socket's access control is its filesystem permissions; a loopback port has none, so every process on the machine could reach the allowlist proxy. The brief requires a `Proxy-Authorization` secret, generated per proxy start, checked before anything else about a request (even whether it's malformed) is inspected, with a 407 and no forwarding on a missing or wrong one. This replaces the bridge with a real fix: `Egress.serveProxy` is now overloaded to listen directly on either a unix socket or a `hostname`/`port`, with a `secret` required (and enforced) only on the TCP form — so each call site still gets back the concrete `UnixSocketListener`/`TCPSocketListener` its own input implies. `EgressRuntime` generates a `crypto.randomUUID()` secret once per seatbelt proxy start and returns `"<port>:<secret>"` as the darwin `egressFor()` value (bubblewrap keeps returning the unix socket path, unaffected). `Sandbox.buildPolicy` splits that back into `Policy.port`/`Policy.secret`, and `plan()`/`wrapArgv()` embed the secret as userinfo in the proxy URL (`http://os:<secret>@127.0.0.1:<port>`), which pip, curl and requests all parse into a `Proxy-Authorization` header. Test coverage: egress.test.ts gets direct unit tests against the TCP listener (binds 127.0.0.1 only; a correctly-authenticated request reaches the dial; a missing or wrong secret gets 407 and never reaches the allowlist check or the dial; the unix-socket listener is unaffected, no auth required there). egress-runtime.test.ts's darwin tests are rewritten for the new shape (no more socket/bridge fields; hostname+port+secret; the same both-directions auth assertions one layer up, through the real lifecycle). sandbox.test.ts's darwin plan()/wrapArgv() tests now use a `"port:secret"` egress value and assert the authenticated proxy URL, plus a new case for a port with no secret (must throw, not silently compose an unauthenticated URL). Linux/bubblewrap paths are unchanged: bubblewrapArgs, serveShim (the in-namespace bridge that already existed for bubblewrap), and the unix-socket half of serveProxy are untouched logic, confirmed by diff against this branch's pre-Task-7 tip and by the full live egress and bwrap-shim suites passing unchanged (test/sandbox/: 97 pass, 0 fail; full suite: 1944 pass / 1 skip / 1 pre-existing unrelated fail).
… fail closed on a cached-wrong-platform proxy, fix two tests, pin the auth seam with real clients Task 7 review, four Important findings: I3 (the one that mattered most): seatbeltProfile emitted only network-outbound, spelled (remote ip ...) — narrower than, and a different filter type than, the reference implementation docs/adr/0002-sandbox-network-policy.md:56-59 already cites (anthropic-experimental/sandbox-runtime: network-bind/network-inbound/ network-outbound, spelled tcp, all on the proxy's loopback port). An independently-guessed narrower profile that has never been measured against a real sandbox-exec is exactly how "allowlist" ships silently unreachable on every Mac. Now emits all three operations, tcp-spelled, matching the ADR's literal quote; the doc comment states plainly that whether network-bind/ network-inbound are even needed, and whether local/remote is the right filter pairing for them, are still open questions only a Mac can answer. I1: egressFor's seatbelt branch interpolated running.secret with no guard — over a proxy already cached under a different injected platform, this composed the literal string "3128:undefined" (truthy, so a bare toBeTruthy() check on the secret half couldn't catch it). Now throws, naming which listener is actually running; added a regression test that forces the ordering and a stronger UUID-shaped assertion on the happy path. I2: a test asserted the opposite of what its own comment claimed to prove, passing for the wrong reason (127.0.0.1 isn't in DEFAULT_RULES, so the request was denied at the allowlist check, never reaching the dial the test claimed to exercise). Fixed by allowlisting 127.0.0.1 before starting the proxy, so the request now genuinely clears auth and the allowlist check. I4: every auth test hand-built the Proxy-Authorization header, leaving the seam between proxyUrl()'s format and serveProxy's parser unpinned. Three new tests drive real curl (absolute-form and --proxytunnel) and Python urllib at the exact URL Sandbox.plan() composes. Also: M1 (stop logging the secret half on an invalid egress warning), M5 (reject ports above 65535), M6 (update the ADR's now-stale "seatbelt falls back to deny" line — the one docs/ line authorized for this round), M7 (the report's Mac-owner verification commands referenced an unassigned shell variable and instructed running them after the proxy that backed them had already been stopped; folded into one runnable script). M2/M3/M4/M8 left as instructed, with a note on each in the report. Hit the same Bun.spawnSync-blocks-the-proxy's-own-event-loop deadlock this codebase already has a comment about (sandbox.test.ts's "Bun.spawn, not spawnSync") while writing the I4 tests; fixed by switching to async Bun.spawn before it shipped. test/sandbox/: 97 -> 104 pass, 0 fail. Full suite: 1944 -> 1951 pass / 1 skip / 1 fail, the fail pre-existing and unrelated.
… (fix round 2) Fix round 2's only code change: writes the one item from that round meant to be recorded in a doc comment rather than only in the Task 7 report — if the host proxy dies while a seatbelt-sandboxed child is still alive, network-bind/network-inbound on that same ephemeral port would let the child itself bind or listen there. Confined to the one port the profile names, not a broader grant; a real, specific consequence of matching the ADR's reference shape, not a hypothetical one, so it belongs next to the other open Mac-only questions already in this comment. Round 2's other finding (M7 — the report's own verification script failed on line 1 because Bun resolves a relative import against the importing file's location, not cwd, and the report told a Mac owner to save the script somewhere that broke that) is a report-only fix: corrected save location, then run verbatim from a clean shell on this machine as far as it allows. Both recorded in the report under "Fix round 2 of 5" (.superpowers/, gitignored, not part of this commit). test/sandbox/: 104 pass, 0 fail, unchanged by this round. Full suite: 1951 pass / 1 skip / 1 fail, the fail pre-existing and unrelated.
Windows has no sandbox backend today, so kernels are denied there outright. Every network-filtering option for it needs administrator rights, and asking for elevation to create a local account and load kernel network filters — in order to run AI-authored code — is indistinguishable at the UAC prompt from malware. This inverts the model instead. An AppContainer without a network capability has no network at all, kernel-enforced and unprivileged; a named pipe ACL'd to its package SID gives it one channel to a broker that performs approved requests on its behalf. No firewall mutation, no elevation. The cost is that Windows becomes capability-mediated rather than socket-transparent: no network capability means no loopback either, so the shim that lets unmodified pip and requests work on Linux cannot exist. A notebook cell cannot fetch a scientific API directly. That is recorded as a decision rather than discovered later. Nothing here has been executed — there is no Windows machine on this project. Four things a Windows owner must confirm first are listed, and one of them would change the design if it came back the other way.
Task 7 built seatbelt support (profile text, authenticated loopback proxy) entirely from Linux, with the platform injected on every assertion, since no Mac exists on this project. Add the live test and CI job that let a real sandbox-exec finally run it: - test/sandbox/egress-live-seatbelt.test.ts: real sandbox-exec, real TCP-loopback Egress.serveProxy, real remote hosts, wired the way Sandbox.plan composes them in production — no platform override, so Sandbox.backend()/decide() resolve for real. Asserts an allowlisted host reaches 200 through the proxy, a denied host does not, direct egress with the proxy env unset fails, DNS resolves nothing inside the sandbox, and an 18MB wheel survives byte-for-byte. Gated on Sandbox.backend() === "seatbelt": skips on Linux, must fail rather than skip on the one machine that can run it. - .github/workflows/ci.yml: new `sandbox` job, matrixed over ubuntu-latest/macos-latest (no windows-latest — Sandbox.backend() is "none" there), mirroring `migration`'s shape and reusing `test`'s bubblewrap install/apparmor workaround for the Linux leg.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The macOS CI leg found five failures, four of them in tests and one real. Real: `shimScript`'s readiness cap was an iteration count (150 polls at 0.02s), which only equals the documented 3s where forking `sleep` is nearly free. A macOS runner measured 17.1s for the same loop — ~114ms per iteration, ~94ms of it fork/exec — a 5.7x overshoot. Any host with expensive process creation drifts the same way. The loop now carries a `date +%s` deadline alongside the count, probed exactly like fractional `sleep` so a build without `%s` degrades to today's count-only behaviour rather than skipping the wait. Measured with a fork-dominated `sleep`: 18.4s before, 3.3s after. Tests: four assumed the ambient platform was bubblewrap, an assumption written before the darwin branch existed and false on a macOS runner. Three called `EgressRuntime.ensure()` and got a TCP listener where they wanted a unix socket; the fourth composed bwrap argv through `Sandbox.plan()` and hit `seatbeltProfile` instead. All four now inject the platform through the seam that already exists for it. The over-broad egress cases were passing on darwin for the wrong reason — they threw "requires an egress port", not "requires an egress socket path" — so they now assert the message too. Also: prettier on docs/specs/windows-sandbox-design.md.
main moved four commits ahead while this branch was in review, and #277 ("Match Claude Science compute, artifacts, and UI") rewrote the kernel, notebook and pty files this branch also touches. One real conflict, in `pty/index.ts`: main gave `terminalEnv` a fourth `command` argument (it picks the PS1/PROMPT shape from the shell) and computed the env before `Sandbox.wrapArgv`. This branch computes it after, because `sandbox.env` carries the proxy variables the egress shim needs. Kept this branch's ordering with main's new argument — reversing the order would drop the proxy env on every terminal. Everything else auto-merged. Verified rather than assumed: typecheck clean, 1953 backend tests pass, and the suites over the merged files specifically (sandbox, server/notebook, pty-environment, frontend notebook runtime) are green at 134 + 31. One pre-existing local failure, untouched by this branch and passing in CI: test/installation/native-package-matrix.test.ts indexes `npm pack --json` output as an array, but npm 12 returns an object keyed by package name. It will break CI too once the runner's npm bumps.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL flagged this as js/incomplete-url-substring-sanitization, high severity. It is a false positive — `host` is the mounted DOM node and `textContent` is rendered text, not a URL, and nothing here sanitizes anything. The production matcher (`Egress.allowed`) splits the port off and compares the host exactly or by suffix; it does no substring test. The line was still wrong for a second reason worth fixing rather than dismissing. It polled on the whole subtree's textContent containing the host and then asserted on a <code> element's exact text — a looser wait than the assertion, so the wait can finish while the assertion still fails, burning all 50 iterations first. Both now use the same predicate.
The merge gate is that pip install works with the allowlist policy on every platform we ship. Until now both live files stopped at curl, which proves the boundary holds but not that it is usable: pip issues its own index request, follows pypi.org -> files.pythonhosted.org (a second allowlist entry, so this also covers a cross-host hop), streams a wheel and unpacks it. None of that was exercised. Both files run the same package with the same flags, deliberately, for the same reason they already share the wheel URL and hash — a divergence between backends should surface as one green and one red, not as two different scenarios that both happen to pass. `--only-binary :all:` keeps it a network test rather than a toolchain test. The venv needs no network of its own: `python3 -m venv` bootstraps pip from the interpreter's bundled ensurepip wheel, which is what makes this work on a machine with no pip on PATH — one of the three original blockers. The seatbelt test additionally covers pip authenticating to the proxy. Seatbelt's loopback port is reachable by every process on the machine, so the proxy URL carries a per-start secret and 407s anything without it; curl and urllib were already covered, urllib3-on-CONNECT (what pip actually uses) was not. Linux: 3 pass, the pip test at 2.29s.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Gives sandboxed kernels and the agent's shell bounded network access — PyPI and a fixed set of
scientific APIs, nothing else — enforced rather than advisory.
sandbox.networkbecomes three-state:deny | allowlist | allow, defaulting toallowlist. In thatmode the sandbox keeps
--unshare-net, which blocks every host including the host's own loopback, anda unix socket bind-mounted in is the only route out. A shim inside the namespace bridges loopback TCP
to it; a proxy on the host resolves names, checks the authority against an allowlist, and pipes bytes.
Before this,
sandbox.networkwas binary. Deny locked kernels out of PyPI, NCBI, UniProt, PDB and EBI —most of what a research tool is for. Allow was unrestricted egress.
Design recorded in
docs/adr/0002-sandbox-network-policy.md.What a user gets
pip installinto a writable venv403from the proxyAlso here: a macOS seatbelt backend, an
allowHostsconfig surface reachable from the CLI, the settingsroute and the GUI, and
docs/specs/windows-sandbox-design.mdrecording a Windows design that is notimplemented.
How did you verify your code works?
By executing it, not by reading it. A green suite proved nothing repeatedly on this branch — nine
defects were found across fourteen review rounds, every one invisible to passing tests because they
asserted on argv strings or pushed five bytes through a stub. Notable ones:
Socket.write's return value with nodrainhandler, soevery transfer above a few KB lost bytes. A 40 MB payload arrived as 2.6 MB;
pip download numpydied with an SSL error. Now fixed and pinned by a live test that pulls an 18 MB wheel through a real
sandbox and checks its sha256.
and
RangeError: Out of memoryin four seconds. Bounded, with real backpressure.unreadableentries, which carrykernelSensitivePaths().test/sandbox/egress-live.test.tsasserts the load-bearing negatives, not just the happy path: adenied host refused, direct egress with the proxy unset failing, and
getentresolving nothing —without those it would prove the proxy works, not that it is the only route out.
bun run typecheckpassesbun test(inbackend/cli) passes — 1951 pass / 3 skip / 1 fail; the failure is a pre-existingunrelated
npm packissue intest/installation/native-package-matrix.test.ts, reproducedidentically on
mainbunx prettier --check .is cleanallowHostseditor; worth a look before mergemacOS: verified against a real
sandbox-execNobody on this project has a Mac, so the seatbelt backend shipped unexecuted and this PR added the CI
job that would be its first run. That run happened
(31466118464) and
Sandbox (macos-latest)is green — 95 pass, 0 fail, including both live tests intest/sandbox/egress-live-seatbelt.test.ts: a real seatbelt sandbox reaching an allowlisted host at200, a denied host not, no direct connection, no DNS, and megabytes arriving byte-for-byte.
The three ways it could have been silently non-functional are all answered in the affirmative for the
profile as written:
network-bind/network-inboundas emitted are sufficient —(deny network*)does not block theimplicit local bind a TCP
connect()performs.tcpspelling of the filter works.(deny network*)viamDNSResponder— the live test assertsresolution fails inside the sandbox, and it does.
The macOS tests are written to fail rather than skip when seatbelt is present, so this is a real
result, not an empty run.
The first red macOS run also found a genuine Linux bug.
shimScript's readiness cap was aniteration count (150 polls at 0.02s), which equals the documented 3s only where forking
sleepisnearly free. The macOS runner measured 17.1s — ~114ms per iteration, ~94ms of it fork/exec, a 5.7x
overshoot that any CPU-throttled or loaded Linux host would hit the same way. The loop now carries a
date +%sdeadline alongside the count, probed like fractionalsleepso a build without%sdegrades to the previous count-only behaviour rather than skipping the wait. Measured against a
fork-dominated
sleep: 18.4s before, 3.3s after, with a regression test that reproduces the conditionon any host.
Known follow-ups, not blocking
--ro-bind / /exposes every host unix socket,so a sandboxed process reaches
docker.sockand can obtain host root — in every network modeincluding
deny, therefore onmaintoday. This PR bounds network egress; local IPC to host daemonsis a separate and wider hole.
allowlist—sshignoresHTTP_PROXYand needsProxyCommand/SOCKS. This is the one regression a user could notice.egress-shim-dev-*artifacts accumulate inGlobal.Path.bin, never collected.ones. Separately, a benign
KernelExecutionErrortrace appears in some runs from a test thatdeliberately crashes a kernel, without the tally changing.
/is read-only, so pip reports'~/.cache/pip' … is not writableand re-downloads every wheel on every install. Correct sandboxbehaviour, real repeat cost for an agent that installs across a session. Whether to bind a writable
per-project cache is a design decision for the install phase, not a bug here.
Installation.isLocal()is false andshimPlanreturns{binary: process.execPath}— the binary is its own shim, no dev bundle. Everytest runs under
bun run, which takes the other branch. Verified once by hand against abun run build --singlebinary (a realpip install tqdmsucceeded inside--unshare-net, with adenied host 403'd, no direct route and no DNS), but nothing keeps it that way.
OPENSCIENCE_CHANNELis a build-time define, so covering it in CI means building a binary in the job.
test/global/data-dir.test.tsintermittently fails withEBUSY: resource busy or lockedcascading into ENOENT — an open-handle race, unrelated to thisbranch. Observed once here, green on re-run of the same commit.
npm pack --jsonshape break, pre-existing.test/installation/native-package-matrix.test.tsindexes the output as an array; npm 12 returns an object keyed by package name. Fails locally on
npm 12, passes on CI's older npm — it will go red when the runner bumps.
Merge gate
Not to be merged until
pip installworks with user approval, undernetwork: "allowlist", on allthree platforms.
allowlistsandbox-exec, authenticated proxySandbox.backend()is"none"; kernels do not run at allThe approval flow (an install tool, a permission gate, an environment store) does not exist on any
platform yet. Windows needs the AppContainer + broker design in
docs/specs/windows-sandbox-design.mdbuilt, and its open question 4 — whether anything inside an AppContainer can listen on loopback —
decides whether pip can run inside the sandbox there at all or must run in the broker's trust domain.