kra-new/docs/code-review-issues.md

91 lines
11 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.

# 代码审查问题清单internal + pkg
- 审查日期2026-08-27 ~ 2026-08-28共五轮全量审查。第五轮2026-08-28核查第四至七轮全部处置声称W/S/P/F/C/L/D 系列已修复与评估保留项)+ 全新视角回归审查;`go build ./...` 编译验证通过
- 文档结构:只保留待修复问题,按**类型**归类(不按轮次);每条标注发现轮次;已修复并经复查确认的、以及经评估决定保留的直接删除
- gva/ 目录是遗留参考库(独立 module 不参与 kra 编译),不在审查范围
- 依赖方向合规确认pkg 无 import internalintegration 不 import data/servicedata 不 import integration/payment无循环依赖
- 第五轮总评:核心业务路径(支付/调度/数据权限/上传)逻辑严谨、分层契约执行到位;历轮修复全部属实;当前债务已收敛为**敏感数据落地防护不对称、文档漂移、中转微文件**三类
---
## 一、安全与正确性缺陷(第五轮新发现,最优先)
| # | 问题 | 位置 | 说明 |
|---|------|------|------|
| ~~V-1~~ | ~~已清空 `configs/config.yaml` 中的 MySQL DSN、数据库密码和 JWT signing key模板仅保留 `KRA_*` 环境变量覆盖说明。泄漏值仍存在于历史提交,需在部署侧轮换数据库密码与 JWT 密钥。~~ | ~~configs/config.yaml:9,13,91~~ | ~~已完成2026-08-28~~ |
| ~~V-2~~ | ~~已让操作审计 Response 复用 `redactJSON`,与访问日志保持同一脱敏和长度上限策略,避免 token/密钥字段明文入库。~~ | ~~server/middleware/audit.gocapture.go~~ | ~~已完成2026-08-28针对性测试通过~~ |
| ~~V-3~~ | ~~已补齐 `key`、`public_cert`、`signing_key`、`secret_key` 等敏感键,并将 integration payment test 路由标记为 `BodyPolicyPaymentConfig`,请求体统一使用摘要策略。~~ | ~~server/middleware/redact.goroutecatalog/catalog.go~~ | ~~已完成2026-08-28针对性测试通过~~ |
| ~~V-4~~ | ~~已在 config watcher 与 data reload 入口补充运行时契约说明watch 只发布不可变快照,`reloadSystem` 才重建数据库/缓存/存储/集成客户端;保留双路径以维护回滚与连接退休安全。~~ | ~~internal/config/runtime.gointernal/data/config_store.go~~ | ~~已完成2026-08-28~~ |
| ~~V-5~~ | ~~已修正 ApplySync 失败文案为「同步失败」。~~ | ~~server/handler/api.go:195~~ | ~~已完成2026-08-28~~ |
| ~~V-6~~ | ~~已移除 audit handler 八处 `err.Error()` 拼接;客户端只收到稳定分类提示,内部错误仍由审计/日志链路保留。~~ | ~~server/handler/audit.go~~ | ~~已完成2026-08-28~~ |
| ~~V-7~~ | ~~已补齐秒传复制的 `Size`、`MD5`、`Mime`、`UserID` 字段,保持与完整上传记录一致。~~ | ~~biz/system/media_upload.go~~ | ~~已完成2026-08-28~~ |
| ~~V-8~~ | ~~`CompleteUploadSession` 失败时现在回退会话状态并返回错误,避免 media 已创建而 session 长期停留 `merging`;非关键分片清理仍保持 best-effort。~~ | ~~biz/system/media_upload.go~~ | ~~已完成2026-08-28~~ |
| ~~V-9~~ | ~~已逐项确认用户重名检查排除软删记录是有意的重新注册语义export 关系替换使用软删以保留审计历史;对软删记录更新匹配 0 行符合不可恢复约束;关联查询显式 `deleted_at IS NULL` 与 Model scope 叠加属于迁移期防御,并新增参数软删除更新回归测试。~~ | ~~data/system/*~~ | ~~评估完成保留2026-08-28~~ |
| ~~V-10~~ | ~~已删除外部退款分支中 `!accepted` 的不可达兜底,统一返回已确认的 `providerErr`。~~ | ~~biz/payment/payment.go~~ | ~~已完成2026-08-28~~ |
## 二、死代码残留(第五轮新扫描)
| 死代码 | 位置 | 证据 |
|--------|------|------|
| ~~PaymentOrderSourceRegistry.Len~~ | ~~已删除:全仓(含测试)零调用。~~ | ~~biz/payment/payment_order.go~~ |
| ~~PaymentFulfillmentRegistry.Len~~ | ~~已删除:全仓(含测试)零调用。~~ | ~~biz/payment/payment.go~~ |
| ~~NewConfiguredPaymentUsecase 恒 nil 错误透传壳~~ | ~~已删除并通过 Wire 重新生成ProviderSet 直接绑定 `NewPaymentUsecase`。~~ | ~~biz/payment/provider.gocmd/wire_gen.go~~ |
| ~~biz/integration mergeIntegrationDefaults 一行转发壳~~ | ~~已删除,调用点统一使用 `MergeIntegrationDefaults`。~~ | ~~biz/integration/integration_config.go~~ |
| ~~P-15 残留~~ | ~~已修正 handler/http.go 注释与 source.go 旧路径标记,并同步移除过时测试样本。~~ | ~~server/handler/http.gointernal/logging/source.go~~ |
| ~~`convertAuthority` 的 DeletedAt 值传递被 json:"-" 丢弃~~ | ~~已删除无效赋值,避免向已隐藏字段传递无效数据。~~ | ~~service/system/user_conversion.go~~ |
## 三、重复实现残留(历轮收敛后的漏网项)
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| ~~D-2 遗漏 1-3~~ | ~~已统一 qq、wechat v3 与 payment data 的支付方式归一化到 `paymentkit.NormalizePaymentMethod`,消除点号/短横线/空格处理漂移。~~ | ~~integration/payment/qq.gowechat_v3.godata/payment/payment.go~~ | ~~已完成2026-08-28针对性测试通过~~ |
| ~~D-31~~ | ~~已复用 `paymentOrderFingerprint` 完成 BeforeCreate 前后不可变校验,保留同等字段覆盖范围并删除 10 个 canonical 局部变量。~~ | ~~biz/payment/payment.go~~ | ~~已完成2026-08-28针对性测试通过~~ |
| ~~D-32~~ | ~~已新增 `BodyPolicyUpload` 并让 AccessLog 按 routecatalog 决定上传请求体限额,删除硬编码 URL 后缀判断。~~ | ~~routecatalog/catalog.goserver/middleware/access_log.go~~ | ~~已完成2026-08-28针对性测试通过~~ |
## 四、文档漂移(第五轮新发现,低成本高收益)
| # | 问题 | 位置 |
|---|------|------|
| ~~F-21~~ | ~~已将 CLAUDE.md 正文同步为 Gin+手写 DTO+make generate 的现行仓库约定,移除 proto/AIP/fieldmask 时代描述。~~ | ~~CLAUDE.md~~ | ~~已完成2026-08-28~~ |
| ~~F-22~~ | ~~已修正 pkg/README.md移除不存在的 logging/httpx/paymentkit 公共包描述。~~ | ~~pkg/README.md~~ | ~~已完成2026-08-28~~ |
| ~~F-23~~ | ~~已修正 internal/README.md 与 internal/server/README.md 的 httpx 路径。~~ | ~~internal/README.mdinternal/server/README.md~~ | ~~已完成2026-08-28~~ |
| ~~F-24~~ | ~~已将 integration README 的 paymentkit 路径改为 `internal/paymentkit`。~~ | ~~internal/integration/README.md~~ | ~~已完成2026-08-28~~ |
| ~~F-25~~ | ~~已修正 service README根包仅聚合 Wire ProviderSet不再声称 re-export 门面。~~ | ~~internal/service/README.md~~ | ~~已完成2026-08-28~~ |
| ~~F-26~~ | ~~已在 AGENTS.md 项目结构中补充 `docs/`。~~ | ~~AGENTS.md~~ | ~~已完成2026-08-28~~ |
## 五、过分拆分残留(微文件清单,行数实测)
| 文件 | 行数 | 说明 |
|---|---|---|
| ~~internal/modules/surface/surface.go~~ | ~~19~~ | ~~评估保留:隔离 routecatalog 到 pkg/module 的适配依赖。~~ |
| ~~internal/data/provider/provider.go~~ | ~~8~~ | ~~评估保留:`Database` seam 被多个 data 子包独立依赖。~~ |
| ~~internal/data/data_scope_record.go~~ | ~~9~~ | ~~评估保留:隔离 data 回调写模型与 system 查询模型。~~ |
| ~~internal/biz/task/task_registry.go~~ | ~~11~~ | ~~评估保留biz 只暴露窄注册接口,避免 worker/pkg/task 类型穿透。~~ |
| ~~internal/service/dto/{email,authentication,permission,system_init}.go~~ | ~~7/8/12/13~~ | ~~评估保留:按 transport 契约分文件,合并只减少文件数。~~ |
| ~~internal/server/handler/{session,navigation,set,http}.go~~ | ~~25/26/26/31~~ | ~~评估保留:单方法 handler 与响应词汇表由 Wire/路由注入约束形成。~~ |
| ~~internal/server/router 8 个单域注册微文件~~ | ~~12-19~~ | ~~评估保留:领域路由注册边界清晰,统一合并会扩大单文件变更面。~~ |
Wire ProviderSet 微文件(约 10 个 ≤9 行)属 Wire 惯例不计债务biz/system 微文件群、SystemConfigService、provider/providers 双文件等已在历轮合并完成。
## 审查后认为合理、不建议改动的部分(历轮评估保留决策汇总)
- **biz 注入面**RegisterBusinessModule/PaymentBusinessModule 是 docs/PAYMENT.md:121-129 明文声明的业务接入契约模板无生产实现属预期PayInternal/RefundInternal/AuthorizeRefund 被 biz 调用链消费
- **F-2/F-4/F-6/F-7/F-9/F-10/F-11**result.go 自有协议语义非纯转发handler/http.go 别名层task 双 usecase 生命周期不同initialize 三层是启动编排倒置链system usecase 壳是 Wire 契约边界security_session 剩余 6 方法跨三层消费biz 接口嵌入是组合窄能力
- **S-5/S-6/S-7/S-8/S-9**payment 大文件承载跨供应商编排dto 跨域文件移动放大契约变化data 转换命名差异需全域迁移media 四文件职责边界清晰;单方法 handler 由 Wire 注入约束
- **D-4/D-6/D-9/D-10/D-11/D-19**handler 样板各域差异明显mq helper 属排除范围config/runtimeconfig 分离已补决策注释;三处清理入口不同各自独立触发;树算法输入模型不同;金额守恒四处输入形态互异(完整结构体/配置化 JSON/XML 值映射/SDK 结构体)
- **L-5/L-10/L-12**错误审计白名单被测试锁定Kratos errors 仅跨层/stdlib 管内部的双体系分层binding 覆盖结构必填、手工覆盖上下文(覆盖不均但新代码倾向 binding未恶化
- **P-3/P-5/P-6/P-7/P-8/P-9/P-10/P-11/P-13**pkg 各包定位经复核成立mq/websocket 属既定排除
- **X-1~X-4/X-6~X-8/X-10/X-11**:路由双声明/集成配置双通道/热重载分工watchLoop 发布快照 vs reloadConfig 重建基础设施,合并需重做锁与退休策略)等均完成影响分析保留
- **C-2/C-4/C-6/C-8**loadUser/loadUsers 查询策略不同;*Data nil 防御覆盖测试替身边界媒体三重限制各守一层PaymentLogger 是审计替换 seam
- **L-7 复核**data 层 Table() 已全 PO 化(仅 2 处导出动态表名合理保留saveRelations 只做 DO→PO 转换
- **质量标杆**第五轮正面确认payment 幂等指纹+回调强制平台查单、退款 lease 语义、task_scheduler 锁序、task_executor SSRF 拨号防护+orphan 跟踪、auth singleflightcontext.WithoutCancel 隔离取消传染、data_scope 回调注入、email CRLF 清洗、ListTasks(0,0) 全量语义、新增缝PaymentAdapterFactory/MergeRuntimeConfig/deletePrefixViaList/AST 缓存/systeminfo均内聚无越界
## 处置建议(按优先级)
1. **立即处理 V-1**(泄漏的数据库密码)→ **V-2/V-3**(审计敏感数据落地防护对称化)
2. **V-4/V-9**(热重载漂移文档化、软删语义逐项确认+补测试)
3. **V-5/V-6/V-7/V-8/V-10 + 二节死代码**(小而具体的清理批次)
4. **三节 D-2 三处遗漏 + D-31/D-32**
5. **四节文档漂移群**(一次 README/CLAUDE.md 同步批)
6. **五节微文件合并**(最后)