fix(incidents): restrict appealIncident to admin/board, no orphan proposal (M2)

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) <[email protected]>

__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<string, unknown> = 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<typeof makeClient>
+
+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: '[email protected]' }
+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<typeof makeClient>) =>
+  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<typeof makeClient>) =>
+  c._calls
+    .filter(x => x.method === 'insert')
+    .map(x => x.args[0])
+    .filter(
+      (o): o is Record<string, unknown> =>
+        !!o && typeof o === 'object' && (o as Record<string, unknown>).proposal_type === 'membership_vote',
+    )
+
+const deleteCalls = (c: ReturnType<typeof makeClient>) =>
+  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)
+  })
+})

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
+ * Admin/board only: the incident UPDATE below is RLS-restricted to
- * to draft status so the petitioner (or board) can review before opening.
+ * 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.
+  // Mark the incident as appealed and link it to the proposal. Assert the
-  const { error: linkErr } = await supabase
+  // 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)