Skip to content

feat(security): hardening de create_organization_with_owner (REVOKE PUBLIC + limite)#8

Merged
adm01-debug merged 1 commit into
mainfrom
qa/secure-create-organization-rpc
May 19, 2026
Merged

feat(security): hardening de create_organization_with_owner (REVOKE PUBLIC + limite)#8
adm01-debug merged 1 commit into
mainfrom
qa/secure-create-organization-rpc

Conversation

@adm01-debug
Copy link
Copy Markdown
Owner

@adm01-debug adm01-debug commented May 19, 2026

O que

Hardening de segurança da RPC create_organization_with_owner criada em 19/mai 11:45 (migration 20260519114532). Adiciona REVOKE PUBLIC, GRANT explícito a authenticated, limite de orgs por usuário e validação de input.

Problemas que existiam

1. Default GRANT TO PUBLIC

Em PostgreSQL, funções têm GRANT EXECUTE TO PUBLIC por padrão. A migration original criou a função sem REVOKE, então qualquer authenticated podia chamar. O check interno IF auth.uid() IS NULL THEN RAISE EXCEPTION bloqueia anon, mas:

  • Estilisticamente fraco (defense in depth ausente)
  • Dependente de detalhe de implementação (e se um dia alguém remover esse check?)

2. Sem limite de orgs por usuário

Qualquer usuário autenticado podia chamar create_organization_with_owner quantas vezes quisesse → criava N organizações sem cap. Vetor de abuso:

  • Inflar tabela organizations (custo de DB)
  • Confundir analytics
  • Em SaaS multi-tenant futuro: ocupar slugs/nomes desejáveis

Fix

CREATE OR REPLACE preservando assinatura, SECURITY DEFINER e search_path = public. Adiciona:

Adição Por quê
REVOKE EXECUTE FROM PUBLIC + FROM anon + GRANT TO authenticated Defense in depth: lockdown explícito de quem pode chamar
Limite de 5 orgs/usuário como owner (constant) Cap razoável para abuso. Configurável via futura tabela de settings se precisar.
Validação de input (name e slug não-vazios, com ERRCODE 22023) Mensagem clara para o frontend
Checagem de slug duplicado explícita (ERRCODE 23505) Mensagem amigável em vez de "duplicate key violation" genérico
RAISE NOTICE em sucesso Audit trail via Postgres logs

Risco

Baixo. CREATE OR REPLACE mantém assinatura (text, text) → uuid. Apenas adiciona validação e lockdown. Chamadas legítimas:

  • Frontend usa via Supabase RPC com user logado → authenticated → tem GRANT → funciona
  • Usuário com <5 orgs → passa o cap → funciona
  • Slug único → passa a checagem → funciona

Bloqueia somente abusos.

Validação local

-- Confirmar permissões
SELECT proname, proacl FROM pg_proc 
WHERE proname = 'create_organization_with_owner';

-- Esperado em proacl:
--   authenticated=X/postgres   (GRANT explícito)
-- NÃO deve aparecer:
--   public=X/postgres ou anon=X/postgres

Aplicação

Migration 20260519230000_* será aplicada na Fase 3 junto com as outras. Lovable não toca este arquivo. Sem efeito imediato no banco em uso até F3.


Summary by cubic

Hardens the create_organization_with_owner RPC by restricting execution to authenticated users, validating inputs, and adding a per-user org limit. Keeps the same function signature and preserves valid use cases.

  • Bug Fixes
    • Revoked EXECUTE from PUBLIC and anon; granted to authenticated.
    • Added cap of 5 organizations per user as owner.
    • Validated name and slug (non-empty) with clear errors (22023).
    • Explicit slug conflict check with friendly error (23505).
    • Kept SECURITY DEFINER and search_path=public; CREATE OR REPLACE maintains (text, text) -> uuid.

Written for commit e1d758a. Summary will update on new commits. Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Reforçada validação ao criar organizações, garantindo nomes e identificadores válidos.
    • Implementado limite de 5 organizações por proprietário para melhor controle.
    • Melhorada detecção de conflitos de identificadores duplicados.
  • Chores

    • Endurecidas permissões de acesso, exigindo autenticação obrigatória.
    • Adicionado registro de auditoria para rastreamento de criação de organizações.

Review Change Stack

RPC criada em 19/mai 11:45 (migration 20260519114532) tinha 2 problemas:
  1. Sem REVOKE EXECUTE FROM PUBLIC (todo authenticated podia chamar)
  2. Sem limite de orgs por usuario (atacante criava N orgs sem cap)

