From 1d9fb11c7b6155cfe284d34e6ae0346e894defc7 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 8 Aug 2026 21:32:52 +0200 Subject: [PATCH] fix: close the nine ways an unattended event loses photos or dies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every one of these was found in the pre-event audit, verified against source, and survives to production on the current main. Grouped by what actually goes wrong. PHOTOS DISAPPEAR * compression.rs no longer soft-deletes on a failed derivative. The guest got a 201, watched the card appear, then watched it vanish — the row left v_feed, find_visible_media and BOTH keepsakes, while its bytes sat on disk for 14 days waiting for a cleanup nothing announced. No screen anywhere lists compression failures, so recovery meant hand-written SQL that also had to re-add the refunded quota. Now it does exactly what the ENOSPC arm beside it already did and documented as correct: keep the row, serve the original, retry on the next boot (bounded by derivative_attempts). `upload-deleted` is no longer emitted; `upload-processed` is, so the card re-renders instead of sitting on a placeholder. * A 413 is now a reversible lock, so the blob survives. The quota moves — free disk falls, uploader count rises — so a guest goes over it having done nothing, and treating that as permanent meant a 400 MB video was pushed across cellular in full and THEN deleted from IndexedDB. Gone on both sides, and unrecoverable for an in-app camera capture that exists nowhere else. * quota_limit_bytes gained a floor and a stable divisor. The ceiling used to decrease monotonically all evening; it now settles at max(uploaders, estimated_guest_count) — a config key that was seeded, validated in the admin whitelist, and read by no code at all. The floor is clamped to what the disk can actually back, so a full volume still yields zero rather than handing out an allowance it cannot honour. * Because that floor gives up the aggregate guarantee the formula used to imply, uploads now check a hard 10 GB reserve first, independent of every quota toggle. postgres_data, media_data and exports_data share one filesystem: the end state was not a degraded feature, it was Postgres unable to write WAL. THE ARCHIVE DISAPPEARS * prune_superseded_archives runs only after the new generation lands. It ran before the preflight, reasoning the old archive was already unreachable — true of reachability, false of recoverability. An epoch is a value that can be rolled back; deleted bytes cannot. Any failed rebuild left the event with NO keepsake at all. * The export preflight reserves the same 10 GB. `free < needed` authorised an export sized at exactly free, which ran for half an hour and landed the box at zero with the keepsake still unfinished. THE APP DIES * The feed reconcile re-reads the id set after its awaits instead of reusing one captured up to three round-trips earlier. The new-upload SSE handler prepends during exactly that window, so the row was both already present and absent from the stale set — prepended twice, and a duplicate key in a keyed {#each} throws in production, not just dev. The SSE handler and loadMore now dedupe too. * Added routes/+error.svelte. Without it any uncaught error fell through to SvelteKit's unstyled English 500 with no reload control — inside a chromeless standalone PWA with no URL bar, for the rest of the evening. THE OPERATOR IS LOCKED OUT * admin_login verifies the password BEFORE charging the rate bucket, and a correct password is never throttled. The old order made this a denial of service against its own operator: every guest shares one NAT IP, the check ran first, so five requests a minute from any phone in the room kept the bucket full — and the escape hatch needed the admin session being blocked. A generous separate ceiling still bounds bcrypt CPU. THE PROJECTOR DIES * The preload budget is now strictly inside the dwell. At the 3s option the 4s budget could never land a commit on a slow uplink, so the wall froze on one photo while the queue drained silently behind it. * The wake lock retries every 30s while visible, and the page says so on screen when the browser has no wake lock API. visibilitychange was the only retry trigger and a kiosk never changes visibility, so one refusal — iOS in Low Power Mode, say — was permanent. * Caddy: /api/v1/upload/*/display joins the cacheable carve-out. The backend set max-age=300 on it and the blanket no-store silently replaced it, so a projector re-fetched a full-size JPEG per slide, ~2-4 GB over an evening on the uplink the guests are uploading over. Also removes Upload::soft_delete, now unreferenced and an unscoped footgun next to soft_delete_in_event. Verified: 146/146 backend tests against a live Postgres, clippy clean, 51/51 vitest, svelte-check 0 errors, eslint clean, vite build, caddy validate, compose YAML parse. Co-Authored-By: Claude Opus 5 (1M context) --- Caddyfile | 23 ++-- backend/src/auth/handlers.rs | 50 ++++++-- backend/src/handlers/upload.rs | 150 ++++++++++++++++++++--- backend/src/models/upload.rs | 36 +----- backend/src/services/compression.rs | 42 ++++--- backend/src/services/export.rs | 54 ++++++-- frontend/src/lib/diashow/wakelock.ts | 40 +++++- frontend/src/lib/event-config-store.ts | 18 ++- frontend/src/lib/upload-queue.ts | 28 ++++- frontend/src/routes/+error.svelte | 40 ++++++ frontend/src/routes/diashow/+page.svelte | 43 ++++++- frontend/src/routes/feed/+page.svelte | 30 ++++- 12 files changed, 448 insertions(+), 106 deletions(-) create mode 100644 frontend/src/routes/+error.svelte diff --git a/Caddyfile b/Caddyfile index f401ee6..09103ef 100644 --- a/Caddyfile +++ b/Caddyfile @@ -31,20 +31,27 @@ @hashed_assets path_regexp hashed /_app/immutable/.*\.[a-f0-9]{8,}\.(js|css|woff2)$ header @hashed_assets Cache-Control "public, max-age=31536000, immutable" - # Preview/thumbnail images. These are served by the app through a visibility-checked - # alias (/api/v1/upload/{id}/{preview,thumbnail}) so moderation can revoke access; - # the app serves no /media route at all, so there is no direct path to the bytes. - # Privately cacheable for a short window (the app sets the same header; this is the - # edge carve-out from the blanket no-store below). Kept short so a moderated image - # stops being served to a direct-URL holder promptly. - @media_api path /api/v1/upload/*/preview /api/v1/upload/*/thumbnail + # Preview/thumbnail/display images. These are served by the app through a + # visibility-checked alias (/api/v1/upload/{id}/{preview,thumbnail,display}) so + # moderation can revoke access; the app serves no /media route at all, so there is no + # direct path to the bytes. Privately cacheable for a short window (the app sets the + # same header; this is the edge carve-out from the blanket no-store below). Kept short + # so a moderated image stops being served to a direct-URL holder promptly. + # + # `display` was missing here while the backend set `private, max-age=300` on it, and + # because `header` REPLACES, the blanket no-store below silently won. That route is the + # ~2048px derivative the diashow uses exclusively, so a projector left running all + # evening re-fetched a full-size JPEG for every slide — roughly 2-4 GB pulled through + # the app over 8 hours, on the same venue uplink 100 guests are uploading over, and a + # blank frame on every network hiccup. + @media_api path /api/v1/upload/*/preview /api/v1/upload/*/thumbnail /api/v1/upload/*/display header @media_api Cache-Control "private, max-age=300" # API and health — never cache, EXCEPT the gated image routes above. A cached health # response would report the last known state rather than the current one. @api { path /api/* /health - not path /api/v1/upload/*/preview /api/v1/upload/*/thumbnail + not path /api/v1/upload/*/preview /api/v1/upload/*/thumbnail /api/v1/upload/*/display } header @api Cache-Control "no-store" diff --git a/backend/src/auth/handlers.rs b/backend/src/auth/handlers.rs index 3c515d1..a537686 100644 --- a/backend/src/auth/handlers.rs +++ b/backend/src/auth/handlers.rs @@ -409,6 +409,14 @@ pub struct AdminLoginResponse { pub display_name: String, } +/// Requests per minute per IP that may reach `verify_password` at all. +/// +/// Not a security control — the failure bucket below is. This exists solely so an unauthenticated +/// endpoint cannot burn the box's CPU on cost-12 bcrypt (~250 ms each) at line rate. Set far above +/// anything a person typing a password can produce, because on venue NAT every guest shares the +/// operator's IP and this ceiling, unlike the failure bucket, can still refuse a correct password. +const ADMIN_LOGIN_CPU_CEILING: usize = 120; + pub async fn admin_login( State(state): State, ConnectInfo(peer): ConnectInfo, @@ -421,21 +429,29 @@ pub async fn admin_login( )); } - // Throttle password attempts. The admin password is bcrypt-hashed (slow to - // verify) but with no IP-level limit a determined attacker can still mount - // a long-running guess campaign. 5 attempts / minute / IP is plenty for - // honest typos. + // Throttling here is in two parts, and the ORDER is the whole point. + // + // A single tight IP-keyed bucket checked before the password was verified made this + // endpoint a denial-of-service against its own operator. Every guest at the venue shares + // one public IP behind NAT, `/admin/login` is a public linkable page, and the check ran + // BEFORE `verify_password` — so five requests a minute from any phone in the room kept the + // bucket permanently full and the admin, on that same IP, could never spend a slot. + // Successful logins consumed budget too, so a typo plus a retry on two devices did it by + // accident. And the escape hatch was circular: `admin_login_rate_enabled` can only be + // flipped through `PATCH /admin/config`, which needs the session being blocked. let ip = client_ip(&headers, &peer.ip().to_string()); let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await; let admin_rate_on = config::get_bool(&state.config_cache, "admin_login_rate_enabled", true).await; - // Stays keyed by IP on purpose: this guards a single shared credential, so a per-user - // or per-name key would just hand an attacker a fresh bucket per guess. + + // Part 1: a deliberately GENEROUS ceiling whose only job is to bound bcrypt CPU. Cost-12 + // verification is ~250 ms of a core, so an unbounded endpoint is a CPU exhaustion vector + // regardless of whether anyone guesses right. No human typing a password reaches this. if rate_limits_on && admin_rate_on && let Err(retry_after_secs) = state.rate_limiter.check_with_retry( - format!("admin_login:{ip}"), - 5, + format!("admin_login_cpu:{ip}"), + ADMIN_LOGIN_CPU_CEILING, Duration::from_secs(60), ) { @@ -452,6 +468,24 @@ pub async fn admin_login( .await; if !valid { + // Part 2: the tight bucket, charged ONLY on a wrong password. A correct password is + // never rate-limited, so no amount of guessing by anyone else can lock the operator + // out — which also dissolves the circular escape hatch above. Brute force is still + // bounded: every wrong guess costs a slot, and slots are per-IP. + if rate_limits_on + && admin_rate_on + && let Err(retry_after_secs) = state.rate_limiter.check_with_retry( + format!("admin_login_fail:{ip}"), + 5, + Duration::from_secs(60), + ) + { + tracing::warn!(ip = %ip, "admin_login: wrong password, failure bucket exhausted"); + return Err(AppError::TooManyRequests( + "Zu viele Anmeldeversuche. Bitte warte kurz und versuche es erneut.".into(), + Some(retry_after_secs), + )); + } tracing::warn!(ip = %ip, "admin_login: wrong password"); return Err(AppError::Unauthorized("Falsches Passwort.".into())); } diff --git a/backend/src/handlers/upload.rs b/backend/src/handlers/upload.rs index 92e1e57..aba954a 100644 --- a/backend/src/handlers/upload.rs +++ b/backend/src/handlers/upload.rs @@ -443,6 +443,38 @@ pub async fn upload( // concurrent uploads from the same user (e.g. phone + laptop) both pass this stale // pre-check and both increment, blowing past the quota. The pre-check stays as a // fast path that avoids the disk write when the user is already clearly over. + // GLOBAL RESERVE, checked before the per-user ceiling and independent of every quota + // toggle. The per-user quota is a fairness mechanism, not a disk guarantee — and since it + // now carries a floor (MIN_QUOTA_LIMIT_BYTES) so a guest's allowance stops shrinking as + // the party fills up, the aggregate ceiling it used to imply is gone entirely. Something + // has to own "do not fill the volume", because `postgres_data`, `media_data` and + // `exports_data` share one filesystem: the end state is not a degraded feature, it is + // Postgres unable to write WAL and the whole event down with nobody watching. + // + // Deliberately NOT gated behind `quota_enabled`. That switch exists so an operator can + // stop rationing space between guests; it was never meant to authorise running the disk + // to zero, and an operator flipping it at 23:00 to unblock a guest should not silently + // disarm the last thing standing between the party and a dead database. + if let Some(free) = crate::services::disk::free_bytes(&state.config.media_path) { + let remaining = (free as i64).saturating_sub(size); + if remaining < DISK_RESERVE_BYTES { + tracing::error!( + free_bytes = free, + upload_size = size, + reserve = DISK_RESERVE_BYTES, + "refusing upload: it would take the media volume below the reserve" + ); + return Err(AppError::QuotaExceeded( + "Der Speicher des Events ist voll. Bitte sag einem Host Bescheid — neue \ + Uploads sind vorübergehend nicht möglich." + .into(), + )); + } + } + // Failing OPEN when the disk can't be read is deliberate and matches the per-user quota + // above: refusing every upload because a `statfs` failed would be a worse outage than the + // one being guarded against. + let mut quota_limit: Option = None; if quota_on && storage_quota_on { let estimate = compute_storage_quota(&state).await; @@ -956,11 +988,52 @@ pub struct QuotaEstimate { pub tolerance: f64, } -/// Pure per-user quota formula: `floor((free_disk * tolerance) / max(active, 1))`. +/// The smallest per-user ceiling this formula is ever allowed to produce. +/// +/// Without a floor the quota is not a limit, it is a moving target: the numerator (free disk) +/// only falls and the denominator (uploaders who have posted) only rises, so the ceiling +/// decreases monotonically across the event. A guest comfortably under it at 20:00 is over it +/// at 22:00 having done nothing, and because a delete refunds the quota but does not free the +/// bytes for 24h, the remedy the error message names ("delete older posts") cannot move it +/// back either. +/// +/// 500 MB is chosen to clear `max_video_size_mb` (500, seeded in 005) — below that the ceiling +/// could refuse a single legal video outright, which is the worst version of this: the guest +/// pushes 500 MB across cellular and is rejected on arrival, every time, with no way to comply. +/// +/// This deliberately trades the quota's disk guarantee for a usability floor. The disk is now +/// bounded by the low-disk warning and the export preflight rather than by this formula alone — +/// see the reserve check in `ensure_export_space`. +const MIN_QUOTA_LIMIT_BYTES: i64 = 500 * 1024 * 1024; + +/// Free space on the media volume that uploads may never consume, whatever any quota says. +/// +/// Matches the host dashboard's low-disk threshold (`handlers::host`), so the banner the host +/// sees and the wall guests hit are the same number rather than two unrelated opinions. 10 GB +/// is chosen to leave Postgres, its WAL and a rotation of container logs comfortable room on +/// the shared filesystem long after new uploads have been refused. +pub const DISK_RESERVE_BYTES: i64 = 10_000_000_000; + +/// Pure per-user quota formula: `max(floor((free_disk * tolerance) / divisor), MIN)`. +/// +/// `divisor` is the LARGER of the observed uploader count and the operator's +/// `estimated_guest_count`, so the ceiling settles at its final value early instead of sliding +/// down all evening as guests arrive. (Before this, `estimated_guest_count` was seeded and +/// validated in the admin whitelist but read by no code at all — an operator who set it +/// expecting a stable divisor changed nothing.) It also blunts the abuse case, where the +/// divisor was attacker-controlled: ~1000 throwaway accounts drove every real guest's ceiling +/// to ~52 MB. +/// /// Extracted from `compute_storage_quota` so it's unit-testable without a DB or disk. -fn quota_limit_bytes(free_disk: i64, tolerance: f64, active_uploaders: i64) -> i64 { - let active = active_uploaders.max(1); - ((free_disk as f64 * tolerance) / active as f64).floor() as i64 +fn quota_limit_bytes(free_disk: i64, tolerance: f64, active_uploaders: i64, expected: i64) -> i64 { + let divisor = active_uploaders.max(expected).max(1); + let budget = (free_disk as f64 * tolerance).max(0.0); + let computed = (budget / divisor as f64).floor() as i64; + // The floor may never exceed what the disk can actually back. Raising a ceiling the volume + // cannot honour would hand out an allowance on a full disk — turning the quota from a + // usability floor into a way to finish filling the filesystem that Postgres writes WAL to. + let backed_floor = MIN_QUOTA_LIMIT_BYTES.min(budget as i64); + computed.max(backed_floor) } /// Computes the per-user storage quota using @@ -979,6 +1052,9 @@ pub async fn compute_storage_quota(state: &AppState) -> QuotaEstimate { .await .unwrap_or((0,)); let active = active_count.max(1); + // The operator's expected headcount, used as a FLOOR on the divisor so the ceiling doesn't + // slide down as guests arrive — see `quota_limit_bytes`. Admin-editable at runtime. + let expected_guests = config::get_i64(&state.config_cache, "estimated_guest_count", 100).await; // Cached disk reading. `None` means we couldn't resolve the media filesystem. let disk = state.disk_cache.snapshot(&state.config.media_path); @@ -986,7 +1062,12 @@ pub async fn compute_storage_quota(state: &AppState) -> QuotaEstimate { let limit_bytes = if quota_on && storage_quota_on { match disk { - Some(d) => Some(quota_limit_bytes(d.free as i64, tolerance, active)), + Some(d) => Some(quota_limit_bytes( + d.free as i64, + tolerance, + active, + expected_guests, + )), // Fail OPEN, not closed: if the disk can't be read we don't know the real // free space, and enforcing a 0-byte limit would reject every upload with a // spurious "quota reached". Skip enforcement this round and warn instead. @@ -1289,7 +1370,7 @@ pub async fn get_thumbnail( #[cfg(test)] mod tests { - use super::{RangeSpec, parse_range, quota_limit_bytes}; + use super::{MIN_QUOTA_LIMIT_BYTES, RangeSpec, parse_range, quota_limit_bytes}; // `Range` handling exists because iOS Safari probes every `