diff --git a/scripts/hooks/mcp-health-check.js b/scripts/hooks/mcp-health-check.js index b880d7d9..475e4aa7 100644 --- a/scripts/hooks/mcp-health-check.js +++ b/scripts/hooks/mcp-health-check.js @@ -338,6 +338,21 @@ function probeCommandServer(serverName, config) { // through shell mode. const UNSAFE_SHELL_CHARS = /[&|<>^%!()\s;]/; + // When spawning via cmd.exe (shell:true) on Windows, Node concatenates + // command + args WITHOUT quoting (DEP0190). An arg containing a space — + // such as a path under "C:\Program Files" — gets re-split by cmd.exe. + // Build a properly-quoted command line instead and pass it as a single + // string with no args array, so cmd.exe sees each token as one unit. + function quoteWin(token) { + // If the token has no characters that need quoting, return it as-is. + if (!/[\s"&|<>^%!();]/.test(token)) { + return token; + } + // Escape embedded double quotes by doubling them, then wrap in double + // quotes. cmd.exe uses "" as an escaped quote inside a quoted string. + return '"' + token.replace(/"/g, '""') + '"'; + } + function attempt(idx) { const tryCommand = candidates[idx]; const isLast = idx + 1 >= candidates.length; @@ -375,12 +390,26 @@ function probeCommandServer(serverName, config) { let child; try { - child = spawn(tryCommand, args, { - env: mergedEnv, - cwd: process.cwd(), - stdio: ['pipe', 'ignore', 'pipe'], - shell: useShell - }); + if (useShell) { + // Build a single quoted command line for cmd.exe. Passing an args + // array with shell:true causes Node to concatenate without quoting + // (DEP0190), which splits space-containing args (e.g. paths under + // "C:\Program Files") at every space boundary. + const quotedCmdline = [tryCommand, ...args].map(quoteWin).join(' '); + child = spawn(quotedCmdline, { + env: mergedEnv, + cwd: process.cwd(), + stdio: ['pipe', 'ignore', 'pipe'], + shell: true + }); + } else { + child = spawn(tryCommand, args, { + env: mergedEnv, + cwd: process.cwd(), + stdio: ['pipe', 'ignore', 'pipe'], + shell: false + }); + } } catch (error) { if ((error.code === 'ENOENT' || error.code === 'EINVAL') && !isLast) { retryNext(); diff --git a/tests/hooks/mcp-health-check.test.js b/tests/hooks/mcp-health-check.test.js index 1fb56cfc..fa05fa67 100644 --- a/tests/hooks/mcp-health-check.test.js +++ b/tests/hooks/mcp-health-check.test.js @@ -1121,6 +1121,77 @@ async function runTests() { } })) passed++; else failed++; + // Windows-only: when a .cmd shim is probed via shell:true, args that contain + // spaces (e.g. paths under "C:\Program Files") must be passed as a single + // quoted token, not split by cmd.exe at every space boundary. + if (process.platform === 'win32') { + if (await asyncTest('windows: .cmd probe preserves space-containing args as single tokens', async () => { + const tempDir = createTempDir(); + const binDir = path.join(tempDir, 'bin'); + const configPath = path.join(tempDir, 'claude.json'); + const statePath = path.join(tempDir, 'mcp-health.json'); + + fs.mkdirSync(binDir, { recursive: true }); + + // This .cmd script writes its first argument verbatim to stderr and then + // keeps running so the probe can time out (= healthy). We inspect stderr + // to verify cmd.exe received the spaced path as one argument, not split. + const cmdPath = path.join(binDir, 'spacedarg.cmd'); + fs.writeFileSync( + cmdPath, + ['@echo off', 'echo ARG1=[%1] 1>&2', 'node -e "setInterval(()=>{},1000)"', ''].join('\r\n') + ); + + // A path containing a space — the canonical trigger for the DEP0190 bug. + const spacedPath = 'C:\\Program Files\\Some Server\\server.exe'; + + try { + writeConfig(configPath, { + mcpServers: { + spacedarg: { + command: 'spacedarg', + args: ['--codesys-path', spacedPath] + } + } + }); + + const input = { tool_name: 'mcp__spacedarg__ping', tool_input: {} }; + const result = runHook(input, { + CLAUDE_HOOK_EVENT_NAME: 'PreToolUse', + ECC_MCP_CONFIG_PATH: configPath, + ECC_MCP_HEALTH_STATE_PATH: statePath, + ECC_MCP_HEALTH_TIMEOUT_MS: '800', + PATH: `${binDir}${path.delimiter}${process.env.PATH || ''}` + }); + + assert.strictEqual( + result.code, + 0, + `Expected .cmd probe with spaced arg to succeed: ${hookFailureDetails(result, statePath)}` + ); + + const state = readState(statePath); + assert.strictEqual( + state.servers.spacedarg.status, + 'healthy', + 'Expected server with space-containing arg to be marked healthy' + ); + + // The .cmd echo writes its first positional (%1). If the path was split + // at the space, %1 would be "C:\Program" (no quotes, no "Files" part). + // If properly quoted, %1 is the full quoted path. + assert.ok( + !result.stderr.includes('ARG1=[C:\\Program]'), + `Space-containing arg was split by cmd.exe (DEP0190 bug still present). stderr: ${result.stderr}` + ); + } finally { + cleanupTempDir(tempDir); + } + })) passed++; else failed++; + } else { + console.log(' - skipped: windows: .cmd probe preserves space-containing args as single tokens (non-Windows)'); + } + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); }