Files
EventSnap/docs/SECURITY-BACKLOG.md
fabi 498b4e256b fix(security): critical — repair PIN reset + enforce real prod secrets
C1: reset_user_pin wrote to a non-existent column (pin_failed_attempts);
    the real column is failed_pin_attempts, so every PIN reset 500'd. Fixed
    the column name; new e2e (pin-reset.spec.ts) proves a reset returns a
    usable PIN and the target can recover with it.

C2: config.rs::validate_secrets now rejects placeholder-ish secrets
    (change_me/dev_secret/placeholder), enforces len>=32 in prod, and
    requires a real ADMIN_PASSWORD_HASH. docker-compose.yml sets
    APP_ENV=production so the guard actually runs. Corrected the false
    "fixed" claim in SECURITY-BACKLOG.md. .env.example documents the rule.

Riders in these files (documented here since git can't split hunks):
- host.rs also carries the event-scoped ban_user fix and the
  close_event/open_event no-op broadcast guard (medium).
- docker-compose.yml also adds ORIGIN (H6), app/frontend healthchecks with
  Caddy waiting on health, and per-service memory limits (medium).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 21:25:39 +02:00

6.4 KiB
Raw Blame History

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).

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; 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 reactivityMostly 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 separationOnly 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 guardStill 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): server-side MIME/ext allowlist on upload, recovery-PIN lockout backoff, DB port no longer publicly exposed, bcrypt offloaded via spawn_blocking, bounded compression concurrency (semaphore), bounded feed queries (LIMIT ≤ 100), and the viewport-fit / reduced-motion / aria a11y pass. An image-decode decompression-bomb cap (image::Limits 12000×12000 / 256 MiB) lives in backend/src/services/compression.rs.

Fixed in the 2026-07 review pass (previously claimed fixed here but were not — corrected):

  • Effective JWT production-secret guardAPP_ENV=production is now set for the app service, and config.rs::validate_secrets rejects any placeholder-ish JWT_SECRET/ADMIN_PASSWORD_HASH (not just the exact dev sentinel) and enforces len ≥ 32 unconditionally in prod.
  • Unspoofable client IP in the rate limiterclient_ip now takes the right-most X-Forwarded-For entry (the hop Caddy appends), so a client-supplied left-most value is ignored.
  • Live role/ban re-check — the auth extractor re-reads the user row and trusts the DB role and is_banned flag rather than the JWT claim, revoking demoted/banned sessions immediately.
  • Container healthchecksapp and frontend now have healthchecks and Caddy waits on service_healthy.
  • Event-scoped ban_user — the ban UPDATE is now scoped by event_id like its siblings.

🅲 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.

  • 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.