Commit e8c1160d202 for nodejs
commit e8c1160d202f324122689c9a7f24905bb73839e2
Author: Rafael Gonzaga <rafael.nunu@hotmail.com>
Date: Tue Nov 10 22:56:58 2026 -0300
permission: prefer --permission over audit mode
When both --permission and --permission-audit are set, run in enforce
mode as documented, instead of only reporting denials and keeping the
environment variables.
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/67036
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
diff --git a/lib/internal/process/permission.js b/lib/internal/process/permission.js
index 92e54fbab49..f3ed889e8c1 100644
--- a/lib/internal/process/permission.js
+++ b/lib/internal/process/permission.js
@@ -53,7 +53,9 @@ module.exports = ObjectFreeze({
isAuditMode() {
if (_audit === undefined) {
const { getOptionValue } = require('internal/options');
- _audit = getOptionValue('--permission-audit');
+ // --permission takes precedence over --permission-audit.
+ _audit = getOptionValue('--permission-audit') &&
+ !getOptionValue('--permission');
}
return _audit;
},
diff --git a/src/env.cc b/src/env.cc
index f14500da9d6..5f8e22073fa 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -1111,7 +1111,9 @@ Environment::Environment(IsolateData* isolate_data,
if (options_->permission || options_->permission_audit) {
permission()->EnablePermissions();
static const std::array args = {std::string("*")};
- if (options_->permission_audit) {
+ // --permission takes precedence over --permission-audit.
+ const bool audit_only = options_->permission_audit && !options_->permission;
+ if (audit_only) {
permission()->EnableWarningOnly();
}
// The process shouldn't be able to neither
@@ -1121,7 +1123,7 @@ Environment::Environment(IsolateData* isolate_data,
// 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) {
+ if (!audit_only) {
options_->allow_native_addons = false;
}
permission()->Apply(this, args, permission::PermissionScope::kAddon);
diff --git a/src/node.cc b/src/node.cc
index 0b5b305d688..8f1a3eaf498 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -1412,7 +1412,8 @@ InitializeOncePerProcessInternal(
permission::ParseEnvAllowList(env_options->allow_env);
const bool allow_all =
std::find(allow.begin(), allow.end(), "*") != allow.end();
- if (env_options->permission_audit && !allow_all) {
+ if (env_options->permission_audit && !env_options->permission &&
+ !allow_all) {
// In audit mode nothing is removed, but accesses to the variables that
// enforcing the permission model would remove are published.
permission::RecordAuditedEnvironmentVariables(allow);
diff --git a/test/parallel/test-permission-audit-fs-does-not-deny.js b/test/parallel/test-permission-audit-fs-does-not-deny.js
index 13284802839..c1200076a3e 100644
--- a/test/parallel/test-permission-audit-fs-does-not-deny.js
+++ b/test/parallel/test-permission-audit-fs-does-not-deny.js
@@ -57,3 +57,34 @@ test('permission-audit logs fs denial without throwing', () => {
test('permission-audit logs fs denial without throwing (eval)', () => {
runAudit('eval');
});
+
+// --permission takes precedence over --permission-audit, so the read is denied.
+function runEnforced(args, extraEnv) {
+ const { status, stdout, stderr } = spawnSync(
+ process.execPath,
+ [...args, '-e', `
+ try {
+ require('node:fs').readFileSync(${JSON.stringify(blockedFile)});
+ console.log('RESULT NO_THROW');
+ } catch (e) {
+ console.log('RESULT THREW ' + e.code);
+ }
+ `],
+ { encoding: 'utf8', env: { ...process.env, ...extraEnv } },
+ );
+ assert.strictEqual(status, 0, stderr);
+ assert.ok(stdout.split('\n').includes('RESULT THREW ERR_ACCESS_DENIED'),
+ stdout);
+}
+
+test('--permission takes precedence over --permission-audit', () => {
+ runEnforced(['--permission', '--permission-audit']);
+});
+
+test('--permission takes precedence over --permission-audit in NODE_OPTIONS', {
+ skip: process.config.variables.node_without_node_options &&
+ 'missing NODE_OPTIONS support',
+}, () => {
+ runEnforced(['--permission'], { NODE_OPTIONS: '--permission-audit' });
+ runEnforced(['--permission-audit'], { NODE_OPTIONS: '--permission' });
+});
diff --git a/test/parallel/test-permission-env-scrub.js b/test/parallel/test-permission-env-scrub.js
index c5d86a87f7a..26a6e71fedf 100644
--- a/test/parallel/test-permission-env-scrub.js
+++ b/test/parallel/test-permission-env-scrub.js
@@ -125,6 +125,16 @@ function run(...flags) {
assert.strictEqual(result.hasAll, false);
}
+// --permission takes precedence over --permission-audit.
+{
+ const result = run(
+ '--permission', '--permission-audit', '--allow-env=PERMISSION_ENV_ALLOWED');
+ assert.deepStrictEqual(result.keys, ['PERMISSION_ENV_ALLOWED', 'TZ']);
+ assert.strictEqual(result.has.PERMISSION_ENV_SECRET, false);
+ assert.strictEqual(result.has.PERMISSION_ENV_ALLOWED, true);
+ assert.strictEqual(result.hasAll, false);
+}
+
// --allow-env requires the permission model.
spawnSyncAndExit(
process.execPath,