diff --git a/assistant/src/daemon/__tests__/conversation-tool-setup.test.ts b/assistant/src/daemon/__tests__/conversation-tool-setup.test.ts new file mode 100644 index 00000000000..38b1ff5c3c3 --- /dev/null +++ b/assistant/src/daemon/__tests__/conversation-tool-setup.test.ts @@ -0,0 +1,122 @@ +/** + * Tests for `isToolActiveForContext` host-tool capability gating. + * + * The chrome-extension interface only supports the `host_browser` host proxy + * capability, so host tools must be gated by + * `supportsHostProxy(transport, capability)` instead of a single + * `hasNoClient` check. These tests assert that: + * + * - Each host tool is only projected for transports whose interface supports + * the matching capability (e.g. `host_bash` is only active for macOS, not + * chrome-extension — regression coverage for the Codex P1 host-tool leak). + * - The `hasNoClient` precondition still takes precedence so HTTP-only paths + * never see host tools. + * - Contexts without a `transportInterface` fall back to the permissive + * coarse-grained behavior so callers that have not yet plumbed the field + * through `SkillProjectionContext` continue to see the host tools their + * `hasNoClient` state allows. + */ + +import { describe, expect, test } from "bun:test"; + +import type { SkillProjectionCache } from "../conversation-skill-tools.js"; +import { + isToolActiveForContext, + type SkillProjectionContext, +} from "../conversation-tool-setup.js"; + +function makeCtx( + overrides: Partial = {}, +): SkillProjectionContext { + return { + skillProjectionState: new Map(), + skillProjectionCache: {} as SkillProjectionCache, + coreToolNames: new Set(), + toolsDisabledDepth: 0, + ...overrides, + }; +} + +describe("isToolActiveForContext — host tool capability gating", () => { + test("host_bash is active for macOS transport (full host proxy support)", () => { + expect( + isToolActiveForContext( + "host_bash", + makeCtx({ hasNoClient: false, transportInterface: "macos" }), + ), + ).toBe(true); + }); + + test("host_bash is NOT active for chrome-extension transport (Codex P1 regression)", () => { + // Regression coverage: chrome-extension only supports `host_browser`, so + // `host_bash` must NOT be projected even though a client is connected + // (`hasNoClient: false`). Without per-capability gating the model could + // attempt host_bash calls that the transport cannot dispatch. + expect( + isToolActiveForContext( + "host_bash", + makeCtx({ + hasNoClient: false, + transportInterface: "chrome-extension", + }), + ), + ).toBe(false); + }); + + test("host_browser is active for chrome-extension transport", () => { + expect( + isToolActiveForContext( + "host_browser", + makeCtx({ + hasNoClient: false, + transportInterface: "chrome-extension", + }), + ), + ).toBe(true); + }); + + test("host_browser is active for macOS transport", () => { + expect( + isToolActiveForContext( + "host_browser", + makeCtx({ hasNoClient: false, transportInterface: "macos" }), + ), + ).toBe(true); + }); + + test("host_file_read is NOT active for chrome-extension transport", () => { + expect( + isToolActiveForContext( + "host_file_read", + makeCtx({ + hasNoClient: false, + transportInterface: "chrome-extension", + }), + ), + ).toBe(false); + }); + + test("host_bash respects hasNoClient even when transport supports it", () => { + // The existing `hasNoClient` gate must continue to take precedence: even + // a macOS-capable transport must not surface host tools when no client is + // actually connected (e.g. the HTTP-only path). + expect( + isToolActiveForContext( + "host_bash", + makeCtx({ hasNoClient: true, transportInterface: "macos" }), + ), + ).toBe(false); + }); + + test("host_bash falls back to permissive behavior when transport is undefined", () => { + // Backwards-compat fallback: contexts that don't pass a transport + // interface (e.g. tests, callers that haven't plumbed the new field) + // keep the coarse-grained behavior so we don't accidentally hide tools. + expect( + isToolActiveForContext( + "host_bash", + makeCtx({ hasNoClient: false, transportInterface: undefined }), + ), + ).toBe(true); + }); +}); diff --git a/assistant/src/daemon/conversation-tool-setup.ts b/assistant/src/daemon/conversation-tool-setup.ts index f6f163528e2..d2f112733c3 100644 --- a/assistant/src/daemon/conversation-tool-setup.ts +++ b/assistant/src/daemon/conversation-tool-setup.ts @@ -6,6 +6,11 @@ * keeping the constructor body focused on wiring. */ +import { + type HostProxyCapability, + type InterfaceId, + supportsHostProxy, +} from "../channels/types.js"; import { isHttpAuthDisabled } from "../config/env.js"; import { getIsPlatform } from "../config/env-registry.js"; import type { CesClient } from "../credential-execution/client.js"; @@ -459,6 +464,15 @@ export interface SkillProjectionContext { subagentAllowedTools?: Set; /** True when this conversation belongs to a subagent spawned by SubagentManager. */ readonly isSubagent?: boolean; + /** + * The interface id of the connected client driving the current turn (e.g. + * "macos", "chrome-extension"). Used to gate host tools by per-capability + * `supportsHostProxy(transport, capability)` so that interfaces which only + * support a subset of the host proxy set (e.g. chrome-extension supports + * `host_browser` but not `host_bash`/`host_file`) do not leak unsupported + * host tools into the LLM tool definitions. + */ + readonly transportInterface?: InterfaceId; } // ── Conditional tool sets ──────────────────────────────────────────── @@ -469,6 +483,25 @@ const HOST_TOOL_NAMES = new Set([ "host_file_write", "host_file_edit", "host_bash", + "host_browser", +]); +/** + * Maps each host tool name to the host proxy capability that the connected + * client interface must support. `isToolActiveForContext` uses this to gate + * each host tool individually so that partial-capability transports (e.g. + * chrome-extension only supports `host_browser`) only see the host tools + * their interface can actually service. + * + * Note: there is no `host_cu` tool exposed via the tool gating layer today; + * computer-use is preactivated as a skill and projected through the skill + * tools path. Only the host tools listed in `HOST_TOOL_NAMES` need entries. + */ +const HOST_TOOL_TO_CAPABILITY = new Map([ + ["host_bash", "host_bash"], + ["host_file_read", "host_file"], + ["host_file_write", "host_file"], + ["host_file_edit", "host_file"], + ["host_browser", "host_browser"], ]); const CLIENT_CAPABILITY_TOOL_NAMES = new Set(["app_open"]); const PLATFORM_TOOL_NAMES = new Set(["request_system_permission"]); @@ -501,7 +534,21 @@ export function isToolActiveForContext( // Host tools require a connected client — without one, there is no human // to approve execution and the guardian auto-approve path would allow // unchecked host command execution on the daemon host. - return !ctx.hasNoClient; + if (ctx.hasNoClient) return false; + // Then gate per-capability against the connected interface so that + // partial-capability transports (e.g. chrome-extension only supports + // `host_browser`) only see host tools their interface can service. + // Without this check, host_bash/host_file_* would surface for + // chrome-extension sessions and the model would attempt calls the + // transport cannot dispatch. + const capability = HOST_TOOL_TO_CAPABILITY.get(name); + const transport = ctx.transportInterface; + // Backwards-compat fallback: contexts that have not yet been plumbed + // with a transport interface (tests, callers that don't pass the new + // field) keep the coarse-grained behavior so we don't accidentally hide + // tools from them. + if (!transport || !capability) return true; + return supportsHostProxy(transport, capability); } if (CLIENT_CAPABILITY_TOOL_NAMES.has(name)) { return !ctx.hasNoClient; diff --git a/assistant/src/daemon/conversation.ts b/assistant/src/daemon/conversation.ts index fbe24f40cee..3b7bb564f4e 100644 --- a/assistant/src/daemon/conversation.ts +++ b/assistant/src/daemon/conversation.ts @@ -18,6 +18,7 @@ import type { ResolvedSystemPrompt } from "../agent/loop.js"; import { AgentLoop } from "../agent/loop.js"; import type { + InterfaceId, TurnChannelContext, TurnInterfaceContext, } from "../channels/types.js"; @@ -1090,6 +1091,16 @@ export class Conversation { return this.currentTurnInterfaceContext; } + /** + * Implements the `transportInterface` field of `SkillProjectionContext` so + * that `isToolActiveForContext` can gate host tools by per-capability + * `supportsHostProxy(transport, capability)`. Derived from the live turn + * interface context so it tracks the connected client across turns. + */ + get transportInterface(): InterfaceId | undefined { + return this.currentTurnInterfaceContext?.userMessageInterface; + } + async persistUserMessage( content: string, attachments: UserMessageAttachment[],