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

doc/api/cli.md

@@ -191,6 +191,58 @@ 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`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1.1 - Active development
+
+When using the [Permission Model][], access to environment variables is
+restricted by default. Reading a restricted variable silently returns
+`undefined`, while writing or deleting throws `ERR_ACCESS_DENIED`.
+Enumerating (`Object.keys(process.env)`) only returns allowed variables.
+
+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`
+
+Examples:
+
+```console
+$ node --permission --allow-fs-read=* -p 'process.env.HOME'
+undefined
+$ node --permission --allow-env=HOME --allow-fs-read=* -p 'process.env.HOME'
+/Users/user
+$ node --permission --allow-env=* --allow-fs-read=* -p 'process.env.HOME'
+/Users/user
+```
+
+Environment variable permissions can be checked programmatically:
+
+```js
+process.permission.has('env'); // True if any env access is allowed
+process.permission.has('env', 'HOME'); // True if HOME is allowed
+```
+
+Attempting to write a restricted variable throws:
+
+```console
+$ node --permission --allow-fs-read=* -e "process.env.FOO = 'bar'"
+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 +2276,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 +3724,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 +4363,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,13 @@ 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 by default. 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 -->
@@ -3189,6 +3196,7 @@ The available scopes are:
 * `fs.write` - File System write operations
 * `child` - Child process spawning operations
 * `worker` - Worker thread spawning operation
+* `env` - Environment variable operations
 * `ffi` - Foreign function interface operations
 
 ```js
@@ -4595,6 +4603,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

doc/node.1

@@ -135,6 +135,25 @@ This behavior also applies to \fBchild_process.spawn()\fR, but in that case, the
 flags are propagated via the \fBNODE_OPTIONS\fR environment variable rather than
 directly through the process arguments.
 .
+.It Fl -allow-env
+When using the Permission Model, the process will be able to access all
+environment variables by default.
+When the \fB--allow-env\fR flag is used, the process will only be able to access
+the specified environment variables.
+.Pp
+The valid arguments for the \fB--allow-env\fR flag are:
+.Bl -bullet
+.It
+\fB*\fR - To allow access to all environment variables.
+.It
+Multiple environment variable names can be allowed using multiple
+\fB--allow-env\fR flags.
+Example \fB--allow-env=HOME --allow-env=PATH\fR
+.El
+.Bd -literal
+$ node --permission --allow-env=HOME --allow-fs-read=* index.js
+.Ed
+.
 .It Fl -allow-ffi
 When using the Permission Model, the process will not be able to use
 \fBnode:ffi\fR by default.
@@ -1146,6 +1165,8 @@ WASI - manageable through \fB--allow-wasi\fR flag
 Addons - manageable through \fB--allow-addons\fR flag
 .It
 FFI - manageable through \fB--allow-ffi\fR flag
+.It
+Environment Variables - manageable through \fB--allow-env\fR flag
 .El
 .
 .It Fl -permission-audit
@@ -1860,6 +1881,8 @@ one is included in the list below.
 .It
 \fB--allow-child-process\fR
 .It
+\fB--allow-env\fR
+.It
 \fB--allow-ffi\fR
 .It
 \fB--allow-fs-read\fR

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, {}, permission::PermissionScope::kEnvVar);
+    } else {
+      permission()->Apply(
+          this, options_->allow_env, 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