diff --git a/server/activities.ts b/server/activities.ts index 892e8ce..bd47834 100644 --- a/server/activities.ts +++ b/server/activities.ts @@ -556,13 +556,32 @@ activitiesRoutes.patch('/:id/sort', requireAuth, async (c) => { return c.json({ error: 'missing:position' }, 400); } const db = getDb(); - // Confirm the activity exists AND the viewer can see it. Anything else is - // a 404 — we don't want callers persisting positions for activities they - // can't see, even though it wouldn't surface anywhere visible. - const visible = db - .prepare('SELECT 1 FROM activities WHERE id = ?') - .get(id); - if (!visible) return c.json({ error: 'not_found' }, 404); + // Apply the same visibility filter as GET /:id and the list endpoint so + // sort doesn't double as an existence oracle for private / friends-only + // activity ids. Hidden rows return 404 (not 403). Earlier this endpoint + // only did a bare `SELECT 1 FROM activities WHERE id = ?` — surfaced by + // /audit security as a HIGH severity finding. + const row = db + .prepare('SELECT visibility, owner_id FROM activities WHERE id = ?') + .get(id) as { visibility: Visibility; owner_id: string } | null; + if (!row) return c.json({ error: 'not_found' }, 404); + if (row.visibility === 'private' && row.owner_id !== userId) { + return c.json({ error: 'not_found' }, 404); + } + if (row.visibility === 'friends' && row.owner_id !== userId) { + const isFriend = !!db + .prepare('SELECT 1 FROM friends WHERE owner_id = ? AND friend_id = ?') + .get(row.owner_id, userId); + if (!isFriend) return c.json({ error: 'not_found' }, 404); + const blocked = !!db + .prepare(` + SELECT 1 FROM user_blocks + WHERE (blocker_id = ? AND blocked_id = ?) + OR (blocker_id = ? AND blocked_id = ?) + `) + .get(row.owner_id, userId, userId, row.owner_id); + if (blocked) return c.json({ error: 'not_found' }, 404); + } db.prepare(` INSERT INTO user_activity_sort (user_id, activity_id, position) VALUES (?, ?, ?) diff --git a/tests/activities.test.ts b/tests/activities.test.ts index a27dcad..3c1b1c9 100644 --- a/tests/activities.test.ts +++ b/tests/activities.test.ts @@ -384,6 +384,41 @@ describe('per-user sort order', () => { expect(res.status).toBe(400); } }); + + ttest('PATCH /sort does not double as an existence oracle for hidden rows', async () => { + // Regression: the endpoint previously did a bare `SELECT 1 FROM activities` + // without visibility scoping, so a logged-in attacker could distinguish + // "private id exists, owned by someone else" (200 ok) from "id doesn't + // exist" (404). /audit security flagged this as HIGH. + const [owner, attacker] = await Promise.all([ + signupAndGetCookie(ctx, 'sort-oracle-owner@test.invalid'), + signupAndGetCookie(ctx, 'sort-oracle-att@test.invalid'), + ]); + const priv = await createActivity(ctx, owner.cookie, { + visibility: 'private', + ciphertext: 'AAAA', + nonce: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA', + } as never); + + // Attacker tries to sort someone else's private row → must get 404 + // (same status as for a truly nonexistent id). + const sortOther = await req(ctx, 'PATCH', `/api/activities/${priv.id}/sort`, { + cookie: attacker.cookie, body: { position: 0 }, + }); + expect(sortOther.status).toBe(404); + + // And for a truly nonexistent id. + const sortMissing = await req(ctx, 'PATCH', '/api/activities/nope-nope-nope/sort', { + cookie: attacker.cookie, body: { position: 0 }, + }); + expect(sortMissing.status).toBe(404); + + // Owner can still sort their own private row. + const sortOwn = await req(ctx, 'PATCH', `/api/activities/${priv.id}/sort`, { + cookie: owner.cookie, body: { position: 0 }, + }); + expect(sortOwn.status).toBe(200); + }); }); describe('owner_display fallback chain (no email leak)', () => {