fix(compartment-mapper): use reliable information to identify package…

…s for policy application

packages/compartment-mapper/src/archive.js

@@ -53,7 +53,7 @@ const parserForLanguage = {
  */
 const resolveLocation = (rel, abs) => new URL(rel, abs).toString();
 
-const { keys, values, entries, fromEntries } = Object;
+const { keys, entries, fromEntries } = Object;
 
 /**
  * We attempt to produce compartment maps that are consistent regardless of
@@ -314,7 +314,7 @@ const digestLocation = async (powers, moduleLocation, options) => {
   });
   await compartment.load(entryModuleSpecifier);
   if (policy) {
-    // retain all attenuators. this is the only reason we still need attenuators field in policy. Can be generated from the rest of policy
+    // retain all attenuators.
     await Promise.all(
       detectAttenuators(policy).map(attenuatorSpecifier =>
         attenuatorsCompartment.load(attenuatorSpecifier),

packages/compartment-mapper/src/compartment-map.js

@@ -30,10 +30,20 @@ const cumulativeLength = (length, term) => {
 };
 
 /**
- * @param {Array<string>} a
- * @param {Array<string>} b
+ * @param {Array<string> | undefined} a
+ * @param {Array<string> | undefined} b
  */
 export const pathCompare = (a, b) => {
+  // Undefined is not preferred
+  if (a === undefined && b === undefined) {
+    return 0;
+  }
+  if (a === undefined) {
+    return 1;
+  }
+  if (b === undefined) {
+    return -1;
+  }
   // Prefer the shortest dependency path.
   if (a.length !== b.length) {
     return a.length - b.length;
@@ -108,9 +118,10 @@ const assertCompartmentModule = (allegedModule, path, url) => {
   const { compartment, module, ...extra } = allegedModule;
   assertEmptyObject(
     extra,
-    `${path} must not have extra properties, got ${q(
-      Object.keys(extra),
-    )} in ${q(url)}`,
+    `${path} must not have extra properties, got ${q({
+      extra,
+      compartment,
+    })} in ${q(url)}`,
   );
   assert.typeof(
     compartment,
@@ -360,10 +371,9 @@ const assertTypes = (allegedTypes, path, url) => {
  * @param {unknown} allegedPolicy
  * @param {string} path
  * @param {string} [url]
- * @returns {asserts policy is object}
  */
 
-export const assertPolicy = (
+const assertPolicy = (
   allegedPolicy,
   path,
   url = '<unknown-compartment-map.json>',

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

@@ -11,6 +11,7 @@
 /** @typedef {import('./types.js').CompartmentDescriptor} CompartmentDescriptor */
 /** @typedef {import('./types.js').ImportHookMaker} ImportHookMaker */
 
+import { gatekeepModuleAccess } from './policy.js';
 import { unpackReadPowers } from './powers.js';
 
 // q, as in quote, for quoting strings in error messages.
@@ -145,6 +146,9 @@ export const makeImportHookMaker = (
       // The `moduleMapHook` captures all third-party dependencies.
       if (moduleSpecifier !== '.' && !moduleSpecifier.startsWith('./')) {
         if (has(exitModules, moduleSpecifier)) {
+          gatekeepModuleAccess(moduleSpecifier, compartmentDescriptor.policy, {
+            exit: true,
+          });
           packageSources[moduleSpecifier] = {
             exit: moduleSpecifier,
           };

packages/compartment-mapper/src/import.js

@@ -70,13 +70,8 @@ export const loadLocation = async (readPowers, moduleLocation, options) => {
 
   /** @type {ExecuteFn} */
   const execute = async (options = {}) => {
-    const {
-      globals,
-      modules,
-      transforms,
-      __shimTransforms__,
-      Compartment,
-    } = options;
+    const { globals, modules, transforms, __shimTransforms__, Compartment } =
+      options;
     const makeImportHook = makeImportHookMaker(
       readPowers,
       packageLocation,

packages/compartment-mapper/src/link.js

@@ -19,7 +19,7 @@ import {
   gatekeepModuleAccess,
   attenuateModuleHook,
   ATTENUATORS_COMPARTMENT,
-  getPolicyFor,
+  diagnoseMissingCompartmentError,
 } from './policy.js';
 
 const { entries, fromEntries, freeze } = Object;
@@ -225,11 +225,7 @@ const makeModuleMapHook = (
         exit,
       } = moduleDescriptor;
       if (exit !== undefined) {
-        // TODO Currenly, every package can connect to built-in modules.
-        // Policies should be able to allow third-party modules to exit to
-        // built-ins explicitly, or have built-ins subverted by modules from
-        // specific compartments.
-        gatekeepModuleAccess(moduleSpecifier, compartmentDescriptor.policy, {
+        gatekeepModuleAccess(moduleSpecifier, compartmentDescriptor, {
           exit: true,
         });
         const module = exitModules[exit];
@@ -254,7 +250,7 @@ const makeModuleMapHook = (
       if (foreignModuleSpecifier !== undefined) {
         if (!moduleSpecifier.startsWith('./')) {
           // archive goes through foreignModuleSpecifier for local modules too
-          gatekeepModuleAccess(moduleSpecifier, compartmentDescriptor.policy, {
+          gatekeepModuleAccess(moduleSpecifier, compartmentDescriptor, {
             exit: false,
           });
         }
@@ -264,20 +260,23 @@ const makeModuleMapHook = (
           throw new Error(
             `Cannot import from missing compartment ${q(
               foreignCompartmentName,
-            )}`,
+            )}${diagnoseMissingCompartmentError({
+              moduleSpecifier,
+              compartmentDescriptor,
+              foreignModuleSpecifier,
+              foreignCompartmentName,
+            })}`,
           );
         }
         return foreignCompartment.module(foreignModuleSpecifier);
       }
     } else if (has(exitModules, moduleSpecifier)) {
-      gatekeepModuleAccess(moduleSpecifier, compartmentDescriptor.policy, {
+      gatekeepModuleAccess(moduleSpecifier, compartmentDescriptor, {
         exit: true,
       });
 
       // When linking off the filesystem as with `importLocation`,
       // there isn't a module descriptor for every module.
-      // TODO grant access to built-in modules contingent on a policy in the
-      // application's entry package descriptor.
       moduleDescriptors[moduleSpecifier] = { exit: moduleSpecifier };
       if (archiveOnly) {
         return inertModuleNamespace;
@@ -302,8 +301,6 @@ const makeModuleMapHook = (
         scopePrefix,
       );
 
-      // TODO: figure out gatekeepModuleAccess params for this
-
       if (foreignModuleSpecifier !== undefined) {
         const { compartment: foreignCompartmentName } = scopeDescriptor;
         if (foreignCompartmentName === undefined) {
@@ -316,18 +313,29 @@ const makeModuleMapHook = (
           throw new Error(
             `Cannot import from missing compartment ${q(
               foreignCompartmentName,
-            )}`,
+            )}${diagnoseMissingCompartmentError({
+              moduleSpecifier,
+              compartmentDescriptor,
+              foreignModuleSpecifier,
+              foreignCompartmentName,
+            })}`,
           );
         }
 
+        // Despite all non-exit modules not allowed by policy being dropped
+        // while building the graph, this check is necessary because module
+        // is written back to the compartment map below.
+        gatekeepModuleAccess(scopePrefix, compartmentDescriptor, {
+          exit: false,
+        });
         // The following line is weird.
         // Information is flowing backward.
         // This moduleMapHook writes back to the `modules` descriptor, from the
         // original compartment map.
         // So the compartment map that was used to create the compartment
         // assembly, can then be captured in an archive, obviating the need for
         // a moduleMapHook when we assemble compartments from the resulting
-        // archiev.
+        // archive.
         moduleDescriptors[moduleSpecifier] = {
           compartment: foreignCompartmentName,
           module: foreignModuleSpecifier,
@@ -369,7 +377,6 @@ export const link = (
     moduleTransforms = {},
     __shimTransforms__ = [],
     modules: exitModules = {},
-    policy,
     archiveOnly = false,
     Compartment = defaultCompartment,
   },
@@ -379,7 +386,11 @@ export const link = (
   /** @type {Record<string, Compartment>} */
   const compartments = Object.create(null);
 
-  const attenuators = v => compartments[ATTENUATORS_COMPARTMENT].import(v);
+  /**
+   * @param {string} attenuatorSpecifier
+   */
+  const attenuators = attenuatorSpecifier =>
+    compartments[ATTENUATORS_COMPARTMENT].import(attenuatorSpecifier);
 
   /** @type {Record<string, ResolveHook>} */
   const resolvers = Object.create(null);