Skip to content

Commit d33f3c2

Browse files
committed
refactor(secure-exec): remove legacy output buffers and sync specs
1 parent e8b02ad commit d33f3c2

29 files changed

Lines changed: 373 additions & 167 deletions

File tree

docs-internal/friction/secure-exec.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
1. **[resolved]** Default console capture buffered unbounded host memory.
66
- Symptom: runtime execution accumulated console output in host-managed `stdout`/`stderr` arrays by default, enabling memory amplification under high-volume logs.
77
- Fix: runtime now drops console output by default and exposes an explicit streaming hook (`onConsoleLog`) for host-controlled log handling.
8-
- Compatibility trade-off: `exec()`/`run()` no longer mirror Node stdout/stderr buffering by default; consumers that need console output must opt into hook-based streaming.
8+
- Compatibility trade-off: `exec()`/`run()` no longer mirror Node stdout/stderr buffering by default; result payloads no longer expose `stdout`/`stderr` fields, so consumers that need logs must opt into hook-based streaming.
9+
- Migration note: switch any `result.stderr` checks to `result.errorMessage` for runtime error assertions.
910
2. **[resolved]** Node module loading depended on allowlist projection setup and split filesystem paths.
1011
- Symptom: sandbox `node_modules` availability varied by `moduleAccess.allowPackages` setup and base filesystem mount location, which added resolver complexity and setup fragility.
1112
- Fix: Node driver now always composes a read-only `/app/node_modules` overlay from `<cwd>/node_modules`, even without a base filesystem adapter. Overlay reads are canonical-path scoped to `<cwd>/node_modules`; writes/mutations remain denied; `.node` native addons are rejected.

docs/node-compatability.mdx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ Unsupported modules use: `"<module> is not supported in sandbox"`.
5656
## Logging Behavior
5757

5858
- `console.log`/`warn`/`error` are supported and serialize arguments with circular-safe bounded formatting.
59-
- By default, secure-exec drops console emissions instead of buffering them into `exec()`/`run()` result `stdout`/`stderr`.
59+
- `exec()`/`run()` results do not expose buffered `stdout`/`stderr` fields.
60+
- By default, secure-exec drops console emissions instead of buffering runtime-managed output.
6061
- Consumers that need logs should use the explicit `onConsoleLog` hook to stream `stdout`/`stderr` events in emission order.
6162

6263
## Node-Modules Overlay Behavior

docs/quickstart.mdx

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,17 @@ description: Get secure-exec running from TypeScript in a few minutes.
1414
import { NodeProcess, createNodeDriver } from "secure-exec";
1515

