fix(compartment-mapper): Missing entries and conflated namespace

packages/compartment-mapper/src/archive.js

@@ -330,26 +330,26 @@ const digestLocation = async (powers, moduleLocation, options) => {
 
   const {
     compartments,
-    entry: { module: entryModuleSpecifier },
+    entry: { module: entryModuleSpecifier, compartment: entryCompartmentName },
   } = compartmentMap;
 
   /** @type {Sources} */
   const sources = Object.create(null);
 
-  const compartmentExitModuleImportHook = exitModuleImportHookMaker({
+  const consolidatedExitModuleImportHook = exitModuleImportHookMaker({
     modules: exitModules,
     exitModuleImportHook,
   });
 
-  const makeImportHook = makeImportHookMaker({
-    readPowers: read,
-    baseLocation: packageLocation,
+  const makeImportHook = makeImportHookMaker(read, packageLocation, {
     sources,
     compartmentDescriptors: compartments,
-    exitModuleImportHook: compartmentExitModuleImportHook,
     archiveOnly: true,
     computeSha512,
     searchSuffixes,
+    entryCompartmentName,
+    entryModuleSpecifier,
+    exitModuleImportHook: consolidatedExitModuleImportHook,
   });
   // Induce importHook to record all the necessary modules to import the given module specifier.
   const { compartment, attenuatorsCompartment } = link(compartmentMap, {

packages/compartment-mapper/src/bundle.js

@@ -45,14 +45,12 @@ const parserForLanguage = {
 /**
  * @param {Record<string, CompartmentDescriptor>} compartmentDescriptors
  * @param {Record<string, CompartmentSources>} compartmentSources
- * @param {Record<string, ResolveHook>} compartmentResolvers
  * @param {string} entryCompartmentName
  * @param {string} entryModuleSpecifier
  */
 const sortedModules = (
   compartmentDescriptors,
   compartmentSources,
-  compartmentResolvers,
   entryCompartmentName,
   entryModuleSpecifier,
 ) => {
@@ -71,7 +69,6 @@ const sortedModules = (
     }
     seen.add(key);
 
-    const resolve = compartmentResolvers[compartmentName];
     const source = compartmentSources[compartmentName][moduleSpecifier];
     if (source !== undefined) {
       const { record, parser, deferredError } = source;
@@ -85,6 +82,9 @@ const sortedModules = (
           /** @type {PrecompiledStaticModuleInterface} */ (record);
         const resolvedImports = Object.create(null);
         for (const importSpecifier of [...imports, ...reexports]) {
+          // If we ever support another module resolution algorithm, that
+          // should be indicated in the compartment descriptor by name and the
+          // corresponding behavior selected here.
           const resolvedSpecifier = resolve(importSpecifier, moduleSpecifier);
           resolvedImports[importSpecifier] = recur(
             compartmentName,
@@ -165,8 +165,8 @@ function getBundlerKitForModule(module) {
  * @param {ModuleTransforms} [options.moduleTransforms]
  * @param {boolean} [options.dev]
  * @param {Set<string>} [options.tags]
- * @param {Array<string>} [options.searchSuffixes]
  * @param {object} [options.commonDependencies]
+ * @param {Array<string>} [options.searchSuffixes]
  * @returns {Promise<string>}
  */
 export const makeBundle = async (read, moduleLocation, options) => {
@@ -206,15 +206,16 @@ export const makeBundle = async (read, moduleLocation, options) => {
   /** @type {Sources} */
   const sources = Object.create(null);
 
-  const makeImportHook = makeImportHookMaker({
-    readPowers: read,
-    baseLocation: packageLocation,
+  const makeImportHook = makeImportHookMaker(read, packageLocation, {
     sources,
     compartmentDescriptors: compartments,
     searchSuffixes,
+    entryCompartmentName,
+    entryModuleSpecifier,
   });
+
   // Induce importHook to record all the necessary modules to import the given module specifier.
-  const { compartment, resolvers } = link(compartmentMap, {
+  const { compartment } = link(compartmentMap, {
     resolve,
     makeImportHook,
     moduleTransforms,
@@ -225,7 +226,6 @@ export const makeBundle = async (read, moduleLocation, options) => {
   const { modules, aliases } = sortedModules(
     compartmentMap.compartments,
     sources,
-    resolvers,
     entryCompartmentName,
     entryModuleSpecifier,
   );

packages/compartment-mapper/src/import-hook.js

@@ -15,6 +15,7 @@
 /** @typedef {import('./types.js').ExitModuleImportHook} ExitModuleImportHook */
 
 import { attenuateModuleHook, enforceModulePolicy } from './policy.js';
+import { resolve } from './node-module-specifier.js';
 import { unpackReadPowers } from './powers.js';
 
 // q, as in quote, for quoting strings in error messages.
@@ -63,7 +64,6 @@ const nodejsConventionSearchSuffixes = [
 ];
 
 /**
- *
  * @param {object} params
  * @param {Record<string, any>=} params.modules
  * @param {ExitModuleImportHook=} params.exitModuleImportHook
@@ -96,35 +96,60 @@ export const exitModuleImportHookMaker = ({
 };
 
 /**
+ * @param {ReadFn|ReadPowers} readPowers
+ * @param {string} baseLocation
  * @param {object} options
- * @param {ReadFn|ReadPowers} options.readPowers
- * @param {string} options.baseLocation
- * @param {Sources=} options.sources
- * @param {Record<string, CompartmentDescriptor>=} options.compartmentDescriptors
- * @param {ExitModuleImportHook=} options.exitModuleImportHook
- * @param {boolean=} options.archiveOnly
- * @param {HashFn=} options.computeSha512
- * @param {Array<string>=} options.searchSuffixes - Suffixes to search if the unmodified specifier is not found.
+ * @param {Sources} [options.sources]
+ * @param {Record<string, CompartmentDescriptor>} [options.compartmentDescriptors]
+ * @param {boolean} [options.archiveOnly]
+ * @param {HashFn} [options.computeSha512]
+ * @param {Array<string>} [options.searchSuffixes] - Suffixes to search if the
+ * unmodified specifier is not found.
  * Pass [] to emulate Node.js’s strict behavior.
  * The default handles Node.js’s CommonJS behavior.
- * Unlike Node.js, the Compartment Mapper lifts CommonJS up, more like a bundler,
- * and does not attempt to vary the behavior of resolution depending on the
- * language of the importing module.
+ * Unlike Node.js, the Compartment Mapper lifts CommonJS up, more like a
+ * bundler, and does not attempt to vary the behavior of resolution depending
+ * on the language of the importing module.
+ * @param {string} options.entryCompartmentName
+ * @param {string} options.entryModuleSpecifier
+ * @param {ExitModuleImportHook} [options.exitModuleImportHook]
  * @returns {ImportHookMaker}
  */
-export const makeImportHookMaker = ({
+export const makeImportHookMaker = (
   readPowers,
   baseLocation,
-  sources = Object.create(null),
-  compartmentDescriptors = Object.create(null),
-  exitModuleImportHook = undefined,
-  archiveOnly = false,
-  computeSha512 = undefined,
-  searchSuffixes = nodejsConventionSearchSuffixes,
-}) => {
-  // Set of specifiers for modules whose parser is not using heuristics to determine imports
-  const strictlyRequired = new Set();
-  // per-assembly:
+  {
+    sources = Object.create(null),
+    compartmentDescriptors = Object.create(null),
+    archiveOnly = false,
+    computeSha512 = undefined,
+    searchSuffixes = nodejsConventionSearchSuffixes,
+    entryCompartmentName,
+    entryModuleSpecifier,
+    exitModuleImportHook = undefined,
+  },
+) => {
+  // Set of specifiers for modules (scoped to compartment) whose parser is not
+  // using heuristics to determine imports.
+  /** @type {Map<string, Set<string>>} compartment name ->* module specifier */
+  const strictlyRequired = new Map([
+    [entryCompartmentName, new Set([entryModuleSpecifier])],
+  ]);
+
+  /**
+   * @param {string} compartmentName
+   */
+  const strictlyRequiredForCompartment = compartmentName => {
+    let compartmentStrictlyRequired = strictlyRequired.get(compartmentName);
+    if (compartmentStrictlyRequired !== undefined) {
+      return compartmentStrictlyRequired;
+    }
+    compartmentStrictlyRequired = new Set();
+    strictlyRequired.set(compartmentName, compartmentStrictlyRequired);
+    return compartmentStrictlyRequired;
+  };
+
+  // per-compartment:
   /** @type {ImportHookMaker} */
   const makeImportHook = ({
     packageLocation,
@@ -154,7 +179,7 @@ export const makeImportHookMaker = ({
       // defer, because importing from esm makes it strictly required.
       // Note that ultimately a situation may arise, with exit modules, where the module never reaches importHook but
       // its imports do. In that case the notion of strictly required is no longer boolean, it's true,false,noidea.
-      if (strictlyRequired.has(specifier)) {
+      if (strictlyRequiredForCompartment(packageLocation).has(specifier)) {
         throw error;
       }
       // Return a place-holder that'd throw an error if executed
@@ -323,10 +348,11 @@ export const makeImportHookMaker = ({
             sha512,
           };
           if (!shouldDeferError(parser)) {
-            getImportsFromRecord(record).forEach(
-              strictlyRequired.add,
-              strictlyRequired,
-            );
+            for (const importSpecifier of getImportsFromRecord(record)) {
+              strictlyRequiredForCompartment(packageLocation).add(
+                resolve(importSpecifier, moduleSpecifier),
+              );
+            }
           }
 
           return record;

packages/compartment-mapper/src/import.js

@@ -85,13 +85,13 @@ export const loadLocation = async (readPowers, moduleLocation, options) => {
       modules,
       exitModuleImportHook,
     });
-    const makeImportHook = makeImportHookMaker({
-      readPowers,
-      baseLocation: packageLocation,
+    const makeImportHook = makeImportHookMaker(readPowers, packageLocation, {
       compartmentDescriptors: compartmentMap.compartments,
-      exitModuleImportHook: compartmentExitModuleImportHook,
-      archiveOnly: false,
       searchSuffixes,
+      archiveOnly: false,
+      entryCompartmentName: packageLocation,
+      entryModuleSpecifier: moduleSpecifier,
+      exitModuleImportHook: compartmentExitModuleImportHook,
     });
     const { compartment, pendingJobsPromise } = link(compartmentMap, {
       makeImportHook,

packages/compartment-mapper/src/link.js

@@ -345,9 +345,6 @@ export const link = (
     compartmentDescriptors,
   );
 
-  /** @type {Record<string, ResolveHook>} */
-  const resolvers = Object.create(null);
-
   const pendingJobs = [];
 
   for (const [compartmentName, compartmentDescriptor] of entries(
@@ -383,6 +380,9 @@ export const link = (
       }
     };
 
+    // If we ever need an alternate resolution algorithm, it should be
+    // indicated in the compartment descriptor and a behavior selected here.
+    const resolveHook = resolve;
     const importHook = makeImportHook({
       packageLocation: location,
       packageName: name,
@@ -398,8 +398,6 @@ export const link = (
       modules,
       scopes,
     );
-    const resolveHook = resolve;
-    resolvers[compartmentName] = resolve;
 
     const compartment = new Compartment(Object.create(null), undefined, {
       resolveHook,
@@ -437,7 +435,6 @@ export const link = (
   return {
     compartment,
     compartments,
-    resolvers,
     attenuatorsCompartment,
     pendingJobsPromise: promiseAllSettled(pendingJobs).then(
       /** @param {PromiseSettledResult<unknown>[]} results */ results => {

packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/left/index.cjs

@@ -0,0 +1,4 @@
+try {
+  // Does not exist in parent directory.
+  require('../inconsistent.js');
+} catch {}

packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/main.js

@@ -0,0 +1,2 @@
+import './left/index.cjs';
+import './right/child/index.mjs';

packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "app",
+  "version": "1.0.0",
+  "type": "module",
+  "main": "./main.js",
+  "scripts": {
+    "preinstall": "echo DO NOT INSTALL TEST FIXTURES; exit -1"
+  }
+}
+

packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/right/child/index.mjs

@@ -0,0 +1,2 @@
+// Does exist in the parent directory and is strictly retained.
+import '../inconsistent.js';

packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/right/inconsistent.js

@@ -0,0 +1 @@
+// Exists in this directory!

packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/main.js

@@ -0,0 +1,2 @@
+import 'left';
+import 'right';

packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/package.json

@@ -0,0 +1,14 @@
+{
+  "name": "app",
+  "version": "1.0.0",
+  "type": "module",
+  "main": "./main.js",
+  "dependencies": {
+    "left": "^1.0.0",
+    "right": "^1.0.0"
+  },
+  "scripts": {
+    "preinstall": "echo DO NOT INSTALL TEST FIXTURES; exit -1"
+  }
+}
+

packages/compartment-mapper/test/test-missing-entry.js

@@ -0,0 +1,23 @@
+import 'ses';
+import test from 'ava';
+import path from 'path';
+import fs from 'fs';
+import url from 'url';
+import crypto from 'crypto';
+
+import { makeAndHashArchive } from '../archive.js';
+import { makeReadPowers } from '../node-powers.js';
+
+const readPowers = makeReadPowers({ fs, url, crypto });
+
+test('missing entry', async t => {
+  const entry = url.pathToFileURL(
+    path.resolve('i-solemnly-swear-i-do-not-exist.js'),
+  );
+  await t.throwsAsync(
+    makeAndHashArchive(readPowers, entry, {}).then(() => {}),
+    {
+      message: /Failed to load/,
+    },
+  );
+});