diff --git a/src/__tests__/main/process-manager/utils/shellEscape.test.ts b/src/__tests__/main/process-manager/utils/shellEscape.test.ts new file mode 100644 index 00000000..7ae1c54e --- /dev/null +++ b/src/__tests__/main/process-manager/utils/shellEscape.test.ts @@ -0,0 +1,140 @@ +import { describe, it, expect } from 'vitest'; +import { + escapeCmdArg, + escapePowerShellArg, + escapeCmdArgs, + escapePowerShellArgs, + isPowerShellShell, + escapeArgsForShell, +} from '../../../../main/process-manager/utils/shellEscape'; + +describe('shellEscape', () => { + describe('escapeCmdArg', () => { + it('should not escape simple arguments', () => { + expect(escapeCmdArg('hello')).toBe('hello'); + expect(escapeCmdArg('world123')).toBe('world123'); + expect(escapeCmdArg('file.txt')).toBe('file.txt'); + }); + + it('should escape arguments with spaces', () => { + expect(escapeCmdArg('hello world')).toBe('"hello world"'); + expect(escapeCmdArg('path with spaces')).toBe('"path with spaces"'); + }); + + it('should escape arguments with special characters', () => { + expect(escapeCmdArg('foo&bar')).toBe('"foo&bar"'); + expect(escapeCmdArg('foo|bar')).toBe('"foo|bar"'); + expect(escapeCmdArg('foobar')).toBe('"foo>bar"'); + }); + + it('should escape double quotes by doubling them', () => { + expect(escapeCmdArg('say "hello"')).toBe('"say ""hello"""'); + }); + + it('should escape carets by doubling them', () => { + expect(escapeCmdArg('foo^bar')).toBe('"foo^^bar"'); + }); + + it('should escape arguments with newlines', () => { + expect(escapeCmdArg('line1\nline2')).toBe('"line1\nline2"'); + expect(escapeCmdArg('line1\r\nline2')).toBe('"line1\r\nline2"'); + }); + + it('should escape long arguments', () => { + const longArg = 'a'.repeat(150); + expect(escapeCmdArg(longArg)).toBe(`"${longArg}"`); + }); + }); + + describe('escapePowerShellArg', () => { + it('should not escape simple arguments', () => { + expect(escapePowerShellArg('hello')).toBe('hello'); + expect(escapePowerShellArg('world123')).toBe('world123'); + expect(escapePowerShellArg('file.txt')).toBe('file.txt'); + }); + + it('should escape arguments with spaces', () => { + expect(escapePowerShellArg('hello world')).toBe("'hello world'"); + }); + + it('should escape arguments with special characters', () => { + expect(escapePowerShellArg('foo&bar')).toBe("'foo&bar'"); + expect(escapePowerShellArg('foo$bar')).toBe("'foo$bar'"); + expect(escapePowerShellArg('foo`bar')).toBe("'foo`bar'"); + }); + + it('should escape single quotes by doubling them', () => { + expect(escapePowerShellArg("it's")).toBe("'it''s'"); + expect(escapePowerShellArg("say 'hello'")).toBe("'say ''hello'''"); + }); + + it('should escape arguments with PowerShell-specific characters', () => { + expect(escapePowerShellArg('foo@bar')).toBe("'foo@bar'"); + expect(escapePowerShellArg('foo{bar}')).toBe("'foo{bar}'"); + expect(escapePowerShellArg('foo[bar]')).toBe("'foo[bar]'"); + }); + + it('should escape long arguments', () => { + const longArg = 'a'.repeat(150); + expect(escapePowerShellArg(longArg)).toBe(`'${longArg}'`); + }); + }); + + describe('escapeCmdArgs', () => { + it('should escape multiple arguments', () => { + const result = escapeCmdArgs(['simple', 'with space', 'say "hi"']); + expect(result).toEqual(['simple', '"with space"', '"say ""hi"""']); + }); + }); + + describe('escapePowerShellArgs', () => { + it('should escape multiple arguments', () => { + const result = escapePowerShellArgs(['simple', 'with space', "it's"]); + expect(result).toEqual(['simple', "'with space'", "'it''s'"]); + }); + }); + + describe('isPowerShellShell', () => { + it('should detect Windows PowerShell', () => { + expect(isPowerShellShell('powershell.exe')).toBe(true); + expect(isPowerShellShell('C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe')).toBe( + true + ); + expect(isPowerShellShell('PowerShell.exe')).toBe(true); + }); + + it('should detect PowerShell Core (pwsh)', () => { + expect(isPowerShellShell('pwsh')).toBe(true); + expect(isPowerShellShell('pwsh.exe')).toBe(true); + expect(isPowerShellShell('C:\\Program Files\\PowerShell\\7\\pwsh.exe')).toBe(true); + }); + + it('should not detect cmd.exe as PowerShell', () => { + expect(isPowerShellShell('cmd.exe')).toBe(false); + expect(isPowerShellShell('C:\\Windows\\System32\\cmd.exe')).toBe(false); + }); + + it('should handle undefined and empty strings', () => { + expect(isPowerShellShell(undefined)).toBe(false); + expect(isPowerShellShell('')).toBe(false); + }); + }); + + describe('escapeArgsForShell', () => { + it('should use PowerShell escaping for PowerShell shells', () => { + const result = escapeArgsForShell(['with space', "it's"], 'powershell.exe'); + expect(result).toEqual(["'with space'", "'it''s'"]); + }); + + it('should use cmd.exe escaping for cmd.exe shells', () => { + const result = escapeArgsForShell(['with space', 'say "hi"'], 'cmd.exe'); + expect(result).toEqual(['"with space"', '"say ""hi"""']); + }); + + it('should default to cmd.exe escaping when no shell specified', () => { + const result = escapeArgsForShell(['with space', 'say "hi"']); + expect(result).toEqual(['"with space"', '"say ""hi"""']); + }); + }); +}); diff --git a/src/cli/services/storage.ts b/src/cli/services/storage.ts index 8e43bc54..739133b6 100644 --- a/src/cli/services/storage.ts +++ b/src/cli/services/storage.ts @@ -22,16 +22,16 @@ function getConfigDir(): string { const home = os.homedir(); if (platform === 'darwin') { - return path.posix.join(home, 'Library', 'Application Support', 'Maestro'); + return path.join(home, 'Library', 'Application Support', 'Maestro'); } else if (platform === 'win32') { - return path.posix.join( - process.env.APPDATA || path.posix.join(home, 'AppData', 'Roaming'), + return path.join( + process.env.APPDATA || path.join(home, 'AppData', 'Roaming'), 'Maestro' ); } else { // Linux and others - return path.posix.join( - process.env.XDG_CONFIG_HOME || path.posix.join(home, '.config'), + return path.join( + process.env.XDG_CONFIG_HOME || path.join(home, '.config'), 'Maestro' ); } @@ -42,7 +42,7 @@ function getConfigDir(): string { * Returns undefined if file doesn't exist */ function readStoreFile(filename: string): T | undefined { - const filePath = path.posix.join(getConfigDir(), filename); + const filePath = path.join(getConfigDir(), filename); try { const content = fs.readFileSync(filePath, 'utf-8'); @@ -101,7 +101,7 @@ export function readGroups(): Group[] { * Check if migration to per-session history format has been completed */ function hasMigrated(): boolean { - const markerPath = path.posix.join(getConfigDir(), 'history-migrated.json'); + const markerPath = path.join(getConfigDir(), 'history-migrated.json'); return fs.existsSync(markerPath); } @@ -109,7 +109,7 @@ function hasMigrated(): boolean { * Get the history directory path */ function getHistoryDir(): string { - return path.posix.join(getConfigDir(), 'history'); + return path.join(getConfigDir(), 'history'); } /** @@ -117,7 +117,7 @@ function getHistoryDir(): string { */ function getSessionHistoryPath(sessionId: string): string { const safeId = sanitizeSessionId(sessionId); - return path.posix.join(getHistoryDir(), `${safeId}.json`); + return path.join(getHistoryDir(), `${safeId}.json`); } /** diff --git a/src/main/process-manager/spawners/ChildProcessSpawner.ts b/src/main/process-manager/spawners/ChildProcessSpawner.ts index f5c857d5..c9550c8c 100644 --- a/src/main/process-manager/spawners/ChildProcessSpawner.ts +++ b/src/main/process-manager/spawners/ChildProcessSpawner.ts @@ -14,6 +14,7 @@ import { ExitHandler } from '../handlers/ExitHandler'; import { buildChildProcessEnv } from '../utils/envBuilder'; import { saveImageToTempFile } from '../utils/imageUtils'; import { buildStreamJsonMessage } from '../utils/streamJsonBuilder'; +import { escapeArgsForShell, isPowerShellShell } from '../utils/shellEscape'; /** * Handles spawning of child processes (non-PTY). @@ -178,46 +179,18 @@ export class ChildProcessSpawner { { command: spawnCommand } ); - // Check if we're using PowerShell (for SSH commands to avoid cmd.exe 8191 char limit) - const isPowerShell = - typeof config.shell === 'string' && config.shell.toLowerCase().includes('powershell'); + // Use the shell escape utility for proper argument escaping + const shellPath = typeof config.shell === 'string' ? config.shell : undefined; + spawnArgs = escapeArgsForShell(finalArgs, shellPath); - if (isPowerShell) { - // Escape arguments for PowerShell (supports longer command lines than cmd.exe) - spawnArgs = finalArgs.map((arg) => { - const needsQuoting = /[ &|<>^%!()"\n\r#?*`$]/.test(arg) || arg.length > 100; - if (needsQuoting) { - // PowerShell escaping: wrap in single quotes, escape single quotes by doubling - const escaped = arg.replace(/'/g, "''"); - return `'${escaped}'`; - } - return arg; - }); - logger.info('[ProcessManager] Escaped args for PowerShell', 'ProcessManager', { - originalArgsCount: finalArgs.length, - escapedArgsCount: spawnArgs.length, - escapedPromptArgLength: spawnArgs[spawnArgs.length - 1]?.length, - escapedPromptArgPreview: spawnArgs[spawnArgs.length - 1]?.substring(0, 200), - argsModified: finalArgs.some((arg, i) => arg !== spawnArgs[i]), - }); - } else { - // Escape arguments for cmd.exe when using shell - spawnArgs = finalArgs.map((arg) => { - const needsQuoting = /[ &|<>^%!()"\n\r#?*]/.test(arg) || arg.length > 100; - if (needsQuoting) { - const escaped = arg.replace(/"/g, '""').replace(/\^/g, '^^'); - return `"${escaped}"`; - } - return arg; - }); - logger.info('[ProcessManager] Escaped args for Windows shell', 'ProcessManager', { - originalArgsCount: finalArgs.length, - escapedArgsCount: spawnArgs.length, - escapedPromptArgLength: spawnArgs[spawnArgs.length - 1]?.length, - escapedPromptArgPreview: spawnArgs[spawnArgs.length - 1]?.substring(0, 200), - argsModified: finalArgs.some((arg, i) => arg !== spawnArgs[i]), - }); - } + const shellType = isPowerShellShell(shellPath) ? 'PowerShell' : 'cmd.exe'; + logger.info(`[ProcessManager] Escaped args for ${shellType}`, 'ProcessManager', { + originalArgsCount: finalArgs.length, + escapedArgsCount: spawnArgs.length, + escapedPromptArgLength: spawnArgs[spawnArgs.length - 1]?.length, + escapedPromptArgPreview: spawnArgs[spawnArgs.length - 1]?.substring(0, 200), + argsModified: finalArgs.some((arg, i) => arg !== spawnArgs[i]), + }); } // Determine shell option to pass to child_process.spawn. diff --git a/src/main/process-manager/utils/shellEscape.ts b/src/main/process-manager/utils/shellEscape.ts new file mode 100644 index 00000000..ba3f6214 --- /dev/null +++ b/src/main/process-manager/utils/shellEscape.ts @@ -0,0 +1,121 @@ +/** + * Shell argument escaping utilities for Windows cmd.exe and PowerShell. + * + * These functions handle escaping command line arguments to prevent injection + * and ensure proper argument passing when spawning processes via shell. + * + * References: + * - cmd.exe escaping: https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/cmd + * - PowerShell escaping: https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_quoting_rules + * + * For production use, consider using the 'shescape' npm package which provides + * more comprehensive cross-platform shell escaping with extensive testing. + */ + +/** + * Characters that require quoting in cmd.exe. + * Based on cmd.exe special characters: https://ss64.com/nt/syntax-esc.html + */ +const CMD_SPECIAL_CHARS = /[ &|<>^%!()"\n\r#?*]/; + +/** + * Characters that require quoting in PowerShell. + * Based on PowerShell special characters: https://ss64.com/ps/syntax-esc.html + */ +const POWERSHELL_SPECIAL_CHARS = /[ &|<>^%!()"\n\r#?*`$@{}[\]';,]/; + +/** + * Escape a single argument for use in cmd.exe. + * + * Strategy: + * 1. If the argument contains special characters or is long, wrap in double quotes + * 2. Escape existing double quotes by doubling them + * 3. Escape carets (^) as they're the escape character in cmd.exe + * + * @param arg - The argument to escape + * @returns The escaped argument safe for cmd.exe + */ +export function escapeCmdArg(arg: string): string { + // If no special characters and not too long, return as-is + if (!CMD_SPECIAL_CHARS.test(arg) && arg.length <= 100) { + return arg; + } + + // Escape double quotes by doubling them, and carets by doubling + const escaped = arg.replace(/"/g, '""').replace(/\^/g, '^^'); + + // Wrap in double quotes + return `"${escaped}"`; +} + +/** + * Escape a single argument for use in PowerShell. + * + * Strategy: + * 1. If the argument contains special characters or is long, wrap in single quotes + * 2. Escape existing single quotes by doubling them (PowerShell's single-quote escape) + * + * Single quotes in PowerShell treat the content as a literal string, which is + * safer than double quotes (which allow variable expansion). + * + * @param arg - The argument to escape + * @returns The escaped argument safe for PowerShell + */ +export function escapePowerShellArg(arg: string): string { + // If no special characters and not too long, return as-is + if (!POWERSHELL_SPECIAL_CHARS.test(arg) && arg.length <= 100) { + return arg; + } + + // Escape single quotes by doubling them (PowerShell's escape mechanism) + const escaped = arg.replace(/'/g, "''"); + + // Wrap in single quotes (prevents variable expansion) + return `'${escaped}'`; +} + +/** + * Escape an array of arguments for use in cmd.exe. + * + * @param args - The arguments to escape + * @returns The escaped arguments safe for cmd.exe + */ +export function escapeCmdArgs(args: string[]): string[] { + return args.map(escapeCmdArg); +} + +/** + * Escape an array of arguments for use in PowerShell. + * + * @param args - The arguments to escape + * @returns The escaped arguments safe for PowerShell + */ +export function escapePowerShellArgs(args: string[]): string[] { + return args.map(escapePowerShellArg); +} + +/** + * Detect if a shell path refers to PowerShell. + * + * @param shellPath - The shell path to check + * @returns True if the shell is PowerShell (either Windows PowerShell or PowerShell Core) + */ +export function isPowerShellShell(shellPath: string | undefined): boolean { + if (!shellPath) return false; + const lower = shellPath.toLowerCase(); + return lower.includes('powershell') || lower.includes('pwsh'); +} + +/** + * Escape arguments based on the target shell. + * + * @param args - The arguments to escape + * @param shell - The shell path or name (optional, defaults to cmd.exe behavior) + * @returns The escaped arguments + */ +export function escapeArgsForShell(args: string[], shell?: string): string[] { + if (isPowerShellShell(shell)) { + return escapePowerShellArgs(args); + } + return escapeCmdArgs(args); +} diff --git a/src/main/process-manager/utils/streamJsonBuilder.ts b/src/main/process-manager/utils/streamJsonBuilder.ts index 60c6e80b..57454d74 100644 --- a/src/main/process-manager/utils/streamJsonBuilder.ts +++ b/src/main/process-manager/utils/streamJsonBuilder.ts @@ -17,20 +17,19 @@ interface TextContent { type MessageContent = ImageContent | TextContent; /** - * Build a stream-json message for Claude Code with images and text + * Build a stream-json message for Claude Code with images and text. + * + * Claude Code expects this format: + * {"type":"user","message":{"role":"user","content":[...]}} + * + * Content array should have images first (Claude convention), then text. */ export function buildStreamJsonMessage(prompt: string, images: string[]): string { const content: MessageContent[] = []; - // Add text content first - content.push({ - type: 'text', - text: prompt, - }); - - // Add image content for each image - for (const imageDataUrl of images) { - const parsed = parseDataUrl(imageDataUrl); + // Add images first (Claude convention: images before text) + for (const dataUrl of images) { + const parsed = parseDataUrl(dataUrl); if (parsed) { content.push({ type: 'image', @@ -43,9 +42,18 @@ export function buildStreamJsonMessage(prompt: string, images: string[]): string } } + // Add text content + content.push({ + type: 'text', + text: prompt, + }); + const message = { - type: 'user_message', - content, + type: 'user', + message: { + role: 'user', + content, + }, }; return JSON.stringify(message); diff --git a/src/renderer/components/NewInstanceModal.tsx b/src/renderer/components/NewInstanceModal.tsx index 46d310e3..8173ccef 100644 --- a/src/renderer/components/NewInstanceModal.tsx +++ b/src/renderer/components/NewInstanceModal.tsx @@ -1136,7 +1136,10 @@ export function NewInstanceModal({ } onSshRemoteConfigChange={(config) => { setAgentSshRemoteConfigs((prev) => { - const newConfigs = { ...prev, _pending_: config }; + const newConfigs: Record = { + ...prev, + _pending_: config, + }; if (selectedAgent) { newConfigs[selectedAgent] = config; } diff --git a/src/renderer/hooks/useInlineWizard.ts b/src/renderer/hooks/useInlineWizard.ts index b189cba4..4de626c4 100644 --- a/src/renderer/hooks/useInlineWizard.ts +++ b/src/renderer/hooks/useInlineWizard.ts @@ -586,7 +586,7 @@ export function useInlineWizard(): UseInlineWizardReturn { // Step 4: Initialize conversation session (only for 'new' or 'iterate' modes) // Only allow wizard for agents that support structured output - const supportedWizardAgents: ToolType[] = ['claude', 'claude-code', 'codex']; + const supportedWizardAgents: ToolType[] = ['claude-code', 'codex']; if ( (mode === 'new' || mode === 'iterate') && agentType && @@ -699,7 +699,7 @@ export function useInlineWizard(): UseInlineWizardReturn { const sendMessage = useCallback( async (content: string, callbacks?: ConversationCallbacks): Promise => { // Only allow wizard for agents that support structured output - const supportedWizardAgents: ToolType[] = ['claude', 'claude-code', 'codex']; + const supportedWizardAgents: ToolType[] = ['claude-code', 'codex']; // Get the tab ID from the current state, ensure currentTabId is set for visibility const tabId = currentTabId || 'default';