103 lines
15 KiB
Markdown
103 lines
15 KiB
Markdown
# 代码审查问题清单（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-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-60；access_log.go:53-57；audit.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:335；biz/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 5（provider 连接细节）、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 只查 uploading，fail() 只置状态不删分片，唯一回收入口是用户手动 Cancel）；② **merging 僵死同样不回收**：Claim 置 merging 后进程崩溃则会话永停 merging（SaveChunk/Complete 均拒）；③ 补偿三步错误均 `_ =` 吞掉无重试/审计 | data/system/media_upload.go:121-125,85-88；biz/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,57；scanUpload.vue:111-115 | 六轮发现，七轮复核未修复 |
|
||
| V-13b | Swagger 开关三处质量缺陷：① 无 env=production 下"swagger 不注册"的测试用例（现有测试全用 `config.Admin{}`）；② env 仅精确匹配 "production"（配 prod/live 等同义值仍暴露，Env 为自由字符串无枚举校验）；③ fail-open——配置缺失时选择暴露而非隐藏，与最小暴露原则相反 | server/gin.go:64-66；config/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 API（ensureCommunicationSurface: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-16b）；CORS 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` 已有同源 fallback（handler/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:112；audit.go:79；capture.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.Database（2 方法）→ data/task/provider.go 别名 → data/system.Provider（4 方法超集）三名字并存；且 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_init（7-13 行）：轻度过拆但按域对称可容忍
|
||
- service 层 access_control/email/permission/security_session 微文件：DTO↔DO 对称模式
|
||
|
||
## 审查后认为合理、不建议改动的部分（历轮评估保留决策汇总）
|
||
|
||
- **biz 注入面**：RegisterBusinessModule/PaymentBusinessModule 是 docs/PAYMENT.md:121-129 声明的业务接入契约；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（WithoutCancel）、log_file.go 的 os.Root+symlink+SameFile 防 TOCTOU、staticfiles 安全、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 不含 body、traceparent 完整校验、支付回调空 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、前端 token、Swagger 开关质量）
|
||
5. **二节死代码 + 三节双轨 + 四节文档**（一批小清理）
|
||
6. **五节过分拆**（4 硬+5 轻+2 组可选+seam 统一——纯文件级减法，零行为变更）
|