fix(runtime): safe-marshal hook ctx so circular host handles don't crash the sandbox (#2674) (#2676)

`QuickJSScriptRunner.installCtx` serialised the hook context into the
sandbox with a bare `JSON.stringify`. A live `setTimeout`/`setInterval`
handle reachable from `ctx` links back on itself
(`Timeout._idlePrev -> TimersList._idleNext -> …`), so `JSON.stringify`
threw `TypeError: Converting circular structure to JSON` and took the
whole hook down.

Route all three host→VM marshalling paths (`setGlobalJson`,
`setObjectJson`, `jsonToHandle`) through a shared `safeJsonStringify`
that drops circular back-edges via a path `WeakSet` and coerces `BigInt`
to a string. Only JSON-safe leaves cross the boundary — unserialisable
host objects are stripped rather than fatal — and the body still runs.

Closes #2674.

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: os-zhuang

.changeset/quickjs-safe-json-marshal.md

@@ -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.

packages/runtime/src/sandbox/quickjs-runner.test.ts

@@ -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', () => {

packages/runtime/src/sandbox/quickjs-runner.ts

@@ -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);
@@ -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();
@@ -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();