fix(browser): sanitize script results like @enclave-vm/core by frontegg-david · Pull Request #94 · agentfront/enclave

frontegg-david · 2026-10-03T23:42:06Z

Stack 3/10: main ← #92 ← #93 ← this ← #95 ← #96 ← #98 ← #99 ← #100 ← #101 ← #102

Fixes #89.

Validation

Reproduced, plus one thing the issue's table hides.

Browser (libs/browser/src/adapters/inner-iframe-bootstrap.ts) returned JSON.parse(JSON.stringify(result)), and catch { safeResult = undefined }:
- a circular result was success: true with value: undefined;
- a Date became an ISO string;
- __proto__ keys survived;
- maxSanitizeDepth / maxSanitizeProperties were put in the config but never read.
Core returned { date: {} } for new Date(0). That's not intended conversion. sanitizeValue (libs/core/src/value-sanitizer.ts) means to keep Dates (allowDates: true), but it checks value instanceof Date against the host's Date. That's false for a Date built in the sandbox realm, so the value fell through to the plain-object branch. Maps, Sets and RegExps from scripts were affected the same way.

Fix

Browser: the inner iframe copies the result with a port of core's sanitizeValue:

same depth and property limits;
'[Circular]' for an object seen twice;
__proto__ / constructor keys dropped;
Date kept;
BigInt → string, Map → object, Set → array, RegExp → string, Error → { name, message }.

A function, a symbol or a limit overrun fails the run. The copy is posted as structured data, so NaN, Infinity and undefined arrive as they do in core. New maxSanitizeDepth / maxSanitizeProperties options override the level's limits, as in core.

Core: sanitizeValue recognizes Dates, Maps, Sets and RegExps by internal slot. It calls the captured builtin (Date.prototype.getTime, the size / source getters) through Reflect.apply, which works across realms and reads no property of the value. A Proxy has no slot, so it's copied as a plain object without its traps being called (before, it threw). Values whose prototype is null or an Object.prototype skip these checks, because each check throws on a miss. With that fast path, sanitizing 1,000 plain objects takes 0.36 ms; without it, 14 ms; on main, 0.77 ms.

Behavior changes

Script returns	core before	core after	browser before	browser after
`{ date: new Date(0), nan: NaN, inf: Infinity, undef: undefined }`	`{ date: {}, nan: NaN, inf: Infinity, undef: undefined }`	`{ date: Date(0), nan: NaN, inf: Infinity, undef: undefined }`	`{ date: "1970-…Z", nan: null, inf: null }`	same as core
circular `a.self = a`	`{ name: 'a', self: '[Circular]' }`	same	`success: true`, `value: undefined`	same as core
> `maxSanitizeProperties` / deeper than `maxSanitizeDepth`	fails	fails	succeeds	fails (`Script result exceeds maximum …`)
a function inside the result	fails	fails	key dropped	fails

Also in core, tool arguments or results holding a sandbox Date, Map, Set or RegExp now convert as documented instead of becoming {}.

Not changed: core's single VM (doubleVm: { enabled: false }) still returns {} for a Date. Its membrane hands the sanitizer a proxy, which the slot check refuses. Fixed on top of this stack in #98.

Red → green

Suite	Before	After
`libs/browser/e2e/results.spec.ts` (new, 7 tests)	7 failed	7 passed
`libs/core/src/__tests__/enclave.result-sanitization.spec.ts` (new, 4 tests)	3 failed (guard: other conversions unchanged)	4 passed

Review follow-ups (`08a240a`)

Inner sanitizer used WeakSet has/add and the Map/Set iterators' next() as they were after the script ran (Copilot). They're now captured at bootstrap and called through Reflect.apply. Test: a validate: false script that replaces them before returning gets the right copy (fails before).
Core RegExp.prototype.toString read source/flags off the value (Copilot), so a sandbox RegExp's own accessors ran on the host. /source/flags is now built from the captured source getter and the per-flag getters, which read the internal slots. Test: a foreign RegExp with its own source/flags accessors, which are not called (fails before). The browser builds the string the same way (there a script can't define accessors anyway: Object.defineProperty is blocked in the iframe).

Verification

core: jest -c libs/core/jest.config.ts --testPathIgnorePatterns /perf/: 63 suites, 2341 passed, 1 skipped
browser: nx run browser:test-e2e: 79 passed
lint, build, prettier: pass

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Browser sandbox results now support safe handling of dates, regular expressions, maps, sets, errors, and circular references.
- Results containing functions or symbols, or exceeding configured depth or property limits, now fail the run instead of silently returning an undefined result. Limits can be configured, with defaults based on the security level.
- Prototype-sensitive keys are excluded from returned results.
Documentation
- Added guidance on result sanitization, supported value conversions, and configurable limits.

coderabbitai · 2026-10-03T23:42:13Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: agentfront/enclave/.coderabbit.yaml
Review profile: CHILL
Plan: Essentials
Run ID: 80086ffd-32ce-4e71-bedd-cdade1838e25

