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