Skip to content

Commit 7b468bb

Browse files
committed
src,permission: add --permission-audit
Add --permission-audit flag that enables the permission model in warning-only mode. Instead of throwing ERR_ACCESS_DENIED, it emits a message via diagnostics channel and allows the operation to continue. Publish permission check results to per-scope diagnostics channels (e.g., node:permission-model:fs) so users can observe permission decisions at runtime via diagnostics_channel. Refs: #59935
1 parent b487456 commit 7b468bb

File tree

10 files changed

+155
-12
lines changed

10 files changed

+155
-12
lines changed

doc/api/cli.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2174,6 +2174,16 @@ following permissions are restricted:
21742174
* WASI - manageable through [`--allow-wasi`][] flag
21752175
* Addons - manageable through [`--allow-addons`][] flag
21762176

2177+
### `--permission-audit`
2178+
2179+
<!-- YAML
2180+
added: REPLACEME
2181+
-->
2182+
2183+
Enable audit only for the permission model. When enabled, permission checks
2184+
are performed but access is not denied. Instead, a warning is emitted for
2185+
each permission violation via diagnostics channel.
2186+
21772187
### `--preserve-symlinks`
21782188

21792189
<!-- YAML
@@ -3660,6 +3670,7 @@ one is included in the list below.
36603670
* `--openssl-legacy-provider`
36613671
* `--openssl-shared-config`
36623672
* `--pending-deprecation`
3673+
* `--permission-audit`
36633674
* `--permission`
36643675
* `--preserve-symlinks-main`
36653676
* `--preserve-symlinks`

doc/node.1

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,6 +1115,11 @@ WASI - manageable through \fB--allow-wasi\fR flag
11151115
Addons - manageable through \fB--allow-addons\fR flag
11161116
.El
11171117
.
1118+
.It Fl -permission-audit
1119+
Enable audit only for the permission model. When enabled, permission checks
1120+
are performed but access is not denied. Instead, a warning is emitted for
1121+
each permission violation via diagnostics channel.
1122+
.
11181123
.It Fl -preserve-symlinks
11191124
Instructs the module loader to preserve symbolic links when resolving and
11201125
caching modules.
@@ -1978,6 +1983,8 @@ one is included in the list below.
19781983
.It
19791984
\fB--permission\fR
19801985
.It
1986+
\fB--permission-audit\fR
1987+
.It
19811988
\fB--preserve-symlinks-main\fR
19821989
.It
19831990
\fB--preserve-symlinks\fR

lib/internal/process/pre_execution.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ function prepareExecution(options) {
124124
// Process initial diagnostic reporting configuration, if present.
125125
initializeReport();
126126

127+
setupDiagnosticsChannel();
128+
127129
// Load permission system API
128130
initializePermission();
129131

@@ -619,8 +621,22 @@ function initializeClusterIPC() {
619621
}
620622
}
621623

