Fix preferLocal argument escaping edge-case on Windows

lib/arguments/command-file.js

@@ -3,7 +3,6 @@ import {Buffer} from 'node:buffer';
 import path from 'node:path';
 import process from 'node:process';
 import {whichCommandSync} from 'which-command';
-import pathKey from 'path-key';
 
 /*
 On Windows, `node:child_process` cannot natively run many kinds of files (`.cmd`, `.bat`, shebang scripts, ...): without a shell it resolves neither `PATHEXT` nor shebangs, and it does not escape arguments. We resolve the command to its full file path and escape its arguments ourselves, so those work without an explicit shell, just like on Unix, where the OS handles all of this for us.
@@ -30,8 +29,13 @@ const escapeWindowsCommand = parsed => {
 	// Resolve the file to an absolute path, following its shebang to the interpreter if any
 	const resolvedFile = resolveWithShebang(parsed);
 
-	// A directly executable file is spawned as-is, bypassing `cmd.exe` and its escaping
+	// A directly executable file is spawned by its resolved path, bypassing `cmd.exe` and its escaping
 	if (resolvedFile !== undefined && directlyExecutableRegExp.test(resolvedFile)) {
+		if (parsed.options.argv0 === undefined) {
+			parsed.options.argv0 = parsed.file;
+		}
+
+		parsed.file = resolvedFile;
 		return parsed;
 	}
 
@@ -45,7 +49,7 @@ const escapeWindowsCommand = parsed => {
 
 	const isDoubleEscape = resolvedFile !== undefined && batchFileRegExp.test(resolvedFile);
 	// POSIX separators must become Windows ones (`foo/bar` -> `foo\bar`), otherwise resolution always fails with ENOENT
-	const escapedFile = escapeMetaChars(path.normalize(parsed.file));
+	const escapedFile = escapeMetaChars(path.normalize(resolvedFile ?? parsed.file));
 	const escapedArguments = parsed.commandArguments.map(argument => escapeArgument(argument, isDoubleEscape));
 	const commandLine = `"${[escapedFile, ...escapedArguments].join(' ')}"`;
 
@@ -75,10 +79,47 @@ const resolveWithShebang = parsed => {
 // Search `PATH` for the command, resolving its Windows executable extension via `PATHEXT`
 const resolvePath = parsed => {
 	const environment = parsed.options.env || process.env;
-	return whichCommandSync(parsed.file, {
-		cwd: parsed.options.cwd,
-		path: environment[pathKey({env: environment})],
-	});
+	const cwd = parsed.options.cwd ?? process.cwd();
+	const environmentPathExt = getWindowsEnvironmentValue(environment, 'PATHEXT');
+	const commandExtension = path.extname(parsed.file);
+	// Commands with an explicit extension must be tried verbatim so shebang scripts work even when their extension is excluded from PATHEXT.
+	const pathExt = commandExtension === '' ? environmentPathExt : `${commandExtension}${path.delimiter}${environmentPathExt ?? ''}`;
+	if (hasWindowsPathSeparator(parsed.file)) {
+		return whichCommandSync(path.resolve(cwd, parsed.file), {cwd, pathExt});
+	}
+
+	const searchPath = getWindowsEnvironmentValue(environment, 'PATH') ?? getWindowsEnvironmentValue(process.env, 'PATH') ?? '';
+	const resolveOptions = {
+		cwd,
+		path: searchPath,
+		pathExt,
+	};
+
+	return shouldSearchCurrentDirectory(environment) ? whichCommandSync(parsed.file, resolveOptions) : resolvePathDirectories(parsed.file, resolveOptions);
+};
+
+const hasWindowsPathSeparator = file => file.includes('/') || file.includes('\\') || file.includes(':');
+
+const shouldSearchCurrentDirectory = environment => getWindowsEnvironmentValue(process.env, 'NODEFAULTCURRENTDIRECTORYINEXEPATH') === undefined && getWindowsEnvironmentValue(environment, 'NODEFAULTCURRENTDIRECTORYINEXEPATH') === undefined;
+
+const resolvePathDirectories = (file, {cwd, path: searchPath, pathExt}) => {
+	for (const directory of searchPath.split(path.delimiter)) {
+		const unquotedDirectory = directory.length > 1 && directory.startsWith('"') && directory.endsWith('"') ? directory.slice(1, -1) : directory;
+		if (unquotedDirectory === '') {
+			continue;
+		}
+
+		const resolvedFile = whichCommandSync(path.resolve(cwd, unquotedDirectory, file), {cwd, pathExt});
+		if (resolvedFile !== undefined) {
+			return resolvedFile;
+		}
+	}
+};
+
+// Node sorts Windows environment keys and uses the first case-insensitive match when spawning.
+const getWindowsEnvironmentValue = (environment, name) => {
+	const environmentKey = Object.keys(environment).sort().find(key => key.toUpperCase() === name);
+	return environmentKey === undefined ? undefined : environment[environmentKey];
 };
 
 const SHEBANG_BYTE_LENGTH = 150;

lib/arguments/options.js

@@ -14,30 +14,33 @@ import {normalizeCwd} from './cwd.js';
 import {normalizeFileUrl} from './file-url.js';
 import {normalizeFdSpecificOptions} from './specific.js';
 
+const cmdExeRegExp = /^cmd(?:\.exe)?$/i;
+
 // Normalize the options object, and sometimes also the file paths and arguments.
 // Applies default values, validate allowed options, normalize them.
 export const normalizeOptions = (filePath, rawArguments, rawOptions) => {
 	// Prevent prototype pollution by copying only own properties to a null-prototype object
 	const sanitizedOptions = {__proto__: null, ...rawOptions};
 	sanitizedOptions.cwd = normalizeCwd(sanitizedOptions.cwd);
 	const [processedFile, processedArguments, processedOptions] = handleNodeOption(filePath, rawArguments, sanitizedOptions);
+	const fdOptions = normalizeFdSpecificOptions(processedOptions);
+	const options = addDefaultOptions(fdOptions);
+	options.env = getEnv(options);
 
-	const {file, commandArguments, options: initialOptions} = parseCommandFile(processedFile, processedArguments, processedOptions);
+	const {file, commandArguments} = parseCommandFile(processedFile, processedArguments, options);
 
-	const fdOptions = normalizeFdSpecificOptions(initialOptions);
-	const options = addDefaultOptions(fdOptions);
 	validateTimeout(options);
 	validateEncoding(options);
 	validateIpcInputOption(options);
 	validateCancelSignal(options);
 	validateGracefulCancel(options);
 	options.shell = normalizeFileUrl(options.shell);
-	options.env = getEnv(options);
 	options.killSignal = normalizeKillSignal(options.killSignal);
 	options.forceKillAfterDelay = normalizeForceKillAfterDelay(options.forceKillAfterDelay);
 	options.lines = options.lines.map((lines, fdNumber) => lines && !BINARY_ENCODINGS.has(options.encoding) && options.buffer[fdNumber]);
 
-	if (process.platform === 'win32' && path.basename(file, '.exe') === 'cmd') {
+	// The file is now an absolute path resolved via `PATHEXT`, so its extension might be uppercase (`cmd.EXE`)
+	if (process.platform === 'win32' && cmdExeRegExp.test(path.basename(file))) {
 		// #116
 		commandArguments.unshift('/q');
 	}

package.json

@@ -58,7 +58,6 @@
 		"is-plain-obj": "^4.1.0",
 		"is-stream": "^4.0.1",
 		"npm-run-path": "^6.0.0",
-		"path-key": "^4.0.0",
 		"pretty-ms": "^9.3.0",
 		"signal-exit": "^4.1.0",
 		"strip-final-newline": "^4.0.0",
@@ -74,6 +73,7 @@
 		"is-running": "^2.1.0",
 		"log-process-errors": "^12.0.1",
 		"path-exists": "^5.0.0",
+		"path-key": "^4.0.0",
 		"tempfile": "^6.0.1",
 		"tsd": "^0.33.0",
 		"typescript": "^6.0.3",

test/arguments/command-resolution.js

@@ -1,4 +1,9 @@
-import {cp, mkdir} from 'node:fs/promises';
+import {
+	cp,
+	mkdir,
+	unlink,
+	writeFile,
+} from 'node:fs/promises';
 import path from 'node:path';
 import process from 'node:process';
 import test from 'ava';
@@ -166,12 +171,171 @@ test('Does not mutate arguments nor options', testNoMutation, {});
 test('Does not mutate arguments nor options with a shell', testNoMutation, {shell: true});
 
 if (isWindows) {
+	const nodeOnlyOptions = {
+		extendEnv: false,
+		env: {
+			Path: path.dirname(process.execPath),
+			PathExt: '.EXE',
+		},
+	};
+
 	// A bare command name without an extension is resolved using `PATHEXT`.
 	test('Resolves command extension using PATHEXT (sync)', t => {
 		const {stdout} = execaSync('hello');
 		t.is(stdout, 'Hello World');
 	});
 
+	test('Uses PATHEXT extension order for direct executables', async t => {
+		const binaryDirectory = path.join(FIXTURES_DIRECTORY, 'node_modules', '.bin');
+		await mkdir(binaryDirectory, {recursive: true});
+		const command = 'node-extension-order';
+		const executablePath = path.join(binaryDirectory, `${command}.exe`);
+		const comPath = path.join(binaryDirectory, `${command}.com`);
+		await Promise.all([
+			cp(process.execPath, executablePath),
+			cp(process.execPath, comPath),
+		]);
+		t.teardown(async () => {
+			await Promise.all([unlink(executablePath), unlink(comPath)]);
+		});
+		const options = {
+			preferLocal: true,
+			localDir: FIXTURES_DIRECTORY,
+			extendEnv: false,
+			env: {
+				Path: path.dirname(process.execPath),
+				PathExt: '.EXE;.COM',
+			},
+		};
+		const nodeExpression = 'JSON.stringify({executablePath: process.execPath, argv0: process.argv0})';
+		const {stdout} = await execa(command, ['--print', nodeExpression], options);
+		const {executablePath: actualExecutablePath, argv0} = JSON.parse(stdout);
+		t.is(actualExecutablePath.toLowerCase(), executablePath.toLowerCase());
+		t.is(argv0, command);
+
+		const {stdout: stdoutSync} = execaSync(command, ['--print', nodeExpression], options);
+		const {executablePath: actualExecutablePathSync, argv0: argv0Sync} = JSON.parse(stdoutSync);
+		t.is(actualExecutablePathSync.toLowerCase(), executablePath.toLowerCase());
+		t.is(argv0Sync, command);
+	});
+
+	test.serial('Respects NoDefaultCurrentDirectoryInExePath', async t => {
+		const binaryDirectory = path.join(FIXTURES_DIRECTORY, 'node_modules', '.bin');
+		await mkdir(binaryDirectory, {recursive: true});
+		const command = 'node-current-directory';
+		const currentDirectoryExecutable = path.join(FIXTURES_DIRECTORY, `${command}.exe`);
+		const pathExecutable = path.join(binaryDirectory, `${command}.exe`);
+		await Promise.all([
+			cp(process.execPath, currentDirectoryExecutable),
+			cp(process.execPath, pathExecutable),
+		]);
+
+		const environmentName = 'NoDefaultCurrentDirectoryInExePath';
+		const originalValue = process.env[environmentName];
+		delete process.env[environmentName];
+		t.teardown(async () => {
+			if (originalValue === undefined) {
+				delete process.env[environmentName];
+			} else {
+				process.env[environmentName] = originalValue;
+			}
+
+			await Promise.all([unlink(currentDirectoryExecutable), unlink(pathExecutable)]);
+		});
+
+		const options = {
+			cwd: FIXTURES_DIRECTORY,
+			extendEnv: false,
+			env: {
+				Path: binaryDirectory,
+				PathExt: '.EXE',
+			},
+		};
+		const {stdout: currentDirectoryStdout} = await execa(command, ['--print', 'process.execPath'], options);
+		t.is(currentDirectoryStdout.toLowerCase(), currentDirectoryExecutable.toLowerCase());
+
+		const {stdout: currentDirectoryStdoutSync} = execaSync(command, ['--print', 'process.execPath'], options);
+		t.is(currentDirectoryStdoutSync.toLowerCase(), currentDirectoryExecutable.toLowerCase());
+
+		process.env[environmentName] = '1';
+		const {stdout} = await execa(command, ['--print', 'process.execPath'], options);
+		t.is(stdout.toLowerCase(), pathExecutable.toLowerCase());
+
+		const {stdout: stdoutSync} = execaSync(command, ['--print', 'process.execPath'], options);
+		t.is(stdoutSync.toLowerCase(), pathExecutable.toLowerCase());
+	});
+
+	test.serial('Uses the resolved batch file when current-directory search is disabled', async t => {
+		const binaryDirectory = path.join(FIXTURES_DIRECTORY, 'node_modules', '.bin');
+		await mkdir(binaryDirectory, {recursive: true});
+		const command = 'batch-current-directory';
+		const currentDirectoryBatchFile = path.join(FIXTURES_DIRECTORY, `${command}.cmd`);
+		const pathBatchFile = path.join(binaryDirectory, `${command}.cmd`);
+		await Promise.all([
+			writeFile(currentDirectoryBatchFile, '@echo current directory'),
+			writeFile(pathBatchFile, '@echo PATH'),
+		]);
+
+		const environmentName = 'NoDefaultCurrentDirectoryInExePath';
+		const originalValue = process.env[environmentName];
+		delete process.env[environmentName];
+		t.teardown(async () => {
+			if (originalValue === undefined) {
+				delete process.env[environmentName];
+			} else {
+				process.env[environmentName] = originalValue;
+			}
+
+			await Promise.all([unlink(currentDirectoryBatchFile), unlink(pathBatchFile)]);
+		});
+
+		const options = {
+			cwd: FIXTURES_DIRECTORY,
+			extendEnv: false,
+			env: {
+				Path: binaryDirectory,
+				PathExt: '.CMD',
+				[environmentName]: '',
+			},
+		};
+		const {stdout} = await execa(command, options);
+		t.is(stdout, 'PATH');
+
+		const {stdout: stdoutSync} = execaSync(command, options);
+		t.is(stdoutSync, 'PATH');
+
+		delete options.env[environmentName];
+		process.env[environmentName] = '';
+		const {stdout: parentEnvironmentStdout} = await execa(command, options);
+		t.is(parentEnvironmentStdout, 'PATH');
+
+		const {stdout: parentEnvironmentStdoutSync} = execaSync(command, options);
+		t.is(parentEnvironmentStdoutSync, 'PATH');
+	});
+
+	test('Does not search PATH for drive-relative commands', async t => {
+		const binaryDirectory = path.join(FIXTURES_DIRECTORY, 'node_modules', '.bin');
+		await mkdir(binaryDirectory, {recursive: true});
+		const commandName = 'node-drive-relative';
+		const pathExecutable = path.join(binaryDirectory, `${commandName}.exe`);
+		await cp(process.execPath, pathExecutable);
+		t.teardown(async () => {
+			await unlink(pathExecutable);
+		});
+		const drive = path.parse(FIXTURES_DIRECTORY).root.slice(0, 2);
+		const command = `${drive}${commandName}`;
+		const options = {
+			cwd: FIXTURES_DIRECTORY,
+			extendEnv: false,
+			env: {
+				Path: binaryDirectory,
+				PathExt: '.EXE',
+			},
+		};
+		await t.throwsAsync(execa(command, [], options));
+		t.throws(() => execaSync(command, [], options));
+	});
+
 	// A `.cmd` file needs `cmd.exe`, so its forward-slash path must be normalized to
 	// backslashes, otherwise it fails with ENOENT.
 	test('Runs a .cmd file given as a relative POSIX-style subpath', async t => {
@@ -200,6 +364,51 @@ if (isWindows) {
 		const {stdout: stdoutSync} = execaSync(shimPath, [commandArgument]);
 		t.is(stdoutSync, commandArgument);
 	});
+
+	test('Double-escapes explicit batch files excluded from PATHEXT', async t => {
+		const commandArgument = '"& whoami &"';
+		const command = path.join(FIXTURES_DIRECTORY, 'echo-shim.cmd');
+		const {stdout} = await execa(command, [commandArgument], nodeOnlyOptions);
+		t.is(stdout, commandArgument);
+
+		const {stdout: stdoutSync} = execaSync(command, [commandArgument], nodeOnlyOptions);
+		t.is(stdoutSync, commandArgument);
+	});
+
+	test('Runs an explicit shebang script excluded from PATHEXT', async t => {
+		const command = path.join(FIXTURES_DIRECTORY, 'echo.js');
+		await testResolvesCommand(t, command, nodeOnlyOptions);
+	});
+
+	test.serial('Double-escapes metacharacters for preferLocal cmd-shims', async t => {
+		await setupCmdShim();
+		const commandArgument = 'a&whoami';
+		const originalPathExt = process.env.PATHEXT;
+		process.env.PATHEXT = '.EXE';
+		const options = {
+			preferLocal: true,
+			localDir: FIXTURES_DIRECTORY,
+			extendEnv: false,
+			env: {
+				Path: path.dirname(process.execPath),
+				PathExt: '.EXE;.CMD',
+			},
+		};
+
+		try {
+			const {stdout} = await execa('echo-cmd-shim', [commandArgument], options);
+			t.is(stdout, commandArgument);
+
+			const {stdout: stdoutSync} = execaSync('echo-cmd-shim', [commandArgument], options);
+			t.is(stdoutSync, commandArgument);
+		} finally {
+			if (originalPathExt === undefined) {
+				delete process.env.PATHEXT;
+			} else {
+				process.env.PATHEXT = originalPathExt;
+			}
+		}
+	});
 }
 
 // On Windows, `.cmd`/`.bat` files are run through `cmd.exe`, which interprets several

test/terminate/timeout.js

@@ -21,7 +21,8 @@ test('timeout kills the subprocess if it times out, in sync mode', async t => {
 	t.true(isTerminated);
 	t.is(signal, 'SIGTERM');
 	t.true(timedOut);
-	t.is(originalMessage, 'spawnSync node ETIMEDOUT');
+	// On Windows, the command is spawned using the absolute path resolved via `PATHEXT`, so it appears in Node.js' own error message
+	t.regex(originalMessage, /^spawnSync .*node(?:\.[A-Za-z]+)? ETIMEDOUT$/);
 	t.is(shortMessage, `Command timed out after 1 milliseconds: node forever.js\n${originalMessage}`);
 	t.is(message, shortMessage);
 });