From 393e9d700eb1ec128e4b2630fa87486b44c8abde Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 18:55:13 -0700 Subject: [PATCH 1/4] test(trust): quote whitespace argv in shared MCP trust prompt helper --- tests/unit/exec/runner.test.ts | 51 ++++++++++++++++++ tests/unit/project-trust.test.ts | 51 ++++++++++++++++++ .../unit/tui/mcp-trust-prompt-parity.test.ts | 54 +++++++++++++++++++ 3 files changed, 156 insertions(+) create mode 100644 tests/unit/tui/mcp-trust-prompt-parity.test.ts diff --git a/tests/unit/exec/runner.test.ts b/tests/unit/exec/runner.test.ts index 264f9382f..073d80ba0 100644 --- a/tests/unit/exec/runner.test.ts +++ b/tests/unit/exec/runner.test.ts @@ -124,6 +124,57 @@ describe("exec MCP trust prompt", () => { }); }); +describe("exec MCP trust prompt argv boundaries", () => { + test("quotes an arg containing whitespace", () => { + expect( + formatExecMcpTrustQuestion({ + name: "notes", + command: "server", + args: ["--dir", "/tmp/my work"], + }), + ).toBe( + 'Trust local MCP server "notes" for this project?\nCommand: server --dir "/tmp/my work"', + ); + }); + + test("renders one spaced arg distinctly from two args", () => { + const one = formatExecMcpTrustQuestion({ + name: "s", + command: "run", + args: ["a b"], + }); + const two = formatExecMcpTrustQuestion({ + name: "s", + command: "run", + args: ["a", "b"], + }); + expect(one).toContain('"a b"'); + expect(one).not.toBe(two); + }); + + test("quotes empty args so they stay visible", () => { + const question = formatExecMcpTrustQuestion({ + name: "s", + command: "run", + args: [""], + }); + expect(question).toContain('""'); + expect(question).not.toBe( + formatExecMcpTrustQuestion({ name: "s", command: "run", args: [] }), + ); + }); + + test("escapes quotes inside a quoted arg", () => { + expect( + formatExecMcpTrustQuestion({ + name: "s", + command: "run", + args: ['say "hi"'], + }), + ).toContain('"say \\"hi\\""'); + }); +}); + describe("formatCaughtError", () => { test("prefers Error.message and stringifies other values", () => { expect(formatCaughtError(new Error("disk full"))).toBe("disk full"); diff --git a/tests/unit/project-trust.test.ts b/tests/unit/project-trust.test.ts index a4d121d4d..2488fea34 100644 --- a/tests/unit/project-trust.test.ts +++ b/tests/unit/project-trust.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { filterMcpServersForConnect, + formatMcpTrustQuestion, isMcpServerTrusted, isPluginTrusted, loadProjectTrust, @@ -358,6 +359,56 @@ describe("project-trust", () => { } }); + test("trust question quotes whitespace args so argv boundaries stay visible", () => { + const one = formatMcpTrustQuestion({ + name: "s", + command: "run", + args: ["a b"], + }); + const two = formatMcpTrustQuestion({ + name: "s", + command: "run", + args: ["a", "b"], + }); + expect(one).toBe( + 'Trust local MCP server "s" for this project?\nCommand: run "a b"', + ); + expect(one).not.toBe(two); + }); + + test("trust question leaves plain args unquoted and hides secrets", () => { + expect( + formatMcpTrustQuestion({ + name: "filesystem", + command: "npx", + args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp/work"], + }), + ).toBe( + 'Trust local MCP server "filesystem" for this project?\nCommand: npx -y @modelcontextprotocol/server-filesystem /tmp/work', + ); + const question = formatMcpTrustQuestion({ + name: "private", + command: "private-server", + env: { API_TOKEN: "super-secret" }, + }); + expect(question).toBe( + 'Trust local MCP server "private" for this project?\nCommand: private-server', + ); + expect(question).not.toContain("super-secret"); + }); + + test("trust question shows an HTTP server URL", () => { + expect( + formatMcpTrustQuestion({ + name: "remote", + type: "http", + url: "https://mcp.example.test/api", + }), + ).toBe( + 'Trust local MCP server "remote" for this project?\nURL: https://mcp.example.test/api', + ); + }); + test("readProjectTrustStore: malformed file with wrong types, missing fields, and extra fields drops bad entries and ignores unknown keys", async () => { const { cwd, home, cleanup } = await scratch(); try { diff --git a/tests/unit/tui/mcp-trust-prompt-parity.test.ts b/tests/unit/tui/mcp-trust-prompt-parity.test.ts new file mode 100644 index 000000000..371775d6b --- /dev/null +++ b/tests/unit/tui/mcp-trust-prompt-parity.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, test } from "bun:test"; +import type { MCPServerConfig } from "../../../src/config/settings.js"; +import { formatExecMcpTrustQuestion } from "../../../src/exec/runner.js"; +import { formatTuiMcpTrustQuestion } from "../../../src/tui/runner/session.js"; + +const parityCases: MCPServerConfig[] = [ + { name: "plain-stdio", command: "node", args: ["server.js", "--port", "3000"] }, + { name: "no-args", command: "node" }, + { name: "empty-args", command: "node", args: [] }, + { name: "spaced-path", command: "server", args: ["--dir", "/tmp/my work"] }, + { name: "one-spaced-arg", command: "run", args: ["a b"] }, + { name: "two-plain-args", command: "run", args: ["a", "b"] }, + { name: "tab-arg", command: "run", args: ["a\tb"] }, + { name: "quoted-arg", command: "run", args: ['say "hi"'] }, + { name: "empty-string-arg", command: "run", args: [""] }, + { + name: "with-secrets", + command: "private-server", + args: ["--token", "super secret"], + env: { API_TOKEN: "super-secret" }, + }, + { name: "http-server", type: "http", url: "https://mcp.example.test/api" }, + { name: "bare-name" }, +]; + +describe("MCP trust prompt TTY/TUI parity", () => { + for (const server of parityCases) { + test(`TTY and TUI render "${server.name}" identically`, () => { + expect(formatTuiMcpTrustQuestion(server)).toBe( + formatExecMcpTrustQuestion(server), + ); + }); + } + + test('both surfaces keep ["a b"] distinct from ["a", "b"]', () => { + const oneArg: MCPServerConfig = { + name: "s", + command: "run", + args: ["a b"], + }; + const twoArgs: MCPServerConfig = { + name: "s", + command: "run", + args: ["a", "b"], + }; + for (const format of [ + formatExecMcpTrustQuestion, + formatTuiMcpTrustQuestion, + ]) { + expect(format(oneArg)).toContain('"a b"'); + expect(format(oneArg)).not.toBe(format(twoArgs)); + } + }); +}); From 1a4c6ad7d7aa65efa6935629d10a61fa70a9d051 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 18:58:51 -0700 Subject: [PATCH 2/4] fix(trust): quote whitespace argv via one shared MCP trust prompt helper --- src/exec/runner.ts | 10 ++----- src/trust/project-trust.ts | 26 +++++++++++++++++++ src/tui/runner/session.ts | 14 +++++----- .../unit/tui/mcp-trust-prompt-parity.test.ts | 6 ++++- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/exec/runner.ts b/src/exec/runner.ts index bbb3f2a39..60cde4be6 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -20,6 +20,7 @@ import { registerSourceCredential, } from "../config/source-credentials.js"; import { formatDirectorSystemPrompt } from "../agent/directors/identity.js"; +import { formatMcpTrustQuestion } from "../trust/project-trust.js"; import { DIRECTOR_REGISTRY } from "../agent/directors/registry.js"; import type { DirectorId, DirectorPackage } from "../agent/directors/types.js"; import { submitOutputDefinition } from "../agent/director.js"; @@ -165,14 +166,7 @@ const logger = getLogger([LOG_NAMESPACE_ROOT, "exec"]); const SELECTED_PROVIDER_FAILURE = "SelectedProviderFailure"; export function formatExecMcpTrustQuestion(server: MCPServerConfig): string { - return ( - `Trust local MCP server "${server.name}" for this project?` + - (server.command !== undefined - ? `\nCommand: ${server.command}${(server.args ?? []).length > 0 ? ` ${(server.args ?? []).join(" ")}` : ""}` - : server.url !== undefined - ? `\nURL: ${server.url}` - : "") - ); + return formatMcpTrustQuestion(server); } export async function refreshSelectedProviderCredential( diff --git a/src/trust/project-trust.ts b/src/trust/project-trust.ts index 3afe52927..e442ae56e 100644 --- a/src/trust/project-trust.ts +++ b/src/trust/project-trust.ts @@ -298,6 +298,32 @@ export function mcpServerFingerprint(server: MCPServerConfig): string { return createHash("sha256").update(payload).digest("hex"); } +// Display-only argv quoting for the MCP trust prompt: an arg containing +// whitespace (or a quote, or empty) renders double-quoted so ["a b"] and +// ["a", "b"] never look alike. Approval identity still comes from +// mcpServerFingerprint above, never from this rendering. +function quoteMcpTrustArg(arg: string): string { + if (arg !== "" && !/[\s"]/.test(arg)) return arg; + return `"${arg.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; +} + +function formatMcpSpawnCommand(command: string, args: string[]): string { + return args.length === 0 + ? command + : `${command} ${args.map(quoteMcpTrustArg).join(" ")}`; +} + +export function formatMcpTrustQuestion(server: MCPServerConfig): string { + return ( + `Trust local MCP server "${server.name}" for this project?` + + (server.command !== undefined + ? `\nCommand: ${formatMcpSpawnCommand(server.command, server.args ?? [])}` + : server.url !== undefined + ? `\nURL: ${server.url}` + : "") + ); +} + export function isMcpServerTrusted( store: ProjectTrustStore, server: MCPServerConfig, diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index 542ee3818..40e6c24d3 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -14,6 +14,7 @@ import { EventEmitter } from "node:events"; import { shellTimeoutFromSettings, toolWatchdogFromSettings, + type MCPServerConfig, } from "../../config/settings.js"; import { isCodexProviderName } from "../../config/codex-providers.js"; import { peekSourceCredentialSecret } from "../../config/source-credentials.js"; @@ -126,6 +127,11 @@ import { } from "./state.js"; import { createParkedOverlayAbortBinding } from "./parked-overlay-abort.js"; import { createTUISettingsWriters } from "./settings-writers.js"; +import { formatMcpTrustQuestion } from "../../trust/project-trust.js"; + +export function formatTuiMcpTrustQuestion(server: MCPServerConfig): string { + return formatMcpTrustQuestion(server); +} export async function assembleTUISession( state: RunnerState, @@ -406,13 +412,7 @@ export async function assembleTUISession( const timeout = approvalTimeout(); const event: OperatorGateEvent = { id: randomUUID(), - question: - `Trust local MCP server "${server.name}" for this project?` + - (server.command !== undefined - ? `\nCommand: ${server.command}${(server.args ?? []).length > 0 ? ` ${(server.args ?? []).join(" ")}` : ""}` - : server.url !== undefined - ? `\nURL: ${server.url}` - : ""), + question: formatTuiMcpTrustQuestion(server), options: ["Trust and connect", "Deny"], resolve: finish, ...(timeout !== undefined ? timeout : {}), diff --git a/tests/unit/tui/mcp-trust-prompt-parity.test.ts b/tests/unit/tui/mcp-trust-prompt-parity.test.ts index 371775d6b..9f1e142ba 100644 --- a/tests/unit/tui/mcp-trust-prompt-parity.test.ts +++ b/tests/unit/tui/mcp-trust-prompt-parity.test.ts @@ -4,7 +4,11 @@ import { formatExecMcpTrustQuestion } from "../../../src/exec/runner.js"; import { formatTuiMcpTrustQuestion } from "../../../src/tui/runner/session.js"; const parityCases: MCPServerConfig[] = [ - { name: "plain-stdio", command: "node", args: ["server.js", "--port", "3000"] }, + { + name: "plain-stdio", + command: "node", + args: ["server.js", "--port", "3000"], + }, { name: "no-args", command: "node" }, { name: "empty-args", command: "node", args: [] }, { name: "spaced-path", command: "server", args: ["--dir", "/tmp/my work"] }, From 70f3e3d5a6ecc78497422ad4ed22b5cb47ad0546 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 19:57:56 -0700 Subject: [PATCH 3/4] fix(trust): escape control characters and quote spaced command in trust prompt --- src/trust/project-trust.ts | 36 +++++++++++++++--- tests/unit/project-trust.test.ts | 37 +++++++++++++++++++ .../unit/tui/mcp-trust-prompt-parity.test.ts | 18 +++++++++ 3 files changed, 85 insertions(+), 6 deletions(-) diff --git a/src/trust/project-trust.ts b/src/trust/project-trust.ts index e442ae56e..4cac62e88 100644 --- a/src/trust/project-trust.ts +++ b/src/trust/project-trust.ts @@ -300,17 +300,41 @@ export function mcpServerFingerprint(server: MCPServerConfig): string { // Display-only argv quoting for the MCP trust prompt: an arg containing // whitespace (or a quote, or empty) renders double-quoted so ["a b"] and -// ["a", "b"] never look alike. Approval identity still comes from -// mcpServerFingerprint above, never from this rendering. +// ["a", "b"] never look alike. Control characters render as visible escape +// sequences (\n, \r, \t, \xNN) so a newline-bearing arg cannot spoof extra +// prompt lines — output is always single-line per token. Approval identity +// still comes from mcpServerFingerprint above, never from this rendering. +function isTrustPromptControlChar(code: number): boolean { + return code <= 0x1f || code === 0x7f; +} + function quoteMcpTrustArg(arg: string): string { - if (arg !== "" && !/[\s"]/.test(arg)) return arg; - return `"${arg.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; + const needsQuotes = + arg === "" || + /[\s"]/.test(arg) || + [...arg].some((ch) => isTrustPromptControlChar(ch.charCodeAt(0))); + if (!needsQuotes) return arg; + const named = arg + .replace(/\\/g, "\\\\") + .replace(/"/g, '\\"') + .replace(/\n/g, "\\n") + .replace(/\r/g, "\\r") + .replace(/\t/g, "\\t"); + let escaped = ""; + for (const ch of named) { + const code = ch.charCodeAt(0); + escaped += isTrustPromptControlChar(code) + ? `\\x${code.toString(16).toUpperCase().padStart(2, "0")}` + : ch; + } + return `"${escaped}"`; } function formatMcpSpawnCommand(command: string, args: string[]): string { + const head = quoteMcpTrustArg(command); return args.length === 0 - ? command - : `${command} ${args.map(quoteMcpTrustArg).join(" ")}`; + ? head + : `${head} ${args.map(quoteMcpTrustArg).join(" ")}`; } export function formatMcpTrustQuestion(server: MCPServerConfig): string { diff --git a/tests/unit/project-trust.test.ts b/tests/unit/project-trust.test.ts index 2488fea34..01027b981 100644 --- a/tests/unit/project-trust.test.ts +++ b/tests/unit/project-trust.test.ts @@ -376,6 +376,43 @@ describe("project-trust", () => { expect(one).not.toBe(two); }); + test("trust question escapes control characters so args stay single-line", () => { + const question = formatMcpTrustQuestion({ + name: "s", + command: "run", + args: ["x\nTrust local MCP server evil", "a\tb", "c\rd"], + }); + // Only the structural header/Command separator newline may remain. + const lines = question.split("\n"); + expect(lines).toHaveLength(2); + for (const line of lines) { + for (const ch of line) { + const code = ch.charCodeAt(0); + expect(code > 0x1f && code !== 0x7f).toBe(true); + } + } + expect(question).toContain('"x\\nTrust local MCP server evil"'); + expect(question).toContain('"a\\tb"'); + expect(question).toContain('"c\\rd"'); + }); + + test("trust question quotes a spaced binary path so the command is unambiguous", () => { + expect( + formatMcpTrustQuestion({ + name: "s", + command: "/tmp/my tool/server", + args: ["--dir", "/tmp/work"], + }), + ).toBe( + 'Trust local MCP server "s" for this project?\nCommand: "/tmp/my tool/server" --dir /tmp/work', + ); + expect( + formatMcpTrustQuestion({ name: "s", command: "/tmp/my tool/server" }), + ).toBe( + 'Trust local MCP server "s" for this project?\nCommand: "/tmp/my tool/server"', + ); + }); + test("trust question leaves plain args unquoted and hides secrets", () => { expect( formatMcpTrustQuestion({ diff --git a/tests/unit/tui/mcp-trust-prompt-parity.test.ts b/tests/unit/tui/mcp-trust-prompt-parity.test.ts index 9f1e142ba..930893696 100644 --- a/tests/unit/tui/mcp-trust-prompt-parity.test.ts +++ b/tests/unit/tui/mcp-trust-prompt-parity.test.ts @@ -55,4 +55,22 @@ describe("MCP trust prompt TTY/TUI parity", () => { expect(format(oneArg)).not.toBe(format(twoArgs)); } }); + + test("both surfaces render tab/newline args escaped with no raw control characters", () => { + const server: MCPServerConfig = { + name: "s", + command: "run", + args: ["a\tb", "x\ny"], + }; + for (const format of [ + formatExecMcpTrustQuestion, + formatTuiMcpTrustQuestion, + ]) { + const rendered = format(server); + expect(rendered).toContain('"a\\tb"'); + expect(rendered).toContain('"x\\ny"'); + expect(rendered).not.toContain("\t"); + expect(rendered.split("\n")).toHaveLength(2); + } + }); }); From 1747d4dfa44601f2bb72e3e06de3dc1d1662372c Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 26 Sep 2026 08:19:52 -0700 Subject: [PATCH 4/4] fix(trust): show connect transport and escape name, url, Unicode The trust prompt showed Command whenever command was set, but connect opens HTTP when type is http or type is unset and url is set. Name and url also interpolated raw. Display now shares isHttpServer and escapes those fields plus Unicode/C1 line breaks. Fingerprint inputs are unchanged. --- src/mcp/client.ts | 8 +- src/mcp/is-http-server.test.ts | 31 +++++ src/mcp/is-http-server.ts | 15 +++ src/trust/project-trust.ts | 69 +++++++----- tests/unit/project-trust.test.ts | 106 ++++++++++++++++++ .../unit/tui/mcp-trust-prompt-parity.test.ts | 19 ++++ 6 files changed, 215 insertions(+), 33 deletions(-) create mode 100644 src/mcp/is-http-server.test.ts create mode 100644 src/mcp/is-http-server.ts diff --git a/src/mcp/client.ts b/src/mcp/client.ts index e804d1e5f..2121ade02 100644 --- a/src/mcp/client.ts +++ b/src/mcp/client.ts @@ -13,6 +13,7 @@ import { normalizeMCPServerURL } from "./auth-store.js"; import type { ResolvedMCPServerConfig } from "./exa.js"; import type { McpToolAnnotations } from "./tool-permissions.js"; import { buildStdioMcpProcessEnv } from "./stdio-env.js"; +import { isHttpServer } from "./is-http-server.js"; import { MCP_CLIENT_NAME } from "../branding.js"; export interface MCPTool { @@ -95,13 +96,6 @@ export interface MCPConnectOptions { onDisconnect?: () => void; } -function isHttpServer(config: ResolvedMCPServerConfig): boolean { - return ( - config.type === "http" || - (config.type === undefined && config.url !== undefined) - ); -} - export function unwrapToolContent(content: unknown): string { if (!Array.isArray(content) || content.length === 0) return ""; return content diff --git a/src/mcp/is-http-server.test.ts b/src/mcp/is-http-server.test.ts new file mode 100644 index 000000000..6f774b35c --- /dev/null +++ b/src/mcp/is-http-server.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, test } from "bun:test"; +import { isHttpServer } from "./is-http-server.js"; + +describe("isHttpServer", () => { + test("HTTP wins when type is unset and url is set, even with command", () => { + const config = { command: "run", url: "https://mcp.example.test" }; + expect(isHttpServer(config)).toBe(true); + }); + + test("type http wins even when command is also set", () => { + const config = { + type: "http" as const, + command: "run", + url: "https://mcp.example.test", + }; + expect(isHttpServer(config)).toBe(true); + }); + + test("type stdio is not HTTP even when url is set", () => { + expect( + isHttpServer({ + type: "stdio", + url: "https://mcp.example.test", + }), + ).toBe(false); + }); + + test("unset type and url is not HTTP", () => { + expect(isHttpServer({})).toBe(false); + }); +}); diff --git a/src/mcp/is-http-server.ts b/src/mcp/is-http-server.ts new file mode 100644 index 000000000..e0dce6dff --- /dev/null +++ b/src/mcp/is-http-server.ts @@ -0,0 +1,15 @@ +/** + * Connect-path transport: HTTP wins when `type` is `"http"`, or when `type` is + * unset and `url` is present — even if `command` is also set. Trust-prompt + * display must use this same predicate so the operator grants the identity + * that `connectMCPServer` will actually open. + */ +export function isHttpServer(config: { + type?: "stdio" | "http"; + url?: string; +}): boolean { + return ( + config.type === "http" || + (config.type === undefined && config.url !== undefined) + ); +} diff --git a/src/trust/project-trust.ts b/src/trust/project-trust.ts index 4cac62e88..fda7075bf 100644 --- a/src/trust/project-trust.ts +++ b/src/trust/project-trust.ts @@ -7,6 +7,7 @@ import { type } from "arktype"; import { getLogger } from "@intx/log"; import type { MCPServerConfig } from "../config/settings.js"; import { isBuiltinExaMCPServer } from "../mcp/exa.js"; +import { isHttpServer } from "../mcp/is-http-server.js"; import { LOG_NAMESPACE_ROOT, SETTINGS_DIR_NAME } from "../branding.js"; const logger = getLogger([LOG_NAMESPACE_ROOT, "trust"]); @@ -298,23 +299,23 @@ export function mcpServerFingerprint(server: MCPServerConfig): string { return createHash("sha256").update(payload).digest("hex"); } -// Display-only argv quoting for the MCP trust prompt: an arg containing -// whitespace (or a quote, or empty) renders double-quoted so ["a b"] and -// ["a", "b"] never look alike. Control characters render as visible escape -// sequences (\n, \r, \t, \xNN) so a newline-bearing arg cannot spoof extra -// prompt lines — output is always single-line per token. Approval identity -// still comes from mcpServerFingerprint above, never from this rendering. +// Display-only quoting for the MCP trust prompt: argv, name, and url all pass +// through the same escape so a newline, quote, or Unicode/C1 line break cannot +// spoof extra prompt lines. An arg containing whitespace (or a quote, or empty) +// renders double-quoted so ["a b"] and ["a", "b"] never look alike. Approval +// identity still comes from mcpServerFingerprint above, never from this rendering. function isTrustPromptControlChar(code: number): boolean { - return code <= 0x1f || code === 0x7f; + return ( + code <= 0x1f || + code === 0x7f || + (code >= 0x80 && code <= 0x9f) || + code === 0x2028 || + code === 0x2029 + ); } -function quoteMcpTrustArg(arg: string): string { - const needsQuotes = - arg === "" || - /[\s"]/.test(arg) || - [...arg].some((ch) => isTrustPromptControlChar(ch.charCodeAt(0))); - if (!needsQuotes) return arg; - const named = arg +function escapeMcpTrustText(value: string): string { + const named = value .replace(/\\/g, "\\\\") .replace(/"/g, '\\"') .replace(/\n/g, "\\n") @@ -323,11 +324,25 @@ function quoteMcpTrustArg(arg: string): string { let escaped = ""; for (const ch of named) { const code = ch.charCodeAt(0); - escaped += isTrustPromptControlChar(code) - ? `\\x${code.toString(16).toUpperCase().padStart(2, "0")}` - : ch; + if (!isTrustPromptControlChar(code)) { + escaped += ch; + continue; + } + escaped += + code <= 0xff + ? `\\x${code.toString(16).toUpperCase().padStart(2, "0")}` + : `\\u${code.toString(16).toUpperCase().padStart(4, "0")}`; } - return `"${escaped}"`; + return escaped; +} + +function quoteMcpTrustArg(arg: string): string { + const needsQuotes = + arg === "" || + /[\s"]/.test(arg) || + [...arg].some((ch) => isTrustPromptControlChar(ch.charCodeAt(0))); + if (!needsQuotes) return arg; + return `"${escapeMcpTrustText(arg)}"`; } function formatMcpSpawnCommand(command: string, args: string[]): string { @@ -338,14 +353,16 @@ function formatMcpSpawnCommand(command: string, args: string[]): string { } export function formatMcpTrustQuestion(server: MCPServerConfig): string { - return ( - `Trust local MCP server "${server.name}" for this project?` + - (server.command !== undefined - ? `\nCommand: ${formatMcpSpawnCommand(server.command, server.args ?? [])}` - : server.url !== undefined - ? `\nURL: ${server.url}` - : "") - ); + const header = `Trust local MCP server "${escapeMcpTrustText(server.name)}" for this project?`; + if (isHttpServer(server)) { + return server.url !== undefined + ? `${header}\nURL: ${quoteMcpTrustArg(server.url)}` + : header; + } + if (server.command !== undefined) { + return `${header}\nCommand: ${formatMcpSpawnCommand(server.command, server.args ?? [])}`; + } + return header; } export function isMcpServerTrusted( diff --git a/tests/unit/project-trust.test.ts b/tests/unit/project-trust.test.ts index 01027b981..dd41cae47 100644 --- a/tests/unit/project-trust.test.ts +++ b/tests/unit/project-trust.test.ts @@ -446,6 +446,112 @@ describe("project-trust", () => { ); }); + test("trust question shows URL not Command when command, args, and url are set without type", () => { + const question = formatMcpTrustQuestion({ + name: "s", + command: "run", + args: ["--secret"], + url: "https://mcp.example.test/api", + }); + expect(question).toContain("\nURL: https://mcp.example.test/api"); + expect(question).not.toContain("Command:"); + expect(question).not.toContain("run"); + }); + + test("trust question shows URL when type is http even if command is also set", () => { + const question = formatMcpTrustQuestion({ + name: "s", + type: "http", + command: "run", + url: "https://mcp.example.test/api", + }); + expect(question).toContain("\nURL: https://mcp.example.test/api"); + expect(question).not.toContain("Command:"); + }); + + test("trust question still shows Command when type is stdio even if url is set", () => { + const question = formatMcpTrustQuestion({ + name: "s", + type: "stdio", + command: "run", + args: ["a"], + url: "https://mcp.example.test/api", + }); + expect(question).toContain("\nCommand: run a"); + expect(question).not.toContain("URL:"); + }); + + test("trust question escapes name so a newline or quote cannot inject extra Command lines", () => { + const question = formatMcpTrustQuestion({ + name: 's"\nCommand: evil', + command: "run", + args: ["a"], + }); + const lines = question.split("\n"); + expect(lines).toHaveLength(2); + expect(lines[0]?.startsWith("Trust local MCP server")).toBe(true); + expect(lines[1]).toBe("Command: run a"); + expect(question).not.toContain("\nCommand: evil"); + expect(question).toContain("\\n"); + expect(question).toContain('\\"'); + }); + + test("trust question escapes url so an embedded newline stays single-line", () => { + const question = formatMcpTrustQuestion({ + name: "remote", + type: "http", + url: "https://mcp.example.test/api\nCommand: evil", + }); + const lines = question.split("\n"); + expect(lines).toHaveLength(2); + expect(lines[1]?.startsWith("URL:")).toBe(true); + expect(question).not.toContain("\nCommand:"); + expect(question).toContain("\\n"); + }); + + test("trust question escapes Unicode line breaks and C1 controls in args", () => { + const question = formatMcpTrustQuestion({ + name: "s", + command: "run", + args: ["x\u2028y", "a\u0085b"], + }); + expect(question.split("\n")).toHaveLength(2); + expect(question).not.toContain("\u2028"); + expect(question).not.toContain("\u0085"); + for (const line of question.split("\n")) { + for (const ch of line) { + const code = ch.charCodeAt(0); + expect( + code > 0x1f && + code !== 0x7f && + !(code >= 0x80 && code <= 0x9f) && + code !== 0x2028 && + code !== 0x2029, + ).toBe(true); + } + } + }); + + test("mcp fingerprint still hashes command and url together", () => { + const mixed: MCPServerConfig = { + name: "s", + command: "run", + args: ["a"], + url: "https://evil.test", + }; + expect(mcpServerFingerprint(mixed)).toBe( + "d06726e3489e2513056178f392b492a77aef02b239c8922c5a6cdca0b4fd886d", + ); + expect( + mcpServerFingerprint({ + name: "s", + type: "http", + command: "run", + url: "https://mcp.example.test", + }), + ).toBe("75b80b4878d818362a918027917cd149c0d948d509b8c0c08b7406fd69de53b9"); + }); + test("readProjectTrustStore: malformed file with wrong types, missing fields, and extra fields drops bad entries and ignores unknown keys", async () => { const { cwd, home, cleanup } = await scratch(); try { diff --git a/tests/unit/tui/mcp-trust-prompt-parity.test.ts b/tests/unit/tui/mcp-trust-prompt-parity.test.ts index 930893696..b6781b13b 100644 --- a/tests/unit/tui/mcp-trust-prompt-parity.test.ts +++ b/tests/unit/tui/mcp-trust-prompt-parity.test.ts @@ -24,6 +24,25 @@ const parityCases: MCPServerConfig[] = [ env: { API_TOKEN: "super-secret" }, }, { name: "http-server", type: "http", url: "https://mcp.example.test/api" }, + { + name: "http-wins-no-type", + command: "run", + args: ["a"], + url: "https://mcp.example.test/api", + }, + { + name: "http-typed-with-command", + type: "http", + command: "run", + url: "https://mcp.example.test/api", + }, + { name: 'inject\nCommand: evil"', command: "run", args: ["a"] }, + { + name: "url-newline", + type: "http", + url: "https://mcp.example.test/api\nCommand: evil", + }, + { name: "unicode-break", command: "run", args: ["x\u2028y", "a\u0085b"] }, { name: "bare-name" }, ];