8 Commits

Author SHA1 Message Date
MechaCat02
9b90929269 test(e2e): re-point the PIN lockout specs at the property that matters
Three specs asserted the OLD policy — that three wrong PINs lock an account —
which is exactly the behaviour the previous commit removed, because that threshold
sat below the per-(IP, name) throttle ceiling and so let any single IP lock any
guest whose display name is readable off the feed.

Rewritten to assert the distinction the fix introduces, which a status code alone
cannot show: both tiers answer 429, but only the account lock costs the VICTIM.
The new specs read the row via db.isPinLocked rather than the response, so:

- one IP hammering /recover is throttled and the account stays UNLOCKED;
- a distributed guesser (counter preloaded via db.setFailedPinAttempts, since no
  single source can reach the threshold any more) still trips the lock, and it
  holds even against the correct PIN;
- concurrent wrong PINs are all counted — the atomicity property the old parallel
  test was really about, now asserted on the counter instead of inferred from a
  429 that the throttle could equally have produced.

The UI spec asserts the user-visible half: after four wrong PINs Dave can still
get into his own account. It also now types the PIN digit by digit rather than
filling and clicking, because the 4th digit auto-submits (pin-auto-submit.spec.ts)
and doing both raced the button's disabled state.

The adversarial spec enables rate_limits_enabled for its own run — it is off by
default in this environment, so without that the throttle tier would silently not
be exercised — and restores it in afterEach so it cannot leak into other specs
sharing the stack.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 23:58:17 +02:00
MechaCat02
117e67fa80 fix(auth,upload): close the admin lockout and four unbounded-input paths
ADMIN LOCKOUT. admin_login looked its user up BY NAME. Migration 007 makes
display_name unique per event case-insensitively and join had no reserved-name
guard, so any guest joining as "admin"/"Admin"/"ADMIN" before the operator's first
login made find(role == Admin) miss, the fallback create("Admin") violate that
index, and `?` return a 500 — permanently, with no in-app recovery. Moderation,
config and gallery release all gone; the fix was hand-editing the database.

The root cause is the lookup key, not the creation. The name was never the
identity. User::find_admin_for_event resolves by role, which makes the whole class
of name collisions irrelevant — including the homoglyph bypasses of the new
reserved-name list, which is now defence in depth rather than the control.

Promoting the squatting row would be the obvious fix and is a serious mistake: it
carries a recovery_pin_hash the guest knows, so it would hand them the admin
dashboard via /recover, permanently, through a path needing no password. A
separate row under a fallback name is worse UX and much better security. Verified
against the real schema — the guest keeps their uploads, PIN and session under a
freed name, and the role lookup then finds exactly one admin.

Second, independent bug in that block: create() followed by a SEPARATE UPDATE ...
SET role = 'admin' manufactures the same poisoned state if anything fails between
them. Collapsed into create_with_role.

UNBOUNDED INPUTS — one root cause, four places: validation ran after the
allocation.
- upload caption/hashtags used Field::text(), which buffers the whole field, on
  the one route whose DefaultBodyLimit is 576 MiB — so 576 MiB of heap per
  concurrent request in a 1 GiB container, with the length check running
  afterwards on a string already built. Now refused mid-read.
- the hashtag CSV was never length-checked at all and was upserted tag by tag
  INSIDE the commit transaction, which holds FOR SHARE on the event row — one
  request could stall every other upload behind tens of thousands of round trips.
  Capped at 30 tags of <=50 chars.
- /recover and /recover/request built rate-limiter keys by format!() from an
  unvalidated, unbounded display name, retained up to 24h in a map pruned hourly:
  the limiter itself became the memory-exhaustion primitive it exists to prevent.
  join validated first; that check is now shared by all three. /recover/request
  also had no per-IP ceiling at all — /join got one in 017, /recover in 019, and
  019's own comment describes exactly this attack. It returns 204 rather than 400
  on a bad name, because a 400 would be a new signal on an endpoint whose contract
  is that it cannot enumerate guests.
- the SSE ticket store had no size cap, no per-session cap and no rate limit on
  its endpoint, while prune ran hourly against a 30s TTL. Now pruned on issue,
  capped, and rate-limited. At capacity it REFUSES rather than evicting a
  stranger's ticket — evicting would let one client deny SSE to the venue. Not
  one-ticket-per-session either: two tabs open their EventSources concurrently.

PATCH /upload/{id} had no rate limit, no validation, and called
invalidate_and_arm unconditionally — outside both `if let Some` guards. So
PATCH {} bumped export_epoch and armed a fresh pair of full-gallery export workers
every call; REGEN_DEBOUNCE bounds the rate of that, not the total work, so a guest
could keep the keepsake permanently un-downloadable. All three fixed. The
validation also resolves a divergence: upload normalised tags while edit stored
them raw, so #Party via edit and party via upload became two hashtag rows.

PIN LOCKOUT was an ordering bug before a policy one: the account-lock threshold
(3) sat BELOW the per-(IP, name) ceiling (5), so three requests from one IP locked
any guest whose name is on the feed, every 15 minutes, forever. The tier meant to
protect a guest was the cheapest way to attack them. Ceiling drops to 4, threshold
rises to 12, so locking a victim now needs at least three distinct sources.
Brute-force cost is unchanged — 48 attempts/hour means 10k PINs still take ~208h
regardless of IP count — and increment_failed_pin now decays the streak after 15
minutes, since the counter previously only cleared on success and honest typos
accumulated across days. Both invariants are pinned by tests rather than comments.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 23:09:20 +02:00
MechaCat02
f275de5c8f perf(feed): stop every feed page from aggregating the whole event
v_feed computed like_count/comment_count with LEFT JOINs and a GROUP BY. Postgres
CAN push `event_id = $1` and the keyset predicate through the view — verified with
EXPLAIN, it uses idx_upload_event_created_id — but it CANNOT push ORDER BY ...
LIMIT across a GroupAggregate. So every request aggregated every upload in the
event, times its likes and comments, and only then sorted and took 21 rows. Cost
grew with the event, not with the page.

Measured on a throwaway database seeded to a real reception (1000 uploads, 100
guests, 27k likes, 10k comments), same query, same data:

  before   GroupAggregate (actual rows=1001) -> Sort -> Limit    449 ms
  after    Index Scan (actual rows=21) -> Limit -> SubPlans      0.58 ms

migration 022 replaces the joins with correlated scalar subqueries, which puts the
counts ABOVE the Limit so they run 21 times instead of 1001. Exactly equivalent,
not merely close: "like" is keyed (upload_id, user_id) so COUNT(DISTINCT user_id)
== count(*), comment.id is the PK so COUNT(DISTINCT c.id) == count(*), and the
GROUP BY was on u.id so it was already one row per upload. Column names, order and
types are unchanged, so no Rust changes. No new index needed — idx_like_upload and
idx_comment_upload already match the subqueries.

Note the existing load harness cannot see any of this: e2e/loadtest/driver.mjs
creates no likes and no comments, so the expensive path had never been exercised.

The amplifier, feed/+page.svelte: every open feed subscribed to `upload-processed`
and refetched page 1 — the most expensive page — so ~100 open feeds each fired one
per completed upload. Now gated on whether this client actually shows the card
that changed, and the debounce is jittered, because a fixed delay just moves a
simultaneous herd 800 ms later. Nothing is lost by skipping: a client without the
card also missed its `new-upload`, and the reconnect `feed-delta` already
schedules a refresh.

Load shedding, because the above reduces the risk rather than removing it: db.rs
set no acquire_timeout, so sqlx's 30 s default applied — longer than the
frontend's own 20 s fetch timeout, meaning the browser gave up while the server
kept holding the slot and the work was done for nobody. Now 5 s, and PoolTimedOut
maps to 503 + Retry-After instead of a generic 500. That mattered because the
upload queue classifies 5xx as transient and retries: a 500 sent the retries
straight back into the saturated pool with nothing to pace them. PoolClosed stays
Internal — it only occurs during shutdown, where a 503 would invite a retry
against a server that is going away.

