Commit Graph

3 Commits

Author SHA1 Message Date
fabi
8e906d7866 refactor(export): single epoch authority; fix upload-path TOCTOU losing photos
A deep investigative review of the export concurrency (four parallel adversarial
reviewers) found that three rounds of race fixes had been chasing the wrong bug, and
that the model itself was the root cause. This replaces the model and fixes the real
data loss.

THE ACTUAL BUG (upload path, not the state machine)
  `upload` checked `uploads_locked_at` BEFORE streaming the body — minutes, for a large
  video — then committed the row without re-checking. A release landing mid-upload let the
  row commit AFTER the export snapshot: the photo appeared in the live feed and was
  PERMANENTLY missing from the keepsake, with nothing to regenerate it. release_gallery's
  comment claims to prevent exactly this; it never did. Every prior round fixed the DB
  state machine, and the leak was upstream of it.
  Fix: re-assert the lock under `SELECT ... FOR SHARE` INSIDE the commit tx. That row lock
  conflicts with release_gallery's `UPDATE event`, so either the upload commits first (and
  the release — hence the snapshot — is ordered after it) or the release wins and the
  upload is rejected as `uploads_locked` (reversible; the client keeps the blob).
  Regression test verified NON-VACUOUS: with the fix reverted it fails, reporting accepted
  uploads missing from the keepsake.

THE MODEL (migration 014)
  "Which generation is current?" had no single answer: it was assembled from three
  separately-written pieces across two tables (`export_released_at`, a per-job
  `release_seq`, and cached `export_{zip,html}_ready` flags). Keeping them in agreement
  took ~10 hand-written guards; each review round found one more path that slipped through.
  Now: ONE monotonic `event.export_epoch`, bumped in the same UPDATE as any change to
  `export_released_at`. Each job carries a copy. Downloadable IFF
  `released AND job.epoch = event.export_epoch AND status = 'done'` — readiness DERIVED,
  never stored. A retired-epoch worker is INERT BY CONSTRUCTION: losing a race can no
  longer corrupt state, only waste work.
  Deleted: both ready-flag columns, both ready-flip blocks, `if ready { continue }`, the
  reopen seq-bump, open_event's transaction, and the cross-table claim guard.

  That last one was also UNSOUND: under READ COMMITTED, a blocked UPDATE re-evaluates its
  WHERE against the updated target row but answers subqueries on OTHER tables from the
  original snapshot — so the previous `EXISTS (SELECT ... FROM event ...)` claim guard
  could admit a claim against an already-reopened event while RETURNING the post-bump seq.
  Every guard is now same-row (`epoch = $n`), which EPQ re-evaluates correctly.

OTHER REAL DEFECTS FIXED
  - release_gallery committed the release and THEN awaited the enqueue inside the handler
    future. Axum drops that future on client disconnect → event released, uploads locked,
    ZERO export_job rows, host unable to retry ("bereits freigegeben"), restart-only
    recovery. Now one tx; workers spawn (detached) after commit.
  - No flush/fsync before the tmp→final rename. async_zip's close() never flushes the inner
    file and tokio's File drop DISCARDS write errors — so a full disk silently produced a
    truncated archive that was then marked done and published, after which the last good
    generation was pruned. Now flush + sync_all, which also surfaces ENOSPC.
  - Download read the ready flag and the file path in TWO queries; a reopen between them
    served a keepsake that should 404. Now one read through the `export_current` view.
  - `if !src.exists() { continue }` then `open()?` — a TOCTOU that turned a tolerated gap
    into a hard failure of the ENTIRE keepsake (unretryable while released). Now open-first,
    warn, and skip.
  - Recovery was flag-driven and never checked the disk: a `done` row whose archive was gone
    was a permanent dead end needing manual DB surgery. Now verifies the file and re-arms.
  - Temp files/dirs leaked on every error path (the leak is what fills the disk that
    corrupts the next archive). Now cleaned up.
  - Export archives were not event-scoped (`viewer_tmp_` already was), so a second event
    would collide on paths and one event's prune would delete another's live keepsake.
  - Host deletes after release never regenerated the keepsake — a photo removed on request
    lived on in the archive forever. Now bumps the epoch and rebuilds without it.
  - claim_job swallowed DB errors as "someone else won", stranding a job pending at 0%.
  - export_status reported retired-epoch rows (a progress bar that never moves).
  - The startup sweep's single-instance assumption is now documented, not hidden.

Verified: backend 40 tests, svelte-check 0 errors, e2e typecheck clean,
e2e 160 passed / 1 skipped on chromium-desktop (incl. 2 new regressions; the
upload-acceptance one confirmed to fail without the fix).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-14 19:48:38 +02:00
fabi
20125fb713 test(e2e): pin security assertions + real cross-user IDOR coverage
Turn "accept-both-outcomes" documentation tests — which pass whether the app is
secure or vulnerable — into assertions that pin the secure behavior, and replace
fake-UUID authz tests that 404'd before ever reaching the ownership guard with
real cross-user resources.

file-upload-attacks:
- SVG-with-<script> → pin 400 (infer returns None for text → stored-XSS defense);
  was [201,400], which accepted the vulnerable outcome.
- zero-byte → pin 400; path-traversal → pin 201 with UUID-derived storage and a
  no-path-echo check (both were [201,400]).
- Fix the application/octet-stream rationale (no "application bypass" exists — the
  handler ignores the declared type and keys off magic bytes).

authorization-deep (IDOR):
- B deleting A's comment: seed a real upload+comment as A, assert 403, assert the
  comment survives, and assert the owner (A) still gets 204 — proving the 403 is
  about identity, not a broken route. (Was a DELETE on the all-zeros UUID → 404
  before the user_id guard, so authorization was never exercised.)
- Add B-deletes-A's-upload (403, countUploads unchanged) and B-edits-A's-caption
  (403, caption intact) — the find_by_id_and_event + ownership guards.

export: pin the ZIP-download 404 (was [404,200] — a 200 is a data-exposure
regression) and split into the two real branches: not-yet-ready, and
ready-but-file-missing (new db.setExportZipReady helper).

All 21 assertions verified green against the live secure backend.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:27:22 +02:00
MechaCat02
e42d8a92a1 feat(e2e): Playwright suite — 134 tests across 9 spec areas + UA matrix
Adds an end-to-end Playwright test suite under e2e/ that spins up an
isolated docker-compose stack (Postgres :55432, Caddy :3101, backend with
EVENTSNAP_TEST_MODE=1, SvelteKit adapter-node frontend) and exercises the
SvelteKit app against the real Rust backend.

Phase 1 — happy paths covering every documented USER_JOURNEYS.md flow:
  01-auth/      join, recover, admin login, leave event, PIN lockout
  02-upload/    gallery picker (API path), rate-limit + admin toggle
  03-feed/      like/comment SSE, filters, SSE reconnect on visibility
  04-host/      event lock API, ban/unban, promote
  05-admin/     config validation, foundational authz guards, stats
  06-export/    /export status + download stub
  __smoke/      cross-UA happy-path (runs on every UA project)

Phase 2 — adversarial + browser chaos:
  07-adversarial/  XSS payloads (6 × display name path), SQLi shapes,
                   length / encoding / RTL override / NUL byte;
                   file-upload boundaries (ELF body claimed as JPEG,
                   oversize vs max_image_size_mb, zero-byte, NUL
                   filename, path-traversal, SVG-with-script);
                   JWT alg:none, signature/payload tamper, expired
                   session, PIN brute-force (serial + parallel),
                   admin password brute-force; deep authz (cross-user
                   delete, banned user across like/comment/feed-read,
                   host→admin escalation); small-scale DDoS (20× /join,
                   10MB comment body, 10 concurrent SSE).
  08-browser-chaos/ localStorage / sessionStorage / cookie purge,
                    IndexedDB drop mid-session, offline → reconnect,
                    slow-3G, 503 flakes, 429 with no retry storm,
                    multi-tab same/different user, no-JS, hostile CSS,
                    clock skew ±1h / -2d, localStorage quota exhausted.

Phase 3 — mobile gestures (runs only on chromium-mobile / Pixel 7):
  09-mobile/    touch-target ≥44px audit, env(safe-area-inset-bottom)
                structural check, long-press (FeedListCard → ContextSheet,
                quick-tap negation, click-suppression), double-tap
                (feed card like + lightbox heart-burst, via synthetic
                pointer events to bypass the first-tap-fires-click trap),
                viewport reflow (portrait/landscape/narrow/phablet),
                plus fixme stubs documenting planned gestures (swipe
                lightbox L/R, swipe-down dismiss, pull-to-refresh,
                long-press-comment).

Cross-UA matrix (chromium-engine projects run @smoke only):
  chromium-pixel7, chromium-galaxy-s22, samsung-internet (Samsung UA
  emulation on Galaxy viewport), edge-android, plus webkit-iphone,
  chrome-ios, firefox-android, firefox-desktop — the latter four need
  libavif16 on the host (Playwright dep) but the configs are in place.

Infrastructure:
  - fixtures/test.ts central test.extend (api, db, adminToken, guest,
    host, signIn). Per-test DB truncate via the dev-only POST
    /admin/__truncate route, gated by EVENTSNAP_TEST_MODE=1.
  - helpers/sse-listener.ts, helpers/upload-client.ts (Node-side
    multipart for adversarial file-upload tests + JPEG/PNG/ELF magic
    constants), helpers/touch.ts (longPress / doubleTap / swipe /
    inlineStyle / computedStyle).
  - 10 page objects covering every route + UploadSheet/Lightbox.
  - global-setup waits for /health, logs in admin, disables every
    rate-limit and quota toggle.
  - .github/workflows/e2e.yml: PR check runs chromium-desktop + the
    smoke matrix in parallel, uploads playwright-report/ and traces on
    failure.

Findings the suite surfaces as live `[finding]` warnings (not silenced):
  1. /admin/login has no rate-limit or lockout (bcrypt cost only).
  2. PIN-attempt counter races under parallel /recover requests.
  3. Zero-byte uploads pass /api/v1/upload.
  4. SVG-with-script can pass the magic-byte check (consider CSP +
     X-Content-Type-Options on /media/*).

Stack-internal docs live in e2e/README.md (UA tier table, Samsung
Internet escalation tiers A/B/C, debugging tips, roadmap).

Final tally: 134 passed / 0 failed / 9 skipped (test.fixme stubs for
not-yet-shipped gestures and one UI-upload-flow investigation).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 19:37:11 +02:00