Files
gaokao-volunteer-system/reports/PROJECT_SYSTEM_REVIEW_2026-06-14.md
Hermes Agent b3ac495f36
Some checks failed
CI / pytest (Python 3.10) (push) Has been cancelled
CI / pytest (Python 3.11) (push) Has been cancelled
CI / pytest (Python 3.12) (push) Has been cancelled
docs(truth): T0-02 mark historical review snapshots
2026-07-05 21:46:24 +08:00

507 lines
17 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
<!-- 历史快照提示:本文件保留为历史审计材料,不再代表当前项目状态。 -->
> ⚠️ **历史快照(非当前真相源)**:本文件仅保留当时审查/收口结论。当前状态与执行顺序以 `docs/CURRENT_STATE.md`、`docs/ACTIVE_REMEDIATION_2026-07-05_REVIEW.md`、`docs/ACTIVE_EXECUTION_BOARD_2026-07-05_REVIEW_REMEDIATION.md`、`docs/plans/2026-07-05-review-remediation-systemic-fix-plan.md` 和 `reports/REVIEW_REPORT_2026-07-05_COMPREHENSIVE_PROJECT_REVIEW.md` 为准。
---
# gaokao-volunteer-system 系统性 Review 报告2026-06-14
> 历史快照2026-06-14。当前真相源请优先阅读`docs/CURRENT_STATE.md` → `docs/ACTIVE_EXECUTION_BOARD_2026-06-13.md` → `docs/ACTIVE_REMEDIATION_2026-06-13.md` → `docs/P0_P1_P2_REMEDIATION_PLAN_2026-06-14.md`。
**评审对象**: `/home/long/project/gaokao-volunteer-system`
**评审方式**: 当前文档真相核对 + 关键源码结构审查 + 安全/测试/运维专项审查 + 本地门禁复核
**评审标准**: 严格标准;以"真实业务闭环、数据安全、验证可信度、可运维性"为主轴
**当前真相源**: `docs/CURRENT_STATE.md`
---
## 1. 结论
**结论**:项目已经具备较完整的“运营后台 + 订单 + 支付抽象 + portal + 分享 + 渠道同步 + 交付事件”骨架,核心订单状态机与 DAO 边界也明显优于很多同规模项目;但按严格标准看,**系统仍未达到“关键链路一致性可靠、支付/删除/交付语义闭环、质量门禁可信、灾备与部署可验证”的稳态**。
**总体评级****有明显工程基础,但不建议把当前版本描述为“已完成的、可放心放量的商业闭环系统”**。
**最重要判断**
1. **订单/支付/退款/交付**仍有多处语义裂缝,主系统真相源不统一。
2. **隐私删除/匿名化**与文档承诺不一致,存在误报“已匿名化/已删除”的合规风险。
3. **CI / 本地 / coverage gate / Docker / 备份恢复**没有形成一致、可复现、可证明的验证链。
4. **支付回调与分享公开面**仍有显著安全收敛空间。
本次未发现必须立即停机级别的 **P0** 远程利用证据,但存在多项 **P1**,足以阻止“系统已成熟闭环”的结论。
---
## 2. 评审范围
### 2.1 文档与约束
- `README.md`
- `docs/CURRENT_STATE.md`
- `docs/LEGAL_PRIVACY_BASELINE.md`
- `docs/DATA_RETENTION_AND_DELETION.md`
- `docs/BACKUP_AND_RECOVERY_PLAN.md`
- `docs/DELIVERY_SERVICE_DESIGN.md`
- `docker-compose.yml`
- `Dockerfile`
- `.github/workflows/ci.yml`
- `codecov.yml`
- `pytest.ini`
- `scripts/dev-verify.sh`
- `scripts/backup_verify.sh`
- `scripts/check_coverage_gate.py`
### 2.2 核心代码链路
- `admin/app.py`
- `admin/auth.py`
- `admin/config.py`
- `admin/password.py`
- `admin/db.py`
- `admin/routes/auth.py`
- `admin/routes/orders.py`
- `admin/routes/web_public.py`
- `data/orders/dao.py`
- `data/orders/state_machine.py`
- `data/orders/public_flow.py`
- `data/orders/deletion_service.py`
- `data/orders/intake_store.py`
- `data/orders/crypto.py`
- `data/orders/masking.py`
- `data/payments/service.py`
- `data/payments/dao.py`
- `data/payments/providers/alipay.py`
- `data/payments/providers/alipay_sim.py`
- `data/channel_sync/webhook_server.py`
- `data/share/permission.py`
- `data/customer_portal/token.py`
- `data/notifications/dispatcher.py`
### 2.3 测试与验证样本
- `admin/tests/test_web_public_alipay_sim_e2e.py`
- `admin/tests/test_order_status_page.py`
- `admin/tests/test_routes_orders.py`
- `data/payments/tests/test_webhook.py`
- `tests/test_delivery_dispatcher.py`
- `tests/test_retention_cleanup.py`
- `tests/test_t5_performance.py`
---
## 3. 严重级别定义
- **P0**: 已存在可直接导致系统失控/严重数据泄漏/资金错误且缺乏现实缓解
- **P1**: 高优先级问题;会破坏主链路一致性、支付/隐私/恢复可信度,阻止成熟上线结论
- **P2**: 中优先级问题;不会立即击穿主链,但会持续积累运维、测试或安全债务
- **P3**: 正向亮点或低优先改进项
---
## 4. 关键问题清单
## 4.1 P1 问题
### P1-1 支付回调存在双写裂缝payment 已记 paid但 order 可能未推进
**证据**
- `data/payments/service.py::PaymentService.handle_webhook()` 先更新 `payments.status=paid`
- `data/payments/dao.py::PaymentDAO.update_status()` 内部立即 `commit`
- 随后才在新的 `OrdersDAO` 上下文里把订单从 `pending` 推到 `paid`
**风险**
这是典型跨表双写一致性问题。任一时刻如果支付表提交成功、订单推进失败(订单异常、锁冲突、状态先被别处推进、进程中断),系统会留下:
- `payments.status = paid`
- `orders.status != paid`
之后 portal、退款、统计、人工判断都会基于不同真相源工作。
**结论**
这是当前最重要的业务一致性问题之一。
---
### P1-2 删除/匿名化与文档承诺不一致,存在“看起来已删,实际上没删”的合规风险
**证据**
- `docs/DATA_RETENTION_AND_DELETION.md` 要求订单、资料提交、报告文件、通知/支付审计都应清理或匿名化
- `data/orders/deletion_service.py::OrderDeletionService.anonymize_order()` 仅更新 `orders` 主表部分字段
- 未覆盖:
- `data/orders/intake_store.py``order_intakes.payload_json`
- `data/payments/dao.py``payments.callback_payload`
- `audit_report` / `pdf_path` 对应文件
**风险**
系统当前容易把“订单主表去标识化”误报成“订单数据已匿名化完成”。对于高考场景中的未成年人/监护人资料,这是明显不够的。
**结论**
当前删除/匿名化能力不足以支撑强隐私合规表述。
---
### P1-3 真实支付宝回调校验过弱,只验签 + 金额,不校验应用/商户/状态边界
**证据**
- `admin/routes/web_public.py``/api/public/payments/alipay/notify` 直接把表单解析结果交给 `PaymentService.handle_webhook()`
- `data/payments/service.py::handle_webhook()` 只校验:签名、`payment_id`、金额一致
- `data/payments/providers/alipay.py` 仅标准化 `out_trade_no/trade_no/total_amount/trade_status` 并做 RSA 验签
- 未见对 `app_id`、商户标识、允许状态白名单、`notify_id``gmt_payment` 等做二次校验
**风险**
系统会把“签名有效且金额匹配”的回调近似当成“业务上确认为成功支付”。这对真实支付系统不够。
**结论**
真实支付回调仍未达到严格上线标准。
---
### P1-4 Webhook server 的数据库连接缓存按模块全局单例保存,且不区分 `db_path`
**证据**
- `data/channel_sync/webhook_server.py``_DB_CONN` 为模块级单例
- `_get_db(db_path)` 只在 `_DB_CONN is None` 时首次打开连接;后续不同 `db_path` 直接复用旧连接
- `make_server(..., db_path=...)` 虽允许传不同库路径,但连接缓存不会切换
**风险**
同一进程先后启动多个 webhook server / 测试实例 / 不同 DB 场景时,后者可能静默写入前一个库。问题隐蔽,影响审计、幂等和订单入库正确性。
**结论**
这是高风险的状态污染问题。
---
### P1-5 退款域模型未闭环:状态机有 `refunded`,支付服务只把 payment 记到 `refund_pending`
**证据**
- `data/orders/state_machine.py` 支持订单进入 `refunded` 终态
- `data/payments/service.py::request_refund()` 仅把 payment 更新到 `refund_pending`
- 未见生产调用路径把订单主状态同步推进到 `refunded`
- `admin/routes/web_public.py` portal 阶段判断优先看 payment 的退款状态,而不是订单主状态
**风险**
订单域与支付域的退款真相源分裂。之后交付限制、统计、保留期清理、人工判断都可能不一致。
**结论**
退款闭环目前不完整。
---
### P1-6 公开分享 `edit/admin` 权限是 allow-by-default未来新增字段极易外泄
**证据**
- `data/share/permission.py``edit/admin``visible_fields=None`
- `render_report_payload()` 在该模式下除 denylist 外默认全部透传
- `_ALWAYS_HIDDEN_FIELDS` 仅覆盖少数内部字段,不覆盖未来可能新增的手机号、身份证、顾问备注等
**风险**
一旦报告 payload 新增敏感字段,公开分享链接可能自动带出这些信息,而不需要任何显式审批。
**结论**
分享策略对未来演进不够安全。
---
### P1-7 覆盖率门禁口径互相冲突,验证链本身不可信
**证据**
- `.github/workflows/ci.yml``--cov-fail-under=60`
- `scripts/dev-verify.sh``--cov-fail-under=80`
- `scripts/check_coverage_gate.py``overall=80%``core=100%`
- `codecov.yml`:也声明整体 80%、核心 100%
**风险**
同一改动可能出现:
- CI 通过
- 本地失败
- codecov 标准不同
- gate 脚本又给出另一套结论
这种门禁不稳定,会持续削弱团队对测试结论的信任。
**结论**
当前质量门禁存在制度级不一致。
---
### P1-8 备份恢复只做到“复制文件 + 打印表名”,还不是恢复演练
**证据**
- `docs/BACKUP_AND_RECOVERY_PLAN.md` 自认仅是“最小恢复基线”,并明确尚无自动化恢复验证
- `scripts/backup_verify.sh` 只复制 DB / 文件,然后验证 SQLite 文件可打开、文件可枚举
- 未见 CI 或自动化测试真正执行“恢复后启动应用并验证 portal / 订单 / 报告”
**风险**
当前只能证明“文件能拷贝”,不能证明“系统能恢复并可用”。这对任何需要对外交付或数据保留承诺的系统都不够。
**结论**
灾备能力目前停留在文档与文件级验证,不是服务级验证。
---
## 4.2 P2 问题
### P2-1 公共下单先写订单、后建支付,失败时会遗留孤儿 `pending` 订单
**证据**
- `admin/routes/web_public.py::create_public_order_endpoint()``create_public_order()`,再 `PaymentService.create_checkout()`
- `data/orders/public_flow.py` 直接先把订单写入库
- `data/payments/service.py::create_checkout()` 再独立创建 payment
**风险**
支付初始化失败时,用户请求失败,但库里已经有订单,且没有补偿动作。
---
### P2-2 渠道同步仍在走已声明被替代的 `dao_extension`,存在双实现漂移风险
**证据**
- `data/orders/dao.py` 已提供 `OrdersDAO.upsert_by_external_id()`
- `data/channel_sync/webhook_server.py``data/channel_sync/poller.py` 仍直接使用 `data.channel_sync.dao_extension.upsert_by_external_id`
**风险**
同一业务规则同时存在两套实现,后续修改很容易只修一套。
---
### P2-3 交付执行器把“文件存在”直接标记为 `sent`,语义过度乐观
**证据**
- `data/notifications/dispatcher.py::dispatch_ready_events()` 校验文件存在后直接 `mark_sent`
- `docs/DELIVERY_SERVICE_DESIGN.md` 又明确说明“邮件/渠道真实发送执行器”尚未完成
**风险**
`sent` 更像“已投递成功”,但当前实际含义只是“本地文件通过检查”。这会误导运营状态与告警语义。
---
### P2-4 Portal token 与后台 JWT 共用同一根 secret
**证据**
- `admin/routes/web_public.py` 创建和校验 portal token 时使用 `settings.jwt_secret`
- `data/customer_portal/token.py` 是独立 HMAC token 格式
- `admin/auth.py` 后台 JWT 也使用同一 secret
**风险**
轮换、隔离、泄露影响面都被耦合,不符合边界最小化原则。
---
### P2-5 payment webhook secret 仍有固定开发默认值,启动阶段也没有 prod fail-closed 校验
**证据**
- `admin/config.py` 默认 `GAOKAO_PAYMENT_WEBHOOK_SECRET=dev-mock-payment-secret`
- `data/payments/service.py::for_db()` 也带同样默认值
- `admin/app.py` 启动只校验 JWT secret 与默认管理员密码,没有校验 payment webhook secret
**风险**
在 mock / alipay_sim 场景中,如果配置漂移到公网环境,伪造回调成本会明显降低。
---
### P2-6 pytest 把 `data/` 直接纳入测试发现根,测试边界偏混杂
**证据**
- `pytest.ini`: `testpaths = admin/tests tests data`
- CI 也直接跑 `pytest admin/tests tests data`
**风险**
业务代码目录与测试入口绑定过紧,不利于区分 unit / integration / e2e / script verification 的语义。
---
### P2-7 部署链只有单容器存活探测,没有业务冒烟、恢复验证、监控告警闭环
**证据**
- `docker-compose.yml` 仅定义一个服务、一个数据卷、一个 `/health` healthcheck
- `Dockerfile` 仅安装运行依赖并直接启动 `admin.app`
- 文档里有监控设计,但未见对应自动化验证或可运行集成
**风险**
当前最多只能说明“进程活着”,不能说明“关键业务链在部署后可用”。
---
## 4.3 P3 亮点
### P3-1 OrdersDAO 把状态机、时间戳、历史记录、交付触发收敛到一条主写路径
**证据**
- `data/orders/dao.py::transition_status()` 单事务处理状态校验、状态更新、history、时间戳
- `delivered` 且交付物齐备时会顺带落 `delivery_notifications`
**评价**
这是当前仓库最稳的边界之一。它明显优于把状态历史散落在各个 route/service 中维护。
---
### P3-2 FastAPI app 装配较整洁,配置校验与 schema 初始化集中在启动期
**证据**
- `admin/app.py::create_app()` 结构清晰
- `_validate_and_log_settings()` 对 JWT / 默认管理员密码做环境分级处理
- `_setup_database()` 统一做 schema/bootstrap
**评价**
应用装配层没有明显失控,是后续继续治理的好基础。
---
### P3-3 管理员密码与分享密码使用 PBKDF2 + 随机盐 + 恒时比较
**证据**
- `admin/password.py`
- `data/share/short_link.py`
**评价**
密码存储方案比很多轻量项目更稳,属于正向设计。
---
### P3-4 登录失败统一口径,且已有基础限流
**证据**
- `admin/routes/auth.py``username@client_ip` 做失败计数与 `retry_after`
- 普通错误统一返回 `AUTH_INVALID_CREDENTIALS`
**评价**
已具备最低限度的暴力破解缓解与账户枚举抑制。
---
## 5. 验证与复核结果
### 5.1 命令复核
在仓库根执行了以下检查:
```bash
python3 -m pytest admin/tests tests data -q
python3 -m ruff check . --exclude .venv,.worktrees
python3 -m mypy .
python3 scripts/check_coverage_gate.py coverage.xml
./.venv/bin/python -m pytest admin/tests tests data -q
./.venv/bin/python -m ruff check . --exclude .venv,.worktrees
./.venv/bin/python -m mypy .
./.venv/bin/python scripts/check_coverage_gate.py coverage.xml
```
### 5.2 观察到的结果
1. **系统 Python 环境**缺少 `pytest / ruff / mypy`,全局命令不可直接复现。
2. **`.venv` 环境下**
- `pytest`: **639 passed, 5 failed**
- 失败主要来自:
- `tests/test_delivery_dispatcher.py`
- `tests/test_retention_cleanup.py`
- `tests/test_t5_performance.py`
3. 失败原因并非随机:
- 若测试通过 `subprocess.run(["python3", ...])` 启动脚本,脚本会落到系统 Python随后因 `admin.__init__ -> admin.app -> import uvicorn``ModuleNotFoundError: uvicorn`
- `locust` 可执行文件缺失,导致性能测试无法启动
4. `ruff check`**通过**
5. `mypy`:未见硬错误输出,但存在 **8 条 `annotation-unchecked` 提示**,集中在测试文件
6. `scripts/check_coverage_gate.py coverage.xml`**失败**,报 `missing core coverage entries`,说明当前 coverage artifact 与 gate 脚本的核心文件映射未对齐。
### 5.3 这组结果说明什么
- 项目不是“完全不可测”,因为大部分测试可在 `.venv` 中通过。
- 但它也**不是稳定可复现的验证闭环**
- 脚本子进程环境与测试 runner 环境脱节
- 性能验证依赖工具未稳定声明/注入
- coverage gate 与覆盖率产物不一致
这与前述 P1/P2 结论一致:**系统已有工程基础,但门禁可信度仍不够强。**
---
## 6. 优先级建议
## 第一优先级(立即处理)
1. **修复支付 webhook 的双写一致性问题**
2. **补齐删除/匿名化的真实覆盖范围,统一 delete vs anonymize 语义**
3. **补强真实支付宝回调业务校验**
4. **修复 webhook server 的全局 DB 连接污染问题**
5. **统一 coverage / CI / 本地 gate 口径**
## 第二优先级(短期完成)
6. **完成退款主状态闭环**
7. **把分享权限改为 allowlist而不是 edit/admin 全量透传**
8. **拆分 portal secret 与 admin JWT secret**
9. **为 payment webhook secret 增加生产环境启动门禁**
10. **把渠道同步彻底 cutover 到 `OrdersDAO.upsert_by_external_id()`**
## 第三优先级(中期治理)
11. **把交付状态拆成“已验证可投递”与“真实已发送”**
12. **补真实 restore drill / Docker 业务冒烟 / 监控告警最小闭环**
13. **收紧 pytest 入口,明确 unit/integration/e2e 语义边界**
---
## 7. 最终判断
如果问题是:
> 这个仓库有没有认真做工程?
答案是:**有,而且部分核心边界(尤其 orders DAO / 状态机)做得不错。**
如果问题是:
> 这个系统是否已经达到“关键闭环可信、合规表述稳妥、验证链稳定、运维可托底”的严格标准?
答案是:**还没有。**
更准确的表述应当是:
> 这是一个已经具备明显产品化骨架、但仍处于“关键一致性 / 支付安全 / 删除合规 / 验证闭环 / 灾备落地”强化期的系统。当前适合继续收敛与修复,不适合对外宣称为完全成熟闭环。