diff --git a/doc/api/cli.md b/doc/api/cli.md index 989a8cce13aa46..ee8b2e5c7dfdae 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -153,6 +153,12 @@ Error: Cannot load native addon because loading addons is disabled. > Stability: 1.1 - Active development @@ -183,11 +189,15 @@ Error: Access to this API has been restricted } ``` -Unlike `child_process.spawn`, the `child_process.fork` API copies the execution -arguments from the parent process. This means that if you start Node.js with the -Permission Model enabled and include the `--allow-child-process` flag, calling -`child_process.fork()` will propagate all Permission Model flags to the child -process. +The `child_process.fork()` API inherits the execution arguments from the +parent process. This means that if Node.js is started with the Permission +Model enabled and the `--allow-child-process` flag is set, any child process +created using `child_process.fork()` will automatically receive all relevant +Permission Model flags. + +This behavior also applies to `child_process.spawn()`, but in that case, the +flags are propagated via the `NODE_OPTIONS` environment variable rather than +directly through the process arguments. ### `--allow-fs-read` diff --git a/doc/api/permissions.md b/doc/api/permissions.md index 40c3dcf737f269..966e37257a565e 100644 --- a/doc/api/permissions.md +++ b/doc/api/permissions.md @@ -195,7 +195,7 @@ easy to configure permissions as needed when using `npx`. There are constraints you need to know before using this system: -* The model does not inherit to a child node process or a worker thread. +* The model does not inherit to a worker thread. * When using the Permission Model the following features will be restricted: * Native modules * Network diff --git a/lib/child_process.js b/lib/child_process.js index 21616c69f877ec..baa0a56d1ecdc7 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -95,6 +95,8 @@ const { const MAX_BUFFER = 1024 * 1024; +const permission = require('internal/process/permission'); + const isZOS = process.platform === 'os390'; let addAbortListener; @@ -536,6 +538,31 @@ function copyProcessEnvToEnv(env, name, optionEnv) { } } +let permissionModelFlagsToCopy; + +function getPermissionModelFlagsToCopy() { + if (permissionModelFlagsToCopy === undefined) { + permissionModelFlagsToCopy = [...permission.availableFlags(), '--permission']; + } + return permissionModelFlagsToCopy; +} + +function copyPermissionModelFlagsToEnv(env, key, args) { + // Do not override if permission was already passed to file + if (args.includes('--permission') || (env[key] && env[key].indexOf('--permission') !== -1)) { + return; + } + + const flagsToCopy = getPermissionModelFlagsToCopy(); + for (const arg of process.execArgv) { + for (const flag of flagsToCopy) { + if (arg.startsWith(flag)) { + env[key] = `${env[key] ? env[key] + ' ' + arg : arg}`; + } + } + } +} + let emittedDEP0190Already = false; function normalizeSpawnArguments(file, args, options) { validateString(file, 'file'); @@ -652,7 +679,8 @@ function normalizeSpawnArguments(file, args, options) { ArrayPrototypeUnshift(args, file); } - const env = options.env || process.env; + // Shallow copy to guarantee changes won't impact process.env + const env = options.env || { ...process.env }; const envPairs = []; // process.env.NODE_V8_COVERAGE always propagates, making it possible to @@ -672,6 +700,10 @@ function normalizeSpawnArguments(file, args, options) { copyProcessEnvToEnv(env, '_EDC_SUSV3', options.env); } + if (permission.isEnabled()) { + copyPermissionModelFlagsToEnv(env, 'NODE_OPTIONS', args); + } + let envKeys = []; // Prototype values are intentionally included. for (const key in env) { diff --git a/lib/internal/process/permission.js b/lib/internal/process/permission.js index bfdfe29fe4739f..7c446d3a1e3c5e 100644 --- a/lib/internal/process/permission.js +++ b/lib/internal/process/permission.js @@ -33,4 +33,15 @@ module.exports = ObjectFreeze({ return permission.has(scope, reference); }, + availableFlags() { + return [ + '--allow-fs-read', + '--allow-fs-write', + '--allow-addons', + '--allow-child-process', + '--allow-net', + '--allow-wasi', + '--allow-worker', + ]; + }, }); diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index b01b539c8ee387..db8e79b7e1cbe1 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -620,16 +620,8 @@ function initializePermission() { }, }); } else { - const availablePermissionFlags = [ - '--allow-fs-read', - '--allow-fs-write', - '--allow-addons', - '--allow-child-process', - '--allow-net', - '--allow-wasi', - '--allow-worker', - ]; - ArrayPrototypeForEach(availablePermissionFlags, (flag) => { + const { availableFlags } = require('internal/process/permission'); + ArrayPrototypeForEach(availableFlags(), (flag) => { const value = getOptionValue(flag); if (value.length) { throw new ERR_MISSING_OPTION('--permission'); diff --git a/test/parallel/test-permission-allow-child-process-cli.js b/test/parallel/test-permission-allow-child-process-cli.js index cf7e79e208d389..f2ec02f02f46f5 100644 --- a/test/parallel/test-permission-allow-child-process-cli.js +++ b/test/parallel/test-permission-allow-child-process-cli.js @@ -13,6 +13,7 @@ const assert = require('assert'); const childProcess = require('child_process'); const fs = require('fs'); +// Child Process (and fork) should inherit permission model flags if (process.argv[2] === 'child') { assert.throws(() => { fs.writeFileSync(__filename, 'should not write'); @@ -34,7 +35,12 @@ if (process.argv[2] === 'child') { // doesNotThrow childProcess.spawnSync(process.execPath, ['--version']); childProcess.execSync(...common.escapePOSIXShell`"${process.execPath}" --version`); + childProcess.execFileSync(process.execPath, ['--version']); + + // Guarantee permission model flags are inherited const child = childProcess.fork(__filename, ['child']); child.on('close', common.mustCall()); - childProcess.execFileSync(process.execPath, ['--version']); + + const { status } = childProcess.spawnSync(process.execPath, [__filename, 'child']); + assert.strictEqual(status, 0); } diff --git a/test/parallel/test-permission-child-process-inherit-flags.js b/test/parallel/test-permission-child-process-inherit-flags.js new file mode 100644 index 00000000000000..fb1506bf274f43 --- /dev/null +++ b/test/parallel/test-permission-child-process-inherit-flags.js @@ -0,0 +1,102 @@ +// Flags: --permission --allow-child-process --allow-fs-read=* --allow-worker +'use strict'; + +const common = require('../common'); +const { isMainThread } = require('worker_threads'); + +if (!isMainThread) { + common.skip('This test only works on a main thread'); +} + +const assert = require('assert'); +const childProcess = require('child_process'); + +{ + assert.ok(process.permission.has('child')); +} + +{ + assert.strictEqual(process.env.NODE_OPTIONS, undefined); +} + +{ + const { status, stdout } = childProcess.spawnSync(process.execPath, + [ + '-e', + ` + console.log(process.permission.has("fs.write")); + console.log(process.permission.has("fs.read")); + console.log(process.permission.has("child")); + console.log(process.permission.has("net")); + console.log(process.permission.has("worker")); + `, + ] + ); + const [fsWrite, fsRead, child, net, worker] = stdout.toString().split('\n'); + assert.strictEqual(status, 0); + assert.strictEqual(fsWrite, 'false'); + assert.strictEqual(fsRead, 'true'); + assert.strictEqual(child, 'true'); + assert.strictEqual(net, 'false'); + assert.strictEqual(worker, 'true'); +} + +// It should not override when --permission is passed +{ + const { status, stdout } = childProcess.spawnSync( + process.execPath, + [ + '--permission', + '--allow-fs-write=*', + '-e', + ` + console.log(process.permission.has("fs.write")); + console.log(process.permission.has("fs.read")); + console.log(process.permission.has("child")); + console.log(process.permission.has("net")); + console.log(process.permission.has("worker")); + `, + ] + ); + const [fsWrite, fsRead, child, net, worker] = stdout.toString().split('\n'); + assert.strictEqual(status, 0); + assert.strictEqual(fsWrite, 'true'); + assert.strictEqual(fsRead, 'false'); + assert.strictEqual(child, 'false'); + assert.strictEqual(net, 'false'); + assert.strictEqual(worker, 'false'); +} + +// It should not override when NODE_OPTIONS with --permission is passed +{ + const { status, stdout } = childProcess.spawnSync( + process.execPath, + [ + '-e', + ` + console.log(process.permission.has("fs.write")); + console.log(process.permission.has("fs.read")); + console.log(process.permission.has("child")); + console.log(process.permission.has("net")); + console.log(process.permission.has("worker")); + `, + ], + { + env: { + ...process.env, + 'NODE_OPTIONS': '--permission --allow-fs-write=*', + } + } + ); + const [fsWrite, fsRead, child, net, worker] = stdout.toString().split('\n'); + assert.strictEqual(status, 0); + assert.strictEqual(fsWrite, 'true'); + assert.strictEqual(fsRead, 'false'); + assert.strictEqual(child, 'false'); + assert.strictEqual(net, 'false'); + assert.strictEqual(worker, 'false'); +} + +{ + assert.strictEqual(process.env.NODE_OPTIONS, undefined); +}