sandbox: charge script CPU time (not wall clock), drop asyncify, precompile the WASM module — layered follow-up to #3259

[os-zhuang]

Implementation-ready spec. #3259 diagnosed the flake; #3270 shipped the mitigation (env-overridable timeout defaults + a 10s CI floor). This issue is the structural fix, split into 3 independently-shippable phases — one PR per phase, in order. Written so another agent can implement without re-deriving. All code sites anchor to packages/runtime/src/sandbox/quickjs-runner.ts (line numbers as of main@32899e6; re-confirm before editing). API facts for Phases 2–3 were verified against the installed quickjs-emscripten@0.32.0 .d.ts files.

Why

QuickJSScriptRunner is production infrastructure: every metadata-authored Hook.body / Action.body runs through it (four stock runners in app-plugin.ts:227,265,557,588), executing untrusted tenant/author JS behind a security boundary. Three structural problems remain after #3270:

1. Wall-clock budget conflates two concerns. One number bounds both "tolerate a slow/loaded host" and "kill a runaway script". Raising OS_SANDBOX_HOOK_TIMEOUT_MS for stability also widens the per-invocation CPU-DoS window. In prod a tripped budget is a failed user write — load-dependent, hard to reproduce.
2. Nested hooks are charged to the parent. start is stamped after VM creation (:163), but a child hook that does a nested cross-object write parks on the host call while the parent's entire hook invocation (its own execute()) runs host-side — and every ms counts against the child's wall clock even though the child VM executes nothing meanwhile.
3. Per-invocation fixed cost. Every call instantiates a fresh asyncify WASM module (newAsyncContext(), :158) and disposes it (:286). Asyncify is bigger, slower to compile, slower per-instruction; under per-row hook storms (bulk writes) that is a real tax + GC churn.

---

Phase 1 — charge script CPU time, not wall clock (PR 1)

The budget from resolveTimeout() (:139-143) becomes a VM-active-time budget; add a separate, generous wall-clock ceiling as the stuck-host backstop. Fixes #1 and #2.

1.1 Runner changes (quickjs-runner.ts)

• Constants (:41-43): add const DEFAULT_WALL_CEILING_MS = 30_000; (30s aligns with the spec cap on ScriptBody.timeoutMs).
• QuickJSScriptRunnerOptions (:45-52): add wallCeilingMs?: number. Resolve it in the constructor (:57-70) like the timeout knobs (see 1.5).
• Rework execute() timing — replace start/deadline/setInterruptHandler (:163-165):

const cpuBudget = args.timeoutMs;                                   // resolveTimeout() result — now a CPU budget
const wallCeiling = Math.max(this.opts.wallCeilingMs, cpuBudget);   // never below the CPU budget
const start = Date.now();
const wallDeadline = start + wallCeiling;
let cpuMs = 0;      // sum of completed VM slices
let sliceStart = 0; // stamped immediately before each VM entry; read by the interrupt handler
runtime.setInterruptHandler(() => {
  const now = Date.now();
  return (cpuMs + (now - sliceStart)) > cpuBudget || now > wallDeadline;
});

• Wrap every VM entry in the slice stopwatch (sliceStart = Date.now() before, cpuMs += Date.now() - sliceStart after):
  • L1 vm.evalCode(wrapped) (:180)
  • L2 await vm.evalCodeAsync(wrapped) (:216) — runs the async IIFE only to its first await, so this slice = the synchronous prefix, correctly charged.
  • each pump runtime.executePendingJobs() (:243).
  • The await new Promise(setImmediate) yield (:241) and host-promise settle time stay OUTSIDE the stopwatch → not charged. That is the fix: idle host waits and nested-hook execution (host-side, while this VM is parked) no longer count.
• Replace the deadline check (:273-276) with two distinct ones:

if (cpuMs > cpuBudget)
  throw new SandboxError(`${origin.kind} '${origin.name}' exceeded CPU budget of ${cpuBudget}ms (after ${pumps} pump iterations)`);
if (Date.now() > wallDeadline)
  throw new SandboxError(`${origin.kind} '${origin.name}' exceeded wall-clock ceiling of ${wallCeiling}ms while awaiting host calls (after ${pumps} pump iterations)`);

