Skip to content

Commit b3db5dc

Browse files
committed
fix(security): strip browser credentials at sandbox proxy and redact git logs
The sandbox preview proxy forwarded every incoming request header into untrusted container code, leaking first-party Cookie/Authorization credentials across the platform-to-container trust boundary. Replace the blanket header spread with a strict allowlist on both the HTTP and WebSocket paths, preserving only safe content/negotiation headers (and the WebSocket handshake + port-target headers) while dropping Cookie, Authorization, X-CSRF-Token, X-Session-*, and X-Api-Key. Also remove the git protocol handler's debug log that dumped all request headers and leaked the first 20 chars of the Authorization bearer token.
1 parent 453a4b0 commit b3db5dc

3 files changed

Lines changed: 136 additions & 13 deletions

File tree

worker/api/handlers/git-protocol.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,6 @@ async function verifyGitAccess(
106106
): Promise<{ hasAccess: boolean; appCreatedAt?: Date }> {
107107
logger.info('Verifying git access', { appId });
108108

109-
// Log all headers for debugging
110-
const headers: Record<string, string> = {};
111-
request.headers.forEach((value, key) => {
112-
headers[key] = key.toLowerCase().includes('auth') ? `${value.substring(0, 20)}...` : value;
113-
});
114-
logger.info('Request headers', { headers, url: request.url });
115-
116109
const appService = new AppService(env);
117110
const app = await appService.getAppDetails(appId);
118111

worker/services/sandbox/request-handler.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,71 @@ describe('proxyToSandbox', () => {
6565
expect(containerFetch).not.toHaveBeenCalled();
6666
});
6767

68+
it('strips Cookie/Authorization but forwards safe headers to the container', async () => {
69+
validatePortToken.mockResolvedValue(true);
70+
containerFetch.mockResolvedValue(new Response('ok'));
71+
72+
await proxyToSandbox(
73+
req(`8001-mysandbox-${TOKEN}.preview.example.dev`, {
74+
headers: {
75+
Cookie: 'session=secret',
76+
Authorization: 'Bearer secret',
77+
'X-Csrf-Token': 'secret',
78+
'X-Api-Key': 'secret',
79+
Accept: 'text/html',
80+
'Content-Type': 'application/json',
81+
},
82+
}),
83+
env,
84+
);
85+
86+
const proxied = containerFetch.mock.calls[0][0] as Request;
87+
expect(proxied.headers.get('Cookie')).toBeNull();
88+
expect(proxied.headers.get('Authorization')).toBeNull();
89+
expect(proxied.headers.get('X-Csrf-Token')).toBeNull();
90+
expect(proxied.headers.get('X-Api-Key')).toBeNull();
91+
expect(proxied.headers.get('Accept')).toBe('text/html');
92+
expect(proxied.headers.get('Content-Type')).toBe('application/json');
93+
});
94+
95+
it('adds proxy X-* headers on the container request', async () => {
96+
validatePortToken.mockResolvedValue(true);
97+
containerFetch.mockResolvedValue(new Response('ok'));
98+
99+
await proxyToSandbox(req(`8001-mysandbox-${TOKEN}.preview.example.dev`), env);
100+
101+
const proxied = containerFetch.mock.calls[0][0] as Request;
102+
expect(proxied.headers.get('X-Sandbox-Name')).toBe('mysandbox');
103+
expect(proxied.headers.get('X-Forwarded-Host')).toBe(
104+
`8001-mysandbox-${TOKEN}.preview.example.dev`,
105+
);
106+
expect(proxied.headers.get('X-Forwarded-Proto')).toBe('https');
107+
});
108+
109+
it('forwards WebSocket handshake headers but strips Cookie on a valid upgrade', async () => {
110+
validatePortToken.mockResolvedValue(true);
111+
sandboxFetch.mockResolvedValue(new Response('ok'));
112+
113+
await proxyToSandbox(
114+
req(`8001-mysandbox-${TOKEN}.preview.example.dev`, {
115+
headers: {
116+
Upgrade: 'websocket',
117+
Connection: 'Upgrade',
118+
'Sec-WebSocket-Version': '13',
119+
Cookie: 'session=secret',
120+
Authorization: 'Bearer secret',
121+
},
122+
}),
123+
env,
124+
);
125+
126+
const forwarded = sandboxFetch.mock.calls[0][0] as Request;
127+
expect(forwarded.headers.get('Upgrade')).toBe('websocket');
128+
expect(forwarded.headers.get('Sec-WebSocket-Version')).toBe('13');
129+
expect(forwarded.headers.get('Cookie')).toBeNull();
130+
expect(forwarded.headers.get('Authorization')).toBeNull();
131+
});
132+
68133
it('rejects a WebSocket upgrade with a bad token (no sandbox.fetch)', async () => {
69134
validatePortToken.mockResolvedValue(false);
70135

worker/services/sandbox/request-handler.ts

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,66 @@ export interface RouteInfo {
3030
*/
3131
const CONTROL_PLANE_PORTS = new Set<number>([3000, 8787]);
3232

33+
/**
34+
* Strict allowlist of request headers forwarded from the browser into the
35+
* (untrusted) container. The container runs LLM-generated code, so the proxy
36+
* must never pass first-party platform credentials across this trust boundary.
37+
* Anything not listed here is dropped — in particular Cookie, Authorization,
38+
* X-CSRF-Token, X-Session-*, and X-Api-Key.
39+
*/
40+
const FORWARDED_REQUEST_HEADERS = new Set<string>([
41+
'accept',
42+
'accept-language',
43+
'accept-encoding',
44+
'content-type',
45+
'content-length',
46+
'user-agent',
47+
'range',
48+
'if-none-match',
49+
'if-modified-since',
50+
'cache-control',
51+
'pragma',
52+
'referer',
53+
'origin',
54+
]);
55+
56+
/**
57+
* Additional headers required to complete a WebSocket handshake. These carry no
58+
* credentials and must survive the allowlist so upgrades still work.
59+
* `cf-container-target-port` is what `switchPort` uses to route to the port.
60+
*/
61+
const WEBSOCKET_HANDSHAKE_HEADERS = new Set<string>([
62+
'upgrade',
63+
'connection',
64+
'sec-websocket-key',
65+
'sec-websocket-version',
66+
'sec-websocket-protocol',
67+
'sec-websocket-extensions',
68+
'cf-container-target-port',
69+
]);
70+
71+
/**
72+
* Build the outbound header set from the incoming request using the strict
73+
* allowlist, then layer on the proxy-added headers in `extra`.
74+
*/
75+
function buildProxyHeaders(
76+
request: Request,
77+
extra: Record<string, string>,
78+
allowExtra?: ReadonlySet<string>,
79+
): Headers {
80+
const headers = new Headers();
81+
request.headers.forEach((value, name) => {
82+
const lower = name.toLowerCase();
83+
if (FORWARDED_REQUEST_HEADERS.has(lower) || allowExtra?.has(lower)) {
84+
headers.set(name, value);
85+
}
86+
});
87+
for (const [name, value] of Object.entries(extra)) {
88+
headers.set(name, value);
89+
}
90+
return headers;
91+
}
92+
3393
export async function proxyToSandbox<E extends SandboxEnv>(
3494
request: Request,
3595
env: E
@@ -69,22 +129,27 @@ export async function proxyToSandbox<E extends SandboxEnv>(
69129
if (upgradeHeader?.toLowerCase() === 'websocket') {
70130
logger.info('[Proxy] WebSocket upgrade request', { sandboxId, port, path });
71131
// WebSocket path: Must use fetch() not containerFetch()
72-
// This bypasses JSRPC serialization boundary which cannot handle WebSocket upgrades
73-
return await sandbox.fetch(switchPort(request, port));
132+
// This bypasses JSRPC serialization boundary which cannot handle WebSocket upgrades.
133+
// Strip credentials while preserving the handshake + port-target headers.
134+
const wsRequest = switchPort(request, port);
135+
return await sandbox.fetch(
136+
new Request(wsRequest, {
137+
headers: buildProxyHeaders(wsRequest, {}, WEBSOCKET_HANDSHAKE_HEADERS),
138+
}),
139+
);
74140
}
75141

76142
// Route directly to user's service on the specified port
77143
const proxyUrl = `http://localhost:${port}${path}${url.search}`;
78144

79145
const proxyRequest = new Request(proxyUrl, {
80146
method: request.method,
81-
headers: {
82-
...Object.fromEntries(request.headers),
147+
headers: buildProxyHeaders(request, {
83148
'X-Original-URL': request.url,
84149
'X-Forwarded-Host': url.hostname,
85150
'X-Forwarded-Proto': url.protocol.replace(':', ''),
86-
'X-Sandbox-Name': sandboxId // Pass the friendly name
87-
},
151+
'X-Sandbox-Name': sandboxId, // Pass the friendly name
152+
}),
88153
body: request.body,
89154
// @ts-expect-error - duplex required for body streaming in modern runtimes
90155
duplex: 'half',

0 commit comments

Comments
 (0)