Fix (CREATE OR REPLACE preservando assinatura/SECURITY DEFINER/search_path):
  1. REVOKE EXECUTE FROM PUBLIC + REVOKE FROM anon + GRANT TO authenticated
  2. Limite: max 5 organizacoes/usuario como owner (constant)
  3. Validacao de input (name e slug nao-vazios, ERRCODE 22023)
  4. Checagem explicita de conflito de slug (ERRCODE 23505 com msg clara)
  5. RAISE NOTICE em sucesso (audit trail via Postgres logs)

Risco: baixo. CREATE OR REPLACE mantem assinatura. Apenas adiciona
validacao e lockdown — nao quebra chamadas legitimas.
Copilot AI review requested due to automatic review settings May 19, 2026 15:41
@vercel
Copy link
Copy Markdown

vercel Bot commented May 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
we-dream-big Ready Ready Preview, Comment May 19, 2026 3:42pm

@adm01-debug adm01-debug merged commit 74c2130 into main May 19, 2026
3 of 17 checks passed
@adm01-debug adm01-debug deleted the qa/secure-create-organization-rpc branch May 19, 2026 15:41
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 765d71b4-8bd4-42a5-a6f0-36fd4352e746

📥 Commits

Reviewing files that changed from the base of the PR and between c8a5da6 and e1d758a.

📒 Files selected for processing (1)
  • supabase/migrations/20260519230000_harden_create_organization_with_owner.sql

Visão Geral

Migração SQL que endurece a função create_organization_with_owner, adicionando requisito de autenticação, validações de entrada, limite de organizações por proprietário, verificação de slug único, operações atômicas no banco e restrição de permissões apenas para usuários autenticados.

Mudanças

Database hardening — Organization creation

Layer / File(s) Resumo
Hardened organization creation function
supabase/migrations/20260519230000_harden_create_organization_with_owner.sql
Função create_organization_with_owner(text, text) agora requer auth.uid(), valida name/slug não-vazios, impõe limite de 5 orgs/owner por usuário, verifica conflito de slug antes de INSERT, executa criação de org e membership de forma atômica, registra auditoria via RAISE NOTICE, e restringe EXECUTE apenas a authenticated (revogando de PUBLIC e anon).

Estimativa de Esforço

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possíveis PRs Relacionadas

  • adm01-debug/Promo_Gifts#225: Também endurece função SECURITY DEFINER (org_has_any_members) revogando EXECUTE de PUBLIC/anon e concedendo apenas a authenticated.
  • adm01-debug/Promo_Gifts#169: Altera a mesma função create_organization_with_owner para SECURITY INVOKER, criando conflito direto de alteração do mesmo objeto.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens the create_organization_with_owner RPC by adding explicit permission lockdown, input validation, a per-user organization cap, and friendlier error messages for slug conflicts.

Changes:

  • Recreates the function via CREATE OR REPLACE preserving signature/SECURITY DEFINER/search_path, adding input validation and a 5-org-per-owner cap.
  • Adds explicit slug-conflict check with ERRCODE 23505 and a RAISE NOTICE audit line.
  • Revokes EXECUTE from PUBLIC/anon and grants it to authenticated.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e1d758a88c

ℹ️ 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".

Comment on lines +60 to +63
SELECT COUNT(*) INTO _existing_owner_count
FROM public.organization_members
WHERE user_id = current_user_id
AND role = 'owner';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Serialize org-cap check to prevent concurrent bypass

The new cap enforcement can be bypassed with concurrent RPC calls because it does SELECT COUNT(*) and then conditionally inserts without any locking/serialization. Under PostgreSQL's default READ COMMITTED behavior, two requests from the same user can both observe _existing_owner_count = 4 and both proceed, resulting in 6 owner orgs despite _max_orgs_per_user = 5. This weakens the abuse-control goal of this migration in production when clients retry or parallelize requests.

Useful? React with 👍 / 👎.

END IF;

-- 4. Conflito de slug: erro melhor que constraint genérica
IF EXISTS (SELECT 1 FROM public.organizations WHERE slug = _slug) THEN
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check slug conflict against normalized slug

The duplicate-slug precheck compares the raw _slug, but the insert stores trim(_slug). For inputs with leading/trailing spaces (for example, _slug = 'acme ' when acme already exists), this check misses the conflict and the function falls through to a generic unique-constraint failure instead of the intended friendly error path. This regression was introduced by adding normalization only at insert time.

Useful? React with 👍 / 👎.

adm01-debug added a commit that referenced this pull request May 26, 2026
…eset, dead code

Bug #2: handleOpenChange não revogava blobUrlRef.current ao fechar.
  Cada ciclo abrir→gerar→fechar vazava um blob:// na memória do browser.
  Fix: revokeBlobUrl() extraído como helper, chamado no fechamento e antes
  de criar novo blob.

