feat: add --allow-env option for environment variable permissions

nabeel378 · nabeel378 · commit 6960cf5c2994 · 2026-04-19T15:21:20.000+05:00
Signed-off-by: nabeel378 &lt;mohammadnabeeljameel@gmail.com&gt;
diff --git 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
@@ -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
@@ -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
@@ -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 <time.h>  // tzset(), _tzset()
@@ -435,6 +436,14 @@ static Intercepted EnvGetter(Local<Name> 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<String> value_string =
       env->env_vars()->Get(env->isolate(), property.As<String>());
 
@@ -453,6 +462,16 @@ static Intercepted EnvSetter(Local<Name> property,
                              const PropertyCallbackInfo<void>& 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<Name> 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<String>());
     bool has_env = (rc != -1);
     TraceEnvVar(env, "query", property.As<String>());
@@ -506,6 +532,13 @@ static Intercepted EnvDeleter(Local<Name> 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<String>());
 
     TraceEnvVar(env, "delete", property.As<String>());
@@ -525,7 +558,24 @@ static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
 
   Local<Array> ret;
   if (env->env_vars()->Enumerate(env->isolate()).ToLocal(&ret)) {
-    info.GetReturnValue().Set(ret);
+    if (env->permission()->enabled()) {
+      LocalVector<Value> filtered(env->isolate());
+      for (uint32_t i = 0; i < ret->Length(); i++) {
+        Local<Value> 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
@@ -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
@@ -147,6 +147,7 @@ class EnvironmentOptions : public Options {
   bool allow_wasi = false;
   bool allow_ffi = false;
   bool allow_worker_threads = false;
+  std::vector<std::string> 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
@@ -0,0 +1,42 @@
+#include "env_var_permission.h"
+
+#include <string>
+#include <vector>
+
+namespace node {
+
+namespace permission {
+
+void EnvVarPermission::Apply(Environment* env,
+                             const std::vector<std::string>& 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
@@ -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 <set>
+#include <string>
+#include <vector>
+#include "permission/permission_base.h"
+
+namespace node {
+
+namespace permission {
+
+class EnvVarPermission final : public PermissionBase {
+ public:
+  void Apply(Environment* env,
+             const std::vector<std::string>& 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<std::string> 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
@@ -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<PermissionBase> net = std::make_shared<NetPermission>();
   std::shared_ptr<PermissionBase> addon = std::make_shared<AddonPermission>();
   std::shared_ptr<FFIPermission> ffi = std::make_shared<FFIPermission>();
+  std::shared_ptr<PermissionBase> env_var =
+      std::make_shared<EnvVarPermission>();
 #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
@@ -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
@@ -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
@@ -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
@@ -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
