Merge pull request #1545 from corbitsdev/cl-10786-readonly-tools-on-main

fix(permission): auto-allow read-only control-plane tools

Author: TheGreatAxios

src/agent/tool-classification.ts

@@ -56,3 +56,19 @@ export const AUTO_ALLOW_READ_TOOLS: ReadonlySet<string> = new Set([
   ),
   "manage_tasks",
 ]);
+
+/**
+ * Read-only control-plane lookups. They read this agent's own catalogs or
+ * worker records and cannot change the workspace or grant authority, so
+ * classifyTool allows them in every mode, including non-auto interactive
+ * sessions that are not yolo. Mutating and authority-granting control-plane
+ * verbs are absent on purpose. gate.ts lists every control-plane verb and
+ * its non-auto, non-yolo effect next to that allow.
+ * SECURITY-RELEVANT: a name added here never prompts.
+ */
+export const READ_ONLY_CONTROL_PLANE_TOOLS: ReadonlySet<string> = new Set([
+  "tool_search",
+  "search_agents",
+  "list_agents",
+  "read_agent_trace",
+]);

src/permission/classify.ts

@@ -30,18 +30,25 @@ import {
   isProductMutationTool,
   productMutationPaths,
 } from "../agent/product-mutation-tools.js";
-import { AUTO_ALLOW_READ_TOOLS as READ_ONLY_TOOLS } from "../agent/tool-classification.js";
+import {
+  AUTO_ALLOW_READ_TOOLS as READ_ONLY_TOOLS,
+  READ_ONLY_CONTROL_PLANE_TOOLS,
+} from "../agent/tool-classification.js";
 import { canonicalToolName } from "../agent/canonical-tool-name.js";
 
 // Read-only tools never need approval while they stay off restricted paths —
 // they cannot change the workspace. `lsp` is included despite activating
 // mid-session: hover/definition lookups are as inert as a grep. `manage_tasks`
 // has no side effect (the director's decide() loop already applied the task
-// list), so an approval denial would prevent nothing. Every other posix tool
-// defaults to "ask"; catastrophic commands are denied earlier by the
-// authorization plugin.
+// list), so an approval denial would prevent nothing. Read-only control-plane
+// lookups (READ_ONLY_CONTROL_PLANE_TOOLS) are the same tier: they read this
+// agent's catalogs or worker records and do not grant authority. gate.ts lists
+// every control-plane verb and its non-auto, non-yolo effect beside that
+// allow. Every other tool defaults to "ask"; catastrophic commands are denied
+// earlier by the authorization plugin.
 //
-// Membership lives in tool-classification.ts (AUTO_ALLOW_READ_TOOLS).
+// Read membership: tool-classification.ts (AUTO_ALLOW_READ_TOOLS,
+// READ_ONLY_CONTROL_PLANE_TOOLS).
 
 // Tools with a single path-like argument the gate checks against restriction
 // (outside the workspace, or writes under the session state root): read-only
@@ -81,7 +88,9 @@ export function classifyTool(
   mcpTiers?: McpToolPermissionRegistry,
 ): Tier {
   const name = canonicalToolName(toolName);
-  if (READ_ONLY_TOOLS.has(name)) return "allow";
+  if (READ_ONLY_TOOLS.has(name) || READ_ONLY_CONTROL_PLANE_TOOLS.has(name)) {
+    return "allow";
+  }
   if (isMcpToolName(name)) {
     const registered = mcpTiers?.tierFor(name);
     if (registered !== undefined) return registered;

src/permission/gate.ts

@@ -325,26 +325,24 @@ function isRequestCoveredByApprovals(
   });
 }
 
