diff --git a/auto-claude-ui/src/main/ipc-handlers/terminal-handlers.ts b/auto-claude-ui/src/main/ipc-handlers/terminal-handlers.ts index b78aca82..6091635c 100644 --- a/auto-claude-ui/src/main/ipc-handlers/terminal-handlers.ts +++ b/auto-claude-ui/src/main/ipc-handlers/terminal-handlers.ts @@ -8,6 +8,7 @@ import { TerminalManager } from '../terminal-manager'; import { projectStore } from '../project-store'; import { terminalNameGenerator } from '../terminal-name-generator'; import { debugLog, debugError } from '../../shared/utils/debug-logger'; +import { escapeShellArg } from '../../shared/utils/shell-escape'; /** @@ -163,10 +164,6 @@ export function registerTerminalHandlers( ipcMain.handle( IPC_CHANNELS.CLAUDE_PROFILE_SET_ACTIVE, async (_, profileId: string): Promise => { - // Always-on tracing to verify handler is called - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Handler invoked, profileId:', profileId); - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] DEBUG env:', process.env.DEBUG); - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] ========== PROFILE SWITCH START =========='); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Requested profile ID:', profileId); @@ -194,7 +191,6 @@ export function registerTerminalHandlers( debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] setActiveProfile result:', success); if (!success) { - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] setActiveProfile returned false'); debugError('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Profile not found, aborting'); return { success: false, error: 'Profile not found' }; } @@ -202,8 +198,6 @@ export function registerTerminalHandlers( // If the profile actually changed, restart Claude in active terminals // This ensures existing Claude sessions use the new profile's OAuth token const profileChanged = previousProfileId !== profileId; - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Profile changed:', profileChanged, - '| Previous:', previousProfileId, '| New:', profileId); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Profile changed:', profileChanged, { previousProfileId, newProfileId: profileId @@ -211,7 +205,6 @@ export function registerTerminalHandlers( if (profileChanged) { const activeTerminalIds = terminalManager.getActiveTerminalIds(); - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Active terminals:', activeTerminalIds.length); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Active terminal IDs:', activeTerminalIds); const switchPromises: Promise[] = []; @@ -220,7 +213,6 @@ export function registerTerminalHandlers( for (const terminalId of activeTerminalIds) { const isClaudeMode = terminalManager.isClaudeMode(terminalId); - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal:', terminalId, '| isClaudeMode:', isClaudeMode); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal check:', { terminalId, isClaudeMode @@ -228,17 +220,15 @@ export function registerTerminalHandlers( if (isClaudeMode) { terminalsInClaudeMode.push(terminalId); - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Queuing terminal for switch:', terminalId); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Queuing terminal for profile switch:', terminalId); switchPromises.push( terminalManager.switchClaudeProfile(terminalId, profileId) .then(() => { - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Switch SUCCESS:', terminalId); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal profile switch SUCCESS:', terminalId); }) .catch((err) => { - console.error('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Switch FAILED:', terminalId, err); debugError('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal profile switch FAILED:', terminalId, err); + throw err; // Re-throw so Promise.allSettled correctly reports rejections }) ); } else { @@ -246,8 +236,6 @@ export function registerTerminalHandlers( } } - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Summary: total=', activeTerminalIds.length, - '| inClaudeMode=', terminalsInClaudeMode.length, '| notInClaudeMode=', terminalsNotInClaudeMode.length); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal summary:', { total: activeTerminalIds.length, inClaudeMode: terminalsInClaudeMode.length, @@ -258,27 +246,22 @@ export function registerTerminalHandlers( // Wait for all switches to complete (but don't fail the main operation if some fail) if (switchPromises.length > 0) { - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Waiting for', switchPromises.length, 'switches...'); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Waiting for', switchPromises.length, 'terminal switches...'); const results = await Promise.allSettled(switchPromises); const fulfilled = results.filter(r => r.status === 'fulfilled').length; const rejected = results.filter(r => r.status === 'rejected').length; - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Results: fulfilled=', fulfilled, '| rejected=', rejected); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Switch results:', { total: results.length, fulfilled, rejected }); } else { - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] No terminals in Claude mode to switch'); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] No terminals in Claude mode to switch'); } } else { - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Same profile, no switches needed'); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Same profile selected, no terminal switches needed'); } - console.warn('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] ========== COMPLETE =========='); debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] ========== PROFILE SWITCH COMPLETE =========='); return { success: true }; } catch (error) { @@ -321,7 +304,7 @@ export function registerTerminalHandlers( const { mkdirSync, existsSync } = await import('fs'); if (!existsSync(profile.configDir)) { mkdirSync(profile.configDir, { recursive: true }); - console.warn('[IPC] Created config directory:', profile.configDir); + debugLog('[IPC] Created config directory:', profile.configDir); } } @@ -330,7 +313,7 @@ export function registerTerminalHandlers( const terminalId = `claude-login-${profileId}-${Date.now()}`; const homeDir = process.env.HOME || process.env.USERPROFILE || '/tmp'; - console.warn('[IPC] Initializing Claude profile:', { + debugLog('[IPC] Initializing Claude profile:', { profileId, profileName: profile.name, configDir: profile.configDir, @@ -348,12 +331,14 @@ export function registerTerminalHandlers( let loginCommand: string; if (!profile.isDefault && profile.configDir) { // Use export and run in subshell to ensure CLAUDE_CONFIG_DIR is properly set - loginCommand = `export CLAUDE_CONFIG_DIR="${profile.configDir}" && echo "Config dir: $CLAUDE_CONFIG_DIR" && claude setup-token`; + // SECURITY: Use escapeShellArg to prevent command injection via configDir + const escapedConfigDir = escapeShellArg(profile.configDir); + loginCommand = `export CLAUDE_CONFIG_DIR=${escapedConfigDir} && echo "Config dir: $CLAUDE_CONFIG_DIR" && claude setup-token`; } else { loginCommand = 'claude setup-token'; } - console.warn('[IPC] Sending login command to terminal:', loginCommand); + debugLog('[IPC] Sending login command to terminal:', loginCommand); // Write the login command to the terminal terminalManager.write(terminalId, `${loginCommand}\r`); @@ -376,7 +361,7 @@ export function registerTerminalHandlers( } }; } catch (error) { - console.error('[IPC] Failed to initialize Claude profile:', error); + debugError('[IPC] Failed to initialize Claude profile:', error); return { success: false, error: error instanceof Error ? error.message : 'Failed to initialize Claude profile' @@ -397,7 +382,7 @@ export function registerTerminalHandlers( } return { success: true }; } catch (error) { - console.error('[IPC] Failed to set OAuth token:', error); + debugError('[IPC] Failed to set OAuth token:', error); return { success: false, error: error instanceof Error ? error.message : 'Failed to set OAuth token' @@ -682,5 +667,5 @@ export function initializeUsageMonitorForwarding(mainWindow: BrowserWindow): voi mainWindow.webContents.send(IPC_CHANNELS.PROACTIVE_SWAP_NOTIFICATION, notification); }); - console.warn('[terminal-handlers] Usage monitor event forwarding initialized'); + debugLog('[terminal-handlers] Usage monitor event forwarding initialized'); } diff --git a/auto-claude-ui/src/main/terminal/claude-integration-handler.ts b/auto-claude-ui/src/main/terminal/claude-integration-handler.ts index e2377312..ae761772 100644 --- a/auto-claude-ui/src/main/terminal/claude-integration-handler.ts +++ b/auto-claude-ui/src/main/terminal/claude-integration-handler.ts @@ -11,6 +11,7 @@ import { getClaudeProfileManager } from '../claude-profile-manager'; import * as OutputParser from './output-parser'; import * as SessionHandler from './session-handler'; import { debugLog, debugError } from '../../shared/utils/debug-logger'; +import { escapeShellArg, buildCdCommand } from '../../shared/utils/shell-escape'; import type { TerminalProcess, WindowGetter, @@ -231,7 +232,8 @@ export function invokeClaude( isDefault: activeProfile?.isDefault }); - const cwdCommand = cwd ? `cd "${cwd}" && ` : ''; + // Use safe shell escaping to prevent command injection + const cwdCommand = buildCdCommand(cwd); const needsEnvOverride = profileId && profileId !== previousProfileId; debugLog('[ClaudeIntegration:invokeClaude] Environment override check:', { @@ -252,16 +254,25 @@ export function invokeClaude( debugLog('[ClaudeIntegration:invokeClaude] Writing token to temp file:', tempFile); fs.writeFileSync(tempFile, `export CLAUDE_CODE_OAUTH_TOKEN="${token}"\n`, { mode: 0o600 }); - // Clear terminal before running command to hide the ugly temp file path - const command = `clear && ${cwdCommand}source "${tempFile}" && rm -f "${tempFile}" && claude\r`; - debugLog('[ClaudeIntegration:invokeClaude] Executing command (temp file method):', command.replace(token, '[TOKEN_REDACTED]')); + // Clear terminal and run command without adding to shell history: + // - HISTFILE= disables history file writing for the current command + // - HISTCONTROL=ignorespace causes commands starting with space to be ignored + // - Leading space ensures the command is ignored even if HISTCONTROL was already set + // - Uses subshell (...) to isolate environment changes + // This prevents temp file paths from appearing in shell history + const command = `clear && ${cwdCommand} HISTFILE= HISTCONTROL=ignorespace bash -c 'source "${tempFile}" && rm -f "${tempFile}" && exec claude'\r`; + debugLog('[ClaudeIntegration:invokeClaude] Executing command (temp file method, history-safe)'); terminal.pty.write(command); debugLog('[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE COMPLETE (temp file) =========='); return; } else if (activeProfile.configDir) { - // Clear terminal before running command - const command = `clear && ${cwdCommand}CLAUDE_CONFIG_DIR="${activeProfile.configDir}" claude\r`; - debugLog('[ClaudeIntegration:invokeClaude] Executing command (configDir method):', command); + // Clear terminal and run command without adding to shell history: + // Same history-disabling technique as temp file method above + // SECURITY: Use escapeShellArg for configDir to prevent command injection + // Set CLAUDE_CONFIG_DIR as env var before bash -c to avoid embedding user input in the command string + const escapedConfigDir = escapeShellArg(activeProfile.configDir); + const command = `clear && ${cwdCommand}HISTFILE= HISTCONTROL=ignorespace CLAUDE_CONFIG_DIR=${escapedConfigDir} bash -c 'exec claude'\r`; + debugLog('[ClaudeIntegration:invokeClaude] Executing command (configDir method, history-safe)'); terminal.pty.write(command); debugLog('[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE COMPLETE (configDir) =========='); return; @@ -313,7 +324,8 @@ export function resumeClaude( let command: string; if (sessionId) { - command = `claude --resume "${sessionId}"`; + // SECURITY: Escape sessionId to prevent command injection + command = `claude --resume ${escapeShellArg(sessionId)}`; terminal.claudeSessionId = sessionId; } else { command = 'claude --continue'; @@ -327,6 +339,103 @@ export function resumeClaude( } } +/** + * Configuration for waiting for Claude to exit + */ +interface WaitForExitConfig { + /** Maximum time to wait for Claude to exit (ms) */ + timeout?: number; + /** Interval between checks (ms) */ + pollInterval?: number; +} + +/** + * Result of waiting for Claude to exit + */ +interface WaitForExitResult { + /** Whether Claude exited successfully */ + success: boolean; + /** Error message if failed */ + error?: string; + /** Whether the operation timed out */ + timedOut?: boolean; +} + +/** + * Shell prompt patterns that indicate Claude has exited and shell is ready + * These patterns match common shell prompts across bash, zsh, fish, etc. + */ +const SHELL_PROMPT_PATTERNS = [ + /[$%#>❯]\s*$/m, // Common prompt endings: $, %, #, >, ❯ + /\w+@[\w.-]+[:\s]/, // user@hostname: format + /^\s*\S+\s*[$%#>❯]\s*$/m, // hostname/path followed by prompt char + /\(.*\)\s*[$%#>❯]\s*$/m, // (venv) or (branch) followed by prompt +]; + +/** + * Wait for Claude to exit by monitoring terminal output for shell prompt + * + * Instead of using fixed delays, this monitors the terminal's outputBuffer + * for patterns indicating that Claude has exited and the shell prompt is visible. + */ +async function waitForClaudeExit( + terminal: TerminalProcess, + config: WaitForExitConfig = {} +): Promise { + const { timeout = 5000, pollInterval = 100 } = config; + + debugLog('[ClaudeIntegration:waitForClaudeExit] Waiting for Claude to exit...'); + debugLog('[ClaudeIntegration:waitForClaudeExit] Config:', { timeout, pollInterval }); + + // Capture current buffer length to detect new output + const initialBufferLength = terminal.outputBuffer.length; + const startTime = Date.now(); + + return new Promise((resolve) => { + const checkForPrompt = () => { + const elapsed = Date.now() - startTime; + + // Check for timeout + if (elapsed >= timeout) { + console.warn('[ClaudeIntegration:waitForClaudeExit] Timeout waiting for Claude to exit after', timeout, 'ms'); + debugLog('[ClaudeIntegration:waitForClaudeExit] Timeout reached, Claude may not have exited cleanly'); + resolve({ + success: false, + error: `Timeout waiting for Claude to exit after ${timeout}ms`, + timedOut: true + }); + return; + } + + // Get new output since we started waiting + const newOutput = terminal.outputBuffer.slice(initialBufferLength); + + // Check if we can see a shell prompt in the new output + for (const pattern of SHELL_PROMPT_PATTERNS) { + if (pattern.test(newOutput)) { + debugLog('[ClaudeIntegration:waitForClaudeExit] Shell prompt detected after', elapsed, 'ms'); + debugLog('[ClaudeIntegration:waitForClaudeExit] Matched pattern:', pattern.toString()); + resolve({ success: true }); + return; + } + } + + // Also check if isClaudeMode was cleared (set by other handlers) + if (!terminal.isClaudeMode) { + debugLog('[ClaudeIntegration:waitForClaudeExit] isClaudeMode flag cleared after', elapsed, 'ms'); + resolve({ success: true }); + return; + } + + // Continue polling + setTimeout(checkForPrompt, pollInterval); + }; + + // Start checking + checkForPrompt(); + }); +} + /** * Switch terminal to a different Claude profile */ @@ -375,14 +484,36 @@ export async function switchClaudeProfile( if (terminal.isClaudeMode) { console.warn('[ClaudeIntegration:switchClaudeProfile] Sending exit commands (Ctrl+C, /exit)'); debugLog('[ClaudeIntegration:switchClaudeProfile] Terminal is in Claude mode, sending exit commands'); + + // Send Ctrl+C to interrupt any ongoing operation debugLog('[ClaudeIntegration:switchClaudeProfile] Sending Ctrl+C (\\x03)'); terminal.pty.write('\x03'); - await new Promise(resolve => setTimeout(resolve, 500)); + + // Wait briefly for Ctrl+C to take effect before sending /exit + await new Promise(resolve => setTimeout(resolve, 100)); + + // Send /exit command debugLog('[ClaudeIntegration:switchClaudeProfile] Sending /exit command'); terminal.pty.write('/exit\r'); - await new Promise(resolve => setTimeout(resolve, 500)); - console.warn('[ClaudeIntegration:switchClaudeProfile] Exit commands sent'); - debugLog('[ClaudeIntegration:switchClaudeProfile] Exit commands sent, waiting for Claude to exit'); + + // Wait for Claude to actually exit by monitoring for shell prompt + const exitResult = await waitForClaudeExit(terminal, { timeout: 5000, pollInterval: 100 }); + + if (exitResult.timedOut) { + console.warn('[ClaudeIntegration:switchClaudeProfile] Timed out waiting for Claude to exit, proceeding with caution'); + debugLog('[ClaudeIntegration:switchClaudeProfile] Exit timeout - terminal may be in inconsistent state'); + + // Even on timeout, we'll try to proceed but log the warning + // The alternative would be to abort, but that could leave users stuck + // If this becomes a problem, we could add retry logic or abort option + } else if (!exitResult.success) { + console.error('[ClaudeIntegration:switchClaudeProfile] Failed to exit Claude:', exitResult.error); + debugError('[ClaudeIntegration:switchClaudeProfile] Exit failed:', exitResult.error); + // Continue anyway - the /exit command was sent + } else { + console.warn('[ClaudeIntegration:switchClaudeProfile] Claude exited successfully'); + debugLog('[ClaudeIntegration:switchClaudeProfile] Claude exited, ready to switch profile'); + } } else { console.warn('[ClaudeIntegration:switchClaudeProfile] NOT in Claude mode, skipping exit commands'); debugLog('[ClaudeIntegration:switchClaudeProfile] Terminal NOT in Claude mode, skipping exit commands'); diff --git a/auto-claude-ui/src/renderer/components/settings/integrations/GitHubIntegration.tsx b/auto-claude-ui/src/renderer/components/settings/integrations/GitHubIntegration.tsx index 52d21915..ad21e613 100644 --- a/auto-claude-ui/src/renderer/components/settings/integrations/GitHubIntegration.tsx +++ b/auto-claude-ui/src/renderer/components/settings/integrations/GitHubIntegration.tsx @@ -74,7 +74,7 @@ export function GitHubIntegration({ // Fetch branches when GitHub is enabled and project path is available useEffect(() => { - debugLog('useEffect[branches] - githubEnabled:', envConfig?.githubEnabled, 'projectPath:', projectPath); + debugLog(`useEffect[branches] - githubEnabled: ${envConfig?.githubEnabled}, projectPath: ${projectPath}`); if (envConfig?.githubEnabled && projectPath) { debugLog('useEffect[branches] - Triggering fetchBranches'); fetchBranches(); diff --git a/auto-claude-ui/src/shared/utils/shell-escape.ts b/auto-claude-ui/src/shared/utils/shell-escape.ts new file mode 100644 index 00000000..57e46275 --- /dev/null +++ b/auto-claude-ui/src/shared/utils/shell-escape.ts @@ -0,0 +1,79 @@ +/** + * Shell Escape Utilities + * + * Provides safe escaping for shell command arguments to prevent command injection. + * IMPORTANT: Always use these utilities when interpolating user-controlled values into shell commands. + */ + +/** + * Escape a string for safe use as a shell argument. + * + * Uses single quotes which prevent all shell expansion (variables, command substitution, etc.) + * except for single quotes themselves, which are escaped as '\'' + * + * Examples: + * - "hello" → 'hello' + * - "hello world" → 'hello world' + * - "it's" → 'it'\''s' + * - "$(rm -rf /)" → '$(rm -rf /)' + * - 'test"; rm -rf / #' → 'test"; rm -rf / #' + * + * @param arg - The argument to escape + * @returns The escaped argument wrapped in single quotes + */ +export function escapeShellArg(arg: string): string { + // Replace single quotes with: end quote, escaped quote, start quote + // This is the standard POSIX-safe way to handle single quotes + const escaped = arg.replace(/'/g, "'\\''"); + return `'${escaped}'`; +} + +/** + * Escape a path for use in a cd command. + * + * @param path - The path to escape + * @returns The escaped path safe for use in shell commands + */ +export function escapeShellPath(path: string): string { + return escapeShellArg(path); +} + +/** + * Build a safe cd command from a path. + * + * @param path - The directory path + * @returns A safe "cd '' && " string, or empty string if path is undefined + */ +export function buildCdCommand(path: string | undefined): string { + if (!path) { + return ''; + } + return `cd ${escapeShellPath(path)} && `; +} + +/** + * Validate that a path doesn't contain obviously malicious patterns. + * This is a defense-in-depth measure - escaping should handle all cases, + * but this can catch obvious attack attempts early. + * + * @param path - The path to validate + * @returns true if the path appears safe, false if it contains suspicious patterns + */ +export function isPathSafe(path: string): boolean { + // Check for obvious shell metacharacters that shouldn't appear in paths + // Note: This is defense-in-depth; escaping handles these, but we can log/reject + const suspiciousPatterns = [ + /\$\(/, // Command substitution $(...) + /`/, // Backtick command substitution + /\|/, // Pipe + /;/, // Command separator + /&&/, // AND operator + /\|\|/, // OR operator + />/, // Output redirection + / pattern.test(path)); +}