src,permission: do not throw on denied access in audit mode

The THROW_IF_INSUFFICIENT_PERMISSIONS and
ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS macros called
ThrowAccessDenied/AsyncThrowAccessDenied unconditionally and only
guarded the `return` with `warning_only()`.
ERR_ACCESS_DENIED_IF_INSUFFICIENT_PERMISSIONS had no `warning_only()`
guard at all — it always set the access-denied error and returned.
As a result, running with `--permission-audit` still produced
ERR_ACCESS_DENIED on any denied operation (fs, net, child_process,
worker, addon, ffi, inspector, wasi), defeating the audit-only purpose
of the flag.

Guard the denied-error path behind `!warning_only()` in all three
macros. In audit mode, the diagnostics-channel message is published
(already done in Permission::is_scope_granted) and execution continues;
in enforce mode (`--permission`), behavior is unchanged — the error is
raised and the call returns.

The tests cover both the direct (top-level) call and an `eval()`-wrapped
call: the direct call exercises the normal script path, and the
`eval()`-wrapped call exercises the V8 script-context boundary (the
diagnostics subscriber is registered in the outer module context while
the denied operation runs inside an eval'd string).

Refs: 9ddd1a9
Signed-off-by: Adrian Estrada <edsadr@gmail.com>
PR-URL: #64426
Backport-PR-URL: #65354
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Author: edsadr

src/permission/permission.h

@@ -35,9 +35,11 @@ namespace permission {
     const auto resource__ = (resource);                                        \
     if (!env__->permission()->is_granted(env__, perm__, resource__))           \
         [[unlikely]] {                                                         \
-      node::permission::Permission::ThrowAccessDenied(                         \
-          env__, perm__, resource__);                                          \
-      if (!env__->permission()->warning_only()) return __VA_ARGS__;            \
+      if (!env__->permission()->warning_only()) {                              \
+        node::permission::Permission::ThrowAccessDenied(                       \
+            env__, perm__, resource__);                                        \
+        return __VA_ARGS__;                                                    \
+      }                                                                        \
     }                                                                          \
   } while (0)
 
@@ -49,9 +51,11 @@ namespace permission {
     const auto resource__ = (resource);                                        \
     if (!env__->permission()->is_granted(env__, perm__, resource__))           \
         [[unlikely]] {                                                         \
-      node::permission::Permission::AsyncThrowAccessDenied(                    \
-          env__, (wrap), perm__, resource__);                                  \
-      if (!env__->permission()->warning_only()) return __VA_ARGS__;            \
+      if (!env__->permission()->warning_only()) {                              \
+        node::permission::Permission::AsyncThrowAccessDenied(                  \
+            env__, (wrap), perm__, resource__);                                \
+        return __VA_ARGS__;                                                    \
+      }                                                                        \
     }                                                                          \
   } while (0)
 
@@ -63,14 +67,17 @@ namespace permission {
     const auto resource__ = (resource);                                        \
     if (!env__->permission()->is_granted(env__, perm__, resource__))           \
         [[unlikely]] {                                                         \
-      Local<Value> err_access;                                                 \
-      if (node::permission::CreateAccessDeniedError(env__, perm__, resource__) \
-              .ToLocal(&err_access)) {                                         \
-        args.GetReturnValue().Set(err_access);                                 \
-      } else {                                                                 \
-        args.GetReturnValue().Set(UV_EACCES);                                  \
+      if (!env__->permission()->warning_only()) {                              \
+        Local<Value> err_access;                                               \
+        if (node::permission::CreateAccessDeniedError(                         \
+                env__, perm__, resource__)                                     \
+                .ToLocal(&err_access)) {                                       \
+          args.GetReturnValue().Set(err_access);                               \
+        } else {                                                               \
+          args.GetReturnValue().Set(UV_EACCES);                                \
+        }                                                                      \
+        return __VA_ARGS__;                                                    \
       }                                                                        \
-      return __VA_ARGS__;                                                      \
     }                                                                          \
   } while (0)
 

test/parallel/test-permission-audit-fs-does-not-deny.js

@@ -0,0 +1,59 @@
+'use strict';
+
+const common = require('../common');
+const { isMainThread } = require('worker_threads');
+
+if (!isMainThread) {
+  common.skip('This test only works on a main thread');
+}
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+const { test } = require('node:test');
+const fixtures = require('../common/fixtures');
+
+const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md');
+
+function runAudit(mode) {
+  const childScript = `
+    const dc = require('node:diagnostics_channel');
+    const msgs = [];
+    dc.subscribe('node:permission-model:fs', (m) => msgs.push({
+      permission: m.permission,
+      resource: m.resource,
+    }));
+    try {
+      ${mode === 'eval' ?
+        `eval('require("node:fs").readFileSync(process.env.BLOCKED_FILE)');` :
+        `require('node:fs').readFileSync(process.env.BLOCKED_FILE);`}
+      console.log('RESULT NO_THROW');
+    } catch (e) {
+      console.log('RESULT THREW ' + e.code);
+    }
+    console.log('AUDIT ' + JSON.stringify(msgs));
+  `;
+
+  const env = { ...process.env, BLOCKED_FILE: blockedFile };
+  const { status, stdout, stderr } = spawnSync(
+    process.execPath,
+    ['--permission-audit', '-e', childScript],
+    { encoding: 'utf8', env },
+  );
+  assert.strictEqual(status, 0, stderr);
+  const lines = stdout.split('\n');
+  assert.ok(lines.includes('RESULT NO_THROW'), stdout);
+  const auditLine = lines.find((l) => l.startsWith('AUDIT '));
+  assert.ok(auditLine, stdout);
+  const msgs = JSON.parse(auditLine.replace('AUDIT ', ''));
+  assert.strictEqual(msgs.length, 1);
+  assert.strictEqual(msgs[0].permission, 'FileSystemRead');
+  assert.ok(msgs[0].resource.endsWith('protected-file.md'));
+}
+
+test('permission-audit logs fs denial without throwing', () => {
+  runAudit('direct');
+});
+
+test('permission-audit logs fs denial without throwing (eval)', () => {
+  runAudit('eval');
+});