From 20125fb71362ea8e2e87d4c35b2d85e83ea2f53b Mon Sep 17 00:00:00 2001
From: fabi <fabi@mc02.dev>
Date: Wed, 1 Jul 2026 07:27:22 +0200
Subject: [PATCH] test(e2e): pin security assertions + real cross-user IDOR
 coverage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 e2e/fixtures/db.ts                            |  11 ++
 e2e/specs/06-export/export.spec.ts            |  48 +++++---
 .../07-adversarial/authorization-deep.spec.ts | 109 +++++++++++++++---
 .../file-upload-attacks.spec.ts               | Bin 6826 -> 6990 bytes
 4 files changed, 141 insertions(+), 27 deletions(-)

diff --git a/e2e/fixtures/db.ts b/e2e/fixtures/db.ts
index 1c285b6..bdbb9b2 100644
--- a/e2e/fixtures/db.ts
+++ b/e2e/fixtures/db.ts
@@ -68,6 +68,17 @@ export const db = {
     );
   },
 
+  /**
+   * Flip the `export_zip_ready` gate directly. The download handler serves bytes
+   * only when this boolean is true AND the file exists on disk, so setting it true
+   * without a file lets tests exercise the "ready but file missing" 404 branch.
+   */
+  async setExportZipReady(slug: string, ready: boolean) {
+    await withClient((c) =>
+      c.query(`UPDATE event SET export_zip_ready = $2 WHERE slug = $1`, [slug, ready])
+    );
+  },
+
   /** Insert a pre-baked export job row to skip the (slow) real compression path. */
   async fakeExportJob(eventSlug: string, type: 'zip' | 'html', status: 'pending' | 'running' | 'done') {
     await withClient(async (c) => {
diff --git a/e2e/specs/06-export/export.spec.ts b/e2e/specs/06-export/export.spec.ts
index fd7fd47..fa17887 100644
--- a/e2e/specs/06-export/export.spec.ts
+++ b/e2e/specs/06-export/export.spec.ts
@@ -42,22 +42,42 @@ test.describe('Export — release and download', () => {
     expect(body.html.status).toBe('done');
   });
 
-  test('ZIP download returns 404 when no file is on disk (export released but never compressed)', async ({ guest, db }) => {
-    const g = await guest('NoFile');
+  const base = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
+
+  // Browser downloads stream to disk via a top-level navigation, so the download
+  // endpoint authenticates with a single-use ticket (no Bearer header).
+  async function mintTicket(jwt: string): Promise<string> {
+    const res = await fetch(base + '/api/v1/export/ticket', {
+      method: 'POST',
+      headers: { Authorization: `Bearer ${jwt}` },
+    });
+    return (await res.json()).ticket;
+  }
+
+  test('ZIP download 404s when the export is not yet marked ready', async ({ guest, db }) => {
+    const g = await guest('NotReady');
+    // Released flag set, but export_zip_ready is still false → must refuse, never serve.
     await db.setExportReleased(SLUG, true);
     await db.fakeExportJob(SLUG, 'zip', 'done');
-    const base = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
-    // Browser downloads stream straight to disk via a top-level navigation, so the
-    // download endpoint authenticates with a single-use ticket (no Bearer header).
-    const ticketRes = await fetch(base + '/api/v1/export/ticket', {
-      method: 'POST',
-      headers: { Authorization: `Bearer ${g.jwt}` },
-    });
-    const { ticket } = await ticketRes.json();
-    // Real backend additionally checks event.export_zip_ready. The faked row is
-    // enough for /status; the download path needs the boolean flag too.
+    const ticket = await mintTicket(g.jwt);
+
     const res = await fetch(base + '/api/v1/export/zip?ticket=' + encodeURIComponent(ticket));
-    // Either 404 ("not available" OR "file not found") — both are valid states for this setup.
-    expect([404, 200]).toContain(res.status);
+    // Pinned to 404 (not [404,200]): a 200 here would mean serving an export that was
+    // never released for download — a data-exposure regression. This hits the
+    // `!export_zip_ready` guard.
+    expect(res.status).toBe(404);
+  });
+
+  test('ZIP download 404s when marked ready but the file is missing on disk', async ({ guest, db }) => {
+    const g = await guest('ReadyNoFile');
+    // Released AND ready, but no Gallery.zip on disk (we never ran a real export) →
+    // the handler must 404 on the missing-file check, not 200/500 or serve a stale file.
+    await db.setExportReleased(SLUG, true);
+    await db.setExportZipReady(SLUG, true);
+    await db.fakeExportJob(SLUG, 'zip', 'done');
+    const ticket = await mintTicket(g.jwt);
+
+    const res = await fetch(base + '/api/v1/export/zip?ticket=' + encodeURIComponent(ticket));
+    expect(res.status).toBe(404);
   });
 });
diff --git a/e2e/specs/07-adversarial/authorization-deep.spec.ts b/e2e/specs/07-adversarial/authorization-deep.spec.ts
index dca3def..4b4a521 100644
--- a/e2e/specs/07-adversarial/authorization-deep.spec.ts
+++ b/e2e/specs/07-adversarial/authorization-deep.spec.ts
@@ -5,26 +5,109 @@
  * with cross-user and banned-user scenarios that span multiple resources.
  */
 import { test, expect } from '../../fixtures/test';
+import { uploadRaw, JPEG_MAGIC } from '../../helpers/upload-client';
 
 const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
 
-test.describe('Adversarial — deep authorization', () => {
-  test('user A cannot delete user B\'s comment via /api/v1/comment/{id}', async ({ api, guest }) => {
-    const a = await guest('CommentA');
-    const b = await guest('CommentB');
+/** Seed a real upload owned by `jwt` and return its id. */
+async function seedUpload(jwt: string, caption?: string): Promise<string> {
+  const body = new Uint8Array(1024);
+  body.set(JPEG_MAGIC, 0);
+  const res = await uploadRaw(jwt, body, { filename: 'a.jpg', contentType: 'image/jpeg', caption });
+  if (res.status !== 201) throw new Error(`seed upload failed: ${res.status} ${await res.text()}`);
+  return (await res.json()).id;
+}
 
-    // We need an upload first; without a multipart helper here we use a placeholder:
-    // post a comment on a non-existent upload to force the path to return 404 / 403 / 401.
-    // The real intent is verified once an upload helper feeds this test a real upload_id.
-    const fakeId = '00000000-0000-0000-0000-000000000000';
-    const res = await fetch(`${BASE}/api/v1/comment/${fakeId}`, {
+/** Seed a real comment on `uploadId` authored by `jwt`; return its id. */
+async function seedComment(jwt: string, uploadId: string, body = 'mine'): Promise<string> {
+  const res = await fetch(`${BASE}/api/v1/upload/${uploadId}/comments`, {
+    method: 'POST',
+    headers: { Authorization: `Bearer ${jwt}`, 'Content-Type': 'application/json' },
+    body: JSON.stringify({ body }),
+  });
+  if (res.status !== 201) throw new Error(`seed comment failed: ${res.status} ${await res.text()}`);
+  return (await res.json()).id;
+}
+
+async function listComments(jwt: string, uploadId: string): Promise<any[]> {
+  const res = await fetch(`${BASE}/api/v1/upload/${uploadId}/comments`, {
+    headers: { Authorization: `Bearer ${jwt}` },
+  });
+  return res.json();
+}
+
+test.describe('Adversarial — deep authorization', () => {
+  // IDOR: user B must not be able to delete user A's REAL comment. This exercises the
+  // ownership guard (`comment.user_id != auth.user_id` → 403) — the previous version fired
+  // at the all-zeros UUID, which 404s at the lookup BEFORE that guard runs, so it never
+  // tested authorization at all.
+  test('user B cannot delete user A\'s comment (real resource → 403, comment survives)', async ({ guest }) => {
+    const a = await guest('CommentOwnerA');
+    const b = await guest('AttackerB');
+
+    const uploadId = await seedUpload(a.jwt);
+    const commentId = await seedComment(a.jwt, uploadId, 'A owns this');
+
+    const res = await fetch(`${BASE}/api/v1/comment/${commentId}`, {
       method: 'DELETE',
       headers: { Authorization: `Bearer ${b.jwt}` },
     });
-    // Acceptable: 403 (not your comment), 404 (no such comment), 401.
-    expect([401, 403, 404]).toContain(res.status);
-    void a;
-    void api;
+    // Must be 403 specifically — the comment exists and is in B's event, so a 404 would
+    // mean the ownership check was skipped/reordered.
+    expect(res.status).toBe(403);
+
+    // No state change: the comment is still there.
+    const after = await listComments(a.jwt, uploadId);
+    expect(after.some((c: any) => c.id === commentId)).toBe(true);
+
+    // Control: the real owner CAN delete it (proves the 403 was about identity, not a broken route).
+    const ownerDel = await fetch(`${BASE}/api/v1/comment/${commentId}`, {
+      method: 'DELETE',
+      headers: { Authorization: `Bearer ${a.jwt}` },
+    });
+    expect(ownerDel.status).toBe(204);
+  });
+
+  // IDOR: user B must not be able to delete user A's REAL upload.
+  test('user B cannot delete user A\'s upload (403, upload survives)', async ({ guest, db }) => {
+    const a = await guest('UploadOwnerA');
+    const b = await guest('AttackerB2');
+
+    const uploadId = await seedUpload(a.jwt);
+    expect(await db.countUploadsForUser(a.userId)).toBe(1);
+
+    const res = await fetch(`${BASE}/api/v1/upload/${uploadId}`, {
+      method: 'DELETE',
+      headers: { Authorization: `Bearer ${b.jwt}` },
+    });
+    expect(res.status).toBe(403);
+
+    // No state change: A's upload is still present (not soft-deleted).
+    expect(await db.countUploadsForUser(a.userId)).toBe(1);
+  });
+
+  // IDOR: user B must not be able to edit (re-caption / re-tag) user A's upload.
+  test('user B cannot edit user A\'s upload caption (403, caption unchanged)', async ({ guest, db }) => {
+    const a = await guest('UploadOwnerA2');
+    const b = await guest('AttackerB3');
+
+    const uploadId = await seedUpload(a.jwt, 'original caption');
+    // Make it visible in the feed so we can read the caption back.
+    await db.setUploadCompressionStatus(uploadId, 'done');
+
+    const res = await fetch(`${BASE}/api/v1/upload/${uploadId}`, {
+      method: 'PATCH',
+      headers: { Authorization: `Bearer ${b.jwt}`, 'Content-Type': 'application/json' },
+      body: JSON.stringify({ caption: 'hacked by B' }),
+    });
+    expect(res.status).toBe(403);
+
+    // No state change: the caption A set is intact.
+    const feedRes = await fetch(`${BASE}/api/v1/feed`, { headers: { Authorization: `Bearer ${a.jwt}` } });
+    const feed: any = await feedRes.json();
+    const list: any[] = feed.uploads ?? feed.items ?? feed;
+    const row = list.find((u: any) => u.id === uploadId);
+    expect(row?.caption).toBe('original caption');
   });
 
   test('banned user cannot toggle a like', async ({ api, host, guest }) => {
diff --git a/e2e/specs/07-adversarial/file-upload-attacks.spec.ts b/e2e/specs/07-adversarial/file-upload-attacks.spec.ts
index baa9dcae701ca7de4a3200272be9c8557ec31294..8cbe49d07da22dcec92e6e8fbb726ff7323fc5a0 100644
GIT binary patch
delta 1594
zcma)6O^X~=6eS;IF~JN()Rk~)(dmZn>QRy`dZHUA#)K%5nTQag{krN_^~+SfSG@PC
zXG)aLLfn}}%0J*n#8qIn?%eAi5Eru*6n}x9_o}O%2!c&F-Bs21zH`n!=YD_nr)y8Y
zx!9AI3_s-9aYDk8n=@5qu0=vKU8zKvx_NxzYV+5{FP2lOQZ|!G#;*U`{Re|7DqUri
zrT4XBI{ETRD4z?J<ZM=LEgecXrz_jr)bH&I$I{g$DH+e*2H`ax6Pp8}lskF!ZEq0K
zU8xkX^p$pW|Bda}hbVn8n2=Z!uINx#c|rvXMR;>jx_X=mGZPt8Dsv{A%d+I8YhDWL
zs1TWqN3+^7EkrI8;iOhWI-COrK><DzvCHcK-*9HwTKoZ*c;*-lvPzh9iH(|{H(zaz
z&tGqT*|_pbAkm@!ti7s~V`JTqKe$ibc(8LUFa3UY363jPPJ0rQa8N1gpfk=8EiAfy
z_r^`oN}kbCW=aE5xEJS=)mk{uSBZiN_oQJE=v$|am!cF99i0<4a=Cz>snJDfv6}9^
zd$)E6L$aEj5er}zc_*3l%$Y*xq5^w`q0QG5J>3P!2(HL0RxzY+50644J}Tj=K9D!R
zZeIErv<Exg3HxY|2c=e)sXYSI=6DV?cqE(?@d2BWt;!ONla|g5H@z=tI%XG-r$h3;
zqi8yany2Tt&hBh9k1xO3Jh?nP7e!mmADeXCd;YsKy9vj+Fz6C%Oh>s-;6V1`w8|zs
zB;uG$Cp?E#=ea&?)eg2HZyOszaO;IXmnKx=;GXc<;$S7h7iZek_%jS=dy`d&BGaAH
zcI=>ZLcHA~8tr>T>nVB%9B^$(rzy3mn{L_Q;^oT52|K6@lSbsloav<Yj6~Piu*id*
z!ic3qc}vnq%g+MNu%$#Gyx<z~cO&aiRZuQwoJZ96eG6#}My%7W?*}0hILVjYxbx0P
z*pY-zJbqYm)`msQ^ny>NYrS7mwDNV<dlqL|z#kw|FE_uvxYfUIV<SuVmgli935Mom
z^wHiP5T^{oG+$k~c3iyl+!@T`)cOg{`N)lC!Uo5LbXn@}eRy;H?*4ms#<%xYh6jXy
zEvb&)dRLu1`WjHZM_Q_-0ZYtxg@HsS<WeCUmd!${%r7JC5V>Gl^XMYiH7_`kg0zvD
z#Mu6G&xJLIxCNt$5UP~`|E#8=*Kr*u3jo-Bg$ABPsEm!SV4ljw8bSvew(_%%ff81Y
T=g%y%=zlCSaQGbg^`E~1d$T0V

delta 1582
zcmbu9&5qne6onPYiV=wj17ro4AZ5By+nz)uiV{sE=tL1LNTfu7gwUv5b$eX2U1h4u
z)9Ij5vp_rm<^|Xx*bvQ{Ju4m}4*+7vl2dlOXJEmGmMpdH^8NYlx#zd*KVJL!`>j#V
zg*epK(MVa@RBQq(nrWl7IUqeFAtl#%)}Ft3z5Q$J>(#Vz4yPy=a?VDr4|saQ&L;UX
zFn#dg?mbVVtGidml(Q6#X9@*VB@VRgkH~`M`~|`<%gAwPobmLbHB2*<!c`rXq)puf
z5=h|->p~*6lZ{>lPluw^N(609N_S4G7Ji5*HXS_s>bowtv{micdwfF+>*nlY)TUI(
z{yEDK4^^b>!ua4=R1?aZa1!pF-CYvhp*N|iODh!RwlPY$CBnrpIg>0|7<TPX+gDc<
z*^Q7f^+ANj)85`5bws}Gq7q4Kye&Ch2xmq+pRSRAc8X)4$Wtx6r%cp3JG_#uv&X(-
z1g+3X-$EQ%PcFBwD$O31&YU$N-5KBNk{%sjJNNr@?bGcE>I3+x(?Ls75)!bBJa2Ze
zaC3;TRpSF08wd(Pcfb<0f%)K{qKt#gVrdJN(#Lo&%uXPs9cY%z0bT7Z$i(TR+8{Le
zOvu}u7EGQ)DEo=W1AS)+nAK>E6J^;WWCE&lFbD;8ssZWJF48W+e=%;Kp1piF_TBz*
zVRx&mw&#HR8_yFRwZEKy{rJ6?e>n4gEDe(056hbOL|vDF5@}><no_=uO@bz#@qxv_
z`!Y`N%dkY<lE_#7T&<f)9X^Xv-)Q{)JPJdxEQJHiO-!ic(y7ds`u;E@kmbj9R4CL#
zgPgs70-{}HA9RTc#;W7UO%%j-B_8i<>>KY_IkL?}Q-=1X3zyqpUwh*%FP*MK7fTMy
zJh;Hx1lljo-8jB`@x?Q%N-R4KC=RiRhacUGiy)Bl**D+f%}EQD_LT^-=w6%*6oZfZ
z;TsqUt4&{z3hgN{6gKz{Q2o0aZ&!p}Zr~3Pkv=FerYQ5#w4YdVM?)+gDvr_>AqZJS
z3!4n`n?8I%*!ZksrA7Akn2=O=!^Rg7zJsj5D-^;+Bf~a9q~)f_cjfKqej<lVZ4+Q3
z>~D617!AHSo=`Lgta`K&j5J>Ycw~hP<NxUepq(sqD3bNahNLmc+6Yj&D$&9}3?VLX
r<ka;&6%=F8C&@tV5p<s1uVaO1$5?Ol3{DTH1+Npg0K_9WN4)+9QClGa