db8823022149c38aab86dec05d24aeeea2c25e8f
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
9666d74a46 |
fix(export): close all 13 findings from the multi-agent review
A 6-lens multi-agent review (103 agents; every finding adversarially verified by three
refute-by-default skeptics) confirmed the epoch redesign is sound — the core invariant
holds — and found 13 real defects around it. All are fixed here.
The redesign's invariant was re-verified and stands: an accepted upload is always in the
keepsake, and a downloadable keepsake always reflects the most recent release.
But a WEAKER adjacent invariant did NOT hold — "a downloadable keepsake reflects the
current content" — and that is the theme of the biggest fixes.
CONTENT REMOVAL NOW ALWAYS REACHES THE KEEPSAKE
Regeneration was wired only to host deletes. Ban, unban, guest self-delete and guest
comment-delete did not trigger it — yet the export query filters `is_banned = FALSE`,
so the pipeline already agreed that content must not be there. Ban someone for abusive
content after release and every guest kept downloading an archive containing it.
All five removal paths now invalidate and rebuild. `query_comments` also gained the
banned/hidden filter it was missing entirely (the ZIP had it; the viewer did not).
Each removal is now ATOMIC with its invalidation (one tx, models made executor-generic).
Previously the delete committed in its own tx and the regeneration in a second: a dropped
handler future between them left the taken-down photo permanently downloadable, and
nothing could notice — the keepsake still looked complete and the host could no longer
even find the upload to retry.
NO MORE TAKEDOWN STORM
Every removal spawned two uncancellable full exports. A superseded worker was INERT, not
STOPPED: it still ran every ffmpeg spawn and image resize and wrote a whole archive before
discovering it had lost. Five deletes left ten workers alive, each holding a
full-gallery-sized temp, in a 1 GB container.
Now: a debounce before `claim_job` (a superseded worker fails its claim and does ZERO
work, collapsing a burst into one export), plus `update_progress` — which already ran
exactly the right liveness predicate and threw the answer away — returning it so both file
loops bail the instant they are retired. Moderating a comment no longer rebuilds the
multi-GB ZIP either: the ZIP is media-only, so it is carried forward, with prune taught to
protect any file a live row still references.
AN ESCAPE HATCH
A failed export was terminal at runtime: release_gallery refuses an already-released event,
recovery only runs at boot, and there was no retry route — the only outs were restarting the
container or reopening uploads to every guest. Added POST /host/export/rebuild. Also widened
mark_failed's guard to `status IN ('running','pending')`: when claim_job itself ERRORED, the
row was still `pending`, so the failure was never recorded and the job sat at 0% forever.
SILENT INVALIDATION
Retiring the epoch 404s the download instantly, but nothing told the clients — guests kept
seeing an enabled download button through the whole rebuild. Now broadcasts an invalidation.
And the download was a top-level <a href>: a 404 (or a 429 from the per-IP export limit that
everyone on the venue WiFi shares) NAVIGATED THE USER OUT OF THE PWA onto raw JSON. It now
targets a hidden iframe, so an error response is harmless.
ALSO
- The HTML export still had the exists()-then-open TOCTOU the ZIP path was fixed to remove,
at three sites, two with a hard `?` that failed the ENTIRE viewer. It is reachable: the
compression worker hard-deletes an original when a transcode fails and can still be running
when the gallery is released.
- Crash-orphaned .tmp files were immortal (prune deliberately skips .tmp). Swept at boot,
where it is unambiguously safe.
- export_status read the epoch in one statement and used it in the next; now one query.
- DiskCache::snapshot IGNORED its path argument on a cache hit, returning whatever filesystem
was measured last. Latent only because both callers pass media_path — it would have silently
misreported the moment anyone measured the exports volume. Now keyed by path.
- Corrected two comments that asserted safety properties the code does not have (claim_job
does NOT prevent a retired-epoch claim — retirement is enforced at READ time; and a
multi-replica reap is NOT contained, it can corrupt the keepsake). Added a rollback runbook
to migration 014, the repo's first destructive migration.
TESTS
The regression guard for the headline FOR SHARE fix was probabilistic — it could pass on
broken code if the interleaving fell the wrong way. It is now STRUCTURAL: the upload is held
open mid-body with its pre-flight check already passed, so the release provably lands inside
the exact window. Verified 5/5 FAILING (201 instead of 403) with the fix reverted, 5/5
passing with it.
Verified: backend 40 tests, svelte-check 0 errors, e2e typecheck clean,
e2e 160 passed / 1 skipped on chromium-desktop.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
|
b3d876fa85 |
fix(review-2): medium/low + docs — config validation, hygiene, doc sync
- compression_concurrency wired to boot config (state.rs) and removed as a dead admin control. - patch_config gains per-key integer/range validation so numerically-valid-but- nonsensical values (-1/NaN/3.5) are rejected instead of silently reverting to default at read time. - clearAuth now also clears the display name (shared-device residual). - e2e/Caddyfile.test mirrors prod security headers + the SSE encode-exclusion so the test stack is representative and can catch header regressions. - .gitignore: ignore root-level Playwright artifacts (test-results/, report). - docs: USER_JOURNEYS §10 and SECURITY-BACKLOG updated to match the implemented read-only-ban behavior and the export-path fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
b1e2e66305 |
test(e2e): address self-review follow-ups (dedup, XSS render guard, SSE hardening)
Follow-ups from the code review of the test-quality batches: - Consolidate duplicated helpers into e2e/helpers/: seed.ts (seedUpload, seedComment, listComments, findFeedRow) and sse.ts (mintSseTicket, openStream, trackStreamOpens). Refactor authorization-deep, xss-injection, like-comment, sse-ticket-abuse, ddos, sse-realtime, multi-tab, and SseListener to use them — the upload/comment/ticket-flow contracts now live in one place each instead of being re-inlined across 3–7 specs. - xss-injection display-name loop: it navigated to /feed (which renders uploader names, not the viewer's) so "nothing fired" passed vacuously — the payload was never rendered. Now navigate to /account (the actual sink) and add a render guard asserting the payload reached the DOM as escaped text before checking __xssFired. - sse-realtime reconnect: snapshot the stream-open count AFTER backgrounding, so the "new connection" assertion is attributable to the foreground event and can't be satisfied by a spurious native/error reconnect before the toggle. - recover-page: correct the comment (auto-submit is the onPinInput handler, not an $effect). 44 affected specs verified green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
136417d6b4 |
test(e2e): make vacuous feed/SSE tests assert real behavior
Several tests ran green while asserting nothing. Replace them with real assertions, and fix the SSE helper they depend on. sse-listener: exchange the JWT for a single-use ticket (POST /stream/ticket) and connect via ?ticket= — the helper still used the dead ?token= scheme, so every SSE-based assertion would have silently failed to receive events. like-comment: - "like is idempotent" asserted nothing (void feed; void b) → now seeds a real upload and pins the like contract: counted once per user, toggles off on repeat (guards double-count), and a second user's like is counted independently. - "comment → SSE to B" asserted length >= 0 (always true) → B now subscribes to the stream, A comments, and B must receive the new-comment event for that upload (comment_count === 1). ~30s due to reverse-proxy SSE buffering; timeout raised. sse-realtime: only checked a nav link was visible → now counts EventSource opens and asserts a fresh stream connection after hidden→visible (also fixes the sim, which set visibilityState but not document.hidden, so the close never fired). multi-tab "SSE delivers to both": only checked nav links → now asserts each tab opens its own stream connection (delivery isn't asserted — it hinges on the ~30s proxy buffering; connection establishment is the reliable, honest signal). safe-area: delete the /join probe whose only assertion was Array.isArray(x) === true (always true); the real sheet-level env() check already exists below it. All verified green against the live backend. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
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>
|