feat: implement automated code review agent prototype with AST and filter governance by CyberX0725 · Pull Request #276 · trpc-group/trpc-agent-python

CyberX0725 · 2026-07-31T14:27:36Z

📝 Proposed Changes / 修改内容

This PR implements an automated code review (CR) agent prototype using the tRPC-Agent-Python framework. The agent reads diff files, runs checks inside a sandboxed workspace runtime, persists structured results to a relational database, and outputs JSON and Markdown reports.
本 PR 基于 tRPC-Agent-Python 框架实现了一个自动代码评审 (CR) Agent 原型。该 Agent 支持读取变更 diff、在隔离沙箱中执行规则检测、将结构化结果入库并最终导出中英双语的 JSON 与 Markdown 评审报告。

🚀 Key Highlights & Enhancements / 核心亮点与升级

High-Precision AST Parsing / 高精度语法树检查
Upgraded the check script (run_checks.py) to parse Python code into Abstract Syntax Trees (AST) using Python's ast module. It performs precise node-level checking and automatically falls back to line-based regex diff checking when syntax is incomplete or files are missing.
升级静态分析逻辑，使用 ast 语法树节点进行高精度匹配。在语法截断或本地源码缺失时，具备健壮的行级正则匹配回退机制。
Git Pre-commit Hook / Git 提交前拦截门禁
Added pre_commit_hook.sh which can be copied to .git/hooks/pre-commit. It runs the agent on staged changes before commits and aborts the commit if CRITICAL or HIGH severity violations are found.
提供一键集成的 pre_commit_hook.sh 脚本，可在本地 commit 前触发评审，自动阻断包含高危漏洞（如明文密钥或 Shell 注入）的代码提交。
Bilingual Documentation / 双语设计说明书
Provided bilingual guides (README.md and README.zh_CN.md) explaining the skill architecture, database schema, filter policy, and sandbox boundaries for ease of review.
配备了中英双语版设计说明文档，详细阐述了架构原理、安全边界与监控字段设计。
Rich Terminal Display with Compatibility Fallback / 终端炫酷输出与自适应
Displays visual tables and summaries using rich. It detects non-UTF8 terminals (like classic Windows cmd/powershell) and automatically falls back to ANSI/ASCII text tables to avoid rendering glitches.
集成了控制台炫酷排版表格，并对非 UTF-8 编码的终端环境提供自适应 ANSI 回退防止乱码。

🛠️ Requirements Fulfillment Check / 任务要求对照表

CR Skill: Implemented SKILL.md and rules.md covering 6 vulnerability categories (exceeding the min 4 requirement).
Sandbox Execution: Configured both container (default) and local (fallback) modes with timeout (15s) and stdout limits (5000 chars).
Input Parsing: Supports reading unified diff files and extracting hunks/lines via parse_diff.py.
Relational Schema: Implemented tables for tasks, runs, findings, reports, and filters inside SQLite/SQLAlchemy.
De-duplication: Filters duplicate occurrences by (file, line, category) and splits findings by confidence.
Sensitive Info Redaction: Plain secrets are masked to [REDACTED] prior to reporting and storage.
Filter Governance: Intercepts rm -rf, path traversal, and over-budget commands before sandbox execution.
Auditing & Monitoring: Audits execution durations, tool counts, interception counts, and severity/exception distributions.

🧪 Verification Plan / 验证与测试结果

All 8 integration tests passed successfully on pytest:
已通过 test_agent.py 中 8 个测试场景的全面验证：

$env:PYTHONPATH="d:\my_document\project\others\trpc-agent-python"; pytest examples/skills_code_review_agent/test_agent.py

Output:

........ [100%]
8 passed, 89 warnings in 4.28s

…nd test suite

helloopenworld · 2026-07-31T14:31:34Z

AI Code Review

发现的问题

🚨 Critical

examples/skills_code_review_agent/agent.py:156、agent.py:199：沙箱命令使用 subprocess.run(..., shell=True) 执行，且命令字符串直接拼接了 diff_file_path、parsed_diff_temp、raw_findings_temp 等外部可控路径，存在命令注入风险。虽然 FilterGovernance.check 只做黑名单子串匹配（见 agent.py:19，且 len("rm -rf /")>4 走纯子串匹配），但无法防住路径中的 shell 元字符（如 ;、$()），攻击者构造的 diff 文件名即可注入任意命令。应改为 shell=False 并以列表传参，对路径做校验。
examples/skills_code_review_agent/agent.py:180-181：当 diff 内容包含 rm -rf 字符串时，代码主动拼接 check_cmd += " && rm -rf /"。这是把“测试沙箱失败/拦截”的演示逻辑混进了真实执行路径：一旦 filter.check 漏判（黑名单子串匹配易被空格/引号变体绕过），rm -rf / 会在宿主机上通过 shell=True 真实执行，构成灾难性破坏。该构造应仅出现在测试 fixture，不应在生产代码路径中直接 append 危险命令。
examples/skills_code_review_agent/agent.py:163-167：FilterGovernance.check 的路径检查仅在显式传入 inputs 时生效，而两处 subprocess.run 调用前 self.filter.check(parse_cmd) / self.filter.check(check_cmd) 均未传入 inputs（agent.py:140、agent.py:183），敏感路径拦截规则实际从未触发，安全边界形同虚设。调用时应把命令和所有路径参数一并传入校验。

⚨ Warning

