Skip to content

fix(safety): scan all script-like filter arguments - #275

Closed
FireCollector wants to merge 8 commits into
trpc-group:mainfrom
FireCollector:codex/fix-safety-scan-all-script-fields
Closed

fix(safety): scan all script-like filter arguments#275
FireCollector wants to merge 8 commits into
trpc-group:mainfrom
FireCollector:codex/fix-safety-scan-all-script-fields

Conversation

@FireCollector

Copy link
Copy Markdown

Summary

  • scan every non-empty script-like Tool argument instead of stopping at the first match
  • keep code_blocks in the same aggregated static-scan input
  • add a regression test covering a safe earlier field followed by a dangerous command field

Problem

ToolSafetyFilter._extract_script() returned immediately after finding the first non-empty value among script, code, command, cmd, python_code, and bash_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 script value and a dangerous command value 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 passed
  • python -m flake8 --jobs=1 trpc_agent_sdk/tools/safety/_filter.py tests/tools/safety/test_wrapper.py
  • python -m yapf --diff trpc_agent_sdk/tools/safety/_filter.py tests/tools/safety/test_wrapper.py

Follow-up to #113 and the Tool Script Safety Guard introduced for #90.

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

发现的问题

⚠️ Warning

  • trpc_agent_sdk/tools/safety/_filter.py:101-117:合并多字段后单语言扫描可能漏检另一语言的危险内容
    • 本次改动将 _SCRIPT_ARG_KEYS 中所有字段拼接后统一交给 scanner,但 _extract_language 仍只根据单一字段推断出一个语言(如请求同时含 code(python) 和 command(bash) 且未显式指定 language 时,会判为 python),scanner 只按该语言规则集扫描整段拼接脚本,导致 bash 字段中的 rm -rf / 等不会命中 BASH_RECURSIVE_DELETE,恰好削弱了本 PR 想要补齐的覆盖目标。建议在拼接多字段时,对各字段按其来源语言分别扫描,或在无显式 language 时走 scanner 的“双语言”分支(unknown)。

💡 Suggestion

  • tests/tools/safety/test_wrapper.py:248-266:测试仅覆盖显式指定 language: bash 的混合字段场景
    • 该用例能验证“多字段被扫描”,但未覆盖上述无显式 language、跨语言字段混合的风险路径。建议补一个不指定 language、含 python+bash 混合危险字段的用例,以锁定预期检测行为。

总结

本次改动修复了安全过滤器只扫描首个脚本字段、漏检其余字段的问题,方向正确且测试基本覆盖核心路径。主要遗留风险是拼接多字段后仍按单一推断语言扫描,跨语言混合请求下可能漏检,建议按字段来源语言分别扫描。

测试建议

  • 补充无显式 language、同时含 code(python) 与 command/bash_code(bash 危险命令) 的请求用例,验证危险内容是否被检测。
  • 建议覆盖仅含空字符串/空白字段与 code_blocks 混合时的拼接行为,避免空字段干扰。

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.25000% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@1153e7f). Learn more about missing BASE report.

Files with missing lines Patch % Lines
trpc_agent_sdk/tools/safety/_filter.py 96.00000% 2 Missing ⚠️
trpc_agent_sdk/tools/safety/_scanner.py 96.66667% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@FireCollector

Copy link
Copy Markdown
Author

recheck

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

基于 diff 与仓库上下文(_filter.py_scanner.py_rules.py 及测试)的静态审查,结论如下。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/tools/safety/_filter.py:131-135(及 _filter.py:101-117):混合语言字段被路由到 language="unknown",会触发对拼接脚本的 ast.parse 误报
    • _extract_language 在同时存在 python 与 bash 源时返回 "unknown",而 _scanner.py:63-65unknown 会同时运行 scan_python_script,后者对包含 bash 内容的拼接脚本(如 print('ok')\nrm -rf /)执行 ast.parse 必然抛 SyntaxError,从而恒定产生一条 PY_PARSE_ERROR_REVIEW(NEEDS_HUMAN_REVIEW)误报。对一个本身无 DENY 的良性混合请求(如 code="print('hi')" + command="ls"),聚合判定会变成 NEEDS_HUMAN_REVIEW;当 block_on_review=True 时会直接阻断此前放行的工具调用。建议在 unknown 路径下按语言分段扫描(例如对 python 字段与 bash 字段分别扫描后再合并 findings),避免跨语言拼接导致的解析失败与误报。
    • 同根因:_extract_script 改为把所有 _SCRIPT_ARG_KEYScode_blocks 拼接后一次性扫描,对原本传入重复/同义字段(如 codepython_code,或 codecode_blocks)的工具行为发生破坏性变化,可能把本应独立扫描的内容拼在一起产生意外 findings。建议对同语言字段去重或仅扫描缺失项。

