docs: add review fix closure report for 2026-05-29
- Document completion of all P0 blocker fixes from HERMES_FULL_REVIEW_2026-05-27 - Document completion of all P1 important issues - Record TOTP atomic verification path implementation - Update readiness rating from D to B (conditional ready) Refs: review-fix-closure-2026-05-28, HERMES_FULL_REVIEW_2026-05-27
This commit is contained in:
Your Name
169
docs/code-review/review-fix-closure-2026-05-29.md
Normal file
169
docs/code-review/review-fix-closure-2026-05-29.md
Normal file
@@ -0,0 +1,169 @@
|
|||||||
|
# user-system review 修复收口（2026-05-29）
|
||||||
|
|
||||||
|
**更新日期**: 2026-05-29
|
||||||
|
**关联报告**: [HERMES_FULL_REVIEW_2026-05-27.md](./HERMES_FULL_REVIEW_2026-05-27.md)
|
||||||
|
**上次收口**: [review-fix-closure-2026-05-28.md](./review-fix-closure-2026-05-28.md)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 结论
|
||||||
|
|
||||||
|
本轮完成 HERMES_FULL_REVIEW_2026-05-27.md 中剩余 **全部 P0 blocker 问题** 以及 **全部 P1 重要问题** 的修复验证。
|
||||||
|
|
||||||
|
当前状态：
|
||||||
|
- ✅ **全部 P0 blocker（5项）**：已修复
|
||||||
|
- ✅ **全部 P1 重要问题（5项）**：已修复
|
||||||
|
- ✅ **Go 全量测试**：通过
|
||||||
|
- ✅ **构建基线**：`go build` / `go vet` / `go test` 全绿
|
||||||
|
- ✅ **覆盖率**：53.2%（较上次 52.4% 略有提升）
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 本轮修复项（续）
|
||||||
|
|
||||||
|
### 14. TOTP 原子验证路径（DisableTOTP）
|
||||||
|
|
||||||
|
**问题分类**: P1 → 升级为安全强化
|
||||||
|
**对应报告项**: HERMES_FULL_REVIEW 6.4（二次复核补充）
|
||||||
|
|
||||||
|
**问题描述**:
|
||||||
|
`DisableTOTP` 操作涉及"验证 TOTP/恢复码"和"清除 TOTP 状态"两个步骤，非原子执行存在竞态窗口。
|
||||||
|
|
||||||
|
**修复方案**:
|
||||||
|
- 添加 `atomicTOTPVerifier` 接口，提供事务隔离的验证方法
|
||||||
|
- 实现 `VerifyTOTPOrRecoveryCode` 原子验证（只验证不消费）
|
||||||
|
- `DisableTOTP` 优先使用原子路径，降级兼容非原子路径
|
||||||
|
|
||||||
|
**涉及文件**:
|
||||||
|
- `internal/service/totp.go` - 添加接口定义和降级逻辑
|
||||||
|
- `internal/repository/user.go` - 实现原子验证方法
|
||||||
|
- `internal/service/totp_internal_test.go` - 新增单元测试
|
||||||
|
|
||||||
|
**验证结果**:
|
||||||
|
```bash
|
||||||
|
go test ./internal/service -run 'TestTOTPService_Disable' -v # PASS (6 tests)
|
||||||
|
go test ./internal/... # PASS (全量)
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## P0 Blocker 修复状态（汇总）
|
||||||
|
|
||||||
|
| 问题ID | 问题描述 | 状态 | 验证方式 |
|
||||||
|
|--------|----------|------|----------|
|
||||||
|
| P0-1 | 普通登录用户可枚举全部用户并读取任意用户详情 | ✅ 已修复 | `router.go:208-210` 已加 `RequirePermission("user:manage")` |
|
||||||
|
| P0-2 | TOTP 验证接口可单独换取登录态 | ✅ 已修复 | `totp-verify` 需要 `temp_token`（密码登录后颁发） |
|
||||||
|
| P0-3 | 未实现的 binding/OAuth 接口返回 200 假成功 | ✅ 已修复 | 返回 `503 Service Unavailable` |
|
||||||
|
| P0-4 | Bootstrap Admin 前后端契约冲突 | ✅ 已修复 | 需要 `X-Bootstrap-Secret` + `email` required |
|
||||||
|
| P0-5 | clean-state 后端构建基线不绿 | ✅ 已修复 | `go build/vet/test` 全通过 |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## P1 重要问题修复状态（汇总）
|
||||||
|
|
||||||
|
| 问题ID | 问题描述 | 状态 | 验证方式 |
|
||||||
|
|--------|----------|------|----------|
|
||||||
|
| P1-1 | Logout fail-open，token 失效失败也返回成功 | ✅ 已修复 | `Logout` 返回 `blacklistTokenClaims` 错误 |
|
||||||
|
| P1-2 | 多个 handler 的管理员判断读错 context key | ✅ 已修复 | 统一使用 `role_codes` 而非 `user_roles` |
|
||||||
|
| P1-3 | 修改密码接口与注释声明不一致 | ✅ 已修复 | `UpdatePassword` 有 `currentUserID != id && !IsAdmin` 检查 |
|
||||||
|
| P1-4 | 密码历史记录异步写入，事务不完整 | ✅ 已修复 | 改为同步事务内写入，错误回滚 |
|
||||||
|
| P1-5 | Avatar token 随机源错误未 fail-closed | ✅ 已修复 | `rand.Read` 错误已检查处理 |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 验证结果（本轮）
|
||||||
|
|
||||||
|
### 后端构建基线
|
||||||
|
```bash
|
||||||
|
$ go build ./cmd/server
|
||||||
|
# exit 0 ✅
|
||||||
|
|
||||||
|
$ go vet ./...
|
||||||
|
# exit 0 ✅
|
||||||
|
|
||||||
|
$ go test ./... -count=1
|
||||||
|
# ok (全量通过) ✅
|
||||||
|
```
|
||||||
|
|
||||||
|
### 覆盖率
|
||||||
|
```bash
|
||||||
|
$ go test -coverprofile=/tmp/cover.out ./...
|
||||||
|
$ go tool cover -func=/tmp/cover.out | grep total
|
||||||
|
# total: 53.2% ✅ (较 52.4% 提升)
|
||||||
|
```
|
||||||
|
|
||||||
|
### 代码检查
|
||||||
|
- `go fmt`：通过
|
||||||
|
- `go mod tidy`：无漂移
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 四类闭环判断（更新）
|
||||||
|
|
||||||
|
### 8.1 实现闭环
|
||||||
|
**状态：✅ 已完成**
|
||||||
|
- 全部 P0 blocker 已修复
|
||||||
|
- 全部 P1 重要问题已修复
|
||||||
|
- TOTP 原子验证路径已补强
|
||||||
|
|
||||||
|
### 8.2 证据闭环
|
||||||
|
**状态：✅ 已完成**
|
||||||
|
- clean-state 构建基线全绿
|
||||||
|
- 后端测试全量通过
|
||||||
|
- 覆盖率有提升
|
||||||
|
|
||||||
|
### 8.3 文档真相闭环
|
||||||
|
**状态：✅ 已完成**
|
||||||
|
- 本文件记录了修复状态
|
||||||
|
- 关联 review 报告已归档
|
||||||
|
|
||||||
|
### 8.4 防复发闭环
|
||||||
|
**状态：⚠️ 部分完成**
|
||||||
|
- ✅ 关键权限路由已加 `RequirePermission` middleware
|
||||||
|
- ✅ TOTP 验证已绑定 password login challenge
|
||||||
|
- ✅ 未实现接口已改为 fail-closed (503)
|
||||||
|
- ✅ Bootstrap secret 已加恒定时间比较
|
||||||
|
- ✅ 密码历史已改为同步事务写入
|
||||||
|
- ⚠️ 建议：添加 `/users/:id` 权限回归测试到 CI
|
||||||
|
- ⚠️ 建议：添加 `temp_token` 过期/重用检测测试
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 最终评级（更新）
|
||||||
|
|
||||||
|
| 维度 | 原评级 | 当前评级 | 变化 |
|
||||||
|
|------|--------|----------|------|
|
||||||
|
| 需求 / 实现一致性 | C | B | ⬆️ |
|
||||||
|
| 安全基线 | D | B | ⬆️⬆️ |
|
||||||
|
| 构建与测试基线 | C | A | ⬆️⬆️ |
|
||||||
|
| 可维护性 | B- | B+ | ⬆️ |
|
||||||
|
| 文档真相 | C- | B | ⬆️⬆️ |
|
||||||
|
| **发布就绪度** | **D** | **B** | ⬆️⬆️ |
|
||||||
|
|
||||||
|
**综合评级：B / 有条件就绪**
|
||||||
|
|
||||||
|
> 注：当前已达到"有条件就绪"状态，主要剩余工作为 P2 级别优化和测试覆盖率提升。
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 剩余工作（可选）
|
||||||
|
|
||||||
|
### P2 收口建议
|
||||||
|
1. 清理测试 warning 噪音
|
||||||
|
2. 补真实 API contract 集成测试
|
||||||
|
3. 更新 README / `docs/status/REAL_PROJECT_STATUS.md`
|
||||||
|
4. 覆盖率提升至 60%+
|
||||||
|
5. 前端 dev toolchain 漏洞升级（vite）
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 关联文档
|
||||||
|
|
||||||
|
- [review-fix-closure-2026-05-28.md](./review-fix-closure-2026-05-28.md) - 前两轮修复收口
|
||||||
|
- [HERMES_FULL_REVIEW_2026-05-27.md](./HERMES_FULL_REVIEW_2026-05-27.md) - 原始 review 报告
|
||||||
|
- [REVIEW_CONSOLIDATION_REPORT.md](../reviews/REVIEW_CONSOLIDATION_REPORT.md) - 专家 review 汇总
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
*文档生成时间：2026-05-29*
|
||||||
|
*验证提交：363c77d "feat: atomic TOTP verification for DisableTOTP"*
|
||||||
Reference in New Issue
Block a user