From 9eb6a7717d62b20ac314daec741aeccd75bac297 Mon Sep 17 00:00:00 2001 From: minhn4 Date: Thu, 13 Aug 2026 14:13:46 +0700 Subject: [PATCH] fix(sandbox): require a per-sandbox secret on the egress broker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found reviewing the sandbox surface: the WSL backend must bind the broker on TCP for the guest to reach the host across NAT, and the broker had no authentication. Anything else able to reach that port — any local process, and whatever the firewall lets onto the LAN — could use it as an open proxy to the model gateway, spending the real token the broker attaches host-side. The unix-socket backends were narrower but equally unauthenticated. createBrokerServer now takes an optional shared secret, checked before the route table is consulted so an unauthorized caller cannot even probe which upstreams exist, and the desktop generates a fresh one per sandbox start and injects it as the guest provider's apiKey. Verified against a running sandbox: an unauthenticated request gets {"error":"sandbox egress unauthorized"}, while the guest's injected token is accepted and forwarded upstream with the real credential swapped in. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 6 +++++ packages/codev-sandbox/src/broker.test.ts | 25 ++++++++++++++++++++ packages/codev-sandbox/src/broker.ts | 14 ++++++++++- packages/codev-sandbox/src/bwrap/provider.ts | 2 +- packages/codev-sandbox/src/vz/provider.ts | 2 +- packages/codev-sandbox/src/wsl/provider.ts | 2 +- packages/desktop/src/main/sandbox/egress.ts | 8 ++++++- 7 files changed, 54 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ecc8bb0b1..b0a7c4d1de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,12 @@ file is the human-readable summary of what changed and why. only on macOS, where the VM backend can honour them: Windows sandboxes follow the VM-global limits in the user's `.wslconfig`, and the Linux backend has no per-sandbox limits. +- **The sandbox egress broker now requires a per-sandbox secret** — each + running sandbox gets its own random token, injected into the guest and + required on every brokered request. Without it the broker on Windows (which + must listen on TCP so the guest can reach the host) would have been usable + by any other process able to reach that port, spending the real gateway + credential the broker attaches on the host side. - **Named egress rules for sandboxes** — administrators can grant sandboxes extra brokered destinations beyond the model gateway by creating `sandbox-egress.json` in the app's data directory: an object of name → diff --git a/packages/codev-sandbox/src/broker.test.ts b/packages/codev-sandbox/src/broker.test.ts index c246953a18..27d1dc7746 100644 --- a/packages/codev-sandbox/src/broker.test.ts +++ b/packages/codev-sandbox/src/broker.test.ts @@ -140,6 +140,31 @@ test("route headers replace the guest's placeholder credential", async () => { upstream.stop(true) }) +test("rejects callers without the shared secret", async () => { + const upstream = Bun.serve({ port: 0, fetch: () => Response.json({ ok: true }) }) + const broker = createBrokerServer({ + routes: [{ prefix: "/llm", upstream: `http://127.0.0.1:${upstream.port}/v1` }], + auth: "secret-token", + }) + const port = await broker.listen() + + const anonymous = await fetch(`http://127.0.0.1:${port}/llm/models`) + expect(anonymous.status).toBe(401) + const wrong = await fetch(`http://127.0.0.1:${port}/llm/models`, { headers: { authorization: "Bearer nope" } }) + expect(wrong.status).toBe(401) + // An unauthorized caller must not be able to probe which routes exist. + const denied = await fetch(`http://127.0.0.1:${port}/not-a-route`) + expect(denied.status).toBe(401) + + const allowed = await fetch(`http://127.0.0.1:${port}/llm/models`, { + headers: { authorization: "Bearer secret-token" }, + }) + expect(allowed.status).toBe(200) + + await broker.close() + upstream.stop(true) +}) + test("reports upstream failures as 502 instead of hanging", async () => { const broker = createBrokerServer({ routes: [{ prefix: "/llm", upstream: "http://127.0.0.1:1/nowhere" }], diff --git a/packages/codev-sandbox/src/broker.ts b/packages/codev-sandbox/src/broker.ts index 1f75c1d94f..5d7c628aaa 100644 --- a/packages/codev-sandbox/src/broker.ts +++ b/packages/codev-sandbox/src/broker.ts @@ -22,6 +22,11 @@ export type SandboxEgress = { routes: BrokerRoute[] guestConfig?: string fetchImpl?: typeof fetch + // Shared secret the guest must present. Required in practice: the WSL + // backend can only reach the host over TCP, so without it any local (or + // LAN-reachable) process could use the broker as an open proxy to the + // gateway, spending the real token the broker attaches host-side. + auth?: string } // Named egress rules: the admin-editable shape for extra routes beyond the @@ -99,9 +104,16 @@ export function forwardableResponseHeaders(headers: Headers) { return Object.fromEntries([...headers.entries()].filter(([name]) => !RESPONSE_HEADER_DROP.has(name.toLowerCase()))) } -export function createBrokerServer(options: { routes: BrokerRoute[]; fetchImpl?: typeof fetch }) { +export function createBrokerServer(options: { routes: BrokerRoute[]; fetchImpl?: typeof fetch; auth?: string }) { const fetchImpl = options.fetchImpl ?? fetch const server = createServer(async (request, response) => { + // Checked before the route table is consulted, so an unauthorized caller + // learns nothing about which upstreams exist. + if (options.auth && request.headers.authorization !== `Bearer ${options.auth}`) { + response.writeHead(401, { "content-type": "application/json" }) + response.end(JSON.stringify({ error: "sandbox egress unauthorized" })) + return + } const resolved = resolveBrokerRoute(options.routes, request.url ?? "/") if (!resolved) { response.writeHead(403, { "content-type": "application/json" }) diff --git a/packages/codev-sandbox/src/bwrap/provider.ts b/packages/codev-sandbox/src/bwrap/provider.ts index 0dcec97c1a..9b063b48b7 100644 --- a/packages/codev-sandbox/src/bwrap/provider.ts +++ b/packages/codev-sandbox/src/bwrap/provider.ts @@ -119,7 +119,7 @@ export function createBwrapProvider(options: BwrapProviderOptions): SandboxProvi // No routes means no egress at all — the config injection below can // still apply without a listener. const broker = egress?.routes.length - ? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl }) + ? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl, auth: egress.auth }) : null if (broker) await broker.listenUnix(join(dirs.runtime, "broker.sock")) if (egress?.guestConfig) await writeFile(join(dirs.bridge, "codev.json"), egress.guestConfig) diff --git a/packages/codev-sandbox/src/vz/provider.ts b/packages/codev-sandbox/src/vz/provider.ts index 26687aef41..a432940477 100644 --- a/packages/codev-sandbox/src/vz/provider.ts +++ b/packages/codev-sandbox/src/vz/provider.ts @@ -131,7 +131,7 @@ export function createVzProvider(options: VzProviderOptions): SandboxProvider { // No routes means no egress at all — the config injection below can // still apply without a listener. const broker = egress?.routes.length - ? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl }) + ? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl, auth: egress.auth }) : null if (broker) await broker.listenUnix(join(dirs.runtime, "broker.sock")) if (egress?.guestConfig) await writeFile(join(dirs.guestConfig, "codev.json"), egress.guestConfig) diff --git a/packages/codev-sandbox/src/wsl/provider.ts b/packages/codev-sandbox/src/wsl/provider.ts index 8c64f38105..e9914e760d 100644 --- a/packages/codev-sandbox/src/wsl/provider.ts +++ b/packages/codev-sandbox/src/wsl/provider.ts @@ -184,7 +184,7 @@ export function createWslProvider(options: WslProviderOptions): SandboxProvider // No routes means no egress at all — the config injection below can // still apply without a listener. const broker = egress?.routes.length - ? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl }) + ? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl, auth: egress.auth }) : null // The guest dials the host at the NAT default gateway address, so the // broker cannot bind loopback here. diff --git a/packages/desktop/src/main/sandbox/egress.ts b/packages/desktop/src/main/sandbox/egress.ts index ad7829a791..231f142332 100644 --- a/packages/desktop/src/main/sandbox/egress.ts +++ b/packages/desktop/src/main/sandbox/egress.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto" import { readFile } from "node:fs/promises" import { join } from "node:path" import { app, net } from "electron" @@ -22,6 +23,10 @@ export async function resolveSandboxEgress(): Promise { // back keeps the common case (hub login, no base_url) from silently // producing a sandbox with no egress. const baseUrl = creds?.baseUrl ?? loadGatewayUrl() + // Per-start secret the guest presents on every brokered request. The WSL + // backend can only reach the host over TCP, so without this the broker would + // be an open proxy to the gateway for anything that can reach the port. + const auth = randomUUID() const gateway = creds?.apiKey && baseUrl && creds.model ? { apiKey: creds.apiKey, baseUrl, model: creds.model } : null return { @@ -40,10 +45,11 @@ export async function resolveSandboxEgress(): Promise { : []), ...(await namedEgressRules()), ], + auth, guestConfig: JSON.stringify( { ...(gateway - ? buildGatewayConfig({ apiKey: "sandbox-broker", baseUrl: GUEST_BROKER_BASE, model: gateway.model }) + ? buildGatewayConfig({ apiKey: auth, baseUrl: GUEST_BROKER_BASE, model: gateway.model }) : {}), // Everything the agent does is confined to the VM and the one mounted // project folder, so permission prompts buy nothing here — auto-allow