fix(exec): filter onToolsActivate names through overlay allow list

Author: TheGreatAxios

src/exec/runner.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, test } from "bun:test";
+import { readFileSync } from "node:fs";
 import { DIRECTOR_REGISTRY } from "../agent/directors/registry.js";
 import { createAdvertisedToolset } from "../session/assemble-runtime.js";
 import {
@@ -114,6 +115,97 @@ describe("exec director allowlist", () => {
     );
   });
 
+  test("the promoter does not commit a name outside the overlay allow list", () => {
+    const overlay = resolveExecDirectorOverlay("explorer");
+    const { activated, computeAdvertised, flushPromotions } =
+      createAdvertisedToolset({
+        sessionMode: "orchestrator",
+        toolAvailability: { languageServerAvailable: true },
+        getProvider: () => ({ providerName: "test", model: "test-model" }),
+        builtInPrefix: overlay.advertisedAllow,
+      });
+    let committed = 0;
+    const promote = createExecToolPromoter({
+      activate: (names) => activated.activate(names),
+      isAllowed: (name) => isExecOverlayToolAllowed(overlay, name),
+      commitWire: () => {
+        if (flushPromotions()) committed += 1;
+      },
+    });
+    const registry = [
+      {
+        name: OUTSIDE_ALLOW,
+        description: "create an issue",
+        inputSchema: { type: "object", properties: {} },
+      },
+      {
+        name: "lsp",
+        description: "language server",
+        inputSchema: { type: "object", properties: {} },
+      },
+    ];
+    promote([OUTSIDE_ALLOW]);
+    expect(activated.has(OUTSIDE_ALLOW)).toBe(false);
+    expect(committed).toBe(0);
+    expect(computeAdvertised(registry).map((d) => d.name)).not.toContain(
+      OUTSIDE_ALLOW,
+    );
+  });
+
+  test("the promoter still commits lsp when the overlay allows it", () => {
+    const overlay = resolveExecDirectorOverlay("explorer");
+    expect(isExecOverlayToolAllowed(overlay, "lsp")).toBe(true);
+    const { activated, computeAdvertised, flushPromotions } =
+      createAdvertisedToolset({
+        sessionMode: "orchestrator",
+        toolAvailability: { languageServerAvailable: true },
+        getProvider: () => ({ providerName: "test", model: "test-model" }),
+        builtInPrefix: overlay.advertisedAllow,
+      });
+    let committed = 0;
+    const promote = createExecToolPromoter({
+      activate: (names) => activated.activate(names),
+      isAllowed: (name) => isExecOverlayToolAllowed(overlay, name),
+      commitWire: () => {
+        if (flushPromotions()) committed += 1;
+      },
+    });
+    const registry = [
+      {
+        name: "lsp",
+        description: "language server",
+        inputSchema: { type: "object", properties: {} },
+      },
+      {
+        name: OUTSIDE_ALLOW,
+        description: "create an issue",
+        inputSchema: { type: "object", properties: {} },
+      },
+    ];
+    promote([OUTSIDE_ALLOW, "lsp"]);
+    expect(activated.has(OUTSIDE_ALLOW)).toBe(false);
+    expect(activated.has("lsp")).toBe(true);
+    expect(committed).toBe(1);
+    const names = computeAdvertised(registry).map((d) => d.name);
+    expect(names).toContain("lsp");
+    expect(names).not.toContain(OUTSIDE_ALLOW);
+  });
+
+  test("exec wires commitWire on the overlay-filtered promoter and onToolsActivate uses it", () => {
+    const source = readFileSync(
+      new URL("./runner.ts", import.meta.url),
+      "utf8",
+    );
+    expect(source).toContain("commitWire: commitPromotedWire");
+    expect(source).toMatch(
+      /createExecToolPromoter\(\{[\s\S]*?isAllowed:\s*\(name\)\s*=>\s*isExecOverlayToolAllowed\(overlay,\s*name\)[\s\S]*?commitWire:\s*commitPromotedWire/,
+    );
+    expect(source).toMatch(
+      /onToolsActivate:\s*\(names\)\s*=>\s*promoteAndCommitWire\(names\)/,
+    );
+    expect(source).toContain("setToolPromoter(promoteAndCommitWire");
+  });
+
   test("skywalker overlay leaves every tool allowed", () => {
     const overlay = resolveExecDirectorOverlay("skywalker");
     expect(isExecOverlayToolAllowed(overlay, OUTSIDE_ALLOW)).toBe(true);

src/exec/runner.ts

@@ -214,8 +214,9 @@ export function resolveExecDirectorOverlay(
 
 /**
  * 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.
+ * permits — tool_search results, promoter activation, onToolsActivate, the
+ * call gate — flows through here, so a tool outside the allow can never
+ * become callable.
  */
 export function isExecOverlayToolAllowed(
   overlay: ExecDirectorOverlay,
@@ -840,16 +841,15 @@ export async function runExec(config: Config): Promise<ExecResult> {
         );
       }
     };
-    agentToolset.setToolPromoter(
-      createExecToolPromoter({
-        activate: (names) => activatedToolNames.activate(names),
-        isAllowed: (name) => isExecOverlayToolAllowed(overlay, name),
-        persist: () => {
-          void persist("running");
-        },
-        commitWire: commitPromotedWire,
-      }),
-    );
+    const promoteAndCommitWire = createExecToolPromoter({
+      activate: (names) => activatedToolNames.activate(names),
+      isAllowed: (name) => isExecOverlayToolAllowed(overlay, name),
+      persist: () => {
+        void persist("running");
+      },
+      commitWire: commitPromotedWire,
+    });
+    agentToolset.setToolPromoter(promoteAndCommitWire);
 
     const workflowHost = new WorkflowHost({
       cwd: config.cwd,
@@ -941,10 +941,7 @@ export async function runExec(config: Config): Promise<ExecResult> {
               tasks: tasks.map((t) => `${t.status}:${t.title}`).join(", "),
             });
           },
-          onToolsActivate: (names) => {
-            if (!activatedToolNames.activate(names)) return;
-            commitPromotedWire();
-          },
+          onToolsActivate: (names) => promoteAndCommitWire(names),
         },
         (message, fields) => logger.debug(message, fields),
       );