From 4d7611651f681b3f743570d43e69bec83c64fdd3 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Tue, 4 Nov 2025 20:22:12 -0300 Subject: [PATCH] src,permission: add --permission-audit Refs: https://github.com/nodejs/node/issues/59935 --- doc/node.1 | 3 ++ lib/internal/process/pre_execution.js | 2 +- src/env.cc | 5 ++- src/node_options.cc | 5 +++ src/node_options.h | 1 + src/node_process-inl.h | 8 +++++ src/node_process.h | 4 +++ src/permission/permission.cc | 50 +++++++++++++++++++++------ src/permission/permission.h | 10 ++++-- 9 files changed, 72 insertions(+), 16 deletions(-) diff --git a/doc/node.1 b/doc/node.1 index ba9af3ac6a2f31..7929ba120a86d1 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -189,6 +189,9 @@ to use as a custom module loader. .It Fl -permission Enable the permission model. . +.It Fl -permission-audit +Enable warning only with the permission model. +. .It Fl -experimental-shadow-realm Use this flag to enable ShadowRealm support. . diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index 76ec7d821cb4cc..aa54340ca826b8 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -602,7 +602,7 @@ function initializeClusterIPC() { } function initializePermission() { - const permission = getOptionValue('--permission'); + const permission = getOptionValue('--permission') || getOptionValue('--permission-audit'); if (permission) { process.binding = function binding(_module) { throw new ERR_ACCESS_DENIED('process.binding'); diff --git a/src/env.cc b/src/env.cc index f6aeb75933bd59..6653fc0a37968b 100644 --- a/src/env.cc +++ b/src/env.cc @@ -909,8 +909,11 @@ Environment::Environment(IsolateData* isolate_data, tracing::CastTracedValue(traced_value)); } - if (options_->permission) { + if (options_->permission || options_->permission_audit) { permission()->EnablePermissions(); + if (options_->permission_audit) { + permission()->EnableWarningOnly(); + } // The process shouldn't be able to neither // spawn/worker nor use addons or enable inspector // unless explicitly allowed by the user diff --git a/src/node_options.cc b/src/node_options.cc index 687cbd5398535f..c10c0aeed60ae9 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -600,6 +600,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { &EnvironmentOptions::permission, kAllowedInEnvvar, false); + AddOption("--permission-audit", + "enable audit only for the permission system", + &EnvironmentOptions::permission_audit, + kAllowedInEnvvar, + false); AddOption("--allow-fs-read", "allow permissions to read the filesystem", &EnvironmentOptions::allow_fs_read, diff --git a/src/node_options.h b/src/node_options.h index 79b90d59014cb9..759f53a47a368c 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -138,6 +138,7 @@ class EnvironmentOptions : public Options { std::string input_type; // Value of --input-type bool entry_is_url = false; bool permission = false; + bool permission_audit = false; std::vector allow_fs_read; std::vector allow_fs_write; bool allow_addons = false; diff --git a/src/node_process-inl.h b/src/node_process-inl.h index 21a448cfdb2a75..585f1362c2e129 100644 --- a/src/node_process-inl.h +++ b/src/node_process-inl.h @@ -19,6 +19,14 @@ inline v8::Maybe ProcessEmitWarning(Environment* env, return ProcessEmitWarningGeneric(env, warning.c_str()); } +template +inline v8::Maybe ProcessEmitWarningSync(Environment* env, + const char* fmt, + Args&&... args) { + std::string warning = SPrintF(fmt, std::forward(args)...); + return ProcessEmitWarningSync(env, warning); +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/node_process.h b/src/node_process.h index 64393302d2cfd9..4ab5329766e27b 100644 --- a/src/node_process.h +++ b/src/node_process.h @@ -37,6 +37,10 @@ template inline v8::Maybe ProcessEmitWarning(Environment* env, const char* fmt, Args&&... args); +template +inline v8::Maybe ProcessEmitWarningSync(Environment* env, + const char* fmt, + Args&&... args); v8::Maybe ProcessEmitWarningSync(Environment* env, std::string_view message); diff --git a/src/permission/permission.cc b/src/permission/permission.cc index 2d0e1723050e30..721508574f6333 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -6,6 +6,7 @@ #include "node_errors.h" #include "node_external_reference.h" #include "node_file.h" +#include "node_process-inl.h" #include "v8.h" @@ -70,7 +71,7 @@ PermissionScope Permission::StringToPermission(const std::string& perm) { } #undef V -Permission::Permission() : enabled_(false) { +Permission::Permission() : enabled_(false), warning_only_(false) { std::shared_ptr fs = std::make_shared(); std::shared_ptr child_p = std::make_shared(); @@ -149,24 +150,45 @@ MaybeLocal CreateAccessDeniedError(Environment* env, void Permission::ThrowAccessDenied(Environment* env, PermissionScope perm, const std::string_view& res) { - Local err; - if (CreateAccessDeniedError(env, perm, res).ToLocal(&err)) { - env->isolate()->ThrowException(err); + // If permission is set to "audit" only. We should not throw, but + // emit warning whenever a permission is "bypassed" + if (!env->permission()->warning_only()) { + Local err; + if (CreateAccessDeniedError(env, perm, res).ToLocal(&err)) { + env->isolate()->ThrowException(err); + } + // If ToLocal returned false, then v8 will have scheduled a + // superseding error to be thrown. + return; } - // If ToLocal returned false, then v8 will have scheduled a - // superseding error to be thrown. + std::string_view perm_str = Permission::PermissionToString(perm); + ProcessEmitWarningSync( + env, + "ERR_ACCESS_DENIED suppressed. Permission: %s, Resource: %s", + perm_str, + res); } void Permission::AsyncThrowAccessDenied(Environment* env, fs::FSReqBase* req_wrap, PermissionScope perm, const std::string_view& res) { - Local err; - if (CreateAccessDeniedError(env, perm, res).ToLocal(&err)) { - return req_wrap->Reject(err); + if (env->permission()->warning_only()) { + Local err; + if (CreateAccessDeniedError(env, perm, res).ToLocal(&err)) { + return req_wrap->Reject(err); + } + // If ToLocal returned false, then v8 will have scheduled a + // superseding error to be thrown. + return; } - // If ToLocal returned false, then v8 will have scheduled a - // superseding error to be thrown. + std::string_view perm_str = Permission::PermissionToString(perm); + // TODO: handle warning error + ProcessEmitWarning( + env, + "ERR_ACCESS_DENIED suppressed. Permission: %s, Resource: %s", + perm_str, + res); } void Permission::EnablePermissions() { @@ -175,6 +197,12 @@ void Permission::EnablePermissions() { } } +void Permission::EnableWarningOnly() { + if (!warning_only_) { + warning_only_ = true; + } +} + void Permission::Apply(Environment* env, const std::vector& allow, PermissionScope scope) { diff --git a/src/permission/permission.h b/src/permission/permission.h index f18a2ea1182c36..1fc9a995461d0f 100644 --- a/src/permission/permission.h +++ b/src/permission/permission.h @@ -37,7 +37,7 @@ namespace permission { [[unlikely]] { \ node::permission::Permission::ThrowAccessDenied( \ env__, perm__, resource__); \ - return __VA_ARGS__; \ + if (!env__->permission()->warning_only()) return __VA_ARGS__; \ } \ } while (0) @@ -51,7 +51,7 @@ namespace permission { [[unlikely]] { \ node::permission::Permission::AsyncThrowAccessDenied( \ env__, (wrap), perm__, resource__); \ - return __VA_ARGS__; \ + if (!env__->permission()->warning_only()) return __VA_ARGS__; \ } \ } while (0) @@ -66,7 +66,7 @@ namespace permission { Local err_access; \ if (node::permission::CreateAccessDeniedError(env__, perm__, resource__) \ .ToLocal(&err_access)) { \ - args.GetReturnValue().Set(err_access); \ + \ args.GetReturnValue().Set(err_access); \ } else { \ args.GetReturnValue().Set(UV_EACCES); \ } \ @@ -100,6 +100,8 @@ class Permission { FORCE_INLINE bool enabled() const { return enabled_; } + FORCE_INLINE bool warning_only() const { return warning_only_; } + static PermissionScope StringToPermission(const std::string& perm); static const char* PermissionToString(PermissionScope perm); static void ThrowAccessDenied(Environment* env, @@ -115,6 +117,7 @@ class Permission { const std::vector& allow, PermissionScope scope); void EnablePermissions(); + void EnableWarningOnly(); private: COLD_NOINLINE bool is_scope_granted(Environment* env, @@ -129,6 +132,7 @@ class Permission { std::unordered_map> nodes_; bool enabled_; + bool warning_only_; }; v8::MaybeLocal CreateAccessDeniedError(Environment* env,