Files
EventSnap/docs/SECURITY-BACKLOG.md
fabi 41ac742af8 test+docs: tighten H1 assertions, cover secret/XFF edges, document boot & SSE-ban
Re-review follow-ups (non-blocking quality items):

Tests:
- moderation H1 specs now assert exact `→ 403` instead of bare .rejects.toThrow(),
  so a spurious 500 can no longer masquerade as "revocation worked".
- config: added case-insensitivity (upper/mixed-case placeholder) and the
  len==32/31 boundary cases for validate_secrets.
- rate_limiter: added the trailing-comma empty-entry case for client_ip (must
  fall back, not return "").
  (Backend unit tests: 35 pass.)

Docs:
- README: note that with APP_ENV=production a placeholder .env makes the app
  refuse to boot and Caddy wait unhealthy — reason is in `docker compose logs app`.
- SECURITY-BACKLOG + sse.rs comment: document that a mid-session ban does not
  tear down an already-open SSE stream (new tickets are blocked; low blast radius).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-02 07:14:33 +02:00

7.0 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.
  • Mid-session ban does not tear down an already-open SSE stream. The live-ban check lives in the AuthUser extractor, but the stream authenticates via ticket→session (handlers/sse.rs::stream), so a user banned while connected keeps receiving broadcast events until their stream drops. New tickets are blocked (issue_ticket uses AuthUser), so they cannot reconnect. Low blast radius (read-only feed events, no re-subscribe); tearing live streams down would need a per-session broadcast filter. Revisit only if bans must take effect within seconds.

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.