Visitar URL original
src,permission: add --permission-audit · nodejs/node@9ddd1a9 · GitHub
Skip to content

Commit 9ddd1a9

Browse files
RafaelGSSaduh95
RafaelGSS
authored and
aduh95
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 PR-URL: #61869 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent 46ee1ed commit 9ddd1a9

10 files changed

Lines changed: 169 additions & 12 deletions

File tree

‎doc/api/cli.md‎

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

2166+
### `--permission-audit`
2167+
2168+
<!-- YAML
2169+
added: REPLACEME
2170+
-->
2171+
2172+
Enable audit only for the permission model. When enabled, permission checks
2173+
are performed but access is not denied. Instead, a warning is emitted for
2174+
each permission violation via diagnostics channel.
2175+
21662176
### `--preserve-symlinks`
21672177

21682178
<!-- YAML
@@ -3647,6 +3657,7 @@ one is included in the list below.
36473657
* `--openssl-legacy-provider`
36483658
* `--openssl-shared-config`
36493659
* `--pending-deprecation`
3660+
* `--permission-audit`
36503661
* `--permission`
36513662
* `--preserve-symlinks-main`
36523663
* `--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
@@ -615,7 +615,7 @@ function setupDiagnosticsChannel() {
615615
}
616616

617617
function initializePermission() {
618-
const permission = getOptionValue('--permission');
618+
const permission = getOptionValue('--permission') || getOptionValue('--permission-audit');
619619
if (permission) {
620620
process.binding = function binding(_module) {
621621
throw new ERR_ACCESS_DENIED('process.binding');

‎src/env.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -910,8 +910,11 @@ Environment::Environment(IsolateData* isolate_data,
910910
tracing::CastTracedValue(traced_value));
911911
}
912912

913-
if (options_->permission) {
913+
if (options_->permission || options_->permission_audit) {
914914
permission()->EnablePermissions();
915+
if (options_->permission_audit) {
916+
permission()->EnableWarningOnly();
917+
}
915918
// The process shouldn't be able to neither
916919
// spawn/worker nor use addons or enable inspector
917920
// 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: 93 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,74 @@ 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 =
227+
v8::Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0);
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(isolate,
236+
res.data(),
237+
v8::NewStringType::kNormal,
238+
static_cast<int>(res.size()))
239+
.ToLocalChecked())
240+
.Check();
241+
ch->Publish(env, msg);
242+
publishing_ = false;
243+
}
244+
}
245+
}
246+
247+
return result;
248+
}
249+
250+
BaseObjectPtr<diagnostics_channel::Channel> 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,
264+
BaseObjectWeakPtr<diagnostics_channel::Channel>(ch));
265+
return BaseObjectPtr<diagnostics_channel::Channel>(ch);
266+
}
267+
return {};
268+
}
269+
178270
void Permission::Apply(Environment* env,
179271
const std::vector<std::string>& allow,
180272
PermissionScope scope) {

‎src/permission/permission.h‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
55

66
#include "debug_utils.h"
7+
#include "node_diagnostics_channel.h"
78
#include "node_options.h"
89
#include "permission/addon_permission.h"
910
#include "permission/child_process_permission.h"
@@ -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',

‎test/parallel/test-permission-diagnostics-channel.js‎

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