Files
user-system/docs/code-review/OPTIMIZATION_PLAN_2026-04-03.md

239 lines
6.1 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 系统性优化方案 - 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<T>`),不验证运行时响应结构。`result.data` 可能是 `undefined`
**受影响文件**:
- `frontend/admin/src/lib/http/client.ts:87-89, 240-245`
### 修复方案
添加运行时验证:
```typescript
interface ApiResponse<T> {
code: number
message: string
data?: T
}
function isApiResponse(obj: unknown): obj is ApiResponse<unknown> {
if (typeof obj !== 'object' || obj === null) return false
const r = obj as Record<string, unknown>
return typeof r.code === 'number' && typeof r.message === 'string'
}
async function parseJsonResponse<T>(response: Response): Promise<T> {
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 | ✅ | 需要补充测试 | - | 集成测试 |