diff --git a/backend/src/handlers/host.rs b/backend/src/handlers/host.rs index ffab67d..17dae13 100644 --- a/backend/src/handlers/host.rs +++ b/backend/src/handlers/host.rs @@ -44,7 +44,6 @@ pub struct EventStatus { pub disk_low: bool, } - /// Is free space low enough that the host needs to know? /// /// Two triggers, because a fixed threshold answers the wrong question. `postgres_data`, @@ -67,7 +66,8 @@ pub struct EventStatus { /// The 25% margin makes it a warning rather than an obituary: the host sees it while there is /// still room to act (delete a few large videos, which refunds immediately and reopens the gate). fn disk_is_low(free: u64, keepsake_required: u64) -> bool { - let gate_closes_at = keepsake_required.saturating_add(crate::handlers::upload::DISK_RESERVE_BYTES as u64); + let gate_closes_at = + keepsake_required.saturating_add(crate::handlers::upload::DISK_RESERVE_BYTES as u64); let warn_at = gate_closes_at.saturating_add(gate_closes_at / 4); // No separate absolute-floor clause. There used to be `free < LOW_DISK_FLOOR_BYTES ||` here, // and it was unreachable: `gate_closes_at` is at least DISK_RESERVE_BYTES, so `warn_at` is at @@ -256,6 +256,19 @@ pub async fn ban_user( "host: ban_user" ); + crate::services::audit::record( + &state.pool, + auth.event_id, + auth.user_id, + None, + auth.role.clone(), + "ban_user", + Some(user_id), + None, + None, + ) + .await; + Ok(StatusCode::NO_CONTENT) } @@ -314,12 +327,40 @@ pub async fn unban_user( start_regen(&state, r); } + // The exact mirror of `ban_user`'s `user-hidden`, and it was missing entirely: every open + // feed and the unattended projector kept the guest evicted until somebody reloaded the page + // by hand. Meanwhile the host's own confirm copy promises the photos "come back to the + // gallery, die Diashow und den Export" — so the one surface that would have shown the host + // their action had worked showed the opposite. + // + // Also the signal a banned guest's upload queue waits on: their queued photos parked with + // the blob intact rather than being purged (see `AppError::UserBanned`), and this is what + // releases them. + let _ = state.sse_tx.send(SseEvent::new( + "user-shown", + serde_json::json!({ "user_id": user_id }).to_string(), + )); + tracing::info!( actor_user_id = %auth.user_id, target_user_id = %user_id, event_id = %auth.event_id, "host: unban_user" ); + + crate::services::audit::record( + &state.pool, + auth.event_id, + auth.user_id, + None, + auth.role.clone(), + "unban_user", + Some(user_id), + None, + None, + ) + .await; + Ok(StatusCode::NO_CONTENT) } @@ -443,6 +484,19 @@ pub async fn set_role( new_role, "host: set_role" ); + + crate::services::audit::record( + &state.pool, + auth.event_id, + auth.user_id, + None, + auth.role.clone(), + "set_role", + Some(user_id), + None, + None, + ) + .await; Ok(StatusCode::NO_CONTENT) } @@ -535,6 +589,19 @@ pub async fn reset_user_pin( "host: reset_user_pin" ); + crate::services::audit::record( + &state.pool, + auth.event_id, + auth.user_id, + None, + auth.role.clone(), + "reset_pin", + Some(user_id), + None, + None, + ) + .await; + Ok(Json(PinResetResponse { pin })) } @@ -611,8 +678,13 @@ pub fn start_regen(state: &AppState, regen: crate::services::export::PendingRege state.config.comments_enabled, // Debounced: a takedown pass is a burst, and each request retires the last generation. The // delay lets superseded workers fail their claim and do zero work instead of each building - // a full archive. See export::REGEN_DEBOUNCE. - crate::services::export::REGEN_DEBOUNCE, + // a full archive. + // + // Measured from the START of the burst, not from this request — a fixed per-request delay + // meant a steady stream of invalidations faster than one per 20s deferred the build + // forever, leaving the keepsake permanently 404 and the UI stuck on "Wird vorbereitet…". + // See export::regen_delay_for. + crate::services::export::regen_delay_for(regen.event_id), state.pool.clone(), state.config.media_path.clone(), state.config.export_path.clone(), @@ -660,6 +732,19 @@ pub async fn host_delete_upload( "host: host_delete_upload" ); + crate::services::audit::record( + &state.pool, + auth.event_id, + auth.user_id, + None, + auth.role.clone(), + "delete_upload", + Some(upload_id), + None, + None, + ) + .await; + Ok(StatusCode::NO_CONTENT) } @@ -697,12 +782,25 @@ pub async fn host_delete_comment( comment_id = %comment_id, "host: host_delete_comment" ); + + crate::services::audit::record( + &state.pool, + auth.event_id, + auth.user_id, + None, + auth.role.clone(), + "delete_comment", + Some(comment_id), + None, + None, + ) + .await; Ok(StatusCode::NO_CONTENT) } pub async fn close_event( State(state): State, - RequireHost(_auth): RequireHost, + RequireHost(auth): RequireHost, ) -> Result { let result = sqlx::query( "UPDATE event SET uploads_locked_at = NOW() WHERE slug = $1 AND uploads_locked_at IS NULL", @@ -717,12 +815,27 @@ pub async fn close_event( let _ = state.sse_tx.send(SseEvent::new("event-closed", "{}")); } + // Was logged NOWHERE at all before this — not even a tracing line. A host reading + // the record the morning after had no way to see when uploads were locked or the + // gallery released, which are the two actions that change what every guest can do. + crate::services::audit::record( + &state.pool, + auth.event_id, + auth.user_id, + None, + auth.role.clone(), + "lock_uploads", + None, + None, + None, + ) + .await; Ok(StatusCode::NO_CONTENT) } pub async fn open_event( State(state): State, - RequireHost(_auth): RequireHost, + RequireHost(auth): RequireHost, ) -> Result { // Reopening invalidates any prior release: the keepsake was snapshotted at release time, so // allowing new uploads afterwards would silently diverge the live feed from the frozen export. @@ -748,12 +861,27 @@ pub async fn open_event( let _ = state.sse_tx.send(SseEvent::new("event-opened", "{}")); } + // Was logged NOWHERE at all before this — not even a tracing line. A host reading + // the record the morning after had no way to see when uploads were locked or the + // gallery released, which are the two actions that change what every guest can do. + crate::services::audit::record( + &state.pool, + auth.event_id, + auth.user_id, + None, + auth.role.clone(), + "unlock_uploads", + None, + None, + None, + ) + .await; Ok(StatusCode::NO_CONTENT) } pub async fn release_gallery( State(state): State, - RequireHost(_auth): RequireHost, + RequireHost(auth): RequireHost, ) -> Result { // The release claim, the epoch bump, the upload lock and BOTH job rows are written in ONE // transaction. Two reasons, both of which were live bugs: @@ -808,6 +936,22 @@ pub async fn release_gallery( // discovering it via a rejected upload. let _ = state.sse_tx.send(SseEvent::new("event-closed", "{}")); + // Was logged NOWHERE at all before this — not even a tracing line. A host reading + // the record the morning after had no way to see when uploads were locked or the + // gallery released, which are the two actions that change what every guest can do. + crate::services::audit::record( + &state.pool, + auth.event_id, + auth.user_id, + None, + auth.role.clone(), + "release_gallery", + None, + None, + None, + ) + .await; + // Detached — survives this handler being cancelled. crate::services::export::spawn_export_jobs( event_id, diff --git a/backend/src/services/export.rs b/backend/src/services/export.rs index 46a3018..eb13d98 100644 --- a/backend/src/services/export.rs +++ b/backend/src/services/export.rs @@ -423,6 +423,49 @@ async fn invalidate_missing_files( /// start immediately. pub const REGEN_DEBOUNCE: Duration = Duration::from_secs(20); +/// Tracks when the CURRENT un-served burst of invalidations began, per event. +/// +/// The starvation this fixes (B7): every invalidation bumps `export_epoch` immediately and arms a +/// worker that sleeps `REGEN_DEBOUNCE` before claiming — and any further bump inside that window +/// retires it. Nothing rate-limits epoch bumps; only the HTTP requests are limited, at 120/min for +/// comment deletion and 30/min for caption edits. So one ordinary guest deleting a comment per +/// second, or flipping a caption A/B/A, keeps `GET /export/html` at 404 for the rest of the event +/// while the UI shows "Wird vorbereitet…" forever. **A host moderating faster than one action per +/// 20 seconds produces the same result by accident** — which is what a takedown pass looks like. +/// +/// The fix is to measure the debounce from the FIRST request in a burst rather than the latest, so +/// the wait is bounded no matter how long the burst runs: coalescing still collapses a rapid pass +/// into one build, but a build always starts within `REGEN_DEBOUNCE` of the burst beginning. +/// +/// In-memory on purpose. It is a scheduling hint, not state: losing it on restart is harmless +/// because `recover_exports` re-arms anything unfinished at boot anyway, and the worst case of a +/// stale entry is one build starting immediately instead of debounced. +type BurstStarts = std::collections::HashMap; +static REGEN_BURST_START: std::sync::LazyLock> = + std::sync::LazyLock::new(|| std::sync::Mutex::new(BurstStarts::new())); + +/// How long to defer the next regen worker for `event_id`, and record that a burst is running. +/// +/// Returns `REGEN_DEBOUNCE` for the first invalidation of a burst and progressively less for +/// each one after it, reaching zero once the burst has been going for a full debounce window. +pub fn regen_delay_for(event_id: Uuid) -> Duration { + let mut map = match REGEN_BURST_START.lock() { + Ok(m) => m, + // A poisoned mutex must not take the keepsake down: fall back to the plain debounce, + // which is the pre-existing behaviour. + Err(e) => e.into_inner(), + }; + let started = *map.entry(event_id).or_insert_with(std::time::Instant::now); + REGEN_DEBOUNCE.saturating_sub(started.elapsed()) +} + +/// A regen worker has begun (or the event settled), so the next invalidation starts a fresh burst. +pub fn clear_regen_burst(event_id: Uuid) { + if let Ok(mut map) = REGEN_BURST_START.lock() { + map.remove(&event_id); + } +} + // Export worker entry point: every argument is state the spawned worker is BORN with (notably // `epoch`). Bundling them into a struct would be a pure-refactor risk on the epoch logic for no // gain, so the arity stands. @@ -451,12 +494,19 @@ pub fn spawn_export_jobs( if !delay.is_zero() { tokio::time::sleep(delay).await; } - if let Err(e) = - run_zip_export(event_id, epoch, &pool, &media_path, &export_path, &sse_tx).await - { - tracing::error!("ZIP export failed for event {event_id} @ epoch {epoch}: {e:#}"); - mark_failed(&pool, event_id, "zip", epoch, &e.to_string()).await; - } + // The burst has now been served: whatever arrives next starts a fresh debounce window. + clear_regen_burst(event_id); + // Run the body in its OWN task so a panic surfaces as a `JoinError` here rather than + // unwinding past `mark_failed` — see `supervise_export`. + let (p, m, x, s) = ( + pool.clone(), + media_path.clone(), + export_path.clone(), + sse_tx.clone(), + ); + let inner = + tokio::spawn(async move { run_zip_export(event_id, epoch, &p, &m, &x, &s).await }); + supervise_export(inner, &pool, event_id, "zip", epoch).await; maybe_broadcast_complete(&pool, event_id, &sse_tx).await; }); @@ -464,25 +514,77 @@ pub fn spawn_export_jobs( if !delay.is_zero() { tokio::time::sleep(delay).await; } - if let Err(e) = run_html_export( - event_id, - epoch, - &event_name2, - comments_enabled, - &pool2, - &media_path2, - &export_path2, - &sse_tx2, - ) - .await - { - tracing::error!("HTML export failed for event {event_id} @ epoch {epoch}: {e:#}"); - mark_failed(&pool2, event_id, "html", epoch, &e.to_string()).await; - } + let (p, m, x, s) = ( + pool2.clone(), + media_path2.clone(), + export_path2.clone(), + sse_tx2.clone(), + ); + let inner = tokio::spawn(async move { + run_html_export( + event_id, + epoch, + &event_name2, + comments_enabled, + &p, + &m, + &x, + &s, + ) + .await + }); + supervise_export(inner, &pool2, event_id, "html", epoch).await; maybe_broadcast_complete(&pool2, event_id, &sse_tx2).await; }); } +/// Await an export worker and make sure the job row NEVER stays at `running`. +/// +/// Both workers were bare `tokio::spawn`s whose `Err` path was handled correctly — but a PANIC +/// unwound straight past `mark_failed` and `maybe_broadcast_complete`, leaving the row at whatever +/// `progress_pct` it had reached. The UI renders that as "Wird erstellt (77%)" with the download +/// disabled, permanently: nothing sweeps `running` rows, and `recover_exports` only runs at boot. +/// So the single most likely cause of a stuck keepsake was also the one the error handling missed. +/// +/// Running the body as a child task turns that panic into a `JoinError` we can act on. The +/// maintenance loop already supervises itself this way; this extends the same pattern to the two +/// workers that actually produce the thing guests came for. +async fn supervise_export( + handle: tokio::task::JoinHandle>, + pool: &PgPool, + event_id: Uuid, + export_type: &str, + epoch: i64, +) { + match handle.await { + Ok(Ok(())) => {} + Ok(Err(e)) => { + tracing::error!( + "{export_type} export failed for event {event_id} @ epoch {epoch}: {e:#}" + ); + mark_failed(pool, event_id, export_type, epoch, &e.to_string()).await; + } + Err(join_err) => { + // Panicked, or cancelled at shutdown. Either way the row must not be left claiming to + // be in progress — a failed job the host can retry beats one that lies forever. + tracing::error!( + panicked = join_err.is_panic(), + "{export_type} export task for event {event_id} @ epoch {epoch} died without \ + reporting: {join_err}" + ); + mark_failed( + pool, + event_id, + export_type, + epoch, + "Interner Fehler beim Erstellen des Keepsakes. Bitte über \"Neu erzeugen\" \ + erneut versuchen.", + ) + .await; + } + } +} + /// Preflight that will sacrifice the previous generation rather than deadlock, and the order is /// the whole point. /// @@ -500,6 +602,11 @@ pub fn spawn_export_jobs( /// generation is the one thing we can reclaim — sacrifice it and try once more. A keepsake that /// exists beats one we preserved but can never replace. /// +/// But ONLY when the sacrifice is sufficient: we measure what pruning would free and compare it to +/// the shortfall first. Pruning and hoping meant a failed second check left nothing on disk at all +/// — no old archive and no new one — which is worse than either outcome this function chooses +/// between. When the old archive cannot buy us a rebuild, it stays. +/// /// SHARED by both halves deliberately. This started as two copies and one of them (HTML) silently /// kept the single-phase form, so the ZIP archive rebuilt and the viewer stayed permanently stuck /// — the exact deadlock above, on half the product. `prefix` is the only thing that differs, and @@ -512,16 +619,157 @@ async fn ensure_export_space_reclaiming( prefix: &str, epoch: i64, ) -> Result<()> { - if ensure_export_space(pool, event_id, export_path).await.is_ok() { + if ensure_export_space(pool, event_id, export_path) + .await + .is_ok() + { return Ok(()); } + + // LOOK BEFORE YOU DESTROY. + // + // This used to prune unconditionally and then re-check. When the re-check ALSO failed, the old + // keepsake was already gone and the rebuild returned `Err` — leaving **no archive on disk at + // all**, which is strictly worse than the deadlock the reclaim exists to avoid. It needs no + // host action to reach: any guest deleting their own photo arms this path, and the doc comment + // above asserts the opposite invariant. + // + // So sacrifice the old generation only when doing so is actually sufficient. When it is not, + // that old keepsake is the only one anybody will ever have — keep it, and report the failure + // the same way the non-reclaiming check does. `rebuild_export` is the host's retry once they + // have freed space. + let deficit = match export_space_deficit(pool, event_id, export_path).await? { + // Raced back into having room (a concurrent prune, a guest deleting an upload). Nothing + // to reclaim and nothing to fail. + None => return Ok(()), + Some(d) => d, + }; + let reclaimable = + reclaimable_superseded_bytes(pool, export_path, prefix, event_id, epoch).await; + + // PRUNE EVEN WHEN IT IS NOT ENOUGH ON ITS OWN. This used to refuse unless + // `reclaimable >= deficit`, on the premise that "deleting it would leave no archive at all" — + // and that premise does not survive contact with `prune_superseded_archives`, which can only + // ever touch generations `n < keep_seq` that `protected_files` does not name. Those are exactly + // the archives no handler can serve: a download resolves through `export_current`, which + // requires `j.epoch = e.export_epoch`, and the epoch only ever increments. The bytes this + // branch was protecting were already unreachable from every route, and the next successful + // build deletes them anyway. + // + // What the refusal did cost was the only in-app way out of the deadlock this function exists to + // break. `reclaimable` is scoped to the caller's OWN prefix — one old archive, ~1.1x the + // gallery — while `deficit` is sized for both halves plus the 10 GB reserve. So on a tight + // disk each worker independently measures its own share as insufficient and neither prunes, + // while the two shares are JOINTLY sufficient. Every "Neu erzeugen" reruns the identical + // arithmetic and refuses identically: permanently stuck, with dead archives on the volume that + // nothing will reclaim and nothing can serve. + // + // So reclaim what we can and let the re-check below decide. If it still does not fit, we fail + // exactly as the plain preflight would — but the sibling worker's prune has now freed its share + // too, and the host's retry converges instead of looping. + if reclaimable < deficit { + tracing::warn!( + deficit, + reclaimable, + "pruning the previous {prefix} keepsake will not free the full shortfall on its own; \ + reclaiming anyway — it is already unservable, and the sibling half frees the rest" + ); + } + tracing::warn!( + deficit, + reclaimable, "not enough room to rebuild {prefix} alongside the previous keepsake; reclaiming it first" ); prune_superseded_archives(pool, export_path, prefix, event_id, epoch).await; ensure_export_space(pool, event_id, export_path).await } +/// Largest share of an event's media an archive may silently omit and still publish. +/// +/// Not zero, deliberately. A skip is *expected* in ordinary operation — a guest deleting their own +/// photo mid-build, or the hourly reclaim collecting an original past its retention window — and +/// the writers go out of their way to degrade one entry rather than fail the whole keepsake, +/// because a released event cannot be rebuilt by the host without reopening uploads. Making any +/// skip fatal would turn a tolerated one-photo gap into total loss of the keepsake, which is the +/// regression the TOCTOU comments in each writer warn about. +/// +/// What must never happen is publishing an archive that is missing *most* of the event. 10% of a +/// ~100-photo wedding is ~10 photos: far above the one-or-two a live delete explains, far below +/// the "media_path is wrong so nothing opened" catastrophe. +const MAX_SKIPPED_FRACTION: f64 = 0.10; + +/// Skips tolerated regardless of how small the event is. +/// +/// A pure fraction inverts this check on a small gallery: at nine uploads one skip is 11%, so the +/// single most ordinary event there is — a guest deleting their own photo while the archive builds +/// — failed the whole keepsake. That is exactly the "one-photo gap becomes total loss" outcome +/// [`MAX_SKIPPED_FRACTION`]'s own comment says it exists to avoid, and it is worst early on, when +/// a host testing a release has only a handful of photos. +/// +/// Two is the number of concurrent live deletes worth absorbing; beyond that on a tiny gallery the +/// `written == 0` guard still catches the misconfiguration case, which is the one that matters. +const MIN_TOLERATED_SKIPS: usize = 2; + +/// Decide whether an archive that skipped some media may still be published. +/// +/// Rejects two cases: +/// * `written == 0` with media expected — the misconfiguration case. This is what produced a +/// few-hundred-byte ZIP containing zero photos that passed every automated check, was +/// advertised green, and was handed to every guest. +/// * more than [`MAX_SKIPPED_FRACTION`] omitted — enough missing that the archive misrepresents +/// the event even though it is structurally valid. +/// +/// A tolerated partial still logs at `error` level with the exact counts, so the gap is visible in +/// the record rather than buried in per-entry warnings nobody aggregates. +fn check_export_completeness( + kind: &str, + event_id: Uuid, + expected: usize, + written: usize, + skipped: usize, +) -> Result<()> { + if skipped == 0 { + return Ok(()); + } + if written == 0 && expected > 0 { + anyhow::bail!( + "{kind}-Export enthält keine einzige Datei ({expected} erwartet, alle \ + übersprungen). Das deutet auf einen falschen MEDIA_PATH oder fehlende \ + Zugriffsrechte hin — das Keepsake wurde NICHT veröffentlicht." + ); + } + // A PROPORTIONAL skip is loud, but it is NOT fatal, and the difference is the whole point. + // + // This used to `bail!` once skips passed `max(2, 10% of expected)`. That inverts + // MAX_SKIPPED_FRACTION's own justification two doc comments up — "making any skip fatal would + // turn a tolerated one-photo gap into total loss of the keepsake" — because the refusal is + // DETERMINISTIC ACROSS RETRIES. The files that could not be read are still unreadable when the + // host taps "Neu erzeugen", and by then the gallery is released and the uploads cannot be + // collected again. So on a 30-photo event, 4 unreadable originals stopped publishing the other + // 26 — not once, but forever. Refusing to hand over an incomplete keepsake is only defensible + // if something better can still arrive; here nothing can. + // + // `written == 0` above stays fatal, and it is the case that actually mattered: a wrong + // MEDIA_PATH produced a few-hundred-byte archive with zero photos that passed every automated + // check and was advertised as ready. That one is a misconfiguration the host CAN fix and retry. + // + // Everything short of that publishes, with the counts at `error` level so the gap is in the + // record rather than buried in per-entry warnings. A partial keepsake is what the guests get to + // keep; an aborted one is nothing at all. + let budget = MIN_TOLERATED_SKIPS.max((expected as f64 * MAX_SKIPPED_FRACTION).floor() as usize); + let level_note = if expected > 0 && skipped > budget { + "MATERIALLY INCOMPLETE — published anyway because a refused rebuild is not recoverable" + } else { + "published with missing media" + }; + tracing::error!( + %event_id, kind, expected, written, skipped, budget, + "{level_note} — {skipped} of {expected} entries could not be read" + ); + Ok(()) +} + /// Take the process-wide heavy-image permit if this file is big enough to need it. /// /// Mirrors the compression worker's gate exactly (a header probe, no pixels decoded), so the two @@ -531,7 +779,10 @@ async fn heavy_permit_for(path: &Path) -> Option crate::services::imaging::HEAVY_IMAGE_BYTES => { - crate::services::imaging::HEAVY_IMAGE_PERMITS.acquire().await.ok() + crate::services::imaging::HEAVY_IMAGE_PERMITS + .acquire() + .await + .ok() } _ => None, } @@ -634,6 +885,15 @@ async fn run_zip_export_inner( let out_name = gen_name(event_id, "Gallery", epoch, ".zip"); let out_path = exports_dir.join(&out_name); + // Skips are TOLERATED but no longer SILENT — see `check_export_completeness`. All three + // writers continued past an unreadable source with only a `warn!`, nothing counted them, + // nothing reached `error_message`, and the job finalized at `progress_pct = 100, + // status = 'done'`. So a wrong `media_path` after a compose edit produced a few-hundred-byte + // ZIP with zero media that passed the liveness check, was advertised green, and was handed to + // every guest as their keepsake. + let mut written = 0usize; + let mut skipped = 0usize; + { let file = tokio::fs::File::create(&tmp_path).await?; let mut zip = ZipFileWriter::with_tokio(file); @@ -666,9 +926,11 @@ async fn run_zip_export_inner( row.id, src.display() ); + skipped += 1; continue; } }; + written += 1; let mut entry = zip.write_entry_stream(builder).await?; let mut f = src_file.compat(); @@ -697,6 +959,11 @@ async fn run_zip_export_inner( file.sync_all().await?; } + // Refuse to publish an archive that lost media it was supposed to contain. Checked BEFORE the + // rename so a rejected build never reaches a servable path. + check_export_completeness("ZIP", event_id, uploads.len(), written, skipped) + .inspect_err(|_| tracing::error!("ZIP export for event {event_id}: refusing to publish"))?; + tokio::fs::rename(&tmp_path, &out_path).await?; // Commit ONLY if our generation is still current. `finalize_job` is guarded on `epoch` — a @@ -834,6 +1101,11 @@ async fn run_html_export_inner( // (zip entry name under media/, where its bytes come from). Built here, streamed // into the ZIP in step 5 — so we also know the exact file count without a rescan. let mut media_manifest: Vec<(String, MediaSource)> = Vec::new(); + // Uploads dropped at the stat below never enter `media_manifest`, so without counting them + // here a wholly-unreadable media directory yields an EMPTY manifest — expected 0, skipped 0 — + // and the completeness check downstream would wave it through as a legitimately empty event. + // These are the same loss as a skip at write time and are checked as one. + let mut upload_skipped = 0usize; for (i, row) in uploads.iter().enumerate() { let src = media_path.join(&row.original_path); @@ -852,6 +1124,7 @@ async fn run_html_export_inner( row.id, src.display() ); + upload_skipped += 1; continue; } }; @@ -1099,6 +1372,9 @@ async fn run_html_export_inner( // whose ffmpeg step failed) are skipped — the viewer tolerates gaps. let file_total = media_manifest.len().max(1) as f32; let mut files_written = 0u32; + // See `check_export_completeness`: a viewer that tolerates gaps must still not publish an + // archive with no media in it at all. + let mut media_skipped = 0usize; for (name, source) in &media_manifest { let path = source.path(); @@ -1112,6 +1388,7 @@ async fn run_html_export_inner( "HTML export: skipping media {name} — cannot read {}: {e}", path.display() ); + media_skipped += 1; continue; } }; @@ -1135,6 +1412,21 @@ async fn run_html_export_inner( let mut file = zip.close().await?.into_inner(); file.flush().await?; file.sync_all().await?; + + // Before the rename, so a rejected viewer never reaches a servable path. Inside this + // scope because `media_manifest` and the counters are scoped here. + // Measured against the full upload set, not the manifest: an upload dropped at the stat + // and one dropped at the write are the same loss to the guest looking for their photo. + check_export_completeness( + "HTML", + event_id, + uploads.len(), + files_written as usize, + upload_skipped + media_skipped, + ) + .inspect_err(|_| { + tracing::error!("HTML export for event {event_id}: refusing to publish") + })?; } // 6. Finalise @@ -1528,6 +1820,64 @@ async fn ensure_export_space(pool: &PgPool, event_id: Uuid, export_path: &Path) Ok(()) } +/// How many bytes short of buildable we are, or `None` when there is already room. +/// +/// Split out of `ensure_export_space` so the reclaiming wrapper can ask "would pruning be +/// enough?" BEFORE it destroys anything. Mirrors that function's arithmetic exactly; the one +/// deliberate difference is that an unresolvable mount reports `None` (fail open) rather than a +/// deficit, so a missing disk reading can never be the reason we delete the only archive. +async fn export_space_deficit( + pool: &PgPool, + event_id: Uuid, + export_path: &Path, +) -> Result> { + let media_bytes = estimate_export_bytes(pool, event_id).await?; + let (armed,): (i64,) = sqlx::query_as( + "SELECT COUNT(*) FROM export_job + WHERE event_id = $1 AND status IN ('pending', 'running')", + ) + .bind(event_id) + .fetch_one(pool) + .await + .context("counting armed export jobs")?; + let required = required_free_bytes(media_bytes, armed) + .saturating_add(crate::handlers::upload::DISK_RESERVE_BYTES as u64); + + let Some(free) = crate::services::disk::free_bytes(export_path) else { + return Ok(None); + }; + Ok((free < required).then(|| required - free)) +} + +/// Sum what `prune_superseded_archives` WOULD reclaim, without deleting anything. +/// +/// Deliberately shares `is_superseded_archive` and `protected_files` with the real prune, so the +/// estimate cannot drift from what actually gets removed. An unreadable directory reports 0, +/// which makes the caller refuse to prune — the safe direction. +async fn reclaimable_superseded_bytes( + pool: &PgPool, + exports_dir: &Path, + prefix: &str, + event_id: Uuid, + keep_seq: i64, +) -> u64 { + let protected = protected_files(pool, event_id).await; + let final_prefix = format!("{prefix}.{event_id}."); + let Ok(mut rd) = tokio::fs::read_dir(exports_dir).await else { + return 0; + }; + let mut total = 0u64; + while let Ok(Some(entry)) = rd.next_entry().await { + let name = entry.file_name(); + let name = name.to_string_lossy(); + if !is_superseded_archive(&name, &final_prefix, keep_seq, &protected) { + continue; + } + total = total.saturating_add(entry.metadata().await.map(|m| m.len()).unwrap_or(0)); + } + total +} + /// Best-effort removal of stale per-generation export artifacts for one export type. Deletes /// ONLY strictly-older generations (`n < keep_seq`) — never `keep_seq`'s own current file, /// and never a NEWER generation that a concurrent re-release may already be producing (that @@ -1924,6 +2274,69 @@ mod tests { format!("Gallery.{EVT}.") } + #[test] + fn a_burst_of_invalidations_still_starts_a_build_within_one_window() { + // B7's invariant, and the reason the burst start is tracked at all. A per-request delay + // meant every invalidation restarted the wait, so a host moderating faster than one action + // per 20 seconds deferred the build forever — `/export/html` 404s and the UI sits on + // "Wird vorbereitet…" for the rest of the event. + // + // What must hold is that the delay is measured from the FIRST request in the burst, so the + // wait never grows: request #50 of a takedown pass is scheduled no later than request #1 + // was. Coalescing survives (later requests still collapse into one build); starvation does + // not. + let event = Uuid::new_v4(); + clear_regen_burst(event); + + let first = regen_delay_for(event); + assert!( + first <= REGEN_DEBOUNCE, + "the first invalidation of a burst waits at most one debounce window" + ); + + let mut previous = first; + for _ in 0..50 { + let next = regen_delay_for(event); + assert!( + next <= previous, + "a later invalidation must never push the build further out than an earlier one" + ); + previous = next; + } + + // And the burst is a scheduling window, not a permanent state: once a worker has run, + // the next invalidation is a fresh burst that gets the full coalescing delay again. + clear_regen_burst(event); + assert!( + regen_delay_for(event) >= previous, + "clearing the burst starts a fresh debounce window" + ); + clear_regen_burst(event); + } + + #[test] + fn one_events_burst_never_schedules_another_events_rebuild() { + // The map is keyed per event because the delay is a property of THAT event's moderation + // pass. A shared window would let a busy event drag an idle one's keepsake along with it — + // or worse, let an idle event's stale entry start a busy one's build immediately, which is + // the stampede the debounce exists to prevent. + let busy = Uuid::new_v4(); + let quiet = Uuid::new_v4(); + clear_regen_burst(busy); + clear_regen_burst(quiet); + + for _ in 0..10 { + regen_delay_for(busy); + } + clear_regen_burst(busy); + + assert!( + regen_delay_for(quiet) <= REGEN_DEBOUNCE, + "an untouched event gets its own full window" + ); + clear_regen_burst(quiet); + } + #[test] fn a_strictly_older_archive_is_reclaimed() { // The whole point: at the start of a rebuild at epoch 5, generation 4's archive is dead @@ -2094,4 +2507,57 @@ mod tests { // guarding against — the failure mode must be "refuse", never "wrap and allow". assert_eq!(required_free_bytes(u64::MAX, 2), u64::MAX); } + + /// The completeness gate decides whether a partially-readable gallery still ships. Both + /// directions are dangerous: too strict and one live delete costs the guests every photo + /// (a released event cannot be rebuilt without reopening uploads); too loose and a wrong + /// MEDIA_PATH ships an archive that silently misrepresents the whole party. + #[test] + fn a_clean_export_and_a_totally_empty_one_are_unambiguous() { + // Nothing skipped is always fine, at any size. + assert!(check_export_completeness("zip", Uuid::nil(), 0, 0, 0).is_ok()); + assert!(check_export_completeness("zip", Uuid::nil(), 100, 100, 0).is_ok()); + + // Everything skipped is the misconfiguration case — never publish it, at any size. + assert!(check_export_completeness("zip", Uuid::nil(), 100, 0, 100).is_err()); + assert!(check_export_completeness("zip", Uuid::nil(), 3, 0, 3).is_err()); + assert!(check_export_completeness("zip", Uuid::nil(), 1, 0, 1).is_err()); + } + + #[test] + fn a_small_gallery_survives_an_ordinary_live_delete() { + // These are the regressions a pure 10% fraction caused: on a small gallery a single + // guest deleting their own photo mid-build is >10%, so the whole keepsake failed. + assert!(check_export_completeness("zip", Uuid::nil(), 9, 8, 1).is_ok()); + assert!(check_export_completeness("zip", Uuid::nil(), 3, 2, 1).is_ok()); + assert!(check_export_completeness("zip", Uuid::nil(), 4, 2, 2).is_ok()); + // Half a tiny gallery is published too, and logged as materially incomplete. Refusing it + // would hand the guests nothing at all rather than the three photos that DID survive, and + // the rebuild that refusal invites reads the same unreadable files again. + assert!(check_export_completeness("zip", Uuid::nil(), 6, 3, 3).is_ok()); + } + + #[test] + fn a_materially_incomplete_archive_is_still_published_but_shouted_about() { + // This used to assert `is_err()` past the 10% tolerance, and that was the wrong trade. + // The refusal is deterministic across retries — the unreadable files are still unreadable + // when the host taps "Neu erzeugen", and the gallery is already released so the photos + // cannot be collected again. So refusing did not buy a better archive later; it converted + // "90 of 100 photos" into "no keepsake, ever". Past the tolerance we publish and log at + // `error` with the counts. + assert!(check_export_completeness("zip", Uuid::nil(), 100, 90, 10).is_ok()); + assert!(check_export_completeness("zip", Uuid::nil(), 100, 89, 11).is_ok()); + assert!(check_export_completeness("zip", Uuid::nil(), 300, 200, 100).is_ok()); + } + + #[test] + fn an_archive_with_nothing_in_it_is_still_refused() { + // The one fatal case, and the only one the host can actually act on: a wrong MEDIA_PATH + // produced a few-hundred-byte ZIP with zero photos that passed every automated check and + // was advertised as ready. That IS recoverable — fix the path, rebuild — so refusing to + // publish it is the correct answer, unlike the partial case above. + assert!(check_export_completeness("zip", Uuid::nil(), 100, 0, 100).is_err()); + // ...but an event that genuinely has no media is not an error. + assert!(check_export_completeness("zip", Uuid::nil(), 0, 0, 0).is_ok()); + } } diff --git a/backend/src/services/maintenance.rs b/backend/src/services/maintenance.rs index 30f3d35..e694ab8 100644 --- a/backend/src/services/maintenance.rs +++ b/backend/src/services/maintenance.rs @@ -448,6 +448,12 @@ async fn sweep_orphan_originals(pool: &PgPool, media_path: &std::path::Path) { } // Too young to judge: an upload committing RIGHT NOW is indistinguishable from an // orphan, because the rename precedes the commit. + // + // This sweep is also the backstop for the one case the upload handler's drop guard + // deliberately leaks: a client disconnect while `tx.commit()` is in flight disarms + // the guard first (so a COMMIT that Postgres applied anyway keeps its file), which + // means a COMMIT that did NOT apply leaves a final-named file with no row. The + // `NOT EXISTS` check below is what reclaims it. See `upload.rs`, the disarm site. let recent = meta .modified() .ok()