fix(mcp): use shell mode for Windows command spawning (#572)

* fix(mcp): use shell mode for Windows command spawning

On Windows, commands like `npx` are actually batch scripts (`npx.cmd`).
When spawning these without `shell: true`, Node.js fails to execute them
properly because it tries to run them as direct executables.

This fix adds `shell: true` on Windows platform for MCP server connection
testing, allowing command-based MCP servers (like perplexity-ask) to
start correctly.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(mcp): add shell metacharacter validation for Windows

Addresses security review feedback by adding validation for shell
metacharacters (&, |, >, <, ^, %, ;, $, `, \n, \r) in args when
running on Windows with shell: true.

This prevents potential command injection via unsanitized args
that could break out of the intended command using shell operators.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(mcp): update error messages to reflect both validation types

Updated error messages from "Args contain dangerous interpreter flags"
to "Args contain dangerous flags or shell metacharacters" to accurately
reflect that areArgsSafe() now validates both dangerous interpreter
flags and shell metacharacters on Windows.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Andy
2026-01-05 01:15:32 +08:00
committed by GitHub
parent b72031241d
commit e1e8943051
@@ -28,6 +28,12 @@ const DANGEROUS_FLAGS = new Set([
'--require', '-r'
]);
/**
* Defense-in-depth: Shell metacharacters that could enable command injection
* when shell: true is used on Windows
*/
const SHELL_METACHARACTERS = ['&', '|', '>', '<', '^', '%', ';', '$', '`', '\n', '\r'];
/**
* Validate that a command is in the safe allowlist
*/
@@ -39,11 +45,22 @@ function isCommandSafe(command: string | undefined): boolean {
}
/**
* Validate that args don't contain dangerous interpreter flags
* Validate that args don't contain dangerous interpreter flags or shell metacharacters
*/
function areArgsSafe(args: string[] | undefined): boolean {
if (!args || args.length === 0) return true;
return !args.some(arg => DANGEROUS_FLAGS.has(arg));
// Check for dangerous interpreter flags
if (args.some(arg => DANGEROUS_FLAGS.has(arg))) return false;
// On Windows with shell: true, check for shell metacharacters that could enable injection
if (process.platform === 'win32') {
if (args.some(arg => SHELL_METACHARACTERS.some(char => arg.includes(char)))) {
return false;
}
}
return true;
}
/**
@@ -171,7 +188,7 @@ async function checkCommandHealth(server: CustomMcpServer, startTime: number): P
return resolve({
serverId: server.id,
status: 'unhealthy',
message: 'Args contain dangerous interpreter flags',
message: 'Args contain dangerous flags or shell metacharacters',
checkedAt: new Date().toISOString(),
});
}
@@ -394,14 +411,17 @@ async function testCommandConnection(server: CustomMcpServer, startTime: number)
return resolve({
serverId: server.id,
success: false,
message: 'Args contain dangerous interpreter flags',
message: 'Args contain dangerous flags or shell metacharacters',
});
}
const args = server.args || [];
// On Windows, use shell: true to properly handle .cmd/.bat scripts like npx
const proc = spawn(server.command!, args, {
stdio: ['pipe', 'pipe', 'pipe'],
timeout: 15000, // OS-level timeout for reliable process termination
shell: process.platform === 'win32', // Required for Windows to run npx.cmd
});
let stdout = '';