Skip to content

Commit 532eb17

Browse files
quickbeardclaude
andauthored
fix(sandbox): require a per-sandbox secret on the egress broker (#220)
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 <noreply@anthropic.com>
1 parent f4ba4f7 commit 532eb17

7 files changed

Lines changed: 54 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ file is the human-readable summary of what changed and why.
3939
only on macOS, where the VM backend can honour them: Windows sandboxes
4040
follow the VM-global limits in the user's `.wslconfig`, and the Linux
4141
backend has no per-sandbox limits.
42+
- **The sandbox egress broker now requires a per-sandbox secret** — each
43+
running sandbox gets its own random token, injected into the guest and
44+
required on every brokered request. Without it the broker on Windows (which
45+
must listen on TCP so the guest can reach the host) would have been usable
46+
by any other process able to reach that port, spending the real gateway
47+
credential the broker attaches on the host side.
4248
- **Named egress rules for sandboxes** — administrators can grant sandboxes
4349
extra brokered destinations beyond the model gateway by creating
4450
`sandbox-egress.json` in the app's data directory: an object of name →

‎packages/codev-sandbox/src/broker.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,31 @@ test("route headers replace the guest's placeholder credential", async () => {
140140
upstream.stop(true)
141141
})
142142

143+
test("rejects callers without the shared secret", async () => {
144+
const upstream = Bun.serve({ port: 0, fetch: () => Response.json({ ok: true }) })
145+
const broker = createBrokerServer({
146+
routes: [{ prefix: "/llm", upstream: `http://127.0.0.1:${upstream.port}/v1` }],
147+
auth: "secret-token",
148+
})
149+
const port = await broker.listen()
150+
151+
const anonymous = await fetch(`http://127.0.0.1:${port}/llm/models`)
152+
expect(anonymous.status).toBe(401)
153+
const wrong = await fetch(`http://127.0.0.1:${port}/llm/models`, { headers: { authorization: "Bearer nope" } })
154+
expect(wrong.status).toBe(401)
155+
// An unauthorized caller must not be able to probe which routes exist.
156+
const denied = await fetch(`http://127.0.0.1:${port}/not-a-route`)
157+
expect(denied.status).toBe(401)
158+
159+
const allowed = await fetch(`http://127.0.0.1:${port}/llm/models`, {
160+
headers: { authorization: "Bearer secret-token" },
161+
})
162+
expect(allowed.status).toBe(200)
163+
164+
await broker.close()
165+
upstream.stop(true)
166+
})
167+
143168
test("reports upstream failures as 502 instead of hanging", async () => {
144169
const broker = createBrokerServer({
145170
routes: [{ prefix: "/llm", upstream: "http://127.0.0.1:1/nowhere" }],

‎packages/codev-sandbox/src/broker.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ export type SandboxEgress = {
2222
routes: BrokerRoute[]
2323
guestConfig?: string
2424
fetchImpl?: typeof fetch
25+
// Shared secret the guest must present. Required in practice: the WSL
26+
// backend can only reach the host over TCP, so without it any local (or
27+
// LAN-reachable) process could use the broker as an open proxy to the
28+
// gateway, spending the real token the broker attaches host-side.
29+
auth?: string
2530
}
2631

2732
// Named egress rules: the admin-editable shape for extra routes beyond the
@@ -99,9 +104,16 @@ export function forwardableResponseHeaders(headers: Headers) {
99104
return Object.fromEntries([...headers.entries()].filter(([name]) => !RESPONSE_HEADER_DROP.has(name.toLowerCase())))
100105
}
101106

102-
export function createBrokerServer(options: { routes: BrokerRoute[]; fetchImpl?: typeof fetch }) {
107+
export function createBrokerServer(options: { routes: BrokerRoute[]; fetchImpl?: typeof fetch; auth?: string }) {
103108
const fetchImpl = options.fetchImpl ?? fetch
104109
const server = createServer(async (request, response) => {
110+
// Checked before the route table is consulted, so an unauthorized caller
111+
// learns nothing about which upstreams exist.
112+
if (options.auth && request.headers.authorization !== `Bearer ${options.auth}`) {
113+
response.writeHead(401, { "content-type": "application/json" })
114+
response.end(JSON.stringify({ error: "sandbox egress unauthorized" }))
115+
return
116+
}
105117
const resolved = resolveBrokerRoute(options.routes, request.url ?? "/")
106118
if (!resolved) {
107119
response.writeHead(403, { "content-type": "application/json" })

‎packages/codev-sandbox/src/bwrap/provider.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export function createBwrapProvider(options: BwrapProviderOptions): SandboxProvi
119119
// No routes means no egress at all — the config injection below can
120120
// still apply without a listener.
121121
const broker = egress?.routes.length
122-
? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl })
122+
? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl, auth: egress.auth })
123123
: null
124124
if (broker) await broker.listenUnix(join(dirs.runtime, "broker.sock"))
125125
if (egress?.guestConfig) await writeFile(join(dirs.bridge, "codev.json"), egress.guestConfig)

‎packages/codev-sandbox/src/vz/provider.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ export function createVzProvider(options: VzProviderOptions): SandboxProvider {
131131
// No routes means no egress at all — the config injection below can
132132
// still apply without a listener.
133133
const broker = egress?.routes.length
134-
? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl })
134+
? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl, auth: egress.auth })
135135
: null
136136
if (broker) await broker.listenUnix(join(dirs.runtime, "broker.sock"))
137137
if (egress?.guestConfig) await writeFile(join(dirs.guestConfig, "codev.json"), egress.guestConfig)

‎packages/codev-sandbox/src/wsl/provider.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ export function createWslProvider(options: WslProviderOptions): SandboxProvider
184184
// No routes means no egress at all — the config injection below can
185185
// still apply without a listener.
186186
const broker = egress?.routes.length
187-
? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl })
187+
? createBrokerServer({ routes: egress.routes, fetchImpl: egress.fetchImpl ?? fetchImpl, auth: egress.auth })
188188
: null
189189
// The guest dials the host at the NAT default gateway address, so the
190190
// broker cannot bind loopback here.

‎packages/desktop/src/main/sandbox/egress.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { randomUUID } from "node:crypto"
12
import { readFile } from "node:fs/promises"
23
import { join } from "node:path"
34
import { app, net } from "electron"
@@ -22,6 +23,10 @@ export async function resolveSandboxEgress(): Promise<SandboxEgress> {
2223
// back keeps the common case (hub login, no base_url) from silently
2324
// producing a sandbox with no egress.
2425
const baseUrl = creds?.baseUrl ?? loadGatewayUrl()
26+
// Per-start secret the guest presents on every brokered request. The WSL
27+
// backend can only reach the host over TCP, so without this the broker would
28+
// be an open proxy to the gateway for anything that can reach the port.
29+
const auth = randomUUID()
2530
const gateway =
2631
creds?.apiKey && baseUrl && creds.model ? { apiKey: creds.apiKey, baseUrl, model: creds.model } : null
2732
return {
@@ -40,10 +45,11 @@ export async function resolveSandboxEgress(): Promise<SandboxEgress> {
4045
: []),
4146
...(await namedEgressRules()),
4247
],
48+
auth,
4349
guestConfig: JSON.stringify(
4450
{
4551
...(gateway
46-
? buildGatewayConfig({ apiKey: "sandbox-broker", baseUrl: GUEST_BROKER_BASE, model: gateway.model })
52+
? buildGatewayConfig({ apiKey: auth, baseUrl: GUEST_BROKER_BASE, model: gateway.model })
4753
: {}),
4854
// Everything the agent does is confined to the VM and the one mounted
4955
// project folder, so permission prompts buy nothing here — auto-allow

0 commit comments

Comments
 (0)