Skip to content

Test ai review bad code#9

Closed
CongkeChen wants to merge 3 commits into
mainfrom
test-ai-review-bad-code
Closed

Test ai review bad code#9
CongkeChen wants to merge 3 commits into
mainfrom
test-ai-review-bad-code

Conversation

@CongkeChen

Copy link
Copy Markdown
Owner

No description provided.

@github-actions

Copy link
Copy Markdown

AI Code Review

这个 diff 只新增了一个测试文件 tests/test_ai_review_bad_code.py,且文件本身就是"故意写坏"的代码(注释明确说明用于 AI review 测试)。下面给出基于 diff 的审查结论。

发现的问题

🚨 Critical

  • tests/test_ai_review_bad_code.py:14:命令注入(shell=True + 未校验输入拼接)

    • 问题:subprocess.run(f"cat {user_input}", shell=True) 将不可信输入 user_input../../etc/passwd)直接拼进 shell 命令字符串并启用 shell=True。这是典型的命令注入模式。虽然 user_input 当前是硬编码字面量,但模式本身极危险,一旦被复制到真实代码中将形成实际漏洞。
    • 影响:恶意/异常输入可注入任意 shell 命令(例如 ; rm -rf … 或反引号、$()),导致任意命令执行;本例的 ../../etc/passwd 也构成路径穿越,可读取敏感文件。作为仓库内"示例代码",会被其他开发者参考复用,污染代码库的安全习惯。
    • 建议:即便作为"坏代码样本",也不应留在仓库中被 CI 误采集或被人复用。若确需保留作为 AI review 的 fixture,应:删除该文件或将其移到非测试目录(如 fixtures/)且被 pytest 自动收集;不要使用 shell=True,改用参数列表形式 subprocess.run(["cat", user_input]);并对路径输入做白名单/规范化校验(如 os.path.realpath 后校验前缀)。
  • tests/test_ai_review_bad_code.py:9:测试函数会被 pytest 自动收集并执行,存在真实副作用与 CI 风险

    • 问题:函数名以 test_ 开头,位于 tests/ 下,会被 pytest 默认收集运行。函数体内调用了 subprocess.run(f"cat ../../etc/passwd", shell=True),会真实尝试读取系统 /etc/passwd
    • 影响:CI 环境每次跑测试都会执行该命令,依赖环境是否存在 cat、对 /etc/passwd 的访问权限;在受限/沙箱 runner 中可能失败或告警,造成 CI 阻塞或安全告警。同时它把"故意坏代码"作为常态引入测试套件,污染测试信号。
    • 建议:将其改为不会被自动收集的形式(重命名为非 test_ 前缀,或移出 tests/),或加 @pytest.mark.skip,或直接从仓库中删除。AI review 的样本不应进入实际测试执行路径。

⚠️ Warning

  • tests/test_ai_review_bad_code.py:12:硬编码伪造凭证字符串

    • 问题:secret_token = "FAKE_SECRET_TOKEN_FOR_REVIEW_TEST" 在源码中硬编码 token。虽标注为 FAKE,但引入了"代码中放置凭证字面量"的反模式。
    • 影响:容易被扫描器(如 secret-scanning)误报,也可能被复制到真实代码中变成真实硬编码凭证;增加安全审计噪声。
    • 建议:若保留该样本,使用明显非凭证占位符(如 "PLACEHOLDER_NOT_A_SECRET")并注释说明;或干脆删除该行。
  • tests/test_ai_review_bad_code.py:15-18:异常被静默吞掉(except Exception: pass

    • 问题:try: int("not-a-number") except Exception: pass 捕获所有异常并忽略,没有任何处理或日志。
    • 影响:作为可被复用的样本,示范了"吞异常"反模式,可能掩盖真实错误、增加排查难度。
    • 建议:样本应明确标注这是反面教材并避免被复制;实际代码中应至少记录日志或缩小异常类型范围。

💡 Suggestion

  • tests/test_ai_review_bad_code.py:9-19:测试有效性为零

    • 问题:assert secret_tokenassert True 不构成任何有意义断言;测试既不验证业务逻辑,也不覆盖真实风险路径,纯为占位。
    • 影响:长期维护性差,给人"已有测试覆盖"的错觉,实际无防护价值。
    • 建议:如该文件仅为 AI review 测试靶材,应与正式测试集合隔离并加注释说明用途;不要作为有效测试计入覆盖率。
  • tests/test_ai_review_bad_code.py:20:文件缺少末尾换行符(No newline at end of file

    • 问题:文件末尾无换行,不符合 POSIX 文本文件惯例。
    • 影响:轻微的可维护性/工具兼容性问题,部分工具 diff 行为异常。
    • 建议:在文件末尾追加一个换行符。

总结

本 PR 仅新增一个名为"故意坏代码"的测试样本文件。代码本身被注释标注为"用于 AI review 测试",但其中包含命令注入(shell=True + 输入拼接)、路径穿越、硬编码凭证、吞异常等高危反模式,并且以 test_ 前缀位于 tests/ 下会被 pytest 自动收集并在 CI 中真实执行 cat /etc/passwd

存在必须修复的问题(Critical):

  1. 命令注入与路径穿越反模式留在仓库中,有被复用扩散的风险;
  2. 作为测试函数会被 CI 实际执行,带来安全告警/CI 阻塞风险。

建议处理方式:不要让"坏代码样本"进入正式测试执行路径。要么删除该文件,要么将其移出 tests/ 并重命名为非 test_ 前缀的 fixture,并配合注释/隔离,使其只作为 AI review 的输入材料、不被 pytest 收集运行。

测试建议

暂无额外测试建议。当前新增的"坏代码"本身不应作为有效测试纳入套件;若要验证 AI review 流程,建议改为对 review 工具/脚本输出的断言式测试(例如给定一份已知坏代码 fixture,断言 review 能产出特定类别告警),而不是把坏代码作为可执行测试函数放入 tests/

@CongkeChen CongkeChen closed this Jul 14, 2026
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.

1 participant