Skip to content

Commit cd1eb3e

Browse files
RafaelGSSaduh95
authored andcommitted
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 Backport-PR-URL: #65354 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 82a0233 commit cd1eb3e

9 files changed

Lines changed: 164 additions & 12 deletions

File tree

doc/api/cli.md

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

2140+
### `--permission-audit`
2141+
2142+
<!-- YAML
2143+
added: REPLACEME
2144+
-->
2145+
2146+
Enable audit only for the permission model. When enabled, permission checks
2147+
are performed but access is not denied. Instead, a warning is emitted for
2148+
each permission violation via diagnostics channel.
2149+
21402150
### `--preserve-symlinks`
21412151

21422152
<!-- YAML
@@ -3661,6 +3671,7 @@ one is included in the list below.
36613671
* `--openssl-legacy-provider`
36623672
* `--openssl-shared-config`
36633673
* `--pending-deprecation`
3674+
* `--permission-audit`
36643675
* `--permission`
36653676
* `--preserve-symlinks-main`
36663677
* `--preserve-symlinks`

doc/node.1

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,11 @@ Among other uses, this can be used to enable FIPS-compliant crypto if Node.js is
468468
.It Fl -pending-deprecation
469469
Emit pending deprecation warnings.
470470
.
471+
.It Fl -permission-audit
472+
Enable audit only for the permission model. When enabled, permission checks
473+
are performed but access is not denied. Instead, a warning is emitted for
474+
each permission violation via diagnostics channel.
475+
.
471476
.It Fl -preserve-symlinks
472477
Instructs the module loader to preserve symbolic links when resolving and caching modules other than the main module.
473478
.

lib/internal/process/pre_execution.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,7 @@ function setupDiagnosticsChannel() {
634634
}
635635

