diff --git a/doc/api/cli.md b/doc/api/cli.md index 85920e71be2cc5..0656c6e35b200d 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -217,15 +217,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(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', diff --git a/doc/api/permissions.md b/doc/api/permissions.md index 80516643e890d4..4a17d949721241 100644 --- a/doc/api/permissions.md +++ b/doc/api/permissions.md @@ -500,15 +500,8 @@ will be restricted. ```console $ node --experimental-permission index.js -node:internal/modules/cjs/loader:171 - const result = internalModuleStat(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 ebccdb28256314..33e80234ce8ff1 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -164,7 +164,6 @@ const policy = getLazy( ); const shouldReportRequiredModules = getLazy(() => process.env.WATCH_REPORT_DEPENDENCIES); -const permission = require('internal/process/permission'); const { vm_dynamic_import_default_internal, } = internalBinding('symbols'); @@ -704,11 +703,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 39a70311f4ad21..cb3361ef789f25 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1045,6 +1045,8 @@ static void ExistsSync(const FunctionCallbackInfo& args) { } // Used to speed up module loading. Returns an array [string, boolean] +// Do not expose this function through public API as it doesn't hold +// Permission Model checks. static void InternalModuleReadJSON(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); @@ -1052,8 +1054,6 @@ static void InternalModuleReadJSON(const FunctionCallbackInfo& args) { CHECK(args[0]->IsString()); node::Utf8Value path(isolate, args[0]); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemRead, path.ToStringView()); if (strlen(*path) != path.length()) { args.GetReturnValue().Set(Array::New(isolate)); diff --git a/test/fixtures/permission/fs-read.js b/test/fixtures/permission/fs-read.js index 03261d975ab94c..5679387cfd6275 100644 --- a/test/fixtures/permission/fs-read.js +++ b/test/fixtures/permission/fs-read.js @@ -281,6 +281,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 new file mode 100644 index 00000000000000..488ccb72494912 --- /dev/null +++ b/test/parallel/test-permission-fs-internal-module-stat.js @@ -0,0 +1,19 @@ +// Flags: --expose-internals --experimental-permission --allow-fs-read=test/common* --allow-fs-read=tools* --allow-fs-read=test/parallel* --allow-child-process +'use strict'; + +const common = require('../common'); +common.skipIfWorker(); + +if (!common.hasCrypto) { + common.skip('no crypto'); +} + +const { internalBinding } = require('internal/test/binding'); +const fixtures = require('../common/fixtures'); + +const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md'); +const internalFsBinding = internalBinding('fs'); + +{ + 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/); +}