Skip to content

feat(crons): vault-based single source of truth para CRON_SECRET + fix ownership-audit UUID bug#223

Merged
adm01-debug merged 1 commit into
mainfrom
feat/cron-vault-sot
May 15, 2026
Merged

feat(crons): vault-based single source of truth para CRON_SECRET + fix ownership-audit UUID bug#223
adm01-debug merged 1 commit into
mainfrom
feat/cron-vault-sot

Conversation

@adm01-debug
Copy link
Copy Markdown
Owner

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

Resumo

Refatora autenticação de 13 cron edge functions + connections-auto-test para ler o CRON_SECRET esperado do vault PostgreSQL via RPC get_edge_function_secret(_name), com fallback para Deno.env.get() (retrocompat).

Resolve a inconsistência onde mudanças no CRON_SECRET precisavam ser propagadas manualmente para múltiplos lugares (vault PG + dashboard de secrets). Agora o vault é a única fonte de verdade — pg_cron + edge functions lêem do mesmo lugar.

Mudanças

supabase/config.toml

Adiciona verify_jwt = false para 13 cron functions. Sem isso, o CI/CD (deploy-edge-functions.yml) sobrescreve com verify_jwt = true (default) e quebra todas as crons → 401 UNAUTHORIZED_NO_AUTH_HEADER.

_shared/dispatcher-auth.ts

  • authorizeCron agora é async, retorna Promise<CronAuthResult>
  • Lê vault first via client.rpc('get_edge_function_secret', { _name })
  • Cache em memória por cold-start (_vaultCache: Map<string, Promise<string>>) → 1 RPC por instância, não por request
  • Fallback Deno.env.get() para retrocompat
  • Novo mode: 'secret_vault' nos logs estruturados
  • Comparação constant-time preservada (anti-timing-attack)

14 callers atualizados

13 crons + connections-auto-test agora usam await authorizeCron(req, ...).

send-scheduled-reports recebe bloco de auth completo (não tinha — era a única cron sem auth).

Migration 20260515120000_fix_audit_ownership_orphans_uuid_only.sql

Fix do bug reportado: ownership-audit retornava HTTP 500 com "invalid input syntax for type uuid: \"system\"".

Causa raiz: enriched_contacts.created_by é coluna TEXT com 48 rows contendo o valor "system". A função audit_ownership_orphans fazia EXECUTE format('... t.%I::uuid)', col) quebrando no cast.

Fix: filtrar por c.data_type = 'uuid' no information_schema (mais robusto que manter blacklist hardcoded de tabelas). Remove o cast ::uuid desnecessário.

Validação esperada após merge

CI deve auto-deployar 13 crons + connections-auto-test com verify_jwt = false e novo código. Validação SQL:

SELECT net.http_post(
  url := 'https://doufsxqlfjyuvxuezpln.supabase.co/functions/v1/<NAME>',
  headers := jsonb_build_object('x-cron-secret', public.get_edge_function_secret('CRON_SECRET'), 'Content-Type','application/json'),
  body := '{}'::jsonb
);

Esperado: 11 retornam HTTP 200, send-notification retorna 400 (payload inválido por design), ownership-audit retorna 200 (bug corrigido pela migration).

Não-impactos

  • webhook-dispatcher mantém autenticação Modo A/B (não usa authorizeCron)
  • Funções já protegidas em outras sessões (F1 IA/svc_role) não são tocadas
  • Token JWT do dashboard pode ser usado opcionalmente como fallback (env)
  • Cron jobs no pg_cron já lêem CRON_SECRET do vault desde PR feat(edges): createEdge template + auth hardening F1/F2 #217

Risco

Baixo. Mudança aditiva: vault first, fallback env. Mesmo se vault retornar string vazia, o Deno.env.get antigo segue funcionando.


Summary by cubic

Make the Postgres vault the single source of truth for cron secrets and fix a UUID cast error in ownership-audit. 13 cron edge functions and connections-auto-test now read CRON_SECRET via RPC with env fallback.

  • New Features

    • authorizeCron reads secrets from the vault via get_edge_function_secret, caches per cold start, and falls back to Deno.env.
    • Updated 13 cron functions and connections-auto-test to await authorizeCron; send-scheduled-reports now enforces x-cron-secret.
    • Set verify_jwt = false for all cron functions in supabase/config.toml to avoid 401s from defaults.
    • Structured logs include mode: 'secret_vault'; constant-time comparison retained.
  • Bug Fixes

    • Migration updates audit_ownership_orphans to only scan uuid columns and removes the ::uuid cast, fixing "invalid input syntax for type uuid: \"system\"" in ownership-audit.