examples/skills_code_review_agent/agent.py:154-156 附近：parse_diff 失败抛 subprocess.SubprocessError 后被 except Exception 吞掉并继续执行 run_checks（agent.py:296-301），但 parsed_diff_temp 可能不存在/内容为空，后续 run_checks 读不到文件会落到 raw_findings=[]，任务仍标记 COMPLETED。沙箱失败时状态应收敛为 FAILED 或在报告中明确，否则掩盖真实失败。
examples/skills_code_review_agent/agent.py:147、db.py:589-590：使用 datetime.datetime.utcnow，Python 3.12+ 已弃用，且 SQLite 默认无时区，跨库切换到 Postgres/MySQL 时行为不一致；建议改用 datetime.now(timezone.utc)。属兼容性隐患。
examples/skills_code_review_agent/pre_commit_hook.sh:8：钩子将 review_report.json / review_report.md 写入当前工作目录（agent.py:233-235 写死相对路径），在任意 git 仓库根目录执行时会产生残留文件，且 grep 计数依赖序列化格式（"severity": "critical" 带空格），一旦 json.dump 风格变化即漏判，导致高危提交不被拦截。建议用 Python 解析 JSON 而非 grep。
examples/skills_code_review_agent/test_agent.py:95-121：test_duplicate_findings 实际未对 run_review 输出做去重断言，而是在测试体内手工构造 raw_findings 并重新实现一遍去重逻辑后断言，等于自测自；真正的 agent 去重路径（agent.py:152-160）未被覆盖。
examples/skills_code_review_agent/run_checks.py:83-85（run_checks 内 TRIGGER_SANDBOX_CRASH 分支）：以 sys.exit(1) 模拟沙箱崩溃，该脚本是生产 skill 脚本（被 agent.py 真实调用），不仅用于测试。任何 diff 中出现该字符串都会让检查进程直接退出，属可被输入触发的异常退出路径，建议改为受控异常而非 sys.exit。

💡 Suggestion

examples/skills_code_review_agent/review_report.json、review_report.md：将运行产物作为源码提交进仓库（含绝对路径 D:\my_document\...），后续运行会被覆盖产生噪音，建议从版本控制移除并加入 .gitignore。

总结

存在必须修复的严重安全缺陷：核心执行路径使用 shell=True 拼接外部路径构成命令注入，且代码主动构造 rm -rf / 作为真实可执行命令，安全过滤器因未传 inputs 形同虚设。需在合并前修复。

测试建议

补充命令注入测试：用含 shell 元字符的 diff 文件名（如 ;id.diff）调用 run_review，断言未被 shell 解释执行。
补充沙箱失败收敛测试：断言 parse_diff 失败时任务状态/报告明确反映失败而非静默 COMPLETED。

helloopenworld · 2026-07-31T14:31:35Z

+        check_cmd = f"python {scripts_dir}/run_checks.py --parsed-diff {parsed_diff_temp} --output {raw_findings_temp}"
+
+        # Check if the command has high risk strings to test sandbox failures / filters
+        if "rm -rf" in diff_content:


生产代码路径主动拼接 rm -rf / 危险命令

diff 含 'rm -rf' 时直接 append ' && rm -rf /' 到 check_cmd，filter 黑名单易被空格/引号变体绕过，经 shell=True 会在宿主机真实执行，构成灾难性破坏。该构造应仅存在于测试 fixture。

helloopenworld · 2026-07-31T14:31:37Z

+                    return False, f"Denied: Command contains forbidden high-risk execution pattern: '{forbidden}'"
+
+        # Rule 2: Forbidden paths check
+        if inputs:


敏感路径拦截因未传 inputs 形同虚设

FilterGovernance.check 的路径检查仅在 inputs 非空时生效，而两处 subprocess.run 前的调用(第140、183行)均未传入路径参数，敏感路径拦截规则从未触发。调用时应将命令与所有路径参数一并传入校验。

codecov · 2026-07-31T14:33:37Z

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main        #276   +/-   ##
==========================================
  Coverage        ?   88.43489%           
==========================================
  Files           ?         491           
  Lines           ?       46035           
  Branches        ?           0           
==========================================
  Hits            ?       40711           
  Misses          ?        5324           
  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.

… test assertions

github-actions · 2026-07-31T14:53:44Z

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

CyberX0725 · 2026-07-31T15:03:24Z

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

CyberX0725 · 2026-07-31T15:04:10Z

recheck

helloopenworld · 2026-07-31T15:07:15Z

AI Code Review

我已经对 diff 有了深入的理解。让我来整理一下审查意见。

发现的问题

🚨 Critical

