X Tutup
The Wayback Machine - https://web.archive.org/web/20250718060957/https://github.com/nodejs/node/commit/2beb4f1f8c
Skip to content

Commit 2beb4f1

Browse files
RafaelGSSruyadorno
RafaelGSS
authored and
ruyadorno
committed
permission: ignore internalModuleStat on module loading
This improves Permission Model usage when allowing read access to specifi modules. To achieve that, the permission model check on internalModuleStat has been removed meaning that on module loading, uv_fs_stat is performed on files and folders even when the permission model is enabled. Although a uv_fs_stat is performed, reading/executing the module will still pass by the permission model check. Without this PR when an app tries to --allow-fs-read=./a.js --allow-fs-read=./b.js where `a` attempt to load b, it will fails as it reads $pwd and no permission has been given to this path. PR-URL: #55797 Backport-PR-URL: #56058 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
1 parent 77e2869 commit 2beb4f1

File tree

11 files changed

+93
-57
lines changed

11 files changed

+93
-57
lines changed

‎doc/api/cli.md

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -219,15 +219,8 @@ The initializer module also needs to be allowed. Consider the following example:
219219

220220
```console
221221
$ node --experimental-permission t.js
222-
node:internal/modules/cjs/loader:162
223-
const result = internalModuleStat(receiver, filename);
224-
^
225222

226223
Error: Access to this API has been restricted
227-
at stat (node:internal/modules/cjs/loader:162:18)
228-
at Module._findPath (node:internal/modules/cjs/loader:640:16)
229-
at resolveMainPath (node:internal/modules/run_main:15:25)
230-
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:53:24)
231224
at node:internal/main/run_main_module:23:47 {
232225
code: 'ERR_ACCESS_DENIED',
233226
permission: 'FileSystemRead',
@@ -298,18 +291,8 @@ new WASI({
298291

299292
```console
300293
$ node --experimental-permission --allow-fs-read=* index.js
301-
node:wasi:99
302-
const wrap = new _WASI(args, env, preopens, stdio);
303-
^
304294

