From 2bbef4ad87133a5c38ef734b01b11360cd32d60d Mon Sep 17 00:00:00 2001 From: Ji Hoon Kang Date: Sun, 30 Aug 2026 14:09:17 +0900 Subject: [PATCH] permission: do not enforce fs and addons in audit mode --permission-audit is documented to perform permission checks without denying access, publishing each violation through the diagnostics channel while execution continues. Three code paths still enforced: * fs.lstat() and fs.symlink() are checked in the JavaScript layer, which threw ERR_ACCESS_DENIED without consulting audit mode. The isAuditMode() helper introduced for ffi was never wired into fs. * Addon loading was disabled upfront in Environment setup, so process.dlopen() failed with ERR_DLOPEN_DISABLED before reaching the audit-aware permission check, and no audit event was published. Skip the throw in the JavaScript checks when audit mode is on, keeping the permission.has() call first so the diagnostics channel event is still published. Leave native addons enabled in audit mode while still applying the kAddon scope, so DLOpen() publishes the denial through the channel and continues to the regular loading path. This also covers the worker path, which inherits kNoNativeAddons from the same option. Each fs API carries its own copy of the check in its sync, callback and promise flavour, so the regression test exercises all six call sites, asserting the --permission enforce behaviour as a control. Fixes: https://github.com/nodejs/node/issues/65419 Refs: https://github.com/nodejs/node/pull/63047 Assisted-by: claude:opus-5 Signed-off-by: Ji Hoon Kang --- lib/fs.js | 12 +- lib/internal/fs/promises.js | 6 +- src/env.cc | 7 +- ...t-permission-audit-addons-does-not-deny.js | 65 ++++++++ ...on-audit-fs-lstat-symlink-does-not-deny.js | 144 ++++++++++++++++++ 5 files changed, 227 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-permission-audit-addons-does-not-deny.js create mode 100644 test/parallel/test-permission-audit-fs-lstat-symlink-does-not-deny.js diff --git a/lib/fs.js b/lib/fs.js index c7b6b7deb08b..040847cb3a44 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -2019,7 +2019,8 @@ function lstat(path, options = { __proto__: null, bigint: false }, callback) { callback = makeStatsCallback(callback); path = getValidatedPath(path); - if (permission.isEnabled() && !permission.has('fs.read', path)) { + if (permission.isEnabled() && !permission.has('fs.read', path) && + !permission.isAuditMode()) { const resource = BufferIsBuffer(path) ? BufferToString(path) : path; callback(new ERR_ACCESS_DENIED('Access to this API has been restricted', 'FileSystemRead', resource)); return; @@ -2132,7 +2133,8 @@ function lstatSync(path, options = { __proto__: null, bigint: false, throwIfNoEn if (result !== undefined) return result; } path = getValidatedPath(path); - if (permission.isEnabled() && !permission.has('fs.read', path)) { + if (permission.isEnabled() && !permission.has('fs.read', path) && + !permission.isAuditMode()) { const resource = BufferIsBuffer(path) ? BufferToString(path) : path; throw new ERR_ACCESS_DENIED('Access to this API has been restricted', 'FileSystemRead', resource); } @@ -2254,7 +2256,8 @@ function symlink(target, path, type, callback) { // Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass // the permission model security guarantees. Thus, this API is disabled unless fs.read // and fs.write permission has been given. - if (permission.isEnabled() && !permission.has('fs')) { + if (permission.isEnabled() && !permission.has('fs') && + !permission.isAuditMode()) { callback(new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.')); return; } @@ -2328,7 +2331,8 @@ function symlinkSync(target, path, type) { // Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass // the permission model security guarantees. Thus, this API is disabled unless fs.read // and fs.write permission has been given. - if (permission.isEnabled() && !permission.has('fs')) { + if (permission.isEnabled() && !permission.has('fs') && + !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.'); } diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index a9d03ebb1d18..49e312f93fa5 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -1751,7 +1751,8 @@ async function symlink(target, path, type) { // Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass // the permission model security guarantees. Thus, this API is disabled unless fs.read // and fs.write permission has been given. - if (permission.isEnabled() && !permission.has('fs')) { + if (permission.isEnabled() && !permission.has('fs') && + !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.'); } @@ -1796,7 +1797,8 @@ async function lstat(path, options = { __proto__: null, bigint: false }) { if (promise !== undefined) return await promise; } path = getValidatedPath(path); - if (permission.isEnabled() && !permission.has('fs.read', path)) { + if (permission.isEnabled() && !permission.has('fs.read', path) && + !permission.isAuditMode()) { const resource = pathModule.toNamespacedPath(BufferIsBuffer(path) ? BufferToString(path) : path); throw new ERR_ACCESS_DENIED('Access to this API has been restricted', 'FileSystemRead', resource); } diff --git a/src/env.cc b/src/env.cc index 7ec8c50b1aba..405736b8fc7a 100644 --- a/src/env.cc +++ b/src/env.cc @@ -986,7 +986,12 @@ Environment::Environment(IsolateData* isolate_data, // spawn/worker nor use addons or enable inspector // unless explicitly allowed by the user if (!options_->allow_addons) { - options_->allow_native_addons = false; + // In audit mode addon loading must stay enabled: the denial is + // published through the diagnostics channel by the permission + // check in DLOpen() instead of being rejected upfront. + if (!options_->permission_audit) { + options_->allow_native_addons = false; + } permission()->Apply(this, args, permission::PermissionScope::kAddon); } if (!options_->allow_inspector) { diff --git a/test/parallel/test-permission-audit-addons-does-not-deny.js b/test/parallel/test-permission-audit-addons-does-not-deny.js new file mode 100644 index 000000000000..dd08058ab3ff --- /dev/null +++ b/test/parallel/test-permission-audit-addons-does-not-deny.js @@ -0,0 +1,65 @@ +'use strict'; + +const common = require('../common'); +const { isMainThread } = require('worker_threads'); + +if (!isMainThread) { + common.skip('This test only works on a main thread'); +} + +// This test ensures that --permission-audit does not disable native addon +// loading. In audit mode process.dlopen() must reach the regular loading +// path (failing with ERR_DLOPEN_FAILED for a missing file, after publishing +// the denial to the diagnostics channel) instead of being rejected upfront +// with ERR_DLOPEN_DISABLED, which is still the expected behavior under +// --permission. + +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const { test } = require('node:test'); + +function run(flag) { + const childScript = ` + const dc = require('node:diagnostics_channel'); + const msgs = []; + dc.subscribe('node:permission-model:addon', (m) => msgs.push({ + permission: m.permission, + resource: m.resource, + })); + try { + process.dlopen({ exports: {} }, '/nonexistent/audit-test.node'); + console.log('RESULT NO_THROW'); + } catch (e) { + console.log('RESULT THREW ' + e.code); + } + console.log('AUDIT ' + JSON.stringify(msgs)); + `; + + const { status, stdout, stderr } = spawnSync( + process.execPath, + [flag, '-e', childScript], + { encoding: 'utf8' }, + ); + assert.strictEqual(status, 0, stderr); + const lines = stdout.split('\n'); + const resultLine = lines.find((l) => l.startsWith('RESULT ')); + assert.ok(resultLine, stdout); + const auditLine = lines.find((l) => l.startsWith('AUDIT ')); + assert.ok(auditLine, stdout); + return { + result: resultLine.replace('RESULT ', ''), + msgs: JSON.parse(auditLine.replace('AUDIT ', '')), + }; +} + +test('audit mode reaches the real dlopen and logs the denial', () => { + const { result, msgs } = run('--permission-audit'); + assert.strictEqual(result, 'THREW ERR_DLOPEN_FAILED'); + assert.strictEqual(msgs.length, 1); + assert.strictEqual(msgs[0].permission, 'Addon'); +}); + +test('enforce mode still disables dlopen upfront', () => { + const { result } = run('--permission'); + assert.strictEqual(result, 'THREW ERR_DLOPEN_DISABLED'); +}); diff --git a/test/parallel/test-permission-audit-fs-lstat-symlink-does-not-deny.js b/test/parallel/test-permission-audit-fs-lstat-symlink-does-not-deny.js new file mode 100644 index 000000000000..f2d98a0b6f83 --- /dev/null +++ b/test/parallel/test-permission-audit-fs-lstat-symlink-does-not-deny.js @@ -0,0 +1,144 @@ +'use strict'; + +const common = require('../common'); +const { isMainThread } = require('worker_threads'); + +if (!isMainThread) { + common.skip('This test only works on a main thread'); +} + +// This test ensures that --permission-audit does not deny fs.lstat() and +// fs.symlink(), whose permission checks live in the JavaScript layer, while +// --permission still denies them. Audit mode must publish the denial through +// the diagnostics channel and let the operation continue. Each API is covered +// in its three flavours (sync, callback and promise), since every flavour +// carries its own copy of the check. + +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const { test } = require('node:test'); +const fixtures = require('../common/fixtures'); +const tmpdir = require('../common/tmpdir'); + +const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md'); + +function run(flag, { op, name }) { + 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, + })); + (async () => { + try { + await (${op}); + 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, + LINK_PATH: tmpdir.resolve(`audit-symlink-${flag.replace(/\W/g, '')}-${name}`), + }; + const { status, stdout, stderr } = spawnSync( + process.execPath, + [flag, '-e', childScript], + { encoding: 'utf8', env }, + ); + assert.strictEqual(status, 0, stderr); + const lines = stdout.split('\n'); + const resultLine = lines.find((l) => l.startsWith('RESULT ')); + assert.ok(resultLine, stdout); + const auditLine = lines.find((l) => l.startsWith('AUDIT ')); + assert.ok(auditLine, stdout); + return { + result: resultLine.replace('RESULT ', ''), + msgs: JSON.parse(auditLine.replace('AUDIT ', '')), + }; +} + +const lstatOps = [ + { + name: 'lstatSync', + api: 'fs.lstatSync()', + op: 'require("node:fs").lstatSync(process.env.BLOCKED_FILE)', + }, + { + name: 'lstatCallback', + api: 'fs.lstat()', + op: 'new Promise((resolve, reject) => require("node:fs").lstat(' + + 'process.env.BLOCKED_FILE, (err) => err ? reject(err) : resolve()))', + }, + { + name: 'lstatPromises', + api: 'fsPromises.lstat()', + op: 'require("node:fs").promises.lstat(process.env.BLOCKED_FILE)', + }, +]; + +const symlinkOps = [ + { + name: 'symlinkSync', + api: 'fs.symlinkSync()', + op: 'require("node:fs").symlinkSync(process.env.BLOCKED_FILE, ' + + 'process.env.LINK_PATH)', + }, + { + name: 'symlinkCallback', + api: 'fs.symlink()', + op: 'new Promise((resolve, reject) => require("node:fs").symlink(' + + 'process.env.BLOCKED_FILE, process.env.LINK_PATH, ' + + '(err) => err ? reject(err) : resolve()))', + }, + { + name: 'symlinkPromises', + api: 'fsPromises.symlink()', + op: 'require("node:fs").promises.symlink(process.env.BLOCKED_FILE, ' + + 'process.env.LINK_PATH)', + }, +]; + +tmpdir.refresh(); + +for (const entry of lstatOps) { + test(`audit mode does not deny ${entry.api} but logs the denial`, () => { + const { result, msgs } = run('--permission-audit', entry); + assert.strictEqual(result, 'NO_THROW'); + assert.strictEqual(msgs.length, 1); + assert.strictEqual(msgs[0].permission, 'FileSystemRead'); + assert.ok(msgs[0].resource.endsWith('protected-file.md')); + }); + + test(`enforce mode still denies ${entry.api}`, () => { + const { result } = run('--permission', entry); + assert.strictEqual(result, 'THREW ERR_ACCESS_DENIED'); + }); +} + +for (const entry of symlinkOps) { + test(`audit mode does not deny ${entry.api} but logs the denial`, (t) => { + if (!common.canCreateSymLink()) { + return t.skip('insufficient privileges to create symlinks'); + } + const { result, msgs } = run('--permission-audit', entry); + assert.strictEqual(result, 'NO_THROW'); + assert.ok( + msgs.some((m) => m.permission === 'FileSystem'), + JSON.stringify(msgs), + ); + }); + + test(`enforce mode still denies ${entry.api}`, (t) => { + if (!common.canCreateSymLink()) { + return t.skip('insufficient privileges to create symlinks'); + } + const { result } = run('--permission', entry); + assert.strictEqual(result, 'THREW ERR_ACCESS_DENIED'); + }); +}