diff --git a/.changeset/modern-permission-policies.md b/.changeset/modern-permission-policies.md new file mode 100644 index 0000000..9d486e8 --- /dev/null +++ b/.changeset/modern-permission-policies.md @@ -0,0 +1,6 @@ +--- +"@moonshot-ai/agent-core": minor +"@moonshot-ai/kimi-code": minor +--- + +Rework tool permissions: reads outside cwd no longer prompt, session approvals match the exact call, and path-based rules are case-insensitive. diff --git a/docs/en/configuration/config-files.md b/docs/en/configuration/config-files.md index 430d142..56a2ddb 100644 --- a/docs/en/configuration/config-files.md +++ b/docs/en/configuration/config-files.md @@ -203,7 +203,7 @@ Rules are written as a `[[permission.rules]]` array of tables, where each rule c | --- | --- | --- | --- | | `decision` | `string` | Yes | Decision result; one of `allow`, `deny`, `ask` | | `scope` | `string` | No | Rule scope; one of `turn-override`, `session-runtime`, `project`, `user`; defaults to `user` | -| `pattern` | `string` | Yes | Match pattern in the form `ToolName` or `ToolName(arg-pattern)`. `ToolName` must match the runtime tool name exactly — built-in tools are `Read`, `Write`, `Edit`, `Bash`, `Grep`, and so on (see [Built-in tools](../reference/tools.md)) | +| `pattern` | `string` | Yes | Match pattern in the form `ToolName` or `ToolName(arg-pattern)`. `ToolName` must match the runtime tool name exactly — built-in tools are `Read`, `Write`, `Edit`, `Bash`, `Grep`, and so on (see [Built-in tools](../reference/tools.md)). Argument patterns are interpreted only by tools with built-in argument matchers, such as `Bash`, file tools, and search tools; MCP tools and custom tools match by tool name only | | `reason` | `string` | No | Rule description for debugging or auditing | Example: diff --git a/docs/en/customization/mcp.md b/docs/en/customization/mcp.md index bbde8cc..28aec71 100644 --- a/docs/en/customization/mcp.md +++ b/docs/en/customization/mcp.md @@ -54,7 +54,7 @@ Stdio entries in a project-level `.kimi-code/mcp.json` execute local commands wh ## Tool naming and permissions -MCP tools are exposed using the naming convention `mcp____`. Permission matching supports `*` and `**` wildcards, so `mcp__github__*` covers every tool from the `github` server. +MCP tools are exposed using the naming convention `mcp____`. Permission matching supports `*` and `**` wildcards in tool names, so `mcp__github__*` covers every tool from the `github` server. MCP tool arguments are not part of permission matching; allow or deny the exact MCP tool name, or use a tool-name wildcard. Calls that do not match any rule trigger an approval request. Choosing "Approve for this session" in the approval prompt auto-approves subsequent matching calls. diff --git a/docs/zh/configuration/config-files.md b/docs/zh/configuration/config-files.md index 54e74c3..45b5a50 100644 --- a/docs/zh/configuration/config-files.md +++ b/docs/zh/configuration/config-files.md @@ -201,7 +201,7 @@ api_key = "sk-xxx" | --- | --- | --- | --- | | `decision` | `string` | 是 | 决策结果,可选 `allow`、`deny`、`ask` | | `scope` | `string` | 否 | 规则作用域,可选 `turn-override`、`session-runtime`、`project`、`user`;默认 `user` | -| `pattern` | `string` | 是 | 匹配模式,格式为 `ToolName` 或 `ToolName(arg-pattern)`。`ToolName` 必须与运行时真实工具名一致——内置工具是 `Read`、`Write`、`Edit`、`Bash`、`Grep` 等(详见 [内置工具](../reference/tools.md)) | +| `pattern` | `string` | 是 | 匹配模式,格式为 `ToolName` 或 `ToolName(arg-pattern)`。`ToolName` 必须与运行时真实工具名一致——内置工具是 `Read`、`Write`、`Edit`、`Bash`、`Grep` 等(详见 [内置工具](../reference/tools.md))。参数模式只由带内置参数 matcher 的工具解释,例如 `Bash`、文件工具和搜索工具;MCP 工具和自定义工具只按工具名匹配 | | `reason` | `string` | 否 | 规则说明,供调试或审计使用 | 示例: diff --git a/docs/zh/customization/mcp.md b/docs/zh/customization/mcp.md index 997032f..8f71302 100644 --- a/docs/zh/customization/mcp.md +++ b/docs/zh/customization/mcp.md @@ -54,7 +54,7 @@ MCP server 配置写在 `mcp.json` 中,分为两层: ## 工具命名与权限 -MCP 工具按 `mcp____` 命名。权限匹配支持 `*` 和 `**` 通配,例如 `mcp__github__*` 命中该 server 下所有工具。 +MCP 工具按 `mcp____` 命名。权限匹配支持工具名中的 `*` 和 `**` 通配,例如 `mcp__github__*` 命中该 server 下所有工具。MCP 工具参数不会参与权限匹配;请放行或拒绝精确的 MCP 工具名,或使用工具名通配。 未命中权限规则的调用会触发审批请求;在审批弹窗中选择 "Approve for this session" 后,后续同类调用将自动放行。 diff --git a/packages/agent-core/src/agent/permission/action-label.ts b/packages/agent-core/src/agent/permission/action-label.ts deleted file mode 100644 index ad1565d..0000000 --- a/packages/agent-core/src/agent/permission/action-label.ts +++ /dev/null @@ -1,142 +0,0 @@ -/** - * describeApprovalAction — coarse action label for approve_for_session. - * - * The label is the key used by `auto_approve_actions` set (session-level - * approve-for-session cache). It must be **coarse** enough that the user - * pressing "approve for session" on one request also unlocks *semantically - * equivalent* future requests — otherwise approve-for-session degrades - * into approve-once. - * - * Derivation priority: - * 1. `ApprovalDisplay.kind` mapping (the display already carries the - * semantic classification the UI renders): - * command → "run command" - * diff → "edit file" - * file_write → "write file" - * task_stop → "stop background task" - * generic → tool-name fallback - * 2. Hard-coded toolName → action map for tools that emit `generic` - * display. - * 3. Last resort: `call `. - */ - -import type { ToolInputDisplay } from '../../tools/display/schemas'; - -/** - * Hard-coded toolName → action label map. Consulted as a fallback so - * tools that carry `generic` displays still get a sensible label. Keys - * match the tool names currently wired in - * `packages/agent-core/src/tools/`. - */ -const TOOL_NAME_TO_ACTION: Readonly> = { - Bash: 'run command', - Shell: 'run command', - BackgroundRun: 'run background command', - BackgroundStop: 'stop background task', - Write: 'edit file', - Edit: 'edit file', - StrReplace: 'edit file', -}; - -/** Inverse table — action label → the representative tool-name pattern. */ -const ACTION_TO_PATTERN: Readonly> = { - 'run command': 'Bash', - 'run command in plan mode': null, - 'run background command': 'BackgroundRun', - 'stop background task': 'BackgroundStop', - 'edit file': 'Write', - 'edit file outside of working directory': 'Write', - 'write file': 'Write', -}; - -export function describeApprovalAction( - toolName: string, - _args: unknown, - display: ToolInputDisplay, - override?: string, -): string { - // Highest priority: explicit override from a hook. - if (override !== undefined && override.length > 0) { - return override; - } - - // Display-driven derivation: the display kind already captures the - // coarse semantic class the UI renders. - switch (display.kind) { - case 'command': - return 'run command'; - case 'diff': - return 'edit file'; - case 'file_io': - switch (display.operation) { - case 'write': - return 'write file'; - case 'edit': - return 'edit file'; - case 'read': - return 'read file'; - case 'glob': - return 'list files'; - case 'grep': - return 'search files'; - } - break; - case 'task_stop': - return 'stop background task'; - case 'plan_review': - return 'review plan'; - case 'agent_call': - return 'spawn agent'; - case 'skill_call': - return 'invoke skill'; - case 'url_fetch': - return 'fetch URL'; - case 'search': - return 'search'; - case 'todo_list': - return 'update todo list'; - case 'background_task': - return 'run background task'; - case 'generic': - // fall through to tool-name map - break; - } - - const mapped = TOOL_NAME_TO_ACTION[toolName]; - if (mapped !== undefined) return mapped; - - // MCP tool naming: `mcp____` gets a coarse - // `"call MCP tool: :"` label so one approve-for-session - // click unlocks every future call to the same server+tool combo. The - // server name is preserved to prevent cross-server privilege escalation. - if (toolName.startsWith('mcp__')) { - const rest = toolName.slice('mcp__'.length); - const sep = rest.indexOf('__'); - if (sep >= 0) { - const serverName = rest.slice(0, sep); - const innerTool = rest.slice(sep + 2); - if (innerTool.length > 0) { - return `call MCP tool: ${serverName}:${innerTool}`; - } - } - } - - return `call ${toolName}`; -} - -/** - * Inverse mapping from an approve_for_session action label to the - * permission-rule pattern that should gate future same-action calls. - * - * When no entry matches, fall back to the concrete tool name. A `null` - * table entry means the action should be cached by action label only, - * without creating a broad PermissionRule. - */ -export function actionToRulePattern( - action: string, - fallbackToolName: string, -): string | undefined { - const mapped = ACTION_TO_PATTERN[action]; - if (mapped !== undefined) return mapped ?? undefined; - return fallbackToolName; -} diff --git a/packages/agent-core/src/agent/permission/check-rules.ts b/packages/agent-core/src/agent/permission/check-rules.ts deleted file mode 100644 index 484cc11..0000000 --- a/packages/agent-core/src/agent/permission/check-rules.ts +++ /dev/null @@ -1,59 +0,0 @@ -import { isDefaultAutoAllowTool } from '../../tools/policies/default-permissions'; -import { matchesRule } from './matches-rule'; -import type { PermissionPathMatchOptions } from './path-glob-match'; -import type { PermissionMode, PermissionRule, PermissionRuleDecision } from './types'; - -export interface CheckRulesResult { - readonly decision: PermissionRuleDecision; - /** Rule that produced `decision`. `undefined` for mode/default auto-allow. */ - readonly matchedRule?: PermissionRule | undefined; -} - -export function checkMatchingRules( - rules: readonly PermissionRule[], - toolName: string, - toolInput: unknown, - mode: PermissionMode, - pathOptions?: PermissionPathMatchOptions, -): CheckRulesResult | undefined { - // Priority 1: deny wins in every mode. - for (const rule of rules) { - if (rule.decision === 'deny' && matchesRule(rule, toolName, toolInput, pathOptions)) { - return { decision: 'deny', matchedRule: rule }; - } - } - - const askRule = firstMatchingRule(rules, 'ask', toolName, toolInput, pathOptions); - const allowRule = firstMatchingRule(rules, 'allow', toolName, toolInput, pathOptions); - if (askRule === undefined && allowRule === undefined) return undefined; - - if (isDefaultAutoAllowTool(toolName)) { - return { decision: 'allow' }; - } - - // Mode overlay: yolo treats everything non-deny as allow. - if (mode === 'yolo' || mode === 'auto') { - return { decision: 'allow' }; - } - - // Priority 2: ask before allow so unresolved ambiguity defers to the user. - if (askRule !== undefined) return { decision: 'ask', matchedRule: askRule }; - - // Priority 3: explicit allow. - return { decision: 'allow', matchedRule: allowRule }; -} - -function firstMatchingRule( - rules: readonly PermissionRule[], - decision: PermissionRuleDecision, - toolName: string, - toolInput: unknown, - pathOptions?: PermissionPathMatchOptions, -): PermissionRule | undefined { - for (const rule of rules) { - if (rule.decision === decision && matchesRule(rule, toolName, toolInput, pathOptions)) { - return rule; - } - } - return undefined; -} diff --git a/packages/agent-core/src/agent/permission/index.ts b/packages/agent-core/src/agent/permission/index.ts index 421eb49..9450aaf 100644 --- a/packages/agent-core/src/agent/permission/index.ts +++ b/packages/agent-core/src/agent/permission/index.ts @@ -1,35 +1,35 @@ import type { Agent } from '..'; -import type { PrepareToolExecutionResult, ToolExecutionHookContext } from '../../loop'; -import type { TelemetryPropertyValue } from '../../telemetry'; -import { isDefaultAutoAllowTool } from '../../tools/policies/default-permissions'; -import type { ToolInputDisplay } from '../../tools/display'; -import { actionToRulePattern, describeApprovalAction } from './action-label'; -import { checkMatchingRules, type CheckRulesResult } from './check-rules'; -import type { PermissionPathMatchOptions } from './path-glob-match'; -import { createBuiltinPermissionPolicies } from './policies'; -import type { PermissionPolicy, PermissionPolicyResult } from './policy'; +import type { PrepareToolExecutionResult } from '../../loop'; +import { createPermissionDecisionPolicies } from './policies'; import type { + ApprovalResponse, PermissionApprovalResultRecord, PermissionData, PermissionMode, - PermissionRule, + PermissionPolicy, + PermissionPolicyContext, + PermissionPolicyResolution, + PermissionPolicyResult, + PermissionRule } from './types'; -export * from './policy'; -export * from './types'; -type ApprovalTelemetryMode = 'manual' | 'yolo' | 'afk' | 'auto_session' | 'cancelled'; +export * from './types'; export interface PermissionManagerOptions { - readonly initialRules?: readonly PermissionRule[] | undefined; - readonly policies?: readonly PermissionPolicy[] | undefined; - readonly parent?: PermissionManager | undefined; + readonly initialRules?: readonly PermissionRule[]; + readonly parent?: PermissionManager; +} + +interface PolicyEvaluation { + readonly policyName: string; + readonly result: PermissionPolicyResult; } export class PermissionManager { rules: PermissionRule[] = []; private modeOverride: PermissionMode | undefined; private readonly parent: PermissionManager | undefined; - private readonly sessionApprovedActions = new Set(); + private readonly localSessionApprovalRulePatterns = new Set(); private readonly policies: readonly PermissionPolicy[]; constructor( @@ -38,7 +38,7 @@ export class PermissionManager { ) { this.rules = [...(options.initialRules ?? [])]; this.parent = options.parent; - this.policies = options.policies ?? createBuiltinPermissionPolicies(); + this.policies = createPermissionDecisionPolicies(this.agent); } get mode(): PermissionMode { @@ -52,7 +52,7 @@ export class PermissionManager { data(): PermissionData { return { mode: this.mode, - rules: this.effectiveRules(), + rules: this.effectiveRules, }; } @@ -81,214 +81,164 @@ export class PermissionManager { if (record.result.decision !== 'approved' || record.result.scope !== 'session') { return; } - if (this.sessionApprovedActions.has(record.action)) return; - - const pattern = actionToRulePattern(record.action, record.toolName); - this.sessionApprovedActions.add(record.action); + const pattern = record.sessionApprovalRule; if (pattern === undefined) return; + this.localSessionApprovalRulePatterns.add(pattern); + } - const rule: PermissionRule = { - decision: 'allow', - scope: 'session-runtime', - pattern, - reason: `approve_for_session: ${record.action}`, - }; - if (!this.hasRule(rule)) { - this.rules.push(rule); - } + get sessionApprovalRulePatterns(): readonly string[] { + return [ + ...this.localSessionApprovalRulePatterns, + ...(this.parent?.sessionApprovalRulePatterns ?? []), + ]; } async beforeToolCall( - context: ToolExecutionHookContext, + context: PermissionPolicyContext, ): Promise { - const name = context.toolCall.name; - const args = context.args; - - const mode = this.mode; - const { decision, matchedRule } = this.checkPermission(name, args, mode); - if (decision === 'deny') { - return { - block: true, - reason: this.formatMessage(name, matchedRule?.reason), - }; - } - - const policyResult = await this.evaluatePolicies(context, matchedRule); - if (policyResult !== undefined) { - return this.permissionPolicyResultToPrepare(policyResult, context); - } - - if (mode === 'auto') { - if (this.wouldAskInManualMode(name, args)) { - this.trackToolApproved(name, 'afk'); - } - return undefined; - } - if (mode === 'yolo') { - if (this.wouldAskInManualMode(name, args)) { - this.trackToolApproved(name, 'yolo'); - } - return undefined; - } - - if (decision === 'allow') { - if (matchedRule?.scope === 'session-runtime') { - this.trackToolApproved(name, 'auto_session', 'session'); - } - return undefined; - } - - // decision === 'ask' → bounce through ApprovalRuntime. - return this.requestToolApproval(context); + const evaluation = await this.evaluatePolicies(context); + if (evaluation === undefined) return undefined; + + this.agent.telemetry.track('permission_policy_decision', { + policy_name: evaluation.policyName, + tool_name: context.toolCall.name, + permission_mode: this.mode, + decision: evaluation.result.kind, + ...evaluation.result.reason, + }); + return this.permissionPolicyResolutionToPrepare( + evaluation.result, + context, + evaluation.policyName, + ); } private async requestToolApproval( - context: ToolExecutionHookContext, - options: { - readonly action?: string | undefined; - readonly display?: ToolInputDisplay | undefined; - } = {}, + context: PermissionPolicyContext, + result: Extract, + policyName: string | undefined, ): Promise { const { signal } = context; const id = context.toolCall.id; const name = context.toolCall.name; - const args = context.args; const display = - options.display ?? ({ + context.execution.display ?? { kind: 'generic', - summary: `Approve ${name}`, - detail: args, - } satisfies ToolInputDisplay); - const action = options.action ?? describeApprovalAction(name, args, display); - if (this.sessionApprovedActions.has(action)) { - this.trackToolApproved(name, 'auto_session', 'session'); - return undefined; + summary: context.execution.description ?? `Approve ${name}`, + detail: context.args, + }; + const action = context.execution.description ?? `Call ${name}`; + const startedAt = Date.now(); + + let response: ApprovalResponse; + try { + response = await this.agent.rpc.requestApproval( + { + turnId: Number(context.turnId), + toolCallId: id, + toolName: name, + action, + display, + }, + { signal }, + ); + } catch (error) { + this.agent.telemetry.track('permission_approval_result', { + policy_name: policyName ?? null, + tool_name: name, + permission_mode: this.mode, + result: 'error', + approval_surface: display.kind, + duration_ms: Date.now() - startedAt, + session_cache_written: false, + has_feedback: false, + }); + const resolved = result.resolveError?.(error); + return resolved === undefined + ? Promise.reject(error) + : this.permissionPolicyResolutionToPrepare(resolved, context, policyName); } - const result = await this.agent.rpc.requestApproval( - { - turnId: Number(context.turnId), - toolCallId: id, - toolName: name, - action, - display, - }, - { signal }, - ); + const sessionApprovalRule = + response.decision === 'approved' && response.scope === 'session' + ? context.execution.approvalRule + : undefined; + this.recordApprovalResult({ turnId: Number(context.turnId), toolCallId: id, toolName: name, action, - result, + sessionApprovalRule, + result: response, + }); + this.agent.telemetry.track('permission_approval_result', { + policy_name: policyName ?? null, + tool_name: name, + permission_mode: this.mode, + result: + response.decision === 'approved' && response.scope === 'session' + ? 'approved_for_session' + : response.decision, + approval_surface: display.kind, + duration_ms: Date.now() - startedAt, + session_cache_written: sessionApprovalRule !== undefined, + has_feedback: response.feedback !== undefined && response.feedback.length > 0, }); - if (result.decision === 'approved') { - this.trackToolApproved( - name, - approvalTelemetryMode(this.mode), - result.scope === 'session' ? 'session' : 'once', - ); - return undefined; + const resolved = result.resolveApproval?.(response); + if (resolved !== undefined) { + return this.permissionPolicyResolutionToPrepare(resolved, context, policyName); } - this.agent.telemetry.track('tool_rejected', { - tool_name: name, - approval_mode: - result.decision === 'cancelled' ? 'cancelled' : approvalTelemetryMode(this.mode), - decision: result.decision, - has_feedback: result.feedback !== undefined && result.feedback.length > 0, - }); + if (response.decision === 'approved') { + return undefined; + } return { block: true, - reason: this.formatApprovalRejectionMessage(name, result), + reason: this.formatApprovalRejectionMessage(name, response), }; } private async evaluatePolicies( - context: ToolExecutionHookContext, - matchedRule: PermissionRule | undefined, - ): Promise { + context: PermissionPolicyContext, + ): Promise { for (const policy of this.policies) { - const result = await policy.evaluate({ - agent: this.agent, - mode: this.mode, - toolCallContext: context, - matchedRule, - recordApprovalResult: (record) => { - this.recordApprovalResult(record); - }, - }); - if (result !== undefined) return result; + const result = await policy.evaluate(context); + if (result !== undefined) { + return { policyName: policy.name, result }; + } } return undefined; } - private checkPermission( - toolName: string, - toolInput: unknown, - mode: PermissionMode = this.mode, - ): CheckRulesResult { - const matched = this.checkMatchingPermissionRules(toolName, toolInput, mode); - if (matched !== undefined) return matched; - if (isDefaultAutoAllowTool(toolName)) return { decision: 'allow' }; - if (mode === 'yolo' || mode === 'auto') return { decision: 'allow' }; - return { decision: 'ask' }; - } - - private checkMatchingPermissionRules( - toolName: string, - toolInput: unknown, - mode: PermissionMode, - ): CheckRulesResult | undefined { - return ( - checkMatchingRules(this.rules, toolName, toolInput, mode, this.pathMatchOptions()) ?? - this.parent?.checkMatchingPermissionRules(toolName, toolInput, mode) - ); - } - - private effectiveRules(): PermissionRule[] { - return [...this.rules, ...(this.parent?.effectiveRules() ?? [])]; - } - - private wouldAskInManualMode(toolName: string, toolInput: unknown): boolean { - return this.checkPermission(toolName, toolInput, 'manual').decision === 'ask'; + private get effectiveRules(): PermissionRule[] { + return [...this.rules, ...(this.parent?.effectiveRules ?? [])]; } - private permissionPolicyResultToPrepare( - result: PermissionPolicyResult, - context: ToolExecutionHookContext, + private permissionPolicyResolutionToPrepare( + result: PermissionPolicyResolution, + context: PermissionPolicyContext, + policyName?: string, ): Promise | PrepareToolExecutionResult | undefined { switch (result.kind) { - case 'allow': + case 'approve': return result.executionMetadata === undefined ? undefined : { executionMetadata: result.executionMetadata }; + case 'deny': + return { + block: true, + reason: result.message ?? this.formatPolicyDenyMessage(context.toolCall.name), + }; case 'ask': - return this.requestToolApproval(context, result); - case 'result': - return result.result; - } - } - - private hasRule(target: PermissionRule): boolean { - return this.rules.some((rule) => { - return ( - rule.decision === target.decision && - rule.scope === target.scope && - rule.pattern === target.pattern && - rule.reason === target.reason - ); - }); - } - - protected formatMessage(toolName: string, reason?: string): string { - const suffix = reason !== undefined && reason.length > 0 ? ` Reason: ${reason}` : ''; - if (this.agent.type === 'sub') { - return `Tool "${toolName}" was denied.${suffix} Try a different approach — don't retry the same call, don't attempt to bypass the restriction.`; + return this.requestToolApproval(context, result, policyName); + case 'result': { + const { kind: _kind, ...prepareResult } = result; + return prepareResult; + } } - return `Tool "${toolName}" was denied by permission rule.${suffix}`; } protected formatApprovalRejectionMessage( @@ -309,32 +259,11 @@ export class PermissionManager { return `${prefix}${suffix}`; } - private pathMatchOptions(): PermissionPathMatchOptions { - return { - cwd: this.agent.config.cwd, - pathClass: this.agent.runtime.kaos.pathClass(), - homeDir: this.agent.runtime.kaos.gethome(), - }; - } - - private trackToolApproved( - toolName: string, - approvalMode: Exclude, - scope?: 'once' | 'session', - ): void { - const properties: Record = { - tool_name: toolName, - approval_mode: approvalMode, - }; - if (scope !== undefined) { - properties['scope'] = scope; + private formatPolicyDenyMessage(toolName: string): string { + const prefix = `Tool "${toolName}" was denied by permission policy.`; + if (this.agent.type === 'sub') { + return `${prefix} Try a different approach — don't retry the same call, don't attempt to bypass the restriction.`; } - this.agent.telemetry.track('tool_approved', properties); + return prefix; } } - -function approvalTelemetryMode( - mode: PermissionMode, -): Extract { - return mode === 'auto' ? 'afk' : mode; -} diff --git a/packages/agent-core/src/agent/permission/matches-rule.ts b/packages/agent-core/src/agent/permission/matches-rule.ts index 546567b..1c62900 100644 --- a/packages/agent-core/src/agent/permission/matches-rule.ts +++ b/packages/agent-core/src/agent/permission/matches-rule.ts @@ -1,125 +1,98 @@ +import picomatch from 'picomatch'; + +import type { RunnableToolExecution } from '../../loop/types'; +import type { PermissionRule } from './types'; + /** - * matchesRule — pure function that decides whether a PermissionRule - * applies to a given tool call. + * DSL parser for PermissionRule `pattern` strings. + * + * Grammar: + * pattern := toolName ( "(" argPattern ")" )? + * toolName := identifier characters (e.g. `Bash`, `mcp__github__*`) + * argPattern := any string interpreted only by a tool-provided matcher * - * Contract: - * - No side effects, no `this`, no IO, no exceptions. - * - Deterministic: same `(rule, toolName, args)` → same result. - * - Returns boolean only; decision semantics (deny/ask/allow) are a - * caller concern (see `check-rules.ts`). + * Examples: + * "Write" -> { toolName: "Write" } + * "Read(/etc/**)" -> { toolName: "Read", argPattern: "/etc/**" } + * "Bash(!rm *)" -> { toolName: "Bash", argPattern: "!rm *" } + * "mcp__github__*" -> { toolName: "mcp__github__*" } */ +export interface ParsedPattern { + readonly toolName: string; + readonly argPattern?: string; +} -import picomatch from 'picomatch'; +export interface PermissionRuleMatchExecution { + readonly matchesRule?: RunnableToolExecution['matchesRule']; +} -import { parsePattern } from './parse-pattern'; -import { globMatch, pathGlobMatch, type PermissionPathMatchOptions } from './path-glob-match'; -import type { PermissionRule } from './types'; +export type PermissionRuleMatchStrategy = 'tool_name_only' | 'matches_rule'; -type ArgFieldKind = 'generic' | 'path'; +export interface PermissionRuleMatch { + readonly rule: PermissionRule; + readonly strategy: PermissionRuleMatchStrategy; + readonly hasRuleArgs: boolean; +} -interface ArgField { - readonly value: string; - readonly kind: ArgFieldKind; +export interface PermissionRuleMatchInput { + readonly rule: PermissionRule; + readonly toolName: string; + readonly execution: PermissionRuleMatchExecution; } /** - * Tool-specific argument field convention. When a rule uses an arg - * pattern (`Read(./src/**)`), we extract the listed field from the tool - * call args and match the glob against its value. - * - * Unknown tools fall back to `undefined`, which means "arg pattern - * cannot match" — rules with an arg pattern on an unknown tool will - * never fire. Rules without an arg pattern (`UnknownTool`) still match - * on name alone. + * Parse a DSL pattern. Throws on malformed input (missing closing paren, + * empty tool name). The parser is the single source of truth for DSL syntax. */ -function extractArgField(toolName: string, args: unknown): ArgField | undefined { - if (args === null || typeof args !== 'object') return undefined; - const rec = args as Record; +export function parsePattern(pattern: string): ParsedPattern { + const trimmed = pattern.trim(); + if (trimmed.length === 0) { + throw new Error('permission pattern: empty string'); + } - switch (toolName) { - case 'Bash': - case 'Shell': - case 'Background': - return typeof rec['command'] === 'string' - ? { value: rec['command'], kind: 'generic' } - : undefined; - case 'Read': - case 'Write': - case 'Edit': - case 'ReadMediaFile': - return typeof rec['path'] === 'string' ? { value: rec['path'], kind: 'path' } : undefined; - case 'Grep': - case 'Glob': - return typeof rec['pattern'] === 'string' - ? { value: rec['pattern'], kind: 'generic' } - : undefined; - case 'Task': - case 'Agent': - return typeof rec['subagent_type'] === 'string' - ? { value: rec['subagent_type'], kind: 'generic' } - : undefined; - default: - return undefined; + const openIdx = trimmed.indexOf('('); + if (openIdx === -1) { + return { toolName: trimmed }; } + + if (!trimmed.endsWith(')')) { + throw new Error(`permission pattern: missing closing paren in "${pattern}"`); + } + + const toolName = trimmed.slice(0, openIdx); + const argPattern = trimmed.slice(openIdx + 1, -1); + if (toolName.length === 0) { + throw new Error(`permission pattern: empty tool name in "${pattern}"`); + } + // `Tool()` parses to no arg pattern so it stays tool-name-only — tools without + // a `matchesRule` matcher (user/MCP/custom) would otherwise stop matching it. + if (argPattern.length === 0) { + return { toolName }; + } + return { toolName, argPattern }; } -/** - * Decide whether a single rule matches a specific tool call. - * - * Algorithm: - * 1. Parse `rule.pattern` into `{toolName, argPattern?}`. - * 2. If parsed toolName is `*`, skip name check; otherwise compare with - * glob semantics so `mcp__github__*` matches `mcp__github__list`. - * 3. If the rule has no argPattern, name match → rule fires. - * 4. Otherwise extract the tool-specific field value and match against - * the glob. Handle the leading `!` negation prefix by flipping the - * final boolean. - */ -export function matchesRule( - rule: PermissionRule, - toolName: string, - args: unknown, - pathOptions?: PermissionPathMatchOptions, -): boolean { +export function matchPermissionRule({ + rule, + toolName, + execution, +}: PermissionRuleMatchInput): PermissionRuleMatch | undefined { let parsed; try { parsed = parsePattern(rule.pattern); } catch { - // Malformed patterns never match. The loader is responsible for - // surfacing DSL errors at load time; matcher stays total. - return false; + return undefined; } - // 1. Tool-name match (support `*` wildcard + glob-style tool names) - const nameGlob = parsed.toolName; - if (nameGlob !== '*' && !picomatch.isMatch(toolName, nameGlob)) return false; - - // 2. No arg pattern → name match is enough - if (parsed.argPattern === undefined) return true; - - // 3. Arg pattern — resolve negation and glob-match the field - const rawPattern = parsed.argPattern; - const negated = rawPattern.startsWith('!'); - const positivePattern = negated ? rawPattern.slice(1) : rawPattern; + if (parsed.toolName !== '*' && !picomatch.isMatch(toolName, parsed.toolName)) { + return undefined; + } - const fieldValue = extractArgField(toolName, args); - if (fieldValue === undefined) { - // No extractable field → positive pattern cannot match; negation - // semantics here mean "the field is not in the disallowed set", - // which by missing-field convention we treat as a non-match to - // avoid accidentally firing deny rules on malformed args. - return false; + if (parsed.argPattern === undefined) { + return { rule, strategy: 'tool_name_only', hasRuleArgs: false }; } - // If the field is a path, use `pathGlobMatch` which handles normalization. - const hit = - fieldValue.kind === 'path' - ? pathGlobMatch(fieldValue.value, positivePattern, { - pathOptions, - conservativeCaseFold: rule.decision === 'deny' && !negated, - }) - : globMatch(fieldValue.value, positivePattern); - return negated ? !hit : hit; + return execution.matchesRule?.(parsed.argPattern) === true + ? { rule, strategy: 'matches_rule', hasRuleArgs: true } + : undefined; } - -export type { PermissionPathMatchOptions } from './path-glob-match'; diff --git a/packages/agent-core/src/agent/permission/parse-pattern.ts b/packages/agent-core/src/agent/permission/parse-pattern.ts deleted file mode 100644 index 9b76ebd..0000000 --- a/packages/agent-core/src/agent/permission/parse-pattern.ts +++ /dev/null @@ -1,50 +0,0 @@ -/** - * DSL parser for PermissionRule `pattern` strings. - * - * Grammar: - * pattern := toolName ( "(" argPattern ")" )? - * toolName := identifier characters (e.g. `Bash`, `mcp__github__*`) - * argPattern := any string (may start with `!` for negation) - * - * Examples: - * "Write" → { toolName: "Write" } - * "Read(/etc/**)" → { toolName: "Read", argPattern: "/etc/**" } - * "Bash(!rm *)" → { toolName: "Bash", argPattern: "!rm *" } - * "mcp__github__*" → { toolName: "mcp__github__*" } - */ - -export interface ParsedPattern { - readonly toolName: string; - readonly argPattern?: string | undefined; -} - -/** - * Parse a DSL pattern. Throws on malformed input (missing closing paren, - * empty tool name). The parser is the single source of truth for DSL - * syntax and is exercised by table-driven tests. - */ -export function parsePattern(pattern: string): ParsedPattern { - const trimmed = pattern.trim(); - if (trimmed.length === 0) { - throw new Error('permission pattern: empty string'); - } - - const openIdx = trimmed.indexOf('('); - if (openIdx === -1) { - return { toolName: trimmed }; - } - - if (!trimmed.endsWith(')')) { - throw new Error(`permission pattern: missing closing paren in "${pattern}"`); - } - - const toolName = trimmed.slice(0, openIdx); - const argPattern = trimmed.slice(openIdx + 1, -1); - if (toolName.length === 0) { - throw new Error(`permission pattern: empty tool name in "${pattern}"`); - } - // Empty arg pattern (`Read()`) is treated as "toolName only" — it - // matches every call to that tool. This aligns with the intuition - // that writing `Read()` is an odd but non-fatal way of saying `Read`. - return { toolName, argPattern: argPattern.length > 0 ? argPattern : undefined }; -} diff --git a/packages/agent-core/src/agent/permission/policies/ask-user-question.ts b/packages/agent-core/src/agent/permission/policies/ask-user-question.ts deleted file mode 100644 index d1af924..0000000 --- a/packages/agent-core/src/agent/permission/policies/ask-user-question.ts +++ /dev/null @@ -1,17 +0,0 @@ -import type { PermissionPolicy } from '../policy'; - -export const AskUserQuestionAutoPermissionPolicy: PermissionPolicy = { - name: 'auto.ask-user-question', - evaluate({ mode, toolCallContext }) { - if (mode !== 'auto') return undefined; - if (toolCallContext.toolCall.name !== 'AskUserQuestion') return undefined; - return { - kind: 'result', - result: { - block: true, - reason: - 'AskUserQuestion is disabled while auto permission mode is active. Make a reasonable decision and continue without asking the user.', - }, - }; - }, -}; diff --git a/packages/agent-core/src/agent/permission/policies/auto-mode-approve.ts b/packages/agent-core/src/agent/permission/policies/auto-mode-approve.ts new file mode 100644 index 0000000..3d8ebde --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/auto-mode-approve.ts @@ -0,0 +1,15 @@ +import type { Agent } from '../..'; +import type { PermissionPolicy, PermissionPolicyResult } from '../types'; + +export class AutoModeApprovePermissionPolicy implements PermissionPolicy { + readonly name = 'auto-mode-approve'; + + constructor(private readonly agent: Agent) {} + + evaluate(): PermissionPolicyResult | undefined { + if (this.agent.permission.mode !== 'auto') return; + return { + kind: 'approve', + }; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/auto-mode-ask-user-question-deny.ts b/packages/agent-core/src/agent/permission/policies/auto-mode-ask-user-question-deny.ts new file mode 100644 index 0000000..f1f31d3 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/auto-mode-ask-user-question-deny.ts @@ -0,0 +1,18 @@ +import type { Agent } from '../..'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../types'; + +export class AutoModeAskUserQuestionDenyPermissionPolicy implements PermissionPolicy { + readonly name = 'auto-mode-ask-user-question-deny'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + if (this.agent.permission.mode !== 'auto') return; + if (context.toolCall.name !== 'AskUserQuestion') return; + return { + kind: 'deny', + message: + 'AskUserQuestion is disabled while auto permission mode is active. Make a reasonable decision and continue without asking the user.', + }; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/default-git-cwd-write.ts b/packages/agent-core/src/agent/permission/policies/default-git-cwd-write.ts deleted file mode 100644 index b5bc7ba..0000000 --- a/packages/agent-core/src/agent/permission/policies/default-git-cwd-write.ts +++ /dev/null @@ -1,127 +0,0 @@ -import * as posixPath from 'node:path/posix'; - -import type { Kaos } from '@moonshot-ai/kaos'; - -import { - DEFAULT_WORKSPACE_ACCESS_POLICY, - isWithinDirectory, - resolvePathAccess, -} from '../../../tools/policies/path-access'; -import { isSensitiveFile } from '../../../tools/policies/sensitive'; -import { - findGitWorkTreeMarker, - type GitWorkTreeMarker, -} from '../../../tools/support/git-worktree'; -import type { PermissionPolicy } from '../policy'; - -const AUTO_REASON = 'default_git_cwd_write'; -const S_IFMT = 0o170000; -const S_IFLNK = 0o120000; - -export function createDefaultGitCwdWritePolicy(): PermissionPolicy { - // Cache positive marker lookups only. A session that starts in a non-git - // directory and later `git init`s should pick up the new work tree on the - // next call; negative results pay one extra stat per call, which is - // acceptable. - const cache = new Map(); - - return { - name: 'default.git-cwd-write', - async evaluate({ agent, mode, matchedRule, toolCallContext }) { - if (mode !== 'manual') return undefined; - if (matchedRule !== undefined) return undefined; - - const toolName = toolCallContext.toolCall.name; - if (toolName !== 'Write' && toolName !== 'Edit') return undefined; - - const kaos = agent.runtime.kaos; - const pathClass = kaos.pathClass(); - if (pathClass !== 'posix') return undefined; - - const cwd = agent.config.cwd; - if (cwd.length === 0) return undefined; - - const path = readStringField(toolCallContext.args, 'path'); - if (path === undefined) return undefined; - - let access; - try { - access = resolvePathAccess( - path, - cwd, - { workspaceDir: cwd, additionalDirs: [] }, - { - operation: 'write', - pathClass, - homeDir: kaos.gethome(), - policy: DEFAULT_WORKSPACE_ACCESS_POLICY, - }, - ); - } catch { - return undefined; - } - if (access.outsideWorkspace) return undefined; - - const marker = cache.get(cwd) ?? (await findGitWorkTreeMarker(kaos, cwd)); - if (marker === null) return undefined; - cache.set(cwd, marker); - - if (isGitControlPath(access.path, cwd, marker)) return undefined; - if (isSensitiveFile(access.path.toLowerCase(), 'posix')) return undefined; - if (await hasSymlinkInPath(kaos, cwd, access.path)) return undefined; - - agent.telemetry.track('tool_approved', { - tool_name: toolName, - approval_mode: 'manual', - auto_reason: AUTO_REASON, - }); - return { kind: 'allow' }; - }, - }; -} - -function readStringField(args: unknown, key: string): string | undefined { - if (args === null || typeof args !== 'object') return undefined; - const value = (args as Record)[key]; - return typeof value === 'string' ? value : undefined; -} - -function isGitControlPath(targetPath: string, cwd: string, marker: GitWorkTreeMarker): boolean { - const foldedTarget = targetPath.toLowerCase(); - return ( - posixPath.relative(cwd.toLowerCase(), foldedTarget).split(posixPath.sep).includes('.git') || - isWithinDirectory(foldedTarget, marker.dotGitPath.toLowerCase(), 'posix') || - isWithinDirectory(foldedTarget, marker.controlDirPath.toLowerCase(), 'posix') - ); -} - -async function hasSymlinkInPath(kaos: Kaos, cwd: string, targetPath: string): Promise { - const relative = posixPath.relative(cwd, targetPath); - const parts = [cwd]; - - let current = cwd; - for (const part of relative.split(posixPath.sep)) { - if (part.length === 0 || part === '.') continue; - current = posixPath.join(current, part); - parts.push(current); - } - - for (let index = 0; index < parts.length; index += 1) { - try { - const stat = await kaos.stat(parts[index]!, { followSymlinks: false }); - if ((stat.stMode & S_IFMT) === S_IFLNK) return true; - } catch (error) { - return !(index === parts.length - 1 && isFileNotFoundError(error)); - } - } - return false; -} - -function isFileNotFoundError(error: unknown): boolean { - if (error === null || typeof error !== 'object') return false; - if ((error as { name?: unknown }).name === 'KaosFileNotFoundError') return true; - const code = (error as { code?: unknown }).code; - if (code === 'ENOENT' || code === 'ENOTDIR' || code === 2) return true; - const message = error instanceof Error ? error.message : ''; - return message.includes('ENOENT') || message.includes('ENOTDIR'); -} diff --git a/packages/agent-core/src/agent/permission/policies/default-tool-approve.ts b/packages/agent-core/src/agent/permission/policies/default-tool-approve.ts new file mode 100644 index 0000000..c2944fa --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/default-tool-approve.ts @@ -0,0 +1,28 @@ +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../types'; + +const DEFAULT_APPROVE_TOOLS = new Set([ + 'Read', + 'Grep', + 'Glob', + 'ReadMediaFile', + 'SetTodoList', + 'TodoList', + 'TaskList', + 'TaskOutput', + 'WebSearch', + 'FetchURL', + 'Agent', + 'AskUserQuestion', + 'Skill', +]); + +export class DefaultToolApprovePermissionPolicy implements PermissionPolicy { + readonly name = 'default-tool-approve'; + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + if (!DEFAULT_APPROVE_TOOLS.has(context.toolCall.name)) return; + return { + kind: 'approve', + }; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/exit-plan-mode-review-ask.ts b/packages/agent-core/src/agent/permission/policies/exit-plan-mode-review-ask.ts new file mode 100644 index 0000000..7cd90ea --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/exit-plan-mode-review-ask.ts @@ -0,0 +1,172 @@ +import type { Agent } from '../..'; +import type { ApprovalResponse, PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../types'; + +interface ExitPlanModeOption { + readonly label: string; + readonly description: string; +} + +interface PlanReviewDisplay { + readonly plan: string; + readonly path?: string | undefined; + readonly options?: readonly ExitPlanModeOption[] | undefined; +} + +export class ExitPlanModeReviewAskPermissionPolicy implements PermissionPolicy { + readonly name = 'exit-plan-mode-review-ask'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + if (context.toolCall.name !== 'ExitPlanMode') return; + if (this.agent.permission.mode === 'auto') return; + if (!this.agent.planMode.isActive) return; + const display = context.execution.display; + if (display?.kind !== 'plan_review') return; + if (display.plan.trim().length === 0) return; + this.agent.telemetry.track('plan_submitted', { + has_options: display.options !== undefined && display.options.length >= 2, + }); + return { + kind: 'ask', + reason: { + has_options: display.options !== undefined, + }, + resolveApproval: (result) => + this.exitPlanModeApprovalResult(result, { + plan: display.plan, + path: display.path, + options: display.options, + }), + }; + } + + private exitPlanModeApprovalResult(result: ApprovalResponse, display: PlanReviewDisplay) { + if (result.decision !== 'approved') { + return this.rejectedExitPlanModeApprovalResult(result); + } + + const selected = selectedExitPlanModeOption(display.options, result.selectedLabel); + + const failed = this.exitPlanMode(); + if (failed !== undefined) { + return { kind: 'result' as const, syntheticResult: failed }; + } + + if (result.selectedLabel !== undefined && result.selectedLabel.length > 0) { + this.agent.telemetry.track('plan_resolved', { + outcome: 'approved', + chosen_option: result.selectedLabel, + }); + } else { + this.agent.telemetry.track('plan_resolved', { outcome: 'approved' }); + } + + const optionPrefix = + selected === undefined + ? '' + : `Selected approach: ${selected.label}\nExecute ONLY the selected approach. Do not execute any unselected alternatives.\n\n`; + const savedTo = display.path !== undefined ? `Plan saved to: ${display.path}\n\n` : ''; + const formattedPlan = `Plan mode deactivated. All tools are now available.\n${savedTo}## Approved Plan:\n${display.plan}`; + return { + kind: 'result' as const, + syntheticResult: { + isError: false, + output: `Exited plan mode. ${optionPrefix}${formattedPlan}`, + }, + }; + } + + private rejectedExitPlanModeApprovalResult(result: ApprovalResponse) { + this.trackRejectedPlanResolution(result); + + if (result.decision === 'cancelled') { + return { + kind: 'result' as const, + syntheticResult: { + isError: false, + output: 'Plan approval dismissed. Plan mode remains active.', + }, + }; + } + + if (result.selectedLabel === 'Reject and Exit') { + const failed = this.exitPlanMode(); + return { + kind: 'result' as const, + syntheticResult: + failed ?? { + isError: true, + stopTurn: true, + output: 'Plan rejected by user. Plan mode deactivated.', + }, + }; + } + + const feedback = result.feedback ?? ''; + if (result.selectedLabel === 'Revise' || feedback.length > 0) { + return { + kind: 'result' as const, + syntheticResult: { + isError: false, + output: + feedback.length > 0 + ? `User rejected the plan. Feedback:\n\n${feedback}` + : 'User requested revisions. Plan mode remains active.', + }, + }; + } + + return { + kind: 'result' as const, + syntheticResult: { + isError: true, + stopTurn: true, + output: 'Plan rejected by user. Plan mode remains active.', + }, + }; + } + + private exitPlanMode(): { isError: true; output: string } | undefined { + try { + this.agent.planMode.exit(); + } catch (error) { + const message = error instanceof Error ? error.message : 'Failed to exit plan mode.'; + return { + isError: true, + output: `Failed to exit plan mode: ${message}`, + }; + } + } + + private trackRejectedPlanResolution(result: ApprovalResponse): void { + if (result.decision === 'cancelled') { + this.agent.telemetry.track('plan_resolved', { outcome: 'dismissed' }); + return; + } + + if (result.selectedLabel === 'Reject and Exit') { + this.agent.telemetry.track('plan_resolved', { outcome: 'rejected_and_exited' }); + return; + } + + const feedback = result.feedback ?? ''; + if (result.selectedLabel === 'Revise' || feedback.length > 0) { + this.agent.telemetry.track('plan_resolved', { + outcome: 'revise', + has_feedback: feedback.length > 0, + }); + return; + } + + this.agent.telemetry.track('plan_resolved', { outcome: 'rejected' }); + } +} + +function selectedExitPlanModeOption( + options: readonly ExitPlanModeOption[] | undefined, + label: string | undefined, +): ExitPlanModeOption | undefined { + if (options === undefined || label === undefined) return; + return options.find((option) => option.label === label); +} diff --git a/packages/agent-core/src/agent/permission/policies/fallback-ask.ts b/packages/agent-core/src/agent/permission/policies/fallback-ask.ts new file mode 100644 index 0000000..3289519 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/fallback-ask.ts @@ -0,0 +1,11 @@ +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../types'; + +export class FallbackAskPermissionPolicy implements PermissionPolicy { + readonly name = 'fallback-ask'; + + evaluate(_context: PermissionPolicyContext): PermissionPolicyResult { + return { + kind: 'ask', + }; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/file-access-ask.ts b/packages/agent-core/src/agent/permission/policies/file-access-ask.ts new file mode 100644 index 0000000..b2c243f --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/file-access-ask.ts @@ -0,0 +1,144 @@ +import * as posixPath from 'node:path/posix'; +import * as win32Path from 'node:path/win32'; + +import type { Agent } from '../..'; +import type { ToolFileAccess } from '../../../loop/tool-access'; +import { isWithinDirectory, type PathClass } from '../../../tools/policies/path-access'; +import { isSensitiveFile } from '../../../tools/policies/sensitive'; +import { + findGitWorkTreeMarker, + type GitWorkTreeMarker, +} from '../../../tools/support/git-worktree'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../types'; + +export class SensitiveFileAccessAskPermissionPolicy implements PermissionPolicy { + readonly name = 'sensitive-file-access-ask'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const pathClass = this.agent.runtime.kaos.pathClass(); + const access = fileAccesses(context).find((fileAccess) => + isSensitiveFile(fileAccess.path, pathClass), + ); + if (access === undefined) return; + return { + kind: 'ask', + reason: fileAccessReason(access, { sensitive_path: true }), + }; + } +} + +export class GitControlPathAccessAskPermissionPolicy implements PermissionPolicy { + private readonly gitMarkerCache = new Map(); + readonly name = 'git-control-path-access-ask'; + + constructor(private readonly agent: Agent) {} + + async evaluate(context: PermissionPolicyContext): Promise { + const cwd = this.agent.config.cwd; + if (cwd.length === 0) return; + const pathClass = this.agent.runtime.kaos.pathClass(); + const accesses = fileAccesses(context); + if (accesses.length === 0) return; + + const directGitAccess = accesses.find((fileAccess) => { + return hasGitPathComponent(fileAccess.path, cwd, pathClass); + }); + if (directGitAccess !== undefined) { + return { + kind: 'ask', + reason: fileAccessReason(directGitAccess, { git_control_path: true }), + }; + } + + const marker = await this.findGitMarker(cwd); + if (marker === null) return; + const access = accesses.find((fileAccess) => { + return isGitControlPath(fileAccess.path, marker, pathClass); + }); + if (access === undefined) return; + return { + kind: 'ask', + reason: fileAccessReason(access, { git_control_path: true }), + }; + } + + private async findGitMarker(cwd: string): Promise { + if (this.gitMarkerCache.has(cwd)) return this.gitMarkerCache.get(cwd) ?? null; + const marker = await findGitWorkTreeMarker(this.agent.runtime.kaos, cwd); + this.gitMarkerCache.set(cwd, marker); + return marker; + } +} + +export class CwdOutsideFileWriteAskPermissionPolicy implements PermissionPolicy { + readonly name = 'cwd-outside-file-write-ask'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const cwd = this.agent.config.cwd; + if (cwd.length === 0) return; + const pathClass = this.agent.runtime.kaos.pathClass(); + const access = writeFileAccesses(context).find((fileAccess) => { + return !isWithinDirectory(fileAccess.path, cwd, pathClass); + }); + if (access === undefined) return; + return { + kind: 'ask', + reason: fileAccessReason(access, { cwd_outside: true }), + }; + } +} + +function fileAccesses(context: PermissionPolicyContext): ToolFileAccess[] { + return ( + context.execution.accesses?.filter((access): access is ToolFileAccess => access.kind === 'file') ?? + [] + ); +} + +export function writeFileAccesses(context: PermissionPolicyContext): ToolFileAccess[] { + return fileAccesses(context).filter( + (access) => access.operation === 'write' || access.operation === 'readwrite', + ); +} + +function fileAccessReason(access: ToolFileAccess, extra: Record) { + return { + file_access_operation: access.operation, + recursive: access.recursive === true, + ...extra, + }; +} + +function hasGitPathComponent( + targetPath: string, + cwd: string, + pathClass: PathClass, +): boolean { + return relativePathParts(targetPath, cwd, pathClass).some((part) => part.toLowerCase() === '.git'); +} + +function isGitControlPath( + targetPath: string, + marker: GitWorkTreeMarker, + pathClass: PathClass, +): boolean { + return ( + isWithinDirectory(targetPath, marker.dotGitPath, pathClass) || + isWithinDirectory(targetPath, marker.controlDirPath, pathClass) + ); +} + +function relativePathParts(targetPath: string, cwd: string, pathClass: PathClass): string[] { + return pathMod(pathClass) + .relative(cwd, targetPath) + .split(/[\\/]+/) + .filter((part) => part.length > 0); +} + +function pathMod(pathClass: PathClass): typeof posixPath { + return pathClass === 'win32' ? win32Path : posixPath; +} diff --git a/packages/agent-core/src/agent/permission/policies/git-cwd-write-approve.ts b/packages/agent-core/src/agent/permission/policies/git-cwd-write-approve.ts new file mode 100644 index 0000000..1b0732e --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/git-cwd-write-approve.ts @@ -0,0 +1,44 @@ +import type { Agent } from '../..'; +import { isWithinDirectory } from '../../../tools/policies/path-access'; +import { + findGitWorkTreeMarker, + type GitWorkTreeMarker, +} from '../../../tools/support/git-worktree'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../types'; +import { writeFileAccesses } from './file-access-ask'; + +export class GitCwdWriteApprovePermissionPolicy implements PermissionPolicy { + private readonly gitMarkerCache = new Map(); + readonly name = 'git-cwd-write-approve'; + + constructor(private readonly agent: Agent) {} + + async evaluate(context: PermissionPolicyContext): Promise { + const toolName = context.toolCall.name; + if (toolName !== 'Write' && toolName !== 'Edit') return; + if (this.agent.runtime.kaos.pathClass() !== 'posix') return; + + const cwd = this.agent.config.cwd; + if (cwd.length === 0) return; + + const writeAccesses = writeFileAccesses(context); + if (writeAccesses.length === 0) return; + if (!writeAccesses.every((access) => isWithinDirectory(access.path, cwd, 'posix'))) { + return; + } + + const marker = await this.findGitMarker(cwd); + if (marker === null) return; + + return { + kind: 'approve', + }; + } + + private async findGitMarker(cwd: string): Promise { + if (this.gitMarkerCache.has(cwd)) return this.gitMarkerCache.get(cwd) ?? null; + const marker = await findGitWorkTreeMarker(this.agent.runtime.kaos, cwd); + this.gitMarkerCache.set(cwd, marker); + return marker; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/index.ts b/packages/agent-core/src/agent/permission/policies/index.ts index 07459fe..38e0bb9 100644 --- a/packages/agent-core/src/agent/permission/policies/index.ts +++ b/packages/agent-core/src/agent/permission/policies/index.ts @@ -1,19 +1,63 @@ -import type { PermissionPolicy } from '../policy'; -import { AskUserQuestionAutoPermissionPolicy } from './ask-user-question'; -import { createDefaultGitCwdWritePolicy } from './default-git-cwd-write'; -import { createPlanPermissionPolicies } from './plan'; -import { YoloOutsideWorkspacePermissionPolicy } from './yolo-workspace-access'; +import type { Agent } from '../..'; +import type { PermissionPolicy } from '../types'; +import { AutoModeApprovePermissionPolicy } from './auto-mode-approve'; +import { AutoModeAskUserQuestionDenyPermissionPolicy } from './auto-mode-ask-user-question-deny'; +import { DefaultToolApprovePermissionPolicy } from './default-tool-approve'; +import { ExitPlanModeReviewAskPermissionPolicy } from './exit-plan-mode-review-ask'; +import { FallbackAskPermissionPolicy } from './fallback-ask'; +import { + CwdOutsideFileWriteAskPermissionPolicy, + GitControlPathAccessAskPermissionPolicy, + SensitiveFileAccessAskPermissionPolicy, +} from './file-access-ask'; +import { GitCwdWriteApprovePermissionPolicy } from './git-cwd-write-approve'; +import { PlanModeGuardDenyPermissionPolicy } from './plan-mode-guard-deny'; +import { PlanModeToolApprovePermissionPolicy } from './plan-mode-tool-approve'; +import { PreToolCallHookPermissionPolicy } from './pre-tool-call-hook'; +import { SessionApprovalHistoryPermissionPolicy } from './session-approval-history'; +import { + UserConfiguredAllowPermissionPolicy, + UserConfiguredAskPermissionPolicy, + UserConfiguredDenyPermissionPolicy, +} from './user-configured-rules'; +import { YoloModeApprovePermissionPolicy } from './yolo-mode-approve'; -export function createBuiltinPermissionPolicies(): readonly PermissionPolicy[] { +/** Permission policies run in order; the first non-undefined result wins. */ +export function createPermissionDecisionPolicies(agent: Agent): readonly PermissionPolicy[] { return [ - ...createPlanPermissionPolicies(), - YoloOutsideWorkspacePermissionPolicy, - createDefaultGitCwdWritePolicy(), - AskUserQuestionAutoPermissionPolicy, + // PreToolUse hook returned a block → deny. + new PreToolCallHookPermissionPolicy(agent), + // auto mode + AskUserQuestion → deny. + new AutoModeAskUserQuestionDenyPermissionPolicy(agent), + // plan mode: Write/Edit outside the plan file, or TaskStop → deny. + new PlanModeGuardDenyPermissionPolicy(agent), + // User-configured deny rule matches → deny. + new UserConfiguredDenyPermissionPolicy(agent), + // auto mode → approve (any auto-mode block must be a deny rule above this). + new AutoModeApprovePermissionPolicy(agent), + // Approve-for-session memorized rule matches → approve. Runs before user-configured ask rules so an in-session grant beats a still-matching ask rule on later calls. + new SessionApprovalHistoryPermissionPolicy(agent), + // User-configured ask rule matches → ask. + new UserConfiguredAskPermissionPolicy(agent), + // User-configured allow rule matches → approve. + new UserConfiguredAllowPermissionPolicy(agent), + // ExitPlanMode with active plan_review + non-empty plan + non-auto → ask (tracks plan_submitted/plan_resolved itself). Runs before session history so a stale session approval can't bypass review of a new plan body. + new ExitPlanModeReviewAskPermissionPolicy(agent), + // EnterPlanMode, Write/Edit on the plan file, or ExitPlanMode with no actionable plan_review → approve. + new PlanModeToolApprovePermissionPolicy(agent), + // Access touches a sensitive file (.env, SSH key, credentials) → ask. + new SensitiveFileAccessAskPermissionPolicy(agent), + // Access touches .git or a git control-dir path → ask. + new GitControlPathAccessAskPermissionPolicy(agent), + // Write target is outside cwd → ask. Reads and searches outside cwd are allowed without prompting. + new CwdOutsideFileWriteAskPermissionPolicy(agent), + // yolo mode → approve. + new YoloModeApprovePermissionPolicy(agent), + // Tool is in the default-approve list (read-only / UI helpers) → approve. + new DefaultToolApprovePermissionPolicy(), + // Write/Edit on POSIX paths inside cwd inside a git work tree → approve. + new GitCwdWriteApprovePermissionPolicy(agent), + // Nothing matched → ask. + new FallbackAskPermissionPolicy(), ]; } - -export { AskUserQuestionAutoPermissionPolicy } from './ask-user-question'; -export { createDefaultGitCwdWritePolicy } from './default-git-cwd-write'; -export { createPlanPermissionPolicies } from './plan'; -export { YoloOutsideWorkspacePermissionPolicy } from './yolo-workspace-access'; diff --git a/packages/agent-core/src/agent/permission/policies/plan-mode-guard-deny.ts b/packages/agent-core/src/agent/permission/policies/plan-mode-guard-deny.ts new file mode 100644 index 0000000..b25e294 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/plan-mode-guard-deny.ts @@ -0,0 +1,54 @@ +import type { Agent } from '../..'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../types'; +import { writeFileAccesses } from './file-access-ask'; + +export class PlanModeGuardDenyPermissionPolicy implements PermissionPolicy { + readonly name = 'plan-mode-guard-deny'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + if (!this.agent.planMode.isActive) return; + + const toolName = context.toolCall.name; + if (toolName === 'Write' || toolName === 'Edit') { + const planFilePath = this.agent.planMode.planFilePath; + if (planFilePath === null) { + return { + kind: 'deny', + message: planModeWriteDeniedMessage(planFilePath), + }; + } + if (writesOnlyPlanFile(context, planFilePath)) { + return; + } + return { + kind: 'deny', + message: planModeWriteDeniedMessage(planFilePath), + }; + } + + if (toolName !== 'TaskStop') return; + return { + kind: 'deny', + message: + 'TaskStop is not available in plan mode. Call ExitPlanMode to exit plan mode before stopping a background task.', + }; + } +} + +function writesOnlyPlanFile( + context: PermissionPolicyContext, + planFilePath: string, +): boolean { + const writeAccesses = writeFileAccesses(context); + if (writeAccesses.length === 0) return false; + return writeAccesses.every((access) => access.path === planFilePath); +} + +function planModeWriteDeniedMessage(planFilePath: string | null): string { + return ( + `Plan mode is active. You may only write to the current plan file: ${planFilePath ?? '(no plan file selected yet)'}. ` + + 'Call ExitPlanMode to exit plan mode before editing other files.' + ); +} diff --git a/packages/agent-core/src/agent/permission/policies/plan-mode-tool-approve.ts b/packages/agent-core/src/agent/permission/policies/plan-mode-tool-approve.ts new file mode 100644 index 0000000..1a534aa --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/plan-mode-tool-approve.ts @@ -0,0 +1,54 @@ +import type { Agent } from '../..'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../types'; +import { writeFileAccesses } from './file-access-ask'; + +export class PlanModeToolApprovePermissionPolicy implements PermissionPolicy { + readonly name = 'plan-mode-tool-approve'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const toolName = context.toolCall.name; + if (toolName === 'EnterPlanMode') { + return { + kind: 'approve', + }; + } + + if ( + (toolName === 'Write' || toolName === 'Edit') && + this.agent.planMode.isActive && + writesOnlyPlanFile(context, this.agent.planMode.planFilePath) + ) { + return { + kind: 'approve', + }; + } + + if (toolName === 'ExitPlanMode') { + if (!this.agent.planMode.isActive) { + return { + kind: 'approve', + }; + } + if (context.execution.display?.kind !== 'plan_review') { + return { + kind: 'approve', + }; + } + if (context.execution.display.plan.trim().length > 0) return; + return { + kind: 'approve', + }; + } + } +} + +function writesOnlyPlanFile( + context: PermissionPolicyContext, + planFilePath: string | null, +): boolean { + if (planFilePath === null) return false; + const writeAccesses = writeFileAccesses(context); + return writeAccesses.every((access) => access.path === planFilePath); +} diff --git a/packages/agent-core/src/agent/permission/policies/plan.ts b/packages/agent-core/src/agent/permission/policies/plan.ts deleted file mode 100644 index bb2f7ac..0000000 --- a/packages/agent-core/src/agent/permission/policies/plan.ts +++ /dev/null @@ -1,324 +0,0 @@ -import type { ExecutableToolResult } from '../../../loop'; -import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; -import type { ApprovalResponse } from '../types'; - -interface ExitPlanModeOption { - readonly label: string; - readonly description: string; -} - -interface ExitPlanModeExecutionMetadata { - readonly selectedOption?: ExitPlanModeOption | undefined; - readonly planTelemetrySubmitted: true; - readonly planTelemetryResolved: true; -} - -export const EnterPlanModePermissionPolicy: PermissionPolicy = { - name: 'plan.enter-plan-mode', - evaluate({ toolCallContext }) { - if (toolCallContext.toolCall.name !== 'EnterPlanMode') return undefined; - return { kind: 'allow' }; - }, -}; - -export const ExitPlanModePermissionPolicy: PermissionPolicy = { - name: 'plan.exit-plan-mode', - async evaluate(context) { - if (context.toolCallContext.toolCall.name !== 'ExitPlanMode') return undefined; - if (context.mode === 'auto') return { kind: 'allow' }; - - const review = await resolveExitPlanModeReview(context); - if (review === null) return { kind: 'allow' }; - - const action = exitPlanModeAction(review.options); - context.agent.telemetry.track('plan_submitted', { - has_options: review.options !== undefined, - }); - let result: ApprovalResponse; - try { - result = await context.agent.rpc.requestApproval( - { - turnId: Number(context.toolCallContext.turnId), - toolCallId: context.toolCallContext.toolCall.id, - toolName: 'ExitPlanMode', - action, - display: { - kind: 'plan_review', - plan: review.plan, - path: review.path, - options: review.options, - }, - }, - { signal: context.toolCallContext.signal }, - ); - } catch (error) { - const message = error instanceof Error ? error.message : 'Plan approval failed.'; - return { - kind: 'result', - result: { - syntheticResult: { - isError: true, - output: `Plan approval failed: ${message}`, - }, - }, - }; - } - - context.recordApprovalResult({ - turnId: Number(context.toolCallContext.turnId), - toolCallId: context.toolCallContext.toolCall.id, - toolName: 'ExitPlanMode', - action, - result, - }); - - trackExitPlanModeResolution(context, result); - return exitPlanModeApprovalResult(context, result, review.options); - }, -}; - -export const PlanModeGuardPermissionPolicy: PermissionPolicy = { - name: 'plan.mode-guard', - evaluate({ agent, toolCallContext }) { - if (!agent.planMode.isActive) return undefined; - - const name = toolCallContext.toolCall.name; - const args = toolCallContext.args; - - if (name === 'Write' || name === 'Edit') { - const path = readStringField(args, 'path'); - if (path === undefined) return undefined; - const planFilePath = agent.planMode.planFilePath; - if (planFilePath !== null && path === planFilePath) return { kind: 'allow' }; - return { - kind: 'result', - result: { - block: true, - reason: - `Plan mode is active. You may only write to the current plan file: ${planFilePath ?? '(no plan file selected yet)'}. ` + - 'Call ExitPlanMode to exit plan mode before editing other files.', - }, - }; - } - - if (name === 'TaskStop') { - return { - kind: 'result', - result: { - block: true, - reason: - 'TaskStop is not available in plan mode. ' + - 'Call ExitPlanMode to exit plan mode before stopping a background task.', - }, - }; - } - - return undefined; - }, -}; - -export function createPlanPermissionPolicies(): readonly PermissionPolicy[] { - return [ - EnterPlanModePermissionPolicy, - ExitPlanModePermissionPolicy, - PlanModeGuardPermissionPolicy, - ]; -} - -async function resolveExitPlanModeReview(context: PermissionPolicyContext): Promise<{ - readonly plan: string; - readonly path?: string | undefined; - readonly options?: readonly ExitPlanModeOption[] | undefined; -} | null> { - if (!context.agent.planMode.isActive) return null; - - let data: Awaited>; - try { - data = await context.agent.planMode.data(); - } catch { - return null; - } - if (data === null || data.content.trim().length === 0) return null; - - return { - plan: data.content, - path: data.path, - options: exitPlanModeOptions(context.toolCallContext.args), - }; -} - -function exitPlanModeApprovalResult( - context: PermissionPolicyContext, - result: ApprovalResponse, - options: readonly ExitPlanModeOption[] | undefined, -): PermissionPolicyResult { - if (result.decision === 'approved') { - const selected = selectedExitPlanModeOption(options, result.selectedLabel); - return { - kind: 'allow', - executionMetadata: exitPlanModeExecutionMetadata(selected), - }; - } - - if (result.decision === 'cancelled') { - return { - kind: 'result', - result: { - syntheticResult: { - isError: false, - output: 'Plan approval dismissed. Plan mode remains active.', - }, - }, - }; - } - - if (result.selectedLabel === 'Reject and Exit') { - const failed = exitPlanModeForRejectedPlan(context); - return { - kind: 'result', - result: { - syntheticResult: - failed ?? { - isError: true, - stopTurn: true, - output: 'Plan rejected by user. Plan mode deactivated.', - }, - }, - }; - } - - const feedback = result.feedback ?? ''; - if (result.selectedLabel === 'Revise' || feedback.length > 0) { - return { - kind: 'result', - result: { - syntheticResult: { - isError: false, - output: - feedback.length > 0 - ? `User rejected the plan. Feedback:\n\n${feedback}` - : 'User requested revisions. Plan mode remains active.', - }, - }, - }; - } - - return { - kind: 'result', - result: { - syntheticResult: { - isError: true, - stopTurn: true, - output: 'Plan rejected by user. Plan mode remains active.', - }, - }, - }; -} - -function exitPlanModeExecutionMetadata( - selectedOption: ExitPlanModeOption | undefined, -): ExitPlanModeExecutionMetadata { - return { - selectedOption, - planTelemetrySubmitted: true, - planTelemetryResolved: true, - }; -} - -function trackExitPlanModeResolution( - context: PermissionPolicyContext, - result: ApprovalResponse, -): void { - const selectedLabel = result.selectedLabel ?? ''; - const normalizedSelectedLabel = normalizeOptionLabel(selectedLabel); - const feedback = result.feedback ?? ''; - const hasFeedback = feedback.length > 0; - - if (result.decision === 'cancelled') { - context.agent.telemetry.track('plan_resolved', { outcome: 'dismissed' }); - return; - } - - if (result.decision === 'approved') { - if (selectedLabel.length > 0) { - context.agent.telemetry.track('plan_resolved', { - outcome: 'approved', - chosen_option: selectedLabel, - }); - return; - } - context.agent.telemetry.track('plan_resolved', { outcome: 'approved' }); - return; - } - - if (normalizedSelectedLabel === 'reject and exit') { - context.agent.telemetry.track('plan_resolved', { outcome: 'rejected_and_exited' }); - return; - } - - if (normalizedSelectedLabel === 'revise' || hasFeedback) { - context.agent.telemetry.track('plan_resolved', { - outcome: 'revise', - has_feedback: hasFeedback, - }); - return; - } - - context.agent.telemetry.track('plan_resolved', { outcome: 'rejected' }); -} - -function exitPlanModeForRejectedPlan( - context: PermissionPolicyContext, -): ExecutableToolResult | undefined { - try { - context.agent.planMode.exit(); - } catch (error) { - const message = error instanceof Error ? error.message : 'Failed to exit plan mode.'; - return { - isError: true, - output: `Failed to exit plan mode: ${message}`, - }; - } -} - -function exitPlanModeOptions(args: unknown): readonly ExitPlanModeOption[] | undefined { - if (args === null || typeof args !== 'object') return undefined; - const options = (args as { readonly options?: unknown }).options; - if (!Array.isArray(options) || options.length < 2) return undefined; - const parsed: ExitPlanModeOption[] = []; - for (const option of options) { - if (option === null || typeof option !== 'object') return undefined; - const label = (option as { readonly label?: unknown }).label; - if (typeof label !== 'string') return undefined; - // `description` is optional in the ExitPlanMode schema (defaults to ''), - // so an option that omits it is still valid. - const description = (option as { readonly description?: unknown }).description; - if (description !== undefined && typeof description !== 'string') return undefined; - parsed.push({ label, description: description ?? '' }); - } - return parsed; -} - -function selectedExitPlanModeOption( - options: readonly ExitPlanModeOption[] | undefined, - label: string | undefined, -): ExitPlanModeOption | undefined { - if (options === undefined || label === undefined) return undefined; - return options.find((option) => option.label === label); -} - -function exitPlanModeAction(options: readonly ExitPlanModeOption[] | undefined): string { - return options !== undefined && options.length >= 2 - ? 'Review plan and choose an option' - : 'Review plan'; -} - -function normalizeOptionLabel(label: string): string { - return label.trim().toLowerCase(); -} - -function readStringField(args: unknown, key: string): string | undefined { - if (args === null || typeof args !== 'object') return undefined; - const value = (args as Record)[key]; - return typeof value === 'string' ? value : undefined; -} diff --git a/packages/agent-core/src/agent/permission/policies/pre-tool-call-hook.ts b/packages/agent-core/src/agent/permission/policies/pre-tool-call-hook.ts new file mode 100644 index 0000000..294125d --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/pre-tool-call-hook.ts @@ -0,0 +1,27 @@ +import type { Agent } from '../..'; +import { isPlainRecord } from '../../turn/canonical-args'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../types'; + +export class PreToolCallHookPermissionPolicy implements PermissionPolicy { + readonly name = 'pre-tool-call-hook'; + + constructor(private readonly agent: Agent) {} + + async evaluate(context: PermissionPolicyContext): Promise { + const hookResult = await this.agent.hooks?.triggerBlock('PreToolUse', { + matcherValue: context.toolCall.name, + signal: context.signal, + inputData: { + toolName: context.toolCall.name, + toolInput: isPlainRecord(context.args) ? context.args : {}, + toolCallId: context.toolCall.id, + }, + }); + context.signal.throwIfAborted(); + if (hookResult === undefined) return; + return { + kind: 'deny', + message: hookResult.reason, + }; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/session-approval-history.ts b/packages/agent-core/src/agent/permission/policies/session-approval-history.ts new file mode 100644 index 0000000..e910728 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/session-approval-history.ts @@ -0,0 +1,46 @@ +import type { Agent } from '../..'; +import { + matchPermissionRule, + type PermissionRuleMatch, +} from '../matches-rule'; +import type { + PermissionPolicy, + PermissionPolicyContext, + PermissionPolicyResult, +} from '../types'; + +export class SessionApprovalHistoryPermissionPolicy implements PermissionPolicy { + readonly name = 'session-approval-history'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const match = this.matchSessionApprovalRule(context); + if (match === undefined) return; + return { + kind: 'approve', + reason: { + has_rule_args: match.hasRuleArgs, + match_strategy: match.strategy, + }, + }; + } + + private matchSessionApprovalRule( + context: PermissionPolicyContext, + ): PermissionRuleMatch | undefined { + for (const pattern of this.agent.permission.sessionApprovalRulePatterns) { + const match = matchPermissionRule({ + rule: { + decision: 'allow', + scope: 'session-runtime', + pattern, + reason: 'approve for session', + }, + toolName: context.toolCall.name, + execution: context.execution, + }); + if (match !== undefined) return match; + } + } +} diff --git a/packages/agent-core/src/agent/permission/policies/user-configured-rules.ts b/packages/agent-core/src/agent/permission/policies/user-configured-rules.ts new file mode 100644 index 0000000..cca532a --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/user-configured-rules.ts @@ -0,0 +1,115 @@ +import type { Agent } from '../..'; +import { + matchPermissionRule, + type PermissionRuleMatch, +} from '../matches-rule'; +import type { + PermissionPolicy, + PermissionPolicyContext, + PermissionPolicyResult, + PermissionRule, + PermissionRuleDecision, + PermissionRuleScope, +} from '../types'; + +const USER_CONFIGURED_SCOPES = new Set([ + 'turn-override', + 'project', + 'user', +]); + +abstract class UserConfiguredPermissionPolicy { + constructor(protected readonly agent: Agent) {} + + protected firstMatchingRule( + context: PermissionPolicyContext, + decision: PermissionRuleDecision, + ): PermissionRuleMatch | undefined { + const rules = this.agent.permission.data().rules.filter((rule): rule is PermissionRule => + USER_CONFIGURED_SCOPES.has(rule.scope), + ); + for (const rule of rules) { + if (rule.decision !== decision) continue; + const match = matchPermissionRule({ + rule, + toolName: context.toolCall.name, + execution: context.execution, + }); + if (match !== undefined) return match; + } + return; + } +} + +export class UserConfiguredDenyPermissionPolicy + extends UserConfiguredPermissionPolicy + implements PermissionPolicy +{ + readonly name = 'user-configured-deny'; + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const match = this.firstMatchingRule(context, 'deny'); + if (match === undefined) return; + return { + kind: 'deny', + reason: userRuleReason('deny', match), + message: formatPermissionRuleDenyMessage( + context.toolCall.name, + match.rule.reason, + this.agent.type, + ), + }; + } +} + +export class UserConfiguredAllowPermissionPolicy + extends UserConfiguredPermissionPolicy + implements PermissionPolicy +{ + readonly name = 'user-configured-allow'; + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const match = this.firstMatchingRule(context, 'allow'); + if (match === undefined) return; + return { + kind: 'approve', + reason: userRuleReason('allow', match), + }; + } +} + +export class UserConfiguredAskPermissionPolicy + extends UserConfiguredPermissionPolicy + implements PermissionPolicy +{ + readonly name = 'user-configured-ask'; + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const match = this.firstMatchingRule(context, 'ask'); + if (match === undefined) return; + return { + kind: 'ask', + reason: userRuleReason('ask', match), + }; + } +} + +function userRuleReason(decision: PermissionRuleDecision, match: PermissionRuleMatch) { + return { + rule_decision: decision, + has_rule_args: match.hasRuleArgs, + match_strategy: match.strategy, + }; +} + +function formatPermissionRuleDenyMessage( + tool: string, + reason: string | undefined, + agentType?: Agent['type'], +): string { + const suffix = reason !== undefined && reason.length > 0 ? ` Reason: ${reason}` : ''; + if (agentType === 'sub') { + return `Tool "${tool}" was denied.${suffix} Try a different approach — don't retry the same call, don't attempt to bypass the restriction.`; + } + return `Tool "${tool}" was denied by permission rule.${suffix}`; +} diff --git a/packages/agent-core/src/agent/permission/policies/yolo-mode-approve.ts b/packages/agent-core/src/agent/permission/policies/yolo-mode-approve.ts new file mode 100644 index 0000000..cf56b96 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/yolo-mode-approve.ts @@ -0,0 +1,15 @@ +import type { Agent } from '../..'; +import type { PermissionPolicy, PermissionPolicyResult } from '../types'; + +export class YoloModeApprovePermissionPolicy implements PermissionPolicy { + readonly name = 'yolo-mode-approve'; + + constructor(private readonly agent: Agent) {} + + evaluate(): PermissionPolicyResult | undefined { + if (this.agent.permission.mode !== 'yolo') return; + return { + kind: 'approve', + }; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/yolo-workspace-access.ts b/packages/agent-core/src/agent/permission/policies/yolo-workspace-access.ts deleted file mode 100644 index d9627b1..0000000 --- a/packages/agent-core/src/agent/permission/policies/yolo-workspace-access.ts +++ /dev/null @@ -1,74 +0,0 @@ -import type { ToolInputDisplay } from '../../../tools/display'; -import { - DEFAULT_WORKSPACE_ACCESS_POLICY, - resolvePathAccess, - type PathAccessOperation, -} from '../../../tools/policies/path-access'; -import type { PermissionPolicy } from '../policy'; - -type FileInputDisplayOperation = Extract['operation']; - -const FILE_ACCESS_TOOLS: Readonly< - Record -> = { - Read: ['read', 'read'], - ReadMediaFile: ['read', 'read'], - Write: ['write', 'write'], - Edit: ['write', 'edit'], - Grep: ['search', 'grep'], -}; - -export const YoloOutsideWorkspacePermissionPolicy: PermissionPolicy = { - name: 'yolo.outside-workspace', - evaluate({ agent, mode, toolCallContext }) { - if (mode !== 'yolo') return undefined; - - const toolName = toolCallContext.toolCall.name; - const toolAccess = FILE_ACCESS_TOOLS[toolName]; - if (toolAccess === undefined) return undefined; - const [operation, displayOperation] = toolAccess; - - const rawPath = readStringField(toolCallContext.args, 'path'); - if (rawPath === undefined) return undefined; - - let access; - try { - access = resolvePathAccess( - rawPath, - agent.config.cwd, - { - workspaceDir: agent.config.cwd, - additionalDirs: agent.skills?.registry.getSkillRoots() ?? [], - }, - { - operation, - pathClass: agent.runtime.kaos.pathClass(), - homeDir: agent.runtime.kaos.gethome(), - policy: { - ...DEFAULT_WORKSPACE_ACCESS_POLICY, - checkSensitive: toolName !== 'Grep', - }, - }, - ); - } catch { - return undefined; - } - - if (!access.outsideWorkspace) return undefined; - return { - kind: 'ask', - display: { - kind: 'file_io', - operation: displayOperation, - path: access.path, - detail: `Outside workspace: ${agent.config.cwd}`, - }, - }; - }, -}; - -function readStringField(args: unknown, key: string): string | undefined { - if (args === null || typeof args !== 'object') return undefined; - const value = (args as Record)[key]; - return typeof value === 'string' ? value : undefined; -} diff --git a/packages/agent-core/src/agent/permission/policy.ts b/packages/agent-core/src/agent/permission/policy.ts deleted file mode 100644 index 5baf777..0000000 --- a/packages/agent-core/src/agent/permission/policy.ts +++ /dev/null @@ -1,42 +0,0 @@ -import type { Agent } from '..'; -import type { PrepareToolExecutionResult, ToolExecutionHookContext } from '../../loop'; -import type { ToolInputDisplay } from '../../tools/display'; -import type { PermissionApprovalResultRecord, PermissionMode, PermissionRule } from './types'; - -export interface PermissionPolicyContext { - readonly agent: Agent; - readonly mode: PermissionMode; - readonly toolCallContext: ToolExecutionHookContext; - /** - * The rule matched by `checkPermission()`, if any. - * - * Policies that want to defer to user-defined rules (e.g. a default-allow - * policy that should not override an explicit `ask`/`deny` rule) inspect - * this to decide whether to fire. `undefined` means the decision came - * from the built-in default permission table rather than a user rule. - */ - readonly matchedRule: PermissionRule | undefined; - readonly recordApprovalResult: (record: PermissionApprovalResultRecord) => void; -} - -export type PermissionPolicyResult = - | { - readonly kind: 'allow'; - readonly executionMetadata?: unknown; - } - | { - readonly kind: 'result'; - readonly result: PrepareToolExecutionResult; - } - | { - readonly kind: 'ask'; - readonly action?: string | undefined; - readonly display?: ToolInputDisplay | undefined; - }; - -export interface PermissionPolicy { - readonly name: string; - evaluate( - context: PermissionPolicyContext, - ): PermissionPolicyResult | undefined | Promise; -} diff --git a/packages/agent-core/src/agent/permission/types.ts b/packages/agent-core/src/agent/permission/types.ts index df51e55..8e18ff0 100644 --- a/packages/agent-core/src/agent/permission/types.ts +++ b/packages/agent-core/src/agent/permission/types.ts @@ -1,11 +1,12 @@ +import type { PrepareToolExecutionResult, ResolvedToolExecutionHookContext } from '../../loop'; import type { ToolInputDisplay } from '../../tools/display'; export type PermissionRuleDecision = 'allow' | 'deny' | 'ask'; /** - * Rule provenance. `session-runtime` is the value used by the runtime - * "approve for session" path; `turn-override`, `project`, and `user` - * are reserved for static-loaded rules surfaced by external callers. + * Rule provenance. `session-runtime` stores rules produced by + * "approve for session"; `turn-override`, `project`, and `user` are + * reserved for static-loaded rules surfaced by external callers. */ export type PermissionRuleScope = 'turn-override' | 'session-runtime' | 'project' | 'user'; @@ -22,14 +23,14 @@ export type PermissionMode = 'manual' | 'yolo' | 'auto'; /** * A single permission rule. `pattern` is the DSL form (`Read(/etc/**)`, - * `Bash(rm *)`, or bare `Write`). See `parse-pattern.ts` for the parser - * and `matches-rule.ts` for the matcher. + * `Bash(rm *)`, or bare `Write`). Rule arguments are interpreted only by + * tools that provide a matcher; other tools match by name only. */ export interface PermissionRule { readonly decision: PermissionRuleDecision; readonly scope: PermissionRuleScope; readonly pattern: string; - readonly reason?: string | undefined; + readonly reason?: string; } export interface ApprovalRequest { @@ -51,6 +52,7 @@ export interface PermissionApprovalResultRecord { readonly toolCallId: string; readonly toolName: string; readonly action: string; + readonly sessionApprovalRule?: string; readonly result: ApprovalResponse; } @@ -58,3 +60,42 @@ export interface PermissionData { mode: PermissionMode; rules: PermissionRule[]; } + +export type PermissionDecision = 'approve' | 'deny' | 'ask'; + +export type PermissionReasonValue = string | number | boolean | null; + +export type PermissionDecisionReason = Readonly>; + +export type PermissionPolicyResolution = + | PermissionPolicyResult + | ({ readonly kind: 'result' } & PrepareToolExecutionResult); + +export interface PermissionPolicyContext extends ResolvedToolExecutionHookContext {} + +export type PermissionPolicyResult = + | { + readonly kind: 'approve'; + readonly reason?: PermissionDecisionReason; + readonly executionMetadata?: unknown; + } + | { + readonly kind: 'deny'; + readonly reason?: PermissionDecisionReason; + readonly message?: string; + } + | { + readonly kind: 'ask'; + readonly reason?: PermissionDecisionReason; + readonly resolveApproval?: ( + result: ApprovalResponse, + ) => PermissionPolicyResolution | undefined; + readonly resolveError?: (error: unknown) => PermissionPolicyResolution | undefined; + }; + +export interface PermissionPolicy { + readonly name: string; + evaluate( + context: PermissionPolicyContext, + ): PermissionPolicyResult | undefined | Promise; +} diff --git a/packages/agent-core/src/agent/records/migration/index.ts b/packages/agent-core/src/agent/records/migration/index.ts index 0c26911..d87a214 100644 --- a/packages/agent-core/src/agent/records/migration/index.ts +++ b/packages/agent-core/src/agent/records/migration/index.ts @@ -1,7 +1,8 @@ import { migrateV1_0ToV1_1 } from './v1.1'; +import { migrateV1_1ToV1_2 } from './v1.2'; // Wire protocol versions currently support only the `number.number` format. -export const AGENT_WIRE_PROTOCOL_VERSION = '1.1'; +export const AGENT_WIRE_PROTOCOL_VERSION = '1.2'; export interface WireMigrationRecord { readonly type: string; @@ -14,7 +15,7 @@ export interface WireMigration { migrateRecord(record: WireMigrationRecord): WireMigrationRecord; } -const MIGRATIONS: readonly WireMigration[] = [migrateV1_0ToV1_1]; +const MIGRATIONS: readonly WireMigration[] = [migrateV1_0ToV1_1, migrateV1_1ToV1_2]; export function isNewerWireVersion(readVersion: string): boolean { return compareWireVersions(readVersion, AGENT_WIRE_PROTOCOL_VERSION) > 0; diff --git a/packages/agent-core/src/agent/records/migration/v1.1.ts b/packages/agent-core/src/agent/records/migration/v1.1.ts index 20f3d97..05caeb0 100644 --- a/packages/agent-core/src/agent/records/migration/v1.1.ts +++ b/packages/agent-core/src/agent/records/migration/v1.1.ts @@ -6,22 +6,27 @@ import type { WireMigration, WireMigrationRecord } from './index'; * v1.1 flattens it to: * { name: 'xxx', arguments: 'yyy' } */ -interface LegacyToolCall { - type: 'function'; - id: string; - function: { - name?: string; - arguments?: string | null; - }; +interface V1_0ContextAppendMessageRecord extends WireMigrationRecord { + readonly type: 'context.append_message'; + readonly message: V1_0ContextMessage; } -function isLegacyToolCall(v: unknown): v is LegacyToolCall { - if (!isRecord(v)) return false; - return v['type'] === 'function' && typeof v['id'] === 'string' && isRecord(v['function']); +interface V1_0ContextMessage { + readonly toolCalls: readonly V1_0ToolCall[]; + readonly [key: string]: unknown; } -function migrateToolCall(v: LegacyToolCall): unknown { - const { function: fn, ...rest } = v; +interface V1_0ToolCall { + readonly type: 'function'; + readonly id: string; + readonly function: { + readonly name?: string; + readonly arguments?: string | null; + }; +} + +function migrateToolCall(toolCall: V1_0ToolCall): WireMigrationRecord { + const { function: fn, ...rest } = toolCall; return { ...rest, name: fn.name, @@ -29,34 +34,18 @@ function migrateToolCall(v: LegacyToolCall): unknown { }; } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - export const migrateV1_0ToV1_1: WireMigration = { sourceVersion: '1.0', targetVersion: '1.1', migrateRecord(record: WireMigrationRecord): WireMigrationRecord { if (record.type !== 'context.append_message') return record; - - const message = record['message'] as { - readonly toolCalls: readonly unknown[]; - }; - - let changed = false; - const toolCalls = message.toolCalls.map((toolCall) => { - if (!isLegacyToolCall(toolCall)) return toolCall; - changed = true; - return migrateToolCall(toolCall); - }); - - if (!changed) return record; + const appendMessageRecord = record as V1_0ContextAppendMessageRecord; return { - ...record, + ...appendMessageRecord, message: { - ...message, - toolCalls, + ...appendMessageRecord.message, + toolCalls: appendMessageRecord.message.toolCalls.map(migrateToolCall), }, }; }, diff --git a/packages/agent-core/src/agent/records/migration/v1.2.ts b/packages/agent-core/src/agent/records/migration/v1.2.ts new file mode 100644 index 0000000..5d4f7af --- /dev/null +++ b/packages/agent-core/src/agent/records/migration/v1.2.ts @@ -0,0 +1,63 @@ +import type { WireMigration, WireMigrationRecord } from './index'; + +interface V1_1ApprovalResult { + readonly decision: 'approved' | 'rejected' | 'cancelled'; + readonly scope?: 'session'; + readonly feedback?: string; + readonly selectedLabel?: string; +} + +interface V1_1ApprovalResultRecord extends WireMigrationRecord { + readonly type: 'permission.record_approval_result'; + readonly turnId: number; + readonly toolCallId: string; + readonly toolName: string; + readonly action: string; + readonly sessionApprovalRule?: string; + readonly result: V1_1ApprovalResult; +} + +const LEGACY_SESSION_APPROVAL_ACTION_TO_PATTERN: Readonly> = { + 'run command': 'Bash', + 'stop background task': 'TaskStop', + 'edit file': 'Write', + 'edit file outside of working directory': 'Write', + 'write file': 'Write', +}; + +// v1.1 cached these action labels directly but did not have enough stable data +// to reconstruct an equivalent v1.2 rule. Migrating to broad `Bash` would +// expand the approval, and there is no safe `Bash(...)` subject to recover — +// in particular, `run background command` would need to encode +// `run_in_background=true`, which `Bash`'s `matchesRule` cannot express. +const LEGACY_SESSION_APPROVAL_UNRESTORABLE_ACTIONS = new Set([ + 'run command in plan mode', + 'run background command', +]); + +export const migrateV1_1ToV1_2: WireMigration = { + sourceVersion: '1.1', + targetVersion: '1.2', + migrateRecord(record: WireMigrationRecord): WireMigrationRecord { + if (record.type !== 'permission.record_approval_result') return record; + const approvalRecord = record as V1_1ApprovalResultRecord; + if ( + approvalRecord.result.decision !== 'approved' || + approvalRecord.result.scope !== 'session' + ) { + return record; + } + if (approvalRecord.sessionApprovalRule !== undefined) return record; + + const pattern = LEGACY_SESSION_APPROVAL_UNRESTORABLE_ACTIONS.has(approvalRecord.action) + ? undefined + : LEGACY_SESSION_APPROVAL_ACTION_TO_PATTERN[approvalRecord.action] ?? + approvalRecord.toolName; + if (pattern === undefined) return record; + + return { + ...record, + sessionApprovalRule: pattern, + }; + }, +}; diff --git a/packages/agent-core/src/agent/tool/index.ts b/packages/agent-core/src/agent/tool/index.ts index 614b8ff..97df803 100644 --- a/packages/agent-core/src/agent/tool/index.ts +++ b/packages/agent-core/src/agent/tool/index.ts @@ -1,8 +1,8 @@ import { uniq } from '@antfu/utils'; import type { ChatProvider, Tool } from '@moonshot-ai/kosong'; +import picomatch from 'picomatch'; import type { Agent } from '..'; -import { globMatch } from '../permission/path-glob-match'; import { makeErrorPayload } from '../../errors'; import type { ExecutableTool } from '../../loop'; import { createMcpAuthTool } from '../../mcp/auth-tool'; @@ -92,6 +92,7 @@ export class ToolManager { parameters, resolveExecution: (args) => { return { + approvalRule: name, execute: async (context) => { return this.agent.rpc.toolCall( { @@ -156,6 +157,7 @@ export class ToolManager { parameters: tool.parameters, resolveExecution: (args) => { return { + approvalRule: qualified, execute: async (context) => { // `args` has already been JSON-parsed and schema-validated by // the loop's preflight (`loop/tool-call.ts`), so the MCP @@ -298,7 +300,7 @@ export class ToolManager { } private isMcpToolEnabled(name: string): boolean { - return this.mcpAccessPatterns.some((pattern) => globMatch(name, pattern)); + return this.mcpAccessPatterns.some((pattern) => picomatch.isMatch(name, pattern)); } *toolInfos(): Iterable { diff --git a/packages/agent-core/src/agent/turn/index.ts b/packages/agent-core/src/agent/turn/index.ts index d8756a7..67787e8 100644 --- a/packages/agent-core/src/agent/turn/index.ts +++ b/packages/agent-core/src/agent/turn/index.ts @@ -433,29 +433,10 @@ export class TurnFlow { ctx.args, ); if (cached !== null) return { syntheticResult: cached }; - const hookResult = await this.agent.hooks?.triggerBlock('PreToolUse', { - matcherValue: ctx.toolCall.name, - signal: ctx.signal, - inputData: { - toolName: ctx.toolCall.name, - toolInput: toolInputRecord(ctx.args), - toolCallId: ctx.toolCall.id, - }, - }); - ctx.signal.throwIfAborted(); - if (hookResult) { - this.agent.telemetry.track('hook_triggered', { - event_type: 'PreToolUse', - action: hookResult.block ? 'block' : 'allow', - }); - return hookResult; - } - const permissionResult = await this.agent.permission.beforeToolCall(ctx); - this.agent.telemetry.track('hook_triggered', { - event_type: 'PreToolUse', - action: permissionResult?.block === true ? 'block' : 'allow', - }); - return permissionResult; + return undefined; + }, + authorizeToolExecution: async (ctx) => { + return this.agent.permission.beforeToolCall(ctx); }, finalizeToolResult: async (ctx) => { // Resolve dedup BEFORE firing the PostToolUse hook so same-step diff --git a/packages/agent-core/src/config/schema.ts b/packages/agent-core/src/config/schema.ts index ea6a653..cd3f33b 100644 --- a/packages/agent-core/src/config/schema.ts +++ b/packages/agent-core/src/config/schema.ts @@ -1,5 +1,5 @@ import { HOOK_EVENT_TYPES } from '#/agent/hooks/types'; -import { parsePattern } from '#/agent/permission/parse-pattern'; +import { parsePattern } from '#/agent/permission/matches-rule'; import { ErrorCodes, KimiError } from '#/errors'; import { z } from 'zod'; diff --git a/packages/agent-core/src/loop/index.ts b/packages/agent-core/src/loop/index.ts index c0071df..7f7b1ef 100644 --- a/packages/agent-core/src/loop/index.ts +++ b/packages/agent-core/src/loop/index.ts @@ -25,7 +25,9 @@ export type { ToolCall, ExecutableToolContext, ToolExecutionHookContext, + ResolvedToolExecutionHookContext, PrepareToolExecutionHook, + AuthorizeToolExecutionHook, PrepareToolExecutionResult, ExecutableToolResult, FinalizeToolResultContext, diff --git a/packages/agent-core/src/loop/tool-access.ts b/packages/agent-core/src/loop/tool-access.ts index f14eecb..9ba40d4 100644 --- a/packages/agent-core/src/loop/tool-access.ts +++ b/packages/agent-core/src/loop/tool-access.ts @@ -1,25 +1,22 @@ export type ToolFileAccessOperation = 'read' | 'write' | 'readwrite' | 'search'; -export type ToolResourceAccess = - | { - readonly kind: 'file'; - readonly operation: ToolFileAccessOperation; - /** - * Canonical file path when the access can be narrowed. Omitted means the - * operation may touch any file in the backend file namespace. - */ - readonly path?: string; - readonly recursive?: boolean; - } - | { - /** - * Arbitrary side effects or resources that cannot be represented as a - * file access. This is intentionally operation-less and globally - * exclusive for concurrency. - */ - readonly kind: 'all'; - }; +export interface ToolFileAccess { + readonly kind: 'file'; + readonly operation: ToolFileAccessOperation; + readonly path: string; + readonly recursive?: boolean; +} + +export interface ToolResourceAccessAll { + /** + * Arbitrary side effects or resources that cannot be represented as a + * file access. This is intentionally operation-less and globally + * exclusive for concurrency. + */ + readonly kind: 'all'; +} +export type ToolResourceAccess = ToolFileAccess | ToolResourceAccessAll; export type ToolAccesses = readonly ToolResourceAccess[]; export const ToolAccesses = { @@ -33,18 +30,10 @@ export const ToolAccesses = { file( operation: ToolFileAccessOperation, - path?: string, + path: string, options: { readonly recursive?: boolean } = {}, ): ToolAccesses { - const access: { - kind: 'file'; - operation: ToolFileAccessOperation; - path?: string; - recursive?: boolean; - } = { kind: 'file', operation }; - if (path !== undefined) access.path = path; - if (options.recursive !== undefined) access.recursive = options.recursive; - return [access]; + return [{ kind: 'file', operation, path, recursive: options.recursive }]; }, readFile(path: string): ToolAccesses { @@ -55,10 +44,6 @@ export const ToolAccesses = { return ToolAccesses.file('read', path, { recursive: true }); }, - readAnyFile(): ToolAccesses { - return ToolAccesses.file('read'); - }, - writeFile(path: string): ToolAccesses { return ToolAccesses.file('write', path); }, @@ -67,10 +52,6 @@ export const ToolAccesses = { return ToolAccesses.file('write', path, { recursive: true }); }, - writeAnyFile(): ToolAccesses { - return ToolAccesses.file('write'); - }, - readWriteFile(path: string): ToolAccesses { return ToolAccesses.file('readwrite', path); }, @@ -79,18 +60,10 @@ export const ToolAccesses = { return ToolAccesses.file('readwrite', path, { recursive: true }); }, - readWriteAnyFile(): ToolAccesses { - return ToolAccesses.file('readwrite'); - }, - searchTree(path: string): ToolAccesses { return ToolAccesses.file('search', path, { recursive: true }); }, - searchAnyFile(): ToolAccesses { - return ToolAccesses.file('search'); - }, - conflict(left: ToolAccesses, right: ToolAccesses): boolean { return left.some((leftAccess) => right.some((rightAccess) => resourceAccessesConflict(leftAccess, rightAccess)), @@ -122,12 +95,7 @@ function fileOperationWrites(operation: ToolFileAccessOperation): boolean { } } -function fileAccessesOverlap( - left: Extract, - right: Extract, -): boolean { - if (left.path === undefined || right.path === undefined) return true; - +function fileAccessesOverlap(left: ToolFileAccess, right: ToolFileAccess): boolean { const leftPath = normalizePath(left.path); const rightPath = normalizePath(right.path); if (leftPath === rightPath) return true; diff --git a/packages/agent-core/src/loop/tool-call.ts b/packages/agent-core/src/loop/tool-call.ts index 617b1c6..c86a570 100644 --- a/packages/agent-core/src/loop/tool-call.ts +++ b/packages/agent-core/src/loop/tool-call.ts @@ -30,6 +30,7 @@ import type { LLM, LLMChatResponse } from './llm'; import { ToolAccesses } from './tool-access'; import { ToolScheduler, type ToolCallTask } from './tool-scheduler'; import type { + AuthorizeToolExecutionResult, ExecutableTool, LoopHooks, ToolCall, @@ -223,46 +224,50 @@ async function prepareToolCall( step: ToolCallStepContext, call: PreflightedToolCall, ): Promise { - if (call.kind === 'rejected') { - await dispatchToolCall(step, call, call.args); - return { task: makeResolvedToolCallTask(makeErrorToolResult(call, call.args, call.output)) }; - } + const settleError = async ( + args: unknown, + output: string, + displayFields?: ToolCallDisplayFields, + ): Promise => { + await dispatchToolCall(step, call, args, displayFields); + return { task: makeResolvedToolCallTask(makeErrorToolResult(call, args, output)) }; + }; - const decision = await runPrepareToolExecutionHook(step, call); - if (decision.kind === 'blocked') { - await dispatchToolCall(step, call, decision.args); + const settleSynthetic = async ( + args: unknown, + result: ExecutableToolResult, + displayFields?: ToolCallDisplayFields, + ): Promise => { + const coerced = coerceToolResult(result, call.toolName); + await dispatchToolCall(step, call, args, displayFields); return { - task: makeResolvedToolCallTask(makeErrorToolResult(call, decision.args, decision.output)), + task: makeResolvedToolCallTask(makeToolResult(call, args, coerced)), + stopBatchAfterThis: toolResultStopsTurn(coerced), }; - } + }; - if (decision.kind === 'hookFailed') { - await dispatchToolCall(step, call, decision.args); - return { - task: makeResolvedToolCallTask(makeErrorToolResult(call, decision.args, decision.output)), - }; - } + if (call.kind === 'rejected') return settleError(call.args, call.output); + const decision = await runPrepareToolExecutionHook(step, call); + if (decision.kind === 'blocked' || decision.kind === 'hookFailed') { + return settleError(decision.args, decision.output); + } if (decision.kind === 'synthetic') { - await dispatchToolCall(step, call, decision.args); - const coerced = coerceToolResult(decision.result, call.toolName); - return { - task: makeResolvedToolCallTask(makeToolResult(call, decision.args, coerced)), - stopBatchAfterThis: toolResultStopsTurn(coerced), - }; + return settleSynthetic(decision.args, decision.result); } const validationError = validateExecutableToolArgs(call.tool, decision.args); if (validationError !== null) { - await dispatchToolCall(step, call, decision.args); - const output = `Invalid args for tool "${call.toolName}" after prepareToolExecution hook: ${validationError}`; - return { task: makeResolvedToolCallTask(makeErrorToolResult(call, decision.args, output)) }; + return settleError( + decision.args, + `Invalid args for tool "${call.toolName}" after prepareToolExecution hook: ${validationError}`, + ); } const effectiveArgs = decision.args; let execution: ToolExecution; try { - execution = call.tool.resolveExecution(effectiveArgs); + execution = await call.tool.resolveExecution(effectiveArgs); } catch (error) { if (!(error instanceof PathSecurityError)) { step.log?.warn('tool execution setup failed', { @@ -275,35 +280,41 @@ async function prepareToolCall( error instanceof PathSecurityError ? error.message : `Tool "${call.toolName}" failed to resolve execution: ${errorMessage(error)}`; - await dispatchToolCall(step, call, effectiveArgs); - return { - task: makeResolvedToolCallTask(makeErrorToolResult(call, effectiveArgs, output)), - }; + return settleError(effectiveArgs, output); } const displayFields = toolCallDisplayFieldsFromExecution(execution); - await dispatchToolCall(step, call, effectiveArgs, displayFields); + const settleAborted = (): Promise => + settleError(effectiveArgs, `Tool "${call.toolName}" was aborted`, displayFields); - if (step.signal.aborted) { - return { - task: makeResolvedToolCallTask( - makeErrorToolResult(call, effectiveArgs, `Tool "${call.toolName}" was aborted`), - ), - }; - } + if (step.signal.aborted) return settleAborted(); if (execution.isError === true) { - return { - task: makeResolvedToolCallTask(makeToolResult(call, effectiveArgs, execution)), - stopBatchAfterThis: execution.stopTurn, - }; + return settleSynthetic(effectiveArgs, execution, displayFields); + } + + const authorization = await runAuthorizeToolExecutionHook(step, call, effectiveArgs, execution); + if (step.signal.aborted) return settleAborted(); + + if (authorization?.block === true) { + return settleError( + effectiveArgs, + authorization.reason ?? `Tool call "${call.toolName}" was blocked`, + displayFields, + ); } + if (authorization?.syntheticResult !== undefined) { + return settleSynthetic(effectiveArgs, authorization.syntheticResult, displayFields); + } + + const executionMetadata = authorization?.executionMetadata ?? decision.metadata; + await dispatchToolCall(step, call, effectiveArgs, displayFields); return { task: { accesses: execution.accesses ?? ToolAccesses.all(), start: async () => ({ - result: runRunnableToolCall(step, call, effectiveArgs, decision.metadata, execution), + result: runRunnableToolCall(step, call, effectiveArgs, executionMetadata, execution), }), }, }; @@ -384,6 +395,40 @@ async function runPrepareToolExecutionHook( return { kind: 'allowed', args: effectiveArgs, metadata: hookResult?.executionMetadata }; } +async function runAuthorizeToolExecutionHook( + step: ToolCallStepContext, + call: RunnableToolCall, + args: unknown, + execution: RunnableToolExecution, +): Promise { + const { hooks, signal, turnId, currentStep, llm } = step; + if (hooks?.authorizeToolExecution === undefined) return undefined; + + try { + return await hooks.authorizeToolExecution({ + toolCall: call.toolCall, + tool: call.tool, + args, + execution, + turnId, + stepNumber: currentStep, + signal, + llm, + }); + } catch (error) { + if (isAbortError(error) || signal.aborted) { + return { + block: true, + reason: `Tool "${call.toolName}" was aborted during authorizeToolExecution hook`, + }; + } + return { + block: true, + reason: `authorizeToolExecution hook failed for "${call.toolName}": ${errorMessage(error)}`, + }; + } +} + function toolCallDisplayFieldsFromExecution( execution: ToolExecution, ): ToolCallDisplayFields | undefined { diff --git a/packages/agent-core/src/loop/types.ts b/packages/agent-core/src/loop/types.ts index 433435c..e106ed3 100644 --- a/packages/agent-core/src/loop/types.ts +++ b/packages/agent-core/src/loop/types.ts @@ -115,13 +115,15 @@ export interface RunnableToolExecution { readonly accesses?: ToolAccesses | undefined; readonly display?: ToolInputDisplay | undefined; readonly description?: string; + readonly approvalRule: string; + readonly matchesRule?: ((ruleArgs: string) => boolean) | undefined; readonly execute: (ctx: ExecutableToolContext) => Promise; } export type ToolExecution = RunnableToolExecution | ExecutableToolErrorResult; export interface ExecutableTool extends Tool { - resolveExecution(input: Input): ToolExecution; + resolveExecution(input: Input): ToolExecution | Promise; } /** @@ -142,14 +144,21 @@ export interface ToolExecutionHookContext extends LoopStepHookContext { readonly args: unknown; } -export interface PrepareToolExecutionResult { +export interface ResolvedToolExecutionHookContext extends ToolExecutionHookContext { + readonly execution: RunnableToolExecution; +} + +export interface AuthorizeToolExecutionResult { readonly block?: boolean | undefined; readonly reason?: string | undefined; - readonly updatedArgs?: unknown; readonly syntheticResult?: ExecutableToolResult | undefined; readonly executionMetadata?: unknown; } +export interface PrepareToolExecutionResult extends AuthorizeToolExecutionResult { + readonly updatedArgs?: unknown; +} + export interface FinalizeToolResultContext extends ToolExecutionHookContext { readonly result: ExecutableToolResult; } @@ -181,6 +190,10 @@ export type PrepareToolExecutionHook = ( ctx: ToolExecutionHookContext, ) => Promise; +export type AuthorizeToolExecutionHook = ( + ctx: ResolvedToolExecutionHookContext, +) => Promise; + export type FinalizeToolResultHook = ( ctx: FinalizeToolResultContext, ) => Promise; @@ -203,6 +216,7 @@ export interface LoopHooks { beforeStep?: BeforeStepHook | undefined; afterStep?: AfterStepHook | undefined; prepareToolExecution?: PrepareToolExecutionHook | undefined; + authorizeToolExecution?: AuthorizeToolExecutionHook | undefined; finalizeToolResult?: FinalizeToolResultHook | undefined; shouldContinueAfterStop?: ShouldContinueAfterStopHook | undefined; } diff --git a/packages/agent-core/src/mcp/auth-tool.ts b/packages/agent-core/src/mcp/auth-tool.ts index 37bd736..3dd64c1 100644 --- a/packages/agent-core/src/mcp/auth-tool.ts +++ b/packages/agent-core/src/mcp/auth-tool.ts @@ -158,6 +158,7 @@ export function createMcpAuthTool(options: CreateMcpAuthToolOptions): Executable resolveExecution: () => { return { description: `Authenticating ${serverName}`, + approvalRule: name, execute, }; }, diff --git a/packages/agent-core/src/telemetry.ts b/packages/agent-core/src/telemetry.ts index e1c9c3a..6f066fb 100644 --- a/packages/agent-core/src/telemetry.ts +++ b/packages/agent-core/src/telemetry.ts @@ -1,4 +1,4 @@ -export type TelemetryPropertyValue = boolean | number | string | null; +export type TelemetryPropertyValue = boolean | number | string | undefined | null; export type TelemetryProperties = Readonly>; diff --git a/packages/agent-core/src/tools/background/task-list.ts b/packages/agent-core/src/tools/background/task-list.ts index 1a8c5cc..6c56c84 100644 --- a/packages/agent-core/src/tools/background/task-list.ts +++ b/packages/agent-core/src/tools/background/task-list.ts @@ -7,6 +7,7 @@ import { z } from 'zod'; import type { BuiltinTool } from '../../agent/tool'; import type { ToolExecution } from '../../loop/types'; import { toInputJsonSchema } from '../support/input-schema'; +import { matchesGlobRuleSubject } from '../support/rule-match'; import type { BackgroundProcessManager, BackgroundTaskInfo } from './manager'; import { isBackgroundTaskTerminal } from './manager'; import TASK_LIST_DESCRIPTION from './task-list.md'; @@ -67,8 +68,11 @@ export class TaskListTool implements BuiltinTool { constructor(private readonly manager: BackgroundProcessManager) {} resolveExecution(args: TaskListInput): ToolExecution { + const listScope = (args.active_only ?? true) ? 'active' : 'all'; return { description: 'Listing background tasks', + approvalRule: this.name, + matchesRule: (ruleArgs) => matchesGlobRuleSubject(ruleArgs, listScope), execute: async () => { await this.manager.settlePendingExits(); const activeOnly = args.active_only ?? true; diff --git a/packages/agent-core/src/tools/background/task-output.ts b/packages/agent-core/src/tools/background/task-output.ts index f77de21..fe912e7 100644 --- a/packages/agent-core/src/tools/background/task-output.ts +++ b/packages/agent-core/src/tools/background/task-output.ts @@ -22,6 +22,7 @@ import { type BackgroundTaskStatus, } from './manager'; import { toInputJsonSchema } from '../support/input-schema'; +import { matchesGlobRuleSubject } from '../support/rule-match'; import TASK_OUTPUT_DESCRIPTION from './task-output.md'; /** @@ -75,6 +76,8 @@ export class TaskOutputTool implements BuiltinTool { resolveExecution(args: TaskOutputInput): ToolExecution { return { description: `Reading output of task ${args.task_id}`, + approvalRule: this.name, + matchesRule: (ruleArgs) => matchesGlobRuleSubject(ruleArgs, args.task_id), execute: () => this.execute(args), }; } diff --git a/packages/agent-core/src/tools/background/task-stop.ts b/packages/agent-core/src/tools/background/task-stop.ts index 20b93fa..c115da7 100644 --- a/packages/agent-core/src/tools/background/task-stop.ts +++ b/packages/agent-core/src/tools/background/task-stop.ts @@ -7,6 +7,7 @@ import { z } from 'zod'; import type { BuiltinTool } from '../../agent/tool'; import type { ToolExecution } from '../../loop/types'; import { toInputJsonSchema } from '../support/input-schema'; +import { matchesGlobRuleSubject } from '../support/rule-match'; import { isBackgroundTaskTerminal, type BackgroundProcessManager } from './manager'; import TASK_STOP_DESCRIPTION from './task-stop.md'; @@ -35,6 +36,8 @@ export class TaskStopTool implements BuiltinTool { resolveExecution(args: TaskStopInput): ToolExecution { return { description: `Stopping task ${args.task_id}`, + approvalRule: this.name, + matchesRule: (ruleArgs) => matchesGlobRuleSubject(ruleArgs, args.task_id), execute: async () => { await this.manager.settlePendingExits(); const info = this.manager.getTask(args.task_id); diff --git a/packages/agent-core/src/tools/builtin/collaboration/agent.ts b/packages/agent-core/src/tools/builtin/collaboration/agent.ts index 062a1ff..a65ed24 100644 --- a/packages/agent-core/src/tools/builtin/collaboration/agent.ts +++ b/packages/agent-core/src/tools/builtin/collaboration/agent.ts @@ -27,6 +27,7 @@ import type { SessionSubagentHost, SubagentHandle } from '../../../session/subag import { createDeadlineAbortSignal, type DeadlineAbortSignal } from '../../../utils/abort'; import type { BackgroundProcessManager } from '../../background/manager'; import { toInputJsonSchema } from '../../support/input-schema'; +import { matchesGlobRuleSubject } from '../../support/rule-match'; import AGENT_BACKGROUND_DISABLED_DESCRIPTION from './agent-background-disabled.md'; import AGENT_BACKGROUND_DESCRIPTION from './agent-background-enabled.md'; import AGENT_DESCRIPTION_BASE from './agent.md'; @@ -144,6 +145,14 @@ export class AgentTool implements BuiltinTool { return { description: `${prefix} ${profileName} agent: ${args.description}`, accesses: ToolAccesses.none(), + display: { + kind: 'agent_call', + agent_name: profileName, + prompt: args.prompt, + background: args.run_in_background, + }, + approvalRule: this.name, + matchesRule: (ruleArgs) => matchesGlobRuleSubject(ruleArgs, profileName), execute: (ctx) => this.execution(args, ctx), }; } diff --git a/packages/agent-core/src/tools/builtin/collaboration/ask-user.ts b/packages/agent-core/src/tools/builtin/collaboration/ask-user.ts index a1dbf61..b06bf03 100644 --- a/packages/agent-core/src/tools/builtin/collaboration/ask-user.ts +++ b/packages/agent-core/src/tools/builtin/collaboration/ask-user.ts @@ -90,6 +90,7 @@ export class AskUserQuestionTool implements BuiltinTool { resolveExecution(args: AskUserQuestionInput): ToolExecution { return { description: 'Asking user questions', + approvalRule: this.name, execute: (ctx) => this.execution(args, ctx), }; } diff --git a/packages/agent-core/src/tools/builtin/collaboration/skill-tool.ts b/packages/agent-core/src/tools/builtin/collaboration/skill-tool.ts index 1e8ec8d..b7350e3 100644 --- a/packages/agent-core/src/tools/builtin/collaboration/skill-tool.ts +++ b/packages/agent-core/src/tools/builtin/collaboration/skill-tool.ts @@ -22,6 +22,7 @@ import type { ExecutableToolResult, ToolExecution } from '../../../loop/types'; import { isInlineSkillType, type SkillDefinition } from '../../../skill'; import { renderPrompt } from '../../../utils/render-prompt'; import { toInputJsonSchema } from '../../support/input-schema'; +import { matchesGlobRuleSubject } from '../../support/rule-match'; import skillDescriptionTemplate from './skill-tool.md'; export const MAX_SKILL_QUERY_DEPTH = 3; @@ -78,6 +79,9 @@ export class SkillTool implements BuiltinTool { resolveExecution(args: SkillToolInput): ToolExecution { return { description: `Invoke skill ${args.skill}`, + display: { kind: 'skill_call', skill_name: args.skill, args: args.args }, + approvalRule: this.name, + matchesRule: (ruleArgs) => matchesGlobRuleSubject(ruleArgs, args.skill), execute: () => this.execution(args), }; } diff --git a/packages/agent-core/src/tools/builtin/file/edit.ts b/packages/agent-core/src/tools/builtin/file/edit.ts index de14c34..0142fbe 100644 --- a/packages/agent-core/src/tools/builtin/file/edit.ts +++ b/packages/agent-core/src/tools/builtin/file/edit.ts @@ -16,6 +16,7 @@ import { ToolAccesses } from '../../../loop/tool-access'; import type { ExecutableToolResult, ToolExecution } from '../../../loop/types'; import { resolvePathAccessPath } from '../../policies/path-access'; import { toInputJsonSchema } from '../../support/input-schema'; +import { literalRulePattern, matchesPathRuleSubject } from '../../support/rule-match'; import type { WorkspaceConfig } from '../../support/workspace'; import { materializeModelText, toModelTextView } from './line-endings'; import EDIT_DESCRIPTION from './edit.md'; @@ -73,6 +74,14 @@ export class EditTool implements BuiltinTool { return { accesses: ToolAccesses.readWriteFile(path), description: `Editing ${args.path}`, + display: { kind: 'file_io', operation: 'edit', path }, + approvalRule: literalRulePattern(this.name, path), + matchesRule: (ruleArgs) => + matchesPathRuleSubject(ruleArgs, path, { + cwd: this.workspace.workspaceDir, + pathClass: this.kaos.pathClass(), + homeDir: this.kaos.gethome(), + }), execute: () => this.execution(args, path), }; } diff --git a/packages/agent-core/src/tools/builtin/file/glob.ts b/packages/agent-core/src/tools/builtin/file/glob.ts index d58e37c..ac7cb15 100644 --- a/packages/agent-core/src/tools/builtin/file/glob.ts +++ b/packages/agent-core/src/tools/builtin/file/glob.ts @@ -38,6 +38,7 @@ import { resolvePathAccessPath } from '../../policies/path-access'; import type { PathClass } from '../../policies/path-access'; import { toInputJsonSchema } from '../../support/input-schema'; import { listDirectory } from '../../support/list-directory'; +import { literalRulePattern, matchesGlobRuleSubject } from '../../support/rule-match'; import type { WorkspaceConfig } from '../../support/workspace'; import GLOB_DESCRIPTION from './glob.md'; @@ -117,6 +118,9 @@ export class GlobTool implements BuiltinTool { return { accesses: ToolAccesses.searchTree(searchRoots[0]!), description: `Searching ${args.pattern}`, + display: { kind: 'file_io', operation: 'glob', path: searchRoots[0]! }, + approvalRule: literalRulePattern(this.name, args.pattern), + matchesRule: (ruleArgs) => matchesGlobRuleSubject(ruleArgs, args.pattern), execute: () => this.execution(args, searchRoots), }; } diff --git a/packages/agent-core/src/tools/builtin/file/grep.ts b/packages/agent-core/src/tools/builtin/file/grep.ts index 00dbcf2..03c3e48 100644 --- a/packages/agent-core/src/tools/builtin/file/grep.ts +++ b/packages/agent-core/src/tools/builtin/file/grep.ts @@ -31,6 +31,7 @@ import type { PathClass } from '../../policies/path-access'; import { isSensitiveFile, SENSITIVE_DOT_VARIANT_SUFFIXES } from '../../policies/sensitive'; import { toInputJsonSchema } from '../../support/input-schema'; import { ensureRgPath, rgUnavailableMessage } from '../../support/rg-locator'; +import { literalRulePattern, matchesGlobRuleSubject } from '../../support/rule-match'; import { ToolResultBuilder } from '../../support/result-builder'; import type { WorkspaceConfig } from '../../support/workspace'; import GREP_DESCRIPTION from './grep.md'; @@ -190,6 +191,9 @@ export class GrepTool implements BuiltinTool { return { accesses: ToolAccesses.searchTree(searchPaths[0]!), description: `Searching for '${args.pattern}' in ${searchPath}`, + display: { kind: 'file_io', operation: 'grep', path: searchPaths[0]! }, + approvalRule: literalRulePattern(this.name, args.pattern), + matchesRule: (ruleArgs) => matchesGlobRuleSubject(ruleArgs, args.pattern), execute: ({ signal }) => this.execution(args, signal, searchPaths), }; } diff --git a/packages/agent-core/src/tools/builtin/file/read-media.ts b/packages/agent-core/src/tools/builtin/file/read-media.ts index b486d74..3b206d2 100644 --- a/packages/agent-core/src/tools/builtin/file/read-media.ts +++ b/packages/agent-core/src/tools/builtin/file/read-media.ts @@ -31,6 +31,7 @@ import { renderPrompt } from '../../../utils/render-prompt'; import { resolvePathAccessPath } from '../../policies/path-access'; import { MEDIA_SNIFF_BYTES, detectFileType, sniffImageDimensions } from '../../support/file-type'; import { toInputJsonSchema } from '../../support/input-schema'; +import { literalRulePattern, matchesPathRuleSubject } from '../../support/rule-match'; import type { WorkspaceConfig } from '../../support/workspace'; import readMediaDescriptionHead from './read-media.md'; @@ -149,6 +150,14 @@ export class ReadMediaFileTool implements BuiltinTool { return { accesses: ToolAccesses.readFile(path), description: `Reading media: ${args.path}`, + display: { kind: 'file_io', operation: 'read', path }, + approvalRule: literalRulePattern(this.name, path), + matchesRule: (ruleArgs) => + matchesPathRuleSubject(ruleArgs, path, { + cwd: this.workspace.workspaceDir, + pathClass: this.kaos.pathClass(), + homeDir: this.kaos.gethome(), + }), execute: () => this.execution(args, path), }; } diff --git a/packages/agent-core/src/tools/builtin/file/read.ts b/packages/agent-core/src/tools/builtin/file/read.ts index 2e27b06..5c71100 100644 --- a/packages/agent-core/src/tools/builtin/file/read.ts +++ b/packages/agent-core/src/tools/builtin/file/read.ts @@ -8,6 +8,7 @@ import { renderPrompt } from '../../../utils/render-prompt'; import { resolvePathAccessPath } from '../../policies/path-access'; import { MEDIA_SNIFF_BYTES, detectFileType } from '../../support/file-type'; import { toInputJsonSchema } from '../../support/input-schema'; +import { literalRulePattern, matchesPathRuleSubject } from '../../support/rule-match'; import type { WorkspaceConfig } from '../../support/workspace'; import { makeCarriageReturnsVisible, type LineEndingStyle } from './line-endings'; import readDescriptionTemplate from './read.md'; @@ -183,6 +184,14 @@ export class ReadTool implements BuiltinTool { return { accesses: ToolAccesses.readFile(path), description: `Reading ${args.path}`, + display: { kind: 'file_io', operation: 'read', path }, + approvalRule: literalRulePattern(this.name, path), + matchesRule: (ruleArgs) => + matchesPathRuleSubject(ruleArgs, path, { + cwd: this.workspace.workspaceDir, + pathClass: this.kaos.pathClass(), + homeDir: this.kaos.gethome(), + }), execute: () => this.execution(args, path), }; } diff --git a/packages/agent-core/src/tools/builtin/file/write.ts b/packages/agent-core/src/tools/builtin/file/write.ts index 5939800..812e9a6 100644 --- a/packages/agent-core/src/tools/builtin/file/write.ts +++ b/packages/agent-core/src/tools/builtin/file/write.ts @@ -18,6 +18,7 @@ import { resolvePathAccessPath, } from '../../policies/path-access'; import { toInputJsonSchema } from '../../support/input-schema'; +import { literalRulePattern, matchesPathRuleSubject } from '../../support/rule-match'; import type { WorkspaceConfig } from '../../support/workspace'; import WRITE_DESCRIPTION from './write.md'; @@ -76,6 +77,14 @@ export class WriteTool implements BuiltinTool { return { accesses: ToolAccesses.writeFile(path), description: `Writing ${args.path}`, + display: { kind: 'file_io', operation: 'write', path }, + approvalRule: literalRulePattern(this.name, path), + matchesRule: (ruleArgs) => + matchesPathRuleSubject(ruleArgs, path, { + cwd: this.workspace.workspaceDir, + pathClass: this.kaos.pathClass(), + homeDir: this.kaos.gethome(), + }), execute: () => this.execution(args, path), }; } diff --git a/packages/agent-core/src/tools/builtin/planning/enter-plan-mode.ts b/packages/agent-core/src/tools/builtin/planning/enter-plan-mode.ts index b15b78c..cf7d43f 100644 --- a/packages/agent-core/src/tools/builtin/planning/enter-plan-mode.ts +++ b/packages/agent-core/src/tools/builtin/planning/enter-plan-mode.ts @@ -29,6 +29,7 @@ export class EnterPlanModeTool implements BuiltinTool { resolveExecution(_args: EnterPlanModeInput): ToolExecution { return { description: 'Requesting to enter plan mode', + approvalRule: this.name, execute: async () => { // Guard: already in plan mode if (this.agent.planMode.isActive) { diff --git a/packages/agent-core/src/tools/builtin/planning/exit-plan-mode.ts b/packages/agent-core/src/tools/builtin/planning/exit-plan-mode.ts index 32a0983..18f6b38 100644 --- a/packages/agent-core/src/tools/builtin/planning/exit-plan-mode.ts +++ b/packages/agent-core/src/tools/builtin/planning/exit-plan-mode.ts @@ -3,16 +3,16 @@ * * The LLM calls this tool to surface a finalised plan to the user and * exit plan mode. The plan must already be written to the current plan - * file; this tool reads that file and flips plan mode off. PermissionManager - * handles plan approval before this tool runs and passes any selected option - * through execution metadata. + * file; this tool reads that file and flips plan mode off. */ import type { Agent } from '#/agent'; +import type { PlanData } from '#/agent/plan'; import { z } from 'zod'; import type { BuiltinTool } from '../../../agent/tool'; -import type { ExecutableToolContext, ExecutableToolResult, ToolExecution } from '../../../loop/types'; +import type { ExecutableToolResult, ToolExecution } from '../../../loop/types'; +import type { ToolInputDisplay } from '../../display'; import { toInputJsonSchema } from '../../support/input-schema'; import DESCRIPTION from './exit-plan-mode.md'; @@ -86,19 +86,38 @@ export class ExitPlanModeTool implements BuiltinTool { constructor(private readonly agent: Agent) {} - resolveExecution(args: ExitPlanModeInput): ToolExecution { + async resolveExecution(args: ExitPlanModeInput): Promise { return { description: 'Presenting plan and exiting plan mode', - execute: (ctx) => this.execution(args, ctx), + display: await this.resolvePlanReviewDisplay(args), + approvalRule: this.name, + execute: () => this.execution(args), }; } - private async execution( + private async resolvePlanReviewDisplay( args: ExitPlanModeInput, - { - metadata, - }: ExecutableToolContext, - ): Promise { + ): Promise { + if (!this.agent.planMode.isActive) return undefined; + let data: PlanData; + try { + data = await this.agent.planMode.data(); + } catch { + return undefined; + } + if (data === null || data.content.trim().length === 0) return undefined; + const display: ToolInputDisplay = { + kind: 'plan_review', + plan: data.content, + path: data.path, + }; + if (args.options !== undefined && args.options.length >= 2) { + display.options = args.options; + } + return display; + } + + private async execution(args: ExitPlanModeInput): Promise { if (!this.agent.planMode.isActive) { return { isError: true, @@ -110,38 +129,18 @@ export class ExitPlanModeTool implements BuiltinTool { const resolvedPlan = await this.resolvePlan(); if (!resolvedPlan.ok) return resolvedPlan.error; - if (!planTelemetryWasSubmitted(metadata)) { - this.agent.telemetry.track('plan_submitted', { - has_options: args.options !== undefined && args.options.length >= 2, - }); - } - return this.exitWithPlan( - resolvedPlan.plan, - resolvedPlan.path, - selectedOptionFromMetadata(metadata), - planTelemetryWasResolved(metadata), - ); - } + this.agent.telemetry.track('plan_submitted', { + has_options: args.options !== undefined && args.options.length >= 2, + }); - private async exitWithPlan( - plan: string, - path: string | undefined, - option: ExitPlanModeOption | undefined = undefined, - telemetryResolved = false, - ): Promise { const failed = this.exitPlanMode(); if (failed !== undefined) return failed; - if (!telemetryResolved) { - this.agent.telemetry.track('plan_resolved', { outcome: 'auto_approved' }); - } - const optionPrefix = - option === undefined - ? '' - : `Selected approach: ${option.label}\nExecute ONLY the selected approach. Do not execute any unselected alternatives.\n\n`; + this.agent.telemetry.track('plan_resolved', { outcome: 'auto_approved' }); + return { isError: false, - output: `Exited plan mode. ${optionPrefix}${formatPlanForOutput(plan, path)}`, + output: `Exited plan mode. ${formatPlanForOutput(resolvedPlan.plan, resolvedPlan.path)}`, }; } @@ -210,32 +209,6 @@ function normalizeOptionLabel(label: string): string { return label.trim().toLowerCase(); } -function selectedOptionFromMetadata(metadata: unknown): ExitPlanModeOption | undefined { - if (metadata === null || typeof metadata !== 'object') return undefined; - const selectedOption = (metadata as { readonly selectedOption?: unknown }).selectedOption; - if (selectedOption === null || typeof selectedOption !== 'object') return undefined; - const label = (selectedOption as { readonly label?: unknown }).label; - const description = (selectedOption as { readonly description?: unknown }).description; - if (typeof label !== 'string' || typeof description !== 'string') return undefined; - return { label, description }; -} - -function planTelemetryWasSubmitted(metadata: unknown): boolean { - return ( - metadata !== null && - typeof metadata === 'object' && - (metadata as { readonly planTelemetrySubmitted?: unknown }).planTelemetrySubmitted === true - ); -} - -function planTelemetryWasResolved(metadata: unknown): boolean { - return ( - metadata !== null && - typeof metadata === 'object' && - (metadata as { readonly planTelemetryResolved?: unknown }).planTelemetryResolved === true - ); -} - function formatPlanForOutput(plan: string, path: string | undefined): string { const savedTo = path !== undefined ? `Plan saved to: ${path}\n\n` : ''; return `Plan mode deactivated. All tools are now available.\n${savedTo}## Approved Plan:\n${plan}`; diff --git a/packages/agent-core/src/tools/builtin/shell/bash.ts b/packages/agent-core/src/tools/builtin/shell/bash.ts index ddcb2f7..f61b359 100644 --- a/packages/agent-core/src/tools/builtin/shell/bash.ts +++ b/packages/agent-core/src/tools/builtin/shell/bash.ts @@ -35,6 +35,7 @@ import type { Environment } from '../../../utils/environment'; import { renderPrompt } from '../../../utils/render-prompt'; import type { BackgroundProcessManager } from '../../background/manager'; import { toInputJsonSchema } from '../../support/input-schema'; +import { literalRulePattern, matchesGlobRuleSubject } from '../../support/rule-match'; import { ToolResultBuilder } from '../../support/result-builder'; import bashDescriptionTemplate from './bash.md'; @@ -173,6 +174,15 @@ export class BashTool implements BuiltinTool { description: args.run_in_background ? `Starting background: ${preview}` : `Running: ${preview}`, + display: { + kind: 'command', + command: args.command, + cwd: args.cwd ?? this.cwd, + description: args.description, + language: 'bash', + }, + approvalRule: literalRulePattern(this.name, args.command), + matchesRule: (ruleArgs) => matchesGlobRuleSubject(ruleArgs, args.command), execute: ({ signal }) => this.execution(args, signal), }; } diff --git a/packages/agent-core/src/tools/builtin/state/todo-list.ts b/packages/agent-core/src/tools/builtin/state/todo-list.ts index 22431ff..8787d96 100644 --- a/packages/agent-core/src/tools/builtin/state/todo-list.ts +++ b/packages/agent-core/src/tools/builtin/state/todo-list.ts @@ -102,6 +102,7 @@ export class TodoListTool implements BuiltinTool { : 'Updating todo list'; return { description, + approvalRule: this.name, execute: async () => { // Query mode — return the current list without mutation. if (args.todos === undefined) { diff --git a/packages/agent-core/src/tools/builtin/web/fetch-url.ts b/packages/agent-core/src/tools/builtin/web/fetch-url.ts index 6c15346..25af04d 100644 --- a/packages/agent-core/src/tools/builtin/web/fetch-url.ts +++ b/packages/agent-core/src/tools/builtin/web/fetch-url.ts @@ -12,6 +12,7 @@ import type { BuiltinTool } from '../../../agent/tool'; import { ToolAccesses } from '../../../loop/tool-access'; import type { ExecutableToolContext, ExecutableToolResult, ToolExecution } from '../../../loop/types'; import { toInputJsonSchema } from '../../support/input-schema'; +import { literalRulePattern, matchesGlobRuleSubject } from '../../support/rule-match'; import { ToolResultBuilder } from '../../support/result-builder'; import DESCRIPTION from './fetch-url.md'; @@ -74,6 +75,9 @@ export class FetchURLTool implements BuiltinTool { return { accesses: ToolAccesses.none(), description: `Fetching: ${preview}`, + display: { kind: 'url_fetch', url: args.url }, + approvalRule: literalRulePattern(this.name, args.url), + matchesRule: (ruleArgs) => matchesGlobRuleSubject(ruleArgs, args.url), execute: (ctx) => this.execution(args, ctx), }; } diff --git a/packages/agent-core/src/tools/builtin/web/web-search.ts b/packages/agent-core/src/tools/builtin/web/web-search.ts index 153a14f..215115e 100644 --- a/packages/agent-core/src/tools/builtin/web/web-search.ts +++ b/packages/agent-core/src/tools/builtin/web/web-search.ts @@ -12,6 +12,7 @@ import type { BuiltinTool } from '../../../agent/tool'; import { ToolAccesses } from '../../../loop/tool-access'; import type { ExecutableToolContext, ExecutableToolResult, ToolExecution } from '../../../loop/types'; import { toInputJsonSchema } from '../../support/input-schema'; +import { literalRulePattern, matchesGlobRuleSubject } from '../../support/rule-match'; import { ToolResultBuilder } from '../../support/result-builder'; import DESCRIPTION from './web-search.md'; @@ -70,6 +71,9 @@ export class WebSearchTool implements BuiltinTool { return { accesses: ToolAccesses.none(), description: `Searching: ${preview}`, + display: { kind: 'search', query: args.query }, + approvalRule: literalRulePattern(this.name, args.query), + matchesRule: (ruleArgs) => matchesGlobRuleSubject(ruleArgs, args.query), execute: (ctx) => this.execution(args, ctx), }; } diff --git a/packages/agent-core/src/tools/policies/default-permissions.ts b/packages/agent-core/src/tools/policies/default-permissions.ts deleted file mode 100644 index 01a562e..0000000 --- a/packages/agent-core/src/tools/policies/default-permissions.ts +++ /dev/null @@ -1,69 +0,0 @@ -/** - * Default permission posture for built-in tools. - * - * This table captures the Python-parity distinction between tools that - * call Approval.request() and tools that run without an approval prompt. - * The rule layer still applies explicit deny rules before consulting this - * table. - */ - -export type BuiltinToolDefaultPermission = 'auto_allow' | 'ask'; -export type KnownBuiltinToolName = - | 'Read' - | 'Grep' - | 'Glob' - | 'ReadMediaFile' - | 'Think' - | 'TodoList' - | 'TaskList' - | 'TaskOutput' - | 'WebSearch' - | 'FetchURL' - | 'Agent' - | 'AskUserQuestion' - | 'EnterPlanMode' - | 'ExitPlanMode' - | 'Skill' - | 'Bash' - | 'Write' - | 'Edit' - | 'TaskStop'; - -type BuiltinToolDefaultPermissionTable = Readonly< - Record ->; - -const BUILTIN_TOOL_DEFAULT_PERMISSION_TABLE: BuiltinToolDefaultPermissionTable = { - Read: 'auto_allow', - Grep: 'auto_allow', - Glob: 'auto_allow', - ReadMediaFile: 'auto_allow', - Think: 'auto_allow', - TodoList: 'auto_allow', - TaskList: 'auto_allow', - TaskOutput: 'auto_allow', - WebSearch: 'auto_allow', - FetchURL: 'auto_allow', - Agent: 'auto_allow', - AskUserQuestion: 'auto_allow', - EnterPlanMode: 'auto_allow', - ExitPlanMode: 'auto_allow', - Skill: 'auto_allow', - Bash: 'ask', - Write: 'ask', - Edit: 'ask', - TaskStop: 'ask', -}; - -export const BUILTIN_TOOL_DEFAULT_PERMISSIONS: BuiltinToolDefaultPermissionTable = - BUILTIN_TOOL_DEFAULT_PERMISSION_TABLE; - -export function getBuiltinToolDefaultPermission( - toolName: string, -): BuiltinToolDefaultPermission | undefined { - return BUILTIN_TOOL_DEFAULT_PERMISSIONS[toolName as KnownBuiltinToolName]; -} - -export function isDefaultAutoAllowTool(toolName: string): boolean { - return getBuiltinToolDefaultPermission(toolName) === 'auto_allow'; -} diff --git a/packages/agent-core/src/agent/permission/path-glob-match.ts b/packages/agent-core/src/tools/support/path-glob-match.ts similarity index 90% rename from packages/agent-core/src/agent/permission/path-glob-match.ts rename to packages/agent-core/src/tools/support/path-glob-match.ts index 5cecc17..15efd9e 100644 --- a/packages/agent-core/src/agent/permission/path-glob-match.ts +++ b/packages/agent-core/src/tools/support/path-glob-match.ts @@ -3,7 +3,7 @@ import * as win32Path from 'node:path/win32'; import picomatch from 'picomatch'; -import { canonicalizePath, type PathClass } from '../../tools/policies/path-access'; +import { canonicalizePath, type PathClass } from '../policies/path-access'; export interface PermissionPathMatchOptions { readonly cwd?: string; @@ -42,20 +42,15 @@ function stripLeadingDotSlash(value: string): string { export function pathGlobMatch( value: string, pattern: string, - options: { - readonly pathOptions?: PermissionPathMatchOptions; - readonly conservativeCaseFold: boolean; - }, + pathOptions?: PermissionPathMatchOptions, ): boolean { - const semantics = pathMatchSemantics(value, pattern, options.pathOptions); - const nocase = - options.pathOptions?.caseInsensitivePaths ?? - (semantics.pathClass === 'win32' || options.conservativeCaseFold); + const semantics = pathMatchSemantics(value, pattern, pathOptions); + const nocase = pathOptions?.caseInsensitivePaths ?? true; if (globMatch(value, pattern, { nocase })) return true; - for (const valueVariant of pathVariants(value, semantics, options.pathOptions)) { - for (const patternVariant of pathVariants(pattern, semantics, options.pathOptions)) { + for (const valueVariant of pathVariants(value, semantics, pathOptions)) { + for (const patternVariant of pathVariants(pattern, semantics, pathOptions)) { if (globMatch(valueVariant, patternVariant, { nocase })) return true; } } diff --git a/packages/agent-core/src/tools/support/rule-match.ts b/packages/agent-core/src/tools/support/rule-match.ts new file mode 100644 index 0000000..fe206ce --- /dev/null +++ b/packages/agent-core/src/tools/support/rule-match.ts @@ -0,0 +1,41 @@ +import { + globMatch, + pathGlobMatch, + type PermissionPathMatchOptions, +} from './path-glob-match'; + +const GLOB_LITERAL_SPECIAL = /[\\*?[\]{}()!+@|]/g; + +export function literalRulePattern(toolName: string, subject: string): string { + return `${toolName}(${escapeRuleSubjectLiteral(subject)})`; +} + +export function escapeRuleSubjectLiteral(subject: string): string { + return subject.replace(GLOB_LITERAL_SPECIAL, '\\$&'); +} + +export function matchesGlobRuleSubject(ruleArgs: string, subject: string): boolean { + return matchRuleSubjects(ruleArgs, [subject], (pattern, value) => globMatch(value, pattern)); +} + +export function matchesPathRuleSubject( + ruleArgs: string, + subject: string, + options?: PermissionPathMatchOptions, +): boolean { + return matchRuleSubjects(ruleArgs, [subject], (pattern, value) => + pathGlobMatch(value, pattern, options), + ); +} + +function matchRuleSubjects( + ruleArgs: string, + subjects: readonly string[], + matchesPositivePattern: (pattern: string, subject: string) => boolean, +): boolean { + if (ruleArgs.length === 0) return true; + const negated = ruleArgs.startsWith('!'); + const positivePattern = negated ? ruleArgs.slice(1) : ruleArgs; + const hit = subjects.some((subject) => matchesPositivePattern(positivePattern, subject)); + return negated ? !hit : hit; +} diff --git a/packages/agent-core/test/agent/basic.test.ts b/packages/agent-core/test/agent/basic.test.ts index 5eade27..5006f94 100644 --- a/packages/agent-core/test/agent/basic.test.ts +++ b/packages/agent-core/test/agent/basic.test.ts @@ -96,7 +96,7 @@ it('runs an agent turn through builtin tool approval and execution', async () => [emit] assistant.delta { "turnId": 0, "delta": "I will run that." } [emit] tool.call.delta { "turnId": 0, "toolCallId": "call_bash", "name": "Bash", "argumentsPart": "{\\"command\\":\\"printf lookup-result\\",\\"timeout\\":60}" } [wire] context.append_loop_event { "event": { "type": "content.part", "uuid": "", "turnId": "0", "step": 1, "stepUuid": "", "part": { "type": "text", "text": "I will run that." } }, "time": "