Skip to content

feat: add tool script safety guard#234

Open
mingri31164 wants to merge 1 commit into
trpc-group:mainfrom
mingri31164:feat/tool-script-safety-guard
Open

feat: add tool script safety guard#234
mingri31164 wants to merge 1 commit into
trpc-group:mainfrom
mingri31164:feat/tool-script-safety-guard

Conversation

@mingri31164

@mingri31164 mingri31164 commented Jul 25, 2026

Copy link
Copy Markdown

Closes #90

Summary

  • add a policy-driven Tool Script Safety Guard for Python AST and Bash lexical scanning
  • return explicit allow, deny, or needs_human_review decisions with structured findings and redacted evidence
  • integrate pre-execution blocking through ToolScriptSafetyFilter and SafetyGuardedCodeExecutor
  • emit compact JSONL audit events and OpenTelemetry tool.safety.* span attributes
  • include a strict YAML policy, 12 runnable samples, generated report/audit artifacts, and English/Chinese docs

Security coverage

The default rules cover protected paths and destructive deletion, non-allowlisted network egress, subprocess/shell injection/pipelines/privilege escalation, runtime dependency installation, unbounded loops/fork bombs/large writes/long sleeps/concurrency, and secret output or exfiltration.

Configuration changes to allowed_domains, allowed_commands, denied_paths, timeout/output/resource limits, rule decisions, and disabled rules require no code change. Human-review decisions fail closed unless an application explicitly continues after reviewing the exact script hash.

This guard is intentionally a pre-execution policy layer, not a sandbox replacement. Runtime deployments must still enforce filesystem/network isolation, least privilege, syscall/PID/CPU/memory/disk limits, timeouts, and output quotas.

Validation

  • public samples: 12/12 scanned, yielding 2 allow / 7 deny / 3 human review
  • high-risk sample detection: 7/7 (100%); safe-sample false positives: 0/2 (0%)
  • dangerous deletion, credential reads, and non-allowlisted egress: 100% detected
  • 500-line Python benchmark (100 runs): mean 5.763 ms, p95 6.738 ms, max 8.552 ms
  • safety suite: 47/47 passed, 93% line coverage
  • adjacent Tool/Filter/CodeExecutor regression suite: 294/294 passed
  • broad repository run: 9,142 passed, 1 existing xfail, 1 pre-existing Python 3.11 Enum compatibility failure
  • source-tree CLI execution and read-only default artifact behavior are covered by regression tests
  • wheel and sdist build succeeded; wheel contains all trpc_agent_sdk/tools/safety modules
  • YAPF 0.40.2 diff, flake8, compileall, and git diff --check passed

Full-suite environment note: optional Claude tests and CLI auto-discovery were excluded because claude-agent-sdk<0.1.64 uses typing.TypedDict on Python 3.11 while current Pydantic requires typing_extensions.TypedDict. The remaining pre-existing failure is tests/common/test_compatible.py::TestCheckenum::test_string_value_is_found_by_value, which expects Python 3.12 Enum behavior on Python 3.11.

Known limitations

Static analysis remains vulnerable to obfuscation, generated code, runtime downloads, aliases/reflection, complex shell expansion, native binaries, symlink/TOCTOU races, and DNS rebinding. The docs describe these false-positive, false-negative, and bypass risks and how to extend rules safely.

@github-actions

github-actions Bot commented Jul 25, 2026

Copy link
Copy Markdown

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

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

已确认:提交的 tool_safety_audit.jsonl 包含了固定时间戳的生成数据。现在我来写最终审查报告。

发现的问题

🚨 Critical

未发现 Critical 级别问题。

⚠️ Warning

  • examples/tool_safety_guard/tool_safety_audit.jsonl:1-12examples/tool_safety_guard/tool_safety_report.json:1-377:提交了带固定时间戳与 duration_ms 的运行产物
    • 这两个文件是 tool_safety_check.py 运行时生成的产物(含 timestampduration_msscript_sha256),被一并提交进仓库。tool_safety_check.py 默认会把报告/审计写回 EXAMPLE_DIR 同名路径,任何人不带 --report/--audit 运行示例就会改写已提交文件,造成与 tests/tools/safety/test_cli.py 固定计数断言之间的漂移和误 diffs。建议把生成产物加入 .gitignore,或在示例中默认写入临时目录、仅在 README 中给出运行命令。

💡 Suggestion

  • trpc_agent_sdk/tools/safety/_executor.py:2400-2412_limit_output 直接修改传入的 result.output
    • 该函数对委托执行器返回的 CodeExecutionResult 对象做就地字段修改后返回,虽然当前调用链没有其他引用、行为正确,但属于共享可变对象的隐式副作用,后续若有调用方复用同一 result 会难以排查。可选改为构造新对象返回,或在函数签名/注释中明确“会就地修改”。

总结

整体实现扎实:扫描器、Filter、CodeExecutor wrapper、审计与遥测的决策流转与 fail-closed 语义均与测试一致,未发现安全漏洞或核心逻辑错误。唯一的 Warning 是将含时间戳的生成产物提交进仓库带来的维护性漂移风险;无必须修复的阻塞问题。

测试建议

暂无额外测试建议。现有测试已覆盖 12 个公开样本决策、规则维度、脱敏、上下文扫描(args/cwd/timeout)、输出截断(含极小阈值)、OpenTelemetry 属性与 CLI 端到端断言,风险路径覆盖充分。

@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.25700% with 53 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@6a2f7f9). Learn more about missing BASE report.

Files with missing lines Patch % Lines
trpc_agent_sdk/tools/safety/_python_scanner.py 88.75740% 19 Missing ⚠️
trpc_agent_sdk/tools/safety/_bash_scanner.py 89.81481% 11 Missing ⚠️
trpc_agent_sdk/tools/safety/_rule_utils.py 89.47368% 10 Missing ⚠️
trpc_agent_sdk/tools/safety/_filter.py 93.84615% 4 Missing ⚠️
trpc_agent_sdk/tools/safety/_executor.py 96.00000% 3 Missing ⚠️
trpc_agent_sdk/tools/safety/_policy.py 95.00000% 3 Missing ⚠️
trpc_agent_sdk/tools/safety/_audit.py 94.44444% 2 Missing ⚠️
trpc_agent_sdk/tools/safety/_scanner.py 98.71795% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main        #234   +/-   ##
==========================================
  Coverage        ?   87.94158%           
==========================================
  Files           ?         493           
  Lines           ?       45943           
  Branches        ?           0           
==========================================
  Hits            ?       40403           
  Misses          ?        5540           
  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.

@mingri31164

mingri31164 commented Jul 25, 2026

Copy link
Copy Markdown
Author

Follow-up incorporated in the latest clean commit:

  • bootstrap the repository root for direct source-tree CLI execution, fixing the CI subprocess import failure
  • make report/audit output opt-in so a default scan no longer rewrites committed snapshots; add a regression test for this behavior
  • align changed SDK files with the runner's YAPF 0.40.2 formatting

Validation after the fix: required CI build/lint/test pass; focused safety tests are 47/47 with 93% package coverage.

@mingri31164
mingri31164 force-pushed the feat/tool-script-safety-guard branch from 2ad7fad to 73afdad Compare July 25, 2026 17:54
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