Skip to content

Commit bd64619

Browse files
committed
feat: enhance Windows MCP command quoting and add tests for cmd metacharacters
1 parent 75b7ae8 commit bd64619

2 files changed

Lines changed: 79 additions & 3 deletions

File tree

src/mcp/mcp-client.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ export function createMcpSpawnSpec(
427427
// On Windows, shell: true lets cmd.exe resolve the command via PATHEXT
428428
// (npx -> npx.cmd, etc.). Join command and args into a single string
429429
// with empty spawn args to avoid Node 24 DEP0190.
430-
// Only quote arguments that contain spaces or double-quotes to prevent
430+
// Only quote arguments that need protection from cmd.exe to prevent
431431
// double-wrapping by Node.js's own shell quoting.
432432
command: [command, ...args].map(quoteWindowsArgIfNeeded).join(" "),
433433
args: [],
@@ -444,7 +444,7 @@ export function createMcpSpawnSpec(
444444
}
445445

446446
function quoteWindowsArgIfNeeded(arg: string): string {
447-
if (arg.includes(" ") || arg.includes('"')) {
447+
if (/[\s"&|<>^()]/.test(arg)) {
448448
return `"${arg.replace(/(\\*)"/g, '$1$1\\"').replace(/\\+$/g, "$&$&")}"`;
449449
}
450450
return arg;

src/tests/mcp-client.test.ts

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { test } from "node:test";
22
import assert from "node:assert/strict";
3-
import { createMcpSpawnSpec } from "../mcp/mcp-client";
3+
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
4+
import { tmpdir } from "node:os";
5+
import path from "node:path";
6+
import { McpClient, createMcpSpawnSpec } from "../mcp/mcp-client";
47

58
test("createMcpSpawnSpec keeps non-Windows MCP launches shell-free", () => {
69
assert.deepEqual(createMcpSpawnSpec("npx", ["-y", "@playwright/mcp@latest"], "darwin"), {
@@ -32,3 +35,76 @@ test("createMcpSpawnSpec quotes Windows command paths and arguments", () => {
3235
);
3336
assert.deepEqual(spec.args, []);
3437
});
38+
39+
test("createMcpSpawnSpec quotes Windows args with cmd metacharacters", () => {
40+
const spec = createMcpSpawnSpec(
41+
"npx",
42+
[
43+
"-y",
44+
"some-mcp",
45+
"--url=https://example.test?a=1&b=2",
46+
"--pipe=a|b",
47+
"--redirect=<in>out",
48+
"--caret=^value",
49+
"--group=(value)",
50+
],
51+
"win32"
52+
);
53+
54+
assert.equal(
55+
spec.command,
56+
[
57+
"npx",
58+
"-y",
59+
"some-mcp",
60+
'"--url=https://example.test?a=1&b=2"',
61+
'"--pipe=a|b"',
62+
'"--redirect=<in>out"',
63+
'"--caret=^value"',
64+
'"--group=(value)"',
65+
].join(" ")
66+
);
67+
assert.deepEqual(spec.args, []);
68+
});
69+
70+
test("McpClient starts a PATH-resolved cmd MCP server on Windows", { skip: process.platform !== "win32" }, async () => {
71+
const serverDir = mkdtempSync(path.join(tmpdir(), "deepcode-mcp-probe-"));
72+
const originalPath = process.env.PATH;
73+
74+
writeFileSync(path.join(serverDir, "mcp-probe.cmd"), '@echo off\r\nnode "%~dp0mcp-probe-server.cjs"\r\n');
75+
writeFileSync(
76+
path.join(serverDir, "mcp-probe-server.cjs"),
77+
[
78+
'const readline = require("node:readline");',
79+
"const rl = readline.createInterface({ input: process.stdin });",
80+
"function send(message) { process.stdout.write(`${JSON.stringify(message)}\\n`); }",
81+
'rl.on("line", (line) => {',
82+
" const request = JSON.parse(line);",
83+
' if (request.method === "initialize") {',
84+
' send({ jsonrpc: "2.0", id: request.id, result: { protocolVersion: "2025-03-26", capabilities: {}, serverInfo: { name: "probe", version: "1.0.0" } } });',
85+
" return;",
86+
" }",
87+
' if (request.method === "tools/list") {',
88+
' send({ jsonrpc: "2.0", id: request.id, result: { tools: [{ name: "probe_tool", inputSchema: { type: "object", properties: {} } }] } });',
89+
" return;",
90+
" }",
91+
"});",
92+
].join("\n")
93+
);
94+
95+
process.env.PATH = `${serverDir}${path.delimiter}${originalPath ?? ""}`;
96+
const client = new McpClient("probe", "mcp-probe", []);
97+
98+
try {
99+
await client.connect(5_000);
100+
const tools = await client.listTools(5_000);
101+
assert.deepEqual(
102+
tools.map((tool) => tool.name),
103+
["probe_tool"]
104+
);
105+
} finally {
106+
client.disconnect();
107+
process.env.PATH = originalPath;
108+
rmSync(serverDir, { recursive: true, force: true });
109+
}
110+
});

0 commit comments

Comments
 (0)