-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(terminal): use Windows cmd.exe compatible shell escaping for Claude invocation #1044
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 3 commits
f3aaf11
2b75496
a308d36
7f3e845
5a832ab
ee0a00d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ import { getClaudeProfileManager, initializeClaudeProfileManager } from '../clau | |
| 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 { escapeShellArg, escapeShellArgWindows, buildCdCommand } from '../../shared/utils/shell-escape'; | ||
| import { getClaudeCliInvocation, getClaudeCliInvocationAsync } from '../claude-cli-utils'; | ||
| import type { | ||
| TerminalProcess, | ||
|
|
@@ -32,6 +32,23 @@ function normalizePathForBash(envPath: string): string { | |
| */ | ||
| const YOLO_MODE_FLAG = ' --dangerously-skip-permissions'; | ||
|
|
||
| /** | ||
| * Check if we're running on Windows | ||
| */ | ||
| const isWindows = process.platform === 'win32'; | ||
|
|
||
| /** | ||
| * Escape a shell argument for the current platform. | ||
| * Uses single quotes on Unix/Mac, double quotes on Windows cmd.exe. | ||
| */ | ||
| function escapeForPlatform(arg: string): string { | ||
| if (isWindows) { | ||
| // On Windows cmd.exe, wrap in double quotes after escaping special chars | ||
| return `"${escapeShellArgWindows(arg)}"`; | ||
| } | ||
| return escapeShellArg(arg); | ||
| } | ||
|
|
||
| // ============================================================================ | ||
| // SHARED HELPERS - Used by both sync and async invokeClaude | ||
| // ============================================================================ | ||
|
|
@@ -43,7 +60,7 @@ const YOLO_MODE_FLAG = ' --dangerously-skip-permissions'; | |
| type ClaudeCommandConfig = | ||
| | { method: 'default' } | ||
| | { method: 'temp-file'; escapedTempFile: string } | ||
| | { method: 'config-dir'; escapedConfigDir: string }; | ||
| | { method: 'config-dir'; escapedConfigDir: string; rawConfigDir: string }; | ||
|
|
||
| /** | ||
| * Build the shell command for invoking Claude CLI. | ||
|
|
@@ -56,8 +73,10 @@ type ClaudeCommandConfig = | |
| * All non-default methods include history-safe prefixes (HISTFILE=, HISTCONTROL=) | ||
| * to prevent sensitive data from appearing in shell history. | ||
| * | ||
| * On Windows, uses cmd.exe-compatible syntax. On Unix/Mac, uses bash syntax. | ||
| * | ||
| * @param cwdCommand - Command to change directory (empty string if no change needed) | ||
| * @param pathPrefix - PATH prefix for Claude CLI (empty string if not needed) | ||
| * @param pathPrefix - PATH prefix for Claude CLI (empty string if not needed, ignored on Windows) | ||
| * @param escapedClaudeCmd - Shell-escaped Claude CLI command | ||
| * @param config - Configuration object with method and required options (discriminated union) | ||
| * @param extraFlags - Optional extra flags to append to the command (e.g., '--dangerously-skip-permissions') | ||
|
|
@@ -80,6 +99,27 @@ export function buildClaudeShellCommand( | |
| extraFlags?: string | ||
| ): string { | ||
| const fullCmd = extraFlags ? `${escapedClaudeCmd}${extraFlags}` : escapedClaudeCmd; | ||
|
|
||
| // Windows uses cmd.exe which has different syntax than bash | ||
| if (isWindows) { | ||
| switch (config.method) { | ||
| case 'temp-file': | ||
| // On Windows, use 'call' to execute the batch file that sets env var, then run claude | ||
| // Use del /Q for quiet (no confirmation prompt) deletion | ||
| return `cls && ${cwdCommand}call ${config.escapedTempFile} && del /Q ${config.escapedTempFile} && ${fullCmd}\r`; | ||
|
|
||
| case 'config-dir': | ||
| // On Windows, use 'set "VAR=value"' syntax to handle paths with spaces correctly | ||
| // The quotes go around the entire assignment, not around the value | ||
| return `cls && ${cwdCommand}set "CLAUDE_CONFIG_DIR=${escapeShellArgWindows(config.rawConfigDir)}" && ${fullCmd}\r`; | ||
|
|
||
| default: | ||
| // On Windows, don't use PATH= prefix syntax - the PTY already has correct PATH | ||
| return `${cwdCommand}${fullCmd}\r`; | ||
| } | ||
| } | ||
|
|
||
| // Unix/Mac bash syntax | ||
| switch (config.method) { | ||
| case 'temp-file': | ||
| return `clear && ${cwdCommand}HISTFILE= HISTCONTROL=ignorespace ${pathPrefix}bash -c "source ${config.escapedTempFile} && rm -f ${config.escapedTempFile} && exec ${fullCmd}"\r`; | ||
|
|
@@ -419,7 +459,7 @@ export function invokeClaude( | |
|
|
||
| const cwdCommand = buildCdCommand(cwd); | ||
| const { command: claudeCmd, env: claudeEnv } = getClaudeCliInvocation(); | ||
| const escapedClaudeCmd = escapeShellArg(claudeCmd); | ||
| const escapedClaudeCmd = escapeForPlatform(claudeCmd); | ||
| const pathPrefix = claudeEnv.PATH | ||
| ? `PATH=${escapeShellArg(normalizePathForBash(claudeEnv.PATH))} ` | ||
| : ''; | ||
|
Comment on lines
463
to
465
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Minor inconsistency: Unlike - const pathPrefix = claudeEnv.PATH
+ const pathPrefix = !isWindows && claudeEnv.PATH
? `PATH=${escapeShellArg(normalizePathForBash(claudeEnv.PATH))} `
: '';Same applies to the async version at lines 646-648. 🤖 Prompt for AI Agents |
||
|
|
@@ -440,14 +480,18 @@ export function invokeClaude( | |
|
|
||
| if (token) { | ||
| const nonce = crypto.randomBytes(8).toString('hex'); | ||
| const tempFile = path.join(os.tmpdir(), `.claude-token-${Date.now()}-${nonce}`); | ||
| const escapedTempFile = escapeShellArg(tempFile); | ||
| // Use .bat extension on Windows, no extension on Unix (sourced as shell script) | ||
| const tempFileExt = isWindows ? '.bat' : ''; | ||
| const tempFile = path.join(os.tmpdir(), `.claude-token-${Date.now()}-${nonce}${tempFileExt}`); | ||
| const escapedTempFile = escapeForPlatform(tempFile); | ||
| debugLog('[ClaudeIntegration:invokeClaude] Writing token to temp file:', tempFile); | ||
| fs.writeFileSync( | ||
| tempFile, | ||
| `export CLAUDE_CODE_OAUTH_TOKEN=${escapeShellArg(token)}\n`, | ||
| { mode: 0o600 } | ||
| ); | ||
|
|
||
| // Write platform-appropriate content | ||
| const tempFileContent = isWindows | ||
| ? `@echo off\r\nset CLAUDE_CODE_OAUTH_TOKEN=${escapeShellArgWindows(token)}\r\n` // Windows batch file | ||
| : `export CLAUDE_CODE_OAUTH_TOKEN=${escapeShellArg(token)}\n`; // Unix shell script | ||
|
|
||
| fs.writeFileSync(tempFile, tempFileContent, { mode: 0o600 }); | ||
kirollosatef marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'temp-file', escapedTempFile }, extraFlags); | ||
| debugLog('[ClaudeIntegration:invokeClaude] Executing command (temp file method, history-safe)'); | ||
|
|
@@ -457,8 +501,8 @@ export function invokeClaude( | |
| debugLog('[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE COMPLETE (temp file) =========='); | ||
| return; | ||
| } else if (activeProfile.configDir) { | ||
| const escapedConfigDir = escapeShellArg(activeProfile.configDir); | ||
| const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'config-dir', escapedConfigDir }, extraFlags); | ||
| const escapedConfigDir = escapeForPlatform(activeProfile.configDir); | ||
| const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'config-dir', escapedConfigDir, rawConfigDir: activeProfile.configDir }, extraFlags); | ||
| debugLog('[ClaudeIntegration:invokeClaude] Executing command (configDir method, history-safe)'); | ||
| terminal.pty.write(command); | ||
| profileManager.markProfileUsed(activeProfile.id); | ||
|
|
@@ -506,8 +550,9 @@ export function resumeClaude( | |
| SessionHandler.releaseSessionId(terminal.id); | ||
|
|
||
| const { command: claudeCmd, env: claudeEnv } = getClaudeCliInvocation(); | ||
| const escapedClaudeCmd = escapeShellArg(claudeCmd); | ||
| const pathPrefix = claudeEnv.PATH | ||
| const escapedClaudeCmd = escapeForPlatform(claudeCmd); | ||
| // On Windows, the PTY already has the correct PATH - don't use bash-style PATH= prefix | ||
| const pathPrefix = !isWindows && claudeEnv.PATH | ||
| ? `PATH=${escapeShellArg(normalizePathForBash(claudeEnv.PATH))} ` | ||
| : ''; | ||
|
|
||
|
|
@@ -597,7 +642,7 @@ export async function invokeClaudeAsync( | |
| // Async CLI invocation - non-blocking | ||
| const cwdCommand = buildCdCommand(cwd); | ||
| const { command: claudeCmd, env: claudeEnv } = await getClaudeCliInvocationAsync(); | ||
| const escapedClaudeCmd = escapeShellArg(claudeCmd); | ||
| const escapedClaudeCmd = escapeForPlatform(claudeCmd); | ||
| const pathPrefix = claudeEnv.PATH | ||
| ? `PATH=${escapeShellArg(normalizePathForBash(claudeEnv.PATH))} ` | ||
| : ''; | ||
|
|
@@ -618,14 +663,18 @@ export async function invokeClaudeAsync( | |
|
|
||
| if (token) { | ||
| const nonce = crypto.randomBytes(8).toString('hex'); | ||
| const tempFile = path.join(os.tmpdir(), `.claude-token-${Date.now()}-${nonce}`); | ||
| const escapedTempFile = escapeShellArg(tempFile); | ||
| // Use .bat extension on Windows, no extension on Unix (sourced as shell script) | ||
| const tempFileExt = isWindows ? '.bat' : ''; | ||
| const tempFile = path.join(os.tmpdir(), `.claude-token-${Date.now()}-${nonce}${tempFileExt}`); | ||
| const escapedTempFile = escapeForPlatform(tempFile); | ||
| debugLog('[ClaudeIntegration:invokeClaudeAsync] Writing token to temp file:', tempFile); | ||
| await fsPromises.writeFile( | ||
| tempFile, | ||
| `export CLAUDE_CODE_OAUTH_TOKEN=${escapeShellArg(token)}\n`, | ||
| { mode: 0o600 } | ||
| ); | ||
|
|
||
| // Write platform-appropriate content | ||
| const tempFileContent = isWindows | ||
| ? `@echo off\r\nset CLAUDE_CODE_OAUTH_TOKEN=${escapeShellArgWindows(token)}\r\n` // Windows batch file | ||
| : `export CLAUDE_CODE_OAUTH_TOKEN=${escapeShellArg(token)}\n`; // Unix shell script | ||
|
|
||
| await fsPromises.writeFile(tempFile, tempFileContent, { mode: 0o600 }); | ||
kirollosatef marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'temp-file', escapedTempFile }, extraFlags); | ||
| debugLog('[ClaudeIntegration:invokeClaudeAsync] Executing command (temp file method, history-safe)'); | ||
|
|
@@ -635,8 +684,8 @@ export async function invokeClaudeAsync( | |
| debugLog('[ClaudeIntegration:invokeClaudeAsync] ========== INVOKE CLAUDE COMPLETE (temp file) =========='); | ||
| return; | ||
| } else if (activeProfile.configDir) { | ||
| const escapedConfigDir = escapeShellArg(activeProfile.configDir); | ||
| const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'config-dir', escapedConfigDir }, extraFlags); | ||
| const escapedConfigDir = escapeForPlatform(activeProfile.configDir); | ||
| const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'config-dir', escapedConfigDir, rawConfigDir: activeProfile.configDir }, extraFlags); | ||
| debugLog('[ClaudeIntegration:invokeClaudeAsync] Executing command (configDir method, history-safe)'); | ||
| terminal.pty.write(command); | ||
| profileManager.markProfileUsed(activeProfile.id); | ||
|
|
@@ -680,8 +729,9 @@ export async function resumeClaudeAsync( | |
|
|
||
| // Async CLI invocation - non-blocking | ||
| const { command: claudeCmd, env: claudeEnv } = await getClaudeCliInvocationAsync(); | ||
| const escapedClaudeCmd = escapeShellArg(claudeCmd); | ||
| const pathPrefix = claudeEnv.PATH | ||
| const escapedClaudeCmd = escapeForPlatform(claudeCmd); | ||
| // On Windows, the PTY already has the correct PATH - don't use bash-style PATH= prefix | ||
| const pathPrefix = !isWindows && claudeEnv.PATH | ||
| ? `PATH=${escapeShellArg(normalizePathForBash(claudeEnv.PATH))} ` | ||
| : ''; | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.