Skip to content

Commit 3dcf561

Browse files
committed
Fix array and non-plain object globals in sandbox
extractMethods was recursing into every object, so arrays, Map, Set, Date, RegExp, and TypedArrays passed as globals were rebuilt as plain objects — losing their type. Only recurse into plain objects and let structured clone carry the rest through the port unchanged. Also fix injectMethods in the worker, which overwrote the constants subtree with a fresh {} whenever the same path had both a constant and a method sibling. Merge into the existing subtree instead. Dispose the sandbox when setGlobals rejects so unserializable globals (e.g. Symbol) don't leak an iframe. Add tests covering every primitive and object type as a global.
1 parent 97a5bf9 commit 3dcf561

3 files changed

Lines changed: 275 additions & 11 deletions

File tree

src/host.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ export type HostService = Service<{
99
onMethod(params: { methodId: number; params: unknown[] }): unknown;
1010
}>;
1111

12+
function isPlainObject(value: object): boolean {
13+
const proto = Object.getPrototypeOf(value);
14+
return proto === Object.prototype || proto === null;
15+
}
16+
1217
/**
1318
* Options for creating a sandboxed execution environment.
1419
*/
@@ -142,13 +147,17 @@ export async function createSandbox(opts?: CreateSandboxOptions): Promise<Sandbo
142147
methodsById.push(value);
143148
break;
144149
case "object": {
145-
if (value != null) {
150+
if (value === null) {
151+
constants[key] = null;
152+
} else if (isPlainObject(value)) {
146153
const child = extractMethods(value as Record<string, unknown>);
147154
constants[key] = child.constants;
148155
if (Object.keys(child.methods).length > 0) {
149156
methods[key] = child.methods;
150157
}
151158
} else {
159+
// Arrays, Date, Map, Set, RegExp, TypedArrays, etc. are passed
160+
// through; structured clone preserves their type across the port.
152161
constants[key] = value;
153162
}
154163
break;
@@ -206,9 +215,6 @@ export async function createSandbox(opts?: CreateSandboxOptions): Promise<Sandbo
206215
},
207216
});
208217

