Merge test/pin-security-assertions: pin security assertions + real IDOR

Turn accept-both-outcomes documentation tests into pinned secure-behavior
assertions (SVG stored-XSS→400, zero-byte→400, path-traversal→201, export→404),
and replace fake-UUID authz tests with real cross-user IDOR coverage (comment
delete, upload delete, caption edit all → 403 with no state change).
This commit is contained in:
fabi
2026-07-01 07:28:23 +02:00
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. */ /** 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') { async fakeExportJob(eventSlug: string, type: 'zip' | 'html', status: 'pending' | 'running' | 'done') {
await withClient(async (c) => { await withClient(async (c) => {

View File

@@ -42,22 +42,42 @@ test.describe('Export — release and download', () => {
expect(body.html.status).toBe('done'); 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 base = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
const g = await guest('NoFile');
// 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.setExportReleased(SLUG, true);
await db.fakeExportJob(SLUG, 'zip', 'done'); await db.fakeExportJob(SLUG, 'zip', 'done');
const base = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; const ticket = await mintTicket(g.jwt);
// 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 res = await fetch(base + '/api/v1/export/zip?ticket=' + encodeURIComponent(ticket)); 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. // Pinned to 404 (not [404,200]): a 200 here would mean serving an export that was
expect([404, 200]).toContain(res.status); // 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. * with cross-user and banned-user scenarios that span multiple resources.
*/ */
import { test, expect } from '../../fixtures/test'; import { test, expect } from '../../fixtures/test';
import { uploadRaw, JPEG_MAGIC } from '../../helpers/upload-client';
const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
test.describe('Adversarial — deep authorization', () => { /** Seed a real upload owned by `jwt` and return its id. */
test('user A cannot delete user B\'s comment via /api/v1/comment/{id}', async ({ api, guest }) => { async function seedUpload(jwt: string, caption?: string): Promise<string> {
const a = await guest('CommentA'); const body = new Uint8Array(1024);
const b = await guest('CommentB'); 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: /** Seed a real comment on `uploadId` authored by `jwt`; return its id. */
// post a comment on a non-existent upload to force the path to return 404 / 403 / 401. async function seedComment(jwt: string, uploadId: string, body = 'mine'): Promise<string> {
// The real intent is verified once an upload helper feeds this test a real upload_id. const res = await fetch(`${BASE}/api/v1/upload/${uploadId}/comments`, {
const fakeId = '00000000-0000-0000-0000-000000000000'; method: 'POST',
const res = await fetch(`${BASE}/api/v1/comment/${fakeId}`, { 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', method: 'DELETE',
headers: { Authorization: `Bearer ${b.jwt}` }, headers: { Authorization: `Bearer ${b.jwt}` },
}); });
// Acceptable: 403 (not your comment), 404 (no such comment), 401. // Must be 403 specifically — the comment exists and is in B's event, so a 404 would
expect([401, 403, 404]).toContain(res.status); // mean the ownership check was skipped/reordered.
void a; expect(res.status).toBe(403);
void api;
// 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 }) => { test('banned user cannot toggle a like', async ({ api, host, guest }) => {