MythEclipse/shiro-neko
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Gate tool calls per command and path, not per tool name

Browse Source 
Approval was a list of tool names: `bash` needed it, `read_file` did not. That
fails in a specific way. `bash` covers `git status` and `rm -rf` equally, so a
user working through a batch presses `a` — always allow — on the first prompt and
every later command runs unasked, including the one they would have refused. The
gate was strongest when it mattered least and gone by the time it mattered.

Rules now match the *subject* of a call: the command for `bash`, the path for a
file tool, the pattern for a search.

    "permission": {
      "bash": { "*": "ask", "git *": "allow", "rm *": "deny" },
      "edit_file": { "*": "deny", "src/generated/*": "allow" }
    }

Plain last-match-wins, with no special case for deny. An earlier version made
deny win wherever it sat, on the theory that a refusal should be impossible to
undo by accident, and it made default-deny-with-exceptions unexpressible — which
is the shape a careful user actually writes, and the same shape as `*.env` denied
while `*.env.example` is allowed. Refusals that must never be configurable stay
in the guard plugin, which runs ahead of this and which --yolo cannot reach.

Three behaviours fall out of it:

- `always` grants the pattern the tool suggests, not the tool. Approving
  `git status` runs `git log` unprompted and still asks about `npm publish`.
- `.env`, `.env.*`, and `.pem` are denied on read by default. Not gated, refused:
  a secret that reaches the context is on the wire and in the session file, and
  there is no taking it back. `.env.example` stays allowed.
- A call repeated identically three times in one turn asks even when allowed. A
  model repeating itself is not making progress, and `bash: allow` is a statement
  about which commands are safe rather than permission to loop.

The prompt now says which rule matched and what `always` would grant:

    bash wants to run
    git status --porcelain
    y allow once | a always allow bash git * | n deny

A typo in a decision string is dropped at parse time, leaving the tool on its
default. Treating an unparseable value as `allow` would mean one misspelling
silently removing the gate.

Written after surveying Claude Code, Codex, opencode, and phi. Three of the four
had already moved to per-pattern rules; the credential deny and the repeat guard
come from opencode directly. ROADMAP records what was deliberately not taken and
why — OS sandboxing needs three platform implementations and is worse than
nothing if half-built, and opencode's own docs say LSP integration is often not a
net positive.

581 tests, up from 572. The engine is a pure function tested on its own, and the
loop is tested through a real Session: an allowed pattern never prompts, a denied
one never executes, and a `.env` read leaves no secret in the transcript.
This commit is contained in:
Muhammad Zakir Ramadhan
2026-09-03 11:19:53 +07:00
parent 9b978fdbe1
commit 7fd578e13b
13 changed files with 1283 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/session.ts
+92 -32
View File
@@ -12,25 +12,26 @@ import { createAskTool, type AskFn } from './ask';
import type { Instructions } from './instructions';
import type { Memory } from './memory';
import { Notebook, type NotebookState } from './notebook';
import { Permissions, type PermissionConfig } from './permission';
import type { PluginHost } from './plugins';
import { systemPrompt } from './prompt';
import { prunePreservingItems } from './prune';
import { createSkillTool, renderSkills, type Skill } from './skills';
import {
MUTATING_TOOLS,
disabledToolNames,
onBashOutput,
tools as builtinTools,
type ToolSetName,
} from './tools';
import { disabledToolNames, onBashOutput, tools as builtinTools, type ToolSetName } from './tools';
export type ApprovalRequest = {
approvalId: string;
toolName: string;
input: unknown;
/** The rule that decided this needs asking, when one did. */
matchedPattern?: string;
/** What `always` would whitelist, e.g. `git *` rather than every bash call. */
suggestedPattern: string;
/** Set when the call is being asked about because it repeated, not because of a rule. */
repeated?: boolean;
};
/** 'once' runs this call only; 'always' whitelists the tool for the rest of the session. */
/** 'once' runs this call only; 'always' whitelists the suggested pattern for the session. */
export type ApprovalDecision = 'once' | 'always' | 'deny';
export type AgentEvent =
@@ -57,6 +58,8 @@ export type SessionOptions = {
extraTools?: ToolSet;
/** Tool sets offered this session; omit for all of them. `core` is always on. */
toolSets?: readonly ToolSetName[];
/** Rules deciding which calls run, ask, or are refused. Omit for the defaults. */
permissions?: PermissionConfig;
/** Tool names that never prompt, e.g. the read-only subagent tool. */
autoApprove?: readonly string[];
/** Prune the history once the estimated token count crosses this. */
@@ -86,6 +89,13 @@ const estimateTokens = (messages: ModelMessage[]) => Math.round(JSON.stringify(m
/** Estimated tokens at which the wire history is pruned. */
const DEFAULT_COMPACT_THRESHOLD = 120_000;
/** Identical calls in one turn before an allowed tool is asked about anyway. */
const REPEAT_LIMIT = 3;
const callKey = (toolName: string, input: unknown) => `${toolName}:${JSON.stringify(input ?? null)}`;
type ApprovalContext = Pick<ApprovalRequest, 'matchedPattern' | 'suggestedPattern' | 'repeated'>;
export class Session {
readonly messages: ModelMessage[];
readonly tools: ToolSet;
@@ -94,7 +104,9 @@ export class Session {
outputTokens = 0;
private model: LanguageModel;
private variant: AgentVariant;
private readonly alwaysAllow = new Set<string>();
private readonly permissions: Permissions;
/** Calls seen this turn, for the repeat guard. Cleared per turn, not per step. */
private readonly seen = new Map<string, number>();
private controller: AbortController | undefined;
constructor(private readonly opts: SessionOptions) {
@@ -112,13 +124,16 @@ export class Session {
};
this.tools = { ...builtinTools, ...sessionTools, ...(opts.plugins?.tools ?? {}), ...(opts.extraTools ?? {}) };
for (const name of [
...(opts.autoApprove ?? []),
...(opts.plugins?.autoApprove ?? []),
...Object.keys(sessionTools),
]) {
this.alwaysAllow.add(name);
}
this.permissions = new Permissions({
...(opts.permissions ? { config: opts.permissions } : {}),
...(opts.yolo ? { yolo: true } : {}),
autoApprove: [
...(opts.autoApprove ?? []),
...(opts.plugins?.autoApprove ?? []),
// A session tool touches the agent's own state, not the workspace.
...Object.keys(sessionTools),
],
});
}
setModel(model: LanguageModel): void {
@@ -186,32 +201,67 @@ export class Session {
});
}
/** Tools that mutate the workspace, plus every externally provided MCP tool. */
private needsApproval(name: string): boolean {
return (MUTATING_TOOLS as readonly string[]).includes(name) || name.startsWith('mcp__');
/**
* How many times this exact call has already been made this turn.
*
* A model that repeats an identical call is not making progress: either it is
* ignoring the result or the result is not what it needed. Three is the point
* where that stops looking like a coincidence.
*/
private repeatCount(toolName: string, input: unknown): number {
const key = callKey(toolName, input);
const count = (this.seen.get(key) ?? 0) + 1;
this.seen.set(key, count);
return count;
}
/**
* Approval decisions, evaluated per call by the SDK.
*
* A plugin guard denies outright and is checked before anything else, so `--yolo`
* cannot bypass it. Only after the guard passes does yolo or the mutating-tool
* rule decide whether the user is asked.
* Order matters, and each step exists for a different reason:
*
* 1. A plugin guard refuses outright. `--yolo` cannot reach it, because a
* refusal is a policy decision rather than a permission question.
* 2. Permission rules decide allow / ask / deny, matched against the call's
* subject — the command, the path — not just the tool name.
* 3. An allowed call that has now repeated three times identically is asked
* about anyway. A rule saying `bash: allow` is a statement about which
* commands are safe, not permission to run one in a loop forever.
*
* `why` collects what the UI needs to explain the prompt, keyed by call, because
* the SDK's own approval request carries only the tool name and input.
*/
private toolApproval(notices: string[]) {
private toolApproval(notices: string[], why: Map<string, ApprovalContext>) {
return async ({ toolCall }: { toolCall: { toolName: string; input: unknown } }) => {
const { toolName, input } = toolCall;
const blocked = await this.opts.plugins?.guard({
toolName: toolCall.toolName,
input: toolCall.input,
toolName,
input,
cwd: this.opts.cwd ?? process.cwd(),
});
if (blocked) {
notices.push(blocked);
return { type: 'denied' as const, reason: blocked };
}
if (this.opts.yolo) return undefined;
if (!this.needsApproval(toolCall.toolName)) return undefined;
if (this.alwaysAllow.has(toolCall.toolName)) return undefined;
const { decision, pattern } = this.permissions.check(toolName, input);
if (decision === 'deny') {
const reason = pattern
? `Refused by the permission rule ${toolName}: "${pattern}" = deny.`
: `Refused by the permission rules: ${toolName} is denied.`;
notices.push(reason);
return { type: 'denied' as const, reason };
}
const repeats = this.repeatCount(toolName, input);
if (decision === 'allow' && repeats < REPEAT_LIMIT) return undefined;
why.set(callKey(toolName, input), {
...(pattern ? { matchedPattern: pattern } : {}),
suggestedPattern: this.permissions.suggest(toolName, input),
...(decision === 'allow' ? { repeated: true } : {}),
});
return 'user-approval' as const;
};
}
@@ -243,6 +293,9 @@ export class Session {
this.controller = new AbortController();
const signal = this.controller.signal;
const threshold = this.compactThreshold();
// Per turn, not per step: a tool called once in each of three steps is the
// loop this guards against.
this.seen.clear();
const outputs: Extract<AgentEvent, { type: 'tool-output' }>[] = [];
onBashOutput(({ toolCallId, chunk }) => {
@@ -269,6 +322,7 @@ export class Session {
const pending: ApprovalRequest[] = [];
const compactions: Extract<AgentEvent, { type: 'compacted' }>[] = [];
const guardNotices: string[] = [];
const why = new Map<string, ApprovalContext>();
let sawError = false;
const result = streamText({
@@ -278,7 +332,7 @@ export class Session {
tools: this.tools,
activeTools: this.activeTools(),
reasoning: sdkReasoning(this.variant.thinking),
toolApproval: this.toolApproval(guardNotices),
toolApproval: this.toolApproval(guardNotices, why),
stopWhen: isStepCount(this.variant.maxSteps ?? this.opts.maxSteps ?? 50),
maxRetries: this.opts.maxRetries ?? 3,
abortSignal: signal,
@@ -336,16 +390,20 @@ export class Session {
case 'tool-error':
yield { type: 'tool-error', id: part.toolCallId, name: part.toolName, error: part.error };
break;
case 'tool-approval-request':
case 'tool-approval-request': {
// A guard denial is answered by the SDK itself and arrives flagged
// automatic; queueing it would prompt the user for a settled call.
if (part.isAutomatic) break;
const context = why.get(callKey(part.toolCall.toolName, part.toolCall.input));
pending.push({
approvalId: part.approvalId,
toolName: part.toolCall.toolName,
input: part.toolCall.input,
suggestedPattern: '*',
...context,
});
break;
}
case 'tool-approval-response':
if (!part.approved) yield { type: 'tool-denied', name: part.toolCall.toolName };
break;
@@ -393,8 +451,10 @@ export class Session {
const responses: ToolApprovalResponse[] = [];
for (const req of pending) {
const decision = this.alwaysAllow.has(req.toolName) ? 'always' : await this.opts.askApproval(req);
if (decision === 'always') this.alwaysAllow.add(req.toolName);
const decision = await this.opts.askApproval(req);
// `always` records the pattern the tool suggested, so approving
// `git status` whitelists `git *` rather than every command.
if (decision === 'always') this.permissions.grant(req.toolName, req.suggestedPattern);
responses.push({
type: 'tool-approval-response',
approvalId: req.approvalId,