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