From fd565321350d3945006845914d2acb5be41c5e7a Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Aug 2026 16:33:16 -0700 Subject: [PATCH 01/11] src: cache permission strings Use env_property strings for permissions since those are fixed. Avoid creating new string instances each time. Also use ToV8Value for a couple since we're in here. Signed-off-by: James M Snell --- src/env-inl.h | 16 ++++++++++++++++ src/env.cc | 37 ++++++++++++++++++++++++++++++++++++ src/env.h | 16 ++++++++++++++++ src/permission/permission.cc | 32 ++++++++++--------------------- src/permission/permission.h | 3 ++- 5 files changed, 81 insertions(+), 23 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 761a7bfc9955..bbeb47b72ce3 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -841,6 +841,14 @@ void Environment::set_process_exit_handler( #undef VY #undef VP +#define V(Name, label, _, __) \ + inline v8::Local \ + IsolateData::Name##_permission_string() const { \ + return Name##_permission_string##_.Get(isolate_); \ + } + PERMISSIONS(V) +#undef V + #define VM(PropertyName) V(PropertyName##_binding_template, v8::ObjectTemplate) #define V(PropertyName, TypeName) \ inline v8::Local IsolateData::PropertyName() const { \ @@ -870,6 +878,14 @@ void Environment::set_process_exit_handler( #undef VY #undef VP +#define V(Name, label, _, __) \ + inline v8::Local \ + Environment::Name##_permission_string() const { \ + return isolate_data()->Name##_permission_string(); \ + } + PERMISSIONS(V) +#undef V + #define V(PropertyName, TypeName) \ inline v8::Local Environment::PropertyName() const { \ return isolate_data()->PropertyName(); \ diff --git a/src/env.cc b/src/env.cc index 87340112fbeb..7b275e09b720 100644 --- a/src/env.cc +++ b/src/env.cc @@ -360,6 +360,12 @@ IsolateDataSerializeInfo IsolateData::Serialize(SnapshotCreator* creator) { #undef VS #undef VP +#define V(Name, label, _, __) \ + info.primitive_values.push_back( \ + creator->AddData(Name##_permission_string##_.Get(isolate))); + PERMISSIONS(V) +#undef V + info.primitive_values.reserve(info.primitive_values.size() + AsyncWrap::PROVIDERS_LENGTH); for (size_t i = 0; i < AsyncWrap::PROVIDERS_LENGTH; i++) { @@ -419,6 +425,21 @@ void IsolateData::DeserializeProperties(const IsolateDataSerializeInfo* info) { #undef VS #undef VP +#define V(Name, label, _, __) \ + do { \ + MaybeLocal maybe_field = \ + isolate_->GetDataFromSnapshotOnce( \ + info->primitive_values[i++]); \ + Local field; \ + if (!maybe_field.ToLocal(&field)) { \ + fprintf(stderr, \ + "Failed to deserialize " #Name "_permission_string\n"); \ + } \ + Name##_permission_string##_.Set(isolate_, field); \ + } while (0); + PERMISSIONS(V) +#undef V + for (size_t j = 0; j < AsyncWrap::PROVIDERS_LENGTH; j++) { MaybeLocal maybe_field = isolate_->GetDataFromSnapshotOnce(info->primitive_values[i++]); @@ -520,6 +541,17 @@ void IsolateData::CreateProperties() { PER_ISOLATE_STRING_PROPERTIES(V) #undef V +#define V(Name, label, _, __) \ + Name##_permission_string##_.Set( \ + isolate_, \ + String::NewFromOneByte(isolate_, \ + reinterpret_cast(#Name), \ + NewStringType::kInternalized, \ + sizeof(#Name) - 1) \ + .ToLocalChecked()); + PERMISSIONS(V) +#undef V + // Create all the provider strings that will be passed to JS. Place them in // an array so the array index matches the PROVIDER id offset. This way the // strings can be retrieved quickly. @@ -630,6 +662,11 @@ void IsolateData::MemoryInfo(MemoryTracker* tracker) const { PER_ISOLATE_STRING_PROPERTIES(V) #undef V +#define V(Name, label, _, __) \ + tracker->TrackField(#Name "_permission_string", Name##_permission_string()); + PERMISSIONS(V) +#undef V + tracker->TrackField("async_wrap_providers", async_wrap_providers_); if (node_allocator_ != nullptr) { diff --git a/src/env.h b/src/env.h index c2caf9790238..c2bf9fdd497a 100644 --- a/src/env.h +++ b/src/env.h @@ -189,6 +189,11 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { #undef VS #undef VP +#define V(Name, label, _, __) \ + inline v8::Local Name##_permission_string() const; + PERMISSIONS(V) +#undef V + #define VM(PropertyName) V(PropertyName##_binding_template, v8::ObjectTemplate) #define V(PropertyName, TypeName) \ inline v8::Local PropertyName() const; \ @@ -234,6 +239,12 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { #undef VS #undef VY #undef VP + +#define V(Name, label, _, __) \ + v8::Eternal Name##_permission_string##_; + PERMISSIONS(V) +#undef V + // Keep a list of all Persistent strings used for AsyncWrap Provider types. std::array, AsyncWrap::PROVIDERS_LENGTH> async_wrap_providers_; @@ -875,6 +886,11 @@ class Environment final : public MemoryRetainer { #undef VY #undef VP +#define V(Name, label, _, __) \ + inline v8::Local Name##_permission_string() const; + PERMISSIONS(V) +#undef V + #define V(PropertyName, TypeName) \ inline v8::Local PropertyName() const; \ inline void set_ ## PropertyName(v8::Local value); diff --git a/src/permission/permission.cc b/src/permission/permission.cc index 919edd0821fd..9815073c987a 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -106,10 +106,11 @@ static void Has(const FunctionCallbackInfo& args) { } // namespace #define V(Name, label, _, __) \ - if (perm == PermissionScope::k##Name) return #Name; -const char* Permission::PermissionToString(const PermissionScope perm) { + if (perm == PermissionScope::k##Name) return env->Name##_permission_string(); +v8::Local Permission::PermissionToString( + Environment* env, const PermissionScope perm) { PERMISSIONS(V) - return nullptr; + UNREACHABLE(); } #undef V @@ -192,12 +193,9 @@ MaybeLocal CreateAccessDeniedError(Environment* env, Local err = ERR_ACCESS_DENIED( env->isolate(), "Access to this API has been restricted. %s", suggestion); - Local perm_string; Local resource_string; - std::string_view perm_str = Permission::PermissionToString(perm); - if (!ToV8Value(env->context(), perm_str, env->isolate()) - .ToLocal(&perm_string) || - !ToV8Value(env->context(), res, env->isolate()) + Local perm_string = Permission::PermissionToString(env, perm); + if (!ToV8Value(env->context(), res, env->isolate()) .ToLocal(&resource_string) || err->Set(env->context(), env->permission_string(), perm_string) .IsNothing() || @@ -263,18 +261,13 @@ bool Permission::is_scope_granted(Environment* env, v8::Local context = env->context(); v8::Local msg = v8::Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0); - const char* perm_str = PermissionToString(permission); msg->Set(context, env->permission_string(), - v8::String::NewFromUtf8(isolate, perm_str).ToLocalChecked()) + PermissionToString(env, permission)) .Check(); msg->Set(context, env->resource_string(), - v8::String::NewFromUtf8(isolate, - res.data(), - v8::NewStringType::kNormal, - static_cast(res.size())) - .ToLocalChecked()) + ToV8Value(context, res).ToLocalChecked()) .Check(); ch->Publish(env, msg); publishing_ = false; @@ -333,18 +326,13 @@ void Permission::Drop(Environment* env, v8::Local context = env->context(); v8::Local msg = v8::Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0); - const char* perm_str = PermissionToString(scope); msg->Set(context, env->permission_string(), - v8::String::NewFromUtf8(isolate, perm_str).ToLocalChecked()) + PermissionToString(env, scope)) .Check(); msg->Set(context, env->resource_string(), - v8::String::NewFromUtf8(isolate, - param.data(), - v8::NewStringType::kNormal, - static_cast(param.size())) - .ToLocalChecked()) + ToV8Value(context, param).ToLocalChecked()) .Check(); msg->Set(context, FIXED_ONE_BYTE_STRING(isolate, "drop"), diff --git a/src/permission/permission.h b/src/permission/permission.h index 5597e5ce2445..674d99c1409a 100644 --- a/src/permission/permission.h +++ b/src/permission/permission.h @@ -113,7 +113,8 @@ class Permission { FORCE_INLINE bool warning_only() const { return warning_only_; } static PermissionScope StringToPermission(const std::string& perm); - static const char* PermissionToString(PermissionScope perm); + static v8::Local PermissionToString(Environment* env, + PermissionScope perm); static void ThrowAccessDenied(Environment* env, PermissionScope perm, const std::string_view& res); From bac89754e2a7b378f58835606592199add1671fd Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Aug 2026 17:01:32 -0700 Subject: [PATCH 02/11] src: use DictionaryTemplate for permission diag channel message Since DiagnosticChannel permission messages always have the same shape and should be as low cost as possible, use a cached DictionaryTemplate for creating them Signed-off-by: James M Snell --- src/env-inl.h | 10 +++--- src/env.cc | 5 ++- src/env_properties.h | 1 + src/permission/permission.cc | 60 ++++++++++++++++++++---------------- 4 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index bbeb47b72ce3..e9f940c63e53 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -842,9 +842,8 @@ void Environment::set_process_exit_handler( #undef VP #define V(Name, label, _, __) \ - inline v8::Local \ - IsolateData::Name##_permission_string() const { \ - return Name##_permission_string##_.Get(isolate_); \ + inline v8::Local IsolateData::Name##_permission_string() const { \ + return Name##_permission_string##_.Get(isolate_); \ } PERMISSIONS(V) #undef V @@ -879,9 +878,8 @@ void Environment::set_process_exit_handler( #undef VP #define V(Name, label, _, __) \ - inline v8::Local \ - Environment::Name##_permission_string() const { \ - return isolate_data()->Name##_permission_string(); \ + inline v8::Local Environment::Name##_permission_string() const { \ + return isolate_data()->Name##_permission_string(); \ } PERMISSIONS(V) #undef V diff --git a/src/env.cc b/src/env.cc index 7b275e09b720..658141f60817 100644 --- a/src/env.cc +++ b/src/env.cc @@ -432,10 +432,9 @@ void IsolateData::DeserializeProperties(const IsolateDataSerializeInfo* info) { info->primitive_values[i++]); \ Local field; \ if (!maybe_field.ToLocal(&field)) { \ - fprintf(stderr, \ - "Failed to deserialize " #Name "_permission_string\n"); \ + fprintf(stderr, "Failed to deserialize " #Name "_permission_string\n"); \ } \ - Name##_permission_string##_.Set(isolate_, field); \ + Name##_permission_string##_.Set(isolate_, field); \ } while (0); PERMISSIONS(V) #undef V diff --git a/src/env_properties.h b/src/env_properties.h index f54efa36b6df..9f69d07e92a3 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -463,6 +463,7 @@ V(naptr_record_template, v8::DictionaryTemplate) \ V(object_stats_template, v8::DictionaryTemplate) \ V(page_stats_template, v8::DictionaryTemplate) \ + V(permission_diagnostic_channel_message, v8::DictionaryTemplate) \ V(pipe_constructor_template, v8::FunctionTemplate) \ V(script_context_constructor_template, v8::FunctionTemplate) \ V(secure_context_constructor_template, v8::FunctionTemplate) \ diff --git a/src/permission/permission.cc b/src/permission/permission.cc index 9815073c987a..512d95333b47 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -8,6 +8,7 @@ #include "node_external_reference.h" #include "node_file.h" +#include "v8-template.h" #include "v8.h" #include @@ -17,11 +18,13 @@ namespace node { using v8::Context; +using v8::DictionaryTemplate; using v8::FunctionCallbackInfo; using v8::IntegrityLevel; using v8::Local; using v8::MaybeLocal; using v8::Object; +using v8::Undefined; using v8::Value; namespace permission { @@ -55,6 +58,20 @@ constexpr std::string_view GetDiagnosticsChannelName(PermissionScope scope) { } } +Local GetPermissionDiagnosicsTemplate(Environment* env) { + auto tmpl = env->permission_diagnostic_channel_message(); + if (tmpl.IsEmpty()) { + static constexpr std::string_view names[] = { + "permission", + "resource", + "drop", + }; + tmpl = DictionaryTemplate::New(env->isolate(), names); + env->set_permission_diagnostic_channel_message(tmpl); + } + return tmpl; +} + // permission.drop('fs.read', '/tmp/') // permission.drop('child') static void Drop(const FunctionCallbackInfo& args) { @@ -259,17 +276,14 @@ bool Permission::is_scope_granted(Environment* env, v8::Isolate* isolate = env->isolate(); v8::HandleScope handle_scope(isolate); v8::Local context = env->context(); - v8::Local msg = - v8::Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0); - msg->Set(context, - env->permission_string(), - PermissionToString(env, permission)) - .Check(); - msg->Set(context, - env->resource_string(), - ToV8Value(context, res).ToLocalChecked()) - .Check(); - ch->Publish(env, msg); + v8::MaybeLocal values[] = { + PermissionToString(env, permission), + ToV8Value(context, res), + Undefined(isolate), + }; + ch->Publish( + env, + GetPermissionDiagnosicsTemplate(env)->NewInstance(context, values)); publishing_ = false; } } @@ -324,21 +338,15 @@ void Permission::Drop(Environment* env, v8::Isolate* isolate = env->isolate(); v8::HandleScope handle_scope(isolate); v8::Local context = env->context(); - v8::Local msg = - v8::Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0); - msg->Set(context, - env->permission_string(), - PermissionToString(env, scope)) - .Check(); - msg->Set(context, - env->resource_string(), - ToV8Value(context, param).ToLocalChecked()) - .Check(); - msg->Set(context, - FIXED_ONE_BYTE_STRING(isolate, "drop"), - v8::Boolean::New(isolate, true)) - .Check(); - ch->Publish(env, msg); + + v8::MaybeLocal values[] = { + PermissionToString(env, scope), + ToV8Value(context, param), + v8::True(isolate), + }; + ch->Publish( + env, + GetPermissionDiagnosicsTemplate(env)->NewInstance(context, values)); publishing_ = false; } } From dfb1e001f0df417429aa2710baca9520f4370bfa Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Aug 2026 17:10:40 -0700 Subject: [PATCH 03/11] src: make minor cleanup to permission checks Getting the name of the channel is unnecessary. Signed-off-by: James M Snell --- src/permission/permission.cc | 43 +++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/permission/permission.cc b/src/permission/permission.cc index 512d95333b47..aad50a7f4039 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -8,6 +8,7 @@ #include "node_external_reference.h" #include "node_file.h" +#include "permission/permission_base.h" #include "v8-template.h" #include "v8.h" @@ -261,6 +262,8 @@ void Permission::EnableWarningOnly() { bool Permission::is_scope_granted(Environment* env, const PermissionScope permission, const std::string_view& res) const { + CHECK(permission != PermissionScope::kPermissionsRoot && + permission != PermissionScope::kPermissionsCount); auto perm_node = nodes_.find(permission); bool result = false; if (perm_node != nodes_.end()) { @@ -268,24 +271,21 @@ bool Permission::is_scope_granted(Environment* env, } if (!result && !publishing_) { - auto channel_name = GetDiagnosticsChannelName(permission); - if (!channel_name.empty()) { - auto ch = GetOrCreateChannel(env, permission); - if (ch && ch->HasSubscribers()) { - publishing_ = true; - v8::Isolate* isolate = env->isolate(); - v8::HandleScope handle_scope(isolate); - v8::Local context = env->context(); - v8::MaybeLocal values[] = { - PermissionToString(env, permission), - ToV8Value(context, res), - Undefined(isolate), - }; - ch->Publish( - env, - GetPermissionDiagnosicsTemplate(env)->NewInstance(context, values)); - publishing_ = false; - } + auto ch = GetOrCreateChannel(env, permission); + if (ch && ch->HasSubscribers()) { + publishing_ = true; + v8::Isolate* isolate = env->isolate(); + v8::HandleScope handle_scope(isolate); + v8::Local context = env->context(); + v8::MaybeLocal values[] = { + PermissionToString(env, permission), + ToV8Value(context, res), + Undefined(isolate), + }; + ch->Publish( + env, + GetPermissionDiagnosicsTemplate(env)->NewInstance(context, values)); + publishing_ = false; } } @@ -294,6 +294,8 @@ bool Permission::is_scope_granted(Environment* env, BaseObjectPtr Permission::GetOrCreateChannel( Environment* env, PermissionScope scope) const { + CHECK(scope != PermissionScope::kPermissionsRoot && + scope != PermissionScope::kPermissionsCount); auto it = channels_.find(scope); if (it != channels_.end()) { // Promote weak ref to strong for the duration of this call. @@ -324,14 +326,15 @@ void Permission::Apply(Environment* env, void Permission::Drop(Environment* env, PermissionScope scope, const std::string_view& param) { + CHECK(scope != PermissionScope::kPermissionsRoot && + scope != PermissionScope::kPermissionsCount); auto permission = nodes_.find(scope); if (permission != nodes_.end()) { permission->second->Drop(env, scope, param); } // Publish to diagnostics channel so observers can track drops - auto channel_name = GetDiagnosticsChannelName(scope); - if (!channel_name.empty() && !publishing_) { + if (!publishing_) { auto ch = GetOrCreateChannel(env, scope); if (ch && ch->HasSubscribers()) { publishing_ = true; From 85cab29dc9fabeda12c755fb82ec40e8290f50c6 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Aug 2026 17:33:26 -0700 Subject: [PATCH 04/11] src: simplify c++ diagnostics channel API Signed-off-by: James M Snell --- src/node_diagnostics_channel.cc | 14 ++++++++------ src/node_diagnostics_channel.h | 3 +-- src/permission/permission.cc | 6 ++---- test/cctest/test_diagnostics_channel.cc | 9 +++++---- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/node_diagnostics_channel.cc b/src/node_diagnostics_channel.cc index cfae019da62f..ab67e6d7c5fa 100644 --- a/src/node_diagnostics_channel.cc +++ b/src/node_diagnostics_channel.cc @@ -178,11 +178,11 @@ void Channel::Unlink() { publish_fn_.Reset(); } -Channel* Channel::Get(Environment* env, const char* name) { +BaseObjectPtr Channel::Get(Environment* env, std::string_view name) { Realm* realm = env->principal_realm(); BindingData* binding = realm->GetBindingData(); if (binding == nullptr) { - return nullptr; + return {}; } uint32_t index = binding->GetOrCreateChannelIndex(std::string(name)); @@ -208,22 +208,24 @@ Channel* Channel::Get(Environment* env, const char* name) { .ToLocalChecked() ->NewInstance(context) .ToLocal(&wrap)) { - return nullptr; + return {}; } binding->channels_[index] = MakeDetachedBaseObject( env, wrap, binding, index, std::string(name)); } - Channel* channel = binding->channels_[index].get(); + auto& channel = binding->channels_[index]; // Late-bind: link to the JS channel when the callback is available. if (!binding->link_callback_.IsEmpty() && !channel->IsLinked()) { Isolate* isolate = env->isolate(); HandleScope handle_scope(isolate); Local context = env->context(); - Local js_name = String::NewFromUtf8(isolate, name).ToLocalChecked(); - Local argv[] = {js_name, Integer::NewFromUnsigned(isolate, index)}; + Local argv[] = { + ToV8Value(context, name).ToLocalChecked(), + Integer::NewFromUnsigned(isolate, index), + }; Local result; if (binding->link_callback_.Get(isolate) ->Call(context, v8::Undefined(isolate), arraysize(argv), argv) diff --git a/src/node_diagnostics_channel.h b/src/node_diagnostics_channel.h index 073e4e4f273b..ca68e75a4361 100644 --- a/src/node_diagnostics_channel.h +++ b/src/node_diagnostics_channel.h @@ -73,8 +73,7 @@ class Channel : public BaseObject { uint32_t index, std::string name); - // Returns a non-owning pointer. Lifetime is managed by BindingData. - static Channel* Get(Environment* env, const char* name); + static BaseObjectPtr Get(Environment* env, std::string_view name); inline bool HasSubscribers() const { return binding_data_ != nullptr && binding_data_->subscribers_[index_] > 0; diff --git a/src/permission/permission.cc b/src/permission/permission.cc index aad50a7f4039..6bf3b0fe04eb 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -304,12 +304,10 @@ BaseObjectPtr Permission::GetOrCreateChannel( channels_.erase(it); } auto channel_name = GetDiagnosticsChannelName(scope); - diagnostics_channel::Channel* ch = - diagnostics_channel::Channel::Get(env, channel_name.data()); - if (ch != nullptr) { + if (auto ch = diagnostics_channel::Channel::Get(env, channel_name)) { channels_.emplace(scope, BaseObjectWeakPtr(ch)); - return BaseObjectPtr(ch); + return ch; } return {}; } diff --git a/test/cctest/test_diagnostics_channel.cc b/test/cctest/test_diagnostics_channel.cc index 30a004b59070..d4d1fd1c9fac 100644 --- a/test/cctest/test_diagnostics_channel.cc +++ b/test/cctest/test_diagnostics_channel.cc @@ -3,6 +3,7 @@ #include "gtest/gtest.h" #include "node_test_fixture.h" +using node::BaseObjectPtr; using node::diagnostics_channel::Channel; class DiagnosticsChannelTest : public EnvironmentTestFixture {}; @@ -279,15 +280,15 @@ TEST_F(DiagnosticsChannelTest, NativeChannelsGrowSubscriberStorage) { "globalThis.__dc.subscribe('test:cctest:grow:0', " " globalThis.__firstSubscriber);"); - Channel* first = Channel::Get(*env, "test:cctest:grow:0"); - ASSERT_NE(first, nullptr); + auto first = Channel::Get(*env, "test:cctest:grow:0"); + ASSERT_TRUE(first); ASSERT_TRUE(first->HasSubscribers()); - Channel* last = nullptr; + BaseObjectPtr last; for (size_t i = 1; i <= 1024; i++) { std::string name = "test:cctest:grow:" + std::to_string(i); last = Channel::Get(*env, name.c_str()); - ASSERT_NE(last, nullptr); + ASSERT_TRUE(last); } RunJS(isolate_, From 551b23b8dc0d8eeda67232f1841c2c03eb9ec90e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Aug 2026 18:19:23 -0700 Subject: [PATCH 05/11] src: apply multiple general cleanups to permissions Signed-off-by: James M Snell --- src/env.cc | 17 ++++++------- src/node_diagnostics_channel.cc | 4 ++-- src/permission/addon_permission.cc | 6 ++--- src/permission/addon_permission.h | 6 ++--- src/permission/child_process_permission.cc | 6 ++--- src/permission/child_process_permission.h | 6 ++--- src/permission/ffi_permission.cc | 6 ++--- src/permission/ffi_permission.h | 6 ++--- src/permission/fs_permission.cc | 10 ++++---- src/permission/fs_permission.h | 10 ++++---- src/permission/inspector_permission.cc | 6 ++--- src/permission/inspector_permission.h | 6 ++--- src/permission/net_permission.cc | 6 ++--- src/permission/net_permission.h | 6 ++--- src/permission/openssl_store_permission.cc | 6 ++--- src/permission/openssl_store_permission.h | 6 ++--- src/permission/permission.cc | 28 +++++++++++----------- src/permission/permission.h | 22 ++++++++--------- src/permission/permission_base.h | 10 ++++---- src/permission/wasi_permission.cc | 6 ++--- src/permission/wasi_permission.h | 6 ++--- src/permission/worker_permission.cc | 6 ++--- src/permission/worker_permission.h | 6 ++--- 23 files changed, 98 insertions(+), 99 deletions(-) diff --git a/src/env.cc b/src/env.cc index 658141f60817..13344ad135e2 100644 --- a/src/env.cc +++ b/src/env.cc @@ -954,6 +954,7 @@ Environment::Environment(IsolateData* isolate_data, if (options_->permission || options_->permission_audit) { permission()->EnablePermissions(); + static const std::array args = {std::string("*")}; if (options_->permission_audit) { permission()->EnableWarningOnly(); } @@ -962,29 +963,29 @@ Environment::Environment(IsolateData* isolate_data, // unless explicitly allowed by the user if (!options_->allow_addons) { options_->allow_native_addons = false; - permission()->Apply(this, {"*"}, permission::PermissionScope::kAddon); + permission()->Apply(this, args, permission::PermissionScope::kAddon); } if (!options_->allow_inspector) { flags_ = flags_ | EnvironmentFlags::kNoCreateInspector; - permission()->Apply(this, {"*"}, permission::PermissionScope::kInspector); + permission()->Apply(this, args, permission::PermissionScope::kInspector); } if (!options_->allow_child_process) { permission()->Apply( - this, {"*"}, permission::PermissionScope::kChildProcess); + this, args, permission::PermissionScope::kChildProcess); } if (!options_->allow_ffi) { - permission()->Apply(this, {"*"}, permission::PermissionScope::kFFI); + permission()->Apply(this, args, permission::PermissionScope::kFFI); } if (!options_->allow_openssl_store) { permission()->Apply( - this, {"*"}, permission::PermissionScope::kOpenSSLStore); + this, args, permission::PermissionScope::kOpenSSLStore); } if (!options_->allow_worker_threads) { permission()->Apply( - this, {"*"}, permission::PermissionScope::kWorkerThreads); + this, args, permission::PermissionScope::kWorkerThreads); } if (!options_->allow_wasi) { - permission()->Apply(this, {"*"}, permission::PermissionScope::kWASI); + permission()->Apply(this, args, permission::PermissionScope::kWASI); } // Implicit allow entrypoint to kFileSystemRead @@ -1019,7 +1020,7 @@ Environment::Environment(IsolateData* isolate_data, } if (options_->allow_net) { - permission()->Apply(this, {"*"}, permission::PermissionScope::kNet); + permission()->Apply(this, args, permission::PermissionScope::kNet); } } } diff --git a/src/node_diagnostics_channel.cc b/src/node_diagnostics_channel.cc index ab67e6d7c5fa..ba0492a0df5e 100644 --- a/src/node_diagnostics_channel.cc +++ b/src/node_diagnostics_channel.cc @@ -223,8 +223,8 @@ BaseObjectPtr Channel::Get(Environment* env, std::string_view name) { HandleScope handle_scope(isolate); Local context = env->context(); Local argv[] = { - ToV8Value(context, name).ToLocalChecked(), - Integer::NewFromUnsigned(isolate, index), + ToV8Value(context, name).ToLocalChecked(), + Integer::NewFromUnsigned(isolate, index), }; Local result; if (binding->link_callback_.Get(isolate) diff --git a/src/permission/addon_permission.cc b/src/permission/addon_permission.cc index 66035556102f..249b266c5919 100644 --- a/src/permission/addon_permission.cc +++ b/src/permission/addon_permission.cc @@ -9,20 +9,20 @@ namespace permission { // Currently, Addon manage a single state // Once denied, it's always denied void AddonPermission::Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) { deny_all_ = true; } void AddonPermission::Drop(Environment* env, PermissionScope scope, - const std::string_view& param) { + std::string_view param) { deny_all_ = true; } bool AddonPermission::is_granted(Environment* env, PermissionScope perm, - const std::string_view& param) const { + std::string_view param) const { return deny_all_ == false; } diff --git a/src/permission/addon_permission.h b/src/permission/addon_permission.h index b3eed910fe9f..04e2ed6fed9a 100644 --- a/src/permission/addon_permission.h +++ b/src/permission/addon_permission.h @@ -13,14 +13,14 @@ namespace permission { class AddonPermission final : public PermissionBase { public: void Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) override; void Drop(Environment* env, PermissionScope scope, - const std::string_view& param = "") override; + std::string_view param) override; bool is_granted(Environment* env, PermissionScope perm, - const std::string_view& param = "") const override; + std::string_view param) const override; private: bool deny_all_; diff --git a/src/permission/child_process_permission.cc b/src/permission/child_process_permission.cc index 7d31ff24f813..25c9713e0570 100644 --- a/src/permission/child_process_permission.cc +++ b/src/permission/child_process_permission.cc @@ -10,20 +10,20 @@ namespace permission { // Currently, ChildProcess manage a single state // Once denied, it's always denied void ChildProcessPermission::Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) { deny_all_ = true; } void ChildProcessPermission::Drop(Environment* env, PermissionScope scope, - const std::string_view& param) { + std::string_view param) { deny_all_ = true; } bool ChildProcessPermission::is_granted(Environment* env, PermissionScope perm, - const std::string_view& param) const { + std::string_view param) const { return deny_all_ == false; } diff --git a/src/permission/child_process_permission.h b/src/permission/child_process_permission.h index 33612b1c10a9..59ab74bb5116 100644 --- a/src/permission/child_process_permission.h +++ b/src/permission/child_process_permission.h @@ -13,14 +13,14 @@ namespace permission { class ChildProcessPermission final : public PermissionBase { public: void Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) override; void Drop(Environment* env, PermissionScope scope, - const std::string_view& param = "") override; + std::string_view param) override; bool is_granted(Environment* env, PermissionScope perm, - const std::string_view& param = "") const override; + std::string_view param) const override; private: bool deny_all_; diff --git a/src/permission/ffi_permission.cc b/src/permission/ffi_permission.cc index 4b00d4c07b9c..674394144471 100644 --- a/src/permission/ffi_permission.cc +++ b/src/permission/ffi_permission.cc @@ -9,20 +9,20 @@ namespace permission { // Currently, FFIPermission manages a single global deny state for FFI. void FFIPermission::Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) { deny_all_ = true; } void FFIPermission::Drop(Environment* env, PermissionScope scope, - const std::string_view& param) { + std::string_view param) { deny_all_ = true; } bool FFIPermission::is_granted(Environment* env, PermissionScope perm, - const std::string_view& param) const { + std::string_view param) const { return perm != PermissionScope::kFFI || !deny_all_; } diff --git a/src/permission/ffi_permission.h b/src/permission/ffi_permission.h index 3acd3c4642a0..fcb8b403c725 100644 --- a/src/permission/ffi_permission.h +++ b/src/permission/ffi_permission.h @@ -13,14 +13,14 @@ namespace permission { class FFIPermission final : public PermissionBase { public: void Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) override; void Drop(Environment* env, PermissionScope scope, - const std::string_view& param = "") override; + std::string_view param) override; bool is_granted(Environment* env, PermissionScope perm, - const std::string_view& param = "") const override; + std::string_view param) const override; private: bool deny_all_ = false; diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc index 98146cf825ad..89f1c6d70c98 100644 --- a/src/permission/fs_permission.cc +++ b/src/permission/fs_permission.cc @@ -52,7 +52,7 @@ void FreeRecursivelyNode( bool is_tree_granted( node::Environment* env, const node::permission::FSPermission::RadixTree* granted_tree, - const std::string_view& param) { + std::string_view param) { std::string resolved_param = node::PathResolve(env, {param}); #ifdef _WIN32 // Remove leading "\\?\" from UNC path @@ -137,7 +137,7 @@ namespace permission { // allow = '*' // allow = '/tmp/,/home/example.js' void FSPermission::Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) { for (const std::string& res : allow) { if (res == "*") { @@ -156,7 +156,7 @@ void FSPermission::Apply(Environment* env, void FSPermission::Drop(Environment* env, PermissionScope scope, - const std::string_view& param) { + std::string_view param) { if (param.empty()) { // Drop all access for this scope if (scope == PermissionScope::kFileSystemRead || @@ -250,7 +250,7 @@ void FSPermission::GrantAccess(PermissionScope perm, const std::string& res) { bool FSPermission::is_granted(Environment* env, PermissionScope perm, - const std::string_view& param = "") const { + std::string_view param = "") const { switch (perm) { case PermissionScope::kFileSystem: return allow_all_in_ && allow_all_out_; @@ -287,7 +287,7 @@ void FSPermission::RadixTree::Clear() { root_node_->is_leaf = false; } -bool FSPermission::RadixTree::Lookup(const std::string_view& s, +bool FSPermission::RadixTree::Lookup(std::string_view s, bool when_empty_return) const { FSPermission::RadixTree::Node* current_node = root_node_; if (current_node->children.empty()) { diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h index 0048ea2de36a..8b15b86426b9 100644 --- a/src/permission/fs_permission.h +++ b/src/permission/fs_permission.h @@ -16,14 +16,14 @@ namespace permission { class FSPermission final : public PermissionBase { public: void Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) override; void Drop(Environment* env, PermissionScope scope, - const std::string_view& param = "") override; + std::string_view param) override; bool is_granted(Environment* env, PermissionScope perm, - const std::string_view& param) const override; + std::string_view param) const override; struct RadixTree { struct Node { @@ -146,8 +146,8 @@ class FSPermission final : public PermissionBase { ~RadixTree(); void Insert(const std::string& s); void Clear(); - bool Lookup(const std::string_view& s) const { return Lookup(s, false); } - bool Lookup(const std::string_view& s, bool when_empty_return) const; + bool Lookup(std::string_view s) const { return Lookup(s, false); } + bool Lookup(std::string_view s, bool when_empty_return) const; private: Node* root_node_; diff --git a/src/permission/inspector_permission.cc b/src/permission/inspector_permission.cc index ee775e778dcf..d884ec4d8208 100644 --- a/src/permission/inspector_permission.cc +++ b/src/permission/inspector_permission.cc @@ -9,20 +9,20 @@ namespace permission { // Currently, Inspector manage a single state // Once denied, it's always denied void InspectorPermission::Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) { deny_all_ = true; } void InspectorPermission::Drop(Environment* env, PermissionScope scope, - const std::string_view& param) { + std::string_view param) { deny_all_ = true; } bool InspectorPermission::is_granted(Environment* env, PermissionScope perm, - const std::string_view& param) const { + std::string_view param) const { return deny_all_ == false; } diff --git a/src/permission/inspector_permission.h b/src/permission/inspector_permission.h index d851fb2fa253..2490149caa91 100644 --- a/src/permission/inspector_permission.h +++ b/src/permission/inspector_permission.h @@ -13,14 +13,14 @@ namespace permission { class InspectorPermission final : public PermissionBase { public: void Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) override; void Drop(Environment* env, PermissionScope scope, - const std::string_view& param = "") override; + std::string_view param) override; bool is_granted(Environment* env, PermissionScope perm, - const std::string_view& param = "") const override; + std::string_view param) const override; private: bool deny_all_; diff --git a/src/permission/net_permission.cc b/src/permission/net_permission.cc index 5f1cc139aa74..da42215ecfdc 100644 --- a/src/permission/net_permission.cc +++ b/src/permission/net_permission.cc @@ -8,20 +8,20 @@ namespace node { namespace permission { void NetPermission::Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) { allow_net_ = true; } void NetPermission::Drop(Environment* env, PermissionScope scope, - const std::string_view& param) { + std::string_view param) { allow_net_ = false; } bool NetPermission::is_granted(Environment* env, PermissionScope perm, - const std::string_view& param) const { + std::string_view param) const { return allow_net_; } diff --git a/src/permission/net_permission.h b/src/permission/net_permission.h index 26b055b255a6..23b643a50222 100644 --- a/src/permission/net_permission.h +++ b/src/permission/net_permission.h @@ -13,14 +13,14 @@ namespace permission { class NetPermission final : public PermissionBase { public: void Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) override; void Drop(Environment* env, PermissionScope scope, - const std::string_view& param = "") override; + std::string_view param) override; bool is_granted(Environment* env, PermissionScope perm, - const std::string_view& param) const override; + std::string_view param) const override; private: bool allow_net_ = false; diff --git a/src/permission/openssl_store_permission.cc b/src/permission/openssl_store_permission.cc index fcae2772b3f5..e799b5373344 100644 --- a/src/permission/openssl_store_permission.cc +++ b/src/permission/openssl_store_permission.cc @@ -10,20 +10,20 @@ namespace permission { // OpenSSLStorePermission manages a single global deny state for the use of // OpenSSL STORE loaders. void OpenSSLStorePermission::Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) { deny_all_ = true; } void OpenSSLStorePermission::Drop(Environment* env, PermissionScope scope, - const std::string_view& param) { + std::string_view param) { deny_all_ = true; } bool OpenSSLStorePermission::is_granted(Environment* env, PermissionScope perm, - const std::string_view& param) const { + std::string_view param) const { return perm != PermissionScope::kOpenSSLStore || !deny_all_; } diff --git a/src/permission/openssl_store_permission.h b/src/permission/openssl_store_permission.h index d64475228e18..8cac21f9f288 100644 --- a/src/permission/openssl_store_permission.h +++ b/src/permission/openssl_store_permission.h @@ -13,14 +13,14 @@ namespace permission { class OpenSSLStorePermission final : public PermissionBase { public: void Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) override; void Drop(Environment* env, PermissionScope scope, - const std::string_view& param = "") override; + std::string_view param) override; bool is_granted(Environment* env, PermissionScope perm, - const std::string_view& param = "") const override; + std::string_view param) const override; private: bool deny_all_ = false; diff --git a/src/permission/permission.cc b/src/permission/permission.cc index 6bf3b0fe04eb..f16fafe5876e 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -59,7 +59,7 @@ constexpr std::string_view GetDiagnosticsChannelName(PermissionScope scope) { } } -Local GetPermissionDiagnosicsTemplate(Environment* env) { +Local GetPermissionDiagnosticsTemplate(Environment* env) { auto tmpl = env->permission_diagnostic_channel_message(); if (tmpl.IsEmpty()) { static constexpr std::string_view names[] = { @@ -93,7 +93,7 @@ static void Drop(const FunctionCallbackInfo& args) { } } - env->permission()->Drop(env, scope); + env->permission()->Drop(env, scope, ""); } // permission.has('fs.in', '/tmp/') @@ -125,8 +125,8 @@ static void Has(const FunctionCallbackInfo& args) { #define V(Name, label, _, __) \ if (perm == PermissionScope::k##Name) return env->Name##_permission_string(); -v8::Local Permission::PermissionToString( - Environment* env, const PermissionScope perm) { +v8::Local Permission::PermissionToString(Environment* env, + PermissionScope perm) { PERMISSIONS(V) UNREACHABLE(); } @@ -134,7 +134,7 @@ v8::Local Permission::PermissionToString( #define V(Name, label, _, __) \ if (perm == label) return PermissionScope::k##Name; -PermissionScope Permission::StringToPermission(const std::string& perm) { +PermissionScope Permission::StringToPermission(std::string_view perm) { PERMISSIONS(V) return PermissionScope::kPermissionsRoot; } @@ -206,7 +206,7 @@ const char* GetErrorFlagSuggestion(node::permission::PermissionScope perm) { MaybeLocal CreateAccessDeniedError(Environment* env, PermissionScope perm, - const std::string_view& res) { + std::string_view res) { const char* suggestion = GetErrorFlagSuggestion(perm); Local err = ERR_ACCESS_DENIED( env->isolate(), "Access to this API has been restricted. %s", suggestion); @@ -226,7 +226,7 @@ MaybeLocal CreateAccessDeniedError(Environment* env, void Permission::ThrowAccessDenied(Environment* env, PermissionScope perm, - const std::string_view& res) { + std::string_view res) { Local err; if (CreateAccessDeniedError(env, perm, res).ToLocal(&err)) { env->isolate()->ThrowException(err); @@ -238,7 +238,7 @@ void Permission::ThrowAccessDenied(Environment* env, void Permission::AsyncThrowAccessDenied(Environment* env, fs::FSReqBase* req_wrap, PermissionScope perm, - const std::string_view& res) { + std::string_view res) { Local err; if (CreateAccessDeniedError(env, perm, res).ToLocal(&err)) { return req_wrap->Reject(err); @@ -260,8 +260,8 @@ void Permission::EnableWarningOnly() { } bool Permission::is_scope_granted(Environment* env, - const PermissionScope permission, - const std::string_view& res) const { + PermissionScope permission, + std::string_view res) const { CHECK(permission != PermissionScope::kPermissionsRoot && permission != PermissionScope::kPermissionsCount); auto perm_node = nodes_.find(permission); @@ -284,7 +284,7 @@ bool Permission::is_scope_granted(Environment* env, }; ch->Publish( env, - GetPermissionDiagnosicsTemplate(env)->NewInstance(context, values)); + GetPermissionDiagnosticsTemplate(env)->NewInstance(context, values)); publishing_ = false; } } @@ -313,7 +313,7 @@ BaseObjectPtr Permission::GetOrCreateChannel( } void Permission::Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) { auto permission = nodes_.find(scope); if (permission != nodes_.end()) { @@ -323,7 +323,7 @@ void Permission::Apply(Environment* env, void Permission::Drop(Environment* env, PermissionScope scope, - const std::string_view& param) { + std::string_view param) { CHECK(scope != PermissionScope::kPermissionsRoot && scope != PermissionScope::kPermissionsCount); auto permission = nodes_.find(scope); @@ -347,7 +347,7 @@ void Permission::Drop(Environment* env, }; ch->Publish( env, - GetPermissionDiagnosicsTemplate(env)->NewInstance(context, values)); + GetPermissionDiagnosticsTemplate(env)->NewInstance(context, values)); publishing_ = false; } } diff --git a/src/permission/permission.h b/src/permission/permission.h index 674d99c1409a..dd2d9b16ef81 100644 --- a/src/permission/permission.h +++ b/src/permission/permission.h @@ -100,8 +100,8 @@ class Permission { Permission(); FORCE_INLINE bool is_granted(Environment* env, - const PermissionScope permission, - const std::string_view& res = "") const { + PermissionScope permission, + std::string_view res = "") const { if (!enabled_) [[likely]] { return true; } @@ -112,32 +112,30 @@ class Permission { FORCE_INLINE bool warning_only() const { return warning_only_; } - static PermissionScope StringToPermission(const std::string& perm); + static PermissionScope StringToPermission(std::string_view perm); static v8::Local PermissionToString(Environment* env, PermissionScope perm); static void ThrowAccessDenied(Environment* env, PermissionScope perm, - const std::string_view& res); + std::string_view res); static void AsyncThrowAccessDenied(Environment* env, fs::FSReqBase* req_wrap, PermissionScope perm, - const std::string_view& res); + std::string_view res); // CLI Call void Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope); // Runtime Call - void Drop(Environment* env, - PermissionScope scope, - const std::string_view& param = ""); + void Drop(Environment* env, PermissionScope scope, std::string_view param); void EnablePermissions(); void EnableWarningOnly(); private: COLD_NOINLINE bool is_scope_granted(Environment* env, - const PermissionScope permission, - const std::string_view& res = "") const; + PermissionScope permission, + std::string_view res = "") const; BaseObjectPtr GetOrCreateChannel( Environment* env, PermissionScope scope) const; @@ -155,7 +153,7 @@ class Permission { v8::MaybeLocal CreateAccessDeniedError(Environment* env, PermissionScope perm, - const std::string_view& res); + std::string_view res); } // namespace permission diff --git a/src/permission/permission_base.h b/src/permission/permission_base.h index 66bfd33a8787..658175611ecd 100644 --- a/src/permission/permission_base.h +++ b/src/permission/permission_base.h @@ -3,10 +3,9 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include +#include #include #include -#include "v8.h" namespace node { @@ -60,15 +59,16 @@ enum class PermissionScope { class PermissionBase { public: + virtual ~PermissionBase() = default; virtual void Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) = 0; virtual void Drop(Environment* env, PermissionScope scope, - const std::string_view& param = "") = 0; + std::string_view param) = 0; virtual bool is_granted(Environment* env, PermissionScope perm, - const std::string_view& param = "") const = 0; + std::string_view param) const = 0; }; } // namespace permission diff --git a/src/permission/wasi_permission.cc b/src/permission/wasi_permission.cc index 00ce927eb625..cf3be848c31f 100644 --- a/src/permission/wasi_permission.cc +++ b/src/permission/wasi_permission.cc @@ -10,20 +10,20 @@ namespace permission { // Currently, WASIPermission manage a single state // Once denied, it's always denied void WASIPermission::Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) { deny_all_ = true; } void WASIPermission::Drop(Environment* env, PermissionScope scope, - const std::string_view& param) { + std::string_view param) { deny_all_ = true; } bool WASIPermission::is_granted(Environment* env, PermissionScope perm, - const std::string_view& param) const { + std::string_view param) const { return deny_all_ == false; } diff --git a/src/permission/wasi_permission.h b/src/permission/wasi_permission.h index b5cdaca928dd..1d341c1a7334 100644 --- a/src/permission/wasi_permission.h +++ b/src/permission/wasi_permission.h @@ -13,14 +13,14 @@ namespace permission { class WASIPermission final : public PermissionBase { public: void Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) override; void Drop(Environment* env, PermissionScope scope, - const std::string_view& param = "") override; + std::string_view param) override; bool is_granted(Environment* env, PermissionScope perm, - const std::string_view& param = "") const override; + std::string_view param) const override; private: bool deny_all_; diff --git a/src/permission/worker_permission.cc b/src/permission/worker_permission.cc index aa6867eb1e0f..395876d29cce 100644 --- a/src/permission/worker_permission.cc +++ b/src/permission/worker_permission.cc @@ -10,20 +10,20 @@ namespace permission { // Currently, PolicyDenyWorker manage a single state // Once denied, it's always denied void WorkerPermission::Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) { deny_all_ = true; } void WorkerPermission::Drop(Environment* env, PermissionScope scope, - const std::string_view& param) { + std::string_view param) { deny_all_ = true; } bool WorkerPermission::is_granted(Environment* env, PermissionScope perm, - const std::string_view& param) const { + std::string_view param) const { return deny_all_ == false; } diff --git a/src/permission/worker_permission.h b/src/permission/worker_permission.h index fc7abe0d50f4..4480b75646d2 100644 --- a/src/permission/worker_permission.h +++ b/src/permission/worker_permission.h @@ -13,14 +13,14 @@ namespace permission { class WorkerPermission final : public PermissionBase { public: void Apply(Environment* env, - const std::vector& allow, + std::span allow, PermissionScope scope) override; void Drop(Environment* env, PermissionScope scope, - const std::string_view& param = "") override; + std::string_view param) override; bool is_granted(Environment* env, PermissionScope perm, - const std::string_view& param = "") const override; + std::string_view param) const override; private: bool deny_all_; From 1998f95e62e75ba9f357e6ff91e63990e7cb040e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Aug 2026 18:31:22 -0700 Subject: [PATCH 06/11] src: make permission storage a bit more efficient Use a fixed array rather than an unordered list Signed-off-by: James M Snell --- src/permission/permission.cc | 72 ++++++++++++++++-------------------- src/permission/permission.h | 11 ++++-- 2 files changed, 39 insertions(+), 44 deletions(-) diff --git a/src/permission/permission.cc b/src/permission/permission.cc index f16fafe5876e..fd4b1ab34907 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -141,53 +141,49 @@ PermissionScope Permission::StringToPermission(std::string_view perm) { #undef V Permission::Permission() : enabled_(false), warning_only_(false) { - std::shared_ptr fs = std::make_shared(); - std::shared_ptr child_p = - std::make_shared(); - std::shared_ptr worker_t = - std::make_shared(); - std::shared_ptr inspector = - std::make_shared(); - std::shared_ptr wasi = std::make_shared(); - std::shared_ptr net = std::make_shared(); - std::shared_ptr addon = std::make_shared(); - std::shared_ptr ffi = std::make_shared(); - std::shared_ptr openssl_store = - std::make_shared(); + auto fs = std::make_shared(); + auto child_p = std::make_shared(); + auto worker_t = std::make_shared(); + auto inspector = std::make_shared(); + auto wasi = std::make_shared(); + auto net = std::make_shared(); + auto addon = std::make_shared(); + auto ffi = std::make_shared(); + auto openssl_store = std::make_shared(); #define V(Name, _, __, ___) \ - nodes_.insert(std::make_pair(PermissionScope::k##Name, fs)); + nodes_[static_cast(PermissionScope::k##Name)] = fs; FILESYSTEM_PERMISSIONS(V) #undef V #define V(Name, _, __, ___) \ - nodes_.insert(std::make_pair(PermissionScope::k##Name, child_p)); + nodes_[static_cast(PermissionScope::k##Name)] = child_p; CHILD_PROCESS_PERMISSIONS(V) #undef V #define V(Name, _, __, ___) \ - nodes_.insert(std::make_pair(PermissionScope::k##Name, worker_t)); + nodes_[static_cast(PermissionScope::k##Name)] = worker_t; WORKER_THREADS_PERMISSIONS(V) #undef V #define V(Name, _, __, ___) \ - nodes_.insert(std::make_pair(PermissionScope::k##Name, inspector)); + nodes_[static_cast(PermissionScope::k##Name)] = inspector; INSPECTOR_PERMISSIONS(V) #undef V #define V(Name, _, __, ___) \ - nodes_.insert(std::make_pair(PermissionScope::k##Name, wasi)); + nodes_[static_cast(PermissionScope::k##Name)] = wasi; WASI_PERMISSIONS(V) #undef V #define V(Name, _, __, ___) \ - nodes_.insert(std::make_pair(PermissionScope::k##Name, net)); + nodes_[static_cast(PermissionScope::k##Name)] = net; NET_PERMISSIONS(V) #undef V #define V(Name, _, __, ___) \ - nodes_.insert(std::make_pair(PermissionScope::k##Name, addon)); + nodes_[static_cast(PermissionScope::k##Name)] = addon; ADDON_PERMISSIONS(V) #undef V #define V(Name, _, __, ___) \ - nodes_.insert(std::make_pair(PermissionScope::k##Name, ffi)); + nodes_[static_cast(PermissionScope::k##Name)] = ffi; FFI_PERMISSIONS(V) #undef V #define V(Name, _, __, ___) \ - nodes_.insert(std::make_pair(PermissionScope::k##Name, openssl_store)); + nodes_[static_cast(PermissionScope::k##Name)] = openssl_store; OPENSSL_STORE_PERMISSIONS(V) #undef V } @@ -264,10 +260,10 @@ bool Permission::is_scope_granted(Environment* env, std::string_view res) const { CHECK(permission != PermissionScope::kPermissionsRoot && permission != PermissionScope::kPermissionsCount); - auto perm_node = nodes_.find(permission); + auto& perm_node = nodes_[static_cast(permission)]; bool result = false; - if (perm_node != nodes_.end()) { - result = perm_node->second->is_granted(env, permission, res); + if (perm_node) { + result = perm_node->is_granted(env, permission, res); } if (!result && !publishing_) { @@ -296,17 +292,13 @@ BaseObjectPtr Permission::GetOrCreateChannel( Environment* env, PermissionScope scope) const { CHECK(scope != PermissionScope::kPermissionsRoot && scope != PermissionScope::kPermissionsCount); - auto it = channels_.find(scope); - if (it != channels_.end()) { - // Promote weak ref to strong for the duration of this call. - BaseObjectPtr ptr(it->second.get()); - if (ptr) return ptr; - channels_.erase(it); - } + auto& weak_ch = channels_[static_cast(scope)]; + // Promote weak ref to strong for the duration of this call. + BaseObjectPtr ptr(weak_ch.get()); + if (ptr) return ptr; auto channel_name = GetDiagnosticsChannelName(scope); if (auto ch = diagnostics_channel::Channel::Get(env, channel_name)) { - channels_.emplace(scope, - BaseObjectWeakPtr(ch)); + weak_ch = BaseObjectWeakPtr(ch.get()); return ch; } return {}; @@ -315,9 +307,9 @@ BaseObjectPtr Permission::GetOrCreateChannel( void Permission::Apply(Environment* env, std::span allow, PermissionScope scope) { - auto permission = nodes_.find(scope); - if (permission != nodes_.end()) { - permission->second->Apply(env, allow, scope); + auto& perm_node = nodes_[static_cast(scope)]; + if (perm_node) { + perm_node->Apply(env, allow, scope); } } @@ -326,9 +318,9 @@ void Permission::Drop(Environment* env, std::string_view param) { CHECK(scope != PermissionScope::kPermissionsRoot && scope != PermissionScope::kPermissionsCount); - auto permission = nodes_.find(scope); - if (permission != nodes_.end()) { - permission->second->Drop(env, scope, param); + auto& perm_node = nodes_[static_cast(scope)]; + if (perm_node) { + perm_node->Drop(env, scope, param); } // Publish to diagnostics channel so observers can track drops diff --git a/src/permission/permission.h b/src/permission/permission.h index dd2d9b16ef81..d93399d07642 100644 --- a/src/permission/permission.h +++ b/src/permission/permission.h @@ -18,8 +18,8 @@ #include "permission/worker_permission.h" #include "v8.h" +#include #include -#include namespace node { @@ -140,14 +140,17 @@ class Permission { BaseObjectPtr GetOrCreateChannel( Environment* env, PermissionScope scope) const; - std::unordered_map> nodes_; + static constexpr size_t kPermissionCount = + static_cast(PermissionScope::kPermissionsCount); + + std::array, kPermissionCount> nodes_; bool enabled_; bool warning_only_; mutable bool publishing_ = false; // Weak refs: BindingData (via BaseObjectPtr) is the sole owner of Channels. // Using weak refs here avoids keeping Channels alive past Realm teardown. - mutable std::unordered_map> + mutable std::array, + kPermissionCount> channels_; }; From 77a3aa4e4a39d4f382dfceb93c45abf98140278e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Aug 2026 18:51:52 -0700 Subject: [PATCH 07/11] src: apply a modest performance perf to permissions Improve the way the RadixTree works and apply a fast api call. Signed-off-by: James M Snell --- src/permission/fs_permission.cc | 21 +++++------- src/permission/fs_permission.h | 58 ++++++++++++++++++++------------- src/permission/permission.cc | 49 +++++++++++++++++++++++++++- 3 files changed, 93 insertions(+), 35 deletions(-) diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc index 89f1c6d70c98..c283fb40f091 100644 --- a/src/permission/fs_permission.cc +++ b/src/permission/fs_permission.cc @@ -39,10 +39,8 @@ void FreeRecursivelyNode( return; } - if (node->children.size()) { - for (auto& c : node->children) { - FreeRecursivelyNode(c.second); - } + for (auto& [label, child] : node->children) { + FreeRecursivelyNode(child); } delete node->wildcard_child; @@ -106,7 +104,7 @@ void PrintTree(const node::permission::FSPermission::RadixTree::Node* node, node::DebugCategory::PERMISSION_MODEL, "%s%s\n", indent, node->prefix); } - if (node->children.size() > 0) { + if (!node->children.empty()) { size_t count = 0; size_t total = node->children.size(); @@ -120,10 +118,10 @@ void PrintTree(const node::permission::FSPermission::RadixTree::Node* node, } } - for (const auto& pair : node->children) { + for (const auto& [label, child] : node->children) { count++; bool child_is_last = (count == total); - PrintTree(pair.second, depth + 1, next_branch_prefix, child_is_last); + PrintTree(child, depth + 1, next_branch_prefix, child_is_last); } } } @@ -278,8 +276,8 @@ FSPermission::RadixTree::~RadixTree() { } void FSPermission::RadixTree::Clear() { - for (auto& c : root_node_->children) { - FreeRecursivelyNode(c.second); + for (auto& [label, child] : root_node_->children) { + FreeRecursivelyNode(child); } root_node_->children.clear(); delete root_node_->wildcard_child; @@ -294,15 +292,14 @@ bool FSPermission::RadixTree::Lookup(std::string_view s, return when_empty_return; } size_t parent_node_prefix_len = current_node->prefix.length(); - const std::string path(s); - auto path_len = path.length(); + auto path_len = s.length(); while (true) { if (parent_node_prefix_len == path_len && current_node->IsEndNode()) { return true; } - auto node = current_node->NextNode(path, parent_node_prefix_len); + auto node = current_node->NextNode(s, parent_node_prefix_len); if (node == nullptr) { return false; } diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h index 8b15b86426b9..6af29ce7bd1b 100644 --- a/src/permission/fs_permission.h +++ b/src/permission/fs_permission.h @@ -5,7 +5,7 @@ #include "v8.h" -#include +#include #include "permission/permission_base.h" #include "util.h" @@ -28,16 +28,30 @@ class FSPermission final : public PermissionBase { struct RadixTree { struct Node { std::string prefix; - std::unordered_map children; - Node* wildcard_child; - bool is_leaf; + std::vector> children; + Node* wildcard_child = nullptr; + bool is_leaf = false; - explicit Node(const std::string& pre) - : prefix(pre), wildcard_child(nullptr), is_leaf(false) {} + explicit Node(std::string_view pre) + : prefix(pre) {} - Node() : wildcard_child(nullptr), is_leaf(false) {} + Node() = default; - Node* CreateChild(const std::string& path_prefix) { + Node* FindChild(char label) const { + for (const auto& [c, node] : children) { + if (c == label) return node; + } + return nullptr; + } + + void SetChild(char label, Node* node) { + for (auto& [c, n] : children) { + if (c == label) { n = node; return; } + } + children.emplace_back(label, node); + } + + Node* CreateChild(std::string_view path_prefix) { if (path_prefix.empty() && !is_leaf) { is_leaf = true; return this; @@ -46,10 +60,11 @@ class FSPermission final : public PermissionBase { CHECK(!path_prefix.empty()); char label = path_prefix[0]; - Node* child = children[label]; + Node* child = FindChild(label); if (child == nullptr) { - children[label] = new Node(path_prefix); - return children[label]; + child = new Node(path_prefix); + children.emplace_back(label, child); + return child; } bool child_was_end_node = child->IsEndNode(); @@ -58,13 +73,13 @@ class FSPermission final : public PermissionBase { size_t prefix_len = path_prefix.length(); for (; i < child->prefix.length(); ++i) { if (i >= prefix_len || path_prefix[i] != child->prefix[i]) { - std::string parent_prefix = child->prefix.substr(0, i); - std::string child_prefix = child->prefix.substr(i); + std::string parent_prefix(child->prefix.substr(0, i)); + std::string child_prefix(child->prefix.substr(i)); child->prefix = child_prefix; Node* split_child = new Node(parent_prefix); - split_child->children[child_prefix[0]] = child; - children[parent_prefix[0]] = split_child; + split_child->children.emplace_back(child_prefix[0], child); + SetChild(parent_prefix[0], split_child); return split_child->CreateChild(path_prefix.substr(i)); } @@ -83,24 +98,23 @@ class FSPermission final : public PermissionBase { return wildcard_child; } - Node* NextNode(const std::string& path, size_t idx) const { + Node* NextNode(std::string_view path, size_t idx) const { if (idx >= path.length()) { return nullptr; } // wildcard node takes precedence if (children.size() > 1) { - auto it = children.find('*'); - if (it != children.end()) { - return it->second; + Node* wc = FindChild('*'); + if (wc != nullptr) { + return wc; } } - auto it = children.find(path[idx]); - if (it == children.end()) { + Node* child = FindChild(path[idx]); + if (child == nullptr) { return nullptr; } - auto child = it->second; // match prefix size_t prefix_len = child->prefix.length(); for (size_t i = 0; i < path.length(); ++i) { diff --git a/src/permission/permission.cc b/src/permission/permission.cc index fd4b1ab34907..cee5172a9ba2 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -3,12 +3,14 @@ #include "env-inl.h" #include "memory_tracker-inl.h" #include "node.h" +#include "node_debug.h" #include "node_diagnostics_channel.h" #include "node_errors.h" #include "node_external_reference.h" #include "node_file.h" #include "permission/permission_base.h" +#include "v8-fast-api-calls.h" #include "v8-template.h" #include "v8.h" @@ -18,13 +20,16 @@ namespace node { +using v8::CFunction; using v8::Context; using v8::DictionaryTemplate; +using v8::FastApiCallbackOptions; using v8::FunctionCallbackInfo; using v8::IntegrityLevel; using v8::Local; using v8::MaybeLocal; using v8::Object; +using v8::String; using v8::Undefined; using v8::Value; @@ -121,6 +126,47 @@ static void Has(const FunctionCallbackInfo& args) { return args.GetReturnValue().Set(env->permission()->is_granted(env, scope)); } +static bool FastHas(Local receiver, + Local scope_arg, + Local resource_arg, + // NOLINTNEXTLINE(runtime/references) This is V8 api. + FastApiCallbackOptions& options) { + TRACK_V8_FAST_API_CALL("permission.has"); + auto isolate = options.isolate; + v8::HandleScope handle_scope(isolate); + auto context = isolate->GetCurrentContext(); + + Environment* env = Environment::GetCurrent(context); + + Local str; + if (!scope_arg->ToString(context).ToLocal(&str)) { + return false; + } + Utf8Value utf8_scope(isolate, str); + PermissionScope scope = + Permission::StringToPermission(utf8_scope.ToStringView()); + if (scope == PermissionScope::kPermissionsRoot) { + return false; + } + + if (resource_arg->IsUndefined()) { + return env->permission()->is_granted(env, scope); + } + + Local res_str; + if (!resource_arg->ToString(context).ToLocal(&res_str)) { + return false; + } + Utf8Value utf8_res(isolate, res_str); + if (utf8_res.length() == 0) { + return false; + } + + return env->permission()->is_granted(env, scope, utf8_res.ToStringView()); +} + +static CFunction fast_has_(CFunction::Make(FastHas)); + } // namespace #define V(Name, label, _, __) \ @@ -349,7 +395,7 @@ void Initialize(Local target, Local unused, Local context, void* priv) { - SetMethodNoSideEffect(context, target, "has", Has); + SetFastMethodNoSideEffect(context, target, "has", Has, &fast_has_); SetMethod(context, target, "drop", Drop); target->SetIntegrityLevel(context, IntegrityLevel::kFrozen).FromJust(); @@ -357,6 +403,7 @@ void Initialize(Local target, void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(Has); + registry->Register(fast_has_); registry->Register(Drop); } From 857db7118060a98f698690e860d7eab36025c8f5 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Aug 2026 19:06:43 -0700 Subject: [PATCH 08/11] src: simplify includes in permissions Signed-off-by: James M Snell --- src/permission/child_process_permission.cc | 1 - src/permission/child_process_permission.h | 1 - src/permission/ffi_permission.cc | 1 - src/permission/ffi_permission.h | 1 - src/permission/fs_permission.cc | 7 ++----- src/permission/fs_permission.h | 2 -- src/permission/net_permission.cc | 1 - src/permission/openssl_store_permission.cc | 1 - src/permission/openssl_store_permission.h | 1 - src/permission/permission.cc | 12 +++++++++--- src/permission/permission.h | 11 ----------- src/permission/wasi_permission.cc | 1 - src/permission/wasi_permission.h | 1 - src/permission/worker_permission.cc | 1 - src/permission/worker_permission.h | 1 - 15 files changed, 11 insertions(+), 32 deletions(-) diff --git a/src/permission/child_process_permission.cc b/src/permission/child_process_permission.cc index 25c9713e0570..2f1c1796db15 100644 --- a/src/permission/child_process_permission.cc +++ b/src/permission/child_process_permission.cc @@ -1,7 +1,6 @@ #include "child_process_permission.h" #include -#include namespace node { diff --git a/src/permission/child_process_permission.h b/src/permission/child_process_permission.h index 59ab74bb5116..1454c8eef51b 100644 --- a/src/permission/child_process_permission.h +++ b/src/permission/child_process_permission.h @@ -3,7 +3,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include #include "permission/permission_base.h" namespace node { diff --git a/src/permission/ffi_permission.cc b/src/permission/ffi_permission.cc index 674394144471..2989ae4179af 100644 --- a/src/permission/ffi_permission.cc +++ b/src/permission/ffi_permission.cc @@ -1,7 +1,6 @@ #include "permission/ffi_permission.h" #include -#include namespace node { diff --git a/src/permission/ffi_permission.h b/src/permission/ffi_permission.h index fcb8b403c725..9562fce9f54a 100644 --- a/src/permission/ffi_permission.h +++ b/src/permission/ffi_permission.h @@ -3,7 +3,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include #include "permission/permission_base.h" namespace node { diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc index c283fb40f091..b2f3cbb31a6e 100644 --- a/src/permission/fs_permission.cc +++ b/src/permission/fs_permission.cc @@ -1,15 +1,12 @@ #include "fs_permission.h" -#include "base_object-inl.h" #include "debug_utils-inl.h" #include "env.h" #include "path.h" -#include "v8.h" #include -#include -#include #include -#include +#include +#include #include #include #include diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h index 6af29ce7bd1b..e26c7a44feaa 100644 --- a/src/permission/fs_permission.h +++ b/src/permission/fs_permission.h @@ -3,8 +3,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "v8.h" - #include #include "permission/permission_base.h" #include "util.h" diff --git a/src/permission/net_permission.cc b/src/permission/net_permission.cc index da42215ecfdc..10b635e82e6d 100644 --- a/src/permission/net_permission.cc +++ b/src/permission/net_permission.cc @@ -1,6 +1,5 @@ #include "net_permission.h" -#include #include namespace node { diff --git a/src/permission/openssl_store_permission.cc b/src/permission/openssl_store_permission.cc index e799b5373344..11d4ff82faf8 100644 --- a/src/permission/openssl_store_permission.cc +++ b/src/permission/openssl_store_permission.cc @@ -1,7 +1,6 @@ #include "permission/openssl_store_permission.h" #include -#include namespace node { diff --git a/src/permission/openssl_store_permission.h b/src/permission/openssl_store_permission.h index 8cac21f9f288..418e93e0d899 100644 --- a/src/permission/openssl_store_permission.h +++ b/src/permission/openssl_store_permission.h @@ -3,7 +3,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include #include "permission/permission_base.h" namespace node { diff --git a/src/permission/permission.cc b/src/permission/permission.cc index cee5172a9ba2..e7e8ba6b2ea2 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -1,7 +1,5 @@ #include "permission.h" -#include "base_object-inl.h" #include "env-inl.h" -#include "memory_tracker-inl.h" #include "node.h" #include "node_debug.h" #include "node_diagnostics_channel.h" @@ -9,6 +7,15 @@ #include "node_external_reference.h" #include "node_file.h" +#include "permission/addon_permission.h" +#include "permission/child_process_permission.h" +#include "permission/ffi_permission.h" +#include "permission/fs_permission.h" +#include "permission/inspector_permission.h" +#include "permission/net_permission.h" +#include "permission/openssl_store_permission.h" +#include "permission/wasi_permission.h" +#include "permission/worker_permission.h" #include "permission/permission_base.h" #include "v8-fast-api-calls.h" #include "v8-template.h" @@ -16,7 +23,6 @@ #include #include -#include namespace node { diff --git a/src/permission/permission.h b/src/permission/permission.h index d93399d07642..6ed211f955b4 100644 --- a/src/permission/permission.h +++ b/src/permission/permission.h @@ -5,18 +5,7 @@ #include "debug_utils.h" #include "node_diagnostics_channel.h" -#include "node_options.h" -#include "permission/addon_permission.h" -#include "permission/child_process_permission.h" -#include "permission/ffi_permission.h" -#include "permission/fs_permission.h" -#include "permission/inspector_permission.h" -#include "permission/net_permission.h" -#include "permission/openssl_store_permission.h" #include "permission/permission_base.h" -#include "permission/wasi_permission.h" -#include "permission/worker_permission.h" -#include "v8.h" #include #include diff --git a/src/permission/wasi_permission.cc b/src/permission/wasi_permission.cc index cf3be848c31f..5891edc92aa3 100644 --- a/src/permission/wasi_permission.cc +++ b/src/permission/wasi_permission.cc @@ -1,7 +1,6 @@ #include "permission/wasi_permission.h" #include -#include namespace node { diff --git a/src/permission/wasi_permission.h b/src/permission/wasi_permission.h index 1d341c1a7334..fbca0e4e0879 100644 --- a/src/permission/wasi_permission.h +++ b/src/permission/wasi_permission.h @@ -3,7 +3,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include #include "permission/permission_base.h" namespace node { diff --git a/src/permission/worker_permission.cc b/src/permission/worker_permission.cc index 395876d29cce..bdf0519d3bb0 100644 --- a/src/permission/worker_permission.cc +++ b/src/permission/worker_permission.cc @@ -1,7 +1,6 @@ #include "permission/worker_permission.h" #include -#include namespace node { diff --git a/src/permission/worker_permission.h b/src/permission/worker_permission.h index 4480b75646d2..ba4b45319ba9 100644 --- a/src/permission/worker_permission.h +++ b/src/permission/worker_permission.h @@ -3,7 +3,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include #include "permission/permission_base.h" namespace node { From 78bfa4fa0d27c1aa4792fb3e3980fb8ffd4d5b08 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Aug 2026 19:21:40 -0700 Subject: [PATCH 09/11] src: simplify permissions with BooleanPermissions Most of the PermissionBase subclasses used the identical simple pattern. Rather than define a bunch of individual identical permissions, use a single utility definition. Special cases like FsPermission are still possible but the simple case is kept... well, simple. Signed-off-by: James M Snell --- node.gyp | 18 +------- src/permission/addon_permission.cc | 30 ------------ src/permission/addon_permission.h | 34 -------------- src/permission/boolean_permission.h | 54 ++++++++++++++++++++++ src/permission/child_process_permission.cc | 30 ------------ src/permission/child_process_permission.h | 33 ------------- src/permission/ffi_permission.cc | 29 ------------ src/permission/ffi_permission.h | 33 ------------- src/permission/fs_permission.h | 8 ++-- src/permission/inspector_permission.cc | 30 ------------ src/permission/inspector_permission.h | 34 -------------- src/permission/net_permission.cc | 28 ----------- src/permission/net_permission.h | 34 -------------- src/permission/openssl_store_permission.cc | 30 ------------ src/permission/openssl_store_permission.h | 33 ------------- src/permission/permission.cc | 45 +++--------------- src/permission/wasi_permission.cc | 30 ------------ src/permission/wasi_permission.h | 33 ------------- src/permission/worker_permission.cc | 30 ------------ src/permission/worker_permission.h | 33 ------------- 20 files changed, 68 insertions(+), 561 deletions(-) delete mode 100644 src/permission/addon_permission.cc delete mode 100644 src/permission/addon_permission.h create mode 100644 src/permission/boolean_permission.h delete mode 100644 src/permission/child_process_permission.cc delete mode 100644 src/permission/child_process_permission.h delete mode 100644 src/permission/ffi_permission.cc delete mode 100644 src/permission/ffi_permission.h delete mode 100644 src/permission/inspector_permission.cc delete mode 100644 src/permission/inspector_permission.h delete mode 100644 src/permission/net_permission.cc delete mode 100644 src/permission/net_permission.h delete mode 100644 src/permission/openssl_store_permission.cc delete mode 100644 src/permission/openssl_store_permission.h delete mode 100644 src/permission/wasi_permission.cc delete mode 100644 src/permission/wasi_permission.h delete mode 100644 src/permission/worker_permission.cc delete mode 100644 src/permission/worker_permission.h diff --git a/node.gyp b/node.gyp index 7bff8e8a4e7f..35bdb0e2e176 100644 --- a/node.gyp +++ b/node.gyp @@ -178,16 +178,8 @@ 'src/node_worker.cc', 'src/node_zlib.cc', 'src/path.cc', - 'src/permission/child_process_permission.cc', - 'src/permission/openssl_store_permission.cc', - 'src/permission/ffi_permission.cc', 'src/permission/fs_permission.cc', - 'src/permission/inspector_permission.cc', 'src/permission/permission.cc', - 'src/permission/wasi_permission.cc', - 'src/permission/worker_permission.cc', - 'src/permission/net_permission.cc', - 'src/permission/addon_permission.cc', 'src/pipe_wrap.cc', 'src/process_wrap.cc', 'src/signal_wrap.cc', @@ -314,16 +306,10 @@ 'src/node_watchdog.h', 'src/node_worker.h', 'src/path.h', - 'src/permission/child_process_permission.h', - 'src/permission/openssl_store_permission.h', - 'src/permission/ffi_permission.h', + 'src/permission/boolean_permission.h', 'src/permission/fs_permission.h', - 'src/permission/inspector_permission.h', 'src/permission/permission.h', - 'src/permission/wasi_permission.h', - 'src/permission/worker_permission.h', - 'src/permission/net_permission.h', - 'src/permission/addon_permission.h', + 'src/permission/permission_base.h', 'src/pipe_wrap.h', 'src/req_wrap.h', 'src/req_wrap-inl.h', diff --git a/src/permission/addon_permission.cc b/src/permission/addon_permission.cc deleted file mode 100644 index 249b266c5919..000000000000 --- a/src/permission/addon_permission.cc +++ /dev/null @@ -1,30 +0,0 @@ -#include "addon_permission.h" - -#include - -namespace node { - -namespace permission { - -// Currently, Addon manage a single state -// Once denied, it's always denied -void AddonPermission::Apply(Environment* env, - std::span allow, - PermissionScope scope) { - deny_all_ = true; -} - -void AddonPermission::Drop(Environment* env, - PermissionScope scope, - std::string_view param) { - deny_all_ = true; -} - -bool AddonPermission::is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const { - return deny_all_ == false; -} - -} // namespace permission -} // namespace node diff --git a/src/permission/addon_permission.h b/src/permission/addon_permission.h deleted file mode 100644 index 04e2ed6fed9a..000000000000 --- a/src/permission/addon_permission.h +++ /dev/null @@ -1,34 +0,0 @@ -#ifndef SRC_PERMISSION_ADDON_PERMISSION_H_ -#define SRC_PERMISSION_ADDON_PERMISSION_H_ - -#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#include -#include "permission/permission_base.h" - -namespace node { - -namespace permission { - -class AddonPermission final : public PermissionBase { - public: - void Apply(Environment* env, - std::span allow, - PermissionScope scope) override; - void Drop(Environment* env, - PermissionScope scope, - std::string_view param) override; - bool is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const override; - - private: - bool deny_all_; -}; - -} // namespace permission - -} // namespace node - -#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#endif // SRC_PERMISSION_ADDON_PERMISSION_H_ diff --git a/src/permission/boolean_permission.h b/src/permission/boolean_permission.h new file mode 100644 index 000000000000..d1a30972dcee --- /dev/null +++ b/src/permission/boolean_permission.h @@ -0,0 +1,54 @@ +#ifndef SRC_PERMISSION_BOOLEAN_PERMISSION_H_ +#define SRC_PERMISSION_BOOLEAN_PERMISSION_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "permission/permission_base.h" + +namespace node { + +namespace permission { + +// A simple boolean permission that either denies or allows access. +// Used for permission scopes that don't need per-resource granularity. +template +class BooleanPermission final : public PermissionBase { + public: + void Apply(Environment* env, + std::span allow, + PermissionScope scope) override { + flag_ = true; + } + + void Drop(Environment* env, + PermissionScope scope, + std::string_view param) override { + flag_ = deny_only; + } + + bool is_granted(Environment* env, + PermissionScope perm, + std::string_view param) const override { + if constexpr (deny_only) { + return !flag_; + } else { + return flag_; + } + } + + private: + bool flag_ = false; +}; + +// Once denied, the permission cannot be re-granted. +using DenyOnlyPermission = BooleanPermission; + +// Apply grants access, Drop revokes. +using AllowRevokePermission = BooleanPermission; + +} // namespace permission + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#endif // SRC_PERMISSION_BOOLEAN_PERMISSION_H_ diff --git a/src/permission/child_process_permission.cc b/src/permission/child_process_permission.cc deleted file mode 100644 index 2f1c1796db15..000000000000 --- a/src/permission/child_process_permission.cc +++ /dev/null @@ -1,30 +0,0 @@ -#include "child_process_permission.h" - -#include - -namespace node { - -namespace permission { - -// Currently, ChildProcess manage a single state -// Once denied, it's always denied -void ChildProcessPermission::Apply(Environment* env, - std::span allow, - PermissionScope scope) { - deny_all_ = true; -} - -void ChildProcessPermission::Drop(Environment* env, - PermissionScope scope, - std::string_view param) { - deny_all_ = true; -} - -bool ChildProcessPermission::is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const { - return deny_all_ == false; -} - -} // namespace permission -} // namespace node diff --git a/src/permission/child_process_permission.h b/src/permission/child_process_permission.h deleted file mode 100644 index 1454c8eef51b..000000000000 --- a/src/permission/child_process_permission.h +++ /dev/null @@ -1,33 +0,0 @@ -#ifndef SRC_PERMISSION_CHILD_PROCESS_PERMISSION_H_ -#define SRC_PERMISSION_CHILD_PROCESS_PERMISSION_H_ - -#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#include "permission/permission_base.h" - -namespace node { - -namespace permission { - -class ChildProcessPermission final : public PermissionBase { - public: - void Apply(Environment* env, - std::span allow, - PermissionScope scope) override; - void Drop(Environment* env, - PermissionScope scope, - std::string_view param) override; - bool is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const override; - - private: - bool deny_all_; -}; - -} // namespace permission - -} // namespace node - -#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#endif // SRC_PERMISSION_CHILD_PROCESS_PERMISSION_H_ diff --git a/src/permission/ffi_permission.cc b/src/permission/ffi_permission.cc deleted file mode 100644 index 2989ae4179af..000000000000 --- a/src/permission/ffi_permission.cc +++ /dev/null @@ -1,29 +0,0 @@ -#include "permission/ffi_permission.h" - -#include - -namespace node { - -namespace permission { - -// Currently, FFIPermission manages a single global deny state for FFI. -void FFIPermission::Apply(Environment* env, - std::span allow, - PermissionScope scope) { - deny_all_ = true; -} - -void FFIPermission::Drop(Environment* env, - PermissionScope scope, - std::string_view param) { - deny_all_ = true; -} - -bool FFIPermission::is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const { - return perm != PermissionScope::kFFI || !deny_all_; -} - -} // namespace permission -} // namespace node diff --git a/src/permission/ffi_permission.h b/src/permission/ffi_permission.h deleted file mode 100644 index 9562fce9f54a..000000000000 --- a/src/permission/ffi_permission.h +++ /dev/null @@ -1,33 +0,0 @@ -#ifndef SRC_PERMISSION_FFI_PERMISSION_H_ -#define SRC_PERMISSION_FFI_PERMISSION_H_ - -#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#include "permission/permission_base.h" - -namespace node { - -namespace permission { - -class FFIPermission final : public PermissionBase { - public: - void Apply(Environment* env, - std::span allow, - PermissionScope scope) override; - void Drop(Environment* env, - PermissionScope scope, - std::string_view param) override; - bool is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const override; - - private: - bool deny_all_ = false; -}; - -} // namespace permission - -} // namespace node - -#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#endif // SRC_PERMISSION_FFI_PERMISSION_H_ diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h index e26c7a44feaa..3fdb0542f681 100644 --- a/src/permission/fs_permission.h +++ b/src/permission/fs_permission.h @@ -30,8 +30,7 @@ class FSPermission final : public PermissionBase { Node* wildcard_child = nullptr; bool is_leaf = false; - explicit Node(std::string_view pre) - : prefix(pre) {} + explicit Node(std::string_view pre) : prefix(pre) {} Node() = default; @@ -44,7 +43,10 @@ class FSPermission final : public PermissionBase { void SetChild(char label, Node* node) { for (auto& [c, n] : children) { - if (c == label) { n = node; return; } + if (c == label) { + n = node; + return; + } } children.emplace_back(label, node); } diff --git a/src/permission/inspector_permission.cc b/src/permission/inspector_permission.cc deleted file mode 100644 index d884ec4d8208..000000000000 --- a/src/permission/inspector_permission.cc +++ /dev/null @@ -1,30 +0,0 @@ -#include "inspector_permission.h" - -#include - -namespace node { - -namespace permission { - -// Currently, Inspector manage a single state -// Once denied, it's always denied -void InspectorPermission::Apply(Environment* env, - std::span allow, - PermissionScope scope) { - deny_all_ = true; -} - -void InspectorPermission::Drop(Environment* env, - PermissionScope scope, - std::string_view param) { - deny_all_ = true; -} - -bool InspectorPermission::is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const { - return deny_all_ == false; -} - -} // namespace permission -} // namespace node diff --git a/src/permission/inspector_permission.h b/src/permission/inspector_permission.h deleted file mode 100644 index 2490149caa91..000000000000 --- a/src/permission/inspector_permission.h +++ /dev/null @@ -1,34 +0,0 @@ -#ifndef SRC_PERMISSION_INSPECTOR_PERMISSION_H_ -#define SRC_PERMISSION_INSPECTOR_PERMISSION_H_ - -#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#include -#include "permission/permission_base.h" - -namespace node { - -namespace permission { - -class InspectorPermission final : public PermissionBase { - public: - void Apply(Environment* env, - std::span allow, - PermissionScope scope) override; - void Drop(Environment* env, - PermissionScope scope, - std::string_view param) override; - bool is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const override; - - private: - bool deny_all_; -}; - -} // namespace permission - -} // namespace node - -#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#endif // SRC_PERMISSION_INSPECTOR_PERMISSION_H_ diff --git a/src/permission/net_permission.cc b/src/permission/net_permission.cc deleted file mode 100644 index 10b635e82e6d..000000000000 --- a/src/permission/net_permission.cc +++ /dev/null @@ -1,28 +0,0 @@ -#include "net_permission.h" - -#include - -namespace node { - -namespace permission { - -void NetPermission::Apply(Environment* env, - std::span allow, - PermissionScope scope) { - allow_net_ = true; -} - -void NetPermission::Drop(Environment* env, - PermissionScope scope, - std::string_view param) { - allow_net_ = false; -} - -bool NetPermission::is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const { - return allow_net_; -} - -} // namespace permission -} // namespace node diff --git a/src/permission/net_permission.h b/src/permission/net_permission.h deleted file mode 100644 index 23b643a50222..000000000000 --- a/src/permission/net_permission.h +++ /dev/null @@ -1,34 +0,0 @@ -#ifndef SRC_PERMISSION_NET_PERMISSION_H_ -#define SRC_PERMISSION_NET_PERMISSION_H_ - -#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#include -#include "permission/permission_base.h" - -namespace node { - -namespace permission { - -class NetPermission final : public PermissionBase { - public: - void Apply(Environment* env, - std::span allow, - PermissionScope scope) override; - void Drop(Environment* env, - PermissionScope scope, - std::string_view param) override; - bool is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const override; - - private: - bool allow_net_ = false; -}; - -} // namespace permission - -} // namespace node - -#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#endif // SRC_PERMISSION_NET_PERMISSION_H_ diff --git a/src/permission/openssl_store_permission.cc b/src/permission/openssl_store_permission.cc deleted file mode 100644 index 11d4ff82faf8..000000000000 --- a/src/permission/openssl_store_permission.cc +++ /dev/null @@ -1,30 +0,0 @@ -#include "permission/openssl_store_permission.h" - -#include - -namespace node { - -namespace permission { - -// OpenSSLStorePermission manages a single global deny state for the use of -// OpenSSL STORE loaders. -void OpenSSLStorePermission::Apply(Environment* env, - std::span allow, - PermissionScope scope) { - deny_all_ = true; -} - -void OpenSSLStorePermission::Drop(Environment* env, - PermissionScope scope, - std::string_view param) { - deny_all_ = true; -} - -bool OpenSSLStorePermission::is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const { - return perm != PermissionScope::kOpenSSLStore || !deny_all_; -} - -} // namespace permission -} // namespace node diff --git a/src/permission/openssl_store_permission.h b/src/permission/openssl_store_permission.h deleted file mode 100644 index 418e93e0d899..000000000000 --- a/src/permission/openssl_store_permission.h +++ /dev/null @@ -1,33 +0,0 @@ -#ifndef SRC_PERMISSION_OPENSSL_STORE_PERMISSION_H_ -#define SRC_PERMISSION_OPENSSL_STORE_PERMISSION_H_ - -#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#include "permission/permission_base.h" - -namespace node { - -namespace permission { - -class OpenSSLStorePermission final : public PermissionBase { - public: - void Apply(Environment* env, - std::span allow, - PermissionScope scope) override; - void Drop(Environment* env, - PermissionScope scope, - std::string_view param) override; - bool is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const override; - - private: - bool deny_all_ = false; -}; - -} // namespace permission - -} // namespace node - -#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#endif // SRC_PERMISSION_OPENSSL_STORE_PERMISSION_H_ diff --git a/src/permission/permission.cc b/src/permission/permission.cc index e7e8ba6b2ea2..9bf1ee21e0af 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -7,15 +7,8 @@ #include "node_external_reference.h" #include "node_file.h" -#include "permission/addon_permission.h" -#include "permission/child_process_permission.h" -#include "permission/ffi_permission.h" +#include "permission/boolean_permission.h" #include "permission/fs_permission.h" -#include "permission/inspector_permission.h" -#include "permission/net_permission.h" -#include "permission/openssl_store_permission.h" -#include "permission/wasi_permission.h" -#include "permission/worker_permission.h" #include "permission/permission_base.h" #include "v8-fast-api-calls.h" #include "v8-template.h" @@ -194,49 +187,25 @@ PermissionScope Permission::StringToPermission(std::string_view perm) { Permission::Permission() : enabled_(false), warning_only_(false) { auto fs = std::make_shared(); - auto child_p = std::make_shared(); - auto worker_t = std::make_shared(); - auto inspector = std::make_shared(); - auto wasi = std::make_shared(); - auto net = std::make_shared(); - auto addon = std::make_shared(); - auto ffi = std::make_shared(); - auto openssl_store = std::make_shared(); #define V(Name, _, __, ___) \ nodes_[static_cast(PermissionScope::k##Name)] = fs; FILESYSTEM_PERMISSIONS(V) #undef V #define V(Name, _, __, ___) \ - nodes_[static_cast(PermissionScope::k##Name)] = child_p; + nodes_[static_cast(PermissionScope::k##Name)] = \ + std::make_shared(); CHILD_PROCESS_PERMISSIONS(V) -#undef V -#define V(Name, _, __, ___) \ - nodes_[static_cast(PermissionScope::k##Name)] = worker_t; WORKER_THREADS_PERMISSIONS(V) -#undef V -#define V(Name, _, __, ___) \ - nodes_[static_cast(PermissionScope::k##Name)] = inspector; INSPECTOR_PERMISSIONS(V) -#undef V -#define V(Name, _, __, ___) \ - nodes_[static_cast(PermissionScope::k##Name)] = wasi; WASI_PERMISSIONS(V) -#undef V -#define V(Name, _, __, ___) \ - nodes_[static_cast(PermissionScope::k##Name)] = net; - NET_PERMISSIONS(V) -#undef V -#define V(Name, _, __, ___) \ - nodes_[static_cast(PermissionScope::k##Name)] = addon; ADDON_PERMISSIONS(V) -#undef V -#define V(Name, _, __, ___) \ - nodes_[static_cast(PermissionScope::k##Name)] = ffi; FFI_PERMISSIONS(V) + OPENSSL_STORE_PERMISSIONS(V) #undef V #define V(Name, _, __, ___) \ - nodes_[static_cast(PermissionScope::k##Name)] = openssl_store; - OPENSSL_STORE_PERMISSIONS(V) + nodes_[static_cast(PermissionScope::k##Name)] = \ + std::make_shared(); + NET_PERMISSIONS(V) #undef V } diff --git a/src/permission/wasi_permission.cc b/src/permission/wasi_permission.cc deleted file mode 100644 index 5891edc92aa3..000000000000 --- a/src/permission/wasi_permission.cc +++ /dev/null @@ -1,30 +0,0 @@ -#include "permission/wasi_permission.h" - -#include - -namespace node { - -namespace permission { - -// Currently, WASIPermission manage a single state -// Once denied, it's always denied -void WASIPermission::Apply(Environment* env, - std::span allow, - PermissionScope scope) { - deny_all_ = true; -} - -void WASIPermission::Drop(Environment* env, - PermissionScope scope, - std::string_view param) { - deny_all_ = true; -} - -bool WASIPermission::is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const { - return deny_all_ == false; -} - -} // namespace permission -} // namespace node diff --git a/src/permission/wasi_permission.h b/src/permission/wasi_permission.h deleted file mode 100644 index fbca0e4e0879..000000000000 --- a/src/permission/wasi_permission.h +++ /dev/null @@ -1,33 +0,0 @@ -#ifndef SRC_PERMISSION_WASI_PERMISSION_H_ -#define SRC_PERMISSION_WASI_PERMISSION_H_ - -#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#include "permission/permission_base.h" - -namespace node { - -namespace permission { - -class WASIPermission final : public PermissionBase { - public: - void Apply(Environment* env, - std::span allow, - PermissionScope scope) override; - void Drop(Environment* env, - PermissionScope scope, - std::string_view param) override; - bool is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const override; - - private: - bool deny_all_; -}; - -} // namespace permission - -} // namespace node - -#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#endif // SRC_PERMISSION_WASI_PERMISSION_H_ diff --git a/src/permission/worker_permission.cc b/src/permission/worker_permission.cc deleted file mode 100644 index bdf0519d3bb0..000000000000 --- a/src/permission/worker_permission.cc +++ /dev/null @@ -1,30 +0,0 @@ -#include "permission/worker_permission.h" - -#include - -namespace node { - -namespace permission { - -// Currently, PolicyDenyWorker manage a single state -// Once denied, it's always denied -void WorkerPermission::Apply(Environment* env, - std::span allow, - PermissionScope scope) { - deny_all_ = true; -} - -void WorkerPermission::Drop(Environment* env, - PermissionScope scope, - std::string_view param) { - deny_all_ = true; -} - -bool WorkerPermission::is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const { - return deny_all_ == false; -} - -} // namespace permission -} // namespace node diff --git a/src/permission/worker_permission.h b/src/permission/worker_permission.h deleted file mode 100644 index ba4b45319ba9..000000000000 --- a/src/permission/worker_permission.h +++ /dev/null @@ -1,33 +0,0 @@ -#ifndef SRC_PERMISSION_WORKER_PERMISSION_H_ -#define SRC_PERMISSION_WORKER_PERMISSION_H_ - -#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#include "permission/permission_base.h" - -namespace node { - -namespace permission { - -class WorkerPermission final : public PermissionBase { - public: - void Apply(Environment* env, - std::span allow, - PermissionScope scope) override; - void Drop(Environment* env, - PermissionScope scope, - std::string_view param) override; - bool is_granted(Environment* env, - PermissionScope perm, - std::string_view param) const override; - - private: - bool deny_all_; -}; - -} // namespace permission - -} // namespace node - -#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#endif // SRC_PERMISSION_WORKER_PERMISSION_H_ From 7dd8d04e5a70b63d38e8acd22627fd3b87ad1448 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Aug 2026 19:25:37 -0700 Subject: [PATCH 10/11] src: apply minor namespace format tweak in permissions Signed-off-by: James M Snell --- src/permission/boolean_permission.h | 8 ++------ src/permission/fs_permission.h | 17 ++++++++--------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/permission/boolean_permission.h b/src/permission/boolean_permission.h index d1a30972dcee..f2fdc8a7f90f 100644 --- a/src/permission/boolean_permission.h +++ b/src/permission/boolean_permission.h @@ -5,9 +5,7 @@ #include "permission/permission_base.h" -namespace node { - -namespace permission { +namespace node::permission { // A simple boolean permission that either denies or allows access. // Used for permission scopes that don't need per-resource granularity. @@ -46,9 +44,7 @@ using DenyOnlyPermission = BooleanPermission; // Apply grants access, Drop revokes. using AllowRevokePermission = BooleanPermission; -} // namespace permission - -} // namespace node +} // namespace node::permission #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #endif // SRC_PERMISSION_BOOLEAN_PERMISSION_H_ diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h index 3fdb0542f681..f2e1720ed9bd 100644 --- a/src/permission/fs_permission.h +++ b/src/permission/fs_permission.h @@ -7,9 +7,7 @@ #include "permission/permission_base.h" #include "util.h" -namespace node { - -namespace permission { +namespace node::permission { class FSPermission final : public PermissionBase { public: @@ -125,9 +123,12 @@ class FSPermission final : public PermissionBase { // Handle optional trailing // path = /home/subdirectory // child = subdirectory/* - if (idx >= path.length() && - child->prefix[i] == node::kPathSeparator) { - continue; + if (idx >= path.length()) { + if (child->prefix[i] == node::kPathSeparator) { + continue; + } + // Path is exhausted but prefix expects more characters + return nullptr; } if (path[idx++] != child->prefix[i]) { @@ -185,9 +186,7 @@ class FSPermission final : public PermissionBase { bool allow_all_out_ = false; }; -} // namespace permission - -} // namespace node +} // namespace node::permission #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #endif // SRC_PERMISSION_FS_PERMISSION_H_ From d110091a7f5330c482da12f5658eecef5aa5a6c8 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 8 Aug 2026 22:19:37 -0700 Subject: [PATCH 11/11] test: add permission fast api test Signed-off-by: James M Snell --- src/permission/permission.cc | 38 ++++++++++++++++--- src/util.cc | 25 ++++++++++++ src/util.h | 6 +++ test/parallel/test-permission-has-fast-api.js | 38 +++++++++++++++++++ 4 files changed, 101 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-permission-has-fast-api.js diff --git a/src/permission/permission.cc b/src/permission/permission.cc index 9bf1ee21e0af..29449c0ec028 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -127,7 +127,6 @@ static void Has(const FunctionCallbackInfo& args) { static bool FastHas(Local receiver, Local scope_arg, - Local resource_arg, // NOLINTNEXTLINE(runtime/references) This is V8 api. FastApiCallbackOptions& options) { TRACK_V8_FAST_API_CALL("permission.has"); @@ -148,8 +147,31 @@ static bool FastHas(Local receiver, return false; } - if (resource_arg->IsUndefined()) { - return env->permission()->is_granted(env, scope); + return env->permission()->is_granted(env, scope); +} + +static bool FastHasResource( + Local receiver, + Local scope_arg, + Local resource_arg, + // NOLINTNEXTLINE(runtime/references) This is V8 api. + FastApiCallbackOptions& options) { + TRACK_V8_FAST_API_CALL("permission.has"); + auto isolate = options.isolate; + v8::HandleScope handle_scope(isolate); + auto context = isolate->GetCurrentContext(); + + Environment* env = Environment::GetCurrent(context); + + Local str; + if (!scope_arg->ToString(context).ToLocal(&str)) { + return false; + } + Utf8Value utf8_scope(isolate, str); + PermissionScope scope = + Permission::StringToPermission(utf8_scope.ToStringView()); + if (scope == PermissionScope::kPermissionsRoot) { + return false; } Local res_str; @@ -164,7 +186,8 @@ static bool FastHas(Local receiver, return env->permission()->is_granted(env, scope, utf8_res.ToStringView()); } -static CFunction fast_has_(CFunction::Make(FastHas)); +static CFunction fast_has_methods_[] = {CFunction::Make(FastHas), + CFunction::Make(FastHasResource)}; } // namespace @@ -370,7 +393,8 @@ void Initialize(Local target, Local unused, Local context, void* priv) { - SetFastMethodNoSideEffect(context, target, "has", Has, &fast_has_); + SetFastMethodNoSideEffect( + context, target, "has", Has, {fast_has_methods_, 2}); SetMethod(context, target, "drop", Drop); target->SetIntegrityLevel(context, IntegrityLevel::kFrozen).FromJust(); @@ -378,7 +402,9 @@ void Initialize(Local target, void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(Has); - registry->Register(fast_has_); + for (const CFunction& method : fast_has_methods_) { + registry->Register(method); + } registry->Register(Drop); } diff --git a/src/util.cc b/src/util.cc index 317b8db0daac..ce45c1ad4ede 100644 --- a/src/util.cc +++ b/src/util.cc @@ -468,6 +468,31 @@ void SetFastMethodNoSideEffect( that->Set(name_string, t); } +void SetFastMethodNoSideEffect( + Local context, + Local that, + const std::string_view name, + v8::FunctionCallback slow_callback, + const v8::MemorySpan& methods) { + Isolate* isolate = Isolate::GetCurrent(); + Local function = FunctionTemplate::NewWithCFunctionOverloads( + isolate, + slow_callback, + Local(), + Local(), + 0, + v8::ConstructorBehavior::kThrow, + v8::SideEffectType::kHasNoSideEffect, + methods) + ->GetFunction(context) + .ToLocalChecked(); + const v8::NewStringType type = v8::NewStringType::kInternalized; + Local name_string = + v8::String::NewFromUtf8(isolate, name.data(), type, name.size()) + .ToLocalChecked(); + that->Set(context, name_string, function).Check(); +} + void SetMethodNoSideEffect(Local context, Local that, const std::string_view name, diff --git a/src/util.h b/src/util.h index 48305bfdc131..d025a6c755ef 100644 --- a/src/util.h +++ b/src/util.h @@ -921,6 +921,12 @@ void SetFastMethodNoSideEffect( const std::string_view name, v8::FunctionCallback slow_callback, const v8::MemorySpan& methods); +void SetFastMethodNoSideEffect( + v8::Local context, + v8::Local that, + const std::string_view name, + v8::FunctionCallback slow_callback, + const v8::MemorySpan& methods); void SetProtoMethod(v8::Isolate* isolate, v8::Local that, const std::string_view name, diff --git a/test/parallel/test-permission-has-fast-api.js b/test/parallel/test-permission-has-fast-api.js new file mode 100644 index 000000000000..526ee4f20944 --- /dev/null +++ b/test/parallel/test-permission-has-fast-api.js @@ -0,0 +1,38 @@ +// Flags: --permission --allow-fs-read=* --allow-fs-write=* --allow-natives-syntax --expose-internals --no-warnings +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { internalBinding } = require('internal/test/binding'); +const path = require('path'); + +// Test that process.permission.has() uses the V8 fast API path. + +// Test with scope only (no resource argument). +function testHasScope() { + assert.strictEqual(process.permission.has('fs.read', __filename), true); + assert.strictEqual(process.permission.has('fs.write', __dirname), true); + assert.strictEqual( + process.permission.has('fs.read', path.resolve('/nonexistent')), + true + ); + assert.strictEqual(process.permission.has('fs.read'), true); + assert.strictEqual(process.permission.has('fs.write'), true); + assert.strictEqual(process.permission.has('child'), false); + assert.strictEqual(process.permission.has('worker'), false); + assert.strictEqual(process.permission.has('invalid-key'), false); +} + +// Warm up and optimize for the fast API path. +eval('%PrepareFunctionForOptimization(testHasScope)'); +testHasScope(); +testHasScope(); + +eval('%OptimizeFunctionOnNextCall(testHasScope)'); +testHasScope(); + +if (common.isDebug) { + const { getV8FastApiCallCount } = internalBinding('debug'); + // After optimization: testHasScope = 4, testHasResource = 3, testHasInvalid = 1 + assert.strictEqual(getV8FastApiCallCount('permission.has'), 8); +}