fix: reconciliar guard-rails e estado GitHub/Supabase - #80
Conversation
There was a problem hiding this comment.
adm01-debug has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
|
Warning Review limit reachedNext included review available in 26 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 107 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Analysis CompleteGenerated ECC bundle from 1 commits | Confidence: 50% View Pull Request #81Repository Profile
Changed Files (4)
Top hotspots
Top directories
Analysis Depth Readiness (commit-history, 7%)ECC Tools uses this to decide whether recommendations should stay at commit-history/setup guidance or expand into CI, security, harness, reference-set, AI-routing, and team backlog work.
Reference Set Readiness (0/7, 0%)
Likely Future Issues (2)
Suggested Follow-up Work (2)
Copy-ready bodies test: add regression coverage for supabase/functions/_shared/unsubscribe.ts + supabase/functions/campaign-health-alert/authz_test.ts ## Summary
- Add regression coverage for the recently touched code paths before more changes stack on top.
## Why
- Backfill regression coverage before another change set lands on the touched code paths.
## Touched paths
- `supabase/functions/_shared/unsubscribe.ts`
- `supabase/functions/campaign-health-alert/authz_test.ts`
## Validation
- Add or extend focused tests that exercise the touched paths.
- Run the affected test suite and verify the new coverage closes the gap.db: add migration follow-up for supabase/functions/_shared/unsubscribe.ts + supabase/functions/campaign-health-alert/authz_test.ts ## Summary
- Add the missing migration or schema rollout step for the recently changed schema surface.
## Why
- Backfill the missing migration artifact before another schema or model change lands on top.
## Touched paths
- `supabase/functions/_shared/unsubscribe.ts`
- `supabase/functions/campaign-health-alert/authz_test.ts`
## Validation
- Create the migration or schema rollout artifact used by this repo.
- Run the repo migration / schema validation flow and verify the changed models still match production expectations.Review Activity (1 reviews, 0 inline comments, 0 unresolved threads)
Latest reviewer states
Review Follow-up Signals (1)
Recommended next actions
Generated Instincts (6)
After merging, import with: Files
|
There was a problem hiding this comment.
adm01-debug has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
adm01-debug has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Decisão de merge P0: lint/typecheck, unitários, build, bundle size, empacotamento Edge, request-id e quality-gate estão verdes. Lighthouse completou três coletas e falhou por ausência de configuração Supabase no CI e débitos reais documentados; thresholds não foram relaxados. E2E geral permanece pendente sem credenciais/fixtures confiáveis, e o job enterprise não define timeout. A exceção não declara esses checks como aprovados. Nenhuma escrita em banco foi executada. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6df597bff
ℹ️ 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".
| const operationalSecretAssignmentPattern = | ||
| /\b(?:const|let|var)\s+[A-Za-z0-9_]*(?:ACCESS_KEY|SECRET|TOKEN|PASSWORD|PRIVATE_KEY)[A-Za-z0-9_]*\s*=\s*(['"`])(?:[0-9a-f]{32,}|[A-Za-z0-9_+/=-]{40,})\1/gi; |
There was a problem hiding this comment.
Execute o scanner nos checks de pull request
Em um PR que reintroduza uma credencial operacional literal, este novo detector não bloqueia o merge: verifiquei pr-checks.yml, edge-functions-bundle.yml, edge-functions-request-id.yml e os demais workflows, e nenhum deles executa npm run security:secrets; a única referência está no script manual de package.json. Portanto, apesar de o relatório classificar a mudança como guard-rail de CI, o padrão só terá efeito quando alguém o executar localmente; inclua o scanner em um workflow obrigatório, com gatilho também para alterações neste script.
AGENTS.md reference: AGENTS.md:L19-L20
Useful? React with 👍 / 👎.
| const operationalSecretAssignmentPattern = | ||
| /\b(?:const|let|var)\s+[A-Za-z0-9_]*(?:ACCESS_KEY|SECRET|TOKEN|PASSWORD|PRIVATE_KEY)[A-Za-z0-9_]*\s*=\s*(['"`])(?:[0-9a-f]{32,}|[A-Za-z0-9_+/=-]{40,})\1/gi; |
There was a problem hiding this comment.
Detecte declarações TypeScript tipadas de segredos
Mesmo após o scanner ser ligado ao CI, uma declaração TypeScript comum como const ACCESS_TOKEN: string = "0123456789abcdef0123456789abcdef" não casa com este padrão, pois ele exige que = venha imediatamente após o identificador. Assim, credenciais literais com anotação de tipo passam silenciosamente pelo guard-rail; aceite anotações opcionais entre o nome e a atribuição ou analise a sintaxe TypeScript em vez de depender apenas desta expressão regular.
Useful? React with 👍 / 👎.
| Deno.serve(withRequestId("migrate-helper", async (req, _ctx) => { | ||
| if (req.method === "OPTIONS") { | ||
| return new Response(null, { headers: corsHeaders }); | ||
| } |
There was a problem hiding this comment.
Respeite a allowlist de CORS no tombstone
Quando ALLOWED_ORIGINS está configurada, o preflight e a resposta 410 deste endpoint ainda retornam Access-Control-Allow-Origin: *, porque usam o valor estático corsHeaders; isso ignora a política configurada e permite que qualquer origem leia a resposta. O próprio contrato em _shared/cors.ts determina que novas Edge Functions usem getCorsHeaders(req) para aplicar a allowlist por requisição, portanto o tombstone deve calcular esses cabeçalhos tanto no OPTIONS quanto na resposta final.
Useful? React with 👍 / 👎.
Reconcilia o pacote funcional do PR #66 com o estado real do GitHub/Supabase e corrige falhas determinísticas encontradas na revalidação.\n\nCorreções: filtro de opt-out usa chunking canônico; testes de autorização/contrato deixam de depender de formato textual; migrate-helper aposentado passa a falhar fechado com 410, sem leitura ou exposição de credenciais; scanner bloqueia segredos operacionais literais; Lighthouse CI passa a servir o build dist como SPA.\n\nValidações: ESLint e typecheck verdes; Vitest 458 aprovados e 2 ignorados; build verde; recorte Deno do PR #66 51/51; regressões alvo 20/20; suíte Deno 543 aprovados e 26 falhas ambientais/de integração explicitadas; 170/170 funções no guard de request ID; três coletas Lighthouse completas. Assertions Lighthouse continuam vermelhas por configuração Supabase ausente e problemas reais documentados; budgets não foram relaxados.\n\nNenhuma escrita em banco foi realizada. O destino usyxfpqlsspldubptrdl permanece bloqueado por MCP 401, CLI 403 e ausência de credenciais no GitHub Actions; o relatório contém o gate seguro para a reconciliação remota.