Skip to content

Commit ac7a449

Browse files
CL-9361: quote whitespace args in shared MCP trust prompt helper (TTY + TUI) (#1183)
* test(trust): quote whitespace argv in shared MCP trust prompt helper * fix(trust): quote whitespace argv via one shared MCP trust prompt helper * fix(trust): escape control characters and quote spaced command in trust prompt * 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.
1 parent 8494ae2 commit ac7a449

9 files changed

Lines changed: 463 additions & 22 deletions

File tree

‎src/exec/runner.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
registerSourceCredential,
2121
} from "../config/source-credentials.js";
2222
import { formatDirectorSystemPrompt } from "../agent/directors/identity.js";
23+
import { formatMcpTrustQuestion } from "../trust/project-trust.js";
2324
import { DIRECTOR_REGISTRY } from "../agent/directors/registry.js";
2425
import type { DirectorId, DirectorPackage } from "../agent/directors/types.js";
2526
import { submitOutputDefinition } from "../agent/director.js";
@@ -165,14 +166,7 @@ const logger = getLogger([LOG_NAMESPACE_ROOT, "exec"]);
165166
const SELECTED_PROVIDER_FAILURE = "SelectedProviderFailure";
166167

167168
export function formatExecMcpTrustQuestion(server: MCPServerConfig): string {
168-
return (
169-
`Trust local MCP server "${server.name}" for this project?` +
170-
(server.command !== undefined
171-
? `\nCommand: ${server.command}${(server.args ?? []).length > 0 ? ` ${(server.args ?? []).join(" ")}` : ""}`
172-
: server.url !== undefined
173-
? `\nURL: ${server.url}`
174-
: "")
175-
);
169+
return formatMcpTrustQuestion(server);
176170
}
177171

178172
export async function refreshSelectedProviderCredential<T>(

‎src/mcp/client.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { normalizeMCPServerURL } from "./auth-store.js";
1313
import type { ResolvedMCPServerConfig } from "./exa.js";
1414
import type { McpToolAnnotations } from "./tool-permissions.js";
1515
import { buildStdioMcpProcessEnv } from "./stdio-env.js";
16+
import { isHttpServer } from "./is-http-server.js";
1617
import { MCP_CLIENT_NAME } from "../branding.js";
1718

1819
export interface MCPTool {
@@ -95,13 +96,6 @@ export interface MCPConnectOptions {
9596
onDisconnect?: () => void;
9697
}
9798

98-
function isHttpServer(config: ResolvedMCPServerConfig): boolean {
99-
return (
100-
config.type === "http" ||
101-
(config.type === undefined && config.url !== undefined)
102-
);
103-
}
104-
10599
export function unwrapToolContent(content: unknown): string {
106100
if (!Array.isArray(content) || content.length === 0) return "";
107101
return content

‎src/mcp/is-http-server.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { isHttpServer } from "./is-http-server.js";
3+
4+
describe("isHttpServer", () => {
5+
test("HTTP wins when type is unset and url is set, even with command", () => {
6+
const config = { command: "run", url: "https://mcp.example.test" };
7+
expect(isHttpServer(config)).toBe(true);
8+
});
9+
10+
test("type http wins even when command is also set", () => {
11+
const config = {
12+
type: "http" as const,
13+
command: "run",
14+
url: "https://mcp.example.test",
15+
};
16+
expect(isHttpServer(config)).toBe(true);
17+
});
18+
19+
test("type stdio is not HTTP even when url is set", () => {
20+
expect(
21+
isHttpServer({
22+
type: "stdio",
23+
url: "https://mcp.example.test",
24+
}),
25+
).toBe(false);
26+
});
27+
28+
test("unset type and url is not HTTP", () => {
29+
expect(isHttpServer({})).toBe(false);
30+
});
31+
});

‎src/mcp/is-http-server.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* Connect-path transport: HTTP wins when `type` is `"http"`, or when `type` is
3+
* unset and `url` is present — even if `command` is also set. Trust-prompt
4+
* display must use this same predicate so the operator grants the identity
5+
* that `connectMCPServer` will actually open.
6+
*/
7+
export function isHttpServer(config: {
8+
type?: "stdio" | "http";
9+
url?: string;
10+
}): boolean {
11+
return (
12+
config.type === "http" ||
13+
(config.type === undefined && config.url !== undefined)
14+
);
15+
}

‎src/trust/project-trust.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { type } from "arktype";
77
import { getLogger } from "@intx/log";
88
import type { MCPServerConfig } from "../config/settings.js";
99
import { isBuiltinExaMCPServer } from "../mcp/exa.js";
10+
import { isHttpServer } from "../mcp/is-http-server.js";
1011
import { LOG_NAMESPACE_ROOT, SETTINGS_DIR_NAME } from "../branding.js";
1112

1213
const logger = getLogger([LOG_NAMESPACE_ROOT, "trust"]);
@@ -298,6 +299,72 @@ export function mcpServerFingerprint(server: MCPServerConfig): string {
298299
return createHash("sha256").update(payload).digest("hex");
299300
}
300301

302+
// Display-only quoting for the MCP trust prompt: argv, name, and url all pass
303+
// through the same escape so a newline, quote, or Unicode/C1 line break cannot
304+
// spoof extra prompt lines. An arg containing whitespace (or a quote, or empty)
305+
// renders double-quoted so ["a b"] and ["a", "b"] never look alike. Approval
306+
// identity still comes from mcpServerFingerprint above, never from this rendering.
307+
function isTrustPromptControlChar(code: number): boolean {
308+
return (
309+
code <= 0x1f ||
310+
code === 0x7f ||
311+
(code >= 0x80 && code <= 0x9f) ||
312+
code === 0x2028 ||
313+
code === 0x2029
314+
);
315+
}
316+
317+
function escapeMcpTrustText(value: string): string {
318+
const named = value
319+
.replace(/\\/g, "\\\\")
320+
.replace(/"/g, '\\"')
321+
.replace(/\n/g, "\\n")
322+
.replace(/\r/g, "\\r")
323+
.replace(/\t/g, "\\t");
324+
let escaped = "";
325+
for (const ch of named) {
326+
const code = ch.charCodeAt(0);
327+
if (!isTrustPromptControlChar(code)) {
328+
escaped += ch;
329+
continue;
330+
}
331+
escaped +=
332+
code <= 0xff
333+
? `\\x${code.toString(16).toUpperCase().padStart(2, "0")}`
334+
: `\\u${code.toString(16).toUpperCase().padStart(4, "0")}`;
335+
}
336+
return escaped;
337+
}
338+
339+
function quoteMcpTrustArg(arg: string): string {
340+
const needsQuotes =
341+
arg === "" ||
342+
/[\s"]/.test(arg) ||
343+
[...arg].some((ch) => isTrustPromptControlChar(ch.charCodeAt(0)));
344+
if (!needsQuotes) return arg;
345+
return `"${escapeMcpTrustText(arg)}"`;
346+
}
347+
348+
function formatMcpSpawnCommand(command: string, args: string[]): string {
349+
const head = quoteMcpTrustArg(command);
350+
return args.length === 0
351+
? head
352+
: `${head} ${args.map(quoteMcpTrustArg).join(" ")}`;
353+
}
354+
355+
export function formatMcpTrustQuestion(server: MCPServerConfig): string {
356+
const header = `Trust local MCP server "${escapeMcpTrustText(server.name)}" for this project?`;
357+
if (isHttpServer(server)) {
358+
return server.url !== undefined
359+
? `${header}\nURL: ${quoteMcpTrustArg(server.url)}`
360+
: header;
361+
}
362+
if (server.command !== undefined) {
363+
return `${header}\nCommand: ${formatMcpSpawnCommand(server.command, server.args ?? [])}`;
364+
}
365+
return header;
366+
}
367+
301368
export function isMcpServerTrusted(
302369
store: ProjectTrustStore,
303370
server: MCPServerConfig,

‎src/tui/runner/session.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { EventEmitter } from "node:events";
1414
import {
1515
shellTimeoutFromSettings,
1616
toolWatchdogFromSettings,
17+
type MCPServerConfig,
1718
} from "../../config/settings.js";
1819
import { isCodexProviderName } from "../../config/codex-providers.js";
1920
import { peekSourceCredentialSecret } from "../../config/source-credentials.js";
@@ -126,6 +127,11 @@ import {
126127
} from "./state.js";
127128
import { createParkedOverlayAbortBinding } from "./parked-overlay-abort.js";
128129
import { createTUISettingsWriters } from "./settings-writers.js";
130+
import { formatMcpTrustQuestion } from "../../trust/project-trust.js";
131+
132+
export function formatTuiMcpTrustQuestion(server: MCPServerConfig): string {
133+
return formatMcpTrustQuestion(server);
134+
}
129135

130136
export async function assembleTUISession(
131137
state: RunnerState,
@@ -406,13 +412,7 @@ export async function assembleTUISession(
406412
const timeout = approvalTimeout();
407413
const event: OperatorGateEvent = {
408414
id: randomUUID(),
409-
question:
410-
`Trust local MCP server "${server.name}" for this project?` +
411-
(server.command !== undefined
412-
? `\nCommand: ${server.command}${(server.args ?? []).length > 0 ? ` ${(server.args ?? []).join(" ")}` : ""}`
413-
: server.url !== undefined
414-
? `\nURL: ${server.url}`
415-
: ""),
415+
question: formatTuiMcpTrustQuestion(server),
416416
options: ["Trust and connect", "Deny"],
417417
resolve: finish,
418418
...(timeout !== undefined ? timeout : {}),

‎tests/unit/exec/runner.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,57 @@ describe("exec MCP trust prompt", () => {
124124
});
125125
});
126126

127+
describe("exec MCP trust prompt argv boundaries", () => {
128+
test("quotes an arg containing whitespace", () => {
129+
expect(
130+
formatExecMcpTrustQuestion({
131+
name: "notes",
132+
command: "server",
133+
args: ["--dir", "/tmp/my work"],
134+
}),
135+
).toBe(
136+
'Trust local MCP server "notes" for this project?\nCommand: server --dir "/tmp/my work"',
137+
);
138+
});
139+
140+
test("renders one spaced arg distinctly from two args", () => {
141+
const one = formatExecMcpTrustQuestion({
142+
name: "s",
143+
command: "run",
144+
args: ["a b"],
145+
});
146+
const two = formatExecMcpTrustQuestion({
147+
name: "s",
148+
command: "run",
149+
args: ["a", "b"],
150+
});
151+
expect(one).toContain('"a b"');
152+
expect(one).not.toBe(two);
153+
});
154+
155+
test("quotes empty args so they stay visible", () => {
156+
const question = formatExecMcpTrustQuestion({
157+
name: "s",
158+
command: "run",
159+
args: [""],
160+
});
161+
expect(question).toContain('""');
162+
expect(question).not.toBe(
163+
formatExecMcpTrustQuestion({ name: "s", command: "run", args: [] }),
164+
);
165+
});
166+
167+
test("escapes quotes inside a quoted arg", () => {
168+
expect(
169+
formatExecMcpTrustQuestion({
170+
name: "s",
171+
command: "run",
172+
args: ['say "hi"'],
173+
}),
174+
).toContain('"say \\"hi\\""');
175+
});
176+
});
177+
127178
describe("formatCaughtError", () => {
128179
test("prefers Error.message and stringifies other values", () => {
129180
expect(formatCaughtError(new Error("disk full"))).toBe("disk full");

0 commit comments

Comments
 (0)