Skip to content

Commit 649e1cc

Browse files
committed
Fail closed when retrying untrusted local MCP servers
Startup already filtered local-source servers before spawn. Retry went through late connect with no trust check, so Deny still launched the command. Apply the same filter on that path.
1 parent 84f90df commit 649e1cc

2 files changed

Lines changed: 182 additions & 8 deletions

File tree

src/agent/tools.ts

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,11 @@ import { createExaMCPServerConfig, isBuiltinExaMCPServer } from "../mcp/exa.js";
3131
import { mcpClientToAgentTools } from "../mcp/plugin.js";
3232
import { createDynamicToolRunner, type DynamicToolRunner } from "../tui/dynamic-tool-runner.js";
3333
import type { MCPServerConfig, Settings } from "../config/settings.js";
34-
import { filterMcpServersForConnect, type ProjectTrustStore } from "../trust/project-trust.js";
34+
import {
35+
filterMcpServersForConnect,
36+
mcpServerFingerprint,
37+
type ProjectTrustStore,
38+
} from "../trust/project-trust.js";
3539
import type { ToolWatchdogConfig } from "../tui/tool-execution-watchdog.js";
3640
import type { SessionMode } from "../config/session-mode.js";
3741
import { sessionModeEnablesSubAgents } from "../config/session-mode.js";
@@ -547,6 +551,38 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
547551
const mcpAbortController = new AbortController();
548552
let disposed = false;
549553
let disposal: Promise<void> | undefined;
554+
let mcpTrustStore: ProjectTrustStore = projectTrust ?? {
555+
trustedPluginPaths: [],
556+
trustedMcpFingerprints: [],
557+
};
558+
const untrustedLocalError = `Not trusted for this project (see ${SETTINGS_DIR_NAME}/trust.json)`;
559+
560+
const filterServersForConnect = async (
561+
servers: MCPServerConfig[],
562+
): Promise<MCPServerConfig[]> => {
563+
const allowed = await filterMcpServersForConnect(servers, {
564+
source: mcpServersSource,
565+
store: mcpTrustStore,
566+
cwd,
567+
...(requestMcpTrust !== undefined ? { requestTrust: requestMcpTrust } : {}),
568+
});
569+
if (mcpServersSource !== "local") return allowed;
570+
// Remember grants so connectOneMCPServer does not re-prompt after startup TOFU.
571+
let fingerprints = mcpTrustStore.trustedMcpFingerprints;
572+
let changed = false;
573+
for (const server of allowed) {
574+
if (isBuiltinExaMCPServer(server)) continue;
575+
const fp = mcpServerFingerprint(server);
576+
if (!fingerprints.includes(fp)) {
577+
fingerprints = [...fingerprints, fp];
578+
changed = true;
579+
}
580+
}
581+
if (changed) {
582+
mcpTrustStore = { ...mcpTrustStore, trustedMcpFingerprints: fingerprints };
583+
}
584+
return allowed;
585+
};
550586