624+
function setupDiagnosticsChannel() {
625+
// The C++ Channel::Publish() API needs a JS callback to dispatch
626+
// messages. This callback is stored in BindingData but cleared during
627+
// snapshot serialization. Since lib/diagnostics_channel.js module body
628+
// may not re-run after snapshot deserialization, we must explicitly
629+
// re-register it here.
630+
const dc = require('diagnostics_channel');
631+
const dc_binding = internalBinding('diagnostics_channel');
632+
dc_binding.setPublishCallback((name, message) => {
633+
const ch = dc.channel(name);
634+
if (ch.hasSubscribers) ch.publish(message);
635+
});
636+
}
637+
622638
function initializePermission() {
623-
const permission = getOptionValue('--permission');
639+
const permission = getOptionValue('--permission') || getOptionValue('--permission-audit');
624640
if (permission) {
625641
process.binding = function binding(_module) {
626642
throw new ERR_ACCESS_DENIED('process.binding');

src/env.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -918,8 +918,11 @@ Environment::Environment(IsolateData* isolate_data,
918918
tracing::CastTracedValue(traced_value));
919919
}
920920

921-
if (options_->permission) {
921+
if (options_->permission || options_->permission_audit) {
922922
permission()->EnablePermissions();
923+
if (options_->permission_audit) {
924+
permission()->EnableWarningOnly();
925+
}
923926
// The process shouldn't be able to neither
924927
// spawn/worker nor use addons or enable inspector
925928
// unless explicitly allowed by the user

src/node_options.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
635635
kAllowedInEnvvar,
636636
false,
637637
OptionNamespaces::kPermissionNamespace);
638+
AddOption("--permission-audit",
639+
"enable audit only for the permission system",
640+
&EnvironmentOptions::permission_audit,
641+
kAllowedInEnvvar,
642+
false);
638643
AddOption("--allow-fs-read",
639644
"allow permissions to read the filesystem",
640645
&EnvironmentOptions::allow_fs_read,

src/node_options.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ class EnvironmentOptions : public Options {
138138
std::string input_type; // Value of --input-type
139139
bool entry_is_url = false;
140140
bool permission = false;
141+
bool permission_audit = false;
141142
std::vector<std::string> allow_fs_read;
142143
std::vector<std::string> allow_fs_write;
143144
bool allow_addons = false;

src/permission/permission.cc

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "env-inl.h"
44
#include "memory_tracker-inl.h"
55
#include "node.h"
6+
#include "node_diagnostics_channel.h"
67
#include "node_errors.h"
78
#include "node_external_reference.h"
89
#include "node_file.h"
@@ -27,6 +28,29 @@ namespace permission {
2728

2829
namespace {
2930

31+
const char* GetDiagnosticsChannelName(PermissionScope scope) {
32+
switch (scope) {
33+
case PermissionScope::kFileSystem:
34+
case PermissionScope::kFileSystemRead:
35+
case PermissionScope::kFileSystemWrite:
36+
return "node:permission-model:fs";
37+
case PermissionScope::kChildProcess:
38+
return "node:permission-model:child";
39+
case PermissionScope::kWorkerThreads:
40+
return "node:permission-model:worker";
41+
case PermissionScope::kNet:
42+
return "node:permission-model:net";
43+
case PermissionScope::kInspector:
44+
return "node:permission-model:inspector";
45+
case PermissionScope::kWASI:
46+
return "node:permission-model:wasi";
47+
case PermissionScope::kAddon:
48+
return "node:permission-model:addon";
49+
default:
50+
return nullptr;
51+
}
52+
}
53+
3054
// permission.has('fs.in', '/tmp/')
3155
// permission.has('fs.in')
3256
static void Has(const FunctionCallbackInfo<Value>& args) {
@@ -70,7 +94,7 @@ PermissionScope Permission::StringToPermission(const std::string& perm) {
7094
}
7195
#undef V
7296

73-
Permission::Permission() : enabled_(false) {
97+
Permission::Permission() : enabled_(false), warning_only_(false) {
7498
std::shared_ptr<PermissionBase> fs = std::make_shared<FSPermission>();
7599
std::shared_ptr<PermissionBase> child_p =
76100
std::make_shared<ChildProcessPermission>();
@@ -175,6 +199,53 @@ void Permission::EnablePermissions() {
175199
}
176200
}
177201

202+
void Permission::EnableWarningOnly() {
203+
if (!warning_only_) {
204+
warning_only_ = true;
205+
}
206+
}
207+
208+
bool Permission::is_scope_granted(Environment* env,
209+
const PermissionScope permission,
210+
const std::string_view& res) const {
211+
auto perm_node = nodes_.find(permission);
212+
bool result = false;
213+
if (perm_node != nodes_.end()) {
214+
result = perm_node->second->is_granted(env, permission, res);
215+
}
216+
217+
if (!result && !publishing_) {
218+
const char* channel_name = GetDiagnosticsChannelName(permission);
219+
if (channel_name != nullptr) {
220+
diagnostics_channel::Channel ch =
221+
diagnostics_channel::Channel::Get(env, channel_name);
222+
if (ch.HasSubscribers()) {
223+
publishing_ = true;
224+
v8::Isolate* isolate = env->isolate();
225+
v8::HandleScope handle_scope(isolate);
226+
v8::Local<v8::Context> context = env->context();
227+
v8::Local<v8::Object> msg = v8::Object::New(isolate);
228+
const char* perm_str = PermissionToString(permission);
229+
msg->Set(context,
230+
FIXED_ONE_BYTE_STRING(isolate, "permission"),
231+
v8::String::NewFromUtf8(isolate, perm_str).ToLocalChecked())
232+
.Check();
233+
msg->Set(context,
234+
FIXED_ONE_BYTE_STRING(isolate, "resource"),
235+
v8::String::NewFromUtf8(
236+
isolate, res.data(), v8::NewStringType::kNormal,
237+
static_cast<int>(res.size()))
238+
.ToLocalChecked())
239+
.Check();
240+
ch.Publish(env, msg);
241+
publishing_ = false;
242+
}
243+
}
244+
}
245+
246+
return result;
247+
}
248+
178249
void Permission::Apply(Environment* env,
179250
const std::vector<std::string>& allow,
180251
PermissionScope scope) {

src/permission/permission.h

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ namespace permission {
3737
[[unlikely]] { \
3838
node::permission::Permission::ThrowAccessDenied( \
3939
env__, perm__, resource__); \
40-
return __VA_ARGS__; \
40+
if (!env__->permission()->warning_only()) return __VA_ARGS__; \
4141
} \
4242
} while (0)
4343

@@ -51,7 +51,7 @@ namespace permission {
5151
[[unlikely]] { \
5252
node::permission::Permission::AsyncThrowAccessDenied( \
5353
env__, (wrap), perm__, resource__); \
54-
return __VA_ARGS__; \
54+
if (!env__->permission()->warning_only()) return __VA_ARGS__; \
5555
} \
5656
} while (0)
5757

@@ -100,6 +100,8 @@ class Permission {
100100

101101
FORCE_INLINE bool enabled() const { return enabled_; }
102102

103+
FORCE_INLINE bool warning_only() const { return warning_only_; }
104+
103105
static PermissionScope StringToPermission(const std::string& perm);
104106
static const char* PermissionToString(PermissionScope perm);
105107
static void ThrowAccessDenied(Environment* env,
@@ -115,20 +117,17 @@ class Permission {
115117
const std::vector<std::string>& allow,
116118
PermissionScope scope);
117119
void EnablePermissions();
120+
void EnableWarningOnly();
118121

119122
private:
120123
COLD_NOINLINE bool is_scope_granted(Environment* env,
121124
const PermissionScope permission,
122-
const std::string_view& res = "") const {
123-
auto perm_node = nodes_.find(permission);
124-
if (perm_node != nodes_.end()) {
125-
return perm_node->second->is_granted(env, permission, res);
126-
}
127-
return false;
128-
}
125+
const std::string_view& res = "") const;
129126

130127
std::unordered_map<PermissionScope, std::shared_ptr<PermissionBase>> nodes_;
131128
bool enabled_;
129+
bool warning_only_;
130+
mutable bool publishing_ = false;
132131
};
133132

134133
v8::MaybeLocal<v8::Value> CreateAccessDeniedError(Environment* env,

test/parallel/test-bootstrap-modules.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ expected.beforePreExec = new Set([
105105
'Internal Binding module_wrap',
106106
'NativeModule internal/modules/cjs/loader',
107107
'NativeModule diagnostics_channel',
108+
'Internal Binding diagnostics_channel',
108109
'Internal Binding wasm_web_api',
109110
'NativeModule internal/events/abort_listener',
110111
'NativeModule internal/modules/typescript',
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Flags: --permission --allow-fs-read=*
2+
'use strict';
3+
4+
const common = require('../common');
5+
const { isMainThread } = require('worker_threads');
6+
7+
if (!isMainThread) {
8+
common.skip('This test only works on a main thread');
9+
}
10+
11+
const assert = require('node:assert');
12+
const dc = require('node:diagnostics_channel');
13+
const fs = require('node:fs');
14+
15+
const messages = [];
16+
dc.subscribe('node:permission-model:fs', (msg) => {
17+
messages.push(msg);
18+
});
19+
20+
// Granted permission should not publish
21+
fs.readFileSync(__filename);
22+
assert.strictEqual(messages.length, 0, 'Granted checks should not publish');
23+
24+
// Denied permission should publish
25+
const hasWrite = process.permission.has('fs.write', '/tmp/test');
26+
assert.strictEqual(hasWrite, false);
27+
28+
assert.ok(messages.length > 0, 'Expected at least one denied message');
29+
assert.strictEqual(messages[0].permission, 'FileSystemWrite');

0 commit comments

Comments
 (0)