policy: fix cascading getting the resource instead of scope

nodejs · Feb 9, 2021 · d74f91e · d74f91e
1 parent 907d6b6
commit d74f91e
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 10 deletions.
diff --git a/lib/internal/policy/manifest.js b/lib/internal/policy/manifest.js
@@ -41,6 +41,8 @@ const shouldAbortOnUncaughtException =
   getOptionValue('--abort-on-uncaught-exception');
 const { abort, exit, _rawDebug } = process;
 
+const TERMINATE = () => null;
+
 // From https://url.spec.whatwg.org/#special-scheme
 const SPECIAL_SCHEMES = new SafeSet([
   'file:',
@@ -76,7 +78,7 @@ function REACTION_LOG(error) {
 
 class Manifest {
   /**
-   * @type {Map<string, DependencyMapper>}
+   * @type {Map<string | null | undefined, DependencyMapper>}
    *
    * Used to compare a resource to the content body at the resource.
    * `true` is used to signify that all integrities are allowed, otherwise,
@@ -139,6 +141,8 @@ class Manifest {
    */
   constructor(obj, manifestURL) {
     const scopes = this.#scopeDependencies;
+    scopes.set(null, TERMINATE);
+    scopes.set(undefined, TERMINATE);
     const integrities = this.#resourceIntegrities;
     const dependencies = this.#resourceDependencies;
     let reaction = REACTION_THROW;
@@ -205,18 +209,20 @@ class Manifest {
       return (toSpecifier, conditions) => {
         if (toSpecifier in dependencyMap !== true) {
           if (cascade === true) {
-            let scopeHREF;
+            /** @type {string | null} */
+            let scopeHREF = resourceHREF;
             if (typeof parentDeps === 'undefined') {
               do {
-                scopeHREF = this.#findScopeHREF(resourceHREF);
+                scopeHREF = this.#findScopeHREF(scopeHREF);
+                if (scopeHREF === resourceHREF) {
+                  scopeHREF = null;
+                }
+                if (scopes.has(scopeHREF)) {
+                  break;
+                }
               } while (
-                scopeHREF !== null &&
-                scopes.has(scopeHREF) !== true
+                scopeHREF !== null
               );
-            }
-            if (scopeHREF === null) {
-              parentDeps = () => null;
-            } else {
               parentDeps = scopes.get(scopeHREF);
             }
             return parentDeps(toSpecifier);

diff --git a/test/parallel/test-policy-scopes.js b/test/parallel/test-policy-scopes.js
@@ -10,8 +10,8 @@ const fixtures = require('../common/fixtures');
 const assert = require('assert');
 const { spawnSync } = require('child_process');
 
-const dep = fixtures.path('policy', 'main.mjs');
 {
+  const dep = fixtures.path('policy', 'main.mjs');
   const depPolicy = fixtures.path(
     'policy',
     'dependencies',
@@ -24,3 +24,17 @@ const dep = fixtures.path('policy', 'main.mjs');
   );
   assert.strictEqual(status, 0);
 }
+{
+  const dep = fixtures.path('policy', 'multi-deps.js');
+  const depPolicy = fixtures.path(
+    'policy',
+    'dependencies',
+    'dependencies-scopes-and-resources-policy.json');
+  const { status } = spawnSync(
+    process.execPath,
+    [
+      '--experimental-policy', depPolicy, dep,
+    ]
+  );
+  assert.strictEqual(status, 0);
+}