diff --git a/supabase/functions/_backend/private/role_bindings.ts b/supabase/functions/_backend/private/role_bindings.ts index 9af4cfc974..d02f0c7272 100644 --- a/supabase/functions/_backend/private/role_bindings.ts +++ b/supabase/functions/_backend/private/role_bindings.ts @@ -712,9 +712,30 @@ app.delete('/:binding_id', requireUserAuth, sValidator('param', bindingIdParamSc if (targetRole && targetRole.priority_rank > callerMaxRank) { return c.json({ error: 'Cannot delete a binding for a role with higher privileges than your own' }, 403) } - await drizzle - .delete(schema.role_bindings) - .where(eq(schema.role_bindings.id, bindingId)) + await drizzle.transaction(async (tx) => { + await tx + .delete(schema.role_bindings) + .where(eq(schema.role_bindings.id, bindingId)) + + if (binding.principal_type === 'user' && binding.scope_type === 'org' && binding.org_id) { + await tx + .update(schema.org_users) + .set({ + user_right: null, + rbac_role_name: null, + updated_at: sql`now()`, + }) + .where( + and( + eq(schema.org_users.user_id, binding.principal_id), + eq(schema.org_users.org_id, binding.org_id), + sql`${schema.org_users.app_id} IS NULL`, + sql`${schema.org_users.channel_id} IS NULL`, + sql`(${schema.org_users.user_right} IS NULL OR ${schema.org_users.user_right}::text NOT LIKE 'invite_%')`, + ), + ) + } + }) cloudlog({ requestId: c.get('requestId'), diff --git a/supabase/migrations/20260502134355_fix_rbac_role_binding_demoted_super_admin.sql b/supabase/migrations/20260502134355_fix_rbac_role_binding_demoted_super_admin.sql new file mode 100644 index 0000000000..44ce588088 --- /dev/null +++ b/supabase/migrations/20260502134355_fix_rbac_role_binding_demoted_super_admin.sql @@ -0,0 +1,160 @@ +-- Fix GHSA-rvvc-rvxv-qcrh: +-- Authorize encrypted-bundle cleanup RPCs through RBAC instead of stale legacy rights. + +CREATE OR REPLACE FUNCTION "public"."count_non_compliant_bundles"( + "org_id" uuid, + "required_key" text DEFAULT NULL +) RETURNS TABLE ( + "non_encrypted_count" bigint, + "wrong_key_count" bigint, + "total_non_compliant" bigint +) +LANGUAGE "plpgsql" +SECURITY DEFINER +SET "search_path" TO '' +AS $$ +DECLARE + non_encrypted bigint := 0; + wrong_key bigint := 0; + caller_user_id uuid; + api_key_text text; +BEGIN + SELECT public.get_identity('{read,upload,write,all}'::public.key_mode[]) INTO caller_user_id; + SELECT public.get_apikey_header() INTO api_key_text; + + IF caller_user_id IS NULL THEN + RAISE EXCEPTION 'Unauthorized: Authentication required'; + END IF; + + -- org.delete is the RBAC/legacy super_admin-equivalent org gate. Using it + -- preserves the previous super_admin-only requirement for this org-wide scan. + IF NOT public.rbac_check_permission_direct( + public.rbac_perm_org_delete(), + caller_user_id, + count_non_compliant_bundles.org_id, + NULL::character varying, + NULL::bigint, + api_key_text + ) THEN + RAISE EXCEPTION 'Unauthorized: Only super_admin can access this function'; + END IF; + + SELECT COUNT(*) INTO non_encrypted + FROM public.app_versions av + INNER JOIN public.apps a ON a.app_id = av.app_id + WHERE a.owner_org = count_non_compliant_bundles.org_id + AND av.deleted = false + AND (av.session_key IS NULL OR av.session_key = ''); + + IF required_key IS NOT NULL AND required_key <> '' THEN + SELECT COUNT(*) INTO wrong_key + FROM public.app_versions av + INNER JOIN public.apps a ON a.app_id = av.app_id + WHERE a.owner_org = count_non_compliant_bundles.org_id + AND av.deleted = false + AND av.session_key IS NOT NULL + AND av.session_key <> '' + AND ( + av.key_id IS NULL + OR av.key_id = '' + -- key_id can store either the 20-char required_key prefix or the full key, so accept both match directions. + OR NOT (av.key_id = LEFT(required_key, 20) OR LEFT(av.key_id, LENGTH(required_key)) = required_key) + ); + END IF; + + RETURN QUERY SELECT non_encrypted, wrong_key, (non_encrypted + wrong_key); +END; +$$; + +ALTER FUNCTION "public"."count_non_compliant_bundles"(uuid, text) OWNER TO "postgres"; +REVOKE ALL ON FUNCTION "public"."count_non_compliant_bundles"(uuid, text) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION "public"."count_non_compliant_bundles"(uuid, text) TO "authenticated"; +GRANT EXECUTE ON FUNCTION "public"."count_non_compliant_bundles"(uuid, text) TO "service_role"; + +CREATE OR REPLACE FUNCTION "public"."delete_non_compliant_bundles"( + "org_id" uuid, + "required_key" text DEFAULT NULL +) RETURNS bigint +LANGUAGE "plpgsql" +SECURITY DEFINER +SET "search_path" TO '' +AS $$ +DECLARE + deleted_count bigint := 0; + bundle_ids bigint[]; + caller_user_id uuid; + api_key_text text; +BEGIN + SELECT public.get_identity('{read,upload,write,all}'::public.key_mode[]) INTO caller_user_id; + SELECT public.get_apikey_header() INTO api_key_text; + + IF caller_user_id IS NULL THEN + RAISE EXCEPTION 'Unauthorized: Authentication required'; + END IF; + + -- org.delete is the RBAC/legacy super_admin-equivalent org gate. Using it + -- preserves the previous super_admin-only requirement for this destructive cleanup. + IF NOT public.rbac_check_permission_direct( + public.rbac_perm_org_delete(), + caller_user_id, + delete_non_compliant_bundles.org_id, + NULL::character varying, + NULL::bigint, + api_key_text + ) THEN + RAISE EXCEPTION 'Unauthorized: Only super_admin can access this function'; + END IF; + + IF required_key IS NULL OR required_key = '' THEN + SELECT ARRAY_AGG(av.id) INTO bundle_ids + FROM public.app_versions av + INNER JOIN public.apps a ON a.app_id = av.app_id + WHERE a.owner_org = delete_non_compliant_bundles.org_id + AND av.deleted = false + AND (av.session_key IS NULL OR av.session_key = ''); + ELSE + SELECT ARRAY_AGG(av.id) INTO bundle_ids + FROM public.app_versions av + INNER JOIN public.apps a ON a.app_id = av.app_id + WHERE a.owner_org = delete_non_compliant_bundles.org_id + AND av.deleted = false + AND ( + (av.session_key IS NULL OR av.session_key = '') + OR ( + av.session_key IS NOT NULL + AND av.session_key <> '' + AND ( + av.key_id IS NULL + OR av.key_id = '' + -- key_id can store either the 20-char required_key prefix or the full key, so accept both match directions. + OR NOT (av.key_id = LEFT(required_key, 20) OR LEFT(av.key_id, LENGTH(required_key)) = required_key) + ) + ) + ); + END IF; + + IF bundle_ids IS NOT NULL AND array_length(bundle_ids, 1) > 0 THEN + UPDATE public.app_versions + SET deleted = true + WHERE id = ANY(bundle_ids); + + deleted_count := array_length(bundle_ids, 1); + + PERFORM public.pg_log('action: DELETED_NON_COMPLIANT_BUNDLES', + jsonb_build_object( + 'org_id', org_id, + 'required_key', required_key, + 'deleted_count', deleted_count, + 'bundle_ids', bundle_ids, + 'caller_user_id', caller_user_id + )); + END IF; + + RETURN deleted_count; +END; +$$; + +ALTER FUNCTION "public"."delete_non_compliant_bundles"(uuid, text) OWNER TO "postgres"; +REVOKE ALL ON FUNCTION "public"."delete_non_compliant_bundles"(uuid, text) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION "public"."delete_non_compliant_bundles"(uuid, text) TO "authenticated"; +GRANT EXECUTE ON FUNCTION "public"."delete_non_compliant_bundles"(uuid, text) TO "service_role"; diff --git a/tests/enforce-encrypted-bundles.test.ts b/tests/enforce-encrypted-bundles.test.ts index 9b447b8e95..e019ca0a42 100644 --- a/tests/enforce-encrypted-bundles.test.ts +++ b/tests/enforce-encrypted-bundles.test.ts @@ -1,5 +1,6 @@ +import { randomUUID } from 'node:crypto' import { afterAll, beforeAll, describe, expect, it } from 'vitest' -import { APIKEY_ENCRYPTED, APP_NAME_ENCRYPTED, getAuthHeadersForCredentials, getEndpointUrl, getSupabaseClient, ORG_ID_ENCRYPTED, USER_ID_ENCRYPTED, USER_PASSWORD } from './test-utils.ts' +import { APIKEY_ENCRYPTED, APP_NAME_ENCRYPTED, getAuthHeadersForCredentials, getEndpointUrl, getSupabaseClient, ORG_ID_ENCRYPTED, SUPABASE_ANON_KEY, SUPABASE_BASE_URL, USER_ID, USER_ID_2, USER_ID_ENCRYPTED, USER_PASSWORD } from './test-utils.ts' // This test file uses ISOLATED test data seeded in seed.sql: // - USER_ID_ENCRYPTED: f6a7b8c9-d0e1-4f2a-9b3c-4d5e6f708193 @@ -13,10 +14,131 @@ const headersEncrypted = { 'Authorization': APIKEY_ENCRYPTED, } const USER_EMAIL_ENCRYPTED = 'encrypted@capgo.app' +const USER_EMAIL_2 = 'test2@capgo.app' let authHeadersEncrypted: Record +let authHeadersUser2: Record + +async function createStaleLegacySuperAdminFixture() { + const id = randomUUID() + const orgId = randomUUID() + const appUuid = randomUUID() + const appId = `com.encrypted.rbac-stale.${id}` + const bundleName = `stale-rbac-${id}` + const supabase = getSupabaseClient() + + const { error: orgError } = await supabase.from('orgs').insert({ + id: orgId, + created_by: USER_ID, + name: `Encrypted RBAC Stale Org ${id}`, + management_email: `encrypted-rbac-stale-${id}@capgo.app`, + use_new_rbac: true, + }) + if (orgError) + throw orgError + + const { error: appError } = await supabase.from('apps').insert({ + id: appUuid, + app_id: appId, + owner_org: orgId, + icon_url: 'encrypted-rbac-stale-icon', + name: `Encrypted RBAC Stale App ${id}`, + }) + if (appError) + throw appError + + const { data: memberRole, error: roleError } = await supabase + .from('roles') + .select('id') + .eq('name', 'org_member') + .single() + if (roleError) + throw roleError + + const { error: memberError } = await supabase.from('org_users').insert({ + org_id: orgId, + user_id: USER_ID_2, + user_right: 'read', + rbac_role_name: 'org_member', + }) + if (memberError) + throw memberError + + await supabase + .from('role_bindings') + .delete() + .eq('principal_type', 'user') + .eq('principal_id', USER_ID_2) + .eq('org_id', orgId) + + const { error: bindingError } = await supabase.from('role_bindings').insert({ + principal_type: 'user', + principal_id: USER_ID_2, + role_id: memberRole!.id, + scope_type: 'org', + org_id: orgId, + granted_by: USER_ID, + reason: 'stale legacy super-admin regression', + is_direct: true, + }) + if (bindingError) + throw bindingError + + const { error: staleLegacyError } = await supabase + .from('org_users') + .update({ user_right: 'super_admin' }) + .eq('org_id', orgId) + .eq('user_id', USER_ID_2) + .is('app_id', null) + .is('channel_id', null) + if (staleLegacyError) + throw staleLegacyError + + const { data: version, error: versionError } = await supabase + .from('app_versions') + .insert({ + app_id: appId, + name: bundleName, + checksum: `stale-rbac-${id}`, + owner_org: orgId, + user_id: USER_ID, + storage_provider: 'r2', + r2_path: `orgs/${orgId}/apps/${appId}/${bundleName}.zip`, + deleted: false, + }) + .select('id') + .single() + if (versionError) + throw versionError + + return { + orgId, + appId, + appUuid, + versionId: version!.id, + cleanup: async () => { + await supabase.from('app_versions').delete().eq('owner_org', orgId) + await supabase.from('role_bindings').delete().eq('org_id', orgId) + await supabase.from('org_users').delete().eq('org_id', orgId) + await supabase.from('apps').delete().eq('id', appUuid) + await supabase.from('orgs').delete().eq('id', orgId) + }, + } +} + +async function callBundleCleanupRpc(functionName: 'count_non_compliant_bundles' | 'delete_non_compliant_bundles', orgId: string) { + return fetch(`${SUPABASE_BASE_URL}/rest/v1/rpc/${functionName}`, { + method: 'POST', + headers: { + ...authHeadersUser2, + apikey: SUPABASE_ANON_KEY, + }, + body: JSON.stringify({ org_id: orgId }), + }) +} beforeAll(async () => { authHeadersEncrypted = await getAuthHeadersForCredentials(USER_EMAIL_ENCRYPTED, USER_PASSWORD) + authHeadersUser2 = await getAuthHeadersForCredentials(USER_EMAIL_2, USER_PASSWORD) // Ensure enforcement is disabled at the start of tests await getSupabaseClient() @@ -447,6 +569,37 @@ describe('[Encrypted Bundles Enforcement]', () => { }) }) + describe('cleanup RPC authorization', () => { + it('rejects stale legacy super_admin when RBAC only grants org_member', async () => { + const fixture = await createStaleLegacySuperAdminFixture() + const supabase = getSupabaseClient() + + try { + const countResponse = await callBundleCleanupRpc('count_non_compliant_bundles', fixture.orgId) + const countData = await countResponse.json() as { message?: string, error?: string } + expect(countResponse.status).not.toBe(200) + expect(countData.message ?? countData.error).toContain('Unauthorized') + + const deleteResponse = await callBundleCleanupRpc('delete_non_compliant_bundles', fixture.orgId) + const deleteData = await deleteResponse.json() as { message?: string, error?: string } + expect(deleteResponse.status).not.toBe(200) + expect(deleteData.message ?? deleteData.error).toContain('Unauthorized') + + const { data: version, error: versionError } = await supabase + .from('app_versions') + .select('deleted') + .eq('id', fixture.versionId) + .single() + + expect(versionError).toBeNull() + expect(version?.deleted).toBe(false) + } + finally { + await fixture.cleanup() + } + }) + }) + describe('get_orgs_v7 includes enforce_encrypted_bundles', () => { it('should include enforce_encrypted_bundles field in get_orgs_v7 response', async () => { // Use get_orgs_v7 with the encrypted user diff --git a/tests/private-role-bindings.test.ts b/tests/private-role-bindings.test.ts index 7532b01c27..33e44939b9 100644 --- a/tests/private-role-bindings.test.ts +++ b/tests/private-role-bindings.test.ts @@ -129,7 +129,7 @@ beforeAll(async () => { }) // /private/role_bindings is currently served by the Supabase private functions stack, not the Cloudflare API worker. -describe.skipIf(USE_CLOUDFLARE)('[POST] /private/role_bindings', () => { +describe.skipIf(USE_CLOUDFLARE)('/private/role_bindings', () => { it('accepts channel-scoped bindings when channel_id is the channel RBAC uuid', async () => { const fixture = await createRoleBindingFixture() @@ -206,6 +206,89 @@ describe.skipIf(USE_CLOUDFLARE)('[POST] /private/role_bindings', () => { await fixture.cleanup() } }) + + it('clears legacy org_users rights when an org-scope user binding is deleted', async () => { + const id = randomUUID() + const orgId = randomUUID() + const supabase = getSupabaseClient() + + try { + const { error: orgError } = await supabase.from('orgs').insert({ + id: orgId, + created_by: USER_ID, + name: `Role Binding Delete Org ${id}`, + management_email: `role-binding-delete-${id}@capgo.app`, + use_new_rbac: true, + }) + expect(orgError).toBeNull() + + const { error: memberError } = await supabase.from('org_users').insert({ + org_id: orgId, + user_id: USER_ID_2, + user_right: 'super_admin', + rbac_role_name: 'org_super_admin', + }) + expect(memberError).toBeNull() + + const { data: superAdminRole, error: roleError } = await supabase + .from('roles') + .select('id') + .eq('name', 'org_super_admin') + .single() + expect(roleError).toBeNull() + + const { error: bindingInsertError } = await supabase.from('role_bindings').insert({ + principal_type: 'user', + principal_id: USER_ID_2, + role_id: superAdminRole!.id, + scope_type: 'org', + org_id: orgId, + granted_by: USER_ID, + reason: 'delete advisory regression', + is_direct: true, + }) + expect(bindingInsertError).toBeNull() + + const { data: binding, error: bindingError } = await supabase + .from('role_bindings') + .select('id') + .eq('principal_type', 'user') + .eq('principal_id', USER_ID_2) + .eq('scope_type', 'org') + .eq('org_id', orgId) + .single() + + expect(bindingError).toBeNull() + expect(binding?.id).toBeTruthy() + + const deleteResponse = await fetch(getEndpointUrl(`/private/role_bindings/${binding!.id}`), { + method: 'DELETE', + headers: authHeaders, + }) + + const deleteData = await deleteResponse.json() as { success?: boolean, error?: string } + expect(deleteResponse.status).toBe(200) + expect(deleteData.success).toBe(true) + + const { data: legacyRows, error: legacyError } = await supabase + .from('org_users') + .select('user_right, rbac_role_name') + .eq('org_id', orgId) + .eq('user_id', USER_ID_2) + .is('app_id', null) + .is('channel_id', null) + + expect(legacyError).toBeNull() + expect(legacyRows).toHaveLength(1) + expect(legacyRows?.[0]?.user_right).toBeNull() + expect(legacyRows?.[0]?.rbac_role_name).toBeNull() + } + finally { + await supabase.from('role_bindings').delete().eq('org_id', orgId) + await supabase.from('org_users').delete().eq('org_id', orgId) + await supabase.from('orgs').delete().eq('id', orgId) + } + }) }) describe('private role bindings helpers', () => {