📥 Commits

Reviewing files that changed from the base of the PR and between 465fc21 and d59ec9a.

📒 Files selected for processing (9)

docs/enclave/core-libraries/enclave-browser/configuration.mdx
docs/enclave/core-libraries/enclave-browser/overview.mdx
libs/browser/e2e/helpers.ts
libs/browser/e2e/results.spec.ts
libs/browser/src/adapters/inner-iframe-bootstrap.ts
libs/browser/src/browser-enclave.ts
libs/browser/src/types.ts
libs/core/src/__tests__/enclave.result-sanitization.spec.ts
libs/core/src/value-sanitizer.ts

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.

📝 Walkthrough

Walkthrough

Browser script results now use configurable sanitization limits and conversions before delivery. Core sanitization recognizes supported built-in values across realms.

Changes

Script result sanitization

Layer / File(s)	Summary
Browser result sanitization and limits `libs/browser/src/types.ts`, `libs/browser/e2e/helpers.ts`, `libs/browser/src/browser-enclave.ts`, `libs/browser/src/adapters/inner-iframe-bootstrap.ts`, `libs/browser/e2e/results.spec.ts`, `docs/enclave/core-libraries/enclave-browser/configuration.mdx`, `docs/enclave/core-libraries/enclave-browser/overview.mdx`	Browser options now expose depth and property limits. The iframe sanitizes successful script results and reports sanitization failures through the execution error path. End-to-end tests and documentation cover conversions, dropped keys, circular references, failures, and configurable limits.
Cross-realm built-in recognition `libs/core/src/value-sanitizer.ts`, `libs/core/src/__tests__/enclave.result-sanitization.spec.ts`	Core sanitization uses captured built-in checks to recognize cross-realm Dates, RegExps, Maps, and Sets. Tests cover those values and a Proxy over a Date.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant BrowserEnclave
  participant InnerIframe
  participant EnclaveCaller
  BrowserEnclave->>InnerIframe: Pass sanitization limits in generated config
  InnerIframe->>InnerIframe: Sanitize script result
  InnerIframe->>EnclaveCaller: Return sanitized result or execution error

Merge Risk: ⚪ Minimal · up to d59ec

The result-sanitization changes appear mergeable after normal checks; no concrete remaining risk was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 7 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Issue `#89` requires browser script results to follow core sanitization limits and conversions, mark repeated references as `[Circular]`, and fail when a result cannot be delivered. `inner-iframe-bootst…
Out of Scope Changes check	✅ Passed	The core sanitizer changes address `#89`'s requirement for consistent browser and core results. The tests and documentation cover the new behavior and limits. No unrelated change is identified in the av…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: browser script results now use the core sanitizer.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 7 files. (2 skipped: 2 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Copilot review overview

🟡 Changes recommended

The same-realm browser sanitizer can invoke script-mutated intrinsics, and RegExp conversion can execute untrusted accessors.

Review effort: Balanced
Findings: 2

Open (2)

Capture WeakSet methods before executing untrusted scripts · New
Avoid RegExp property accessors during stringification · New

What changed in this PR

Aligns browser result sanitization with core while adding cross-realm built-in recognition.

Changes:

Adds browser-side result sanitization and configurable limits.
Recognizes cross-realm Date, Map, Set, and RegExp values in core.
Adds regression tests and documentation.

File	Description
`libs/core/src/value-sanitizer.ts`	Adds cross-realm built-in detection.
`libs/core/src/__tests__/enclave.result-sanitization.spec.ts`	Tests cross-realm sanitization.
`libs/browser/src/types.ts`	Exposes sanitization options.
`libs/browser/src/browser-enclave.ts`	Applies option overrides.
`libs/browser/src/adapters/inner-iframe-bootstrap.ts`	Sanitizes iframe results.
`libs/browser/e2e/results.spec.ts`	Tests browser result behavior.
`libs/browser/e2e/helpers.ts`	Supports new test options.
`docs/enclave/core-libraries/enclave-browser/overview.mdx`	Documents result conversions.
`docs/enclave/core-libraries/enclave-browser/configuration.mdx`	Documents sanitization limits.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

