From d9025f783af51233c312b8c1c18f5617d77b6493 Mon Sep 17 00:00:00 2001
From: fabi <fabi@mc02.dev>
Date: Tue, 30 Jun 2026 20:26:57 +0200
Subject: [PATCH] fix(security): port image-decode DoS cap + audit
 backlog/media-auth docs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

From the 2026-06-27 audit branch (fix/audit-2026-06-27-critical-medium), whose
work was ~80% absorbed into main via the batch branches. This ports the pieces
that were NOT in main:

- compression.rs: cap image decode with image::Limits (12000x12000, 256 MiB
  max_alloc) via ImageReader instead of image::open(). The upload body-size cap
  bounds the file on disk but not the *decoded* dimensions, so a small
  decompression-bomb image could OOM the box during decode/resize. Surgical port
  of the audit's decode guard only (not its image/video semaphore split).

- docs/SECURITY-BACKLOG.md: the audit's triaged backlog, reconciled against main
  (each item tagged done / open / contingent-on-signed-media). Records the still-
  open items: host moderation UI gap, owner-delete SSE broadcast, quota
  mount-detection (starts_with) + low-disk guard.

- docs/DECISION-media-auth.md: writes up the one real architectural divergence —
  main serves media unauthenticated (ServeDir + UUID-capability) while the audit
  built a signed/TTL'd gateway. Lays out the tradeoff (leaked URL = permanent vs
  ~24h access; banned-user access) and a recommendation, for a human decision.

Verified: cargo build clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 backend/src/services/compression.rs | 17 +++++-
 docs/DECISION-media-auth.md         | 79 ++++++++++++++++++++++++++
 docs/SECURITY-BACKLOG.md            | 88 +++++++++++++++++++++++++++++
 3 files changed, 182 insertions(+), 2 deletions(-)
 create mode 100644 docs/DECISION-media-auth.md
 create mode 100644 docs/SECURITY-BACKLOG.md

