Skip to content

Commit fe4e405

Browse files
mcollinaaduh95
authored andcommitted
child_process: fix permission model propagation via NODE_OPTIONS
The substring check env[key].indexOf(--permission) !== -1 in copyPermissionModelFlagsToEnv falsely treats unrelated NODE_OPTIONS values like --title=--permission as if the child already has an explicit Permission Model policy. This prevents flag propagation, causing the child to run without process.permission. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #63972 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 592544a commit fe4e405

2 files changed

Lines changed: 114 additions & 1 deletion

File tree

‎lib/child_process.js‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,9 +547,26 @@ function getPermissionModelFlagsToCopy() {
547547
returnpermissionModelFlagsToCopy;
548548
}
549549

550+
functionhasPermissionFlagInEnv(nodeOptions){
551+
// Parse NODE_OPTIONS into individual tokens and check if any token
552+
// is an actual --permission or --permission-audit flag. We use exact
553+
// token matching rather than substring matching to avoid false positives
554+
// when unrelated option values contain '--permission' (e.g.,
555+
// --title=--permission).
556+
if(!nodeOptions)returnfalse;
557+
consttokens=nodeOptions.split(/\s+/);
558+
returntokens.some((token)=>
559+
token==='--permission'||
560+
token.startsWith('--permission=')||
561+
token==='--permission-audit'||
562+
token.startsWith('--permission-audit='),
563+
);
564+
}
565+
550566
functioncopyPermissionModelFlagsToEnv(env,key,args){
551567
// Do not override if permission was already passed to file
552-
if(args.includes('--permission')||(env[key]&&env[key].indexOf('--permission')!==-1)){
568+
if(args.includes('--permission')||args.includes('--permission-audit')||
569+
hasPermissionFlagInEnv(env[key])){
553570
return;
554571
}
555572

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// Flags: --permission --allow-child-process --allow-fs-read=* --allow-worker
2+
// Tests that NODE_OPTIONS values containing '--permission' as a substring
3+
// in unrelated option values (e.g., --title=--permission) do NOT suppress
4+
// Permission Model flag propagation to child processes.
5+
'use strict';
6+
7+
constcommon=require('../common');
8+
const{ isMainThread }=require('worker_threads');
9+
10+
if(!isMainThread){
11+
common.skip('This test only works on a main thread');
12+
}
13+
if(process.config.variables.node_without_node_options){
14+
common.skip('missing NODE_OPTIONS support');
15+
}
16+
17+
constassert=require('assert');
18+
constchildProcess=require('child_process');
19+
20+
// Verify that the parent has Permission Model enabled
21+
assert.ok(process.permission.has('child'));
22+
assert.strictEqual(process.env.NODE_OPTIONS,undefined);
23+
24+
// Test cases: NODE_OPTIONS values that contain '--permission' as a substring
25+
// but are NOT actual permission flags. These should NOT suppress propagation.
26+
consttestCases=[
27+
{name: 'title',nodeOptions: '--title=--permission'},
28+
{name: 'conditions',nodeOptions: '--conditions=--permission'},
29+
{name: 'trace-event-categories',nodeOptions: '--trace-event-categories=--permission'},
30+
{name: 'title-audit',nodeOptions: '--title=--permission-audit'},
31+
];
32+
33+
for(const{ name, nodeOptions }oftestCases){
34+
// Spawn a child with the problematic NODE_OPTIONS value.
35+
// Without the fix, the substring check causes propagation to be skipped,
36+
// and the child will not have process.permission.
37+
const{ status, stdout }=childProcess.spawnSync(
38+
process.execPath,
39+
[
40+
'-e',
41+
`
42+
console.log(typeof process.permission);
43+
console.log(process.permission && process.permission.has("child"));
44+
console.log(process.env.NODE_OPTIONS);
45+
`,
46+
],
47+
{
48+
env: {
49+
...process.env,
50+
'NODE_OPTIONS': nodeOptions,
51+
}
52+
}
53+
);
54+
55+
assert.strictEqual(status,0,`child process for ${name} exited with status ${status}`);
56+
57+
const[permType,hasChild]=stdout.toString().split('\n');
58+
59+
// Verify the child has Permission Model enabled (the bug caused it to be absent)
60+
assert.strictEqual(permType,'object',`child ${name} should have process.permission object`);
61+
62+
// Verify the child inherited child permission
63+
assert.strictEqual(hasChild,'true',`child ${name} should have child permission`);
64+
}
65+
66+
// Also verify that a child with a real --permission flag in NODE_OPTIONS
67+
// still gets its own flags honored (regression test for existing behavior).
68+
{
69+
const{ status, stdout }=childProcess.spawnSync(
70+
process.execPath,
71+
[
72+
'-e',
73+
`
74+
console.log(process.permission.has("fs.write"));
75+
console.log(process.permission.has("fs.read"));
76+
console.log(process.permission.has("child"));
77+
`,
78+
],
79+
{
80+
env: {
81+
...process.env,
82+
'NODE_OPTIONS': '--permission --allow-fs-write=*',
83+
}
84+
}
85+
);
86+
87+
assert.strictEqual(status,0);
88+
const[fsWrite,fsRead,child]=stdout.toString().split('\n');
89+
assert.strictEqual(fsWrite,'true');
90+
assert.strictEqual(fsRead,'false');
91+
assert.strictEqual(child,'false');
92+
}
93+
94+
{
95+
assert.strictEqual(process.env.NODE_OPTIONS,undefined);
96+
}

0 commit comments

Comments
 (0)