diff --git a/benchmark/fs/readfile-permission-enabled.js b/benchmark/fs/readfile-permission-enabled.js index 08ac831cd73d1b..a98c370f08216a 100644 --- a/benchmark/fs/readfile-permission-enabled.js +++ b/benchmark/fs/readfile-permission-enabled.js @@ -24,6 +24,7 @@ const bench = common.createBenchmark(main, { '--allow-fs-read=*', '--allow-fs-write=*', '--allow-child-process', + '--allow-env', ], }); diff --git a/benchmark/permission/permission-fs-read.js b/benchmark/permission/permission-fs-read.js index bd81814e55861a..24482d19d36e27 100644 --- a/benchmark/permission/permission-fs-read.js +++ b/benchmark/permission/permission-fs-read.js @@ -13,6 +13,7 @@ const options = { flags: [ '--experimental-permission', `--allow-fs-read=${rootPath}`, + '--allow-env', ], }; diff --git a/doc/api/cli.md b/doc/api/cli.md index dfb29cc4e20667..08684b659f9a9d 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -141,6 +141,61 @@ Error: Access to this API has been restricted } ``` +### `--allow-env`, `--allow-env-name` + + + +> 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 flag are: + +* Empty - Used to allow access to all environment variables. The empty argument + is only available on `--allow-env`. +* Strings delimited by comma (`,`) - Used to allow access to specified + environment variables. + +Examples can be found in the [Environment Permissions][] documentation. + +Example: + +```js +// Attempt to bypass the permission +process.env.PORT; +``` + +```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. (/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: 'PORT' +} +``` + +The process needs to have access to the `PORT` in the environment variables: + +```console +$ node --experimental-permission --allow-fs-read=* --allow-env=PORT index.js +``` + ### `--allow-fs-read` * `--allow-child-process` +* `--allow-env`, `--allow-env-name` * `--allow-fs-read` * `--allow-fs-write` * `--allow-worker` @@ -2561,6 +2617,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 diff --git a/doc/api/permissions.md b/doc/api/permissions.md index e29cbc7580e75d..d031b8988a99d0 100644 --- a/doc/api/permissions.md +++ b/doc/api/permissions.md @@ -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 @@ -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`][] @@ -506,6 +510,27 @@ 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: + +* Empty - Used to allow access to all environment variables. +* Strings delimited by comma (`,`) - Used to allow access to specified + environment variable names. + +Example: + +* `--allow-env` - It will allow accees to all environment variables. +* `--allow-env=HOME` - It will allow accees to `HOME`. +* `--allow-env=HOME,PORT` - It will allow accees to both `HOME` and `PORT`. + #### File System Permissions To allow access to the file system, use the [`--allow-fs-read`][] and @@ -554,6 +579,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-env-name [`--allow-fs-read`]: cli.md#--allow-fs-read [`--allow-fs-write`]: cli.md#--allow-fs-write [`--allow-worker`]: cli.md#--allow-worker diff --git a/doc/api/process.md b/doc/api/process.md index ad94a9721de3b7..9da99a8b847ec7 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -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` diff --git a/doc/node.1 b/doc/node.1 index 90d0e42e90deb4..b0b321492298c3 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -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. . diff --git a/lib/child_process.js b/lib/child_process.js index 59c37b97672d39..69bafe1a4416fd 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -43,6 +43,11 @@ const { StringPrototypeSlice, StringPrototypeToUpperCase, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { convertToValidSignal, @@ -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]; } } @@ -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}"`]; @@ -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 diff --git a/lib/cluster.js b/lib/cluster.js index 7ca8b532ee243f..7fdccff60928ef 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -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'; module.exports = require(`internal/cluster/${childOrPrimary}`); diff --git a/lib/internal/cluster/child.js b/lib/internal/cluster/child.js index 1bddd3ca0ac103..8450c6e643cd9d 100644 --- a/lib/internal/cluster/child.js +++ b/lib/internal/cluster/child.js @@ -8,6 +8,11 @@ const { SafeMap, SafeSet, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const assert = require('internal/assert'); const path = require('path'); @@ -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', }); diff --git a/lib/internal/cluster/primary.js b/lib/internal/cluster/primary.js index 945f440cd19797..a8084053545715 100644 --- a/lib/internal/cluster/primary.js +++ b/lib/internal/cluster/primary.js @@ -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'); @@ -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') @@ -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) { diff --git a/lib/internal/console/constructor.js b/lib/internal/console/constructor.js index 44fa0f32d76ff3..1a6b08fb81499c 100644 --- a/lib/internal/console/constructor.js +++ b/lib/internal/console/constructor.js @@ -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, @@ -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 { diff --git a/lib/internal/debugger/inspect_repl.js b/lib/internal/debugger/inspect_repl.js index 7fba613f09c632..2c5f41313f2b80 100644 --- a/lib/internal/debugger/inspect_repl.js +++ b/lib/internal/debugger/inspect_repl.js @@ -43,6 +43,11 @@ const { StringPrototypeToUpperCase, StringPrototypeTrim, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { ERR_DEBUGGER_ERROR } = require('internal/errors').codes; @@ -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'); diff --git a/lib/internal/main/repl.js b/lib/internal/main/repl.js index da1764a9c80d95..48ec5bf0bef35c 100644 --- a/lib/internal/main/repl.js +++ b/lib/internal/main/repl.js @@ -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')) { @@ -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; } diff --git a/lib/internal/main/watch_mode.js b/lib/internal/main/watch_mode.js index 5d0d29cc2ff9da..98a11392459948 100644 --- a/lib/internal/main/watch_mode.js +++ b/lib/internal/main/watch_mode.js @@ -8,6 +8,11 @@ const { ArrayPrototypeSlice, StringPrototypeStartsWith, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { prepareMainThreadExecution, @@ -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; diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index 12ae4a9b23212d..ffb0eb134ca3a0 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -16,6 +16,11 @@ const { SharedArrayBuffer, }, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { prepareWorkerThreadExecution, @@ -65,7 +70,7 @@ const port = getEnvMessagePort(); // If the main thread is spawned with env NODE_CHANNEL_FD, it's probably // spawned by our child_process module. In the work threads, mark the // related IPC properties as unavailable. -if (process.env.NODE_CHANNEL_FD) { +if (process[env_private_symbol].NODE_CHANNEL_FD) { const workerThreadSetup = require('internal/process/worker_thread_only'); ObjectDefineProperty(process, 'channel', { __proto__: null, diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 60fa7e79d19693..77798cf0c4d923 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -96,6 +96,7 @@ const { internalModuleStat } = internalBinding('fs'); const { safeGetenv } = internalBinding('credentials'); const { privateSymbols: { + env_private_symbol, require_private_symbol, }, } = internalBinding('util'); @@ -113,7 +114,7 @@ const { getOptionValue, getEmbedderOptions } = require('internal/options'); const policy = getLazy( () => (getOptionValue('--experimental-policy') ? require('internal/process/policy') : null), ); -const shouldReportRequiredModules = getLazy(() => process.env.WATCH_REPORT_DEPENDENCIES); +const shouldReportRequiredModules = getLazy(() => process[env_private_symbol].WATCH_REPORT_DEPENDENCIES); const getCascadedLoader = getLazy( () => require('internal/process/esm_loader').esmLoader, @@ -1383,8 +1384,8 @@ function createRequire(filename) { Module.createRequire = createRequire; Module._initPaths = function() { - const homeDir = isWindows ? process.env.USERPROFILE : safeGetenv('HOME'); - const nodePath = isWindows ? process.env.NODE_PATH : safeGetenv('NODE_PATH'); + const homeDir = isWindows ? process[env_private_symbol].USERPROFILE : safeGetenv('HOME'); + const nodePath = isWindows ? process[env_private_symbol].NODE_PATH : safeGetenv('NODE_PATH'); // process.execPath is $PREFIX/bin/node except on Windows where it is // $PREFIX\node.exe where $PREFIX is the root of the Node.js installation. diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 9d94b7275fdbdd..68fe46c928959f 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -12,6 +12,11 @@ const { SafePromiseAllReturnArrayLike, SafeWeakMap, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { ERR_UNKNOWN_MODULE_FORMAT, @@ -211,7 +216,7 @@ class DefaultModuleLoader { getOptionValue('--inspect-brk') ); - if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) { + if (process[env_private_symbol].WATCH_REPORT_DEPENDENCIES && process.send) { process.send({ 'watch:import': [url] }); } diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 21703d63f6aa41..87ec2b34ce7d9a 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -23,6 +23,11 @@ const { StringPrototypeSplit, StringPrototypeStartsWith, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const internalFS = require('internal/fs/utils'); const { BuiltinModule } = require('internal/bootstrap/realm'); const { realpathSync } = require('fs'); @@ -220,7 +225,7 @@ function finalizeResolution(resolved, base, preserveSymlinks) { throw err; } else if (stats !== 0) { // Check for !stats.isFile() - if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) { + if (process[env_private_symbol].WATCH_REPORT_DEPENDENCIES && process.send) { process.send({ 'watch:require': [path || resolved.pathname] }); } throw new ERR_MODULE_NOT_FOUND( diff --git a/lib/internal/options.js b/lib/internal/options.js index effe3249888efd..53e23d0fea7a82 100644 --- a/lib/internal/options.js +++ b/lib/internal/options.js @@ -4,6 +4,11 @@ const { getCLIOptions, getEmbedderOptions: getEmbedderOptionsFromBinding, } = internalBinding('options'); +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); let warnOnAllowUnauthorized = true; @@ -51,7 +56,7 @@ function getOptionValue(optionName) { } function getAllowUnauthorized() { - const allowUnauthorized = process.env.NODE_TLS_REJECT_UNAUTHORIZED === '0'; + const allowUnauthorized = process[env_private_symbol].NODE_TLS_REJECT_UNAUTHORIZED === '0'; if (allowUnauthorized && warnOnAllowUnauthorized) { warnOnAllowUnauthorized = false; diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index fc6d9ccf4f517b..3e31cc9787b3c8 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -10,6 +10,11 @@ const { StringPrototypeStartsWith, globalThis, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { getOptionValue, @@ -204,7 +209,7 @@ function setupWarningHandler() { resetForSerialization, } = require('internal/process/warning'); if (getOptionValue('--warnings') && - process.env.NODE_NO_WARNINGS !== '1') { + process[env_private_symbol].NODE_NO_WARNINGS !== '1') { process.on('warning', onWarning); // The code above would add the listener back during deserialization, @@ -308,10 +313,10 @@ function setupCodeCoverage() { // to child processes even when they switch cwd. Don't do anything if the // --experimental-test-coverage flag is present, as the test runner will // handle coverage. - if (process.env.NODE_V8_COVERAGE && + if (process[env_private_symbol].NODE_V8_COVERAGE && !getOptionValue('--experimental-test-coverage')) { - process.env.NODE_V8_COVERAGE = - setupCoverageHooks(process.env.NODE_V8_COVERAGE); + process[env_private_symbol].NODE_V8_COVERAGE = + setupCoverageHooks(process[env_private_symbol].NODE_V8_COVERAGE); } } @@ -349,7 +354,7 @@ function initializeReport() { } function setupDebugEnv() { - require('internal/util/debuglog').initializeDebugEnv(process.env.NODE_DEBUG); + require('internal/util/debuglog').initializeDebugEnv(process[env_private_symbol].NODE_DEBUG); if (getOptionValue('--expose-internals')) { require('internal/bootstrap/realm').BuiltinModule.exposeInternals(); } @@ -476,18 +481,18 @@ function initializeDeprecations() { } function setupChildProcessIpcChannel() { - if (process.env.NODE_CHANNEL_FD) { + if (process[env_private_symbol].NODE_CHANNEL_FD) { const assert = require('internal/assert'); - const fd = NumberParseInt(process.env.NODE_CHANNEL_FD, 10); + const fd = NumberParseInt(process[env_private_symbol].NODE_CHANNEL_FD, 10); assert(fd >= 0); // Make sure it's not accidentally inherited by child processes. - delete process.env.NODE_CHANNEL_FD; + delete process[env_private_symbol].NODE_CHANNEL_FD; const serializationMode = - process.env.NODE_CHANNEL_SERIALIZATION_MODE || 'json'; - delete process.env.NODE_CHANNEL_SERIALIZATION_MODE; + process[env_private_symbol].NODE_CHANNEL_SERIALIZATION_MODE || 'json'; + delete process[env_private_symbol].NODE_CHANNEL_SERIALIZATION_MODE; require('child_process')._forkChild(fd, serializationMode); assert(process.send); @@ -495,11 +500,11 @@ function setupChildProcessIpcChannel() { } function initializeClusterIPC() { - if (process.argv[1] && process.env.NODE_UNIQUE_ID) { + if (process.argv[1] && process[env_private_symbol].NODE_UNIQUE_ID) { const cluster = require('cluster'); cluster._setupWorker(); // Make sure it's not accidentally inherited by child processes. - delete process.env.NODE_UNIQUE_ID; + delete process[env_private_symbol].NODE_UNIQUE_ID; } } @@ -536,6 +541,7 @@ function initializePermission() { '--allow-fs-write', '--allow-child-process', '--allow-worker', + '--allow-env', ]; ArrayPrototypeForEach(availablePermissionFlags, (flag) => { if (getOptionValue(flag)) { diff --git a/lib/internal/readline/interface.js b/lib/internal/readline/interface.js index df08875cc79ae6..7ffd8db04a2c96 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -31,6 +31,11 @@ const { SymbolAsyncIterator, SafeStringIterator, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { codes } = require('internal/errors'); @@ -383,7 +388,7 @@ class Interface extends InterfaceConstructor { */ prompt(preserveCursor) { if (this.paused) this.resume(); - if (this.terminal && process.env.TERM !== 'dumb') { + if (this.terminal && process[env_private_symbol].TERM !== 'dumb') { if (!preserveCursor) this.cursor = 0; this[kRefreshLine](); } else { diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js index f3697aa9b68ae2..4160742abb6d6b 100644 --- a/lib/internal/repl/utils.js +++ b/lib/internal/repl/utils.js @@ -20,6 +20,11 @@ const { StringPrototypeTrim, Symbol, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { tokTypes: tt, Parser: AcornParser } = require('internal/deps/acorn/acorn/dist/acorn'); @@ -142,7 +147,7 @@ function isRecoverableError(e, code) { function setupPreview(repl, contextSymbol, bufferSymbol, active) { // Simple terminals can't handle previews. - if (process.env.TERM === 'dumb' || !active) { + if (process[env_private_symbol].TERM === 'dumb' || !active) { return { showPreview() {}, clearPreview() {} }; } @@ -512,7 +517,7 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { function setupReverseSearch(repl) { // Simple terminals can't use reverse search. - if (process.env.TERM === 'dumb') { + if (process[env_private_symbol].TERM === 'dumb') { return { reverseSearch() { return false; } }; } diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index ee1f9c404e8b6f..92af8df02bb08f 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -9,6 +9,11 @@ const { SafeMap, StringPrototypeSplit, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); // See https://sourcemaps.info/spec.html for SourceMap V3 specification. const { Buffer } = require('buffer'); @@ -107,7 +112,7 @@ function extractSourceMapURLMagicComment(content) { function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource, sourceURL, sourceMapURL) { const sourceMapsEnabled = getSourceMapsEnabled(); - if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return; + if (!(process[env_private_symbol].NODE_V8_COVERAGE || sourceMapsEnabled)) return; try { const { normalizeReferrerURL } = require('internal/modules/helpers'); filename = normalizeReferrerURL(filename); @@ -172,7 +177,7 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo function maybeCacheGeneratedSourceMap(content) { const sourceMapsEnabled = getSourceMapsEnabled(); - if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return; + if (!(process[env_private_symbol].NODE_V8_COVERAGE || sourceMapsEnabled)) return; const sourceURL = extractSourceURLMagicComment(content); if (sourceURL === null) { diff --git a/lib/internal/test_runner/coverage.js b/lib/internal/test_runner/coverage.js index 5a098f71af633b..74bf2f5c9ecd79 100644 --- a/lib/internal/test_runner/coverage.js +++ b/lib/internal/test_runner/coverage.js @@ -14,6 +14,11 @@ const { StringPrototypeLocaleCompare, StringPrototypeStartsWith, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { copyFileSync, mkdirSync, @@ -242,11 +247,11 @@ class TestCoverage { // Restore the original value of process.env.NODE_V8_COVERAGE. Then, copy // all of the created coverage files to the original coverage directory. if (this.originalCoverageDirectory === undefined) { - delete process.env.NODE_V8_COVERAGE; + delete process[env_private_symbol].NODE_V8_COVERAGE; return; } - process.env.NODE_V8_COVERAGE = this.originalCoverageDirectory; + process[env_private_symbol].NODE_V8_COVERAGE = this.originalCoverageDirectory; let dir; try { @@ -275,7 +280,7 @@ function sortCoverageFiles(a, b) { } function setupCoverage() { - let originalCoverageDirectory = process.env.NODE_V8_COVERAGE; + let originalCoverageDirectory = process[env_private_symbol].NODE_V8_COVERAGE; const cwd = process.cwd(); if (originalCoverageDirectory) { @@ -296,7 +301,7 @@ function setupCoverage() { // Ensure that NODE_V8_COVERAGE is set so that coverage can propagate to // child processes. - process.env.NODE_V8_COVERAGE = coverageDirectory; + process[env_private_symbol].NODE_V8_COVERAGE = coverageDirectory; return new TestCoverage(coverageDirectory, originalCoverageDirectory, cwd); } diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 056955f04566c3..7e2d975610da59 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -27,6 +27,11 @@ const { TypedArrayPrototypeGetLength, TypedArrayPrototypeSubarray, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { spawn } = require('child_process'); const { readdirSync, statSync } = require('fs'); @@ -328,7 +333,7 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) { const subtest = root.createSubtest(FileTest, path, async (t) => { const args = getRunArgs({ path, inspectPort, testNamePatterns }); const stdio = ['pipe', 'pipe', 'pipe']; - const env = { ...process.env, NODE_TEST_CONTEXT: 'child-v8' }; + const env = { ...process[env_private_symbol], NODE_TEST_CONTEXT: 'child-v8' }; if (filesWatcher) { stdio.push('ipc'); env.WATCH_REPORT_DEPENDENCIES = '1'; diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js index de9d4e5a1e4345..70bf9248cc0b56 100644 --- a/lib/internal/test_runner/utils.js +++ b/lib/internal/test_runner/utils.js @@ -10,6 +10,11 @@ const { RegExpPrototypeExec, SafeMap, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { basename, relative } = require('path'); const { createWriteStream } = require('fs'); @@ -170,8 +175,8 @@ function parseCommandLine() { const isTestRunner = getOptionValue('--test'); const coverage = getOptionValue('--experimental-test-coverage'); - const isChildProcess = process.env.NODE_TEST_CONTEXT === 'child'; - const isChildProcessV8 = process.env.NODE_TEST_CONTEXT === 'child-v8'; + const isChildProcess = process[env_private_symbol].NODE_TEST_CONTEXT === 'child'; + const isChildProcessV8 = process[env_private_symbol].NODE_TEST_CONTEXT === 'child-v8'; let destinations; let reporters; let testNamePatterns; diff --git a/lib/internal/tty.js b/lib/internal/tty.js index 4d3c768f65b6cd..0ccb5f18a8e872 100644 --- a/lib/internal/tty.js +++ b/lib/internal/tty.js @@ -30,6 +30,11 @@ const { } = primordials; const { validateInteger } = require('internal/validators'); +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); let OSRelease; @@ -103,7 +108,7 @@ function warnOnDeactivatedColors(env) { // The `getColorDepth` API got inspired by multiple sources such as // https://github.com/chalk/supports-color, // https://github.com/isaacs/color-support. -function getColorDepth(env = process.env) { +function getColorDepth(env = process[env_private_symbol]) { // Use level 0-3 to support the same levels as `chalk` does. This is done for // consistency throughout the ecosystem. if (env.FORCE_COLOR !== undefined) { diff --git a/lib/internal/util/colors.js b/lib/internal/util/colors.js index 9e1ed05d88130e..77c745b29d1fe8 100644 --- a/lib/internal/util/colors.js +++ b/lib/internal/util/colors.js @@ -1,5 +1,11 @@ 'use strict'; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); + let internalTTy; function lazyInternalTTY() { internalTTy ??= require('internal/tty'); @@ -15,7 +21,7 @@ module.exports = { clear: '', hasColors: false, shouldColorize(stream) { - if (process.env.FORCE_COLOR !== undefined) { + if (process[env_private_symbol].FORCE_COLOR !== undefined) { return lazyInternalTTY().getColorDepth() > 2; } return stream?.isTTY && ( diff --git a/lib/internal/util/inspector.js b/lib/internal/util/inspector.js index 0d9580c83224e4..092f3b7802e379 100644 --- a/lib/internal/util/inspector.js +++ b/lib/internal/util/inspector.js @@ -10,6 +10,11 @@ const { RegExpPrototypeExec, SafeWeakMap, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { validatePort } = require('internal/validators'); @@ -23,7 +28,7 @@ function isUsingInspector(execArgv = process.execArgv) { if (!_isUsingInspector.has(execArgv)) { _isUsingInspector.set(execArgv, ArrayPrototypeSome(execArgv, (arg) => RegExpPrototypeExec(kInspectArgRegex, arg) !== null) || - RegExpPrototypeExec(kInspectArgRegex, process.env.NODE_OPTIONS) !== null); + RegExpPrototypeExec(kInspectArgRegex, process[env_private_symbol].NODE_OPTIONS) !== null); } return _isUsingInspector.get(execArgv); } diff --git a/lib/internal/worker.js b/lib/internal/worker.js index eda9dbcdeb5c04..be5cb8872bb40f 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -23,6 +23,11 @@ const { Uint32Array, globalThis: { Atomics, SharedArrayBuffer }, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const EventEmitter = require('events'); const assert = require('internal/assert'); @@ -192,7 +197,7 @@ class Worker extends EventEmitter { ({ 0: key, 1: value }) => { env[key] = `${value}`; }, ); } else if (options.env == null) { - env = process.env; + env = process[env_private_symbol]; } else if (options.env !== SHARE_ENV) { throw new ERR_INVALID_ARG_TYPE( 'options.env', @@ -209,7 +214,7 @@ class Worker extends EventEmitter { debug('instantiating Worker.', `url: ${url}`, `doEval: ${doEval}`); // Set up the C++ handle for the worker, as well as some internal wiring. this[kHandle] = new WorkerImpl(url, - env === process.env ? null : env, + env === process[env_private_symbol] ? null : env, options.execArgv, parseResourceLimits(options.resourceLimits), !!(options.trackUnmanagedFds ?? true), diff --git a/lib/net.js b/lib/net.js index 0c12ff72aa3c50..8c842cf94edbaf 100644 --- a/lib/net.js +++ b/lib/net.js @@ -59,7 +59,12 @@ const { } = internalBinding('uv'); const { Buffer } = require('buffer'); -const { guessHandleType } = internalBinding('util'); +const { + guessHandleType, + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { ShutdownWrap } = internalBinding('stream_wrap'); const { TCP, @@ -1744,7 +1749,7 @@ function createServerHandle(address, port, addressType, fd, flags) { } else if (port === -1 && addressType === -1) { handle = new Pipe(PipeConstants.SERVER); if (isWindows) { - const instances = NumberParseInt(process.env.NODE_PENDING_PIPE_INSTANCES); + const instances = NumberParseInt(process[env_private_symbol].NODE_PENDING_PIPE_INSTANCES); if (!NumberIsNaN(instances)) { handle.setPendingInstances(instances); } @@ -2296,8 +2301,8 @@ if (isWindows) { } if (simultaneousAccepts === undefined) { - simultaneousAccepts = (process.env.NODE_MANY_ACCEPTS && - process.env.NODE_MANY_ACCEPTS !== '0'); + simultaneousAccepts = (process[env_private_symbol].NODE_MANY_ACCEPTS && + process[env_private_symbol].NODE_MANY_ACCEPTS !== '0'); } if (handle._simultaneousAccepts !== simultaneousAccepts) { diff --git a/lib/os.js b/lib/os.js index 34391697b5891c..d0e5e9512be1f4 100644 --- a/lib/os.js +++ b/lib/os.js @@ -30,6 +30,11 @@ const { StringPrototypeSlice, SymbolToPrimitive, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { safeGetenv } = internalBinding('credentials'); const constants = internalBinding('constants').os; @@ -181,9 +186,9 @@ platform[SymbolToPrimitive] = () => process.platform; function tmpdir() { let path; if (isWindows) { - path = process.env.TEMP || - process.env.TMP || - (process.env.SystemRoot || process.env.windir) + '\\temp'; + path = process[env_private_symbol].TEMP || + process[env_private_symbol].TMP || + (process[env_private_symbol].SystemRoot || process[env_private_symbol].windir) + '\\temp'; if (path.length > 1 && StringPrototypeEndsWith(path, '\\') && !StringPrototypeEndsWith(path, ':\\')) path = StringPrototypeSlice(path, 0, -1); diff --git a/lib/path.js b/lib/path.js index 1a2b3e38eca03f..9bdf514a251fe3 100644 --- a/lib/path.js +++ b/lib/path.js @@ -30,6 +30,11 @@ const { StringPrototypeSlice, StringPrototypeToLowerCase, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { CHAR_UPPERCASE_A, @@ -182,7 +187,7 @@ const win32 = { // absolute path, get cwd for that drive, or the process cwd if // the drive cwd is not available. We're sure the device is not // a UNC path at this points, because UNC paths are always absolute. - path = process.env[`=${resolvedDevice}`] || process.cwd(); + path = process[env_private_symbol][`=${resolvedDevice}`] || process.cwd(); // Verify that a cwd was found and that it actually points // to our drive. If not, default to the drive's root. diff --git a/lib/readline.js b/lib/readline.js index b9c6f17c52b4b0..64f81a1707c04f 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -31,6 +31,11 @@ const { PromiseReject, StringPrototypeSlice, } = primordials; +const { + privateSymbols: { + env_private_symbol, + }, +} = internalBinding('util'); const { clearLine, @@ -113,7 +118,7 @@ function Interface(input, output, completer, terminal) { FunctionPrototypeCall(InterfaceConstructor, this, input, output, completer, terminal); - if (process.env.TERM === 'dumb') { + if (process[env_private_symbol].TERM === 'dumb') { this._ttyWrite = FunctionPrototypeBind(_ttyWriteDumb, this); } } diff --git a/lib/repl.js b/lib/repl.js index df038901d7834a..bebbf1b00f4025 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -171,6 +171,9 @@ const { SKIP_SYMBOLS, }, getOwnNonIndexProperties, + privateSymbols: { + env_private_symbol, + }, } = internalBinding('util'); const { startSigintWatchdog, @@ -271,7 +274,9 @@ function REPLServer(prompt, if (options.terminal && options.useColors === undefined) { // If possible, check if stdout supports colors or not. - options.useColors = shouldColorize(options.output) || process.env.NODE_DISABLE_COLORS === undefined; + options.useColors = + shouldColorize(options.output) || + process[env_private_symbol].NODE_DISABLE_COLORS === undefined; } // TODO(devsnek): Add a test case for custom eval functions. diff --git a/node.gyp b/node.gyp index 4562d23c728dc6..f2a56a0375f188 100644 --- a/node.gyp +++ b/node.gyp @@ -138,6 +138,7 @@ 'src/node_worker.cc', 'src/node_zlib.cc', 'src/permission/child_process_permission.cc', + 'src/permission/env_permission.cc', 'src/permission/fs_permission.cc', 'src/permission/permission.cc', 'src/permission/worker_permission.cc', @@ -257,6 +258,7 @@ 'src/node_watchdog.h', 'src/node_worker.h', 'src/permission/child_process_permission.h', + 'src/permission/env_permission.h', 'src/permission/fs_permission.h', 'src/permission/permission.h', 'src/permission/worker_permission.h', diff --git a/src/env.cc b/src/env.cc index ff8d3952cf20a3..c3b635a7521bb8 100644 --- a/src/env.cc +++ b/src/env.cc @@ -813,6 +813,11 @@ Environment::Environment(IsolateData* isolate_data, permission()->Apply(options_->allow_fs_write, permission::PermissionScope::kFileSystemWrite); } + + if (options_->allow_env || !options_->allow_env_name.empty()) { + permission()->Apply(options_->allow_env_name, + permission::PermissionScope::kEnvironment); + } } } diff --git a/src/env_properties.h b/src/env_properties.h index 3ef589397dbdcc..25d12f72ee53b4 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -25,7 +25,8 @@ V(napi_wrapper, "node:napi:wrapper") \ V(untransferable_object_private_symbol, "node:untransferableObject") \ V(exit_info_private_symbol, "node:exit_info_private_symbol") \ - V(require_private_symbol, "node:require_private_symbol") + V(require_private_symbol, "node:require_private_symbol") \ + V(env_private_symbol, "node:env_private_symbol") // Symbols are per-isolate primitives but Environment proxies them // for the sake of convenience. @@ -342,6 +343,7 @@ V(compiled_fn_entry_template, v8::ObjectTemplate) \ V(crypto_key_object_handle_constructor, v8::FunctionTemplate) \ V(env_proxy_template, v8::ObjectTemplate) \ + V(env_privileged_proxy_template, v8::ObjectTemplate) \ V(env_proxy_ctor_template, v8::FunctionTemplate) \ V(dir_instance_template, v8::ObjectTemplate) \ V(fd_constructor_template, v8::ObjectTemplate) \ diff --git a/src/node_env_var.cc b/src/node_env_var.cc index 94936ea1c2bd22..c129e3a7f10612 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -4,10 +4,12 @@ #include "node_external_reference.h" #include "node_i18n.h" #include "node_process-inl.h" +#include "permission/permission.h" #include // tzset(), _tzset() namespace node { +using permission::PermissionScope; using v8::Array; using v8::Boolean; using v8::Context; @@ -336,6 +338,12 @@ Maybe KVStore::AssignToObject(v8::Isolate* isolate, return Just(true); } +template +inline static bool HasEnvAccess(const PropertyCallbackInfo& info) { + if (!Environment::GetCurrent(info)->permission()->is_enabled()) return true; + return info.Data()->IsBoolean(); +} + static void EnvGetter(Local property, const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); @@ -344,8 +352,18 @@ static void EnvGetter(Local property, return info.GetReturnValue().SetUndefined(); } CHECK(property->IsString()); - MaybeLocal value_string = - env->env_vars()->Get(env->isolate(), property.As()); + + Isolate* isolate = env->isolate(); + Local key = property.As(); + + if (UNLIKELY(!HasEnvAccess(info))) { + THROW_IF_INSUFFICIENT_PERMISSIONS(env, + PermissionScope::kEnvironment, + Utf8Value(isolate, key).ToStringView(), + info.GetReturnValue().SetUndefined()); + } + + MaybeLocal value_string = env->env_vars()->Get(isolate, key); if (!value_string.IsEmpty()) { info.GetReturnValue().Set(value_string.ToLocalChecked()); } @@ -380,7 +398,15 @@ static void EnvSetter(Local property, return; } - env->env_vars()->Set(env->isolate(), key, value_string); + Isolate* isolate = env->isolate(); + + if (UNLIKELY(!HasEnvAccess(info))) { + THROW_IF_INSUFFICIENT_PERMISSIONS(env, + PermissionScope::kEnvironment, + Utf8Value(isolate, key).ToStringView()); + } + + env->env_vars()->Set(isolate, key, value_string); // Whether it worked or not, always return value. info.GetReturnValue().Set(value); @@ -391,7 +417,15 @@ static void EnvQuery(Local property, Environment* env = Environment::GetCurrent(info); CHECK(env->has_run_bootstrapping_code()); if (property->IsString()) { - int32_t rc = env->env_vars()->Query(env->isolate(), property.As()); + Isolate* isolate = env->isolate(); + Local key = property.As(); + + if (UNLIKELY(!HasEnvAccess(info))) { + THROW_IF_INSUFFICIENT_PERMISSIONS(env, + PermissionScope::kEnvironment, + Utf8Value(isolate, key).ToStringView()); + } + int32_t rc = env->env_vars()->Query(isolate, key); if (rc != -1) info.GetReturnValue().Set(rc); } } @@ -401,7 +435,15 @@ static void EnvDeleter(Local property, Environment* env = Environment::GetCurrent(info); CHECK(env->has_run_bootstrapping_code()); if (property->IsString()) { - env->env_vars()->Delete(env->isolate(), property.As()); + Isolate* isolate = env->isolate(); + Local key = property.As(); + + if (UNLIKELY(!HasEnvAccess(info))) { + THROW_IF_INSUFFICIENT_PERMISSIONS(env, + PermissionScope::kEnvironment, + Utf8Value(isolate, key).ToStringView()); + } + env->env_vars()->Delete(isolate, key); } // process.env never has non-configurable properties, so always @@ -413,8 +455,26 @@ static void EnvEnumerator(const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); CHECK(env->has_run_bootstrapping_code()); - info.GetReturnValue().Set( - env->env_vars()->Enumerate(env->isolate())); + Isolate* isolate = env->isolate(); + Local context = isolate->GetCurrentContext(); + Local keys = env->env_vars()->Enumerate(isolate); + + if (UNLIKELY(!HasEnvAccess(info) && !keys.IsEmpty())) { + uint32_t keys_length = keys->Length(); + for (uint32_t i = 0; i < keys_length; i++) { + Local key = keys->Get(context, i).ToLocalChecked(); + CHECK(key->IsString()); + // We may want to remove access denied keys from the array instead of + // throwing an error here. In this case, users won't know exactly if the + // keys doesn't exist, or if they don't have access to it. + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + PermissionScope::kEnvironment, + Utf8Value(isolate, key.As()).ToStringView()); + } + } + + info.GetReturnValue().Set(keys); } static void EnvDefiner(Local property, @@ -475,6 +535,21 @@ void CreateEnvProxyTemplate(Isolate* isolate, IsolateData* isolate_data) { PropertyHandlerFlags::kHasNoSideEffect)); isolate_data->set_env_proxy_template(env_proxy_template); isolate_data->set_env_proxy_ctor_template(env_proxy_ctor_template); + + Local env_privileged_proxy_template = + ObjectTemplate::New(isolate); + env_privileged_proxy_template->SetHandler(NamedPropertyHandlerConfiguration( + EnvGetter, + EnvSetter, + EnvQuery, + EnvDeleter, + EnvEnumerator, + EnvDefiner, + nullptr, + True(isolate), + PropertyHandlerFlags::kHasNoSideEffect)); + isolate_data->set_env_privileged_proxy_template( + env_privileged_proxy_template); } void RegisterEnvVarExternalReferences(ExternalReferenceRegistry* registry) { diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 19b393bb308db6..ca4a9b1a9a5b82 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -8,10 +8,12 @@ #include "node_errors.h" #include "node_external_reference.h" #include "node_process-inl.h" +#include "permission/permission.h" #include "util-inl.h" using node::contextify::ContextifyContext; using node::errors::TryCatchScope; +using node::permission::PermissionScope; using v8::Array; using v8::ArrayBuffer; using v8::BackingStore; @@ -328,14 +330,33 @@ class SerializerDelegate : public ValueSerializer::Delegate { if (!env_proxy_ctor_template.IsEmpty() && env_proxy_ctor_template->HasInstance(object)) { HandleScope scope(isolate); + Local context = env_->context(); + // We may want to remove access denied keys from the object newly created + // below instead of throwing an error here. In this case, users won't know + // exactly if the keys doesn't exist, or if they don't have access to it. + if (UNLIKELY(env_->permission()->is_enabled())) { + Local keys = env_->env_vars()->Enumerate(isolate); + if (!keys.IsEmpty()) { + uint32_t keys_length = keys->Length(); + for (uint32_t i = 0; i < keys_length; i++) { + Local key = keys->Get(context, i).ToLocalChecked(); + CHECK(key->IsString()); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env_, + PermissionScope::kEnvironment, + Utf8Value(isolate, key.As()).ToStringView(), + Nothing()); + } + } + } // TODO(bnoordhuis) Prototype-less object in case process.env contains // a "__proto__" key? process.env has a prototype with concomitant // methods like toString(). It's probably confusing if that gets lost // in transmission. Local normal_object = Object::New(isolate); - env_->env_vars()->AssignToObject(isolate, env_->context(), normal_object); + env_->env_vars()->AssignToObject(isolate, context, normal_object); serializer->WriteUint32(kNormalObject); // Instead of a BaseObject. - return serializer->WriteValue(env_->context(), normal_object); + return serializer->WriteValue(context, normal_object); } ThrowDataCloneError(env_->clone_unsupported_type_str()); diff --git a/src/node_options.cc b/src/node_options.cc index 4e3633e5b5b8a6..8e546a52a5d040 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -416,6 +416,16 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { &EnvironmentOptions::experimental_policy_integrity, kAllowedInEnvvar); Implies("--policy-integrity", "[has_policy_integrity_string]"); + AddOption( + "--allow-env-name", + "allow permissions to access specified keys of the environment variables", + &EnvironmentOptions::allow_env_name, + kAllowedInEnvvar); + AddOption("--allow-env", + "allow permissions to access the environment variables", + &EnvironmentOptions::allow_env, + kAllowedInEnvvar); + AddAlias("--allow-env=", {"--allow-env-name", "--allow-env"}); AddOption("--allow-fs-read", "allow permissions to read the filesystem", &EnvironmentOptions::allow_fs_read, diff --git a/src/node_options.h b/src/node_options.h index e0d338f203b0c4..c6f10416bd408a 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -121,6 +121,8 @@ class EnvironmentOptions : public Options { std::string experimental_policy_integrity; bool has_policy_integrity_string = false; bool experimental_permission = false; + bool allow_env = false; + std::string allow_env_name; std::string allow_fs_read; std::string allow_fs_write; bool allow_child_process = false; diff --git a/src/node_realm.cc b/src/node_realm.cc index 80b2f9d2784611..c003789bf575b3 100644 --- a/src/node_realm.cc +++ b/src/node_realm.cc @@ -332,6 +332,7 @@ MaybeLocal PrincipalRealm::BootstrapRealm() { return MaybeLocal(); } + Local ctx = context(); Local env_string = FIXED_ONE_BYTE_STRING(isolate_, "env"); Local env_proxy; CreateEnvProxyTemplate(isolate_, env_->isolate_data()); @@ -340,6 +341,17 @@ MaybeLocal PrincipalRealm::BootstrapRealm() { return MaybeLocal(); } + Local env_privileged_proxy; + if (env_->env_privileged_proxy_template()->NewInstance(ctx).ToLocal( + &env_privileged_proxy)) { + // process[env_private_symbol] + if (process_object() + ->SetPrivate(ctx, env_->env_private_symbol(), env_privileged_proxy) + .IsNothing()) { + return MaybeLocal(); + } + } + return v8::True(isolate_); } diff --git a/src/permission/env_permission.cc b/src/permission/env_permission.cc new file mode 100644 index 00000000000000..b9d2ecab0417bb --- /dev/null +++ b/src/permission/env_permission.cc @@ -0,0 +1,39 @@ +#include "permission/env_permission.h" +#include "util-inl.h" + +namespace node { + +namespace permission { + +void EnvPermission::Apply(const std::string& allow, PermissionScope scope) { + if (scope == PermissionScope::kEnvironment) { + if (allow.empty()) { + allow_all_ = true; + return; + } + for (const auto& token : SplitString(allow, ',')) { +#ifdef _WIN32 + allowed_.insert(ToUpper(token)); +#else + allowed_.insert(token); +#endif + } + } +} + +bool EnvPermission::is_granted(PermissionScope scope, + const std::string_view& param) { + if (scope == PermissionScope::kEnvironment) { + const std::string token = std::string(param); +#ifdef _WIN32 + if (allow_all_ || allowed_.find(ToUpper(token)) != allowed_.end()) + return true; +#else + if (allow_all_ || allowed_.find(token) != allowed_.end()) return true; +#endif + } + return false; +} + +} // namespace permission +} // namespace node diff --git a/src/permission/env_permission.h b/src/permission/env_permission.h new file mode 100644 index 00000000000000..1bb1a8733dea3c --- /dev/null +++ b/src/permission/env_permission.h @@ -0,0 +1,30 @@ +#ifndef SRC_PERMISSION_ENV_PERMISSION_H_ +#define SRC_PERMISSION_ENV_PERMISSION_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include +#include +#include "permission/permission_base.h" + +namespace node { + +namespace permission { + +class EnvPermission final : public PermissionBase { + public: + void Apply(const std::string& allow, PermissionScope scope) override; + bool is_granted(PermissionScope scope, + const std::string_view& param = "") override; + + private: + bool allow_all_ = false; + std::unordered_set allowed_; +}; + +} // namespace permission + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#endif // SRC_PERMISSION_ENV_PERMISSION_H_ diff --git a/src/permission/permission.cc b/src/permission/permission.cc index ee2e83ff22d7d8..908df134548365 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -79,6 +79,8 @@ Permission::Permission() : enabled_(false) { std::make_shared(); std::shared_ptr worker_t = std::make_shared(); + std::shared_ptr env_permission = + std::make_shared(); #define V(Name, _, __) \ nodes_.insert(std::make_pair(PermissionScope::k##Name, fs)); FILESYSTEM_PERMISSIONS(V) @@ -91,6 +93,10 @@ Permission::Permission() : enabled_(false) { nodes_.insert(std::make_pair(PermissionScope::k##Name, worker_t)); WORKER_THREADS_PERMISSIONS(V) #undef V +#define V(Name, _, __) \ + nodes_.insert(std::make_pair(PermissionScope::k##Name, env_permission)); + ENVIRONMENT_PERMISSIONS(V) +#undef V } void Permission::ThrowAccessDenied(Environment* env, diff --git a/src/permission/permission.h b/src/permission/permission.h index 61341ab29134f2..dd9dac5a83d65c 100644 --- a/src/permission/permission.h +++ b/src/permission/permission.h @@ -6,6 +6,7 @@ #include "debug_utils.h" #include "node_options.h" #include "permission/child_process_permission.h" +#include "permission/env_permission.h" #include "permission/fs_permission.h" #include "permission/permission_base.h" #include "permission/worker_permission.h" @@ -39,6 +40,8 @@ class Permission { return is_scope_granted(permission, res); } + FORCE_INLINE bool is_enabled() { return enabled_; } + static PermissionScope StringToPermission(const std::string& perm); static const char* PermissionToString(PermissionScope perm); static void ThrowAccessDenied(Environment* env, diff --git a/src/permission/permission_base.h b/src/permission/permission_base.h index 86fefa06e65c57..cdb186f0030c1d 100644 --- a/src/permission/permission_base.h +++ b/src/permission/permission_base.h @@ -22,10 +22,13 @@ namespace permission { #define WORKER_THREADS_PERMISSIONS(V) \ V(WorkerThreads, "worker", PermissionsRoot) +#define ENVIRONMENT_PERMISSIONS(V) V(Environment, "env", PermissionsRoot) + #define PERMISSIONS(V) \ FILESYSTEM_PERMISSIONS(V) \ CHILD_PROCESS_PERMISSIONS(V) \ - WORKER_THREADS_PERMISSIONS(V) + WORKER_THREADS_PERMISSIONS(V) \ + ENVIRONMENT_PERMISSIONS(V) #define V(name, _, __) k##name, enum class PermissionScope { diff --git a/test/addons/no-addons/permission.js b/test/addons/no-addons/permission.js index 0fbcd2bb1ee782..a06fead1ff42e1 100644 --- a/test/addons/no-addons/permission.js +++ b/test/addons/no-addons/permission.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* +// Flags: --experimental-permission --allow-env --allow-fs-read=* 'use strict'; diff --git a/test/fixtures/errors/force_colors.snapshot b/test/fixtures/errors/force_colors.snapshot index a38745e396a8d9..8738e3391a9cb3 100644 --- a/test/fixtures/errors/force_colors.snapshot +++ b/test/fixtures/errors/force_colors.snapshot @@ -4,10 +4,10 @@ throw new Error('Should include grayed stack trace') Error: Should include grayed stack trace at Object. (/test*force_colors.js:1:7) - at Module._compile (node:internal*modules*cjs*loader:1255:14) - at Module._extensions..js (node:internal*modules*cjs*loader:1309:10) - at Module.load (node:internal*modules*cjs*loader:1113:32) - at Module._load (node:internal*modules*cjs*loader:960:12) + 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 diff --git a/test/parallel/test-permission-allow-child-process-cli.js b/test/parallel/test-permission-allow-child-process-cli.js index 6cffc19719350b..1d7ee731226d21 100644 --- a/test/parallel/test-permission-allow-child-process-cli.js +++ b/test/parallel/test-permission-allow-child-process-cli.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-child-process --allow-fs-read=* +// Flags: --experimental-permission --allow-env --allow-child-process --allow-fs-read=* 'use strict'; const common = require('../common'); diff --git a/test/parallel/test-permission-allow-worker-cli.js b/test/parallel/test-permission-allow-worker-cli.js index ae5a28fdae3597..5791ff7e0c9c9d 100644 --- a/test/parallel/test-permission-allow-worker-cli.js +++ b/test/parallel/test-permission-allow-worker-cli.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-worker --allow-fs-read=* +// Flags: --experimental-permission --allow-env --allow-worker --allow-fs-read=* 'use strict'; require('../common'); diff --git a/test/parallel/test-permission-child-process-cli.js b/test/parallel/test-permission-child-process-cli.js index 3ce473ab498e0e..3528d63948540b 100644 --- a/test/parallel/test-permission-child-process-cli.js +++ b/test/parallel/test-permission-child-process-cli.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* +// Flags: --experimental-permission --allow-env --allow-fs-read=* 'use strict'; const common = require('../common'); diff --git a/test/parallel/test-permission-env-enum-clone.js b/test/parallel/test-permission-env-enum-clone.js new file mode 100644 index 00000000000000..8e11376d0a6b68 --- /dev/null +++ b/test/parallel/test-permission-env-enum-clone.js @@ -0,0 +1,104 @@ +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-child-process +'use strict'; + +require('../common'); +const { spawnSync } = require('node:child_process'); +const { debuglog } = require('node:util'); +const { strictEqual } = require('node:assert'); +const { describe, it } = require('node:test'); + +const debug = debuglog('test'); + +function runTest(args, options = {}) { + const { status, stdout, stderr } = spawnSync( + process.execPath, + ['--experimental-permission', '--allow-fs-read=*', ...args], + options + ); + debug('status:', status); + if (status) debug('stdout:', stdout?.toString().split('\n')); + if (status) debug('stderr:', stderr?.toString().split('\n')); + return { status, stdout, stderr }; +} + +describe('permission: enumerate', () => { + const error = JSON.stringify({ + code: 'ERR_ACCESS_DENIED', + permission: 'Environment', + }); + + it('enumerate with --allow-env', () => { + const { status } = runTest([ + '--allow-env', + '-e', + ` + // doesNotThrow + Object.keys(process.env); + `, + ]); + strictEqual(status, 0); + }); + + it('enumerate with --allow-env=ALLOWED_ONLY', () => { + const { status } = runTest( + [ + '--allow-env=ALLOWED_ONLY', + '-e', + ` + const { throws } = require('node:assert'); + throws(() => { + Object.keys(process.env); + }, ${error}); + `, + ], + { + env: { + ...process.env, + ALLOWED_ONLY: 0, + }, + } + ); + strictEqual(status, 0); + }); +}); + +describe('permission: structuredClone', () => { + const error = JSON.stringify({ + code: 'ERR_ACCESS_DENIED', + permission: 'Environment', + }); + + it('structuredClone process.env with --allow-env', () => { + const { status } = runTest([ + '--allow-env', + '-e', + ` + // doesNotThrow + structuredClone(process.env); + `, + ]); + strictEqual(status, 0); + }); + + it('structuredClone process.env with --allow-env=ALLOWED_ONLY', () => { + const { status } = runTest( + [ + '--allow-env=ALLOWED_ONLY', + '-e', + ` + const { throws } = require('node:assert'); + throws(() => { + structuredClone(process.env); + }, ${error}); + `, + ], + { + env: { + ...process.env, + ALLOWED_ONLY: 0, + }, + } + ); + strictEqual(status, 0); + }); +}); diff --git a/test/parallel/test-permission-env-has.js b/test/parallel/test-permission-env-has.js new file mode 100644 index 00000000000000..71d97232d69c43 --- /dev/null +++ b/test/parallel/test-permission-env-has.js @@ -0,0 +1,117 @@ +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-child-process +'use strict'; + +require('../common'); +const { spawnSync } = require('node:child_process'); +const { debuglog } = require('node:util'); +const { strictEqual, deepStrictEqual } = require('node:assert'); +const { describe, it } = require('node:test'); + +const debug = debuglog('test'); + +function runTest(args, options = {}) { + const { status, stdout, stderr } = spawnSync( + process.execPath, + ['--experimental-permission', '--allow-fs-read=*', ...args], + options + ); + debug('status:', status); + if (status) debug('stdout:', stdout.toString().split('\n')); + if (status) debug('stderr:', stderr.toString().split('\n')); + return { status, stdout, stderr }; +} + +describe('permission: has "env" with the reference', () => { + const code = ` + const has = (...args) => console.log(process.permission.has(...args)); + has('env', 'HOME'); + has('env', 'PORT'); + has('env', 'DEBUG'); + `; + + for (const flag of ['--allow-env', '--allow-env-name']) { + it(`has: ${flag}=HOME`, () => { + const { status, stdout } = runTest([`${flag}=HOME`, '-e', code]); + strictEqual(status, 0); + deepStrictEqual(stdout.toString().split('\n').slice(0, -1), [ + 'true', + 'false', + 'false', + ]); + }); + + it(`has: ${flag}=HOME,PORT`, () => { + const { status, stdout } = runTest([`${flag}=HOME,PORT`, '-e', code]); + + strictEqual(status, 0); + deepStrictEqual(stdout.toString().split('\n').slice(0, -1), [ + 'true', + 'true', + 'false', + ]); + }); + } + + it('has: --allow-env', () => { + const { status, stdout } = runTest(['--allow-env', '-e', code]); + + strictEqual(status, 0); + deepStrictEqual(stdout.toString().split('\n').slice(0, -1), [ + 'true', + 'true', + 'true', + ]); + }); +}); + +describe('permission: has "env" with no reference', () => { + it('initial state', () => { + const { status } = runTest([ + '-e', + 'require("assert").strictEqual(process.permission.has("env"), false);', + ]); + strictEqual(status, 0); + }); + + it('has no reference: --allow-env', () => { + const { status } = runTest([ + '--allow-env', + '-e', + 'require("assert").strictEqual(process.permission.has("env"), true);', + ]); + strictEqual(status, 0); + }); + + for (const flag of ['--allow-env', '--allow-env-name']) { + it(`has no reference: ${flag}=HOME`, () => { + const { status } = runTest([ + `${flag}=HOME`, + '-e', + 'require("assert").strictEqual(process.permission.has("env"), false);', + ]); + strictEqual(status, 0); + }); + } +}); + +describe('permission: has "env" reference', () => { + for (const flag of ['--allow-env', '--allow-env-name']) { + it(`reference is case-sensitive: ${flag}=FoO`, () => { + const { status, stdout } = runTest([ + `${flag}=FoO`, + '-e', + ` + console.log(process.permission.has('env', 'FOO')); + console.log(process.permission.has('env', 'foo')); + console.log(process.permission.has('env', 'FoO')); + `, + ]); + strictEqual(status, 0); + deepStrictEqual(stdout.toString().split('\n').slice(0, -1), [ + 'false', + 'false', + 'true', + ]); + }); + } +}); diff --git a/test/parallel/test-permission-env-windows.js b/test/parallel/test-permission-env-windows.js new file mode 100644 index 00000000000000..0a22d1980b5204 --- /dev/null +++ b/test/parallel/test-permission-env-windows.js @@ -0,0 +1,61 @@ +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-child-process +'use strict'; + +const common = require('../common'); +const { spawnSync } = require('node:child_process'); +const { debuglog } = require('node:util'); +const { strictEqual } = require('node:assert'); +const { describe, it } = require('node:test'); + +if (!common.isWindows) { + common.skip('process.env on Windows test'); +} + +const debug = debuglog('test'); + +function runTest(args, options = {}) { + const { status, stdout, stderr } = spawnSync( + process.execPath, + ['--experimental-permission', '--allow-fs-read=*', ...args], + options + ); + debug('status:', status); + if (status) debug('stdout:', stdout?.toString().split('\n')); + if (status) debug('stderr:', stderr?.toString().split('\n')); + return { status, stdout, stderr }; +} + +describe('permission: reference', () => { + const error = JSON.stringify({ + code: 'ERR_ACCESS_DENIED', + permission: 'Environment', + }); + + it('reference on Windows is case-insensitive', () => { + const { status } = runTest([ + '--allow-env=FoO', + '-e', + ` + const { throws, strictEqual } = require('node:assert'); + + strictEqual(process.permission.has('env', 'FOO'), true); + strictEqual(process.permission.has('env', 'foo'), true); + strictEqual(process.permission.has('env', 'FoO'), true); + + // doesNotThrow + process.env.FOO; + + // doesNotThrow + process.env.foo; + + // doesNotThrow + process.env.FoO; + + throws(() => { + process.env.OTHERS; + }, ${error}); + `, + ]); + strictEqual(status, 0); + }); +}); diff --git a/test/parallel/test-permission-env-worker.js b/test/parallel/test-permission-env-worker.js new file mode 100644 index 00000000000000..db24f16faedc97 --- /dev/null +++ b/test/parallel/test-permission-env-worker.js @@ -0,0 +1,80 @@ +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-child-process +'use strict'; + +require('../common'); +const { spawnSync } = require('node:child_process'); +const { debuglog } = require('node:util'); +const { strictEqual } = require('node:assert'); +const { describe, it } = require('node:test'); + +const debug = debuglog('test'); + +function runTest(args, options = {}) { + const { status, stdout, stderr } = spawnSync( + process.execPath, + ['--experimental-permission', '--allow-fs-read=*', ...args], + options + ); + debug('status:', status); + if (status) debug('stdout:', stdout?.toString().split('\n')); + if (status) debug('stderr:', stderr?.toString().split('\n')); + return { status, stdout, stderr }; +} + +describe('permission: "env" access on worker thread', () => { + it('worker_threads', () => { + const { status } = runTest([ + '--allow-worker', + '-e', + ` + const assert = require('node:assert'); + const { Worker, SHARE_ENV } = require('node:worker_threads'); + const w = new Worker('process.env.UNDEFINED', { eval: true }); + w.on('error', () => {}); + w.on('exit', (code) => assert.strictEqual(code, 1)); + `, + ]); + strictEqual(status, 0); + }); + + it('worker_threads with SHARE_ENV', () => { + const { status } = runTest([ + '--allow-worker', + '-e', + ` + const assert = require('node:assert'); + const { Worker, SHARE_ENV } = require('node:worker_threads'); + const w = new Worker('process.env.UNDEFINED', { eval: true, env: SHARE_ENV }); + w.on('error', () => {}); + w.on('exit', (code) => assert.strictEqual(code, 1)); + `, + ]); + strictEqual(status, 0); + }); + + it('worker_threads with --allow-env=ALLOED_IN_MAIN_THREAD', () => { + const error = JSON.stringify({ + code: 'ERR_ACCESS_DENIED', + permission: 'Environment', + }); + + const { status } = runTest([ + '--allow-worker', + '--allow-env=ALLOED_IN_MAIN_THREAD', + '-e', + ` + const { throws, strictEqual } = require('node:assert'); + const { Worker } = require('node:worker_threads'); + const w = new Worker('process.env.DENIED_IN_MAIN_THREAD', { + eval: true, + env: { DENIED_IN_MAIN_THREAD: 1 }, + }); + w.on('exit', (code) => strictEqual(code, 0)); + throws(() => { + process.env['DENIED_IN_MAIN_THREAD'] + }, ${error}); + `, + ]); + strictEqual(status, 0); + }); +}); diff --git a/test/parallel/test-permission-env.js b/test/parallel/test-permission-env.js new file mode 100644 index 00000000000000..2dab41417345f9 --- /dev/null +++ b/test/parallel/test-permission-env.js @@ -0,0 +1,175 @@ +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-child-process +'use strict'; + +require('../common'); +const { spawnSync } = require('node:child_process'); +const { debuglog } = require('node:util'); +const { strictEqual } = require('node:assert'); +const { describe, it } = require('node:test'); + +const debug = debuglog('test'); + +function runTest(args, options = {}) { + const { status, stdout, stderr } = spawnSync( + process.execPath, + ['--experimental-permission', '--allow-fs-read=*', ...args], + options + ); + debug('status:', status); + if (status) debug('stdout:', stdout?.toString().split('\n')); + if (status) debug('stderr:', stderr?.toString().split('\n')); + return { status, stdout, stderr }; +} + +describe('permission: "env" access', () => { + const error = JSON.stringify({ + code: 'ERR_ACCESS_DENIED', + permission: 'Environment', + }); + + it('get', () => { + const { status } = runTest([ + '-e', + ` + const { throws } = require('node:assert'); + throws(() => { + process.env.UNDEFINED; + }, ${error});`, + ]); + strictEqual(status, 0); + }); + + it('set', () => { + const { status } = runTest([ + '-e', + ` + const { throws } = require('node:assert'); + throws(() => { + process.env.UNDEFINED = 0; + }, ${error});`, + ]); + strictEqual(status, 0); + }); + + it('query', () => { + const { status } = runTest([ + '-e', + ` + const { throws } = require('node:assert'); + throws(() => { + 'UNDEFINED' in process.env; + }, ${error});`, + ]); + strictEqual(status, 0); + }); + + it('delete', () => { + const { status } = runTest([ + '-e', + ` + const { throws } = require('node:assert'); + throws(() => { + delete process.env.UNDEFINED; + }, ${error});`, + ]); + strictEqual(status, 0); + }); + + it('enumerate', () => { + const { status } = runTest([ + '-e', + ` + const { throws } = require('node:assert'); + throws(() => { + Object.keys(process.env); + }, ${error});`, + ]); + strictEqual(status, 0); + }); + + it('structuredClone', () => { + const { status } = runTest([ + '-e', + ` + const { throws } = require('node:assert'); + throws(() => { + structuredClone(process.env); + }, ${error});`, + ]); + strictEqual(status, 0); + }); +}); + +describe('permission: "env" access with --allow-env', () => { + it('get', () => { + const { status } = runTest([ + '--allow-env', + '-e', + ` + // doesNotThrow + process.env.UNDEFINED; + `, + ]); + strictEqual(status, 0); + }); + + it('set', () => { + const { status } = runTest([ + '--allow-env', + '-e', + ` + // doesNotThrow + process.env.UNDEFINED = 0; + `, + ]); + strictEqual(status, 0); + }); + + it('query', () => { + const { status } = runTest([ + '--allow-env', + '-e', + ` + // doesNotThrow + 'UNDEFINED' in process.env; + `, + ]); + strictEqual(status, 0); + }); + + it('delete', () => { + const { status } = runTest([ + '--allow-env', + '-e', + ` + // doesNotThrow + delete process.env.UNDEFINED; + `, + ]); + strictEqual(status, 0); + }); + + it('enumerate', () => { + const { status } = runTest([ + '--allow-env', + '-e', + ` + // doesNotThrow + Object.keys(process.env); + `, + ]); + strictEqual(status, 0); + }); + + it('structuredClone', () => { + const { status } = runTest([ + '--allow-env', + '-e', + ` + // doesNotThrow + structuredClone(process.env); + `, + ]); + strictEqual(status, 0); + }); +}); diff --git a/test/parallel/test-permission-experimental.js b/test/parallel/test-permission-experimental.js index bec66e5a731a95..f3f9e8eaa5b551 100644 --- a/test/parallel/test-permission-experimental.js +++ b/test/parallel/test-permission-experimental.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* +// Flags: --experimental-permission --allow-env --allow-fs-read=* 'use strict'; const common = require('../common'); diff --git a/test/parallel/test-permission-fs-read.js b/test/parallel/test-permission-fs-read.js index 010a5932c4eae1..8a32e4aa696f00 100644 --- a/test/parallel/test-permission-fs-read.js +++ b/test/parallel/test-permission-fs-read.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-fs-write=* --allow-child-process 'use strict'; const common = require('../common'); @@ -28,7 +28,10 @@ const commonPath = path.join(__filename, '../../common'); const { status, stderr } = spawnSync( process.execPath, [ - '--experimental-permission', `--allow-fs-read=${file},${commonPathWildcard}`, file, + '--experimental-permission', + '--allow-env', + `--allow-fs-read=${file},${commonPathWildcard}`, + file, ], { env: { diff --git a/test/parallel/test-permission-fs-relative-path.js b/test/parallel/test-permission-fs-relative-path.js index 74b4095e86663a..1acd18782cd812 100644 --- a/test/parallel/test-permission-fs-relative-path.js +++ b/test/parallel/test-permission-fs-relative-path.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* --allow-child-process +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-child-process 'use strict'; const common = require('../common'); diff --git a/test/parallel/test-permission-fs-symlink-target-write.js b/test/parallel/test-permission-fs-symlink-target-write.js index f6a99fe06c9d1f..da2f6f4e105494 100644 --- a/test/parallel/test-permission-fs-symlink-target-write.js +++ b/test/parallel/test-permission-fs-symlink-target-write.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-fs-write=* --allow-child-process 'use strict'; const common = require('../common'); @@ -36,6 +36,7 @@ fs.writeFileSync(path.join(readWriteFolder, 'file'), 'NO evil file contents'); process.execPath, [ '--experimental-permission', + '--allow-env', `--allow-fs-read=${file},${commonPathWildcard},${readOnlyFolder},${readWriteFolder}`, `--allow-fs-write=${readWriteFolder},${writeOnlyFolder}`, file, diff --git a/test/parallel/test-permission-fs-symlink.js b/test/parallel/test-permission-fs-symlink.js index 96d4fdad4d2d4b..33a9eb0a9eb90b 100644 --- a/test/parallel/test-permission-fs-symlink.js +++ b/test/parallel/test-permission-fs-symlink.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-fs-write=* --allow-child-process 'use strict'; const common = require('../common'); @@ -37,6 +37,7 @@ const symlinkFromBlockedFile = path.join(tmpdir.path, 'example-symlink.md'); process.execPath, [ '--experimental-permission', + '--allow-env', `--allow-fs-read=${file},${commonPathWildcard},${symlinkFromBlockedFile}`, `--allow-fs-write=${symlinkFromBlockedFile}`, file, diff --git a/test/parallel/test-permission-fs-wildcard.js b/test/parallel/test-permission-fs-wildcard.js index eb8d4ca53ced04..4bdd8d44c54d86 100644 --- a/test/parallel/test-permission-fs-wildcard.js +++ b/test/parallel/test-permission-fs-wildcard.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* --allow-child-process +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-child-process 'use strict'; const common = require('../common'); @@ -85,6 +85,7 @@ if (common.isWindows) { process.execPath, [ '--experimental-permission', + '--allow-env', `--allow-fs-read=${file},${commonPathWildcard},${allowList.join(',')}`, file, ], diff --git a/test/parallel/test-permission-fs-windows-path.js b/test/parallel/test-permission-fs-windows-path.js index b64cef12b47c18..8074f088f24611 100644 --- a/test/parallel/test-permission-fs-windows-path.js +++ b/test/parallel/test-permission-fs-windows-path.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* --allow-child-process +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-child-process 'use strict'; const common = require('../common'); diff --git a/test/parallel/test-permission-fs-write.js b/test/parallel/test-permission-fs-write.js index 9f257df86f8672..4e52c37304ed12 100644 --- a/test/parallel/test-permission-fs-write.js +++ b/test/parallel/test-permission-fs-write.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* --allow-child-process +// Flags: --experimental-permission --allow-env --allow-fs-read=* --allow-child-process 'use strict'; const common = require('../common'); @@ -25,6 +25,7 @@ const file = fixtures.path('permission', 'fs-write.js'); process.execPath, [ '--experimental-permission', + '--allow-env', '--allow-fs-read=*', `--allow-fs-write=${regularFile},${commonPath}`, file, diff --git a/test/parallel/test-permission-has.js b/test/parallel/test-permission-has.js index f0fb582959f721..55c0f5971a211d 100644 --- a/test/parallel/test-permission-has.js +++ b/test/parallel/test-permission-has.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* +// Flags: --experimental-permission --allow-env --allow-fs-read=* 'use strict'; const common = require('../common'); diff --git a/test/parallel/test-permission-worker-threads-cli.js b/test/parallel/test-permission-worker-threads-cli.js index e817a7877226c1..8ee55bc666b669 100644 --- a/test/parallel/test-permission-worker-threads-cli.js +++ b/test/parallel/test-permission-worker-threads-cli.js @@ -1,4 +1,4 @@ -// Flags: --experimental-permission --allow-fs-read=* +// Flags: --experimental-permission --allow-env --allow-fs-read=* 'use strict'; const common = require('../common');