Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

permission: add initial environment permission #48077

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,62 @@ Error: Access to this API has been restricted
}
```

### `--allow-env`

<!-- YAML
added: REPLACEME
-->

> Stability: 1 - Experimental

When using the [Permission Model][], access to the environment variables via
`process.env` is denied by default. Attempts to do so will throw an
`ERR_ACCESS_DENIED`. Users can explicitly configure permissions by passing the
`--allow-env` flag when starting Node.js.

The valid arguments for the `--allow-env` flag are:
daeyeon marked this conversation as resolved.
Show resolved Hide resolved

* Strings delimited by comma (`,`).
* `*` - Used as a wildcard.
* `-` - Used to deny access. By prefixing environment variables with `-`, you
can restrict access to them. It's worth noting that denied permissions always
take precedence over allowed permissions, regardless of the input order.

See the [Environment Permissions][] documentation for more details.

Example:

```js
// Attempt to bypass the permission
process.env.SECRET;
```

```console
$ node --experimental-permission --allow-fs-read=* index.js
node:internal/modules/cjs/loader:162
const result = internalModuleStat(filename);
^

Error: Access to this API has been restricted
at Object.<anonymous> (/home/index.js.js:1:13)
at Module._compile (node:internal/modules/cjs/loader:1256:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
at Module.load (node:internal/modules/cjs/loader:1114:32)
at Module._load (node:internal/modules/cjs/loader:961:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12)
at node:internal/main/run_main_module:23:47 {
code: 'ERR_ACCESS_DENIED',
permission: 'Environment',
resource: 'SECRET'
}
```

The process needs to have access to the `SECRET` in the environment variables:

```console
$ node --experimental-permission --allow-fs-read=* --allow-env=SECRET index.js
```

### `--allow-fs-read`

<!-- YAML
Expand Down Expand Up @@ -2101,6 +2157,7 @@ Node.js options that are allowed are:
<!-- node-options-node start -->

* `--allow-child-process`
* `--allow-env`
* `--allow-fs-read`
* `--allow-fs-write`
* `--allow-worker`
Expand Down Expand Up @@ -2561,6 +2618,7 @@ done
[CustomEvent Web API]: https://dom.spec.whatwg.org/#customevent
[ECMAScript module]: esm.md#modules-ecmascript-modules
[ECMAScript module loader]: esm.md#loaders
[Environment Permissions]: permissions.md#environment-permissions
[Fetch API]: https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API
[File System Permissions]: permissions.md#file-system-permissions
[Modules loaders]: packages.md#modules-loaders
Expand Down
50 changes: 47 additions & 3 deletions doc/api/permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -464,9 +464,10 @@ will restrict access to all available permissions.
The available permissions are documented by the [`--experimental-permission`][]
flag.

When starting Node.js with `--experimental-permission`,
the ability to access the file system through the `fs` module, spawn processes,
and use `node:worker_threads` will be restricted.
When starting Node.js with `--experimental-permission`, the ability to access
the file system through the `fs` module, access environment variables through
`process.env`, spawn processes, and use `node:worker_threads` will be
restricted.

```console
$ node --experimental-permission index.js
Expand All @@ -488,6 +489,9 @@ Error: Access to this API has been restricted
Allowing access to spawning a process and creating worker threads can be done
using the [`--allow-child-process`][] and [`--allow-worker`][] respectively.

Allowing access to environment variables through `process.env` can be done using
the [`--allow-env`][].

#### Runtime API

When enabling the Permission Model through the [`--experimental-permission`][]
Expand All @@ -506,6 +510,45 @@ process.permission.has('fs.read'); // true
process.permission.has('fs.read', '/home/rafaelgss/protected-folder'); // false
```

#### Environment Permissions

To configure permission to access the environment variables via `process.env`,
use the [`--allow-env`][] flag:

```console
$ node --experimental-permission --allow-fs-read=* --allow-env=* index.js
```

The valid arguments for the flag are:

* Strings delimited by comma (`,`).
* `*` - Used as a wildcard. To allow access to all environment variables, simply
use `--allow-env=*`.
* Use a string ended with `*` to allow access to variables that match the
specified prefix. For example, `--allow-env=NODE_*` will allow access to all
variables that start with `NODE_`.
* If `*` is not used as the last character of a string, such as `N*DE`,`*ODE`,
or `*ODE*`, the string is ignored.
* `-` - Used to deny access. By prefixing variables with `-`, you can restrict
daeyeon marked this conversation as resolved.
Show resolved Hide resolved
access to them. For example, `--allow-env=*,-SECURE_KEY` allows access to all
environment variables except `SECURE_KEY`. It's worth noting that denied
permissions always take precedence over allowed permissions, regardless of the
input order.

Example:

* `--allow-env=DATABASE_HOST` - `DATABASE_HOST` is allowed.
* `--allow-env=DATABASE_HOST,DATABASE_PORT` - `DATABASE_HOST` and
`DATABASE_PORT` are allowed.
* `--allow-env=DATABASE_*` - Environment variables starting with `DATABASE_` are
allowed.
* `--allow-env=DATABASE_*,-DATABASE_PASSWORD` - Environment variables starting
with `DATABASE_` except `DATABASE_PASSWORD` are allowed.
* `--allow-env=*,-DATABASE_PASSWORD` - All environment variables except
`DATABASE_PASSWORD` are allowed.
* `--allow-env=DATABASE_HOST,*_PORT` - `DATABASE_HOST` is allowed. (`*_PORT` is
ignored.)

#### File System Permissions

To allow access to the file system, use the [`--allow-fs-read`][] and
Expand Down Expand Up @@ -554,6 +597,7 @@ There are constraints you need to know before using this system:
[Import maps]: https://url.spec.whatwg.org/#relative-url-with-fragment-string
[Security Policy]: https://github.com/nodejs/node/blob/main/SECURITY.md
[`--allow-child-process`]: cli.md#--allow-child-process
[`--allow-env`]: cli.md#--allow-env
[`--allow-fs-read`]: cli.md#--allow-fs-read
[`--allow-fs-write`]: cli.md#--allow-fs-write
[`--allow-worker`]: cli.md#--allow-worker
Expand Down
6 changes: 6 additions & 0 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -2659,12 +2659,18 @@ The available scopes are:
* `fs.write` - File System write operations
* `child` - Child process spawning operations
* `worker` - Worker thread spawning operation
* `env` - Environment variable access operations

```js
// Check if the process has permission to read the README file
process.permission.has('fs.read', './README.md');
// Check if the process has read permission operations
process.permission.has('fs.read');

// Check if the process has permission to access SECRET in environment variables
process.permission.has('env', 'SECRET');
// Check if the process has permission to access all environment variables
process.permission.has('env');
```

## `process.pid`
Expand Down
3 changes: 3 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ Allow spawning process when using the permission model.
.It Fl -allow-worker
Allow creating worker threads when using the permission model.
.
.It Fl -allow-env
Allow environment variables access when using the permission model.
.
.It Fl -completion-bash
Print source-able bash completion script for Node.js.
.
Expand Down
13 changes: 9 additions & 4 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ const {
StringPrototypeSlice,
StringPrototypeToUpperCase,
} = primordials;
const {
privateSymbols: {
env_private_symbol,
},
} = internalBinding('util');

const {
convertToValidSignal,
Expand Down Expand Up @@ -534,10 +539,10 @@ ObjectDefineProperty(execFile, promisify.custom, {
});

function copyProcessEnvToEnv(env, name, optionEnv) {
if (process.env[name] &&
if (process[env_private_symbol][name] &&
(!optionEnv ||
!ObjectPrototypeHasOwnProperty(optionEnv, name))) {
env[name] = process.env[name];
env[name] = process[env_private_symbol][name];
}
}

Expand Down Expand Up @@ -622,7 +627,7 @@ function normalizeSpawnArguments(file, args, options) {
if (typeof options.shell === 'string')
file = options.shell;
else
file = process.env.comspec || 'cmd.exe';
file = process[env_private_symbol].comspec || 'cmd.exe';
// '/d /s /c' is used only for cmd.exe.
if (RegExpPrototypeExec(/^(?:.*\\)?cmd(?:\.exe)?$/i, file) !== null) {
args = ['/d', '/s', '/c', `"${command}"`];
Expand All @@ -647,7 +652,7 @@ function normalizeSpawnArguments(file, args, options) {
ArrayPrototypeUnshift(args, file);
}

const env = options.env || process.env;
const env = options.env || process[env_private_symbol];
const envPairs = [];

// process.env.NODE_V8_COVERAGE always propagates, making it possible to
Expand Down
8 changes: 7 additions & 1 deletion lib/cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,11 @@

'use strict';

const childOrPrimary = 'NODE_UNIQUE_ID' in process.env ? 'child' : 'primary';
const {
privateSymbols: {
env_private_symbol,
},
} = internalBinding('util');

const childOrPrimary = 'NODE_UNIQUE_ID' in process[env_private_symbol] ? 'child' : 'primary';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you may want to use ObjectHasOwn for this, since potentially anyone could add NODE_UNIQUE_ID to the prototype chain.

Suggested change
const childOrPrimary = 'NODE_UNIQUE_ID' in process[env_private_symbol] ? 'child' : 'primary';
const childOrPrimary = ObjectHasOwn(process[env_private_symbol], 'NODE_UNIQUE_ID') ? 'child' : 'primary';

module.exports = require(`internal/cluster/${childOrPrimary}`);
7 changes: 6 additions & 1 deletion lib/internal/cluster/child.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ const {
SafeMap,
SafeSet,
} = primordials;
const {
privateSymbols: {
env_private_symbol,
},
} = internalBinding('util');

const assert = require('internal/assert');
const path = require('path');
Expand All @@ -34,7 +39,7 @@ cluster.Worker = Worker;

cluster._setupWorker = function() {
const worker = new Worker({
id: +process.env.NODE_UNIQUE_ID | 0,
id: +process[env_private_symbol].NODE_UNIQUE_ID | 0,
process: process,
state: 'online',
});
Expand Down
9 changes: 7 additions & 2 deletions lib/internal/cluster/primary.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ const {
ERR_SOCKET_BAD_PORT,
},
} = require('internal/errors');
const {
privateSymbols: {
env_private_symbol,
},
} = internalBinding('util');

const assert = require('internal/assert');
const { fork } = require('child_process');
Expand Down Expand Up @@ -45,7 +50,7 @@ let ids = 0;
let initialized = false;

// XXX(bnoordhuis) Fold cluster.schedulingPolicy into cluster.settings?
let schedulingPolicy = process.env.NODE_CLUSTER_SCHED_POLICY;
let schedulingPolicy = process[env_private_symbol].NODE_CLUSTER_SCHED_POLICY;
if (schedulingPolicy === 'rr')
schedulingPolicy = SCHED_RR;
else if (schedulingPolicy === 'none')
Expand Down Expand Up @@ -116,7 +121,7 @@ function setupSettingsNT(settings) {
}

function createWorkerProcess(id, env) {
const workerEnv = { ...process.env, ...env, NODE_UNIQUE_ID: `${id}` };
const workerEnv = { ...process[env_private_symbol], ...env, NODE_UNIQUE_ID: `${id}` };
const execArgv = [...cluster.settings.execArgv];

if (cluster.settings.inspectPort === null) {
Expand Down
9 changes: 7 additions & 2 deletions lib/internal/console/constructor.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@ const {
validateInteger,
validateObject,
} = require('internal/validators');
const { previewEntries } = internalBinding('util');
const {
previewEntries,
privateSymbols: {
env_private_symbol,
},
} = internalBinding('util');
const { Buffer: { isBuffer } } = require('buffer');
const {
inspect,
Expand Down Expand Up @@ -442,7 +447,7 @@ const consoleMethods = {
clear() {
// It only makes sense to clear if _stdout is a TTY.
// Otherwise, do nothing.
if (this._stdout.isTTY && process.env.TERM !== 'dumb') {
if (this._stdout.isTTY && process[env_private_symbol].TERM !== 'dumb') {
// The require is here intentionally to avoid readline being
// required too early when console is first loaded.
const {
Expand Down
7 changes: 6 additions & 1 deletion lib/internal/debugger/inspect_repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ const {
StringPrototypeToUpperCase,
StringPrototypeTrim,
} = primordials;
const {
privateSymbols: {
env_private_symbol,
},
} = internalBinding('util');

const { ERR_DEBUGGER_ERROR } = require('internal/errors').codes;

Expand Down Expand Up @@ -887,7 +892,7 @@ function createRepl(inspector) {
}

Debugger.on('paused', ({ callFrames, reason /* , hitBreakpoints */ }) => {
if (process.env.NODE_INSPECT_RESUME_ON_START === '1' &&
if (process[env_private_symbol].NODE_INSPECT_RESUME_ON_START === '1' &&
reason === 'Break on start') {
debuglog('Paused on start, but NODE_INSPECT_RESUME_ON_START' +
' environment variable is set to 1, resuming');
Expand Down
12 changes: 9 additions & 3 deletions lib/internal/main/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,20 @@ const { getOptionValue } = require('internal/options');

const { exitCodes: { kInvalidCommandLineArgument } } = internalBinding('errors');

const {
privateSymbols: {
env_private_symbol,
},
} = internalBinding('util');

prepareMainThreadExecution();

markBootstrapComplete();

if (process.env.NODE_REPL_EXTERNAL_MODULE) {
if (process[env_private_symbol].NODE_REPL_EXTERNAL_MODULE) {
require('internal/modules/cjs/loader')
.Module
._load(process.env.NODE_REPL_EXTERNAL_MODULE, undefined, true);
._load(process[env_private_symbol].NODE_REPL_EXTERNAL_MODULE, undefined, true);
} else {
// --input-type flag not supported in REPL
if (getOptionValue('--input-type')) {
Expand All @@ -41,7 +47,7 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) {
'Type ".help" for more information.');

const cliRepl = require('internal/repl');
cliRepl.createInternalRepl(process.env, (err, repl) => {
cliRepl.createInternalRepl(process[env_private_symbol], (err, repl) => {
if (err) {
throw err;
}
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/main/watch_mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ const {
ArrayPrototypeSlice,
StringPrototypeStartsWith,
} = primordials;
const {
privateSymbols: {
env_private_symbol,
},
} = internalBinding('util');

const {
prepareMainThreadExecution,
Expand Down Expand Up @@ -54,7 +59,10 @@ let exited;
function start() {
exited = false;
const stdio = kShouldFilterModules ? ['inherit', 'inherit', 'inherit', 'ipc'] : 'inherit';
child = spawn(process.execPath, args, { stdio, env: { ...process.env, WATCH_REPORT_DEPENDENCIES: '1' } });
child = spawn(process.execPath, args, {
stdio,
env: { ...process[env_private_symbol], WATCH_REPORT_DEPENDENCIES: '1' },
});
watcher.watchChildProcessModules(child);
child.once('exit', (code) => {
exited = true;
Expand Down
Loading