💡 Suggestion

  • trpc_agent_sdk/tools/safety/_filter.py:132-133has_python_source / has_bash_source 的字段集合与 _SCRIPT_ARG_KEYS 重复定义且不同步(前者不含 script,后者含)。若未来调整脚本键,两处容易漂移。可考虑复用同一份键集合或在注释中明确两者职责差异,便于长期维护。

总结

该 PR 修复了“仅扫描首个脚本字段、漏扫混合语言中 bash 危险命令”的真实缺陷,方向正确且测试覆盖了关键路径。主要遗留风险是混合语言走 unknown 后跨语言拼接扫描会恒定产生 PY_PARSE_ERROR_REVIEW 误报,在 block_on_review=True 下可能误阻断良性请求——建议按语言分段扫描。不存在必须修复的 Critical 问题。

测试建议

  • 补充一条良性混合语言用例(如 code="print('hi')" + command="ls",无 DENY),断言默认 block_on_review=Falseis_continue is True 且不产生 PY_PARSE_ERROR_REVIEW,以锁定未来分段扫描的修复不回归。
  • 补充同语言重复字段(如同时传 codepython_code)用例,验证拼接不会引入意外 findings。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

现在让我来撰写评审。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/tools/safety/_filter.py:155-188_merge_reports 重用首个子报告作为合并结果的容器,导致部分字段的合并语义不正确

    • 合并报告直接复用 reports[0] 对象,其 scan_idtimestamptool_nameblocked 仍来自首个子扫描;当 language == "mixed"tool_name 虽一致但 scan_id/timestamp 并不代表“合并结果”。更关键的是合并后的 telemetry_attributes 只更新了 decision/risk/rule_id/sanitized/duration,丢失了 tool.safety.scan_idtool.safety.blockedtool.safety.tool_name(这些键来自首个子报告,scan_id 仍是子扫描 id,blocked 是子扫描的原始值而非 set_blocked 后的值,且 record_safety_attributesset_blocked 之前调用,会推送错误的 scan_id/blocked 到 telemetry span)。建议合并时为新报告生成统一的 scan_id/timestamp,并基于合并 findings 重建完整 telemetry_attributes,避免审计/监控关联错误。
  • trpc_agent_sdk/tools/safety/_filter.py:126-142:上下文(command_args/cwd/env/tool_metadata)只附加到第一个语言分组,其余分组静默丢弃

    • 通过 include_context = index == 0 仅给首个分组挂载执行上下文,依赖 grouped_parts 插入顺序(python → bash → generic → code → code_blocks)。若请求同时包含 bash 脚本与 timeout/输出大小等 tool_metadata 限制,当首个分组为 python 时,_scan_execution_context 的 RESOURCE_*_LIMIT 规则只会针对 python 分组求值;同时首个分组无 cwd 时不产生 EXECUTION_DENIED_CWD,但其他分组的 bash 脚本仍可能执行于被禁目录。建议至少把 cwd/tool_metadata 等与脚本无关的执行上下文对所有分组生效,或在文档中明确该限制并记录被丢弃项。
  • trpc_agent_sdk/tools/safety/_filter.py:110-111code 字段在无显式 language 时被硬编码为 python,导致混合脚本被误判语言

    • code_language = _extract_explicit_language(req) or "python",当请求含 code 但无 language、tool_name 也无 python 提示时,code 强制按 python 扫描;若 code 实为 bash 片段(如 rm -rf /),由于 scanner 对 python 走 scan_python_script+scan_bash_script 双扫仍可能命中,但 code 字段语义并不必然为 python,硬编码会让 language 上报与实际不符。建议对 code 复用 generic_language(unknown 走双扫),而非默认 python。
  • trpc_agent_sdk/tools/safety/_filter.py:113-124:code_blocks 中非 dict 对象取 language 缺失属性时会抛 AttributeError

    • 当 block 是既非 dict、又没有 language 属性的对象(如裸字符串)时,getattr(block, "language", "") 虽有默认值但 getattr(block, "code", "") 同样兜底,然而若 block 为字符串,getattr 会返回 "" 进而跳过;但对无 code 属性、有 language 的对象仍会进入 _canonical_language。问题在于外层 if isinstance(block, dict) / else 分支对任意对象直接 getattr,与原实现一致但放大了分组遍历面,建议显式校验 code 非空后再归类语言,避免空 code 也写入分组。

