Skip to content

Commit 04fca6b

Browse files
committed
fix(permission): auto-allow read-only control-plane tools
1 parent 99f67eb commit 04fca6b

4 files changed

Lines changed: 132 additions & 14 deletions

File tree

‎src/agent/tool-classification.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,19 @@ export const AUTO_ALLOW_READ_TOOLS: ReadonlySet<string> = new Set([
5656
),
5757
"manage_tasks",
5858
]);
59+
60+
/**
61+
* Read-only control-plane lookups. They read this agent's own catalogs or
62+
* worker records and cannot change the workspace or grant authority, so
63+
* classifyTool allows them in every mode, including non-auto interactive
64+
* sessions that are not yolo. Mutating and authority-granting control-plane
65+
* verbs are absent on purpose. gate.ts lists every control-plane verb and
66+
* its non-auto, non-yolo effect next to that allow.
67+
* SECURITY-RELEVANT: a name added here never prompts.
68+
*/
69+
export const READ_ONLY_CONTROL_PLANE_TOOLS: ReadonlySet<string> = new Set([
70+
"tool_search",
71+
"search_agents",
72+
"list_agents",
73+
"read_agent_trace",
74+
]);

‎src/permission/classify.ts‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,25 @@ import {
3030
isProductMutationTool,
3131
productMutationPaths,
3232
} from "../agent/product-mutation-tools.js";
33-
import { AUTO_ALLOW_READ_TOOLS as READ_ONLY_TOOLS } from "../agent/tool-classification.js";
33+
import {
34+
AUTO_ALLOW_READ_TOOLS as READ_ONLY_TOOLS,
35+
READ_ONLY_CONTROL_PLANE_TOOLS,
36+
} from "../agent/tool-classification.js";
3437
import { canonicalToolName } from "../agent/canonical-tool-name.js";
3538

3639
// Read-only tools never need approval while they stay off restricted paths —
3740
// they cannot change the workspace. `lsp` is included despite activating
3841
// mid-session: hover/definition lookups are as inert as a grep. `manage_tasks`
3942
// has no side effect (the director's decide() loop already applied the task
40-
// list), so an approval denial would prevent nothing. Every other posix tool
41-
// defaults to "ask"; catastrophic commands are denied earlier by the
42-
// authorization plugin.
43+
// list), so an approval denial would prevent nothing. Read-only control-plane
44+
// lookups (READ_ONLY_CONTROL_PLANE_TOOLS) are the same tier: they read this
45+
// agent's catalogs or worker records and do not grant authority. gate.ts lists
46+
// every control-plane verb and its non-auto, non-yolo effect beside that
47+
// allow. Every other tool defaults to "ask"; catastrophic commands are denied
48+
// earlier by the authorization plugin.
4349
//
44-
// Membership lives in tool-classification.ts (AUTO_ALLOW_READ_TOOLS).
50+
// Read membership: tool-classification.ts (AUTO_ALLOW_READ_TOOLS,
51+
// READ_ONLY_CONTROL_PLANE_TOOLS).
4552

4653
// Tools with a single path-like argument the gate checks against restriction
4754
// (outside the workspace, or writes under the session state root): read-only
@@ -81,7 +88,9 @@ export function classifyTool(
8188
mcpTiers?: McpToolPermissionRegistry,
8289
): Tier {
8390
const name = canonicalToolName(toolName);
84-
if (READ_ONLY_TOOLS.has(name)) return "allow";
91+
if (READ_ONLY_TOOLS.has(name) || READ_ONLY_CONTROL_PLANE_TOOLS.has(name)) {
92+
return "allow";
93+
}
8594
if (isMcpToolName(name)) {
8695
const registered = mcpTiers?.tierFor(name);
8796
if (registered !== undefined) return registered;

‎src/permission/gate.ts‎

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -325,26 +325,24 @@ function isRequestCoveredByApprovals(
325325
});
326326
}
327327

328-
// Non-shell tools auto-mode auto-allows without prompting; reads, run_shell,
329-
// and read-only MCP auto-allow via their own paths, everything else prompts.
328+
// Non-shell tools in this set auto-allow when auto mode is on. Reads,
329+
// read-only control-plane lookups, run_shell, and read-only MCP auto-allow
330+
// on their own paths, including outside auto mode. approve_worker_call is
331+
// intentionally absent so it still prompts in auto mode.
330332
const AUTO_ALLOWED_TOOLS = new Set([
331333
...PRODUCT_MUTATION_TOOLS,
332334
"manage_tasks",
333335
"present",
334-
"tool_search",
335336
"use_skill",
336337
"skill_search",
337338
"web_fetch",
338339
"web_search",
339-
"search_agents",
340340
"spawn_agent",
341341
"wait_agents",
342-
"list_agents",
343342
"send_input",
344343
"interrupt_agent",
345344
"close_agent",
346345
"resume_agent",
347-
"read_agent_trace",
348346
]);
349347

