7 Commits

Author	SHA1	Message	Date
MechaCat02	ef6d3a077a	fix: close what nine adversarial reviews found, most of it mine ... Nine focused reviews (export state machine, upload path, auth/abuse, client queue, guest UI, database, deploy/ops, regression hunt, test honesty). Every finding below was re-verified against the code before being acted on; several plausible-sounding ones were checked and rejected. ## Data loss and denial of service **One request could OOM-kill the app container.** `client_upload_id` was read with `Field::text()` — axum builds its multipart reader with no SizeLimit, so the only bound was the route's 576 MiB body limit, then decoded into a second full String. `caption` and `hashtags` go through `read_text_field_bounded` for exactly this reason; this field arrived later and missed it. Any guest, one request, and every SSE stream drops and every in-flight temp file is stranded. **Nothing bounded concurrent upload bodies.** The headroom gate can only refuse to COMMIT — the body is already streamed to a temp file by the time it runs, and neither axum, the tower stack nor Caddy limits how many stream at once. ~100 guests tapping "upload all" after the ceremony puts 10-20 GB of .tmp on a 40 GB volume, invisible to the gate, eating the reserve that keeps Postgres able to write WAL. New `UploadAdmission` budgets bytes (not requests, so one video and two hundred photos coexist) via a permit that releases on drop, so every exit path returns it. **The export decode bypassed the memory permit the compression path takes.** Same class of work — decode + resize every image in the gallery — in a bare spawn_blocking. A release fired while the last photos were still compressing put both in the same 1 GiB cgroup; the OOM kill marks the export failed and `recover_exports` re-spawns it into the same conditions on the next boot. The permit is now process-wide in `imaging`, because the constraint it expresses is the container's memory, not one worker's. **`MediaTotalCache` cached its own failure as 0.** For the whole TTL the gate then saw an empty event and collapsed to the flat reserve — the behaviour the two-halves design replaced — with no log line. And the trigger correlates with the danger: with max_connections 10 the query fails exactly during a burst. Now falls back to the last good reading and says so. **V8's heap ceiling sat above the frontend container's entire budget** (measured: 259 MB inside a 256M limit), so GC could never intervene and the only backpressure was SIGKILL under an arrival burst. ## Guest-visible **The feed stopped being newest-first after the first reconcile.** It fetches whole 100-item server pages while `uploads` grows in 20s, so everything in the gap was absent from `present`, classified as new, and prepended — ~80 photos from earlier in the evening above the newest ones. It also stalled infinite scroll, since the cursor still pointed at item 20 and the observer only re-fires on a change. The union is now sorted on the server's own (created_at, id) key, which additionally places an SSE arrival correctly. **A stale `loadMoreError` outlived every refresh and filter change**, leaving a false error above a button that returns immediately on `!nextCursor`. **A failed derivative toasted "Ein Upload konnte nicht verarbeitet werden."** for a photo sitting right there on screen — the handler still assumed 1d9fb11's pre-fix behaviour (row deleted, quota refunded, card evicted), none of which is true any more. It was the last surviving route for the "your photo is gone" signal that fix set out to remove. ## Enforcement that existed only in comments `recover_name_rate_per_15min` is clamped at the point of use: the ordering `3 x ceiling <= PIN_LOCK_THRESHOLD` is the whole control against one source locking any guest whose name is on the feed, it was asserted in a comment, and `patch_config` accepted 1..100_000. The test pinned the default constant rather than the enforced bound; it now pins the bound. ## Tests that could not fail - The gate test asserted only its own premise (`500MB x 100 > 35GB`) and never touched the gate. It now checks both controls against the same state and requires them to disagree in the right direction. - `the_banner_always_fires_before_the_upload_gate_closes` reduced to `G < G + G/4` — true for any margin, including zero, so it could not detect the banner moving to exactly the gate. It now pins the gap. - `disk_is_low`'s `free < LOW_DISK_FLOOR_BYTES` clause was unreachable (warn_at is always >= 12.5 GB against a 10 GB floor). Two tests were named after it and neither could fail if it were deleted. Clause and constant removed. - The suspension test I added last commit hard-coded the credit cap instead of importing it, so changing STALL_TIMEOUT_MS would leave it passing against a system that no longer exists. Now imports MAX_SUSPEND_CREDIT_MS. ## Stale comments corrected The prune doc still argued at length for the pre-build ordering that 1d9fb11 reversed — a reader trusting it would reopen the blocker 0506369 fixed. DISK_RESERVE_BYTES claimed to equal the banner threshold that 0506369 deliberately offset by 25%. And host.rs kept its own duplicate 10 GB literal instead of importing the constant. 154/154 backend, 59/59 vitest, clippy clean, svelte-check 0 errors, eslint clean, both builds, compose + caddy validate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-09 14:51:58 +02:00
MechaCat02	61119be817	Merge branch 'fix/deploy-unattended-blockers' into main ... Two independent lines of production hardening diverged at 7d0334b and attacked overlapping problems. Neither was a superset, so this is a merge of substance rather than a fast-forward: every conflict was resolved on the merits, and the losing side's intent was re-checked against the winner rather than assumed. MIGRATIONS. The branch's 021/022/023 collided with main's already-DEPLOYED 021_hashtag_counts_respect_bans and 022_client_upload_idempotency. Renumbered to 023/024/025 in a prior commit — main's versions are applied in production, so their version numbers are immutable and the branch's had to move. Verified by running the full sqlx::test suite, which applies the whole chain from scratch. RESOLVED IN MAIN'S FAVOUR (the branch would have regressed these): * upload-queue.ts wholesale — the branch's copy has ZERO client_upload_id references, so taking it would have silently destroyed end-to-end upload idempotency, the one thing standing between a lost response and a duplicate photo charged twice against the guest's quota. * maintenance.rs supervisor — the branch replaced it with a bare tokio::spawn, where one panic silently stops session pruning, media reclaim, the temp sweep and both HashMap prunes, permanently and with no log line. * The decode-budget probe on spawn_blocking, not inline on the async runtime. * feed/+page.svelte's 8s debounce + jitter + max-wait + hidden-tab deferral, against the branch's naive 800ms — at 100 guests the branch's version walks straight into the per-user feed rate limit. * db.rs pool tuning, /uploaders, and the docker-compose deployment story. * ONE /health, still DB-backed. The branch's split (dependency-free liveness + DB-backed readiness) is defensible, but a constant-"ok" /health is the exact defect faea555 fixed and verified live, its motive (Caddy's boot gate) is already covered by app depends_on db: service_healthy, and the two handlers were the same SELECT 1 under two names. TAKEN FROM THE BRANCH: * The large-PNG OOM guard and its bounded-retry counter (023). Together these turn a single upload that can OOM-kill a 1G container into a bounded failure instead of an infinite restart loop under `restart: unless-stopped`. * 024_feed_scalar_counts — the feed no longer aggregates the whole event per page. Pure SQL; column names, order and types are unchanged by design. * The admin-lockout fix: look the admin up BY ROLE, never by name. 025 also frees any guest already squatting on a reserved name. * PIN lockout tier ordering, bounded caption/hashtag reads, SSE ticket caps, PoolTimedOut -> 503 + Retry-After, and the ffmpeg stderr drain. * backfill_video_posters, which main lacked entirely. * TempFileGuard, plus sweep_orphan_originals wired into main's SUPERVISED loop (not the branch's bare one) — it reclaims final-named originals whose commit never happened, a class main's .tmp-only sweep structurally cannot see. * shouldAbortForStall, hand-ported into main's upload-queue.ts since that file was resolved to main. Widens the watchdog at loadend instead of disarming it, bounding a half-open socket at 2 minutes rather than handing the window to xhr.timeout (5-60 min) with the whole queue's `processing` latch held. ALSO: RUST_LOG and EXPORT_PATH pinned in compose. The code fallback was `debug` (a line per request, all night) and EXPORT_PATH was the one path with a mount-shaped default that nothing validated. Verified: cargo check --all-targets, cargo clippy (clean), 144/144 backend tests against a live Postgres including upload_idempotency and upload_concurrency, 51/51 vitest, svelte-check 0 errors, eslint clean, vite build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-08 21:16:31 +02:00
Fabian Hamm (Privat)	2f952494c2	fix(media): stop a poster-frame failure from deleting the guest's video ... Reproduced live, by accident, while smoke-testing on a machine with no ffmpeg: the clip uploaded fine, returned 201, and roughly six seconds later had `deleted_at` set and was gone from the feed. The `Ok(None)` "this clip yields no frame" case was already handled — that fix landed when sub-second clips were being destroyed. But the `?` on the call itself still routed every OTHER failure into the same give-up path, which soft-deletes: ffmpeg missing from the image, ffmpeg hanging on a truncated `.mov` and tripping the timeout, an ENOSPC on `thumbnails/`, or a DB blip in `set_thumbnail_path`. None of those says anything about the video, and `get_original` serves the file byte-for-byte, so a post that merely lacks a poster is fully watchable. No failure in the video branch may fail the upload. iPhone `.mov` is exactly the input most likely to trip it, and a wedding clip is not retakeable. ENOSPC gets its own classifier. It was the one failure the retry loop actively made worse: a disk does not drain during six seconds of backoff, so all three attempts failed identically while holding a compression permit that photos were queued behind — and the give-up path then refunded the quota and soft-deleted the row while deliberately KEEPING the original. That freed nothing, removed the photo seconds after a 201, and handed the guest the allowance to upload it again into the same full disk. Now: no retry, no refund, no delete. The row stays live and the photo is served from its original, and `backfill_stale_derivatives` regenerates the derivatives on the next start once there is room. `is_storage_full_error` has to look inside `ImageError::IoError` as well as at bare io errors, because `image` wraps rather than sources it and a plain chain walk would miss every derivative-write failure. FFMPEG_TIMEOUT drops 120s -> 45s. It was never a budget for honest work — a poster from a phone clip takes well under a second, and `-ss` before `-i` means even a 500 MB file seeks rather than scans. It is the ceiling on how long a pathological input holds a permit that guests' photos are waiting behind, so it should be as tight as it can be without cutting off real work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-03 18:34:23 +02:00
MechaCat02	a4bac03628	fix(compression): stop a single large PNG from OOM-killing the container ... An 8000x8000 RGBA PNG passes admission — 256,000,000 bytes is just under the 256 MiB max_alloc, and smooth content is under 3 MB on disk, far below any size cap. Processing it peaked at ~1250 MiB inside a 1 GiB cgroup. Measured, not argued: the new (ignored) test builds exactly that image and reads VmHWM around the pipeline, resetting the watermark via /proc/self/clear_refs so the number covers only the code under test. Three independent causes, all of which had to go: 1. The decode outlived everything. `resize` takes &self, and the no-downscale arm bound `img` into `display`, so the ~244 MiB buffer was still alive when oxipng ran — and oxipng decodes the PNG *again*, holding a full-size buffer per filter trial. The decode now lives in a block that yields the display derivative; the else arm moves `img` out, which is what makes "the block's value is the only survivor" true in both arms. 2. oxipng was unbounded in every dimension: preset 2 with timeout: None, and the default features pull in rayon, which evaluates filter trials concurrently with a full-size buffer each and has no Options knob to cap it. Now gated at 8 MP, given a 20 s timeout, and built with default-features = false so oxipng's own sequential shim is used. "filetime" is kept — without it preserve_attrs silently no-ops. Dropping "binary" also stops compiling clap/glob/env_logger (a CLI's deps) into the server image. 3. Even with those fixed it still measured 516 MiB, and compression_concurrency defaults to 2 — so two guests uploading big photos at once was another OOM, 1032 MiB against a 1 GiB limit. The cost is dominated by image's Lanczos3 resize, which accumulates in f32: the intermediate is new_width * old_height * 16 bytes, i.e. 262 MiB for this image — larger than the decode itself, and invisible to max_alloc. Two changes: the preview now derives from the 2048px display instead of re-resizing the original (one full-size pass, not two), and a job whose header-estimated peak exceeds 150 MiB takes an exclusive permit so two giants can never overlap. Ordinary photos (a 12 MP JPEG estimates ~50 MiB) never touch that permit, so throughput is unchanged for everything except the case that must not run in parallel. Chaining 8000 -> 2048 -> 800 for the preview is not a quality trade: a staged Lanczos3 downscale is standard for large ratios and is visually indistinguishable at 800px. The blocking half is now a free function so its memory behaviour is testable — the fix is a scoping property a future edit could silently undo. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 22:42:03 +02:00
fabi	31faccfdf8	fix(audit-4): refuse undecodable images at admission, stop retrying permanent failures ... Fourth round: reject at the door what the worker can only fail on, and stop retrying errors that cannot succeed — while keeping IoError retryable so ENOSPC still gets another attempt. Squashed from 2 commits, original messages preserved below. ──────── fix(upload): refuse undecodable images at the door, and stop retrying them Two halves of the same complaint: an oversized photo was accepted with a 201 and then silently soft-deleted minutes later, after the worker had burned six seconds of backoff re-reaching a conclusion it could not change. Admission. The compression budget now runs at upload time, against the header only, so a guest is told immediately and told why: "Bild hat zu viele Bildpunkte (ca. 99 Megapixel) und kann nicht verarbeitet werden. Bitte verkleinere es und lade es erneut hoch." instead of watching the photo vanish behind a vague "could not be processed" — which arrived only if they happened to still be on the feed with that card loaded. Nothing is stored, so there is no row to soft-delete and no orphan for the sweep to reclaim. Admission and the worker share ONE function (`decoder_within_budget`), so they cannot drift apart and start disagreeing about what is acceptable — a photo accepted at the door and rejected by the worker would be worse than either behaviour alone. The worker keeps its own check: the backfill decodes files that predate this check, and defence in depth is the whole reason the budget exists. Retries. The loop retried every failure, including ones that are a property of the input. An image over the budget, a corrupt file, an unsupported format: each fails identically on all three attempts, so the only effect was 2s + 4s of sleep and three near-identical warnings before the same outcome. `is_permanent_image_error` classifies the `ImageError` variants that cannot change between attempts — Limits, Unsupported, Decoding — and the loop gives up on those at once. `IoError` is deliberately excluded: an ENOSPC while writing a derivative is exactly the transient case the retry exists for, and misclassifying it would turn a blip back into the data loss round 1 fixed. Measured: retry log lines went from 3 per oversized upload to 0. Tests: unit tests for both sides of the classifier (a Limits error is permanent, a missing file is not) and for admission agreeing with the decoder on accept AND reject. The e2e spec is rewritten for the new contract — 400 with an actionable message, nothing stored, backend alive after a burst of four — plus a mirror asserting an ordinary photo still uploads and processes, since a budget that rejected everything would satisfy the other two. ──────── fix(upload): narrow the admission check to the memory budget only The admission check I just added rejected ANY image the decoder couldn't build — corrupt, truncated, or unsupported, not only over-budget. That broke two adversarial tests, and they were right to break. 07-adversarial/file-upload-attacks pins, deliberately, that acceptance follows the MAGIC BYTES: a payload whose first three bytes are a JPEG header is accepted regardless of what follows, because the security property under test is that the client-declared Content-Type has no influence. Both failing cases upload 1024 bytes of JPEG magic followed by zeros. Rejecting those at admission is a different, broader contract than the one asked for, and rewriting an adversarial test to match new behaviour is precisely the thing that needs justifying rather than doing quietly. So admission now checks only what it was meant to: `exceeds_decode_budget` returns true solely for `ImageError::Limits`. A corrupt file goes to the compression worker exactly as before — which handles it gracefully and, since the retry classifier in the previous commit, no longer burns backoff on it. The resource guard is the part that had to move earlier; nothing else did. Tests: the size agreement between admission and the worker is still asserted in both directions, plus a new one writing a magic-bytes-only stub and asserting admission accepts it WHILE the worker still rejects it — pinning the boundary between the two checks so a future widening fails here rather than in the adversarial suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-29 07:55:56 +02:00
fabi	06ade4e158	fix(audit-3): restore the decode allocation guard, route /health in production ... Third round. The decode allocation guard is a regression from round 1: swapping reader.decode() for into_decoder() silently dropped the max_alloc enforcement while keeping the comment that claimed it held. Squashed from 3 commits, original messages preserved below. ──────── fix(imaging): restore the decode allocation guard I removed in round 1 This is a regression I introduced, not a pre-existing gap. Before 05948d8 the compression worker used `ImageReader::decode()`, which does: let mut decoder = Self::make_decoder(format, self.inner, limits.clone())?; limits.reserve(decoder.total_bytes())?; // enforces max_alloc decoder.set_limits(limits)?; Reading the EXIF orientation tag needs `into_decoder()` instead, and that skips the reserve entirely — the crate's own FIXME concedes `from_decoder` doesn't compensate. Nothing else enforces `max_alloc`: the JPEG decoder's `set_limits` only checks support and dimensions. So the 256 MiB budget has been inert since that commit, and round 2 then propagated the weakened path into export.rs through the shared helper, in a commit whose message claimed the helper "carries" the decompression-bomb cap. It didn't, and the comment saying max_alloc "hard-caps the decode allocation" was simply false. What was left was only the per-axis cap, which permits 12000x12000 — 412 MiB decoded, 824 MiB for the two concurrent decodes the worker runs by default, against a 1 GiB container. Deploy-blocking right now because bumping DERIVATIVES_REV makes the first boot after a deploy re-decode the entire gallery two at a time: an OOM kill there restarts the container, which re-runs the backfill. A boot loop, on the first deploy of these fixes. Re-add the reserve exactly as `decode()` does it. Per the budget decision it stays at 256 MiB (~89 MP for RGB8, above any mainstream phone's real output); two concurrent decodes now peak at 512 MiB. Oversized images take the graceful path from round 1 — original retained, quota refunded, upload-error toast — and fail after the header parse but BEFORE any pixels are read, so they cost a header read rather than an allocation. Measured peak during a concurrent oversized burst: 3.0 MiB. Test parity is the other half, and the reason this was invisible: the e2e app container had NO memory limit while production is capped at 1 GiB, so a decode that would OOM-kill production simply succeeded in CI. Mirror the 1 GiB cap in docker-compose.test.yml. That is the third divergence of this shape, after WebKit missing from CI and /health existing only in Caddyfile.test. Tests: a fixture that is 568 KiB on disk and 283 MiB decoded (11000x9000 = 99 MP, deliberately UNDER the per-axis cap so the axis check cannot be what rejects it). A unit test asserts the refusal — it fails against the old code, which decoded it into an 11000x9000 buffer — with a companion asserting an ordinary photo still decodes AND still gets its orientation applied, so the guard didn't become a blanket refusal. An e2e test uploads it singly and as a concurrent pair, asserting compression lands in 'failed' and the backend is still serving and still processing afterwards. ──────── fix(deploy): route /health in production, and actually apply Caddyfile changes Two defects in the update procedure I wrote last round, both of which make a successful-looking deploy a lie. 1. The documented health check could never pass. `curl -fsS https://DOMAIN/health` 404s against a perfectly healthy production stack. The backend registers /health on its ROOT router, not under /api/v1, and the production Caddyfile proxies only /api/* and /media/* — so /health fell through to the SvelteKit catch-all, which has no such route and returns its 404 page. With -f, curl exits 22 and the `&& echo` never runs. My own gloss ("Anything other than ok means check the logs") then sent the operator chasing a phantom outage. e2e/Caddyfile.test has carried `reverse_proxy /health app:3000` since it was written — precisely because the catch-all would otherwise swallow it. Production never did. Per the fix-the-gap-not-the-doc call, production gets the same line, and /health joins the no-store matcher so a cached response can't report the last known state instead of the current one. Verified by running the production Caddyfile against the real backend: /health -> 200 "ok", Cache-Control: no-store, with /api/v1/event and / unaffected. 2. The sequence never reloaded Caddy, so a Caddyfile-only change was dropped. `--build` only rebuilds services with a `build:` section, and caddy is a pinned upstream image. Compose decides whether to recreate a container from its config hash, which covers the mount SPECIFICATION but not the mounted file's CONTENTS — so a git pull that changes ./Caddyfile produces no delta, Compose reports `Running`, and Caddy serves its old config indefinitely. Exit code 0 throughout. Round 1's iOS download fix (137c4ee) is exactly this shape: Caddyfile plus four e2e files, so 100% of its production effect is in that one file. Following the README to the letter deployed it, showed both image IDs changing, and left iOS downloads broken. Demonstrated rather than assumed — added a probe header to a Caddyfile, ran the old sequence (`up -d --build`): header absent, change silently dropped. Ran the new step 4 (`up -d --force-recreate caddy`): header served. `--force-recreate` rather than `restart` or `caddy reload` because the bind mount is resolved to an inode at container-create time and git pull replaces the file rather than editing in place, so a restart can re-read the stale content — the exact failure I hit in round 1 when `caddy reload` didn't pick up an edit. Also rewrites the "db and caddy are untouched … so data volumes survive" sentence. I wrote it as reassurance; "caddy is untouched" was the bug. ──────── chore: take the Bash(*) permission change back out of the shared settings `.claude/settings.json` is committed and applies to anyone who clones. Fabi's local `allow: ["Bash(*)"]` plus deny list ended up in it, inside f0d69f1 — a commit about the image decode guard, which has nothing to do with permissions. That was my mistake, twice. The file was already modified when I started the round: my `git status --short` check printed "(clean)" from an unconditional `echo` rather than from the status output, so I read a dirty tree as clean. Then `git add -A` swept it into an unrelated commit, and I reported afterwards that I had left it untouched. Neither the check nor the claim was true. Restores the shared file to its previous three narrow entries. The permission setup itself is preserved, moved to `.claude/settings.local.json`, which `.gitignore:34` covers precisely so per-user permissions stay per-user — the existing 442 entries there are kept alongside it. Not rewriting f0d69f1 to erase this: main is unpushed so it would be safe, but a visible correction is worth more than a tidy history, and a rebase across the merge commits carries more risk than the mistake does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-29 07:18:08 +02:00
fabi	b601c062bd	fix(audit-2): video playback, role identity, keepsake EXIF, recover ceiling, WebKit CI ... Second round: the two regressions the first round introduced, the remaining gaps it left, and the CI change that makes the iOS guarantees actually gate a change rather than being asserted. Squashed from 9 commits, original messages preserved below. ──────── docs(deploy): document the update path — `up -d` alone ships nothing The README only ever described a fresh install. There was no update section anywhere, and `--build` appeared nowhere in the docs. That matters because `app` and `frontend` are `build:` services with no published image tag, and Compose has no source-change detection: if an image by that name exists it is reused. So the natural `git pull && docker compose up -d` reports "Container app-1 Running", rebuilds nothing, and exits 0. A deploy that shipped none of the new code is indistinguishable from a successful one — which is how eleven merged fixes can sit in the repo and never reach the box. Verified both halves against a real stack rather than asserting them: with a source change staged, `up -d` left the image ID untouched; `up -d --build` produced a new image ID and a healthy /health. Adds an "Updating an existing deployment" section covering backup-before-migrate, pull, rebuild, health check, and an image-ID comparison to prove a build actually happened. Also spells out the rollback trap: migrations run on boot and are not undone by checking out an older commit, so rolling back code without restoring the snapshot leaves the schema ahead of the binary and the app refusing to start. ──────── fix(video): play the actual video, and answer Range requests Every video in the app was unplayable. Two independent defects, either one sufficient on its own, and nothing in the suite covered either — no test anywhere played media or asserted a `<video>` src. 1. The lightbox handed `<video>` a JPEG. `pickMediaUrl` is mime-agnostic, and compression only ever produces a THUMBNAIL for a video (one `ffmpeg -vframes 1` frame) — no preview, no display. So in the DEFAULT saver mode the element's src resolved to `/api/v1/upload/{id}/thumbnail`, served as `image/jpeg` with `nosniff` so the browser can't even sniff its way out. Chromium reports DEMUXER_ERROR_COULD_NOT_OPEN. Fixed in the lightbox rather than in `pickMediaUrl`: FeedListCard shares that helper and legitimately wants the thumbnail for its `<img>` poster, so a central mime branch would break the feed. This mirrors the rule the diashow already applies ("videos play the original file directly"). Added `preload="none"` so saver-mode guests on cellular still fetch nothing until they press play — there is no smaller video derivative to offer them — plus `playsinline`, without which iOS hijacks playback into fullscreen. 2. `stream_media_file` ignored Range entirely. It took no request headers, so it could not see `Range`; it always returned 200 with the whole body and never sent Accept-Ranges or Content-Range. iOS Safari opens every `<video>` with a `Range: bytes=0-1` probe and abandons the load without a 206 — so video failed on the app's primary platform even in `original` mode, where the src was already correct. Adds single-range support (`bytes=N-`, `bytes=N-M`, `bytes=-S`) with 206 + Content-Range, 416 + `bytes */len` past EOF, and Accept-Ranges advertised on every response. Anything it won't handle — multi-range, non-bytes units, garbage — falls back to a full 200, which RFC 9110 explicitly permits and which is safer than guessing. All four media routes share the helper, so seeking works uniformly. `get_original` now serves `inline` instead of `attachment`. An attachment disposition is hostile to a `<video>` element, and this route is the only source of playable video bytes; it also matches what the UI promises, since the action is labelled "Original anzeigen" — view, not download. `no-store` is deliberately kept so a takedown still revokes access promptly; ranges work fine under it, the client just re-fetches. Tests: 11 unit tests pin the parser (the iOS `bytes=0-1` probe, inclusive ends, suffix ranges, clamping past EOF, 416 vs 200, malformed fallbacks). A new 03-feed/video-playback spec asserts the src is the original and not the thumbnail, that the browser accepts the bytes as media (readyState > 0, no MediaError), that no video bytes are delivered before play, and that Range returns the correct 206 slices and a 416 past EOF — verified on both Chromium and WebKit. The "not downloaded before play" test asserts no *delivered body* rather than no request: WebKit opens a connection for a preload="none" video and immediately aborts it (GET, no Range, status 0, nothing transferred) while Chromium issues nothing at all. The portable guarantee is that no response carrying bytes completes. ──────── fix(auth): bind the role store to the identity, not to the tab The role store I added in the moderation work is a module-level singleton seeded ONCE at import. `goto()` is a client-side navigation, so leaving and re-joining in the same tab re-imports no module and re-runs no onMount — the previous user's role simply stayed resident. Nothing reset it: not join, recover, admin login, "Event verlassen", `clearAuth`, nor the api.ts 401 auto-clear. So a host who left, followed by a guest joining on the same phone, left that guest with `isStaff === true` and a "🚫 Beitrag entfernen" action on other people's photos. The backend 403s the delete, so this was a false affordance rather than a privilege escalation — but `/feed` never fetched `/me/context`, so unlike every other route it never self-corrected either. It survived until a hard reload. The mirror case was equally broken and easier to overlook: a guest who recovered into a host account got NO host affordances. `clearAuth` already had a hook registry for exactly this shape of problem, with a comment explaining it exists to avoid circular imports. Add the missing mirror, `onSetAuth`, fired by both `setAuth` and `setAdminAuth` after the new token is resident, and have the role store register on both sides: clear to null on logout, re-seed from the new token on login. That also gives `syncRoleFromToken` — dead code with zero callers since I introduced it — its intended purpose. Seeding from the claim fixes the reported bug, but the claim is frozen for the token's 30-day life, so a promotion or demotion still wouldn't reach the feed. `/feed` now calls the existing `refreshEventState()` on mount, which fetches `/me/context` and applies both the authoritative role and the lock/release state in one request. The feed is the one route gating a destructive action on the role, so it should not be the only route running on a stale claim. Tests: 04-host/role-identity-reset drives the real flows. The first asserts the host DOES see the action before asserting the newcomer does not — a negative assertion alone would pass against a build that shipped no moderation at all. The second covers the mirror, promoting a guest server-side while their resident token still claims `role: guest`, so a fix that only cleared the role would fail it. ──────── fix(compression): reclaim failed originals instead of leaking them Round 1 stopped the compression worker deleting an upload's original on failure — a transient ENOSPC or a codec panic must never destroy the only copy of a photo a guest cannot retake. But it left `Upload::soft_delete`'s quota refund in place, so the bytes stayed on disk while the uploader was charged nothing for them. That is worse than it first looks. The row is soft-deleted, so the file is invisible and unowned; a guest hitting a reproducible codec failure can accumulate orphans indefinitely at zero personal cost. And `active_uploaders` counts only users with non-deleted uploads, so dropping out of that count RAISES everyone's per-user ceiling — the leak loosens the very quota meant to contain it. Keep the refund: the uploader didn't cause the failure and shouldn't silently lose quota to it. Bound the leak instead, with an hourly sweep alongside the existing session cleanup in `spawn_periodic_tasks`, reclaiming failed originals older than 14 days — comfortably longer than any single event, so an operator investigating a failed upload still has the file. The selection predicate is the entire safety argument, so it is deliberately narrow: `compression_status = 'failed'` AND soft-deleted AND past the window AND `original_path <> ''`. That is exactly the state the give-up path leaves behind, and it cannot reach a live upload, an owner-deleted one, or a failure still inside its recovery window. `original_path` is cleared after a successful reclaim, which makes the sweep idempotent — otherwise a row whose file is already gone is re-selected on every tick forever. The row itself is kept as the audit trail. Tests reproduce the selection verbatim (same pattern as upload_concurrency) and assert it against five near-misses that must survive, both sides of the retention boundary, and the idempotence property. Also fixes two comments in export.rs still claiming "the compression worker hard-deletes an original when its transcode fails" — no longer true, and the defensive handling they justify is now justified by this sweep and by ordinary deletes instead. ──────── fix(export): apply EXIF orientation in the keepsake too Round 1 fixed EXIF orientation in the compression worker, which corrected the live app — feed preview and diashow display. The export worker was missed, and it does not reuse those derivatives: it re-decodes the originals itself with `image::open`, which ignores the orientation tag, then re-encodes to JPEG, which drops the tag — so the viewer has no way to recover it. The damage was oddly shaped, which is exactly why it reads as a viewer bug: Gallery.zip originals correct (byte-copied, EXIF intact) Memories viewer grid thumbnails SIDEWAYS (always) Memories viewer full image >5 MB SIDEWAYS (re-encoded at 2000px) Memories viewer full image ≤5 MB correct (streamed byte-for-byte) So in the keepsake people actually keep, every portrait photo in the grid was on its side, and clicking through silently "fixed" small photos but not large ones. Rather than paste the decoder dance a third time, extract `services::imaging:: decode_oriented` and route both workers through it, so there is exactly one way to turn a file on disk into a DynamicImage. It carries a second invariant that had also drifted: `image::open` applies NO decode limits, so the export path was decoding arbitrary user-supplied images unbounded — the decompression-bomb cap existed only in the compression worker. Both now come as a pair, which is the point of having one function. Not done: switching export to consume the existing `display` derivative. It would fix orientation and drop a redundant full-resolution decode per photo, but it would also replace the pristine ≤5 MB originals in the keepsake with 2048px re-encodes — a real quality regression in the one artefact people keep forever. Test uploads the round-1 fixture (40x20 landscape tagged Orientation=6), runs a real export, pulls the thumbnail out of Memories.zip and asserts it came back portrait — with a sanity check that the source really is stored landscape, so the test can't pass against a pipeline that does nothing. ──────── fix(recover): cap name cycling, and stop bcrypt blocking the runtime Round 1 gave /join a per-IP ceiling and left /recover with only its `recover:{ip}:{name}` bucket. That key is right for the job it was written for — stopping someone who knows a display name (they're listed on the feed) from burning the victim's 3-strike PIN counter and locking them out on repeat. But the name is ATTACKER-CHOSEN, so cycling names mints a fresh 5-attempt bucket every time and the per-IP cost is unbounded. What sits behind that limiter makes it worse than a normal flood: every call runs a cost-12 bcrypt verify, including an UNCONDITIONAL throwaway verify for names that don't exist — added deliberately to close a timing oracle. So an unknown name is the single cheapest way to make the server do ~200ms of hashing. Adds `recover_ip_rate_per_min` (default 30, migration 019), checked BEFORE the per-name bucket so a name generator can't walk past it. 30/min is far above any real recovery attempt while capping a flood. The per-name bucket is untouched and remains the anti-guessing control. The second half matters as much as the first: bcrypt was running inline on the async runtime everywhere. At cost 12 that pins a tokio worker thread for ~200ms, and there is only one per core — so a login flood stalled every other request on the box, including the feed. There was no spawn_blocking anywhere in the auth module, despite SECURITY-BACKLOG claiming bcrypt had been offloaded. Route all of it through `verify_password` / `hash_password` on the blocking pool. That covers /recover, /admin/login, the host PIN reset, and — the one most likely to bite at a real event — the PIN hash minted on every single /join. Saturating the blocking pool degrades logins; saturating the worker threads degrades everything. Tests: cycling distinct names from one IP now hits the ceiling with a Retry-After, and — the assertion that keeps the fix honest — repeated wrong PINs against ONE name are still throttled with the ceiling set generously high, so the ceiling added protection rather than replacing it. ──────── ci(e2e): run WebKit, so the iOS guarantees actually gate a PR The workflow installed only Chromium and ran chromium-desktop + chromium-mobile. iOS Safari is the app's stated primary user — a wedding guest opening a QR link — and WebKit is the only engine in the matrix that reproduces two of its behaviours: - it enforces X-Frame-Options on the hidden download iframe, so a site-wide DENY makes the keepsake download silently do nothing. Blink hands attachments to the download manager before the frame check and never notices. - it abandons a <video> load unless its Range probe gets a 206. Both of those shipped. Adding 06-export to the webkit project in the round-1 fix bought nothing on a PR, because CI never ran that project at all — the regression test written specifically to catch the blocker only ever executed locally. Runs 71 tests (67 pass, 4 skip on the documented IndexedDB-blob harness limitation) in ~1.5 minutes locally, using the exact command added here. ──────── chore(backend): satisfy cargo fmt `checks.yml` runs `cargo fmt --check`, and it has been failing since the round-1 audit fixes: I gated those on `cargo build` and `cargo clippy` but never ran fmt, so three files drifted then and eight more this round. Pure formatting — no behaviour change; clippy stays at zero and all 70 backend tests still pass. Worth noting for next time: clippy passing is not evidence fmt does. ──────── chore: satisfy prettier in frontend and e2e `checks.yml` runs `npm run format:check` for both projects and both were failing. - frontend/src/lib/ui-store.ts is mine, unformatted since the round-1 upload-queue badge fix — the same miss as the rustfmt one: I gated on svelte-check and eslint but never on format:check. - e2e/loadtest/* and e2e/shots.mjs have been unformatted since 7758270 and are unrelated to the audit work. Fixed here because they block the same gate and the fix is mechanical; no behaviour change in either project. Still red and deliberately NOT fixed here: `npm run lint` in the frontend reports `svelte/prefer-svelte-reactivity` on routes/diashow/+page.svelte:208 (a mutable `Set` where the rule wants `SvelteSet`), pre-existing since 5009590. That one is a real reactivity change in code I have no test coverage for, so it belongs in its own change rather than smuggled into a formatting commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-28 21:28:11 +02:00