💡 Suggestion

  • trpc_agent_sdk/tools/safety/_filter.py:146-152_add_script_partvalue not in parts 做 O(n) 去重并依赖列表线性查找,对大 code_blocks 场景可读性与性能均不佳;可改用按语言维护 dict[str, set] 做去重键,保留插入顺序输出,逻辑更清晰。

总结

整体改动方向正确(按语言分组扫描并聚合决策),核心安全决策通过 aggregate_decision/max_risk_level 合并且 findings 经 _dedupe_findings 去重,无明显安全旁路。但存在若干正确性缺陷:合并报告复用首个子报告导致 scan_id/telemetry 不准、执行上下文仅挂载首个分组可能漏检、code 字段语言硬编码为 python。无 Critical 阻塞问题,建议修复上述 Warning 后合入。

测试建议

  • 补充一个多分组请求同时带 bash 脚本与 tool_metadata={"timeout": 超限值} 的用例,验证 RESOURCE_TIMEOUT_LIMIT_EXCEEDED 不因分组顺序而丢失。
  • 补充一个 code 为 bash 危险片段且无 language 字段的用例,断言其能被 deny 且上报 language 合理。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

现在我已经掌握了足够的上下文。在最终确认前,让我先核实一个关于合并后的报告 scan_id/timestamp 的具体细节。

扫描器在每次扫描时会生成一个新的 scan_idtimestamp (_scanner.py:75-76)。在 _merge_reports 中,report = reports[0] 复用了第一个子扫描的 scan_id/timestamp,且从不为聚合结果重新生成它们。审计事件 (_audit.py:19-30) 和遥测数据携带了该 ID。已确认。

让我再确认一下 codescript 的语言拆分是否如我所想。

  • script_GENERIC_ARG_KEYS,分组依据为 generic_language = _extract_language(req, tool_name),在没有语言提示时可能为 "unknown" → 扫描器的 else 分支会同时作为 bash 和 python 进行双重扫描 (_scanner.py:63-65)。
  • codecode_language = _extract_explicit_language(req) or "python",在没有显式语言时强制为 "python" → 仅进行 Python 扫描 (_scanner.py:59-60)。

已确认存在差异。现在开始撰写审查报告。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/tools/safety/_filter.py:151-176:合并多语言扫描报告时复用首个子报告的 scan_id/timestamp
    • _merge_reportsreports[0] 为基底并直接返回,未为聚合后的整体决策重新生成 scan_id/timestamp,导致 write_audit_event 写出的审计事件和 telemetry_attributes["tool.safety.scan_id"] 只反映第一个子扫描,而 decision/risk_level/findings 却是全部字段聚合的结果,审计与遥测可追溯性不一致。建议为合并报告生成新的 scan_id/timestamp,或显式标注其来源为聚合报告。
    ...
    report = reports[0]
    if len(reports) == 1:
        return report
    report.findings = [finding for item in reports for finding in item.findings]
    ...

