Skip to content

Commit 1747d4d

Browse files
committed
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 70f3e3d commit 1747d4d

6 files changed

Lines changed: 215 additions & 33 deletions

File tree

‎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: 43 additions & 26 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,23 +299,23 @@ export function mcpServerFingerprint(server: MCPServerConfig): string {
298299
return createHash("sha256").update(payload).digest("hex");
299300
}
300301

301-
// Display-only argv quoting for the MCP trust prompt: an arg containing
302-
// whitespace (or a quote, or empty) renders double-quoted so ["a b"] and
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.
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.
307307
function isTrustPromptControlChar(code: number): boolean {
308-
return code <= 0x1f || code === 0x7f;
308+
return (
309+
code <= 0x1f ||
310+
code === 0x7f ||
311+
(code >= 0x80 && code <= 0x9f) ||
312+
code === 0x2028 ||
313+
code === 0x2029
314+
);
309315
}
310316

311-
function quoteMcpTrustArg(arg: string): string {
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
317+
function escapeMcpTrustText(value: string): string {
318+
const named = value
318319
.replace(/\\/g, "\\\\")
319320
.replace(/"/g, '\\"')
320321
.replace(/\n/g, "\\n")
@@ -323,11 +324,25 @@ function quoteMcpTrustArg(arg: string): string {
323324
let escaped = "";
324325
for (const ch of named) {
325326
const code = ch.charCodeAt(0);
326-
escaped += isTrustPromptControlChar(code)
327-
? `\\x${code.toString(16).toUpperCase().padStart(2, "0")}`
328-
: ch;
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")}`;
329335
}
330-
return `"${escaped}"`;
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)}"`;
331346
}
332347

333348
function formatMcpSpawnCommand(command: string, args: string[]): string {
@@ -338,14 +353,16 @@ function formatMcpSpawnCommand(command: string, args: string[]): string {
338353
}
339354

340355
export function formatMcpTrustQuestion(server: MCPServerConfig): string {
341-
return (
342-
`Trust local MCP server "${server.name}" for this project?` +
343-
(server.command !== undefined
344-
? `\nCommand: ${formatMcpSpawnCommand(server.command, server.args ?? [])}`
345-
: server.url !== undefined
346-
? `\nURL: ${server.url}`
347-
: "")
348-
);
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;
349366
}
350367

351368
export function isMcpServerTrusted(

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

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,112 @@ describe("project-trust", () => {
446446
);
447447
});
448448

449+
test("trust question shows URL not Command when command, args, and url are set without type", () => {
450+
const question = formatMcpTrustQuestion({
451+
name: "s",
452+
command: "run",
453+
args: ["--secret"],
454+
url: "https://mcp.example.test/api",
455+
});
456+
expect(question).toContain("\nURL: https://mcp.example.test/api");
457+
expect(question).not.toContain("Command:");
458+
expect(question).not.toContain("run");
459+
});
460+
461+
test("trust question shows URL when type is http even if command is also set", () => {
462+
const question = formatMcpTrustQuestion({
463+
name: "s",
464+
type: "http",
465+
command: "run",
466+
url: "https://mcp.example.test/api",
467+
});
468+
expect(question).toContain("\nURL: https://mcp.example.test/api");
469+
expect(question).not.toContain("Command:");
470+
});
471+
472+
test("trust question still shows Command when type is stdio even if url is set", () => {
473+
const question = formatMcpTrustQuestion({
474+
name: "s",
475+
type: "stdio",
476+
command: "run",
477+
args: ["a"],
478+
url: "https://mcp.example.test/api",
479+
});
480+
expect(question).toContain("\nCommand: run a");
481+
expect(question).not.toContain("URL:");
482+
});
483+
484+
test("trust question escapes name so a newline or quote cannot inject extra Command lines", () => {
485+
const question = formatMcpTrustQuestion({
486+
name: 's"\nCommand: evil',
487+
command: "run",
488+
args: ["a"],
489+
});
490+
const lines = question.split("\n");
491+
expect(lines).toHaveLength(2);
492+
expect(lines[0]?.startsWith("Trust local MCP server")).toBe(true);
493+
expect(lines[1]).toBe("Command: run a");
494+
expect(question).not.toContain("\nCommand: evil");
495+
expect(question).toContain("\\n");
496+
expect(question).toContain('\\"');
497+
});
498+
499+
test("trust question escapes url so an embedded newline stays single-line", () => {
500+
const question = formatMcpTrustQuestion({
501+
name: "remote",
502+
type: "http",
503+
url: "https://mcp.example.test/api\nCommand: evil",
504+
});
505+
const lines = question.split("\n");
506+
expect(lines).toHaveLength(2);
507+
expect(lines[1]?.startsWith("URL:")).toBe(true);
508+
expect(question).not.toContain("\nCommand:");
509+
expect(question).toContain("\\n");
510+
});
511+
512+
test("trust question escapes Unicode line breaks and C1 controls in args", () => {
513+
const question = formatMcpTrustQuestion({
514+
name: "s",
515+
command: "run",
516+
args: ["x\u2028y", "a\u0085b"],
517+
});
518+
expect(question.split("\n")).toHaveLength(2);
519+
expect(question).not.toContain("\u2028");
520+
expect(question).not.toContain("\u0085");
521+
for (const line of question.split("\n")) {
522+
for (const ch of line) {
523+
const code = ch.charCodeAt(0);
524+
expect(
525+
code > 0x1f &&
526+
code !== 0x7f &&
527+
!(code >= 0x80 && code <= 0x9f) &&
528+
code !== 0x2028 &&
529+
code !== 0x2029,
530+
).toBe(true);
531+
}
532+
}
533+
});
534+
535+
test("mcp fingerprint still hashes command and url together", () => {
536+
const mixed: MCPServerConfig = {
537+
name: "s",
538+
command: "run",
539+
args: ["a"],
540+
url: "https://evil.test",
541+
};
542+
expect(mcpServerFingerprint(mixed)).toBe(
543+
"d06726e3489e2513056178f392b492a77aef02b239c8922c5a6cdca0b4fd886d",
544+
);
545+
expect(
546+
mcpServerFingerprint({
547+
name: "s",
548+
type: "http",
549+
command: "run",
550+
url: "https://mcp.example.test",
551+
}),
552+
).toBe("75b80b4878d818362a918027917cd149c0d948d509b8c0c08b7406fd69de53b9");
553+
});
554+
449555
test("readProjectTrustStore: malformed file with wrong types, missing fields, and extra fields drops bad entries and ignores unknown keys", async () => {
450556
const { cwd, home, cleanup } = await scratch();
451557
try {

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,25 @@ const parityCases: MCPServerConfig[] = [
2424
env: { API_TOKEN: "super-secret" },
2525
},
2626
{ name: "http-server", type: "http", url: "https://mcp.example.test/api" },
27+
{
28+
name: "http-wins-no-type",
29+
command: "run",
30+
args: ["a"],
31+
url: "https://mcp.example.test/api",
32+
},
33+
{
34+
name: "http-typed-with-command",
35+
type: "http",
36+
command: "run",
37+
url: "https://mcp.example.test/api",
38+
},
39+
{ name: 'inject\nCommand: evil"', command: "run", args: ["a"] },
40+
{
41+
name: "url-newline",
42+
type: "http",
43+
url: "https://mcp.example.test/api\nCommand: evil",
44+
},
45+
{ name: "unicode-break", command: "run", args: ["x\u2028y", "a\u0085b"] },
2746
{ name: "bare-name" },
2847
];
2948

0 commit comments

Comments
 (0)