Skip to content

Commit 4d76116

Browse files
RafaelGSSRafaelGSS
committed
src,permission: add --permission-audit
Refs: #59935
1 parent 53c4a39 commit 4d76116

File tree

9 files changed

+72
-16
lines changed

9 files changed

+72
-16
lines changed

‎doc/node.1‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,9 @@ to use as a custom module loader.
189189
.It Fl -permission
190190
Enable the permission model.
191191
.
192+
.It Fl -permission-audit
193+
Enable warning only with the permission model.
194+
.
192195
.It Fl -experimental-shadow-realm
193196
Use this flag to enable ShadowRealm support.
194197
.

‎lib/internal/process/pre_execution.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ function initializeClusterIPC() {
602602
}
603603

604604
function initializePermission() {
605-
const permission = getOptionValue('--permission');
605+
const permission = getOptionValue('--permission') || getOptionValue('--permission-audit');
606606
if (permission) {
607607
process.binding = function binding(_module) {
608608
throw new ERR_ACCESS_DENIED('process.binding');

‎src/env.cc‎

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

912-
if (options_->permission) {
912+
if (options_->permission || options_->permission_audit) {
913913
permission()->EnablePermissions();
914+
if (options_->permission_audit) {
915+
permission()->EnableWarningOnly();
916+
}
914917
// The process shouldn't be able to neither
915918
// spawn/worker nor use addons or enable inspector
916919
// unless explicitly allowed by the user

‎src/node_options.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
600600
&EnvironmentOptions::permission,
601601
kAllowedInEnvvar,
602602
false);
603+
AddOption("--permission-audit",
604+
"enable audit only for the permission system",
605+
&EnvironmentOptions::permission_audit,
606+
kAllowedInEnvvar,
607+
false);
603608
AddOption("--allow-fs-read",
604609
"allow permissions to read the filesystem",
605610
&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/node_process-inl.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ inline v8::Maybe<bool> ProcessEmitWarning(Environment* env,
1919
return ProcessEmitWarningGeneric(env, warning.c_str());
2020
}
2121

22+
template <typename... Args>
23+
inline v8::Maybe<void> ProcessEmitWarningSync(Environment* env,
24+
const char* fmt,
25+
Args&&... args) {
26+
std::string warning = SPrintF(fmt, std::forward<Args>(args)...);
27+
return ProcessEmitWarningSync(env, warning);
28+
}
29+
2230
} // namespace node
2331

2432
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

‎src/node_process.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ template <typename... Args>
3737
inline v8::Maybe<bool> ProcessEmitWarning(Environment* env,
3838
const char* fmt,
3939
Args&&... args);
40+
template <typename... Args>
41+
inline v8::Maybe<void> ProcessEmitWarningSync(Environment* env,
42+
const char* fmt,
43+
Args&&... args);
4044

4145
v8::Maybe<void> ProcessEmitWarningSync(Environment* env,
4246
std::string_view message);

‎src/permission/permission.cc‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "node_errors.h"
77
#include "node_external_reference.h"
88
#include "node_file.h"
9+
#include "node_process-inl.h"
910

1011
#include "v8.h"
1112

@@ -70,7 +71,7 @@ PermissionScope Permission::StringToPermission(const std::string& perm) {
7071
}
7172
#undef V
7273

73-
Permission::Permission() : enabled_(false) {
74+
Permission::Permission() : enabled_(false), warning_only_(false) {
7475
std::shared_ptr<PermissionBase> fs = std::make_shared<FSPermission>();
7576
std::shared_ptr<PermissionBase> child_p =
7677
std::make_shared<ChildProcessPermission>();
@@ -149,24 +150,45 @@ MaybeLocal<Value> CreateAccessDeniedError(Environment* env,
149150
void Permission::ThrowAccessDenied(Environment* env,
150151
PermissionScope perm,
151152
const std::string_view& res) {
152-
Local<Value> err;
153-
if (CreateAccessDeniedError(env, perm, res).ToLocal(&err)) {
154-
env->isolate()->ThrowException(err);
153+
// If permission is set to "audit" only. We should not throw, but
154+
// emit warning whenever a permission is "bypassed"
155+
if (!env->permission()->warning_only()) {
156+
Local<Value> err;
157+
if (CreateAccessDeniedError(env, perm, res).ToLocal(&err)) {
158+
env->isolate()->ThrowException(err);
159+
}
160+
// If ToLocal returned false, then v8 will have scheduled a
161+
// superseding error to be thrown.
162+
return;
155163
}
156-
// If ToLocal returned false, then v8 will have scheduled a
157-
// superseding error to be thrown.
164+
std::string_view perm_str = Permission::PermissionToString(perm);
165+
ProcessEmitWarningSync(
166+
env,
167+
"ERR_ACCESS_DENIED suppressed. Permission: %s, Resource: %s",
168+
perm_str,
169+
res);
158170
}
159171

160172
void Permission::AsyncThrowAccessDenied(Environment* env,
161173
fs::FSReqBase* req_wrap,
162174
PermissionScope perm,
163175
const std::string_view& res) {
164-
Local<Value> err;
165-
if (CreateAccessDeniedError(env, perm, res).ToLocal(&err)) {
166-
return req_wrap->Reject(err);
176+
if (env->permission()->warning_only()) {
177+
Local<Value> err;
178+
if (CreateAccessDeniedError(env, perm, res).ToLocal(&err)) {
179+
return req_wrap->Reject(err);
180+
}
181+
// If ToLocal returned false, then v8 will have scheduled a
182+
// superseding error to be thrown.
183+
return;
167184
}
168-
// If ToLocal returned false, then v8 will have scheduled a
169-
// superseding error to be thrown.
185+
std::string_view perm_str = Permission::PermissionToString(perm);
186+
// TODO: handle warning error
187+
ProcessEmitWarning(
188+
env,
189+
"ERR_ACCESS_DENIED suppressed. Permission: %s, Resource: %s",
190+
perm_str,
191+
res);
170192
}
171193

172194
void Permission::EnablePermissions() {
@@ -175,6 +197,12 @@ void Permission::EnablePermissions() {
175197
}
176198
}
177199

200+
void Permission::EnableWarningOnly() {
201+
if (!warning_only_) {
202+
warning_only_ = true;
203+
}
204+
}
205+
178206
void Permission::Apply(Environment* env,
179207
const std::vector<std::string>& allow,
180208
PermissionScope scope) {

‎src/permission/permission.h‎

Lines changed: 7 additions & 3 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

@@ -66,7 +66,7 @@ namespace permission {
6666
Local<Value> err_access; \
6767
if (node::permission::CreateAccessDeniedError(env__, perm__, resource__) \
6868
.ToLocal(&err_access)) { \
69-
args.GetReturnValue().Set(err_access); \
69+
\ args.GetReturnValue().Set(err_access); \
7070
} else { \
7171
args.GetReturnValue().Set(UV_EACCES); \
7272
} \
@@ -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,6 +117,7 @@ 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,
@@ -129,6 +132,7 @@ class Permission {
129132

130133
std::unordered_map<PermissionScope, std::shared_ptr<PermissionBase>> nodes_;
131134
bool enabled_;
135+
bool warning_only_;
132136
};
133137

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

0 commit comments

Comments
 (0)