313 lines
9.7 KiB
Markdown
313 lines
9.7 KiB
Markdown
# 代码审查报告 - 2026-03-30
|
||
|
||
**审查日期**: 2026-03-30
|
||
**审查范围**: 全项目代码（后端 + 前端）
|
||
**审查依据**: PRD_IMPLEMENTATION_GAP_ANALYSIS.md
|
||
**审查轮次**: 第三次深度审查
|
||
|
||
---
|
||
|
||
## 一、审查摘要
|
||
|
||
本次审查对 PRD 文档中的问题进行了第三次验证，重点检查问题修复状态。总体情况如下：
|
||
|
||
| 类别 | 总数 | 已修复 | 未修复 | 修复率 |
|
||
|------|------|--------|--------|--------|
|
||
| 高危安全问题 | 8 | 6 | 2 | 75% |
|
||
| 中危安全问题 | 7 | 2 | 5 | 29% |
|
||
| 性能问题 | 9 | 2 | 7 | 22% |
|
||
| 代码质量问题 | 10 | 2 | 8 | 20% |
|
||
| **总计** | **34** | **12** | **22** | **35%** |
|
||
|
||
---
|
||
|
||
## 二、高危安全问题修复状态
|
||
|
||
### 2.1 🔴 SEC-01: OAuth ValidateToken 方法形同虚设
|
||
|
||
| 项目 | 状态 |
|
||
|------|------|
|
||
| **文件位置** | `internal/auth/oauth.go:438-457` |
|
||
| **问题描述** | 原代码始终返回 true，没有验证 token 有效性 |
|
||
| **修复状态** | ✅ **已修复** |
|
||
| **修复方式** | 改为遍历所有 provider 尝试验证 |
|
||
|
||
**修复后代码**:
|
||
```go
|
||
func (m *DefaultOAuthManager) ValidateToken(token string) (bool, error) {
|
||
if len(token) == 0 {
|
||
return false, nil
|
||
}
|
||
providers := m.GetEnabledProviders()
|
||
if len(providers) == 0 {
|
||
return false, errors.New("no OAuth providers configured")
|
||
}
|
||
tokenObj := &OAuthToken{AccessToken: token}
|
||
for _, p := range providers {
|
||
if _, err := m.GetUserInfo(p.Provider, tokenObj); err == nil {
|
||
return true, nil
|
||
}
|
||
}
|
||
return false, nil
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 2.2 🔴 SEC-02: 敏感操作验证绕过
|
||
|
||
| 项目 | 状态 |
|
||
|------|------|
|
||
| **文件位置** | `internal/service/auth.go:1068-1103` |
|
||
| **问题描述** | 无密码无TOTP时直接返回成功 |
|
||
| **修复状态** | ✅ **已修复** |
|
||
| **修复方式** | 增加前置检查，禁止无凭证用户执行敏感操作 |
|
||
|
||
**修复后代码**:
|
||
```go
|
||
// 如果用户既没有密码也没有启用TOTP，禁止执行敏感操作
|
||
if !hasPassword && !hasTOTP {
|
||
return errors.New("请先设置密码或启用两步验证")
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 2.3 🔴 SEC-03: 恢复码明文存储
|
||
|
||
| 项目 | 状态 |
|
||
|------|------|
|
||
| **文件位置** | `internal/auth/totp.go:121-125`, `internal/service/totp.go:47-52` |
|
||
| **问题描述** | TOTP 恢复码以明文 JSON 存储 |
|
||
| **修复状态** | ✅ **已修复** |
|
||
| **修复方式** | 使用 SHA256 哈希后存储 |
|
||
|
||
**修复后代码**:
|
||
```go
|
||
// Hash recovery codes before storing (SEC-03 fix)
|
||
hashedCodes := make([]string, len(setup.RecoveryCodes))
|
||
for i, code := range setup.RecoveryCodes {
|
||
hashedCodes[i], _ = auth.HashRecoveryCode(code)
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 2.4 🔴 SEC-04: TOTP 算法使用 SHA1
|
||
|
||
| 项目 | 状态 |
|
||
|------|------|
|
||
| **文件位置** | `internal/auth/totp.go:28` |
|
||
| **问题描述** | 代码使用 SHA1 作为 TOTP 算法 |
|
||
| **修复状态** | ❌ **未修复** |
|
||
| **当前代码** | `TOTPAlgorithm = otp.AlgorithmSHA1` |
|
||
|
||
**建议**: 建议升级到 SHA256 或 SHA512
|
||
|
||
---
|
||
|
||
### 2.5 🔴 SEC-05: X-Forwarded-For IP 伪造风险
|
||
|
||
| 项目 | 状态 |
|
||
|------|------|
|
||
| **文件位置** | `internal/api/middleware/ip_filter.go:55-94` |
|
||
| **问题描述** | 中间件直接信任 X-Forwarded-For 请求头 |
|
||
| **修复状态** | ✅ **已修复** |
|
||
| **修复方式** | 增加 TrustProxy 配置，只接受可信代理的 X-Forwarded-For |
|
||
|
||
**修复后代码**:
|
||
```go
|
||
// 如果不信任代理，直接使用 TCP 连接 IP
|
||
if !m.config.TrustProxy {
|
||
return c.ClientIP()
|
||
}
|
||
// 检查是否是可信代理
|
||
if !m.isTrustedProxy(ip) {
|
||
continue // 不是可信代理，跳过
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 2.6 🔴 SEC-06: JTI 包含可预测的时间戳
|
||
|
||
| 项目 | 状态 |
|
||
|------|------|
|
||
| **文件位置** | `internal/auth/jwt.go:65` |
|
||
| **问题描述** | JTI 格式追加了可预测的时间戳 |
|
||
| **修复状态** | ❌ **未修复** |
|
||
| **当前代码** | `return fmt.Sprintf("%x-%d", b, time.Now().UnixNano()), nil` |
|
||
|
||
**建议**: 移除时间戳，仅使用随机数
|
||
|
||
---
|
||
|
||
### 2.7 🔴 SEC-07: OAuth State 验证存在 TOCTOU 竞态条件
|
||
|
||
| 项目 | 状态 |
|
||
|------|------|
|
||
| **文件位置** | `internal/auth/oauth_utils.go:44-63` |
|
||
| **问题描述** | 过期检查和删除操作不在同一个锁区域内 |
|
||
| **修复状态** | ✅ **已修复** |
|
||
| **修复方式** | 使用单个 Lock 替代 RLock+Lock |
|
||
|
||
**修复后代码**:
|
||
```go
|
||
func ValidateState(state string) bool {
|
||
stateStore.mu.Lock() // 使用 Lock 替代 RLock
|
||
defer stateStore.mu.Unlock()
|
||
// 检查和删除在同锁区域内
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 2.8 🔴 SEC-08: 刷新令牌接口缺少速率限制
|
||
|
||
| 项目 | 状态 |
|
||
|------|------|
|
||
| **文件位置** | `internal/api/router/router.go:108` |
|
||
| **问题描述** | `/auth/refresh` 接口没有限流中间件 |
|
||
| **修复状态** | ❌ **未修复** |
|
||
| **当前代码** | `authGroup.POST("/refresh", r.authHandler.RefreshToken)` |
|
||
|
||
**建议**: 添加 `r.rateLimitMiddleware.Login()` 限流
|
||
|
||
---
|
||
|
||
### 2.9 🔴 NEW-SEC-01: Webhook SSRF 风险
|
||
|
||
| 项目 | 状态 |
|
||
|------|------|
|
||
| **文件位置** | `internal/service/webhook.go:175-178, 375-446` |
|
||
| **问题描述** | Webhook URL 未进行 SSRF 过滤 |
|
||
| **修复状态** | ✅ **已修复** |
|
||
| **修复方式** | 添加 isSafeURL 函数进行完整 URL 安全检查 |
|
||
|
||
**修复后代码**:
|
||
```go
|
||
// NEW-SEC-01 修复：检查 URL 安全性
|
||
if !isSafeURL(wh.URL) {
|
||
s.recordDelivery(task, 0, "", "webhook URL 不安全: 可能存在 SSRF 风险", false)
|
||
return
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
## 三、中危安全问题修复状态
|
||
|
||
| ID | 问题 | 文件位置 | 修复状态 |
|
||
|----|------|----------|----------|
|
||
| SEC-09 | CSRF Token 接口无 CSRF 保护 | auth.go:673-683 | ❌ 未修复 |
|
||
| SEC-10 | Session Presence Cookie 不是 HttpOnly | auth.go:117 | ❌ 未修复 |
|
||
| SEC-11 | rand.Read 错误被忽略 | oauth_utils.go:30 | ❌ 未修复 |
|
||
| SEC-12 | 日志泄露敏感信息 | 多处 | ❌ 未修复 |
|
||
| SEC-14 | 默认 Argon2 参数偏弱 | password.go:29 | ❌ 未修复 |
|
||
| SEC-15 | 登录失败时泄露用户存在性 | auth.go:649-652 | ✅ 已修复 |
|
||
| SEC-16 | Cache 失效时锁定机制失效 | auth.go:640-647 | ✅ 已修复 |
|
||
|
||
---
|
||
|
||
## 四、性能问题修复状态
|
||
|
||
| ID | 问题 | 文件位置 | 修复状态 | 备注 |
|
||
|----|------|----------|----------|------|
|
||
| PERF-01 | 认证请求 4 次 DB 查询 | middleware/auth.go:131-177 | ✅ 已优化 | 缓存 TTL 增至 30 分钟 |
|
||
| PERF-02 | OAuth State 无自动清理 | oauth_utils.go:23 | ❌ 未修复 | |
|
||
| PERF-03 | findUserForLogin 串行查询 | auth_runtime.go:32-54 | ❌ 未修复 | |
|
||
| PERF-04 | 限流器清理策略不完善 | ratelimit.go:175 | ❌ 未修复 | |
|
||
| PERF-05 | 重复缓存用户信息 | auth.go:686,1195 | ❌ 未修复 | |
|
||
| PERF-06 | CacheManager 同步双写 | cache_manager.go:42 | ❌ 未修复 | |
|
||
| PERF-07 | goroutine 无超时写 DB | auth.go:470 | ❌ 未修复 | |
|
||
| PERF-08 | L1Cache 无自动清理 | l1.go | ❌ 未修复 | |
|
||
| PERF-09 | AnomalyDetector 无上限 | ip_filter.go:258 | ❌ 未修复 | |
|
||
|
||
---
|
||
|
||
## 五、代码质量问题修复状态
|
||
|
||
| ID | 问题 | 文件位置 | 修复状态 |
|
||
|----|------|----------|----------|
|
||
| 5.1.1 | N+1 查询 | middleware/auth.go:131-177 | ✅ 已优化 |
|
||
| 5.1.2 | 正则重复编译 | validator.go:98-101 | ❌ 未修复 |
|
||
| 5.1.3 | 设备查询无分页限制 | device.go:142-152 | ❌ 未修复 |
|
||
| 5.2.1 | 敏感操作验证绕过 | auth.go:1068-1103 | ✅ 已修复 |
|
||
| 5.2.2 | 设备字段长度未校验 | device.go:52-92 | ❌ 未修复 |
|
||
| 5.2.3 | 登录日志异步写入无告警 | auth.go:470-474 | ❌ 未修复 |
|
||
| 5.3.1 | 用户名生成循环查询 | auth.go:262-271 | ❌ 未修复 |
|
||
| 5.3.2 | 字符串处理重复 | 多处 | ❌ 未修复 |
|
||
| 5.3.3 | 错误处理不一致 | auth.go 多处 | ❌ 未修复 |
|
||
| 5.3.4 | 魔法数字 | 多处 | ❌ 未修复 |
|
||
|
||
---
|
||
|
||
## 六、修复情况总结
|
||
|
||
### 6.1 已修复问题清单（12 个）
|
||
|
||
| 优先级 | 问题 ID | 问题描述 |
|
||
|--------|---------|----------|
|
||
| 🔴 P0 | SEC-01 | OAuth ValidateToken 始终返回 true |
|
||
| 🔴 P0 | SEC-02 | 敏感操作验证绕过 |
|
||
| 🔴 P0 | SEC-03 | 恢复码明文存储 |
|
||
| 🔴 P0 | SEC-05 | X-Forwarded-For IP 伪造风险 |
|
||
| 🔴 P0 | SEC-07 | OAuth State TOCTOU 竞态 |
|
||
| 🔴 P0 | NEW-SEC-01 | Webhook SSRF 风险 |
|
||
| 🟡 P1 | SEC-15 | 登录失败时泄露用户存在性 |
|
||
| 🟡 P1 | SEC-16 | Cache 失效时锁定机制失效 |
|
||
| 🟡 P1 | PERF-01 | 认证请求 4 次 DB 查询 |
|
||
| 🟡 P1 | 5.1.1 | N+1 查询 |
|
||
| 🟡 P1 | 5.2.1 | 敏感操作验证绕过 |
|
||
|
||
### 6.2 未修复问题清单（22 个）
|
||
|
||
#### 🔴 仍需修复（4 个）
|
||
1. SEC-04: TOTP 算法使用 SHA1
|
||
2. SEC-06: JTI 包含可预测的时间戳
|
||
3. SEC-08: refresh 接口缺少速率限制
|
||
4. SEC-09: CSRF Token 接口无 CSRF 保护
|
||
|
||
#### 🟡 建议修复（18 个）
|
||
- SEC-10 ~ SEC-14, SEC-11, SEC-12
|
||
- PERF-02 ~ PERF-09
|
||
- 5.1.2 ~ 5.3.4
|
||
|
||
---
|
||
|
||
## 七、整体评估
|
||
|
||
### 7.1 修复质量评估
|
||
|
||
| 评估项 | 评分 | 说明 |
|
||
|--------|------|------|
|
||
| 修复完整性 | 35% | 34 个问题中修复 12 个 |
|
||
| 修复质量 | 高 | 已修复问题代码质量良好 |
|
||
| 安全提升 | 显著 | 6/8 高危问题已修复 |
|
||
|
||
### 7.2 风险等级
|
||
|
||
| 等级 | 剩余问题 | 风险说明 |
|
||
|------|----------|----------|
|
||
| 🔴 高危 | 2 个 | TOTP SHA1, JTI 时间戳 |
|
||
| 🟡 中危 | 5 个 | Cookie HttpOnly, 限流等 |
|
||
| 💭 低危 | 15 个 | 性能优化、代码质量 |
|
||
|
||
### 7.3 建议
|
||
|
||
1. **立即修复剩余 2 个高危问题**: SEC-04, SEC-06
|
||
2. **尽快修复 SEC-08**: refresh 接口限流
|
||
3. **规划修复中危问题**: 特别是 SEC-09, SEC-10
|
||
4. **持续优化性能问题**: 特别是 N+1 查询相关
|
||
|
||
---
|
||
|
||
## 八、文档更新
|
||
|
||
本次审查更新了以下文档：
|
||
- `docs/code-review/CODE_REVIEW_REPORT_2026-03-30.md`（本报告）
|
||
|
||
---
|
||
|
||
*本报告由代码审查专家 Agent 生成，审查日期：2026-03-30*
|