From 8eae6e642765133501905a7ddd8acdf76c8eb401 Mon Sep 17 00:00:00 2001 From: ehmicky Date: Sat, 14 Sep 2024 20:24:38 +0100 Subject: [PATCH] Simplify Windows-related code (#79) --- source/spawn.js | 8 ++------ source/windows.js | 14 +++++++------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/source/spawn.js b/source/spawn.js index 3772895..d31eedd 100644 --- a/source/spawn.js +++ b/source/spawn.js @@ -1,7 +1,7 @@ import {spawn} from 'node:child_process'; import {once} from 'node:events'; import process from 'node:process'; -import {getForcedShell, escapeArguments} from './windows.js'; +import {applyForceShell} from './windows.js'; import {getResultError} from './result.js'; export const spawnSubprocess = async (file, commandArguments, options, context) => { @@ -13,11 +13,7 @@ export const spawnSubprocess = async (file, commandArguments, options, context) ? [process.execPath, [...process.execArgv.filter(flag => !flag.startsWith('--inspect')), ...commandArguments]] : [file, commandArguments]; - const forcedShell = await getForcedShell(file, options); - const instance = spawn(...escapeArguments(file, commandArguments, forcedShell), { - ...options, - shell: options.shell || forcedShell, - }); + const instance = spawn(...await applyForceShell(file, commandArguments, options)); bufferOutput(instance.stdout, context, 'stdout'); bufferOutput(instance.stderr, context, 'stderr'); diff --git a/source/windows.js b/source/windows.js index eeb5a98..9c39d42 100644 --- a/source/windows.js +++ b/source/windows.js @@ -2,12 +2,18 @@ import {stat} from 'node:fs/promises'; import path from 'node:path'; import process from 'node:process'; +// When setting `shell: true` under-the-hood, we must manually escape the file and arguments. +// This ensures arguments are properly split, and prevents command injection. +export const applyForceShell = async (file, commandArguments, options) => await shouldForceShell(file, options) + ? [escapeFile(file), commandArguments.map(argument => escapeArgument(argument)), {...options, shell: true}] + : [file, commandArguments, options]; + // On Windows, running most executable files (except *.exe and *.com) requires using a shell. // This includes *.cmd and *.bat, which itself includes Node modules binaries. // We detect this situation and automatically: // - Set the `shell: true` option // - Escape shell-specific characters -export const getForcedShell = async (file, {shell, cwd, env = process.env}) => process.platform === 'win32' +const shouldForceShell = async (file, {shell, cwd, env = process.env}) => process.platform === 'win32' && !shell && !(await isExe(file, cwd, env)); @@ -60,12 +66,6 @@ const mIsExe = async (file, cwd, PATH) => { // Other file extensions require using a shell const exeExtensions = ['.exe', '.com']; -// When setting `shell: true` under-the-hood, we must manually escape the file and arguments. -// This ensures arguments are properly split, and prevents command injection. -export const escapeArguments = (file, commandArguments, forcedShell) => forcedShell - ? [escapeFile(file), commandArguments.map(argument => escapeArgument(argument))] - : [file, commandArguments]; - // `cmd.exe` escaping for arguments. // Taken from https://github.com/moxystudio/node-cross-spawn const escapeArgument = argument => escapeFile(escapeFile(`"${argument