551587
const connectOneMCPServer = (
552588
config: MCPServerConfig,
@@ -563,6 +599,18 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
563599
: AbortSignal.any([mcpAbortController.signal, signal]);
564600

565601
const run = (async () => {
602+
if (mcpServersSource === "local") {
603+
const allowed = await filterServersForConnect([config]);
604+
if (disposed) return;
605+
if (allowed.length === 0) {
606+
callbacks.onStatus({
607+
name: config.name,
608+
state: "failed",
609+
error: untrustedLocalError,
610+
});
611+
return;
612+
}
613+
}
566614
callbacks.onStatus({ name: config.name, state: "connecting" });
567615
let result: MCPConnectResult;
568616
try {
@@ -657,12 +705,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
657705
signal?: AbortSignal,
658706
): Promise<void> => {
659707
if (disposed) return;
660-
const toConnect = await filterMcpServersForConnect(mcpServers, {
661-
source: mcpServersSource,
662-
store: projectTrust ?? { trustedPluginPaths: [], trustedMcpFingerprints: [] },
663-
cwd,
664-
...(requestMcpTrust !== undefined ? { requestTrust: requestMcpTrust } : {}),
665-
});
708+
const toConnect = await filterServersForConnect(mcpServers);
666709
if (disposed) return;
667710
await Promise.all(toConnect.map((config) => connectOneMCPServer(config, callbacks, signal)));
668711
if (disposed) return;
@@ -674,7 +717,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
674717
callbacks.onStatus({
675718
name: server.name,
676719
state: "failed",
677-
error: `Not trusted for this project (see ${SETTINGS_DIR_NAME}/trust.json)`,
720+
error: untrustedLocalError,
678721
});
679722
}
680723
}

tests/unit/tui/agent-tools.test.ts

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { ToolDefinition, ToolCall } from "@intx/types/runtime";
33
import { TOOL_NAMES } from "@intx/tools-posix";
44
import { createPermissionGate } from "../../../src/permission/gate.js";
55
import type { PermissionGate } from "../../../src/permission/gate.js";
6+
import { mcpServerFingerprint } from "../../../src/trust/project-trust.js";
67
import { withMockedModule } from "../../helpers/mock-module.js";
78

89
const mockDispose = mock(async () => {});
@@ -362,6 +363,136 @@ test("headless MCP connection does not wait for interactive OAuth", async () =>
362363
expect(mockConnectMCPServer.mock.calls[0]?.[1]?.onAuthURL).toBeUndefined();
363364
});
364365

366+
const localStdioServer = { name: "evil", command: "evil-bin" };
367+
const globalHttpServer = {
368+
name: "linear",
369+
type: "http" as const,
370+
url: "https://mcp.example.test/mcp",
371+
};
372+
373+
test("late connect of an untrusted local-source server does not spawn", async () => {
374+
mockConnectMCPServer.mockClear();
375+
const statuses: { name: string; state: string; error?: string }[] = [];
376+
const toolset = await createAgentToolset({
377+
cwd: "/fake",
378+
permissionGate: fakePermissionGate,
379+
onOperatorGate: async () => ({ kind: "cancel" }),
380+
mcpServers: [localStdioServer],
381+
mcpServersSource: "local",
382+
projectTrust: { trustedPluginPaths: [], trustedMcpFingerprints: [] },
383+
});
384+
385+
await toolset.connectMCPServer(localStdioServer, {
386+
interactiveAuth: false,
387+
onStatus: (status) => statuses.push(status),
388+
onToolsChanged: () => {},
389+
});
390+
391+
expect(mockConnectMCPServer).not.toHaveBeenCalled();
392+
expect(statuses).toHaveLength(1);
393+
expect(statuses[0]?.name).toBe("evil");
394+
expect(statuses[0]?.state).toBe("failed");
395+
expect(statuses[0]?.error).toMatch(/Not trusted for this project/);
396+
await toolset.dispose();
397+
});
398+
399+
test("late connect of an untrusted local-source server fail-closes when requestMcpTrust denies", async () => {
400+
mockConnectMCPServer.mockClear();
401+
let trustAsks = 0;
402+
const toolset = await createAgentToolset({
403+
cwd: "/fake",
404+
permissionGate: fakePermissionGate,
405+
onOperatorGate: async () => ({ kind: "cancel" }),
406+
mcpServers: [localStdioServer],
407+
mcpServersSource: "local",
408+
projectTrust: { trustedPluginPaths: [], trustedMcpFingerprints: [] },
409+
requestMcpTrust: async () => {
410+
trustAsks += 1;
411+
return false;
412+
},
413+
});
414+
415+
await toolset.connectMCPServer(localStdioServer, {
416+
interactiveAuth: false,
417+
onStatus: () => {},
418+
onToolsChanged: () => {},
419+
});
420+
421+
expect(trustAsks).toBe(1);
422+
expect(mockConnectMCPServer).not.toHaveBeenCalled();
423+
await toolset.dispose();
424+
});
425+
426+
test("late connect of a trusted local-source server still connects", async () => {
427+
mockConnectMCPServer.mockClear();
428+
const toolset = await createAgentToolset({
429+
cwd: "/fake",
430+
permissionGate: fakePermissionGate,
431+
onOperatorGate: async () => ({ kind: "cancel" }),
432+
mcpServers: [localStdioServer],
433+
mcpServersSource: "local",
434+
projectTrust: {
435+
trustedPluginPaths: [],
436+
trustedMcpFingerprints: [mcpServerFingerprint(localStdioServer)],
437+
},
438+
});
439+
440+
await toolset.connectMCPServer(localStdioServer, {
441+
interactiveAuth: false,
442+
onStatus: () => {},
443+
onToolsChanged: () => {},
444+
});
445+
446+
expect(mockConnectMCPServer).toHaveBeenCalledTimes(1);
447+
expect(mockConnectMCPServer.mock.calls[0]?.[0]).toEqual(localStdioServer);
448+
await toolset.dispose();
449+
});
450+
451+
test("late connect of a global-source HTTP server does not require trust", async () => {
452+
mockConnectMCPServer.mockClear();
453+
const toolset = await createAgentToolset({
454+
cwd: "/fake",
455+
permissionGate: fakePermissionGate,
456+
onOperatorGate: async () => ({ kind: "cancel" }),
457+
mcpServers: [globalHttpServer],
458+
mcpServersSource: "global",
459+
projectTrust: { trustedPluginPaths: [], trustedMcpFingerprints: [] },
460+
});
461+
462+
await toolset.connectMCPServer(globalHttpServer, {
463+
interactiveAuth: false,
464+
onStatus: () => {},
465+
onToolsChanged: () => {},
466+
});
467+
468+
expect(mockConnectMCPServer).toHaveBeenCalledTimes(1);
469+
expect(mockConnectMCPServer.mock.calls[0]?.[0]).toEqual(globalHttpServer);
470+
await toolset.dispose();
471+
});
472+
473+
test("startup connectMCP still fail-closes untrusted local servers", async () => {
474+
mockConnectMCPServer.mockClear();
475+
const statuses: { name: string; state: string; error?: string }[] = [];
476+
const toolset = await createAgentToolset({
477+
cwd: "/fake",
478+
permissionGate: fakePermissionGate,
479+
onOperatorGate: async () => ({ kind: "cancel" }),
480+
mcpServers: [localStdioServer],
481+
mcpServersSource: "local",
482+
projectTrust: { trustedPluginPaths: [], trustedMcpFingerprints: [] },
483+
});
484+
485+
await toolset.connectMCP({
486+
interactiveAuth: false,
487+
onStatus: (status) => statuses.push(status),
488+
onToolsChanged: () => {},
489+
});
490+
491+
expect(mockConnectMCPServer).not.toHaveBeenCalled();
492+
expect(statuses.some((s) => s.name === "evil" && s.state === "failed")).toBe(true);
493+
await toolset.dispose();
494+
});
495+
365496
test("dispose calls posixTools.dispose", async () => {
366497
mockDispose.mockClear();
367498

0 commit comments

Comments
 (0)