From 31581b47edc447fa3d02692fdf0991db98847e9d Mon Sep 17 00:00:00 2001 From: David Antoon Date: Sun, 4 Oct 2026 03:00:12 +0300 Subject: [PATCH] fix(core): return Dates from the single VM as Dates 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 --- .../enclave.result-sanitization.spec.ts | 52 +++++++++++++++++-- libs/core/src/adapters/vm-adapter.ts | 4 +- libs/core/src/secure-proxy.ts | 16 ++++++ libs/core/src/value-sanitizer.ts | 28 ++++++---- 4 files changed, 84 insertions(+), 16 deletions(-) diff --git a/libs/core/src/__tests__/enclave.result-sanitization.spec.ts b/libs/core/src/__tests__/enclave.result-sanitization.spec.ts index ddaf693..8f826a9 100644 --- a/libs/core/src/__tests__/enclave.result-sanitization.spec.ts +++ b/libs/core/src/__tests__/enclave.result-sanitization.spec.ts @@ -3,13 +3,16 @@ * * The sanitizer used to recognize Dates, Maps, Sets and RegExps with `instanceof`, which is false * for values built in the sandbox realm, so a script's `new Date(0)` came back as `{}`. It now - * checks the builtin's internal slot, as `@enclave-vm/browser` does inside its iframe. (The single - * VM still returns `{}`: its membrane hands the sanitizer a proxy, which the check refuses.) + * checks the builtin's internal slot, as `@enclave-vm/browser` does inside its iframe. The single + * VM's membrane wraps the script's Dates in a proxy, which has no slot, so that adapter hands the + * sanitizer `unwrapSecureProxy` to reach the Date behind it. */ import * as vm from 'vm'; import { Enclave } from '../enclave'; +import { createSecureProxy, unwrapSecureProxy } from '../secure-proxy'; import { sanitizeValue } from '../value-sanitizer'; +import type { CreateEnclaveOptions } from '../types'; describe('sanitizeValue with values from another realm', () => { const foreign = vm.runInNewContext(`({ @@ -52,11 +55,47 @@ describe('sanitizeValue with values from another realm', () => { }); }); -describe('script results on the double VM', () => { +describe('unwrapSecureProxy', () => { + it('returns the object a secure proxy wraps', () => { + const date = new Date(0); + + expect(unwrapSecureProxy(createSecureProxy(date))).toBe(date); + }); + + it('returns any other value unchanged', () => { + const plain = { a: 1 }; + const foreignProxy = new Proxy(new Date(0), {}); + + expect(unwrapSecureProxy(plain)).toBe(plain); + expect(unwrapSecureProxy(foreignProxy)).toBe(foreignProxy); + expect(unwrapSecureProxy(null)).toBeNull(); + expect(unwrapSecureProxy(1)).toBe(1); + }); + + it('lets sanitizeValue read a wrapped Date', () => { + const wrapped = { date: createSecureProxy(new Date(0)) }; + + expect(sanitizeValue(wrapped)).toEqual({ date: {} }); + expect(sanitizeValue(wrapped, { unwrap: unwrapSecureProxy })).toEqual({ date: new Date(0) }); + }); +}); + +describe.each<[string, CreateEnclaveOptions]>([ + ['double VM', {}], + ['single VM', { doubleVm: { enabled: false } }], +])('script results on the %s', (_name, adapterOptions) => { let enclave: Enclave; + beforeAll(() => { + jest.spyOn(console, 'warn').mockImplementation(() => undefined); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + beforeEach(() => { - enclave = new Enclave({ timeout: 5000 }); + enclave = new Enclave({ timeout: 5000, ...adapterOptions }); }); afterEach(() => { @@ -64,11 +103,14 @@ describe('script results on the double VM', () => { }); it('returns a Date as a Date', async () => { - const result = await enclave.run<{ date: Date }>('return { date: new Date(0) };'); + const result = await enclave.run<{ date: Date; nested: Date[] }>( + 'return { date: new Date(0), nested: [new Date(1)] };', + ); expect(result.error).toBeUndefined(); expect(result.value?.date).toBeInstanceOf(Date); expect(result.value?.date.getTime()).toBe(0); + expect(result.value?.nested[0].getTime()).toBe(1); }); it('keeps the other conversions', async () => { diff --git a/libs/core/src/adapters/vm-adapter.ts b/libs/core/src/adapters/vm-adapter.ts index 5c3022b..73040bf 100644 --- a/libs/core/src/adapters/vm-adapter.ts +++ b/libs/core/src/adapters/vm-adapter.ts @@ -10,7 +10,7 @@ import * as vm from 'vm'; import type { SandboxAdapter, ExecutionContext, ExecutionResult, SecurityLevel } from '../types'; import { createSafeRuntime } from '../safe-runtime'; -import { createSafeReflect, createSecureProxy } from '../secure-proxy'; +import { createSafeReflect, createSecureProxy, unwrapSecureProxy } from '../secure-proxy'; import { createSafeError } from '../safe-error'; import { MemoryTracker, MemoryLimitError } from '../memory-tracker'; import { createHostToolBridge } from '../tool-bridge'; @@ -1186,6 +1186,8 @@ export class VmAdapter implements SandboxAdapter { maxProperties: clampedProperties, allowDates: true, allowErrors: true, + // The membrane wraps the script's Dates; read them through their targets. + unwrap: unwrapSecureProxy, }); return { diff --git a/libs/core/src/secure-proxy.ts b/libs/core/src/secure-proxy.ts index 9bf2e15..2e59d34 100644 --- a/libs/core/src/secure-proxy.ts +++ b/libs/core/src/secure-proxy.ts @@ -1028,6 +1028,22 @@ export function isSecureProxy(obj: unknown): boolean { return proxySet.has(obj as object); } +/** + * The object a secure proxy wraps, or `value` itself when it is not one. + * + * For host code that must read a sandbox value's internal slots, which a proxy does not carry + * (a proxied Date fails `Date.prototype.getTime`). Never hand the result back to the sandbox. + */ +export function unwrapSecureProxy(value: T): T { + let current: unknown = value; + while (current !== null && (typeof current === 'object' || typeof current === 'function')) { + const target = proxyToTarget.get(current as object); + if (target === undefined) break; + current = target; + } + return current as T; +} + /** * Clear the proxy cache * diff --git a/libs/core/src/value-sanitizer.ts b/libs/core/src/value-sanitizer.ts index 0d1e971..a762a8c 100644 --- a/libs/core/src/value-sanitizer.ts +++ b/libs/core/src/value-sanitizer.ts @@ -38,6 +38,13 @@ export interface SanitizeOptions { * @default true */ allowErrors?: boolean; + + /** + * Returns the object to check for a Date, Map, Set or RegExp in place of a wrapper around it + * (the single VM's secure proxy, which has no internal slots). Only builtins are read from the + * unwrapped object; everything else is still read through `value`. + */ + unwrap?: (value: object) => object; } /** @@ -270,19 +277,20 @@ export function sanitizeValue( return result; } - const isPlain = isPlainObject(value as object); + const inspected = options.unwrap ? options.unwrap(value as object) : (value as object); + const isPlain = isPlainObject(inspected); // Handle Date objects, including the sandbox realm's. // SECURITY: read the timestamp via the captured Date.prototype.getTime (Reflect.apply), // never `value.getTime()`, so the value cannot supply its own method. - if (!isPlain && hasBrand(DateProtoGetTime, value as object)) { - const time = ReflectApply(DateProtoGetTime, value, []) as number; + if (!isPlain && hasBrand(DateProtoGetTime, inspected)) { + const time = ReflectApply(DateProtoGetTime, inspected, []) as number; if (allowDates) { // Return a new Date to prevent reference sharing return new Date(time); } // Convert to ISO string if dates not allowed - return ReflectApply(DateProtoToISOString, value, []) as string; + return ReflectApply(DateProtoToISOString, inspected, []) as string; } // Handle Error objects @@ -305,17 +313,17 @@ export function sanitizeValue( } // Handle RegExp objects (convert to their /source/flags string) - if (!isPlain && hasBrand(RegExpProtoSourceGetter, value as object)) { - return regExpToString(value as object); + if (!isPlain && hasBrand(RegExpProtoSourceGetter, inspected)) { + return regExpToString(inspected); } // Handle Map objects, including the sandbox realm's. // SECURITY: enumerate with the captured Map.prototype.entries (Reflect.apply) rather than // `value.entries()`, so the value cannot hijack the iteration. The returned iterator is a // genuine host Map Iterator, safe to `for..of`. - if (!isPlain && hasBrand(MapProtoSizeGetter, value as object)) { + if (!isPlain && hasBrand(MapProtoSizeGetter, inspected)) { const sanitizedMap: Record = ObjectCreate(null); - const entries = ReflectApply(MapProtoEntries, value, []) as IterableIterator<[unknown, unknown]>; + const entries = ReflectApply(MapProtoEntries, inspected, []) as IterableIterator<[unknown, unknown]>; for (const [key, val] of entries) { if (typeof key !== 'string') continue; // Only string keys if (DANGEROUS_KEYS.has(key)) continue; // Skip dangerous keys @@ -328,8 +336,8 @@ export function sanitizeValue( // Handle Set objects (convert to array), including the sandbox realm's. // SECURITY: enumerate with the captured Set.prototype.values (Reflect.apply), not // `Array.from(value)` / the value's own iterator, so the value cannot hijack it. - if (!isPlain && hasBrand(SetProtoSizeGetter, value as object)) { - const iter = ReflectApply(SetProtoValues, value, []) as IterableIterator; + if (!isPlain && hasBrand(SetProtoSizeGetter, inspected)) { + const iter = ReflectApply(SetProtoValues, inspected, []) as IterableIterator; const result: unknown[] = []; for (const item of iter) { context.propCount.count++;