Skip to content

Commit b164038

Browse files
committed
permission: fix spawnSync permission check
Fixes: nodejs-private/node-private#394 Signed-off-by: RafaelGSS <[email protected]> PR-URL: #46975 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Richard Lau <[email protected]>
1 parent 0c4f8f2 commit b164038

8 files changed

+48
-5
lines changed

benchmark/fs/readfile-permission-enabled.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,12 @@ const bench = common.createBenchmark(main, {
1919
len: [1024, 16 * 1024 * 1024],
2020
concurrent: [1, 10],
2121
}, {
22-
flags: ['--experimental-permission', '--allow-fs-read=*', '--allow-fs-write=*'],
22+
flags: [
23+
'--experimental-permission',
24+
'--allow-fs-read=*',
25+
'--allow-fs-write=*',
26+
'--allow-child-process',
27+
],
2328
});
2429

2530
function main({ len, duration, concurrent, encoding }) {

src/spawn_sync.cc

+2
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,8 @@ void SyncProcessRunner::Initialize(Local<Object> target,
369369

370370
void SyncProcessRunner::Spawn(const FunctionCallbackInfo<Value>& args) {
371371
Environment* env = Environment::GetCurrent(args);
372+
THROW_IF_INSUFFICIENT_PERMISSIONS(
373+
env, permission::PermissionScope::kChildProcess, "");
372374
env->PrintSyncTrace();
373375
SyncProcessRunner p(env);
374376
Local<Value> result;

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

+18
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,24 @@ if (process.argv[2] === 'child') {
2424
code: 'ERR_ACCESS_DENIED',
2525
permission: 'ChildProcess',
2626
}));
27+
assert.throws(() => {
28+
childProcess.spawnSync(process.execPath, ['--version']);
29+
}, common.expectsError({
30+
code: 'ERR_ACCESS_DENIED',
31+
permission: 'ChildProcess',
32+
}));
2733
assert.throws(() => {
2834
childProcess.exec(process.execPath, ['--version']);
2935
}, common.expectsError({
3036
code: 'ERR_ACCESS_DENIED',
3137
permission: 'ChildProcess',
3238
}));
39+
assert.throws(() => {
40+
childProcess.execSync(process.execPath, ['--version']);
41+
}, common.expectsError({
42+
code: 'ERR_ACCESS_DENIED',
43+
permission: 'ChildProcess',
44+
}));
3345
assert.throws(() => {
3446
childProcess.fork(__filename, ['child']);
3547
}, common.expectsError({
@@ -42,4 +54,10 @@ if (process.argv[2] === 'child') {
4254
code: 'ERR_ACCESS_DENIED',
4355
permission: 'ChildProcess',
4456
}));
57+
assert.throws(() => {
58+
childProcess.execFileSync(process.execPath, ['--version']);
59+
}, common.expectsError({
60+
code: 'ERR_ACCESS_DENIED',
61+
permission: 'ChildProcess',
62+
}));
4563
}

test/parallel/test-permission-deny-child-process.js

+18
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,24 @@ if (process.argv[2] === 'child') {
3131
code: 'ERR_ACCESS_DENIED',
3232
permission: 'ChildProcess',
3333
}));
34+
assert.throws(() => {
35+
childProcess.spawnSync(process.execPath, ['--version']);
36+
}, common.expectsError({
37+
code: 'ERR_ACCESS_DENIED',
38+
permission: 'ChildProcess',
39+
}));
3440
assert.throws(() => {
3541
childProcess.exec(process.execPath, ['--version']);
3642
}, common.expectsError({
3743
code: 'ERR_ACCESS_DENIED',
3844
permission: 'ChildProcess',
3945
}));
46+
assert.throws(() => {
47+
childProcess.execSync(process.execPath, ['--version']);
48+
}, common.expectsError({
49+
code: 'ERR_ACCESS_DENIED',
50+
permission: 'ChildProcess',
51+
}));
4052
assert.throws(() => {
4153
childProcess.fork(__filename, ['child']);
4254
}, common.expectsError({
@@ -49,4 +61,10 @@ if (process.argv[2] === 'child') {
4961
code: 'ERR_ACCESS_DENIED',
5062
permission: 'ChildProcess',
5163
}));
64+
assert.throws(() => {
65+
childProcess.execFileSync(process.execPath, ['--version']);
66+
}, common.expectsError({
67+
code: 'ERR_ACCESS_DENIED',
68+
permission: 'ChildProcess',
69+
}));
5270
}

test/parallel/test-permission-deny-fs-symlink-target-write.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=*
1+
// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process
22
'use strict';
33

44
const common = require('../common');

test/parallel/test-permission-deny-fs-symlink.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=*
1+
// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process
22
'use strict';
33

44
const common = require('../common');

test/parallel/test-permission-fs-relative-path.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --experimental-permission --allow-fs-read=*
1+
// Flags: --experimental-permission --allow-fs-read=* --allow-child-process
22
'use strict';
33

44
const common = require('../common');

test/parallel/test-permission-fs-windows-path.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=*
1+
// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process
22
'use strict';
33

44
const common = require('../common');

0 commit comments

Comments
 (0)