Skip to content

Commit 4db6e58

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 f1d5005 commit 4db6e58

File tree

10 files changed

+168
-12
lines changed

10 files changed

+168
-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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ function setupDiagnosticsChannel() {
630630
}
631631

632632
function initializePermission() {
633-
const permission = getOptionValue('--permission');
633+
const permission = getOptionValue('--permission') || getOptionValue('--permission-audit');
634634
if (permission) {
635635
process.binding = function binding(_module) {
636636
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: 92 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+
constexpr std::string_view 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 {};
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,73 @@ 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+
auto channel_name = GetDiagnosticsChannelName(permission);
219+
if (!channel_name.empty()) {
220+
auto ch = GetOrCreateChannel(env, permission);
221+
if (ch && ch->HasSubscribers()) {
222+
publishing_ = true;
223+
v8::Isolate* isolate = env->isolate();
224+
v8::HandleScope handle_scope(isolate);
225+
v8::Local<v8::Context> context = env->context();
226+
v8::Local<v8::Object> msg = v8::Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0);
227+
const char* perm_str = PermissionToString(permission);
228+
msg->Set(context,
229+
FIXED_ONE_BYTE_STRING(isolate, "permission"),
230+
v8::String::NewFromUtf8(isolate, perm_str).ToLocalChecked())
231+
.Check();
232+
msg->Set(context,
233+
FIXED_ONE_BYTE_STRING(isolate, "resource"),
234+
v8::String::NewFromUtf8(isolate,
235+
res.data(),
236+
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+
249+
BaseObjectPtr<diagnostics_channel::Channel>
250+
Permission::GetOrCreateChannel(
251+
Environment* env, PermissionScope scope) const {
252+
auto it = channels_.find(scope);
253+
if (it != channels_.end()) {
254+
// Promote weak ref to strong for the duration of this call.
255+
BaseObjectPtr<diagnostics_channel::Channel> ptr(it->second.get());
256+
if (ptr) return ptr;
257+
channels_.erase(it);
258+
}
259+
auto channel_name = GetDiagnosticsChannelName(scope);
260+
diagnostics_channel::Channel* ch =
261+
diagnostics_channel::Channel::Get(env, channel_name.data());
262+
if (ch != nullptr) {
263+
channels_.emplace(scope, BaseObjectWeakPtr<diagnostics_channel::Channel>(ch));
264+
return BaseObjectPtr<diagnostics_channel::Channel>(ch);
265+
}
266+
return {};
267+
}
268+
178269
void Permission::Apply(Environment* env,
179270
const std::vector<std::string>& allow,
180271
PermissionScope scope) {

src/permission/permission.h

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "permission/permission_base.h"
1414
#include "permission/wasi_permission.h"
1515
#include "permission/worker_permission.h"
16+
#include "node_diagnostics_channel.h"
1617
#include "v8.h"
1718

1819
#include <string_view>
@@ -37,7 +38,7 @@ namespace permission {
3738
[[unlikely]] { \
3839
node::permission::Permission::ThrowAccessDenied( \
3940
env__, perm__, resource__); \
40-
return __VA_ARGS__; \
41+
if (!env__->permission()->warning_only()) return __VA_ARGS__; \
4142
} \
4243
} while (0)
4344

@@ -51,7 +52,7 @@ namespace permission {
5152
[[unlikely]] { \
5253
node::permission::Permission::AsyncThrowAccessDenied( \
5354
env__, (wrap), perm__, resource__); \
54-
return __VA_ARGS__; \
55+
if (!env__->permission()->warning_only()) return __VA_ARGS__; \
5556
} \
5657
} while (0)
5758

@@ -100,6 +101,8 @@ class Permission {
100101

101102
FORCE_INLINE bool enabled() const { return enabled_; }
102103

104+
FORCE_INLINE bool warning_only() const { return warning_only_; }
105+
103106
static PermissionScope StringToPermission(const std::string& perm);
104107
static const char* PermissionToString(PermissionScope perm);
105108
static void ThrowAccessDenied(Environment* env,
@@ -115,20 +118,25 @@ class Permission {
115118
const std::vector<std::string>& allow,
116119
PermissionScope scope);
117120
void EnablePermissions();
121+
void EnableWarningOnly();
118122

119123
private:
120124
COLD_NOINLINE bool is_scope_granted(Environment* env,
121125
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-
}
126+
const std::string_view& res = "") const;
127+
128+
BaseObjectPtr<diagnostics_channel::Channel> GetOrCreateChannel(
129+
Environment* env, PermissionScope scope) const;
129130

130131
std::unordered_map<PermissionScope, std::shared_ptr<PermissionBase>> nodes_;
131132
bool enabled_;
133+
bool warning_only_;
134+
mutable bool publishing_ = false;
135+
// Weak refs: BindingData (via BaseObjectPtr) is the sole owner of Channels.
136+
// Using weak refs here avoids keeping Channels alive past Realm teardown.
137+
mutable std::unordered_map<PermissionScope,
138+
BaseObjectWeakPtr<diagnostics_channel::Channel>>
139+
channels_;
132140
};
133141

134142
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)