fabi/EventSnap
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Compare commits

base: fabi:fix/deploy-unattended-blockers
Branches Tags
fabi:main fabi:fix/production-readiness-audit fabi:worktree-agent-afef83dfda4730d6f fabi:fix/deploy-unattended-blockers fabi:fix/video-thumbnail-seek fabi:fix/track-e2e-fixtures fabi:fix/postgres-password-deploy-trap fabi:backup/pre-squash-20260729-2208 fabi:fix/reject-zero-quota-tolerance fabi:fix/keepsake-viewer-integrity fabi:refactor/share-export-visibility-filter fabi:fix/video-poster-race fabi:chore/db-memory-and-social-rate-limits fabi:docs/backup-restore-and-quota-tolerance fabi:feat/low-disk-warning fabi:fix/reclaim-deleted-originals fabi:fix/export-disk-preflight fabi:fix/mobile-flake-ssr-2026-07-16 fabi:fix/audit-2026-06-27-critical-medium
fabi:v0.12.0 fabi:v0.11.0 fabi:v0.10.0 fabi:v0.9.0 fabi:v0.8.0 fabi:v0.7.0 fabi:v0.6.0 fabi:v0.5.0 fabi:v0.4.0 fabi:v0.3.0 fabi:v0.2.0 fabi:v0.1.0
..
compare: fabi:chore/db-memory-and-social-rate-limits
Branches Tags
fabi:fix/production-readiness-audit fabi:main fabi:worktree-agent-afef83dfda4730d6f fabi:fix/deploy-unattended-blockers fabi:fix/video-thumbnail-seek fabi:fix/track-e2e-fixtures fabi:fix/postgres-password-deploy-trap fabi:backup/pre-squash-20260729-2208 fabi:fix/reject-zero-quota-tolerance fabi:fix/keepsake-viewer-integrity fabi:refactor/share-export-visibility-filter fabi:fix/video-poster-race fabi:chore/db-memory-and-social-rate-limits fabi:docs/backup-restore-and-quota-tolerance fabi:feat/low-disk-warning fabi:fix/reclaim-deleted-originals fabi:fix/export-disk-preflight fabi:fix/mobile-flake-ssr-2026-07-16 fabi:fix/audit-2026-06-27-critical-medium
fabi:v0.12.0 fabi:v0.11.0 fabi:v0.10.0 fabi:v0.9.0 fabi:v0.8.0 fabi:v0.7.0 fabi:v0.6.0 fabi:v0.5.0 fabi:v0.4.0 fabi:v0.3.0 fabi:v0.2.0 fabi:v0.1.0

51 Commits
fix/deploy ... chore/db-m

Author SHA1 Message Date
fabi
35390800c7 chore: raise the db memory limit and rate-limit social writes 
Two smaller operational items.

POSTGRES 512M -> 1G. DATABASE_MAX_CONNECTIONS is 30 for a ~100-guest event (feed
polling + SSE + uploads at once), and 30 backends plus Postgres 16's default
shared_buffers leaves very little headroom at 512M. An OOM here doesn't degrade one
feature -- every request path touches the database, so it takes the event down.
Memory is the cheaper knob than shrinking the pool back and reintroducing the
queueing it was raised to fix. .env.example now names the pairing explicitly, the way
it already does for COMPRESSION_WORKER_CONCURRENCY.

SOCIAL WRITES WERE UNTHROTTLED. toggle_like, add_comment and delete_comment were the
only mutating endpoints in the app with no limit at all -- upload, join, recover,
export and admin login all carry one. Asymmetric coverage rather than a deliberate
decision.

Low severity, and honestly so: a like fans an SSE broadcast to every client, but the
export regeneration a comment deletion triggers is contained (REGEN_DEBOUNCE 20s,
workers born with their epoch, superseded ones inert). So the ceiling is 120/min --
far above anything a real guest produces. This bounds a script, not an enthusiastic
double-tapper.

ONE bucket across all three actions: separate buckets would let a caller triple the
aggregate write rate by alternating between them. Keyed per USER, matching the feed
and upload limits -- at a venue every guest is behind one NAT, and an IP key is what
made the /join and /feed limits turn guests away in the first place.

Migration 020 seeds both keys, and both are wired into the admin allowlist, the
config UI and the e2e reseed -- the step two earlier per-area toggles missed, which
left switches that existed in code and could never be flipped.

Tests: 4 e2e, including that the shared bucket really is shared (the part most likely
to be lost in a refactor) and that one guest hitting the ceiling doesn't block
another behind the same IP.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
 2026-07-29 19:57:34 +02:00
fabi
14ebe1e543 Merge branch 'docs/backup-restore-and-quota-tolerance' 2026-07-29 19:51:57 +02:00
fabi
a4a4e46c53 docs: add a restore procedure, fix the backup cadence, and correct quota_tolerance 
Four things, all found by the same question: what does an operator standing at the
venue actually need?

A RESTORE PROCEDURE. There was none anywhere, and a backup you have never restored
isn't a backup. Two hazards worth writing down: media must be extracted preserving
ownership (the app runs as uid 100 / gid 101, and a root-owned restore makes every
upload fail with EACCES surfacing as a generic 500), and the app must be STOPPED
first, because migrations run on boot and a live pool will fight the restore.

Both the backup and the restore commands were run against the real stack before being
written down, which caught two that would have failed:

  - The plain `pg_dump` did not restore: `psql` aborted on `ERROR: schema
    "_sqlx_test" already exists`. pg_dump emits no DROPs without --clean --if-exists,
    so the documented dump could only ever be restored into an empty database. Fixed
    at the source (the dump is now self-cleaning) and verified end to end: 16 tables
    back, exit 0.
  - `--same-owner` does not exist in BusyBox tar, which is what `alpine` ships, so
    the extract aborted before unpacking anything. `--numeric-owner` plus the
    explicit chown, verified to land 100:101.

BACKUP CADENCE. "Weekly offsite" is the wrong shape when every irreplaceable byte is
created in one eight-hour window and nobody can retake a wedding. The backup that
matters runs that night, and again after the release so the keepsake is captured.
Also: take the DB dump and the media tarball back to back, or you get rows pointing
at files the dump doesn't know about.

quota_tolerance WAS DOCUMENTED AS SOMETHING IT ISN'T. .env.example called it "fraction
of disk that triggers the low-storage warning". It is the multiplier in
`floor(free_disk * tolerance / active_uploaders)` -- so an operator who wants "warn me
later" and sets 0.95 is actually authorising guests to fill 95% of the disk, moving
the fixed point from 43% to ~49% and eating the export headroom. The admin UI labelled
it "Toleranz (0-1)" with no explanation at all, which invites exactly that reading;
it is now "Speicher-Anteil für Gäste" with the formula in the hint. Wrong docs on a
tuning knob are worse than no docs.

SIZING. New section with the arithmetic: three volumes on one filesystem, the quota
fixed point at tolerance/(1+tolerance), and the fact the 80 GB baseline does not cover
the keepsake -- both archives are built concurrently and each is roughly a second copy
of every original. Provision ~3x expected media, or give exports its own volume.

Also ticks the low-disk alert off the roadmap, since it now exists.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
 2026-07-29 19:51:48 +02:00
fabi
e6e8a52d87 Merge branch 'feat/low-disk-warning' 2026-07-29 19:48:06 +02:00
fabi
43c2a0d09c feat(host): warn about low disk before it becomes unrecoverable 
Storage visibility existed in exactly one place: a passive Speicherauslastung widget
on the ADMIN dashboard. A host who isn't the admin had no view of it, and nothing
warned anyone. README carried "Low-disk alert (< 10 GB free)" under Planned since v1.

Two things make this a safety net rather than a nice-to-have. postgres_data,
media_data and exports_data are all Docker named volumes on ONE filesystem, so
running out doesn't degrade a subsystem -- Postgres stops being able to write and the
whole event goes down. And the keepsake needs room for two gallery-sized archives,
which the export preflight can only ever refuse AFTER the release, when the event is
over and every remedy is harder.

So the threshold is not a fixed number alone. It fires on the 10 GB floor the README
always named, OR on "you could not build the keepsake right now" -- the trigger a
host can still act on, computed with the same arithmetic the preflight uses. Unknown
free space is NOT low: it fails open like the upload quota and the preflight do,
because a banner that cries wolf on an unreadable mount is a banner nobody reads.

Carried on GET /host/event, which the dashboard already fetches on load and on every
reload -- no new endpoint, no new poll. Rendered above everything else including the
PIN-reset queue, and it names the consequence (the event, not just the download)
rather than only the number.

Also fixes the host page's formatBytes, which topped out at MB: 30 GB free would have
rendered as "30720.0 MB", and a guest with 2 GB of uploads was already being shown
that way in the user list.