diff --git a/backend/src/services/compression.rs b/backend/src/services/compression.rs
index d42e990..504f278 100644
--- a/backend/src/services/compression.rs
+++ b/backend/src/services/compression.rs
@@ -93,8 +93,21 @@ impl CompressionWorker {
 
         // Run blocking image operations in a spawn_blocking task
         tokio::task::spawn_blocking(move || -> Result<()> {
-            let img = image::open(&original)
-                .context("failed to open image")?;
+            // Reject decompression bombs *before* fully decoding: the upload body
+            // cap bounds the file size on disk, but a small file can still decode to
+            // enormous dimensions (e.g. a ~1 MB image expanding to 50k×50k px →
+            // gigabytes), OOM-ing the box during decode/resize. 12000×12000 covers
+            // any real phone photo; max_alloc hard-caps the decode allocation.
+            let mut reader = image::ImageReader::open(&original)
+                .context("failed to open image")?
+                .with_guessed_format()
+                .context("failed to read image header")?;
+            let mut limits = image::Limits::default();
+            limits.max_image_width = Some(12_000);
+            limits.max_image_height = Some(12_000);
+            limits.max_alloc = Some(256 * 1024 * 1024);
+            reader.limits(limits);
+            let img = reader.decode().context("failed to decode image")?;
 
             // Resize to max 800px wide, preserving aspect ratio
             let preview = img.resize(800, 800, image::imageops::FilterType::Lanczos3);
diff --git a/docs/DECISION-media-auth.md b/docs/DECISION-media-auth.md
new file mode 100644
index 0000000..6db4208
--- /dev/null
+++ b/docs/DECISION-media-auth.md
@@ -0,0 +1,79 @@
+# Decision: media serving — unauthenticated UUID vs. signed gateway
+
+**Status:** OPEN — needs a call. Written 2026-06-30 from the 2026-06-27 audit
+(`fix/audit-2026-06-27-critical-medium`), which implemented the signed-gateway option that
+`main` did not adopt.
+
+**Scope:** how `main` serves uploaded photos/videos (originals + previews/thumbnails) to the
+`<img>`/`<video>` tags in the feed, lightbox, and diashow.
+
+---
+
+## The two models
+
+### A. Current `main` — unauthenticated, UUID-as-capability
+- `/api/v1/upload/{id}/original` — **no auth**; the unguessable upload UUID *is* the capability.
+  (Documented as intentional in `frontend/src/lib/data-mode-store.ts`.)
+- `/media/*` — static `axum` `ServeDir`, **no auth layer**, serves preview/thumbnail files by path.
+- No expiry, no signature, no per-request authorization on the bytes.
+- **Works with** plain `<img src>` (no Authorization header needed) and browser caching, at zero
+  per-request backend cost.
+
+### B. Audit branch — authenticated signed gateway
+- `/media/{kind}/{id}?sig=…` via `handlers::media::serve`.
+- HMAC signature (keyed off `jwt_secret`), **time-boxed** (~24 h, bucketed to a 1 h
+  URL-stability window so warm fetches stay cacheable).
+- Authorizes by **signature + uploader visibility**, not requester identity.
+- Still `<img>`-compatible (capability rides in the query string, not a header).
+- Cost: token mint/verify, ~2 DB queries per *cold* fetch, and the HMAC key currently reuses
+  `jwt_secret` (see "Media HMAC domain separation" in [SECURITY-BACKLOG.md](SECURITY-BACKLOG.md)).
+
+---
+
+## What actually differs (the tradeoff)
+
+| | A. UUID (main) | B. Signed gateway (audit) |
+|---|---|---|
+| Leaked URL/UUID (forwarded link, browser history, referer, logs) | **Permanent** full-res access | Access **expires** (~24 h); URL can't be re-minted |
+| Banned / departed guest | Retains **permanent** access to every original whose UUID they hold | Can't mint new URLs; can replay **held** URLs ≤ TTL only |
+| Revocation | None (UUID is forever) | Rotate the signing key → all outstanding URLs die |
+| Enumeration | Mitigated by UUIDv4 unguessability | Same, plus signature |
+| Per-request cost | Zero (static serve) | Token verify + ~2 DB queries (cold) |
+| Plumbing / failure surface | Minimal | Token mint/verify, key mgmt, cache-bucket logic |
+
+Neither model solves the fundamental `<img>`-can't-send-a-Bearer constraint: once a browser holds
+a media URL, it is replayable for that URL's lifetime. Model B simply **bounds that lifetime** and
+**adds revocability**; Model A's lifetime is *forever*.
+
+## Threat model (this deployment)
+
+Private single-box event app, ~100 guests, ~1 000 files, one event at a time. Media is **shared
+with all guests by design** — it is personal but not secret *within* the event. The real risk is
+**a URL escaping the event boundary** (a guest forwards a link; it lands in chat history, a public
+post, or server/proxy logs) granting an outsider — or a removed guest — access.
+
+## Recommendation
+
+This is a judgment call, not a clear-cut bug, so it's yours to make. My leaning:
+
+- **Adopt Model B (signed gateway)** if post-event link leakage or removed-guest access is a real
+  concern for you — it's the materially stronger posture (bounded exposure + key-rotation
+  revocation) and the implementation already exists on the audit branch. If adopted, also do the
+  cheap **HKDF domain-separation** for the HMAC key (backlog 🅱).
+- **Keep Model A (UUID)** as an *explicitly accepted risk* if you're comfortable that a leaked
+  link = permanent access at this scale. If so, two cheap hardening steps are still worth doing:
+  1. **Log hygiene** — ensure the upload UUID never lands in access logs with enough context to
+     correlate (the `TraceLayer` logs the request *path*, and `/api/v1/upload/{id}/original`
+     puts the UUID *in the path*). Confirm logs aren't shipped/retained where that matters.
+  2. **Document the decision** in `README`/`PROJECT.md` so "unauthenticated media" is a recorded
+     choice, not an oversight.
+
+**Sharpest single fact to decide on:** in Model A, a guest you *ban* keeps full-resolution access
+to every original they ever loaded, forever. In Model B, that access dies within ~24 h. If that
+asymmetry matters for your events, adopt the gateway.
+
+## If you adopt B
+The implementation lives on `origin/fix/audit-2026-06-27-critical-medium`:
+`backend/src/handlers/media.rs` (+ `media_token`), the `/media/{kind}/{id}` route in `main.rs`, and
+the feed/upload/host changes that emit signed URLs (`models/upload.rs`, `handlers/feed.rs`). It
+would need re-basing onto current `main` (which has since diverged across ~80 files).
diff --git a/docs/SECURITY-BACKLOG.md b/docs/SECURITY-BACKLOG.md
new file mode 100644
index 0000000..f50685b
--- /dev/null
+++ b/docs/SECURITY-BACKLOG.md
@@ -0,0 +1,88 @@
+# Security & Hardening Backlog
+
+Tracks the deliberately-deferred items from the 2026-06-27 security audit and its review passes.
+
+**Provenance & reconciliation.** This document was extracted from the
+`fix/audit-2026-06-27-critical-medium` branch and reconciled against `main` on 2026-06-30.
+That audit's Critical→Medium findings were **largely re-implemented into `main`** through the
+later batch branches (security-review-followups, security-review-batch-2, the UX batches) rather
+than by merging the audit branch — so `git` shows no merge, but the controls are present. Each
+item below is tagged with its **current status in `main`**:
+
+- ✅ **Done in main** — addressed (possibly via a different implementation).
+- ⬜ **Open** — still applies to `main`.
+- 🔀 **Contingent** — only relevant if `main` adopts the audit's signed-media gateway (see
+  [DECISION-media-auth.md](DECISION-media-auth.md)).
+
+> The audit branch additionally implemented an **authenticated, signed media gateway** that `main`
+> did **not** adopt — `main` serves media unauthenticated (static `ServeDir` + UUID-capability
+> `/api/v1/upload/{id}/original`). That architectural choice is written up separately in
+> [DECISION-media-auth.md](DECISION-media-auth.md); the "by-design notes" at the bottom of this
+> file describe the *audit branch's* model and apply to `main` only if that gateway is adopted.
+
+---
+
+## 🅱 Worth a tracked ticket (real, not one-liners)
+
+- ⬜ **Moderation UI gap** — the backend `DELETE /host/upload/{id}` and `DELETE /host/comment/{id}`
+  endpoints have **no frontend caller**, so a host cannot remove a *guest's* content from the UI
+  (the feed `ContextSheet` only offers delete for the viewer's *own* uploads —
+  `target.user_id === myUserId`). Still a functional hole in `main`. Needs a host-facing "remove"
+  action wired to those endpoints, gated on host/admin role.
+
+- ✅/⬜ **Feed reactivity** — *Mostly fixed in `main`.* The full-reload-on-every-SSE-event problem
+  (a single `like-update`/`new-comment`/`upload-processed` calling `loadFeed(true)` and collapsing
+  a scrolled feed) was fixed in the UX batch: `main` now patches the affected card in place
+  (`patchCount`) and debounces processing (`scheduleInPlaceRefresh`). ⬜ **Remaining sub-item:**
+  owner-deleted uploads are still not broadcast — `delete_upload` returns `204` with no
+  `upload-deleted` SSE, so other clients keep showing a deleted post until refresh (only host
+  deletes broadcast). Emit `upload-deleted` from the owner delete path too.
+
+- 🔀 **Media HMAC domain separation** — *Only applies if the signed-media gateway is adopted.* The
+  audit's signed-media tokens reuse `jwt_secret` as the HMAC key; a dedicated derived key
+  (`HKDF(jwt_secret, "media-url")`) would isolate the domains so a future change to one can't
+  weaken the other. N/A to `main` as it stands (no signed media).
+
+- ⬜ **Quota mount-detection + low-disk guard** — *Still applies to `main`.* `compute_storage_quota`
+  (`backend/src/handlers/upload.rs`) and `admin.rs` pick the disk via `starts_with`, so root (`/`)
+  is a wildcard prefix that can match the wrong device; and there's no hard min-free-space
+  precheck when the quota is disabled. Use longest-prefix match; add an unconditional 507/429 when
+  free space is critically low.
+
+## ✅ Fixed in main since the audit (for the record)
+
+These audit findings are present in `main` today (verified 2026-06-30): event-scoped social
+handlers (cross-event authz), server-side MIME/ext allowlist on upload, recovery-PIN lockout
+backoff, unspoofable client IP in the rate limiter, effective JWT production-secret guard, DB port
+no longer publicly exposed, container healthchecks, bcrypt offloaded via `spawn_blocking`,
+bounded compression concurrency (semaphore), bounded feed queries (`LIMIT ≤ 100`), and the
+viewport-fit / reduced-motion / aria a11y pass. **New since the audit:** an image-decode
+decompression-bomb cap (`image::Limits` 12000×12000 / 256 MiB) ported into
+`backend/src/services/compression.rs`.
+
+## 🅲 Consciously won't-fix at ~100-guest single-box scale
+
+Diminishing returns vs. the deployment's actual threat model. Revisit only if the scale or
+tenancy model changes.
+
+- Rate-limiter `HashMap` key LRU/cap (attacker-chosen `recover:{ip}:{name}` keys accumulate up to
+  the 24h prune ceiling) — bounded and pruned; not worth an LRU.
+- "Last host" / host↔host role-churn guard — operational, low blast radius.
+- Performance micro-indexes (`idx_like_user_upload`, comment pagination index) — current queries
+  are sub-ms at this row count.
+- Optimistic-like in-flight guard, ownership-snapshot-at-mount, assorted copy tweaks — UX polish.
+
+## By-design notes (audit branch's signed-media model — see DECISION-media-auth.md)
+
+> These describe the **audit branch's** signed-gateway model. `main` does **not** serve media this
+> way; for `main`'s actual media-access posture and the tradeoff, see
+> [DECISION-media-auth.md](DECISION-media-auth.md).
+
+- **Banned user retains ≤24h media access via already-held signed URLs.** The audit's media gateway
+  authorizes by *signature + uploader visibility*, not requester identity, and signed URLs are
+  time-boxed (24h, bucketed). A banned/revoked-session user cannot mint new URLs (every API call
+  401/403s) but can replay URLs they already hold until expiry — only for content they already saw.
+  Accepted there: tightening would require per-request identity on every `<img>` load, which the
+  `<img>`-can't-send-a-Bearer constraint precludes.
+- **Two DB queries per *cold* media fetch** (`upload` row + uploader row). Mitigated by browser
+  caching and the stable bucketed URL. Could be one JOIN if it ever shows up in profiling.