1487 lines
178 KiB
Markdown
1487 lines
178 KiB
Markdown
# xlgo v1.1.1 缺陷修复执行进度
|
||
|
||
> 执行依据:`version_1.1.1_report.md`(13 CRITICAL + 8 HIGH 权威清单)。
|
||
> 执行顺序:P0 → P1 → P2 → P3。每项遵循"读全局 → 最小修复 → 机械验证 + 行为闭环 → 独立对抗性复审 → 三个全局问题写入本报告"的闭环节奏。
|
||
> 本机约束:H:\worker 有 svn 干扰,`go build/test` 需 `-buildvcs=false`;gcc/cgo 已安装(2026-06-28),`-race` 现可运行;staticcheck 因 go1.24/1.25 版本不匹配暂不可用。
|
||
|
||
---
|
||
|
||
## 🚀 快速接续(下次开启先读这里)
|
||
|
||
**当前状态(截至 2026-06-29)**:**P0 全部闭环** ✅ + **P1 全部闭环** ✅ + **P2 H6/H8 闭环** ✅(13 CRITICAL + 8 HIGH + H6 + H8)— 13 CRITICAL + 8 HIGH 全部修复并通过验证,P2 框架集成一致性阶段 H6(BaseRepo 接入 GetDBFromContext + 事务 join + 分页事务)与 H8(路由/health 统一 + Apply 幂等 + metrics 去调用顺序依赖 + 全局注册中心 atomic)均已闭环。
|
||
|
||
**已闭环清单**(每项含:修复 + 回归用例红绿验证 + `-race`/vet/gosec 通过):
|
||
- C6(csrf map 遮蔽)、C8(recover 返 200)、C4(storage 路径穿越)、C5(compress Zip-Slip)
|
||
- C2(ws 死锁/panic/泄漏)、C1(cache/lock panic/泄漏)、C7(CORS 绕过)、C3(SSE 断连泄漏)
|
||
- H2(默认关 TLS)、H1(不安全 RNG,删 RandString/RandDigit + Secure 系列)、C9b(刷新令牌 fail-closed)、H4a(限流误限流)
|
||
- C10(config 全局 Manager 无锁置换 + 热重载绕过 Validate + StopWatcher 空函数)
|
||
- C11(database 池泄漏 + Master/Replicas 无锁读 + 健康状态陈旧 + 包级 Close 仅关主库)
|
||
- C9c(jwt 包级 DefaultJWT/tokenBlacklist 无锁置换 → atomic.Pointer)
|
||
- H3(middleware/logger body OOM — 请求/响应 body 读取均封顶,下游仍得完整 body)
|
||
- H7(logger 包级 Logger/sugar/apiLog/dbLog 写有锁读无锁 → atomic.Pointer + Field.Duration 签名修复)
|
||
- C12(cron 数据竞争 + 重叠执行 + 漂移 + Weekly 跳周 + cron 解析缺陷)
|
||
- C13(trace nil-panic + 未实现导出器/传播器 + Middleware 不更新 c.Request)
|
||
- H5(handler BadRequest/InternalError 硬编 HTTP 400/500 绕过 Mode + 丢失 RequestID → 委托 response 体系)
|
||
- H4b(CustomRateLimit goroutine 泄漏 → 登记入 customLimiters 表,StopRateLimiters/InitRateLimiters 统一停止)
|
||
- **H4c(RedisRateLimiter fail-open + 裸断言 → fail-closed 可配置 + comma-ok + LoginRedisRateLimit 改 fail-closed)**
|
||
- **H6(BaseRepo 接入 GetDBFromContext + 读写分离 + 事务 join(r.tx + database.WithTx/TxFromContext)+ 分页单事务 + UpdateFields + QueryBuilder 克隆)**
|
||
- **H8(全局注册中心 atomic.Pointer + ensureRegistry 守卫 + Apply 幂等 + metrics 经 SetMetricsMiddleware 在 Apply 内作首个全局中间件 + /health 收敛单一 healthHandler + handler.HealthCheck schema 对齐)**
|
||
|
||
**下一项**:P2 H8 已闭环;进入 **P3 清理**(MEDIUM/MINOR 各项 + 全量补 `-race`)。
|
||
|
||
**关键规则**(CLAUDE.md 防复发约束):
|
||
- 每项必跑 `go test -race -buildvcs=false`;行为变更写入 CHANGELOG `[Unreleased]` 升级说明;交付前独立 agent 对抗性复审(file:line 证据)。
|
||
- 新增测试依赖 `github.com/alicebob/miniredis/v2` 已在 go.mod(cache/jwt 真实闭环测试用)。
|
||
- 工作区改动**未提交**(用户未要求 commit 前不要自行 commit)。`v1.1.1_fix_progress.md`/`v_1.1.1_fix.md`/`version_1.1.1_report.md` 为 untracked 进度/依据文件。
|
||
|
||
**待修复完整清单**:见文末「待修复(按优先级)」。
|
||
|
||
---
|
||
|
||
## ✅ C6 `middleware/csrf.go` API CSRF 模式功能性失效(map 遮蔽)— P0
|
||
|
||
### 需求分析结果
|
||
API 模式 CSRF 整体不可用:`CSRFForAPI()` 闭包校验读的是**局部** `tokens` map,而 `GenerateAPIToken` 写的是**包级** `tokens`,两者从不相交 → 所有非安全方法请求被判"CSRF Token 无效"拒绝。叠加包级 map 只增不减/无过期/不消费(内存 DoS + 永久重放),以及 `DoubleSubmitCookie` cookie HttpOnly=true 与"JS 回填 X-CSRF-Token 头"自相矛盾。
|
||
|
||
### 方案设计摘要
|
||
- 删除 `CSRFForAPI()` 内的局部 `tokens`/`mu` 声明,统一绑定包级存储。
|
||
- 包级存储改 `map[string]time.Time`(token → 颁发时间),写锁内"查—删"原子完成单次消费;`time.Since(issuedAt) > apiTokenTTL` 拒绝过期;`len>256` 时懒清理过期项防内存增长。
|
||
- `DoubleSubmitCookie` 的 cookie 改 `HttpOnly=false`(前端 JS 必须能读 cookie 才能回填头);`CSRF()` cookie 模式维持 `HttpOnly=true`(token 经 body/上下文下发,非读 cookie)。
|
||
- TTL 常量 `apiTokenTTL = 30 * time.Minute`;存储仍为进程内内存,注释声明仅适用单实例,多实例用 Redis `SETEX`+`GETDEL`。
|
||
|
||
涉及模块:`middleware/csrf.go`、`middleware/middleware_test.go`、`middleware/csrf_internal_test.go`。
|
||
DB 影响:无。API 影响:`CSRFForAPI`/`GenerateAPIToken`/`DoubleSubmitCookie` 行为变更(见下)。安全影响:正向——修复 CSRF 失效、防重放、防内存 DoS。潜在风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `middleware/csrf.go`:新增 `time` 导入;重写 `CSRFForAPI()`(删局部 map、单次消费+TTL+懒清理);重写 `GenerateAPIToken`(写包级 `apiTokens`);包级存储改名 `apiTokens`/`apiTokensMu` + `apiTokenTTL` 常量;`DoubleSubmitCookie` cookie `HttpOnly=false`。
|
||
- `middleware/middleware_test.go`:新增 `respCode`/`apiCSRFToken` helper 与 5 个回归用例。
|
||
- `middleware/csrf_internal_test.go`(新):TTL 过期分支用例(直接注入过期时间戳)。
|
||
|
||
### 实现内容摘要
|
||
颁发→校验闭环打通(同源包级 map);单次消费(写锁内 delete);TTL 30min;懒清理;DoubleSubmit cookie 可被 JS 读取。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。全程复用 `response.Fail/Success/ServerError`,未硬编状态码,未绕过响应模式系统。未引入 DB/路由/认证层面的新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(须 CHANGELOG 声明)**:
|
||
- API 模式 CSRF token 现为**单次消费**——每次成功 POST 后需重新 `GenerateAPIToken` 颁发新 token。原实现整体不可用,故不存在"原本可用现被破坏"的真实回归,但下游若已绕过该中间件需重新评估。
|
||
- API token 现有 30min TTL。
|
||
- `DoubleSubmitCookie` 的 cookie `HttpOnly` 由 true 改 false(前端必须能读 cookie)。
|
||
- 无 API 签名变更、无配置变更、无 migration。
|
||
3. **新并发/生命周期/泄漏隐患?** 包级 `apiTokens`+`apiTokensMu`(RWMutex)保护,查-删在同一写锁域原子;无 channel/goroutine/close 路径,无 send-on-closed 风险;懒清理在写锁内安全。无裸类型断言新增。**`-race` 已于 2026-06-28 补跑通过**(`go test -race -buildvcs=false ./middleware/`,1.980s 无竞争)。
|
||
|
||
### 代码审查结果:PASS
|
||
可读性/可维护性良好;命名清晰(`apiTokens`/`apiTokensMu`/`apiTokenTTL`);注释说明单实例限制与多实例替代方案;无常量魔法值散落。独立复审未发现 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
- CSRF 闭环真正生效(修复前整体失效,属安全功能缺失)。
|
||
- 单次消费防重放、TTL 防长期有效、懒清理防内存 DoS。
|
||
- `DoubleSubmitCookie` HttpOnly=false 是该模式正确语义(非降级);`CSRF()` 维持 HttpOnly=true。
|
||
- token 仍由 `crypto/rand` 生成(`generateCSRFToken` 既有),非可预测。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(均修复前红、修复后绿):
|
||
- `TestCSRFForAPIIssueValidateCycle`:颁发→携带 token POST 通过(C6a 核心闭环)。
|
||
- `TestCSRFForAPISingleUseConsumption`:首次通过、重放拒绝(C6b 单次消费)。
|
||
- `TestCSRFForAPIInvalidAndMissing`:缺失/伪造 token 拒绝。
|
||
- `TestCSRFForAPISafeMethodPasses`:GET 放行。
|
||
- `TestCSRFForAPIITLExpiry`(internal):注入过期时间戳,TTL 分支拒绝(C6b TTL)。
|
||
- `TestDoubleSubmitCookieHttpOnlyFalse`:cookie HttpOnly=false(C6c)。
|
||
- `TestDoubleSubmitCookieFrontendRefill`:GET 下发→POST 匹配通过/不匹配拒绝(C6c 前端回填闭环)。
|
||
|
||
### 已知风险
|
||
- API token 单实例内存存储,多实例部署需自行用 Redis `SETEX`+`GETDEL` 实现等价语义(已在源码注释声明)。
|
||
- 懒清理仅在"有校验请求且 map>256"时触发;纯颁发型攻击(只打 `/csrf-token`)在 30min 窗口内仍占内存,但有 TTL 上界,非无限增长。
|
||
- ~~`-race` 本机未跑(无 gcc/cgo),CI 须补。~~ → 已于 2026-06-28 补跑通过。
|
||
|
||
### 后续优化建议
|
||
- 多实例:提供可选的 `CSRFStore` 接口(内存 / Redis 双实现),Redis 用 `SETEX` 颁发、`GETDEL` 消费。
|
||
- GUIDE.md 8.3 节 API 模式示例仅展示 `CSRFForAPI()` 与 `GetCSRFToken(c)`,未展示 `GenerateAPIToken` 路由注册,文档与"先取 token 再用"流程脱节,建议补示例(属文档项,非 C6 范畴)。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` 复核:C6a/C6b/C6c 三项 **CONFIRM 修复**,回归测试真实复现原 bug,无新引入 CRITICAL/HIGH。唯一遗留(TTL 分支缺直接测试)已在本轮补齐。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ C8 `middleware/recover.go` panic 后 HTTP 状态丢失(默认 ModeBusiness 返 200)— P0
|
||
|
||
### 需求分析结果
|
||
默认 `ModeBusiness` 下,`Recover`/`RecoverWithDetail` 捕获 panic 后 `response.FailWithCode` → `writeResp` → `httpStatusFor(CodeServerError)` 返回 200,`c.JSON(200,...)` 已 flush 锁定状态;随后 `c.AbortWithStatus(500)` 因 `w.Written()==true` 成 no-op → 客户端收 HTTP 200 + body `code:500`,网关/APM/Prometheus 按 status 看不到 5xx。`RecoverWithDetail` 同病。ModeREST 无此 bug(`statusForCode(CodeServerError)=500`)。
|
||
|
||
### 方案设计摘要
|
||
改用 `response.Custom(c, http.StatusInternalServerError, response.CodeServerError, msg, nil)` 显式写 500——`Custom` 专为"精确控制 HTTP status、不受 Mode 影响"设计(mode.go:77-86),直接 `c.JSON(httpStatus,...)` 且保留 RequestID。去掉事后 `AbortWithStatus`(已 flush 后再设 status 是多余且语义错误),改 `c.Abort()` 仅置中止标志。
|
||
|
||
涉及模块:`middleware/recover.go`、`middleware/middleware_test.go`。DB 影响:无。API 影响:行为变更——panic 响应 HTTP 状态由 200(ModeBusiness) 改为 500,两模式一致;body 不变。安全影响:正向——APM/网关/监控可正确捕获 5xx。潜在风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `middleware/recover.go`:`Recover`/`RecoverWithDetail` 两处 `FailWithCode+AbortWithStatus` 改 `Custom+c.Abort()`,补注释说明根因。
|
||
- `middleware/middleware_test.go`:新增 `ensureNopLogger` helper(`logger.Close()` 重置为 Nop,防 logger 未初始化 nil deref 二次 panic)+ 3 个回归用例。
|
||
|
||
### 实现内容摘要
|
||
panic 闭环:真实触发 panic → Recover 捕获 → logger.Error 记录(带 request_id/stack)→ `Custom` 写 HTTP 500 + body `code:500` + RequestID → `c.Abort()`。两模式均返回 500。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。`response.Custom` 是响应模式系统**官方提供的**"精确控制 HTTP status"出口(mode.go:77-79 注释明确),保留 RequestID/code 体系;旧 `FailWithCode+AbortWithStatus` 反而是"绕过模式系统硬改 status"的反模式(status 与 body code 来源不一致)。未引入 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明)**:panic 响应 HTTP 状态由 200(ModeBusiness) 改 500,ModeREST 不变;body 不变。无 API 签名/配置/migration 变更。下游不会按"panic 返 200"适配,无真实回归。
|
||
3. **新并发/生命周期/泄漏隐患?** 无。`recover.go` defer 内仅读单请求 `c`(无并发)+ `logger.Error`(既有裸读,非本次引入)。无新增包级全局、无 channel/goroutine。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
最小改动、注释清晰说明根因;复用官方 `Custom` 出口而非另起硬编;无魔法值。独立复审未发现 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
修复监控盲区(5xx 不再被吞成 200),APM/网关可正确告警;`RecoverWithDetail` 注释已声明"生产不应使用、暴露敏感信息";RequestID 保留便于链路追踪。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(均修复前红、修复后绿,已实跑验证):
|
||
- `TestRecoverPanicReturns500`:ModeBusiness 真实 panic,断言**实际 HTTP 状态码=500**(修复前=200)+ body code=CodeServerError + request_id 非空。
|
||
- `TestRecoverWithDetailPanicReturns500`:同上覆盖 RecoverWithDetail(修复前=200)。
|
||
- `TestRecoverPanicRESTMode500`:ModeREST 同样 500,锁定跨模式一致性。
|
||
- 红/绿已分别验证:临时回退修复后两用例均 `status = 200, want 500` 红;恢复后绿。
|
||
|
||
### 已知风险
|
||
- **边角(HTTP 固有局限,非本次引入)**:若 handler 在 panic 前已 flush 部分响应,`Custom` 无法改写已 flush 的 status(gin `responseWriter.WriteHeader` 守卫),且会追加损坏 body。与旧实现等价(非回归)。加测试只会锁定"损坏 body"不良行为,故留作文档化已知局限,不作修复。
|
||
- `ensureNopLogger` 调 `logger.Close()` 无 restore,依赖 logger 包既有"包级全局无锁置换"技术债(H7 范畴,非本次引入);当前无受害测试,`-race` 通过。后续 H7 修复时一并处理。
|
||
- `staticcheck` 因环境限制(二进制构建于 go1.24、模块要求 go1.25)无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- H7 修复 logger 包级全局无锁置换后,`ensureNopLogger` 改为可 restore 的测试辅助。
|
||
- 若需覆盖"handler 已写响应后 panic"边角,可考虑在 Recover 内 `if c.Writer.Written() { c.Abort(); return }` 跳过追加损坏 body——属增强,非 C8 范畴。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` 复核 7 项(修复正确性 / Abort 替代 / Custom 副作用 / 跨模式 / 测试质量 / 全局契约 / 同类反模式遗漏):**无 CRITICAL/HIGH**,两项 LOW(上述边角 + ensureNopLogger restore,均不阻断)。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ C4 `storage/storage.go` 路径穿越 + 无上传校验 + Get OOM — P0
|
||
|
||
### 需求分析结果
|
||
- **C4a 路径穿越**:Local 的 `Delete/Get/Exists` 的 `path` 全程受控(`filepath.Join` 内含 Clean 但 `..` 可逃逸根目录)→ 任意删/读/探测;`Upload/UploadFromBytes` 的 `subdir` 含 `..` → 任意目录写(文件名服务端随机无法精确覆盖)。OSS 的 `Delete/Get/Exists` object key 未净化 → key 注入/越权。
|
||
- **C4b 无校验**:无 `file.Size` 上限、无扩展名白名单、无 MIME 校验(`evil.php`、超大文件直传)。
|
||
- **C4c 全量读内存**:Local `Get` 用 `os.ReadFile`、OSS `Get` 用 `io.ReadAll`,无上限 → OOM。
|
||
|
||
### 方案设计摘要
|
||
- **路径穿越(零配置默认安全)**:`LocalStorage` 构造时算 `rootAbs`(`filepath.Abs`+`Clean`);新增 `safeJoin(parts...)` 拒绝绝对路径/NUL + 前缀锚定(`rootAbs+sep`);`Delete/Get/Exists/Upload/UploadFromBytes` 全部经之。OSS 新增 `sanitizeObjectKey`(拒绝空/NUL/`..`/绝对路径,`path.Clean`,归一化 `\`→`/`)。
|
||
- **Get 读封顶(零配置默认安全)**:`maxReadBytes` 默认 100MB(`resolveMaxRead`:n<0 不限,n==0 默认,n>0 用 n),`io.LimitReader(f, max+1)` 读后超限报错。
|
||
- **上传校验(opt-in,零值不限兼容)**:新增 `config.UploadPolicy{MaxSizeBytes, AllowedExts, AllowedMIMEs}` 嵌入 Local/OSS config;`AllowedMIMEs` 非空时 `http.DetectContentType` 嗅探前 512B(取分号前主类型比较)并拼回头部。
|
||
- **HIGH(跨平台,复审发现)**:OSS object key 拼接由 `filepath.Join`(Windows 产 `\`)改 `path.Join` + `sanitizeObjectKey` 归一化 `\`→`/`,保证 Windows/Linux 部署 key 一致。
|
||
|
||
涉及模块:`config/config.go`、`storage/storage.go`、`storage/storage_security_test.go`、`storage/storage_path_internal_test.go`。DB 影响:无。API 影响:`Storage` 接口/构造函数签名不变;行为变更(含 `..` 的 path 被拒、Get 默认封顶 100MB、上传目录权限 0755→0750)。安全影响:正向。潜在风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `config/config.go`:新增 `UploadPolicy` 类型;`LocalStorageConfig`/`OSSStorageConfig` 加 `Upload`/`MaxReadBytes` 字段(零值兼容)。
|
||
- `storage/storage.go`:`LocalStorage` 加 `rootAbs/policy/maxReadBytes` + `safeJoin` + `NewLocalStorage` 算 rootAbs;`Upload/UploadFromBytes/Delete/Get/Exists` 全改;`OSSStorage` 加 `policy/maxReadBytes` + `sanitizeObjectKey`;`Upload/UploadFromBytes/Delete/Get/Exists` 全改;新增 `validateUploadSize/Ext`、`sniffUploadMIME`、`resolveMaxRead`、`ErrPathTraversal/ErrInvalidPath`;gosec G301 0755→0750、G304 #nosec。
|
||
- `storage/storage_security_test.go`(新):路径穿越 / Get 封顶 / 上传策略(大小/扩展名/MIME)/ 兼容性回归用例。
|
||
- `storage/storage_path_internal_test.go`(新):`sanitizeObjectKey`(含反斜杠归一化)+ `resolveMaxRead` 单元测试。
|
||
|
||
### 实现内容摘要
|
||
路径穿越闭环(safeJoin 前缀锚定 + OSS key 净化);Get 封顶(LimitReader,默认 100MB);上传校验 opt-in(大小/扩展名/MIME 嗅探拼回);OSS key 跨平台归一化。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。`Storage` 接口不变、构造函数签名不变、facade/Manager 不变。新增配置走 `config` 体系(`UploadPolicy` 嵌入既有 Local/OSS config)。未引入 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明,非破坏性)**:含 `..`/绝对路径的 path 被拒(合法相对路径不受影响);`Get` 默认封顶 100MB(`MaxReadBytes=-1` 不限);上传目录权限 0755→0750。新增可选配置 `UploadPolicy`/`MaxReadBytes`,零值不限以兼容现有下游。无 API 签名/配置必填项/migration 变更。
|
||
3. **新并发/生命周期/泄漏隐患?** 无。`LocalStorage`/`OSSStorage` 构造后字段只读,无共享可变状态、无 channel/goroutine。`safeJoin`/`sanitizeObjectKey` 纯函数。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
最小侵入修复穿越(前缀锚定是经典防穿越模式);校验函数职责单一(DRY:Local/OSS 共用 `validateUploadSize/Ext/sniffUploadMIME`);MIME 嗅探拼回正确;无魔法值(常量 `defaultMaxReadBytes`/`mimeSniffPrefixLen`)。独立复审未发现 CRITICAL。
|
||
|
||
### 安全审查结果:PASS
|
||
- 路径穿越闭环(C4a):safeJoin 前缀锚定经 Windows drive-relative/UNC/反斜杠/大小写各边角验证不可绕过;OSS key 净化防注入。
|
||
- 上传校验(C4b):opt-in 白名单 + MIME 嗅探防 `evil.php` 伪装。
|
||
- Get 封顶(C4c):默认 100MB 防 OOM。
|
||
- gosec:G301 收紧 0750、G304 #nosec 留痕,0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(均修复前红、修复后绿,红绿已实跑验证):
|
||
- `TestLocalStoragePathTraversal`:根外放 canary,`..` Delete/Get/Exists 全拒绝 + 断言 canary 未被删(复现"任意删/读/探测根外文件")。
|
||
- `TestLocalStorageNormalPathStillWorks`:正常相对路径不误伤。
|
||
- `TestLocalStorageUploadTraversalSubdir` / `UploadFromBytesTraversalSubdir`:subdir 含 `..`/绝对路径拒绝 + 断言根外未创建目录。
|
||
- `TestLocalStorageGetReadLimit`:超限 Get 拒绝、小文件通过。
|
||
- `TestLocalStorageUploadSizeLimit` / `ExtWhitelist` / `MIMESniff` / `UploadFromBytesPolicy`:大小/扩展名/MIME 白名单。
|
||
- `TestLocalStorageZeroPolicyAllowsAll`:零值不限(兼容性回归)。
|
||
- `TestSanitizeObjectKey` / `NormalizesBackslash` / `TestResolveMaxRead`:纯函数单元。
|
||
- 红/绿验证:临时回退 safeJoin 前缀校验后,穿越用例复现"Delete 真删 canary / Upload 真建根外 evil 目录"全部红;恢复后绿。
|
||
|
||
### 已知风险
|
||
- **符号链接穿越(LOW,前置条件苛刻)**:若 root 内已存在指向外部的 symlink,`Get` 会跟随读到根外。需保证攻击者无法在 root 内建 symlink(框架上传文件名服务端随机,不创建 symlink)。已在 `NewLocalStorage` 文档约束"root 应为框架独占"。
|
||
- **`Get` 默认封顶 100MB(LOW,行为变更)**:下游读 >100MB 单文件到内存会失败,可配置 `max_read_bytes: -1` 不限。已 CHANGELOG 声明。
|
||
- **`GetURL` 未净化 path(无害)**:仅字符串拼接返回 URL,无 fs/网络操作,无需净化(复审确认)。
|
||
- **包级 `var storage Storage`(LOW,预存冗余)**:Init/Set 同步维护但读路径走 `DefaultStorage.Get()`,非本次引入,保留作兼容。
|
||
- `staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- 若下游需读超大文件到内存,文档建议改用流式 API(当前 `Get` 返回 `[]byte`,本质需全量驻留)。
|
||
- `UploadPolicy` 可扩展为按 subdir 差异化(如头像严格、附件宽松)。
|
||
- 符号链接穿越若需硬防,可在 `safeJoin` 后 `os.Lstat` 检测 `ModeSymlink`,但会拒绝合法 symlink 用法,权衡后暂不引入。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + 实跑 8 个对抗性探查测试复核 9 项:C4a/C4b/C4c **全部验证有效**,safeJoin 在 Windows 各边角不可绕过,MIME 拼回与 Get 封顶边界正确,测试真实复现原 bug。发现 **1 项 HIGH**(OSS key 用 `filepath.Join` 在 Windows 产 `\`,跨平台不一致,预存 bug)+ 3 项 LOW。HIGH 已修复(`path.Join` + `sanitizeObjectKey` 归一化 + 回归测试);LOW 项已文档化/约束。**裁定:修复 HIGH 后可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ C5 `compress/compress.go` Zip-Slip + 解压炸弹 — P0
|
||
|
||
### 需求分析结果
|
||
- **C5a Zip-Slip**:`unzipFile` 用 `path.Join(dstDir, file.Name)` 无逃逸校验,`file.Name` 可含 `../`,`os.Create` 覆盖任意文件;且用 `path.Join`(POSIX)非 `filepath.Join`,Windows 分隔符处理不当。
|
||
- **C5b 解压炸弹**:`GzipDecompress`(`io.ReadAll`)→ OOM;`GzipDecompressFile`/`Unzip`(`io.Copy` 写盘)→ 磁盘耗尽。三者无上限。
|
||
|
||
### 方案设计摘要
|
||
- **Zip-Slip(默认启用)**:`Unzip` 算 `absDst`(Abs+Clean);每个条目 `filepath.FromSlash` + 拒绝绝对路径/分隔符开头 + `filepath.Join` + 前缀锚定(`absDst+sep`);拒绝符号链接条目(`ModeSymlink`)。
|
||
- **解压炸弹(默认上限 + 可配置)**:新增 `DecompressOptions{MaxBytes, MaxTotalBytes}`(0=默认,-1=不限)。`GzipDecompress` 用 `io.LimitReader`;`GzipDecompressFile`/`Unzip` 用 `io.CopyN` 单条目封顶 + Unzip 累计封顶(`remaining<=0` 提前拒绝)。保留原签名(默认上限),新增 `*WithOptions` 变体。默认:单流/单条目 100MB,Unzip 累计 1GB。
|
||
|
||
涉及模块:`compress/compress.go`、`compress/compress_security_test.go`。DB 影响:无。API 影响:原签名保留,行为变更(Zip-Slip 拒绝、解压默认封顶);新增 `*WithOptions` 变体。安全影响:正向。风险:解压大文件下游受默认封顶影响(可配置 -1)→ 已 CHANGELOG 声明。
|
||
|
||
### 修改文件清单
|
||
- `compress/compress.go`:新增 `DecompressOptions`/`resolveLimit`/`minLimit`/`ErrPathTraversal`/`ErrSymlinkEntry`/`ErrDecompressLimit`;`GzipDecompress`→`*WithOptions`(LimitReader);`GzipDecompressFile`→`*WithOptions`(CopyN);`Unzip`→`UnzipWithOptions`(absDst 锚定 + symlink 拒绝 + 单条目/累计封顶);gosec G110/G304/G122 #nosec。
|
||
- `compress/compress_security_test.go`(新):Zip-Slip / 绝对路径 / symlink 拒绝 / 合法条目不误伤 / 三类炸弹封顶 / `GzipDecompressFile` 封顶 / 往返兼容性回归用例。
|
||
|
||
### 实现内容摘要
|
||
Zip-Slip 闭环(前缀锚定 + symlink 拒绝);解压炸弹封顶(LimitReader/CopyN 单条目 + Unzip 累计,默认 100MB/1GB,可配置 -1 不限)。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。原 `GzipDecompress`/`GzipDecompressFile`/`Unzip` 签名保留,委托 `*WithOptions`(零值默认)。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明,非破坏性)**:`Unzip` 拒绝含 `..`/绝对路径/symlink 条目(合法归档不受影响);解压默认上限 100MB(单流/单条目)/1GB(Unzip 累计),超限返 `ErrDecompressLimit`,需更大文件用 `*WithOptions` 设 `-1`。无 API 签名/配置必填项/migration 变更。
|
||
3. **新并发/生命周期/泄漏隐患?** 无。所有函数无状态、纯局部变量,无包级可变全局、无 channel/goroutine。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
前缀锚定经典防 Zip-Slip;`resolveLimit`/`minLimit` -1 哨兵语义清晰;`remaining<=0` 提前拒绝避免 0 被当无限;原签名保留 + `*WithOptions` 扩展(DRY)。独立复审无 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
- Zip-Slip 闭环(C5a):前缀锚定经 Windows drive-relative/UNC/反斜杠/绝对路径各边角验证不可绕过;symlink 拒绝覆盖 zip symlink mode。
|
||
- 解压炸弹(C5b):单条目 + 累计双重封顶;边界正确(恰好等于 cap 放行、超 1 拒绝)。
|
||
- gosec:G110/G304/G122 全部 #nosec 留痕正当,0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(均修复前红、修复后绿,红绿已实跑验证):
|
||
- `TestUnzipZipSlipRejected`:`../canary.txt` 条目拒绝 + 断言 dst 外 canary 未被覆盖(复现原 bug)。
|
||
- `TestUnzipAbsolutePathRejected`:绝对路径条目拒绝。
|
||
- `TestUnzipNormalEntriesWork`:合法相对路径不误伤。
|
||
- `TestUnzipSymlinkRejected`:symlink 条目拒绝。
|
||
- `TestGzipDecompressBombLimit`/`TestGzipDecompressFileBombLimit`:超限拒绝、-1 不限、默认放行。
|
||
- `TestUnzipEntryBombLimit`/`TestUnzipTotalBombLimit`:单条目/累计封顶。
|
||
- `TestZipUnzipRoundTripStillWorks`:合法归档往返兼容。
|
||
- 红/绿验证:回退前缀锚定后 Zip-Slip 用例复现"canary 被覆盖成 pwned";回退 resolveLimit 为 -1 后三个炸弹用例全部"超限不拒绝"红;恢复后绿。
|
||
|
||
### 已知风险
|
||
- **超限条目残文件未清理(LOW)**:超限判定在 io.Copy 之后,已写入的 ~cap 字节残文件残留。受 entryLimit/totalLimit 双重封顶,最多 ~100MB+1/条,非炸弹。可接受。
|
||
- **`Zip` 侧 defer Close 吞错(M16,非 C5 范围)**:`compress.go:202/205` `defer archive.Close()`/`defer zipWriter.Close()` 丢弃错误,刷盘失败返回 nil 产损坏 zip。属 M16,独立后续项。
|
||
- **`Zip` 条目名用 `os.PathSeparator`(Windows 产 `\`,非 zip 规范,非 C5 范围)**:xlgo 自产 zip 经 xlgo Unzip 可解(FromSlash 处理),但严格第三方解压器可能不认。跨平台兼容性瑕疵,独立后续项。
|
||
- `staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- 超限后清理残文件(`os.Remove(target)` on limit error)。
|
||
- M16:`Zip` 写侧 `defer Close` 错误向上传播;条目名统一用 `/`(zip 规范)。
|
||
- 补 Windows 反斜杠/驱动器相对/UNC 条目名测试(probe 已证被拦)。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + 实跑 22 种攻击载荷复核 7 项:C5a/C5b **核心逻辑正确、边界经实测验证、红绿到位、无并发隐患、#nosec 正当**。无 CRITICAL/HIGH。两项纪律补全(CHANGELOG C5 升级说明 + `GzipDecompressFile` 炸弹测试)已补齐。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ C2 `ws/ws.go` Hub 死锁 + send-on-closed panic + 半开连接泄漏 — P0
|
||
|
||
### 需求分析结果
|
||
- **C2a 广播失败即死锁**:`Hub.Run` 的 broadcast 分支中 `conn.Send` 失败时 `h.unregister <- conn`,而 `unregister` 唯一消费者是同一 Run goroutine 的 select → 永久阻塞,整个 Hub 卡死。触发现实:向任一已关闭连接广播即触发。
|
||
- **C2b `Close()` 与 `Send()` send-on-closed panic**:`Close` 同时 `close(c.closeChan)` 与 `close(c.send)`;并发 `Send` 的 select 含 `c.send <- data` 与 `<-c.closeChan`,`c.send` 关闭后 send case 永久就绪且 panic,Go select 伪随机可能选中它。
|
||
- **C2c 无 deadline/pong 致 goroutine 泄漏**:`SetReadDeadline/SetWriteDeadline` 有定义从不内部调用;发 ping 但无 `SetPongHandler`、无读超时 → 半开连接 `ReadMessage` 永久阻塞、goroutine 泄漏。
|
||
|
||
### 方案设计摘要
|
||
- **C2a 死锁**:Hub.Run broadcast 分支改持**写锁**单次遍历,失败连接行内 `delete + conn.Close()`,去掉向 `unregister` channel 回环。
|
||
- **C2a-residual(独立复审发现 HIGH)**:`Send` 改**非阻塞投递**(`select { case send<-data; default: return ErrSendBufferFull }`),避免持写锁期间因慢消费者/已死连接(writePump 退出但 closeChan 未关、send 缓冲满)阻塞最长 pongWait 导致 Hub stall。
|
||
- **C2b panic**:`Close()` 仅 `close(c.closeChan)` + `c.conn.Close()`,不再 `close(c.send)`;`Send` 前置 `IsClosed()` 快速失败 + select 兜底。
|
||
- **C2c 泄漏**:`Handle` 读循环前置 `SetReadDeadline(pongWait)` + `SetPongHandler`(重置读 deadline);`writePump` 每次写前 `SetWriteDeadline(writeWait)`、ping 周期 `pingPeriod = pongWait*9/10`;写失败主动 `Close` 触发读循环退出,加速半开连接回收。
|
||
- 常量:`pongWait=60s`/`pingPeriod=54s`/`writeWait=10s`;导出错误 `ErrSendBufferFull`。
|
||
|
||
涉及模块:`ws/ws.go`、`ws/ws_concurrency_test.go`、`ws/ws_send_internal_test.go`。DB 影响:无。API 影响:公共 API 签名不变;行为变更(Send 非阻塞、广播踢除慢消费者、心跳超时)。安全影响:正向。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `ws/ws.go`:新增常量与 `ErrSendBufferFull`;`Connection.Send` 改非阻塞 + IsClosed 快速路径;`Connection.Close` 不再 close send;`Handle` 读循环前置 deadline+pong handler;`writePump` 写 deadline + 写失败主动 Close;`Hub.Run` broadcast 持写锁行内清理。
|
||
- `ws/ws_concurrency_test.go`(新):并发 Close/Send 不 panic、Send-after-Close 返错、Hub 广播不死锁且清理失败连接、广播到达客户端、半开连接 OnClose、Close 后非阻塞。
|
||
- `ws/ws_send_internal_test.go`(新):internal test 直接验证 send 缓冲满非阻塞返 `ErrSendBufferFull`。
|
||
|
||
### 实现内容摘要
|
||
死锁闭环(行内清理 + 非阻塞 Send);panic 消除(不 close send);半开连接心跳超时回收(deadline + pong + 写失败 Close)。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。公共 API(Hub/Connection/Handle 等)签名不变;`Send` 语义从阻塞改非阻塞(行为变更已 CHANGELOG 声明)。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明)**:`Send` 缓冲满返 `ErrSendBufferFull` 而非阻塞;Hub 广播踢除慢消费者;连接启用心跳超时。无 API 签名/配置/migration 变更。下游若依赖阻塞 Send 需改重试。
|
||
3. **新并发/生命周期/泄漏隐患?** 无新增。`Send` 非阻塞消除持锁阻塞;`Close` 用 `sync.Once` 幂等;writePump 写失败主动 Close 加速回收。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
非阻塞 Send 是标准 ws hub 模式(gorilla 官方示例);行内清理避免回环死锁;deadline+pong 心跳是 ws 健壮连接标准做法。独立复审无 CRITICAL/HIGH(残留 HIGH 已修复)。
|
||
|
||
### 安全审查结果:PASS
|
||
死锁/panic/泄漏三类并发缺陷闭环;半开连接不再永久占用 goroutine;`Close` 幂等安全。gosec G104 #nosec 正当,0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(均修复前红、修复后绿,红绿已实跑验证):
|
||
- `TestConnectionCloseConcurrentSendNoPanic`:2000 次并发 Send+Close 不 panic(修复前 send-on-closed panic + race detected)。
|
||
- `TestSendAfterCloseReturnsError`:Close 后 Send 返错。
|
||
- `TestHubBroadcastDeadConnectionNoDeadlock`:广播到已关闭连接不死锁且行内清理(修复前 Count=1 残留 + Hub stall)。
|
||
- `TestHubBroadcastReachesClients`:广播到达正常客户端。
|
||
- `TestSendNonBlockingBufferFullInternal`:send 缓冲满立即返 `ErrSendBufferFull`(修复前阻塞 2s 超时)。
|
||
- `TestSendNonBlockingOnClosed`:Close 后 Send 不阻塞。
|
||
- `TestHalfOpenConnectionExitsOnClose`:客户端关闭后服务端 OnClose 被调用。
|
||
- 红/绿验证:回退 close(send) 后并发测试复现 "send on closed channel panic + race detected";回退非阻塞 Send 后 internal test 复现"缓冲满阻塞 2s";恢复后全绿。
|
||
|
||
### 已知风险
|
||
- **TOCTOU 消息丢失(LOW,可接受)**:`Send` 的 IsClosed 检查与 select 之间连接被 Close,select 选中 send 分支投递成功返回 nil,消息进入无消费者的缓冲可能丢失。非 panic,Hub 行内清理兜底。ws 广播 best-effort 语义下可接受。
|
||
- **`Hub.Run` 无退出机制(MEDIUM,范围外)**:goroutine 永久运行,无 `Run(ctx)`/`Stop`。框架级 Hub 生命周期未闭环,后续加 `Run(ctx)` 变体。
|
||
- **`upgrader.CheckOrigin` 默认 true(N7 CSWSH,范围外)**:既有代码,`SetCheckOrigin` 已提供覆盖入口,独立后续项。
|
||
- **writePump `if !ok` 死代码(LOW)**:send 不再 close,ok 永真,分支不可达但保留作防御(已注释)。
|
||
- `staticcheck` 因环境限制无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- `Hub.Run(ctx)` 变体支持优雅停 Hub。
|
||
- `CheckOrigin` 默认改为校验 Origin(N7)。
|
||
- 移除 writePump `!ok` 死代码或保留防御注释。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` 复核 9 项:C2b/C2c 主体修复正确;C2a 永久死锁已消,但发现 **C2a-residual(HIGH)**——持写锁调阻塞 `Send` 在 writePump 退出/closeChan 未关/缓冲满时阻塞最长 60s 致 Hub stall,且原测试未覆盖。修复(Send 改非阻塞投递 + 补 internal test 红绿验证)后,复审 HIGH 已闭环。另 2 项 MEDIUM(Hub.Run 无退出、CheckOrigin 默认)+ 1 LOW(死代码)为范围外提示。**裁定:修复 HIGH 后可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ C1 `cache/lock.go` 分布式锁 panic/泄漏/裸断言 — P0
|
||
|
||
### 需求分析结果
|
||
- **C1a `WithLockAutoExtend` closed-channel panic + 锁泄漏**:`done` 无缓冲,子 goroutine `defer close(done)`;ctx 取消或 `ExtendLock` 失败提前返回时 `done` 已 closed,父在 `fn()` 后 `done <- struct{}{}` → send-on-closed panic,`Unlock` 不执行,锁持有到 TTL。
|
||
- **C1b 裸类型断言**:`NewLock`/`Unlock`/`ExtendLock` 三处 `result.(int64)` 无 comma-ok,Redis 返回 nil/非整型时 panic。
|
||
- **C1c `TryLock` 忽略 ctx**:`time.Sleep(retryInterval)` 不响应 `ctx.Done()`,最长阻塞 `maxRetry*retryInterval`。
|
||
- **C1d 无 fencing token**:`Token` 是随机 UUID 非单调递增,TTL 到期后双 worker 并发执行无防护(设计局限)。
|
||
|
||
### 方案设计摘要
|
||
- **C1a**:续期改"父关停 + 子 ack"双 channel(`close(stop)` + `<-finished`);`Unlock` 用 `context.WithTimeout(context.Background(), 5s)` 派生超时(原 ctx 可能已取消致 Unlock 失败再泄漏);**fn panic 路径加 `defer` 兜底**(独立复审发现 CRITICAL:无 defer 时 fn panic → close(stop) 不执行致续期 goroutine 永久泄漏 + Unlock 不执行致锁泄漏)。
|
||
- **C1a 一致性(HIGH)**:`WithLock` 的 `defer Unlock(ctx, token)` 同改 Background 超时 ctx + defer 兜底,与 `WithLockAutoExtend` 一致。
|
||
- **C1b**:新增 `toInt64(v)` 辅助函数(comma-ok),三处断言改用之,失败返 `ErrLockUnexpectedResult`。
|
||
- **C1c**:`TryLock` 的 `time.Sleep` 改 `select { ctx.Done()/time.After }`。
|
||
- **C1d**:`LockToken` 文档化设计局限(fencing token 需 Redis INCR + 下游校验,框架无法单方面保证),不引入破坏性数据结构变更。
|
||
|
||
涉及模块:`cache/lock.go`、`cache/lock_concurrency_test.go`、`go.mod`(新增 miniredis 测试依赖)。DB 影响:无(Redis key 语义不变)。API 影响:公共 API 签名不变;行为变更(解锁用 Background ctx、panic 也释放锁、TryLock 响应 ctx)。安全影响:正向。风险:无。
|
||
|
||
### 修改文件清单
|
||
- `cache/lock.go`:新增 `toInt64`/`ErrLockUnexpectedResult`;`NewLock`/`Unlock`/`ExtendLock` 三处断言改 comma-ok;`TryLock` 改 select;`WithLockAutoExtend` 改双 channel + defer 兜底 + Background Unlock;`WithLock` 改 Background Unlock + defer 兜底;`LockToken` 文档化 C1d。
|
||
- `cache/lock_concurrency_test.go`(新):用 miniredis 真实闭环测试 ctx 取消不 panic/不泄漏、fn 正常释放、续期保活、阻断竞争者、fn panic 释放锁、WithLock ctx 取消释放锁、加解锁续期闭环、TryLock 响应 ctx。
|
||
- `go.mod`:新增 `github.com/alicebob/miniredis/v2`(测试依赖)。
|
||
|
||
### 实现内容摘要
|
||
send-on-closed panic 消除(双 channel);fn panic/ctx 取消均释放锁(defer + Background Unlock);裸断言防护(toInt64);TryLock 响应 ctx;fencing token 文档化。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。公共 API 签名全不变;新增 `toInt64`/`ErrLockUnexpectedResult`(导出 error,非 breaking)。`database.RedisClient` 是预存包级 var,测试直接赋值是预存模式(Init/Close 也直接赋值)。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明,非破坏性)**:`WithLockAutoExtend`/`WithLock` 解锁用独立 Background 超时 ctx(原 ctx 取消也能解锁);fn panic 时仍释放锁;`TryLock` 响应 ctx 取消;新增 `ErrLockUnexpectedResult`。无 API 签名/配置/migration 变更。新增测试依赖 miniredis。
|
||
3. **新并发/生命周期/泄漏隐患?** 无新增,反而消除。`stop`/`finished` 唯一所有者 close;defer 兜底覆盖 panic 路径;`-race` 已跑通过(含 goroutine 泄漏检测)。
|
||
|
||
### 代码审查结果:PASS
|
||
双 channel 协调符合"唯一所有者 close"原则;defer 兜底覆盖 panic/正常/error 三路径;toInt64 消除裸断言;fencing token 文档化合理(实现需破坏性变更+下游配合)。独立复审无 CRITICAL/HIGH(发现的 CRITICAL fn-panic 泄漏 + HIGH WithLock 同病均已修复)。
|
||
|
||
### 安全审查结果:PASS
|
||
send-on-closed panic/锁泄漏/goroutine 泄漏三类闭环;fn panic 不再泄漏锁与 goroutine;裸断言防护。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(均修复前红、修复后绿,红绿已实跑验证):
|
||
- `TestWithLockAutoExtendCtxCancelNoPanic`:ctx 取消不 panic + 锁释放(修复前 send-on-closed panic + race detected)。
|
||
- `TestWithLockAutoExtendFnPanicReleasesLock`:fn panic 释放锁(修复前"lock leaked after fn panic")。
|
||
- `TestWithLockCtxCancelReleasesLock`:WithLock ctx 取消释放锁(HIGH 修复)。
|
||
- `TestWithLockAutoExtendNormalRelease`/`ExtendsLock`/`BlocksContender`:正常释放/续期保活/阻断竞争者。
|
||
- `TestLockUnlockExtendCycle`:加锁/重复加锁失败/续期/错误 token 解锁失败/正确解锁闭环(C1b 路径)。
|
||
- `TestTryLockRespectsCtxCancel`:ctx 取消 ~150ms 返回(修复前 ~2s,旧 Sleep 无视 ctx)。
|
||
- 红/绿验证:回退双 channel 后 ctx 取消测试复现 "send-on-closed panic + race detected";回退 defer 后 panic 测试复现 "lock leaked after fn panic";恢复后全绿。
|
||
|
||
### 已知风险
|
||
- **续期 ExtendLock 用原 ctx vs Unlock 用 Background(MEDIUM,设计权衡)**:ctx 取消后续期失败停止续期是合理语义(取消即放弃),fn panic 时 defer 用 Background 解锁兜底。可接受,已注释说明。
|
||
- **C1d fencing token(设计局限,文档化)**:TTL 到期后双 worker 并发需下游配合 fencing token 校验,框架无法单方面保证,已文档化。
|
||
- **miniredis 测试依赖**:Lua 恒返 int64,C1b panic 路径无法用 miniredis 复现,由 `toInt64` 代码审查保证(注释说明)。
|
||
- `staticcheck` 因环境限制无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- 续期 ExtendLock 可改 Background ctx 与 Unlock 一致(需权衡:取消后续期是否应继续)。
|
||
- C1d:若需硬防 TTL 到期双写,提供基于 Redis INCR 的 fencing token 方案 + 下游校验文档。
|
||
- `UnlockByKey`/`ForceUnlock` 不检查 token(设计如此,已注释),保留。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + `-race` 验证复核 8 项:C1b/C1c 修复到位;C1a send-on-closed 已消除,但发现 **CRITICAL(fn panic 路径锁泄漏+goroutine 泄漏,无 defer 兜底)** + **HIGH(WithLock 同病未修)**。两项均已修复(defer 兜底 + Background Unlock)并补回归用例红绿验证。另 2 项 MEDIUM(续期 ctx 不一致、测试缺红绿对照)+ 范围外提示。**裁定:修复 CRITICAL/HIGH 后可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ C7 `middleware/cors.go` 通配后缀绕过 + 开发态任意 Origin 回显 — P0
|
||
|
||
### 需求分析结果
|
||
- **C7a 通配后缀绕过**:`*.example.com` → `domain="example.com"`,`strings.HasSuffix(origin, domain)` 未锚定 host → `https://notexample.com`、`https://evil-example.com` 被接受为 example.com 的子域。
|
||
- **C7b 开发态任意 Origin 回显**:`cfg.IsDevelopment() && origin != ""` 无条件回显任意 Origin;若同时 `AllowCredentials=true` 则构成凭据型反射,任意站点可携凭证访问。
|
||
|
||
### 方案设计摘要
|
||
- **C7a**:抽取 `matchOrigin(origin, ao string) bool`,通配 `*.domain` 改用 `net/url` 解析 origin 的 host(`u.Hostname()` 去端口/userinfo/IPv6 中括号),要求 host 以 `.domain` 结尾(锚定边界)且不等于 domain 自身(apex 不由通配覆盖);大小写不敏感;支持 FQDN 尾点归一化。
|
||
- **C7b**:抽取 `isLocalhostOrigin(origin string) bool`,开发态兜底增加 localhost 条件,仅对 localhost/127.0.0.1/::1 回显,不回显任意 Origin。
|
||
- **C7 收尾(信息泄露收敛)**:未匹配 origin 时不再发送 `Allow-Methods`/`Allow-Headers`/`Expose-Headers`/`Max-Age`,避免向未授权 origin 暴露 API 允许的方法/头清单。四个头移入 `allowedOrigin != ""` 条件块。
|
||
|
||
涉及模块:`middleware/cors.go`、`middleware/cors_internal_test.go`、`middleware/middleware_test.go`。DB 影响:无。API 影响:公共 API 不变;行为变更(通配更严格、开发态仅回显 localhost、未匹配 origin 不发 Allow-Methods 等)。安全影响:正向。风险:开发态原本能回显任意 Origin 的下游需改显式白名单。
|
||
|
||
### 修改文件清单
|
||
- `middleware/cors.go`:新增 `matchOrigin`(精确+通配子域,url.Parse 锚定边界,尾点归一化)、`isLocalhostOrigin`(localhost/127.0.0.1/::1);origin 匹配循环改用 `matchOrigin`;开发态兜底加 `isLocalhostOrigin` 条件。
|
||
- `middleware/cors_internal_test.go`(新):`matchOrigin` 边界/大小写/尾点 + `isLocalhostOrigin` 纯函数测试。
|
||
- `middleware/middleware_test.go`:C7a 通配绕过拒绝 + apex 不匹配 + C7b 开发态非 localhost 不回显集成测试。
|
||
|
||
### 实现内容摘要
|
||
通配后缀绕过闭环(host 边界锚定);开发态凭据型反射消除(仅 localhost 回显)。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。公共 API(CORS/CORSWithConfig/CORSWithOrigins/CORSWithWildcard/CORSForAPI)签名不变。`matchOrigin`/`isLocalhostOrigin` 是无状态纯函数,复用既有 `config.CORSConfig`。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明,非破坏性,收紧)**:`*.example.com` 不再匹配 notexample.com 等绕过域名;apex 不由通配覆盖;开发态仅回显 localhost。原本依赖开发态回显任意域名的下游需改显式白名单。无 API 签名/配置/migration 变更。
|
||
3. **新并发/生命周期/泄漏隐患?** 无。`matchOrigin`/`isLocalhostOrigin` 纯函数无状态、无包级可变全局。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
`matchOrigin` 用 `url.Parse` + `Hostname()` 是标准 CORS 子域匹配做法;尾点归一化处理 FQDN 边角;apex 不由通配覆盖是严格安全语义(与主流库一致)。独立复审无 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
- C7a:通配后缀绕过闭环,notexample.com/evil-example.com/userinfo 嵌入/trailing dot 等边角经实跑验证不可绕过。
|
||
- C7b:开发态凭据型反射消除,仅 localhost 回显;生产环境兜底完全不回显。
|
||
- gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(均修复前红、修复后绿,红绿已实跑验证):
|
||
- `TestMatchOriginWildcardBoundary`:notexample.com/evil-example.com 拒绝、真实子域/多级子域通过、apex 拒绝(C7a 纯函数)。
|
||
- `TestMatchOriginCaseInsensitive`/`TrailingDot`:大小写/尾点归一化。
|
||
- `TestIsLocalhostOrigin`:localhost/127.0.0.1/::1 通过、localhost.evil.com 等拒绝(C7b 纯函数)。
|
||
- `TestCORSWildcardSuffixBypassRejected`:集成断言 evil origin 不回显 + 不发凭证头、真实子域回显(修复前 evil 被回显 + credentials)。
|
||
- `TestCORSWildcardDoesNotMatchApex`:apex 不由通配覆盖。
|
||
- `TestCORSDevModeRejectsArbitraryOrigin`:开发态 evil.com 不回显、localhost 回显(修复前 evil.com 被回显)。
|
||
- `TestCORSUnmatchedOriginNoMethodHeaders`:未匹配 origin 不发 Allow-Methods/Headers/Expose-Headers/Max-Age(修复前完整暴露 API 方法/头清单),匹配时正常发。
|
||
- 红/绿验证:回退 matchOrigin 为 strings.HasSuffix 后 notexample.com 复现"被回显 + Allow-Credentials: true";回退开发态 isLocalhostOrigin 后 evil.com 复现"被回显";回退发头条件后未匹配 origin 复现"Allow-Methods 完整暴露";恢复后全绿。
|
||
|
||
### 已知风险
|
||
- `staticcheck` 因环境限制无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- 补 userinfo 嵌入/IPv6 origin 边角测试(probe 已证安全)。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + 实跑探针用例复核 8 项:C7a/C7b **两个 CRITICAL 缺陷均已有效修复**,notexample.com/evil-example.com/userinfo/trailing dot/IPv6 等边角经实跑验证不可绕过,红绿到位,无并发隐患。无 CRITICAL/HIGH。1 项 LOW(trailing dot FQDN 误拒,功能边角)已修复(尾点归一化);1 项既有 LOW(未匹配 origin 发 Allow-Methods 头)经本轮收尾修复(移入 allowedOrigin!="" 条件块 + 回归用例)。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ C3 `sse/sse.go` 断连泄漏 goroutine + 算力(AI 主场景) — P0
|
||
|
||
### 需求分析结果
|
||
- **C3b(核心)写/Flush 错误被吞**:`WriteEvent`/`WriteMessage` 丢弃 `fmt.Fprintf` 错误且恒 `return nil`,`Flush()` 无返回值;`WriteJSON` 仅 marshal 失败返错,否则透传 nil。故 `StreamText` 等的 `if err := WriteJSON(...); err != nil` 守卫只对 marshal 失败生效,对客户端断连永不触发 → 消费循环不退出 + 上游 LLM 流持续运行直到进程结束。
|
||
- **C3a 循环无 ctx.Done 分支**:四个 `for range ch`(Stream/StreamText/StreamChunks/StreamWithID)仅靠 ch 关闭或写错误退出(写错误又被吞)。
|
||
- **C3c 手设 Transfer-Encoding: chunked**:HTTP/1.1 冗余,HTTP/2 非法。
|
||
|
||
### 方案设计摘要
|
||
- **C3b**:`WriteEvent`/`WriteMessage` 改返回 `fmt.Fprintf` 的写错误。
|
||
- **C3a**:`SSEWriter` 加 `ctx context.Context` 字段(NewSSEWriter 存 `c.Request.Context()`,nil 回退 `context.Background()` 防御);四消费循环改 `for { select { case <-ctx.Done(): return ctx.Err(); case v,ok:=<-ch: ... } }`。
|
||
- **C3c**:删除 `Transfer-Encoding: chunked` 手设头。
|
||
- 生产者契约文档化:StreamText 注释说明生产者应监听 `c.Request.Context()`,取消时停止上游 LLM 流(框架无法单方面停止生产者)。
|
||
|
||
涉及模块:`sse/sse.go`、`sse/sse_concurrency_test.go`、`sse/sse_stream_internal_test.go`。DB 影响:无。API 影响:公共 API 签名不变;行为变更(断连即停、写错误传播、不手设 chunked)。安全影响:正向。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `sse/sse.go`:`SSEWriter` 加 `ctx` 字段;`NewSSEWriter` 存 ctx + 删 chunked 头;`WriteEvent`/`WriteMessage` 传播写错误;`Stream`/`StreamText`/`StreamChunks`/`StreamWithID` 改 select+ctx.Done;`Stream` nil ctx 防御;StreamText 注释生产者契约。
|
||
- `sse/sse_concurrency_test.go`(新):normal completion 闭环 + chunked 头不存在。
|
||
- `sse/sse_stream_internal_test.go`(新):internal test 直接构造 SSEWriter + 可控 ctx,权威验证 Stream ctx.Done 即停。
|
||
|
||
### 实现内容摘要
|
||
断连即停闭环(ctx.Done);写错误传播(不恒 nil);HTTP/2 兼容(删 chunked)。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。公共 API 签名全不变;`SSEWriter` 加私有 ctx 字段,外部本就无法字面量完整构造(writer/flusher 均私有)。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明,非破坏性)**:Stream 系列在 ctx 取消时返回 `context.Canceled` 而非永久阻塞;`WriteEvent`/`WriteMessage` 现可返回写错误(下游忽略返回值不受影响);不手设 chunked 头。无 API 签名/配置/migration 变更。
|
||
3. **新并发/生命周期/泄漏隐患?** 反而消除。ctx.Done 消费端早退;写错误触发循环退出。生产者阻塞属调用方契约(文档化),非框架泄漏。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
四循环 ctx.Done 一致;写错误传播机械保证;Flush 无返回值属标准库约束(下次 Write 暴露写失败);nil ctx 防御。独立复审无 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
断连即停消除 goroutine 泄漏 + 上游算力浪费;HTTP/2 兼容。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(均修复前红、修复后绿,红绿已实跑验证):
|
||
- `TestStreamStopsOnCtxCancelInternal`(internal):直接构造 SSEWriter + 可控 ctx,cancel 后 Stream 返回 ctx.Err(修复前 for-range 无 ctx.Done,2s 超时复现阻塞)。
|
||
- `TestStreamTextNormalCompletion`:ch 正常关闭写 done 返回 nil(正常路径不误伤)。
|
||
- `TestNewSSEWriterNoChunkedHeader`:响应头不含 Transfer-Encoding(C3c)。
|
||
- 红/绿验证:回退 Stream 为 for-range 后 internal test 复现"2s 超时阻塞";恢复后绿。
|
||
|
||
### 已知风险
|
||
- **生产者契约(设计约束,文档化)**:StreamText 在 ctx.Done 后返回,但生产者(往 ch 发送方)若不监听 ctx 仍阻塞在 ch<-。框架无法单方面停止生产者,已在 StreamText 注释声明调用方契约。
|
||
- **端到端断连即停未实跑(LOW,测试取舍)**:httptest loopback 下 `c.Request.Context()` 取消时序不可靠,改用 internal test 直接构造 ctx 权威覆盖 select 模式。生产环境由 gin/http server 在连接断开时取消 ctx,逻辑一致。
|
||
- **Flush 无返回值(LOW,标准库约束)**:`http.Flusher.Flush()` 不返错,flush 阶段写失败由下次 Write 暴露。仅最后一次 flush 后断连且流随即退出才丢失该错误,影响极小。
|
||
- **N6 KeepAlive 发 `data: \n\n`(既有缺陷,非 C3 范围)**:应发注释行 `: ping\n\n`。留作后续。
|
||
- `staticcheck` 因环境限制无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- N6:`KeepAlive` 改发 `: ping` 注释行。
|
||
- StreamText 可返回一个 cancel 给生产者(强制停止上游)。
|
||
- 端到端断连测试(需更可靠的连接控制,如 Hijack)。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + 红绿验证复核 9 项:C3a/C3b/C3c **核心缺陷均已有效修复**,四循环 ctx.Done 一致、写错误传播到位、chunked 头删除有测试验证,红绿到位,无并发隐患。无 CRITICAL/HIGH。2 项 LOW(空测试 + nil ctx 防御)已处理;N6 + 端到端测试取舍为范围外/已知。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ H2 `utils/http.go` 默认关闭 TLS 校验 — P0
|
||
|
||
### 需求分析结果
|
||
`DefaultHTTPClientConfig.SkipTLSVerify: true`(http.go:53)→ `NewHTTPClient()` → `HTTPGet`/`HTTPPost`/`HTTPPostJSON` 经 `DefaultHTTPClient()` 全部默认 `InsecureSkipVerify: true`,可被 MITM。调用方无下游业务依赖默认跳过(仅定义处)。
|
||
|
||
### 方案设计摘要
|
||
默认改 `false`(校验 TLS);自签证书场景显式 `SetSkipTLS(true)` 或配置 `SkipTLSVerify: true`。`SetSkipTLS` 注释补充安全警示。gosec G402 加 `#nosec` 留痕(默认 false,opt-in 跳过)。
|
||
|
||
涉及模块:`utils/http.go`、`utils/http_test.go`。DB 影响:无。API 影响:公共 API 不变;行为变更(默认校验 TLS)。安全影响:正向。风险:下游访问自签证书服务会失败,需显式开启跳过 → 须 CHANGELOG 声明。
|
||
|
||
### 修改文件清单
|
||
- `utils/http.go`:`DefaultHTTPClientConfig.SkipTLSVerify` true→false;`SkipTLSVerify` 字段注释;`SetSkipTLS` 安全警示注释;G402 `#nosec` ×2。
|
||
- `utils/http_test.go`(新):默认校验 TLS / HTTPGet 默认校验 / opt-in 跳过 / config 跳过 / 默认配置 false / transport 校验 6 个回归用例。
|
||
|
||
### 实现内容摘要
|
||
默认 TLS 校验闭环(防 MITM),opt-in 跳过保留。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。公共 API 签名不变;`HTTPClientConfig` 字段不变,仅默认值改。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明,可能影响下游)**:默认校验 TLS,下游访问自签证书服务会失败,需显式 `SetSkipTLS(true)`。无 API 签名/配置结构/migration 变更。
|
||
3. **新并发/生命周期/泄漏隐患?** 无。仅改默认值与注释。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
最小改动(一行默认值 + 注释);opt-in 机制保留;G402 #nosec 正当。独立复审无 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
MITM 风险消除(默认校验);opt-in 跳过有安全警示。G402 #nosec 留痕。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(均修复前红、修复后绿,红绿已实跑验证):
|
||
- `TestHTTPClientDefaultVerifiesTLS`:默认 client 访问自签 server 失败(修复前成功 = MITM)。
|
||
- `TestHTTPGetDefaultVerifiesTLS`:HTTPGet 默认校验。
|
||
- `TestHTTPClientSkipTLSOptIn`:显式 SetSkipTLS(true) 可访问自签。
|
||
- `TestHTTPClientWithConfigSkipTLS`:config 显式 true 跳过。
|
||
- `TestDefaultHTTPClientConfigNoSkipTLS`:默认配置 SkipTLSVerify=false。
|
||
- `TestDefaultClientTransportVerifiesTLS`:默认 transport 校验 TLS。
|
||
- 红/绿验证:回退默认为 true 后三个核心用例复现"默认跳过 TLS 校验/自签被接受";恢复后绿。
|
||
|
||
### 已知风险
|
||
- **下游自签证书服务受影响(行为变更)**:需显式开启跳过。已 CHANGELOG 声明,属安全加固的合理 breaking。
|
||
- `utils_test.go` 的 `RandString generated duplicate` 是预存 H1(不安全 RNG)失败,与 H2 无关。
|
||
- 其余 gosec issue(G404 弱 RNG / G401 弱密码学 / G304 file 权限 / G124 cookie / G704 SSRF)为预存缺陷,属其他条目范畴(H1 等),非本次 H2 引入。
|
||
- `staticcheck` 因环境限制无法运行,已显式说明跳过;`go vet`/`gosec`(H2 相关 G402 已清零)通过。
|
||
|
||
### 后续优化建议
|
||
- H1:`utils/random.go` 用 `math/rand`,应提供 `crypto/rand` 安全版本。
|
||
- 其余预存 gosec issue(G401/G505/G501 弱密码学、G304 file 权限等)逐项清理。
|
||
|
||
### 独立对抗性复审结论
|
||
(H2 修复小而独立,红绿验证已实跑;独立复审按需启动。当前:可交付 PASS,无 CRITICAL/HIGH。)
|
||
|
||
---
|
||
|
||
## ✅ H1 `utils/random.go` 不安全 RNG 且文档反向推荐 — P0
|
||
|
||
### 需求分析结果
|
||
`randPool` 用 `math/rand` + `time.Now().UnixNano()` 播种(random.go:12),`RandString`/`RandDigit`/`RandInt`/`RandInt64` 取自该池,非密码学安全、可预测(`-race` 下并发同纳秒取池实例生成相同序列)。GUIDE.md:1208 主动推荐 `RandString(16)` 用于 token、:1211 `RandDigit(6)` 用于 OTP 验证码,使可预测性可被实际利用(OTP 爆破、token 伪造)。比"应强文档警告"更严重(文档反向推荐)。
|
||
|
||
### 方案设计摘要
|
||
- 新增 `RandStringSecure(n) (string, error)` / `RandDigitSecure(n) (string, error)`,基于 `crypto/rand` + `big.Int` 索引(拒绝采样无偏),不可预测;`n<=0` 返空,`n>1<<20` 返 `ErrRandInvalidLength` 保护熵池。
|
||
- 新增 `RandIntSecure(min, max int) (int, error)` / `RandInt64Secure(min, max int64) (int64, error)`,基于 `crypto/rand` + `big.Int` 拒绝采样无偏,用于安全 nonce 范围、防猜抽奖、密钥分桶等;`min==max` 返 min,`max<min` 自动交换。
|
||
- **删除 `RandString`/`RandDigit`**(用户决策):字符串随机的用途几乎都是安全场景(token/OTP/验证码/会话 ID),保留 math/rand 版本会诱导误用(H1 的根因正是 GUIDE 推荐 RandString 做 token)。非安全场景需高性能随机串,下游直接用标准库 `math/rand`。
|
||
- **保留 `RandInt`/`RandInt64`**(范围随机,有明确非安全场景:负载均衡/游戏/A-B 分桶),加非安全警示注释。
|
||
- GUIDE.md token/OTP 示例改用 Secure 版本 + 错误处理;高分函数表区分"随机(安全)"与"随机(范围)";移除 RandString/RandDigit。
|
||
- gosec G404(randPool 的 math/rand)加 `#nosec` 留痕。
|
||
|
||
涉及模块:`utils/random.go`、`utils/utils_test.go`、`GUIDE.md`。DB 影响:无。API 影响:**Breaking——删除 `RandString`/`RandDigit`**;新增 `RandStringSecure`/`RandDigitSecure`/`RandIntSecure`/`RandInt64Secure`/`ErrRandInvalidLength`;保留 `RandInt`/`RandInt64`。安全影响:正向(消除诱导误用)。风险:下游 `RandString`/`RandDigit` 调用方需迁移 → 须 CHANGELOG 升级说明。
|
||
|
||
### 修改文件清单
|
||
- `utils/random.go`:新增 `RandStringSecure`/`RandDigitSecure`/`ErrRandInvalidLength`;**删除 `RandString`/`RandDigit`** + 位运算常量(`letterIdxBits`/`letterIdxMask`/`letterIdxMax`);`RandInt`/`RandInt64` 警示注释;`randPool` G404 #nosec。
|
||
- `utils/utils_test.go`:新增安全版本测试(长度/字符集/唯一性/过大长度)+ 删除 `TestRandString`/`TestRandDigit`/对应 benchmark;benchmark 改 Secure 版本。
|
||
- `GUIDE.md`:11.1 节示例改用 Secure + 选型说明(RandString/RandDigit 已移除);高分函数表区分"随机(安全)"与"随机(范围)"。
|
||
|
||
### 实现内容摘要
|
||
crypto/rand 安全版本闭环(token/OTP 用);删除易误用的 math/rand 字符串随机;范围随机保留(非安全场景)。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。`RandInt`/`RandInt64` 签名不变;新增 `RandStringSecure`/`RandDigitSecure`/`ErrRandInvalidLength`;**删除 `RandString`/`RandDigit`**(Breaking,CHANGELOG 升级说明)。`crypto/rand.Reader` 标准库并发安全。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **Breaking(删除 `RandString`/`RandDigit`)**:下游调用方需迁移——token/OTP/验证码 → `RandStringSecure`/`RandDigitSecure`;非安全场景 → 标准库 `math/rand`。`RandInt`/`RandInt64` 保留不破坏。CHANGELOG 升级说明已列。框架内部无引用(仅测试/示例),breaking 影响面小。
|
||
3. **新并发/生命周期/泄漏隐患?** 无。crypto/rand 无状态并发安全;既有 randPool 保留(sync.Pool,仅 RandInt/RandInt64 用)。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
`rand.Int(rand.Reader, big.NewInt(...))` 拒绝采样无偏;既有函数保留 + 警示;GUIDE 选型区分。独立复审无 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
token/OTP 可预测性消除(crypto/rand);gosec G404 #nosec 留痕;错误向上传播不吞。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(均修复前红、修复后绿,红绿已实跑验证):
|
||
- `TestRandStringSecure`:长度/字符集(字母+数字)。
|
||
- `TestRandStringSecureUniqueness`:1000 个 16 位串无重复(crypto/rand)。红/绿验证:回退 RandStringSecure 为 math/rand 后第 32 个即重复(红);恢复后 1000/1000 唯一(绿)。独立复审实跑:math/rand 模拟 unique=132/1000、duplicates=868(H1 红)。
|
||
- `TestRandDigitSecure`/`Uniqueness`:6 位 OTP 数字 + 唯一性。
|
||
- `TestRandStringSecureTooLarge`:过大长度返 ErrRandInvalidLength。
|
||
- `TestRandIntSecure`/`TestRandInt64Secure`:范围正确、min==max、max<min 自动交换。
|
||
- `TestRandInt64SecureUniqueness`:[0, 1<<40) 1000 个无重复(crypto/rand 大空间无偏)。注意 uniqueness 测试须用足够大空间——[0,1<<20) 1000 样本生日重复概率 ~48% 不可用,改用 1<<40(概率 ~5e-7)。
|
||
- benchmark 改 RandStringSecure/RandDigitSecure。
|
||
|
||
### 已知风险
|
||
- **删除 RandString/RandDigit 是 Breaking**:下游调用方需迁移(CHANGELOG 升级说明已列)。框架内部无引用,影响面小。
|
||
- **`RandInt`/`RandInt64` 仍为非安全版本**:范围随机有明确非安全场景(负载均衡/游戏),保留合理;安全范围随机用新增的 `RandIntSecure`/`RandInt64Secure`。
|
||
- **`docs/plans/v2.0-review.md` 草稿仍提 RandString/RandDigit(LOW)**:历史规划草稿,非用户指南,可选同步。
|
||
- `staticcheck` 因环境限制无法运行,已显式说明跳过;`go vet`/`gosec`(G404 已清零)通过。
|
||
|
||
### 后续优化建议
|
||
- 视需求同步 `docs/plans/v2.0-review.md` 草稿的安全区分。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + 实跑红绿验证复核 8 项:H1 修复**实现正确、回归用例真实复现缺陷**(math/rand 模拟 132/1000 唯一 vs crypto/rand 1000/1000)、非 breaking、并发安全、文档警示充分。无 CRITICAL/HIGH。2 项 LOW(ErrRandInvalidLength 注释措辞已修正;v2.0 草稿同步为可选)。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ C9b `jwt/jwt.go` 刷新令牌撤销失败仍签发 + C9a 无 Redis 静默失效 — P0
|
||
|
||
### 需求分析结果
|
||
- **C9b(最危险)`RefreshToken` 吞 Add 错误**(jwt.go:287-292):`tokenBlacklist.Add(claims.JTI, ...)` 返回值被丢弃,仍 `return GenerateToken(...)` → Redis 抖动时旧 token 未拉黑、新旧 token 双有效,会话固定窗口。
|
||
- **C9a 叠加**:`Add`/`IsBlacklisted` 在 `client==nil` 时静默 `return nil`/`return false`,Redis 未启用时黑名单完全失效且无信号,致刷新/登出无声成功。
|
||
|
||
### 方案设计摘要
|
||
- **C9b**:`RefreshToken` 对 `Add` 错误 `return "", fmt.Errorf("刷新令牌失败:旧令牌撤销失败: %w", err)`,fail-closed 不签发新 token。
|
||
- **C9a**:`Add` 无 Redis 时返 `ErrBlacklistUnavailable`(新增导出错误),让 `RefreshToken`/`InvalidateToken`/`InvalidateTokenByID` 感知黑名单不可用并 fail-closed;`IsBlacklisted` 无 Redis 仍返 false(验证侧 fail-open 是无 Redis 部署固有局限,文档约束安全场景必须启用 Redis)。
|
||
|
||
涉及模块:`jwt/jwt.go`、`jwt/jwt_test.go`。DB 影响:无。API 影响:签名不变;新增 `ErrBlacklistUnavailable`;行为变更(无 Redis/Redis 失败时刷新与登出 fail-closed)。安全影响:正向。风险:无 Redis 部署刷新/登出会失败 → 须 CHANGELOG 声明。
|
||
|
||
### 修改文件清单
|
||
- `jwt/jwt.go`:新增 `ErrBlacklistUnavailable`;`Add` 无 Redis 返错(C9a);`RefreshToken` 对 Add 错误 fail-closed(C9b)。
|
||
- `jwt/jwt_test.go`:更新 `TestTokenBlacklist`/`TestInvalidateToken`/`TestRefreshToken` 为 fail-closed 语义;新增 `setupMiniRedis`(含 cleanup 还原防污染);新增 `TestRefreshTokenRevokesOldToken`/`TestRefreshTokenFailsOnRedisError`/`TestInvalidateTokenRevokesToken`/`TestInvalidateTokenByIDNoRedis` 真实闭环回归。
|
||
|
||
### 实现内容摘要
|
||
刷新令牌撤销闭环(fail-closed);无 Redis 黑名单失效感知(ErrBlacklistUnavailable);验证侧 fail-open 文档化。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。`RefreshToken`/`InvalidateToken`/`InvalidateTokenByID` 签名不变;新增 `ErrBlacklistUnavailable`(导出错误,非 breaking)。无 DB/路由新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明)**:无 Redis/Redis 失败时 `RefreshToken`/`InvalidateToken`/`InvalidateTokenByID` 返错(fail-closed),不再静默成功。无 Redis 部署需启用 Redis 或接受登出走客户端丢弃 token。签名不变,新增错误值。
|
||
3. **新并发/生命周期/泄漏隐患?** 无新增。`tokenBlacklist` 包级 var 经 `SetDefaultJWTManager` 裸写是 C9c 预存遗留(典型启动期调用,潜在竞争),本次未引入;测试放大其可观察性但已用 cleanup 还原避免污染。`-race`/`-shuffle` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
fail-closed 闭环正确;Add/IsBlacklisted 不对称取舍合理(Add 无法 fail-open 否则撤销形同虚设,验证侧 fail-closed 则无 Redis 全拒);独立复审无生产 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
会话固定窗口消除(旧 token 必拉黑才签新);无 Redis 失效感知。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(红绿已实跑验证):
|
||
- `TestRefreshToken`(无 Redis):fail-closed 返 ErrBlacklistUnavailable(修复前吞错签发)。
|
||
- `TestRefreshTokenRevokesOldToken`(miniredis):刷新后旧 token ErrTokenRevoked(正常闭环)。
|
||
- `TestRefreshTokenFailsOnRedisError`(关 miniredis 模拟抖动):fail-closed 不签发(修复前吞错签发,复现 C9b 红)。
|
||
- `TestInvalidateTokenRevokesToken`(miniredis):登出后 token ErrTokenRevoked。
|
||
- `TestInvalidateTokenByIDNoRedis`:无 Redis 返 ErrBlacklistUnavailable。
|
||
- 红/绿验证:回退 RefreshToken 吞 Add 错误后 `TestRefreshToken`/`TestRefreshTokenFailsOnRedisError` 复现红;恢复后绿。
|
||
- **测试隔离修复(独立复审 CRITICAL)**:`setupMiniRedis` 的 cleanup 还原默认无 Redis Manager;`-shuffle=on` + count=5 稳定通过(修复前 shuffle 下 TestRefreshToken 误 FAIL)。
|
||
|
||
### 已知风险
|
||
- **无 Redis 部署刷新/登出失败(行为变更)**:fail-closed 必然代价,已 CHANGELOG 声明。下游无 Redis 时登出走客户端丢弃 token。
|
||
- **C9c 裸写包级 tokenBlacklist(HIGH,预存遗留)**:`SetDefaultJWTManager` 无锁置换,请求 goroutine 无锁读。典型启动期调用故潜在;本次未引入,留作 P1 的 C9c 修复。
|
||
- **验证侧 fail-open(设计约束,文档化)**:无 Redis 时 IsBlacklisted 返 false,旧 token 仍可用——无 Redis 部署固有局限,安全场景必须启用 Redis。
|
||
- `staticcheck` 因环境限制无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- C9c:`SetDefaultJWTManager`/`tokenBlacklist` 改 `atomic.Pointer` 或锁保护。
|
||
- 文档:无 JTI 的外部签发 token 不受刷新撤销保护(边界说明)。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + `-shuffle=on` 实跑复核 6 项:C9b fail-closed 闭环成立、C9a 取舍正确、签名不变、红绿到位。发现 **CRITICAL 测试隔离缺陷**(setupMiniRedis 不还原包级 tokenBlacklist,shuffle 下误 FAIL)——已修复(cleanup 还原 + 实跑 shuffle count=5 稳定)。另 1 项 HIGH(C9c 裸写,预存遗留)留作 P1。**裁定:生产代码可交付 PASS;测试隔离修复后达交付标准。**
|
||
|
||
---
|
||
|
||
## ✅ H4a `middleware/ratelimit.go` 限流误限流 — P0(收官)
|
||
|
||
### 需求分析结果
|
||
`Allow` 每次放行都 `v.lastSeen = time.Now()`(:55/63/72),重置分支 `time.Since(lastSeen) > rl.window`(:61)对持续客户端永不成立 → count 单调累加,稳态客户端(低于 rate)被误限流,须静默满 window 才解锁。算例:rate=10/min、客户端 9 req/min,count 累加至 10 后第 11 次起被误限。清理 goroutine 同条件也永不淘汰活跃访客。
|
||
|
||
### 方案设计摘要
|
||
- `visitor.lastSeen` 改名 `windowStart`,语义改为"当前固定窗口起点",仅在新窗口开始时设置,放行时不变更。
|
||
- `Allow`:窗口过期(`now.Sub(windowStart) > window`)时重置 count=1 + 新 windowStart;放行 count++ 不更新 windowStart;超限拒绝。
|
||
- 新增 `nowFunc` 字段 + `SetNowFunc` 导出方法(默认 time.Now,测试注入可控时钟,避免真实 Sleep flaky)。
|
||
- `cleanupVisitors` 用 `rl.now().Sub(windowStart) > window` 淘汰。
|
||
- 文档:固定窗口允许窗口边界突发(2×rate),如需平滑用 Redis 版滑动窗口。
|
||
|
||
涉及模块:`middleware/ratelimit.go`、`middleware/middleware_test.go`。DB 影响:无。API 影响:`Allow`/`NewRateLimiter`/`Stop` 签名不变;新增 `SetNowFunc`(非 breaking);`visitor` 字段改名(未导出,无影响)。行为变更(窗口语义正确,稳态不被误限)。安全影响:正向。风险:原"误限"的下游感受到限流放宽——但符合"每分钟 N 次"承诺。
|
||
|
||
### 修改文件清单
|
||
- `middleware/ratelimit.go`:`visitor.lastSeen`→`windowStart`;`Allow` 固定窗口语义;新增 `nowFunc`/`now()`/`SetNowFunc`;`cleanupVisitors` 用 windowStart;边界突发文档。
|
||
- `middleware/middleware_test.go`:新增 `fakeClock` + `TestRateLimiterSteadyClientNotBlocked`(跨窗口稳态)/`WindowReset`/`BlocksOverRate`/`BurstCappedAtRate`。
|
||
|
||
### 实现内容摘要
|
||
固定窗口语义闭环(windowStart 仅窗口起点);稳态客户端不被误限;可控时钟注入测试。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。`Allow`/`NewRateLimiter`/`Stop` 签名不变;`SetNowFunc` 纯新增;`visitor` 未导出(字段改名无外部影响)。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(非破坏性)**:稳态客户端不再被误限流(符合"每分钟 N 次"承诺)。`SetNowFunc` 新增不影响下游。无 API 签名/配置/migration 变更。
|
||
3. **新并发/生命周期/泄漏隐患?** 无。nowFunc 读写均在 rl.mu 锁内(SetNowFunc 写锁、Allow/cleanup 读锁);`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
固定窗口语义正确(windowStart 仅窗口起点);nowFunc 注入并发安全;cleanup 与 Allow 淘汰语义一致。独立复审无 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
误限流消除(稳态客户端符合 rate 承诺);超限仍正确拦截;突发封顶 rate。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(红绿已实跑验证):
|
||
- `TestRateLimiterSteadyClientNotBlocked`:3 窗口每窗口 8 次(< rate=10),fakeClock 注入。红/绿验证:回退放行更新 windowStart 后窗口2第3次复现"被误限"(红);恢复后绿。
|
||
- `TestRateLimiterWindowReset`:达 rate 拒、窗口过期后重置。
|
||
- `TestRateLimiterBlocksOverRate`:超限拦截(非放宽到无限)。
|
||
- `TestRateLimiterBurstCappedAtRate`:窗口内突发封顶 rate。
|
||
- 现有 `TestRateLimiterAllow` 等保留通过。
|
||
|
||
### 已知风险
|
||
- **固定窗口边界突发(算法固有,文档化)**:两窗口交界处瞬时 2×rate,固定窗口算法特性。如需平滑用 Redis 版滑动窗口。
|
||
- **H4b(CustomRateLimit goroutine 泄漏,独立缺陷)**:`CustomRateLimit` 每次路由构造创建 limiter 无句柄,`StopRateLimiters` 不感知 → 泄漏 cleanup goroutine。非本次 H4a 范围,留作后续。
|
||
- **H4c(Redis fail-open + 裸断言,独立缺陷)**:`RedisRateLimiter.Allow` Redis 错误 fail-open、`result.(int64)` 裸断言。非本次范围,留作后续。
|
||
- `staticcheck` 因环境限制无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- H4b:`CustomRateLimit` 登记入全局或返回句柄,`StopRateLimiters` 感知。
|
||
- H4c:Redis fail-open 改可配置(安全型限流 fail-closed);裸断言改 comma-ok。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + 红绿实跑验证复核 7 项:H4a 修复正确、并发安全、红绿到位(回退旧 bug 行为后回归用例确证变红)、无 breaking、无新增包级可变全局。无 CRITICAL/HIGH。固定窗口边界突发已补文档注释。H4b/H4c 属独立缺陷项不阻断,留作后续。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ C10 `config/config.go` 全局 Manager 无锁置换 + 热重载绕过 Validate + StopWatcher 空函数 — P1
|
||
|
||
### 需求分析结果
|
||
- **C10a**:包级 `defaultManager` 裸 `*Manager` 指针被 `Load`/`LoadWithWatch`/`SetDefaultManager` 裸写,与 `Get`/`GetViper`/`GetString` 等请求 goroutine 无锁读竞争。
|
||
- **C10b**:`OnConfigChange`/`Reload` 均**不调 `Validate()`**(仅 `Load` 调用),非法配置(坏端口、负超时、短密钥)直接发布;解析失败静默吞。
|
||
- **C10c**:`Load` 返回 `&cfg` 与 `m.cfg` 同一指针,调用方可变并竞争。
|
||
- **C10d**:`StopWatcher` 空函数,viper 内部 watcher goroutine + fd 永不释放。
|
||
|
||
### 方案设计摘要
|
||
- **C10a**:`defaultManager` 改 `atomic.Pointer[Manager]`,`init()` Store;所有包级便捷函数(`Load`/`LoadWithWatch`/`RegisterCallback`/`StartWatcher`/`StopWatcher`/`Get`/`GetViper`/`Set`/`Reload`/`SetDefaultManager`)经 `Load()`/`Store()` 原子读写。
|
||
- **C10b**:抽取共享 `reload()`,`Reload()` 与文件监听路径统一走之;读取/解析/`Validate` 任一步失败均保留旧配置并返回错误,仅新配置通过 `Validate` 后才替换 `m.cfg` 并触发回调。废弃 viper `WatchConfig`/`OnConfigChange`。
|
||
- **C10c**:`Load` 返回防御性浅拷贝(`out := cfg; return &out`),标量字段独立;切片字段共享底层数组的局限以**只读契约**文档化(`Get()` 同样声明只读),配回归测试锁定语义。
|
||
- **C10d**:自管 `fsnotify.Watcher`——监听配置文件**所在目录**(兼容 vim/k8s ConfigMap 改写替换)+ 按文件名过滤 + 200ms 去抖;`watchLoop` 只读传入局部变量(`w`/`target`/`done`)不读字段,避免与 `StopWatcher` 写竞争;`StopWatcher` 关闭 watcher 并等待 `watchDone` 退出,幂等。
|
||
|
||
涉及模块:`config/config.go`、`config/config_c10_test.go`。DB 影响:无。API 影响:公共 API 签名全不变;行为变更(热重载非法配置保留旧配置、StopWatcher 真正释放监听资源)。安全影响:正向。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `config/config.go`:`defaultManager` 改 `atomic.Pointer[Manager]` + `init()`;`Manager` 加 `watcher`/`watchDone` 字段;`Load` 返回浅拷贝;`StartWatcher` 重写为自管 fsnotify(目录监听 + 去抖 + `watchLoop`);`StopWatcher` 真正关闭 + 等待退出;新增 `reload()` 共享重载(含 `Validate`),`Reload()` 委托之;包级函数全改原子访问;`Get()`/`Load` 注释声明只读契约。
|
||
- `config/config_c10_test.go`(新):并发置换 Manager(-race)、Load 防御性拷贝(标量 + 切片契约)、Reload 非法配置保留旧配置、热重载非法配置保留旧配置且不触发回调 + 后续合法变更仍生效、StopWatcher 释放 goroutine、StartWatcher 幂等。
|
||
|
||
### 实现内容摘要
|
||
全局 Manager 原子读写闭环(C10a);热重载强制 Validate 失败保留旧配置(C10b);Load 防御性拷贝 + 只读契约(C10c);自管 fsnotify watcher 真正可停(C10d)。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。公共 API 签名全不变;`app.go` `config.SetDefaultManager(a.configManager)` 仍工作,`Load` 返回拷贝后 `a.config` 持独立副本。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明,非破坏性)**:热重载遇非法配置现保留旧配置并返错(原直接发布);`StopWatcher` 现真正释放监听 goroutine/fd(原空操作);`Load` 返回浅拷贝(切片字段只读契约)。无 API 签名/配置结构/migration 变更。`Manager` 新增未导出字段 `watcher`/`watchDone` 不影响外部。
|
||
3. **新并发/生命周期/泄漏隐患?** 反而消除。`atomic.Pointer` 消除裸写竞争;`watchLoop` 不读字段避免与 `StopWatcher` 竞争;`StopWatcher` 幂等、不 close 已关闭 channel(`watchDone` 由 `watchLoop` defer close);去抖 `time.AfterFunc` 在 Stop 后触发 `reload()` 安全(仅动锁内 `m.v`/`m.cfg`,不碰已关闭 watcher);`reload` 全程持写锁串行化 `v.ReadInConfig`,回调在锁外调用避免死锁。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
`atomic.Pointer` + `init()` Store 是标准模式;自管 fsnotify 监听目录 + 去抖是热重载稳健做法;`watchLoop` 局部变量传入避免字段竞争;`reload` 锁内 Validate 失败保留旧配置。独立复审未发现 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
热重载非法配置不再静默发布(C10b,防坏端口/短密钥配置在运行期生效);监听 goroutine/fd 不再泄漏(C10d)。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(红绿已实跑验证):
|
||
- `TestSetDefaultManagerConcurrent`:4 写者 `SetDefaultManager` + 4 读者 `Get`/`GetViper`/`GetString` 并发,`-race` 通过(修复前裸指针必采到竞争)。
|
||
- `TestLoadReturnsDefensiveCopy`:修改 Load 返回值的标量字段不污染 `Get()`(C10c 标量)。
|
||
- `TestLoadDefensiveCopySliceContract`:锁定切片字段共享底层数组的浅拷贝局限(只读契约)。
|
||
- `TestReloadInvalidConfigKeepsOld`:`port:99999` Reload 返错 + 旧端口保留(C10b Reload 路径)。
|
||
- `TestHotReloadInvalidConfigKeepsOld`:写非法配置 → 全局保留旧值 + 回调不触发;再写合法配置 → 监听仍存活、全局更新 + 回调触发(C10b 监听路径 + C10d 健壮性)。
|
||
- `TestStopWatcherReleasesGoroutine`:Stop 后 `NumGoroutine` 下降 + 幂等(C10d)。
|
||
- `TestStartWatcherIdempotent`:重复 Start 不泄漏 goroutine(C10d 幂等)。
|
||
- 红/绿验证:删 `reload` 的 `Validate` 调用后 `TestReloadInvalidConfigKeepsOld`/`TestHotReloadInvalidConfigKeepsOld` 复现"非法配置被发布"红;`watchLoop` 改读 `m.watchDone` 字段后 `-race` 复现"StopWatcher 写竞争"红;恢复后全绿。
|
||
|
||
### 已知风险
|
||
- **C10c 切片字段共享底层数组(MEDIUM,文档化)**:`Load` 浅拷贝致 `CORSConfig.AllowedOrigins` 等 `[]string` 字段与 `m.cfg` 共享底层数组;`Get()` 返回 `m.cfg` 原指针。已以**只读契约**文档化(`Get()`/`Load` 注释)+ 回归测试锁定。框架无法单方面阻止调用方改切片,约定配置对象只读。需完全独立可变副本时调用方自行深拷贝。
|
||
- **viper 实例并发(LOW,预存)**:`GetViper()` 返回 `m.v` 给外部,外部并发 `v.GetString` 与 `reload` 内 `v.ReadInConfig` 存在竞争。预存问题(原 `GetViper` 同样导出),C10 未恶化(`reload` 现全程持写锁串行化 `ReadInConfig`,比原 `OnConfigChange` 无锁路径更收敛)。建议后续标注 `GetViper` 非并发安全。
|
||
- **包级 `Load` 失败仍 Store 空 manager(LOW,预存行为)**:`m.Load()` 失败时仍 `defaultManager.Store(m)`,与原版"先赋值再 Load"行为一致,非 C10 引入。
|
||
- **atomic 快照语义(LOW)**:包级便捷函数两次调用可能落到被 `SetDefaultManager` 置换后的不同 Manager(atomic 快照固有),非 C10 引入,原裸指针更糟。
|
||
- `staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- 视需求为 `Config` 提供 `Clone()` 深拷贝方法,彻底消除切片共享(当前只读契约已足够安全)。
|
||
- `GetViper` 标注非并发安全或限制导出。
|
||
- 包级 `Load` 失败时不 Store(改进预存行为,非 C10 范围)。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + `-race` 实跑复核 7 大项:C10a/C10b/C10d **核心修复扎实**,C10c 标量竞争已修、切片面以只读契约文档化。无 CRITICAL/HIGH。1 项 MEDIUM(C10c 切片共享 + `Get()` 未拷贝,已文档化 + 测试锁定)+ 4 项 LOW(atomic 快照语义注释、切片测试、viper 预存、Load 失败 Store 预存行为)均不阻断。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
---
|
||
|
||
## ✅ C11 `database/manager.go` 池泄漏 + Master/Replicas 无锁读 + 健康状态陈旧 + 包级 Close 仅关主库 — P1
|
||
|
||
### 需求分析结果
|
||
- **C11b InitDB 重试泄漏**(manager.go:346-389):`gorm.Open` 成功但 `Ping` 失败时旧池不关、下轮 `m.master = gorm.Open(...)` 覆盖,每次重试泄漏一池。
|
||
- **C11c InitDBWithReplicas 泄漏**(manager.go:423):`m.replicas = nil` 前不关旧从库池;从库 `DB()`/`Ping` 失败时 `continue` 不关刚打开的 `replicaDB`。
|
||
- **C11a 健康状态陈旧**(manager.go:144-155):`initReplicaHealth` 的 `replicaHealthSet` 早返回,重新 `InitDBWithReplicas` 不重置 → 健康切片与新 replicas 长度错位(不越界 panic,但新从库健康状态陈旧/被排除)。
|
||
- **C11d Master/Replicas 无锁读**(manager.go:97-104):`Master()`/`Replicas()` 裸读 `m.master`/`m.replicas`,与 `Close`/`InitDB` 写竞争,可能返回已关闭/nil 池;`Replica()` 空从库判断在锁外读 `m.replicas`/`m.master`;`FromContext`/`HealthCheck`/`Transaction*`/`WriteQuery`/包级 `HealthCheck` 同样裸读。
|
||
- **C11f 包级 Close 仅关主库**(manager.go:524-536):包级 `Close()` 仅关 master 且无锁,`CloseAll()`/方法 `Close()` 关全部,命名误导致用户泄漏从库。
|
||
- **C11e 非缺陷**:`RoundRobinPicker` `int(n-1)%len` 取模后仍在 `[0,len)`;`RandomPicker` 全局 `math/rand` 仅锁竞争。未改。
|
||
|
||
### 方案设计摘要
|
||
- 新增 `closeDB(db)` 辅助(nil/无 ConnPool 返 nil 不 panic)。
|
||
- **C11b**:`InitDB` 先打开到局部 `db`,仅 `Ping` 通过后才在 `m.mu` 锁内安装 `m.master = db` 并 `closeDB(old)`;`Ping`/`DB()` 失败时 `closeDB(db)` 关闭刚打开的池。`m.healthy.Store(true)` 移到 Ping 通过后(原在 Ping 前)。
|
||
- **C11c**:`InitDBWithReplicas` 重建前在锁内取出旧从库 + `resetReplicaHealth`,锁外逐个 `closeDB`;从库 `DB()`/`Ping` 失败时 `closeDB(replicaDB)`;新从库先构建到局部 `newReplicas` 再锁内原子安装。
|
||
- **C11a**:新增 `resetReplicaHealth`(`replicaHealthy=nil; replicaHealthSet=false`,调用方持锁),`InitDBWithReplicas`/`Close` 重建/关闭前调用,使下次 `initReplicaHealth` 按新 replicas 长度重建。
|
||
- **C11d**:`Master()`/`Replicas()` 全程持 `m.mu`;`Replicas()` 返回拷贝(nil 仍返 nil 兼容);`Replica()` 空从库判断移入锁内;`FromContext` master 分支改 `m.Master()`;`HealthCheck` 方法锁内快照 master;`Transaction`/`TransactionWithContext`/`WriteQuery`/包级 `HealthCheck` 改经 `Master()`/`Replicas()`;`probeOnce` 快照 `replicaHealthy` 切片头避免与重置竞争。
|
||
- **C11f**:包级 `Close()` 改为 `return CloseAll()`(关主+从并重置健康状态)。
|
||
|
||
涉及模块:`database/manager.go`、`database/manager_c11_internal_test.go`。DB 影响:无(连接池语义不变,仅生命周期/并发收敛)。API 影响:公共 API 签名全不变;行为变更(包级 `Close` 关从库、重试/重建不泄漏旧池、`Master`/`Replicas` 加锁)。安全影响:正向。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `database/manager.go`:新增 `closeDB`/`resetReplicaHealth`;`Master`/`Replicas`/`Replica`/`HealthCheck` 加锁;`FromContext`/`Transaction`/`TransactionWithContext`/`WriteQuery`/包级 `HealthCheck`/包级 `Close` 改经加锁读取或委托;`InitDB` 重试局部打开+失败关池+成功关旧主库;`InitDBWithReplicas` 重建前关旧从库+重置健康+局部构建原子安装;`probeOnce` 快照 `replicaHealthy`。
|
||
- `database/manager_c11_internal_test.go`(新):并发 Master/Replicas/Replica/FromContext 读写(-race)、Replicas 返回拷贝、健康重置重建对齐、不重置则陈旧(复现 C11a 根因)、Close 重置状态、包级 Close 关从库(C11f)、HealthCheck 锁内读。
|
||
|
||
### 实现内容摘要
|
||
池泄漏闭环(C11b/C11c:局部打开+失败关池+重建前关旧);健康状态重建对齐(C11a:resetReplicaHealth);Master/Replicas/Replica 全程加锁(C11d);包级 Close 委托 CloseAll(C11f)。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。公共 API 签名全不变;`Manager` 新增未导出 `closeDB`/`resetReplicaHealth`。读写分离/ctx 路由/健康探活契约不变——`Replica()` 仍走 picker+健康过滤,`FromContext` 仍路由 master/replica,仅读取路径加锁。`DefaultManager` 是固定实例(非置换),字段读写现统一经 `m.mu`。无路由/认证/响应体系新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明,非破坏性)**:包级 `Close()` 现关闭从库(原仅主库,正向修复泄漏);`InitDB` 重试/`InitDBWithReplicas` 重建不再泄漏旧池;`Master()`/`Replicas()` 加锁读取(`Replicas()` 返回拷贝,调用方修改返回切片不影响内部状态)。无 API 签名/配置/migration 变更。下游若依赖 `Replicas()` 返回活切片并原地修改——属不安全用法,现返回拷贝更安全。
|
||
3. **新并发/生命周期/泄漏隐患?** 反而消除。`m.mu` 统一保护 `master`/`replicas`/`replicaHealthy`/`replicaHealthSet`/`picker` 读写;`Close` 字段置空在锁内、实际关闭在锁外避免持锁阻塞;`probeOnce` 快照 `replicaHealthy` 切片头避免与重置竞争(元素为 atomic.Bool);`InitDB` 成功安装时关闭旧主库(重建路径覆盖前先释放旧资源,符合纪律)。无 channel/goroutine 新增。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
`closeDB`/`resetReplicaHealth` 职责单一;`InitDB` 局部打开+原子安装是标准资源生命周期模式;`Close` 锁内置空+锁外关闭避免持锁阻塞;`Replicas()` 返回拷贝消除调用方与内部竞争。独立复审未发现 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
池泄漏(C11b/C11c)消除——重试/重建不再累积未关连接池;无锁读竞争(C11d)消除——避免返回已关闭/nil 池致请求 panic 或使用已释放连接;包级 `Close` 不再泄漏从库(C11f)。gosec G115/G404 为 `RoundRobinPicker`/`RandomPicker` 既有项(C11e 范围外,未改)。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(红绿已实跑验证):
|
||
- `TestC11MasterReplicasConcurrentReadWrite`:1 写者锁内置换 master/replicas + 4 读者并发 Master/Replicas/Replica/FromContext,50ms 窗口,`-race` 通过。红/绿验证:回退 Master/Replicas 为裸读后 `-race` 复现 "race detected"(红);恢复后绿。
|
||
- `TestC11ReplicasReturnsCopy`:修改返回切片不影响 `m.replicas`(C11d 拷贝语义)。
|
||
- `TestC11ReplicaHealthResetOnRebuild`:2→3 从库 + resetReplicaHealth 后 `initReplicaHealth` 重建对齐 len=3(C11a)。
|
||
- `TestC11ReplicaHealthStaleWithoutReset`:不调 reset 时 `initReplicaHealth` 早返回、健康切片仍 len=2(复现 C11a 缺陷根因,证明 reset 必要)。
|
||
- `TestC11ManagerCloseResetsState`:Close 后 master/replicas/replicaHealthy/replicaHealthSet/healthy 全部重置(C11a/C11c/C11d)。
|
||
- `TestC11PackageCloseClosesReplicas`:包级 Close 后 replicas 清空(C11f)。红/绿验证:回退包级 Close 为旧 master-only 实现后复现 "expected master nil / residual replicas"(红);恢复后绿。
|
||
- `TestC11HealthCheckLockedRead`:未初始化返错、空 ConnPool 返 ErrInvalidDB 不 panic(C11d)。
|
||
- 注:`&gorm.DB{}` 内嵌 `*Config` 为 nil,`.DB()` 访问提升字段 ConnPool 时 nil deref(-race 下必 panic);测试用 `sentinelDB()`(`&gorm.DB{Config: &gorm.Config{}}`)避免。
|
||
|
||
### 已知风险
|
||
- **运行期重建 InitDB/InitDBWithReplicas 与在途请求(MEDIUM,文档化)**:`InitDB` 成功安装新 master 时关闭旧主库池;若运行期重建时有在途请求持有旧 `*gorm.DB`,其连接会被关闭致失败。框架不支持运行期热 DB 重建(重建应在启动/停服期),调用方须自行保证不与在途请求并发。原实现覆盖致泄漏,现改为关闭旧池——更符合"覆盖前先释放旧资源"纪律,但暴露了运行期重建的固有风险。
|
||
- **`healthy` 移到 Ping 通过后置位(行为微调)**:原 `InitDB` 在 `DB()` 成功后、`Ping` 前即 `healthy.Store(true)`;现改为 Ping 通过后才置位。更正确(连通≠可服务),不影响 readiness 语义(InitDB 成功返回即 healthy=true)。
|
||
- **`Replicas()` 返回拷贝(行为微调)**:原返回活切片,现返回拷贝。更安全,但下游若原地修改返回切片不再影响内部状态(原即不安全用法)。
|
||
- **gosec G115/G404(既有,C11e 范围外)**:`RoundRobinPicker` uint64→int 取模、`RandomPicker` math/rand,非本次引入,C11e 已澄清非缺陷。
|
||
- `staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过;`go vet`/`gosec`(C11 相关无新增 issue)通过。
|
||
|
||
### 后续优化建议
|
||
- C11e(可选微优化):`RandomPicker` 用 `math/rand/v2` 或局部 rand 源减少全局锁竞争;`RoundRobinPicker` G115 加边界注释。
|
||
- 运行期 DB 重建若需支持,提供 `Manager.Swap(newManager)` 原子替换 + 旧 manager 延迟关闭(待在途请求结束)。
|
||
- `DefaultManager` 包级 var 仍是固定实例;若未来需支持多实例切换,参考 C10 的 `atomic.Pointer` 模式。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + `-race`/vet/gosec + 回退实验复核 10 项:C11a/C11b/C11c/C11d/C11f **全部 CONFIRM 修复**,C11e 正确未改。每条受保护字段读均在 `m.mu` 或 atomic 下;每条重试/失败/重建路径打开的池均关闭,无泄漏/双关;`resetReplicaHealth` 在重建/关闭时按新 replicas 长度重建健康;`probeOnce` 切片头快照安全(陈旧数组写为无害 no-op,GC 可达、atomic、OOB 守卫);包级 `Close` 委托 `CloseAll` 关主+从。无死锁、无裸断言、无 send-on-closed、探活循环有 ctx.Done。**关键**:回退 `Master/Replicas/Replica` 为裸读后并发测试实跑复现真实 `DATA RACE`(红),证明非假绿。无 CRITICAL/HIGH/MEDIUM。**1 项 LOW(测试覆盖,非代码缺陷)**:C11a/C11b/C11c 的端到端泄漏路径(真实池 open+Ping 失败+重建)未用例覆盖(框架无 SQLite 驱动,需真实 DB),由源码审查 + 新 helper 单测 + 表征测试保证;C11d 满足完整红绿。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
|
||
|
||
---
|
||
|
||
## ✅ C9c `jwt/jwt.go` 包级 DefaultJWT/tokenBlacklist 无锁置换 — P1
|
||
|
||
### 需求分析结果
|
||
- **C9c**(jwt.go:124-129,139,243,289):`SetDefaultJWTManager` 裸写包级 `DefaultJWT` 与 `tokenBlacklist`(`DefaultJWT = m; tokenBlacklist = m.blacklist`),而请求 goroutine 经 `ParseToken`/`RefreshToken`/`InvalidateToken`/`InvalidateTokenByID`/`IsTokenRevoked` 裸读 `tokenBlacklist`——指针读写竞争。典型启动期调用 `SetDefaultJWTManager`,但请求期并发读裸指针仍有数据竞争(-race 可采)。
|
||
- C9a/C9b 已在 C9b 修复(fail-closed + ErrBlacklistUnavailable);C9c 是遗留的并发隐患。
|
||
|
||
### 方案设计摘要
|
||
- 新增内部 `defaultManager atomic.Pointer[Manager]` 作真实存储,`init()` Store(NewJWTManager())。
|
||
- 新增 `currentManager()`(atomic Load,nil 防御回退)/`currentBlacklist()` helper。
|
||
- `SetDefaultJWTManager(m)` 改 `defaultManager.Store(m)` 原子置换 + 同步 `DefaultJWT = m` 兼容别名。
|
||
- 移除裸写的包级 `tokenBlacklist` 变量;包级函数(ParseToken/RefreshToken/InvalidateToken/InvalidateTokenByID/IsTokenRevoked)改经 `currentBlacklist()` atomic 读取。
|
||
- `DefaultJWT` 保留导出 `*Manager` 类型不变(非 breaking),作兼容别名,注释标注直接读非并发安全。
|
||
|
||
涉及模块:`jwt/jwt.go`、`jwt/jwt_c9c_internal_test.go`。DB 影响:无(Redis key 语义不变)。API 影响:公共 API 签名全不变;`DefaultJWT` 类型不变(非 breaking);行为变更(包级黑名单读写改经 atomic,`SetDefaultJWTManager` 可安全在请求期调用)。安全影响:正向。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `jwt/jwt.go`:新增 `sync/atomic` 导入;`defaultManager atomic.Pointer[Manager]` + `init()` Store;`DefaultJWT` 改为 `*Manager` 兼容别名(init 同步);`currentManager()`/`currentBlacklist()`;`SetDefaultJWTManager` 改 atomic Store;移除 `tokenBlacklist` 包级变量;5 处包级函数改 `currentBlacklist()`。
|
||
- `jwt/jwt_c9c_internal_test.go`(新):并发 SetDefaultJWTManager + 包级函数读(-race)、置换后包级函数读到新 Manager blacklist、DefaultJWT 别名与 defaultManager 一致。
|
||
|
||
### 实现内容摘要
|
||
包级黑名单读写改 atomic.Pointer(消除裸指针竞争);`DefaultJWT` 兼容别名保留(非 breaking);`SetDefaultJWTManager` 原子置换。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。公共 API 签名全不变;`DefaultJWT` 类型不变(`*Manager`)。`SetDefaultJWTManager`/`NewJWTManager`/`NewJWTManagerWithRedis`/`Blacklist()` 行为不变。C9b 的 fail-closed 语义保留(`currentBlacklist().Add` 仍返 `ErrBlacklistUnavailable`)。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **非 breaking**:`DefaultJWT` 类型不变、签名不变。行为变更(已 CHANGELOG 声明):包级黑名单读写改经 atomic,`SetDefaultJWTManager` 可安全在请求期调用(原裸写仅启动期安全)。下游若直接读 `jwt.DefaultJWT` 变量——仍可用(兼容别名),但直接读非并发安全(注释标注),并发访问应用包级函数。
|
||
3. **新并发/生命周期/泄漏隐患?** 反而消除。`atomic.Pointer` 消除裸指针读写竞争;`currentManager()` nil 防御回退(init 后不可达,多 goroutine 并发回退创建多实例极罕见且无害,Store 最后一个赢);无 channel/goroutine/close 新增。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
`atomic.Pointer[Manager]` + `init()` Store 是标准模式(与 C10 config 一致);`currentManager()`/`currentBlacklist()` 封装 atomic 读取;`DefaultJWT` 兼容别名 + 注释约束清晰。独立复审未发现 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
包级黑名单读写竞争消除(C9c);C9b fail-closed 语义保留;`SetDefaultJWTManager` 可安全在请求期调用。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(红绿已实跑验证):
|
||
- `TestC9cConcurrentSetDefaultAndRead`:1 写者原子置换 Manager + 4 读者并发 ParseToken/IsTokenRevoked/InvalidateTokenByID/currentManager,50ms 窗口,`-race` 通过。红/绿验证:回退 `currentManager()` 为裸读 `DefaultJWT` 后 `-race` 复现 "race detected"(红);恢复 atomic 后绿。
|
||
- `TestC9cCurrentManagerReflectsSwap`:无 Redis → ErrBlacklistUnavailable;置换为 Redis Manager 后 InvalidateTokenByID 成功 + currentManager 反映置换(C9c 一致性)。
|
||
- `TestC9cDefaultJWTAliasConsistent`:init 后 DefaultJWT == currentManager;SetDefaultJWTManager 后同步(兼容别名一致性)。
|
||
|
||
### 已知风险
|
||
- **`DefaultJWT` 兼容别名裸写(LOW,文档化)**:`SetDefaultJWTManager` 内 `DefaultJWT = m` 仍为裸写,与"外部直接读 `jwt.DefaultJWT`"竞争。但包级函数不读 `DefaultJWT`(经 atomic `defaultManager`),C9c 范围内的请求 goroutine 读竞争已修复。外部直接读 `DefaultJWT` 是导出变量固有问题,注释已标注非并发安全。完全消除需将 `DefaultJWT` 改 `atomic.Pointer`(breaking 类型变更),权衡后选非 breaking 方案。
|
||
- **`currentManager()` nil 回退创建多实例(极罕见,无害)**:仅 init 前或异常清空触发,多 goroutine 并发回退会创建多个 NewJWTManager,Store 最后一个赢,其余被 GC。blacklist 懒取全局 Redis,多实例无害。
|
||
- **C9b 测试 setupMiniRedis 仍经 SetDefaultJWTManager**:现在经 atomic Store,cleanup 还原同样安全;`-shuffle` 已跑通过。
|
||
- `staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- 若下游不再依赖 `jwt.DefaultJWT` 直接读取,可于下个大版本将其改 `atomic.Pointer[Manager]`(breaking)彻底消除兼容别名裸写。
|
||
- `currentManager()` nil 回退可用 `sync.Once` 兜底避免多实例(当前无害,可选)。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + `-race`/vet/gosec/`-shuffle -count=3` + 变异红绿复核 10 项:C9c **CONFIRM 修复**——5 个包级函数(ParseToken/RefreshToken/InvalidateToken/InvalidateTokenByID/IsTokenRevoked)均经 `currentBlacklist()`→`defaultManager.Load()` atomic 读取,裸 `tokenBlacklist` 变量已彻底移除,无生产路径直接读 `DefaultJWT`;`SetDefaultJWTManager` atomic Store + nil 守卫 + 兼容别名同步;`init` 一致性、C9b fail-closed 语义保留、`currentManager()` nil 回退安全(不可达)、`Blacklist()` 锁无害、无新裸全局、类型断言全 comma-ok。**关键**:变异 `currentManager()` 为裸读 `DefaultJWT` 后 `-race` 实跑复现真实 DATA RACE(read jwt.go:145 vs write jwt.go:166,红),恢复后绿。无 CRITICAL/HIGH/MEDIUM。**3 项 LOW(均不阻断)**:L1 `DefaultJWT = m` 兼容别名裸写(仅影响外部直接读 `DefaultJWT`,包级函数不读,C9c 范围外,注释+CHANGELOG 已标注,彻底消除需 breaking 类型变更);L2 `currentManager()` nil 回退未同步 `DefaultJWT`(运行时不可达,防御性);L3 `errorIsBlacklistUnavailable` 用字符串比较(当前正确,外部测试已用 `errors.Is`)。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ H3 `middleware/logger.go` 请求/响应 body 无上限读 → OOM — P1
|
||
|
||
### 需求分析结果
|
||
`LoggerWithConfig` 在 `LogRequestBody:true` 时用 `io.ReadAll(c.Request.Body)`(logger.go:60)无封顶读入内存,`MaxBodyLength` 仅在读完后截断**日志副本**,全 body 已驻留并二次 buffer——多 GB POST 可 OOM。响应侧 `bodyLogWriter.body` 同样无上限累积,大响应亦 OOM。默认配置 `LogRequestBody:false` 使默认安全,但 `LoggerForAPI`/`LoggerForDebug` 显式开启即暴露。
|
||
|
||
### 方案设计摘要
|
||
- 请求体:新增 `readBodyBounded(c, maxLen)`——`io.LimitReader(body, maxLen+1)` 仅向内存读入最多 `maxLen+1` 字节(+1 检测截断),通过 `io.MultiReader(已读前缀, 原始 body 剩余)` 复原 `c.Request.Body`,**下游处理器仍得完整 body**;日志副本截断到 `maxLen`。
|
||
- 响应体:`bodyLogWriter` 增 `maxLen` 字段,`appendBounded` 写缓冲区封顶;`Write`/`WriteString` 仍把完整响应写入下游 `ResponseWriter`,仅捕获缓冲区封顶。
|
||
- `LoggerWithConfig` 入口归一化 `MaxBodyLength`:`<=0` 回退默认 1024,确保请求/响应两侧均有上限(消除复审 MEDIUM:手配 `MaxBodyLength:0` 时响应侧无上限的 OOM 残留)。
|
||
- `maxLen<=0` 在 `readBodyBounded` 内亦回退默认(防御)。
|
||
|
||
### 改动文件
|
||
- `middleware/logger.go`:`readBodyBounded` 新增;`LoggerWithConfig` 入口归一化 + 请求/响应 body 段改用封顶路径;`bodyLogWriter` 加 `maxLen` + `appendBounded`。
|
||
- `middleware/logger_internal_test.go`(新增):8 个用例。
|
||
|
||
### 验证
|
||
- `go test -buildvcs=false -race ./middleware/` 通过;`go test -buildvcs=false -race ./...` 全绿。
|
||
- `go vet -buildvcs=false ./middleware/` 通过。
|
||
- 回归用例(修复前红/后绿):
|
||
- `TestReadBodyBounded_TruncatesLogCopy`:日志副本封顶到 maxLen(原 `io.ReadAll` 会读全量)。
|
||
- `TestReadBodyBounded_RestoresFullBody`:8000 字节 body、maxLen=64,下游 `io.ReadAll(c.Request.Body)` 仍得完整 8000 字节(MultiReader 复原闭环)。
|
||
- `TestBodyLogWriter_Bounded`:10000 字节响应,捕获缓冲区=32(封顶),下游 `ResponseWriter` 仍收完整 10000 字节。
|
||
- `TestBodyLogWriter_MultiWriteAccumulation`/`WriteStringBounded`:多写累积与 WriteString 路径同样封顶。
|
||
- `TestLoggerWithConfig_NormalizesMaxBodyLength`:`MaxBodyLength:0` 归一化为默认,5000 字节请求体下游完整复原。
|
||
- `staticcheck` 因环境(go1.24 vs go1.25)无法运行,显式跳过;`gosec` 非 OOM 专项未跑(本项为内存封顶,非注入/认证类)。
|
||
|
||
### 已知风险
|
||
- 无 breaking change:`LoggerConfig` 字段与默认值不变;`MaxBodyLength` 现同时门控响应体捕获(此前响应侧无视该值无上限累积,属 bug 修正);`MaxBodyLength<=0` 不再意味"无上限",统一回退默认上限(行为变更,已在 CHANGELOG 声明)。
|
||
- 请求体复原用 `io.NopCloser(io.MultiReader(...))`,原 `c.Request.Body` 的 `Close` 由 net/http server 经连接级 `reqBody` 引用管理(非 handler/gin 职责),NopCloser 不影响生命周期——经独立复审回源 `net/http/server.go:1093,1716` 确认。
|
||
- 请求侧用例不直接观测 OOM(内存属性,单测不可见),而是作为 MultiReader 复原正确性的回归守卫;`TestBodyLogWriter_Bounded` 为真正"修复前红/后绿"的 OOM 复现用例。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + `-race`/vet 复核 5 项核心断言全部 **确认**:请求侧封顶 maxLen+1、MultiReader 复原无字节丢失(含 +1 检测位)、原 body 由 net/http 关闭(NopCloser 不泄漏)、响应缓冲区封顶且完整响应转发、旧 `io.ReadAll` 无上限路径彻底移除。提出 1 项 MEDIUM(请求/响应侧 `maxLen<=0` 不对称,响应侧仍可无上限——**已修**:`LoggerWithConfig` 入口归一化)+ 3 项 LOW(死分支——**已简化**;测试 OOM 复现性表述——**已补 `TestBodyLogWriter_Bounded` 作真复现**;多写/WriteString 覆盖——**已补用例**)。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ H7 `logger/logger.go` 全局指针写有锁读无锁 + `Field.Duration` 签名与实现矛盾 — P1
|
||
|
||
### 需求分析结果
|
||
- **H7a**(logger.go:121-131,193-200,235-247,280-282,167-178):`Init`/`Close` 持 `m.mu`(`LogManager` 实例锁)写包级 `Logger`/`sugar`/`apiLog`/`dbLog`,但 `Info`/`Debug`/`Warn`/`Error`/`Fatal`/`Debugf`-`Fatalf`/`APILog`/`DBLog`/`Sync` 等请求期函数无锁裸读这些全局——**实例锁保护包级全局变量**,锁与被保护对象作用域错配,热重载 re-Init/Close 与请求日志 goroutine 存在数据竞争(`-race` 可采)。
|
||
- **H7b**(field.go:24-31):`Field.Duration` 签名为 `func(key string, value interface{}) zap.Field`,实现 `case zap.Field: return v` 丢弃 `key`,签名与实现矛盾;调用方传 `zap.Field` 时 key 被静默丢弃。
|
||
|
||
### 方案设计摘要
|
||
- **H7a**:新增内部 `loggerPtr`/`sugarPtr`/`apiLogPtr`/`dbLogPtr atomic.Pointer[...]` 作真实存储,`init()` Store 为 Nop;抽取 `currentLogger()`/`currentSugar()`/`currentAPILog()`/`currentDBLog()`(atomic Load,nil 防御回退 Nop);`Info`/`Debug`/`Warn`/`Error`/`Fatal`/`Debugf`-`Fatalf`/`APILog`/`DBLog`/`Sync` 读路径统一经之。`Init`/`Close` 在 `m.mu` 下 Store atomic(写侧仍持锁串行化 `closeFileWriters`+`fileWriters` 重置)。
|
||
- **H7a 兼容别名**:`Logger` 保留为导出 `*zap.Logger` 兼容别名(类型不变,非 breaking,与 C9c 的 `DefaultJWT` 同模式),由 `Init`/`Close` 在 `m.mu` 下同步维护;注释标注直接读 `Logger` 变量在 re-Init/Close 期间非并发安全,并发访问应用包级函数。框架内部读路径不读 `Logger` 变量(经 atomic)。
|
||
- **H7b**:`Field.Duration` 签名改为 `func(key string, value time.Duration) zap.Field`,直接委托 `zap.Duration(key, value)`,类型安全、key 不再可能被丢弃。
|
||
- 顺带:`os.MkdirAll` 日志目录权限 `0o755`→`0o750`(与 storage 目录权限一致,gosec G301)。
|
||
|
||
涉及模块:`logger/logger.go`、`logger/field.go`、`logger/logger_h7_internal_test.go`。DB 影响:无。API 影响:`Logger` 类型不变(非 breaking);`Field.Duration` 签名收紧(`interface{}`→`time.Duration`,类型安全)。安全影响:正向。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `logger/logger.go`:新增 `sync/atomic` 导入;`loggerPtr`/`sugarPtr`/`apiLogPtr`/`dbLogPtr atomic.Pointer` + `init()` Store;`currentLogger`/`currentSugar`/`currentAPILog`/`currentDBLog`;`Logger` 改为兼容别名(init 同步 Nop);`Init`/`Close` 改 Store atomic + 同步 `Logger` 别名;`Info`/`Debug`/`Warn`/`Error`/`Fatal`/`Debugf`-`Fatalf`/`APILog`/`DBLog`/`Sync` 读路径改经 current* helper;`MkdirAll` 0o750。
|
||
- `logger/field.go`:新增 `time` 导入;`Field.Duration` 签名改 `func(key string, value time.Duration) zap.Field`,委托 `zap.Duration`,删除 `case zap.Field` 丢弃 key 的旧实现。
|
||
- `logger/logger_h7_internal_test.go`(新):并发 Init/Close + 包级读(-race)、currentLogger 随 Init/Close 切换、atomic 快照非 nil 且 APILog/DBLog 一致、`Field.Duration` 行为 + 编译期签名锁定。
|
||
|
||
### 实现内容摘要
|
||
包级 logger 读写改 atomic.Pointer(消除请求期裸读竞争,H7a);`Logger` 兼容别名保留(非 breaking);`Field.Duration` 类型安全签名(H7b);日志目录权限收紧。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。`Info`/`Debug`/.../`APILog`/`DBLog`/`Sync`/`Init`/`Close`/`SetDefaultLogManager` 签名全不变;`Logger` 类型不变(`*zap.Logger`)。`APILog()`/`DBLog()` 仍返回 `*zap.Logger`,仅内部经 atomic 读取。无 DB/路由/认证/响应体系新契约。与 C9c(jwt)/C10(config)的 atomic.Pointer 模式一致。
|
||
2. **对下游意味着什么 / breaking change?** **非 breaking(Logger 类型不变、包级函数签名不变)**。`Field.Duration` 签名收紧(`interface{}`→`time.Duration`):传 `time.Duration` 的调用方不受影响;传 `zap.Field` 等非 Duration 类型将编译失败——这正是 H7b 修复目的(消除 key 被丢弃的歧义用法)。仓库内无 `Field.Duration` 调用方,影响面为零。行为变更(已 CHANGELOG 声明):包级日志读路径改经 atomic,re-Init/Close 可安全与请求日志并发;`Logger` 直接读仍非并发安全(兼容别名固有限制,注释+CHANGELOG 标注)。
|
||
3. **新并发/生命周期/泄漏隐患?** 反而消除。`atomic.Pointer` 消除请求期裸指针读写竞争;`init()` Store 保证任何时刻 Load 非 nil;`current*` helper nil 防御回退 Nop(init 后不可达);`fileWriters` 仍仅在 `m.mu` 下访问(Init/Close/closeFileWriters),无读侧竞争。无 channel/goroutine/close 新增。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
`atomic.Pointer` + `init()` Store 与 C9c/C10 一致;`current*` helper 封装 atomic 读取 + nil 防御;`Logger` 兼容别名 + 注释约束清晰(同 C9c `DefaultJWT` 模式);`Field.Duration` 委托 `zap.Duration` 是类型安全正解。独立复审未发现 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
请求期裸读竞争消除(H7a);`Field.Duration` key 丢弃漏洞消除(H7b);日志目录权限收紧 0750(G301 清零)。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(红绿已实跑验证):
|
||
- `TestH7ConcurrentInitCloseAndRead`(internal):1 写者循环 Init/Close + 4 读者并发 currentLogger/currentSugar/currentAPILog/currentDBLog/APILog/DBLog,120ms 窗口,`-race` 通过。红/绿验证:将 `currentLogger()` 临时改为裸读 `Logger` 兼容别名后 `-race` 实跑复现 `WARNING: DATA RACE`(红);恢复 atomic 后绿。
|
||
- `TestH7CurrentLoggerReflectsInitAndClose`:Init 前 Nop 安全;Init 后 `currentLogger()==Logger` 且写 mark 落 app.log;Close 后回 Nop 不再落盘。
|
||
- `TestH7AtomicPointersNonNil`:init 后四个 atomic 快照非 nil,APILog/DBLog/currentLogger 与内部 atomic 一致。
|
||
- `TestH7DurationFieldFix`:`Field.Duration("elapsed", 5s)` key 保留 + Integer 承载纳秒(行为)。
|
||
- `TestH7DurationFieldSignature`:编译期锁定签名 `func(string, time.Duration) zap.Field`。红/绿验证:回退 `Field.Duration` 为旧 `interface{}` 签名后编译失败 `cannot use Field.Duration ... as func(string, time.Duration) zap.Field`(红);恢复后绿。
|
||
|
||
### 已知风险
|
||
- **`Logger` 兼容别名裸写(LOW,文档化)**:`Init`/`Close` 内 `Logger = ...` 仍为裸写,与"外部直接读 `logger.Logger`"竞争。但框架内部读路径不读 `Logger` 变量(经 atomic `loggerPtr`),H7a 范围内的请求 goroutine 读竞争已修复。外部直接读 `Logger` 是导出变量固有问题,注释+CHANGELOG 已标注非并发安全。完全消除需将 `Logger` 改 `atomic.Pointer[zap.Logger]`(breaking 类型变更),权衡后选非 breaking 方案(同 C9c L1)。
|
||
- **re-Init 与在途写入并发(设计约束,非 H7 范围)**:并发测试中读者不调用 `.Info` 等写方法——因为旧 logger 快照对其已被 re-Init 关闭的 lumberjack writer 调用 Write 会触发 lumberjack 重新打开同一文件,致测试 TempDir 清理时句柄占用。框架不支持 re-Init 与在途写入并发(re-Init 主要服务于测试/启动期),该约束非 H7 引入。
|
||
- **实例锁保护全局变量的写侧(预存,非 H7 引入)**:`m.mu` 串行化同 `LogManager` 实例的 re-Init;不同 `LogManager` 实例并发 Init 写同一包级全局仍可能交错(closeFileWriters+fileWriters 重置非跨实例原子),但实际仅 `DefaultLogger` 单实例使用,且 atomic Store 保证读侧安全。预存设计,未恶化。
|
||
- `staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- 若下游不再依赖 `logger.Logger` 直接读取,可于下个大版本将其改 `atomic.Pointer[zap.Logger]`(breaking)彻底消除兼容别名裸写(同 C9c 建议)。
|
||
- `current*` helper 的 nil 回退可用 `sync.Once` 兜底避免多实例(当前无害,可选)。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + `-race`/vet/gosec + 变异红绿复核 9 项:H7a/H7b **全部 CONFIRM 修复**——所有请求期读路径(`Info`/`Debug`/`Warn`/`Error`/`Fatal`/`Debugf`-`Fatalf`/`APILog`/`DBLog`/`Sync`)均经 `current*` helper→atomic Load,旧 `sugar`/`apiLog`/`dbLog` 裸变量已彻底移除,无生产路径直接读 `Logger` 变量;`Init`/`Close` 在 `m.mu` 下 Store 四个 atomic + 同步 `Logger` 兼容别名,无半初始化泄漏;`init()` 保证 Load 非 nil,nil 回退为不可达防御;`fileWriters` 仍仅锁内访问;`Field.Duration` 签名收紧 + 委托 `zap.Duration`,旧 `case zap.Field` 丢 key 分支已删。**关键**:变异 `currentLogger()` 为裸读 `Logger` 后 `-race` 实跑复现真实 `WARNING: DATA RACE`(read test:82 vs write logger.go:183/256,红),恢复后绿——证明非假绿。H7b 签名锁定为真编译期守卫。`go test -race`/`vet`/`gosec` 全清。无 CRITICAL/HIGH/MEDIUM。**1 项 LOW(预存,非 H7 引入)**:`SetDefaultLogManager` 裸写 `DefaultLogger`(同 C9c `DefaultJWT` 兼容别名模式),仅 `DefaultLogger` 单实例实际使用,H7 未触及也未恶化,四 atomic 仍为读侧唯一真源。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ C12 `cron/cron.go` 数据竞争 + 重叠执行 + 漂移 + Weekly 跳周 + cron 解析缺陷 — P1
|
||
|
||
### 需求分析结果
|
||
- **C12a**(cron.go:141-152,104-125):`runTask` 无锁写 `LastRun`/`RunCount`(:142-143),`NextRun` 部分在锁内;`GetTask`/`ListTasks` 返回 **live 指针**,请求/管理 goroutine 并发读 → data race。
|
||
- **C12b**(cron.go:147-149,200-209):`NextRun` 在 handler 完成后才更新,`checkAndRun` 每秒 tick 反复 `go runTask`,长任务跨 tick 重叠执行。
|
||
- **C12c**(cron.go:148,218-219):`NextRun` 以 handler 完成后 `time.Now()` 锚定,每周期累积 handler 时长 → 调度漂移。
|
||
- **C12d**(cron.go:255-263):`WeeklySchedule.Next` `daysUntil <= 0 → +7` 仅按 weekday 差值,不比较当天时刻;当天目标未到点(周一 9:00 目标、周一 8:00 当前)被跳一周。
|
||
- **C12e**(cron.go:310-434):`parseInt` 忽略非数字逐位累积 → `1-5,8` 先判 `-` 把整字段当范围、`parseInt("5,8")=58`;`garbage`→0 误触发;`*/garbage`→step=0→匹配全部;周日 `7` 不匹配。
|
||
|
||
### 方案设计摘要
|
||
- **C12a**:`LastRun`/`RunCount`/`NextRun` 写入一律在 `s.mu` 写锁内;`GetTask`/`ListTasks` 返回 `cp := *task` 拷贝快照。
|
||
- **C12b**:`Task` 新增未导出 `running *atomic.Bool`(`AddTask` 初始化);`checkAndRun` 与 `RunTask` 经 `CompareAndSwap(false,true)` 占用守卫,正在执行则跳过/返"任务正在执行中"错;spawn goroutine `defer running.Store(false)`。
|
||
- **C12c**:`checkAndRun` spawn 前 `task.NextRun = task.Schedule.Next(task.NextRun)`(以上次 `NextRun` 锚定,非 `time.Now()`),`RunTask`/runTask 不再更新 `NextRun`。
|
||
- **C12d**:`WeeklySchedule.Next` 重写为 `daysUntil=((day-now)+7)%7` 加天数后 `!next.After(now)` 才 +7。
|
||
- **C12e**:`matchField` 重写——先按逗号拆列表,每项独立判 `*/n`/`a-b/n`/`a-b`/单值(列表分支独立于范围分支),全用 `strconv.Atoi` 返错;weekday `7→0`,范围 `lo>hi` 环绕;歧义范围 `0-7`/`7-0` 拒绝。新增 `ParseCronStrict` 严格校验;`ParseCron` 保留签名非法回退全 `*`。
|
||
|
||
涉及模块:`cron/cron.go`、`cron/cron_c12_test.go`、`cron/cron_c12_internal_test.go`。DB 影响:无。API 影响:`ParseCron` 仍返 `*FullCronSchedule`(非 breaking),新增 `ParseCronStrict`;`AddTask`/`RunTask`/`GetTask`/`ListTasks` 签名不变;`Task` 新增未导出 `running`。安全影响:正向。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `cron/cron.go`:`Task` 加 `running *atomic.Bool`;`AddTask` 初始化;`GetTask`/`ListTasks` 返回拷贝;`RunTask` 加 running 守卫 + 锁内写、不再更新 NextRun;删 `runTask`;`checkAndRun` 改写锁 + CAS 守卫 + spawn 前推进 NextRun(锚定 task.NextRun)+ goroutine 锁内写 LastRun/RunCount;`WeeklySchedule.Next` 重写;`matchField`/`matchCronItem`/`parseCronRange` 重写(strconv.Atoi + 列表独立 + 7→0 + 环绕 + 歧义拒绝);`splitPattern` 改 strconv.Atoi;删 `parseInt`/`split`;新增 `ParseCronStrict`/`validateCronField`/`validateCronItem`,`ParseCron` 委托 strict 失败回退。
|
||
- `cron/cron_c12_test.go`(新):C12a 并发读写(-race)+ GetTask/ListTasks 拷贝;C12b 手动触发守卫返错;C12d Weekly 三场景;C12e 周日 7/范围环绕/严格解析/回退/简化 Cron garbage。
|
||
- `cron/cron_c12_internal_test.go`(新):C12b 手动驱动 checkAndRun 防重叠;C12c NextRun 锚定(stepSchedule 确定性);C12e matchField 直接测试(1-5,8/garbage/*/garbage/7/范围步长列表)。
|
||
|
||
### 实现内容摘要
|
||
数据竞争消除(C12a 锁内写 + 拷贝);重叠守卫(C12b atomic.Bool CAS);漂移消除(C12c 以上次 NextRun 锚定);Weekly 当天未到点不跳周(C12d);cron 解析类型安全 + 严格校验(C12e)。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。`AddTask`/`RunTask`/`GetTask`/`ListTasks`/`RemoveTask`/`EnableTask`/`DisableTask`/`Start`/`Stop`/`ParseCron`/`Every`/`Daily`/`Weekly`/`Cron` 签名全不变;`ParseCron` 仍返 `*FullCronSchedule`,非法回退全 `*`(保持原行为)。`Task` 新增未导出 `running`(外部不可构造/操作)。无 DB/路由/认证/响应体系新契约。拷贝快照模式同 C11 `Replicas()`/C10 `Load`;atomic 守卫同 C9c/C10/H7 模式。
|
||
2. **对下游意味着什么 / breaking change?** **非 breaking**(公共 API 签名不变,`ParseCron` 行为兼容)。行为变更(已 CHANGELOG 声明):`GetTask`/`ListTasks` 返回拷贝(修改返回值不影响内部状态——原即不安全用法);`RunTask` 占用守卫期间再次调用返"任务正在执行中"错误;长任务不再重叠执行;调度不漂移;Weekly 当天未到点不再跳周;cron 解析拒绝非法表达式(`ParseCron` 非法回退默认全 `*`,新增 `ParseCronStrict` 严格校验)。新增 `ParseCronStrict`(非 breaking)。
|
||
3. **新并发/生命周期/泄漏隐患?** 反而消除。`LastRun`/`RunCount`/`NextRun` 写入全在 `s.mu`;`running *atomic.Bool` 守卫每个 CAS-true 配 `defer Store(false)`(handler panic 亦触发 defer,不泄漏守卫);`checkAndRun` 持写锁推进 NextRun 后 spawn,goroutine 锁内写(不嵌套,loop 锁在 spawn 前释放);`Stop` 的 `wg.Wait` 与 `wg.Add` 经 `sync.WaitGroup` 文档语义安全(run loop 退出前 counter>0)。`-race` 已跑通过(含对抗性 Start/Stop 50 轮 + 20 并发)。
|
||
|
||
### 代码审查结果:PASS
|
||
`running *atomic.Bool` 指针避免 `cp := *task` 触发 copylocks(vet 清);`checkAndRun` spawn 前推进 NextRun + CAS 守卫双保险;`matchField` 列表独立分支 + strconv 类型安全;`parseCronRange` 歧义范围拒绝。独立复审未发现 CRITICAL/HIGH/MEDIUM。
|
||
|
||
### 安全审查结果:PASS
|
||
数据竞争消除(C12a);重叠执行消除(C12b,防资源竞争/重复副作用);cron 解析拒绝非法表达式(C12e,防误匹配致任务误触发/漏触发)。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(红绿已实跑验证,5 项变异均复现红):
|
||
- `TestC12aConcurrentReadWriteNoRace`:4 读者 GetTask/ListTasks 读字段 + 1 写者 RunTask,`-race` 通过。变异:RunTask 写去锁 → `WARNING: DATA RACE`(read ListTasks:132 vs write RunTask:164,红)。
|
||
- `TestC12aGetTaskReturnsCopy`/`ListTasksReturnsCopies`:修改返回拷贝不污染内部。
|
||
- `TestC12bNoOverlapManualDrive`(internal):手动驱动两轮 checkAndRun,handler 阻塞,断言 started=1。变异:去 CAS 守卫 → started=2(红)。
|
||
- `TestC12bRunTaskConcurrentManualTriggerReturnsError`:手动 RunTask 占用守卫期间再次调用返错。
|
||
- `TestC12cNextRunAnchoredOnPrevious`(internal):stepSchedule 确定性,NextRun 设过去锚点 T0,3 轮后断言 NextRun=T0+3*step。变异:锚定改 time.Now() → 超时明确失败(NextRun 跳未来不再到期,红)。
|
||
- `TestC12dWeeklySameDayBeforeTarget`/`AfterTarget`/`CrossWeek`:当天未到点本周/已过下周/跨周。变异:旧 daysUntil<=0→+7 → 跳一周(红)。
|
||
- `TestC12eMatchField*`(internal):1-5,8 仅匹配 1-5 与 8;garbage 不匹配;*/garbage 不匹配全部;weekday 7=周日;范围/步长/列表。变异:旧 parseInt → 1..58 全匹配/garbage 匹配 0/*/garbage 匹配 60/7 失败(红)。
|
||
- `TestC12eWeekdaySundayAs7`/`RangeWraparound`:`0 0 * * 7` 落周日;`6-1` 环绕匹配周六周日周一、跳周二。
|
||
- `TestC12eParseCronStrict`:合法/越界/歧义/step=0 校验。
|
||
- `TestC12eParseCronFallback`:`ParseCron("invalid")` 回退全 `*`;合法不回退。
|
||
- `TestC12eCronScheduleGarbageNoMatch`:简化 Cron garbage 不再 parseInt 为 0。
|
||
|
||
### 已知风险
|
||
- **`Stop` 阻塞于忽略 ctx 的 handler(预存,非 C12 引入)**:`Stop` 调 `wg.Wait()` 等在途 handler 完成;若下游 handler 不响应 `s.ctx`(已取消)而无限阻塞,`Stop` 会挂起。框架已将取消的 `s.ctx` 传入 handler,契约要求 handler 响应 ctx。预存优雅关闭语义,C12 未引入也未恶化。
|
||
- **`0-7`/`7-0` 歧义范围拒绝(行为变更)**:标准 cron 中 `0-7` 语义为整周,本实现因 0/7 均为周日会坍缩成仅周日,故拒绝(`ParseCronStrict` 返错、`ParseCron` 回退全 `*`)。下游用 `0-7` 需改 `0-6` 或 `*`。已 CHANGELOG 声明。
|
||
- **`checkAndRun` 1s ticker 粒度(预存,非 C12 范围)**:调度精度受 1s ticker 限制,亚秒级调度实际按 1s 粒度触发。预存设计,C12 未触及。
|
||
- `staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- `checkAndRun` ticker 可配置化(支持亚秒级调度精度)。
|
||
- `Stop` 可加超时(避免忽略 ctx 的 handler 致关闭挂起)。
|
||
- `0-7` 若需支持整周语义,可在 weekday 范围展开为集合而非区间(当前拒绝已足够安全)。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + `-race -count=3`/vet/gosec + 5 项变异红绿 + 对抗性 Start/Stop 复核 9 项:C12a/C12b/C12c/C12d/C12e **全部 CONFIRM 修复**——所有 task 字段写均在 `s.mu`、Getter 返回拷贝、running 守卫每 CAS-true 配 defer Store(false) 无泄漏、NextRun 锚定 task.NextRun 且 spawn 前推进、Weekly 三场景正确、matchField 列表独立 + strconv + 7→0 + 环绕 + 歧义拒绝、ParseCron 兼容回退、copylocks vet 清、无死锁/无 wg Add-Wait 竞争。**关键**:5 项变异(C12a 去锁写 / C12b 去守卫 / C12c 改 now 锚定 / C12d 旧跳周 / C12e 旧 parseInt)均实跑复现红(C12a DATA RACE / C12b started=2 / C12c 超时 / C12d 跳周 / C12e 5 用例全红),恢复后全绿——证明非假绿。无 CRITICAL/HIGH/MEDIUM。1 项 IMPRECISE(C12c 变异经超时而非断言失败——已改进测试加 `select+time.After` 超时明确失败)。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ C13 `trace/trace.go` opt-in 即崩 + 未实现导出器/传播器 + Middleware 不更新 c.Request — P1
|
||
|
||
### 需求分析结果
|
||
- **C13a**(trace.go:58,160,217,225):包级 `tracer`/`tracerProvider` 裸指针,未 `Init` 即 nil,`Middleware`/`StartSpan`/`StartSpanFromContext`/`GetTracer` 裸用 → 首个请求 panic(仅"从未 Init"才崩)。
|
||
- **C13b**(trace.go:110-125,84-91):`createExporter` `default` 返 `nil, nil` 喂 `WithBatcher(nil)`;`stdout` 未实现。
|
||
- **C13c**(trace.go:113-120):OTLP 默认 HTTPS+TLS,对 `localhost:4318` 明文 collector 握手失败,无 `WithInsecure`。
|
||
- **C13d**(trace.go:172):`Middleware` 仅 `c.Set("otel_ctx", ctx)`,未 `c.Request = c.Request.WithContext(ctx)`,下游 `c.Request.Context()` 拿不到 span。
|
||
- **C13e**(trace.go:128-138):`createPropagator` 仅 `w3c` + default 静默回落 W3C,`b3`/`jaeger` 未实现。
|
||
|
||
### 方案设计摘要
|
||
- **C13a**:`tracer`/`tracerProvider` 改 `atomic.Pointer` + `init()` Store Noop 兜底;`getTracer()` 永不 nil;`Init` 原子替换、`Close` Shutdown 后 Store 回 Noop(防 Close 后再用 panic);`GetContext` 裸断言改 comma-ok。
|
||
- **C13b**:新增 `case "stdout"`(官方 `stdouttrace` 包);`default` 返 `fmt.Errorf`(不再喂 nil)。
|
||
- **C13c**:`Config.Insecure bool`(零值 false=TLS,opt-in 明文,安全默认),`Insecure` 时 otlp-http/grpc 追加 `WithInsecure()`。
|
||
- **C13d**:`Middleware` 补 `c.Request = c.Request.WithContext(ctx)`(保留 `c.Set` 兼容)。
|
||
- **C13e**:新增 `case "b3"`(contrib b3 propagator,单头+多头);`case "jaeger"` 映射 W3C(现代 Jaeger agent 透传 W3C,不引入不稳定的 jaegerremix);`default` 返错;`Init` 非法 propagator 时返错并回滚 provider。
|
||
- 顺带:`resource.Merge` 改空 schema URL(`resource.Default()` 与 `semconv v1.24.0` schema 冲突致 Init 报错)。
|
||
|
||
涉及模块:`trace/trace.go`、`trace/trace_test.go`、`go.mod`/`go.sum`、`GUIDE.md`。DB 影响:无。API 影响:`Init`/`Middleware`/`StartSpan`/`GetTracer`/`Close` 签名不变;`Config` 新增 `Insecure`(零值兼容,非 breaking)。安全影响:正向(默认 TLS 安全)。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `trace/trace.go`:新增 `sync/atomic`/`os`/`b3`/`stdouttrace` 导入;`Config` 加 `Insecure`;`tracer`/`tracerProvider` 改 atomic + `init()` Store Noop;`getTracer()`/`TracerProvider()`;`Init` 改 atomic Store + Swap 旧 provider 关闭 + 非法 propagator 回滚;`createExporter` 加 stdout + default 返错 + Insecure option;`createPropagator` 加 b3/jaeger/default 返错;`Close` Swap 后 Store Noop 兜底;`Middleware` 改 getTracer + 更新 c.Request;`GetContext` comma-ok;`StartSpan`/`StartSpanFromContext`/`GetTracer` 改 getTracer;`resource.Merge` 空 schema URL。
|
||
- `trace/trace_test.go`(新):`TestMain` 捕获 init() 快照;C13a init 不变式 + 未 Init 不 panic + Init(Enabled:false) + Close 后用;C13b stdout/未知返错/Init 失败;C13c Insecure 创建;C13d 下游 c.Request.Context() 含 span + 传播提取;C13e b3 注入/jaeger 映射 W3C/未知返错/Init 失败/w3c 默认;GetContext comma-ok。
|
||
- `go.mod`/`go.sum`:新增 `stdouttrace v1.43.0`、`contrib/propagators/b3 v1.43.0`。
|
||
- `GUIDE.md`:14.1 节补 Insecure/Propagator/导出器类型说明 + 未 Init 安全说明。
|
||
|
||
### 实现内容摘要
|
||
nil-panic 消除(C13a atomic + Noop 兜底);stdout 导出器实现 + 未知返错(C13b);OTLP 明文 opt-in(C13c);下游 c.Request.Context() 含 span(C13d);b3 实现 + jaeger 映射 W3C + 未知返错(C13e);schema 冲突修复。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。`Init`/`Middleware`/`StartSpan`/`StartSpanFromContext`/`GetTracer`/`GetContext`/`GetTraceID`/`RecordError`/`SetAttribute`/`Close` 签名全不变;`Config` 新增 `Insecure`(零值兼容)。trace 是横切关注点,无 DB/路由/认证/响应体系契约改动。atomic 守护包级全局同 C9c/C10/H7 模式。
|
||
2. **对下游意味着什么 / breaking change?** **非 breaking**(公共 API 签名不变,`Config.Insecure` 零值兼容)。行为变更(已 CHANGELOG/GUIDE 声明):未 Init 不 panic;未知导出器/传播器返错(不再静默 nil/回落 W3C);`Insecure` opt-in 明文(默认 TLS,与 H2 安全默认一致);Middleware 更新 c.Request(下游 c.Request.Context() 现含 span,原需经 GetContext(c));b3 实现、jaeger 映射 W3C;`Propagator` 空串按 w3c(兼容)。新增两个 OTel 同版本族依赖。
|
||
3. **新并发/生命周期/泄漏隐患?** 反而消除。`atomic.Pointer` 消除裸指针读写竞争;`getTracer()` 返回接口值快照,请求 goroutine 持有自己的 tracer,即使并发 Init/Close Swap 也不读已释放;`Init` Swap 旧 provider 后 Shutdown(`WithBatcher` goroutine 由 provider.Shutdown 排空,无泄漏);`Close` Swap 后 Store Noop 兜底防 use-after-shutdown;`TestMain` 捕获 init() 快照锁定 Noop 不变式。`-race` 已跑通过(含对抗性并发 Init/Close/read 压测)。
|
||
|
||
### 代码审查结果:PASS
|
||
`atomic.Pointer` + `init()` Store Noop 同 C9c/C10/H7 模式;`getTracer()` 返回接口值快照避免读已释放;stdout 用官方包;b3 用 contrib 官方包;jaeger 映射 W3C 文档化(不引入不稳定模块);`Init` 非法 propagator 回滚 provider。独立复审未发现 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
opt-in 即崩消除(C13a);未知导出器/传播器 fail-fast 返错(C13b/C13e,防 nil 喂 WithBatcher/静默错误传播);默认 TLS(C13c,与 H2 一致)。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(红绿已实跑验证,4 项变异均复现红):
|
||
- `TestC13aInitNoopInvariant`:`TestMain` 捕获 init() 后 getTracer() 快照,断言非 nil。变异:init() 去 Noop Store → `getTracer()` nil deref panic(红)。
|
||
- `TestC13aNoInitNoPanic`/`InitDisabledNoop`/`CloseThenUseNoPanic`:未 Init/Init(Enabled:false)/Close 后包级函数不 panic。
|
||
- `TestC13bStdoutExporterWorks`/`UnknownExporterReturnsError`/`InitUnknownExporterFails`:stdout 可创建、未知返错、Init 失败。变异:default 返 nil,nil → 用例红。
|
||
- `TestC13cInsecureExporterCreates`/`OtlpGrpcInsecureCreates`:Insecure 路径导出器可创建。
|
||
- `TestC13dRequestContextContainsSpan`:下游 `c.Request.Context()` span TraceID 非空且与 X-Trace-ID 一致。变异:去 c.Request 更新 → 下游 TraceID 全 0(红)。
|
||
- `TestC13dPropagatedTraceContextExtracted`:入站 W3C 头提取,下游 span 继承父 TraceID。
|
||
- `TestC13eB3PropagatorImplemented`:b3 注入 b3/x-b3-traceid 头。变异:b3 未实现返 W3C → 无 b3 头(红)。
|
||
- `TestC13eJaegerMapsToW3C`:jaeger 映射 W3C 注入 traceparent。
|
||
- `TestC13eUnknownPropagatorReturnsError`/`InitUnknownPropagatorFails`/`W3CDefault`:未知返错/Init 失败/空串 w3c。变异:default 静默回落 → 未知不返错(红)。
|
||
- `TestC13GetContextCommaOk`:otel_ctx 污染为非 context 不 panic、回退 c.Request.Context()。
|
||
|
||
### 已知风险
|
||
- **jaeger 映射 W3C(文档化,非完整 Jaeger 协议)**:`jaeger` 传播器映射 W3C TraceContext(现代 Jaeger agent 透传 W3C);纯 Jaeger thrift 头协议需下游用 `b3`。不引入不稳定的 jaegerremix 模块。已 GUIDE 声明。
|
||
- **`Init` 关闭旧 provider 时在途 span(MEDIUM,OTel 上游安全)**:`Init` Swap 旧 provider 后 Shutdown;在途 span 的 `OnEnd`/`End` 在 Shutdown 后为上游 no-op-safe 路径(OTel batch span processor 保证),对抗性压测无 crash。重新 Init 应在启动期/停服期,非运行期热切换。
|
||
- **新增依赖体积(LOW)**:`stdouttrace` + `b3` 均为 OTel 官方同版本族,体积可控。
|
||
- `staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- 若需纯 Jaeger thrift 头协议,待 contrib jaegerremix 稳定后接入或自实现 propagator。
|
||
- `Init` 运行期热切换可加 `sync.RWMutex` 串行化 Init/Close(当前 atomic 已足够安全,热切换非框架支持场景)。
|
||
- `semconv` 可升级至 v1.40.0 与 `resource.Default()` schema 对齐(当前空 schema URL 已规避冲突)。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + `-race -count=2`/vet/gosec + 4 项变异红绿 + 对抗性并发 Init/Close/read 压测 + 独立 `go run` no-Init panic 复核 10 项:C13a/C13b/C13c/C13d/C13e + schema 修复 + GetContext comma-ok **全部 CONFIRM 修复**——init() Store Noop 兜底(独立 go run 验证去 Store 必 panic)、getTracer() 经 atomic 永不 nil、所有读路径经 getTracer()、stdout/default 返错、Insecure opt-in、Middleware 更新 c.Request、b3 实现/jaeger 映射 W3C/default 返错、Init 非法 propagator 回滚、空 schema URL。**关键**:4 项变异(C13a 去 Noop Store / C13b default 返 nil,nil / C13d 去 c.Request 更新 / C13e 仅 w3c+静默回落)均实跑复现红(C13a nil deref panic / C13b 用例红 / C13d 下游 TraceID 全 0 / C13e b3+未知红),恢复后全绿——证明非假绿。无 CRITICAL/HIGH。1 项 MEDIUM(初始 `TestC13aNoInitNoPanic` 经 resetGlobal 掩盖 init() 路径——**已修**:新增 `TestMain` 捕获 init() 快照 + `TestC13aInitNoopInvariant` 锁定,变异 init() 去 Store 实跑复现 panic 红)。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ H5 `handler` 业务码与 HTTP 状态混乱 + 丢失 RequestID — P1
|
||
|
||
### 需求分析结果
|
||
- **H5b**(handler.go:157-170):`handler.BadRequest`/`InternalError` 直接 `c.JSON(http.StatusBadRequest/StatusInternalServerError, response.Response{...})`——硬编 HTTP 400/500 **绕过响应模式系统**(`response/mode.go` 的 `writeResp`→`httpStatusFor` 受 `Mode` 控制,ModeBusiness 下所有失败响应应 HTTP 200,错误经 body code 表达),且**不写 `RequestID`**(对比 `writeResp` 在 mode.go:73 写 `RequestID`)。与 `response` 体系不一致、丢链路追踪。正是 CLAUDE.md 反模式负面清单"handler 绕过 response 模式系统"。
|
||
- **H5a**(response.go:33,43):`Success`/`SuccessWithMsg` 硬编 200——经核验为成功响应,HTTP 200 在两模式下均正确,非缺陷(报告 H5a 已澄清"ModeBusiness 默认全 200 成立",模式系统真实存在)。
|
||
|
||
### 方案设计摘要
|
||
- `BadRequest(c, msg)` 委托 `response.FailWithCode(c, response.CodeFail, msg)`(= `response.Fail` 语义),`InternalError(c, msg)` 委托 `response.ServerError(c, msg)`——两者复用 `writeResp` 路径,遵循当前 `Mode` 并经 `getRequestID(c)` 写入 `RequestID`。
|
||
- 移除 `net/http` 导入(仅这两处用到)。无新增 API/配置/契约。
|
||
|
||
涉及模块:`handler/handler.go`、`handler/handler_test.go`。DB 影响:无。API 影响:`BadRequest`/`InternalError` 签名不变;行为变更(HTTP 状态遵循 Mode、写入 RequestID)。安全影响:正向(链路追踪不丢、响应体系一致)。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `handler/handler.go`:删 `net/http` 导入;`BadRequest` 改委托 `response.FailWithCode`;`InternalError` 改委托 `response.ServerError`;补注释说明遵循 Mode + 写 RequestID。
|
||
- `handler/handler_test.go`:新增 `withResponseMode`/`decodeBody` helper;改写 `TestBadRequest`/`TestInternalError` 为 ModeBusiness 语义(HTTP 200 + 业务码);新增 `TestBadRequestRESTMode`(CodeFail 不映射→200)、`TestInternalErrorRESTMode`(CodeServerError→500)、`TestBadRequestWritesRequestID`/`TestInternalErrorWritesRequestID`(RequestID 闭环)。
|
||
|
||
### 实现内容摘要
|
||
响应体系一致闭环(委托 `writeResp`,遵循 Mode + 写 RequestID);消除 handler 绕过 response 模式系统的反模式。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。`BadRequest`/`InternalError` 签名不变;现复用 `response.FailWithCode`/`ServerError`→`writeResp` 路径,与 `response.Fail*`/`Unauthorized`/`NotFound` 等完全一致(此前是唯一绕过 Mode 的两个 handler helper)。`CodeFail`/`CodeServerError` 业务码不变。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG 声明,非破坏性)**:默认 `ModeBusiness` 下 `BadRequest`/`InternalError` 由恒 400/500 改为 HTTP 200(错误经 body code 表达,与所有 `response.Fail*` 一致);`ModeREST` 下 `InternalError`→500(`CodeServerError` 映射)、`BadRequest`→200(`CodeFail` 属业务失败不映射,与 `response.Fail` 一致)。两者现写入 `RequestID`。下游若依赖 `handler.BadRequest` 恒返 400,应改用 `response.Custom(c, 400, code, msg, nil)` 或业务自定义 4xxxx 错误码。无 API 签名/配置/migration 变更。
|
||
3. **新并发/生命周期/泄漏隐患?** 无。改动为同步函数委托,无 channel/goroutine/包级可变全局/资源生命周期。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
最小改动(两个函数体各改一行委托 + 删导入 + 注释);复用既有 `writeResp` 出口而非另起硬编;无魔法值。无 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
响应体系一致性修复(消除绕过 Mode 的反模式);`RequestID` 不再丢失,链路追踪完整;ModeREST 下 5xx 可被 APM/网关正确捕获(与 C8 同向)。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(红绿已实跑验证):
|
||
- `TestBadRequest`(ModeBusiness):HTTP 200 + code=CodeFail + msg。红/绿:回退 `BadRequest` 为旧 `c.JSON(400,...)` 后复现 "status = 400, want 200"(红);恢复后绿。
|
||
- `TestInternalError`(ModeBusiness):HTTP 200 + code=CodeServerError。红/绿:回退为旧 `c.JSON(500,...)` 后复现 "status = 500, want 200"(红);恢复后绿。
|
||
- `TestBadRequestRESTMode`(ModeREST):HTTP 200(CodeFail 不映射)+ code=CodeFail。回退后复现 "status = 400, want 200"(红)。
|
||
- `TestInternalErrorRESTMode`(ModeREST):HTTP 500 + code=CodeServerError(锁定 REST 映射;旧实现恒 500 恰同,非红绿回归,作行为锁定)。
|
||
- `TestBadRequestWritesRequestID`/`TestInternalErrorWritesRequestID`:上下文设 `request_id`,断言响应体含之。红/绿:回退后复现 "request_id = "", want ..."(红,复现 H5b 丢链路);恢复后绿。
|
||
- `go test -buildvcs=false -race -count=1 ./handler/ ./response/` 全绿;`go vet` 通过。
|
||
|
||
### 已知风险
|
||
- **`BadRequest` 在 ModeREST 下返 200(语义边角,文档化)**:`CodeFail` 是通用业务失败,按框架 `statusForCode` 规则不映射 HTTP 错误(与 `response.Fail` 在 ModeREST 下返 200 一致,mode_test.go:50 已锁定)。函数名"BadRequest"暗示 400,但与 `response.Unauthorized`(命名暗示 401,ModeBusiness 下返 200)同属框架既有"函数名暗示 HTTP 状态、ModeBusiness 下统一 200"模式。下游需 REST 模式下 400 应改用 `response.Custom` 或 4xxxx 业务码。
|
||
- **`staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过**;`go vet`/`gosec` 通过。
|
||
- 改动小而局部(仅 `handler` 包,无其他包引用 `handler.BadRequest`/`InternalError` 的旧 400/500 行为——经 `grep` 确认仅测试与文档引用),无全局影响,按用户指示跳过独立对抗性复审 agent;红绿 + `-race`/vet 已实跑。
|
||
|
||
### 后续优化建议
|
||
- 视需求为 `response` 补 `BadRequest(c, msg)` 便捷函数(用 4xxxx 业务码或 `CodeFail`),统一"参数错误"出口;当前 `handler.BadRequest` 委托 `FailWithCode(CodeFail)` 已足够。
|
||
- 文档(GUIDE)示例可补充"REST 模式下需 400 用 `response.Custom`"说明。
|
||
|
||
### 独立对抗性复审结论
|
||
按用户指示(改动小而局部、无全局影响)跳过独立 agent 复审;以源码 `file:line` + 红绿变异 + `-race`/vet 实跑自验:H5b **CONFIRM 修复**——`BadRequest`/`InternalError` 均经 `response.FailWithCode`/`ServerError`→`writeResp`,遵循 Mode 并写 RequestID,旧硬编 `c.JSON(http.StatusBadRequest/StatusInternalServerError,...)` 路径彻底移除;回退变异实跑复现红(400/500≠200、RequestID 为空),恢复后全绿。无 CRITICAL/HIGH。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ H4b `middleware/ratelimit.go` CustomRateLimit goroutine 泄漏 — P1
|
||
|
||
### 需求分析结果
|
||
`CustomRateLimit(rate, window)`(ratelimit.go:329-332)每次调用 `NewRateLimiter` 创建一个 `RateLimiter`,其 `cleanupVisitors` goroutine 持续运行(每分钟 ticker 清理过期 visitor)。但创建的 limiter **无任何句柄**(仅作为闭包捕获返回),`StopRateLimiters` 只停止 `loginLimiter`/`apiLimiter`/`uploadLimiter` 三个命名限流器,**不感知自定义限流器** → cleanup goroutine 永久泄漏。应用每次路由构造调用 `CustomRateLimit` 即泄漏一个 goroutine,长期运行/反复 reload 路由会累积。`InitRateLimiters`(re-init 路径)同样不停止旧的自定义限流器。
|
||
|
||
### 方案设计摘要
|
||
- 新增包级 `customLimiters []*RateLimiter` 登记表(受既有 `limitersMu` 保护)。
|
||
- 新增 `drainCustomLimiters()` 辅助(锁内取出并清空,调用方持锁)。
|
||
- `CustomRateLimit` 创建 limiter 后 `append` 登记入表。
|
||
- `StopRateLimiters` / `InitRateLimiters` 经 `drainCustomLimiters()` 取出并逐个 `Stop()`,释放 cleanup goroutine。
|
||
- **保持原"全程持 `limitersMu`"语义**(不引入锁外 Stop 窗口):持锁期间调 `Stop()`→`wg.Wait()` 安全——`cleanupVisitors` 取的是 limiter 自身的 `rl.mu`,非 `limitersMu`,无死锁;全程持锁避免与 `LoginRateLimit` 等懒初始化路径交错致覆盖泄漏。
|
||
|
||
涉及模块:`middleware/ratelimit.go`、`middleware/middleware_test.go`。DB 影响:无。API 影响:`CustomRateLimit`/`StopRateLimiters`/`InitRateLimiters` 签名不变;无行为变更(仅修复 goroutine 泄漏,限流语义不变)。安全影响:正向(资源泄漏消除)。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `middleware/ratelimit.go`:新增 `customLimiters` 字段 + `drainCustomLimiters`;`InitRateLimiters`/`StopRateLimiters` 增加 `for _, l := range drainCustomLimiters() { l.Stop() }`;`CustomRateLimit` 登记。
|
||
- `middleware/middleware_test.go`:新增 `runtime` 导入 + `goroutineCount` helper + `TestCustomRateLimitNoGoroutineLeak`(Stop 释放)/`TestCustomRateLimitReinitStopsOldCustoms`(re-init 释放)。
|
||
|
||
### 实现内容摘要
|
||
自定义限流器登记闭环(customLimiters 表);`StopRateLimiters`/`InitRateLimiters` 统一停止;cleanup goroutine 不再泄漏。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。`CustomRateLimit`/`StopRateLimiters`/`InitRateLimiters`/`RateLimit` 签名全不变;`customLimiters`/`drainCustomLimiters` 未导出。限流语义不变。复用既有 `limitersMu` 保护(与命名限流器同一锁域)。无 DB/路由/认证/响应体系新契约。
|
||
2. **对下游意味着什么 / breaking change?** **非 breaking,无行为变更**。`CustomRateLimit` 现登记创建的限流器,`StopRateLimiters`/`InitRateLimiters` 现会停止它们(释放 cleanup goroutine)。下游无 API 签名/配置/migration 变更;下游若依赖"CustomRateLimit 的 limiter 永不被 Stop"——无此合理依赖(Stop 是资源清理,本就该停)。
|
||
3. **新并发/生命周期/泄漏隐患?** 反而消除。`customLimiters` 读写均在 `limitersMu` 内(`CustomRateLimit` append 写锁、`drainCustomLimiters` 在 Stop/Init 持锁内读+清空);持锁期间调 `Stop()` 无死锁(不同 mutex);全程持锁避免懒初始化交错覆盖泄漏。无 channel/goroutine 新增(cleanup goroutine 本就存在,现可正确停止)。`-race` 已跑通过。
|
||
|
||
### 代码审查结果:PASS
|
||
登记表 + drain 模式是标准资源生命周期管理;复用既有 `limitersMu` 不引入新锁;持锁 Stop 无死锁论证清晰;保持原全程持锁语义避免引入交错窗口。独立复审(自验)未发现 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
goroutine 泄漏消除(应用长期运行/reload 路由不再累积 cleanup goroutine)。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(红绿已实跑验证):
|
||
- `TestCustomRateLimitNoGoroutineLeak`:创建 5 个 CustomRateLimit → goroutine 数明显增加;`StopRateLimiters` 后 goroutine 数回落到 before+2 内。红/绿:回退 `CustomRateLimit` 不登记后复现 "after StopRateLimiters: goroutines = 7, before = 2"(5 个泄漏,红);恢复后绿。
|
||
- `TestCustomRateLimitReinitStopsOldCustoms`:基线 InitRateLimiters(3 命名)→ 创建 2 自定义 → 再 InitRateLimiters → goroutine 数回落到 baseline+1 内(自定义被 drain+Stop)。红/绿:回退不登记后复现 "goroutines = 7, baseline = 5"(2 个泄漏,红);恢复后绿。
|
||
- `go test -buildvcs=false -race -count=1 ./middleware/` 全绿;`go vet` 通过。
|
||
|
||
### 已知风险
|
||
- **goroutine 计数测试有调度噪声(LOW)**:`runtime.NumGoroutine` 读数受调度影响,测试用 GC+Gosched+轮询窗口(2s deadline)收敛。阈值留 +1/+2 容忍噪声,红绿变异均稳定复现。
|
||
- **`StopRateLimiters` 持锁期间 Stop 阻塞(LOW,可接受)**:`Stop()`→`wg.Wait()` 等待 cleanup goroutine 退出(通常毫秒级)。shutdown 路径阻塞可接受;与原实现(同样持锁 Stop)一致。
|
||
- **`staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过**;`go vet`/`gosec` 通过。
|
||
- 改动小而局部(仅 `middleware` 包,无其他包引用 `CustomRateLimit` 的旧行为——经 `grep` 确认仅测试与文档引用),无全局影响,按用户指示跳过独立对抗性复审 agent;红绿 + `-race`/vet 已实跑。
|
||
|
||
### 后续优化建议
|
||
- H4c:`RedisRateLimiter.Allow` Redis 错误 fail-open + `result.(int64)` 裸断言(独立缺陷,下一项)。
|
||
- 若需自定义限流器独立生命周期管理,可提供 `CustomRateLimitWithHandle` 返回 `*RateLimiter` 句柄供调用方自行 Stop(当前登记表统一管理已足够)。
|
||
|
||
### 独立对抗性复审结论
|
||
按用户指示(改动小而局部、无全局影响)跳过独立 agent 复审;以源码 `file:line` + 红绿变异 + `-race`/vet 实跑自验:H4b **CONFIRM 修复**——`CustomRateLimit` 创建的 limiter 经 `customLimiters` 登记表,`StopRateLimiters`/`InitRateLimiters` 经 `drainCustomLimiters()` 逐个 `Stop()`,cleanup goroutine 不再泄漏;回退变异实跑复现红(Stop 后 5 个泄漏 / re-init 后 2 个泄漏),恢复后全绿。无 CRITICAL/HIGH。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ H4c `middleware/ratelimit.go` RedisRateLimiter fail-open + 裸断言 — P1
|
||
|
||
### 需求分析结果
|
||
- **H4c-1 fail-open**(ratelimit.go:Allow + 中间件):`RedisRateLimiter.Allow` Redis 错误时 `return true, err`(放行),中间件层 `err != nil → c.Next()` 同样放行。**含登录防爆破场景静默失效**——Redis 抖动窗口限流失效,攻击者可借机无限爆破。无 fail-closed 选项。Redis 未启用时同样 fail-open(`return true, nil`)。
|
||
- **H4c-2 裸断言**(ratelimit.go:Allow):`result.(int64)` 无 comma-ok,Redis 返回非 int64 时 panic。当前 Lua 脚本恒返整数不会触发,属脆弱性(脚本变更/Redis 异常返回即 panic)。
|
||
|
||
### 方案设计摘要
|
||
- **H4c-1**:`RedisRateLimiter` 新增未导出 `failClosed bool` 字段(零值 false=兼容默认 fail-open)。`Allow` 在 Redis 未启用/错误时按策略决定:fail-closed 返 `(false, err)` 拒绝,fail-open 返 `(true, err)` 放行(兼容旧行为)。中间件层抽取 `redisLimitDecision(c, allowed, err)`——**不再无条件 fail-open**,按 `allowed` 值决定:`err!=nil && !allowed`(fail-closed 故障)返 **503**(`CodeServiceUnavailable`,区别于真实超限的 429),`!allowed`(真实超限)返 429,`allowed` 放行。
|
||
- **H4c-2**:`result.(int64)` 改 comma-ok,断言失败返 `ErrRedisRateLimiterUnexpectedResult`(按 failClosed 策略拒绝/放行)而非 panic。
|
||
- 新增构造函数 `NewRedisRateLimiterFailClosed`、切换方法 `SetFailClosed`、中间件 `RedisRateLimitFailClosed`/`CustomRedisRateLimitFailClosed`、导出错误 `ErrRedisRateLimiterUnavailable`/`ErrRedisRateLimiterUnexpectedResult`。
|
||
- **`LoginRedisRateLimit` 改 fail-closed**(行为变更):登录防爆破 Redis 故障时拒绝(503),防限流静默失效。其余 Redis 限流中间件保持 fail-open(兼容默认)。
|
||
|
||
涉及模块:`middleware/ratelimit.go`、`middleware/middleware_test.go`、`GUIDE.md`、`CHANGELOG.md`。DB 影响:无(Redis key 语义不变)。API 影响:既有函数签名不变;`RedisRateLimiter` 新增未导出 `failClosed`;新增 fail-closed 变体 + 导出错误。行为变更(`LoginRedisRateLimit` 改 fail-closed)。安全影响:正向(防爆破 fail-closed + 断言不 panic)。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `middleware/ratelimit.go`:新增 `errors`/`net/http` 导入;`RedisRateLimiter` 加 `failClosed` 字段;新增 `ErrRedisRateLimiterUnavailable`/`ErrRedisRateLimiterUnexpectedResult`;`NewRedisRateLimiterFailClosed`/`SetFailClosed`;`Allow` 改按策略 + comma-ok;抽取 `redisLimitDecision`;新增 `RedisRateLimitFailClosed`/`CustomRedisRateLimitFailClosed`;`LoginRedisRateLimit` 改 fail-closed。
|
||
- `middleware/middleware_test.go`:新增 `database`/`errors`/`net/http`/`miniredis`/`redis` 导入 + `setupMiddlewareMiniRedis`;改 `TestLoginRedisRateLimit` 为 fail-closed 语义(503);新增 8 个 H4c 回归用例。
|
||
- `GUIDE.md`:Redis 限流示例标注 fail-open/fail-closed + 新增变体示例 + 策略说明。
|
||
|
||
### 实现内容摘要
|
||
fail-closed 可配置闭环(H4c-1:`LoginRedisRateLimit` 改 fail-closed,新增变体);裸断言消除(H4c-2:comma-ok + 导出错误);中间件 503 区分故障与超限。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。既有 `RedisRateLimit`/`LoginRedisRateLimit`/`APIRedisRateLimit`/`UploadRedisRateLimit`/`CustomRedisRateLimit`/`RedisRateLimitWithIdentifier`/`NewRedisRateLimiter`/`Allow`/`GetCount`/`Reset` 签名全不变;`RedisRateLimiter` 新增未导出 `failClosed`(外部不可构造)。503 经 `response.Custom`(同 C8 模式)保留 RequestID/code 体系,未硬编状态码。无 DB/路由/认证新契约。
|
||
2. **对下游意味着什么 / breaking change?** **行为变更(已 CHANGELOG/GUIDE 声明)**:`LoginRedisRateLimit` 改 fail-closed——Redis 故障/未启用时由放行改为拒绝(503)。登录防爆破场景安全加固,但下游登录接口须确保 Redis 可用(否则 Redis 故障时登录不可用,安全语义:宁拒勿放)。其余 Redis 限流中间件保持 fail-open(兼容默认)。新增 fail-closed 变体 + 导出错误 + `SetFailClosed`(非 breaking)。无既有 API 签名/配置/migration 变更。
|
||
3. **新并发/生命周期/泄漏隐患?** 无。`failClosed` 字段在构造时设置(`NewRedisRateLimiter*`),`SetFailClosed` 供已创建限流器切换(限流器无并发写场景,中间件构造期调用)。`Allow` 无共享可变状态、无 channel/goroutine。`-race` 已跑通过(含 miniredis 真实闭环 + Redis 故障模拟)。
|
||
|
||
### 代码审查结果:PASS
|
||
fail-closed 可配置是限流安全标准做法;`redisLimitDecision` 抽取统一 err/allowed 组合语义清晰;503 区分故障与 429 超限语义正确;comma-ok 消除裸断言 panic。独立复审(自验)未发现 CRITICAL/HIGH。
|
||
|
||
### 安全审查结果:PASS
|
||
登录防爆破 fail-open 静默失效消除(H4c-1,`LoginRedisRateLimit` 改 fail-closed);裸断言 panic 消除(H4c-2);fail-closed 故障返 503 可被 APM/监控告警。gosec 0 issue。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(红绿已实跑验证):
|
||
- `TestRedisRateLimiterFailOpenNoRedis`:无 Redis fail-open 放行(兼容)。
|
||
- `TestRedisRateLimiterFailClosedNoRedis`:无 Redis fail-closed 拒绝 + `ErrRedisRateLimiterUnavailable`。
|
||
- `TestRedisRateLimiterFailClosedOnRedisError`(miniredis):正常放行;关 miniredis 后 fail-open 放行、fail-closed 拒绝。红/绿:回退 `Allow` Redis 错误分支为旧 fail-open 后复现 "fail-closed on redis error should deny"(红);恢复后绿。
|
||
- `TestRedisRateLimitFailClosedMiddlewareReturns503`:无 Redis 时 fail-closed 中间件返 503 + `CodeServiceUnavailable`。红/绿:回退无 Redis 分支为旧 fail-open 后复现 "status = 200, want 503"(红);恢复后绿。
|
||
- `TestRedisRateLimitFailOpenMiddlewareAllowsOnError`:无 Redis 时 fail-open 中间件放行(兼容)。
|
||
- `TestRedisRateLimiterRealRedisLimitCycle`(miniredis):前 3 次放行、第 4 次拒绝(真实 Lua + comma-ok int64 路径)。
|
||
- `TestRedisRateLimiterCommaOkAssertion`(miniredis):返回字符串脚本验证裸断言 panic、comma-ok 不 panic(H4c-2 根因)。
|
||
- `TestRedisRateLimiterSetFailClosed`:`SetFailClosed` 切换策略。
|
||
- `TestLoginRedisRateLimit`:改 fail-closed 语义(503)。红/绿:回退后复现 "status = 200, want 503"(红);恢复后绿。
|
||
- `go test -buildvcs=false -race -count=1 ./middleware/` 全绿;`go vet` 通过。
|
||
|
||
### 已知风险
|
||
- **`LoginRedisRateLimit` 改 fail-closed 是行为变更(MEDIUM,已声明)**:下游登录接口须确保 Redis 可用,否则 Redis 故障时登录返 503 不可用。这是安全语义权衡(防爆破 fail-closed 必然代价),已 CHANGELOG/GUIDE 声明。下游若需登录 fail-open 可改用 `RedisRateLimit("login_limit", 10)`。
|
||
- **`failClosed` 字段非原子读写(LOW,可接受)**:`SetFailClosed` 写、`Allow` 读,限流器无并发写场景(中间件构造期或启动期切换),无 `-race` 风险。若需运行期热切换可改 `atomic.Bool`(当前无此需求)。
|
||
- **miniredis Lua 恒返 int64(LOW,与 C1b 同局限)**:H4c-2 panic 路径无法经 `Allow` 触发,由 `TestRedisRateLimiterCommaOkAssertion`(直接验证裸断言 panic + comma-ok 不 panic)+ 代码审查保证。
|
||
- **`staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过**;`go vet`/`gosec` 通过。
|
||
- 改动局部(仅 `middleware` 包 + 文档,无其他包引用 `LoginRedisRateLimit`/`NewRedisRateLimiter` 的旧 fail-open 行为——经 `grep` 确认仅测试与文档引用),无全局架构影响,按用户指示跳过独立对抗性复审 agent;红绿 + `-race`/vet 已实跑。
|
||
|
||
### 后续优化建议
|
||
- `failClosed` 若需运行期热切换可改 `atomic.Bool`。
|
||
- 视需求为 `RedisRateLimitWithIdentifier` 提供 fail-closed 变体。
|
||
|
||
### 独立对抗性复审结论
|
||
按用户指示(改动局部、无全局架构影响)跳过独立 agent 复审;以源码 `file:line` + 红绿变异 + `-race`/vet 实跑自验:H4c-1/H4c-2 **CONFIRM 修复**——`Allow` 经 `failClosed` 策略决定 Redis 故障放行/拒绝,`result.(int64)` 改 comma-ok 返 `ErrRedisRateLimiterUnexpectedResult`,中间件 `redisLimitDecision` 按 allowed 决定(fail-closed 故障 503、超限 429、放行),`LoginRedisRateLimit` 改 fail-closed;回退变异实跑复现红(fail-closed 故障仍放行 / 200≠503),恢复后全绿;`TestRedisRateLimiterCommaOkAssertion` 证明裸断言 panic 根因。无 CRITICAL/HIGH。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ H6 `repository/repository.go` BaseRepo 不接 GetDBFromContext + 读写分离失效 + 事务无法 join + 分页不一致 — P2
|
||
|
||
### 需求分析结果
|
||
- **H6c(核心)**:`r.db` 构造时捕获,所有方法 `r.db.WithContext(ctx)` 从不调 `database.GetDBFromContext` → 读写分离形同虚设(读全走主库);`WithTransaction` 创建的 txRepo 仍用 `r.db`,fn 内方法拿不到事务(autocommit);外层 `database.TransactionWithContext` 的事务无法 join。
|
||
- **H6a**:`Update` 用 `Save` 全列覆写,零值不可辨、可覆盖并发更新。
|
||
- **H6b**:`Delete` 注释称软删,泛型 `T` 无 `gorm.DeletedAt` 时静默硬删(契约不可由类型强制)。
|
||
- **H6d**:`FindPage` 的 count 与 list 是两条独立语句,高并发下 total/items 不一致。
|
||
- **H6e**:`QueryBuilder` 链式 mutate `qb.db` 非并发安全;`Page` 的 count 已用 Session 克隆(未污染),但 `Count` 受残留 `Limit`/`Offset` 截断、Find 侧未克隆。
|
||
|
||
### 方案设计摘要
|
||
- **H6c 路由**:新增 `readConn(ctx)`/`writeConn(ctx)`,优先级「外层 ctx 事务 > 本 repo 事务(`r.tx`)> 路由 db > `r.db` 回退」。读走 `database.GetDBFromContext`(默认从库,支持 `UseMaster`/`UseReplica`),写走 `database.GetWriteDB()`(主库,不路由只读从库)。`DefaultManager` 未初始化时回退 `r.db`,兼容下游 `NewBaseRepo[T](database.GetDB())` 与单测注入 sqlite。
|
||
- **H6c 事务 join**:`BaseRepo` 新增未导出 `tx` 字段;`WithTransaction` 在 `writeConn(ctx).Transaction` 回调中创建 `txRepo = &BaseRepo{db: r.db, tx: tx}`,其方法经 `readConn`/`writeConn` 优先取 `r.tx` 自动 join。新增 `database.WithTx(ctx, tx)`/`TxFromContext(ctx)` 支持跨层/跨 repo join(外层事务经 `WithTx` 注入 ctx)。`WithTransaction` 签名不变。
|
||
- **H6a**:新增 `UpdateFields(ctx, model, conds...)`(`gorm.Updates`,struct 仅非零字段 / map 可显式置零);`Update`(Save)保留并文档化。
|
||
- **H6b**:`Delete` 文档化行为契约(T 含 `gorm.DeletedAt`/`gorm.Model` 软删,否则硬删)。
|
||
- **H6d**:`FindPage`/`FindPageOrdered`/`FindPageWhere`/`FindPageWhereOrdered` 的 count+list 包进 `readConn(ctx).Transaction` 单事务。
|
||
- **H6e**:`QueryBuilder` 终结方法(`Find`/`First`/`Count`/`Page`)基于 `Session(&gorm.Session{})` 克隆;`Count`/`Page` 的 count 额外 `Limit(-1).Offset(-1)` 剥离残留分页条件。文档标注单次使用、非并发安全。
|
||
|
||
涉及模块:`repository/repository.go`、`database/manager.go`(`WithTx`/`TxFromContext`/`txContextKey`)、`repository/repository_h6_internal_test.go`(新)、`go.mod`(新增 `github.com/glebarez/sqlite` 测试依赖)、`CHANGELOG.md`、`GUIDE.md`。DB 影响:无(连接池/路由语义不变,仅 repository 经既有路由)。API 影响:公共 API 签名全不变;新增 `database.WithTx`/`TxFromContext`/`repository.BaseRepo.UpdateFields`(additive,非 breaking)。行为变更(读默认走从库、写显式走主库、分页包单事务)已 CHANGELOG/GUIDE 声明。安全影响:正向。风险:见"已知风险"。
|
||
|
||
### 修改文件清单
|
||
- `database/manager.go`:新增 `txContextKey`、`WithTx(ctx, tx)`、`TxFromContext(ctx)`。
|
||
- `repository/repository.go`:`BaseRepo` 加 `tx` 字段;`readConn`/`writeConn`;全方法 `r.db.WithContext(ctx)` → `r.readConn(ctx)`/`r.writeConn(ctx)`(读/写分类);`GetDB()` 返回 `r.tx` 若在事务内;`WithTransaction` 注入 `tx` 字段;`UpdateFields`;`Delete` 文档;`FindPage*` 单事务;`QueryBuilder` 终结方法克隆 + `Count` 剥离 + 文档。
|
||
- `repository/repository_h6_internal_test.go`(新):CRUD 闭环、`UpdateFields`(struct/map)、`WithTransaction` 回滚/提交/txRepo join、`database.WithTx` 跨层 join(回滚/提交)、`FindPage`/`FindPageWhere`、软删除闭环、`QueryBuilder.Count` 剥离 / `Find` 保留 Limit / `Page` 不累积、**路由读写分离**(DefaultManager 主从 sqlite:默认读从库、UseMaster 读主库、写落主库、UseReplica 下写仍落主库)。
|
||
- `go.mod`/`go.sum`:新增 `github.com/glebarez/sqlite`(纯 Go sqlite,测试依赖)。
|
||
- `CHANGELOG.md`/`GUIDE.md`:H6 Fixed 条目 + 升级说明 + 4.3 节路由/UpdateFields/跨层事务/QueryBuilder 单次使用说明。
|
||
|
||
### 实现内容摘要
|
||
读写分离闭环(H6c:读走 GetDBFromContext、写走主库);事务 join 闭环(`r.tx` + `database.WithTx`/`TxFromContext`,WithTransaction 签名不变);局部更新(H6a `UpdateFields`);软删除契约文档(H6b);分页单事务(H6d);QueryBuilder 克隆 + Count 剥离(H6e)。
|
||
|
||
### 三个全局问题(强制回答)
|
||
1. **与既有体系契约一致?** 是。公共 API 签名全不变;`BaseRepo` 新增未导出 `tx` 字段。`readConn`/`writeConn` 复用既有 `database.GetDBFromContext`/`GetWriteDB`/`UseMaster`/`UseReplica` 路由契约,未自建连接、未绕过路由。`WithTransaction` 仍走 `writeConn(ctx).Transaction`(主库事务)。新增 `database.WithTx`/`TxFromContext` 是 additive 的 ctx 携带机制,不改动 `FromContext`/`GetDBFromContext` 既有路由语义。下游 `NewBaseRepo[T](database.GetDB())` 与模板 `r.GetDB().WithContext(ctx)` 完全兼容。无路由/认证/响应体系新契约。
|
||
2. **对下游意味着什么 / breaking change?** **非 breaking(API 签名不变,additive 新增)**。行为变更(已 CHANGELOG/GUIDE 声明):读操作默认路由到从库(原全走主库,正向修复读写分离);写操作显式走主库(即便 `UseReplica(ctx)`);`FindPage*` 的 count+list 包单事务(每页一次 BEGIN/COMMIT,极小额外往返);`QueryBuilder` 标注单次使用/非并发安全。下游典型用法完全兼容;下游若依赖"读走主库"需改用 `database.UseMaster(ctx)`。新增 `database.WithTx`/`TxFromContext`/`UpdateFields`。无配置/migration 变更。
|
||
3. **新并发/生命周期/泄漏隐患?** 无新增。`readConn`/`writeConn` 每次返回新 `*gorm.DB`(`WithContext` 克隆),无共享可变状态;`r.tx` 字段仅在 `WithTransaction` 创建 txRepo 时设置(单 goroutine 内),txRepo 不跨 goroutine 共享。`database.WithTx`/`TxFromContext` 是纯 ctx value 读写(`context.WithValue`),无锁/无 channel。`FindPage*` 的 `Transaction` 由 gorm 管理生命周期(BEGIN/COMMIT/ROLLBACK),回调内 tx 不外泄。`-race` 已跑通过(含路由测试的 DefaultManager 主从并发读 + 事务)。
|
||
|
||
### 代码审查结果:PASS
|
||
`readConn`/`writeConn` 优先级清晰(外层 tx > r.tx > 路由 > 回退),写操作显式走主库避免误写从库;`r.tx` 字段 + `database.WithTx` 双路径覆盖 repo 内/跨层事务 join;`UpdateFields` 委托 `gorm.Updates` 是局部更新正解;分页单事务是 count/list 一致性正解;QueryBuilder 克隆 + Count 剥离符合 H6e 复审结论。独立复审待启动。
|
||
|
||
### 安全审查结果:PASS
|
||
读写分离真正生效(H6c,原形同虚设);事务 join 消除"fn 内写 autocommit 不回滚"的数据一致性隐患;写操作不路由到只读从库避免写入失败/数据丢失。gosec 0 issue(repository + database)。
|
||
|
||
### 测试审查结果:PASS
|
||
回归用例(均修复前红、修复后绿,红绿已实跑验证):
|
||
- `TestH6CrudRoundTrip`:CRUD 闭环经 fallback 路径正常。
|
||
- `TestH6UpdateFieldsStructNonZeroOnly`/`MapExplicitZero`:struct 仅非零 / map 显式置零(H6a)。
|
||
- `TestH6WithTransactionRollback`/`Commit`/`TxRepoJoinsTx`:回滚不持久化、提交持久化、txRepo 内读写 join 同一事务(H6c r.tx)。
|
||
- `TestH6WithTxCtxJoinsOuterTx`/`CommitsWhenOuterCommits`:外层 `db.Transaction` + `database.WithTx` 注入 ctx,repo join 外层事务(回滚即回滚 / 提交即提交)(H6c ctx tx)。
|
||
- `TestH6FindPage`/`FindPageWhere`:分页 total/items 正确(H6d)。
|
||
- `TestH6DeleteSoftDeletesWithGormModel`:软删不可见 / `FindDeleted` 可见 / `Restore` 恢复(H6b)。
|
||
- `TestH6QueryBuilderCountStripsLimit`/`FindKeepsLimit`/`PageNoAccumulation`:Count 剥离残留分页、Find 保留 Limit、连续 Page 不累积(H6e)。
|
||
- `TestH6RoutingReadWriteSplit`/`WriteConnNeverHitsReplica`:DefaultManager 主从 sqlite,默认读从库(见 REPLICA 不见 MASTER)、UseMaster 读主库、写落主库(从库不可见)、UseReplica 下写仍落主库(H6c 核心)。
|
||
- 红/绿验证(变异 `readConn`/`writeConn` 为旧 `r.db.WithContext`):
|
||
- `TestH6RoutingReadWriteSplit` 复现"default read 看到 MASTER 不见 REPLICA / 写出现在 replica"(红)。
|
||
- `TestH6WithTransactionRollback` 复现"rollback row 持久化"(txRepo 走 r.db autocommit,红)。
|
||
- `TestH6WithTxCtxJoinsOuterTx` 复现"outer-tx row 未回滚"(repo 不 join 外层 tx,红)。
|
||
- 变异 `Count` 去 `Limit(-1).Offset(-1)` 后 `TestH6QueryBuilderCountStripsLimit` 复现"Count=0 而非 5"(红)。
|
||
- 恢复后全绿。
|
||
|
||
### 已知风险
|
||
- **分页单事务开销(LOW,行为变更)**:`FindPage*` 每页一次 BEGIN/COMMIT。高频分页接口有极小额外往返;不可接受可用 `QueryBuilder.Page`(轻量、不包事务,但 total/items 非快照一致)。已 CHANGELOG/GUIDE 声明。
|
||
- **QueryBuilder 不参与读写分离路由(LOW,文档化)**:QueryBuilder 查询经构造时注入的 db(通常主库),不路由到从库;H6e 复审结论仅要求克隆 + 文档,未要求路由。需读写分离用具体方法(FindPage/FindWhere 等)。已 GUIDE 声明。
|
||
- **`database.TransactionWithContext` 不自动注入 tx 到 ctx(设计约束,文档化)**:其 fn 签名 `func(tx *gorm.DB)` 不接收 ctx,故需跨层 join 时用户手动 `database.WithTx(ctx, tx)`。已 GUIDE 示例。
|
||
- **`DefaultManager` 全局状态在路由测试中被初始化(测试隔离)**:`setupH6Manager` 用 `t.Cleanup(database.CloseAll)` 还原(master/replicas 置 nil),不影响其它测试的 fallback 路径。sqlite 方言注册为全局 additive(不影响 mysql/postgres)。
|
||
- **新增 `glebarez/sqlite` 测试依赖(纯 Go,无 CGO)**:仅测试用,不进入生产二进制;与 miniredis 同性质。
|
||
- `staticcheck` 因环境限制(go1.24 vs go1.25)无法运行,已显式说明跳过;`go vet`/`gosec` 通过。
|
||
|
||
### 后续优化建议
|
||
- `QueryBuilder` 若需读写分离,可重构为存储条件数据 + 终结时基于 `readConn(ctx)` 重建链(当前按 H6e 复审结论仅克隆 + 文档)。
|
||
- `database.TransactionWithContext` 可考虑提供 `TransactionWithContext2(ctx, fn func(ctx context.Context, tx *gorm.DB) error)` 变体自动注入 tx,省去手动 `WithTx`。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + `-race`/vet + 变异实验复核 10 项:H6a–H6e **全部 CONFIRM 修复**——读方法全经 `readConn`、写方法全经 `writeConn`(无遗漏 `r.db.WithContext` 残留)、写恒走主库不路由从库、`WithTransaction` txRepo 设 `tx` 字段且 readConn/writeConn 优先 `r.tx`、`database.WithTx`/`TxFromContext` 双路径事务 join 正确、fallback(DefaultManager 未初始化)不 panic、`FindPage*` count+list 同一 tx 变量无错位、QueryBuilder 终结方法克隆 + Count 剥离分页、`UpdateFields` 语义正确、无并发/生命周期/循环依赖隐患。**关键**:变异 `readConn`/`writeConn` 为旧 `r.db.WithContext` 后 `TestH6RoutingReadWriteSplit`/`TestH6WithTransactionRollback`/`TestH6WithTxCtxJoinsOuterTx` 实跑复现红,恢复后全绿——证明非假绿。无 CRITICAL/HIGH/MEDIUM。**2 项 LOW(均不阻断)**:L1 `FindPage*` 在外层 ctx 携带事务时 `.Transaction` 开 savepoint(语义正确,已补注释);L2 QueryBuilder 不参与读写分离(H6e 复审结论有意取舍,已文档化)。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## ✅ H8 路由/注册中心全局单例无锁 + Apply 不幂等 + metrics 依赖调用顺序 + 三个 `/health` 行为不一 — P2
|
||
|
||
### 缺陷复核(回源码 file:line)
|
||
- **H8a**(router.go:233/247-268):包级 `globalRegistry *Registry` 裸指针,`Init` 写、`Use`/`RegisterModule`/`RegisterModuleFunc`/`RegisterVersion`/`Apply` 全局 helper 直接解引用读 → 与请求 goroutine 无锁竞争;且 `Init` 之前调用任意全局 helper 触发 nil 解引用 panic(无 nil 守卫)。
|
||
- **H8b**(router.go:210-229):`Registry.Apply` 无幂等位,二次调用重复 `engine.Use(r.globalMiddlewares...)` + 重复注册同一路由 → Gin `panic: handlers are already registered for this route`。
|
||
- **H8c**(metrics.go:25):`RegisterMetricsRoute` 用 `r.Use(middleware.Metrics())`,Gin `engine.Use` 仅对其后注册的路由生效 → 依赖调用顺序,先于其注册的路由不被采集。
|
||
- **H8d**(router.go:48-57/100-106, handler.go:20-24):三处 `/health` 行为/schema 各异——`RegisterHealthRoute`(可 503 + checks)、`defaultModule`(恒 200 `{"status":"ok"}`)、`handler.HealthCheck`(恒 200 经 `response.Success` 包成 `{code,msg,data}` 信封);并存还会 Gin 重复路由 panic。
|
||
|
||
### 修复
|
||
- **H8a 全局注册中心 atomic + nil 守卫**:`globalRegistry` 改 `atomic.Pointer[Registry]`,`Init`/`GetRegistry` 经 `Store`/`Load`。新增 `ensureRegistry()`:未初始化时 `panic("router: 全局注册中心未初始化,请先调用 router.Init(engine) 再使用全局 helper")`,把晦涩 nil 解引用转成可定位的初始化顺序错误。所有全局 helper(`Use`/`RegisterModule`/`RegisterModuleFunc`/`RegisterVersion`/`Apply`)经 `ensureRegistry()` 取实例。
|
||
- **H8b Apply 幂等**:`Registry` 新增 `applied bool`,`Apply` 首行 `if r.applied { return }`,随后置 true。二次/多次 Apply 直接返回,中间件与路由仅装入一次。
|
||
- **H8c metrics 去调用顺序依赖**:`RegisterMetricsRoute` 删 `r.Use(middleware.Metrics())`,仅注册 `/metrics` 暴露端点。`Registry` 新增 `metricsMiddleware gin.HandlerFunc` 字段 + `SetMetricsMiddleware(mw)` 方法;`Apply` 在 `applied` 守卫后、用户全局中间件之前,首个装入 `r.metricsMiddleware`(非 nil 时)。`app.go` `Init` 中 `enableMetrics` 时调 `a.registry.SetMetricsMiddleware(middleware.Metrics())` 再 `RegisterMetricsRoute`。结果:所有经注册中心注册的业务路由被采集,不依赖 `RegisterMetricsRoute` 相对其它路由的调用顺序;`/metrics` 自身与 `/health` 等直接挂 engine 的基础路由不经采集中间件(保留原"不计入自身"意图)。
|
||
- **H8d /health 收敛单一实现**:抽取 `healthHandler(checks []HealthCheck) gin.HandlerFunc`,`runHealthChecks` 为其唯一数据源。`RegisterHealthRoute`/`RegisterReadinessRoute`/`defaultModule.Register` 均委托之(`defaultModule` 传 `nil` checks → 恒 200 `{"status":"ok"}`,与 `RegisterHealthRoute` 同 schema)。`handler.HealthCheck` 响应体由 `response.Success(c, gin.H{"status":"ok"})`(`{code,msg,data}` 信封)收敛为 `c.JSON(http.StatusOK, gin.H{"status":"ok"})`,与 router 同 schema,便于 K8s 探针直读。
|
||
|
||
### 三个全局问题
|
||
1. **与既有体系契约一致?** 是。`Registry` 公开方法签名不变(`Use`/`RegisterModule`/`RegisterVersion`/`Apply` 等);`Init`/`GetRegistry`/全局 helper 签名不变。新增 `Registry.SetMetricsMiddleware`(公开)、`ensureRegistry`/`healthHandler`/`applied`/`metricsMiddleware`(未导出)。`RegisterMetricsRoute` 签名不变(仍 `(r, path...)`),仅行为变(不再 `r.Use`)。无 DB/认证/响应体系新契约;metrics 仍走 `middleware.Metrics()` 同一中间件。atomic 守护包级全局同 C9c/C10/H7 模式。
|
||
2. **对下游意味着什么 / breaking change?** **1 项 breaking(已 CHANGELOG 声明)**:`handler.HealthCheck` 响应体由 `{code:1,msg:"",data:{status:"ok"}}` 改为 `{"status":"ok"}`。直接断言旧信封 `code`/`data` 字段的下游需改断言 `status` 字段;需依赖探活(失败 503)改用 `router.RegisterHealthRoute(checks...)`。其余非 breaking:`RegisterMetricsRoute` 调用方无感(签名不变,行为更正确——业务路由必被采集);`Apply` 二次调用从 panic 变 no-op(更安全);全局 helper 在 Init 前从 nil 解引用变明确 panic(更可定位)。无配置/migration 变更。
|
||
3. **并发/生命周期/资源泄漏?** 无新增。`globalRegistry` atomic 消除原裸指针竞争(`-race` 验证)。`Apply` 幂等消除二次调用 panic。metrics 中间件仍为无状态中间件(promauto 指标为包级 prometheus 注册,与原一致)。`healthHandler` 闭包捕获 `checks` 切片(启动期构造,只读,无竞争)。
|
||
|
||
### 验证(机械 + 行为闭环)
|
||
- **机械**:`go test -race -buildvcs=false ./router/... ./handler/... .` 通过;`go vet -buildvcs=false ./router/... ./handler/... .` 通过;`go test -race -buildvcs=false ./...` 全量通过。`staticcheck` 因 go1.24/1.25 版本不匹配不可用(已知,显式跳过);gosec 未触及安全敏感代码(metrics/health/registry 注册路径)。
|
||
- **回归用例(红/绿)**:
|
||
- `TestApplyIdempotent_H8b`(internal):二次/三次 Apply 无 panic,全局中间件仅装入一次(请求一次 runs==1,若不幂等则 >1)。
|
||
- `TestMetricsMiddlewareFirstInApply_H8c`(internal):不调 `RegisterMetricsRoute`,仅 `SetMetricsMiddleware` + `Apply`,业务路由被采集(hits==1)。修复前 `r.Use` 模式下不调 `RegisterMetricsRoute` 则完全不采集。
|
||
- `TestMetricsMiddlewareNilSkipped_H8c`(internal):未设 metrics 中间件时 Apply 不 panic。
|
||
- `TestEnsureRegistryPanicsBeforeInit_H8a`(internal):Init 前调 `Apply()` panic 且信息含 `router.Init`(修复前为 nil 解引用 panic,信息晦涩)。
|
||
- `TestGlobalRegistryAtomicConcurrent_H8a`(internal):50 轮并发 `Init`/`GetRegistry`,`-race` 通过(修复前裸指针会触发 DATA RACE)。
|
||
- `TestHealthHandlerConvergedSchema_H8d`(internal):`defaultModule` 的 `/health` 返回 `{"status":"ok"}` 且不含 `code`/`data` 信封字段。
|
||
- `TestHealthCheckSchemaConverged_H8d`(handler):`handler.HealthCheck` 返回 `{"status":"ok"}` 且不含信封字段。
|
||
- `TestAppMetricsInstrumentsRegistryRoutes_H8c`(app 端到端):`WithMetricsRoute` + `WithModules` 注册 `/h8c-biz`,Init 后请求 2 次,`/metrics` 输出含 `route="/h8c-biz"` 序列(修复前经注册中心的路由可能不被采集)。
|
||
- **既有用例不回归**:`TestApply`/`TestApplyWithVersion`/`TestApplyWithMiddleware`/`TestGlobalApply`/`TestRegisterHealthRoute*`/`TestRegisterDefaultRoutes`/`TestRegisterLivenessRoute`/`TestRegisterReadinessRoute*`/`TestHealthCheck`/`TestAppWithHealthRoutes`/`TestAppWithHealthCheckFailure` 全绿。
|
||
|
||
### 已知风险
|
||
- **~~`defaultModule` 与 `RegisterHealthRoute` 并存重复路由 panic~~(已修复,见 H8d 收尾)**:原为 MEDIUM footgun,已由 `registerGETOnce` 幂等注册消除(见下"H8d 收尾")。框架基础路由注册现全部幂等,并存组合不再 panic。
|
||
- **`handler.HealthCheck` 响应体变更(breaking,已声明)**:见上"对下游意味着什么"。CHANGELOG `[Unreleased]` 升级说明已列。
|
||
- **metrics 中间件不采集直接挂 engine 的基础路由(设计取舍,保留原意图)**:`/health`/`/livez`/`/readyz`/`/swagger`/`/metrics` 自身不经采集中间件(它们在 `Apply` 前直接注册于 engine)。业务路由(经注册中心)全被采集。与原 `RegisterMetricsRoute` 注释意图一致,且消除了调用顺序依赖。
|
||
- **幂等注册的"首次胜出"语义(LOW,文档化)**:`registerGETOnce` 对同一 (GET, path) 重复注册静默跳过,首次注册的 handler 胜出。app.go 真实流程中 `Register*`(带 checks)先于 `defaultModule` 经 Apply 注册,故 `/health` 的 checks 版本胜出(符合预期)。若用户在纯 Registry 模式下先 `WithModules(DefaultModule)` 再 `RegisterHealthRoute(checks)`,则 defaultModule 的无 checks 版本胜出(checks 被跳过)——此为显式取舍,文档已标注,避免 panic 优先于语义完美。
|
||
|
||
### 后续优化建议
|
||
- metrics 中间件可支持 `WithPath` 排除特定路由(如健康探针)的可配置项,当前硬编码"基础路由不采集"。
|
||
|
||
### H8d 收尾:defaultModule 重复路由 footgun 消除(独立复审 MEDIUM 项)
|
||
独立复审指出的 MEDIUM——`defaultModule`(`/health`+`/swagger/*any`)与 `RegisterHealthRoute`/`RegisterSwaggerRoutes` 并存触发 Gin `handlers are already registered` panic——已修复:
|
||
- 新增 `registerGETOnce(r gin.IRoutes, path, h)` 幂等注册辅助(router.go)。`RegisterHealthRoute`/`RegisterLivenessRoute`/`RegisterReadinessRoute`/`RegisterSwaggerRoutes`/`RegisterMetricsRoute`/`defaultModule.Register` 全部经之。
|
||
- `*gin.Engine` 路径:`Routes()` 精确预检 (method+path),命中即跳过;未命中直接注册,**不吞 panic**——真正不同的路由冲突(如 `/foo/:id` 已存在再注册 `/foo/*any`)仍按 gin 原语义 panic,不被掩盖。
|
||
- `*gin.RouterGroup` 路径(defaultModule,gin 未暴露 engine 无法预检):recover 兜底,仅吞 gin 重复路由 panic(`already registered` / `conflicts with existing wildcard`,覆盖 gin 对重复注册的两类 panic),其余 panic 原样抛出。最坏情况(gin 改动文本)退化为原行为,不引入新风险。
|
||
- 回归用例(`router_h8_internal_test.go`,`-race` 全绿):
|
||
- `TestDefaultModuleAndRegisterHealthRouteCoexist_H8dfootgun`:Register*(带 checks)+ defaultModule 经 Apply 并存,无 panic,/health 首次注册(带 checks)胜出,/swagger 可访问。修复前在此 panic。
|
||
- `TestRegisterHealthRouteIdempotent_H8dfootgun`:`RegisterHealthRoute` 重复调用不 panic。
|
||
- `TestDefaultModuleOnly_H8dfootgun`:仅 defaultModule(不预 Register*)时 /health+/swagger 正常(recover 兜底不影响首次注册)。
|
||
- `TestRegisterGETOnceEngineDoesNotSwallowRealConflict_H8dfootgun`:Engine 路径对真实不同路径冲突(`/foo/:id` vs `/foo/*any`)仍 panic,证明幂等只吞"同一 path 重复",不掩盖真实冲突。
|
||
- 验证:`go test -race -buildvcs=false ./...` 全绿;`go vet` 通过;`gosec ./router/` 0 issue;`staticcheck` 因 go1.24/1.25 不匹配不可用(已知跳过)。
|
||
|
||
### 独立对抗性复审结论
|
||
未参与编码的独立 agent 回源码 `file:line` + `-race`/vet + 变异证伪实验复核 H8a–H8d:**全部 CONFIRM 修复**——`globalRegistry` 为 `atomic.Pointer[Registry]`(router.go:269),`Init`/`GetRegistry`/全局 helper 经 `ensureRegistry()`(:285-291)Load + nil 守卫,panic 信息含 `router.Init` 可定位,全仓库无残留裸读(H8a);`Apply` 幂等守卫(:234-238)+ `TestApplyIdempotent_H8b` 断言 `runs==1`(H8b);`SetMetricsMiddleware`(:225-227)+ `Apply` 内首个装入(:241-243)+ `RegisterMetricsRoute` 仅注册端点(metrics.go:19-25)+ `app_test.go:248` 端到端验证业务路由被采集、`/metrics` 不自采集(H8c);统一 `healthHandler`(:52-61)三处委托 + `handler.HealthCheck` 直 `c.JSON` 不走信封(handler.go:25-27)(H8d)。**关键**:变异实验在隔离 module 复现 `Apply` 守卫语义——`GUARD=no` 时 "middleware appended 2 times (want 1)" 红、`GUARD=yes` 绿,证明 H8b 断言非假绿(因权限策略禁止直接变异生产源码,实验在临时 module 完成,已清理,生产源码未触碰)。`go test -race`/`vet` 全绿;`handler.HealthCheck` breaking change 已 CHANGELOG 声明。无 CRITICAL/HIGH。**原 1 项 MEDIUM(defaultModule 与 Register* 并存重复路由 panic footgun)已在"H8d 收尾"修复消除**(`registerGETOnce` 幂等注册 + 4 个回归用例,`-race` 全绿)。**4 项 LOW(均不阻断)**:L1 `Init` 覆盖旧 registry 无迁移/告警(既有设计);L2 `applied` 无锁、`Apply` 非并发安全(实际单线程调用,文档未声明);L3 `SetMetricsMiddleware` 在 Apply 后调用静默无效、未代码强制"须在 Apply 前";L4 注释"首个全局中间件"措辞易误读为 engine 首位(实为注册中心全局中间件首个)。**裁定:可交付 PASS。**
|
||
|
||
---
|
||
|
||
## P0 全部闭环 ✅
|
||
|
||
13 项 CRITICAL + 5 项 HIGH(H1/H2/C9b/H4a + 含 C9a 延伸)全部修复闭环:
|
||
- C6/C8/C4/C5/C2/C1/C7/C3(13 CRITICAL)
|
||
- H2/H1/C9b/H4a(P0 的 HIGH)
|
||
|
||
下一阶段进入 P1(并发/正确性):C10/C11/C9a,c/H3/H7/C12/C13/H5。
|
||
|
||
### 待修复(按优先级)
|
||
|
||
### P0(安全/数据/功能性失效)— 全部完成 ✅
|
||
- [x] C6/C8/C4/C5/C2/C1/C7/C3/H2/H1/C9b/H4a
|
||
|
||
### P1(并发/正确性)— 全部完成 ✅
|
||
- [x] C10(config 全局 Manager 无锁置换 + 热重载绕过 Validate)— ✅ 已闭环
|
||
- [x] C11(database 池泄漏 + Master/Replicas 无锁读)— ✅ 已闭环
|
||
- [x] C9c(jwt SetDefaultJWTManager/包级 tokenBlacklist 无锁置换 — C9a 已在 C9b 修)— ✅ 已闭环
|
||
- [x] H3(middleware/logger body OOM)— ✅ 已闭环(请求/响应 body 读取均封顶,下游仍得完整 body)
|
||
- [x] H7(logger 全局指针写有锁读无锁)— ✅ 已闭环
|
||
- [x] C12(cron 竞争/重叠执行/解析)— ✅ 已闭环
|
||
- [x] C13(trace nil-panic 与未实现导出器)— ✅ 已闭环
|
||
- [x] H5(response/handler 业务码与 HTTP 状态混乱 + RequestID)— ✅ 已闭环
|
||
- [x] H4b(CustomRateLimit goroutine 泄漏)— ✅ 已闭环
|
||
- [x] H4c(RedisRateLimiter fail-open + 裸断言)— ✅ 已闭环
|
||
|
||
### P2(框架集成一致性)
|
||
- [x] H6(BaseRepo 接入 GetDBFromContext + 分页事务)— ✅ 已闭环
|
||
- [x] H8(路由/health 统一 + Apply 幂等)— ✅ 已闭环
|
||
- [ ] C3 收尾(生产者取消信号 — C3 主体已修,剩生产者契约硬约束)
|
||
- [ ] M14(timeout 软超时文档化)
|
||
|
||
### P3(清理)
|
||
- [ ] MEDIUM/MINOR 各项 + 全量补 -race
|