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: https://github.com/nodejs/node/commit/9ddd1a9c27c253f46d587a8c906ccd83417b4606
Signed-off-by: Adrian Estrada <edsadr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/64426
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>

src/permission/permission.h

@@ -37,9 +37,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)
 
@@ -51,9 +53,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)
 
@@ -65,14 +69,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');
+});

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

@@ -0,0 +1,69 @@
+'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 net = require('net');
+
+async function runAudit(mode) {
+  const server = net.createServer();
+
+  await new Promise((resolve, reject) => {
+    server.on('error', reject);
+    server.listen(0, '127.0.0.1', resolve);
+  });
+  const { port } = server.address();
+  const env = { ...process.env, PORT: String(port), HOST: '127.0.0.1' };
+
+  const childScript = `
+    const dc = require('node:diagnostics_channel');
+    const net = require('node:net');
+    const msgs = [];
+    dc.subscribe('node:permission-model:net', (m) => msgs.push({
+      permission: m.permission,
+      resource: m.resource,
+    }));
+    const s = ${mode === 'eval' ?
+      `eval('net.connect(Number(process.env.PORT), process.env.HOST)')` :
+      `net.connect(Number(process.env.PORT), process.env.HOST)`};
+    s.on('connect', () => { s.destroy(); console.log('RESULT CONNECTED'); });
+    s.on('error', (e) => { console.log('RESULT ERROR ' + e.code); });
+    s.on('close', () => {
+      console.log('AUDIT ' + JSON.stringify(msgs));
+    });
+  `;
+
+  try {
+    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.strictEqual(lines[0], 'RESULT CONNECTED', 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, 'Net');
+    assert.ok(msgs[0].resource.includes('127.0.0.1'));
+  } finally {
+    server.close();
+  }
+}
+
+test('permission-audit logs net denial without blocking connect', async () => {
+  await runAudit('direct');
+});
+
+test('permission-audit logs net denial without blocking connect (eval)', async () => {
+  await runAudit('eval');
+});