[INCORPORATED] Fix/credentials audit burn down sprint1-4#458
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughEste PR migra a obtenção de credenciais em sete edge functions, movendo de ChangesMigração para Credential Vault
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Updates to Preview Branch (fix/credentials-audit-burn-down-sprint1-4) ↗︎
Tasks are run on every commit but only new migration files are pushed.
❌ Branch Error • Tue, 26 May 2026 16:26:46 UTC View logs for this Workflow Run ↗︎. |
Contributor
There was a problem hiding this comment.
Pull request overview
Este PR migra duas Edge Functions (materials-api e health-check) para resolver credenciais do Supabase externo (Promobrind) via credential vault (getCredential), reduzindo acessos diretos a Deno.env e atendendo itens da auditoria de credenciais (SSOT-bypass / leituras em module-scope).
Changes:
materials-api: troca leitura deEXTERNAL_SUPABASE_*porgetCredential('EXTERNAL_PROMOBRIND_*')e mantém fallback de RPC→query.health-check: troca leitura deEXTERNAL_SUPABASE_*porgetCredential('EXTERNAL_PROMOBRIND_*')no checker do banco externo.- Ajustes de formatação/compactação de trechos (sem mudança intencional de comportamento).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| supabase/functions/materials-api/index.ts | Usa credential vault para conectar no Supabase externo e simplifica trechos do handler/respostas. |
| supabase/functions/health-check/index.ts | Usa credential vault no checker do DB externo e mantém agregação do status de health. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const externalKey = await getCredential('EXTERNAL_PROMOBRIND_SERVICE_ROLE_KEY'); | ||
|
|
||
| if (!externalUrl || !externalKey) { | ||
| console.warn('[materials-api] EXTERNAL_SUPABASE_URL/KEY not configured — returning empty payload'); |
Comment on lines
+213
to
+216
| const errorMessage = error instanceof Error ? error.message : (error?.message || error?.error_description || error?.hint || error?.details || JSON.stringify(error) || 'Erro desconhecido'); | ||
| const errorCode = error?.code ?? null; | ||
| console.error('Materials API error:', errorMessage, 'code:', errorCode, 'raw:', JSON.stringify(error)); | ||
|
|
||
| return new Response( | ||
| JSON.stringify({ error: errorMessage, code: errorCode }), | ||
| { status: 500, headers: { ...corsHeaders, 'Content-Type': 'application/json' } } | ||
| ); | ||
| return new Response(JSON.stringify({ error: errorMessage, code: errorCode }), { status: 500, headers: { ...corsHeaders, 'Content-Type': 'application/json' } }); |
Comment on lines
1
to
+5
| import { createClient, SupabaseClient } from "https://esm.sh/@supabase/supabase-js@2.49.1"; | ||
| import { handleCorsPreflight } from "../_shared/cors.ts"; | ||
| import { getOrCreateRequestId } from "../_shared/request-id.ts"; | ||
| import { createStructuredLogger } from "../_shared/structured-logger.ts"; | ||
| import { getCredential } from "../_shared/credentials.ts"; |
Comment on lines
57
to
60
| // fix: ssot-bypass + module-scope-credential-read — use credential vault | ||
| const url = await getCredential('EXTERNAL_PROMOBRIND_URL'); | ||
| const key = await getCredential('EXTERNAL_PROMOBRIND_SERVICE_ROLE_KEY'); | ||
|
|
Comment on lines
+47
to
+49
| // fix: ssot-bypass — use credential vault (aliases EXTERNAL_SUPABASE_URL / SERVICE_ROLE_KEY) | ||
| const externalUrl = await getCredential('EXTERNAL_PROMOBRIND_URL'); | ||
| const externalKey = await getCredential('EXTERNAL_PROMOBRIND_SERVICE_ROLE_KEY'); |
|
Deployment failed with the following error: |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
adm01-debug
force-pushed
the
fix/credentials-audit-burn-down-sprint1-4
branch
from
c93f780 to
bdba710
Compare
3 tasks
![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
🧹 Nitpick comments (1)
supabase/functions/expert-chat/index.ts (1)
1001-1003: ⚡ Quick winEvite lookup duplicado do vault no fallback
As credenciais já são carregadas e validadas no bloco principal; no fallback elas são buscadas novamente sem necessidade. Isso adiciona latência e um ponto extra de falha por requisição. Reutilize
EXT_URL/EXT_KEYjá obtidos.💡 Diff sugerido
- // fix: ssot-bypass — credential vault - const EXT_URL = await getCredential('EXTERNAL_PROMOBRIND_URL'); - const EXT_KEY = await getCredential('EXTERNAL_PROMOBRIND_SERVICE_ROLE_KEY'); + // fix: ssot-bypass — credential vault + const EXT_URL = await getCredential('EXTERNAL_PROMOBRIND_URL'); + const EXT_KEY = await getCredential('EXTERNAL_PROMOBRIND_SERVICE_ROLE_KEY'); if (!EXT_URL || !EXT_KEY) { - console.error('External DB env vars not set — cannot fetch products'); + console.error('External DB credentials not set — cannot fetch products'); } else if ( expansion.intent === 'product_search' || expansion.intent === 'proposal' || expansion.searchTerms.length > 0 || fallbackTerms.length > 0 ) { const extClient = createClient(EXT_URL, EXT_KEY); ... } else { - // fix: ssot-bypass — credential vault - const EXT_URL2 = await getCredential('EXTERNAL_PROMOBRIND_URL'); - const EXT_KEY2 = await getCredential('EXTERNAL_PROMOBRIND_SERVICE_ROLE_KEY'); - if (EXT_URL2 && EXT_KEY2) { - const extClient = createClient(EXT_URL2, EXT_KEY2); + const extClient = createClient(EXT_URL, EXT_KEY); let q = extClient .from('products') .select( 'id, name, sku, sale_price, primary_image_url, category_id, description, brand, stock_quantity', ) .eq('active', true); ... - } }Also applies to: 1283-1285
🤖 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/expert-chat/index.ts` around lines 1001 - 1003, No bloco de fallback está sendo feito um lookup duplicado ao chamar getCredential('EXTERNAL_PROMOBRIND_URL') e getCredential('EXTERNAL_PROMOBRIND_SERVICE_ROLE_KEY') novamente; em vez disso reutilize as variáveis já carregadas EXT_URL e EXT_KEY (definidas no escopo principal) dentro do fallback para evitar latência e pontos extras de falha; localize o trecho que chama getCredential na seção de fallback e substitua essas chamadas por referências a EXT_URL e EXT_KEY (mantendo a validação existente caso necessário).
🤖 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/functions/simulation-orchestrator/index.ts`:
- Around line 63-64: Remove the hardcoded fallback by deleting the "??
'sim-secret'" default; call getCredential("N8N_PRODUCT_WEBHOOK_SECRET") (or
Deno.env.get("N8N_PRODUCT_WEBHOOK_SECRET") per guidelines) and if it returns
undefined, fail closed by returning/throwing a 503 response (log the missing
secret) instead of using a predictable secret; update the n8nSecret usage sites
to assume a present secret after this check (ref: n8nSecret variable and
getCredential call).
---
Nitpick comments:
In `@supabase/functions/expert-chat/index.ts`:
- Around line 1001-1003: No bloco de fallback está sendo feito um lookup
duplicado ao chamar getCredential('EXTERNAL_PROMOBRIND_URL') e
getCredential('EXTERNAL_PROMOBRIND_SERVICE_ROLE_KEY') novamente; em vez disso
reutilize as variáveis já carregadas EXT_URL e EXT_KEY (definidas no escopo
principal) dentro do fallback para evitar latência e pontos extras de falha;
localize o trecho que chama getCredential na seção de fallback e substitua essas
chamadas por referências a EXT_URL e EXT_KEY (mantendo a validação existente
caso necessário).
🪄 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: 962238b8-d424-496f-868d-91b375a540d5
📒 Files selected for processing (8)
.audit-credentials-baseline.jsonsupabase/functions/expert-chat/index.tssupabase/functions/product-webhook/index.tssupabase/functions/secure-upload/index.tssupabase/functions/send-scheduled-reports/index.tssupabase/functions/simulation-orchestrator/index.tssupabase/functions/sync-external-db/index.tssupabase/functions/webhook-dispatcher/index.ts
Comment on lines
+63
to
+64
| // fix: ssot-bypass — credential vault | ||
| const n8nSecret = await getCredential("N8N_PRODUCT_WEBHOOK_SECRET") ?? "sim-secret"; |
There was a problem hiding this comment.
Remover fallback de segredo hardcoded em Line 64.
?? "sim-secret" introduz segredo previsível quando o vault falha/ausenta credencial, enfraquecendo a proteção do webhook. Falhe fechado com 503 em vez de usar default secreto.
💡 Patch sugerido
- const n8nSecret = await getCredential("N8N_PRODUCT_WEBHOOK_SECRET") ?? "sim-secret";
+ const n8nSecret = await getCredential("N8N_PRODUCT_WEBHOOK_SECRET");
+ if (!n8nSecret) {
+ return new Response(JSON.stringify({ error: "service_misconfigured" }), {
+ status: 503,
+ headers: { ...corsHeaders, ...responseHeaders, "Content-Type": "application/json" },
+ });
+ }As per coding guidelines "Secrets sempre via Deno.env.get(), NUNCA hardcoded".
🤖 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/simulation-orchestrator/index.ts` around lines 63 - 64,
Remove the hardcoded fallback by deleting the "?? 'sim-secret'" default; call
getCredential("N8N_PRODUCT_WEBHOOK_SECRET") (or
Deno.env.get("N8N_PRODUCT_WEBHOOK_SECRET") per guidelines) and if it returns
undefined, fail closed by returning/throwing a 503 response (log the missing
secret) instead of using a predictable secret; update the n8nSecret usage sites
to assume a present secret after this check (ref: n8nSecret variable and
getCredential call).
…{}) + regenerate from clean state
Previous baseline used deprecated v2-counts schema with signatures{} object.
Audit script expects issues[] array format. Regenerated from actual scan:
28 (original) → 6 remaining (categories-api, cleanup-novelties, external-db-inspect).
cnpj-lookup already fixed by another PR in main.
- useGlobalSearch.ts: remove type SimpleQueryBuilder orphan (ficou do PR #380 mas untypedFrom do main não o usa) - MockupGenerator.tsx: remove import Badge não usado - MockupGenerator.tsx: renomeia summary → _summary (definida mas não usada no JSX) ESLint baseline gate: ✅ zero regressões (drift positivo: -19 erros)
adm01-debug
force-pushed
the
fix/credentials-audit-burn-down-sprint1-4
branch
from
110c0a2 to
0d42b1a
Compare
adm01-debug
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fechado: conteúdo incorporado ao main via commits diretos no VPS durante resolução de conflitos. As correções de credentials vault (expert-chat, sync-external-db, webhook-dispatcher, simulation-orchestrator, send-scheduled-reports, secure-upload, product-webhook) e o baseline atualizado (issues 28→6→0) já estão em origin/main. Diff atual entre main e esse branch: apenas arquivos modificados pelo Lovable após a data do PR, que não fazem parte desse escopo.