BrowserEnclave returned script results through JSON.parse(JSON.stringify(result)) and dropped anything that failed: a Date became an ISO string, a circular result was a successful undefined, __proto__ keys survived, and maxSanitizeDepth and maxSanitizeProperties were read into the config but never used. The inner iframe now copies the result with a port of core's sanitizeValue: same depth and property limits, '[Circular]' for an object seen twice, __proto__ and constructor keys dropped, Date kept, BigInt to string, Map to object, Set to array, RegExp to string, Error to { name, message }. A function, a symbol or a limit overrun fails the run. The value is posted as structured data, so NaN, Infinity and undefined arrive as in core. New maxSanitizeDepth and maxSanitizeProperties options override the security level's limits, as in core. Core recognized Dates, Maps, Sets and RegExps with instanceof, which is false for values built in the sandbox realm, so a script's Date came back from the double VM as {}. It now checks the builtin's internal slot with the captured builtins (no property of the value is read), so both packages return a Date. Values whose prototype is null or an Object.prototype skip those checks (each throws on a miss), which keeps sanitizing 1,000 plain objects at 0.36 ms instead of 14 ms (0.77 ms before this change). The single VM still returns {} for a Date: its membrane hands the sanitizer a proxy, which the check refuses. Fixes #89 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Review follow-up. Browser: the inner iframe's result sanitizer called WeakSet has/add and the Map and Set iterators' next() as they were after the script ran, and those prototypes are not frozen, so a script could make its result look circular or hide Map entries. It now captures them at bootstrap and calls them through Reflect.apply, like its other intrinsics. Core: RegExp.prototype.toString is generic and reads source and flags off the value, so a RegExp from the sandbox realm with its own accessors for them ran those accessors on the host. The sanitizer now builds /source/flags from the captured source getter and the per-flag getters, which read the internal slot. The browser does the same. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

With the double VM off, a script's `new Date(0)` still came back as {}. The single VM's membrane wraps every Date a script constructs in a secure proxy, and a proxy has no [[DateValue]] slot, so the sanitizer's builtin check (#94) refused it and copied it as a plain object. The membrane now exports unwrapSecureProxy (from its existing proxy-to-target map; not part of the package API), and sanitizeValue takes an unwrap option. The single VM passes it for the script result: the sanitizer recognizes and reads builtins (Date, Map, Set, RegExp) from the unwrapped target with the captured host builtins and returns a new host Date. Everything else is still read through the proxy, and nothing unwrapped is handed back to the sandbox. Tool arguments were not affected (Dates already arrive as ISO strings, or as Dates with the direct bridge). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

With the double VM off, a script's `new Date(0)` still came back as {}. The single VM's membrane wraps every Date a script constructs in a secure proxy, and a proxy has no [[DateValue]] slot, so the sanitizer's builtin check (#94) refused it and copied it as a plain object. The membrane now exports unwrapSecureProxy (from its existing proxy-to-target map; not part of the package API), and sanitizeValue takes an unwrap option. The single VM passes it for the script result: the sanitizer recognizes and reads builtins (Date, Map, Set, RegExp) from the unwrapped target with the captured host builtins and returns a new host Date. Everything else is still read through the proxy, and nothing unwrapped is handed back to the sandbox. Tool arguments were not affected (Dates already arrive as ISO strings, or as Dates with the direct bridge). Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

This was referenced Oct 3, 2026

fix(browser): apply core's timeout rules to tool calls #95

Merged

feat(core): accept a host-supplied sandboxAdapter #96

Merged

fix(ast): count UTF-8 bytes without Node's Buffer #92

Merged

fix(browser): reject function-valued globals at construction #93

Merged

frontegg-david added this pull request to stack #97 October 3, 2026 23:55

frontegg-david requested a balanced review from Copilot October 3, 2026 23:56

frontegg-david force-pushed the fix/browser-sanitize-results branch from 07635d9 to d59ec9a Compare October 3, 2026 23:56

Copilot started reviewing on behalf of frontegg-david October 3, 2026 23:56

View session

Copilot AI reviewed Oct 3, 2026

View reviewed changes

Comment thread libs/browser/src/adapters/inner-iframe-bootstrap.ts Outdated

Comment thread libs/core/src/value-sanitizer.ts Outdated

This was referenced Oct 4, 2026

fix(core): return Dates from the single VM as Dates #98

Merged

ci: run the browser Playwright suite #99

Merged

alexmercerpo approved these changes Oct 4, 2026

View reviewed changes

frontegg-david and others added 2 commits October 4, 2026 03:16

frontegg-david force-pushed the fix/browser-sanitize-results branch from d59ec9a to 08a240a Compare October 4, 2026 00:19

This was referenced Oct 4, 2026

fix(core): convert worker results like the other adapters #100

Merged

chore(deps): update Nx to 22.7.12 #101

Merged

chore(deps): update vulnerable transitive dependencies #102

Merged

frontegg-david merged commit e024bb0 into main Oct 4, 2026
8 checks passed

frontegg-david deleted the fix/browser-sanitize-results branch October 4, 2026 10:55

Navigation Menu

Repository navigation

fix(browser): sanitize script results like @enclave-vm/core - #94

fix(browser): sanitize script results like @enclave-vm/core#94
frontegg-david merged 2 commits into
fix/browser-reject-function-globalsfrom
fix/browser-sanitize-results

frontegg-david commented Oct 3, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 3, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

frontegg-david commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Validation

Fix

Behavior changes

Red → green

Review follow-ups (08a240a)

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

frontegg-david commented Oct 3, 2026 •

edited

Loading

Review follow-ups (`08a240a`)

coderabbitai Bot commented Oct 3, 2026 •

edited

Loading