From 639de2a68f38970a02a80a18cf50b972ec96e5ad Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Tue, 16 Jan 2024 08:46:42 -0800 Subject: [PATCH 1/3] fix(ses): Add `@@toStringTag` property to `proxiedExports` Adds a `Symbol.toStringTag` property to the `proxiedExports` object in order to make it appear like an ESM Module Namespace object. During testing, the output of `console.log()` is indistinguishable from an actual ESM Module Namespace object in Chromium. In Node, however, we would need to outsmart `node:util.inspect()` by implementing the `@@node:util.inspect.custom` method on our object. (The call chain for logging non-strings in Node is: `console.log()` -> `node:util.format()` -> `node:util.inspect()`.) --- packages/ses/src/module-proxy.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/ses/src/module-proxy.js b/packages/ses/src/module-proxy.js index 4b34927011..787528d5ce 100644 --- a/packages/ses/src/module-proxy.js +++ b/packages/ses/src/module-proxy.js @@ -25,6 +25,7 @@ import { reflectHas, reflectIsExtensible, reflectPreventExtensions, + toStringTagSymbol, weakmapSet, } from './commons.js'; import { assert } from './error/assert.js'; @@ -51,7 +52,15 @@ const { quote: q } = assert; // export const deferExports = () => { let active = false; - const proxiedExports = create(null); + const proxiedExports = create(null, { + // Make this appear like an ESM module namespace object. + [toStringTagSymbol]: { + value: 'Module', + writable: false, + enumerable: false, + configurable: false, + }, + }); return freeze({ activate() { active = true; From f6f851021bd2c00efde89194a39a43c9fd1ad2f5 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Tue, 16 Jan 2024 09:31:30 -0800 Subject: [PATCH 2/3] refactor(docs): Rename `proxiedExports` to `exportsTarget` Renames deferred exports `proxiedExports` property to `exportsTarget` and adds a docstring to `getDeferredExports` to explain the purpose of the `exportsProxy`, `exportsTarget`, and `activate` function. --- packages/ses/src/module-instance.js | 20 +++++------ packages/ses/src/module-proxy.js | 47 ++++++++++++++++++-------- packages/ses/test/test-module-proxy.js | 20 +++++------ packages/ses/types.d.ts | 2 +- 4 files changed, 54 insertions(+), 35 deletions(-) diff --git a/packages/ses/src/module-instance.js b/packages/ses/src/module-instance.js index 37a6a1a90e..a4bbdd8b81 100644 --- a/packages/ses/src/module-instance.js +++ b/packages/ses/src/module-instance.js @@ -32,7 +32,7 @@ export const makeThirdPartyModuleInstance = ( moduleSpecifier, resolvedImports, ) => { - const { exportsProxy, proxiedExports, activate } = getDeferredExports( + const { exportsProxy, exportsTarget, activate } = getDeferredExports( compartment, weakmapGet(compartmentPrivateFields, compartment), moduleAliases, @@ -51,7 +51,7 @@ export const makeThirdPartyModuleInstance = ( ); } arrayForEach(staticModuleRecord.exports, name => { - let value = proxiedExports[name]; + let value = exportsTarget[name]; const updaters = []; const get = () => value; @@ -63,7 +63,7 @@ export const makeThirdPartyModuleInstance = ( } }; - defineProperty(proxiedExports, name, { + defineProperty(exportsTarget, name, { get, set, enumerable: true, @@ -75,9 +75,9 @@ export const makeThirdPartyModuleInstance = ( update(value); }; }); - // This is enough to support import * from cjs - the '*' field doesn't need to be in exports nor proxiedExports because import will only ever access it via notifiers + // This is enough to support import * from cjs - the '*' field doesn't need to be in exports nor exportsTarget because import will only ever access it via notifiers notifiers['*'] = update => { - update(proxiedExports); + update(exportsTarget); }; } @@ -97,7 +97,7 @@ export const makeThirdPartyModuleInstance = ( try { // eslint-disable-next-line @endo/no-polymorphic-call staticModuleRecord.execute( - proxiedExports, + exportsTarget, compartment, resolvedImports, ); @@ -143,7 +143,7 @@ export const makeModuleInstance = ( const { __shimTransforms__, importMetaHook } = compartmentFields; - const { exportsProxy, proxiedExports, activate } = getDeferredExports( + const { exportsProxy, exportsTarget, activate } = getDeferredExports( compartment, compartmentFields, moduleAliases, @@ -342,7 +342,7 @@ export const makeModuleInstance = ( ); const notifyStar = update => { - update(proxiedExports); + update(exportsTarget); }; notifiers['*'] = notifyStar; @@ -435,10 +435,10 @@ export const makeModuleInstance = ( // and the string must correspond to a valid identifier, sorting these // properties works for this specific case. arrayForEach(arraySort(keys(exportsProps)), k => - defineProperty(proxiedExports, k, exportsProps[k]), + defineProperty(exportsTarget, k, exportsProps[k]), ); - freeze(proxiedExports); + freeze(exportsTarget); activate(); } diff --git a/packages/ses/src/module-proxy.js b/packages/ses/src/module-proxy.js index 787528d5ce..ec85f1b8d1 100644 --- a/packages/ses/src/module-proxy.js +++ b/packages/ses/src/module-proxy.js @@ -52,7 +52,7 @@ const { quote: q } = assert; // export const deferExports = () => { let active = false; - const proxiedExports = create(null, { + const exportsTarget = create(null, { // Make this appear like an ESM module namespace object. [toStringTagSymbol]: { value: 'Module', @@ -65,8 +65,8 @@ export const deferExports = () => { activate() { active = true; }, - proxiedExports, - exportsProxy: new Proxy(proxiedExports, { + exportsTarget, + exportsProxy: new Proxy(exportsTarget, { get(_target, name, receiver) { if (!active) { throw TypeError( @@ -75,7 +75,7 @@ export const deferExports = () => { )} of module exports namespace, the module has not yet begun to execute`, ); } - return reflectGet(proxiedExports, name, receiver); + return reflectGet(exportsTarget, name, receiver); }, set(_target, name, _value) { throw TypeError( @@ -90,7 +90,7 @@ export const deferExports = () => { )}, the module has not yet begun to execute`, ); } - return reflectHas(proxiedExports, name); + return reflectHas(exportsTarget, name); }, deleteProperty(_target, name) { throw TypeError( @@ -103,7 +103,7 @@ export const deferExports = () => { 'Cannot enumerate keys, the module has not yet begun to execute', ); } - return ownKeys(proxiedExports); + return ownKeys(exportsTarget); }, getOwnPropertyDescriptor(_target, name) { if (!active) { @@ -113,7 +113,7 @@ export const deferExports = () => { )}, the module has not yet begun to execute`, ); } - return reflectGetOwnPropertyDescriptor(proxiedExports, name); + return reflectGetOwnPropertyDescriptor(exportsTarget, name); }, preventExtensions(_target) { if (!active) { @@ -121,7 +121,7 @@ export const deferExports = () => { 'Cannot prevent extensions of module exports namespace, the module has not yet begun to execute', ); } - return reflectPreventExtensions(proxiedExports); + return reflectPreventExtensions(exportsTarget); }, isExtensible() { if (!active) { @@ -129,7 +129,7 @@ export const deferExports = () => { 'Cannot check extensibility of module exports namespace, the module has not yet begun to execute', ); } - return reflectIsExtensible(proxiedExports); + return reflectIsExtensible(exportsTarget); }, getPrototypeOf(_target) { return null; @@ -156,11 +156,30 @@ export const deferExports = () => { }); }; -// `getDeferredExports` memoizes the creation of a deferred module exports -// namespace proxy for any abritrary full specifier in a compartment. -// It also records the compartment and specifier affiliated with that module -// exports namespace proxy so it can be used as an alias into another -// compartment when threaded through a compartment's `moduleMap` argument. +/** + * @typedef {object} DeferredExports + * @property {Record} exportsTarget - The object to which a + * module's exports will be added. + * @property {Record} exportsProxy - A proxy over the `exportsTarget`, + * used to expose its "exports" to other compartments. + * @property {() => void} activate - Activate the `exportsProxy` such that it can + * be used as a module namespace object. + */ + +/** + * Memoizes the creation of a deferred module exports namespace proxy for any + * arbitrary full specifier in a compartment. It also records the compartment + * and specifier affiliated with that module exports namespace proxy so it + * can be used as an alias into another compartment when threaded through + * a compartment's `moduleMap` argument. + * + * @param {*} compartment - The compartment to retrieve deferred exports from. + * @param {*} compartmentPrivateFields - The private fields of the compartment. + * @param {*} moduleAliases - The module aliases of the compartment. + * @param {string} specifier - The module specifier to retrieve deferred exports for. + * @returns {DeferredExports} - The deferred exports for the module specifier of + * the compartment. + */ export const getDeferredExports = ( compartment, compartmentPrivateFields, diff --git a/packages/ses/test/test-module-proxy.js b/packages/ses/test/test-module-proxy.js index 76e4f001f6..f0f10bb781 100644 --- a/packages/ses/test/test-module-proxy.js +++ b/packages/ses/test/test-module-proxy.js @@ -4,7 +4,7 @@ import { deferExports } from '../src/module-proxy.js'; test('proxied exports keys are readable', t => { t.plan(2); - const { proxiedExports, exportsProxy, activate } = deferExports(); + const { exportsTarget, exportsProxy, activate } = deferExports(); t.throws( () => { keys(exportsProxy); @@ -12,16 +12,16 @@ test('proxied exports keys are readable', t => { { message: /^Cannot enumerate keys/ }, 'keys fails for inactive module', ); - proxiedExports.a = 10; - proxiedExports.b = 20; + exportsTarget.a = 10; + exportsTarget.b = 20; activate(); t.deepEqual(keys(exportsProxy), ['a', 'b']); }); test('proxied exports is not extensible', t => { t.plan(1); - const { proxiedExports, exportsProxy, activate } = deferExports(); - seal(proxiedExports); + const { exportsTarget, exportsProxy, activate } = deferExports(); + seal(exportsTarget); activate(); t.truthy( !isExtensible(exportsProxy), @@ -31,7 +31,7 @@ test('proxied exports is not extensible', t => { test('proxied exports has own keys', t => { t.plan(3); - const { proxiedExports, exportsProxy, activate } = deferExports(); + const { exportsTarget, exportsProxy, activate } = deferExports(); t.throws( () => { 'irrelevant' in exportsProxy; @@ -39,7 +39,7 @@ test('proxied exports has own keys', t => { { message: /^Cannot check property/ }, 'module must throw error for owns trap before it begins executing', ); - proxiedExports.present = 'here'; + exportsTarget.present = 'here'; activate(seal()); t.truthy('present' in exportsProxy, 'module has key'); t.truthy(!('absent' in exportsProxy), 'module does not have key'); @@ -47,7 +47,7 @@ test('proxied exports has own keys', t => { test('proxied exports set/get round-trip', t => { t.plan(3); - const { proxiedExports, exportsProxy, activate } = deferExports(); + const { exportsTarget, exportsProxy, activate } = deferExports(); t.throws( () => { exportsProxy.ceciNEstPasUnePipe; @@ -63,8 +63,8 @@ test('proxied exports set/get round-trip', t => { 'properties must not be mutable', ); - proxiedExports.ceciNEstPasUnePipe = 'pipe'; - seal(proxiedExports); + exportsTarget.ceciNEstPasUnePipe = 'pipe'; + seal(exportsTarget); activate(); t.throws( diff --git a/packages/ses/types.d.ts b/packages/ses/types.d.ts index c2dee680f1..79fec88bcf 100644 --- a/packages/ses/types.d.ts +++ b/packages/ses/types.d.ts @@ -62,7 +62,7 @@ export interface ThirdPartyStaticModuleInterface { * Note that this value does _not_ contain any numeric or symbol property keys, which can theoretically be members of `exports` in a CommonJS module. */ execute( - proxiedExports: Record, + exportsTarget: Record, compartment: Compartment, resolvedImports: Record, ): void; From d1e1d7112a523c2ac1bd6055ec1c8fb23872e1f5 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Tue, 16 Jan 2024 19:36:27 -0800 Subject: [PATCH 3/3] Handle Module @@toStringTag in compartment-mapper --- packages/compartment-mapper/src/bundle-cjs.js | 10 +++++++++- packages/compartment-mapper/src/bundle.js | 11 ++++++++++- packages/compartment-mapper/test/scaffold.js | 11 +++++++++++ packages/compartment-mapper/test/test-bundle.js | 17 +++++++++-------- packages/compartment-mapper/test/test-policy.js | 14 +++++++------- 5 files changed, 46 insertions(+), 17 deletions(-) diff --git a/packages/compartment-mapper/src/bundle-cjs.js b/packages/compartment-mapper/src/bundle-cjs.js index c219c91f8f..2f3d6635a4 100644 --- a/packages/compartment-mapper/src/bundle-cjs.js +++ b/packages/compartment-mapper/src/bundle-cjs.js @@ -34,7 +34,15 @@ const runtime = function wrapCjsFunctor(num) { .filter(k => moduleCells[k] === undefined) .map(k => (moduleCells[k] = cell(k, cModule.exports[k]))); // Update the star cell from all cells. - const starExports = Object.create(null); + const starExports = Object.create(null, { + // Make this appear like an ESM module namespace object. + [Symbol.toStringTag]: { + value: 'Module', + writable: false, + enumerable: false, + configurable: false, + }, + }); Object.keys(moduleCells) .filter(k => k !== '*') .map(k => Object.defineProperty(starExports, k, moduleCells[k])); diff --git a/packages/compartment-mapper/src/bundle.js b/packages/compartment-mapper/src/bundle.js index ba07e9a433..f8701fa48f 100644 --- a/packages/compartment-mapper/src/bundle.js +++ b/packages/compartment-mapper/src/bundle.js @@ -302,7 +302,16 @@ ${''.concat(...modules.map(m => m.bundlerKit.getCells()))}\ ${''.concat(...modules.map(m => m.bundlerKit.getReexportsWiring()))}\ - const namespaces = cells.map(cells => Object.freeze(Object.create(null, cells))); +const namespaces = cells.map(cells => Object.freeze(Object.create(null, { + ...cells, + // Make this appear like an ESM module namespace object. + [Symbol.toStringTag]: { + value: 'Module', + writable: false, + enumerable: false, + configurable: false, + }, + }))); for (let index = 0; index < namespaces.length; index += 1) { cells[index]['*'] = cell('*', namespaces[index]); diff --git a/packages/compartment-mapper/test/scaffold.js b/packages/compartment-mapper/test/scaffold.js index bedbe37296..bc4fe9dd9f 100644 --- a/packages/compartment-mapper/test/scaffold.js +++ b/packages/compartment-mapper/test/scaffold.js @@ -420,3 +420,14 @@ export function scaffold( }); } } + +// Modifies the given object to make it appear to be an ESM module namespace object. +export const moduleify = obj => { + Object.defineProperty(obj, Symbol.toStringTag, { + value: 'Module', + writable: false, + enumerable: false, + configurable: false, + }); + return Object.setPrototypeOf(obj, null); +}; diff --git a/packages/compartment-mapper/test/test-bundle.js b/packages/compartment-mapper/test/test-bundle.js index 924ae74422..b8e61b69ac 100644 --- a/packages/compartment-mapper/test/test-bundle.js +++ b/packages/compartment-mapper/test/test-bundle.js @@ -4,6 +4,7 @@ import url from 'url'; import test from 'ava'; import { makeBundle, makeArchive, parseArchive } from '../index.js'; import { makeReadPowers } from '../node-powers.js'; +import { moduleify } from './scaffold.js'; const fixture = new URL( 'fixtures-0/node_modules/bundle/main.js', @@ -17,40 +18,40 @@ const expectedLog = [ 'are other fingers.', 'dependency', 'foo', - { + moduleify({ c: 'sea', i: 'eye', q: 'cue', k: 'que', u: 'you', y: 'why', - }, - { + }), + moduleify({ c: 'sea', i: 'eye', q: 'cue', k: 'que', u: 'you', y: 'why', - }, + }), 'fizz', 'buzz', 'blue', 'qux', '#777', - { + moduleify({ red: '#f00', green: '#0f0', blue: '#00f', - }, - { + }), + moduleify({ default: { zzz: 1, fromMjs: 'foo', }, fromMjs: 'foo', zzz: 1, - }, + }), ]; test('bundles work', async t => { diff --git a/packages/compartment-mapper/test/test-policy.js b/packages/compartment-mapper/test/test-policy.js index 0203b620c0..7884ab9d43 100644 --- a/packages/compartment-mapper/test/test-policy.js +++ b/packages/compartment-mapper/test/test-policy.js @@ -1,7 +1,7 @@ // import "./ses-lockdown.js"; import 'ses'; import test from 'ava'; -import { scaffold, sanitizePaths } from './scaffold.js'; +import { moduleify, scaffold, sanitizePaths } from './scaffold.js'; function combineAssertions(...assertionFunctions) { return async (...args) => { @@ -86,7 +86,7 @@ const anyPolicy = { }; const defaultExpectations = { - namespace: { + namespace: moduleify({ alice: { bluePill: 'undefined', redPill: 'number', @@ -102,23 +102,23 @@ const defaultExpectations = { scopedBob: { scoped: 1 }, builtins: '{"a":1,"b":2,"default":{"a":1,"b":2}}', builtins2: '{"c":3,"default":{"c":3}}', - }, + }), }; const anyExpectations = { - namespace: { + namespace: moduleify({ ...defaultExpectations.namespace, carol: { bluePill: 'number', redPill: 'number', purplePill: 'number' }, - }, + }), }; const powerlessCarolExpectations = { - namespace: { + namespace: moduleify({ ...defaultExpectations.namespace, carol: { bluePill: 'undefined', redPill: 'undefined', purplePill: 'undefined', }, - }, + }), }; const makeResultAssertions =