636636
function initializePermission() {
637-
const permission = getOptionValue('--permission');
637+
const permission = getOptionValue('--permission') || getOptionValue('--permission-audit');
638638
if (permission) {
639639
process.binding = function binding(_module) {
640640
throw new ERR_ACCESS_DENIED('process.binding');

src/env.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -900,8 +900,11 @@ Environment::Environment(IsolateData* isolate_data,
900900
tracing::CastTracedValue(traced_value));
901901
}
902902

903-
if (options_->permission) {
903+
if (options_->permission || options_->permission_audit) {
904904
permission()->EnablePermissions();
905+
if (options_->permission_audit) {
906+
permission()->EnableWarningOnly();
907+
}
905908
// The process shouldn't be able to neither
906909
// spawn/worker nor use addons or enable inspector
907910
// unless explicitly allowed by the user

src/node_options.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
639639
&EnvironmentOptions::permission,
640640
kAllowedInEnvvar,
641641
false);
642+
AddOption("--permission-audit",
643+
"enable audit only for the permission system",
644+
&EnvironmentOptions::permission_audit,
645+
kAllowedInEnvvar,
646+
false);
642647
AddOption("--allow-fs-read",
643648
"allow permissions to read the filesystem",
644649
&EnvironmentOptions::allow_fs_read,

src/node_options.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ class EnvironmentOptions : public Options {
143143
std::string input_type; // Value of --input-type
144144
bool entry_is_url = false;
145145
bool permission = false;
146+
bool permission_audit = false;
146147
std::vector<std::string> allow_fs_read;
147148
std::vector<std::string> allow_fs_write;
148149
bool allow_addons = false;

src/permission/permission.cc

Lines changed: 91 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,27 @@ 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::kInspector:
42+
return "node:permission-model:inspector";
43+
case PermissionScope::kWASI:
44+
return "node:permission-model:wasi";
45+
case PermissionScope::kAddon:
46+
return "node:permission-model:addon";
47+
default:
48+
return {};
49+
}
50+
}
51+
3052
// permission.has('fs.in', '/tmp/')
3153
// permission.has('fs.in')
3254
static void Has(const FunctionCallbackInfo<Value>& args) {
@@ -70,7 +92,7 @@ PermissionScope Permission::StringToPermission(const std::string& perm) {
7092
}
7193
#undef V
7294

73-
Permission::Permission() : enabled_(false) {
95+
Permission::Permission() : enabled_(false), warning_only_(false) {
7496
std::shared_ptr<PermissionBase> fs = std::make_shared<FSPermission>();
7597
std::shared_ptr<PermissionBase> child_p =
7698
std::make_shared<ChildProcessPermission>();
@@ -170,6 +192,74 @@ void Permission::EnablePermissions() {
170192
}
171193
}
172194

195+
void Permission::EnableWarningOnly() {
196+
if (!warning_only_) {
197+
warning_only_ = true;
198+
}
199+
}
200+
201+
bool Permission::is_scope_granted(Environment* env,
202+
const PermissionScope permission,
203+
const std::string_view& res) const {
204+
auto perm_node = nodes_.find(permission);
205+
bool result = false;
206+
if (perm_node != nodes_.end()) {
207+
result = perm_node->second->is_granted(env, permission, res);
208+
}
209+
210+
if (!result && !publishing_) {
211+
auto channel_name = GetDiagnosticsChannelName(permission);
212+
if (!channel_name.empty()) {
213+
auto ch = GetOrCreateChannel(env, permission);
214+
if (ch && ch->HasSubscribers()) {
215+
publishing_ = true;
216+
v8::Isolate* isolate = env->isolate();
217+
v8::HandleScope handle_scope(isolate);
218+
v8::Local<v8::Context> context = env->context();
219+
v8::Local<v8::Object> msg =
220+
v8::Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0);
221+
const char* perm_str = PermissionToString(permission);
222+
msg->Set(context,
223+
FIXED_ONE_BYTE_STRING(isolate, "permission"),
224+
v8::String::NewFromUtf8(isolate, perm_str).ToLocalChecked())
225+
.Check();
226+
msg->Set(context,
227+
FIXED_ONE_BYTE_STRING(isolate, "resource"),
228+
v8::String::NewFromUtf8(isolate,
229+
res.data(),
230+
v8::NewStringType::kNormal,
231+
static_cast<int>(res.size()))
232+
.ToLocalChecked())
233+
.Check();
234+
ch->Publish(env, msg);
235+
publishing_ = false;
236+
}
237+
}
238+
}
239+
240+
return result;
241+
}
242+
243+
BaseObjectPtr<diagnostics_channel::Channel> Permission::GetOrCreateChannel(
244+
Environment* env, PermissionScope scope) const {
245+
auto it = channels_.find(scope);
246+
if (it != channels_.end()) {
247+
// Promote weak ref to strong for the duration of this call.
248+
BaseObjectPtr<diagnostics_channel::Channel> ptr(it->second.get());
249+
if (ptr) return ptr;
250+
channels_.erase(it);
251+
}
252+
auto channel_name = GetDiagnosticsChannelName(scope);
253+
diagnostics_channel::Channel* ch =
254+
diagnostics_channel::Channel::Get(env, channel_name.data());
255+
if (ch != nullptr) {
256+
channels_.emplace(scope,
257+
BaseObjectWeakPtr<diagnostics_channel::Channel>(ch));
258+
return BaseObjectPtr<diagnostics_channel::Channel>(ch);
259+
}
260+
return {};
261+
}
262+
173263
void Permission::Apply(Environment* env,
174264
const std::vector<std::string>& allow,
175265
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"
@@ -36,7 +37,7 @@ namespace permission {
3637
[[unlikely]] { \
3738
node::permission::Permission::ThrowAccessDenied( \
3839
env__, perm__, resource__); \
39-
return __VA_ARGS__; \
40+
if (!env__->permission()->warning_only()) return __VA_ARGS__; \
4041
} \
4142
} while (0)
4243

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

@@ -99,6 +100,8 @@ class Permission {
99100

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

103+
FORCE_INLINE bool warning_only() const { return warning_only_; }
104+
102105
static PermissionScope StringToPermission(const std::string& perm);
103106
static const char* PermissionToString(PermissionScope perm);
104107
static void ThrowAccessDenied(Environment* env,
@@ -114,20 +117,25 @@ class Permission {
114117
const std::vector<std::string>& allow,
115118
PermissionScope scope);
116119
void EnablePermissions();
120+
void EnableWarningOnly();
117121

118122
private:
119123
COLD_NOINLINE bool is_scope_granted(Environment* env,
120124
const PermissionScope permission,
121-
const std::string_view& res = "") const {
122-
auto perm_node = nodes_.find(permission);
123-
if (perm_node != nodes_.end()) {
124-
return perm_node->second->is_granted(env, permission, res);
125-
}
126-
return false;
127-
}
125+
const std::string_view& res = "") const;
126+
127+
BaseObjectPtr<diagnostics_channel::Channel> GetOrCreateChannel(
128+
Environment* env, PermissionScope scope) const;
128129

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

133141
} // namespace permission
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)