From f3c41bdb7ef8ac3594ebddcd576a5c620304bcf8 Mon Sep 17 00:00:00 2001 From: _Kerman Date: Mon, 25 May 2026 13:20:05 +0800 Subject: [PATCH 01/35] refactor: reorganize permission decision policies --- PERMISSION_DECISION_MODEL.md | 88 ++++++++ .../agent-core/src/agent/permission/index.ts | 17 +- .../permission/policies/ask-user-question.ts | 28 +-- .../permission/policies/cwd-outside-ask.ts | 35 +++ .../permission/policies/default-auto-allow.ts | 11 + .../policies/default-git-cwd-write.ts | 119 ++++++----- .../agent/permission/policies/fallback-ask.ts | 9 + .../src/agent/permission/policies/index.ts | 40 ++-- .../policies/permission-mode-approve.ts | 21 ++ .../src/agent/permission/policies/plan.ts | 200 ++++++++---------- .../policies/session-approval-history.ts | 35 +++ .../policies/system-safety-path-ask.ts | 90 ++++++++ .../policies/user-configured-rules.ts | 65 ++++++ .../src/agent/permission/policies/utils.ts | 164 ++++++++++++++ .../policies/yolo-workspace-access.ts | 74 ------- .../agent-core/src/agent/permission/policy.ts | 16 +- .../agent-core/test/agent/permission.test.ts | 17 +- .../test/tools/plan-mode-hard-block.test.ts | 109 +++++----- 18 files changed, 785 insertions(+), 353 deletions(-) create mode 100644 PERMISSION_DECISION_MODEL.md create mode 100644 packages/agent-core/src/agent/permission/policies/cwd-outside-ask.ts create mode 100644 packages/agent-core/src/agent/permission/policies/default-auto-allow.ts create mode 100644 packages/agent-core/src/agent/permission/policies/fallback-ask.ts create mode 100644 packages/agent-core/src/agent/permission/policies/permission-mode-approve.ts create mode 100644 packages/agent-core/src/agent/permission/policies/session-approval-history.ts create mode 100644 packages/agent-core/src/agent/permission/policies/system-safety-path-ask.ts create mode 100644 packages/agent-core/src/agent/permission/policies/user-configured-rules.ts create mode 100644 packages/agent-core/src/agent/permission/policies/utils.ts delete mode 100644 packages/agent-core/src/agent/permission/policies/yolo-workspace-access.ts diff --git a/PERMISSION_DECISION_MODEL.md b/PERMISSION_DECISION_MODEL.md new file mode 100644 index 0000000..f58cac3 --- /dev/null +++ b/PERMISSION_DECISION_MODEL.md @@ -0,0 +1,88 @@ +# Tool Permission Decision Model + +本文记录当前讨论中的 tool call permission 判断模型。讨论范围只包含一个已经通过前置校验、可以被执行的 tool call 如何被分类为 `approve` / `deny` / `ask`；不包含工具是否存在、是否启用、参数 schema 是否合法、ask 交互过程、以及工具执行时报错。 + +- **PreToolCall Hook Decision** + - Hook 返回 `block` -> `deny` + - Hook 返回 `allow` 或无结果 -> 继续后续判断 + +- **User Configured Permission Rules** + - 用户配置 `deny` rule 命中 -> `deny` + - 用户配置 `ask` rule 命中 -> `ask` + - 用户配置 `allow` rule 命中 -> `approve` + - 同时命中多个用户 rule 时 -> `deny > ask > allow` + - `Bash(pattern)` 匹配 `command` + - `Read(pattern)` 匹配 `path` + - `Write(pattern)` 匹配 `path` + - `Edit(pattern)` 匹配 `path` + - `ReadMediaFile(pattern)` 匹配 `path` + - `Grep(pattern)` 匹配 `pattern` + - `Glob(pattern)` 匹配 `pattern` + - `Agent(pattern)` 匹配 `subagent_type` + - `mcp__server__tool` / `mcp__server__*` 匹配 MCP tool name + - `ToolName` 无参数模式时只匹配 tool name + - `*(...)` 可作为全工具匹配模式 + - 路径 rule 匹配应基于 cwd 做规范化比较 + +- **Session Approval Memorized History** + - 记住的 `run command` action 命中 -> `approve` + - 记住的 `read file in cwd` action 命中 -> `approve` + - 记住的 `read file outside cwd` action 命中 -> `approve` + - 记住的 `write file in cwd` action 命中 -> `approve` + - 记住的 `write file outside cwd` action 命中 -> `approve` + - 记住的 `edit file in cwd` action 命中 -> `approve` + - 记住的 `edit file outside cwd` action 命中 -> `approve` + - 记住的 `stop background task` action 命中 -> `approve` + - 记住的 `call MCP tool: server:tool` action 命中 -> `approve` + - 记住的 `spawn agent` action 命中 -> `approve` + - 记住的 `invoke skill` action 命中 -> `approve` + - 记住的 action 必须按安全边界区分粒度，cwd 内外不能共用同一条 session rule + +- **System Deny Queue** + - `permissionMode=auto` 且 tool 是 `AskUserQuestion` -> `deny` + - plan mode active 且 `Write` 目标不是当前 plan file -> `deny` + - plan mode active 且 `Edit` 目标不是当前 plan file -> `deny` + - plan mode active 且当前没有 plan file path 时调用 `Write` -> `deny` + - plan mode active 且当前没有 plan file path 时调用 `Edit` -> `deny` + - plan mode active 且 tool 是 `TaskStop` -> `deny` + +- **System Ask Queue** + - `Read` 目标 path 在 cwd 外 -> `ask` + - `ReadMediaFile` 目标 path 在 cwd 外 -> `ask` + - `Write` 目标 path 在 cwd 外 -> `ask` + - `Edit` 目标 path 在 cwd 外 -> `ask` + - 目标 path 是敏感文件 `.env` / SSH private key / credentials 时 -> `ask` + - 目标 path 落在 `.git` 控制目录或 git control dir 内时 -> `ask` + - `ExitPlanMode` 且 plan mode active 且 plan 内容非空且 `permissionMode!=auto` -> `ask` + - 其他未来安全边界如果需要用户确认 -> `ask` + +- **System Approve Queue** + - `permissionMode=yolo` -> `approve` + - `permissionMode=auto` -> `approve` + - 默认 auto-allow tool `Read` -> `approve` + - 默认 auto-allow tool `Grep` -> `approve` + - 默认 auto-allow tool `Glob` -> `approve` + - 默认 auto-allow tool `ReadMediaFile` -> `approve` + - 默认 auto-allow tool `Think` -> `approve` + - 默认 auto-allow tool `SetTodoList` -> `approve` + - 默认 auto-allow tool `TaskList` -> `approve` + - 默认 auto-allow tool `TaskOutput` -> `approve` + - 默认 auto-allow tool `WebSearch` -> `approve` + - 默认 auto-allow tool `FetchURL` -> `approve` + - 默认 auto-allow tool `Agent` -> `approve` + - 默认 auto-allow tool `AskUserQuestion` -> `approve` + - 默认 auto-allow tool `EnterPlanMode` -> `approve` + - 默认 auto-allow tool `ExitPlanMode` -> `approve` + - 默认 auto-allow tool `Skill` -> `approve` + - `Write/Edit` 在 POSIX git cwd 内、目标在 cwd 内、不是敏感文件、不是 git 控制路径、路径中无 symlink -> `approve` + - `EnterPlanMode` -> `approve` + - `ExitPlanMode` 不在 plan mode active 状态 -> `approve` + - `ExitPlanMode` 在 plan mode active 但没有有效 plan 内容 -> `approve` + +- **Fallback** + - 以上全部未命中 -> `ask` + - `manual` 下的 `Bash` 通常靠 fallback -> `ask` + - `manual` 下的 `Write` 通常靠 fallback -> `ask` + - `manual` 下的 `Edit` 通常靠 fallback -> `ask` + - `manual` 下的 `TaskStop` 通常靠 fallback -> `ask` + - `manual` 下的 MCP tool / user tool 通常靠 fallback -> `ask` diff --git a/packages/agent-core/src/agent/permission/index.ts b/packages/agent-core/src/agent/permission/index.ts index 7628d84..0abfac3 100644 --- a/packages/agent-core/src/agent/permission/index.ts +++ b/packages/agent-core/src/agent/permission/index.ts @@ -6,7 +6,7 @@ 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 { createPermissionDecisionPolicies } from './policies'; import type { PermissionPolicy, PermissionPolicyResult } from './policy'; import type { PermissionApprovalResultRecord, @@ -38,7 +38,7 @@ export class PermissionManager { ) { this.rules = [...(options.initialRules ?? [])]; this.parent = options.parent; - this.policies = options.policies ?? createBuiltinPermissionPolicies(); + this.policies = options.policies ?? createPermissionDecisionPolicies(this.agent); } get mode(): PermissionMode { @@ -212,13 +212,8 @@ export class PermissionManager { ): Promise { for (const policy of this.policies) { const result = await policy.evaluate({ - agent: this.agent, - mode: this.mode, - toolCallContext: context, + ...context, matchedRule, - recordApprovalResult: (record) => { - this.recordApprovalResult(record); - }, }); if (result !== undefined) return result; } @@ -267,8 +262,10 @@ export class PermissionManager { : { executionMetadata: result.executionMetadata }; case 'ask': return this.requestToolApproval(context, result); - case 'result': - return result.result; + case 'result': { + const { kind: _kind, ...prepareResult } = result; + return prepareResult; + } } } 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 index a2edcc4..fa0cf77 100644 --- a/packages/agent-core/src/agent/permission/policies/ask-user-question.ts +++ b/packages/agent-core/src/agent/permission/policies/ask-user-question.ts @@ -1,17 +1,19 @@ -import type { PermissionPolicy } from '../policy'; +import type { Agent } from '../..'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; -export const AskUserQuestionAutoPermissionPolicy: PermissionPolicy = { - name: 'auto.ask-user-question', - evaluate({ mode, toolCallContext }) { - if (mode !== 'auto') return undefined; - if (toolCallContext.toolCall.function.name !== 'AskUserQuestion') return undefined; +export class AskUserQuestionAutoPermissionPolicy implements PermissionPolicy { + readonly name = 'auto.ask-user-question'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + if (this.agent.permission.mode !== 'auto') return undefined; + if (context.toolCall.function.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.', - }, + 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/cwd-outside-ask.ts b/packages/agent-core/src/agent/permission/policies/cwd-outside-ask.ts new file mode 100644 index 0000000..d413e3d --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/cwd-outside-ask.ts @@ -0,0 +1,35 @@ +import type { Agent } from '../..'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; +import { fileInputDisplay, resolvePathToolAccess } from './utils'; + +export class CwdOutsideAskPermissionPolicy implements PermissionPolicy { + readonly name = 'system.ask.cwd-outside-path'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const access = resolvePathToolAccess(this.agent, context); + if (access === undefined || !access.outsideCwd) return undefined; + + return { + kind: 'ask', + action: outsideCwdAction(context.toolCall.function.name), + display: fileInputDisplay(access, 'Path is outside the current working directory.'), + }; + } +} + +function outsideCwdAction(tool: string): string { + switch (tool) { + case 'Read': + return 'read file outside of working directory'; + case 'ReadMediaFile': + return 'read media file outside of working directory'; + case 'Write': + return 'write file outside of working directory'; + case 'Edit': + return 'edit file outside of working directory'; + default: + return `call ${tool} outside of working directory`; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/default-auto-allow.ts b/packages/agent-core/src/agent/permission/policies/default-auto-allow.ts new file mode 100644 index 0000000..74df9b1 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/default-auto-allow.ts @@ -0,0 +1,11 @@ +import { isDefaultAutoAllowTool } from '../../../tools/policies/default-permissions'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; + +export class DefaultAutoAllowPermissionPolicy implements PermissionPolicy { + readonly name = 'system.approve.default-auto-allow'; + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + if (!isDefaultAutoAllowTool(context.toolCall.function.name)) return undefined; + return { kind: 'allow' }; + } +} 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 index 656f208..c357d97 100644 --- 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 @@ -2,6 +2,7 @@ import * as posixPath from 'node:path/posix'; import type { Kaos } from '@moonshot-ai/kaos'; +import type { Agent } from '../..'; import { DEFAULT_WORKSPACE_ACCESS_POLICY, isWithinDirectory, @@ -12,72 +13,76 @@ import { findGitWorkTreeMarker, type GitWorkTreeMarker, } from '../../../tools/support/git-worktree'; -import type { PermissionPolicy } from '../policy'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; const AUTO_REASON = 'default_git_cwd_write'; const S_IFMT = 0o170000; const S_IFLNK = 0o120000; -export function createDefaultGitCwdWritePolicy(): PermissionPolicy { +export class DefaultGitCwdWritePermissionPolicy implements 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.function.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' }; - }, - }; + private readonly cache = new Map(); + readonly name = 'default.git-cwd-write'; + + constructor(private readonly agent: Agent) {} + + async evaluate({ + matchedRule, + toolCall, + args, + }: PermissionPolicyContext): Promise { + if (this.agent.permission.mode !== 'manual') return undefined; + if (matchedRule !== undefined) return undefined; + + const name = toolCall.function.name; + if (name !== 'Write' && name !== 'Edit') return undefined; + + const kaos = this.agent.runtime.kaos; + const pathClass = kaos.pathClass(); + if (pathClass !== 'posix') return undefined; + + const cwd = this.agent.config.cwd; + if (cwd.length === 0) return undefined; + + const path = readStringField(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 = this.cache.get(cwd) ?? (await findGitWorkTreeMarker(kaos, cwd)); + if (marker === null) return undefined; + this.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; + + this.agent.telemetry.track('tool_approved', { + tool_name: name, + approval_mode: 'manual', + auto_reason: AUTO_REASON, + }); + return { kind: 'allow' }; + } } function readStringField(args: unknown, key: string): string | undefined { 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..0a82606 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/fallback-ask.ts @@ -0,0 +1,9 @@ +import type { PermissionPolicy, PermissionPolicyResult } from '../policy'; + +export class FallbackAskPermissionPolicy implements PermissionPolicy { + readonly name = 'fallback.ask'; + + evaluate(): PermissionPolicyResult { + return { kind: 'ask' }; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/index.ts b/packages/agent-core/src/agent/permission/policies/index.ts index 07459fe..3be0101 100644 --- a/packages/agent-core/src/agent/permission/policies/index.ts +++ b/packages/agent-core/src/agent/permission/policies/index.ts @@ -1,19 +1,33 @@ +import type { Agent } from '../..'; 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 { CwdOutsideAskPermissionPolicy } from './cwd-outside-ask'; +import { DefaultAutoAllowPermissionPolicy } from './default-auto-allow'; +import { DefaultGitCwdWritePermissionPolicy } from './default-git-cwd-write'; +import { FallbackAskPermissionPolicy } from './fallback-ask'; +import { PermissionModeApprovePolicy } from './permission-mode-approve'; +import { + EnterPlanModePermissionPolicy, + ExitPlanModePermissionPolicy, + PlanModeGuardPermissionPolicy, +} from './plan'; +import { SessionApprovalHistoryPermissionPolicy } from './session-approval-history'; +import { SystemSafetyPathAskPermissionPolicy } from './system-safety-path-ask'; +import { UserConfiguredPermissionRulesPolicy } from './user-configured-rules'; -export function createBuiltinPermissionPolicies(): readonly PermissionPolicy[] { +export function createPermissionDecisionPolicies(agent: Agent): readonly PermissionPolicy[] { return [ - ...createPlanPermissionPolicies(), - YoloOutsideWorkspacePermissionPolicy, - createDefaultGitCwdWritePolicy(), - AskUserQuestionAutoPermissionPolicy, + new UserConfiguredPermissionRulesPolicy(agent), + new AskUserQuestionAutoPermissionPolicy(agent), + new PlanModeGuardPermissionPolicy(agent), + new SystemSafetyPathAskPermissionPolicy(agent), + new SessionApprovalHistoryPermissionPolicy(agent), + new CwdOutsideAskPermissionPolicy(agent), + new ExitPlanModePermissionPolicy(agent), + new PermissionModeApprovePolicy(agent), + new EnterPlanModePermissionPolicy(), + new DefaultAutoAllowPermissionPolicy(), + new DefaultGitCwdWritePermissionPolicy(agent), + 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/permission-mode-approve.ts b/packages/agent-core/src/agent/permission/policies/permission-mode-approve.ts new file mode 100644 index 0000000..79b9ae1 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/permission-mode-approve.ts @@ -0,0 +1,21 @@ +import type { Agent } from '../..'; +import { isDefaultAutoAllowTool } from '../../../tools/policies/default-permissions'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; + +export class PermissionModeApprovePolicy implements PermissionPolicy { + readonly name = 'system.approve.permission-mode'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const mode = this.agent.permission.mode; + if (mode !== 'yolo' && mode !== 'auto') return undefined; + const name = context.toolCall.function.name; + if (isDefaultAutoAllowTool(name)) return undefined; + this.agent.telemetry.track('tool_approved', { + tool_name: name, + approval_mode: mode === 'auto' ? 'afk' : 'yolo', + }); + return { kind: 'allow' }; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/plan.ts b/packages/agent-core/src/agent/permission/policies/plan.ts index 7747992..85cf9bb 100644 --- a/packages/agent-core/src/agent/permission/policies/plan.ts +++ b/packages/agent-core/src/agent/permission/policies/plan.ts @@ -1,6 +1,8 @@ +import type { Agent } from '../..'; import type { ExecutableToolResult } from '../../../loop'; import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; import type { ApprovalResponse } from '../types'; +import { readStringField } from './utils'; interface ExitPlanModeOption { readonly label: string; @@ -13,33 +15,38 @@ interface ExitPlanModeExecutionMetadata { readonly planTelemetryResolved: true; } -export const EnterPlanModePermissionPolicy: PermissionPolicy = { - name: 'plan.enter-plan-mode', - evaluate({ toolCallContext }) { - if (toolCallContext.toolCall.function.name !== 'EnterPlanMode') return undefined; +export class EnterPlanModePermissionPolicy implements PermissionPolicy { + readonly name = 'plan.enter-plan-mode'; + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + if (context.toolCall.function.name !== 'EnterPlanMode') return undefined; return { kind: 'allow' }; - }, -}; + } +} + +export class ExitPlanModePermissionPolicy implements PermissionPolicy { + readonly name = 'plan.exit-plan-mode'; -export const ExitPlanModePermissionPolicy: PermissionPolicy = { - name: 'plan.exit-plan-mode', - async evaluate(context) { - if (context.toolCallContext.toolCall.function.name !== 'ExitPlanMode') return undefined; - if (context.mode === 'auto') return { kind: 'allow' }; + constructor(private readonly agent: Agent) {} - const review = await resolveExitPlanModeReview(context); + async evaluate(context: PermissionPolicyContext): Promise { + const agent = this.agent; + if (context.toolCall.function.name !== 'ExitPlanMode') return undefined; + if (agent.permission.mode === 'auto') return { kind: 'allow' }; + + const review = await resolveExitPlanModeReview(agent, context); if (review === null) return { kind: 'allow' }; const action = exitPlanModeAction(review.options); - context.agent.telemetry.track('plan_submitted', { + agent.telemetry.track('plan_submitted', { has_options: review.options !== undefined, }); let result: ApprovalResponse; try { - result = await context.agent.rpc.requestApproval( + result = await agent.rpc.requestApproval( { - turnId: Number(context.toolCallContext.turnId), - toolCallId: context.toolCallContext.toolCall.id, + turnId: Number(context.turnId), + toolCallId: context.toolCall.id, toolName: 'ExitPlanMode', action, display: { @@ -49,41 +56,43 @@ export const ExitPlanModePermissionPolicy: PermissionPolicy = { options: review.options, }, }, - { signal: context.toolCallContext.signal }, + { signal: context.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}`, - }, + syntheticResult: { + isError: true, + output: `Plan approval failed: ${message}`, }, }; } - context.recordApprovalResult({ - turnId: Number(context.toolCallContext.turnId), - toolCallId: context.toolCallContext.toolCall.id, + agent.permission.recordApprovalResult({ + turnId: Number(context.turnId), + toolCallId: context.toolCall.id, toolName: 'ExitPlanMode', action, result, }); - trackExitPlanModeResolution(context, result); - return exitPlanModeApprovalResult(context, result, review.options); - }, -}; + trackExitPlanModeResolution(agent, result); + return exitPlanModeApprovalResult(agent, result, review.options); + } +} + +export class PlanModeGuardPermissionPolicy implements PermissionPolicy { + readonly name = 'plan.mode-guard'; -export const PlanModeGuardPermissionPolicy: PermissionPolicy = { - name: 'plan.mode-guard', - evaluate({ agent, toolCallContext }) { + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const agent = this.agent; if (!agent.planMode.isActive) return undefined; - const name = toolCallContext.toolCall.function.name; - const args = toolCallContext.args; + const name = context.toolCall.function.name; + const args = context.args; if (name === 'Write' || name === 'Edit') { const path = readStringField(args, 'path'); @@ -92,49 +101,45 @@ export const PlanModeGuardPermissionPolicy: PermissionPolicy = { 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.', - }, + 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.', - }, + 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[] { +export function createPlanPermissionPolicies(agent: Agent): readonly PermissionPolicy[] { return [ - EnterPlanModePermissionPolicy, - ExitPlanModePermissionPolicy, - PlanModeGuardPermissionPolicy, + new EnterPlanModePermissionPolicy(), + new ExitPlanModePermissionPolicy(agent), + new PlanModeGuardPermissionPolicy(agent), ]; } -async function resolveExitPlanModeReview(context: PermissionPolicyContext): Promise<{ +async function resolveExitPlanModeReview(agent: Agent, context: PermissionPolicyContext): Promise<{ readonly plan: string; readonly path?: string | undefined; readonly options?: readonly ExitPlanModeOption[] | undefined; } | null> { - if (!context.agent.planMode.isActive) return null; + if (!agent.planMode.isActive) return null; - let data: Awaited>; + let data: Awaited>; try { - data = await context.agent.planMode.data(); + data = await agent.planMode.data(); } catch { return null; } @@ -143,12 +148,12 @@ async function resolveExitPlanModeReview(context: PermissionPolicyContext): Prom return { plan: data.content, path: data.path, - options: exitPlanModeOptions(context.toolCallContext.args), + options: exitPlanModeOptions(context.args), }; } function exitPlanModeApprovalResult( - context: PermissionPolicyContext, + agent: Agent, result: ApprovalResponse, options: readonly ExitPlanModeOption[] | undefined, ): PermissionPolicyResult { @@ -163,27 +168,23 @@ function exitPlanModeApprovalResult( if (result.decision === 'cancelled') { return { kind: 'result', - result: { - syntheticResult: { - isError: false, - output: 'Plan approval dismissed. Plan mode remains active.', - }, + syntheticResult: { + isError: false, + output: 'Plan approval dismissed. Plan mode remains active.', }, }; } if (result.selectedLabel === 'Reject and Exit') { - const failed = exitPlanModeForRejectedPlan(context); + const failed = exitPlanModeForRejectedPlan(agent); return { kind: 'result', - result: { - syntheticResult: - failed ?? { - isError: true, - stopTurn: true, - output: 'Plan rejected by user. Plan mode deactivated.', - }, - }, + syntheticResult: + failed ?? { + isError: true, + stopTurn: true, + output: 'Plan rejected by user. Plan mode deactivated.', + }, }; } @@ -191,26 +192,22 @@ function exitPlanModeApprovalResult( 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.', - }, + 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.', - }, + syntheticResult: { + isError: true, + stopTurn: true, + output: 'Plan rejected by user. Plan mode remains active.', }, }; } @@ -225,53 +222,48 @@ function exitPlanModeExecutionMetadata( }; } -function trackExitPlanModeResolution( - context: PermissionPolicyContext, - result: ApprovalResponse, -): void { +function trackExitPlanModeResolution(agent: Agent, 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' }); + agent.telemetry.track('plan_resolved', { outcome: 'dismissed' }); return; } if (result.decision === 'approved') { if (selectedLabel.length > 0) { - context.agent.telemetry.track('plan_resolved', { + agent.telemetry.track('plan_resolved', { outcome: 'approved', chosen_option: selectedLabel, }); return; } - context.agent.telemetry.track('plan_resolved', { outcome: 'approved' }); + agent.telemetry.track('plan_resolved', { outcome: 'approved' }); return; } if (normalizedSelectedLabel === 'reject and exit') { - context.agent.telemetry.track('plan_resolved', { outcome: 'rejected_and_exited' }); + agent.telemetry.track('plan_resolved', { outcome: 'rejected_and_exited' }); return; } if (normalizedSelectedLabel === 'revise' || hasFeedback) { - context.agent.telemetry.track('plan_resolved', { + agent.telemetry.track('plan_resolved', { outcome: 'revise', has_feedback: hasFeedback, }); return; } - context.agent.telemetry.track('plan_resolved', { outcome: 'rejected' }); + agent.telemetry.track('plan_resolved', { outcome: 'rejected' }); } -function exitPlanModeForRejectedPlan( - context: PermissionPolicyContext, -): ExecutableToolResult | undefined { +function exitPlanModeForRejectedPlan(agent: Agent): ExecutableToolResult | undefined { try { - context.agent.planMode.exit(); + agent.planMode.exit(); } catch (error) { const message = error instanceof Error ? error.message : 'Failed to exit plan mode.'; return { @@ -290,8 +282,6 @@ function exitPlanModeOptions(args: unknown): readonly ExitPlanModeOption[] | und 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 ?? '' }); @@ -316,9 +306,3 @@ function exitPlanModeAction(options: readonly ExitPlanModeOption[] | undefined): 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/session-approval-history.ts b/packages/agent-core/src/agent/permission/policies/session-approval-history.ts new file mode 100644 index 0000000..efb0323 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/session-approval-history.ts @@ -0,0 +1,35 @@ +import type { Agent } from '../..'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; +import { firstMatchingRuleDecision } from './utils'; + +export class SessionApprovalHistoryPermissionPolicy implements PermissionPolicy { + readonly name = 'session.approval-history'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + if ( + context.matchedRule?.scope === 'session-runtime' && + context.matchedRule.decision === 'allow' + ) { + trackSessionApproval(this.agent, context); + return { kind: 'allow' }; + } + + const sessionRules = this.agent.permission.rules.filter( + (rule) => rule.scope === 'session-runtime', + ); + const match = firstMatchingRuleDecision(sessionRules, this.agent, context); + if (match?.decision !== 'allow') return undefined; + trackSessionApproval(this.agent, context); + return { kind: 'allow' }; + } +} + +function trackSessionApproval(agent: Agent, context: PermissionPolicyContext): void { + agent.telemetry.track('tool_approved', { + tool_name: context.toolCall.function.name, + approval_mode: 'auto_session', + scope: 'session', + }); +} diff --git a/packages/agent-core/src/agent/permission/policies/system-safety-path-ask.ts b/packages/agent-core/src/agent/permission/policies/system-safety-path-ask.ts new file mode 100644 index 0000000..372e3c1 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/system-safety-path-ask.ts @@ -0,0 +1,90 @@ +import * as posixPath from 'node:path/posix'; +import * as win32Path from 'node:path/win32'; + +import type { Agent } from '../..'; +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 '../policy'; +import { fileInputDisplay, resolvePathToolAccess } from './utils'; + +export class SystemSafetyPathAskPermissionPolicy implements PermissionPolicy { + private readonly gitMarkerCache = new Map(); + readonly name = 'system.ask.safety-path'; + + constructor(private readonly agent: Agent) {} + + async evaluate(context: PermissionPolicyContext): Promise { + const access = resolvePathToolAccess(this.agent, context); + if (access === undefined) return undefined; + + if (isSensitiveFile(access.path, access.pathClass)) { + return { + kind: 'ask', + action: freshSafetyAction(context, 'sensitive file', access.path), + display: fileInputDisplay(access, 'Sensitive file access requires explicit approval.'), + }; + } + + const cwd = this.agent.config.cwd; + if (!mayBeGitControlPath(access.path, cwd, access.pathClass)) return undefined; + + const marker = + this.gitMarkerCache.get(cwd) ?? (await findGitWorkTreeMarker(this.agent.runtime.kaos, cwd)); + if (marker !== null) this.gitMarkerCache.set(cwd, marker); + if (marker !== null && isGitControlPath(access.path, cwd, marker, access.pathClass)) { + return { + kind: 'ask', + action: freshSafetyAction(context, 'git control path', access.path), + display: fileInputDisplay(access, 'Git control path access requires explicit approval.'), + }; + } + + return undefined; + } +} + +function freshSafetyAction( + context: PermissionPolicyContext, + boundary: string, + path: string, +): string { + const name = context.toolCall.function.name; + const id = context.toolCall.id; + return `Confirm ${name} on ${boundary}: ${path} for tool call ${id}`; +} + +function mayBeGitControlPath(targetPath: string, cwd: string, pathClass: PathClass): boolean { + return relativePathParts(targetPath, cwd, pathClass).some((part) => + part.toLowerCase().startsWith('.git'), + ); +} + +function isGitControlPath( + targetPath: string, + cwd: string, + marker: GitWorkTreeMarker, + pathClass: PathClass, +): boolean { + if (relativePathParts(targetPath, cwd, pathClass).some((part) => part.toLowerCase() === '.git')) { + return true; + } + 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/user-configured-rules.ts b/packages/agent-core/src/agent/permission/policies/user-configured-rules.ts new file mode 100644 index 0000000..4caf3e6 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/user-configured-rules.ts @@ -0,0 +1,65 @@ +import type { Agent } from '../..'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; +import type { PermissionRule, PermissionRuleScope } from '../types'; +import { + firstMatchingRuleDecision, + formatPermissionRuleDenyMessage, + genericInputDisplay, +} from './utils'; + +const USER_CONFIGURED_SCOPES = new Set([ + 'turn-override', + 'project', + 'user', +]); + +export class UserConfiguredPermissionRulesPolicy implements PermissionPolicy { + readonly name = 'user.configured-rules'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + if ( + context.matchedRule !== undefined && + USER_CONFIGURED_SCOPES.has(context.matchedRule.scope) + ) { + return permissionRuleResult(context, context.matchedRule.decision, context.matchedRule); + } + + const rules = this.agent.permission.rules.filter((rule): rule is PermissionRule => + USER_CONFIGURED_SCOPES.has(rule.scope), + ); + const match = firstMatchingRuleDecision(rules, this.agent, context); + if (match === undefined) return undefined; + + return permissionRuleResult(context, match.decision, match.rule); + } +} + +function permissionRuleResult( + context: PermissionPolicyContext, + decision: PermissionRule['decision'], + rule: PermissionRule, +): PermissionPolicyResult { + const name = context.toolCall.function.name; + switch (decision) { + case 'deny': + return { + kind: 'result', + block: true, + reason: formatPermissionRuleDenyMessage(name, rule.reason), + }; + case 'ask': + return { + kind: 'ask', + action: `Approve ${name} due to permission rule`, + display: genericInputDisplay(`Approve ${name}`, { + rule: rule.pattern, + reason: rule.reason, + args: context.args, + }), + }; + case 'allow': + return { kind: 'allow' }; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/utils.ts b/packages/agent-core/src/agent/permission/policies/utils.ts new file mode 100644 index 0000000..7db9a74 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/utils.ts @@ -0,0 +1,164 @@ +import type { Agent } from '../..'; +import type { ToolInputDisplay } from '../../../tools/display'; +import { + resolvePathAccess, + type PathAccessOperation, + type PathClass, +} from '../../../tools/policies/path-access'; +import { matchesRule } from '../matches-rule'; +import type { PermissionPathMatchOptions } from '../path-glob-match'; +import type { PermissionPolicyContext } from '../policy'; +import type { PermissionRule, PermissionRuleDecision } from '../types'; + +export type PathToolDisplayOperation = Extract< + ToolInputDisplay, + { kind: 'file_io' } +>['operation']; + +export interface PathToolAccess { + readonly inputPath: string; + readonly path: string; + readonly outsideCwd: boolean; + readonly pathClass: PathClass; + readonly operation: PathAccessOperation; + readonly displayOperation: PathToolDisplayOperation; +} + +export 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; +} + +export function resolvePathToolAccess( + agent: Agent, + context: PermissionPolicyContext, +): PathToolAccess | undefined { + const name = context.toolCall.function.name; + const operation = pathAccessOperationForTool(name); + if (operation === undefined) return undefined; + + const inputPath = readStringField(context.args, 'path'); + if (inputPath === undefined) return undefined; + + const cwd = agent.config.cwd; + if (cwd.length === 0) return undefined; + + const kaos = agent.runtime.kaos; + const pathClass = kaos.pathClass(); + let access; + try { + access = resolvePathAccess( + inputPath, + cwd, + { workspaceDir: cwd, additionalDirs: [] }, + { + operation, + pathClass, + homeDir: kaos.gethome(), + policy: { guardMode: 'disabled', checkSensitive: false }, + }, + ); + } catch { + return undefined; + } + + return { + inputPath, + path: access.path, + outsideCwd: access.outsideWorkspace, + pathClass, + operation, + displayOperation: pathToolDisplayOperation(name), + }; +} + +export function pathAccessOperationForTool(tool: string): PathAccessOperation | undefined { + switch (tool) { + case 'Read': + case 'ReadMediaFile': + return 'read'; + case 'Write': + case 'Edit': + return 'write'; + default: + return undefined; + } +} + +export function pathToolDisplayOperation(tool: string): PathToolDisplayOperation { + switch (tool) { + case 'Write': + return 'write'; + case 'Edit': + return 'edit'; + default: + return 'read'; + } +} + +export function fileInputDisplay( + access: PathToolAccess, + detail: string | undefined, +): ToolInputDisplay { + return { + kind: 'file_io', + operation: access.displayOperation, + path: access.path, + detail, + }; +} + +export function genericInputDisplay(summary: string, detail?: unknown): ToolInputDisplay { + return { + kind: 'generic', + summary, + detail, + }; +} + +export function permissionPathMatchOptions( + agent: Agent, +): PermissionPathMatchOptions { + return { + cwd: agent.config.cwd, + pathClass: agent.runtime.kaos.pathClass(), + homeDir: agent.runtime.kaos.gethome(), + }; +} + +export function firstMatchingRuleDecision( + rules: readonly PermissionRule[], + agent: Agent, + context: PermissionPolicyContext, +): { readonly decision: PermissionRuleDecision; readonly rule: PermissionRule } | undefined { + for (const decision of ['deny', 'ask', 'allow'] as const) { + const rule = firstMatchingRule(rules, decision, agent, context); + if (rule !== undefined) return { decision, rule }; + } + return undefined; +} + +function firstMatchingRule( + rules: readonly PermissionRule[], + decision: PermissionRuleDecision, + agent: Agent, + context: PermissionPolicyContext, +): PermissionRule | undefined { + const name = context.toolCall.function.name; + const args = context.args; + const pathOptions = permissionPathMatchOptions(agent); + for (const rule of rules) { + if (rule.decision !== decision) continue; + if (matchesRule(rule, name, args, pathOptions)) return rule; + } + return undefined; +} + +export function formatPermissionRuleDenyMessage( + tool: string, + reason: string | undefined, +): string { + const suffix = reason !== undefined && reason.length > 0 ? ` Reason: ${reason}` : ''; + return `Tool "${tool}" was denied by permission rule.${suffix}`; +} 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 b95b954..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.function.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 index 5baf777..6302a3b 100644 --- a/packages/agent-core/src/agent/permission/policy.ts +++ b/packages/agent-core/src/agent/permission/policy.ts @@ -1,12 +1,8 @@ -import type { Agent } from '..'; import type { PrepareToolExecutionResult, ToolExecutionHookContext } from '../../loop'; import type { ToolInputDisplay } from '../../tools/display'; -import type { PermissionApprovalResultRecord, PermissionMode, PermissionRule } from './types'; +import type { PermissionRule } from './types'; -export interface PermissionPolicyContext { - readonly agent: Agent; - readonly mode: PermissionMode; - readonly toolCallContext: ToolExecutionHookContext; +export interface PermissionPolicyContext extends ToolExecutionHookContext { /** * The rule matched by `checkPermission()`, if any. * @@ -14,9 +10,8 @@ export interface PermissionPolicyContext { * 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 = @@ -24,10 +19,7 @@ export type PermissionPolicyResult = readonly kind: 'allow'; readonly executionMetadata?: unknown; } - | { - readonly kind: 'result'; - readonly result: PrepareToolExecutionResult; - } + | ({ readonly kind: 'result' } & PrepareToolExecutionResult) | { readonly kind: 'ask'; readonly action?: string | undefined; diff --git a/packages/agent-core/test/agent/permission.test.ts b/packages/agent-core/test/agent/permission.test.ts index 987e87b..048ca38 100644 --- a/packages/agent-core/test/agent/permission.test.ts +++ b/packages/agent-core/test/agent/permission.test.ts @@ -523,7 +523,7 @@ describe('Permission auto mode', () => { describe('Permission policy chain', () => { it('keeps plan-specific policies under the permission module plan namespace', () => { - expect(createPlanPermissionPolicies().map((policy) => policy.name)).toEqual([ + expect(createPlanPermissionPolicies({} as Agent).map((policy) => policy.name)).toEqual([ 'plan.enter-plan-mode', 'plan.exit-plan-mode', 'plan.mode-guard', @@ -535,10 +535,8 @@ describe('Permission policy chain', () => { name: 'test.block-bash', evaluate: vi.fn(async (): Promise => ({ kind: 'result', - result: { - block: true, - reason: 'blocked by custom policy', - }, + block: true, + reason: 'blocked by custom policy', })), }; const { manager, requestApproval } = makePermissionManager( @@ -552,11 +550,8 @@ describe('Permission policy chain', () => { }); expect(policy.evaluate).toHaveBeenCalledWith( expect.objectContaining({ - agent: expect.any(Object), - mode: 'manual', - toolCallContext: expect.objectContaining({ - toolCall: expect.objectContaining({ id: 'call_policy' }), - }), + toolCall: expect.objectContaining({ id: 'call_policy' }), + matchedRule: undefined, }), ); expect(requestApproval).not.toHaveBeenCalled(); @@ -1842,6 +1837,7 @@ function makePermissionManager( }, } as unknown as Agent; manager = new PermissionManager(agent, options); + Object.assign(agent, { permission: manager }); return { manager, record, requestApproval, telemetryTrack }; } @@ -1892,6 +1888,7 @@ function makePlanPermissionManager(input: { }, } as unknown as Agent; const manager = new PermissionManager(agent); + Object.assign(agent, { permission: manager }); manager.mode = input.mode; return { manager, record, requestApproval, exit }; } diff --git a/packages/agent-core/test/tools/plan-mode-hard-block.test.ts b/packages/agent-core/test/tools/plan-mode-hard-block.test.ts index 2cea3b4..f4c198c 100644 --- a/packages/agent-core/test/tools/plan-mode-hard-block.test.ts +++ b/packages/agent-core/test/tools/plan-mode-hard-block.test.ts @@ -47,20 +47,25 @@ function hookContext(toolName: string, args: unknown): ToolExecutionHookContext } function policyContext( - agent: Agent, toolName: string, args: unknown, - mode: PermissionMode = 'manual', + _mode: PermissionMode = 'manual', ): PermissionPolicyContext { return { - agent, - mode, - toolCallContext: hookContext(toolName, args), + ...hookContext(toolName, args), matchedRule: undefined, - recordApprovalResult: vi.fn(), }; } +function evaluatePlanPolicy( + agent: Agent, + toolName: string, + args: unknown, + mode: PermissionMode = 'manual', +) { + return new PlanModeGuardPermissionPolicy(agent).evaluate(policyContext(toolName, args, mode)); +} + describe('Plan mode permission policy', () => { it('allows Write and Edit to the active plan file', async () => { const { agent, planMode } = await activePlanAgent(); @@ -68,15 +73,17 @@ describe('Plan mode permission policy', () => { if (planPath === null) throw new Error('expected plan path'); expect( - await PlanModeGuardPermissionPolicy.evaluate(policyContext(agent, 'Write', { path: planPath })), + await evaluatePlanPolicy(agent, 'Write', { path: planPath }), ).toEqual({ kind: 'allow' }); expect( - await PlanModeGuardPermissionPolicy.evaluate( - policyContext(agent, 'Edit', { + await evaluatePlanPolicy( + agent, + 'Edit', + { path: planPath, old_string: 'A', new_string: 'B', - }), + }, ), ).toEqual({ kind: 'allow' }); }); @@ -84,41 +91,38 @@ describe('Plan mode permission policy', () => { it('blocks Write and Edit to non-plan files before permission approval', async () => { const { agent } = await activePlanAgent(); - const write = await PlanModeGuardPermissionPolicy.evaluate( - policyContext(agent, 'Write', { path: '/workspace/src/main.ts', content: 'x' }), - ); - const edit = await PlanModeGuardPermissionPolicy.evaluate( - policyContext(agent, 'Edit', { - path: '/workspace/src/main.ts', - old_string: 'A', - new_string: 'B', - }), - ); - - expect(write).toMatchObject({ kind: 'result', result: { block: true } }); - expect(write?.kind === 'result' ? write.result.reason : '').toContain('current plan file'); - expect(write?.kind === 'result' ? write.result.reason : '').toContain('ExitPlanMode'); - expect(edit).toMatchObject({ kind: 'result', result: { block: true } }); - expect(edit?.kind === 'result' ? edit.result.reason : '').toContain('current plan file'); + const write = await evaluatePlanPolicy(agent, 'Write', { + path: '/workspace/src/main.ts', + content: 'x', + }); + const edit = await evaluatePlanPolicy(agent, 'Edit', { + path: '/workspace/src/main.ts', + old_string: 'A', + new_string: 'B', + }); + + expect(write).toMatchObject({ kind: 'result', block: true }); + expect(write?.kind === 'result' ? write.reason : '').toContain('current plan file'); + expect(write?.kind === 'result' ? write.reason : '').toContain('ExitPlanMode'); + expect(edit).toMatchObject({ kind: 'result', block: true }); + expect(edit?.kind === 'result' ? edit.reason : '').toContain('current plan file'); }); it('blocks file edits when plan mode has no selected plan file path', async () => { const { agent, planMode } = await activePlanAgent(); (planMode as unknown as { _planFilePath: string | null })._planFilePath = null; - const result = await PlanModeGuardPermissionPolicy.evaluate( - policyContext(agent, 'Edit', { - path: '/workspace/src/other.ts', - old_string: 'A', - new_string: 'B', - }), - ); + const result = await evaluatePlanPolicy(agent, 'Edit', { + path: '/workspace/src/other.ts', + old_string: 'A', + new_string: 'B', + }); - expect(result).toMatchObject({ kind: 'result', result: { block: true } }); - expect(result?.kind === 'result' ? result.result.reason : '').toContain( + expect(result).toMatchObject({ kind: 'result', block: true }); + expect(result?.kind === 'result' ? result.reason : '').toContain( '(no plan file selected yet)', ); - expect(result?.kind === 'result' ? result.result.reason : '').toContain('ExitPlanMode'); + expect(result?.kind === 'result' ? result.reason : '').toContain('ExitPlanMode'); }); it.each(['manual', 'yolo', 'auto'] as const)( @@ -127,14 +131,10 @@ describe('Plan mode permission policy', () => { const { agent } = await activePlanAgent(); expect( - await PlanModeGuardPermissionPolicy.evaluate( - policyContext(agent, 'Bash', { command: 'rm foo.txt' }, mode), - ), + await evaluatePlanPolicy(agent, 'Bash', { command: 'rm foo.txt' }, mode), ).toBeUndefined(); expect( - await PlanModeGuardPermissionPolicy.evaluate( - policyContext(agent, 'Bash', { command: 'ls -la' }, mode), - ), + await evaluatePlanPolicy(agent, 'Bash', { command: 'ls -la' }, mode), ).toBeUndefined(); }, ); @@ -144,13 +144,16 @@ describe('Plan mode permission policy', () => { async (mode) => { const { agent } = await activePlanAgent(); - const result = await PlanModeGuardPermissionPolicy.evaluate( - policyContext(agent, 'TaskStop', { task_id: 'bash-abc12345' }, mode), + const result = await evaluatePlanPolicy( + agent, + 'TaskStop', + { task_id: 'bash-abc12345' }, + mode, ); - expect(result).toMatchObject({ kind: 'result', result: { block: true } }); - expect(result?.kind === 'result' ? result.result.reason : '').toContain('plan mode'); - expect(result?.kind === 'result' ? result.result.reason : '').toContain('ExitPlanMode'); + expect(result).toMatchObject({ kind: 'result', block: true }); + expect(result?.kind === 'result' ? result.reason : '').toContain('plan mode'); + expect(result?.kind === 'result' ? result.reason : '').toContain('ExitPlanMode'); }, ); @@ -159,19 +162,13 @@ describe('Plan mode permission policy', () => { planMode.exit(); expect( - await PlanModeGuardPermissionPolicy.evaluate( - policyContext(agent, 'Write', { path: '/workspace/src/main.ts' }), - ), + await evaluatePlanPolicy(agent, 'Write', { path: '/workspace/src/main.ts' }), ).toBeUndefined(); expect( - await PlanModeGuardPermissionPolicy.evaluate( - policyContext(agent, 'Bash', { command: 'rm foo.txt' }), - ), + await evaluatePlanPolicy(agent, 'Bash', { command: 'rm foo.txt' }), ).toBeUndefined(); expect( - await PlanModeGuardPermissionPolicy.evaluate( - policyContext(agent, 'TaskStop', { task_id: 'bash-abc12345' }), - ), + await evaluatePlanPolicy(agent, 'TaskStop', { task_id: 'bash-abc12345' }), ).toBeUndefined(); }); }); From 44be73568f52a8b088d9eb8ca789105f4e17ed9e Mon Sep 17 00:00:00 2001 From: _Kerman Date: Mon, 25 May 2026 18:15:06 +0800 Subject: [PATCH 02/35] feat: rework permission decision policies --- .changeset/modern-permission-policies.md | 6 + PERMISSION_DECISION_MODEL.md | 209 ++++++---- .../src/agent/permission/action-label.ts | 142 ------- .../src/agent/permission/check-rules.ts | 59 --- .../agent-core/src/agent/permission/index.ts | 382 +++++++++--------- .../src/agent/permission/matches-rule.ts | 174 ++++---- .../src/agent/permission/parse-pattern.ts | 7 +- .../permission/policies/auto-mode-approve.ts | 15 + ...ts => auto-mode-ask-user-question-deny.ts} | 9 +- .../permission/policies/cwd-outside-ask.ts | 35 -- .../permission/policies/default-auto-allow.ts | 11 - .../policies/default-git-cwd-write.ts | 132 ------ .../policies/default-tool-approve.ts | 29 ++ .../policies/exit-plan-mode-review-ask.ts | 130 ++++++ .../agent/permission/policies/fallback-ask.ts | 10 +- .../permission/policies/file-access-ask.ts | 128 ++++++ .../policies/git-cwd-write-approve.ts | 53 +++ .../src/agent/permission/policies/index.ts | 54 ++- .../policies/permission-mode-approve.ts | 21 - .../policies/plan-mode-guard-deny.ts | 62 +++ .../policies/plan-mode-tool-approve.ts | 25 ++ .../src/agent/permission/policies/plan.ts | 308 -------------- .../permission/policies/pre-tool-call-hook.ts | 36 ++ .../policies/session-approval-history.ts | 32 +- .../policies/system-safety-path-ask.ts | 90 ----- .../policies/user-configured-rules.ts | 125 ++++-- .../src/agent/permission/policies/utils.ts | 164 -------- .../permission/policies/yolo-mode-approve.ts | 15 + .../agent-core/src/agent/permission/policy.ts | 50 ++- .../src/agent/permission/stable-args.ts | 25 ++ .../agent-core/src/agent/permission/types.ts | 3 +- packages/agent-core/src/agent/turn/index.ts | 27 +- packages/agent-core/src/loop/index.ts | 2 + packages/agent-core/src/loop/tool-call.ts | 67 ++- packages/agent-core/src/loop/types.ts | 12 +- .../src/tools/background/task-list.ts | 3 + .../src/tools/background/task-output.ts | 2 + .../src/tools/background/task-stop.ts | 2 + .../src/tools/builtin/collaboration/agent.ts | 8 + .../tools/builtin/collaboration/skill-tool.ts | 3 + .../agent-core/src/tools/builtin/file/edit.ts | 3 + .../agent-core/src/tools/builtin/file/glob.ts | 3 + .../agent-core/src/tools/builtin/file/grep.ts | 3 + .../src/tools/builtin/file/read-media.ts | 3 + .../agent-core/src/tools/builtin/file/read.ts | 3 + .../src/tools/builtin/file/write.ts | 3 + .../tools/builtin/planning/exit-plan-mode.ts | 27 +- .../src/tools/builtin/shell/bash.ts | 9 + .../src/tools/builtin/web/fetch-url.ts | 3 + .../src/tools/builtin/web/web-search.ts | 3 + .../src/tools/support/rule-match.ts | 12 + .../agent-core/test/agent/permission.test.ts | 340 +++++++--------- .../test/tools/fixtures/execute-tool.ts | 13 +- .../test/tools/plan-mode-hard-block.test.ts | 61 ++- .../planning/exit-plan-mode-telemetry.test.ts | 19 +- 55 files changed, 1458 insertions(+), 1714 deletions(-) create mode 100644 .changeset/modern-permission-policies.md delete mode 100644 packages/agent-core/src/agent/permission/action-label.ts delete mode 100644 packages/agent-core/src/agent/permission/check-rules.ts create mode 100644 packages/agent-core/src/agent/permission/policies/auto-mode-approve.ts rename packages/agent-core/src/agent/permission/policies/{ask-user-question.ts => auto-mode-ask-user-question-deny.ts} (76%) delete mode 100644 packages/agent-core/src/agent/permission/policies/cwd-outside-ask.ts delete mode 100644 packages/agent-core/src/agent/permission/policies/default-auto-allow.ts delete mode 100644 packages/agent-core/src/agent/permission/policies/default-git-cwd-write.ts create mode 100644 packages/agent-core/src/agent/permission/policies/default-tool-approve.ts create mode 100644 packages/agent-core/src/agent/permission/policies/exit-plan-mode-review-ask.ts create mode 100644 packages/agent-core/src/agent/permission/policies/file-access-ask.ts create mode 100644 packages/agent-core/src/agent/permission/policies/git-cwd-write-approve.ts delete mode 100644 packages/agent-core/src/agent/permission/policies/permission-mode-approve.ts create mode 100644 packages/agent-core/src/agent/permission/policies/plan-mode-guard-deny.ts create mode 100644 packages/agent-core/src/agent/permission/policies/plan-mode-tool-approve.ts delete mode 100644 packages/agent-core/src/agent/permission/policies/plan.ts create mode 100644 packages/agent-core/src/agent/permission/policies/pre-tool-call-hook.ts delete mode 100644 packages/agent-core/src/agent/permission/policies/system-safety-path-ask.ts delete mode 100644 packages/agent-core/src/agent/permission/policies/utils.ts create mode 100644 packages/agent-core/src/agent/permission/policies/yolo-mode-approve.ts create mode 100644 packages/agent-core/src/agent/permission/stable-args.ts create mode 100644 packages/agent-core/src/tools/support/rule-match.ts diff --git a/.changeset/modern-permission-policies.md b/.changeset/modern-permission-policies.md new file mode 100644 index 0000000..5aba0d9 --- /dev/null +++ b/.changeset/modern-permission-policies.md @@ -0,0 +1,6 @@ +--- +"@moonshot-ai/agent-core": minor +"@moonshot-ai/kimi-code": minor +--- + +Rework tool permission decisions around resolved execution metadata, with exact-argument session approvals and centralized permission telemetry. diff --git a/PERMISSION_DECISION_MODEL.md b/PERMISSION_DECISION_MODEL.md index f58cac3..aefe288 100644 --- a/PERMISSION_DECISION_MODEL.md +++ b/PERMISSION_DECISION_MODEL.md @@ -1,88 +1,125 @@ # Tool Permission Decision Model -本文记录当前讨论中的 tool call permission 判断模型。讨论范围只包含一个已经通过前置校验、可以被执行的 tool call 如何被分类为 `approve` / `deny` / `ask`；不包含工具是否存在、是否启用、参数 schema 是否合法、ask 交互过程、以及工具执行时报错。 - -- **PreToolCall Hook Decision** - - Hook 返回 `block` -> `deny` - - Hook 返回 `allow` 或无结果 -> 继续后续判断 - -- **User Configured Permission Rules** - - 用户配置 `deny` rule 命中 -> `deny` - - 用户配置 `ask` rule 命中 -> `ask` - - 用户配置 `allow` rule 命中 -> `approve` - - 同时命中多个用户 rule 时 -> `deny > ask > allow` - - `Bash(pattern)` 匹配 `command` - - `Read(pattern)` 匹配 `path` - - `Write(pattern)` 匹配 `path` - - `Edit(pattern)` 匹配 `path` - - `ReadMediaFile(pattern)` 匹配 `path` - - `Grep(pattern)` 匹配 `pattern` - - `Glob(pattern)` 匹配 `pattern` - - `Agent(pattern)` 匹配 `subagent_type` - - `mcp__server__tool` / `mcp__server__*` 匹配 MCP tool name - - `ToolName` 无参数模式时只匹配 tool name - - `*(...)` 可作为全工具匹配模式 - - 路径 rule 匹配应基于 cwd 做规范化比较 - -- **Session Approval Memorized History** - - 记住的 `run command` action 命中 -> `approve` - - 记住的 `read file in cwd` action 命中 -> `approve` - - 记住的 `read file outside cwd` action 命中 -> `approve` - - 记住的 `write file in cwd` action 命中 -> `approve` - - 记住的 `write file outside cwd` action 命中 -> `approve` - - 记住的 `edit file in cwd` action 命中 -> `approve` - - 记住的 `edit file outside cwd` action 命中 -> `approve` - - 记住的 `stop background task` action 命中 -> `approve` - - 记住的 `call MCP tool: server:tool` action 命中 -> `approve` - - 记住的 `spawn agent` action 命中 -> `approve` - - 记住的 `invoke skill` action 命中 -> `approve` - - 记住的 action 必须按安全边界区分粒度，cwd 内外不能共用同一条 session rule - -- **System Deny Queue** - - `permissionMode=auto` 且 tool 是 `AskUserQuestion` -> `deny` - - plan mode active 且 `Write` 目标不是当前 plan file -> `deny` - - plan mode active 且 `Edit` 目标不是当前 plan file -> `deny` - - plan mode active 且当前没有 plan file path 时调用 `Write` -> `deny` - - plan mode active 且当前没有 plan file path 时调用 `Edit` -> `deny` - - plan mode active 且 tool 是 `TaskStop` -> `deny` - -- **System Ask Queue** - - `Read` 目标 path 在 cwd 外 -> `ask` - - `ReadMediaFile` 目标 path 在 cwd 外 -> `ask` - - `Write` 目标 path 在 cwd 外 -> `ask` - - `Edit` 目标 path 在 cwd 外 -> `ask` - - 目标 path 是敏感文件 `.env` / SSH private key / credentials 时 -> `ask` - - 目标 path 落在 `.git` 控制目录或 git control dir 内时 -> `ask` - - `ExitPlanMode` 且 plan mode active 且 plan 内容非空且 `permissionMode!=auto` -> `ask` - - 其他未来安全边界如果需要用户确认 -> `ask` - -- **System Approve Queue** - - `permissionMode=yolo` -> `approve` - - `permissionMode=auto` -> `approve` - - 默认 auto-allow tool `Read` -> `approve` - - 默认 auto-allow tool `Grep` -> `approve` - - 默认 auto-allow tool `Glob` -> `approve` - - 默认 auto-allow tool `ReadMediaFile` -> `approve` - - 默认 auto-allow tool `Think` -> `approve` - - 默认 auto-allow tool `SetTodoList` -> `approve` - - 默认 auto-allow tool `TaskList` -> `approve` - - 默认 auto-allow tool `TaskOutput` -> `approve` - - 默认 auto-allow tool `WebSearch` -> `approve` - - 默认 auto-allow tool `FetchURL` -> `approve` - - 默认 auto-allow tool `Agent` -> `approve` - - 默认 auto-allow tool `AskUserQuestion` -> `approve` - - 默认 auto-allow tool `EnterPlanMode` -> `approve` - - 默认 auto-allow tool `ExitPlanMode` -> `approve` - - 默认 auto-allow tool `Skill` -> `approve` - - `Write/Edit` 在 POSIX git cwd 内、目标在 cwd 内、不是敏感文件、不是 git 控制路径、路径中无 symlink -> `approve` - - `EnterPlanMode` -> `approve` - - `ExitPlanMode` 不在 plan mode active 状态 -> `approve` - - `ExitPlanMode` 在 plan mode active 但没有有效 plan 内容 -> `approve` - -- **Fallback** - - 以上全部未命中 -> `ask` - - `manual` 下的 `Bash` 通常靠 fallback -> `ask` - - `manual` 下的 `Write` 通常靠 fallback -> `ask` - - `manual` 下的 `Edit` 通常靠 fallback -> `ask` - - `manual` 下的 `TaskStop` 通常靠 fallback -> `ask` - - `manual` 下的 MCP tool / user tool 通常靠 fallback -> `ask` +本文记录 tool call permission 判断模型：一个已经通过前置校验、可以被执行的 tool call 如何被分类为 `approve` / `deny` / `ask`。本文不覆盖工具是否存在、是否启用、参数 schema 是否合法、ask 交互过程、以及工具执行时报错。 + +**设计目标：** + +- Permission policy 模块化、单队列。每个 H2 对应一个 `PermissionPolicy`，按文档顺序执行，首个命中的 policy 给出最终 decision。 +- Permission 层不硬编码每个 tool 的参数结构或 UI 文本。 + +**Execution contract：** + +- 文件安全边界由 `execution.accesses` 表达，policy 基于它判断 cwd 外、敏感文件、git control path 等规则 +- 用户配置规则的括号参数由 `execution.matchesRule(ruleArgs)` 解释 +- `matchesRule` 接收 `ToolName(...)` 括号内的原始字符串，不由通用 rule parser 拆分逗号或解释字段语义 +- `matchesRule` 必须是同步纯函数，只基于 `resolveExecution(args)` 已经解析好的信息判断，不做 IO +- 如果 execution 没有提供 `matchesRule`，则使用稳定序列化后的完整 tool args 作为 fallback subject；匹配语义参考 kimi-cli hook matcher：空 pattern 命中，非空 pattern 按 regex search 判断，非法 regex 不命中 +- fallback matching 是兼容机制，不表达 tool 专属语义；需要自然、精确的参数规则时，tool 应实现 `matchesRule` +- approval UI 使用 `execution.display` / `execution.description`；需要状态型审批 UI 的 tool（例如 `ExitPlanMode` 的 plan 内容与选项）也必须由自己的 `resolveExecution` 返回展示信息；`PermissionPolicy` 不返回、不拼接、不改写 UI 展示文本 + +**Telemetry contract：** + +- `PermissionPolicy` 不直接打 telemetry；单队列执行器在 policy 命中和 approval 完成时统一打点 +- `PermissionPolicy` 可以返回开放结构的 `reason` object；`reason` 只放额外的非敏感 primitive 摘要，没有额外信息时省略 +- Policy 命中时记录 `permission_policy_decision` + - `policy_name`: 命中的 policy name，例如 `cwd-outside-file-access-ask` + - `decision`: `approve` / `deny` / `ask` + - `tool_name`, `permission_mode` + - 如果 policy 返回 `reason`，展开记录其中的非敏感摘要，例如 file access operation、是否 cwd 外、是否敏感路径、是否 git control path +- 用户完成 ask 时记录 `permission_approval_result` + - `policy_name`: 发起 ask 的 policy name + - `result`: `approved` / `approved_for_session` / `rejected` / `cancelled` / `error` + - `tool_name`, `permission_mode` + - 可记录 `has_feedback`、`approval_surface`、`duration_ms`、`session_cache_written` +- `reason` 和 telemetry 都不记录 raw tool args、raw path、raw command、用户配置 rule 原文、approval UI 文本 +- 用户配置规则命中时只记录非敏感匹配摘要，例如 `rule_decision`、`has_rule_args`、`match_strategy`，其中 `match_strategy` 可为 `tool_name_only` / `matches_rule` / `stable_args_fallback` / `single_field_fallback` + +## pre-tool-call-hook: PreToolCall Hook Decision + +- Hook 返回 `block` -> `deny` +- Hook 返回 `allow` 或无结果 -> 继续后续判断 + +## auto-mode-ask-user-question-deny: Auto Mode AskUserQuestion Deny + +- `permissionMode=auto` 且 tool 是 `AskUserQuestion` -> `deny` + +## plan-mode-guard-deny: Plan Mode Guard Deny + +- plan mode active 且当前没有 plan file path 时调用 `Write`/`Edit` -> `deny` +- plan mode active 且 `Write`/`Edit` 目标不是当前 plan file -> `deny` +- plan mode active 且 tool 是 `TaskStop` -> `deny` + +## user-configured-deny: User Configured Deny Rules + +- 用户配置 `deny` rule 命中 -> `deny` +- 本阶段只处理用户配置 `deny` rule；`allow` rule 在 Auto Mode Approve 之后判断，`ask` rule 在 Session Approval Memorized History 之后判断 +- 用户规则按配置顺序扫描，命中第一条本阶段对应 decision 的 rule 即返回 +- `ToolName` 无括号参数时只匹配 tool name +- `ToolName(ruleArgs)` 先匹配 tool name；tool name 命中后调用 `execution.matchesRule(ruleArgs)`，返回 `true` 才算命中 +- 如果 rule 带括号参数但 execution 没有提供 `matchesRule`，则用 `ruleArgs` 匹配稳定序列化后的完整 tool args +- fallback subject 的稳定序列化必须让 object key 顺序不影响结果，数组顺序和字符串内容保持原样 +- fallback 匹配语义参考 kimi-cli hook matcher：空 `ruleArgs` 命中；非空 `ruleArgs` 作为 regex pattern 对 fallback subject 做 search；非法 regex 不命中；不是 fullmatch，也不是 literal substring +- 如果 tool args 是只有一个实际字段的 object，fallback 可以同时把该唯一字段的值作为第二个 subject，并使用同一套 regex search 语义匹配 +- `mcp__server__tool` / `mcp__server__*` 仍通过 tool name 匹配 MCP tool name +- `*(ruleArgs)` 可作为全工具匹配模式；tool name 命中所有工具，括号参数仍按 `matchesRule` 或 fallback matching 解释 +- 路径、命令、搜索表达式、agent type、skill identity 等参数语义由对应 tool 的 `resolveExecution` / `matchesRule` 定义，不在 policy 中硬编码 + +## auto-mode-approve: Auto Mode Approve + +- `permissionMode=auto` -> `approve` +- 任何在 auto mode 下也必须阻止的规则都必须表达为 deny，并放在本 policy 之前 + +## user-configured-allow: User Configured Allow Rules + +- 用户配置 `allow` rule 命中 -> `approve` +- 使用与 User Configured Deny Rules 相同的 rule 匹配方式 + +## session-approval-history: Session Approval Memorized History + +- `Approve for session` 记住的是本次 tool call 的 exact key：tool name + 完整 tool args +- 后续请求只有在 tool name 相同，且规范化后的完整 tool args 与已记住记录完全一致时 -> `approve` +- tool args 比较使用结构化数据的稳定序列化结果；object key 顺序不应影响匹配，数组顺序和字符串内容必须保持精确匹配 +- 当用户选择 `Approve for session` 时，把本次 tool name + 完整 tool args 写入 session history +- session history 只表达用户在本 session 内对同一 tool call 参数的临时批准，不等价于用户配置的 allow rule；持久化或跨 session 的信任应进入用户配置规则 + +## user-configured-ask: User Configured Ask Rules + +- 用户配置 `ask` rule 命中 -> `ask` + +## sensitive-file-access-ask: Sensitive File Access Ask + +- `execution.accesses` 中存在敏感文件 `.env` / SSH private key / credentials path -> `ask` + +## git-control-path-access-ask: Git Control Path Access Ask + +- `execution.accesses` 中存在 `.git` 控制目录或 git control dir path -> `ask` + +## cwd-outside-file-access-ask: CWD Outside File Access Ask + +- `execution.accesses` 中存在 `read` / `write` / `readwrite` / `search` file access，且目标 path 在 cwd 外 -> `ask` + +## exit-plan-mode-review-ask: ExitPlanMode Review Ask + +- `ExitPlanMode` 且 plan mode active 且 plan 内容非空且 `permissionMode!=auto` -> `ask` + +## yolo-mode-approve: YOLO Mode Approve + +- `permissionMode=yolo` -> `approve` + +## default-tool-approve: Default Tool Approve + +- 默认 `Read` / `Grep` / `Glob` / `ReadMediaFile` / `Think` / `SetTodoList` / `TaskList` / `TaskOutput` / `WebSearch` / `FetchURL` / `Agent` / `AskUserQuestion` / `Skill` -> `approve` + +## git-cwd-write-approve: Git CWD Write Approve + +- tool name 为 `Write` / `Edit`，且 `execution.accesses` 中的写目标在 POSIX git cwd 内、目标在 cwd 内 -> `approve` + +## plan-mode-tool-approve: Plan Mode Tool Approve + +- `EnterPlanMode` -> `approve` +- `ExitPlanMode` 不在 plan mode active 状态 -> `approve` +- `ExitPlanMode` 在 plan mode active 但没有有效 plan 内容 -> `approve` + +## fallback-ask: Fallback + +- 以上全部未命中 -> `ask` 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 0abfac3..c47957b 100644 --- a/packages/agent-core/src/agent/permission/index.ts +++ b/packages/agent-core/src/agent/permission/index.ts @@ -1,35 +1,43 @@ import type { Agent } from '..'; -import type { PrepareToolExecutionResult, ToolExecutionHookContext } from '../../loop'; -import type { TelemetryPropertyValue } from '../../telemetry'; -import { isDefaultAutoAllowTool } from '../../tools/policies/default-permissions'; +import type { PrepareToolExecutionResult } from '../../loop'; +import type { TelemetryProperties } from '../../telemetry'; 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 { createPermissionDecisionPolicies } from './policies'; -import type { PermissionPolicy, PermissionPolicyResult } from './policy'; import type { + PermissionDecisionReason, + PermissionPolicy, + PermissionPolicyContext, + PermissionPolicyResolution, + PermissionPolicyResult, + PermissionReasonValue, +} from './policy'; +import type { + ApprovalResponse, PermissionApprovalResultRecord, PermissionData, PermissionMode, PermissionRule, } from './types'; +import { stableToolArgsKey } from './stable-args'; + export * from './policy'; export * from './types'; -type ApprovalTelemetryMode = 'manual' | 'yolo' | 'afk' | 'auto_session' | 'cancelled'; - 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 sessionApprovedKeys = new Set(); private readonly policies: readonly PermissionPolicy[]; constructor( @@ -38,7 +46,7 @@ export class PermissionManager { ) { this.rules = [...(options.initialRules ?? [])]; this.parent = options.parent; - this.policies = options.policies ?? createPermissionDecisionPolicies(this.agent); + this.policies = createPermissionDecisionPolicies(this.agent); } get mode(): PermissionMode { @@ -81,187 +89,134 @@ 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); - if (pattern === undefined) return; + if (record.sessionApprovalKey === undefined) return; + this.sessionApprovedKeys.add(record.sessionApprovalKey); + } - const rule: PermissionRule = { - decision: 'allow', - scope: 'session-runtime', - pattern, - reason: `approve_for_session: ${record.action}`, - }; - if (!this.hasRule(rule)) { - this.rules.push(rule); - } + hasSessionApprovedKey(key: string): boolean { + return this.sessionApprovedKeys.has(key) || this.parent?.hasSessionApprovedKey(key) === true; } async beforeToolCall( - context: ToolExecutionHookContext, + context: PermissionPolicyContext, ): Promise { - const name = context.toolCall.function.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.trackPolicyDecision(evaluation.policyName, context, evaluation.result); + 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.function.name; - const args = context.args; - const display = - options.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; - } - - const result = await this.agent.rpc.requestApproval( - { - turnId: Number(context.turnId), - toolCallId: id, + const display = approvalDisplayForExecution(name, context.execution); + const action = approvalActionForExecution(name, context.execution); + const sessionApprovalKey = stableToolArgsKey(name, context.args); + 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.trackApprovalResult({ + policyName, toolName: name, - action, display, - }, - { signal }, - ); + result: 'error', + durationMs: Date.now() - startedAt, + sessionCacheWritten: false, + }); + const resolved = result.resolveError?.(error); + return resolved === undefined + ? Promise.reject(error) + : this.permissionPolicyResolutionToPrepare(resolved, context, policyName); + } + this.recordApprovalResult({ turnId: Number(context.turnId), toolCallId: id, toolName: name, action, - result, + sessionApprovalKey, + result: response, + }); + this.trackApprovalResult({ + policyName, + toolName: name, + display, + result: approvalTelemetryResult(response), + durationMs: Date.now() - startedAt, + sessionCacheWritten: response.decision === 'approved' && response.scope === 'session', + hasFeedback: 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({ - ...context, - matchedRule, - }); - 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 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.function.name), + }; case 'ask': - return this.requestToolApproval(context, result); + return this.requestToolApproval(context, result, policyName); case 'result': { const { kind: _kind, ...prepareResult } = result; return prepareResult; @@ -269,25 +224,6 @@ export class PermissionManager { } } - 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 `Tool "${toolName}" was denied by permission rule.${suffix}`; - } - protected formatApprovalRejectionMessage( toolName: string, result: { decision: 'approved' | 'rejected' | 'cancelled'; feedback?: string }, @@ -306,32 +242,100 @@ 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 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.`; + } + return prefix; } - private trackToolApproved( - toolName: string, - approvalMode: Exclude, - scope?: 'once' | 'session', + private trackPolicyDecision( + policyName: string, + context: PermissionPolicyContext, + result: PermissionPolicyResult, ): void { - const properties: Record = { - tool_name: toolName, - approval_mode: approvalMode, + const properties: Record = { + policy_name: policyName, + tool_name: context.toolCall.function.name, + permission_mode: this.mode, + decision: result.kind, + }; + addReasonProperties(properties, result.reason); + if (result.kind === 'ask') { + properties['approval_surface'] = context.execution.display?.kind ?? null; + } + if (result.kind === 'approve') { + properties['has_execution_metadata'] = result.executionMetadata !== undefined; + } + this.agent.telemetry.track('permission_policy_decision', properties); + } + + private trackApprovalResult(input: { + readonly policyName: string | undefined; + readonly toolName: string; + readonly display: ToolInputDisplay; + readonly result: 'approved' | 'approved_for_session' | 'rejected' | 'cancelled' | 'error'; + readonly durationMs: number; + readonly sessionCacheWritten: boolean; + readonly hasFeedback?: boolean; + }): void { + const properties: Record = { + policy_name: input.policyName ?? null, + tool_name: input.toolName, + permission_mode: this.mode, + result: input.result, + approval_surface: input.display.kind, + duration_ms: input.durationMs, + session_cache_written: input.sessionCacheWritten, + has_feedback: input.hasFeedback === true, }; - if (scope !== undefined) { - properties['scope'] = scope; + this.agent.telemetry.track('permission_approval_result', properties); + } +} + +function approvalDisplayForExecution( + toolName: string, + execution: PermissionPolicyContext['execution'], +): ToolInputDisplay { + return ( + execution.display ?? { + kind: 'generic', + summary: execution.description ?? `Approve ${toolName}`, } - this.agent.telemetry.track('tool_approved', properties); + ); +} + +function approvalActionForExecution( + toolName: string, + execution: PermissionPolicyContext['execution'], +): string { + return execution.description ?? `Call ${toolName}`; +} + +function approvalTelemetryResult( + result: ApprovalResponse, +): 'approved' | 'approved_for_session' | 'rejected' | 'cancelled' { + if (result.decision === 'approved' && result.scope === 'session') return 'approved_for_session'; + return result.decision; +} + +function addReasonProperties( + properties: Record, + reason: PermissionDecisionReason | undefined, +): void { + if (reason === undefined) return; + for (const [key, value] of Object.entries(reason)) { + if (!isReasonTelemetryValue(value)) continue; + properties[key] = value; } } -function approvalTelemetryMode( - mode: PermissionMode, -): Extract { - return mode === 'auto' ? 'afk' : mode; +function isReasonTelemetryValue(value: unknown): value is PermissionReasonValue { + return ( + value === null || + typeof value === 'string' || + typeof value === 'number' || + typeof value === 'boolean' + ); } diff --git a/packages/agent-core/src/agent/permission/matches-rule.ts b/packages/agent-core/src/agent/permission/matches-rule.ts index 546567b..d8ef0a9 100644 --- a/packages/agent-core/src/agent/permission/matches-rule.ts +++ b/packages/agent-core/src/agent/permission/matches-rule.ts @@ -1,125 +1,95 @@ -/** - * matchesRule — pure function that decides whether a PermissionRule - * applies to a given tool call. - * - * 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`). - */ - import picomatch from 'picomatch'; +import type { RunnableToolExecution } from '../../loop/types'; +import { matchesRuleSubject } from '../../tools/support/rule-match'; import { parsePattern } from './parse-pattern'; -import { globMatch, pathGlobMatch, type PermissionPathMatchOptions } from './path-glob-match'; +import { stableSerialize } from './stable-args'; import type { PermissionRule } from './types'; -type ArgFieldKind = 'generic' | 'path'; - -interface ArgField { - readonly value: string; - readonly kind: ArgFieldKind; +export interface PermissionRuleMatchExecution { + readonly matchesRule?: RunnableToolExecution['matchesRule']; + readonly cwd?: unknown; + readonly pathClass?: unknown; } -/** - * 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. - */ -function extractArgField(toolName: string, args: unknown): ArgField | undefined { - if (args === null || typeof args !== 'object') return undefined; - const rec = args as Record; +export type PermissionRuleMatchStrategy = + | 'tool_name_only' + | 'matches_rule' + | 'stable_args_fallback' + | 'single_field_fallback'; - 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; - } +export interface PermissionRuleMatch { + readonly rule: PermissionRule; + readonly strategy: PermissionRuleMatchStrategy; + readonly hasRuleArgs: boolean; } -/** - * 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 interface PermissionRuleMatchInput { + readonly rule: PermissionRule; + readonly toolName: string; + readonly args: unknown; + readonly execution: PermissionRuleMatchExecution; +} + +export function matchPermissionRule({ + rule, + toolName, + args, + 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; + if (parsed.toolName !== '*' && !picomatch.isMatch(toolName, parsed.toolName)) { + return undefined; + } - // 2. No arg pattern → name match is enough - if (parsed.argPattern === undefined) return true; + if (parsed.argPattern === undefined) { + return { rule, strategy: 'tool_name_only', hasRuleArgs: false }; + } - // 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 (execution.matchesRule !== undefined) { + return execution.matchesRule(parsed.argPattern) + ? { rule, strategy: 'matches_rule', hasRuleArgs: true } + : 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 (matchesRuleSubject(parsed.argPattern, stableSerialize(args))) { + return { rule, strategy: 'stable_args_fallback', hasRuleArgs: true }; } - // 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; + const singleField = singleActualFieldValue(args); + if ( + singleField !== undefined && + matchesRuleSubject(parsed.argPattern, singleFieldSubject(singleField)) + ) { + return { rule, strategy: 'single_field_fallback', hasRuleArgs: true }; + } + + return undefined; +} + +export function matchesRule( + rule: PermissionRule, + toolName: string, + args: unknown, + execution: PermissionRuleMatchExecution = {}, +): boolean { + return matchPermissionRule({ rule, toolName, args, execution }) !== undefined; } -export type { PermissionPathMatchOptions } from './path-glob-match'; +function singleActualFieldValue(args: unknown): unknown { + if (args === null || typeof args !== 'object' || Array.isArray(args)) return undefined; + const entries = Object.entries(args as Record).filter( + ([, value]) => typeof value !== 'undefined', + ); + return entries.length === 1 ? entries[0]![1] : undefined; +} + +function singleFieldSubject(value: unknown): string { + return typeof value === 'string' ? value : stableSerialize(value); +} diff --git a/packages/agent-core/src/agent/permission/parse-pattern.ts b/packages/agent-core/src/agent/permission/parse-pattern.ts index 9b76ebd..e483f96 100644 --- a/packages/agent-core/src/agent/permission/parse-pattern.ts +++ b/packages/agent-core/src/agent/permission/parse-pattern.ts @@ -15,7 +15,7 @@ export interface ParsedPattern { readonly toolName: string; - readonly argPattern?: string | undefined; + readonly argPattern?: string; } /** @@ -43,8 +43,5 @@ export function parsePattern(pattern: string): ParsedPattern { 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 }; + return { toolName, argPattern }; } 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..8a5ad48 --- /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 '../policy'; + +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 undefined; + return { + kind: 'approve', + }; + } +} diff --git a/packages/agent-core/src/agent/permission/policies/ask-user-question.ts b/packages/agent-core/src/agent/permission/policies/auto-mode-ask-user-question-deny.ts similarity index 76% rename from packages/agent-core/src/agent/permission/policies/ask-user-question.ts rename to packages/agent-core/src/agent/permission/policies/auto-mode-ask-user-question-deny.ts index fa0cf77..3d8602d 100644 --- a/packages/agent-core/src/agent/permission/policies/ask-user-question.ts +++ b/packages/agent-core/src/agent/permission/policies/auto-mode-ask-user-question-deny.ts @@ -1,8 +1,8 @@ import type { Agent } from '../..'; import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; -export class AskUserQuestionAutoPermissionPolicy implements PermissionPolicy { - readonly name = 'auto.ask-user-question'; +export class AutoModeAskUserQuestionDenyPermissionPolicy implements PermissionPolicy { + readonly name = 'auto-mode-ask-user-question-deny'; constructor(private readonly agent: Agent) {} @@ -10,9 +10,8 @@ export class AskUserQuestionAutoPermissionPolicy implements PermissionPolicy { if (this.agent.permission.mode !== 'auto') return undefined; if (context.toolCall.function.name !== 'AskUserQuestion') return undefined; return { - kind: 'result', - block: true, - reason: + 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/cwd-outside-ask.ts b/packages/agent-core/src/agent/permission/policies/cwd-outside-ask.ts deleted file mode 100644 index d413e3d..0000000 --- a/packages/agent-core/src/agent/permission/policies/cwd-outside-ask.ts +++ /dev/null @@ -1,35 +0,0 @@ -import type { Agent } from '../..'; -import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; -import { fileInputDisplay, resolvePathToolAccess } from './utils'; - -export class CwdOutsideAskPermissionPolicy implements PermissionPolicy { - readonly name = 'system.ask.cwd-outside-path'; - - constructor(private readonly agent: Agent) {} - - evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { - const access = resolvePathToolAccess(this.agent, context); - if (access === undefined || !access.outsideCwd) return undefined; - - return { - kind: 'ask', - action: outsideCwdAction(context.toolCall.function.name), - display: fileInputDisplay(access, 'Path is outside the current working directory.'), - }; - } -} - -function outsideCwdAction(tool: string): string { - switch (tool) { - case 'Read': - return 'read file outside of working directory'; - case 'ReadMediaFile': - return 'read media file outside of working directory'; - case 'Write': - return 'write file outside of working directory'; - case 'Edit': - return 'edit file outside of working directory'; - default: - return `call ${tool} outside of working directory`; - } -} diff --git a/packages/agent-core/src/agent/permission/policies/default-auto-allow.ts b/packages/agent-core/src/agent/permission/policies/default-auto-allow.ts deleted file mode 100644 index 74df9b1..0000000 --- a/packages/agent-core/src/agent/permission/policies/default-auto-allow.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { isDefaultAutoAllowTool } from '../../../tools/policies/default-permissions'; -import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; - -export class DefaultAutoAllowPermissionPolicy implements PermissionPolicy { - readonly name = 'system.approve.default-auto-allow'; - - evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { - if (!isDefaultAutoAllowTool(context.toolCall.function.name)) return undefined; - return { kind: 'allow' }; - } -} 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 c357d97..0000000 --- a/packages/agent-core/src/agent/permission/policies/default-git-cwd-write.ts +++ /dev/null @@ -1,132 +0,0 @@ -import * as posixPath from 'node:path/posix'; - -import type { Kaos } from '@moonshot-ai/kaos'; - -import type { Agent } from '../..'; -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, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; - -const AUTO_REASON = 'default_git_cwd_write'; -const S_IFMT = 0o170000; -const S_IFLNK = 0o120000; - -export class DefaultGitCwdWritePermissionPolicy implements 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. - private readonly cache = new Map(); - readonly name = 'default.git-cwd-write'; - - constructor(private readonly agent: Agent) {} - - async evaluate({ - matchedRule, - toolCall, - args, - }: PermissionPolicyContext): Promise { - if (this.agent.permission.mode !== 'manual') return undefined; - if (matchedRule !== undefined) return undefined; - - const name = toolCall.function.name; - if (name !== 'Write' && name !== 'Edit') return undefined; - - const kaos = this.agent.runtime.kaos; - const pathClass = kaos.pathClass(); - if (pathClass !== 'posix') return undefined; - - const cwd = this.agent.config.cwd; - if (cwd.length === 0) return undefined; - - const path = readStringField(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 = this.cache.get(cwd) ?? (await findGitWorkTreeMarker(kaos, cwd)); - if (marker === null) return undefined; - this.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; - - this.agent.telemetry.track('tool_approved', { - tool_name: name, - 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..f5576b8 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/default-tool-approve.ts @@ -0,0 +1,29 @@ +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; + +const DEFAULT_APPROVE_TOOLS = new Set([ + 'Read', + 'Grep', + 'Glob', + 'ReadMediaFile', + 'Think', + '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.function.name)) return undefined; + 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..70c468a --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/exit-plan-mode-review-ask.ts @@ -0,0 +1,130 @@ +import type { Agent } from '../..'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; +import type { ApprovalResponse } from '../types'; + +interface ExitPlanModeOption { + readonly label: string; + readonly description: string; +} + +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.function.name !== 'ExitPlanMode') return undefined; + if (this.agent.permission.mode === 'auto') return undefined; + if (!this.agent.planMode.isActive) return undefined; + const display = context.execution.display; + if (display?.kind !== 'plan_review') return undefined; + if (display.plan.trim().length === 0) return undefined; + return { + kind: 'ask', + reason: { + has_options: display.options !== undefined, + }, + resolveApproval: (result) => exitPlanModeApprovalResult(this.agent, result, display.options), + resolveError: (error) => { + const message = error instanceof Error ? error.message : 'Plan approval failed.'; + return { + kind: 'result', + syntheticResult: { + isError: true, + output: `Plan approval failed: ${message}`, + }, + }; + }, + }; + } +} + +function exitPlanModeApprovalResult( + agent: Agent, + result: ApprovalResponse, + options: readonly ExitPlanModeOption[] | undefined, +) { + if (result.decision === 'approved') { + const selected = selectedExitPlanModeOption(options, result.selectedLabel); + return { + kind: 'approve' as const, + executionMetadata: + selected === undefined + ? { + planTelemetrySubmitted: true, + planTelemetryResolved: true, + } + : { + selectedOption: selected, + planTelemetrySubmitted: true, + planTelemetryResolved: true, + }, + }; + } + + 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 = exitPlanModeForRejectedPlan(agent); + 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.', + }, + }; +} + +function exitPlanModeForRejectedPlan(agent: Agent) { + try { + 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 selectedExitPlanModeOption( + options: readonly ExitPlanModeOption[] | undefined, + label: string | undefined, +): ExitPlanModeOption | undefined { + if (options === undefined || label === undefined) return undefined; + 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 index 0a82606..7e4de3f 100644 --- a/packages/agent-core/src/agent/permission/policies/fallback-ask.ts +++ b/packages/agent-core/src/agent/permission/policies/fallback-ask.ts @@ -1,9 +1,11 @@ -import type { PermissionPolicy, PermissionPolicyResult } from '../policy'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; export class FallbackAskPermissionPolicy implements PermissionPolicy { - readonly name = 'fallback.ask'; + readonly name = 'fallback-ask'; - evaluate(): PermissionPolicyResult { - return { kind: '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..474269c --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/file-access-ask.ts @@ -0,0 +1,128 @@ +import * as posixPath from 'node:path/posix'; +import * as win32Path from 'node:path/win32'; + +import type { Agent } from '../..'; +import type { ToolResourceAccess } 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 '../policy'; + +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 = firstFileAccess(context, (fileAccess) => + fileAccess.path === undefined ? false : isSensitiveFile(fileAccess.path, pathClass), + ); + if (access === undefined) return undefined; + 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 undefined; + const pathClass = this.agent.runtime.kaos.pathClass(); + const marker = await this.findGitMarker(cwd); + const access = firstFileAccess(context, (fileAccess) => { + if (fileAccess.path === undefined) return false; + return isGitControlPath(fileAccess.path, cwd, marker, pathClass); + }); + if (access === undefined) return undefined; + return { + kind: 'ask', + reason: fileAccessReason(access, { git_control_path: true }), + }; + } + + private async findGitMarker(cwd: string): Promise { + const cached = this.gitMarkerCache.get(cwd); + if (cached !== undefined) return cached; + const marker = await findGitWorkTreeMarker(this.agent.runtime.kaos, cwd); + if (marker !== null) this.gitMarkerCache.set(cwd, marker); + return marker; + } +} + +export class CwdOutsideFileAccessAskPermissionPolicy implements PermissionPolicy { + readonly name = 'cwd-outside-file-access-ask'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const cwd = this.agent.config.cwd; + if (cwd.length === 0) return undefined; + const pathClass = this.agent.runtime.kaos.pathClass(); + const access = firstFileAccess(context, (fileAccess) => { + if (fileAccess.path === undefined) return false; + return !isWithinDirectory(fileAccess.path, cwd, pathClass); + }); + if (access === undefined) return undefined; + return { + kind: 'ask', + reason: fileAccessReason(access, { cwd_outside: true }), + }; + } +} + +type FileAccess = Extract; + +function firstFileAccess( + context: PermissionPolicyContext, + predicate: (access: FileAccess) => boolean, +): FileAccess | undefined { + return context.execution.accesses?.find( + (access): access is FileAccess => access.kind === 'file' && predicate(access), + ); +} + +function fileAccessReason(access: FileAccess, extra: Record) { + return { + file_access_operation: access.operation, + recursive: access.recursive === true, + ...extra, + }; +} + +function isGitControlPath( + targetPath: string, + cwd: string, + marker: GitWorkTreeMarker | null, + pathClass: PathClass, +): boolean { + if (relativePathParts(targetPath, cwd, pathClass).some((part) => part.toLowerCase() === '.git')) { + return true; + } + return ( + marker !== null && + (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..8c58785 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/git-cwd-write-approve.ts @@ -0,0 +1,53 @@ +import type { Agent } from '../..'; +import type { ToolResourceAccess } from '../../../loop/tool-access'; +import { isWithinDirectory } from '../../../tools/policies/path-access'; +import { + findGitWorkTreeMarker, + type GitWorkTreeMarker, +} from '../../../tools/support/git-worktree'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; + +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.function.name; + if (toolName !== 'Write' && toolName !== 'Edit') return undefined; + if (this.agent.runtime.kaos.pathClass() !== 'posix') return undefined; + + const cwd = this.agent.config.cwd; + if (cwd.length === 0) return undefined; + + const writeAccesses = + context.execution.accesses?.filter( + (access): access is FileAccess => + access.kind === 'file' && + access.path !== undefined && + (access.operation === 'write' || access.operation === 'readwrite'), + ) ?? []; + if (writeAccesses.length === 0) return undefined; + if (!writeAccesses.every((access) => isWithinDirectory(access.path, cwd, 'posix'))) { + return undefined; + } + + const marker = await this.findGitMarker(cwd); + if (marker === null) return undefined; + + return { + kind: 'approve', + }; + } + + private async findGitMarker(cwd: string): Promise { + const cached = this.gitMarkerCache.get(cwd); + if (cached !== undefined) return cached; + const marker = await findGitWorkTreeMarker(this.agent.runtime.kaos, cwd); + if (marker !== null) this.gitMarkerCache.set(cwd, marker); + return marker; + } +} + +type FileAccess = Extract & { readonly path: string }; diff --git a/packages/agent-core/src/agent/permission/policies/index.ts b/packages/agent-core/src/agent/permission/policies/index.ts index 3be0101..7a76b67 100644 --- a/packages/agent-core/src/agent/permission/policies/index.ts +++ b/packages/agent-core/src/agent/permission/policies/index.ts @@ -1,33 +1,45 @@ import type { Agent } from '../..'; import type { PermissionPolicy } from '../policy'; -import { AskUserQuestionAutoPermissionPolicy } from './ask-user-question'; -import { CwdOutsideAskPermissionPolicy } from './cwd-outside-ask'; -import { DefaultAutoAllowPermissionPolicy } from './default-auto-allow'; -import { DefaultGitCwdWritePermissionPolicy } from './default-git-cwd-write'; +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 { PermissionModeApprovePolicy } from './permission-mode-approve'; import { - EnterPlanModePermissionPolicy, - ExitPlanModePermissionPolicy, - PlanModeGuardPermissionPolicy, -} from './plan'; + CwdOutsideFileAccessAskPermissionPolicy, + 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 { SystemSafetyPathAskPermissionPolicy } from './system-safety-path-ask'; -import { UserConfiguredPermissionRulesPolicy } from './user-configured-rules'; +import { + UserConfiguredAllowPermissionPolicy, + UserConfiguredAskPermissionPolicy, + UserConfiguredDenyPermissionPolicy, +} from './user-configured-rules'; +import { YoloModeApprovePermissionPolicy } from './yolo-mode-approve'; export function createPermissionDecisionPolicies(agent: Agent): readonly PermissionPolicy[] { return [ - new UserConfiguredPermissionRulesPolicy(agent), - new AskUserQuestionAutoPermissionPolicy(agent), - new PlanModeGuardPermissionPolicy(agent), - new SystemSafetyPathAskPermissionPolicy(agent), + new PreToolCallHookPermissionPolicy(agent), + new AutoModeAskUserQuestionDenyPermissionPolicy(agent), + new PlanModeGuardDenyPermissionPolicy(agent), + new UserConfiguredDenyPermissionPolicy(agent), + new AutoModeApprovePermissionPolicy(agent), + new UserConfiguredAllowPermissionPolicy(agent), new SessionApprovalHistoryPermissionPolicy(agent), - new CwdOutsideAskPermissionPolicy(agent), - new ExitPlanModePermissionPolicy(agent), - new PermissionModeApprovePolicy(agent), - new EnterPlanModePermissionPolicy(), - new DefaultAutoAllowPermissionPolicy(), - new DefaultGitCwdWritePermissionPolicy(agent), + new UserConfiguredAskPermissionPolicy(agent), + new SensitiveFileAccessAskPermissionPolicy(agent), + new GitControlPathAccessAskPermissionPolicy(agent), + new CwdOutsideFileAccessAskPermissionPolicy(agent), + new ExitPlanModeReviewAskPermissionPolicy(agent), + new YoloModeApprovePermissionPolicy(agent), + new DefaultToolApprovePermissionPolicy(), + new GitCwdWriteApprovePermissionPolicy(agent), + new PlanModeToolApprovePermissionPolicy(), new FallbackAskPermissionPolicy(), ]; } diff --git a/packages/agent-core/src/agent/permission/policies/permission-mode-approve.ts b/packages/agent-core/src/agent/permission/policies/permission-mode-approve.ts deleted file mode 100644 index 79b9ae1..0000000 --- a/packages/agent-core/src/agent/permission/policies/permission-mode-approve.ts +++ /dev/null @@ -1,21 +0,0 @@ -import type { Agent } from '../..'; -import { isDefaultAutoAllowTool } from '../../../tools/policies/default-permissions'; -import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; - -export class PermissionModeApprovePolicy implements PermissionPolicy { - readonly name = 'system.approve.permission-mode'; - - constructor(private readonly agent: Agent) {} - - evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { - const mode = this.agent.permission.mode; - if (mode !== 'yolo' && mode !== 'auto') return undefined; - const name = context.toolCall.function.name; - if (isDefaultAutoAllowTool(name)) return undefined; - this.agent.telemetry.track('tool_approved', { - tool_name: name, - approval_mode: mode === 'auto' ? 'afk' : 'yolo', - }); - return { kind: 'allow' }; - } -} 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..d9d6be2 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/plan-mode-guard-deny.ts @@ -0,0 +1,62 @@ +import type { Agent } from '../..'; +import type { ToolResourceAccess } from '../../../loop/tool-access'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; + +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 undefined; + + const toolName = context.toolCall.function.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 undefined; + } + return { + kind: 'deny', + message: planModeWriteDeniedMessage(planFilePath), + }; + } + + if (toolName !== 'TaskStop') return undefined; + 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 = + context.execution.accesses?.filter( + (access): access is FileAccess => + access.kind === 'file' && + access.path !== undefined && + (access.operation === 'write' || access.operation === 'readwrite'), + ) ?? []; + if (writeAccesses.length === 0) return false; + return writeAccesses.every((access) => access.path === planFilePath); +} + +type FileAccess = Extract & { readonly path: string }; + +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..ea67e85 --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/plan-mode-tool-approve.ts @@ -0,0 +1,25 @@ +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; + +export class PlanModeToolApprovePermissionPolicy implements PermissionPolicy { + readonly name = 'plan-mode-tool-approve'; + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const toolName = context.toolCall.function.name; + if (toolName === 'EnterPlanMode') { + return { + kind: 'approve', + }; + } + + if (toolName !== 'ExitPlanMode') return undefined; + if (context.execution.display?.kind !== 'plan_review') { + return { + kind: 'approve', + }; + } + if (context.execution.display.plan.trim().length > 0) return undefined; + return { + kind: 'approve', + }; + } +} 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 85cf9bb..0000000 --- a/packages/agent-core/src/agent/permission/policies/plan.ts +++ /dev/null @@ -1,308 +0,0 @@ -import type { Agent } from '../..'; -import type { ExecutableToolResult } from '../../../loop'; -import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; -import type { ApprovalResponse } from '../types'; -import { readStringField } from './utils'; - -interface ExitPlanModeOption { - readonly label: string; - readonly description: string; -} - -interface ExitPlanModeExecutionMetadata { - readonly selectedOption?: ExitPlanModeOption | undefined; - readonly planTelemetrySubmitted: true; - readonly planTelemetryResolved: true; -} - -export class EnterPlanModePermissionPolicy implements PermissionPolicy { - readonly name = 'plan.enter-plan-mode'; - - evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { - if (context.toolCall.function.name !== 'EnterPlanMode') return undefined; - return { kind: 'allow' }; - } -} - -export class ExitPlanModePermissionPolicy implements PermissionPolicy { - readonly name = 'plan.exit-plan-mode'; - - constructor(private readonly agent: Agent) {} - - async evaluate(context: PermissionPolicyContext): Promise { - const agent = this.agent; - if (context.toolCall.function.name !== 'ExitPlanMode') return undefined; - if (agent.permission.mode === 'auto') return { kind: 'allow' }; - - const review = await resolveExitPlanModeReview(agent, context); - if (review === null) return { kind: 'allow' }; - - const action = exitPlanModeAction(review.options); - agent.telemetry.track('plan_submitted', { - has_options: review.options !== undefined, - }); - let result: ApprovalResponse; - try { - result = await agent.rpc.requestApproval( - { - turnId: Number(context.turnId), - toolCallId: context.toolCall.id, - toolName: 'ExitPlanMode', - action, - display: { - kind: 'plan_review', - plan: review.plan, - path: review.path, - options: review.options, - }, - }, - { signal: context.signal }, - ); - } catch (error) { - const message = error instanceof Error ? error.message : 'Plan approval failed.'; - return { - kind: 'result', - syntheticResult: { - isError: true, - output: `Plan approval failed: ${message}`, - }, - }; - } - - agent.permission.recordApprovalResult({ - turnId: Number(context.turnId), - toolCallId: context.toolCall.id, - toolName: 'ExitPlanMode', - action, - result, - }); - - trackExitPlanModeResolution(agent, result); - return exitPlanModeApprovalResult(agent, result, review.options); - } -} - -export class PlanModeGuardPermissionPolicy implements PermissionPolicy { - readonly name = 'plan.mode-guard'; - - constructor(private readonly agent: Agent) {} - - evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { - const agent = this.agent; - if (!agent.planMode.isActive) return undefined; - - const name = context.toolCall.function.name; - const args = context.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', - 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', - 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(agent: Agent): readonly PermissionPolicy[] { - return [ - new EnterPlanModePermissionPolicy(), - new ExitPlanModePermissionPolicy(agent), - new PlanModeGuardPermissionPolicy(agent), - ]; -} - -async function resolveExitPlanModeReview(agent: Agent, context: PermissionPolicyContext): Promise<{ - readonly plan: string; - readonly path?: string | undefined; - readonly options?: readonly ExitPlanModeOption[] | undefined; -} | null> { - if (!agent.planMode.isActive) return null; - - let data: Awaited>; - try { - data = await 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.args), - }; -} - -function exitPlanModeApprovalResult( - agent: Agent, - 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', - syntheticResult: { - isError: false, - output: 'Plan approval dismissed. Plan mode remains active.', - }, - }; - } - - if (result.selectedLabel === 'Reject and Exit') { - const failed = exitPlanModeForRejectedPlan(agent); - return { - kind: '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', - 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', - 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(agent: Agent, result: ApprovalResponse): void { - const selectedLabel = result.selectedLabel ?? ''; - const normalizedSelectedLabel = normalizeOptionLabel(selectedLabel); - const feedback = result.feedback ?? ''; - const hasFeedback = feedback.length > 0; - - if (result.decision === 'cancelled') { - agent.telemetry.track('plan_resolved', { outcome: 'dismissed' }); - return; - } - - if (result.decision === 'approved') { - if (selectedLabel.length > 0) { - agent.telemetry.track('plan_resolved', { - outcome: 'approved', - chosen_option: selectedLabel, - }); - return; - } - agent.telemetry.track('plan_resolved', { outcome: 'approved' }); - return; - } - - if (normalizedSelectedLabel === 'reject and exit') { - agent.telemetry.track('plan_resolved', { outcome: 'rejected_and_exited' }); - return; - } - - if (normalizedSelectedLabel === 'revise' || hasFeedback) { - agent.telemetry.track('plan_resolved', { - outcome: 'revise', - has_feedback: hasFeedback, - }); - return; - } - - agent.telemetry.track('plan_resolved', { outcome: 'rejected' }); -} - -function exitPlanModeForRejectedPlan(agent: Agent): ExecutableToolResult | undefined { - try { - 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; - 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(); -} 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..86cf3bd --- /dev/null +++ b/packages/agent-core/src/agent/permission/policies/pre-tool-call-hook.ts @@ -0,0 +1,36 @@ +import type { Agent } from '../..'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; + +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.function.name, + signal: context.signal, + inputData: { + toolName: context.toolCall.function.name, + toolInput: toolInputRecord(context.args), + toolCallId: context.toolCall.id, + }, + }); + context.signal.throwIfAborted(); + if (hookResult === undefined) return undefined; + return { + kind: 'deny', + message: hookResult.reason, + }; + } +} + +function toolInputRecord(args: unknown): Record { + return isPlainRecord(args) ? args : {}; +} + +function isPlainRecord(value: unknown): value is Record { + if (value === null || typeof value !== 'object') return false; + const proto = Object.getPrototypeOf(value); + return proto === Object.prototype || proto === null; +} 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 index efb0323..1fb401b 100644 --- a/packages/agent-core/src/agent/permission/policies/session-approval-history.ts +++ b/packages/agent-core/src/agent/permission/policies/session-approval-history.ts @@ -1,35 +1,17 @@ import type { Agent } from '../..'; import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; -import { firstMatchingRuleDecision } from './utils'; +import { stableToolArgsKey } from '../stable-args'; export class SessionApprovalHistoryPermissionPolicy implements PermissionPolicy { - readonly name = 'session.approval-history'; + readonly name = 'session-approval-history'; constructor(private readonly agent: Agent) {} evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { - if ( - context.matchedRule?.scope === 'session-runtime' && - context.matchedRule.decision === 'allow' - ) { - trackSessionApproval(this.agent, context); - return { kind: 'allow' }; - } - - const sessionRules = this.agent.permission.rules.filter( - (rule) => rule.scope === 'session-runtime', - ); - const match = firstMatchingRuleDecision(sessionRules, this.agent, context); - if (match?.decision !== 'allow') return undefined; - trackSessionApproval(this.agent, context); - return { kind: 'allow' }; + const key = stableToolArgsKey(context.toolCall.function.name, context.args); + if (!this.agent.permission.hasSessionApprovedKey(key)) return undefined; + return { + kind: 'approve', + }; } } - -function trackSessionApproval(agent: Agent, context: PermissionPolicyContext): void { - agent.telemetry.track('tool_approved', { - tool_name: context.toolCall.function.name, - approval_mode: 'auto_session', - scope: 'session', - }); -} diff --git a/packages/agent-core/src/agent/permission/policies/system-safety-path-ask.ts b/packages/agent-core/src/agent/permission/policies/system-safety-path-ask.ts deleted file mode 100644 index 372e3c1..0000000 --- a/packages/agent-core/src/agent/permission/policies/system-safety-path-ask.ts +++ /dev/null @@ -1,90 +0,0 @@ -import * as posixPath from 'node:path/posix'; -import * as win32Path from 'node:path/win32'; - -import type { Agent } from '../..'; -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 '../policy'; -import { fileInputDisplay, resolvePathToolAccess } from './utils'; - -export class SystemSafetyPathAskPermissionPolicy implements PermissionPolicy { - private readonly gitMarkerCache = new Map(); - readonly name = 'system.ask.safety-path'; - - constructor(private readonly agent: Agent) {} - - async evaluate(context: PermissionPolicyContext): Promise { - const access = resolvePathToolAccess(this.agent, context); - if (access === undefined) return undefined; - - if (isSensitiveFile(access.path, access.pathClass)) { - return { - kind: 'ask', - action: freshSafetyAction(context, 'sensitive file', access.path), - display: fileInputDisplay(access, 'Sensitive file access requires explicit approval.'), - }; - } - - const cwd = this.agent.config.cwd; - if (!mayBeGitControlPath(access.path, cwd, access.pathClass)) return undefined; - - const marker = - this.gitMarkerCache.get(cwd) ?? (await findGitWorkTreeMarker(this.agent.runtime.kaos, cwd)); - if (marker !== null) this.gitMarkerCache.set(cwd, marker); - if (marker !== null && isGitControlPath(access.path, cwd, marker, access.pathClass)) { - return { - kind: 'ask', - action: freshSafetyAction(context, 'git control path', access.path), - display: fileInputDisplay(access, 'Git control path access requires explicit approval.'), - }; - } - - return undefined; - } -} - -function freshSafetyAction( - context: PermissionPolicyContext, - boundary: string, - path: string, -): string { - const name = context.toolCall.function.name; - const id = context.toolCall.id; - return `Confirm ${name} on ${boundary}: ${path} for tool call ${id}`; -} - -function mayBeGitControlPath(targetPath: string, cwd: string, pathClass: PathClass): boolean { - return relativePathParts(targetPath, cwd, pathClass).some((part) => - part.toLowerCase().startsWith('.git'), - ); -} - -function isGitControlPath( - targetPath: string, - cwd: string, - marker: GitWorkTreeMarker, - pathClass: PathClass, -): boolean { - if (relativePathParts(targetPath, cwd, pathClass).some((part) => part.toLowerCase() === '.git')) { - return true; - } - 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/user-configured-rules.ts b/packages/agent-core/src/agent/permission/policies/user-configured-rules.ts index 4caf3e6..792d73b 100644 --- a/packages/agent-core/src/agent/permission/policies/user-configured-rules.ts +++ b/packages/agent-core/src/agent/permission/policies/user-configured-rules.ts @@ -1,11 +1,10 @@ import type { Agent } from '../..'; -import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; -import type { PermissionRule, PermissionRuleScope } from '../types'; import { - firstMatchingRuleDecision, - formatPermissionRuleDenyMessage, - genericInputDisplay, -} from './utils'; + matchPermissionRule, + type PermissionRuleMatch, +} from '../matches-rule'; +import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; +import type { PermissionRule, PermissionRuleDecision, PermissionRuleScope } from '../types'; const USER_CONFIGURED_SCOPES = new Set([ 'turn-override', @@ -13,53 +12,93 @@ const USER_CONFIGURED_SCOPES = new Set([ 'user', ]); -export class UserConfiguredPermissionRulesPolicy implements PermissionPolicy { - readonly name = 'user.configured-rules'; +export class UserConfiguredDenyPermissionPolicy implements PermissionPolicy { + readonly name = 'user-configured-deny'; constructor(private readonly agent: Agent) {} evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { - if ( - context.matchedRule !== undefined && - USER_CONFIGURED_SCOPES.has(context.matchedRule.scope) - ) { - return permissionRuleResult(context, context.matchedRule.decision, context.matchedRule); - } + const match = firstMatchingRule(this.agent, context, 'deny'); + if (match === undefined) return undefined; + return { + kind: 'deny', + reason: userRuleReason('deny', match), + message: formatPermissionRuleDenyMessage( + context.toolCall.function.name, + match.rule.reason, + this.agent.type, + ), + }; + } +} + +export class UserConfiguredAllowPermissionPolicy implements PermissionPolicy { + readonly name = 'user-configured-allow'; - const rules = this.agent.permission.rules.filter((rule): rule is PermissionRule => - USER_CONFIGURED_SCOPES.has(rule.scope), - ); - const match = firstMatchingRuleDecision(rules, this.agent, context); + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const match = firstMatchingRule(this.agent, context, 'allow'); if (match === undefined) return undefined; + return { + kind: 'approve', + reason: userRuleReason('allow', match), + }; + } +} - return permissionRuleResult(context, match.decision, match.rule); +export class UserConfiguredAskPermissionPolicy implements PermissionPolicy { + readonly name = 'user-configured-ask'; + + constructor(private readonly agent: Agent) {} + + evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined { + const match = firstMatchingRule(this.agent, context, 'ask'); + if (match === undefined) return undefined; + return { + kind: 'ask', + reason: userRuleReason('ask', match), + }; } } -function permissionRuleResult( +function firstMatchingRule( + agent: Agent, context: PermissionPolicyContext, - decision: PermissionRule['decision'], - rule: PermissionRule, -): PermissionPolicyResult { - const name = context.toolCall.function.name; - switch (decision) { - case 'deny': - return { - kind: 'result', - block: true, - reason: formatPermissionRuleDenyMessage(name, rule.reason), - }; - case 'ask': - return { - kind: 'ask', - action: `Approve ${name} due to permission rule`, - display: genericInputDisplay(`Approve ${name}`, { - rule: rule.pattern, - reason: rule.reason, - args: context.args, - }), - }; - case 'allow': - return { kind: 'allow' }; + decision: PermissionRuleDecision, +): PermissionRuleMatch | undefined { + const rules = 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.function.name, + args: context.args, + execution: context.execution, + }); + if (match !== undefined) return match; + } + return undefined; +} + +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/utils.ts b/packages/agent-core/src/agent/permission/policies/utils.ts deleted file mode 100644 index 7db9a74..0000000 --- a/packages/agent-core/src/agent/permission/policies/utils.ts +++ /dev/null @@ -1,164 +0,0 @@ -import type { Agent } from '../..'; -import type { ToolInputDisplay } from '../../../tools/display'; -import { - resolvePathAccess, - type PathAccessOperation, - type PathClass, -} from '../../../tools/policies/path-access'; -import { matchesRule } from '../matches-rule'; -import type { PermissionPathMatchOptions } from '../path-glob-match'; -import type { PermissionPolicyContext } from '../policy'; -import type { PermissionRule, PermissionRuleDecision } from '../types'; - -export type PathToolDisplayOperation = Extract< - ToolInputDisplay, - { kind: 'file_io' } ->['operation']; - -export interface PathToolAccess { - readonly inputPath: string; - readonly path: string; - readonly outsideCwd: boolean; - readonly pathClass: PathClass; - readonly operation: PathAccessOperation; - readonly displayOperation: PathToolDisplayOperation; -} - -export 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; -} - -export function resolvePathToolAccess( - agent: Agent, - context: PermissionPolicyContext, -): PathToolAccess | undefined { - const name = context.toolCall.function.name; - const operation = pathAccessOperationForTool(name); - if (operation === undefined) return undefined; - - const inputPath = readStringField(context.args, 'path'); - if (inputPath === undefined) return undefined; - - const cwd = agent.config.cwd; - if (cwd.length === 0) return undefined; - - const kaos = agent.runtime.kaos; - const pathClass = kaos.pathClass(); - let access; - try { - access = resolvePathAccess( - inputPath, - cwd, - { workspaceDir: cwd, additionalDirs: [] }, - { - operation, - pathClass, - homeDir: kaos.gethome(), - policy: { guardMode: 'disabled', checkSensitive: false }, - }, - ); - } catch { - return undefined; - } - - return { - inputPath, - path: access.path, - outsideCwd: access.outsideWorkspace, - pathClass, - operation, - displayOperation: pathToolDisplayOperation(name), - }; -} - -export function pathAccessOperationForTool(tool: string): PathAccessOperation | undefined { - switch (tool) { - case 'Read': - case 'ReadMediaFile': - return 'read'; - case 'Write': - case 'Edit': - return 'write'; - default: - return undefined; - } -} - -export function pathToolDisplayOperation(tool: string): PathToolDisplayOperation { - switch (tool) { - case 'Write': - return 'write'; - case 'Edit': - return 'edit'; - default: - return 'read'; - } -} - -export function fileInputDisplay( - access: PathToolAccess, - detail: string | undefined, -): ToolInputDisplay { - return { - kind: 'file_io', - operation: access.displayOperation, - path: access.path, - detail, - }; -} - -export function genericInputDisplay(summary: string, detail?: unknown): ToolInputDisplay { - return { - kind: 'generic', - summary, - detail, - }; -} - -export function permissionPathMatchOptions( - agent: Agent, -): PermissionPathMatchOptions { - return { - cwd: agent.config.cwd, - pathClass: agent.runtime.kaos.pathClass(), - homeDir: agent.runtime.kaos.gethome(), - }; -} - -export function firstMatchingRuleDecision( - rules: readonly PermissionRule[], - agent: Agent, - context: PermissionPolicyContext, -): { readonly decision: PermissionRuleDecision; readonly rule: PermissionRule } | undefined { - for (const decision of ['deny', 'ask', 'allow'] as const) { - const rule = firstMatchingRule(rules, decision, agent, context); - if (rule !== undefined) return { decision, rule }; - } - return undefined; -} - -function firstMatchingRule( - rules: readonly PermissionRule[], - decision: PermissionRuleDecision, - agent: Agent, - context: PermissionPolicyContext, -): PermissionRule | undefined { - const name = context.toolCall.function.name; - const args = context.args; - const pathOptions = permissionPathMatchOptions(agent); - for (const rule of rules) { - if (rule.decision !== decision) continue; - if (matchesRule(rule, name, args, pathOptions)) return rule; - } - return undefined; -} - -export function formatPermissionRuleDenyMessage( - tool: string, - reason: string | undefined, -): string { - const suffix = reason !== undefined && reason.length > 0 ? ` Reason: ${reason}` : ''; - 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..f826d94 --- /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 '../policy'; + +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 undefined; + return { + kind: 'approve', + }; + } +} diff --git a/packages/agent-core/src/agent/permission/policy.ts b/packages/agent-core/src/agent/permission/policy.ts index 6302a3b..e135cd9 100644 --- a/packages/agent-core/src/agent/permission/policy.ts +++ b/packages/agent-core/src/agent/permission/policy.ts @@ -1,29 +1,37 @@ -import type { PrepareToolExecutionResult, ToolExecutionHookContext } from '../../loop'; -import type { ToolInputDisplay } from '../../tools/display'; -import type { PermissionRule } from './types'; - -export interface PermissionPolicyContext extends 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; -} +import type { PrepareToolExecutionResult, ResolvedToolExecutionHookContext } from '../../loop'; +import type { ExecutableToolResult } from '../../loop/types'; +import type { ApprovalResponse } from './types'; + +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: 'allow'; + readonly kind: 'approve'; + readonly reason?: PermissionDecisionReason; readonly executionMetadata?: unknown; } - | ({ readonly kind: 'result' } & PrepareToolExecutionResult) + | { + readonly kind: 'deny'; + readonly reason?: PermissionDecisionReason; + readonly message?: string; + } | { readonly kind: 'ask'; - readonly action?: string | undefined; - readonly display?: ToolInputDisplay | undefined; + readonly reason?: PermissionDecisionReason; + readonly resolveApproval?: ( + result: ApprovalResponse, + ) => PermissionPolicyResolution | undefined; + readonly resolveError?: (error: unknown) => PermissionPolicyResolution | undefined; }; export interface PermissionPolicy { @@ -32,3 +40,7 @@ export interface PermissionPolicy { context: PermissionPolicyContext, ): PermissionPolicyResult | undefined | Promise; } + +export function syntheticResult(result: ExecutableToolResult): PermissionPolicyResolution { + return { kind: 'result', syntheticResult: result }; +} diff --git a/packages/agent-core/src/agent/permission/stable-args.ts b/packages/agent-core/src/agent/permission/stable-args.ts new file mode 100644 index 0000000..a26165b --- /dev/null +++ b/packages/agent-core/src/agent/permission/stable-args.ts @@ -0,0 +1,25 @@ +import { createHash } from 'node:crypto'; + +export function stableToolArgsKey(toolName: string, args: unknown): string { + const hash = createHash('sha256').update(stableSerialize(args)).digest('hex'); + return `${toolName}:${hash}`; +} + +export function stableSerialize(value: unknown): string { + if (value === null) return 'null'; + if (typeof value === 'string') return JSON.stringify(value); + if (typeof value === 'number' || typeof value === 'boolean') return JSON.stringify(value); + if (typeof value === 'undefined') return 'undefined'; + if (Array.isArray(value)) { + return `[${value.map((item) => stableSerialize(item)).join(',')}]`; + } + if (typeof value === 'object') { + const entries = Object.entries(value as Record).sort(([left], [right]) => + left.localeCompare(right), + ); + return `{${entries + .map(([key, item]) => `${JSON.stringify(key)}:${stableSerialize(item)}`) + .join(',')}}`; + } + return JSON.stringify(String(value)); +} diff --git a/packages/agent-core/src/agent/permission/types.ts b/packages/agent-core/src/agent/permission/types.ts index df51e55..32ecd0d 100644 --- a/packages/agent-core/src/agent/permission/types.ts +++ b/packages/agent-core/src/agent/permission/types.ts @@ -29,7 +29,7 @@ export interface PermissionRule { readonly decision: PermissionRuleDecision; readonly scope: PermissionRuleScope; readonly pattern: string; - readonly reason?: string | undefined; + readonly reason?: string; } export interface ApprovalRequest { @@ -51,6 +51,7 @@ export interface PermissionApprovalResultRecord { readonly toolCallId: string; readonly toolName: string; readonly action: string; + readonly sessionApprovalKey?: string; readonly result: ApprovalResponse; } diff --git a/packages/agent-core/src/agent/turn/index.ts b/packages/agent-core/src/agent/turn/index.ts index 1b14b2a..e0ca296 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.function.name, - signal: ctx.signal, - inputData: { - toolName: ctx.toolCall.function.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/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-call.ts b/packages/agent-core/src/loop/tool-call.ts index 6981ebb..3513666 100644 --- a/packages/agent-core/src/loop/tool-call.ts +++ b/packages/agent-core/src/loop/tool-call.ts @@ -261,7 +261,7 @@ async function prepareToolCall( 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', { @@ -281,9 +281,9 @@ async function prepareToolCall( } const displayFields = toolCallDisplayFieldsFromExecution(execution); - await dispatchToolCall(step, call, effectiveArgs, displayFields); if (step.signal.aborted) { + await dispatchToolCall(step, call, effectiveArgs, displayFields); return { task: makeResolvedToolCallTask( makeErrorToolResult(call, effectiveArgs, `Tool "${call.toolName}" was aborted`), @@ -292,17 +292,44 @@ async function prepareToolCall( } if (execution.isError === true) { + await dispatchToolCall(step, call, effectiveArgs, displayFields); return { task: makeResolvedToolCallTask(makeToolResult(call, effectiveArgs, execution)), stopBatchAfterThis: execution.stopTurn, }; } + const authorization = await runAuthorizeToolExecutionHook(step, call, effectiveArgs, execution); + if (authorization?.block === true) { + await dispatchToolCall(step, call, effectiveArgs, displayFields); + return { + task: makeResolvedToolCallTask( + makeErrorToolResult( + call, + effectiveArgs, + authorization.reason ?? `Tool call "${call.toolName}" was blocked`, + ), + ), + }; + } + + if (authorization?.syntheticResult !== undefined) { + await dispatchToolCall(step, call, effectiveArgs, displayFields); + return { + task: makeResolvedToolCallTask( + makeToolResult(call, effectiveArgs, authorization.syntheticResult), + ), + stopBatchAfterThis: toolResultStopsTurn(authorization.syntheticResult), + }; + } + + 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), }), }, }; @@ -383,6 +410,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..8b49370 100644 --- a/packages/agent-core/src/loop/types.ts +++ b/packages/agent-core/src/loop/types.ts @@ -115,13 +115,14 @@ export interface RunnableToolExecution { readonly accesses?: ToolAccesses | undefined; readonly display?: ToolInputDisplay | undefined; readonly description?: 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,6 +143,10 @@ export interface ToolExecutionHookContext extends LoopStepHookContext { readonly args: unknown; } +export interface ResolvedToolExecutionHookContext extends ToolExecutionHookContext { + readonly execution: RunnableToolExecution; +} + export interface PrepareToolExecutionResult { readonly block?: boolean | undefined; readonly reason?: string | undefined; @@ -181,6 +186,10 @@ export type PrepareToolExecutionHook = ( ctx: ToolExecutionHookContext, ) => Promise; +export type AuthorizeToolExecutionHook = ( + ctx: ResolvedToolExecutionHookContext, +) => Promise; + export type FinalizeToolResultHook = ( ctx: FinalizeToolResultContext, ) => Promise; @@ -203,6 +212,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/tools/background/task-list.ts b/packages/agent-core/src/tools/background/task-list.ts index 1a8c5cc..5572740 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 { matchesRuleSubject } from '../support/rule-match'; import type { BackgroundProcessManager, BackgroundTaskInfo } from './manager'; import { isBackgroundTaskTerminal } from './manager'; import TASK_LIST_DESCRIPTION from './task-list.md'; @@ -69,6 +70,8 @@ export class TaskListTool implements BuiltinTool { resolveExecution(args: TaskListInput): ToolExecution { return { description: 'Listing background tasks', + matchesRule: (ruleArgs) => + matchesRuleSubject(ruleArgs, (args.active_only ?? true) ? 'active' : 'all'), 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..8023b12 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 { matchesRuleSubject } from '../support/rule-match'; import TASK_OUTPUT_DESCRIPTION from './task-output.md'; /** @@ -75,6 +76,7 @@ export class TaskOutputTool implements BuiltinTool { resolveExecution(args: TaskOutputInput): ToolExecution { return { description: `Reading output of task ${args.task_id}`, + matchesRule: (ruleArgs) => matchesRuleSubject(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..c9f7089 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 { matchesRuleSubject } from '../support/rule-match'; import { isBackgroundTaskTerminal, type BackgroundProcessManager } from './manager'; import TASK_STOP_DESCRIPTION from './task-stop.md'; @@ -35,6 +36,7 @@ export class TaskStopTool implements BuiltinTool { resolveExecution(args: TaskStopInput): ToolExecution { return { description: `Stopping task ${args.task_id}`, + matchesRule: (ruleArgs) => matchesRuleSubject(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..5b20cf7 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 { matchesRuleSubject } 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,13 @@ 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, + }, + matchesRule: (ruleArgs) => matchesRuleSubject(ruleArgs, profileName), 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..63a0e19 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 { matchesRuleSubject } from '../../support/rule-match'; import skillDescriptionTemplate from './skill-tool.md'; export const MAX_SKILL_QUERY_DEPTH = 3; @@ -78,6 +79,8 @@ 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 }, + matchesRule: (ruleArgs) => matchesRuleSubject(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..3236beb 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 { matchesAnyRuleSubject } 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,8 @@ export class EditTool implements BuiltinTool { return { accesses: ToolAccesses.readWriteFile(path), description: `Editing ${args.path}`, + display: { kind: 'file_io', operation: 'edit', path }, + matchesRule: (ruleArgs) => matchesAnyRuleSubject(ruleArgs, [path, args.path]), 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..9a9b603 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 { matchesRuleSubject } from '../../support/rule-match'; import type { WorkspaceConfig } from '../../support/workspace'; import GLOB_DESCRIPTION from './glob.md'; @@ -117,6 +118,8 @@ export class GlobTool implements BuiltinTool { return { accesses: ToolAccesses.searchTree(searchRoots[0]!), description: `Searching ${args.pattern}`, + display: { kind: 'file_io', operation: 'glob', path: searchRoots[0]! }, + matchesRule: (ruleArgs) => matchesRuleSubject(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..6a1bb7b 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 { matchesRuleSubject } 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,8 @@ 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]! }, + matchesRule: (ruleArgs) => matchesRuleSubject(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..9f85834 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 { matchesAnyRuleSubject } from '../../support/rule-match'; import type { WorkspaceConfig } from '../../support/workspace'; import readMediaDescriptionHead from './read-media.md'; @@ -149,6 +150,8 @@ export class ReadMediaFileTool implements BuiltinTool { return { accesses: ToolAccesses.readFile(path), description: `Reading media: ${args.path}`, + display: { kind: 'file_io', operation: 'read', path }, + matchesRule: (ruleArgs) => matchesAnyRuleSubject(ruleArgs, [path, args.path]), 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..4bac8ba 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 { matchesAnyRuleSubject } 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,8 @@ export class ReadTool implements BuiltinTool { return { accesses: ToolAccesses.readFile(path), description: `Reading ${args.path}`, + display: { kind: 'file_io', operation: 'read', path }, + matchesRule: (ruleArgs) => matchesAnyRuleSubject(ruleArgs, [path, args.path]), 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..9cb2ba2 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 { matchesAnyRuleSubject } from '../../support/rule-match'; import type { WorkspaceConfig } from '../../support/workspace'; import WRITE_DESCRIPTION from './write.md'; @@ -76,6 +77,8 @@ export class WriteTool implements BuiltinTool { return { accesses: ToolAccesses.writeFile(path), description: `Writing ${args.path}`, + display: { kind: 'file_io', operation: 'write', path }, + matchesRule: (ruleArgs) => matchesAnyRuleSubject(ruleArgs, [path, args.path]), execute: () => this.execution(args, path), }; } 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..69986e3 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 @@ -9,10 +9,12 @@ */ 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 { ToolInputDisplay } from '../../display'; import { toInputJsonSchema } from '../../support/input-schema'; import DESCRIPTION from './exit-plan-mode.md'; @@ -86,13 +88,36 @@ 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', + display: await this.resolvePlanReviewDisplay(args), execute: (ctx) => this.execution(args, ctx), }; } + private async resolvePlanReviewDisplay( + args: ExitPlanModeInput, + ): 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, { diff --git a/packages/agent-core/src/tools/builtin/shell/bash.ts b/packages/agent-core/src/tools/builtin/shell/bash.ts index 148b899..e55dd7c 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 { matchesRuleSubject } from '../../support/rule-match'; import { ToolResultBuilder } from '../../support/result-builder'; import bashDescriptionTemplate from './bash.md'; @@ -173,6 +174,14 @@ 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', + }, + matchesRule: (ruleArgs) => matchesRuleSubject(ruleArgs, args.command), execute: ({ signal }) => this.execution(args, signal), }; } 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..9698164 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 { matchesRuleSubject } from '../../support/rule-match'; import { ToolResultBuilder } from '../../support/result-builder'; import DESCRIPTION from './fetch-url.md'; @@ -74,6 +75,8 @@ export class FetchURLTool implements BuiltinTool { return { accesses: ToolAccesses.none(), description: `Fetching: ${preview}`, + display: { kind: 'url_fetch', url: args.url }, + matchesRule: (ruleArgs) => matchesRuleSubject(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..c0ebf67 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 { matchesRuleSubject } from '../../support/rule-match'; import { ToolResultBuilder } from '../../support/result-builder'; import DESCRIPTION from './web-search.md'; @@ -70,6 +71,8 @@ export class WebSearchTool implements BuiltinTool { return { accesses: ToolAccesses.none(), description: `Searching: ${preview}`, + display: { kind: 'search', query: args.query }, + matchesRule: (ruleArgs) => matchesRuleSubject(ruleArgs, args.query), execute: (ctx) => this.execution(args, ctx), }; } 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..5e668f7 --- /dev/null +++ b/packages/agent-core/src/tools/support/rule-match.ts @@ -0,0 +1,12 @@ +export function matchesRuleSubject(ruleArgs: string, subject: string): boolean { + if (ruleArgs.length === 0) return true; + try { + return new RegExp(ruleArgs).test(subject); + } catch { + return false; + } +} + +export function matchesAnyRuleSubject(ruleArgs: string, subjects: readonly string[]): boolean { + return subjects.some((subject) => matchesRuleSubject(ruleArgs, subject)); +} diff --git a/packages/agent-core/test/agent/permission.test.ts b/packages/agent-core/test/agent/permission.test.ts index 048ca38..830c2be 100644 --- a/packages/agent-core/test/agent/permission.test.ts +++ b/packages/agent-core/test/agent/permission.test.ts @@ -10,19 +10,12 @@ import { type PermissionMode, type PermissionRule, } from '../../src/agent/permission'; -import { - actionToRulePattern, - describeApprovalAction, -} from '../../src/agent/permission/action-label'; -import { checkMatchingRules } from '../../src/agent/permission/check-rules'; import { matchesRule } from '../../src/agent/permission/matches-rule'; import { parsePattern } from '../../src/agent/permission/parse-pattern'; -import { createPlanPermissionPolicies } from '../../src/agent/permission/policies/plan'; -import type { - PermissionPolicy, - PermissionPolicyResult, -} from '../../src/agent/permission/policy'; -import type { ToolExecutionHookContext } from '../../src/loop'; +import { createPermissionDecisionPolicies } from '../../src/agent/permission/policies'; +import type { PermissionPolicyContext } from '../../src/agent/permission/policy'; +import { stableToolArgsKey } from '../../src/agent/permission/stable-args'; +import { ToolAccesses } from '../../src/loop'; import type { ToolInputDisplay } from '../../src/tools/display'; import { createFakeKaos } from '../tools/fixtures/fake-kaos'; import { createCommandKaos, testAgent } from './harness/agent'; @@ -388,19 +381,46 @@ describe('Permission auto mode', () => { }, ); - it.each([ - ['Read', { path: '/tmp/notes.md' }, 'read'], - ['ReadMediaFile', { path: '/tmp/image.png' }, 'read'], - ['Write', { path: '/tmp/notes.md', content: 'x' }, 'write'], - ['Edit', { path: '/tmp/notes.md', old_string: 'a', new_string: 'b' }, 'edit'], - ['Grep', { pattern: 'TODO', path: '/tmp' }, 'grep'], - ] as const)( - 'requests approval for %s outside the workspace in yolo mode', - async (toolName, args, operation) => { + it.each( + (['manual', 'yolo', 'auto'] as const).flatMap((mode) => + [ + [ + mode, + 'Read', + { path: '/tmp/notes.md' }, + 'read', + 'read file outside of working directory', + ], + [ + mode, + 'ReadMediaFile', + { path: '/tmp/image.png' }, + 'read', + 'read media file outside of working directory', + ], + [ + mode, + 'Write', + { path: '/tmp/notes.md', content: 'x' }, + 'write', + 'write file outside of working directory', + ], + [ + mode, + 'Edit', + { path: '/tmp/notes.md', old_string: 'a', new_string: 'b' }, + 'edit', + 'edit file outside of working directory', + ], + ] as const, + ), + )( + 'requests approval in %s mode for %s outside the cwd', + async (mode, toolName, args, operation, action) => { const { manager, requestApproval } = makePermissionManager(async () => ({ decision: 'approved', })); - manager.setMode('yolo'); + manager.setMode(mode); await expect( manager.beforeToolCall(hookContext({ id: `call_${toolName}`, toolName, args })), @@ -409,11 +429,12 @@ describe('Permission auto mode', () => { expect(requestApproval).toHaveBeenCalledWith( expect.objectContaining({ toolName, + action, display: { kind: 'file_io', operation, path: args.path, - detail: 'Outside workspace: /workspace', + detail: 'Path is outside the current working directory.', }, }), expect.any(Object), @@ -443,8 +464,8 @@ describe('Permission auto mode', () => { }, ); - it.each(['manual', 'auto'] as const)( - 'does not apply the outside-workspace yolo policy in %s mode', + it.each(['manual', 'yolo', 'auto'] as const)( + 'does not classify Grep path as an outside-cwd boundary in %s mode', async (mode) => { const { manager, requestApproval } = makePermissionManager(async () => ({ decision: 'approved', @@ -454,9 +475,9 @@ describe('Permission auto mode', () => { await expect( manager.beforeToolCall( hookContext({ - id: `call_read_${mode}`, - toolName: 'Read', - args: { path: '/tmp/notes.md' }, + id: `call_grep_${mode}`, + toolName: 'Grep', + args: { pattern: 'TODO', path: '/tmp' }, }), ), ).resolves.toBeUndefined(); @@ -512,74 +533,36 @@ describe('Permission auto mode', () => { await expect(call()).resolves.toBeUndefined(); expect(requestApproval).toHaveBeenCalledTimes(1); - expect(manager.data().rules).toContainEqual({ - decision: 'allow', - scope: 'session-runtime', - pattern: 'Read', - reason: 'approve_for_session: read file', - }); + expect(manager.hasSessionApprovedKey(stableToolArgsKey('Read', { path: '/tmp/notes.md' }))).toBe( + true, + ); + expect(manager.data().rules).toEqual([]); }); }); describe('Permission policy chain', () => { - it('keeps plan-specific policies under the permission module plan namespace', () => { - expect(createPlanPermissionPolicies({} as Agent).map((policy) => policy.name)).toEqual([ - 'plan.enter-plan-mode', - 'plan.exit-plan-mode', - 'plan.mode-guard', + it('keeps built-in policies in document order', () => { + expect(createPermissionDecisionPolicies({} as Agent).map((policy) => policy.name)).toEqual([ + 'pre-tool-call-hook', + 'auto-mode-ask-user-question-deny', + 'plan-mode-guard-deny', + 'user-configured-deny', + 'auto-mode-approve', + 'user-configured-allow', + 'session-approval-history', + 'user-configured-ask', + 'sensitive-file-access-ask', + 'git-control-path-access-ask', + 'cwd-outside-file-access-ask', + 'exit-plan-mode-review-ask', + 'yolo-mode-approve', + 'default-tool-approve', + 'git-cwd-write-approve', + 'plan-mode-tool-approve', + 'fallback-ask', ]); }); - it('runs custom policies after deny rules and before generic approval', async () => { - const policy: PermissionPolicy = { - name: 'test.block-bash', - evaluate: vi.fn(async (): Promise => ({ - kind: 'result', - block: true, - reason: 'blocked by custom policy', - })), - }; - const { manager, requestApproval } = makePermissionManager( - async () => ({ decision: 'approved' }), - { policies: [policy] }, - ); - - await expect(manager.beforeToolCall(hookContext({ id: 'call_policy' }))).resolves.toEqual({ - block: true, - reason: 'blocked by custom policy', - }); - expect(policy.evaluate).toHaveBeenCalledWith( - expect.objectContaining({ - toolCall: expect.objectContaining({ id: 'call_policy' }), - matchedRule: undefined, - }), - ); - expect(requestApproval).not.toHaveBeenCalled(); - }); - - it('keeps explicit deny rules higher priority than custom policies', async () => { - const policy: PermissionPolicy = { - name: 'test.allow-bash', - evaluate: vi.fn(async (): Promise => ({ kind: 'allow' })), - }; - const { manager, requestApproval } = makePermissionManager( - async () => ({ decision: 'approved' }), - { policies: [policy] }, - ); - manager.rules.push({ - decision: 'deny', - scope: 'user', - pattern: 'Bash', - reason: 'blocked before policy', - }); - - await expect(manager.beforeToolCall(hookContext({ id: 'call_deny' }))).resolves.toMatchObject({ - block: true, - reason: 'Tool "Bash" was denied by permission rule. Reason: blocked before policy', - }); - expect(policy.evaluate).not.toHaveBeenCalled(); - expect(requestApproval).not.toHaveBeenCalled(); - }); }); describe('Permission live derive', () => { @@ -599,7 +582,7 @@ describe('Permission live derive', () => { expect(child.manager.mode).toBe('manual'); }); - it('uses child matching rules before parent rules', async () => { + it('applies rule decision priority across child and parent rules', async () => { const parent = makePermissionManager(async () => ({ decision: 'approved' })); parent.manager.rules.push({ decision: 'deny', @@ -617,8 +600,11 @@ describe('Permission live derive', () => { reason: 'child allow', }); - await expect(child.manager.beforeToolCall(hookContext({ id: 'call_allow' }))).resolves - .toBeUndefined(); + await expect(child.manager.beforeToolCall(hookContext({ id: 'call_parent_deny' }))).resolves + .toMatchObject({ + block: true, + reason: 'Tool "Bash" was denied by permission rule. Reason: parent deny', + }); expect(child.requestApproval).not.toHaveBeenCalled(); const parentAllow = makePermissionManager(async () => ({ decision: 'approved' })); @@ -1175,11 +1161,11 @@ describe('Default git CWD Write/Edit permission', () => { }); } - function writeHook(args: Record, id = 'call_write'): ToolExecutionHookContext { + function writeHook(args: Record, id = 'call_write'): PermissionPolicyContext { return hookContext({ id, toolName: 'Write', args }); } - function editHook(args: Record, id = 'call_edit'): ToolExecutionHookContext { + function editHook(args: Record, id = 'call_edit'): PermissionPolicyContext { return hookContext({ id, toolName: 'Edit', args }); } @@ -1665,18 +1651,6 @@ describe('Permission rule helpers', () => { ).toBe(false); }); - it('matches path rules against relative tool paths using the active cwd', () => { - expect( - checkMatchingRules( - [permissionRule('Read(/workspace/project/secret.txt)', 'deny')], - 'Read', - { path: './secret.txt' }, - 'manual', - { cwd: '/workspace/project', pathClass: 'posix' }, - ), - ).toMatchObject({ decision: 'deny' }); - }); - it('matches win32 path rules across separators, dot segments, and case variants', () => { const secret = 'C:\\workspace\\project\\secret.txt'; const denyRule = permissionRule(`Read(${secret})`, 'deny'); @@ -1705,85 +1679,6 @@ describe('Permission rule helpers', () => { ).toBe(true); }); - it('matches win32 path rules against relative tool paths using the active cwd', () => { - expect( - checkMatchingRules( - [permissionRule('Read(C:\\workspace\\project\\secret.txt)', 'deny')], - 'Read', - { path: '.\\SECRET.txt' }, - 'manual', - { cwd: 'C:\\workspace\\project', pathClass: 'win32' }, - ), - ).toMatchObject({ decision: 'deny' }); - }); - - it('applies explicit rule priority and mode overlay in order', () => { - expect(checkMatchingRules([], 'Write', { path: '/tmp/a' }, 'manual')).toBeUndefined(); - expect( - checkMatchingRules( - [permissionRule('Write', 'allow')], - 'Write', - { path: '/tmp/a' }, - 'manual', - ), - ).toMatchObject({ - decision: 'allow', - }); - expect( - checkMatchingRules( - [permissionRule('Write', 'allow'), permissionRule('Write', 'ask')], - 'Write', - {}, - 'manual', - ), - ).toMatchObject({ decision: 'ask' }); - expect( - checkMatchingRules( - [permissionRule('Write', 'allow'), permissionRule('Write', 'deny')], - 'Write', - {}, - 'manual', - ), - ).toMatchObject({ decision: 'deny' }); - expect( - checkMatchingRules([permissionRule('Write', 'ask')], 'Write', { path: '/tmp/a' }, 'yolo'), - ).toMatchObject({ - decision: 'allow', - }); - expect( - checkMatchingRules([permissionRule('Write', 'deny')], 'Write', { path: '/tmp/a' }, 'yolo'), - ).toMatchObject({ - decision: 'deny', - }); - }); - - it('derives approval action labels from display semantics and MCP names', () => { - expect(describeApprovalAction('Bash', {}, { kind: 'command', command: 'git status' })).toBe( - 'run command', - ); - expect( - describeApprovalAction('Write', {}, { kind: 'file_io', operation: 'write', path: 'x' }), - ).toBe('write file'); - expect( - describeApprovalAction('ExitPlanMode', {}, { kind: 'plan_review', plan: '# Plan' }), - ).toBe('review plan'); - expect( - describeApprovalAction( - 'Agent', - {}, - { kind: 'agent_call', agent_name: 'review', prompt: 'x' }, - ), - ).toBe('spawn agent'); - expect(describeApprovalAction('Skill', {}, { kind: 'skill_call', skill_name: 'commit' })).toBe( - 'invoke skill', - ); - expect(describeApprovalAction('mcp__files__get_files', {}, genericDisplay())).toBe( - 'call MCP tool: files:get_files', - ); - expect(actionToRulePattern('edit file outside of working directory', 'Edit')).toBe('Write'); - expect(actionToRulePattern('run command in plan mode', 'Bash')).toBeUndefined(); - expect(actionToRulePattern('call CustomTool', 'CustomTool')).toBe('CustomTool'); - }); }); function bashCall(): ToolCall { @@ -1800,7 +1695,6 @@ function bashCall(): ToolCall { function makePermissionManager( handleApproval: (request: unknown) => Promise, options: { - readonly policies?: readonly PermissionPolicy[]; readonly parent?: PermissionManager | undefined; readonly planModeActive?: boolean; readonly kaos?: Kaos; @@ -1897,7 +1791,7 @@ function hookContext(input: { readonly id: string; readonly toolName?: string | undefined; readonly args?: Record | undefined; -}): ToolExecutionHookContext { +}): PermissionPolicyContext { const toolName = input.toolName ?? 'Bash'; const args = input.args ?? { command: 'printf first', timeout: 60 }; const toolCall: ToolCall = { @@ -1912,18 +1806,10 @@ function hookContext(input: { turnId: '0', stepNumber: 1, signal: new AbortController().signal, - llm: {} as ToolExecutionHookContext['llm'], + llm: {} as PermissionPolicyContext['llm'], toolCall, args, - }; -} - -function sessionAllowRule(): PermissionRule { - return { - decision: 'allow', - scope: 'session-runtime', - pattern: 'Bash', - reason: 'approve_for_session: run command', + execution: testExecution(toolName, args), }; } @@ -1938,6 +1824,68 @@ function permissionRule( }; } +function sessionAllowRule(): PermissionRule { + return { + decision: 'allow', + scope: 'session-runtime', + pattern: 'Bash', + reason: 'approve_for_session: run command', + }; +} + function genericDisplay(): ToolInputDisplay { return { kind: 'generic', summary: 'Approve tool', detail: {} }; } + +function testExecution( + toolName: string, + args: Record, +): PermissionPolicyContext['execution'] { + return { + description: testDescription(toolName), + display: testDisplay(toolName, args), + accesses: testAccesses(toolName, args), + execute: async () => ({ output: '' }), + }; +} + +function testDescription(toolName: string): string { + switch (toolName) { + case 'Bash': + return 'run command'; + case 'ExitPlanMode': + return 'review plan'; + case 'Read': + return 'read file'; + case 'Write': + return 'write file'; + case 'Edit': + return 'edit file'; + default: + return `call ${toolName}`; + } +} + +function testDisplay(toolName: string, args: Record): ToolInputDisplay { + const path = typeof args['path'] === 'string' ? args['path'] : '/workspace/file.txt'; + switch (toolName) { + case 'Bash': + return { kind: 'command', command: String(args['command'] ?? '') }; + case 'Read': + return { kind: 'file_io', operation: 'read', path }; + case 'Write': + return { kind: 'file_io', operation: 'write', path }; + case 'Edit': + return { kind: 'file_io', operation: 'edit', path }; + default: + return genericDisplay(); + } +} + +function testAccesses(toolName: string, args: Record) { + const path = typeof args['path'] === 'string' ? args['path'] : undefined; + if (toolName === 'Read' && path !== undefined) return ToolAccesses.readFile(path); + if (toolName === 'Write' && path !== undefined) return ToolAccesses.writeFile(path); + if (toolName === 'Edit' && path !== undefined) return ToolAccesses.readWriteFile(path); + return ToolAccesses.none(); +} diff --git a/packages/agent-core/test/tools/fixtures/execute-tool.ts b/packages/agent-core/test/tools/fixtures/execute-tool.ts index 4ea5c2f..04a80bd 100644 --- a/packages/agent-core/test/tools/fixtures/execute-tool.ts +++ b/packages/agent-core/test/tools/fixtures/execute-tool.ts @@ -1,18 +1,23 @@ -import type { ExecutableTool, ExecutableToolContext, ToolExecution } from '../../../src/loop'; +import type { + ExecutableTool, + ExecutableToolContext, + ExecutableToolResult, + ToolExecution, +} from '../../../src/loop'; import { PathSecurityError } from '../../../src/tools/policies/path-access'; export type TestExecutableToolContext = ExecutableToolContext & { readonly args: Input; }; -export function executeTool( +export async function executeTool( tool: ExecutableTool, context: TestExecutableToolContext, -) { +): Promise { const { args, ...executionContext } = context; let execution: ToolExecution; try { - execution = tool.resolveExecution(args); + execution = await tool.resolveExecution(args); } catch (error) { const output = error instanceof PathSecurityError diff --git a/packages/agent-core/test/tools/plan-mode-hard-block.test.ts b/packages/agent-core/test/tools/plan-mode-hard-block.test.ts index f4c198c..7abd969 100644 --- a/packages/agent-core/test/tools/plan-mode-hard-block.test.ts +++ b/packages/agent-core/test/tools/plan-mode-hard-block.test.ts @@ -2,10 +2,14 @@ import type { ToolCall } from '@moonshot-ai/kosong'; import { describe, expect, it, vi } from 'vitest'; import type { Agent } from '../../src/agent'; -import { PlanModeGuardPermissionPolicy } from '../../src/agent/permission/policies/plan'; +import { PlanModeGuardDenyPermissionPolicy } from '../../src/agent/permission/policies/plan-mode-guard-deny'; import type { PermissionMode } from '../../src/agent/permission/types'; -import type { PermissionPolicyContext } from '../../src/agent/permission/policy'; +import type { + PermissionPolicyContext, + PermissionPolicyResult, +} from '../../src/agent/permission/policy'; import { PlanMode } from '../../src/agent/plan'; +import { ToolAccesses } from '../../src/loop'; import type { ToolExecutionHookContext } from '../../src/loop'; const signal = new AbortController().signal; @@ -53,7 +57,10 @@ function policyContext( ): PermissionPolicyContext { return { ...hookContext(toolName, args), - matchedRule: undefined, + execution: { + accesses: toolAccesses(toolName, args), + execute: async () => ({ output: '' }), + }, }; } @@ -63,7 +70,7 @@ function evaluatePlanPolicy( args: unknown, mode: PermissionMode = 'manual', ) { - return new PlanModeGuardPermissionPolicy(agent).evaluate(policyContext(toolName, args, mode)); + return new PlanModeGuardDenyPermissionPolicy(agent).evaluate(policyContext(toolName, args, mode)); } describe('Plan mode permission policy', () => { @@ -72,9 +79,7 @@ describe('Plan mode permission policy', () => { const planPath = planMode.planFilePath; if (planPath === null) throw new Error('expected plan path'); - expect( - await evaluatePlanPolicy(agent, 'Write', { path: planPath }), - ).toEqual({ kind: 'allow' }); + expect(await evaluatePlanPolicy(agent, 'Write', { path: planPath })).toBeUndefined(); expect( await evaluatePlanPolicy( agent, @@ -85,7 +90,7 @@ describe('Plan mode permission policy', () => { new_string: 'B', }, ), - ).toEqual({ kind: 'allow' }); + ).toBeUndefined(); }); it('blocks Write and Edit to non-plan files before permission approval', async () => { @@ -101,11 +106,11 @@ describe('Plan mode permission policy', () => { new_string: 'B', }); - expect(write).toMatchObject({ kind: 'result', block: true }); - expect(write?.kind === 'result' ? write.reason : '').toContain('current plan file'); - expect(write?.kind === 'result' ? write.reason : '').toContain('ExitPlanMode'); - expect(edit).toMatchObject({ kind: 'result', block: true }); - expect(edit?.kind === 'result' ? edit.reason : '').toContain('current plan file'); + const writeDeny = expectDeny(write); + expect(writeDeny.message ?? '').toContain('current plan file'); + expect(writeDeny.message ?? '').toContain('ExitPlanMode'); + const editDeny = expectDeny(edit); + expect(editDeny.message ?? '').toContain('current plan file'); }); it('blocks file edits when plan mode has no selected plan file path', async () => { @@ -118,11 +123,9 @@ describe('Plan mode permission policy', () => { new_string: 'B', }); - expect(result).toMatchObject({ kind: 'result', block: true }); - expect(result?.kind === 'result' ? result.reason : '').toContain( - '(no plan file selected yet)', - ); - expect(result?.kind === 'result' ? result.reason : '').toContain('ExitPlanMode'); + const deny = expectDeny(result); + expect(deny.message ?? '').toContain('(no plan file selected yet)'); + expect(deny.message ?? '').toContain('ExitPlanMode'); }); it.each(['manual', 'yolo', 'auto'] as const)( @@ -151,9 +154,9 @@ describe('Plan mode permission policy', () => { mode, ); - expect(result).toMatchObject({ kind: 'result', block: true }); - expect(result?.kind === 'result' ? result.reason : '').toContain('plan mode'); - expect(result?.kind === 'result' ? result.reason : '').toContain('ExitPlanMode'); + const deny = expectDeny(result); + expect(deny.message ?? '').toContain('plan mode'); + expect(deny.message ?? '').toContain('ExitPlanMode'); }, ); @@ -172,3 +175,19 @@ describe('Plan mode permission policy', () => { ).toBeUndefined(); }); }); + +function toolAccesses(toolName: string, args: unknown) { + const path = args !== null && typeof args === 'object' ? (args as { path?: unknown }).path : undefined; + if (typeof path !== 'string') return ToolAccesses.none(); + if (toolName === 'Write') return ToolAccesses.writeFile(path); + if (toolName === 'Edit') return ToolAccesses.readWriteFile(path); + return ToolAccesses.none(); +} + +function expectDeny( + result: PermissionPolicyResult | undefined, +): Extract { + expect(result).toMatchObject({ kind: 'deny' }); + if (result?.kind !== 'deny') throw new Error('expected deny result'); + return result; +} diff --git a/packages/agent-core/test/tools/planning/exit-plan-mode-telemetry.test.ts b/packages/agent-core/test/tools/planning/exit-plan-mode-telemetry.test.ts index ddba6e4..57388fb 100644 --- a/packages/agent-core/test/tools/planning/exit-plan-mode-telemetry.test.ts +++ b/packages/agent-core/test/tools/planning/exit-plan-mode-telemetry.test.ts @@ -6,7 +6,7 @@ import { type ApprovalResponse, type PermissionMode, } from '../../../src/agent/permission'; -import type { ToolExecutionHookContext } from '../../../src/loop'; +import type { PermissionPolicyContext } from '../../../src/agent/permission/policy'; import { ExitPlanModeTool, type ExitPlanModeInput, @@ -77,12 +77,20 @@ async function execute(agent: Agent, args: ExitPlanModeInput = {}) { }); } -function permissionContext(args: ExitPlanModeInput): ToolExecutionHookContext { +function permissionContext(args: ExitPlanModeInput): PermissionPolicyContext { + const display: PermissionPolicyContext['execution']['display'] = { + kind: 'plan_review', + plan: '# Plan', + path: '/tmp/kimi-plan.md', + }; + if (args.options !== undefined && args.options.length >= 2) { + display.options = args.options; + } return { turnId: '7', stepNumber: 1, signal: new AbortController().signal, - llm: {} as ToolExecutionHookContext['llm'], + llm: {} as PermissionPolicyContext['llm'], toolCall: { id: 'call_exit_plan', type: 'function', @@ -92,6 +100,11 @@ function permissionContext(args: ExitPlanModeInput): ToolExecutionHookContext { }, }, args, + execution: { + description: 'Presenting plan and exiting plan mode', + display, + execute: async () => ({ output: '' }), + }, }; } From 1f0679b9de7f603b67f16e8779f5d682501757ff Mon Sep 17 00:00:00 2001 From: _Kerman Date: Mon, 25 May 2026 19:23:36 +0800 Subject: [PATCH 03/35] fix: align permission policies with execution metadata --- PERMISSION_DECISION_MODEL.md | 13 +- .../src/agent/permission/matches-rule.ts | 2 +- .../policies/exit-plan-mode-review-ask.ts | 37 +- .../src/agent/permission/policies/index.ts | 2 +- .../policies/plan-mode-tool-approve.ts | 32 + .../src/agent/permission/stable-args.ts | 8 +- .../tools/builtin/planning/exit-plan-mode.ts | 114 +++- packages/agent-core/test/agent/basic.test.ts | 8 +- packages/agent-core/test/agent/config.test.ts | 8 +- .../test/agent/harness/snapshots.ts | 14 +- .../agent-core/test/agent/permission.test.ts | 611 ++++++++++++------ packages/agent-core/test/agent/plan.test.ts | 10 +- packages/agent-core/test/agent/tool.test.ts | 8 +- packages/agent-core/test/agent/turn.test.ts | 44 +- .../agent-core/test/loop/fixtures/tools.ts | 4 +- .../test/session/lifecycle-hooks.test.ts | 2 +- .../test/session/subagent-host.test.ts | 14 +- .../test/tools/fixtures/execute-tool.ts | 11 +- .../test/tools/plan-mode-hard-block.test.ts | 32 +- .../planning/exit-plan-mode-telemetry.test.ts | 54 +- 20 files changed, 702 insertions(+), 326 deletions(-) diff --git a/PERMISSION_DECISION_MODEL.md b/PERMISSION_DECISION_MODEL.md index aefe288..cc23661 100644 --- a/PERMISSION_DECISION_MODEL.md +++ b/PERMISSION_DECISION_MODEL.md @@ -86,6 +86,13 @@ - 用户配置 `ask` rule 命中 -> `ask` +## plan-mode-tool-approve: Plan Mode Tool Approve + +- `EnterPlanMode` -> `approve` +- plan mode active 且 `Write` / `Edit` 目标是当前 plan file -> `approve` +- `ExitPlanMode` 不在 plan mode active 状态 -> `approve` +- `ExitPlanMode` 在 plan mode active 但没有有效 plan 内容 -> `approve` + ## sensitive-file-access-ask: Sensitive File Access Ask - `execution.accesses` 中存在敏感文件 `.env` / SSH private key / credentials path -> `ask` @@ -114,12 +121,6 @@ - tool name 为 `Write` / `Edit`，且 `execution.accesses` 中的写目标在 POSIX git cwd 内、目标在 cwd 内 -> `approve` -## plan-mode-tool-approve: Plan Mode Tool Approve - -- `EnterPlanMode` -> `approve` -- `ExitPlanMode` 不在 plan mode active 状态 -> `approve` -- `ExitPlanMode` 在 plan mode active 但没有有效 plan 内容 -> `approve` - ## fallback-ask: Fallback - 以上全部未命中 -> `ask` diff --git a/packages/agent-core/src/agent/permission/matches-rule.ts b/packages/agent-core/src/agent/permission/matches-rule.ts index d8ef0a9..708474a 100644 --- a/packages/agent-core/src/agent/permission/matches-rule.ts +++ b/packages/agent-core/src/agent/permission/matches-rule.ts @@ -85,7 +85,7 @@ export function matchesRule( function singleActualFieldValue(args: unknown): unknown { if (args === null || typeof args !== 'object' || Array.isArray(args)) return undefined; const entries = Object.entries(args as Record).filter( - ([, value]) => typeof value !== 'undefined', + ([, value]) => value !== undefined, ); return entries.length === 1 ? entries[0]![1] : undefined; } 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 index 70c468a..432fda5 100644 --- 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 @@ -44,24 +44,21 @@ function exitPlanModeApprovalResult( result: ApprovalResponse, options: readonly ExitPlanModeOption[] | undefined, ) { - if (result.decision === 'approved') { - const selected = selectedExitPlanModeOption(options, result.selectedLabel); - return { - kind: 'approve' as const, - executionMetadata: - selected === undefined - ? { - planTelemetrySubmitted: true, - planTelemetryResolved: true, - } - : { - selectedOption: selected, - planTelemetrySubmitted: true, - planTelemetryResolved: true, - }, - }; + const selected = selectedExitPlanModeOption(options, result.selectedLabel); + if (result.decision !== 'approved') { + return rejectedExitPlanModeApprovalResult(agent, result); } + return { + kind: 'approve' as const, + executionMetadata: { + planApproval: approvalMetadata(result), + selectedOption: selected, + }, + }; +} + +function rejectedExitPlanModeApprovalResult(agent: Agent, result: ApprovalResponse) { if (result.decision === 'cancelled') { return { kind: 'result' as const, @@ -121,6 +118,14 @@ function exitPlanModeForRejectedPlan(agent: Agent) { } } +function approvalMetadata(result: ApprovalResponse) { + return { + decision: result.decision, + selectedLabel: result.selectedLabel, + feedback: result.feedback, + }; +} + function selectedExitPlanModeOption( options: readonly ExitPlanModeOption[] | undefined, label: string | undefined, diff --git a/packages/agent-core/src/agent/permission/policies/index.ts b/packages/agent-core/src/agent/permission/policies/index.ts index 7a76b67..07e0590 100644 --- a/packages/agent-core/src/agent/permission/policies/index.ts +++ b/packages/agent-core/src/agent/permission/policies/index.ts @@ -32,6 +32,7 @@ export function createPermissionDecisionPolicies(agent: Agent): readonly Permiss new UserConfiguredAllowPermissionPolicy(agent), new SessionApprovalHistoryPermissionPolicy(agent), new UserConfiguredAskPermissionPolicy(agent), + new PlanModeToolApprovePermissionPolicy(agent), new SensitiveFileAccessAskPermissionPolicy(agent), new GitControlPathAccessAskPermissionPolicy(agent), new CwdOutsideFileAccessAskPermissionPolicy(agent), @@ -39,7 +40,6 @@ export function createPermissionDecisionPolicies(agent: Agent): readonly Permiss new YoloModeApprovePermissionPolicy(agent), new DefaultToolApprovePermissionPolicy(), new GitCwdWriteApprovePermissionPolicy(agent), - new PlanModeToolApprovePermissionPolicy(), new FallbackAskPermissionPolicy(), ]; } 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 index ea67e85..c0eaf5b 100644 --- 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 @@ -1,8 +1,12 @@ +import type { Agent } from '../..'; +import type { ToolResourceAccess } from '../../../loop/tool-access'; import type { PermissionPolicy, PermissionPolicyContext, PermissionPolicyResult } from '../policy'; 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.function.name; if (toolName === 'EnterPlanMode') { @@ -11,6 +15,16 @@ export class PlanModeToolApprovePermissionPolicy implements PermissionPolicy { }; } + if ( + this.agent.planMode.isActive && + (toolName === 'Write' || toolName === 'Edit') && + writesOnlyPlanFile(context, this.agent.planMode.planFilePath) + ) { + return { + kind: 'approve', + }; + } + if (toolName !== 'ExitPlanMode') return undefined; if (context.execution.display?.kind !== 'plan_review') { return { @@ -23,3 +37,21 @@ export class PlanModeToolApprovePermissionPolicy implements PermissionPolicy { }; } } + +type FileAccess = Extract & { readonly path: string }; + +function writesOnlyPlanFile( + context: PermissionPolicyContext, + planFilePath: string | null, +): boolean { + if (planFilePath === null) return false; + const writeAccesses = + context.execution.accesses?.filter( + (access): access is FileAccess => + access.kind === 'file' && + access.path !== undefined && + (access.operation === 'write' || access.operation === 'readwrite'), + ) ?? []; + if (writeAccesses.length === 0) return false; + return writeAccesses.every((access) => access.path === planFilePath); +} diff --git a/packages/agent-core/src/agent/permission/stable-args.ts b/packages/agent-core/src/agent/permission/stable-args.ts index a26165b..9524e68 100644 --- a/packages/agent-core/src/agent/permission/stable-args.ts +++ b/packages/agent-core/src/agent/permission/stable-args.ts @@ -9,17 +9,19 @@ export function stableSerialize(value: unknown): string { if (value === null) return 'null'; if (typeof value === 'string') return JSON.stringify(value); if (typeof value === 'number' || typeof value === 'boolean') return JSON.stringify(value); - if (typeof value === 'undefined') return 'undefined'; + if (value === undefined) return 'undefined'; if (Array.isArray(value)) { return `[${value.map((item) => stableSerialize(item)).join(',')}]`; } if (typeof value === 'object') { - const entries = Object.entries(value as Record).sort(([left], [right]) => + const entries = Object.entries(value as Record).toSorted(([left], [right]) => left.localeCompare(right), ); return `{${entries .map(([key, item]) => `${JSON.stringify(key)}:${stableSerialize(item)}`) .join(',')}}`; } - return JSON.stringify(String(value)); + if (typeof value === 'bigint') return JSON.stringify(value.toString()); + if (typeof value === 'symbol') return JSON.stringify(value.description ?? ''); + return JSON.stringify('[function]'); } 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 69986e3..964868b 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 @@ -4,7 +4,7 @@ * 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 + * handles plan approval before this tool runs and passes the approval outcome * through execution metadata. */ @@ -79,6 +79,12 @@ type ResolvePlanResult = | { readonly ok: true; readonly plan: string; readonly path?: string | undefined } | { readonly ok: false; readonly error: ExecutableToolResult }; +interface PlanApprovalMetadata { + readonly decision: 'approved' | 'rejected' | 'cancelled'; + readonly selectedLabel?: string; + readonly feedback?: string; +} + // ── Implementation ─────────────────────────────────────────────────── export class ExitPlanModeTool implements BuiltinTool { @@ -140,6 +146,19 @@ export class ExitPlanModeTool implements BuiltinTool { has_options: args.options !== undefined && args.options.length >= 2, }); } + const planApproval = planApprovalFromMetadata(metadata); + if (planApproval !== undefined) { + trackPlanApprovalResolution(this.agent, planApproval); + if (planApproval.decision !== 'approved') { + return this.planApprovalRejectedResult(planApproval); + } + return this.exitWithPlan( + resolvedPlan.plan, + resolvedPlan.path, + selectedOptionFromMetadata(metadata), + true, + ); + } return this.exitWithPlan( resolvedPlan.plan, resolvedPlan.path, @@ -170,6 +189,43 @@ export class ExitPlanModeTool implements BuiltinTool { }; } + private planApprovalRejectedResult(approval: PlanApprovalMetadata): ExecutableToolResult { + if (approval.decision === 'cancelled') { + return { + isError: false, + output: 'Plan approval dismissed. Plan mode remains active.', + }; + } + + if (normalizeOptionLabel(approval.selectedLabel ?? '') === 'reject and exit') { + const failed = this.exitPlanMode(); + return ( + failed ?? { + isError: true, + stopTurn: true, + output: 'Plan rejected by user. Plan mode deactivated.', + } + ); + } + + const feedback = approval.feedback ?? ''; + if (normalizeOptionLabel(approval.selectedLabel ?? '') === 'revise' || feedback.length > 0) { + return { + isError: false, + output: + feedback.length > 0 + ? `User rejected the plan. Feedback:\n\n${feedback}` + : 'User requested revisions. Plan mode remains active.', + }; + } + + return { + isError: true, + stopTurn: true, + output: 'Plan rejected by user. Plan mode remains active.', + }; + } + private exitPlanMode(): ExecutableToolResult | undefined { try { this.agent.planMode.exit(); @@ -245,6 +301,62 @@ function selectedOptionFromMetadata(metadata: unknown): ExitPlanModeOption | und return { label, description }; } +function planApprovalFromMetadata(metadata: unknown): PlanApprovalMetadata | undefined { + if (metadata === null || typeof metadata !== 'object') return undefined; + const planApproval = (metadata as { readonly planApproval?: unknown }).planApproval; + if (planApproval === null || typeof planApproval !== 'object') return undefined; + const decision = (planApproval as { readonly decision?: unknown }).decision; + if (decision !== 'approved' && decision !== 'rejected' && decision !== 'cancelled') { + return undefined; + } + const selectedLabel = (planApproval as { readonly selectedLabel?: unknown }).selectedLabel; + const feedback = (planApproval as { readonly feedback?: unknown }).feedback; + return { + decision, + selectedLabel: typeof selectedLabel === 'string' ? selectedLabel : undefined, + feedback: typeof feedback === 'string' ? feedback : undefined, + }; +} + +function trackPlanApprovalResolution(agent: Agent, approval: PlanApprovalMetadata): void { + const selectedLabel = approval.selectedLabel ?? ''; + const normalizedSelectedLabel = normalizeOptionLabel(selectedLabel); + const feedback = approval.feedback ?? ''; + const hasFeedback = feedback.length > 0; + + if (approval.decision === 'cancelled') { + agent.telemetry.track('plan_resolved', { outcome: 'dismissed' }); + return; + } + + if (approval.decision === 'approved') { + if (selectedLabel.length > 0) { + agent.telemetry.track('plan_resolved', { + outcome: 'approved', + chosen_option: selectedLabel, + }); + return; + } + agent.telemetry.track('plan_resolved', { outcome: 'approved' }); + return; + } + + if (normalizedSelectedLabel === 'reject and exit') { + agent.telemetry.track('plan_resolved', { outcome: 'rejected_and_exited' }); + return; + } + + if (normalizedSelectedLabel === 'revise' || hasFeedback) { + agent.telemetry.track('plan_resolved', { + outcome: 'revise', + has_feedback: hasFeedback, + }); + return; + } + + agent.telemetry.track('plan_resolved', { outcome: 'rejected' }); +} + function planTelemetryWasSubmitted(metadata: unknown): boolean { return ( metadata !== null && diff --git a/packages/agent-core/test/agent/basic.test.ts b/packages/agent-core/test/agent/basic.test.ts index 0a3e4da..1da6d1b 100644 --- a/packages/agent-core/test/agent/basic.test.ts +++ b/packages/agent-core/test/agent/basic.test.ts @@ -98,7 +98,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": "