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++;