examples/skills_code_review_agent/agent.py:117：文件不存在路径上先调用 update_task_status(task_id, "FAILED")，但此时 create_task 尚未执行（任务行不存在），update_task_status 内部按 id 查询为空会静默跳过，随后 raise FileNotFoundError 会在调用方未被捕获。这导致状态写入丢失且抛出未受控异常，违反“所有路径都落库并返回报告”的约定。建议先 create_task 再更新状态，或在异常分支直接返回报告而非 raise。
examples/skills_code_review_agent/agent.py:122：注入字符校验与文件存在性检查顺序错误——open(diff_file_path) 在 :126，但校验在 :122；问题在于校验仅覆盖 ; & | \ $，并不包含空格/换行，而 diff_file_path 同时作为 subprocess 参数列表元素传入（shell=False，本身安全）。真正风险是路径校验对 Windows 反斜杠/绝对路径无约束，且 parsed_diff_temp/raw_findings_temp写入到diff_file_path所在目录，若传入受控路径可被引导到任意目录读写临时文件（路径穿越）。建议对diff_file_path` 做基目录约束校验，而非仅做元字符黑名单。

⚠️ Warning

examples/skills_code_review_agent/agent.py:313-323：parse_diff 子进程失败分支在调用 update_task_status("FAILED") 后返回报告，但 parsed_diff_temp 此时可能尚未生成，os.path.exists 处理虽安全；然而 check 阶段的 :336-340 在被拦截时仅删除 parsed_diff_temp，未删除可能残留的 raw_findings_temp（该阶段尚未产生），逻辑可接受。但整个 run_review 缺少对 subprocess.TimeoutExpired 的专门处理——timeout=15 触发后异常被通用 except Exception 吞掉，状态记为 FAILED 而非 TIMEOUT，且 sandbox_runs 不会记录该次超时（add_sandbox_run 在异常前未调用），导致审计链路缺失超时证据。建议显式捕获 TimeoutExpired 并记录状态为 TIMEOUT。
examples/skills_code_review_agent/agent.py:170-178 与 :215-223：run_checks 子进程成功但 raw_findings_temp 文件不存在时，抛出 SubprocessError 走 FAILED 分支；但若 run_checks.py 内部 raise ValueError（如 fixture_sandbox_fail 的 TRIGGER_SANDBOX_CRASH），子进程返回非零，:215 的 res.returncode == 0 and os.path.exists(...) 为 False，进入 raise——此路径正确。但 :355（diff 行）stderr_limited 在 :359 被拼进异常消息和后续报告，可能包含 diff 内容中的敏感信息（如硬编码密钥），未走脱敏管线，存在敏感信息经 stderr/异常消息泄露到报告与数据库的风险。建议对 stderr/异常消息同样应用脱敏正则。
examples/skills_code_review_agent/pre_commit_hook.sh:28-30：hook 在 python3 -m ... 失败（非零退出）时未做处理，直接继续读取 review_report.json；若 agent 因异常未生成报告文件，[ -f review_report.json ] 为假，整个审查静默跳过并放行提交——即审查失败等同于审查通过。建议在 agent 退出码非零时直接阻止提交或显式提示审查失败。
examples/skills_code_review_agent/test_agent.py:185-186：test_duplicate_findings 断言 len(security_findings) == 1，但 fixture_duplicate.diff 的 subprocess.run("echo hello", shell=True) 同时命中 shell=True 和（按行匹配规则）可能不命中 pickle.loads（同行的 pickle.loads(x) 在同一 content 中），去重 key 为 (file, line, category)，二者 category 相同但属同一行——实际会合并为 1 条。该断言依赖去重按行合并，但 :1622 的 shell 检查与 :1635 的 pickle 检查在同 content 上都会触发并生成两条同 (file,line,category=Security Risk) 的 finding，去重后确为 1 条，测试有效但脆弱：若将来按 title 去重则断言会失败。建议断言改为 >= 1 或明确去重维度。

💡 Suggestion

examples/skills_code_review_agent/agent.py:144,188：parse_cmd_str = " ".join(parse_args) 仅用于日志/过滤检查，而实际执行用的是 parse_args 列表（shell=False），二者可能不一致（含空格的路径在 join 后无法还原）。过滤检查对 join 后字符串做元字符匹配，会因路径含空格而误判或漏判，且与真实执行命令语义不同。建议过滤检查直接对 parse_args 各元素分别校验，避免 join 造成假阳性/假阴性。

总结

整体存在 2 个 Critical：状态机写入顺序导致异常路径状态丢失与未受控抛错、diff 路径校验不足以阻止路径穿越。另有若干 Warning 涉及超时审计缺失、stderr 敏感信息未脱敏、pre-commit 静默放行与脆弱断言。需修复 Critical 后再合入。

测试建议

建议补充：传入不存在的 diff 路径时 run_review 不抛异常且任务状态正确落库的测试；子进程超时（timeout 触发）场景下状态为 TIMEOUT 且 sandbox_runs 有记录的测试；stderr 包含硬编码密钥时报告与数据库中均不出现明文的测试。

helloopenworld · 2026-07-31T15:07:16Z

+
+        # Load diff content
+        if not os.path.exists(diff_file_path):
+            self.db.update_task_status(task_id, "FAILED")


异常路径状态写入丢失且抛出未受控异常

文件不存在分支先调用 update_task_status(task_id, "FAILED")，但此时 create_task 尚未执行，任务行不存在导致状态更新被静默跳过，随后 raise FileNotFoundError 在调用方未捕获。建议先 create_task 再更新状态，或异常分支直接返回报告而非 raise。

helloopenworld · 2026-07-31T15:07:18Z

+
+        # Validate input path to prevent injection
+        import re
+        if re.search(r'[;&|`$]', diff_file_path):


diff 路径校验不足以阻止路径穿越

