Skip to content

Commit 52a2d74

Browse files
authored
docs(adr): ADR-0102 — sandbox CPU-time budget + per-invocation sync WASM engine (#3275) (#3299)
Adds docs/adr/0102-sandbox-cpu-budget-and-engine-variant.md (Proposed): D1 CPU-time budget + wall ceiling, D2 per-invocation sync WASM modules (drop asyncify), D3 precompile-once (deferrable), D4 shared-module rejected on threat-model grounds. Front-loads the contract for phases #3295/#3296/#3297. Refs #3275, #3259, #3270.
1 parent e0859b1 commit 52a2d74

1 file changed

Lines changed: 76 additions & 0 deletions

File tree

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# ADR-0102: Sandbox Execution Budget & Engine Variant — CPU-time budget with a wall ceiling; per-invocation sync WASM modules, precompiled
2+
3+
**Status**: Proposed (2026-07-19) — spec'd and API-verified in framework#3275; lands as Phase 1 #3295 (D1), Phase 2 #3296 (D2), Phase 3 #3297 (D3). Flips to **Accepted** when Phase 1 merges; D3 is explicitly deferrable without weakening D1/D2.
4+
**Deciders**: ObjectStack Protocol Architects
5+
**Builds on**: the #1867 sandbox redesign (deferred-promise host calls + pump loop) — the mechanism that both retired asyncify's only remaining justification (D2) and created the discrete VM-entry points D1 meters. No earlier ADR covers the sandbox runner; prior art is code history (#3232 honor body `timeoutMs`, #3264 test de-flake, #3270 env-overridable defaults).
6+
**Tracking**: framework#3275 (implementation-ready spec) · #3295 / #3296 / #3297 (phases) · #3259 (motivating CI flake, closed by the #3270 stopgap)
7+
**Consumers**: `@objectstack/runtime` (`QuickJSScriptRunner`, the four stock runners in `app-plugin.ts`), `@objectstack/types` (`resolveSandboxTimeoutMs`), `.github/workflows/ci.yml` (the temporary `OS_SANDBOX_HOOK_TIMEOUT_MS=10000` floor this ADR makes removable), `content/docs/deployment/environment-variables.mdx`
8+
9+
---
10+
11+
## TL;DR
12+
13+
The QuickJS sandbox (`packages/runtime/src/sandbox/quickjs-runner.ts`) is the production execution engine for every metadata-authored `Hook.body` / `Action.body` — untrusted tenant/author JS behind a security boundary. Two structural decisions:
14+
15+
1. **D1 — the per-invocation budget charges *script CPU time*, not wall clock.** The existing knobs (`body.timeoutMs`, `hookTimeoutMs` / `actionTimeoutMs`, `OS_SANDBOX_{HOOK,ACTION}_TIMEOUT_MS`) keep their names and defaults but become a **VM-active-time budget**; wall clock survives only as a separate, generous **ceiling** (default 30s) bounding stuck host calls.
16+
2. **D2/D3 — the engine is a fresh, physically isolated *sync* WASM module per invocation (D2), instantiated from a once-compiled `WebAssembly.Module` (D3).** Asyncify is dropped — its only consumer disappeared with the #1867 deferred-promise redesign — while the per-invocation linear-memory isolation is retained as deliberate defense-in-depth.
17+
18+
The tempting third shape — **one shared module with per-invocation contexts — is rejected** (D4) on threat-model grounds and recorded here so it is not re-proposed as an "optimization".
19+
20+
## Context
21+
22+
Every sandbox invocation today creates a fresh **asyncify** WASM module (`newAsyncContext()`), enforces one **wall-clock** deadline over the whole invocation (interrupt handler + pump-loop check), and disposes the module on settle. Three structural problems, all surfaced by the #3259 flake and its investigation:
23+
24+
- **One number, two unrelated meanings.** The wall-clock budget simultaneously bounds "tolerate a slow/loaded host" and "kill a runaway script". An operator who raises `OS_SANDBOX_HOOK_TIMEOUT_MS` for stability on constrained hardware also widens the per-invocation CPU-DoS window for a `while(1)` body — the knob cannot serve both masters. In production a tripped budget is a **failed user write**, load-dependent and hard to reproduce.
25+
- **Nested hooks are billed to the parent.** A hook that performs a nested cross-object write parks on the host call while the *child object's* hook runs its own full invocation (module creation + eval + pump) host-side. Every one of those milliseconds counts against the *parent's* wall clock even though the parent VM executes nothing meanwhile. This is exactly the #1867 rollup pattern the sandbox exists to support.
26+
- **Per-invocation fixed cost, doubled by asyncify.** The asyncify build is bigger (instrumentation), slower to compile/instantiate, and slower per instruction. Under per-row hook storms (bulk writes) the fixed cost is a real tax plus allocation/GC churn — and on an oversubscribed CI runner it alone intermittently blew the 250ms hook default while the VM was still making progress (the #3259 flake signature: variable pump counts, same wall deadline).
27+
28+
Why asyncify is now vestigial: the #1867 redesign moved every host call to a **deferred QuickJS promise settled from the host event loop**, drained by a pump loop (`executePendingJobs`). Nothing suspends the WASM stack anymore — `installApiMethod` deliberately does not use `newAsyncifiedFunction`, and the initial `evalCodeAsync` only runs the wrapped async IIFE to its first `await`. The historical crashes that forced per-invocation modules (`memory access out of bounds`, HostRef double-free) were asyncify's one-suspended-stack-per-module limit — not a property of module sharing per se. The isolation hierarchy in `quickjs-emscripten@0.32.0` (verified against the installed `.d.ts`): separate **module** = own WebAssembly instance + own linear memory ("the most isolation guarantees possible"); separate **runtime** on one module = shared memory, no object exchange; separate **context** = shared memory, objects shareable.
29+
30+
## Decisions
31+
32+
### D1 — The budget is script CPU time; wall clock is only a ceiling
33+
34+
The value resolved by `resolveTimeout()` (body `timeoutMs` › enclosing `opts.timeoutMs` › runner default, smaller-explicit-wins) becomes a **VM-active-time budget**:
35+
36+
- The host stopwatches every VM entry — the initial `evalCode`, and each `executePendingJobs` slice in the pump loop. Idle pump yields (`setImmediate`), host-promise settle time, and nested-hook execution are **not charged**.
37+
- The interrupt handler cuts a runaway synchronous loop mid-slice: `(cpuMs + currentSliceElapsed) > cpuBudget || now > wallDeadline`. A mid-slice interrupt is mapped back to the clean budget error, not surfaced as `InternalError: interrupted`.
38+
- A separate **wall-clock ceiling**`max(DEFAULT_WALL_CEILING_MS = 30_000, cpuBudget)`, constructor-overridable (`wallCeilingMs`), 30s chosen to match the spec cap on `ScriptBody.timeoutMs` — is the *only* wall bound, and exists solely to cut a hook parked forever on a host call that never settles (the interrupt handler cannot fire while no VM code runs).
39+
- Distinct, greppable errors: `exceeded CPU budget of {N}ms …` vs `exceeded wall-clock ceiling of {N}ms while awaiting host calls …`.
40+
- Accuracy: slice wall-time over-estimates CPU under scheduler preemption — the failure mode is cutting a runaway *early*, never letting it run *late*. A `process.cpuUsage()`-delta refinement is compatible and deferred.
41+
42+
Consequence for the knobs: names, defaults (250ms hooks / 5000ms actions), and precedence are unchanged; only the *dimension* changes. Raising the CPU budget no longer trades away host-latency tolerance, and vice versa. This restores meaning to the 250ms default on any hardware — which is what lets CI drop its 10s floor (a stopgap that today also loosens the runaway bound for every test).
43+
44+
Open point to settle at Phase 1 review: whether the ceiling also gets `OS_SANDBOX_WALL_CEILING_MS` (recommended — symmetric with the CPU knobs, wanted by slow-IO deployments) or stays constructor-only.
45+
46+
### D2 — Drop asyncify: per-invocation *sync* modules (isolation unchanged)
47+
48+
Switch `newAsyncContext()``newQuickJSWASMModule().newContext()` (the sync release variant `@jitl/quickjs-wasmfile-release-sync` is already installed) and `evalCodeAsync``evalCode`. The sync surface is otherwise identical (`newPromise`, `executePendingJobs`, `setInterruptHandler`, `setMemoryLimit`, `setMaxStackSize`).
49+
50+
- **Invariant (normative): one fresh, isolated WASM module per invocation.** `newQuickJSWASMModule()` creates a new WebAssembly instance with its own linear memory; nothing may downgrade this to runtime- or context-level sharing (see D4).
51+
- Disposal transfers from the context to the pair: dispose context, then module — the module owns the linear memory.
52+
- Wins: no asyncify state machine on every call (per-instruction speedup), smaller binary, faster compile/instantiate, and removal of an entire class of suspended-stack failure modes the current code only avoids by convention.
53+
54+
### D3 — Compile the WASM once; instantiate per invocation (deferrable)
55+
56+
Cache one compiled `WebAssembly.Module` per process and build a customized variant via `newVariant(RELEASE_SYNC, { wasmModule })` (verified in 0.32: *"Emscripten will instantiate the WebAssembly.Instance from this existing WebAssembly.Module"*), so each invocation pays only instantiation — fresh linear memory, shared stateless bytecode. The `instantiateWasm` Emscripten hook is the lower-level fallback. **`wasmMemory` must never be passed** — injecting a shared memory would silently recreate the rejected D4 shape. If clean access to the variant's `.wasm` bytes proves awkward, D3 defers without weakening D1/D2.
57+
58+
### D4 — No shared singleton module (rejected alternative: shared module + per-invocation contexts)
59+
60+
Rejected, permanently, for this codebase's threat model — the sandbox runs **attacker-authorable JS**:
61+
62+
- A QuickJS C-heap bug (historically recurrent in JS engines) yields arbitrary R/W **within the module's linear memory**. With per-invocation modules the blast radius is the attacker's own invocation; with a shared module it reaches **other concurrent invocations' marshalled record data in the same linear memory** — one engine bug becomes a cross-tenant confidentiality/integrity incident. Physical per-invocation memory is defense-in-depth the platform's "AI agents can operate it safely" promise pays for.
63+
- WASM linear memory never shrinks: a shared module's RSS ratchets to peak concurrency forever; per-invocation modules return everything on dispose.
64+
- After D1–D3 the residual per-call cost is a cheap instantiate; no remaining performance gap justifies the trade.
65+
66+
## Consequences
67+
68+
- **Positive**
69+
- The #3259 class of flake dies at the root: on a loaded host, idle waits and nested hooks no longer consume the script's budget; the stock 250ms default becomes meaningful everywhere, and CI's `OS_SANDBOX_HOOK_TIMEOUT_MS=10000` floor becomes removable (follow-up PR after Phase 1 soaks).
70+
- Security posture *tightens*: the CPU-DoS bound decouples from host-latency tolerance, and per-invocation physical isolation is now a written invariant rather than an accident of asyncify's limitations.
71+
- Hook-storm throughput improves (no asyncify tax; with D3, no per-call compile).
72+
- **Negative / accepted**
73+
- User-visible error-message change (`timeout of Nms``CPU budget of Nms` / `wall-clock ceiling of Nms`) — a semantics change shipped with a changeset stating FROM → TO, plus docs updates.
74+
- A stuck host call can now hold a VM (and its rolled-back-on-dispose transaction state) up to the 30s ceiling instead of the old small wall budget — bounded, and the correct trade: the old behavior killed *legitimate* slow writes to punish *hypothetical* stuck ones.
75+
- Slice-based CPU accounting is approximate under preemption (fails toward early cutoff; `process.cpuUsage()` refinement available if it ever matters).
76+
- **Verification** (normative for each phase; details in #3275): the full `src/sandbox/` suite green — including the nested-write integration tests running at the **stock 250ms** budget with the explicit 10s overrides removed; a `quickjs-runner.bench.ts` before/after benchmark; an RSS soak asserting memory returns on dispose (guards D2/D3's isolation invariant); a flake canary (`nested-write` ×N under CPU stress at stock defaults).

0 commit comments

Comments
 (0)