From 085e5054f902f573bf696353747eb84231191035 Mon Sep 17 00:00:00 2001 From: Peter Vyboch Date: Thu, 2 Apr 2026 20:02:19 +0200 Subject: [PATCH] fix: preserve quoted args with spaces when launching via CLI --- cli/__tests__/cli.test.ts | 27 ++++++ client/bin/start.js | 2 +- package-lock.json | 3 + server/__tests__/config-args.test.ts | 137 +++++++++++++++++++++++++++ server/jest.config.cjs | 14 +++ server/package.json | 6 +- server/src/index.ts | 26 ++++- server/tsconfig.test.json | 8 ++ 8 files changed, 218 insertions(+), 5 deletions(-) create mode 100644 server/__tests__/config-args.test.ts create mode 100644 server/jest.config.cjs create mode 100644 server/tsconfig.test.json diff --git a/cli/__tests__/cli.test.ts b/cli/__tests__/cli.test.ts index b263f618c..a3498a3ae 100644 --- a/cli/__tests__/cli.test.ts +++ b/cli/__tests__/cli.test.ts @@ -868,4 +868,31 @@ describe("CLI Tests", () => { expectCliFailure(result); }); }); + + describe("Args With Spaces", () => { + it("passes an arg containing spaces to the child process as a single argv element", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + // Extra arg passed to the test MCP server — contains a space + "--description", + "get todays date", + "--cli", + "--method", + "resources/read", + "--uri", + "test://argv", + ]); + + expectCliSuccess(result); + const json = expectValidJson(result); + const argv: string[] = JSON.parse(json.contents[0].text); + + // "get todays date" must appear as one element, not split into three + expect(argv).toContain("get todays date"); + expect(argv).not.toContain("get"); + expect(argv).not.toContain("todays"); + }); + }); }); diff --git a/client/bin/start.js b/client/bin/start.js index 7e7e013af..8d44f0fb2 100755 --- a/client/bin/start.js +++ b/client/bin/start.js @@ -106,7 +106,7 @@ async function startProdServer(serverOptions) { inspectorServerPath, ...(command ? [`--command=${command}`] : []), ...(mcpServerArgs && mcpServerArgs.length > 0 - ? [`--args=${mcpServerArgs.join(" ")}`] + ? [`--args=${JSON.stringify(mcpServerArgs)}`] : []), ...(transport ? [`--transport=${transport}`] : []), ...(serverUrl ? [`--server-url=${serverUrl}`] : []), diff --git a/package-lock.json b/package-lock.json index 19b73655b..a3ec906da 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14022,8 +14022,11 @@ "devDependencies": { "@types/cors": "^2.8.19", "@types/express": "^5.0.0", + "@types/jest": "^29.5.14", "@types/shell-quote": "^1.7.5", "@types/ws": "^8.5.12", + "jest": "^29.7.0", + "ts-jest": "^29.4.0", "tsx": "^4.19.0", "typescript": "^5.6.2" } diff --git a/server/__tests__/config-args.test.ts b/server/__tests__/config-args.test.ts new file mode 100644 index 000000000..da8e4c789 --- /dev/null +++ b/server/__tests__/config-args.test.ts @@ -0,0 +1,137 @@ +/** + * Integration tests for the /config endpoint's args handling. + * + * These tests spawn the real proxy server process and make actual HTTP requests + * to verify that args containing spaces survive the serialisation round-trip + * introduced by the start.js → server path. + */ + +import { spawn, type ChildProcess } from "child_process"; +import { resolve } from "path"; + +const SERVER_SRC = resolve(__dirname, "../src/index.ts"); + +// Fixed credentials — the server is bound to localhost only and killed after each test. +const AUTH_TOKEN = "test-token"; +const CLIENT_PORT = "17274"; +const ORIGIN = `http://localhost:${CLIENT_PORT}`; + +// Use a different port per test to avoid EADDRINUSE across sequential runs. +let portSeed = 17280; + +interface ServerHandle { + port: number; + process: ChildProcess; +} + +async function startServer(extraArgs: string[] = []): Promise { + const port = portSeed++; + + const proc = spawn("tsx", [SERVER_SRC, ...extraArgs], { + env: { + ...process.env, + SERVER_PORT: String(port), + CLIENT_PORT, + MCP_PROXY_AUTH_TOKEN: AUTH_TOKEN, + ALLOWED_ORIGINS: ORIGIN, + }, + stdio: "pipe", + }); + + await waitForHealth(port); + return { port, process: proc }; +} + +async function waitForHealth(port: number, timeoutMs = 5000): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + try { + const res = await fetch(`http://localhost:${port}/health`); + if (res.ok) return; + } catch { + // server not up yet + } + await new Promise((r) => setTimeout(r, 100)); + } + throw new Error( + `Proxy server on port ${port} did not become healthy in ${timeoutMs}ms`, + ); +} + +async function fetchConfig(handle: ServerHandle): Promise<{ + defaultArgs: string; + defaultCommand: string; +}> { + const res = await fetch(`http://localhost:${handle.port}/config`, { + headers: { + Origin: ORIGIN, + "x-mcp-proxy-auth": `Bearer ${AUTH_TOKEN}`, + }, + }); + if (!res.ok) throw new Error(`/config returned ${res.status}`); + return res.json() as Promise<{ defaultArgs: string; defaultCommand: string }>; +} + +let currentHandle: ServerHandle | null = null; + +afterEach(() => { + currentHandle?.process.kill(); + currentHandle = null; +}); + +describe("proxy server /config: args passed from start.js", () => { + it("converts JSON-array args (new start.js format) to a shell-quoted string", async () => { + // start.js now does: `--args=${JSON.stringify(mcpServerArgs)}` + const args = ["--description", "get todays date", "--command", "date"]; + currentHandle = await startServer([`--args=${JSON.stringify(args)}`]); + + const config = await fetchConfig(currentHandle); + + // The /config endpoint must shell-quote the array so the client UI can + // display it and shellParseArgs can round-trip it correctly. + expect(config.defaultArgs).toBe( + "--description 'get todays date' --command date", + ); + }); + + it("passes a legacy plain shell string through unchanged (backward compat)", async () => { + // Direct invocations of the server binary that pass --args as a plain + // shell string must continue to work. + currentHandle = await startServer([ + "--args=--description 'get todays date' --command date", + ]); + + const config = await fetchConfig(currentHandle); + + expect(config.defaultArgs).toBe( + "--description 'get todays date' --command date", + ); + }); + + it("returns an empty string when no --args flag is given", async () => { + currentHandle = await startServer([]); + const config = await fetchConfig(currentHandle); + expect(config.defaultArgs).toBe(""); + }); + + it("handles args with backslashes", async () => { + const args = ["--path", "C:\\Users\\foo"]; + currentHandle = await startServer([`--args=${JSON.stringify(args)}`]); + const config = await fetchConfig(currentHandle); + + // Verify the round-trip: parse the shell-quoted string back into an array + const { parse } = await import("shell-quote"); + const parsed = parse(config.defaultArgs) as string[]; + expect(parsed).toEqual(args); + }); + + it("handles args that look like JSON themselves", async () => { + const args = ["--config", '{"key":"val"}']; + currentHandle = await startServer([`--args=${JSON.stringify(args)}`]); + const config = await fetchConfig(currentHandle); + + const { parse } = await import("shell-quote"); + const parsed = parse(config.defaultArgs) as string[]; + expect(parsed).toEqual(args); + }); +}); diff --git a/server/jest.config.cjs b/server/jest.config.cjs new file mode 100644 index 000000000..cafe1ecc3 --- /dev/null +++ b/server/jest.config.cjs @@ -0,0 +1,14 @@ +module.exports = { + preset: "ts-jest", + testEnvironment: "node", + transform: { + "^.+\\.tsx?$": [ + "ts-jest", + { + tsconfig: "tsconfig.test.json", + }, + ], + }, + transformIgnorePatterns: ["node_modules/(?!(@modelcontextprotocol)/)"], + testRegex: "(/__tests__/.*|(\\.|/)(test|spec))\\.(jsx?|tsx?)$", +}; diff --git a/server/package.json b/server/package.json index c674018a8..dc54504ec 100644 --- a/server/package.json +++ b/server/package.json @@ -19,13 +19,17 @@ "build": "tsc && shx cp -R static build", "start": "node build/index.js", "dev": "tsx watch --clear-screen=false src/index.ts", - "dev:windows": "tsx watch --clear-screen=false src/index.ts < NUL" + "dev:windows": "tsx watch --clear-screen=false src/index.ts < NUL", + "test": "jest --config jest.config.cjs" }, "devDependencies": { "@types/cors": "^2.8.19", "@types/express": "^5.0.0", "@types/shell-quote": "^1.7.5", "@types/ws": "^8.5.12", + "@types/jest": "^29.5.14", + "jest": "^29.7.0", + "ts-jest": "^29.4.0", "tsx": "^4.19.0", "typescript": "^5.6.2" }, diff --git a/server/src/index.ts b/server/src/index.ts index 4d1fffa29..852d5002a 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -2,7 +2,7 @@ import cors from "cors"; import { parseArgs } from "node:util"; -import { parse as shellParseArgs } from "shell-quote"; +import { parse as shellParseArgs, quote as shellQuoteArgs } from "shell-quote"; import nodeFetch, { Headers as NodeHeaders } from "node-fetch"; // Type-compatible wrappers for node-fetch to work with browser-style types @@ -375,7 +375,16 @@ const createTransport = async ( if (transportType === "stdio") { const command = (query.command as string).trim(); - const origArgs = shellParseArgs(query.args as string) as string[]; + const rawArgs = query.args as string; + let origArgs: string[]; + try { + const parsed = JSON.parse(rawArgs); + origArgs = Array.isArray(parsed) + ? (parsed as string[]) + : (shellParseArgs(rawArgs) as string[]); + } catch { + origArgs = shellParseArgs(rawArgs) as string[]; + } const queryEnv = query.env ? JSON.parse(query.env as string) : {}; const env = { ...defaultEnvironment, ...process.env, ...queryEnv }; @@ -786,7 +795,18 @@ app.get("/config", originValidationMiddleware, authMiddleware, (req, res) => { res.json({ defaultEnvironment, defaultCommand: values.command, - defaultArgs: values.args, + defaultArgs: (() => { + if (!values.args) return values.args; + try { + const parsed = JSON.parse(values.args); + if (Array.isArray(parsed)) { + return shellQuoteArgs(parsed); + } + } catch { + // Not JSON — legacy shell string, pass through unchanged + } + return values.args; + })(), defaultTransport: values.transport, defaultServerUrl: values["server-url"], }); diff --git a/server/tsconfig.test.json b/server/tsconfig.test.json new file mode 100644 index 000000000..34207388d --- /dev/null +++ b/server/tsconfig.test.json @@ -0,0 +1,8 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "module": "CommonJS", + "moduleResolution": "Node" + }, + "include": ["__tests__/**/*", "src/**/*"] +}