Skip to content

feat(examples): add tool security scanning and filter mechanism (#90) - #288

Open
xyaohubery wants to merge 1 commit into
trpc-group:mainfrom
xyaohubery:split/issue-90-tool-security
Open

feat(examples): add tool security scanning and filter mechanism (#90)#288
xyaohubery wants to merge 1 commit into
trpc-group:mainfrom
xyaohubery:split/issue-90-tool-security

Conversation

@xyaohubery

Copy link
Copy Markdown

Summary

Add a tool security scanning example with configurable policies.

  • Pattern-based security scanning (shell injection, path traversal, network exfiltration, etc.)
  • Filter/block policy for dangerous tools
  • Integrity checks (size limits, JSON depth)

Fixes #90

This is a split from the original PR #166, addressing only issue #90 per reviewer feedback.

Co-Authored-By: Claude noreply@anthropic.com

…-group#90)

Add a pattern-based tool security scanner with configurable policies
for detecting dangerous operations before execution.

- Pattern-based security scanning (shell injection, path traversal,
  network exfiltration, etc.)
- Filter/block policy for dangerous tools
- Integrity checks (size limits, JSON depth)

Fixes trpc-group#90

Co-Authored-By: Claude <noreply@anthropic.com>
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

示例代码不受 CI lint 检查,因此不会阻塞 CI 流程。但根据 PR 自身的 README,该示例理应可运行——它目前完全无法执行。这是影响“核心功能”的关键问题(根据 PR 目的,该示例应能正常运行)。

现在开始撰写审查意见。

发现的问题

🚨 Critical

  • examples/tool_security_scan/run_agent.py:28-31:示例完全无法运行,Runner 的构造与调用都与 SDK 实际 API 不符
    • Runner(agent=..., session_service=...) 缺少必填的 app_name 参数(trpc_agent_sdk/runners.py:186-199 全为 keyword-only 且 app_name 必填);且 Runner 上不存在 run(prompt) 方法,SDK 只有 run_async(*, user_id, session_id, new_message)trpc_agent_sdk/runners.py:361)。同时 async for event in runner.run(prompt) 里把 event.content 当字符串 print(..., end="") 处理,但 Event 的内容结构是 event.content.parts[].text(见 examples/webfetch_tool/run_agent.py:77-93 的正确用法)。该 PR 的 README 把 python run_agent.py 作为 Quick Start,但代码无法启动,属于核心功能失败。建议按现有示例改为 Runner(app_name=..., agent=..., session_service=...) + runner.run_async(user_id=..., session_id=..., new_message=Content(...)),并按 parts 解析事件。

⚠️ Warning

  • examples/tool_security_scan/agent/tools.py:19,64-67path_traversalfindall 但正则含捕获组,导致记录的是子串而非完整命中

    • re.findall 在模式含捕获组时只返回组内内容,因此 /etc/passwd 命中后 match 变成 "passwd",写入 violation 的 match 字段丢失上下文,且 allow_patternssearch 是针对 "passwd" 而非完整路径做白名单匹配,可能误放行或误拦截。建议改用 pattern.finditer(args_str) 并取 m.group(0),或把 (passwd|shadow|sudoers) 改成非捕获组 (?:...)。此外第 67 行 match_str = match if isinstance(match, str) else match 是恒等表达式,对多分组返回的 tuple 无任何处理,后续 match_str[:200] 在 tuple 上会语义错误,应一并修复。
  • examples/tool_security_scan/agent/tools.py:84-103filter_tool_call 的“允许”分支直接原样返回 tool_args,与 docstring 声称的 sanitize 行为不符

    • 函数注释和 README 都宣称 “sanitize inputs”,但放行路径 return {"allowed": True, ..., "sanitized_args": tool_args} 未做任何清洗;拦截路径也只是返回空 dict 而非清洗后的参数。这会让调用方误以为返回值已脱敏而直接使用,存在误导风险。建议要么实现真正的清洗逻辑,要么把字段语义/命名改为 args 并修正文档,避免“已 sanitize”的假象。
  • examples/tool_security_scan/agent/config.py:3-6:API key 未设置时静默回退到字符串 "EMPTY" 而非报错

    • os.environ.get("TRPC_AGENT_API_KEY", "EMPTY") 在缺少凭据时不会失败,而是用一个无效 key 构造客户端,问题被推迟到运行期才以模糊的鉴权错误暴露,增加排障成本。建议在 key 缺失时直接抛出明确异常(或与其它示例一致使用 load_dotenv() 并校验)。
  • examples/tool_security_scan/agent/tools.py:25-29,76-78:完整性检查中 json_depthrepeated_input 定义了但从未被调用

    • scan_tool_input 只调用了 INTEGRITY_PATTERNS["large_size"]json_depth/repeated_input 是死代码,README 宣称的 “JSON depth limits” 实际未生效,给使用者错误的安全感。建议要么接入这两项检查(注意 repeated_input 需要 seen 集合,当前调用点无法提供),要么从文档与代码中移除,避免夸大能力。
  • examples/tool_security_scan/agent/tools.py:112-117_json_depth 对 list 不递归加深,深度判定语义错误

    • list 分支 _json_depth(v, depth) 未 +1,嵌套 list 的深度被低估;即便后续接入也会误判。建议 list 分支同样 depth + 1

💡 Suggestion

  • examples/tool_security_scan/agent/tools.py:17-22SECURITY_PATTERNSshell_injection 字符类 [;&|$(){}]会匹配几乎任何含括号/分号的正常 JSON(如函数调用参数里出现的()),在 json.dumps后的串上误报率高。建议先对tool_args` 各字段值分别评估,而非对整段 JSON 字符串做粗暴正则,以降低误报。

  • examples/tool_security_scan/**:新增文件缺少其它示例统一携带的 Tencent/Apache 许可头(对比 examples/webfetch_tool/run_agent.py:1-5)。建议补齐以保持仓库一致性与合规要求。

总结

最严重的问题是 run_agent.py 与 SDK 真实 API 完全不匹配、按 README 直接运行会失败,属于必须修复的 Critical;其余多为安全扫描工具自身的逻辑缺陷(捕获组取值错误、声称的 sanitize 未实现、完整性检查死代码),会误导复用者但不阻塞 CI。建议至少修复 Critical 与前两条 Warning 后再合入。

测试建议

  • 补充 scan_tool_input / filter_tool_call 的单测:覆盖 /etc/passwd..%2F..rm -rf、嵌套 JSON 超深/超大、正常含括号参数等路径,断言 violation 的 match 为完整命中串而非子串,并验证 filter_tool_call 放行分支的返回语义。
  • 增加 run_agent.py 的最小冒烟测试(mock LLM/Runner),确保 Runner 构造与 run_async 调用签名正确、事件解析不抛错。


runner = Runner(agent=agent, session_service=session_service)
async for event in runner.run(prompt):
if event.content:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

示例完全无法运行,Runner 构造与调用与 SDK 实际 API 不符

Runner 缺少必填的 app_name 参数,且不存在 run(prompt) 方法;SDK 实际为 run_async(user_id, session_id, new_message)。同时 event.content 被当字符串处理,实际结构为 content.parts[].text。建议改为 Runner(app_name=..., agent=..., session_service=...) + run_async(Content(...)),并按 parts 解析事件。

@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@12388ad). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             main        #288   +/-   ##
==========================================
  Coverage        ?   88.44269%           
==========================================
  Files           ?         491           
  Lines           ?       46118           
  Branches        ?           0           
==========================================
  Hits            ?       40788           
  Misses          ?        5330           
  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.

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.

构建 Tool 执行脚本安全扫描、Filter 拦截与监控机制

2 participants