Adversarial re-review of the persona-audit + audit-followup rounds (6411747..)
found one HIGH and one MED regression plus LOW gaps. All fixed with coverage.
HIGH — export stale-keepsake resurrected by an open_event race
A reopen landing in the window between a *current* export worker's finalize_job
and its ready-flag flip cleared export_released_at + the ready flags but left the
export_job row `done` at the same release_seq. The seq-guarded flip then still
matched and re-set export_{zip,html}_ready=TRUE on a pre-reopen snapshot; the next
re-release read that stale TRUE and skipped regeneration (`if ready { continue }`),
serving a keepsake missing every upload from the reopen window — the exact data
loss migration 012 exists to prevent. Both ready-flip UPDATEs are now additionally
anchored on `export_released_at IS NOT NULL`, so a landed reopen makes the flip a
no-op and the re-release regenerates cleanly.
MED — queue dedup broke for reloaded items
loadQueue rebuilt QueueItems from IndexedDB without copying lastModified, which the
new addToQueue dedup keys on. A file re-selected after a page reload / PWA relaunch
missed the duplicate check and uploaded twice. Rehydration now carries lastModified
(extracted to a pure, tested entryToQueueItem helper).
LOW
- diashow: clear the upload-processed debounce timer in onDestroy (no stray
post-unmount /feed fetch).
- USER_JOURNEYS §9.5: document the reconnect-delta ban replay (hidden_user_ids /
uploads_hidden_at, migration 013), not just the live user-hidden SSE.
- e2e api-client: drop the misleading hide_uploads param from banUser — the backend
takes no body and always hides; strip the dead boolean at all call sites.
Tests
- Extract isReversibleLock (the terminal-403 KEEP-vs-PURGE-blob discriminator) into a
pure exported helper + unit tests, so the data-loss-critical branch is covered
without an XHR harness.
- entryToQueueItem unit tests lock the lastModified-carry regression.
- Document the export flip-race guard in the reopen/re-release spec (the sub-ms
finalize↔flip interleave isn't deterministically forceable with fast fixtures;
covered by the SQL guard + the end-to-end completeness test).
Verified: backend 40 tests, frontend 44 unit tests, svelte-check 0 errors,
e2e 156 passed / 1 skipped on chromium-desktop.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
145 lines
6.2 KiB
TypeScript
145 lines
6.2 KiB
TypeScript
/**
|
|
* Phase 2 adversarial — deeper authorization escalation paths.
|
|
*
|
|
* Complements the foundational 403/401 checks in 05-admin/authorization.spec.ts
|
|
* with cross-user and banned-user scenarios that span multiple resources.
|
|
*/
|
|
import { test, expect } from '../../fixtures/test';
|
|
import { seedUpload, seedComment, listComments, findFeedRow } from '../../helpers/seed';
|
|
|
|
const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
|
|
|
|
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}` },
|
|
});
|
|
// 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 }) => {
|
|
const a = await guest('UploadOwnerA2');
|
|
const b = await guest('AttackerB3');
|
|
|
|
// seedUpload marks compression done, so the upload is feed-visible for the read-back.
|
|
const uploadId = await seedUpload(a.jwt, { caption: 'original caption' });
|
|
|
|
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 row = findFeedRow(await feedRes.json(), uploadId);
|
|
expect(row?.caption).toBe('original caption');
|
|
});
|
|
|
|
test('banned user cannot toggle a like', async ({ api, host, guest }) => {
|
|
const target = await guest('BannedLike');
|
|
await api.banUser(host.jwt, target.userId);
|
|
|
|
const res = await fetch(`${BASE}/api/v1/upload/00000000-0000-0000-0000-000000000000/like`, {
|
|
method: 'POST',
|
|
headers: { Authorization: `Bearer ${target.jwt}` },
|
|
});
|
|
expect([403, 404]).toContain(res.status);
|
|
});
|
|
|
|
test('banned user cannot post a comment', async ({ api, host, guest }) => {
|
|
const target = await guest('BannedComment');
|
|
await api.banUser(host.jwt, target.userId);
|
|
|
|
const res = await fetch(`${BASE}/api/v1/upload/00000000-0000-0000-0000-000000000000/comments`, {
|
|
method: 'POST',
|
|
headers: { Authorization: `Bearer ${target.jwt}`, 'Content-Type': 'application/json' },
|
|
body: JSON.stringify({ body: 'should be rejected' }),
|
|
});
|
|
expect([403, 404]).toContain(res.status);
|
|
});
|
|
|
|
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);
|
|
|
|
const res = await fetch(`${BASE}/api/v1/feed`, {
|
|
headers: { Authorization: `Bearer ${target.jwt}` },
|
|
});
|
|
// The journey docs explicitly state banned users keep read access.
|
|
expect(res.status).toBe(200);
|
|
});
|
|
|
|
test('host cannot delete another host\'s session via /api/v1/session', async ({ api, host, guest }) => {
|
|
const otherHost = await guest('OtherHost');
|
|
// Promote so they have a host JWT to play with.
|
|
await api.setRole(host.jwt, otherHost.userId, 'host');
|
|
|
|
// The /session DELETE endpoint deletes the caller's own session by token hash.
|
|
// A host cannot pass another host's token here (no way to authenticate as them),
|
|
// so this is structurally safe — we assert by trying to delete with the wrong
|
|
// Authorization header and checking that only the caller's session is gone.
|
|
await api.logout(host.jwt);
|
|
const stillWorks = await fetch(`${BASE}/api/v1/me/context`, {
|
|
headers: { Authorization: `Bearer ${otherHost.jwt}` },
|
|
});
|
|
expect(stillWorks.status).toBe(200);
|
|
});
|
|
|
|
test('promote endpoint cannot be used to make oneself admin', async ({ host }) => {
|
|
const res = await fetch(`${BASE}/api/v1/host/users/${'00000000-0000-0000-0000-000000000000'}/role`, {
|
|
method: 'PATCH',
|
|
headers: { Authorization: `Bearer ${host.jwt}`, 'Content-Type': 'application/json' },
|
|
body: JSON.stringify({ role: 'admin' }),
|
|
});
|
|
// 400 (invalid role for host-callable endpoint) or 403/404.
|
|
expect([400, 403, 404]).toContain(res.status);
|
|
// Critically, NOT 200/204.
|
|
expect([200, 204]).not.toContain(res.status);
|
|
});
|
|
});
|