From efce03fbfbd8057aab2ab35bc6db9b03bae20c9d Mon Sep 17 00:00:00 2001 From: nabeel378 Date: Sun, 19 Apr 2026 15:04:25 +0500 Subject: [PATCH 1/5] permission: add --allow-env option for environment variable permissions Signed-off-by: nabeel378 --- lib/internal/process/permission.js | 1 + node.gyp | 2 + src/env.cc | 7 ++ src/node_env_var.cc | 52 +++++++++++++- src/node_options.cc | 6 ++ src/node_options.h | 1 + src/permission/env_var_permission.cc | 42 ++++++++++++ src/permission/env_var_permission.h | 35 ++++++++++ src/permission/permission.cc | 6 ++ src/permission/permission.h | 1 + src/permission/permission_base.h | 4 +- .../parallel/test-permission-env-allow-all.js | 23 +++++++ test/parallel/test-permission-env-cli.js | 68 +++++++++++++++++++ test/parallel/test-permission-env-deny-all.js | 38 +++++++++++ 14 files changed, 283 insertions(+), 3 deletions(-) create mode 100644 src/permission/env_var_permission.cc create mode 100644 src/permission/env_var_permission.h create mode 100644 test/parallel/test-permission-env-allow-all.js create mode 100644 test/parallel/test-permission-env-cli.js create mode 100644 test/parallel/test-permission-env-deny-all.js diff --git a/lib/internal/process/permission.js b/lib/internal/process/permission.js index 97ea0265fa15d9..494b41bcd770b7 100644 --- a/lib/internal/process/permission.js +++ b/lib/internal/process/permission.js @@ -46,6 +46,7 @@ module.exports = ObjectFreeze({ '--allow-fs-write', '--allow-addons', '--allow-child-process', + '--allow-env', '--allow-net', '--allow-inspector', '--allow-wasi', diff --git a/node.gyp b/node.gyp index 6a21e715c89d22..5d75d715b2b2ce 100644 --- a/node.gyp +++ b/node.gyp @@ -173,6 +173,7 @@ 'src/path.cc', 'src/permission/child_process_permission.cc', 'src/permission/ffi_permission.cc', + 'src/permission/env_var_permission.cc', 'src/permission/fs_permission.cc', 'src/permission/inspector_permission.cc', 'src/permission/permission.cc', @@ -309,6 +310,7 @@ 'src/path.h', 'src/permission/child_process_permission.h', 'src/permission/ffi_permission.h', + 'src/permission/env_var_permission.h', 'src/permission/fs_permission.h', 'src/permission/inspector_permission.h', 'src/permission/permission.h', diff --git a/src/env.cc b/src/env.cc index f618fca2e2d157..263a247f0c93e6 100644 --- a/src/env.cc +++ b/src/env.cc @@ -949,6 +949,13 @@ Environment::Environment(IsolateData* isolate_data, permission()->Apply(this, {"*"}, permission::PermissionScope::kWASI); } + if (!options_->allow_env.empty()) { + permission()->Apply( + this, options_->allow_env, permission::PermissionScope::kEnvVar); + } else { + permission()->Apply(this, {}, permission::PermissionScope::kEnvVar); + } + // Implicit allow entrypoint to kFileSystemRead if (!options_->has_eval_string && !options_->force_repl) { std::string first_argv; diff --git a/src/node_env_var.cc b/src/node_env_var.cc index e94180cd659d35..d8f1de4cc5ff2e 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -4,6 +4,7 @@ #include "node_external_reference.h" #include "node_i18n.h" #include "node_process-inl.h" +#include "permission/permission.h" #include "util.h" #include // tzset(), _tzset() @@ -435,6 +436,14 @@ static Intercepted EnvGetter(Local property, return Intercepted::kYes; } CHECK(property->IsString()); + + Utf8Value key(env->isolate(), property); + if (env->permission()->enabled() && + !env->permission()->is_granted( + env, permission::PermissionScope::kEnvVar, key.ToStringView())) { + return Intercepted::kNo; + } + MaybeLocal value_string = env->env_vars()->Get(env->isolate(), property.As()); @@ -453,6 +462,16 @@ static Intercepted EnvSetter(Local property, const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); CHECK(env->has_run_bootstrapping_code()); + + if (property->IsString()) { + Utf8Value key(env->isolate(), property); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + permission::PermissionScope::kEnvVar, + key.ToStringView(), + Intercepted::kYes); + } + // calling env->EmitProcessEnvWarning() sets a variable indicating that // warnings have been emitted. It should be called last after other // conditions leading to a warning have been met. @@ -489,6 +508,13 @@ static Intercepted EnvQuery(Local property, Environment* env = Environment::GetCurrent(info); CHECK(env->has_run_bootstrapping_code()); if (property->IsString()) { + Utf8Value key(env->isolate(), property); + if (env->permission()->enabled() && + !env->permission()->is_granted( + env, permission::PermissionScope::kEnvVar, key.ToStringView())) { + return Intercepted::kNo; + } + int32_t rc = env->env_vars()->Query(env->isolate(), property.As()); bool has_env = (rc != -1); TraceEnvVar(env, "query", property.As()); @@ -506,6 +532,13 @@ static Intercepted EnvDeleter(Local property, Environment* env = Environment::GetCurrent(info); CHECK(env->has_run_bootstrapping_code()); if (property->IsString()) { + Utf8Value key(env->isolate(), property); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + permission::PermissionScope::kEnvVar, + key.ToStringView(), + Intercepted::kYes); + env->env_vars()->Delete(env->isolate(), property.As()); TraceEnvVar(env, "delete", property.As()); @@ -525,7 +558,24 @@ static void EnvEnumerator(const PropertyCallbackInfo& info) { Local ret; if (env->env_vars()->Enumerate(env->isolate()).ToLocal(&ret)) { - info.GetReturnValue().Set(ret); + if (env->permission()->enabled()) { + LocalVector filtered(env->isolate()); + for (uint32_t i = 0; i < ret->Length(); i++) { + Local elem; + if (!ret->Get(env->context(), i).ToLocal(&elem)) continue; + Utf8Value key(env->isolate(), elem); + if (env->permission()->is_granted( + env, + permission::PermissionScope::kEnvVar, + key.ToStringView())) { + filtered.push_back(elem); + } + } + info.GetReturnValue().Set( + Array::New(env->isolate(), filtered.data(), filtered.size())); + } else { + info.GetReturnValue().Set(ret); + } } } diff --git a/src/node_options.cc b/src/node_options.cc index bbb72d2ba1bcf4..bdde46b1f73385 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -712,6 +712,12 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { kAllowedInEnvvar, false, OptionNamespaces::kPermissionNamespace); + AddOption("--allow-env", + "allow access to environment variables when any permissions are " + "set", + &EnvironmentOptions::allow_env, + kAllowedInEnvvar, + OptionNamespaces::kPermissionNamespace); AddOption("--experimental-repl-await", "experimental await keyword support in REPL", &EnvironmentOptions::experimental_repl_await, diff --git a/src/node_options.h b/src/node_options.h index 8ee0f55e65e8a6..6bd07ac80e6a06 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -147,6 +147,7 @@ class EnvironmentOptions : public Options { bool allow_wasi = false; bool allow_ffi = false; bool allow_worker_threads = false; + std::vector allow_env; bool experimental_repl_await = true; bool experimental_vm_modules = false; bool async_context_frame = true; diff --git a/src/permission/env_var_permission.cc b/src/permission/env_var_permission.cc new file mode 100644 index 00000000000000..a5698032610dc4 --- /dev/null +++ b/src/permission/env_var_permission.cc @@ -0,0 +1,42 @@ +#include "env_var_permission.h" + +#include +#include + +namespace node { + +namespace permission { + +void EnvVarPermission::Apply(Environment* env, + const std::vector& allow, + PermissionScope scope) { + deny_all_ = true; + if (!allow.empty()) { + if (allow.size() == 1 && allow[0] == "*") { + allow_all_ = true; + return; + } + for (const std::string& arg : allow) { + allowed_env_vars_.insert(arg); + } + } +} + +bool EnvVarPermission::is_granted(Environment* env, + PermissionScope perm, + const std::string_view& param) const { + if (deny_all_) { + if (allow_all_) { + return true; + } + if (param.empty()) { + return false; + } + return allowed_env_vars_.find(std::string(param)) != + allowed_env_vars_.end(); + } + return true; +} + +} // namespace permission +} // namespace node diff --git a/src/permission/env_var_permission.h b/src/permission/env_var_permission.h new file mode 100644 index 00000000000000..71d64ba087e5b6 --- /dev/null +++ b/src/permission/env_var_permission.h @@ -0,0 +1,35 @@ +#ifndef SRC_PERMISSION_ENV_VAR_PERMISSION_H_ +#define SRC_PERMISSION_ENV_VAR_PERMISSION_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include +#include +#include +#include "permission/permission_base.h" + +namespace node { + +namespace permission { + +class EnvVarPermission final : public PermissionBase { + public: + void Apply(Environment* env, + const std::vector& allow, + PermissionScope scope) override; + bool is_granted(Environment* env, + PermissionScope perm, + const std::string_view& param = "") const override; + + private: + bool deny_all_ = false; + bool allow_all_ = false; + std::set allowed_env_vars_; +}; + +} // namespace permission + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#endif // SRC_PERMISSION_ENV_VAR_PERMISSION_H_ diff --git a/src/permission/permission.cc b/src/permission/permission.cc index 0da8fdcd0b9c1e..a8b51e37220a71 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -48,6 +48,8 @@ constexpr std::string_view GetDiagnosticsChannelName(PermissionScope scope) { return "node:permission-model:addon"; case PermissionScope::kFFI: return "node:permission-model:ffi"; + case PermissionScope::kEnvVar: + return "node:permission-model:env"; default: return {}; } @@ -108,6 +110,8 @@ Permission::Permission() : enabled_(false), warning_only_(false) { std::shared_ptr net = std::make_shared(); std::shared_ptr addon = std::make_shared(); std::shared_ptr ffi = std::make_shared(); + std::shared_ptr env_var = + std::make_shared(); #define V(Name, _, __, ___) \ nodes_.insert(std::make_pair(PermissionScope::k##Name, fs)); FILESYSTEM_PERMISSIONS(V) @@ -139,6 +143,8 @@ Permission::Permission() : enabled_(false), warning_only_(false) { #define V(Name, _, __, ___) \ nodes_.insert(std::make_pair(PermissionScope::k##Name, ffi)); FFI_PERMISSIONS(V) + nodes_.insert(std::make_pair(PermissionScope::k##Name, env_var)); + ENV_VAR_PERMISSIONS(V) #undef V } diff --git a/src/permission/permission.h b/src/permission/permission.h index b43c9648093b08..873e46d89d48e4 100644 --- a/src/permission/permission.h +++ b/src/permission/permission.h @@ -9,6 +9,7 @@ #include "permission/addon_permission.h" #include "permission/child_process_permission.h" #include "permission/ffi_permission.h" +#include "permission/env_var_permission.h" #include "permission/fs_permission.h" #include "permission/inspector_permission.h" #include "permission/net_permission.h" diff --git a/src/permission/permission_base.h b/src/permission/permission_base.h index 4c796c770dccf8..93f65192c7aa25 100644 --- a/src/permission/permission_base.h +++ b/src/permission/permission_base.h @@ -36,6 +36,8 @@ namespace permission { V(Addon, "addon", PermissionsRoot, "--allow-addons") #define FFI_PERMISSIONS(V) V(FFI, "ffi", PermissionsRoot, "--allow-ffi") +#define ENV_VAR_PERMISSIONS(V) \ + V(EnvVar, "env", PermissionsRoot, "--allow-env") #define PERMISSIONS(V) \ FILESYSTEM_PERMISSIONS(V) \ @@ -46,9 +48,7 @@ namespace permission { NET_PERMISSIONS(V) \ ADDON_PERMISSIONS(V) \ FFI_PERMISSIONS(V) - #define V(name, _, __, ___) k##name, -enum class PermissionScope { kPermissionsRoot = -1, PERMISSIONS(V) kPermissionsCount }; diff --git a/test/parallel/test-permission-env-allow-all.js b/test/parallel/test-permission-env-allow-all.js new file mode 100644 index 00000000000000..7e4e10538d575b --- /dev/null +++ b/test/parallel/test-permission-env-allow-all.js @@ -0,0 +1,23 @@ +// Flags: --permission --allow-fs-read=* --allow-env=* +'use strict'; + +const common = require('../common'); +const { isMainThread } = require('worker_threads'); + +if (!isMainThread) { + common.skip('This test only works on a main thread'); +} + +const assert = require('assert'); + +// With --allow-env=* all env vars should be accessible +{ + assert.ok(process.permission.has('env')); + // Should not throw for any env var + const home = process.env.HOME; + const path = process.env.PATH; + process.env.TEST_PERMISSION_VAR = 'test'; + assert.strictEqual(process.env.TEST_PERMISSION_VAR, 'test'); + delete process.env.TEST_PERMISSION_VAR; + assert.strictEqual(process.env.TEST_PERMISSION_VAR, undefined); +} diff --git a/test/parallel/test-permission-env-cli.js b/test/parallel/test-permission-env-cli.js new file mode 100644 index 00000000000000..8b7a276ef2fb2d --- /dev/null +++ b/test/parallel/test-permission-env-cli.js @@ -0,0 +1,68 @@ +// Flags: --permission --allow-fs-read=* --allow-env=HOME --allow-env=PATH +'use strict'; + +const common = require('../common'); +const { isMainThread } = require('worker_threads'); + +if (!isMainThread) { + common.skip('This test only works on a main thread'); +} + +const assert = require('assert'); + +// Guarantee the initial state +{ + assert.ok(!process.permission.has('env')); +} + +// Allowed env vars should be accessible +{ + // Reading allowed env vars should not throw + const home = process.env.HOME; + const path = process.env.PATH; + assert.ok(process.permission.has('env', 'HOME')); + assert.ok(process.permission.has('env', 'PATH')); +} + +// Disallowed env vars should return undefined (silently denied) +{ + assert.strictEqual(process.env.SECRET_KEY, undefined); +} + +// Setting a disallowed env var should throw +{ + assert.throws(() => { + process.env.NEW_VAR = 'value'; + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'EnvVar', + resource: 'NEW_VAR', + })); +} + +// Deleting a disallowed env var should throw +{ + assert.throws(() => { + delete process.env.SECRET_KEY; + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'EnvVar', + resource: 'SECRET_KEY', + })); +} + +// Querying a disallowed env var should return false (not found) +{ + assert.strictEqual('SECRET_KEY' in process.env, false); +} + +// Enumerating should only return allowed env vars +{ + const keys = Object.keys(process.env); + for (const key of keys) { + assert.ok( + key === 'HOME' || key === 'PATH', + `Unexpected env var in enumeration: ${key}` + ); + } +} diff --git a/test/parallel/test-permission-env-deny-all.js b/test/parallel/test-permission-env-deny-all.js new file mode 100644 index 00000000000000..e3be9387395c1d --- /dev/null +++ b/test/parallel/test-permission-env-deny-all.js @@ -0,0 +1,38 @@ +// Flags: --permission --allow-fs-read=* +'use strict'; + +const common = require('../common'); +const { isMainThread } = require('worker_threads'); + +if (!isMainThread) { + common.skip('This test only works on a main thread'); +} + +const assert = require('assert'); + +// When --permission is set without --allow-env, all env access is denied +{ + assert.ok(!process.permission.has('env')); +} + +// Reading any env var should return undefined (silently denied) +{ + assert.strictEqual(process.env.HOME, undefined); + assert.strictEqual(process.env.PATH, undefined); +} + +// Setting any env var should throw +{ + assert.throws(() => { + process.env.TEST_VAR = 'value'; + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'EnvVar', + })); +} + +// Enumerating should return empty +{ + const keys = Object.keys(process.env); + assert.strictEqual(keys.length, 0); +} From 31e4b5fc1c83c1f6f26bbaa6927e417c4a6004ca Mon Sep 17 00:00:00 2001 From: nabeel378 Date: Sun, 19 Apr 2026 15:52:10 +0500 Subject: [PATCH 2/5] doc: add documentation for --allow-env flag in CLI and permissions Signed-off-by: nabeel378 --- doc/api/cli.md | 42 ++++++++++++++++++++++++++++++++++++++++++ doc/api/permissions.md | 4 +++- doc/api/process.md | 7 +++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index e979ec95c4259d..24eb539603461a 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -191,6 +191,45 @@ This behavior also applies to `child_process.spawn()`, but in that case, the flags are propagated via the `NODE_OPTIONS` environment variable rather than directly through the process arguments. +### `--allow-env` + +> Stability: 1.1 - Active development + +When using the [Permission Model][], access to environment variables is +restricted by default unless the user explicitly passes the `--allow-env` +flag when starting Node.js. + +* Reading a restricted variable (`process.env.HOME`) silently returns + `undefined`. +* Writing (`process.env.FOO = 'bar'`) or deleting (`delete process.env.FOO`) + a restricted variable throws `ERR_ACCESS_DENIED`. + +The valid arguments for the `--allow-env` flag are: + +* `*` - To allow access to all environment variables. +* Specific environment variable names can be allowed using a comma-separated + list. Example: `--allow-env=HOME,PATH,NODE_ENV` + +Example: + +```js +console.log(process.env.HOME); // undefined (silently denied) +process.env.FOO = 'bar'; // ERR_ACCESS_DENIED (throws) +``` + +```console +$ node --permission --allow-fs-read=* index.js +node:internal/process/per_thread:12 + throw new ERR_ACCESS_DENIED('EnvVar', name); + ^ + +Error: Access to this API has been restricted + at node:internal/main/run_main_module:17:47 { + code: 'ERR_ACCESS_DENIED', + permission: 'EnvVar' +} +``` + ### `--allow-ffi` @@ -4596,6 +4602,7 @@ cases: [`'message'`]: child_process.md#event-message [`'uncaughtException'`]: #event-uncaughtexception [`--no-deprecation`]: cli.md#--no-deprecation +[`--allow-env`]: cli.md#--allow-env [`--permission`]: cli.md#--permission [`--unhandled-rejections`]: cli.md#--unhandled-rejectionsmode [`Buffer`]: buffer.md From f3544f8d8d661b329951d9d39053e7bd79ef7866 Mon Sep 17 00:00:00 2001 From: nabeel378 Date: Sun, 19 Apr 2026 15:59:49 +0500 Subject: [PATCH 3/5] permission: add --allow-env option for environment variable permissions --- doc/api/process.md | 2 +- src/permission/permission.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/process.md b/doc/api/process.md index 6b333ac4e4f658..8c9d484d0d1400 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -4601,8 +4601,8 @@ cases: [`'exit'`]: #event-exit [`'message'`]: child_process.md#event-message [`'uncaughtException'`]: #event-uncaughtexception -[`--no-deprecation`]: cli.md#--no-deprecation [`--allow-env`]: cli.md#--allow-env +[`--no-deprecation`]: cli.md#--no-deprecation [`--permission`]: cli.md#--permission [`--unhandled-rejections`]: cli.md#--unhandled-rejectionsmode [`Buffer`]: buffer.md diff --git a/src/permission/permission.h b/src/permission/permission.h index 873e46d89d48e4..b0d461d3ed95f7 100644 --- a/src/permission/permission.h +++ b/src/permission/permission.h @@ -6,14 +6,14 @@ #include "debug_utils.h" #include "node_diagnostics_channel.h" #include "node_options.h" +#include "permission/permission_base.h" #include "permission/addon_permission.h" #include "permission/child_process_permission.h" -#include "permission/ffi_permission.h" #include "permission/env_var_permission.h" +#include "permission/ffi_permission.h" #include "permission/fs_permission.h" #include "permission/inspector_permission.h" #include "permission/net_permission.h" -#include "permission/permission_base.h" #include "permission/wasi_permission.h" #include "permission/worker_permission.h" #include "v8.h" From c54086a44cd904e62b77439409e263375dea5f6e Mon Sep 17 00:00:00 2001 From: nabeel378 Date: Sun, 19 Apr 2026 17:06:20 +0500 Subject: [PATCH 4/5] permission,doc: fix --allow-env docs and expand test coverage Signed-off-by: nabeel378 --- doc/api/cli.md | 2 +- src/permission/permission.cc | 2 + test/parallel/test-permission-env-cli.js | 43 +++++++++++++++---- test/parallel/test-permission-env-deny-all.js | 4 -- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 24eb539603461a..a44d4989ed37e8 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -218,7 +218,7 @@ process.env.FOO = 'bar'; // ERR_ACCESS_DENIED (throws) ``` ```console -$ node --permission --allow-fs-read=* index.js +$ node --permission index.js node:internal/process/per_thread:12 throw new ERR_ACCESS_DENIED('EnvVar', name); ^ diff --git a/src/permission/permission.cc b/src/permission/permission.cc index a8b51e37220a71..af03d3584c1356 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -143,6 +143,8 @@ Permission::Permission() : enabled_(false), warning_only_(false) { #define V(Name, _, __, ___) \ nodes_.insert(std::make_pair(PermissionScope::k##Name, ffi)); FFI_PERMISSIONS(V) +#undef V +#define V(Name, _, __, ___) \ nodes_.insert(std::make_pair(PermissionScope::k##Name, env_var)); ENV_VAR_PERMISSIONS(V) #undef V diff --git a/test/parallel/test-permission-env-cli.js b/test/parallel/test-permission-env-cli.js index 8b7a276ef2fb2d..62d48c7c7bda7c 100644 --- a/test/parallel/test-permission-env-cli.js +++ b/test/parallel/test-permission-env-cli.js @@ -10,26 +10,21 @@ if (!isMainThread) { const assert = require('assert'); -// Guarantee the initial state { assert.ok(!process.permission.has('env')); } -// Allowed env vars should be accessible { - // Reading allowed env vars should not throw const home = process.env.HOME; const path = process.env.PATH; assert.ok(process.permission.has('env', 'HOME')); assert.ok(process.permission.has('env', 'PATH')); } -// Disallowed env vars should return undefined (silently denied) { assert.strictEqual(process.env.SECRET_KEY, undefined); } -// Setting a disallowed env var should throw { assert.throws(() => { process.env.NEW_VAR = 'value'; @@ -40,7 +35,6 @@ const assert = require('assert'); })); } -// Deleting a disallowed env var should throw { assert.throws(() => { delete process.env.SECRET_KEY; @@ -51,12 +45,10 @@ const assert = require('assert'); })); } -// Querying a disallowed env var should return false (not found) { assert.strictEqual('SECRET_KEY' in process.env, false); } -// Enumerating should only return allowed env vars { const keys = Object.keys(process.env); for (const key of keys) { @@ -66,3 +58,38 @@ const assert = require('assert'); ); } } + +{ + const keys = []; + for (const key in process.env) { + keys.push(key); + } + for (const key of keys) { + assert.ok( + key === 'HOME' || key === 'PATH', + `Unexpected env var in for...in: ${key}` + ); + } +} + +{ + const copy = Object.assign({}, process.env); + const keys = Object.keys(copy); + for (const key of keys) { + assert.ok( + key === 'HOME' || key === 'PATH', + `Unexpected env var in Object.assign: ${key}` + ); + } +} + +{ + const parsed = JSON.parse(JSON.stringify(process.env)); + const keys = Object.keys(parsed); + for (const key of keys) { + assert.ok( + key === 'HOME' || key === 'PATH', + `Unexpected env var in JSON.stringify: ${key}` + ); + } +} diff --git a/test/parallel/test-permission-env-deny-all.js b/test/parallel/test-permission-env-deny-all.js index e3be9387395c1d..3c93118f261990 100644 --- a/test/parallel/test-permission-env-deny-all.js +++ b/test/parallel/test-permission-env-deny-all.js @@ -10,18 +10,15 @@ if (!isMainThread) { const assert = require('assert'); -// When --permission is set without --allow-env, all env access is denied { assert.ok(!process.permission.has('env')); } -// Reading any env var should return undefined (silently denied) { assert.strictEqual(process.env.HOME, undefined); assert.strictEqual(process.env.PATH, undefined); } -// Setting any env var should throw { assert.throws(() => { process.env.TEST_VAR = 'value'; @@ -31,7 +28,6 @@ const assert = require('assert'); })); } -// Enumerating should return empty { const keys = Object.keys(process.env); assert.strictEqual(keys.length, 0); From b6a89cf40de932077d75568e4ad798eee2afbbc3 Mon Sep 17 00:00:00 2001 From: nabeel378 Date: Sun, 19 Apr 2026 22:18:00 +0500 Subject: [PATCH 5/5] permission: refactor permission checks and improve test assertions for environment variables Signed-off-by: nabeel378 --- src/node_env_var.cc | 25 ++++++++----------- src/permission/permission.h | 2 +- src/permission/permission_base.h | 8 +++--- .../parallel/test-permission-env-allow-all.js | 4 +-- test/parallel/test-permission-env-cli.js | 4 +-- 5 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/node_env_var.cc b/src/node_env_var.cc index d8f1de4cc5ff2e..e4a542abcdb542 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -465,11 +465,10 @@ static Intercepted EnvSetter(Local property, if (property->IsString()) { Utf8Value key(env->isolate(), property); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, - permission::PermissionScope::kEnvVar, - key.ToStringView(), - Intercepted::kYes); + THROW_IF_INSUFFICIENT_PERMISSIONS(env, + permission::PermissionScope::kEnvVar, + key.ToStringView(), + Intercepted::kYes); } // calling env->EmitProcessEnvWarning() sets a variable indicating that @@ -533,11 +532,10 @@ static Intercepted EnvDeleter(Local property, CHECK(env->has_run_bootstrapping_code()); if (property->IsString()) { Utf8Value key(env->isolate(), property); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, - permission::PermissionScope::kEnvVar, - key.ToStringView(), - Intercepted::kYes); + THROW_IF_INSUFFICIENT_PERMISSIONS(env, + permission::PermissionScope::kEnvVar, + key.ToStringView(), + Intercepted::kYes); env->env_vars()->Delete(env->isolate(), property.As()); @@ -564,10 +562,9 @@ static void EnvEnumerator(const PropertyCallbackInfo& info) { Local elem; if (!ret->Get(env->context(), i).ToLocal(&elem)) continue; Utf8Value key(env->isolate(), elem); - if (env->permission()->is_granted( - env, - permission::PermissionScope::kEnvVar, - key.ToStringView())) { + if (env->permission()->is_granted(env, + permission::PermissionScope::kEnvVar, + key.ToStringView())) { filtered.push_back(elem); } } diff --git a/src/permission/permission.h b/src/permission/permission.h index b0d461d3ed95f7..ed63d547421c4a 100644 --- a/src/permission/permission.h +++ b/src/permission/permission.h @@ -6,7 +6,6 @@ #include "debug_utils.h" #include "node_diagnostics_channel.h" #include "node_options.h" -#include "permission/permission_base.h" #include "permission/addon_permission.h" #include "permission/child_process_permission.h" #include "permission/env_var_permission.h" @@ -14,6 +13,7 @@ #include "permission/fs_permission.h" #include "permission/inspector_permission.h" #include "permission/net_permission.h" +#include "permission/permission_base.h" #include "permission/wasi_permission.h" #include "permission/worker_permission.h" #include "v8.h" diff --git a/src/permission/permission_base.h b/src/permission/permission_base.h index 93f65192c7aa25..6ce18edd3ea078 100644 --- a/src/permission/permission_base.h +++ b/src/permission/permission_base.h @@ -36,8 +36,7 @@ namespace permission { V(Addon, "addon", PermissionsRoot, "--allow-addons") #define FFI_PERMISSIONS(V) V(FFI, "ffi", PermissionsRoot, "--allow-ffi") -#define ENV_VAR_PERMISSIONS(V) \ - V(EnvVar, "env", PermissionsRoot, "--allow-env") +#define ENV_VAR_PERMISSIONS(V) V(EnvVar, "env", PermissionsRoot, "--allow-env") #define PERMISSIONS(V) \ FILESYSTEM_PERMISSIONS(V) \ @@ -47,7 +46,10 @@ namespace permission { INSPECTOR_PERMISSIONS(V) \ NET_PERMISSIONS(V) \ ADDON_PERMISSIONS(V) \ - FFI_PERMISSIONS(V) + FFI_PERMISSIONS(V) \ + ENV_VAR_PERMISSIONS(V) + +enum class PermissionScope { #define V(name, _, __, ___) k##name, kPermissionsRoot = -1, PERMISSIONS(V) kPermissionsCount diff --git a/test/parallel/test-permission-env-allow-all.js b/test/parallel/test-permission-env-allow-all.js index 7e4e10538d575b..5ca5d06a421f80 100644 --- a/test/parallel/test-permission-env-allow-all.js +++ b/test/parallel/test-permission-env-allow-all.js @@ -14,8 +14,8 @@ const assert = require('assert'); { assert.ok(process.permission.has('env')); // Should not throw for any env var - const home = process.env.HOME; - const path = process.env.PATH; + assert.ok(process.env.HOME !== undefined); + assert.ok(process.env.PATH !== undefined); process.env.TEST_PERMISSION_VAR = 'test'; assert.strictEqual(process.env.TEST_PERMISSION_VAR, 'test'); delete process.env.TEST_PERMISSION_VAR; diff --git a/test/parallel/test-permission-env-cli.js b/test/parallel/test-permission-env-cli.js index 62d48c7c7bda7c..889a9370b87b58 100644 --- a/test/parallel/test-permission-env-cli.js +++ b/test/parallel/test-permission-env-cli.js @@ -15,8 +15,8 @@ const assert = require('assert'); } { - const home = process.env.HOME; - const path = process.env.PATH; + assert.ok(process.env.HOME !== undefined); + assert.ok(process.env.PATH !== undefined); assert.ok(process.permission.has('env', 'HOME')); assert.ok(process.permission.has('env', 'PATH')); }