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:
@@ -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 = '';
|
||||
|
||||
Reference in New Issue
Block a user