diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 0b6af7747d..e9145e524b 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -592,7 +592,7 @@ export class McpHub { const command = isWindows && !isAlreadyWrapped ? "cmd.exe" : configInjected.command const args = isWindows && !isAlreadyWrapped - ? ["/c", configInjected.command, ...(configInjected.args || [])] + ? ["/c", `"${configInjected.command}"`, ...(configInjected.args || [])] : configInjected.args transport = new StdioClientTransport({ diff --git a/src/services/mcp/__tests__/McpHub.spec.ts b/src/services/mcp/__tests__/McpHub.spec.ts index eaa9af908c..4e796e56f9 100644 --- a/src/services/mcp/__tests__/McpHub.spec.ts +++ b/src/services/mcp/__tests__/McpHub.spec.ts @@ -776,7 +776,7 @@ describe("McpHub", () => { expect(StdioClientTransport).toHaveBeenCalledWith( expect.objectContaining({ command: "cmd.exe", - args: ["/c", "npx", "-y", "@modelcontextprotocol/server-filesystem", "/test/path"], + args: ["/c", '"npx"', "-y", "@modelcontextprotocol/server-filesystem", "/test/path"], }), ) }) @@ -975,7 +975,7 @@ describe("McpHub", () => { expect(StdioClientTransport).toHaveBeenCalledWith( expect.objectContaining({ command: "cmd.exe", - args: ["/c", "npx", "-y", "@modelcontextprotocol/server-example"], + args: ["/c", '"npx"', "-y", "@modelcontextprotocol/server-example"], env: expect.objectContaining({ FNM_DIR: "C:\\Users\\test\\.fnm", FNM_NODE_DIST_MIRROR: "https://nodejs.org/dist", @@ -1046,5 +1046,65 @@ describe("McpHub", () => { }), ) }) + + it("should properly quote commands with special characters like ampersand", async () => { + // Mock Windows platform + Object.defineProperty(process, "platform", { + value: "win32", + writable: true, + enumerable: true, + configurable: true, + }) + + // Mock StdioClientTransport + const mockTransport = { + start: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + stderr: { + on: vi.fn(), + }, + onerror: null, + onclose: null, + } + + StdioClientTransport.mockImplementation((config: any) => { + // Store the config for verification + return mockTransport + }) + + // Mock Client + Client.mockImplementation(() => ({ + connect: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + getInstructions: vi.fn().mockReturnValue("test instructions"), + request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }), + })) + + // Create a new McpHub instance + const mcpHub = new McpHub(mockProvider as ClineProvider) + + // Mock file system operations + vi.mocked(fs.readFile).mockResolvedValue( + JSON.stringify({ + mcpServers: { + "test-server": { + command: "c:\\path with &\\mymcp\\mcp.exe", + args: ["--verbose"], + }, + }, + }), + ) + + // Initialize servers (this will trigger connectToServer) + await mcpHub["initializeGlobalMcpServers"]() + + // Verify StdioClientTransport was called with properly quoted command + expect(StdioClientTransport).toHaveBeenCalledWith( + expect.objectContaining({ + command: "cmd.exe", + args: ["/c", '"c:\\path with &\\mymcp\\mcp.exe"', "--verbose"], + }), + ) + }) }) })