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

107 lines
15 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 internalintegration 不 import data/servicedata 不 import integration/payment无循环依赖
---
## 一、安全与正确性缺陷(最高优先级)
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| V-11 | **脱敏退化洞(部分修复后残留核心)**maxBytes 已改为 `max(1MiB, logLimit)` 跟随配置,但 captureWriter **无 truncated 标志**redactJSON 原文分支用 `>` 而非 `>=`——当 `access_log_max_bytes ≥ 1MiB` 且响应超该值时,截断体长度恰等于 limit未脱敏明文仍进入 resp_data 与操作审计 Response`zap.access_resp_data` 默认 true。修法captureWriter 加 truncated 标志,截断体直接返回占位符 | server/middleware/capture.go:19-39,56-60access_log.go:53-57audit.go:79 | 六轮发现,七轮复核部分修复 |
| V-19 | **integration 掩码/恢复键集不对称(高危破坏链)**读路径掩码service 层 contains cert/p12 启发式宽于写路径恢复data 层 definition Secret + 后缀表)——① **`key_pem` 两层均不命中明文泄漏**biz 校验 :335 接受的微信 V2 退款私钥别名);② **回显保存会把 `******` 落库破坏支付配置**被读掩码但不被写恢复的键cert_pem/pkcs12_file/pkcs12_content/cert_path/cert_content/app_cert_path/root_cert_content/public_cert_content 及 QQ cert_file——definition 标 Secret=false 却被 contains "cert" 掩码)经前端回显提交后原样落库并 publish 到 runtime支付退款能力静默损坏且非空校验仍通过③ 空值也被掩码 + 新建分支无恢复,未配置渠道保存即落库 `******`;④ data 层掩码/恢复不递归数组service 层递归);⑤ service 层掩码值硬编码 `"******"` 未引用 config.MaskedSecret。修法统一两侧判定为共享键表 | service/integration/integration_config.go:69-89,79掩码data/integration/integration_config.go:224-281,258恢复biz/integration/integration_config.go:335biz/integration/integration_config_definition.go:169 | 六轮发现,七轮复核部分修复+发现破坏链 |
| V-15 | **handler 层 service 错误泄露 86 处(零收敛)**organization.go 16 处、version.go 15 处stage 分类骨架已建但所有分支仍拼 err.Error()形同虚设、parameter.go 7、authority.go 9、payment.go 11含 TestProvider 的 `Write(c, CodeError, result, err.Error())`、integration_config.go 5provider 连接细节、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。audit 域V-6与各读路径固定文案保持良好failLogViewer 范式未推广 | server/handler/organization.go:24-248、version.go:107-120,158-167、media.go:222-298 等 | 六轮发现,七轮复核未修复 |
| V-16b | **支付方式词表残余不对齐**V-16 三处主表已修其他渠道漏网douyin.go:116 缺 `miniapp`/`mini_app`其他渠道均接受lakala.go:41 缺 `mini_app`alipay.go:440 缺裸词 `mini`/`miniprogram`。另有测试缺口alipayCreateMethod/saobei/lakala/allinpay 无单测wechat_v2/v3 未将新增同义词作为用例字面量 | integration/payment/douyin.go:116、lakala.go:41、alipay.go:440 | 七轮 |
| V-17 | 秒传一致性两处残留:① 秒传 copy 直接沿用 `media.Mime` 不走 CompleteUpload 的 TypeByExtension 兜底(源记录 Mime 空则落空串);② 秒传 Tag 沿用源文件扩展而 Name 用新名,扩展与 Tag 不一致(普通路径 Tag 取 session.FileName 扩展,行为分叉) | biz/system/media_upload.go:100 vs :250-255 | 六轮发现,七轮复核部分修复 |
| V-18 | 上传会话回收缺口(孤儿 media 补偿已加 :259-263 best-effort① failed 会话分片无人回收StaleUploadSessionIDs:121-125 只查 uploadingfail() 只置状态不删分片,唯一回收入口是用户手动 Cancel**merging 僵死同样不回收**Claim 置 merging 后进程崩溃则会话永停 mergingSaveChunk/Complete 均拒);③ 补偿三步错误均 `_ =` 吞掉无重试/审计 | data/system/media_upload.go:121-125,85-88biz/system/media_upload.go:205-208,259-263 | 六轮发现,七轮复核部分修复 |
| V-20 | 前端 token 拼入 URL 未修复:完整长期登录 token 仍拼入二维码 URL`console.log(codeUrl.value)` 把含 token 的 URL 打进控制台 | web/src/components/upload/QR-code.vue:55,57scanUpload.vue:111-115 | 六轮发现,七轮复核未修复 |
| V-13b | Swagger 开关三处质量缺陷:① 无 env=production 下"swagger 不注册"的测试用例(现有测试全用 `config.Admin{}`);② env 仅精确匹配 "production"(配 prod/live 等同义值仍暴露Env 为自由字符串无枚举校验);③ fail-open——配置缺失时选择暴露而非隐藏与最小暴露原则相反 | server/gin.go:64-66config/types.go:186-190 | 七轮V-13 修复的保留项) |
## 二、死代码与碎屑
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| Z-1 | audit.go isDownloadResponse 不可达分支仍在redactJSON 返回值恒 ≤limit`len>maxBytes` 数学恒假):连带死代码群 operationDownloadHeaders 表(:168-178+ isDownloadResponse:180-188整体不可达——上轮只删了注释未删分支留下比注释更大的死块:82-84 下载截断判断与 redactJSON 内部逻辑冗余 | server/middleware/audit.go:82-84,166-188 | 六轮发现,七轮复核部分修复 |
| Z-2 | 两个 migration step 重复播种同一条 test APIensureCommunicationSurface:88 已含+已授权ensureCommunicationTestSurface:145-153 再种一遍)——新装环境纯冗余、存量环境一次性补种后永久空转 | data/system/migrations.go:88,145-153,31-32 | 六轮发现,七轮复核未修复 |
| 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 != true &&` 前缀使该行更长;新功能本身正确且有测试) | server/middleware/error_audit.go:25 | 六轮发现,七轮复核恶化 |
| Z-6 | 测试缺口五处OperationAudit 端到端测试现有仅函数级paymentCreateRequiresNotifyURL 测试NormalizePaymentMethod 直接单元测试;支付词表新增同义无用例(见 V-16bCORS allow-all 无回归测试credentials=false 一旦回退不会被捕获) | 各处 | 六轮发现,七轮扩展 |
| Z-7 | audit.go capturedBody 改造引入的新死赋值::46-47 `requestBody = []byte(stringValue(value))` 在 ctxReqBodyKey 存在分支执行后必被 :89 再次命中置 capturedBody=true赋的值永不被读:88-100 if/else 两终分支重复调用逐字相同的 operationRequestBody 表达式,可折叠 | server/middleware/audit.go:46-47,88-100 | 七轮 |
| Z-8 | routecatalog 描述含糊getSysParam 与 getSysParamsList 描述均为「获取参数列表」(前者语义是按 key 取单值),管理端菜单两条同名接口 | routecatalog/catalog.go:118-119 | 七轮 |
## 三、重复实现 / 双轨残留
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| Y-1 | 上传限额 fallback 复制access_log.go:42-46 手写 `MaxFileSize>0?…:Default`+1MB 边际biz `EffectiveMaxFileSize` 已有同源 fallbackhandler/media.go 已复用——middleware 已 import biz/system 可收敛;调整即静默漂移 | server/middleware/access_log.go:42-46 vs biz/system/settings.go:27-32 | 六轮发现,七轮复核未修复 |
| Y-2 | 响应体双重 JSON 处理请求体方向已修audit.go 优先复用 AccessLog 已脱敏值access_log.go:112 与 audit.go:79 仍各自对同一响应体做 解析+脱敏+重序列化 两次;且 capture.go:55-67 先 marshal 后查超限,超限时 marshal 工作白做——建议超限先判长度 | server/middleware/access_log.go:112audit.go:79capture.go:55-67 | 六轮发现,七轮复核部分修复 |
## 四、文档漂移(残余项)
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| F-33 | CLAUDE.md 残余矛盾四处(原样未动)::77-78 "API error reason enum"proto 残留vs AGENTS.md "stable reason strings":102 "HTTP/gRPC servers"(项目无 gRPC:8-25 目录树缺 docs/、internal/logging/、internal/paymentkit/AGENTS.md 均有);:115 Wiring 简化版与 AGENTS.md 详细版不同步 | CLAUDE.md:8-25,77-78,102,115 | 六轮发现,七轮复核未修复 |
| F-34 | pkg/README.md「当前包含」漏列 mq/、websocket/ 实际包;且 :7-10 与 :10 重复罗列 database/module/task | 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"单一词汇表"目标未达成包内双风格 | 删转发改直用 httpx或补齐统一风格 |
| data/system/time.go | 15 | deletedAtPointer 单函数无独立文件必要 | 并入 models.go |
| data/system/audit.go | 17 | 3 repo 构造器与实现跨文件分离方法散布 5 个文件阅读跳 2 | 构造器移回各自首个实现文件 |
### 5.3 可选合并模式性过拆2 组)
- **router 21 个微文件**12-33 /)→ 并入 routes.go 420 routecatalog 449 行体量一致项目已证明可接受 wire/测试/依赖方向约束仅剩"每域一文件"对称风格
- **modules 4 definition ** 1 文件 17-24 单一消费者 catalog.go)→ 可合并为 catalog.go 单文件 ~110 保留理由是模块插件式对称若不打算支持外部模块注册属过度形式化
### 5.4 接口碎片化(结构性)
- **同一 DB seam 三套名字同包双 seam**data/provider.Database2 方法)→ data/task/provider.go 别名 data/system.Provider4 方法超集三名字并存 data/system/security.go import 外部 2 方法版而不用同包超集——同包两个 seam 混用建议task 删别名system 包内统一用一个 seam
### 5.5 判定为合理的(复检确认,防误报)
- **modules/surface保留理由升级为编译级硬约束**——catalog.go:7-10 import 四个子包 definition子包 definition.go:7 import surface若并入根包即循环导入Go 禁止独立成包是唯一解
- 单文件但职责完整的包cache/email/runtimeconfig/systeminfo/routecatalog/httpx/staticfiles/utils 两包/gormkit/pagination/module/task 24 均有真实多消费者或分层必需
- biz 域微文件cache/email/errors/maintenance/permission 8 分层契约自然形态
- dto email/permission/system_init7-13 轻度过拆但按域对称可容忍
- service access_control/email/permission/security_session 微文件DTODO 对称模式
## 审查后认为合理、不建议改动的部分(历轮评估保留决策汇总)
- **biz 注入面**RegisterBusinessModule/PaymentBusinessModule docs/PAYMENT.md:121-129 声明的业务接入契约PayInternal/RefundInternal/AuthorizeRefund biz 调用链消费
- **F-2/F-4http.go 5.2 重判/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**历轮影响分析后的保留决策理由均已复核与代码相符
- **质量标杆**第七轮正面确认payment 幂等指纹+回调强制平台查单+hook 前后指纹校验退款 lease 语义task_scheduler 锁序SSRF 拨号防护auth singleflightWithoutCancel)、log_file.go os.Root+symlink+SameFile TOCTOUstaticfiles 安全media_upload 分片校验链system 配置掩码+preserve 闭环BodyPolicyUpload 闭环V-12/13/14/21/22 修复质量test policy 运行时判定+测试Swagger env 开关CORS credentials=false+Vary、GetAndDelete 原子化)、V-16 三处词表补齐+paymentkit 统一归一化方向正确recovery panic dump 不含 bodytraceparent 完整校验支付回调空 buffer 隔离
## 处置建议(按优先级)
1. **V-19 掩码/恢复键集不对称**回显保存会把 `******` 落库破坏支付配置——统一共享键表后连带解决 key_pem 泄漏
2. **V-11 截断标记保护**captureWriter truncated 标志+占位符
3. **V-15 错误泄露全域收敛**推广 failLogViewer 范式86 + **V-16b 词表对齐**douyin/lakala/alipay 三处补齐+测试
4. **V-17/V-18/V-20/V-13b**秒传一致性会话回收扩 failed+merging前端 tokenSwagger 开关质量
5. **二节死代码 + 三节双轨 + 四节文档**一批小清理
6. **五节过分拆**4 +5 +2 组可选+seam 统一——纯文件级减法零行为变更
## 已修复(划线标记)
本轮已确认并完成~~V-11~~、~~V-15~~、~~V-16b~~、~~V-17~~、~~V-18~~、~~V-19~~、~~V-20移除控制台泄露~~、~~V-13b补充 prod/live/staging 环境屏蔽~~、~~Z-1~~、~~Y-1~~。