• Interrupt-error mapping (important). When the handler returns true mid-slice, QuickJS raises InternalError: interrupted, surfacing as result.error/evalRes.error/pending.error (:181,217,244) — today dumped as "… threw: InternalError: interrupted". At each of those three sites, first if (cpuMs + (Date.now()-sliceStart) > cpuBudget) → throw the CPU-budget error; else if (Date.now() > wallDeadline) → wall-ceiling error; else fall through to the existing "threw:" dump (a genuine script error). Factor into one throwIfBudgetExceeded(pumps) helper.
• Accuracy caveat (document in code). Slice wall-time ≈ CPU only when unpreempted; under contention it over-estimates (cuts a runaway early, never late) — acceptable, and it still excludes the dominant inflation (idle waits, nested hooks, setImmediate). Optional refinement: process.cpuUsage(before) deltas around each synchronous slice give true main-thread CPU (the delta around a synchronous executePendingJobs() is exactly this VM's CPU). Ship the Date.now() version in Phase 1 (no syscall in the hot interrupt path); note the upgrade.
• Optional: add cpuMs to ScriptResult next to durationMs (:196,270) for observability; durationMs stays wall time.

1.2 Test deltas (quickjs-runner.test.ts) — REQUIRED (or the suite hangs)

The current timeout tests use a never-settling host call and assert /timeout of Nms/. Under CPU semantics a parked VM burns ~0 CPU → they'd wait the 30s wall ceiling → exceed vitest's 10s per-test timeout → red. Rewrite:

• respects the timeoutMs cap (:56, while(true){}, 50ms): already a real CPU burner → tighten .rejects.toThrow() → /CPU budget of 50ms/.
• Timeout-resolution suite (:372-481, pinned defaultRunner 250ms):
  • honors a hook body timeoutMs above the 250ms default (host settles ~600ms): VM parks on the idle host call → resolves unchanged; now also proves idle time isn't charged.
  • still applies the 250ms hook default … and lets a hook body LOWER its timeout …: rewrite body → while(true){}; assert /CPU budget of 250ms/ and /CPU budget of 50ms/.
• Env-override suite env-overridable timeout defaults (#3259) (:441-481): all four use neverSettles + /timeout of Nms/. Rewrite: swap neverSettles → while(true){}; assert /CPU budget of Nms/ (250/150/50/250). Env resolution is orthogonal to CPU-vs-wall.
• Pump-budget suite (:484-560, timeoutMs 30000 + fast host calls): still pass. The >1000 event-loop turns case (:497) is now trivially safe; optionally re-assert with a SMALL cpu budget (e.g. 100) to lock in the semantics.

Add: (1) wall-ceiling fires — new QuickJSScriptRunner({ hookTimeoutMs: 10_000, wallCeilingMs: 300 }), never-settling host → /wall-clock ceiling of 300ms/. (2) THE signature test / #3259 root-cause guard — hookTimeoutMs: 250, host update settling ~500ms (setTimeout) → must resolve (old wall-clock 250ms would have killed it; CPU budget doesn't). (3) nested hook charged to itself (mirror nested-write.integration.test.ts): small hook budget, child→parent nested write → child resolves.

1.3 Integration tests

nested-write*.integration.test.ts pass hookTimeoutMs: 10_000 today. Under Phase 1 they pass at stock 250ms — drop the explicit 10_000 so they actively assert the new semantics (keep a comment on why 250ms now suffices).

1.4 Rollout

• Changeset (@objectstack/runtime minor; types minor if the env helper gains a wallCeiling kind): user-visible semantics change — error text changes and OS_SANDBOX_HOOK_TIMEOUT_MS now means CPU-time. State FROM→TO.
• Docs content/docs/deployment/environment-variables.mdx: change the OS_SANDBOX_*_TIMEOUT_MS rows from "wall-clock budget" → "script CPU-time budget"; add OS_SANDBOX_WALL_CEILING_MS if adopted.
• ADR docs/adr/0102-*.md (next free number; follow the 0101 header format): record the timeout semantics decision + the Phase 2/3 engine-variant decision incl. the rejected shared-module option (below).
• CI: once Phase 1 soaks, a separate small PR drops OS_SANDBOX_HOOK_TIMEOUT_MS=10000 from .github/workflows/ci.yml. Not inside Phase 1.

1.5 Decide before coding

Wall ceiling as constructor option only (wallCeilingMs, needed for fast tests) vs also OS_SANDBOX_WALL_CEILING_MS (symmetric with the CPU knob; useful for slow-IO backends). Recommend both; if env, extend resolveSandboxTimeoutMs (packages/types/src/env.ts) with a wallCeiling kind or a sibling helper.

---

Phase 2 — drop asyncify (sync variant), keep per-invocation modules (PR 2)

Verified against quickjs-emscripten@0.32.0: the sync release variant @jitl/quickjs-wasmfile-release-sync (RELEASE_SYNC) is already installed — no new dependency. The sync QuickJSContext exposes evalCode() + newPromise(), and its .runtime exposes executePendingJobs() / setInterruptHandler() / setMemoryLimit() / setMaxStackSize() — identical to the async path; only evalCodeAsync has no sync-context use.

Why it's safe now: the original crashes (memory access out of bounds, HostRef double-free) were an asyncify limitation (one suspended stack per module). The deferred-promise + pump redesign already removed all wasm-stack suspension — installApiMethod (:525-588) uses plain vm.newPromise() (:552), not newAsyncifiedFunction; host calls are ordinary QuickJS promises driven by executePendingJobs. The only async APIs left are newAsyncContext()/evalCodeAsync(), and the initial eval only runs to the first await.

Change:

• :158 const vm = await newAsyncContext(); → const mod = await newQuickJSWASMModule(); const vm = mod.newContext();
  • newQuickJSWASMModule() defaults to RELEASE_SYNC and creates a new, completely isolated WebAssembly module (own linear memory) per call — isolation model UNCHANGED (confirmed in the .d.ts: fresh module = own linear memory; only module.newContext() on a shared module shares memory — which we do NOT do).
• :216 await vm.evalCodeAsync(wrapped) → vm.evalCode(wrapped) (sync).
• Imports (:27-31): newAsyncContext → newQuickJSWASMModule; drop type QuickJSAsyncContext (use QuickJSContext). Update the QuickJSAsyncContext param types in installCtx/installApiMethod/tx-leaf helpers.
• Disposal: newAsyncContext() tied module lifetime to the context (vm.dispose() freed both, :284-286). With mod.newContext(), hold BOTH refs and in finally dispose the context then the module (verify whether disposing the context alone frees the module in 0.32; dispose the module explicitly to be safe — it's the object that owns the linear memory).
• Everything else — pump loop, installCtx, deferred-promise host calls, transaction leaves, vm.alive guards — unchanged.

Wins: no ASYNCIFY instrumentation → smaller binary, faster compile/instantiate, faster per-instruction. Safety net: re-entrancy / fan-out / lvl4-nesting / real-sqlite suites stay green unchanged.

---

Phase 3 — compile the WASM module once, instantiate per invocation (PR 3)

Verified API (0.32): newVariant(baseVariant, options) with CustomizeVariantOptions.wasmModule?: OrLoader<WebAssembly.Module> — "If given, Emscripten will instantiate the WebAssembly.Instance from this existing WebAssembly.Module." The compiled WebAssembly.Module is stateless bytecode; each newQuickJSWASMModule(variant) still allocates a fresh linear memory (isolation preserved) as long as you do NOT also pass wasmMemory (which would deliberately share memory = the rejected model).

Change: module-level, once per process:

const wasmModule = await WebAssembly.compile(baseWasmBytes);           // compile ONCE
const FAST_VARIANT = newVariant(RELEASE_SYNC, { wasmModule });         // reuse across instantiations

then Phase 2's newQuickJSWASMModule() → newQuickJSWASMModule(FAST_VARIANT).

• Obtain baseWasmBytes once — confirm how to read the RELEASE_SYNC variant's .wasm (variant exposes wasmLocation; CustomizeVariantOptions also accepts wasmBinary?: OrLoader<ArrayBuffer>). Lower-level fallback: the Emscripten instantiateWasm(imports, onSuccess) hook via CustomizeVariantOptions.emscriptenModule — build new WebAssembly.Instance(cachedModule, imports) per call.
• Assert the invariant in an RSS soak test: fresh instance ⇒ fresh linear memory (only compiled code shared).
• If raw-bytes access proves awkward in 0.32, use the instantiateWasm fallback; if neither is clean, Phase 3 is deferrable — Phase 2 already removes the asyncify cost.

---

Explicitly REJECTED — shared singleton module + per-invocation newContext()

Recorded here and in the ADR so it's not re-proposed as an "optimization". The .d.ts confirms the isolation hierarchy: separate QuickJSWASMModule (own linear memory) > separate runtime (shared memory, no object exchange) > separate context (shared memory).

• Threat model: the sandbox runs attacker-authorable JS; a QuickJS C-heap bug yields arbitrary R/W within the module's linear memory. Today the blast radius is the attacker's own invocation; a shared module reaches other concurrent invocations' marshalled record data in the same linear memory — one engine bug ⇒ cross-tenant confidentiality/integrity incident. Per-invocation physical memory isolation is deliberate defense-in-depth for a platform whose promise is "AI agents can operate it safely".
• RSS ratchet: wasm linear memory never shrinks — a shared module grows to peak concurrency and stays; per-invocation modules return everything on dispose.
• After Phases 1–3 the residual cost is a cheap instantiate; no performance gap justifies this trade. (newVariant({ wasmMemory }) would also share memory — do not use it.)

---

Verification (each phase)

• Green unchanged: nested-write.integration, nested-write-real-sqlite.integration, re-entrancy / fan-out / lvl4 nesting, crash regressions — the whole src/sandbox/ suite.
• Micro-benchmark: add packages/runtime/src/sandbox/quickjs-runner.bench.ts (bench/describe from vitest, mirror packages/spec/src/benchmark.bench.ts, run npx vitest bench): per-invocation overhead cold + under CPU contention, before/after Phases 2–3; single hook and a child→parent nested pair.
• RSS soak: bulk write driving per-row hooks at high throughput; watch RSS across N invocations (guards the isolation / memory-return invariant, esp. Phase 3).
• Flake canary: nested-write suite ×N under parallel CPU stress at the stock 250ms budget — must stay green (proves Phase 1 killed the load flake).

Refs

#3259 (flake) · #3270 (mitigation, merged) · #3264 / 9498eb4 (earlier per-test bumps).

🤖 Generated with Claude Code