Skip to content

Dev/internal pipeline#189

Closed
CongkeChen wants to merge 4 commits into
internal_pipeline_testfrom
dev/internal_pipeline
Closed

Dev/internal pipeline#189
CongkeChen wants to merge 4 commits into
internal_pipeline_testfrom
dev/internal_pipeline

Conversation

@CongkeChen

Copy link
Copy Markdown
Contributor

No description provided.

@CongkeChen

Copy link
Copy Markdown
Contributor Author

AI Code Review

发现的问题

🚨 Critical

  • tests/ai_test.py:8:CI 测试路径中执行 shell=True 命令注入并读取系统文件
    • subprocess.run(f"cat {user_input}", shell=True)user_input="../../etc/passwd" 在真实 CI 测试中被收集执行(testpaths=["tests"],文件名匹配 test_*),会经 shell 拼接执行命令并尝试读取 /etc/passwd,属于命令注入 + 路径穿越的实际风险路径,而非仅静态示例。即便注释标明"intentionally bad code",它确实在自建 runner 上执行。修复:若仅用于 AI review 评测,应改名为非 test_ 前缀或加 pytest.mark.skip/移出 tests/,避免被 pytest 收集;若必须保留,去掉 shell=True 并用参数列表形式 ["cat", user_input],且不要读系统文件。

⚠️ Warning

  • tests/ai_test.py:9-13:异常被吞没、断言恒真,测试无任何有效性
    • except Exception: pass 掩盖所有错误后紧跟 assert True,使该测试无论逻辑是否正确都通过,不提供任何回归保护却带来上述执行副作用。建议删除该测试,或改为真正断言被测逻辑;如作为 AI review 评测夹具,应与 CI 测试集隔离。
  • tests/ai_test.py:7:硬编码疑似密钥字符串
    • secret_token = "FAKE_SECRET_TOKEN_FOR_REVIEW_TEST" 虽为伪造值,但硬编码密钥形态字符串进入仓库,易被误当真实凭证或被未来复制为模板。建议直接用占位常量命名而非密钥形态字符串。

💡 Suggestion

  • tests/ai_test.py:13:文件末尾缺少换行符(No newline at end of file),建议补一个换行以符合 POSIX 文本文件约定。

总结

新增的 tests/ai_test.py 会被 pytest 在 CI 中真实收集并执行,其中 shell=True 拼接命令读取 /etc/passwd 构成实际的命令注入/路径穿越风险,属于必须修复的 Critical 问题;其余为测试有效性与卫生问题。.github/ai-review-prompt.md 的变更仅为提示词文本完善,无风险。

测试建议

暂无额外测试建议;当前应优先移除或隔离 tests/ai_test.py,使其不进入 CI 执行路径,而非补充测试。

@CongkeChen CongkeChen closed this Jul 15, 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