Skip to content

feat(security): promove secure-upload de public para authenticated#68

Closed
adm01-debug wants to merge 1 commit into
claude/fix-secure-upload-authz-loggingfrom
claude/harden-secure-upload-authenticated
Closed

feat(security): promove secure-upload de public para authenticated#68
adm01-debug wants to merge 1 commit into
claude/fix-secure-upload-authz-loggingfrom
claude/harden-secure-upload-authenticated

Conversation

@adm01-debug
Copy link
Copy Markdown
Owner

Contexto

No PR #67 a edge secure-upload foi registrada no manifest como public para não quebrar comportamento existente, mas com uma nota explícita sugerindo promoção futura para authenticated. Este PR é esse follow-up.

Problema

A edge secure-upload:

  1. Usa SUPABASE_SERVICE_ROLE_KEY (bypassa RLS, escreve em qualquer bucket de Storage)
  2. Aceitava chamadas anônimas (Authorization header opcional)
  3. Os únicos callers reais são áreas autenticadas (SecureUploadManager em admin/security, ImageUploadButton em forms autenticados)

Isso constitui um vetor: qualquer um na internet pode chamar a edge sem JWT e fazer upload pra personalization-images ou quarantine. O VirusTotal mitiga malware mas não impede storage abuse / cost amplification / fingerprinting.

Solução

edge-authz-manifest.ts

-  "secure-upload": { category: "public", rationale: "... considerar promover a authenticated em PR futuro" },
+  "secure-upload": { category: "authenticated", rationale: "Upload com scan VirusTotal anti-malware — exige JWT (uso de SERVICE_ROLE_KEY interno; auditoria em file_scan_logs com user_id obrigatório)" },

secure-upload/index.ts

Reusa o helper SSOT authenticateRequest (mesmo usado por outras 24 edges authenticated):

// 1. Autenticação obrigatória — rejeita anônimos antes de qualquer trabalho.
let auth;
try {
  auth = await authenticateRequest(req);
} catch (err) {
  log.warn("auth_failed", { err });
  return log.respond(authErrorResponse(err, corsHeaders));
}

const supabaseAdmin = auth.localServiceClient;
log.info("request_start", { user_id: auth.userId });

Bonus de limpeza

  • Remove a criação manual de createClient(SERVICE_ROLE_KEY) — reusa auth.localServiceClient
  • Tipo ScanLog.user_id: string | nullstring (sempre presente; antes podia ser null para anônimos)
  • Todos os eventos do logger agora carregam user_id (correlation com auth.users)
  • Novo evento auth_failed para auditoria de tentativas não autenticadas

Compatibilidade

Caller Antes Depois Ação necessária
SecureUploadManager.tsx (admin/security) manda JWT auto via supabase-js idêntico nenhuma
ImageUploadButton.tsx (forms autenticados) manda JWT auto via supabase-js idêntico nenhuma
Chamadas anônimas hipotéticas 200 com user_id: null no log 401 {error: "Token de autenticação ausente"} quebra esperada

Nenhum caller real é afetado — todos já enviam Authorization: Bearer <session.access_token> automaticamente quando o usuário está logado.

Validação

✅ npm run typecheck                                  — 0 erros
✅ check-edge-authorization     — 85/85 (authenticated 24→25, public 22→21)
✅ check-edge-structured-logging — OK
✅ check-edge-request-id-propagation — OK

Por que stacked sobre o #67

Este PR depende das mudanças do #67 (entrada no manifest + structured logger). Quando o #67 mergear no main, eu faço rebase deste PR e ele vira um diff limpo de ±23 linhas.

DEPENDS-ON

Sugestões para PRs futuros (não cobertos aqui)

  • Estender tests/security/edge-authz-bypass.test.ts para também varrer categoria authenticated (hoje só cobre supervisor/dev). Isso garantiria que regressões como esta sejam capturadas automaticamente.
  • Adicionar runBotProtection (presente em outras edges públicas como image-proxy e analyze-logo-colors) com rate-limit por usuário em secure-upload, para limitar abuso por usuário comprometido.

🤖 Gerado pelo Claude (Opus 4.7) operando via container claude-code na VPS Atomica + Portainer MCP.

…ardening)

Fecha vetor onde a edge usa SERVICE_ROLE_KEY mas aceitava chamadas anônimas.

MUDANÇAS

1. edge-authz-manifest.ts
   - secure-upload: public → authenticated
   - rationale atualizado: "exige JWT (uso de SERVICE_ROLE_KEY interno;
     auditoria em file_scan_logs com user_id obrigatório)"

2. secure-upload/index.ts
   - Adiciona authenticateRequest(req) no início do handler
   - Retorna 401 imediato (via authErrorResponse) se anon ou JWT inválido
   - Reusa auth.localServiceClient (já é SERVICE_ROLE) em vez de criar
     supabaseAdmin manualmente
   - Tipo ScanLog.user_id: string | null → string (sempre presente)
   - Eventos do logger agora incluem user_id em todos:
     request_start / security_check_failed / upload_blocked_malware /
     upload_ok / upload_failed
   - Novo evento auth_failed para auditoria de tentativas anônimas

COMPATIBILIDADE

- Callers no frontend (SecureUploadManager, ImageUploadButton) já
  enviam JWT automaticamente via supabase.functions.invoke() —
  sem mudança necessária.
- Body de resposta inalterado para callers autenticados.
- Anônimos antes obtinham 200 com user_id=null no log; agora obtem
  401 com {error: "Token de autenticação ausente"}.

VALIDAÇÃO

- npm run typecheck: 0 erros
- check-edge-authorization: 85/85 (authenticated 24→25, public 22→21)
- check-edge-structured-logging: OK (84 legadas, 1 migrada)
- check-edge-request-id-propagation: OK (9 críticas validadas)

DEPENDS-ON: #67
Copilot AI review requested due to automatic review settings April 29, 2026 12:43
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: edd972db-db40-4c58-af05-8ebf1956874b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/harden-secure-upload-authenticated

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

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

Promove a edge function secure-upload de public para authenticated, tornando obrigatória a autenticação via JWT antes de executar upload/scan (reduzindo superfície de abuso de Storage via chamada anônima), e alinhando a edge ao uso real pelos callers autenticados do frontend.

Changes:

  • Move secure-upload no edge-authz-manifest.ts de public para authenticated, com rationale atualizado.
  • Atualiza secure-upload/index.ts para exigir authenticateRequest(req) e reutilizar auth.localServiceClient (service-role) em vez de criar client manual.
  • Endurece auditoria tipando ScanLog.user_id como obrigatório e adiciona user_id aos eventos relevantes do logger.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
supabase/functions/secure-upload/index.ts Passa a exigir autenticação, remove lookup opcional de user e reforça logs/auditoria com user_id.
supabase/functions/_shared/edge-authz-manifest.ts Reclassifica secure-upload como authenticated no SSOT de autorização.

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

Comment on lines +36 to +37
const supabaseAdmin = auth.localServiceClient;
log.info("request_start", { user_id: auth.userId });
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