-// Non-shell tools auto-mode auto-allows without prompting; reads, run_shell,
-// and read-only MCP auto-allow via their own paths, everything else prompts.
+// Non-shell tools in this set auto-allow when auto mode is on. Reads,
+// read-only control-plane lookups, run_shell, and read-only MCP auto-allow
+// on their own paths, including outside auto mode. approve_worker_call is
+// intentionally absent so it still prompts in auto mode.
 const AUTO_ALLOWED_TOOLS = new Set([
   ...PRODUCT_MUTATION_TOOLS,
   "manage_tasks",
   "present",
-  "tool_search",
   "use_skill",
   "skill_search",
   "web_fetch",
   "web_search",
-  "search_agents",
   "spawn_agent",
   "wait_agents",
-  "list_agents",
   "send_input",
   "interrupt_agent",
   "close_agent",
   "resume_agent",
-  "read_agent_trace",
 ]);
 
 export interface PermissionGateOptions {
@@ -757,6 +755,46 @@ export function createPermissionGate(
       shellSecretInspectionRequiresApproval(
         inspectShellSecretReference(shellCmd, effectiveCwd, isExtraDenied),
       );
+    // classifyTool === "allow" never prompts while the call stays off a
+    // restricted path. That tier is workspace reads plus read-only
+    // control-plane tools. Effects below are for a non-auto, non-yolo
+    // interactive session (skip-permissions already returned allow above).
+    //
+    // Allow in every mode — catalog or worker-record lookup, or a plan
+    // update whose handler cannot undo anything:
+    //   tool_search — tool/skill catalog. Matches may join the advertised
+    //     set next turn; that is not a workspace or authority change.
+    //   skill_search — skill catalog (AUTO_ALLOW_READ_TOOLS).
+    //   use_skill — load a skill body already on the director read surface
+    //     (AUTO_ALLOW_READ_TOOLS).
+    //   search_agents — spawnable agent profile catalog. Tier-1 mount only;
+    //     the lookup itself grants nothing.
+    //   list_agents — worker id, status, and pending question. Also notes
+    //     that parked asks were surfaced so a repeat poll is refused; it
+    //     does not grant or mutate a worker.
+    //   read_agent_trace — read a worker's on-disk trace.
+    //   manage_tasks — plan checklist. The handler has no side effect
+    //     because the director already applied the list
+    //     (AUTO_ALLOW_READ_TOOLS).
+    //
+    // Ask here; auto mode allows them via AUTO_ALLOWED_TOOLS because they
+    // start, message, collect, or stop a worker:
+    //   spawn_agent — start a worker.
+    //   send_input — deliver text to a worker.
+    //   wait_agents — block and take that worker's mailbox record.
+    //   interrupt_agent — stop a worker's current turn.
+    //   close_agent — permanently close a worker.
+    //   resume_agent — send a completed or interrupted worker its next turn.
+    //
+    // Ask in auto and non-auto. Not in AUTO_ALLOWED_TOOLS; grants authority:
+    //   approve_worker_call — authorize one pending worker-denied call.
+    //
+    // Worker sessions do not use this prompt. submit_result, ask_director,
+    // and nested fleet verbs other than spawn_agent and search_agents allow
+    // on the reactor worker path without a parent grant. On this primary
+    // gate they still ask:
+    //   submit_result — worker posts its structured result.
+    //   ask_director — worker parks a question for its parent.
     if (!restricted && classifyTool(call.name, mcpTiers) === "allow") {
       return { kind: "allow" };
     }

src/permission/permission.test.ts

@@ -1166,12 +1166,10 @@ describe("createPermissionGate", () => {
       auto: false,
     });
     const tools = [
-      "list_agents",
       "send_input",
       "interrupt_agent",
       "close_agent",
       "resume_agent",
-      "read_agent_trace",
     ];
     for (const [index, name] of tools.entries()) {
       approval = index % 2 === 0;
@@ -1222,6 +1220,64 @@ describe("createPermissionGate", () => {
     expect(asked.count).toBe(0);
   });
 
+  // Pins the control-plane inventory commented beside the allow in gate.ts.
+  // A declined prompt is the stand-in for "ask": allow:false makes a prompt
+  // observable without depending on prompt copy.
+  test("control-plane tools keep their effect in auto and non-auto interactive mode", async () => {
+    const allowEveryMode = [
+      "tool_search",
+      "skill_search",
+      "use_skill",
+      "search_agents",
+      "list_agents",
+      "read_agent_trace",
+      "manage_tasks",
+    ];
+    const allowOnlyInAuto = [
+      "spawn_agent",
+      "send_input",
+      "wait_agents",
+      "interrupt_agent",
+      "close_agent",
+      "resume_agent",
+    ];
+    const askInBothModes = [
+      "approve_worker_call",
+      "submit_result",
+      "ask_director",
+    ];
+    for (const auto of [false, true]) {
+      const { gate, asked } = gatedPrompts({ allow: false }, { auto });
+      for (const name of allowEveryMode) {
+        const verdict = await gate.evaluate({
+          id: `${name}-${String(auto)}`,
+          name,
+          arguments: {},
+        });
+        expect(verdict.allowed).toBe(true);
+      }
+      for (const name of allowOnlyInAuto) {
+        const verdict = await gate.evaluate({
+          id: `${name}-${String(auto)}`,
+          name,
+          arguments: {},
+        });
+        expect(verdict.allowed).toBe(auto);
+      }
+      for (const name of askInBothModes) {
+        const verdict = await gate.evaluate({
+          id: `${name}-${String(auto)}`,
+          name,
+          arguments: {},
+        });
+        expect(verdict.allowed).toBe(false);
+      }
+      const prompted =
+        (auto ? 0 : allowOnlyInAuto.length) + askInBothModes.length;
+      expect(asked.count).toBe(prompted);
+    }
+  });
+
   test("setAuto toggles auto mode live", async () => {
     const { gate, asked } = gatedPrompts({ allow: false }, { auto: false });
     expect(gate.getAuto()).toBe(false);