Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/quickjs-safe-json-marshal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@objectstack/runtime': patch
---

Sandbox: stop `QuickJSScriptRunner` from crashing when a hook context holds a non-serialisable host object.

`installCtx` marshalled `ctx` into the QuickJS sandbox with a bare `JSON.stringify`. If the context (or anything reachable from it) held a live `setTimeout`/`setInterval` handle, `JSON.stringify` threw `TypeError: Converting circular structure to JSON` (`Timeout._idlePrev -> TimersList._idleNext -> …`) and took the whole hook down (#2674). Marshalling now goes through a shared `safeJsonStringify` that drops circular back-edges via a path `WeakSet` and coerces `BigInt` to a string, so only JSON-safe leaves cross the boundary and the body still runs.
35 changes: 35 additions & 0 deletions packages/runtime/src/sandbox/quickjs-runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,41 @@ describe('QuickJSScriptRunner — L2 hook script', () => {
expect(err!.message).toContain("action 'lead_apply_convert' threw:");
expect(err!.innerMessage).toBe('线索信息不完整');
});

it('marshals ctx.input containing a circular Timeout handle without crashing (#2674)', async () => {
// A live setInterval handle links back on itself
// (Timeout._idlePrev -> TimersList._idleNext -> …). Naive JSON.stringify
// over ctx would throw "Converting circular structure to JSON" and take the
// hook down. The runner must strip the back-edge and run the body.
const timer = setInterval(() => {}, 1_000);
try {
const r = await runner.runScript(
{
language: 'js',
source: 'return { ok: true, n: ctx.input.n };',
capabilities: [],
},
ctx({ input: { n: 5, timer } as unknown as Record<string, unknown> }),
hookOpts,
);
expect(r.value).toEqual({ ok: true, n: 5 });
} finally {
clearInterval(timer);
}
});

it('marshals a BigInt in ctx.input by coercing to string rather than throwing', async () => {
const r = await runner.runScript(
{
language: 'js',
source: 'return { big: ctx.input.big };',
capabilities: [],
},
ctx({ input: { big: 42n } as unknown as Record<string, unknown> }),
hookOpts,
);
expect(r.value).toEqual({ big: '42' });
});
});

describe('QuickJSScriptRunner — L2 action script', () => {
Expand Down
38 changes: 35 additions & 3 deletions packages/runtime/src/sandbox/quickjs-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -569,9 +569,41 @@ function installApiMethod(
fn.dispose();
}

/**
* Serialise a host value for marshalling into the VM, tolerating shapes that
* plain `JSON.stringify` chokes on. Only JSON-safe leaves cross the sandbox
* boundary anyway, so anything unserialisable is dropped rather than fatal:
*
* - **Circular references** — a live `setTimeout`/`setInterval` handle (or any
* node host object) reachable from `ctx` links back on itself
* (`Timeout._idlePrev -> TimersList._idleNext -> …`). Naive `JSON.stringify`
* throws `TypeError: Converting circular structure to JSON` and takes the
* whole hook down (issue #2674). A `WeakSet` of ancestors on the current path
* drops the back-edge instead.
* - **BigInt** — `JSON.stringify` throws outright; we coerce to a string.
*
* The replacer is deliberately conservative: it strips only what would crash
* the serialiser, leaving legitimate data intact.
*/
function safeJsonStringify(v: unknown): string {
const seen = new WeakSet<object>();
const json = JSON.stringify(v ?? null, function (this: unknown, _key, value) {
if (typeof value === 'bigint') return value.toString();
if (typeof value === 'object' && value !== null) {
if (seen.has(value)) return undefined; // drop circular back-edge
seen.add(value);
}
return value;
});
// `JSON.stringify` returns `undefined` when the top-level value is itself
// unserialisable (e.g. a bare function); normalise to a JSON literal so the
// downstream `vm.evalCode` never receives `(undefined)`.
return json ?? 'null';
}

/** Marshal a host JSON-serializable value into a QuickJS handle. */
function jsonToHandle(vm: QuickJSAsyncContext, v: unknown): QuickJSHandle {
const json = JSON.stringify(v ?? null);
const json = safeJsonStringify(v);
const r = vm.evalCode(`(${json})`);
if (r.error) {
const msg = vm.dump(r.error);
Expand All @@ -582,7 +614,7 @@ function jsonToHandle(vm: QuickJSAsyncContext, v: unknown): QuickJSHandle {
}

function setGlobalJson(vm: QuickJSAsyncContext, name: string, v: unknown): void {
const json = JSON.stringify(v ?? null);
const json = safeJsonStringify(v);
const result = vm.evalCode(`(${json})`);
if (result.error) {
result.error.dispose();
Expand All @@ -593,7 +625,7 @@ function setGlobalJson(vm: QuickJSAsyncContext, name: string, v: unknown): void
}

function setObjectJson(vm: QuickJSAsyncContext, parent: QuickJSHandle, key: string, v: unknown): void {
const json = JSON.stringify(v ?? null);
const json = safeJsonStringify(v);
const result = vm.evalCode(`(${json})`);
if (result.error) {
result.error.dispose();
Expand Down