305295
Error: Access to this API has been restricted
306-
at new WASI (node:wasi:99:18)
307-
at Object.<anonymous> (/home/index.js:3:1)
308-
at Module._compile (node:internal/modules/cjs/loader:1476:14)
309-
at Module._extensions..js (node:internal/modules/cjs/loader:1555:10)
310-
at Module.load (node:internal/modules/cjs/loader:1288:32)
311-
at Module._load (node:internal/modules/cjs/loader:1104:12)
312-
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:191:14)
313296
at node:internal/main/run_main_module:30:49 {
314297
code: 'ERR_ACCESS_DENIED',
315298
permission: 'WASI',
@@ -339,18 +322,8 @@ new Worker(__filename);
339322

340323
```console
341324
$ node --experimental-permission --allow-fs-read=* index.js
342-
node:internal/worker:188
343-
this[kHandle] = new WorkerImpl(url,
344-
^
345325

346326
Error: Access to this API has been restricted
347-
at new Worker (node:internal/worker:188:21)
348-
at Object.<anonymous> (/home/index.js.js:3:1)
349-
at Module._compile (node:internal/modules/cjs/loader:1120:14)
350-
at Module._extensions..js (node:internal/modules/cjs/loader:1174:10)
351-
at Module.load (node:internal/modules/cjs/loader:998:32)
352-
at Module._load (node:internal/modules/cjs/loader:839:12)
353-
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
354327
at node:internal/main/run_main_module:17:47 {
355328
code: 'ERR_ACCESS_DENIED',
356329
permission: 'WorkerThreads'

‎doc/api/permissions.md

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,8 @@ will be restricted.
4747

4848
```console
4949
$ node --experimental-permission index.js
50-
node:internal/modules/cjs/loader:171
51-
const result = internalModuleStat(receiver, filename);
52-
^
5350

5451
Error: Access to this API has been restricted
55-
at stat (node:internal/modules/cjs/loader:171:18)
56-
at Module._findPath (node:internal/modules/cjs/loader:627:16)
57-
at resolveMainPath (node:internal/modules/run_main:19:25)
58-
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:24)
5952
at node:internal/main/run_main_module:23:47 {
6053
code: 'ERR_ACCESS_DENIED',
6154
permission: 'FileSystemRead',

‎lib/internal/modules/cjs/loader.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,6 @@ const packageJsonReader = require('internal/modules/package_json_reader');
159159
const { getOptionValue, getEmbedderOptions } = require('internal/options');
160160
const shouldReportRequiredModules = getLazy(() => process.env.WATCH_REPORT_DEPENDENCIES);
161161

162-
const permission = require('internal/process/permission');
163162
const {
164163
vm_dynamic_import_default_internal,
165164
} = internalBinding('symbols');
@@ -736,11 +735,8 @@ Module._findPath = function(request, paths, isMain) {
736735
// For each path
737736
for (let i = 0; i < paths.length; i++) {
738737
// Don't search further if path doesn't exist
739-
// or doesn't have permission to it
740738
const curPath = paths[i];
741-
if (insidePath && curPath &&
742-
((permission.isEnabled() && !permission.has('fs.read', curPath)) || _stat(curPath) < 1)
743-
) {
739+
if (insidePath && curPath && _stat(curPath) < 1) {
744740
continue;
745741
}
746742

‎src/node_file.cc

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,8 @@ static void ExistsSync(const FunctionCallbackInfo<Value>& args) {
10311031
// Used to speed up module loading. Returns 0 if the path refers to
10321032
// a file, 1 when it's a directory or < 0 on error (usually -ENOENT.)
10331033
// The speedup comes from not creating thousands of Stat and Error objects.
1034+
// Do not expose this function through public API as it doesn't hold
1035+
// Permission Model checks.
10341036
static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
10351037
Environment* env = Environment::GetCurrent(args);
10361038

@@ -1039,8 +1041,6 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
10391041
BufferValue path(env->isolate(), args[1]);
10401042
CHECK_NOT_NULL(*path);
10411043
ToNamespacedPath(env, &path);
1042-
THROW_IF_INSUFFICIENT_PERMISSIONS(
1043-
env, permission::PermissionScope::kFileSystemRead, path.ToStringView());
10441044

10451045
uv_fs_t req;
10461046
int rc = uv_fs_stat(env->event_loop(), &req, *path, nullptr);
@@ -1067,15 +1067,8 @@ static int32_t FastInternalModuleStat(
10671067
}
10681068

10691069
HandleScope scope(isolate);
1070-
Environment* env = Environment::GetCurrent(recv->GetCreationContextChecked());
10711070

10721071
auto path = std::filesystem::path(input.data, input.data + input.length);
1073-
if (!env->permission()->is_granted(
1074-
env, permission::PermissionScope::kFileSystemRead, path.string()))
1075-
[[unlikely]] {
1076-
options.fallback = true;
1077-
return -1;
1078-
}
10791072

10801073
switch (std::filesystem::status(path).type()) {
10811074
case std::filesystem::file_type::directory:

‎test/fixtures/permission/fs-read.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,13 @@ const regularFile = __filename;
282282
permission: 'FileSystemRead',
283283
resource: path.toNamespacedPath(blockedFolder),
284284
}));
285+
assert.throws(() => {
286+
fs.readdirSync(blockedFolder, { recursive: true });
287+
}, common.expectsError({
288+
code: 'ERR_ACCESS_DENIED',
289+
permission: 'FileSystemRead',
290+
resource: path.toNamespacedPath(blockedFolder),
291+
}));
285292
assert.throws(() => {
286293
fs.readdirSync(blockedFolder);
287294
}, common.expectsError({

‎test/fixtures/permission/main-module.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
require('./required-module');

‎test/fixtures/permission/main-module.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import './required-module.mjs';

‎test/fixtures/permission/required-module.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
console.log('ok');

‎test/fixtures/permission/required-module.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
console.log('ok');

‎test/parallel/test-permission-fs-internal-module-stat.js

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,14 @@ if (!common.hasCrypto) {
99
}
1010

1111
const { internalBinding } = require('internal/test/binding');
12-
const assert = require('node:assert');
13-
const path = require('node:path');
1412
const fixtures = require('../common/fixtures');
1513

1614
const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md');
1715
const internalFsBinding = internalBinding('fs');
1816

1917
// Run this inside a for loop to trigger the fast API
2018
for (let i = 0; i < 10_000; i++) {
21-
assert.throws(() => {
22-
internalFsBinding.internalModuleStat(internalFsBinding, blockedFile);
23-
}, {
24-
code: 'ERR_ACCESS_DENIED',
25-
permission: 'FileSystemRead',
26-
resource: path.toNamespacedPath(blockedFile),
27-
});
19+
// internalModuleStat does not use permission model.
20+
// doesNotThrow
21+
internalFsBinding.internalModuleStat(internalFsBinding, blockedFile);
2822
}

0 commit comments

Comments
 (0)
X Tutup