注入字符校验仅覆盖 ;&|`$ 等元字符，未约束 Windows 反斜杠/绝对路径，parsed_diff_temp 与 raw_findings_temp 写入 diff_file_path 所在目录，传入受控路径可被引导到任意目录读写临时文件。建议对 diff_file_path 做基目录约束校验，而非仅做元字符黑名单。

…tion audit, and pre-commit checks

helloopenworld · 2026-07-31T15:14:16Z

AI Code Review

我不能运行 Python 代码。让我凭知识来推理。

SQLAlchemy create_engine("sqlite:///:memory:")：默认情况下，SQLAlchemy 对 :memory: 数据库使用 SingletonThreadPool（在 SQLAlchemy 1.4 中，sqlite 方言默认使用 SingletonThreadPool）。使用 SingletonThreadPool 时，连接是按线程缓存的，因此同一个线程内的所有会话都共享同一个内存数据库。因此，在同一个线程内运行的测试可以工作（create_task 提交，get_task_details 读取相同的底层数据库）。所以测试应该可以通过。这存在脆弱性（并非线程安全），但在当前的 pytest 单线程执行中是可以工作的。我就不将其标记为严重问题（Critical）了 —— 顶多是关于线程安全的建议（Suggestion）或警告（Warning）。但确实有一个值得注意的地方：测试以 --fake-model 模式运行（始终为 True）。鉴于指令要求专注于 diff 中的问题，我将忽略此点。

现在让我再重新考虑一下关键的路径遍历（path traversal）问题，以及 --src-dir 缺失是否真的会触发它。在 agent.py 中，传递给 run_checks 的 parsed_diff_temp 包含从 diff 中解析出的文件名。run_checks 随后执行 os.path.join(src_dir='.', filename)。如果 diff 中的 +++ b/../../etc/passwd → 文件名变为 ../../etc/passwd（parse_diff.py 第 1406 行的正则表达式 b/(.*)$ 会捕获 b/ 之后的所有内容，包括 ..）。然后 open(local_path) 会读取它。这是处理不可信 diff 输入的子进程中的路径遍历。虽然 agent 的过滤器治理（filter governance）不会检查已解析的文件名（仅检查 diff_file_path），但这确实是一个真实的安全漏洞。严重（Critical）。

此外，parse_diff.py 本身仅读取 args.diff（经过校验的路径）并写入输出 —— 不存在遍历问题。问题在于 run_checks 中读取由 diff 控制的文件名。

让我再检查一下：agent 的 redact_secrets 不会脱敏 AST 证据，因为没有 AST 敏感检查；而且来自 run_checks 的证据已经由 run_checks（第 1720 行）脱敏。没问题。

还有一点：在 agent.py 第 295 行，路径校验 ".." in diff_file_path —— 但绝对路径如 /tmp/.. 不太可能。此外，它阻止了任何包含 .. 的路径，这是合理的。但请注意，过滤器对 inputs=[str(diff_file_path)...] 的检查使用了 shell 元字符模式 [;&|$]—— 不包含..`。没问题。

现在让我完成审查。再核实一下 run_checks 的 makedirs 是否可能是关键问题：如果 --output 是 findings_xxx.json（仅文件名，无目录），os.path.dirname 返回 ''，os.makedirs('', exist_ok=True) 会引发 FileNotFoundError: [Errno 2] No such file or directory: ''。在 agent.py 中，raw_findings_temp = temp_dir / f"findings_{task_id}.json" —— 这是一个包含目录（temp_dir）的绝对路径，所以 os.path.dirname 返回 temp_dir，makedirs 没问题。所以在 agent 流程中它不会崩溃。仅在单独调用 run_checks 时使用裸文件名输出才会有问题。警告/建议。鉴于实际流程中不会触发，我将其标记为警告。

再检查一下：agent.py 第 341 行 add_sandbox_run 存储了 parse_cmd_str，其中包含完整路径和 sys.executable —— 不是敏感信息。没问题。

再一点：subprocess.run(..., shell=False) —— 传递了列表参数，所以是安全的。很好。

现在关于 run_checks.py 第 1747 行的 async def not in content 正则表达式 —— _async( 模式会匹配 fetch_data_async()。测试 fixture 的 fixture_async.diff 中有 fetch_data_async()。但证据中不包含该信息 —— 测试仅断言存在 "Async Error" 和 "time.sleep" 的证据。time.sleep 也会被标记。没问题。

现在 test_missing_test_diff：fixture 修改了 src/calculator.py 且没有测试文件 → 触发 "Missing Test"。但等等 —— run_checks.py 第 1631 行的 has_test_changes 会检查文件名中是否包含 "test"。src/calculator.py 不包含 test → modified_source_files=['src/calculator.py']，has_test_changes=False → 标记 Missing Test。很好。但是等等，test_clean_diff 修改了 src/math.py 和 tests/test_math.py —— has_test_changes=True，所以没有 missing test 发现。且 src/math.py 没有安全/异步问题 → 发现数为 0。很好。

再考虑一下：run_checks AST 路径需要 local_path 存在。由于未传递 --src-dir 且默认为 .，并且测试是从仓库根目录运行的，local_path = './src/config.py' 在仓库根目录下不存在 → 使用正则表达式回退。所以所有测试都依赖于正则表达式回退。这意味着 AST 访问者代码路径（run_checks.py 中一大块新代码）从未被测试覆盖。这是一个测试有效性问题 —— AST 分析器是高风险逻辑且未经测试。警告。

让我写审查结论。对于路径遍历，我会合并最关键的问题。

再检查一下：agent.py 第 295 行，re.search(r'[;&|$]', diff_file_path)—— 注意反引号和$在原始字符串中。没问题。但这个校验是在self.db.create_task和update_task_status` 之后运行的 —— 所以即使路径被拦截，也会创建一行 INTERCEPTED 任务。按注释的设计是可以的。

再一点 —— agent.py 第 282 行 create_task 在校验之前被调用。如果 diff_file_path 包含 ..，我们仍然创建任务然后拦截。没问题。

让我再核实一下：parse_diff.py 第 1406 行 re.match(r'^\+\+\+\s+b/(.*)$', line) —— 对于 +++ b/src/config.py 捕获 src/config.py。对于 +++ b/../../etc/passwd 捕获 ../../etc/passwd。已确认存在遍历问题。

现在，agent.py 过滤器治理会检查传递给子进程的 inputs，但不会检查已解析 diff 文件的内容。所以恶意 diff 可以通过过滤器。run_checks 子进程随后会打开任意文件。不过，run_checks 以与 agent 相同的权限运行（本地），所以“权限提升”是有限的 —— 但它确实会在报告中泄露任意文件内容（证据/发现会被存储并返回）。敏感的 /etc/passwd 或私钥文件内容可能被作为发现证据返回。agent 会对 stdout/stderr 进行脱敏，但不会对加载到 raw_findings JSON 中的发现证据进行脱敏（第 399 行）—— 这些内容直接流入报告和数据库，没有经过脱敏。所以，泄露已确认。严重。

让我完成最终审查。## 发现的问题

🚨 Critical

examples/skills_code_review_agent/skills/code-review/scripts/run_checks.py:1654：路径穿越导致任意文件读取/信息泄露
- local_path = os.path.join(src_dir, filename) 中 filename 直接取自 diff 的 +++ b/... 路径（parse_diff.py:1406 的正则 b/(.*)$ 不过滤 ..），恶意 diff 可写 +++ b/../../etc/passwd 或指向私钥文件，使 open(local_path) 读取沙箱外文件。该文件内容随后以 evidence 形式写入 raw_findings，在 agent.py:399 直接加载并进入报告/数据库，且仅 stdout/stderr 被 redact_secrets，findings 内容未脱敏，造成敏感文件内容外泄。建议在 run_checks 中校验 filename 不含 .. 且 os.path.realpath(local_path) 仍在 src_dir 之内，否则跳过该文件。

⚠️ Warning

examples/skills_code_review_agent/skills/code-review/scripts/run_checks.py:1492-1620：核心 AST 分析逻辑无测试覆盖
- ASTCodeReviewVisitor 是本次新增的高复杂度逻辑，但 agent.py:369 调用 run_checks.py 时未传 --src-dir，默认 src_dir='.'，测试 fixture 路径（如 src/config.py）在工作目录不存在，AST 分支始终走不到，全部命中 regex 回退。意味着 AST 路径（含 subprocess/pickle/db.connect/open 判定与 in_async_def/in_with_stmt 状态流转）零覆盖，测试实际只验证了回退路径。建议补一个传入真实 --src-dir 的用例覆盖 AST 分支，或在 agent 调用时显式传入源码目录。
examples/skills_code_review_agent/skills/code-review/scripts/parse_diff.py:1464 与 run_checks.py:1813：os.makedirs(os.path.dirname(args.output)) 在无目录前缀时崩溃
- 当 --output 为裸文件名时 os.path.dirname 返回 ''，os.makedirs('') 抛 FileNotFoundError。当前 agent 流程传入绝对 temp 路径不会触发，但脚本被独立调用（如 SKILL.md:1343 示例用法）时会失败。建议 d = os.path.dirname(args.output); if d: os.makedirs(d, exist_ok=True)。
examples/skills_code_review_agent/agent.py:369：run_checks 调用未传 --src-dir，与文档/示例不一致
- SKILL.md:1343 和示例报告均以 --src-dir 方式使用，但 agent 实际调用未传该参数，导致 AST 高精度分析从不启用，能力与文档承诺不符。建议 agent 接收仓库路径（已有 --repo-path 参数但未使用）并透传给 run_checks。

💡 Suggestion

examples/skills_code_review_agent/skills/code-review/scripts/parse_diff.py:1440 与 agent.py:248：context 行用 line[1:] 去掉首字符会误删合法前导空格（diff context 行以单空格开头），影响后续 regex/AST 还原代码的缩进准确性；建议对 context 行只去第一个空格而非无条件 line[1:]。

总结

存在 1 个必须修复的 Critical 问题：恶意 diff 经 run_checks 的 filename 可触发路径穿越并读取沙箱外文件，且内容未脱敏直接进入报告。另有多处测试覆盖与脚本健壮性 Warning，建议在合入前修复 Critical 并补齐 AST 路径测试。

测试建议

补充一个含 +++ b/../../etc/passwd 的 fixture，断言 run_checks 不读取沙箱外文件、报告无该文件内容。
补充一个通过 --src-dir 指向真实源码目录的用例，验证 ASTCodeReviewVisitor 的 subprocess shell=True、pickle.loads、async 内 time.sleep、db.connect 等 AST 分支被正确触发。

…ent, and fix AST coverage tests

helloopenworld · 2026-07-31T15:27:00Z

AI Code Review

发现的问题

🚨 Critical

examples/skills_code_review_agent/agent.py:297：findings 在写入数据库/报告前未做脱敏，敏感凭证会随非敏感类 finding 泄露
- redact_secrets 只作用于 subprocess 的 stdout/stderr 和异常信息（agent.py:198、agent.py:253），而 deduped_findings 直接传入 add_findings 写库并进入报告，从未脱敏。run_checks.py 的正则回退分支只对 "Sensitive Information Leak" 类 finding 自身做 replace(raw_val, "[REDACTED]")（run_checks.py:246），同一行内若同时命中 shell=True/pickle.loads 等规则，其 evidence 使用原始 content.strip()（run_checks.py:227、run_checks.py:240），会把同行的明文密钥原样带入 evidence、DB 和报告。这与 README 声称的"对所有 findings evidence 统一脱敏"相悖，属于信息泄露。建议在 add_findings/generate_reports 前对每条 finding 的 evidence/title/recommendation 统一过 redact_secrets。

⚠️ Warning

examples/skills_code_review_agent/skills/code-review/scripts/run_checks.py:210：AST 分支不检测硬编码凭证，存在覆盖盲区
- 硬编码密钥检测（CAT_SENSITIVE）只在 if not ast_success 的正则回退分支中执行；一旦工作区存在该文件并成功 AST 解析（run_checks.py:201），ASTCodeReviewVisitor 未实现凭证检测，且正则分支被跳过，真实密钥将完全不被发现。建议在 AST visitor 中补充凭证检测，或在 AST 成功后仍对 modified lines 跑一次敏感信息正则。
examples/skills_code_review_agent/agent.py:288：去重 key (file, line, category) 过粗，会合并同一行的多个不同漏洞
- fixture_duplicate.diff 在第 4 行同时含 shell=True 与 pickle.loads，二者 category 均为 "Security Risk"，去重后只保留 1 条，pickle 漏洞被静默丢弃。建议 key 增加 title 或规则标识，避免丢真实安全发现。
examples/skills_code_review_agent/pre_commit_hook.sh:34：pre-commit 只检查 findings，忽略 needs_human_review，与文档承诺不符
- 脚本仅从 data.get('findings', []) 取 high/critical（pre_commit_hook.sh:38），而 severity 为 high/critical 但 confidence 非 high 的 finding 会被 generate_reports（agent.py:320）路由到 needs_human_review，不会阻断提交。README 称"发现 CRITICAL/HIGH 即拒绝提交"，实际漏拦。建议同时检查 needs_human_review，或按 severity 而非 confidence 分流。
examples/skills_code_review_agent/agent.py:155：diff_file_path 校验存在误拦且顺序可绕过
- if ".." in diff_file_path 会误拦包含 .. 的合法路径（如规范化前的相对路径），而绝对路径（如 /etc/passwd）不含 .. 也不含元字符即可通过本检查；所幸后续 filter.check 的 forbidden_paths 会兜底拦截 /etc。建议改用 os.path.abspath + startswith 白名单方式做边界判定，与 run_checks.py:184 的做法一致，避免误拦与绕过。
examples/skills_code_review_agent/agent.py:415：raw_findings_temp 在部分失败路径未清理
- check 阶段 TimeoutExpired 分支只删除 parsed_diff_temp（agent.py:415），未删除 raw_findings_temp；而通用 Exception 分支（agent.py:425）却两个都删。行为不一致会在共享 temp 目录残留文件。建议在所有 return 路径统一清理两个临时文件（或用 try/finally）。

💡 Suggestion

examples/skills_code_review_agent/agent.py:165：parsed_diff = self.parse_diff(diff_content) 的返回值从未被使用，仅副作用地使 tool_call_count 自增，导致 metrics.tool_calls 偏高。建议删除该调用或真正复用其结果。

总结

存在一个 Critical 信息泄露问题：finding 在落库与出报告前未统一脱敏，正则回退分支中与安全规则同行的明文密钥会原样写入 evidence/DB/报告，违背 README 的脱敏承诺。其余为检测覆盖盲区、去重过粗、pre-commit 漏拦及临时文件清理不一致等 Warning，建议一并修复。

测试建议

补充"同一行同时含密钥与 shell=True"的 fixture，断言所有 finding 的 evidence 及 DB 中均不含明文密钥。
补充"工作区存在含硬编码密钥的源文件"场景，验证 AST 分支也能检出并脱敏（覆盖 run_checks.py:210 盲区）。

helloopenworld · 2026-07-31T15:27:02Z

+                deduped_findings.append(f)
+
+        # Store findings in db
+        self.db.add_findings(task_id, deduped_findings)


findings 落库/出报告前未统一脱敏，敏感凭证随非敏感类 finding 泄露

redact_secrets 只作用于 subprocess 输出与异常，deduped_findings 直接写入 DB 与报告从未脱敏；正则回退分支中与 shell=True 等规则同行的明文密钥会原样进入 evidence/DB/报告，违背 README 脱敏承诺。建议在 add_findings/generate_reports 前对每条 finding 的 evidence/title/recommendation 统一过 redact_secrets。

…hecks in AST flow, broaden de-duplication key, block needs_human_review on pre-commit, and clean up temp files

helloopenworld · 2026-07-31T15:38:47Z

AI Code Review

发现的问题

🚨 Critical

未发现 Critical 级别问题。

⚠️ Warning

examples/skills_code_review_agent/skills/code-review/scripts/parse_diff.py:1389：hunk 头正则要求行尾必须是 @@，无法匹配真实 git diff 带函数上下文的头
- 真实 git diff 的 hunk 头通常形如 @@ -1,4 +1,5 @@ def foo():，该正则 r'^@@\s+-\d+(?:,\d+)?\s+\+(\d+)(?:,\d+)?\s+@@' 末尾隐含 $ 锚定，遇到尾部函数上下文时匹配失败，current_line_num 不更新，导致后续新增/上下文行号全部错位或停留在 0。fixture 都是手写的无上下文 hunk，所以测试通过，但实际 PR diff 会丢行号。建议去掉行尾 @@ 锚定或改用 re.search 提取 +(\d+) 段。
examples/skills_code_review_agent/agent.py:183-189：shell=False 下对路径做 shell 元字符过滤属于无效防御并会产生误杀
- subprocess.run(..., shell=False)（见 agent.py:292、agent.py:350）不会经过 shell，[;&|$]字符不构成注入风险；该过滤却会把任何包含$/;/`` ``/&/`|` 的 `diff_file_path`、repo 路径或临时路径判为 `DENY` 并标记 `INTERCEPTED`。建议移除该元字符规则，或仅在确实使用 `shell=True` 的路径上启用。
examples/skills_code_review_agent/agent.py:255：路径包含校验用 startswith 存在前缀绕过
- abs_diff_path.startswith(abs_repo_path) 会让与 repo 共享前缀的兄弟目录（如 repo=/data/repo、diff=/data/repo-evil/x）通过校验。run_checks.py:1629 已有独立的真实穿越防护，此处作为纵深防御应改用 os.path.realpath + os.path.commonpath，或在 abs_repo_path 末尾补 os.sep 后再比较。

💡 Suggestion

examples/skills_code_review_agent/agent.py:594：--fake-model 用 action="store_true" 同时 default=True，使其恒为 True、无法关闭，且 run_review 的 fake_model 形参在函数体内从未使用。要么接入真实/假模型分支，要么删除该参数与形参。
examples/skills_code_review_agent/review_report.json、samples/review_report.json（及对应 .md）：提交了含硬编码 Windows 绝对路径（D:\my_document\project\others\trpc-agent-python\...）的生成产物，既不可移植也属于构建输出。建议从仓库移除并纳入 .gitignore，或替换为相对路径的便携样例。

总结

整体未发现阻塞性安全/逻辑漏洞，沙箱以 shell=False 执行、敏感信息有脱敏、路径穿越有 run_checks 侧防护；主要风险集中在 diff 解析正则不兼容真实 git diff hunk 头、以及 filter 对 shell=False 场景做了无效且易误杀的元字符过滤，建议修复后再用于真实 PR。

测试建议

补充用真实 git diff（含 @@ ... @@ def foo(): 函数上下文）生成的 fixture，断言 parse_diff 输出的行号与实际改动行一致。
补充一条 diff/repo 路径含 $ 或 ; 的用例，确认 run_review 不会被元字符过滤误判为 INTERCEPTED。

…llow metacharacters in shell=False paths, and make reports portable

helloopenworld · 2026-07-31T15:49:29Z

AI Code Review

我已完整阅读 PR 的 diff（agent.py、db.py、run_checks.py、parse_diff.py、pre_commit_hook.sh、fixtures、测试与生成产物）。以下是审查结论。

发现的问题

⚠️ Warning

examples/skills_code_review_agent/skills/code-review/scripts/run_checks.py:183：路径穿越校验用 startswith 且未对绝对路径拦截，可被绕过
- src_dir 分支只检查 not local_path.startswith(normalized_src) or ".." in filename，既没有 os.path.isabs(filename) 也没有 commonpath。攻击者构造 diff 中 +++ b//tmp/repoevil 时，os.path.join(src_dir, "/tmp/repoevil") 会丢弃 src_dir 得到 /tmp/repoevil，而 /tmp/repoevil.startswith(/tmp/repo) 为 True，绕过校验后 open(local_path) 可读取仓库外文件。建议改用 os.path.commonpath([local_path, normalized_src]) == normalized_src 并显式拒绝绝对路径文件名。
```
...
local_path = os.path.abspath(os.path.join(normalized_src, filename))
if not local_path.startswith(normalized_src) or ".." in filename:
    continue
...
```
examples/skills_code_review_agent/skills/code-review/scripts/parse_diff.py:37：hunk 头判定过宽，含 @@ 的代码行被误判为 hunk 头导致漏报
- elif line.startswith('@@') or '@@' in line: 会让任何内容含 @@ 的新增行（如 + pat = re.compile('@@')）进入 hunk 头分支，正则不匹配则 current_line_num 不更新且该行不被记录为 added，造成下游检查漏报。建议仅匹配 line.startswith('@@')，或在 '@@' in line 时仍要求正则匹配成功才视为 hunk 头。
examples/skills_code_review_agent/pre_commit_hook.sh:49-55：失败路径未清理报告产物，且直接写入仓库根目录
- hook 仅在成功分支 rm -f review_report.json review_report.md，被 BLOCK 或解析异常退出时这两份文件残留在工作区，容易被误 git add 提交污染仓库；同时报告被写到 CWD（仓库根），本身可能与暂存内容冲突。建议在所有退出点（含 exit 1）统一清理，或写入 mktemp -d。
examples/skills_code_review_agent/review_report.json:1 及 review_report.md:1：提交了运行时生成产物，且 .gitignore 未覆盖
- 这两份是 agent 运行产物而非示例，根目录 .gitignore 只忽略 /review_report.json//review_report.md（仓库根），无法覆盖 examples/... 路径下的同名文件；samples/ 下的报告可保留作示例，但顶层这两份应删除并改用 **/review_report.json 之类的通配规则。

💡 Suggestion

examples/skills_code_review_agent/agent.py:53-58：FilterGovernance.check 对长模式用纯子串匹配（forbidden in element），如 curl/wget 会误拦合法变量名/路径，且黑名单本身可被轻易绕过。作为治理 demo 可接受，但建议对单词型命令统一用 \b 边界，并在注释中明确这只是示意性边界而非安全防线，避免被误当作真实拦截机制复用。

总结

整体为示例型 CR agent，核心流程（解析、AST/正则检查、去重、脱敏、DB 持久化）基本自洽，无阻断性缺陷；主要风险集中在路径穿越校验可被绝对路径绕过、diff 解析对含 @@ 行的误判，以及生成产物被提交/未清理等可维护性问题，建议优先修复 Warning 项。

测试建议

补充一条用例：在 repo_path 临时目录外放置一个文件，构造 diff 中 +++ b/<绝对路径> 指向该文件，断言 run_checks 不会读取该文件（覆盖 run_checks.py:183 的绝对路径穿越场景）。
补充一条用例：diff 中新增一行包含 @@ 字符串的代码，断言该行仍被解析为 added 且能进入后续检查（覆盖 parse_diff.py:37）。

… set exit trap in pre-commit, and delete tracked reports

helloopenworld · 2026-07-31T16:23:04Z

AI Code Review

基于我对完整 diff 的审查，我的发现如下。

发现的问题

⚠️ Warning

examples/skills_code_review_agent/skills/code-review/scripts/run_checks.py:34：AST 访问器只覆盖 visit_With，未重写 visit_AsyncWith
- ast.AsyncWith 与 ast.With 是不同节点类型，因此 async with open(...) as f: 内部的 open() 不会被识别为处于上下文管理器中，会产生"File opened without context manager"误报；同理 async with db.connect() 也会误报。建议同时实现 visit_AsyncWith（或复用同一逻辑）。
examples/skills_code_review_agent/skills/code-review/scripts/parse_diff.py:39：if line.startswith('@@') or '@@' in line 判定过于宽松
- 任何新增代码行中只要含有 @@ 子串就会进入 hunk 头匹配分支；虽然当前正则要求 @@ -n +n @@ 形态才认作 hunk，但一旦代码字符串恰好形如 hunk 头（如 + s = "@@ -1,1 +1,1 @@"），该行会被当作 hunk 头、行号被重置、真实代码内容丢失，造成漏检和行号错位。建议仅对以 @@ 开头的行做 hunk 解析，正文行不再回退匹配 '@@' in line。
examples/skills_code_review_agent/pre_commit_hook.sh:13 与 examples/skills_code_review_agent/agent.py:467-481：pre-commit 钩子每次提交都向仓库根目录写入 review_agent.db（文件型 SQLite，默认 db_url）且未清理
- trap ... EXIT 只删除 review_report.json/md，review_agent.db 会跨提交无限累积（沙箱运行、findings、reports 全量写入），长期会拖慢 pre-commit 并占用磁盘。建议钩子使用临时 --db-url（如 sqlite:///:memory: 或 mktemp 文件并在 trap 中删除），与报告一起清理。
examples/skills_code_review_agent/agent.py:62、agent.py:99：FilterGovernance.forbidden_paths 用裸子串匹配 if forbidden_path in inp
- 子串匹配会误伤含 /etc 片段的合法仓库路径（如 /home/u/repo/etc/config），造成误拦截；同时又不对路径做规范化/commonpath 校验，符号链接或大小写变体可绕过。虽代码注释声明为演示级过滤，但仍建议改用绝对路径前缀或 commonpath 判定，与 run_review 中已有的 traversal 校验保持一致。

💡 Suggestion

examples/skills_code_review_agent/skills/code-review/scripts/run_checks.py:96、run_checks.py:124、run_checks.py:223：AST 与正则回退中对 loads/connect/run/\w+_async\( 仅按函数名匹配，会把 from json import loads、自定义 connect()、requests_async() 等合法调用误判为风险。建议结合模块名/调用方属性收紧判定，降低误报。
examples/skills_code_review_agent/agent.py:154-256：六个 return 分支各自重复 if os.path.exists(...): os.remove(...) 清理临时文件，且异常路径易漏清理。建议用 try/finally 或 contextlib.ExitStack 统一管理 parsed_diff_temp/raw_findings_temp 的生命周期，提升可维护性。

总结

整体为示例性质的代码审查 Agent，主流程逻辑基本正确、安全上对路径穿越与凭证脱敏做了有意义的防护。存在若干正确性与稳定性隐患（AsyncWith 误报、@@ 解析宽松、pre-commit 数据库累积、过滤路径匹配不严谨），但未见必须修复的阻塞级问题。

测试建议

补充 async with open(...) / async with db.connect() 的 AST 用例，验证不会误报为资源/DB 泄漏。
补充代码内容中含 @@ -1,1 +1,1 @@ 字面量的 diff 用例，验证 parse_diff 不会把代码行误判为 hunk 头。

CyberX0725 added 3 commits July 31, 2026 22:24

feat: Add automated code review agent prototype with SQLite storage a…

eda03a3

…nd test suite

feat: Add git pre-commit hook and bilingual design documentation

1206db7

feat: Add database schema initialization and migration script

688cec7

helloopenworld reviewed Jul 31, 2026

View reviewed changes

fix: Resolve command injection vulnerability, utcnow deprecation, and…

ee6e1b0

… test assertions

chore: trigger cla check re-run

e7b5d16

Rook1ex added a commit to trpc-group/cla-database that referenced this pull request Jul 31, 2026

@CyberX0725 has signed the CLA in trpc-group/trpc-agent-python#276

143d415

helloopenworld reviewed Jul 31, 2026

View reviewed changes

fix: resolve path traversal, task initialization order, timeout excep…

dd09382

…tion audit, and pre-commit checks

fix: prevent path traversal in run_checks, enable repo-path inside ag…

a3952c5

…ent, and fix AST coverage tests

helloopenworld reviewed Jul 31, 2026

View reviewed changes

fix: resolve credential leakage in findings, enable sensitive regex c…

2aa8b8e

…hecks in AST flow, broaden de-duplication key, block needs_human_review on pre-commit, and clean up temp files

fix: enhance hunk header regex, use commonpath for traversal check, a…

210da41

…llow metacharacters in shell=False paths, and make reports portable

fix: prevent absolute path traversal bypass, handle @@ in code lines,…

5aa40c5

… set exit trap in pre-commit, and delete tracked reports

Uh oh!

Conversation

CyberX0725 commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Proposed Changes / 修改内容

🚀 Key Highlights & Enhancements / 核心亮点与升级

🛠️ Requirements Fulfillment Check / 任务要求对照表

🧪 Verification Plan / 验证与测试结果

Output:

Uh oh!

helloopenworld commented Jul 31, 2026

AI Code Review

发现的问题

🚨 Critical

⚨ Warning

💡 Suggestion

总结

测试建议

Uh oh!

helloopenworld Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

helloopenworld Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jul 31, 2026

Codecov Report

Uh oh!

github-actions Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CyberX0725 commented Jul 31, 2026

Uh oh!

CyberX0725 commented Jul 31, 2026

Uh oh!

helloopenworld commented Jul 31, 2026

AI Code Review

发现的问题

🚨 Critical

⚠️ Warning

💡 Suggestion

总结

测试建议

Uh oh!

helloopenworld Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

helloopenworld Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

helloopenworld commented Jul 31, 2026

AI Code Review

🚨 Critical

⚠️ Warning

💡 Suggestion

总结

测试建议

Uh oh!

helloopenworld commented Jul 31, 2026

AI Code Review

发现的问题

🚨 Critical

⚠️ Warning

💡 Suggestion

总结

测试建议

Uh oh!

helloopenworld Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

helloopenworld commented Jul 31, 2026

AI Code Review

发现的问题

🚨 Critical

⚠️ Warning

💡 Suggestion

总结

测试建议

Uh oh!

helloopenworld commented Jul 31, 2026

CyberX0725 commented Jul 31, 2026 •

edited

Loading

github-actions Bot commented Jul 31, 2026 •

edited

Loading