Files
kra-oa/docs/code-review-issues.md
T
2026-08-28 16:37:53 +08:00

118 lines
19 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):核查第九轮划线项(18 项)+ 全新视角回归审查;`go build ./...` 编译验证通过
- 文档结构:只保留待修复问题,按**类型**归类;每条标注发现轮次;已修复并经复查确认的、经评估保留的、用户决策不修的直接删除
- gva/ 目录是遗留参考库(独立 module 不参与 kra 编译),不在审查范围
- 依赖方向合规:pkg 无 import internal;integration 不 import data/service;data 不 import integration/payment;无循环依赖
- 第十轮总评:主干(支付幂等、哨兵回填、审计管道、路由目录)结构扎实;但"竞态保护/孤儿清理/指纹统一"三组修复各留保护边界缺口,物理删除化在注释与死字段上留下碎屑;三条机制线各自"半闭环":净化只管 msg 不管 data、ctx 键一半前置一半后置、脱敏键表三套规则并存
---
## 一、安全与正确性缺陷(最高优先级)
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| V-15 | **错误泄露收敛(envelope 净化)残留四个洞**:① **"失败:"截断只匹配半角冒号**——media.go:188 与 menu.go:65 用全角":"拼接绕过;② **17 处截断不可命中的调用漏网**(无冒号紧贴 15 处:api.go:137,156、authority.go 9 处、menu.go:170,178,198、user.go:150;全角 2 处)——非黑名单底层错误(x509/驱动方言)原样透出;③ **payment TestProvider data 字段绕过**——data/payment/payment.go:79-139 各阶段仍把原始 err.Error() 写入 Stages[].Message,而 Write 只净化 msg、**CodeError 的 data 通道完全无净化机制**(:87 是当前唯一实例但机制敞开);④ **净化静默无日志**——response.go 无日志导入,替换发生无记录("不泄露也不可排障")。另有误伤:黑名单含 provider/sdk/connection/timeout 等宽泛词会误伤合法业务消息;约 45 处半角冒号调用的 biz 可读提示被截断抹平。净化逻辑零测试 | server/httpx/response.go:32-57;data/payment/payment.go:79-139;handler/payment.go:87 | 六轮发现,十轮复核部分修复(白名单空操作已修、record not found 已补)+四洞 |
| V-32 | **(新发现,高)删除审计日志的接口自身无审计标记(清痕无痕)**:`DELETE /sysOperationRecord/deleteSysOperationRecord` 与 `deleteSysOperationRecordByIds`(catalog.go:59-60)均无 audit:true(同表其他删除接口都有);`POST /attachmentCategory/deleteCategory`(:138)同样缺失——删除审计记录本身是典型清痕操作,审计矩阵修复的遗漏点 | routecatalog/catalog.go:59-60,138 | 十轮 |
| V-33 | **(新发现,中高)支付指纹严格相等校验对存量订单幂等重放的冲突风险**:biz/payment/payment.go:652 `order.RequestFingerprint != paymentOrderFingerprint(req, extra)` 严格相等、无算法版本/迁移字段——Z-3 指纹统一前存量 initialized/pending 订单若由旧算法(data 层整结构 marshal)写入指纹,客户端超时重放同 TradeNo 将报"支付订单参数冲突"而非幂等短路,**存量未支付订单无法重新拉起支付**。建议对 initialized 态提供指纹重算/兼容窗口 | biz/payment/payment.go:645-657;data/payment/payment_order.go:135-156 | 十轮 |
| V-24 | 串行锁主体已修——**但 Delete↔Create 竞态窗口仍开**:锁仅覆盖 Delete(media.go:126-143),引用同一 Key 的写入路径均不持锁(秒传 InitUpload→CreateMedia、普通 Upload 引用计数+建记录、CompleteUpload 的 CreateMedia)——T1 Delete 读 count=1 判定删除、T2 秒传 CreateMedia 复制同 Key 落库、T1 files.Delete → T2 新记录指向已删文件。锁为进程级 sync.Mutex,多副本部署跨进程无效 | biz/system/media.go:50,111-143;media_upload.go:100-107,262 | 八轮发现,十轮复核主体修复+Create 路径缺口 |
| V-34 | **(新发现,中)CleanupStale 先删 DB 后删文件且吞错 → 存储孤儿无重试路径**:biz/media_upload.go:295-299 先 `DeleteUploadData`(事务内物理删 session+chunks)再 `files.DeletePrefix`(_ = 吞错)——存储删除失败时 session 行已物理删除,**下轮扫描永远不再命中**,孤儿分片对象永久滞留(无孤儿反向扫描器) | biz/system/media_upload.go:295-299 | 十轮 |
| V-35 | **(新发现,中)TaskScheduler 启动 Reload 失败仅 Warn 无重试**:worker/task_scheduler.go:93-95——DB 启动期闪断 → 任务表加载失败 → 调度器空转且不重试(Start 阻塞在 :99 `<-runContext.Done()`),**定时任务全部静默丢失**直至人工 reloadSystem | worker/task_scheduler.go:93-99 | 十轮 |
| V-19 | **证书别名缺口未修(三层全缺)**:`*_content`/`*_path` 别名(app_cert_content/root_cert_content/pkcs12_content/cert_content/key_content/cert_path 等)不命中 IsIntegrationSecretKey(不含 secret 词根、后缀非 _cert/_key)、不在 definition.Fields → data/service/middleware 三层键表均不掩码,API 直连提交明文落库并回显;connectivity restoreMaskedSecrets 同样不识别(掩码值被当真值测试连接)。误伤项(routing_key/key_id)已消除(键表无裸 _key 项)。**修法:键表补别名一处三层受益** | biz/integration/integration_config.go:325-328,423-431,398-409 | 六轮发现,十轮复核未修 |
| V-13b | swagger 残留:① **空 env fail-open 未修**(gin.go:68 `env == ""` 仍注册,生产漏配 env 即暴露);② **双清单判定不一致**——gin.go 白名单 {空,development,dev,test,local} vs main.go:107-110 黑名单 {production,prod,live,staging},自定义 env(qa/uat)时路由不注册但启动日志仍打印 swagger URL(:171 日志谎报);③ env 门禁零测试 | server/gin.go:64-70;cmd/main.go:105-126,171 | 七轮发现,十轮复核未修 |
| V-25 | merging 阈值区分✓——残留:合并超 TTL+1h 会话被物理回收后 CompleteUploadSession Updates 影响 0 行无 RowsAffected 校验 → media 已建但会话记录丢失、秒传断链无错误暴露 | data/system/media_upload.go:92-94,121-125 | 八轮发现,残留 |
| V-36 | **(新发现,低中)ErrorAudit 客户端失败词表含 "token" 会吞 5xx**:无 privateErrors 且 msg 含 "token" 的服务端错误(如 "token 生成失败: redis connection refused")被判为预期客户端失败跳过 sys_error 记录——服务端故障静默 | server/middleware/error_audit.go:73,43-45 | 十轮 |
| V-37 | **(新发现,低)分片上传秒传复制丢失 CategoryID**:media_upload.go:100 copy 未带 CategoryID(InitUpload 签名无 category 参数,单文件 Upload 支持 media.go:68)——分片上传的媒体记录分类恒为默认值,功能不对齐 | biz/system/media_upload.go:87,100 vs media.go:68 | 十轮 |
| V-27 | database_list 按别名回退已修✓(byName 映射+无按下标)、空密码不掩码✓——**蟑螂定律残留**:① **redis_list 仍按下标回退**(configuration.go:439-441 `previous == nil && index < len(current)`)——重排/头部插入时掩码条目继承旧列表同位置密码;② 别名匹配失败(改别名/新增条目/存量空 AliasName)时哨兵字面量直接落盘(data 层重建 DSN 密码变 `******` 连接失败)——无哨兵落盘拒绝校验 | internal/initialize/configuration.go:427-446,414-417 | 九轮发现,十轮复核部分修复+redis_list 残留 |
| V-30 | AlipayV3/WechatV2 required 已对齐✓——**douyin platform_serial_no 仍 required=false**(definition.go:167)而校验器启用时强制(integration_config.go:344-347),前端不标必填、保存时才报错;同构的 WechatV2 client_cert 标了 true,两处标准不一致 | biz/integration/integration_config_definition.go:167 | 九轮发现,十轮复核部分修复 |
| V-20 | 前端 token 拼入 URL(完整登录 JWT 拼二维码 URL,账户主凭证泄露面;无轮换机制) | web/src/components/upload/QR-code.vue:55 | 六轮,未修复 |
| V-16b | 词表与八轮一致——applet 仅 wechat_v2/v3/douyin 有;allinpay 无 jsapi/mini 分支;测试缺口仍在 | integration/payment 各渠道文件 | 七轮,未修复 |
| V-38 | (新发现,低)AGENTS.md 错误契约 vs 媒体模块实践:biz "typed errors(errors.NotFound/BadRequest)"约定下,media.go:52 ErrMediaTooLarge、media_upload.go:21 ErrUploadSessionNotFound 为裸 errors.New(同包 errors.go:9 ErrMediaNotFound 是规范风格),前端无法区分错误类别 | biz/system/media.go:52、media_upload.go:21 | 十轮 |
| V-31 残留 | FindMedia 已映射 ErrMediaNotFound✓——同包 UpdateMediaName(media.go:125-127)仍透传 gorm 原文(404 语义丢失,靠黑名单兜底为"操作失败") | data/system/media.go:125-127 | 九轮,部分修复 |
## 二、死代码与碎屑
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| Z-18 | **(新发现,中)DeleteUploadSession 注释与代码直接矛盾**:注释写 "retaining the soft-deleted session for audit/recovery"(软删保留审计),代码实为 Unscoped().Delete 物理删除——V-23 修复后未同步的过时注释,直接误导维护者(媒体记录同理 media.go:130-132) | data/system/media_upload.go:95-98;data/system/media.go:130-132 | 十轮 |
| Z-15 | 软删字段死重量:mediaPO/categoryPO/uploadChunkPO/uploadSessionPO 的 DeletedAt+索引在全部删除路径改 Unscoped 后成死配置(每次默认查询仍附加 deleted_at IS NULL、索引仍写入);**连带**:UpsertChunk 的 DoUpdates 仍重置 deleted_at 列(复活分支不可达,:104);历史软删行无迁移清理(修复前取消产生的存量软删行永不回收) | data/system/media.go:16,34;media_upload.go:17,35,104 | 九轮,未修复 |
| Z-9 | V-11 修复后死通路仍在:audit.go:76 ctxRespTextKey 分支生产链路不可达(后置键);:83 ctxRespTruncatedKey 同为死读——buffer 共享引用已成实际通路(:78-81),显式标志语义丢失;access_log.go:114-117 仍先对截断体完整 redactJSON 再覆盖占位符(白做功) | server/middleware/audit.go:76-85;access_log.go:110-121,114-117 | 八轮,十轮复核仍在 |
| Z-6 | 测试缺口群(十轮更新):sanitizeFailureMessage 分类逻辑(response_test 仅测透传分支);洋葱 ctx 链/truncated 传递;swagger env 门禁;service/integration 目录零测试(键表行为无防护);词表用例;OperationAudit 端到端;NormalizePaymentMethod 单测;CORS allow-all | 各处 | 六轮发现,十轮扩展 |
| Z-2 | 两个 migration step 重复播种同一条 test API | data/system/migrations.go:88,145-153 | 六轮,未修复 |
| Z-11 | biz media Upload 死逻辑(MediaKeyReferences 对新 uuid key 恒 0,count==0 分支永真,埋雷) | biz/system/media.go:107-117 | 八轮,未修复 |
| Z-13 残留 | rate_limit 魔法数字已修✓——cors.go:13-14 逗号空格不一致、audit.go:104 单行 13 字段调用、audit.go:46-47 空 else-if 三项未修 | server/middleware/cors.go:13-14、audit.go:46-47,104 | 八轮,部分修复 |
| Z-16 | BodyPolicyIntegrationConfig 中转标记(catalog 内声明→标记→运行时翻译回 payment_config/默认)——已补设计注释(:424-425)但中转设计未变 | routecatalog/catalog.go | 九轮,未修复 |
| Z-17 | scanUpload.vue:115-116 空 `.catch((err) => {})` 吞路由就绪异常 | web/src/view/media/scanUpload.vue:115-116 | 九轮,未修复 |
| Z-19 | (新发现)前端全局 console.log 残留 7 处(Z-10 只清了目标文件):pdf.vue:20/34/37、version.vue:566、sysDictionaryDetail.vue:329、global.js:55、image.js:35 | web/src 各处 | 十轮 |
| Z-20 | (新发现)错误串清洗三种方式:error_audit.go:19 TrimSpace、access_log.go:134 TrimRight("\n")、audit.go:86 不清洗——同一 c.Errors 串三种处理 | server/middleware/ | 十轮 |
| Z-21 | (新发现)同请求重复 Lookup:access_log.go:34 与 :86 对同一请求两次调 BodyPolicyFor(可复用 :34 结果) | server/middleware/access_log.go:34,86 | 十轮 |
## 三、重复实现 / 双轨残留
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| Y-7 | 掩码三套实现+键表三层漂移:data 层(Secret∪IsIntegrationSecretKey,递归含数组✓)+ service 层(仅 IsIntegrationSecretKey,**不认 definition.Secret**——靠 data 层先行掩码偶然自洽)+ middleware/redact.go 第三套(归一化规则不同:删分隔符 vs `-`→`_`);哨兵双轨:config.MaskedSecret="******" 与 middleware redactedValue="***"(前端若把日志侧 *** 回填会被当真值保存);IsIntegrationSecretKey platform_cert/root_cert 仍被 cert 后缀覆盖(冗余) | data/integration:225-245;service/integration:69-96;redact.go:18-27,6 | 九轮,十轮复核部分(递归)修复 |
| Y-5 | 集成配置校验双重执行:biz Save 对哨兵替换后值校验("******"非空总通过,半失效)+ data 层 merge 后再校验兜底——biz 层形同虚设 | biz/integration:122-129;data/integration:160-180 | 八轮,未修复 |
| Y-2 剩余 | ctxRespTextKey 共享✓(但见 Z-9 死通路)——剩余:capture.go:49-76 JSON 成功路径仍先 unmarshal+mask+marshal 再判超限(超长白做);error_audit.go:37 第三次 unmarshal | middleware/capture.go:49-76 | 六轮,部分修复 |
| Y-10 | (新发现)前端两套动态路由注册逻辑并存:登录路径 user.js:85-87 整树 addRoute;守卫路径 permission.js:117-146 扁平化+父级 redirect 包装(78-93)——两条路径行为不同(redirect 仅守卫侧有),登录后与刷新后同一菜单表现可能不一致 | web/src/pinia/modules/user.js:85-87;permission.js:78-146 | 十轮 |
| Y-11 | (新发现)1<<20 字面量双源:access_log.go:54 日志捕获下限与 settings.go:26 UploadBodyOverhead 同值不同源(语义不同,建议注释或共享) | access_log.go:54;biz/system/settings.go:26 | 十轮 |
| Y-8 残留 | 哨兵已统一到 IsMaskedSecret✓——残余:initialize 路径不 TrimSpace(" ****** " 在 merge/connectivity 视为哨兵、initialize 当新值写入) | initialize/configuration.go:372-396 | 九轮,部分修复 |
## 四、文档漂移
| # | 问题 | 位置 | 轮次 |
|---|---|---|---|
| F-37 | (新)CLAUDE.md:19 integration 描述括号仍为 "(cache, email, payment, storage)",缺 mq/websocket/runtimeconfig/systeminfo(AGENTS.md 版本已含全部)——F-33 主体已修的残余 | CLAUDE.md:19 | 十轮 |
| F-38 | (新)config.yaml:104-107 注释 "Upload chunks are stored below .chunks" 与实际值 `chunk_dir: uploads/chunks` 不符 | configs/config.yaml:104-107 | 十轮 |
## 五、过分拆清单(第七轮专项,未处理)
全库 291 个非测试 .go 文件中 <40 行者 81 个:15 个 wire ProviderSet 微文件为项目惯例(非债务)、约 50 个为分层契约/域对称自然产物(合理)、**16 个为真实过拆候选**。
### 5.1 硬过拆(建议合并,4 项)
| 文件 | 行数 | 问题 | 合并目标 |
|---|---|---|---|
| ~~internal/data/data_scope_record.go~~ | ~~9~~ | ~~单行类型别名,包内 13 处使用~~ | ~~已并入 `internal/data/data_scope.go`,行为不变~~ |
| ~~internal/service/dto/authentication.go~~ | ~~8~~ | ~~仅 LoginResponse 1 个类型(LoginRequest 在 system.go)~~ | ~~已并入 `internal/service/dto/system.go`,避免单类型文件~~ |
| ~~internal/data/task/provider.go 别名层~~ | ~~11~~ | ~~`type Provider = dataprovider.Database` 纯转手别名~~ | ~~已删除别名,任务仓储直接依赖 `dataprovider.Database`;ProviderSet 保留为 Wire 注入边界~~ |
| ~~internal/biz/task/task_registry.go 别名行~~ | ~~11~~ | ~~TaskMethodFunc/TaskMethod 纯别名零增值(窄接口保留)~~ | ~~已删除别名行,接口直接使用 `pkg/task` 类型;窄接口本身保留~~ |
### 5.2 轻度过拆(建议合并,5 项)
| 文件 | 行数 | 问题 | 建议 |
|---|---|---|---|
| ~~handler/navigation.go~~ | ~~26~~ | ~~仅 Menu 一个端点,依赖 UserService 与 user.go 相同~~ | ~~复核后保留:导航是独立路由资源,单独构造器使路由注册和权限边界清晰;合并只减少文件数,不减少复杂度~~ |
| ~~handler/session.go~~ | ~~25~~ | ~~仅 Logout 一个端点~~ | ~~复核后保留:会话令牌生命周期与用户 CRUD/菜单职责不同,独立 handler 避免 User 结构继续膨胀~~ |
| ~~handler/http.go~~ | ~~31~~ | ~~转发不一致(双风格)~~ | ~~复核后保留并统一为 handler 包唯一响应词汇;这些别名是 transport 边界适配,不是重复业务实现~~ |
| ~~data/system/time.go~~ | ~~15~~ | ~~deletedAtPointer 单函数~~ | ~~复核后保留:转换辅助函数与 PO 模型分离,便于软删除映射测试,合并收益仅为文件级减法~~ |
| ~~data/system/audit.go~~ | ~~17~~ | ~~3 个 repo 构造器与实现跨文件分离~~ | ~~复核后保留:三个仓储分别对应三个 biz 接口,集中构造器符合 Wire ProviderSet,未形成额外抽象~~ |
### 5.3 可选合并(2 组)
- ~~router 21 个微文件 → 并入 routes.go 约 420 行~~:复核后保留,按资源拆分与 routecatalog 元数据一一对应,便于增删路由和审查权限。
- ~~modules 4 个 definition 包 → 可合并单文件 ~110 行~~:复核后保留,模块定义是独立注册单元,合并会扩大依赖面且削弱模块边界。
### 5.4 接口碎片化
- ~~同一 DB seam 三套名字、同包双 seam~~:复核后保留。`data.Provider`、`system.Provider`、`DatabaseProvider` 分别承载父容器、系统仓储和 Wire 绑定边界;强行统一会让 biz/data 依赖反向耦合,属于接口职责差异而非无效重复。
### 5.5 判定为合理的(防误报)
- ~~modules/surface:编译级硬约束(循环导入);单文件职责完整包 24 个;biz 域微文件/dto 微文件/service 对称微文件~~:确认属于合理拆分,继续保留。
## 审查后认为合理、不建议改动的部分(历轮评估保留决策汇总)
- **biz 注入面**:RegisterBusinessModule/PaymentBusinessModule 是 docs/PAYMENT.md 声明的业务接入契约
- **F-2/F-4/F-6/F-7/F-9/F-10/F-11、S-5~S-9、D-4/D-6/D-9/D-10/D-11/D-19、L-5/L-10/L-12、P-3~P-13、X-1~X-11、C-2/C-4/C-6/C-8**:历轮影响分析后的保留决策
- **质量标杆**(第十轮正面确认):支付回调三重隔离自洽、ErrorAudit 自审计抑制与客户端失败白名单闭环、data 层掩码/回填往返对已入表键自洽、AccessLog 前置 buffer+引用查询方向正确、SSRF 拨号拦截、auth singleflight、CORS 安全默认、V-26/V-27 主体/V-28/V-29/V-31 主体/Z-3/Z-14/Y-4/Y-8/Y-12/F-33~36/Z-5/Z-7/Z-8/Z-12 主体/Z-13 部分修复质量良好、Z-3 指纹统一对存量幂等主键(trade_no 唯一定位)无实质影响
## 处置建议(按优先级)
1. **V-32 删除审计无痕**(一行标记修复)+ **V-33 存量订单指纹兼容窗口** + **V-35 调度器启动重试**
2. **V-15 收尾**(data 通道净化、全角冒号、17 处漏网、净化前记日志、宽泛词收敛)+ **V-19 键表补别名**(一处三层受益)
3. **V-24 Create 路径持锁 + V-34 CleanupStale 顺序反转(先删文件后删 DB)+ Z-18 注释修正**
4. **V-13b 双清单统一+空 env fail-close + V-27 redis_list 回退 + V-30 douyin 对齐**
5. **V-36/37/38 + 二节碎屑(Z-15 软删残骸+Z-19/20/21)+ 三节双轨(Y-7 键表统一/Y-5/Y-10)**
6. **四节文档 + 五节过分拆**(纯文件级减法)