fix: properly quote Windows .cmd/.bat paths in spawn() calls (#889)

* fix: properly quote Windows .cmd/.bat paths in spawn() calls

Fixes Claude Code detection failure when Windows username contains spaces
(e.g., C:\Users\First Last\AppData\Roaming\npm\claude.cmd).

When spawn() is called with shell:true for .cmd/.bat files, the command
path must be quoted to prevent the shell from breaking at spaces. Without
quoting, a path like "C:\Users\First Last\..." is parsed as:
  - Command: "C:\Users\First"
  - Args: "Last\..."

Changes:
- Add getSpawnCommand() to wrap .cmd/.bat paths in quotes on Windows
- Update spawn() calls to use getSpawnCommand() for proper quoting
- Add comprehensive tests for getSpawnCommand() with space handling

Fixes ACS-176

* refactor: make shouldUseShell/getSpawnCommand robust to already-quoted paths

- shouldUseShell() now correctly detects .cmd/.bat extensions even when
  the path is already wrapped in quotes
- getSpawnCommand() is now idempotent - calling it multiple times or with
  an already-quoted command returns the same result
- Both functions now trim whitespace before processing

This makes the public API more robust to edge cases and prevents issues
if callers accidentally pass quoted commands.

* refactor: make getSpawnCommand consistently trim whitespace on all platforms

Previously, getSpawnCommand() only trimmed whitespace for Windows shell
cases (.cmd/.bat files) but returned the original untrimmed command for
non-shell cases (macOS/Linux). This caused inconsistent behavior.

Now getSpawnCommand() always returns a trimmed value regardless of platform,
ensuring consistent whitespace handling for all callers.

Also added tests to verify whitespace trimming on macOS and Linux platforms.

* fix: address PR review findings for getSpawnCommand

- Add quote stripping for non-.cmd/.bat files (.exe, extensionless) to
  prevent returning quoted commands with shell:false
- Update validateClaude/validateClaudeAsync to use getSpawnCommand()
  instead of manual quoting (DRY principle)
- Add tests for quote-stripping behavior on .exe and extensionless files

These changes make getSpawnCommand() more robust and ensure consistent
behavior across all file types, while eliminating duplicate quoting logic.

* test: add getSpawnCommand mock to cli-tool-manager tests

The env-utils mock in cli-tool-manager.test.ts was missing getSpawnCommand,
causing tests to fail when validateClaude() tried to call it.

Added getSpawnCommand mock that mirrors the actual implementation:
- On Windows: quotes .cmd/.bat files idempotently
- For other files: returns trimmed value (strips quotes if present)

---------

Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com>
Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
This commit is contained in:
StillKnotKnown
2026-01-11 01:20:56 +02:00
committed by GitHub
parent a6bd8842f8
commit 6dc538c86f
5 changed files with 246 additions and 10 deletions
@@ -105,6 +105,23 @@ vi.mock('../env-utils', () => ({
} }
return /\.(cmd|bat)$/i.test(command); return /\.(cmd|bat)$/i.test(command);
}), }),
getSpawnCommand: vi.fn((command: string) => {
// Mock getSpawnCommand to match actual behavior
const trimmed = command.trim();
// On Windows, quote .cmd/.bat files
if (process.platform === 'win32' && /\.(cmd|bat)$/i.test(trimmed)) {
// Idempotent - if already quoted, return as-is
if (trimmed.startsWith('"') && trimmed.endsWith('"')) {
return trimmed;
}
return `"${trimmed}"`;
}
// For non-.cmd/.bat files, return trimmed (strip quotes if present)
if (trimmed.startsWith('"') && trimmed.endsWith('"')) {
return trimmed.slice(1, -1);
}
return trimmed;
}),
getSpawnOptions: vi.fn((command: string, baseOptions?: any) => ({ getSpawnOptions: vi.fn((command: string, baseOptions?: any) => ({
...baseOptions, ...baseOptions,
shell: /\.(cmd|bat)$/i.test(command) && process.platform === 'win32' shell: /\.(cmd|bat)$/i.test(command) && process.platform === 'win32'
@@ -1,5 +1,5 @@
import { describe, expect, it, beforeEach, afterEach } from 'vitest'; import { describe, expect, it, beforeEach, afterEach } from 'vitest';
import { shouldUseShell, getSpawnOptions } from '../env-utils'; import { shouldUseShell, getSpawnOptions, getSpawnCommand } from '../env-utils';
describe('shouldUseShell', () => { describe('shouldUseShell', () => {
const originalPlatform = process.platform; const originalPlatform = process.platform;
@@ -198,3 +198,181 @@ describe('getSpawnOptions', () => {
}); });
}); });
}); });
describe('getSpawnCommand', () => {
const originalPlatform = process.platform;
afterEach(() => {
// Restore original platform after each test
Object.defineProperty(process, 'platform', {
value: originalPlatform,
writable: true,
configurable: true,
});
});
describe('Windows platform', () => {
beforeEach(() => {
Object.defineProperty(process, 'platform', {
value: 'win32',
writable: true,
configurable: true,
});
});
it('should quote .cmd files with spaces', () => {
const cmd = getSpawnCommand('C:\\Users\\First Last\\AppData\\Roaming\\npm\\claude.cmd');
expect(cmd).toBe('"C:\\Users\\First Last\\AppData\\Roaming\\npm\\claude.cmd"');
});
it('should quote .cmd files without spaces too (idempotent)', () => {
const cmd = getSpawnCommand('C:\\Users\\admin\\AppData\\Roaming\\npm\\claude.cmd');
expect(cmd).toBe('"C:\\Users\\admin\\AppData\\Roaming\\npm\\claude.cmd"');
});
it('should quote .bat files with spaces', () => {
const cmd = getSpawnCommand('D:\\Program Files (x86)\\scripts\\setup.bat');
expect(cmd).toBe('"D:\\Program Files (x86)\\scripts\\setup.bat"');
});
it('should NOT quote .exe files', () => {
const cmd = getSpawnCommand('C:\\Program Files\\Git\\cmd\\git.exe');
expect(cmd).toBe('C:\\Program Files\\Git\\cmd\\git.exe');
});
it('should NOT quote extensionless files', () => {
const cmd = getSpawnCommand('D:\\Git\\bin\\bash');
expect(cmd).toBe('D:\\Git\\bin\\bash');
});
it('should handle uppercase .CMD and .BAT extensions', () => {
expect(getSpawnCommand('D:\\Tools\\CLAUDE.CMD')).toBe('"D:\\Tools\\CLAUDE.CMD"');
expect(getSpawnCommand('C:\\Scripts\\SETUP.BAT')).toBe('"C:\\Scripts\\SETUP.BAT"');
});
it('should be idempotent - already quoted .cmd files stay quoted', () => {
const cmd = getSpawnCommand('"C:\\Users\\admin\\AppData\\Roaming\\npm\\claude.cmd"');
expect(cmd).toBe('"C:\\Users\\admin\\AppData\\Roaming\\npm\\claude.cmd"');
});
it('should be idempotent - already quoted .bat files stay quoted', () => {
const cmd = getSpawnCommand('"D:\\Program Files\\scripts\\setup.bat"');
expect(cmd).toBe('"D:\\Program Files\\scripts\\setup.bat"');
});
it('should be idempotent - double-quoting does not occur', () => {
const once = getSpawnCommand('C:\\Users\\admin\\npm\\claude.cmd');
const twice = getSpawnCommand(once);
expect(once).toBe(twice);
expect(once).toBe('"C:\\Users\\admin\\npm\\claude.cmd"');
});
it('should trim whitespace before processing', () => {
const cmd = getSpawnCommand(' C:\\Users\\admin\\npm\\claude.cmd ');
expect(cmd).toBe('"C:\\Users\\admin\\npm\\claude.cmd"');
});
it('should handle already-quoted .cmd with spaces', () => {
const cmd = getSpawnCommand('"C:\\Users\\First Last\\npm\\claude.cmd"');
expect(cmd).toBe('"C:\\Users\\First Last\\npm\\claude.cmd"');
});
it('should strip quotes from .exe files (defensive: no quotes with shell:false)', () => {
const cmd = getSpawnCommand('"C:\\Program Files\\Git\\cmd\\git.exe"');
expect(cmd).toBe('C:\\Program Files\\Git\\cmd\\git.exe');
});
it('should strip quotes from extensionless files (defensive: no quotes with shell:false)', () => {
const cmd = getSpawnCommand('"D:\\Git\\bin\\bash"');
expect(cmd).toBe('D:\\Git\\bin\\bash');
});
it('should strip quotes and trim whitespace from .exe files', () => {
const cmd = getSpawnCommand(' "C:\\Program Files\\Git\\cmd\\git.exe" ');
expect(cmd).toBe('C:\\Program Files\\Git\\cmd\\git.exe');
});
});
describe('Non-Windows platforms', () => {
it('should NOT quote commands on macOS', () => {
Object.defineProperty(process, 'platform', {
value: 'darwin',
writable: true,
configurable: true,
});
expect(getSpawnCommand('/usr/local/bin/claude')).toBe('/usr/local/bin/claude');
expect(getSpawnCommand('/opt/homebrew/bin/claude.cmd')).toBe('/opt/homebrew/bin/claude.cmd');
});
it('should NOT quote commands on Linux', () => {
Object.defineProperty(process, 'platform', {
value: 'linux',
writable: true,
configurable: true,
});
expect(getSpawnCommand('/usr/bin/claude')).toBe('/usr/bin/claude');
expect(getSpawnCommand('/home/user/.local/bin/claude.bat')).toBe('/home/user/.local/bin/claude.bat');
});
it('should trim whitespace on macOS', () => {
Object.defineProperty(process, 'platform', {
value: 'darwin',
writable: true,
configurable: true,
});
expect(getSpawnCommand(' /usr/local/bin/claude ')).toBe('/usr/local/bin/claude');
expect(getSpawnCommand('\t/opt/homebrew/bin/claude\t')).toBe('/opt/homebrew/bin/claude');
});
it('should trim whitespace on Linux', () => {
Object.defineProperty(process, 'platform', {
value: 'linux',
writable: true,
configurable: true,
});
expect(getSpawnCommand(' /usr/bin/claude ')).toBe('/usr/bin/claude');
expect(getSpawnCommand('\t/home/user/.local/bin/claude\t')).toBe('/home/user/.local/bin/claude');
});
});
});
describe('shouldUseShell with quoted paths', () => {
const originalPlatform = process.platform;
afterEach(() => {
// Restore original platform after each test
Object.defineProperty(process, 'platform', {
value: originalPlatform,
writable: true,
configurable: true,
});
});
describe('Windows platform', () => {
beforeEach(() => {
Object.defineProperty(process, 'platform', {
value: 'win32',
writable: true,
configurable: true,
});
});
it('should detect .cmd files in quoted paths', () => {
expect(shouldUseShell('"C:\\Users\\admin\\npm\\claude.cmd"')).toBe(true);
expect(shouldUseShell('"D:\\Tools\\CLAUDE.CMD"')).toBe(true);
});
it('should detect .bat files in quoted paths', () => {
expect(shouldUseShell('"C:\\Scripts\\setup.bat"')).toBe(true);
expect(shouldUseShell('"D:\\Program Files\\script.BAT"')).toBe(true);
});
it('should NOT detect .exe files in quoted paths', () => {
expect(shouldUseShell('"C:\\Program Files\\git.exe"')).toBe(false);
});
it('should handle whitespace around quoted paths', () => {
expect(shouldUseShell(' "C:\\Users\\admin\\npm\\claude.cmd" ')).toBe(true);
});
});
});
+3 -3
View File
@@ -26,7 +26,7 @@ import path from 'path';
import os from 'os'; import os from 'os';
import { promisify } from 'util'; import { promisify } from 'util';
import { app } from 'electron'; import { app } from 'electron';
import { findExecutable, findExecutableAsync, getAugmentedEnv, getAugmentedEnvAsync, shouldUseShell, existsAsync } from './env-utils'; import { findExecutable, findExecutableAsync, getAugmentedEnv, getAugmentedEnvAsync, shouldUseShell, existsAsync, getSpawnCommand } from './env-utils';
import type { ToolDetectionResult } from '../shared/types'; import type { ToolDetectionResult } from '../shared/types';
const execFileAsync = promisify(execFile); const execFileAsync = promisify(execFile);
@@ -923,7 +923,7 @@ class CLIToolManager {
if (needsShell) { if (needsShell) {
// For .cmd/.bat files on Windows, use execSync with quoted path // For .cmd/.bat files on Windows, use execSync with quoted path
// execFileSync doesn't handle spaces in .cmd paths correctly even with shell:true // execFileSync doesn't handle spaces in .cmd paths correctly even with shell:true
const quotedCmd = `"${claudeCmd}"`; const quotedCmd = getSpawnCommand(claudeCmd);
version = execSync(`${quotedCmd} --version`, { version = execSync(`${quotedCmd} --version`, {
encoding: 'utf-8', encoding: 'utf-8',
timeout: 5000, timeout: 5000,
@@ -1039,7 +1039,7 @@ class CLIToolManager {
if (needsShell) { if (needsShell) {
// For .cmd/.bat files on Windows, use exec with quoted path // For .cmd/.bat files on Windows, use exec with quoted path
const quotedCmd = `"${claudeCmd}"`; const quotedCmd = getSpawnCommand(claudeCmd);
const result = await execAsync(`${quotedCmd} --version`, { const result = await execAsync(`${quotedCmd} --version`, {
encoding: 'utf-8', encoding: 'utf-8',
timeout: 5000, timeout: 5000,
+43 -2
View File
@@ -463,8 +463,12 @@ export function shouldUseShell(command: string): boolean {
return false; return false;
} }
const trimmed = command.trim();
const unquoted =
trimmed.startsWith('"') && trimmed.endsWith('"') ? trimmed.slice(1, -1) : trimmed;
// Check if command ends with .cmd or .bat (case-insensitive) // Check if command ends with .cmd or .bat (case-insensitive)
return /\.(cmd|bat)$/i.test(command); return /\.(cmd|bat)$/i.test(unquoted);
} }
/** /**
@@ -473,6 +477,9 @@ export function shouldUseShell(command: string): boolean {
* Provides a consistent way to create spawn options that work across platforms. * Provides a consistent way to create spawn options that work across platforms.
* Handles the shell requirement for Windows .cmd/.bat files automatically. * Handles the shell requirement for Windows .cmd/.bat files automatically.
* *
* For .cmd/.bat files on Windows, returns options that tell the caller to use
* proper quoting for paths with spaces.
*
* @param command - The command path to execute * @param command - The command path to execute
* @param baseOptions - Base spawn options to merge with (optional) * @param baseOptions - Base spawn options to merge with (optional)
* @returns Spawn options with correct shell setting * @returns Spawn options with correct shell setting
@@ -480,7 +487,7 @@ export function shouldUseShell(command: string): boolean {
* @example * @example
* ```typescript * ```typescript
* const opts = getSpawnOptions(claudeCmd, { cwd: '/project', env: {...} }); * const opts = getSpawnOptions(claudeCmd, { cwd: '/project', env: {...} });
* spawn(claudeCmd, ['--version'], opts); * spawn(getSpawnCommand(claudeCmd), ['--version'], opts);
* ``` * ```
*/ */
export function getSpawnOptions( export function getSpawnOptions(
@@ -505,3 +512,37 @@ export function getSpawnOptions(
shell: shouldUseShell(command), shell: shouldUseShell(command),
}; };
} }
/**
* Get the properly quoted command for use with spawn()
*
* For .cmd/.bat files on Windows with shell:true, the command path must be
* quoted to handle paths containing spaces correctly (e.g., C:\Users\OXFAM MONS\...).
*
* @param command - The command path to execute
* @returns The command (quoted if needed for .cmd/.bat files on Windows)
*
* @example
* ```typescript
* const cmd = getSpawnCommand(claudeCmd); // "C:\Users\OXFAM MONS\...\claude.cmd"
* const opts = getSpawnOptions(claudeCmd, { cwd: '/project', env: {...} });
* spawn(cmd, ['--version'], opts);
* ```
*/
export function getSpawnCommand(command: string): string {
// For .cmd/.bat files on Windows, quote the command to handle spaces
// The shell will parse the quoted path correctly
const trimmed = command.trim();
if (shouldUseShell(trimmed)) {
// Idempotent if already quoted
if (trimmed.startsWith('"') && trimmed.endsWith('"')) {
return trimmed;
}
return `"${trimmed}"`;
}
// For non-.cmd/.bat files, strip quotes if present (defensive: no double quotes with shell:false)
if (trimmed.startsWith('"') && trimmed.endsWith('"')) {
return trimmed.slice(1, -1);
}
return trimmed;
}
@@ -10,7 +10,7 @@ import { projectStore } from '../project-store';
import { parseEnvFile } from './utils'; import { parseEnvFile } from './utils';
import { getClaudeCliInvocation, getClaudeCliInvocationAsync } from '../claude-cli-utils'; import { getClaudeCliInvocation, getClaudeCliInvocationAsync } from '../claude-cli-utils';
import { debugError } from '../../shared/utils/debug-logger'; import { debugError } from '../../shared/utils/debug-logger';
import { getSpawnOptions } from '../env-utils'; import { getSpawnOptions, getSpawnCommand } from '../env-utils';
// GitLab environment variable keys // GitLab environment variable keys
const GITLAB_ENV_KEYS = { const GITLAB_ENV_KEYS = {
@@ -603,7 +603,7 @@ ${existingVars['GRAPHITI_DB_PATH'] ? `GRAPHITI_DB_PATH=${existingVars['GRAPHITI_
try { try {
// Check if Claude CLI is available and authenticated // Check if Claude CLI is available and authenticated
const result = await new Promise<ClaudeAuthResult>((resolve) => { const result = await new Promise<ClaudeAuthResult>((resolve) => {
const proc = spawn(claudeCmd, ['--version'], getSpawnOptions(claudeCmd, { const proc = spawn(getSpawnCommand(claudeCmd), ['--version'], getSpawnOptions(claudeCmd, {
cwd: project.path, cwd: project.path,
env: claudeEnv, env: claudeEnv,
})); }));
@@ -623,7 +623,7 @@ ${existingVars['GRAPHITI_DB_PATH'] ? `GRAPHITI_DB_PATH=${existingVars['GRAPHITI_
if (code === 0) { if (code === 0) {
// Claude CLI is available, check if authenticated // Claude CLI is available, check if authenticated
// Run a simple command that requires auth // Run a simple command that requires auth
const authCheck = spawn(claudeCmd, ['api', '--help'], getSpawnOptions(claudeCmd, { const authCheck = spawn(getSpawnCommand(claudeCmd), ['api', '--help'], getSpawnOptions(claudeCmd, {
cwd: project.path, cwd: project.path,
env: claudeEnv, env: claudeEnv,
})); }));
@@ -692,7 +692,7 @@ ${existingVars['GRAPHITI_DB_PATH'] ? `GRAPHITI_DB_PATH=${existingVars['GRAPHITI_
try { try {
// Run claude setup-token which will open browser for OAuth // Run claude setup-token which will open browser for OAuth
const result = await new Promise<ClaudeAuthResult>((resolve) => { const result = await new Promise<ClaudeAuthResult>((resolve) => {
const proc = spawn(claudeCmd, ['setup-token'], getSpawnOptions(claudeCmd, { const proc = spawn(getSpawnCommand(claudeCmd), ['setup-token'], getSpawnOptions(claudeCmd, {
cwd: project.path, cwd: project.path,
env: claudeEnv, env: claudeEnv,
stdio: 'inherit' // This allows the terminal to handle the interactive auth stdio: 'inherit' // This allows the terminal to handle the interactive auth