fix: harden avatar upload path and sync review truth
This commit is contained in:
Your Name
@@ -60,6 +60,70 @@
|
||||
- `internal/api/middleware/ratelimit.go`
|
||||
- `frontend/admin/scripts/run-playwright-auth-e2e.sh`
|
||||
|
||||
### 7. 内存限流器全局误伤与条目泄漏风险
|
||||
- 问题：`internal/api/middleware/ratelimit.go` 之前按 endpoint 只创建单一 limiter，导致同一接口上的所有用户共享一个桶；同时缺少空闲条目清理策略，无法对历史 client key 做收敛。
|
||||
- 修复：改为按 `endpoint + user_id/IP` 分桶，并在访问路径上按 TTL 清理长期空闲的 limiter 条目。
|
||||
- 回归测试：
|
||||
- 不同 IP 的登录限流相互独立
|
||||
- 共享 IP 下不同 `user_id` 的 API 限流相互独立
|
||||
- 空闲 limiter 会被清理，不再无限累积
|
||||
- 涉及文件：
|
||||
- `internal/api/middleware/ratelimit.go`
|
||||
- `internal/api/middleware/ratelimit_test.go`
|
||||
|
||||
### 8. handler context 类型断言补强
|
||||
- 问题：`SSOHandler` 与 `WebhookHandler` 仍存在 `user_id.(int64)` / `username.(string)` 直接断言，若 middleware 注入异常类型会触发 panic。
|
||||
- 修复：统一复用 `getUserIDFromContext` / `getUsernameFromContext`，类型不匹配时返回 `401 unauthorized`，避免 handler panic。
|
||||
- 回归测试：
|
||||
- `SSOHandler.Authorize` 非法 context 类型返回 `401`
|
||||
- `SSOHandler.UserInfo` 非法 context 类型返回 `401`
|
||||
- `WebhookHandler.CreateWebhook/ListWebhooks` 非法 context 类型返回 `401`
|
||||
- 涉及文件：
|
||||
- `internal/api/handler/auth_handler.go`
|
||||
- `internal/api/handler/sso_handler.go`
|
||||
- `internal/api/handler/webhook_handler.go`
|
||||
- `internal/api/handler/context_guard_test.go`
|
||||
|
||||
### 9. 密码强度 + 静默错误补强
|
||||
- 问题：review 报告中指出两类尾部问题：
|
||||
- 默认密码校验对刚好达到最小长度的短密码过于宽松
|
||||
- TOTP / 操作日志链路存在 `_ = err`、`_ = json.Unmarshal(...)`、`_ = repo.Create(...)` 这类静默吞错
|
||||
- 修复：
|
||||
- `validatePasswordStrength` 改为对“刚好达到最小长度”的密码要求至少 3 种字符类型；较长密码仍保留 2 种类型可过的兼容行为
|
||||
- `TOTPService` 对恢复码摘要、JSON 编解码、`UpdateTOTP` 持久化失败全部显式返回错误，不再静默忽略
|
||||
- `OperationLogMiddleware` 对 nil repo fail-safe 返回；异步落库失败改为写日志，不再无声吞错
|
||||
- 回归测试：
|
||||
- 8 位两类字符密码被拒绝，8 位三类字符密码通过，较长两类字符密码仍通过
|
||||
- 损坏的恢复码 JSON 会返回解析错误
|
||||
- 恢复码消费后持久化失败会显式返回更新错误
|
||||
- operation log 在 nil repo 情况下不会 panic，参数脱敏/非 JSON fallback 继续受测
|
||||
- 涉及文件：
|
||||
- `internal/service/auth.go`
|
||||
- `internal/service/auth_service_test.go`
|
||||
- `internal/service/auth_password_internal_test.go`
|
||||
- `internal/service/totp.go`
|
||||
- `internal/service/totp_internal_test.go`
|
||||
- `internal/api/middleware/operation_log.go`
|
||||
- `internal/api/middleware/operation_log_test.go`
|
||||
|
||||
### 10. review 报告真相校准 + avatar 路径硬化
|
||||
- 真相校准：`PROJECT_REVIEW_REPORT.md` 中一批条目已不再代表当前仓库真相，至少包括：
|
||||
- `uploadAvatar` 字段名错误：前后端当前都使用 `avatar`，该条为陈旧误报
|
||||
- `StateManager` 无法停止、`L1Cache` 无容量限制、密码强度过宽松、操作日志未转义、Webhooks 客户端全量分页、`ContactBindingsSection` 未复用：均已在后续提交中关闭
|
||||
- 仍值得继续跟踪、但已不构成功能 blocker 的尾项：
|
||||
- `social_account_repo.go` 仍是原生 SQL 实现
|
||||
- `AuthProvider` 仍保留 React state + session store 双轨状态管理
|
||||
- `ApiResponse.data` 空值建模仍偏乐观（`T` 而非 `T | null`）
|
||||
- 本轮额外修复：
|
||||
- 将头像上传目录从运行时相对路径解析改为绝对路径归一化，避免 cwd 漂移导致文件落盘位置不稳定
|
||||
- 扩展名校验统一转小写，避免 `.JPG/.PNG` 这类常见文件名被误拒
|
||||
- 回归测试：
|
||||
- `resolveAvatarUploadDir("")` 返回绝对路径且收敛到 `/uploads/avatars`
|
||||
- 自定义根目录会被保留并归一化到 `<root>/avatars`
|
||||
- 涉及文件：
|
||||
- `internal/api/handler/avatar_handler.go`
|
||||
- `internal/api/handler/avatar_handler_path_test.go`
|
||||
|
||||
## 验证结果
|
||||
|
||||
### 后端
|
||||
|
||||
@@ -28,7 +28,7 @@
|
||||
- 浏览器级真实 E2E 已闭环
|
||||
|
||||
**当前活跃阻塞：**
|
||||
- 无新的功能性阻塞；剩余工作主要是提交边界整理与文档/工作树卫生收口
|
||||
- 无新的功能性阻塞；review 报告已完成真相校准，剩余工作以维护性尾项（如 raw SQL / 前端状态收敛 / 类型真相）和提交边界整理为主
|
||||
|
||||
### 当前可诚实复用的一句话状态
|
||||
|
||||
@@ -1206,6 +1206,25 @@
|
||||
- 前端 `window.alert/confirm/prompt/open` 保护链路已确认存在且有测试覆盖：
|
||||
- [`frontend/admin/src/app/bootstrap/installWindowGuards.ts`](/D:/project/frontend/admin/src/app/bootstrap/installWindowGuards.ts)
|
||||
|
||||
## 2026-05-28 review 后续修复补充
|
||||
|
||||
- 修复 `internal/api/middleware/ratelimit.go` 的真实运行时缺陷：
|
||||
- 旧实现按 endpoint 共享单一内存桶，导致同一路由上的所有用户共用限流额度，存在全局误伤。
|
||||
- 旧实现也缺少历史 client limiter 的空闲清理策略，长期运行下存在条目累积风险。
|
||||
- 新实现改为按 `endpoint + user_id/IP` 分桶，并在访问路径上按 TTL 清理空闲 limiter 条目。
|
||||
- 补齐 handler context 类型守卫：`SSOHandler`、`WebhookHandler` 不再直接做 `user_id.(int64)` / `username.(string)` 断言，异常 context 会稳定返回 `401` 而不是 panic。
|
||||
- 新增回归测试覆盖：
|
||||
- 不同 IP 的登录限流互不影响
|
||||
- 共享 IP 下不同 `user_id` 的 API 限流互不影响
|
||||
- 空闲 limiter 条目会被回收
|
||||
- `SSOHandler` / `WebhookHandler` 非法 context 类型返回 `401`
|
||||
- 本轮后端验证已执行通过：
|
||||
- `go test ./internal/api/middleware -count=1`
|
||||
- `go test ./internal/api/handler -count=1`
|
||||
- `go test ./... -count=1`
|
||||
- `go vet ./...`
|
||||
- `go build ./cmd/server`
|
||||
|
||||
## 当前运行时真实能力
|
||||
|
||||
- 密码登录：启用
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
|
||||
@@ -42,6 +43,21 @@ func generateSecureToken(length int) (string, error) {
|
||||
return hex.EncodeToString(bytes)[:length], nil
|
||||
}
|
||||
|
||||
func resolveAvatarUploadDir(baseDir string) (string, error) {
|
||||
if baseDir == "" {
|
||||
baseDir = "./uploads"
|
||||
}
|
||||
cleanRoot := filepath.Clean(baseDir)
|
||||
if !filepath.IsAbs(cleanRoot) {
|
||||
absRoot, err := filepath.Abs(cleanRoot)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("resolve upload root: %w", err)
|
||||
}
|
||||
cleanRoot = absRoot
|
||||
}
|
||||
return filepath.Join(cleanRoot, "avatars"), nil
|
||||
}
|
||||
|
||||
// UploadAvatar 上传用户头像
|
||||
// @Summary 上传用户头像
|
||||
// @Description 上传并更新用户头像（仅本人或管理员）
|
||||
@@ -92,7 +108,7 @@ func (h *AvatarHandler) UploadAvatar(c *gin.Context) {
|
||||
}
|
||||
|
||||
// Validate file type
|
||||
ext := filepath.Ext(file.Filename)
|
||||
ext := strings.ToLower(filepath.Ext(file.Filename))
|
||||
allowedExts := map[string]bool{".jpg": true, ".jpeg": true, ".png": true, ".gif": true, ".webp": true}
|
||||
if !allowedExts[ext] {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"code": 400, "message": "invalid file type, allowed: jpg, jpeg, png, gif, webp"})
|
||||
@@ -139,7 +155,11 @@ func (h *AvatarHandler) UploadAvatar(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
avatarFilename := fmt.Sprintf("avatar_%d_%s%s", userID, token, ext)
|
||||
uploadDir := "./uploads/avatars"
|
||||
uploadDir, err := resolveAvatarUploadDir("")
|
||||
if err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"code": 500, "message": "failed to resolve upload directory"})
|
||||
return
|
||||
}
|
||||
|
||||
// Create upload directory if not exists
|
||||
if err := os.MkdirAll(uploadDir, 0o755); err != nil {
|
||||
|
||||
33
internal/api/handler/avatar_handler_path_test.go
Normal file
33
internal/api/handler/avatar_handler_path_test.go
Normal file
@@ -0,0 +1,33 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestResolveAvatarUploadDir_DefaultRootBecomesAbsolute(t *testing.T) {
|
||||
dir, err := resolveAvatarUploadDir("")
|
||||
if err != nil {
|
||||
t.Fatalf("resolveAvatarUploadDir() error = %v", err)
|
||||
}
|
||||
if !filepath.IsAbs(dir) {
|
||||
t.Fatalf("resolveAvatarUploadDir() = %q, want absolute path", dir)
|
||||
}
|
||||
if !strings.HasSuffix(filepath.ToSlash(dir), "/uploads/avatars") {
|
||||
t.Fatalf("resolveAvatarUploadDir() = %q, want suffix /uploads/avatars", dir)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveAvatarUploadDir_CustomRootPreserved(t *testing.T) {
|
||||
dir, err := resolveAvatarUploadDir("testdata/uploads-root")
|
||||
if err != nil {
|
||||
t.Fatalf("resolveAvatarUploadDir() error = %v", err)
|
||||
}
|
||||
if !filepath.IsAbs(dir) {
|
||||
t.Fatalf("resolveAvatarUploadDir() = %q, want absolute path", dir)
|
||||
}
|
||||
if !strings.HasSuffix(filepath.ToSlash(dir), "/testdata/uploads-root/avatars") {
|
||||
t.Fatalf("resolveAvatarUploadDir() = %q, want custom root suffix", dir)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user