Written for commit 0599e28. Summary will update on new commits.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Corrigida auditoria de propriedade para considerar apenas colunas de tipo UUID, evitando erros de conversão de dados.
    • Melhorada confiabilidade de funções agendadas com autenticação aprimorada.
  • Refactor

    • Implementado armazenamento seguro de secrets em vault para autenticação de tarefas agendadas, com fallback para variáveis de ambiente.
    • Otimizado fluxo de validação de segurança das funções cron com processamento assíncrono.

Review Change Stack

Copilot AI review requested due to automatic review settings May 15, 2026 12:52
@vercel
Copy link
Copy Markdown

vercel Bot commented May 15, 2026

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

Project Deployment Actions Updated (UTC)
promo-gifts Ready Ready Preview, Comment May 15, 2026 0:52am

@supabase
Copy link
Copy Markdown

supabase Bot commented May 15, 2026

Updates to Preview Branch (feat/cron-vault-sot) ↗︎

Deployments Status Updated
Database Fri, 15 May 2026 12:55:10 UTC
Services Fri, 15 May 2026 12:55:10 UTC
APIs Fri, 15 May 2026 12:55:10 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Fri, 15 May 2026 12:55:20 UTC
Migrations Fri, 15 May 2026 12:55:23 UTC
Seeding ⏸️ Fri, 15 May 2026 12:52:51 UTC
Edge Functions ⏸️ Fri, 15 May 2026 12:52:51 UTC

❌ Branch Error • Fri, 15 May 2026 12:55:24 UTC

ERROR: duplicate key value violates unique constraint "schema_migrations_pkey" (SQLSTATE 23505)
Key (version)=(20250102000000) already exists.
At statement: 21
INSERT INTO supabase_migrations.schema_migrations(version, name, statements) VALUES($1, $2, $3)

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Walkthrough

PR refatora autenticação de crons para usar vault como source-of-truth de secrets com fallback para env vars, adiciona cache por cold-start, transforma authorizeCron em async, integra await em 13 handlers e adiciona nova validação em send-scheduled-reports. Simultaneamente, corrige audit_ownership_orphans para auditar apenas colunas UUID, evitando cast inválido em TEXT.

Changes

Vault-Based Cron Authentication

Layer / File(s) Summary
Configuration and Type System
supabase/config.toml, supabase/functions/_shared/dispatcher-auth.ts
config.toml declara verify_jwt = false para 13 cron functions; CronAuthMode adiciona enum secret_vault; documentação de dispatcher-auth atualizada para vault SoT com fallback/cache.
Vault Secret Caching Infrastructure
supabase/functions/_shared/dispatcher-auth.ts
getVaultSecret() privado fetcha secret via RPC get_edge_function_secret(_name) com cache _vaultCache por cold-start; fallback retorna string vazia em falhas/indisponibilidade.
authorizeCron Async Refactor
supabase/functions/_shared/dispatcher-auth.ts
authorizeCron muda de síncrona para async, resolve secret no vault primeiro (via getVaultSecret definindo viaVault), fallback para Deno.env.get(), logs/return incluem via_vault flag, modo retornado é secret_vault (vault) ou secret (env). Pequenos ajustes em authorizeDispatcher Modo B e comentários auxiliares.
Cron Handlers - Async Auth Integration
supabase/functions/cleanup-notifications/..., supabase/functions/cleanup-novelties/..., supabase/functions/collections-watcher/..., supabase/functions/comparison-price-watcher/..., supabase/functions/connections-auto-test/..., supabase/functions/connections-health-check/..., supabase/functions/favorites-watcher/..., supabase/functions/ownership-audit/..., supabase/functions/process-queue/..., supabase/functions/process-scheduled-reports/..., supabase/functions/quote-followup-reminders/..., supabase/functions/send-digest/..., supabase/functions/send-notification/...
13 cron functions adicionam await em authorizeCron(...), garantindo que cronAuth seja resolvido antes de validar cronAuth.ok e proceder ou retornar erro 401.
send-scheduled-reports Authorization Addition
supabase/functions/send-scheduled-reports/index.ts
Novo handler adiciona validação inicial de authorizeCron com secretEnvName: "CRON_SECRET" esperando header x-cron-secret; retorna cronAuth.response (401) se falhar antes de processar relatórios.

Audit Ownership Orphans UUID-Only Fix

Layer / File(s) Summary
audit_ownership_orphans UUID Type Filter
supabase/migrations/20260515120000_fix_audit_ownership_orphans_uuid_only.sql
Nova migration cria public.audit_ownership_orphans(_triggered_by) SECURITY DEFINER que audita apenas colunas data_type='uuid' em ownership (seller_id, user_id, owner_id, created_by), evitando cast ::uuid inválido em TEXT (ex.: enriched_contacts.created_by='system'). Verifica permissions, varre tabelas públicas excluindo lista fixa, conta NULLs/orphans, agrega details em jsonb, calcula RLS coverage via public.audit_rls_coverage(), insere resumo em public.ownership_audit_reports, retorna report id.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • adm01-debug/Promo_Gifts#189: Introduziu authorizeCron e uso de secrets/headers para connections-auto-test; este PR refatora esse mesmo guard para async com vault-based secrets.
  • adm01-debug/Promo_Gifts#193: Adiciona testes unitários (cronAuth.test.ts, dispatcherAuth.test.ts) para os mesmos guards que esta PR refatora.
  • adm01-debug/Promo_Gifts#168: Toca public.audit_ownership_orphans revogando EXECUTE; este PR corrige a lógica interna da mesma função.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed O título descreve precisamente as duas mudanças principais: refatoração de vault para cron secrets e correção do bug de UUID no ownership-audit.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/cron-vault-sot

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

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0599e2879e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +278 to +281
let expectedSecret = await getVaultSecret(secretEnvName);
const viaVault = !!expectedSecret;
if (!expectedSecret) {
expectedSecret = Deno.env.get(secretEnvName) ?? "";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Allow CRON_SECRET through the vault helper

When these cron functions pass secretEnvName: "CRON_SECRET", this vault lookup always fails because the existing public.get_edge_function_secret helper only permits WEBHOOK_DISPATCHER_SECRET and CONNECTIONS_AUTO_TEST_SECRET (and the vault setup migration only creates those two). In a deployment that follows this change’s new vault-based source of truth and removes the edge env fallback, expectedSecret stays empty and the code falls through to legacy_no_auth, so the newly verify_jwt=false cron endpoints accept unauthenticated requests instead of requiring the cron secret. The migration/setup needs to add CRON_SECRET to the vault and helper whitelist before relying on this path.

Useful? React with 👍 / 👎.

Comment thread supabase/config.toml
Comment on lines +55 to +56
[functions.process-queue]
verify_jwt = false
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Pass x-cron-secret from existing pg_cron jobs

Disabling JWT verification for process-queue makes the function depend on the in-function x-cron-secret guard, but the existing pg_cron job in supabase/cron/cron-config.sql still posts only Authorization and Content-Type headers to /functions/v1/process-queue (checked lines 18-22). Once CRON_SECRET is actually available through vault/env, that scheduled queue processor will receive no x-cron-secret and return 401 every minute; the schedule needs to include the same vault-backed secret header before relying on this auth mode.

Useful? React with 👍 / 👎.

Comment thread supabase/config.toml
Comment on lines +64 to +65
[functions.send-digest]
verify_jwt = false
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add the cron secret to the remaining scheduled calls

The same auth switch also affects send-digest and cleanup-notifications, but their existing schedules in supabase/cron/cron-config.sql still send only the service-role bearer plus Content-Type headers (checked lines 37-40 and 56-59). After CRON_SECRET is configured, both jobs will start getting 401s from the new x-cron-secret guard and stop running, so those schedules need to pass the vault-backed cron header before JWT verification is disabled for these functions.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refatora a autenticação de cron Edge Functions para buscar o secret esperado via RPC do vault (com fallback para env) e corrige um bug no audit_ownership_orphans que causava erro ao tentar cast de colunas TEXT para uuid.

Changes:

  • authorizeCron passa a ser async e tenta ler o secret no vault via RPC (cache por instância) antes de cair no Deno.env.get.
  • 13 cron functions + connections-auto-test atualizadas para await authorizeCron(...) (e send-scheduled-reports recebe bloco de auth que não existia).
  • Migration ajusta audit_ownership_orphans para só considerar colunas com data_type = 'uuid', evitando casts inválidos.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
supabase/functions/_shared/dispatcher-auth.ts Implementa leitura via vault (RPC) + cache e torna authorizeCron async.
supabase/config.toml Define verify_jwt = false para as cron functions para evitar 401 no deploy.
supabase/functions/cleanup-notifications/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/cleanup-novelties/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/collections-watcher/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/comparison-price-watcher/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/connections-health-check/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/favorites-watcher/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/ownership-audit/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/process-queue/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/process-scheduled-reports/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/quote-followup-reminders/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/send-digest/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/send-notification/index.ts Atualiza chamada para await authorizeCron.
supabase/functions/send-scheduled-reports/index.ts Adiciona autenticação cron via authorizeCron.
supabase/functions/connections-auto-test/index.ts Atualiza chamada para await authorizeCron.
supabase/migrations/20260515120000_fix_audit_ownership_orphans_uuid_only.sql Corrige audit_ownership_orphans para ignorar colunas não-UUID e evitar 500.
Comments suppressed due to low confidence (2)

supabase/functions/_shared/dispatcher-auth.ts:275

  • authorizeCron agora é async e retorna Promise<CronAuthResult>, mas há chamadores no repo ainda tratando como síncrono (ex.: _shared/createEdge.ts e _shared/dispatcher-auth.test.ts). Isso deve quebrar o build/runtime (acesso a .ok em uma Promise). Atualize esses call sites para await authorizeCron(...) e ajuste os testes para async/await.
export async function authorizeCron(
  req: Request,
  opts: { corsHeaders: Record<string, string>; secretEnvName: string; headerName: string },
): Promise<CronAuthResult> {
  const { corsHeaders, secretEnvName, headerName } = opts;

supabase/functions/_shared/dispatcher-auth.ts:100

  • Do jeito que getVaultSecret trata erros (retorna "" e ainda faz cache do Promise), um erro/transiente no RPC pode deixar a instância inteira “presa” sem secret (sem retries) e, se o env também não estiver setado, authorizeCron entra em legacy_no_auth e aceita chamadas anônimas. Para hardening, diferencie “secret inexistente” de “vault indisponível” e evite cachear falhas (ex.: remover do cache quando resolver vazio/erro) ou falhar fechado (503/401) quando vault estiver indisponível e não houver fallback seguro.
async function getVaultSecret(name: string): Promise<string> {
  if (_vaultCache.has(name)) return _vaultCache.get(name)!;
  const promise = (async () => {
    if (!SUPABASE_URL || !SERVICE_KEY) return "";
    try {
      const client = createClient(SUPABASE_URL, SERVICE_KEY, {
        auth: { persistSession: false, autoRefreshToken: false },
      });
      const { data, error } = await client.rpc("get_edge_function_secret", { _name: name });
      if (error || !data) return "";
      return data as string;
    } catch {
      return "";
    }
  })();
  _vaultCache.set(name, promise);
  return promise;
}

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

Comment on lines +83 to +93
async function getVaultSecret(name: string): Promise<string> {
if (_vaultCache.has(name)) return _vaultCache.get(name)!;
const promise = (async () => {
if (!SUPABASE_URL || !SERVICE_KEY) return "";
try {
const client = createClient(SUPABASE_URL, SERVICE_KEY, {
auth: { persistSession: false, autoRefreshToken: false },
});
const { data, error } = await client.rpc("get_edge_function_secret", { _name: name });
if (error || !data) return "";
return data as string;
-- com valores como "system" em enriched_contacts.created_by. Agora só
-- considera colunas com data_type='uuid'. Mais robusto que manter blacklist.
--
-- Data: 15/mai/2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 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/_shared/dispatcher-auth.ts`:
- Around line 83-100: The getVaultSecret function currently caches an empty
string on RPC failure which makes that failure sticky; change the caching logic
around _vaultCache so an empty result or an error is not stored permanently:
call createClient and invoke the get_edge_function_secret RPC as before, but
after the promise resolves remove the cache entry if the returned value is ""
(or throw/catch produced no value), otherwise keep/store the non-empty secret;
alternatively only insert into _vaultCache when the resolved value is non-empty
and ensure any caught errors also avoid caching by deleting _vaultCache entry
for the given name. Ensure you update references to getVaultSecret, _vaultCache,
createClient, and get_edge_function_secret accordingly.

In `@supabase/functions/connections-auto-test/index.ts`:
- Around line 130-133: The cron handler uses a specific env name
"CONNECTIONS_AUTO_TEST_SECRET" instead of the single source-of-truth cron
secret, so update the authorizeCron call to use the canonical secret name (e.g.,
the shared CRON_SECRET constant or env key used across crons) by replacing
secretEnvName: "CONNECTIONS_AUTO_TEST_SECRET" with the central secret
identifier; adjust any references near the authorizeCron invocation (function
authorizeCron, parameter secretEnvName) to point to the shared CRON secret so
the endpoint uses the same vault/env key as other crons and avoids falling back
to legacy_no_auth.

In
`@supabase/migrations/20260515120000_fix_audit_ownership_orphans_uuid_only.sql`:
- Around line 38-50: The loop SELECT returns (table_name, column_name) so
v_tables_scanned is being incremented per column, not per table; modify the
logic to count distinct tables instead—either change the query to return
DISTINCT table_name (e.g., SELECT DISTINCT c.table_name ...) and iterate over
that, or keep the current query and add a tracker variable (e.g., v_last_table)
and only increment v_tables_scanned when v_table.table_name differs from
v_last_table; ensure total_tables_scanned is set from this distinct-table
counter and apply the same fix where total_tables_scanned is assigned later (the
block around v_tables_scanned/total_tables_scanned usage).
- Around line 34-36: The current check in the audit_ownership_orphans function
lets calls with auth.uid() IS NULL bypass authorization; change the condition to
deny when auth.uid() IS NULL or when the uid lacks the required roles (e.g., use
IF auth.uid() IS NULL OR NOT (has_role(auth.uid(), 'admin'::app_role) OR
has_role(auth.uid(), 'dev'::app_role)) THEN RAISE EXCEPTION ...), and if this
function is not public revoke EXECUTE in the same migration; also ensure the
function uses SECURITY DEFINER only if strictly necessary and sets an explicit
search_path per guidelines.
🪄 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: 9f378795-2585-4b45-910b-a92e13baf5b0

📥 Commits

Reviewing files that changed from the base of the PR and between 840f2ef and 0599e28.

📒 Files selected for processing (17)
  • supabase/config.toml
  • supabase/functions/_shared/dispatcher-auth.ts
  • supabase/functions/cleanup-notifications/index.ts
  • supabase/functions/cleanup-novelties/index.ts
  • supabase/functions/collections-watcher/index.ts
  • supabase/functions/comparison-price-watcher/index.ts
  • supabase/functions/connections-auto-test/index.ts
  • supabase/functions/connections-health-check/index.ts
  • supabase/functions/favorites-watcher/index.ts
  • supabase/functions/ownership-audit/index.ts
  • supabase/functions/process-queue/index.ts
  • supabase/functions/process-scheduled-reports/index.ts
  • supabase/functions/quote-followup-reminders/index.ts
  • supabase/functions/send-digest/index.ts
  • supabase/functions/send-notification/index.ts
  • supabase/functions/send-scheduled-reports/index.ts
  • supabase/migrations/20260515120000_fix_audit_ownership_orphans_uuid_only.sql

Comment on lines +83 to +100
async function getVaultSecret(name: string): Promise<string> {
if (_vaultCache.has(name)) return _vaultCache.get(name)!;
const promise = (async () => {
if (!SUPABASE_URL || !SERVICE_KEY) return "";
try {
const client = createClient(SUPABASE_URL, SERVICE_KEY, {
auth: { persistSession: false, autoRefreshToken: false },
});
const { data, error } = await client.rpc("get_edge_function_secret", { _name: name });
if (error || !data) return "";
return data as string;
} catch {
return "";
}
})();
_vaultCache.set(name, promise);
return promise;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Não cacheie resultado vazio do Vault como definitivo.

Se a primeira RPC falhar/retornar vazio, esse "" fica preso no _vaultCache durante todo o cold-start. Em funções com verify_jwt = false, isso pode prolongar fallback/legacy_no_auth além do necessário.

💡 Ajuste sugerido
 async function getVaultSecret(name: string): Promise<string> {
   if (_vaultCache.has(name)) return _vaultCache.get(name)!;
   const promise = (async () => {
@@
     } catch {
       return "";
     }
   })();
   _vaultCache.set(name, promise);
+  promise.then((value) => {
+    if (!value) _vaultCache.delete(name);
+  }).catch(() => {
+    _vaultCache.delete(name);
+  });
   return promise;
 }
🤖 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/dispatcher-auth.ts` around lines 83 - 100, The
getVaultSecret function currently caches an empty string on RPC failure which
makes that failure sticky; change the caching logic around _vaultCache so an
empty result or an error is not stored permanently: call createClient and invoke
the get_edge_function_secret RPC as before, but after the promise resolves
remove the cache entry if the returned value is "" (or throw/catch produced no
value), otherwise keep/store the non-empty secret; alternatively only insert
into _vaultCache when the resolved value is non-empty and ensure any caught
errors also avoid caching by deleting _vaultCache entry for the given name.
Ensure you update references to getVaultSecret, _vaultCache, createClient, and
get_edge_function_secret accordingly.

Comment on lines +130 to 133
const auth = await authorizeCron(req, {
corsHeaders,
secretEnvName: "CONNECTIONS_AUTO_TEST_SECRET",
headerName: "x-cron-secret",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

connections-auto-test ficou fora do SoT de CRON_SECRET.

Aqui a autenticação usa CONNECTIONS_AUTO_TEST_SECRET, divergindo da estratégia declarada de secret único para crons. Se esse nome não estiver provisionado em vault/env, o fluxo pode cair em legacy_no_auth enquanto verify_jwt está desativado.

💡 Ajuste sugerido
   const auth = await authorizeCron(req, {
     corsHeaders,
-    secretEnvName: "CONNECTIONS_AUTO_TEST_SECRET",
+    secretEnvName: "CRON_SECRET",
     headerName: "x-cron-secret",
   });
📝 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.

Suggested change
const auth = await authorizeCron(req, {
corsHeaders,
secretEnvName: "CONNECTIONS_AUTO_TEST_SECRET",
headerName: "x-cron-secret",
const auth = await authorizeCron(req, {
corsHeaders,
secretEnvName: "CRON_SECRET",
headerName: "x-cron-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 `@supabase/functions/connections-auto-test/index.ts` around lines 130 - 133,
The cron handler uses a specific env name "CONNECTIONS_AUTO_TEST_SECRET" instead
of the single source-of-truth cron secret, so update the authorizeCron call to
use the canonical secret name (e.g., the shared CRON_SECRET constant or env key
used across crons) by replacing secretEnvName: "CONNECTIONS_AUTO_TEST_SECRET"
with the central secret identifier; adjust any references near the authorizeCron
invocation (function authorizeCron, parameter secretEnvName) to point to the
shared CRON secret so the endpoint uses the same vault/env key as other crons
and avoids falling back to legacy_no_auth.

Comment on lines +34 to +36
IF auth.uid() IS NOT NULL AND NOT (has_role(auth.uid(), 'admin'::app_role) OR has_role(auth.uid(), 'dev'::app_role)) THEN
RAISE EXCEPTION 'audit_ownership_orphans: acesso negado';
END IF;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Feche o bypass de autorização para chamadas sem auth.uid().

Do jeito que está, qualquer execução com auth.uid() IS NULL passa pela checagem e roda com privilégios do dono da função. Em contexto Supabase isso pode ser necessário para service_role, mas aqui também deixa passar qualquer caller com EXECUTE se o grant não estiver bem fechado.

🐛 Ajuste sugerido
-  IF auth.uid() IS NOT NULL AND NOT (has_role(auth.uid(), 'admin'::app_role) OR has_role(auth.uid(), 'dev'::app_role)) THEN
+  IF NOT (
+    auth.role() = 'service_role'
+    OR (
+      auth.uid() IS NOT NULL
+      AND (
+        has_role(auth.uid(), 'admin'::app_role)
+        OR has_role(auth.uid(), 'dev'::app_role)
+      )
+    )
+  ) THEN
     RAISE EXCEPTION 'audit_ownership_orphans: acesso negado';
   END IF;

Se a função não for para uso público, vale revogar EXECUTE no mesmo migration. As per coding guidelines "Funções com SECURITY DEFINER apenas quando absolutamente necessário e com search_path explícito".

📝 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.

Suggested change
IF auth.uid() IS NOT NULL AND NOT (has_role(auth.uid(), 'admin'::app_role) OR has_role(auth.uid(), 'dev'::app_role)) THEN
RAISE EXCEPTION 'audit_ownership_orphans: acesso negado';
END IF;
IF NOT (
auth.role() = 'service_role'
OR (
auth.uid() IS NOT NULL
AND (
has_role(auth.uid(), 'admin'::app_role)
OR has_role(auth.uid(), 'dev'::app_role)
)
)
) THEN
RAISE EXCEPTION 'audit_ownership_orphans: acesso negado';
END IF;
🤖 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/20260515120000_fix_audit_ownership_orphans_uuid_only.sql`
around lines 34 - 36, The current check in the audit_ownership_orphans function
lets calls with auth.uid() IS NULL bypass authorization; change the condition to
deny when auth.uid() IS NULL or when the uid lacks the required roles (e.g., use
IF auth.uid() IS NULL OR NOT (has_role(auth.uid(), 'admin'::app_role) OR
has_role(auth.uid(), 'dev'::app_role)) THEN RAISE EXCEPTION ...), and if this
function is not public revoke EXECUTE in the same migration; also ensure the
function uses SECURITY DEFINER only if strictly necessary and sets an explicit
search_path per guidelines.

Comment on lines +38 to +50
FOR v_table IN
SELECT c.table_name, c.column_name
FROM information_schema.columns c
JOIN information_schema.tables t ON t.table_schema = c.table_schema AND t.table_name = c.table_name
WHERE c.table_schema = 'public'
AND c.column_name = ANY(v_owner_columns)
AND c.data_type = 'uuid' -- FIX: ignora colunas TEXT (ex: enriched_contacts.created_by='system')
AND t.table_type = 'BASE TABLE'
AND c.table_name NOT IN ('login_attempts','step_up_audit_log','search_analytics','query_telemetry','mcp_access_violations','product_views','quote_history','optimization_queue','kit_templates')
ORDER BY c.table_name
LOOP
v_col := v_table.column_name;
v_tables_scanned := v_tables_scanned + 1;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

total_tables_scanned está contando colunas, não tabelas.

A consulta do loop retorna (table_name, column_name). Em Line 50, o contador sobe por iteração; em Line 68, esse valor é salvo como total_tables_scanned. Se uma tabela tiver dois campos UUID de ownership, o relatório grava 2 mesmo tendo escaneado uma única tabela.

🐛 Ajuste sugerido
 DECLARE
+  v_seen_tables text[] := ARRAY[]::text[];
 BEGIN
   FOR v_table IN
     SELECT c.table_name, c.column_name
@@
-    v_tables_scanned := v_tables_scanned + 1;
+    IF NOT (v_table.table_name = ANY(v_seen_tables)) THEN
+      v_seen_tables := array_append(v_seen_tables, v_table.table_name);
+      v_tables_scanned := v_tables_scanned + 1;
+    END IF;

Also applies to: 67-71

🤖 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/20260515120000_fix_audit_ownership_orphans_uuid_only.sql`
around lines 38 - 50, The loop SELECT returns (table_name, column_name) so
v_tables_scanned is being incremented per column, not per table; modify the
logic to count distinct tables instead—either change the query to return
DISTINCT table_name (e.g., SELECT DISTINCT c.table_name ...) and iterate over
that, or keep the current query and add a tracker variable (e.g., v_last_table)
and only increment v_tables_scanned when v_table.table_name differs from
v_last_table; ensure total_tables_scanned is set from this distinct-table
counter and apply the same fix where total_tables_scanned is assigned later (the
block around v_tables_scanned/total_tables_scanned usage).

@adm01-debug adm01-debug merged commit 5acf8cd into main May 15, 2026
23 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants