Conversation
There was a problem hiding this comment.
Pull request overview
This PR is a broad “dev” hardening and feature update spanning auth (httpOnly refresh cookie + in-memory access token), API security controls (rate limiting, token revocation, SSRF mitigation), admin photo management, and multiple frontend UX improvements, with documentation updates to match.
Changes:
- Migrate refresh tokens to httpOnly cookies and move access tokens to in-memory client state with refresh synchronization.
- Add backend security/ops improvements: fixed-window rate limiting, token blacklist on logout, tighter CORS, security headers, and safer external image downloading.
- Expand admin + photo lifecycle features (admin photo list/delete, scheduled cleanup) and refine several frontend panels/UI behaviors.
Reviewed changes
Copilot reviewed 57 out of 58 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/dev-setup.sh | Improve Homebrew PostgreSQL service start logic on macOS. |
| frontend/vite.config.ts | Add dev proxies for /uploads and /admin; formatting updates. |
| frontend/src/stores/auth.ts | Switch auth flow to cookie refresh + in-memory token; add logout call. |
| frontend/src/lib/auth.ts | Adjust auth API shapes; always include credentials; add logout API. |
| frontend/src/lib/apiClient.ts | In-memory access token + refresh callback; cookie-based refresh handling. |
| frontend/src/constants/sprites.ts | Adjust sprite positions. |
| frontend/src/components/react/PearlShell.tsx | Hide camera preview; keep hidden video element for tracking. |
| frontend/src/components/overlay/MemoryPanel.vue | Add memoir text editing + image regenerate + delete old generated photo. |
| frontend/src/components/overlay/CommunityPanel.vue | Replace alert with in-panel toast error UI. |
| frontend/src/components/overlay/ChatPanel.vue | Improve session ID generation fallback using Web Crypto. |
| frontend/src/components/overlay/CarPage.vue | Use all photos for PearlShell; admin link; wall position collision avoidance; layout tweaks. |
| frontend/src/components/overlay/AuthPanel.vue | Add autocomplete for nickname field. |
| frontend/package-lock.json | Lockfile metadata changes (peer flags). |
| docs/getting-started.md | Update Go prerequisite version. |
| docs/features.md | Expand/clarify feature list across modules. |
| docs/development.md | Update Go prerequisite version. |
| docs/configuration.md | Document IMAGE_MODEL and Firecrawl web search env var. |
| docs/architecture.md | Update tech stack and structure notes; auth/rate limit mentions. |
| docs/README.md | Add links to other repository documents. |
| backend/pkg/openai/client.go | Add timeouts + response size limits; sanitize error logging. |
| backend/pkg/firecrawl/client.go | Add timeout + response size limit; sanitize error logging. |
| backend/internal/service/photo.go | Photo limits, prompt styling, local deletion via fileutil, SSRF validation + download limits. |
| backend/internal/service/community_ai.go | Add IsMentioned helper. |
| backend/internal/service/community.go | Ensure comment-like cleanup before deleting comments/answers. |
| backend/internal/service/chat.go | Cap guest session memory with eviction. |
| backend/internal/service/auth.go | Expose JWT secret getter for logout blacklist parsing. |
| backend/internal/service/admin.go | Add photo stats + admin photo list/delete; hide DB URL fully. |
| backend/internal/scheduler/photo_cleanup.go | Add background cleanup of old off-wall photos. |
| backend/internal/router/router.go | Add rate limiting per route group; add logout route; wire AdminChecker; new admin photo routes. |
| backend/internal/repository/sort.go | Shared allowlist for sort order direction. |
| backend/internal/repository/question.go | Sanitize question sort columns to prevent ORDER BY injection. |
| backend/internal/repository/photo.go | Add expired-off-wall query + admin delete + admin list pagination. |
| backend/internal/repository/answer.go | Sanitize answer sort columns to prevent ORDER BY injection. |
| backend/internal/repository/admin.go | Add photo count stats. |
| backend/internal/middleware/tokenblacklist.go | Add in-memory token blacklist with periodic cleanup. |
| backend/internal/middleware/security.go | Add standard security headers middleware. |
| backend/internal/middleware/ratelimit.go | Add fixed-window per-IP rate limiter middleware. |
| backend/internal/middleware/cors.go | Make CORS origins configurable; allow credentials. |
| backend/internal/middleware/auth.go | Add AdminChecker-based admin enforcement; remove cookie-based access-token extraction; check blacklist. |
| backend/internal/handler/user.go | Validate avatar upload content via magic bytes. |
| backend/internal/handler/question.go | Trigger AI auto-reply only when @小石光 is mentioned. |
| backend/internal/handler/photo.go | Validate photo upload content via magic bytes; fix DELETE response to 204 status-only. |
| backend/internal/handler/echo.go | Cap memoir list limit query param at 100. |
| backend/internal/handler/auth.go | Set/rotate refresh cookie; return access token only; add logout that blacklists access token. |
| backend/internal/handler/admin.go | Move admin check into middleware via AdminChecker; add admin photo endpoints. |
| backend/internal/fileutil/photo.go | Centralize safe deletion of uploaded files. |
| backend/internal/dto/question.go | Add max content length validation. |
| backend/internal/dto/comment.go | Add max content length validation. |
| backend/internal/dto/auth.go | Increase password min length; add access-token-only response DTO. |
| backend/internal/dto/answer.go | Add max content length validation. |
| backend/internal/dto/admin.go | Add photo stats + admin photo list DTOs. |
| backend/internal/database/database.go | Make GORM log level configurable. |
| backend/internal/config/config.go | Add CORS/log config; update model defaults. |
| backend/internal/admin/admin.html | Add admin navigation + photo management UI; pin CDN versions; add “back to site”. |
| backend/cmd/server/main.go | Wire new services/middleware/scheduler; set trusted proxies to nil. |
| SECURITY.md | Update supported versions and security guidance for current stack. |
| README.md | Update Go badge and expand module list/structure. |
| CHANGELOG.md | Add v1 security/features changelog entries. |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
Comments suppressed due to low confidence (3)
backend/internal/service/photo.go:304
downloadFromURLvalidates the initial URL but then useshttp.DefaultClient.Do, which follows redirects. A malicious image URL can redirect to an internal/private address and bypass the SSRF checks. Use anhttp.Clientwith redirects disabled or validate every redirect hop (and ideally also enforce network-level restrictions in the transport).
backend/internal/handler/user.go:106- The magic-bytes check only verifies the upload is an allowed image type, but it doesn't verify it matches the declared
Content-Type. As written, a JPEG uploaded withContent-Type: image/pngwill be saved with a.pngextension. Consider (1) rejecting whendetectedType != contentType(with a small alias map), and/or (2) deriving the file extension fromdetectedTypeinstead of the header.
contentType := header.Header.Get("Content-Type")
if !allowedImageTypes[contentType] {
c.JSON(http.StatusBadRequest, gin.H{"error": "仅支持 JPG、PNG、GIF、WebP 格式"})
return
}
// Validate actual file content via magic bytes
buf := make([]byte, 512)
n, _ := file.Read(buf)
detectedType := http.DetectContentType(buf[:n])
if !allowedImageTypes[detectedType] {
c.JSON(http.StatusBadRequest, gin.H{"error": "文件内容与格式不匹配"})
return
}
if _, err := file.Seek(0, 0); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "上传失败"})
return
}
ext := ".jpg"
switch contentType {
case "image/png":
ext = ".png"
case "image/gif":
ext = ".gif"
case "image/webp":
ext = ".webp"
}
backend/internal/handler/photo.go:96
- Same issue as avatar upload: the magic-bytes check only verifies the file is an allowed image type, but doesn't ensure it matches the declared
Content-Type, and the extension is derived from the header. Consider rejecting mismatches and/or deriving the extension fromdetectedTypeto avoid saving content under the wrong file type.
contentType := header.Header.Get("Content-Type")
if !allowedPhotoTypes[contentType] {
c.JSON(http.StatusBadRequest, gin.H{"error": "仅支持 JPG、PNG、GIF、WebP 格式"})
return
}
// Validate actual file content via magic bytes
buf := make([]byte, 512)
n, _ := file.Read(buf)
detectedType := http.DetectContentType(buf[:n])
if !allowedPhotoTypes[detectedType] {
c.JSON(http.StatusBadRequest, gin.H{"error": "文件内容与格式不匹配"})
return
}
// Reset reader position for SaveUploadedFile
if _, err := file.Seek(0, 0); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "上传失败"})
return
}
ext := ".jpg"
switch contentType {
case "image/png":
ext = ".png"
case "image/gif":
ext = ".gif"
case "image/webp":
ext = ".webp"
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+14
to
+21
| - **httpOnly cookie authentication**: Migrated auth tokens from localStorage to httpOnly cookies, eliminating XSS token theft vectors | ||
| - **Fixed-window rate limiting**: Added per-endpoint rate limiting to all API routes to prevent abuse | ||
| - **Input validation hardening**: Strengthened validation across all user-facing endpoints | ||
| - **SQL injection prevention**: Added allowlist validation for ORDER BY clauses in repository layer (defense-in-depth for `question.go` and `answer.go`) | ||
| - **Insecure randomness fix**: Replaced `Math.random()` fallback with `crypto.getRandomValues()` for cryptographically secure session ID generation in `ChatPanel.vue` | ||
| - **Auth token extraction hardening**: Improved multi-source token extraction security (header, cookie) | ||
| - **OpenAI error sanitization**: Removed response body from OpenAI error messages to prevent data exposure | ||
|
|
There was a problem hiding this comment.
The changelog entry says "auth tokens" were migrated to httpOnly cookies and mentions token extraction via "header, cookie". In this PR, only the refresh token is cookie-based; access tokens are kept in-memory and sent via Authorization/X-Access-Token, and cookie-based access-token extraction was removed. Please adjust these bullet points so the changelog reflects the actual auth model.
Comment on lines
122
to
+126
| r := gin.New() | ||
| _ = r.SetTrustedProxies(nil) // Don't trust any proxy headers by default | ||
| r.Use(middleware.Recovery()) | ||
| r.Use(middleware.CORS()) | ||
| r.Use(middleware.SecurityHeaders()) | ||
| r.Use(middleware.CORS(cfg)) |
There was a problem hiding this comment.
r.SetTrustedProxies(nil) makes c.ClientIP() resolve to the reverse proxy's IP (e.g. 127.0.0.1 in the bundled Nginx container), which will cause the new per-IP rate limiting to throttle all users together in production. Configure trusted proxies instead (e.g. loopback / container network), ideally via config, so X-Forwarded-For can be used safely.
Comment on lines
+377
to
+382
| // Delete likes on comments before deleting the comments themselves | ||
| if comments, cErr := s.commentRepo.FindByAnswerID(aid); cErr == nil { | ||
| for _, c := range comments { | ||
| _ = s.interactionRepo.DeleteLikesByTarget("comment", c.ID) | ||
| } | ||
| } |
There was a problem hiding this comment.
This introduces an N+1 delete pattern (one DB delete per comment like) when deleting a question. Consider adding a bulk delete method (e.g. delete likes where target_type='comment' and target_id IN (...)) or a repository method that deletes likes for all comments under an answer in a single query to keep deletes fast on large threads.
Comment on lines
+543
to
+548
| // Delete likes on comments before deleting the comments themselves | ||
| if comments, cErr := s.commentRepo.FindByAnswerID(answerID); cErr == nil { | ||
| for _, c := range comments { | ||
| _ = s.interactionRepo.DeleteLikesByTarget("comment", c.ID) | ||
| } | ||
| } |
There was a problem hiding this comment.
This introduces an N+1 delete pattern (one DB delete per comment like) when deleting an answer. Consider a bulk delete in interactionRepo for comment likes by answer (e.g. delete where target_type='comment' and target_id IN (comment IDs for this answer)) to avoid many round-trips.
Comment on lines
+95
to
+98
| - JWT access tokens (30 min) + refresh tokens (7 days), stored in httpOnly cookies | ||
| - Tokens extracted from `Authorization: Bearer`, `X-Access-Token` header, or `access_token` cookie | ||
| - Admin role verified per-request in handler via `authService.GetUserByID` | ||
| - Fixed-window rate limiting on all API endpoints |
There was a problem hiding this comment.
The authentication bullets are out of date with the new middleware: access tokens are no longer extracted from an access_token cookie (cookie-based access-token auth was removed), and admin checking is now enforced by AdminRequired middleware via an AdminChecker (not in each handler). Please update these bullets to match the current behavior.
Comment on lines
+8
to
+13
| // TokenBlacklist is an in-memory blacklist for revoked JWT tokens. | ||
| var TokenBlacklist = &tokenBlacklistStore{ | ||
| tokens: make(map[string]time.Time), | ||
| } | ||
|
|
||
| const maxBlacklistSize = 100000 |
There was a problem hiding this comment.
This token blacklist is purely in-memory. In a multi-instance / horizontally scaled deployment, logging out on one instance won't revoke tokens validated by other instances, and a restart clears the blacklist. If this is intended for single-instance only, consider documenting the limitation; otherwise, back it with shared storage (e.g. Redis) or switch to short-lived access tokens + refresh rotation without blacklist.
Comment on lines
+144
to
+160
| // Evict oldest sessions if at capacity | ||
| if len(s.guestMemory) >= maxGuestSessions { | ||
| // Delete ~10% of sessions to avoid evicting on every new request | ||
| toDelete := maxGuestSessions / 10 | ||
| if toDelete < 1 { | ||
| toDelete = 1 | ||
| } | ||
| deleted := 0 | ||
| for k := range s.guestMemory { | ||
| delete(s.guestMemory, k) | ||
| delete(s.guestProfiles, k) | ||
| deleted++ | ||
| if deleted >= toDelete { | ||
| break | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The comment says "Evict oldest sessions" but the eviction loop deletes arbitrary map entries (Go map iteration order is random) and doesn't track recency/age. Either reword the comment to reflect random eviction, or implement true LRU/oldest eviction by tracking last-seen timestamps (and evict based on that).
Comment on lines
+148
to
+149
| // Generate an AI photo based on the memoir content | ||
| triggerImageGeneration(memoir.content || memoir.title || theme || '记忆贴纸') |
There was a problem hiding this comment.
generatePhoto is backed by a server-side prompt validation of max=500 chars. Passing memoir.content directly can easily exceed that limit and make image generation fail. Consider truncating/summarizing the prompt client-side (e.g. slice to 500 chars, or build a shorter prompt from title/theme) before calling triggerImageGeneration.
Comment on lines
68
to
+73
| function logout() { | ||
| clearTokens(); | ||
| // The /logout endpoint requires authentication, so only call the server | ||
| // when we have a token. The server clears the httpOnly refresh cookie. | ||
| if (accessToken.value) { | ||
| apiLogout(accessToken.value).catch(() => {}); | ||
| } |
There was a problem hiding this comment.
With refresh tokens now stored in an httpOnly cookie, a failed /logout request means the cookie won't be cleared and the user can be silently re-authenticated on the next refresh. Consider making logout async and awaiting apiLogout, handling failures explicitly (or adjust the backend logout to allow clearing the refresh cookie even when the access token is missing/expired).
Comment on lines
+291
to
+296
| function showPanelError(msg: string) { | ||
| panelError.value = msg | ||
| setTimeout(() => { | ||
| panelError.value = '' | ||
| }, 4000) | ||
| } |
There was a problem hiding this comment.
showPanelError schedules a setTimeout every time it's called, but doesn't clear any previous timer. If multiple errors occur within 4s, an earlier timer can clear a newer message prematurely. Track the timeout ID and clearTimeout before starting a new timer.
…d permissions policy - Add allowlist validation for ORDER BY clauses in question and answer repositories to prevent SQL injection (defense-in-depth) - Remove Math.random() fallback for session ID generation; use crypto.randomUUID() exclusively - Derive cookie Secure flag from request TLS / X-Forwarded-Proto instead of fragile CORS config string matching - Call SetSameSite before SetCookie so Gin applies SameSite=Lax attribute - Add SameSite to clearRefreshCookie for consistent cookie attributes - Change Permissions-Policy camera=() to camera=(self) to allow MediaPipe hand tracking in PearlShell component
- Add setOnTokenRefreshed callback so Axios refresh interceptor updates the Pinia store's accessToken (keeps isAuthenticated in sync) - Make apiLogout accessToken parameter optional so logout always calls the server to clear the httpOnly refresh cookie - Always call logout endpoint even when no access token is held in memory - Check response.ok in apiLogout to surface server-side failures
…undant admin checks - Extract duplicated photo file deletion logic from service/photo.go, service/admin.go, and scheduler/photo_cleanup.go into a shared fileutil.RemoveUploadedFile helper - Remove requireAdmin() from all admin handler methods since the router already applies middleware.AdminRequired, eliminating a redundant DB query per admin API request - Fix photo_cleanup comment to match actual 365-day expiration constant
- CHANGELOG: add comprehensive Unreleased section covering security hardening, photo gallery, memoir editing, whisper, tasks, community enhancements, and frontend features - README: fix Go version badge (1.25), add missing feature modules (Echo, Photo, Whisper, Tasks), update project structure - SECURITY: update supported versions (1.x.x), fix SQLite references to PostgreSQL, update module names and secret variable names - docs/features: add Echo/Memoir, Photo Gallery, Whisper, Tasks sections - docs/architecture: update Go version, add GSAP/Three.js/MediaPipe/ Firecrawl to tech stack, add scheduler and rate limiting - docs/configuration: add IMAGE_MODEL and FIRECRAWL_API_KEY variables - docs/development, getting-started: update Go version to 1.25+ - docs/README: add links to Changelog, Security, Code of Conduct
The previous allowlist approach reassigned the parameter variables, but CodeQL's static taint tracker still saw user-controlled data flowing into the query string. Refactor to use lookup maps that return fresh string literals from map values, and move the sanitization into dedicated functions (sanitizeQuestionSort, sanitizeAnswerSort) so the Order() call never references the original tainted parameters.
…implify path validation - Make isSecureRequest a method on AuthHandler, only trust X-Forwarded-Proto when cfg.TrustProxy is explicitly enabled - Add TrustProxy bool config field with getEnvBool helper - Remove redundant ".." check in RemoveUploadedFile since filepath.Clean already resolves traversal components - Move allowedSortOrders to dedicated sort.go for discoverability - Add TRUST_PROXY to .env.example with documentation
…ack, document callback - Change "logout failed" to "退出登录失败" for consistent i18n - Restore crypto.getRandomValues fallback for non-secure HTTP contexts - Add documentation comment about single-callback limitation in apiClient
Remove dynamic isSecureRequest logic and always pass Secure=true to SetCookie. Browsers treat localhost as a secure context so local dev on http://localhost still works. This eliminates CodeQL alert #93 (cookie-secure-not-set) and removes the now-unused TrustProxy config.
- Add typeof check for crypto.getRandomValues before calling it; add last-resort Math.random fallback for envs without Web Crypto - Restore token guard in logout() since /logout requires AuthRequired middleware — unauthenticated calls would just get 401
The actual ratelimit.go implementation uses a counter that resets after each window duration, which is fixed-window, not sliding-window. Also correct CHANGELOG entry for session ID generation to reflect the crypto.getRandomValues fallback.
koishi510
approved these changes
Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Summary
Change Type
Self-Check Checklist
Backend (Go):
go build ./...passesgo vet ./...passesgofmtproduces no diffFrontend (Vue):
npm run lintpassesnpm run typecheckpassesGeneral:
Test Steps