diff --git a/server/api/controllers/__tests__/program-application.test.js b/server/api/controllers/__tests__/program-application.test.js index f71ad31..ae723ac 100644 --- a/server/api/controllers/__tests__/program-application.test.js +++ b/server/api/controllers/__tests__/program-application.test.js @@ -221,7 +221,7 @@ describe('ProgramController.updateApplicationStatus', () => { it('updates on valid payload', async () => { programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding' }); - applicationService.getById.mockResolvedValue({ id: 'a1', status: 'submitted' }); + applicationService.getById.mockResolvedValue({ id: 'a1', programId: 'p1', status: 'submitted' }); applicationService.updateStatus.mockResolvedValue({ id: 'a1', status: 'accepted', projectId: 'proj-1' }); notificationService.notifyProjectTeam.mockResolvedValue([]); const req = { @@ -242,7 +242,7 @@ describe('ProgramController.updateApplicationStatus', () => { it('maps PostgREST no-rows error to 404', async () => { programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding' }); - applicationService.getById.mockResolvedValue({ id: 'missing', status: 'submitted' }); + applicationService.getById.mockResolvedValue({ id: 'missing', programId: 'p1', status: 'submitted' }); const err = Object.assign(new Error('no rows'), { code: 'PGRST116' }); applicationService.updateStatus.mockRejectedValue(err); const req = { @@ -257,7 +257,7 @@ describe('ProgramController.updateApplicationStatus', () => { it('calls notifyProjectTeam with application_accepted when submitted → accepted', async () => { programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding 2026' }); - applicationService.getById.mockResolvedValue({ id: 'app-1', status: 'submitted' }); + applicationService.getById.mockResolvedValue({ id: 'app-1', programId: 'p1', status: 'submitted' }); applicationService.updateStatus.mockResolvedValue({ id: 'app-1', status: 'accepted', projectId: 'proj-1' }); notificationService.notifyProjectTeam.mockResolvedValue([]); const req = { @@ -278,7 +278,7 @@ describe('ProgramController.updateApplicationStatus', () => { it('calls notifyProjectTeam with application_rejected when submitted → rejected', async () => { programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding 2026' }); - applicationService.getById.mockResolvedValue({ id: 'app-2', status: 'submitted' }); + applicationService.getById.mockResolvedValue({ id: 'app-2', programId: 'p1', status: 'submitted' }); applicationService.updateStatus.mockResolvedValue({ id: 'app-2', status: 'rejected', projectId: 'proj-2' }); notificationService.notifyProjectTeam.mockResolvedValue([]); const req = { @@ -299,7 +299,7 @@ describe('ProgramController.updateApplicationStatus', () => { it('does not call notifyProjectTeam when new status is submitted (no real transition)', async () => { programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding 2026' }); - applicationService.getById.mockResolvedValue({ id: 'app-3', status: 'submitted' }); + applicationService.getById.mockResolvedValue({ id: 'app-3', programId: 'p1', status: 'submitted' }); applicationService.updateStatus.mockResolvedValue({ id: 'app-3', status: 'submitted', projectId: 'proj-3' }); const req = { params: { slug: 'dogfooding-2026', applicationId: 'app-3' }, @@ -314,7 +314,7 @@ describe('ProgramController.updateApplicationStatus', () => { it('still returns 200 even when notifyProjectTeam rejects', async () => { programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding 2026' }); - applicationService.getById.mockResolvedValue({ id: 'app-4', status: 'submitted' }); + applicationService.getById.mockResolvedValue({ id: 'app-4', programId: 'p1', status: 'submitted' }); applicationService.updateStatus.mockResolvedValue({ id: 'app-4', status: 'accepted', projectId: 'proj-4' }); notificationService.notifyProjectTeam.mockRejectedValue(new Error('boom')); const req = { @@ -327,20 +327,33 @@ describe('ProgramController.updateApplicationStatus', () => { expect(res.status).toHaveBeenCalledWith(200); }); - it('still updates and returns 200 when the prior-status lookup (getById) fails', async () => { + it('404s (no write) when the application belongs to a DIFFERENT program (IDOR)', async () => { programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding 2026' }); - applicationService.getById.mockRejectedValue(new Error('supabase transport error')); - applicationService.updateStatus.mockResolvedValue({ id: 'app-5', status: 'accepted', projectId: 'proj-5' }); + // Application exists but is scoped to another program. + applicationService.getById.mockResolvedValue({ id: 'other', programId: 'p2', status: 'submitted' }); const req = { - params: { slug: 'dogfooding-2026', applicationId: 'app-5' }, + params: { slug: 'dogfooding-2026', applicationId: 'other' }, body: { status: 'accepted' }, user: { address: 'admin' }, }; const res = mockRes(); await programController.updateApplicationStatus(req, res); - // The status update still succeeds; getById failure only skips the notify gate. - expect(applicationService.updateStatus).toHaveBeenCalled(); - expect(res.status).toHaveBeenCalledWith(200); + expect(res.status).toHaveBeenCalledWith(404); + expect(applicationService.updateStatus).not.toHaveBeenCalled(); expect(notificationService.notifyProjectTeam).not.toHaveBeenCalled(); }); + + it('404s (no write) when the application does not exist', async () => { + programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding 2026' }); + applicationService.getById.mockResolvedValue(null); + const req = { + params: { slug: 'dogfooding-2026', applicationId: 'ghost' }, + body: { status: 'accepted' }, + user: { address: 'admin' }, + }; + const res = mockRes(); + await programController.updateApplicationStatus(req, res); + expect(res.status).toHaveBeenCalledWith(404); + expect(applicationService.updateStatus).not.toHaveBeenCalled(); + }); }); diff --git a/server/api/controllers/program.controller.js b/server/api/controllers/program.controller.js index d0cecaa..7921e07 100644 --- a/server/api/controllers/program.controller.js +++ b/server/api/controllers/program.controller.js @@ -120,15 +120,14 @@ class ProgramController { return res.status(404).json({ status: 'error', message: 'Program not found' }); } - // Prior-status lookup feeds only the notification gate below — a failure - // here must not break the status update or change the HTTP response. - let prevStatus; - try { - const existing = await programApplicationService.getById(applicationId); - prevStatus = existing?.status; - } catch (err) { - logger.error('Failed to read application prior status for notification gating:', err); + // Cross-check program scoping: an application from program B must not be + // updatable via program A's slug (IDOR). Authoritative — reject before any + // write. The fetched row also feeds the prior-status notification gate. + const existing = await programApplicationService.getById(applicationId); + if (!existing || existing.programId !== program.id) { + return res.status(404).json({ status: 'error', message: 'Application not found' }); } + const prevStatus = existing.status; const reviewedBy = req.user?.address || req.auth?.address || 'unknown'; const updated = await programApplicationService.updateStatus({ diff --git a/supabase/migrations/20260616000000_enable_rls_public_tables.sql b/supabase/migrations/20260616000000_enable_rls_public_tables.sql new file mode 100644 index 0000000..793cd77 --- /dev/null +++ b/supabase/migrations/20260616000000_enable_rls_public_tables.sql @@ -0,0 +1,28 @@ +-- Enable Row Level Security on every public table (deny-by-default). +-- +-- WHY: the Supabase Data API (PostgREST) exposes the `public` schema to the +-- `anon` / `authenticated` roles via the public anon key, which ships in the +-- client bundle. With RLS disabled, that key can read AND write every public +-- table, bypassing the Express API and its SIWS/admin auth. Verified on prod: +-- anon could SELECT and DELETE/UPDATE projects, program_admins, multisig_*. +-- +-- WHY THIS IS SAFE HERE: the browser never queries these tables — the client +-- uses Supabase only for auth (supabase.auth.*), and ALL data access goes +-- through the Express server using the SERVICE_ROLE key, which has BYPASSRLS. +-- So enabling RLS with NO policies denies anon/authenticated while the server +-- is unaffected. If a table ever needs direct anon/authenticated access, add an +-- explicit policy for it. +-- +-- Idempotent: ENABLE on an already-RLS-enabled table is a no-op. + +DO $$ +DECLARE r RECORD; +BEGIN + FOR r IN + SELECT tablename + FROM pg_tables + WHERE schemaname = 'public' + LOOP + EXECUTE format('ALTER TABLE public.%I ENABLE ROW LEVEL SECURITY;', r.tablename); + END LOOP; +END $$;