314 lines
12 KiB
Markdown
314 lines
12 KiB
Markdown
# 代码审查标准与流程规范
|
||
|
||
**文档版本**: 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 审查工具配置
|
||
|
||
```yaml
|
||
# .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 格式示例
|
||
|
||
```markdown
|
||
🔴 **安全：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*
|