feat(ses): Opt out of namespace boxes (#2380)

Refs: #400 ## Description To reduce mismatched behavior between SES and XS, this change adds an option (to be removed at the next major version) that changes the compartment import method such that it returns a promise for an exports namespace instead of a promise for a box containing the imports namespace. This opts-in to the same thenable behavior as dynamic import in the language. ### Security Considerations With this option, we must be weary of thenable modules, as we must be for thenable modules across the rest of the JavaScript ecosystem. ### Scaling Considerations None. ### Documentation Considerations This option will necessarily appear in all portable code examples on hardenedjs.org once it is available. ### Testing Considerations Tests have been adjusted to take advantage of the new option, except legacy tests which remain to test the behavior without the option enabled. ### Compatibility Considerations This is a backward compatible change. Namespace boxes are hereafter deprecated and we hope to remove this option on the next major release, making the new behavior mandatory or at least opt-out. ### Upgrade Considerations None.
endojs · Jul 25, 2024 · a772166 · a772166
2 parents d8ffbfc + 82a315e
commit a772166
Show file tree

Hide file tree

Showing 11 changed files with 954 additions and 237 deletions.
diff --git a/packages/ses/NEWS.md b/packages/ses/NEWS.md
@@ -25,6 +25,12 @@ User-visible changes in SES:
   the stacktrace line-numbers point back into the original
   source, as they do on Node without SES.
 
+- Adds a `__noNamespaceBox__` option that aligns the behavior of the `import`
+  method on SES `Compartment` with the behavior of XS and the behavior we will
+  champion for compartment standards.
+  All use of `Compartment` should migrate to use this option as the standard
+  behavior will be enabled by default with the next major version of SES.
+
 # v1.5.0 (2024-05-06)
 
 - Adds `importNowHook` to the `Compartment` options. The compartment will invoke the hook whenever it encounters a missing dependency while running `compartmentInstance.importNow(specifier)`, which cannot use an asynchronous `importHook`.

diff --git a/packages/ses/src/compartment.js b/packages/ses/src/compartment.js
@@ -104,6 +104,8 @@ export const CompartmentPrototype = {
   },
 
   async import(specifier) {
+    const { noNamespaceBox } = weakmapGet(privateFields, this);
+
     if (typeof specifier !== 'string') {
       throw TypeError('first argument of import() must be a string');
     }
@@ -117,6 +119,11 @@ export const CompartmentPrototype = {
           /** @type {Compartment} */ (this),
           specifier,
         );
+        if (noNamespaceBox) {
+          return namespace;
+        }
+        // Legacy behavior: box the namespace object so that thenable modules
+        // do not get coerced into a promise accidentally.
         return { namespace };
       },
     );
@@ -172,7 +179,11 @@ export const makeCompartmentConstructor = (
   markVirtualizedNativeFunction,
   parentCompartment = undefined,
 ) => {
-  function Compartment(endowments = {}, moduleMap = {}, options = {}) {
+  function Compartment(
+    endowmentsOption = {},
+    moduleMapOption = {},
+    options = {},
+  ) {
     if (new.target === undefined) {
       throw TypeError(
         "Class constructor Compartment cannot be invoked without 'new'",
@@ -189,8 +200,11 @@ export const makeCompartmentConstructor = (
       importNowHook,
       moduleMapHook,
       importMetaHook,
+      __noNamespaceBox__: noNamespaceBox = false,
     } = options;
     const globalTransforms = [...transforms, ...__shimTransforms__];
+    const endowments = { __proto__: null, ...endowmentsOption };
+    const moduleMap = { __proto__: null, ...moduleMapOption };
 
     // Map<FullSpecifier, ModuleCompartmentRecord>
     const moduleRecords = new Map();
@@ -249,6 +263,7 @@ export const makeCompartmentConstructor = (
       deferredExports,
       instances,
       parentCompartment,
+      noNamespaceBox,
     });
   }
 

diff --git a/packages/ses/test/compartment-transforms.test.js b/packages/ses/test/compartment-transforms.test.js
@@ -52,9 +52,13 @@ test('transforms do not apply to imported modules', async t => {
   const resolveHook = () => '';
   const importHook = () =>
     new ModuleSource('export default "Farewell, World!";');
-  const c = new Compartment({}, {}, { transforms, resolveHook, importHook });
+  const c = new Compartment(
+    {},
+    {},
+    { transforms, resolveHook, importHook, __noNamespaceBox__: true },
+  );
 
-  const { namespace } = await c.import('any-string-here');
+  const namespace = await c.import('any-string-here');
   const { default: greeting } = namespace;
 
   t.is(greeting, 'Farewell, World!');
@@ -82,10 +86,15 @@ test('__shimTransforms__ do apply to imported modules', async t => {
   const c = new Compartment(
     {},
     {},
-    { __shimTransforms__: transforms, resolveHook, importHook },
+    {
+      __shimTransforms__: transforms,
+      __noNamespaceBox__: true,
+      resolveHook,
+      importHook,
+    },
   );
 
-  const { namespace } = await c.import('any-string-here');
+  const namespace = await c.import('any-string-here');
   const { default: greeting } = namespace;
 
   t.is(greeting, 'Hello, World!');

diff --git a/packages/ses/test/import-cjs.test.js b/packages/ses/test/import-cjs.test.js
@@ -86,11 +86,13 @@ test('import a CommonJS module with exports assignment', async t => {
     );
   };
 
-  const compartment = new Compartment({}, {}, { resolveHook, importHook });
+  const compartment = new Compartment(
+    {},
+    {},
+    { resolveHook, importHook, __noNamespaceBox__: true },
+  );
   const module = compartment.module('.');
-  const {
-    namespace: { meaning },
-  } = await compartment.import('.');
+  const { meaning } = await compartment.import('.');
 
   t.is(meaning, 42, 'exports seen');
   t.is(module.meaning, 42, 'exports seen through deferred proxy');
@@ -109,11 +111,13 @@ test('import a CommonJS module with exports replacement', async t => {
     );
   };
 
-  const compartment = new Compartment({}, {}, { resolveHook, importHook });
+  const compartment = new Compartment(
+    {},
+    {},
+    { resolveHook, importHook, __noNamespaceBox__: true },
+  );
   const module = compartment.module('.');
-  const {
-    namespace: { default: meaning },
-  } = await compartment.import('.');
+  const { default: meaning } = await compartment.import('.');
 
   t.is(meaning, 42, 'exports seen');
   t.is(module.default, 42, 'exports seen through deferred proxy');
@@ -144,10 +148,12 @@ test('CommonJS module imports CommonJS module by name', async t => {
     throw Error(`Cannot load module ${specifier}`);
   };
 
-  const compartment = new Compartment({}, {}, { resolveHook, importHook });
-  const {
-    namespace: { odd },
-  } = await compartment.import('./odd');
+  const compartment = new Compartment(
+    {},
+    {},
+    { resolveHook, importHook, __noNamespaceBox__: true },
+  );
+  const { odd } = await compartment.import('./odd');
 
   t.is(odd(1), true);
   t.is(odd(2), false);
@@ -178,8 +184,12 @@ test('CommonJS module imports CommonJS module as default', async t => {
     throw Error(`Cannot load module ${specifier}`);
   };
 
-  const compartment = new Compartment({}, {}, { resolveHook, importHook });
-  const { namespace } = await compartment.import('./odd');
+  const compartment = new Compartment(
+    {},
+    {},
+    { resolveHook, importHook, __noNamespaceBox__: true },
+  );
+  const namespace = await compartment.import('./odd');
   const { default: odd } = namespace;
 
   t.is(odd(1), true);
@@ -211,10 +221,12 @@ test('ESM imports CommonJS module as default', async t => {
     throw Error(`Cannot load module ${specifier}`);
   };
 
-  const compartment = new Compartment({}, {}, { resolveHook, importHook });
-  const {
-    namespace: { default: odd },
-  } = await compartment.import('./odd');
+  const compartment = new Compartment(
+    {},
+    {},
+    { resolveHook, importHook, __noNamespaceBox__: true },
+  );
+  const { default: odd } = await compartment.import('./odd');
 
   t.is(odd(1), true);
   t.is(odd(2), false);
@@ -245,10 +257,12 @@ test('ESM imports CommonJS module as star', async t => {
     throw Error(`Cannot load module ${specifier}`);
   };
 
-  const compartment = new Compartment({}, {}, { resolveHook, importHook });
-  const {
-    namespace: { default: odd },
-  } = await compartment.import('./odd');
+  const compartment = new Compartment(
+    {},
+    {},
+    { resolveHook, importHook, __noNamespaceBox__: true },
+  );
+  const { default: odd } = await compartment.import('./odd');
 
   t.is(odd(1), true);
   t.is(odd(2), false);
@@ -281,10 +295,12 @@ test('ESM imports CommonJS module with replaced exports as star', async t => {
     throw Error(`Cannot load module ${specifier}`);
   };
 
-  const compartment = new Compartment({}, {}, { resolveHook, importHook });
-  const {
-    namespace: { default: odd },
-  } = await compartment.import('./odd');
+  const compartment = new Compartment(
+    {},
+    {},
+    { resolveHook, importHook, __noNamespaceBox__: true },
+  );
+  const { default: odd } = await compartment.import('./odd');
 
   t.is(odd(1), true);
   t.is(odd(2), false);
@@ -315,10 +331,12 @@ test('ESM imports CommonJS module by name', async t => {
     throw Error(`Cannot load module ${specifier}`);
   };
 
-  const compartment = new Compartment({}, {}, { resolveHook, importHook });
-  const {
-    namespace: { default: odd },
-  } = await compartment.import('./odd');
+  const compartment = new Compartment(
+    {},
+    {},
+    { resolveHook, importHook, __noNamespaceBox__: true },
+  );
+  const { default: odd } = await compartment.import('./odd');
 
   t.is(odd(1), true);
   t.is(odd(2), false);
@@ -349,10 +367,12 @@ test('CommonJS module imports ESM as default', async t => {
     throw Error(`Cannot load module ${specifier}`);
   };
 
-  const compartment = new Compartment({}, {}, { resolveHook, importHook });
-  const {
-    namespace: { default: odd },
-  } = await compartment.import('./odd');
+  const compartment = new Compartment(
+    {},
+    {},
+    { resolveHook, importHook, __noNamespaceBox__: true },
+  );
+  const { default: odd } = await compartment.import('./odd');
 
   t.is(odd(1), true);
   t.is(odd(2), false);
@@ -385,10 +405,12 @@ test('CommonJS module imports ESM by name', async t => {
     throw Error(`Cannot load module ${specifier}`);
   };
 
-  const compartment = new Compartment({}, {}, { resolveHook, importHook });
-  const {
-    namespace: { odd },
-  } = await compartment.import('./odd');
+  const compartment = new Compartment(
+    {},
+    {},
+    { resolveHook, importHook, __noNamespaceBox__: true },
+  );
+  const { odd } = await compartment.import('./odd');
 
   t.is(odd(1), true);
   t.is(odd(2), false);
@@ -440,7 +462,11 @@ test('cross import ESM and CommonJS modules', async t => {
     throw Error(`Cannot load module for specifier ${specifier}`);
   };
 
-  const compartment = new Compartment({ t }, {}, { resolveHook, importHook });
+  const compartment = new Compartment(
+    { t },
+    {},
+    { resolveHook, importHook, __noNamespaceBox__: true },
+  );
   await compartment.import('./src/main.js');
 });
 

diff --git a/packages/ses/test/import-gauntlet.test.js b/packages/ses/test/import-gauntlet.test.js
@@ -49,10 +49,11 @@ test('import all from module', async t => {
     {
       resolveHook: resolveNode,
       importHook: makeImportHook('https://example.com'),
+      __noNamespaceBox__: true,
     },
   );
 
-  const { namespace } = await compartment.import('./main.js');
+  const namespace = await compartment.import('./main.js');
 
   t.is(namespace.default.a, 10);
   t.is(namespace.default.b, 20);
@@ -78,10 +79,11 @@ test('import named exports from me', async t => {
     {
       resolveHook: resolveNode,
       importHook: makeImportHook('https://example.com'),
+      __noNamespaceBox__: true,
     },
   );
 
-  const { namespace } = await compartment.import('./main.js');
+  const namespace = await compartment.import('./main.js');
 
   t.is(namespace.default.fizz, 10);
   t.is(namespace.default.buzz, 20);
@@ -106,10 +108,11 @@ test('import color from module', async t => {
     {
       resolveHook: resolveNode,
       importHook: makeImportHook('https://example.com'),
+      __noNamespaceBox__: true,
     },
   );
 
-  const { namespace } = await compartment.import('./main.js');
+  const namespace = await compartment.import('./main.js');
 
   t.is(namespace.color, 'blue');
 });
@@ -132,10 +135,11 @@ test('import and reexport', async t => {
     {
       resolveHook: resolveNode,
       importHook: makeImportHook('https://example.com'),
+      __noNamespaceBox__: true,
     },
   );
 
-  const { namespace } = await compartment.import('./main.js');
+  const namespace = await compartment.import('./main.js');
 
   t.is(namespace.qux, 42);
 });
@@ -159,10 +163,11 @@ test('import and export all', async t => {
     {
       resolveHook: resolveNode,
       importHook: makeImportHook('https://example.com'),
+      __noNamespaceBox__: true,
     },
   );
 
-  const { namespace } = await compartment.import('./main.js');
+  const namespace = await compartment.import('./main.js');
 
   t.is(namespace.alpha, 0);
   t.is(namespace.omega, 23);
@@ -189,10 +194,11 @@ test('live binding', async t => {
     {
       resolveHook: resolveNode,
       importHook: makeImportHook('https://example.com'),
+      __noNamespaceBox__: true,
     },
   );
 
-  const { namespace } = await compartment.import('./main.js');
+  const namespace = await compartment.import('./main.js');
 
   t.is(namespace.default, 'Hello, World!');
 });