209-
const guestClient = createMessagePortClient<GuestService>(port);
210-
await guestClient.call("setGlobals", { constants, methods });
211-
212218
let disposeReject: (err: Error) => void;
213219
const disposePromise = new Promise<never>((_, reject) => {
214220
disposeReject = reject;
@@ -221,6 +227,14 @@ export async function createSandbox(opts?: CreateSandboxOptions): Promise<Sandbo
221227
iframe.remove();
222228
};
223229

230+
const guestClient = createMessagePortClient<GuestService>(port);
231+
try {
232+
await guestClient.call("setGlobals", { constants, methods });
233+
} catch (err) {
234+
dispose();
235+
throw err;
236+
}
237+
224238
function callImpl<T>(
225239
callPromise: Promise<T>,
226240
execOpts: ExecutionOptions | undefined,

src/worker.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,13 @@ createWorkerServer<GuestService>(self, {
4949
for (const [key, value] of Object.entries(methods)) {
5050
if (typeof value === "object") {
5151
if (value) {
52-
const child = {};
53-
dest[key] = child;
52+
// Merge into the existing constants subtree so sibling constants
53+
// at the same path aren't overwritten.
54+
let child = dest[key] as Record<string, unknown> | undefined;
55+
if (typeof child !== "object" || child === null) {
56+
child = {};
57+
dest[key] = child;
58+
}
5459
injectMethods(value as Record<string, unknown>, child);
5560
}
5661
} else if (typeof value === "number") {

tests/basics.test.ts

Lines changed: 250 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,257 @@ describe("globals", () => {
206206
expect(await expression(sandbox, "1 + 1")).toBe(2);
207207
});
208208

209-
test("skips unserializable globals", async () => {
210-
// Hmmm, not sure if this is correct
211-
sandbox = await createSandbox({
212-
globals: { url: new URL("https://test.invalid") },
209+
test("rejects unserializable globals", async () => {
210+
await expect(createSandbox({ globals: { sym: Symbol("nope") } })).rejects.toThrow();
211+
});
212+
});
213+
214+
describe("global value types", () => {
215+
describe("primitives", () => {
216+
test("number (including Infinity, NaN, -0)", async () => {
217+
sandbox = await createSandbox({
218+
globals: { int: 42, float: 3.14, neg: -1, inf: Infinity, negInf: -Infinity, nan: NaN },
219+
});
220+
expect(await expression(sandbox, "int")).toBe(42);
221+
expect(await expression(sandbox, "float")).toBe(3.14);
222+
expect(await expression(sandbox, "neg")).toBe(-1);
223+
expect(await expression(sandbox, "inf")).toBe(Infinity);
224+
expect(await expression(sandbox, "negInf")).toBe(-Infinity);
225+
expect(await expression(sandbox, "Number.isNaN(nan)")).toBe(true);
226+
});
227+
228+
test("string", async () => {
229+
sandbox = await createSandbox({
230+
globals: { empty: "", hello: "hello", emoji: "🍕", unicode: "café" },
231+
});
232+
expect(await expression(sandbox, "empty")).toBe("");
233+
expect(await expression(sandbox, "hello")).toBe("hello");
234+
expect(await expression(sandbox, "emoji")).toBe("🍕");
235+
expect(await expression(sandbox, "unicode")).toBe("café");
236+
});
237+
238+
test("boolean", async () => {
239+
sandbox = await createSandbox({ globals: { t: true, f: false } });
240+
expect(await expression(sandbox, "t")).toBe(true);
241+
expect(await expression(sandbox, "f")).toBe(false);
242+
});
243+
244+
test("null", async () => {
245+
sandbox = await createSandbox({ globals: { n: null } });
246+
expect(await expression(sandbox, "n")).toBe(null);
247+
});
248+
249+
test("undefined", async () => {
250+
sandbox = await createSandbox({ globals: { u: undefined } });
251+
expect(await expression(sandbox, "typeof u")).toBe("undefined");
252+
expect(await expression(sandbox, "u")).toBeUndefined();
253+
});
254+
255+
test("bigint", async () => {
256+
sandbox = await createSandbox({
257+
globals: { big: 123n, bigNeg: -999999999999999999n },
258+
});
259+
expect(await expression(sandbox, "big")).toEqual(123n);
260+
expect(await expression(sandbox, "bigNeg")).toEqual(-999999999999999999n);
261+
});
262+
});
263+
264+
describe("objects", () => {
265+
test("plain object", async () => {
266+
sandbox = await createSandbox({
267+
globals: { obj: { a: 1, b: "two", c: true, d: null } },
268+
});
269+
expect(await expression(sandbox, "obj")).toEqual({ a: 1, b: "two", c: true, d: null });
270+
});
271+
272+
test("empty object", async () => {
273+
sandbox = await createSandbox({ globals: { obj: {} } });
274+
expect(await expression(sandbox, "obj")).toEqual({});
275+
});
276+
277+
test("array of primitives", async () => {
278+
sandbox = await createSandbox({ globals: { arr: [1, 2, 3] } });
279+
expect(await expression(sandbox, "Array.isArray(arr)")).toBe(true);
280+
expect(await expression(sandbox, "arr")).toEqual([1, 2, 3]);
281+
expect(await expression(sandbox, "arr.length")).toBe(3);
282+
});
283+
284+
test("empty array", async () => {
285+
sandbox = await createSandbox({ globals: { arr: [] } });
286+
expect(await expression(sandbox, "Array.isArray(arr)")).toBe(true);
287+
expect(await expression(sandbox, "arr")).toEqual([]);
288+
});
289+
290+
test("array of mixed primitives", async () => {
291+
sandbox = await createSandbox({
292+
globals: { arr: [1, "two", true, null] },
293+
});
294+
expect(await expression(sandbox, "arr")).toEqual([1, "two", true, null]);
295+
});
296+
297+
test("array of objects", async () => {
298+
sandbox = await createSandbox({
299+
globals: { arr: [{ a: 1 }, { b: 2 }] },
300+
});
301+
expect(await expression(sandbox, "arr")).toEqual([{ a: 1 }, { b: 2 }]);
302+
});
303+
304+
test("nested arrays", async () => {
305+
sandbox = await createSandbox({
306+
globals: {
307+
arr: [
308+
[1, 2],
309+
[3, 4],
310+
],
311+
},
312+
});
313+
expect(await expression(sandbox, "arr")).toEqual([
314+
[1, 2],
315+
[3, 4],
316+
]);
317+
expect(await expression(sandbox, "Array.isArray(arr[0])")).toBe(true);
318+
});
319+
320+
test("object containing arrays", async () => {
321+
sandbox = await createSandbox({
322+
globals: { obj: { fruits: ["apple", "banana"], ints: [1, 2, 3] } },
323+
});
324+
expect(await expression(sandbox, "Array.isArray(obj.fruits)")).toBe(true);
325+
expect(await expression(sandbox, "obj.fruits")).toEqual(["apple", "banana"]);
326+
expect(await expression(sandbox, "obj.ints")).toEqual([1, 2, 3]);
327+
});
328+
329+
test("README mutable array example", async () => {
330+
sandbox = await createSandbox({
331+
globals: { fruit: ["apple", "banana"] },
332+
});
333+
await sandbox.run('fruit.push("cherry")');
334+
expect(await sandbox.evaluate("fruit")).toEqual(["apple", "banana", "cherry"]);
335+
});
336+
337+
test("Date", async () => {
338+
const date = new Date("2024-01-01T00:00:00Z");
339+
sandbox = await createSandbox({ globals: { d: date } });
340+
expect(await expression(sandbox, "d instanceof Date")).toBe(true);
341+
expect(await expression(sandbox, "d.getTime()")).toBe(date.getTime());
342+
expect(await expression(sandbox, "d.toISOString()")).toBe(date.toISOString());
343+
});
344+
345+
test("RegExp", async () => {
346+
sandbox = await createSandbox({ globals: { re: /foo/i } });
347+
expect(await expression(sandbox, "re instanceof RegExp")).toBe(true);
348+
expect(await expression(sandbox, "re.source")).toBe("foo");
349+
expect(await expression(sandbox, "re.flags")).toBe("i");
350+
expect(await expression(sandbox, 're.test("FOO")')).toBe(true);
351+
expect(await expression(sandbox, 're.test("bar")')).toBe(false);
352+
});
353+
354+
test("Map", async () => {
355+
sandbox = await createSandbox({
356+
globals: {
357+
m: new Map<string, unknown>([
358+
["a", 1],
359+
["b", "two"],
360+
]),
361+
},
362+
});
363+
expect(await expression(sandbox, "m instanceof Map")).toBe(true);
364+
expect(await expression(sandbox, "m.size")).toBe(2);
365+
expect(await expression(sandbox, 'm.get("a")')).toBe(1);
366+
expect(await expression(sandbox, 'm.get("b")')).toBe("two");
367+
});
368+
369+
test("Set", async () => {
370+
sandbox = await createSandbox({
371+
globals: { s: new Set([1, 2, 3]) },
372+
});
373+
expect(await expression(sandbox, "s instanceof Set")).toBe(true);
374+
expect(await expression(sandbox, "s.size")).toBe(3);
375+
expect(await expression(sandbox, "s.has(2)")).toBe(true);
376+
expect(await expression(sandbox, "s.has(99)")).toBe(false);
377+
});
378+
379+
test("ArrayBuffer", async () => {
380+
const buf = new Uint8Array([1, 2, 3, 4]).buffer;
381+
sandbox = await createSandbox({ globals: { buf } });
382+
expect(await expression(sandbox, "buf instanceof ArrayBuffer")).toBe(true);
383+
expect(await expression(sandbox, "buf.byteLength")).toBe(4);
384+
expect(await expression(sandbox, "new Uint8Array(buf)[2]")).toBe(3);
385+
});
386+
387+
test("Uint8Array", async () => {
388+
sandbox = await createSandbox({
389+
globals: { arr: new Uint8Array([10, 20, 30]) },
390+
});
391+
expect(await expression(sandbox, "arr instanceof Uint8Array")).toBe(true);
392+
expect(await expression(sandbox, "arr.length")).toBe(3);
393+
expect(await expression(sandbox, "arr[1]")).toBe(20);
394+
});
395+
396+
test("Int32Array", async () => {
397+
sandbox = await createSandbox({
398+
globals: { arr: new Int32Array([-1, 0, 1]) },
399+
});
400+
expect(await expression(sandbox, "arr instanceof Int32Array")).toBe(true);
401+
expect(await expression(sandbox, "arr[0]")).toBe(-1);
402+
expect(await expression(sandbox, "arr.length")).toBe(3);
403+
});
404+
});
405+
406+
describe("mixed", () => {
407+
test("plain object containing an array and a function", async () => {
408+
sandbox = await createSandbox({
409+
globals: {
410+
mod: {
411+
items: ["a", "b", "c"],
412+
getItem: (i: number) => ["a", "b", "c"][i],
413+
},
414+
},
415+
});
416+
expect(await expression(sandbox, "Array.isArray(mod.items)")).toBe(true);
417+
expect(await expression(sandbox, "mod.items")).toEqual(["a", "b", "c"]);
418+
expect(await expression(sandbox, "await mod.getItem(1)")).toBe("b");
419+
});
420+
421+
test("plain object with every primitive type", async () => {
422+
sandbox = await createSandbox({
423+
globals: {
424+
all: {
425+
num: 1,
426+
str: "s",
427+
bool: true,
428+
nil: null,
429+
undef: undefined,
430+
big: 10n,
431+
},
432+
},
433+
});
434+
expect(await expression(sandbox, "all.num")).toBe(1);
435+
expect(await expression(sandbox, "all.str")).toBe("s");
436+
expect(await expression(sandbox, "all.bool")).toBe(true);
437+
expect(await expression(sandbox, "all.nil")).toBe(null);
438+
expect(await expression(sandbox, "typeof all.undef")).toBe("undefined");
439+
expect(await expression(sandbox, "all.big")).toEqual(10n);
440+
});
441+
442+
test("deeply nested mix of arrays, objects, and functions", async () => {
443+
sandbox = await createSandbox({
444+
globals: {
445+
root: {
446+
list: [1, 2, 3],
447+
child: {
448+
tags: ["x", "y"],
449+
count: () => 99,
450+
},
451+
},
452+
},
453+
});
454+
expect(await expression(sandbox, "root.list")).toEqual([1, 2, 3]);
455+
expect(await expression(sandbox, "Array.isArray(root.list)")).toBe(true);
456+
expect(await expression(sandbox, "root.child.tags")).toEqual(["x", "y"]);
457+
expect(await expression(sandbox, "Array.isArray(root.child.tags)")).toBe(true);
458+
expect(await expression(sandbox, "await root.child.count()")).toBe(99);
213459
});
214-
expect(await expression(sandbox, "url.href")).toBeUndefined();
215460
});
216461
});
217462

0 commit comments

Comments
 (0)