permission: add --allow-env flag for environment variable access control

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 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`
 
 <!-- YAML
@@ -2224,6 +2263,7 @@ following permissions are restricted:
 * Worker Threads - manageable through [`--allow-worker`][] flag
 * WASI - manageable through [`--allow-wasi`][] flag
 * Addons - manageable through [`--allow-addons`][] flag
+* Environment Variables - manageable through [`--allow-env`][] flag
 * FFI - manageable through [`--allow-ffi`](#--allow-ffi) flag
 
 ### `--permission-audit`
@@ -3671,6 +3711,7 @@ one is included in the list below.
 
 * `--allow-addons`
 * `--allow-child-process`
+* `--allow-env`
 * `--allow-ffi`
 * `--allow-fs-read`
 * `--allow-fs-write`
@@ -4309,6 +4350,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
 [`"type"`]: packages.md#type
 [`--allow-addons`]: #--allow-addons
 [`--allow-child-process`]: #--allow-child-process
+[`--allow-env`]: #--allow-env
 [`--allow-fs-read`]: #--allow-fs-read
 [`--allow-fs-write`]: #--allow-fs-write
 [`--allow-net`]: #--allow-net

doc/api/permissions.md

@@ -71,7 +71,8 @@ using the [`--allow-child-process`][] and [`--allow-worker`][] respectively.
 To allow network access, use [`--allow-net`][] and for allowing native addons
 when using permission model, use the [`--allow-addons`][]
 flag. For WASI, use the [`--allow-wasi`][] flag. For FFI, use the
-[`--allow-ffi`][] flag. The [`node:ffi`](ffi.md) module also requires the
+[`--allow-ffi`][] flag. For environment variables, use the
+[`--allow-env`][] flag. The [`node:ffi`](ffi.md) module also requires the
 `--experimental-ffi` flag and is only available in builds with FFI support.
 
 #### Runtime API
@@ -283,6 +284,7 @@ Developers relying on --permission to sandbox untrusted code should be aware tha
 [Security Policy]: https://github.com/nodejs/node/blob/main/SECURITY.md
 [`--allow-addons`]: cli.md#--allow-addons
 [`--allow-child-process`]: cli.md#--allow-child-process
+[`--allow-env`]: cli.md#--allow-env
 [`--allow-ffi`]: cli.md#--allow-ffi
 [`--allow-fs-read`]: cli.md#--allow-fs-read
 [`--allow-fs-write`]: cli.md#--allow-fs-write

doc/api/process.md

@@ -1559,6 +1559,12 @@ changes:
 The `process.env` property returns an object containing the user environment.
 See environ(7).
 
+When the [Permission Model][] is enabled, access to environment variables is
+restricted. Reading a restricted variable silently returns `undefined`, while
+writing or deleting throws `ERR_ACCESS_DENIED`. Use `--allow-env` to grant
+access to all variables or `--allow-env=HOME,PATH` to grant access to specific
+ones. See the [`--allow-env`][] documentation for more details.
+
 An example of this object looks like:
 
 <!-- eslint-skip -->
@@ -4595,6 +4601,7 @@ cases:
 [`'exit'`]: #event-exit
 [`'message'`]: child_process.md#event-message
 [`'uncaughtException'`]: #event-uncaughtexception
+[`--allow-env`]: cli.md#--allow-env
 [`--no-deprecation`]: cli.md#--no-deprecation
 [`--permission`]: cli.md#--permission
 [`--unhandled-rejections`]: cli.md#--unhandled-rejectionsmode

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',

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',

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;

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,15 @@ 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 +507,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 +531,12 @@ 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 +556,23 @@ 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);
+    }
   }
 }
 

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,

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;

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

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_

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)
@@ -140,6 +144,10 @@ Permission::Permission() : enabled_(false), warning_only_(false) {
   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
 }
 
 const char* GetErrorFlagSuggestion(node::permission::PermissionScope perm) {

src/permission/permission.h

@@ -8,6 +8,7 @@
 #include "node_options.h"
 #include "permission/addon_permission.h"
 #include "permission/child_process_permission.h"
+#include "permission/env_var_permission.h"
 #include "permission/ffi_permission.h"
 #include "permission/fs_permission.h"
 #include "permission/inspector_permission.h"

src/permission/permission_base.h

@@ -36,6 +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 PERMISSIONS(V)                                                         \
   FILESYSTEM_PERMISSIONS(V)                                                    \
@@ -45,10 +46,11 @@ namespace permission {
   INSPECTOR_PERMISSIONS(V)                                                     \
   NET_PERMISSIONS(V)                                                           \
   ADDON_PERMISSIONS(V)                                                         \
-  FFI_PERMISSIONS(V)
+  FFI_PERMISSIONS(V)                                                           \
+  ENV_VAR_PERMISSIONS(V)
 
-#define V(name, _, __, ___) k##name,
 enum class PermissionScope {
+#define V(name, _, __, ___) k##name,
   kPermissionsRoot = -1,
   PERMISSIONS(V) kPermissionsCount
 };