- Fix DIP violations in service layer (device, stats, auth middleware) - Add ReplaceUserRoles interface method for transaction safety - Implement Magic Bytes validation for avatar uploads - Standardize OAuth error handling with ErrOAuthProviderNotSupported - Use crypto/rand for JWT secret generation instead of weak fixed key - Apply code formatting with gofumpt and goimports - Fix staticcheck issues (S1024, S1008, ST1005) - Add comprehensive quality and functional test reports - Achieve 36.3% test coverage (up from 16.3%) - All E2E, integration, and business logic tests passing
14 KiB
代码审查流程规范 v2.0
文档版本: v2.0
更新日期: 2026-04-12
适用范围: User Management System (UMS) 项目
配套标准: CODE_REVIEW_STANDARD_V4.md
配套 Checklist: REVIEW_EXECUTION_CHECKLIST.md
一、核心原则
1.1 零信任文档原则
任何"已完成"的声明,必须附带可重现的命令和输出,否则视为未完成。
历史教训:
- v2.0 时期因依赖文档自述,评分虚高至 9.7/10
- 2026-04-11 发现前端构建实际失败,但文档标注 "PASS"
- v4.0 要求:工具证据先于文档断言
1.2 教学优先原则
审查的目的是让代码更好、让开发者成长,不是门卫把关:
- 每个问题说明"为什么是问题",而非只说"改掉"
- 赞扬好的实践,具体表扬有教学价值
1.3 优先级纪律
| 级别 | 处理规则 | 不遵守的后果 |
|---|---|---|
| 🔴 P0 | 禁止合并,4h 内修复 | 永久 Block |
| 🟠 P1 | 禁止合并,当天修复 | 永久 Block |
| 🟡 P2 | 附计划后可合并,本周修复 | 跟踪 Issue |
| 🔵 P3 | 可合并,本 Sprint 修复 | 技术债台账 |
| 💭 P4 | 可忽略 | - |
二、角色与职责
| 角色 | 职责 | SLA |
|---|---|---|
| 作者 | 自审 → 提 PR → 修复问题 → 更新文档 | 当日响应 |
| 审查者 | 执行 Checklist → 标注问题 → 给出建议 → Approve | P0:1h / P1:4h / P2:8h |
| Tech Lead | SLA 超时升级,争议仲裁,流程优化 | 1个工作日 |
2.1 作者自审清单(提 PR 前必须执行)
# 最小自审(2分钟)
cd d:\usersystem
go build ./cmd/server # 必须通过
go vet ./... # 必须通过
go test ./... -short -count=1 # 必须通过
cd frontend\admin
npm.cmd run lint # 必须通过
npm.cmd run build # 必须通过 ← 重点,历史有谎报
2.2 作者 PR 描述模板
## 变更目的
[1-2句说明:解决什么问题,为什么这样解决]
## 影响范围
- [ ] 后端(Go)
- [ ] 前端(React/TypeScript)
- [ ] 数据库(schema变更)
- [ ] 部署配置
- [ ] 文档
## 验证命令与结果
```bash
$ go build ./cmd/server
# 输出: [粘贴实际输出]
$ go test ./... -short
# 输出: ok ... [粘贴实际输出]
$ npm.cmd run build
# 输出: [粘贴实际输出]
是否需要 E2E 测试?
- 是 → 已执行
npm.cmd run e2e:full:win(粘贴结果) - 否 → 理由:[说明为何不需要]
剩余已知问题(P2及以下)
- [问题1] #issue-link
---
## 三、审查执行流程(SOP)
┌─────────────────────────────────────────────────────────────────┐ │ 作者自审 + 提 PR │ │ □ go build / go vet / go test -short 全通过 │ │ □ npm lint / npm build 全通过(无 TS 错误!) │ │ □ PR 描述包含验证命令输出 │ └─────────────────────────┬───────────────────────────────────────┘ ▼ ┌─────────────────────────────────────────────────────────────────┐ │ 阶段 1:自动化门禁(CI,约5分钟) │ │ □ go build + go vet + go test -race │ │ □ 覆盖率 ≥ 60% │ │ □ govulncheck(无已知CVE) │ │ □ npm lint + npm build + npm test │ │ □ npm audit(high漏洞=0) │ │ │ │ ⚠️ 任一失败 → PR 自动 Block,作者修复后重新触发 │ └─────────────────────────┬───────────────────────────────────────┘ ▼(CI 全通过) ┌─────────────────────────────────────────────────────────────────┐ │ 阶段 2:审查者人工审查(10-20分钟) │ │ │ │ 按优先级审查顺序: │ │ 1. 安全维度(P0 优先)—— 新 API 权限?文件上传?SQL 注入? │ │ 2. API 契约 —— 响应格式统一?HTTP状态码正确? │ │ 3. 前后端集成 —— 路径/字段名/类型一致? │ │ 4. 业务逻辑 —— 功能正确?边界处理? │ │ 5. 测试质量 —— 测试是真实的?非虚假断言? │ │ 6. 运维影响 —— 配置变更?Runbook 需要更新? │ │ │ │ 使用 REVIEW_EXECUTION_CHECKLIST.md 逐项执行 │ └─────────────────────────┬───────────────────────────────────────┘ ▼ ┌─────────────────────────────────────────────────────────────────┐ │ 阶段 3:问题标注 │ │ 使用标准格式(见第四节) │ │ P0/P1 → 逐项说明问题+原因+建议修复 │ │ P2/P3 → 可集中列表 │ │ 亮点 → 至少指出 1 个好的做法 │ └─────────────────────────┬───────────────────────────────────────┘ ▼ ┌─────────────────────────────────────────────────────────────────┐ │ 阶段 4:作者修复 │ │ P0/P1 → 修复后回复每条评论,附命令输出证明 │ │ P2 → 修复或创建 Issue 跟踪,评论 Issue 链接 │ │ P3 → 修复或在 PR 评论说明原因 │ └─────────────────────────┬───────────────────────────────────────┘ ▼ ┌─────────────────────────────────────────────────────────────────┐ │ 阶段 5:E2E 检查(条件触发) │ │ 触发条件(满足任一): │ │ - 认证相关变更 │ │ - 路由守卫变更 │ │ - 导航组件变更 │ │ - Token 管理变更 │ │ 命令:cd frontend/admin && npm.cmd run e2e:full:win │ └─────────────────────────┬───────────────────────────────────────┘ ▼ ┌─────────────────────────────────────────────────────────────────┐ │ 阶段 6:Approve + 合并 │ │ □ 所有 🔴🟠 问题已修复(有验证命令证明) │ │ □ P2 有 Issue 跟踪计划 │ │ □ 覆盖率未下降 > 5% │ │ □ 文档已同步(API 变更 → Swagger,配置变更 → .env.example) │ │ □ Approve │ └─────────────────────────────────────────────────────────────────┘
---
## 四、审查评论格式规范
### 问题标注格式
```markdown
🔴 **[P0 - 安全] 文件上传缺少 Magic Bytes 校验**
📍 位置:`internal/api/handler/avatar_handler.go:95`
**问题描述**:
当前仅校验文件扩展名,攻击者可将 PHP Shell 命名为 `.jpg` 绕过检查。
**风险**:
恶意文件可能被服务端执行,导致 RCE(远程代码执行)。
**建议修复**:
```go
src, _ := file.Open()
buf := make([]byte, 512)
n, _ := src.Read(buf)
contentType := http.DetectContentType(buf[:n])
allowedMIME := map[string]bool{
"image/jpeg": true, "image/png": true,
}
if !allowedMIME[contentType] {
c.JSON(400, gin.H{"message": "invalid file content"})
return
}
src.Seek(0, io.SeekStart)
🟡 [P2 - 可维护性] context.Background() 在请求链路中截断追踪
📍 位置:internal/api/middleware/auth.go:131
问题描述:缓存查询使用 context.Background() 而非请求 context,导致 Trace ID 无法传播。
建议:将函数签名改为接收 ctx context.Context,传递调用者的 context。
✅ 做得好:Argon2id 密码哈希配置优秀
internal/auth/password.go 中 64MB 内存、5次迭代的 Argon2id 配置超越行业基准,
有效防御 GPU 暴力破解。
### Approve 评论格式
```markdown
## 审查结论
✅ **批准合并**
**综合评分**:X.X/10
**亮点**:
- Argon2id 配置超越行业基准
- 游标分页 P99=53ms,性能优秀
**遗留 P2(已有 Issue 跟踪)**:
- #123 OpenAPI 注释完善
- #124 pagination 包测试
**合并后 24h 内请确认**:
- 生产监控无异常告警
- 关键业务指标(登录成功率)正常
LGTM 🚀
五、审查时效 SLA
| PR 优先级 | 首次审查 | 修复后复核 | 最大总周期 |
|---|---|---|---|
| P0 安全紧急 | 30 分钟 | 15 分钟 | 2 小时 |
| P1 重要修复 | 1 小时 | 30 分钟 | 4 小时 |
| P2 常规功能 | 4 小时 | 2 小时 | 24 小时 |
| P3 重构/文档 | 8 小时 | 4 小时 | 48 小时 |
超时处理
超 SLA 50% → 作者 @审查者 催促
超 SLA 100% → 作者 @Tech Lead 升级
超 3 个工作日无响应 → Tech Lead 仲裁
六、特殊场景处理
6.1 大型 PR(>500 行)
优先请求作者拆分,按以下维度拆:
- 后端/前端 分开
- 功能/测试 分开
- 重构/新功能 分开
如必须整体审查:
1. 分批审查(核心安全逻辑优先)
2. 明确标记哪些部分已审查
3. 剩余部分安排跟进审查
6.2 生产紧急修复(Hotfix)
流程:
1. Tech Lead 批准先合并(P0 安全问题)
2. 24 小时内完成完整审查
3. 发现问题立即 Hotfix v2
4. 72 小时内完成事后复盘
条件:
- 只允许 P0 安全/稳定性问题
- 必须在 Hotfix 分支(hotfix/XXX)
- 合并后必须同步更新所有文档
6.3 安全相关变更(额外严格)
触发条件(满足任一):
- 认证/授权逻辑
- 密码/Token 处理
- 文件上传
- 外部服务调用(OAuth/SMS/Email)
- 数据库 schema(含敏感字段)
额外要求:
- Tech Lead 必须参与审查
- 发布前必须运行完整安全扫描(gosec + govulncheck)
- 需要额外的攻击场景测试
七、争议解决
| 争议类型 | 解决方式 |
|---|---|
| 问题级别分歧 | 参照标准,模糊取高;Tech Lead 终裁 |
| 是否必须修复 | 审查者决定,作者提仲裁请求 |
| 技术方案选择 | 量化数据支持(性能/复杂度),Tech Lead 仲裁 |
| 代码风格偏好 | 参考项目规范,无标准则接受 |
八、审查记录归档
| 内容 | 位置 | 保存期限 |
|---|---|---|
| 全面审查报告 | docs/code-review/COMPREHENSIVE_REVIEW_YYYY-MM-DD.md |
永久 |
| 专项审查报告 | docs/code-review/[主题]_REVIEW_YYYY-MM-DD.md |
永久 |
| 问题跟踪 | Gitea Issues | 永久 |
| 工具扫描结果 | docs/evidence/ |
90天 |
九、持续改进
9.1 回顾周期
| 周期 | 内容 | 负责人 |
|---|---|---|
| 每次 Sprint 结束 | 审查效率/质量小结 | Tech Lead |
| 每月 | 流程优化讨论(缺陷逃逸率、审查一致性) | Team |
| 每季度 | 标准文档更新(CODE_REVIEW_STANDARD_VX.md) | 代码审查专家 |
9.2 关键质量指标(目标)
| 指标 | 当前 | 目标 |
|---|---|---|
| 缺陷逃逸率 | 未量化 | < 2个/Sprint |
| P0/P1 修复时效 | 未量化 | 100% 在 SLA 内 |
| 审查覆盖率 | 100% | 保持 |
| 虚假完成率 | 历史有案例 | 0 |
文档版本: v2.0
更新时间: 2026-04-12
主要变更: 新增零信任文档原则 + SOP 流程图 + E2E 触发条件 + 各维度专项检查要求