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

118 lines
18 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 internalintegration 不 import data/servicedata 不 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-57data/payment/payment.go:79-139handler/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-657data/payment/payment_order.go:135-156 | 十轮 |
| V-24 | 串行锁主体已修——**但 Delete↔Create 竞态窗口仍开**:锁仅覆盖 Deletemedia.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-143media_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},自定义 envqa/uat时路由不注册但启动日志仍打印 swagger URL:171 日志谎报);③ env 门禁零测试 | server/gin.go:64-70cmd/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 未带 CategoryIDInitUpload 签名无 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 errorserrors.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✓——同包 UpdateMediaNamemedia.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-98data/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,34media_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-85access_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 恒 0count==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 | (新发现)同请求重复 Lookupaccess_log.go:34 与 :86 对同一请求两次调 BodyPolicyFor可复用 :34 结果) | server/middleware/access_log.go:34,86 | 十轮 |
## 三、重复实现 / 双轨残留
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| Y-7 | 掩码三套实现+键表三层漂移data 层SecretIsIntegrationSecretKey递归含数组✓+ service 层(仅 IsIntegrationSecretKey**不认 definition.Secret**——靠 data 层先行掩码偶然自洽)+ middleware/redact.go 第三套(归一化规则不同:删分隔符 vs `-`→`_`哨兵双轨config.MaskedSecret="******" 与 middleware redactedValue="***"(前端若把日志侧 *** 回填会被当真值保存IsIntegrationSecretKey platform_cert/root_cert 仍被 cert 后缀覆盖(冗余) | data/integration:225-245service/integration:69-96redact.go:18-27,6 | 九轮,十轮复核部分(递归)修复 |
| Y-5 | 集成配置校验双重执行biz Save 对哨兵替换后值校验("******"非空总通过,半失效)+ data 层 merge 后再校验兜底——biz 层形同虚设 | biz/integration:122-129data/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-87permission.js:78-146 | 十轮 |
| Y-11 | 新发现1<<20 字面量双源access_log.go:54 日志捕获下限与 settings.go:26 UploadBodyOverhead 同值不同源语义不同建议注释或共享 | access_log.go:54biz/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/systeminfoAGENTS.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 处使用 | 并入 data/data_scope.go |
| internal/service/dto/authentication.go | 8 | LoginResponse 1 个类型LoginRequest system.go | 并入 dto/system.go |
| internal/data/task/provider.go 别名层 | 11 | `type Provider = dataprovider.Database` 纯转手别名 | 删别名直接用 dataprovider.Database |
| internal/biz/task/task_registry.go 别名行 | 11 | TaskMethodFunc/TaskMethod 纯别名零增值窄接口保留 | :5-6 别名行 |
### 5.2 轻度过拆建议合并5 项)
| 文件 | 行数 | 问题 | 建议 |
|---|---|---|---|
| handler/navigation.go | 26 | Menu 一个端点依赖 UserService user.go 相同 | 并入 user.go |
| handler/session.go | 25 | Logout 一个端点 | 并入 user.go public.go |
| handler/http.go | 31 | 转发不一致双风格 | 删转发或统一 |
| data/system/time.go | 15 | deletedAtPointer 单函数 | 并入 models.go |
| data/system/audit.go | 17 | 3 repo 构造器与实现跨文件分离 | 构造器归位首个实现文件 |
### 5.3 可选合并2 组)
- router 21 个微文件 并入 routes.go 420
- modules 4 definition 可合并单文件 ~110
### 5.4 接口碎片化
- 同一 DB seam 三套名字同包双 seam
### 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-11S-5~S-9D-4/D-6/D-9/D-10/D-11/D-19L-5/L-10/L-12P-3~P-13X-1~X-11C-2/C-4/C-6/C-8**历轮影响分析后的保留决策
- **质量标杆**第十轮正面确认支付回调三重隔离自洽ErrorAudit 自审计抑制与客户端失败白名单闭环data 层掩码/回填往返对已入表键自洽AccessLog 前置 buffer+引用查询方向正确SSRF 拨号拦截auth singleflightCORS 安全默认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. **四节文档 + 五节过分拆**纯文件级减法