Dev/internal pipeline by CongkeChen · Pull Request #196 · trpc-group/trpc-agent-python

CongkeChen · 2026-07-16T07:07:36Z

No description provided.

CongkeChen · 2026-07-16T07:09:49Z

AI Code Review

Based on the diff and repo context, I've verified the relevant facts:

pyproject.toml:204-208 sets testpaths = ["tests"] with default collection (test_*.py / *_test.py); this file is named ai_review_demo_bad_code.py with no test_ functions, so pytest will not collect or execute it. CI runs pytest --cov-fail-under=80 tests/.
CI's flake8 step lints all changed .py files; .flake8 sets max-line-length=120, ignore=E402,W503. I checked: no line in this file exceeds 120 chars, imports are all used, so it should not break the lint gate.
Commit 9f515bc message: "test: add intentional bad code for AI review pipeline" — confirming the file is a deliberate review-pipeline demo.

发现的问题

🚨 Critical

tests/ai_review_demo_bad_code.py:9-10：硬编码凭证（API_KEY、DB_PASSWORD）入库
- 即便值是 do-not-use 占位串，写入仓库会污染密钥扫描基线、可能被误当作真实凭据；演示用敏感值应放到独立的非受控 fixture 或从环境变量注入，避免进入版本库历史。由于该文件不被 pytest 收集（见下），当前不存在运行时泄露路径，但入库行为本身就是风险。
tests/ai_review_demo_bad_code.py:17-19：shell=True 命令注入
- subprocess.run(cmd, shell=True, ...) 直接执行外部传入字符串。作为 demo 其危害可控（不被收集执行），但若未来被 test_* 复用即成真实漏洞；应改为参数列表 subprocess.run(shlex.split(cmd), ...) 并去掉 shell=True。

⚠️ Warning

tests/ai_review_demo_bad_code.py:22-25：路径穿越
- read_user_file 直接字符串拼接 base_dir + "/" + relative_path，../ 可越界；write_report（75-78）同样用 name 直接拼路径可写任意位置。建议用 Path(base_dir).resolve() 校验最终路径仍在 base_dir 内。
tests/ai_review_demo_bad_code.py:38-39：鉴权逻辑反转
- is_authorized 在 role 为空/None 时返回 True，等于“无角色即放行”；authenticate（13-14）用 == 比对 token 且允许 user="admin" 任意口令风格弱校验。若 demo 被复用为真实鉴权样板会直接造成权限绕过，建议修正默认拒绝语义。
tests/ai_review_demo_bad_code.py:46-54：资源未释放 + 解析鲁棒性
- load_config 用 open() 未 with 关闭文件句柄；值含 = 时 split("=",1) 仅取两段尚可，但空行/注释/多值无处理。建议改 with open(...) 并忽略无 = 的行（当前仅 if "=" in line，注释行含 #= 仍会误解析）。
tests/ai_review_demo_bad_code.py:57-61：SSRF / 无超时
- call_remote_api 直接 urlopen(url) 无 scheme 校验、无超时、无重试，可访问内网地址且易挂死。建议校验 urlparse(url).scheme in {"http","https"} 并加 timeout。
tests/ai_review_demo_bad_code.py:64-66：敏感信息输出
- dump_debug 默认 print 整个 os.environ，会将 CODECOV_TOKEN 等密钥打印到日志。即便 demo 不执行，模板若被复用会泄露真实环境密钥；建议显式白名单字段。
tests/ai_review_demo_bad_code.py:42-43、69-72：边界/可变性缺陷
- average 对空列表 len(scores)==0 会抛 ZeroDivisionError；merge_settings 直接 defaults.update(overrides) 原地修改入参字典。建议空列表返回 0.0/抛明确异常，合并改为构造新 dict。

💡 Suggestion

tests/ai_review_demo_bad_code.py:1-78：该文件置于 tests/ 但无 test_ 前缀与测试函数，pytest 不会收集，既无测试价值也不计入覆盖率，且会被 flake8 gate 检查（已验证当前不超长、无未用导入，可通过）。若仅作 AI review 样本，建议放到 tests/fixtures/ 或 examples/ 等非收集目录，并在文件顶部加注释说明“故意构造、请勿复用”，避免其他开发者误当样板导入。

总结

这是一个为 AI review 流水线故意构造的“坏代码”样例：因文件名不匹配 pytest 收集规则且无测试函数，上述安全/逻辑缺陷当前不存在 CI 执行或线上风险，不存在必须修复的阻塞性问题；但从长期维护看，硬编码凭证入库与可被误复用的危险样板仍建议至少隔离目录并加注释。

测试建议

暂无额外测试建议——该文件本意非被测对象；若希望验证 review pipeline 能命中这些缺陷，可另起一个独立的断言型测试（如对 run_user_command 注入 ; echo 断言其被识别为风险）放在 test_* 文件中，避免让本 demo 进入收集路径。