1616
const proc = new NodeProcess({ driver: createNodeDriver({}) });
17-
const result = await proc.exec("console.log('hello from secure-exec')");
18-
console.log(result.stdout);
17+
const logs: string[] = [];
18+
const result = await proc.exec("console.log('hello from secure-exec')", {
19+
onConsoleLog: (event) => {
20+
logs.push(`[${event.channel}] ${event.message}`);
21+
},
22+
});
23+
if (result.errorMessage) {
24+
throw new Error(result.errorMessage);
25+
}
26+
console.log(logs.join("\n"));
1927
await proc.dispose();
2028
```
2129
</Step>
2230
</Steps>
23-

docs/security-model.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,13 @@ Setting `timingMitigation: "off"` gives you normal advancing clocks but weaker s
7373

7474
Two controls prevent runaway execution:
7575

76-
- **`cpuTimeLimitMs`** — CPU time budget for the runtime. When exceeded, the process exits with code `124` and stderr includes `CPU time limit exceeded`.
76+
- **`cpuTimeLimitMs`** — CPU time budget for the runtime. When exceeded, the process exits with code `124` and `errorMessage` includes `CPU time limit exceeded`.
7777
- **`memoryLimit`** — Isolate memory cap in MB (default `128`).
7878

7979
The bridge also enforces isolate-boundary payload limits so oversized base64 file transfers and oversized isolate-originated JSON payloads are rejected deterministically with `ERR_SANDBOX_PAYLOAD_TOO_LARGE` instead of exhausting host memory. Hosts can tune these limits within bounded safe ranges, but cannot disable enforcement.
8080

8181
## Logging Contract
8282

83-
- Console output is **not buffered** into runtime-managed `stdout`/`stderr` by default.
83+
- Console output is **not buffered** into runtime-managed execution result fields by default (`exec()`/`run()` do not expose `stdout`/`stderr` result fields).
8484
- To consume logs, configure the optional `onConsoleLog` streaming hook and forward events to your own sink.
8585
- This avoids implicit host-memory growth from untrusted high-volume logging.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
schema: spec-driven
2+
created: 2026-03-01
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
## Context
2+
3+
`secure-exec` runtime behavior was intentionally changed to drop console output by default and stream logs via `onConsoleLog`, but the public result types still expose legacy `stdout`/`stderr` fields. The repository also has stale artifacts tied to old behavior:
4+
5+
- `packages/secure-exec/tests/logging-load.test.ts` still expects multi-megabyte `result.stdout` buffering and currently fails.
6+
- `docs/quickstart.mdx` still demonstrates `console.log(result.stdout)`.
7+
- strict unused-symbol scan (`tsc --noEmit --noUnusedLocals --noUnusedParameters`) reports dead symbols in runtime/bridge modules.
8+
9+
This mismatch increases maintenance cost, obscures intended behavior, and leaves unnecessary memory-amplification paths.
10+
11+
## Goals / Non-Goals
12+
13+
**Goals:**
14+
- Remove legacy execution-result output capture fields from runtime APIs.
15+
- Keep output delivery hook-only and non-buffering by default across runtimes.
16+
- Delete confirmed dead symbols/properties/methods that no longer participate in runtime behavior.
17+
- Replace stale logging tests/docs with exploit-oriented no-buffer assertions.
18+
19+
**Non-Goals:**
20+
- Redesign child-process API semantics beyond removal of dead placeholders.
21+
- Introduce a new persistent logging backend or retention store.
22+
- Change project-matrix pass/fail fixture policy.
23+
24+
## Decisions
25+
26+
1. Remove `stdout`/`stderr` from `ExecResult` and `RunResult` (breaking change).
27+
- Rationale: current contract is misleading and encourages polling result buffers that should stay absent for safety.
28+
- Alternative considered: keep empty fields indefinitely for compatibility. Rejected because it preserves ambiguous API and stale usage patterns.
29+
30+
2. Preserve deterministic failure visibility with explicit non-buffered error metadata.
31+
- Rationale: callers still need failure reasons without reopening unbounded output capture.
32+
- Alternative considered: throw for all failures and remove result metadata. Rejected to avoid broad behavioral churn in existing `exec` flow.
33+
34+
3. Remove dead symbols found by strict compiler scan and manual bridge review.
35+
- Targets include unused flags in `NodeProcess`, dead helper/types in `bridge/fs.ts` and `bridge/module.ts`, and unused imports in permission wrappers.
36+
- Alternative considered: keep symbols as comments or `_` placeholders. Rejected because these are not compatibility contracts and add maintenance noise.
37+
38+
4. Replace legacy logging-load test with exploit-focused regression tests.
39+
- Rationale: testing should enforce that high-volume logs do not accumulate in runtime-managed buffers.
40+
- Alternative considered: delete stale tests without replacement. Rejected because we would lose regression coverage for a known resource-exhaustion vector.
41+
42+
## Risks / Trade-offs
43+
44+
- [Consumer breakage from result type changes] -> Mitigation: mark breaking in proposal/specs, update type tests, provide migration examples using `onConsoleLog`.
45+
- [Loss of convenience for callers that relied on `result.stderr`] -> Mitigation: provide explicit bounded error metadata in results and updated docs.
46+
- [Node/browser runtime behavior drift] -> Mitigation: align contracts in shared API types and add parity tests for logging defaults.
47+
- [Over-removal of symbols that are intentionally reserved] -> Mitigation: limit removals to symbols proven unused by compiler diagnostics or direct file-scope analysis.
48+
49+
## Migration Plan
50+
51+
1. Update shared result types and runtime return shapes.
52+
2. Refactor Node and browser execution pipelines to remove legacy `stdout`/`stderr` result plumbing.
53+
3. Update tests:
54+
- remove/replace stale buffering assertions
55+
- add exploit regression checks for high-volume logs
56+
- update type tests for new result contract
57+
4. Update docs (`quickstart`, `node-compatability`, `security-model`, friction notes).
58+
5. Run focused checks (`check-types`, targeted vitest files) before merge.
59+
60+
Rollback: revert result-type and runtime-shape commits as one unit; keep dead-code removals isolated in separate commits if split rollout is needed.
61+
62+
## Open Questions
63+
64+
- Should runtime failure metadata be `error?: { message: string; kind?: string }` or a flat `errorMessage?: string`?
65+
- Should browser worker expose the same `onConsoleLog` hook API in this change or in a follow-up?
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
## Why
2+
3+
Recent runtime changes made console output drop-by-default with optional streaming hooks, but several legacy output-capture surfaces and dead symbols remain. This creates confusing API/documentation contracts, stale tests, and unnecessary host-memory risk from leftover buffering paths.
4+
5+
## What Changes
6+
7+
- **BREAKING**: remove `stdout`/`stderr` fields from `ExecResult` and `RunResult`; expose only deterministic execution status (`code`, `exports` for `run`) plus explicit error metadata for runtime failures.
8+
- Keep console output delivery exclusively through `onConsoleLog` streaming hooks; no runtime-managed result buffering in Node or browser runtimes.
9+
- Remove confirmed dead/legacy symbols found by strict unused checks (unused fields, helpers, and type aliases) in runtime/bridge sources.
10+
- Replace stale log-buffering regression test with exploit-oriented tests that assert no accumulation under high-volume logging.
11+
- Update quickstart/security/compat docs to remove `result.stdout` examples and show hook-based log consumption.
12+
13+
## Capabilities
14+
15+
### New Capabilities
16+
- `execution-result-minimal-contract`: Define minimal non-buffered execution result shape and migration expectations.
17+
18+
### Modified Capabilities
19+
- `node-runtime`: Runtime result contract and logging behavior wording updated to remove legacy `stdout`/`stderr` result fields.
20+
- `documentation-site`: Quickstart/runtime examples updated to match hook-based logging contract.
21+
- `compatibility-governance`: Ensure required exploit-focused tests cover memory-amplification regressions for logging/output paths.
22+
23+
## Impact
24+
25+
- Affected code: `packages/secure-exec/src/shared/api-types.ts`, `packages/secure-exec/src/index.ts`, `packages/secure-exec/src/execution.ts`, browser runtime files, bridge cleanup sites (`bridge/fs.ts`, `bridge/module.ts`, `shared/permissions.ts`, `bridge/child-process.ts`), tests, docs.
26+
- API impact: Type-level and runtime response shape changes for `exec()`/`run()` consumers.
27+
- Governance impact: stronger regression coverage around memory/CPU amplification vectors tied to output handling.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
## MODIFIED Requirements
2+
3+
### Requirement: Logging Capture Contract Changes MUST Update Compatibility And Security Docs
4+
Any change that introduces or modifies runtime log-capture defaults or hook-based logging behavior MUST update compatibility/friction/security documentation in the same change and MUST include exploit-oriented regression tests for host resource amplification.
5+
6+
#### Scenario: Runtime switches default logging behavior
7+
- **WHEN** runtime logging defaults change (for example from buffered capture to log-drop)
8+
- **THEN** `docs-internal/friction/secure-exec.md` MUST document the compatibility impact and resource-exhaustion rationale in the same change
9+
10+
#### Scenario: Runtime introduces or changes log-stream hook behavior
11+
- **WHEN** runtime log-stream hook contract changes (event shape, ordering semantics, or failure behavior)
12+
- **THEN** `docs/security-model.mdx` MUST describe trust-boundary and resource-consumption implications and `docs/node-compatability.mdx` MUST reflect user-visible behavior changes where applicable
13+
14+
#### Scenario: Logging changes include exploit regression coverage
15+
- **WHEN** logging/output behavior is changed in runtime or bridge paths
16+
- **THEN** the same change MUST include tests that assert high-volume log emission does not create unbounded host-memory accumulation
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
## MODIFIED Requirements
2+
3+
### Requirement: Quickstart Uses Steps With Runnable Example
4+
The Quickstart page SHALL present onboarding steps using Mintlify `<Steps>` and SHALL include at least one basic runnable example that verifies setup success using the current runtime logging contract.
5+
6+
#### Scenario: Steps component structures onboarding
7+
- **WHEN** the Quickstart page is rendered
8+
- **THEN** the page MUST contain a `<Steps>` block with ordered setup actions
9+
10+
#### Scenario: Quickstart includes basic verification example
11+
- **WHEN** a user follows the Quickstart page
12+
- **THEN** the page MUST provide at least one concrete command example and expected successful outcome text
13+
14+
#### Scenario: Quickstart does not rely on legacy buffered output fields
15+
- **WHEN** Quickstart demonstrates how to read execution logs
16+
- **THEN** it MUST use hook-based log streaming examples and MUST NOT instruct users to read `result.stdout` or `result.stderr`
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
## ADDED Requirements
2+
3+
### Requirement: Execution Results MUST Exclude Buffered Output Fields
4+
The runtime SHALL return minimal execution results that do not include runtime-managed `stdout` or `stderr` capture fields.
5+
6+
#### Scenario: Exec result contains status without output buffers
7+
- **WHEN** `NodeProcess.exec()` completes in default logging mode
8+
- **THEN** the returned result MUST include execution status fields and MUST NOT include `stdout`/`stderr` properties
9+
10+
#### Scenario: Run result contains exports without output buffers
11+
- **WHEN** `NodeProcess.run()` completes and returns module exports
12+
- **THEN** the returned result MUST include `code` and `exports` semantics and MUST NOT include `stdout`/`stderr` properties
13+
14+
### Requirement: Failure Details MUST Be Exposed Without Log Buffer Retention
15+
The runtime SHALL expose deterministic failure metadata without retaining unbounded per-execution log buffers.
16+
17+
#### Scenario: Runtime error remains observable without stderr capture
18+
- **WHEN** execution fails due to runtime error
19+
- **THEN** callers MUST receive deterministic failure metadata sufficient for debugging without requiring buffered `stderr` output capture

0 commit comments

Comments
 (0)