diff --git a/Endpoints/EndpointHelpers.cs b/Endpoints/EndpointHelpers.cs index b89f5b9..1b85e94 100644 --- a/Endpoints/EndpointHelpers.cs +++ b/Endpoints/EndpointHelpers.cs @@ -39,24 +39,28 @@ internal static class EndpointHelpers var state = await db.AppState.FirstAsync(); + var changed = false; + // Auto-upgrade any legacy Reveal phase to Vote to avoid blank screens if (player.CurrentPhase == Phase.Reveal) { player.CurrentPhase = Phase.Vote; + changed = true; } // Keep phases aligned with results availability if (state.ResultsOpen && player.CurrentPhase != Phase.Results) { player.CurrentPhase = Phase.Results; + changed = true; } else if (!state.ResultsOpen && player.CurrentPhase == Phase.Results) { player.CurrentPhase = Phase.Vote; player.VotesFinal = false; + changed = true; } - var changed = db.ChangeTracker.HasChanges(); if (changed) { await db.SaveChangesAsync(); diff --git a/TASKS.md b/TASKS.md index b234338..6c9fce7 100644 --- a/TASKS.md +++ b/TASKS.md @@ -3,7 +3,7 @@ - [x] **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. - [x] **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. +- [x] **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. - [x] **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 `` or `` 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.