feat(contracts): unified 422 envelope + v1/v2 versioning for webhooks#68
feat(contracts): unified 422 envelope + v1/v2 versioning for webhooks#68adm01-debug wants to merge 4 commits into
Conversation
…or webhooks
Adds a single canonical error shape for every webhook and edge-function
validation failure, with a versioned contract (v1 legacy → v2 canonical) and
69 new offline contract tests asserting backwards compatibility, missing
fields, wrong types, empty values, and cross-field rules.
What changes for callers:
- Status code for schema failures: now 422 (was 400/422 inconsistently).
- v1 default: { error, details } — unchanged for existing integrations.
- v2 opt-in: { code, message, version, fields[{path, code, message}] } via
`X-API-Version: v2` header, `?api_version=v2` query, or vendor Accept.
Endpoints covered: product-webhook, webhook-dispatcher, webhook-inbound.
Schemas extracted to _shared/webhook-schemas.ts with a Node mirror at
src/lib/webhook-schemas.ts (parity-checked in CI).
…formatting Prettier in src/ uses single quotes and breaks long method chains across lines; Deno style guide uses double quotes and inline form. Update the normalizer to strip whitespace around syntactic punctuation and unify quotes before byte-comparing the two mirrors, so semantic parity passes without forcing one style on the other.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughEste PR implementa um contrato unificado de validação de webhooks com suporte a duas versões de resposta de erro (v1 retrocompatível e v2 canônico), incluindo schemas Zod compartilhados entre Node e Deno, negociação automática de versão e uma infraestrutura completa de testes offline e e2e. ChangesContrato de Validação de Webhooks v1/v2
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
There was a problem hiding this comment.
Pull request overview
Implements a unified validation contract for Edge Functions/webhooks, standardizing schema failures to HTTP 422 while supporting v1 (legacy default) and v2 (opt-in canonical) error envelopes with version negotiation.
Changes:
- Added shared validation error builders (v1/v2) + contract version negotiation, and wired them into the three webhook Edge Functions.
- Centralized canonical Zod schemas for webhook payloads (Deno) with a Node mirror, enforced by a parity test.
- Added extensive offline contract tests (Vitest), an online HTTP contract script, and reference documentation for the contract.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/edge-functions/webhook-schemas.contract.test.ts | Contract tests for webhook schemas + v1↔v2 semantic compatibility assertions. |
| tests/edge-functions/webhook-schemas-parity.test.ts | Ensures Deno ↔ Node schema/validation mirrors stay in sync. |
| tests/edge-functions/validation-errors.test.ts | Contract tests for version negotiation and v1/v2 builder invariants. |
| supabase/functions/webhook-inbound/index.ts | Adopts unified validation/error responses and validates the inbound routing envelope. |
| supabase/functions/webhook-dispatcher/index.ts | Switches body validation to canonical schema + unified error builders; updates CORS headers. |
| supabase/functions/product-webhook/index.ts | Replaces in-file schemas with shared canonical schema + unified error responses. |
| supabase/functions/_shared/zod-validate.ts | Refactors request parsing helpers to use unified 400/422 error builders and adds parseObjectWithSchema. |
| supabase/functions/_shared/webhook-schemas.ts | Introduces canonical webhook schemas for Deno runtime. |
| supabase/functions/_shared/validation-errors.ts | Adds unified 422 builder + version negotiation + generic error response builder. |
| supabase/functions/_shared/cors.ts | Allows/exposes x-api-version for contract negotiation/echo. |
| src/lib/webhook-schemas.ts | Node mirror of canonical webhook schemas for tests/frontend consumers. |
| src/lib/validation-errors.ts | Node mirror of validation error logic + type guards for consumers/tests. |
| scripts/contract-testing.mjs | Adds live HTTP-level contract testing across v1/v2 for deployed functions. |
| docs/WEBHOOKS_CONTRACT.md | Documents the unified contract, negotiation rules, and test strategy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Body parse falha → 422 antes da auth (não vaza info). | ||
| let rawJson: unknown; | ||
| try { | ||
| const text = await req.text(); | ||
| rawJson = text ? JSON.parse(text) : {}; |
| * sync. Both files are validated together by tests/edge-functions/ | ||
| * validation-error-contract.test.ts which imports schemas from both sides. |
| return ( | ||
| typeof o.error === 'string' && | ||
| 'details' in o && | ||
| (typeof o.details === 'object' || Array.isArray(o.details)) |
| const SUPABASE_URL = process.env.SUPABASE_URL || "https://pqpdolkaeqlyzpdpbizo.supabase.co"; | ||
| // Usando a chave de simulação estável definida para este projeto | ||
| const SERVICE_ROLE_KEY = "a46c3981-244a-4f81-9f57-bab5c45b5cde"; | ||
| const SERVICE_ROLE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY || "a46c3981-244a-4f81-9f57-bab5c45b5cde"; | ||
| const PRODUCT_SECRET = process.env.N8N_PRODUCT_WEBHOOK_SECRET || "sim-secret"; |
| Quando v1 for descontinuado: | ||
|
|
||
| 1. Anuncie via `Deprecation: true` e `Sunset: <date>` headers nas respostas | ||
| v1 (a infra atual já suporta — basta estender `buildValidationError`). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
supabase/functions/_shared/cors.ts (1)
216-216:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistência:
Access-Control-Expose-Headersnão incluix-api-versionnos endpoints públicos.
CORS_HEADERS_BASE(linha 59) expõex-request-id, x-api-version, masbuildPublicCorsHeaderssó expõex-request-id. Se o contrato v2 retornaX-API-Versionno response e o endpoint usa CORS público, o browser não conseguirá ler esse header.🔧 Sugestão de correção
return { 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Headers': Array.from(merged).join(', '), 'Access-Control-Allow-Methods': opts.allowMethods ?? CORS_HEADERS_BASE['Access-Control-Allow-Methods'], - 'Access-Control-Expose-Headers': 'x-request-id', + 'Access-Control-Expose-Headers': 'x-request-id, x-api-version', 'X-Content-Type-Options': 'nosniff',🤖 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/functions/_shared/cors.ts` at line 216, A configuração de CORS público está inconsistente: CORS_HEADERS_BASE declara exposição de "x-request-id, x-api-version" mas a função buildPublicCorsHeaders só expõe "x-request-id", impedindo o browser de ler X-API-Version; atualize buildPublicCorsHeaders para incluir "x-api-version" em 'Access-Control-Expose-Headers' (use o mesmo identificador usado em CORS_HEADERS_BASE: x-api-version) para garantir que o header X-API-Version seja exposto nos endpoints públicos.supabase/functions/webhook-inbound/index.ts (1)
114-119:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCatch block não foi atualizado e vaza mensagem de erro.
O bloco catch ainda usa
new Response()manual em vez debuildErrorResponse, quebrando a consistência do contrato. Além disso,err.messageé retornado diretamente, podendo vazar detalhes internos (SQL, nomes de tabela, etc).🔧 Sugestão: usar buildErrorResponse com mensagem genérica
} catch (err) { const msg = err instanceof Error ? err.message : "Erro"; - return new Response(JSON.stringify({ error: msg }), { - status: 500, headers: { ...corsHeaders, "Content-Type": "application/json" }, - }); + console.error("[webhook-inbound] internal error:", err); + return buildErrorResponse("internal_error", "Erro interno do servidor", req, corsHeaders, { status: 500 }); }🤖 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/functions/webhook-inbound/index.ts` around lines 114 - 119, The catch block currently returns a manual new Response exposing err.message; replace that with a call to buildErrorResponse to preserve the contract and avoid leaking internal details: in the catch handling around the webhook handler (the block that currently computes msg from err and returns new Response), call buildErrorResponse({ status: 500, message: "Internal server error" }, corsHeaders) (or the codebase's buildErrorResponse signature) instead of returning err.message, and ensure you still pass the existing corsHeaders so CORS behavior is unchanged; do not include err.message or other internal error details in the response body.
🤖 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 `@scripts/contract-testing.mjs`:
- Around line 26-29: Remove the hardcoded fallbacks for SUPABASE_URL,
SERVICE_ROLE_KEY, and PRODUCT_SECRET in the top-level constants (SUPABASE_URL,
SERVICE_ROLE_KEY, PRODUCT_SECRET) and make the script fail-fast when any
required env var is missing: read the values directly from process.env and if
any is undefined or empty, log a clear error and exit/throw (e.g., throw new
Error or process.exit(1)) so the runner doesn't continue with insecure defaults.
- Around line 243-247: The current code calls response.json() and, on failure,
tries response.text(), but the body has already been consumed; update the logic
around the response variable so the raw body is preserved: either call await
response.text() once into a raw variable and then attempt JSON.parse(raw) (set
responseData to the parsed object or to { _raw: raw } on parse failure), or use
response.clone() to call clone().json() and fall back to response.text() from
the original/cloned response; adjust handling of responseData accordingly so the
raw payload is never lost.
In `@src/lib/validation-errors.ts`:
- Around line 104-111: The type guard isValidationErrorV1 currently treats
details: null as valid because typeof null === 'object'; update the predicate to
explicitly exclude null by changing the details check to require either
Array.isArray(o.details) or (typeof o.details === 'object' && o.details !==
null) so that ValidationErrorV1 (details: Record<string,string[]> | string[]) is
not satisfied by null; adjust the return expression in isValidationErrorV1
accordingly, referencing the function isValidationErrorV1 and the local variable
o/details/ValidationErrorV1 for locating the change.
In `@supabase/functions/_shared/webhook-schemas.ts`:
- Around line 127-131: InboundWebhookEnvelopeSchema currently marks the
signature field as optional which allows unsigned requests to pass envelope
validation; update the schema by removing the .optional() on the signature
property in InboundWebhookEnvelopeSchema so signature remains required (keep the
existing regex/validation), and make the identical change in the mirror schema
at src/lib/webhook-schemas.ts to maintain parity with the shared secret/HMAC
webhook validation guideline.
- Around line 58-59: Replace the permissive z.any() usage with z.unknown() for
the schema fields that mirror runtime product data: change variations to
z.array(z.unknown()).max(200).optional() and metadata to
z.record(z.unknown()).optional(); apply the same replacements in the counterpart
schema in src/lib/webhook-schemas.ts so both webhook schema definitions
(variations and metadata) use z.unknown() instead of z.any().
In `@supabase/functions/webhook-dispatcher/index.ts`:
- Around line 276-279: No bloco catch que usa buildErrorResponse, não retorne
err.message diretamente ao cliente; em vez disso registre o erro completo (por
exemplo via console.error or processLogger.error) para diagnóstico e passe uma
mensagem genérica como "Erro interno" para buildErrorResponse; atualize o catch
que referencia err e buildErrorResponse para logar todo o objeto de erro e
substituir msg por um texto não revelador antes de retornar.
---
Outside diff comments:
In `@supabase/functions/_shared/cors.ts`:
- Line 216: A configuração de CORS público está inconsistente: CORS_HEADERS_BASE
declara exposição de "x-request-id, x-api-version" mas a função
buildPublicCorsHeaders só expõe "x-request-id", impedindo o browser de ler
X-API-Version; atualize buildPublicCorsHeaders para incluir "x-api-version" em
'Access-Control-Expose-Headers' (use o mesmo identificador usado em
CORS_HEADERS_BASE: x-api-version) para garantir que o header X-API-Version seja
exposto nos endpoints públicos.
In `@supabase/functions/webhook-inbound/index.ts`:
- Around line 114-119: The catch block currently returns a manual new Response
exposing err.message; replace that with a call to buildErrorResponse to preserve
the contract and avoid leaking internal details: in the catch handling around
the webhook handler (the block that currently computes msg from err and returns
new Response), call buildErrorResponse({ status: 500, message: "Internal server
error" }, corsHeaders) (or the codebase's buildErrorResponse signature) instead
of returning err.message, and ensure you still pass the existing corsHeaders so
CORS behavior is unchanged; do not include err.message or other internal error
details in the response body.
🪄 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: 2d4db4b0-9168-4f3e-b28b-407ff3a7ba27
📒 Files selected for processing (14)
docs/WEBHOOKS_CONTRACT.mdscripts/contract-testing.mjssrc/lib/validation-errors.tssrc/lib/webhook-schemas.tssupabase/functions/_shared/cors.tssupabase/functions/_shared/validation-errors.tssupabase/functions/_shared/webhook-schemas.tssupabase/functions/_shared/zod-validate.tssupabase/functions/product-webhook/index.tssupabase/functions/webhook-dispatcher/index.tssupabase/functions/webhook-inbound/index.tstests/edge-functions/validation-errors.test.tstests/edge-functions/webhook-schemas-parity.test.tstests/edge-functions/webhook-schemas.contract.test.ts
| const SUPABASE_URL = process.env.SUPABASE_URL || "https://pqpdolkaeqlyzpdpbizo.supabase.co"; | ||
| // Usando a chave de simulação estável definida para este projeto | ||
| const SERVICE_ROLE_KEY = "a46c3981-244a-4f81-9f57-bab5c45b5cde"; | ||
| const SERVICE_ROLE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY || "a46c3981-244a-4f81-9f57-bab5c45b5cde"; | ||
| const PRODUCT_SECRET = process.env.N8N_PRODUCT_WEBHOOK_SECRET || "sim-secret"; | ||
|
|
There was a problem hiding this comment.
Remova fallback hardcoded de URL/chaves no runner de contrato.
Nas Lines 26-29, URL e credenciais estão com valores hardcoded de fallback. Isso cria risco de segurança e também mascara ambiente mal configurado. Faça fail-fast quando as variáveis obrigatórias não estiverem definidas.
🔧 Patch sugerido
-const SUPABASE_URL = process.env.SUPABASE_URL || "https://pqpdolkaeqlyzpdpbizo.supabase.co";
-const SERVICE_ROLE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY || "a46c3981-244a-4f81-9f57-bab5c45b5cde";
-const PRODUCT_SECRET = process.env.N8N_PRODUCT_WEBHOOK_SECRET || "sim-secret";
+const SUPABASE_URL = process.env.SUPABASE_URL;
+const SERVICE_ROLE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY;
+const PRODUCT_SECRET = process.env.N8N_PRODUCT_WEBHOOK_SECRET;
+
+if (!SUPABASE_URL || !SERVICE_ROLE_KEY || !PRODUCT_SECRET) {
+ throw new Error(
+ "Missing required env vars: SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY, N8N_PRODUCT_WEBHOOK_SECRET",
+ );
+}As per coding guidelines **/*.{ts,tsx,js,jsx}: "Tokens, secrets ou URLs de API hardcoded (mover para env)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const SUPABASE_URL = process.env.SUPABASE_URL || "https://pqpdolkaeqlyzpdpbizo.supabase.co"; | |
| // Usando a chave de simulação estável definida para este projeto | |
| const SERVICE_ROLE_KEY = "a46c3981-244a-4f81-9f57-bab5c45b5cde"; | |
| const SERVICE_ROLE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY || "a46c3981-244a-4f81-9f57-bab5c45b5cde"; | |
| const PRODUCT_SECRET = process.env.N8N_PRODUCT_WEBHOOK_SECRET || "sim-secret"; | |
| const SUPABASE_URL = process.env.SUPABASE_URL; | |
| const SERVICE_ROLE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY; | |
| const PRODUCT_SECRET = process.env.N8N_PRODUCT_WEBHOOK_SECRET; | |
| if (!SUPABASE_URL || !SERVICE_ROLE_KEY || !PRODUCT_SECRET) { | |
| throw new Error( | |
| "Missing required env vars: SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY, N8N_PRODUCT_WEBHOOK_SECRET", | |
| ); | |
| } |
🤖 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 `@scripts/contract-testing.mjs` around lines 26 - 29, Remove the hardcoded
fallbacks for SUPABASE_URL, SERVICE_ROLE_KEY, and PRODUCT_SECRET in the
top-level constants (SUPABASE_URL, SERVICE_ROLE_KEY, PRODUCT_SECRET) and make
the script fail-fast when any required env var is missing: read the values
directly from process.env and if any is undefined or empty, log a clear error
and exit/throw (e.g., throw new Error or process.exit(1)) so the runner doesn't
continue with insecure defaults.
| try { | ||
| responseData = await response.json(); | ||
| } catch { | ||
| responseData = { _raw: await response.text().catch(() => "") }; | ||
| } |
There was a problem hiding this comment.
Evite perder o payload bruto ao tratar JSON inválido.
Na Line 243, response.json() consome o body; se falhar, na Line 246 o response.text() geralmente não consegue reler. Isso reduz diagnóstico de falhas reais no contrato.
🔧 Patch sugerido
- let responseData;
- try {
- responseData = await response.json();
- } catch {
- responseData = { _raw: await response.text().catch(() => "") };
- }
+ const raw = await response.text();
+ let responseData;
+ try {
+ responseData = raw ? JSON.parse(raw) : {};
+ } catch {
+ responseData = { _raw: raw };
+ }🤖 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 `@scripts/contract-testing.mjs` around lines 243 - 247, The current code calls
response.json() and, on failure, tries response.text(), but the body has already
been consumed; update the logic around the response variable so the raw body is
preserved: either call await response.text() once into a raw variable and then
attempt JSON.parse(raw) (set responseData to the parsed object or to { _raw: raw
} on parse failure), or use response.clone() to call clone().json() and fall
back to response.text() from the original/cloned response; adjust handling of
responseData accordingly so the raw payload is never lost.
| export function isValidationErrorV1(p: unknown): p is ValidationErrorV1 { | ||
| if (!p || typeof p !== 'object') return false; | ||
| const o = p as Record<string, unknown>; | ||
| return ( | ||
| typeof o.error === 'string' && | ||
| 'details' in o && | ||
| (typeof o.details === 'object' || Array.isArray(o.details)) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verificar cobertura e usos do guard para evitar falso positivo com details:null
rg -n "isValidationErrorV1\\(" -C2
rg -n "details:\\s*null|ValidationErrorV1" tests -C2Repository: adm01-debug/promo-gifts-v4
Length of output: 7521
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspecionar definição de ValidationErrorV1 e implementação de buildValidationErrorV1/buildValidationError
# (reduz leitura usando sed por faixa de linhas)
FILE="src/lib/validation-errors.ts"
echo "=== isValidationErrorV1 e tipos próximos ==="
sed -n '1,220p' "$FILE" | nl -ba | sed -n '80,180p'
echo
echo "=== buildValidationErrorV1/buildValidationError (busca e trechos) ==="
rg -n "buildValidationErrorV1|buildValidationError\\b" "$FILE" -n
rg -n "type ValidationErrorV1|interface ValidationErrorV1|ValidationErrorV1\\b" "$FILE"
echo
echo "=== Buscar por details: null no contexto de validation-errors ==="
rg -n "details:\\s*null" src tests -S
echo
echo "=== Buscar por isValidationErrorV1 com cenários negativos ==="
rg -n "isValidationErrorV1\\(" tests/edge-functions -SRepository: adm01-debug/promo-gifts-v4
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/lib/validation-errors.ts"
echo "=== isValidationErrorV1 e tipos próximos (arquivo inteiro até um limite) ==="
# mostrar trecho com numeração sem usar nl
cat -n "$FILE" | sed -n '80,180p'
echo
echo "=== Ocorrências de buildValidationErrorV1/buildValidationError no arquivo ==="
rg -n "buildValidationErrorV1|buildValidationError\\b" "$FILE"
echo
echo "=== Definição/declarações de ValidationErrorV1 no arquivo ==="
rg -n "type ValidationErrorV1|interface ValidationErrorV1|ValidationErrorV1\\b" "$FILE"
echo
echo "=== Buscar por detalhes null no contexto validation-errors ==="
rg -n "details:\\s*null" src tests -S
echo
echo "=== isValidationErrorV1 com cenários negativos (tests edge-functions) ==="
rg -n "isValidationErrorV1\\(" tests/edge-functions -SRepository: adm01-debug/promo-gifts-v4
Length of output: 2949
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/lib/validation-errors.ts"
echo "=== ValidationErrorV1 interface (context) ==="
cat -n "$FILE" | sed -n '1,80p'
echo
echo "=== buildValidationErrorV1 implementation (context) ==="
cat -n "$FILE" | sed -n '55,120p'Repository: adm01-debug/promo-gifts-v4
Length of output: 5394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "isValidationErrorV1\\(" -S src testsRepository: adm01-debug/promo-gifts-v4
Length of output: 484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "isValidationErrorV1\\(" -S src testsRepository: adm01-debug/promo-gifts-v4
Length of output: 484
Type guard v1 aceita details: null como válido.
Em isValidationErrorV1 (src/lib/validation-errors.ts, linhas 104-111), typeof o.details === 'object' retorna true para null, então um payload com details: null passa no narrowing. Isso conflita com ValidationErrorV1, onde details é Record<string, string[]> | string[] (não-null).
Correção sugerida
export function isValidationErrorV1(p: unknown): p is ValidationErrorV1 {
if (!p || typeof p !== 'object') return false;
const o = p as Record<string, unknown>;
return (
typeof o.error === 'string' &&
'details' in o &&
- (typeof o.details === 'object' || Array.isArray(o.details))
+ o.details !== null &&
+ (Array.isArray(o.details) || typeof o.details === 'object')
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function isValidationErrorV1(p: unknown): p is ValidationErrorV1 { | |
| if (!p || typeof p !== 'object') return false; | |
| const o = p as Record<string, unknown>; | |
| return ( | |
| typeof o.error === 'string' && | |
| 'details' in o && | |
| (typeof o.details === 'object' || Array.isArray(o.details)) | |
| ); | |
| export function isValidationErrorV1(p: unknown): p is ValidationErrorV1 { | |
| if (!p || typeof p !== 'object') return false; | |
| const o = p as Record<string, unknown>; | |
| return ( | |
| typeof o.error === 'string' && | |
| 'details' in o && | |
| o.details !== null && | |
| (Array.isArray(o.details) || typeof o.details === 'object') | |
| ); | |
| } |
🤖 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 `@src/lib/validation-errors.ts` around lines 104 - 111, The type guard
isValidationErrorV1 currently treats details: null as valid because typeof null
=== 'object'; update the predicate to explicitly exclude null by changing the
details check to require either Array.isArray(o.details) or (typeof o.details
=== 'object' && o.details !== null) so that ValidationErrorV1 (details:
Record<string,string[]> | string[]) is not satisfied by null; adjust the return
expression in isValidationErrorV1 accordingly, referencing the function
isValidationErrorV1 and the local variable o/details/ValidationErrorV1 for
locating the change.
| variations: z.array(z.any()).max(200).optional(), | ||
| metadata: z.record(z.any()).optional(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirmar onde z.any() ainda aparece e onde ProductPayload é consumido
rg -n "z\\.any\\(" -C2
rg -n "\\bProductPayload\\b|\\bProductWebhookPayload\\b" --type=ts -C2Repository: adm01-debug/promo-gifts-v4
Length of output: 3242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Localizar usos de variations/metadata nos consumidores do schema
rg -n --type=ts "\\bvariations\\b|\\bmetadata\\b" supabase/functions/product-webhook -S -C 2 || true
rg -n --type=ts "\\bvariations\\b|\\bmetadata\\b" supabase/functions -S -C 2 || true
# Ver rapidamente o tipo inferido e o schema correspondente
sed -n '40,120p' supabase/functions/_shared/webhook-schemas.ts
sed -n '40,120p' src/lib/webhook-schemas.tsRepository: adm01-debug/promo-gifts-v4
Length of output: 38790
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspecionar o schema completo e como variations/metadata são inferidos
echo "---- supabase/functions/_shared/webhook-schemas.ts (variations/metadata) ----"
sed -n '1,140p' supabase/functions/_shared/webhook-schemas.ts | sed -n '40,120p'
echo "---- src/lib/webhook-schemas.ts (variations/metadata) ----"
sed -n '1,160p' src/lib/webhook-schemas.ts | sed -n '40,120p'
# Inspecionar como ProductPayload é consumido no product-webhook
echo "---- supabase/functions/product-webhook/index.ts (uso de variations/metadata) ----"
sed -n '130,260p' supabase/functions/product-webhook/index.tsRepository: adm01-debug/promo-gifts-v4
Length of output: 10693
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Ver todo o uso (direto) de variations/metadata dentro do product-webhook
rg -n --type=ts "\\bproduct\\.(variations|metadata)\\b|\\bvariations\\b|\\bmetadata\\b" supabase/functions/product-webhook/index.ts -n -S
# Mostrar contexto ao redor das linhas onde são atribuídos
sed -n '160,230p' supabase/functions/product-webhook/index.tsRepository: adm01-debug/promo-gifts-v4
Length of output: 2513
Trocar z.any() por z.unknown() em variations e metadata
supabase/functions/product-webhook/index.ts repassa product.variations/product.metadata direto para productData (insert/update) sem narrowing/validação posterior; z.any() enfraquece os tipos no domínio. Faça o espelhamento também em src/lib/webhook-schemas.ts.
Refactor sugerido
- variations: z.array(z.any()).max(200).optional(),
- metadata: z.record(z.any()).optional(),
+ variations: z.array(z.unknown()).max(200).optional(),
+ metadata: z.record(z.unknown()).optional(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| variations: z.array(z.any()).max(200).optional(), | |
| metadata: z.record(z.any()).optional(), | |
| variations: z.array(z.unknown()).max(200).optional(), | |
| metadata: z.record(z.unknown()).optional(), |
🤖 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/functions/_shared/webhook-schemas.ts` around lines 58 - 59, Replace
the permissive z.any() usage with z.unknown() for the schema fields that mirror
runtime product data: change variations to
z.array(z.unknown()).max(200).optional() and metadata to
z.record(z.unknown()).optional(); apply the same replacements in the counterpart
schema in src/lib/webhook-schemas.ts so both webhook schema definitions
(variations and metadata) use z.unknown() instead of z.any().
| export const InboundWebhookEnvelopeSchema = z.object({ | ||
| slug: z.string().min(1).max(120).regex(/^[a-z0-9-]+$/, "slug must be kebab-case"), | ||
| event_type: z.string().min(1).max(120).default("unknown"), | ||
| signature: z.string().regex(/^(sha256=)?[a-f0-9]{64}$/i, "signature must be hex sha256, optionally prefixed").optional(), | ||
| }); |
There was a problem hiding this comment.
Assinatura HMAC ficou opcional no schema inbound.
Em Line [130], signature com .optional() permite requisição sem assinatura passar na validação de envelope, enfraquecendo o controle de autenticação do webhook.
Correção sugerida
export const InboundWebhookEnvelopeSchema = z.object({
slug: z.string().min(1).max(120).regex(/^[a-z0-9-]+$/, "slug must be kebab-case"),
event_type: z.string().min(1).max(120).default("unknown"),
- signature: z.string().regex(/^(sha256=)?[a-f0-9]{64}$/i, "signature must be hex sha256, optionally prefixed").optional(),
+ signature: z.string().regex(/^(sha256=)?[a-f0-9]{64}$/i, "signature must be hex sha256, optionally prefixed"),
});Também aplique o mesmo ajuste no mirror src/lib/webhook-schemas.ts para manter paridade.
As per coding guidelines "Validação de payload em webhooks (shared secret, assinatura HMAC quando aplicável)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const InboundWebhookEnvelopeSchema = z.object({ | |
| slug: z.string().min(1).max(120).regex(/^[a-z0-9-]+$/, "slug must be kebab-case"), | |
| event_type: z.string().min(1).max(120).default("unknown"), | |
| signature: z.string().regex(/^(sha256=)?[a-f0-9]{64}$/i, "signature must be hex sha256, optionally prefixed").optional(), | |
| }); | |
| export const InboundWebhookEnvelopeSchema = z.object({ | |
| slug: z.string().min(1).max(120).regex(/^[a-z0-9-]+$/, "slug must be kebab-case"), | |
| event_type: z.string().min(1).max(120).default("unknown"), | |
| signature: z.string().regex(/^(sha256=)?[a-f0-9]{64}$/i, "signature must be hex sha256, optionally prefixed"), | |
| }); |
🤖 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/functions/_shared/webhook-schemas.ts` around lines 127 - 131,
InboundWebhookEnvelopeSchema currently marks the signature field as optional
which allows unsigned requests to pass envelope validation; update the schema by
removing the .optional() on the signature property in
InboundWebhookEnvelopeSchema so signature remains required (keep the existing
regex/validation), and make the identical change in the mirror schema at
src/lib/webhook-schemas.ts to maintain parity with the shared secret/HMAC
webhook validation guideline.
| } catch (err) { | ||
| const msg = err instanceof Error ? err.message : "Erro desconhecido"; | ||
| return new Response(JSON.stringify({ error: msg }), { | ||
| status: 500, headers: { ...corsHeaders, "Content-Type": "application/json" }, | ||
| }); | ||
| return buildErrorResponse("internal_error", msg, req, corsHeaders, { status: 500 }); | ||
| } |
There was a problem hiding this comment.
Potencial vazamento de informação no erro interno.
A mensagem de erro err.message é retornada diretamente ao cliente. Se o erro vier do Supabase/Postgres, pode conter nomes de tabelas, colunas ou fragmentos SQL. Conforme as guidelines, erros não devem vazar detalhes internos.
🔧 Sugestão: logar o erro completo mas retornar mensagem genérica
} catch (err) {
const msg = err instanceof Error ? err.message : "Erro desconhecido";
+ console.error("[webhook-dispatcher] internal error:", err);
- return buildErrorResponse("internal_error", msg, req, corsHeaders, { status: 500 });
+ return buildErrorResponse("internal_error", "Erro interno do servidor", req, corsHeaders, { status: 500 });
}🤖 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/functions/webhook-dispatcher/index.ts` around lines 276 - 279, No
bloco catch que usa buildErrorResponse, não retorne err.message diretamente ao
cliente; em vez disso registre o erro completo (por exemplo via console.error or
processLogger.error) para diagnóstico e passe uma mensagem genérica como "Erro
interno" para buildErrorResponse; atualize o catch que referencia err e
buildErrorResponse para logar todo o objeto de erro e substituir msg por um
texto não revelador antes de retornar.
![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: 97066e206d
ℹ️ 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".
| let rawJson: unknown; | ||
| try { | ||
| const text = await req.text(); | ||
| rawJson = text ? JSON.parse(text) : {}; |
There was a problem hiding this comment.
This path maps an empty request body to {} (rawJson = text ? JSON.parse(text) : {}), so POST /webhook-dispatcher with no body now fails schema validation and returns 422 validation_failed instead of the contract’s malformed/empty-body class (400, e.g. empty_body). That makes empty-body handling inconsistent with the other webhook handlers and with the shared validation contract, and can break clients that distinguish transport/parsing errors from semantic validation failures.
Useful? React with 👍 / 👎.
| name: "webhook-dispatcher", | ||
| endpoint: "webhook-dispatcher", | ||
| scenarios: [ |
There was a problem hiding this comment.
The dispatcher contract block defines no auth header, but webhook-dispatcher rejects requests with 401 unauthorized whenever WEBHOOK_DISPATCHER_SECRET is configured (the hardening path in the function). In those environments, every dispatcher scenario here will fail before schema validation and never reach the expected 422 assertions, so the live contract suite becomes unusable unless it sends x-dispatcher-secret.
Useful? React with 👍 / 👎.
| if (!envelopeParse.success) { | ||
| return buildValidationErrorResponse(envelopeParse.error, req, corsHeaders); |
There was a problem hiding this comment.
Malformed x-signature-256/x-webhook-signature values are now rejected at envelope validation time and return immediately, so those requests never reach inbound_webhook_events insertion or endpoint counters. Previously they were recorded as invalid signature attempts; with this change, malformed-signature traffic disappears from audit/monitoring data, which can hide real attack noise and skew invalid-rate metrics.
Useful? React with 👍 / 👎.
| 'Access-Control-Allow-Headers': ALLOWED_HEADERS_VALUE, | ||
| 'Access-Control-Allow-Methods': 'GET, POST, PUT, DELETE, OPTIONS', | ||
| 'Access-Control-Expose-Headers': 'x-request-id', | ||
| 'Access-Control-Expose-Headers': 'x-request-id, x-api-version', |
There was a problem hiding this comment.
This change introduces X-API-Version negotiation/echoing, but only the restricted CORS base expose list was updated here. The webhook endpoints use buildPublicCorsHeaders, which still exposes only x-request-id, so browser clients making cross-origin calls cannot read X-API-Version from responses despite the new contract behavior.
Useful? React with 👍 / 👎.
| signature: signatureHeader || undefined, | ||
| }); | ||
| if (!envelopeParse.success) { | ||
| return buildValidationErrorResponse(envelopeParse.error, req, corsHeaders); |
There was a problem hiding this comment.
By validating signature syntax in the envelope phase, malformed signature headers now return 422 validation_failed instead of the webhook auth error path (401 invalid_signature). That splits one authentication failure mode into two status/code families and can break callers and monitors that currently treat all bad signatures uniformly as invalid_signature.
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.
8 issues found across 14 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="scripts/contract-testing.mjs">
<violation number="1" location="scripts/contract-testing.mjs:27">
P0: Remova o fallback hardcoded de `SUPABASE_SERVICE_ROLE_KEY`; manter credencial no código viola a política de segredos e pode expor acesso privilegiado.</violation>
<violation number="2" location="scripts/contract-testing.mjs:246">
P2: O fallback de parsing lê o body duas vezes; após `response.json()` falhar, `response.text()` pode não retornar o conteúdo real, quebrando validações de resposta inválida.</violation>
</file>
<file name="supabase/functions/webhook-dispatcher/index.ts">
<violation number="1" location="supabase/functions/webhook-dispatcher/index.ts:22">
P2: `X-API-Version` is returned but not exposed in CORS for this endpoint, so browser callers cannot read the negotiated version header.</violation>
<violation number="2" location="supabase/functions/webhook-dispatcher/index.ts:63">
P2: Handle empty request bodies before schema parsing; coercing `""` to `{}` turns an `empty_body` case into a 422 validation error and breaks the unified error contract.</violation>
</file>
<file name="supabase/functions/_shared/validation-errors.ts">
<violation number="1" location="supabase/functions/_shared/validation-errors.ts:99">
P2: `buildValidationErrorV1` drops `formErrors` when field errors are present, which can hide root-level validation failures in mixed-error cases.</violation>
</file>
<file name="src/lib/validation-errors.ts">
<violation number="1" location="src/lib/validation-errors.ts:100">
P2: O type guard de v2 está incompleto: valide também o formato de cada item de `fields` para evitar narrowing incorreto e erros em runtime.</violation>
<violation number="2" location="src/lib/validation-errors.ts:110">
P2: O guard de v1 aceita `details: null`; adicione checagem de não-nulo antes de validar tipo.</violation>
</file>
<file name="supabase/functions/webhook-inbound/index.ts">
<violation number="1" location="supabase/functions/webhook-inbound/index.ts:14">
P2: `X-API-Version` is not exposed via CORS for this public webhook, so browser clients cannot read the negotiated version header.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const SUPABASE_URL = process.env.SUPABASE_URL || "https://pqpdolkaeqlyzpdpbizo.supabase.co"; | ||
| // Usando a chave de simulação estável definida para este projeto | ||
| const SERVICE_ROLE_KEY = "a46c3981-244a-4f81-9f57-bab5c45b5cde"; | ||
| const SERVICE_ROLE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY || "a46c3981-244a-4f81-9f57-bab5c45b5cde"; |
There was a problem hiding this comment.
P0: Remova o fallback hardcoded de SUPABASE_SERVICE_ROLE_KEY; manter credencial no código viola a política de segredos e pode expor acesso privilegiado.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/contract-testing.mjs, line 27:
<comment>Remova o fallback hardcoded de `SUPABASE_SERVICE_ROLE_KEY`; manter credencial no código viola a política de segredos e pode expor acesso privilegiado.</comment>
<file context>
@@ -1,112 +1,288 @@
const SUPABASE_URL = process.env.SUPABASE_URL || "https://pqpdolkaeqlyzpdpbizo.supabase.co";
-// Usando a chave de simulação estável definida para este projeto
-const SERVICE_ROLE_KEY = "a46c3981-244a-4f81-9f57-bab5c45b5cde";
+const SERVICE_ROLE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY || "a46c3981-244a-4f81-9f57-bab5c45b5cde";
+const PRODUCT_SECRET = process.env.N8N_PRODUCT_WEBHOOK_SECRET || "sim-secret";
+
</file context>
| const SERVICE_ROLE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY || "a46c3981-244a-4f81-9f57-bab5c45b5cde"; | |
| const SERVICE_ROLE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY; | |
| if (!SERVICE_ROLE_KEY) throw new Error("SUPABASE_SERVICE_ROLE_KEY is required"); |
| try { | ||
| responseData = await response.json(); | ||
| } catch { | ||
| responseData = { _raw: await response.text().catch(() => "") }; |
There was a problem hiding this comment.
P2: O fallback de parsing lê o body duas vezes; após response.json() falhar, response.text() pode não retornar o conteúdo real, quebrando validações de resposta inválida.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/contract-testing.mjs, line 246:
<comment>O fallback de parsing lê o body duas vezes; após `response.json()` falhar, `response.text()` pode não retornar o conteúdo real, quebrando validações de resposta inválida.</comment>
<file context>
@@ -1,112 +1,288 @@
+ try {
+ responseData = await response.json();
+ } catch {
+ responseData = { _raw: await response.text().catch(() => "") };
+ }
</file context>
| const corsHeaders = buildPublicCorsHeaders({ | ||
| allowMethods: "POST, OPTIONS", | ||
| extraAllowHeaders: ["x-api-version", "x-dispatcher-secret"], | ||
| }); |
There was a problem hiding this comment.
P2: X-API-Version is returned but not exposed in CORS for this endpoint, so browser callers cannot read the negotiated version header.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/webhook-dispatcher/index.ts, line 22:
<comment>`X-API-Version` is returned but not exposed in CORS for this endpoint, so browser callers cannot read the negotiated version header.</comment>
<file context>
@@ -11,20 +11,17 @@
- // Test mode (Onda 13 #9): dispatch to a specific webhook, no metrics, no breaker, no DB log
- test_mode: z.boolean().optional(),
- test_webhook_id: z.string().uuid().optional(),
+const corsHeaders = buildPublicCorsHeaders({
+ allowMethods: "POST, OPTIONS",
+ extraAllowHeaders: ["x-api-version", "x-dispatcher-secret"],
</file context>
| const corsHeaders = buildPublicCorsHeaders({ | |
| allowMethods: "POST, OPTIONS", | |
| extraAllowHeaders: ["x-api-version", "x-dispatcher-secret"], | |
| }); | |
| const corsHeaders = { | |
| ...buildPublicCorsHeaders({ | |
| allowMethods: "POST, OPTIONS", | |
| extraAllowHeaders: ["x-api-version", "x-dispatcher-secret"], | |
| }), | |
| "Access-Control-Expose-Headers": "x-request-id, x-api-version", | |
| }; |
| const hasFieldErrors = Object.keys(fieldErrors).length > 0; | ||
| return { | ||
| error: "Validation failed", | ||
| details: hasFieldErrors |
There was a problem hiding this comment.
P2: buildValidationErrorV1 drops formErrors when field errors are present, which can hide root-level validation failures in mixed-error cases.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/_shared/validation-errors.ts, line 99:
<comment>`buildValidationErrorV1` drops `formErrors` when field errors are present, which can hide root-level validation failures in mixed-error cases.</comment>
<file context>
@@ -0,0 +1,182 @@
+ const hasFieldErrors = Object.keys(fieldErrors).length > 0;
+ return {
+ error: "Validation failed",
+ details: hasFieldErrors
+ ? (fieldErrors as Record<string, string[]>)
+ : formErrors,
</file context>
| return ( | ||
| typeof o.error === 'string' && | ||
| 'details' in o && | ||
| (typeof o.details === 'object' || Array.isArray(o.details)) |
There was a problem hiding this comment.
P2: O guard de v1 aceita details: null; adicione checagem de não-nulo antes de validar tipo.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/validation-errors.ts, line 110:
<comment>O guard de v1 aceita `details: null`; adicione checagem de não-nulo antes de validar tipo.</comment>
<file context>
@@ -0,0 +1,112 @@
+ return (
+ typeof o.error === 'string' &&
+ 'details' in o &&
+ (typeof o.details === 'object' || Array.isArray(o.details))
+ );
+}
</file context>
| o.code === VALIDATION_ERROR_CODE && | ||
| o.version === 'v2' && | ||
| typeof o.message === 'string' && | ||
| Array.isArray(o.fields) |
There was a problem hiding this comment.
P2: O type guard de v2 está incompleto: valide também o formato de cada item de fields para evitar narrowing incorreto e erros em runtime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/validation-errors.ts, line 100:
<comment>O type guard de v2 está incompleto: valide também o formato de cada item de `fields` para evitar narrowing incorreto e erros em runtime.</comment>
<file context>
@@ -0,0 +1,112 @@
+ o.code === VALIDATION_ERROR_CODE &&
+ o.version === 'v2' &&
+ typeof o.message === 'string' &&
+ Array.isArray(o.fields)
+ );
+}
</file context>
| const corsHeaders = buildPublicCorsHeaders({ | ||
| extraAllowHeaders: ["x-signature-256", "x-event", "x-webhook-signature", "x-api-version"], | ||
| allowMethods: "POST, OPTIONS", | ||
| }); |
There was a problem hiding this comment.
P2: X-API-Version is not exposed via CORS for this public webhook, so browser clients cannot read the negotiated version header.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/webhook-inbound/index.ts, line 14:
<comment>`X-API-Version` is not exposed via CORS for this public webhook, so browser clients cannot read the negotiated version header.</comment>
<file context>
@@ -5,8 +5,16 @@ import { createClient } from "https://esm.sh/@supabase/supabase-js@2.49.4";
+} from "../_shared/validation-errors.ts";
-const corsHeaders = buildPublicCorsHeaders({ extraAllowHeaders: ["x-signature-256","x-event"], allowMethods: "POST, OPTIONS" });
+const corsHeaders = buildPublicCorsHeaders({
+ extraAllowHeaders: ["x-signature-256", "x-event", "x-webhook-signature", "x-api-version"],
+ allowMethods: "POST, OPTIONS",
</file context>
| const corsHeaders = buildPublicCorsHeaders({ | |
| extraAllowHeaders: ["x-signature-256", "x-event", "x-webhook-signature", "x-api-version"], | |
| allowMethods: "POST, OPTIONS", | |
| }); | |
| const corsHeaders = { | |
| ...buildPublicCorsHeaders({ | |
| extraAllowHeaders: ["x-signature-256", "x-event", "x-webhook-signature", "x-api-version"], | |
| allowMethods: "POST, OPTIONS", | |
| }), | |
| "Access-Control-Expose-Headers": "x-request-id, x-api-version", | |
| }; |
| let rawJson: unknown; | ||
| try { | ||
| const text = await req.text(); | ||
| rawJson = text ? JSON.parse(text) : {}; |
There was a problem hiding this comment.
P2: Handle empty request bodies before schema parsing; coercing "" to {} turns an empty_body case into a 422 validation error and breaks the unified error contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/webhook-dispatcher/index.ts, line 63:
<comment>Handle empty request bodies before schema parsing; coercing `""` to `{}` turns an `empty_body` case into a 422 validation error and breaks the unified error contract.</comment>
<file context>
@@ -53,20 +50,23 @@ Deno.serve(async (req) => {
+ let rawJson: unknown;
+ try {
+ const text = await req.text();
+ rawJson = text ? JSON.parse(text) : {};
+ } catch {
+ return buildErrorResponse("invalid_json", "Invalid JSON in request body", req, corsHeaders, { status: 400 });
</file context>
| rawJson = text ? JSON.parse(text) : {}; | |
| if (!text || text.trim() === "") { | |
| return buildErrorResponse("empty_body", "Request body is required", req, corsHeaders, { status: 400 }); | |
| } | |
| rawJson = JSON.parse(text); |
Migrates every Edge Function that already validates with Zod to use the
shared `buildValidationErrorResponse` helper introduced in the previous
commit. Now the entire surface — webhooks, internal APIs, MCP key
management — emits a consistent v1/v2-aware response shape.
Tools added:
- scripts/migrate-edge-validation-errors.mjs (codemod that converted
the 8 inline patterns to the unified call site).
- scripts/check-unified-validation-errors.mjs (CI guard: forbids new
inline `Validation failed` / `Invalid input` / `validation_failed`
response shapes outside the shared helper).
- npm run check:unified-validation
Special cases handled manually:
- mcp-keys-{issue,revoke,rotate,update}: preserve their audit-log
side-effects and request_id propagation via the helper
`buildValidationErrorV2FromFields` for cross-field business rules.
- full-op-diagnostics: switched to `buildValidationErrorResponse`.
- validate-access: intentionally exempt (silent passthrough fallback;
never returns a validation error).
Tests: 104 (was 101) — adds 3 cases for buildValidationErrorV2FromFields.
Lists all 38 Edge Functions now on the unified envelope (3 main webhooks with canonical schemas + 35 internal endpoints using buildValidationErrorResponse). Documents the validate-access exemption.
There was a problem hiding this comment.
5 issues found across 42 files (changes from recent commits).
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/functions/mcp-keys-issue/index.ts">
<violation number="1" location="supabase/functions/mcp-keys-issue/index.ts:235">
P1: Using `buildValidationErrorV2` unconditionally bypasses version negotiation and breaks backward compatibility for callers expecting the v1 format. Use `detectContractVersion(req)` + `buildValidationError(parsed.error, version)` instead, so v1 callers still receive the legacy shape and only opt-in v2 callers get the canonical format.</violation>
</file>
<file name="supabase/functions/mcp-keys-revoke/index.ts">
<violation number="1" location="supabase/functions/mcp-keys-revoke/index.ts:130">
P1: The response uses the hardcoded v2 builder instead of honoring client version negotiation.</violation>
</file>
<file name="scripts/check-unified-validation-errors.mjs">
<violation number="1" location="scripts/check-unified-validation-errors.mjs:24">
P1: CI guard can be bypassed by constructing the error object in a variable before passing to JSON.stringify/jsonResponse. The three regex patterns only match inline object literals, so variable-assigned error objects bypass detection entirely.</violation>
</file>
<file name="supabase/functions/external-db-bridge/index.ts">
<violation number="1" location="supabase/functions/external-db-bridge/index.ts:457">
P2: Validation error responses will lose the `request_id` field and `REQUEST_ID_HEADER` that `jsonResponse` normally injects, degrading debuggability.</violation>
</file>
<file name="supabase/functions/mcp-keys-update/index.ts">
<violation number="1" location="supabase/functions/mcp-keys-update/index.ts:145">
P1: As chamadas `buildValidationErrorV2` e `buildValidationErrorV2FromFields` retornam sempre o formato v2, ignorando a negociação de versão descrita no PR. O `req` está disponível no escopo do handler, e a biblioteca compartilhada oferece `detectContractVersion` e `buildValidationError` para decidir o formato com base no header/query param. Sem essa negociação, clientes existentes que esperam o formato v1 (ou o formato inline anterior `{ error, fields }`) receberão um shape diferente, quebrando a retrocompatibilidade prometida no PR.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const fields = parsed.error.flatten().fieldErrors; | ||
| await auditFailure("denied", "mcp_key.issue_denied", { reason: "validation_failed", fields }); | ||
| return jsonResponse({ error: "validation_failed", fields }, 422, requestId); | ||
| return jsonResponse(buildValidationErrorV2(parsed.error), 422, requestId); |
There was a problem hiding this comment.
P1: Using buildValidationErrorV2 unconditionally bypasses version negotiation and breaks backward compatibility for callers expecting the v1 format. Use detectContractVersion(req) + buildValidationError(parsed.error, version) instead, so v1 callers still receive the legacy shape and only opt-in v2 callers get the canonical format.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/mcp-keys-issue/index.ts, line 235:
<comment>Using `buildValidationErrorV2` unconditionally bypasses version negotiation and breaks backward compatibility for callers expecting the v1 format. Use `detectContractVersion(req)` + `buildValidationError(parsed.error, version)` instead, so v1 callers still receive the legacy shape and only opt-in v2 callers get the canonical format.</comment>
<file context>
@@ -231,7 +232,7 @@ Deno.serve(async (req) => {
const fields = parsed.error.flatten().fieldErrors;
await auditFailure("denied", "mcp_key.issue_denied", { reason: "validation_failed", fields });
- return jsonResponse({ error: "validation_failed", fields }, 422, requestId);
+ return jsonResponse(buildValidationErrorV2(parsed.error), 422, requestId);
}
const { name, scopes, expires_at, justification, step_up_token, target_repo, target_tool } = parsed.data;
</file context>
| const fields = parsed.error.flatten().fieldErrors; | ||
| await auditFailure("denied", "mcp_key.revoke_denied", { reason: "validation_failed", fields }); | ||
| return jsonResponse({ error: "validation_failed", fields }, 422, requestId); | ||
| return jsonResponse(buildValidationErrorV2(parsed.error), 422, requestId); |
There was a problem hiding this comment.
P1: The response uses the hardcoded v2 builder instead of honoring client version negotiation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/mcp-keys-revoke/index.ts, line 130:
<comment>The response uses the hardcoded v2 builder instead of honoring client version negotiation.</comment>
<file context>
@@ -126,7 +127,7 @@ Deno.serve(async (req) => {
const fields = parsed.error.flatten().fieldErrors;
await auditFailure("denied", "mcp_key.revoke_denied", { reason: "validation_failed", fields });
- return jsonResponse({ error: "validation_failed", fields }, 422, requestId);
+ return jsonResponse(buildValidationErrorV2(parsed.error), 422, requestId);
}
const { key_id, reason, step_up_token } = parsed.data;
</file context>
| ]); | ||
|
|
||
| // Patterns that signal an inline validation-error response. | ||
| const FORBIDDEN_PATTERNS = [ |
There was a problem hiding this comment.
P1: CI guard can be bypassed by constructing the error object in a variable before passing to JSON.stringify/jsonResponse. The three regex patterns only match inline object literals, so variable-assigned error objects bypass detection entirely.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/check-unified-validation-errors.mjs, line 24:
<comment>CI guard can be bypassed by constructing the error object in a variable before passing to JSON.stringify/jsonResponse. The three regex patterns only match inline object literals, so variable-assigned error objects bypass detection entirely.</comment>
<file context>
@@ -0,0 +1,72 @@
+]);
+
+// Patterns that signal an inline validation-error response.
+const FORBIDDEN_PATTERNS = [
+ // new Response(JSON.stringify({ error: "Validation failed" | "Invalid input" | ... + ZodErr.flatten() }))
+ /JSON\.stringify\(\s*\{[^{}]*error:[^{}]*["'](?:Validation failed|Invalid input|Dados inválidos|Payload inválido|invalid_input|validation_failed)["'][^{}]*\.error\.flatten\(\)[^{}]*\}\s*\)/,
</file context>
| const fields = parsed.error.flatten().fieldErrors; | ||
| await auditFailure("denied", { reason: "validation_failed", fields }); | ||
| return jsonResponse({ error: "validation_failed", fields }, 422, requestId); | ||
| return jsonResponse(buildValidationErrorV2(parsed.error), 422, requestId); |
There was a problem hiding this comment.
P1: As chamadas buildValidationErrorV2 e buildValidationErrorV2FromFields retornam sempre o formato v2, ignorando a negociação de versão descrita no PR. O req está disponível no escopo do handler, e a biblioteca compartilhada oferece detectContractVersion e buildValidationError para decidir o formato com base no header/query param. Sem essa negociação, clientes existentes que esperam o formato v1 (ou o formato inline anterior { error, fields }) receberão um shape diferente, quebrando a retrocompatibilidade prometida no PR.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/mcp-keys-update/index.ts, line 145:
<comment>As chamadas `buildValidationErrorV2` e `buildValidationErrorV2FromFields` retornam sempre o formato v2, ignorando a negociação de versão descrita no PR. O `req` está disponível no escopo do handler, e a biblioteca compartilhada oferece `detectContractVersion` e `buildValidationError` para decidir o formato com base no header/query param. Sem essa negociação, clientes existentes que esperam o formato v1 (ou o formato inline anterior `{ error, fields }`) receberão um shape diferente, quebrando a retrocompatibilidade prometida no PR.</comment>
<file context>
@@ -141,7 +142,7 @@ Deno.serve(async (req) => {
const fields = parsed.error.flatten().fieldErrors;
await auditFailure("denied", { reason: "validation_failed", fields });
- return jsonResponse({ error: "validation_failed", fields }, 422, requestId);
+ return jsonResponse(buildValidationErrorV2(parsed.error), 422, requestId);
}
const { key_id, name, description, scopes, expires_at, justification, confirmation_phrase, step_up_token } = parsed.data;
</file context>
| const parsed = TopLevelBodySchema.safeParse(rawBody); | ||
| if (!parsed.success) { | ||
| return jsonResponse({ error: 'Validation failed', details: parsed.error.flatten().fieldErrors }, 400, corsHeaders); | ||
| return buildValidationErrorResponse(parsed.error, req, corsHeaders); |
There was a problem hiding this comment.
P2: Validation error responses will lose the request_id field and REQUEST_ID_HEADER that jsonResponse normally injects, degrading debuggability.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/external-db-bridge/index.ts, line 457:
<comment>Validation error responses will lose the `request_id` field and `REQUEST_ID_HEADER` that `jsonResponse` normally injects, degrading debuggability.</comment>
<file context>
@@ -453,7 +454,7 @@ Deno.serve((req) => {
const parsed = TopLevelBodySchema.safeParse(rawBody);
if (!parsed.success) {
- return jsonResponse({ error: 'Validation failed', details: parsed.error.flatten().fieldErrors }, 400, corsHeaders);
+ return buildValidationErrorResponse(parsed.error, req, corsHeaders);
}
</file context>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40ec889533
ℹ️ 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".
| } | ||
| return map; | ||
| } |
There was a problem hiding this comment.
The validation replacement in secrets-manager left a dangling return map;/brace sequence after if (!parsed.success), which removes the original parsed.data destructuring and helper definitions and makes the function syntactically invalid (Expected "finally" but found "async" when parsed). This means the edge function cannot be built/deployed in this state, so all secrets-manager requests are effectively blocked until this block is restored.
Useful? React with 👍 / 👎.
| const rawBody = await req.json().catch(() => ({})); | ||
| const parsed = CategoriesRequestSchema.safeParse(rawBody); | ||
| if (!parsed.success) { | ||
| return new Response(JSON.stringify({ error: 'Validation failed', details: parsed.error.flatten().fieldErrors }), { | ||
| status: 400, headers: { ...corsHeaders, 'Content-Type': 'application/json' }, | ||
| }); | ||
| return buildValidationErrorResponse(parsed.error, req, corsHeaders); |
There was a problem hiding this comment.
This handler swallows JSON parse failures with req.json().catch(() => ({})) and then routes them through buildValidationErrorResponse, which now emits a 422 schema error instead of a malformed-body 400. That reclassifies invalid JSON payloads (transport/parsing errors) as semantic validation failures, which conflicts with the new contract’s invalid_json/400 behavior and can break clients that branch on 400 vs 422.
Useful? React with 👍 / 👎.
| if (!parsed.success) { | ||
| return new Response( | ||
| JSON.stringify({ error: 'Parâmetros inválidos', details: parsed.error.flatten().fieldErrors }), | ||
| { status: 400, headers: { ...corsHeaders, 'Content-Type': 'application/json' } }, | ||
| ); | ||
| } | ||
|
|
||
| const name = (parsed.data.name ?? '').trim(); | ||
| const items = parsed.data.items ?? []; | ||
| if (!name && items.length === 0) { | ||
| return new Response(JSON.stringify({ error: 'Forneça name ou items' }), { | ||
| status: 400, headers: { ...corsHeaders, 'Content-Type': 'application/json' }, | ||
| }); | ||
| return buildValidationErrorResponse(parsed.error, req, corsHeaders); | ||
| } |
There was a problem hiding this comment.
Replacing the validation error block here also removed the parsed.data assignments/guard for name and items, but the function still uses both variables when building itemList and userPrompt. On any valid request this now throws a runtime ReferenceError before the AI call, causing the endpoint to fail instead of returning a suggestion.
Useful? React with 👍 / 👎.
|
Fechando como duplicata. PR #45 ( Este PR cobre os mesmos 3 webhooks que #45 com formato muito próximo (envelope 422 + v1/v2). Principais diferenças:
A próxima onda de migração para mais 13 endpoints está em #69. Obrigado — análise aqui foi útil para validar a abordagem. |
Resumo
Implementa o contrato unificado de validação para todos os webhooks e Edge Functions do Promo Gifts V4, conforme escopo de auditoria.
Cobertura inicial:
product-webhook,webhook-dispatcher,webhook-inbound(os três webhooks do projeto). A infraestrutura criada é genérica e pronta para ser propagada às 40+ Edge Functions restantes que já usam Zod (próximo passo, fora deste PR).O que muda para os callers
{ "error": "Validation failed", "details": { "sku": ["..."] } }{ "code": "validation_failed", "message": "Validation failed", "version": "v2", "fields": [{ "path": "product.images.0", "code": "invalid_string", "message": "..." }] }?api_version=v2, headerX-API-Version: v2, ouAccept: application/vnd.promogifts.v2+json.Arquitetura
supabase/functions/_shared/validation-errors.tssupabase/functions/_shared/webhook-schemas.tssupabase/functions/_shared/zod-validate.tsparseBodyWithSchemarefatorado para usar a infra unificadasrc/lib/validation-errors.tssrc/lib/webhook-schemas.tstests/edge-functions/validation-errors.test.tstests/edge-functions/webhook-schemas.contract.test.tstests/edge-functions/webhook-schemas-parity.test.tsscripts/contract-testing.mjsdocs/WEBHOOKS_CONTRACT.mdCenários de contrato cobertos (offline)
✅ Happy path para cada ação (
upsert,sync,batch_upsert,delete)✅ Campos obrigatórios ausentes (
sku,name,price,event,slug, ...)✅ Tipos incorretos (
price: "abc",test_mode: "yes", ...)✅ Valores vazios (
sku: "",event: "",external_ids: [], ...)✅ Regras cross-field (
upsertsemproduct,test_modesemtest_webhook_id, ...)✅ Propagação de erros aninhados (
product.sku,products.2.price)✅ Limites de tamanho (
products> 500,images> 50)✅ Versionamento: v1 e v2 carregam o mesmo conteúdo semântico
✅ Invariante v1 ⊂ v2 — nenhuma informação é perdida na transição
Resultado: 101 testes em
tests/edge-functions/, todos verdes em ~3s.Compatibilidade retroativa
contract versioning: v1 ↔ v2 backwards compatibilityvalida que toda chave dedetailsem v1 corresponde a pelo menos umfields[].pathem v2.docs/WEBHOOKS_CONTRACT.md(announce → 90 dias → sunset).Auditoria — observações adicionais (não corrigidas neste PR)
Para entrega completa, fica registrado para próximos PRs:
buildValidationErrorResponsea todas.npm run testglobal são anteriores a este PR (snapshots, integrações React) e fora de escopo do contrato.Test plan
npm run test -- tests/edge-functions/— 101/101 passamsrc/lib/*.ts) — 0 errosnpm run test:contract(HTTP E2E) — depende de deploy em stagingGenerated by Claude Code
Summary by cubic
Unifies the HTTP 422 validation error envelope with v1/v2 versioning across all webhooks and internal APIs (38 Edge Functions total). Adds a CI guard and codemod to enforce the contract, with updated docs and tests.
New Features
mcp-keys-{issue,revoke,rotate,update}usebuildValidationErrorV2/buildValidationErrorV2FromFieldsto preserve audit side effects;full-op-diagnosticsmoved to the shared builder.scripts/migrate-edge-validation-errors.mjs(codemod) andscripts/check-unified-validation-errors.mjswithnpm run check:unified-validationto block inline error shapes in CI.X-API-Version; contract tests expanded to 104 andscripts/contract-testing.mjsenhanced; docs updated to list all 38 endpoints and thevalidate-accessexemption.Migration
?api_version=v2,X-API-Version: v2, or vendor Accept.Written for commit 40ec889. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Release Notes
Documentation
New Features
Tests