diff --git a/doc/api/cli.md b/doc/api/cli.md index e166252d80e45c..2e2578e883e903 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -219,15 +219,8 @@ The initializer module also needs to be allowed. Consider the following example: ```console $ node --experimental-permission t.js -node:internal/modules/cjs/loader:162 - const result = internalModuleStat(receiver, filename); - ^ Error: Access to this API has been restricted - at stat (node:internal/modules/cjs/loader:162:18) - at Module._findPath (node:internal/modules/cjs/loader:640:16) - at resolveMainPath (node:internal/modules/run_main:15:25) - at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:53:24) at node:internal/main/run_main_module:23:47 { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemRead', @@ -300,18 +293,8 @@ new WASI({ ```console $ node --experimental-permission --allow-fs-read=* index.js -node:wasi:99 - const wrap = new _WASI(args, env, preopens, stdio); - ^ Error: Access to this API has been restricted - at new WASI (node:wasi:99:18) - at Object. (/home/index.js:3:1) - at Module._compile (node:internal/modules/cjs/loader:1476:14) - at Module._extensions..js (node:internal/modules/cjs/loader:1555:10) - at Module.load (node:internal/modules/cjs/loader:1288:32) - at Module._load (node:internal/modules/cjs/loader:1104:12) - at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:191:14) at node:internal/main/run_main_module:30:49 { code: 'ERR_ACCESS_DENIED', permission: 'WASI', @@ -341,18 +324,8 @@ new Worker(__filename); ```console $ node --experimental-permission --allow-fs-read=* index.js -node:internal/worker:188 - this[kHandle] = new WorkerImpl(url, - ^ Error: Access to this API has been restricted - at new Worker (node:internal/worker:188:21) - at Object. (/home/index.js.js:3:1) - at Module._compile (node:internal/modules/cjs/loader:1120:14) - at Module._extensions..js (node:internal/modules/cjs/loader:1174:10) - at Module.load (node:internal/modules/cjs/loader:998:32) - at Module._load (node:internal/modules/cjs/loader:839:12) - at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12) at node:internal/main/run_main_module:17:47 { code: 'ERR_ACCESS_DENIED', permission: 'WorkerThreads' diff --git a/doc/api/permissions.md b/doc/api/permissions.md index e37c2982bc146a..a03285e28641e8 100644 --- a/doc/api/permissions.md +++ b/doc/api/permissions.md @@ -47,15 +47,8 @@ will be restricted. ```console $ node --experimental-permission index.js -node:internal/modules/cjs/loader:171 - const result = internalModuleStat(receiver, filename); - ^ Error: Access to this API has been restricted - at stat (node:internal/modules/cjs/loader:171:18) - at Module._findPath (node:internal/modules/cjs/loader:627:16) - at resolveMainPath (node:internal/modules/run_main:19:25) - at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:24) at node:internal/main/run_main_module:23:47 { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemRead', diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 3f161daa7f3c55..95763de0701581 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -160,7 +160,6 @@ const packageJsonReader = require('internal/modules/package_json_reader'); const { getOptionValue, getEmbedderOptions } = require('internal/options'); const shouldReportRequiredModules = getLazy(() => process.env.WATCH_REPORT_DEPENDENCIES); -const permission = require('internal/process/permission'); const { vm_dynamic_import_default_internal, } = internalBinding('symbols'); @@ -729,11 +728,8 @@ Module._findPath = function(request, paths, isMain) { // For each path for (let i = 0; i < paths.length; i++) { // Don't search further if path doesn't exist - // or doesn't have permission to it const curPath = paths[i]; - if (insidePath && curPath && - ((permission.isEnabled() && !permission.has('fs.read', curPath)) || _stat(curPath) < 1) - ) { + if (insidePath && curPath && _stat(curPath) < 1) { continue; } diff --git a/src/node_file.cc b/src/node_file.cc index dfdc25fd1d8986..5a50aacb1b939d 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1037,6 +1037,8 @@ static void ExistsSync(const FunctionCallbackInfo& args) { // Used to speed up module loading. Returns 0 if the path refers to // a file, 1 when it's a directory or < 0 on error (usually -ENOENT.) // The speedup comes from not creating thousands of Stat and Error objects. +// Do not expose this function through public API as it doesn't hold +// Permission Model checks. static void InternalModuleStat(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -1045,8 +1047,6 @@ static void InternalModuleStat(const FunctionCallbackInfo& args) { BufferValue path(env->isolate(), args[1]); CHECK_NOT_NULL(*path); ToNamespacedPath(env, &path); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemRead, path.ToStringView()); uv_fs_t req; int rc = uv_fs_stat(env->event_loop(), &req, *path, nullptr); @@ -1069,8 +1069,6 @@ static int32_t FastInternalModuleStat( HandleScope scope(env->isolate()); auto path = std::filesystem::path(input.data, input.data + input.length); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemRead, path.string(), -1); switch (std::filesystem::status(path).type()) { case std::filesystem::file_type::directory: diff --git a/test/fixtures/permission/fs-read.js b/test/fixtures/permission/fs-read.js index b189af295793e6..186117a6b768dd 100644 --- a/test/fixtures/permission/fs-read.js +++ b/test/fixtures/permission/fs-read.js @@ -282,6 +282,13 @@ const regularFile = __filename; permission: 'FileSystemRead', resource: path.toNamespacedPath(blockedFolder), })); + assert.throws(() => { + fs.readdirSync(blockedFolder, { recursive: true }); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFolder), + })); assert.throws(() => { fs.readdirSync(blockedFolder); }, common.expectsError({ diff --git a/test/fixtures/permission/main-module.js b/test/fixtures/permission/main-module.js new file mode 100644 index 00000000000000..cac52e04dddd24 --- /dev/null +++ b/test/fixtures/permission/main-module.js @@ -0,0 +1 @@ +require('./required-module'); \ No newline at end of file diff --git a/test/fixtures/permission/main-module.mjs b/test/fixtures/permission/main-module.mjs new file mode 100644 index 00000000000000..e7c28f7f6cab19 --- /dev/null +++ b/test/fixtures/permission/main-module.mjs @@ -0,0 +1 @@ +import './required-module.mjs'; \ No newline at end of file diff --git a/test/fixtures/permission/required-module.js b/test/fixtures/permission/required-module.js new file mode 100644 index 00000000000000..e8dbf442c5b1a2 --- /dev/null +++ b/test/fixtures/permission/required-module.js @@ -0,0 +1 @@ +console.log('ok'); \ No newline at end of file diff --git a/test/fixtures/permission/required-module.mjs b/test/fixtures/permission/required-module.mjs new file mode 100644 index 00000000000000..e8dbf442c5b1a2 --- /dev/null +++ b/test/fixtures/permission/required-module.mjs @@ -0,0 +1 @@ +console.log('ok'); \ No newline at end of file diff --git a/test/parallel/test-permission-fs-internal-module-stat.js b/test/parallel/test-permission-fs-internal-module-stat.js index 2f000f4814a0ca..f0b9d86f0809a8 100644 --- a/test/parallel/test-permission-fs-internal-module-stat.js +++ b/test/parallel/test-permission-fs-internal-module-stat.js @@ -9,8 +9,6 @@ if (!common.hasCrypto) { } const { internalBinding } = require('internal/test/binding'); -const assert = require('node:assert'); -const path = require('node:path'); const fixtures = require('../common/fixtures'); const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md'); @@ -18,11 +16,7 @@ const internalFsBinding = internalBinding('fs'); // Run this inside a for loop to trigger the fast API for (let i = 0; i < 10_000; i++) { - assert.throws(() => { - internalFsBinding.internalModuleStat(internalFsBinding, blockedFile); - }, { - code: 'ERR_ACCESS_DENIED', - permission: 'FileSystemRead', - resource: path.toNamespacedPath(blockedFile), - }); + // internalModuleStat does not use permission model. + // doesNotThrow + internalFsBinding.internalModuleStat(internalFsBinding, blockedFile); } diff --git a/test/parallel/test-permission-fs-require.js b/test/parallel/test-permission-fs-require.js new file mode 100644 index 00000000000000..6a2e9201dac7b4 --- /dev/null +++ b/test/parallel/test-permission-fs-require.js @@ -0,0 +1,76 @@ +// Flags: --experimental-permission --allow-fs-read=* --allow-child-process +'use strict'; + +const common = require('../common'); +common.skipIfWorker(); +const fixtures = require('../common/fixtures'); + +const assert = require('node:assert'); +const { spawnSync } = require('node:child_process'); + +{ + const mainModule = fixtures.path('permission', 'main-module.js'); + const requiredModule = fixtures.path('permission', 'required-module.js'); + const { status, stdout, stderr } = spawnSync( + process.execPath, + [ + '--experimental-permission', + '--allow-fs-read', mainModule, + '--allow-fs-read', requiredModule, + mainModule, + ] + ); + + assert.strictEqual(status, 0, stderr.toString()); + assert.strictEqual(stdout.toString(), 'ok\n'); +} + +{ + // When required module is not passed as allowed path + const mainModule = fixtures.path('permission', 'main-module.js'); + const { status, stderr } = spawnSync( + process.execPath, + [ + '--experimental-permission', + '--allow-fs-read', mainModule, + mainModule, + ] + ); + + assert.strictEqual(status, 1, stderr.toString()); + assert.match(stderr.toString(), /Error: Access to this API has been restricted/); +} + +{ + // ESM loader test + const mainModule = fixtures.path('permission', 'main-module.mjs'); + const requiredModule = fixtures.path('permission', 'required-module.mjs'); + const { status, stdout, stderr } = spawnSync( + process.execPath, + [ + '--experimental-permission', + '--allow-fs-read', mainModule, + '--allow-fs-read', requiredModule, + mainModule, + ] + ); + + assert.strictEqual(status, 0, stderr.toString()); + assert.strictEqual(stdout.toString(), 'ok\n'); +} + +{ + // When required module is not passed as allowed path (ESM) + const mainModule = fixtures.path('permission', 'main-module.mjs'); + const { status, stderr } = spawnSync( + process.execPath, + [ + '--experimental-permission', + '--allow-fs-read', mainModule, + mainModule, + ] + ); + + assert.strictEqual(status, 1, stderr.toString()); + assert.match(stderr.toString(), /Error: Access to this API has been restricted/); +}