From 7fc08c5165fb1c03d16e3dd55c695a6b20c0b0ec Mon Sep 17 00:00:00 2001 From: Moheeb Zara Date: Sat, 1 Aug 2026 21:31:51 -0700 Subject: [PATCH] fix(incidents): restrict appealIncident to admin/board, no orphan proposal (M2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit appealIncident was gated by requireMember but the incident UPDATE is RLS- restricted to admin/board, so a reporter's appeal inserted a draft proposal, updated 0 incident rows, and returned success — an orphan proposal. Gate with ADMIN_ROLES to match the write, and assert the link UPDATE affected a row (.select), rolling back the draft proposal if it did not. Co-Authored-By: Claude Opus 4.8 (1M context) --- __tests__/incident-appeal.test.ts | 157 ++++++++++++++++++++++++++++++ lib/actions/incidents.ts | 23 +++-- 2 files changed, 174 insertions(+), 6 deletions(-) create mode 100644 __tests__/incident-appeal.test.ts diff --git a/__tests__/incident-appeal.test.ts b/__tests__/incident-appeal.test.ts new file mode 100644 index 0000000..63ea3c9 --- /dev/null +++ b/__tests__/incident-appeal.test.ts @@ -0,0 +1,157 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +/** + * Authorization + orphan-proposal guard for appealIncident. + * + * The incident UPDATE inside appealIncident is RLS-restricted to admin/board. + * Before the fix the action was gated by requireMember (ANY member): a plain + * member/reporter would insert a draft proposal, update ZERO incident rows, and + * still get `{ data: proposal }` back — an orphan proposal the incident never + * links to. These tests drive the REAL action through a recording mock client + * and assert (a) a non-admin/board caller is rejected with NO proposal created, + * and (b) on the admin path the proposal survives only when the incident update + * actually affects a row, otherwise it is rolled back. + */ + +// ─── Recording mock Supabase client (shared style with actions.test.ts) ─────── +type QueueItem = { data?: unknown; error?: unknown } + +function makeClient(opts: { user?: { id: string; email?: string } | null; queue?: QueueItem[] }) { + const calls: Array<{ method: string; args: unknown[] }> = [] + const queue = [...(opts.queue ?? [])] + const next = (): QueueItem => (queue.length ? queue.shift()! : { data: null, error: null }) + + const builder: Record = new Proxy( + {}, + { + get(_t, prop: string) { + if (prop === 'then') { + return (resolve: (v: QueueItem) => void) => resolve(next()) + } + if (prop === 'single' || prop === 'maybeSingle') { + return (...args: unknown[]) => { + calls.push({ method: prop, args }) + return Promise.resolve(next()) + } + } + return (...args: unknown[]) => { + calls.push({ method: prop, args }) + return builder + } + }, + }, + ) + + const client = { + auth: { + getUser: vi.fn(() => Promise.resolve({ data: { user: opts.user ?? null } })), + }, + from: (...args: unknown[]) => { + calls.push({ method: 'from', args }) + return builder + }, + _calls: calls, + } + return client +} + +let currentClient: ReturnType + +vi.mock('@/lib/supabase/server', () => ({ + createClient: () => Promise.resolve(currentClient), +})) +vi.mock('@/lib/supabase/admin', () => ({ + createAdminClient: () => makeClient({ user: null, queue: [] }), +})) +vi.mock('next/cache', () => ({ revalidatePath: vi.fn() })) + +import { appealIncident } from '@/lib/actions/incidents' + +const INCIDENT_ID = '22222222-2222-2222-2222-222222222222' +const user = { id: 'user-A', email: 'a@example.com' } +const memberRow = (role: string, space_id = 'space-A') => ({ + id: 'member-A', + space_id, + user_id: user.id, + role, + status: 'current', + display_name: 'Admin A', + handle: 'admin-a', +}) + +/** Recorded `.from(table)` targets, in order. */ +const fromTables = (c: ReturnType) => + c._calls.filter(x => x.method === 'from').map(x => x.args[0]) + +/** Recorded `.insert(payload)` calls into a given table are hard to attribute + * to a table directly, so detect the proposal insert by its shape. */ +const proposalInserts = (c: ReturnType) => + c._calls + .filter(x => x.method === 'insert') + .map(x => x.args[0]) + .filter( + (o): o is Record => + !!o && typeof o === 'object' && (o as Record).proposal_type === 'membership_vote', + ) + +const deleteCalls = (c: ReturnType) => + c._calls.filter(x => x.method === 'delete') + +beforeEach(() => { + vi.clearAllMocks() +}) + +describe('appealIncident authorization', () => { + it('rejects a plain member with an admin/board message and creates NO proposal', async () => { + currentClient = makeClient({ user, queue: [{ data: memberRow('member') }] }) + const res = await appealIncident({ incidentId: INCIDENT_ID, title: 'Appeal' }) + expect(res).toHaveProperty('error') + expect((res as { error: string }).error).toMatch(/admin/i) + // The action must bail before ever touching incidents or proposals. + expect(proposalInserts(currentClient)).toHaveLength(0) + expect(fromTables(currentClient)).not.toContain('proposals') + }) +}) + +describe('appealIncident orphan-proposal guard', () => { + it('links the incident and returns the proposal when the update affects a row', async () => { + currentClient = makeClient({ + user, + queue: [ + { data: memberRow('admin', 'space-A') }, // requireMember + { data: { id: INCIDENT_ID, status: 'decided', space_id: 'space-A' } }, // incident lookup + { data: { id: 'proposal-1' } }, // proposal insert().select().single() + { data: [{ id: INCIDENT_ID }] }, // incident update().select() → 1 row affected + { error: null }, // activity_log insert + ], + }) + const res = await appealIncident({ incidentId: INCIDENT_ID, title: 'Appeal' }) + expect(res).not.toHaveProperty('error') + expect((res as { data: { id: string } }).data.id).toBe('proposal-1') + expect(proposalInserts(currentClient)).toHaveLength(1) + // A successful update must NOT roll the proposal back. + expect(deleteCalls(currentClient)).toHaveLength(0) + }) + + it('rolls the draft proposal back (no orphan) when the incident update affects zero rows', async () => { + currentClient = makeClient({ + user, + queue: [ + { data: memberRow('admin', 'space-A') }, // requireMember + { data: { id: INCIDENT_ID, status: 'decided', space_id: 'space-A' } }, // incident lookup + { data: { id: 'proposal-1' } }, // proposal insert().select().single() + { data: [], error: null }, // incident update().select() → RLS-silent 0 rows + ], + }) + const res = await appealIncident({ incidentId: INCIDENT_ID, title: 'Appeal' }) + expect(res).toHaveProperty('error') + // The orphan draft proposal must be deleted, not left dangling. + expect(proposalInserts(currentClient)).toHaveLength(1) + const deletes = deleteCalls(currentClient) + expect(deletes.length).toBeGreaterThan(0) + // And the incident-linking UPDATE must be scoped and asserted via .select(). + const eqCols = currentClient._calls.filter(x => x.method === 'eq').map(x => x.args[0]) + expect(eqCols).toContain('space_id') + expect(currentClient._calls.some(x => x.method === 'select')).toBe(true) + }) +}) diff --git a/lib/actions/incidents.ts b/lib/actions/incidents.ts index 0e37eb4..1f107bd 100644 --- a/lib/actions/incidents.ts +++ b/lib/actions/incidents.ts @@ -200,8 +200,11 @@ export async function addIncidentUpdate(formData: { /** * Appeal a dismissed incident by spawning a membership-vote proposal. - * The reporter or admin/board can trigger the appeal; the proposal goes - * to draft status so the petitioner (or board) can review before opening. + * Admin/board only: the incident UPDATE below is RLS-restricted to + * admin/board, so a non-privileged caller could otherwise insert a draft + * proposal, update zero incident rows, and still see success — an orphan + * proposal that never links back. The proposal goes to draft status so the + * board can review before opening. */ export async function appealIncident(formData: { incidentId: string @@ -212,7 +215,7 @@ export async function appealIncident(formData: { if (!v.ok) return { error: v.error } const supabase = await createClient() - const auth = await requireMember(supabase) + const auth = await requireMemberWithRole(supabase, ADMIN_ROLES, 'Admin or board access required') if (!auth.ok) return { error: auth.error } const { member } = auth @@ -243,12 +246,20 @@ export async function appealIncident(formData: { .single() if (proposalErr) return { error: proposalErr.message } - // Mark the incident as appealed and link it to the proposal. - const { error: linkErr } = await supabase + // Mark the incident as appealed and link it to the proposal. Assert the + // update actually touched a row via .select(): if RLS rejects the write + // (or the row is gone), roll back the draft proposal so we never leave an + // orphan that isn't linked from any incident. + const { data: linked, error: linkErr } = await supabase .from('incidents') .update({ status: 'appealed', appeal_proposal_id: proposal.id }) .eq('id', v.data.incidentId) - if (linkErr) return { error: linkErr.message } + .eq('space_id', member.space_id) + .select('id') + if (linkErr || !linked || linked.length === 0) { + await supabase.from('proposals').delete().eq('id', proposal.id) + return { error: linkErr?.message ?? 'Could not link the appeal to the incident.' } + } await logActivity(supabase, member, 'appealed', 'incident', v.data.incidentId)