CongkeChen · 2026-07-16T07:39:27Z

AI Code Review

发现的问题

说明：本 PR 仅新增 tests/ai_review_demo_bad_code.py。该文件位于 tests/ 下，会被 CI 的 pytest tests/（.github/workflows/ci.yml:78）收集并在导入期执行模块级代码；按真实风险评估，不因其为“示例/演示”降级。

🚨 Critical

tests/ai_review_demo_bad_code.py:9-10：硬编码凭证
- 模块级 API_KEY/DB_PASSWORD 以明文常量入库，任何密钥扫描或日志采集都会命中并可能误判为真实泄露。应改为从环境变量/密钥管理读取，演示场景可用明显占位符且不进入仓库。
```
API_KEY = "sk-live-hardcoded-secret-do-not-use"
DB_PASSWORD = "P@ssw0rd123"
```
tests/ai_review_demo_bad_code.py:17-19：命令注入
- subprocess.run(cmd, shell=True) 直接执行外部字符串，存在命令注入。应使用列表参数并禁用 shell。
```
result = subprocess.run(cmd, shell=True, capture_output=True, text=True)
```
tests/ai_review_demo_bad_code.py:22-24 与 tests/ai_review_demo_bad_code.py:75-78：路径穿越
- read_user_file 用字符串拼接 relative_path、write_report 用 name 直接拼路径，均未校验 ../绝对路径，可越权读写任意文件。应对拼接结果做基于 base_dir/output_dir 的 resolve() 边界校验。
```
target = base_dir + "/" + relative_path
...
p = Path(output_dir) / (name + ".txt")
```

⚠️ Warning

tests/ai_review_demo_bad_code.py:32-35：鉴权逻辑反转
- is_authorized 在 role 为空（None/空串）时直接返回 True，构成权限绕过。空角色应默认拒绝，或显式定义“匿名”语义。
tests/ai_review_demo_bad_code.py:38-40：空值解引用
- fetch_profile 对 user_id 调用 .upper()，当传入 None 时抛 AttributeError。应在入口判空或返回占位结果。
tests/ai_review_demo_bad_code.py:42-43：除零
- average 未处理空列表，sum([])/len([]) 抛 ZeroDivisionError。应校验 scores 非空。
tests/ai_review_demo_bad_code.py:46-55：资源未释放与异常未处理
- load_config 用裸 open 未 with/close，异常时文件泄漏；且无编码/IO 错误处理。应改用 with open(...)。
tests/ai_review_demo_bad_code.py:57-60：SSRF/无超时
- call_remote_api 对外部 url 无校验、无超时，可被用于内网请求或因对端无响应挂死。应校验协议/主机并设置 timeout。
tests/ai_review_demo_bad_code.py:64-66：敏感信息输出
- dump_debug 默认打印整个 os.environ，易将 TOKEN/密钥等输出到日志。应过滤敏感键或仅输出白名单键。
tests/ai_review_demo_bad_code.py:69-72：可变默认/原地修改
- merge_settings 用 defaults.update(overrides) 直接修改入参字典并返回，调用方持有的默认对象被污染。应拷贝后再合并。
tests/ai_review_demo_bad_code.py:13-15：非常量时间比较
- authenticate 用 == 比较令牌，存在时序侧信道风险。敏感比对建议 secrets.compare_digest。
tests/ai_review_demo_bad_code.py:28-30：折扣计算逻辑错误
- apply_discount 用 price + percent/100，语义应为 price * (1 - percent/100)，且无 percent 范围校验，结果错误且可能产生负价。

💡 Suggestion

tests/ai_review_demo_bad_code.py:57-60：函数内 import urllib.request 不必要，建议上移至模块顶部，保持导入风格一致。
tests/ai_review_demo_bad_code.py:1-85：若该文件确为 AI 审查流水线的“靶子”样例，建议放在独立目录（如 tests/ai_review_fixtures/）并用 conftest.py/pyproject.toml 的 norecursedirs/collect_ignore 排除，避免被 CI 的 pytest tests/ 误收集或被密钥扫描误报。

总结

整体为新增演示性“坏代码”样例，但其中包含硬编码凭证、命令注入、路径穿越等多项真实高危缺陷，且文件会被 CI 测试路径收集，存在被扫描误报与误用风险；存在多条必须修复的 Critical 问题。

测试建议

暂无额外测试建议：若保留该样例，应补充针对 is_authorized（空角色）、average（空列表）、read_user_file/write_report（.. 穿越）等高风险路径的断言用例；若仅作静态靶子，则应将其从 pytest tests/ 收集范围中排除。

CongkeChen · 2026-07-16T07:39:28Z

+from typing import Any
+
+
+API_KEY = "sk-live-hardcoded-secret-do-not-use"


