feat(permissions): AI auto-approve judge with approval notes, user rules, sensitive resources, and unattended tiers - #2229
Conversation
…he AI auto-approve judge
…tiers, and review exemption
Extends the AI auto-approve permission mode with three capabilities on top of
the approval-notes work:
1. Configurable sensitive resources. `ProjectPermissionConfig` gains a
`sensitive_resources: Vec<String>` list persisted in the project-level
`tool_permissions.json` (same file as project rules). User markers append
to the built-in sensitive list (`.env`, credentials, private keys, tokens,
...) and use the same case-insensitive substring matching. The list is
deliberately NOT exposed to the judge prompt: it exists only to stop
read-only tools from taking the deterministic fast path when a requested
resource hits a marker, forcing judge/escalate instead.
2. AI auto-approve tiers. `AiAutoApproveMode::{Aggressive, Standard, Passive}`
(default Standard) is a session-level setting that only changes the
`escalate` path: Aggressive auto-allows escalated calls with the fixed note
`aggressive mode, auto-approve all escalated tool calls`; Passive
auto-rejects them with `passive mode, reject all escalated tool calls`;
Standard keeps the interactive prompt. Already-`allow`ed safe calls and
already-`deny`ed critical calls are unaffected. The judge system prompt
explains the two fixed notes and marks `.bitfun/tmp`, `.bitfun/config`, and
`.bitfun/data` as the app's own safe scratch/configure space; it does not
write the dynamic tier value, so switching tiers does not invalidate the
turn-level KV cache.
3. Review exemption. Deep Review / review agents (CodeReview, DeepReview,
ReviewJudge, ReviewFixer, ReviewWorker and legacy review worker types) are
read-only and run unattended, so an interactive prompt would stall the whole
review. The sensitive-resource branch is skipped for them and their reads
proceed, on the assumption that secrets are already excluded by the
repository's ignore rules.
Wire/UI: `PermissionRequest` gains `permission_mode` so the frontend can
consume it without a global lookup; `get/save_project_permission_rules`
round-trips `sensitive_resources`; the project permissions dialog adds a
sensitive-resources editor; the request panel hides the "always allow" button
in ai_auto mode; session config persists the tier. Three locales updated.
|
@wsp1911 Prioritize reviewing the reasonableness of runtime changes and the rationality of the review process. |
wsp1911
left a comment
There was a problem hiding this comment.
当前实现存在权限安全边界和跨会话状态隔离问题,建议修复以确保 PR 描述中的用户规则和敏感资源语义真正成立。
[P1] 未知风险等级会被放行
File: src/crates/assembly/core/src/agentic/execution/permission_ai_judge.rs
Lines: 563–579
JudgeResponse.risk_level 是普通 String,因此未知值可以通过 JSON 反序列化。parse_risk_level 对未知值返回 None,但 (JudgeDecision::Allow, _) 随后仍返回 Allow。
这种响应不会触发现有重试,因为反序列化已经成功。例如模型返回:
{"decision":"allow","risk_level":"critcal"}最终会被自动批准。这是权限判断的 fail-open 路径。未知风险等级应重试,重试耗尽后返回 Escalate,不能继续放行。
[P1] 原始命令可能泄露给权限模型
Files:
src/crates/assembly/core/src/agentic/tools/implementations/bash_tool.rssrc/crates/assembly/core/src/agentic/execution/permission_ai_judge.rs
Bash 会把完整命令作为 PermissionIntent.resources,而 render_current_tool_call 对 resources 只做路径处理和 XML escaping,没有进行敏感信息脱敏。
arguments 的脱敏逻辑也只检查 JSON key 是否包含 token、password、auth 等关键词,不会检查 command 字符串内部。因此:
curl -H "Authorization: Bearer ..." ...会通过 resources 和 arguments 进入 fast model prompt。
在把命令发送给权限模型前,需要对命令字符串本身进行可靠脱敏;无法安全脱敏的敏感命令应直接升级给用户,而不是发送原文。
[P1] AI 自动批准档位实际是全局设置
Files:
src/web-ui/src/flow_chat/components/ChatInput.tsxsrc/web-ui/src/infrastructure/config/services/PermissionConfigService.tssrc/crates/assembly/core/src/agentic/execution/round_executor.rs
handleAiAutoApproveModeChange 将 aggressive/standard/passive 写入:
tool_permissions.interaction.ai_auto_approve_mode
这是用户级全局配置。运行时每个 round 也直接从 global_config.tool_permissions 读取该值。
虽然总体 permission mode 已支持 session/turn override,但 AI auto-approve 的子档位没有进入对应的 session/turn 状态。因此一个会话切换到 aggressive 后,其他处于 ai_auto 的会话也会使用 aggressive。
该档位位于 session permission 控件中,并被呈现为当前会话的权限状态,因此应随现有 session/turn permission mode 链路传递,而不是修改全局配置。
[P1] Review Agent 会绕过敏感资源保护
File: src/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs
Lines: 809–825
所有 review 类型都跳过了 sensitive_read_forced 分支,但显式 Read 不受 .gitignore 保护。更重要的是,ReviewFixer 明确拥有 Edit、Write 和 ExecCommand,并且 is_readonly() 返回 false。
因此,review agent 读取用户配置的敏感路径时可能静默通过。无人值守不应成为读取敏感资源的授权依据。
[P2] Tool history 保存了错误的 action
File: src/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs
Lines: 629–634
judge_tool_history 当前保存:
action: task.tool_call.tool_name.clone()因此历史 action 是 Write、ExecCommand 等 wire tool name,而当前 permission request 使用的是 edit、bash 等 PermissionIntent.action。
Prompt 要求通过 “same action and matching resource” 匹配用户之前带 note 的批准。当前实现无法进行稳定、确定的同 action 匹配,只能依赖模型猜测 Write 与 edit、ExecCommand 与 bash 是否等价。
历史记录应保存实际参与权限判断的 PermissionIntent.action。
[P2] 保存敏感资源配置后没有立即失效缓存
Files:
src/apps/desktop/src/api/agentic_api.rssrc/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs
敏感资源配置按 workspace 缓存 30 秒。ToolPipeline 已提供:
clear_sensitive_markers_cache_for_workspace但当前仓库中没有调用点。save_project_permission_rules 写入 tool_permissions.json 后直接返回,没有通知 runtime 清理缓存。
如果缓存已经预热,用户新增敏感路径后,旧配置最多仍会使用约 30 秒。在此期间该路径仍可能走确定性只读批准。
保存成功后应通过 runtime owner 立即清除对应 workspace 的缓存,并覆盖本地和远程 workspace 路径。
[P2] Always approval 的 note 在后续 turn 丢失
File: src/crates/assembly/core/src/agentic/execution/permission_ai_judge.rs
Lines: 427–428, 737–743
PermissionReply::Always { feedback } 正确保存了用户 note,但渲染 UserRuleKind::AlwaysApproved 时只输出:
Always approved: ...
没有输出 rule.note。
同一 turn 内 note 可能仍通过 tool history 出现,但进入后续 turn、依赖持久化 user rule 时,该限制或说明会丢失。这与 “approvals with a note” 的语义不一致。
AlwaysApproved 的 prompt 表达也应包含经过 escaping 的 note。
[P2] Tool history 没有稳定的 turn-wide 顺序
Files:
src/crates/assembly/core/src/agentic/tools/pipeline/state_manager.rssrc/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs
get_dialog_turn_tasks 直接收集 DashMap 的迭代结果,没有排序;judge_tool_history 随后按该顺序渲染。DashMap 不承诺迭代顺序,因此既有历史条目可能在后续 judge 请求中重排。
这无法严格保证代码和 prompt 声称的 “monotonically growing tool history”,并会产生两个问题:
- 用户 note、成功/失败结果可能不按因果顺序呈现。
<tool_history>部分可能重排,降低后续 prompt prefix/KV cache 的稳定性。
现有 tool_call_order 只表示当前模型 round 内的位置,每次 execute_tools 都从 0 开始,不能单独作为整个 turn 的排序键。
这里需要稳定的 turn-wide sequence,或者由稳定的 round/时间顺序与确定性 tie-breaker 组成的排序键,并增加跨 round、并行完成和重复构建 history 的测试。
建议至少补充以下回归覆盖:
allow + unknown risk_level必须升级而不是批准;- shell command 中 Authorization/Bearer/password 的脱敏;
- 两个 session 使用不同 AI auto-approve 档位;
ReviewFixer读取用户标记的敏感路径;- tool history 使用 permission action;
- 保存敏感资源后立即观察到新配置;
- Always approval note 在后续 turn 仍进入 prompt;
- 多 round、多 tool 和并行完成情况下 history 顺序稳定且只追加。
Fixes #2228
Summary
Extends the AI auto-approve permission mode with five capabilities:
<user_rules>section the fast-model judge reads between the stable session context and the growing tool history. Subagents inherit the parent session's rules.sensitive_resourceslist extends the built-in sensitive markers; it is used only to stop read-only tools from taking the deterministic fast path, never exposed to the judge prompt.aggressive / standard / passiveswitch that only changes theescalatepath, for unattended runs.Design
1. Approval notes: wire-compatible DTO extension
PermissionReply::OnceandPermissionReply::Alwaysbecome struct variants with an optionalfeedbackfield (#[serde(default, skip_serializing_if = "Option::is_none")]):The old wire shape
{"reply":"once"}still serializes unchanged and still deserializes (contract test covers both directions). All construction/match sites across the workspace were updated (desktop command, CLI peer host, ACP prompt, IPC protocol tests, app-server tests).2. Note propagation
respond_permissionfillsfeedbackintoOnce/Alwaysreplies (the request DTO already carried the field for rejections).PermissionAuthorization::Allowednow carriesuser_feedback; the pipeline appendsUser approved this tool call with feedback: {note}to the tool result so the agent honors the intent in the same turn.(user note: "...")so the judge sees in-turn intent.source: user.3. Session-scoped user rules for the judge
Prompt structure (stable prefix, then growing history, then the call):
Extraction (
load_session_rules): from the project permission audit, filtered to the session (plus the parent session for subagents) and tosource: userreplies with durable intent:Alwaysapprovals →Always approved: {action} on {resources}User approved {action} on {resources} with note: "..."User rejected {action} on {resources}: "..."(so the agent stops retrying refused operations)Project grants are appended separately as
Persistent grant:authorization facts so the judge knows what is already auto-allowed for the project.Ordering and cap: newest-first (audit recency), deduplicated by a stable rule id (kind + action + resources), capped at
MAX_USER_RULES = 50.Turn stability / KV cache: rules are built once per dialog turn (cache key = project + session + dialog turn) and reused for every judge call in that turn — the prefix is byte-stable and KV-cache friendly. New approvals inside a turn only appear in the growing
<tool_history>with their note marker; they are formalized into<user_rules>at the next turn boundary.Fail-closed: the rules section opens with a fixed preamble — rules express user intent, are not blank checks; only directly matching calls count, and dangerous operations are never approved by a rule. The system prompt additionally teaches the judge to treat an in-turn
(user note: "...")on a directly matching history entry as pre-approved intent.Subagent inheritance:
load_user_rules_for_batchmergesdelegation.parent_session_idinto the session filter, so delegated tool calls judge against the same user intent.4. LRU ordering was implemented, measured, and removed
The first iteration ordered rules by an LRU of "rule hits": each judged request was structurally matched (action + wildcard resources) against the rules and matches were recorded, with per-session persistence (
permission-rule-lru/<session>.json).Real-world testing showed this mechanism did not work as intended:
The LRU machinery (matcher, port, JSON store, record/flush methods) was therefore removed entirely. Ordering is plain audit recency — simple, predictable, and aligned with "the most recent approval expresses the user's current intent". The per-turn byte-stable cache remains, as it is an independent KV-cache win.
5. Configurable sensitive resources
ProjectPermissionConfiggainssensitive_resources: Vec<String>(#[serde(default, skip_serializing_if = "Vec::is_empty")]), persisted in the project-leveltool_permissions.json(same file as project rules) and managed from a new "sensitive resources" editor in the project-permissions dialog.normalize_sensitive_resourcestrims and drops blank entries on both the desktop command and the frontend save path.6. AI auto-approve tiers (unattended)
AiAutoApproveMode::{Aggressive, Standard, Passive}(defaultStandard) is a session-level setting that only changes theescalatepath:aggressive→ escalated calls are auto-allowed with the fixed noteaggressive mode, auto-approve all escalated tool calls.passive→ escalated calls are auto-rejected with the fixed notepassive mode, reject all escalated tool calls.standard→ escalated calls keep the interactive prompt (previous behavior).Calls the model already judged
allow(including the read-only fast path) and calls it already judgeddeny+criticalare unaffected by the tier: tiers never weaken the deny-critical rejection nor broaden allow.The judge system prompt explains the meaning of the two fixed notes (so it understands those history entries were produced unattended, not by per-call user approval) but does not write the dynamic tier value — switching tiers therefore does not invalidate the turn-level KV cache. The same prompt declares
.bitfun/tmp,.bitfun/config, and.bitfun/datato be the app's own scratch/configure/data space and safe to read/write.7. Review exemption
Deep Review / review agents (
CodeReview,DeepReview,ReviewJudge,ReviewFixer,ReviewWorker, and the legacy review worker types) are read-only by construction and run unattended: an interactive prompt would stall the entire review. The sensitive-resource branch is skipped for them and their reads proceed, on the assumption that secrets are already excluded by the repository's ignore rules. All other safety boundaries still apply.8. Safety boundaries (unchanged behavior)
.env, credentials, private keys, tokens, ... plus the user-configured list) still go through the judge.rm -rf /and similar destructive/secret-exposing/system-wide operations are still rejected outright (deny+critical→Reject), and the judge is explicitly told rules never cover them.escalate(ask the user).Testing
{"reply":"once"}round-trip); permission manager reply/audit with notes; sensitive-resource normalization (trim + blank filtering); tier parsing/defaults.tool_pipeline): approval with a note → tool executes and the result contains the note; approval without a note → result text unchanged; escalation → user approval with note flows through; sensitive read forced past a static allow policy into the judge; aggressive/passive tiers auto-reply to escalated calls with the fixed notes; review agent + sensitive resource → read proceeds without an interactive prompt.<user_rules>rendered at the next turn with the user's notes; similar operations auto-approved from the next turn; subagent judge input contains the parent session's rules;rm -rf /still rejected outright with "not covered by any user rule"; sensitive read (.envundersecrets/) escalated to the user; unattended tiers auto-approve/auto-reject escalated calls; review runs not stalled by sensitive reads.