From 5a63c62a43ded0fe2ee02308658ecbd78898e2b3 Mon Sep 17 00:00:00 2001 From: Benedict Date: Mon, 19 Oct 2026 20:58:14 +0530 Subject: [PATCH 1/2] permission: keep directories after radix tree split Granting two directories that share a prefix, such as /srv/app and /srv/appx, splits the radix tree node that holds the first directory's `/*` grant. The directory itself then stops being an end node, so fs.stat(), fs.readdir(), fs.access() and process.permission.has() deny access to an explicitly granted directory. Treat a node with an exact `/*` wildcard child as an end node, since granting `/*` also grants the directory itself. Fixes: https://github.com/nodejs/node/issues/66812 Signed-off-by: Benedict Chacko Assisted-by: opencode --- src/permission/fs_permission.h | 17 +++++- .../test-permission-fs-prefix-split.js | 54 +++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-permission-fs-prefix-split.js diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h index 2a5c838c14c..abe0a90881a 100644 --- a/src/permission/fs_permission.h +++ b/src/permission/fs_permission.h @@ -167,7 +167,22 @@ class FSPermission final : public PermissionBase { if (children.empty()) { return true; } - return is_leaf; + if (is_leaf) { + return true; + } + // A directory is also granted when only its `/*` child is + // granted. This usually ends at this node, but inserting a sibling + // that shares a prefix can split the node into a `` node with + // separate `/*` and `/*` children. + for (const auto& [c, child] : children) { + if (child->prefix.size() == 2 && + child->prefix[0] == node::kPathSeparator && + child->prefix[1] == '*' && + child->wildcard_child != nullptr) { + return true; + } + } + return false; } }; diff --git a/test/parallel/test-permission-fs-prefix-split.js b/test/parallel/test-permission-fs-prefix-split.js new file mode 100644 index 00000000000..41238bce25b --- /dev/null +++ b/test/parallel/test-permission-fs-prefix-split.js @@ -0,0 +1,54 @@ +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const { spawnSync } = require('child_process'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +const app = path.join(tmpdir.path, 'app'); +const appx = path.join(tmpdir.path, 'appx'); +const denied = path.join(tmpdir.path, 'ap'); +fs.mkdirSync(app); +fs.mkdirSync(appx); +fs.writeFileSync(path.join(app, 'file.txt'), 'hello'); + +const script = ` + const assert = require('assert'); + const fs = require('fs'); + const path = require('path'); + const app = ${JSON.stringify(app)}; + const appx = ${JSON.stringify(appx)}; + const denied = ${JSON.stringify(denied)}; + const file = path.join(app, 'file.txt'); + + for (const scope of ['fs.read', 'fs.write']) { + assert.strictEqual(process.permission.has(scope, app), true); + assert.strictEqual(process.permission.has(scope, appx), true); + assert.strictEqual(process.permission.has(scope, file), true); + assert.strictEqual(process.permission.has(scope, denied), false); + } + + assert.strictEqual(fs.existsSync(app), true); + assert.strictEqual(fs.statSync(app).isDirectory(), true); + assert.ok(fs.readdirSync(app).includes('file.txt')); + + fs.writeFileSync(file, 'updated'); + assert.strictEqual(fs.readFileSync(file, 'utf8'), 'updated'); +`; + +// Granting two sibling directories used to split the radix tree node holding +// the first directory's `/*` grant, which denied access to the directory +// itself. Check both insertion orders. +for (const grants of [[app, appx], [appx, app]]) { + const { status, stderr } = spawnSync(process.execPath, [ + '--permission', + ...grants.map((grant) => `--allow-fs-read=${grant}`), + ...grants.map((grant) => `--allow-fs-write=${grant}`), + '-e', + script, + ]); + assert.strictEqual(status, 0, stderr.toString()); +} From e82313a285dfe56f355654e6cd30beb2f35f1275 Mon Sep 17 00:00:00 2001 From: Benedict Date: Mon, 19 Oct 2026 21:07:22 +0530 Subject: [PATCH 2/2] permission: handle wildcard split at the path separator Nested grants (child first, then parent) split the radix tree inside the trailing `/*`, leaving the directory node with a trailing separator and a `*` child. Looking up the parent then failed because the wildcard was one level below the node and the node path had one extra character. Recognize both wildcard suffixes in IsEndNode() and accept a node whose path only appends the separator during lookup. Refs: https://github.com/nodejs/node/issues/66812 Signed-off-by: Benedict Chacko Assisted-by: opencode --- src/permission/fs_permission.cc | 8 ++++- src/permission/fs_permission.h | 17 ++++----- .../test-permission-fs-prefix-split.js | 35 +++++++++++++++---- 3 files changed, 45 insertions(+), 15 deletions(-) diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc index 0baf1291a96..e36bbf317b6 100644 --- a/src/permission/fs_permission.cc +++ b/src/permission/fs_permission.cc @@ -285,7 +285,13 @@ bool FSPermission::RadixTree::Lookup(std::string_view s, auto path_len = s.length(); while (true) { - if (parent_node_prefix_len == path_len && current_node->IsEndNode()) { + // A split can leave the directory node with a trailing separator and + // the wildcard one level below it. See IsEndNode(). + if ((parent_node_prefix_len == path_len || + (parent_node_prefix_len == path_len + 1 && + !current_node->prefix.empty() && + current_node->prefix.back() == node::kPathSeparator)) && + current_node->IsEndNode()) { return true; } diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h index abe0a90881a..4ee80d7fbf8 100644 --- a/src/permission/fs_permission.h +++ b/src/permission/fs_permission.h @@ -170,15 +170,16 @@ class FSPermission final : public PermissionBase { if (is_leaf) { return true; } - // A directory is also granted when only its `/*` child is - // granted. This usually ends at this node, but inserting a sibling - // that shares a prefix can split the node into a `` node with - // separate `/*` and `/*` children. + // Granting `/*` also grants ``, but a sibling that shares + // a prefix can split the tree and detach the wildcard child from + // the end node. Treat the node as an end node when a direct child + // is the remaining wildcard suffix (`*` or `/*`). for (const auto& [c, child] : children) { - if (child->prefix.size() == 2 && - child->prefix[0] == node::kPathSeparator && - child->prefix[1] == '*' && - child->wildcard_child != nullptr) { + if (child->wildcard_child == nullptr) continue; + if (child->prefix == "*" || + (child->prefix.size() == 2 && + child->prefix[0] == node::kPathSeparator && + child->prefix[1] == '*')) { return true; } } diff --git a/test/parallel/test-permission-fs-prefix-split.js b/test/parallel/test-permission-fs-prefix-split.js index 41238bce25b..a3e43ac7e77 100644 --- a/test/parallel/test-permission-fs-prefix-split.js +++ b/test/parallel/test-permission-fs-prefix-split.js @@ -10,10 +10,13 @@ tmpdir.refresh(); const app = path.join(tmpdir.path, 'app'); const appx = path.join(tmpdir.path, 'appx'); +const nested = path.join(app, 'nested'); const denied = path.join(tmpdir.path, 'ap'); fs.mkdirSync(app); fs.mkdirSync(appx); +fs.mkdirSync(nested); fs.writeFileSync(path.join(app, 'file.txt'), 'hello'); +fs.writeFileSync(path.join(nested, 'file.txt'), 'hello'); const script = ` const assert = require('assert'); @@ -21,34 +24,54 @@ const script = ` const path = require('path'); const app = ${JSON.stringify(app)}; const appx = ${JSON.stringify(appx)}; + const nested = ${JSON.stringify(nested)}; const denied = ${JSON.stringify(denied)}; + const expected = JSON.parse(process.argv[1]); const file = path.join(app, 'file.txt'); + const nestedFile = path.join(nested, 'file.txt'); for (const scope of ['fs.read', 'fs.write']) { assert.strictEqual(process.permission.has(scope, app), true); - assert.strictEqual(process.permission.has(scope, appx), true); + assert.strictEqual(process.permission.has(scope, appx), expected.appx); + assert.strictEqual(process.permission.has(scope, nested), true); assert.strictEqual(process.permission.has(scope, file), true); + assert.strictEqual(process.permission.has(scope, nestedFile), true); assert.strictEqual(process.permission.has(scope, denied), false); } assert.strictEqual(fs.existsSync(app), true); assert.strictEqual(fs.statSync(app).isDirectory(), true); - assert.ok(fs.readdirSync(app).includes('file.txt')); + assert.deepStrictEqual(fs.readdirSync(app).sort(), ['file.txt', 'nested']); + + assert.strictEqual(fs.existsSync(nested), true); + assert.strictEqual(fs.statSync(nested).isDirectory(), true); + assert.deepStrictEqual(fs.readdirSync(nested), ['file.txt']); fs.writeFileSync(file, 'updated'); assert.strictEqual(fs.readFileSync(file, 'utf8'), 'updated'); + fs.writeFileSync(nestedFile, 'updated'); + assert.strictEqual(fs.readFileSync(nestedFile, 'utf8'), 'updated'); `; -// Granting two sibling directories used to split the radix tree node holding -// the first directory's `/*` grant, which denied access to the directory -// itself. Check both insertion orders. -for (const grants of [[app, appx], [appx, app]]) { +// Granting directories that share a prefix can split the radix tree node +// holding a `/*` grant, denying access to the directory itself. Cover +// sibling grants in both orders and nested grants where the parent is +// inserted after the child. +const cases = [ + { grants: [app, appx], appx: true }, + { grants: [appx, app], appx: true }, + { grants: [nested, app], appx: false }, + { grants: [app, nested], appx: false }, +]; + +for (const { grants, appx: hasAppx } of cases) { const { status, stderr } = spawnSync(process.execPath, [ '--permission', ...grants.map((grant) => `--allow-fs-read=${grant}`), ...grants.map((grant) => `--allow-fs-write=${grant}`), '-e', script, + JSON.stringify({ appx: hasAppx }), ]); assert.strictEqual(status, 0, stderr.toString()); }