fix(cli): validate ACP tool allowlists after discovery
This commit is contained in:
@@ -349,7 +349,7 @@ export interface AcpSessionFactoryOptions {
|
||||
sessionDir?: string;
|
||||
authStorage: AuthStorage;
|
||||
modelRegistry: ModelRegistry;
|
||||
parsedArgs: Pick<Args, "apiKey" | "trustedExtensions">;
|
||||
parsedArgs: Pick<Args, "apiKey" | "trustedExtensions" | "tools">;
|
||||
rawArgs: string[];
|
||||
createSession: (options: CreateAgentSessionOptions) => Promise<CreateAgentSessionResult>;
|
||||
}
|
||||
@@ -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;
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user