Test ai review bad code

[CongkeChen]

No description provided.

[github-actions]

AI Code Review

总结

本 PR 仅新增了一个测试文件 tests/test_ai_review_bad_code.py，内容是一个故意包含多种问题的示例函数，用于验证 AI 代码审查工具是否能识别坏代码。整体风险很低，文件本身不是生产代码。

但存在一个值得注意的问题：文件命名以 test_ 开头却未包含任何真正的测试断言，会被测试框架当作测试用例收集并“通过”，可能掩盖真实的测试缺失。

发现的问题

1. 文件名为 test_*.py 但没有任何测试断言

• 文件路径：tests/test_ai_review_bad_code.py
• 问题描述：文件命名为 test_ai_review_bad_code.py，pytest/unittest 等测试框架会按惯例自动收集该文件中的函数。但其中 obviously_bad_function 既不是 test_ 开头的测试函数（不会被当作测试运行），也不包含任何 assert，整体只是示例代码。如果本意是作为测试，则缺失实际断言；如果本意只是被 AI 审查工具分析的样本，则放在 tests/ 目录并使用 test_ 前缀会误导测试收集与 CI 统计。
• 风险等级：Warning
• 修复建议：明确该文件的用途。
  • 若是 AI 审查工具的输入样本，建议移到非 tests/ 目录（如 samples/ 或 fixtures/），并去掉 test_ 前缀，避免被测试框架误收集。
  • 若确为测试，应补充实际的断言，例如断言 AI 审查工具对该文件能检出硬编码口令、eval 使用等问题。

测试建议

• 当前文件作为“坏代码样本”验证 AI 审查能力是合理场景，但应补一个真正的测试：调用 AI 审查逻辑对 obviously_bad_function 进行分析，并断言结果中至少包含“硬编码口令”与“eval 滥用”两类问题，以防止审查工具回归漏报。
• 建议验证该样本文件不会被 CI 的测试步骤误判为“通过 1 个测试”，从而导致测试数量虚高。

[github-actions]

AI Code Review

发现的问题

🚨 Critical

• tests/test_ai_review_bad_code.py:8：测试在执行时真实触发命令注入与路径穿越
  • 问题：subprocess.run(f"cat {user_input}", shell=True) 中 user_input = "../../etc/passwd"（第 4 行）。该函数名为 test_bad_code_example，会被 pytest 自动收集并实际执行。shell=True 配合 f-string 拼接不可信输入，运行时会真实执行 cat ../../etc/passwd，相对于测试工作目录解析为系统敏感文件（如 /etc/passwd）。即便这是“故意写错的样例”，它并不是被静态展示的文本，而是会被 CI/本地测试实际运行的代码。
  • 影响：每次跑测试都会真实读取宿主文件系统文件；若 user_input 被修改为更危险的命令（如 ; rm -rf ...）也会被 shell 直接执行，构成命令注入。在 CI runner 上读取 /etc/passwd 等属于不必要的敏感信息访问，且一旦样例值被改动可能造成实际破坏。
  • 建议：将“坏代码样例”做成不会被执行的载体，例如存为纯文本字符串/单独的 .txt 资源供 AI review 流程读取，或给该测试加 pytest.mark.skip / pytest.importorskip 守卫，确保它只作为静态样本存在而不在测试运行时执行。若必须保留为可执行测试，至少不要使用 shell=True 与真实路径，改用不会产生副作用的占位写法。

⚠️ Warning

• tests/test_ai_review_bad_code.py:6：硬编码 token 字样

  • 问题：secret_token = "FAKE_SECRET_TOKEN_FOR_REVIEW_TEST" 以明文形式写入了“密钥”形态的字符串。虽然内容是假值且用于测试 AI review，但其命名与形式与真实凭据一致。
  • 影响：可能被凭据扫描工具（secret scanning）误报或被加入扫描白名单后掩盖真实泄露；同时也容易让后续贡献者误以为在测试中硬编码密钥是可接受的做法。
  • 建议：如仅需触发 AI review 的“硬编码凭据”检测，可将其放在不被执行的样本文本中；若必须保留，明确标注并在 README/注释中说明此为非真实凭据，避免被纳入 secret 扫描白名单。

• tests/test_ai_review_bad_code.py:11-12：吞掉所有异常

  • 问题：except Exception: pass 静默吞掉 int("not-a-number") 抛出的异常，且不记录、不重新抛出。
  • 影响：虽然此处理论上是“坏代码样例”，但它会被实际执行；这种模式会掩盖真实错误，作为可运行测试时会让失败路径完全不可见。
  • 建议：与 Critical 项一并处理——若改为静态样本则无此问题；若仍可执行，应避免 except Exception: pass 这种全量静默吞异常。

• tests/test_ai_review_bad_code.py:7,13：断言无意义，测试无实际验证价值

  • 问题：assert secret_token（第 7 行）对非空字符串恒真；assert True（第 13 行）同样恒真。整个测试不验证任何被测行为，只是“跑过即通过”。
  • 影响：该测试给出虚假的覆盖率/通过信号，让人误以为有测试保护，实则未覆盖任何风险路径。
  • 建议：若目的是给 AI review 提供坏代码样本，应将其从 pytest 收集范围中移除（见 Critical 项），避免产生伪测试通过信号。

💡 Suggestion

• tests/test_ai_review_bad_code.py:1：未使用的导入

  • 问题：import os 在文件中未被使用。
  • 影响：轻微的可维护性问题，可能误导读者以为用到了 os 相关功能。
  • 建议：删除未使用的 import os；或在改为静态样本后整体重构该文件。

• tests/test_ai_review_bad_code.py:20：文件末尾缺少换行符

  • 问题：diff 末尾标注 No newline at end of file。
  • 影响：不符合 POSIX 文本文件规范，部分工具/diff 会有告警。
  • 建议：在文件末尾补一个换行符。

总结

本 PR 仅新增一个 tests/test_ai_review_bad_code.py 文件，内容是“故意写错的代码样例”，用于测试 AI 代码审查流程能否检出问题。核心风险在于：该样例被写成了一个会被 pytest 实际收集并执行的测试函数，其中 subprocess.run(f"cat {user_input}", shell=True) 会在每次测试运行时真实触发路径穿越/命令执行，属于必须修复的问题（Critical）。其余为硬编码凭据样态、静默吞异常、无意义断言等次要问题。建议将该坏代码样本改为不被执行的静态载体，从根本上消除执行期风险。

测试建议

• 该文件本身不应作为可执行测试参与 pytest 运行。建议补充一条机制性校验：确认“坏代码样本”不会被 pytest 默认收集执行（例如放在非 test_ 前缀的文件中，或通过 conftest.py 的收集忽略规则排除），并在 CI 中验证 AI review 流程能从该样本中检出命令注入、硬编码凭据、吞异常等问题。
• 可为 AI review 检测能力本身增加断言式测试（例如对样本文本运行审查逻辑并断言输出中包含对应问题类别），以避免样本被改动后检测能力静默失效。