Governed package installation for kernels - #281
Draft
KB (KB-syntheticsciences) wants to merge 49 commits into
Draft
Governed package installation for kernels#281KB (KB-syntheticsciences) wants to merge 49 commits into
KB (KB-syntheticsciences) wants to merge 49 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 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.
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.
The spec and the (unwired) capability contract were written on
proto/kernel-package-install before the allowlist proxy existed. Both make
claims the proxy has since falsified, so they come across corrected rather
than verbatim.
The material one: "Redirect on failure" assumed a shell `pip install` fails
anyway, leaving the redirect's only job to replace a confusing error with a
helpful one. Under network "allowlist" it succeeds — measured in the agent's
own sandbox, no tool and no approval card involved:
python3 -m venv <workspace>/venv && <workspace>/venv/bin/pip install tqdm → 4.70.0
The workspace is writable and pypi is allowlisted, so read-only system
site-packages stops nothing; the agent builds its own venv beside the
project. The proxy did not create the intent to bypass, it removed the
accident that used to prevent it. So the shell path must be refused in the
bash tool before execution, not merely redirected after failure — otherwise
an agent that never calls the tool never shows a card, and the approval
gate is decorative. Recorded with its limits: refusal is a contract
boundary, not a security one, since the same egress can fetch a wheel by
hand.
`prompt.ts` said the same thing to the agent in its own words ("the agent
shell has no network"), which is now simply false. Rewritten to promise
refusal, and to name the venv-in-workspace route the old wording implied
was impossible.
Also refreshed "Not verified", which had gone stale in three entries: the
macOS proxy is now designed and verified against a real sandbox-exec, the
proxy's backpressure/bounds/timeouts are built and measured (concurrency
caps, audit log and hostile-upstream behaviour are still open), and the
release-mode shim is added as newly-known-untested.
Closes the hole the allowlist proxy opened. Before it, a shell `pip install` died at DNS, so the install contract held by accident. Measured on feat/sandbox-network-policy, this now succeeds with no tool and no approval card: python3 -m venv <workspace>/venv && <workspace>/venv/bin/pip install tqdm The workspace is writable and pypi is allowlisted, so read-only system site-packages stops nothing. Until this lands, every part of the install work that follows is decorative: an agent that never calls package_install never shows a card. Matching is over the tokenised argv bash.ts already builds from tree-sitter, not the raw line — `echo "pip install numpy"` is one command named `echo`, and a regex cannot tell the difference. Covers the routes a bare `pip install` match misses: `<path>/bin/pip`, `python -m pip`, `uv pip`, conda/mamba/poetry. Read-only subcommands (`list`, `show`, `--version`) stay allowed; refusing them would break ordinary inspection. The throw sits before any ctx.ask, so the user is never asked to approve a command that is then refused anyway. This is a contract boundary, not a security boundary — the same egress can fetch a wheel by hand — and the code says so rather than claiming more. Verified the wiring is load-bearing by removing it: two of the three tool-boundary tests fail (the permission ask fires and the refusal message never appears). 30 tests pass with it, typecheck clean, no regression in the 133-test tool suite.
PackagePrompt was written during prototyping and never wired, so nothing told the agent that package_install exists or that shell installers are refused. Wired the way ComputePrompt already is — unconditionally, into the system array on every request for every agent. That unconditional injection is the load-bearing mechanism, not a skill override. 199 of the 293 shipped SKILL.md files mention `pip install`, and editing them would be neither necessary nor sufficient: the skill tool never intercepts reference files, and skills cloned from GitHub are not this repo's to edit. One block on every request reaches all of them. Sequenced after the bash refusal deliberately — the contract now promises something that is true. 29 package tests pass, 236 session tests unaffected, typecheck clean.
The manifest is the source of truth and the directory is derived, therefore a cache — which is why they live in different roots. Global.Path.cache can be cleared by the user or a cleaner at any time, and an environment has to be rebuildable from its manifest afterwards; putting the manifest inside the directory would turn a cache clean into unrecoverable data loss. additive() is what decides whether kernels restart. Adding a package leaves a live kernel correct because a new module imports on first use; an upgrade, downgrade or removal does not, because a module already loaded stays at its old version in memory while the files on disk disagree. That failure is silent, which is why it is worse than an obvious restart — and why this is a comparison rather than "restart on every install". The lock chains on the previous entry with `previous.then(fn, fn)` so a failed install does not cancel work queued behind it, and clears its slot in a `finally` only when the slot is still its own. A lock surviving a throw would brick that environment for the process lifetime — the same latching bug the egress runtime shipped with, and the reason both cases are pinned by tests. 16 tests, typecheck clean.
…edaction A deliberate subset — name, extras, specifiers, markers, and the `name @ url` form — with markers captured but never evaluated, because nothing needs them. The spec's "real parser" requirement means specifically: do not split on `==`, which mishandles `numpy>=2.4`, `pandas[performance]` and `tqdm; python_version >= "3.9"`. Anything outside the grammar throws rather than being guessed at, because a mis-parsed name becomes a wrong permission pattern and a wrong pattern approves something other than what runs. The tests caught a real bug in the first implementation. Validating the specifier with a prefix test, `/^(===|==|…|>|<)\s*\S/`, accepts `numpy >= `: the alternation backtracks to the single-character `>` and consumes the `=` as the version, so a dangling operator passes validation and reaches pip as a literal requirement. Replaced with a clause regex anchored end to end and applied per comma-separated clause, with the seven dangling-operator forms pinned as regressions. Names normalise per PEP 503 so Foo_Bar, Foo.Bar and foo-bar are one package — treating them as three would let an upgrade look additive to Environment.additive, which is what decides whether live kernels restart. The pattern is names-only and sorted: sorted so the same set in a different argument order matches an existing grant, names-only because resolution happens after approval, so pinning a version must not fragment a grant the user already gave. Index credentials are stripped before matching, so rotating a token does not invalidate a standing grant and no secret reaches a card. 39 tests, typecheck clean.
`npm pack --json` returns an array of entries on npm 11 and earlier, and an object keyed by package name on npm 12. The test indexed [0], so it failed with "npm pack did not return a tarball" on any machine running a current npm while still passing on CI's older one — a green build hiding a broken test, until the runner bumps and it goes red for everyone at once. Accepts either shape rather than pinning a version: this test is about npm's package selection across the native target matrix, not about its output format.
The install runs in the same sandbox as the kernel, not a second more permissive one — the allowlist proxy removed the asymmetry that justified a separate install sandbox, so the only difference is that the environment directory is writable. Adding a real install test here rather than deferring it to the end-to-end task immediately found a defect the rest of the suite could not see. The ladder prefers uv when present, and `uv venv` does NOT bootstrap pip the way `python3 -m venv` does, while install() shells out to `python -m pip` regardless of who created the environment. So on any machine with uv the ladder produced an environment the installer could not use: "No module named pip", from a venv that looked perfectly healthy from outside the sandbox. Every other test passed, because the one asserting a venv has pip forces the venv branch explicitly. Fixed with `uv venv --seed`. Seeding rather than adding a second `uv pip install` path keeps one install code path to test and leaves the environment usable by hand; the cost is a few hundred ms at creation only. Pinned by a uv-branch test and an invariant test, so a future third branch has to satisfy it too. explain() translates the two failure surfaces that matter — the wheels-only rejection that reads as "no such package", and a build failure whose summary names the package while the `fatal error:` line above names the actually missing system header. An unrecognised log passes through untouched; inventing a diagnosis for an unanticipated failure is worse than showing it. 18 tests including a real tqdm install through bwrap with allowlist egress, verified by reading the version back out of the environment rather than trusting pip's exit code. Typecheck clean.
The ordinary permission contract, not modal's: installing a library must not be gated more strictly than running arbitrary code, because it costs nothing. So no digest, no spendFilter entry, and a readable canonical command string as the pattern — `install tqdm → t1 [pypi.org/simple]` — which changes whenever the approved action changes, so the prompt reappears for free when it should. `always: ["install*"]` offers the standing grant from the first card, mirroring notebook.ts. Resolution happens after approval. Approving two names must not silently approve the closure they pull in, so the card shows the request and freeze() runs afterwards. A fully-satisfied request skips outright: no card, no install, no restart, because nothing privileged happens. Running the tool for real, rather than only reading its source, found a silent data-loss bug. `execute` is reachable without zod having applied parameter defaults; `language` was then undefined, JSON.stringify dropped the key, and the manifest it wrote could never be parsed back. The environment existed on disk with packages installed in it and was invisible to the inventory — indistinguishable from "never created" at every call site, with no error anywhere. Fixed at the class rather than the instance: Environment.write now validates before writing and throws naming the offending field, so no caller can produce an unreadable manifest. The tool also defaults language explicitly. TypeScript already enforces resolved params at the tool boundary — that is belt and braces for other callers of write(). Both return branches now share one metadata shape; two shapes would force every consumer to handle a union whose arms differ only in which keys exist. restartEnvironment lands as a no-op with its final signature, so the tool has no forward reference; the kernel-binding task gives it a body. Named `boundEnvironment` rather than `environment` deliberately — the registry entry already has an `environment` field of type KernelEnvironment, which is the kernel's runtime context and nothing to do with packages. 114 package tests including three that run the real tool through a real sandbox; 340 tool+science tests unaffected; typecheck clean.
…change Binding is a property of the registry entry, never part of KernelIdentity — that tuple is hashed into the storage key, so adding to it would orphan every persisted record. It is named `boundEnvironment` because the entry already has an `environment` field of type KernelEnvironment, which is the kernel's runtime context (cwd, sandbox platform) and nothing to do with packages; merging them would bind kernels to the wrong thing. Staleness is compared at the registry level because ExecutionAuthority.require takes no kernel identity and so cannot see which environment a kernel is bound to. Writing a test that runs a real kernel, rather than asserting on plumbing, found three defects: 1. The kernel could not see the environment at all — "No such file or directory" for an interpreter that exists. Production would have worked by luck, since ~/.cache is covered by `--ro-bind / /`, but `--tmpfs /tmp` masks it wherever the cache root lives under /tmp. Fixed with an explicit `readable` passthrough on wrapArgv. Deliberately read-only, not writable: a writable environment would let arbitrary kernel code run `subprocess.run([sys.executable, "-m", "pip", "install", ...])` over the same allowlisted egress, reopening through the notebook tool the exact bypass the bash-tool refusal closes. 2. wrapArgv replaced policy.readBind wholesale with the egress shim's paths at two call sites, silently discarding anything buildPolicy had put there. Merged instead. The env bind was being dropped on the floor with no error. 3. `six==1.17.0` against an installed 1.16.0 was reported "already installed". The skip check compared package NAMES only, so an upgrade became a silent no-op that also reported the change as additive — leaving the environment on the old version, telling the agent it had the new one, and leaving bound kernels un-restarted. Any requirement carrying a specifier, URL or extras now goes through pip, which already implements PEP 440 comparison and no-ops when the request is genuinely satisfied. R binds by library path rather than interpreter, since it has no per-environment binary: R_LIBS_USER, already in the kernel env allowlist, also read-only so cell code cannot call install.packages() around the card. restartEnvironment releases rather than respawns — kernels are lazy, so the next cell boots a fresh one, and eager respawning would pay startup cost for kernels the session may never touch again. 126 package tests, 446 sandbox+tool+science, typecheck clean. The behavioural pair proves what plumbing tests cannot: a package installed into one environment imports there and is genuinely absent in another real one, and an additive install keeps kernel state while a version change discards it.
PackagePrompt.system() read a single global environments.json that nothing
has ever written, so the agent was told "No environments exist yet" forever
— including immediately after installing something. That makes the
contract's own first rule ("whether a package is available is a read-only
question, answer it from the inventory above") actively misleading: the
agent either installs what it already has or runs code to find out, both of
which the contract forbids.
Now assembled from the real per-project manifests. `busy` comes from the
live in-memory lock rather than a stored flag: a persisted flag would
survive a crash and permanently mark a healthy environment as installing,
with nothing to clear it.
The store import is aliased to `Store` because this namespace exports its
own `Environment` — the render schema — which shadows it inside every
function body here. Caught by typecheck, but it would have been a confusing
runtime failure.
Pinned by a test that installs for real and then reads the block back: it
says "No environments exist yet" before and lists "seen (python): tqdm"
after. 10 prompt tests, 236 session tests unaffected, typecheck clean.
`wait: true` stays the default and covers the common case — a two-second install should cost one turn, not two. `wait: false` still takes the lock, so a second install queues exactly as it would otherwise; what changes is that the turn does not hold open. Its output deliberately carries no versions and does not claim success, because there are none yet and inventing one is what the contract forbids. Reconcile is the half that matters. An install that cannot be proven still running is `unknown`, never `fine`: pip has no transactions, so an interrupted one may have left a partial tree, and silently trusting it turns into a mystery ImportError several turns later. Writing this against the real `science/kernel/process.ts` rather than the plan's assumption corrected three things the plan had wrong. `token` was module-private (now exported as `startToken`), synchronous rather than async, and — the one that mattered — **undefined on Windows**, which has neither the /proc nor the `ps -o lstart=` branch. The plan's rule, `running = Boolean(t) && t === stored`, would therefore have marked every Windows install unknown forever and every environment permanently suspect. `matches()` already solves this with `if (!identity.token) return true`; reconcile now shares that rule through `KernelProcessIdentity.running(pid, token?)`, with the untokened case pinned by its own test. Claims live under Global.Path.state, not data: per-machine liveness should not survive a restore onto another machine. 139 package tests, 340 tool+science unaffected, typecheck clean.
The simpler backend, as the spec predicted: no ladder to probe and no pip to bootstrap, because install.packages is part of base R and the binding is a library path rather than a per-environment interpreter. cran.r-project.org is already in Egress.DEFAULT_RULES, so the allowlist is unchanged. Only the backend differs by language. The card, the lock, the manifest write and the additivity check are shared, because they are properties of the contract rather than of pip or CRAN. Two things R needs that Python does not: - `install.packages()` signals failure with a warning and still exits 0, so the script checks installed.packages() afterwards and quits non-zero on anything missing. Without it a package that does not exist reads as a successful install. - `lib` is passed explicitly as well as through R_LIBS_USER. Left to .libPaths() ordering, install.packages picks the first writable entry, which on a machine with a user library already configured is the wrong directory — and would leak the environment's packages into every other project. Requirement.parse is deliberately NOT applied to R names. PEP 503 normalisation would turn data.table into data-table, which never matches what CRAN installed; R names are case-sensitive and `.` is meaningful. Verified where it can be: 146 package tests pass. The two live R tests SKIP on this machine because Rscript is not installed, so the CRAN round trip itself is unverified here — the same honest gap the macOS seatbelt work had before CI could run it.
The condition for merging this work is that a package installs WITH THE USER'S APPROVAL, under network "allowlist", on every platform we ship. Everything else in test/package/ tests a component; this tests the claim. Both halves are asserted together deliberately. A green install with an open shell bypass is not the gate met — an agent that never calls the tool never shows a card — so the refusal of `<venv>/bin/pip install`, `pip install`, `uv pip install` and `python3 -m pip install` sits in the same file as the approval assertion, not in an adjacent one. The install assertion reads the version back out of the environment rather than trusting pip's exit code, and checks that the policy in force really was "allowlist" rather than a disabled sandbox — otherwise the test could pass with no boundary at all. The CI sandbox job now runs test/package/ alongside test/sandbox/ on both legs, so the gate is a fact on Linux and macOS rather than a claim about one of them. Timeout raised 20 -> 30 minutes: these are real installs against real pypi, which the sandbox suite alone never did. Locally: 255 pass / 5 skip / 0 fail for exactly what CI runs, and 2111 pass / 0 fail for the whole backend suite.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // environment's packages into every other project on the machine. | ||
| expect(source.includes("R_LIBS_USER")).toBe(true) | ||
| expect(source.includes("install.packages")).toBe(true) | ||
| expect(source.includes("cran.r-project.org")).toBe(true) |
test/package/ creates real projects with tmpdir({ git: true }), and
`git commit` exits 128 on a runner with no global user.name/user.email. The
`test` job has configured one for exactly this reason since before this
branch; the sandbox job never needed it until test/package/ joined it.
Ten failures on the first run of the new job, all of them this and none of
them the installs they looked like.
…ming it The dead-pid reconcile test spawned /bin/true, awaited exit, and claimed the pid — but `await proc.exited` is not the same as "the pid is gone". A just-reaped child can stay signalable briefly, and on the macOS runner it did: process.kill(pid, 0) still succeeded, so the claim reconciled as "running" and the test failed for a reason that had nothing to do with reconcile. Now polls until the premise actually holds and asserts it, so a future failure here means reconcile is wrong rather than that the fixture raced.
/bin/true does not exist on macOS — it is /usr/bin/true — so posix_spawn returned ENOENT and the dead-pid reconcile test failed there for a reason unrelated to reconcile. My first reading of that red run blamed a zombie pid still being signalable; the polling added for it is reasonable hardening and stays, but it was not the cause. Uses process.execPath, which is present by definition.
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 the agent a governed way to install packages — and closes the only ungoverned way.
A new
package_installtool installs into named, language-scoped environments behind an approvalcard. Kernels bind to an environment through an
environmentparameter besidekernel, and aninstall restarts them only when the change is not purely additive. Shell installers are refused
before they run.
Stacked on #280 (bounded network egress); merge that first.
What a user gets
package_install, one approval cardinstall tqdm → default [pypi.org/simple], with a standinginstall*grantpip installin the agent shellinstall.packagesintoR_LIBS_USERWhy the refusal is part of this PR, not a follow-up
#280 made shell installs work. Measured on that branch, inside the agent's own sandbox, with no
tool and no card:
The workspace is writable and pypi is allowlisted, so read-only system site-packages stops nothing.
Before the proxy this died at DNS and the install contract held by accident; the proxy removed the
accident, not the intent. An approval card that an agent can walk around is decorative, so the
refusal and the tool ship together and are asserted in the same test.
It is a contract boundary, not a security boundary — the same egress can fetch a wheel by hand —
and the code says so rather than claiming more.
How did you verify your code works?
By running it. Six defects were found by tests the plan did not call for, every one invisible to the
suite as planned:
uv venvdoes not install pip. The ladder prefers uv, butinstall()shells out topython -m pip, so on any machine with uv it produced an environment the installer could not use.Every other test passed, because the one asserting "a venv has pip" forces the venv branch. Fixed
with
uv venv --seed.executeis reachable without zod defaults applied,so
languagewas undefined,JSON.stringifydropped the key, andread's parse rejected thefile. The environment existed on disk with packages in it and was invisible to the inventory.
Environment.writenow validates before writing.exists. Production would have worked by luck (
~/.cacheis under--ro-bind / /);--tmpfs /tmpmasks it wherever the cache root lives under /tmp.
wrapArgvdiscardedpolicy.readBindat two call sites, replacing it wholesale with theegress shim's paths. The environment bind was dropped on the floor with no error.
six==1.17.0over an installed 1.16.0 reported "already installed". The skip check comparednames only, so an upgrade became a silent no-op that also reported the change as additive —
leaving the old version in place while telling the agent it had the new one.
token()is undefined on Windows. The plan's reconcile rule would have marked every Windowsinstall
unknownforever and every environment permanently suspect.Also fixed
npm pack --json, which returns an object on npm 12 and an array before it — the testindexed
[0], so it failed on any current npm while passing on CI's older one.bun run typecheckpassesbun test(inbackend/cli) — 2111 pass / 6 skip / 0 failbunx prettier --check .cleanMerge gate
test/package/install-live.test.tsstates the condition as an assertion, and the CI sandbox job runstest/package/on both legs so it is a fact on each platform rather than a claim about one:allowlistSandbox.backend()is"none"Windows still blocks the gate, exactly as before this PR: kernels do not run there at all, so the
suite skips rather than fails. It needs the AppContainer + broker design in
docs/specs/windows-sandbox-design.md.Known gaps, stated rather than hidden
Rscriptis not installed on the development machine, sothe two live R tests skip. Everything else about the R path — the explicit
lib, thewarning-not-exit-code failure check, the message translation — is tested.
for a read-only
/, real repeat cost across a session. Whether to bind a writable per-projectcache is a deliberate open decision.
wait: falsepath has no notification channel; a later call is how the agent learns theoutcome, and the tool's output says so rather than implying otherwise.