security: fix rate-limit bypass, payment amount check, stats leak, owner-dir collision, video panic guard

- clientIP: stop trusting client-supplied X-Forwarded-For; use gin ClientIP
  with trusted proxies limited to private docker networks
- payment: verify notify money matches order amount; remove hardcoded
  gateway default, fail loudly when api_base is unset
- stats: public endpoint now serves aggregates only; recent files (owner
  names) moved to admin-only /stats/recent
- auth: reject signups whose username/email-local collides with another
  user's media directory namespace
- v1: recover panics in background video jobs, refund + mark failed
This commit is contained in:
dokploy-bot
2026-09-19 22:13:08 +08:00
parent 34f808e7e3
commit 42208c0753
8 changed files with 107 additions and 12 deletions
@@ -187,6 +187,17 @@ func (h *AdminReadHandler) Stats(c *gin.Context) {
c.JSON(http.StatusOK, stats) c.JSON(http.StatusOK, stats)
} }
// StatsRecent serves the recent-files list (owner names included) — only
// mounted on the admin-session route group.
func (h *AdminReadHandler) StatsRecent(c *gin.Context) {
data, err := h.admin.StatsRecent(c.Request.Context())
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"detail": "failed to load recent files"})
return
}
c.JSON(http.StatusOK, data)
}
func (h *AdminReadHandler) Dashboard(c *gin.Context) { func (h *AdminReadHandler) Dashboard(c *gin.Context) {
data, err := h.admin.Dashboard(c.Request.Context()) data, err := h.admin.Dashboard(c.Request.Context())
if err != nil { if err != nil {
+6 -7
View File
@@ -307,14 +307,13 @@ func writeLoginLocked(c *gin.Context, err error) bool {
return false return false
} }
// clientIP resolves the caller for rate limiting. It must NOT read
// client-supplied X-Forwarded-For / X-Real-Ip directly: the first XFF value
// is attacker-controlled and rotating it resets every throttle bucket.
// gin's ClientIP walks the header chain right-to-left and only honors
// forwarding headers from the trusted proxies configured in the router
// (the in-stack traefik/nginx on private docker networks).
func clientIP(c *gin.Context) string { func clientIP(c *gin.Context) string {
if fwd := strings.TrimSpace(c.GetHeader("X-Forwarded-For")); fwd != "" {
parts := strings.Split(fwd, ",")
return strings.TrimSpace(parts[0])
}
if real := strings.TrimSpace(c.GetHeader("X-Real-Ip")); real != "" {
return real
}
return c.ClientIP() return c.ClientIP()
} }
+14
View File
@@ -37,6 +37,16 @@ func New(cfg *config.Config, auth *service.AuthService, handlers Handlers) *gin.
} }
engine := gin.New() engine := gin.New()
// Only the in-stack reverse proxies (traefik → frontend nginx, all on
// private docker networks) may supply forwarding headers; ClientIP then
// resolves the real client instead of a spoofable X-Forwarded-For.
_ = engine.SetTrustedProxies([]string{
"10.0.0.0/8",
"172.16.0.0/12",
"192.168.0.0/16",
"127.0.0.0/8",
"::1/128",
})
engine.Use(gin.Recovery()) engine.Use(gin.Recovery())
engine.Use(middleware.RequestID()) engine.Use(middleware.RequestID())
engine.Use(cors.New(cors.Config{ engine.Use(cors.New(cors.Config{
@@ -64,6 +74,8 @@ func New(cfg *config.Config, auth *service.AuthService, handlers Handlers) *gin.
publicAdmin.GET("/site", handlers.Site.Public) publicAdmin.GET("/site", handlers.Site.Public)
publicAdmin.GET("/showcase", handlers.Showcase.List) publicAdmin.GET("/showcase", handlers.Showcase.List)
publicAdmin.GET("/managed-models", handlers.AdminRead.Models) publicAdmin.GET("/managed-models", handlers.AdminRead.Models)
// Aggregates only (counts/sizes/latency) — the recent-files list with
// owner names is admin-only, see /stats/recent below.
publicAdmin.GET("/stats", handlers.AdminRead.Stats) publicAdmin.GET("/stats", handlers.AdminRead.Stats)
publicAdmin.GET("/video-presets", handlers.UserGen.VideoPresets) publicAdmin.GET("/video-presets", handlers.UserGen.VideoPresets)
publicAdmin.GET("/catalog", handlers.UserGen.Catalog) publicAdmin.GET("/catalog", handlers.UserGen.Catalog)
@@ -106,6 +118,8 @@ func New(cfg *config.Config, auth *service.AuthService, handlers Handlers) *gin.
authed.Use(middleware.RequireAdminSession(auth, cfg)) authed.Use(middleware.RequireAdminSession(auth, cfg))
{ {
authed.GET("/dashboard", handlers.AdminRead.Dashboard) authed.GET("/dashboard", handlers.AdminRead.Dashboard)
// Recent generated files include owner-directory names — admin only.
authed.GET("/stats/recent", handlers.AdminRead.StatsRecent)
authed.GET("/users", handlers.AdminRead.Users) authed.GET("/users", handlers.AdminRead.Users)
authed.GET("/invites", handlers.AdminRead.Invites) authed.GET("/invites", handlers.AdminRead.Invites)
authed.POST("/users", handlers.AdminWrite.CreateUser) authed.POST("/users", handlers.AdminWrite.CreateUser)
+12
View File
@@ -129,6 +129,18 @@ func (r *UserRepository) ExistsName(ctx context.Context, name, excludeUserID str
return count > 0, nil return count > 0, nil
} }
// ListOwnerCandidates returns every user's name+email. Consumed by the
// registration-time owner-directory collision check (media paths derive
// from sanitized names / email local-parts, so those namespaces must stay
// disjoint across users). Self-hosted user counts keep this scan cheap.
func (r *UserRepository) ListOwnerCandidates(ctx context.Context) ([]model.User, error) {
var users []model.User
if err := r.db.WithContext(ctx).Select("name", "email").Find(&users).Error; err != nil {
return nil, err
}
return users, nil
}
func (r *UserRepository) GetByAPIKeyHash(ctx context.Context, keyHash string) (*model.User, error) { func (r *UserRepository) GetByAPIKeyHash(ctx context.Context, keyHash string) (*model.User, error) {
var apiKey model.APIKey var apiKey model.APIKey
if err := r.db.WithContext(ctx).First(&apiKey, "key_hash = ?", keyHash).Error; err != nil { if err := r.db.WithContext(ctx).First(&apiKey, "key_hash = ?", keyHash).Error; err != nil {
+12 -2
View File
@@ -221,7 +221,6 @@ func (s *AdminReadService) Stats(ctx context.Context) (map[string]any, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
recentFiles, _ := s.RecentImages(ctx, 24)
files, _, _ := s.scanGeneratedFiles(ctx) files, _, _ := s.scanGeneratedFiles(ctx)
// Exclude homepage showcase media so 已生成作品 matches the 作品管理 total. // Exclude homepage showcase media so 已生成作品 matches the 作品管理 total.
pinned := map[string]struct{}{} pinned := map[string]struct{}{}
@@ -239,15 +238,26 @@ func (s *AdminReadService) Stats(ctx context.Context) (map[string]any, error) {
count++ count++
size += f.Size size += f.Size
} }
// Aggregate-only: `recent` (owner dirs + filenames) is served via
// StatsRecent behind an admin session — it must not leak publicly.
return map[string]any{ return map[string]any{
"generated_count": count, "generated_count": count,
"generated_size_bytes": size, "generated_size_bytes": size,
"recent": recentFiles,
"avg_elapsed_ms": stats.AvgElapsedMS, "avg_elapsed_ms": stats.AvgElapsedMS,
"avg_elapsed_ms_24h": stats.AvgElapsedMS24, "avg_elapsed_ms_24h": stats.AvgElapsedMS24,
}, nil }, nil
} }
// StatsRecent backs the admin image picker; the route lives in the
// RequireAdminSession group because entries carry per-user owner names.
func (s *AdminReadService) StatsRecent(ctx context.Context) (map[string]any, error) {
recentFiles, err := s.RecentImages(ctx, 24)
if err != nil {
return nil, err
}
return map[string]any{"recent": recentFiles}, nil
}
// Dashboard assembles the admin overview's analytics entirely server-side // Dashboard assembles the admin overview's analytics entirely server-side
// (event windows, hourly trend, top models/failures/spenders) plus CDK / invite // (event windows, hourly trend, top models/failures/spenders) plus CDK / invite
// / checkin summaries. This replaces the old client-side math over the last 200 // / checkin summaries. This replaces the old client-side math over the last 200
+31
View File
@@ -100,6 +100,34 @@ func ownsImageDir(user *model.User, owner string) bool {
return false return false
} }
// checkOwnerDirCollision rejects a signup whose sanitized username or email
// local-part collides with any existing user's directory candidates. Media
// authorization (ownsImageDir) matches on those candidates, so without this
// check registering as <victim-name>@any-domain would grant access to the
// victim's /images/<name>/ directory.
func (s *AuthService) checkOwnerDirCollision(ctx context.Context, username, email string) error {
newName := sanitizeOwnerName(username)
newEmail := sanitizeOwnerName(strings.Split(email, "@")[0])
if newName == "" && newEmail == "" {
return nil
}
users, err := s.users.ListOwnerCandidates(ctx)
if err != nil {
return err
}
for _, u := range users {
oldName := sanitizeOwnerName(u.Name)
oldEmail := sanitizeOwnerName(strings.Split(u.Email, "@")[0])
if newName != "" && (oldName == newName || oldEmail == newName) {
return errors.New("用户名与已有用户冲突,请更换")
}
if newEmail != "" && newName != newEmail && (oldName == newEmail || oldEmail == newEmail) {
return errors.New("邮箱前缀与已有用户冲突,请更换")
}
}
return nil
}
func (s *AuthService) CurrentUserFromBearer(ctx context.Context, authHeader string) (*model.User, *SessionPayload, error) { func (s *AuthService) CurrentUserFromBearer(ctx context.Context, authHeader string) (*model.User, *SessionPayload, error) {
token := ParseBearer(authHeader) token := ParseBearer(authHeader)
return s.currentUserFromToken(ctx, token) return s.currentUserFromToken(ctx, token)
@@ -272,6 +300,9 @@ func (s *AuthService) Register(ctx context.Context, email, username, password, i
if exists { if exists {
return nil, "", nil, errors.New("用户名已存在") return nil, "", nil, errors.New("用户名已存在")
} }
if err := s.checkOwnerDirCollision(ctx, normalizedUsername, normalizedEmail); err != nil {
return nil, "", nil, err
}
passwordHash, err := HashPassword(password) passwordHash, err := HashPassword(password)
if err != nil { if err != nil {
+10 -3
View File
@@ -55,9 +55,6 @@ func (s *PaymentService) get(ctx context.Context, key string) string {
func (s *PaymentService) Settings(ctx context.Context) PaySettings { func (s *PaymentService) Settings(ctx context.Context) PaySettings {
api := strings.TrimRight(strings.TrimSpace(s.get(ctx, "pay.api_base")), "/") api := strings.TrimRight(strings.TrimSpace(s.get(ctx, "pay.api_base")), "/")
if api == "" {
api = "https://www.ezfpy.cn"
}
ratio, _ := strconv.Atoi(s.get(ctx, "pay.points_ratio")) ratio, _ := strconv.Atoi(s.get(ctx, "pay.points_ratio"))
if ratio <= 0 { if ratio <= 0 {
ratio = 100 ratio = 100
@@ -147,6 +144,11 @@ func (s *PaymentService) CreateOrder(ctx context.Context, user *model.User, amou
if cfg.PID == "" || cfg.Key == "" { if cfg.PID == "" || cfg.Key == "" {
return nil, ErrPayNotConfig return nil, ErrPayNotConfig
} }
// Never guess a gateway: an empty api_base must fail loudly instead of
// silently routing signed payment traffic to some hardcoded third party.
if cfg.APIBase == "" {
return nil, ErrPayNotConfig
}
if !cfg.allows(method) { if !cfg.allows(method) {
return nil, ErrPayMethod return nil, ErrPayMethod
} }
@@ -227,6 +229,11 @@ func (s *PaymentService) HandleNotify(ctx context.Context, params map[string]str
if err != nil || order == nil { if err != nil || order == nil {
return false, ErrOrderNotFound return false, ErrOrderNotFound
} }
// The gateway signs whatever the cashier actually collected — verify the
// paid amount matches the order, or an underpaid notify credits full points.
if money, perr := strconv.ParseFloat(strings.TrimSpace(params["money"]), 64); perr != nil || math.Abs(money-order.Amount) > 0.01 {
return false, errors.New("notify amount mismatch")
}
credited, err := s.orders.MarkPaid(ctx, order.ID, params["trade_no"], time.Now()) credited, err := s.orders.MarkPaid(ctx, order.ID, params["trade_no"], time.Now())
if err != nil { if err != nil {
return false, err return false, err
+11
View File
@@ -10,6 +10,7 @@ import (
"log" "log"
"net/http" "net/http"
"path/filepath" "path/filepath"
"runtime/debug"
"sort" "sort"
"strings" "strings"
"sync" "sync"
@@ -969,6 +970,16 @@ func (s *V1Service) runVideoJob(ctx context.Context, principal *APIPrincipal, in
defer s.inflight.Done(eventID) defer s.inflight.Done(eventID)
startedAt := time.Now() startedAt := time.Now()
// A panic in provider fan-out must fail the event (and refund), not
// take down the whole API process with it.
defer func() {
if r := recover(); r != nil {
log.Printf("[v1] panic in runVideoJob event=%s model=%s: %v\n%s", eventID, modelItem.Name, r, debug.Stack())
_ = s.refundIfNeeded(ctx, principal, eventID, price)
_ = s.events.UpdateStatus(ctx, eventID, "failed", fmt.Sprintf("internal error: %v", r), 0)
}
}()
// No-store: capture only the UPSTREAM video URL. /content streams it on demand // No-store: capture only the UPSTREAM video URL. /content streams it on demand
// (grok URLs are auth-gated → fetched with the generating account's token). // (grok URLs are auth-gated → fetched with the generating account's token).
var videoURL string var videoURL string