Skip to content

Commit 24f828f

Browse files
committed
fix(trust): escape control characters and quote spaced command in trust prompt
1 parent f8eca49 commit 24f828f

3 files changed

Lines changed: 85 additions & 6 deletions

File tree

‎src/trust/project-trust.ts‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -300,17 +300,41 @@ export function mcpServerFingerprint(server: MCPServerConfig): string {
300300

301301
// Display-only argv quoting for the MCP trust prompt: an arg containing
302302
// whitespace (or a quote, or empty) renders double-quoted so ["a b"] and
303-
// ["a", "b"] never look alike. Approval identity still comes from
304-
// mcpServerFingerprint above, never from this rendering.
303+
// ["a", "b"] never look alike. Control characters render as visible escape
304+
// sequences (\n, \r, \t, \xNN) so a newline-bearing arg cannot spoof extra
305+
// prompt lines — output is always single-line per token. Approval identity
306+
// still comes from mcpServerFingerprint above, never from this rendering.
307+
function isTrustPromptControlChar(code: number): boolean {
308+
return code <= 0x1f || code === 0x7f;
309+
}
310+
305311
function quoteMcpTrustArg(arg: string): string {
306-
if (arg !== "" && !/[\s"]/.test(arg)) return arg;
307-
return `"${arg.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`;
312+
const needsQuotes =
313+
arg === "" ||
314+
/[\s"]/.test(arg) ||
315+
[...arg].some((ch) => isTrustPromptControlChar(ch.charCodeAt(0)));
316+
if (!needsQuotes) return arg;
317+
const named = arg
318+
.replace(/\\/g, "\\\\")
319+
.replace(/"/g, '\\"')
320+
.replace(/\n/g, "\\n")
321+
.replace(/\r/g, "\\r")
322+
.replace(/\t/g, "\\t");
323+
let escaped = "";
324+
for (const ch of named) {
325+
const code = ch.charCodeAt(0);
326+
escaped += isTrustPromptControlChar(code)
327+
? `\\x${code.toString(16).toUpperCase().padStart(2, "0")}`
328+
: ch;
329+
}
330+
return `"${escaped}"`;
308331
}
309332

310333
function formatMcpSpawnCommand(command: string, args: string[]): string {
334+
const head = quoteMcpTrustArg(command);
311335
return args.length === 0
312-
? command
313-
: `${command} ${args.map(quoteMcpTrustArg).join(" ")}`;
336+
? head
337+
: `${head} ${args.map(quoteMcpTrustArg).join(" ")}`;
314338
}
315339

316340
export function formatMcpTrustQuestion(server: MCPServerConfig): string {

‎tests/unit/project-trust.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,43 @@ describe("project-trust", () => {
376376
expect(one).not.toBe(two);
377377
});
378378

379+
test("trust question escapes control characters so args stay single-line", () => {
380+
const question = formatMcpTrustQuestion({
381+
name: "s",
382+
command: "run",
383+
args: ["x\nTrust local MCP server evil", "a\tb", "c\rd"],
384+
});
385+
// Only the structural header/Command separator newline may remain.
386+
const lines = question.split("\n");
387+
expect(lines).toHaveLength(2);
388+
for (const line of lines) {
389+
for (const ch of line) {
390+
const code = ch.charCodeAt(0);
391+
expect(code > 0x1f && code !== 0x7f).toBe(true);
392+
}
393+
}
394+
expect(question).toContain('"x\\nTrust local MCP server evil"');
395+
expect(question).toContain('"a\\tb"');
396+
expect(question).toContain('"c\\rd"');
397+
});
398+
399+
test("trust question quotes a spaced binary path so the command is unambiguous", () => {
400+
expect(
401+
formatMcpTrustQuestion({
402+
name: "s",
403+
command: "/tmp/my tool/server",
404+
args: ["--dir", "/tmp/work"],
405+
}),
406+
).toBe(
407+
'Trust local MCP server "s" for this project?\nCommand: "/tmp/my tool/server" --dir /tmp/work',
408+
);
409+
expect(
410+
formatMcpTrustQuestion({ name: "s", command: "/tmp/my tool/server" }),
411+
).toBe(
412+
'Trust local MCP server "s" for this project?\nCommand: "/tmp/my tool/server"',
413+
);
414+
});
415+
379416
test("trust question leaves plain args unquoted and hides secrets", () => {
380417
expect(
381418
formatMcpTrustQuestion({

‎tests/unit/tui/mcp-trust-prompt-parity.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,22 @@ describe("MCP trust prompt TTY/TUI parity", () => {
5555
expect(format(oneArg)).not.toBe(format(twoArgs));
5656
}
5757
});
58+
59+
test("both surfaces render tab/newline args escaped with no raw control characters", () => {
60+
const server: MCPServerConfig = {
61+
name: "s",
62+
command: "run",
63+
args: ["a\tb", "x\ny"],
64+
};
65+
for (const format of [
66+
formatExecMcpTrustQuestion,
67+
formatTuiMcpTrustQuestion,
68+
]) {
69+
const rendered = format(server);
70+
expect(rendered).toContain('"a\\tb"');
71+
expect(rendered).toContain('"x\\ny"');
72+
expect(rendered).not.toContain("\t");
73+
expect(rendered.split("\n")).toHaveLength(2);
74+
}
75+
});
5876
});

0 commit comments

Comments
 (0)