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: #58853
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>

Author: RafaelGSS

doc/api/cli.md

@@ -154,6 +154,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
@@ -184,11 +190,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

@@ -190,7 +190,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
 * Child process

lib/child_process.js

@@ -95,6 +95,8 @@ const {
 
 constMAX_BUFFER=1024*1024;
 
+constpermission=require('internal/process/permission');
+
 constisZOS=process.platform==='os390';
 letaddAbortListener;
 
@@ -536,6 +538,31 @@ function copyProcessEnvToEnv(env, name, optionEnv) {
 }
 }
 
+letpermissionModelFlagsToCopy;
+
+functiongetPermissionModelFlagsToCopy(){
+if(permissionModelFlagsToCopy===undefined){
+permissionModelFlagsToCopy=[...permission.availableFlags(),'--permission'];
+}
+returnpermissionModelFlagsToCopy;
+}
+
+functioncopyPermissionModelFlagsToEnv(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;
+}
+
+constflagsToCopy=getPermissionModelFlagsToCopy();
+for(constargofprocess.execArgv){
+for(constflagofflagsToCopy){
+if(arg.startsWith(flag)){
+env[key]=`${env[key] ? env[key]+' '+arg : arg}`;
+}
+}
+}
+}
+
 functionnormalizeSpawnArguments(file,args,options){
 validateString(file,'file');
 validateArgumentNullCheck(file,'file');
@@ -643,7 +670,8 @@ function normalizeSpawnArguments(file, args, options) {
 ArrayPrototypeUnshift(args,file);
 }
 
-constenv=options.env||process.env;
+// Shallow copy to guarantee changes won't impact process.env
+constenv=options.env||{ ...process.env};
 constenvPairs=[];
 
 // process.env.NODE_V8_COVERAGE always propagates, making it possible to
@@ -663,6 +691,10 @@ function normalizeSpawnArguments(file, args, options) {
 copyProcessEnvToEnv(env,'_EDC_SUSV3',options.env);
 }
 
+if(permission.isEnabled()){
+copyPermissionModelFlagsToEnv(env,'NODE_OPTIONS',args);
+}
+
 letenvKeys=[];
 // Prototype values are intentionally included.
 for(constkeyinenv){

lib/internal/process/permission.js

@@ -33,4 +33,14 @@ module.exports = ObjectFreeze({
 
 returnpermission.has(scope,reference);
 },
+availableFlags(){
+return[
+'--allow-fs-read',
+'--allow-fs-write',
+'--allow-addons',
+'--allow-child-process',
+'--allow-wasi',
+'--allow-worker',
+];
+},
 });

lib/internal/process/pre_execution.js

@@ -666,15 +666,8 @@ function initializePermission() {
 },
 });
 }else{
-constavailablePermissionFlags=[
-'--allow-fs-read',
-'--allow-fs-write',
-'--allow-addons',
-'--allow-child-process',
-'--allow-wasi',
-'--allow-worker',
-];
-ArrayPrototypeForEach(availablePermissionFlags,(flag)=>{
+const{ availableFlags }=require('internal/process/permission');
+ArrayPrototypeForEach(availableFlags(),(flag)=>{
 constvalue=getOptionValue(flag);
 if(value.length){
 thrownewERR_MISSING_OPTION('--permission');

test/parallel/test-permission-allow-child-process-cli.js

@@ -13,6 +13,7 @@ const assert = require('assert');
 constchildProcess=require('child_process');
 constfs=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
 constchild=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';
+
+constcommon=require('../common');
+const{ isMainThread }=require('worker_threads');
+
+if(!isMainThread){
+common.skip('This test only works on a main thread');
+}
+
+constassert=require('assert');
+constchildProcess=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);
+}