硬编码凭证

模块级 API_KEY/DB_PASSWORD 以明文常量入库，易被密钥扫描或日志采集命中并误判为真实泄露。应改为从环境变量/密钥管理读取，演示场景可用明显占位符且不进入仓库。

CongkeChen · 2026-07-16T07:39:29Z

+    return token == API_KEY and user == "admin"
+
+
+def run_user_command(cmd: str) -> str:


命令注入

subprocess.run(cmd, shell=True) 直接执行外部字符串，存在命令注入风险。应使用列表参数并禁用 shell。

CongkeChen · 2026-07-16T07:39:30Z

+    return result.stdout
+
+
+def read_user_file(base_dir: str, relative_path: str) -> str:


路径穿越(read_user_file)

read_user_file 用字符串拼接 relative_path，未校验 .. 或绝对路径，可越权读取任意文件。应对拼接结果做基于 base_dir 的 resolve() 边界校验。

CongkeChen · 2026-07-16T07:39:31Z

+def merge_settings(defaults: dict[str, Any], overrides: dict[str, Any] | None) -> dict[str, Any]:
+    if overrides:
+        defaults.update(overrides)
+    return defaults


路径穿越(write_report)

write_report 用 name 直接拼路径写文件，未校验 .. 或绝对路径，可越权写入任意文件。应对拼接结果做基于 output_dir 的 resolve() 边界校验。

CongkeChen · 2026-07-16T09:13:43Z

AI Code Review

根据审查要求，第 10 条明确指出：测试代码、示例代码、演示代码也按真实执行风险评估；不要因"仅用于示例/测试"而降低等级。这个文件名为 demo_bad_code，但会进入 CI/测试路径且可能被复用，需按真实风险审查。

发现的问题

🚨 Critical

tests/ai_review_demo_bad_code.py:15-16：硬编码真实凭证
- 代码中硬编码了形似真实密钥的 API_KEY 和 DB_PASSWORD，即使标注 "do-not-use"，进入仓库即构成凭证泄露风险，且 authenticate 直接用该常量做鉴权，演示价值也在传递错误模式。应移除硬编码值，改用从环境变量/配置注入的占位常量。
tests/ai_review_demo_bad_code.py:24：shell=True 的命令注入
- run_user_command 将外部传入的 cmd 直接交给 shell=True 执行，且 capture_output=True 但未设置 timeout，属于明确的命令注入 + 资源挂死风险。应改为参数列表形式（shell=False）并对参数做白名单/转义，补充 timeout。
tests/ai_review_demo_bad_code.py:28-29：路径穿越
- read_user_file 用字符串拼接 base_dir + "/" + relative_path，relative_path 未校验，可构造 ../ 逃逸出 base_dir 读取任意文件。应使用 Path(base_dir).resolve() 并校验结果仍位于 base_dir 之内。

⚠️ Warning

tests/ai_review_demo_bad_code.py:38-41：鉴权逻辑反转导致权限绕过
- is_authorized 在 role 为空/None 时 return True，即"无角色"被当作"已授权"，与函数语义相反，属权限绕过。空角色应拒绝或显式处理，默认应返回 False。
tests/ai_review_demo_bad_code.py:34-35：折扣计算逻辑错误
- apply_discount 用 price + percent/100 而非 price * (1 - percent/100)，属明确逻辑错误，会随输入增大反而加价。应修正为正确的折扣公式并补单测。
tests/ai_review_demo_bad_code.py:44-45：空值未处理导致崩溃
- fetch_profile 在 user_id 为 None 时调用 user_id.upper() 抛 AttributeError。应先校验非空或返回默认值。
tests/ai_review_demo_bad_code.py:48-49：空列表除零
- average 未处理空 scores，空列表时 len(scores)==0 抛 ZeroDivisionError。应显式处理空输入。
tests/ai_review_demo_bad_code.py:63-66：未校验 URL 且无超时
- call_remote_api 直接 urlopen(url)，url 来源未做 scheme/域名校验（易被 file://、内网地址滥用），且无 timeout 易长时间挂起。应校验 url 必须为 https 目标并设置 timeout。
tests/ai_review_demo_bad_code.py:52-53：资源未释放
- load_config 中 f = open(...) 未用 with 管理，异常时文件句柄泄漏。应改用 with open(...)。
tests/ai_review_demo_bad_code.py:75-77：可变默认参数副作用
- merge_settings 直接 defaults.update(overrides) 并返回原 dict，会修改调用方传入的可变对象，违反不可变预期。应基于 defaults 拷贝后再合并。
tests/ai_review_demo_bad_code.py:81-83：写入路径穿越
- write_report 用 name 直接拼接输出路径，若 name 含 ../ 可写到 output_dir 之外。应校验 name 不含路径分隔符或对最终路径做归属校验。

总结

