fix(safety): scan all script-like filter arguments - #275
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
AI Code Review发现的问题
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #275 +/- ##
==========================================
Coverage ? 88.43967%
==========================================
Files ? 491
Lines ? 46080
Branches ? 0
==========================================
Hits ? 40753
Misses ? 5327
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
recheck |
AI Code Review基于 diff 与仓库上下文( 发现的问题
|
AI Code Review现在让我来撰写评审。 发现的问题
|
AI Code Review现在我已经掌握了足够的上下文。在最终确认前,让我先核实一个关于合并后的报告 扫描器在每次扫描时会生成一个新的 让我再确认一下
已确认存在差异。现在开始撰写审查报告。 发现的问题
|
AI Code Review我已经掌握了足够的信息。让我完成这次审查。 发现的问题
|
AI Code Review我已经审阅了 PR diff 及相关的扫描器/规则上下文。以下是我的审查结论。 发现的问题
|
AI Code Review基于对 diff 及相关上下文( 发现的问题
|
|
I have read the CLA Document and I hereby sign the CLA |
Unknown language segments may contain valid non-Python input. Continue Bash scanning, but merge Python findings only when AST parsing succeeds so strict review mode does not block safe Bash scripts. Updates trpc-group#90 RELEASE NOTES: NONE
AI Code Review我现在已经有了完整的认识。让我再验证一个潜在的真正 bug:在 现在让我看看当请求有 让我评估一下是否真的存在 Concrete bug。让我重新考虑去重键(dedupe key)与跨段落 python 行号之间的相互作用。实际上,存在一个真正的正确性顾虑:在未知段落中,当 python 解析成功时,python 的发现结果会保留,但它们的 让我检查最后一个潜在问题 —— 现在让我验证一下是否存在真正的 bug:当没有显式语言且 tool_name 既不是 python 也不是 bash,且只有 我现在确信代码逻辑是合理的。让我检查一下 然而,确实存在一个细微的问题:对于显式的 "ruby",报告的语言将是 "ruby"(单种语言),而不是 "mixed"。没问题。 我已经完成了全面的分析。该 diff 结构良好且测试覆盖充分。让我整理一下我的发现 —— 我认为没有关键或警告级别的阻塞问题。让我记录下微小的建议。 让我重新考虑一件事,作为潜在的警告: 让我验证 基于我的彻底审查,该 diff 是干净且测试充分的。我会指出最多一个微小的建议。让我重新检查 让我做最后的检查 —— 我认为没有阻塞问题。让我写下结论。我已对 发现的问题未发现明显阻塞问题。 本次变更将单一脚本扫描重构为按语言分组的分段扫描(
💡 Suggestion
总结整体风险低,分段扫描重构逻辑正确、边界处理完备、执行上下文与去重均有针对性测试覆盖,不存在必须修复的问题。 测试建议暂无额外测试建议。建议覆盖的场景(mixed 语言归类、未知语言 Python finding 抑制、跨分段去重、执行上下文单次扫描)已在新增测试中体现。 |
|
感谢自动审查。本轮已在提交 c460688 中修复 unknown 语言段对安全 Bash 内容产生 PY_PARSE_ERROR_REVIEW 误报的问题,并补充了以下回归覆盖:\n\n- unknown 语言的安全 Bash 片段在严格模式下仍可正常放行;\n- unknown 语言内容在 AST 解析成功时仍保留有效的 Python 风险发现;\n- 跨分段 findings 去重行为保持正确。\n\n相关 scanner、wrapper 测试及项目 CI 均已通过,最新 AI Code Review 也确认不存在必须修复的阻塞问题。烦请维护者在方便时进行人工评审,谢谢! |
|
Closing as related to #90. |
Summary
code_blocksin the same aggregated static-scan inputProblem
ToolSafetyFilter._extract_script()returned immediately after finding the first non-empty value amongscript,code,command,cmd,python_code, andbash_code. A Tool or MCP request containing more than one of these fields could therefore leave later executable content outside the pre-execution scan.For example, a request with a safe
scriptvalue and a dangerouscommandvalue scanned only the safe value. The regression test keeps both values as static strings; no command is executed.Fix
Collect all recognized non-empty script fields and code blocks, then join them into the scanner input. This preserves existing single-field behavior while ensuring later executable fields are also evaluated.
Validation
python -m pytest tests/tools/safety/test_wrapper.py -q— 22 passedpython -m flake8 --jobs=1 trpc_agent_sdk/tools/safety/_filter.py tests/tools/safety/test_wrapper.pypython -m yapf --diff trpc_agent_sdk/tools/safety/_filter.py tests/tools/safety/test_wrapper.pyFollow-up to #113 and the Tool Script Safety Guard introduced for #90.