From 33ae0038a0054a8d40b591e093a1755fc4ce9784 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Wed, 14 Oct 2026 08:05:30 -0300 Subject: [PATCH] fs: do not disable fd-based APIs in permission audit mode Refs: https://github.com/nodejs/node/pull/65659 Refs: https://github.com/nodejs/node/issues/65419 Signed-off-by: RafaelGSS --- doc/api/permissions.md | 3 + lib/fs.js | 22 ++--- lib/internal/fs/promises.js | 12 +-- ...rmission-audit-fs-fd-apis-does-not-deny.js | 86 +++++++++++++++++++ 4 files changed, 108 insertions(+), 15 deletions(-) create mode 100644 test/parallel/test-permission-audit-fs-fd-apis-does-not-deny.js diff --git a/doc/api/permissions.md b/doc/api/permissions.md index 386671b339dc..dedf1f6b63fd 100644 --- a/doc/api/permissions.md +++ b/doc/api/permissions.md @@ -273,6 +273,9 @@ both to the top-level `node:fs` functions and to the equivalent `FileHandle` methods, and currently includes `fsync`/`fdatasync`, `fchmod`, `fchown`, and `futimes` (and their synchronous variants). +In [audit mode](#audit-mode), these operations are not disabled and run normally. No +diagnostics channel message is published for them. + #### Environment variable permissions When the Permission Model is enforced, the process only has access to the diff --git a/lib/fs.js b/lib/fs.js index 4436fa2df6e6..4f6180e3996d 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1609,7 +1609,9 @@ function fdatasync(fd, callback) { const req = new FSReqCallback(); req.oncomplete = callback; - if (permission.isEnabled()) { + // TODO(rafaelgss): publish a diagnostics channel event for fd-based APIs in + // audit mode. + if (permission.isEnabled() && !permission.isAuditMode()) { callback(new ERR_ACCESS_DENIED('fdatasync API is disabled when Permission Model is enabled.')); return; } @@ -1630,7 +1632,7 @@ function fdatasyncSync(fd) { if (result !== undefined) return; } - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('fdatasync API is disabled when Permission Model is enabled.'); } binding.fdatasync(fd); @@ -1651,7 +1653,7 @@ function fsync(fd, callback) { const req = new FSReqCallback(); req.oncomplete = callback; - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { callback(new ERR_ACCESS_DENIED('fsync API is disabled when Permission Model is enabled.')); return; } @@ -1671,7 +1673,7 @@ function fsyncSync(fd) { if (result !== undefined) return; } - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('fsync API is disabled when Permission Model is enabled.'); } binding.fsync(fd); @@ -2452,7 +2454,7 @@ function fchmod(fd, mode, callback) { const h = vfsState.handlers; if (h !== null && vfsVoid(h.fchmod(fd, mode), callback)) return; - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { callback(new ERR_ACCESS_DENIED('fchmod API is disabled when Permission Model is enabled.')); return; } @@ -2477,7 +2479,7 @@ function fchmodSync(fd, mode) { if (result !== undefined) return; } - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('fchmod API is disabled when Permission Model is enabled.'); } binding.fchmod(fd, mode); @@ -2628,7 +2630,7 @@ function fchown(fd, uid, gid, callback) { const h = vfsState.handlers; if (h !== null && vfsVoid(h.fchown(fd), callback)) return; - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { callback(new ERR_ACCESS_DENIED('fchown API is disabled when Permission Model is enabled.')); return; } @@ -2655,7 +2657,7 @@ function fchownSync(fd, uid, gid) { if (result !== undefined) return; } - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('fchown API is disabled when Permission Model is enabled.'); } @@ -2769,7 +2771,7 @@ function futimes(fd, atime, mtime, callback) { const h = vfsState.handlers; if (h !== null && vfsVoid(h.futimes(fd, atime, mtime), callback)) return; - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { callback(new ERR_ACCESS_DENIED('futimes API is disabled when Permission Model is enabled.')); return; } @@ -2798,7 +2800,7 @@ function futimesSync(fd, atime, mtime) { if (result !== undefined) return; } - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('futimes API is disabled when Permission Model is enabled.'); } diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index c21b1e5740fc..4f6700a4a03b 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -1679,7 +1679,9 @@ async function rmdir(path, options) { } async function fdatasync(handle) { - if (permission.isEnabled()) { + // TODO(rafaelgss): publish a diagnostics channel event for fd-based APIs in + // audit mode. + if (permission.isEnabled() && !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('fdatasync API is disabled when Permission Model is enabled.'); } return await PromisePrototypeThen( @@ -1690,7 +1692,7 @@ async function fdatasync(handle) { } async function fsync(handle) { - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('fsync API is disabled when Permission Model is enabled.'); } return await PromisePrototypeThen( @@ -1964,7 +1966,7 @@ async function unlink(path) { } async function fchmod(handle, mode) { - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('fchmod API is disabled when Permission Model is enabled.'); } mode = parseFileMode(mode, 'mode'); @@ -2026,7 +2028,7 @@ async function lchown(path, uid, gid) { async function fchown(handle, uid, gid) { validateInteger(uid, 'uid', -1, kMaxUserId); validateInteger(gid, 'gid', -1, kMaxUserId); - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('fchown API is disabled when Permission Model is enabled.'); } return await PromisePrototypeThen( @@ -2072,7 +2074,7 @@ async function utimes(path, atime, mtime) { } async function futimes(handle, atime, mtime) { - if (permission.isEnabled()) { + if (permission.isEnabled() && !permission.isAuditMode()) { throw new ERR_ACCESS_DENIED('futimes API is disabled when Permission Model is enabled.'); } atime = toUnixTimestamp(atime, 'atime'); diff --git a/test/parallel/test-permission-audit-fs-fd-apis-does-not-deny.js b/test/parallel/test-permission-audit-fs-fd-apis-does-not-deny.js new file mode 100644 index 000000000000..5ea4cb2ae0da --- /dev/null +++ b/test/parallel/test-permission-audit-fs-fd-apis-does-not-deny.js @@ -0,0 +1,86 @@ +'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 fs = require('fs'); +const { spawnSync } = require('child_process'); +const { test } = require('node:test'); +const fixtures = require('../common/fixtures'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +const target = tmpdir.resolve('audit-fd-apis.txt'); +fs.copyFileSync(fixtures.path('permission', 'deny', 'regular-file.md'), target); + +function run(flag, op) { + const childScript = ` + const fs = require('node:fs'); + (async () => { + const fd = fs.openSync(process.env.TARGET, 'r+'); + const fh = await fs.promises.open(process.env.TARGET, 'r+'); + const cb = (fn, ...args) => new Promise((resolve, reject) => { + fn(fd, ...args, (err) => err ? reject(err) : resolve()); + }); + try { + await (${op}); + console.log('RESULT NO_THROW'); + } catch (e) { + console.log('RESULT THREW ' + e.code); + } finally { + await fh.close(); + fs.closeSync(fd); + } + })(); + `; + const { status, stdout, stderr } = spawnSync( + process.execPath, + [ + flag, + '--allow-fs-read=*', + '--allow-fs-write=*', + '--allow-env=TARGET', + '-e', + childScript, + ], + { encoding: 'utf8', env: { ...process.env, TARGET: target } }, + ); + assert.strictEqual(status, 0, stderr); + const resultLine = stdout.split('\n').find((l) => l.startsWith('RESULT ')); + assert.ok(resultLine, stdout); + return resultLine.replace('RESULT ', ''); +} + +const ops = [ + ['fs.fsyncSync()', 'fs.fsyncSync(fd)'], + ['fs.fsync()', 'cb(fs.fsync)'], + ['filehandle.sync()', 'fh.sync()'], + ['fs.fdatasyncSync()', 'fs.fdatasyncSync(fd)'], + ['fs.fdatasync()', 'cb(fs.fdatasync)'], + ['filehandle.datasync()', 'fh.datasync()'], + ['fs.fchmodSync()', 'fs.fchmodSync(fd, 0o644)'], + ['fs.fchmod()', 'cb(fs.fchmod, 0o644)'], + ['filehandle.chmod()', 'fh.chmod(0o644)'], + ['fs.fchownSync()', 'fs.fchownSync(fd, -1, -1)'], + ['fs.fchown()', 'cb(fs.fchown, -1, -1)'], + ['filehandle.chown()', 'fh.chown(-1, -1)'], + ['fs.futimesSync()', 'fs.futimesSync(fd, 1, 1)'], + ['fs.futimes()', 'cb(fs.futimes, 1, 1)'], + ['filehandle.utimes()', 'fh.utimes(1, 1)'], +]; + +for (const [api, op] of ops) { + test(`audit mode does not disable ${api}`, () => { + assert.strictEqual(run('--permission-audit', op), 'NO_THROW'); + }); + + test(`enforce mode still disables ${api}`, () => { + assert.strictEqual(run('--permission', op), 'THREW ERR_ACCESS_DENIED'); + }); +}