From fc01bcb6f121ec06ba8ac761d9909f2fef9f30aa Mon Sep 17 00:00:00 2001 From: James Garbutt <43081j@users.noreply.github.com> Date: Thu, 3 Sep 2026 10:13:28 +0100 Subject: [PATCH] fix: escape all cmd/bat file args --- src/normalize.ts | 13 +++++++------ src/test/main_test.ts | 11 +++++++++++ src/test/normalize_test.ts | 17 +++++++++++++++++ test/fixtures/echo_args.cmd | 2 ++ 4 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 test/fixtures/echo_args.cmd diff --git a/src/normalize.ts b/src/normalize.ts index ab9a618..3ab6221 100644 --- a/src/normalize.ts +++ b/src/normalize.ts @@ -13,7 +13,7 @@ import {getPathFromEnv} from './env.js'; const metaCharsRegExp = /([()\][%!^"`<>&|;, *?])/g; const shebangRegExp = /^#!\s*(.+)/; const isWindowsExecutableRegExp = /\.(?:com|exe)$/i; -const isNodeModulesCmdRegExp = /node_modules[\\/]\.bin[\\/][^\\/]+\.cmd$/i; +const isBatchFileRegExp = /\.(?:cmd|bat)$/i; const isWindows = process.platform === 'win32'; const defaultPathExt = ['.EXE', '.CMD', '.BAT', '.COM']; const noPathExt = ['']; @@ -84,12 +84,13 @@ export function normalizeSpawnCommand( // We don't need a shell if the command filename is resolved and an executable if (file === null || !isWindowsExecutableRegExp.test(file)) { - // Need to double escape meta chars if the command is a cmd-shim located in `node_modules/.bin/` - // The cmd-shim simply calls execute the package bin file with NodeJS, proxying any argument - // Because the escape of metachars with ^ gets interpreted when the cmd.exe is first called, - // we need to double escape them + // Meta chars need double escaping if the command is a batch file. cmd.exe + // basically consumes the first layer of escaping. + // 1. `cmd.exe /c "command.cmd arg1 arg2"` + // 2. `command.cmd` sees `arg1` and `arg2` as `%1` and `%2` + // 3. `command.cmd` subs those in as actual commands const needsDoubleEscapeMetaChars = - file !== null && isNodeModulesCmdRegExp.test(file); + file !== null && isBatchFileRegExp.test(file); // Normalize posix paths into OS compatible paths (e.g.: foo/bar -> foo\bar) // This is necessary otherwise it will always fail with ENOENT in those cases diff --git a/src/test/main_test.ts b/src/test/main_test.ts index dbb5cba..ecc757c 100644 --- a/src/test/main_test.ts +++ b/src/test/main_test.ts @@ -225,6 +225,17 @@ if (isWindows) { ); expect(result.stdout).toBe(''); }); + + test('double escapes for batch files', async () => { + const result = await x(path.join(fixturesDir, 'echo_args.cmd'), [ + '"&echo INJECTED' + ]); + + // the batch file echoes `%*`, so the arg text itself is expected in + // stdout. a line of its own means `&` was parsed as a separator + expect(result.stdout).toContain('&echo INJECTED'); + expect(result.stdout.split(/\r?\n/)).not.toContain('INJECTED'); + }); }); describe('exec (windows) (async)', () => { diff --git a/src/test/normalize_test.ts b/src/test/normalize_test.ts index fdd12ca..d66513f 100644 --- a/src/test/normalize_test.ts +++ b/src/test/normalize_test.ts @@ -79,6 +79,23 @@ describe('normalizeSpawnCommand', () => { expect(normalized.args).toEqual([scriptPath]); }); + test('double escapes meta chars in args to a batch file', () => { + const cmdPath = path.join(fixturesPath, 'echo_args.cmd'); + const normalized = normalizeSpawnCommand(cmdPath, ['"&injected']); + + // the batch interpreter consumes a second layer of `^` escapes when it + // expands `%*`, so `&` must still be escaped at that point + expect(normalized.args[3]).toBe( + `"${cmdPath} ^^^"\\^^^"^^^&injected^^^""` + ); + }); + + test('single escapes meta chars in args to a non-batch command', () => { + const normalized = normalizeSpawnCommand('nonexistent', ['"&injected']); + + expect(normalized.args[3]).toBe('"nonexistent ^"\\^"^&injected^""'); + }); + test('handles relative commands without extension', () => { const relativePath = path.relative( cwd, diff --git a/test/fixtures/echo_args.cmd b/test/fixtures/echo_args.cmd new file mode 100644 index 0000000..4a4a371 --- /dev/null +++ b/test/fixtures/echo_args.cmd @@ -0,0 +1,2 @@ +@echo off +echo %*