350348
export interface PermissionGateOptions {
@@ -757,6 +755,46 @@ export function createPermissionGate(
757755
shellSecretInspectionRequiresApproval(
758756
inspectShellSecretReference(shellCmd, effectiveCwd, isExtraDenied),
759757
);
758+
// classifyTool === "allow" never prompts while the call stays off a
759+
// restricted path. That tier is workspace reads plus read-only
760+
// control-plane tools. Effects below are for a non-auto, non-yolo
761+
// interactive session (skip-permissions already returned allow above).
762+
//
763+
// Allow in every mode — catalog or worker-record lookup, or a plan
764+
// update whose handler cannot undo anything:
765+
// tool_search — tool/skill catalog. Matches may join the advertised
766+
// set next turn; that is not a workspace or authority change.
767+
// skill_search — skill catalog (AUTO_ALLOW_READ_TOOLS).
768+
// use_skill — load a skill body already on the director read surface
769+
// (AUTO_ALLOW_READ_TOOLS).
770+
// search_agents — spawnable agent profile catalog. Tier-1 mount only;
771+
// the lookup itself grants nothing.
772+
// list_agents — worker id, status, and pending question. Also notes
773+
// that parked asks were surfaced so a repeat poll is refused; it
774+
// does not grant or mutate a worker.
775+
// read_agent_trace — read a worker's on-disk trace.
776+
// manage_tasks — plan checklist. The handler has no side effect
777+
// because the director already applied the list
778+
// (AUTO_ALLOW_READ_TOOLS).
779+
//
780+
// Ask here; auto mode allows them via AUTO_ALLOWED_TOOLS because they
781+
// start, message, collect, or stop a worker:
782+
// spawn_agent — start a worker.
783+
// send_input — deliver text to a worker.
784+
// wait_agents — block and take that worker's mailbox record.
785+
// interrupt_agent — stop a worker's current turn.
786+
// close_agent — permanently close a worker.
787+
// resume_agent — send a completed or interrupted worker its next turn.
788+
//
789+
// Ask in auto and non-auto. Not in AUTO_ALLOWED_TOOLS; grants authority:
790+
// approve_worker_call — authorize one pending worker-denied call.
791+
//
792+
// Worker sessions do not use this prompt. submit_result, ask_director,
793+
// and nested fleet verbs other than spawn_agent and search_agents allow
794+
// on the reactor worker path without a parent grant. On this primary
795+
// gate they still ask:
796+
// submit_result — worker posts its structured result.
797+
// ask_director — worker parks a question for its parent.
760798
if (!restricted && classifyTool(call.name, mcpTiers) === "allow") {
761799
return { kind: "allow" };
762800
}

‎src/permission/permission.test.ts‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,12 +1166,10 @@ describe("createPermissionGate", () => {
11661166
auto: false,
11671167
});
11681168
const tools = [
1169-
"list_agents",
11701169
"send_input",
11711170
"interrupt_agent",
11721171
"close_agent",
11731172
"resume_agent",
1174-
"read_agent_trace",
11751173
];
11761174
for (const [index, name] of tools.entries()) {
11771175
approval = index % 2 === 0;
@@ -1222,6 +1220,63 @@ describe("createPermissionGate", () => {
12221220
expect(asked.count).toBe(0);
12231221
});
12241222

1223+
// Pins the control-plane inventory commented beside the allow in gate.ts.
1224+
// A declined prompt is the stand-in for "ask": allow:false makes a prompt
1225+
// observable without depending on prompt copy.
1226+
test("control-plane tools keep their effect in auto and non-auto interactive mode", async () => {
1227+
const allowEveryMode = [
1228+
"tool_search",
1229+
"skill_search",
1230+
"use_skill",
1231+
"search_agents",
1232+
"list_agents",
1233+
"read_agent_trace",
1234+
"manage_tasks",
1235+
];
1236+
const allowOnlyInAuto = [
1237+
"spawn_agent",
1238+
"send_input",
1239+
"wait_agents",
1240+
"interrupt_agent",
1241+
"close_agent",
1242+
"resume_agent",
1243+
];
1244+
const askInBothModes = [
1245+
"approve_worker_call",
1246+
"submit_result",
1247+
"ask_director",
1248+
];
1249+
for (const auto of [false, true]) {
1250+
const { gate, asked } = gatedPrompts({ allow: false }, { auto });
1251+
for (const name of allowEveryMode) {
1252+
const verdict = await gate.evaluate({
1253+
id: `${name}-${String(auto)}`,
1254+
name,
1255+
arguments: {},
1256+
});
1257+
expect(verdict.allowed).toBe(true);
1258+
}
1259+
for (const name of allowOnlyInAuto) {
1260+
const verdict = await gate.evaluate({
1261+
id: `${name}-${String(auto)}`,
1262+
name,
1263+
arguments: {},
1264+
});
1265+
expect(verdict.allowed).toBe(auto);
1266+
}
1267+
for (const name of askInBothModes) {
1268+
const verdict = await gate.evaluate({
1269+
id: `${name}-${String(auto)}`,
1270+
name,
1271+
arguments: {},
1272+
});
1273+
expect(verdict.allowed).toBe(false);
1274+
}
1275+
const prompted = (auto ? 0 : allowOnlyInAuto.length) + askInBothModes.length;
1276+
expect(asked.count).toBe(prompted);
1277+
}
1278+
});
1279+
12251280
test("setAuto toggles auto mode live", async () => {
12261281
const { gate, asked } = gatedPrompts({ allow: false }, { auto: false });
12271282
expect(gate.getAuto()).toBe(false);

0 commit comments

Comments
 (0)