Merge pull request #1010 from corbitsdev/cl-6822-exec-director-tool-allowlist-is-advisory-tool_search-re

Enforce exec director tool allowlists through tool_search and promote

Author: TheGreatAxios

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");

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(

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),

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.

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);
+  });
+});

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<ExecResult> {
         : {}),
       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<ExecResult> {
 
     // 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,

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) => {