src,lib: add --allow-env permission

benchmark/common.js

@@ -198,6 +198,21 @@ class Benchmark {
   }
 
   _run() {
+    // A forked child is told to run the benchmark function directly, rather
+    // than build its own queue and fork again, through the
+    // NODE_RUN_BENCHMARK_FN environment variable. A child always inherits
+    // this.flags in its execArgv, so reaching _run() with those flags already
+    // applied means the variable did not survive to the child and every
+    // generation would keep forking. Fail loudly instead of forking forever.
+    if (process.send &&
+        this.flags.length > 0 &&
+        this.flags.every((flag) => process.execArgv.includes(flag))) {
+      throw new Error(
+        'Benchmark child process was started with the benchmark flags but ' +
+        'without NODE_RUN_BENCHMARK_FN, refusing to fork again. Something ' +
+        'removed the variable from the child environment.');
+    }
+
     // If forked, report to the parent.
     if (process.send) {
       process.send({
@@ -213,6 +228,20 @@ class Benchmark {
       this.originalOptions.setup(this.queue);
     }
 
+    // Enforcing the permission model removes the environment variables
+    // --allow-env does not grant access to at startup, which would drop the
+    // NODE_RUN_BENCHMARK_FN set below. The child only ever sees the
+    // environment this process hands it, so granting access to all of it does
+    // not widen what the benchmark can reach. Audit mode removes nothing, so
+    // it is left alone to keep its diagnostics intact.
+    const childExecArgv = this.flags.concat(process.execArgv);
+    const enforcesPermission = (arg) =>
+      arg === '--permission' || arg.startsWith('--permission=');
+    if (childExecArgv.some(enforcesPermission) &&
+        !childExecArgv.some((arg) => arg.startsWith('--allow-env'))) {
+      childExecArgv.push('--allow-env=*');
+    }
+
     const recursive = (queueIndex) => {
       const config = this.queue[queueIndex];
 
@@ -233,7 +262,7 @@ class Benchmark {
 
       const child = child_process.fork(require.main.filename, childArgs, {
         env: childEnv,
-        execArgv: this.flags.concat(process.execArgv),
+        execArgv: childExecArgv,
       });
       child.on('message', sendResult);
       child.on('close', (code) => {

doc/api/cli.md

@@ -191,6 +191,51 @@ 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][], the process starts without the environment
+variables it has not been granted access to. At startup, every variable that
+`--allow-env` does not match is removed from the process environment. Removed
+variables are absent from `process.env`, from diagnostic reports, from native
+code calling `getenv()`, and from the environment of child processes and worker
+threads.
+
+The valid values are:
+
+* `*` - Grants access to every environment variable.
+* A variable name, for example `--allow-env=DATABASE_URL`.
+* A variable name prefix followed by `*`, for example `--allow-env=APP_*`.
+
+Multiple values can be passed by repeating the flag, or by separating them with
+commas: `--allow-env=PORT,APP_*`. Variable names are case-insensitive on
+Windows.
+
+Example:
+
+```js
+console.log(process.env.DATABASE_URL);
+console.log(process.env.AWS_SECRET_ACCESS_KEY);
+```
+
+```console
+$ node --permission --allow-fs-read=* --allow-env=DATABASE_URL index.js
+postgres://localhost/app
+undefined
+(node:1234) Warning: The permission model removed the environment variable "AWS_SECRET_ACCESS_KEY" at startup. Use --allow-env to manage permissions.
+```
+
+The variables that Node.js and its bundled dependencies read, such as
+`NODE_OPTIONS`, `PATH`, `HOME`, `TZ`, and `SSL_CERT_FILE`, are always kept, as
+are the variables defined in [`--env-file`][] files. `NODE_ENV` is not kept
+by default, so applications and libraries that read it need
+`--allow-env=NODE_ENV`. See [Environment variable permissions][] for details.
+
 ### `--allow-ffi`
 
 <!-- YAML
@@ -402,6 +447,11 @@ This flag grants broad authority to configured OpenSSL STORE loaders. A loader
 may access files, devices, tokens, or the network. Access performed by a loader
 is not constrained by the `fs.read`, `fs.write`, or `net` permission scopes.
 
+Loaders and the modules they load are subject to [`--allow-env`][], however.
+Environment variables they rely on, such as `SOFTHSM2_CONF` for SoftHSM, are
+removed at startup unless they are granted explicitly with `--allow-env`. See
+[Environment variable permissions][] for details.
+
 ### `--allow-wasi`
 
 <!-- YAML
@@ -2538,6 +2588,7 @@ following permissions are restricted:
 * File System - manageable through
   [`--allow-fs-read`][], [`--allow-fs-write`][] flags
 * Network - manageable through [`--allow-net`][] flag
+* Environment variables - manageable through [`--allow-env`][] flag
 * Child Process - manageable through [`--allow-child-process`][] flag
 * Worker Threads - manageable through [`--allow-worker`][] flag
 * WASI - manageable through [`--allow-wasi`][] flag
@@ -4179,6 +4230,7 @@ one is included in the list below.
 
 * `--allow-addons`
 * `--allow-child-process`
+* `--allow-env`
 * `--allow-ffi`
 * `--allow-fs-read`
 * `--allow-fs-vfs`
@@ -4826,6 +4878,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
 [CommonJS module]: modules.md
 [DEP0025 warning]: deprecations.md#dep0025-requirenodesys
 [ECMAScript module]: esm.md#modules-ecmascript-modules
+[Environment variable permissions]: permissions.md#environment-variable-permissions
 [EventSource Web API]: https://html.spec.whatwg.org/multipage/server-sent-events.html#server-sent-events
 [ExperimentalWarning: `vm.measureMemory` is an experimental feature]: vm.md#vmmeasurememoryoptions
 [FIPS mode]: crypto.md#fips-mode
@@ -4849,6 +4902,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
 [`'crypto.fips.indicator'`]: diagnostics_channel.md#event-cryptofipsindicator
 [`--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/embedding.md

@@ -72,6 +72,51 @@ int main(int argc, char** argv) {
 }
 ```
 
+### Restricting access to environment variables
+
+<!-- YAML
+added: REPLACEME
+-->
+
+When the arguments passed to `node::InitializeOncePerProcess()` enable the
+[Permission Model][] without `--allow-env=*`, the process environment must not
+contain any variable that [`--allow-env`][] does not grant access to.
+`node::InitializeOncePerProcess()` fails otherwise. Unlike the `node`
+executable, embedders own the process environment, so Node.js does not remove
+these variables itself.
+
+`node::ScrubProcessEnvironment()` removes them. Because it modifies the process
+environment without any locking that native code calling `getenv()`
+participates in, it must be called before starting any thread that may read the
+environment, and before `node::InitializeOncePerProcess()`:
+
+```cpp
+int main(int argc, char** argv) {
+  argv = uv_setup_args(argc, argv);
+  std::vector<std::string> args(argv, argv + argc);
+
+  // Keep the variables the embedder itself reads, in addition to the ones
+  // Node.js reads (see node::GetRuntimeEnvironmentDefaults()).
+  node::ProcessEnvironmentScrubOptions scrub_options;
+  scrub_options.allow = {"PORT", "APP_*"};
+  if (node::ScrubProcessEnvironment(scrub_options).IsNothing()) {
+    return 1;
+  }
+
+  // args contains, for example, --permission --allow-env=PORT
+  std::unique_ptr<node::InitializationResult> result =
+      node::InitializeOncePerProcess(args, {
+        node::ProcessInitializationFlags::kNoInitializeV8,
+        node::ProcessInitializationFlags::kNoInitializeNodeV8Platform
+      });
+  // ...
+}
+```
+
+`process.permission.drop('env', name)` removes a variable from the process
+environment, so it throws when called from a `node::Environment` created
+without `node::EnvironmentFlags::kOwnsProcessState`.
+
 ### Setting up a per-instance state
 
 <!-- YAML
@@ -178,6 +223,8 @@ int RunNodeInstance(MultiIsolatePlatform* platform,
 ```
 
 [CLI options]: cli.md
+[Permission Model]: permissions.md#permission-model
+[`--allow-env`]: cli.md#--allow-env
 [`process.memoryUsage()`]: process.md#processmemoryusage
 [deprecation policy]: deprecations.md
 [embedtest.cc]: https://github.com/nodejs/node/blob/HEAD/test/embedding/embedtest.cc