From f6e1ac1124d45cebf5c4ca66e875d7bffeffed6e Mon Sep 17 00:00:00 2001
From: ehmicky <ehmicky@users.noreply.github.com>
Date: Sat, 30 Mar 2024 03:25:32 +0000
Subject: [PATCH] Improve validation of `execaSync()` options (#940)

---
 lib/stdio/sync.js    | 16 ++++++++++++++--
 lib/sync.js          | 16 ++++++++++++++--
 test/exit/cancel.js  |  9 ++++++++-
 test/exit/cleanup.js |  8 +++++++-
 test/stdio/handle.js | 11 +++++++++++
 5 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/lib/stdio/sync.js b/lib/stdio/sync.js
index b1f9372879..8d0118bf2b 100644
--- a/lib/stdio/sync.js
+++ b/lib/stdio/sync.js
@@ -11,7 +11,19 @@ export const handleInputSync = (options, verboseInfo) => {
 };
 
 const forbiddenIfSync = ({type, optionName}) => {
-	throw new TypeError(`The \`${optionName}\` option cannot be ${TYPE_TO_MESSAGE[type]} in sync mode.`);
+	throwInvalidSyncValue(optionName, TYPE_TO_MESSAGE[type]);
+};
+
+const forbiddenNativeIfSync = ({optionName, value}) => {
+	if (value === 'ipc') {
+		throwInvalidSyncValue(optionName, `"${value}"`);
+	}
+
+	return {};
+};
+
+const throwInvalidSyncValue = (optionName, value) => {
+	throw new TypeError(`The \`${optionName}\` option cannot be ${value} in sync mode.`);
 };
 
 const addProperties = {
@@ -21,7 +33,7 @@ const addProperties = {
 	webTransform: forbiddenIfSync,
 	duplex: forbiddenIfSync,
 	iterable: forbiddenIfSync,
-	native() {},
+	native: forbiddenNativeIfSync,
 };
 
 const addPropertiesSync = {
diff --git a/lib/sync.js b/lib/sync.js
index effec0e932..3903b3d0e6 100644
--- a/lib/sync.js
+++ b/lib/sync.js
@@ -29,10 +29,22 @@ const handleSyncArguments = (rawFile, rawArgs, rawOptions) => {
 
 const normalizeSyncOptions = options => options.node && !options.ipc ? {...options, ipc: false} : options;
 
-const validateSyncOptions = ({ipc}) => {
+const validateSyncOptions = ({ipc, detached, cancelSignal}) => {
 	if (ipc) {
-		throw new TypeError('The "ipc: true" option cannot be used with synchronous methods.');
+		throwInvalidSyncOption('ipc: true');
 	}
+
+	if (detached) {
+		throwInvalidSyncOption('detached: true');
+	}
+
+	if (cancelSignal) {
+		throwInvalidSyncOption('cancelSignal');
+	}
+};
+
+const throwInvalidSyncOption = value => {
+	throw new TypeError(`The "${value}" option cannot be used with synchronous methods.`);
 };
 
 const spawnSubprocessSync = ({file, args, options, command, escapedCommand, fileDescriptors, startTime}) => {
diff --git a/test/exit/cancel.js b/test/exit/cancel.js
index 8a745b9cac..0acf14c508 100644
--- a/test/exit/cancel.js
+++ b/test/exit/cancel.js
@@ -80,6 +80,13 @@ test('calling abort on a successfully completed subprocess does not make result.
 test('Throws when using the former "signal" option name', t => {
 	const abortController = new AbortController();
 	t.throws(() => {
-		execa('noop.js', {signal: abortController.signal});
+		execa('empty.js', {signal: abortController.signal});
 	}, {message: /renamed to "cancelSignal"/});
 });
+
+test('Cannot use cancelSignal, sync', t => {
+	const abortController = new AbortController();
+	t.throws(() => {
+		execaSync('empty.js', {cancelSignal: abortController.signal});
+	}, {message: /The "cancelSignal" option cannot be used/});
+});
diff --git a/test/exit/cleanup.js b/test/exit/cleanup.js
index f498e63112..dc4a31f4c8 100644
--- a/test/exit/cleanup.js
+++ b/test/exit/cleanup.js
@@ -3,7 +3,7 @@ import {setTimeout} from 'node:timers/promises';
 import test from 'ava';
 import {pEvent} from 'p-event';
 import isRunning from 'is-running';
-import {execa} from '../../index.js';
+import {execa, execaSync} from '../../index.js';
 import {setFixtureDir} from '../helpers/fixtures-dir.js';
 
 setFixtureDir();
@@ -87,3 +87,9 @@ test('detach subprocess', async t => {
 
 	process.kill(pid, 'SIGKILL');
 });
+
+test('Cannot use "detached" option, sync', t => {
+	t.throws(() => {
+		execaSync('empty.js', {detached: true});
+	}, {message: /The "detached: true" option cannot be used/});
+});
diff --git a/test/stdio/handle.js b/test/stdio/handle.js
index 38c4a2206b..f171e16d4a 100644
--- a/test/stdio/handle.js
+++ b/test/stdio/handle.js
@@ -63,6 +63,17 @@ test('stdio[*] can be ["inherit"]', testNoPipeOption, ['inherit'], 3);
 test('stdio[*] can be 3', testNoPipeOption, 3, 3);
 test('stdio[*] can be [3]', testNoPipeOption, [3], 3);
 
+const testNoIpcSync = (t, fdNumber) => {
+	t.throws(() => {
+		execaSync('empty.js', getStdio(fdNumber, 'ipc'));
+	}, {message: /cannot be "ipc" in sync mode/});
+};
+
+test('stdin cannot be "ipc", sync', testNoIpcSync, 0);
+test('stdout cannot be "ipc", sync', testNoIpcSync, 1);
+test('stderr cannot be "ipc", sync', testNoIpcSync, 2);
+test('stdio[*] cannot be "ipc", sync', testNoIpcSync, 3);
+
 const testInvalidArrayValue = (t, invalidStdio, fdNumber, execaMethod) => {
 	t.throws(() => {
 		execaMethod('empty.js', getStdio(fdNumber, ['pipe', invalidStdio]));