fix(db): T35-T37d — segurança completa: RLS, default privileges e 322 violações SECURITY DEFINER eliminadas#169
Conversation
…rivileges T35: A política T25b introduziu um subquery corrompido que referenciava a coluna da tabela externa (material_groups.organization_id) dentro do FROM profiles, fazendo a condição ser sempre verdadeira → exposição cross-tenant. Substituído pelo padrão current_setting correto em todas as 4 políticas (mg_select, mg_insert, mg_update, mg_delete). T36: ALTER DEFAULT PRIVILEGES para que funções futuras criadas pelo role postgres no schema public não herdem EXECUTE do PUBLIC; apenas authenticated e service_role recebem EXECUTE por padrão. Ambas as migrações já foram aplicadas ao banco via MCP apply_migration. https://claude.ai/code/session_01PTSGxhx3TFtkEbjH9Fq1o6
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughEsse PR consolida duas semanas de auditoria de segurança Postgres: (1) corrige vulnerabilidade RLS em material_groups que vazava dados entre orgs, e (2) refatora 160+ funções públicas para reduzir superfície de ataque, convertendo funções seguras a SECURITY INVOKER e revogando acesso de ChangesCorreção de Vulnerabilidade RLS
Endurecimento Sistemático de Funções Postgres
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Justificativa: Mudanças de segurança sistematicamente aplicadas (5 migrations SQL claras), mas exigem validação de:
Baixa densidade lógica por linha (enumerações de ALTER/REVOKE), mas alto impacto de segurança. Possibly Related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…iner_function_executable Reduz authenticated_security_definer_function_executable de 322 → 0: T37a (63 funções): Converte funções sem acesso direto a tabelas para SECURITY INVOKER T37c (49 funções): Revoga EXECUTE de authenticated em funções trigger (45) e vault (4) T37b (156 funções): Converte funções candidate seguras para SECURITY INVOKER T37d (54 funções): Revoga EXECUTE de authenticated em funções cron/backend/admin Critérios de conversão para INVOKER: sem acesso cross-tenant, sem bypass de RLS, sem acesso ao schema vault. Funções trigger permanecem SECURITY DEFINER (correto) mas não precisam ser executáveis pelo role authenticated. https://claude.ai/code/session_01PTSGxhx3TFtkEbjH9Fq1o6
adm01-debug
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6000b9c39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ALTER FUNCTION public.get_top_favorited_products(integer, integer) SECURITY INVOKER; | ||
| ALTER FUNCTION public.get_unread_count() SECURITY INVOKER; | ||
| ALTER FUNCTION public.get_upcoming_commemorative_dates(integer) SECURITY INVOKER; | ||
| ALTER FUNCTION public.get_user_org_ids(uuid) SECURITY INVOKER; |
There was a problem hiding this comment.
When an authenticated user reads organizations or organization_members, this helper now runs as the caller and its SELECT organization_id FROM public.organization_members is subject to RLS. The existing organization_members SELECT policy calls public.get_user_org_ids(auth.uid()) again, and the original migration explicitly made this function SECURITY DEFINER to avoid that recursion, so this change will make org membership reads fail with recursive RLS instead of returning the user's orgs.
Useful? React with 👍 / 👎.
| ALTER FUNCTION public.is_org_member(uuid, uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.is_org_owner_or_admin(uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.is_supervisor_or_above(uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.log_access_denied(text, text, text, text) SECURITY INVOKER; |
There was a problem hiding this comment.
For non-supervisor users who hit a guarded route, the frontend calls this RPC specifically as a server-side audit trail, but the function inserts into admin_audit_log while that table's INSERT policy only allows public.is_supervisor_or_above(auth.uid()). The original definition notes it must be SECURITY DEFINER to write through restrictive RLS, so changing it to invoker causes exactly the denied users we want to audit to silently lose the server-side audit entry.
Useful? React with 👍 / 👎.
| ALTER FUNCTION public.registrar_entrada_estoque(character varying, integer, numeric, character varying, character varying, text, uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.registrar_saida_estoque(character varying, integer, character varying, character varying, text, uuid, boolean) SECURITY INVOKER; | ||
| ALTER FUNCTION public.repair_ownership_orphans(uuid, boolean, text) SECURITY INVOKER; | ||
| ALTER FUNCTION public.request_step_up_challenge(step_up_action, text, inet, text) SECURITY INVOKER; |
There was a problem hiding this comment.
When step-up-verify calls request_step_up_challenge through the user's JWT, this function must insert into step_up_challenges and step_up_audit_log, but those tables only have SELECT policies for users and the original migration states that nobody inserts/updates them directly except via SECURITY DEFINER RPCs. After this conversion the first step of the step-up flow will fail under RLS instead of creating the OTP challenge.
Useful? React with 👍 / 👎.
| ALTER FUNCTION public.user_can_skip_step_up(uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.user_is_org_member(uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.validate_mcp_key(text) SECURITY INVOKER; | ||
| ALTER FUNCTION public.verify_step_up_otp(uuid, text) SECURITY INVOKER; |
There was a problem hiding this comment.
The OTP verification RPC is invoked with the user's JWT and then updates step_up_challenges, inserts into step_up_tokens, and writes step_up_audit_log; those tables expose only user SELECT policies, with no INSERT/UPDATE policy for authenticated callers. Converting this function to invoker means a correct OTP can no longer issue a token because the database writes are rejected by RLS.
Useful? React with 👍 / 👎.
| ALTER FUNCTION public.clear_user_token_revocations(uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.compare_quote_snapshots(uuid, integer, integer) SECURITY INVOKER; | ||
| ALTER FUNCTION public.compute_quote_snapshot_hash(uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.consume_step_up_token(text, step_up_action, text) SECURITY INVOKER; |
There was a problem hiding this comment.
The MCP key edge functions call consume_step_up_token with the user's JWT, and this RPC must mark the matching row in step_up_tokens as consumed and append audit rows. Since step_up_tokens and step_up_audit_log have no authenticated UPDATE/INSERT policies, running as invoker makes valid step-up tokens fail at consumption time, blocking issue/rotate/revoke flows that rely on this gate.
Useful? React with 👍 / 👎.
| ALTER FUNCTION public.is_admin() SECURITY INVOKER; | ||
| ALTER FUNCTION public.is_admin(uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.is_admin_or_above(uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.is_admin_strict(uuid) SECURITY INVOKER; |
There was a problem hiding this comment.
is_admin_strict reads public.user_roles, but it is also used inside the user_roles INSERT/UPDATE/DELETE policies that decide whether role edits are allowed. Running it as the caller makes those policy checks recurse back through RLS on user_roles, so dev/admin users can no longer pass the role-management policies that this helper was created to evaluate.
Useful? React with 👍 / 👎.
|
|
||
| -- SECURITY AUTOMATION / CRON | ||
| REVOKE EXECUTE ON FUNCTION public.auto_block_extreme_offenders() FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.check_telemetry_regression() FROM authenticated; |
There was a problem hiding this comment.
The admin telemetry UI still calls check_telemetry_regression directly from the browser Supabase client in both useRegressionGuardrail and useOptimizationQueue. Revoking EXECUTE from authenticated makes that dashboard report permission errors instead of the guardrail status for logged-in admins.
Useful? React with 👍 / 👎.
| REVOKE EXECUTE ON FUNCTION public.claim_next_optimization() FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.complete_optimization(uuid, text, text, text, jsonb, text) FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.enqueue_optimization(text, text, text, integer) FROM authenticated; |
There was a problem hiding this comment.
src/pages/admin/telemetry/useOptimizationQueue.ts calls these RPCs from the authenticated browser client to enqueue, claim, and complete optimization jobs. After this revoke, the admin queue controls will fail with permission errors even for authorized users because the frontend no longer has EXECUTE on the functions it invokes.
Useful? React with 👍 / 👎.
| REVOKE EXECUTE ON FUNCTION public.process_supplier_product(uuid, jsonb, text) FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.process_supplier_products_batch(uuid, integer) FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.sincronizar_estoque_spot(jsonb, uuid) FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.sync_external_connections_from_credentials() FROM authenticated; |
There was a problem hiding this comment.
LastSyncRunPanel runs supabase.rpc("sync_external_connections_from_credentials") from the authenticated admin UI when the user clicks the manual sync action. Revoking the no-arg overload from authenticated turns that button into a permission failure rather than starting the sync.
Useful? React with 👍 / 👎.
| REVOKE EXECUTE ON FUNCTION public.purge_old_audit_logs() FROM authenticated; | ||
|
|
||
| -- ADMIN BULK / DEV | ||
| REVOKE EXECUTE ON FUNCTION public.execute_role_migration_batch(text, text, jsonb, boolean) FROM authenticated; |
There was a problem hiding this comment.
useRoleMigration submits role migration batches through supabase.rpc("execute_role_migration_batch", ...) using the authenticated frontend client. Removing EXECUTE for authenticated blocks that admin/dev workflow before the function's own authorization logic can run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR applies a set of Supabase security migrations (T35–T37d) aimed at fixing a critical RLS issue, tightening future function default privileges, and eliminating advisor findings related to SECURITY DEFINER functions being executable by authenticated.
Changes:
- Fixes
material_groupsRLS policies to enforce tenant isolation viacurrent_setting('app.current_org_id'). - Sets default privileges so newly created
publicfunctions don’t inheritEXECUTEfromPUBLIC(granting only toauthenticatedandservice_role). - Reduces
authenticated_security_definer_function_executablefindings by converting many functions toSECURITY INVOKERand revokingEXECUTEfromauthenticatedfor backend/trigger/cron-only functions.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/migrations/20260512000013_t35_fix_material_groups_rls_broken_subquery.sql | Replaces broken material_groups RLS policy condition with the standard current_setting('app.current_org_id') pattern. |
| supabase/migrations/20260512000014_t36_alter_default_privileges_functions.sql | Updates default privileges for future public-schema functions created by postgres to avoid PUBLIC execute inheritance. |
| supabase/migrations/20260513000001_t37a_security_invoker_safe_batch.sql | Converts a “safe” batch of functions from SECURITY DEFINER to SECURITY INVOKER. |
| supabase/migrations/20260513000002_t37c_revoke_authenticated_trigger_vault.sql | Revokes EXECUTE from authenticated for trigger and vault functions. |
| supabase/migrations/20260513000003_t37b1_security_invoker_candidate_batch1.sql | Converts a first batch of candidate functions to SECURITY INVOKER (but includes signature and privilege-impact risks). |
| supabase/migrations/20260513000004_t37b2_security_invoker_candidate_batch2.sql | Converts a second batch of candidate functions to SECURITY INVOKER. |
| supabase/migrations/20260513000005_t37d_revoke_authenticated_cron_backend.sql | Revokes EXECUTE from authenticated for cron/backend/admin-only functions (but currently includes functions used by the admin frontend). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| -- SECURITY AUTOMATION / CRON | ||
| REVOKE EXECUTE ON FUNCTION public.auto_block_extreme_offenders() FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.check_telemetry_regression() FROM authenticated; |
| -- OPTIMIZATION QUEUE (backend workers) | ||
| REVOKE EXECUTE ON FUNCTION public.claim_next_optimization() FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.complete_optimization(uuid, text, text, text, jsonb, text) FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.enqueue_optimization(text, text, text, integer) FROM authenticated; |
| REVOKE EXECUTE ON FUNCTION public.process_supplier_products_batch(uuid, integer) FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.sincronizar_estoque_spot(jsonb, uuid) FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.sync_external_connections_from_credentials() FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.sync_external_connections_from_credentials(text, text, uuid) FROM authenticated; | ||
| REVOKE EXECUTE ON FUNCTION public.update_image_after_sync(uuid, character varying, text, bigint, integer, integer, character varying) FROM authenticated; |
| REVOKE EXECUTE ON FUNCTION public.purge_old_audit_logs() FROM authenticated; | ||
|
|
||
| -- ADMIN BULK / DEV | ||
| REVOKE EXECUTE ON FUNCTION public.execute_role_migration_batch(text, text, jsonb, boolean) FROM authenticated; |
| ALTER FUNCTION public.check_ai_quota(uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.check_auth_throttling(text, text) SECURITY INVOKER; | ||
| ALTER FUNCTION public.check_geo_country_allowed(text) SECURITY INVOKER; | ||
| ALTER FUNCTION public.check_hardening_status() SECURITY INVOKER; |
| ALTER FUNCTION public.get_bundle_suggestions(uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.get_category_descendants(uuid) SECURITY INVOKER; | ||
| ALTER FUNCTION public.get_client_seasonality(uuid, integer) SECURITY INVOKER; | ||
| ALTER FUNCTION public.get_client_top_products(uuid, integer) SECURITY INVOKER; |
| ALTER FUNCTION public.get_industry_benchmark_stats(uuid[], integer) SECURITY INVOKER; | ||
| ALTER FUNCTION public.get_industry_seasonality(uuid[], integer) SECURITY INVOKER; | ||
| ALTER FUNCTION public.get_industry_top_products(uuid[], integer, integer) SECURITY INVOKER; |
Resumo
Esta PR contém todas as migrations de segurança T35 a T37d, eliminando todas as violações restantes do Supabase advisor e atingindo a meta 10/10.
T35 — Correção crítica de segurança: RLS quebrado em
material_groupsA migração T25b introduziu um subquery corrompido nas 4 políticas de
material_groups. O bug tornava a condição sempre verdadeira → cross-tenant data exposure.Corrigido: substituído por
current_setting('app.current_org_id')nas 4 policies (mg_select,mg_insert,mg_update,mg_delete).T36 —
ALTER DEFAULT PRIVILEGESpara funções futurasGarante que funções criadas futuramente pelo role
postgresnão herdemEXECUTEdoPUBLIC. Complementa T33b.T37a-T37d — Elimina 322 violações
authenticated_security_definer_function_executableResultado: 322 → 0 violações.
Critérios aplicados
authenticated: funções trigger (nunca chamadas via RPC), vault (backend/service_role only), cron/manutenção (pg_cron)authenticatedmantém EXECUTE em todas as RPCs de frontend — nenhuma quebra de compatibilidadePlano de testes
authenticated_security_definer_function_executable= 0https://claude.ai/code/session_01PTSGxhx3TFtkEbjH9Fq1o6
Summary by CodeRabbit
Release Notes
Bug Fixes
Security