该 PR 新增文件本身即以"反面示例"形式集中了硬编码凭证、命令注入、路径穿越、权限绕过、逻辑错误等高风险问题；按真实执行风险评估，存在多条必须修复的 Critical（凭证泄露、命令注入、路径穿越）。若该文件仅作为 AI 审查流程的样例输入而非被 import/执行，建议在文件头明确标注并放入非运行目录，或将其内容改为不会通过安全扫描的"已脱敏"版本；但只要它进入仓库与 CI，上述问题仍需处理。

测试建议

该文件以"bad code"为名却无对应断言其行为的测试。如果意图是验证 AI 审查能检出这些问题，应补一个测试断言审查流水线对本文件至少能报出"命令注入/路径穿越/硬编码凭证"三类 Critical；否则建议直接移除该演示文件，避免将带漏洞代码长期留在仓库中。

CongkeChen · 2026-07-16T09:13:44Z

+
+def authenticate(user: str, token: str) -> bool:
+    return token == API_KEY and user == "admin"
+


硬编码真实凭证

代码中硬编码了形似真实密钥的 API_KEY 和 DB_PASSWORD，进入仓库即构成凭证泄露风险，且 authenticate 直接用该常量做鉴权，演示价值也在传递错误模式。应移除硬编码值，改用从环境变量/配置注入的占位常量。

CongkeChen · 2026-07-16T09:13:45Z

+
+def read_user_file(base_dir: str, relative_path: str) -> str:
+    target = base_dir + "/" + relative_path
+    with open(target, "r", encoding="utf-8") as f:


shell=True 的命令注入

run_user_command 将外部传入的 cmd 直接交给 shell=True 执行，且未设置 timeout，存在命令注入 + 资源挂死风险。应改为参数列表形式（shell=False）并对参数做白名单/转义，补充 timeout。

CongkeChen · 2026-07-16T09:13:46Z

+        return f.read()
+
+
+def apply_discount(price: float, percent: float) -> float:


路径穿越

read_user_file 用字符串拼接 base_dir + relative_path 且未校验，可构造 ../ 逃逸出 base_dir 读取任意文件。应使用 Path(base_dir).resolve() 并校验结果仍位于 base_dir 之内。

CongkeChen added 2 commits July 16, 2026 10:42

test: add intentional bad code for AI review pipeline

9f515bc

renew test script

9c3ed20

CongkeChen closed this Jul 16, 2026

CongkeChen reopened this Jul 16, 2026

CongkeChen closed this Jul 16, 2026

CongkeChen reopened this Jul 16, 2026

CongkeChen commented Jul 16, 2026

View reviewed changes

CongkeChen closed this Jul 16, 2026

CongkeChen reopened this Jul 16, 2026

CongkeChen commented Jul 16, 2026

View reviewed changes

CongkeChen closed this Jul 16, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Dev/internal pipeline#196

Dev/internal pipeline#196
CongkeChen wants to merge 2 commits into
internal_pipeline_testfrom
dev/internal_pipeline

CongkeChen commented Jul 16, 2026

Uh oh!

CongkeChen commented Jul 16, 2026

Uh oh!

CongkeChen commented Jul 16, 2026

Uh oh!

CongkeChen Jul 16, 2026

Uh oh!

CongkeChen Jul 16, 2026

Uh oh!

CongkeChen Jul 16, 2026

Uh oh!

CongkeChen Jul 16, 2026

Uh oh!

CongkeChen commented Jul 16, 2026

Uh oh!

CongkeChen Jul 16, 2026

Uh oh!

CongkeChen Jul 16, 2026

Uh oh!

CongkeChen Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		from typing import Any


		API_KEY = "sk-live-hardcoded-secret-do-not-use"

		return token == API_KEY and user == "admin"


		def run_user_command(cmd: str) -> str:

		return result.stdout


		def read_user_file(base_dir: str, relative_path: str) -> str:


		def authenticate(user: str, token: str) -> bool:
		return token == API_KEY and user == "admin"

		return f.read()


		def apply_discount(price: float, percent: float) -> float:

Uh oh!

Conversation

CongkeChen commented Jul 16, 2026

Uh oh!

CongkeChen commented Jul 16, 2026

AI Code Review

发现的问题

🚨 Critical

⚠️ Warning

💡 Suggestion

总结

测试建议

Uh oh!

CongkeChen commented Jul 16, 2026

AI Code Review

发现的问题

🚨 Critical

⚠️ Warning

💡 Suggestion

总结

测试建议

Uh oh!

CongkeChen Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

CongkeChen Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

CongkeChen Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

CongkeChen Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

CongkeChen commented Jul 16, 2026

AI Code Review

发现的问题

🚨 Critical

⚠️ Warning

总结

测试建议

Uh oh!

CongkeChen Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

CongkeChen Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

CongkeChen Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant