feat(security): 统一工具执行安全拦截并打通 Ask 审批闭环#45
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
minorcell
left a comment
There was a problem hiding this comment.
代码审查总结
本 PR 为工具执行添加了安全拦截与 Ask 审批闭环。整体实现扎实,但有以下问题需修复:
1. internal/server/infra/tools/security.go
问题: securityApprovalKey 在输入为空时返回空字符串,导致 ApproveSecurityAsk 和 consumeSecurityAskApproval 静默失败。
影响: 当 toolType 或 target 为空时,审批逻辑不会执行任何操作,可能导致审批记录丢失。
建议: 对无效输入返回错误或记录警告日志,而非静默返回空字符串。
2. internal/server/infra/tools/bash.go
问题: Windows 平台的 shell 回退方案可能不适用于所有 Windows 系统。
影响: 代码假设 Windows 上有 PowerShell,但某些系统可能只有 cmd.exe 或禁用了 PowerShell。
建议: 添加 cmd.exe 作为备选方案,或使 shell 可配置。
3. internal/tui/core/update.go
问题: /y 命令在 toolExecuting 为 true 时存在竞态条件。
影响: 如果在执行工具时调用 /y,函数会提前返回但不会恢复 pendingApproval,导致状态不一致。
建议: 将审批排队等待后续执行,或明确拒绝 /y 命令并提示用户当前有其他工具在执行。
4. internal/server/service/security_service.go
问题: SecurityService 接口定义与实现不一致。
影响: 接口定义了 Initialize 方法,但不同文件中的接口定义可能造成混淆。
建议: 确保所有文件中的接口定义一致,或使用单一接口定义。
总结
整体实现良好,合并前请修复以上问题:
- 处理
securityApprovalKey的空输入 - 改进 Windows shell 兼容性
- 修复审批处理的竞态条件
- 明确接口定义
| "go-llm-demo/configs" | ||
| "go-llm-demo/internal/server/infra/provider" |
There was a problem hiding this comment.
“go-llm-demo” 这个表述或许应该换了,可以在下一个PR中处理。
| - 严禁执行高危破坏性命令(如无边界删除、系统级破坏、恶意下载执行等)。 | ||
| - 涉及路径操作时,默认限定在当前工作区内,不对工作区外路径进行读写。 | ||
|
|
||
| 你可以调用edit,grep,list,read,write,bash工具,规范和opencode保持一致 |
There was a problem hiding this comment.
模型知道 opencode 是什么吗?
模型知道 opencode 规范是什么吗?
There was a problem hiding this comment.
收到,豆包写的提示词,我这就改
| 当需要在工作区目录内执行任意终端命令、脚本、系统操作时,调用 bash 工具。 | ||
| 必填参数:command(待执行的 bash 命令字符串,如 grep 'error' ./log) | ||
| 可选参数:workdir(命令执行目录,字符串,默认工作区根目录),timeout(命令超时时间,整数,单位毫秒,默认 120000),description(命令用途说明,字符串,默认空) |
There was a problem hiding this comment.
这些不应该放在系统提示词里,你们的工具定义中已经有 tool params 的 schema 了吧
There was a problem hiding this comment.
是的,历史遗留,这个pr就处理
There was a problem hiding this comment.
看了一下,工具定义我还没加到上下文,这个可能要等到下个pr修,这个pr管不来这么多
There was a problem hiding this comment.
可以,这些问题。下一个PR要做的事情,都可以先记录一个issue
| } | ||
|
|
||
| 调用工具时,你只能严格遵守json格式,不能输出多余字符,一次只能调用一个工具 | ||
| 输出json完成后,主动终止对话,等待工具响应 No newline at end of file |
There was a problem hiding this comment.
“主动终止对话,等待工具响应”
这个不是模型的行为吧(这个 loop 本质上还是我们自己控制的)
所以这里你告诉模型这些信息,它也理解不了,甚至可能会是一种噪音。
There was a problem hiding this comment.
明白了,主要是开始调试没有Todo list的时候有一些思考模型不会终止流式对话,会出现2次输出,为了方便调试加的,这就删
There was a problem hiding this comment.
看了一下,工具定义我还没加到上下文,这个可能要等到下个pr修,这个pr管不来这么多
There was a problem hiding this comment.
看了一下,工具定义我还没加到上下文,这个可能要等到下个pr修,这个pr管不来这么多
这个评论不是在这里的吧?
|
欸不是怎么还有冲突不是拉下来重写了吗,我再看看 |
方便快速会议一下吗? |
背景
当前项目已具备黑/白/黄名单策略与匹配能力,但在工具执行链路上仍存在“策略未必被强制接入”和“Ask 命中后缺少完整审批闭环”的风险。为确保安全策略真正落地,本 PR 一次性完成从启动注入、执行前拦截到用户确认放行的全链路收敛。
本 PR 主要改动
1) 启动阶段统一注入安全服务(TUI / Server)
涉及:
cmd/tui/main.gocmd/server/main.go实现内容:
SecurityConfigRepositorySecurityServiceInitialize(<workspaceRoot>/configs/security)加载黑/白/黄名单tools.SetSecurityChecker(...)注入工具层效果:安全策略从“可配置”升级为“启动即生效”。
2) 工具层新增统一安全守卫,并支持 Ask 一次性放行
涉及:
internal/server/infra/tools/security.go实现内容:
SetSecurityChecker/getSecurityCheckerguardToolExecution统一处理allow / deny / askApproveSecurityAsk(toolType, target)发放一次性放行令牌consumeSecurityAskApproval(...)在下一次同目标调用时消费令牌效果:
ask不再只是“阻断”,而是可控地“确认后继续执行”。3) Bash 工具执行前接入安全拦截
涉及:
internal/server/infra/tools/bash.go实现内容:
guardToolExecution("Bash", command, toolName)策略行为:
deny:拒绝执行并返回安全错误ask:挂起执行,等待上层确认效果:bash 从“直接执行”升级为“安全决策前置执行”。
4) TUI 打通 Ask 审批闭环(确认后自动重试)
涉及:
internal/tui/core/model.gointernal/tui/core/update.go实现内容:
pendingApproval状态保存待确认工具调用ask后提示用户确认并挂起调用/y:批准本次调用(发放一次性放行并重试原调用)/n:拒绝本次调用效果:形成“命中 ask → 用户确认 → 执行重试”的完整交互闭环。
5) 安全服务接口补齐初始化能力
涉及:
internal/server/service/security_service.go实现内容:
SecurityService接口包含domain.SecurityCheckerInitialize(configDir string) error效果:入口层可标准化初始化并统一注入安全检查器。
6) Persona 安全指引增强
涉及:
configs/persona.txtconfigs/persona.txt.example新增约束:
deny不可绕过ask先确认用户效果:模型行为与运行时安全策略保持一致。
7) 测试补充与回归验证
涉及:
internal/server/infra/tools/bash_security_test.go已验证:
go test ./internal/tui/core ./internal/server/infra/tools ./internal/server/servicego test ./...结果:全部通过。
预期收益
Close 【Test】验证 ReAct loop 中 AI 输出 JSON 兼容及工具执行稳定性 #20
风险与兼容性说明
/y与/n(如文档存在旧命令需同步更新)。(toolType, target)精确匹配并单次消费,不会长期授权。