d9025f783a
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>
89 lines
5.6 KiB
Markdown
89 lines
5.6 KiB
Markdown
# 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.
|