src: handle duplicate paths granted

This commit fixes a crash whenever someone tries to allow access to the same path twice. PR-URL: #56591 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
nodejs · Jan 16, 2025 · c6960ee · c6960ee
1 parent 0e7ec5e
commit c6960ee
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 2 deletions.
diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc
@@ -143,10 +143,12 @@ void FSPermission::Apply(Environment* env,
 
 void FSPermission::GrantAccess(PermissionScope perm, const std::string& res) {
   const std::string path = WildcardIfDir(res);
-  if (perm == PermissionScope::kFileSystemRead) {
+  if (perm == PermissionScope::kFileSystemRead &&
+      !granted_in_fs_.Lookup(path)) {
     granted_in_fs_.Insert(path);
     deny_all_in_ = false;
-  } else if (perm == PermissionScope::kFileSystemWrite) {
+  } else if (perm == PermissionScope::kFileSystemWrite &&
+             !granted_out_fs_.Lookup(path)) {
     granted_out_fs_.Insert(path);
     deny_all_out_ = false;
   }

diff --git a/test/parallel/test-permission-fs-repeat-path.js b/test/parallel/test-permission-fs-repeat-path.js
@@ -0,0 +1,40 @@
+// Flags: --permission --allow-fs-read=* --allow-child-process
+'use strict';
+
+const common = require('../common');
+const path = require('path');
+common.skipIfWorker();
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+{
+  // Relative path as CLI args are supported
+  const { status, stdout } = spawnSync(
+    process.execPath,
+    [
+      '--permission',
+      '--allow-fs-write', path.resolve('../fixtures/permission/deny/regular-file.md'),
+      '--allow-fs-write', path.resolve('../fixtures/permission/deny/regular-file.md'),
+      '--allow-fs-read', path.resolve('../fixtures/permission/deny/regular-file.md'),
+      '--allow-fs-read', path.resolve('../fixtures/permission/deny/regular-file.md'),
+      '-e',
+      `
+      const path = require("path");
+      const absolutePath = path.resolve("../fixtures/permission/deny/regular-file.md");
+      const blockedPath = path.resolve("../fixtures/permission/deny/protected-file.md");
+      console.log(process.permission.has("fs.write", absolutePath));
+      console.log(process.permission.has("fs.read", absolutePath));
+      console.log(process.permission.has("fs.read", blockedPath));
+      console.log(process.permission.has("fs.write", blockedPath));
+       `,
+    ]
+  );
+
+  const [fsWrite, fsRead, fsBlockedRead, fsBlockedWrite] = stdout.toString().split('\n');
+  assert.strictEqual(status, 0);
+  assert.strictEqual(fsWrite, 'true');
+  assert.strictEqual(fsRead, 'true');
+  assert.strictEqual(fsBlockedRead, 'false');
+  assert.strictEqual(fsBlockedWrite, 'false');
+}