diff --git a/backend/src/services/export.rs b/backend/src/services/export.rs index 6b82c71..85d22e5 100644 --- a/backend/src/services/export.rs +++ b/backend/src/services/export.rs @@ -303,11 +303,19 @@ async fn run_zip_export_inner( return Ok(()); } - // Flip the ready flag only while still current (a re-release between finalize and here - // would have cleared it; don't resurrect it for a superseded generation). + // Flip the ready flag only while still current AND still released. The `release_seq` + // EXISTS check alone is NOT enough: a reopen (`open_event`) landing in the window between + // our `finalize_job` above and this UPDATE clears `export_released_at` + both ready flags + // but leaves our `export_job` row `done` at this same seq — so EXISTS would still match and + // we'd resurrect `export_zip_ready = TRUE` on a keepsake that predates the reopen. The next + // re-release would then read that stale TRUE, skip regeneration (`if ready { continue }`), + // and serve a snapshot missing every upload added during the reopen window — the exact + // stale-keepsake data loss migration 012 exists to prevent. Anchoring on + // `export_released_at IS NOT NULL` makes the flip a no-op once a reopen has landed. sqlx::query( "UPDATE event SET export_zip_ready = TRUE WHERE id = $1 + AND export_released_at IS NOT NULL AND EXISTS (SELECT 1 FROM export_job WHERE event_id = $1 AND type = 'zip'::export_type AND release_seq = $2 AND status = 'done')", @@ -644,9 +652,12 @@ async fn run_html_export_inner( return Ok(()); } + // Same released-anchored guard as the ZIP flip (see run_zip_export): a reopen between our + // finalize and here must not resurrect `export_html_ready` on a pre-reopen snapshot. sqlx::query( "UPDATE event SET export_html_ready = TRUE WHERE id = $1 + AND export_released_at IS NOT NULL AND EXISTS (SELECT 1 FROM export_job WHERE event_id = $1 AND type = 'html'::export_type AND release_seq = $2 AND status = 'done')", diff --git a/docs/USER_JOURNEYS.md b/docs/USER_JOURNEYS.md index e738a76..a80887c 100644 --- a/docs/USER_JOURNEYS.md +++ b/docs/USER_JOURNEYS.md @@ -167,7 +167,11 @@ the Host can clean up later). 5. Banning **always hides**: the user's existing uploads are filtered out of the feed for everyone (`v_feed`, `find_visible_media`, and the export query all enforce `is_banned = FALSE`), and a live `user-hidden` SSE event evicts their cards from every - open feed + the diashow without a reload. + open feed + the diashow without a reload. A client that was offline/disconnected during + the live event doesn't miss the eviction: `uploads_hidden_at` is stamped at ban time + (migration 013) and the reconnect delta (`GET /feed/delta`) returns the banned users in + `hidden_user_ids`, so the feed and diashow replay the eviction on the next reconnect — + the projector-missed-the-live-push case is exactly why this exists. ## 11. Admin — instance configuration diff --git a/e2e/fixtures/api-client.ts b/e2e/fixtures/api-client.ts index c86a61a..238ae0b 100644 --- a/e2e/fixtures/api-client.ts +++ b/e2e/fixtures/api-client.ts @@ -109,10 +109,11 @@ export class ApiClient { }); } - async banUser(token: string, userId: string, hideUploads = false) { + // A ban ALWAYS hides the user's uploads — the backend takes no body and ignores any + // `hide_uploads` flag (the old opt-out was removed). No per-request options. + async banUser(token: string, userId: string) { return this.request('POST', `/host/users/${userId}/ban`, { token, - body: { hide_uploads: hideUploads }, expectedStatus: [200, 204], }); } diff --git a/e2e/specs/04-host/moderation.spec.ts b/e2e/specs/04-host/moderation.spec.ts index c019255..ba10ea4 100644 --- a/e2e/specs/04-host/moderation.spec.ts +++ b/e2e/specs/04-host/moderation.spec.ts @@ -9,7 +9,7 @@ import { seedUpload, seedComment } from '../../helpers/seed'; test.describe('Host — moderation API', () => { test('ban with hide_uploads=true sets the right flags', async ({ api, host, guest }) => { const target = await guest('Banned1'); - await api.banUser(host.jwt, target.userId, true); + await api.banUser(host.jwt, target.userId); const users = await api.listUsers(host.jwt); const row = users.find((u: any) => u.id === target.userId); expect(row?.is_banned).toBe(true); @@ -20,7 +20,7 @@ test.describe('Host — moderation API', () => { // Ban is now unconditionally a hide: even asking NOT to hide still hides, because a // banned user's content is "gone" everywhere. The legacy hide_uploads arg is ignored. const target = await guest('Banned2'); - await api.banUser(host.jwt, target.userId, false); + await api.banUser(host.jwt, target.userId); const users = await api.listUsers(host.jwt); const row = users.find((u: any) => u.id === target.userId); expect(row?.is_banned).toBe(true); @@ -29,7 +29,7 @@ test.describe('Host — moderation API', () => { test('banned user cannot call /upload', async ({ api, host, guest }) => { const target = await guest('Banned3'); - await api.banUser(host.jwt, target.userId, false); + await api.banUser(host.jwt, target.userId); // Direct fetch — multipart body shape is just a marker; the auth middleware should reject before parsing. const res = await fetch((process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101') + '/api/v1/upload', { @@ -88,14 +88,14 @@ test.describe('Host — live role/ban revocation (H1)', () => { // Sanity: host token works. await api.listUsers(host.jwt); - await api.banUser(adminToken, host.userId, false); + await api.banUser(adminToken, host.userId); // Banned users are rejected with 403 by the auth extractor before any handler runs. await expect(api.listUsers(host.jwt)).rejects.toThrow(/→ 403/); }); test('a banned host cannot unban themselves', async ({ api, adminToken, host }) => { - await api.banUser(adminToken, host.userId, false); + await api.banUser(adminToken, host.userId); await expect( api.unbanUser(host.jwt, host.userId, { expectedStatus: [204] }) ).rejects.toThrow(/→ 403/); @@ -114,7 +114,7 @@ test.describe('Host — live role/ban revocation (H1)', () => { const ownUpload = await seedUpload(target.jwt, { caption: 'mine' }); const ownComment = await seedComment(target.jwt, ownUpload, 'my comment'); - await api.banUser(host.jwt, target.userId, false); + await api.banUser(host.jwt, target.userId); // Reads still succeed. const read = await fetch(base + '/api/v1/me/context', { headers: auth(target.jwt) }); diff --git a/e2e/specs/04-host/sse-eviction.spec.ts b/e2e/specs/04-host/sse-eviction.spec.ts index a7f698c..463647a 100644 --- a/e2e/specs/04-host/sse-eviction.spec.ts +++ b/e2e/specs/04-host/sse-eviction.spec.ts @@ -41,7 +41,7 @@ test.describe('Host — live SSE eviction (H3)', () => { const sse = new SseListener(); await sse.start(host.jwt); - await api.banUser(host.jwt, target.userId, true); + await api.banUser(host.jwt, target.userId); await sse.waitForEvent( 'user-hidden', @@ -69,7 +69,7 @@ test.describe('Host — live SSE eviction (H3)', () => { await expect(page.getByText('evict-me-live-xyz').first()).toBeVisible(); // Host hides the target — the viewer's feed must drop the card via SSE, no reload. - await api.banUser(host.jwt, target.userId, true); + await api.banUser(host.jwt, target.userId); await expect(page.getByText('evict-me-live-xyz')).toHaveCount(0, { timeout: 15_000 }); }); }); diff --git a/e2e/specs/07-adversarial/authorization-deep.spec.ts b/e2e/specs/07-adversarial/authorization-deep.spec.ts index 964dfb4..7a22943 100644 --- a/e2e/specs/07-adversarial/authorization-deep.spec.ts +++ b/e2e/specs/07-adversarial/authorization-deep.spec.ts @@ -82,7 +82,7 @@ test.describe('Adversarial — deep authorization', () => { test('banned user cannot toggle a like', async ({ api, host, guest }) => { const target = await guest('BannedLike'); - await api.banUser(host.jwt, target.userId, false); + await api.banUser(host.jwt, target.userId); const res = await fetch(`${BASE}/api/v1/upload/00000000-0000-0000-0000-000000000000/like`, { method: 'POST', @@ -93,7 +93,7 @@ test.describe('Adversarial — deep authorization', () => { test('banned user cannot post a comment', async ({ api, host, guest }) => { const target = await guest('BannedComment'); - await api.banUser(host.jwt, target.userId, false); + await api.banUser(host.jwt, target.userId); const res = await fetch(`${BASE}/api/v1/upload/00000000-0000-0000-0000-000000000000/comments`, { method: 'POST', @@ -105,7 +105,7 @@ test.describe('Adversarial — deep authorization', () => { test('banned user can still read the feed (read-only access preserved)', async ({ api, host, guest }) => { const target = await guest('BannedRead'); - await api.banUser(host.jwt, target.userId, false); + await api.banUser(host.jwt, target.userId); const res = await fetch(`${BASE}/api/v1/feed`, { headers: { Authorization: `Bearer ${target.jwt}` }, diff --git a/e2e/specs/10-flow-review/export-reopen-rerelease.spec.ts b/e2e/specs/10-flow-review/export-reopen-rerelease.spec.ts index 949a93c..705d05f 100644 --- a/e2e/specs/10-flow-review/export-reopen-rerelease.spec.ts +++ b/e2e/specs/10-flow-review/export-reopen-rerelease.spec.ts @@ -17,6 +17,18 @@ * superseded worker discards its output instead of resurrecting a stale keepsake. The * download follows the current `done` row's `file_path`, never a fixed name. * + * A second, narrower window on the SAME class of bug (fixed alongside): a reopen (`open_event`) + * landing between a *current* worker's `finalize_job` and its ready-flag flip. `open_event` + * clears `export_released_at` + both ready flags but leaves the `export_job` row `done` at the + * current seq — so the seq-guarded flip would still match and resurrect `export_zip_ready=TRUE` + * on a pre-reopen snapshot, and the next re-release would `if ready { continue }` and skip + * regeneration. The flip UPDATEs are therefore additionally anchored on + * `export_released_at IS NOT NULL`, making them a no-op once a reopen has landed. The + * "completeness" test below exercises the real reopen→re-release worker path end-to-end; the + * sub-millisecond finalize↔flip interleave itself isn't deterministically forceable with the + * fast fixtures, so that exact window is covered by the SQL guard + code review rather than a + * timing-dependent assertion. + * * Coverage: * - churn integrity: rapid reopen→re-release yields exactly one INTACT ZIP, no stuck jobs. * - completeness: an upload added during the reopen window IS present in the re-released diff --git a/frontend/src/lib/upload-queue.test.ts b/frontend/src/lib/upload-queue.test.ts index 70e0961..632e79a 100644 --- a/frontend/src/lib/upload-queue.test.ts +++ b/frontend/src/lib/upload-queue.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { classifyUploadStatus } from './upload-queue'; +import { classifyUploadStatus, isReversibleLock, entryToQueueItem } from './upload-queue'; /** * Regression guard for the upload-queue retry policy (H2 + M1). The bug being locked out: @@ -40,3 +40,70 @@ describe('classifyUploadStatus', () => { expect(classifyUploadStatus(503)).toBe('transient'); }); }); + +/** + * Regression guard for the reversible-lock discrimination inside the `terminal` bucket — the + * branch that decides whether a 4xx KEEPS the blob (event closed / gallery released: a host can + * reopen and the photo resumes) or PURGES it (permanent ban / quota). Getting this wrong either + * loses a photo the guest expected to survive a reopen, or lets a banned device retry forever. + */ +describe('isReversibleLock', () => { + it('an `uploads_locked` code is reversible at any status (event closed / released)', () => { + expect(isReversibleLock(403, 'uploads_locked')).toBe(true); + expect(isReversibleLock(409, 'uploads_locked')).toBe(true); + }); + + it('a `forbidden` 403 (banned) is PERMANENT — purge, never resume', () => { + expect(isReversibleLock(403, 'forbidden')).toBe(false); + }); + + it('an unidentifiable 403 (unparseable proxy/WAF/captive-portal body) is treated reversible', () => { + // Losing a photo is the worst outcome; 403 is the reversible-lock status here. + expect(isReversibleLock(403, undefined)).toBe(true); + expect(isReversibleLock(403, null)).toBe(true); + expect(isReversibleLock(403, '')).toBe(true); + }); + + it('a non-403 permanent 4xx (e.g. 413 quota) is NOT reversible unless explicitly locked', () => { + expect(isReversibleLock(413, undefined)).toBe(false); + expect(isReversibleLock(400, 'bad_request')).toBe(false); + expect(isReversibleLock(413, 'uploads_locked')).toBe(true); // explicit tag still wins + }); +}); + +/** + * Regression guard for the queue-rehydration mapping. The bug this locks out: `loadQueue` + * rebuilt items from IndexedDB WITHOUT copying `lastModified`, so a reloaded item had + * `lastModified === undefined`. addToQueue's dedup keys on (name, size, lastModified), so + * re-selecting the same file after a reload would MISS the duplicate and queue it twice. + */ +describe('entryToQueueItem', () => { + const base = { + id: 'e1', + userId: 'u1', + fileName: 'photo.jpg', + fileSize: 1234, + lastModified: 1_700_000_000_000, + mimeType: 'image/jpeg', + status: 'pending' as const + }; + + it('carries lastModified across rehydration (dedup depends on it)', () => { + expect(entryToQueueItem(base).lastModified).toBe(1_700_000_000_000); + }); + + it('downgrades an interrupted `uploading` entry to `pending` so it resumes', () => { + expect(entryToQueueItem({ ...base, status: 'uploading' }).status).toBe('pending'); + }); + + it('a `done` entry reports 100% progress; others start at 0', () => { + expect(entryToQueueItem({ ...base, status: 'done' }).progress).toBe(100); + expect(entryToQueueItem(base).progress).toBe(0); + }); + + it('defaults caption/hashtags to empty strings', () => { + const item = entryToQueueItem(base); + expect(item.caption).toBe(''); + expect(item.hashtags).toBe(''); + }); +}); diff --git a/frontend/src/lib/upload-queue.ts b/frontend/src/lib/upload-queue.ts index 780a29e..a6505f0 100644 --- a/frontend/src/lib/upload-queue.ts +++ b/frontend/src/lib/upload-queue.ts @@ -211,6 +211,57 @@ export function classifyUploadStatus(status: number): UploadOutcome { return 'transient'; } +/** + * Within the `terminal` bucket, decide whether a 4xx is a REVERSIBLE lock (keep the blob, + * park retryable for a host reopen) rather than a permanent rejection (purge the blob). + * Pure + exported so this data-loss-critical rule is unit-testable without an XHR harness. + * + * Reversible when: + * - the backend tagged it `uploads_locked` (event closed / gallery released — a host can reopen), OR + * - it's ANY 403 we can't positively identify as a permanent ban (`forbidden`). An unparseable + * 403 body (proxy/WAF/captive portal) must NOT purge the blob — losing a photo is the worst + * outcome, and 403 is the reversible-lock status here. + * A `forbidden` 403 (banned) and every non-403 4xx (e.g. 413 quota) are permanent → purge. + */ +export function isReversibleLock(status: number, errorCode: unknown): boolean { + return errorCode === 'uploads_locked' || (status === 403 && errorCode !== 'forbidden'); +} + +/** + * Rehydrate a persisted IndexedDB entry into an in-memory `QueueItem`. Pure + exported so the + * field-mapping is unit-testable. The rule that must not regress: `lastModified` MUST be carried + * across — addToQueue's dedup keys on it, so an item restored from IndexedDB (page reload / PWA + * relaunch) with an undefined lastModified would fail to match a re-selection of the same file + * and silently queue it twice. `uploading` is downgraded to `pending` (an interrupted in-flight + * upload must resume, not stay stuck spinning). + */ +export function entryToQueueItem(entry: { + id: string; + userId: string; + fileName: string; + fileSize: number; + lastModified?: number; + mimeType: string; + caption?: string; + hashtags?: string; + status: QueueItem['status'] | 'uploading'; + error?: string; +}): QueueItem { + return { + id: entry.id, + userId: entry.userId, + fileName: entry.fileName, + fileSize: entry.fileSize, + lastModified: entry.lastModified, + mimeType: entry.mimeType, + caption: entry.caption ?? '', + hashtags: entry.hashtags ?? '', + status: entry.status === 'uploading' ? 'pending' : entry.status, + progress: entry.status === 'done' ? 100 : 0, + error: entry.error + }; +} + export async function loadQueue(): Promise { const database = await getDb(); const myUserId = getUserId(); @@ -220,18 +271,7 @@ export async function loadQueue(): Promise { // explicit logout via `clearQueue`). const items: QueueItem[] = all .filter((entry) => entry.userId && entry.userId === myUserId) - .map((entry) => ({ - id: entry.id, - userId: entry.userId, - fileName: entry.fileName, - fileSize: entry.fileSize, - mimeType: entry.mimeType, - caption: entry.caption ?? '', - hashtags: entry.hashtags ?? '', - status: entry.status === 'uploading' ? 'pending' : entry.status, - progress: entry.status === 'done' ? 100 : 0, - error: entry.error - })); + .map(entryToQueueItem); queueItems.set(items); // Staged-but-unsent items from a prior session (queued offline, tab closed before // reconnect) must resume now — otherwise the "queue flushes when you're back online" @@ -501,10 +541,7 @@ async function uploadItem(id: string): Promise { // = banned) as reversible: an unparseable 403 body (proxy/WAF/captive // portal) must NOT purge the blob — losing a photo is the worst outcome, and // 403 is the reversible-lock status here. - if ( - body?.error === 'uploads_locked' || - (xhr.status === 403 && body?.error !== 'forbidden') - ) { + if (isReversibleLock(xhr.status, body?.error)) { reject(new LockedError(body?.message || 'Event ist geschlossen.')); break; } diff --git a/frontend/src/routes/diashow/+page.svelte b/frontend/src/routes/diashow/+page.svelte index 8879344..e8239d5 100644 --- a/frontend/src/routes/diashow/+page.svelte +++ b/frontend/src/routes/diashow/+page.svelte @@ -240,6 +240,7 @@ showBottomNav.set(true); clearTimer(); if (overlayHideTimer) clearTimeout(overlayHideTimer); + if (processedDebounce) clearTimeout(processedDebounce); void releaseWakeLock(); for (const unsub of unsubs) unsub(); });