9759c7c6695f08e8f2407fe12dd4e34db88e6449
117 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
fabi
|
cfc8bd0016 |
test: replace coverage that could not fail with coverage that can
`backend/tests/` follows a house rule of copying production SQL character-for-
character rather than calling `src/`, because the crate is a binary and nothing
in it is importable from an integration test. For pinning behaviour that already
existed that is a defensible trade. Applied to a NEW fix whose only coverage is
the copy, it proves nothing: the fix and its test become two independent
implementations, and deleting the fix leaves the test green.
`audit_names.rs` did exactly that. It never called `audit::record` — it
reimplemented `resolve_names` and the INSERT inside the test file, down to a
hardcoded `.bind("host")`, and then asserted `actor_role == "host"` against its
own literal. That assertion could not fail for any change to the code it named,
and grep confirmed there was no other coverage of the audit-name work anywhere.
Moved into `#[cfg(test)]` inside `services/audit.rs`, where the real function IS
callable. CI already runs `cargo test --all-features` with a live DATABASE_URL,
so `#[sqlx::test]` works there; verified all four run and pass. The role
assertion now compares against `UserRole::as_str()` itself rather than a literal,
so it tracks a rename instead of pretending to, plus an explicit `assert_ne!`
against the Debug spelling.
Also:
- `retry-after-release.spec.ts` filtered the feed on `u.id === original.id` to
prove "no second row was created". A duplicate gets a fresh uuid and could
never match, so the filter yielded exactly 1 whether the gallery held one copy
or five. Counts by uploader now, with the original's identity asserted
separately. (The rest of that spec is sound — its 403 control and replay-id
check both fail if the header fast-path is reverted.)
- `upload_after_release_commits_sees_the_lock_and_is_rejected` claimed the
handler answers `UploadsLocked`. It answers `GalleryReleased` since the check
order was inverted on this branch, and the test asserts no variant at all.
Documented what it actually covers (the locked READ) and where the ordering IS
covered (two e2e specs).
- Two `// SRC:` pointers had drifted ~130 lines into unrelated code, which is how
a hand-copied fixture silently stops matching its original. Now named, not
numbered.
- `emptyOutDir: false` claimed a failed viewer build "leaves the last good
artifact in place". True for the `generateBundle` error, false for the newer
`writeBundle` assertion, which fires after Vite has already written the file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
fabi
|
9bae5d77ed |
fix(join): concurrent first joins no longer 500 on the QR-scan burst
`Event::find_or_create` was check-then-insert against a UNIQUE slug, and its only callers are `/join` and `/admin/login` — both of which run before the row exists, at the single most concurrent moment the app ever sees: the QR code goes up and every phone in the room posts `/join` within the same second. All of them miss the SELECT, all of them INSERT, one wins, and the rest get a bare unique violation surfaced as a 500 on the very first screen of the event. There is no retry on that path and nothing in the UI explains it. In the documented timeline the host's T-5 admin login creates the row first, so the blast radius is small — but it is one `down -v` or one `EVENT_SLUG` edit away from being live on the night. `ON CONFLICT (slug) DO UPDATE SET slug = EXCLUDED.slug` — a deliberate no-op write, because `DO NOTHING` returns no row on conflict and would put the loser back at square one. It touches only `slug`, so `name`, `export_epoch` and the lock/release timestamps are never disturbed by a late arrival; a test pins that. The read fast-path stays, so every join after the first is still a plain SELECT and takes no row lock. Tests live in `src/` rather than `tests/` because the crate is a binary and the function is not importable from an integration test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
fabi
|
20c15c3500 |
fix(backend): three ways the end of the night could go wrong
**1. Releasing the gallery could arm the keepsake with no worker.** `release_gallery` ran `tx.commit()` -> SSE `event-closed` -> `audit::record().await` -> `spawn_export_jobs`. The audit write is two pool round-trips, each able to wait the full 5s acquire timeout, and it runs in the same instant `event-closed` fans out to ~100 phones whose upload queues all hit the API at once. Axum drops the handler future when the client disconnects — the host taps "Freigeben" and pockets the phone. The release has COMMITTED: event closed, uploads locked, epoch bumped, both `export_job` rows pending, and no worker. `/export/*` 404s, the page sits on "Wird vorbereitet…", `recover_exports` only runs at boot, and a second release is refused. Every other regen call site spawns first; `me.rs` says so in a comment. This was the sole violator, and the only path that arms the FIRST build of the keepsake. Spawn moved immediately after the commit. **2. The event could be left with no operator.** `remaining_operators` was an unlocked pool COUNT followed by a separate UPDATE, so `ban_user` and `set_role` raced each other and `DELETE /me`: an admin demotes host B while host A deletes themselves, each check sees the other still present, both commit, and nobody can moderate, release the gallery, or appoint anyone — appointing requires being an operator. The count now runs inside the writing transaction behind the same advisory lock `delete_account` uses, via one shared helper so the key cannot drift between copies. The lock is taken FIRST in all three, and the order is load-bearing: `delete_account` previously took it last, after row locks on `upload` and `event`, while the two new call sites take it before locking those same rows — an ABBA that Postgres would resolve by killing one transaction with a 500. The ordering rule is documented on the helper. **3. The keepsake could become unbuildable the moment uploads stopped.** The upload gate and the export preflight computed the IDENTICAL threshold (`required_free_bytes(media, 2) + DISK_RESERVE_BYTES`), leaving zero margin between them. Once the gate refused its first upload the preflight was already at its own limit, so anything written afterwards decided the keepsake's fate: WAL up to `max_wal_size`, 30 MB x 4 of container logs, and the compression backlog draining at exactly that hour. The release commits before the workers bail, so the failure lands at 01:00 with no second release possible. The gate now demands `UPLOAD_GATE_HEADROOM_BYTES` more than the preflight, costing ~0.5 GB of media ceiling — the trade README already argues for. The dashboard banner mirrors the new threshold so its lead is unchanged, and a new test pins gate-before-preflight at six gallery sizes. Also: the global disk gate fails OPEN when the mount cannot be read, which is deliberate, but did it SILENTLY — no log line at all, while the export preflight warns on the identical condition. Inside a container `/` is an overlay rather than a `/dev` device, so this is reachable, and when it happens the only global disk bound is gone and the box fills until Postgres cannot write WAL. README's sizing table was also arithmetically self-contradictory (it showed ~27 GB free against a ~27.6 GB requirement); recomputed for the new gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
fabi
|
06e0bea0e9 |
fix(export): a refused download mint no longer strands the running transfer
`/export/ticket` mints before charging the daily limit, deliberately: charging first meant a store-capacity 503 — a server-side condition the guest cannot see or cause — still cost one of their three daily downloads, with no refund path. But a mint refused with 429 left its ticket in the store. Download tickets live six hours (they must, so a 1.4 GB transfer can resume with `Range`), and the per-session cap is four tickets OF THE SAME KIND. So: the transfer starts on ticket A -> the bar looks stuck on venue wifi -> the guest taps "Herunterladen" again -> mints 2 and 3 succeed, 4 and 5 return 429 but still mint -> the fifth evicts the oldest download ticket for the session, which is A -> the transfer drops, resumes, and 401s -> re-minting is impossible, they are at the daily limit The keepsake is unreachable until the next day, for tapping a button that appeared to do nothing. This is the failure `sse_churn_cannot_evict_a_running_ download` was written to prevent, reintroduced through the one channel that test does not cover: download tickets evicting each other. Discarding the ticket on the refusal path keeps both properties that put the mint first — a capacity 503 still costs no download, and a refused download now costs no slot. Also corrects three doc comments in this path still describing download tickets as "single-use, 30s TTL", false since they were made resumable. Stale comments here have already sent one review down the wrong path. The new spec asserts the two 429s explicitly, so it cannot pass vacuously if the daily limit stops being enforced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
fabi
|
ac04e27e34 |
fix(upload): a retry after release returns the stored photo instead of refusing it
The idempotency key was only readable as a multipart FIELD, and a field cannot be read until the body is being parsed — which happens after the lock/release pre-flight. So the replay was unreachable in exactly the case it exists for: the photo commits → the response is lost on the way back (the flaky-wifi failure the key was added for) → the host releases the gallery at the end of the night → the phone's retry answers `gallery_released`. The guest is told a photo that is sitting in the gallery was never sent. And the remedy the client offers is destructive: `open_event` clears `export_released_at` AND bumps `export_epoch`, retiring the whole keepsake generation and forcing a multi-GB rebuild on a 2-vCPU box at midnight — to re-send a photo that was never missing. Several guests on one flaky evening make this likely to happen at least once. The key is now also sent as `X-Client-Upload-Id`, which arrives with the request line, so the answer is knowable before anything is decided about locks. The multipart field stays for the concurrent case and as a fallback. Placed ahead of the hourly rate limiter too, which was the same mistake one layer up: a 40-photo burst with two retries apiece exhausted the guest's hour on uploads that had all committed the first time. The body is still drained rather than abandoned — replying before reading it makes the proxy see a broken pipe and turn a clean 200 into a 502. The spec carries its own control: a DIFFERENT photo is asserted to still be refused with `gallery_released` after the release, so the replay cannot be green merely because the gate was open. |
||
|
fabi
|
55b57fc037 |
fix(me): two hosts deleting at once can no longer leave the event with no operator
The last-operator guard ran on the pool, before the transaction opened. Two hosts deleting themselves at the same moment each saw the other, both passed, and the event was left with nobody who can moderate, nobody who can release the gallery, and no way to appoint anyone — because appointing requires a host. Not recoverable from inside the app. The fix is a transaction-scoped ADVISORY lock, and the two obvious alternatives are both worse: * `FOR UPDATE` on the other operators' rows DEADLOCKS. Each deleter locks the other's row and then tries to delete its own, so Postgres resolves it by killing one. The invariant survives; the loser gets a 500 instead of the sentence explaining what to do. My first attempt did exactly this, and the test caught it. * Locking the `event` row serialises cleanly but inverts the lock order every moderation path takes (upload/user rows first, event last). That is an ABBA against a path that runs constantly during the event, traded for one that runs approximately never. An advisory lock is a separate lock space, so it cannot interact with the row-lock graph at all, and it is released when the transaction ends. The loser waits, counts zero once the winner's row is gone, and is refused with the sentence it should have got. The test is a genuine concurrency test — it spawns the second deleter and asserts it unblocks to see no remaining operator. It fails against the pre-check-outside-the-transaction version and against the FOR UPDATE version. |
||
|
fabi
|
19b59d6fee |
docs(upload): stop claiming a proxy bandwidth control that does not exist
`get_original`'s comment said bandwidth abuse "belongs at the proxy, where per-connection limits still work", which reads as though the removed per-IP limiter had been replaced by something. It was not: the Caddyfile sets timeouts and no rate or concurrency directive, and the tower stack is TraceLayer alone. Removing the limiter was right — the venue is one NAT address, so that bucket throttled the whole party's feed — but the route is now unbounded, and the comment should say so rather than imply cover. Records the actual cost (no-store plus the derivative fallback plus the nonce'd retry, against a 15-slot pool that upload commits compete for) and the shape a real fix would take: a concurrency semaphore over media streaming, not a request-rate bucket. |
||
|
fabi
|
010bcc0e3c |
fix(build): stop a stale or missing keepsake viewer from shipping silently
Three ways the compiled-in viewer could be wrong, none of which anything would have reported. Found by mutation-testing the guard added below — it failed when it should have passed, and the reason was the second bullet. * `include_dir!` registers NO rebuild dependency. Run `npm run build` in frontend/export-viewer, then `cargo build`, and cargo sees no source change and reuses the cached binary — carrying the PREVIOUS index.html. The file on disk and the file in the binary disagree, git is clean, every check passes, and Memories.zip ships a stale viewer. Confirmed empirically: after replacing the artifact the compiled-in copy did not change until a source file was touched. A build.rs now declares `rerun-if-changed` for `static/export-viewer` AND `migrations` — sqlx::migrate!() embeds its directory the same way, and there the stale snapshot is worse still: the binary boots against a database that already ran a newer migration and crash-loops with VersionMissing. * `emptyOutDir: true` deleted the committed artifact BEFORE generating. That was safe while the build could not fail; it no longer is, because `inlineThemeFonts` now calls `this.error` on a keepsake that is not self-contained. A failed build left the directory empty — and include_dir! over an empty directory compiles fine, while `write_viewer_with_data` iterates zero files and returns Ok. The result is a valid archive with every photo and no viewer. The output is one overwritten file, so nothing accumulates without the wipe. * Nothing asserted the viewer was there at all. Now asserted at the point of use (bail rather than write a viewer-less keepsake) and in a test that checks presence, plausible size, and that no `url(/...)` survived inlining — the three ways it can be present but useless. The Dockerfile copies build.rs with the sources rather than with Cargo.toml, so the dependency-cache layer stays byte-identical and the dummy build does not run it. |
||
|
fabi
|
301e6636a5 |
fix(audit): give the audit trail the names that make it readable
Migration 029 made `actor_id`/`target_id` non-FK on the stated grounds that
"the record must survive the actor's account being removed, which is exactly
when it is most likely to be wanted". All eleven call sites then passed None
for both name columns — so what survived a deletion was a bare uuid resolving
to nothing: the guarantee, minus the only thing that made it useful.
`record` now resolves whatever the caller omitted, in one query, so no call
site can forget. `me::delete_account` passes its names explicitly because it
has already hard-deleted the row by then — that is the one record a host is
most likely to be reading the next morning ("whose photos disappeared?").
Also: `actor_role` is written with `as_str()` rather than
`format!("{actor_role:?}")`. The Debug spelling is not a stable wire format,
so a derive change or a renamed variant would have silently started writing a
different string into a column nothing validates.
Migration 029's header lists three action slugs (`promote_user`,
`demote_user`, `delete_user`) that no call site has ever emitted, and it
cannot be corrected — editing an applied migration changes its checksum and
crash-loops every database that ran it. The real list, verified against the
call sites, is documented in this module instead, along with the fact that
there is no read endpoint and the query to use by hand.
A NULL name fails silently, so it is now asserted: names resolved from ids,
names surviving the row's deletion, and a row still written when neither can
be resolved (an audit write must never fail the action it records).
|
||
|
fabi
|
4916eed436 |
fix(deploy): ship the swap ceilings, pin the last boot-fatal env var, and correct docs that misdirect
* memswap_limit is now IN docker-compose.yml on all four services. Compose sets Memory but leaves MemorySwap unset, and Docker then permits swap equal to the memory limit — so following §5's "add 2 GB of swap" silently DOUBLED every ceiling, to ~5 GiB on a 3.82 GiB box. Nothing OOMs; instead Postgres's working set becomes swap-eligible on a shared-tenancy SSD, turning a bounded OOM-kill that restarts in seconds into unbounded latency with no signal but "everything is slow". The runbook told the operator to hand-add it, which also broke §0's own gate that docker-compose.yml must be unmodified. Verified rather than assumed: service-level memswap_limit does compose with deploy.resources.limits.memory (docker inspect → Memory=1073741824 MemorySwap=1207959552). * DATABASE_MAX_CONNECTIONS pinned in compose. It is the one env var that is now boot-FATAL when unparseable — the right call, but it means a stray quote or a trailing inline comment in .env crash-loops the app behind a live Caddy. MEDIA_PATH, EXPORT_PATH and APP_PORT are pinned for weaker reasons. * .env.example's quota narrative was sized for a CX33: "~30 GB of a fresh 70 GB" on a box with 40 GB. And on THIS box the fixed point never binds at all — ~210 MB/guest is below the 500 MiB floor, so everyone gets the floor and the per-user quota stops bounding aggregate growth. What actually stops uploads is the keepsake preflight at ~8 GB of media. That paragraph is what an operator reads when a guest is blocked, and it pointed at the wrong knob. * The emergency card gains the one disk symptom that can appear mid-event, where `df -h` — its only disk instruction — actively misleads: the gate fires ~10 GB + 2.2x media BEFORE the disk is full, so df shows ~20 GB free at the moment uploads are being refused. * Two code comments that now assert the opposite of the code: claim_job promised that "the update_progress liveness check bails such a worker out early" — it cannot, its predicate is on the job row, which a reopen does not touch, so a mid-export reopen grinds the whole gallery to completion on a 2-vCPU box during the live event. And prune_superseded_archives still argued "deleted bytes cannot be rolled back" as an invariant, after the reclaim path was changed to prune even when that will not close the shortfall. Both now describe what the code does. * Smaller corrections: runbook §3's "two 48 MP photos ≈ 800 MB" scenario is unreachable (compression.rs takes an exclusive heavy permit, so they serialise) and contradicted .env.example; "all four healthy" is wrong since caddy has no healthcheck; a README line reference pointed at a comment added by the same commit that broke it. |
||
|
fabi
|
182e712a0e |
fix(export): a resumed download can no longer splice two archives together
`serve_file` emitted no validator — no ETag, no Last-Modified — and ignored If-Range entirely, while `resolve_export_file` re-reads `export_current` on EVERY request and a download ticket survives 20 redemptions over 6 hours. So: a guest's 500 MB Gallery.zip drops at 500 MB. The host takes a photo down — epoch bumps, the rebuild lands, the old generation is pruned. The client resumes with `Range: bytes=500000000-`. The ticket and session are both still valid, the handler resolves the NEW archive, seeks 500 MB into a different file of a different length, and streams. The client concatenates the halves into a structurally corrupt ZIP. Nothing logs an error anywhere; a 404 would have been the correct answer. Now every response carries an ETag over the generation-stamped filename plus the length, and a partial is served only against a matching If-Range. A Range with no validator — curl -C -, wget -c, the Android download manager, all of which resume blindly — gets the whole file instead. Restarting a download is a cost; a corrupt keepsake is not recoverable. Browsers send If-Range, so this is also the first release where their resume works at all: with no validator to send, they simply refused to try. |
||
|
fabi
|
a2b3cb0e8d |
fix(db): run migrations on their own connection, not a pooled one
after_connect puts lock_timeout = 5s on every pooled connection, and the migrator inherited it. Migrations that take ACCESS EXCLUSIVE — 026's index swap, 027's ADD COLUMN — then turn a short WAIT into a hard FAILURE. The runbook installs an hourly pg_dump (§10.2) and tells the operator to back up before deploying; pg_dump holds ACCESS SHARE on `upload` and `"user"` for its whole run, and the runbook is full of psql snippets that do the same. Boot into that window and the migration aborts, create_pool errors, main exits 1, and `restart: unless-stopped` crash-loops the app behind a live Caddy. The rollback is clean and a later retry succeeds, which is precisely what makes it a baffling intermittent outage rather than an obvious one. 026's own comment reasons that "this runs at boot before the server accepts requests, so the brief lock costs nothing" — true of the app's own sessions, and it does not cover anything else on the database. statement_timeout is dropped for the migrator too: a migration on a real table can legitimately outlast the 15s a request is allowed. |
||
|
fabi
|
a428fe6957 |
fix(social): make the counts clients patch with ban-aware, like the view
Migration 028 added `NOT is_banned` to v_feed.like_count and v_feed.comment_count, but not to the two scalar counts in social.rs — which are returned in the response AND broadcast over SSE, and which clients use to patch a card in place rather than refetching. So the two disagreed the moment anyone was banned: the host bans a guest, the feed correctly drops to the lower number, and the very next like on that photo pushes the unfiltered count back to every open client — including the host's, who is watching that number to confirm the ban took. It stayed wrong until a full page-1 refetch. Both call sites carried comments asserting they mirror the view. 028 made those comments false without touching them; this makes them true again. |
||
|
fabi
|
6afb33e5b6 |
fix(export): four ways the keepsake could be lost, stranded, or published empty
* The HTML completeness guard counted manifest ROWS, and there are up to two per upload — a thumbnail and a full variant. Thumbnails are 400px JPEGs the export generates itself into its own temp dir, so they are no evidence that any original was captured. After the guard was relaxed to bail only on "nothing written at all", that case could no longer fire while thumbnails kept succeeding: if the media volume became unreadable after the stat pass, every original open failed, every thumb open succeeded, and a keepsake with 100 thumbnails and ZERO full-resolution photos published green, done at the live epoch, with the download button lit. Boot recovery skips a done job, so nothing would ever have rebuilt it. Now counts photos, not files. * A decoder panic failed the ENTIRE keepsake. The `?` was on the JoinError, not on the closure's Result, so a panic in the image crate propagated out where the same file merely failing costs one tile — and it was deterministic, because "Neu erzeugen" reads the same poison file and dies the same way. That is the exact failure shape the completeness guard was relaxed to eliminate, arriving through the other door. * delete_account armed both export jobs and then spawned the workers AFTER an awaited file-removal loop. Axum drops a handler future on client disconnect, and every other invalidate_and_arm call site spawns with no intervening await. Dropped inside that loop, the keepsake is left with the epoch bumped, both rows pending at that epoch, and no worker: the downloads 404 and the UI sits on "Wird vorbereitet..." until someone reboots the app. Deleting your account from a phone that walks out of range is enough. * The daily download quota was charged before the ticket could fail, so a store-capacity 503 — a server-side condition the guest cannot see or cause — still cost one of their three downloads. There is no refund path. |
||
|
fabi
|
9b38d31f97 |
fix(upload): stop a late retry from undoing a host takedown
Migration 026 freed the idempotency key as soon as deleted_at was set, so a
retry after a delete uploads afresh instead of 409ing forever. That rationale
only considered the GUEST deleting. deleted_at is also set by
host_delete_upload, and there the same rule reverses a moderation decision:
1. Guest uploads; the row commits and the photo appears, but the response
is lost on the way back — the flaky-wifi case the key exists for — so
the phone keeps the queue item.
2. The host takes the photo down. Epoch bumped, keepsake rebuilt without it.
3. The phone reconnects ten minutes later and retries. The key is free, the
INSERT succeeds, and the photo is back — in the feed and in the next
keepsake, under a NEW uuid that matches nothing in the host's moderation
history, with nothing logged to say a takedown was reversed.
Migration 031 keeps the key claimed for a host takedown and releases it only
for a guest's own delete, so the retry resolves to the duplicate path and is
refused. The refusal now says why ("von den Gastgebern entfernt") rather than
"already processed", which invites another try.
The index predicate and the ON CONFLICT arbiter are changed in lockstep;
these queries are not compile-checked, so a drift between them is a 500 on
exactly the retries the index exists to serve. Verified against a real
Postgres: live retry suppressed, host takedown holds the key, guest delete
releases it. The integration test's copy of the insert is updated too — it is
verbatim by design, and a stale copy would have kept passing.
|
||
|
fabi
|
1b3ca46f8a |
fix(auth): three ways one guest on the venue NAT could lock everyone else out
All three are the same mistake in different clothes: a limit keyed on an IP that, behind the venue's NAT, is the entire party plus the host. * join_ip_rate_per_min was raised 60 -> 300 last round and it never took effect. A config default is only a fallback for a MISSING key, and migration 017 seeds this one, so the seed won and the raise was dead code on every real install. Migration 030 raises the seeded value the way 015 already did for upload_rate_per_hour. The e2e guard could not see this: it fires 12 concurrent joins, which is green at 60 and at 300 alike. * /recover's per-(IP, name) bucket charged EVERY request, including successful ones, and refused before verifying the PIN. Its ceiling clamps to 4. So four POSTs naming "Braut Sophie" with PIN 0000, from any phone on the venue wifi, locked Sophie out of her own recovery for fifteen minutes WITH THE CORRECT PIN — and four more every fifteen minutes sustained it indefinitely, at a rate far under every volume ceiling above it. The benign version needs no attacker: the host mistypes their own PIN four times. Hosts are promoted guests whose only credential is that PIN, and /recover is their only way back after losing a session. Now it counts failures, and a spent budget changes what a FAILURE answers instead of refusing outright. Guessing is bounded exactly as before — wrong PINs are what spend it — with the per-account lockout underneath. * /admin/login's pre-verify ceiling had the same shape, and the escape hatch was circular: admin_login_rate_enabled is only flippable through PATCH /admin/config, which needs the session being refused. One phone posting twice a minute cost the operator moderation, gallery release and every config key, including the ones that would undo it. Exceeding the ceiling now shortens the hash-permit wait rather than refusing: the CPU bound was always the semaphore, never this bucket, so a flood still sheds itself while a correct password gets a truthful answer. Adds a regression test that reads the value a fresh database actually ends up with, by replaying the migrations — the drift that made the first bullet invisible is not otherwise detectable from the code. |
||
|
fabi
|
f5c55d6f92 |
chore(backend): route wiring, error mapping, and a crossbeam-epoch bump
Cargo.lock moves crossbeam-epoch to 0.9.20, clearing RUSTSEC-2026-0204. Targeted rather than a broad `cargo update` across 406 crates, which is not a change to make days before a live event. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
fabi
|
8720571beb |
fix(export-viewer): inline the webfonts so the offline keepsake is self-contained
The viewer inherits `src: url('/fonts/Inter.woff2')` from the shared theme CSS. That is
correct for the app, which serves `static/fonts/` from the site root — but the keepsake is
opened from file:// off a USB stick or a Downloads folder, where `/fonts/...` resolves to
the root of the guest's DISK. Both requests 404, and `font-display: swap` makes it silent:
the viewer renders in a fallback system font with nothing server-side able to report it.
Found by opening a real released keepsake in a browser and watching `requestfailed` — no
other signal exists, which is the recurring lesson about this artifact.
Fixing it in the shared CSS would inline ~154 KB into every app page load for nothing, and
shipping a `fonts/` folder beside index.html gives the guest a directory they can break by
moving one file. So the substitution belongs in the build that knows its output has no
origin. The plugin errors the build if the theme ever stops referencing those URLs, rather
than silently shipping another keepsake in Times New Roman.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
fabi
|
c9a4d4a9c0 |
fix(export): stop the keepsake guards from destroying the keepsake
Two guards added to protect the archive each had a failure mode worse than the one they prevented, and both were unrecoverable — which is what makes them worth reverting rather than tuning. The completeness gate refused to publish once skips passed max(2, 10% of expected). That 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 uploads cannot be collected again. On a 30-photo event, four bad files meant nobody ever got the other 26. That is precisely the "one-photo gap becomes total loss" outcome MAX_SKIPPED_FRACTION's own comment says it exists to avoid. Anything short of an empty archive now publishes and logs the counts at error level. `written == 0` stays fatal — a wrong MEDIA_PATH is a misconfiguration the host CAN fix and retry, and it once shipped a few-hundred-byte ZIP containing zero photos that passed every automated check. The space reclaim refused to prune unless it freed the entire shortfall, to protect an archive that no handler can serve: a download resolves through `export_current`, which requires `job.epoch = event.export_epoch`, and the epoch only increments. Meanwhile `reclaimable` is scoped to the caller's own prefix — one old archive — while `deficit` is sized for both halves plus the reserve. So on a tight disk each worker measured its own share as insufficient and neither pruned, though the two shares were jointly sufficient. Every "Neu erzeugen" reran the identical arithmetic and refused identically: permanently stuck, with dead archives nothing would reclaim and nothing could serve. It now prunes what it can and lets the re-check decide, so the sibling's prune lets the host's retry converge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
fabi
|
7154b3a810 |
fix(upload): remove the /original rate limit that would have broken the feed
The limiter added here was justified as bounding "100 guests occasionally tapping Original anzeigen". That is not what this route is. `pickMediaUrl` resolves to `preview_url ?? thumbnail_url ?? /original`, and a freshly committed upload has BOTH derivatives null until the compression worker reaches it — at COMPRESSION_WORKER_CONCURRENCY=2 that is minutes during a post-ceremony burst. So /original is the feed's hot path for exactly the newest photos, in a newest-first grid, at the busiest moment. With every guest behind one NAT the 600/min bucket is venue-wide: six new photos fanned out by `upload-new` to ~100 open feeds exhausts it, and then every original fetch from anyone 429s for the rest of the window. The tiles' own 4-second retry uses a fresh `?r=` nonce, so the clients hold the bucket saturated themselves — the whole venue watching the newest photos render as broken tiles while the projector skips slides. A per-IP bucket cannot separate one scraper from the entire party when they share an address, and these media routes are unauthenticated by design (an `<img>` cannot send a bearer token), so there is no per-user key to move to. Bandwidth abuse belongs at the proxy. Also here: the release/lock check order. `release ⇒ lock`, so testing the lock first made the `GalleryReleased` arm unreachable dead code and every post-release upload answered `uploads_locked`. The codes are not interchangeable to the client — `uploads_locked` charges a retry attempt and re-pushes the whole photo on the backoff ladder against an answer that cannot change, while `gallery_released` parks it and says the photo is safe but the hosts must reopen. Both sites now test release first, so the fast path and the commit-time re-check agree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
fabi
|
ec7c7f18ca |
fix(auth): stop one guest on the venue NAT from locking everyone else out
Every guest at the venue shares one public IP, so an IP-keyed limiter throttles the whole party as a single client. Three separate limits got that wrong, and the host — whose only credential is a 4-digit PIN — was the one who could not absorb it. /recover carried a cross-name failure budget checked BEFORE the account lookup, so it refused a CORRECT PIN. Thirty POSTs with invented names spent the shared budget for fifteen minutes and ~2 requests/minute sustained it indefinitely, denying PIN recovery to everyone including a host locked out of their own event. The budget is now carried as a flag: a correct PIN authenticates regardless, while wrong ones answer 429 instead of 401. Guessing stays bounded where it always really was — the per-(IP,name) ceiling and the per-account 3-strike lockout, neither of which an attacker on any IP can evade. join_ip went from 60/min to 300. A 100-guest wedding does not trickle in; it arrives when the QR code goes up, all from one address, and guests 61-100 were turned away on the one screen with no auto-retry. This limit only bounds raw volume — the per-name bucket is the anti-spam control and BCRYPT_PERMITS is the CPU bound — so it can sit well above the peak. Download tickets are now bound to ONE archive via `TicketKind::Download(ExportKind)`. Both download routes share an authenticator, so a bare ticket opened either; combined with the resume budget that made a single mint worth 40 transfers of a multi-GB keepsake while the per-day limiter, charged only at mint, never moved. `kind` is consequently required at /export/ticket; every shipped client already sends it. The per-session ticket cap is now per-kind. A 6-hour download ticket is always the oldest entry for its session, so ordinary SSE churn evicted it first — and /export opens its own SSE connection on the session that just minted it. A couple of wifi flaps mid-transfer killed the ticket, 401'd the resume, and cost the guest another of three daily downloads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
fabi
|
963f6449a1 |
feat(db): idempotency keys, ban-aware counts, and a host audit trail
Four migrations, all additive against a database that already has 001-025 applied. 026 narrows the client-upload idempotency index with `AND deleted_at IS NULL`. The old index made a soft-deleted row keep its key forever, so a guest who deleted a photo and re-sent the same one had the retry silently swallowed. The new indexed set is a strict subset of the old, so it cannot fail on existing rows. 027 adds `client_join_id`, which lets a join retry after a lost response resume the same account instead of 409ing on a name the caller itself owns. Every existing row gets NULL and the partial index excludes NULLs, so it indexes nothing at creation. 028 brings the feed view's like/comment counts in line with what the feed actually renders: a banned guest's rows were still counted, so a card showed "3 comments" above two. `comment.rs` gets the matching `NOT u.is_banned` on the live read path — the export and hashtag queries already filtered it, so the two views of one moderation action disagreed. 029 records host moderation actions, which were previously invisible after the fact. Verified by applying 001-029 to a real Postgres against seeded data, including a soft-deleted row holding a key and a banned user's like and comment. 026's down-migration legitimately fails where a deleted and a live row share a key — that is inherent to the direction, documented in the file, and sqlx never runs downs at boot. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
MechaCat02
|
ef6d3a077a |
fix: close what nine adversarial reviews found, most of it mine
Some checks failed
Checks / Backend — cargo test + clippy + fmt (push) Failing after 1m5s
Checks / Frontend — vitest + svelte-check (push) Failing after 5m55s
Checks / E2E — typecheck + lint (push) Failing after 49s
E2E / Playwright E2E (chromium + webkit) (push) Failing after 10m42s
E2E / Cross-UA smoke matrix (push) Failing after 7m57s
Audit / cargo audit (backend) (push) Failing after 11m12s
Audit / npm audit (frontend) (push) Successful in 44s
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 |
||
|
MechaCat02
|
214f9e3062 |
fix: close four confirmed defects an adversarial review found
Findings from a multi-angle review, most of them in code I wrote in the last
few commits. Each was verified against the code before being acted on.
## Backend
**The export daily limit was bypassable ~60x/minute.** `SseTicketStore` is
untyped, and the export download quietly started reusing it. `POST
/stream/ticket` is free and rate-limited at 60/min per user; `POST
/export/ticket` charges one of three PER-DAY downloads. So a guest could mint
at the cheap endpoint and redeem at the expensive one, each redemption
streaming the whole multi-GB keepsake, `no-store`, off the same filesystem
Postgres writes WAL to. Tickets now carry a `TicketKind` and `consume` requires
it to match, asserted in both directions. The comment claiming "one mint is at
most one download" was simply false.
**`export_ticket` answered 200 `{"ticket": null}` when the store was full** —
after charging a daily slot. `issue` returns `Option`; `sse.rs` handles the
None with a 503 and this call site unwrapped it into the JSON body. The page
toasted success, the iframe navigated to `?ticket=null`, and one of three
downloads was gone. That is the phantom-success failure the pre-validation in
|
||
|
MechaCat02
|
253878e027 |
fix(export): give the keepsake viewer the two-phase preflight it was meant to get
The two-phase preflight from
|
||
|
MechaCat02
|
5b705317ef |
fix: close the last three guest-facing dead ends (items 7-9)
C1 — a failed page-append silently ended infinite scroll `loadMore`'s catch showed a toast and changed no state, unlike every sibling error path in the file. `nextCursor` survived so the feed was still technically paginable, but the IntersectionObserver only fires on a CHANGE: after a failed append nothing scrolls and no rows are added, so it never re-fires. One 429 or wifi blip and the guest concluded the gallery was 20 photos. Now leaves a retry control at the sentinel — a toast that fades in 5s is not an affordance — resuming from the untouched cursor. C2 — the export page reported downloads that never happened `downloadFile` toasted 'Download gestartet' the instant it assigned the iframe's src, before a single byte existed. Since the iframe swallows errors BY DESIGN (a top-level navigation to a 404 would unload the PWA), a failure produced a green success message, a consumed single-use ticket, and one of only three daily slots spent — repeatable until the day's allowance was gone, on the screen that is the whole point of the app. Root cause is two sources of truth: `export_status` reports `done` from `export_job` and enables the button, while the download resolves through `export_current.file_path` plus a `Path::exists()`. They can legitimately disagree. `export_ticket` now takes a `kind` and calls the existing `resolve_export_file` BEFORE charging the rate slot, so a missing archive fails honestly on a plain fetch that `toastError` already renders. Not the HEAD probe ruled out elsewhere: it reads the same indexed row the download will read and touches no ticket, so it cannot consume anything. The parameter is optional, so an older client degrades to today's behaviour rather than breaking. C3 — the WhatsApp journey could dead-end with no error at all The join link travels through guest group chats, and a link tapped inside one opens in that app's browser, where the file picker and getUserMedia both depend on the host app having wired them up. When they aren't, the buttons do nothing — no error, nothing to act on. Two targeted changes rather than a UI rebuild: the camera error panel now offers "Aus Galerie wählen" (its advice to change "Browsereinstellungen" refers to settings that do not exist in a webview, so retrying could never help those guests), and the sheet carries a standing one-line hint to open the link in Safari or Chrome. Deliberately no user-agent sniffing: a sniff list is wrong for every browser it has not heard of, while a quiet standing hint costs one line and is never wrong. The hint lives in UploadSheet rather than the root layout because both layout banners are gated on `$showBottomNav`, which `/upload` turns off — one there would never render on the composer. Verified: 151/151 backend tests against a live Postgres, clippy clean, 58/58 vitest, svelte-check 0 errors, eslint clean, both builds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
MechaCat02
|
05063694d2 |
fix: close eight regressions the audit pass found, five of them mine
Two adversarial reviews over |
||
|
MechaCat02
|
23e2f485dd |
fix(upload): correct two errors in the keepsake headroom gate
Both found reviewing my own change rather than by a test, which is the point. DOUBLE-SUBTRACTION. The gate computed `free - size`, but the body is streamed to its temp file during multipart parsing — far above the gate — so the free-space reading already excludes those bytes. Subtracting again refused uploads a full file-size early; with max_video_size_mb at 500 that is half a gigabyte of phantom pressure. `media_total` genuinely does need `+ size` (its row is not committed yet), which is what made the asymmetry easy to miss. BLOCKING SCAN ON THE HOTTEST PATH. It called `disk::free_bytes`, whose doc comment says it deliberately bypasses DiskCache — but that rationale is the export preflight's: a rare, high-stakes decision where a sibling worker can move free space by tens of GB inside the TTL. Per upload it means sysinfo re-scanning every mount, synchronously, on the async runtime, on a 2 vCPU box with two worker threads. Now uses the cached snapshot, the same 15s staleness the quota check immediately below already accepts for the same question. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
MechaCat02
|
eb0e405562 |
fix: gate uploads on keepsake headroom, and close five unattended-event gaps
The box is 2 vCPU / 4 GB / 40 GB, not the 4 vCPU / 8 GB / 80 GB that the audit, the committed comments and README's sizing section all assumed. That correction is what the first change is about; the rest are the remaining pre-event items. THE ARCHIVE COULD BECOME UNBUILDABLE WHILE UPLOADS KEPT SUCCEEDING `required_free_bytes` is `media × 1.1 × 2` — the ZIP and the HTML viewer are each gallery-sized — and the export preflight also wants DISK_RESERVE_BYTES on top. The upload gate, though, only refused below a FLAT 10 GB reserve. On 40 GB that let uploads run to ~25 GB of media while a release needed `2.2 × 25 + 10` = 65 GB free. Every upload in that band succeeded and the keepsake could then never be built: the product's entire promise, failing silently at the end of the night with nobody there. The gate now enforces the invariant that actually matters — never accept an upload that would make the keepsake unbuildable — sharing `required_free_bytes` with the preflight so the two cannot drift into disagreeing about the same question. Uploads stop at ~8 GB of media on this disk, with a German message naming the cause. Refusing the 1001st photo beats losing all 1000. `media_total.rs` backs it: SUM(user.total_upload_bytes) over ~100 rows, cached 5s, rather than `estimate_export_bytes`'s join across every upload. It counts hidden and banned users' bytes, which the export excludes — skew in the SAFE direction, so the gate closes marginally early rather than late. Fails open on a query error. A test pins the gate against the preflight across the whole gallery-size range, and a second asserts the per-user floor alone would over-commit the volume — i.e. that the global gate is what must bind. THE WATCHDOG ABORTED HEALTHY UPLOADS EVERY TIME A PHONE WAS POCKETED `Date.now()` advances while a backgrounded phone is frozen but `setInterval` does not, so the first tick after a screen lock read the whole sleep as silence and aborted — re-sending a video from byte zero and burning one of five PERMANENT auto-attempts. The interval is now its own suspension detector: a tick that arrives 125s late for a 5s schedule credits that window back, because a period the watchdog could not observe is not evidence of silence. Chosen over a `visibilitychange` listener, which only covers causes that fire that event — a throttled-but-visible tab, a closed lid and an occluded window all freeze timers without one — and which would have needed module state, an SSR guard and a teardown for strictly less coverage. `performance.now()` was rejected because Safari pauses it across system sleep on some paths and Chrome does not. The credit buys one fresh window, not immunity: a socket iOS reaped while backgrounded still aborts ~90s after resume rather than hanging for `xhr.timeout` (5-60 min) with the queue's `processing` latch held. Two latent leaks found while in there: `xhr.abort()` on a request already in readyState DONE emits no `abort` event, so `settle()` never ran and the interval re-aborted every 5s forever while `activeUploads` kept a stale entry (the ✕ button silently stopped working); and a synchronous throw from `xhr.send` — a blob whose backing store the OS purged — leaked the same way. Both closed. OKLCH MADE THE DELETE BUTTON INVISIBLE ON SAMSUNG'S DEFAULT BROWSER red/amber/green were never in the @theme block and fell through to Tailwind v4's `oklch()` defaults, which Safari <15.4, Chrome <111 and Samsung Internet <22 cannot parse: `var(--color-red-600)` is then invalid at computed-value time, `background-color` falls back to transparent, and `.btn-danger` renders white text on nothing. Pinned to Tailwind's own defaults gamut-mapped to sRGB by Lightning CSS — the converter already in this pipeline — so modern browsers render exactly what they render today. Verified against seven hex fallbacks it had already emitted for the /alpha forms. rose and teal (avatar chips) had the same leak. The app CSS goes from 40 oklch declarations to 0. Also fixes `--color-purple-950`, which was simply missing: `dark:bg-purple-950/50` on the host dashboard was rendering default violet on EVERY browser, off-brand. The keepsake viewer only picks this up on a rebuild, so its committed artefact is rebuilt here too — still single-file, still zero external references. A BRICKED BOOT LOOKED LIKE A SPINNER FOREVER With `ssr = false` the page is empty until the bundle mounts, so a chunk 404 after a redeploy or a dead uplink left the guest on the boot spinner with no message, no reload control, and in a standalone PWA no URL bar. A 15s timeout in the existing nonce'd IIFE (no CSP change) swaps in German copy and a reload button. Deliberately a timeout rather than feature detection: a SyntaxError in the bundle is invisible to any capability check. Plus a <noscript>, since there was nothing at all to see without JS. EVERY 4xx WAS INVISIBLE AT ANY LOG LEVEL tower_http counts 4xx as a success, so it logs at DEBUG while production runs at info. If guests spend the evening hitting 429s or 413s, the post-event logs said nothing. Now one WARN per client error; 5xx excluded because Internal already logs its source chain and the pool-exhaustion 503 logs at construction. A DEAD FRONTEND SERVED A BLANK 502 `handle_errors 5xx` with an inline German page (the caddy service mounts only the Caddyfile, so there is no volume to ship a static file through). Verified empirically against this config, not from documentation: an upstream 404 through `reverse_proxy` still arrives as untouched `application/json`, and only a dial failure renders the page. That mattered — the keepsake download navigates a hidden iframe and DEPENDS on a real 404/429 arriving, and swallowing those would have been worse than the blank 502. CONFIG CORRECTIONS FOR THE REAL HARDWARE DATABASE_MAX_CONNECTIONS 30 → 15: sized to 2 vCPU rather than to the guest count. Since migration 024 a feed page costs well under a millisecond, so connections are no longer spent waiting, and 30 backends crowd the db container's 1 GB on a 4 GB host. COMPRESSION_WORKER_CONCURRENCY stays at 2 — the merged heavy-image permit already serialises anything over 150 MiB, so the "two 48 MP photos" worst case that number was sized against is unreachable; dropping to 1 would halve light-path throughput and push more feed tiles onto full-size originals. README's sizing section rewritten for the actual disk. Verified: 149/149 backend tests against a live Postgres, clippy clean, 57/57 vitest, svelte-check 0 errors, eslint clean, vite build, export-viewer rebuild, caddy validate, compose YAML parse. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
MechaCat02
|
1d9fb11c7b |
fix: close the nine ways an unattended event loses photos or dies
Every one of these was found in the pre-event audit, verified against source, and
survives to production on the current main. Grouped by what actually goes wrong.
PHOTOS DISAPPEAR
* compression.rs no longer soft-deletes on a failed derivative. The guest got a
201, watched the card appear, then watched it vanish — the row left v_feed,
find_visible_media and BOTH keepsakes, while its bytes sat on disk for 14 days
waiting for a cleanup nothing announced. No screen anywhere lists compression
failures, so recovery meant hand-written SQL that also had to re-add the
refunded quota. Now it does exactly what the ENOSPC arm beside it already did
and documented as correct: keep the row, serve the original, retry on the next
boot (bounded by derivative_attempts). `upload-deleted` is no longer emitted;
`upload-processed` is, so the card re-renders instead of sitting on a
placeholder.
* A 413 is now a reversible lock, so the blob survives. The quota moves — free
disk falls, uploader count rises — so a guest goes over it having done nothing,
and treating that as permanent meant a 400 MB video was pushed across cellular
in full and THEN deleted from IndexedDB. Gone on both sides, and unrecoverable
for an in-app camera capture that exists nowhere else.
* quota_limit_bytes gained a floor and a stable divisor. The ceiling used to
decrease monotonically all evening; it now settles at max(uploaders,
estimated_guest_count) — a config key that was seeded, validated in the admin
whitelist, and read by no code at all. The floor is clamped to what the disk
can actually back, so a full volume still yields zero rather than handing out
an allowance it cannot honour.
* Because that floor gives up the aggregate guarantee the formula used to imply,
uploads now check a hard 10 GB reserve first, independent of every quota
toggle. postgres_data, media_data and exports_data share one filesystem: the
end state was not a degraded feature, it was Postgres unable to write WAL.
THE ARCHIVE DISAPPEARS
* prune_superseded_archives runs only after the new generation lands. It ran
before the preflight, reasoning the old archive was already unreachable — true
of reachability, false of recoverability. An epoch is a value that can be
rolled back; deleted bytes cannot. Any failed rebuild left the event with NO
keepsake at all.
* The export preflight reserves the same 10 GB. `free < needed` authorised an
export sized at exactly free, which ran for half an hour and landed the box at
zero with the keepsake still unfinished.
THE APP DIES
* The feed reconcile re-reads the id set after its awaits instead of reusing one
captured up to three round-trips earlier. The new-upload SSE handler prepends
during exactly that window, so the row was both already present and absent from
the stale set — prepended twice, and a duplicate key in a keyed {#each} throws
in production, not just dev. The SSE handler and loadMore now dedupe too.
* Added routes/+error.svelte. Without it any uncaught error fell through to
SvelteKit's unstyled English 500 with no reload control — inside a chromeless
standalone PWA with no URL bar, for the rest of the evening.
THE OPERATOR IS LOCKED OUT
* admin_login verifies the password BEFORE charging the rate bucket, and a
correct password is never throttled. The old order made this a denial of
service against its own operator: every guest shares one NAT IP, the check ran
first, so five requests a minute from any phone in the room kept the bucket
full — and the escape hatch needed the admin session being blocked. A generous
separate ceiling still bounds bcrypt CPU.
THE PROJECTOR DIES
* The preload budget is now strictly inside the dwell. At the 3s option the 4s
budget could never land a commit on a slow uplink, so the wall froze on one
photo while the queue drained silently behind it.
* The wake lock retries every 30s while visible, and the page says so on screen
when the browser has no wake lock API. visibilitychange was the only retry
trigger and a kiosk never changes visibility, so one refusal — iOS in Low Power
Mode, say — was permanent.
* Caddy: /api/v1/upload/*/display joins the cacheable carve-out. The backend set
max-age=300 on it and the blanket no-store silently replaced it, so a projector
re-fetched a full-size JPEG per slide, ~2-4 GB over an evening on the uplink
the guests are uploading over.
Also removes Upload::soft_delete, now unreferenced and an unscoped footgun next
to soft_delete_in_event.
Verified: 146/146 backend tests against a live Postgres, clippy clean, 51/51
vitest, svelte-check 0 errors, eslint clean, vite build, caddy validate, compose
YAML parse.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
MechaCat02
|
61119be817 |
Merge branch 'fix/deploy-unattended-blockers' into main
Two independent lines of production hardening diverged at |
||
|
MechaCat02
|
e1c689d1a7 |
chore(migrations): renumber 021-023 to 023-025 to clear the collision with main
main independently shipped 021_hashtag_counts_respect_bans and 022_client_upload_idempotency. sqlx::migrate! embeds ./migrations and refuses two files per version, so these three had to move before the branch could merge at all. Contents are untouched — only the version prefixes change. Renumbering (rather than renumbering main's) is the safe direction: main is already deployed, so its 021 and 022 are applied in production and their versions are now immutable. |
||
|
Fabian Hamm (Privat)
|
edc5f1f62c |
chore(export-viewer): rebuild the embedded bundle, and fix the lint ignores
The offline keepsake viewer had the same two filter defects as the app: typed suggestions were capped so a matching tag could be unselectable, and the dropdown used `onmousedown` with a backdrop that swallowed the selection. Rebuilt into `backend/static/export-viewer/index.html`, which `include_dir!` embeds in the binary. The eslint ignores were unanchored, so once the export-viewer's dependencies were installed its nested `.svelte-kit` output was linted as source. Anchored with `**/`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Fabian Hamm (Privat)
|
87d01a8a26 |
fix(export): charge the download limit where the client can see the answer
The keepsake download is an iframe navigation, so its response is invisible to the page. The rate limit was enforced inside the zip/html handler — i.e. inside that navigation — while the ticket POST in front of it always returned 200. A guest over the limit therefore tapped "Herunterladen" and absolutely nothing happened, forever, with no explanation, on the one screen that is the emotional payoff of the whole app. With the default of 3/day, ZIP + HTML costs 2 and one retry locks them out until tomorrow. Minting is a normal `fetch`, so the limit moves there and the 429 reaches the user. The limit is not weakened: tickets are single-use with a 30s TTL and can only be obtained from that authenticated endpoint, so one mint is at most one download — and charging it in both places would have cost every download two slots. The message named the wrong timescale too. It shared the generic "warte kurz" wording with the per-minute limiters, but this bucket is a DAY, so a guest was told to wait a moment for something that could not work again until tomorrow. Verified live: three mints succeed, the fourth returns 429 in German; raising `export_rate_per_day` through the admin API takes effect on the next request with no restart, and the HTML keepsake then downloads. Also here, from the same pass: - `looks_bcrypt` checks the SHAPE of ADMIN_PASSWORD_HASH, not just placeholder-ness. A hash corrupted by shell or Compose escaping is not a placeholder, so the app booted green, `/health` said ok, and every admin login 401'd — unrecoverable mid-event, because the Admin row is only created BY a successful admin login and promoting a host requires one. - The rate limiter indexed `timestamps[0]` while holding its mutex, so a `max == 0` configuration panicked and poisoned the lock process-wide. Uses `first()`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Fabian Hamm (Privat)
|
496dba5a1f |
fix(feed): filter on the server, exactly, and keep banned uploads out of the chips
Filtering was split across two independent client-side states and applied to whatever page 1 happened to hold, by caption SUBSTRING. So a tag chip selected in the list view was silently still applied in the grid without being shown; a filter matched photos whose caption merely contained the text; and anything past the first page was invisible to it. Verified against the seeded data: `hashtag=tanz` returned 6 photos by substring, 1 by tag. `FeedQuery` now carries `hashtag` (single, list view), `hashtags` (CSV, OR'd, grid chips) and `uploader` (exact, AND'd), normalised through one function that trims, strips `#`, lowercases and dedupes, and yields None when empty — so an empty filter means "no filter", never "match nothing". The two SQL branches collapse into one with `h.tag = ANY($4)`. Tag-OR plus tag+user-AND is a specified feature, not an accident: `e2e/specs/03-feed/filter-search.spec.ts` and USER_JOURNEYS §8 pin it, which is why the semantics moved to the server rather than being simplified away. Tags travel as CSV safely because the backend restricts them to ASCII alphanumerics and `_`; `uploader` stays a single exact parameter because a display name can contain a comma. New `GET /api/v1/uploaders` reads `v_feed`, so banned and hidden uploaders are excluded for free. Migration 021 gives `v_hashtag_counts` the same treatment. It counted every upload regardless of the uploader's ban state, so banning a guest left their tags in the chip list as ghost filters that lead to an empty feed. Verified: after banning the guest who owned all six `tanz*` photos, the chips went 6 -> 0. `?limit=-5` returned a 500 — only the upper bound was clamped, so Postgres was asked for `LIMIT -4`. Clamped at both ends. `is_banned` is added to `/me/context` so the client can show a read-only notice instead of letting a banned guest discover the ban one 403 toast at a time. `add_comment` sorts and dedupes hashtags on the normalised key, matching the upload path — the two disagreed, which is a lock-ordering deadlock between concurrent upserts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Fabian Hamm (Privat)
|
1d0df3ebf6 |
feat(upload): make uploads idempotent so a lost response cannot duplicate a photo
The ordinary mobile failure, not an exotic one: the server receives the body, validates it, commits the row — and the response is lost on the way back because the guest walked out of range or the AP dropped the connection. The client sees a network error with the blob still in hand and re-sends it, both when the guest taps "Erneut" and automatically when the queue requeues on reconnect. Every attempt minted a fresh `Uuid::new_v4()` server-side, so the same photo landed in the gallery two or three times and was charged against the guest's storage quota each time. The client already has a stable per-queue-item UUID, so it costs nothing to send. Migration 022 adds `client_upload_id` with a partial unique index — partial so the NULLs of every pre-022 upload, and of any caller that doesn't send one, keep working untouched. Two paths, because there are two races: - Sequential retry: a lookup before the transaction finds the stored row, deletes the re-sent bytes and replays the original response as 200. The body has necessarily already been streamed, since the key arrives as a multipart field — re-sending is the client's cost and is already paid by the time we see it. What must be prevented is a second ROW. - Concurrent retry: two attempts in flight at once. `ON CONFLICT DO NOTHING` returns no row to the loser, which abandons its transaction (quota increment included) and replays the winner. Letting the unique index raise instead would only surface after the transaction had aborted, as an opaque error the caller would have to string-match. The replay reads live state rather than assuming a fresh row: a reconnect can be minutes later, by which time the derivatives may exist and the photo may have been liked. Every read there fails soft — the upload is already safely stored, so a sparser response is fine and failing the request is not. Verified live: the same photo sent three times returns 201, 200, 200 with one id, one row, and the quota charged exactly once. Also in this file: the two image-header probes at admission now run on `spawn_blocking`. Both open the file and run the codec's header parse synchronously, and `#[tokio::main]` gives two worker threads on a 2-vCPU box — so every upload stalled half the runtime's request-serving capacity. Everything else that blocks here (image encode, bcrypt) was already offloaded; this was the one that wasn't. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Fabian Hamm (Privat)
|
faea555967 |
fix(ops): make /health a readiness probe, and bound how long the pool waits
`/health` returned the literal string "ok" and touched nothing. Every request in this app needs the database, so that answered a question nobody asked: the container reported healthy while every request 500'd, and with no operator watching during the event there was no signal at all. It now runs `SELECT 1` with a 2s timeout. Verified live against the production stack: 200, stop Postgres, 503 "database timeout", start Postgres, 200 again — with no app restart, because sqlx revalidates on acquire. Deliberately NOT wired to automatic recovery. Compose's `restart: unless-stopped` does not react to healthcheck state anyway, and an autoheal sidecar would be actively wrong here: it would truncate every in-flight upload to "fix" an outage that, as the test above shows, clears on its own. This is a diagnostic — including for the runbook's event-day `curl`. The pool had only `max_connections` set. Three additions: - `acquire_timeout(5s)`. sqlx defaults to 30, so a DB blip parked every request AND all ~100 SSE session revalidations for half a minute before erroring — the app looked hung rather than degraded, and the backlog outlived the blip. - `min_connections(2)`, so the first request after the setup-to-guests-arriving gap doesn't pay TCP + auth. - `statement_timeout=15s` / `lock_timeout=5s` per connection. Without them a single pathological query holds a pool slot indefinitely and no client-side timeout can take it back, because the slot is only released when Postgres finishes. Those two SETs are sent as two statements. `sqlx::query` uses the extended query protocol, which permits exactly one per call — as `SET a; SET b` every new connection failed, which surfaced as the pool never opening one and `create_pool` reporting a connect timeout. Caught by booting against an empty database rather than a warm one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Fabian Hamm (Privat)
|
157499d493 |
fix(ops): reclaim abandoned upload temp files, and supervise the task that does
`stream_field_to_file` removes its `.tmp` on every error return, which covers everything the handler can see. It cannot cover what actually happens at a party: the client goes away — a phone sleeps, a guest walks out of range, the PWA is evicted mid-video — and axum DROPS the handler future rather than returning an error, so no cleanup runs at all. The shutdown backstop force-exits in-flight handlers for the same net effect. Nothing else reclaimed them. `cleanup_deleted_media` only visits rows with `deleted_at`, and an abandoned upload never got a row; `export::sweep_orphan_temps` is only ever pointed at the exports volume. `grep -rn read_dir src/` had three hits, all in export.rs — the media tree was never read by anything. So every abandonment stranded up to `max_video_size_mb` of unowned bytes permanently, on the same 40 GB filesystem as `postgres_data`. Worse than a leak: the per-user quota is computed from live free disk, so those bytes were also subtracted from what everyone else was allowed to upload. An evening of flaky venue wifi could take the event down. The threshold is on modification time, not creation time, which is what makes an hour safe: a live upload is written to continuously so its mtime keeps advancing and it can never age into the sweep no matter how slow the connection. The clock only starts once the writer stops. The periodic task is now supervised. It carries every piece of recurring hygiene in the app — session pruning, media reclamation, this sweep, and the rate-limiter and SSE-ticket maps — as a bare `tokio::spawn` with no retained handle, so a single panic anywhere inside it stopped all five permanently and silently. No log line, no symptom until the disk or a HashMap grew into one. Tested: an hour-old temp is reclaimed, a temp still being written to is not (deleting that one destroys a live upload), a committed `.jpg` is never touched, and a media tree that does not exist yet is a silent no-op rather than an error logged 24 times a day. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
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> |
||
|
Fabian Hamm (Privat)
|
43d37269b6 |
deploy: pull prebuilt images instead of building on the event server
The production compose still carried `build:` keys and no `image:` keys, so a
`git clone` onto the CX22 followed by `docker compose up -d` would have started a
fat-LTO release build of 427 crates on a 2-vCPU/4 GB box — the outcome the whole
build-on-the-Mac decision exists to avoid, reached silently because `pull` skips a
service it is told to build rather than failing.
Both services now pull `registry.mc02.dev/eventsnap/*:${EVENTSNAP_VERSION}` with the
`:?` form, so a missing tag fails the command instead of resolving to an empty one.
`docker-compose.build.yml` restores the `build:` keys for the workstation that
produces the images, from the same context paths.
Also here:
- `DOMAIN` gets the same `:?` guard. Blank did not fail — it produced `https://` for
the frontend's ORIGIN and collapsed the Caddyfile's site block into a malformed
global block, so the stack came up with no TLS and no site.
- `stop_grace_period: 20s` on the app. Docker's default stop timeout is 10s, exactly
the app's own drain budget, so a redeploy could SIGKILL the process at the moment it
was finishing — truncating the in-flight upload the graceful shutdown protects.
- `COMMENTS_ENABLED` is pinned "false" alongside MEDIA_PATH. It is a product decision
for this event, and `.env.example` ships the generic `true`; pinning it means an
operator who copies the example and edits only the secrets cannot ship comments on.
- The frontend runtime stage now copies the lockfile and uses `npm ci`. Without it the
three `^`-ranged deps re-resolved at build time, so an image rebuilt days later could
differ from the one that was tested. Image also drops 120 MB -> 65 MB.
- `docker-compose.dev.yml` told the operator that production had the same `$`-eating
bug and to escape the hash as `$$` in `.env`. That is wrong and it breaks a working
deployment: Compose uses single-quoted env_file values literally, and doubling
produces a 74-character string `looks_bcrypt` rejects. Verified with `printenv`.
The runbook's rollback pointed at `v0.12.0`, which has 6 migrations against HEAD's 22
and was never built or pushed — running the emergency card's rollback line would have
crash-looped the app with `VersionMissing` during the event. §9 now has you tag one
build twice so the rollback target is bit-identical, and says plainly what that can and
cannot fix. Every `$DOMAIN` command gained the `set -a; . ./.env` it needs, the
down-migration psql commands are wrapped in `sh -c` so the container expands the
credentials rather than sending `-U ""`, and the advice to lower `max_video_size_mb`
is withdrawn: the client guard it was premised on does exist, but is pinned to a
compile-time constant, so lowering the DB value only moves failures later.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
MechaCat02
|
117e67fa80 |
fix(auth,upload): close the admin lockout and four unbounded-input paths
ADMIN LOCKOUT. admin_login looked its user up BY NAME. Migration 007 makes
display_name unique per event case-insensitively and join had no reserved-name
guard, so any guest joining as "admin"/"Admin"/"ADMIN" before the operator's first
login made find(role == Admin) miss, the fallback create("Admin") violate that
index, and `?` return a 500 — permanently, with no in-app recovery. Moderation,
config and gallery release all gone; the fix was hand-editing the database.
The root cause is the lookup key, not the creation. The name was never the
identity. User::find_admin_for_event resolves by role, which makes the whole class
of name collisions irrelevant — including the homoglyph bypasses of the new
reserved-name list, which is now defence in depth rather than the control.
Promoting the squatting row would be the obvious fix and is a serious mistake: it
carries a recovery_pin_hash the guest knows, so it would hand them the admin
dashboard via /recover, permanently, through a path needing no password. A
separate row under a fallback name is worse UX and much better security. Verified
against the real schema — the guest keeps their uploads, PIN and session under a
freed name, and the role lookup then finds exactly one admin.
Second, independent bug in that block: create() followed by a SEPARATE UPDATE ...
SET role = 'admin' manufactures the same poisoned state if anything fails between
them. Collapsed into create_with_role.
UNBOUNDED INPUTS — one root cause, four places: validation ran after the
allocation.
- upload caption/hashtags used Field::text(), which buffers the whole field, on
the one route whose DefaultBodyLimit is 576 MiB — so 576 MiB of heap per
concurrent request in a 1 GiB container, with the length check running
afterwards on a string already built. Now refused mid-read.
- the hashtag CSV was never length-checked at all and was upserted tag by tag
INSIDE the commit transaction, which holds FOR SHARE on the event row — one
request could stall every other upload behind tens of thousands of round trips.
Capped at 30 tags of <=50 chars.
- /recover and /recover/request built rate-limiter keys by format!() from an
unvalidated, unbounded display name, retained up to 24h in a map pruned hourly:
the limiter itself became the memory-exhaustion primitive it exists to prevent.
join validated first; that check is now shared by all three. /recover/request
also had no per-IP ceiling at all — /join got one in 017, /recover in 019, and
019's own comment describes exactly this attack. It returns 204 rather than 400
on a bad name, because a 400 would be a new signal on an endpoint whose contract
is that it cannot enumerate guests.
- the SSE ticket store had no size cap, no per-session cap and no rate limit on
its endpoint, while prune ran hourly against a 30s TTL. Now pruned on issue,
capped, and rate-limited. At capacity it REFUSES rather than evicting a
stranger's ticket — evicting would let one client deny SSE to the venue. Not
one-ticket-per-session either: two tabs open their EventSources concurrently.
PATCH /upload/{id} had no rate limit, no validation, and called
invalidate_and_arm unconditionally — outside both `if let Some` guards. So
PATCH {} bumped export_epoch and armed a fresh pair of full-gallery export workers
every call; REGEN_DEBOUNCE bounds the rate of that, not the total work, so a guest
could keep the keepsake permanently un-downloadable. All three fixed. The
validation also resolves a divergence: upload normalised tags while edit stored
them raw, so #Party via edit and party via upload became two hashtag rows.
PIN LOCKOUT was an ordering bug before a policy one: the account-lock threshold
(3) sat BELOW the per-(IP, name) ceiling (5), so three requests from one IP locked
any guest whose name is on the feed, every 15 minutes, forever. The tier meant to
protect a guest was the cheapest way to attack them. Ceiling drops to 4, threshold
rises to 12, so locking a victim now needs at least three distinct sources.
Brute-force cost is unchanged — 48 attempts/hour means 10k PINs still take ~208h
regardless of IP count — and increment_failed_pin now decays the streak after 15
minutes, since the counter previously only cleared on success and honest typos
accumulated across days. Both invariants are pinned by tests rather than comments.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
MechaCat02
|
f275de5c8f |
perf(feed): stop every feed page from aggregating the whole event
v_feed computed like_count/comment_count with LEFT JOINs and a GROUP BY. Postgres CAN push `event_id = $1` and the keyset predicate through the view — verified with EXPLAIN, it uses idx_upload_event_created_id — but it CANNOT push ORDER BY ... LIMIT across a GroupAggregate. So every request aggregated every upload in the event, times its likes and comments, and only then sorted and took 21 rows. Cost grew with the event, not with the page. Measured on a throwaway database seeded to a real reception (1000 uploads, 100 guests, 27k likes, 10k comments), same query, same data: before GroupAggregate (actual rows=1001) -> Sort -> Limit 449 ms after Index Scan (actual rows=21) -> Limit -> SubPlans 0.58 ms migration 022 replaces the joins with correlated scalar subqueries, which puts the counts ABOVE the Limit so they run 21 times instead of 1001. Exactly equivalent, not merely close: "like" is keyed (upload_id, user_id) so COUNT(DISTINCT user_id) == count(*), comment.id is the PK so COUNT(DISTINCT c.id) == count(*), and the GROUP BY was on u.id so it was already one row per upload. Column names, order and types are unchanged, so no Rust changes. No new index needed — idx_like_upload and idx_comment_upload already match the subqueries. Note the existing load harness cannot see any of this: e2e/loadtest/driver.mjs creates no likes and no comments, so the expensive path had never been exercised. The amplifier, feed/+page.svelte: every open feed subscribed to `upload-processed` and refetched page 1 — the most expensive page — so ~100 open feeds each fired one per completed upload. Now gated on whether this client actually shows the card that changed, and the debounce is jittered, because a fixed delay just moves a simultaneous herd 800 ms later. Nothing is lost by skipping: a client without the card also missed its `new-upload`, and the reconnect `feed-delta` already schedules a refresh. Load shedding, because the above reduces the risk rather than removing it: db.rs set no acquire_timeout, so sqlx's 30 s default applied — longer than the frontend's own 20 s fetch timeout, meaning the browser gave up while the server kept holding the slot and the work was done for nobody. Now 5 s, and PoolTimedOut maps to 503 + Retry-After instead of a generic 500. That mattered because the upload queue classifies 5xx as transient and retries: a 500 sent the retries straight back into the saturated pool with nothing to pace them. PoolClosed stays Internal — it only occurs during shutdown, where a 503 would invite a retry against a server that is going away. The Retry-After extraction in into_response matches on variants, so unlike message() a missing arm is not a compile error — it would silently drop the header. Pinned by a test covering both retry-carrying variants. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
MechaCat02
|
e52b2f1cd1 |
fix(upload): reclaim the bytes of uploads that never finish
Reclaim was a dozen explicit remove_file calls on the handler's return paths. That covers every way the handler can FINISH and none of the ways it can simply STOP. When a client disconnects mid-body — a phone leaving wifi, iOS killing a backgrounded PWA, the user hitting back — axum drops the handler future at an .await inside field.chunk() and no return path runs at all. The partial file then survives forever: it has no upload row, so cleanup_deleted_media (row-driven) can never see it, and no sweeper covered the originals directory. The triggers are routine rather than adversarial, and the client keeps the blob and auto- retries on every `online` event, so one large video over bad wifi leaves several copies. Those bytes are also invisible to the quota while still consuming the free disk that compute_storage_quota divides among guests — so orphans silently shrink every guest's ceiling while the admin widget under-reports. All three volumes share one filesystem; the end state is Postgres unable to write WAL. TempFileGuard is an RAII guard, because dropping the future is exactly what runs Drop — it is the only construct that survives cancellation. Armed before the file can exist, disarmed only after tx.commit() succeeds. The twelve explicit cleanups are deleted so one owner holds the rule. The subtler half is the rename. It happens BEFORE the commit, so between them the file exists under its final name with no row pointing at it — an orphan that looks legitimate. The guard is RETARGETED there rather than disarmed, and the retarget sits on the same poll as the rename with no .await between, which is what makes that window uncancellable. sweep_orphan_originals is the backstop for the process that was killed, where no Drop can run at all. Hourly, alongside the existing sweeps: .tmp files past the window go unconditionally (a .tmp never has a row by construction), other files are batched 500 at a time through a single NOT EXISTS query. Two things that look like oversights and are not, both commented in place: - the 6h window is what makes the sweep safe against the rename-before-commit ordering, since a committing upload is briefly indistinguishable from an orphan. It must not be shortened to speed up a test. - the NOT EXISTS deliberately does NOT filter deleted_at IS NULL. A soft-deleted row still points at its file during its retention window, and reclaiming that is cleanup_deleted_media's job; filtering here would race the two sweeps and destroy the files the recovery window exists to preserve. Verified against the real schema: given a live original, a soft-deleted one and a true orphan, the query returns only the orphan. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
MechaCat02
|
5969ec74ea |
fix(compression): bound derivative retries so one bad upload can't loop forever
The OOM in the previous commit was survivable; what made it an outage was that it repeated. The upload row is committed before compression starts, derivatives_rev defaults to 0, and set_derivatives_rev only runs on success — so a row whose processing killed the container survived at rev 0, and backfill_stale_derivatives (called unconditionally at every boot) re-selected it and re-ran the identical workload. With restart: unless-stopped that is an infinite kill loop, and every cycle also drops every SSE stream and truncates every in-flight upload. Verified end to end against the real schema in a scratch database: with the new guard the backfill selects the row on boots 1-3 and zero rows from boot 4 on, and a later success resets the counter. migration 021 adds derivative_attempts and derivative_last_error. The counter is incremented WRITE-AHEAD, before the work is attempted. This is the whole design: the failure being bounded is a cgroup SIGKILL, so no Err is returned, no error handler runs and no Drop fires. A counter bumped in a failure path increments zero times per crash and the loop would be unchanged. set_derivatives_rev clears it, so success is the only reset and both the live path and the backfill get it without a new call site to forget. Also in the backfill: - one task walking the rows sequentially instead of one task per row. A large backlog used to spawn thousands of tasks, each holding a pool handle and queueing on the same two permits, competing with live uploads for a whole boot. - LIMIT 200 per boot, and original_path <> '' replacing an IS NOT NULL that was dead (the column is NOT NULL; cleanup_deleted_media blanks it instead). - a once-per-boot error log naming how many uploads have given up. Without it the give-up is invisible — the loop stops, which is the point, but the photos keep a stale derivative forever with nothing to notice. Adds backfill_video_posters for the mirror-image gap: a video interrupted by a restart has its compression_status flipped processing -> failed by startup_recovery and is never re-enqueued, so thumbnail_path stays NULL for the rest of the event while the clip itself plays fine. It shares the same attempt budget, which means a genuinely posterless sub-second clip (Live Photo, mis-tap) stops being re-ffmpeg'd after three boots. That is intended, not a bug to fix later — Ok(false) is a normal permanent outcome there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
MechaCat02
|
6fd75adb27 |
fix(ops): bound logs, drain ffmpeg's stderr, and stop three silent hangs
Six independent operational defects, none of which needed a new feature to fix. Log rotation. Docker's json-file driver is unbounded by default, and those files land on the HOST filesystem — outside every deploy.resources.limits in the compose file, and on the same disk as postgres_data and media_data. A full disk stops Postgres writing WAL, which takes the event down. Capped at 10m x 3 per service. Log level. RUST_LOG was set in neither .env.example nor docker-compose.yml, so the code fallback WAS the production level — and it was `debug`, with tower_http=debug emitting a line per request and per response into that unrotated file. Now info, with tower_http=warn to state that those spans are diagnostics, not an access log. ffmpeg pipe deadlock. run_ffmpeg piped stdout and stderr and then called wait(), which drains neither. Once the ~64 KiB pipe buffer filled, ffmpeg blocked writing and wait() never returned — burning the full 120s timeout, twice per seek position, three times per compression attempt. And the timeout is an Err, so the end state was a soft-deleted upload: a guest's playable video destroyed by a poster-frame failure. Now stdout is null (nothing ever read it) and stderr is drained by wait_with_output, whose tail is logged on a non-zero exit. Note wait_with_output consumes the child, so the old kill-on-timeout is gone; kill_on_drop(true) already covers it. Readiness probe. /health never touched the pool, so the disk-full endgame above stayed green all the way down. Adds /health/ready (SELECT 1 under 2s) as a SECOND route — the compose healthcheck deliberately keeps pointing at /health, because caddy gates its startup on it and a DB-dependent probe would turn a Postgres blip into the reverse proxy refusing to start. api.ts request timeout. The abort timer was cleared in a finally around fetch(), which resolves on the response HEAD — leaving res.text() uncovered and no longer abortable. An upstream that sends headers then stalls the body hung the call forever. The timer now lives until the body is read, including the 204 path (which otherwise leaked a live 20s timer per no-content request). Upload XHR watchdog. The XHR had no timeout while processQueue held the isProcessing latch across it; on a half-open socket neither error nor abort ever fires, so the latch pinned and the queue wedged. Bounds SILENCE rather than total duration — a 500 MB video over a venue uplink legitimately runs 30+ minutes while making steady progress. Rejects as NetworkError, which is already the retryable branch, so a stalled upload now recovers like any network blip. IndexedDB failures. addToQueue called getDb() unguarded and handleSubmit had no catch, so a private-mode refusal or a QuotaExceededError on a large blob left a permanent "Wird hochgeladen…" spinner, no toast, and — for an in-app camera capture — the only copy of the photo gone. Now reported as 'failed', which keeps the staged files on screen and stays on the page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
fabi
|
402215d405 |
fix(video): extract a real poster frame, and stop claiming one that isn't there
Both the compression worker and the HTML export ran the same invocation:
ffmpeg -i <src> -vframes 1 -ss 00:00:01 -vf scale=… -y <out>
`-ss` AFTER `-i` is an output-side seek. Against a clip of a second or less ffmpeg
exits 0 and writes nothing, and both call sites gated on the exit status:
- the worker wrote `thumbnail_path` and logged "thumbnail generated" for a file that
was never created, so GET /upload/{id}/thumbnail 404s in the live feed;
- the export listed media/<id>_thumb.jpg in data.json while the ZIP writer skipped the
unopenable file, so the keepsake drew a broken image tile.
Any clip at or under a second, which phones produce constantly — mis-taps, Live
Photos, boomerangs. Not data loss; the .mp4 is in both archives and plays. Every
server-side signal stayed green.
New `services/video.rs` owns the extraction for both callers, mirroring the imaging.rs
precedent (created for the same duplication, and it paid off when the max_alloc fix
landed in both workers at once). Three changes in it:
- `-ss` before `-i`, an input-side seek. NOT sufficient alone: verified against the
production image, seeking to 1s in a 1.000s clip is still past the last frame and
still exits 0 with no file. The 0s fallback is what actually fixes this, and 1s is
tried first only because an opening frame makes a poor poster.
- Verify the artifact, not the exit status. This is the check both sites were missing.
- Carry compression.rs's 120s timeout. export.rs had NONE — a hung ffmpeg there would
strand the job at `running` and the keepsake would never complete.
The worker's call used `?`. Tightening the check without also making a missing poster
non-fatal would have been far worse than the bug: every sub-second clip would fail
compression, exhaust its retries and be soft-deleted. It now logs a warning and leaves
`thumbnail_path` NULL, which FeedListCard, VirtualFeed and LightboxModal already
handle.
The export now sets `thumb: ""` and skips the manifest entry when there is no poster —
and does the same for the IMAGE branch, whose decode failure left the identical
dangling reference. No viewer change was needed: +page.svelte already guards
`{#if post.media.thumb}` and falls back to a video tile with a play glyph. The comment
claiming "viewer handles missing thumbs gracefully" was true about the viewer and false
about what the backend sent — the guard never fired because the string was never empty.
e2e/specs/06-export/export-video.spec.ts had DOCUMENTED this as intended behaviour
("the fixture clip is <1s, so ffmpeg extracts no thumbnail frame — but exits 0 …
that's the intended shape here"). sample.mp4 is exactly 1.000s, so every video test in
the suite ran at that boundary and none ever fetched the poster. That comment is now
corrected to say what it actually was.
Tests: 2 unit; a new sample-5s.mp4 fixture so the ordinary first-seek path is covered
at all; video-playback now FETCHES the poster rather than asserting the attribute (the
one extra request that nine rounds of green never made); a new spec covering both
fixtures plus the mirror that a posterless video still uploads and plays; and a
keepsake spec asserting every <img> in the opened viewer resolves — naturalWidth === 0
is exactly the broken-tile case, whatever produced it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
fabi
|
3bcb7c6a76 |
fix(deploy): close the POSTGRES_PASSWORD trap that broke a fresh deploy either way
README step 2 said to set "DOMAIN, JWT_SECRET, ADMIN_PASSWORD_HASH, EVENT_NAME, etc." -- POSTGRES_PASSWORD was not in that list. .env.example said to set it and keep it in sync with DATABASE_URL. The two documents disagreed, and both readings ended badly. Branch A, README verbatim: the stack came up GREEN and healthy on CHANGE_ME_use_a_strong_password -- a database credential published in the public repo. The production secret guard covered JWT_SECRET and ADMIN_PASSWORD_HASH, and nothing anywhere looked at the Postgres password. Branch B, .env.example verbatim: a permanent restart loop, "password authentication failed for user eventsnap". The guard's own design made Branch B near-certain. It stops the APP on the first `docker compose up -d` -- but not the `db` service in that same command, which initialises its data directory and bakes in whatever password was in .env at that moment. POSTGRES_PASSWORD is honoured ONLY at initdb. So the intended recovery -- see the refusal, fix your secrets, boot again -- was exactly the sequence that broke it. Nothing in the error named the cause, and the remedy (`down -v`) is both unguessable and the one command you must never run once real data exists. Three changes, which have to ship together: the guard alone would just move operators out of Branch A and into Branch B. - The guard now rejects a placeholder DATABASE_URL in production (the password rides in that URL, which is what the app actually reads). Branch A can no longer boot. - It reports EVERY unset secret in one message instead of returning on the first. Fixing two secrets used to cost two boot cycles, on a stack where Caddy waits on the unhealthy app throughout, and each avoidable cycle is another chance to reach for -v. - A 28P01 handler in db.rs turns the unguessable failure into a self-explaining one: it names the initdb semantics, gives `down -v` with an explicit "deletes db + media + exports, no undo", and gives the ALTER ROLE alternative for when data already exists. Docs: step 2 now names POSTGRES_PASSWORD and says every secret must be set BEFORE the first up; the troubleshooting block covers the auth-failure loop and both remedies. Also replaces htpasswd (apache2-utils -- not on a stock VPS) with `docker run --rm caddy:2-alpine caddy hash-password`, an image the stack already pulls, in README, .env.example and the guard's own message. Verified the output ($2a$14) against the shipped $2y$12 example. Verified on a real Postgres, not just in tests: Branch A refuses and names DATABASE_URL; all three placeholders report in one boot; a volume initialised with one password and connected to with another prints the diagnostic; an unrelated connect failure (dead port) stays silent. 8 unit tests, including that non-prod ignores all of it so the e2e stack is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
fabi
|
08d92b7531 |
fix(audit-7): keepsake viewer integrity, readable archives, quota_tolerance floor
Seventh round, both defects in the keepsake — the one artifact that leaves the
system entirely, and so the one where a green server-side signal carries no
information at all.
Squashed from 2 commits, original messages preserved below.
──────── fix(export): stop a caption bricking the viewer, and ship readable archives
Two defects in the keepsake, both silent server-side and both only visible by
extracting the real artifact and trying to use it.
1. A CAPTION COULD BRICK THE VIEWER.
The viewer's data is inlined as `<script>window.__EXPORT_DATA__={…}</script>` -- it has
to be, since guests open index.html over file:// where fetching a sibling data.json is
blocked. The escape was `</` -> `<\/`. Against XSS that holds; I fired
`</script><img src=x onerror=…>` through a real Chromium parser and it round-trips
inert.
It does not stop the caption steering the HTML TOKENIZER. `<!--<script` with no later
`-->` drives the parser into script-data-double-escaped state, where the template's own
`</script>` only steps back to script-data-escaped instead of closing the element.
Everything after it -- including the viewer bundle -- is swallowed as script data.
Nothing executes and nothing leaks: `__EXPORT_DATA__` is simply never assigned and the
keepsake opens blank. A denial of the deliverable, not an XSS.
Reproduced in Chromium before changing anything, and the near-miss is worth recording:
`<!--<script>alert(1)</script>-->` comes back CLEAN, because the trailing `-->` returns
the parser to script-data state. A probe using the terminated form quietly repairs the
thing it is testing for.
Fix: escape every `<` as `<`, not just `</`. `<` never appears in JSON structural
syntax -- only inside string values -- so a global replace is sound, and one rule covers
`</script`, `<!--` and `<script` together. That is the point: the old escape was named
for the single case it handled. Only the INLINED copy is escaped; data.json is written
separately, in no HTML context, and stays literal.
2. EVERY ENTRY IN BOTH ARCHIVES WAS STORED MODE 0000.
`ZipEntryBuilder::new` leaves the external file attribute at zero and async_zip's host
compatibility defaults to Unix, so `unzip -Z` showed `?---------` on every line of both
Gallery.zip and Memories.zip. Windows Explorer ignores Unix modes, which is why this
survived; on Linux and macOS `unzip` faithfully applies what the archive asks for and
the guest gets a folder of photos none of which they can open.
Unconditional -- every keepsake ever produced, no hostile input required -- and
invisible server-side: the export succeeds, the ZIP is well-formed, the job writes
`done`, /export/status is green.
Found by accident. The browser test for defect 1 failed with ERR_ACCESS_DENIED on
file://, which looked exactly like a Playwright sandbox quirk; I twice "worked around"
it (fresh context, then a separately launched browser) before checking the extracted
files and finding mode 000. The workaround was suppressing a real bug. Both workarounds
are gone -- the ordinary `page` fixture loads the archive fine now.
Fix: all six ZipEntryBuilder sites route through one `keepsake_entry` helper stamping
`S_IFREG | 0644`, so the mode cannot be forgotten at a call site.
Tests: 3 unit (no `<` survives; the payload still decodes to the original value, because
this is a transport encoding and not a sanitiser; a clean payload is untouched) and 2
e2e that release for real, download the real archives, and check them from outside the
app -- one opening index.html over file:// in Chromium and asserting the viewer booted,
the captions came back verbatim and nothing executed; one asserting every stored mode
and every extracted file is readable. Both assertions verified to FAIL against the
pre-fix artifacts.
──────── fix(admin): reject quota_tolerance = 0 instead of silently blocking every upload
Zero is inside the documented 0–1 range and catastrophic. The per-user limit is
`free_disk * tolerance / active_uploaders`, so a tolerance of 0 makes every limit 0 and
refuses EVERY upload -- mid-event, with "Du hast dein Upload-Limit für dieses Event
erreicht", an error naming the wrong cause entirely. An admin reaching for an off-switch
wants `storage_quota_enabled`; the rejection now says so.
Rejecting the value rather than raising the floor. A floor of 0.01 was the obvious fix
and it is wrong: very small tolerances are legitimate -- they are how a large disk is
throttled down to a sensible per-guest ceiling, and how the quota specs steer it
(tolerance = target * active / free lands around 1e-5 on the 174 GB volume this suite
runs on). A floor would forbid real configurations, and would have broken the entire
storage-quota describe block, to prevent one typo. Verified: those four tests still pass.
Tests: the rejection, that the stored value is untouched (validation fully precedes any
write), and the mirror -- 0.00001 still round-trips -- so the guard can't quietly become
a floor later.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
fabi
|
06bc9ddcb3 |
refactor(export): share the visibility filter between the row query and the estimate
`query_uploads` selects the rows the archives are built from; `estimate_export_bytes` sizes them for the disk preflight. They stated the same WHERE clause separately, and the direction of drift matters: an estimate that MISSES rows the archive writes under-reserves, which is precisely the ENOSPC the preflight exists to prevent. The integration test claimed to guard this and cannot. Both sides of `the_estimate_sums_exactly_the_rows_the_archive_will_contain` are `SRC:`-marked hand-copies in tests/common/mod.rs -- neither is production code -- so drift means production moved while both copies sat still, and the test goes on passing. The convention is sound for pinning behaviour; it is structurally incapable of detecting divergence from the thing it copies. So fix it where it can be fixed. One `export_visibility_where!()` fragment, `concat!`-ed into both queries at compile time (still `&'static str`, no allocation), with the `u`/`usr` alias contract stated. Divergence is now impossible by construction rather than watched for. The tests keep their value and lose the overclaim: the docstrings now say they pin WHICH uploads may be counted -- each excluded row in the fixture is excluded by a different predicate, so weakening any one of them still fails here -- and say plainly that they do not detect drift, with a pointer to what does. No behaviour change. The filters were verified identical before the hoist (`u.event_id = $1 AND u.deleted_at IS NULL AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE`); 99 backend tests still pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |