feat(compartment-mapper): allow skipping powerless packages in policy…

… resources WIP
endojs · Nov 23, 2023 · 071dcb4 · 071dcb4
1 parent 2cd6a72
commit 071dcb4
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 248 deletions.
diff --git a/packages/compartment-mapper/src/archive.js b/packages/compartment-mapper/src/archive.js
@@ -145,16 +145,13 @@ const translateCompartmentMap = (compartments, sources, compartmentRenames) => {
       if (compartment.modules) {
         for (const name of keys(compartmentModules).sort()) {
           const module = compartmentModules[name];
-          if (
-            module.compartment !== undefined &&
-            compartmentRenames[module.compartment] !== undefined
-          ) {
+          if (module.compartment !== undefined) {
             modules[name] = {
               ...module,
               compartment: compartmentRenames[module.compartment],
             };
           } else {
-            // If compartment is not defined, the module must not be part of the compartment map.
+            modules[name] = module;
           }
         }
       }

diff --git a/packages/compartment-mapper/src/import-archive.js b/packages/compartment-mapper/src/import-archive.js
@@ -95,6 +95,7 @@ const makeArchiveImportHookMaker = (
           // module is a "builtin" module and the policy needs to be enforced.
           enforceModulePolicy(moduleSpecifier, compartmentDescriptor, {
             exit: true,
+            errorHint: 'The module was not in the archive and an attempt was made to load it as a builtin',
           });
           const record = await exitModuleImportHook(moduleSpecifier);
           if (record) {

diff --git a/packages/compartment-mapper/src/import-hook.js b/packages/compartment-mapper/src/import-hook.js
@@ -220,6 +220,7 @@ export const makeImportHookMaker = (
             // hook returns something. Otherwise, we need to fall back to the 'cannot find' error below.
             enforceModulePolicy(moduleSpecifier, compartmentDescriptor, {
               exit: true,
+              errorHint: 'The module was not in the compartment map and an attempt was made to load it as a builtin',
             });
             if (archiveOnly) {
               // Return a place-holder.

diff --git a/packages/compartment-mapper/src/link.js b/packages/compartment-mapper/src/link.js
@@ -19,7 +19,6 @@ import { parseExtension } from './extension.js';
 import {
   enforceModulePolicy,
   ATTENUATORS_COMPARTMENT,
-  diagnoseMissingCompartmentError,
   attenuateGlobals,
   makeDeferredAttenuatorsProvider,
 } from './policy.js';
@@ -236,6 +235,7 @@ const makeModuleMapHook = (
           // archive goes through foreignModuleSpecifier for local modules too
           enforceModulePolicy(moduleSpecifier, compartmentDescriptor, {
             exit: false,
+            errorHint: `TODO`, // TODO: add a testcase for this and provide a helpful hint
           });
         }
 
@@ -244,12 +244,7 @@ const makeModuleMapHook = (
           throw Error(
             `Cannot import from missing compartment ${q(
               foreignCompartmentName,
-            )}${diagnoseMissingCompartmentError({
-              moduleSpecifier,
-              compartmentDescriptor,
-              foreignModuleSpecifier,
-              foreignCompartmentName,
-            })}`,
+            )}}`,
           );
         }
         return foreignCompartment.module(foreignModuleSpecifier);
@@ -279,20 +274,19 @@ const makeModuleMapHook = (
           throw 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.
         enforceModulePolicy(scopePrefix, compartmentDescriptor, {
           exit: false,
+          errorHint: ` There is an alias defined that causes another package to be imported as ${q(
+            moduleSpecifier,
+          )}. Package ${q(moduleSpecifier)} resolves to ${q(
+            foreignModuleSpecifier,
+          )} in ${q(
+            foreignCompartmentName,
+          )}.`,
         });
         // The following line is weird.
         // Information is flowing backward.

diff --git a/packages/compartment-mapper/src/node-modules.js b/packages/compartment-mapper/src/node-modules.js
@@ -611,11 +611,6 @@ const translateGraph = (
       },
       policy,
     );
-    // do not include compartments for packages not covered by policy
-    if (policy && !packagePolicy) {
-      // eslint-disable-next-line no-continue
-      continue;
-    }
 
     /**
      * @param {string} dependencyName

diff --git a/packages/compartment-mapper/src/policy.js b/packages/compartment-mapper/src/policy.js
@@ -113,29 +113,6 @@ export const dependencyAllowedByPolicy = (namingKit, packagePolicy) => {
   return !!policyLookupHelper(packagePolicy, 'packages', canonicalName);
 };
 
-const validateDependencies = (policy, canonicalName) => {
-  const packages = policy.resources[canonicalName].packages;
-  if (!packages || isAllowingEverything(packages)) {
-    return;
-  }
-
-  const packageNames = keys(packages);
-  const attenuators = detectAttenuators(policy);
-  // Join attenuators with packageNames into a Set to deduplicate and check if all are listed in policy.resources
-  const allSpecifiers = new Set([...packageNames, ...attenuators]);
-  for (const specifier of allSpecifiers) {
-    if (!(specifier in policy.resources)) {
-      throw Error(
-        `Package ${q(specifier)} is allowed for ${q(
-          canonicalName,
-        )} to import but its policy is not defined. Please add a policy for ${q(
-          specifier,
-        )}`,
-      );
-    }
-  }
-};
-
 /**
  * Returns the policy applicable to the canonicalName of the package
  *
@@ -157,14 +134,11 @@ export const getPolicyForPackage = (namingKit, policy) => {
       packages: 'any',
     };
   }
-  if (policy.resources && policy.resources[canonicalName]) {
-    validateDependencies(policy, canonicalName);
+  if (policy.resources && policy.resources[canonicalName] !== undefined) {
     return policy.resources[canonicalName];
   } else {
-    console.warn(
-      `No policy for '${canonicalName}', omitting from compartment map.`,
-    );
-    return undefined;
+    // Allow skipping policy entries for packages with no powers.
+    return {};
   }
 };
 
@@ -354,25 +328,33 @@ export const attenuateGlobals = (
   freezeGlobalThisUnlessOptedOut();
 };
 
+
+const diagnoseModulePolicy = (errorHint) => {
+  if(!errorHint) {
+    return '';
+  }
+  return ` (${errorHint})`;
+}
+
 /**
  * Throws if importing of the specifier is not allowed by the policy
  *
  * @param {string} specifier
  * @param {object} compartmentDescriptor
  * @param {object} [info]
  */
-export const enforceModulePolicy = (specifier, compartmentDescriptor, info) => {
+export const enforceModulePolicy = (specifier, compartmentDescriptor, info = {}) => {
   const { policy, modules } = compartmentDescriptor;
   if (!policy) {
     return;
   }
-
+  
   if (!info.exit) {
     if (!modules[specifier]) {
       throw Error(
         `Importing ${q(specifier)} was not allowed by policy packages:${q(
           policy.packages,
-        )}`,
+        )}${diagnoseModulePolicy(info.errorHint)}`,
       );
     }
     return;
@@ -382,7 +364,7 @@ export const enforceModulePolicy = (specifier, compartmentDescriptor, info) => {
     throw Error(
       `Importing ${q(specifier)} was not allowed by policy builtins:${q(
         policy.builtins,
-      )}`,
+      )}${diagnoseModulePolicy(info.errorHint)}`,
     );
   }
 };
@@ -456,61 +438,3 @@ export const attenuateModuleHook = async (
     originalModuleRecord,
   });
 };
-
-const padDiagnosis = text => ` (${text})`;
-/**
- * Provide dignostic information for a missing compartment error
- *
- * @param {object}  args
- * @param {string}  args.moduleSpecifier
- * @param {object}  args.compartmentDescriptor
- * @param {string}  args.foreignModuleSpecifier
- * @param {string}  args.foreignCompartmentName
- * @returns {string}
- */
-export const diagnoseMissingCompartmentError = ({
-  moduleSpecifier,
-  compartmentDescriptor,
-  foreignModuleSpecifier,
-  foreignCompartmentName,
-}) => {
-  const { policy, name, scopes } = compartmentDescriptor;
-
-  if (policy) {
-    if (!policy.packages) {
-      return padDiagnosis(
-        `There were no allowed packages specified in policy for ${q(name)}`,
-      );
-    }
-    if (name === ATTENUATORS_COMPARTMENT) {
-      return padDiagnosis(
-        `Attenuator ${q(
-          moduleSpecifier,
-        )} was imported but there is no policy resources entry defined for it.`,
-      );
-    }
-
-    const scopeNames = entries(scopes)
-      .filter(([_name, scope]) => scope.compartment === foreignCompartmentName)
-      .map(([scopeName]) => scopeName);
-    if (scopeNames.length === 1 && scopeNames[0] === moduleSpecifier) {
-      return padDiagnosis(
-        `Package ${q(
-          moduleSpecifier,
-        )} is missing. Are you sure there is an entry in policy resources specified for it?`,
-      );
-    } else {
-      return padDiagnosis(
-        `Package ${q(moduleSpecifier)} resolves to ${q(
-          foreignModuleSpecifier,
-        )} in ${q(
-          foreignCompartmentName,
-        )} which seems disallowed by policy. There is likely an override defined that causes another package to be imported as ${q(
-          moduleSpecifier,
-        )}.`,
-      );
-    }
-  }
-  // Omit diagnostics when parent package had no policy - it means there was no policy.
-  return '';
-};
diff --git a/packages/compartment-mapper/test/snapshots/test-policy.js.md b/packages/compartment-mapper/test/snapshots/test-policy.js.md
@@ -4,113 +4,41 @@ The actual snapshot is saved in `test-policy.js.snap`.
 
 Generated by [AVA](https://avajs.dev).
 
-## policy - insufficient policy detected early / loadLocation
-
-> Snapshot 1
-
-    'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"'
-
-## policy - insufficient policy detected early / importLocation
-
-> Snapshot 1
-
-    'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"'
-
-## policy - insufficient policy detected early / makeArchive / parseArchive
-
-> Snapshot 1
-
-    'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"'
-
-## policy - insufficient policy detected early / makeArchive / parseArchive with a prefix
-
-> Snapshot 1
-
-    'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"'
-
-## policy - insufficient policy detected early / writeArchive / loadArchive
-
-> Snapshot 1
-
-    'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"'
-
-## policy - insufficient policy detected early / writeArchive / importArchive
-
-> Snapshot 1
-
-    'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"'
-
-## policy - malfunction resulting in missing compartment / loadLocation
-
-> Snapshot 1
-
-    'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)'
-
-## policy - malfunction resulting in missing compartment / importLocation
-
-> Snapshot 1
-
-    'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)'
-
-## policy - malfunction resulting in missing compartment / makeArchive / parseArchive
-
-> Snapshot 1
-
-    'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)'
-
-## policy - malfunction resulting in missing compartment / makeArchive / parseArchive with a prefix
-
-> Snapshot 1
-
-    'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)'
-
-## policy - malfunction resulting in missing compartment / writeArchive / loadArchive
-
-> Snapshot 1
-
-    'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)'
-
-## policy - malfunction resulting in missing compartment / writeArchive / importArchive
-
-> Snapshot 1
-
-    'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)'
-
 ## policy - attack - browser alias / loadLocation
 
 > Snapshot 1
 
-    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)'
+    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)'
 
 ## policy - attack - browser alias / importLocation
 
 > Snapshot 1
 
-    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)'
+    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)'
 
 ## policy - attack - browser alias / makeArchive / parseArchive
 
 > Snapshot 1
 
-    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)'
+    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)'
 
 ## policy - attack - browser alias / makeArchive / parseArchive with a prefix
 
 > Snapshot 1
 
-    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)'
+    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)'
 
 ## policy - attack - browser alias / writeArchive / loadArchive
 
 > Snapshot 1
 
-    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)'
+    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)'
 
 ## policy - attack - browser alias / writeArchive / importArchive
 
 > Snapshot 1
 
-    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)'
+    'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)'
 
 ## policy - attenuator error aggregation / loadLocation
 

diff --git a/packages/compartment-mapper/test/snapshots/test-policy.js.snap b/packages/compartment-mapper/test/snapshots/test-policy.js.snap