Skip to content

Commit 32fd506

Browse files
committed
Harden routing regression coverage
1 parent dd066e9 commit 32fd506

2 files changed

Lines changed: 64 additions & 16 deletions

File tree

‎src/lib/browser-routing.ts‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,14 @@ function buildRoutedInit(
296296
headers: Headers,
297297
): RequestInit {
298298
const method = request.method.toUpperCase();
299+
const body = method === 'GET' || method === 'HEAD' ? undefined : requestBodyForFetch(request, originalInit);
300+
if (derivesOwnContentType(body) && !new Headers(originalInit?.headers).get('content-type')) {
301+
// `request` was constructed from the same body, so its content-type carries
302+
// that construction's multipart boundary. The routed fetch re-encodes the
303+
// body with a new boundary, so let it derive the header again.
304+
headers.delete('content-type');
305+
}
306+
299307
const routedInit = {
300308
...((originalInit ?? {}) as Record<string, unknown>),
301309
method,
@@ -307,17 +315,10 @@ function buildRoutedInit(
307315
delete routedInit['body'];
308316
delete routedInit['duplex'];
309317

318+
if (body !== undefined) {
319+
routedInit.body = body;
320+
}
310321
if (method !== 'GET' && method !== 'HEAD') {
311-
const body = requestBodyForFetch(request, originalInit);
312-
if (body !== undefined) {
313-
routedInit.body = body;
314-
if (derivesOwnContentType(body) && !new Headers(originalInit?.headers).get('content-type')) {
315-
// `request` was constructed from the same body, so its content-type carries
316-
// that construction's multipart boundary. The routed fetch re-encodes the
317-
// body with a new boundary, so let it derive the header again.
318-
headers.delete('content-type');
319-
}
320-
}
321322
if (originalInit?.duplex !== undefined) {
322323
routedInit.duplex = originalInit.duplex;
323324
} else if (requiresHalfDuplex(body)) {

‎tests/lib/browser-routing.test.ts‎

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
import Kernel, { toFile } from '@onkernel/sdk';
1+
import http from 'http';
2+
import { AddressInfo } from 'net';
3+
import { Readable } from 'stream';
4+
import Kernel, { APIConnectionError, toFile } from '@onkernel/sdk';
25

36
import {
47
BrowserRouteCache,
@@ -744,7 +747,7 @@ describe('browser routing', () => {
744747
headers: new Headers(request.headers),
745748
body: new Uint8Array(await request.arrayBuffer()),
746749
});
747-
return new Response(null, { status: 204 });
750+
return new Response(null, { status: 201 });
748751
},
749752
});
750753
kernel.browserRouteCache.set({
@@ -783,7 +786,7 @@ describe('browser routing', () => {
783786
headers: new Headers(request.headers),
784787
body: await request.text(),
785788
});
786-
return new Response(null, { status: 204 });
789+
return new Response(null, { status: 201 });
787790
},
788791
});
789792
kernel.browserRouteCache.set({
@@ -897,7 +900,7 @@ describe('browser routing', () => {
897900
if (request.url.includes('browser-session.test')) {
898901
return new Response('Invalid JWT', { status: 401, headers: { 'content-type': 'text/plain' } });
899902
}
900-
return new Response(null, { status: 204 });
903+
return new Response(null, { status: 201 });
901904
},
902905
});
903906
kernel.browserRouteCache.set({
@@ -935,7 +938,7 @@ describe('browser routing', () => {
935938
if (request.url.includes('browser-session.test')) {
936939
return new Response('Invalid JWT', { status: 403, headers: { 'content-type': 'text/plain' } });
937940
}
938-
return new Response(null, { status: 204 });
941+
return new Response(null, { status: 201 });
939942
},
940943
});
941944
kernel.browserRouteCache.set({
@@ -956,6 +959,50 @@ describe('browser routing', () => {
956959
});
957960
});
958961

962+
test('does not send a consumed stream to the control plane after a stale JWT', async () => {
963+
await withBrowserRoutingEnv(undefined, async () => {
964+
const paths: string[] = [];
965+
const server = http.createServer((request, response) => {
966+
paths.push(request.url ?? '');
967+
request.resume();
968+
request.on('end', () => {
969+
const direct = request.url?.startsWith('/browser/kernel/') ?? false;
970+
response.writeHead(direct ? 401 : 201, { 'content-type': 'text/plain' });
971+
response.end(direct ? 'Invalid JWT' : '');
972+
});
973+
});
974+
const baseURL = await new Promise<string>((resolve) => {
975+
server.listen(0, '127.0.0.1', () => {
976+
const address = server.address() as AddressInfo;
977+
resolve(`http://127.0.0.1:${address.port}`);
978+
});
979+
});
980+
981+
try {
982+
const kernel = new Kernel({ apiKey: 'k', baseURL, maxRetries: 0 });
983+
kernel.browserRouteCache.set({
984+
sessionId: 'sess-1',
985+
baseURL: `${baseURL}/browser/kernel`,
986+
jwt: 'token-abc',
987+
});
988+
989+
await expect(
990+
kernel.browsers.fs.writeFile('sess-1', Readable.from([Buffer.from('payload')]) as never, {
991+
path: '/tmp/x',
992+
}),
993+
).rejects.toBeInstanceOf(APIConnectionError);
994+
995+
expect(paths).toHaveLength(1);
996+
expect(paths[0]).toContain('/browser/kernel/fs/write_file');
997+
expect(kernel.browserRouteCache.get('sess-1')).toBeUndefined();
998+
} finally {
999+
const closed = new Promise<void>((resolve) => server.close(() => resolve()));
1000+
server.closeAllConnections();
1001+
await closed;
1002+
}
1003+
});
1004+
});
1005+
9591006
test('env override keeps fs and logs on the control plane', async () => {
9601007
await withBrowserRoutingEnv('computer', async () => {
9611008
const calls: string[] = [];
@@ -1039,7 +1086,7 @@ describe('browser routing', () => {
10391086
dest: form.get('files[0][dest_path]'),
10401087
file: await (form.get('files[0][file]') as File).text(),
10411088
});
1042-
return new Response(null, { status: 204 });
1089+
return new Response(null, { status: 201 });
10431090
},
10441091
});
10451092
kernel.browserRouteCache.set({

0 commit comments

Comments
 (0)