refactor: enhance terminal command handling and security

- Introduced shell escape utilities to prevent command injection in terminal commands.
- Updated terminal handlers to use safe command construction for profile switching and OAuth token initialization.
- Removed unnecessary console warnings, replacing them with debug logs for cleaner output.
- Implemented a wait mechanism to monitor terminal output for Claude exit, improving profile switching reliability.

This update improves the security and reliability of terminal command execution, ensuring user inputs are safely handled.
This commit is contained in:
AndyMik90
2025-12-20 01:24:40 +01:00
parent c5b72451af
commit 52e12d8d2a
4 changed files with 234 additions and 39 deletions
@@ -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<IPCResult> => {
// 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<void>[] = [];
@@ -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');
}
@@ -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<WaitForExitResult> {
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');
@@ -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();
@@ -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 '<path>' && " 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
/</, // Input redirection
/\n/, // Newlines
/\r/, // Carriage returns
];
return !suspiciousPatterns.some(pattern => pattern.test(path));
}