diff --git a/src/agent/fleet-verbs-mount.test.ts b/src/agent/fleet-verbs-mount.test.ts index fc8bb3c15..ce572269b 100644 --- a/src/agent/fleet-verbs-mount.test.ts +++ b/src/agent/fleet-verbs-mount.test.ts @@ -55,6 +55,48 @@ describe("primary fleet verb mount", () => { await toolset.dispose(); }); + test("createAgentToolset mounts tool_search by default", async () => { + const cwd = mkdtempSync(join(tmpdir(), "corbits-fleet-mount-")); + const { createAgentToolset } = await import("./tools.js"); + const permissionGate = { + check: async () => ({ allowed: true }), + getSkipPermissions: () => false, + } as never; + + const toolset = await createAgentToolset({ cwd, permissionGate }); + try { + const names = toolset.dynamicRunner + .currentDefinitions() + .map((d) => d.name); + expect(names).toContain("tool_search"); + } finally { + await toolset.dispose(); + } + }); + + test("createAgentToolset omits tool_search when the allow list excludes it", async () => { + const cwd = mkdtempSync(join(tmpdir(), "corbits-fleet-mount-")); + const { createAgentToolset } = await import("./tools.js"); + const permissionGate = { + check: async () => ({ allowed: true }), + getSkipPermissions: () => false, + } as never; + + const toolset = await createAgentToolset({ + cwd, + permissionGate, + toolSearchAllow: ["read_file"], + }); + try { + const names = toolset.dynamicRunner + .currentDefinitions() + .map((d) => d.name); + expect(names).not.toContain("tool_search"); + } finally { + await toolset.dispose(); + } + }); + test("createAgentToolset mounts wait_agents when mountWaitAgents is true (exec primary)", async () => { const cwd = mkdtempSync(join(tmpdir(), "corbits-fleet-mount-")); const { createAgentToolset } = await import("./tools.js"); diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index a65be8e69..01b0f3b26 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -227,6 +227,12 @@ describe("createToolIndex", () => { test("returns nothing for an empty query", () => { expect(index.search(" ")).toEqual([]); }); + + test("with an allow list, never returns tools outside the allow list", () => { + const allowed = createToolIndex(() => defs, [], ["present"]); + expect(allowed.search("pages")).toContain("present"); + expect(allowed.search("linear")).not.toContain("mcp__linear__create_issue"); + }); }); function call( diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index 303017252..e51ebdb6c 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -237,6 +237,9 @@ function tokenize(text: string): string[] { export function createToolIndex( getDefs: () => readonly ToolDefinition[], advertisedNames: readonly string[] = ADVERTISED_TOOL_NAMES, + // Closed allow list (exec director overlays): when set, the index only + // surfaces allowed tools so search cannot promote outside the allow. + allow?: readonly string[] | undefined, ): ToolIndex { const score = ( def: ToolDefinition, @@ -264,6 +267,7 @@ export function createToolIndex( if (queryTokens.length === 0) return []; return getDefs() .filter((def) => !advertisedNames.includes(def.name)) + .filter((def) => allow === undefined || allow.includes(def.name)) .map((def) => ({ name: def.name, score: score(def, queryTokens, rawQuery), diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 2a7461e0d..840393820 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -108,6 +108,7 @@ import { createToolIndex, createToolSearchTool, TOOL_SEARCH_PENDING_WAIT_MS, + toolSearchDefinition, } from "./tool-search.js"; import { createSearchAgentsTool } from "./agent-search.js"; import { createReadAgentTraceTool } from "../subagent/trace-tool.js"; @@ -266,6 +267,13 @@ export interface AgentToolsetArgs { * and collect worker reports from mailbox mail instead. */ mountWaitAgents?: boolean; + /** + * Closed allow list (exec director overlays). tool_search is mounted only + * when the allow includes it, and the search index only surfaces allowed + * tools so search cannot promote outside the allow. Omit for the product + * default (tool_search mounted, index over the live registry). + */ + toolSearchAllow?: readonly string[]; } // Per-server connection state surfaced to the TUI. @@ -722,21 +730,32 @@ export async function createAgentToolset( const toolIndex = createToolIndex( () => runnerHolder.current?.currentDefinitions() ?? [], advertisedBuiltIns, + args.toolSearchAllow, ); - baseTools.push( - createToolSearchTool({ - search: (query) => toolIndex.search(query), - lookup: (name) => - runnerHolder.current?.currentDefinitions().find((d) => d.name === name), - promote: (names) => promoter.promote(names), - // Misses wait briefly for in-flight MCP handshakes (bounded, so hung - // OAuth cannot hang the call) and re-search before answering. Reads the - // connection map live — declared below, populated by the time any - // search runs. - awaitPendingConnections: (timeoutMs = TOOL_SEARCH_PENDING_WAIT_MS) => - awaitPendingMcpConnections(timeoutMs), - }), - ); + // Closed exec allow lists omit tool_search itself (leaf posture); when the + // allow excludes it the tool is never mounted, so there is nothing to + // search with and nothing the promoter can activate. + if ( + args.toolSearchAllow === undefined || + args.toolSearchAllow.includes(toolSearchDefinition.name) + ) { + baseTools.push( + createToolSearchTool({ + search: (query) => toolIndex.search(query), + lookup: (name) => + runnerHolder.current + ?.currentDefinitions() + .find((d) => d.name === name), + promote: (names) => promoter.promote(names), + // Misses wait briefly for in-flight MCP handshakes (bounded, so hung + // OAuth cannot hang the call) and re-search before answering. Reads the + // connection map live — declared below, populated by the time any + // search runs. + awaitPendingConnections: (timeoutMs = TOOL_SEARCH_PENDING_WAIT_MS) => + awaitPendingMcpConnections(timeoutMs), + }), + ); + } // Codex apply_patch mounts when isCodex; primary strips it so Corbits DIY // stays on write_file/edit_file/delete_file. Leaves keep it via BUILD/DOCS allowlists. diff --git a/src/exec/runner.test.ts b/src/exec/runner.test.ts new file mode 100644 index 000000000..106a182d6 --- /dev/null +++ b/src/exec/runner.test.ts @@ -0,0 +1,116 @@ +import { describe, expect, test } from "bun:test"; +import { DIRECTOR_REGISTRY } from "../agent/directors/registry.js"; +import { createAdvertisedToolset } from "../session/assemble-runtime.js"; +import { + createExecToolCallGate, + createExecToolPromoter, + isExecOverlayToolAllowed, + resolveExecDirectorOverlay, + resolveExecDirectorOverlayForPackage, +} from "./runner.js"; + +const OUTSIDE_ALLOW = "mcp__linear__create_issue"; + +describe("exec director allowlist", () => { + test("explorer overlay narrows advertised tools to the package allow list", () => { + const overlay = resolveExecDirectorOverlay("explorer"); + expect(overlay.advertisedAllow).toBeDefined(); + expect(overlay.advertisedAllow).toContain("read_file"); + expect(overlay.advertisedAllow).toContain("run_shell"); + expect(overlay.advertisedAllow).not.toContain("tool_search"); + expect(overlay.advertisedAllow).not.toContain(OUTSIDE_ALLOW); + }); + + test("critic overlay narrows advertised tools to the package allow list", () => { + const overlay = resolveExecDirectorOverlay("critic"); + expect(overlay.advertisedAllow).toBeDefined(); + expect(overlay.advertisedAllow).not.toContain("tool_search"); + expect(overlay.advertisedAllow).not.toContain(OUTSIDE_ALLOW); + }); + + test("skywalker keeps the product default — no allow list", () => { + const overlay = resolveExecDirectorOverlay("skywalker"); + expect(overlay.advertisedAllow).toBeUndefined(); + expect(overlay.mountFleet).toBe(true); + }); + + test("deny entries are subtracted from the allow list", () => { + const pkg = { + ...DIRECTOR_REGISTRY.explorer, + tools: { allow: ["read_file", "run_shell"], deny: ["run_shell"] }, + }; + expect(resolveExecDirectorOverlayForPackage(pkg).advertisedAllow).toEqual([ + "read_file", + ]); + }); + + test("an allow that deny empties is rejected loudly", () => { + const pkg = { + ...DIRECTOR_REGISTRY.explorer, + tools: { allow: ["run_shell"], deny: ["run_shell"] }, + }; + expect(() => resolveExecDirectorOverlayForPackage(pkg)).toThrow(/empty/); + }); + + test("a deny-only package config is rejected loudly", () => { + const pkg = { + ...DIRECTOR_REGISTRY.explorer, + tools: { deny: ["run_shell"] }, + }; + expect(() => resolveExecDirectorOverlayForPackage(pkg)).toThrow(/deny/); + }); + + test("promote cannot make an outside-allow tool callable under explorer", () => { + const overlay = resolveExecDirectorOverlay("explorer"); + expect(isExecOverlayToolAllowed(overlay, OUTSIDE_ALLOW)).toBe(false); + const { activated, computeAdvertised, isAdvertised } = + createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: true }, + getProvider: () => ({ providerName: "test", model: "test-model" }), + builtInPrefix: overlay.advertisedAllow, + }); + const promote = createExecToolPromoter({ + activate: (names) => activated.activate(names), + isAllowed: (name) => isExecOverlayToolAllowed(overlay, name), + currentDefinitions: () => [], + computeAdvertised, + updateDirectorTools: () => undefined, + }); + promote([OUTSIDE_ALLOW]); + expect(activated.has(OUTSIDE_ALLOW)).toBe(false); + expect(isAdvertised(OUTSIDE_ALLOW)).toBe(false); + expect( + createExecToolCallGate(isAdvertised, { isCodex: false })(OUTSIDE_ALLOW), + ).toBe(false); + }); + + test("the promoter gates allow itself — a raw activate caller gets no bypass", () => { + const overlay = resolveExecDirectorOverlay("explorer"); + const { activated, computeAdvertised } = createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: true }, + getProvider: () => ({ providerName: "test", model: "test-model" }), + builtInPrefix: overlay.advertisedAllow, + }); + let advertisedCount = 0; + const promote = createExecToolPromoter({ + activate: (names) => activated.activate(names), + isAllowed: (name) => isExecOverlayToolAllowed(overlay, name), + currentDefinitions: () => [], + computeAdvertised, + updateDirectorTools: () => { + advertisedCount += 1; + }, + }); + promote([OUTSIDE_ALLOW, "read_file"]); + expect(activated.has(OUTSIDE_ALLOW)).toBe(false); + expect(activated.has("read_file")).toBe(true); + expect(advertisedCount).toBe(1); + }); + + test("skywalker overlay leaves every tool allowed", () => { + const overlay = resolveExecDirectorOverlay("skywalker"); + expect(isExecOverlayToolAllowed(overlay, OUTSIDE_ALLOW)).toBe(true); + }); +}); diff --git a/src/exec/runner.ts b/src/exec/runner.ts index e2d659eda..9ee7aa48c 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -16,7 +16,7 @@ import { import { xaiProfileFromProviderName } from "../config/xai-providers.js"; import { formatDirectorSystemPrompt } from "../agent/directors/identity.js"; import { DIRECTOR_REGISTRY } from "../agent/directors/registry.js"; -import type { DirectorId } from "../agent/directors/types.js"; +import type { DirectorId, DirectorPackage } from "../agent/directors/types.js"; import { submitOutputDefinition } from "../agent/director.js"; import { shellDefinition, @@ -265,19 +265,58 @@ export function resolveExecDirectorOverlay( if (director === undefined || director === "skywalker") { return { mountFleet: true }; } - const pkg = DIRECTOR_REGISTRY[director]; + return resolveExecDirectorOverlayForPackage(DIRECTOR_REGISTRY[director]); +} + +/** + * Single enforcement point for the exec allowlist. Everything the overlay + * permits — tool_search results, promoter activation, the call gate — flows + * through here, so a tool outside the allow can never become callable. + */ +export function isExecOverlayToolAllowed( + overlay: ExecDirectorOverlay, + name: string, +): boolean { + return ( + overlay.advertisedAllow === undefined || + overlay.advertisedAllow.includes(name) + ); +} + +export function resolveExecDirectorOverlayForPackage( + pkg: DirectorPackage, +): ExecDirectorOverlay { const allow = pkg.tools?.allow; - const advertisedAllow = + const deny = pkg.tools?.deny ?? []; + if ((allow === undefined || allow.length === 0) && deny.length > 0) { + throw new Error( + `Director package "${pkg.id}" sets tools.deny without tools.allow — ` + + "exec overlays enforce a closed allow list, so a deny-only package " + + "has no list to subtract from. Add tools.allow.", + ); + } + const allowed = allow !== undefined && allow.length > 0 + ? allow.filter((name) => !deny.includes(name)) + : undefined; + if (allowed !== undefined && allowed.length === 0) { + throw new Error( + `Director package "${pkg.id}" tools.allow minus tools.deny is empty — ` + + "exec overlays enforce a closed allow list, so no tool would be " + + "advertised. Keep an allow entry outside tools.deny.", + ); + } + const advertisedAllow = + allowed !== undefined ? pkg.spawn.maySpawn ? [ - ...allow, + ...allowed, // Exec mounts wait_agents beside the fleet verbs (mountWaitAgents), // so it stays advertised here even though the package allow omits // it for TUI/nested mailbox-mail collection. - ...(!allow.includes("wait_agents") ? ["wait_agents"] : []), + ...(!allowed.includes("wait_agents") ? ["wait_agents"] : []), ] - : allow.filter( + : allowed.filter( (name) => ![ "search_agents", @@ -359,13 +398,14 @@ export function createExecToolCallGate( export function createExecToolPromoter(args: { activate: (names: readonly string[]) => boolean; + isAllowed: (name: string) => boolean; currentDefinitions: () => readonly ToolDefinition[]; computeAdvertised: (all: readonly ToolDefinition[]) => ToolDefinition[]; updateDirectorTools: (defs: ToolDefinition[]) => void; persist?: () => void; }): (names: string[]) => void { return (names) => { - if (!args.activate(names)) return; + if (!args.activate(names.filter((name) => args.isAllowed(name)))) return; args.updateDirectorTools(args.computeAdvertised(args.currentDefinitions())); args.persist?.(); }; @@ -649,6 +689,11 @@ export async function runExec(config: Config): Promise { : {}), sessionMode, toolAvailability, + // Closed director overlays unmount tool_search (unless allowed) and + // filter its index, so search cannot surface outside-allow tools. + ...(overlay.advertisedAllow !== undefined + ? { toolSearchAllow: overlay.advertisedAllow } + : {}), // Exec-primary keeps wait_agents mounted (with an advertised allow): // headless runs have no mailbox-mail flush, so wait_agents stays the // collection path here. TUI primary and nested orchestrators omit it. @@ -839,10 +884,12 @@ export async function runExec(config: Config): Promise { // tool_search starts as a no-op promoter; without this, the call gate // refuses MCP/present/plugin names the result just told the model to - // invoke. + // invoke. Under a closed overlay the promoter only activates allowed + // names, so outside-allow tools can never become advertised or callable. agentToolset.setToolPromoter( createExecToolPromoter({ activate: (names) => activatedToolNames.activate(names), + isAllowed: (name) => isExecOverlayToolAllowed(overlay, name), currentDefinitions: () => agentToolset.dynamicRunner.currentDefinitions(), computeAdvertised, diff --git a/tests/unit/exec/runner.test.ts b/tests/unit/exec/runner.test.ts index 057cc6e53..d705b7c19 100644 --- a/tests/unit/exec/runner.test.ts +++ b/tests/unit/exec/runner.test.ts @@ -784,6 +784,7 @@ describe("exec tool call gate and promoter", () => { const directorNames: string[][] = []; const promote = createExecToolPromoter({ activate: (names) => activated.activate(names), + isAllowed: () => true, currentDefinitions: () => runner.currentDefinitions(), computeAdvertised, updateDirectorTools: (defs) => {