[v20.x] permission: ignore internalModuleStat on module loading

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',

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',

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;
     }
 

src/node_file.cc

@@ -1045,15 +1045,15 @@ static void ExistsSync(const FunctionCallbackInfo<Value>& 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<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();
   uv_loop_t* loop = env->event_loop();
 
   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));
@@ -1149,8 +1149,6 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
 
   CHECK(args[0]->IsString());
   node::Utf8Value path(env->isolate(), args[0]);
-  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);

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({

test/fixtures/permission/main-module.js

@@ -0,0 +1 @@
+require('./required-module');

test/fixtures/permission/main-module.mjs

@@ -0,0 +1 @@
+import './required-module.mjs';

test/fixtures/permission/required-module.js

@@ -0,0 +1 @@
+console.log('ok');

test/fixtures/permission/required-module.mjs

@@ -0,0 +1 @@
+console.log('ok');

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(blockedFile);
+}

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/);
+}