feat(security): promove secure-upload de public para authenticated (re-target #68 → main)#72
Conversation
…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
|
Caution Review failedThe pull request is closed. Warning
|
| Cohort / File(s) | Summary |
|---|---|
Authorization Manifest Update supabase/functions/_shared/edge-authz-manifest.ts |
Changes secure-upload category from "public" to "authenticated" and updates rationale to specify JWT requirement with internal SERVICE_ROLE_KEY usage and mandatory user_id auditing. |
Secure Upload Function supabase/functions/secure-upload/index.ts |
Refactors authentication to use shared authenticateRequest() utility with early rejection of unauthenticated requests; obtains Supabase admin client from authenticated context instead of environment variables; updates ScanLog.user_id from nullable to non-nullable string; ensures all audit logs consistently include user_id for correlation with auth.users. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
#67: Predecessor PR that adds the initialsecure-uploadmanifest entry and structured logging foundation; this PR depends on and builds directly upon those changes to promote authentication from public to authenticated.
Poem
🐰 A hop toward safety, the manifest's changed,
From open to guarded, the access rearranged!
JWT tokens now required to pass through,
Every scan logged withuser_idso true. ✨
✨ 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
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.
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Pull request overview
Promove a edge function secure-upload para exigir autenticação (JWT) no SSOT de autorização, reduzindo o vetor de abuso de Storage causado por chamadas anônimas em uma função que opera com SERVICE_ROLE_KEY.
Changes:
- Move
secure-uploaddecategory: "public"para"authenticated"noedge-authz-manifest.ts. - Adiciona autenticação obrigatória via
authenticateRequest(req)no início da edgesecure-upload. - Reaproveita
auth.localServiceClient(service-role client) e incluiuser_idnos logs/eventos e auditoria.
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 JWT via authenticateRequest e registra user_id consistentemente, reutilizando o service client do helper de auth. |
| supabase/functions/_shared/edge-authz-manifest.ts | Reclassifica secure-upload como authenticated no manifest SSOT. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const corsHeaders = { | ||
| "Access-Control-Allow-Origin": "*", | ||
| "Access-Control-Allow-Headers": "authorization, x-client-info, apikey, content-type", | ||
| }; |
🎯 Re-targeted from #68 to main
PR #68 estava stacked sobre
claude/fix-secure-upload-authz-logging(que veio de PR #67, já mergeado em main em 29/abr0a5784e7). Esta PR é o mesmo conteúdo com base =mainpara permitir merge direto.Contexto
Promove a edge function
secure-uploadda categoriapublicparaauthenticatedno manifest SSOT. Essa edge usaSUPABASE_SERVICE_ROLE_KEY(bypassa RLS) e aceitava chamadas anônimas — vetor de abuso de Storage.Mudanças
supabase/functions/_shared/edge-authz-manifest.ts:category: "public"→"authenticated"supabase/functions/secure-upload/index.ts: adicionaauthenticateRequest(req)no início + reusaauth.localServiceClientDiff
23+ / 22- em 2 arquivos.
Compat
SecureUploadManager.tsxImageUploadButton.tsxValidação
Realizada em #68: typecheck, check-edge-authorization (85/85), check-edge-structured-logging, check-edge-request-id-propagation — tudo OK.
Closes #68 (re-targeted).
Summary by CodeRabbit