diff --git a/benchmark/common.js b/benchmark/common.js index 8443da40d79e..fa108b706831 100644 --- a/benchmark/common.js +++ b/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) => { diff --git a/doc/api/cli.md b/doc/api/cli.md index 1fd1e9864338..3d0e86aaa5a4 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -191,6 +191,50 @@ 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][], 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. See +[Environment variable permissions][] for details. + ### `--allow-ffi` + +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 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 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