permission: propagate permission model flags on spawn

Previously, only child_process.fork propagated the exec
arguments (execvArgs) to the child process.
This commit adds support for spawn and spawnSync to
propagate permission model flags — except when they are
already provided explicitly via arguments or through
NODE_OPTIONS.

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/58853
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>

doc/api/cli.md

@@ -153,6 +153,12 @@ Error: Cannot load native addon because loading addons is disabled.
 
 <!-- YAML
 added: v20.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/58853
+    description: When spawning process with the permission model enabled.
+                 The flags are inherit to the child Node.js process through
+                 NODE_OPTIONS environment variable.
 -->
 
 > 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`
 

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

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) {

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',
+    ];
+  },
 });

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');

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);
 }

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);
+}