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.94805%
==========================================
Files ? 494
Lines ? 46200
Branches ? 0
==========================================
Hits ? 40632
Misses ? 5568
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)。
b85b7d7 to
a304488
Compare
AI Code Review现在我已经掌握了准确的行号。让我再重新思考一下，关于 BashTool 的稳定性问题是否真的值得提出警告——因为它周围的外部代码中确实有一个宽泛的 让我再重新考虑一下 SafeCodeExecutor 是否暴露出了一个更严重的问题——它没有宽泛的 except，所以扫描器的异常会向上抛出给调用链，这可能导致 agent 在执行代码时崩溃。这比 BashTool 的问题更严重。值得作为警告提出。 我还需要验证一下 让我完成最终总结。 发现的问题
|
AI Code Review我已经完整审查了 发现的问题🚨 Critical
|
|
|
||
| # Block? | ||
| if should_block: | ||
| rsp.rsp = { |
There was a problem hiding this comment.
ToolSafetyFilter._before 通过 mutation 阻断但未 return FilterResult
_before 直接修改传入的 rsp（rsp.rsp=...、rsp.is_continue=False）来阻断执行，却没有 return 阻断结果。当前依赖框架对同一对象的副作用传递，属脆弱契约；一旦上游改为以 _before 返回值作为结果，阻断会静默失效并放行危险命令。建议显式 return 阻断结果或与框架确认契约，并补一条阻断后 handle 不被调用的端到端测试。
AI Code Review我将停止运行命令（它们被持续拦截），并基于我对代码的深入阅读来完成我的审查。我已经有足够的上下文来提供审查结果。 让我记录下从阅读
此外，
让我专注于验证过的、高置信度的问题。最清晰的是：
让我再重新考虑一下去重问题，这是最能从代码中验证的“正确性” Bug：
但是多行上的 真正的一个：在 让我重新考虑 现在是一个具体的 Bug： 让我敲定那些高置信度的问题： 警告 1: 警告 2: 警告 3: 鉴于我已经深入阅读且命令执行受阻，让我写一份包含最可辩护发现的审查。我会保持简洁，并使用 diff 文件行号。 让我验证 对于执行器扫描不传递元数据的问题： 我还要指出 让我再检查一下： 一个是警告级别的真正安全问题： 一个是明确的 Bug：在 让我写出最终的审查报告。我将包含：
我还将指出 我还要重新考虑测试有效性问题： 现在开始撰写。基于对 发现的问题
|
…atching to the is_path_denied function
AI Code Review正确的 API 应该是 现在我已经有了所需的一切。让我再验证一下 让我快速确认一下 Python 版本的行为不会产生干扰 —— 即使属性没有 setter，在 Python 3.11 及以上版本中，普通实例的属性赋值仍可能成功吗？不会：一个没有 setter 的 让我来写评审报告。我还想指出一个值得一提的稳定性问题： 再检查一个潜在问题： 再看一下： 现在开始撰写评审报告。 发现的问题🚨 Critical
|
| Each tool gets its own filter instance to avoid state leakage. | ||
| """ | ||
| for tool in tools: | ||
| tool.filters = [f for f in tool.filters if not isinstance(f, ToolSafetyFilter)] |
There was a problem hiding this comment.
向只读 filters 属性赋值导致安全注入崩溃
add_tool_safety_filter 直接对 tool.filters 赋值，而 FilterRunner 上的 filters 是只读 @property 无 setter，真实 BaseTool/MCPTool/SkillTool 实例调用会抛 AttributeError，使 SafetyWrappedToolSet.get_tools() 及示例注入路径运行即崩溃、安全防护失效。应改用 tool.add_one_filter(ToolSafetyFilter(...))。
…ignment and sample issues
AI Code Review我已经准备好了。让我来写最终的审查报告。 发现的问题🚨 Critical（无）
|
…zation of README.md
AI Code Review已确认。现在我来写最终的审查报告。 发现的问题🚨 Critical
|
| has_network_tool = True | ||
| break | ||
|
|
||
| # Check http/https URLs against domain whitelist |
There was a problem hiding this comment.
无 scheme 的 curl/wget 裸域名绕过高危阻断
_check_network_egress 仅对 http(s):// URL 和 nc/netcat/socat 裸主机名做白名单校验；curl/wget 后跟裸域名（如 curl evil.com/exfil）既不命中 _URL_RE 也不命中 _HOSTNAME_RE，只产出 MEDIUM 的 R002，最终决策为 NEEDS_HUMAN_REVIEW 而非 DENY，可绕过高危阻断。建议将 curl/wget 首个非选项参数纳入域名解析与白名单检查，或至少提升到 HIGH。
AI Code Review我已经获取了所需信息。让我来整理最终审查结论。 发现的问题
|
AI Code Review我已经掌握了所需的所有信息。让我来编写最终的审查报告。 发现的问题🚨 Critical未发现必须修复的阻塞性问题。扫描器整体采用 fail-closed 策略，无命令注入/凭证泄露/路径穿越风险，opt-in 默认关闭不破坏现有行为。
|
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 行代码扫描：
✅ 策略配置修改可实时生效