The Retry-After extraction in into_response matches on variants, so unlike
message() a missing arm is not a compile error — it would silently drop the
header. Pinned by a test covering both retry-carrying variants.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 23:01:42 +02:00
MechaCat02
3f0f9c098b fix(upload-queue): rehydrate the persisted queue app-wide, not only on /upload
loadQueue() had exactly one call site in the entire frontend: 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. The photo never leaves the phone and the guest is
never told.

The root cause was narrower than "loadQueue isn't called enough".
requeueRetriable() read IndexedDB but only .map()'d over whatever the in-memory
store already held, so it could reset statuses and never ADD an entry — and
processQueue reads only that store. That is why the `online` listener and the SSE
resume hooks, which both call it, could not recover a cold start either. It now
REBUILDS the store from IndexedDB, which makes all three resume paths work.

Rebuilding needs one guard: entryToQueueItem downgrades `uploading` to `pending`
with progress 0, and this runs on every `online` event and every SSE reconnect,
so a blind rebuild would visibly reset the progress bar of a request still on the
wire. In-flight items are carried over by id.

Hydration is module-level, SSR-guarded and idempotent, re-armed via
onSetAuth/onClearAuth because login is a client-side goto() — no module
re-import, no onMount re-run — so a hydration that no-oped for lack of a token
gets a second chance. Module level rather than a layout onMount because
+layout.svelte already imports this module on every entry point, it matches the
file's own bindOnline()/bindSse() pattern, and a store owning its own persistence
keeps the layout free of a concern it cannot test. auth.ts does not import this
module, so no cycle.

The burst-queue e2e test no longer navigates to /upload after its reload — it now
asserts the resume happens wherever the reload lands, which is the actual
regression guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 22:58:26 +02:00
MechaCat02
e52b2f1cd1 fix(upload): reclaim the bytes of uploads that never finish
Reclaim was a dozen explicit remove_file calls on the handler's return paths.
That covers every way the handler can FINISH and none of the ways it can simply
STOP. When a client disconnects mid-body — a phone leaving wifi, iOS killing a
backgrounded PWA, the user hitting back — axum drops the handler future at an
.await inside field.chunk() and no return path runs at all. The partial file then
survives forever: it has no upload row, so cleanup_deleted_media (row-driven)
can never see it, and no sweeper covered the originals directory. The triggers
are routine rather than adversarial, and the client keeps the blob and auto-
retries on every `online` event, so one large video over bad wifi leaves several
copies.

Those bytes are also invisible to the quota while still consuming the free disk
that compute_storage_quota divides among guests — so orphans silently shrink
every guest's ceiling while the admin widget under-reports. All three volumes
share one filesystem; the end state is Postgres unable to write WAL.

TempFileGuard is an RAII guard, because dropping the future is exactly what runs
Drop — it is the only construct that survives cancellation. Armed before the file
can exist, disarmed only after tx.commit() succeeds. The twelve explicit cleanups
are deleted so one owner holds the rule.

The subtler half is the rename. It happens BEFORE the commit, so between them the
file exists under its final name with no row pointing at it — an orphan that
looks legitimate. The guard is RETARGETED there rather than disarmed, and the
retarget sits on the same poll as the rename with no .await between, which is
what makes that window uncancellable.

sweep_orphan_originals is the backstop for the process that was killed, where no
Drop can run at all. Hourly, alongside the existing sweeps: .tmp files past the
window go unconditionally (a .tmp never has a row by construction), other files
are batched 500 at a time through a single NOT EXISTS query.

Two things that look like oversights and are not, both commented in place:
- the 6h window is what makes the sweep safe against the rename-before-commit
  ordering, since a committing upload is briefly indistinguishable from an
  orphan. It must not be shortened to speed up a test.
- the NOT EXISTS deliberately does NOT filter deleted_at IS NULL. A soft-deleted
  row still points at its file during its retention window, and reclaiming that
  is cleanup_deleted_media's job; filtering here would race the two sweeps and
  destroy the files the recovery window exists to preserve.

Verified against the real schema: given a live original, a soft-deleted one and a
true orphan, the query returns only the orphan.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 22:56:26 +02:00
MechaCat02
5969ec74ea fix(compression): bound derivative retries so one bad upload can't loop forever
The OOM in the previous commit was survivable; what made it an outage was that it
repeated. The upload row is committed before compression starts, derivatives_rev
defaults to 0, and set_derivatives_rev only runs on success — so a row whose
processing killed the container survived at rev 0, and backfill_stale_derivatives
(called unconditionally at every boot) re-selected it and re-ran the identical
workload. With restart: unless-stopped that is an infinite kill loop, and every
cycle also drops every SSE stream and truncates every in-flight upload.

Verified end to end against the real schema in a scratch database: with the new
guard the backfill selects the row on boots 1-3 and zero rows from boot 4 on,
and a later success resets the counter.

migration 021 adds derivative_attempts and derivative_last_error.

The counter is incremented WRITE-AHEAD, before the work is attempted. This is the
whole design: the failure being bounded is a cgroup SIGKILL, so no Err is
returned, no error handler runs and no Drop fires. A counter bumped in a failure
path increments zero times per crash and the loop would be unchanged.
set_derivatives_rev clears it, so success is the only reset and both the live
path and the backfill get it without a new call site to forget.

Also in the backfill:
- one task walking the rows sequentially instead of one task per row. A large
  backlog used to spawn thousands of tasks, each holding a pool handle and
  queueing on the same two permits, competing with live uploads for a whole boot.
- LIMIT 200 per boot, and original_path <> '' replacing an IS NOT NULL that was
  dead (the column is NOT NULL; cleanup_deleted_media blanks it instead).
- a once-per-boot error log naming how many uploads have given up. Without it the
  give-up is invisible — the loop stops, which is the point, but the photos keep
  a stale derivative forever with nothing to notice.

Adds backfill_video_posters for the mirror-image gap: a video interrupted by a
restart has its compression_status flipped processing -> failed by
startup_recovery and is never re-enqueued, so thumbnail_path stays NULL for the
rest of the event while the clip itself plays fine. It shares the same attempt
budget, which means a genuinely posterless sub-second clip (Live Photo, mis-tap)
stops being re-ffmpeg'd after three boots. That is intended, not a bug to fix
later — Ok(false) is a normal permanent outcome there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 22:52:48 +02:00
MechaCat02
a4bac03628 fix(compression): stop a single large PNG from OOM-killing the container
An 8000x8000 RGBA PNG passes admission — 256,000,000 bytes is just under the
256 MiB max_alloc, and smooth content is under 3 MB on disk, far below any size
cap. Processing it peaked at ~1250 MiB inside a 1 GiB cgroup. Measured, not
argued: the new (ignored) test builds exactly that image and reads VmHWM around
the pipeline, resetting the watermark via /proc/self/clear_refs so the number
covers only the code under test.

Three independent causes, all of which had to go:

1. The decode outlived everything. `resize` takes &self, and the no-downscale
   arm bound `img` into `display`, so the ~244 MiB buffer was still alive when
   oxipng ran — and oxipng decodes the PNG *again*, holding a full-size buffer
   per filter trial. The decode now lives in a block that yields the display
   derivative; the else arm moves `img` out, which is what makes "the block's
   value is the only survivor" true in both arms.

