fix(db): validate_quote_real_discount fail-closed em NULL (Onda 7, B-4)#198
Conversation
Bloqueador B-4 da auditoria de 10/mai. A função trigger validate_quote_real_discount tinha bypass NULL: quando o vendedor não tinha linha em seller_discount_limits, _max_allowed=NULL e a condição "_max_allowed IS NOT NULL AND _real_pct > _max_allowed" era FALSA, deixando passar DESCONTO INFINITO. Cenário concreto: 1. Admin promove novo vendedor 2. Esquece de cadastrar limite em seller_discount_limits 3. Vendedor cria orçamento com 99% de desconto 4. Trigger não bloqueia → margem evapora FIX (Opção B da auditoria): + COALESCE(_max_allowed, 0) após o SELECT INTO + Mensagem distinta para caso "sem cadastro" vs "estourou limite" ESCOPO LIMITADO: O check de admin (role='admin') NÃO foi alterado. Verificação no banco mostrou que existem 2 usuários reais com role='admin' (auditoria assumia 0). Tema do "dual admin pattern" está deferido para decisão arquitetural. VALIDAÇÃO: - Migration aplicada via MCP apply_migration (ADR 0006: zero db push) - pg_proc confirma COALESCE no corpo da função - 5 vendedores ativos hoje têm limite — fix não quebra ninguém - Vendedor novo agora recebe mensagem clara de "sem cadastro" Versão da migration: 20260514165252 (já em prod). Detalhes: docs/hardening/ONDA-7-DISCOUNT-FAIL-CLOSED.md
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughPR implementa correção de bug de segurança no trigger de validação de desconto real. Quando vendedor não possui cadastro em ChangesFail-Closed Discount Validation
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Corrige um bypass por NULL na trigger public.validate_quote_real_discount para que vendedores sem registro em seller_discount_limits não consigam aplicar descontos arbitrários, e documenta a decisão/rollout da Onda 7 (B-4).
Changes:
- Atualiza
validate_quote_real_discount()para tratar_max_allowed=NULLcomo0(fail-closed) e bloquear descontos > 0 sem aprovação. - Ajusta mensagens de erro para distinguir “sem cadastro” vs “estourou limite”.
- Adiciona documentação de hardening com contexto, opções consideradas e validação em prod.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| supabase/migrations/20260514165252_onda7_validate_quote_real_discount_fail_closed_null.sql | Atualiza a função trigger validate_quote_real_discount() para fail-closed quando não há limite cadastrado. |
| docs/hardening/ONDA-7-DISCOUNT-FAIL-CLOSED.md | Documenta o bug, a correção aplicada (Opção B) e o rollout/rollback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SELECT max_discount_percent INTO _max_allowed | ||
| FROM public.seller_discount_limits WHERE user_id = NEW.seller_id; | ||
|
|
||
| -- Onda 7 (B-4): fail-CLOSED em NULL. Vendedor sem linha em seller_discount_limits | ||
| -- nao tem mais desconto ilimitado — agora trata como 0% (precisa aprovacao para qualquer desconto). | ||
| _max_allowed := COALESCE(_max_allowed, 0); | ||
|
|
||
| IF _real_pct > _max_allowed THEN | ||
| IF NOT EXISTS ( | ||
| SELECT 1 FROM public.discount_approval_requests | ||
| WHERE quote_id = NEW.id AND status = 'approved' | ||
| AND requested_discount_percent >= _real_pct | ||
| ) THEN | ||
| -- Mensagens distintas: "sem cadastro" (Onda 7) vs "estourou limite" (comportamento original) | ||
| IF _max_allowed = 0 THEN | ||
| RAISE EXCEPTION | ||
| 'Vendedor sem limite de desconto cadastrado. Solicite ao administrador o cadastro em seller_discount_limits, ou peca aprovacao para o desconto de %%%.', |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@supabase/migrations/20260514165252_onda7_validate_quote_real_discount_fail_closed_null.sql`:
- Around line 53-69: The current logic coalesces NULL to 0 and later uses
"_max_allowed = 0" to infer "no cadastro", which misclassifies an explicit 0
limit as "no cadastro"; change the query flow to detect whether a row existed
(use the SELECT ... INTO followed by checking FOUND or assign a boolean like
_has_row) and keep _max_allowed as-is (or COALESCE to 0 only for arithmetic) but
use the row-existence flag to decide the "sem cadastro" message; update the
conditional around _max_allowed and the RAISE EXCEPTION paths (referencing
_max_allowed, the SELECT INTO from public.seller_discount_limits for
NEW.seller_id, and the subsequent IF NOT EXISTS check against
public.discount_approval_requests) so explicit max_discount_percent = 0 produces
the "estourou limite" message while a missing row triggers "sem cadastro".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ed650aa5-f505-48a8-b230-0069928d3a80
📒 Files selected for processing (2)
docs/hardening/ONDA-7-DISCOUNT-FAIL-CLOSED.mdsupabase/migrations/20260514165252_onda7_validate_quote_real_discount_fail_closed_null.sql
| SELECT max_discount_percent INTO _max_allowed | ||
| FROM public.seller_discount_limits WHERE user_id = NEW.seller_id; | ||
|
|
||
| -- Onda 7 (B-4): fail-CLOSED em NULL. Vendedor sem linha em seller_discount_limits | ||
| -- nao tem mais desconto ilimitado — agora trata como 0% (precisa aprovacao para qualquer desconto). | ||
| _max_allowed := COALESCE(_max_allowed, 0); | ||
|
|
||
| IF _real_pct > _max_allowed THEN | ||
| IF NOT EXISTS ( | ||
| SELECT 1 FROM public.discount_approval_requests | ||
| WHERE quote_id = NEW.id AND status = 'approved' | ||
| AND requested_discount_percent >= _real_pct | ||
| ) THEN | ||
| -- Mensagens distintas: "sem cadastro" (Onda 7) vs "estourou limite" (comportamento original) | ||
| IF _max_allowed = 0 THEN | ||
| RAISE EXCEPTION | ||
| 'Vendedor sem limite de desconto cadastrado. Solicite ao administrador o cadastro em seller_discount_limits, ou peca aprovacao para o desconto de %%%.', |
There was a problem hiding this comment.
Diferenciação de erro pode ficar incorreta para limite explícito 0%.
Em Line 67, a regra usa _max_allowed = 0 para inferir “sem cadastro”. Se existir linha em seller_discount_limits com max_discount_percent = 0, a mensagem “sem limite cadastrado” fica errada (há cadastro, com limite zero).
💡 Ajuste sugerido
DECLARE
_markup NUMERIC := COALESCE(NEW.negotiation_markup_percent, 0);
_apparent_pct NUMERIC := COALESCE(NEW.discount_percent, 0);
_presented NUMERIC := COALESCE(NEW.subtotal, 0);
_real_sub NUMERIC;
_final NUMERIC;
_real_pct NUMERIC;
+ _max_allowed_raw NUMERIC;
_max_allowed NUMERIC;
+ _has_limit BOOLEAN := FALSE;
_is_admin BOOLEAN;
BEGIN
@@
- SELECT max_discount_percent INTO _max_allowed
+ SELECT max_discount_percent INTO _max_allowed_raw
FROM public.seller_discount_limits WHERE user_id = NEW.seller_id;
+ _has_limit := FOUND;
- _max_allowed := COALESCE(_max_allowed, 0);
+ _max_allowed := COALESCE(_max_allowed_raw, 0);
@@
- IF _max_allowed = 0 THEN
+ IF NOT _has_limit OR _max_allowed_raw IS NULL THEN
RAISE EXCEPTION
'Vendedor sem limite de desconto cadastrado. Solicite ao administrador o cadastro em seller_discount_limits, ou peca aprovacao para o desconto de %%%.',
_real_pct
USING ERRCODE = 'check_violation';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@supabase/migrations/20260514165252_onda7_validate_quote_real_discount_fail_closed_null.sql`
around lines 53 - 69, The current logic coalesces NULL to 0 and later uses
"_max_allowed = 0" to infer "no cadastro", which misclassifies an explicit 0
limit as "no cadastro"; change the query flow to detect whether a row existed
(use the SELECT ... INTO followed by checking FOUND or assign a boolean like
_has_row) and keep _max_allowed as-is (or COALESCE to 0 only for arithmetic) but
use the row-existence flag to decide the "sem cadastro" message; update the
conditional around _max_allowed and the RAISE EXCEPTION paths (referencing
_max_allowed, the SELECT INTO from public.seller_discount_limits for
NEW.seller_id, and the subsequent IF NOT EXISTS check against
public.discount_approval_requests) so explicit max_discount_percent = 0 produces
the "estourou limite" message while a missing row triggers "sem cadastro".
![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: ed175270dd
ℹ️ 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".
|
|
||
| -- Onda 7 (B-4): fail-CLOSED em NULL. Vendedor sem linha em seller_discount_limits | ||
| -- nao tem mais desconto ilimitado — agora trata como 0% (precisa aprovacao para qualquer desconto). | ||
| _max_allowed := COALESCE(_max_allowed, 0); |
There was a problem hiding this comment.
With this migration, a non-admin seller who has no seller_discount_limits row now has an effective limit of 0%, so any positive real discount on a draft/pending quote raises here. I checked the quote builder path: useQuoteBuilderState currently returns false for isDiscountExceeded when maxDiscountPercent === null, and it only creates approval requests when maxDiscountPercent !== null, so the UI still shows the normal create/save action and then hits this database exception instead of offering the approval flow. Either the frontend needs to treat a missing limit as 0 or the migration needs a compatible path for creating approval requests before blocking these saves.
Useful? React with 👍 / 👎.
| NEW.real_subtotal := ROUND(_real_sub, 2); | ||
| NEW.real_discount_percent := _real_pct; | ||
|
|
||
| IF NEW.status IN ('draft', 'pending') AND NEW.seller_id IS NOT NULL AND _real_pct > 0 THEN |
There was a problem hiding this comment.
This fail-closed logic only runs for draft and pending, but I checked the active quote RLS and quotes_insert_scope/quotes_update_scope allow a seller to insert or update their own quote without a status restriction, while pending_approval/sent are valid quote statuses. A seller with no discount-limit row can therefore submit the same discounted quote as pending_approval or sent and skip the _max_allowed check entirely; move the positive-discount validation to all seller-controlled statuses, or require an approved request before any status can bypass it.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/migrations/20260514165252_onda7_validate_quote_real_discount_fail_closed_null.sql">
<violation number="1" location="supabase/migrations/20260514165252_onda7_validate_quote_real_discount_fail_closed_null.sql:67">
P2: A condição `_max_allowed = 0` não distingue “sem cadastro” de “limite cadastrado em 0%”, então pode exibir a mensagem errada para vendedores com limite real de 0%.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| AND requested_discount_percent >= _real_pct | ||
| ) THEN | ||
| -- Mensagens distintas: "sem cadastro" (Onda 7) vs "estourou limite" (comportamento original) | ||
| IF _max_allowed = 0 THEN |
There was a problem hiding this comment.
P2: A condição _max_allowed = 0 não distingue “sem cadastro” de “limite cadastrado em 0%”, então pode exibir a mensagem errada para vendedores com limite real de 0%.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260514165252_onda7_validate_quote_real_discount_fail_closed_null.sql, line 67:
<comment>A condição `_max_allowed = 0` não distingue “sem cadastro” de “limite cadastrado em 0%”, então pode exibir a mensagem errada para vendedores com limite real de 0%.</comment>
<file context>
@@ -0,0 +1,87 @@
+ AND requested_discount_percent >= _real_pct
+ ) THEN
+ -- Mensagens distintas: "sem cadastro" (Onda 7) vs "estourou limite" (comportamento original)
+ IF _max_allowed = 0 THEN
+ RAISE EXCEPTION
+ 'Vendedor sem limite de desconto cadastrado. Solicite ao administrador o cadastro em seller_discount_limits, ou peca aprovacao para o desconto de %%%.',
</file context>
…I e testes - Resolve conflitos em baselines (ESLint 853, TSC 1262) e test files - Corrige non-null assertion em sentry.ts (lint-staged pre-commit) - Mantém: SidebarNavGroup tests fixed, CI workflow fixes, baseline melhorado https://claude.ai/code/session_01MuNDxFSRRaJLsvkBdyQ2dK
Bloqueador B-4 da auditoria de 10/mai. A função trigger validate_quote_real_discount tinha bypass NULL: quando o vendedor não tinha linha em seller_discount_limits, _max_allowed=NULL e a condição "_max_allowed IS NOT NULL AND _real_pct > _max_allowed" era FALSA, deixando passar DESCONTO INFINITO.
Cenário concreto:
FIX (Opção B da auditoria):
ESCOPO LIMITADO:
O check de admin (role='admin') NÃO foi alterado. Verificação no banco mostrou que existem 2 usuários reais com role='admin' (auditoria assumia 0). Tema do "dual admin pattern" está deferido para decisão arquitetural.
VALIDAÇÃO:
Versão da migration: 20260514165252 (já em prod).
Detalhes: docs/hardening/ONDA-7-DISCOUNT-FAIL-CLOSED.md
📋 Descrição
🎯 Tipo de mudança
🔗 Issues relacionadas
Closes #
Refs #
🌐 Sistemas afetados
🧪 Como testar
✅ Checklist pré-merge
Qualidade
npx tsc --noEmitpassa sem errosnpm run test)Segurança
console.logcom payloads sensíveis (usarlogger.*)Documentação
mem://) se a mudança afetar arquitetura/regras_backup_*_YYYYMMDDse destrutivasUI
📸 Screenshots (se UI)
🔄 Plano de rollback
Summary by cubic
Corrige o bypass de NULL na trigger
validate_quote_real_discount, fechando o caminho para “desconto infinito” quando o vendedor não tem limite cadastrado. Resolve o bloqueador B-4 da auditoria (Onda 7) e mantém o fluxo atual para quem já tem limite.COALESCE(_max_allowed, 0)e checagem direta_real_pct > _max_allowed.role='admin'mantido (fora do escopo do B-4).20260514165252_onda7_validate_quote_real_discount_fail_closed_null.sqle docdocs/hardening/ONDA-7-DISCOUNT-FAIL-CLOSED.md.Written for commit ed17527. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Bug Fixes
Documentation