Skip to content

Commit 6e5c337

Browse files
authored
fix(sandbox): undefine the raw fetch host bridge before user code runs (#6761)
* fix(sandbox): undefine the raw fetch host bridge before user code runs * test(sandbox): scope the hardening assertions to each execution path * fix(sandbox): preserve the fetch global's property attributes
1 parent 8a44621 commit 6e5c337

2 files changed

Lines changed: 98 additions & 5 deletions

File tree

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Guards the isolate hardening contract in `isolated-vm-worker.cjs`: no raw
5+
* `ivm.Reference` host bridge may survive as an isolate global once user code
6+
* runs. Each bootstrap must capture its bridges in a closure and list their
7+
* global names in its own `undefined_globals`.
8+
*
9+
* The two execution paths harden independently, so every assertion is scoped to
10+
* one path's source slice — a bridge installed by `executeCode` but undefined
11+
* only by `executeTask` must still fail.
12+
*/
13+
import { readFileSync } from 'node:fs'
14+
import { join } from 'node:path'
15+
import { describe, expect, it } from 'vitest'
16+
17+
const WORKER_SOURCE = readFileSync(join(__dirname, 'isolated-vm-worker.cjs'), 'utf8')
18+
19+
const EXECUTE_CODE_MARKER = 'async function executeCode('
20+
const EXECUTE_TASK_MARKER = 'async function executeTask('
21+
22+
/**
23+
* Source of one execution path, from its function declaration to the start of
24+
* the next one (or end of file for the last path).
25+
*/
26+
function pathSource(marker: string, nextMarker?: string): string {
27+
const start = WORKER_SOURCE.indexOf(marker)
28+
expect(start, `${marker} not found — worker layout changed`).toBeGreaterThan(-1)
29+
const end = nextMarker ? WORKER_SOURCE.indexOf(nextMarker, start) : WORKER_SOURCE.length
30+
expect(end, `${nextMarker} not found — worker layout changed`).toBeGreaterThan(start)
31+
return WORKER_SOURCE.slice(start, end)
32+
}
33+
34+
const EXECUTION_PATHS = [
35+
{ name: 'executeCode', source: pathSource(EXECUTE_CODE_MARKER, EXECUTE_TASK_MARKER) },
36+
{ name: 'executeTask', source: pathSource(EXECUTE_TASK_MARKER) },
37+
]
38+
39+
/**
40+
* Globals bound to a raw `ivm.Reference`. The worker names these `__*Ref`;
41+
* `ivm.Callback` bridges (`__log`, `__textEncode`, …) are plain isolate
42+
* functions that expose no host handle and are deliberately not matched.
43+
*/
44+
function referenceBridges(source: string): string[] {
45+
return [...source.matchAll(/jail\.set\('(__\w+Ref)'/g)].map((match) => match[1])
46+
}
47+
48+
function hardeningList(source: string): string {
49+
const list = source.match(/const undefined_globals = \[[\s\S]*?\]/)
50+
expect(list, 'no undefined_globals hardening list in this execution path').not.toBeNull()
51+
return (list as RegExpMatchArray)[0]
52+
}
53+
54+
describe('isolated-vm worker hardening', () => {
55+
it.each(EXECUTION_PATHS)(
56+
'$name undefines every ivm.Reference bridge it installs',
57+
({ name, source }) => {
58+
const bridges = referenceBridges(source)
59+
expect(
60+
bridges.length,
61+
`${name} installs no __*Ref bridges — detection is stale`
62+
).toBeGreaterThan(0)
63+
64+
const list = hardeningList(source)
65+
for (const bridge of bridges) {
66+
expect(
67+
list.includes(`'${bridge}'`),
68+
`${name} sets ${bridge} as an isolate global but its hardening list omits it`
69+
).toBe(true)
70+
}
71+
}
72+
)
73+
74+
it.each(EXECUTION_PATHS)('$name undefines the isolated-vm escape globals', ({ source }) => {
75+
const list = hardeningList(source)
76+
for (const name of ['Isolate', 'Context', 'Script', 'Reference', 'ExternalCopy']) {
77+
expect(list).toContain(`'${name}'`)
78+
}
79+
})
80+
})

apps/sim/lib/execution/isolated-vm-worker.cjs

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,12 @@ async function executeCode(request, executionId) {
310310
info: (...args) => __log(...args),
311311
};
312312
313-
// Set up fetch function that uses the host's secure fetch
314-
async function fetch(url, options) {
313+
// Set up fetch function that uses the host's secure fetch. The raw
314+
// host bridge is captured in this closure so the hardening step below
315+
// can undefine the global without breaking fetch().
316+
(() => {
317+
const __fetch = globalThis.__fetchRef;
318+
const fetchImpl = async function fetch(url, options) {
315319
let optionsJson;
316320
if (options) {
317321
try {
@@ -323,7 +327,7 @@ async function executeCode(request, executionId) {
323327
throw new Error('fetch options exceed maximum payload size');
324328
}
325329
}
326-
const resultJson = await __fetchRef.apply(undefined, [url, optionsJson], { result: { promise: true } });
330+
const resultJson = await __fetch.apply(undefined, [url, optionsJson], { result: { promise: true } });
327331
let result;
328332
try {
329333
result = JSON.parse(resultJson);
@@ -355,7 +359,16 @@ async function executeCode(request, executionId) {
355359
blob: async () => { throw new Error('blob() not supported in sandbox'); },
356360
arrayBuffer: async () => { throw new Error('arrayBuffer() not supported in sandbox'); },
357361
};
358-
}
362+
};
363+
// Same property attributes a top-level \`function fetch\` declaration
364+
// produced, so user code sees an unchanged global.
365+
Object.defineProperty(global, 'fetch', {
366+
value: fetchImpl,
367+
writable: true,
368+
enumerable: true,
369+
configurable: false
370+
});
371+
})();
359372
360373
const sim = (() => {
361374
const broker = __brokerRef;
@@ -408,7 +421,7 @@ async function executeCode(request, executionId) {
408421
const undefined_globals = [
409422
'Isolate', 'Context', 'Script', 'Module', 'Callback', 'Reference',
410423
'ExternalCopy', 'process', 'require', 'module', 'exports', '__dirname', '__filename',
411-
'__brokerRef', '__broker', '__callSimBroker'
424+
'__fetchRef', '__brokerRef', '__broker', '__callSimBroker'
412425
];
413426
for (const name of undefined_globals) {
414427
try {

0 commit comments

Comments
 (0)