2. oxipng was unbounded in every dimension: preset 2 with timeout: None, and the
   default features pull in rayon, which evaluates filter trials concurrently
   with a full-size buffer each and has no Options knob to cap it. Now gated at
   8 MP, given a 20 s timeout, and built with default-features = false so
   oxipng's own sequential shim is used. "filetime" is kept — without it
   preserve_attrs silently no-ops. Dropping "binary" also stops compiling
   clap/glob/env_logger (a CLI's deps) into the server image.

3. Even with those fixed it still measured 516 MiB, and compression_concurrency
   defaults to 2 — so two guests uploading big photos at once was another OOM,
   1032 MiB against a 1 GiB limit. The cost is dominated by image's Lanczos3
   resize, which accumulates in f32: the intermediate is new_width * old_height
   * 16 bytes, i.e. 262 MiB for this image — larger than the decode itself, and
   invisible to max_alloc. Two changes: the preview now derives from the 2048px
   display instead of re-resizing the original (one full-size pass, not two),
   and a job whose header-estimated peak exceeds 150 MiB takes an exclusive
   permit so two giants can never overlap. Ordinary photos (a 12 MP JPEG
   estimates ~50 MiB) never touch that permit, so throughput is unchanged for
   everything except the case that must not run in parallel.

Chaining 8000 -> 2048 -> 800 for the preview is not a quality trade: a staged
Lanczos3 downscale is standard for large ratios and is visually
indistinguishable at 800px.

The blocking half is now a free function so its memory behaviour is testable —
the fix is a scoping property a future edit could silently undo.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 22:42:03 +02:00
MechaCat02
6fd75adb27 fix(ops): bound logs, drain ffmpeg's stderr, and stop three silent hangs
Six independent operational defects, none of which needed a new feature to fix.

Log rotation. Docker's json-file driver is unbounded by default, and those files
land on the HOST filesystem — outside every deploy.resources.limits in the compose
file, and on the same disk as postgres_data and media_data. A full disk stops
Postgres writing WAL, which takes the event down. Capped at 10m x 3 per service.

Log level. RUST_LOG was set in neither .env.example nor docker-compose.yml, so the
code fallback WAS the production level — and it was `debug`, with tower_http=debug
emitting a line per request and per response into that unrotated file. Now info,
with tower_http=warn to state that those spans are diagnostics, not an access log.

ffmpeg pipe deadlock. run_ffmpeg piped stdout and stderr and then called wait(),
which drains neither. Once the ~64 KiB pipe buffer filled, ffmpeg blocked writing
and wait() never returned — burning the full 120s timeout, twice per seek position,
three times per compression attempt. And the timeout is an Err, so the end state was
a soft-deleted upload: a guest's playable video destroyed by a poster-frame failure.
Now stdout is null (nothing ever read it) and stderr is drained by wait_with_output,
whose tail is logged on a non-zero exit. Note wait_with_output consumes the child, so
the old kill-on-timeout is gone; kill_on_drop(true) already covers it.

Readiness probe. /health never touched the pool, so the disk-full endgame above
stayed green all the way down. Adds /health/ready (SELECT 1 under 2s) as a SECOND
route — the compose healthcheck deliberately keeps pointing at /health, because
caddy gates its startup on it and a DB-dependent probe would turn a Postgres blip
into the reverse proxy refusing to start.

api.ts request timeout. The abort timer was cleared in a finally around fetch(),
which resolves on the response HEAD — leaving res.text() uncovered and no longer
abortable. An upstream that sends headers then stalls the body hung the call
forever. The timer now lives until the body is read, including the 204 path (which
otherwise leaked a live 20s timer per no-content request).

Upload XHR watchdog. The XHR had no timeout while processQueue held the
isProcessing latch across it; on a half-open socket neither error nor abort ever
fires, so the latch pinned and the queue wedged. Bounds SILENCE rather than total
duration — a 500 MB video over a venue uplink legitimately runs 30+ minutes while
making steady progress. Rejects as NetworkError, which is already the retryable
branch, so a stalled upload now recovers like any network blip.

IndexedDB failures. addToQueue called getDb() unguarded and handleSubmit had no
catch, so a private-mode refusal or a QuotaExceededError on a large blob left a
permanent "Wird hochgeladen…" spinner, no toast, and — for an in-app camera
capture — the only copy of the photo gone. Now reported as 'failed', which keeps
the staged files on screen and stays on the page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 22:25:23 +02:00
35 changed files with 2425 additions and 492 deletions

View File

@@ -32,6 +32,13 @@ POSTGRES_DB=eventsnap
# OOM in Postgres doesn't degrade one feature, it takes the whole event down.
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 ────────────────────────────────────────────────────────────
# Generate with: openssl rand -hex 64
JWT_SECRET=change_me_to_a_random_64_byte_hex_string

192
backend/Cargo.lock generated
View File

@@ -65,56 +65,6 @@ dependencies = [
"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]]
name = "anyhow"
version = "1.0.102"
@@ -554,46 +504,12 @@ dependencies = [
"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]]
name = "color_quant"
version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3d7b894f5411737b7867f4827955924d7c254fc9f4d91a6aad6b097804b1018b"
[[package]]
name = "colorchoice"
version = "1.0.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570"
[[package]]
name = "compression-codecs"
version = "0.4.37"
@@ -677,15 +593,6 @@ dependencies = [
"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]]
name = "crossbeam-deque"
version = "0.8.6"
@@ -816,27 +723,6 @@ dependencies = [
"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]]
name = "equator"
version = "0.4.2"
@@ -1229,12 +1115,6 @@ dependencies = [
"weezl",
]
[[package]]
name = "glob"
version = "0.3.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280"
[[package]]
name = "governor"
version = "0.6.3"
@@ -1622,7 +1502,6 @@ checksum = "7714e70437a7dc3ac8eb7e6f8df75fd8eb422675fc7678aff7364301092b1017"
dependencies = [
"equivalent",
"hashbrown 0.16.1",
"rayon",
"serde",
"serde_core",
]
@@ -1656,12 +1535,6 @@ dependencies = [
"syn",
]
[[package]]
name = "is_terminal_polyfill"
version = "1.70.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695"
[[package]]
name = "itertools"
version = "0.14.0"
@@ -1795,12 +1668,6 @@ dependencies = [
"vcpkg",
]
[[package]]
name = "linux-raw-sys"
version = "0.12.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53"
[[package]]
name = "litemap"
version = "0.8.1"
@@ -2089,12 +1956,6 @@ version = "1.21.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50"
[[package]]
name = "once_cell_polyfill"
version = "1.70.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe"
[[package]]
name = "oxipng"
version = "9.1.5"
@@ -2102,18 +1963,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "26c613f0f566526a647c7473f6a8556dbce22c91b13485ee4b4ec7ab648e4973"
dependencies = [
"bitvec",
"clap",
"crossbeam-channel",
"env_logger",
"filetime",
"glob",
"indexmap",
"libdeflater",
"log",
"rayon",
"rgb",
"rustc-hash",
"zopfli",
]
[[package]]
@@ -2607,19 +2462,6 @@ version = "2.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
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]]
name = "rustversion"
version = "1.0.22"
@@ -3060,12 +2902,6 @@ dependencies = [
"unicode-properties",
]
[[package]]
name = "strsim"
version = "0.11.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f"
[[package]]
name = "subtle"
version = "2.6.1"
@@ -3120,16 +2956,6 @@ version = "1.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
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]]
name = "thiserror"
version = "1.0.69"
@@ -3505,12 +3331,6 @@ version = "1.0.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be"
[[package]]
name = "utf8parse"
version = "0.2.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821"
[[package]]
name = "uuid"
version = "1.23.0"
@@ -4187,18 +4007,6 @@ version = "1.0.21"
source = "registry+https://github.com/rust-lang/crates.io-index"
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]]
name = "zstd"
version = "0.13.3"

View File

@@ -27,7 +27,17 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] }
dotenvy = "0.15"
sysinfo = "0.32"
image = "0.25"
oxipng = "9"
# default-features = false drops "parallel", which is what actually bounds oxipng's memory:
# 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"] }
include_dir = "0.7"
infer = "0.15"

View File

@@ -0,0 +1,3 @@
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;

View File

@@ -0,0 +1,23 @@
-- 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;