💡 Suggestion

  • trpc_agent_sdk/tools/safety/_filter.py:104-108scriptcode 字段的语言回退策略不一致,威胁覆盖存在差异
    • scriptgeneric_language,无语言提示时为 "unknown",扫描器会同时做 bash+python 双扫描(_scanner.py:63-65)能命中 BASH_RECURSIVE_DELETE;而 code 强制 code_language = _extract_explicit_language(req) or "python",无显式语言时仅做 Python 扫描,非 Python 内容(如 rm -rf /)只会触发 PY_PARSE_ERROR_REVIEW(NEEDS_HUMAN_REVIEW)而漏掉 DENY 级别规则。该差异由本次拆分引入,建议将 code 也按 generic_language 分组,或在无显式语言时同样走双扫描。

总结

本次变更是对安全过滤器的正确增强:将原先只扫描首个匹配字段改为按语言分组扫描全部脚本类字段并合并报告,修复了多字段场景下漏扫的风险,整体无阻塞问题。主要遗留点是合并报告复用首子扫描的 scan_id/timestamp 造成审计/遥测归属不准确,以及 codescript 的语言回退策略不一致导致非 Python 内容威胁覆盖有差异。

测试建议

  • 建议补充用例:多字段聚合场景下断言 scan_id/timestamp 唯一且与聚合决策一致,覆盖审计可追溯性。
  • 建议补充用例:{"code": "rm -rf /"}(无 language)的预期决策,明确该路径是否应命中 BASH_RECURSIVE_DELETE,以固化 code 字段的语言回退行为。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

我已经掌握了足够的信息。让我完成这次审查。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/tools/safety/_scanner.py:78-80:unknown 语言段会同时跑 bash 和 python 扫描,非合法 Python 的纯 bash 片段会被 ast.parse 判为 SyntaxError 而产生 PY_PARSE_ERROR_REVIEWNEEDS_HUMAN_REVIEW)。

    • 当某字段落入 unknown 语言(无显式 language/lang 且工具名无 python/bash/shell 提示)时,回退分支会对任意 shell 命令额外触发一次 Python 解析审查,给原本正常的 bash 调用注入人工审查结论,在 block_on_review=True 下可能直接阻断安全命令。建议对 unknown 段只保留 bash 扫描,或仅当片段能被 ast.parse 解析时才跑 python 规则。
  • trpc_agent_sdk/tools/safety/_scanner.py:67-84:多段扫描缺少跨段去重,bash 段与 unknown 段的 bash 回退会产生重复 findings。

    • _dedupe_findings 只在单次 scan_* 调用内去重;当一个请求同时含 command(bash 段)和 script(unknown→bash 回退)且内容相似时,同一条规则(如 BASH_RECURSIVE_DELETE)会被报告两次。不影响决策(aggregate_decision 幂等),但会污染 findings 列表与遥测 rule_id 统计。建议在 scan_segments 末尾对全部 findings 做一次去重。

💡 Suggestion

  • trpc_agent_sdk/tools/safety/_scanner.py:94-95language 在多段时固定为 "mixed",丢失了实际语言集合。如需保留更可观测的信息,可在 SafetyReport.language 或 telemetry 中记录归一化后的语言列表,便于后续排查。

总结

整体改动方向正确:按语言拆段扫描、execution_context 仅扫一次的设计合理,测试覆盖了 mixed/unknown/strict 等关键路径。存在两类需关注的问题——unknown 段的 Python 解析回退可能误报并阻断安全命令(Warning,建议修复),以及多段扫描缺跨段去重导致 findings/遥测重复(Warning)。

测试建议

  • 补充一个 unknown 语言段为纯 bash 命令(如 git status)且 block_on_review=True 的用例,断言不产生 PY_PARSE_ERROR_REVIEWdecision == "allow",以锁定回退误报问题。
  • 补充一个同时含 commandscript 且命中同一条 bash 规则的用例,断言该 rule_id 在 findings 中仅出现一次,覆盖跨段去重。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

我已经审阅了 PR diff 及相关的扫描器/规则上下文。以下是我的审查结论。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/tools/safety/_filter.py:105code 字段在无显式 language 且工具名未提示语言时,由原来的“仅按 Python 扫描”改为“按 unknown 同时扫描 bash+python”,会导致原本允许的合法 Python 代码被误判为 deny。

    • 旧逻辑 _extract_language"code" in req → pythoncode 只走 scan_python_script;新逻辑 code_language = _extract_explicit_language(req) or generic_language,无语言时落到 unknown,触发 scan_bash_script。Python 字符串/字面量里包含 rm -rfcurl ... 等模式时(如 x = "rm -rf /"),_scan_bash_line 的正则会命中 BASH_RECURSIVE_DELETE 并直接 DENY,而旧逻辑不会。这是兼容性破坏 + 误报风险。建议:对无法推断语言的 code 默认仍按 Python 处理,或仅当 bash 扫描无 Python 语法冲突时才合并告警,以保留对 code 字段的历史契约。
    • code_language = _extract_explicit_language(req) or generic_language
      _add_script_part(grouped_parts, code_language, req.get("code"))
  • trpc_agent_sdk/tools/safety/_scanner.py:84:自定义规则(_scan_custom_rules)现在按每个 segment 分别执行,但其结果去重依赖 _dedupe_findings(rule_id, line, evidence) 键;若自定义规则对不同 segment 返回 evidence 相同但语义不同的告警,会被错误合并,缺少针对多 segment 自定义规则的测试覆盖。

    • 现有自定义规则测试(test_scanner.py:414421)只走单 segment 的 scan_script,未覆盖 scan_segments 多段场景。建议补充:多段输入下自定义规则的执行次数与去重行为测试,确认跨 segment 的自定义告警不会被误去重。

💡 Suggestion

  • trpc_agent_sdk/tools/safety/_filter.py:159_extract_explicit_language_extract_language 内部已调用一次,code_language 处又独立调用一次,对同一 req 重复遍历 _LANGUAGE_ARG_KEYS。可提取为局部变量复用,减少重复扫描,便于后续维护。

总结

整体重构方向正确,将单字段提取改为按语言分组多段扫描并去重,安全覆盖面明显增强,且核心路径有测试覆盖。主要风险是 code 字段在无语言标注时从“仅 Python”变为“bash+python”双扫,可能对合法 Python 代码产生误判 deny,属于兼容性/误报问题,建议确认是否为预期取舍。

测试建议

  • 补充用例:code 字段为合法 Python 但包含 bash 模式字面量(如 x = "rm -rf /")且无 language/python 工具名时,验证是否被错误 deny。
  • 补充 scan_segments 直接调用(非经 filter)的多段场景测试,覆盖自定义规则跨段执行与去重、language="mixed" 的判定。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

基于对 diff 及相关上下文(_rules.py_scanner.py_types.py_audit.py)的核查,结论如下。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/tools/safety/_filter.py:102-104trpc_agent_sdk/tools/safety/_scanner.py:79-81script/无语言 code_blocks 被归为 unknown 语言段,而 unknown 分支会同时执行 scan_python_script,对任何非 Python 内容都会产生 PY_PARSE_ERROR_REVIEW(NEEDS_HUMAN_REVIEW)误报。
    • 本 PR 将扫描范围从“首个匹配字段”扩展为“所有 script-like 字段”,使得原先被忽略的 bash-only script 字段也会进入 unknown→Python 解析路径。例如 {command:"ls", script:"git status", tool_name:"custom"}block_on_review=True 下会因 PY_PARSE_ERROR_REVIEW 被误阻断。
    • 该误报已被新增的 test_filter_deduplicates_findings_across_segmentstests/tools/safety/test_wrapper.py:339-356)静默包含:script:"rm -rf /" 段会产出一条 PY_PARSE_ERROR_REVIEW,但测试未断言其存在与否,说明风险未被覆盖。
    • 修复方向:对 unknown 段不触发 Python 解析错误类规则(或在 bash 扫描已命中时抑制解析错误),并补充一条断言 PY_PARSE_ERROR_REVIEW 不存在的测试。

💡 Suggestion

总结

整体逻辑稳健,分段扫描、执行上下文单次扫描、跨段去重的核心实现正确,未发现安全或数据正确性层面的 Critical 问题。主要风险是 unknown 语言段的 Python 解析误报在本 PR 扩大扫描面后会引入新的误阻断路径,建议修复并补测。

