From a761941d4b69f37dfc19427338a0ff4cabe7706d Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:57:28 +0000 Subject: [PATCH 01/10] computerd: Inherit only named variables into exec Every spawned command received the daemon's whole environment. That carried the daemon's own configuration into the workspace, and it means any secret placed in that environment reaches any command the workspace runs. A command now inherits a named set: PATH, HOME, TMPDIR, TZ, LANG, TERM, the LC_ family, and anything prefixed COMPUTER_VAR_, which arrives with the prefix stripped so COMPUTER_VAR_NODE_ENV becomes NODE_ENV. A prefixed value is applied last and may replace one of the standard variables, which is how an operator pins a toolchain onto PATH. The runner's configured environment and the per-call environment still layer on top. An allowlist rather than a denylist, so that adding a variable to the daemon does not expose it by default. PATH is the one that had to survive. /bin/sh falls back to a compiled-in default covering /usr/bin, so dropping it leaves standard tools working while hiding anything an image installed elsewhere, which is how images usually ship language runtimes and vendored toolchains. This narrows what a command sees. A workspace reading some other inherited variable will stop finding it and needs the prefix. --- packages/computerd/src/exec/env.test.ts | 71 ++++++++++++++++++++++ packages/computerd/src/exec/env.ts | 49 +++++++++++++++ packages/computerd/src/exec/runner.test.ts | 26 ++++++++ packages/computerd/src/exec/runner.ts | 3 +- 4 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 packages/computerd/src/exec/env.test.ts create mode 100644 packages/computerd/src/exec/env.ts diff --git a/packages/computerd/src/exec/env.test.ts b/packages/computerd/src/exec/env.test.ts new file mode 100644 index 00000000..9779a74e --- /dev/null +++ b/packages/computerd/src/exec/env.test.ts @@ -0,0 +1,71 @@ +import { expect, test } from "vitest"; + +import { inheritedEnv } from "./env.js"; + +test("forwards the standard variables a shell environment expects", () => { + const env = inheritedEnv({ + PATH: "/opt/toolchain/bin:/usr/bin", + HOME: "/root", + TMPDIR: "/tmp", + TZ: "UTC", + LANG: "C.UTF-8", + LC_ALL: "C.UTF-8", + TERM: "xterm", + }); + + expect(env).toEqual({ + PATH: "/opt/toolchain/bin:/usr/bin", + HOME: "/root", + TMPDIR: "/tmp", + TZ: "UTC", + LANG: "C.UTF-8", + LC_ALL: "C.UTF-8", + TERM: "xterm", + }); +}); + +test("drops the daemon's own configuration and anything else unrecognized", () => { + // The secret is the reason this list is an allowlist rather than a + // denylist: a new one must not reach a spawned process by default. + const env = inheritedEnv({ + PATH: "/usr/bin", + RPC_CLIENT_SECRET: "sssh", + PORT: "8080", + MOUNT_POINT: "/workspace", + FUSE_MOUNT: "auto", + LOG_FILE: "/tmp/computerd.log", + EXEC_LOG_MAX_BYTES: "1024", + AWS_SECRET_ACCESS_KEY: "also-sssh", + }); + + expect(env).toEqual({ PATH: "/usr/bin" }); +}); + +test("forwards COMPUTER_VAR_ values with the prefix stripped", () => { + const env = inheritedEnv({ + COMPUTER_VAR_NODE_ENV: "production", + COMPUTER_VAR_EMPTY: "", + }); + + expect(env).toEqual({ NODE_ENV: "production", EMPTY: "" }); +}); + +test("lets a prefixed value replace a standard variable", () => { + const env = inheritedEnv({ + PATH: "/usr/bin", + COMPUTER_VAR_PATH: "/opt/only/bin", + }); + + expect(env.PATH).toBe("/opt/only/bin"); +}); + +test("ignores a bare prefix with no name after it", () => { + expect(inheritedEnv({ COMPUTER_VAR_: "nameless" })).toEqual({}); +}); + +test("omits variables that are absent rather than defining them empty", () => { + const env = inheritedEnv({ PATH: "/usr/bin", HOME: undefined }); + + expect(env).toEqual({ PATH: "/usr/bin" }); + expect("HOME" in env).toBe(false); +}); diff --git a/packages/computerd/src/exec/env.ts b/packages/computerd/src/exec/env.ts new file mode 100644 index 00000000..c904466c --- /dev/null +++ b/packages/computerd/src/exec/env.ts @@ -0,0 +1,49 @@ +// Builds the environment a spawned command inherits from the daemon. +// +// This is an allowlist, not a denylist. The daemon's own environment +// carries its configuration and, when the host sets one, the shared +// secret that authorizes requests to its HTTP surface. Spreading all of +// it into every `shell.exec` child would hand that secret to any command +// the workspace runs, and a denylist would leak the next secret somebody +// adds. So nothing crosses unless it is named here. + +// Variables a shell environment is expected to carry. PATH is the +// load-bearing one: /bin/sh falls back to a compiled-in default that +// covers /usr/bin, so dropping it leaves standard tools working while +// silently hiding anything an image installed elsewhere, which is a +// common pattern for language runtimes and vendored toolchains. +const FORWARDED = ["PATH", "HOME", "TMPDIR", "TZ", "LANG", "TERM"] as const; + +// Locale variables are a family rather than a fixed name. +const FORWARDED_PREFIX = "LC_"; + +// The operator's channel for reaching a command's environment. The +// prefix is stripped on the way through, so COMPUTER_VAR_NODE_ENV +// arrives as NODE_ENV. +export const ENV_VAR_PREFIX = "COMPUTER_VAR_"; + +export function inheritedEnv(source: NodeJS.ProcessEnv): Record { + const env: Record = {}; + + for (const name of FORWARDED) { + const value = source[name]; + if (value !== undefined) env[name] = value; + } + + for (const [name, value] of Object.entries(source)) { + if (value === undefined) continue; + if (name.startsWith(FORWARDED_PREFIX)) env[name] = value; + } + + // Applied last so an operator can deliberately replace one of the + // standard variables, e.g. COMPUTER_VAR_PATH to pin a toolchain. + for (const [name, value] of Object.entries(source)) { + if (value === undefined) continue; + if (!name.startsWith(ENV_VAR_PREFIX)) continue; + const stripped = name.slice(ENV_VAR_PREFIX.length); + if (stripped.length === 0) continue; + env[stripped] = value; + } + + return env; +} diff --git a/packages/computerd/src/exec/runner.test.ts b/packages/computerd/src/exec/runner.test.ts index 4258017b..09f417da 100644 --- a/packages/computerd/src/exec/runner.test.ts +++ b/packages/computerd/src/exec/runner.test.ts @@ -491,3 +491,29 @@ test("heartbeat seq is monotonically increasing with other events", async () => dispose(); } }); + +test("a spawned command sees the allowlisted environment, not the daemon's", async () => { + // The daemon's environment holds its own configuration and, when the + // host sets one, the secret authorizing requests to its HTTP surface. + // Neither may reach a command the workspace runs. + process.env.RPC_CLIENT_SECRET = "must-not-escape"; + process.env.COMPUTER_VAR_GREETING = "hello"; + const { runner, dispose } = fixture(); + try { + // Written without brace expansion so the assertion reads as shell + // rather than as a mistyped template literal. + const handle = runner.exec( + 'printf \'[%s][%s]\' "$RPC_CLIENT_SECRET" "$GREETING"; test -n "$PATH" && printf \'[path]\'', + ); + const events = await drain(handle.events); + const stdout = events + .filter((event) => event.name === "stdout") + .map((event) => decode(event.value as Uint8Array)) + .join(""); + expect(stdout).toBe("[][hello][path]"); + } finally { + delete process.env.RPC_CLIENT_SECRET; + delete process.env.COMPUTER_VAR_GREETING; + dispose(); + } +}); diff --git a/packages/computerd/src/exec/runner.ts b/packages/computerd/src/exec/runner.ts index 347fd227..21333eaa 100644 --- a/packages/computerd/src/exec/runner.ts +++ b/packages/computerd/src/exec/runner.ts @@ -20,6 +20,7 @@ import { randomUUID } from "node:crypto"; import { type Database, stat } from "@cloudflare/dofs"; +import { inheritedEnv } from "./env.js"; import { createLog, type EventLog, openLog } from "./log.js"; import { clearExecState, initializeExecSchema } from "./schema.js"; import { ExecError, type ExecEvent, type ExecOptions, type RunnerOptions } from "./types.js"; @@ -111,7 +112,7 @@ export class Runner { if (existing !== undefined) this.disposeRecord(existing); const cwd = options.cwd ?? this.opts.cwd; - const env = { ...process.env, ...this.opts.env, ...options.env }; + const env = { ...inheritedEnv(process.env), ...this.opts.env, ...options.env }; // Pre-flight the cwd via dofs's stat which walks vfs_nodes / // vfs_dirents in SQLite directly — no node:fs.statSync, no // FUSE callback. Preserves the historical ENOENT-cwd error From d6fe55b9acf3ea7ee4b39ec5cd05abf0b322f1f8 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:02:25 +0000 Subject: [PATCH 02/10] computerd: Authorize the HTTP surface with a shared secret POST /connect makes the daemon dial a caller-supplied address and serve a full WorkspaceRPC session over it, which carries filesystem access and shell execution. Nothing authorized the request, so anything able to reach the port could point that session wherever it liked. Setting RPC_CLIENT_SECRET now requires it as a bearer token on every route and on the websocket upgrade. The check runs before routing, so an unauthorized caller cannot map the surface, and it compares with timingSafeEqual after a length check, since timingSafeEqual throws on mismatched lengths and the length is not worth leaking either. Readiness stays open. The host polls /health before it holds a session, and gating it would turn a bad token into what looks like a container that never started. Leaving the variable unset disables the checks, which is what the container harnesses and local runs rely on. The daemon reads the secret once and deletes it from its environment. The exec allowlist already keeps it away from spawned commands; removing it means a later code path that spreads process.env cannot reintroduce the leak. --- packages/computerd/src/cli/computerd.test.ts | 129 ++++++++++++++----- packages/computerd/src/cli/computerd.ts | 51 +++++++- 2 files changed, 149 insertions(+), 31 deletions(-) diff --git a/packages/computerd/src/cli/computerd.test.ts b/packages/computerd/src/cli/computerd.test.ts index d84665d9..18edd338 100644 --- a/packages/computerd/src/cli/computerd.test.ts +++ b/packages/computerd/src/cli/computerd.test.ts @@ -595,9 +595,37 @@ async function waitForHTTPOK(url, child, output, timeoutMs = 5_000) { throw new Error(`timed out waiting for ${url}\n${output()}`); } -function request(url) { +// Write a request onto a raw socket and return the response head. +// http.request() will not send a malformed websocket handshake, and +// fetch() will not send one at all, so the handshake cases need this. +function rawRequest(port, lines) { return new Promise((resolve, reject) => { - const request = http.get(url, (response) => { + const socket = net.connect(port, "127.0.0.1", () => { + socket.write(`${lines.join("\r\n")}\r\n\r\n`); + }); + let buf = ""; + socket.setEncoding("utf8"); + socket.on("data", (chunk) => { + buf += chunk; + }); + socket.once("error", reject); + // The server may hold the socket open after refusing the + // handshake, so settle on the end of the response head. + const settle = () => { + socket.destroy(); + resolve(buf); + }; + socket.on("data", () => { + if (buf.includes("\r\n\r\n")) settle(); + }); + socket.once("close", () => resolve(buf)); + setTimeout(settle, 2_000); + }); +} + +function request(url, options = {}) { + return new Promise((resolve, reject) => { + const request = http.get(url, options, (response) => { response.setEncoding("utf8"); let body = ""; response.on("data", (chunk) => { @@ -632,34 +660,6 @@ async function waitFor(predicate, { timeoutMs = 2_000, intervalMs = 10 } = {}) { throw new Error("waitFor: predicate did not become true within the timeout"); } -// Write a request onto a raw socket and return the response head. -// http.request() will not send a malformed websocket handshake, and -// fetch() will not send one at all, so the handshake cases need this. -function rawRequest(port, lines) { - return new Promise((resolve, reject) => { - const socket = net.connect(port, "127.0.0.1", () => { - socket.write(`${lines.join("\r\n")}\r\n\r\n`); - }); - let buf = ""; - socket.setEncoding("utf8"); - socket.on("data", (chunk) => { - buf += chunk; - }); - socket.once("error", reject); - // The server may hold the socket open after refusing the - // handshake, so settle on the end of the response head. - const settle = () => { - socket.destroy(); - resolve(buf); - }; - socket.on("data", () => { - if (buf.includes("\r\n\r\n")) settle(); - }); - socket.once("close", () => resolve(buf)); - setTimeout(settle, 2_000); - }); -} - function postJson(url, body) { const payload = JSON.stringify(body); return new Promise((resolve, reject) => { @@ -709,3 +709,72 @@ function stopProcess(child) { child.kill("SIGTERM"); }); } + +test("RPC_CLIENT_SECRET gates the HTTP surface but not /health", async (_ctx) => { + const secret = "0123456789abcdef0123456789abcdef"; + const port = await getAvailablePort(); + const mountPoint = await fs.mkdtemp(path.join(os.tmpdir(), "computerd-auth-")); + await startComputerd({ + port, + mountPoint, + env: { FUSE_MOUNT: "none", RPC_CLIENT_SECRET: secret }, + }); + const base = `http://127.0.0.1:${port}`; + const bearer = { authorization: `Bearer ${secret}` }; + + // Readiness has to stay reachable: the host polls it before it has + // any session, and a gated probe turns a bad token into what looks + // like a container that never came up. + expect((await request(`${base}/health`)).statusCode).toBe(200); + + for (const route of ["/", "/__computerd/info", "/api/watermarks"]) { + const anonymous = await request(`${base}${route}`); + expect(anonymous.statusCode, `${route} without a token`).toBe(401); + const authorized = await request(`${base}${route}`, { headers: bearer }); + expect(authorized.statusCode, `${route} with the token`).toBe(200); + } + + // Wrong token, right shape. + const wrong = await request(`${base}/__computerd/info`, { + headers: { authorization: `Bearer ${"f".repeat(secret.length)}` }, + }); + expect(wrong.statusCode).toBe(401); + + // A token of a different length must not be treated as a match. + const short = await request(`${base}/__computerd/info`, { + headers: { authorization: "Bearer short" }, + }); + expect(short.statusCode).toBe(401); + + // Another scheme is not a bearer token. + const basic = await request(`${base}/__computerd/info`, { + headers: { authorization: `Basic ${secret}` }, + }); + expect(basic.statusCode).toBe(401); + + // /connect is gated too, and refused before its body is considered. + const connect = await postJson(`${base}/connect`, {}); + expect(connect.statusCode).toBe(401); + + // The upgrade is gated as well. + const upgrade = await rawRequest(port, [ + "GET /api HTTP/1.1", + `Host: 127.0.0.1:${port}`, + "Upgrade: websocket", + "Connection: Upgrade", + "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==", + "Sec-WebSocket-Version: 13", + ]); + expect(upgrade).toMatch(/^HTTP\/1\.1 401 /); +}); + +test("without RPC_CLIENT_SECRET every route stays open", async (_ctx) => { + const port = await getAvailablePort(); + const mountPoint = await fs.mkdtemp(path.join(os.tmpdir(), "computerd-noauth-")); + await startComputerd({ port, mountPoint, env: { FUSE_MOUNT: "none" } }); + const base = `http://127.0.0.1:${port}`; + + for (const route of ["/health", "/", "/__computerd/info", "/api/watermarks"]) { + expect((await request(`${base}${route}`)).statusCode, route).toBe(200); + } +}); diff --git a/packages/computerd/src/cli/computerd.ts b/packages/computerd/src/cli/computerd.ts index 0de544b0..995fa6eb 100644 --- a/packages/computerd/src/cli/computerd.ts +++ b/packages/computerd/src/cli/computerd.ts @@ -1,5 +1,6 @@ #!/usr/bin/env node +import { timingSafeEqual } from "node:crypto"; import { mkdir } from "node:fs/promises"; import { createServer, type IncomingMessage, type Server, type ServerResponse } from "node:http"; import type { Socket } from "node:net"; @@ -99,6 +100,29 @@ function requestPath(request: IncomingMessage): string { return url.pathname; } +// Routes reachable without the shared secret. Readiness has to stay +// open: the host polls it before it holds any session, and a gated +// probe turns a bad token into what looks like a container that never +// started. +const UNAUTHENTICATED_PATHS = new Set(["/health"]); + +// When RPC_CLIENT_SECRET is set, every other route requires it as a +// bearer token. When it is unset there is nothing to check and the +// surface is open, which is how the harnesses and local runs work. +function isAuthorized(request: IncomingMessage, secret: string | undefined): boolean { + if (secret === undefined) return true; + const header = request.headers.authorization; + if (typeof header !== "string") return false; + const scheme = "Bearer "; + if (!header.startsWith(scheme)) return false; + const presented = Buffer.from(header.slice(scheme.length)); + const expected = Buffer.from(secret); + // timingSafeEqual throws on a length mismatch, and the length of the + // secret is not worth leaking through the comparison either. + if (presented.length !== expected.length) return false; + return timingSafeEqual(presented, expected); +} + // Strip heartbeat events from a Runner stream before it reaches the // RPC layer. Heartbeats are a local observability signal only; the // computer-rpc wire contract carries only stdout, stderr, and exit. @@ -162,6 +186,7 @@ interface HTTPHandle { function createHTTPServer( info: ComputerdInfo, rpc: ReturnType, + secret: string | undefined, getStats?: () => Record, ): HTTPHandle { // Holds the current outbound capnweb session opened via /connect. @@ -172,6 +197,16 @@ function createHTTPServer( const server = createServer((request, response) => { const path = requestPath(request); + // Checked before routing so an unauthorized caller cannot learn + // which routes exist. + if (!UNAUTHENTICATED_PATHS.has(path) && !isAuthorized(request, secret)) { + send(response, 401, "unauthorized\n", { + "www-authenticate": "Bearer", + "content-type": "text/plain; charset=utf-8", + }); + return; + } + // /api — the capnweb endpoint. It carries one transport, a // websocket, so a request that reaches the ordinary handler here // has no Upgrade header and cannot be served. Say so plainly: @@ -307,6 +342,13 @@ function createHTTPServer( acceptWebSocketSession(ws, rpc); }); server.on("upgrade", (request, socket, head) => { + if (!isAuthorized(request, secret)) { + socket.write( + "HTTP/1.1 401 Unauthorized\r\nWWW-Authenticate: Bearer\r\nConnection: close\r\n\r\n", + ); + socket.destroy(); + return; + } if (requestPath(request) !== "/api") { socket.write("HTTP/1.1 404 Not Found\r\n\r\n"); socket.destroy(); @@ -522,6 +564,13 @@ async function main(): Promise { rejectLegacyFuseEnv(process.env); + // Read the shared secret once and drop it from the environment. The + // exec allowlist already keeps it out of spawned commands; removing + // it here means a future code path that spreads process.env cannot + // reintroduce the leak. + const clientSecret = process.env.RPC_CLIENT_SECRET?.trim() || undefined; + delete process.env.RPC_CLIENT_SECRET; + const port = parsePort(process.env.PORT); const mountPoint = parseMountPoint(process.env.MOUNT_POINT); // FUSE_MOUNT picks the backend. auto (default) probes /dev/fuse @@ -615,7 +664,7 @@ async function main(): Promise { } : {}), }); - const http = createHTTPServer(info, rpc, () => ({ + const http = createHTTPServer(info, rpc, clientSecret, () => ({ ...collectDbStats(db), ...(fuse?.getBufferStats?.() ?? {}), })); From 7aea6130efcdc171d79d9ec880df9195040bfb30 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:08:00 +0000 Subject: [PATCH 03/10] computer: Authorize container requests with a durable secret The container now requires a bearer token on its HTTP surface when RPC_CLIENT_SECRET is set. The host generates one, hands it to the container in that variable at launch, and presents it on POST /connect. The secret is persisted next to the container runtime identity rather than generated per call, because a durable object can be reconstructed while its container keeps running. start() does not relaunch in that case, so the container still holds the environment from its original launch; a fresh secret each incarnation would not match and every request would be refused, breaking the reconnect POST /connect exists to serve. Persisting also covers a replaced container, which is launched with the value already stored. Thirty-two hex characters from getRandomValues, carrying 128 bits. ContainerRuntimeInfo gained the secret so it reaches the backend by the same route as the runtime id, which the backend already threads from start() through to the connect request. --- .../container/cloudflare-container.test.ts | 15 ++++- .../container/cloudflare-container.ts | 19 ++++-- .../container/container-client-secret.test.ts | 67 +++++++++++++++++++ .../container/container-client-secret.ts | 42 ++++++++++++ .../src/backends/container/container-host.ts | 26 +++++-- 5 files changed, 156 insertions(+), 13 deletions(-) create mode 100644 packages/computer/src/backends/container/container-client-secret.test.ts create mode 100644 packages/computer/src/backends/container/container-client-secret.ts diff --git a/packages/computer/src/backends/container/cloudflare-container.test.ts b/packages/computer/src/backends/container/cloudflare-container.test.ts index 51eebe92..e8dd13bf 100644 --- a/packages/computer/src/backends/container/cloudflare-container.test.ts +++ b/packages/computer/src/backends/container/cloudflare-container.test.ts @@ -35,6 +35,8 @@ interface FakeHost { host: IWorkspaceContainerAPI; calls: { name: string; args: unknown[] }[]; connectBody?: Record; + connectAuthorization?: string | null; + clientSecret?: string; startEnv?: Record; enableInternet?: boolean; interceptedHost?: string; @@ -77,11 +79,15 @@ function makeFakeHost(opts: FakeHostOptions = {}): FakeHost { state.enableInternet = enableInternet; await opts.start?.(); if (!state.running) state.runtimeId = crypto.randomUUID(); + state.clientSecret ??= "00112233445566778899aabbccddeeff"; state.running = true; // A successful start clears any prior exit, matching // WorkspaceContainerAPI.start. state.exit = null; - return { runtimeId: state.runtimeId ?? "missing-runtime" }; + return { + runtimeId: state.runtimeId ?? "missing-runtime", + clientSecret: state.clientSecret ?? "unset", + }; }, async interceptOutboundHttp(host, ref) { calls.push({ name: "interceptOutboundHttp", args: [host, ref] }); @@ -107,6 +113,7 @@ function makeFakeHost(opts: FakeHostOptions = {}): FakeHost { .clone() .json() .catch(() => undefined); + state.connectAuthorization = request.headers.get("authorization"); if (connectStatus !== 200) { return new Response(`/connect ${connectStatus}`, { status: connectStatus }); } @@ -125,7 +132,8 @@ function makeFakeHost(opts: FakeHostOptions = {}): FakeHost { state.running = true; state.exit = null; state.runtimeId = crypto.randomUUID(); - return { runtimeId: state.runtimeId }; + state.clientSecret ??= "00112233445566778899aabbccddeeff"; + return { runtimeId: state.runtimeId, clientSecret: state.clientSecret }; }, async status() { calls.push({ name: "status", args: [] }); @@ -404,6 +412,9 @@ describe("CloudflareContainerBackend", () => { api: "/api", }); expect(typeof fake.connectBody?.healthTimeoutMs).toBe("number"); + // The daemon refuses an unauthorized request once it has a secret, + // so the bearer token travels with the dial-back instruction. + expect(fake.connectAuthorization).toBe(`Bearer ${fake.clientSecret}`); }); test("connect() throws a transport error when the /api upgrade never arrives", async () => { diff --git a/packages/computer/src/backends/container/cloudflare-container.ts b/packages/computer/src/backends/container/cloudflare-container.ts index 155a7cb1..c4b7aa63 100644 --- a/packages/computer/src/backends/container/cloudflare-container.ts +++ b/packages/computer/src/backends/container/cloudflare-container.ts @@ -216,8 +216,12 @@ export class CloudflareContainerBackend implements WorkspaceBackend { ...this.#options.containerEnv, }; let runtimeId: string; + // The container requires this on its HTTP surface. It is durable, so + // the value is the same across incarnations and across a replaced + // container. + let clientSecret: string; try { - ({ runtimeId } = await host.start(env, this.#egress.mode === "direct")); + ({ runtimeId, clientSecret } = await host.start(env, this.#egress.mode === "direct")); } catch (error) { throw new WorkspaceTransportError( this.#formatStageError("start", { @@ -254,7 +258,7 @@ export class CloudflareContainerBackend implements WorkspaceBackend { this.#armUpgrade(); runtimeId = await this.#readyWithRestarts(host, env, deadline, priorExit, runtimeId); - await this.#postConnect(host, deadline); + await this.#postConnect(host, deadline, clientSecret); const ws = await this.#waitForUpgrade(deadline); const stub = newWebSocketRpcSession( @@ -519,13 +523,20 @@ export class CloudflareContainerBackend implements WorkspaceBackend { ); } - async #postConnect(host: IWorkspaceContainerAPI, deadline: number): Promise { + async #postConnect( + host: IWorkspaceContainerAPI, + deadline: number, + clientSecret: string, + ): Promise { const remaining = Math.max(0, deadline - Date.now()); let res: Response; try { res = await host.fetchPort(this.#options.containerPort, "http://container/connect", { method: "POST", - headers: { "content-type": "application/json" }, + headers: { + "content-type": "application/json", + authorization: `Bearer ${clientSecret}`, + }, body: JSON.stringify({ base: `http://${this.#options.egressHost}`, health: EGRESS_HEALTH_PATH, diff --git a/packages/computer/src/backends/container/container-client-secret.test.ts b/packages/computer/src/backends/container/container-client-secret.test.ts new file mode 100644 index 00000000..3368f8d0 --- /dev/null +++ b/packages/computer/src/backends/container/container-client-secret.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, test } from "vitest"; + +import { ContainerClientSecret } from "./container-client-secret.js"; + +function fakeStorage(initial: Record = {}) { + const values = new Map(Object.entries(initial)); + let writes = 0; + return { + writes: () => writes, + values, + async get(key: string): Promise { + return values.get(key) as T | undefined; + }, + async put(key: string, value: unknown): Promise { + writes++; + values.set(key, value); + }, + }; +} + +describe("ContainerClientSecret", () => { + test("generates 32 hex characters on first use", async () => { + const storage = fakeStorage(); + const secret = await new ContainerClientSecret(storage).ensure(); + + expect(secret).toMatch(/^[0-9a-f]{32}$/); + expect(storage.writes()).toBe(1); + }); + + test("returns the stored value on later calls without rewriting it", async () => { + const storage = fakeStorage(); + const store = new ContainerClientSecret(storage); + + const first = await store.ensure(); + const second = await store.ensure(); + + expect(second).toBe(first); + expect(storage.writes()).toBe(1); + }); + + test("a reconstructed instance reuses the persisted value", async () => { + // This is the case that matters: a durable object rebuilt against a + // container that is still running has to present the secret that + // container was launched with. + const storage = fakeStorage(); + const before = await new ContainerClientSecret(storage).ensure(); + + const after = await new ContainerClientSecret(storage).ensure(); + + expect(after).toBe(before); + }); + + test("two workspaces do not share a secret", async () => { + const one = await new ContainerClientSecret(fakeStorage()).ensure(); + const two = await new ContainerClientSecret(fakeStorage()).ensure(); + + expect(one).not.toBe(two); + }); + + test("replaces a stored value that is empty", async () => { + const storage = fakeStorage({ "computer:container-client-secret": "" }); + + const secret = await new ContainerClientSecret(storage).ensure(); + + expect(secret).toMatch(/^[0-9a-f]{32}$/); + }); +}); diff --git a/packages/computer/src/backends/container/container-client-secret.ts b/packages/computer/src/backends/container/container-client-secret.ts new file mode 100644 index 00000000..b7bb2ea7 --- /dev/null +++ b/packages/computer/src/backends/container/container-client-secret.ts @@ -0,0 +1,42 @@ +// Shared secret authorizing the host's requests to the container's HTTP +// surface. +// +// It has to be durable rather than generated per call. A durable object +// can be reconstructed while its container keeps running, and in that +// case start() does not relaunch, so the environment the container holds +// is the one from the original launch. A fresh secret each incarnation +// would not match it and every request would be refused, breaking the +// reconnect that POST /connect exists to serve. +// +// Persisting it also covers the other direction: when a replacement +// container is launched, it receives the value already stored, so both +// ends stay in step without the host having to read anything back. + +interface ClientSecretStorage { + get(key: string): Promise; + put(key: string, value: unknown): Promise; +} + +const STORAGE_KEY = "computer:container-client-secret"; + +// 16 random bytes, hex encoded: 32 characters carrying 128 bits. +const SECRET_BYTES = 16; + +function generate(): string { + const bytes = crypto.getRandomValues(new Uint8Array(SECRET_BYTES)); + return Array.from(bytes, (byte) => byte.toString(16).padStart(2, "0")).join(""); +} + +export class ContainerClientSecret { + constructor(private readonly storage: ClientSecretStorage) {} + + // Returns the stored secret, generating and persisting one the first + // time. Callers may invoke this on every start; only the first writes. + async ensure(): Promise { + const existing = await this.storage.get(STORAGE_KEY); + if (typeof existing === "string" && existing.length > 0) return existing; + const secret = generate(); + await this.storage.put(STORAGE_KEY, secret); + return secret; + } +} diff --git a/packages/computer/src/backends/container/container-host.ts b/packages/computer/src/backends/container/container-host.ts index eba11ba9..d90d27ae 100644 --- a/packages/computer/src/backends/container/container-host.ts +++ b/packages/computer/src/backends/container/container-host.ts @@ -20,6 +20,7 @@ import { RpcTarget } from "cloudflare:workers"; import { WorkspaceTransportError } from "../../transport-failure.js"; +import { ContainerClientSecret } from "./container-client-secret.js"; import { type ContainerExitInfo, containerExitInfo, @@ -48,6 +49,10 @@ export interface WorkspaceRef { // the `ws` accessor that withWorkspaceContainer installs. export interface ContainerRuntimeInfo { runtimeId: string; + // Shared secret the container requires on its HTTP surface. Durable, + // so a reconstructed durable object presents the value the running + // container was launched with. + clientSecret: string; } export interface IWorkspaceContainerAPI { @@ -104,6 +109,7 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai readonly #container: NonNullable; readonly #ctx: DurableObjectState; readonly #runtimeIdentity: CurrentContainerRuntimeIdentity; + readonly #clientSecret: ContainerClientSecret; constructor(ctx: DurableObjectState) { super(); @@ -113,6 +119,7 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai this.#container = ctx.container; this.#ctx = ctx; this.#runtimeIdentity = new CurrentContainerRuntimeIdentity(ctx.storage); + this.#clientSecret = new ContainerClientSecret(ctx.storage); } async start(env: Record, enableInternet: boolean) { @@ -123,6 +130,10 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai // destroy resolves, so guarding the start against it would let // a stale-running flag skip the re-launch entirely. const priorExit = containerExitInfo(this.#ctx); + // Resolved before any launch so the value written into the + // container's environment is the same one later incarnations read + // back and present on their requests. + const clientSecret = await this.#clientSecret.ensure(); let runtime: ContainerRuntimeIdentity; if (priorExit !== null) { try { @@ -131,9 +142,9 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai // best-effort — the next start() will surface any real // platform-side failure. } - runtime = await this.#launch(env, enableInternet); + runtime = await this.#launch(env, enableInternet, clientSecret); } else if (!this.#container.running) { - runtime = await this.#launch(env, enableInternet); + runtime = await this.#launch(env, enableInternet, clientSecret); } else { // A Durable Object incarnation can be reconstructed while its // container stays alive. Reuse the durable runtime id rather @@ -141,7 +152,7 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai runtime = (await this.#runtimeIdentity.get()) ?? (await this.#runtimeIdentity.markStarted()); } installContainerMonitor(this.#ctx, this.#container, () => this.#runtimeIdentity.clear(runtime)); - return { runtimeId: runtime.id }; + return { runtimeId: runtime.id, clientSecret }; } async restart(env: Record, enableInternet: boolean) { @@ -157,15 +168,16 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai // succeed against a fresh generation or surface its own // failure. } - const runtime = await this.#launch(env, enableInternet); + const clientSecret = await this.#clientSecret.ensure(); + const runtime = await this.#launch(env, enableInternet, clientSecret); installContainerMonitor(this.#ctx, this.#container, () => this.#runtimeIdentity.clear(runtime)); - return { runtimeId: runtime.id }; + return { runtimeId: runtime.id, clientSecret }; } - async #launch(env: Record, enableInternet: boolean) { + async #launch(env: Record, enableInternet: boolean, clientSecret: string) { const runtime = await this.#runtimeIdentity.markStarted(); try { - this.#container.start({ enableInternet, env }); + this.#container.start({ enableInternet, env: { ...env, RPC_CLIENT_SECRET: clientSecret } }); return runtime; } catch (error) { await this.#runtimeIdentity.clear(runtime); From dbf7c9e34afcb62229c727c42c67fcf1263a0bfb Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:09:59 +0000 Subject: [PATCH 04/10] docs: Record the shared secret and the exec environment allowlist Add RPC_CLIENT_SECRET and COMPUTER_VAR_ to the environment tables, note on the route table that /health is the only route reachable without the secret, and say in the daemon readme what an unauthorized request gets. Changeset entries for both, since each is a change a consumer may have to act on: a workspace reading an inherited variable now needs the prefix. --- .changeset/authorize-container-http.md | 5 +++++ .changeset/exec-env-allowlist.md | 5 +++++ docs/07_injected_service.md | 4 +++- packages/computerd/README.md | 9 ++++++++- 4 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 .changeset/authorize-container-http.md create mode 100644 .changeset/exec-env-allowlist.md diff --git a/.changeset/authorize-container-http.md b/.changeset/authorize-container-http.md new file mode 100644 index 00000000..5a30d69c --- /dev/null +++ b/.changeset/authorize-container-http.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/computer": minor +--- + +The container's HTTP surface now requires a bearer token. The host generates a secret, passes it to the container as RPC_CLIENT_SECRET at launch, and sends it on /connect. Readiness at /health stays open, and leaving the variable unset disables the checks. diff --git a/.changeset/exec-env-allowlist.md b/.changeset/exec-env-allowlist.md new file mode 100644 index 00000000..a9dc991a --- /dev/null +++ b/.changeset/exec-env-allowlist.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/computer": minor +--- + +A command run through the shell no longer inherits the container's whole environment. It receives PATH, HOME, TMPDIR, TZ, LANG, TERM, the LC_ family, and any variable prefixed COMPUTER_VAR_, which arrives with the prefix stripped so COMPUTER_VAR_NODE_ENV becomes NODE_ENV. A workspace that relied on some other inherited variable needs the prefix. diff --git a/docs/07_injected_service.md b/docs/07_injected_service.md index e3b8b58f..91d167c0 100644 --- a/docs/07_injected_service.md +++ b/docs/07_injected_service.md @@ -49,7 +49,7 @@ backend pins it to `8080`) and serves: | Route | Method | Purpose | | --- | --- | --- | -| `/health` | `GET`, `HEAD` | Liveness probe; `200 ok\n` as soon as the HTTP server binds. | +| `/health` | `GET`, `HEAD` | Liveness probe; `200 ok\n` as soon as the HTTP server binds. The only route reachable without the shared secret. | | `/__computerd/info` | `GET` | Runtime info: FUSE backend, mount point, port. | | `/api` | `GET` (upgrade) | WebSocket capnweb transport — the bootstrap stub is `WorkspaceRPC`. Only the exact path upgrades. A request without an `Upgrade` header gets `400`; an unsupported `Sec-WebSocket-Version` gets `426` and the versions the server speaks. | | `/api/watermarks` | `GET`, `HEAD` | Sync revisions: `currentRev`, `pushRev`, `fetchCursor`. The same values `sync.watermarks()` returns, for callers that want a few numbers without holding a session. | @@ -163,6 +163,8 @@ These are the variables `computerd` actually consumes (see | `MOUNT_POINT` | `/workspace` | Absolute path inside the container to mount the FUSE filesystem at. Ignored when `FUSE_MOUNT=none`. | | `FUSE_MOUNT` | `auto` | Backend selector: `auto` probes `/dev/fuse` (linux) or macFUSE (darwin) and falls back to the userspace shim; `fuse` / `macfuse` require the corresponding real backend; `shim` forces the userspace shim; `none` skips the mount entirely. | | `EXEC_LOG_MAX_BYTES` | runner default | Caps the per-exec stdout/stderr log retained in-memory. | +| `RPC_CLIENT_SECRET` | unset | When set, every route except `/health` requires it as `Authorization: Bearer `, including the `/api` upgrade. Unset leaves the surface open. The Cloudflare backend generates one per workspace and sets it at launch. | +| `COMPUTER_VAR_*` | unset | Forwarded into every `shell.exec` command with the prefix stripped, so `COMPUTER_VAR_NODE_ENV` arrives as `NODE_ENV`. | | `LOG_FILE` | unset | If set, every `console.log` / `console.error` line and any `uncaughtException` / `unhandledRejection` is also appended to this file. Stdout/stderr behaviour is unchanged. | When `LOG_FILE` is set, `computerd` mirrors console output into the file in diff --git a/packages/computerd/README.md b/packages/computerd/README.md index 7ba1088e..64345557 100644 --- a/packages/computerd/README.md +++ b/packages/computerd/README.md @@ -33,6 +33,11 @@ Current endpoints: All other paths and methods return `404`/`405` with a `text/plain` body. +When `RPC_CLIENT_SECRET` is set, every route above except `/health` requires +it as `Authorization: Bearer `, and so does the `/api` upgrade. +Requests without it get `401`. Leaving the variable unset disables the +check, which is what the container harnesses rely on. + Current filesystem support: - `@platformatic/vfs` in-memory filesystem provided by `@cloudflare/dofs`'s node provider. @@ -107,7 +112,9 @@ FUSE_MOUNT=none # skip the mount entirely; HTTP and /api still come up Additional environment variables: ```sh -EXEC_LOG_MAX_BYTES=1048576 # cap the in-memory exec log buffer (bytes) +EXEC_LOG_MAX_BYTES=1048576 # cap the in-memory exec log buffer (bytes) +RPC_CLIENT_SECRET= # require Authorization: Bearer on every route but /health +COMPUTER_VAR_NODE_ENV=production # forwarded into exec as NODE_ENV ``` `FUSE_MOUNT=auto` is the friendly default: if `/dev/fuse` (or macFUSE) is available `computerd` mounts a real FUSE filesystem, otherwise it transparently falls back to the userspace shim. Pin the value (`fuse` / `macfuse` / `shim` / `none`) when a test needs to assert a specific code path. From 90e8caa15b7a4d8cf2f39bef1724808f81140676 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Tue, 18 Aug 2026 11:24:18 +0000 Subject: [PATCH 05/10] computer: Relaunch a container whose launch spec does not match The environment and the outbound-internet flag can only be set when a container process starts. start() accepted both, discarded them when a container was already running, and returned as though they had applied. Anything that pre-started a container therefore decided the configuration for whichever workspace later adopted it, and nothing detected the disagreement. The warm pool in examples/think-compare-runtimes is the case in point. It starts containers itself, so the shared secret was never injected and the daemon ran with its HTTP surface unauthenticated while the host sent a bearer token it was happy to ignore. The internet flag has the same shape: the pool hardcodes it on, and a workspace configured for the none or http-gateway egress modes would have inherited direct internet access regardless. The two launch inputs are now one ContainerLaunchSpec, and each launch records what it used beside the runtime identity: the internet flag, and a digest of the environment rather than the environment itself, since containerEnv is consumer-supplied and may hold their own secrets. Adoption compares, and relaunches on any difference. A container started outside this API leaves no record, which reads as a mismatch, so it is replaced rather than trusted. That restores the guarantee even for a caller that never adopts the new entry point. ContainerRuntimeInfo reports which of launched, adopted or relaunched happened, and a relaunch keeps the durable secret so the replacement receives the value the host already holds. setInactivityTimeout joins the interface so a warm pool has no reason to reach past it to ctx.container. --- .../container/cloudflare-container.test.ts | 20 ++- .../container/cloudflare-container.ts | 10 +- .../container/container-host-adoption.test.ts | 162 ++++++++++++++++++ .../src/backends/container/container-host.ts | 108 +++++++++--- .../container/container-launch-record.test.ts | 96 +++++++++++ .../container/container-launch-record.ts | 70 ++++++++ 6 files changed, 435 insertions(+), 31 deletions(-) create mode 100644 packages/computer/src/backends/container/container-host-adoption.test.ts create mode 100644 packages/computer/src/backends/container/container-launch-record.test.ts create mode 100644 packages/computer/src/backends/container/container-launch-record.ts diff --git a/packages/computer/src/backends/container/cloudflare-container.test.ts b/packages/computer/src/backends/container/cloudflare-container.test.ts index e8dd13bf..5b0bfcfb 100644 --- a/packages/computer/src/backends/container/cloudflare-container.test.ts +++ b/packages/computer/src/backends/container/cloudflare-container.test.ts @@ -73,7 +73,8 @@ function makeFakeHost(opts: FakeHostOptions = {}): FakeHost { } state.host = { - async start(env, enableInternet) { + async start(spec) { + const { env, enableInternet } = spec; calls.push({ name: "start", args: [env, enableInternet] }); state.startEnv = env; state.enableInternet = enableInternet; @@ -87,6 +88,7 @@ function makeFakeHost(opts: FakeHostOptions = {}): FakeHost { return { runtimeId: state.runtimeId ?? "missing-runtime", clientSecret: state.clientSecret ?? "unset", + outcome: "launched", }; }, async interceptOutboundHttp(host, ref) { @@ -109,10 +111,10 @@ function makeFakeHost(opts: FakeHostOptions = {}): FakeHost { return new Response(null, { status: 200 }); } if (url.pathname === "/connect") { - state.connectBody = await request + state.connectBody = (await request .clone() .json() - .catch(() => undefined); + .catch(() => undefined)) as Record | undefined; state.connectAuthorization = request.headers.get("authorization"); if (connectStatus !== 200) { return new Response(`/connect ${connectStatus}`, { status: connectStatus }); @@ -124,7 +126,8 @@ function makeFakeHost(opts: FakeHostOptions = {}): FakeHost { port() { throw new Error("cross-boundary Fetchers should not be used by CloudflareContainerBackend"); }, - async restart(env, enableInternet) { + async restart(spec) { + const { env, enableInternet } = spec; calls.push({ name: "restart", args: [env, enableInternet] }); if (opts.restart) { await opts.restart(); @@ -133,7 +136,14 @@ function makeFakeHost(opts: FakeHostOptions = {}): FakeHost { state.exit = null; state.runtimeId = crypto.randomUUID(); state.clientSecret ??= "00112233445566778899aabbccddeeff"; - return { runtimeId: state.runtimeId, clientSecret: state.clientSecret }; + return { + runtimeId: state.runtimeId, + clientSecret: state.clientSecret, + outcome: "launched", + }; + }, + async setInactivityTimeout(durationMs: number) { + calls.push({ name: "setInactivityTimeout", args: [durationMs] }); }, async status() { calls.push({ name: "status", args: [] }); diff --git a/packages/computer/src/backends/container/cloudflare-container.ts b/packages/computer/src/backends/container/cloudflare-container.ts index c4b7aa63..2299e57e 100644 --- a/packages/computer/src/backends/container/cloudflare-container.ts +++ b/packages/computer/src/backends/container/cloudflare-container.ts @@ -221,7 +221,10 @@ export class CloudflareContainerBackend implements WorkspaceBackend { // container. let clientSecret: string; try { - ({ runtimeId, clientSecret } = await host.start(env, this.#egress.mode === "direct")); + ({ runtimeId, clientSecret } = await host.start({ + env, + enableInternet: this.#egress.mode === "direct", + })); } catch (error) { throw new WorkspaceTransportError( this.#formatStageError("start", { @@ -446,7 +449,10 @@ export class CloudflareContainerBackend implements WorkspaceBackend { if (attempt < maxAttempts) { try { - ({ runtimeId } = await host.restart(env, this.#egress.mode === "direct")); + ({ runtimeId } = await host.restart({ + env, + enableInternet: this.#egress.mode === "direct", + })); restarts++; } catch (error) { this.#rejectUpgrade?.(error); diff --git a/packages/computer/src/backends/container/container-host-adoption.test.ts b/packages/computer/src/backends/container/container-host-adoption.test.ts new file mode 100644 index 00000000..aa28d558 --- /dev/null +++ b/packages/computer/src/backends/container/container-host-adoption.test.ts @@ -0,0 +1,162 @@ +// What start() does when it finds a container already running. +// +// The environment and the outbound-internet flag can only be set at +// launch, so adopting a container launched with different ones would +// silently discard what the caller asked for. These cover the three +// outcomes and, in particular, the case that motivated them: a warm +// pool that started the container itself and left no record. +import { describe, expect, test, vi } from "vitest"; + +import { WorkspaceContainerAPI } from "./container-host.js"; +import type { ContainerLaunchSpec } from "./container-launch-record.js"; + +function fakeCtx(options: { running?: boolean } = {}) { + const values = new Map(); + const starts: { enableInternet: boolean; env: Record }[] = []; + let destroys = 0; + // monitor() has to settle when the container goes away: the destroy + // path waits on the current generation's monitor before the caller + // installs the next one. + let exited: (() => void) | undefined; + const container = { + running: options.running ?? false, + start(spec: { enableInternet: boolean; env: Record }) { + starts.push(spec); + container.running = true; + }, + async destroy() { + destroys += 1; + container.running = false; + exited?.(); + exited = undefined; + }, + async setInactivityTimeout() {}, + monitor: () => + new Promise((resolve) => { + exited = resolve; + }), + getTcpPort: () => ({}) as Fetcher, + }; + const ctx = { + container, + storage: { + async get(key: string): Promise { + return values.get(key) as T | undefined; + }, + async put(key: string, value: unknown): Promise { + values.set(key, value); + }, + async delete(key: string): Promise { + return values.delete(key); + }, + }, + blockConcurrencyWhile: async (fn: () => Promise) => fn(), + } as unknown as DurableObjectState; + return { ctx, container, starts, destroyCount: () => destroys, values }; +} + +const spec: ContainerLaunchSpec = { + env: { PORT: "8080", MOUNT_POINT: "/workspace" }, + enableInternet: false, +}; + +describe("WorkspaceContainerAPI.start", () => { + test("launches when nothing is running", async () => { + const { ctx, starts } = fakeCtx(); + + const info = await new WorkspaceContainerAPI(ctx).start(spec); + + expect(info.outcome).toBe("launched"); + expect(starts).toHaveLength(1); + expect(starts[0]?.enableInternet).toBe(false); + expect(starts[0]?.env.RPC_CLIENT_SECRET).toMatch(/^[0-9a-f]{32}$/); + }); + + test("adopts a container it launched with the same spec", async () => { + const { ctx, starts, destroyCount } = fakeCtx(); + const api = new WorkspaceContainerAPI(ctx); + const first = await api.start(spec); + + const second = await api.start(spec); + + expect(second.outcome).toBe("adopted"); + expect(second.runtimeId).toBe(first.runtimeId); + expect(second.clientSecret).toBe(first.clientSecret); + expect(starts).toHaveLength(1); + expect(destroyCount()).toBe(0); + }); + + test("relaunches a container started outside this API", async () => { + // The warm-pool case. A container running with no launch record was + // started by something that never injected the secret, so adopting + // it would leave the daemon unauthenticated and any requested + // environment unapplied. + const { ctx, container, starts, destroyCount } = fakeCtx(); + container.running = true; + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const info = await new WorkspaceContainerAPI(ctx).start(spec); + + expect(info.outcome).toBe("relaunched"); + expect(destroyCount()).toBe(1); + expect(starts).toHaveLength(1); + expect(starts[0]?.env.RPC_CLIENT_SECRET).toMatch(/^[0-9a-f]{32}$/); + expect(warn).toHaveBeenCalled(); + } finally { + warn.mockRestore(); + } + }); + + test("relaunches when the internet flag differs", async () => { + // The egress case: a container launched with the internet enabled + // must not serve a workspace that asked for it off. + const { ctx, starts } = fakeCtx(); + const api = new WorkspaceContainerAPI(ctx); + await api.start({ ...spec, enableInternet: true }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const info = await api.start({ ...spec, enableInternet: false }); + + expect(info.outcome).toBe("relaunched"); + expect(starts).toHaveLength(2); + expect(starts[1]?.enableInternet).toBe(false); + } finally { + warn.mockRestore(); + } + }); + + test("relaunches when the environment differs", async () => { + const { ctx, starts } = fakeCtx(); + const api = new WorkspaceContainerAPI(ctx); + await api.start(spec); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const info = await api.start({ + ...spec, + env: { ...spec.env, FUSE_MOUNT: "none" }, + }); + + expect(info.outcome).toBe("relaunched"); + expect(starts).toHaveLength(2); + expect(starts[1]?.env.FUSE_MOUNT).toBe("none"); + } finally { + warn.mockRestore(); + } + }); + + test("keeps the same secret across a relaunch", async () => { + // The secret is durable, so a replacement container is launched with + // the value the host already holds. + const { ctx } = fakeCtx(); + const api = new WorkspaceContainerAPI(ctx); + const first = await api.start(spec); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const second = await api.start({ ...spec, enableInternet: true }); + + expect(second.clientSecret).toBe(first.clientSecret); + } finally { + warn.mockRestore(); + } + }); +}); diff --git a/packages/computer/src/backends/container/container-host.ts b/packages/computer/src/backends/container/container-host.ts index d90d27ae..848a1b36 100644 --- a/packages/computer/src/backends/container/container-host.ts +++ b/packages/computer/src/backends/container/container-host.ts @@ -21,16 +21,20 @@ import { RpcTarget } from "cloudflare:workers"; import { WorkspaceTransportError } from "../../transport-failure.js"; import { ContainerClientSecret } from "./container-client-secret.js"; +import { + type ContainerLaunchRecord, + type ContainerLaunchSpec, + CurrentContainerLaunchRecord, + launchRecordFor, + sameLaunch, +} from "./container-launch-record.js"; import { type ContainerExitInfo, containerExitInfo, destroyContainerExpectingExit, installContainerMonitor, } from "./container-lifecycle.js"; -import { - type ContainerRuntimeIdentity, - CurrentContainerRuntimeIdentity, -} from "./container-runtime-identity.js"; +import { CurrentContainerRuntimeIdentity } from "./container-runtime-identity.js"; export type { ContainerExitInfo } from "./container-lifecycle.js"; @@ -53,13 +57,20 @@ export interface ContainerRuntimeInfo { // so a reconstructed durable object presents the value the running // container was launched with. clientSecret: string; + // What the call actually did. `adopted` means a container was already + // running with the requested launch spec and was reused. `relaunched` + // means one was running with a different spec, or with none recorded, + // and had to be replaced: the environment and the internet flag can + // only be set at launch, so adopting it would have quietly ignored + // what the caller asked for. + outcome: "launched" | "adopted" | "relaunched"; } export interface IWorkspaceContainerAPI { // Idempotent start. Returns the durable identity of the running // container process once the runtime has accepted the start command; // readiness is verified by the backend through probeComputerdHealth. - start(env: Record, enableInternet: boolean): Promise; + start(spec: ContainerLaunchSpec): Promise; // Wire `host` → workspace inside the container's egress table. // Called once per backend connect(). The implementation @@ -82,7 +93,12 @@ export interface IWorkspaceContainerAPI { // current generation dead. Implementation: destroy() the // container, then start({ env }). Callers bound the number of // restart attempts — this method does no looping of its own. - restart(env: Record, enableInternet: boolean): Promise; + restart(spec: ContainerLaunchSpec): Promise; + + // Set the platform's idle timeout for the attached container. Exposed + // so a caller that pre-starts containers, such as a warm pool, never + // needs to reach past this API to ctx.container. + setInactivityTimeout(durationMs: number): Promise; // Coarse diagnostic state. The `running` flag reports whether // the platform still has a container instance attached; it does @@ -110,6 +126,7 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai readonly #ctx: DurableObjectState; readonly #runtimeIdentity: CurrentContainerRuntimeIdentity; readonly #clientSecret: ContainerClientSecret; + readonly #launchRecord: CurrentContainerLaunchRecord; constructor(ctx: DurableObjectState) { super(); @@ -120,9 +137,10 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai this.#ctx = ctx; this.#runtimeIdentity = new CurrentContainerRuntimeIdentity(ctx.storage); this.#clientSecret = new ContainerClientSecret(ctx.storage); + this.#launchRecord = new CurrentContainerLaunchRecord(ctx.storage); } - async start(env: Record, enableInternet: boolean) { + async start(spec: ContainerLaunchSpec): Promise { // If a prior generation has died, commit to a fresh one: the // destroy clears any platform-side carcass, and the start that // follows is unconditional. We cannot rely on @@ -134,7 +152,8 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai // container's environment is the same one later incarnations read // back and present on their requests. const clientSecret = await this.#clientSecret.ensure(); - let runtime: ContainerRuntimeIdentity; + const requested = await launchRecordFor(spec); + if (priorExit !== null) { try { await destroyContainerExpectingExit(this.#ctx, this.#container); @@ -142,20 +161,47 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai // best-effort — the next start() will surface any real // platform-side failure. } - runtime = await this.#launch(env, enableInternet, clientSecret); - } else if (!this.#container.running) { - runtime = await this.#launch(env, enableInternet, clientSecret); - } else { - // A Durable Object incarnation can be reconstructed while its - // container stays alive. Reuse the durable runtime id rather - // than treating the new WebSocket as a new process. - runtime = (await this.#runtimeIdentity.get()) ?? (await this.#runtimeIdentity.markStarted()); + return this.#launchAs(spec, clientSecret, requested, "launched"); + } + if (!this.#container.running) { + return this.#launchAs(spec, clientSecret, requested, "launched"); + } + + // A container is already running. Adopting it is only correct if it + // was launched with the spec being asked for now: the environment + // and the internet flag cannot be changed on a live container, so + // adopting a mismatched one would silently drop what this caller + // wants. An absent record means something started the container + // without going through here, which is the same problem. + const actual = await this.#launchRecord.get(); + if (actual === null || !sameLaunch(actual, requested)) { + console.warn({ + message: + actual === null + ? "container was started outside WorkspaceContainerAPI; relaunching so the requested environment applies" + : "running container was launched with a different spec; relaunching", + component: "workspace-container", + requested, + actual, + }); + try { + await destroyContainerExpectingExit(this.#ctx, this.#container); + } catch { + // best-effort, as above. + } + return this.#launchAs(spec, clientSecret, requested, "relaunched"); } + + // A Durable Object incarnation can be reconstructed while its + // container stays alive. Reuse the durable runtime id rather + // than treating the new WebSocket as a new process. + const runtime = + (await this.#runtimeIdentity.get()) ?? (await this.#runtimeIdentity.markStarted()); installContainerMonitor(this.#ctx, this.#container, () => this.#runtimeIdentity.clear(runtime)); - return { runtimeId: runtime.id, clientSecret }; + return { runtimeId: runtime.id, clientSecret, outcome: "adopted" }; } - async restart(env: Record, enableInternet: boolean) { + async restart(spec: ContainerLaunchSpec): Promise { // destroy() resolves once the platform has torn down the // attached container. A subsequent start() launches a fresh // generation — ports re-bind, the computerd daemon comes up clean. @@ -169,20 +215,34 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai // failure. } const clientSecret = await this.#clientSecret.ensure(); - const runtime = await this.#launch(env, enableInternet, clientSecret); - installContainerMonitor(this.#ctx, this.#container, () => this.#runtimeIdentity.clear(runtime)); - return { runtimeId: runtime.id, clientSecret }; + return this.#launchAs(spec, clientSecret, await launchRecordFor(spec), "launched"); + } + + async setInactivityTimeout(durationMs: number): Promise { + await this.#container.setInactivityTimeout(durationMs); } - async #launch(env: Record, enableInternet: boolean, clientSecret: string) { + async #launchAs( + spec: ContainerLaunchSpec, + clientSecret: string, + record: ContainerLaunchRecord, + outcome: "launched" | "relaunched", + ): Promise { const runtime = await this.#runtimeIdentity.markStarted(); try { - this.#container.start({ enableInternet, env: { ...env, RPC_CLIENT_SECRET: clientSecret } }); - return runtime; + this.#container.start({ + enableInternet: spec.enableInternet, + env: { ...spec.env, RPC_CLIENT_SECRET: clientSecret }, + }); } catch (error) { await this.#runtimeIdentity.clear(runtime); throw error; } + // Written after the start is accepted, so a failed launch does not + // leave a record claiming the container holds this spec. + await this.#launchRecord.set(record); + installContainerMonitor(this.#ctx, this.#container, () => this.#runtimeIdentity.clear(runtime)); + return { runtimeId: runtime.id, clientSecret, outcome }; } async status() { diff --git a/packages/computer/src/backends/container/container-launch-record.test.ts b/packages/computer/src/backends/container/container-launch-record.test.ts new file mode 100644 index 00000000..f0c96133 --- /dev/null +++ b/packages/computer/src/backends/container/container-launch-record.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, test } from "vitest"; + +import { + type ContainerLaunchSpec, + CurrentContainerLaunchRecord, + launchRecordFor, + sameLaunch, +} from "./container-launch-record.js"; + +function spec(overrides: Partial = {}): ContainerLaunchSpec { + return { + env: { PORT: "8080", MOUNT_POINT: "/workspace" }, + enableInternet: false, + ...overrides, + }; +} + +function fakeStorage(initial: Record = {}) { + const values = new Map(Object.entries(initial)); + return { + values, + async get(key: string): Promise { + return values.get(key) as T | undefined; + }, + async put(key: string, value: unknown): Promise { + values.set(key, value); + }, + }; +} + +describe("launchRecordFor", () => { + test("is stable regardless of the order keys were written in", async () => { + const one = await launchRecordFor(spec({ env: { A: "1", B: "2" } })); + const two = await launchRecordFor(spec({ env: { B: "2", A: "1" } })); + + expect(one.envDigest).toBe(two.envDigest); + expect(sameLaunch(one, two)).toBe(true); + }); + + test("changes when a value changes", async () => { + const before = await launchRecordFor(spec({ env: { FUSE_MOUNT: "auto" } })); + const after = await launchRecordFor(spec({ env: { FUSE_MOUNT: "none" } })); + + expect(sameLaunch(before, after)).toBe(false); + }); + + test("changes when a variable is added", async () => { + const before = await launchRecordFor(spec({ env: { PORT: "8080" } })); + const after = await launchRecordFor(spec({ env: { PORT: "8080", EXTRA: "" } })); + + expect(sameLaunch(before, after)).toBe(false); + }); + + test("distinguishes the internet flag", async () => { + // This is the egress case: a pool that launches with the internet + // enabled must not be adopted by a workspace that asked for it off. + const off = await launchRecordFor(spec({ enableInternet: false })); + const on = await launchRecordFor(spec({ enableInternet: true })); + + expect(sameLaunch(off, on)).toBe(false); + }); + + test("does not carry the environment in the clear", async () => { + // containerEnv is consumer-supplied and may hold their own secrets, + // so only a digest is persisted. + const record = await launchRecordFor(spec({ env: { API_TOKEN: "hunter2" } })); + + expect(JSON.stringify(record)).not.toContain("hunter2"); + expect(record.envDigest).toMatch(/^[0-9a-f]{64}$/); + }); +}); + +describe("CurrentContainerLaunchRecord", () => { + test("round-trips a record", async () => { + const storage = fakeStorage(); + const store = new CurrentContainerLaunchRecord(storage); + const record = await launchRecordFor(spec()); + + await store.set(record); + + expect(await store.get()).toEqual(record); + }); + + test("reads null before anything has been launched", async () => { + expect(await new CurrentContainerLaunchRecord(fakeStorage()).get()).toBeNull(); + }); + + test("reads null when a caller launched the container behind our back", async () => { + // A container started outside this API leaves no record. Returning + // null is what makes the adoption check relaunch it rather than + // trust it. + const storage = fakeStorage({ "computer:container-runtime-identity": { id: "abc" } }); + + expect(await new CurrentContainerLaunchRecord(storage).get()).toBeNull(); + }); +}); diff --git a/packages/computer/src/backends/container/container-launch-record.ts b/packages/computer/src/backends/container/container-launch-record.ts new file mode 100644 index 00000000..8af2b281 --- /dev/null +++ b/packages/computer/src/backends/container/container-launch-record.ts @@ -0,0 +1,70 @@ +// What the running container process was launched with. +// +// The environment and the outbound-internet flag can only be set when +// the process starts. A durable object that finds a container already +// running therefore cannot apply either one, and until it can tell what +// the container was launched with it has no way to know whether adopting +// it is safe. A warm pool that pre-starts containers is the case in +// point: the workspace that later adopts one may want a different +// environment, or no internet access at all. +// +// So each launch records what it used, and adoption compares. A +// container launched outside this API leaves no record at all, which +// reads as a mismatch and gets it relaunched rather than trusted. + +export interface ContainerLaunchSpec { + // Environment for the container image. The launch adds + // RPC_CLIENT_SECRET on top, so no caller needs to know it exists and + // it stays out of the digest below. + env: Record; + // Platform switch for outbound internet. Cannot be changed on a live + // container, which is why a mismatch has to relaunch. + enableInternet: boolean; +} + +export interface ContainerLaunchRecord { + enableInternet: boolean; + // A digest rather than the environment itself: containerEnv is + // consumer-supplied and may carry their own secrets, and this record + // only ever needs to answer "the same or not". + envDigest: string; +} + +interface LaunchRecordStorage { + get(key: string): Promise; + put(key: string, value: unknown): Promise; +} + +const STORAGE_KEY = "computer:container-launch-record"; + +export async function launchRecordFor(spec: ContainerLaunchSpec): Promise { + return { enableInternet: spec.enableInternet, envDigest: await digestEnv(spec.env) }; +} + +export function sameLaunch(a: ContainerLaunchRecord, b: ContainerLaunchRecord): boolean { + return a.enableInternet === b.enableInternet && a.envDigest === b.envDigest; +} + +// Sorted so two callers building the same environment in a different +// order agree, and length-prefixed so no combination of names and +// values can be rearranged into the same input. +async function digestEnv(env: Record): Promise { + const canonical = Object.keys(env) + .sort() + .map((name) => `${name.length}:${name}=${env[name]?.length ?? 0}:${env[name] ?? ""}`) + .join(";"); + const digest = await crypto.subtle.digest("SHA-256", new TextEncoder().encode(canonical)); + return Array.from(new Uint8Array(digest), (byte) => byte.toString(16).padStart(2, "0")).join(""); +} + +export class CurrentContainerLaunchRecord { + constructor(private readonly storage: LaunchRecordStorage) {} + + async get(): Promise { + return (await this.storage.get(STORAGE_KEY)) ?? null; + } + + async set(record: ContainerLaunchRecord): Promise { + await this.storage.put(STORAGE_KEY, record); + } +} From ce02e2507d046bfd39fb1465683517755f5b3a21 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:12:49 +0000 Subject: [PATCH 06/10] examples/think-compare-runtimes: Warm-start containers through the workspace API The pool started containers with ctx.container.start() and an environment it built itself, which skipped everything the workspace API adds. The shared secret never reached the container, so its HTTP surface stayed unauthenticated while the host sent a bearer token it ignored, and the hardcoded internet flag became whatever the adopting workspace inherited. Warm starts now go through the API, so the launch carries the secret and is recorded for the adoption check. The environment and the internet flag travel together as a ContainerLaunchSpec, which is also what the adopting workspace compares against. The retry loop calls start() unconditionally now. It adopts a container already running with the same spec, so there is nothing left for the caller to decide from a `running` flag. A pool still cannot know the egress policy of the workspace that will adopt a container, so enableInternet has to agree with it by configuration. Disagreeing now costs a relaunch on adoption rather than the policy. ContainerLaunchSpec and ContainerRuntimeInfo are exported from the container entry point, since a consumer that pre-starts containers needs both. --- .../worker/computer-container-pool.test.ts | 32 +++++++---- .../worker/computer-container-pool.ts | 56 +++++++++++-------- .../computer/src/backends/container/index.ts | 2 + 3 files changed, 56 insertions(+), 34 deletions(-) diff --git a/examples/think-compare-runtimes/worker/computer-container-pool.test.ts b/examples/think-compare-runtimes/worker/computer-container-pool.test.ts index 75ab3c6c..9249ac34 100644 --- a/examples/think-compare-runtimes/worker/computer-container-pool.test.ts +++ b/examples/think-compare-runtimes/worker/computer-container-pool.test.ts @@ -1,3 +1,4 @@ +import type { ContainerLaunchSpec } from "@cloudflare/computer/backends/container"; import { describe, expect, test, vi } from "vitest"; import type { WorkspaceContainerHost } from "./computer-container-pool"; @@ -11,8 +12,11 @@ describe("createWorkspaceWarmPoolRuntime", () => { const { createWorkspaceWarmPoolRuntime } = await import("./computer-container-pool"); const calls: string[] = []; const host = { - async startWarmContainer(env: Record, inactivityTimeoutMs: number) { - calls.push(`start ${env.PORT} ${env.MOUNT_POINT} ${env.FUSE_MOUNT} ${inactivityTimeoutMs}`); + async startWarmContainer(spec: ContainerLaunchSpec, inactivityTimeoutMs: number) { + const { env, enableInternet } = spec; + calls.push( + `start ${env.PORT} ${env.MOUNT_POINT} ${env.FUSE_MOUNT} internet=${enableInternet} ${inactivityTimeoutMs}`, + ); }, async destroyWarmContainer() { calls.push("destroy"); @@ -36,22 +40,25 @@ describe("createWorkspaceWarmPoolRuntime", () => { await runtime.startContainer("warm-a"); await expect(runtime.isContainerRunning("warm-a")).resolves.toBe(true); - expect(calls).toEqual(["start 8080 /workspace shim 120000", "healthy"]); + expect(calls).toEqual(["start 8080 /workspace shim internet=true 120000", "healthy"]); }); test("retries Workspace container placement while waiting for health", async () => { const { startWorkspaceContainerAndWait } = await import("./computer-container-pool"); const calls: string[] = []; let healthAttempts = 0; - const container = { - running: false, + // Stands in for the workspace container API rather than + // ctx.container: the warm start goes through the API so the launch + // carries the shared secret and is recorded for adoption. + const api = { async setInactivityTimeout(durationMs: number) { calls.push(`timeout ${durationMs}`); }, - start() { + async start() { calls.push("start"); + return { runtimeId: "runtime", clientSecret: "secret", outcome: "launched" as const }; }, - getTcpPort() { + port() { return { async fetch() { healthAttempts += 1; @@ -61,17 +68,18 @@ describe("createWorkspaceWarmPoolRuntime", () => { "There is no container instance that can be provided to this Durable Object, try again later", ); } - container.running = true; return new Response(null, { status: 200 }); }, } as unknown as Fetcher; }, }; - await startWorkspaceContainerAndWait(container, { PORT: "8080" }, 120_000, { - attempts: 3, - wait: async () => {}, - }); + await startWorkspaceContainerAndWait( + api, + { env: { PORT: "8080" }, enableInternet: true }, + 120_000, + { attempts: 3, wait: async () => {} }, + ); expect(calls).toEqual([ "timeout 120000", diff --git a/examples/think-compare-runtimes/worker/computer-container-pool.ts b/examples/think-compare-runtimes/worker/computer-container-pool.ts index dbee7364..5ba3111d 100644 --- a/examples/think-compare-runtimes/worker/computer-container-pool.ts +++ b/examples/think-compare-runtimes/worker/computer-container-pool.ts @@ -1,5 +1,6 @@ import { DurableObject } from "cloudflare:workers"; import { + type ContainerLaunchSpec, type IWorkspaceContainerAPI, withWorkspaceContainer, } from "@cloudflare/computer/backends/container"; @@ -17,7 +18,7 @@ export interface WorkspacePoolEnv extends ContainerPoolConfigEnv { export interface WorkspaceContainerHostHandle { getWorkspaceContainer(): IWorkspaceContainerAPI | Promise; - startWarmContainer(env: Record, inactivityTimeoutMs: number): Promise; + startWarmContainer(spec: ContainerLaunchSpec, inactivityTimeoutMs: number): Promise; destroyWarmContainer(): Promise; isWarmContainerHealthy(): Promise; } @@ -35,11 +36,12 @@ class WorkspaceContainerHostBase extends withWorkspaceContainer( ) {} export class WorkspaceContainerHost extends WorkspaceContainerHostBase { - async startWarmContainer( - env: Record, - inactivityTimeoutMs: number, - ): Promise { - await startWorkspaceContainerAndWait(this.getContainer(), env, inactivityTimeoutMs); + async startWarmContainer(spec: ContainerLaunchSpec, inactivityTimeoutMs: number): Promise { + // Through the workspace API rather than ctx.container, so the launch + // carries whatever the API adds — today the shared secret the + // daemon's HTTP surface requires — and is recorded, so the workspace + // that adopts this container can tell it matches. + await startWorkspaceContainerAndWait(this.getWorkspaceContainer(), spec, inactivityTimeoutMs); } async destroyWarmContainer(): Promise { @@ -69,7 +71,7 @@ export function createWorkspaceWarmPoolRuntime(env: WorkspacePoolEnv): WarmPoolR async startContainer(containerId) { const host = getWorkspaceContainerHost(env, containerId); try { - await host.startWarmContainer(workspaceContainerEnv(env), containerSleepAfterMs(env)); + await host.startWarmContainer(workspaceLaunchSpec(env), containerSleepAfterMs(env)); } catch (error) { console.warn({ message: "Workspace warm container failed to start", @@ -103,19 +105,28 @@ function getWorkspaceContainerHost( ) as unknown as WorkspaceContainerHostHandle; } -function workspaceContainerEnv(env: WorkspacePoolEnv): Record { +function workspaceLaunchSpec(env: WorkspacePoolEnv): ContainerLaunchSpec { return { - PORT: String(WORKSPACE_PORT), - MOUNT_POINT: "/workspace", - ...(env.FUSE_MOUNT ? { FUSE_MOUNT: env.FUSE_MOUNT } : {}), + env: { + PORT: String(WORKSPACE_PORT), + MOUNT_POINT: "/workspace", + ...(env.FUSE_MOUNT ? { FUSE_MOUNT: env.FUSE_MOUNT } : {}), + }, + // A pool cannot know the egress policy of the workspace that will + // adopt a container, so this has to agree with it by configuration. + // Disagreeing costs a relaunch on adoption, not the policy: the + // adopting workspace compares this spec against its own and + // replaces the container rather than inheriting the wrong one. + enableInternet: true, }; } -interface WorkspaceContainerControl { - readonly running: boolean; +// The subset of the workspace container API a warm start needs. +// Structurally satisfied by IWorkspaceContainerAPI. +interface WorkspaceWarmStartAPI { setInactivityTimeout(durationMs: number): Promise; - start(options: { enableInternet: boolean; env: Record }): void; - getTcpPort(port: number): Fetcher; + start(spec: ContainerLaunchSpec): Promise; + port(port: number): Fetcher; } interface WorkspaceStartWaitOptions { @@ -124,22 +135,23 @@ interface WorkspaceStartWaitOptions { } export async function startWorkspaceContainerAndWait( - container: WorkspaceContainerControl, - env: Record, + api: WorkspaceWarmStartAPI, + spec: ContainerLaunchSpec, inactivityTimeoutMs: number, options: WorkspaceStartWaitOptions = {}, ): Promise { const attempts = options.attempts ?? 120; const wait = options.wait ?? ((durationMs) => scheduler.wait(durationMs)); - await container.setInactivityTimeout(inactivityTimeoutMs); + await api.setInactivityTimeout(inactivityTimeoutMs); let lastError: unknown; for (let attempt = 0; attempt < attempts; attempt += 1) { - if (!container.running) { - container.start({ enableInternet: true, env }); - } + // Called unconditionally: start() adopts a container already running + // with this spec, so there is no need to check `running` first the + // way a raw ctx.container.start() did. + await api.start(spec); try { - await waitForWorkspaceHealth(() => container.getTcpPort(WORKSPACE_PORT), 1); + await waitForWorkspaceHealth(() => api.port(WORKSPACE_PORT), 1); return; } catch (error) { lastError = error; diff --git a/packages/computer/src/backends/container/index.ts b/packages/computer/src/backends/container/index.ts index 2aa20d7a..5ad9453f 100644 --- a/packages/computer/src/backends/container/index.ts +++ b/packages/computer/src/backends/container/index.ts @@ -18,8 +18,10 @@ export { type CloudflareContainerBackendOptions, } from "./cloudflare-container.js"; export { + type ContainerRuntimeInfo, type IWorkspaceContainerAPI, WorkspaceContainerAPI, type WorkspaceRef, withWorkspaceContainer, } from "./container-host.js"; +export type { ContainerLaunchSpec } from "./container-launch-record.js"; From de9fceb33ef6294c129f0497ecb12d7a35de9e02 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:13:42 +0000 Subject: [PATCH 07/10] docs: Record the container launch spec and its adoption rule The boot sequence described start() as a call straight to the Cloudflare Containers API. It goes through WorkspaceContainerAPI, which adds the shared secret and records what the container was launched with, so a container found already running is adopted only when it matches and relaunched when it does not. Changeset entry for the interface change, since a consumer that pre-starts containers has to move to the new shape. --- .changeset/container-launch-spec.md | 5 +++++ docs/07_injected_service.md | 21 ++++++++++++++------- 2 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 .changeset/container-launch-spec.md diff --git a/.changeset/container-launch-spec.md b/.changeset/container-launch-spec.md new file mode 100644 index 00000000..7cc4b90c --- /dev/null +++ b/.changeset/container-launch-spec.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/computer": minor +--- + +`IWorkspaceContainerAPI.start()` and `restart()` now take a single `ContainerLaunchSpec` of `{ env, enableInternet }` instead of two arguments, and return which of `launched`, `adopted` or `relaunched` happened. Each launch records its spec, and a container found already running is relaunched unless it matches, because neither the environment nor the internet flag can be changed on a live container. A container started outside this API has no record and is relaunched rather than trusted. `setInactivityTimeout()` joins the interface so a caller that pre-starts containers, such as a warm pool, does not need to reach past it. diff --git a/docs/07_injected_service.md b/docs/07_injected_service.md index 91d167c0..a980eec1 100644 --- a/docs/07_injected_service.md +++ b/docs/07_injected_service.md @@ -121,13 +121,20 @@ Provider-agnostic shape — three steps, in order: `CloudflareContainerBackend` (`packages/computer/src/backends/container/cloudflare-container.ts`) wires it like this: -1. **Start.** `container.start({ enableInternet, env })` on the - Cloudflare Containers API — not the `@cloudflare/sandbox` SDK. - Idempotence comes from `container.running` plus a cached `#handle`; - there is no process-name registry, no `startProcess`/`getProcess`, - and no `node /app/...` command (the container's `ENTRYPOINT` runs - `computerd` directly). `containerEnv` pins `PORT=8080` and lets the - image's own `FUSE_MOUNT` value (typically `auto`) win. +1. **Start.** `WorkspaceContainerAPI.start({ env, enableInternet })`, + which reaches the Cloudflare Containers API — not the + `@cloudflare/sandbox` SDK. There is no process-name registry, no + `startProcess`/`getProcess`, and no `node /app/...` command (the + container's `ENTRYPOINT` runs `computerd` directly). `containerEnv` + pins `PORT=8080` and lets the image's own `FUSE_MOUNT` value + (typically `auto`) win, and the API adds `RPC_CLIENT_SECRET`. + + Neither the environment nor the internet flag can be changed on a + running container, so the launch records both and a container found + already running is only adopted when it matches. Otherwise it is + relaunched, which is what keeps a warm pool from handing a workspace + a container configured for something else. A container started + outside this API has no record and is relaunched too. 2. **Wire egress.** `container.interceptOutboundHttp(egressHost, egress)` routes outbound HTTP from the container at `egressHost` back to a Worker `Fetcher` the DO controls. From 7e5a15e6b836c5e8880da03ecc5b90956e03b4ec Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:05:59 +0000 Subject: [PATCH 08/10] computer: Require the container to enforce its secret before connecting Injecting the secret at launch cannot prove the running image honors it. A container built before the daemon understood RPC_CLIENT_SECRET ignores the variable and serves every route, and that is invisible from the host: the bearer token goes out and the container is content either way. After readiness, the backend now makes one unauthenticated request and refuses to connect unless it comes back 401. A container that answers it is not authorizing anyone, and handing it a session while believing otherwise is worse than failing: recycling a container or image that predates the secret is the cost of the upgrade. The probe target is /api rather than a diagnostic route, so the check does not depend on which of those a given build exposes. An enforcing daemon answers 401 before it looks at the route; one that is not answers whatever that route says for an unauthenticated GET. The request is bounded by healthProbeTimeoutMs, clamped to what is left of the connect deadline, matching every other request this file makes to the container. Unbounded, a container that accepted the connection and then stopped serving would hang the connect past its own timeout, and a merely slow one would consume the budget the upgrade wait needs and surface as an upgrade that never arrived. A probe that cannot complete is still allowed through. A timeout is not evidence that the container is unauthenticated, and the readiness loop is what decides whether it is alive. --- .changeset/authorize-container-http.md | 2 +- .../container/cloudflare-container.test.ts | 71 +++++++++++++++++++ .../container/cloudflare-container.ts | 49 +++++++++++++ 3 files changed, 121 insertions(+), 1 deletion(-) diff --git a/.changeset/authorize-container-http.md b/.changeset/authorize-container-http.md index 5a30d69c..764ec934 100644 --- a/.changeset/authorize-container-http.md +++ b/.changeset/authorize-container-http.md @@ -2,4 +2,4 @@ "@cloudflare/computer": minor --- -The container's HTTP surface now requires a bearer token. The host generates a secret, passes it to the container as RPC_CLIENT_SECRET at launch, and sends it on /connect. Readiness at /health stays open, and leaving the variable unset disables the checks. +The container's HTTP surface now requires a bearer token. The host generates a secret, passes it to the container as RPC_CLIENT_SECRET at launch, and sends it on /connect. Readiness at /health stays open, and leaving the variable unset disables the checks. Before opening a session the host checks that the container refuses an unauthenticated request and fails the connect if it does not, so a container or image predating this has to be recycled. diff --git a/packages/computer/src/backends/container/cloudflare-container.test.ts b/packages/computer/src/backends/container/cloudflare-container.test.ts index 5b0bfcfb..a53bc590 100644 --- a/packages/computer/src/backends/container/cloudflare-container.test.ts +++ b/packages/computer/src/backends/container/cloudflare-container.test.ts @@ -16,6 +16,11 @@ import { CloudflareContainerBackend } from "./cloudflare-container.js"; import type { IWorkspaceContainerAPI, WorkspaceRef } from "./container-host.js"; interface FakeHostOptions { + // Status the container returns for an unauthenticated request to a + // gated route. Defaults to 401, meaning the secret is enforced. + authProbeStatus?: number; + // Accept the probe and never answer it, to prove the request is bounded. + authProbeHang?: boolean; healthy?: boolean; // Health probe sequence: each connect() reads from the head of // this array. true = answer 200, false = throw "connection @@ -110,6 +115,18 @@ function makeFakeHost(opts: FakeHostOptions = {}): FakeHost { if (!nextHealthy()) throw new Error("connection refused"); return new Response(null, { status: 200 }); } + if (url.pathname === "/api") { + // An enforcing container refuses an unauthenticated request here. + // authProbeStatus models one that does not; authProbeHang models + // one that accepts the connection and never answers. + if (opts.authProbeHang) { + await new Promise((resolve) => { + init?.signal?.addEventListener("abort", resolve, { once: true }); + }); + throw new Error("aborted"); + } + return new Response(null, { status: opts.authProbeStatus ?? 401 }); + } if (url.pathname === "/connect") { state.connectBody = (await request .clone() @@ -427,6 +444,60 @@ describe("CloudflareContainerBackend", () => { expect(fake.connectAuthorization).toBe(`Bearer ${fake.clientSecret}`); }); + test("connect() fails when the container accepts an unauthenticated request", async () => { + // An image predating RPC_CLIENT_SECRET ignores the variable and serves + // everything. Connecting anyway would hand a session to a container + // that is not authorizing anyone, so this refuses instead. + const fake = makeFakeHost({ authProbeStatus: 200 }); + const backend = new CloudflareContainerBackend({ + container: () => ({ getWorkspaceContainer: () => fake.host }), + workspace: fakeWorkspace, + connectTimeoutMs: 600, + }); + + const error = await backend.connect().catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(WorkspaceTransportError); + expect(String(error)).toMatch(/stage=auth/); + expect(String(error)).toMatch(/without authorization/); + expect(String(error)).toMatch(/recycled/); + }); + + test("connect() gets past the check when the container refuses an unauthenticated request", async () => { + const fake = makeFakeHost(); + const backend = new CloudflareContainerBackend({ + container: () => ({ getWorkspaceContainer: () => fake.host }), + workspace: fakeWorkspace, + connectTimeoutMs: 600, + }); + + const error = await backend.connect().catch((caught: unknown) => caught); + + // It still fails, but on the upgrade rather than the auth check: there + // is no WebSocketPair in this environment. + expect(String(error)).not.toMatch(/stage=auth/); + expect(fake.calls.map((c) => c.name)).toContain("fetchPort"); + }); + + test("connect() does not hang when the container never answers the check", async () => { + // The probe is on the critical path, so an unbounded request would + // wedge the connect past its own timeout. + const fake = makeFakeHost({ authProbeHang: true }); + const backend = new CloudflareContainerBackend({ + container: () => ({ getWorkspaceContainer: () => fake.host }), + workspace: fakeWorkspace, + connectTimeoutMs: 600, + healthProbeTimeoutMs: 100, + }); + + const started = Date.now(); + await backend.connect().catch(() => {}); + + // A probe that cannot answer is inconclusive, so the connect carries + // on and fails later on the upgrade, well inside its own budget. + expect(Date.now() - started).toBeLessThan(5_000); + }); + test("connect() throws a transport error when the /api upgrade never arrives", async () => { const fake = makeFakeHost(); const backend = new CloudflareContainerBackend({ diff --git a/packages/computer/src/backends/container/cloudflare-container.ts b/packages/computer/src/backends/container/cloudflare-container.ts index 2299e57e..7a54c6c6 100644 --- a/packages/computer/src/backends/container/cloudflare-container.ts +++ b/packages/computer/src/backends/container/cloudflare-container.ts @@ -261,6 +261,7 @@ export class CloudflareContainerBackend implements WorkspaceBackend { this.#armUpgrade(); runtimeId = await this.#readyWithRestarts(host, env, deadline, priorExit, runtimeId); + await this.#requireAuthEnforced(host, deadline); await this.#postConnect(host, deadline, clientSecret); const ws = await this.#waitForUpgrade(deadline); @@ -529,6 +530,54 @@ export class CloudflareContainerBackend implements WorkspaceBackend { ); } + // Confirm the container refuses an unauthorized request before handing + // it a session. Launching with the secret is arranged elsewhere, but an + // image built before the daemon understood RPC_CLIENT_SECRET ignores it + // and serves everything, and that is invisible from the host: the + // bearer token goes out and the container is content either way. + // + // /api is the probe target because it exists on every daemon that has a + // capnweb endpoint at all, so this check does not depend on which + // diagnostic routes a given build happens to expose. An enforcing + // daemon answers 401 before it looks at the route; one that is not + // enforcing answers whatever the route says for an unauthenticated + // GET. + // + // A definite answer other than 401 fails the connect. Recycling a + // container that predates the secret is the cost of the upgrade, and it + // is preferable to a workspace that believes it is authorized and is + // not. A probe that cannot complete is not evidence either way and is + // allowed through: the readiness loop above is what decides whether the + // container is alive. + async #requireAuthEnforced(host: IWorkspaceContainerAPI, deadline: number): Promise { + let status: number; + try { + const res = await host.fetchPort(this.#options.containerPort, "http://container/api", { + // Bounded like every other request this file makes to the + // container. Unbounded, a container that accepts the connection + // and then stops serving would hang the connect, and a slow one + // would eat the budget #waitForUpgrade needs, surfacing as an + // upgrade that never arrived. + signal: AbortSignal.timeout( + Math.min(this.#options.healthProbeTimeoutMs, Math.max(50, deadline - Date.now())), + ), + }); + status = res.status; + // Release the body; nothing here reads it. + await res.text().catch(() => ""); + } catch { + return; + } + if (status === 401) return; + this.#rejectUpgrade?.(new Error("container is not enforcing RPC_CLIENT_SECRET")); + this.#clearUpgrade(); + throw new WorkspaceTransportError( + `CloudflareContainerBackend(${this.id}) [stage=auth]: container served an unauthenticated ` + + `request to /api with ${status}, so this workspace would run without authorization. ` + + `A container or image predating RPC_CLIENT_SECRET has to be recycled.`, + ); + } + async #postConnect( host: IWorkspaceContainerAPI, deadline: number, From d996bb2bba742f013e1707d9c6910d490ea0ef2f Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Tue, 18 Aug 2026 16:38:32 +0000 Subject: [PATCH 09/10] computerd: Accept the bearer scheme in any case The check compared the literal prefix "Bearer ", so a client sending "bearer " was refused with a 401 that looks like a wrong token. The HTTP grammar makes the scheme token case-insensitive and allows more than one space before the credentials, so all of those spellings are valid requests. The scheme is now compared case-insensitively and the credentials are taken from whatever follows the first space, trimmed. Only the credentials are compared byte for byte, in constant time, as before. The host in this repository always sends "Bearer", so this only affected clients written against the documented header. --- packages/computerd/src/cli/computerd.test.ts | 13 +++++++++++++ packages/computerd/src/cli/computerd.ts | 11 ++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/computerd/src/cli/computerd.test.ts b/packages/computerd/src/cli/computerd.test.ts index 18edd338..871e2a8a 100644 --- a/packages/computerd/src/cli/computerd.test.ts +++ b/packages/computerd/src/cli/computerd.test.ts @@ -746,6 +746,19 @@ test("RPC_CLIENT_SECRET gates the HTTP surface but not /health", async (_ctx) => }); expect(short.statusCode).toBe(401); + // The scheme token is case-insensitive per the HTTP grammar, and the + // credentials may be preceded by more than one space. + for (const header of [ + `Bearer ${secret}`, + `bearer ${secret}`, + `BEARER ${secret}`, + `BeArEr ${secret}`, + `Bearer ${secret}`, + ]) { + const res = await request(`${base}/__computerd/info`, { headers: { authorization: header } }); + expect(res.statusCode, JSON.stringify(header)).toBe(200); + } + // Another scheme is not a bearer token. const basic = await request(`${base}/__computerd/info`, { headers: { authorization: `Basic ${secret}` }, diff --git a/packages/computerd/src/cli/computerd.ts b/packages/computerd/src/cli/computerd.ts index 995fa6eb..fbe6576e 100644 --- a/packages/computerd/src/cli/computerd.ts +++ b/packages/computerd/src/cli/computerd.ts @@ -113,9 +113,14 @@ function isAuthorized(request: IncomingMessage, secret: string | undefined): boo if (secret === undefined) return true; const header = request.headers.authorization; if (typeof header !== "string") return false; - const scheme = "Bearer "; - if (!header.startsWith(scheme)) return false; - const presented = Buffer.from(header.slice(scheme.length)); + // The scheme token is case-insensitive and may be followed by more + // than one space, so "bearer " from a hand-written client is + // as valid as "Bearer ". Only the credentials that follow are + // compared byte for byte. + const separator = header.indexOf(" "); + if (separator === -1) return false; + if (header.slice(0, separator).toLowerCase() !== "bearer") return false; + const presented = Buffer.from(header.slice(separator + 1).trim()); const expected = Buffer.from(secret); // timingSafeEqual throws on a length mismatch, and the length of the // secret is not worth leaking through the comparison either. From bd8a96d5f0319a962579754355eb793820c46828 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Tue, 18 Aug 2026 16:47:56 +0000 Subject: [PATCH 10/10] computer, computerd: Authorize the dial-back as well The container had to present a bearer token to reach the daemon, but the reverse direction was open. connect() arms a slot for the container's outbound upgrade and hands its session to whatever arrives first, and that endpoint is reachable from inside the container through the egress interceptor. Any command the workspace runs could win that race. Winning it does not hand the caller the durable object's authority, since the durable object is the client of that session and exposes nothing. It does something worse in one respect: the impostor becomes the container this workspace trusts, so pushOnce ships the workspace's file contents to it, pullOnce takes its entries into the authoritative store, and exec sends it the agent's commands and believes its output. The daemon now presents the secret it was launched with on the outbound dial, and handleFetch requires it before accepting the socket. Both ends already hold that secret: the host generates it and sets it at launch. The comparison on the host side walks every byte rather than stopping at the first difference. crypto.subtle.timingSafeEqual would be the primitive to reach for, but the SubtleCrypto this package compiles against does not declare it. An absent expected secret refuses everything. handleFetch only runs after connect() has recorded what it launched the container with, so an absent one means the upgrade arrived without that having happened. This gap predates the branch; the surrounding change is what makes the in-container party the party being defended against. --- .changeset/authorize-container-http.md | 2 +- .../container/cloudflare-container.test.ts | 67 +++++++++++++++++ .../container/cloudflare-container.ts | 44 +++++++++++ packages/computerd/src/cli/computerd.test.ts | 73 ++++++++++++++++++- packages/computerd/src/cli/computerd.ts | 12 ++- 5 files changed, 194 insertions(+), 4 deletions(-) diff --git a/.changeset/authorize-container-http.md b/.changeset/authorize-container-http.md index 764ec934..8899fceb 100644 --- a/.changeset/authorize-container-http.md +++ b/.changeset/authorize-container-http.md @@ -2,4 +2,4 @@ "@cloudflare/computer": minor --- -The container's HTTP surface now requires a bearer token. The host generates a secret, passes it to the container as RPC_CLIENT_SECRET at launch, and sends it on /connect. Readiness at /health stays open, and leaving the variable unset disables the checks. Before opening a session the host checks that the container refuses an unauthenticated request and fails the connect if it does not, so a container or image predating this has to be recycled. +The container's HTTP surface now requires a bearer token. The host generates a secret, passes it to the container as RPC_CLIENT_SECRET at launch, and sends it on /connect. Readiness at /health stays open, and leaving the variable unset disables the checks. Before opening a session the host checks that the container refuses an unauthenticated request and fails the connect if it does not, so a container or image predating this has to be recycled. The container's dial-back to the host carries the same secret, and the host refuses an upgrade that does not present it. diff --git a/packages/computer/src/backends/container/cloudflare-container.test.ts b/packages/computer/src/backends/container/cloudflare-container.test.ts index a53bc590..f34a0135 100644 --- a/packages/computer/src/backends/container/cloudflare-container.test.ts +++ b/packages/computer/src/backends/container/cloudflare-container.test.ts @@ -531,6 +531,73 @@ describe("CloudflareContainerBackend", () => { expect(res.status).toBe(400); }); + test("handleFetch refuses a dial-back that does not present the secret", async () => { + // The armed slot hands its session to whoever arrives first, and this + // endpoint is reachable from inside the container. Without a token, + // any command the workspace runs could take the daemon's place. + const fake = makeFakeHost(); + const backend = new CloudflareContainerBackend({ + container: () => ({ getWorkspaceContainer: () => fake.host }), + workspace: fakeWorkspace, + connectTimeoutMs: 600, + }); + // connect() records the secret and arms the slot; it then fails on the + // upgrade, which is what leaves the slot open for these requests. + await backend.connect().catch(() => {}); + const upgrade = { upgrade: "websocket" }; + + const none = await backend.handleFetch( + new Request("http://computer.internal/api", { headers: upgrade }), + ); + expect(none.status).toBe(401); + expect(none.headers.get("www-authenticate")).toBe("Bearer"); + + const wrong = await backend.handleFetch( + new Request("http://computer.internal/api", { + headers: { ...upgrade, authorization: `Bearer ${"f".repeat(32)}` }, + }), + ); + expect(wrong.status).toBe(401); + + const shortToken = await backend.handleFetch( + new Request("http://computer.internal/api", { + headers: { ...upgrade, authorization: "Bearer short" }, + }), + ); + expect(shortToken.status).toBe(401); + + const otherScheme = await backend.handleFetch( + new Request("http://computer.internal/api", { + headers: { ...upgrade, authorization: `Basic ${fake.clientSecret}` }, + }), + ); + expect(otherScheme.status).toBe(401); + }); + + test("handleFetch accepts a dial-back presenting the secret, in any scheme case", async () => { + const fake = makeFakeHost(); + const backend = new CloudflareContainerBackend({ + container: () => ({ getWorkspaceContainer: () => fake.host }), + workspace: fakeWorkspace, + connectTimeoutMs: 600, + }); + await backend.connect().catch(() => {}); + + // WebSocketPair does not exist outside workerd, so getting past the + // check is what is observable here: it fails constructing the pair + // rather than answering 401. + for (const scheme of ["Bearer", "bearer", "BEARER"]) { + const res = await backend + .handleFetch( + new Request("http://computer.internal/api", { + headers: { upgrade: "websocket", authorization: `${scheme} ${fake.clientSecret}` }, + }), + ) + .catch(() => null); + expect(res?.status, scheme).not.toBe(401); + } + }); + test("connect() consults host.exitInfo() before host.start()", async () => { const fake = makeFakeHost(); const backend = new CloudflareContainerBackend({ diff --git a/packages/computer/src/backends/container/cloudflare-container.ts b/packages/computer/src/backends/container/cloudflare-container.ts index 7a54c6c6..fc7445b6 100644 --- a/packages/computer/src/backends/container/cloudflare-container.ts +++ b/packages/computer/src/backends/container/cloudflare-container.ts @@ -151,6 +151,33 @@ const DEFAULT_HEALTH_PROBE_TIMEOUT_MS = 2_000; const DEFAULT_HEALTH_RETRY_INITIAL_DELAY_MS = 250; const DEFAULT_HEALTH_RETRY_MAX_DELAY_MS = 2_000; +// Bearer check for the dial-back. The scheme token is case-insensitive +// and may be followed by more than one space, matching what the daemon +// accepts on its own surface. +// +// The comparison walks every byte rather than stopping at the first +// difference, so it does not leak how much of the secret was correct. +// crypto.subtle.timingSafeEqual would be the primitive to reach for, but +// the SubtleCrypto this package compiles against does not declare it. +// +// An absent expected secret refuses everything rather than allowing it. +// This only runs once connect() has recorded the secret it launched the +// container with, so an absent one means the upgrade arrived without that +// having happened. +function bearerMatches(header: string | null, expected: string | undefined): boolean { + if (expected === undefined || header === null) return false; + const separator = header.indexOf(" "); + if (separator === -1) return false; + if (header.slice(0, separator).toLowerCase() !== "bearer") return false; + const presented = header.slice(separator + 1).trim(); + if (presented.length !== expected.length) return false; + let differences = 0; + for (let i = 0; i < expected.length; i += 1) { + differences |= presented.charCodeAt(i) ^ expected.charCodeAt(i); + } + return differences === 0; +} + export class CloudflareContainerBackend implements WorkspaceBackend { readonly type = "cloudflare-container"; readonly id: string; @@ -164,6 +191,9 @@ export class CloudflareContainerBackend implements WorkspaceBackend { Pick; readonly #egress: WorkspaceEgressPolicy; readonly #egressToken: string | undefined; + // Set once start() reports it, before the upgrade slot is armed, so + // handleFetch can check the dial-back against it. + #clientSecret: string | undefined; // State for the in-flight /api upgrade. handleFetch() resolves // #pendingUpgrade; connect() awaits it. @@ -255,6 +285,8 @@ export class CloudflareContainerBackend implements WorkspaceBackend { ); } + this.#clientSecret = clientSecret; + // Arm the upgrade promise before posting /connect — computerd // dials back as soon as /health on the egress answers, so // the upgrade can arrive before the POST resolves. @@ -376,6 +408,18 @@ export class CloudflareContainerBackend implements WorkspaceBackend { if (req.headers.get("upgrade") !== "websocket") { return new Response(`${EGRESS_API_PATH} requires a websocket upgrade`, { status: 400 }); } + // The slot armed by connect() hands its session to whoever arrives + // first, and this endpoint is reachable from inside the container + // through the egress interceptor. Without a token, any command the + // workspace runs could take the daemon's place and become the peer + // this durable object pushes its files to and takes its exec output + // from. The daemon presents the secret it was launched with. + if (!bearerMatches(req.headers.get("authorization"), this.#clientSecret)) { + return new Response("unauthorized", { + status: 401, + headers: { "www-authenticate": "Bearer" }, + }); + } const pair = new WebSocketPair(); const [client, server] = [pair[0], pair[1]]; diff --git a/packages/computerd/src/cli/computerd.test.ts b/packages/computerd/src/cli/computerd.test.ts index 871e2a8a..cb5d2ce1 100644 --- a/packages/computerd/src/cli/computerd.test.ts +++ b/packages/computerd/src/cli/computerd.test.ts @@ -389,6 +389,7 @@ test("/connect re-dial tears down the prior WebSocket session", async (_ctx) => const { WebSocketServer } = await import("ws"); const peerPort = await getAvailablePort(); const opened = []; + const upgradeAuthorizations = []; const peerSockets = new Set(); // Deliberately non-default paths: the daemon must use what the // request names, not paths of its own. @@ -411,6 +412,9 @@ test("/connect re-dial tears down the prior WebSocket session", async (_ctx) => return; } wss.handleUpgrade(req, socket, head, (ws) => { + // The host arms a slot for this upgrade and hands the first arrival + // its session, so the dial-back has to prove it is this daemon. + upgradeAuthorizations.push(req.headers.authorization ?? null); const entry = { closed: false, closeCode: null }; ws.on("close", (code) => { entry.closed = true; @@ -457,6 +461,72 @@ test("/connect re-dial tears down the prior WebSocket session", async (_ctx) => await waitFor(() => opened[0].closed); expect(opened[0].closed).toBe(true, "first peer WS should be closed after re-POST /connect"); expect(opened[1].closed).toBe(false, "second peer WS should still be open"); + + // No secret is configured here, so there is nothing to present. + expect(upgradeAuthorizations).toEqual([null, null]); +}); + +test("/connect presents the shared secret on the dial-back", async (_ctx) => { + // The host arms a slot for this upgrade and gives the first arrival its + // session. Anything that can reach the host's endpoint — which includes + // every command this daemon runs — could take the daemon's place, so + // the dial has to carry the secret the daemon was launched with. + const { WebSocketServer } = await import("ws"); + const secret = "0123456789abcdef0123456789abcdef"; + const peerPort = await getAvailablePort(); + const seen = []; + const peerSockets = new Set(); + const peerServer = http.createServer((req, res) => { + if (req.url === "/probe") { + res.writeHead(200, { "content-type": "text/plain" }); + res.end("ok\n"); + return; + } + res.writeHead(404).end(); + }); + peerServer.on("connection", (sock) => { + peerSockets.add(sock); + sock.on("close", () => peerSockets.delete(sock)); + }); + const wss = new WebSocketServer({ noServer: true }); + peerServer.on("upgrade", (req, socket, head) => { + seen.push(req.headers.authorization ?? null); + wss.handleUpgrade(req, socket, head, () => {}); + }); + await new Promise((resolve) => peerServer.listen(peerPort, "127.0.0.1", resolve)); + onTestFinished( + () => + new Promise((resolve) => { + for (const sock of peerSockets) sock.destroy(); + wss.close(); + peerServer.close(() => resolve()); + }), + ); + + const port = await getAvailablePort(); + const mountPoint = await fs.mkdtemp(path.join(os.tmpdir(), "computerd-dialback-")); + await startComputerd({ + port, + mountPoint, + env: { FUSE_MOUNT: "none", RPC_CLIENT_SECRET: secret }, + }); + + const res = await postJson(`http://127.0.0.1:${port}/connect`, { + base: `http://127.0.0.1:${peerPort}`, + health: "/probe", + api: "/socket", + }); + expect(res.statusCode, "the /connect request itself needs the token too").toBe(401); + + const authorized = await postJson( + `http://127.0.0.1:${port}/connect`, + { base: `http://127.0.0.1:${peerPort}`, health: "/probe", api: "/socket" }, + { authorization: `Bearer ${secret}` }, + ); + expect(authorized.statusCode).toBe(200); + + await waitFor(() => seen.length === 1); + expect(seen[0]).toBe(`Bearer ${secret}`); }); test("/connect rejects a body that does not name every part", async (_ctx) => { @@ -660,7 +730,7 @@ async function waitFor(predicate, { timeoutMs = 2_000, intervalMs = 10 } = {}) { throw new Error("waitFor: predicate did not become true within the timeout"); } -function postJson(url, body) { +function postJson(url, body, headers = {}) { const payload = JSON.stringify(body); return new Promise((resolve, reject) => { const req = http.request( @@ -670,6 +740,7 @@ function postJson(url, body) { headers: { "content-type": "application/json", "content-length": Buffer.byteLength(payload), + ...headers, }, }, (response) => { diff --git a/packages/computerd/src/cli/computerd.ts b/packages/computerd/src/cli/computerd.ts index fbe6576e..5036db4b 100644 --- a/packages/computerd/src/cli/computerd.ts +++ b/packages/computerd/src/cli/computerd.ts @@ -237,7 +237,7 @@ function createHTTPServer( }); return; } - void handleConnect(request, response, rpc, upstreamSlot); + void handleConnect(request, response, rpc, upstreamSlot, secret); return; } @@ -445,6 +445,7 @@ async function handleConnect( response: ServerResponse, rpc: ReturnType, upstreamSlot: { ws: WebSocket | undefined }, + secret: string | undefined, ): Promise { let body: ConnectBody; try { @@ -508,7 +509,14 @@ async function handleConnect( } const wsUrl = `${toWebSocketUrl(baseUrl)}${body.api}`; - const ws = new WebSocket(wsUrl); + // Present the shared secret on the way out as well. The host arms a + // slot for this upgrade and hands the first arrival its session, so + // without a token anything that can reach the host's endpoint — which + // includes every command this daemon runs — could take the workspace's + // place. The host holds the same secret, having set it at launch. + const ws = new WebSocket(wsUrl, { + ...(secret !== undefined ? { headers: { authorization: `Bearer ${secret}` } } : {}), + }); upstreamSlot.ws = ws; ws.once("open", () => { console.log(`/connect: attached RPC session to ${wsUrl}`);