Merge pull request #466 from kaitranntt/kai/fix/doctor-claude-detection

fix(doctor): use cmd.exe compatible quoting for Windows shell execution

src/utils/shell-executor.ts

@@ -9,33 +9,32 @@ import { ErrorManager } from './error-manager';
 import { getWebSearchHookEnv } from './websearch-manager';
 
 /**
- * Escape arguments for shell execution (Windows compatibility)
- * Handles PowerShell special characters: backticks, $variables, double quotes
+ * Escape arguments for shell execution (cross-platform)
+ *
+ * IMPORTANT: On Windows, spawn({ shell: true }) uses cmd.exe by default,
+ * NOT PowerShell. cmd.exe does NOT recognize single quotes as string delimiters.
+ * We must use double quotes for cmd.exe compatibility.
  */
 export function escapeShellArg(arg: string): string {
   const isWindows = process.platform === 'win32';
 
   if (isWindows) {
-    // PowerShell: Use single quotes for literal strings to prevent variable expansion
-    // Escape single quotes by doubling them (PowerShell syntax)
-    // Fallback to double quotes with escapes if single quotes present
-    if (arg.includes("'")) {
-      // Contains single quote - use double quotes with escape sequences
-      return (
-        '"' +
-        String(arg)
-          .replace(/\$/g, '`$') // Escape $ to prevent variable expansion
-          .replace(/`/g, '``') // Escape backticks
-          .replace(/"/g, '`"') + // Escape double quotes
-        '"'
-      );
-    } else {
-      // No single quotes - use single quotes for literal string (safest)
-      return "'" + String(arg) + "'";
-    }
+    // cmd.exe: Use double quotes, escape inner double quotes by doubling them
+    // cmd.exe interprets "" as escaped double quote inside quoted string
+    // Strip newlines/tabs that can break cmd.exe parsing
+    return (
+      '"' +
+      String(arg)
+        .replace(/[\r\n\t]/g, ' ') // Replace newlines/tabs with space
+        .replace(/%/g, '%%') // Escape percent signs
+        .replace(/\^/g, '^^') // Escape carets
+        .replace(/!/g, '^^!') // Escape exclamation marks (delayed expansion)
+        .replace(/"/g, '""') + // Escape quotes
+      '"'
+    );
   } else {
     // Unix/macOS: Double quotes with escaped inner quotes
-    return '"' + String(arg).replace(/"/g, '""') + '"';
+    return '"' + String(arg).replace(/"/g, '\\"') + '"';
   }
 }
 

tests/unit/utils/shell-executor.test.ts

@@ -0,0 +1,84 @@
+import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
+
+// We need to mock process.platform for cross-platform testing
+const originalPlatform = process.platform;
+
+describe('escapeShellArg', () => {
+  describe('Unix (non-Windows)', () => {
+    beforeEach(() => {
+      Object.defineProperty(process, 'platform', { value: 'linux' });
+    });
+    afterEach(() => {
+      Object.defineProperty(process, 'platform', { value: originalPlatform });
+    });
+
+    it('wraps argument in double quotes', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('arg')).toBe('"arg"');
+    });
+
+    it('escapes inner double quotes with backslash', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('say "hello"')).toBe('"say \\"hello\\""');
+    });
+
+    it('handles paths with spaces', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('/path/to/my file')).toBe('"/path/to/my file"');
+    });
+
+    it('handles empty string', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('')).toBe('""');
+    });
+  });
+
+  describe('Windows (cmd.exe)', () => {
+    beforeEach(() => {
+      Object.defineProperty(process, 'platform', { value: 'win32' });
+    });
+    afterEach(() => {
+      Object.defineProperty(process, 'platform', { value: originalPlatform });
+    });
+
+    it('wraps argument in double quotes', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('arg')).toBe('"arg"');
+    });
+
+    it('escapes inner double quotes by doubling them', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('say "hello"')).toBe('"say ""hello"""');
+    });
+
+    it('escapes percent signs', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('%PATH%')).toBe('"%%PATH%%"');
+    });
+
+    it('escapes caret characters', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('a^b')).toBe('"a^^b"');
+    });
+
+    it('replaces newlines with spaces', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('line1\nline2')).toBe('"line1 line2"');
+    });
+
+    it('replaces tabs with spaces', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('col1\tcol2')).toBe('"col1 col2"');
+    });
+
+    it('handles Windows paths with spaces', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('C:\\Program Files\\App')).toBe('"C:\\Program Files\\App"');
+    });
+
+    it('escapes exclamation marks for delayed expansion', async () => {
+      const { escapeShellArg } = await import('../../../src/utils/shell-executor');
+      expect(escapeShellArg('hello!')).toBe('"hello^^!"');
+    });
+  });
+});