From 4ce4874e7800a7c26332f8c74ce635b747eecfb8 Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 13 Aug 2026 16:53:12 +0200 Subject: [PATCH] fix(cli): validate ACP tool allowlists after discovery --- packages/coding-agent/src/main.ts | 13 ++++++- .../test/acp-mcp-isolation.test.ts | 37 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 54ff3b1d1..4a8f65cee 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -349,7 +349,7 @@ export interface AcpSessionFactoryOptions { sessionDir?: string; authStorage: AuthStorage; modelRegistry: ModelRegistry; - parsedArgs: Pick; + parsedArgs: Pick; rawArgs: string[]; createSession: (options: CreateAgentSessionOptions) => Promise; } @@ -426,7 +426,7 @@ export function createAcpSessionFactory(args: AcpSessionFactoryOptions): AcpSess args.authStorage.setRuntimeApiKey(nextSession.model.provider, args.parsedArgs.apiKey); } const runner = nextSession.extensionRunner; - applyExtensionFlags( + const reparsedArgs = applyExtensionFlags( runner ? { getFlags: () => runner.getFlags(), @@ -437,6 +437,15 @@ export function createAcpSessionFactory(args: AcpSessionFactoryOptions): AcpSess : undefined, args.rawArgs, ); + const requestedTools = reparsedArgs?.tools ?? args.parsedArgs.tools; + if (requestedTools) { + try { + validateToolNames(requestedTools, nextSession.getAllToolNames()); + } catch (error) { + await nextSession.dispose(); + throw error; + } + } return nextSession; }; } diff --git a/packages/coding-agent/test/acp-mcp-isolation.test.ts b/packages/coding-agent/test/acp-mcp-isolation.test.ts index d42231ebe..2f50660af 100644 --- a/packages/coding-agent/test/acp-mcp-isolation.test.ts +++ b/packages/coding-agent/test/acp-mcp-isolation.test.ts @@ -75,6 +75,43 @@ describe("createAcpSessionFactory MCP isolation (issue #1234)", () => { } }); + it("rejects allowlisted tools absent from the completed ACP session registry", async () => { + const tempDir = TempDir.createSync("@pi-acp-tool-allowlist-"); + let authStorage: AuthStorage | undefined; + try { + authStorage = await AuthStorage.create(tempDir.join("auth.db")); + const modelRegistry = new ModelRegistry(authStorage); + const settings = Settings.isolated({}); + let disposed = false; + const fakeSession = { + extensionRunner: undefined, + getAllToolNames: () => ["read"], + dispose: async () => { + disposed = true; + }, + } as unknown as AgentSession; + const factory = createAcpSessionFactory({ + baseOptions: {} as CreateAgentSessionOptions, + settings, + sessionDir: tempDir.join("sessions"), + authStorage, + modelRegistry, + parsedArgs: { tools: ["read", "missing"] }, + rawArgs: ["--tools", "read,missing"], + createSession: async () => ({ session: fakeSession }) as CreateAgentSessionResult, + }); + + await expect(factory(tempDir.path())).rejects.toThrow(/Unknown tool in --tools: missing/); + expect(disposed).toBe(true); + } finally { + try { + authStorage?.close(); + } finally { + await tempDir.remove(); + } + } + }); + it("shares the trusted extension EventBus with the ACP session", async () => { const tempDir = TempDir.createSync("@pi-acp-trusted-extension-"); let authStorage: AuthStorage | undefined;