Bug #4: ESC ou clique fora fechava o dialog durante generateProposalPDFv2.
  A operação assíncrona continuava em background (zumbi), podendo tentar
  atualizar state em componente já desmontado.
  Fix: onInteractOutside + onEscapeKeyDown previnem fechamento enquanto
  stage === 'generating'. Mensagem 'Aguarde, não feche esta janela' adicionada.

Bug #5: progressLabel e pdfVersion não resetavam ao fechar o dialog.
  Na próxima abertura, progressLabel mostrava valor antigo; pdfVersion
  começava em v2/v3 em vez de v1.
  Fix: ambos resetados em handleOpenChange junto com demais states.

Bug #8: clientPhone, approvalLink, onWhatsApp, onShareLink declarados na
  interface mas nunca usados. Marcados como @deprecated para backward-compat.

Bug #9: ActionButton.variant 'whatsapp' tinha CSS idêntico a 'primary'.
  Tipo simplificado para 'default' | 'primary'.
adm01-debug added a commit that referenced this pull request May 26, 2026
…Propostas PDF (#438)

* fix(pdf-generator): root.unmount() movido para finally externo — sem memory leak em erro

Bug: se html2canvas lançava exceção durante captura de qualquer página,
root.unmount() nunca era chamado. ReactDOM.createRoot ficava pendurado,
consumindo memória indefinidamente.

Fix:
- `let root` declarado fora do try block
- `root?.unmount()` movido para o finally externo (sempre executa)
- `document.body.removeChild(container)` mantido no mesmo finally
- img.complete check aprimorado: naturalWidth > 0 evita tratar imagem
  com src vazio como 'carregada'

* fix(PdfGenerationDialog): 5 bugs — memory leak, zombie async, state reset, dead code

Bug #2: handleOpenChange não revogava blobUrlRef.current ao fechar.
  Cada ciclo abrir→gerar→fechar vazava um blob:// na memória do browser.
  Fix: revokeBlobUrl() extraído como helper, chamado no fechamento e antes
  de criar novo blob.

Bug #4: ESC ou clique fora fechava o dialog durante generateProposalPDFv2.
  A operação assíncrona continuava em background (zumbi), podendo tentar
  atualizar state em componente já desmontado.
  Fix: onInteractOutside + onEscapeKeyDown previnem fechamento enquanto
  stage === 'generating'. Mensagem 'Aguarde, não feche esta janela' adicionada.

Bug #5: progressLabel e pdfVersion não resetavam ao fechar o dialog.
  Na próxima abertura, progressLabel mostrava valor antigo; pdfVersion
  começava em v2/v3 em vez de v1.
  Fix: ambos resetados em handleOpenChange junto com demais states.

Bug #8: clientPhone, approvalLink, onWhatsApp, onShareLink declarados na
  interface mas nunca usados. Marcados como @deprecated para backward-compat.

Bug #9: ActionButton.variant 'whatsapp' tinha CSS idêntico a 'primary'.
  Tipo simplificado para 'default' | 'primary'.

* fix(ProposalProductTable): imagens em branco no PDF + coluna Total ausente

Bug #3a: loading="lazy" em imagens offscreen.
  O browser só carrega lazy quando o elemento entra no viewport.
  O template é renderizado a -10000px (fora do viewport), então
  nenhuma imagem carregava → PDF gerado com espaços em branco.
  Fix: loading="eager".

Bug #3b: useState("") causa img.complete falso positivo.
  Com src="" inicial, img.complete retorna true imediatamente (sem
  nenhuma imagem ter sido carregada). O generator do PDF interpreta
  isso como "imagem carregada" e chama html2canvas com src vazio.
  Fix: useState(src) — imagem sempre tem src válido desde o início;
  processLogoTransparent atualiza depois com versão sem fundo.

Bug #6: coluna "Total" ausente na tabela de produtos.
  Cliente recebia proposta sem totais por linha, precisando calcular
  manualmente. Fix: nova coluna Total = allInUnitPrice × quantity −
  itemDiscount. colSpan dos cabeçalhos de kit ajustado (3→4 / 4→5).

* fix(PropostaComercialTailwind): mutable itemIndex in render → React 18 StrictMode bug

Bug #7: `let itemIndex = 0` era mutado dentro do .map() do JSX.
  Em React 18 StrictMode (desenvolvimento), React renderiza o componente
  duas vezes para detectar side-effects indesejados. Na segunda passagem,
  itemIndex já tinha o valor acumulado da primeira, dobrando os índices
  de linha: item 0 virava item 4, item 4 virava item 8, etc.
  Resultado: numeração errada das linhas na tabela do PDF em dev.

Fix: startIndices[] pré-computado com Array.reduce() antes do JSX.
  Imutável — não sofre efeito de dupla renderização.
  Substitui: `const startIdx = itemIndex; itemIndex += pageItems.length;`
  Por:       `const startIdx = startIndices[pageIdx];`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants