policy: fix cascade getting scope

PR-URL: #37298 Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Co-authored-by: James M Snell <jasnell@gmail.com>
nodejs · May 1, 2021 · 506f1d4 · 506f1d4
1 parent 2a3feff
commit 506f1d4
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 12 deletions.
diff --git a/lib/internal/policy/manifest.js b/lib/internal/policy/manifest.js
@@ -41,8 +41,10 @@ const shouldAbortOnUncaughtException =
   getOptionValue('--abort-on-uncaught-exception');
 const { abort, exit, _rawDebug } = process;
 
+const kTerminate = () => null;
+
 // From https://url.spec.whatwg.org/#special-scheme
-const SPECIAL_SCHEMES = new SafeSet([
+const kSpecialSchemes = new SafeSet([
   'file:',
   'ftp:',
   'http:',
@@ -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, kTerminate);
+    scopes.set(undefined, kTerminate);
     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);
@@ -416,7 +422,7 @@ class Manifest {
       protocol = currentURL.protocol;
     }
     // Only a few schemes are hierarchical
-    if (SPECIAL_SCHEMES.has(currentURL.protocol)) {
+    if (kSpecialSchemes.has(currentURL.protocol)) {
       // Make first '..' act like '.'
       if (currentURL.pathname.slice(-1) !== '/') {
         currentURL.pathname += '/';

diff --git a/test/fixtures/policy/dependencies/dependencies-scopes-and-resources-policy.json b/test/fixtures/policy/dependencies/dependencies-scopes-and-resources-policy.json
@@ -0,0 +1,14 @@
+{
+  "resources": {
+    "../multi-deps.js": {
+      "integrity": true,
+      "cascade": true
+    }
+  },
+  "scopes": {
+    "../": {
+      "integrity": true,
+      "dependencies": true
+    }
+  }
+}
diff --git a/test/fixtures/policy/multi-deps.js b/test/fixtures/policy/multi-deps.js
@@ -0,0 +1,3 @@
+'use strict';
+require('fs');
+require('process');
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);
+}