Skip to content

fix(less-computer): 护栏高危系统命令设为不可批准,修批准注入畸形规则 (#679)#688

Open
appergb wants to merge 1 commit into
betafrom
fix/issue-679-guard-non-approvable
Open

fix(less-computer): 护栏高危系统命令设为不可批准,修批准注入畸形规则 (#679)#688
appergb wants to merge 1 commit into
betafrom
fix/issue-679-guard-non-approvable

Conversation

@appergb

@appergb appergb commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

User description

关联 issue

Closes #679

问题

Less Computer 护栏审批流(coordinator/dictation.rs)对每个命中的 HIGH_RISK_PATTERNS 子串一律 format!("Bash({p}:*)") 生成 allow 规则:

  • "sudo "(带空格)→ Bash(sudo :*) ≠ deny Bash(sudo:*) → deny 未被移除(批准静默失效),同时把畸形规则注入 allowed_tools
  • "dd if=" / "chmod -r 777 /" / "chown -r" 同理。

方案(A:安全优先)

新增 guard::deny_rule_for_pattern(pattern) -> Option<&'static str>

  • 可批准命令(rm -rf / rm -fr / git push --force / git push -f / git reset --hard / git clean -fd / git clean -f -d)→ Some(精确 deny 规则),批准时按此从 deny 精确移除、并加入等值 allow(移除的 deny 与加入的 allow 严格一致)。
  • 不可批准命令(sudo / dd / mkfs / chmod / chown / shutdown / reboot / 管道执行 / fork 炸弹 / 写块设备)→ None:即使被「批准」也保持拦截(fail-closed),不注入任何 allow。

审批流改用 filter_map(deny_rule_for_pattern) 构造 allow 规则。

这是把无法用 Bash(<prefix>:*) 安全表达、或提权/毁盘/系统级的命令设为「不可批准」(不给 agent 这些权限),而不是让批准去解禁它们。

测试

cargo test --lib guard::   # 11 passed
  approvable_pattern_maps_to_existing_deny_rule ... ok   (每个可批准 pattern 的映射规则确实在 default_deny_rules 中)
  dangerous_system_commands_are_not_approvable ... ok    (sudo/dd/mkfs/chmod/chown/shutdown/reboot/管道/fork 一律不可批准)
  approvable_git_and_rm_map_to_exact_rules ... ok

来源:2026-06-16 全仓多 Agent 审计(护栏专项)。安全相关,按方案 A(不可批准、fail-closed)实现。


PR Type

Bug fix, Enhancement


Description

  • 新增deny_rule_for_pattern函数,区分可批准与不可批准命令

  • 修改审批流使用filter_map,避免畸形allow注入

  • 添加单测验证可批准映射与危险命令不可批准


Diagram Walkthrough

flowchart LR
  A["approved patterns"] --> B["expand equivalents"]
  B --> C["deny_rule_for_pattern"]
  C -- "Some(rule)" --> D["Remove from deny, add allow"]
  C -- "None" --> E["Keep blocked, no injection"]
Loading

File Walkthrough

Relevant files
Enhancement
guard.rs
添加deny_rule_for_pattern及测试                                                             

openless-all/app-tauri/src-tauri/src/coding_agent/guard.rs

  • 新增deny_rule_for_pattern函数,将可批准模式映射到精确deny规则,不可批准命令返回None
  • 添加三个测试函数:验证可批准规则存在、危险命令不可批准、GIT和rm映射正确
Bug fix
dictation.rs
修改审批流使用filter_map                                                                               

openless-all/app-tauri/src-tauri/src/coordinator/dictation.rs

  • 修改allow_rules生成逻辑:使用filter_mapdeny_rule_for_pattern替代直接格式化
  • 不可批准命令即使被批准也保持拦截,不再注入畸形allow规则

旧审批流对所有命中 pattern 一律 format!("Bash({p}:*)") 生成 allow 规则:
- 对带空格/参数的子串("sudo " → "Bash(sudo :*)","dd if=" → "Bash(dd if=:*)",
  "chmod -r 777 /"、"chown -r")生成的串既不等于 deny "Bash(sudo:*)" 等(故 deny
  没被移除、批准静默失效),又把畸形规则注入了 allowed_tools。

改为:新增 guard::deny_rule_for_pattern(pattern),把「可批准」命令(rm -rf / git
push --force / git reset --hard / git clean 等)映射到其在 default_deny_rules 里的
**精确** deny 规则;提权/毁盘/系统级命令(sudo / dd / mkfs / chmod / chown / shutdown
/ reboot / 管道执行 / fork 炸弹)返回 None = 不可批准,即使被批准也保持拦截
(fail-closed),不再注入畸形 allow。审批流用 filter_map 取精确规则,移除的 deny 与
加入的 allow 严格一致。

附单测:每个可批准 pattern 的映射规则确实存在于 default_deny_rules;危险系统命令一律
不可批准。
@appergb

appergb commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

@claude 请审核(安全相关):把无法安全前缀表达 / 提权毁盘系统级命令设为「不可批准」(deny_rule_for_pattern→None,fail-closed),并修掉旧的「批准注入畸形 Bash(sudo :*)、deny 又没移除」不一致。重点看 (1) 可批准集(rm -rf / git push --force / git reset --hard / git clean)是否合适、是否过宽;(2) 不可批准集是否完整覆盖了 HIGH_RISK_PATTERNS 里所有危险项;(3) filter_map 后 allow 与移除的 deny 是否严格一致。

@claude

claude Bot commented Jun 16, 2026

Copy link
Copy Markdown

Claude finished @appergb's task in 3m 16s —— View job


安全审核结果

  • 初始化审核任务
  • 读取 guard.rs 变更
  • 读取 dictation.rs 变更
  • 审核要点 (1):可批准集是否合适/过宽
  • 审核要点 (2):不可批准集是否完整覆盖 HIGH_RISK_PATTERNS
  • 审核要点 (3):filter_map 后 allow 与 deny 是否严格一致
  • 输出审核结论

总结

方向正确,核心修复逻辑无误,有一个中等风险设计问题 + 一个测试覆盖缺口。


(1) 可批准集:是否合适?是否过宽?

可批准集(7 项)rm -rf / rm -fr / git push --force / git push -f / git reset --hard / git clean -fd / git clean -f -d

总体合适,但有一个值得关注的问题:

⚠️ git clean -fd / git clean -f -d 设为可批准存在数据丢失风险

这两条命令永久删除未跟踪文件create_git_snapshot(调用链见 dictation.rs:621)调用的是标准 git stash,默认 不包含未跟踪文件(需要 -u/--include-untracked 标志)。

  • rm -rf 批准后虽然宽泛,但被删的通常是已跟踪内容,gitlog / reflog 可找回;
  • git clean -fd 批准后删掉的是未跟踪文件,如果快照时没加 -u,回滚路径不存在。

建议:核实 create_git_snapshot 是否使用 git stash -u;若没有,考虑将 git clean -fd / git clean -f -d 移入不可批准集,或在批准前展示一条"无法回滚未跟踪文件"的警告。

前缀规则天然宽泛Bash(rm -rf:*) = 允许一切 rm -rf 参数)是已知设计取舍,注释也有说明,不另计问题。


(2) 不可批准集:是否完整覆盖 HIGH_RISK_PATTERNS 全部 20 项?

覆盖完整deny_rule_for_pattern_ => return None 是穷尽默认值,凡不在显式 Some(...) 分支里的 pattern 全部落到 None。把 HIGH_RISK_PATTERNS 20 项逐一对照:

pattern deny_rule_for_pattern 结论
rm -rf / rm -fr Some(...) 可批准
git push --force / git push -f Some(...) 可批准
git reset --hard Some(...) 可批准
git clean -fd / git clean -f -d Some(...) 可批准(见上节)
sudo / mkfs / dd if= None via _ 不可批准 ✓
shutdown / reboot None via _ 不可批准 ✓
chmod -r 777 / / chown -r None via _ 不可批准 ✓
> /dev/sd / :(){ None via _ 不可批准 ✓
| sh / | bash None via _ 不可批准 ✓
|sh / |bash(无空格变体) None via _ 不可批准 ✓(但测试缺失,见下)

⚠️ 测试缺口dangerous_system_commands_are_not_approvable 测试了 "| sh" / "| bash",但遗漏了 HIGH_RISK_PATTERNS 中的 "|sh""|bash"(无空格变体,guard.rs:28-29)。虽然 _ arm 保证了运行时正确,但若将来有人给这两个 pattern 加了显式 Some(...) 分支,测试不会报警。

Fix this →


(3) filter_map 后 allow 与移除的 deny 是否严格一致?

严格一致,逻辑无漏洞。 关键路径(dictation.rs:764-773):

let allow_rules: Vec<String> = approved_patterns
    .iter()
    .filter_map(|p| crate::coding_agent::guard::deny_rule_for_pattern(p))  // &'static str
    .map(|rule| rule.to_string())
    .collect();
if !allow_rules.is_empty() {
    deny.retain(|d| !allow_rules.iter().any(|a| a == d));  // 精确字符串匹配
}
// ...
req.allowed_tools.extend(allow_rules);  // 同一份字符串

三点一致性保障:

  1. deny_rule_for_pattern 返回 &'static str 字面量,与 default_deny_rules() 里的字符串物理上同一来源guard.rs:80-90guard.rs:100-137 对比逐一确认);
  2. deny.retain== 精确匹配删除,不存在前缀/模糊匹配误删;
  3. req.allowed_tools.extend(allow_rules) 添加的是同一批 String,「移除的 deny」与「加入的 allow」值相等;
  4. approvable_pattern_maps_to_existing_deny_rule 测试强制验证该不变量(每个可批准 pattern 映射到的 rule 必须真实存在于 default_deny_rules()),是最关键的回归防线。

其他观察(非 blocker)

|sh/|bash/:(){/> /dev/sddefault_deny_rules() 中无对应条目:代码注释已解释原因(命令中段/shell 语法,无法用前缀规则表达),运行时靠 is_high_risk_command 探测兜底。这是 headless claude -p 的已知架构限制,不属本 PR 引入,但值得在后续跟进(如考虑在 maybe_request_approval 或命令执行前加更严格的 shell 语法检查)。


结论

要点 结果
核心 bug 修复(畸形规则注入 + deny 未移除) ✅ 正确
可批准集合理性 ⚠️ git clean -fd 需确认快照是否含未跟踪文件
不可批准集完整性(逻辑) ✅ 完整(_ arm 穷尽)
不可批准集测试覆盖 ⚠️|sh / |bash 无空格变体
allow/deny 严格一致性 ✅ 一致

@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

679 - Fully compliant

Compliant requirements:

  • Fixed the mismatch by introducing a single mapping function deny_rule_for_pattern that links each high-risk pattern to its exact deny rule.
  • Approved commands now correctly remove the corresponding deny rule (deny.retain checks for exact match).
  • Non-approvable patterns return None, so no malformed allow rules are generated.
  • Commands like sudo, dd, mkfs, chmod, chown, shutdown, reboot, pipe execution, fork bomb, write block device are explicitly marked non-approvable.
  • Three unit tests added covering: approvable pattern map existence, dangerous commands not approvable, and exact rule mappings.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@HKLHaoBin

Copy link
Copy Markdown
Contributor

PR 合并冲突提示

1. dictation.rs(与 #682#692#666

2. coding_agent/guard.rs(与 #638

建议: 若两 PR 都会合,请 rebase #638 时留意 guard.rs 与本 PR 的合并结果。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[security][less-computer] 批准 sudo/dd/chmod/chown 后护栏 deny 未解除(静默空操作)+ 注入畸形 allow 规则

2 participants