测试建议

  • 补充:script/code_blocks 含 bash 内容且无 language 字段时,断言不产生 PY_PARSE_ERROR_REVIEW,并覆盖 block_on_review=True 下安全 bash 脚本不被阻断的场景。

@FireCollector

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

Rook1ex added a commit to trpc-group/cla-database that referenced this pull request Jul 31, 2026
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
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

我现在已经有了完整的认识。让我再验证一个潜在的真正 bug:在 test_filter_scans_mixed_language_fields_without_language 测试中,code: print('ok') 且没有显式语言,tool_name 为 "mixed_tool"。code_language = _extract_explicit_language(req) or "python" → "python"。generic_language = _extract_language → 无显式语言,tool_name 不含 python/bash → "unknown"。所以 code→python 组,command→bash 组。两个组 → 语言为 "mixed"。code 以 python 解析 → 无 PY_PARSE_ERROR → python 结果保留。command rm -rf / → bash DENY。很好,符合预期。

现在让我看看当请求有 unknown-language script 但解析失败,且同时包含 bash 字段时,是否存在真正的 bug 抑制问题 —— PY_PARSE_ERROR_REVIEW 被抑制,所以严格模式不会因解析错误而阻塞。这是预期且已测试的行为。

让我评估一下是否真的存在 Concrete bug。让我重新考虑去重键(dedupe key)与跨段落 python 行号之间的相互作用。实际上,存在一个真正的正确性顾虑:在未知段落中,当 python 解析成功时,python 的发现结果会保留,但它们的 line 号是相对于该段落脚本的。由于每个段落都是独立的请求/脚本,这是没问题的。没有 bug。

让我检查最后一个潜在问题 —— code 字段以前会被 _extract_language"code" in req 检查覆盖以返回 "python"。现在 code 总是通过 code_language。当同时存在 language: bashcode 字段时,考虑其行为。以前:code_SCRIPT_ARG_KEYS 中 → _extract_script 返回 code_extract_language 检测到显式 language=bash → 返回 "bash"。所以 code 以 bash 解析。现在:code_language = explicit = "bash"code 以 bash 解析。一致。很好。

现在让我验证一下是否存在真正的 bug:当没有显式语言且 tool_name 既不是 python 也不是 bash,且只有 code 字段(没有 script)存在时。code_language = "python"。所以 code 以 python 解析。之前:_extract_script 返回 code_extract_language:无显式语言,"code" in req → 返回 "python"。一致。

我现在确信代码逻辑是合理的。让我检查一下 _canonical_language 对于未知显式语言(如 "ruby")返回原值是否存在问题 —— 被归为 "ruby",扫描器 _normalize_language 返回 "ruby",落入未知分支 → bash+python 扫描。对于具有显式非 python/bash 语言的 code 字段,python 解析可能会失败并触发 PY_PARSE_ERROR_REVIEW。等等 —— 在这种情况下,该段落不是 "unknown";它的语言是 "ruby"。让我检查一下:在 scan_segments 中,未知分支是 else(不是 python,也不是 bash/sh/shell)。所以 "ruby" → else 分支 → bash 扫描 + python 扫描并带有解析错误抑制。所以,如果用户显式设置 language: rubycode 为实际的 ruby 代码,python 解析会失败,PY_PARSE_ERROR_REVIEW 被抑制,只保留 bash 扫描结果。这可以说是正确的行为。不是 bug。

然而,确实存在一个细微的问题:对于显式的 "ruby",报告的语言将是 "ruby"(单种语言),而不是 "mixed"。没问题。

我已经完成了全面的分析。该 diff 结构良好且测试覆盖充分。让我整理一下我的发现 —— 我认为没有关键或警告级别的阻塞问题。让我记录下微小的建议。

让我重新考虑一件事,作为潜在的警告:scan_segments 在空列表上引发 ValueError —— 但 _before 有防护,所以没问题。而 scan() 总是传递一个包含一个元素的列表。没有问题。

让我验证 test_filter_scans_execution_context_once_for_mixed_fields 测试的超时去重:python_code: print('ok') + command: echo ok,tool_metadata 超时 301。两个段落,_scan_execution_contextbase_request 上调用一次 → 一个超时发现。断言 len == 1。很好。这实际上测试了正确的行为。

基于我的彻底审查,该 diff 是干净且测试充分的。我会指出最多一个微小的建议。让我重新检查 _add_script_part 的去重:它会去重相同分组的精确字符串值,这可能会合并来自不同字段的意图相同的脚本 —— 但这是预期的去重。没有问题。

让我做最后的检查 —— _canonical_language 没有将 None 归一化问题:(language or "unknown") 处理了 None。很好。而且 _extract_language 现在调用 _extract_explicit_language,它返回 ""if explicit_language 为假 → 会落入 tool_name 检查。很好。

我认为没有阻塞问题。让我写下结论。我已对 pr.diff 完成审查,并交叉核对了 _scanner.py_rules.py_filter.py 中的被调用方与去重/聚合逻辑。

发现的问题

未发现明显阻塞问题。

本次变更将单一脚本扫描重构为按语言分组的分段扫描(scan_segments),核心逻辑是正确的:

  • _extract_scan_requests 按 python/bash/generic 三类键分组并归并同类脚本片段,_add_script_part 做了空值与非空校验及精确去重,行为与旧 _extract_script 兼容。
  • _scan_execution_context 仅对 base_request 调用一次是合理的——所有分段共享同一份 command_args/cwd/env/tool_metadata,多次调用反而会重复产出 context 类 finding,并被测试 test_filter_scans_execution_context_once_for_mixed_fields 验证(断言 RESOURCE_TIMEOUT_LIMIT_EXCEEDED 仅一条)。
  • unknown 分支保留 Python finding 仅在 AST 解析成功时的逻辑(PY_PARSE_ERROR_REVIEW 抑制)与最近一次提交目的一致,且有正向/反向测试覆盖。
  • mixed 语言判定基于去重后的 language 列表长度,scan() 通过 scan_segments([request]) 保持向后兼容,_beforerequests 为空时提前返回,ValueError 不会进入实际调用路径。
  • 最终 _dedupe_findings(rule_id, line, evidence) 去重,跨分段同证据 finding 合并符合预期,并由 test_filter_deduplicates_findings_across_segments 覆盖。

💡 Suggestion

  • trpc_agent_sdk/tools/safety/_filter.py:105code 字段的语言推断使用 _extract_explicit_language(req) or "python",而 script 字段使用 _extract_language(req, tool_name)(可回退到 tool_name 推断或 unknown)。两者回退路径不一致:当无显式 language 且 tool_name 含 python/bash 时,script 会跟随 tool_name 归类,而 code 始终回退到 python。当前不影响正确性(均有测试覆盖),但若同一请求同时带 scriptcode 且 tool_name 为 PythonRunner,二者会落入不同语言组并产生 mixed 语言报告,可能不符合调用方直觉。可考虑统一 code 也走 _extract_language 回退逻辑以保持归类一致。

总结

整体风险低,分段扫描重构逻辑正确、边界处理完备、执行上下文与去重均有针对性测试覆盖,不存在必须修复的问题。

测试建议

暂无额外测试建议。建议覆盖的场景(mixed 语言归类、未知语言 Python finding 抑制、跨分段去重、执行上下文单次扫描)已在新增测试中体现。

@FireCollector

Copy link
Copy Markdown
Author

感谢自动审查。本轮已在提交 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 也确认不存在必须修复的阻塞问题。烦请维护者在方便时进行人工评审,谢谢!

@raychen911

Copy link
Copy Markdown
Contributor

Closing as related to #90.

@raychen911 raychen911 closed this Aug 2, 2026
@FireCollector

Copy link
Copy Markdown
Author

感谢说明,理解 issue #90 已完成并关闭。\n\n该 PR 主要修复的是当前安全实现中多脚本字段漏扫、跨语言分段扫描以及 unknown 语言误报的问题,相关测试与 CI 均已通过。请问项目是否希望我将这些修改整理为不再关联 #90 的独立 bugfix PR?\n\n在得到确认前,我不会自行重新提交或重开 PR。谢谢!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants