diff --git a/apps/backend/src/integration/base-knowledge.integration.test.ts b/apps/backend/src/integration/base-knowledge.integration.test.ts index 68ab9007..c7ffb811 100644 --- a/apps/backend/src/integration/base-knowledge.integration.test.ts +++ b/apps/backend/src/integration/base-knowledge.integration.test.ts @@ -685,4 +685,317 @@ describe("Base Knowledge Integration Tests", () => { await supabaseAdminClient.from("documents").delete().eq("id", testDoc.id); } }); + + describe("RLS Policy Tests for Base Knowledge Chunk and Summary Mutations", () => { + const signedInUserClient = supabaseAnonClient; + const serviceRoleClient = supabaseAdminClient; + + let personalDocumentId: number; + + const tableFixtures = [ + { + table: "document_chunks", + textColumn: "content", + newBaseKnowledgeRow: () => ({ + document_id: documentId, + content: "Base knowledge chunk.", + page: 1, + chunk_index: 1, + owned_by_user_id: null, + folder_id: null, + access_group_id: accessGroupId, + chunk_mistral_embedding: JSON.stringify( + createDeterministicEmbedding(), + ), + }), + newPersonalRow: () => ({ + document_id: personalDocumentId, + content: "Personal chunk.", + page: 1, + chunk_index: 0, + owned_by_user_id: testUserId, + folder_id: null, + access_group_id: null, + chunk_mistral_embedding: JSON.stringify( + createDeterministicEmbedding(), + ), + }), + }, + { + table: "document_summaries", + textColumn: "summary", + newBaseKnowledgeRow: () => ({ + document_id: documentId, + summary: "Base knowledge summary.", + short_summary: "Base knowledge.", + owned_by_user_id: null, + folder_id: null, + access_group_id: accessGroupId, + tags: ["base-knowledge"], + }), + newPersonalRow: () => ({ + document_id: personalDocumentId, + summary: "Personal summary.", + short_summary: "Personal.", + owned_by_user_id: testUserId, + folder_id: null, + access_group_id: null, + tags: ["personal"], + }), + }, + ] as const; + + type MutableTable = (typeof tableFixtures)[number]["table"]; + + type InsertRow = Database["public"]["Tables"][MutableTable]["Insert"]; + + const insertRowWithServiceRole = async ( + table: MutableTable, + row: InsertRow, + ): Promise => { + const { data, error } = await serviceRoleClient + .from(table) + .insert(row) + .select("id") + .single(); + expect(error).toBeNull(); + expect(data).not.toBeNull(); + return data?.id ?? 0; + }; + + const readRowWithServiceRole = async (table: MutableTable, id: number) => { + const { data, error } = await serviceRoleClient + .from(table) + .select("*") + .eq("id", id) + .maybeSingle(); + expect(error).toBeNull(); + return data as Record | null; + }; + + const deleteRowWithServiceRole = async ( + table: MutableTable, + id: number, + ) => { + await serviceRoleClient.from(table).delete().eq("id", id); + }; + + const visibleRowIdsAsUser = async ( + table: MutableTable, + ids: number[], + ): Promise => { + const { data, error } = await signedInUserClient + .from(table) + .select("id") + .in("id", ids); + expect(error).toBeNull(); + return (data ?? []).map((row) => row.id).sort((a, b) => a - b); + }; + + // RLS never raises on UPDATE/DELETE; it just leaves forbidden rows out of the affected set. + const updateRowAsUser = async ( + table: MutableTable, + id: number, + values: Record, + ): Promise => { + const { data, error } = await signedInUserClient + .from(table) + .update(values) + .eq("id", id) + .select("id"); + expect(error).toBeNull(); + return (data ?? []).map((row) => row.id); + }; + + const deleteRowAsUser = async ( + table: MutableTable, + id: number, + ): Promise => { + const { data, error } = await signedInUserClient + .from(table) + .delete() + .eq("id", id) + .select("id"); + expect(error).toBeNull(); + return (data ?? []).map((row) => row.id); + }; + + const withTestUserAsAdmin = async (run: () => Promise) => { + const { error: grantError } = await serviceRoleClient + .from("application_admins") + .insert({ user_id: testUserId }); + expect(grantError).toBeNull(); + + try { + await run(); + } finally { + await serviceRoleClient + .from("application_admins") + .delete() + .eq("user_id", testUserId); + } + }; + + const withTestUserBanned = async (run: () => Promise) => { + const { error: banError } = + await serviceRoleClient.auth.admin.updateUserById(testUserId, { + ban_duration: "1h", + }); + expect(banError).toBeNull(); + + try { + await run(); + } finally { + await serviceRoleClient.auth.admin.updateUserById(testUserId, { + ban_duration: "none", + }); + } + }; + + beforeAll(async () => { + const { data: personalDocument, error } = await serviceRoleClient + .from("documents") + .insert({ + file_name: "rls-mutation-personal.pdf", + source_type: "personal_document", + source_url: `${testUserId}/rls-mutation-personal.pdf`, + file_checksum: "rls-mutation-personal-checksum", + file_size: SMALL_FILE_SIZE, + num_pages: 1, + folder_id: null, + owned_by_user_id: testUserId, + access_group_id: null, + processing_finished_at: new Date().toISOString(), + }) + .select("id") + .single(); + expect(error).toBeNull(); + personalDocumentId = personalDocument?.id ?? 0; + expect(personalDocumentId).toBeGreaterThan(0); + }); + + afterAll(async () => { + await serviceRoleClient + .from("documents") + .delete() + .eq("id", personalDocumentId); + }); + + describe.each(tableFixtures)( + "$table", + ({ table, textColumn, newBaseKnowledgeRow, newPersonalRow }) => { + let baseKnowledgeRowId: number; + let personalRowId: number; + + beforeEach(async () => { + baseKnowledgeRowId = await insertRowWithServiceRole( + table, + newBaseKnowledgeRow(), + ); + personalRowId = await insertRowWithServiceRole( + table, + newPersonalRow(), + ); + }); + + afterEach(async () => { + await deleteRowWithServiceRole(table, baseKnowledgeRowId); + await deleteRowWithServiceRole(table, personalRowId); + }); + + it("lets a regular user read base knowledge rows and their own rows", async () => { + const visibleIds = await visibleRowIdsAsUser(table, [ + baseKnowledgeRowId, + personalRowId, + ]); + + expect(visibleIds).toEqual([baseKnowledgeRowId, personalRowId]); + }); + + it("stops a regular user from deleting base knowledge rows", async () => { + const deletedIds = await deleteRowAsUser(table, baseKnowledgeRowId); + + expect(deletedIds).toEqual([]); + expect( + await readRowWithServiceRole(table, baseKnowledgeRowId), + ).not.toBeNull(); + }); + + it("stops a regular user from rewriting base knowledge rows and taking them over", async () => { + const rowBefore = await readRowWithServiceRole( + table, + baseKnowledgeRowId, + ); + const takeOverAsOwnRow = { + [textColumn]: "poisoned", + owned_by_user_id: testUserId, + access_group_id: null, + }; + + const updatedIds = await updateRowAsUser( + table, + baseKnowledgeRowId, + takeOverAsOwnRow, + ); + + expect(updatedIds).toEqual([]); + expect( + await readRowWithServiceRole(table, baseKnowledgeRowId), + ).toEqual(rowBefore); + }); + + it("lets an admin update base knowledge rows", async () => { + await withTestUserAsAdmin(async () => { + const updatedIds = await updateRowAsUser( + table, + baseKnowledgeRowId, + { [textColumn]: "updated by admin" }, + ); + + expect(updatedIds).toEqual([baseKnowledgeRowId]); + + const rowAfter = await readRowWithServiceRole( + table, + baseKnowledgeRowId, + ); + expect(rowAfter?.[textColumn]).toBe("updated by admin"); + expect(rowAfter?.owned_by_user_id).toBeNull(); + }); + }); + + it("lets an admin delete base knowledge rows", async () => { + await withTestUserAsAdmin(async () => { + const deletedIds = await deleteRowAsUser(table, baseKnowledgeRowId); + + expect(deletedIds).toEqual([baseKnowledgeRowId]); + expect( + await readRowWithServiceRole(table, baseKnowledgeRowId), + ).toBeNull(); + }); + }); + + it("lets the owner update and delete their own rows", async () => { + const updatedIds = await updateRowAsUser(table, personalRowId, { + [textColumn]: "updated by owner", + }); + expect(updatedIds).toEqual([personalRowId]); + + const deletedIds = await deleteRowAsUser(table, personalRowId); + expect(deletedIds).toEqual([personalRowId]); + expect(await readRowWithServiceRole(table, personalRowId)).toBeNull(); + }); + + it("hides every row from a banned user who still holds a pre-ban token", async () => { + await withTestUserBanned(async () => { + const visibleIds = await visibleRowIdsAsUser(table, [ + baseKnowledgeRowId, + personalRowId, + ]); + + expect(visibleIds).toEqual([]); + }); + }); + }, + ); + }); }); diff --git a/apps/backend/supabase/migrations/20260828000001_fix_document_chunk_summary_policies.sql b/apps/backend/supabase/migrations/20260828000001_fix_document_chunk_summary_policies.sql new file mode 100644 index 00000000..5c62d9c5 --- /dev/null +++ b/apps/backend/supabase/migrations/20260828000001_fix_document_chunk_summary_policies.sql @@ -0,0 +1,175 @@ +-- Split the FOR ALL policies on document_chunks and document_summaries into +-- per-command policies. PostgreSQL applies only USING to DELETE and to UPDATE +-- row selection, and the FOR ALL USING admitted every base-knowledge row +-- (owned_by_user_id IS NULL), so any authenticated user could delete or +-- rewrite the shared RAG corpus. +-- +-- Some environments were patched by hand with the per-command policy names +-- below, so every name is dropped first and the migration converges either way. +-- ── document_chunks ────────────────────────────────────────────────────────── +DROP POLICY IF EXISTS "Allow authenticated users to access own or public document_chun" ON public.document_chunks; + +DROP POLICY IF EXISTS "Allow users to read own or public document_chunks" ON public.document_chunks; + +DROP POLICY IF EXISTS "Allow users to insert own; admins public document_chunks" ON public.document_chunks; + +DROP POLICY IF EXISTS "Allow users to update own; admins public document_chunks" ON public.document_chunks; + +DROP POLICY IF EXISTS "Allow users to delete own; admins public document_chunks" ON public.document_chunks; + +CREATE POLICY "Allow users to read own or public document_chunks" ON public.document_chunks FOR +SELECT + TO authenticated USING ( + ( + owned_by_user_id IS NULL + OR owned_by_user_id = ( + SELECT + auth.uid () + ) + ) + AND NOT public.is_current_user_banned () + ); + +CREATE POLICY "Allow users to insert own; admins public document_chunks" ON public.document_chunks FOR insert TO authenticated +WITH + CHECK ( + ( + owned_by_user_id = ( + SELECT + auth.uid () + ) + AND NOT public.is_current_user_banned () + ) + OR ( + public.is_application_admin () + AND owned_by_user_id IS NULL + ) + ); + +CREATE POLICY "Allow users to update own; admins public document_chunks" ON public.document_chunks +FOR UPDATE + TO authenticated USING ( + ( + owned_by_user_id = ( + SELECT + auth.uid () + ) + AND NOT public.is_current_user_banned () + ) + OR ( + public.is_application_admin () + AND owned_by_user_id IS NULL + ) + ) +WITH + CHECK ( + ( + owned_by_user_id = ( + SELECT + auth.uid () + ) + AND NOT public.is_current_user_banned () + ) + OR ( + public.is_application_admin () + AND owned_by_user_id IS NULL + ) + ); + +CREATE POLICY "Allow users to delete own; admins public document_chunks" ON public.document_chunks FOR delete TO authenticated USING ( + ( + owned_by_user_id = ( + SELECT + auth.uid () + ) + AND NOT public.is_current_user_banned () + ) + OR ( + public.is_application_admin () + AND owned_by_user_id IS NULL + ) +); + +-- ── document_summaries ─────────────────────────────────────────────────────── +DROP POLICY IF EXISTS "Allow authenticated users to access own or public document_summ" ON public.document_summaries; + +DROP POLICY IF EXISTS "Allow users to read own or public document_summaries" ON public.document_summaries; + +DROP POLICY IF EXISTS "Allow users to insert own; admins public document_summaries" ON public.document_summaries; + +DROP POLICY IF EXISTS "Allow users to update own; admins public document_summaries" ON public.document_summaries; + +DROP POLICY IF EXISTS "Allow users to delete own; admins public document_summaries" ON public.document_summaries; + +CREATE POLICY "Allow users to read own or public document_summaries" ON public.document_summaries FOR +SELECT + TO authenticated USING ( + ( + owned_by_user_id IS NULL + OR owned_by_user_id = ( + SELECT + auth.uid () + ) + ) + AND NOT public.is_current_user_banned () + ); + +CREATE POLICY "Allow users to insert own; admins public document_summaries" ON public.document_summaries FOR insert TO authenticated +WITH + CHECK ( + ( + owned_by_user_id = ( + SELECT + auth.uid () + ) + AND NOT public.is_current_user_banned () + ) + OR ( + public.is_application_admin () + AND owned_by_user_id IS NULL + ) + ); + +CREATE POLICY "Allow users to update own; admins public document_summaries" ON public.document_summaries +FOR UPDATE + TO authenticated USING ( + ( + owned_by_user_id = ( + SELECT + auth.uid () + ) + AND NOT public.is_current_user_banned () + ) + OR ( + public.is_application_admin () + AND owned_by_user_id IS NULL + ) + ) +WITH + CHECK ( + ( + owned_by_user_id = ( + SELECT + auth.uid () + ) + AND NOT public.is_current_user_banned () + ) + OR ( + public.is_application_admin () + AND owned_by_user_id IS NULL + ) + ); + +CREATE POLICY "Allow users to delete own; admins public document_summaries" ON public.document_summaries FOR delete TO authenticated USING ( + ( + owned_by_user_id = ( + SELECT + auth.uid () + ) + AND NOT public.is_current_user_banned () + ) + OR ( + public.is_application_admin () + AND owned_by_user_id IS NULL + ) +);