diff --git a/packages/codemode/CHANGELOG.md b/packages/codemode/CHANGELOG.md index 03e40bf8a..fee564838 100644 --- a/packages/codemode/CHANGELOG.md +++ b/packages/codemode/CHANGELOG.md @@ -2,6 +2,14 @@ ## [Unreleased] +### Changed + +- Built-ins are frozen and built-in globals are read-only before the script runs, so patches such as `Array.prototype.toJSON = ...` have no effect. Instances can still override `constructor`, `name`, `message`, `toString`, `toLocaleString`, `valueOf`, `toJSON`, and `Object.prototype` members ([#10444](https://github.com/earendil-works/pi/issues/10444)) + +### Fixed + +- Fixed a script that patched built-ins crashing the host process and leaving `execute()` unsettled. Malformed payloads from the worker now fail the execution with a `sandbox` error ([#10444](https://github.com/earendil-works/pi/issues/10444)) + ## [1.0.3] - 2026-10-05 ## [1.0.2] - 2026-10-04 diff --git a/packages/codemode/src/runtime/host.ts b/packages/codemode/src/runtime/host.ts index 940359b5f..21a935205 100644 --- a/packages/codemode/src/runtime/host.ts +++ b/packages/codemode/src/runtime/host.ts @@ -46,15 +46,54 @@ function serializeStore(store: Readonly> | undefined): R return serialized; } +/** + * The prelude that serializes values for the host runs in the same VM as the script, so a script + * that patches built-ins (for example `Array.prototype.toJSON`) can make it send malformed data. + * Payloads that do not decode throw this error, and the execution fails as a sandbox error. + */ +class BridgeError extends Error {} + +function parseBridgeJson(json: string, what: string): unknown { + try { + return JSON.parse(json); + } catch { + throw new BridgeError(`${what} is not valid JSON`); + } +} + function parseStoreWrites(json: string): CodemodeStoreWrites { + const entries = parseBridgeJson(json, "store writes"); + if (!Array.isArray(entries)) throw new BridgeError("store writes are not an array"); const writes: CodemodeStoreWrites = { set: {}, delete: [] }; - for (const [key, value] of JSON.parse(json) as [string, string?][]) { + for (const entry of entries) { + if ( + !Array.isArray(entry) || + typeof entry[0] !== "string" || + (entry.length === 2 ? typeof entry[1] !== "string" : entry.length !== 1) + ) { + throw new BridgeError("store writes contain a malformed entry"); + } + const [key, value] = entry as [string, string?]; if (value === undefined) writes.delete.push(key); - else writes.set[key] = JSON.parse(value); + else writes.set[key] = parseBridgeJson(value, `store value for ${JSON.stringify(key)}`); } return writes; } +function parseScriptError(json: string): CodemodeError { + const parsed = parseBridgeJson(json, "script error"); + if (typeof parsed !== "object" || parsed === null) throw new BridgeError("script error is not an object"); + const { name, message, stack } = parsed as Record; + if ( + typeof message !== "string" || + (name !== undefined && typeof name !== "string") || + (stack !== undefined && typeof stack !== "string") + ) { + throw new BridgeError("script error is malformed"); + } + return { kind: "script", name, message, stack }; +} + function defaultWorkerUrl(): URL { // `.ts` when running from source (tests, tsx), `.js` from the published dist. return new URL(import.meta.url.endsWith(".ts") ? "./worker.ts" : "./worker.js", import.meta.url); @@ -158,7 +197,20 @@ class Execution { return; } this.worker = worker; - worker.on("message", (message: unknown) => this.handleMessage(message)); + worker.on("message", (message: unknown) => { + // An exception here would be uncaught in the host process and leave the execution unsettled. + try { + this.handleMessage(message); + } catch (error) { + this.finish({ + kind: "sandbox", + message: + error instanceof BridgeError + ? `Sandbox bridge broken: ${error.message}. The script may have modified built-ins such as a prototype's toJSON.` + : `Sandbox host failed: ${errorMessage(error)}`, + }); + } + }); worker.on("error", (error: unknown) => { this.finish({ kind: "sandbox", @@ -181,12 +233,14 @@ class Execution { } private handleMessage(message: unknown): void { - if (this.finished || !isWorkerToHostMessage(message)) return; + if (this.finished) return; + if (!isWorkerToHostMessage(message)) throw new BridgeError("unknown message from the worker"); switch (message.type) { case "output": this.output.push(message.item); break; case "call": + if (this.pending.has(message.id)) throw new BridgeError(`duplicate call id ${message.id}`); void this.handleCall(message); break; case "done": @@ -199,12 +253,13 @@ class Execution { } private handleDone(message: Extract): void { + // Decode everything before finish(), which must not throw once it starts. if (!message.ok) { - const parsed = JSON.parse(message.error) as Omit; - this.finish({ kind: "script", ...parsed }); + this.finish(parseScriptError(message.error)); return; } - this.finish(undefined, message.value === undefined ? undefined : JSON.parse(message.value), message.writes); + const value = message.value === undefined ? undefined : parseBridgeJson(message.value, "return value"); + this.finish(undefined, value, parseStoreWrites(message.writes)); } private async handleCall(message: Extract): Promise { @@ -239,7 +294,7 @@ class Execution { this.post(reply); } - private finish(error: CodemodeError | undefined, value?: unknown, writes?: string): void { + private finish(error: CodemodeError | undefined, value?: unknown, writes?: CodemodeStoreWrites): void { if (this.finished) return; this.finished = true; clearTimeout(this.timer); @@ -259,7 +314,7 @@ class Execution { value, output: this.output, calls: this.calls, - storeWrites: writes === undefined ? { set: {}, delete: [] } : parseStoreWrites(writes), + storeWrites: writes ?? { set: {}, delete: [] }, }; if (!this.worker) { this.resolveResult(result); diff --git a/packages/codemode/src/runtime/prelude-source.ts b/packages/codemode/src/runtime/prelude-source.ts index 62eddec56..fa2bd0c46 100644 --- a/packages/codemode/src/runtime/prelude-source.ts +++ b/packages/codemode/src/runtime/prelude-source.ts @@ -24,6 +24,10 @@ * `globalsJson` lists `{ name, spread }`; `a.b` names are grouped into a frozen `a` object. * `bridge(kind, a, b, c)` with kind "call" or "global" (id, name, argsJson), * "output" ("text", text) or ("image", data, mimeType), or "done" (ok, valueJsonOrErrorJson, writesJson). + * + * Before anything else the prelude freezes the built-ins and makes the built-in globals read-only. + * The prelude shares them with the script, so a script that patched one (for example + * `Array.prototype.toJSON`) could otherwise corrupt what the prelude reports to the host. */ export const MAX_STORE_VALUE_CHARS = 256 * 1024; export const MAX_STORE_TOTAL_CHARS = 1024 * 1024; @@ -41,6 +45,96 @@ const IMAGE_HELPER_EXPECTS = export const PRELUDE_SOURCE: string = `(function (bridge, toolsJson, globalsJson, storeJson) { "use strict"; + + // Freeze every object reachable from the built-in globals, plus the intrinsics that are only + // reachable from instances (iterator and generator prototypes, %TypedArray%). + // + // Freezing alone breaks ordinary code through the "override mistake": a data property that is + // read-only on a prototype cannot be assigned on an instance either, so + // this.name = "MyError" in an Error subclass would throw. Commonly overridden properties + // become accessors whose setter defines an own property on the instance instead. + (function lockdown() { + const OVERRIDABLE = new Set(["constructor", "name", "message", "toString", "toLocaleString", "valueOf", "toJSON"]); + const seen = new Set([globalThis]); + const queue = []; + const add = (value) => { + if ((typeof value === "object" && value !== null) || typeof value === "function") { + if (!seen.has(value)) { + seen.add(value); + queue.push(value); + } + } + }; + + // Accessors from an object literal have no own prototype object, unlike function expressions, + // whose prototype.constructor would be converted again without end. + function allowOverride(object, key, value, enumerable) { + const { get, set } = Object.getOwnPropertyDescriptor( + { + get accessor() { + return value; + }, + set accessor(next) { + if (this === object) { + throw new TypeError("Cannot assign to read only property '" + String(key) + "' of a built-in"); + } + if ((typeof this !== "object" || this === null) && typeof this !== "function") return; + Object.defineProperty(this, key, { value: next, writable: true, enumerable: true, configurable: true }); + }, + }, + "accessor", + ); + Object.defineProperty(object, key, { get, set, enumerable, configurable: false }); + add(get); + add(set); + } + + for (const key of Reflect.ownKeys(globalThis)) { + const descriptor = Object.getOwnPropertyDescriptor(globalThis, key); + add(descriptor.value); + add(descriptor.get); + add(descriptor.set); + if ("value" in descriptor && descriptor.configurable) { + Object.defineProperty(globalThis, key, { writable: false, configurable: false }); + } + } + add(Object.getPrototypeOf(function* () {})); + add(Object.getPrototypeOf(async function () {})); + add(Object.getPrototypeOf(async function* () {})); + add(Object.getPrototypeOf(Int8Array)); + add(Object.getPrototypeOf([][Symbol.iterator]())); + add(Object.getPrototypeOf(new Map()[Symbol.iterator]())); + add(Object.getPrototypeOf(new Set()[Symbol.iterator]())); + add(Object.getPrototypeOf(""[Symbol.iterator]())); + add(Object.getPrototypeOf(/a/[Symbol.matchAll](""))); + if (typeof Iterator === "function") { + if (typeof Iterator.prototype.map === "function") add(Object.getPrototypeOf([].values().map((x) => x))); + if (typeof Iterator.from === "function") add(Object.getPrototypeOf(Iterator.from({ next() {} }))); + } + + while (queue.length > 0) { + const object = queue.pop(); + add(Object.getPrototypeOf(object)); + const descriptors = Object.getOwnPropertyDescriptors(object); + for (const key of Reflect.ownKeys(descriptors)) { + const descriptor = descriptors[key]; + if ("value" in descriptor) { + add(descriptor.value); + if ( + descriptor.writable && + descriptor.configurable && + (object === Object.prototype || OVERRIDABLE.has(key)) + ) { + allowOverride(object, key, descriptor.value, descriptor.enumerable); + } + } else { + add(descriptor.get); + add(descriptor.set); + } + } + Object.freeze(object); + } + })(); const stringify = JSON.stringify; const parse = JSON.parse; const promiseThen = Promise.prototype.then; @@ -85,11 +179,17 @@ export const PRELUDE_SOURCE: string = `(function (bridge, toolsJson, globalsJson } } + // The host requires string fields. A script can set an error's name or message to anything, so + // they are coerced, and a value whose coercion throws gets a placeholder. function describeError(error) { - if (error instanceof ErrorCtor) { - return stringify({ name: error.name, message: error.message, stack: errorText(error) }); + try { + if (error instanceof ErrorCtor) { + return stringify({ name: String(error.name), message: String(error.message), stack: errorText(error) }); + } + return stringify({ message: String(format(error)) }); + } catch { + return stringify({ message: "The script threw a value that cannot be described" }); } - return stringify({ message: format(error) }); } function caller(kind, name, spread) { diff --git a/packages/codemode/test/fixtures/raw-worker.ts b/packages/codemode/test/fixtures/raw-worker.ts new file mode 100644 index 000000000..dacf83836 --- /dev/null +++ b/packages/codemode/test/fixtures/raw-worker.ts @@ -0,0 +1,7 @@ +/** + * Stands in for the real worker in host tests: posts the JSON message passed as the script source, + * so tests can send the host payloads the real prelude never produces. + */ +import { parentPort, workerData } from "node:worker_threads"; + +parentPort?.postMessage(JSON.parse((workerData as { code: string }).code)); diff --git a/packages/codemode/test/sandbox.test.ts b/packages/codemode/test/sandbox.test.ts index 0fcd6fac4..7645dcfc1 100644 --- a/packages/codemode/test/sandbox.test.ts +++ b/packages/codemode/test/sandbox.test.ts @@ -651,6 +651,24 @@ describe("limits and lifetime", () => { error: { kind: "sandbox", message: "Failed to load QuickJS: no wasm" }, }); }); + + // #10444: a malformed payload from the worker must fail the run as a sandbox error instead of + // throwing in the host's message listener and never settling. + it.each([ + [{ type: "done", ok: true, value: "1", writes: "null" }, "store writes are not an array"], + [{ type: "done", ok: true, value: "1", writes: "[1]" }, "store writes contain a malformed entry"], + [{ type: "done", ok: true, value: "1", writes: '[["k", "{"]]' }, 'store value for "k" is not valid JSON'], + [{ type: "done", ok: true, value: "{", writes: "[]" }, "return value is not valid JSON"], + [{ type: "done", ok: false, error: "5" }, "script error is not an object"], + [{ type: "done", ok: false, error: "{}" }, "script error is malformed"], + [{ type: "nonsense" }, "unknown message from the worker"], + ])("reports a broken bridge as a sandbox error: %j", async (message, reason) => { + const sandbox = new CodemodeSandbox({ workerUrl: new URL("./fixtures/raw-worker.ts", import.meta.url) }); + sandboxes.push(sandbox); + const result = await sandbox.execute(JSON.stringify(message)); + expect(result).toMatchObject({ ok: false, error: { kind: "sandbox" } }); + expect(result.ok ? undefined : result.error.message).toContain(`Sandbox bridge broken: ${reason}`); + }); }); describe("escape hatches", () => { @@ -702,6 +720,70 @@ describe("escape hatches", () => { expect(result.value).not.toBe("imported"); }); + // #10444: the prelude shares the built-ins with the script, so patching them could corrupt what + // the prelude sends to the host. + it("ignores patches to built-ins and built-in globals", async () => { + const sandbox = createSandbox([echo]); + const result = await sandbox.execute(` + Array.prototype.toJSON = () => null; + Object.prototype.toJSON = () => 5; + Promise.prototype.then = () => {}; + Map.prototype.get = () => undefined; + globalThis.JSON = { stringify: () => "x", parse: () => "x" }; + store("k", [1]); + return [await tools.echo([2]), JSON.stringify({ a: 1 })]; + `); + expect(result).toMatchObject({ ok: true, value: [[2], '{"a":1}'], storeWrites: { set: { k: [1] } } }); + }); + + it("freezes intrinsics that are only reachable from instances", async () => { + const sandbox = createSandbox(); + const result = await sandbox.execute(` + return [ + Object.getPrototypeOf(function* () {}).prototype, + Object.getPrototypeOf(async function () {}), + Object.getPrototypeOf(Int8Array).prototype, + Object.getPrototypeOf([][Symbol.iterator]()), + Object.getPrototypeOf(Object.getPrototypeOf([][Symbol.iterator]())), + Object.getPrototypeOf(new Map()[Symbol.iterator]()), + Object.getPrototypeOf(/a/[Symbol.matchAll]("")), + ].every((object) => Object.isFrozen(object)); + `); + expect(result).toMatchObject({ ok: true, value: true }); + }); + + it("still lets instances override properties of frozen prototypes", async () => { + const sandbox = createSandbox(); + const result = await sandbox.execute(` + const object = {}; + object.toString = () => "custom"; + function Legacy() {} + Legacy.prototype = Object.create(Error.prototype); + Legacy.prototype.constructor = Legacy; + const bare = new Error(); + bare.message = "set later"; + class MyError extends Error { + constructor(message) { + super(message); + this.name = "MyError"; + } + } + let patched = "silent"; + try { Error.prototype.name = "Patched"; } catch (error) { patched = error.constructor.name; } + return [String(object), new Legacy().constructor === Legacy, bare.message, new MyError("x").name, Error.prototype.name, patched]; + `); + expect(result).toMatchObject({ + ok: true, + value: ["custom", true, "set later", "MyError", "Error", "TypeError"], + }); + }); + + it("reports errors whose name or message is not a string", async () => { + const sandbox = createSandbox(); + const result = await sandbox.execute("const error = new Error('x'); error.message = 42; throw error;"); + expect(result).toMatchObject({ ok: false, error: { kind: "script", name: "Error", message: "42" } }); + }); + it("keeps tools and console frozen", async () => { const sandbox = createSandbox([echo]); const result = await sandbox.execute(`