permission: keep explicit fs grants when dropping a covering directory

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>

Author: RafaelGSS

src/permission/fs_permission.cc

@@ -218,15 +218,26 @@ void FSPermission::RebuildTree(PermissionScope scope) {
 
 void FSPermission::GrantAccess(PermissionScope perm, const std::string& res) {
   const std::string path = WildcardIfDir(res);
-  if (perm == PermissionScope::kFileSystemRead &&
-      !granted_in_fs_.Lookup(path)) {
-    granted_in_fs_.Insert(path);
-    granted_paths_in_.push_back(path);
+  // Track every explicit grant, even when already covered by another one,
+  // so that it still applies if the covering grant is dropped later.
+  if (perm == PermissionScope::kFileSystemRead) {
+    if (std::find(granted_paths_in_.begin(), granted_paths_in_.end(), path) ==
+        granted_paths_in_.end()) {
+      granted_paths_in_.push_back(path);
+    }
+    if (!granted_in_fs_.Lookup(path)) {
+      granted_in_fs_.Insert(path);
+    }
     deny_all_in_ = false;
-  } else if (perm == PermissionScope::kFileSystemWrite &&
-             !granted_out_fs_.Lookup(path)) {
-    granted_out_fs_.Insert(path);
-    granted_paths_out_.push_back(path);
+  } else if (perm == PermissionScope::kFileSystemWrite) {
+    if (std::find(granted_paths_out_.begin(),
+                  granted_paths_out_.end(),
+                  path) == granted_paths_out_.end()) {
+      granted_paths_out_.push_back(path);
+    }
+    if (!granted_out_fs_.Lookup(path)) {
+      granted_out_fs_.Insert(path);
+    }
     deny_all_out_ = false;
   }
 }

test/parallel/test-permission-drop-fs-granted-path.js

@@ -133,6 +133,40 @@ fs.writeFileSync(path.join(dir, 'item2.txt'), 'bbb');
   assert.strictEqual(child.status, 0);
 }
 
+// Grant a directory and a file inside it separately, drop the directory
+// the explicit file grant still applies regardless of the flag order
+for (const files of [
+  [dir, path.join(dir, 'item1.txt')],
+  [path.join(dir, 'item1.txt'), dir],
+]) {
+  const child = spawnSync(process.execPath, [
+    '--permission',
+    ...files.map((file) => `--allow-fs-read=${file}`),
+    ...files.map((file) => `--allow-fs-write=${file}`),
+    '-e',
+    `
+      const assert = require('assert');
+      const fs = require('fs');
+      const dir = ${JSON.stringify(dir)};
+
+      for (const scope of ['fs.read', 'fs.write']) {
+        assert.ok(process.permission.has(scope, dir + '/item1.txt'));
+        assert.ok(process.permission.has(scope, dir + '/item2.txt'));
+
+        process.permission.drop(scope, dir);
+
+        assert.ok(process.permission.has(scope, dir + '/item1.txt'));
+        assert.ok(!process.permission.has(scope, dir + '/item2.txt'));
+      }
+      assert.strictEqual(fs.readFileSync(dir + '/item1.txt', 'utf8'), 'aaa');
+    `,
+  ]);
+  if (child.status !== 0) {
+    console.error('Case 5 stderr:', child.stderr?.toString());
+  }
+  assert.strictEqual(child.status, 0);
+}
+
 // Drop entire scope without reference - revokes everything
 {
   const child = spawnSync(process.execPath, [