Skip to content

Commit 9a78558

Browse files
committed
fix: properly quote MCP server paths containing ampersands on Windows (#4933)
1 parent b31250d commit 9a78558

File tree

2 files changed

+63
-3
lines changed

2 files changed

+63
-3
lines changed

src/services/mcp/McpHub.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ export class McpHub {
592592
const command = isWindows && !isAlreadyWrapped ? "cmd.exe" : configInjected.command
593593
const args =
594594
isWindows && !isAlreadyWrapped
595-
? ["/c", configInjected.command, ...(configInjected.args || [])]
595+
? ["/c", `"${configInjected.command}"`, ...(configInjected.args || [])]
596596
: configInjected.args
597597

598598
transport = new StdioClientTransport({

src/services/mcp/__tests__/McpHub.spec.ts

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,7 @@ describe("McpHub", () => {
776776
expect(StdioClientTransport).toHaveBeenCalledWith(
777777
expect.objectContaining({
778778
command: "cmd.exe",
779-
args: ["/c", "npx", "-y", "@modelcontextprotocol/server-filesystem", "/test/path"],
779+
args: ["/c", '"npx"', "-y", "@modelcontextprotocol/server-filesystem", "/test/path"],
780780
}),
781781
)
782782
})
@@ -975,7 +975,7 @@ describe("McpHub", () => {
975975
expect(StdioClientTransport).toHaveBeenCalledWith(
976976
expect.objectContaining({
977977
command: "cmd.exe",
978-
args: ["/c", "npx", "-y", "@modelcontextprotocol/server-example"],
978+
args: ["/c", '"npx"', "-y", "@modelcontextprotocol/server-example"],
979979
env: expect.objectContaining({
980980
FNM_DIR: "C:\\Users\\test\\.fnm",
981981
FNM_NODE_DIST_MIRROR: "https://nodejs.org/dist",
@@ -1046,5 +1046,65 @@ describe("McpHub", () => {
10461046
}),
10471047
)
10481048
})
1049+
1050+
it("should properly quote commands with special characters like ampersand", async () => {
1051+
// Mock Windows platform
1052+
Object.defineProperty(process, "platform", {
1053+
value: "win32",
1054+
writable: true,
1055+
enumerable: true,
1056+
configurable: true,
1057+
})
1058+
1059+
// Mock StdioClientTransport
1060+
const mockTransport = {
1061+
start: vi.fn().mockResolvedValue(undefined),
1062+
close: vi.fn().mockResolvedValue(undefined),
1063+
stderr: {
1064+
on: vi.fn(),
1065+
},
1066+
onerror: null,
1067+
onclose: null,
1068+
}
1069+
1070+
StdioClientTransport.mockImplementation((config: any) => {
1071+
// Store the config for verification
1072+
return mockTransport
1073+
})
1074+
1075+
// Mock Client
1076+
Client.mockImplementation(() => ({
1077+
connect: vi.fn().mockResolvedValue(undefined),
1078+
close: vi.fn().mockResolvedValue(undefined),
1079+
getInstructions: vi.fn().mockReturnValue("test instructions"),
1080+
request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }),
1081+
}))
1082+
1083+
// Create a new McpHub instance
1084+
const mcpHub = new McpHub(mockProvider as ClineProvider)
1085+
1086+
// Mock file system operations
1087+
vi.mocked(fs.readFile).mockResolvedValue(
1088+
JSON.stringify({
1089+
mcpServers: {
1090+
"test-server": {
1091+
command: "c:\\path with &\\mymcp\\mcp.exe",
1092+
args: ["--verbose"],
1093+
},
1094+
},
1095+
}),
1096+
)
1097+
1098+
// Initialize servers (this will trigger connectToServer)
1099+
await mcpHub["initializeGlobalMcpServers"]()
1100+
1101+
// Verify StdioClientTransport was called with properly quoted command
1102+
expect(StdioClientTransport).toHaveBeenCalledWith(
1103+
expect.objectContaining({
1104+
command: "cmd.exe",
1105+
args: ["/c", '"c:\\path with &\\mymcp\\mcp.exe"', "--verbose"],
1106+
}),
1107+
)
1108+
})
10491109
})
10501110
})

0 commit comments

Comments
 (0)