diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 044d02f4..05a6a1ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,6 +54,43 @@ jobs: shell: bash working-directory: backend/cli + # Task 7 gave macOS a seatbelt profile for network:"allowlist" — an SBPL + # profile plus an authenticated loopback proxy, built and unit-tested + # entirely from Linux with the platform injected, because no Mac exists on + # this project. `sandbox-exec` (macOS) and `bwrap --unshare-net` (Linux) + # are unrelated OS-level mechanisms underneath the same `Sandbox` API, so a + # green Linux run says nothing about whether seatbelt actually confines a + # real process the way the profile text claims — only this leg's macOS run + # does. See test/sandbox/egress-live-seatbelt.test.ts's doc comment for + # exactly what a red run here would mean. + sandbox: + name: Sandbox (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + timeout-minutes: 20 + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest] + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: ./.github/actions/setup-bun + - name: Install and verify Linux sandbox + if: matrix.os == 'ubuntu-latest' + run: | + sudo apt-get update + sudo apt-get install --yes bubblewrap + # Ubuntu 24.04's host-wide AppArmor policy blocks unprivileged user + # namespaces on the hosted runner before bubblewrap can apply our + # stricter per-process profile. This runner is disposable; enable + # user namespaces for the job, then prove the sandbox can start. + if [[ -e /proc/sys/kernel/apparmor_restrict_unprivileged_userns ]]; then + echo 0 | sudo tee /proc/sys/kernel/apparmor_restrict_unprivileged_userns + fi + bwrap --ro-bind / / --dev /dev --proc /proc --unshare-pid --die-with-parent -- true + - run: bun test test/sandbox/ + shell: bash + working-directory: backend/cli + test: name: Test runs-on: ubuntu-latest diff --git a/backend/cli/src/cli/cmd/sandbox.ts b/backend/cli/src/cli/cmd/sandbox.ts index 3e345e4d..4c0cae97 100644 --- a/backend/cli/src/cli/cmd/sandbox.ts +++ b/backend/cli/src/cli/cmd/sandbox.ts @@ -30,9 +30,10 @@ function printStatus(config?: Config.Sandbox) { }`, ) if (enabled) { - UI.println(` network ${config?.network ?? "deny"}`) + UI.println(` network ${config?.network ?? "allowlist"}`) UI.println(` on missing backend ${config?.onUnavailable ?? "error"}`) if (config?.allowWrite?.length) UI.println(` extra writable ${config.allowWrite.join(", ")}`) + if (config?.allowHosts?.length) UI.println(` extra hosts ${config.allowHosts.join(", ")}`) } if (enabled && !d.available) { UI.println("") @@ -67,14 +68,19 @@ const EnableCommand = cmd({ builder: (yargs: Argv) => yargs .option("network", { - choices: ["allow", "deny"] as const, - describe: "allow or deny network egress from sandboxed commands (default: deny)", + choices: ["deny", "allowlist", "allow"] as const, + describe: "network egress from sandboxed commands: deny, allowlist (default), or allow", }) .option("allow", { type: "string", array: true, describe: "extra absolute path the sandbox may write to (repeatable)", }) + .option("allow-host", { + type: "string", + array: true, + describe: "extra host the sandbox may reach when network is 'allowlist' (repeatable)", + }) .option("on-unavailable", { choices: ["warn", "error", "allow"] as const, describe: "what to do when no backend exists on a machine (default: error)", @@ -84,10 +90,12 @@ const EnableCommand = cmd({ directory: process.cwd(), async fn() { const patch: Partial = { enabled: true } - if (args.network) patch.network = args.network as "allow" | "deny" + if (args.network) patch.network = args.network if (args["on-unavailable"]) patch.onUnavailable = args["on-unavailable"] as "warn" | "error" | "allow" const allow = args.allow as string[] | undefined if (allow?.length) patch.allowWrite = allow + const allowHosts = args["allow-host"] as string[] | undefined + if (allowHosts?.length) patch.allowHosts = allowHosts await Config.setSandbox(patch) UI.empty() UI.println(`${S.TEXT_SUCCESS_BOLD}Sandbox enabled${S.TEXT_NORMAL} ${S.TEXT_DIM}(global config)${S.TEXT_NORMAL}`) diff --git a/backend/cli/src/compute/jobs.ts b/backend/cli/src/compute/jobs.ts index 8c195747..2be52c44 100644 --- a/backend/cli/src/compute/jobs.ts +++ b/backend/cli/src/compute/jobs.ts @@ -9,6 +9,7 @@ import { OpenScience } from "../openscience" import { Shell } from "../shell/shell" import { Instance } from "../project/instance" import { Sandbox } from "../sandbox/sandbox" +import { EgressRuntime } from "../sandbox/egress-runtime" import { Filesystem } from "../util/filesystem" import { ProvenanceEnvelope } from "../science/provenance/envelope" import { ExecutionAuthority } from "../project/execution" @@ -169,6 +170,10 @@ export namespace ComputeJobs { recovery_attempts: z.number().int().nonnegative().optional(), recovery_retry_at: z.string().optional(), session_id: z.string().startsWith("ses_").optional(), + // Persists the whole Decision, including its own sandbox.network — a + // second copy of the persisted enum below `sandbox.network` carries; see + // the comment on ExecutionAuthority.Decision for the downgrade cost of + // widening either one. authority: ExecutionAuthority.Decision.optional(), scope: z .object({ @@ -181,7 +186,10 @@ export namespace ComputeJobs { requested: z.boolean(), enforced: z.boolean(), backend: z.enum(["seatbelt", "bubblewrap", "none"]), - network: z.enum(["allow", "deny"]), + // Persisted — widening this costs an older binary its ability to + // read a newer record. `authority.sandbox.network` above is the same + // enum persisted a second time; see ExecutionAuthority.Decision. + network: z.enum(["deny", "allowlist", "allow"]), warning: z.string().optional(), }) .optional(), @@ -246,6 +254,9 @@ export namespace ComputeJobs { type Launch = { argv: string[] sandbox?: Job["sandbox"] + /** Proxy variables `execute` must merge into the spawned process's env + * for the loopback shim to be reachable — see `Sandbox.Wrapped.env`. */ + env?: Record } const active = new Map() @@ -683,12 +694,13 @@ export namespace ComputeJobs { ): Promise { const spec = command(job, host) if (host) { + const egress = await EgressRuntime.egressFor(authority.sandbox) const planned = Sandbox.wrapArgv({ file: spec.argv[0]!, args: spec.argv.slice(1), workspace: authority.writable, unreadable: OpenScience.kernelSensitivePaths(), - options: authority.sandbox, + options: { ...authority.sandbox, egress }, }) return { argv: [planned.file, ...planned.args], @@ -699,19 +711,21 @@ export namespace ComputeJobs { network: authority.sandbox.network, warning: planned.warning, }, + env: planned.env, } } await fs.mkdir(logsOf(scope.root), { recursive: true }) await fs.writeFile(exitOf(scope.root, job.id), "", { mode: 0o600 }) const wrapped = `(${job.command}\n); code=$?; printf %s "$code" > ${quote(exitOf(scope.root, job.id))}; exit "$code"` + const egress = await EgressRuntime.egressFor(authority.sandbox) const planned = Sandbox.wrapArgv({ file: Shell.acceptable(), args: ["-lc", wrapped], workspace: authority.writable, extraWritable: [exitOf(scope.root, job.id)], unreadable: OpenScience.kernelSensitivePaths(), - options: authority.sandbox, + options: { ...authority.sandbox, egress }, }) return { argv: [planned.file, ...planned.args], @@ -722,6 +736,7 @@ export namespace ComputeJobs { network: authority.sandbox.network, warning: planned.warning, }, + env: planned.env, } } @@ -730,16 +745,17 @@ export namespace ComputeJobs { cwd: string, authority: ExecutionAuthority.Decision, ): Promise { + const egress = await EgressRuntime.egressFor(authority.sandbox) const planned = Sandbox.wrapArgv({ file: argv[0]!, args: argv.slice(1), workspace: authority.writable, unreadable: OpenScience.kernelSensitivePaths(), - options: authority.sandbox, + options: { ...authority.sandbox, egress }, }) const proc = Bun.spawn([planned.file, ...planned.args], { cwd, - env: await OpenScience.subprocessEnv(process.env), + env: { ...(await OpenScience.subprocessEnv(process.env)), ...(planned.env ?? {}) }, stdin: "ignore", stdout: "pipe", stderr: "ignore", @@ -1103,7 +1119,7 @@ export namespace ComputeJobs { await fs.mkdir(logsOf(scope.root), { recursive: true }) const log = path.join(logsOf(scope.root), `${job.id}.log`) const output = await fs.open(log, "a", 0o600) - const env = await OpenScience.subprocessEnv(process.env) + const env = { ...(await OpenScience.subprocessEnv(process.env)), ...(launch.env ?? {}) } const queued = (await read(scope.root)).find((item) => item.id === job.id) if (queued?.status === "cancelled") { await output.close() @@ -1828,12 +1844,12 @@ export namespace ComputeJobs { args: spec.argv.slice(1), workspace: job.authority.writable, unreadable: OpenScience.kernelSensitivePaths(), - options: job.authority.sandbox, + options: { ...job.authority.sandbox, egress: await EgressRuntime.egressFor(job.authority.sandbox) }, }) - : { file: spec.argv[0]!, args: spec.argv.slice(1) } + : { file: spec.argv[0]!, args: spec.argv.slice(1), env: undefined } const proc = spawn(planned.file, planned.args, { cwd: job.authority?.workspace, - env: await OpenScience.subprocessEnv(process.env), + env: { ...(await OpenScience.subprocessEnv(process.env)), ...(planned.env ?? {}) }, windowsHide: true, stdio: "ignore", }) diff --git a/backend/cli/src/config/config.ts b/backend/cli/src/config/config.ts index 4f7372c5..e8e329b4 100644 --- a/backend/cli/src/config/config.ts +++ b/backend/cli/src/config/config.ts @@ -757,9 +757,15 @@ export namespace Config { "Run local terminals, kernels, and shell commands inside an OS sandbox (macOS Seatbelt / Linux bubblewrap) that confines writes to authorized project roots. Enabled by default.", ), network: z - .enum(["allow", "deny"]) + .enum(["deny", "allowlist", "allow"]) .optional() - .describe("Whether sandboxed commands may reach the network. Default: deny."), + .describe("Whether sandboxed commands may reach the network. Default: allowlist."), + allowHosts: z + .array(z.string()) + .optional() + .describe( + "Extra hosts sandboxed processes may reach when network is 'allowlist'. A leading dot matches subdomains, e.g. '.internal.example.com'.", + ), allowWrite: z .array(z.string()) .optional() @@ -1718,7 +1724,8 @@ export namespace Config { const policy = { ...(base ?? {}), ...(managed ?? {}) } return { enabled: policy.enabled ?? true, - network: policy.network ?? "deny", + network: policy.network ?? "allowlist", + allowHosts: policy.allowHosts ?? [], allowWrite: policy.allowWrite ?? [], onUnavailable: policy.onUnavailable ?? "error", } diff --git a/backend/cli/src/index.ts b/backend/cli/src/index.ts index 09d00e66..ae0d1cc1 100644 --- a/backend/cli/src/index.ts +++ b/backend/cli/src/index.ts @@ -41,6 +41,24 @@ import { SandboxCommand } from "./cli/cmd/sandbox" import { InitCommand, DoctorCommand } from "./cli/onboard" import { OpenScience } from "./openscience" +// Handle the hidden egress shim before any other CLI machinery — yargs +// construction and its global `.middleware` below — is reached. This runs +// inside the sandboxed namespace `bubblewrapArgs` builds, where `/` is +// read-only (the middleware's `Log.init` opens a log file — EROFS) and the +// network is unshared except for the one bind-mounted socket (the +// middleware's `OpenScience.refreshIfStale` is an HTTP fetch — hangs against +// a severed network). The shim only opens a listener and forwards bytes; it +// must never reach either. This is a plain check against argv, before yargs +// (or anything yargs triggers) parses anything, so it cannot regress if the +// middleware grows later — there is no yargs command path to keep in sync. +if (process.argv[2] === "__egress-shim") { + const { Egress } = await import("./sandbox/egress") + const { SHIM_READY_MARKER } = await import("./sandbox/egress-shim-marker") + Egress.serveShim({ port: Number(process.argv[3]), socket: process.argv[4] as string }) + await Bun.write(SHIM_READY_MARKER, "").catch(() => {}) + await new Promise(() => {}) +} + process.on("unhandledRejection", (e) => { Log.Default.error("rejection", { e: e instanceof Error ? e.message : e, diff --git a/backend/cli/src/openscience/index.ts b/backend/cli/src/openscience/index.ts index 870d9b55..7393bcc3 100644 --- a/backend/cli/src/openscience/index.ts +++ b/backend/cli/src/openscience/index.ts @@ -121,6 +121,12 @@ const KERNEL_RUNTIME_KEYS = new Set([ "WINDIR", "PATHEXT", "COMSPEC", + "HTTP_PROXY", + "HTTPS_PROXY", + "NO_PROXY", + "http_proxy", + "https_proxy", + "no_proxy", ]) const SAFE_SYNCED_KEYS = new Set([ ...BYOK_LLM_ENV_KEYS, diff --git a/backend/cli/src/project/execution.ts b/backend/cli/src/project/execution.ts index 53c37e24..a6e6c41b 100644 --- a/backend/cli/src/project/execution.ts +++ b/backend/cli/src/project/execution.ts @@ -32,6 +32,20 @@ export namespace ExecutionAuthority { ]) export type Capability = z.infer + // Not pure in-memory state: `compute/jobs.ts`'s `Job.authority` field + // stores a `Decision` verbatim in the on-disk job history (`jobs.json`), so + // `sandbox.network` below is a *second* copy of the persisted enum that + // `Job.sandbox.network` also carries — widening it (e.g. adding + // "allowlist") has the same one-directional compatibility cost: a job + // record this binary writes with the new value is rejected by an older + // binary reading the same history. The stakes are higher than "that one + // record" though — `ComputeJobs`'s `read()` runs `Job.array().safeParse()` + // over the *whole file* and throws `ComputeJobsCorruptError` for all of it + // on any single unparseable record (`compute/jobs.ts`'s `read()`), moving + // the file aside as `.corrupt-` rather than skipping the bad one. So + // one job with `sandbox.network: "allowlist"` written by a newer binary + // makes an older binary reject its entire compute job history, not just + // fail to display that job. export const Decision = z.object({ allowed: z.boolean(), reason: z.enum(["allowed", "project_untrusted", "sandbox_unavailable"]), @@ -46,7 +60,7 @@ export namespace ExecutionAuthority { writable: z.array(z.string()), sandbox: z.object({ enabled: z.boolean(), - network: z.enum(["allow", "deny"]), + network: z.enum(["deny", "allowlist", "allow"]), allowWrite: z.array(z.string()), onUnavailable: z.enum(["warn", "error", "allow"]), backend: z.enum(["seatbelt", "bubblewrap", "none"]), diff --git a/backend/cli/src/pty/index.ts b/backend/cli/src/pty/index.ts index a9656744..54a1df59 100644 --- a/backend/cli/src/pty/index.ts +++ b/backend/cli/src/pty/index.ts @@ -10,6 +10,7 @@ import { lazy } from "@synsci/util/lazy" import { Shell } from "@/shell/shell" import { ExecutionAuthority } from "@/project/execution" import { Sandbox } from "@/sandbox/sandbox" +import { EgressRuntime } from "@/sandbox/egress-runtime" import { OpenScience } from "@/openscience" import { terminalArgs, terminalEnv } from "./environment" @@ -108,14 +109,21 @@ export namespace Pty { const args = terminalArgs(command) const cwd = authority.workspace const source = await OpenScience.subprocessEnv(process.env) - const env = terminalEnv(source, Instance.project.id, input.sessionID, command) + const egress = await EgressRuntime.egressFor(authority.sandbox) const sandbox = Sandbox.wrapArgv({ file: command, args, workspace: authority.writable, unreadable: OpenScience.kernelSensitivePaths(), - options: authority.sandbox, + options: { ...authority.sandbox, egress }, }) + // terminalEnv gained a `command` argument on main (it picks the PS1/PROMPT + // shape from the shell); the env still has to be built *after* wrapArgv, + // because sandbox.env carries the proxy variables the shim needs. + const env = { + ...terminalEnv(source, Instance.project.id, input.sessionID, command), + ...(sandbox.env ?? {}), + } log.info("creating session", { id, cmd: command, args, cwd }) const spawn = await pty() diff --git a/backend/cli/src/sandbox/egress-runtime.ts b/backend/cli/src/sandbox/egress-runtime.ts new file mode 100644 index 00000000..e3dbcb33 --- /dev/null +++ b/backend/cli/src/sandbox/egress-runtime.ts @@ -0,0 +1,289 @@ +import crypto from "crypto" +import fs from "fs/promises" +import path from "path" +import { Config } from "@/config/config" +import { Global } from "@/global" +import { GlobalBus } from "@/bus/global" +import { Event } from "@/server/event" +import { Log } from "@/util/log" +import { Egress } from "./egress" +import { Sandbox } from "./sandbox" + +const log = Log.create({ service: "egress-runtime" }) + +/** + * Lifecycle for the host-side allowlist proxy — the listening end of + * `Egress.serveProxy` (see `egress.ts` for the proxy itself and + * `docs/adr/0002-sandbox-network-policy.md` for why it exists). + * + * One proxy per process, held lazily with a disposer, the same shape + * `science/kernel/registry.ts` uses for its kernel table: nothing runs until + * the first `ensure()`, and `stop()` tears down the server and unlinks the + * socket. Unlike that table, this is not `Instance.state` — the proxy must + * outlive any single project instance, since a global config write disposes + * every open instance (`Config`'s `patchConfigPath`) and the proxy must not + * go down with them, or every kernel bound to its socket would lose its only + * route out. + * + * `rules` is a live array, not a snapshot. `Egress.serveProxy` reads it by + * reference on every connection, so refreshing its *contents* in place — + * on every `ensure()`, and reactively whenever global config changes — is + * what lets an allowlist edit reach a kernel that is already running, + * without restarting the proxy or the kernel. Building a fresh array once + * at construction and handing it to `serveProxy` would silently defeat + * that: the proxy would keep the rules it was born with until the process + * itself restarted. This is also why `allowHosts` stays out of + * `ExecutionAuthority.generation` — that hash exists to decide when a + * kernel must be torn down and rebooted, and an allowlist edit is + * deliberately not that kind of change. + */ +export namespace EgressRuntime { + /** + * Bubblewrap (Linux) carries `socket` — the bind-mounted socket itself is + * the sandboxed process's only route in. Seatbelt (macOS) has no network + * namespace to bind a socket into, so `Egress.serveProxy` listens directly + * on a loopback TCP port instead (see its own doc comment and + * `sandbox.ts`'s `seatbeltProfile`), carried as `hostname`/`secret` — + * `secret` is the per-start `Proxy-Authorization` credential that port + * requires, since a loopback TCP port, unlike a unix socket, carries no + * filesystem permissions of its own. Optional fields rather than a + * discriminated union: every real caller narrows by checking `socket` + * (see `stop()`, `ensure()`, `egressFor()` below), and a union would force + * that same narrowing onto every *test* that reaches these fields too, + * including the bubblewrap-only ones this task must leave unchanged. + */ + type Running = { + socket?: string + hostname?: string + port: number + secret?: string + // Not `ReturnType`: TS resolves that utility + // against an overloaded function's LAST signature only (the TCP one + // here), not a union of all of them — this field needs both, since + // `startBubblewrap`'s `server` really is a `UnixSocketListener`. + server: Bun.UnixSocketListener | Bun.TCPSocketListener + rules: Egress.Rule[] + onGlobalChange: (event: { directory?: string; payload: unknown }) => void + } + + const state: { running?: Promise } = {} + + async function currentRules(): Promise { + const policy = await Config.trustedSandbox() + return [...Egress.DEFAULT_RULES, ...(policy.allowHosts ?? [])] + } + + /** Re-populate `rules` in place (same array reference) rather than + * replacing it, so `Egress.serveProxy`'s closure over that reference + * observes the update on its very next connection. A failed re-read + * (config file briefly unreadable mid-write, for example) keeps + * whatever rules were already live rather than clearing the allowlist. */ + async function refresh(rules: Egress.Rule[]) { + const next = await currentRules().catch((error) => { + log.warn("failed to refresh the sandbox allowlist, keeping the previous rules", { error }) + return undefined + }) + if (!next) return + rules.length = 0 + rules.push(...next) + } + + function isGlobalConfigChange(event: { directory?: string; payload: unknown }): boolean { + if (event.directory !== "global") return false + const payload = event.payload + if (typeof payload !== "object" || payload === null || !("type" in payload)) return false + return payload.type === Event.Disposed.type + } + + function listener(rules: Egress.Rule[]) { + const onGlobalChange = (event: { directory?: string; payload: unknown }) => { + if (!isGlobalConfigChange(event)) return + refresh(rules).catch(() => {}) + } + GlobalBus.on("event", onGlobalChange) + return onGlobalChange + } + + /** Bubblewrap (Linux): a bind-mountable unix socket under the state dir — + * unchanged from before Task 7 (macOS seatbelt support) added the + * loopback-TCP branch below. */ + async function startBubblewrap(): Promise { + const socket = path.join(Global.Path.state, `egress-${process.pid}.sock`) + // A stale socket file from a killed previous process (same pid, unlikely + // but possible after a pid wraparound) would make Bun.listen refuse to + // bind with EADDRINUSE. + await fs.rm(socket, { force: true }) + const rules = await currentRules() + // Bun.listen throws synchronously, with a message ("Failed to listen at + // ") that says nothing about what depends on it. Every sandboxed + // spawn does, so name that here rather than letting a bare bind error + // surface out of an unrelated-looking `bash`/kernel/job call. + const server = (() => { + try { + return Egress.serveProxy({ socket, rules, onEvent: (line) => log.info(line) }) + } catch (e) { + throw new Error( + `Could not start the sandbox allowlist proxy on ${socket}: ${e instanceof Error ? e.message : String(e)}. ` + + `Sandboxed commands need it to reach the network — retry once the path is writable, or set sandbox.network to "deny" or "allow".`, + ) + } + })() + const onGlobalChange = listener(rules) + log.info("egress proxy listening", { socket }) + return { socket, port: Sandbox.SHIM_PORT, server, rules, onGlobalChange } + } + + /** + * Seatbelt (macOS): no network namespace to bind a unix socket into, so + * `Egress.serveProxy` listens directly on a loopback TCP port instead — + * decision 1 of the Task 7 brief, deliberately *not* a host-side bridge + * from TCP to a unix socket (that would just be a second component doing + * what one listener already can). `port: 0` asks the OS for an ephemeral + * port: unlike bubblewrap's `SHIM_PORT`, nothing needs this value fixed in + * advance — no shim script embeds it as a literal, since seatbelt has no + * shim at all (see `sandbox.ts`'s `shimPlan` doc comment) — and a fixed + * port here, with no namespace to keep it private, would collide across + * every concurrently sandboxed process on the machine. + * + * `secret` is generated fresh per proxy start (decision 2): a loopback TCP + * port, unlike a unix socket, carries no filesystem permissions of its + * own, so every request to it must additionally prove it holds this — + * enforced inside `Egress.serveProxy` itself, not here. + */ + async function startSeatbelt(): Promise { + const rules = await currentRules() + const secret = crypto.randomUUID() + const server = (() => { + try { + return Egress.serveProxy({ hostname: "127.0.0.1", port: 0, secret, rules, onEvent: (line) => log.info(line) }) + } catch (e) { + throw new Error( + `Could not start the sandbox allowlist proxy on 127.0.0.1: ${e instanceof Error ? e.message : String(e)}. ` + + `Sandboxed commands need it to reach the network — retry, or set sandbox.network to "deny" or "allow".`, + ) + } + })() + const onGlobalChange = listener(rules) + log.info("egress proxy listening", { hostname: "127.0.0.1", port: server.port }) + return { hostname: "127.0.0.1", port: server.port, secret, server, rules, onGlobalChange } + } + + /** + * `platform` decides which of the two listeners above starts — defaulting + * to the real platform, like every other platform-injectable seam this + * branch added (`Sandbox.backend`, `plan`/`wrapArgv`), so the seatbelt + * branch is exercisable, deterministically, from a machine that has none. + */ + function start(platform: NodeJS.Platform = process.platform): Promise { + return platform === "darwin" ? startSeatbelt() : startBubblewrap() + } + + /** Start the proxy if it is not already running, and return where to + * reach it. Idempotent — a second call returns the same address without + * restarting anything. Also refreshes the live rules from the current + * config, so a caller composing a new sandboxed argv always gets the + * latest allowlist even between reactive updates. + * + * A failure is loud but never permanent. Caching the promise is what makes + * the success path idempotent, and it would just as happily cache a + * rejection: one transient failure — a state directory briefly unwritable, + * a socket path momentarily taken — would then be replayed to every later + * caller for the life of the process, and since every bash command, + * terminal, kernel and compute job routes through here under the + * "allowlist" default, that is the whole product failing until restart. + * So a rejected start un-caches itself and the next call genuinely + * retries. It still throws rather than degrading to no-proxy: `wrapArgv` + * would reject an "allowlist" policy with no egress socket anyway, and a + * silent downgrade is exactly the failure this feature keeps producing — + * a sandbox that looks like it has bounded egress and in fact has none. + * + * `platform` decides only which listener `start()` picks — see its doc + * comment — and is only ever non-default from a test; every real caller + * (`egressFor` below) leaves it at the real one. The proxy itself is not + * re-created per platform: `state.running` is one proxy for the process + * lifetime, same as before this parameter existed, so a caller that wants + * a differently-platformed proxy exercised must `stop()` first. */ + export async function ensure( + platform: NodeJS.Platform = process.platform, + ): Promise<{ socket?: string; hostname?: string; port: number; secret?: string }> { + const pending = (state.running ??= start(platform)) + const running = await pending.catch((error) => { + if (state.running === pending) state.running = undefined + throw error + }) + await refresh(running.rules) + return { socket: running.socket, hostname: running.hostname, port: running.port, secret: running.secret } + } + + /** Stop the proxy. The CLI process otherwise leaves this running for its + * own lifetime; tests use this to reset between cases. A no-op when + * nothing is running, and — because a caller reaching for the escape + * hatch after a failed start must not be handed that same failure again — + * when the last start rejected. Unlinks the unix socket on bubblewrap; + * seatbelt's loopback listener leaves nothing on disk to clean up. */ + export async function stop() { + const pending = state.running + state.running = undefined + if (!pending) return + const running = await pending.catch(() => undefined) + if (!running) return + GlobalBus.off("event", running.onGlobalChange) + running.server.stop(true) + if (running.socket) await fs.rm(running.socket, { force: true }) + } + + /** The value to pass as `Sandbox.Options.egress`, or `undefined` when the + * proxy would not actually be used: the sandbox is off, network isn't + * "allowlist", or the platform's backend is neither bubblewrap nor + * seatbelt. The shape differs by backend, matching `Options.egress`'s own + * doc comment: bubblewrap gets the bind-mountable unix socket path, since + * the bind-mounted socket itself is the sandboxed process's only route in; + * seatbelt gets `":"` — `buildPolicy` in sandbox.ts is what + * splits that back apart into `Policy.port`/`Policy.secret`, the same + * division of labour it already has for bubblewrap's `Policy.egress`. A + * disabled/deny/allow policy skips starting the proxy entirely — pure + * waste when nothing would ever connect to it. Every `wrapArgv` / + * `plan()` caller should route through this rather than calling `ensure()` + * directly, so a terminal or kernel with network "deny" never pays for a + * proxy it has no way to reach. + * + * `platform` defaults to the real one — the same injectable seam + * `Sandbox.backend`/`plan`/`wrapArgv` use — so the seatbelt branch is + * exercisable, deterministically, from a machine that has none. + * + * `ensure()` caches ONE proxy for the process lifetime (see its doc + * comment); `platform` only decides which listener `start()` picks when + * nothing is running yet. Asking for `"darwin"` after a differently- + * platformed proxy is already cached (a real caller never does this — + * `process.platform` is constant for the life of a process — but a test + * injecting platform explicitly can) silently reuses that cached + * listener instead of starting a seatbelt one. Interpolating a + * bubblewrap `Running`'s missing `secret` into the template literal + * below would then produce the *string* `"undefined"` — truthy, and + * therefore indistinguishable from a real secret to any check that only + * asks whether the value is present. Guarded explicitly rather than + * trusting the interpolation to fail loudly on its own, because it + * doesn't: confirmed by execution (Task 7 fix round 1 review) that it + * silently composes `":undefined"` instead. */ + export async function egressFor( + policy: Sandbox.Options, + platform: NodeJS.Platform = process.platform, + ): Promise { + const { enabled, network } = Sandbox.resolved(policy) + if (!enabled) return undefined + if (network !== "allowlist") return undefined + const b = Sandbox.backend(platform) + if (b === "bubblewrap") return (await ensure(platform)).socket + if (b === "seatbelt") { + const running = await ensure(platform) + if (!running.secret) { + throw new Error( + "sandbox egress proxy is already running as the bubblewrap (unix-socket) listener, not seatbelt's — " + + "call EgressRuntime.stop() first if a seatbelt proxy is genuinely needed here", + ) + } + return `${running.port}:${running.secret}` + } + return undefined + } +} diff --git a/backend/cli/src/sandbox/egress-shim-entry.ts b/backend/cli/src/sandbox/egress-shim-entry.ts new file mode 100644 index 00000000..1fc33795 --- /dev/null +++ b/backend/cli/src/sandbox/egress-shim-entry.ts @@ -0,0 +1,56 @@ +/** + * Minimal, dependency-light entry point for the sandboxed loopback shim. In + * development `Sandbox.shimPlan()` bundles this file and execs `bun` against + * the bundle — never against `src/index.ts` — because `src/index.ts`'s graph + * pulls in `Global` (an unguarded top-level `await Bun.file(...).write(...)` + * at `src/global/index.ts` — `EROFS` under a read-only source tree) and + * `ModelsDev` (a live fetch at module-eval time). Both run before any argv + * check could skip them and would kill the shim under exactly the + * read-only/no-network conditions this mechanism exists to survive. This + * file imports only `./egress` (nothing but a Bun type) and the marker + * constant below (a single string, no other exports), so evaluating it does + * no I/O beyond the two lines that matter. + * + * `shimPlan()` does not exec this file: it runs `bun build` over it and execs + * the resulting self-contained bundle, because only the bundle's own path has + * to be visible inside the sandbox, where `--tmpfs /tmp` masks whatever it + * covers. So an added import does not have to live anywhere in particular — + * a sibling module and an npm package are equally fine, and the npm case is + * specifically what a package-root bind got wrong before (in this bun + * workspace `node_modules/` is a symlink into the monorepo-root store, + * above the package root: the link was bound, its target was not). + * + * Four things are still not safe to add here, and none of them are about + * where files live. Import-time side effects (a top-level fetch, a top-level + * write) run in the bundle exactly as they would in the source, and would + * reintroduce the failure this file exists to avoid — the shim dies under the + * read-only, no-network conditions it is supposed to survive, with its output + * on /dev/null. Resolving anything from `import.meta.dir`/`url` points at + * `Global.Path.bin`, where the bundle runs, not at this directory. A runtime + * `import(expression)` cannot be inlined by the bundler, so it would resolve + * against a path nothing bound (a literal `import("./x")` is inlined and + * fine). And a dependency that loads a native binding bundles cleanly but + * still `dlopen`s a `.so` at run time, from a path nothing bound and nothing + * checks — see `shimPlan`'s residual list for the measurement. + * + * A compiled release has no separate entry to redirect to — `bun --compile` + * embeds a single one — so it still goes through `index.ts`'s + * `__egress-shim` argv check and therefore still evaluates that full graph. + * See `sandbox.ts`'s `shimPlan` doc comment and the Task 4 report for what + * that leaves reachable in a compiled binary. + * + * `Sandbox.shimScript` composes one call shape for both modes — + * ` __egress-shim ` — because the compiled path needs + * the "__egress-shim" token to dispatch inside `index.ts`'s single-entry + * argv check. This file has no dispatching to do, so it ignores that token + * and reads port/socket positionally from the end instead of assuming a + * fixed prefix, which also means it still works if `shimScript` ever calls + * it without the token. + */ +import { Egress } from "./egress" +import { SHIM_READY_MARKER } from "./egress-shim-marker" + +const [port, socket] = process.argv.slice(-2) +Egress.serveShim({ port: Number(port), socket: socket! }) +await Bun.write(SHIM_READY_MARKER, "").catch(() => {}) +await new Promise(() => {}) diff --git a/backend/cli/src/sandbox/egress-shim-marker.ts b/backend/cli/src/sandbox/egress-shim-marker.ts new file mode 100644 index 00000000..275023cf --- /dev/null +++ b/backend/cli/src/sandbox/egress-shim-marker.ts @@ -0,0 +1,17 @@ +/** + * Readiness marker `Sandbox.shimScript`'s composed wait loop polls for, and + * the `__egress-shim` handler (`index.ts`, `egress-shim-entry.ts`) touches + * once `Egress.serveShim`'s listener is bound. + * + * A single exported constant, not three independently hardcoded copies of + * the same string: the three call sites drifting apart is a silent 3s stall + * on every sandboxed command, not a loud failure, so nothing would catch it + * happening. This file has no other exports and does nothing at import + * time, so importing it (including from `egress-shim-entry.ts`, which must + * stay dependency-light) costs nothing. + * + * Lives under `/tmp` deliberately: `bubblewrapArgs` always mounts `/tmp` as + * a fresh, process-private tmpfs, so a fixed name here can't collide across + * sandboxed processes or persist from a previous run. + */ +export const SHIM_READY_MARKER = "/tmp/.openscience-egress-shim.ready" diff --git a/backend/cli/src/sandbox/egress.ts b/backend/cli/src/sandbox/egress.ts new file mode 100644 index 00000000..59b91d7d --- /dev/null +++ b/backend/cli/src/sandbox/egress.ts @@ -0,0 +1,572 @@ +import type { Socket, SocketHandler } from "bun" + +/** + * Allowlist egress proxy for sandboxed kernels. + * + * `sandbox.network` is otherwise binary: deny (`--unshare-net`) locks out + * NCBI, UniProt, PDB and PyPI, which is most of the product's purpose; allow + * is unrestricted egress. This gives a middle: an allowlist proxy, with no + * direct DNS inside the sandbox — name resolution happens at the proxy. + * + * A bind-mounted unix socket still crosses `--unshare-net`'s network + * namespace, so it is the only route out, and the proxy on the other end + * decides what is reachable. No pasta, no nftables, no root. + * + * Two roles: + * serveProxy — runs on the HOST, speaks HTTP proxy. Listens on a unix + * socket for bubblewrap (Linux), or directly on a loopback TCP + * port for seatbelt (macOS), which has no network namespace to + * bind a socket into — see sandbox.ts's seatbeltProfile. The + * TCP form additionally requires a Proxy-Authorization secret, + * since a loopback port (unlike a unix socket) carries no + * filesystem permissions of its own. + * serveShim — runs INSIDE the sandbox, TCP on loopback → the unix socket, + * because pip/requests/curl take a host:port proxy, not a + * unix path. Bubblewrap-only: seatbelt's serveProxy needs no + * bridge, since it already listens on TCP directly. + * + * Ported from the feasibility spike on `proto/sandbox-allowlist-proxy` + * (`src/sandbox/prototype/proxy.ts`); see that branch's README for the + * measurements behind the design. + */ +export namespace Egress { + export type Rule = string + + /** Exact host, or a leading dot for suffix match: ".ncbi.nlm.nih.gov". */ + export function allowed(host: string, rules: Rule[]): boolean { + const name = host.toLowerCase().split(":")[0] + return rules.some((rule) => { + const value = rule.toLowerCase() + if (value.startsWith(".")) return name === value.slice(1) || name.endsWith(value) + return name === value + }) + } + + export const DEFAULT_RULES: Rule[] = [ + // package registries + "pypi.org", + ".pypi.org", + "files.pythonhosted.org", + ".pythonhosted.org", + "cran.r-project.org", + ".bioconductor.org", + // scientific APIs + ".ncbi.nlm.nih.gov", + ".uniprot.org", + ".rcsb.org", + ".ebi.ac.uk", + ".ensembl.org", + "arxiv.org", + ".arxiv.org", + ] + + /** + * One direction of a bridged pair, with backpressure. + * + * `Socket.write` returns how many bytes the socket actually accepted, and + * that is fewer than the whole chunk the moment the kernel send buffer + * fills. Writing and discarding the count silently drops the remainder: + * measured on this proxy before this existed, a 40 MB transfer arrived as + * 2.6 MB through the proxy alone and 11.9 MB through shim + proxy, while + * the same origin read directly delivered all 40 MB. Small responses fit in + * one buffer and never show it, which is why every test that pushed + * `hello` through passed. + * + * So: queue whatever the destination refused, flush it from the + * destination's own `drain`, and pause the *source* while a backlog exists + * so the queue tracks the slower end's pace instead of growing to the size + * of the transfer. `end()` is deferred until the queue has actually gone + * out — an upstream that closes right after a large body must not truncate + * what is still in flight to the client. + */ + function pump(target: Socket) { + const queue: Buffer[] = [] + const hold = (chunk: Buffer) => { + // Copied, not retained: the buffer handed to a `data` callback belongs + // to the caller for the duration of that call, and this outlives it. + queue.push(Buffer.from(chunk)) + held.source?.pause() + } + const held = { + /** The socket feeding this direction; paused while a backlog exists. */ + source: undefined as Socket | undefined, + ending: false, + send(chunk: Buffer) { + if (queue.length > 0) return hold(chunk) + const wrote = target.write(chunk) + if (wrote >= chunk.length) return + hold(chunk.subarray(Math.max(wrote, 0))) + }, + /** Drive from the target socket's `drain` handler, nowhere else. */ + flush() { + while (queue.length > 0) { + const head = queue[0]! + const wrote = target.write(head) + if (wrote < head.length) { + if (wrote > 0) queue[0] = head.subarray(wrote) + return + } + queue.shift() + } + held.source?.resume() + if (held.ending) target.end() + }, + end() { + held.ending = true + if (queue.length === 0) target.end() + }, + } + return held + } + + type Pump = ReturnType + + /** + * A client connection's progress through the proxy, tracked explicitly + * because `data` is async and Bun does not serialize its handlers: a second + * chunk re-enters `data` while the first is parked on `await Bun.connect`. + * Without a state set *before* that await, the re-entrant call finds no + * link yet, re-parses the same still-buffered head, and dials the origin a + * second time — measured: one client POST whose body followed the head by + * 1/5/10ms produced 2 upstream connections to a local origin and 4 to a + * real remote one, each carrying a duplicate of a non-idempotent request, + * with `toUpstream` left pointing at whichever dial resolved last. + * + * head — still reading the request head + * dialing — head parsed and allowed, upstream connect in flight + * linked — bytes flow both ways + * closed — denied, unreachable, or the client went away + * + * `closed` is what a dial in flight checks when it resolves, so a client + * that aborts mid-dial cannot strand an upstream socket nobody will ever + * close. + */ + type Phase = "head" | "dialing" | "linked" | "closed" + + /** What both bridges track per client connection. The shim has no head to + * read, so it simply starts at `dialing`. */ + type Link = { phase: Phase; toClient: Pump; toUpstream?: Pump } + + type Pending = Link & { buffer: string } + + const state = new WeakMap, Pending>() + + /** Mark a client gone and release whatever it owns. Setting the phase is + * what a dial still in flight sees when it resolves; without it, the + * socket that dial produces is owned by nobody — `close` has already run + * and found no link to tear down. */ + function shut(held?: Link) { + if (!held) return + held.phase = "closed" + held.toUpstream?.end() + } + + /** Read the phase through a call, not a comparison in place: assigning + * `dialing` earlier in the same scope narrows the property to that literal, + * and the compiler has no way to know `shut` can change it while an + * `await` is parked — which is the entire point of asking. */ + const gone = (held: Link) => held.phase === "closed" + + const refuse = (status: string, reason: string) => + `HTTP/1.1 ${status}\r\nContent-Type: text/plain\r\nConnection: close\r\n\r\n${reason}\n` + + const deny = (reason: string) => refuse("403 Forbidden", reason) + + /** + * Sent only by the TCP/loopback listener (darwin's seatbelt path — see + * `egress-runtime.ts`). A unix socket's access control is the filesystem + * permissions on the path itself; a loopback TCP port has none — every + * process on the machine can dial it — so that listener additionally + * requires a `Proxy-Authorization` header carrying a secret generated once + * per proxy start, and refuses (without forwarding anything) a request + * missing it or carrying the wrong one. `Proxy-Authenticate` names the + * scheme per RFC 7235. + */ + const unauthorized = () => + `HTTP/1.1 407 Proxy Authentication Required\r\nProxy-Authenticate: Basic realm="os"\r\nContent-Type: text/plain\r\nConnection: close\r\n\r\nMissing or invalid Proxy-Authorization\n` + + const latin1 = (text: string) => Buffer.from(text, "latin1") + + /** + * How much of a request head to accept before refusing the connection. + * + * The head phase has no natural end other than `\r\n\r\n`, so a client that + * never sends one is buffered without limit — and because no dial is + * attempted on that path, nothing downstream bounds it either. Measured + * against this proxy before this cap existed: 93 MiB of never-terminated + * head took the *host* process from 36 MB to 1.34 GB of RSS in 8 seconds, + * and it was still climbing when the client stopped. + * + * 64 KiB is Squid's `request_header_max_size` default — the most generous of + * the conventional caps (nginx `large_client_header_buffers` 8k, Apache + * `LimitRequestFieldSize` 8190, Node `--max-http-header-size` 16 KiB) and the + * closest analogue, Squid being a forward proxy that speaks CONNECT. The + * clients here are pip, curl and requests, whose heads run 200-600 bytes, so + * this cannot plausibly refuse a real one. + */ + const HEAD_LIMIT = 64 * 1024 + + /** + * How long to wait for an upstream TCP connect before giving up. + * + * Linux retries a SYN for ~130 s by default, so an allowlisted host that + * black-holes packets — a firewall that drops rather than rejects — pins the + * client connection and its fd for over two minutes and then fails with no + * explanation. 30 s is far above any real handshake, which costs one RTT + * plus name resolution, and turns that wait into a legible 504. + */ + const DIAL_TIMEOUT = 30_000 + + type ServeProxyCommon = { rules: Rule[]; onEvent?: (line: string) => void; dialTimeout?: number } + + /** + * Host side. Proxies only allowlisted hosts. Listens on a unix socket + * (bubblewrap, Linux) or directly on a loopback TCP port with a required + * `secret` (seatbelt, macOS — see the module doc comment and + * `egress-runtime.ts`). Overloaded, not one union signature, so each call + * site gets back the concrete `UnixSocketListener`/`TCPSocketListener` its + * own input shape implies — `egress-runtime.ts`'s seatbelt path reads + * `.port` off the result, which only `TCPSocketListener` has. + * `dialTimeout` overrides `DIAL_TIMEOUT`; it exists so the timeout can be + * exercised in milliseconds rather than by making a test wait half a + * minute for the real one. + */ + export function serveProxy(input: ServeProxyCommon & { socket: string }): Bun.UnixSocketListener + export function serveProxy( + input: ServeProxyCommon & { hostname: string; port: number; secret: string }, + ): Bun.TCPSocketListener + export function serveProxy( + input: ServeProxyCommon & ({ socket: string } | { hostname: string; port: number; secret: string }), + ) { + const log = input.onEvent ?? (() => {}) + const budget = input.dialTimeout ?? DIAL_TIMEOUT + // Set only by the TCP/loopback listener — see `unauthorized` above for why. + const authorization = + "secret" in input ? `Basic ${Buffer.from(`os:${input.secret}`).toString("base64")}` : undefined + + // Branched, and the handlers built once and passed to whichever branch + // fires, rather than a spread of the two option shapes into one object: + // Bun.listen is overloaded on unix vs hostname/port, and a union spread + // matches neither overload (the same reason this file's own tests branch + // Bun.connect for the mirror image of this call). + const listen = (socket: SocketHandler) => + "socket" in input + ? Bun.listen({ unix: input.socket, socket }) + : Bun.listen({ hostname: input.hostname, port: input.port, socket }) + + return listen({ + open(client) { + state.set(client, { buffer: "", phase: "head", toClient: pump(client) }) + }, + async data(client, chunk) { + const held = state.get(client) + if (!held) return + if (held.phase === "linked") { + held.toUpstream?.send(chunk) + return + } + if (held.phase === "closed") return + + // Everything before the link is one buffer, so body bytes that land + // while the dial is in flight are simply still here when it + // resolves — `rest` is sliced after the await, not before it. + held.buffer += chunk.toString("latin1") + if (held.phase === "dialing") return + const end = held.buffer.indexOf("\r\n\r\n") + if (end === -1) { + if (held.buffer.length <= HEAD_LIMIT) return + // Fail closed. Unlike the dial window below there is no + // backpressure to apply here: the terminator is what the parse is + // waiting for, so refusing to read simply deadlocks the connection + // instead of ending it. A head this long is a protocol error. + log(`OVERSIZE ${held.buffer.length} bytes of head with no terminator`) + held.phase = "closed" + held.buffer = "" + held.toClient.send( + latin1(refuse("431 Request Header Fields Too Large", `Proxy request head exceeded ${HEAD_LIMIT} bytes`)), + ) + held.toClient.end() + return + } + + const head = held.buffer.slice(0, end) + const lines = head.split("\r\n") + const request = lines[0] ?? "" + const [method, target, version = "HTTP/1.1"] = request.split(" ") + + // Checked before anything about the request is even inspected for + // validity — an unauthenticated caller learns nothing about + // whether its target was well-formed, let alone allowlisted. + if (authorization) { + const header = lines.slice(1).find((line) => /^proxy-authorization:/i.test(line)) + const provided = header?.slice(header.indexOf(":") + 1).trim() + if (provided !== authorization) { + log(`AUTH missing or invalid Proxy-Authorization`) + held.phase = "closed" + held.buffer = "" + held.toClient.send(latin1(unauthorized())) + held.toClient.end() + return + } + } + + // CONNECT host:443 for TLS; absolute-form GET http://host/path for plain. + const url = + method === "CONNECT" + ? undefined + : (() => { + try { + return new URL(target) + } catch { + return undefined + } + })() + const authority = method === "CONNECT" ? target : url?.host + + if (!authority) { + log(`malformed ${request.slice(0, 60)}`) + held.phase = "closed" + held.toClient.send(latin1(deny("Malformed proxy request"))) + held.toClient.end() + return + } + + if (!allowed(authority, input.rules)) { + log(`DENY ${authority}`) + held.phase = "closed" + held.toClient.send(latin1(deny(`Host not on the sandbox allowlist: ${authority.split(":")[0]}`))) + held.toClient.end() + return + } + + const [hostname, port] = authority.split(":") + // Claim the dial before yielding. Everything above this line is + // synchronous, so no second chunk can be part-way through the same + // parse when it runs. + held.phase = "dialing" + // Backpressure, not a buffer limit. Everything the client sends + // while the dial is in flight would otherwise be held here, and a + // dial can be slow for as long as the OS retries a SYN: measured + // against a black-holed allowlisted origin, 8 seconds of blasting + // took the host process from 36 MB to 2.12 GB and then killed it + // outright with `RangeError: Out of memory` — and this proxy runs in + // the CLI's own process, so that is the supervisor dying at the hands + // of the thing the sandbox exists to contain. + // + // Pausing costs nothing and has no arbitrary limit: the bytes wait in + // the client's own socket buffer, and then in the client. Round 3 + // declined to do this on the grounds that "pausing the client is what + // would stop the FIN that tells us it left" — that is not so, and was + // never measured. A paused socket still reports its peer's departure: + // with delivery demonstrably stopped (0.21 MiB through a paused + // socket against 256 MiB through an unpaused one), the peer's `end()` + // still produced `close` while the pause was in force, for both FIN + // and RST. + client.pause() + // An allowlisted host that black-holes packets otherwise holds this + // connection for the kernel's whole SYN-retry budget. The phase is + // what makes this safe to fire late: `closed` is exactly what the + // dial below checks when it finally resolves, so the socket it + // produces is still ended by nobody-owns-it handling rather than + // stranded. + const timer = setTimeout(() => { + if (held.phase !== "dialing") return + log(`TIMEOUT ${authority}`) + held.phase = "closed" + held.buffer = "" + client.resume() + held.toClient.send(latin1(refuse("504 Gateway Timeout", `Timed out connecting to ${authority}`))) + held.toClient.end() + }, budget) + const upstream = await Bun.connect({ + hostname, + port: Number(port ?? (method === "CONNECT" ? 443 : 80)), + socket: { + data(_sock, payload) { + held.toClient.send(payload) + }, + drain() { + held.toUpstream?.flush() + }, + close() { + held.toClient.end() + }, + error() { + held.toClient.end() + }, + }, + }).catch(() => undefined) + clearTimeout(timer) + + // The client can have gone away while the dial was in flight — or the + // dial can have timed out above — in which case this is the only + // place that can release the socket it just produced. + if (gone(held)) { + upstream?.end() + return + } + + if (!upstream) { + log(`FAIL ${authority}`) + held.phase = "closed" + held.buffer = "" + client.resume() + held.toClient.send(latin1(deny(`Cannot reach ${authority}`))) + held.toClient.end() + return + } + + log(`ALLOW ${authority}`) + const toUpstream = pump(upstream) + toUpstream.source = client + held.toClient.source = upstream + held.toUpstream = toUpstream + held.phase = "linked" + // Sliced now rather than before the dial, so anything the client + // sent while it was in flight goes upstream in arrival order. + const rest = held.buffer.slice(end + 4) + held.buffer = "" + // Resumed before anything is forwarded, not after: `toUpstream` owns + // the client as its source from here, so a forward that has to queue + // re-pauses it through the pump. Resuming afterwards would undo that. + client.resume() + // CONNECT: acknowledge, then the client starts its TLS handshake. + // Plain HTTP: replay the request head we already consumed. + if (method === "CONNECT") { + held.toClient.send(latin1("HTTP/1.1 200 Connection Established\r\n\r\n")) + if (rest) toUpstream.send(latin1(rest)) + return + } + + // A proxy must rewrite absolute-form to origin-form. Forwarding + // `GET http://pypi.org/simple/ HTTP/1.1` verbatim is legal per RFC 7230 + // §5.3.2 but origin servers routinely reject it — measured: 403 from + // pypi.org on the plain-HTTP path while CONNECT to the same host + // returned 200. Also drop hop-by-hop `Proxy-*` headers, which are for + // us and must not travel upstream. + const origin = `${url!.pathname}${url!.search}` || "/" + const headers = lines + .slice(1) + .filter((line) => !/^proxy-/i.test(line)) + .filter((line) => !/^host:/i.test(line)) + const rewritten = [`${method} ${origin} ${version}`, `Host: ${url!.host}`, ...headers].join("\r\n") + toUpstream.send(latin1(`${rewritten}\r\n\r\n${rest}`)) + }, + drain(client) { + state.get(client)?.toClient.flush() + }, + close(client) { + shut(state.get(client)) + state.delete(client) + }, + error(client) { + shut(state.get(client)) + state.delete(client) + }, + }) + } + + /** + * Sandbox side. pip, requests and curl take `http://host:port` from + * HTTP_PROXY — none of them speak unix-socket proxies — so a loopback + * listener inside the namespace forwards raw bytes to the bind-mounted + * socket. + */ + export function serveShim(input: { port: number; socket: string }) { + // `pending` is for the window *before* the link exists — distinct from the + // backpressure queue inside `pump`, which is for after it does. `open` is + // async, so a client that writes immediately — curl sends CONNECT the + // moment the TCP handshake completes — arrives before the upstream link + // exists. Without this buffer those bytes are dropped and the connection + // hangs: the listener accepts, nothing is ever forwarded, and the client + // times out with the socket showing LISTEN the whole time. + // + // It is now a safety net rather than the main path: `open` pauses the + // client before it yields, so in practice nothing is delivered into + // `pending` at all. Keeping it costs nothing and is what stops a byte from + // being dropped should anything ever slip through ahead of the pause — + // dropping one here does not fail loudly, it hangs the connection. + // + // The `closed` phase covers the mirror image: a client that goes away + // *during* that same window. Its `close` runs while `toUpstream` is still + // undefined, so it has nothing to tear down, and the socket the dial then + // produces is owned by nobody. Measured on 300 connect-then-immediately- + // close connections, that stranded one fd per connection in this process + // and one in the host proxy on the other end of it — and a kernel or a + // terminal is a sandbox that lives for hours. + type Bridge = Link & { pending: Buffer[] } + const links = new WeakMap, Bridge>() + + return Bun.listen({ + hostname: "127.0.0.1", + port: input.port, + socket: { + async open(client) { + const held: Bridge = { pending: [], phase: "dialing", toClient: pump(client) } + links.set(client, held) + // The same backpressure the host proxy applies around its own dial, + // and for the same reason: without it a client that starts blasting + // before the link exists is buffered in `pending` without limit. The + // blast radius is smaller here — the shim lives inside the sandbox, + // so it is the sandbox's own memory — but it is the same defect, and + // an unbounded shim would in any case hand the whole blast to the + // host proxy the moment the link came up. + client.pause() + const upstream = await Bun.connect({ + unix: input.socket, + socket: { + data(_sock, payload) { + held.toClient.send(payload) + }, + drain() { + held.toUpstream?.flush() + }, + close() { + held.toClient.end() + }, + error() { + held.toClient.end() + }, + }, + }).catch(() => undefined) + if (gone(held)) { + upstream?.end() + held.pending.length = 0 + return + } + if (!upstream) { + client.resume() + held.toClient.end() + return + } + const toUpstream = pump(upstream) + toUpstream.source = client + held.toClient.source = upstream + held.toUpstream = toUpstream + held.phase = "linked" + // Before the replay, for the reason given in `serveProxy`: a queueing + // send re-pauses the client through the pump, and resuming after + // would undo it. + client.resume() + for (const chunk of held.pending) toUpstream.send(chunk) + held.pending.length = 0 + }, + data(client, chunk) { + const held = links.get(client) + if (!held || gone(held)) return + if (held.toUpstream) return void held.toUpstream.send(chunk) + held.pending.push(Buffer.from(chunk)) + }, + drain(client) { + links.get(client)?.toClient.flush() + }, + close(client) { + shut(links.get(client)) + }, + error(client) { + shut(links.get(client)) + }, + }, + }) + } +} diff --git a/backend/cli/src/sandbox/sandbox.ts b/backend/cli/src/sandbox/sandbox.ts index ab1a6abe..791a098c 100644 --- a/backend/cli/src/sandbox/sandbox.ts +++ b/backend/cli/src/sandbox/sandbox.ts @@ -1,10 +1,14 @@ import path from "path" import os from "os" import fs from "fs" +import { createHash } from "crypto" import { spawn, spawnSync } from "child_process" import { lazy } from "@/util/lazy" import { Log } from "@/util/log" import { Shell } from "@/shell/shell" +import { Global } from "@/global" +import { Installation } from "@/installation" +import { SHIM_READY_MARKER } from "./egress-shim-marker" const log = Log.create({ service: "sandbox" }) @@ -39,8 +43,43 @@ export namespace Sandbox { writable: string[] /** Exact host files the sandboxed process must not be able to read. */ unreadable?: string[] - /** Whether the sandboxed process may reach the network. */ - network: boolean + /** How the sandboxed process may reach the network. */ + network: "deny" | "allowlist" | "allow" + /** + * Unix socket that is the only egress route on Linux — bubblewrap's + * `--unshare-net` severs everything else. Required when network is + * "allowlist" and the backend is bubblewrap. + */ + egress?: string + /** + * TCP loopback port that is the only egress route on macOS. Seatbelt has + * no network namespace to sever, so there is no socket to bind-mount — + * `seatbeltProfile` instead narrows `network-outbound` to this one port + * (see its doc comment). Required when network is "allowlist" and the + * backend is seatbelt; carried on `Policy` rather than read from ambient + * state so profile generation stays a pure function of its input. + */ + port?: number + /** + * The `Proxy-Authorization` secret the loopback proxy requires on macOS + * — a TCP port, unlike `egress`'s unix socket, carries no filesystem + * permissions of its own, so `plan`/`wrapArgv` embed this in the proxy + * URL (`http://os:@127.0.0.1:`) rather than pointing the + * sandboxed process at an unauthenticated one. Not consumed by + * `seatbeltProfile` itself — the profile only narrows the network layer + * to `port`; the secret is enforced by `Egress.serveProxy` on the other + * end. Set together with `port` or not at all (see `buildPolicy`). + */ + secret?: string + /** + * Read-only paths to bind into the namespace after `--tmpfs /tmp`, so + * they stay reachable regardless of where they happen to live on the + * host — including under `/tmp`, which `--tmpfs /tmp` otherwise masks + * unconditionally, `--ro-bind / /` notwithstanding. Used for the egress + * shim's executable — in dev, the generated launcher and the bundle it + * runs — and the interpreter that launcher execs. + */ + readBind?: string[] } /** A ready-to-spawn argv: `spawn(file, args)` with no shell wrapping. */ @@ -52,11 +91,46 @@ export namespace Sandbox { /** User-facing config knobs (mirrors Config.Sandbox, kept dependency-free). */ export interface Options { enabled?: boolean - network?: "allow" | "deny" + network?: "deny" | "allowlist" | "allow" + /** + * Address of the only egress route, in whatever shape the resolved + * backend needs: a bind-mountable unix socket path for bubblewrap, or + * `":"` for seatbelt (`EgressRuntime.egressFor` is the one + * producer, and it returns one string either way). `buildPolicy` is what + * interprets this per backend, into `Policy.egress` or + * `Policy.port`/`Policy.secret` respectively. + */ + egress?: string allowWrite?: string[] onUnavailable?: "warn" | "error" | "allow" } + /** + * The `enabled`/`network` an `Options` resolves to — the one place that + * answers both questions, so `decide()` and `buildPolicy()` below and + * `EgressRuntime.egressFor()` (which has to precompute the socket that will + * become `options.egress` *before* either of them runs) can't quietly + * disagree on what "unset" means. They did: a missing `enabled` used to + * read as off in `decide()` and on in `egressFor()`, and a missing + * `network` used to read as `"allowlist"` in `buildPolicy()` and not in + * `egressFor()` — each divergence invisible from the five production + * callers, all of which pass an already-fully-resolved policy, but real for + * any caller that doesn't. + * + * A wholly missing `Options` stays off: `enabled` requires an explicit + * `true`, matching `decide()`'s existing contract (see the "no options → + * runs the raw command unchanged" test in sandbox.test.ts) — this module is + * dependency-free and does not itself default a caller into being + * sandboxed. `network` unset defaults to `"allowlist"`, matching + * `buildPolicy()` and `Config.trustedSandbox()`. + */ + export function resolved(options?: Options): { enabled: boolean; network: "deny" | "allowlist" | "allow" } { + return { + enabled: options?.enabled === true, + network: options?.network ?? "allowlist", + } + } + export interface Plan { /** Program to spawn. */ file: string @@ -69,6 +143,13 @@ export namespace Sandbox { backend: Backend /** One-time human-readable note (e.g. sandbox requested but unavailable). */ warning?: string + /** + * Proxy variables the caller must set on the child's env for the loopback + * shim to be used. Present only when the command was actually wrapped + * through the shim (bubblewrap, network "allowlist", a usable egress + * socket) — same condition as `Wrapped.env`. + */ + env?: Record } /** Result of wrapping a raw argv (used by the notebook/R kernels). */ @@ -80,6 +161,12 @@ export namespace Sandbox { sandboxed: boolean backend: Backend warning?: string + /** + * Proxy variables the caller must set on the child's env for the loopback + * shim to be used. Present only when the argv was actually wrapped through + * the shim (bubblewrap, network "allowlist", a usable egress socket). + */ + env?: Record } export class UnavailableError extends Error { @@ -120,9 +207,30 @@ export namespace Sandbox { return "none" }) - /** The sandbox backend usable on this machine right now, or "none". */ - export function backend(): Backend { - return detected() + /** + * The sandbox backend for `platform`, defaulting to this machine's real + * one right now. + * + * For the default (or an explicitly-matching) `platform` this is exactly + * `detected()` — cached, and probed for real (`Bun.which`, + * `probeBubblewrap`) — so every existing zero-arg caller is unaffected. + * + * An explicitly *different* platform is the seam that lets the seatbelt + * code paths in `plan`/`wrapArgv`/`EgressRuntime` be exercised from Linux, + * where no Mac exists to install `sandbox-exec` on or probe for: probing a + * binary that cannot be present on the machine actually running the test + * would just report "none" and defeat the whole point. So a mismatched + * platform skips probing and assumes the backend that platform normally + * has — `sandbox-exec` ships with every macOS install, `bwrap` is what the + * real Linux branch above already probes for — trading "verified installed + * here" for "what plan()/wrapArgv() would compose for that platform", + * which is the property these tests actually need. + */ + export function backend(platform: NodeJS.Platform = process.platform): Backend { + if (platform === process.platform) return detected() + if (platform === "darwin") return "seatbelt" + if (platform === "linux") return "bubblewrap" + return "none" } export function available(): boolean { @@ -174,6 +282,16 @@ export namespace Sandbox { return [...out] } + /** + * True when `p` is exactly `root` or lies inside it. Both arguments must + * already be `dedupe()`-normalized (`path.resolve()`d) — this does exact + * string comparison, the same convention `tooBroadToConfine` uses for the + * same reason. + */ + function isWithin(p: string, root: string): boolean { + return p === root || p.startsWith(root + path.sep) + } + /** * A path too broad to ever be a sandbox writable root: granting write here * would hand back most of the filesystem and defeat containment. Guards @@ -205,12 +323,24 @@ export namespace Sandbox { return roots.includes(p) } - /** Assemble the writable allowlist for a policy, dropping over-broad roots. */ + /** + * Assemble the writable allowlist for a policy, dropping over-broad roots, + * and route `options.egress` to whichever of `Policy.egress`/`Policy.port` + * the resolved `backend` actually consumes. + * + * `backend` is required (not read from ambient state) for the same reason + * `plan`/`wrapArgv` take a `platform` parameter: it is what makes the + * seatbelt branch here exercisable from Linux, and it is also simply + * correct — the caller already resolved it before deciding whether to + * sandbox at all, and re-deriving it here from `process.platform` would + * silently disagree with that decision on an injected platform. + */ function buildPolicy(input: { workspace: string[] extraWritable?: string[] unreadable?: string[] options: Options + backend: Backend }): Policy { const candidates = dedupe([ ...input.workspace, @@ -225,11 +355,51 @@ export namespace Sandbox { } return true }) - return { - writable, - unreadable: dedupe(input.unreadable ?? []).filter((value) => !tooBroadToConfine(value)), - network: (input.options.network ?? "allow") !== "deny", + const unreadable = dedupe(input.unreadable ?? []).filter((value) => !tooBroadToConfine(value)) + const network = resolved(input.options).network + + // Seatbelt's egress route is a bare TCP loopback port plus the + // `Proxy-Authorization` secret that port requires (see seatbeltProfile + // and Options.egress's doc comment), not a filesystem path — + // options.egress here is ":", and none of the path + // machinery below (dedupe's path.resolve, tooBroadToConfine) applies to + // it: resolving "52341:abc" against cwd would silently turn it into an + // absolute path and corrupt it. Port and secret are validated and + // dropped together — a port with no secret would compose a proxy URL + // seatbelt's own proxy always rejects, which is a confusing way to fail + // compared to the same "allowlist requires an egress port" throw a wholly + // missing value already produces (seatbeltProfile is the fail-closed + // enforcement point, the same division of labour bubblewrapArgs already + // has with the path branch immediately below). + if (input.backend === "seatbelt") { + const raw = input.options.egress + const at = raw?.indexOf(":") ?? -1 + const port = at > 0 ? Number(raw!.slice(0, at)) : undefined + const secret = at > 0 ? raw!.slice(at + 1) : undefined + const valid = port !== undefined && Number.isInteger(port) && port > 0 && !!secret + if (raw !== undefined && !valid) { + // Only the port half, never the secret: this is a warning, not an + // error path guarded by anything that stops it reaching a log + // sink — logging the credential half here would defeat the whole + // point of requiring one. + log.warn("refusing to grant sandbox egress access to an invalid port/secret pair", { + port: at > 0 ? raw!.slice(0, at) : raw, + }) + } + return { writable, unreadable, network, ...(valid ? { port, secret } : {}) } + } + + // dedupe() applies the same path.resolve() normalization used for + // writable/unreadable above, so a trailing slash, a double slash, or an + // unresolved ".." can't slip an over-broad path past tooBroadToConfine's + // string checks — the two normalization paths cannot drift apart because + // this is the exact same helper, not a parallel implementation of it. + const [egress] = dedupe(input.options.egress ? [input.options.egress] : []) + const egressOk = egress !== undefined && !tooBroadToConfine(egress) + if (egress !== undefined && !egressOk) { + log.warn("refusing to grant sandbox egress access to an over-broad path", { path: egress }) } + return { writable, unreadable, network, ...(egressOk ? { egress } : {}) } } // ── macOS: Seatbelt (sandbox-exec) ────────────────────────────────────────── @@ -250,9 +420,83 @@ export namespace Sandbox { return [...out] } + /** + * `(deny network*)` then, for "allowlist" only, a narrow re-allow scoped + * to exactly one loopback port — the host-side proxy `EgressRuntime` + * starts for seatbelt (see egress-runtime.ts). Seatbelt has no network + * namespace to sever the way bubblewrap's `--unshare-net` does, so there + * is no unix socket to bind-mount either: the profile itself is the only + * boundary, which is why the deny must always precede the allow (an + * allow with no prior deny is the unfiltered, unrestricted-egress shape + * this function must never produce) and why a missing, non-positive, or + * out-of-range port throws rather than silently falling back to a bare + * deny — the same fail-closed rule `bubblewrapArgs` applies to a missing + * egress socket. Falling back to a plain deny instead of throwing would + * look identical to a user asking for `network: "deny"`, which is not + * what "allowlist" means and is exactly the kind of silent downgrade this + * branch exists to avoid. + * + * Three allow lines, not one: `docs/adr/0002-sandbox-network-policy.md` + * records the reference implementation + * (`anthropic-experimental/sandbox-runtime`) as permitting + * `network-bind`/`network-inbound`/`network-outbound`, all narrowed to the + * proxy's loopback port, filter spelled `tcp` — not the single + * `network-outbound` with `(remote ip ...)` this function emitted before + * Task 7's fix round 1. That original, narrower shape was never measured; + * it was this function's author's own guess at what a TCP `connect()` + * needs, and a Task 7 review flagged the failure mode a wrong guess + * produces here: if seatbelt classifies the implicit local port a + * `connect()` allocates under `network-bind` (this sandboxed process is + * never a listener, so `network-inbound` is included for the same + * uncertainty, not because a genuine inbound connection is expected), a + * profile missing that allow would make "allowlist" unreachable on every + * real Mac — silently, indistinguishable from the network simply being + * down, which is the one direction this task must not ship in. Matching a + * documented, cited-as-working reference is the safer default than an + * independently-derived narrower profile that has never been measured + * against a real `sandbox-exec`. `local`/`remote` for `network-bind`+ + * `network-inbound` vs `network-outbound` follows ordinary SBPL + * convention (bind/inbound describe the local endpoint, outbound the + * remote one) — the ADR does not itself quote a filter spelling for the + * first two, only for `network-outbound`, so that pairing is this + * function's own inference, not a documented fact. See the Task 7 + * report's unverified section: whether seatbelt needs `network-bind`/ + * `network-inbound` at all, and whether `local`/`remote` is the right + * pairing for them, are both open questions only a Mac can answer. + * + * A narrow, accepted consequence of matching that reference shape (Task 7 + * fix round 2): if the host proxy dies while the sandboxed child is still + * alive, `network-bind`+`network-inbound` on that same ephemeral port + * would let the child itself bind or listen there. That is still confined + * to the one port this profile names — not a broader network grant, and + * not a route to any host the child couldn't already reach through the + * (now-dead) proxy — so it is not treated as a defect. It is a real + * property of this design, not a hypothetical one, and belongs next to + * the other open questions above rather than being silently true. + * + * Never asserts enforcement — that a real `sandbox-exec` actually honours + * this text — only the text itself, its ordering, and this function's own + * refusal to emit an unfiltered allow. No Mac exists on this project to + * verify the former; see the Task 7 report for exactly what a Mac owner + * still needs to run. + */ export function seatbeltProfile(policy: Policy): string { const lines = ["(version 1)", "(allow default)"] - if (!policy.network) lines.push("(deny network*)") + // No namespace equivalent on macOS, so "allowlist" cannot be enforced by + // severing the network device the way bubblewrapArgs does. Deny is the + // safe reading of a request for bounded egress; the allow lines below + // narrow that back to exactly the loopback proxy port when one reached + // the policy. + if (policy.network !== "allow") lines.push("(deny network*)") + if (policy.network === "allowlist") { + const port = policy.port + if (typeof port !== "number" || !Number.isInteger(port) || port <= 0 || port > 65535) { + throw new Error("sandbox network 'allowlist' requires an egress port") + } + lines.push(`(allow network-bind (local tcp "localhost:${port}"))`) + lines.push(`(allow network-inbound (local tcp "localhost:${port}"))`) + lines.push(`(allow network-outbound (remote tcp "localhost:${port}"))`) + } const unreadable = withPrivateAliases(dedupe(policy.unreadable ?? [])) if (unreadable.length) { lines.push(`(deny file-read* ${unreadable.map((value) => `(literal "${sbpl(value)}")`).join(" ")})`) @@ -274,11 +518,15 @@ export namespace Sandbox { // Whole fs read-only, a fresh /dev and /proc, and a throwaway writable /tmp; // then re-mount the bits that must be writable on top. const args = ["--ro-bind", "/", "/", "--dev", "/dev", "--proc", "/proc", "--tmpfs", "/tmp"] - for (const p of dedupe(policy.writable)) { - // Skip only the /tmp mount root itself — it is provided as a fresh tmpfs and - // re-binding host /tmp would defeat it. A workspace that lives *under* /tmp - // still needs binding on top of the tmpfs, or its writes vanish. - if (p === "/tmp") continue + // "/tmp" itself is always in policy.writable (tempDirs() adds it unconditionally) + // but is deliberately never actually bound here — the fresh tmpfs above already + // provides it, and re-binding host /tmp would defeat that. So it's excluded from + // this set up front and shared with the readBind exclusion below: a path under + // "/tmp" is writable only where something *else* in policy.writable specifically + // covers it (e.g. a workspace that lives under /tmp), never merely because "/tmp" + // is nominally a writable root. + const boundWritable = dedupe(policy.writable).filter((p) => p !== "/tmp") + for (const p of boundWritable) { // --bind-try: don't abort if the source path doesn't exist. args.push("--bind-try", p, p) } @@ -291,16 +539,81 @@ export namespace Sandbox { if (!fs.existsSync(value)) continue args.push("--ro-bind-try", "/dev/null", value) } - if (!policy.network) args.push("--unshare-net") + if (policy.network !== "allow") args.push("--unshare-net") + if (policy.network === "allowlist") { + if (!policy.egress) throw new Error("sandbox network 'allowlist' requires an egress socket path") + // --unshare-net (above) is what makes this the only route to the network — + // there is no other network device inside the namespace. The --ro-bind here + // only makes the socket path reachable at all, for when it lives under a + // path the sandbox re-mounts (the /tmp tmpfs, a fresh /dev or /proc). + // Read-only, not read-write: the bind shares the host inode, so a + // sandboxed process could otherwise discover the path via + // /proc/self/mountinfo and `chmod 000` it, which persists on the host + // and disables egress for every other kernel/terminal/job sharing this + // one process-lifetime socket. A read-only bind blocks chmod (EROFS) + // while still permitting connect() — verified live: chmod fails with + // "Read-only file system" and a plain client still receives a reply + // over the same bind. + args.push("--ro-bind", policy.egress, policy.egress) + } + // Explicit, not a location choice: --tmpfs /tmp (above) masks the whole + // host /tmp subtree unconditionally, so anything under it — a generated + // launcher or bundle, the interpreter of a portable install — would + // otherwise silently not exist in here. Binding each path back in by its + // own name, after the tmpfs, is what makes it reachable regardless of + // where it actually lives on the host. + // + // Skip anything already inside a writable root — boundWritable specifically + // (not the raw policy.writable list), since that's what's actually mounted + // above; "/tmp" itself is nominally writable but was never bound, so it + // must not short-circuit this check (a launcher living under /tmp — e.g. + // Global.Path.bin redirected there by bun test's isolation — needs its + // own explicit bind same as anywhere else). bwrap mounts are applied in + // argument order and a later mount at (or inside) a path shadows whatever + // an earlier one put there — so a read-only bind here, coming after the + // --bind-try loop above, would silently turn part of an already-writable + // workspace read-only again wherever the two overlap. + // + // Only that one containment direction is guarded (a readBind path inside + // a writable root, not the reverse — a writable root nested inside a + // readBind path). The reason is narrow and specific to today's bind set, + // not a claim about what workspaces look like: writable roots are *not* + // only project roots (SessionFilesystem.processWriteRoots returns + // arbitrary user-granted paths, and options.allowWrite / extraWritable + // are arbitrary too), but every readBind path shimPlan() produces is a + // regular file — two generated artifacts under Global.Path.bin and the + // interpreter — and nothing can be nested inside a file, so the reverse + // direction has no trigger at all rather than an unlikely one. By the + // same token the exclusion changes nothing observable for today's set — + // re-binding a file the sandbox never writes read-only is a no-op — and + // is kept because the moment a directory joins that set, the shadowing + // above becomes reachable again. + // + // Order shadows the unreadable masks above too, which is the other reason + // to keep this set to individual files: a readBind *directory* containing + // an `unreadable` path re-exposes that path's real contents (measured: + // when this list still held the package root, a /dev/null mask on a file + // inside it was silently undone). Every entry here is a generated + // artifact or the interpreter, none of which is ever an unreadable + // candidate, so no mask can be defeated by name. + for (const p of dedupe(policy.readBind ?? [])) { + if (boundWritable.some((root) => isWithin(p, root))) continue + args.push("--ro-bind-try", p, p) + } // --unshare-pid: don't share the host PID namespace, so /proc//root of a // same-uid host process can't be used to write through the read-only bind. args.push("--unshare-pid", "--die-with-parent") return args } - /** Wrap an arbitrary argv under the active backend, or null when unavailable. */ - function specForArgv(argv: string[], policy: Policy): Spec | null { - switch (backend()) { + /** + * Wrap an arbitrary argv under `b`, or null when unavailable. `b` is + * passed in rather than read from `backend()` here — the caller already + * resolved it (via a possibly-injected `platform`), and re-deriving it + * from ambient state would silently disagree with that resolution. + */ + function specForArgv(argv: string[], policy: Policy, b: Backend): Spec | null { + switch (b) { case "seatbelt": return { file: "sandbox-exec", args: ["-p", seatbeltProfile(policy), ...argv] } case "bubblewrap": @@ -310,8 +623,286 @@ export namespace Sandbox { } } + // ── egress shim composition ───────────────────────────────────────────────── + + /** POSIX single-quote escaping: close, insert an escaped quote, reopen. */ + const quote = (value: string) => `'${value.replaceAll("'", `'"'"'`)}'` + + /** + * The sandboxed process needs a proxy at a host:port, but the only route out + * is a unix socket. This backgrounds a loopback bridge inside the namespace, + * waits (bounded) for it to signal readiness, and then execs the real + * command — so the sandbox still holds exactly one long-lived process, and + * the real command doesn't get a proxy env pointing at a port nothing is + * listening on yet. + * + * The wait is a marker-file poll, not a network probe: a POSIX `/bin/sh` + * (dash/busybox, not bash) has no built-in way to test a TCP connection — + * bash's `/dev/tcp` isn't portable here and `nc`/`curl` aren't guaranteed + * present. + * + * *Why the granularity is chosen at run time.* Fractional `sleep` is a + * GNU/BSD coreutils extension, not POSIX, and some busybox builds reject it + * outright (`sleep: invalid interval`) — which, in a loop, would print an + * error line per iteration to the real command's own stderr (this wait runs + * in the foreground, unlike the backgrounded shim) and, worse, skip the + * wait entirely, since a failing `sleep` doesn't slow a loop down at all. + * So the interval is settled once, before the loop, by attempting a single + * fractional `sleep` with its stderr discarded: it either works, and the + * loop polls at 0.02s, or it fails instantly and everything falls back to + * the whole seconds POSIX guarantees. That probe is the only place a + * fractional interval is ever attempted, its diagnostic can't reach the + * command's stderr, and its cost isn't waste — it is time the shim needs + * anyway. + * + * *Why not whole seconds throughout, as this did before.* Measured shim + * readiness (fork/exec, bundle load, listener bound) is ~12ms — the + * 600ms–1.1s in Task 4's report predates bundling the shim entry. At + * whole-second granularity the first check therefore always lost and every + * spawn paid a flat second: n=8, `network: "allowlist"` 1006-1007ms against + * `deny` 3-4ms, on `sh -c true`, i.e. 335x for a command that never touches + * the network. Every `ls` and every `git status` the agent ran paid it. At + * 0.02s the same measurement is 24-25ms. + * + * *Why there is a wall-clock deadline and not just an iteration count.* The + * count alone (150 * 0.02, 3 * 1) only equals 3s where forking `sleep` is + * nearly free. It isn't everywhere: a macOS CI runner measured 17.1s for + * the 150-iteration loop — ~114ms per iteration, of which ~94ms is + * fork/exec of `/bin/sleep`, a 5.7x overshoot of the documented cap. Any + * machine with expensive process creation (a CPU-throttled container, a + * loaded box) drifts the same way, so the loop carries an explicit deadline + * as well. `date +%s` is probed exactly like fractional `sleep` — a build + * without it leaves the deadline unset and the count is the only cap, which + * is the behaviour that shipped before — and `+ 4` rather than `+ 3` + * because `%s` truncates to whole seconds, which would otherwise cut a + * nominal 3s wait as short as 2.0s. + * + * The cap is therefore ~3s in both modes, and 3–4s when the deadline is the + * one that fires. If the shim never signals, the loop still exits at the cap + * and the real command runs anyway — against a closed proxy port, which + * fails fast and visibly (connection refused) rather than hanging forever. + */ + export function shimScript(input: { binary: string; port: number; socket: string; file: string; args: string[] }) { + const shim = [quote(input.binary), "__egress-shim", String(input.port), quote(input.socket)].join(" ") + const real = [quote(input.file), ...input.args.map(quote)].join(" ") + const marker = quote(SHIM_READY_MARKER) + // `s`/`n`/`i`/`d`/`t` are plain shell variables, never exported, and + // `exec` replaces this shell — so none of them reach the real command. + // `${t:-0}` keeps a `date` that starts failing mid-loop from breaking out + // early or printing to the real command's stderr: it degrades to the + // count-only cap, the same direction the probe failing does. + const wait = [ + `s=0.02; n=150`, + `sleep "$s" 2>/dev/null || { s=1; n=3; }`, + `d=$(date +%s 2>/dev/null); case "$d" in ''|*[!0-9]*) d= ;; *) d=$((d + 4)) ;; esac`, + `i=0; while [ ! -f ${marker} ] && [ "$i" -lt "$n" ] && { [ -z "$d" ] || { t=$(date +%s 2>/dev/null); [ "\${t:-0}" -lt "$d" ]; }; }; do sleep "$s"; i=$((i + 1)); done`, + ].join("; ") + return `${shim} >/dev/null 2>&1 & ${wait}; exec ${real}` + } + + /** + * Bubblewrap-only. Loopback port the shim binds inside the sandboxed + * network namespace. Fixed rather than negotiated: `--unshare-net` gives + * every sandboxed process its own private namespace, so this port can + * never collide across sandboxed processes or with anything on the host. + * Exported so `egress-runtime.ts` can hand it back to callers alongside + * the proxy's socket — one source of truth, rather than a second + * module-private 3128 that could drift from this one. + * + * Seatbelt has no such namespace — every process on the machine shares one + * loopback, so a fixed well-known port would collide across concurrent + * sandboxed processes the way it structurally cannot here. Its egress port + * (`Policy.port`) is instead assigned by the OS per proxy instance; see + * `egress-runtime.ts`. + */ + export const SHIM_PORT = 3128 + + /** + * Write one of the dev shim's generated artifacts, idempotently. Callers + * pass a content-addressed name, so a file already at that name already has + * this content and there is nothing to do; comparing anyway costs a few KB + * and repairs a truncated leftover. The write goes to a per-process + * temporary name and is renamed into place, which is atomic within a + * directory — two processes generating the same artifact concurrently write + * byte-identical content, and no third process can observe a half-written + * file at the real name. + * + * A missing file (first run, a fresh worktree, a test tmpdir) is the + * expected case, not a failure, so read errors of any kind just mean "write + * it" and are swallowed separately from the write's own errors. Those fail + * loud with an actionable message rather than a raw EACCES/EROFS out of + * `wrapArgv`: network "allowlist" without a working shim is a security- + * relevant misconfiguration (the caller explicitly asked for bounded + * egress), not something to silently downgrade. + */ + function place(file: string, content: Buffer, mode: number) { + const current = (() => { + try { + return fs.readFileSync(file) + } catch { + return undefined + } + })() + if (current?.equals(content)) return + const temp = `${file}.${process.pid}` + try { + fs.mkdirSync(path.dirname(file), { recursive: true }) + fs.writeFileSync(temp, content, { mode }) + fs.chmodSync(temp, mode) + fs.renameSync(temp, file) + } catch (e) { + fs.rmSync(temp, { force: true }) + throw new Error(`Could not write the dev egress shim to ${file}: ${e instanceof Error ? e.message : String(e)}`) + } + } + + /** + * The single executable `shimScript` execs as the loopback bridge, plus + * every read-only path that must be explicitly bound into the namespace + * (`Policy.readBind`, consumed by `bubblewrapArgs`) for that executable to + * actually be reachable from inside, regardless of where it lives on the + * host. + * + * In a compiled release `process.execPath` IS the openscience binary, so + * `openscience __egress-shim ...` runs directly — one self-contained file, + * no extra artifact ships. + * + * Under `bun run src/index.ts` in development no such file exists: + * `process.execPath` is `bun`, and `bun __egress-shim ...` is not a valid + * bun invocation (it needs an entry script too), while `shimScript`'s + * `binary` is a single shell word once quoted, so a two-word "bun " + * invocation cannot be smuggled through it. So dev *builds* the missing + * file: `bun build` bundles `egress-shim-entry.ts` (a sibling of this file) + * into one self-contained module, and a tiny `sh` launcher execs `bun` + * against it — the same trick `ensureAtlasBinDir` in + * `src/openscience/index.ts` uses to expose a package's JS entry as one + * executable path. The entry is that file and never `src/index.ts`, whose + * graph pulls in `Global` (an unguarded top-level file write) and a live + * models.dev fetch, both of which run before any argv check could skip + * them. A compiled binary has no separate entry to redirect to, so it + * still goes through `index.ts`'s `__egress-shim` check and still + * evaluates that graph — see the Task 4 report for what that leaves + * reachable there; restructuring `index.ts` so nothing runs before the + * check, for both modes, is a materially bigger change than this fix. + * + * *Why bundle instead of running the source.* `--tmpfs /tmp` masks the + * whole host `/tmp` subtree, `--ro-bind / /` notwithstanding, so every path + * the shim touches has to be bound back by name — and the set of paths + * *running a source file* touches is open-ended: the entry, its imports, + * their imports, and for an npm import both the `node_modules` symlink and + * its target, which in this bun workspace is the monorepo-root store, one + * level *above* the package root. Successive revisions of this function + * bound the paths their author thought of (the launcher, then this file's + * directory, then the interpreter and the whole package root) and each + * time missed one — the last of them that npm target, latent only because + * nothing in the graph resolves a package today. A + * bundle ends that class rather than extending the list: at run time bun + * opens the bundle and nothing else, so the bound set is closed by + * construction — launcher, bundle, interpreter — instead of having to keep + * pace with an import graph. + * + * *What that does not cover*, stated precisely because "no future edit can + * break this" is the claim that was false the last four times: `bun build` + * inlines statically resolvable imports only, so a runtime + * `import(expression)` reaching outside the bundle would still resolve + * against an unbound path (a literal `import("./x")` is inlined and fine); + * the bundle executes from `Global.Path.bin`, so anything resolving off its + * own `import.meta.dir`/`url` no longer lands in the source tree; and an + * import the bundler cannot inline fails the build here, loudly, at + * `wrapArgv` time instead of silently inside the sandbox. Import-time side + * effects stay forbidden for the separate reason in + * `egress-shim-entry.ts`'s own comment — bundling relocates that code, it + * does not stop it running. + * + * The fourth one escapes the framing rather than sitting inside it: a + * dependency that loads a *native binding* bundles cleanly and still + * resolves a path at run time. Measured with `bun-pty` as a probe — the + * build succeeds, the JS is inlined, and the output then carries + * `dlopen("….so")`, `import.meta.require` and `process.cwd`, with only + * `bun:ffi` left external. A `dlopen` argument is not an import specifier, + * so neither "bun opens the bundle and nothing else" nor the static test + * that enforces it covers this; such a dependency would need its shared + * library bound by name the way the artifacts are. + * + * *Where the artifacts live does not need to be "safe."* Earlier revisions + * tried to pick a location `--tmpfs /tmp` couldn't mask — `Global.Path.state`, + * then this file's own directory — and both were live-verified broken: + * `Global.Path.*` resolves under `os.tmpdir()` during `bun test` + * (`test/preload.ts` redirects every XDG dir there for isolation) and + * possibly for a real user with `$HOME` under `/tmp`; the repo checkout + * resolves under `/tmp` for a `git worktree add /tmp/...` (this repo's own + * workflow), a CI `mktemp -d` clone, or a container build. There is no + * location immune to both. The fix is the one `bubblewrapArgs` already uses + * for the egress socket: bind the exact path back in, explicitly, after + * `--tmpfs /tmp` — `--ro-bind-try`, not `--bind`, since these are executed + * and read, never written to, from inside. `process.execPath` is bound for + * the same reason and is a structurally separate input, not implied by + * binding the artifacts: a portable bun install, or `$HOME` under `/tmp`, + * puts the interpreter the launcher execs under the tmpfs too. + * + * Bubblewrap-only. Every one of the artifacts this produces exists to get + * a launcher into a severed network namespace and bind it back in by name + * — problems seatbelt does not have, since it has no namespace and the + * sandboxed process dials the loopback proxy directly (see + * `seatbeltProfile`). `plan()`/`wrapArgv()` only ever call this behind a + * `backend === "bubblewrap"` guard, so on darwin — real or + * platform-injected — this function, and everything it writes to + * `Global.Path.bin`, is never reached at all. + */ + const shimPlan = lazy((): { binary: string; bind: string[] } => { + if (!Installation.isLocal()) return { binary: process.execPath, bind: [process.execPath] } + const entry = path.resolve(import.meta.dir, "egress-shim-entry.ts") + const built = Bun.spawnSync([process.execPath, "build", "--target=bun", entry]) + if (!built.success) { + throw new Error(`Could not bundle the dev egress shim from ${entry}: ${built.stderr.toString().trim()}`) + } + // Content-addressed, not fixed names: a rebuilt bundle is a different + // file rather than an overwrite of the one another process may be + // executing, and a name that already exists already holds this exact + // content, by construction. The launcher's own digest covers the bundle's + // path, so a new bundle always produces a new launcher pointing at it — + // a stale pair cannot form. What makes the bytes differ is (source, bun, + // cwd): bun build writes cwd-relative module banners into the output, so + // the same source built from `backend/cli` and from anywhere else are + // different files. Only correctness is claimed here, not thrift — cwd is + // the dimension that varies per invocation, so it is also the one that + // drives how many of these accumulate. + const stamp = (value: Buffer) => createHash("sha256").update(value).digest("hex").slice(0, 16) + // .mjs, not .js: nothing should make bun's module-type detection for this + // file depend on a package.json above Global.Path.bin. + const bundle = path.join(Global.Path.bin, `egress-shim-dev-${stamp(built.stdout)}.mjs`) + const script = Buffer.from(`#!/bin/sh\nexec ${quote(process.execPath)} ${quote(bundle)} "$@"\n`) + const launcher = path.join(Global.Path.bin, `egress-shim-dev-${stamp(script)}.sh`) + place(bundle, built.stdout, 0o644) + place(launcher, script, 0o755) + return { binary: launcher, bind: [launcher, bundle, process.execPath] } + }) + // ── planning (consumed by the bash tool and the kernels) ──────────────────── + /** + * The `HTTP_PROXY`-shaped URL the sandboxed process should use, or + * `undefined` when nothing composed a route to the proxy at all. + * Bubblewrap: the shim's fixed `SHIM_PORT`, unauthenticated — the + * bind-mounted unix socket underneath it is already the sandboxed + * process's only route out, so the loopback hop inside the namespace + * needs no credential of its own. Seatbelt: no shim, so the sandboxed + * process dials `policy.port` directly, and — because that loopback port, + * unlike a unix socket, carries no filesystem permissions of its own — + * the URL embeds `policy.secret` as userinfo + * (`http://os:@127.0.0.1:`), which pip, curl and requests + * all parse into a `Proxy-Authorization` header. Both must be present, not + * just `port`: `buildPolicy` only ever sets them together, so a `port` + * with no `secret` means something upstream broke that invariant, and + * this fails closed to "no proxy configured" rather than emitting a URL + * seatbelt's own proxy would just reject with 407 anyway. + */ + function proxyUrl(shim: string | undefined, b: Backend, policy: Policy): string | undefined { + if (shim) return `http://127.0.0.1:${SHIM_PORT}` + if (b !== "seatbelt" || policy.network !== "allowlist" || !policy.port || !policy.secret) return undefined + return `http://os:${policy.secret}@127.0.0.1:${policy.port}` + } + // Warn only once per process so every command doesn't repeat the same notice. const warned = { unavailable: false } @@ -320,31 +911,50 @@ export namespace Sandbox { } /** - * Resolve which backend a command should use given the config. Returns - * backend "none" (run unsandboxed) with an optional one-time warning, or the + * Resolve which backend a command should use given the config and + * `platform` (default the real one — see `backend()`). Returns backend + * "none" (run unsandboxed) with an optional one-time warning, or the * active backend. Throws UnavailableError only when `onUnavailable: "error"` * and no backend exists. */ - function decide(options?: Options): { backend: Backend; warning?: string } { - if (options?.enabled !== true) return { backend: "none" } - const b = backend() + function decide( + options: Options | undefined, + platform: NodeJS.Platform = process.platform, + ): { backend: Backend; warning?: string } { + if (!resolved(options).enabled) return { backend: "none" } + const b = backend(platform) if (b !== "none") return { backend: b } - const mode = options.onUnavailable ?? "warn" + const mode = options?.onUnavailable ?? "warn" if (mode === "error") throw new UnavailableError(unavailableMessage()) const warning = mode === "warn" && !warned.unavailable ? unavailableMessage() : undefined if (warning) { warned.unavailable = true - log.warn("sandbox enabled but unavailable", { platform: process.platform }) + log.warn("sandbox enabled but unavailable", { platform }) } return { backend: "none", warning } } /** * Decide how to run a shell command given the sandbox config and the - * workspace. Never throws unless `onUnavailable: "error"` and no backend - * exists. The `cwd` is *not* granted write access unless it lies within the - * workspace — an approved external working directory is a permission decision, - * not a reason to widen the write boundary to the escape target. + * workspace. Throws only in two cases: `onUnavailable: "error"` with no + * backend available, or `network: "allowlist"` with no `egress` socket path + * (directly, or because the supplied path was rejected as over-broad). The + * `cwd` is *not* granted write access unless it lies within the workspace — + * an approved external working directory is a permission decision, not a + * reason to widen the write boundary to the escape target. + * + * Composes the same loopback shim `wrapArgv` does, under the same + * condition (bubblewrap, network "allowlist", a usable egress socket) — + * `pip`/`curl`/`uv` run through here, not `wrapArgv`, so this is the path + * the feature's motivating case actually needs. `shimScript` already + * treats its `file`/`args` as an arbitrary argv to `exec`, so a shell + * invocation composes by feeding it `input.shell`/`["-c", input.command]` + * exactly as the no-shim branch below already passes to `specForArgv` — + * one shape, not a second implementation of "wrap a shell command". + * + * `platform` defaults to the real one — the only reason to pass a + * different value is to exercise the seatbelt/darwin branch from a + * machine that isn't one; see `backend()`. */ export function plan(input: { command: string @@ -353,15 +963,44 @@ export namespace Sandbox { /** Workspace roots (Instance.directory + worktree) that stay writable. */ workspace: string[] options?: Options + platform?: NodeJS.Platform }): Plan { - const { backend: b, warning } = decide(input.options) + const platform = input.platform ?? process.platform + const { backend: b, warning } = decide(input.options, platform) if (b === "none") { return { file: input.command, useShell: input.shell, sandboxed: false, backend: "none", warning } } - const policy = buildPolicy({ workspace: input.workspace, options: input.options! }) - const s = specForArgv([input.shell, "-c", input.command], policy)! + const policy = buildPolicy({ workspace: input.workspace, options: input.options!, backend: b }) + // Bubblewrap's loopback shim bridges a bind-mounted unix socket that only + // exists inside its own network namespace. Seatbelt has no namespace, so + // there is nothing to bridge and no shim to compose — the sandboxed + // process instead dials the loopback proxy port seatbeltProfile allowed + // directly, which is why this guard stays bubblewrap-only rather than + // "any backend with allowlist + an egress value". + const shimmed = b === "bubblewrap" && policy.network === "allowlist" && policy.egress ? shimPlan() : undefined + const shim = shimmed + ? shimScript({ + binary: shimmed.binary, + port: SHIM_PORT, + socket: policy.egress!, + file: input.shell, + args: ["-c", input.command], + }) + : undefined + const argv = shim ? ["/bin/sh", "-c", shim] : [input.shell, "-c", input.command] + const s = specForArgv(argv, shimmed ? { ...policy, readBind: shimmed.bind } : policy, b)! log.info("sandboxing command", { backend: b, network: policy.network, writable: policy.writable.length }) - return { file: s.file, args: s.args, useShell: false, sandboxed: true, backend: b, warning } + const proxy = proxyUrl(shim, b, policy) + const env = proxy ? { HTTP_PROXY: proxy, HTTPS_PROXY: proxy, http_proxy: proxy, https_proxy: proxy } : undefined + return { + file: s.file, + args: s.args, + useShell: false, + sandboxed: true, + backend: b, + warning, + ...(env ? { env } : {}), + } } /** @@ -369,6 +1008,10 @@ export namespace Sandbox { * which spawn an interpreter directly. When the sandbox is off or unavailable * the original `file`/`args` are returned unchanged, so callers can spawn the * result verbatim. + * + * `platform` defaults to the real one — the only reason to pass a + * different value is to exercise the seatbelt/darwin branch from a + * machine that isn't one; see `backend()`. */ export function wrapArgv(input: { file: string @@ -380,8 +1023,10 @@ export namespace Sandbox { /** Exact host credential files to mask from the process. */ unreadable?: string[] options?: Options + platform?: NodeJS.Platform }): Wrapped { - const { backend: b, warning } = decide(input.options) + const platform = input.platform ?? process.platform + const { backend: b, warning } = decide(input.options, platform) if (b === "none") { return { file: input.file, args: input.args, sandboxed: false, backend: "none", warning } } @@ -390,10 +1035,23 @@ export namespace Sandbox { extraWritable: input.extraWritable, unreadable: input.unreadable, options: input.options!, + backend: b, }) - const s = specForArgv([input.file, ...input.args], policy)! + // Only bubblewrap's --unshare-net + --bind gives the shim anything to + // bridge: seatbelt has no namespace, so there is nothing to bridge and no + // shim to compose — the sandboxed process instead dials the loopback + // proxy port seatbeltProfile allowed directly (see plan()'s identical + // guard for the shell-command path). + const plan = b === "bubblewrap" && policy.network === "allowlist" && policy.egress ? shimPlan() : undefined + const shim = plan + ? shimScript({ binary: plan.binary, port: SHIM_PORT, socket: policy.egress!, file: input.file, args: input.args }) + : undefined + const argv = shim ? ["/bin/sh", "-c", shim] : [input.file, ...input.args] + const s = specForArgv(argv, plan ? { ...policy, readBind: plan.bind } : policy, b)! log.info("sandboxing process", { backend: b, network: policy.network, writable: policy.writable.length }) - return { file: s.file, args: s.args, sandboxed: true, backend: b, warning } + const proxy = proxyUrl(shim, b, policy) + const env = proxy ? { HTTP_PROXY: proxy, HTTPS_PROXY: proxy, http_proxy: proxy, https_proxy: proxy } : undefined + return { file: s.file, args: s.args, sandboxed: true, backend: b, warning, ...(env ? { env } : {}) } } // ── self-test (proves the boundary actually holds on this machine) ────────── diff --git a/backend/cli/src/science/kernel/types.ts b/backend/cli/src/science/kernel/types.ts index c69ac480..ec9c3891 100644 --- a/backend/cli/src/science/kernel/types.ts +++ b/backend/cli/src/science/kernel/types.ts @@ -32,7 +32,7 @@ export const KernelEnvironment = z.object({ requested: z.boolean(), enforced: z.boolean(), backend: z.enum(["seatbelt", "bubblewrap", "none"]), - network: z.enum(["allow", "deny"]), + network: z.enum(["deny", "allowlist", "allow"]), platform: z.string(), available: z.boolean(), tool: z.string().optional(), diff --git a/backend/cli/src/server/routes/settings/sandbox.ts b/backend/cli/src/server/routes/settings/sandbox.ts index 83b175d7..1bd356c8 100644 --- a/backend/cli/src/server/routes/settings/sandbox.ts +++ b/backend/cli/src/server/routes/settings/sandbox.ts @@ -10,7 +10,8 @@ const log = Log.create({ service: "settings-sandbox" }) const PatchSchema = z.object({ enabled: z.boolean().optional(), - network: z.enum(["allow", "deny"]).optional(), + network: z.enum(["deny", "allowlist", "allow"]).optional(), + allowHosts: z.array(z.string()).optional(), allowWrite: z.array(z.string()).optional(), onUnavailable: z.enum(["warn", "error", "allow"]).optional(), }) diff --git a/backend/cli/src/tool/bash.ts b/backend/cli/src/tool/bash.ts index b91dd33f..314d01ae 100644 --- a/backend/cli/src/tool/bash.ts +++ b/backend/cli/src/tool/bash.ts @@ -17,6 +17,7 @@ import { BashArity } from "@/permission/arity" import { Truncate } from "./truncation" import { OpenScience } from "@/openscience" import { Sandbox } from "@/sandbox/sandbox" +import { EgressRuntime } from "@/sandbox/egress-runtime" import { SessionFilesystem } from "@/session/filesystem" import { Filesystem } from "@/util/filesystem" import { Provenance } from "@/science/provenance/store" @@ -277,18 +278,19 @@ export const BashTool = Tool.define("bash", async () => { // provider keys (auth.json + shell env), not just synced managed ones. await OpenScience.refreshByokSecrets(process.env).catch(() => {}) - const env = await OpenScience.subprocessEnv(process.env) // Wrap the command in the authority's effective OS-sandbox policy. The // permission checks above decide *whether* to run; this decides *with what // authority*. An explicit trusted machine-level opt-out returns the raw // command unchanged. + const egress = await EgressRuntime.egressFor(authority.sandbox) const sandbox = Sandbox.plan({ command: params.command, shell, cwd, workspace: writable, - options: authority.sandbox, + options: { ...authority.sandbox, egress }, }) + const env = { ...(await OpenScience.subprocessEnv(process.env)), ...(sandbox.env ?? {}) } const started = Date.now() const proc = sandbox.sandboxed diff --git a/backend/cli/src/tool/biology/notebook.ts b/backend/cli/src/tool/biology/notebook.ts index 94bd8ec8..66903234 100644 --- a/backend/cli/src/tool/biology/notebook.ts +++ b/backend/cli/src/tool/biology/notebook.ts @@ -8,6 +8,7 @@ import { Shell } from "@/shell/shell" import { Instance } from "@/project/instance" import { OpenScience } from "@/openscience" import { Sandbox } from "@/sandbox/sandbox" +import { EgressRuntime } from "@/sandbox/egress-runtime" import { ExecutionAuthority } from "@/project/execution" const KERNEL_SCRIPT = ` @@ -186,19 +187,21 @@ async function getKernel(sessionID: string): Promise { const pythonBin = await findPython() // Confine the kernel to the workspace when the execution sandbox is on: it runs // arbitrary agent-authored code — the same threat model as the bash tool. + const egress = await EgressRuntime.egressFor(authority.sandbox) const sandboxed = Sandbox.wrapArgv({ file: pythonBin, args: ["-u", scriptPath], workspace: authority.writable, extraWritable: [scriptPath, configPath, cachePath], unreadable: OpenScience.kernelSensitivePaths(), - options: authority.sandbox, + options: { ...authority.sandbox, egress }, }) const proc = spawn(sandboxed.file, sandboxed.args, { cwd: authority.workspace, env: { ...OpenScience.kernelEnv(process.env), ...OpenScience.pythonThreadCapEnv(process.env), + ...(sandboxed.env ?? {}), ATLAS_CLI_CONFIG_PATH: configPath, MPLCONFIGDIR: path.join(cachePath, "matplotlib"), XDG_CACHE_HOME: path.join(cachePath, "xdg"), diff --git a/backend/cli/src/tool/notebook.ts b/backend/cli/src/tool/notebook.ts index fd8f63de..bb597f8d 100644 --- a/backend/cli/src/tool/notebook.ts +++ b/backend/cli/src/tool/notebook.ts @@ -10,6 +10,7 @@ import { OpenScience } from "@/openscience" import { Config } from "@/config/config" import { SessionFilesystem } from "@/session/filesystem" import { Sandbox } from "@/sandbox/sandbox" +import { EgressRuntime } from "@/sandbox/egress-runtime" import { KernelQueue } from "@/science/kernel/queue" import { KernelProcessIdentity } from "@/science/kernel/process" import { KernelRuntime } from "@/science/kernel/registry" @@ -291,13 +292,14 @@ class PythonKernel implements Kernel { // notebook runs arbitrary agent-authored code — the same threat model as the // bash tool — so it must not be able to escape the boundary bash respects. const policy = await Config.trustedSandbox() + const egress = await EgressRuntime.egressFor(policy) const sandboxed = Sandbox.wrapArgv({ file: bin, args: ["-u", scriptPath], workspace, extraWritable: [scriptPath, configPath, cachePath], unreadable: OpenScience.kernelSensitivePaths(), - options: policy, + options: { ...policy, egress }, }) const cwd = opts?.cwd ?? (opts?.sessionID ? await SessionFilesystem.workspace(opts.sessionID) : Instance.directory) this.environment = { @@ -317,6 +319,7 @@ class PythonKernel implements Kernel { env: { ...OpenScience.kernelEnv(process.env), ...OpenScience.pythonThreadCapEnv(process.env), + ...(sandboxed.env ?? {}), ...(opts?.env ?? {}), ATLAS_CLI_CONFIG_PATH: configPath, MPLCONFIGDIR: path.join(cachePath, "matplotlib"), diff --git a/backend/cli/src/tool/rkernel.ts b/backend/cli/src/tool/rkernel.ts index bf845640..1186a882 100644 --- a/backend/cli/src/tool/rkernel.ts +++ b/backend/cli/src/tool/rkernel.ts @@ -10,6 +10,7 @@ import { OpenScience } from "@/openscience" import { Config } from "@/config/config" import { SessionFilesystem } from "@/session/filesystem" import { Sandbox } from "@/sandbox/sandbox" +import { EgressRuntime } from "@/sandbox/egress-runtime" import { KernelQueue } from "@/science/kernel/queue" import { KernelProcessIdentity } from "@/science/kernel/process" import { KernelRuntime } from "@/science/kernel/registry" @@ -258,13 +259,14 @@ class RKernel implements Kernel { // kernel runs arbitrary agent-authored code — the same threat model as the // bash tool — so it must respect the same boundary. const policy = await Config.trustedSandbox() + const egress = await EgressRuntime.egressFor(policy) const sandboxed = Sandbox.wrapArgv({ file: bin, args: ["--vanilla", scriptPath], workspace, extraWritable: [scriptPath, configPath], unreadable: OpenScience.kernelSensitivePaths(), - options: policy, + options: { ...policy, egress }, }) const cwd = opts?.cwd ?? (opts?.sessionID ? await SessionFilesystem.workspace(opts.sessionID) : Instance.directory) this.environment = { @@ -283,6 +285,7 @@ class RKernel implements Kernel { cwd, env: { ...OpenScience.kernelEnv(process.env), + ...(sandboxed.env ?? {}), ...(opts?.env ?? {}), ATLAS_CLI_CONFIG_PATH: configPath, }, diff --git a/backend/cli/test/project/execution-authority.test.ts b/backend/cli/test/project/execution-authority.test.ts index 54f17396..bb29e53a 100644 --- a/backend/cli/test/project/execution-authority.test.ts +++ b/backend/cli/test/project/execution-authority.test.ts @@ -79,7 +79,7 @@ test("read-only project authority rejects terminal, shell, and kernel before pro trustRevision: 2, sandbox: { enabled: true, - network: "deny", + network: "allowlist", onUnavailable: "error", }, }) @@ -190,7 +190,7 @@ test("trusted terminal derives its process contract from the owning session", as sandbox: { enabled: true, enforced: true, - network: "deny", + network: "allowlist", }, }, status: "running", diff --git a/backend/cli/test/sandbox/egress-live-seatbelt.test.ts b/backend/cli/test/sandbox/egress-live-seatbelt.test.ts new file mode 100644 index 00000000..36914c5d --- /dev/null +++ b/backend/cli/test/sandbox/egress-live-seatbelt.test.ts @@ -0,0 +1,246 @@ +import { describe, expect, test } from "bun:test" +import crypto from "crypto" +import { Egress } from "../../src/sandbox/egress" +import { Sandbox } from "../../src/sandbox/sandbox" +import { tmpdir } from "../fixture/fixture" + +/** + * The seatbelt counterpart to egress-live.test.ts: a real `sandbox-exec`, a + * real TCP-loopback `Egress.serveProxy`, and a real remote host, wired + * together exactly the way `Sandbox.plan` composes them in production. Task + * 7 (see `.superpowers/sdd/2026-08-09-sandbox-network-policy/task-7-report.md`) + * built the seatbelt profile and the authenticated loopback proxy entirely + * from Linux, with `platform: "darwin"` injected on every assertion — nobody + * on the project has a Mac, so none of it had ever reached a real + * `sandbox-exec`. This file is what runs when a Mac finally does. + * + * `platform` is deliberately never passed to `Sandbox.plan` below. Every + * seatbelt-specific test elsewhere in `test/sandbox/` injects `"darwin"` to + * exercise the branch from Linux; this file's whole reason to exist is to + * let `Sandbox.backend()`/`decide()` resolve for real, from a real + * `Bun.which("sandbox-exec")` probe, on a machine where that probe can + * actually succeed. + * + * Two open questions from the Task 7 report are what a red run here would + * mean, and this file is written so a reader can tell which: + * + * 1. Whether `network-bind`/`network-inbound` are needed at all for the + * implicit local bind a TCP `connect()` performs, or whether + * `(deny network*)` blocks it regardless of the three narrow allows — + * in which case the sandboxed process never reaches the proxy at all. + * A failure here shows up as the FIRST test below failing to reach + * "200" for the allowlisted host (the process can't dial the proxy + * port in the first place), typically with curl reporting a connection + * error in `stderr` rather than any HTTP status. + * 2. Whether the filter spelling seatbeltProfile emits — `(remote tcp + * "localhost:PORT")` — is what a real `sandbox-exec` expects, versus + * the `(remote ip ...)` this function used before Task 7's fix round 1. + * A failure here shows up the same way as (1) — a wrong filter keyword + * either fails `sandbox-exec -p` outright (a syntax/parse error in + * `stderr`, non-zero exit before the script's own commands ever run) + * or silently fails to match any traffic, which reads identically to + * (1) from this test's vantage point. Either way "the proxy is + * unreachable" is the shared symptom; telling the two apart needs a + * human reading `stderr` for a `sandbox-exec` parse error specifically + * — present means (2), absent means (1) or a genuine enforcement gap. + * + * Everything downstream of that — denied host refused, direct egress with + * the proxy env unset failing, DNS resolving nothing inside the sandbox, + * and volume surviving byte-for-byte through the seatbelt-side proxy path — + * is new coverage of its own kind, not a restatement of the Linux file: + * `Egress.serveProxy`'s TCP/authenticated branch (used only by seatbelt) has + * never taken a live client through a real OS network boundary before this. + * + * Gated on `Sandbox.backend() === "seatbelt"`, real and non-injected — this + * skips on Linux (where it stays exercised by the darwin-injected unit tests + * elsewhere in this directory) and runs, unskipped, on the one machine that + * can: a broken profile on that machine must fail this test, not quietly + * skip it. + */ + +const curl = Bun.which("curl") +const python = Bun.which("python3") + +/** + * A direct, un-sandboxed HEAD against a host the shipped default allowlist + * already permits, run once at collection time — same purpose as + * egress-live.test.ts's `reachable()`: without it, a macOS runner with no + * route to the internet would see the checks below fail exactly the way a + * broken profile would, which is not the defect this file exists to catch. + */ +function reachable() { + if (!curl) return false + const probe = Bun.spawnSync([ + curl, + "-sS", + "-I", + "-m", + "5", + "-o", + "/dev/null", + "-w", + "%{http_code}", + "https://pypi.org/simple/", + ]) + return probe.exitCode === 0 && probe.stdout.toString().trim() === "200" +} + +const skip = Sandbox.backend() !== "seatbelt" || !curl || !python || !reachable() + +/** A real host-side allowlist proxy on an OS-assigned loopback port, fed the + * real shipped `DEFAULT_RULES` — same shape as egress-live.test.ts's + * `proxy()`, but the seatbelt/TCP overload of `Egress.serveProxy` (a fresh + * `crypto.randomUUID()` secret per call, matching what `EgressRuntime`'s + * `startSeatbelt` does for a real proxy start) rather than a unix socket. */ +function proxy(rules: Egress.Rule[]) { + const secret = crypto.randomUUID() + const server = Egress.serveProxy({ hostname: "127.0.0.1", port: 0, secret, rules }) + return { + port: server.port, + secret, + stop: () => server.stop(true), + } +} + +/** Run `script` the way a sandboxed shell command actually runs in + * production — through `Sandbox.plan`, a real `sandbox-exec`-wrapped shell, + * and the real per-connection proxy allowlist check. `egress` is + * `":"`, the exact shape `EgressRuntime.egressFor` produces + * for seatbelt; `buildPolicy` splits it back into `Policy.port`/ + * `Policy.secret`. No `platform` override — see the file doc comment. */ +async function run(script: string, work: string, egress: string) { + const spec = Sandbox.plan({ + command: script, + shell: "/bin/sh", + cwd: work, + workspace: [work], + options: { enabled: true, network: "allowlist", egress }, + }) + const proc = Bun.spawn([spec.file, ...(spec.args ?? [])], { + cwd: work, + env: { ...process.env, ...spec.env }, + stdout: "pipe", + stderr: "pipe", + }) + const [stdout, stderr] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]) + await proc.exited + return { stdout, stderr } +} + +describe.skipIf(skip)("egress: a real seatbelt sandbox, a real proxy, a real remote host", () => { + test("an allowlisted host reaches 200, a denied one does not, and neither a direct connection nor DNS works outside the shim", async () => { + await using work = await tmpdir() + const host = proxy(Egress.DEFAULT_RULES) + try { + const egress = `${host.port}:${host.secret}` + const script = [ + `pypi=$(curl -sS -I -m 30 -o /dev/null -w '%{http_code}' https://pypi.org/simple/)`, + `eutils=$(curl -sS -m 30 -o /dev/null -w '%{http_code}' 'https://eutils.ncbi.nlm.nih.gov/entrez/eutils/einfo.fcgi')`, + `example=$(curl -sS -I -m 15 -o /dev/null -w '%{http_code}' https://example.com/)`, + // Same load-bearing pair as egress-live.test.ts, and the same + // reasoning: `unset` inside a subshell strips every proxy var for + // this one curl only, so a 200 here would mean the loopback port is + // a convenience rather than the only way out. + `direct=$(unset HTTP_PROXY HTTPS_PROXY http_proxy https_proxy ALL_PROXY all_proxy NO_PROXY no_proxy; curl -sS -I -m 10 -o /dev/null -w '%{http_code}' https://pypi.org/)`, + // No `getent` on macOS. `python3 -c` resolves the same host and this + // captures only its exit status: 0 if `gethostbyname` returned an + // address (DNS worked, which it must not, inside the sandbox), a + // Python traceback's exit code (1, unhandled `socket.gaierror`) + // otherwise. Output is discarded either way — only the exit code + // is load-bearing, so nothing here depends on Python's traceback + // format. + `dns=$(python3 -c "import socket; socket.gethostbyname('pypi.org')" >/dev/null 2>&1; printf '%s' "$?")`, + `printf 'PYPI=%s\\nEUTILS=%s\\nEXAMPLE=%s\\nDIRECT=%s\\nDNS=%s\\n' "$pypi" "$eutils" "$example" "$direct" "$dns"`, + ].join("\n") + + const { stdout, stderr } = await run(script, work.path, egress) + const field = (name: string) => stdout.match(new RegExp(`^${name}=(.*)$`, "m"))?.[1] + const detail = `stdout=${stdout} stderr=${stderr}` + + // Guarded with a shape check first on every field, matching + // egress-live.test.ts's convention: without it, a parsing regression + // that made `field()` return undefined would still satisfy + // `not.toBe(...)` and pass with nothing actually verified. + expect(field("PYPI"), detail).toBe("200") + expect(field("EUTILS"), detail).toBe("200") + expect(field("EXAMPLE"), detail).toMatch(/^\d{3}$/) + expect(field("EXAMPLE"), detail).not.toBe("200") + expect(field("DIRECT"), detail).toMatch(/^\d{3}$/) + expect(field("DIRECT"), detail).not.toBe("200") + expect(field("DNS"), detail).toMatch(/^\d+$/) + expect(field("DNS"), detail).not.toBe("0") + } finally { + host.stop() + } + }, 120_000) + + // Same file, same size, same hash as egress-live.test.ts's wheel test — + // deliberately not re-derived, so a divergence between the two backends' + // handling of the exact same bytes would show up as one green and one red + // rather than two different payloads that happen to both pass. `pump` + // (egress.ts) is shared code between the unix-socket and TCP/loopback + // listeners; this is the first time its TCP branch has moved anything + // this large through a real OS network boundary rather than a stubbed one. + const WHEEL_URL = + "https://files.pythonhosted.org/packages/3a/d0/edc009c27b406c4f9cbc79274d6e46d634d139075492ad055e3d68445925/numpy-1.26.4-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl" + const WHEEL_SIZE = 18_252_005 + const WHEEL_SHA256 = "666dbfb6ec68962c033a450943ded891bed2d54e6755e35e5835d63f4f6931d5" + + test("megabytes through a real proxy and a real host arrive byte-for-byte", async () => { + await using work = await tmpdir() + const host = proxy(Egress.DEFAULT_RULES) + try { + const egress = `${host.port}:${host.secret}` + const out = `${work.path}/numpy.whl` + // Same budget reasoning as egress-live.test.ts: curl's own timeout + // stays comfortably inside the outer 120_000ms so a genuinely slow + // download fails with a legible `curl: (28)` rather than racing the + // outer bun:test timeout and losing the diagnostic. + const script = `curl -sS -m 90 -o ${JSON.stringify(out)} -w '%{http_code} %{size_download}' ${JSON.stringify(WHEEL_URL)}` + + const { stdout, stderr } = await run(script, work.path, egress) + expect(stdout.trim(), `stderr=${stderr}`).toBe(`200 ${WHEEL_SIZE}`) + + const bytes = await Bun.file(out).arrayBuffer() + expect(bytes.byteLength).toBe(WHEEL_SIZE) + expect(new Bun.CryptoHasher("sha256").update(bytes).digest("hex")).toBe(WHEEL_SHA256) + } finally { + host.stop() + } + }, 120_000) + + // The seatbelt counterpart to egress-live.test.ts's pip test, and the + // macOS half of the merge gate: `pip install` under `network: "allowlist"` + // must work on every platform we ship, not just the one it was developed + // on. Deliberately the same package and the same flags as the Linux file, + // for the same reason the wheel test shares its URL and hash — a + // divergence between the two backends should surface as one green and one + // red, not as two different scenarios that happen to both pass. + // + // One thing this covers that the Linux file cannot: pip authenticating to + // the proxy. Seatbelt's loopback port is reachable by every process on the + // machine, so `Sandbox.plan` puts a per-start secret in the proxy URL + // (`http://os:@127.0.0.1:`) and the proxy 407s anything + // without it. curl and urllib are already covered; pip reaches the proxy + // through urllib3, whose own `Proxy-Authorization` handling on CONNECT is + // exercised here for the first time. + test("pip install reaches pypi through the authenticated proxy and the installed package imports", async () => { + await using work = await tmpdir() + const host = proxy(Egress.DEFAULT_RULES) + try { + const egress = `${host.port}:${host.secret}` + const venv = `${work.path}/venv` + const script = [ + `set -e`, + `python3 -m venv ${JSON.stringify(venv)}`, + `${JSON.stringify(`${venv}/bin/pip`)} install --only-binary :all: --disable-pip-version-check -q tqdm`, + `printf 'TQDM=%s\\n' "$(${JSON.stringify(`${venv}/bin/python`)} -c 'import tqdm; print(tqdm.__version__)')"`, + ].join("\n") + + const { stdout, stderr } = await run(script, work.path, egress) + expect(stdout.match(/^TQDM=(.+)$/m)?.[1], `stdout=${stdout} stderr=${stderr}`).toMatch(/^\d+\.\d+/) + } finally { + host.stop() + } + }, 240_000) +}) diff --git a/backend/cli/test/sandbox/egress-live.test.ts b/backend/cli/test/sandbox/egress-live.test.ts new file mode 100644 index 00000000..a8aabf67 --- /dev/null +++ b/backend/cli/test/sandbox/egress-live.test.ts @@ -0,0 +1,228 @@ +import { describe, expect, test } from "bun:test" +import fs from "fs" +import os from "os" +import path from "path" +import { Egress } from "../../src/sandbox/egress" +import { Sandbox } from "../../src/sandbox/sandbox" +import { tmpdir } from "../fixture/fixture" + +/** + * The composition nobody had committed: a real `bwrap --unshare-net`, a + * real `Egress.serveProxy` on the host, and a real remote host, wired + * together exactly the way `Sandbox.plan`/`wrapArgv` wire them in + * production. Every live test elsewhere in `test/sandbox/` stops short of + * this — `sandbox.test.ts`'s shim tests terminate at a stub `Bun.listen` + * standing in for the proxy, and `egress.test.ts`'s volume tests dial the + * proxy directly, never through a sandboxed process. Neither proves a + * sandboxed command can actually reach pypi.org, and that gap is exactly + * why the proxy shipped silently truncating every transfer above a few KB + * for four review rounds: every test that pushed a few bytes through + * passed. + * + * Two things this file asserts and nothing else does: + * - the socket is the ONLY route out (a denied host gets refused by the + * proxy, AND a direct connection with the proxy variables unset fails, + * AND DNS itself resolves nothing inside the namespace) — without that + * trio this would prove the proxy works, not that it is the only way + * out, which is the actual security claim + * - real volume survives byte-for-byte, not just "curl exited 0" + */ + +const curl = Bun.which("curl") +const python = Bun.which("python3") + +/** + * A direct, un-sandboxed HEAD against a host the shipped default allowlist + * already permits, run once at collection time. Without this, a machine + * with no route to the internet would see the PYPI/EUTILS checks below come + * back non-200 — which reads exactly like the policy defect this file + * exists to catch, when it is really just an unplugged network. Synchronous + * because bun:test needs the skip condition before any test body runs. + */ +function reachable() { + if (!curl) return false + const probe = Bun.spawnSync([ + curl, + "-sS", + "-I", + "-m", + "5", + "-o", + "/dev/null", + "-w", + "%{http_code}", + "https://pypi.org/simple/", + ]) + return probe.exitCode === 0 && probe.stdout.toString().trim() === "200" +} + +// bubblewrap, curl, getent and timeout are all load-bearing below — getent +// proves DNS resolves nothing inside the namespace, timeout bounds it in +// case that ever changes. Absent any of them, or with no network, this +// skips rather than fails: a red run here should mean the egress boundary +// broke, not that the host running the suite is a Mac or is offline. +const skip = + Sandbox.backend() !== "bubblewrap" || !curl || !Bun.which("getent") || !Bun.which("timeout") || !reachable() + +/** A real host-side allowlist proxy on a scratch unix socket — same shape + * as egress.test.ts's `proxy()`, but fed the real shipped `DEFAULT_RULES` + * rather than a synthetic rule, so pypi.org and the NCBI eutils subdomain + * are allowed and example.com is not, exactly as they are for a real user. */ +function proxy(rules: Egress.Rule[]) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "egress-live-")) + const socket = path.join(dir, "e.sock") + const server = Egress.serveProxy({ socket, rules }) + return { + socket, + stop: () => { + server.stop(true) + fs.rmSync(dir, { recursive: true, force: true }) + }, + } +} + +/** Run `script` the way a sandboxed shell command actually runs in + * production — through `Sandbox.plan`, a real `bwrap --unshare-net`, the + * real composed shim script, and the real per-connection proxy allowlist + * check. Nothing here is stubbed. */ +async function run(script: string, work: string, socket: string) { + const spec = Sandbox.plan({ + command: script, + shell: "/bin/sh", + cwd: work, + workspace: [work], + options: { enabled: true, network: "allowlist", egress: socket }, + }) + const proc = Bun.spawn([spec.file, ...(spec.args ?? [])], { + cwd: work, + env: { ...process.env, ...spec.env }, + stdout: "pipe", + stderr: "pipe", + }) + const [stdout, stderr] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]) + await proc.exited + return { stdout, stderr } +} + +describe.skipIf(skip)("egress: a real sandbox, a real proxy, a real remote host", () => { + test("an allowlisted host reaches 200, a denied one does not, and neither a direct connection nor DNS works outside the shim", async () => { + await using work = await tmpdir() + const host = proxy(Egress.DEFAULT_RULES) + try { + const script = [ + `pypi=$(curl -sS -I -m 30 -o /dev/null -w '%{http_code}' https://pypi.org/simple/)`, + `eutils=$(curl -sS -m 30 -o /dev/null -w '%{http_code}' 'https://eutils.ncbi.nlm.nih.gov/entrez/eutils/einfo.fcgi')`, + `example=$(curl -sS -I -m 15 -o /dev/null -w '%{http_code}' https://example.com/)`, + // The load-bearing pair. `unset` inside a subshell strips every proxy + // var — including `ALL_PROXY`, curl's protocol-agnostic fallback, + // which a host could export even with `HTTPS_PROXY` unset — for this + // one curl only; the checks above still route through the shim. So a + // 200 here would mean the socket is a convenience rather than the + // only way out, which is the entire claim this file exists to prove. + `direct=$(unset HTTP_PROXY HTTPS_PROXY http_proxy https_proxy ALL_PROXY all_proxy NO_PROXY no_proxy; curl -sS -I -m 10 -o /dev/null -w '%{http_code}' https://pypi.org/)`, + `resolved=$(timeout 10 getent hosts pypi.org)`, + `printf 'PYPI=%s\\nEUTILS=%s\\nEXAMPLE=%s\\nDIRECT=%s\\nGETENT=[%s]\\n' "$pypi" "$eutils" "$example" "$direct" "$resolved"`, + ].join("\n") + + const { stdout, stderr } = await run(script, work.path, host.socket) + const field = (name: string) => stdout.match(new RegExp(`^${name}=(.*)$`, "m"))?.[1] + const detail = `stdout=${stdout} stderr=${stderr}` + + expect(field("PYPI"), detail).toBe("200") + expect(field("EUTILS"), detail).toBe("200") + // Guarded with a shape check first: without it, a parsing regression + // that made `field()` return undefined would still satisfy + // `not.toBe("200")` and pass with nothing actually verified. + expect(field("EXAMPLE"), detail).toMatch(/^\d{3}$/) + expect(field("EXAMPLE"), detail).not.toBe("200") + expect(field("DIRECT"), detail).toMatch(/^\d{3}$/) + expect(field("DIRECT"), detail).not.toBe("200") + expect(field("GETENT"), detail).toBe("[]") + } finally { + host.stop() + } + }, 120_000) + + // Not pypi.org/simple/ itself: that index's byte length changes as + // packages are published, so only a content-addressed release file has a + // size and sha256 that stay true forever. This is numpy 1.26.4's + // manylinux cp311 wheel from files.pythonhosted.org — re-hashed directly + // against pypi.org while writing this test — chosen for size (18 MB, + // comfortably past the send-buffer boundary where the historical + // truncation bug was invisible) over the alternative of re-fetching + // pypi.org/simple/ (~45 MB) and asserting only its size, which drifts. + const WHEEL_URL = + "https://files.pythonhosted.org/packages/3a/d0/edc009c27b406c4f9cbc79274d6e46d634d139075492ad055e3d68445925/numpy-1.26.4-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl" + const WHEEL_SIZE = 18_252_005 + const WHEEL_SHA256 = "666dbfb6ec68962c033a450943ded891bed2d54e6755e35e5835d63f4f6931d5" + + test("megabytes through a real proxy and a real host arrive byte-for-byte", async () => { + await using work = await tmpdir() + const host = proxy(Egress.DEFAULT_RULES) + try { + const out = path.join(work.path, "numpy.whl") + // curl's own budget stays comfortably inside the outer 120_000ms: the + // bwrap spawn and shim-readiness wait run before curl even starts, and + // `finally`'s cleanup runs after it ends. Matching the two would let a + // download that genuinely needs close to 120s race the outer bun:test + // timeout instead of curl's own — trading a diagnostic + // `curl: (28) Operation timed out` for a generic "test timed out" and + // deferring `host.stop()` until the abandoned promise chain resolves. + const script = `curl -sS -m 90 -o ${JSON.stringify(out)} -w '%{http_code} %{size_download}' ${JSON.stringify(WHEEL_URL)}` + + const { stdout, stderr } = await run(script, work.path, host.socket) + expect(stdout.trim(), `stderr=${stderr}`).toBe(`200 ${WHEEL_SIZE}`) + + // The status code and curl's own byte count are what a truncation + // bug can still get right — the response frame ends early but + // cleanly. The independent check is reading the file back and + // hashing what actually landed on disk. + const bytes = await Bun.file(out).arrayBuffer() + expect(bytes.byteLength).toBe(WHEEL_SIZE) + expect(new Bun.CryptoHasher("sha256").update(bytes).digest("hex")).toBe(WHEEL_SHA256) + } finally { + host.stop() + } + }, 120_000) + + // The case this whole branch exists to enable: a real `pip install` from + // pypi, inside the sandbox, with `network: "allowlist"` as the only route + // out. The tests above prove the boundary holds; this proves the boundary + // is *usable*, which is a different claim. pip does more than one curl + // does — it issues its own index request, follows a redirect from pypi.org + // to files.pythonhosted.org (a second allowlist entry, so this also covers + // a cross-host hop through the proxy), streams a wheel, and unpacks it. + // + // `--only-binary :all:` keeps this a network test rather than a toolchain + // test: a source build failing for want of a compiler would be a red run + // that says nothing about egress. tqdm is pure Python, small, and pulls no + // dependencies, so a version printed back means the index request, the + // download and the install all crossed the proxy. + // + // The venv itself needs no network — `python3 -m venv` bootstraps pip from + // the wheel bundled in the interpreter's own `ensurepip`. That is why this + // works on a machine with no pip on PATH at all, which was one of the + // three blockers that started this work. + test.skipIf(!python)( + "pip install reaches pypi through the proxy and the installed package imports", + async () => { + await using work = await tmpdir() + const host = proxy(Egress.DEFAULT_RULES) + try { + const venv = path.join(work.path, "venv") + const script = [ + `set -e`, + `python3 -m venv ${JSON.stringify(venv)}`, + `${JSON.stringify(path.join(venv, "bin/pip"))} install --only-binary :all: --disable-pip-version-check -q tqdm`, + `printf 'TQDM=%s\\n' "$(${JSON.stringify(path.join(venv, "bin/python"))} -c 'import tqdm; print(tqdm.__version__)')"`, + ].join("\n") + + const { stdout, stderr } = await run(script, work.path, host.socket) + expect(stdout.match(/^TQDM=(.+)$/m)?.[1], `stdout=${stdout} stderr=${stderr}`).toMatch(/^\d+\.\d+/) + } finally { + host.stop() + } + }, + 240_000, + ) +}) diff --git a/backend/cli/test/sandbox/egress-runtime.test.ts b/backend/cli/test/sandbox/egress-runtime.test.ts new file mode 100644 index 00000000..a8b512ba --- /dev/null +++ b/backend/cli/test/sandbox/egress-runtime.test.ts @@ -0,0 +1,483 @@ +import { afterEach, expect, test } from "bun:test" +import fs from "fs/promises" +import path from "path" +import { Config } from "../../src/config/config" +import { Global } from "../../src/global" +import { EgressRuntime } from "../../src/sandbox/egress-runtime" +import { Sandbox } from "../../src/sandbox/sandbox" + +// A global config write is process-wide and outlives any one test, so every +// test that touches sandbox config must undo it — otherwise it leaks into +// whichever test file bun happens to run next in this process. +async function cleanGlobalSandboxConfig() { + for (const name of ["openscience.jsonc", "openscience.json", "config.json"]) { + await fs.rm(path.join(Global.Path.config, name), { force: true }).catch(() => {}) + } + Config.global.reset() +} + +afterEach(async () => { + await EgressRuntime.stop() + await cleanGlobalSandboxConfig() +}) + +test("ensure is idempotent and returns a stable address", async () => { + const first = await EgressRuntime.ensure() + const second = await EgressRuntime.ensure() + expect(second.socket).toBe(first.socket) + expect(second.port).toBe(first.port) + await EgressRuntime.stop() +}) + +test("a failed start does not latch — the next call really retries", async () => { + // Making the state directory unwritable is the cheapest real way to make + // the bind fail; every other route (a port already taken, a path too long) + // is either not applicable to a unix socket or harder to arrange + // deterministically. Global.Path.state is a per-test-process tmpdir (see + // test/preload.ts), so this cannot touch a developer's real state dir — + // but it is still restored in `finally`, because leaving it read-only + // would break every later test in this process rather than just this one. + const dir = Global.Path.state + await fs.mkdir(dir, { recursive: true }) + const mode = (await fs.stat(dir)).mode & 0o7777 + + const failure = await (async () => { + try { + await fs.chmod(dir, 0o500) + // platform "linux", not the ambient one: an unwritable state directory + // only fails the bubblewrap listener, which is the one that binds a + // unix socket there. The darwin listener binds a loopback port and + // would have started happily, leaving `failure` undefined. + return await EgressRuntime.ensure("linux").then( + () => undefined, + (error) => error as Error, + ) + } finally { + await fs.chmod(dir, mode) + } + })() + + // Loud: the message has to name the thing that broke and what depends on + // it, since the caller is an unrelated-looking bash/kernel/job spawn. + expect(failure?.message).toContain("sandbox allowlist proxy") + + // Recoverable: the rejected promise must not have been cached. Caching it + // would make one transient failure permanent for the process — and under + // the "allowlist" default that is every bash command, terminal, kernel and + // compute job failing until restart. + const recovered = await EgressRuntime.ensure("linux") + // Non-null: the bubblewrap listener always carries a socket — + // `egress-runtime.ts`'s `Running` type makes the field optional only + // because the darwin branch (added by Task 7) carries a TCP endpoint + // instead. + await expect(fs.stat(recovered.socket!)).resolves.toBeDefined() +}) + +test("stop is safe after a start that failed", async () => { + const dir = Global.Path.state + await fs.mkdir(dir, { recursive: true }) + const mode = (await fs.stat(dir)).mode & 0o7777 + try { + await fs.chmod(dir, 0o500) + // platform "linux": same reason as the test above — only the bubblewrap + // listener fails on an unwritable state directory, and a start that + // succeeded would not exercise the escape hatch this test is about. + await EgressRuntime.ensure("linux").catch(() => {}) + } finally { + await fs.chmod(dir, mode) + } + // The escape hatch must not hand back the same failure it exists to clear. + await expect(EgressRuntime.stop()).resolves.toBeUndefined() +}) + +test("the socket is created under the state directory, not the workspace", async () => { + // platform "linux": there is no socket to place at all on darwin, which + // listens on a loopback port instead. + const { socket } = await EgressRuntime.ensure("linux") + // Non-null: the bubblewrap listener always carries a socket. + // Not Bun.file(socket).exists(): Bun.file() only recognizes regular files, + // and a unix socket is a distinct inode type (S_IFSOCK) — verified with an + // isolated Bun.listen({ unix }) that Bun.file(...).exists() reports false + // for it while fs.stat sees it fine. fs.stat is the correct check here. + await expect(fs.stat(socket!)).resolves.toBeDefined() + expect(socket).not.toContain(process.cwd()) + await EgressRuntime.stop() +}) + +/** Speaks the proxy's wire format directly (see egress.ts) rather than going + * through the sandbox/shim, so this stays a test of EgressRuntime's rule + * freshness and not of the loopback bridge. CONNECT is used because its + * authority is the raw request target — no URL parsing to get right — and + * because both outcomes under test (denied vs. attempted-and-unreachable) + * answer with a 403 whose body text is the only thing distinguishing them. */ +function proxyRequest(socket: string, authority: string): Promise { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error(`no response from the proxy for ${authority}`)), 2_000) + let body = "" + Bun.connect({ + unix: socket, + socket: { + open(client) { + client.write(`CONNECT ${authority} HTTP/1.1\r\nHost: ${authority}\r\n\r\n`) + }, + data(_client, chunk) { + body += chunk.toString() + }, + close() { + clearTimeout(timeout) + resolve(body) + }, + error(_client, error) { + clearTimeout(timeout) + reject(error) + }, + }, + }).catch(reject) + }) +} + +test("an allowlist edit reaches a running proxy without restarting it", async () => { + // Nothing listens on loopback:1 (a privileged, essentially never-bound + // port), so a request that clears the allowlist check still gets a 403 — + // "cannot reach", not "not on the allowlist". That distinction is what + // proves the check ran, without needing a real upstream. + const authority = "127.0.0.1:1" + // platform "linux": `proxyRequest` speaks to a unix socket, which only the + // bubblewrap listener has. The freshness behaviour under test is the + // proxy's, not the listener's, so pinning the transport keeps this one + // test meaningful on either kind of machine. + const first = await EgressRuntime.ensure("linux") + + // Non-null: the bubblewrap listener always carries a socket. + const before = await proxyRequest(first.socket!, authority) + expect(before).toContain("not on the sandbox allowlist") + + await Config.setSandbox({ allowHosts: ["127.0.0.1"] }) + + // The proxy was never restarted — the same server, on the same socket, + // now answers differently because it re-reads the allowlist per + // connection rather than the snapshot it was born with. Retried rather + // than asserted on the first attempt: the update reaches the running + // proxy through a reactive config-change listener, not synchronously + // with Config.setSandbox's own return. + const deadline = Date.now() + 2_000 + let after = before + while (Date.now() < deadline && after.includes("not on the sandbox allowlist")) { + after = await proxyRequest(first.socket!, authority) + } + expect(after).not.toContain("not on the sandbox allowlist") + expect(after).toContain("Cannot reach") + + const second = await EgressRuntime.ensure("linux") + expect(second.socket).toBe(first.socket) // same proxy the whole time, not a restart +}) + +/** + * `egressFor` has to answer, ahead of time, the same "would this actually be + * sandboxed with an allowlist" question that `Sandbox.plan()`/`wrapArgv()` + * answer for real via `decide()` + `buildPolicy()` — its socket becomes their + * `options.egress`. The two used to default the unset cases in opposite + * directions from each other on both fields, invisibly, because production's + * five callers always pass an already-fully-resolved policy. These pin the + * shared default (`Sandbox.resolved`) so a future edit that reintroduces a + * hand-rolled check in just one of the two places fails here instead of + * shipping. + */ +test("egressFor treats a missing enabled the same way decide() does: off", async () => { + // Old behaviour: only an explicit `enabled: false` opted out, so this + // started a real proxy nothing could ever reach — decide() never wraps a + // command whose `options.enabled` isn't literally `true`. + const egress = await EgressRuntime.egressFor({ network: "allowlist" }) + expect(egress).toBeUndefined() +}) + +test.skipIf(Sandbox.backend() !== "bubblewrap")( + "egressFor treats a missing network the same way buildPolicy() does: allowlist", + async () => { + // Old behaviour: a missing `network` read as "not allowlist" here, so + // this returned undefined while buildPolicy() (used by the same + // options a moment later, in Sandbox.plan/wrapArgv) still defaulted + // network to "allowlist" and demanded an egress socket — the exact + // "requires an egress socket path" crash, reproduced below without the + // fix. + const egress = await EgressRuntime.egressFor({ enabled: true }) + expect(egress).toBeDefined() + expect(() => + Sandbox.plan({ + command: "true", + shell: "/bin/sh", + cwd: "/tmp", + workspace: ["/tmp"], + options: { enabled: true, egress }, + }), + ).not.toThrow() + }, +) + +/** + * Same wire technique as `proxyRequest` above, but dialed over TCP loopback + * rather than the unix socket — what a seatbelt-sandboxed process reaches + * directly, since seatbelt has no namespace to bind a unix socket into and + * `Egress.serveProxy` listens on that loopback port itself (decision 1 of + * the Task 7 brief: no host-side bridge). `auth`, when given, is sent as the + * `Proxy-Authorization` secret the TCP listener requires (decision 2) — + * omitted or wrong, the request must never reach the allowlist check at all. + * + * This is the one seatbelt-specific piece of Task 7 that genuinely runs, + * without a Mac: everything here is a plain `Bun.connect`/`Bun.listen` pair + * with nothing namespace- or platform-specific about it, so starting + * `EgressRuntime` with `platform: "darwin"` injected and dialing it for real + * proves the proxy → auth → allowlist path actually runs. What it cannot + * prove is whether a real `sandbox-exec` restricts a sandboxed process to + * dialing only this one port in the first place — see the Task 7 report for + * exactly what a Mac owner still needs to run. + */ +function tcpProxyRequest(port: number, authority: string, auth?: string): Promise { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error(`no response from the proxy for ${authority}`)), 2_000) + let body = "" + const header = auth ? `\r\nProxy-Authorization: Basic ${Buffer.from(`os:${auth}`).toString("base64")}` : "" + Bun.connect({ + hostname: "127.0.0.1", + port, + socket: { + open(client) { + client.write(`CONNECT ${authority} HTTP/1.1\r\nHost: ${authority}${header}\r\n\r\n`) + }, + data(_client, chunk) { + body += chunk.toString() + }, + close() { + clearTimeout(timeout) + resolve(body) + }, + error(_client, error) { + clearTimeout(timeout) + reject(error) + }, + }, + }).catch(reject) + }) +} + +test("ensure with platform darwin listens on a loopback TCP port, not a unix socket", async () => { + const running = await EgressRuntime.ensure("darwin") + expect(running.hostname).toBe("127.0.0.1") + // Ephemeral, not the bwrap shim's fixed SHIM_PORT: seatbelt has no + // namespace to keep a fixed port private across concurrently sandboxed + // processes the way --unshare-net does for bubblewrap. + expect(running.port).not.toBe(Sandbox.SHIM_PORT) + expect(running.port).toBeGreaterThan(0) + // A secret was generated for this start — required to reach the proxy at + // all, since a loopback port (unlike a unix socket) carries no filesystem + // permissions of its own. + expect(running.secret).toBeTruthy() + // And no unix socket on this path at all — decision 1 of the Task 7 + // brief: serveProxy listens on TCP directly, no host-side bridge to one. + expect(running.socket).toBeUndefined() +}) + +test("the darwin proxy forwards a correctly-authenticated request past both auth and the allowlist check, live", async () => { + // 127.0.0.1 is not in Egress.DEFAULT_RULES, so it has to be added + // explicitly here — otherwise a request to 127.0.0.1:1 is denied at the + // allowlist check before the auth → dial chain this test exists to prove + // is ever reached at all. (Task 7 fix round 1, I2: the unfixed version of + // this test asserted "not on the sandbox allowlist" — the denial text — + // while its own comment claimed the opposite outcome. Both the comment + // and the assertion described a dial that never actually happened; + // measured by the reviewer, confirmed here by fixing it forward instead + // of just correcting the prose.) + await Config.setSandbox({ allowHosts: ["127.0.0.1"] }) + const running = await EgressRuntime.ensure("darwin") + // Nothing listens on loopback:1 (a privileged, essentially never-bound + // port), so a request that clears BOTH the auth check and the allowlist + // check still gets a 403 "Cannot reach" — not "not on the sandbox + // allowlist" and not 407 — which is what proves the whole + // auth → allowlist → dial chain actually ran end to end. + const body = await tcpProxyRequest(running.port, "127.0.0.1:1", running.secret) + expect(body).toContain("Cannot reach") + expect(body).not.toContain("not on the sandbox allowlist") + expect(body).not.toContain("407") +}) + +test("the darwin proxy refuses a request with no Proxy-Authorization, and never forwards it", async () => { + const running = await EgressRuntime.ensure("darwin") + const body = await tcpProxyRequest(running.port, "127.0.0.1:1") + expect(body).toContain("407 Proxy Authentication Required") + // Neither downstream outcome appears — the request was refused before the + // allowlist check or the dial ever ran. + expect(body).not.toContain("not on the sandbox allowlist") + expect(body).not.toContain("Cannot reach") +}) + +test("the darwin proxy refuses a request with the wrong secret", async () => { + const running = await EgressRuntime.ensure("darwin") + const body = await tcpProxyRequest(running.port, "127.0.0.1:1", `${running.secret}-wrong`) + expect(body).toContain("407 Proxy Authentication Required") +}) + +test('egressFor on darwin returns "port:secret", not a socket path', async () => { + const egress = await EgressRuntime.egressFor({ enabled: true, network: "allowlist" }, "darwin") + expect(egress).toBeDefined() + const [portPart, secretPart] = egress!.split(":") + expect(Number.isInteger(Number(portPart))).toBe(true) + // Shape-checked against crypto.randomUUID()'s actual format, not just + // toBeTruthy(): the string "undefined" — what a missing `secret` coerces + // to inside a template literal — is itself truthy, so a bare truthiness + // check structurally cannot catch the I1 defect the next test reproduces. + expect(secretPart).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/) + expect(egress).not.toContain("/") + expect(egress).not.toContain(".sock") +}) + +// Task 7 fix round 1, I1: egressFor's seatbelt branch used to interpolate +// `running.secret` with no guard. ensure()/start() cache ONE proxy for the +// process lifetime (see ensure()'s doc comment); `platform` only decides +// what starts when nothing is running yet. Asking for "darwin" after a +// bubblewrap proxy is already cached — impossible for a real caller, since +// process.platform never changes mid-process, but reachable here because +// platform is deliberately injectable for testing — used to silently reuse +// that cached listener and return the literal string "3128:undefined" +// (Buffer-safe, syntactically valid, and — per the test above — exactly +// what a bare `toBeTruthy()` on the secret half cannot distinguish from a +// real one). Confirmed by execution before the fix; asserts the fail-closed +// replacement here. +test("egressFor on darwin fails closed rather than composing an undefined secret when a differently-platformed proxy is already cached", async () => { + // Force the FIRST proxy to be the bubblewrap (unix-socket) shape, + // deterministically regardless of what machine actually runs this test — + // the same platform-injection seam every darwin test in this file uses, + // just pointed at the other platform. + await EgressRuntime.ensure("linux") + await expect(EgressRuntime.egressFor({ enabled: true, network: "allowlist" }, "darwin")).rejects.toThrow( + /already running as the bubblewrap/, + ) +}) + +test("egressFor with network deny or allow never starts a proxy on darwin", async () => { + const deny = await EgressRuntime.egressFor({ enabled: true, network: "deny" }, "darwin") + expect(deny).toBeUndefined() + const allow = await EgressRuntime.egressFor({ enabled: true, network: "allow" }, "darwin") + expect(allow).toBeUndefined() +}) + +test("egressFor on linux/bubblewrap keeps returning the unix socket path, unaffected by the darwin branch", async () => { + if (Sandbox.backend() !== "bubblewrap") return + const egress = await EgressRuntime.egressFor({ enabled: true, network: "allowlist" }) + expect(egress).toContain(".sock") +}) + +/** + * Task 7 fix round 1, I4: every auth test above hand-builds the + * `Proxy-Authorization` header itself, which leaves the seam joining + * `sandbox.ts`'s `proxyUrl()` (`http://os:@host:port`) to + * `egress.ts`'s own parser of that header unpinned in-suite — a rename of + * the userinfo user ("os") in one place only would still pass every other + * test here. These drive a real `curl`, an independent HTTP client + * implementation, at the *exact* URL `Sandbox.plan()` composes, covering + * both wire forms curl uses to talk to a proxy: an absolute-form GET (its + * default for a plain `http://` target) and a CONNECT tunnel (forced with + * `--proxytunnel`, and also what curl uses unprompted for an `https://` + * target — see `egress-live.test.ts` for that shape against a real host). + * A third, with Python's `urllib`, covers a second independent client + * library — the same proxy-auth mechanism pip itself relies on. + */ +function planProxyUrl(egress: string): string { + const plan = Sandbox.plan({ + command: "true", + shell: "/bin/sh", + cwd: "/tmp", + workspace: ["/tmp"], + options: { enabled: true, network: "allowlist", egress }, + platform: "darwin", + }) + const proxy = plan.env?.HTTP_PROXY + expect(proxy).toMatch(/^http:\/\/os:.+@127\.0\.0\.1:\d+$/) + return proxy! +} + +/** `Bun.spawn`, never `Bun.spawnSync`: the origin and the proxy both reply + * from `Bun.listen`/`Bun.serve` callbacks on this same event loop, so a + * *synchronous* spawn would block that loop for as long as the child runs + * — the child blocks on recv() waiting for a reply the loop can't yet + * deliver, deadlocking both sides until the test times out. Reproduced + * while writing these three tests (all three hung at 5s with empty stdout) + * — the same defect class `sandbox.test.ts`'s own "Bun.spawn, not + * spawnSync" comment documents for an identical reason. */ +async function runCapture(cmd: string[]) { + const proc = Bun.spawn(cmd, { stdout: "pipe", stderr: "pipe" }) + const [stdout, stderr] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]) + await proc.exited + return { stdout, stderr } +} + +test.skipIf(!Bun.which("curl"))( + "a real curl (absolute-form GET) using the exact URL Sandbox.plan() emits authenticates and is forwarded", + async () => { + const origin = Bun.serve({ port: 0, fetch: () => new Response("ok") }) + try { + await Config.setSandbox({ allowHosts: ["127.0.0.1"] }) + const running = await EgressRuntime.ensure("darwin") + const proxy = planProxyUrl(`${running.port}:${running.secret}`) + const { stdout, stderr } = await runCapture([ + "curl", + "-sS", + "-m", + "5", + "-x", + proxy, + `http://127.0.0.1:${origin.port}/`, + ]) + expect(stdout, stderr).toBe("ok") + } finally { + origin.stop(true) + } + }, +) + +test.skipIf(!Bun.which("curl"))( + "a real curl --proxytunnel (forced CONNECT) using the exact URL Sandbox.plan() emits authenticates and is forwarded", + async () => { + const origin = Bun.serve({ port: 0, fetch: () => new Response("ok") }) + try { + await Config.setSandbox({ allowHosts: ["127.0.0.1"] }) + const running = await EgressRuntime.ensure("darwin") + const proxy = planProxyUrl(`${running.port}:${running.secret}`) + const { stdout, stderr } = await runCapture([ + "curl", + "-sS", + "-m", + "5", + "--proxytunnel", + "-x", + proxy, + `http://127.0.0.1:${origin.port}/`, + ]) + expect(stdout, stderr).toBe("ok") + } finally { + origin.stop(true) + } + }, +) + +test.skipIf(!Bun.which("python3"))( + "a real Python urllib request using the exact URL Sandbox.plan() emits authenticates and is forwarded", + async () => { + const origin = Bun.serve({ port: 0, fetch: () => new Response("ok") }) + try { + await Config.setSandbox({ allowHosts: ["127.0.0.1"] }) + const running = await EgressRuntime.ensure("darwin") + const proxy = planProxyUrl(`${running.port}:${running.secret}`) + const target = `http://127.0.0.1:${origin.port}/` + const script = [ + "import urllib.request", + `handler = urllib.request.ProxyHandler({"http": ${JSON.stringify(proxy)}})`, + "opener = urllib.request.build_opener(handler)", + `print(opener.open(${JSON.stringify(target)}, timeout=5).read().decode(), end="")`, + ].join("\n") + const { stdout, stderr } = await runCapture(["python3", "-c", script]) + expect(stdout, stderr).toBe("ok") + } finally { + origin.stop(true) + } + }, +) diff --git a/backend/cli/test/sandbox/egress.test.ts b/backend/cli/test/sandbox/egress.test.ts new file mode 100644 index 00000000..69eb0e4c --- /dev/null +++ b/backend/cli/test/sandbox/egress.test.ts @@ -0,0 +1,679 @@ +import { afterEach, expect, test } from "bun:test" +import fs from "fs" +import os from "os" +import path from "path" +import type { Socket } from "bun" +import { Egress } from "../../src/sandbox/egress" + +test("an exact rule matches only that host", () => { + expect(Egress.allowed("pypi.org", ["pypi.org"])).toBe(true) + expect(Egress.allowed("evil-pypi.org", ["pypi.org"])).toBe(false) +}) + +test("a leading dot matches the domain and its subdomains", () => { + expect(Egress.allowed("eutils.ncbi.nlm.nih.gov", [".ncbi.nlm.nih.gov"])).toBe(true) + expect(Egress.allowed("ncbi.nlm.nih.gov", [".ncbi.nlm.nih.gov"])).toBe(true) + expect(Egress.allowed("ncbi.nlm.nih.gov.evil.com", [".ncbi.nlm.nih.gov"])).toBe(false) +}) + +test("a port on the authority is ignored when matching", () => { + expect(Egress.allowed("pypi.org:443", ["pypi.org"])).toBe(true) +}) + +test("matching is case-insensitive in both directions", () => { + expect(Egress.allowed("PyPI.ORG", ["pypi.org"])).toBe(true) + expect(Egress.allowed("pypi.org", ["PyPI.ORG"])).toBe(true) +}) + +test("an empty ruleset allows nothing", () => { + expect(Egress.allowed("pypi.org", [])).toBe(false) +}) + +test("the shipped defaults cover the registries and scientific APIs the product needs", () => { + for (const host of [ + "pypi.org", + "files.pythonhosted.org", + "cran.r-project.org", + "eutils.ncbi.nlm.nih.gov", + "rest.uniprot.org", + ]) { + expect(Egress.allowed(host, Egress.DEFAULT_RULES)).toBe(true) + } +}) + +test("the shipped defaults do not permit general browsing", () => { + for (const host of ["example.com", "www.google.com", "raw.githubusercontent.com"]) { + expect(Egress.allowed(host, Egress.DEFAULT_RULES)).toBe(false) + } +}) + +// ── volume ────────────────────────────────────────────────────────────────── +// +// Everything above, and every earlier test of the bridge itself, moves a few +// bytes. That is exactly the size at which a dropped-backpressure bug is +// invisible: one small write fits the send buffer whole, so the byte count +// `Socket.write` returns equals what was asked of it and discarding that count +// costs nothing. Past a send buffer it does not — before `pump` existed, 8 MB +// through the proxy arrived as ~2.6 MB, and `pip download numpy` inside a real +// sandbox died with `SSL: RECORD_LAYER_FAILURE` while an 11 KB package +// installed fine. So these transfer real volume, in both directions, and +// compare the bytes rather than counting them. + +const VOLUME = 8 * 1024 * 1024 + +/** Not a constant fill: a repeated byte would pass even if the bridge + * duplicated or reordered a chunk, which is the other way a backpressure + * queue goes wrong. This makes position observable. */ +const sample = Buffer.from(Uint8Array.from({ length: VOLUME }, (_, i) => (i * 31 + (i >> 13)) % 251)) + +const opened: { stop: () => void }[] = [] + +afterEach(() => { + for (const it of opened.splice(0)) it.stop() +}) + +/** A raw TCP origin. Sends `sample` at whatever pace the peer accepts (so the + * test measures the bridge's backpressure, not the origin's), collects + * everything sent to it, and closes only once both halves are complete — + * which is also what makes an early `end()` on the bridge observable, since a + * close that jumps a queue truncates the tail rather than hanging. */ +type Talker = { sent: number; got: number; head: number; received: Buffer[] } + +function origin() { + const uploads: Buffer[][] = [] + const talkers = new WeakMap, Talker>() + + const push = (sock: Socket, held: Talker) => { + while (held.sent < VOLUME) { + const wrote = sock.write(sample.subarray(held.sent)) + if (wrote <= 0) return + held.sent += wrote + } + if (held.head >= 0 && held.got - held.head >= VOLUME) sock.end() + } + + const server = Bun.listen({ + hostname: "127.0.0.1", + port: 0, + socket: { + open(sock) { + const held: Talker = { sent: 0, got: 0, head: -1, received: [] } + uploads.push(held.received) + talkers.set(sock, held) + }, + data(sock, chunk) { + const held = talkers.get(sock) + if (!held) return + held.received.push(Buffer.from(chunk)) + held.got += chunk.length + // The request head is the cue to start sending, and its length is what + // makes "the whole upload arrived" a byte count rather than a guess. + if (held.head < 0) { + const end = Buffer.concat(held.received).indexOf("\r\n\r\n") + if (end < 0) return + held.head = end + 4 + } + push(sock, held) + }, + drain(sock) { + const held = talkers.get(sock) + if (held) push(sock, held) + }, + error() {}, + }, + }) + + opened.push({ stop: () => server.stop(true) }) + return { port: server.port, uploads } +} + +function proxy(rules: string[]) { + const socket = path.join(fs.mkdtempSync(path.join(os.tmpdir(), "egress-vol-")), "e.sock") + const server = Egress.serveProxy({ socket, rules }) + opened.push({ + stop: () => { + server.stop(true) + fs.rmSync(path.dirname(socket), { recursive: true, force: true }) + }, + }) + return socket +} + +function shim(socket: string) { + // Port 0 lets the OS pick, so concurrent test files cannot collide the way a + // fixed 3128 would. Inside a real sandbox the port is fixed instead, because + // --unshare-net makes collision impossible there. + const server = Egress.serveShim({ port: 0, socket }) + opened.push({ stop: () => server.stop(true) }) + return server.port +} + +/** Speak CONNECT to the proxy, upload `send`, then read until close. */ +function transfer(to: { unix: string } | { hostname: string; port: number }, authority: string, send: Buffer) { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error("the transfer never completed")), 60_000) + const chunks: Buffer[] = [] + const state = { established: false, at: 0 } + const upload = (sock: Socket) => { + while (state.at < send.length) { + const wrote = sock.write(send.subarray(state.at)) + if (wrote <= 0) return + state.at += wrote + } + } + const done = (result: Buffer) => { + clearTimeout(timeout) + resolve(result) + } + const handlers = { + open(sock: Socket) { + sock.write(`CONNECT ${authority} HTTP/1.1\r\nHost: ${authority}\r\n\r\n`) + }, + data(sock: Socket, chunk: Buffer) { + if (state.established) return void chunks.push(Buffer.from(chunk)) + const end = chunk.indexOf("\r\n\r\n") + if (end === -1) return + state.established = true + chunks.push(Buffer.from(chunk.subarray(end + 4))) + sock.write("GET / HTTP/1.0\r\n\r\n") + upload(sock) + }, + drain: upload, + close: () => done(Buffer.concat(chunks)), + error: () => done(Buffer.concat(chunks)), + } + // Branched rather than spread: Bun.connect is overloaded on unix vs + // hostname/port, and a union spread into one object literal matches + // neither overload. + const dial = + "unix" in to + ? Bun.connect({ unix: to.unix, socket: handlers }) + : Bun.connect({ hostname: to.hostname, port: to.port, socket: handlers }) + dial.catch(reject) + }) +} + +test("megabytes survive the proxy byte for byte, in both directions", async () => { + const upstream = origin() + const socket = proxy(["127.0.0.1"]) + + const down = await transfer({ unix: socket }, `127.0.0.1:${upstream.port}`, sample) + + expect(down.length).toBe(VOLUME) + expect(down.equals(sample)).toBe(true) + // The uploaded copy arrives behind the "GET / HTTP/1.0" that cued the + // download, so drop that prefix before comparing. + const up = Buffer.concat(upstream.uploads[0]!) + const body = up.subarray(up.indexOf("\r\n\r\n") + 4) + expect(body.length).toBe(VOLUME) + expect(body.equals(sample)).toBe(true) +}, 120_000) + +test("megabytes survive the shim and the proxy together, in both directions", async () => { + const upstream = origin() + const socket = proxy(["127.0.0.1"]) + const port = shim(socket) + + const down = await transfer({ hostname: "127.0.0.1", port }, `127.0.0.1:${upstream.port}`, sample) + + expect(down.length).toBe(VOLUME) + expect(down.equals(sample)).toBe(true) + const up = Buffer.concat(upstream.uploads[0]!) + const body = up.subarray(up.indexOf("\r\n\r\n") + 4) + expect(body.length).toBe(VOLUME) + expect(body.equals(sample)).toBe(true) +}, 120_000) + +// ── one client, one upstream ──────────────────────────────────────────────── +// +// Both bridges dial their upstream 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 distinct defects +// live in that window, and neither is visible to a test that moves bytes +// through a connection that behaves politely from start to finish. + +/** Handlers for an upstream that counts connections and, crucially, whether + * each one was ever closed. Counting sockets rather than file descriptors + * keeps this honest on platforms without /proc, and measures the actual + * invariant: nothing a bridge dials may be left with no owner. Each + * connection's bytes accumulate in their own entry, so a concurrent second + * connection cannot have its bytes attributed to the first. */ +function counter() { + const seen: { text: string }[] = [] + const counts = { opened: 0, closed: 0 } + const entries = new WeakMap, { text: string }>() + const socket = { + open(sock: Socket) { + counts.opened++ + const entry = { text: "" } + seen.push(entry) + entries.set(sock, entry) + }, + data(sock: Socket, chunk: Buffer) { + const entry = entries.get(sock) + if (entry) entry.text += chunk.toString() + }, + close() { + counts.closed++ + }, + error() {}, + } + return { counts, seen, socket } +} + +function unixCounter(at: string) { + const held = counter() + const server = Bun.listen({ unix: at, socket: held.socket }) + opened.push({ stop: () => server.stop(true) }) + return held +} + +function tcpCounter() { + const held = counter() + const server = Bun.listen({ hostname: "127.0.0.1", port: 0, socket: held.socket }) + opened.push({ stop: () => server.stop(true) }) + return { ...held, port: server.port } +} + +function scratch(name: string) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "egress-abort-")) + opened.push({ stop: () => fs.rmSync(dir, { recursive: true, force: true }) }) + return path.join(dir, name) +} + +/** Wait for the counts to stop moving rather than guessing at the race. */ +async function settle(counts: { opened: number; closed: number }) { + for (let i = 0; i < 40 && counts.closed < counts.opened; i++) await Bun.sleep(50) +} + +/** connect(), then FIN in the same turn, with nothing sent. */ +function abort(port: number) { + return Bun.connect({ + hostname: "127.0.0.1", + port, + socket: { + open(sock) { + sock.end() + }, + data() {}, + close() {}, + error() {}, + }, + }) +} + +test("a client that aborts mid-dial does not strand the shim's upstream", async () => { + const socket = scratch("u.sock") + const upstream = unixCounter(socket) + const port = shim(socket) + + const N = 60 + for (let i = 0; i < N; i++) await abort(port) + await settle(upstream.counts) + + // The dial really happened — otherwise this would pass trivially. + expect(upstream.counts.opened).toBeGreaterThan(0) + // And every one of them was closed. Before the shim tracked the client's + // departure, `close` ran while `toUpstream` was still undefined, so it had + // nothing to tear down: measured at 300 aborts across separate processes, + // 0.897 fd/conn stranded in the shim and the same in the host proxy, held + // for as long as the sandbox lives — hours, for a kernel or a terminal. + expect(upstream.counts.closed).toBe(upstream.counts.opened) +}, 60_000) + +test("a client that aborts mid-dial does not strand the proxy's upstream", async () => { + const socket = scratch("p.sock") + // "localhost" rather than 127.0.0.1: the dial then includes a name lookup, + // which is what holds the window open long enough to observe any of this. + // Resolved from /etc/hosts, so no network is involved. + const upstream = tcpCounter() + const server = Egress.serveProxy({ socket, rules: ["localhost"] }) + opened.push({ stop: () => server.stop(true) }) + + const N = 40 + for (let i = 0; i < N; i++) { + const client = await Bun.connect({ + unix: socket, + socket: { + open(sock) { + // A complete head, so the proxy commits to dialling, then leave. + sock.write(`CONNECT localhost:${upstream.port} HTTP/1.1\r\nHost: localhost\r\n\r\n`) + sock.end() + }, + data() {}, + close() {}, + error() {}, + }, + }) + client.end() + } + await settle(upstream.counts) + + expect(upstream.counts.opened).toBeGreaterThan(0) + expect(upstream.counts.closed).toBe(upstream.counts.opened) +}, 60_000) + +test("a body that arrives after its head still produces exactly one upstream", async () => { + const pieces = ["id=1", "&id=2", "&x"] + const body = pieces.join("") + const socket = scratch("d.sock") + const upstream = tcpCounter() + const server = Egress.serveProxy({ socket, rules: ["localhost"] }) + opened.push({ stop: () => server.stop(true) }) + + // The shape NCBI E-utilities recommends for a large id list, and the shape + // `HTTP_PROXY` routes through this branch: a plain-http POST whose body + // follows the head across separate segments. Each of those segments used to + // re-enter `data`, find no link yet, re-parse the same buffered head and + // dial again — 2 upstream connections against a local origin, 4 against a + // real remote one, every one of them carrying a duplicate of a + // non-idempotent request. + // Run in parallel, and not as a nod to realism: the dial has to still be in + // flight when the next segment lands, and on loopback with a warm resolver a + // single dial finishes inside the 1ms gap. Concurrency is what holds the + // window open — enough simultaneous name lookups to queue behind the + // resolver — and it is the only trigger measured here that survives a warm + // cache. Against the unfixed proxy this produced 200 upstream connections + // for these 100 clients, three runs out of three; at 25 clients it was one + // run in three, and sequentially it needed a cold cache to reproduce at all. + const target = `localhost:${upstream.port}` + const clients = 100 + const socks = await Promise.all( + Array.from({ length: clients }, async () => { + const client = await Bun.connect({ + unix: socket, + socket: { data() {}, close() {}, error() {} }, + }) + client.write(`POST http://${target}/eutils HTTP/1.1\r\nHost: ${target}\r\nContent-Length: ${body.length}\r\n\r\n`) + for (const [i, gap] of [1, 5, 10].entries()) { + await Bun.sleep(gap) + client.write(pieces[i]!) + } + return client + }), + ) + await Bun.sleep(1_000) + + expect(upstream.counts.opened).toBe(clients) + // And each one carries a whole request, rather than the head being + // duplicated onto a second connection with the body split between them. + expect(upstream.seen.length).toBe(clients) + for (const entry of upstream.seen) { + expect(entry.text).toContain("POST /eutils") + expect(entry.text).toContain(body) + } + for (const s of socks) s.end() +}, 60_000) + +// ── what one client can make the host allocate ────────────────────────────── +// +// serveProxy runs on the HOST, outside the sandbox, and holds memory on behalf +// of a process the sandbox exists to contain — in the CLI's own process, so an +// OOM there is the supervisor dying, not a worker. Two sub-phases buffer +// without a link to push into, and neither was bounded: +// +// head never terminated by CRLFCRLF, so no dial is ever attempted and +// nothing downstream limits it. Measured: 93 MiB of head took the +// host process from 36.0 MB to 1344.9 MB of RSS in 8 s, climbing. +// dialing a complete head for an allowlisted host that black-holes SYNs. +// Measured: 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`, +// with the dial still in flight and ~2 minutes of SYN retries left. +// +// Both assertions below are on bytes the proxy was willing to *accept*, which +// is what the growth was made of, and both are orders of magnitude clear of +// the fixed bounds so neither is timing-sensitive in the passing direction. + +/** Blast at a target until it stops accepting or `ms` elapses, and report how + * much it took. `stalled` distinguishes "the proxy stopped reading" — the + * backpressure this is looking for — from "we ran out of time". */ +function flood(to: { unix: string }, head: string, ms: number) { + return new Promise<{ sent: number; response: string; stalled: boolean }>((resolve, reject) => { + const CHUNK = Buffer.alloc(1 << 20, 0x41) // 'A' — cannot contain CRLFCRLF + const state = { sent: 0, response: "", done: false, stalled: false } + const finish = () => { + if (state.done) return + state.done = true + clearTimeout(timer) + resolve({ sent: state.sent, response: state.response, stalled: state.stalled }) + } + const timer = setTimeout(finish, ms) + const push = (sock: Socket) => { + state.stalled = false + while (!state.done) { + const wrote = sock.write(CHUNK) + if (wrote <= 0) return void (state.stalled = true) + state.sent += wrote + } + } + Bun.connect({ + unix: to.unix, + socket: { + open(sock) { + if (head) sock.write(head) + push(sock) + }, + drain: push, + data(_sock, chunk) { + state.response += chunk.toString("latin1") + }, + close: finish, + error: finish, + }, + }).catch(reject) + }) +} + +test("a head that never ends is refused rather than buffered", async () => { + const socket = proxy(["127.0.0.1"]) + + // No CRLFCRLF anywhere in the payload, so the parse never completes and the + // proxy never dials — this phase is not bounded by a connect() at all. + const flooded = await flood({ unix: socket }, "", 4_000) + + // Fail closed, and say why: 431 is the status RFC 6585 defines for exactly + // this. Against the unbounded version the client got no response whatsoever, + // because there is nothing in that code path that ever answers. + expect(flooded.response).toContain("431 Request Header Fields Too Large") + expect(flooded.response).toContain("Proxy request head exceeded") + + // And it was cut off early. The cap is 64 KiB; the client stops at whatever + // was already in flight when the proxy closed, which is a socket buffer or + // two. 16 MiB is a bound no correct implementation approaches and the + // unbounded one blows through in well under a second — it took 93 MiB in 8 s + // while growing the host by 1.3 GB. + expect(flooded.sent).toBeLessThan(16 * 1024 * 1024) +}, 60_000) + +test("a client cannot flood the host while its dial is in flight", async () => { + // 192.0.2.1 is TEST-NET-1 (RFC 5737): reserved for documentation, routed + // nowhere, so a SYN to it is dropped rather than refused and the dial stays + // in flight for the kernel's whole retry budget — ~130 s on Linux. That is + // the window under test, and allowlisting it is what gets the proxy to + // commit to dialling. Confirmed black-holed here by a bare connect that hung + // past 20 s with no RST and no ICMP. + const socket = proxy(["192.0.2.1"]) + const head = "CONNECT 192.0.2.1:443 HTTP/1.1\r\nHost: 192.0.2.1:443\r\n\r\n" + + const flooded = await flood({ unix: socket }, head, 4_000) + + // Precondition, asserted rather than assumed: the dial has to still be + // outstanding for this to be measuring anything. Any response at all — a 403 + // "Cannot reach", a 504 — means the environment answered TEST-NET-1 quickly + // and the window never opened, so the test would otherwise pass vacuously. + expect(flooded.response).toBe("") + + // The invariant: the proxy stopped reading, so the client cannot even + // generate the bytes — they stay in its socket buffer and then in it. This + // is backpressure rather than a cap, so there is no limit to tune; the bound + // asserted here is just the socket buffers on either side of the pause. + // Against the unbounded version this reached ~1 GiB inside 4 s. + expect(flooded.sent).toBeLessThan(16 * 1024 * 1024) + expect(flooded.stalled).toBe(true) +}, 60_000) + +test("a dial that never completes gives up and says so", async () => { + // Same black hole, but now waiting for the timeout rather than racing it. + // 150 ms stands in for the shipped 30 s so this costs the suite no real + // time; what it exercises is that the timer fires, answers, and closes, + // instead of the connection hanging for the kernel's ~130 s SYN budget. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "egress-timeout-")) + const socket = path.join(dir, "e.sock") + const server = Egress.serveProxy({ socket, rules: ["192.0.2.1"], dialTimeout: 150 }) + opened.push({ + stop: () => { + server.stop(true) + fs.rmSync(dir, { recursive: true, force: true }) + }, + }) + + const started = Date.now() + const answer = await new Promise((resolve, reject) => { + const fail = setTimeout(() => reject(new Error("the proxy never gave up on the dial")), 20_000) + const body = { text: "" } + Bun.connect({ + unix: socket, + socket: { + open(sock) { + sock.write("CONNECT 192.0.2.1:443 HTTP/1.1\r\nHost: 192.0.2.1:443\r\n\r\n") + }, + data(_sock, chunk) { + body.text += chunk.toString("latin1") + }, + close() { + clearTimeout(fail) + resolve(body.text) + }, + error() { + clearTimeout(fail) + resolve(body.text) + }, + }, + }).catch(reject) + }) + + expect(answer).toContain("504 Gateway Timeout") + expect(answer).toContain("192.0.2.1:443") + // Bounded by the budget, not by the kernel. Generous upper bound so a loaded + // machine cannot flake it, but far below the ~130 s this used to take. + expect(Date.now() - started).toBeLessThan(10_000) +}, 60_000) + +// ── seatbelt: TCP loopback + Proxy-Authorization ──────────────────────────── +// +// Everything above dials a unix socket. macOS has no network namespace to +// bind one into, so serveProxy listens directly on a loopback TCP port +// instead (Task 7 brief, decision 1 — no host-side bridge between the two; +// see the module doc comment and sandbox.ts's seatbeltProfile). A loopback +// TCP port, unlike a unix socket, carries no filesystem permissions of its +// own — every process on the machine can dial it — so that listener +// additionally requires a `Proxy-Authorization` secret (decision 2). These +// test the listener and that requirement directly, with no EgressRuntime or +// sandbox in between; `test/sandbox/egress-runtime.test.ts` covers the same +// property one layer up, through the lifecycle that actually generates and +// threads the secret in production. + +function tcpProxy(rules: string[], secret: string) { + const server = Egress.serveProxy({ hostname: "127.0.0.1", port: 0, secret, rules }) + opened.push({ stop: () => server.stop(true) }) + return server +} + +/** Speaks CONNECT directly over TCP loopback, optionally with a + * Proxy-Authorization header — the wire shape pip/curl/requests produce + * from a `http://os:@host:port` proxy URL. */ +function tcpRequest(port: number, authority: string, auth?: string): Promise { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error(`no response from the proxy for ${authority}`)), 5_000) + let body = "" + const header = + auth !== undefined ? `\r\nProxy-Authorization: Basic ${Buffer.from(`os:${auth}`).toString("base64")}` : "" + Bun.connect({ + hostname: "127.0.0.1", + port, + socket: { + open(sock) { + sock.write(`CONNECT ${authority} HTTP/1.1\r\nHost: ${authority}${header}\r\n\r\n`) + }, + data(_sock, chunk) { + body += chunk.toString() + }, + close() { + clearTimeout(timeout) + resolve(body) + }, + error(_sock, error) { + clearTimeout(timeout) + reject(error) + }, + }, + }).catch(reject) + }) +} + +test("serveProxy on TCP binds 127.0.0.1, never 0.0.0.0", () => { + const server = tcpProxy([], "s") + expect(server.hostname).toBe("127.0.0.1") +}) + +test("a correctly-authenticated TCP request passes auth and reaches the dial", async () => { + const port = tcpProxy(["127.0.0.1"], "right-secret").port + // 127.0.0.1:1 is allowlisted but nothing listens there (a privileged, + // essentially never-bound port) — "Cannot reach" (not 407, not "not on + // the sandbox allowlist") is what proves the secret was accepted and the + // request reached the dial, the same distinction `proxyRequest`-based + // tests elsewhere in this suite use for the unix-socket listener. + const body = await tcpRequest(port, "127.0.0.1:1", "right-secret") + expect(body).toContain("Cannot reach") +}) + +test("a TCP request with no Proxy-Authorization is refused with 407 and never reaches the dial or the allowlist check", async () => { + const port = tcpProxy(["127.0.0.1"], "right-secret").port + const body = await tcpRequest(port, "127.0.0.1:1") + expect(body).toContain("407 Proxy Authentication Required") + expect(body).not.toContain("sandbox allowlist") + expect(body).not.toContain("Cannot reach") +}) + +test("a TCP request with the wrong secret is refused with 407", async () => { + const port = tcpProxy(["127.0.0.1"], "right-secret").port + const body = await tcpRequest(port, "127.0.0.1:1", "wrong-secret") + expect(body).toContain("407 Proxy Authentication Required") +}) + +test("the unix-socket listener requires no Proxy-Authorization — auth is TCP-only", async () => { + // Regression guard for the other direction: adding auth to the TCP branch + // must not leak onto bubblewrap's unix socket, which has no `secret` to + // check in the first place — filesystem permissions on the path are its + // access control (unchanged Linux behaviour this task must not regress). + const socket = proxy(["127.0.0.1"]) + const body = await proxyRequestNoAuth(socket, "127.0.0.1:1") + expect(body).not.toContain("407") + expect(body).toContain("Cannot reach") +}) + +function proxyRequestNoAuth(socket: string, authority: string): Promise { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error(`no response for ${authority}`)), 5_000) + let body = "" + Bun.connect({ + unix: socket, + socket: { + open(sock) { + sock.write(`CONNECT ${authority} HTTP/1.1\r\nHost: ${authority}\r\n\r\n`) + }, + data(_sock, chunk) { + body += chunk.toString() + }, + close() { + clearTimeout(timeout) + resolve(body) + }, + error(_sock, error) { + clearTimeout(timeout) + reject(error) + }, + }, + }).catch(reject) + }) +} diff --git a/backend/cli/test/sandbox/sandbox.test.ts b/backend/cli/test/sandbox/sandbox.test.ts index f1f3d40e..886e4c38 100644 --- a/backend/cli/test/sandbox/sandbox.test.ts +++ b/backend/cli/test/sandbox/sandbox.test.ts @@ -1,40 +1,42 @@ import { describe, expect, test } from "bun:test" import fs from "fs" +import { builtinModules } from "module" import os from "os" import path from "path" import { Sandbox } from "../../src/sandbox/sandbox" +import { SHIM_READY_MARKER } from "../../src/sandbox/egress-shim-marker" import { tmpdir } from "../fixture/fixture" const shell = "/bin/sh" describe("Sandbox.seatbeltProfile", () => { test("denies writes by default and re-allows the workspace", () => { - const profile = Sandbox.seatbeltProfile({ writable: ["/work/project"], network: true }) + const profile = Sandbox.seatbeltProfile({ writable: ["/work/project"], network: "allow" }) expect(profile).toContain("(version 1)") expect(profile).toContain("(allow default)") expect(profile).toContain("(deny file-write*)") expect(profile).toContain('(subpath "/work/project")') }) - test("network:false adds a network deny; network:true does not", () => { - expect(Sandbox.seatbeltProfile({ writable: ["/w"], network: false })).toContain("(deny network*)") - expect(Sandbox.seatbeltProfile({ writable: ["/w"], network: true })).not.toContain("(deny network*)") + test('network:"deny" adds a network deny; network:"allow" does not', () => { + expect(Sandbox.seatbeltProfile({ writable: ["/w"], network: "deny" })).toContain("(deny network*)") + expect(Sandbox.seatbeltProfile({ writable: ["/w"], network: "allow" })).not.toContain("(deny network*)") }) test("a path outside the allowlist is not granted write access", () => { - const profile = Sandbox.seatbeltProfile({ writable: ["/work/project"], network: true }) + const profile = Sandbox.seatbeltProfile({ writable: ["/work/project"], network: "allow" }) expect(profile).not.toContain('(subpath "/etc/passwd")') expect(profile).not.toContain(process.env.HOME + "/.ssh") }) test("adds the macOS /private firmlink alias for /tmp", () => { - const profile = Sandbox.seatbeltProfile({ writable: ["/tmp"], network: true }) + const profile = Sandbox.seatbeltProfile({ writable: ["/tmp"], network: "allow" }) expect(profile).toContain('(subpath "/tmp")') expect(profile).toContain('(subpath "/private/tmp")') }) test("escapes quotes in paths so the profile cannot be broken out of", () => { - const profile = Sandbox.seatbeltProfile({ writable: ['/weird/pa"th'], network: true }) + const profile = Sandbox.seatbeltProfile({ writable: ['/weird/pa"th'], network: "allow" }) expect(profile).toContain('/weird/pa\\"th') }) @@ -42,7 +44,7 @@ describe("Sandbox.seatbeltProfile", () => { const profile = Sandbox.seatbeltProfile({ writable: ["/work/project"], unreadable: ["/home/user/.config/atlas-cli/config.json"], - network: true, + network: "allow", }) expect(profile).toContain('(deny file-read* (literal "/home/user/.config/atlas-cli/config.json"))') }) @@ -50,7 +52,7 @@ describe("Sandbox.seatbeltProfile", () => { describe("Sandbox.bubblewrapArgs", () => { test("mounts the fs read-only then re-binds the workspace writable", () => { - const args = Sandbox.bubblewrapArgs({ writable: ["/work/project"], network: true }) + const args = Sandbox.bubblewrapArgs({ writable: ["/work/project"], network: "allow" }) expect(args.slice(0, 3)).toEqual(["--ro-bind", "/", "/"]) // whole fs read-only first expect(args).toContain("--die-with-parent") const i = args.indexOf("--bind-try") @@ -59,13 +61,13 @@ describe("Sandbox.bubblewrapArgs", () => { expect(args[i + 2]).toBe("/work/project") }) - test("network:false unshares the network namespace", () => { - expect(Sandbox.bubblewrapArgs({ writable: ["/w"], network: false })).toContain("--unshare-net") - expect(Sandbox.bubblewrapArgs({ writable: ["/w"], network: true })).not.toContain("--unshare-net") + test('network:"deny" unshares the network namespace', () => { + expect(Sandbox.bubblewrapArgs({ writable: ["/w"], network: "deny" })).toContain("--unshare-net") + expect(Sandbox.bubblewrapArgs({ writable: ["/w"], network: "allow" })).not.toContain("--unshare-net") }) test("skips the /tmp tmpfs root but binds workspace paths under it", () => { - const args = Sandbox.bubblewrapArgs({ writable: ["/tmp", "/tmp/sub"], network: true }) + const args = Sandbox.bubblewrapArgs({ writable: ["/tmp", "/tmp/sub"], network: "allow" }) expect(args).toContain("--tmpfs") const binds = args.flatMap((a, n) => (a === "--bind-try" ? [args[n + 1]!] : [])) // the /tmp mount root itself is never bound from the host (the tmpfs provides it) @@ -76,7 +78,7 @@ describe("Sandbox.bubblewrapArgs", () => { }) test("unshares the PID namespace so /proc escape vectors are closed", () => { - expect(Sandbox.bubblewrapArgs({ writable: ["/w"], network: true })).toContain("--unshare-pid") + expect(Sandbox.bubblewrapArgs({ writable: ["/w"], network: "allow" })).toContain("--unshare-pid") }) test("masks host credential files with an empty device", () => { @@ -86,7 +88,7 @@ describe("Sandbox.bubblewrapArgs", () => { const args = Sandbox.bubblewrapArgs({ writable: ["/work/project"], unreadable: [file], - network: true, + network: "allow", }) const mask = args.findIndex((value, index) => value === "--ro-bind-try" && args[index + 1] === "/dev/null") expect(args.slice(mask, mask + 3)).toEqual(["--ro-bind-try", "/dev/null", file]) @@ -101,7 +103,7 @@ describe("Sandbox.bubblewrapArgs", () => { const args = Sandbox.bubblewrapArgs({ writable: ["/work/project"], unreadable: [file], - network: true, + network: "allow", }) expect(args).not.toContain(file) }) @@ -118,7 +120,7 @@ describe("Sandbox.bubblewrapArgs", () => { const args = Sandbox.bubblewrapArgs({ writable: [tmp.path], unreadable: [present, missing], - network: false, + network: "deny", }) const proc = Bun.spawn(["bwrap", ...args, "--", "/bin/echo", "ok"], { stdout: "pipe", stderr: "pipe" }) const [out, error, exit] = await Promise.all([ @@ -160,7 +162,9 @@ describe("Sandbox.plan", () => { }) test("enabled → sandboxed when a backend exists, else degrades", () => { - const p = Sandbox.plan({ ...base, options: { enabled: true } }) + // network is orthogonal to what this test checks; pin it to "allow" so the + // assertions below aren't coupled to the "allowlist" default's egress requirement + const p = Sandbox.plan({ ...base, options: { enabled: true, network: "allow" } }) if (Sandbox.available()) { expect(p.sandboxed).toBe(true) expect(["sandbox-exec", "bwrap"]).toContain(p.file) @@ -185,7 +189,7 @@ describe("Sandbox.plan", () => { shell, cwd: "/work/elsewhere", workspace: ["/work/project"], - options: { enabled: true }, + options: { enabled: true, network: "allow" }, }) const argv = (p.args ?? []).join(" ") expect(argv).toContain("/work/project") @@ -201,7 +205,7 @@ describe("Sandbox.plan", () => { shell, cwd: "/work/project", workspace: ["/work/project", "/"], - options: { enabled: true, allowWrite: [os.homedir()] }, + options: { enabled: true, network: "allow", allowWrite: [os.homedir()] }, }) const argv = (p.args ?? []).join(" ") expect(argv).toContain("/work/project") @@ -212,3 +216,861 @@ describe("Sandbox.plan", () => { expect(argv).not.toContain(`(subpath "${os.homedir()}")`) }) }) + +describe("Sandbox network policy", () => { + test("deny unshares the network and binds no socket", () => { + const args = Sandbox.bubblewrapArgs({ writable: ["/w"], network: "deny" }) + expect(args).toContain("--unshare-net") + expect(args.join(" ")).not.toContain(".sock") + }) + + test("allow neither unshares nor binds", () => { + const args = Sandbox.bubblewrapArgs({ writable: ["/w"], network: "allow" }) + expect(args).not.toContain("--unshare-net") + }) + + // The namespace must stay severed — the socket is the ONLY route out. If + // --unshare-net were dropped here the proxy would become advisory. + test("allowlist unshares the network AND binds the socket read-only", () => { + const args = Sandbox.bubblewrapArgs({ writable: ["/w"], network: "allowlist", egress: "/run/os/e.sock" }) + expect(args).toContain("--unshare-net") + const at = args.indexOf("/run/os/e.sock") + expect(at).toBeGreaterThan(0) + // --ro-bind, not --bind: the bind shares the host inode, so a read-write + // bind would let a sandboxed process `chmod 000` the socket and disable + // egress host-wide (persists past this process, shared by every + // kernel/terminal/job). Read-only blocks chmod while still permitting + // connect() — verified live in the fix-round report. + expect(args[at - 1]).toBe("--ro-bind") + }) + + test("allowlist without a socket path is refused rather than silently opened", () => { + expect(() => Sandbox.bubblewrapArgs({ writable: ["/w"], network: "allowlist" })).toThrow() + }) + + // buildPolicy filters `egress` through the same tooBroadToConfine gate as + // `writable`/`unreadable`, normalized the same way (dedupe()'s path.resolve()) + // before the gate sees it — so a lexical variant of an over-broad path (a + // trailing slash, a double slash, an unresolved "..") can't slip past the + // gate's string checks the way the raw string comparison once did. An + // over-broad egress must never reach argv as a --bind at all — even + // read-only, that would expose the whole subtree's contents, not just + // widen network access. Proven previously by calling bubblewrapArgs + // directly with an unfiltered + // `egress: $HOME`, which emitted "--bind $HOME $HOME" and let a sandboxed + // write escape to the real $HOME — and, before normalization was added, the + // exact same escape via `egress: $HOME + "/"` (a trailing slash was enough + // to dodge the raw string check). Going through the public plan() (which + // runs buildPolicy) instead: the over-broad path is dropped, so "allowlist" + // is left without an egress socket and refuses to run — it fails closed + // rather than silently binding it, for the whole class of lexical variants, + // not just the one literal string. + test.each([ + ["exact", os.homedir()], + ["trailing slash", os.homedir() + "/"], + ["double slash", os.homedir() + "//"], + ["unresolved ..", os.homedir() + "/foo/.."], + ["root", "/"], + ])("an over-broad egress path (%s) is dropped, not bound as a read-write escape hatch", (_label, egress) => { + // platform "linux", not the ambient one: this asserts bubblewrap argv, + // and on a darwin runner the same call reaches `seatbeltProfile` and + // throws for an entirely different reason ("requires an egress port"), + // which a bare .toThrow() would have accepted as a pass. Asserting the + // message closes that hole for good. + expect(() => + Sandbox.plan({ + command: "true", + shell, + cwd: "/work/project", + workspace: ["/work/project"], + options: { enabled: true, network: "allowlist", egress }, + platform: "linux", + }), + ).toThrow("requires an egress socket path") + }) + + test("a legitimate, non-broad egress socket is still bound, read-only", () => { + const p = Sandbox.plan({ + command: "true", + shell, + cwd: "/work/project", + workspace: ["/work/project"], + options: { enabled: true, network: "allowlist", egress: "/run/os/e.sock" }, + platform: "linux", + }) + expect(p.args).toContain("/run/os/e.sock") + const at = (p.args ?? []).indexOf("/run/os/e.sock") + expect((p.args ?? [])[at - 1]).toBe("--ro-bind") + }) + + // Live regression for the read-only bind's actual purpose: the egress + // socket is one-per-CLI-process, shared by every kernel/terminal/job, so a + // sandboxed process that could `chmod 000` it would disable egress + // host-wide until restart (the bind shares the host inode, so the mode + // change persists on the host — even the host proxy can no longer + // connect()). Runs a real bwrap with the exact args bubblewrapArgs() + // produces (not the full shim/proxy plan() composes, which is exercised + // elsewhere) to prove both properties of --ro-bind at once: chmod fails + // closed inside the sandbox, and a plain client can still connect() + // through the same bind. + const python = Bun.which("python3") ?? Bun.which("python") + test.skipIf(Sandbox.backend() !== "bubblewrap" || !python)( + "the sandboxed process can connect through the egress bind but cannot chmod it", + async () => { + await using tmp = await tmpdir() + const sockPath = path.join(tmp.path, "e.sock") + const received: string[] = [] + const server = Bun.listen({ + unix: sockPath, + socket: { + data(sock, chunk) { + received.push(chunk.toString()) + sock.write(`ACK:${chunk.toString()}`) + }, + }, + }) + try { + const args = Sandbox.bubblewrapArgs({ writable: [tmp.path], network: "allowlist", egress: sockPath }) + + const chmod = Bun.spawnSync({ + cmd: ["bwrap", ...args, "--", "chmod", "000", sockPath], + stdout: "pipe", + stderr: "pipe", + }) + expect(chmod.exitCode).not.toBe(0) + expect(chmod.stderr.toString()).toContain("Read-only file system") + // Mode must be unchanged on the host — the whole point of --ro-bind. + expect(fs.statSync(sockPath).mode & 0o777).toBeGreaterThan(0) + + // Bun.spawn, not spawnSync: the server above replies from a Bun.listen + // "data" callback on this same event loop, so a *synchronous* spawn + // would block that loop for as long as the child runs — the child + // blocks on recv() waiting for a reply the loop can't yet deliver, + // deadlocking both sides until the test times out (reproduced while + // writing this test). + const clientSource = [ + "import socket", + "s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)", + `s.connect(${JSON.stringify(sockPath)})`, + "s.send(b'hi')", + "print(s.recv(1024).decode(), end='')", + ].join("\n") + const connectProc = Bun.spawn({ + cmd: ["bwrap", ...args, "--", python!, "-c", clientSource], + stdout: "pipe", + stderr: "pipe", + }) + const [connectOut, connectErr] = await Promise.all([ + new Response(connectProc.stdout).text(), + new Response(connectProc.stderr).text(), + ]) + await connectProc.exited + expect(connectOut, connectErr).toBe("ACK:hi") + expect(received).toContain("hi") + } finally { + server.stop(true) + } + }, + ) + + // Seatbelt has no namespace, so bwrap's --unshare-net has no equivalent + // here: the profile text itself is the only boundary. "allowlist" is + // therefore carried by Policy.port, not Policy.egress (that field stays + // bubblewrap's unix socket path — see Policy's doc comment). + // + // Task 7 fix round 1, I3: this used to emit only network-outbound, spelled + // (remote ip ...). docs/adr/0002-sandbox-network-policy.md:56-59 records + // the reference implementation as permitting network-bind/network-inbound/ + // network-outbound, all narrowed to the proxy's loopback port, spelled tcp + // — a narrower, unmeasured guess on the one platform this project cannot + // execute against is exactly the failure mode that makes "allowlist" + // silently unreachable on every real Mac. See seatbeltProfile's own doc + // comment for why network-bind/network-inbound are included even though + // this sandboxed process is only ever a TCP client, never a listener. + test("allowlist with a port emits deny before all three narrow allows, spelled tcp", () => { + const profile = Sandbox.seatbeltProfile({ writable: ["/w"], network: "allowlist", port: 54321 }) + const deny = profile.indexOf("(deny network*)") + expect(deny).toBeGreaterThan(-1) + for (const line of [ + '(allow network-bind (local tcp "localhost:54321"))', + '(allow network-inbound (local tcp "localhost:54321"))', + '(allow network-outbound (remote tcp "localhost:54321"))', + ]) { + const at = profile.indexOf(line) + expect(at).toBeGreaterThan(deny) + } + }) + + // The safety rule from the task brief: a missing/invalid port must never + // silently downgrade to a plain deny (which would look identical to a user + // asking for network:"deny", not what "allowlist" means) or, worse, to an + // unfiltered allow. Same fail-closed contract bubblewrapArgs already + // applies to a missing egress socket. + test("allowlist with no port throws rather than silently degrading to deny", () => { + expect(() => Sandbox.seatbeltProfile({ writable: ["/w"], network: "allowlist" })).toThrow( + "sandbox network 'allowlist' requires an egress port", + ) + }) + + // Task 7 fix round 1, M5: 65536 and above are not valid TCP ports at all; + // an unbounded check let them through and would have composed a profile + // narrowing egress to a port number that can never exist. + test.each([ + ["zero", 0], + ["negative", -1], + ["fractional", 3.5], + ["one past the max valid port", 65536], + ["absurdly large", 1e21], + ])("allowlist with an invalid port (%s) throws", (_label, port) => { + expect(() => Sandbox.seatbeltProfile({ writable: ["/w"], network: "allowlist", port })).toThrow( + "sandbox network 'allowlist' requires an egress port", + ) + }) + + test("allowlist accepts the maximum valid port, 65535", () => { + expect(() => Sandbox.seatbeltProfile({ writable: ["/w"], network: "allowlist", port: 65535 })).not.toThrow() + }) + + // The dangerous direction named in the brief: a malformed or over-broad + // allow is the only failure mode that makes a macOS user worse off than + // today's plain deny. Pin the exact shapes seatbeltProfile can produce — + // never a bare, unfiltered allow of any of the three network operations. + test("never emits an unfiltered network-bind/network-inbound/network-outbound allow in any mode", () => { + for (const network of ["deny", "allow"] as const) { + const profile = Sandbox.seatbeltProfile({ writable: ["/w"], network }) + expect(profile).not.toContain("(allow network-bind") + expect(profile).not.toContain("(allow network-inbound") + expect(profile).not.toContain("(allow network-outbound") + } + const profile = Sandbox.seatbeltProfile({ writable: ["/w"], network: "allowlist", port: 4000 }) + const lines = profile.split("\n").filter((line) => /network-(bind|inbound|outbound)/.test(line)) + expect(lines).toEqual([ + '(allow network-bind (local tcp "localhost:4000"))', + '(allow network-inbound (local tcp "localhost:4000"))', + '(allow network-outbound (remote tcp "localhost:4000"))', + ]) + }) + + // Pins the deny/allow branches byte-for-byte: Policy.port only ever + // affects the "allowlist" branch, so these two must come out exactly as + // they did before this field existed. + test("deny and allow profiles are unaffected by the allowlist port machinery", () => { + const deny = Sandbox.seatbeltProfile({ writable: ["/w"], network: "deny" }) + expect(deny.split("\n")).toEqual([ + "(version 1)", + "(allow default)", + "(deny network*)", + "(deny file-write*)", + '(allow file-write* (subpath "/w"))', + '(allow file-write* (subpath "/dev"))', + ]) + const allow = Sandbox.seatbeltProfile({ writable: ["/w"], network: "allow" }) + expect(allow.split("\n")).toEqual([ + "(version 1)", + "(allow default)", + "(deny file-write*)", + '(allow file-write* (subpath "/w"))', + '(allow file-write* (subpath "/dev"))', + ]) + }) +}) + +describe("Sandbox.backend(platform)", () => { + // The injectable seam every darwin-only assertion in this branch depends + // on: nobody on this project can install sandbox-exec, so exercising the + // seatbelt code paths in plan()/wrapArgv()/EgressRuntime from Linux is + // only possible if `platform` overrides the real, probed detection below. + test("darwin resolves to seatbelt regardless of the machine actually running the test", () => { + expect(Sandbox.backend("darwin")).toBe("seatbelt") + }) + + test("linux resolves to bubblewrap regardless of the machine actually running the test", () => { + expect(Sandbox.backend("linux")).toBe("bubblewrap") + }) + + test("an unsupported platform resolves to none", () => { + expect(Sandbox.backend("win32")).toBe("none") + }) + + // The doc comment's exact claim: an explicit platform that matches the + // real one is the same code path as the zero-arg call, not a parallel + // implementation that could drift from the probed one. + test("an explicitly-matching platform is identical to the zero-arg call", () => { + expect(Sandbox.backend(process.platform)).toBe(Sandbox.backend()) + }) +}) + +describe("Sandbox.plan/wrapArgv on darwin (seatbelt)", () => { + // ":" — the shape EgressRuntime.egressFor() produces for + // seatbelt (see egress-runtime.ts); buildPolicy splits it back into + // Policy.port/Policy.secret. + const port = "54321" + const secret = "topsecret123" + const egress = `${port}:${secret}` + + test("plan composes no shim and dials the proxy port directly, with the secret in the proxy URL", () => { + const p = Sandbox.plan({ + command: "echo hi", + shell, + cwd: "/work/project", + workspace: ["/work/project"], + options: { enabled: true, network: "allowlist", egress }, + platform: "darwin", + }) + expect(p.sandboxed).toBe(true) + expect(p.backend).toBe("seatbelt") + expect(p.file).toBe("sandbox-exec") + const argv = (p.args ?? []).join(" ") + // No unix-socket shim exists on darwin: no launcher, no bundle, no + // __egress-shim marker — the real command runs directly under sandbox-exec. + expect(argv).not.toContain("__egress-shim") + expect(argv).toContain("echo hi") + // A loopback TCP port carries no filesystem permissions of its own (a + // unix socket does), so the URL embeds the per-start secret as userinfo + // — pip/curl/requests all parse this into Proxy-Authorization. + expect(p.env?.HTTP_PROXY).toBe(`http://os:${secret}@127.0.0.1:${port}`) + expect(p.env?.http_proxy).toBe(p.env?.HTTP_PROXY) + }) + + test("wrapArgv composes no shim and dials the proxy port directly, with the secret in the proxy URL", () => { + const w = Sandbox.wrapArgv({ + file: "python3", + args: ["-u", "/tmp/k.py"], + workspace: ["/work/project"], + options: { enabled: true, network: "allowlist", egress }, + platform: "darwin", + }) + expect(w.sandboxed).toBe(true) + expect(w.backend).toBe("seatbelt") + const argv = w.args.join(" ") + expect(argv).not.toContain("__egress-shim") + expect(argv).toContain("python3") + expect(w.env?.HTTP_PROXY).toBe(`http://os:${secret}@127.0.0.1:${port}`) + }) + + test("allowlist with no egress port throws rather than silently degrading to deny", () => { + expect(() => + Sandbox.wrapArgv({ + file: "python3", + args: ["-u", "/tmp/k.py"], + workspace: ["/work/project"], + options: { enabled: true, network: "allowlist" }, + platform: "darwin", + }), + ).toThrow() + }) + + // A port with no secret is exactly as fail-closed as no port at all — see + // buildPolicy's doc comment: the two are validated and dropped together, + // so a malformed "port with no secret" pairing can never compose a proxy + // URL missing the credential the darwin listener requires. + test("allowlist with a port but no secret (malformed egress) throws, not an unauthenticated proxy URL", () => { + expect(() => + Sandbox.wrapArgv({ + file: "python3", + args: ["-u", "/tmp/k.py"], + workspace: ["/work/project"], + options: { enabled: true, network: "allowlist", egress: `${port}:` }, + platform: "darwin", + }), + ).toThrow() + }) + + test("deny and allow never compose a shim or set a proxy env on darwin", () => { + const deny = Sandbox.wrapArgv({ + file: "python3", + args: ["-u", "/tmp/k.py"], + workspace: ["/work/project"], + options: { enabled: true, network: "deny" }, + platform: "darwin", + }) + expect(deny.args.join(" ")).not.toContain("__egress-shim") + expect(deny.env).toBeUndefined() + + const allow = Sandbox.wrapArgv({ + file: "python3", + args: ["-u", "/tmp/k.py"], + workspace: ["/work/project"], + options: { enabled: true, network: "allow" }, + platform: "darwin", + }) + expect(allow.args.join(" ")).not.toContain("__egress-shim") + expect(allow.env).toBeUndefined() + }) + + // Regression guard for the real-platform paths: an explicit platform that + // matches this machine's own must not diverge from the zero-arg call — + // "allow" keeps this cheap (no shim/proxy machinery) while still routing + // through decide()/buildPolicy() with a platform argument threaded in. + test("an explicitly-matching platform reproduces the zero-arg plan on this machine", () => { + const base = { + command: "echo hi", + shell, + cwd: "/work/project", + workspace: ["/work/project"], + options: { enabled: true, network: "allow" as const }, + } + expect(Sandbox.plan({ ...base, platform: process.platform })).toEqual(Sandbox.plan(base)) + }) +}) + +describe("Sandbox.shimScript", () => { + test("backgrounds the shim and execs the real command", () => { + const script = Sandbox.shimScript({ + binary: "/usr/bin/openscience", + port: 3128, + socket: "/run/os/e.sock", + file: "python3", + args: ["-u", "/tmp/k.py"], + }) + expect(script).toContain("__egress-shim") + expect(script).toContain("&") + expect(script).toContain("exec ") + }) + + test("quotes every interpolated value so a path with a space cannot split", () => { + const script = Sandbox.shimScript({ + binary: "/opt/my apps/openscience", + port: 3128, + socket: "/run/my dir/e.sock", + file: "python3", + args: ["-c", "print('hi there')"], + }) + expect(script).toContain("'/opt/my apps/openscience'") + expect(script).toContain("'/run/my dir/e.sock'") + expect(script).not.toMatch(/[^']\/opt\/my apps/) + }) + + test("a single quote in an argument cannot break out of the quoting", () => { + const script = Sandbox.shimScript({ + binary: "/usr/bin/openscience", + port: 3128, + socket: "/run/os/e.sock", + file: "python3", + args: ["-c", "x = 'a'; print(x)"], + }) + expect(script).toContain(`'"'"'`) + }) +}) + +// The readiness wait is the whole per-spawn cost of network "allowlist", and +// it is paid by every sandboxed command whether or not it touches the network. +// These run the composed script through a real /bin/sh — the only place its +// behaviour actually lives — with a stand-in for the shim binary, so they need +// no bubblewrap and no proxy. +describe("Sandbox.shimScript readiness wait", () => { + const posix = process.platform !== "win32" + + /** Runs the composed script and reports how long it took, plus whatever the + * wait leaked to the real command's stderr (it runs in the foreground, so + * anything it prints lands in the command's own output). */ + async function run(script: string, prefixPath?: string) { + const started = Date.now() + const proc = Bun.spawn(["/bin/sh", "-c", script], { + stdout: "pipe", + stderr: "pipe", + env: prefixPath ? { ...process.env, PATH: `${prefixPath}:${process.env["PATH"]}` } : process.env, + }) + const [stdout, stderr] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]) + await proc.exited + return { ms: Date.now() - started, stdout, stderr } + } + + /** A `sleep` that rejects fractional intervals the way some busybox builds + * do, so the fallback branch is exercised on a host whose real coreutils + * accepts them. */ + function busyboxishSleep(dir: string) { + const real = Bun.which("sleep") ?? "/bin/sleep" + const file = path.join(dir, "sleep") + fs.writeFileSync( + file, + `#!/bin/sh\ncase "$1" in\n *.*) echo "sleep: invalid number '$1'" >&2; exit 1 ;;\nesac\nexec ${real} "$@"\n`, + { mode: 0o755 }, + ) + return dir + } + + /** A `sleep` whose cost is dominated by process creation rather than by the + * interval asked for. A macOS CI runner measured ~114ms per iteration of + * the 0.02s poll — ~94ms of fork/exec — which stretched the nominal 3s cap + * to 17.1s and is what put the wall-clock deadline in `shimScript`. This + * reproduces that condition on any host. */ + function expensiveSleep(dir: string) { + const real = Bun.which("sleep") ?? "/bin/sleep" + const file = path.join(dir, "sleep") + fs.writeFileSync(file, `#!/bin/sh\nexec ${real} 0.12\n`, { mode: 0o755 }) + return dir + } + + test.skipIf(!posix)( + "waits for the shim, and only for as long as the shim takes", + async () => { + await using dir = await tmpdir() + // Stands in for the shim: ignores its arguments, becomes ready quickly. + const fake = path.join(dir.path, "shim") + fs.writeFileSync(fake, `#!/bin/sh\nsleep 0.15\n: > ${JSON.stringify(SHIM_READY_MARKER)}\n`, { mode: 0o755 }) + fs.rmSync(SHIM_READY_MARKER, { force: true }) + + try { + const { ms, stdout } = await run( + Sandbox.shimScript({ binary: fake, port: 3128, socket: "/run/os/e.sock", file: "/bin/echo", args: ["ran"] }), + ) + expect(stdout.trim()).toBe("ran") + // It really waited: the marker only lands at ~150ms. + expect(ms).toBeGreaterThanOrEqual(140) + // And it did not round that up to a whole second. Before the poll + // interval was chosen at run time this was a flat ~1.0s for a shim that + // is ready in ~12ms — measured 1006ms against 3ms for network "deny". + expect(ms).toBeLessThan(600) + } finally { + fs.rmSync(SHIM_READY_MARKER, { force: true }) + } + }, + 30_000, + ) + + test.skipIf(!posix)( + "a sleep that rejects fractions still waits, and says nothing about it", + async () => { + await using dir = await tmpdir() + fs.rmSync(SHIM_READY_MARKER, { force: true }) + // /bin/true ignores the shim arguments and never signals readiness, so the + // wait runs to its cap — which is the point: a fractional `sleep` that + // errors out returns instantly, so a loop that ignored the failure would + // spin through all its iterations in microseconds and skip the wait + // entirely, silently, while printing one error line per iteration. + const { ms, stderr } = await run( + Sandbox.shimScript({ + binary: "/bin/true", + port: 3128, + socket: "/run/os/e.sock", + file: "/bin/echo", + args: ["ran"], + }), + busyboxishSleep(dir.path), + ) + expect(stderr).toBe("") + expect(ms).toBeGreaterThanOrEqual(2_500) + expect(ms).toBeLessThan(6_000) + }, + 30_000, + ) + + test.skipIf(!posix)( + "the cap is the same 3s whichever granularity the shell supports", + async () => { + fs.rmSync(SHIM_READY_MARKER, { force: true }) + const { ms } = await run( + Sandbox.shimScript({ + binary: "/bin/true", + port: 3128, + socket: "/run/os/e.sock", + file: "/bin/echo", + args: ["ran"], + }), + ) + expect(ms).toBeGreaterThanOrEqual(2_500) + expect(ms).toBeLessThan(6_000) + }, + 30_000, + ) + + // The regression this file's macOS run found: the cap used to be an + // iteration count, so it only equalled 3s where forking `sleep` was nearly + // free. Without the deadline, 150 iterations at 0.12s each run for 18s. + test.skipIf(!posix)( + "a `sleep` whose real cost is fork/exec cannot stretch the cap", + async () => { + await using dir = await tmpdir() + fs.rmSync(SHIM_READY_MARKER, { force: true }) + const { ms, stderr } = await run( + Sandbox.shimScript({ + binary: "/bin/true", + port: 3128, + socket: "/run/os/e.sock", + file: "/bin/echo", + args: ["ran"], + }), + expensiveSleep(dir.path), + ) + // Still silent: the deadline probe's own diagnostics are discarded the + // same way the fractional-sleep probe's are. + expect(stderr).toBe("") + expect(ms).toBeGreaterThanOrEqual(2_500) + expect(ms).toBeLessThan(6_000) + }, + 30_000, + ) +}) + +describe("Sandbox.wrapArgv egress shim", () => { + test.skipIf(Sandbox.backend() !== "bubblewrap")( + "allowlist composes the shim into the argv and returns proxy env", + () => { + const wrapped = Sandbox.wrapArgv({ + file: "python3", + args: ["-u", "/tmp/k.py"], + workspace: ["/work/project"], + options: { enabled: true, network: "allowlist", egress: "/run/os/e.sock" }, + }) + expect(wrapped.sandboxed).toBe(true) + const argv = wrapped.args.join(" ") + expect(argv).toContain("__egress-shim") + expect(argv).toContain("/run/os/e.sock") + expect(argv).toContain("exec 'python3'") + expect(wrapped.env?.HTTP_PROXY).toMatch(/^http:\/\/127\.0\.0\.1:\d+$/) + expect(wrapped.env?.http_proxy).toBe(wrapped.env?.HTTP_PROXY) + }, + ) + + test.skipIf(Sandbox.backend() !== "bubblewrap")("deny and allow never compose the shim", () => { + const deny = Sandbox.wrapArgv({ + file: "python3", + args: ["-u", "/tmp/k.py"], + workspace: ["/work/project"], + options: { enabled: true, network: "deny" }, + }) + expect(deny.args.join(" ")).not.toContain("__egress-shim") + expect(deny.env).toBeUndefined() + + const allow = Sandbox.wrapArgv({ + file: "python3", + args: ["-u", "/tmp/k.py"], + workspace: ["/work/project"], + options: { enabled: true, network: "allow" }, + }) + expect(allow.args.join(" ")).not.toContain("__egress-shim") + expect(allow.env).toBeUndefined() + }) + + // Everything above asserts on the composed argv string without ever running + // it — which is exactly what let the shim silently die inside the real + // sandbox (EROFS from the CLI's logging middleware) go undetected. This + // spawns the actual `wrapArgv` output — real bwrap, the real composed + // `sh -c` script, and (in dev, which this test runs as) the real on-disk + // launcher `shimPlan()` writes — and proves a TCP client inside the + // namespace gets a connection accepted and bridged to the unix socket, not + // a refused one. Resolved once, from the same gate the skip condition uses + // — bash lives at /bin/bash on Alpine and non-usrmerge Debian, not + // /usr/bin/bash, and a hardcoded path there would fail instead of skip. + const bash = Bun.which("bash") + test.skipIf(Sandbox.backend() !== "bubblewrap" || !bash || !Bun.which("timeout") || !Bun.which("head"))( + "the composed script actually starts the shim inside a real sandbox and bridges a connection", + async () => { + await using tmp = await tmpdir() + const socket = path.join(tmp.path, "e.sock") + const received: string[] = [] + const server = Bun.listen({ + unix: socket, + socket: { + data(sock, chunk) { + received.push(chunk.toString()) + sock.write(`ACK:${chunk.toString()}`) + }, + }, + }) + try { + const wrapped = Sandbox.wrapArgv({ + file: bash!, + args: ["-c", "exec 3<>/dev/tcp/127.0.0.1/3128; printf hello >&3; timeout 3 head -c 9 <&3"], + workspace: [tmp.path], + options: { enabled: true, network: "allowlist", egress: socket }, + }) + const proc = Bun.spawn([wrapped.file, ...wrapped.args], { stdout: "pipe", stderr: "pipe" }) + const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]) + await proc.exited + expect(out, `stdout=${out} stderr=${err}`).toContain("ACK:hello") + expect(received).toContain("hello") + } finally { + server.stop(true) + } + }, + 15000, + ) + + // Regression guard for the bug that survived two consecutive rounds: the + // interpreter the launcher execs (`process.execPath`) can itself live + // under /tmp — a portable bun install, `$HOME` under /tmp — independent of + // where the checkout or Global.Path.bin happen to be. `shimPlan()` reads + // `process.execPath` from the *current* process, so the only way to + // actually exercise this is to run under a /tmp-staged bun. Stages a real + // copy (a symlink wouldn't reproduce — Bun resolves it) and drives a small + // standalone script through it, since `bun:test` itself isn't the thing + // under test here. + test.skipIf(Sandbox.backend() !== "bubblewrap" || !bash || !Bun.which("timeout") || !Bun.which("head"))( + "the composed script still starts the shim when the interpreter itself is staged under /tmp", + async () => { + const stage = fs.mkdtempSync(path.join(os.tmpdir(), "staged-bun-")) + const stagedBun = path.join(stage, "bun") + const driver = path.join(stage, "driver.ts") + try { + fs.copyFileSync(process.execPath, stagedBun) + fs.chmodSync(stagedBun, 0o755) + const sandboxPath = path.resolve(import.meta.dir, "..", "..", "src", "sandbox", "sandbox.ts") + fs.writeFileSync( + driver, + [ + `import path from "path"`, + `import fs from "fs"`, + `import os from "os"`, + `import { Sandbox } from ${JSON.stringify(sandboxPath)}`, + `const work = fs.mkdtempSync(path.join(os.tmpdir(), "staged-bun-driver-"))`, + `const socket = path.join(work, "e.sock")`, + `const server = Bun.listen({ unix: socket, socket: { data(sock, chunk) { sock.write("ACK:" + chunk) } } })`, + `try {`, + ` const wrapped = Sandbox.wrapArgv({`, + ` file: ${JSON.stringify(bash)},`, + ` args: ["-c", "exec 3<>/dev/tcp/127.0.0.1/3128; printf hello >&3; timeout 3 head -c 9 <&3"],`, + ` workspace: [work],`, + ` options: { enabled: true, network: "allowlist", egress: socket },`, + ` })`, + ` const proc = Bun.spawn([wrapped.file, ...wrapped.args], { stdout: "pipe", stderr: "pipe" })`, + ` const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()])`, + ` await proc.exited`, + ` console.log(out.includes("ACK:hello") ? "STAGED_BUN_PASS" : "STAGED_BUN_FAIL " + JSON.stringify({ out, err }))`, + `} finally {`, + ` server.stop(true)`, + ` fs.rmSync(work, { recursive: true, force: true })`, + `}`, + ].join("\n"), + ) + const proc = Bun.spawn([stagedBun, "run", driver], { stdout: "pipe", stderr: "pipe" }) + const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]) + await proc.exited + expect(out, `stdout=${out} stderr=${err}`).toContain("STAGED_BUN_PASS") + } finally { + fs.rmSync(stage, { recursive: true, force: true }) + } + }, + 20000, + ) + + // Regression guard for the fifth variant of "a path the shim needs is + // masked by --tmpfs /tmp": an npm import in the shim's graph resolves + // through a node_modules symlink whose target is the monorepo-root store, + // above the package root, so binding the package root left the target + // unbound. Reproducing that directly needs a /tmp-relocated checkout with a + // real hoisted store — a fixture too elaborate to keep honest here. These + // two tests assert the property that makes the whole class impossible + // instead: the shim resolves nothing from disk at run time. + // + // First, statically, on the artifact `shimPlan()` actually generated: a + // bundle with no import specifiers left in it cannot resolve anything, + // whether the import was a sibling file or a package. Builtins are allowed + // through — they come from inside bun, not the filesystem — so this keeps + // passing if the shim ever imports node:net, and fails if a future import + // is left external or the plan goes back to executing source. It does not + // see a native binding: `dlopen("…so")` in a bundled dependency is not an + // import specifier (see shimPlan's residual list). + // + // Classifying a specifier is the whole guard, so it is done against the + // real builtin list rather than by prefix. `bun build` *strips* the node: + // prefix — a bundled `import "node:net"` comes out as `from "net"` — so a + // prefix test flags a legitimate builtin, and the natural reaction to that + // false alarm is to loosen the one check standing between here and variant + // six. In the other direction a bare /^bun/ would quietly excuse any + // package named bun-something. builtinModules already carries bun's own + // entries (bun, bun:ffi, …), so stripping node: and asking it is both + // directions at once. + test.skipIf(Sandbox.backend() !== "bubblewrap")("the generated dev shim bundle resolves nothing from disk", () => { + const wrapped = Sandbox.wrapArgv({ + file: "python3", + args: ["-u", "/tmp/k.py"], + workspace: ["/work/project"], + options: { enabled: true, network: "allowlist", egress: "/run/os/e.sock" }, + }) + const bundle = wrapped.args.find((value) => value.endsWith(".mjs")) + expect(bundle, `argv=${wrapped.args.join(" ")}`).toBeDefined() + const source = fs.readFileSync(bundle!, "utf8") + // The three shapes an unbundled dependency can survive as: `from "x"` + // (covers `import x from` and `export … from`), a call — `require("x")`, + // `import("x")` — and a bare side-effect `import "x"`. + const found = [ + ...source.matchAll(/\bfrom\s*"([^"]+)"|\b(?:require|import)\(\s*"([^"]+)"\s*\)|\bimport\s*"([^"]+)"/g), + ] + const builtin = new Set(builtinModules) + const external = found + .map((match) => match[1] ?? match[2] ?? match[3]!) + .filter((spec) => !builtin.has(spec.replace(/^node:/, ""))) + expect(external).toEqual([]) + }) + + // Second, live: mask the shim's own source entry with /dev/null (the + // sandbox's existing `unreadable` mechanism) and run the real composed + // script anyway. If the shim still bridges, nothing it ran came from the + // source tree — which is what makes where that tree lives, and what it + // imports, irrelevant. Executing the source instead reads an empty file, + // starts no listener, and the connection is refused (verified: pointing the + // launcher back at the entry fails this test in 3s, the readiness cap). + // The mask only holds because readBind is files: a readBind *directory* + // over the entry would re-expose it and this test would pass regardless, + // which is why the static check above is the one that pins the bundle. + test.skipIf(Sandbox.backend() !== "bubblewrap" || !bash || !Bun.which("timeout") || !Bun.which("head"))( + "the composed script still starts the shim when its own source entry is masked", + async () => { + await using tmp = await tmpdir() + const socket = path.join(tmp.path, "e.sock") + const server = Bun.listen({ + unix: socket, + socket: { + data(sock, chunk) { + sock.write(`ACK:${chunk.toString()}`) + }, + }, + }) + try { + const wrapped = Sandbox.wrapArgv({ + file: bash!, + args: ["-c", "exec 3<>/dev/tcp/127.0.0.1/3128; printf hello >&3; timeout 3 head -c 9 <&3"], + workspace: [tmp.path], + unreadable: [path.resolve(import.meta.dir, "..", "..", "src", "sandbox", "egress-shim-entry.ts")], + options: { enabled: true, network: "allowlist", egress: socket }, + }) + const proc = Bun.spawn([wrapped.file, ...wrapped.args], { stdout: "pipe", stderr: "pipe" }) + const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]) + await proc.exited + expect(out, `stdout=${out} stderr=${err}`).toContain("ACK:hello") + } finally { + server.stop(true) + } + }, + 15000, + ) + + // End-to-end guard on the property Important B was about: self-hosting — + // opening OpenScience on its own checkout, the case Task 5 will dogfood — + // must not lose write access to part of the workspace just because the + // sandbox also needs some path bound read-only. It caught a real bug when + // shimPlan's readBind held the package root and bubblewrapArgs emitted it + // after the writable --bind-try loop. That trigger is gone (readBind is now + // two generated files plus the interpreter, none inside a checkout), so the + // test no longer has a failing negative control — it passes by the bind set + // being small rather than by the overlap exclusion doing anything. Kept as + // the assertion that the property still holds however the set changes. + test.skipIf(Sandbox.backend() !== "bubblewrap" || !bash)( + "a workspace path under the package root stays writable under allowlist", + async () => { + await using tmp = await tmpdir() + const socket = path.join(tmp.path, "e.sock") + const server = Bun.listen({ unix: socket, socket: { data() {} } }) + const packageRoot = path.resolve(import.meta.dir, "..", "..") + const probe = path.join(packageRoot, "src", "sandbox", `.regression-write-probe-${process.pid}`) + try { + const wrapped = Sandbox.wrapArgv({ + file: bash!, + args: ["-c", `echo probe > '${probe}' && cat '${probe}' && rm '${probe}' && echo WRITE_OK`], + workspace: [packageRoot], + options: { enabled: true, network: "allowlist", egress: socket }, + }) + const proc = Bun.spawn([wrapped.file, ...wrapped.args], { stdout: "pipe", stderr: "pipe" }) + const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]) + await proc.exited + expect(out, `stdout=${out} stderr=${err}`).toContain("WRITE_OK") + } finally { + server.stop(true) + fs.rmSync(probe, { force: true }) + } + }, + 15000, + ) +}) diff --git a/backend/cli/test/server/notebook.test.ts b/backend/cli/test/server/notebook.test.ts index ec75bc61..63395356 100644 --- a/backend/cli/test/server/notebook.test.ts +++ b/backend/cli/test/server/notebook.test.ts @@ -190,7 +190,7 @@ describe("/notebook routes", () => { requested: expect.any(Boolean), enforced: expect.any(Boolean), backend: expect.any(String), - network: expect.stringMatching(/^(allow|deny)$/), + network: expect.stringMatching(/^(allow|allowlist|deny)$/), platform: process.platform, }, }) @@ -377,7 +377,10 @@ describe("/notebook routes", () => { const waitForStarting = async (attempt = 0): Promise => { const result = (await (await status()).json()) as { state?: string } if (result.state === "starting") return - if (attempt >= 100) throw new Error("kernel did not start") + // 500 * 10ms = 5s: covers the loopback shim's own up-to-3s readiness + // wait under sandbox network "allowlist" (the default), plus normal + // interpreter startup, with margin. See sandbox.ts's shimScript. + if (attempt >= 500) throw new Error("kernel did not start") await Bun.sleep(10) return waitForStarting(attempt + 1) } @@ -445,7 +448,10 @@ describe("/notebook routes", () => { ) const status = (await response.json()) as { active?: boolean } if (status.active) return - if (attempt >= 100) throw new Error("kernel did not start") + // 500 * 10ms = 5s: covers the loopback shim's own up-to-3s readiness + // wait under sandbox network "allowlist" (the default), plus normal + // interpreter startup, with margin. See sandbox.ts's shimScript. + if (attempt >= 500) throw new Error("kernel did not start") await Bun.sleep(10) return waitForKernel(attempt + 1) } @@ -728,7 +734,10 @@ describe("/notebook routes", () => { const waitForStarting = async (attempt = 0): Promise => { const result = (await (await status()).json()) as { state?: string } if (result.state === "starting") return - if (attempt >= 100) throw new Error("kernel did not start") + // 500 * 10ms = 5s: covers the loopback shim's own up-to-3s readiness + // wait under sandbox network "allowlist" (the default), plus normal + // interpreter startup, with margin. See sandbox.ts's shimScript. + if (attempt >= 500) throw new Error("kernel did not start") await Bun.sleep(10) return waitForStarting(attempt + 1) } @@ -794,7 +803,8 @@ describe("/notebook routes", () => { const waitForRunning = async (attempt = 0): Promise => { const response = (await (await status()).json()) as { state?: string } if (response.state === "running") return - if (attempt >= 100) throw new Error("kernel did not start running") + // See the "kernel did not start" budget above for why 500 * 10ms. + if (attempt >= 500) throw new Error("kernel did not start running") await Bun.sleep(10) return waitForRunning(attempt + 1) } @@ -864,7 +874,8 @@ describe("/notebook routes", () => { await app.request(`/kernels?sessionID=${encodeURIComponent(session.id)}`) ).json()) as typeof kernels if (current.kernels.find((value) => value.id === kernel.id)?.state === "running") return - if (attempt >= 100) throw new Error("kernel did not start running") + // See the "kernel did not start" budget above for why 500 * 10ms. + if (attempt >= 500) throw new Error("kernel did not start running") await Bun.sleep(10) return waitForRunning(attempt + 1) } diff --git a/backend/cli/test/shell/shell.test.ts b/backend/cli/test/shell/shell.test.ts index 427ef8f8..ddf79de6 100644 --- a/backend/cli/test/shell/shell.test.ts +++ b/backend/cli/test/shell/shell.test.ts @@ -88,8 +88,17 @@ test("killTree SIGKILLs a detached group even after its leader exits", async () exited: () => true, }), ) - expect(groupKill).toHaveBeenNthCalledWith(1, -4321, "SIGTERM") - expect(groupKill).toHaveBeenNthCalledWith(2, -4321, "SIGKILL") + // Filter to this test's own pid rather than asserting on the mock's + // absolute call order: process.kill is one process-wide global, and under + // a full suite run a real sandboxed spawn elsewhere can land its own + // (real, unrelated) killTree cleanup on this same mock while it's active, + // interleaving with these two calls by index without changing what this + // test is actually verifying — that ITS SIGTERM precedes ITS SIGKILL. + const own = groupKill.mock.calls.filter(([pid]) => pid === -4321) + expect(own).toEqual([ + [-4321, "SIGTERM"], + [-4321, "SIGKILL"], + ]) expect(proc.kill).not.toHaveBeenCalled() } finally { groupKill.mockRestore() diff --git a/backend/cli/test/tool/command-runtime.test.ts b/backend/cli/test/tool/command-runtime.test.ts index d04764c0..0e284f37 100644 --- a/backend/cli/test/tool/command-runtime.test.ts +++ b/backend/cli/test/tool/command-runtime.test.ts @@ -29,7 +29,10 @@ test("bash registers only its live process in the project compute ledger", async const find = async (attempt = 0): Promise[number]> => { const command = CommandRuntime.list(Instance.project.id, session.id)[0] if (command) return command - if (attempt >= 100) throw new Error("Live command did not enter the compute ledger") + // 500 * 10ms = 5s: the real command doesn't start until the loopback + // shim signals ready (up to ~3s under sandbox network "allowlist", + // the default) or its wait caps out. See sandbox.ts's shimScript. + if (attempt >= 500) throw new Error("Live command did not enter the compute ledger") await Bun.sleep(10) return find(attempt + 1) } diff --git a/docs/adr/0002-sandbox-network-policy.md b/docs/adr/0002-sandbox-network-policy.md new file mode 100644 index 00000000..0428c90f --- /dev/null +++ b/docs/adr/0002-sandbox-network-policy.md @@ -0,0 +1,73 @@ +# ADR 0002: Sandbox network policy becomes three-state, allowlist by default + +Status: accepted + +## Context + +`sandbox.network` is `"allow" | "deny"`. Deny locks kernels out of PyPI, NCBI, UniProt, PDB and +EBI, which is most of what a research tool is for. Allow is unrestricted egress. Neither state is +what the product needs: a kernel that cannot reach a package index or a sequence database cannot +do the work it exists to do, and a kernel with unrestricted egress can send agent-controlled data +anywhere on the internet. Because the installer needed the egress the kernel was denied, earlier +design work gave it a second, network-enabled sandbox purely so it could have that egress the +kernel could not. + +## Decision + +`sandbox.network` becomes three-state: `"deny" | "allowlist" | "allow"`, defaulting to +`"allowlist"`. + +Enforcement is `--unshare-net` plus a bind-mounted unix socket, established by two measurements +taken on the spike branch `proto/sandbox-allowlist-proxy` before the proxy itself was written: +inside `bwrap --unshare-net`, TCP to any host — including the host's own loopback — returns `000`, +and a unix socket bind-mounted into that same network namespace still crosses it. The socket is +therefore the only route out of the namespace. The proxy on the host end of that socket resolves +names itself, which is why the sandboxed process has no DNS of its own. + +One policy covers kernel and installer. There is no separate network-enabled install sandbox: +under `"allowlist"` the installer reaches the same allowlisted hosts through the same proxy the +kernel uses, so the asymmetry that motivated a second sandbox no longer exists. + +Proxy policy is not part of the `ExecutionAuthority.generation` hash. `generation` hashes trust, +filesystem grants, and sandbox policy, and changing it tears down and reboots every live kernel +bound to it. Editing the allowlist is not that kind of change: it takes effect on the next +connection through the running proxy, without tearing down live kernels. + +This ADR decides the default allowlist ships in code; per-project additions live in config. + +## Consequences + +This is a breaking change to a documented config key. Existing `"deny"` and `"allow"` values keep +working unchanged; only the default moves, from `"deny"` to `"allowlist"`. + +`HTTP_PROXY`, `HTTPS_PROXY` and `NO_PROXY` join `SAFE_ENV_PREFIXES` so they reach a kernel +process, and `Sandbox.wrapArgv` must compose a shim into the sandboxed argv, because pip, +requests and curl take an `http://host:port` proxy from those variables and none of them speak +unix sockets directly. Some process also has to start and stop the proxy across the CLI's own +lifecycle; this ADR does not fix that shape here, only that it is needed. + +The boundary is host-level, not content-level. The proxy pipes bytes after checking the +authority; it cannot see inside TLS, so an allowlisted host can still be sent anything a client +sends it. Allowlisting bounds where a kernel can talk, not what it says once it is talking. + +Unresolved: seatbelt has no namespace, so the _mechanism_ above — sever the network device, cross +back in only through a bind-mounted socket — does not transfer as written. That is not the same as +saying macOS cannot reach the same bounded-egress outcome: seatbelt can restrict +`network-outbound` to a specific local port via `(allow network-outbound (remote tcp +"localhost:PORT"))`, which is exactly the shape `anthropic-experimental/sandbox-runtime` ships (a +default network deny, then a selective allow for `network-bind`/`network-inbound`/ +`network-outbound` on the proxy's loopback port). Neither OS can filter by hostname at the +sandbox-profile level — that is what the proxy is for on Linux too — so this is achievable via a +different mechanism, not impossible. Task 7 built it: the host-side proxy listens on a loopback +TCP port directly on macOS (no bind-mounted socket, no bridge — seatbelt has no namespace to put +either behind), the profile permits `network-bind`/`network-inbound`/`network-outbound` on +exactly that port as described above, and — because a loopback port, unlike a unix socket, carries +no filesystem permissions of its own — every request to it must additionally carry a +`Proxy-Authorization` secret generated fresh per proxy start. This is unverified in the same sense +the rest of this ADR's Linux side was before it was measured: nobody on this project has run +`sandbox-exec`, so whether the profile text above is actually _accepted and enforced_ as written — +including whether `network-bind`/`network-inbound` are the right operations to permit at all, and +whether `(local ...)` is the right filter for them — is a real, open question, not merely +theoretical caution. See `.superpowers/sdd/2026-08-09-sandbox-network-policy/task-7-report.md` for +exactly what a Mac owner still needs to run to close it. Windows has no sandbox backend at all, so +the question does not apply there. diff --git a/docs/specs/windows-sandbox-design.md b/docs/specs/windows-sandbox-design.md new file mode 100644 index 00000000..29499f68 --- /dev/null +++ b/docs/specs/windows-sandbox-design.md @@ -0,0 +1,172 @@ +# Windows sandbox — design + +Status: proposed, not implemented +Date: 2026-08-11 +Branch: `feat/sandbox-network-policy` (Linux and macOS land there; this does not) + +## Problem + +`sandbox.network` is three-state — `deny | allowlist | allow` — defaulting to `allowlist`, where a sandboxed +process reaches an approved set of hosts and nothing else. Linux enforces this with a network namespace plus a +unix socket; macOS with a seatbelt profile permitting one loopback address. + +Windows has neither, and `Sandbox.backend()` returns `"none"` there today. Combined with `enabled: true`, +`available: false` and `onUnavailable: "error"`, that means Windows users cannot use kernels **at all** — +before any of this work. A Linux-only sandbox makes the default policy a lie on the platform with the most +users. + +## The guarantee this design targets + +> The agent may reach only approved network resources, while OpenScience itself runs **without administrator +> privileges**. + +Both halves matter. Dropping the second is easy and unacceptable: OpenScience installs per-user today, and an +application that runs AI-authored code asking for elevation to create a local account and load kernel network +filters is indistinguishable, at the UAC prompt, from malware. Users will refuse, EDR will flag it, and they +will be right to. + +## Why not the firewall + +The obvious approach — Windows Filtering Platform, a `ALE_AUTH_CONNECT` filter permitting only loopback to a +proxy port — is what `anthropic-experimental/sandbox-runtime` does on Windows. It works, and it requires a +dedicated local user account plus WFP filter installation, hence its one-time elevated `windows-install`. + +The model is backwards for us: + +``` +firewall model: the machine can reach the network + → add rules to restrict this process + → requires ADMIN + +capability model: the process cannot reach the network + → grant it capabilities explicitly + → no machine-level mutation at all +``` + +The privileged operation in the firewall model is not the restriction. It is the **permission** — Windows will +take all network away for free and charge administrator rights to give one endpoint back. + +## Design: AppContainer with no network capability, plus a broker + +``` +┌──────────────────────────────────────┐ +│ OpenScience (normal user) │ +│ │ +│ ┌────────────────────────────────┐ │ +│ │ Policy broker │ │ +│ │ network · filesystem · creds │──┼──▶ approved hosts +│ └───────────────▲────────────────┘ │ +└──────────────────┼───────────────────┘ + │ named pipe, ACL'd to the AppContainer SID +┌──────────────────┴───────────────────┐ +│ Agent sandbox │ +│ │ +│ AppContainer, low integrity │ +│ network capability: NONE │ +│ filesystem: workspace only │ +│ credentials: none │ +│ │ +│ socket("evil.com", 443) ✗ │ +│ socket("8.8.8.8", 53) ✗ │ +│ named pipe → broker ✓ │ +└──────────────────────────────────────┘ +``` + +Three properties, none requiring elevation: + +1. **An AppContainer without a network capability has no network.** Kernel-enforced. Not a rule layered over + an otherwise-connected process — the capability was never granted. +2. **`CreateAppContainerProfile` creates a per-user profile** and returns a stable `S-1-15-2-…` package SID. + Microsoft's own `mxc` calls it from an ordinary backend, and the SID anchors filesystem ACEs downstream. +3. **Named pipes can be ACL'd to that SID.** Creating the pipe with the right ACL is an ordinary user-mode + operation, and it is the direct analogue of the bind-mounted unix socket that makes Linux work. + +`internetClient` is deliberately **not** granted. It means "outbound internet" wholesale, which cannot express +"github.com yes, `169.254.169.254` no" — exactly the distinction that matters when data exfiltration and SSRF +are in the threat model. + +The broker is then a network reference monitor: parse URL → check scheme, host, port, method → apply DNS and +rebinding policy → perform the request → return the response over IPC. The policy engine is the same allowlist +matcher used on Linux and macOS; only the transport differs. + +## What this changes about the model, and the cost + +Linux and macOS are **socket-transparent**: unmodified `pip`, `curl` and `requests` work, because a shim inside +the sandbox speaks HTTP-proxy protocol and forwards over the socket. That shim needs to listen on loopback. + +An AppContainer with no network capability has **no loopback either**, so no shim can exist. Windows is +therefore **capability-mediated**: code must _ask_ the broker, not _connect_. + +Better security, worse compatibility. Concretely: + +| | Linux / macOS | Windows | +| ----------------------------------- | -------------------- | ------------------------------------------- | +| Agent tools (`webfetch`, `compute`) | works | works | +| `pip install` in a kernel | works via proxy | **needs the installer outside the sandbox** | +| `requests.get(uniprot)` in a cell | works | **blocked** | +| A malicious package phoning home | bounded by allowlist | blocked outright | + +Two consequences worth deciding deliberately rather than discovering: + +- **Package installation must run in the broker's trust domain**, not inside the AppContainer, with the package + set already approved by its card. This is what the original spec described before the Linux proxy made a + separate install path unnecessary; Windows keeps it. +- **A notebook cell cannot fetch a scientific API directly.** For a research product this is a real capability + gap, and it is the strongest argument against this design. The mitigation is a broker-backed fetch tool the + agent calls instead of using sockets — which works, but is a different programming model from the other two + platforms. + +## Upgrade path: `Experimental_CreateProcessInSandbox` + +Windows 11 exposes `Experimental_CreateProcessInSandbox` from `processmodel.dll`. It composes AppContainer, +filesystem allowlists, integrity level, Win32k and UI restrictions, capabilities, and a `network_policy` +including a **proxy** — very close to this design, natively. + +`microsoft/mxc` ships on it: it resolves `processmodel.dll!Experimental_CreateProcessInSandbox` for its "Tier +1" path and falls back to lower tiers when the symbol is absent. + +Treat it the same way. It is experimental, subject to change, Windows 11 only, has no public header, and must +be located by dynamic load. Build the broker design as the foundation and adopt the native API as a tier above +it when it stabilises — the tier-fallback pattern is the lesson, not just the API. + +## Alternatives considered + +| Approach | Runtime admin | Network isolation | Per-destination policy | Verdict | +| ---------------------------------- | ---------------------------------------------------------- | ----------------- | ---------------------- | ------------------------------------------------- | +| WFP / firewall rules | usually yes | strong | yes | rejected — elevation we cannot justify asking for | +| AppContainer + `internetClient` | no | strong | **no** | rejected — cannot express an allowlist | +| **AppContainer + broker** | **no** | **strong** | **strong** | **proposed** | +| Userspace proxy only, `HTTP_PROXY` | no | **none** | yes | rejected — advisory; a raw socket ignores it | +| Restricted token / job object | no | **none** | no | rejected — no network isolation at all | +| Run under WSL2 | no (WSL install is elevated, but it is Microsoft's prompt) | strong | strong | viable fallback; reuses the Linux path unchanged | +| Hyper-V / Windows Sandbox | setup privileged | strong | yes | rejected — heavyweight, Pro/Enterprise only | + +WSL2 deserves a second look before committing to a third backend: where it is present, everything already built +for Linux applies unchanged, and the elevated step belongs to Microsoft's installer rather than ours. + +## Verification status + +**Nothing in this document has been executed.** There is no Windows machine on this project. Every claim about +Linux in this branch was measured; every claim here is research and reasoning, on the platform where reasoning +has already needed correcting twice. + +Before any of it is built, a Windows owner should confirm: + +1. `CreateAppContainerProfile` succeeds as a standard user, unelevated. +2. A process in an AppContainer with no network capability genuinely cannot open a socket — including to + loopback. +3. A named pipe ACL'd to that package SID is reachable from inside, unelevated. +4. Whether _any_ in-container loopback listener is possible, since that single answer decides whether the + socket-transparent model can be recovered and `pip` can work inside the sandbox after all. + +Question 4 is the one that would most change this design. + +## Sources + +- [CreateProcessInSandbox](https://learn.microsoft.com/en-us/windows/win32/secauthz/createprocessinsandbox) +- [AppContainer for legacy applications](https://learn.microsoft.com/en-us/windows/win32/secauthz/appcontainer-for-legacy-applications-) +- [CreateAppContainerProfile](https://learn.microsoft.com/en-us/windows/win32/api/userenv/nf-userenv-createappcontainerprofile) +- [microsoft/mxc — base process container](https://github.com/microsoft/mxc/blob/main/docs/base-process-container/guide.md) +- [MXC internals](https://www.originhq.com/research/mxc-execution-containers-internals) +- [anthropic-experimental/sandbox-runtime](https://github.com/anthropic-experimental/sandbox-runtime) +- [Tyranid's Lair — UWP localhost network isolation](https://www.tiraniddo.dev/2018/07/uwp-localhost-network-isolation-and-edge.html) diff --git a/frontend/docs/src/content/openscience/sandbox.mdx b/frontend/docs/src/content/openscience/sandbox.mdx index 870a9493..9111f704 100644 --- a/frontend/docs/src/content/openscience/sandbox.mdx +++ b/frontend/docs/src/content/openscience/sandbox.mdx @@ -22,7 +22,7 @@ openscience sandbox # show status: backend + current policy - **Writes** are denied everywhere except the workspace (your working directory and its worktree), the system temp dirs, and any extra paths you allow. Everything else on disk is read-only to the agent. - **Reads** stay open. The threat model is *tampering and exfiltration*, not hiding your files from a tool that needs to read them. -- **Network** is allowed by default; set it to deny to stop sandboxed commands from reaching the network at all. +- **Network** is bounded by default (`allowlist`): sandboxed commands can reach a fixed set of research hosts (PyPI, NCBI, UniProt, PDB, EBI, and any hosts you add) through a host-side proxy, and nothing else. Set it to `deny` to block network egress entirely, or `allow` to remove the boundary and permit unrestricted egress. ## Backends @@ -39,7 +39,9 @@ Check what's available on your machine with `openscience sandbox` or `openscienc ```bash openscience sandbox # status (backend + config) openscience sandbox enable # turn on -openscience sandbox enable --network deny # also block network egress +openscience sandbox enable --network deny # block network egress entirely +openscience sandbox enable --network allow # remove the network boundary (unrestricted egress) +openscience sandbox enable --allow-host pypi.example.com # extra host reachable under allowlist (repeatable) openscience sandbox enable --allow /data/shared # extra writable path (repeatable) openscience sandbox enable --on-unavailable error # refuse to run where no backend exists openscience sandbox disable # turn off @@ -50,7 +52,8 @@ The sandbox is a machine-wide safety setting, so it is always written to your ** | Flag | Meaning | | --- | --- | -| `--network` | `allow` (default) or `deny` network egress from sandboxed commands. | +| `--network` | `deny`, `allowlist` (default), or `allow` network egress from sandboxed commands. `allowlist` bounds egress to a fixed set of research hosts plus anything you add with `--allow-host`; `allow` removes that boundary entirely. | +| `--allow-host` | An extra host the sandbox may reach when `--network` is `allowlist`. Repeatable. | | `--allow` | An absolute path, beyond the workspace and temp dirs, the sandbox may write to. Repeatable. | | `--on-unavailable` | Behaviour where no backend exists: `warn` (default, runs unsandboxed with a notice), `error` (refuses to run), `allow` (runs unsandboxed silently). | @@ -66,7 +69,8 @@ The sandbox is a `sandbox` block in your **global** `openscience.json` (or an en { "sandbox": { "enabled": true, - "network": "deny", + "network": "allowlist", + "allowHosts": ["internal.example.com"], "allowWrite": ["/data/shared"], "onUnavailable": "error" } @@ -74,7 +78,8 @@ The sandbox is a `sandbox` block in your **global** `openscience.json` (or an en ``` - `enabled` — master switch. Off by default. -- `network` — `allow` (default) or `deny`. +- `network` — `deny`, `allowlist` (default), or `allow`. +- `allowHosts` — extra hosts sandboxed processes may reach when `network` is `allowlist`. A leading dot matches subdomains, e.g. `.internal.example.com`. - `allowWrite` — extra absolute paths the sandbox may write to. - `onUnavailable` — `warn` (default) · `error` · `allow`, for machines with no backend. diff --git a/frontend/workspace/src/atlas/execution-authority.ts b/frontend/workspace/src/atlas/execution-authority.ts index 5af41f43..8e126963 100644 --- a/frontend/workspace/src/atlas/execution-authority.ts +++ b/frontend/workspace/src/atlas/execution-authority.ts @@ -27,7 +27,7 @@ export interface ExecutionDecision { writable: string[] sandbox: { enabled: boolean - network: "allow" | "deny" + network: "deny" | "allowlist" | "allow" allowWrite: string[] onUnavailable: "warn" | "error" | "allow" backend: "seatbelt" | "bubblewrap" | "none" diff --git a/frontend/workspace/src/components/settings/Sandbox.test.tsx b/frontend/workspace/src/components/settings/Sandbox.test.tsx new file mode 100644 index 00000000..c64969b6 --- /dev/null +++ b/frontend/workspace/src/components/settings/Sandbox.test.tsx @@ -0,0 +1,287 @@ +import { afterAll, afterEach, describe, expect, test } from "bun:test" +import { createServer as createHttpServer } from "node:http" +import { fileURLToPath } from "node:url" +import { createServer } from "vite" +import solid from "vite-plugin-solid" + +// Full real render: the same Vite SSR-load + happy-dom harness KernelCard.test.tsx +// uses, but this panel additionally needs the app's real context stack (it calls +// useGlobalSDK()/usePlatform() to reach the settings API), so the provider chain +// is assembled here too rather than stubbed — a real ServerProvider computes a +// real active URL, a real GlobalSDKProvider wraps a real fetch, and a real +// (in-process) HTTP server answers /settings/sandbox. This is what proves the +// dropdown a user actually opens shows "allowlist", not just that the types +// compile. +const vite = await createServer({ + root: fileURLToPath(new URL("../../..", import.meta.url)), + mode: "production", + logLevel: "silent", + plugins: [solid({ ssr: false, dev: false })], + server: { middlewareMode: true }, + appType: "custom", + resolve: { conditions: ["browser", "production"], dedupe: ["solid-js", "solid-js/web"] }, + ssr: { noExternal: true, resolve: { conditions: ["browser", "production"] } }, +}) + +const [Sandbox, PlatformCtx, ServerCtx, GlobalSDKCtx, Solid, web] = await Promise.all([ + vite.ssrLoadModule("/src/components/settings/Sandbox.tsx") as Promise, + vite.ssrLoadModule("/src/context/platform.tsx") as Promise, + vite.ssrLoadModule("/src/context/server.tsx") as Promise, + vite.ssrLoadModule("/src/context/global-sdk.tsx") as Promise, + vite.ssrLoadModule("solid-js") as Promise, + vite.ssrLoadModule("solid-js/web") as Promise, +]) + +type SandboxConfig = { + enabled?: boolean + network?: "deny" | "allowlist" | "allow" + allowHosts?: string[] + allowWrite?: string[] + onUnavailable?: "warn" | "error" | "allow" +} + +// A real HTTP server implementing the GET/PUT /settings/sandbox contract +// (see backend/cli/src/server/routes/settings/sandbox.ts) against real +// in-memory state, so a PATCH the panel issues is a real request/response +// round trip, not an asserted call. Built on node:http rather than +// Bun.serve(): happy-dom's GlobalRegistrator (this workspace's shared test +// preload) replaces the process-global Response/fetch/Request classes, and +// Bun.serve()'s handler return value is checked against ITS OWN native +// Response identity — a happy-dom Response fails that check with "Expected +// a Response object, but received ...". node:http's request/response +// objects are a different API entirely, so they don't collide. +function fakeServer(initial: SandboxConfig) { + let config: SandboxConfig = { ...initial } + const puts: SandboxConfig[] = [] + let gets = 0 + const status = { platform: "linux", backend: "bubblewrap" as const, available: true, tool: "bwrap" } + + // happy-dom's fetch() enforces real CORS (unlike Bun's native fetch): the + // window's origin differs from this server's, so every response needs an + // explicit allow-origin, and the JSON content-type on PUT triggers a + // preflight OPTIONS the server must answer. + const cors = { "access-control-allow-origin": "*", "access-control-allow-headers": "*" } + const server = createHttpServer((req, res) => { + const url = new URL(req.url ?? "/", "http://internal") + if (req.method === "OPTIONS") { + res.writeHead(204, { ...cors, "access-control-allow-methods": "GET,PUT,POST,OPTIONS" }) + res.end() + return + } + if (url.pathname === "/settings/sandbox" && req.method === "GET") { + gets++ + res.writeHead(200, { ...cors, "content-type": "application/json" }) + res.end(JSON.stringify({ config, status })) + return + } + if (url.pathname === "/settings/sandbox" && req.method === "PUT") { + const chunks: Buffer[] = [] + req.on("data", (c) => chunks.push(c)) + req.on("end", () => { + const patch = JSON.parse(Buffer.concat(chunks).toString() || "{}") as SandboxConfig + puts.push(patch) + config = { ...config, ...patch } + res.writeHead(200, { ...cors, "content-type": "application/json" }) + res.end(JSON.stringify({ config, status })) + }) + return + } + // Health probe / event stream the surrounding app context makes on + // mount — not under test here; just must not hang or throw. + res.writeHead(200, { ...cors, "content-type": "application/json" }) + res.end("{}") + }) + + return { + listen: () => + new Promise((resolve) => { + server.listen(0, "127.0.0.1", resolve) + }), + get url() { + const address = server.address() + if (!address || typeof address === "string") throw new Error("fakeServer not listening") + return `http://127.0.0.1:${address.port}` + }, + get config() { + return config + }, + get puts() { + return puts + }, + get gets() { + return gets + }, + stop: () => new Promise((resolve) => server.close(() => resolve())), + } +} + +const platformValue = { + platform: "web" as const, + openLink: () => {}, + restart: async () => {}, + back: () => {}, + forward: () => {}, + notify: async () => {}, +} + +const cleanups: Array<() => void> = [] +afterAll(async () => { + cleanups.splice(0).forEach((c) => c()) + await vite.close() +}) +afterEach(() => { + cleanups.splice(0).forEach((c) => c()) + document.body.replaceChildren() +}) + +const settle = () => new Promise((r) => setTimeout(r, 20)) + +const mount = async (initial: SandboxConfig) => { + const api = fakeServer(initial) + await api.listen() + const host = document.createElement("div") + document.body.append(host) + const view = () => + Solid.createComponent(PlatformCtx.PlatformProvider, { + value: platformValue, + get children() { + return Solid.createComponent(ServerCtx.ServerProvider, { + defaultUrl: api.url, + get children() { + return Solid.createComponent(GlobalSDKCtx.GlobalSDKProvider, { + get children() { + return Solid.createComponent(Sandbox.default, {}) + }, + }) + }, + }) + }, + }) + const dispose = web.render(view, host) + cleanups.push(() => { + dispose() + api.stop() + }) + // Real async settling: persisted-storage ready gate -> defaultUrl effect -> + // the panel's own createResource GET -> re-render with real data. Poll on + // the actual GET landing (not merely on a select existing — the trigger + // renders immediately from the panel's own pre-load fallback, which would + // satisfy an existence check before the real config ever arrives). + for (let i = 0; i < 100 && api.gets === 0; i++) await settle() + for (let i = 0; i < 20 && !host.querySelector("[data-slot='select-select-trigger-value']"); i++) await settle() + await settle() + return { host, api } +} + +// Kobalte's Select opens/selects on a full pointer sequence, not a bare +// "click" — happy-dom needs pointerdown/mousedown/pointerup/mouseup/click +// dispatched explicitly for its internal press handling to fire. +const pointerActivate = (el: HTMLElement) => { + el.focus() + for (const [Ctor, type] of [ + [PointerEvent, "pointerdown"], + [MouseEvent, "mousedown"], + [PointerEvent, "pointerup"], + [MouseEvent, "mouseup"], + [MouseEvent, "click"], + ] as const) { + el.dispatchEvent( + new Ctor(type, { + bubbles: true, + cancelable: true, + button: 0, + ...(Ctor === PointerEvent ? { pointerId: 1 } : {}), + }), + ) + } +} + +const networkTriggerValue = (host: HTMLElement) => { + const row = [...host.querySelectorAll("span")] + .find((el) => el.textContent === "Network egress") + ?.closest("div.flex.flex-wrap") + return row?.querySelector("[data-slot='select-select-trigger-value']")?.textContent +} + +describe("Sandbox settings panel — network policy", () => { + test("a config that has never set network still shows the shipped allowlist default", async () => { + const { host } = await mount({}) + expect(networkTriggerValue(host)).toContain("Allowlist") + }) + + test("a config explicitly persisted as allowlist displays as allowlist, not blank", async () => { + const { host } = await mount({ network: "allowlist" }) + expect(networkTriggerValue(host)).toContain("Allowlist") + }) + + test("a config persisted as deny still displays as deny", async () => { + const { host } = await mount({ network: "deny" }) + expect(networkTriggerValue(host)).toContain("Deny") + }) + + test("selecting Allow from the dropdown round-trips a real PATCH and the trigger updates", async () => { + const { host, api } = await mount({ network: "allowlist" }) + expect(networkTriggerValue(host)).toContain("Allowlist") + + const row = [...host.querySelectorAll("span")] + .find((el) => el.textContent === "Network egress") + ?.closest("div.flex.flex-wrap") + const trigger = row?.querySelector("[data-slot='select-select-trigger']") + expect(trigger).toBeTruthy() + pointerActivate(trigger!) + + let option: HTMLElement | undefined + for (let i = 0; i < 50 && !option; i++) { + option = [...document.querySelectorAll("[data-slot='select-select-item']")].find((el) => + el.textContent?.includes("Allow — unrestricted"), + ) + if (!option) await settle() + } + expect(option).toBeTruthy() + pointerActivate(option!) + + for (let i = 0; i < 50 && api.puts.length === 0; i++) await settle() + expect(api.puts).toEqual([{ network: "allow" }]) + expect(api.config.network).toBe("allow") + + for (let i = 0; i < 50 && !networkTriggerValue(host)?.includes("Allow —"); i++) await settle() + expect(networkTriggerValue(host)).toContain("Allow — unrestricted") + }) +}) + +describe("Sandbox settings panel — extra allowed hosts", () => { + test("the editor is shown under allowlist and hidden under deny", async () => { + const allowlisted = await mount({ network: "allowlist" }) + expect([...allowlisted.host.querySelectorAll("span")].some((el) => el.textContent === "Extra allowed hosts")).toBe( + true, + ) + + const denied = await mount({ network: "deny" }) + expect([...denied.host.querySelectorAll("span")].some((el) => el.textContent === "Extra allowed hosts")).toBe(false) + }) + + test("adding a host round-trips a real PATCH and the new host renders back", async () => { + const { host, api } = await mount({ network: "allowlist", allowHosts: [] }) + const input = [...host.querySelectorAll("input")].find((el) => el.placeholder === "pypi.example.com") + expect(input).toBeTruthy() + + input!.value = ".internal.example.com" + input!.dispatchEvent(new Event("input", { bubbles: true })) + const add = [...host.querySelectorAll("button")].find((el) => el.textContent === "Add" && !el.disabled) + expect(add).toBeTruthy() + pointerActivate(add!) + + for (let i = 0; i < 50 && api.puts.length === 0; i++) await settle() + expect(api.puts).toEqual([{ allowHosts: [".internal.example.com"] }]) + + // Poll on exactly the predicate being asserted. Waiting on a looser one + // (the whole subtree's textContent containing the host) can go true while + // the element this asserts on has not rendered, spending all 50 + // iterations and then failing anyway — and a substring test against a + // rendered hostname is also what CodeQL flags as incomplete URL + // sanitization, which it is not, but the strict check is better regardless. + const rendered = () => [...host.querySelectorAll("code")].some((el) => el.textContent === ".internal.example.com") + for (let i = 0; i < 50 && !rendered(); i++) await settle() + expect(rendered()).toBe(true) + }) +}) diff --git a/frontend/workspace/src/components/settings/Sandbox.tsx b/frontend/workspace/src/components/settings/Sandbox.tsx index 1fa98530..4e565e88 100644 --- a/frontend/workspace/src/components/settings/Sandbox.tsx +++ b/frontend/workspace/src/components/settings/Sandbox.tsx @@ -17,7 +17,8 @@ import { settingsApi } from "./api" interface SandboxConfig { enabled?: boolean - network?: "allow" | "deny" + network?: "deny" | "allowlist" | "allow" + allowHosts?: string[] allowWrite?: string[] onUnavailable?: "warn" | "error" | "allow" } @@ -46,8 +47,9 @@ interface SelfTest { } const NETWORK_OPTS = [ - { value: "allow" as const, label: "Allow" }, - { value: "deny" as const, label: "Deny" }, + { value: "deny" as const, label: "Deny — block all network access" }, + { value: "allowlist" as const, label: "Allowlist — bounded egress (default)" }, + { value: "allow" as const, label: "Allow — unrestricted egress" }, ] const UNAVAILABLE_OPTS = [ { value: "warn" as const, label: "Warn & run" }, @@ -67,9 +69,10 @@ const Sandbox: Component = () => { const [test, setTest] = createSignal() const [testing, setTesting] = createSignal(false) const [newPath, setNewPath] = createSignal("") + const [newHost, setNewHost] = createSignal("") const config = (): SandboxConfig => - data()?.config ?? { enabled: true, network: "deny", allowWrite: [], onUnavailable: "error" } + data()?.config ?? { enabled: true, network: "allowlist", allowHosts: [], allowWrite: [], onUnavailable: "error" } const status = () => data()?.status const patch = async (body: SandboxConfig, failure: string) => { @@ -108,6 +111,17 @@ const Sandbox: Component = () => { const removePath = (p: string) => patch({ allowWrite: (config().allowWrite ?? []).filter((x) => x !== p) }, "Couldn't remove the path") + const addHost = () => { + const h = newHost().trim() + if (!h) return + const next = [...(config().allowHosts ?? [])] + if (!next.includes(h)) next.push(h) + setNewHost("") + patch({ allowHosts: next }, "Couldn't add the host") + } + const removeHost = (h: string) => + patch({ allowHosts: (config().allowHosts ?? []).filter((x) => x !== h) }, "Couldn't remove the host") + return (
@@ -116,7 +130,8 @@ const Sandbox: Component = () => {

Permissions decide whether the agent runs a shell command — not what it can reach once it does. OpenScience confines local terminals, kernels, and shell commands by default: writes are limited to - authorized project roots and network egress is denied unless you explicitly relax the machine-wide policy. + authorized project roots and network egress is bounded to an allowlist of approved hosts unless you + explicitly widen or restrict the machine-wide policy.

@@ -178,12 +193,13 @@ const Sandbox: Component = () => {
Network egress - Deny to stop sandboxed commands reaching the network. + Allowlist bounds sandboxed commands to approved hosts (default). Allow removes that boundary — + unrestricted egress. Deny blocks the network entirely.
setNewHost(e.currentTarget.value)} + onKeyDown={(e) => e.key === "Enter" && addHost()} + /> + + + + + {/* extra writable paths */}
Extra writable paths diff --git a/frontend/workspace/src/notebook/runtime.test.ts b/frontend/workspace/src/notebook/runtime.test.ts index ad0fddcb..0adfdb13 100644 --- a/frontend/workspace/src/notebook/runtime.test.ts +++ b/frontend/workspace/src/notebook/runtime.test.ts @@ -135,7 +135,7 @@ describe("kernel runtime presentation", () => { }) test("states network reach on its own, with an open network as the notable case", () => { - const sandboxed = (network: "deny" | "allow", enforced = true) => + const sandboxed = (network: "deny" | "allowlist" | "allow", enforced = true) => kernel({ environment: { cwd: "/work/project", @@ -146,8 +146,14 @@ describe("kernel runtime presentation", () => { expect(kernelNetworkLabel(sandboxed("deny"))).toBe("Network disabled") expect(kernelNetworkTone(sandboxed("deny"))).toBe("muted") + + // "allowlist" is bounded egress, not the open network "allow" is — it must + // read as neither "disabled" nor plain "allowed". + expect(kernelNetworkLabel(sandboxed("allowlist"))).toBe("Network bounded") + expect(kernelNetworkTone(sandboxed("allowlist"))).toBe("pending") + expect(kernelNetworkLabel(sandboxed("allow"))).toBe("Network allowed") - expect(kernelNetworkTone(sandboxed("allow"))).toBe("pending") + expect(kernelNetworkTone(sandboxed("allow"))).toBe("danger") // A sandbox that was asked for but never took hold does not block anything, // whatever its recorded network setting says. diff --git a/frontend/workspace/src/notebook/runtime.ts b/frontend/workspace/src/notebook/runtime.ts index 840e9914..f171d092 100644 --- a/frontend/workspace/src/notebook/runtime.ts +++ b/frontend/workspace/src/notebook/runtime.ts @@ -11,7 +11,7 @@ export type KernelEnvironment = { requested: boolean enforced: boolean backend: "seatbelt" | "bubblewrap" | "none" - network: "allow" | "deny" + network: "deny" | "allowlist" | "allow" platform: string available: boolean tool?: string @@ -250,7 +250,10 @@ export function kernelEnvironmentLabel(kernel?: KernelStatus) { export function kernelNetworkLabel(kernel?: KernelStatus) { const sandbox = kernel?.environment?.sandbox if (!sandbox) return null - return sandbox.enforced && sandbox.network === "deny" ? "Network disabled" : "Network allowed" + if (!sandbox.enforced) return "Network allowed" + if (sandbox.network === "deny") return "Network disabled" + if (sandbox.network === "allowlist") return "Network bounded" + return "Network allowed" } /** @@ -259,10 +262,20 @@ export function kernelNetworkLabel(kernel?: KernelStatus) { * kernelEnvironmentTone, which reads an enforced sandbox as the good outcome: * there the question is whether the sandbox holds, here it is what the run can * still touch through it. + * + * "allowlist" gets its own middle tone rather than folding into either + * neighbor: it is not inert like "deny" (the kernel does reach the network), + * and collapsing it into "allow" would hide the one fact — a fixed host set + * vs. no boundary at all — this label exists to surface. "allow" escalates to + * "danger": it is the explicit widening away from the bounded default, not + * the default itself. */ export function kernelNetworkTone(kernel?: KernelStatus): KernelTone { const sandbox = kernel?.environment?.sandbox - return sandbox?.enforced && sandbox.network === "deny" ? "muted" : "pending" + if (!sandbox?.enforced) return "pending" + if (sandbox.network === "deny") return "muted" + if (sandbox.network === "allowlist") return "pending" + return "danger" } export function kernelEnvironmentTone(kernel?: KernelStatus): KernelTone {