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>
This commit is contained in:
fabi
2026-07-01 07:27:22 +02:00
parent 6ccf2d0011
commit 20125fb713
4 changed files with 141 additions and 27 deletions

View File

@@ -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) => {

View File

@@ -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);
});
});

View File

@@ -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 }) => {