feat: add Tool Script Safety Guard#232
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
AI Code Review好的， 现在我已经有了全面的理解。让我再验证一下 让我检查一下是否存在这样一个问题： 现在有一个关键点： 现在让我完成最后的工作。让我确认一下 BashTool 的默认超时时间是否确实为 300。 BashTool 描述显示默认值为 300。代码中为 让我再检查一下是否没有测试捕获到 BashTool 默认超时时间与安全防护之间的交互。集成测试都显式传递了 timeout。因此这个过度阻塞的问题未被测试覆盖。已确认。 现在撰写审查意见。 让我再确认一个潜在的真正问题： 让我确认一下 实际上，更大的问题是：每个过滤器都会打开/关闭文件（每次调用 让我再验证一下 让我对 现在让我检查 pydantic 的 我已经完成了审查，让我来写最终的审查结论。 发现的问题🚨 Critical
|
|
|
||
| Each tool gets its own filter instance to avoid state leakage. | ||
| """ | ||
| for tool in tools: |
There was a problem hiding this comment.
安全 filter 随请求无界累积
add_tool_safety_filter 每次调用都对工具 filters 列表 append，而 get_tools 每次请求都会调用它，导致同一工具被重复挂载 ToolSafetyFilter，请求被重复扫描 N 次、审计事件重复写 N 份，且 filters 随请求数无界增长。建议注入前剔除已有实例或用去重标记位，保证每个工具只挂一个实例。
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #232 +/- ##
==========================================
Coverage ? 87.98307%
==========================================
Files ? 494
Lines ? 46085
Branches ? 0
==========================================
Hits ? 40547
Misses ? 5538
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AI Code Review我已获取所有必要信息，正在编写审查报告。 发现的问题🚨 Critical（无）
|
AI Code ReviewI have enough context now. Let me write the final review. 发现的问题🚨 Critical
（经核对，安全守卫整体为 fail-closed 设计，未发现会导致核心功能失败或权限绕过的 Critical 问题。）
|
AI Code Review沙盒环境正在反复拦截 基于我的静态分析，以下是我的代码审查结论： 发现的问题🚨 Critical
|
| policy: Optional[PolicyConfig] = None, | ||
| audit_path: Optional[str] = None, | ||
| block_on_review: bool = False) -> None: | ||
| """Attach a fresh ToolSafetyFilter instance to each tool. |
There was a problem hiding this comment.
ToolSafetyFilter 阻塞返回结构缺少统一 success/error
阻塞执行时仅设置 rsp.rsp={...} 和 rsp.is_continue=False，未设置 rsp.error，且返回 dict 与 BashTool 的 {success,error,return_code} schema 不一致，下游 FunctionTool 解析可能无法识别 blocked 结果。建议复用 BashTool 返回结构或在 rsp.error 上明确设置错误。
| blocked: bool | ||
| sanitized: bool | ||
| target: ScanTarget | ||
| language: ScriptLanguage |
There was a problem hiding this comment.
AuditLogger 锁缓存竞态与 resolve() 异常风险
_path_locks 为类级共享 dict，if key not in _path_locks 与赋值非原子，并发首次写同一路径可能生成多把锁导致行交错；Path(path).resolve() 在路径不存在时可能抛异常，使整条工具执行链抛错。建议用 threading.Lock 保护 dict 访问并对 resolve() 做异常兜底。
AI Code Review所有行号已确认。现在开始撰写审查意见。 发现的问题🚨 Critical
|
| )) | ||
|
|
||
| # Check cwd against denied paths | ||
| if request.cwd and self._policy.is_path_denied(request.cwd): |
There was a problem hiding this comment.
基于 cwd 的路径拒绝判定使用 startswith 导致误阻断
is_path_denied 对 denied_paths 用 startswith 匹配，当进程在 /root、/etc 下运行时 execution_dir 会命中 R001_SYSTEM_PATH_OVERWRITE（CRITICAL→DENY），使开启安全守卫后所有 bash 命令被误阻断。建议对 cwd 改用路径等值或直接子目录的精确匹配。
AI Code Review我已经掌握了所需的所有信息，现在让我来写审查报告。 发现的问题🚨 Critical
|
|
|
||
| # Check for sensitive path access (e.g. cat ~/.ssh/id_rsa) | ||
| for sensitive in SENSITIVE_PATHS: | ||
| if sensitive in line and not sensitive.startswith("*"): |
There was a problem hiding this comment.
敏感路径 glob 规则 (.pem/.key) 永远无法命中，造成 R001 漏报
SENSITIVE_PATHS 中 .pem、.key 是 glob 形式，但匹配用的是子串匹配且 not sensitive.startswith("") 直接跳过它们；Python 侧 _check_sensitive_path 同样是子串匹配。建议改为按后缀匹配（如 text.endswith(".pem")）或用 .pem/.key 子串形式，并移除 startswith("") 跳过逻辑。
AI Code Review现在我已经获取了所有信息。让我来编写审查报告。 发现的问题🚨 Critical
|
| from trpc_agent_sdk.skills import SkillToolSet | ||
| return SkillToolSet( | ||
| paths=[str(SKILL_ROOT)], | ||
| filters=[safety_filter], |
There was a problem hiding this comment.
SkillToolSet 用错误参数挂载安全过滤器
SkillToolSet 构造签名只接受 tool_filter 和 **run_tool_kwargs，没有 filters/allowed_cmds 形参，传入的 filters=[safety_filter] 被静默吞入 run_tool_kwargs，ToolSafetyFilter 永远不会挂到 skill 工具上，skill 执行路径实际无安全防护。应改为通过 add_tool_safety_filter 或 toolset 实际暴露的过滤器接入方式注入。
| connection_params=StdioConnectionParams( | ||
| server_params={"command": sys.executable, "args": [str(MCP_SERVER)]}, | ||
| ), | ||
| tool_filter=safety_filter, |
There was a problem hiding this comment.
MCPToolset 把 ToolSafetyFilter 错误地当作 tool_filter 传入
MCPToolset.tool_filter 期望 ToolPredicate 或工具名列表，而非 BaseFilter；且 is_include_all_tools 默认为 True 时 _is_tool_selected 直接返回 True，过滤器被完全忽略，安全过滤器从未挂到 MCP 工具上。应使用 filters=[safety_filter]。
AI Code Review我已经了解了。让我最后确认一件事：关于 在 其实 让我整理一下审查结果。最重要的真正问题是：
让我再次验证 #1 的严重性，并通过检查在 查看 让我撰写审查报告。Based on my review of the safety guard implementation, the integration points, and the tests, here are my findings. 发现的问题
|
AI Code Review我现在已经有了完整且深入的了解。让我来总结一下我的发现。 发现的问题🚨 Critical
|
| block_on_review: If True, NEEDS_HUMAN_REVIEW decisions also block | ||
| execution. Default False (only DENY blocks). | ||
| """ | ||
|
|
There was a problem hiding this comment.
扫描异常路径构造 SafetyReport 缺少必填字段且未真正阻断
scan 抛异常时构造的 DENY SafetyReport 缺少必填的 language/target，未设置 sanitized，会在构造时抛 TypeError 被外层 except 捕获，路径与设计不符且丢失安全上下文。应从 scan_req 补齐 language/target/sanitized=False 并显式 set_blocked(True)。
| if should_block: | ||
| rsp.rsp = { | ||
| "success": False, | ||
| "error": f"TOOL_SAFETY_BLOCKED: {report.summary}", |
There was a problem hiding this comment.
filter 阻断路径未调用 set_blocked(True) 导致审计字段不一致
阻断时只设置 rsp 与 is_continue=False，未调用 report.set_blocked(True)，使审计日志 blocked 记为 False，与 BashTool 路径不一致。block_on_review 触发 NEEDS_HUMAN_REVIEW 阻断时同样遗漏，应在阻断前补 report.set_blocked(should_block)。
2 participants
Description
实现 Tool Script Safety Guard，用于在 Tool、Skill、MCP Tool 和 CodeExecutor 执行前进行静态安全扫描和风险控制。
Resolves #90
新增
trpc_agent_sdk/tools/safety/模块，支持 Python 和 Bash 脚本安全检测，覆盖以下风险类型：Key Features
Python AST 静态分析：
Bash 静态分析：
shlex解析命令结构策略控制：
tool_safety_policy.yaml风险决策：
allowdenyneeds_human_review提供多种接入方式：
Integration
接入已有执行链路：
trpc_agent_sdk/tools/file_tools/_bash_tool.pyenable_safety_guardsafety_scannerblock_on_reviewtrpc_agent_sdk/code_executors/local/_unsafe_local_code_executor.py_scan_code_block()默认保持关闭：
不影响已有用户行为，保持向后兼容。
Wrapper Supporting
新增：
支持对已有 CodeExecutor、ToolSet 和 MCP Tool 进行安全包装。
Examples and Tests
新增示例：
新增测试覆盖：
Validation
测试结果：
✅ 188 tests passed
✅ 23/23 安全扫描样例通过
✅ 高危样本检出率 100%（32 条）
✅ 安全样本误报率 0%（10 条）
✅ 密钥、删除、网络风险检测覆盖率 100%
✅ 500 行代码扫描：
✅ 策略配置修改可实时生效