Skip to content

Commit 319aee7

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 d433576 commit 319aee7

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";
@@ -535,6 +539,38 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
535539
const mcpAbortController = new AbortController();
536540
let disposed = false;
537541
let disposal: Promise<void> | undefined;
542+
let mcpTrustStore: ProjectTrustStore = projectTrust ?? {
543+
trustedPluginPaths: [],
544+
trustedMcpFingerprints: [],
545+
};
546+
const untrustedLocalError = `Not trusted for this project (see ${SETTINGS_DIR_NAME}/trust.json)`;
547+
548+
const filterServersForConnect = async (
549+
servers: MCPServerConfig[],
550+
): Promise<MCPServerConfig[]> => {
551+
const allowed = await filterMcpServersForConnect(servers, {
552+
source: mcpServersSource,
553+
store: mcpTrustStore,
554+
cwd,
555+
...(requestMcpTrust !== undefined ? { requestTrust: requestMcpTrust } : {}),
556+
});
557+
if (mcpServersSource !== "local") return allowed;
558+
// Remember grants so connectOneMCPServer does not re-prompt after startup TOFU.
559+
let fingerprints = mcpTrustStore.trustedMcpFingerprints;
560+
let changed = false;
561+
for (const server of allowed) {
562+
if (isBuiltinExaMCPServer(server)) continue;
563+
const fp = mcpServerFingerprint(server);
564+
if (!fingerprints.includes(fp)) {
565+
fingerprints = [...fingerprints, fp];
566+
changed = true;
567+
}
568+
}
569+
if (changed) {
570+
mcpTrustStore = { ...mcpTrustStore, trustedMcpFingerprints: fingerprints };
571+
}
572+
return allowed;
573+
};
538574

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

553589
const run = (async () => {
590+
if (mcpServersSource === "local") {
591+
const allowed = await filterServersForConnect([config]);
592+
if (disposed) return;
593+
if (allowed.length === 0) {
594+
callbacks.onStatus({
595+
name: config.name,
596+
state: "failed",
597+
error: untrustedLocalError,
598+
});
599+
return;
600+
}
601+
}
554602
callbacks.onStatus({ name: config.name, state: "connecting" });
555603
let result: MCPConnectResult;
556604
try {
@@ -645,12 +693,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
645693
signal?: AbortSignal,
646694
): Promise<void> => {
647695
if (disposed) return;
648-
const toConnect = await filterMcpServersForConnect(mcpServers, {
649-
source: mcpServersSource,
650-
store: projectTrust ?? { trustedPluginPaths: [], trustedMcpFingerprints: [] },
651-
cwd,
652-
...(requestMcpTrust !== undefined ? { requestTrust: requestMcpTrust } : {}),
653-
});
696+
const toConnect = await filterServersForConnect(mcpServers);
654697
if (disposed) return;
655698
await Promise.all(toConnect.map((config) => connectOneMCPServer(config, callbacks, signal)));
656699
if (disposed) return;
@@ -662,7 +705,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
662705
callbacks.onStatus({
663706
name: server.name,
664707
state: "failed",
665-
error: `Not trusted for this project (see ${SETTINGS_DIR_NAME}/trust.json)`,
708+
error: untrustedLocalError,
666709
});
667710
}
668711
}

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

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

78
const mockDispose = mock(async () => {});
@@ -352,6 +353,136 @@ test("headless MCP connection does not wait for interactive OAuth", async () =>
352353
expect(mockConnectMCPServer.mock.calls[0]?.[1]?.onAuthURL).toBeUndefined();
353354
});
354355

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

0 commit comments

Comments
 (0)