View File

@@ -0,0 +1,26 @@
-- 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;

View File

@@ -0,0 +1,55 @@
-- 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;

View File

@@ -0,0 +1,11 @@
-- 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;

View File

@@ -0,0 +1,48 @@
-- 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;

View File

@@ -19,6 +19,45 @@ use crate::services::config;
use crate::services::rate_limiter::client_ip;
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)]
pub struct JoinRequest {
pub display_name: String,
@@ -62,19 +101,13 @@ pub async fn join(
}
}
let display_name = body.display_name.trim();
let name_chars = display_name.chars().count();
if name_chars == 0 || name_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 display_name.contains('\0') {
return Err(AppError::BadRequest(
"Name enthält ungültige Zeichen.".into(),
));
let display_name = validate_display_name(&body.display_name)?;
if is_reserved_display_name(display_name) {
// 409, matching the name-taken response below, so the frontend's existing handling
// works unchanged. See RESERVED_DISPLAY_NAMES for why this exists.
return Err(AppError::Conflict(format!(
"Der Name \"{display_name}\" ist reserviert. Bitte wähle einen anderen."
)));
}
// Per-guest bucket, keyed like the `recover:{ip}:{name}` limiter below. This carries
@@ -151,6 +184,28 @@ 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)]
pub struct RecoverRequest {
pub display_name: String,
@@ -203,13 +258,15 @@ pub async fn recover(
headers: HeaderMap,
Json(body): Json<RecoverRequest>,
) -> Result<Json<RecoverResponse>, AppError> {
let display_name = body.display_name.trim();
// Validated BEFORE it is used as a rate-limiter key — see `validate_display_name`. The
// 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 3-strike counter. Without this
// an attacker who knows a display name (they're visible on the feed) can
// burn through 3 wrong PINs and lock the victim for 15 minutes — repeated
// every 15 minutes, indefinitely. 5 attempts per 15 minutes per (IP, name)
// softens that into a real cost.
// Per-IP+name throttle BEFORE the per-user lockout counter. Without this an attacker who
// knows a display name (they're visible on the feed) can burn through the victim's wrong-PIN
// budget and lock them out, repeatedly. The ceiling here MUST stay below
// PIN_LOCK_THRESHOLD — see the constant for why that ordering is the whole control.
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 recover_rate_on = config::get_bool(&state.config_cache, "recover_rate_enabled", true).await;
@@ -234,10 +291,16 @@ 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();
if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("recover:{ip}:{name_key}"),
5,
name_ceiling,
Duration::from_secs(15 * 60),
) {
return Err(AppError::TooManyRequests(
@@ -317,7 +380,7 @@ pub async fn recover(
attempts,
"recover: wrong PIN"
);
if attempts >= 3 {
if attempts >= PIN_LOCK_THRESHOLD {
let lockout = Utc::now() + chrono::Duration::minutes(15);
User::lock_pin(&state.pool, user.id, lockout).await?;
tracing::warn!(
@@ -400,27 +463,16 @@ pub async fn admin_login(
)
.await?;
// Find or create the admin user for this event
let admin_name = "Admin";
let users = User::find_by_event_and_name(&state.pool, event.id, admin_name).await?;
let admin_user = if let Some(u) = users.into_iter().find(|u| u.role == UserRole::Admin) {
u
} else {
// Admin authenticates via password, but the schema still requires a PIN
// hash. Generate a random unguessable PIN so the recovery path remains
// unusable as an escalation route even if the role flag ever got 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.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")))?
// Find or create the admin user for this event — BY ROLE, never by name.
//
// The name lookup this replaces is what made admin login brickable. Migration 007 makes
// display_name unique per event case-insensitively and `join` had no reserved-name guard,
// so a guest joining as "admin" before the operator's first login made the lookup miss on
// role, the fallback `create("Admin")` violate that index, and `?` return a permanent 500 —
// taking out moderation, config and gallery release with no in-app recovery.
let admin_user = match User::find_admin_for_event(&state.pool, event.id).await? {
Some(u) => u,
None => create_admin_user(&state, event.id).await?,
};
tracing::info!(user_id = %admin_user.id, event_id = %event.id, ip = %ip, "admin_login: success");
@@ -445,6 +497,51 @@ 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> {
Session::delete_by_token_hash(&state.pool, &auth.token_hash).await?;
Ok(StatusCode::NO_CONTENT)
@@ -476,9 +573,38 @@ pub async fn request_pin_reset(
headers: HeaderMap,
Json(body): Json<PinResetRequestBody>,
) -> Result<StatusCode, AppError> {
let display_name = body.display_name.trim();
let ip = client_ip(&headers, &peer.ip().to_string());
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 {
let name_key = display_name.to_lowercase();
if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
@@ -492,9 +618,6 @@ 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
// (same event+user index scans, an INSERT that matches 0 rows for an unknown name) —
@@ -524,3 +647,62 @@ pub async fn request_pin_reset(
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());
}
}

View File

@@ -4,6 +4,15 @@ use sqlx::postgres::PgPoolOptions;
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";
@@ -54,6 +63,7 @@ pub async fn create_pool(database_url: &str) -> Result<PgPool> {
let pool = match PgPoolOptions::new()
.max_connections(max_connections)
.acquire_timeout(ACQUIRE_TIMEOUT)
.connect(database_url)
.await
{

View File

@@ -19,6 +19,12 @@ pub enum AppError {
/// the client can treat it as *terminal* (413, no retry) instead of backing off and
/// retrying a permanently-failing upload forever.
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),
}
@@ -33,6 +39,9 @@ impl AppError {
Self::Conflict(_) => (StatusCode::CONFLICT, "conflict"),
Self::TooManyRequests(..) => (StatusCode::TOO_MANY_REQUESTS, "too_many_requests"),
Self::QuotaExceeded(_) => (StatusCode::PAYLOAD_TOO_LARGE, "quota_exceeded"),
Self::ServiceUnavailable(..) => {
(StatusCode::SERVICE_UNAVAILABLE, "service_unavailable")
}
Self::Internal(_) => (StatusCode::INTERNAL_SERVER_ERROR, "internal_error"),
}
}
@@ -46,6 +55,7 @@ impl AppError {
| Self::NotFound(msg)
| Self::Conflict(msg) => msg.clone(),
Self::TooManyRequests(msg, _) => msg.clone(),
Self::ServiceUnavailable(msg, _) => msg.clone(),
Self::QuotaExceeded(msg) => msg.clone(),
Self::Internal(err) => {
tracing::error!("internal error: {err:#}");
@@ -58,10 +68,13 @@ impl AppError {
impl IntoResponse for AppError {
fn into_response(self) -> Response {
let (status, code) = self.status_and_code();
let retry_after_secs = if let Self::TooManyRequests(_, Some(secs)) = &self {
Some(*secs)
} else {
None
// BOTH retry-carrying variants must be matched here. `message()` would fail to
// compile on a missing arm; this one would not — it would silently drop the header and
// the `retry_after_secs` body field, which is exactly the sort of omission that only
// shows up under the load the 503 exists for.
let retry_after_secs = match &self {
Self::TooManyRequests(_, secs) | Self::ServiceUnavailable(_, secs) => *secs,
_ => None,
};
let message = self.message();
@@ -93,6 +106,84 @@ impl From<anyhow::Error> for AppError {
impl From<sqlx::Error> for AppError {
fn from(err: sqlx::Error) -> Self {
Self::Internal(err.into())
match err {
// 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")
);
}
}

View File

@@ -0,0 +1,43 @@
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")
}
}
}

View File

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

View File

@@ -38,7 +38,26 @@ pub async fn issue_ticket(
State(state): State<AppState>,
auth: AuthUser,
) -> Result<Json<StreamTicketResponse>, AppError> {
let ticket = state.sse_tickets.issue(auth.token_hash);
// The endpoint had no rate limit at all. Authentication is not a bound here: one valid
// 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()")
.fetch_one(&state.pool)
.await?;

View File

@@ -17,6 +17,135 @@ use crate::state::AppState;
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
/// 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
@@ -107,13 +236,18 @@ pub async fn upload(
.media_path
.join(format!("originals/{event_slug}"));
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 caption: Option<String> = None;
let mut hashtags_csv: Option<String> = None;
// Wrap the multipart read so any error after the temp file is created still cleans
// it up (a mid-stream parse failure must not leave a stray `.tmp` on disk).
// The multipart read is wrapped so the field loop can use `?` freely; reclaiming the temp
// file on failure is `file_guard`'s job, not this block's.
let parse_result: Result<(), AppError> = async {
while let Some(field) = multipart
.next_field()
@@ -143,20 +277,10 @@ pub async fn upload(
streamed = Some(stream_field_to_file(field, &temp_abs, cap_bytes).await?);
}
"caption" => {
caption = Some(
field
.text()
.await
.map_err(|e| AppError::BadRequest(e.to_string()))?,
);
caption = Some(read_text_field_bounded(field, MAX_CAPTION_BYTES).await?);
}
"hashtags" => {
hashtags_csv = Some(
field
.text()
.await
.map_err(|e| AppError::BadRequest(e.to_string()))?,
);
hashtags_csv = Some(read_text_field_bounded(field, MAX_HASHTAGS_BYTES).await?);
}
_ => {}
}
@@ -165,13 +289,10 @@ pub async fn upload(
}
.await;
if let Err(e) = parse_result {
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(e);
}
parse_result?;
// From here on the temp file may exist; every validation failure removes it before
// returning so a rejected upload never leaves bytes behind.
// From here on the temp file may exist. Every exit reclaims it via `file_guard` — see
// TempFileGuard for why the explicit per-branch cleanup this replaced was not enough.
let (size, head) = match streamed {
Some(s) => s,
None => return Err(AppError::BadRequest("Keine Datei hochgeladen.".into())),
@@ -183,7 +304,6 @@ pub async fn upload(
if let Some(ref cap) = caption
&& cap.chars().count() > MAX_CAPTION_LENGTH
{
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(AppError::BadRequest(format!(
"Beschreibung ist zu lang. Maximum: {} Zeichen.",
MAX_CAPTION_LENGTH
@@ -198,7 +318,6 @@ pub async fn upload(
let kind = match infer::get(&head) {
Some(k) => k,
None => {
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(AppError::BadRequest(
"Dateityp nicht erkannt oder nicht unterstützt.".into(),
));
@@ -211,7 +330,6 @@ pub async fn upload(
{
Some(v) => v,
None => {
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(AppError::BadRequest(format!(
"Dateityp wird nicht unterstützt: {}.",
kind.mime_type()
@@ -226,7 +344,6 @@ pub async fn upload(
max_image_mb * 1024 * 1024
};
if size > max_bytes {
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(AppError::BadRequest(format!(
"Datei ist zu groß. Maximum: {} MB.",
max_bytes / (1024 * 1024)
@@ -245,7 +362,6 @@ pub async fn upload(
%mime, megapixels = ?mp,
"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)"));
return Err(AppError::BadRequest(format!(
"Bild hat zu viele Bildpunkte{detail} und kann nicht verarbeitet werden. \
@@ -271,7 +387,6 @@ pub async fn upload(
quota_limit = Some(limit);
let prospective_total = user.total_upload_bytes.saturating_add(size);
if prospective_total > limit {
let _ = tokio::fs::remove_file(&temp_abs).await;
return Err(AppError::QuotaExceeded(
"Du hast dein Upload-Limit für dieses Event erreicht.".into(),
));
@@ -286,22 +401,20 @@ pub async fn upload(
tokio::fs::rename(&temp_abs, &absolute_path)
.await
.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
let mut tags: Vec<String> = Vec::new();
if let Some(ref cap) = caption {
tags.extend(hashtag::extract_hashtags(cap));
}
if let Some(ref csv) = hashtags_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();
// Process hashtags from caption and explicit CSV, capped — see `normalize_tags`.
let tags = normalize_tags(
caption
.as_deref()
.map(hashtag::extract_hashtags)
.unwrap_or_default(),
hashtags_csv.as_deref(),
);
// Quota accounting, the upload row, and its hashtag links must be atomic: a
// crash between the bytes increment and the insert would permanently charge
@@ -386,15 +499,10 @@ pub async fn upload(
}
.await;
// The file is already on disk at `absolute_path`. If the transaction failed, no DB
// row will ever reference it, so remove it now rather than orphan bytes on disk.
let upload = match tx_result {
Ok(u) => u,
Err(e) => {
let _ = tokio::fs::remove_file(&absolute_path).await;
return Err(e);
}
};
// The committed row now references these bytes — hand ownership over. Anything other than
// a successful commit leaves the guard armed, so the file is reclaimed on the way out.
let upload = tx_result?;
file_guard.disarm();
// Spawn compression task
state
@@ -449,6 +557,57 @@ pub async fn edit_upload(
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
// mid-relink can't leave the upload with its hashtags stripped.
//
@@ -464,7 +623,7 @@ pub async fn edit_upload(
if let Some(ref caption) = body.caption {
Upload::update_caption(&mut *tx, upload_id, Some(caption)).await?;
}
if let Some(ref hashtags) = body.hashtags {
if let Some(ref hashtags) = normalized_tags {
Hashtag::unlink_all_from_upload(&mut *tx, upload_id).await?;
for tag in hashtags {
let h = Hashtag::upsert(&mut *tx, auth.event_id, tag).await?;
@@ -1078,4 +1237,103 @@ mod tests {
fn full_tolerance_is_identity_for_a_single_uploader() {
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());
}
}
}

View File

@@ -27,8 +27,15 @@ async fn main() -> Result<()> {
tracing_subscriber::registry()
.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()
.unwrap_or_else(|_| "eventsnap_backend=debug,tower_http=debug".into()),
.unwrap_or_else(|_| "eventsnap_backend=info,tower_http=warn".into()),
)
.with(tracing_subscriber::fmt::layer())
.init();
@@ -51,6 +58,12 @@ async fn main() -> Result<()> {
// originals are never touched, so a failure just retries on the next start.
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
// (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
@@ -248,7 +261,11 @@ async fn main() -> Result<()> {
// four subtrees. Deleting the route removes the vector outright rather than racing the
// decoder; `/media/**` now 404s regardless of encoding.
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" }))
// Readiness. Touches the pool; for an external monitor, not for the compose gate.
.route("/health/ready", get(handlers::health::ready))
.merge(api)
.layer(TraceLayer::new_for_http())
.with_state(state);

View File

@@ -150,10 +150,62 @@ impl Upload {
/// 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.
///
/// 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> {
sqlx::query("UPDATE upload SET derivatives_rev = $2 WHERE id = $1")
sqlx::query(
"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(rev)
.bind(truncated)
.execute(pool)
.await?;
Ok(())

View File

@@ -60,6 +60,54 @@ impl User {
.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> {
sqlx::query_as::<_, Self>("SELECT * FROM \"user\" WHERE id = $1")
.bind(id)
@@ -96,14 +144,31 @@ impl User {
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> {
let row: (i16,) = sqlx::query_as(
"UPDATE \"user\"
SET failed_pin_attempts = failed_pin_attempts + 1
SET failed_pin_attempts = CASE
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
RETURNING failed_pin_attempts",
)
.bind(id)
.bind(Self::PIN_ATTEMPT_DECAY_MINUTES.to_string())
.fetch_one(pool)
.await?;
Ok(row.0)
@@ -124,7 +189,9 @@ impl User {
pub async fn reset_pin_attempts(pool: &PgPool, id: Uuid) -> Result<(), sqlx::Error> {
sqlx::query(
"UPDATE \"user\" SET failed_pin_attempts = 0, pin_locked_until = NULL WHERE id = $1",
"UPDATE \"user\"
SET failed_pin_attempts = 0, pin_locked_until = NULL, last_failed_pin_at = NULL
WHERE id = $1",
)
.bind(id)
.execute(pool)

View File

@@ -13,6 +13,9 @@ use crate::state::SseEvent;
#[derive(Clone)]
pub struct CompressionWorker {
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,
media_path: PathBuf,
sse_tx: broadcast::Sender<SseEvent>,
@@ -31,6 +34,7 @@ impl CompressionWorker {
) -> Self {
Self {
semaphore: Arc::new(Semaphore::new(concurrency)),
heavy: Arc::new(Semaphore::new(1)),
pool,
media_path,
sse_tx,
@@ -58,6 +62,21 @@ impl CompressionWorker {
/// next start. Rev 1 = EXIF orientation is applied.
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.
pub fn process(&self, upload_id: Uuid, original_path: String, mime_type: String) {
let worker = self.clone();
@@ -164,6 +183,21 @@ impl CompressionWorker {
let original = self.media_path.join(original_path);
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
.generate_image_derivatives(upload_id, &original, mime_type)
.await?;
@@ -205,6 +239,38 @@ impl CompressionWorker {
/// Longest edge of the phone-feed "preview" (data-saver default).
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)
/// and the 2048px `display` (diashow). Returns `(preview_rel, display_rel)`.
async fn generate_image_derivatives(
@@ -223,53 +289,28 @@ impl CompressionWorker {
let display_path = displays_dir.join(&filename);
let original = original.to_path_buf();
let mime_owned = mime_type.to_string();
let preview_max = Self::PREVIEW_MAX_EDGE;
let display_max = Self::DISPLAY_MAX_EDGE;
// Estimate the peak from the HEADER (no pixels decoded — the same kind of cheap probe
// 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
tokio::task::spawn_blocking(move || -> Result<()> {
// 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(())
tokio::task::spawn_blocking(move || {
write_image_derivatives(upload_id, &original, &mime_owned, &preview_path, &display_path)
})
.await??;
@@ -290,17 +331,29 @@ impl CompressionWorker {
///
/// 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.
///
/// 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) {
// `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)>(
"SELECT id, original_path, mime_type FROM upload
WHERE deleted_at IS NULL AND mime_type LIKE 'image/%'
AND original_path IS NOT NULL
AND original_path <> ''
AND derivative_attempts < $2
AND (
(display_path IS NULL AND preview_path IS NOT NULL)
OR derivatives_rev < $1
)",
)
ORDER BY created_at DESC
LIMIT $3",
)
.bind(Self::DERIVATIVES_REV)
.bind(Self::MAX_DERIVATIVE_ATTEMPTS)
.bind(Self::BACKFILL_BATCH)
.fetch_all(&self.pool)
.await;
let rows = match rows {
@@ -310,14 +363,32 @@ impl CompressionWorker {
return;
}
};
self.report_exhausted_derivatives().await;
if rows.is_empty() {
return;
}
tracing::info!("regenerating derivatives for {} upload(s)", rows.len());
for (id, original_path, mime_type) in rows {
let worker = self.clone();
tokio::spawn(async move {
// ONE task for the whole batch. The previous shape spawned a task per row, so a large
// backlog created thousands of live tasks that each held a pool handle and queued on
// 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;
// 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);
match worker
.generate_image_derivatives(id, &original, &mime_type)
@@ -326,6 +397,8 @@ impl CompressionWorker {
Ok((preview_rel, display_rel)) => {
let _ = Upload::set_preview_path(&worker.pool, id, &preview_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 _ =
Upload::set_derivatives_rev(&worker.pool, id, Self::DERIVATIVES_REV)
.await;
@@ -333,12 +406,133 @@ impl CompressionWorker {
}
Err(e) => {
// Leave the existing derivatives and the original intact; this row is
// simply retried on the next start. The rev stays behind, which is the
// marker that it still needs doing.
// retried on the next start until its attempt budget runs out. The rev
// stays behind, which is the marker that it still needs doing.
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"
);
}
}
@@ -362,3 +556,250 @@ impl CompressionWorker {
Ok(produced.then(|| format!("thumbnails/{thumb_filename}")))
}
}
/// The blocking half of [`CompressionWorker::generate_image_derivatives`]: decode once, write
/// both derivatives, then optionally shrink a PNG original in place.
///
/// A free function rather than an inline closure so its memory behaviour is directly testable —
/// this is the code path that OOM-killed the container, and the fix is a scoping property that a
/// future edit could silently undo.
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();
}
// Everything above is fixture setup, not the code under test.
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);
}
}

View File

@@ -88,6 +88,40 @@ fn decoder_within_budget(path: &Path) -> Result<impl image::ImageDecoder> {
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
/// to put a concrete number in the message the guest sees.
pub fn megapixels(path: &Path) -> Option<f64> {

View File

@@ -120,6 +120,19 @@ 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:
/// - deletes session rows whose `expires_at` is more than a day in the past
/// - prunes the in-memory rate-limiter HashMap of empty windows
@@ -140,6 +153,7 @@ pub fn spawn_periodic_tasks(
tick.tick().await;
cleanup_sessions(&pool).await;
cleanup_deleted_media(&pool, &media_path).await;
sweep_orphan_originals(&pool, &media_path).await;
rate_limiter.prune();
sse_tickets.prune();
}
@@ -262,3 +276,123 @@ async fn cleanup_sessions(pool: &PgPool) {
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"
);
}
}

View File

@@ -13,6 +13,16 @@ use rand::Rng;
/// stream open. Tickets are consumed on use and expire after `TTL`.
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)]
pub struct SseTicketStore {
inner: Arc<Mutex<HashMap<String, Entry>>>,
@@ -39,9 +49,47 @@ impl SseTicketStore {
}
/// 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 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(
ticket.clone(),
Entry {
@@ -49,7 +97,7 @@ impl SseTicketStore {
issued_at: Instant::now(),
},
);
ticket
Some(ticket)
}
/// Consume a ticket. Returns `Some(token_hash)` if the ticket exists and is
@@ -84,10 +132,16 @@ fn random_ticket() -> String {
mod tests {
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]
fn issue_then_consume_returns_the_hash_exactly_once() {
let store = SseTicketStore::new();
let ticket = store.issue("hash-1".into());
let ticket = issue(&store, "hash-1");
assert_eq!(store.consume(&ticket).as_deref(), Some("hash-1"));
// Single-use: a replay of the same ticket is rejected.
assert_eq!(
@@ -106,8 +160,8 @@ mod tests {
#[test]
fn issued_tickets_are_unique_and_hex() {
let store = SseTicketStore::new();
let a = store.issue("h".into());
let b = store.issue("h".into());
let a = issue(&store, "h");
let b = issue(&store, "h");
assert_ne!(a, b, "each ticket must be unique");
assert_eq!(a.len(), 48, "24 random bytes → 48 hex chars");
assert!(a.chars().all(|c| c.is_ascii_hexdigit()));
@@ -116,29 +170,104 @@ mod tests {
#[test]
fn fresh_ticket_survives_prune() {
let store = SseTicketStore::new();
let ticket = store.issue("h".into());
let ticket = issue(&store, "h");
store.prune(); // not expired → kept
assert_eq!(store.consume(&ticket).as_deref(), Some("h"));
}
#[test]
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();
/// Build an entry that is already past the TTL.
fn insert_stale(store: &SseTicketStore, key: &str, token_hash: &str) {
store.inner.lock().unwrap().insert(
stale.clone(),
key.to_string(),
Entry {
token_hash: "h".into(),
token_hash: token_hash.into(),
issued_at: Instant::now()
.checked_sub(TTL + Duration::from_secs(1))
.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!(
store.consume(&stale),
store.consume("stale-ticket"),
None,
"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"
);
}
}

View File

@@ -71,7 +71,7 @@ pub async fn extract_poster_frame(src: &Path, dest: &Path, width: u32) -> Result
/// 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 mut child = tokio::process::Command::new("ffmpeg")
let child = tokio::process::Command::new("ffmpeg")
.args([
// BEFORE -i: an input-side seek. See SEEK_POSITIONS.
"-ss",
@@ -85,24 +85,56 @@ async fn run_ffmpeg(src: &Path, dest: &Path, width: u32, seek: &str) -> Result<(
"-y",
dest.to_str().unwrap_or_default(),
])
.stdout(std::process::Stdio::piped())
// 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")?;
match tokio::time::timeout(FFMPEG_TIMEOUT, child.wait()).await {
Ok(res) => {
res.context("ffmpeg wait failed")?;
}
Err(_) => {
let _ = child.kill().await;
anyhow::bail!("ffmpeg timed out after {}s", FFMPEG_TIMEOUT.as_secs());
}
// `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::*;
@@ -137,4 +169,65 @@ mod tests {
);
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);
}
}

View File

@@ -1,7 +1,20 @@
# 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:
db:
image: postgres:16-alpine
restart: unless-stopped
logging: *default-logging
env_file: .env
environment:
POSTGRES_USER: ${POSTGRES_USER}
@@ -32,8 +45,13 @@ services:
context: ./backend
dockerfile: Dockerfile
restart: unless-stopped
logging: *default-logging
env_file: .env
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
# placeholder JWT_SECRET / ADMIN_PASSWORD_HASH.
APP_ENV: production
@@ -75,6 +93,7 @@ services:
context: ./frontend
dockerfile: Dockerfile
restart: unless-stopped
logging: *default-logging
env_file: .env
environment:
# adapter-node behind Caddy TLS needs the public origin for CSRF checks on
@@ -100,6 +119,7 @@ services:
caddy:
image: caddy:2-alpine
restart: unless-stopped
logging: *default-logging
environment:
# 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

View File

@@ -37,6 +37,52 @@ 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) {
await withClient((c) =>
c.query(`UPDATE session SET expires_at = NOW() - interval '1 hour' WHERE user_id = $1`, [

View File

@@ -0,0 +1,4 @@
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.

View File

@@ -75,7 +75,11 @@ test.describe('Auth — join flow', () => {
expect(storage.pin).toBe(original.pin);
});
test('wrong PIN three times locks the account for 15 minutes', async ({ page, guest, db }) => {
test('repeated wrong PINs are throttled without locking the guest out', async ({
page,
guest,
db,
}) => {
const dave = await guest('Dave');
await clearAllStorage(page);
@@ -85,22 +89,33 @@ test.describe('Auth — join flow', () => {
await join.submit();
await expect(join.recoveryPinInput).toBeVisible();
// Wrong PIN (real one is dave.pin)
// Wrong PIN (real one is dave.pin), four times — one more than the OLD lock threshold of 3.
// 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';
for (let i = 0; i < 3; i++) {
await join.recoveryPinInput.fill(wrong);
await join.recoverySubmit.click();
for (let i = 0; i < 4; i++) {
await join.recoveryPinInput.fill('');
await join.recoveryPinInput.pressSequentially(wrong, { delay: 30 });
await expect(join.recoveryError).toBeVisible();
await expect(join.recoverySubmit).toBeEnabled();
}
// Fourth attempt should hit the 429 lockout (even with the correct PIN now)
await join.recoveryPinInput.fill(dave.pin);
await join.recoverySubmit.click();
await expect(join.recoveryError).toContainText(/15 Minuten/);
// THE PROPERTY THIS TEST EXISTS FOR, stated the way a guest experiences it: Dave can still
// get into his own account.
//
// The lock threshold used to be 3, BELOW the per-(IP, name) ceiling — so these very
// 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);
// Sanity: DB row reflects the lock
// (The handler sets pin_locked_until directly — verify via API "recover" returning 429)
void db; // unused for now, documenting that db.lockUserPin exists if we want shortcut path
await join.recoveryPinInput.fill('');
await join.recoveryPinInput.pressSequentially(dave.pin, { delay: 30 });
await page.waitForURL('**/feed');
});
test('"Anderen Namen wählen" returns to the normal join form', async ({ page, guest }) => {

View File

@@ -199,9 +199,12 @@ test.describe('Upload — client queue under a burst', () => {
// a closed tab / killed PWA. The remaining pending items live only in
// IndexedDB now.
await page.reload();
// The queue only resumes where loadQueue() runs — the /upload route's
// onMount. Navigating there is the "reopen the composer" recovery path.
await page.goto('/upload');
// Deliberately NOT navigating to /upload. Rehydration is now module-level and
// auth-gated (upload-queue.ts `hydrateQueue`), so the queue resumes wherever the
// reload lands. This assertion is the regression guard for the defect it replaced:
// `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.
// `>=` not `===`: the only imperfection possible is a DUPLICATE (an upload

View File

@@ -89,15 +89,43 @@ test.describe('Adversarial — JWT', () => {
});
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 wrong = g.pin === '0000' ? '1111' : '0000';
// Do them serially so the failed_pin_attempts counter increments
// monotonically. Parallel attempts race and may never accumulate to 3 in
// the current handler implementation — that's a separate finding.
// Serially, so the failed-PIN counter increments monotonically. Well past the per-(IP, name)
// ceiling of 4, and past the OLD lock threshold of 3.
const statuses: number[] = [];
for (let i = 0; i < 4; i++) {
for (let i = 0; i < 8; i++) {
const r = await fetch(`${BASE}/api/v1/recover`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
@@ -105,26 +133,52 @@ test.describe('Adversarial — PIN brute-force', () => {
});
statuses.push(r.status);
}
// First three are 401, fourth (or later) is 429.
expect(statuses.filter((s) => s === 200)).toHaveLength(0);
expect(statuses.some((s) => s === 429)).toBe(true);
expect(statuses.filter((s) => s === 200), 'a wrong PIN must never authenticate').toHaveLength(
0
);
expect(statuses.some((s) => s === 429), 'the attacker must be throttled').toBe(true);
// Now even the correct PIN fails until lockout expires.
expect(
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`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
headers: { 'Content-Type': 'application/json', 'X-Forwarded-For': '203.0.113.78' },
body: JSON.stringify({ display_name: g.displayName, pin: g.pin }),
});
expect(correct.status).toBe(429);
});
test('parallel wrong-PIN attempts still lock the account (counter is not lost to the race)', async ({
guest,
}) => {
test('the wrong-PIN streak is atomic under concurrency', async ({ guest, db }) => {
const g = await guest('BruteParallel');
const wrong = g.pin === '0000' ? '1111' : '0000';
const attempts = await Promise.all(
await Promise.all(
Array.from({ length: 10 }, () =>
fetch(`${BASE}/api/v1/recover`, {
method: 'POST',
@@ -133,29 +187,14 @@ test.describe('Adversarial — PIN brute-force', () => {
})
)
);
const statuses = attempts.map((r) => r.status);
expect(
statuses.filter((s) => s === 200),
'a wrong PIN must never authenticate'
).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 }),
});
// 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(
correct.status,
'after 10 wrong PINs the account must be locked, even for the right PIN'
).toBe(429);
await db.failedPinAttempts(g.userId),
'concurrent wrong PINs must all be counted'
).toBeGreaterThan(1);
});
});

View File

@@ -27,6 +27,13 @@ async function request<T>(method: string, path: string, body?: unknown): Promise
// Abort hung requests so a dead connection surfaces as a friendly error
// 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 timer = setTimeout(() => controller.abort(), TIMEOUT_MS);
@@ -38,6 +45,26 @@ async function request<T>(method: string, path: string, body?: unknown): Promise
body: body !== undefined ? JSON.stringify(body) : undefined,
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) {
if (e instanceof DOMException && e.name === 'AbortError') {
throw new ApiError(0, 'timeout', 'Zeitüberschreitung bitte erneut versuchen.');
@@ -47,13 +74,6 @@ async function request<T>(method: string, path: string, body?: unknown): Promise
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;
if (raw) {
try {

View File

@@ -1,5 +1,10 @@
import { describe, it, expect } from 'vitest';
import { classifyUploadStatus, isReversibleLock, entryToQueueItem } from './upload-queue';
import {
classifyUploadStatus,
isReversibleLock,
entryToQueueItem,
shouldAbortForStall
} from './upload-queue';
/**
* Regression guard for the upload-queue retry policy (H2 + M1). The bug being locked out:
@@ -107,3 +112,32 @@ describe('entryToQueueItem', () => {
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);
});
});

View File

@@ -1,6 +1,6 @@
import { openDB, type IDBPDatabase } from 'idb';
import { writable, get } from 'svelte/store';
import { getToken, getUserId, clearAuth } from './auth';
import { getToken, getUserId, clearAuth, onSetAuth, onClearAuth } from './auth';
import { onSseEvent } from './sse';
import { refreshQuota } from './quota-store';
import { toast } from './toast-store';
@@ -37,6 +37,37 @@ const STORE_NAME = 'queue';
/** Hard cap on queued items per device — bounds IndexedDB growth from stuck blobs. */
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;
// Resume the queue as soon as connectivity returns. Registered once, guarded for SSR.
@@ -46,10 +77,7 @@ let onlineBound = false;
function bindOnline(): void {
if (onlineBound || typeof window === 'undefined') return;
window.addEventListener('online', () => {
void (async () => {
await requeueRetriable();
await processQueue();
})();
void loadQueue();
});
onlineBound = true;
}
@@ -69,10 +97,7 @@ let sseBound = false;
function bindSse(): void {
if (sseBound || typeof window === 'undefined') return;
const resume = () => {
void (async () => {
await requeueRetriable();
await processQueue();
})();
void loadQueue();
};
onSseEvent('event-opened', resume);
onSseEvent('feed-delta', resume);
@@ -81,28 +106,42 @@ function bindSse(): void {
bindSse();
/**
* Flip transient `error` items (5xx / a network drop that got marked before we could
* reclassify it) back to `pending` so a resume actually retries them. Terminal `blocked`
* items (403/413) are left alone — retrying those never succeeds.
* Rebuild the in-memory queue from IndexedDB, flipping transient `error` items (5xx / a
* network drop that got marked before we could reclassify it) back to `pending` so a resume
* actually retries them. Terminal `blocked` items (403/413) are left alone — retrying those
* 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> {
const database = await getDb();
const myUserId = getUserId();
const all = await database.getAll(STORE_NAME);
for (const entry of all) {
if (entry.userId === myUserId && entry.status === 'error' && entry.blob) {
// Only surface entries that belong to the current user. Entries from a previous guest on
// this device are filtered out (and are wiped on their next explicit logout via
// `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.error = undefined;
await database.put(STORE_NAME, entry);
}
}
queueItems.update((items) =>
items.map((item) =>
item.status === 'error'
? { ...item, status: 'pending' as const, progress: 0, error: undefined }
: item
)
// Preserve anything actually on the wire. `entryToQueueItem` downgrades `uploading` to
// `pending` with progress 0, so rebuilding blindly would visibly reset the progress bar of
// a request still in flight — and this runs on every `online` event and every SSE
// reconnect, not just at startup.
const inFlight = new Map(
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> {
@@ -139,8 +178,10 @@ async function getDb(): Promise<IDBPDatabase> {
* blamed for) the previous guest's pending uploads.
*/
export async function clearQueue(): Promise<void> {
const database = await getDb();
await database.clear(STORE_NAME);
// Always clear the in-memory view, even if the store is unreachable: this runs on logout,
// and leaving the previous guest's items on screen for the next one is the worse failure.
const database = await getDbSafe();
if (database) await database.clear(STORE_NAME);
queueItems.set([]);
rateLimitRetryAt.set(null);
}
@@ -265,37 +306,77 @@ 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> {
const database = await getDb();
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();
})();
await requeueRetriable();
void 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
* actually queued (deduped, or the queue is full of un-evictable in-flight items). */
export type EnqueueResult = 'queued' | 'duplicate' | 'full';
* actually queued (deduped, the queue is full of un-evictable in-flight items, or the
* local store itself is unusable). */
export type EnqueueResult = 'queued' | 'duplicate' | 'full' | 'failed';
export async function addToQueue(
file: File,
caption: string,
hashtags: string
): Promise<EnqueueResult> {
const database = await getDb();
// IndexedDB is not guaranteed available: Safari private mode refuses to open a DB, an
// 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();
// 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
@@ -348,7 +429,14 @@ export async function addToQueue(
status: 'pending',
blob: file
};
await database.put(STORE_NAME, entry);
// Persist BEFORE touching the store: a failure here (quota exceeded on a large blob) must
// 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) => [
...items,
@@ -370,8 +458,21 @@ export async function addToQueue(
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> {
const database = await getDb();
const database = await getDbSafe();
if (!database) return;
const entry = await database.get(STORE_NAME, id);
if (!entry) return;
@@ -389,13 +490,15 @@ export async function retryItem(id: string): Promise<void> {
}
export async function removeItem(id: string): Promise<void> {
const database = await getDb();
const database = await getDbSafe();
if (!database) return;
await database.delete(STORE_NAME, id);
queueItems.update((items) => items.filter((item) => item.id !== id));
}
export async function clearCompleted(): Promise<void> {
const database = await getDb();
const database = await getDbSafe();
if (!database) return;
const items = get(queueItems);
for (const item of items) {
if (item.status === 'done') {
@@ -495,7 +598,24 @@ async function uploadItem(id: string): Promise<void> {
xhr.open('POST', '/api/v1/upload');
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) => {
lastActivityAt = Date.now();
if (e.lengthComputable) {
const pct = Math.round((e.loaded / e.total) * 100);
queueItems.update((items) =>
@@ -557,9 +677,27 @@ 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('abort', () => reject(new NetworkError('Abgebrochen')));
xhr.addEventListener('abort', () =>
reject(new NetworkError(timedOut ? 'Zeitüberschreitung' : 'Abgebrochen'))
);
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

View File

@@ -273,7 +273,23 @@
// A processed upload gains preview/thumbnail URLs. Coalesce bursts (bulk
// 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.
onSseEvent('upload-processed', () => scheduleInPlaceRefresh()),
onSseEvent('upload-processed', (data) => {
// 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) => {
try {
const payload = JSON.parse(data) as { upload_id: string };
@@ -399,12 +415,21 @@
// Debounced page-1 fetch that *merges* (updates existing cards in place, prepends
// genuinely new ones) rather than replacing the array — preserves scroll and any
// 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() {
if (inPlaceRefreshTimer) return;
inPlaceRefreshTimer = setTimeout(() => {
inPlaceRefreshTimer = null;
void refreshFeedInPlace();
}, 800);
inPlaceRefreshTimer = setTimeout(
() => {
inPlaceRefreshTimer = null;
void refreshFeedInPlace();
},
IN_PLACE_REFRESH_MIN_MS + Math.random() * IN_PLACE_REFRESH_JITTER_MS
);
}
async function refreshFeedInPlace() {

View File

@@ -105,9 +105,24 @@
vibrate(10);
const hashtagsString = captionTags.join(',');
let full = 0;
for (const sf of stagedFiles) {
const result = await addToQueue(sf.file, caption, hashtagsString);
if (result === 'full') full++;
let failed = 0;
try {
for (const sf of stagedFiles) {
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.
if (full > 0) {
@@ -117,6 +132,18 @@
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();
goto('/feed');
}