Conversation
- Remove password reset token from API response to prevent token leak - Replace wildcard CORS with explicit origin allowlist from config - Strengthen AdminRequired middleware with DB-backed role verification - Add token revocation via in-memory blacklist and /logout endpoint - Remove cookie-based auth to prevent CSRF attacks (Bearer-only) - Increase minimum password length from 6 to 8 characters - Add CORS_ORIGINS and DB_LOG_LEVEL config fields
- Add per-IP sliding window rate limiter middleware - Auth endpoints: 10 req/min, AI endpoints: 20 req/min, general: 120 req/min - Auto-cleanup of expired entries every 5 minutes
- Add io.LimitReader on OpenAI (1MB/10MB) and Firecrawl (1MB) responses - Remove raw response body from log output to prevent data exposure - Add magic byte content-type validation on photo and avatar uploads - Add SSRF prevention with URL validation and private IP blocking - Fix path traversal in photo deletion with filepath.Clean - Add guest chat session eviction to prevent memory leak (max 1000) - Use configurable DB log level (default: warn instead of info) - Pin admin panel CDN versions (Tailwind 3.4.17, Alpine.js 3.14.8)
- Gate AI auto-reply behind @小石光 mention check to prevent spam - Clean up orphaned comment likes on question and answer deletion - Fix wall position collision by using lowest free slot (0-8) - Replace blocking alert() with inline toast in CommunityPanel - Fix HTTP 204 response sending unnecessary JSON body - Fix unreliable postgresql@* wildcard in dev-setup.sh
…xposure Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add HTTP client timeouts (60s openai, 30s firecrawl) to prevent goroutine hangs - Add security headers middleware (X-Frame-Options, X-Content-Type-Options, etc.) - Set trusted proxies to nil so rate limiter uses RemoteAddr, not X-Forwarded-For - Cap token blacklist at 100k entries with emergency cleanup to prevent OOM - Add double-check against ".." in photo deletion path to prevent traversal - Remove response body from firecrawl error message to prevent data leak - Cap echo handler pagination limit to 100 - Add max length constraints to question/answer/comment content DTOs - Fully mask DATABASE_URL in admin config endpoint
Backend: - Login/refresh endpoints now set refresh token as httpOnly cookie (Secure, SameSite=Lax, Path=/api/v1/auth) - Refresh endpoint reads token from cookie first, falls back to JSON body - Logout clears the refresh cookie - New AccessTokenResponse DTO (only access_token in response body) - AuthHandler now accepts config for cookie settings Frontend: - Access token stored in memory only (Vue ref + module variable) - Removed all localStorage/sessionStorage token management - Removed X-Access-Token redundant header - apiClient uses withCredentials:true to send cookies - Auth store syncs in-memory token via watch() - Session restored on page load via /auth/refresh (cookie auto-sent) Security improvement: XSS can no longer steal refresh tokens since they are invisible to JavaScript (httpOnly). Access tokens are short-lived and memory-only. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Resolves Chrome DevTools DOM warning about missing autocomplete attribute on the registration nickname input field.
- Add background scheduler that deletes off-wall photos older than 30 days, including disk files (6h interval, batch processing) - Add admin photo API: GET /admin/photos (paginated, filtered), DELETE /admin/photos/:id (with disk file removal) - Add photo stats (total_photos, wall_photos) to admin dashboard - Add "照片管理" tab to admin HTML panel with filters, pagination, delete confirmation modal, and "返回主站" navigation link - Adapt AI image generation protagonist gender based on user role: dad (守护者) defaults to male, mom (溯源者) defaults to female
- Add /admin proxy rule in Vite dev server config - Add admin panel entry button in CarPage settings (visible to admins only) - Links navigate in same tab instead of opening new window
- Allow editing memoir title and content after generation - Add regenerate image button to retry AI photo generation - Clean up previous photo on regeneration to avoid orphaned files
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR is a comprehensive security hardening and feature enhancement update for the MomShell application. It refactors authentication to use httpOnly cookies for refresh tokens (instead of localStorage), adds rate limiting, security headers, token blacklisting for logout, SSRF protection, CORS restrictions, input validation improvements, an admin photo management panel, a background photo cleanup scheduler, and various frontend/backend cleanups including removing the camera preview overlay and reducing debug logging.
Changes:
- Auth security overhaul: Refresh tokens moved to httpOnly cookies, access tokens kept in-memory only, token blacklisting for logout, cookie-based auth removed from middleware to prevent CSRF, minimum password length raised to 8.
- Backend hardening: Rate limiting (3 tiers), security headers, SSRF validation for image downloads, response body size limits, CORS origin whitelist (replacing
AllowAllOrigins), input length limits on content fields, guest session eviction cap. - Admin & cleanup features: Photo management in admin panel (list, delete, dashboard stats), background photo cleanup scheduler, admin middleware now verifies admin role via DB lookup.
Reviewed changes
Copilot reviewed 43 out of 44 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
backend/internal/handler/auth.go |
httpOnly cookie refresh token, logout endpoint, SameSite settings |
backend/internal/middleware/auth.go |
AdminChecker type, cookie-auth removal, token blacklist check |
backend/internal/middleware/cors.go |
Configurable CORS origins instead of allow-all |
backend/internal/middleware/ratelimit.go |
New per-IP fixed-window rate limiter |
backend/internal/middleware/security.go |
New security headers middleware |
backend/internal/middleware/tokenblacklist.go |
New in-memory token blacklist with cleanup |
backend/internal/handler/admin.go |
Admin photo management endpoints, IsAdmin checker |
backend/internal/handler/photo.go |
File content validation via magic bytes, NoContent fix |
backend/internal/handler/user.go |
File content validation via magic bytes |
backend/internal/handler/question.go |
AI reply only on @mention |
backend/internal/handler/echo.go |
Limit cap on memoir queries |
backend/internal/service/photo.go |
SSRF validation, image prompt builder, path traversal protection, user role lookup |
backend/internal/service/admin.go |
Photo listing/deletion, dashboard photo stats |
backend/internal/service/auth.go |
GetJWTSecret for logout |
backend/internal/service/chat.go |
Guest session eviction cap |
backend/internal/service/community.go |
Delete comment likes before deleting comments |
backend/internal/service/community_ai.go |
IsMentioned method |
backend/internal/scheduler/photo_cleanup.go |
New background photo cleanup scheduler |
backend/internal/repository/photo.go |
Admin queries, expired photo queries |
backend/internal/repository/admin.go |
Photo count stats |
backend/internal/router/router.go |
Rate limiters, admin middleware, new admin/auth routes |
backend/internal/config/config.go |
CORS_ORIGINS, DB_LOG_LEVEL config |
backend/internal/database/database.go |
Configurable DB log level |
backend/internal/dto/*.go |
Input length limits, password min=8, AccessTokenResponse, admin photo DTOs |
backend/internal/admin/admin.html |
Photo management UI, pinned CDN versions, back links |
backend/pkg/openai/client.go |
Response size limits, HTTP timeout, reduced debug logging |
backend/pkg/firecrawl/client.go |
Response size limit, HTTP timeout |
backend/cmd/server/main.go |
Wire up new dependencies, scheduler, security middleware |
frontend/src/lib/auth.ts |
httpOnly cookie auth, AccessTokenResponse, apiLogout |
frontend/src/lib/apiClient.ts |
In-memory token, withCredentials, interceptor refresh |
frontend/src/stores/auth.ts |
Cookie-based refresh, watch token sync, logout API |
frontend/src/components/overlay/MemoryPanel.vue |
Edit/regenerate functionality |
frontend/src/components/overlay/CommunityPanel.vue |
Toast errors instead of alert() |
frontend/src/components/overlay/CarPage.vue |
Photo URLs from allPhotos, wall slot allocation, admin link |
frontend/src/components/overlay/AuthPanel.vue |
Autocomplete attribute |
frontend/src/components/react/PearlShell.tsx |
Camera preview removed (hidden video) |
frontend/src/constants/sprites.ts |
Sprite position adjustments |
frontend/vite.config.ts |
/admin proxy, quote style |
frontend/package-lock.json |
peer dependency markers |
scripts/dev-setup.sh |
PostgreSQL versioned service start fix |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if photo.ImageURL != "" { | ||
| localPath := filepath.Clean("." + photo.ImageURL) | ||
| if strings.HasPrefix(localPath, "uploads"+string(filepath.Separator)) && | ||
| !strings.Contains(localPath, "..") { | ||
| _ = os.Remove(localPath) | ||
| } |
There was a problem hiding this comment.
The file deletion logic (filepath.Clean + prefix check + .. check + os.Remove) is duplicated in three places: service/photo.go:190-195, service/admin.go:276-280, and scheduler/photo_cleanup.go:69-72. Consider extracting this into a shared helper function (like the removePhotoFile function already defined in photo_cleanup.go) that can be reused across all three locations.
| func (h *AuthHandler) setRefreshCookie(c *gin.Context, refreshToken string, maxAge int) { | ||
| secure := !strings.Contains(h.cfg.CORSOrigins, "localhost") | ||
| c.SetCookie(refreshTokenCookie, refreshToken, maxAge, refreshCookiePath, "", secure, true) | ||
| c.SetSameSite(http.SameSiteLaxMode) | ||
| } |
There was a problem hiding this comment.
Bug: c.SetSameSite() must be called before c.SetCookie() in Gin. Gin's SetSameSite sets an internal field on the context that SetCookie reads when constructing the http.Cookie. Since SetSameSite is called after SetCookie here, the SameSite attribute will not be applied to the refresh token cookie, leaving it with the browser's default behavior (which may not be Lax in all contexts). Swap the order so SetSameSite comes first.
| func (h *AuthHandler) clearRefreshCookie(c *gin.Context) { | ||
| secure := !strings.Contains(h.cfg.CORSOrigins, "localhost") | ||
| c.SetCookie(refreshTokenCookie, "", -1, refreshCookiePath, "", secure, true) | ||
| } |
There was a problem hiding this comment.
clearRefreshCookie does not set SameSite, which means the clearing cookie may not match the attributes of the cookie that was originally set (with SameSiteLaxMode). Browsers may not clear the cookie if the attributes don't match. Add c.SetSameSite(http.SameSiteLaxMode) before c.SetCookie here as well, for consistency with setRefreshCookie.
| expireAge = 365 * 24 * time.Hour | ||
| batchSize = 100 | ||
| ) | ||
|
|
||
| // StartPhotoCleanup launches a background goroutine that periodically deletes | ||
| // photos with is_on_wall=false older than 30 days, including their disk files. |
There was a problem hiding this comment.
The comment on line 20 says "older than 30 days" but the expireAge constant on line 15 is set to 365 * 24 * time.Hour (365 days). Either the comment or the constant is wrong — please update one to match the intended behavior.
| c.Header("X-Content-Type-Options", "nosniff") | ||
| c.Header("X-XSS-Protection", "1; mode=block") | ||
| c.Header("Referrer-Policy", "strict-origin-when-cross-origin") | ||
| c.Header("Permissions-Policy", "camera=(), microphone=(), geolocation=()") |
There was a problem hiding this comment.
The Permissions-Policy: camera=() header completely disables camera access for the page, but the PearlShell component (frontend/src/components/react/PearlShell.tsx) actively uses the camera via @mediapipe/camera_utils for hand tracking. This header will break the hand gesture recognition feature. You should change the camera policy to allow same-origin access, e.g., camera=(self) instead of camera=().
| // Refresh token is sent automatically via httpOnly cookie | ||
| const resp = await apiRefresh(); | ||
| currentAccessToken = resp.access_token; |
There was a problem hiding this comment.
When the axios interceptor auto-refreshes the token (line 82), it only updates the module-level currentAccessToken but does not update the Pinia store's accessToken ref. This means the store's isAuthenticated computed property and accessToken.value become stale. More concretely, when logout() is called later, it will send the old (potentially expired) access token to the blacklist endpoint instead of the current one. Consider exposing a callback or event that the store can listen to, so the store stays in sync when the interceptor refreshes the token.
| func (h *AdminHandler) ListPhotos(c *gin.Context) { | ||
| if _, ok := h.requireAdmin(c); !ok { | ||
| return | ||
| } |
There was a problem hiding this comment.
The AdminRequired middleware (used in the router) now performs a database lookup via isAdmin(userID) to verify admin status. However, every admin handler method (e.g., ListPhotos, DeletePhoto) also calls h.requireAdmin(c) which performs a second identical database lookup. This results in two DB queries per admin API request to check the same thing. Since the middleware already guarantees the user is an admin, you can remove the requireAdmin calls from the handler methods that are behind the AdminRequired middleware.
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