Files
user-system/docs/code-review/CODE_REVIEW_STANDARD.md

12 KiB
Raw Permalink Blame History

代码审查标准与流程规范

文档版本: v1.0
生成日期: 2026-03-29
适用范围: User Management System (UMS) 项目


一、审查目标

本规范旨在建立系统化的代码审查机制,确保代码质量达到生产级标准,同时提升团队成员的技术能力和协作效率。


二、审查范围

2.1 技术栈覆盖

层级 技术 审查重点
后端 Go + Gin + Gorm 安全性、性能、并发安全
前端 React + TypeScript + Ant Design 组件质量、类型安全、用户体验
数据库 PostgreSQL 索引、查询优化、事务安全
基础设施 Docker, CI/CD 部署安全、配置管理

2.2 代码分类审查要求

代码类型 审查深度 必须审查项
认证/鉴权 深度审查 安全漏洞、权限绕过、Token 安全
支付/敏感操作 深度审查 数据完整性、幂等性、审计日志
数据查询 标准审查 SQL 注入、N+1 查询、索引
业务逻辑 标准审查 错误处理、边界条件
工具/辅助函数 简化审查 可测试性、边界情况
UI/样式 简化审查 可访问性、响应式

三、审查标准

3.1 安全标准(🔴 必须通过)

规则 ID 规则描述 检查方法 违规处理
SEC-01 禁止 SQL 注入 代码扫描 + 参数化查询 🔴 阻塞
SEC-02 禁止 XSS 漏洞 输入验证 + 输出编码 🔴 阻塞
SEC-03 认证接口必须有限流 检查中间件配置 🔴 阻塞
SEC-04 敏感操作必须二次验证 检查 verifySensitiveAction 🔴 阻塞
SEC-05 Token 必须安全存储 检查 HttpOnly + Secure 🔴 阻塞
SEC-06 禁止硬编码密钥 扫描 secrets/keys 🔴 阻塞
SEC-07 禁止明文存储密码/恢复码 检查哈希算法 🔴 阻塞
SEC-08 禁止信任客户端输入 检查 validation 🔴 阻塞
SEC-09 必须使用 crypto/rand 生成密钥 检查随机数生成器 🔴 阻塞
SEC-10 禁止忽略随机数生成错误 检查 rand.Read 错误处理 🔴 阻塞
SEC-11 Webhook URL 必须 SSRF 过滤 检查 isSafeURL 调用 🔴 阻塞

3.2 正确性标准(🟡 必须修复)

规则 ID 规则描述 建议处理
CORR-01 错误必须被处理 不允许忽略 error
CORR-02 并发访问必须同步 检查 goroutine + mutex
CORR-03 资源必须释放 defer/cleanup 审查
CORR-04 边界条件必须处理 nil/empty/zero 审查
CORR-05 事务边界必须正确 检查 Begin/Commit/Rollback

3.3 性能标准(🟡 建议修复)

规则 ID 规则描述 建议处理
PERF-01 禁止 N+1 查询 使用批量查询
PERF-02 禁止循环内数据库操作 重构到循环外
PERF-03 禁止重复编译正则表达式 预编译并复用
PERF-04 大数据必须分页 检查 pageSize 限制
PERF-05 缓存必须设置 TTL 检查过期策略

3.4 可维护性标准(💭 建议优化)

规则 ID 规则描述 建议处理
MAIN-01 函数长度不超过 50 行 拆分函数
MAIN-02 禁止重复代码 提取公共函数
MAIN-03 命名必须有意义 检查变量/函数名
MAIN-04 必须添加注释 复杂逻辑必须有注释
MAIN-05 魔法数字必须定义常量 替换为具名常量

四、审查流程

4.1 流程图

┌─────────────────────────────────────────────────────────────────┐
│                        代码提交阶段                              │
└───────────────────────────┬─────────────────────────────────────┘
                            ▼
┌─────────────────────────────────────────────────────────────────┐
│  1. 自审查 (Self Review)                                        │
│     - 开发者对照检查清单进行自检                                  │
│     - 运行单元测试和 lint 检查                                   │
└───────────────────────────┬─────────────────────────────────────┘
                            ▼
┌─────────────────────────────────────────────────────────────────┐
│  2. 代码审查 (Code Review) - 必须 1 人以上                        │
│     - 审查者检查安全问题、性能问题                                │
│     - 给出修改建议                                              │
│     - 标记阻塞/建议/挑剔级别                                     │
└───────────────────────────┬─────────────────────────────────────┘
                            ▼
┌─────────────────────────────────────────────────────────────────┐
│  3. 问题修复 (Fix Phase)                                         │
│     - 🔴 阻塞问题:必须修复后才能合并                             │
│     - 🟡 建议问题:应在本次或近期迭代修复                         │
│     - 💭 挑剔问题:鼓励修复,可后续处理                           │
└───────────────────────────┬─────────────────────────────────────┘
                            ▼
┌─────────────────────────────────────────────────────────────────┐
│  4. 审查通过 (Approval)                                          │
│     - 所有 🔴 问题已修复                                         │
│     - 审查者 approve                                             │
└───────────────────────────┬─────────────────────────────────────┘
                            ▼
┌─────────────────────────────────────────────────────────────────┐
│  5. 合并 (Merge)                                                 │
│     - CI/CD 检查通过                                             │
│     - 合并到目标分支                                             │
└─────────────────────────────────────────────────────────────────┘

4.2 审查角色

角色 职责 要求
作者 (Author) 自审查、修复问题 熟悉代码逻辑
审查者 (Reviewer) 检查代码、提出建议 了解业务和安全要求
仲裁者 (Arbiter) 解决争议 资深开发者/架构师

4.3 审查工具配置

# .golangci.yml (Go 语言)
linters:
  enable:
    - gosec       # 安全扫描
    - govet       # 代码诊断
    - gocyclo     # 圈复杂度
    - revive      # 代码风格
    - unused      # 未使用代码

# eslint.config.js (前端)
rules:
  security/detect-object-injection: error
  security/detect-non-literal-regexp: error

五、审查检查清单

5.1 安全检查清单

  • 所有用户输入都经过验证
  • 敏感操作需要二次验证
  • SQL 查询使用参数化
  • 密码/恢复码已哈希存储
  • Token 存储使用 HttpOnly
  • 速率限制已配置
  • 错误消息不泄露敏感信息
  • 日志不记录敏感数据
  • 随机数使用 crypto/rand非 math/rand
  • rand.Read 错误被正确处理
  • Webhook URL 经过 SSRF 过滤
  • 非测试代码不使用 panic

5.2 性能检查清单

  • 无 N+1 查询
  • 循环内无数据库操作
  • 正则表达式已预编译
  • 大数据查询已分页
  • 缓存已设置 TTL
  • 无不必要的内存分配

5.3 代码质量检查清单

  • 错误已正确处理
  • 并发访问已同步
  • 资源已正确释放
  • 魔法数字已定义为常量
  • 重复代码已提取
  • 命名有意义
  • 复杂逻辑有注释

六、问题分级标准

🔴 阻塞 (Blocker)

  • 安全漏洞(注入、认证绕过)
  • 数据丢失/损坏风险
  • 编译失败
  • 关键功能不可用

🟡 建议 (Major)

  • 性能问题N+1 查询)
  • 错误处理不当
  • 代码重复
  • 可维护性问题

💭 挑剔 (Minor)

  • 代码风格不一致
  • 命名不够清晰
  • 注释不够完善
  • 轻微优化空间

七、审查评论规范

7.1 格式示例

🔴 **安全SQL注入风险**
位置: `auth.go:42`

**问题**: 用户输入直接拼接到 SQL 查询中。

**原因**: 攻击者可注入 `'; DROP TABLE users; --` 作为 name 参数。

**建议**: 使用参数化查询
```go
db.Query('SELECT * FROM users WHERE name = $1', [name])

### 7.2 评论原则

1. **具体**:指出具体文件和行号
2. **解释原因**:说明为什么这是个问题
3. **提供建议**:给出修复建议或参考资料
4. **保持尊重**:对事不对人

---

## 八、持续改进

### 8.1 审查指标

| 指标 | 目标值 |
|------|--------|
| 平均审查时间 | < 24 小时 |
| 首次审查通过率 | > 60% |
| 阻塞问题数量 | < 5 个/版本 |

### 8.2 知识沉淀

- 审查发现的安全问题同步到 `docs/security/`
- 性能优化案例同步到 `docs/performance/`
- 重大争议同步到 `docs/team/`

---

## 九、附录

### 9.1 参考资源

- OWASP Top 10: https://owasp.org/www-project-top-ten/
- Go Code Review Comments: https://github.com/golang/go/wiki/CodeReviewComments
- Google Engineering Practices: https://google.github.io/eng-practices/

### 9.2 快速检查命令

```bash
# Go 后端
go vet ./...
go build ./cmd/server
go test ./... -count=1

# 前端
cd frontend/admin && npm.cmd run lint
cd frontend/admin && npm.cmd run build
cd frontend/admin && npm.cmd run test

# E2E 测试
cd frontend/admin && npm.cmd run e2e:full:win

9.3 审查周期建议

审查类型 频率 负责人
代码自审 每次提交前 开发者
同行审查 每个 PR 团队成员
安全审查 每月一次 安全负责人
全面审查 每季度一次 代码审查专家

9.4 审查报告模板

审查报告应包含以下部分:

  1. 执行摘要 - 关键指标和总体评估
  2. 问题清单 - 按优先级分类的问题列表
  3. 历史问题验证 - 之前发现问题的修复状态
  4. 代码质量评估 - 各维度评分
  5. 修复建议 - 优先级排序的修复计划
  6. 附录 - 参考文档和工具

本文档由代码审查专家 Agent 生成,版本: v1.0