From 3f6dafba0580c9ea2b219b33b191cbf8903e643f Mon Sep 17 00:00:00 2001 From: fabi Date: Fri, 3 Jul 2026 07:19:09 +0200 Subject: [PATCH] test(review-2): strengthen CR2 export-leak test to drive a real export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior test asserted 404 on /media/exports/Gallery.zip against an empty stack — it would pass even if exports were still written under /media, because no archive was ever produced. Now it: 1. seeds an upload, releases the gallery, and polls until the real zip job writes Gallery.zip to disk; 2. asserts the archive is NOT served from public /media (the CR2 leak); and 3. asserts it IS retrievable via the gated ticket endpoint (200 + PK zip magic) — proving the 404 means "not public", not "no file". Also fixes a test-isolation gap the CR2 relocation introduced: __truncate wiped media_path but not export_path, so a real export would leave Gallery.zip on disk and break export.spec's "ready-but-file-missing → 404" test. truncate_all now purges export_path too. Full 06-export dir: 5/5 green, no contamination. Co-Authored-By: Claude Opus 4.8 --- backend/src/handlers/test_admin.rs | 6 +++ e2e/specs/06-export/export-leak.spec.ts | 52 +++++++++++++++++++++---- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/backend/src/handlers/test_admin.rs b/backend/src/handlers/test_admin.rs index f3d0cc4..0d6b2e6 100644 --- a/backend/src/handlers/test_admin.rs +++ b/backend/src/handlers/test_admin.rs @@ -70,6 +70,12 @@ pub async fn truncate_all( let _ = tokio::fs::remove_dir_all(&state.config.media_path).await; let _ = tokio::fs::create_dir_all(&state.config.media_path).await; + // Wipe the export directory too. Exports moved OUT of media_path (CR2 fix), so + // the media wipe above no longer covers them — without this a real export in + // one test would leave Gallery.zip on disk and contaminate the next. + let _ = tokio::fs::remove_dir_all(&state.config.export_path).await; + let _ = tokio::fs::create_dir_all(&state.config.export_path).await; + // The rate limiter holds an in-memory HashMap; clear it so a previous test's // counters don't leak into the next one. state.rate_limiter.clear(); diff --git a/e2e/specs/06-export/export-leak.spec.ts b/e2e/specs/06-export/export-leak.spec.ts index ff1a21c..4515d2a 100644 --- a/e2e/specs/06-export/export-leak.spec.ts +++ b/e2e/specs/06-export/export-leak.spec.ts @@ -3,20 +3,58 @@ * were written under media_path/exports, and /media is a public ServeDir — so * anyone could GET /media/exports/Gallery.zip and download the whole gallery, * bypassing the ticket + export_*_ready gate. Exports now live OUTSIDE media_path - * and are reachable only via the gated /api/v1/export/{zip,html} handlers (covered - * by export.spec.ts). Here we assert the public path is dead. + * and are reachable only via the gated /api/v1/export/{zip,html} handlers. + * + * This drives a REAL export (release → job runs → archive on disk) and then + * asserts the archive is NOT public but IS gated. Asserting a 404 on an empty + * stack would pass even if exports were still under /media (the file just wouldn't + * exist yet) — so we produce a real file first, then prove it can't leak. */ import { test, expect } from '../../fixtures/test'; +import { seedUpload } from '../../helpers/seed'; const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; test.describe('Export — no public leak (CR2)', () => { - test('archives are not reachable through the public /media path', async () => { + test('a real export is downloadable only via the gated endpoint, never via /media', async ({ + host, + }) => { + test.setTimeout(60_000); + const bearer = { Authorization: `Bearer ${host.jwt}` }; + + // Seed content so the archive actually contains a file. + await seedUpload(host.jwt, { caption: 'in the export' }); + + // Host releases the gallery → spawns the real zip/html export jobs. + const rel = await fetch(`${BASE}/api/v1/host/gallery/release`, { method: 'POST', headers: bearer }); + expect(rel.status).toBe(204); + + // Wait for the real zip job to finish writing the archive to disk. + await expect + .poll( + async () => { + const res = await fetch(`${BASE}/api/v1/export/status`, { headers: bearer }); + return (await res.json()).zip?.status; + }, + { timeout: 45_000, intervals: [500] } + ) + .toBe('done'); + + // The archive now EXISTS on disk. It must NOT be reachable via public /media… for (const name of ['Gallery.zip', 'Memories.zip']) { - const res = await fetch(`${BASE}/media/exports/${name}`); - // 404 (not 200): a 200 here would mean the whole-gallery archive is - // downloadable without any auth — the CR2 data-exposure regression. - expect(res.status).toBe(404); + const leak = await fetch(`${BASE}/media/exports/${name}`); + // A 200 here = the whole-gallery archive is downloadable with no auth (CR2). + expect(leak.status, `${name} must not be served from public /media`).toBe(404); } + + // …but IS retrievable via the gated single-use ticket endpoint. This proves the + // 404 above means "not public", not merely "no file was produced". + const ticketRes = await fetch(`${BASE}/api/v1/export/ticket`, { method: 'POST', headers: bearer }); + const { ticket } = await ticketRes.json(); + const dl = await fetch(`${BASE}/api/v1/export/zip?ticket=${encodeURIComponent(ticket)}`); + expect(dl.status).toBe(200); + // Real ZIP payload: the archive starts with the PK local-file-header magic. + const head = new Uint8Array(await dl.arrayBuffer()).subarray(0, 2); + expect(Array.from(head)).toEqual([0x50, 0x4b]); // "PK" }); });