From 8dcc3a7a98876adba1d776cf2845f89cb1ad75ab Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Mon, 17 Aug 2026 17:53:56 +0200 Subject: [PATCH 1/3] fix(upload): a raised size limit destroyed videos instead of refusing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MAX_UPLOAD_BYTES` (576 MiB) is the router's `DefaultBodyLimit` on the upload route. Its coupling to the admin-tunable `max_image_size_mb` / `max_video_size_mb` was enforced by a COMMENT — "if an admin raises max_video_size_mb above this, bump MAX_UPLOAD_BYTES" — while `patch_config` accepted 1024 and 10240 respectively and the dashboard rendered "Max. Videogröße (MB)" as a bare number field with no stated ceiling. Set it to 1000 and every video between 576 MB and the new limit is not refused, it is DESTROYED, and the shape is worse than the size: * The body limit trips MID-UPLOAD, inside `field.chunk()`, so `stream_field_to_file` maps it to `AppError::BadRequest` — a 400, not a 413 carrying the `quota_exceeded` code the client knows how to keep a blob for. * `classifyUploadStatus` puts every non-401/408/429 4xx in the `terminal` bucket, and `isReversibleLock(400, 'bad_request')` is false — so the queue DELETES the blob from IndexedDB and moves the row to `blocked`, which by design offers no retry button. * All of that after the guest pushed 600 MB over cellular, and the message they get names a read failure rather than a limit. "Raise the video limit" is exactly the change a host makes after a guest complains a clip was too big, so this is reachable by an operator doing the obvious thing. The ceiling is now DERIVED from the body limit rather than written down twice (`MAX_CONFIGURABLE_UPLOAD_MB`, 575) and enforced at both ends, because either alone leaves a hole: `patch_config` bounds what can be WRITTEN, and the upload handler clamps what it READS, since a row stored before this bound existed — or edited straight into the `config` table — would sail past the first check. A compile-time assertion pins both directions against the ACTUAL field caps (`MAX_CAPTION_BYTES + MAX_HASHTAGS_BYTES + MAX_CLIENT_UPLOAD_ID_BYTES` plus framing), so raising `MAX_CAPTION_LENGTH` fails the build rather than silently eating the envelope margin; a lower bound keeps the ceiling clear of the 500 MB `max_video_size_mb` seeded by migration 005. Ordering is now guaranteed: at 575 MiB the handler's own cap trips while the body is ~1 MiB short of axum's, so the clean "Datei ist zu groß" 400 always wins the race against the mid-stream abort. Frontend, both halves of the same rule: * the composer's pre-flight moves from 576 MiB (the raw body limit) to 575 MB, so a guest is rejected locally against the same number the server enforces and never pushes the file to find out; * both size fields gain a hint naming the 575 ceiling, so the operator learns the bound from the form instead of from an error after typing 1000. Co-Authored-By: Claude Opus 5 (1M context) --- backend/src/handlers/admin.rs | 10 ++- backend/src/handlers/upload.rs | 34 ++++++++-- backend/src/main.rs | 88 ++++++++++++++++++++++++- frontend/src/routes/admin/+page.svelte | 21 +++++- frontend/src/routes/upload/+page.svelte | 18 +++-- 5 files changed, 153 insertions(+), 18 deletions(-) diff --git a/backend/src/handlers/admin.rs b/backend/src/handlers/admin.rs index 5faa338..026de3c 100644 --- a/backend/src/handlers/admin.rs +++ b/backend/src/handlers/admin.rs @@ -115,9 +115,15 @@ pub async fn patch_config( // accept but that silently revert to the hardcoded default at read time // (get_usize/get_i64 can't parse negatives/NaN/fractionals). `compression_concurrency` // is intentionally absent — it's read once at boot, so a live edit was a no-op. + // The two size limits are bounded by what the router's `DefaultBodyLimit` can carry, NOT by + // a round number. They used to allow 1024 and 10240 MB against a 576 MiB body cap, so the + // admin dashboard's plain "Max. Videogröße (MB)" field could be set to a value that destroys + // every video above the cap — the limit trips mid-body, surfaces as a 400, and the client + // purges the blob as terminal. See `crate::MAX_CONFIGURABLE_UPLOAD_MB` for the full chain. + const MAX_SIZE_MB: f64 = crate::MAX_CONFIGURABLE_UPLOAD_MB as f64; const NUMERIC_SPECS: &[(&str, bool, f64, f64)] = &[ - ("max_image_size_mb", true, 1.0, 1024.0), - ("max_video_size_mb", true, 1.0, 10240.0), + ("max_image_size_mb", true, 1.0, MAX_SIZE_MB), + ("max_video_size_mb", true, 1.0, MAX_SIZE_MB), ("upload_rate_per_hour", true, 1.0, 100_000.0), ("feed_rate_per_min", true, 1.0, 100_000.0), ("export_rate_per_day", true, 1.0, 100_000.0), diff --git a/backend/src/handlers/upload.rs b/backend/src/handlers/upload.rs index a9798b5..14aea2a 100644 --- a/backend/src/handlers/upload.rs +++ b/backend/src/handlers/upload.rs @@ -25,10 +25,15 @@ const MAX_CAPTION_LENGTH: usize = 2000; /// `MAX_CAPTION_LENGTH` check only ran afterwards, on a string that had already been built. /// 4 bytes per code point is the worst case for UTF-8, so this can never reject a caption the /// character limit would have accepted. -const MAX_CAPTION_BYTES: usize = MAX_CAPTION_LENGTH * 4; +pub(crate) const MAX_CAPTION_BYTES: usize = MAX_CAPTION_LENGTH * 4; /// Byte ceiling for the raw hashtag CSV. Generous next to what the tag caps below allow. -const MAX_HASHTAGS_BYTES: usize = 4 * 1024; +pub(crate) const MAX_HASHTAGS_BYTES: usize = 4 * 1024; + +/// Byte ceiling for the `client_upload_id` field. 64 bytes fits a hyphenated UUID (36) with +/// room to spare; named rather than inline so the multipart-envelope test in `main.rs` can +/// account for every text field this handler will read. +pub(crate) const MAX_CLIENT_UPLOAD_ID_BYTES: usize = 64; /// Hashtags stored per upload. The CSV was never length-checked at all and was split into an /// unbounded `Vec`, then upserted TAG BY TAG inside the commit transaction — which holds a @@ -280,9 +285,24 @@ pub async fn upload( return Err(AppError::UploadsLocked("Uploads sind gesperrt.".into())); } - // Read config limits from DB - let max_image_mb: i64 = config::get_i64(&state.config_cache, "max_image_size_mb", 20).await; - let max_video_mb: i64 = config::get_i64(&state.config_cache, "max_video_size_mb", 500).await; + // Read config limits from DB. + // + // CLAMPED to what the router's `DefaultBodyLimit` can actually carry. `patch_config` now + // refuses a larger value, but that guard only covers values written THROUGH it: a config row + // stored before the bound existed, or edited straight into the table, would otherwise sail + // past it and hand the guest the worst failure in the app — the body limit tripping mid-upload, + // surfacing as a 400, and the client purging the blob as terminal. See + // `crate::MAX_CONFIGURABLE_UPLOAD_MB`. + // + // Clamping (rather than refusing the upload) is right here: the operator's intent was "allow + // bigger files", and the honest answer to an unsatisfiable limit is the largest one that + // works, applied consistently by both the streaming cap and the per-class check below. + let max_image_mb: i64 = config::get_i64(&state.config_cache, "max_image_size_mb", 20) + .await + .min(crate::MAX_CONFIGURABLE_UPLOAD_MB); + let max_video_mb: i64 = config::get_i64(&state.config_cache, "max_video_size_mb", 500) + .await + .min(crate::MAX_CONFIGURABLE_UPLOAD_MB); // The uploaded file is streamed straight to a temp file on disk (never buffered // whole in memory — a 500 MB video used to cost 500 MB of RAM per concurrent @@ -379,8 +399,8 @@ pub async fn upload( // `hashtags` were bounded by the helper for exactly this reason; this field // arrived later (migration 022) and missed it. // - // 64 bytes fits a hyphenated UUID (36) with room to spare. - let raw = read_text_field_bounded(field, 64).await?; + // See `MAX_CLIENT_UPLOAD_ID_BYTES`. + let raw = read_text_field_bounded(field, MAX_CLIENT_UPLOAD_ID_BYTES).await?; // A malformed key is not worth rejecting an upload over — the photo is the // thing the guest cares about. Drop the key and lose only the retry // protection, which is exactly where we were before it existed. diff --git a/backend/src/main.rs b/backend/src/main.rs index 1d4cd74..4f28926 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -19,7 +19,36 @@ use state::AppState; /// Hard HTTP body cap for the upload endpoint (576 MiB). Backstop against /// memory-exhaustion; precise per-class size limits are enforced in the handler. -const MAX_UPLOAD_BYTES: usize = 576 * 1024 * 1024; +pub(crate) const MAX_UPLOAD_BYTES: usize = 576 * 1024 * 1024; + +/// Largest per-file size limit (MB) an operator may configure for `max_image_size_mb` / +/// `max_video_size_mb`, derived from [`MAX_UPLOAD_BYTES`] rather than written down twice. +/// +/// This used to be enforced by a COMMENT — "if an admin raises max_video_size_mb above this, +/// bump MAX_UPLOAD_BYTES" — while `patch_config` happily accepted 10240 and the admin dashboard +/// rendered "Max. Videogröße (MB)" as a bare number field. Set it to 1000 and every video +/// between 576 MB and the new limit is destroyed, in a shape that is much worse than a refusal: +/// +/// * The body limit trips mid-upload, inside `field.chunk()`, so `stream_field_to_file` maps +/// it to `AppError::BadRequest` — a 400, not a 413 with a `quota_exceeded` code. +/// * `classifyUploadStatus` (frontend/src/lib/upload-queue.ts) puts every non-401/408/429 4xx +/// in the `terminal` bucket, and `isReversibleLock(400, 'bad_request')` is false — so the +/// queue DELETES the blob from IndexedDB and moves the row to `blocked`, which by design +/// offers no retry button. +/// * All of that after the guest has already pushed 600 MB over cellular, and the message they +/// get names a read failure rather than a limit. +/// +/// So the ceiling is enforced where the value is SET (`patch_config`) and again where it is READ +/// (`handlers::upload`), because a value stored before this bound existed — or written by hand +/// into the `config` table — would otherwise walk straight past the first check. +/// +/// The subtraction is the multipart envelope: the body carries the file PLUS the boundary +/// framing and the `caption` / `hashtags` / `client_upload_id` fields. 1 MiB is enormously more +/// than those can occupy (see the test below, which pins it against their actual caps) and costs +/// nothing — the alternative is a limit that is satisfiable in theory and off-by-a-header in +/// practice. +pub(crate) const MAX_CONFIGURABLE_UPLOAD_MB: i64 = + (MAX_UPLOAD_BYTES as i64 - 1024 * 1024) / (1024 * 1024); #[tokio::main] async fn main() -> Result<()> { @@ -118,7 +147,9 @@ async fn main() -> Result<()> { // the precise per-class limits from DB config (max_image/video_size_mb); this // layer just stops a multi-GB body from being buffered into memory before that // check runs. Sized generously above the default 500 MB video limit + multipart - // overhead — if an admin raises max_video_size_mb above this, bump MAX_UPLOAD_BYTES. + // overhead. The DB-configured limits can no longer exceed it: both are clamped to + // MAX_CONFIGURABLE_UPLOAD_MB at write time and at read time — see that constant + // for why a comment was not enough. .route( "/api/v1/upload", post(handlers::upload::upload).route_layer(DefaultBodyLimit::max(MAX_UPLOAD_BYTES)), @@ -467,3 +498,56 @@ async fn shutdown_signal() { std::process::exit(0); }); } + +#[cfg(test)] +mod tests { + use super::{MAX_CONFIGURABLE_UPLOAD_MB, MAX_UPLOAD_BYTES}; + use crate::handlers::upload::{ + MAX_CAPTION_BYTES, MAX_CLIENT_UPLOAD_ID_BYTES, MAX_HASHTAGS_BYTES, + }; + + /// Boundary lines, `Content-Disposition` / `Content-Type` headers and CRLFs for the four + /// fields the handler reads. A few hundred bytes in reality; 4 KiB is a deliberately fat + /// allowance so this test asserts the invariant rather than a precise byte count. + const MULTIPART_FRAMING_BYTES: usize = 4 * 1024; + + /// Both bounds on `MAX_CONFIGURABLE_UPLOAD_MB`, asserted at COMPILE time. + /// + /// `const _: () = assert!(...)` rather than a runtime `assert!`, matching the precedent in + /// `handlers::upload` and `services::compression`: every operand is a constant, so a + /// violation is a build failure rather than something that has to be run to be noticed. The + /// cost is that a const panic takes a static message — the reasoning lives here instead. + /// + /// UPPER: a file at the largest configurable limit must still fit inside the body limit the + /// router enforces, envelope included. If it does not, an operator can set a limit the + /// handler accepts and the router then refuses MID-BODY — a 400 that the upload queue + /// classifies as terminal and answers by deleting the guest's only copy of the photo. See + /// `MAX_CONFIGURABLE_UPLOAD_MB`. Written against the field caps rather than a hardcoded + /// number, so raising `MAX_CAPTION_LENGTH` (or adding another text field to the envelope) + /// fails HERE instead of silently eating the margin. + /// + /// LOWER: the ceiling must not be so conservative that it forbids the shipped default. + /// `max_video_size_mb` is seeded at 500 (migration 005), so a bound below that would clamp + /// every video upload on a stock install and reject the stock config through `patch_config`. + #[test] + fn the_configurable_ceiling_is_bounded_at_both_ends() { + const FILE: usize = MAX_CONFIGURABLE_UPLOAD_MB as usize * 1024 * 1024; + const ENVELOPE: usize = MAX_CAPTION_BYTES + + MAX_HASHTAGS_BYTES + + MAX_CLIENT_UPLOAD_ID_BYTES + + MULTIPART_FRAMING_BYTES; + const _: () = { + assert!( + FILE + ENVELOPE <= MAX_UPLOAD_BYTES, + "a file at MAX_CONFIGURABLE_UPLOAD_MB plus its multipart envelope exceeds \ + MAX_UPLOAD_BYTES — an operator could configure a limit that DESTROYS uploads \ + (400 mid-body, blob purged as terminal) instead of refusing them" + ); + assert!( + MAX_CONFIGURABLE_UPLOAD_MB >= 500, + "MAX_CONFIGURABLE_UPLOAD_MB must clear the 500 MB max_video_size_mb default \ + seeded by migration 005, or a stock install clamps every video upload" + ); + }; + } +} diff --git a/frontend/src/routes/admin/+page.svelte b/frontend/src/routes/admin/+page.svelte index 9dc93a8..3c38b82 100644 --- a/frontend/src/routes/admin/+page.svelte +++ b/frontend/src/routes/admin/+page.svelte @@ -80,8 +80,25 @@ { title: 'Limits & Größen', fields: [ - { key: 'max_image_size_mb', label: 'Max. Bildgröße (MB)', kind: 'number' }, - { key: 'max_video_size_mb', label: 'Max. Videogröße (MB)', kind: 'number' } + // The 575 MB ceiling is not arbitrary and it is not a policy choice: it is what the + // upload route's HTTP body limit can carry (backend MAX_CONFIGURABLE_UPLOAD_MB, + // derived from MAX_UPLOAD_BYTES minus the multipart envelope). Above it the body + // limit trips MID-UPLOAD, which the upload queue reads as a terminal 4xx and + // answers by deleting the guest's photo — so the backend refuses the value, and + // these hints exist so the operator learns the bound from the form rather than + // from an error after typing 1000. + { + key: 'max_image_size_mb', + label: 'Max. Bildgröße (MB)', + kind: 'number', + hint: 'Maximal 575 — darüber kann der Server den Upload nicht mehr entgegennehmen.' + }, + { + key: 'max_video_size_mb', + label: 'Max. Videogröße (MB)', + kind: 'number', + hint: 'Maximal 575 — darüber kann der Server den Upload nicht mehr entgegennehmen.' + } // compression_concurrency is set via COMPRESSION_WORKER_CONCURRENCY at // boot, not live — omitted so it isn't a dead no-op control. ] diff --git a/frontend/src/routes/upload/+page.svelte b/frontend/src/routes/upload/+page.svelte index 2240d26..3734326 100644 --- a/frontend/src/routes/upload/+page.svelte +++ b/frontend/src/routes/upload/+page.svelte @@ -26,11 +26,19 @@ const MAX_CAPTION_LENGTH = 2000; - // Mirrors MAX_UPLOAD_BYTES in backend/src/main.rs — the axum body limit, which is a - // BOOT CONSTANT rather than an admin-tunable value, so checking it here cannot drift - // out of sync with the dashboard the way max_image_size_mb / max_video_size_mb would. - // Anything above this is refused by the server no matter how the event is configured. - const HARD_MAX_UPLOAD_BYTES = 576 * 1024 * 1024; + // Mirrors MAX_CONFIGURABLE_UPLOAD_MB in backend/src/main.rs — the largest per-file limit an + // operator can configure, itself derived from the axum body limit. Both are BOOT CONSTANTS + // rather than admin-tunable values, so checking it here cannot drift out of sync with the + // dashboard the way max_image_size_mb / max_video_size_mb would. Anything above this is + // refused by the server no matter how the event is configured. + // + // 575 MB, not the raw 576 MiB body limit this used to mirror. The gap is the multipart + // envelope, and it is the difference between two failure shapes: at or below the file + // ceiling the UPLOAD HANDLER rejects with "Datei ist zu groß. Maximum: 575 MB." — a clean, + // self-explaining 400 — whereas past the body limit axum aborts the request MID-STREAM and + // the handler answers with a read error instead. Rejecting here at the lower of the two + // means a guest never reaches the confusing one, and never pushes 575 MB to find out. + const HARD_MAX_UPLOAD_BYTES = 575 * 1024 * 1024; /** * Reject files the server is certain to refuse, BEFORE any bytes leave the phone. From e645d78a6e0838d12d573dd90693c5b4e737e311 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Mon, 17 Aug 2026 17:54:11 +0200 Subject: [PATCH 2/3] fix(disk): the free-space cache stopped caching once a second volume was asked about MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DiskCache` held ONE slot carrying its own key, so a lookup for a different path was a miss that OVERWROTE the previous reading. The app asks about two paths that are distinct mounts in production: `MEDIA_PATH=/media` and `EXPORT_PATH=/exports`. * the upload gate and the per-user quota ask about the media volume on EVERY photo (`handlers::upload`); * `host::get_event_status` asks about the exports volume on every host dashboard load. So while a host had the dashboard open the two evicted each other and the hit rate collapsed to zero, putting an uncached `sysinfo::Disks::new_with_refreshed_list()` — a synchronous scan of every mount, on the async runtime — back on the busiest write path in the app. That is precisely the cost `handlers::upload`'s own comment says this cache exists to avoid, on the 2-vCPU box it says it matters on, and it degrades hardest exactly when a host is watching the disk because uploads are failing. Correctness was never affected — the slot carried its key, so it never returned the WRONG filesystem's numbers. It missed and re-measured instead, which is the quieter failure and the one that cost. Now one entry per path. The key space cannot grow: both paths come from `AppConfig`, never from request input. `invalidate` clears ALL volumes, since the e2e TRUNCATE moves free space on every one of them and a survivor would let the next test compute against the previous test's disk. Three tests. The one that matters most is `a_stale_entry_is_refreshed_without_ deadlocking`: the hit check holds a READ guard and an expired entry falls through to a WRITE guard on the same non-reentrant `RwLock`, so whether they overlap depends on when the `if let` scrutinee's temporary is dropped — edition 2024 drops it before the fall-through, the 2021 rules did not. Too subtle to leave to a reading of the edition, so it is pinned; it HANGS rather than fails if that ever regresses. Co-Authored-By: Claude Opus 5 (1M context) --- backend/src/services/disk.rs | 139 ++++++++++++++++++++++++++++++----- 1 file changed, 122 insertions(+), 17 deletions(-) diff --git a/backend/src/services/disk.rs b/backend/src/services/disk.rs index 2afc197..9cf0bcf 100644 --- a/backend/src/services/disk.rs +++ b/backend/src/services/disk.rs @@ -7,6 +7,7 @@ //! barely moves. [`DiskCache`] refreshes it at most once per [`TTL`] and serves the //! rest from memory. +use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::{Arc, RwLock}; use std::time::{Duration, Instant}; @@ -20,52 +21,72 @@ pub struct DiskInfo { pub free: u64, } -/// Cheap-to-clone cache of the media filesystem's total/free bytes. Lives in -/// `AppState`. +/// Cheap-to-clone cache of a filesystem's total/free bytes, keyed by the path asked about. +/// Lives in `AppState`. +/// +/// ONE ENTRY PER PATH, not one entry total. This held a single slot carrying its own key, so a +/// lookup for a different path was a miss that OVERWROTE the previous reading — and the app asks +/// about two paths that are distinct mounts in production (`MEDIA_PATH=/media` and +/// `EXPORT_PATH=/exports`, docker-compose.yml). The upload gate and the per-user quota ask about +/// the media volume on every single photo; `host::get_event_status` asks about the exports volume +/// on every host-dashboard load. So while a host had the dashboard open the two evicted each +/// other and the cache hit rate collapsed to zero, putting an uncached +/// `sysinfo::Disks::new_with_refreshed_list()` — a synchronous scan of every mount, on the async +/// runtime — back on the busiest write path in the app. That is precisely the cost the comment in +/// `handlers::upload` says this cache exists to avoid, on the 2-vCPU box it says it matters on. +/// +/// The key space cannot grow: both paths come from `AppConfig`, never from request input, so this +/// map holds at most as many entries as there are configured volumes. #[derive(Clone)] pub struct DiskCache { - inner: Arc>>, + inner: Arc>>, } impl DiskCache { pub fn new() -> Self { Self { - inner: Arc::new(RwLock::new(None)), + inner: Arc::new(RwLock::new(HashMap::new())), } } - /// Drop the cached reading so the next `snapshot()` re-measures the filesystem. + /// Drop every cached reading so the next `snapshot()` re-measures the filesystem. /// /// Used by the e2e TRUNCATE endpoint. Truncating deletes every uploaded file, which materially /// changes free space — but the cached reading survives for up to the TTL, so the next test can /// compute a quota from the PREVIOUS test's disk. That matters now that the quota tests steer /// the per-user limit off `free_disk_bytes`: a stale reading makes the limit wrong and the test /// flaky, for reasons that have nothing to do with the code under test. + /// + /// Clears ALL paths, not just the media one: truncation moves free space on every volume the + /// app measures, and the export volume feeds the host dashboard's low-disk banner. pub fn invalidate(&self) { - *self.inner.write().unwrap() = None; + self.inner.write().unwrap().clear(); } - /// Cached `(total, free)` bytes for the filesystem that holds `media_path`. + /// Cached `(total, free)` bytes for the filesystem backing `path`. /// /// Returns `None` when the mount can't be resolved — callers MUST treat that as /// "unknown", never "zero free". (The quota path in particular fails *open* on /// `None`: enforcing a 0-byte limit would lock every user out of uploading.) - /// Cached free-space reading for `path`. /// - /// The cache is keyed BY PATH. It used to hold a single slot and ignore its argument on a hit, - /// so it would happily return the media volume's numbers for any other path within the TTL. - /// That was invisible only because every caller happened to pass `media_path` — the first - /// caller to ask about a different volume (e.g. the exports volume, which is a separate mount) - /// would have silently got the wrong filesystem's free space. + /// The cache is keyed BY PATH, and it RETAINS every path it has been asked about — see the + /// struct doc for why holding one slot made the cache stop caching as soon as the app asked + /// about its second volume. + /// + /// (The original single slot also carried its key, so it never returned the WRONG + /// filesystem's numbers. What it did instead was miss and re-measure on every alternation, + /// which is the quieter failure and the one that cost.) pub fn snapshot(&self, path: &Path) -> Option { - if let Some((cached_path, info, at)) = self.inner.read().unwrap().as_ref() - && cached_path == path + if let Some((info, at)) = self.inner.read().unwrap().get(path) && at.elapsed() < TTL { return Some(*info); } let info = read_disk_for_path(path)?; - *self.inner.write().unwrap() = Some((path.to_path_buf(), info, Instant::now())); + self.inner + .write() + .unwrap() + .insert(path.to_path_buf(), (info, Instant::now())); Some(info) } } @@ -129,7 +150,91 @@ fn select_disk(mounts: &[(String, u64, u64)], media_path: &str) -> Option Date: Mon, 17 Aug 2026 17:54:34 +0200 Subject: [PATCH 3/3] docs(release): the rollback section's facts were wrong, and it is the emergency path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §6 and §9 both asserted "latest existing tag is v0.12.0". It is v0.17.4 — twelve tags newer — and HEAD is 82 commits past that. Everything built on that premise was therefore wrong at the one moment nobody has time to check it. **The dangerous half.** §9 argued that a rollback to the previous release could not even get off the ground: "No v0.12.0 image was ever built or pushed either, so the pre-pull would fail with `manifest unknown` before you ever got that far." That presents the registry as a safety net. It is not one. Images for the earlier v0.17.x releases may well be resident — and after §9's own T-2 pre-pull step, on the server — in which case the rollback pulls cleanly, starts, and crash-loops on `VersionMissing` (16 migrations against a database carrying 31) behind a live Caddy. That is the exact permanent outage §9 exists to prevent, reached through the one door it said was closed. Rewritten to say what actually decides: the migration set, checked with `git ls-tree`, never the registry. A successful `docker pull` is not evidence. The heading changes from "no older image you can roll back to" to "no released tag is a VALID rollback target" — older images very probably do exist; what none of them has is a schema the live database can boot against. The twin-tag scheme below it was always the right answer and is unchanged. **The version half.** §6 told you to build and push `v0.17.5`... under the name `v0.13.0`, which has existed as a git tag since April and carries 6 migrations. That image would disagree with the tree of the same name, and any host still pinned to that tag silently swaps builds on its next pull. `.env.example` shipped `EVENTSNAP_VERSION=v0.13.0` too, and README used a third number (`v0.13.1`). All three now say v0.17.5 (+ the `-a` rollback twin). **And the commands now reproduce their own output.** The `git tag` invocation this commit first reached for returned `archive/pre-squash-20260729`, not v0.17.4 — the repo carries a non-release tag that sorts first. Both call sites now use `--list 'v*' --sort=-v:refname`, and all three commands in the §9 evidence block were run and produce exactly what the block claims. Verified: `git tag --list 'v*' --sort=-v:refname | head -1` → v0.17.4; `git ls-tree --name-only v0.17.4 backend/migrations/ | grep -c up.sql` → 16; `git rev-list --count v0.17.4..HEAD` → 82. Co-Authored-By: Claude Opus 5 (1M context) --- .env.example | 13 +++++---- DEPLOYMENT_RUNBOOK.md | 68 +++++++++++++++++++++++++++++-------------- README.md | 4 +-- 3 files changed, 55 insertions(+), 30 deletions(-) diff --git a/.env.example b/.env.example index 5fffd0a..b587787 100644 --- a/.env.example +++ b/.env.example @@ -13,15 +13,16 @@ DOMAIN=my-event.example.com # Always an immutable tag, never `latest`: rollback is `EVENTSNAP_VERSION=` # + `docker compose up -d`, which works offline if that image is still resident locally. # -# ⚠ THIS TAG DOES NOT EXIST YET. The newest git tag is v0.12.0; v0.13.0 is the release you -# cut for the event. Build and push it (plus its identical rollback twin v0.13.0-a) BEFORE +# ⚠ THIS TAG DOES NOT EXIST YET. The newest git tag is v0.17.4; v0.17.5 is the release you +# cut for the event. Build and push it (plus its identical rollback twin v0.17.5-a) BEFORE # the first `docker compose up -d` — see DEPLOYMENT_RUNBOOK.md §6 (build) and §9 (rollback). # Copying this file and starting the stack without that step fails with `manifest unknown`. # -# Do NOT "fix" this by dropping back to v0.12.0: no image was ever built for it, and a -# 6-migration tree booting against a 31-migration database returns VersionMissing and -# crash-loops forever behind a live Caddy. §9 covers this in full. -EVENTSNAP_VERSION=v0.13.0 +# Do NOT "fix" this by dropping back to an older release tag. v0.17.4 carries 16 migrations +# against a database that will hold 31, so it returns VersionMissing and crash-loops forever +# behind a live Caddy — and unlike an unbuilt tag, an image for it may well PULL cleanly, so a +# successful `docker pull` is not evidence that the tag is safe. §9 covers this in full. +EVENTSNAP_VERSION=v0.17.5 # ── App server ──────────────────────────────────────────────────────────────── APP_PORT=3000 diff --git a/DEPLOYMENT_RUNBOOK.md b/DEPLOYMENT_RUNBOOK.md index 666e19f..851b935 100644 --- a/DEPLOYMENT_RUNBOOK.md +++ b/DEPLOYMENT_RUNBOOK.md @@ -180,7 +180,7 @@ EVENT_NAME=<...> EVENT_SLUG=<...> # ── Image version (NEW — drives the image: tags in docker-compose.yml) ──── -EVENTSNAP_VERSION=v0.13.0 +EVENTSNAP_VERSION=v0.17.5 # ── Secrets — ALL of them, before the first `up -d` ─────────────────────── JWT_SECRET= @@ -421,8 +421,13 @@ the host). docker run --rm --platform linux/amd64 alpine uname -m # must print x86_64 cd /Users/fabianhammprivat/Projects/EventSnap -VERSION=v0.13.0 # latest existing tag is v0.12.0 — see §9 before reusing it -ROLLBACK=v0.13.0-a # the SAME source, tagged twice; §9 explains why +# Newest EXISTING tag is v0.17.4 (16 migrations); HEAD has 31, so v0.17.5 is the release you cut +# for the event. Confirm before building — `git tag --list 'v*' --sort=-v:refname | head -1`, and +# keep the `--list 'v*'` filter: an unfiltered listing puts the archive/ tag first. Never reuse an +# existing tag: the image name would then disagree with the tree of the same name, and any host +# still pinned to that tag silently swaps builds on its next pull. +VERSION=v0.17.5 +ROLLBACK=v0.17.5-a # the SAME source, tagged twice; §9 explains why SHA=$(git rev-parse --short HEAD) docker buildx create --name eventsnap --use 2>/dev/null || docker buildx use eventsnap @@ -572,25 +577,44 @@ Then, from a phone on cellular (not the office wifi): ## 9. Rollback -### There is no older image you can roll back to. Build the rollback target yourself. +### No released tag is a valid rollback target. Build the rollback target yourself. + + + Read this before the event, not during it. The obvious move — drop `EVENTSNAP_VERSION` back to the previous released tag — **takes the app down permanently** and looks like a crash loop with no explanation: ``` -$ git ls-tree --name-only v0.12.0 backend/migrations/ | wc -l -12 # 6 migrations. HEAD has 31. -$ git rev-list --count v0.12.0..HEAD -217 +# `--list 'v*'` is load-bearing: the repo also carries non-release tags +# (archive/pre-squash-20260729), and an unfiltered listing puts one of them first. +$ git tag --list 'v*' --sort=-v:refname | head -1 +v0.17.4 +$ git ls-tree --name-only v0.17.4 backend/migrations/ | grep -c up.sql +16 # 16 migrations. HEAD has 31. +$ git rev-list --count v0.17.4..HEAD +82 ``` -`db.rs` runs `sqlx::migrate!()` with no `set_ignore_missing`, so an image built from a 6-migration -tree, booting against a database that already carries versions 007–031, returns `VersionMissing`. +`db.rs` runs `sqlx::migrate!()` with no `set_ignore_missing`, so an image built from a 16-migration +tree, booting against a database that already carries versions 017–031, returns `VersionMissing`. `create_pool` errors, `main` exits 1, and `restart: unless-stopped` restarts it forever — with Caddy still routing traffic to it. (`014_export_epoch.up.sql` documents this failure mode; §0 restates it.) -No `v0.12.0` image was ever built or pushed either, so the pre-pull would fail with -`manifest unknown` before you ever got that far. + +**Do not treat a successful `docker pull` as evidence that a tag is safe.** This section used to +claim the previous release had never been built or pushed, so "the pre-pull would fail with +`manifest unknown` before you ever got that far" — i.e. that the registry itself would stop you. +Do not rely on that. Images for the earlier v0.17.x releases may well be resident in the registry +(and, after a §9 pre-pull, on the server), in which case the rollback pulls cleanly, starts, and +crash-loops on `VersionMissing` — the failure this section exists to prevent, reached through the +one door it used to say was closed. The migration set is the only thing that decides whether a tag +is safe, and it is checked by `git ls-tree`, never by the registry. + +Every existing tag is older than HEAD's migration set, so **no released tag is a valid rollback +target.** That is what the twin-tag scheme below is for. **So: at build time, tag the SAME frozen commit twice.** Two identical images, two names. The rollback then swaps to a binary that is bit-for-bit what you tested and carries the identical @@ -598,8 +622,8 @@ migration set, which makes it a genuine no-op rather than a gamble: ```bash # In §6, push both tags from the one build: -VERSION=v0.13.0 -ROLLBACK=v0.13.0-a # same source, different name — the rollback target +VERSION=v0.17.5 +ROLLBACK=v0.17.5-a # same source, different name — the rollback target docker buildx build --platform linux/amd64 \ -t registry.mc02.dev/eventsnap/app:$VERSION \ @@ -611,17 +635,17 @@ docker buildx build --platform linux/amd64 \ **Rolling back — ~30 seconds, no network:** ```bash -sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.13.0-a/' .env +sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.17.5-a/' .env docker compose up -d app frontend ``` This only works offline if both are already resident. **Pre-pull all four at T‑2:** ```bash -docker pull registry.mc02.dev/eventsnap/app:v0.13.0 -docker pull registry.mc02.dev/eventsnap/frontend:v0.13.0 -docker pull registry.mc02.dev/eventsnap/app:v0.13.0-a -docker pull registry.mc02.dev/eventsnap/frontend:v0.13.0-a +docker pull registry.mc02.dev/eventsnap/app:v0.17.5 +docker pull registry.mc02.dev/eventsnap/frontend:v0.17.5 +docker pull registry.mc02.dev/eventsnap/app:v0.17.5-a +docker pull registry.mc02.dev/eventsnap/frontend:v0.17.5-a docker image ls | grep eventsnap # confirm all four ``` @@ -668,8 +692,8 @@ harness — `backend/scripts/rehearse-014.sh`. Run it once against a real dump b **Registry-down transport fallback** (layers are already compressed — do not add gzip): ```bash -docker save registry.mc02.dev/eventsnap/app:v0.13.0 \ - registry.mc02.dev/eventsnap/frontend:v0.13.0 | ssh root@SERVER 'docker load' +docker save registry.mc02.dev/eventsnap/app:v0.17.5 \ + registry.mc02.dev/eventsnap/frontend:v0.17.5 | ssh root@SERVER 'docker load' ``` --- @@ -943,7 +967,7 @@ docker compose restart app # Roll back to the identically-built sibling image — see §9 for what this can and cannot fix. # Do NOT substitute an older release tag here; it will crash-loop on the migration set. -sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.13.0-a/' .env && docker compose up -d app frontend +sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.17.5-a/' .env && docker compose up -d app frontend # Disk check df -h /var/lib/docker diff --git a/README.md b/README.md index b065779..adb8864 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,7 @@ Caddy automatically obtains a Let's Encrypt certificate on first start. The app # ── On your workstation: build and push the new tag ─────────────────────────── # Push the rollback twin at the same time, from the same source — see # DEPLOYMENT_RUNBOOK.md §9 for why an identical second tag is the rollback target. -VERSION=v0.13.1 +VERSION=v0.17.5 docker buildx build --platform linux/amd64 \ -t registry.mc02.dev/eventsnap/app:$VERSION \ -t registry.mc02.dev/eventsnap/app:$VERSION-a --push ./backend @@ -179,7 +179,7 @@ cd /path/to/eventsnap git pull # 3. Point the stack at the new tag. -sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.13.1/' .env +sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.17.5/' .env # 4. Pull explicitly, BEFORE restarting. A failure here (bad tag, registry down) leaves the # running stack untouched; letting `up -d` discover it takes the app down first.