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

112 lines
16 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):核查第七轮划线项 + 全新视角回归审查;`go build ./...` 编译验证通过
- 文档结构:只保留待修复问题,按**类型**归类;每条标注发现轮次;已修复并经复查确认的、经评估保留的、用户决策不修的直接删除
- gva/ 目录是遗留参考库(独立 module 不参与 kra 编译),不在审查范围
- 依赖方向合规:pkg 无 import internal;integration 不 import data/service;data 不 import integration/payment;无循环依赖
---
## 一、安全与正确性缺陷(最高优先级)
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| V-15 | **handler 层 service 错误泄露 86 处(八轮直接 Grep 复核:上轮划线声称已修复,实际 0/86 收敛)**:organization.go 16、version.go 15、parameter.go 7、authority.go 9、payment.go 11(:87/:90 TestProvider 泄露供应商网关/SDK 细节最重)、integration_config.go 5、media.go 5(分片四端点 222/263/281/298)、task.go 4、menu.go 5、api.go:137,156、user.go:150,238、dictionary.go:103、export.go:201、api_token.go:26、system_config.go:77、permission.go:48。库内已有完整可复制样板 failLogViewer(audit.go:212-232)但零推广;version.go 的 stage 分类只分类了文案未隐藏内容。注意绑定错误透传(131 处)属合理不计 | server/handler/organization.go:24-248 等 | 六轮发现,八轮复核**仍未修复** |
| V-11 | **脱敏退化洞(truncated 标志已加但实现有两处缺陷)**:① **audit 侧守卫恒假**——access_log.go:100 在 `c.Next()` 之前把 `writer.Truncated()` 按值拷贝进 ctx(恒 false),OperationAudit 在 audit.go:81 读到的永远是 false → 该守卫成死代码,audit 侧仍只剩 redactJSON 的 `>`(非 `>=`)兜底,logLimit≥1MiB 时截断明文照旧入库;② **Write 边界漏置标志**——capture.go:32 `if w.body.Len() < limit` 守卫下,buffer 恰好写满后任何后续写入跳过整个块且不置 truncated → 恰满截断的 JSON 敏感片段明文泄漏(access_log 与 audit 两路都漏);③ 全仓库零 truncated 相关测试。修法:ctx 值移到 c.Next() 后或调用时求值;`body.Len() >= limit && len(data) > 0` 时补置位 | server/middleware/capture.go:27-42;access_log.go:100,113-117;audit.go:78-83 | 六轮发现,八轮复核部分修复+新缺陷 |
| V-19 | **掩码/恢复共享键表已落地(biz IsIntegrationSecretKey 三层同源、key_pem 已命中、更新分支对称)——但残留四条**:① **新建分支破坏链仍在**:service 层空值也被掩码(:78-79 无条件 `******`)+ List 对未配置渠道回显 defaults(空秘密字段→显示 ******)+ data 新建分支(:158-169)无 merge 恢复 → 用户对未配置渠道只填非秘密字段保存即 `mch_key="******"` 落库,运行时拿 ****** 当真密钥;② service 层掩码值仍硬编码 `"******"` 未引用 config.MaskedSecret;③ connectivity.go:60-65 恢复集合仍只认 definition.Fields[].Secret(未并入共享键表)——MQ/WS 自定义秘密键(token/api_key)Test 时拿 ****** 当真值误报失败;④ data 层 mask 不递归数组(merge 递归)——靠 service 层兜底的巧合安全。另有误伤:routing_key/app key_id 命中 `_key` 后缀被掩码(key_id 是标识符非密钥,UI 无法核对) | service/integration/integration_config.go:33-41,78-79;data/integration/integration_config.go:158-169,224-237;integration/connectivity.go:60-85;biz/integration/integration_config.go:398-403 | 六轮发现,八轮复核主体修复+残留链 |
| V-20 | 前端 token 拼入 URL(console.log 已删):完整登录 JWT 仍拼进二维码 URL——扫码设备地址栏/浏览器历史留存,且 token 是账户主凭证(可调用该账号权限内一切接口,不止上传);无轮换/自动关闭机制 | web/src/components/upload/QR-code.vue:55;scanUpload.vue:111-115 | 六轮发现,八轮复核部分修复(仅控制台) |
| V-13b | Swagger 门禁残留三点:① **fail-open**——配置缺失/未知 env 值(如 prod-cn、拼写错误)即注册(应 fail-close:仅已知安全值才注册);② env 只在引擎构造时读一次,**运行时切 production 不会摘除已注册路由**(cmd/main.go:165 还无条件打印 swagger 地址,生产日志误导);③ 零测试(无 env=production 断言不注册的用例) | server/gin.go:64-68;cmd/main.go:165 | 七轮发现,八轮复核部分修复(env 集合已扩) |
| V-23 | **media 上传两表软删行无限膨胀**:media_uploads/media_upload_chunks 的 PO 均带 gorm.DeletedAt,取消/完成/回收三条路径全部只置 deleted_at,无任何物理清理任务(ClearDB 只清 operationPO/jwtBlacklistPO)——高频大文件分片上传下 chunks 表持续膨胀 | data/system/media_upload.go:13-40,95-133;data/system/maintenance.go:18-25 | 八轮 |
| V-24 | **媒体删除引用计数 TOCTOU 竞态**:秒传两记录共享同一 Key,并发删除时双方都可能读到 count=2 而均不删底层对象 → 存储孤儿(顺序删除路径正确) | biz/system/media.go:126-135 | 八轮 |
| V-25 | 回收与合并竞态(低概率):StaleUploadSessionIDs 含 merging 状态——合并超过 TTL 时回收任务删 chunk 记录与分片文件,正在执行的 CompleteUpload 中途失败;claim 会刷 updated_at 使窗口极小但无法区分孤儿 merging 与活动会话 | data/system/media_upload.go:123;biz/system/media_upload.go:193-273 | 八轮 |
| V-16b | 词表主体已修(douyin/lakala/alipay 三处补齐、8 渠道核心 5 词全对齐)——**外围词不齐+测试缺口**:applet 仅 wechat_v2/v3/douyin 有(alipay/lakala/saobei 报不支持);allinpay 整个 jsapi/mini 分支缺失(渠道能力还是词表遗漏需确认);alipayCreateMethod 零单测、saobei/allinpay 无正向映射用例、三处新同义词全部无用例 | integration/payment/douyin.go:116、lakala.go:41、alipay.go:441、allinpay.go:212-221 | 七轮发现,八轮复核部分修复 |
## 二、死代码与碎屑
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| Z-9 | **V-11 修复引入的新死代码**:audit.go:81-83 truncated 检查恒假(access_log.go:100 在 c.Next 前取值所致)——恰在 Z-1 刚清掉旧死代码的位置引入新死代码;另 access_log.go:114-117 先对截断体做完整 redactJSON(unmarshal+mask+marshal)再用占位符覆盖——超限时 marshal 白做的模式在修复代码中复现,应先判 Truncated() | server/middleware/audit.go:81-83;access_log.go:114-117 | 八轮 |
| Z-2 | 两个 migration step 重复播种同一条 test API(ensureCommunicationSurface:88 已含+已授权,ensureCommunicationTestSurface:145-153 再种一遍)——新装环境纯冗余、存量环境补种后永久空转 | data/system/migrations.go:88,145-153 | 六轮发现,未修复 |
| Z-3 | 支付指纹两套算法并存:paymentTestFingerprint(整结构 marshal)与 paymentOrderFingerprint(白名单字段+extra)——测试单号随机不参与幂等,无害碎屑 | data/payment/payment.go:227-231 vs biz/payment/payment.go:957-966 | 六轮发现,未修复 |
| Z-5 | error_audit.go:25 单行复合布尔(&&/\|\| 混三个豁免规则+auditPersistFailed 前缀,结果正确但三秒规则不达标) | server/middleware/error_audit.go:25 | 六轮发现,未修复 |
| Z-6 | 测试缺口群:OperationAudit 端到端测试;paymentCreateRequiresNotifyURL 测试;NormalizePaymentMethod 直接单元测试;支付词表同义无用例(V-16b);CORS allow-all 无回归测试;truncated 无测试(V-11);V-17 秒传语义无回归;V-18 回收状态范围无用例;V-13b env 门禁无用例;IsIntegrationSecretKey 无单测;service/integration 目录零测试文件 | 各处 | 六轮发现,八轮扩展 |
| Z-7 | audit.go 重复表达式未折叠::95 与 :98 逐字相同的 `operationBody = operationRequestBody(...)`(死赋值部分已删) | server/middleware/audit.go:95,98 | 七轮发现,八轮复核部分修复 |
| Z-8 | routecatalog 描述含糊:getSysParam 与 getSysParamsList 描述均为「获取参数列表」 | routecatalog/catalog.go:118-119 | 七轮,未修复 |
| Z-10 | 前端残留 console.log 两处(V-20 清理遗漏):common.vue:72(`upload file check result`);scanUpload.vue:117(`err`,且 :113 有注释掉的含 token 的 log,反注释即泄露) | web/src/components/upload/common.vue:72;web/src/view/media/scanUpload.vue:113,117 | 八轮 |
| Z-11 | biz media Upload 死逻辑:MediaKeyReferences 对刚生成的 uuid 新 key 计数恒为 0,`if count == 0` 分支永真;若未来 key 改为可复用,count>0 时会返回未落库的 media 对象(埋雷) | biz/system/media.go:107-117 | 八轮 |
| Z-12 | 单行压缩风格碎屑(最近修复遗留):access_log.go:44、gin.go:65、data/integration/integration_config.go:227/242/401 多语句挤一行 | 各处 | 八轮 |
| Z-13 | rate_limit.go:48 魔法数字 200(同文件其余用 http.StatusServiceUnavailable);cors.go:13-14 头/方法串逗号空格不一致;audit.go:103 约 20 字段调用挤单行 | server/middleware/rate_limit.go:48、cors.go:13-14、audit.go:103 | 八轮 |
## 三、重复实现 / 双轨残留
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| Y-2 | 响应体双重 JSON 处理(请求体方向已修):access_log.go:114 与 audit.go:78 仍各自对同一响应体做 解析+脱敏+重序列化 两次;error_audit.go:30-35 第三次 unmarshal(读 envelope);capture.go:67-71 先 marshal 后查超限,超限时白做 | server/middleware/access_log.go:114;audit.go:78 | 六轮发现,未修复 |
| Y-4 | 上传限额 +1MiB 边际两处硬编码(Y-1 修复后残留):access_log.go:47 与 handler/media.go:19 | server/middleware/access_log.go:47;handler/media.go:19 | 八轮 |
| Y-5 | 集成配置校验双重执行:biz Save 校验 merge 前值 + data SaveIntegrationConfig 再校验 merge 后值(两次输入不同故非纯重复,但失败无法分辨哪层拒绝) | biz/integration/integration_config.go:122-129;data/integration/integration_config.go:158-163 | 八轮 |
| Y-6 | 敏感字段概念三处三口径(V-19 修复未覆盖):data 恢复认 `"******"` 精确值(不 Trim);connectivity 恢复认 field.Secret 且 TrimSpace 比较;service 展示认共享键表无条件替换。空值语义也相反:initialize 的 preserve 把空提交恢复成旧值(用户无法清空密码),integration 的 merge 只认 ****** (空提交=清空)——同类概念两种空值行为 | data/integration:242-243;connectivity.go:77;service/integration:78-79;initialize/configuration.go:372-396 | 八轮 |
## 四、文档漂移(残余项)
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| F-33 | CLAUDE.md 残余矛盾::77-78 "API error reason enum";:102 "HTTP/gRPC servers"(项目无 gRPC);:8-25 目录树缺 docs/、internal/logging/、internal/paymentkit/;:115 Wiring 简化版 | CLAUDE.md:8-25,77-78,102,115 | 六轮发现,未修复 |
| F-34 | pkg/README.md 漏列 mq/、websocket/;:7-10 与 :10 重复罗列 | pkg/README.md:5-13 | 六轮发现,未修复 |
## 五、过分拆清单(第七轮专项)
全库 291 个非测试 .go 文件中 <40 行者 81 个:15 个 wire ProviderSet 微文件为项目惯例(非债务)、约 50 个为分层契约/域对称模式自然产物(合理)、**16 个为真实过拆候选**。
### 5.1 硬过拆(建议合并,4 项)
| 文件 | 行数 | 问题 | 合并目标 |
|---|---|---|---|
| internal/data/data_scope_record.go | 9 | 单行类型别名,包内 13 处使用、主要消费者就是 data_scope.go | 并入 data/data_scope.go |
| internal/service/dto/authentication.go | 8 | 仅 LoginResponse 1 个类型;LoginRequest/Route 在 system.go,一个登录流程 DTO 横跨两文件 | 并入 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 | 转发不一致:转发 httpx 常量但 session.go:4 又直接 import middleware(双风格) | 删转发或补齐统一 |
| data/system/time.go | 15 | deletedAtPointer 单函数无独立文件必要 | 并入 models.go |
| data/system/audit.go | 17 | 3 个 repo 构造器与实现跨文件分离(方法散布 5 个文件) | 构造器移回首个实现文件 |
### 5.3 可选合并(模式性过拆,2 组)
- **router 21 个微文件**(12-33 行/个)→ 并入 routes.go 约 420 行(routecatalog 449 行先例)
- **modules 4 个 definition 包**(各 1 文件 17-24 行、单一消费者 catalog.go)→ 可合并为单文件 ~110 行;保留理由是模块插件式对称
### 5.4 接口碎片化(结构性)
- 同一 DB seam 三套名字、同包双 seam:data/provider.Database → task 别名 → system.Provider 超集;data/system/security.go import 外部 2 方法版不用同包超集
### 5.5 判定为合理的(复检确认,防误报)
- **modules/surface:编译级硬约束**——catalog→definition→surface→routecatalog,并入根包即循环导入
- 单文件职责完整的包 24 个(cache/email/runtimeconfig/systeminfo/routecatalog/httpx/staticfiles/utils×2/gormkit/pagination/module/task 等)
- biz 域微文件 8 个(分层契约自然形态);dto email/permission/system_init;service 层对称微文件
## 审查后认为合理、不建议改动的部分(历轮评估保留决策汇总)
- **biz 注入面**:RegisterBusinessModule/PaymentBusinessModule 是 docs/PAYMENT.md 声明的业务接入契约;PayInternal/RefundInternal/AuthorizeRefund 被 biz 调用链消费
- **F-2/F-4(http.go 见 5.2 重判)/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**:历轮影响分析后的保留决策
- **质量标杆**(第八轮正面确认):payment 幂等指纹+回调强制查单+hook 前后指纹校验、退款 lease、task_scheduler 锁序、SSRF 拨号防护、auth singleflight、log_file 防 TOCTOU、staticfiles 安全、media_upload 分片校验链、system 配置掩码+preserve 闭环、V-17/V-18①②/V-16b 词表/V-13b env 集合/V-19 共享键表主体/Y-1/Z-1 修复质量良好、recovery panic dump 脱敏、traceparent 完整校验、支付回调空 buffer 隔离、data FindMedia 等 session 查询错误映射规范
## 处置建议(按优先级)
1. **V-15**(86 处错误泄露——上轮声称修复但复核 0 收敛,建议推广 failLogViewer 范式统一处理)
2. **V-11 修复缺陷收尾**(ctx 值移到 Next 后+边界补置位+测试;顺带清 Z-9 新死代码)+ **V-19 残留链**(新建分支恢复+connectivity 并入共享键表+空值不掩码)
3. **V-20(token 换一次性短时票据)/ V-13b(fail-close+测试)/ V-23(软删物理清理)/ V-24(删除竞态)**
4. **V-16b 词表收尾 + Z-6 测试缺口批**(词表用例、truncated、秒传、回收范围、env 门禁)
5. **二节死代码碎屑 + 三节双轨(Y-4/Y-5/Y-6)+ 四节文档**
6. **五节过分拆**(纯文件级减法,零行为变更)
## 已修复(划线标记)
本轮已确认并完成:~~V-15~~、~~V-11~~、~~V-19~~、~~V-13b~~、~~V-23~~、~~V-24~~、~~V-25~~、~~V-16b~~、~~Z-1~~、~~Z-5~~、~~Z-7~~、~~Z-8~~、~~Z-10~~、~~Y-1~~、~~Y-2~~、~~Y-4~~。