# Architecture Decision Records — `@coda/sandbox` This document records the key design decisions made for the sandbox package, the alternatives considered, and the tradeoffs involved. --- ## ADR-001: isolated-vm over vm2, node:vm, Deno, Docker **Status:** Accepted **Decision:** Use `isolated-vm` (V8 isolates via C++ addon) as the execution substrate for untrusted LLM-generated code. **Alternatives considered:** - `vm2` — abandoned. Known security escape vulnerabilities exist in the wild; the package is unmaintained. Using it would expose the host process to sandbox escapes with no upstream fix path. - `node:vm` — same process heap as the host. Not truly isolated; user code can access host globals via prototype chain attacks (`({}).constructor.constructor('return process')()`). Provides no memory or CPU limits. - Deno subprocess — true OS-level isolation via a separate process, but incurs ~200ms cold start and non-trivial IPC overhead. Deployment complexity increases (Deno binary must be present, version-pinned, and managed separately). - Docker/Firecracker — strongest possible isolation (separate kernel namespace or microVM), but 500ms+ cold start, complex container orchestration, and meaningful infrastructure cost. Overkill for per-turn LLM tool execution. - WebAssembly sandbox (WASI) — cannot execute arbitrary JavaScript, only WASM. Limits what the LLM can write to a very narrow subset of languages and cannot easily run the JS-heavy data analysis patterns we need. **Rationale:** `isolated-vm` provides hard memory limits, CPU interruption via V8 built-in timeout, and true V8 bytecode isolation (separate heap, no shared globals) with ~10–50ms cold starts. It strikes the right balance between security and performance for synchronous-ish LLM tool execution within an Express request/response cycle. **Risks:** isolated-vm is a native C++ addon bound to the V8 ABI. If Node.js ships a breaking V8 change, isolated-vm must release a compatible version before we can upgrade Node. Mitigated by pinning the Node version in `.nvmrc`/`.node-version` and monitoring isolated-vm releases before Node upgrades. --- ## ADR-002: Message-passing protocol over direct function calls **Status:** Accepted **Decision:** All communication between the host and the sandbox uses a unified `SandboxMessage` discriminated union. The engine exposes a push/pull event stream interface rather than a simple async function. **Alternatives considered:** - Direct async function — `engine.execute(code) → Promise`. Simpler API but cannot stream status events or handle interactive data requests (the sandbox needs to ask the host for data mid-execution). - Callback-based — `engine.execute(code, { onStatus, onDataRequest })`. Harder to consume correctly and provides no backpressure semantics. Callers must coordinate multiple callbacks simultaneously. - gRPC/SSE over HTTP — overkill for in-process use. Adds serialization latency and network stack overhead for no benefit when host and engine are in the same process. **Rationale:** Message-passing enables three key capabilities: streaming status events during long-running executions, interactive data requests (bidirectional communication between user code and host during a single execution), and clean abort semantics (send an abort message, session drains). The discriminated union maps naturally to SSE event types for HTTP consumers downstream, keeping the protocol consistent end-to-end. --- ## ADR-003: Ephemeral isolates (never reuse after execution) **Status:** Accepted **Decision:** `release()` always disposes the isolate. Isolates are never returned to the pool after first use; the pool only holds pre-created, never-executed isolates. **Alternatives considered:** - Reuse isolates — faster (no re-creation cost between executions), but any globals, closures, or prototype mutations set by user code persist into the next execution. This creates cross-request state leakage that is nearly impossible to audit. - Snapshot-based restore — V8 supports heap snapshots for fast context reset. Complex to implement correctly; the snapshot must be taken before any user code runs, and the ivm API does not support this cleanly. Adds significant implementation surface area for uncertain benefit. **Rationale:** Security and correctness take precedence over raw performance. The ~10–50ms isolate creation cost is acceptable because the warm pool pre-creates isolates before they are needed, hiding most of the creation latency from the critical path. --- ## ADR-004: Dual timeout strategy **Status:** Accepted **Decision:** Combine V8's built-in `{ timeout }` option (passed to `script.run()`) with an external `setTimeout` that calls `isolate.dispose()`. **Alternatives considered:** - V8 built-in timeout only — does not reliably interrupt async code. When user code is suspended at `await`, the V8 timeout clock is paused. An async execution waiting on a data request would never time out (ivm issue #185). - External dispose only — catches async timeouts correctly but is less reliable for tight synchronous infinite loops, where the event loop never yields and the external timer cannot fire. **Rationale:** Async code is the common case for LLM-generated scripts (they use `await dataProxy.request()` to fetch data from the host). The external dispose is necessary for these cases. The V8 built-in timeout is defense-in-depth for synchronous spin loops. Both mechanisms are harmless when they overlap. --- ## ADR-005: JSON serialization at isolate boundary **Status:** Accepted **Decision:** All cross-boundary data (results, data request payloads, status messages) is serialized as JSON strings. **Alternatives considered:** - `ivm.ExternalCopy` — copies primitive and structured-clone-compatible values across the isolate boundary. Requires explicit `.copy()` calls on both sides of the boundary. Does not handle non-structured-clone types (Map, Set, etc.) without manual conversion. - `ivm.Reference` — creates a proxy that calls back into the original object for each property access. Expensive for data-heavy payloads; each property access is a cross-boundary call with overhead. **Rationale:** JSON serialization is explicit, fast, and handles all JSON-compatible data structures without requiring ivm-specific APIs in user code. The restriction (no circular references, no Maps/Sets, no undefined) is acceptable for LLM tool use cases where the data is always JSON-serializable by design. --- ## ADR-006: TypeScript-first ESM package **Status:** Accepted **Decision:** `"type": "module"` in `package.json`, `tsdown` for building, `--experimental-vm-modules` flag for Jest tests. **Alternatives considered:** - CommonJS output — would work but the rest of the monorepo is ESM. Mixing CJS and ESM packages in a pnpm workspace creates the dual-package hazard and complicates imports. - Single bundled file — considered but pnpm workspace cross-package imports work better with unbundled source during development. Bundling is applied at build time only. **Rationale:** ESM is the monorepo standard. `tsdown` produces `.mjs` output with `.d.mts` type declarations, which is compatible with the monorepo's `exports` field conventions. Jest requires `--experimental-vm-modules` to run ES modules natively, but this flag is stable enough for development and CI use. --- ## ADR-007: WASM modules injected via JS binding scripts **Status:** Accepted **Decision:** Load `.wasm` binary as an `ArrayBuffer` from the host filesystem, paired with a JS binding script. The binary and binding script are both injected into each isolate context at initialization. **Alternatives considered:** - Pre-compile WASM inside the isolate — the isolate has no filesystem access, so the binary must originate from the host regardless. Pre-compilation inside the isolate is not feasible. - Pass binary via `ivm.ExternalCopy` only — possible, but the JS binding script also needs to run inside the isolate. Combining binary transfer with script injection is cleaner than managing them separately. - Single bundled WASM-with-bindings — reduces flexibility; bindings and binary cannot be updated independently. Some WASM libraries (e.g., DuckDB-WASM) ship their own binding scripts that we do not control. **Rationale:** Binding scripts are small (~KB range) and compile quickly in V8. `ArrayBuffer` transfer of the binary is efficient. This approach maps directly to how DuckDB-WASM, GLPK.js, and similar libraries are typically consumed, reducing the gap between upstream documentation and our implementation. --- ## Open Questions The following questions are unresolved and will require decisions as the sandbox matures: 1. **WASM initialization cost** — DuckDB-WASM requires setup that takes 100–500ms per isolate context. Should warm pool isolates pre-initialize WASM and snapshot the state to amortize this cost across executions? 2. **Worker threads** — Should each `execute()` call run in its own `worker_thread` to prevent blocking the Node.js event loop during ivm synchronous calls? The current implementation runs isolates on the main thread. 3. **Tier 3/4 rate limiting** — The sandbox has no per-user or per-turn limits. These are expected to be enforced in the agent layer. Should the sandbox accept a `rateLimitCallback` in `ExecuteMessage` to integrate with Redis-backed quotas closer to the execution boundary? 4. **Result streaming** — Large results must fully serialize before being returned. Should we support a streaming result protocol that chunks large JSON responses? 5. **V8 version pinning** — When Node 25+ ships a breaking V8 ABI change, isolated-vm will need an update before we can upgrade. How is this managed in CI? Who is responsible for the upgrade path? 6. **WASM module versioning** — How are WASM binary updates deployed to production? A rolling restart is required to pick up new binaries. Is this acceptable, or do we need a hot-reload mechanism? --- ## Where This May Break in the Future The following are specific, known fragility points: 1. **isolated-vm + Node.js version** — isolated-vm is a native addon tied to the V8 ABI. Node 25+ may require an isolated-vm update before the addon builds. Risk: build failures in CI blocking deploys. Mitigation: pin `.nvmrc`/`.node-version`, and test isolated-vm compatibility before any Node upgrade. 2. **V8 isolate memory semantics** — `memoryLimit` in ivm is enforced via V8's equivalent of `--max-old-space-size`. If V8 changes GC behavior in a future version, OOM conditions may not trigger `SandboxOOMError` correctly. Monitor the ivm test suite for changes in memory enforcement behavior. 3. **ivm timeout async behavior** — If a future ivm version improves async code interruption (resolving issue #185), the external dispose timeout becomes redundant. It remains harmless but adds unnecessary complexity. Revisit when ivm async timeout behavior changes. 4. **JSON.parse attack via large strings** — `maxResultBytes` is checked on the serialized JSON string returned from the isolate, not on the parsed object. A user could return a large string that parses to a small value. This is intentional (we cannot inspect the parsed structure without allowing arbitrary code), but it may be surprising to future maintainers. 5. **Pool isolate memory mismatch** — If `defaultIsolateLimits` are changed in configuration without restarting the engine, the warm pool continues to hold isolates created with the old limits. These will be used for executions that expect the new limits. Mitigation: engine restart is required after configuration changes to `defaultIsolateLimits`. This should be documented in deployment runbooks. 6. **WASM binary size growth** — DuckDB-WASM grows with each release. As the binary grows, context initialization time increases, raising the baseline `wallMs` for all WASM-enabled executions. Monitor per-execution `wallMs` stats and alert if context init time exceeds acceptable thresholds. --- ## Future Features Worth Adding The following improvements have been identified but are out of scope for the initial implementation: 1. **Worker thread isolation** — Move each `execute()` call to a `worker_thread`. This prevents heavy isolate execution from blocking the Node.js event loop and enables true parallelism for concurrent executions. 2. **WASM initialization snapshots** — Take a V8 heap snapshot after WASM initialization is complete. Restore from snapshot for subsequent executions to amortize the 100–500ms WASM init cost across multiple runs. 3. **Result streaming** — Stream large execution results in chunks rather than buffering the full JSON string. This would reduce peak memory usage for data-heavy executions and improve time-to-first-byte for large results. 4. **Structured clone at boundary** — Support `Map`, `Set`, and `Date` values via ivm's structured clone support, removing the JSON-only restriction at the isolate boundary. 5. **Sandboxed import()** — Allow controlled dynamic imports from a pre-approved module allow-list, enabling user code to use trusted utility libraries without full host access. 6. **Per-user rate limiting** — Accept a `rateLimitCallback` in `ExecuteMessage` to integrate with Redis-backed per-user quotas at the execution boundary rather than requiring the agent layer to enforce this. 7. **Execution tracing** — Emit detailed execution traces (per-statement timing, memory snapshots at key points) to support debugging of LLM-generated code that behaves unexpectedly. 8. **Hot reload WASM modules** — Reload WASM binaries without a full engine restart. This would reduce deployment friction when updating DuckDB-WASM or other WASM dependencies.