diff --git a/src/utils/shell-executor.ts b/src/utils/shell-executor.ts index 9ee00511..a3206d6d 100644 --- a/src/utils/shell-executor.ts +++ b/src/utils/shell-executor.ts @@ -9,33 +9,32 @@ import { ErrorManager } from './error-manager'; import { getWebSearchHookEnv } from './websearch-manager'; /** - * Escape arguments for shell execution (Windows compatibility) - * Handles PowerShell special characters: backticks, $variables, double quotes + * Escape arguments for shell execution (cross-platform) + * + * IMPORTANT: On Windows, spawn({ shell: true }) uses cmd.exe by default, + * NOT PowerShell. cmd.exe does NOT recognize single quotes as string delimiters. + * We must use double quotes for cmd.exe compatibility. */ export function escapeShellArg(arg: string): string { const isWindows = process.platform === 'win32'; if (isWindows) { - // PowerShell: Use single quotes for literal strings to prevent variable expansion - // Escape single quotes by doubling them (PowerShell syntax) - // Fallback to double quotes with escapes if single quotes present - if (arg.includes("'")) { - // Contains single quote - use double quotes with escape sequences - return ( - '"' + - String(arg) - .replace(/\$/g, '`$') // Escape $ to prevent variable expansion - .replace(/`/g, '``') // Escape backticks - .replace(/"/g, '`"') + // Escape double quotes - '"' - ); - } else { - // No single quotes - use single quotes for literal string (safest) - return "'" + String(arg) + "'"; - } + // cmd.exe: Use double quotes, escape inner double quotes by doubling them + // cmd.exe interprets "" as escaped double quote inside quoted string + // Strip newlines/tabs that can break cmd.exe parsing + return ( + '"' + + String(arg) + .replace(/[\r\n\t]/g, ' ') // Replace newlines/tabs with space + .replace(/%/g, '%%') // Escape percent signs + .replace(/\^/g, '^^') // Escape carets + .replace(/!/g, '^^!') // Escape exclamation marks (delayed expansion) + .replace(/"/g, '""') + // Escape quotes + '"' + ); } else { // Unix/macOS: Double quotes with escaped inner quotes - return '"' + String(arg).replace(/"/g, '""') + '"'; + return '"' + String(arg).replace(/"/g, '\\"') + '"'; } } diff --git a/tests/unit/utils/shell-executor.test.ts b/tests/unit/utils/shell-executor.test.ts new file mode 100644 index 00000000..cca6f0cb --- /dev/null +++ b/tests/unit/utils/shell-executor.test.ts @@ -0,0 +1,84 @@ +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; + +// We need to mock process.platform for cross-platform testing +const originalPlatform = process.platform; + +describe('escapeShellArg', () => { + describe('Unix (non-Windows)', () => { + beforeEach(() => { + Object.defineProperty(process, 'platform', { value: 'linux' }); + }); + afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + }); + + it('wraps argument in double quotes', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('arg')).toBe('"arg"'); + }); + + it('escapes inner double quotes with backslash', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('say "hello"')).toBe('"say \\"hello\\""'); + }); + + it('handles paths with spaces', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('/path/to/my file')).toBe('"/path/to/my file"'); + }); + + it('handles empty string', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('')).toBe('""'); + }); + }); + + describe('Windows (cmd.exe)', () => { + beforeEach(() => { + Object.defineProperty(process, 'platform', { value: 'win32' }); + }); + afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + }); + + it('wraps argument in double quotes', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('arg')).toBe('"arg"'); + }); + + it('escapes inner double quotes by doubling them', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('say "hello"')).toBe('"say ""hello"""'); + }); + + it('escapes percent signs', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('%PATH%')).toBe('"%%PATH%%"'); + }); + + it('escapes caret characters', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('a^b')).toBe('"a^^b"'); + }); + + it('replaces newlines with spaces', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('line1\nline2')).toBe('"line1 line2"'); + }); + + it('replaces tabs with spaces', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('col1\tcol2')).toBe('"col1 col2"'); + }); + + it('handles Windows paths with spaces', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('C:\\Program Files\\App')).toBe('"C:\\Program Files\\App"'); + }); + + it('escapes exclamation marks for delayed expansion', async () => { + const { escapeShellArg } = await import('../../../src/utils/shell-executor'); + expect(escapeShellArg('hello!')).toBe('"hello^^!"'); + }); + }); +});