permission: fix spawnSync permission check

RafaelGSS · RafaelGSS · commit b164038c863d · 2023-03-07T13:07:20.000-03:00
Fixes: nodejs-private/node-private#394 Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: #46975 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Richard Lau <rlau@redhat.com>
diff --git a/benchmark/fs/readfile-permission-enabled.js b/benchmark/fs/readfile-permission-enabled.js
@@ -19,7 +19,12 @@ const bench = common.createBenchmark(main, {
   len: [1024, 16 * 1024 * 1024],
   concurrent: [1, 10],
 }, {
-  flags: ['--experimental-permission', '--allow-fs-read=*', '--allow-fs-write=*'],
+  flags: [
+    '--experimental-permission',
+    '--allow-fs-read=*',
+    '--allow-fs-write=*',
+    '--allow-child-process',
+  ],
 });
 
 function main({ len, duration, concurrent, encoding }) {
diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc
@@ -369,6 +369,8 @@ void SyncProcessRunner::Initialize(Local<Object> target,
 
 void SyncProcessRunner::Spawn(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kChildProcess, "");
   env->PrintSyncTrace();
   SyncProcessRunner p(env);
   Local<Value> result;
diff --git a/test/parallel/test-permission-deny-child-process-cli.js b/test/parallel/test-permission-deny-child-process-cli.js
@@ -24,12 +24,24 @@ if (process.argv[2] === 'child') {
     code: 'ERR_ACCESS_DENIED',
     permission: 'ChildProcess',
   }));
+  assert.throws(() => {
+    childProcess.spawnSync(process.execPath, ['--version']);
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'ChildProcess',
+  }));
   assert.throws(() => {
     childProcess.exec(process.execPath, ['--version']);
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'ChildProcess',
   }));
+  assert.throws(() => {
+    childProcess.execSync(process.execPath, ['--version']);
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'ChildProcess',
+  }));
   assert.throws(() => {
     childProcess.fork(__filename, ['child']);
   }, common.expectsError({
@@ -42,4 +54,10 @@ if (process.argv[2] === 'child') {
     code: 'ERR_ACCESS_DENIED',
     permission: 'ChildProcess',
   }));
+  assert.throws(() => {
+    childProcess.execFileSync(process.execPath, ['--version']);
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'ChildProcess',
+  }));
 }
diff --git a/test/parallel/test-permission-deny-child-process.js b/test/parallel/test-permission-deny-child-process.js
@@ -31,12 +31,24 @@ if (process.argv[2] === 'child') {
     code: 'ERR_ACCESS_DENIED',
     permission: 'ChildProcess',
   }));
+  assert.throws(() => {
+    childProcess.spawnSync(process.execPath, ['--version']);
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'ChildProcess',
+  }));
   assert.throws(() => {
     childProcess.exec(process.execPath, ['--version']);
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'ChildProcess',
   }));
+  assert.throws(() => {
+    childProcess.execSync(process.execPath, ['--version']);
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'ChildProcess',
+  }));
   assert.throws(() => {
     childProcess.fork(__filename, ['child']);
   }, common.expectsError({
@@ -49,4 +61,10 @@ if (process.argv[2] === 'child') {
     code: 'ERR_ACCESS_DENIED',
     permission: 'ChildProcess',
   }));
+  assert.throws(() => {
+    childProcess.execFileSync(process.execPath, ['--version']);
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'ChildProcess',
+  }));
 }
diff --git a/test/parallel/test-permission-deny-fs-symlink-target-write.js b/test/parallel/test-permission-deny-fs-symlink-target-write.js
@@ -1,4 +1,4 @@
-// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=*
+// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-permission-deny-fs-symlink.js b/test/parallel/test-permission-deny-fs-symlink.js
@@ -1,4 +1,4 @@
-// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=*
+// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-permission-fs-relative-path.js b/test/parallel/test-permission-fs-relative-path.js
@@ -1,4 +1,4 @@
-// Flags: --experimental-permission --allow-fs-read=*
+// Flags: --experimental-permission --allow-fs-read=* --allow-child-process
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-permission-fs-windows-path.js b/test/parallel/test-permission-fs-windows-path.js
@@ -1,4 +1,4 @@
-// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=*
+// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process
 'use strict';
 
 const common = require('../common');

Original file line number	Diff line number	Diff 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`
`2`	`2`	`'use strict';`
`3`	`3`
`4`	`4`	`const common = require('../common');`