Tests: 5 unit on the threshold (including that plenty of free space is still low when
the keepsake wouldn't fit -- the case a fixed threshold misses entirely), 3 e2e.
The e2e drives it through `original_size_bytes` rather than a genuinely full disk:
the estimate is pure SQL over that column, so overstating one row moves the
accounting without touching a byte on disk.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
 2026-07-29 19:48:06 +02:00
fabi
6818cabf91 Merge branch 'fix/reclaim-deleted-originals' 2026-07-29 19:42:03 +02:00
fabi
f777764839 fix(maintenance): reclaim the media of deliberately deleted uploads 
The quota stopped bounding the disk. `soft_delete_in_event` stamps `deleted_at` and
refunds `total_upload_bytes`, but nothing ever removed the bytes, and the hourly
sweep reached only `compression_status = 'failed'`. Upload 500 MB, delete, quota back
to zero, upload another 500 MB. Not an attack -- a guest curating their camera roll,
which is what people do. The host then sees guests hitting "Du hast dein Upload-Limit
erreicht" while the admin widget shows a disk full of files no upload row points at,
and the quota message is actively misleading because the space really is gone, just
not to anyone the accounting can name.

Two retention windows, because the two deletes mean different things. A compression
failure keeps its 14 days: the guest didn't ask for it and may not be able to retake
the photo. A deliberate removal gets 24 hours -- 14 days outlives the whole event, so
a deliberate delete would never reclaim anything while it mattered, and a day still
covers a mis-tap.

Wider than reported: ALL FOUR paths are reclaimed, not just the original. Preview,
display and thumbnail are each a separate file, none counted in
`original_size_bytes`, and nothing ever removed them either. That was invisible while
the sweep only saw failed compressions (which produce no derivatives) and becomes
three leaked files per upload the moment it reaches a successful one. A row is
re-selected until every path is cleared, and the columns are cleared only once every
file for that upload is gone -- clearing after a partial success would strand the
survivors in exactly the unowned state this drains.

`backfill_stale_derivatives` selects on `display_path IS NULL AND preview_path IS NOT
NULL`, which is close enough to the post-sweep state to be worth pinning: it is
guarded on `deleted_at IS NULL`, so it cannot re-decode an original that is no longer
on disk. Covered.

Residual, deliberately: within the 24h window the bytes are still spent and still
unaccounted, so delete-and-re-upload through an eight-hour event can outrun the
sweep. Bounding that means holding the quota until the file is reclaimed rather than
refunding at `deleted_at`. The low-disk warning is the net under it.

Tests: 6 DB-backed, replacing 3. The one asserting an owner-deleted upload IS
reclaimed is the exact inverse of what this file used to assert.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
 2026-07-29 19:41:51 +02:00
fabi
aeb958f6ba Merge branch 'fix/export-disk-preflight' 2026-07-29 19:38:39 +02:00
fabi
281eb3bec7 fix(export): refuse an export that cannot fit, and stop peaking at two generations 
Nothing in export.rs ever asked whether the keepsake would fit. Both archives write
their media `Compression::Stored`, so each is essentially a byte-for-byte second copy
of the originals -- Gallery.zip always, and Memories.zip for every video and every
image at or under 5 MB. On the documented CX33 (80 GB, all three volumes on one
filesystem) the upload quota's fixed point leaves ~40 GB free, and a release spawns
BOTH halves concurrently against it.

The failure is not "the export failed", it is "the deliverable is stuck":

  1. ENOSPC lands partway through a multi-GB write.
  2. The epoch has already moved, so the job row is `failed` at the CURRENT
     generation and readiness (epoch = event.export_epoch AND status = 'done') is
     false -- GET /export/zip 404s.
  3. The last good archive sits on disk, unreferenced and unreachable.
  4. POST /host/export/rebuild, the only escape, re-arms the same doomed write.

Three changes.

Reclaim before building. `prune_stale_export_files` ran only after the new archive
was written, renamed and finalised. That reads as durability but buys nothing: the
moment `invalidate_and_arm` bumps the epoch the old archive is ALREADY unreachable,
so keeping it reserves gigabytes for a download nobody can perform -- and for a
takedown it is content someone explicitly asked to have removed. Peak usage is now
one generation. Narrower than the post-finalize prune on purpose: final archives
only, never a `.tmp` or a `viewer_tmp_` dir, since a superseded worker can still be
streaming into those and at build START is far more likely to be alive.

Preflight the space. SUM(original_size_bytes) over exactly `query_uploads`'
visibility filter, +10% for ZIP overhead, multiplied by the number of armed jobs --
without that multiplier each of the two concurrent halves independently sees "it
fits" and together they don't. Runs AFTER claim_job, not before as reported: bailing
before the claim leaves the row `pending` with no worker and no error, the
spinner-forever state `mark_failed`'s status guard exists to prevent. Fails open when
the mount can't be read, exactly as the upload quota does.

Show the host the reason. /export/status returned {status, progress_pct} and nothing
else, so the host dashboard could only render "fehlgeschlagen" next to the retry
button. The message was written to the row and surfaced solely in the ADMIN job list
-- a different screen, possibly a different person. It now travels with the status,
and only on a failure, so a message left on a since-succeeded row can't appear beside
a green "ist bereit".

Tests: 10 unit (the u128 clamp caught a real bug in the first draft -- saturating_mul
then /100 turns an overflow into a number ~100x too small, the one direction that
authorises the write being guarded against; the carried-forward archive must survive
its own older epoch in the filename), 4 DB-backed (the estimate is asserted against
the row set the archive actually contains, not against a restatement of the WHERE
clause, so the two queries cannot drift), 3 e2e over the four-hop plumbing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
 2026-07-29 19:38:26 +02:00
fabi
8c93cbb045 Merge branch 'fix/narrow-admission-check' 2026-07-29 07:55:56 +02:00
fabi
ceb68939a7 fix(upload): narrow the admission check to the memory budget only 
The admission check I just added rejected ANY image the decoder couldn't build —
corrupt, truncated, or unsupported, not only over-budget. That broke two
adversarial tests, and they were right to break.

07-adversarial/file-upload-attacks pins, deliberately, that acceptance follows the
MAGIC BYTES: a payload whose first three bytes are a JPEG header is accepted
regardless of what follows, because the security property under test is that the
client-declared Content-Type has no influence. Both failing cases upload 1024
bytes of JPEG magic followed by zeros. Rejecting those at admission is a
different, broader contract than the one asked for, and rewriting an adversarial
test to match new behaviour is precisely the thing that needs justifying rather
than doing quietly.

So admission now checks only what it was meant to: `exceeds_decode_budget`
returns true solely for `ImageError::Limits`. A corrupt file goes to the
compression worker exactly as before — which handles it gracefully and, since the
retry classifier in the previous commit, no longer burns backoff on it. The
resource guard is the part that had to move earlier; nothing else did.

Tests: the size agreement between admission and the worker is still asserted in
both directions, plus a new one writing a magic-bytes-only stub and asserting
admission accepts it WHILE the worker still rejects it — pinning the boundary
between the two checks so a future widening fails here rather than in the
adversarial suite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-29 07:55:56 +02:00
fabi
674ea87bbd Merge branch 'fix/no-retry-on-permanent-failure' 2026-07-29 07:29:21 +02:00
fabi
fae12bd7ec fix(upload): refuse undecodable images at the door, and stop retrying them 
Two halves of the same complaint: an oversized photo was accepted with a 201 and
then silently soft-deleted minutes later, after the worker had burned six seconds
of backoff re-reaching a conclusion it could not change.

Admission. The compression budget now runs at upload time, against the header
only, so a guest is told immediately and told why:

  "Bild hat zu viele Bildpunkte (ca. 99 Megapixel) und kann nicht verarbeitet
   werden. Bitte verkleinere es und lade es erneut hoch."

instead of watching the photo vanish behind a vague "could not be processed" —
which arrived only if they happened to still be on the feed with that card
loaded. Nothing is stored, so there is no row to soft-delete and no orphan for
the sweep to reclaim.

Admission and the worker share ONE function (`decoder_within_budget`), so they
cannot drift apart and start disagreeing about what is acceptable — a photo
accepted at the door and rejected by the worker would be worse than either
behaviour alone. The worker keeps its own check: the backfill decodes files that
predate this check, and defence in depth is the whole reason the budget exists.

Retries. The loop retried every failure, including ones that are a property of
the input. An image over the budget, a corrupt file, an unsupported format: each
fails identically on all three attempts, so the only effect was 2s + 4s of sleep
and three near-identical warnings before the same outcome. `is_permanent_image_error`
classifies the `ImageError` variants that cannot change between attempts — Limits,
Unsupported, Decoding — and the loop gives up on those at once. `IoError` is
deliberately excluded: an ENOSPC while writing a derivative is exactly the
transient case the retry exists for, and misclassifying it would turn a blip back
into the data loss round 1 fixed. Measured: retry log lines went from 3 per
oversized upload to 0.

Tests: unit tests for both sides of the classifier (a Limits error is permanent, a
missing file is not) and for admission agreeing with the decoder on accept AND
reject. The e2e spec is rewritten for the new contract — 400 with an actionable
message, nothing stored, backend alive after a burst of four — plus a mirror
asserting an ordinary photo still uploads and processes, since a budget that
rejected everything would satisfy the other two.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-29 07:29:21 +02:00
fabi
2bd54d7f0b Merge branch 'chore/unshare-permission-settings' 2026-07-29 07:18:08 +02:00
fabi
528960d201 chore: take the Bash(*) permission change back out of the shared settings 
`.claude/settings.json` is committed and applies to anyone who clones. Fabi's
local `allow: ["Bash(*)"]` plus deny list ended up in it, inside f0d69f1 — a
commit about the image decode guard, which has nothing to do with permissions.

That was my mistake, twice. The file was already modified when I started the
round: my `git status --short` check printed "(clean)" from an unconditional
`echo` rather than from the status output, so I read a dirty tree as clean. Then
`git add -A` swept it into an unrelated commit, and I reported afterwards that I
had left it untouched. Neither the check nor the claim was true.

Restores the shared file to its previous three narrow entries. The permission
setup itself is preserved, moved to `.claude/settings.local.json`, which
`.gitignore:34` covers precisely so per-user permissions stay per-user — the
existing 442 entries there are kept alongside it.

Not rewriting f0d69f1 to erase this: main is unpushed so it would be safe, but a
visible correction is worth more than a tidy history, and a rebase across the
merge commits carries more risk than the mistake does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-29 07:18:08 +02:00
fabi
6b7da8fb07 Merge branch 'fix/deploy-health-and-caddy-reload' 2026-07-28 22:34:01 +02:00
fabi
faf2e62a29 fix(deploy): route /health in production, and actually apply Caddyfile changes 
Two defects in the update procedure I wrote last round, both of which make a
successful-looking deploy a lie.

1. The documented health check could never pass.

`curl -fsS https://DOMAIN/health` 404s against a perfectly healthy production
stack. The backend registers /health on its ROOT router, not under /api/v1, and
the production Caddyfile proxies only /api/* and /media/* — so /health fell
through to the SvelteKit catch-all, which has no such route and returns its 404
page. With -f, curl exits 22 and the `&& echo` never runs. My own gloss
("Anything other than ok means check the logs") then sent the operator chasing a
phantom outage.

e2e/Caddyfile.test has carried `reverse_proxy /health app:3000` since it was
written — precisely because the catch-all would otherwise swallow it. Production
never did. Per the fix-the-gap-not-the-doc call, production gets the same line,
and /health joins the no-store matcher so a cached response can't report the last
known state instead of the current one. Verified by running the production
Caddyfile against the real backend: /health -> 200 "ok", Cache-Control: no-store,
with /api/v1/event and / unaffected.

2. The sequence never reloaded Caddy, so a Caddyfile-only change was dropped.

`--build` only rebuilds services with a `build:` section, and caddy is a pinned
upstream image. Compose decides whether to recreate a container from its config
hash, which covers the mount SPECIFICATION but not the mounted file's CONTENTS —
so a git pull that changes ./Caddyfile produces no delta, Compose reports
`Running`, and Caddy serves its old config indefinitely. Exit code 0 throughout.

Round 1's iOS download fix (137c4ee) is exactly this shape: Caddyfile plus four
e2e files, so 100% of its production effect is in that one file. Following the
README to the letter deployed it, showed both image IDs changing, and left iOS
downloads broken.

Demonstrated rather than assumed — added a probe header to a Caddyfile, ran the
old sequence (`up -d --build`): header absent, change silently dropped. Ran the
new step 4 (`up -d --force-recreate caddy`): header served.

`--force-recreate` rather than `restart` or `caddy reload` because the bind mount
is resolved to an inode at container-create time and git pull replaces the file
rather than editing in place, so a restart can re-read the stale content — the
exact failure I hit in round 1 when `caddy reload` didn't pick up an edit.

Also rewrites the "db and caddy are untouched … so data volumes survive" sentence.
I wrote it as reassurance; "caddy is untouched" was the bug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 22:34:01 +02:00
fabi
6d5c488e14 Merge branch 'fix/decode-allocation-guard' 2026-07-28 22:31:09 +02:00
fabi
f0d69f1cda fix(imaging): restore the decode allocation guard I removed in round 1 
This is a regression I introduced, not a pre-existing gap. Before 05948d8 the
compression worker used `ImageReader::decode()`, which does:

    let mut decoder = Self::make_decoder(format, self.inner, limits.clone())?;
    limits.reserve(decoder.total_bytes())?;   // enforces max_alloc
    decoder.set_limits(limits)?;

Reading the EXIF orientation tag needs `into_decoder()` instead, and that skips
the reserve entirely — the crate's own FIXME concedes `from_decoder` doesn't
compensate. Nothing else enforces `max_alloc`: the JPEG decoder's `set_limits`
only checks support and dimensions. So the 256 MiB budget has been inert since
that commit, and round 2 then propagated the weakened path into export.rs through
the shared helper, in a commit whose message claimed the helper "carries" the
decompression-bomb cap. It didn't, and the comment saying max_alloc "hard-caps
the decode allocation" was simply false.

What was left was only the per-axis cap, which permits 12000x12000 — 412 MiB
decoded, 824 MiB for the two concurrent decodes the worker runs by default,
against a 1 GiB container. Deploy-blocking right now because bumping
DERIVATIVES_REV makes the first boot after a deploy re-decode the entire gallery
two at a time: an OOM kill there restarts the container, which re-runs the
backfill. A boot loop, on the first deploy of these fixes.

Re-add the reserve exactly as `decode()` does it. Per the budget decision it stays
at 256 MiB (~89 MP for RGB8, above any mainstream phone's real output); two
concurrent decodes now peak at 512 MiB. Oversized images take the graceful path
from round 1 — original retained, quota refunded, upload-error toast — and fail
after the header parse but BEFORE any pixels are read, so they cost a header read
rather than an allocation. Measured peak during a concurrent oversized burst: 3.0
MiB.

Test parity is the other half, and the reason this was invisible: the e2e app
container had NO memory limit while production is capped at 1 GiB, so a decode
that would OOM-kill production simply succeeded in CI. Mirror the 1 GiB cap in
docker-compose.test.yml. That is the third divergence of this shape, after WebKit
missing from CI and /health existing only in Caddyfile.test.

Tests: a fixture that is 568 KiB on disk and 283 MiB decoded (11000x9000 = 99 MP,
deliberately UNDER the per-axis cap so the axis check cannot be what rejects it).
A unit test asserts the refusal — it fails against the old code, which decoded it
into an 11000x9000 buffer — with a companion asserting an ordinary photo still
decodes AND still gets its orientation applied, so the guard didn't become a
blanket refusal. An e2e test uploads it singly and as a concurrent pair, asserting
compression lands in 'failed' and the backend is still serving and still
processing afterwards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 22:31:09 +02:00
fabi
64eccb8672 Merge branch 'chore/prettier' 2026-07-28 21:28:11 +02:00
fabi
eefa476765 chore: satisfy prettier in frontend and e2e 
`checks.yml` runs `npm run format:check` for both projects and both were failing.

- frontend/src/lib/ui-store.ts is mine, unformatted since the round-1 upload-queue
  badge fix — the same miss as the rustfmt one: I gated on svelte-check and eslint
  but never on format:check.
- e2e/loadtest/* and e2e/shots.mjs have been unformatted since 7758270 and are
  unrelated to the audit work. Fixed here because they block the same gate and the
  fix is mechanical; no behaviour change in either project.

Still red and deliberately NOT fixed here: `npm run lint` in the frontend reports
`svelte/prefer-svelte-reactivity` on routes/diashow/+page.svelte:208 (a mutable
`Set` where the rule wants `SvelteSet`), pre-existing since 5009590. That one is a
real reactivity change in code I have no test coverage for, so it belongs in its
own change rather than smuggled into a formatting commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 21:28:11 +02:00
fabi
0932e2a470 Merge branch 'chore/rustfmt' 2026-07-28 21:26:28 +02:00
fabi
117c0c547f chore(backend): satisfy cargo fmt 
`checks.yml` runs `cargo fmt --check`, and it has been failing since the round-1
audit fixes: I gated those on `cargo build` and `cargo clippy` but never ran fmt,
so three files drifted then and eight more this round. Pure formatting — no
behaviour change; clippy stays at zero and all 70 backend tests still pass.

Worth noting for next time: clippy passing is not evidence fmt does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 21:26:28 +02:00
fabi
c49bf875d9 Merge branch 'ci/webkit' 2026-07-28 20:57:08 +02:00
fabi
6e7c4565cd ci(e2e): run WebKit, so the iOS guarantees actually gate a PR 
The workflow installed only Chromium and ran chromium-desktop + chromium-mobile.
iOS Safari is the app's stated primary user — a wedding guest opening a QR link —
and WebKit is the only engine in the matrix that reproduces two of its behaviours:

  - it enforces X-Frame-Options on the hidden download iframe, so a site-wide DENY
    makes the keepsake download silently do nothing. Blink hands attachments to
    the download manager before the frame check and never notices.
  - it abandons a <video> load unless its Range probe gets a 206.

Both of those shipped. Adding 06-export to the webkit project in the round-1 fix
bought nothing on a PR, because CI never ran that project at all — the regression
test written specifically to catch the blocker only ever executed locally.

Runs 71 tests (67 pass, 4 skip on the documented IndexedDB-blob harness
limitation) in ~1.5 minutes locally, using the exact command added here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 20:57:08 +02:00
fabi
1a7a531c90 Merge branch 'fix/recover-ip-ceiling' 2026-07-28 20:54:58 +02:00
fabi
6920e5bf7a fix(recover): cap name cycling, and stop bcrypt blocking the runtime 
Round 1 gave /join a per-IP ceiling and left /recover with only its
`recover:{ip}:{name}` bucket. That key is right for the job it was written for —
stopping someone who knows a display name (they're listed on the feed) from
burning the victim's 3-strike PIN counter and locking them out on repeat. But the
name is ATTACKER-CHOSEN, so cycling names mints a fresh 5-attempt bucket every
time and the per-IP cost is unbounded.

What sits behind that limiter makes it worse than a normal flood: every call runs
a cost-12 bcrypt verify, including an UNCONDITIONAL throwaway verify for names
that don't exist — added deliberately to close a timing oracle. So an unknown name
is the single cheapest way to make the server do ~200ms of hashing.

Adds `recover_ip_rate_per_min` (default 30, migration 019), checked BEFORE the
per-name bucket so a name generator can't walk past it. 30/min is far above any
real recovery attempt while capping a flood. The per-name bucket is untouched and
remains the anti-guessing control.

The second half matters as much as the first: bcrypt was running inline on the
async runtime everywhere. At cost 12 that pins a tokio worker thread for ~200ms,
and there is only one per core — so a login flood stalled every other request on
the box, including the feed. There was no spawn_blocking anywhere in the auth
module, despite SECURITY-BACKLOG claiming bcrypt had been offloaded.

Route all of it through `verify_password` / `hash_password` on the blocking pool.
That covers /recover, /admin/login, the host PIN reset, and — the one most likely
to bite at a real event — the PIN hash minted on every single /join. Saturating
the blocking pool degrades logins; saturating the worker threads degrades
everything.

Tests: cycling distinct names from one IP now hits the ceiling with a Retry-After,
and — the assertion that keeps the fix honest — repeated wrong PINs against ONE
name are still throttled with the ceiling set generously high, so the ceiling
added protection rather than replacing it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 20:54:58 +02:00
fabi
58f718bdce Merge branch 'fix/export-exif-orientation' 2026-07-28 20:46:38 +02:00
fabi
3c984e2932 fix(export): apply EXIF orientation in the keepsake too 
Round 1 fixed EXIF orientation in the compression worker, which corrected the live
app — feed preview and diashow display. The export worker was missed, and it does
not reuse those derivatives: it re-decodes the originals itself with `image::open`,
which ignores the orientation tag, then re-encodes to JPEG, which drops the tag —
so the viewer has no way to recover it.

The damage was oddly shaped, which is exactly why it reads as a viewer bug:

  Gallery.zip originals              correct  (byte-copied, EXIF intact)
  Memories viewer grid thumbnails    SIDEWAYS (always)
  Memories viewer full image >5 MB   SIDEWAYS (re-encoded at 2000px)
  Memories viewer full image ≤5 MB   correct  (streamed byte-for-byte)

So in the keepsake people actually keep, every portrait photo in the grid was on
its side, and clicking through silently "fixed" small photos but not large ones.

Rather than paste the decoder dance a third time, extract `services::imaging::
decode_oriented` and route both workers through it, so there is exactly one way to
turn a file on disk into a DynamicImage. It carries a second invariant that had
also drifted: `image::open` applies NO decode limits, so the export path was
decoding arbitrary user-supplied images unbounded — the decompression-bomb cap
existed only in the compression worker. Both now come as a pair, which is the
point of having one function.

Not done: switching export to consume the existing `display` derivative. It would
fix orientation and drop a redundant full-resolution decode per photo, but it
would also replace the pristine ≤5 MB originals in the keepsake with 2048px
re-encodes — a real quality regression in the one artefact people keep forever.

Test uploads the round-1 fixture (40x20 landscape tagged Orientation=6), runs a
real export, pulls the thumbnail out of Memories.zip and asserts it came back
portrait — with a sanity check that the source really is stored landscape, so the
test can't pass against a pipeline that does nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 20:46:38 +02:00
fabi
d6fdc13da9 Merge branch 'fix/compression-orphan-quota' 2026-07-28 20:40:15 +02:00
fabi
c14ccd2df1 fix(compression): reclaim failed originals instead of leaking them 
Round 1 stopped the compression worker deleting an upload's original on failure —
a transient ENOSPC or a codec panic must never destroy the only copy of a photo a
guest cannot retake. But it left `Upload::soft_delete`'s quota refund in place, so
the bytes stayed on disk while the uploader was charged nothing for them.

That is worse than it first looks. The row is soft-deleted, so the file is
invisible and unowned; a guest hitting a reproducible codec failure can accumulate
orphans indefinitely at zero personal cost. And `active_uploaders` counts only
users with non-deleted uploads, so dropping out of that count RAISES everyone's
per-user ceiling — the leak loosens the very quota meant to contain it.

Keep the refund: the uploader didn't cause the failure and shouldn't silently lose
quota to it. Bound the leak instead, with an hourly sweep alongside the existing
session cleanup in `spawn_periodic_tasks`, reclaiming failed originals older than
14 days — comfortably longer than any single event, so an operator investigating a
failed upload still has the file.

The selection predicate is the entire safety argument, so it is deliberately
narrow: `compression_status = 'failed'` AND soft-deleted AND past the window AND
`original_path <> ''`. That is exactly the state the give-up path leaves behind,
and it cannot reach a live upload, an owner-deleted one, or a failure still inside
its recovery window. `original_path` is cleared after a successful reclaim, which
makes the sweep idempotent — otherwise a row whose file is already gone is
re-selected on every tick forever. The row itself is kept as the audit trail.

Tests reproduce the selection verbatim (same pattern as upload_concurrency) and
assert it against five near-misses that must survive, both sides of the retention
boundary, and the idempotence property.

Also fixes two comments in export.rs still claiming "the compression worker
hard-deletes an original when its transcode fails" — no longer true, and the
defensive handling they justify is now justified by this sweep and by ordinary
deletes instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 20:40:15 +02:00
fabi
9c8cc7c069 Merge branch 'fix/role-store-identity-reset' 2026-07-28 20:35:40 +02:00
fabi
1485df5469 fix(auth): bind the role store to the identity, not to the tab 
The role store I added in the moderation work is a module-level singleton seeded
ONCE at import. `goto()` is a client-side navigation, so leaving and re-joining in
the same tab re-imports no module and re-runs no onMount — the previous user's
role simply stayed resident. Nothing reset it: not join, recover, admin login,
"Event verlassen", `clearAuth`, nor the api.ts 401 auto-clear.

So a host who left, followed by a guest joining on the same phone, left that guest
with `isStaff === true` and a "🚫 Beitrag entfernen" action on other people's
photos. The backend 403s the delete, so this was a false affordance rather than a
privilege escalation — but `/feed` never fetched `/me/context`, so unlike every
other route it never self-corrected either. It survived until a hard reload.

The mirror case was equally broken and easier to overlook: a guest who recovered
into a host account got NO host affordances.

`clearAuth` already had a hook registry for exactly this shape of problem, with a
comment explaining it exists to avoid circular imports. Add the missing mirror,
`onSetAuth`, fired by both `setAuth` and `setAdminAuth` after the new token is
resident, and have the role store register on both sides: clear to null on
logout, re-seed from the new token on login. That also gives
`syncRoleFromToken` — dead code with zero callers since I introduced it — its
intended purpose.

Seeding from the claim fixes the reported bug, but the claim is frozen for the
token's 30-day life, so a promotion or demotion still wouldn't reach the feed.
`/feed` now calls the existing `refreshEventState()` on mount, which fetches
`/me/context` and applies both the authoritative role and the lock/release state
in one request. The feed is the one route gating a destructive action on the role,
so it should not be the only route running on a stale claim.

Tests: 04-host/role-identity-reset drives the real flows. The first asserts the
host DOES see the action before asserting the newcomer does not — a negative
assertion alone would pass against a build that shipped no moderation at all. The
second covers the mirror, promoting a guest server-side while their resident token
still claims `role: guest`, so a fix that only cleared the role would fail it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 20:35:40 +02:00
fabi
81e5017f27 Merge branch 'fix/video-playback' 2026-07-28 20:29:16 +02:00
fabi
813a9fa500 fix(video): play the actual video, and answer Range requests 
Every video in the app was unplayable. Two independent defects, either one
sufficient on its own, and nothing in the suite covered either — no test
anywhere played media or asserted a `<video>` src.

1. The lightbox handed `<video>` a JPEG.

`pickMediaUrl` is mime-agnostic, and compression only ever produces a THUMBNAIL
for a video (one `ffmpeg -vframes 1` frame) — no preview, no display. So in the
DEFAULT saver mode the element's src resolved to `/api/v1/upload/{id}/thumbnail`,
served as `image/jpeg` with `nosniff` so the browser can't even sniff its way
out. Chromium reports DEMUXER_ERROR_COULD_NOT_OPEN.

Fixed in the lightbox rather than in `pickMediaUrl`: FeedListCard shares that
helper and legitimately wants the thumbnail for its `<img>` poster, so a central
mime branch would break the feed. This mirrors the rule the diashow already
applies ("videos play the original file directly"). Added `preload="none"` so
saver-mode guests on cellular still fetch nothing until they press play — there
is no smaller video derivative to offer them — plus `playsinline`, without which
iOS hijacks playback into fullscreen.

2. `stream_media_file` ignored Range entirely.

It took no request headers, so it could not see `Range`; it always returned 200
with the whole body and never sent Accept-Ranges or Content-Range. iOS Safari
opens every `<video>` with a `Range: bytes=0-1` probe and abandons the load
without a 206 — so video failed on the app's primary platform even in `original`
mode, where the src was already correct.

Adds single-range support (`bytes=N-`, `bytes=N-M`, `bytes=-S`) with 206 +
Content-Range, 416 + `bytes */len` past EOF, and Accept-Ranges advertised on
every response. Anything it won't handle — multi-range, non-bytes units, garbage
— falls back to a full 200, which RFC 9110 explicitly permits and which is safer
than guessing. All four media routes share the helper, so seeking works
uniformly.

`get_original` now serves `inline` instead of `attachment`. An attachment
disposition is hostile to a `<video>` element, and this route is the only source
of playable video bytes; it also matches what the UI promises, since the action
is labelled "Original anzeigen" — view, not download. `no-store` is deliberately
kept so a takedown still revokes access promptly; ranges work fine under it, the
client just re-fetches.

Tests: 11 unit tests pin the parser (the iOS `bytes=0-1` probe, inclusive ends,
suffix ranges, clamping past EOF, 416 vs 200, malformed fallbacks). A new
03-feed/video-playback spec asserts the src is the original and not the
thumbnail, that the browser accepts the bytes as media (readyState > 0, no
MediaError), that no video bytes are delivered before play, and that Range
returns the correct 206 slices and a 416 past EOF — verified on both Chromium
and WebKit.

The "not downloaded before play" test asserts no *delivered body* rather than no
request: WebKit opens a connection for a preload="none" video and immediately
aborts it (GET, no Range, status 0, nothing transferred) while Chromium issues
nothing at all. The portable guarantee is that no response carrying bytes
completes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 20:29:16 +02:00
fabi
f03e392f8c Merge branch 'docs/upgrade-path' 2026-07-28 20:17:32 +02:00
fabi
3d94bbd6fb docs(deploy): document the update path — up -d alone ships nothing 
The README only ever described a fresh install. There was no update section
anywhere, and `--build` appeared nowhere in the docs.

That matters because `app` and `frontend` are `build:` services with no published
image tag, and Compose has no source-change detection: if an image by that name
exists it is reused. So the natural `git pull && docker compose up -d` reports
"Container app-1 Running", rebuilds nothing, and exits 0. A deploy that shipped
none of the new code is indistinguishable from a successful one — which is how
eleven merged fixes can sit in the repo and never reach the box.

Verified both halves against a real stack rather than asserting them: with a
source change staged, `up -d` left the image ID untouched; `up -d --build`
produced a new image ID and a healthy /health.

Adds an "Updating an existing deployment" section covering backup-before-migrate,
pull, rebuild, health check, and an image-ID comparison to prove a build actually
happened. Also spells out the rollback trap: migrations run on boot and are not
undone by checking out an older commit, so rolling back code without restoring
the snapshot leaves the schema ahead of the binary and the app refusing to start.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 20:17:32 +02:00
fabi
96a22cfe27 Merge branch 'test/webkit-idb-blob-limitation' 2026-07-28 19:14:37 +02:00
fabi
c6e9350f78 test(e2e): document why WebKit can't run the client-queue upload tests 
Three 02-upload tests have been failing on `webkit-iphone` on main — `02-upload`
was already in that project's testMatch, so this is pre-existing red, not
something the audit work introduced.

Root cause is the harness, not the app. Playwright's Linux WebKit build cannot
store Blobs in IndexedDB at all: `put()` fails with "UnknownError: Error
preparing Blob/File data to be stored in object store". Confirmed it is not
about how Playwright delivers files — a Blob constructed in-page with
`new Blob([bytes])` fails identically, while Chromium stores both that and a
`setInputFiles` File without complaint.

That breaks every test driving the composer (FAB → UploadSheet → /upload →
submit), because `handleSubmit` awaits `addToQueue`, which persists the file
before it can navigate. The symptom is a submit button stuck on "Wird
hochgeladen…" and a timeout waiting for /feed — which reads like an app hang and
cost real time to run down.

Skip those four (the three above plus the new rejection-visible) on WebKit only,
behind a named helper carrying the full explanation, so the next person gets the
answer instead of the investigation. Deliberately narrow: WebKit still runs every
API-driven upload test, all of 01-auth, 03-feed and 06-export — including the
keepsake download, which only WebKit can meaningfully verify. Chromium continues
to run all 14.

Worth being explicit, since these tests exist to protect iOS: real Safari
supports Blobs in IndexedDB, so this is NOT evidence that the offline upload
queue is broken on the platform. It does mean that guarantee is currently
unverifiable in CI and rests on Chromium coverage plus manual device testing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 19:14:37 +02:00
fabi
537a11b0a4 Merge branch 'docs/backup-and-test-defaults' 2026-07-28 07:49:52 +02:00
fabi
27e4004cc8 docs(backup): make the backup commands work; fix the e2e/prod divergences 
Backup. Both documented commands failed on the shipped stack, and the sentence
explaining them was wrong too:

- `pg_dump $DATABASE_URL` — `DATABASE_URL` is only ever in the compose
  environment, never an operator's shell, and it points at `db:5432`, which is
  compose-internal DNS. The app image has no postgres client either.
- `> /media/backups/…` — `/media` is a named volume mounted inside the app
  container, not a host path, and nothing ever creates a `backups` subdirectory.
- `rsync /opt/eventsnap/media/` — that path does not exist anywhere.
- "a single path to back up" — false, and dangerously so: exports were moved to
  their own `exports_data` volume precisely so a keepsake (which contains every
  photo in the event) can't be served off the media tree. Backing up only
  `media_data` silently loses every generated keepsake.

Rewritten as three commands — db via `docker compose exec -T db pg_dump`, and one
`docker run … tar` per volume — all verified against the running stack. The
volume mounts use `/src`, not `/media`: I hit the footgun while testing this.
Docker pre-populates an EMPTY volume from the image's own directory and chowns it
to match, so `-v media_data:/media alpine` tars alpine's cdrom/floppy/usb, writes
them into the volume, and leaves it root-owned so the non-root app can no longer
write. Mounting where the image has nothing avoids all of it. Documented inline
so the next person doesn't rediscover it.

Also correct the architecture notes: `/media/*` no longer routes to the backend
(that static tree was removed as a gating bypass), and `exports_data` was missing
from the volume list — the one volume an operator most needs to know about.

e2e stack: add the `EXPORT_PATH` + `/exports` volume it was missing. The file
says "mirrors production layout"; without these, exports landed on the container's
writable layer at the default path, so export-leak and export-video wrote real
archives into ephemeral storage and the "exports live outside media" invariant
was never actually exercised.

Pre-existing red test, unrelated to the audit: all four 02-upload/quota tests
have been failing since 4464147 "stop /me/quota leaking raw disk to guests"
(2026-07-19), which post-dates the spec's last edit. `setLimitTo` calibrated
`quota_tolerance` from `free_disk_bytes` read through the GUEST's token — a field
that commit deliberately zeroes for non-staff. Dividing by it yields a NaN
tolerance, so every test in the block died in the helper. Read the calibration
inputs through a staff token and keep reading the ceiling back through the guest,
whose limit is the thing under test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 07:49:52 +02:00
fabi
c4e9b89af0 Merge branch 'fix/upload-pipeline-integrity' 2026-07-28 07:19:25 +02:00
fabi
05948d8268 fix(upload): stop destroying originals, apply EXIF orientation, surface rejections 
Three defects in the same pipeline, each of which loses a photo or misrepresents
one.

1. A transient error destroyed the guest's only copy.

`process`'s error arm unconditionally `remove_file`d the original. Every failure
routed there: `create_dir_all`, both derivative `save_with_format` calls (disk
full is the canonical case, and it arrives exactly when many guests upload at
once), a panic inside the image codec, or a momentary DB-pool exhaustion. The
row is only SOFT-deleted, so the bytes were the sole unrecoverable part — and
they were the part we deleted. The author already knew this was wrong next door:
`backfill_missing_display` says it "must NEVER soft-delete an upload that already
has a working preview".

Retry up to 3 times with backoff (re-checking the e2e generation guard after each
sleep), and on final failure keep the refund + soft-delete but leave the original
on disk, logging its path. A failed upload is now recoverable instead of gone.

2. Every portrait photo was stored sideways.

Phones don't rotate sensor data — they record the camera orientation in EXIF and
store the pixels as shot. `decode()` returns those raw pixels and the JPEG
re-encode writes no EXIF, so the 800px preview, the 2048px diashow display and
the keepsake were all rotated 90°, while "Original anzeigen" rendered upright
because the original keeps its tag. That asymmetry is why it reads as a viewer
bug. There was no EXIF handling anywhere in the repo and no exif crate.

Read the tag via `into_decoder()` (which carries the decode Limits through, so
the decompression-bomb cap is untouched) and apply it. Missing/malformed tags
fall back to NoTransforms — most images have none.

Existing derivatives are already baked wrong, so migration 018 adds
`derivatives_rev` and `backfill_missing_display` becomes
`backfill_stale_derivatives`: it now also picks up anything below the current rev
and regenerates it once from the original, which still carries its EXIF. Videos
are marked current in the migration — ffmpeg already honours the rotation matrix.
Bump DERIVATIVES_REV for any future change that invalidates derivatives.

3. A rejected upload vanished without a word.

`UploadQueue.svelte` — 162 lines holding the ONLY renderer of an item's error
text, the only "Erneut" retry button and the only rate-limit countdown — was
never imported anywhere, so `retryItem`, `removeItem` and `clearCompleted` were
unreachable at runtime. On a terminal rejection the store purged the blob and
wrote a clear German reason into `entry.error` "so the UI shows a clear reason".
There was no such UI. And `uploadBadgeCount` counted only pending/uploading, so
the badge decremented exactly as if the upload had succeeded.

Mount the queue on /upload, toast the reason immediately (the flow sends the user
to /feed straight after staging, so the list alone would still miss them), and
count blocked/error in the badge so a failure can't read as success.

Tests: 02-upload/exif-orientation uploads a 40x20 fixture tagged Orientation=6
and asserts both derivatives come back PORTRAIT, with a sanity check that the
source really is stored landscape. 02-upload/rejection-visible bans the uploader
between staging and sending, then asserts the toast, the queue row with the
server's reason, and that the item is still counted.

Note: 02-upload/quota's 4 failures are pre-existing and unrelated — see the next
commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-28 07:19:25 +02:00
fabi
d4237ad2ad Merge branch 'feat/host-moderation-ui' 2026-07-27 22:33:33 +02:00
fabi
be6d56f278 feat(moderation): let a host remove a guest's photo or comment from the UI 
`DELETE /host/upload/{id}` and `DELETE /host/comment/{id}` were complete on the
backend — transactional, SSE-broadcasting, audit-logged — and had zero frontend
callers. The feed context sheet offered "Löschen" only when
`target.user_id === myUserId`, so the only lever a host actually had against an
unwanted photo was banning the uploader.

That is both disproportionate and ineffective. A ban doesn't retract what was
already posted, and it makes things strictly worse for comments: the ban check
runs BEFORE the ownership check on the guest delete route, so banning an abusive
author leaves their comment on screen and permanently undeletable by them. With
no host affordance, nobody could remove it at all.

- feed: hosts/admins get "Beitrag entfernen" on other people's posts, routed to
  the host endpoint (the guest route 403s anything the caller doesn't own) with
  moderation-specific confirm copy. Own-post "Löschen" is unchanged.
- lightbox: same for comments, via /host/comment/{id}.
- Ban semantics are deliberately untouched (USER_JOURNEYS §10 — banned users keep
  read access and cannot write). The deadlock is broken by giving the host a way
  in, not by loosening the ban.

Live role (this had to come first). `getRole()` decodes the JWT claim, but the
token is never reissued — the backend slides the session row forward and treats
the DB row as authoritative. The claim is therefore frozen for the token's
lifetime: up to 30 days. A guest promoted at the party saw no Host-Dashboard and
no moderation actions until they signed out and back in, even though
`/me/context` had been returning their real role on every page load and 4 of its
6 call sites dropped the field on the floor.

Add `role-store.ts`: seeded from the claim so there's no flash of the wrong nav,
then corrected by every `/me/context` response. Point the ad-hoc `getRole()`
callers at it (account, upload, host, admin, and the new feed gate). The host and
admin dashboards now derive `myRole` reactively, so a demotion disables their
controls immediately instead of at next login.

Tests: 04-host/moderation-ui drives the real UI — host removes a guest photo and
it's gone from /feed server-side; a plain guest is offered nothing on someone
else's post (the mirror that keeps the first test honest); a promoted guest gains
the dashboard on reload while their token still carries `role: guest`; and a host
removes the comment of an already-banned guest, asserting first that the author's
own delete 403s so the deadlock is real.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-27 22:33:33 +02:00
fabi
0d8e83d392 Merge branch 'fix/rate-limit-shared-nat' 2026-07-27 21:56:49 +02:00
fabi
89057d605f fix(rate-limit): key the guest-facing limiters per user, not per IP 
At a venue every guest is behind one NAT, so an IP-keyed limiter hands the
whole party a single bucket. On a fresh deploy 12 guests arriving together
meant 5 joined and 7 were turned away, with no Retry-After telling them when
to retry. `/feed` (60/min) and `/export` (3 per DAY — the fourth guest to
fetch their keepsake locked out until tomorrow) had the same defect.

`feed_delta` was already keyed per-user and its comment states the exact
rationale ("so one client can't starve others behind a shared NAT"); this
makes its siblings match.

- feed:{ip}   -> feed:{user_id}    (auth was already in scope)
- export:{ip} -> export:{user_id}  (resolved from the download ticket's
  session, which was previously looked up and discarded)
- join:{ip}: pre-auth, so there is no user to key on. Split in two — a loose
  per-IP ceiling that only bounds raw volume (new `join_ip_rate_per_min`,
  default 60, migration 017), plus the real 5/60s anti-spam bucket keyed
  per (ip, name), mirroring the existing `recover:{ip}:{name}`.

admin_login / recover / pin_reset_req stay IP-keyed on purpose and are now
commented as such: they guard credential guessing, where a per-user or
per-name key would just hand an attacker a fresh bucket per guess.

Retry-After: the machinery existed but 7 of 8 sites called `check()` and
hard-coded `None`, so a throttled client was told to back off but never for
how long. Delete the bool `check()` wrapper entirely so `check_with_retry`
is the only entry point and the delay cannot be discarded by accident. Also
surface it for the PIN lockout, where the deadline was already known.

Fix the "unknown" fallback while here: every client_ip() caller passed that
literal, so any request without X-Forwarded-For — anything reaching the app
directly rather than through Caddy — shared ONE global bucket. Serve with
connect-info and use the peer address.

Tests: the reseed forces every limiter toggle off before each test, which is
why this whole class was invisible. Add 01-auth/rate-limit-shared-nat, which
enables them and asserts 12 guests share an IP without collision, that one
guest hammering their own name IS still throttled (so the fix re-keys rather
than removes the limit), and that feed/export buckets are per-user. Retarget
the ddos join test at the new per-IP ceiling — it asserted the defect.

Also seed `admin_login_rate_enabled` (read by the handler, seeded by no
migration and no reseed) and register `join_ip_rate_per_min` in the admin
config allowlist. Unrelated pre-existing red test fixed: 01-auth/join
asserted a "Willkommen!" heading the wedding redesign removed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-27 21:56:49 +02:00
fabi
688dc614d7 Merge branch 'fix/media-gating-percent-escape' 2026-07-27 21:21:18 +02:00
fabi
42416d76e2 fix(media): close the percent-escape bypass of the media gate 
`/media/%70reviews/{id}.jpg` served a taken-down photo to anyone,
unauthenticated. Verified against the running stack: the literal path 404s,
the escaped one returned 200 with the full image. Same for displays,
thumbnails and originals, and any escaped byte in any position works.

Cause: the block was four `nest_service("/media/previews", 404)` route
matches sitting above a `ServeDir` on `/media`. axum matches on the RAW path
(matchit does no percent-decoding), while `ServeDir` percent-decodes when it
resolves the file. So `%70reviews` missed every blocker, fell through to the
ServeDir, and was decoded back to `previews/` on disk — reaching the bytes
with no soft-delete and no ban-hide check. That defeats a host takedown,
which is the entire point of the gate.

Remove the `/media` route tree outright instead of racing the decoder.
Nothing needs it: every media URL the backend emits is already a gated
`/api/v1/upload/{id}/{original,preview,display,thumbnail}` alias
(handlers::feed), the frontend contains zero `/media/` references, and the
`/media` in config.rs/disk.rs is the filesystem path while `media/` in
export.rs is a path inside the zip. `/media/**` now 404s regardless of
encoding. The route's own comment already said it "serves nothing" — it
wasn't a backstop, it was the vector.

Caddy keeps proxying /media/* deliberately: the app 404s it, and forwarding
means the e2e gating specs exercise the app's refusal exactly as production
would rather than being masked by the SvelteKit 404 page.

Extend the gating spec with the encoded variants — asserting only the literal
spelling is what let this sit undetected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-27 21:21:18 +02:00
fabi
cec69e804a Merge branch 'fix/ios-keepsake-download-webkit' 2026-07-27 21:10:41 +02:00
fabi
137c4ee8a1 fix(export): let the keepsake download through X-Frame-Options on iOS 
The keepsake download navigates a hidden, same-origin iframe (deliberately:
a top-level navigation to a 404/429 would unload the PWA). Caddy stamped a
site-wide `X-Frame-Options: DENY` that also covered the proxied `/api/*`.

Blink hands a `Content-Disposition: attachment` response to the download
manager at the network layer, so Chromium never noticed. WebKit enforces XFO
on the frame navigation first and aborts the load — so on iOS Safari, the
app's primary platform, tapping Download did nothing at all, silently.

Carve the two export endpoints out to SAMEORIGIN, which still blocks
cross-origin framing. Implemented as two disjoint matchers rather than an
override: Caddy applies the FIRST `header` directive outermost, so it wins on
write and a later, more specific `header` is silently ignored (verified
against the running test stack).

Also close the test gap that let this ship:

- `06-export` ran on chromium-desktop only; add it to `webkit-iphone`, the
  only engine that enforces XFO on the download frame.
- No test in the suite ever clicked a download button — every archive
  assertion used Node `fetch`, which has no frame and no XFO enforcement.
  Add a spec that clicks it and awaits a real `download` event. Verified
  falsifiable: with the blanket DENY reinstated it fails and reports the
  WebKit refusal as the cause.
- Fix `ExportPage`'s card-scoped locators, which matched nothing: the cards
  carry `class="card p-5"` (a Tailwind `@apply` component class), never the
  `rounded-xl` the page object looked for. This had left the "shows enabled
  download buttons" test red on main.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-27 21:10:26 +02:00
56 changed files with 625 additions and 3596 deletions
Expand all files Collapse all files

19
.env.example
View File

@@ -12,14 +12,6 @@ APP_ENV=production
# ── Database ────────────────────────────────────────────────────────────────── # ── Database ──────────────────────────────────────────────────────────────────
# Set a strong password and keep it in sync between DATABASE_URL and # Set a strong password and keep it in sync between DATABASE_URL and
# POSTGRES_PASSWORD. Generate one with: openssl rand -hex 24 # POSTGRES_PASSWORD. Generate one with: openssl rand -hex 24
#
# SET THIS BEFORE THE FIRST `docker compose up -d`. Postgres reads POSTGRES_PASSWORD
# only when it initialises its data directory, on that very first boot. Change it
# afterwards and the app authenticates with the new password against a volume still
# holding the old one — a permanent restart loop ("password authentication failed").
# The only ways out are restoring the old password or `docker compose down -v`, which
# deletes the database, the media and the exports. In production the app refuses to
# boot while this is still the placeholder below, so it cannot be missed by accident.
DATABASE_URL=postgres://eventsnap:CHANGE_ME_use_a_strong_password@db:5432/eventsnap DATABASE_URL=postgres://eventsnap:CHANGE_ME_use_a_strong_password@db:5432/eventsnap
POSTGRES_USER=eventsnap POSTGRES_USER=eventsnap
POSTGRES_PASSWORD=CHANGE_ME_use_a_strong_password POSTGRES_PASSWORD=CHANGE_ME_use_a_strong_password
@@ -32,22 +24,13 @@ POSTGRES_DB=eventsnap
# OOM in Postgres doesn't degrade one feature, it takes the whole event down. # OOM in Postgres doesn't degrade one feature, it takes the whole event down.
DATABASE_MAX_CONNECTIONS=30 DATABASE_MAX_CONNECTIONS=30
# Log level. `info` is the right production default: at `debug` the tower-http trace
# layer writes a line per request AND per response, which on a busy event is a large
# multiple of the useful output. Container logs are capped at 10m x 3 per service
# (docker-compose.yml), so a chatty level buys you a shorter history, not more of it.
# To debug a live event: RUST_LOG=eventsnap_backend=debug docker compose up -d app
RUST_LOG=info
# ── Authentication ──────────────────────────────────────────────────────────── # ── Authentication ────────────────────────────────────────────────────────────
# Generate with: openssl rand -hex 64 # Generate with: openssl rand -hex 64
JWT_SECRET=change_me_to_a_random_64_byte_hex_string JWT_SECRET=change_me_to_a_random_64_byte_hex_string
SESSION_EXPIRY_DAYS=30 SESSION_EXPIRY_DAYS=30
# Admin dashboard password (bcrypt hash). # Admin dashboard password (bcrypt hash).
# Generate with an image the stack already pulls (htpasswd needs apache2-utils, which # Generate with: htpasswd -bnBC 12 "" yourpassword | tr -d ':\n'
# a stock VPS does not have):
# docker run --rm caddy:2-alpine caddy hash-password --plaintext 'yourpassword'
# IMPORTANT: keep the SINGLE QUOTES. A bcrypt hash is full of `$` (e.g. $2b$12$…$…), # IMPORTANT: keep the SINGLE QUOTES. A bcrypt hash is full of `$` (e.g. $2b$12$…$…),
# and both Docker Compose's env_file interpolation and dotenvy's variable substitution # and both Docker Compose's env_file interpolation and dotenvy's variable substitution
# would otherwise eat the `$…` segments (reading them as unset vars) and corrupt the # would otherwise eat the `$…` segments (reading them as unset vars) and corrupt the

12
.gitignore vendored
View File

@@ -13,16 +13,8 @@ frontend/build/
frontend/export-viewer/node_modules/ frontend/export-viewer/node_modules/
frontend/export-viewer/.svelte-kit/ frontend/export-viewer/.svelte-kit/
# Media uploads. In production these live in the `media_data` DOCKER VOLUME, never in the # Media uploads (mounted volume in production)
# working tree — so this pattern is anchored to the repo root and exists only for a local media/
# bind-mount experiment.
#
# It used to read `media/`, unanchored, which matches a directory of that name at ANY depth.
# The only one in the repo is `e2e/fixtures/media/`, so the rule's entire practical effect was
# to keep every E2E fixture untracked: a fresh clone got the specs and none of the images or
# videos they read. `.github/workflows/e2e.yml` does a plain checkout and generates nothing, so
# the committed CI job could not have run the upload, video or export suites at all.
/media/
# Playwright E2E suite — runtime artifacts (the suite itself is committed) # Playwright E2E suite — runtime artifacts (the suite itself is committed)
e2e/node_modules/ e2e/node_modules/

53
README.md
View File

@@ -97,51 +97,17 @@ eventsnap/
git clone https://git.mc02.dev/fabi/EventSnap.git eventsnap git clone https://git.mc02.dev/fabi/EventSnap.git eventsnap
cd eventsnap cd eventsnap
# 2. Configure environment — set EVERY secret NOW, before step 3. # 2. Configure environment
cp .env.example .env cp .env.example .env
nano .env # DOMAIN, EVENT_NAME, EVENT_SLUG, nano .env # set DOMAIN, JWT_SECRET, ADMIN_PASSWORD_HASH, EVENT_NAME, etc.
# JWT_SECRET, ADMIN_PASSWORD_HASH,
# POSTGRES_PASSWORD *and* the same password inside DATABASE_URL
# (see "Generate required secrets" below)
# 3. Start the stack # 3. Start the stack
docker compose up -d docker compose up -d
``` ```
> **Set every secret before step 3 — `POSTGRES_PASSWORD` especially.** Postgres reads it **only
> when it initialises its data directory**, which happens on the very first `docker compose up -d`.
> Changing it in `.env` afterwards does not change the stored password: the app then authenticates
> with the new one against a volume holding the old one, and you get a permanent restart loop with
> `password authentication failed for user "eventsnap"`. The only fixes are restoring the old
> password or `docker compose down -v`, which **deletes the database, the media and the exports**.
> Getting it right once, up front, costs nothing; getting it wrong costs the volume.
Caddy automatically obtains a Let's Encrypt certificate on first start. The app is live at `https://DOMAIN` within ~30 seconds. Caddy automatically obtains a Let's Encrypt certificate on first start. The app is live at `https://DOMAIN` within ~30 seconds.
> **If the site never comes up:** with `APP_ENV=production` the backend **refuses to boot** while > **If the site never comes up:** with `APP_ENV=production` the backend **refuses to boot** while `JWT_SECRET`/`ADMIN_PASSWORD_HASH` still hold the `.env.example` placeholders (this is deliberate — a publicly-known signing key is worse than downtime). Caddy then waits on the unhealthy `app` container and never serves. Check `docker compose logs app` — a "Refusing to start … placeholder …" line means you skipped step 2. Rotate the secrets (see below) and restart.
> `JWT_SECRET`, `ADMIN_PASSWORD_HASH` or the password inside `DATABASE_URL` still hold the
> `.env.example` placeholders (this is deliberate — a publicly-known signing key or database
> password is worse than downtime). Caddy then waits on the unhealthy `app` container and never
> serves. Check `docker compose logs app` — a "Refusing to start … placeholder …" line lists
> **every** unset secret at once, so one edit fixes them all.
>
> **If it comes up but keeps restarting with `password authentication failed for user
> "eventsnap"`:** `POSTGRES_PASSWORD` was changed after the database volume was created. Postgres
> applies that variable only at initialisation, so `.env` and the stored password have drifted
> apart permanently. `docker compose logs app` spells this out. Before the event, with nothing
> worth keeping:
>
> ```bash
> docker compose down -v && docker compose up -d # -v DELETES db + media + exports. No undo.
> ```
>
> **Once the event has real data, never do that.** Put the original password back into
> `DATABASE_URL`, or change the stored one instead:
>
> ```bash
> docker compose exec db psql -U "$POSTGRES_USER" -c \
> "ALTER ROLE eventsnap WITH PASSWORD 'the-password-now-in-your-.env';"
> ```
> **Production note:** `docker compose up -d` does **not** expose the database — Postgres is reachable only on the internal Docker network. For local development where you need host access to Postgres, opt into the dev overlay explicitly: > **Production note:** `docker compose up -d` does **not** expose the database — Postgres is reachable only on the internal Docker network. For local development where you need host access to Postgres, opt into the dev overlay explicitly:
> ```bash > ```bash
@@ -213,19 +179,10 @@ TLS certificate and all data volumes survive.
# JWT secret (64 random bytes) # JWT secret (64 random bytes)
openssl rand -hex 64 openssl rand -hex 64
# Database password (goes in BOTH DATABASE_URL and POSTGRES_PASSWORD) # Admin password hash (bcrypt, cost 12)
openssl rand -hex 24 htpasswd -bnBC 12 "" yourpassword | tr -d ':\n'
# Admin password hash (bcrypt). Uses an image the stack already pulls, so it needs
# nothing installed on the host — `htpasswd` lives in apache2-utils, which a stock
# VPS does not have. Emits cost 14 rather than 12; that is fine (admin login is
# rate-limited and hashed off the async runtime), and any $2a/$2b/$2y hash verifies.
docker run --rm caddy:2-alpine caddy hash-password --plaintext 'yourpassword'
``` ```
Wrap the resulting hash in **single quotes** in `.env` — see the note there; a bcrypt
hash is full of `$`, and both Compose and dotenvy would otherwise eat those segments.
### Environment Variables ### Environment Variables
See [.env.example](.env.example) for the full list with descriptions and defaults. Key variables: See [.env.example](.env.example) for the full list with descriptions and defaults. Key variables:

192
backend/Cargo.lock generated
View File

@@ -65,6 +65,56 @@ dependencies = [
"libc", "libc",
] ]
[[package]]
name = "anstream"
version = "1.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "824a212faf96e9acacdbd09febd34438f8f711fb84e09a8916013cd7815ca28d"
dependencies = [
"anstyle",
"anstyle-parse",
"anstyle-query",
"anstyle-wincon",
"colorchoice",
"is_terminal_polyfill",
"utf8parse",
]
[[package]]
name = "anstyle"
version = "1.0.14"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "940b3a0ca603d1eade50a4846a2afffd5ef57a9feac2c0e2ec2e14f9ead76000"
[[package]]
name = "anstyle-parse"
version = "1.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "52ce7f38b242319f7cabaa6813055467063ecdc9d355bbb4ce0c68908cd8130e"
dependencies = [
"utf8parse",
]
[[package]]
name = "anstyle-query"
version = "1.1.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc"
dependencies = [
"windows-sys 0.61.2",
]
[[package]]
name = "anstyle-wincon"
version = "3.0.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d"
dependencies = [
"anstyle",
"once_cell_polyfill",
"windows-sys 0.61.2",
]
[[package]] [[package]]
name = "anyhow" name = "anyhow"
version = "1.0.102" version = "1.0.102"
@@ -504,12 +554,46 @@ dependencies = [
"inout", "inout",
] ]
[[package]]
name = "clap"
version = "4.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b193af5b67834b676abd72466a96c1024e6a6ad978a1f484bd90b85c94041351"
dependencies = [
"clap_builder",
]
[[package]]
name = "clap_builder"
version = "4.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "714a53001bf66416adb0e2ef5ac857140e7dc3a0c48fb28b2f10762fc4b5069f"
dependencies = [
"anstream",
"anstyle",
"clap_lex",
"strsim",
"terminal_size",
]
[[package]]
name = "clap_lex"
version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9"
[[package]] [[package]]
name = "color_quant" name = "color_quant"
version = "1.1.0" version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3d7b894f5411737b7867f4827955924d7c254fc9f4d91a6aad6b097804b1018b" checksum = "3d7b894f5411737b7867f4827955924d7c254fc9f4d91a6aad6b097804b1018b"
[[package]]
name = "colorchoice"
version = "1.0.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570"
[[package]] [[package]]
name = "compression-codecs" name = "compression-codecs"
version = "0.4.37" version = "0.4.37"
@@ -593,6 +677,15 @@ dependencies = [
"cfg-if", "cfg-if",
] ]
[[package]]
name = "crossbeam-channel"
version = "0.5.15"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "82b8f8f868b36967f9606790d1903570de9ceaf870a7bf9fbbd3016d636a2cb2"
dependencies = [
"crossbeam-utils",
]
[[package]] [[package]]
name = "crossbeam-deque" name = "crossbeam-deque"
version = "0.8.6" version = "0.8.6"
@@ -723,6 +816,27 @@ dependencies = [
"cfg-if", "cfg-if",
] ]
[[package]]
name = "env_filter"
version = "1.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "32e90c2accc4b07a8456ea0debdc2e7587bdd890680d71173a15d4ae604f6eef"
dependencies = [
"log",
]
[[package]]
name = "env_logger"
version = "0.11.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0621c04f2196ac3f488dd583365b9c09be011a4ab8b9f37248ffcc8f6198b56a"
dependencies = [
"anstream",
"anstyle",
"env_filter",
"log",
]
[[package]] [[package]]
name = "equator" name = "equator"
version = "0.4.2" version = "0.4.2"
@@ -1115,6 +1229,12 @@ dependencies = [
"weezl", "weezl",
] ]
[[package]]
name = "glob"
version = "0.3.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280"
[[package]] [[package]]
name = "governor" name = "governor"
version = "0.6.3" version = "0.6.3"
@@ -1502,6 +1622,7 @@ checksum = "7714e70437a7dc3ac8eb7e6f8df75fd8eb422675fc7678aff7364301092b1017"
dependencies = [ dependencies = [
"equivalent", "equivalent",
"hashbrown 0.16.1", "hashbrown 0.16.1",
"rayon",
"serde", "serde",
"serde_core", "serde_core",
] ]
@@ -1535,6 +1656,12 @@ dependencies = [
"syn", "syn",
] ]
[[package]]
name = "is_terminal_polyfill"
version = "1.70.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695"
[[package]] [[package]]
name = "itertools" name = "itertools"
version = "0.14.0" version = "0.14.0"
@@ -1668,6 +1795,12 @@ dependencies = [
"vcpkg", "vcpkg",
] ]
[[package]]
name = "linux-raw-sys"
version = "0.12.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53"
[[package]] [[package]]
name = "litemap" name = "litemap"
version = "0.8.1" version = "0.8.1"
@@ -1956,6 +2089,12 @@ version = "1.21.4"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50"
[[package]]
name = "once_cell_polyfill"
version = "1.70.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe"
[[package]] [[package]]
name = "oxipng" name = "oxipng"
version = "9.1.5" version = "9.1.5"
@@ -1963,12 +2102,18 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "26c613f0f566526a647c7473f6a8556dbce22c91b13485ee4b4ec7ab648e4973" checksum = "26c613f0f566526a647c7473f6a8556dbce22c91b13485ee4b4ec7ab648e4973"
dependencies = [ dependencies = [
"bitvec", "bitvec",
"clap",
"crossbeam-channel",
"env_logger",
"filetime", "filetime",
"glob",
"indexmap", "indexmap",
"libdeflater", "libdeflater",
"log", "log",
"rayon",
"rgb", "rgb",
"rustc-hash", "rustc-hash",
"zopfli",
] ]
[[package]] [[package]]
@@ -2462,6 +2607,19 @@ version = "2.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "94300abf3f1ae2e2b8ffb7b58043de3d399c73fa6f4b73826402a5c457614dbe" checksum = "94300abf3f1ae2e2b8ffb7b58043de3d399c73fa6f4b73826402a5c457614dbe"
[[package]]
name = "rustix"
version = "1.1.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190"
dependencies = [
"bitflags",
"errno",
"libc",
"linux-raw-sys",
"windows-sys 0.61.2",
]
[[package]] [[package]]
name = "rustversion" name = "rustversion"
version = "1.0.22" version = "1.0.22"
@@ -2902,6 +3060,12 @@ dependencies = [
"unicode-properties", "unicode-properties",
] ]
[[package]]
name = "strsim"
version = "0.11.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f"
[[package]] [[package]]
name = "subtle" name = "subtle"
version = "2.6.1" version = "2.6.1"
@@ -2956,6 +3120,16 @@ version = "1.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369"
[[package]]
name = "terminal_size"
version = "0.4.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "230a1b821ccbd75b185820a1f1ff7b14d21da1e442e22c0863ea5f08771a8874"
dependencies = [
"rustix",
"windows-sys 0.61.2",
]
[[package]] [[package]]
name = "thiserror" name = "thiserror"
version = "1.0.69" version = "1.0.69"
@@ -3331,6 +3505,12 @@ version = "1.0.4"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be" checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be"
[[package]]
name = "utf8parse"
version = "0.2.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821"
[[package]] [[package]]
name = "uuid" name = "uuid"
version = "1.23.0" version = "1.23.0"
@@ -4007,6 +4187,18 @@ version = "1.0.21"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b8848ee67ecc8aedbaf3e4122217aff892639231befc6a1b58d29fff4c2cabaa" checksum = "b8848ee67ecc8aedbaf3e4122217aff892639231befc6a1b58d29fff4c2cabaa"
[[package]]
name = "zopfli"
version = "0.8.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f05cd8797d63865425ff89b5c4a48804f35ba0ce8d125800027ad6017d2b5249"
dependencies = [
"bumpalo",
"crc32fast",
"log",
"simd-adler32",
]
[[package]] [[package]]
name = "zstd" name = "zstd"
version = "0.13.3" version = "0.13.3"

12
backend/Cargo.toml
View File

@@ -27,17 +27,7 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] }
dotenvy = "0.15" dotenvy = "0.15"
sysinfo = "0.32" sysinfo = "0.32"
image = "0.25" image = "0.25"
# default-features = false drops "parallel", which is what actually bounds oxipng's memory: oxipng = "9"
# with rayon it evaluates row filters concurrently, each trial holding its own full-size
# buffer, and there is no Options knob to cap that. Without the feature, lib.rs swaps in a
# sequential shim (oxipng's own supported path) so peak scales with ONE trial, not N.
# PNG optimisation gets slower; it is a background, best-effort, lossless size saving.
#
# "filetime" must be KEPT: without it OutFile::Path { preserve_attrs: true } silently no-ops.
# Dropping "binary" also removes clap/glob/env_logger — a CLI's dependencies that were being
# compiled into a server image — and "zopfli", which preset 2 does not use (it selects
# Deflaters::Libdeflater, which is not feature-gated).
oxipng = { version = "9", default-features = false, features = ["filetime"] }
async_zip = { version = "0.0.17", features = ["tokio", "deflate"] } async_zip = { version = "0.0.17", features = ["tokio", "deflate"] }
include_dir = "0.7" include_dir = "0.7"
infer = "0.15" infer = "0.15"

3
backend/migrations/021_derivative_attempts.down.sql
View File

@@ -1,3 +0,0 @@
DROP INDEX IF EXISTS idx_upload_derivative_backfill;
ALTER TABLE upload DROP COLUMN IF EXISTS derivative_last_error;
ALTER TABLE upload DROP COLUMN IF EXISTS derivative_attempts;

23
backend/migrations/021_derivative_attempts.up.sql
View File

@@ -1,23 +0,0 @@
-- Bound how many times a permanently-failing upload can be re-processed.
--
-- Without this, one poisoned row is an outage. 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 kills the container survives at rev 0, the unconditional startup
-- backfill re-selects it on the next boot, and `restart: unless-stopped` turns that into an
-- infinite kill loop. Every restart also drops every SSE stream and truncates every in-flight
-- upload. That was reachable via a single large PNG (see services/compression.rs), but the
-- shape is general: any input that can kill or hang the worker repeats forever.
--
-- The counter is incremented WRITE-AHEAD, before the work is attempted, because the failure
-- mode being defended against is a SIGKILL — no error is returned, no handler runs, no Drop
-- fires. A counter bumped in an error path increments zero times per crash and changes nothing.
ALTER TABLE upload ADD COLUMN IF NOT EXISTS derivative_attempts SMALLINT NOT NULL DEFAULT 0;
-- Last failure text, so a row that has given up can be diagnosed without reproducing it.
-- Nothing reads this in code; it exists for the operator.
ALTER TABLE upload ADD COLUMN IF NOT EXISTS derivative_last_error TEXT;
-- Serves the backfill selection, which now filters on both columns.
CREATE INDEX IF NOT EXISTS idx_upload_derivative_backfill
ON upload (derivatives_rev, derivative_attempts)
WHERE deleted_at IS NULL;

26
backend/migrations/022_feed_scalar_counts.down.sql
View File

@@ -1,26 +0,0 @@
-- Restore the 016 definition verbatim.
DROP VIEW IF EXISTS v_feed;
CREATE VIEW v_feed AS
SELECT
u.id,
u.event_id,
u.user_id,
usr.display_name AS uploader_name,
usr.is_banned,
usr.uploads_hidden,
u.preview_path,
u.thumbnail_path,
u.display_path,
u.mime_type,
u.caption,
u.created_at,
COUNT(DISTINCT l.user_id) AS like_count,
COUNT(DISTINCT c.id) AS comment_count
FROM upload u
JOIN "user" usr ON u.user_id = usr.id
LEFT JOIN "like" l ON l.upload_id = u.id
LEFT JOIN comment c ON c.upload_id = u.id AND c.deleted_at IS NULL
WHERE u.deleted_at IS NULL
AND usr.uploads_hidden = FALSE
AND usr.is_banned = FALSE
GROUP BY u.id, usr.display_name, usr.is_banned, usr.uploads_hidden;

55
backend/migrations/022_feed_scalar_counts.up.sql
View File

@@ -1,55 +0,0 @@
-- Make a feed page cost a page, not the whole event.
--
-- The previous definition (016) computed like_count/comment_count with LEFT JOINs and a
-- GROUP BY. Postgres CAN push the `event_id = $1` qual 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 feed request aggregated every upload in
-- the event (times its likes and comments) and only then sorted and took 21 rows. The cost
-- grew with the event, not with the page, and page 1 — the most expensive one — is exactly
-- what refreshFeedInPlace refetches on every completed upload, from every open feed in the
-- venue.
--
-- Correlated scalar subqueries move the counts ABOVE the Limit in the plan: they are evaluated
-- once per returned row, so 21 index lookups instead of a full aggregation.
--
-- The rewrite is EXACTLY equivalent, not merely close:
-- * "like" is keyed (upload_id, user_id), so COUNT(DISTINCT l.user_id) == count(*).
-- * comment.id is the primary key, so COUNT(DISTINCT c.id) == count(*).
-- * one row per upload either way — the GROUP BY was on u.id.
-- Column names, order and types are unchanged (count(*) and COUNT(DISTINCT ...) are both
-- bigint), so no Rust code changes.
--
-- No new index needed: idx_like_upload plus the (upload_id, user_id) PK serve the like
-- subquery, and idx_comment_upload ... WHERE deleted_at IS NULL matches the comment
-- subquery's predicate exactly.
--
-- One thing a future editor needs to know: the hashtag-filtered feed joins upload_hashtag
-- against this view. That was safe before only because the GROUP BY collapsed the join
-- fan-out; it is safe now because the view is one row per upload and upload_hashtag is keyed
-- (upload_id, hashtag_id) with a single tag filtered. Adding a second tag filter would need
-- fresh thought.
-- Not CASCADE: if something ever comes to depend on this view, the migration should fail
-- loudly rather than silently drop it.
DROP VIEW IF EXISTS v_feed;
CREATE VIEW v_feed AS
SELECT
u.id,
u.event_id,
u.user_id,
usr.display_name AS uploader_name,
usr.is_banned,
usr.uploads_hidden,
u.preview_path,
u.thumbnail_path,
u.display_path,
u.mime_type,
u.caption,
u.created_at,
(SELECT count(*) FROM "like" l WHERE l.upload_id = u.id) AS like_count,
(SELECT count(*) FROM comment c WHERE c.upload_id = u.id AND c.deleted_at IS NULL) AS comment_count
FROM upload u
JOIN "user" usr ON u.user_id = usr.id
WHERE u.deleted_at IS NULL
AND usr.uploads_hidden = FALSE
AND usr.is_banned = FALSE;

11
backend/migrations/023_reserved_names_and_pin_decay.down.sql
View File

@@ -1,11 +0,0 @@
-- NOTE: the reserved-name rename in the up migration is NOT reversible. The original names
-- are not recorded anywhere, and reversing it would in any case re-create the state that
-- bricked admin login. Rolling back the schema does not roll back that data change.
DELETE FROM config WHERE key IN (
'recover_name_rate_per_15min',
'pin_reset_ip_rate_per_min',
'upload_edit_rate_per_min',
'upload_edit_rate_enabled'
);
ALTER TABLE "user" DROP COLUMN IF EXISTS last_failed_pin_at;

48
backend/migrations/023_reserved_names_and_pin_decay.up.sql
View File

@@ -1,48 +0,0 @@
-- Two independent auth defects that share a migration because they share a table.
-- 1. RESERVED NAMES — free any guest squatting on a name the admin path used to depend on.
--
-- Migration 007 made display_name unique per event case-insensitively, and `join` had no
-- reserved-name guard. So any guest could join as "admin"/"Admin"/"ADMIN" before the operator's
-- first admin login; admin_login then looked its user up BY NAME, missed (wrong role), fell
-- through to creating "Admin", violated that unique index, and returned a 500 — permanently,
-- with no in-app recovery. Moderation, config and gallery release all gone, fixed only by SQL.
--
-- The real fix is in code (look the admin up by role, never by name — see auth/handlers.rs).
-- This clears the state an already-deployed database may be carrying.
--
-- RENAMED, NEVER DELETED: the guest keeps their uploads, their PIN and their session. Only
-- non-admin rows are touched — a real admin row named "Admin" is the expected state.
UPDATE "user" u
SET display_name = u.display_name || ' (' || left(u.id::text, 8) || ')'
WHERE u.role <> 'admin'
AND lower(u.display_name) IN ('admin', 'administrator', 'host', 'eventsnap');
-- 2. PIN LOCKOUT DECAY.
--
-- failed_pin_attempts only ever cleared on a successful recovery or after a lockout expired, so
-- honest typos accumulated across days: a guest who fat-fingered their PIN twice last night
-- arrives today already two-thirds of the way to being locked out. With the threshold now
-- raised (see below) a decay window is what keeps that raise safe rather than merely lenient.
ALTER TABLE "user" ADD COLUMN IF NOT EXISTS last_failed_pin_at TIMESTAMPTZ;
-- Rate-limit knobs introduced with this release.
--
-- recover_name_rate_per_15min (4, was a hardcoded 5): the per-(IP, name) ceiling. It MUST stay
-- below the account-lock threshold, which is the whole defect — at 5-per-IP against a 3-strike
-- lock, three requests from one IP locked any guest whose name is visible on the feed, every 15
-- minutes, forever. The lock threshold moves to 12 in code, so locking a victim now needs at
-- least three distinct sources while an honest guest never comes close.
--
-- pin_reset_ip_rate_per_min (30): /recover/request was the one unauthenticated endpoint with no
-- per-IP ceiling at all — /join got one in 017 and /recover in 019, and this third one was
-- simply missed. Its per-name key is attacker-chosen, so cycling names minted a fresh bucket
-- every time and the per-IP cost was unbounded.
--
-- upload_edit_rate_per_min (30): PATCH /upload/{id} had no rate limit of any kind.
INSERT INTO config (key, value) VALUES
('recover_name_rate_per_15min', '4'),
('pin_reset_ip_rate_per_min', '30'),
('upload_edit_rate_per_min', '30'),
('upload_edit_rate_enabled', 'true')
ON CONFLICT (key) DO NOTHING;

274
backend/src/auth/handlers.rs
View File

@@ -19,45 +19,6 @@ use crate::services::config;
use crate::services::rate_limiter::client_ip; use crate::services::rate_limiter::client_ip;
use crate::state::AppState; use crate::state::AppState;
/// Names a guest may not take.
///
/// Defence in depth only. The real fix for the admin-lockout defect is that `admin_login` now
/// resolves its user by ROLE rather than by name (see `User::find_admin_for_event`), which is
/// why homoglyph and zero-width bypasses of this list are not a concern: the name is no longer
/// load-bearing for anything. What this buys is that a guest cannot impersonate the host in the
/// feed's byline, and that "Admin" stays available for the admin row.
const RESERVED_DISPLAY_NAMES: &[&str] = &["admin", "administrator", "host", "eventsnap"];
fn is_reserved_display_name(name: &str) -> bool {
let name = name.trim().to_lowercase();
RESERVED_DISPLAY_NAMES.contains(&name.as_str())
}
/// Trim and bounds-check a display name.
///
/// Shared by `join`, `recover` and `request_pin_reset` so the length check happens BEFORE the
/// name is used to build a rate-limiter key. It was inline in `join` only, so on the other two
/// endpoints `format!("...:{ip}:{name_key}")` allocated from an unbounded, attacker-chosen
/// string and stored it in a HashMap pruned once an hour with a 24 h ceiling — turning the
/// limiter itself into the memory-exhaustion primitive it exists to prevent.
fn validate_display_name(raw: &str) -> Result<&str, AppError> {
let name = raw.trim();
let chars = name.chars().count();
if chars == 0 || chars > 50 {
return Err(AppError::BadRequest(
"Name muss zwischen 1 und 50 Zeichen lang sein.".into(),
));
}
// Postgres rejects 0x00 in TEXT columns with a 500. Catch it here so callers see a clean
// 400 instead of an internal error.
if name.contains('\0') {
return Err(AppError::BadRequest(
"Name enthält ungültige Zeichen.".into(),
));
}
Ok(name)
}
#[derive(Deserialize)] #[derive(Deserialize)]
pub struct JoinRequest { pub struct JoinRequest {
pub display_name: String, pub display_name: String,
@@ -101,13 +62,19 @@ pub async fn join(
} }
} }
let display_name = validate_display_name(&body.display_name)?; let display_name = body.display_name.trim();
if is_reserved_display_name(display_name) { let name_chars = display_name.chars().count();
// 409, matching the name-taken response below, so the frontend's existing handling if name_chars == 0 || name_chars > 50 {
// works unchanged. See RESERVED_DISPLAY_NAMES for why this exists. return Err(AppError::BadRequest(
return Err(AppError::Conflict(format!( "Name muss zwischen 1 und 50 Zeichen lang sein.".into(),
"Der Name \"{display_name}\" ist reserviert. Bitte wähle einen anderen." ));
))); }
// Postgres rejects 0x00 in TEXT columns with a 500. Catch it here so callers
// see a clean 400 instead of an internal error.
if display_name.contains('\0') {
return Err(AppError::BadRequest(
"Name enthält ungültige Zeichen.".into(),
));
} }
// Per-guest bucket, keyed like the `recover:{ip}:{name}` limiter below. This carries // Per-guest bucket, keyed like the `recover:{ip}:{name}` limiter below. This carries
@@ -184,28 +151,6 @@ pub async fn join(
)) ))
} }
/// Default for `recover_name_rate_per_15min` — wrong PINs allowed per (IP, name) per 15 min.
/// Mirrors migration 023; kept here so the invariant below can be asserted in a test.
const RECOVER_NAME_CEILING_DEFAULT: usize = 4;
/// Wrong PINs, from ALL sources, before the ACCOUNT itself is locked for 15 minutes.
///
/// This was 3, which sat BELOW the per-(IP, name) ceiling of 5 — and that ordering, not the
/// number, was the defect. Display names are public on the feed, so three requests from a single
/// IP locked any guest out of their own account, repeatable every 15 minutes, indefinitely. The
/// tier meant to protect a guest was the easiest way to attack them.
///
/// Raised deliberately far above the per-IP tier so the two do different jobs. The per-(IP, name)
/// bucket is what stops a guesser, and it costs the ATTACKER. This tier is the last line against
/// a DISTRIBUTED guesser, and it is the only one an attacker can turn on a victim — so reaching
/// it must require at least three distinct sources inside the decay window.
///
/// Brute-force cost is unchanged: 12 attempts per 15 minutes is 48/hour against one account, so
/// 10 000 four-digit PINs still take ~208 hours no matter how many IPs are used. An honest guest
/// fat-fingering a 4-digit PIN never comes close, and `increment_failed_pin` now decays the
/// streak after 15 minutes so yesterday's typos don't count toward today's.
const PIN_LOCK_THRESHOLD: i16 = 12;
#[derive(Deserialize)] #[derive(Deserialize)]
pub struct RecoverRequest { pub struct RecoverRequest {
pub display_name: String, pub display_name: String,
@@ -258,15 +203,13 @@ pub async fn recover(
headers: HeaderMap, headers: HeaderMap,
Json(body): Json<RecoverRequest>, Json(body): Json<RecoverRequest>,
) -> Result<Json<RecoverResponse>, AppError> { ) -> Result<Json<RecoverResponse>, AppError> {
// Validated BEFORE it is used as a rate-limiter key — see `validate_display_name`. The let display_name = body.display_name.trim();
// per-IP ceiling below is keyed only on the IP, so it is safe to run either side of this;
// the per-NAME bucket is not.
let display_name = validate_display_name(&body.display_name)?;
// Per-IP+name throttle BEFORE the per-user lockout counter. Without this an attacker who // Per-IP+name throttle BEFORE the per-user 3-strike counter. Without this
// knows a display name (they're visible on the feed) can burn through the victim's wrong-PIN // an attacker who knows a display name (they're visible on the feed) can
// budget and lock them out, repeatedly. The ceiling here MUST stay below // burn through 3 wrong PINs and lock the victim for 15 minutes — repeated
// PIN_LOCK_THRESHOLD — see the constant for why that ordering is the whole control. // every 15 minutes, indefinitely. 5 attempts per 15 minutes per (IP, name)
// softens that into a real cost.
let ip = client_ip(&headers, &peer.ip().to_string()); let ip = client_ip(&headers, &peer.ip().to_string());
let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await; let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await;
let recover_rate_on = config::get_bool(&state.config_cache, "recover_rate_enabled", true).await; let recover_rate_on = config::get_bool(&state.config_cache, "recover_rate_enabled", true).await;
@@ -291,16 +234,10 @@ pub async fn recover(
)); ));
} }
let name_ceiling = config::get_usize(
&state.config_cache,
"recover_name_rate_per_15min",
RECOVER_NAME_CEILING_DEFAULT,
)
.await;
let name_key = display_name.to_lowercase(); let name_key = display_name.to_lowercase();
if let Err(retry_after_secs) = state.rate_limiter.check_with_retry( if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("recover:{ip}:{name_key}"), format!("recover:{ip}:{name_key}"),
name_ceiling, 5,
Duration::from_secs(15 * 60), Duration::from_secs(15 * 60),
) { ) {
return Err(AppError::TooManyRequests( return Err(AppError::TooManyRequests(
@@ -380,7 +317,7 @@ pub async fn recover(
attempts, attempts,
"recover: wrong PIN" "recover: wrong PIN"
); );
if attempts >= PIN_LOCK_THRESHOLD { if attempts >= 3 {
let lockout = Utc::now() + chrono::Duration::minutes(15); let lockout = Utc::now() + chrono::Duration::minutes(15);
User::lock_pin(&state.pool, user.id, lockout).await?; User::lock_pin(&state.pool, user.id, lockout).await?;
tracing::warn!( tracing::warn!(
@@ -463,16 +400,27 @@ pub async fn admin_login(
) )
.await?; .await?;
// Find or create the admin user for this event — BY ROLE, never by name. // Find or create the admin user for this event
// let admin_name = "Admin";
// The name lookup this replaces is what made admin login brickable. Migration 007 makes let users = User::find_by_event_and_name(&state.pool, event.id, admin_name).await?;
// display_name unique per event case-insensitively and `join` had no reserved-name guard, let admin_user = if let Some(u) = users.into_iter().find(|u| u.role == UserRole::Admin) {
// so a guest joining as "admin" before the operator's first login made the lookup miss on u
// role, the fallback `create("Admin")` violate that index, and `?` return a permanent 500 — } else {
// taking out moderation, config and gallery release with no in-app recovery. // Admin authenticates via password, but the schema still requires a PIN
let admin_user = match User::find_admin_for_event(&state.pool, event.id).await? { // hash. Generate a random unguessable PIN so the recovery path remains
Some(u) => u, // unusable as an escalation route even if the role flag ever got cleared.
None => create_admin_user(&state, event.id).await?, let dummy_pin: String = (0..32)
.map(|_| rand::rng().random_range(b'a'..=b'z') as char)
.collect();
let dummy_hash = hash_password(dummy_pin.clone(), 4).await?;
let user = User::create(&state.pool, event.id, admin_name, &dummy_hash).await?;
sqlx::query("UPDATE \"user\" SET role = 'admin' WHERE id = $1")
.bind(user.id)
.execute(&state.pool)
.await?;
User::find_by_id(&state.pool, user.id)
.await?
.ok_or_else(|| AppError::Internal(anyhow::anyhow!("admin user creation failed")))?
}; };
tracing::info!(user_id = %admin_user.id, event_id = %event.id, ip = %ip, "admin_login: success"); tracing::info!(user_id = %admin_user.id, event_id = %event.id, ip = %ip, "admin_login: success");
@@ -497,51 +445,6 @@ pub async fn admin_login(
})) }))
} }
/// Create this event's admin row on first successful admin login.
///
/// Prefers the name "Admin". If a legacy database has a guest squatting on it — the state
/// migration 023 renames away, but a row could also predate that or be created between
/// migrations — falls back to a suffixed name rather than failing the login.
///
/// PROMOTING THE SQUATTING ROW WOULD BE A SERIOUS MISTAKE, and is the obvious-looking fix, so
/// it is spelled out: that row carries a `recovery_pin_hash` the guest knows. Setting
/// `role = 'admin'` on it would hand them the admin dashboard through `/recover`, permanently,
/// via a path that needs no password. A separate row under an uglier name is worse UX and much
/// better security — and since the lookup is now by role, the fallback name never has to be
/// guessed again on a later login.
async fn create_admin_user(state: &AppState, event_id: Uuid) -> Result<User, AppError> {
// Admin authenticates via password, but the schema still requires a PIN hash. Generate a
// random unguessable one so the recovery path stays unusable as an escalation route even if
// the role flag were ever cleared.
let dummy_pin: String = (0..32)
.map(|_| rand::rng().random_range(b'a'..=b'z') as char)
.collect();
let dummy_hash = hash_password(dummy_pin, 4).await?;
match User::create_with_role(&state.pool, event_id, "Admin", &dummy_hash, UserRole::Admin).await
{
Ok(u) => Ok(u),
Err(sqlx::Error::Database(db)) if db.is_unique_violation() => {
let fallback = format!("Admin-{}", &Uuid::new_v4().to_string()[..8]);
tracing::warn!(
%event_id, %fallback,
"the name \"Admin\" is held by a non-admin user; creating the admin under a \
fallback name. Rename that guest to free it — do NOT promote their row, they \
know its recovery PIN."
);
Ok(User::create_with_role(
&state.pool,
event_id,
&fallback,
&dummy_hash,
UserRole::Admin,
)
.await?)
}
Err(e) => Err(e.into()),
}
}
pub async fn logout(State(state): State<AppState>, auth: AuthUser) -> Result<StatusCode, AppError> { pub async fn logout(State(state): State<AppState>, auth: AuthUser) -> Result<StatusCode, AppError> {
Session::delete_by_token_hash(&state.pool, &auth.token_hash).await?; Session::delete_by_token_hash(&state.pool, &auth.token_hash).await?;
Ok(StatusCode::NO_CONTENT) Ok(StatusCode::NO_CONTENT)
@@ -573,38 +476,9 @@ pub async fn request_pin_reset(
headers: HeaderMap, headers: HeaderMap,
Json(body): Json<PinResetRequestBody>, Json(body): Json<PinResetRequestBody>,
) -> Result<StatusCode, AppError> { ) -> Result<StatusCode, AppError> {
let display_name = body.display_name.trim();
let ip = client_ip(&headers, &peer.ip().to_string()); let ip = client_ip(&headers, &peer.ip().to_string());
let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await; let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await;
// Coarse per-IP ceiling FIRST, keyed only on the IP so its key is bounded by construction.
// /join got one of these in migration 017 and /recover in 019; this third unauthenticated
// endpoint was simply missed — migration 019's own comment describes exactly this attack.
// Without it, the per-name bucket below is no ceiling at all: the name is attacker-chosen,
// so cycling names mints a fresh bucket every request.
if rate_limits_on {
let ip_ceiling =
config::get_usize(&state.config_cache, "pin_reset_ip_rate_per_min", 30).await;
if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("pin_reset_ip:{ip}"),
ip_ceiling,
Duration::from_secs(60),
) {
return Err(AppError::TooManyRequests(
"Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(),
Some(retry_after_secs),
));
}
}
// Validated BEFORE the per-name key is built, so an unbounded name can never be retained in
// the limiter map. NOTE the 204: this endpoint's contract is that it answers identically
// whether or not the name exists, so it cannot enumerate guests. A 400 here would be a new
// signal — it would distinguish a malformed name from a well-formed unknown one. Silence is
// the correct response, and matches what an empty name already did.
let Ok(display_name) = validate_display_name(&body.display_name) else {
return Ok(StatusCode::NO_CONTENT);
};
if rate_limits_on { if rate_limits_on {
let name_key = display_name.to_lowercase(); let name_key = display_name.to_lowercase();
if let Err(retry_after_secs) = state.rate_limiter.check_with_retry( if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
@@ -618,6 +492,9 @@ pub async fn request_pin_reset(
)); ));
} }
} }
if display_name.is_empty() {
return Ok(StatusCode::NO_CONTENT);
}
// Single statement so the existing-name and unknown-name paths do IDENTICAL work // Single statement so the existing-name and unknown-name paths do IDENTICAL work
// (same event+user index scans, an INSERT that matches 0 rows for an unknown name) — // (same event+user index scans, an INSERT that matches 0 rows for an unknown name) —
@@ -647,62 +524,3 @@ pub async fn request_pin_reset(
Ok(StatusCode::NO_CONTENT) Ok(StatusCode::NO_CONTENT)
} }
#[cfg(test)]
mod tests {
use super::*;
/// THE defect, stated as arithmetic: the account-lock threshold sat BELOW the per-(IP, name)
/// attempt ceiling, so a single IP could exhaust it and lock any guest whose display name is
/// visible on the feed — every 15 minutes, indefinitely. The tier meant to protect a guest
/// was the cheapest way to attack them.
///
/// The fix is the ORDERING, not either number on its own, so that is what this pins.
#[test]
fn one_ip_cannot_reach_the_account_lock() {
assert!(
PIN_LOCK_THRESHOLD as usize >= RECOVER_NAME_CEILING_DEFAULT * 3,
"locking a victim must require at least three distinct sources; \
threshold {PIN_LOCK_THRESHOLD} vs per-IP ceiling {RECOVER_NAME_CEILING_DEFAULT}"
);
}
/// Raising the threshold must not quietly weaken brute-force resistance. 4-digit PINs, and
/// the lockout window is 15 minutes, so an attacker gets PIN_LOCK_THRESHOLD tries per window.
#[test]
fn the_raised_threshold_still_makes_guessing_a_four_digit_pin_impractical() {
let attempts_per_hour = PIN_LOCK_THRESHOLD as u64 * 4; // four 15-minute windows
let hours_for_full_keyspace = 10_000 / attempts_per_hour;
assert!(
hours_for_full_keyspace >= 168,
"exhausting 10k PINs would take {hours_for_full_keyspace}h — under a week is too fast"
);
}
#[test]
fn reserved_names_are_matched_case_insensitively_and_trimmed() {
for name in ["admin", "Admin", "ADMIN", " Host ", "EventSnap"] {
assert!(is_reserved_display_name(name), "{name} must be reserved");
}
}
/// A substring match here would reject perfectly ordinary names, which is a worse outcome
/// than the impersonation the list guards against.
#[test]
fn names_that_merely_contain_a_reserved_word_are_allowed() {
for name in ["Administrata", "Hostess", "Adminah", "Ghost", "hosting"] {
assert!(!is_reserved_display_name(name), "{name} must be allowed");
}
}
#[test]
fn display_names_are_bounded_before_they_can_become_a_rate_limit_key() {
assert!(validate_display_name(" Lena ").is_ok());
assert_eq!(validate_display_name(" Lena ").unwrap(), "Lena");
// The case that made the limiter itself the exhaustion primitive.
assert!(validate_display_name(&"a".repeat(51)).is_err());
assert!(validate_display_name(&"a".repeat(2_000_000)).is_err());
assert!(validate_display_name(" ").is_err());
assert!(validate_display_name("bad\0name").is_err());
}
}

180
backend/src/config.rs
View File

@@ -20,53 +20,21 @@ fn looks_placeholder(s: &str) -> bool {
/// Enforce secret hygiene. In production every guard is hard-fail: a booting app /// Enforce secret hygiene. In production every guard is hard-fail: a booting app
/// with a publicly-known signing key is worse than one that refuses to start. /// with a publicly-known signing key is worse than one that refuses to start.
/// Outside production the dev sentinel is tolerated (warned) so local dev is frictionless. /// Outside production the dev sentinel is tolerated (warned) so local dev is frictionless.
/// fn validate_secrets(is_prod: bool, jwt_secret: &str, admin_password_hash: &str) -> Result<()> {
/// EVERY failure is collected and reported together. Returning on the first one made fixing two
/// secrets cost two boot cycles — the operator rotates JWT_SECRET, restarts, and only then learns
/// about ADMIN_PASSWORD_HASH. Restarting this stack is not free (Caddy waits on the unhealthy app),
/// and each avoidable cycle is another chance to reach for `down -v`.
fn validate_secrets(
is_prod: bool,
jwt_secret: &str,
admin_password_hash: &str,
database_url: &str,
) -> Result<()> {
if is_prod { if is_prod {
let mut problems: Vec<&str> = Vec::new();
if looks_placeholder(jwt_secret) { if looks_placeholder(jwt_secret) {
problems.push( return Err(anyhow!(
"JWT_SECRET is still the .env.example placeholder — rotate it \ "Refusing to start in production with a placeholder JWT_SECRET — \
(openssl rand -hex 64).", rotate it (openssl rand -hex 64)."
); ));
} else if jwt_secret.len() < 32 { }
problems.push("JWT_SECRET must be at least 32 characters."); if jwt_secret.len() < 32 {
return Err(anyhow!("JWT_SECRET must be at least 32 characters."));
} }
if admin_password_hash.is_empty() || looks_placeholder(admin_password_hash) { if admin_password_hash.is_empty() || looks_placeholder(admin_password_hash) {
problems.push(
"ADMIN_PASSWORD_HASH is unset or still the .env.example placeholder — generate one \
(docker run --rm caddy:2-alpine caddy hash-password --plaintext '<password>').",
);
}
// The DATABASE_URL carries the Postgres password, so a placeholder here means the stack is
// running on `CHANGE_ME_use_a_strong_password` — a credential published in the repo. The
// app used to boot green on it, because this guard only ever covered the two secrets it
// was written for and nothing else looked at POSTGRES_PASSWORD at all.
//
// Read POSTGRES_PASSWORD's docs before changing this: it is applied ONLY at initdb, so the
// remedy is not "edit .env and restart" — see the 28P01 diagnostic in db.rs.
if looks_placeholder(database_url) {
problems.push(
"DATABASE_URL still carries the .env.example placeholder password — set a strong \
one (openssl rand -hex 24) in BOTH DATABASE_URL and POSTGRES_PASSWORD.",
);
}
if !problems.is_empty() {
return Err(anyhow!( return Err(anyhow!(
"Refusing to start in production — {} secret(s) still unset or placeholder:\n - {}\n\ "Refusing to start in production without a real ADMIN_PASSWORD_HASH — \
ALL secrets must be set BEFORE the first `docker compose up -d`: Postgres bakes \ generate one (htpasswd -bnBC 12 '' <password> | tr -d ':\\n')."
POSTGRES_PASSWORD into its data directory on first boot and ignores later changes.",
problems.len(),
problems.join("\n - ")
)); ));
} }
} else if jwt_secret == DEV_JWT_SECRET_SENTINEL { } else if jwt_secret == DEV_JWT_SECRET_SENTINEL {
@@ -121,12 +89,11 @@ impl AppConfig {
let jwt_secret = std::env::var("JWT_SECRET").context("JWT_SECRET must be set")?; let jwt_secret = std::env::var("JWT_SECRET").context("JWT_SECRET must be set")?;
let admin_password_hash = std::env::var("ADMIN_PASSWORD_HASH").unwrap_or_default(); let admin_password_hash = std::env::var("ADMIN_PASSWORD_HASH").unwrap_or_default();
let database_url = std::env::var("DATABASE_URL").context("DATABASE_URL must be set")?;
validate_secrets(is_prod, &jwt_secret, &admin_password_hash, &database_url)?; validate_secrets(is_prod, &jwt_secret, &admin_password_hash)?;
Ok(Self { Ok(Self {
database_url, database_url: std::env::var("DATABASE_URL").context("DATABASE_URL must be set")?,
jwt_secret, jwt_secret,
session_expiry_days: std::env::var("SESSION_EXPIRY_DAYS") session_expiry_days: std::env::var("SESSION_EXPIRY_DAYS")
.unwrap_or_else(|_| "30".to_string()) .unwrap_or_else(|_| "30".to_string())
@@ -174,18 +141,12 @@ mod tests {
const REAL_SECRET: &str = "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2"; const REAL_SECRET: &str = "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2";
const REAL_HASH: &str = "$2y$12$abcdefghijklmnopqrstuv.wxyzABCDEFGHIJKLMNOPQRSTUVWXYZ012"; const REAL_HASH: &str = "$2y$12$abcdefghijklmnopqrstuv.wxyzABCDEFGHIJKLMNOPQRSTUVWXYZ012";
const REAL_DB_URL: &str = "postgres://eventsnap:7f3a9c1e5b2d8a4f@db:5432/eventsnap";
#[test] #[test]
fn prod_rejects_shipped_placeholder_secret() { fn prod_rejects_shipped_placeholder_secret() {
// The exact string shipped in `.env` — >32 chars, so it must be caught by // The exact string shipped in `.env` — >32 chars, so it must be caught by
// the substring guard, not the length check. // the substring guard, not the length check.
let err = validate_secrets( let err = validate_secrets(true, "change_me_to_a_random_64_byte_hex_string", REAL_HASH);
true,
"change_me_to_a_random_64_byte_hex_string",
REAL_HASH,
REAL_DB_URL,
);
assert!( assert!(
err.is_err(), err.is_err(),
"placeholder JWT_SECRET must be rejected in prod" "placeholder JWT_SECRET must be rejected in prod"
@@ -194,108 +155,29 @@ mod tests {
#[test] #[test]
fn prod_rejects_dev_sentinel_and_short_secret() { fn prod_rejects_dev_sentinel_and_short_secret() {
assert!(validate_secrets(true, DEV_JWT_SECRET_SENTINEL, REAL_HASH, REAL_DB_URL).is_err()); assert!(validate_secrets(true, DEV_JWT_SECRET_SENTINEL, REAL_HASH).is_err());
assert!(validate_secrets(true, "tooshort", REAL_HASH, REAL_DB_URL).is_err()); assert!(validate_secrets(true, "tooshort", REAL_HASH).is_err());
} }
#[test] #[test]
fn prod_rejects_missing_or_placeholder_admin_hash() { fn prod_rejects_missing_or_placeholder_admin_hash() {
assert!(validate_secrets(true, REAL_SECRET, "", REAL_DB_URL).is_err()); assert!(validate_secrets(true, REAL_SECRET, "").is_err());
assert!( assert!(validate_secrets(true, REAL_SECRET, "$2y$12$placeholder_replace_me").is_err());
validate_secrets(
true,
REAL_SECRET,
"$2y$12$placeholder_replace_me",
REAL_DB_URL
)
.is_err()
);
}
/// The stack used to come up GREEN on the database password published in the repo: this guard
/// covered the two secrets it was written for, and nothing anywhere looked at the Postgres
/// credential. README step 2 doesn't name POSTGRES_PASSWORD either, so following the
/// documented procedure verbatim shipped it.
#[test]
fn prod_rejects_the_shipped_placeholder_database_password() {
let shipped = "postgres://eventsnap:CHANGE_ME_use_a_strong_password@db:5432/eventsnap";
let err = validate_secrets(true, REAL_SECRET, REAL_HASH, shipped).unwrap_err();
assert!(
err.to_string().contains("DATABASE_URL"),
"the refusal must name DATABASE_URL, not just fail: {err}"
);
// And it must point at the initdb trap, or the operator edits .env, restarts, and lands
// in a permanent auth-failure loop instead.
assert!(
err.to_string().contains("POSTGRES_PASSWORD"),
"the refusal must name POSTGRES_PASSWORD as the other half: {err}"
);
}
/// Every problem in ONE message. Reporting them one per boot made fixing two secrets cost two
/// restart cycles, on a stack where Caddy waits on the unhealthy app the whole time.
#[test]
fn prod_reports_every_placeholder_at_once() {
let err = validate_secrets(
true,
"change_me_to_a_random_64_byte_hex_string",
"$2y$12$placeholder_replace_me",
"postgres://eventsnap:CHANGE_ME_use_a_strong_password@db:5432/eventsnap",
)
.unwrap_err()
.to_string();
for expected in ["JWT_SECRET", "ADMIN_PASSWORD_HASH", "DATABASE_URL"] {
assert!(err.contains(expected), "{expected} missing from: {err}");
}
assert!(
err.contains("3 secret(s)"),
"the count must match what is listed: {err}"
);
} }
#[test] #[test]
fn prod_accepts_real_secrets() { fn prod_accepts_real_secrets() {
assert!(validate_secrets(true, REAL_SECRET, REAL_HASH, REAL_DB_URL).is_ok()); assert!(validate_secrets(true, REAL_SECRET, REAL_HASH).is_ok());
}
/// A real password that happens to contain no placeholder substring must pass — including one
/// with URL-ish punctuation, so the guard can't be mistaken for a URL validator.
#[test]
fn prod_accepts_a_real_database_url_with_awkward_punctuation() {
assert!(
validate_secrets(
true,
REAL_SECRET,
REAL_HASH,
"postgres://eventsnap:aB3%24xY9-_.qW@db:5432/eventsnap"
)
.is_ok()
);
}
/// The e2e stack runs without APP_ENV=production, so none of this applies there — but assert
/// it, because a guard that tripped in e2e would be found the hard way.
#[test]
fn non_prod_ignores_a_placeholder_database_url() {
assert!(
validate_secrets(
false,
REAL_SECRET,
"",
"postgres://eventsnap:CHANGE_ME_use_a_strong_password@db:5432/eventsnap"
)
.is_ok()
);
} }
#[test] #[test]
fn non_prod_tolerates_dev_sentinel() { fn non_prod_tolerates_dev_sentinel() {
assert!(validate_secrets(false, DEV_JWT_SECRET_SENTINEL, "", REAL_DB_URL).is_ok()); assert!(validate_secrets(false, DEV_JWT_SECRET_SENTINEL, "").is_ok());
} }
#[test] #[test]
fn non_prod_still_rejects_short_non_sentinel_secret() { fn non_prod_still_rejects_short_non_sentinel_secret() {
assert!(validate_secrets(false, "tooshort", "", REAL_DB_URL).is_err()); assert!(validate_secrets(false, "tooshort", "").is_err());
} }
#[test] #[test]
@@ -303,23 +185,9 @@ mod tests {
// looks_placeholder lowercases before matching — an upper/mixed-case // looks_placeholder lowercases before matching — an upper/mixed-case
// placeholder must still be rejected in prod. // placeholder must still be rejected in prod.
assert!( assert!(
validate_secrets( validate_secrets(true, "CHANGE_ME_TO_A_RANDOM_64_BYTE_HEX_STRING", REAL_HASH).is_err()
true,
"CHANGE_ME_TO_A_RANDOM_64_BYTE_HEX_STRING",
REAL_HASH,
REAL_DB_URL
)
.is_err()
);
assert!(
validate_secrets(
true,
REAL_SECRET,
"$2Y$12$PLACEHOLDER_replace_me",
REAL_DB_URL
)
.is_err()
); );
assert!(validate_secrets(true, REAL_SECRET, "$2Y$12$PLACEHOLDER_replace_me").is_err());
} }
#[test] #[test]
@@ -329,7 +197,7 @@ mod tests {
const LEN_31: &str = "abcdefghijklmnopqrstuvwxyz01234"; const LEN_31: &str = "abcdefghijklmnopqrstuvwxyz01234";
assert_eq!(LEN_32.len(), 32); assert_eq!(LEN_32.len(), 32);
assert_eq!(LEN_31.len(), 31); assert_eq!(LEN_31.len(), 31);
assert!(validate_secrets(true, LEN_32, REAL_HASH, REAL_DB_URL).is_ok()); assert!(validate_secrets(true, LEN_32, REAL_HASH).is_ok());
assert!(validate_secrets(true, LEN_31, REAL_HASH, REAL_DB_URL).is_err()); assert!(validate_secrets(true, LEN_31, REAL_HASH).is_err());
} }
} }

62
backend/src/db.rs
View File

@@ -4,75 +4,17 @@ use sqlx::postgres::PgPoolOptions;
const DEFAULT_MAX_CONNECTIONS: u32 = 10; const DEFAULT_MAX_CONNECTIONS: u32 = 10;
/// How long a request waits for a pool connection before being shed.
///
/// sqlx's default is 30 s — longer than the frontend's own 20 s fetch timeout (api.ts
/// TIMEOUT_MS), so under saturation the browser gave up while the server kept holding the
/// slot: the client saw a timeout, the server saw a completed request, and the work was done
/// for nobody. Failing fast sheds load instead of compounding it, and `From<sqlx::Error>` in
/// error.rs turns the timeout into a 503 with a Retry-After rather than a bare 500.
const ACQUIRE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5);
/// SQLSTATE for `invalid_password`.
const PG_INVALID_PASSWORD: &str = "28P01";
/// Turn the one connect failure with an unguessable cause into a self-explaining one.
///
/// `POSTGRES_PASSWORD` is honoured ONLY when Postgres initialises its data directory. Change it in
/// `.env` afterwards and the app authenticates with the new password against a volume that still
/// holds the old one — a permanent restart loop whose only symptom is
/// `password authentication failed`.
///
/// The production secret guard makes that sequence NEARLY CERTAIN rather than rare: it stops the
/// app on the first `docker compose up -d`, but not the `db` service in that same command, which
/// initialises and bakes in whatever password was in `.env` at that moment. So the intended
/// recovery — see the refusal, fix your secrets, boot again — is exactly the sequence that breaks
/// it. Nothing in the error names the cause, and the remedy destroys data, so it is the last thing
/// an operator should guess at.
fn explain_auth_failure(err: &sqlx::Error) {
let is_auth_failure = match err {
sqlx::Error::Database(db) => db.code().as_deref() == Some(PG_INVALID_PASSWORD),
_ => false,
};
if !is_auth_failure {
return;
}
tracing::error!(
"Postgres rejected the credentials in DATABASE_URL (SQLSTATE {PG_INVALID_PASSWORD}).\n\
\n\
This almost always means POSTGRES_PASSWORD was changed AFTER the database volume was \
first created. Postgres applies that variable only when it initialises its data \
directory; editing .env and restarting does not change the stored password, so the two \
drift apart permanently.\n\
\n\
If the event has NOT started and you have no data worth keeping:\n\n \
docker compose down -v && docker compose up -d\n\n\
(-v DELETES the database, the uploaded media and the exports. There is no undo.)\n\
\n\
If you DO have data: restore the old password into DATABASE_URL instead, or change the \
stored one with ALTER ROLE inside the running db container. Never reach for -v to fix a \
login problem on a live event."
);
}
pub async fn create_pool(database_url: &str) -> Result<PgPool> { pub async fn create_pool(database_url: &str) -> Result<PgPool> {
let max_connections = std::env::var("DATABASE_MAX_CONNECTIONS") let max_connections = std::env::var("DATABASE_MAX_CONNECTIONS")
.ok() .ok()
.and_then(|s| s.parse::<u32>().ok()) .and_then(|s| s.parse::<u32>().ok())
.unwrap_or(DEFAULT_MAX_CONNECTIONS); .unwrap_or(DEFAULT_MAX_CONNECTIONS);
let pool = match PgPoolOptions::new() let pool = PgPoolOptions::new()
.max_connections(max_connections) .max_connections(max_connections)
.acquire_timeout(ACQUIRE_TIMEOUT)
.connect(database_url) .connect(database_url)
.await .await
{ .context("failed to connect to database")?;
Ok(pool) => pool,
Err(e) => {
explain_auth_failure(&e);
return Err(e).context("failed to connect to database");
}
};
sqlx::migrate!() sqlx::migrate!()
.run(&pool) .run(&pool)

101
backend/src/error.rs
View File

@@ -19,12 +19,6 @@ pub enum AppError {
/// the client can treat it as *terminal* (413, no retry) instead of backing off and /// the client can treat it as *terminal* (413, no retry) instead of backing off and
/// retrying a permanently-failing upload forever. /// retrying a permanently-failing upload forever.
QuotaExceeded(String), QuotaExceeded(String),
/// The server is temporarily unable to serve this request — currently only pool
/// saturation. Distinct from `Internal` because it is TRANSIENT and the client should be
/// told so: a 500 reads as "this request is broken", while a 503 + Retry-After reads as
/// "come back shortly", which is what the upload queue's retry classifier needs to make
/// the right call. Second field: optional retry-after seconds.
ServiceUnavailable(String, Option<u64>),
Internal(anyhow::Error), Internal(anyhow::Error),
} }
@@ -39,9 +33,6 @@ impl AppError {
Self::Conflict(_) => (StatusCode::CONFLICT, "conflict"), Self::Conflict(_) => (StatusCode::CONFLICT, "conflict"),
Self::TooManyRequests(..) => (StatusCode::TOO_MANY_REQUESTS, "too_many_requests"), Self::TooManyRequests(..) => (StatusCode::TOO_MANY_REQUESTS, "too_many_requests"),
Self::QuotaExceeded(_) => (StatusCode::PAYLOAD_TOO_LARGE, "quota_exceeded"), Self::QuotaExceeded(_) => (StatusCode::PAYLOAD_TOO_LARGE, "quota_exceeded"),
Self::ServiceUnavailable(..) => {
(StatusCode::SERVICE_UNAVAILABLE, "service_unavailable")
}
Self::Internal(_) => (StatusCode::INTERNAL_SERVER_ERROR, "internal_error"), Self::Internal(_) => (StatusCode::INTERNAL_SERVER_ERROR, "internal_error"),
} }
} }
@@ -55,7 +46,6 @@ impl AppError {
| Self::NotFound(msg) | Self::NotFound(msg)
| Self::Conflict(msg) => msg.clone(), | Self::Conflict(msg) => msg.clone(),
Self::TooManyRequests(msg, _) => msg.clone(), Self::TooManyRequests(msg, _) => msg.clone(),
Self::ServiceUnavailable(msg, _) => msg.clone(),
Self::QuotaExceeded(msg) => msg.clone(), Self::QuotaExceeded(msg) => msg.clone(),
Self::Internal(err) => { Self::Internal(err) => {
tracing::error!("internal error: {err:#}"); tracing::error!("internal error: {err:#}");
@@ -68,13 +58,10 @@ impl AppError {
impl IntoResponse for AppError { impl IntoResponse for AppError {
fn into_response(self) -> Response { fn into_response(self) -> Response {
let (status, code) = self.status_and_code(); let (status, code) = self.status_and_code();
// BOTH retry-carrying variants must be matched here. `message()` would fail to let retry_after_secs = if let Self::TooManyRequests(_, Some(secs)) = &self {
// compile on a missing arm; this one would not — it would silently drop the header and Some(*secs)
// the `retry_after_secs` body field, which is exactly the sort of omission that only } else {
// shows up under the load the 503 exists for. None
let retry_after_secs = match &self {
Self::TooManyRequests(_, secs) | Self::ServiceUnavailable(_, secs) => *secs,
_ => None,
}; };
let message = self.message(); let message = self.message();
@@ -106,84 +93,6 @@ impl From<anyhow::Error> for AppError {
impl From<sqlx::Error> for AppError { impl From<sqlx::Error> for AppError {
fn from(err: sqlx::Error) -> Self { fn from(err: sqlx::Error) -> Self {
match err { Self::Internal(err.into())
// Pool saturation is load, not a bug. Reporting it as a 500 was actively harmful:
// the frontend's upload-queue classifier treats 5xx as transient and retries, so
// the retries piled straight back into the saturated pool with no Retry-After to
// pace them. A 503 says the same thing honestly and carries the backoff.
//
// `PoolClosed` stays `Internal` — it only happens during shutdown, where a 503
// would invite a retry against a server that is going away.
sqlx::Error::PoolTimedOut => {
tracing::warn!("database pool exhausted; shedding a request with 503");
Self::ServiceUnavailable(
"Server ist gerade ausgelastet. Bitte versuche es in ein paar Sekunden erneut."
.into(),
Some(POOL_TIMEOUT_RETRY_AFTER_SECS),
)
}
other => Self::Internal(other.into()),
}
}
}
/// Retry-After for a shed request. Short: pool saturation clears in seconds once the queue
/// drains, and a long value would make a brief spike feel like an outage.
const POOL_TIMEOUT_RETRY_AFTER_SECS: u64 = 3;
#[cfg(test)]
mod tests {
use super::*;
/// `into_response` extracts `retry_after_secs` by MATCHING ON VARIANTS, so unlike
/// `message()` a missing arm is not a compile error — it silently drops the header. Pin the
/// behaviour for both retry-carrying variants.
#[test]
fn both_retry_carrying_variants_emit_retry_after() {
for err in [
AppError::TooManyRequests("slow down".into(), Some(42)),
AppError::ServiceUnavailable("busy".into(), Some(3)),
] {
let expected = match &err {
AppError::TooManyRequests(_, Some(s)) | AppError::ServiceUnavailable(_, Some(s)) => {
s.to_string()
}
_ => unreachable!(),
};
let resp = err.into_response();
assert_eq!(
resp.headers()
.get(axum::http::header::RETRY_AFTER)
.and_then(|v| v.to_str().ok()),
Some(expected.as_str()),
"a shed/throttled client must be told when to come back"
);
}
}
/// Pool saturation is load, not a bug. A 500 makes the frontend's retry classifier pile
/// straight back into the saturated pool with no backoff to pace it.
#[test]
fn pool_exhaustion_sheds_with_503_but_shutdown_does_not() {
let shed: AppError = sqlx::Error::PoolTimedOut.into();
assert_eq!(
shed.status_and_code(),
(StatusCode::SERVICE_UNAVAILABLE, "service_unavailable")
);
// PoolClosed only happens during shutdown; a 503 there would invite a retry against a
// server that is going away.
let closing: AppError = sqlx::Error::PoolClosed.into();
assert_eq!(
closing.status_and_code(),
(StatusCode::INTERNAL_SERVER_ERROR, "internal_error")
);
// Everything else must keep its existing mapping.
let missing: AppError = sqlx::Error::RowNotFound.into();
assert_eq!(
missing.status_and_code(),
(StatusCode::INTERNAL_SERVER_ERROR, "internal_error")
);
} }
} }

17
backend/src/handlers/admin.rs
View File

@@ -197,23 +197,6 @@ pub async fn patch_config(
"Wert für {key} liegt außerhalb des zulässigen Bereichs ({min}–{max})." "Wert für {key} liegt außerhalb des zulässigen Bereichs ({min}–{max})."
))); )));
} }
// Zero is in range and catastrophic. `quota_tolerance` is the multiplier in
// `free_disk * tolerance / active_uploaders`, so 0 makes every per-user 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. `storage_quota_enabled` is the
// intended off-switch.
//
// Rejecting the value rather than raising the floor: very small tolerances are
// legitimate (they are how a large disk is throttled down to a sensible per-guest
// ceiling, and how the e2e quota tests steer it — around 1e-5 on a 174 GB volume), so
// a floor of, say, 0.01 would forbid real configurations to prevent one typo.
if key_str == "quota_tolerance" && n == 0.0 {
return Err(AppError::BadRequest(
"quota_tolerance = 0 würde jeden Upload blockieren. Zum Abschalten der \
Speicher-Quote stattdessen „Speicher-Quote aktiv“ ausschalten."
.into(),
));
}
} else if BOOL_KEYS.contains(&key_str) { } else if BOOL_KEYS.contains(&key_str) {
match value.trim().to_ascii_lowercase().as_str() { match value.trim().to_ascii_lowercase().as_str() {
"true" | "false" | "1" | "0" | "yes" | "no" | "on" | "off" => {} "true" | "false" | "1" | "0" | "yes" | "no" | "on" | "off" => {}

43
backend/src/handlers/health.rs
View File

@@ -1,43 +0,0 @@
use std::time::Duration;
use axum::extract::State;
use axum::http::StatusCode;
use crate::state::AppState;
/// How long a readiness probe waits for the database before calling the app not-ready.
///
/// Deliberately shorter than the pool's own `acquire_timeout`: a saturated pool IS a
/// not-ready condition, and reporting it as such is the point. Do not "fix" the two to
/// match — that would make the probe wait out the very saturation it exists to surface.
const READY_TIMEOUT: Duration = Duration::from_secs(2);
/// Readiness: can this instance actually serve a request end to end?
///
/// Separate from `/health` (liveness) ON PURPOSE, and the compose healthcheck must keep
/// pointing at `/health`. `caddy` declares `depends_on: app: {condition: service_healthy}`,
/// so a DB-dependent healthcheck would turn a transient Postgres hiccup during boot into
/// the reverse proxy refusing to start — converting a blip into a total outage. This route
/// is for an external monitor, which should page rather than restart.
///
/// The gap this closes: every request path touches the database, so with a dead or
/// saturated pool the app is useless while `/health` still answers "ok" — the disk-full
/// endgame (media volume fills, Postgres cannot write WAL) stayed green all the way down.
pub async fn ready(State(state): State<AppState>) -> (StatusCode, &'static str) {
let probe = sqlx::query_scalar::<_, i32>("SELECT 1").fetch_one(&state.pool);
match tokio::time::timeout(READY_TIMEOUT, probe).await {
Ok(Ok(_)) => (StatusCode::OK, "ready"),
Ok(Err(e)) => {
tracing::warn!(error = ?e, "readiness probe failed: database unreachable");
(StatusCode::SERVICE_UNAVAILABLE, "database unavailable")
}
Err(_) => {
tracing::warn!(
timeout_secs = READY_TIMEOUT.as_secs(),
"readiness probe timed out; the pool is saturated or the database is hung"
);
(StatusCode::SERVICE_UNAVAILABLE, "database timeout")
}
}
}

1
backend/src/handlers/mod.rs
View File

@@ -1,6 +1,5 @@
pub mod admin; pub mod admin;
pub mod feed; pub mod feed;
pub mod health;
pub mod host; pub mod host;
pub mod me; pub mod me;
pub mod public; pub mod public;

21
backend/src/handlers/sse.rs
View File

@@ -38,26 +38,7 @@ pub async fn issue_ticket(
State(state): State<AppState>, State(state): State<AppState>,
auth: AuthUser, auth: AuthUser,
) -> Result<Json<StreamTicketResponse>, AppError> { ) -> Result<Json<StreamTicketResponse>, AppError> {
// The endpoint had no rate limit at all. Authentication is not a bound here: one valid let ticket = state.sse_tickets.issue(auth.token_hash);
// session could loop it freely. 60/min is far above a real client (one ticket per SSE
// (re)connect, and reconnects are backed off) while capping a loop.
if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("sse_ticket:{}", auth.user_id),
60,
Duration::from_secs(60),
) {
return Err(AppError::TooManyRequests(
"Zu viele Verbindungsversuche. Bitte warte kurz.".into(),
Some(retry_after_secs),
));
}
let ticket = state.sse_tickets.issue(auth.token_hash).ok_or_else(|| {
AppError::ServiceUnavailable(
"Server ist gerade ausgelastet. Live-Updates folgen in Kürze.".into(),
Some(30),
)
})?;
let server_time = sqlx::query_scalar("SELECT NOW()") let server_time = sqlx::query_scalar("SELECT NOW()")
.fetch_one(&state.pool) .fetch_one(&state.pool)
.await?; .await?;

360
backend/src/handlers/upload.rs
View File

@@ -17,135 +17,6 @@ use crate::state::AppState;
const MAX_CAPTION_LENGTH: usize = 2000; const MAX_CAPTION_LENGTH: usize = 2000;
/// Byte ceiling for the caption field, enforced WHILE reading it.
///
/// `Field::text()` buffers the entire field before returning, and this is the one route whose
/// `DefaultBodyLimit` is raised to 576 MiB (main.rs) — so `caption=<576 MiB of text>` allocated
/// 576 MiB of heap per concurrent request inside a 1 GiB container, and the
/// `MAX_CAPTION_LENGTH` check only ran afterwards, on a string that had already been built.
/// 4 bytes per code point is the worst case for UTF-8, so this can never reject a caption the
/// character limit would have accepted.
const MAX_CAPTION_BYTES: usize = MAX_CAPTION_LENGTH * 4;
/// Byte ceiling for the raw hashtag CSV. Generous next to what the tag caps below allow.
const MAX_HASHTAGS_BYTES: usize = 4 * 1024;
/// Hashtags stored per upload. The CSV was never length-checked at all and was split into an
/// unbounded `Vec`, then upserted TAG BY TAG inside the commit transaction — which holds a
/// `FOR SHARE` lock on the event row, so one request could stall every other upload behind
/// tens of thousands of round trips.
const MAX_HASHTAGS_PER_UPLOAD: usize = 30;
/// Characters per stored tag. `extract_hashtags` already self-bounds at 40; this covers the CSV
/// path, which had no bound of its own.
const MAX_HASHTAG_LENGTH: usize = 50;
/// Read a multipart text field, refusing it the moment it exceeds `max_bytes`.
///
/// The point is to fail DURING the read rather than after it — `Field::text()` cannot, because
/// it has already allocated the whole thing by the time it returns.
async fn read_text_field_bounded(
mut field: axum::extract::multipart::Field<'_>,
max_bytes: usize,
) -> Result<String, AppError> {
let mut buf: Vec<u8> = Vec::new();
while let Some(chunk) = field
.chunk()
.await
.map_err(|e| AppError::BadRequest(e.to_string()))?
{
if buf.len() + chunk.len() > max_bytes {
return Err(AppError::BadRequest(
"Eingabe ist zu lang.".to_string(),
));
}
buf.extend_from_slice(&chunk);
}
String::from_utf8(buf).map_err(|_| AppError::BadRequest("Ungültige Zeichenkodierung.".into()))
}
/// Normalise, dedupe and CAP the tags for one upload.
///
/// Extracted as a pure function so the caps are testable without standing up multipart, and
/// shared by the upload and edit paths — which previously disagreed: upload lowercased and
/// stripped `#`, while edit upserted raw strings, so `#Party` via edit and `party` via upload
/// became two different hashtag rows.
///
/// Truncates rather than rejecting. `extract_hashtags` legitimately derives tags from a
/// 2000-character caption, and 400-ing a guest for writing an enthusiastic caption would be a
/// worse outcome than silently keeping the first 30.
fn normalize_tags(caption_tags: Vec<String>, csv: Option<&str>) -> Vec<String> {
let mut tags = caption_tags;
if let Some(csv) = csv {
for tag in csv.split(',') {
let t = tag.trim().trim_start_matches('#').to_lowercase();
if !t.is_empty() {
tags.push(t);
}
}
}
tags.sort();
tags.dedup();
tags.retain(|t| t.chars().count() <= MAX_HASHTAG_LENGTH);
tags.truncate(MAX_HASHTAGS_PER_UPLOAD);
tags
}
/// Owns the bytes an in-flight upload has written to disk, and deletes them unless the
/// request reaches the point where a database row takes ownership.
///
/// Reclaim used to be 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 a `.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` (which is row-driven) can never see it,
/// and no sweeper existed for the originals directory. Those bytes are also invisible to the
/// quota while still consuming the free disk that `quota_limit_bytes` divides among guests.
///
/// A drop guard is the only construct that survives cancellation, because dropping the future
/// is exactly what runs it.
struct TempFileGuard {
/// `None` once disarmed — a row now owns these bytes.
path: Option<std::path::PathBuf>,
}
impl TempFileGuard {
fn new(path: std::path::PathBuf) -> Self {
Self { path: Some(path) }
}
/// Follow the bytes to their new location after a rename.
///
/// NOT `disarm`. Between the rename and the commit the file exists under its FINAL name
/// with still no row pointing at it, so that window needs guarding just as much as the
/// `.tmp` did — arguably more, since a leftover final-named original looks legitimate.
fn retarget(&mut self, path: std::path::PathBuf) {
self.path = Some(path);
}
/// Hand ownership to the committed row. Only correct after `tx.commit()` succeeds.
fn disarm(&mut self) {
self.path = None;
}
}
impl Drop for TempFileGuard {
fn drop(&mut self) {
let Some(path) = self.path.take() else {
return;
};
// std::fs, not tokio::fs: `Drop` cannot await, and a runtime-dependent unlink is not
// guaranteed a live runtime here (shutdown drops in-flight tasks).
match std::fs::remove_file(&path) {
Ok(()) => tracing::debug!(path = %path.display(), "reclaimed an abandoned upload"),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
Err(e) => {
tracing::warn!(error = ?e, path = %path.display(), "failed to reclaim an abandoned upload")
}
}
}
}
/// Allowlist of accepted media types, keyed by the MIME that `infer` derives from /// Allowlist of accepted media types, keyed by the MIME that `infer` derives from
/// the file's magic bytes. The detected MIME (not the client-declared one) is what /// the file's magic bytes. The detected MIME (not the client-declared one) is what
/// we trust, store, and hand to the compression pipeline — so a text-based payload /// we trust, store, and hand to the compression pipeline — so a text-based payload
@@ -236,18 +107,13 @@ pub async fn upload(
.media_path .media_path
.join(format!("originals/{event_slug}")); .join(format!("originals/{event_slug}"));
let temp_abs = originals_dir.join(format!("{upload_id}.tmp")); let temp_abs = originals_dir.join(format!("{upload_id}.tmp"));
// Armed before anything can create the file, so there is no window in which bytes exist
// unowned. From here on, EVERY exit — return, `?`, panic, or the future being dropped
// mid-body by a client disconnect — reclaims them, and the explicit `remove_file` calls
// that used to be sprinkled over the return paths are gone. One owner, one rule.
let mut file_guard = TempFileGuard::new(temp_abs.clone());
let mut streamed: Option<(i64, Vec<u8>)> = None; // (size, head bytes for sniffing) let mut streamed: Option<(i64, Vec<u8>)> = None; // (size, head bytes for sniffing)
let mut caption: Option<String> = None; let mut caption: Option<String> = None;
let mut hashtags_csv: Option<String> = None; let mut hashtags_csv: Option<String> = None;
// The multipart read is wrapped so the field loop can use `?` freely; reclaiming the temp // Wrap the multipart read so any error after the temp file is created still cleans
// file on failure is `file_guard`'s job, not this block's. // it up (a mid-stream parse failure must not leave a stray `.tmp` on disk).
let parse_result: Result<(), AppError> = async { let parse_result: Result<(), AppError> = async {
while let Some(field) = multipart while let Some(field) = multipart
.next_field() .next_field()
@@ -277,10 +143,20 @@ pub async fn upload(
streamed = Some(stream_field_to_file(field, &temp_abs, cap_bytes).await?); streamed = Some(stream_field_to_file(field, &temp_abs, cap_bytes).await?);
} }
"caption" => { "caption" => {
caption = Some(read_text_field_bounded(field, MAX_CAPTION_BYTES).await?); caption = Some(
field
.text()
.await
.map_err(|e| AppError::BadRequest(e.to_string()))?,
);
} }
"hashtags" => { "hashtags" => {
hashtags_csv = Some(read_text_field_bounded(field, MAX_HASHTAGS_BYTES).await?); hashtags_csv = Some(
field
.text()
.await
.map_err(|e| AppError::BadRequest(e.to_string()))?,
);
} }
_ => {} _ => {}
} }
@@ -289,10 +165,13 @@ pub async fn upload(
} }
.await; .await;
parse_result?; if let Err(e) = parse_result {
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(e);
}
// From here on the temp file may exist. Every exit reclaims it via `file_guard` — see // From here on the temp file may exist; every validation failure removes it before
// TempFileGuard for why the explicit per-branch cleanup this replaced was not enough. // returning so a rejected upload never leaves bytes behind.
let (size, head) = match streamed { let (size, head) = match streamed {
Some(s) => s, Some(s) => s,
None => return Err(AppError::BadRequest("Keine Datei hochgeladen.".into())), None => return Err(AppError::BadRequest("Keine Datei hochgeladen.".into())),
@@ -304,6 +183,7 @@ pub async fn upload(
if let Some(ref cap) = caption if let Some(ref cap) = caption
&& cap.chars().count() > MAX_CAPTION_LENGTH && cap.chars().count() > MAX_CAPTION_LENGTH
{ {
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(AppError::BadRequest(format!( return Err(AppError::BadRequest(format!(
"Beschreibung ist zu lang. Maximum: {} Zeichen.", "Beschreibung ist zu lang. Maximum: {} Zeichen.",
MAX_CAPTION_LENGTH MAX_CAPTION_LENGTH
@@ -318,6 +198,7 @@ pub async fn upload(
let kind = match infer::get(&head) { let kind = match infer::get(&head) {
Some(k) => k, Some(k) => k,
None => { None => {
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(AppError::BadRequest( return Err(AppError::BadRequest(
"Dateityp nicht erkannt oder nicht unterstützt.".into(), "Dateityp nicht erkannt oder nicht unterstützt.".into(),
)); ));
@@ -330,6 +211,7 @@ pub async fn upload(
{ {
Some(v) => v, Some(v) => v,
None => { None => {
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(AppError::BadRequest(format!( return Err(AppError::BadRequest(format!(
"Dateityp wird nicht unterstützt: {}.", "Dateityp wird nicht unterstützt: {}.",
kind.mime_type() kind.mime_type()
@@ -344,6 +226,7 @@ pub async fn upload(
max_image_mb * 1024 * 1024 max_image_mb * 1024 * 1024
}; };
if size > max_bytes { if size > max_bytes {
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(AppError::BadRequest(format!( return Err(AppError::BadRequest(format!(
"Datei ist zu groß. Maximum: {} MB.", "Datei ist zu groß. Maximum: {} MB.",
max_bytes / (1024 * 1024) max_bytes / (1024 * 1024)
@@ -362,6 +245,7 @@ pub async fn upload(
%mime, megapixels = ?mp, %mime, megapixels = ?mp,
"rejecting an image that exceeds the decode budget at admission" "rejecting an image that exceeds the decode budget at admission"
); );
let _ = tokio::fs::remove_file(&temp_abs).await;
let detail = mp.map_or(String::new(), |mp| format!(" (ca. {mp:.0} Megapixel)")); let detail = mp.map_or(String::new(), |mp| format!(" (ca. {mp:.0} Megapixel)"));
return Err(AppError::BadRequest(format!( return Err(AppError::BadRequest(format!(
"Bild hat zu viele Bildpunkte{detail} und kann nicht verarbeitet werden. \ "Bild hat zu viele Bildpunkte{detail} und kann nicht verarbeitet werden. \
@@ -387,6 +271,7 @@ pub async fn upload(
quota_limit = Some(limit); quota_limit = Some(limit);
let prospective_total = user.total_upload_bytes.saturating_add(size); let prospective_total = user.total_upload_bytes.saturating_add(size);
if prospective_total > limit { if prospective_total > limit {
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(AppError::QuotaExceeded( return Err(AppError::QuotaExceeded(
"Du hast dein Upload-Limit für dieses Event erreicht.".into(), "Du hast dein Upload-Limit für dieses Event erreicht.".into(),
)); ));
@@ -401,20 +286,22 @@ pub async fn upload(
tokio::fs::rename(&temp_abs, &absolute_path) tokio::fs::rename(&temp_abs, &absolute_path)
.await .await
.map_err(|e| AppError::Internal(e.into()))?; .map_err(|e| AppError::Internal(e.into()))?;
// THERE MUST BE NO `.await` BETWEEN THE RENAME AND THIS LINE. Both statements resolve on
// the same poll, so the future cannot be dropped between them and the guard is never
// pointing at a path that no longer holds the bytes. If the rename fails the guard still
// owns `temp_abs`, which is why retargeting comes after it rather than before.
file_guard.retarget(absolute_path.clone());
// Process hashtags from caption and explicit CSV, capped — see `normalize_tags`. // Process hashtags from caption and explicit CSV
let tags = normalize_tags( let mut tags: Vec<String> = Vec::new();
caption if let Some(ref cap) = caption {
.as_deref() tags.extend(hashtag::extract_hashtags(cap));
.map(hashtag::extract_hashtags) }
.unwrap_or_default(), if let Some(ref csv) = hashtags_csv {
hashtags_csv.as_deref(), for tag in csv.split(',') {
); let t = tag.trim().trim_start_matches('#').to_lowercase();
if !t.is_empty() {
tags.push(t);
}
}
}
tags.sort();
tags.dedup();
// Quota accounting, the upload row, and its hashtag links must be atomic: a // Quota accounting, the upload row, and its hashtag links must be atomic: a
// crash between the bytes increment and the insert would permanently charge // crash between the bytes increment and the insert would permanently charge
@@ -499,10 +386,15 @@ pub async fn upload(
} }
.await; .await;
// The committed row now references these bytes — hand ownership over. Anything other than // The file is already on disk at `absolute_path`. If the transaction failed, no DB
// a successful commit leaves the guard armed, so the file is reclaimed on the way out. // row will ever reference it, so remove it now rather than orphan bytes on disk.
let upload = tx_result?; let upload = match tx_result {
file_guard.disarm(); Ok(u) => u,
Err(e) => {
let _ = tokio::fs::remove_file(&absolute_path).await;
return Err(e);
}
};
// Spawn compression task // Spawn compression task
state state
@@ -557,57 +449,6 @@ pub async fn edit_upload(
return Err(AppError::Forbidden("Nur eigene Uploads bearbeiten.".into())); return Err(AppError::Forbidden("Nur eigene Uploads bearbeiten.".into()));
} }
// This endpoint had no rate limit of any kind, while every other mutating route has one.
let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await;
let edit_rate_on = config::get_bool(&state.config_cache, "upload_edit_rate_enabled", true).await;
if rate_limits_on && edit_rate_on {
let edit_rate =
config::get_i64(&state.config_cache, "upload_edit_rate_per_min", 30).await as usize;
if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("upload_edit:{}", auth.user_id),
edit_rate,
Duration::from_secs(60),
) {
return Err(AppError::TooManyRequests(
"Zu viele Änderungen. Bitte warte kurz.".into(),
Some(retry_after_secs),
));
}
}
// Validate to the same limits as the upload path. This route had none at all, so a caption
// rejected at upload could be set here instead, and the tags went in raw — meaning `#Party`
// via edit and `party` via upload became two different hashtag rows.
if let Some(ref caption) = body.caption
&& caption.chars().count() > MAX_CAPTION_LENGTH
{
return Err(AppError::BadRequest(format!(
"Beschreibung ist zu lang. Maximum: {MAX_CAPTION_LENGTH} Zeichen."
)));
}
let normalized_tags = body
.hashtags
.as_ref()
.map(|tags| normalize_tags(tags.clone(), None));
// A PATCH that changes nothing must not retire the keepsake generation.
//
// `invalidate_and_arm` below ran unconditionally, outside both `if let Some(...)` guards, so
// `PATCH {}` — which any authenticated guest can send in a loop against their own upload —
// bumped export_epoch and armed a fresh pair of full-gallery export workers every time.
// REGEN_DEBOUNCE bounds the rate of that, not the total work, so the keepsake could be kept
// permanently un-downloadable.
//
// Residual, deliberately not fixed: re-sending an IDENTICAL hashtag list still counts as a
// change. Comparing would need another query, and unlike `PATCH {}` it is not a free loop.
let caption_changed = match (&body.caption, &upload.caption) {
(Some(new), existing) => Some(new.as_str()) != existing.as_deref(),
(None, _) => false,
};
if !caption_changed && normalized_tags.is_none() {
return Ok(StatusCode::OK);
}
// Caption update + hashtag wipe-then-relink in one transaction, so a crash // Caption update + hashtag wipe-then-relink in one transaction, so a crash
// mid-relink can't leave the upload with its hashtags stripped. // mid-relink can't leave the upload with its hashtags stripped.
// //
@@ -623,7 +464,7 @@ pub async fn edit_upload(
if let Some(ref caption) = body.caption { if let Some(ref caption) = body.caption {
Upload::update_caption(&mut *tx, upload_id, Some(caption)).await?; Upload::update_caption(&mut *tx, upload_id, Some(caption)).await?;
} }
if let Some(ref hashtags) = normalized_tags { if let Some(ref hashtags) = body.hashtags {
Hashtag::unlink_all_from_upload(&mut *tx, upload_id).await?; Hashtag::unlink_all_from_upload(&mut *tx, upload_id).await?;
for tag in hashtags { for tag in hashtags {
let h = Hashtag::upsert(&mut *tx, auth.event_id, tag).await?; let h = Hashtag::upsert(&mut *tx, auth.event_id, tag).await?;
@@ -1237,103 +1078,4 @@ mod tests {
fn full_tolerance_is_identity_for_a_single_uploader() { fn full_tolerance_is_identity_for_a_single_uploader() {
assert_eq!(quota_limit_bytes(500, 1.0, 1), 500); assert_eq!(quota_limit_bytes(500, 1.0, 1), 500);
} }
/// The guard is the only reclaim mechanism that survives a client disconnect, so its three
/// states have to be exactly right — a wrong `disarm` leaks bytes forever, a wrong `Drop`
/// deletes a committed guest's photo.
mod temp_file_guard {
use super::super::TempFileGuard;
fn scratch(name: &str) -> std::path::PathBuf {
let dir = std::env::temp_dir().join(format!("es-guard-{}", std::process::id()));
std::fs::create_dir_all(&dir).unwrap();
let p = dir.join(name);
std::fs::write(&p, b"bytes").unwrap();
p
}
#[test]
fn dropping_an_armed_guard_reclaims_the_file() {
let p = scratch("armed.tmp");
drop(TempFileGuard::new(p.clone()));
assert!(!p.exists(), "an abandoned upload must not survive the request");
}
#[test]
fn a_disarmed_guard_leaves_the_file_alone() {
let p = scratch("committed.jpg");
let mut g = TempFileGuard::new(p.clone());
g.disarm();
drop(g);
assert!(p.exists(), "a committed upload's bytes must never be deleted");
}
#[test]
fn retarget_follows_the_rename_and_forgets_the_old_path() {
let old = scratch("old.tmp");
let new = scratch("new.jpg");
let mut g = TempFileGuard::new(old.clone());
// The rename already moved the bytes; only the new path is at risk now.
std::fs::remove_file(&old).unwrap();
g.retarget(new.clone());
drop(g);
assert!(!new.exists(), "the final-named original is orphaned too until the row commits");
}
#[test]
fn a_guard_whose_file_is_already_gone_is_harmless() {
let p = scratch("vanished.tmp");
std::fs::remove_file(&p).unwrap();
drop(TempFileGuard::new(p)); // must not panic
}
}
/// The CSV hashtag path had no length check at all and was upserted tag-by-tag INSIDE the
/// commit transaction — which holds a FOR SHARE lock on the event row, so one request could
/// stall every other upload behind tens of thousands of round trips.
mod hashtag_caps {
use super::super::{MAX_HASHTAGS_PER_UPLOAD, MAX_HASHTAG_LENGTH, normalize_tags};
#[test]
fn a_huge_csv_is_capped_not_upserted_in_full() {
let csv = (0..10_000)
.map(|i| format!("tag{i}"))
.collect::<Vec<_>>()
.join(",");
let tags = normalize_tags(vec![], Some(&csv));
assert_eq!(tags.len(), MAX_HASHTAGS_PER_UPLOAD);
}
#[test]
fn an_overlong_tag_is_dropped_rather_than_stored() {
let long = "a".repeat(MAX_HASHTAG_LENGTH + 1);
let tags = normalize_tags(vec![], Some(&format!("ok,{long}")));
assert_eq!(tags, vec!["ok"]);
}
#[test]
fn tags_are_normalised_and_deduped_across_both_sources() {
// The upload path lowercased and stripped `#` while the edit path did not, so
// `#Party` and `party` became two different hashtag rows. One helper, one rule.
let tags = normalize_tags(vec!["party".into()], Some("#Party, PARTY ,tanz"));
assert_eq!(tags, vec!["party", "tanz"]);
}
#[test]
fn truncation_keeps_a_stable_prefix_not_an_arbitrary_one() {
// Sorted before truncation, so the same input always yields the same tags —
// otherwise an edit could silently shuffle which 30 survived.
let csv = "zulu,alpha,mike,bravo";
assert_eq!(
normalize_tags(vec![], Some(csv)),
normalize_tags(vec![], Some("bravo,mike,alpha,zulu"))
);
}
#[test]
fn an_empty_or_absent_csv_yields_nothing() {
assert!(normalize_tags(vec![], None).is_empty());
assert!(normalize_tags(vec![], Some(",, ,")).is_empty());
}
}
} }

19
backend/src/main.rs
View File

@@ -27,15 +27,8 @@ async fn main() -> Result<()> {
tracing_subscriber::registry() tracing_subscriber::registry()
.with( .with(
// `info`, not `debug`. A stock deploy sets RUST_LOG nowhere (it is absent from
// .env.example and was absent from docker-compose.yml), so this fallback IS the
// production level — and at `debug` the TraceLayer below emits a line per request
// AND per response, into a log file that had no rotation. `tower_http=warn`
// rather than `info` states the intent: those spans are diagnostics, not an
// access log, and a future `DefaultOnResponse::new().level(Level::INFO)` should
// not silently re-enable them.
tracing_subscriber::EnvFilter::try_from_default_env() tracing_subscriber::EnvFilter::try_from_default_env()
.unwrap_or_else(|_| "eventsnap_backend=info,tower_http=warn".into()), .unwrap_or_else(|_| "eventsnap_backend=debug,tower_http=debug".into()),
) )
.with(tracing_subscriber::fmt::layer()) .with(tracing_subscriber::fmt::layer())
.init(); .init();
@@ -58,12 +51,6 @@ async fn main() -> Result<()> {
// originals are never touched, so a failure just retries on the next start. // originals are never touched, so a failure just retries on the next start.
state.compression.backfill_stale_derivatives().await; state.compression.backfill_stale_derivatives().await;
// Re-extract poster frames for videos a restart interrupted. `startup_recovery` above
// marks their compression `failed` but nothing re-enqueued them, so `thumbnail_path`
// stayed NULL for the rest of the event. Shares the attempt budget with the image
// backfill, so a clip that genuinely yields no frame stops being retried.
state.compression.backfill_video_posters().await;
// Re-spawn exports for events that were released but whose keepsake never finished // Re-spawn exports for events that were released but whose keepsake never finished
// (crash mid-export). Needs the media/export paths + SSE sender, so it runs here // (crash mid-export). Needs the media/export paths + SSE sender, so it runs here
// rather than inside `startup_recovery`. Fire-and-forget: the workers run in the // rather than inside `startup_recovery`. Fire-and-forget: the workers run in the
@@ -261,11 +248,7 @@ async fn main() -> Result<()> {
// four subtrees. Deleting the route removes the vector outright rather than racing the // four subtrees. Deleting the route removes the vector outright rather than racing the
// decoder; `/media/**` now 404s regardless of encoding. // decoder; `/media/**` now 404s regardless of encoding.
let router = Router::new() let router = Router::new()
// Liveness. Stays dependency-free — the compose healthcheck gates Caddy's startup
// on it, so anything that can fail transiently must NOT be in here.
.route("/health", get(|| async { "ok" })) .route("/health", get(|| async { "ok" }))
// Readiness. Touches the pool; for an external monitor, not for the compose gate.
.route("/health/ready", get(handlers::health::ready))
.merge(api) .merge(api)
.layer(TraceLayer::new_for_http()) .layer(TraceLayer::new_for_http())
.with_state(state); .with_state(state);

56
backend/src/models/upload.rs
View File

@@ -150,62 +150,10 @@ impl Upload {
/// Stamp which revision of the derivative pipeline produced this row's preview/display, /// Stamp which revision of the derivative pipeline produced this row's preview/display,
/// so the startup backfill can find rows generated by an older one exactly once. /// so the startup backfill can find rows generated by an older one exactly once.
///
/// Also clears the attempt counter: success is the only thing that resets it, and folding
/// the reset in here means both the live path and the backfill get it with no extra call
/// site to forget.
pub async fn set_derivatives_rev(pool: &PgPool, id: Uuid, rev: i16) -> Result<(), sqlx::Error> { pub async fn set_derivatives_rev(pool: &PgPool, id: Uuid, rev: i16) -> Result<(), sqlx::Error> {
sqlx::query( sqlx::query("UPDATE upload SET derivatives_rev = $2 WHERE id = $1")
"UPDATE upload
SET derivatives_rev = $2, derivative_attempts = 0, derivative_last_error = NULL
WHERE id = $1",
)
.bind(id)
.bind(rev)
.execute(pool)
.await?;
Ok(())
}
/// Record that derivative processing is ABOUT to be attempted, returning the new count.
///
/// WRITE-AHEAD ON PURPOSE. The failure this bounds is a cgroup SIGKILL: the process
/// vanishes mid-work, so no `Err` is returned, no error handler runs and no `Drop` fires.
/// A counter incremented after a failure would increment zero times per crash and the
/// boot loop would be unchanged. Counting the ATTEMPT is the only thing that survives the
/// process dying. The cost is that a genuinely transient failure also burns an attempt —
/// acceptable, because the retry budget is per-boot-loop, not per-request, and success
/// resets it to zero.
/// `None` when the row no longer exists (hard-deleted, or an e2e TRUNCATE landed while the
/// task waited on the semaphore) — the caller should abandon quietly rather than treat a
/// missing row as a processing failure.
pub async fn begin_derivative_attempt(
pool: &PgPool,
id: Uuid,
) -> Result<Option<i16>, sqlx::Error> {
sqlx::query_scalar(
"UPDATE upload
SET derivative_attempts = derivative_attempts + 1
WHERE id = $1
RETURNING derivative_attempts",
)
.bind(id)
.fetch_optional(pool)
.await
}
/// Store why the last derivative attempt failed. Diagnostics only — nothing branches on it.
pub async fn record_derivative_failure(
pool: &PgPool,
id: Uuid,
error: &str,
) -> Result<(), sqlx::Error> {
// Bounded: an anyhow chain can be long, and this is written on a failure path that may
// repeat across every row of a bad batch.
let truncated: String = error.chars().take(500).collect();
sqlx::query("UPDATE upload SET derivative_last_error = $2 WHERE id = $1")
.bind(id) .bind(id)
.bind(truncated) .bind(rev)
.execute(pool) .execute(pool)
.await?; .await?;
Ok(()) Ok(())

71
backend/src/models/user.rs
View File

@@ -60,54 +60,6 @@ impl User {
.await .await
} }
/// Create a user with an explicit role, in ONE statement.
///
/// `create` + a separate `UPDATE ... SET role` is not equivalent: a crash or a pool error
/// between the two leaves a GUEST row holding a reserved name, which is exactly the
/// poisoned state that bricked admin login — now self-inflicted, and invisible to a
/// role-based lookup, so the next login would create yet another.
pub async fn create_with_role(
pool: &PgPool,
event_id: Uuid,
display_name: &str,
pin_hash: &str,
role: UserRole,
) -> Result<Self, sqlx::Error> {
sqlx::query_as::<_, Self>(
"INSERT INTO \"user\" (event_id, display_name, recovery_pin_hash, role)
VALUES ($1, $2, $3, $4)
RETURNING *",
)
.bind(event_id)
.bind(display_name)
.bind(pin_hash)
.bind(role)
.fetch_one(pool)
.await
}
/// The event's admin, looked up BY ROLE.
///
/// The name is not the identity and never was. Looking the admin up by `display_name`
/// meant any guest who joined as "Admin" first made the lookup miss, and the fallback
/// `create` then violated the case-insensitive unique index from migration 007 — a
/// permanent 500 on admin login, recoverable only by hand-editing the database.
///
/// `ORDER BY created_at` so a database that somehow acquired two admin rows resolves to a
/// stable one rather than alternating between them.
pub async fn find_admin_for_event(
pool: &PgPool,
event_id: Uuid,
) -> Result<Option<Self>, sqlx::Error> {
sqlx::query_as::<_, Self>(
"SELECT * FROM \"user\" WHERE event_id = $1 AND role = 'admin'
ORDER BY created_at ASC LIMIT 1",
)
.bind(event_id)
.fetch_optional(pool)
.await
}
pub async fn find_by_id(pool: &PgPool, id: Uuid) -> Result<Option<Self>, sqlx::Error> { pub async fn find_by_id(pool: &PgPool, id: Uuid) -> Result<Option<Self>, sqlx::Error> {
sqlx::query_as::<_, Self>("SELECT * FROM \"user\" WHERE id = $1") sqlx::query_as::<_, Self>("SELECT * FROM \"user\" WHERE id = $1")
.bind(id) .bind(id)
@@ -144,31 +96,14 @@ impl User {
Ok(row.0) Ok(row.0)
} }
/// Window after which a failed-PIN streak is forgotten. Matches the lockout duration, so
/// "wait out the cooldown" and "start clean" are the same interval to a guest.
const PIN_ATTEMPT_DECAY_MINUTES: i64 = 15;
/// Record a wrong PIN and return the CURRENT streak length.
///
/// The counter decays: before this, it only ever cleared on a successful recovery or after
/// a lockout expired, so ordinary typos accumulated across days and a guest could arrive at
/// an event already most of the way to being locked out by mistakes made the night before.
/// Decay is what makes the raised lock threshold safe rather than merely lenient.
pub async fn increment_failed_pin(pool: &PgPool, id: Uuid) -> Result<i16, sqlx::Error> { pub async fn increment_failed_pin(pool: &PgPool, id: Uuid) -> Result<i16, sqlx::Error> {
let row: (i16,) = sqlx::query_as( let row: (i16,) = sqlx::query_as(
"UPDATE \"user\" "UPDATE \"user\"
SET failed_pin_attempts = CASE SET failed_pin_attempts = failed_pin_attempts + 1
WHEN last_failed_pin_at IS NULL
OR last_failed_pin_at < NOW() - ($2 || ' minutes')::interval
THEN 1
ELSE failed_pin_attempts + 1
END,
last_failed_pin_at = NOW()
WHERE id = $1 WHERE id = $1
RETURNING failed_pin_attempts", RETURNING failed_pin_attempts",
) )
.bind(id) .bind(id)
.bind(Self::PIN_ATTEMPT_DECAY_MINUTES.to_string())
.fetch_one(pool) .fetch_one(pool)
.await?; .await?;
Ok(row.0) Ok(row.0)
@@ -189,9 +124,7 @@ impl User {
pub async fn reset_pin_attempts(pool: &PgPool, id: Uuid) -> Result<(), sqlx::Error> { pub async fn reset_pin_attempts(pool: &PgPool, id: Uuid) -> Result<(), sqlx::Error> {
sqlx::query( sqlx::query(
"UPDATE \"user\" "UPDATE \"user\" SET failed_pin_attempts = 0, pin_locked_until = NULL WHERE id = $1",
SET failed_pin_attempts = 0, pin_locked_until = NULL, last_failed_pin_at = NULL
WHERE id = $1",
) )
.bind(id) .bind(id)
.execute(pool) .execute(pool)

616
backend/src/services/compression.rs
View File

@@ -13,9 +13,6 @@ use crate::state::SseEvent;
#[derive(Clone)] #[derive(Clone)]
pub struct CompressionWorker { pub struct CompressionWorker {
semaphore: Arc<Semaphore>, semaphore: Arc<Semaphore>,
/// Serialises the memory-heavy image jobs — see `HEAVY_IMAGE_BYTES`. Separate from
/// `semaphore` so ordinary photos keep full concurrency.
heavy: Arc<Semaphore>,
pool: PgPool, pool: PgPool,
media_path: PathBuf, media_path: PathBuf,
sse_tx: broadcast::Sender<SseEvent>, sse_tx: broadcast::Sender<SseEvent>,
@@ -34,7 +31,6 @@ impl CompressionWorker {
) -> Self { ) -> Self {
Self { Self {
semaphore: Arc::new(Semaphore::new(concurrency)), semaphore: Arc::new(Semaphore::new(concurrency)),
heavy: Arc::new(Semaphore::new(1)),
pool, pool,
media_path, media_path,
sse_tx, sse_tx,
@@ -62,21 +58,6 @@ impl CompressionWorker {
/// next start. Rev 1 = EXIF orientation is applied. /// next start. Rev 1 = EXIF orientation is applied.
const DERIVATIVES_REV: i16 = 1; const DERIVATIVES_REV: i16 = 1;
/// How many times derivative generation may be ATTEMPTED for one upload before it is left
/// alone. Counted write-ahead and reset on success — see `Upload::begin_derivative_attempt`.
///
/// This is what turns a fatal input from an outage into a blemish. The startup backfill
/// runs unconditionally on every boot, so before this bound a row whose processing killed
/// the process was re-selected and re-run forever, and `restart: unless-stopped` made that
/// an infinite loop that also dropped every SSE stream and truncated every in-flight
/// upload on each cycle. Three attempts absorbs genuinely transient infrastructure
/// failures (an ENOSPC spike, a pool blip) without ever becoming unbounded.
const MAX_DERIVATIVE_ATTEMPTS: i16 = 3;
/// Rows regenerated per boot. Bounds both the query and the amount of work a single start
/// can queue; whatever is left is picked up on the next boot.
const BACKFILL_BATCH: i64 = 200;
/// Spawn a background task to process an uploaded file. /// Spawn a background task to process an uploaded file.
pub fn process(&self, upload_id: Uuid, original_path: String, mime_type: String) { pub fn process(&self, upload_id: Uuid, original_path: String, mime_type: String) {
let worker = self.clone(); let worker = self.clone();
@@ -183,21 +164,6 @@ impl CompressionWorker {
let original = self.media_path.join(original_path); let original = self.media_path.join(original_path);
if mime_type.starts_with("image/") { if mime_type.starts_with("image/") {
// Count the attempt BEFORE doing the work — see `begin_derivative_attempt`. If this
// input is the one that kills the container, this write is the only record that
// survives, and it is what stops the boot backfill replaying it forever.
match Upload::begin_derivative_attempt(&self.pool, upload_id).await? {
Some(attempts) if attempts > Self::MAX_DERIVATIVE_ATTEMPTS => {
anyhow::bail!(
"derivative generation gave up after {} attempt(s)",
attempts - 1
);
}
Some(_) => {}
// The row vanished while this task waited on the semaphore. Nothing to do, and
// reporting a failure would broadcast into a stream that no longer has a card.
None => return Ok(()),
}
let (preview_rel, display_rel) = self let (preview_rel, display_rel) = self
.generate_image_derivatives(upload_id, &original, mime_type) .generate_image_derivatives(upload_id, &original, mime_type)
.await?; .await?;
@@ -206,26 +172,9 @@ impl CompressionWorker {
Upload::set_derivatives_rev(&self.pool, upload_id, Self::DERIVATIVES_REV).await?; Upload::set_derivatives_rev(&self.pool, upload_id, Self::DERIVATIVES_REV).await?;
tracing::info!("preview + display generated for upload {upload_id}"); tracing::info!("preview + display generated for upload {upload_id}");
} else if mime_type.starts_with("video/") { } else if mime_type.starts_with("video/") {
// A missing poster must NOT fail the upload. `set_thumbnail_path` is only reached when let thumb_rel = self.generate_video_thumbnail(upload_id, &original).await?;
// a file really exists, so `thumbnail_path` stays NULL otherwise — which every consumer Upload::set_thumbnail_path(&self.pool, upload_id, &thumb_rel).await?;
// already handles (FeedListCard, VirtualFeed, LightboxModal are all null-safe). tracing::info!("thumbnail generated for upload {upload_id}");
//
// The `?` here used to hide the defect; making the check strict without also making
// this non-fatal would have been far worse than the bug. Every clip of a second or less
// would fail compression, exhaust its retries and be soft-deleted — a cosmetic defect
// turned into data loss, on exactly the mis-tap/Live-Photo clips guests produce most.
match self.generate_video_thumbnail(upload_id, &original).await? {
Some(thumb_rel) => {
Upload::set_thumbnail_path(&self.pool, upload_id, &thumb_rel).await?;
tracing::info!("thumbnail generated for upload {upload_id}");
}
None => {
tracing::warn!(
%upload_id,
"no poster frame could be extracted; the video keeps its own tile"
);
}
}
} }
Upload::set_compression_status(&self.pool, upload_id, "done").await?; Upload::set_compression_status(&self.pool, upload_id, "done").await?;
@@ -239,38 +188,6 @@ impl CompressionWorker {
/// Longest edge of the phone-feed "preview" (data-saver default). /// Longest edge of the phone-feed "preview" (data-saver default).
const PREVIEW_MAX_EDGE: u32 = 800; const PREVIEW_MAX_EDGE: u32 = 800;
/// Above this pixel count the PNG original is stored as uploaded, unoptimised.
///
/// oxipng's peak memory scales with PIXELS, not file size: it decodes the PNG itself and
/// then evaluates row filters, each trial holding a full-size buffer. That is why a 2.82
/// MiB file could measure 1250 MiB of peak RSS inside a 1 GiB container — smooth,
/// synthetic content compresses to almost nothing on disk while still being 8000x8000.
/// 8 MP covers every real phone photo; beyond it we decline the (lossless, cosmetic)
/// saving rather than risk the OOM kill.
const OXIPNG_MAX_PIXELS: u64 = 8_000_000;
/// Estimated peak heap above which an image job takes the exclusive `heavy` permit.
///
/// `compression_concurrency` (default 2) bounds how many jobs run at once, but says
/// nothing about how much memory each one costs, and the container gets 1 GiB total. A
/// single 8000x8000 original measures ~516 MiB peak even with the decode correctly scoped
/// — two of those overlapping is 1032 MiB and another OOM kill, from nothing more exotic
/// than two guests uploading big photos at the same moment.
///
/// 150 MiB sits far above a normal phone photo (a 12 MP JPEG costs ~50 MiB all-in) so the
/// common path never serialises, and far below the point where two jobs stop fitting.
/// Throughput is unaffected for everything except the rare giant, which is exactly the
/// case that must not run in parallel with another giant.
const HEAVY_IMAGE_BYTES: u64 = 150 * 1024 * 1024;
/// Wall-clock ceiling for one oxipng run.
///
/// Bounds TIME, NOT MEMORY — oxipng checks the deadline between trials, so a single trial
/// still allocates in full. The pixel gate above and the sequential build (see
/// `default-features = false` in Cargo.toml) are what bound memory. Do not treat this
/// constant as the OOM fix.
const OXIPNG_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(20);
/// Decode the image ONCE and emit both derivatives — the 800px `preview` (phone feed) /// Decode the image ONCE and emit both derivatives — the 800px `preview` (phone feed)
/// and the 2048px `display` (diashow). Returns `(preview_rel, display_rel)`. /// and the 2048px `display` (diashow). Returns `(preview_rel, display_rel)`.
async fn generate_image_derivatives( async fn generate_image_derivatives(
@@ -289,28 +206,53 @@ impl CompressionWorker {
let display_path = displays_dir.join(&filename); let display_path = displays_dir.join(&filename);
let original = original.to_path_buf(); let original = original.to_path_buf();
let mime_owned = mime_type.to_string(); let mime_owned = mime_type.to_string();
let preview_max = Self::PREVIEW_MAX_EDGE;
// Estimate the peak from the HEADER (no pixels decoded — the same kind of cheap probe let display_max = Self::DISPLAY_MAX_EDGE;
// the upload handler already does via `exceeds_decode_budget`) and, if this job is a
// giant, take the exclusive permit so it cannot overlap another giant. Held for the
// whole blocking section, released on drop including on error.
let estimate =
crate::services::imaging::estimated_processing_peak_bytes(&original, Self::DISPLAY_MAX_EDGE);
let _heavy_permit = match estimate {
Some(bytes) if bytes > Self::HEAVY_IMAGE_BYTES => {
tracing::debug!(
%upload_id,
estimated_mib = bytes / (1024 * 1024),
"waiting for the heavy-image permit"
);
Some(self.heavy.acquire().await)
}
_ => None,
};
// Run blocking image operations in a spawn_blocking task // Run blocking image operations in a spawn_blocking task
tokio::task::spawn_blocking(move || { tokio::task::spawn_blocking(move || -> Result<()> {
write_image_derivatives(upload_id, &original, &mime_owned, &preview_path, &display_path) // Decompression-bomb limits + EXIF orientation, both in one place — see
// services::imaging for why neither may be skipped.
let img = crate::services::imaging::decode_oriented(&original)?;
// Preview: max 800px, preserving aspect ratio (data-saver feed).
img.resize(
preview_max,
preview_max,
image::imageops::FilterType::Lanczos3,
)
.save_with_format(&preview_path, image::ImageFormat::Jpeg)
.context("failed to save preview")?;
// Display: max 2048px for the diashow. Only DOWNSCALE — never upscale a smaller
// original (that adds bytes with no quality gain); re-encode it as JPEG as-is.
let display = if img.width() > display_max || img.height() > display_max {
img.resize(
display_max,
display_max,
image::imageops::FilterType::Lanczos3,
)
} else {
img
};
display
.save_with_format(&display_path, image::ImageFormat::Jpeg)
.context("failed to save display")?;
// If the original is PNG, try lossless compression in-place
if mime_owned == "image/png" {
let opts = oxipng::Options::from_preset(2);
let _ = oxipng::optimize(
&oxipng::InFile::Path(original),
&oxipng::OutFile::Path {
path: None,
preserve_attrs: true,
},
&opts,
);
}
Ok(())
}) })
.await??; .await??;
@@ -331,29 +273,17 @@ impl CompressionWorker {
/// ///
/// Unlike the failure path in `process`, a backfill error is logged and skipped — it must /// Unlike the failure path in `process`, a backfill error is logged and skipped — it must
/// NEVER destroy or soft-delete an upload that already has a working preview. /// NEVER destroy or soft-delete an upload that already has a working preview.
///
/// Bounded in three ways, all of them load-bearing on a box that restarts itself:
/// `derivative_attempts` stops a fatal row being replayed on every boot, `BACKFILL_BATCH`
/// stops one start queueing unbounded work, and the whole thing runs as ONE task walking
/// the rows sequentially rather than N tasks racing for the same semaphore.
pub async fn backfill_stale_derivatives(&self) { pub async fn backfill_stale_derivatives(&self) {
// `original_path IS NOT NULL` was dead — the column is NOT NULL. What actually needs
// excluding is the blanked path `cleanup_deleted_media` leaves behind.
let rows = sqlx::query_as::<_, (Uuid, String, String)>( let rows = sqlx::query_as::<_, (Uuid, String, String)>(
"SELECT id, original_path, mime_type FROM upload "SELECT id, original_path, mime_type FROM upload
WHERE deleted_at IS NULL AND mime_type LIKE 'image/%' WHERE deleted_at IS NULL AND mime_type LIKE 'image/%'
AND original_path <> '' AND original_path IS NOT NULL
AND derivative_attempts < $2
AND ( AND (
(display_path IS NULL AND preview_path IS NOT NULL) (display_path IS NULL AND preview_path IS NOT NULL)
OR derivatives_rev < $1 OR derivatives_rev < $1
) )",
ORDER BY created_at DESC
LIMIT $3",
) )
.bind(Self::DERIVATIVES_REV) .bind(Self::DERIVATIVES_REV)
.bind(Self::MAX_DERIVATIVE_ATTEMPTS)
.bind(Self::BACKFILL_BATCH)
.fetch_all(&self.pool) .fetch_all(&self.pool)
.await; .await;
let rows = match rows { let rows = match rows {
@@ -363,32 +293,14 @@ impl CompressionWorker {
return; return;
} }
}; };
self.report_exhausted_derivatives().await;
if rows.is_empty() { if rows.is_empty() {
return; return;
} }
tracing::info!("regenerating derivatives for {} upload(s)", rows.len()); tracing::info!("regenerating derivatives for {} upload(s)", rows.len());
for (id, original_path, mime_type) in rows {
// ONE task for the whole batch. The previous shape spawned a task per row, so a large let worker = self.clone();
// backlog created thousands of live tasks that each held a pool handle and queued on tokio::spawn(async move {
// the same two semaphore permits, competing with live uploads for the entire boot.
let worker = self.clone();
tokio::spawn(async move {
for (id, original_path, mime_type) in rows {
let _permit = worker.semaphore.acquire().await; let _permit = worker.semaphore.acquire().await;
// Write-ahead, exactly as in the live path: if this row is the one that kills
// the process, this increment is the only thing that outlives the SIGKILL.
match Upload::begin_derivative_attempt(&worker.pool, id).await {
Ok(Some(n)) if n > Self::MAX_DERIVATIVE_ATTEMPTS => continue,
Ok(Some(_)) => {}
Ok(None) => continue,
Err(e) => {
tracing::warn!(error = ?e, %id, "could not record a backfill attempt; skipping");
continue;
}
}
let original = worker.media_path.join(&original_path); let original = worker.media_path.join(&original_path);
match worker match worker
.generate_image_derivatives(id, &original, &mime_type) .generate_image_derivatives(id, &original, &mime_type)
@@ -397,8 +309,6 @@ impl CompressionWorker {
Ok((preview_rel, display_rel)) => { Ok((preview_rel, display_rel)) => {
let _ = Upload::set_preview_path(&worker.pool, id, &preview_rel).await; let _ = Upload::set_preview_path(&worker.pool, id, &preview_rel).await;
let _ = Upload::set_display_path(&worker.pool, id, &display_rel).await; let _ = Upload::set_display_path(&worker.pool, id, &display_rel).await;
// Clears derivative_attempts too, so a row that failed transiently is
// not one boot closer to being abandoned.
let _ = let _ =
Upload::set_derivatives_rev(&worker.pool, id, Self::DERIVATIVES_REV) Upload::set_derivatives_rev(&worker.pool, id, Self::DERIVATIVES_REV)
.await; .await;
@@ -406,400 +316,64 @@ impl CompressionWorker {
} }
Err(e) => { Err(e) => {
// Leave the existing derivatives and the original intact; this row is // Leave the existing derivatives and the original intact; this row is
// retried on the next start until its attempt budget runs out. The rev // simply retried on the next start. The rev stays behind, which is the
// stays behind, which is the marker that it still needs doing. // marker that it still needs doing.
tracing::warn!(error = ?e, %id, "derivative backfill failed; leaving as-is"); tracing::warn!(error = ?e, %id, "derivative backfill failed; leaving as-is");
let _ =
Upload::record_derivative_failure(&worker.pool, id, &format!("{e:#}"))
.await;
} }
} }
} });
});
}
/// Re-extract poster frames for videos that never got one.
///
/// A video interrupted by a restart is stranded: `startup_recovery` flips its
/// `compression_status` from `processing` to `failed` and nothing re-enqueues it, so
/// `thumbnail_path` stays NULL forever while the clip itself plays fine. The feed shows a
/// posterless tile for the rest of the event, and after
/// `FAILED_ORIGINAL_RETENTION_DAYS` the reclaim sweep is entitled to the original.
///
/// Shares `derivative_attempts` with the image backfill on purpose. Note the consequence,
/// which is intended rather than a bug to fix later: `extract_poster_frame` returning
/// `Ok(false)` is a NORMAL, permanent outcome for a sub-second clip (Live Photos,
/// mis-taps), and since the counter is write-ahead and only cleared by a real success,
/// those clips stop being re-ffmpeg'd on every boot once the budget is spent.
pub async fn backfill_video_posters(&self) {
let rows = sqlx::query_as::<_, (Uuid, String)>(
"SELECT id, original_path FROM upload
WHERE deleted_at IS NULL AND mime_type LIKE 'video/%'
AND thumbnail_path IS NULL
AND original_path <> ''
AND derivative_attempts < $1
ORDER BY created_at DESC
LIMIT $2",
)
.bind(Self::MAX_DERIVATIVE_ATTEMPTS)
.bind(Self::BACKFILL_BATCH)
.fetch_all(&self.pool)
.await;
let rows = match rows {
Ok(r) => r,
Err(e) => {
tracing::warn!(error = ?e, "video poster backfill query failed");
return;
}
};
if rows.is_empty() {
return;
}
tracing::info!("re-extracting posters for {} video(s)", rows.len());
let worker = self.clone();
tokio::spawn(async move {
for (id, original_path) in rows {
let _permit = worker.semaphore.acquire().await;
match Upload::begin_derivative_attempt(&worker.pool, id).await {
Ok(Some(n)) if n > Self::MAX_DERIVATIVE_ATTEMPTS => continue,
Ok(Some(_)) => {}
Ok(None) => continue,
Err(e) => {
tracing::warn!(error = ?e, %id, "could not record a poster attempt; skipping");
continue;
}
}
let original = worker.media_path.join(&original_path);
match worker.generate_video_thumbnail(id, &original).await {
Ok(Some(thumb_rel)) => {
if Upload::set_thumbnail_path(&worker.pool, id, &thumb_rel)
.await
.is_ok()
{
// Clears the attempt counter: a video that eventually succeeded
// must not carry a budget scar into a future pipeline revision.
let _ = Upload::set_derivatives_rev(
&worker.pool,
id,
Self::DERIVATIVES_REV,
)
.await;
tracing::info!("poster regenerated for upload {id}");
}
}
// No frame at all — normal for a very short clip. The tile stays
// posterless and the attempt is spent, which is what stops the retry.
Ok(None) => {
tracing::debug!(%id, "still no poster frame; leaving the tile as-is");
}
Err(e) => {
tracing::warn!(error = ?e, %id, "poster backfill failed; leaving as-is");
let _ =
Upload::record_derivative_failure(&worker.pool, id, &format!("{e:#}"))
.await;
}
}
}
});
}
/// Say out loud, once per boot, that some uploads have stopped being retried.
///
/// Without this the give-up is invisible: the loop stops (which is the point) but the
/// affected photos keep a stale or missing derivative forever with nothing to notice. The
/// originals are untouched, so this is recoverable once the cause is fixed — reset
/// `derivative_attempts` to 0 and restart.
async fn report_exhausted_derivatives(&self) {
let exhausted: Result<i64, _> = sqlx::query_scalar(
"SELECT count(*) FROM upload
WHERE deleted_at IS NULL AND mime_type LIKE 'image/%'
AND derivative_attempts >= $2
AND (
(display_path IS NULL AND preview_path IS NOT NULL)
OR derivatives_rev < $1
)",
)
.bind(Self::DERIVATIVES_REV)
.bind(Self::MAX_DERIVATIVE_ATTEMPTS)
.fetch_one(&self.pool)
.await;
if let Ok(count) = exhausted
&& count > 0
{
tracing::error!(
count,
"{count} upload(s) exhausted derivative regeneration and will no longer be \
retried; their originals are intact — see upload.derivative_last_error, fix \
the cause, then reset derivative_attempts to 0 and restart"
);
} }
} }
/// Extract the feed poster for a video. `Ok(None)` when the clip yields no frame — see async fn generate_video_thumbnail(&self, upload_id: Uuid, original: &Path) -> Result<String> {
/// [`crate::services::video::extract_poster_frame`], which owns the seek order, the timeout and
/// the artifact check that this function used to be missing.
async fn generate_video_thumbnail(
&self,
upload_id: Uuid,
original: &Path,
) -> Result<Option<String>> {
let thumbs_dir = self.media_path.join("thumbnails"); let thumbs_dir = self.media_path.join("thumbnails");
tokio::fs::create_dir_all(&thumbs_dir).await?; tokio::fs::create_dir_all(&thumbs_dir).await?;
let thumb_filename = format!("{upload_id}.jpg"); let thumb_filename = format!("{upload_id}.jpg");
let thumb_path = thumbs_dir.join(&thumb_filename); let thumb_path = thumbs_dir.join(&thumb_filename);
let produced = // Hard timeout — a malformed video can hang `ffmpeg` indefinitely. Without a
crate::services::video::extract_poster_frame(original, &thumb_path, 800).await?; // cap, the held compression-worker semaphore permit is never released and the
// pool eventually deadlocks (no further uploads ever processed). 120s is well
// above the time to extract one frame from any sane input.
let mut child = tokio::process::Command::new("ffmpeg")
.args([
"-i",
original.to_str().unwrap_or_default(),
"-vframes",
"1",
"-ss",
"00:00:01",
"-vf",
"scale=800:-1",
"-y",
thumb_path.to_str().unwrap_or_default(),
])
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped())
.kill_on_drop(true)
.spawn()
.context("failed to spawn ffmpeg")?;
Ok(produced.then(|| format!("thumbnails/{thumb_filename}"))) let status =
} match tokio::time::timeout(std::time::Duration::from_secs(120), child.wait()).await {
} Ok(res) => res.context("ffmpeg wait failed")?,
Err(_) => {
let _ = child.kill().await;
anyhow::bail!("ffmpeg timeout after 120s");
}
};
/// The blocking half of [`CompressionWorker::generate_image_derivatives`]: decode once, write if !status.success() {
/// both derivatives, then optionally shrink a PNG original in place. // Best-effort: drain stderr for the log.
/// let mut stderr = Vec::new();
/// A free function rather than an inline closure so its memory behaviour is directly testable — if let Some(mut handle) = child.stderr.take() {
/// this is the code path that OOM-killed the container, and the fix is a scoping property that a use tokio::io::AsyncReadExt;
/// future edit could silently undo. let _ = handle.read_to_end(&mut stderr).await;
fn write_image_derivatives(
upload_id: Uuid,
original: &Path,
mime_type: &str,
preview_path: &Path,
display_path: &Path,
) -> Result<()> {
let preview_max = CompressionWorker::PREVIEW_MAX_EDGE;
let display_max = CompressionWorker::DISPLAY_MAX_EDGE;
// THE FULL-SIZE DECODE IS SCOPED TO THIS BLOCK ON PURPOSE, and the block yields the
// DISPLAY derivative rather than the original.
//
// `img` is up to 256 MiB (imaging::decode_limits max_alloc) and `resize` only BORROWS it,
// so it used to stay alive through both resizes AND the oxipng call below — which decodes
// the PNG a second time and holds a full-size buffer per filter trial. That measured
// ~1250 MiB of peak RSS for a 2.8 MiB input, inside a 1 GiB cgroup: the container was
// SIGKILLed, taking every SSE stream and every in-flight upload with it.
//
// A block rather than a bare `drop(img)` because a `drop` call is one careless edit away
// from being removed as redundant-looking — and note the `else` arm MOVES `img` out, which
// is what makes "the block's value is the only survivor" true in both arms.
let (display, width, height) = {
// Decompression-bomb limits + EXIF orientation, both in one place — see
// services::imaging for why neither may be skipped.
let img = crate::services::imaging::decode_oriented(original)?;
let (width, height) = (img.width(), img.height());
// Display: max 2048px for the diashow. Only DOWNSCALE — never upscale a smaller
// original (that adds bytes with no quality gain); re-encode it as JPEG as-is.
let display = if width > display_max || height > display_max {
img.resize(
display_max,
display_max,
image::imageops::FilterType::Lanczos3,
)
} else {
img
};
(display, width, height)
};
display
.save_with_format(display_path, image::ImageFormat::Jpeg)
.context("failed to save display")?;
// Preview: max 800px, derived from the DISPLAY, not from the original.
//
// Both derivatives used to resize the full-size decode independently, so a 8000x8000
// original paid for two full-size Lanczos passes and their intermediates — measured 520
// MiB peak even after the scoping fix above, which two concurrent workers cannot fit in a
// 1 GiB container. Chaining 8000 -> 2048 -> 800 makes the second pass operate on 2048px
// input, and the full-size buffer is already freed by the time it runs. Quality is not the
// trade-off here: a staged Lanczos3 downscale to 800px is visually indistinguishable from
// a single-step one (and is a standard technique for large ratios).
display
.resize(
preview_max,
preview_max,
image::imageops::FilterType::Lanczos3,
)
.save_with_format(preview_path, image::ImageFormat::Jpeg)
.context("failed to save preview")?;
drop(display);
let pixels = u64::from(width) * u64::from(height);
// If the original is PNG, try lossless compression in place — but only when its pixel count
// is inside the budget, and never for longer than OXIPNG_TIMEOUT. This is a best-effort size
// saving: declining it costs disk, while attempting it unbounded cost the whole container.
if mime_type == "image/png" {
if pixels <= CompressionWorker::OXIPNG_MAX_PIXELS {
let mut opts = oxipng::Options::from_preset(2);
opts.timeout = Some(CompressionWorker::OXIPNG_TIMEOUT);
let _ = oxipng::optimize(
&oxipng::InFile::Path(original.to_path_buf()),
&oxipng::OutFile::Path {
path: None,
preserve_attrs: true,
},
&opts,
);
} else {
tracing::info!(
%upload_id, pixels,
"skipping oxipng: above the pixel budget; the original is stored as uploaded"
);
}
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
/// Peak resident set of THIS process, in bytes, from `/proc/self/status`.
fn peak_rss_bytes() -> u64 {
let status = std::fs::read_to_string("/proc/self/status").expect("procfs");
let line = status
.lines()
.find(|l| l.starts_with("VmHWM:"))
.expect("VmHWM");
let kb: u64 = line
.split_whitespace()
.nth(1)
.and_then(|v| v.parse().ok())
.expect("VmHWM value");
kb * 1024
}
/// Reset the kernel's peak-RSS watermark so the measurement covers only what follows.
/// Linux 4.0+; writing "5" to `clear_refs` resets `VmHWM` to the current RSS.
fn reset_peak_rss() {
let _ = std::fs::write("/proc/self/clear_refs", "5");
}
/// The pixel gate has to sit below what the axis limits allow, or it can never fire.
#[test]
fn the_oxipng_gate_is_reachable_within_the_decode_limits() {
const _: () = {
// imaging::decode_limits permits 12_000 x 12_000 = 144 MP. A gate above that would
// never skip anything.
assert!(CompressionWorker::OXIPNG_MAX_PIXELS < 12_000 * 12_000);
// ...and it must stay above a 48 MP camera, so real photos still get optimised.
assert!(CompressionWorker::OXIPNG_MAX_PIXELS >= 8_000_000);
};
}
/// The heavy-image gate has to classify the two cases the way the sizing assumed:
/// an ordinary phone photo must NOT serialise, and the giant must.
#[test]
fn the_heavy_gate_separates_a_phone_photo_from_a_giant() {
let dir = std::env::temp_dir().join(format!("es-heavy-{}", std::process::id()));
std::fs::create_dir_all(&dir).unwrap();
// 12 MP, the shape of a default phone capture.
let ordinary = dir.join("ordinary.jpg");
image::RgbImage::new(4032, 3024).save(&ordinary).unwrap();
let ordinary_peak = crate::services::imaging::estimated_processing_peak_bytes(
&ordinary,
CompressionWorker::DISPLAY_MAX_EDGE,
)
.expect("header readable");
assert!(
ordinary_peak <= CompressionWorker::HEAVY_IMAGE_BYTES,
"a 12 MP photo estimated at {} MiB would serialise the common path",
ordinary_peak / 1048576
);
// The 64 MP RGBA case that measured ~516 MiB peak.
let giant = dir.join("giant.png");
image::RgbaImage::new(8000, 8000).save(&giant).unwrap();
let giant_peak = crate::services::imaging::estimated_processing_peak_bytes(
&giant,
CompressionWorker::DISPLAY_MAX_EDGE,
)
.expect("header readable");
assert!(
giant_peak > CompressionWorker::HEAVY_IMAGE_BYTES,
"an 8000x8000 RGBA original estimated at only {} MiB would be allowed to run \
concurrently with another one — 2x its real ~516 MiB peak does not fit in 1 GiB",
giant_peak / 1048576
);
// The estimate must also be in the right ballpark, not merely on the right side of the
// threshold: 244 MiB decode + 262 MiB f32 resize intermediate.
assert!(
(400..700).contains(&(giant_peak / 1048576)),
"estimate {} MiB is far from the measured ~516 MiB peak",
giant_peak / 1048576
);
let _ = std::fs::remove_dir_all(&dir);
}
/// The OOM that took the container down, measured rather than argued.
///
/// An 8000x8000 RGBA PNG passes admission: 256,000,000 bytes is just under the 256 MiB
/// `max_alloc`, and smooth content is a few MB on disk, far under any size cap. The old
/// code kept that ~244 MiB decode alive across an unbounded, multi-threaded oxipng run and
/// peaked at ~1250 MiB — inside a 1 GiB cgroup. Being SIGKILLed there is not a blip: the
/// row was already committed, so the boot backfill replayed the identical workload on every
/// restart.
///
/// `#[ignore]` because it allocates ~250 MiB and takes a few seconds. Run explicitly:
/// cargo test --release oom -- --ignored --nocapture --test-threads=1
/// It must run ALONE — `VmHWM` is per process, so a concurrent test would pollute it.
#[test]
#[ignore = "heavy: allocates ~250 MiB; run with --ignored --test-threads=1"]
fn a_large_png_stays_far_below_the_container_limit() {
const EDGE: u32 = 8_000;
let dir = std::env::temp_dir().join(format!("es-oom-{}", std::process::id()));
std::fs::create_dir_all(&dir).unwrap();
let original = dir.join("big.png");
// Smooth gradient: ~244 MiB decoded, a couple of MB on disk. That gap is the whole
// point — file size tells you nothing about what a PNG costs to process.
{
let mut buf = image::RgbaImage::new(EDGE, EDGE);
for (x, y, px) in buf.enumerate_pixels_mut() {
*px = image::Rgba([(x >> 5) as u8, (y >> 5) as u8, ((x + y) >> 6) as u8, 255]);
} }
buf.save(&original).unwrap(); anyhow::bail!("ffmpeg failed: {}", String::from_utf8_lossy(&stderr));
} }
// Everything above is fixture setup, not the code under test. Ok(format!("thumbnails/{thumb_filename}"))
reset_peak_rss();
let before = peak_rss_bytes();
write_image_derivatives(
Uuid::new_v4(),
&original,
"image/png",
&dir.join("preview.jpg"),
&dir.join("display.jpg"),
)
.expect("derivatives");
let peak = peak_rss_bytes();
let on_disk = std::fs::metadata(&original).unwrap().len();
eprintln!(
"input {:.2} MiB on disk ({EDGE}x{EDGE}); peak RSS {:.0} MiB (was {:.0} MiB before)",
on_disk as f64 / 1048576.0,
peak as f64 / 1048576.0,
before as f64 / 1048576.0
);
assert!(dir.join("preview.jpg").exists() && dir.join("display.jpg").exists());
// The container gets 1 GiB and runs two of these concurrently. 600 MiB is a generous
// ceiling that the old code (~1250 MiB) could not have met.
assert!(
peak < 600 * 1024 * 1024,
"peak RSS {} MiB — the decode is being held across oxipng again, or the pixel \
gate stopped firing",
peak / 1048576
);
let _ = std::fs::remove_dir_all(&dir);
} }
} }

255
backend/src/services/export.rs
View File

@@ -20,29 +20,6 @@ use crate::state::SseEvent;
static VIEWER_DIR: Dir<'_> = include_dir!("$CARGO_MANIFEST_DIR/static/export-viewer"); static VIEWER_DIR: Dir<'_> = include_dir!("$CARGO_MANIFEST_DIR/static/export-viewer");
// ── Shared visibility filter ─────────────────────────────────────────────────
/// The predicate that decides what lands in a keepsake, as ONE definition.
///
/// Two queries have to agree on it: [`query_uploads`], which selects the rows the archives are
/// built from, and [`estimate_export_bytes`], which sizes them for the disk preflight. They used
/// to state it separately, and the direction of drift matters — an estimate that misses rows the
/// archive writes UNDER-reserves, which is the exact ENOSPC the preflight exists to prevent.
///
/// A `SRC:`-marked copy in the integration tests cannot catch that: drift means production moved
/// and the copy didn't, so both sides of such a test sit still and it keeps passing. Sharing the
/// fragment removes the failure by construction instead, and leaves the test doing what it is
/// actually good at — pinning the behaviour.
///
/// CONTRACT: callers must alias `upload` as `u` and join `"user"` as `usr`, and bind the event id
/// as `$1`.
macro_rules! export_visibility_where {
() => {
"WHERE u.event_id = $1 AND u.deleted_at IS NULL
AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE"
};
}
// ── DB query rows ──────────────────────────────────────────────────────────── // ── DB query rows ────────────────────────────────────────────────────────────
#[derive(sqlx::FromRow)] #[derive(sqlx::FromRow)]
@@ -580,7 +557,7 @@ async fn run_zip_export_inner(
}; };
let entry_name = format!("{folder}/{date}_{name_safe}_{}.{ext}", row.id); let entry_name = format!("{folder}/{date}_{name_safe}_{}.{ext}", row.id);
let builder = keepsake_entry(entry_name, Compression::Stored); let builder = ZipEntryBuilder::new(entry_name.into(), Compression::Stored);
// Open BEFORE writing the entry header. A missing source is skipped (the media file // Open BEFORE writing the entry header. A missing source is skipped (the media file
// was deleted, or its processing failed) — but it must be skipped without aborting the // was deleted, or its processing failed) — but it must be skipped without aborting the
@@ -785,32 +762,38 @@ async fn run_html_export_inner(
let full_ext = ext_from_path(&row.original_path); let full_ext = ext_from_path(&row.original_path);
let full = format!("{id_str}.{full_ext}"); let full = format!("{id_str}.{full_ext}");
// Poster frame via the shared helper, which owns the seek order, the 120s timeout // Video thumbnail via ffmpeg
// (this call site had NONE — a hung ffmpeg would strand the export at `running`
// forever) and the artifact check.
let thumb_path = media_tmp.join(&thumb); let thumb_path = media_tmp.join(&thumb);
let produced = let ffmpeg_result = tokio::process::Command::new("ffmpeg")
match crate::services::video::extract_poster_frame(&src, &thumb_path, 400).await { .args([
Ok(produced) => produced, "-i",
Err(e) => { src.to_str().unwrap_or_default(),
tracing::warn!("poster extraction errored for upload {}: {e:#}", row.id); "-vframes",
false "1",
} "-ss",
}; "00:00:01",
if !produced { "-vf",
tracing::info!( "scale=400:-1",
upload_id = %row.id, "-y",
"no poster frame for this video; exporting it without one" thumb_path.to_str().unwrap_or_default(),
); ])
.output()
.await;
match ffmpeg_result {
Ok(output) if output.status.success() => {}
_ => {
tracing::warn!(
"ffmpeg thumbnail failed for upload {}, skipping thumb",
row.id
);
// Missing thumb entry — viewer handles missing thumbs gracefully.
}
} }
// Stream the video full-res straight from the original at ZIP time — no // Stream the video full-res straight from the original at ZIP time — no
// copy to temp (that used to transiently double disk usage per video). // copy to temp (that used to transiently double disk usage per video).
( (thumb, full, MediaSource::Original(src.clone()))
produced.then(|| thumb.clone()),
full,
MediaSource::Original(src.clone()),
)
} else { } else {
let thumb = format!("{id_str}_thumb.jpg"); let thumb = format!("{id_str}_thumb.jpg");
let ext = ext_from_path(&row.original_path); let ext = ext_from_path(&row.original_path);
@@ -836,17 +819,9 @@ async fn run_html_export_inner(
}) })
.await?; .await?;
// Same dangling-reference hazard as the video branch: a failure here left `thumb` if let Err(e) = thumb_result {
// pointing at a file the ZIP writer would then skip, so `data.json` advertised an tracing::warn!("thumbnail generation failed for upload {}: {e:#}", row.id);
// entry the archive didn't contain. An undecodable image is rarer than a sub-second }
// clip, but the broken tile is identical.
let thumb_ok = match thumb_result {
Ok(()) => true,
Err(e) => {
tracing::warn!("thumbnail generation failed for upload {}: {e:#}", row.id);
false
}
};
// Full variant: compress to temp if >5MB, otherwise stream the original // Full variant: compress to temp if >5MB, otherwise stream the original
// as-is (no temp copy). `src_meta` was stat'd once at the top of the loop. // as-is (no temp copy). `src_meta` was stat'd once at the top of the loop.
@@ -884,16 +859,15 @@ async fn run_html_export_inner(
MediaSource::Original(src.clone()) MediaSource::Original(src.clone())
}; };
(thumb_ok.then_some(thumb), full, full_source) (thumb, full, full_source)
}; };
// Register this post's media entries. The thumbnail is registered ONLY when one was // Register this post's two media entries. Thumbnails always come from temp
// actually produced: pushing a manifest entry for a file that doesn't exist made the ZIP // (they're freshly generated); the full variant's source was decided above.
// writer skip it silently while `data.json` still advertised it — the viewer then drew a media_manifest.push((
// broken image tile for an entry the archive never contained. thumb_name.clone(),
if let Some(name) = &thumb_name { MediaSource::Temp(media_tmp.join(&thumb_name)),
media_manifest.push((name.clone(), MediaSource::Temp(media_tmp.join(name)))); ));
}
media_manifest.push((full_name.clone(), full_source)); media_manifest.push((full_name.clone(), full_source));
// Build comments for this upload // Build comments for this upload
@@ -928,15 +902,7 @@ async fn run_html_export_inner(
} else { } else {
"image".to_string() "image".to_string()
}, },
// Empty when there is no poster. The viewer already guards on this thumb: format!("media/{thumb_name}"),
// (`{#if post.media.thumb}` → a video tile with a play glyph, or the placeholder
// icon for an image), so telling it the truth is the entire fix — no schema
// change, no viewer rebuild. What was broken was the backend always claiming a
// thumbnail existed.
thumb: thumb_name
.as_ref()
.map(|n| format!("media/{n}"))
.unwrap_or_default(),
full: format!("media/{full_name}"), full: format!("media/{full_name}"),
}, },
}); });
@@ -984,7 +950,7 @@ async fn run_html_export_inner(
// Write data.json // Write data.json
{ {
let builder = keepsake_entry("data.json".into(), Compression::Deflate); let builder = ZipEntryBuilder::new("data.json".into(), Compression::Deflate);
let mut entry = zip.write_entry_stream(builder).await?; let mut entry = zip.write_entry_stream(builder).await?;
let mut cursor = AllowStdIo::new(std::io::Cursor::new(data_json.as_bytes())); let mut cursor = AllowStdIo::new(std::io::Cursor::new(data_json.as_bytes()));
fcopy(&mut cursor, &mut entry).await?; fcopy(&mut cursor, &mut entry).await?;
@@ -993,7 +959,7 @@ async fn run_html_export_inner(
// Write README.txt // Write README.txt
{ {
let builder = keepsake_entry("README.txt".into(), Compression::Deflate); let builder = ZipEntryBuilder::new("README.txt".into(), Compression::Deflate);
let mut entry = zip.write_entry_stream(builder).await?; let mut entry = zip.write_entry_stream(builder).await?;
let mut cursor = AllowStdIo::new(std::io::Cursor::new(README_TEXT.as_bytes())); let mut cursor = AllowStdIo::new(std::io::Cursor::new(README_TEXT.as_bytes()));
fcopy(&mut cursor, &mut entry).await?; fcopy(&mut cursor, &mut entry).await?;
@@ -1026,7 +992,7 @@ async fn run_html_export_inner(
}; };
let entry_name = format!("media/{name}"); let entry_name = format!("media/{name}");
let builder = keepsake_entry(entry_name, Compression::Stored); let builder = ZipEntryBuilder::new(entry_name.into(), Compression::Stored);
let mut zip_entry = zip.write_entry_stream(builder).await?; let mut zip_entry = zip.write_entry_stream(builder).await?;
let mut f = src_file.compat(); let mut f = src_file.compat();
fcopy(&mut f, &mut zip_entry).await?; fcopy(&mut f, &mut zip_entry).await?;
@@ -1085,7 +1051,7 @@ async fn run_html_export_inner(
// ── DB helpers ─────────────────────────────────────────────────────────────── // ── DB helpers ───────────────────────────────────────────────────────────────
async fn query_uploads(pool: &PgPool, event_id: Uuid) -> Result<Vec<ExportUploadRow>> { async fn query_uploads(pool: &PgPool, event_id: Uuid) -> Result<Vec<ExportUploadRow>> {
Ok(sqlx::query_as::<_, ExportUploadRow>(concat!( Ok(sqlx::query_as::<_, ExportUploadRow>(
"SELECT u.id, u.original_path, u.mime_type, u.caption, "SELECT u.id, u.original_path, u.mime_type, u.caption,
usr.display_name AS uploader_name, usr.display_name AS uploader_name,
COUNT(DISTINCT l.user_id) AS like_count, COUNT(DISTINCT l.user_id) AS like_count,
@@ -1093,12 +1059,11 @@ async fn query_uploads(pool: &PgPool, event_id: Uuid) -> Result<Vec<ExportUpload
FROM upload u FROM upload u
JOIN \"user\" usr ON usr.id = u.user_id JOIN \"user\" usr ON usr.id = u.user_id
LEFT JOIN \"like\" l ON l.upload_id = u.id LEFT JOIN \"like\" l ON l.upload_id = u.id
", WHERE u.event_id = $1 AND u.deleted_at IS NULL
export_visibility_where!(), AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE
"
GROUP BY u.id, usr.display_name GROUP BY u.id, usr.display_name
ORDER BY u.created_at ASC", ORDER BY u.created_at ASC",
)) )
.bind(event_id) .bind(event_id)
.fetch_all(pool) .fetch_all(pool)
.await?) .await?)
@@ -1302,16 +1267,15 @@ fn is_superseded_archive(
/// downscaled to 2000px first, which only makes this estimate more conservative — the direction we /// downscaled to 2000px first, which only makes this estimate more conservative — the direction we
/// want, since being wrong low means ENOSPC halfway through. /// want, since being wrong low means ENOSPC halfway through.
/// ///
/// Shares [`query_uploads`]' visibility filter via [`export_visibility_where`], so hidden/banned /// Matches [`query_uploads`]' visibility filter exactly, so hidden/banned uploads aren't counted.
/// uploads can't be counted here but skipped there (or the reverse, which under-reserves).
pub async fn estimate_export_bytes(pool: &PgPool, event_id: Uuid) -> Result<u64> { pub async fn estimate_export_bytes(pool: &PgPool, event_id: Uuid) -> Result<u64> {
let (bytes,): (i64,) = sqlx::query_as(concat!( let (bytes,): (i64,) = sqlx::query_as(
"SELECT COALESCE(SUM(u.original_size_bytes), 0)::bigint "SELECT COALESCE(SUM(u.original_size_bytes), 0)::bigint
FROM upload u FROM upload u
JOIN \"user\" usr ON usr.id = u.user_id JOIN \"user\" usr ON usr.id = u.user_id
", WHERE u.event_id = $1 AND u.deleted_at IS NULL
export_visibility_where!(), AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE",
)) )
.bind(event_id) .bind(event_id)
.fetch_one(pool) .fetch_one(pool)
.await .await
@@ -1563,36 +1527,6 @@ async fn maybe_broadcast_complete(
/// double-clicking `index.html` (file://), where browsers block a cross-origin /// double-clicking `index.html` (file://), where browsers block a cross-origin
/// `fetch()` of a sibling `data.json` — so the data is inlined into the page. /// `fetch()` of a sibling `data.json` — so the data is inlined into the page.
/// (`data.json` is still written separately for the http-served case.) /// (`data.json` is still written separately for the http-served case.)
/// Permissions stamped on every entry in both archives: `rw-r--r--`.
///
/// `ZipEntryBuilder::new` leaves the external file attribute at zero, and the host compatibility
/// defaults to Unix — so every entry was written with a stored mode of **0000**. Windows Explorer
/// ignores Unix modes and was fine, which is exactly why this survived: on Linux and macOS
/// `unzip` faithfully applies what the archive asks for, and the guest gets a directory of files
/// none of which they can open. `?---------` on every line of `unzip -Z`.
///
/// That is the keepsake — the artifact the whole event exists to produce — arriving unreadable,
/// after distribution, with no server-side symptom at all.
/// `S_IFREG | 0644`. The type bits are included because the mode is written whole into the high
/// half of the external file attribute: without them extractors see a file of type "unknown"
/// (`unzip -Z` renders `?rw-r--r--`), which works but is not what the archive means to say.
const KEEPSAKE_ENTRY_MODE: u16 = 0o100_644;
/// Build a ZIP entry for the keepsake. ALL entries in both archives go through here so the mode
/// can't be forgotten at one of the six call sites.
fn keepsake_entry(name: String, compression: Compression) -> ZipEntryBuilder {
ZipEntryBuilder::new(name.into(), compression).unix_permissions(KEEPSAKE_ENTRY_MODE)
}
/// Escape a JSON payload for inlining inside a `<script>` element.
///
/// See the call site in [`write_viewer_with_data`] for why this is every `<` and not just `</`.
/// Kept separate so the property that matters — no `<` survives, and the value still decodes to
/// the original — can be asserted without building a ZIP.
fn escape_json_for_script(data_json: &str) -> String {
data_json.replace('<', "\\u003c")
}
async fn write_viewer_with_data( async fn write_viewer_with_data(
dir: &include_dir::Dir<'_>, dir: &include_dir::Dir<'_>,
zip: &mut ZipFileWriter<tokio::fs::File>, zip: &mut ZipFileWriter<tokio::fs::File>,
@@ -1604,31 +1538,8 @@ async fn write_viewer_with_data(
if path == "index.html" { if path == "index.html" {
let html = std::str::from_utf8(file.contents()) let html = std::str::from_utf8(file.contents())
.context("export-viewer index.html is not valid UTF-8")?; .context("export-viewer index.html is not valid UTF-8")?;
// Escape EVERY `<`, not just `</`. // Escape `</` so a caption containing `</script>` can't break out of the tag.
// let safe = data_json.replace("</", "<\\/");
// `</` -> `<\/` stops the obvious break-out (`</script><img onerror=…>`) and is inert
// against XSS. It does not stop the caption steering the HTML TOKENIZER. A caption
// containing `<!--<script` with no later `-->` puts the parser into
// script-data-double-escaped state; from there the template's own `</script>` only
// steps back to script-data-escaped instead of closing the element, and the rest of the
// document — including the viewer bundle — is swallowed as script data. Nothing
// executes and nothing leaks; `window.__EXPORT_DATA__` is simply never assigned and the
// keepsake opens blank.
//
// That failure is silent and POST-DISTRIBUTION: the export succeeds, the ZIP is
// well-formed, the job writes `done`, /export/status is green, and the host hands out a
// file that only fails when a guest double-clicks index.html — in every copy, with no
// way to fix it after the fact. Reachable from any guest-authored caption or comment,
// since both are embedded in the viewer.
//
// `<` never appears in JSON structural syntax — only inside string values — so a global
// replace is sound, and `<` is valid in both JSON and a JS string literal. One
// rule covers `</script`, `<!--` and `<script` together, which is the point: the
// previous escape was named for the single case it did handle.
//
// NOTE this is deliberately only for the INLINED copy. `data.json` is written
// separately, in no HTML context, and must stay literal.
let safe = escape_json_for_script(data_json);
// Match the live app's colour theme: inject the same `:root:root{…}` override // Match the live app's colour theme: inject the same `:root:root{…}` override
// the app builds at runtime so a rose/sage/custom event exports a rose/sage // the app builds at runtime so a rose/sage/custom event exports a rose/sage
// keepsake (not the embedded default gold). The CSS is generated purely from // keepsake (not the embedded default gold). The CSS is generated purely from
@@ -1642,13 +1553,13 @@ async fn write_viewer_with_data(
Some(idx) => format!("{}{}{}", &html[..idx], head_inject, &html[idx..]), Some(idx) => format!("{}{}{}", &html[..idx], head_inject, &html[idx..]),
None => format!("{head_inject}{html}"), None => format!("{head_inject}{html}"),
}; };
let builder = keepsake_entry(path, Compression::Deflate); let builder = ZipEntryBuilder::new(path.into(), Compression::Deflate);
let mut entry = zip.write_entry_stream(builder).await?; let mut entry = zip.write_entry_stream(builder).await?;
let mut cursor = AllowStdIo::new(std::io::Cursor::new(injected.as_bytes())); let mut cursor = AllowStdIo::new(std::io::Cursor::new(injected.as_bytes()));
fcopy(&mut cursor, &mut entry).await?; fcopy(&mut cursor, &mut entry).await?;
entry.close().await?; entry.close().await?;
} else { } else {
let builder = keepsake_entry(path, Compression::Deflate); let builder = ZipEntryBuilder::new(path.into(), Compression::Deflate);
let mut entry = zip.write_entry_stream(builder).await?; let mut entry = zip.write_entry_stream(builder).await?;
let mut cursor = AllowStdIo::new(std::io::Cursor::new(file.contents())); let mut cursor = AllowStdIo::new(std::io::Cursor::new(file.contents()));
fcopy(&mut cursor, &mut entry).await?; fcopy(&mut cursor, &mut entry).await?;
@@ -1879,62 +1790,6 @@ mod tests {
} }
} }
/// Every `<` is escaped, whatever it is part of.
///
/// PREVENTS the regression to `</` -> `<\\/`, which is named for the one case it handles.
/// `<!--<script` with no later `-->` drives the HTML tokenizer into
/// script-data-double-escaped state, where the template's own `</script>` no longer closes
/// the element — the viewer bundle is swallowed as script data, `__EXPORT_DATA__` is never
/// assigned, and the keepsake opens blank in every copy the host has already handed out.
#[test]
fn no_left_angle_bracket_survives_inlining() {
for payload in [
r#"{"caption":"<!--<script"}"#,
r#"{"caption":"</script><img src=x onerror=alert(1)>"}"#,
r#"{"caption":"<!--"}"#,
r#"{"caption":"<script>"}"#,
r#"{"caption":"a < b"}"#,
] {
let escaped = escape_json_for_script(payload);
assert!(
!escaped.contains('<'),
"a surviving `<` can still steer the tokenizer: {escaped}"
);
}
}
/// The escape must not change what the viewer READS — it is a transport encoding, not a
/// sanitiser. A caption is guest-authored text that has to render back exactly.
#[test]
fn the_payload_still_decodes_to_the_original_value() {
// `<` appears only inside JSON string values, never in structural syntax, so a global
// replace is sound — this is the assertion that says so.
for caption in [
"<!--<script",
"</script><img src=x onerror=alert(1)>",
"a < b und c > d",
"ganz normale Bildunterschrift",
"Herz <3",
] {
let json = serde_json::json!({ "posts": [{ "caption": caption }] }).to_string();
let escaped = escape_json_for_script(&json);
let back: serde_json::Value =
serde_json::from_str(&escaped).expect("the escaped form must still be valid JSON");
assert_eq!(
back["posts"][0]["caption"].as_str(),
Some(caption),
"the caption must survive the round trip unchanged"
);
}
}
/// Nothing else in the document is touched.
#[test]
fn a_payload_with_no_angle_brackets_is_unchanged() {
let json = r#"{"posts":[{"caption":"schönes Foto"}]}"#;
assert_eq!(escape_json_for_script(json), json);
}
#[test] #[test]
fn a_lone_armed_job_reserves_for_one_archive() { fn a_lone_armed_job_reserves_for_one_archive() {
// A ViewerOnly regeneration re-arms only the HTML half — reserving for two would refuse // A ViewerOnly regeneration re-arms only the HTML half — reserving for two would refuse

34
backend/src/services/imaging.rs
View File

@@ -88,40 +88,6 @@ fn decoder_within_budget(path: &Path) -> Result<impl image::ImageDecoder> {
Ok(decoder) Ok(decoder)
} }
/// Rough peak heap an image will cost to turn into derivatives, read from the HEADER only —
/// no pixels are decoded. `None` when the header can't be read or the image is over budget
/// (the caller is about to fail on it anyway).
///
/// Two terms, and the second is the one that surprises:
///
/// - the decoded buffer, `width * height * channels`; and
/// - the resize intermediate. `image`'s Lanczos3 path accumulates in `f32`, so the buffer
/// between the horizontal and vertical passes is `new_width * old_height * 4 channels * 4
/// bytes` — 16 bytes per pixel-row-slot, not the 4 the output uses. For an 8000x8000
/// original that is 262 MiB on top of a 244 MiB decode, measured. It is bigger than the
/// decode for any tall image, which is why "the decode is bounded by max_alloc" was never
/// the whole story.
///
/// Used to decide whether an image is heavy enough to need exclusive use of the box's memory
/// headroom, NOT to reject anything.
pub fn estimated_processing_peak_bytes(path: &Path, display_edge: u32) -> Option<u64> {
let decoder = decoder_within_budget(path).ok()?;
let (width, height) = decoder.dimensions();
let decoded = decoder.total_bytes();
// Aspect-preserving fit into `display_edge`, matching DynamicImage::resize. No downscale
// means no intermediate at all.
let intermediate = if width > display_edge || height > display_edge {
let ratio = f64::from(display_edge) / f64::from(width.max(height));
let new_width = (f64::from(width) * ratio).round().max(1.0) as u64;
new_width * u64::from(height) * 16
} else {
0
};
Some(decoded.saturating_add(intermediate))
}
/// Megapixels an image would decode to, or `None` if its header can't be read. Used only /// Megapixels an image would decode to, or `None` if its header can't be read. Used only
/// to put a concrete number in the message the guest sees. /// to put a concrete number in the message the guest sees.
pub fn megapixels(path: &Path) -> Option<f64> { pub fn megapixels(path: &Path) -> Option<f64> {

134
backend/src/services/maintenance.rs
View File

@@ -120,19 +120,6 @@ pub async fn startup_recovery(pool: &PgPool) {
} }
} }
/// How long a file in `originals/` may exist without a database row before it is treated as
/// abandoned.
///
/// This window is the ONLY thing making the sweep safe, because the upload handler renames the
/// temp file into its final path BEFORE committing the row: for a short moment a perfectly
/// healthy upload legitimately looks exactly like an orphan. Six hours is far beyond any live
/// request (a 576 MiB body over a bad venue uplink is minutes, and the request itself is bounded
/// by the reverse proxy) while still reclaiming the leak inside a single event.
///
/// DO NOT SHORTEN THIS to make a test faster — a value below the longest possible in-flight
/// upload deletes photos out from under the request that is committing them.
const ORPHAN_UPLOAD_RETENTION_HOURS: u64 = 6;
/// Spawns a background task that periodically: /// Spawns a background task that periodically:
/// - deletes session rows whose `expires_at` is more than a day in the past /// - deletes session rows whose `expires_at` is more than a day in the past
/// - prunes the in-memory rate-limiter HashMap of empty windows /// - prunes the in-memory rate-limiter HashMap of empty windows
@@ -153,7 +140,6 @@ pub fn spawn_periodic_tasks(
tick.tick().await; tick.tick().await;
cleanup_sessions(&pool).await; cleanup_sessions(&pool).await;
cleanup_deleted_media(&pool, &media_path).await; cleanup_deleted_media(&pool, &media_path).await;
sweep_orphan_originals(&pool, &media_path).await;
rate_limiter.prune(); rate_limiter.prune();
sse_tickets.prune(); sse_tickets.prune();
} }
@@ -276,123 +262,3 @@ async fn cleanup_sessions(pool: &PgPool) {
Err(e) => tracing::warn!("session cleanup failed: {e:#}"), Err(e) => tracing::warn!("session cleanup failed: {e:#}"),
} }
} }
/// Reclaim files in `originals/` that no upload row references.
///
/// The backstop behind [`TempFileGuard`](crate::handlers::upload). The guard covers the
/// process that is running; this covers the process that was killed — a SIGKILL, an OOM, or a
/// power cut leaves whatever bytes had been written with no `Drop` to reclaim them, and those
/// files are then permanently invisible: they have no row, so `cleanup_deleted_media` (which is
/// row-driven) can never see them, and they are not counted against any quota while still
/// consuming the free disk that `compute_storage_quota` divides among guests. On a single box
/// where all three volumes share a filesystem, that ends with Postgres unable to write WAL.
///
/// Two classes:
/// - `*.tmp` — an upload that never got as far as being renamed. Always safe past the window.
/// - everything else — a final-named original whose commit never happened.
async fn sweep_orphan_originals(pool: &PgPool, media_path: &std::path::Path) {
let originals = media_path.join("originals");
let cutoff = Duration::from_secs(ORPHAN_UPLOAD_RETENTION_HOURS * 3600);
// originals/{event_slug}/{uuid}.{ext} — one level of per-event directories.
let mut event_dirs = match tokio::fs::read_dir(&originals).await {
Ok(rd) => rd,
// Nothing uploaded yet; the directory is created lazily by the upload handler.
Err(_) => return,
};
let mut candidates: Vec<(String, std::path::PathBuf)> = Vec::new();
let mut temps_removed = 0u32;
while let Ok(Some(event_dir)) = event_dirs.next_entry().await {
if !event_dir
.file_type()
.await
.map(|t| t.is_dir())
.unwrap_or(false)
{
continue;
}
let slug = event_dir.file_name().to_string_lossy().to_string();
let Ok(mut files) = tokio::fs::read_dir(event_dir.path()).await else {
continue;
};
while let Ok(Some(entry)) = files.next_entry().await {
let Ok(meta) = entry.metadata().await else {
continue;
};
if !meta.is_file() {
continue;
}
// Too young to judge: an upload committing RIGHT NOW is indistinguishable from an
// orphan, because the rename precedes the commit.
let recent = meta
.modified()
.ok()
.and_then(|m| m.elapsed().ok())
.is_none_or(|age| age < cutoff);
if recent {
continue;
}
let name = entry.file_name().to_string_lossy().to_string();
if name.ends_with(".tmp") {
// A `.tmp` never has a row by construction — no DB check needed.
if tokio::fs::remove_file(entry.path()).await.is_ok() {
temps_removed += 1;
}
continue;
}
candidates.push((format!("originals/{slug}/{name}"), entry.path()));
}
}
if temps_removed > 0 {
tracing::warn!(
"reclaimed {temps_removed} abandoned upload temp file(s) older than \
{ORPHAN_UPLOAD_RETENTION_HOURS}h"
);
}
if candidates.is_empty() {
return;
}
// One query per batch, not one per file: a backlog of thousands of orphans must not turn
// into thousands of round trips on an hourly timer.
let mut orphans_removed = 0u32;
for chunk in candidates.chunks(500) {
let paths: Vec<String> = chunk.iter().map(|(rel, _)| rel.clone()).collect();
// NO `deleted_at IS NULL` FILTER HERE. A soft-deleted row still points at its file
// during its retention window, and reclaiming that file is `cleanup_deleted_media`'s
// job — filtering here would race the two sweeps and destroy the exact files the
// recovery window exists to preserve.
let unreferenced: Result<Vec<(String,)>, _> = sqlx::query_as(
"SELECT p FROM unnest($1::text[]) AS p
WHERE NOT EXISTS (SELECT 1 FROM upload u WHERE u.original_path = p)",
)
.bind(&paths)
.fetch_all(pool)
.await;
let unreferenced = match unreferenced {
Ok(rows) => rows,
Err(e) => {
tracing::warn!(error = ?e, "orphan-original sweep query failed");
return;
}
};
for (rel,) in unreferenced {
if let Some((_, abs)) = chunk.iter().find(|(r, _)| *r == rel)
&& tokio::fs::remove_file(abs).await.is_ok()
{
orphans_removed += 1;
}
}
}
if orphans_removed > 0 {
tracing::warn!(
"reclaimed {orphans_removed} original(s) with no upload row, older than \
{ORPHAN_UPLOAD_RETENTION_HOURS}h"
);
}
}

1
backend/src/services/mod.rs
View File

@@ -6,4 +6,3 @@ pub mod imaging;
pub mod maintenance; pub mod maintenance;
pub mod rate_limiter; pub mod rate_limiter;
pub mod sse_tickets; pub mod sse_tickets;
pub mod video;

157
backend/src/services/sse_tickets.rs
View File

@@ -13,16 +13,6 @@ use rand::Rng;
/// stream open. Tickets are consumed on use and expire after `TTL`. /// stream open. Tickets are consumed on use and expire after `TTL`.
const TTL: Duration = Duration::from_secs(30); const TTL: Duration = Duration::from_secs(30);
/// Ceiling on outstanding tickets across the whole process.
///
/// Not really about the bytes (~120 each) — about `issue` having had no bound of any kind.
/// Sized well above a real event: ~1000 concurrent clients each holding one live 30 s ticket.
const MAX_TICKETS: usize = 4096;
/// Live tickets one session may hold. Above 1 because two tabs sharing a token open their
/// EventSources concurrently; 4 absorbs that without letting a reconnect loop accumulate.
const MAX_TICKETS_PER_SESSION: usize = 4;
#[derive(Clone)] #[derive(Clone)]
pub struct SseTicketStore { pub struct SseTicketStore {
inner: Arc<Mutex<HashMap<String, Entry>>>, inner: Arc<Mutex<HashMap<String, Entry>>>,
@@ -49,47 +39,9 @@ impl SseTicketStore {
} }
/// Mint a new ticket bound to the caller's session (identified by token hash). /// Mint a new ticket bound to the caller's session (identified by token hash).
/// pub fn issue(&self, token_hash: String) -> String {
/// `None` when the store is at capacity — the caller should answer 503, not evict.
///
/// Three bounds, because `issue` had none: no size cap, no per-caller cap, and no rate
/// limit on the endpoint, while `prune` ran only hourly against a 30-second TTL. So any
/// authenticated session could loop the endpoint and grow the map for an hour.
pub fn issue(&self, token_hash: String) -> Option<String> {
let ticket = random_ticket(); let ticket = random_ticket();
let mut map = self.inner.lock().unwrap(); let mut map = self.inner.lock().unwrap();
// Prune on issue rather than only hourly. This alone changes the bound from "tickets
// minted since the last maintenance tick" to "tickets live at once", which is what the
// 30 s TTL was always meant to express.
map.retain(|_, e| e.issued_at.elapsed() <= TTL);
// Cap the caller's own outstanding tickets, evicting their oldest. NOT one-per-session:
// two tabs sharing a token open their EventSources concurrently, and having tab B
// invalidate tab A's unconsumed ticket looks exactly like a flaky SSE connection.
let mut mine: Vec<(String, Instant)> = map
.iter()
.filter(|(_, e)| e.token_hash == token_hash)
.map(|(k, e)| (k.clone(), e.issued_at))
.collect();
if mine.len() >= MAX_TICKETS_PER_SESSION {
mine.sort_by_key(|(_, issued)| *issued);
for (key, _) in mine.iter().take(mine.len() - MAX_TICKETS_PER_SESSION + 1) {
map.remove(key);
}
}
// At capacity, REFUSE — never evict a stranger's ticket. Evicting would let one
// misbehaving client deny SSE to the whole venue, which is worse than failing the
// request that hit the ceiling.
if map.len() >= MAX_TICKETS {
tracing::warn!(
outstanding = map.len(),
"SSE ticket store at capacity; refusing to mint"
);
return None;
}
map.insert( map.insert(
ticket.clone(), ticket.clone(),
Entry { Entry {
@@ -97,7 +49,7 @@ impl SseTicketStore {
issued_at: Instant::now(), issued_at: Instant::now(),
}, },
); );
Some(ticket) ticket
} }
/// Consume a ticket. Returns `Some(token_hash)` if the ticket exists and is /// Consume a ticket. Returns `Some(token_hash)` if the ticket exists and is
@@ -132,16 +84,10 @@ fn random_ticket() -> String {
mod tests { mod tests {
use super::*; use super::*;
/// `issue` now returns `Option`; in every test below the store is far from capacity, so an
/// `expect` here documents that refusing is exceptional rather than routine.
fn issue(store: &SseTicketStore, hash: &str) -> String {
store.issue(hash.into()).expect("store has capacity")
}
#[test] #[test]
fn issue_then_consume_returns_the_hash_exactly_once() { fn issue_then_consume_returns_the_hash_exactly_once() {
let store = SseTicketStore::new(); let store = SseTicketStore::new();
let ticket = issue(&store, "hash-1"); let ticket = store.issue("hash-1".into());
assert_eq!(store.consume(&ticket).as_deref(), Some("hash-1")); assert_eq!(store.consume(&ticket).as_deref(), Some("hash-1"));
// Single-use: a replay of the same ticket is rejected. // Single-use: a replay of the same ticket is rejected.
assert_eq!( assert_eq!(
@@ -160,8 +106,8 @@ mod tests {
#[test] #[test]
fn issued_tickets_are_unique_and_hex() { fn issued_tickets_are_unique_and_hex() {
let store = SseTicketStore::new(); let store = SseTicketStore::new();
let a = issue(&store, "h"); let a = store.issue("h".into());
let b = issue(&store, "h"); let b = store.issue("h".into());
assert_ne!(a, b, "each ticket must be unique"); assert_ne!(a, b, "each ticket must be unique");
assert_eq!(a.len(), 48, "24 random bytes → 48 hex chars"); assert_eq!(a.len(), 48, "24 random bytes → 48 hex chars");
assert!(a.chars().all(|c| c.is_ascii_hexdigit())); assert!(a.chars().all(|c| c.is_ascii_hexdigit()));
@@ -170,104 +116,29 @@ mod tests {
#[test] #[test]
fn fresh_ticket_survives_prune() { fn fresh_ticket_survives_prune() {
let store = SseTicketStore::new(); let store = SseTicketStore::new();
let ticket = issue(&store, "h"); let ticket = store.issue("h".into());
store.prune(); // not expired → kept store.prune(); // not expired → kept
assert_eq!(store.consume(&ticket).as_deref(), Some("h")); assert_eq!(store.consume(&ticket).as_deref(), Some("h"));
} }
/// Build an entry that is already past the TTL. #[test]
fn insert_stale(store: &SseTicketStore, key: &str, token_hash: &str) { fn expired_ticket_consumes_to_none() {
// Construct an entry that is already past the TTL and confirm consume() rejects it.
let store = SseTicketStore::new();
let stale = "stale-ticket".to_string();
store.inner.lock().unwrap().insert( store.inner.lock().unwrap().insert(
key.to_string(), stale.clone(),
Entry { Entry {
token_hash: token_hash.into(), token_hash: "h".into(),
issued_at: Instant::now() issued_at: Instant::now()
.checked_sub(TTL + Duration::from_secs(1)) .checked_sub(TTL + Duration::from_secs(1))
.expect("host uptime should exceed the ticket TTL"), .expect("host uptime should exceed the ticket TTL"),
}, },
); );
}
#[test]
fn expired_ticket_consumes_to_none() {
let store = SseTicketStore::new();
insert_stale(&store, "stale-ticket", "h");
assert_eq!( assert_eq!(
store.consume("stale-ticket"), store.consume(&stale),
None, None,
"an expired ticket must not authenticate" "an expired ticket must not authenticate"
); );
} }
/// The TTL is 30 s but `prune` only ran hourly, so the map was really bounded by "tickets
/// minted in the last hour" — which is unbounded for a client in a loop.
#[test]
fn issuing_prunes_expired_entries() {
let store = SseTicketStore::new();
insert_stale(&store, "stale-a", "someone-else");
insert_stale(&store, "stale-b", "someone-else");
issue(&store, "h");
assert_eq!(
store.inner.lock().unwrap().len(),
1,
"issue must reclaim expired slots, not merely add to them"
);
}
/// Two tabs sharing a token is normal, so the per-session cap must be above 1 — but a
/// reconnect loop must not accumulate. The caller's OWN oldest is what gets evicted.
#[test]
fn a_session_is_capped_and_evicts_only_its_own_oldest() {
let store = SseTicketStore::new();
let stranger = issue(&store, "other-session");
let mut mine: Vec<String> = Vec::new();
for _ in 0..MAX_TICKETS_PER_SESSION + 2 {
mine.push(issue(&store, "mine"));
}
let live = mine
.iter()
.filter(|t| store.inner.lock().unwrap().contains_key(*t))
.count();
assert_eq!(live, MAX_TICKETS_PER_SESSION, "one session, bounded");
assert!(
store.inner.lock().unwrap().contains_key(&mine[mine.len() - 1]),
"the newest ticket is the one the caller is about to use"
);
assert_eq!(
store.consume(&stranger).as_deref(),
Some("other-session"),
"another session's ticket must survive — evicting it would let one client deny \
SSE to the venue"
);
}
/// At capacity the store REFUSES rather than evicting a stranger. Refusing fails the one
/// request that hit the ceiling; evicting would break an unrelated client's live stream.
#[test]
fn at_capacity_the_store_refuses_instead_of_evicting() {
let store = SseTicketStore::new();
{
let mut map = store.inner.lock().unwrap();
for i in 0..MAX_TICKETS {
map.insert(
format!("filler-{i}"),
Entry {
token_hash: format!("session-{i}"),
issued_at: Instant::now(),
},
);
}
}
assert_eq!(
store.issue("newcomer".into()),
None,
"a full store must refuse, so the caller can answer 503"
);
assert!(
store.inner.lock().unwrap().contains_key("filler-0"),
"no existing ticket may be sacrificed to make room"
);
}
} }

233
backend/src/services/video.rs
View File

@@ -1,233 +0,0 @@
//! Poster-frame extraction, shared by the compression worker and the HTML export.
//!
//! Both used to spawn `ffmpeg` themselves with the same broken invocation:
//!
//! ```text
//! 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, so neither noticed. The worker
//! then wrote `thumbnail_path` for a file that was never created (404 in the live feed) and the
//! export listed the entry in `data.json` while the ZIP writer skipped it (a broken image tile in
//! the keepsake). Every server-side signal stayed green. Phones produce such clips constantly:
//! mis-taps, Live Photos, boomerangs.
//!
//! This module exists for the same reason `imaging.rs` does — that one was created when compression
//! and export duplicated decode logic, and it paid off immediately when the `max_alloc` fix landed
//! in both workers at once. Same duplication, same fix.
use std::path::Path;
use std::time::Duration;
use anyhow::{Context, Result};
/// A malformed video can hang `ffmpeg` indefinitely. In the compression worker that never releases
/// the semaphore permit and the pool eventually deadlocks; in the export worker it strands the job
/// at `running` so the keepsake never completes. `export.rs` had NO timeout at all before this
/// module — sharing the spawn fixes that too.
const FFMPEG_TIMEOUT: Duration = Duration::from_secs(120);
/// Seek positions to try, in order.
///
/// One second first: the opening frame of a real video is often black, a fade-in, or motion-blurred
/// as the camera settles, so it makes a poor poster. Zero second as the fallback, which is what
/// makes short clips work — and it is genuinely required, not defensive. Moving `-ss` before `-i`
/// (an input-side seek) is necessary but NOT sufficient: seeking to 1 s in a 1.000 s clip is still
/// past the last frame, and ffmpeg still exits 0 having written nothing. Verified against the real
/// production image.
const SEEK_POSITIONS: &[&str] = &["00:00:01", "0"];
/// Extract one poster frame from `src` into `dest`, scaled to `width` px wide.
///
/// `Ok(false)` means the video yielded no frame — a normal outcome for a very short or unusual
/// clip, NOT an error. Callers must degrade (no poster) rather than fail the upload: treating this
/// as an error would soft-delete every sub-second video, turning a cosmetic defect into data loss.
///
/// `Err` is reserved for something genuinely wrong — a hang we had to kill, or a failure to spawn.
pub async fn extract_poster_frame(src: &Path, dest: &Path, width: u32) -> Result<bool> {
for seek in SEEK_POSITIONS {
// A stale file from a previous attempt would be indistinguishable from a fresh success.
let _ = tokio::fs::remove_file(dest).await;
run_ffmpeg(src, dest, width, seek).await?;
// THE CHECK BOTH CALL SITES WERE MISSING: ask the filesystem, not the exit status.
// Non-empty, because a zero-byte file is not a poster either.
if tokio::fs::metadata(dest)
.await
.map(|m| m.is_file() && m.len() > 0)
.unwrap_or(false)
{
return Ok(true);
}
}
// Leave nothing behind for a caller to mistake for a result.
let _ = tokio::fs::remove_file(dest).await;
Ok(false)
}
/// Run one ffmpeg attempt. A non-zero exit is NOT an error here — the artifact check above is the
/// authority, and a corrupt input that fails at 1 s may still yield a frame at 0.
async fn run_ffmpeg(src: &Path, dest: &Path, width: u32, seek: &str) -> Result<()> {
let child = tokio::process::Command::new("ffmpeg")
.args([
// BEFORE -i: an input-side seek. See SEEK_POSITIONS.
"-ss",
seek,
"-i",
src.to_str().unwrap_or_default(),
"-vframes",
"1",
"-vf",
&format!("scale={width}:-1"),
"-y",
dest.to_str().unwrap_or_default(),
])
// ffmpeg writes the poster to `dest` itself; nothing here ever reads stdout, so
// giving it a pipe only created something that could fill.
.stdout(std::process::Stdio::null())
// stderr IS piped — it is the only diagnostic when a clip yields no frame — but it
// must be DRAINED, which is the whole point of `wait_with_output` below.
.stderr(std::process::Stdio::piped())
.kill_on_drop(true)
.spawn()
.context("failed to spawn ffmpeg")?;
// `wait_with_output`, NOT `wait`. ffmpeg is verbose on stderr (banner, stream info,
// per-frame progress) and `wait()` reads neither pipe — so once the ~64 KiB pipe buffer
// filled, ffmpeg blocked writing, `wait()` never returned, and the call burned the full
// timeout. That is not merely slow: the timeout is an `Err`, so after 2 seek positions x
// 3 compression attempts the caller soft-deletes a perfectly playable video for a
// poster-frame failure. `wait_with_output` polls the pipe and the exit status together.
//
// It also CONSUMES the child, so the explicit `child.kill()` that used to sit on the
// timeout arm cannot exist here — and is not needed: `kill_on_drop(true)` is set above,
// and dropping the future on timeout drops the child with it.
let out = match tokio::time::timeout(FFMPEG_TIMEOUT, child.wait_with_output()).await {
Ok(res) => res.context("ffmpeg wait failed")?,
Err(_) => anyhow::bail!("ffmpeg timed out after {}s", FFMPEG_TIMEOUT.as_secs()),
};
// A non-zero exit is not an error (see the doc comment) — the artifact check in
// `extract_poster_frame` is the authority. Log the tail so a systematically failing
// format is diagnosable without turning it into data loss.
if !out.status.success() {
tracing::debug!(
seek,
status = ?out.status,
stderr = %tail_lines(&out.stderr, 10),
"ffmpeg exited non-zero; the artifact check decides"
);
}
Ok(())
}
/// Last `n` lines of a child's stderr, lossily decoded.
///
/// Bounded on purpose: ffmpeg's stderr is unbounded, and the reason we now drain it is that
/// unbounded output used to be a hazard. Emitting all of it into a log line — into container
/// logs that are themselves size-capped — would just move the problem.
fn tail_lines(bytes: &[u8], n: usize) -> String {
let text = String::from_utf8_lossy(bytes);
let lines: Vec<&str> = text.lines().filter(|l| !l.trim().is_empty()).collect();
lines[lines.len().saturating_sub(n)..].join(" | ")
}
#[cfg(test)]
mod tests {
use super::*;
/// The order is the whole fix. `-ss` must precede `-i`, and 0 must be tried after 1 s.
#[test]
fn the_fallback_seek_exists_and_comes_last() {
assert_eq!(
SEEK_POSITIONS,
&["00:00:01", "0"],
"1s first for a better poster, 0 as the fallback that makes short clips work"
);
}
/// A missing input yields no frame rather than an error: the caller must degrade to "no
/// poster", never fail the upload. `Err` is reserved for a hang or a spawn failure.
#[tokio::test]
async fn a_missing_source_yields_no_frame_rather_than_an_error() {
let dir = std::env::temp_dir().join(format!("es-video-{}", std::process::id()));
std::fs::create_dir_all(&dir).unwrap();
let dest = dir.join("out.jpg");
let got = extract_poster_frame(Path::new("/nonexistent/clip.mp4"), &dest, 400).await;
match got {
Ok(false) => {}
other => panic!("expected Ok(false) for a missing input, got {other:?}"),
}
assert!(
!dest.exists(),
"a failed extraction must leave nothing a caller could mistake for a poster"
);
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn the_stderr_tail_is_bounded_and_survives_invalid_utf8() {
let noisy: Vec<u8> = (0..500)
.map(|i| format!("line {i}\n"))
.collect::<String>()
.into_bytes();
let got = tail_lines(&noisy, 3);
assert_eq!(got, "line 497 | line 498 | line 499");
// ffmpeg emits filenames verbatim, so its stderr is not guaranteed to be UTF-8.
assert_eq!(tail_lines(&[b'o', b'k', 0xff], 5), "ok\u{fffd}");
assert_eq!(tail_lines(b"", 5), "");
}
/// A real extraction must finish in a small fraction of `FFMPEG_TIMEOUT`.
///
/// Wall-clock is the ONLY observable of the bug this guards: piping stderr and then
/// calling `wait()` (which drains nothing) blocks ffmpeg on a full pipe buffer until the
/// timeout fires, and the timeout is an `Err`, so the upload is soft-deleted. The
/// assertion is deliberately on elapsed time, not on the exit status.
///
/// Honest limitation: our fixture is quiet enough not to fill a 64 KiB pipe on its own,
/// so this catches a regression to `wait()` only in combination with a verbose input. It
/// is still worth pinning — a reverted drain plus any chatty clip is data loss.
#[tokio::test]
async fn a_real_clip_yields_a_poster_well_inside_the_timeout() {
if tokio::process::Command::new("ffmpeg")
.arg("-version")
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
.status()
.await
.is_err()
{
eprintln!("skipping: ffmpeg not on PATH");
return;
}
let src = Path::new("../e2e/fixtures/media/sample.mp4");
if !src.exists() {
eprintln!("skipping: {} missing", src.display());
return;
}
let dir = std::env::temp_dir().join(format!("es-video-ok-{}", std::process::id()));
std::fs::create_dir_all(&dir).unwrap();
let dest = dir.join("poster.jpg");
let started = std::time::Instant::now();
let got = extract_poster_frame(src, &dest, 400).await;
let elapsed = started.elapsed();
assert!(matches!(got, Ok(true)), "expected a poster, got {got:?}");
assert!(dest.metadata().unwrap().len() > 0);
assert!(
elapsed < FFMPEG_TIMEOUT / 4,
"extraction took {elapsed:?}; a drained stderr finishes in well under \
{FFMPEG_TIMEOUT:?} — this is the pipe-deadlock regression guard"
);
let _ = std::fs::remove_dir_all(&dir);
}
}

8
backend/tests/common/mod.rs
View File

@@ -293,10 +293,6 @@ pub async fn set_user_moderation(pool: &PgPool, user_id: Uuid, banned: bool, hid
/// SRC: `services/export.rs::query_uploads` — the visibility filter, verbatim, projected down to /// SRC: `services/export.rs::query_uploads` — the visibility filter, verbatim, projected down to
/// `(id, original_size_bytes)`. This is the row set that ACTUALLY lands in the archives. /// `(id, original_size_bytes)`. This is the row set that ACTUALLY lands in the archives.
///
/// Production builds this WHERE from `export_visibility_where!()`, shared with
/// `estimate_export_bytes`. A copy here can pin the behaviour but CANNOT detect production moving
/// away from it — that is what sharing the fragment is for, not this.
pub async fn export_visible_uploads(pool: &PgPool, event_id: Uuid) -> Vec<(Uuid, i64)> { pub async fn export_visible_uploads(pool: &PgPool, event_id: Uuid) -> Vec<(Uuid, i64)> {
sqlx::query_as( sqlx::query_as(
"SELECT u.id, u.original_size_bytes "SELECT u.id, u.original_size_bytes
@@ -313,9 +309,7 @@ pub async fn export_visible_uploads(pool: &PgPool, event_id: Uuid) -> Vec<(Uuid,
.expect("export_visible_uploads") .expect("export_visible_uploads")
} }
/// SRC: `services/export.rs::estimate_export_bytes` — verbatim. Same caveat as above: production /// SRC: `services/export.rs::estimate_export_bytes` — verbatim.
/// shares its WHERE with `query_uploads` via `export_visibility_where!()`, so these two copies
/// agreeing proves the behaviour, not the absence of drift.
pub async fn estimate_export_bytes(pool: &PgPool, event_id: Uuid) -> i64 { pub async fn estimate_export_bytes(pool: &PgPool, event_id: Uuid) -> i64 {
let (bytes,): (i64,) = sqlx::query_as( let (bytes,): (i64,) = sqlx::query_as(
"SELECT COALESCE(SUM(u.original_size_bytes), 0)::bigint "SELECT COALESCE(SUM(u.original_size_bytes), 0)::bigint

23
backend/tests/export_preflight.rs
View File

@@ -16,18 +16,10 @@
//! //!
//! What these tests pin is the ESTIMATE — the part that decides. The arithmetic on top of it lives //! What these tests pin is the ESTIMATE — the part that decides. The arithmetic on top of it lives
//! in `services/export.rs`'s unit tests; the filesystem selection lives in `is_superseded_archive`. //! in `services/export.rs`'s unit tests; the filesystem selection lives in `is_superseded_archive`.
//! //! The risk here is drift: if `query_uploads` ever gains or loses a visibility predicate and
//! ON DRIFT, precisely, because it is easy to overclaim here. The hazard is that `query_uploads` //! `estimate_export_bytes` doesn't, the preflight silently sizes the wrong gallery. So rather than
//! (which selects the rows the archives are built from) and `estimate_export_bytes` (which sizes //! restating the filter, these assert the estimate against the row set the archive actually
//! them) could disagree — and an estimate missing rows the archive writes UNDER-reserves, the one //! contains.
//! direction that reintroduces the ENOSPC. **These tests cannot catch that**, and neither can any
//! test in this harness: both sides here are `SRC:`-marked hand-copies in `tests/common/mod.rs`,
//! so if production moved and the copies didn't, they would sit still and keep passing.
//!
//! That is fixed where it can be — the two queries now share one `export_visibility_where!()`
//! fragment in `services/export.rs`, so they cannot diverge by construction. What is left for
//! these tests is what the convention is genuinely good at: pinning the BEHAVIOUR, so a change
//! that deliberately alters the filter has to come here and say so.
mod common; mod common;
@@ -37,10 +29,9 @@ use sqlx::PgPool;
/// The estimate must equal the sum over EXACTLY the rows `query_uploads` returns — computed from /// The estimate must equal the sum over EXACTLY the rows `query_uploads` returns — computed from
/// that row set, not from a restatement of its WHERE clause. /// that row set, not from a restatement of its WHERE clause.
/// ///
/// PINS: which uploads the preflight is allowed to count. Each excluded row below is excluded by a /// PREVENTS: the two queries drifting apart. An estimate that counts rows the archive skips is
/// DIFFERENT predicate, so a change that drops or weakens any one of them fails here and has to be /// merely pessimistic; one that MISSES rows the archive writes under-reserves, which is the whole
/// argued for. (It does not detect production drifting away from these copies — see the file /// failure being guarded against.
/// header; `export_visibility_where!()` is what makes that impossible.)
#[sqlx::test] #[sqlx::test]
async fn the_estimate_sums_exactly_the_rows_the_archive_will_contain(pool: PgPool) { async fn the_estimate_sums_exactly_the_rows_the_archive_will_contain(pool: PgPool) {
let event_id = seed_event(&pool, "wedding").await; let event_id = seed_event(&pool, "wedding").await;

20
docker-compose.yml
View File

@@ -1,20 +1,7 @@
# Docker's default json-file driver is UNBOUNDED. Those files land on the HOST
# filesystem, outside every `deploy.resources.limits` below — so the container memory
# caps do nothing to stop them. On a single-box deployment the host disk is also where
# the postgres_data and media_data volumes live, and a full disk stops Postgres writing
# WAL, which takes the whole event down. 4 services x 3 x 10m caps the worst case at
# ~120 MiB. Applied to every service via the anchor; a new service must opt in too.
x-logging: &default-logging
driver: json-file
options:
max-size: "10m"
max-file: "3"
services: services:
db: db:
image: postgres:16-alpine image: postgres:16-alpine
restart: unless-stopped restart: unless-stopped
logging: *default-logging
env_file: .env env_file: .env
environment: environment:
POSTGRES_USER: ${POSTGRES_USER} POSTGRES_USER: ${POSTGRES_USER}
@@ -45,13 +32,8 @@ services:
context: ./backend context: ./backend
dockerfile: Dockerfile dockerfile: Dockerfile
restart: unless-stopped restart: unless-stopped
logging: *default-logging
env_file: .env env_file: .env
environment: environment:
# Default to info. The code fallback in main.rs is info too, but a stock deploy
# sets RUST_LOG nowhere, and this is the layer an operator will actually find when
# they need to raise it for a single event ("RUST_LOG=eventsnap_backend=debug").
RUST_LOG: ${RUST_LOG:-info}
# Activates the production secret guard in config.rs — refuses to boot with # Activates the production secret guard in config.rs — refuses to boot with
# placeholder JWT_SECRET / ADMIN_PASSWORD_HASH. # placeholder JWT_SECRET / ADMIN_PASSWORD_HASH.
APP_ENV: production APP_ENV: production
@@ -93,7 +75,6 @@ services:
context: ./frontend context: ./frontend
dockerfile: Dockerfile dockerfile: Dockerfile
restart: unless-stopped restart: unless-stopped
logging: *default-logging
env_file: .env env_file: .env
environment: environment:
# adapter-node behind Caddy TLS needs the public origin for CSRF checks on # adapter-node behind Caddy TLS needs the public origin for CSRF checks on
@@ -119,7 +100,6 @@ services:
caddy: caddy:
image: caddy:2-alpine image: caddy:2-alpine
restart: unless-stopped restart: unless-stopped
logging: *default-logging
environment: environment:
# The Caddyfile's site address is `{$DOMAIN}`, read from THIS container's env. # The Caddyfile's site address is `{$DOMAIN}`, read from THIS container's env.
# Without it, `{$DOMAIN}` expands to empty, the site block collapses, and Caddy # Without it, `{$DOMAIN}` expands to empty, the site block collapses, and Caddy

46
e2e/fixtures/db.ts
View File

@@ -37,52 +37,6 @@ export const db = {
); );
}, },
/**
* Is this user's account currently PIN-locked?
*
* Distinguishes the two ways /recover can answer 429 — the per-(IP, name) throttle, which
* costs the attacker, and the account lock, which costs the VICTIM. Only the second one is
* weaponizable, so a test asserting "a single IP cannot lock a guest out" has to look at the
* row, not at the status code.
*/
async isPinLocked(userId: string): Promise<boolean> {
return withClient(async (c) => {
const r = await c.query<{ locked: boolean }>(
`SELECT (pin_locked_until IS NOT NULL AND pin_locked_until > NOW()) AS locked
FROM "user" WHERE id = $1`,
[userId]
);
return r.rows[0]?.locked ?? false;
});
},
/**
* Preload the wrong-PIN streak, standing in for failures that arrived from other IPs.
*
* The account lock is deliberately out of reach of any single source, so a test that wants to
* exercise it has to simulate the distributed case rather than hammer from one address.
* `last_failed_pin_at` is set to now so the 15-minute decay does not immediately reset it.
*/
async setFailedPinAttempts(userId: string, attempts: number) {
await withClient((c) =>
c.query(
`UPDATE "user" SET failed_pin_attempts = $2, last_failed_pin_at = NOW() WHERE id = $1`,
[userId, attempts]
)
);
},
/** Current wrong-PIN streak. Decays after 15 minutes — see User::increment_failed_pin. */
async failedPinAttempts(userId: string): Promise<number> {
return withClient(async (c) => {
const r = await c.query<{ failed_pin_attempts: number }>(
`SELECT failed_pin_attempts FROM "user" WHERE id = $1`,
[userId]
);
return r.rows[0]?.failed_pin_attempts ?? 0;
});
},
async expireSession(userId: string) { async expireSession(userId: string) {
await withClient((c) => await withClient((c) =>
c.query(`UPDATE session SET expires_at = NOW() - interval '1 hour' WHERE user_id = $1`, [ c.query(`UPDATE session SET expires_at = NOW() - interval '1 hour' WHERE user_id = $1`, [

BIN
e2e/fixtures/media/huge-99mp.jpg
View File

Binary file not shown.
Side by Side

Before

Width:  |  Height:  |  Size: 568 KiB

4
e2e/fixtures/media/not-an-image.jpg
View File

@@ -1,4 +0,0 @@
This is plain text, not an image at all.
This is plain text, not an image at all.
This is plain text, not an image at all.
This is plain text, not an image at all.

BIN
e2e/fixtures/media/portrait-exif6.jpg
View File

Binary file not shown.
Side by Side

Before

Width:  |  Height:  |  Size: 807 B

BIN
e2e/fixtures/media/sample-5s.mp4
View File

Binary file not shown.

BIN
e2e/fixtures/media/sample.jpg
View File

Binary file not shown.
Side by Side

Before

Width:  |  Height:  |  Size: 27 KiB

BIN
e2e/fixtures/media/sample.mp4
View File

Binary file not shown.

BIN
e2e/fixtures/media/sample2.jpg
View File

Binary file not shown.
Side by Side

Before

Width:  |  Height:  |  Size: 35 KiB

39
e2e/specs/01-auth/join.spec.ts
View File

@@ -75,11 +75,7 @@ test.describe('Auth — join flow', () => {
expect(storage.pin).toBe(original.pin); expect(storage.pin).toBe(original.pin);
}); });
test('repeated wrong PINs are throttled without locking the guest out', async ({ test('wrong PIN three times locks the account for 15 minutes', async ({ page, guest, db }) => {
page,
guest,
db,
}) => {
const dave = await guest('Dave'); const dave = await guest('Dave');
await clearAllStorage(page); await clearAllStorage(page);
@@ -89,33 +85,22 @@ test.describe('Auth — join flow', () => {
await join.submit(); await join.submit();
await expect(join.recoveryPinInput).toBeVisible(); await expect(join.recoveryPinInput).toBeVisible();
// Wrong PIN (real one is dave.pin), four times — one more than the OLD lock threshold of 3. // Wrong PIN (real one is dave.pin)
// Typed digit by digit so the 4th character auto-submits (see pin-auto-submit.spec.ts);
// clicking as well would double-submit and race the disabled state of the button.
const wrong = dave.pin === '0000' ? '1111' : '0000'; const wrong = dave.pin === '0000' ? '1111' : '0000';
for (let i = 0; i < 4; i++) { for (let i = 0; i < 3; i++) {
await join.recoveryPinInput.fill(''); await join.recoveryPinInput.fill(wrong);
await join.recoveryPinInput.pressSequentially(wrong, { delay: 30 }); await join.recoverySubmit.click();
await expect(join.recoveryError).toBeVisible(); await expect(join.recoveryError).toBeVisible();
await expect(join.recoverySubmit).toBeEnabled();
} }
// THE PROPERTY THIS TEST EXISTS FOR, stated the way a guest experiences it: Dave can still // Fourth attempt should hit the 429 lockout (even with the correct PIN now)
// get into his own account. await join.recoveryPinInput.fill(dave.pin);
// await join.recoverySubmit.click();
// The lock threshold used to be 3, BELOW the per-(IP, name) ceiling — so these very await expect(join.recoveryError).toContainText(/15 Minuten/);
// keystrokes locked Dave out for 15 minutes, and anyone who can read his name off the feed
// could do it to him on repeat. Rate limits are disabled in this environment (see
// config `rate_limits_enabled`), so what is exercised here is purely the account-lock tier;
// the throttle tier is covered in 07-adversarial/auth-tampering.spec.ts.
expect(
await db.isPinLocked(dave.userId),
'four wrong PINs from one device must not lock a guest out of their own account'
).toBe(false);
await join.recoveryPinInput.fill(''); // Sanity: DB row reflects the lock
await join.recoveryPinInput.pressSequentially(dave.pin, { delay: 30 }); // (The handler sets pin_locked_until directly — verify via API "recover" returning 429)
await page.waitForURL('**/feed'); void db; // unused for now, documenting that db.lockUserPin exists if we want shortcut path
}); });
test('"Anderen Namen wählen" returns to the normal join form', async ({ page, guest }) => { test('"Anderen Namen wählen" returns to the normal join form', async ({ page, guest }) => {

9
e2e/specs/02-upload/burst-queue.spec.ts
View File

@@ -199,12 +199,9 @@ test.describe('Upload — client queue under a burst', () => {
// a closed tab / killed PWA. The remaining pending items live only in // a closed tab / killed PWA. The remaining pending items live only in
// IndexedDB now. // IndexedDB now.
await page.reload(); await page.reload();
// Deliberately NOT navigating to /upload. Rehydration is now module-level and // The queue only resumes where loadQueue() runs — the /upload route's
// auth-gated (upload-queue.ts `hydrateQueue`), so the queue resumes wherever the // onMount. Navigating there is the "reopen the composer" recovery path.
// reload lands. This assertion is the regression guard for the defect it replaced: await page.goto('/upload');
// `loadQueue()` used to have a single call site in the whole app — the /upload
// route's onMount — so a guest who reloaded anywhere else saw a 0 badge and their
// staged photos never left the phone, having already been shown a success.
// (4) RESUME: every file ends up server-side without re-staging anything. // (4) RESUME: every file ends up server-side without re-staging anything.
// `>=` not `===`: the only imperfection possible is a DUPLICATE (an upload // `>=` not `===`: the only imperfection possible is a DUPLICATE (an upload

20
e2e/specs/03-feed/video-playback.spec.ts
View File

@@ -40,19 +40,10 @@ test.describe('Video — the lightbox plays it', () => {
page, page,
guest, guest,
signIn, signIn,
db,
}) => { }) => {
const g = await guest('VideoWatcher'); const g = await guest('VideoWatcher');
const id = await seedVideo(g.jwt); const id = await seedVideo(g.jwt);
// The poster assertion below needs the ffmpeg thumbnail to EXIST — the lightbox binds
// `poster={upload.thumbnail_url ?? undefined}`, so the attribute is simply absent until
// compression finishes. Without this wait the test races the worker and fails against a
// cold stack (first run after `stack:down -v`, cold ffmpeg), which is exactly when a suite
// is least likely to be believed. The `src` assertion is unconditional; only the poster
// needs the wait.
await expect.poll(() => db.compressionStatus(id), { timeout: 60_000 }).toBe('done');
await signIn(page, g); await signIn(page, g);
await page.goto('/feed'); await page.goto('/feed');
@@ -69,17 +60,6 @@ test.describe('Video — the lightbox plays it', () => {
// The poster SHOULD still be the thumbnail — that's what it's for. // The poster SHOULD still be the thumbnail — that's what it's for.
await expect(video).toHaveAttribute('poster', `/api/v1/upload/${id}/thumbnail`); await expect(video).toHaveAttribute('poster', `/api/v1/upload/${id}/thumbnail`);
// …and it must actually RESOLVE. Asserting only the attribute is what let a phantom thumbnail
// survive nine rounds of green: `thumbnail_path` was written for a file ffmpeg never created,
// so this URL 404'd for every clip of a second or less while the attribute looked perfect.
// One extra fetch is the whole difference.
const poster = await fetch(`${BASE}/api/v1/upload/${id}/thumbnail`, {
headers: { Authorization: `Bearer ${g.jwt}` },
});
expect(poster.status, 'the poster URL must serve real bytes, not just exist').toBe(200);
expect(poster.headers.get('content-type')).toContain('image/');
expect((await poster.arrayBuffer()).byteLength).toBeGreaterThan(0);
// And the browser must accept the bytes as media. preload="none" means nothing is // And the browser must accept the bytes as media. preload="none" means nothing is
// fetched until we ask, so drive a load explicitly and wait for metadata. // fetched until we ask, so drive a load explicitly and wait for metadata.
const readyState = await video.evaluate(async (el: HTMLVideoElement) => { const readyState = await video.evaluate(async (el: HTMLVideoElement) => {

84
e2e/specs/03-feed/video-poster.spec.ts
View File

@@ -1,84 +0,0 @@
/**
* Regression guard — a short video gets a real poster frame, and the keepsake never shows a broken
* tile.
*
* 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. So:
*
* - the worker wrote `thumbnail_path` and logged "thumbnail generated" for a file that was never
* created → `GET /upload/{id}/thumbnail` 404s in the live feed;
* - the export listed `media/…_thumb.jpg` in `data.json` while the ZIP writer skipped the
* unopenable file → the keepsake rendered a broken image tile.
*
* Any clip at or under a second, which phones produce constantly: mis-taps, Live Photos, boomerangs.
* Every server-side signal stayed green throughout.
*
* Two fixtures on purpose, because they take different paths through the fix:
* - `sample.mp4` is exactly 1.000 s. An input-side seek to 1 s is STILL past its last frame, so it
* is the 0 s fallback that saves it. Moving `-ss` before `-i` alone does not fix this file.
* - `sample-5s.mp4` is 5 s and succeeds on the first seek — the normal path, which no test covered
* at all before, because the suite only ever had the boundary fixture.
*/
import { test, expect } from '../../fixtures/test';
import { uploadRaw } from '../../helpers/upload-client';
import { readFileSync } from 'node:fs';
import { join } from 'node:path';
import { BASE } from '../../helpers/env';
const CLIPS = [
{ file: 'sample.mp4', label: '1.000s — needs the 0s fallback' },
{ file: 'sample-5s.mp4', label: '5s — succeeds on the first seek' },
];
async function uploadClip(jwt: string, file: string): Promise<string> {
const bytes = readFileSync(join(process.cwd(), 'fixtures', 'media', file));
const res = await uploadRaw(jwt, bytes, { filename: file, contentType: 'video/mp4' });
expect(res.status, `uploading ${file}`).toBe(201);
return ((await res.json()) as { id: string }).id;
}
test.describe('Video — the poster frame is real', () => {
for (const { file, label } of CLIPS) {
test(`${file} (${label}) gets a fetchable poster`, async ({ guest, db }) => {
test.setTimeout(60_000);
const g = await guest(`Poster${file.replace(/\W/g, '')}`);
const id = await uploadClip(g.jwt, file);
await expect.poll(() => db.compressionStatus(id), { timeout: 45_000 }).toBe('done');
// The DB must not claim a thumbnail that isn't there — that claim IS the defect.
const res = await fetch(`${BASE}/api/v1/upload/${id}/thumbnail`, {
headers: { Authorization: `Bearer ${g.jwt}` },
});
expect(res.status, `${file}: the poster must exist, not just be recorded`).toBe(200);
expect(res.headers.get('content-type')).toContain('image/');
expect((await res.arrayBuffer()).byteLength).toBeGreaterThan(0);
});
}
test('a video upload still succeeds even if no poster can be extracted', async ({
guest,
db,
}) => {
// The mirror that keeps the fix honest. Tightening the check to "the file must exist" without
// also making a missing poster non-fatal would have been far worse than the bug: the worker's
// call used `?`, so every sub-second clip would fail compression, exhaust its retries and be
// soft-deleted. A cosmetic defect turned into data loss.
//
// `compression_status = 'done'` with the upload still present is exactly that guarantee.
const g = await guest('PosterSurvivor');
const id = await uploadClip(g.jwt, 'sample.mp4');
await expect.poll(() => db.compressionStatus(id), { timeout: 45_000 }).toBe('done');
expect(await db.countUploadsForUser(g.userId)).toBe(1);
// And the video itself is playable regardless of the poster.
const orig = await fetch(`${BASE}/api/v1/upload/${id}/original`, {
headers: { Authorization: `Bearer ${g.jwt}` },
});
expect(orig.status).toBe(200);
expect(orig.headers.get('content-type')).toContain('video/');
});
});

34
e2e/specs/05-admin/config.spec.ts
View File

@@ -68,40 +68,6 @@ test.describe('Admin — config API', () => {
expect(cfg.privacy_note).toBe(note); expect(cfg.privacy_note).toBe(note);
await api.patchConfig(adminToken, { privacy_note: '' }); await api.patchConfig(adminToken, { privacy_note: '' });
}); });
test('quota_tolerance = 0 is rejected, with a pointer to the real off-switch', async ({
api,
adminToken,
}) => {
// Zero is inside the documented 0–1 range and catastrophic: the per-user limit is
// `free_disk * tolerance / active_uploaders`, so 0 refuses EVERY upload — mid-event, with
// "Du hast dein Upload-Limit für dieses Event erreicht", which names the wrong cause
// entirely. `storage_quota_enabled` is what an admin reaching for an off-switch wants.
const res = await fetch(
(process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101') + '/api/v1/admin/config',
{
method: 'PATCH',
headers: { Authorization: `Bearer ${adminToken}`, 'Content-Type': 'application/json' },
body: JSON.stringify({ quota_tolerance: '0' }),
}
);
expect(res.status).toBe(400);
expect(
(await res.text()).toLowerCase(),
'the error must name the switch the admin actually wanted'
).toContain('speicher-quote');
// The value is untouched — validation fully precedes any write.
expect((await api.getConfig(adminToken)).quota_tolerance).toBe('0.75');
});
test('a very small quota_tolerance is still accepted', async ({ api, adminToken }) => {
// The mirror. Rejecting 0 must not become a floor: small tolerances are how a large disk is
// throttled to a sensible per-guest ceiling, and how the quota specs steer it (~1e-5 on a
// 174 GB volume). A floor of 0.01 would forbid real configurations to prevent one typo.
await api.patchConfig(adminToken, { quota_tolerance: '0.00001' });
expect((await api.getConfig(adminToken)).quota_tolerance).toBe('0.00001');
await api.patchConfig(adminToken, { quota_tolerance: '0.75' });
});
}); });
test.describe('Admin — stats', () => { test.describe('Admin — stats', () => {

103
e2e/specs/06-export/archive-permissions.spec.ts
View File

@@ -1,103 +0,0 @@
/**
* Regression guard — the keepsake extracts to files a guest can actually open.
*
* `ZipEntryBuilder::new` leaves the external file attribute at zero, and async_zip's host
* compatibility defaults to Unix — so every entry in BOTH archives was written with a stored mode
* of 0000. `unzip -Z` showed `?---------` on every line.
*
* Windows Explorer ignores Unix modes, which is exactly 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 — plus an index.html the browser refuses with ERR_ACCESS_DENIED.
*
* Unconditional: it affected every keepsake ever produced, no hostile input required. And it is
* invisible server-side — the export succeeds, the ZIP is well-formed, the job writes `done`,
* /export/status is green. The only way to see it is to extract the real artifact and try to read
* it, which is what this does.
*
* Found while chasing an unrelated ERR_ACCESS_DENIED that looked like a Playwright quirk.
*/
import { test, expect } from '../../fixtures/test';
import { execFileSync } from 'node:child_process';
import { mkdtempSync, writeFileSync, rmSync, readFileSync, statSync, readdirSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { seedUpload } from '../../helpers/seed';
import { BASE } from '../../helpers/env';
/** Walk every file under `dir`, ignoring the archives we dropped there ourselves. */
function walk(dir: string, skip: string[] = []): string[] {
return readdirSync(dir, { withFileTypes: true }).flatMap((e) => {
const p = join(dir, e.name);
if (e.isDirectory()) return walk(p, skip);
return skip.includes(e.name) ? [] : [p];
});
}
test.describe('Export — the archives extract to readable files', () => {
test('every entry in both keepsake archives is owner-readable', async ({ host, guest, db }) => {
test.setTimeout(120_000);
const bearer = { Authorization: `Bearer ${host.jwt}` };
const g = await guest('Archivist');
const id = await seedUpload(g.jwt, { caption: 'ein Foto' });
await expect.poll(() => db.compressionStatus(id), { timeout: 30_000 }).toBe('done');
expect(
(await fetch(`${BASE}/api/v1/host/gallery/release`, { method: 'POST', headers: bearer }))
.status
).toBe(204);
await expect
.poll(
async () => {
const res = await fetch(`${BASE}/api/v1/export/status`, { headers: bearer });
const s = await res.json();
return s.zip?.status === 'done' && s.html?.status === 'done';
},
{ timeout: 90_000, intervals: [500] }
)
.toBe(true);
for (const kind of ['zip', 'html'] as const) {
const ticketRes = await fetch(`${BASE}/api/v1/export/ticket`, {
method: 'POST',
headers: bearer,
});
const { ticket } = (await ticketRes.json()) as { ticket: string };
const dl = await fetch(`${BASE}/api/v1/export/${kind}?ticket=${encodeURIComponent(ticket)}`);
expect(dl.status, `downloading the ${kind} archive`).toBe(200);
const dir = mkdtempSync(join(tmpdir(), `eventsnap-perms-${kind}-`));
try {
const zipPath = join(dir, 'archive.zip');
writeFileSync(zipPath, Buffer.from(await dl.arrayBuffer()));
// The mode as STORED in the archive — this is what a guest's unzip will apply. Reading it
// from the central directory catches the defect even on a filesystem that would mask it.
const listing = execFileSync('unzip', ['-Z', zipPath], { encoding: 'utf8' });
const modes = listing
.split('\n')
.filter((l) => /^[?d-][rwx-]{9}\s/.test(l))
.map((l) => l.slice(0, 10));
expect(modes.length, `${kind}: no entries listed`).toBeGreaterThan(0);
for (const m of modes) {
expect(m, `${kind}: an entry is stored mode ${m} — the guest cannot open it`).toMatch(
/^.r[w-]-/
);
}
// And extraction really does produce readable files.
execFileSync('unzip', ['-qo', zipPath, '-d', dir]);
const files = walk(dir, ['archive.zip']);
expect(files.length, `${kind}: nothing extracted`).toBeGreaterThan(0);
for (const f of files) {
expect(statSync(f).mode & 0o400, `${f} is not owner-readable`).toBeTruthy();
// The assertion that matters to a guest: the bytes actually come out.
expect(() => readFileSync(f)).not.toThrow();
}
} finally {
rmSync(dir, { recursive: true, force: true });
}
}
});
});

21
e2e/specs/06-export/export-video.spec.ts
View File

@@ -6,16 +6,9 @@
* this drives a real video upload → export → and proves the video entry lands in * this drives a real video upload → export → and proves the video entry lands in
* Memories.zip (i.e. the streamed-from-original path works and the video isn't dropped). * Memories.zip (i.e. the streamed-from-original path works and the video isn't dropped).
* *
* NOTE ON A PREVIOUS VERSION OF THIS COMMENT. It used to read: "The fixture clip is <1s, so ffmpeg * The fixture clip is <1s, so ffmpeg extracts no thumbnail frame — but exits 0, so the
* extracts no thumbnail frame — but exits 0, so the compression worker keeps the upload. That's the * compression worker keeps the upload (it isn't auto-cleaned). That's the intended
* intended shape here." None of that was intended. `-ss` sat AFTER `-i` (an output-side seek), so * shape here: the full video is exported even when its thumbnail is absent.
* against `sample.mp4` — which is exactly 1.000 s — ffmpeg exited 0 having written nothing, and
* both the worker and the export gated on the exit status. The missing thumbnail was observed here
* and written down as expected behaviour instead of investigated; every video test in the suite ran
* against that one boundary fixture, and none of them ever fetched the poster.
*
* The seek is now input-side with a 0 s fallback and the artifact is verified rather than the exit
* code, so this clip DOES get a thumbnail. The assertion at the bottom pins that.
*/ */
import { test, expect } from '../../fixtures/test'; import { test, expect } from '../../fixtures/test';
import { uploadRaw } from '../../helpers/upload-client'; import { uploadRaw } from '../../helpers/upload-client';
@@ -72,13 +65,5 @@ test.describe('Export — video streaming (P4)', () => {
const needle = `media/${id}.mp4`; const needle = `media/${id}.mp4`;
const haystack = new TextDecoder('latin1').decode(bytes); const haystack = new TextDecoder('latin1').decode(bytes);
expect(haystack.includes(needle), `Memories.zip must contain ${needle}`).toBe(true); expect(haystack.includes(needle), `Memories.zip must contain ${needle}`).toBe(true);
// And its poster is really in the archive. This clip is 1.000 s — the exact case the old
// output-side seek produced nothing for, silently, while `data.json` still advertised the
// entry. See the note at the top of this file.
expect(
haystack.includes(`media/${id}_thumb.jpg`),
`Memories.zip must contain the poster for ${id}, not just reference it`
).toBe(true);
}); });
}); });

125
e2e/specs/06-export/viewer-caption-injection.spec.ts
View File

@@ -1,125 +0,0 @@
/**
* Regression guard — a guest-authored caption cannot brick the offline keepsake.
*
* The viewer's data is inlined as `<script>window.__EXPORT_DATA__={…};</script>` (it must be:
* guests open index.html over file://, where a cross-origin fetch of a sibling data.json is
* blocked). Captions and comments are guest text and land in that payload.
*
* The escape used to be `</` → `<\/`. Against XSS that holds — `</script><img src=x onerror=…>`
* 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>` steps back to script-data-escaped instead of closing the element. Everything after —
* including the viewer bundle — is swallowed as script data. Nothing executes and nothing leaks;
* `__EXPORT_DATA__` is never assigned and the keepsake renders blank.
*
* What makes it worth a browser-level test rather than a unit test alone: the failure is SILENT and
* POST-DISTRIBUTION. The export succeeds, the ZIP is well-formed, the job writes `done`,
* /export/status is green, and the host hands out a file that only fails when a guest
* double-clicks it — in every copy, unfixably. It is not visible by reading the escape. It is only
* visible by running a real parser over the real artifact, which is what this does: release, pull
* the actual Memories.zip, extract index.html, open it over file:// in Chromium, and assert the
* viewer actually booted.
*
* The near-miss 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 very thing it is testing for. Only the unterminated variant exposes it.
*/
import { test, expect } from '../../fixtures/test';
import { execFileSync } from 'node:child_process';
import { mkdtempSync, writeFileSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { seedUpload } from '../../helpers/seed';
import { BASE } from '../../helpers/env';
/** Unterminated on purpose — see the header. The terminated form self-repairs. */
const TOKENIZER_PAYLOAD = '<!--<script';
/** The classic break-out. Already handled, kept so the fix can never regress on it. */
const BREAKOUT_PAYLOAD = '</script><img src=x onerror=window.__XSS__=1>';
test.describe('Export — a caption cannot brick the keepsake viewer', () => {
test('the exported viewer boots with a tokenizer-hostile caption in it', async ({
page,
host,
guest,
db,
}) => {
test.setTimeout(120_000);
const bearer = { Authorization: `Bearer ${host.jwt}` };
const g = await guest('Trickster');
const a = await seedUpload(g.jwt, { caption: TOKENIZER_PAYLOAD });
const b = await seedUpload(g.jwt, { caption: BREAKOUT_PAYLOAD });
for (const id of [a, b]) {
await expect.poll(() => db.compressionStatus(id), { timeout: 30_000 }).toBe('done');
}
expect(
(await fetch(`${BASE}/api/v1/host/gallery/release`, { method: 'POST', headers: bearer }))
.status
).toBe(204);
await expect
.poll(
async () => {
const res = await fetch(`${BASE}/api/v1/export/status`, { headers: bearer });
return (await res.json()).html?.status;
},
{ timeout: 90_000, intervals: [500] }
)
.toBe('done');
const ticketRes = await fetch(`${BASE}/api/v1/export/ticket`, {
method: 'POST',
headers: bearer,
});
const { ticket } = (await ticketRes.json()) as { ticket: string };
const dl = await fetch(`${BASE}/api/v1/export/html?ticket=${encodeURIComponent(ticket)}`);
expect(dl.status).toBe(200);
const dir = mkdtempSync(join(tmpdir(), 'eventsnap-viewer-'));
try {
const zipPath = join(dir, 'Memories.zip');
writeFileSync(zipPath, Buffer.from(await dl.arrayBuffer()));
// Extract the WHOLE archive: index.html pulls in the viewer's own JS/CSS, and the point of
// this test is that those later resources are still reachable by the parser.
execFileSync('unzip', ['-qo', zipPath, '-d', dir]);
// file://, not http://. That is how a guest actually opens the keepsake, and it is the
// whole reason the data is inlined rather than fetched from a sibling data.json.
let xss = false;
page.on('dialog', (d) => {
xss = true;
void d.dismiss();
});
await page.goto('file://' + join(dir, 'index.html'));
// 1. The payload was assigned at all. This is the assertion that fails on the old escape —
// the second script block is never reached, so the global stays undefined.
const captions = await page.evaluate(() => {
const d = (window as unknown as { __EXPORT_DATA__?: { posts?: { caption?: string }[] } })
.__EXPORT_DATA__;
return d?.posts?.map((p) => p.caption ?? '') ?? null;
});
expect(captions, '__EXPORT_DATA__ was never assigned — the viewer is bricked').not.toBeNull();
// 2. The captions survived verbatim. The escape is a transport encoding, not a sanitiser:
// a guest's text has to come back exactly, or we have silently rewritten their words.
expect(captions).toContain(TOKENIZER_PAYLOAD);
expect(captions).toContain(BREAKOUT_PAYLOAD);
// 3. And nothing executed.
expect(
await page.evaluate(() => (window as unknown as { __XSS__?: number }).__XSS__ === 1),
'the caption must be inert, not merely non-fatal'
).toBe(false);
expect(xss).toBe(false);
// 4. The viewer actually rendered — the whole document parsed, not just the head. If the
// tokenizer had swallowed the bundle, the body would be empty of viewer output.
await expect(page.locator('body')).not.toBeEmpty();
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
});

113
e2e/specs/06-export/viewer-no-broken-tiles.spec.ts
View File

@@ -1,113 +0,0 @@
/**
* Regression guard — the keepsake never renders a broken image tile.
*
* The HTML export wrote `thumb: "media/<id>_thumb.jpg"` into `data.json` unconditionally. When
* ffmpeg produced no poster frame — which it did, silently and with exit 0, for any clip of a
* second or less — the ZIP writer skipped the unopenable file but `data.json` still advertised it.
* The viewer then requested an entry the archive did not contain and drew a broken `<img>`.
*
* The viewer was never the problem: `+page.svelte` already guards `{#if post.media.thumb}` and
* falls back to a proper dark video tile with a play glyph. The guard simply never fired, because
* the backend always handed it a non-empty string. The fix is the backend telling the truth —
* `thumb: ""` when there is no poster — so no viewer change was needed.
*
* This asserts the property that actually matters to a guest and that no server-side signal can
* report: **every image in the opened keepsake resolves**. `naturalWidth > 0` is false for exactly
* the broken-tile case, whatever produced it — a missing video poster, a failed image thumbnail, or
* some future path nobody has thought of yet. It is deliberately not an assertion about ffmpeg.
*
* Runs over `file://`, the way a guest opens it.
*/
import { test, expect } from '../../fixtures/test';
import { execFileSync } from 'node:child_process';
import { mkdtempSync, writeFileSync, rmSync, readFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { uploadRaw } from '../../helpers/upload-client';
import { seedUpload } from '../../helpers/seed';
import { BASE } from '../../helpers/env';
test.describe('Export — the keepsake has no broken tiles', () => {
test('every image in the opened viewer resolves', async ({ page, host, guest, db }) => {
test.setTimeout(150_000);
const bearer = { Authorization: `Bearer ${host.jwt}` };
const g = await guest('TileChecker');
const ids: string[] = [await seedUpload(g.jwt, { caption: 'ein Foto' })];
// Both clips: the 1.000 s one is the case that produced the broken tile, the 5 s one is the
// ordinary path that had no coverage at all.
for (const file of ['sample.mp4', 'sample-5s.mp4']) {
const bytes = readFileSync(join(process.cwd(), 'fixtures', 'media', file));
const res = await uploadRaw(g.jwt, bytes, { filename: file, contentType: 'video/mp4' });
expect(res.status).toBe(201);
ids.push(((await res.json()) as { id: string }).id);
}
for (const id of ids) {
await expect.poll(() => db.compressionStatus(id), { timeout: 45_000 }).toBe('done');
}
expect(
(await fetch(`${BASE}/api/v1/host/gallery/release`, { method: 'POST', headers: bearer }))
.status
).toBe(204);
await expect
.poll(
async () => {
const res = await fetch(`${BASE}/api/v1/export/status`, { headers: bearer });
return (await res.json()).html?.status;
},
{ timeout: 120_000, intervals: [500] }
)
.toBe('done');
const ticketRes = await fetch(`${BASE}/api/v1/export/ticket`, {
method: 'POST',
headers: bearer,
});
const { ticket } = (await ticketRes.json()) as { ticket: string };
const dl = await fetch(`${BASE}/api/v1/export/html?ticket=${encodeURIComponent(ticket)}`);
expect(dl.status).toBe(200);
const dir = mkdtempSync(join(tmpdir(), 'eventsnap-tiles-'));
try {
const zipPath = join(dir, 'Memories.zip');
writeFileSync(zipPath, Buffer.from(await dl.arrayBuffer()));
execFileSync('unzip', ['-qo', zipPath, '-d', dir]);
await page.goto('file://' + join(dir, 'index.html'));
// Every post is present, whether or not it has a poster.
const posts = await page.evaluate(() => {
const d = (
window as unknown as {
__EXPORT_DATA__?: { posts?: { media?: { thumb?: string; type?: string } }[] };
}
).__EXPORT_DATA__;
return d?.posts?.map((p) => ({ thumb: p.media?.thumb ?? '', type: p.media?.type })) ?? null;
});
expect(posts, '__EXPORT_DATA__ was never assigned').not.toBeNull();
expect(posts!.length).toBe(3);
// Any thumb data.json DOES advertise must be a file the archive actually contains.
const entries = execFileSync('unzip', ['-Z1', zipPath], { encoding: 'utf8' }).split('\n');
for (const p of posts!.filter((p) => p.thumb)) {
expect(
entries.includes(p.thumb),
`data.json advertises ${p.thumb} but the archive does not contain it`
).toBe(true);
}
// THE assertion: nothing rendered broken. Give the images a moment to settle first.
await page.waitForLoadState('networkidle');
const broken = await page.evaluate(() =>
Array.from(document.querySelectorAll('img'))
.filter((i) => i.complete && i.naturalWidth === 0)
.map((i) => i.getAttribute('src') ?? '(no src)')
);
expect(broken, `broken image tiles in the keepsake: ${broken.join(', ')}`).toEqual([]);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
});

111
e2e/specs/07-adversarial/auth-tampering.spec.ts
View File

@@ -89,43 +89,15 @@ test.describe('Adversarial — JWT', () => {
}); });
test.describe('Adversarial — PIN brute-force', () => { test.describe('Adversarial — PIN brute-force', () => {
/** test('sequential wrong-PIN attempts lock the account after 3 attempts', async ({ guest }) => {
* The PIN defence has two tiers, and telling them apart is the whole point of these tests:
*
* - the per-(IP, name) throttle, which refuses the ATTACKER; and
* - the account lock, which refuses the VICTIM — the only tier that can be weaponised.
*
* Both answer 429, so the status code alone proves nothing. The regression these guard is that
* the lock threshold used to sit BELOW the throttle ceiling (3 vs 5), so three requests from a
* single IP locked any guest whose display name is readable off the feed, every 15 minutes,
* indefinitely. The tier meant to protect a guest was the cheapest way to attack them.
*/
// Restore the default. The first test turns the limiter on for the whole instance, and leaving
// it on would throttle unrelated specs sharing this stack. Runs even on failure.
test.afterEach(async ({ api, adminToken }) => {
await api.patchConfig(adminToken, { rate_limits_enabled: 'false' });
});
test('a single IP is throttled without ever locking the victim out', async ({
api,
adminToken,
guest,
db,
}) => {
// Rate limits are off by default in this environment, and the throttle IS the tier under
// test — without it the run would silently assert only half the property.
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
recover_rate_enabled: 'true',
});
const g = await guest('Brute'); const g = await guest('Brute');
const wrong = g.pin === '0000' ? '1111' : '0000'; const wrong = g.pin === '0000' ? '1111' : '0000';
// Serially, so the failed-PIN counter increments monotonically. Well past the per-(IP, name) // Do them serially so the failed_pin_attempts counter increments
// ceiling of 4, and past the OLD lock threshold of 3. // monotonically. Parallel attempts race and may never accumulate to 3 in
// the current handler implementation — that's a separate finding.
const statuses: number[] = []; const statuses: number[] = [];
for (let i = 0; i < 8; i++) { for (let i = 0; i < 4; i++) {
const r = await fetch(`${BASE}/api/v1/recover`, { const r = await fetch(`${BASE}/api/v1/recover`, {
method: 'POST', method: 'POST',
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
@@ -133,52 +105,26 @@ test.describe('Adversarial — PIN brute-force', () => {
}); });
statuses.push(r.status); statuses.push(r.status);
} }
expect(statuses.filter((s) => s === 200), 'a wrong PIN must never authenticate').toHaveLength( // First three are 401, fourth (or later) is 429.
0 expect(statuses.filter((s) => s === 200)).toHaveLength(0);
); expect(statuses.some((s) => s === 429)).toBe(true);
expect(statuses.some((s) => s === 429), 'the attacker must be throttled').toBe(true);
expect( // Now even the correct PIN fails until lockout expires.
await db.isPinLocked(g.userId),
'one IP must not be able to lock a guest out of their own account'
).toBe(false);
});
test('the account still locks once the failure count is reached', async ({ guest, db }) => {
const g = await guest('BruteDistributed');
const wrong = g.pin === '0000' ? '1111' : '0000';
// The per-IP throttle is what a single source hits first, so drive the counter the way a
// DISTRIBUTED attacker would — the tier this test covers is the last line against exactly
// that, and it must not have been removed while fixing the weaponisation above.
await db.setFailedPinAttempts(g.userId, 11);
const r = await fetch(`${BASE}/api/v1/recover`, {
method: 'POST',
headers: { 'Content-Type': 'application/json', 'X-Forwarded-For': '203.0.113.77' },
body: JSON.stringify({ display_name: g.displayName, pin: wrong }),
});
expect(r.status).toBe(401);
expect(
await db.isPinLocked(g.userId),
'a distributed guesser must still trip the account lock'
).toBe(true);
// And the lock holds even against the correct PIN, which is what makes it a real control.
const correct = await fetch(`${BASE}/api/v1/recover`, { const correct = await fetch(`${BASE}/api/v1/recover`, {
method: 'POST', method: 'POST',
headers: { 'Content-Type': 'application/json', 'X-Forwarded-For': '203.0.113.78' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ display_name: g.displayName, pin: g.pin }), body: JSON.stringify({ display_name: g.displayName, pin: g.pin }),
}); });
expect(correct.status).toBe(429); expect(correct.status).toBe(429);
}); });
test('the wrong-PIN streak is atomic under concurrency', async ({ guest, db }) => { test('parallel wrong-PIN attempts still lock the account (counter is not lost to the race)', async ({
guest,
}) => {
const g = await guest('BruteParallel'); const g = await guest('BruteParallel');
const wrong = g.pin === '0000' ? '1111' : '0000'; const wrong = g.pin === '0000' ? '1111' : '0000';
await Promise.all( const attempts = await Promise.all(
Array.from({ length: 10 }, () => Array.from({ length: 10 }, () =>
fetch(`${BASE}/api/v1/recover`, { fetch(`${BASE}/api/v1/recover`, {
method: 'POST', method: 'POST',
@@ -187,14 +133,29 @@ test.describe('Adversarial — PIN brute-force', () => {
}) })
) )
); );
const statuses = attempts.map((r) => r.status);
// How many of the 10 get past the throttle is genuinely racy and cannot be asserted. What is
// NOT racy is that every one that DID reach the handler incremented the counter — it is a
// single `SET x = x + 1 ... RETURNING`, so none of them can be lost to the race.
expect( expect(
await db.failedPinAttempts(g.userId), statuses.filter((s) => s === 200),
'concurrent wrong PINs must all be counted' 'a wrong PIN must never authenticate'
).toBeGreaterThan(1); ).toHaveLength(0);
// The in-flight requests all read `pin_locked_until` before any of them wrote it, so
// *which* of the 10 come back 429 is genuinely racy and can't be asserted. What is NOT
// racy — and is the property this test exists to guard — is the state left behind:
// `failed_pin_attempts` is incremented with an atomic `SET x = x + 1 ... RETURNING`, so
// 10 wrong PINs must push it past the 3-strike threshold and leave the account locked.
//
// We prove that with a follow-up request using the CORRECT pin: it must be refused with
// 429 (locked), not 200. Delete the lockout counter and this line goes 200 → red.
const correct = await fetch(`${BASE}/api/v1/recover`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ display_name: g.displayName, pin: g.pin }),
});
expect(
correct.status,
'after 10 wrong PINs the account must be locked, even for the right PIN'
).toBe(429);
}); });
}); });

34
frontend/src/lib/api.ts
View File

@@ -27,13 +27,6 @@ async function request<T>(method: string, path: string, body?: unknown): Promise
// Abort hung requests so a dead connection surfaces as a friendly error // Abort hung requests so a dead connection surfaces as a friendly error
// instead of a spinner that never resolves. // instead of a spinner that never resolves.
//
// The timer must stay armed until the BODY has been read, not just the headers.
// `fetch` resolves as soon as the response head arrives, so clearing it in a `finally`
// around the fetch left `res.text()` below completely uncovered — and no longer
// abortable, since the controller had already been disarmed. An upstream that sends
// headers and then stalls the body (the shape of a half-dead proxy, or of the pool
// saturation this same release adds shedding for) hung that call forever.
const controller = new AbortController(); const controller = new AbortController();
const timer = setTimeout(() => controller.abort(), TIMEOUT_MS); const timer = setTimeout(() => controller.abort(), TIMEOUT_MS);
@@ -45,26 +38,6 @@ async function request<T>(method: string, path: string, body?: unknown): Promise
body: body !== undefined ? JSON.stringify(body) : undefined, body: body !== undefined ? JSON.stringify(body) : undefined,
signal: controller.signal signal: controller.signal
}); });
} catch (e) {
clearTimeout(timer);
if (e instanceof DOMException && e.name === 'AbortError') {
throw new ApiError(0, 'timeout', 'Zeitüberschreitung – bitte erneut versuchen.');
}
throw new ApiError(0, 'network', 'Netzwerkfehler – bitte Verbindung prüfen.');
}
if (res.status === 204) {
// Must clear on this path too, or every no-content request (logout, delete, like)
// leaks a live 20 s timer.
clearTimeout(timer);
return undefined as T;
}
// A 5xx behind a proxy (or a crash page) can return HTML, not JSON — parsing
// it directly would throw an opaque SyntaxError. Read text, parse defensively.
let raw: string;
try {
raw = await res.text();
} catch (e) { } catch (e) {
if (e instanceof DOMException && e.name === 'AbortError') { if (e instanceof DOMException && e.name === 'AbortError') {
throw new ApiError(0, 'timeout', 'Zeitüberschreitung – bitte erneut versuchen.'); throw new ApiError(0, 'timeout', 'Zeitüberschreitung – bitte erneut versuchen.');
@@ -74,6 +47,13 @@ async function request<T>(method: string, path: string, body?: unknown): Promise
clearTimeout(timer); clearTimeout(timer);
} }
if (res.status === 204) {
return undefined as T;
}
// A 5xx behind a proxy (or a crash page) can return HTML, not JSON — parsing
// it directly would throw an opaque SyntaxError. Read text, parse defensively.
const raw = await res.text();
let data: { error?: string; message?: string } | unknown = null; let data: { error?: string; message?: string } | unknown = null;
if (raw) { if (raw) {
try { try {

36
frontend/src/lib/upload-queue.test.ts
View File

@@ -1,10 +1,5 @@
import { describe, it, expect } from 'vitest'; import { describe, it, expect } from 'vitest';
import { import { classifyUploadStatus, isReversibleLock, entryToQueueItem } from './upload-queue';
classifyUploadStatus,
isReversibleLock,
entryToQueueItem,
shouldAbortForStall
} from './upload-queue';
/** /**
* Regression guard for the upload-queue retry policy (H2 + M1). The bug being locked out: * Regression guard for the upload-queue retry policy (H2 + M1). The bug being locked out:
@@ -112,32 +107,3 @@ describe('entryToQueueItem', () => {
expect(item.hashtags).toBe(''); expect(item.hashtags).toBe('');
}); });
}); });
/**
* The upload XHR had no timeout of any kind while `processQueue` held the `isProcessing`
* latch across it. On a half-open socket neither `error` nor `abort` ever fires, so the
* latch was pinned forever and the whole queue wedged with no recovery but a reload.
*
* The policy that matters: bound SILENCE, not total duration. A 500 MB video over a venue
* uplink legitimately runs 30+ minutes while making steady progress, and a flat total cap
* would kill exactly the uploads worth keeping.
*/
describe('shouldAbortForStall', () => {
const now = 1_000_000;
it('lets a long upload run as long as progress keeps arriving', () => {
// Two hours in, but progress landed a second ago.
expect(shouldAbortForStall(now - 1_000, now, false)).toBe(false);
});
it('aborts once the body stalls past the no-progress ceiling', () => {
expect(shouldAbortForStall(now - 89_000, now, false)).toBe(false);
expect(shouldAbortForStall(now - 91_000, now, false)).toBe(true);
});
it('applies the wider ceiling once the body is sent and progress goes quiet', () => {
// Silence that would abort mid-body is normal while waiting for the response.
expect(shouldAbortForStall(now - 91_000, now, true)).toBe(false);
expect(shouldAbortForStall(now - 121_000, now, true)).toBe(true);
});
});

234
frontend/src/lib/upload-queue.ts
View File

@@ -1,6 +1,6 @@
import { openDB, type IDBPDatabase } from 'idb'; import { openDB, type IDBPDatabase } from 'idb';
import { writable, get } from 'svelte/store'; import { writable, get } from 'svelte/store';
import { getToken, getUserId, clearAuth, onSetAuth, onClearAuth } from './auth'; import { getToken, getUserId, clearAuth } from './auth';
import { onSseEvent } from './sse'; import { onSseEvent } from './sse';
import { refreshQuota } from './quota-store'; import { refreshQuota } from './quota-store';
import { toast } from './toast-store'; import { toast } from './toast-store';
@@ -37,37 +37,6 @@ const STORE_NAME = 'queue';
/** Hard cap on queued items per device — bounds IndexedDB growth from stuck blobs. */ /** Hard cap on queued items per device — bounds IndexedDB growth from stuck blobs. */
const MAX_QUEUE_ITEMS = 100; const MAX_QUEUE_ITEMS = 100;
// ── Upload watchdog ───────────────────────────────────────────────────────────
// The upload XHR had no timeout of any kind while `processQueue` held the `processing` /
// `isProcessing` latch across it. A half-open socket — the classic phone-leaves-wifi case,
// where no `error` and no `abort` event ever fires — pinned that latch forever and wedged
// the whole queue with no recovery short of a reload.
//
// Deliberately NOT a flat total timeout: a legitimate 500 MB video over a venue uplink can
// run 30+ minutes while making perfectly steady progress, and a total cap would kill exactly
// the uploads that matter most. What we bound is SILENCE.
/** No-progress ceiling while the request body is still being sent. */
const UPLOAD_STALL_MS = 90_000;
/** Ceiling for the window AFTER the last byte is sent. `upload.progress` is silent there by
* definition (the server is sniffing, committing and answering), so the stall detector has
* no signal and this coarser bound takes over. Sized above the backend's own worst-case
* commit path, not above compression — compression is async and does not hold the response. */
const UPLOAD_RESPONSE_TIMEOUT_MS = 120_000;
/** How often the watchdog re-checks. Coarse on purpose; it only needs to bound the wedge. */
const UPLOAD_WATCHDOG_INTERVAL_MS = 5_000;
/** Pure predicate behind the watchdog, extracted so the policy is unit-testable without
* standing up an XHR harness. */
export function shouldAbortForStall(
lastActivityAt: number,
now: number,
bodySent: boolean
): boolean {
const ceiling = bodySent ? UPLOAD_RESPONSE_TIMEOUT_MS : UPLOAD_STALL_MS;
return now - lastActivityAt > ceiling;
}
let db: IDBPDatabase | null = null; let db: IDBPDatabase | null = null;
// Resume the queue as soon as connectivity returns. Registered once, guarded for SSR. // Resume the queue as soon as connectivity returns. Registered once, guarded for SSR.
@@ -77,7 +46,10 @@ let onlineBound = false;
function bindOnline(): void { function bindOnline(): void {
if (onlineBound || typeof window === 'undefined') return; if (onlineBound || typeof window === 'undefined') return;
window.addEventListener('online', () => { window.addEventListener('online', () => {
void loadQueue(); void (async () => {
await requeueRetriable();
await processQueue();
})();
}); });
onlineBound = true; onlineBound = true;
} }
@@ -97,7 +69,10 @@ let sseBound = false;
function bindSse(): void { function bindSse(): void {
if (sseBound || typeof window === 'undefined') return; if (sseBound || typeof window === 'undefined') return;
const resume = () => { const resume = () => {
void loadQueue(); void (async () => {
await requeueRetriable();
await processQueue();
})();
}; };
onSseEvent('event-opened', resume); onSseEvent('event-opened', resume);
onSseEvent('feed-delta', resume); onSseEvent('feed-delta', resume);
@@ -106,42 +81,28 @@ function bindSse(): void {
bindSse(); bindSse();
/** /**
* Rebuild the in-memory queue from IndexedDB, flipping transient `error` items (5xx / a * Flip transient `error` items (5xx / a network drop that got marked before we could
* network drop that got marked before we could reclassify it) back to `pending` so a resume * reclassify it) back to `pending` so a resume actually retries them. Terminal `blocked`
* actually retries them. Terminal `blocked` items (403/413) are left alone — retrying those * items (403/413) are left alone — retrying those never succeeds.
* never succeeds.
*
* REBUILDS rather than patches, and that is the fix. This used to `.map()` over whatever the
* store already held, so it could reset statuses but never ADD an entry — and `processQueue`
* reads only the in-memory store. After a reload or an iOS tab discard the store starts empty,
* so persisted items were invisible to every resume path: the badge read 0 and photos the
* guest had been shown as queued never left the phone.
*/ */
async function requeueRetriable(): Promise<void> { async function requeueRetriable(): Promise<void> {
const database = await getDb(); const database = await getDb();
const myUserId = getUserId(); const myUserId = getUserId();
const all = await database.getAll(STORE_NAME); const all = await database.getAll(STORE_NAME);
// Only surface entries that belong to the current user. Entries from a previous guest on for (const entry of all) {
// this device are filtered out (and are wiped on their next explicit logout via if (entry.userId === myUserId && entry.status === 'error' && entry.blob) {
// `clearQueue`).
const mine = all.filter((entry) => entry.userId && entry.userId === myUserId);
for (const entry of mine) {
if (entry.status === 'error' && entry.blob) {
entry.status = 'pending'; entry.status = 'pending';
entry.error = undefined; entry.error = undefined;
await database.put(STORE_NAME, entry); await database.put(STORE_NAME, entry);
} }
} }
// Preserve anything actually on the wire. `entryToQueueItem` downgrades `uploading` to queueItems.update((items) =>
// `pending` with progress 0, so rebuilding blindly would visibly reset the progress bar of items.map((item) =>
// a request still in flight — and this runs on every `online` event and every SSE item.status === 'error'
// reconnect, not just at startup. ? { ...item, status: 'pending' as const, progress: 0, error: undefined }
const inFlight = new Map( : item
get(queueItems) )
.filter((i) => i.status === 'uploading')
.map((i) => [i.id, i])
); );
queueItems.set(mine.map((entry) => inFlight.get(entry.id) ?? entryToQueueItem(entry)));
} }
async function getDb(): Promise<IDBPDatabase> { async function getDb(): Promise<IDBPDatabase> {
@@ -178,10 +139,8 @@ async function getDb(): Promise<IDBPDatabase> {
* blamed for) the previous guest's pending uploads. * blamed for) the previous guest's pending uploads.
*/ */
export async function clearQueue(): Promise<void> { export async function clearQueue(): Promise<void> {
// Always clear the in-memory view, even if the store is unreachable: this runs on logout, const database = await getDb();
// and leaving the previous guest's items on screen for the next one is the worse failure. await database.clear(STORE_NAME);
const database = await getDbSafe();
if (database) await database.clear(STORE_NAME);
queueItems.set([]); queueItems.set([]);
rateLimitRetryAt.set(null); rateLimitRetryAt.set(null);
} }
@@ -306,77 +265,37 @@ export function entryToQueueItem(entry: {
}; };
} }
/**
* Load the persisted queue into memory and start draining it.
*
* Idempotent and cheap — safe to call from anywhere, as often as you like.
*/
export async function loadQueue(): Promise<void> { export async function loadQueue(): Promise<void> {
await requeueRetriable(); const database = await getDb();
void processQueue(); const myUserId = getUserId();
const all = await database.getAll(STORE_NAME);
// Only surface entries that belong to the current user. Entries from a previous
// guest on this device are filtered out (and would also be wiped on their next
// explicit logout via `clearQueue`).
const items: QueueItem[] = all
.filter((entry) => entry.userId && entry.userId === myUserId)
.map(entryToQueueItem);
queueItems.set(items);
// Staged-but-unsent items from a prior session (queued offline, tab closed before
// reconnect) must resume now — otherwise the "queue flushes when you're back online"
// promise only holds if the user manually re-stages a file. Reclaim transient errors
// (a network drop from a prior session) so they retry instead of stalling.
void (async () => {
await requeueRetriable();
await processQueue();
})();
} }
/**
* Rehydrate the queue once per signed-in session, app-wide.
*
* `loadQueue` used to have exactly ONE call site: the `/upload` route's `onMount`. So after a
* reload, an iOS tab discard, or a PWA relaunch, staged photos sat in IndexedDB while the badge
* read 0, and nothing sent them unless the guest happened to navigate back to `/upload` — which
* they have no reason to do, having already been shown a success.
*
* Module level rather than a layout `onMount`, for three reasons: `+layout.svelte` already
* imports this module on every entry point so the side effect runs everywhere with no new
* import; it matches the file's own `bindOnline()` / `bindSse()` pattern directly above; and a
* store module owning its own persistence keeps the layout free of a concern it cannot test.
*
* Re-armed on auth changes because login is a client-side `goto()` — no module re-import and no
* `onMount` re-run — so a hydration that no-oped for lack of a token must get a second chance.
* `auth.ts` does not import this module, so these imports create no cycle.
*/
let hydrated = false;
async function hydrateQueue(): Promise<void> {
if (hydrated || typeof window === 'undefined') return;
if (!getToken() || !getUserId()) return;
hydrated = true;
try {
await loadQueue();
} catch (e) {
// Let a later signal try again rather than wedging the queue for the session.
hydrated = false;
console.warn('upload queue rehydration failed', e);
}
}
hydrateQueue();
onSetAuth(() => {
hydrated = false;
void hydrateQueue();
});
onClearAuth(() => {
hydrated = false;
});
/** Outcome of an `addToQueue` call, so the caller can tell the user when a file was NOT /** Outcome of an `addToQueue` call, so the caller can tell the user when a file was NOT
* actually queued (deduped, the queue is full of un-evictable in-flight items, or the * actually queued (deduped, or the queue is full of un-evictable in-flight items). */
* local store itself is unusable). */ export type EnqueueResult = 'queued' | 'duplicate' | 'full';
export type EnqueueResult = 'queued' | 'duplicate' | 'full' | 'failed';
export async function addToQueue( export async function addToQueue(
file: File, file: File,
caption: string, caption: string,
hashtags: string hashtags: string
): Promise<EnqueueResult> { ): Promise<EnqueueResult> {
// IndexedDB is not guaranteed available: Safari private mode refuses to open a DB, an const database = await getDb();
// upgrade can be blocked by another tab, and `put` of a 500 MB blob can hit a
// QuotaExceededError. Every one of those used to reject out of here into a `handleSubmit`
// with no catch — leaving a permanent "Wird hochgeladen…" spinner, no toast, and (for an
// in-app camera capture) the only copy of the photo gone. Report it instead.
let database: IDBPDatabase;
try {
database = await getDb();
} catch (e) {
console.warn('upload queue unavailable (IndexedDB)', e);
return 'failed';
}
const userId = getUserId(); const userId = getUserId();
// Not authenticated — nothing to queue. Return the silent 'duplicate' outcome rather than // Not authenticated — nothing to queue. Return the silent 'duplicate' outcome rather than
// 'full' so the caller doesn't show a misleading "queue full" toast. Practically // 'full' so the caller doesn't show a misleading "queue full" toast. Practically
@@ -429,14 +348,7 @@ export async function addToQueue(
status: 'pending', status: 'pending',
blob: file blob: file
}; };
// Persist BEFORE touching the store: a failure here (quota exceeded on a large blob) must await database.put(STORE_NAME, entry);
// not leave a phantom item the UI shows as queued but nothing can ever upload.
try {
await database.put(STORE_NAME, entry);
} catch (e) {
console.warn('upload queue write failed (IndexedDB)', e);
return 'failed';
}
queueItems.update((items) => [ queueItems.update((items) => [
...items, ...items,
@@ -458,21 +370,8 @@ export async function addToQueue(
return 'queued'; return 'queued';
} }
/** `getDb` that reports unavailability instead of rejecting. For the UI-driven queue
* operations, where an unusable IndexedDB must degrade to "nothing happened" rather than
* leave a caller awaiting a rejected promise it never catches. */
async function getDbSafe(): Promise<IDBPDatabase | null> {
try {
return await getDb();
} catch (e) {
console.warn('upload queue unavailable (IndexedDB)', e);
return null;
}
}
export async function retryItem(id: string): Promise<void> { export async function retryItem(id: string): Promise<void> {
const database = await getDbSafe(); const database = await getDb();
if (!database) return;
const entry = await database.get(STORE_NAME, id); const entry = await database.get(STORE_NAME, id);
if (!entry) return; if (!entry) return;
@@ -490,15 +389,13 @@ export async function retryItem(id: string): Promise<void> {
} }
export async function removeItem(id: string): Promise<void> { export async function removeItem(id: string): Promise<void> {
const database = await getDbSafe(); const database = await getDb();
if (!database) return;
await database.delete(STORE_NAME, id); await database.delete(STORE_NAME, id);
queueItems.update((items) => items.filter((item) => item.id !== id)); queueItems.update((items) => items.filter((item) => item.id !== id));
} }
export async function clearCompleted(): Promise<void> { export async function clearCompleted(): Promise<void> {
const database = await getDbSafe(); const database = await getDb();
if (!database) return;
const items = get(queueItems); const items = get(queueItems);
for (const item of items) { for (const item of items) {
if (item.status === 'done') { if (item.status === 'done') {
@@ -598,24 +495,7 @@ async function uploadItem(id: string): Promise<void> {
xhr.open('POST', '/api/v1/upload'); xhr.open('POST', '/api/v1/upload');
xhr.setRequestHeader('Authorization', `Bearer ${token}`); xhr.setRequestHeader('Authorization', `Bearer ${token}`);
// Watchdog state — see UPLOAD_STALL_MS. `timedOut` distinguishes our own abort from
// a user-initiated one so the surfaced message stays honest.
let lastActivityAt = Date.now();
let bodySent = false;
let timedOut = false;
let watchdog: ReturnType<typeof setInterval> | undefined;
const stopWatchdog = () => {
if (watchdog !== undefined) {
clearInterval(watchdog);
watchdog = undefined;
}
};
// `loadend` on the XHR itself fires on success, error, abort and timeout alike — the
// one hook that guarantees the interval is released on every exit path.
xhr.addEventListener('loadend', stopWatchdog);
xhr.upload.addEventListener('progress', (e) => { xhr.upload.addEventListener('progress', (e) => {
lastActivityAt = Date.now();
if (e.lengthComputable) { if (e.lengthComputable) {
const pct = Math.round((e.loaded / e.total) * 100); const pct = Math.round((e.loaded / e.total) * 100);
queueItems.update((items) => queueItems.update((items) =>
@@ -677,27 +557,9 @@ async function uploadItem(id: string): Promise<void> {
} }
}); });
// Body fully handed to the socket: `upload.progress` goes quiet from here, so switch
// the watchdog to the response ceiling rather than let it fire on normal waiting.
xhr.upload.addEventListener('loadend', () => {
bodySent = true;
lastActivityAt = Date.now();
});
xhr.addEventListener('error', () => reject(new NetworkError('Netzwerkfehler'))); xhr.addEventListener('error', () => reject(new NetworkError('Netzwerkfehler')));
xhr.addEventListener('abort', () => xhr.addEventListener('abort', () => reject(new NetworkError('Abgebrochen')));
reject(new NetworkError(timedOut ? 'Zeitüberschreitung' : 'Abgebrochen'))
);
xhr.send(formData); xhr.send(formData);
// Armed only after send(), so the clock starts with the request. NetworkError is
// already the retryable branch (blob kept, "Erneut" offered), so a stalled upload
// now recovers exactly like a network blip instead of wedging the queue.
watchdog = setInterval(() => {
if (!shouldAbortForStall(lastActivityAt, Date.now(), bodySent)) return;
stopWatchdog();
timedOut = true;
xhr.abort();
}, UPLOAD_WATCHDOG_INTERVAL_MS);
}); });
// Success — remove blob from IndexedDB, mark done // Success — remove blob from IndexedDB, mark done

35
frontend/src/routes/feed/+page.svelte
View File

@@ -273,23 +273,7 @@
// A processed upload gains preview/thumbnail URLs. Coalesce bursts (bulk // A processed upload gains preview/thumbnail URLs. Coalesce bursts (bulk
// uploads fire one per file) into a single in-place merge so the feed // uploads fire one per file) into a single in-place merge so the feed
// neither hammers the server nor collapses to page 1 / loses scroll. // neither hammers the server nor collapses to page 1 / loses scroll.
onSseEvent('upload-processed', (data) => { onSseEvent('upload-processed', () => scheduleInPlaceRefresh()),
// Only refetch if this client actually SHOWS the card that changed. Without the
// membership test every open feed in the venue (~100 at a reception) fired a
// page-1 refetch for every completed upload — a self-inflicted thundering herd
// on the most expensive page, triggered by the most common event.
//
// Nothing is lost by skipping: a client that doesn't have the card also missed
// its `new-upload`, and the reconnect `feed-delta` below already calls
// scheduleInPlaceRefresh().
try {
const { upload_id } = JSON.parse(data) as { upload_id: string };
if (!uploads.some((u) => u.id === upload_id)) return;
} catch {
return;
}
scheduleInPlaceRefresh();
}),
onSseEvent('upload-deleted', (data) => { onSseEvent('upload-deleted', (data) => {
try { try {
const payload = JSON.parse(data) as { upload_id: string }; const payload = JSON.parse(data) as { upload_id: string };
@@ -415,21 +399,12 @@
// Debounced page-1 fetch that *merges* (updates existing cards in place, prepends // Debounced page-1 fetch that *merges* (updates existing cards in place, prepends
// genuinely new ones) rather than replacing the array — preserves scroll and any // genuinely new ones) rather than replacing the array — preserves scroll and any
// pages already loaded below the fold. // pages already loaded below the fold.
/** Debounce floor. */
const IN_PLACE_REFRESH_MIN_MS = 800;
/** Jitter added on top. Every open feed receives the same SSE at the same instant, so a
* fixed delay just moves the herd 800 ms later instead of spreading it. */
const IN_PLACE_REFRESH_JITTER_MS = 1200;
function scheduleInPlaceRefresh() { function scheduleInPlaceRefresh() {
if (inPlaceRefreshTimer) return; if (inPlaceRefreshTimer) return;
inPlaceRefreshTimer = setTimeout( inPlaceRefreshTimer = setTimeout(() => {
() => { inPlaceRefreshTimer = null;
inPlaceRefreshTimer = null; void refreshFeedInPlace();
void refreshFeedInPlace(); }, 800);
},
IN_PLACE_REFRESH_MIN_MS + Math.random() * IN_PLACE_REFRESH_JITTER_MS
);
} }
async function refreshFeedInPlace() { async function refreshFeedInPlace() {

33
frontend/src/routes/upload/+page.svelte
View File

@@ -105,24 +105,9 @@
vibrate(10); vibrate(10);
const hashtagsString = captionTags.join(','); const hashtagsString = captionTags.join(',');
let full = 0; let full = 0;
let failed = 0; for (const sf of stagedFiles) {
try { const result = await addToQueue(sf.file, caption, hashtagsString);
for (const sf of stagedFiles) { if (result === 'full') full++;
const result = await addToQueue(sf.file, caption, hashtagsString);
if (result === 'full') full++;
else if (result === 'failed') failed++;
}
} catch (e) {
// `addToQueue` reports its own storage failures as 'failed', so reaching here means
// something genuinely unexpected. Treat the whole batch as not-queued rather than
// navigating away from photos that were never persisted.
console.error('staging failed', e);
failed = stagedFiles.length;
} finally {
// Must be released on EVERY path. Without this the submit button — disabled on
// `submitting` — stayed stuck on "Wird hochgeladen…" forever after any throw, with
// no error shown and no way forward.
submitting = false;
} }
// Don't let a full queue silently swallow photos the user thinks were queued. // Don't let a full queue silently swallow photos the user thinks were queued.
if (full > 0) { if (full > 0) {
@@ -132,18 +117,6 @@
6000 6000
); );
} }
// Local storage is unusable (private mode, blocked upgrade, quota). Keep the staged
// files ON SCREEN and stay on the page — navigating away would destroy in-app camera
// captures that exist nowhere else. Re-submitting is safe and is not a double-queue:
// `addToQueue` dedups on name+size+lastModified+userId while an item is pending.
if (failed > 0) {
toast(
`${failed} ${failed === 1 ? 'Foto konnte' : 'Fotos konnten'} nicht gespeichert werden. Bitte Speicherplatz prüfen und erneut versuchen.`,
'error',
6000
);
return;
}
clearPending(); clearPending();
goto('/feed'); goto('/feed');
} }