GameList/TASKS.md

TASKS - Review findings

• Critical - Unsigned auth cookie allows impersonation (Infrastructure/PlayerIdentityExtensions.cs, Endpoints/EndpointHelpers.cs, wwwroot/js/api.js): The player cookie stores a raw GUID and is trusted on every request; /api/suggestions/all exposes other players' IDs, so a user can copy an ID, set the cookie, and become that account (including admins). Move to ASP.NET Core cookie auth with signed/encrypted tickets or a server-side session table, and avoid exposing player IDs where not necessary.
• Critical - Stored XSS via suggestion content/links (wwwroot/js/ui.js, wwwroot/index.html, Endpoints/SuggestEndpoints.cs): Names, descriptions, and URLs are injected with innerHTML and unescaped links; gameUrl/youtubeUrl accept any scheme. A malicious suggestion can run script when viewed. Sanitize/escape on render (use textContent), validate URLs to http/https on the server, and consider HTML sanitization for descriptions.
• High - SSRF surface in screenshot validation (Endpoints/EndpointHelpers.IsReachableImageAsync, SuggestEndpoints.cs): The server performs HEAD/GET requests to arbitrary user-provided URLs with minimal limits, enabling internal port probing and latency spikes. Restrict to allowlisted hosts or disable remote fetch; at minimum block private IPs/localhost, disallow redirects, and enforce tight size/time limits.
• High - Phase recalculation writes on every poll (Endpoints/EndpointHelpers.GetPhase, wwwroot/app.js): GetPhase unconditionally calls SaveChangesAsync; the client polls /api/state every 4s, causing constant writes and SQLite WAL churn/locks even when nothing changes. Track a changed flag and persist only when state mutations occur; consider reducing poll frequency or using long polling/server push later.
• Medium - Admin key accepted via query string (Endpoints/EndpointHelpers.IsAdmin): Accepting key in the query leaks secrets via logs and referrers. Require the X-Admin-Key header (or authenticated admin account) only, and rate-limit/monitor attempts.
• Medium - Brittle base path detection on the client (wwwroot/js/api.js): The heuristic that picks the first path segment can double-prefix or break when the app is nested (e.g., /picknplay/). Prefer explicit <meta name="app-base"> or <base> and drop the autodetect fallback.
• Maintenance - Centralize auth/phase concerns (multiple endpoints): Each endpoint manually fetches the player, admin status, and phase gating, repeating logic and increasing drift risk. Introduce endpoint filters/middleware to attach the current player and enforce phase/admin requirements declaratively.
• Maintenance - Clear invalid cookies (Infrastructure/PlayerIdentityExtensions.cs): When a cookie references a deleted player, we keep reissuing it; endpoints respond 401 but the cookie persists. Clear the cookie if lookup fails to avoid client loops.