Skip to content

src,lib: add --allow-env permission - #66132

Open
jasnell wants to merge 2 commits into
nodejs:mainfrom
jasnell:jasnell/allow-env
Open

jasnell wants to merge 2 commits into
nodejs:mainfrom
jasnell:jasnell/allow-env

Conversation

@jasnell

@jasnell jasnell commented Sep 19, 2026

Copy link
Copy Markdown
Member

Necessarily semver-major.

Alternative for a previous attempt in #62827 that stalled out.

When --permission is on, every env var not matched by --allow-env is removed at startup. It takes names, patterns (PREFI_*), or *, repeatable or comma-sep'd.

There are a range of env vars that Node.js itself uses, and a default range that are generally known to be safe in common usage. These are never scrubbed. These include things like NODE_OPTIONS, NODE_EXTRA_CA_CERTS, PATH, HOME, etc.

Env vars can be dropped at runtime after reading using permission.drop(). This is a stronger protection than using process.env.FOO = undefined because it will scrub the env var also from the environment block.

On Linux, the removed entries are overwritten in the initial environment block and fs reads of /proc/*/environ are denied.

On Windows, removal also clears the C runtime's copy of the environ using _wputenv_s

Reading a removed name returns undefined, warns once per name, and publishes to a diagnostics channel.

Env file keys are allowed. If the user had reason to pass in an env file the assumption is they meant to allow them.

File-source config (node.config.json and NODE_OPTIONS from a .env file can only narrow the allow list.

Embedders must call ScrubProcessEnvironment() themselves on startup. This is left up to the embedder to determine the exact timing but needs to be called before startup actually happens.

Child processes are started with --allow-env=*. Those either receive the explicit env they were started with or only the env they inherit from the parent. Since the parent process is scrubbed, it should never be more than what the parent can see.

The key motivation here is that environment variables are the primary mechanism for injecting secrets into applications. These can be trivially exfiltrated using simple one-liners like fetch('...', { headers: { secret: process.env.SECRET } }). Simply unsetting those via process.env.SECRET = undefined is typically not enough since those can still be read from the underlying environment block. This PR provides stronger protection but it obviously cannot be 100% since depending on how it is used, the strings can still be in memory.

The prior attempt to add this in #62827 had a number of flaws that this version addresses. That PR used an incomplete view filter. process.report.getReport(), native addons, FFI,
and /proc/self/environ could all still read the real environment. Scrubbing
before any JavaScript runs protects those paths. This PR also fixes a number of other breaking changes the other PR would have introduced:

  • Node.js's own configuration variables are always kept.
  • Writes to process.env and process.loadEnvFile() keep working.
  • Keys defined in --env-file files are allowed.
  • Child processes get --allow-env=*, because the environment they inherit has already been scrubbed.

$ ENV_A=1 ENV_B=2 ./node --permission -pe "process.env"
{
  COLORTERM: 'truecolor',
  TERM_PROGRAM_VERSION: '3.4',
  TMUX: '/tmp/tmux-1000/default,1258,1',
  HOME: '/home/dev',
  LANG: 'C.UTF-8',
  TERM: 'tmux-256color',
  PATH: '/home/dev/bin:/usr/lib/ccache:/opt/rust/cargo/bin:/opt/nvm/versions/node/v26.9.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin',
  TERM_PROGRAM: 'tmux'
}

$ ENV_A=1 ENV_B=2 ./node --permission -pe "process.env" --allow-env=ENV_*
{
  ENV_B: '2',
  ENV_A: '1',
  COLORTERM: 'truecolor',
  TERM_PROGRAM_VERSION: '3.4',
  TMUX: '/tmp/tmux-1000/default,1258,1',
  HOME: '/home/dev',
  LANG: 'C.UTF-8',
  TERM: 'tmux-256color',
  PATH: '/home/dev/bin:/usr/lib/ccache:/opt/rust/cargo/bin:/opt/nvm/versions/node/v26.9.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin',
  TERM_PROGRAM: 'tmux'
}
$

@jasnell jasnell added semver-major PRs that contain breaking changes and should be released in the next major version. permission Issues and PRs related to the Permission Model. labels Sep 19, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 19, 2026
@codecov

codecov Bot commented Sep 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 62 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.27%. Comparing base (dd5dfb5) to head (9127642).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
src/permission/env_permission.cc 86.93% 1 Missing and 28 partials ⚠️
lib/child_process.js 0.00% 12 Missing ⚠️
src/node.cc 92.39% 3 Missing and 4 partials ⚠️
src/node_env_var.cc 78.12% 3 Missing and 4 partials ⚠️
src/permission/permission.cc 64.70% 3 Missing and 3 partials ⚠️
src/node_dotenv.cc 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #66132    +/-   ##
========================================
  Coverage   90.27%   90.27%            
========================================
  Files         790      792     +2     
  Lines      271651   272380   +729     
  Branches    51842    52013   +171     
========================================
+ Hits       245228   245889   +661     
- Misses      16928    16946    +18     
- Partials     9495     9545    +50     
Files with missing lines Coverage Δ
lib/internal/process/permission.js 92.47% <100.00%> (+0.08%) ⬆️
src/env.cc 82.10% <100.00%> (+0.01%) ⬆️
src/node.h 92.15% <100.00%> (+0.49%) ⬆️
src/node_dotenv.h 100.00% <ø> (ø)
src/node_options.cc 80.88% <100.00%> (+0.16%) ⬆️
src/node_options.h 95.63% <ø> (ø)
src/permission/env_permission.h 100.00% <100.00%> (ø)
src/permission/permission.h 100.00% <ø> (ø)
src/permission/permission_base.h 100.00% <ø> (ø)
src/node_dotenv.cc 84.76% <85.71%> (+0.03%) ⬆️
... and 5 more

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jasnell
jasnell marked this pull request as draft September 19, 2026 16:19
@jasnell

This comment was marked as resolved.

@jasnell
jasnell marked this pull request as ready for review September 19, 2026 17:29
Necessarily semver-major.

When `--permission` is on, every env var not matched by
`--allow-env` is removed at startup. It takes names,
patterns (`PREFI_*`), or `*`, repeatable or comma-sep'd.

There are a range of env vars that Node.js itself uses,
and a default range that are generally known to be safe
in common usage. These are never scrubbed. These include
things like `NODE_OPTIONS`, `NODE_EXTRA_CA_CERTS`, `PATH`,
`HOME`, etc.

Env vars can be dropped at runtime after reading using
`permission.drop()`. This is a stronger protection than
using `process.env.FOO = undefined` because it will
scrub the env var also from the environment block.

On Linux, the removed entries are overwritten in the
initial environment block and fs reads of /proc/*/environ
are denied.

On Windows, removal also clears the C runtime's copy
of the environ using _wputenv_s

Reading a removed name returns undefined, warns once per
name, and publishes to a diagnostics channel.

Env file keys are allowed. If the user had reason to pass
in an env file the assumption is they meant to allow them.

File-source config (node.config.json and NODE_OPTIONS
from a .env file can only narrow the allow list.

Embedders must call ScrubProcessEnvironment() themselves
on startup. This is left up to the embedder to determine
the exact timing but needs to be called before startup
actually happens.

Child processes are started with `--allow-env=*`. Those
either receive the explicit env they were started with
or only the env they inherit from the parent. Since the
parent process is scrubbed, it should never be more than
what the parent can see.

Main part of the impl was done by hand. Docs, tests,
verification pass, and cleanup nits were automated.

Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. permission Issues and PRs related to the Permission Model. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants