diff --git a/docs/code-review/OPTIMIZATION_PLAN_2026-04-03.md b/docs/code-review/OPTIMIZATION_PLAN_2026-04-03.md new file mode 100644 index 0000000..9963379 --- /dev/null +++ b/docs/code-review/OPTIMIZATION_PLAN_2026-04-03.md @@ -0,0 +1,238 @@ +# 系统性优化方案 - 2026-04-03 + +## 待处理问题 + +| # | 类别 | 问题 | 严重度 | 优先级 | +|---|------|------|--------|--------| +| 1 | 前端 | OAuth auth_url 缺少 origin 验证 | MEDIUM | P1 | +| 2 | 前端 | API 响应缺少运行时类型验证 | MEDIUM | P2 | +| 3 | 后端 | 缓存击穿 (cache stampede) 风险 | MEDIUM | P2 | +| 4 | 后端 | Webhook 服务 Goroutine 生命周期管理 | MEDIUM | P2 | + +--- + +## 1. OAuth auth_url 缺少 origin 验证 + +### 问题描述 +`window.location.assign(result.auth_url)` 直接使用后端返回的 `auth_url`,未验证其 origin 是否属于可信域。 + +**风险**: 后端被攻击或配置错误时可能返回恶意 URL 导致用户被重定向到钓鱼站点。 + +**受影响文件**: +- `frontend/admin/src/pages/auth/LoginPage/LoginPage.tsx:137-141` +- `frontend/admin/src/pages/admin/ProfileSecurityPage/ProfileSecurityPage.tsx:195-205` + +### 修复方案 + +在 `oauth.ts` 添加 origin 验证函数: + +```typescript +export function validateOAuthUrl(authUrl: string): boolean { + try { + const url = new URL(authUrl) + // 仅允许同源或已知可信 OAuth 提供商 + const trustedOrigins = [ + 'https://github.com', + 'https://google.com', + 'https://facebook.com', + // ... 其他可信提供商 + ] + return url.origin === window.location.origin || + trustedOrigins.includes(url.origin) + } catch { + return false + } +} +``` + +然后在调用前验证: + +```typescript +const result = await getOAuthAuthorizationUrl(provider, ...) +if (!validateOAuthUrl(result.auth_url)) { + throw new Error('Invalid OAuth authorization URL') +} +window.location.assign(result.auth_url) +``` + +### 验证方式 +- 单元测试覆盖恶意 URL 场景 +- E2E 测试验证重定向行为 + +--- + +## 2. API 响应缺少运行时类型验证 + +### 问题描述 +`parseJsonResponse` 使用 TypeScript 类型断言 (`as ApiResponse`),不验证运行时响应结构。`result.data` 可能是 `undefined`。 + +**受影响文件**: +- `frontend/admin/src/lib/http/client.ts:87-89, 240-245` + +### 修复方案 + +添加运行时验证: + +```typescript +interface ApiResponse { + code: number + message: string + data?: T +} + +function isApiResponse(obj: unknown): obj is ApiResponse { + if (typeof obj !== 'object' || obj === null) return false + const r = obj as Record + return typeof r.code === 'number' && typeof r.message === 'string' +} + +async function parseJsonResponse(response: Response): Promise { + const raw = await response.json() + + if (!isApiResponse(raw)) { + throw new Error('Invalid API response structure') + } + + if (raw.code !== 0) { + throw AppError.fromResponse(raw, response.status) + } + + if (raw.data === undefined) { + throw new Error('API response missing data field') + } + + return raw.data as T +} +``` + +### 验证方式 +- 用 mock 测试验证无效响应被正确拒绝 +- 用有效响应验证数据正确解析 + +--- + +## 3. 缓存击穿 (Cache Stampede) 风险 + +### 问题描述 +`AuthMiddleware.isJTIBlacklisted` 在 L1 cache miss 时,所有并发请求同时打 L2 (Redis),无 singleflight 保护。 + +**受影响文件**: +- `internal/api/middleware/auth.go:113-130` + +### 修复方案 + +使用 `golang.org/x/sync/singleflight`: + +```go +import "golang.org/x/sync/singleflight" + +var sfGroup singleflight.Group + +func (m *AuthMiddleware) isJTIBlacklisted(jti string) bool { + if jti == "" { + return false + } + + key := "jwt_blacklist:" + jti + if _, ok := m.l1Cache.Get(key); ok { + return true + } + + if m.cacheManager != nil { + // 使用 singleflight 防止缓存击穿 + val, err := sfGroup.Do(key, func() (interface{}, error) { + return m.cacheManager.Get(context.Background(), key) + }) + if err == nil && val != nil { + return true + } + } + + return false +} +``` + +### 验证方式 +- 并发测试验证 singleflight 正确聚合请求 +- 单元测试验证缓存未命中时行为正确 + +--- + +## 4. Webhook 服务 Goroutine 生命周期管理 + +### 问题描述 +`WebhookService` 启动 worker pool 和 `time.AfterFunc` 重试,无 `Shutdown()` 方法。服务器关闭时 goroutine 被强制终止,可能丢失 delivery 记录。 + +**受影响文件**: +- `internal/service/webhook.go:110-123, 235-254` + +### 修复方案 + +添加 `Shutdown` 方法: + +```go +// Shutdown 优雅关闭 Webhook 服务 +// 等待所有处理中的 delivery 完成,最多等待 timeout +func (s *WebhookService) Shutdown(ctx context.Context) error { + // 1. 停止接收新任务 + close(s.queue) + + // 2. 等待现有 worker 完成 + done := make(chan struct{}) + go func() { + s.wg.Wait() + close(done) + }() + + select { + case <-done: + // 正常完成 + case <-ctx.Done(): + return ctx.Err() + } + + // 3. 清理 AfterFunc 重试(可选:等待或取消) + // time.AfterFunc 无法直接取消,但 goroutine 会在下一次执行时检查 shutdown 状态 + + return nil +} +``` + +在 `main.go` 的服务器关闭逻辑中调用: + +```go +// 创建带超时的 context +shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 30*time.Second) +defer shutdownCancel() + +// 关闭 Webhook 服务 +if err := webhookSvc.Shutdown(shutdownCtx); err != nil { + log.Printf("Webhook service shutdown error: %v", err) +} +``` + +### 验证方式 +- 单元测试验证 Shutdown 等待 worker 完成 +- 集成测试验证关闭时 delivery 记录不丢失 + +--- + +## 优先级排序 + +| 优先级 | 问题 | 理由 | +|--------|------|------| +| P1 | OAuth origin 验证 | 直接影响用户安全,有实际攻击面 | +| P2 | 缓存击穿 | 高并发场景下影响性能,不是立即安全问题 | +| P2 | API 响应验证 | 数据一致性,但现有代码已有基础校验 | +| P2 | Webhook 生命周期 | 关闭时可能丢数据,但不是高频场景 | + +--- + +## 验证矩阵 + +| 问题 | go build | go test | npm run build | 手动验证 | +|------|----------|---------|---------------|----------| +| OAuth origin | - | 需要补充测试 | - | E2E | +| API 响应验证 | - | 需要补充测试 | - | 单元测试 | +| 缓存击穿 | ✅ | 需要补充测试 | - | 并发测试 | +| Webhook shutdown | ✅ | 需要补充测试 | - | 集成测试 |