Skip to content

fix(edge): registra secure-upload no manifest e migra para structured logger SSOT#67

Merged
adm01-debug merged 1 commit into
mainfrom
claude/fix-secure-upload-authz-logging
Apr 29, 2026
Merged

fix(edge): registra secure-upload no manifest e migra para structured logger SSOT#67
adm01-debug merged 1 commit into
mainfrom
claude/fix-secure-upload-authz-logging

Conversation

@adm01-debug
Copy link
Copy Markdown
Owner

@adm01-debug adm01-debug commented Apr 29, 2026

Contexto

Os checks SSOT do projeto (scripts/check-edge-*.mjs) sinalizaram 2 violações na edge function secure-upload, recém-adicionada ao repo. Este PR corrige ambas, sem alterar comportamento da função em produção.

Violações detectadas

❌ check-edge-authorization
   [manifest] Edge function "secure-upload" não está declarada em
   edge-authz-manifest.ts.

❌ check-edge-structured-logging
   • secure-upload não importa createStructuredLogger.

Mudanças

1. supabase/functions/_shared/edge-authz-manifest.ts

Adicionada entrada secure-upload na seção public com rationale explícito:

"secure-upload": {
  category: "public",
  rationale: "Upload com scan VirusTotal anti-malware (aceita anônimo;
              uso real em áreas autenticadas  considerar promover a
              authenticated em PR futuro)"
},

⚠️ Nota de segurança para discussão futura: a função usa SUPABASE_SERVICE_ROLE_KEY e aceita requests anônimas no código, embora os únicos callers no frontend (SecureUploadManager, ImageUploadButton) sejam áreas autenticadas. Vale promover para authenticated num PR separado adicionando authenticateRequest(req) — não fiz aqui para manter este PR puramente cosmético/SSOT.

2. supabase/functions/secure-upload/index.ts

Migração para o logger estruturado SSOT (createStructuredLogger) seguindo o padrão de docs/OBSERVABILITY.md §2:

Antes Depois
serve() de deno.land/std@0.168.0 Deno.serve nativo
console.error('Security Check Failed:', reason) log.error("security_check_failed", { err, reason })
console.error('Final Error:', error.message) log.error("upload_failed", { err: error })
scan_result: any / catch (err: any) Record<string, unknown> / catch (err) com narrow
Resposta sem correlation-id request_id propagado em header (X-Request-Id) e body

Eventos emitidos:

  • request_start — início do request
  • security_check_failed — falha/timeout do VirusTotal
  • upload_blocked_malware — arquivo classificado como malicioso (vai pra quarantine)
  • upload_ok — upload bem-sucedido em personalization-images
  • upload_failed — exceção genérica não tratada

Validação local

✅ npm run typecheck                                  — 0 erros
✅ node scripts/check-edge-authorization.mjs          — 85/85 declaradas
✅ node scripts/check-edge-structured-logging.mjs     — 1 migrada (secure-upload)
✅ node scripts/check-edge-request-id-propagation.mjs — 9 críticas validadas

Riscos / impacto

  • Comportamento da função: preservado integralmente. Mesmo fluxo, mesmas respostas, mesmos status_code, mesma persistência em file_scan_logs.
  • Compat com callers: sem mudança de contrato. O body de resposta apenas ganha o campo request_id (campo novo, opcional).
  • CI: este PR faz os 2 gates mencionados passarem; o lint:baseline segue inalterado (arquivos em supabase/ não entram no eslint src).

Refs

  • docs/OBSERVABILITY.md §2 — padrão do logger estruturado
  • supabase/functions/_shared/structured-logger.ts — implementação SSOT
  • supabase/functions/_shared/request-id.ts — helper de correlation-id

🤖 Gerado pelo Claude (Opus 4.7) operando via container claude-code na VPS Atomica + Portainer MCP. Diagnóstico, fix, validação e push feitos sem intervenção manual no código.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved error handling for file uploads with clearer error reporting.
    • Enhanced malware scan result processing for more consistent detection.
  • Infrastructure

    • Implemented structured request tracking to improve logging and reliability of upload operations.

… logger

Corrige 2 violações detectadas pelos checks SSOT:

1. check-edge-authorization: secure-upload não estava declarada em
   edge-authz-manifest.ts. Adicionada na categoria "public" com
   rationale explicando o comportamento atual (aceita anônimo) e nota
   sugerindo promoção futura para "authenticated" (uso real é apenas
   em áreas autenticadas via SecureUploadManager / ImageUploadButton).

2. check-edge-structured-logging: secure-upload não usava o logger SSOT.
   Migrada para createStructuredLogger + getOrCreateRequestId, com:
   - eventos: request_start, security_check_failed, upload_blocked_malware,
     upload_ok, upload_failed
   - request_id propagado no header e no body de resposta
   - serve() legacy substituído por Deno.serve nativo
   - tipos any substituídos por Record<string, unknown>

Resultado dos gates:
- check-edge-authorization: 85/85 declaradas ✓
- check-edge-structured-logging: 85 edges (84 legadas, 1 migrada) ✓
- check-edge-request-id-propagation: 9 críticas validadas ✓
- typecheck: 0 erros ✓

Refs: docs/OBSERVABILITY.md §2
Copilot AI review requested due to automatic review settings April 29, 2026 12:37
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

📝 Walkthrough

Walkthrough

The changes add authorization manifest configuration for the secure-upload Edge Function and refactor its implementation to use structured logging with request correlation ID, replacing console logging and routing all responses through a centralized logging handler.

Changes

Cohort / File(s) Summary
Authorization Manifest
supabase/functions/_shared/edge-authz-manifest.ts
Adds secure-upload function entry categorized as "public" with VirusTotal anti-malware scanning rationale.
Structured Logging Implementation
supabase/functions/secure-upload/index.ts
Refactors handler to use Deno.serve, structured logging via createStructuredLogger, and request correlation with getOrCreateRequestId. Routes all responses (OPTIONS, errors, successful uploads) through log.respond(...). Narrows ScanLog.scan_result type and refines VirusTotal scan result evaluation logic to extract numeric malicious/suspicious counts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 With structured logs now tracing every request,
Each carrot upload gets security blessed,
Correlation IDs hop through the night,
VirusTotal scans keep your files tight,
Safe journeys logged for transparency's delight! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: registering secure-upload in the manifest and migrating to structured logger SSOT.
Description check ✅ Passed The description is comprehensive, covering context, violations fixed, detailed changes, validation steps, and risk assessment—exceeding typical template requirements.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 claude/fix-secure-upload-authz-logging

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
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

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

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: 5

🧹 Nitpick comments (2)
supabase/functions/secure-upload/index.ts (2)

82-91: Clear the timeout in a finally block.

If fetch throws before clearTimeout, the timer stays pending unnecessarily.

Proposed patch
-        const controller = new AbortController();
-        const timeoutId = setTimeout(() => controller.abort(), 10000);
-        const vtRes = await fetch(
-          `https://www.virustotal.com/api/v3/files/${hashHex}`,
-          {
-            headers: { "x-apikey": vtApiKey },
-            signal: controller.signal,
-          },
-        );
-        clearTimeout(timeoutId);
+        const controller = new AbortController();
+        const timeoutId = setTimeout(() => controller.abort(), 10000);
+        let vtRes: Response;
+        try {
+          vtRes = await fetch(
+            `https://www.virustotal.com/api/v3/files/${hashHex}`,
+            {
+              headers: { "x-apikey": vtApiKey },
+              signal: controller.signal,
+            },
+          );
+        } finally {
+          clearTimeout(timeoutId);
+        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/secure-upload/index.ts` around lines 82 - 91, The timeout
created for the AbortController (timeoutId) may not be cleared if fetch throws;
wrap the fetch call and its clearTimeout(timeoutId) in a try/finally so
clearTimeout(timeoutId) always runs (and keep using controller.signal and
controller.abort()); update the block around controller, timeoutId, fetch
(vtRes) to call clearTimeout in a finally to ensure the timer is always cleared
even on exceptions.

151-151: Avoid as ScanLog assertion here; build a concrete ScanLog object.

The cast bypasses compiler guarantees and can hide missing required fields during future edits.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/secure-upload/index.ts` at line 151, Replace the unsafe
type assertion when inserting into the DB by constructing a concrete ScanLog
object from auditData instead of using "as ScanLog": validate and map fields
from the existing auditData into a new const scanLog: ScanLog (ensuring all
required properties of the ScanLog type are present and correctly typed) and
then call supabaseAdmin.from("file_scan_logs").insert(scanLog); this ensures
missing or incorrectly typed fields are caught by the compiler and at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@supabase/functions/_shared/edge-authz-manifest.ts`:
- Line 72: The manifest entry for "secure-upload" currently sets category:
"public" which allows anonymous use of endpoints that perform privileged writes
with SUPABASE_SERVICE_ROLE_KEY; update the "secure-upload" entry in
edge-authz-manifest.ts to use a hardened category (e.g., change category from
"public" to "authenticated" or to "scoped" and add signed token validation) so
the endpoint requires authenticated/scoped access and ensure any downstream
logic that relies on this manifest enforces the new auth check.

In `@supabase/functions/secure-upload/index.ts`:
- Around line 41-44: The handler currently assumes multipart fields are the
correct types; before calling file.arrayBuffer() or using folder, validate
runtime types and return a 400 for malformed payloads. Specifically, after const
formData = await req.formData() and where you assign const file =
formData.get("file") and const folder = (formData.get("folder") as string) ||
"uploads", check that file is actually a File-like object (e.g., instanceof File
or has a callable arrayBuffer method) and that folder is a string; if not,
respond with status 400 and an error message. Apply the same runtime checks at
the later usage site(s) around the code referenced on lines 54-57 where
file/folder are used, and bail out early with 400 rather than letting
file.arrayBuffer() throw a 500.
- Around line 191-193: The response currently includes errorObj.message which
may leak internal details; change the response body to a stable generic message
(e.g., "Internal server error") and include only the requestId for correlation,
while still logging the full error (errorObj and requestId) internally; update
the JSON.stringify call that builds the 500 response (the block using requestId,
errorObj, corsHeaders) to return { error: "Internal server error", request_id:
requestId } and ensure any logging statements record errorObj.message/stack with
requestId for debugging.
- Line 7: The CORS response header list is missing "x-request-id", preventing
browsers from sending X-Request-Id cross-origin; update the
Access-Control-Allow-Headers value (the header key
"Access-Control-Allow-Headers" where the string currently contains
"authorization, x-client-info, apikey, content-type") to include "x-request-id"
so browsers can forward the request ID for end-to-end correlation.
- Around line 56-57: Add a hard byte-limit check before buffering the uploaded
file: define a MAX_UPLOAD_BYTES constant and, inside the request handler (before
calling file.arrayBuffer() / before computing hash via crypto.subtle.digest),
inspect the file.size (or Content-Length) and immediately return a
413/appropriate error if it exceeds the limit; only call file.arrayBuffer() and
continue with hashBuffer = await crypto.subtle.digest(...) after the size check
passes.

---

Nitpick comments:
In `@supabase/functions/secure-upload/index.ts`:
- Around line 82-91: The timeout created for the AbortController (timeoutId) may
not be cleared if fetch throws; wrap the fetch call and its
clearTimeout(timeoutId) in a try/finally so clearTimeout(timeoutId) always runs
(and keep using controller.signal and controller.abort()); update the block
around controller, timeoutId, fetch (vtRes) to call clearTimeout in a finally to
ensure the timer is always cleared even on exceptions.
- Line 151: Replace the unsafe type assertion when inserting into the DB by
constructing a concrete ScanLog object from auditData instead of using "as
ScanLog": validate and map fields from the existing auditData into a new const
scanLog: ScanLog (ensuring all required properties of the ScanLog type are
present and correctly typed) and then call
supabaseAdmin.from("file_scan_logs").insert(scanLog); this ensures missing or
incorrectly typed fields are caught by the compiler and at runtime.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: b6d54143-3e95-4f72-8502-a7d594908d28

📥 Commits

Reviewing files that changed from the base of the PR and between 9e3f79e and 61e0767.

📒 Files selected for processing (2)
  • supabase/functions/_shared/edge-authz-manifest.ts
  • supabase/functions/secure-upload/index.ts

"detect-new-device": { category: "public", rationale: "Anti-fraude pré-login" },
"bi-share-dossier": { category: "public", rationale: "Dossiê compartilhado por token" },
"dropbox-list": { category: "public", rationale: "Listagem pública de arquivos curados" },
"secure-upload": { category: "public", rationale: "Upload com scan VirusTotal anti-malware (aceita anônimo; uso real em áreas autenticadas — considerar promover a authenticated em PR futuro)" },
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

secure-upload should not remain classified as public with privileged backend writes.

This codifies anonymous access for an endpoint that writes using SUPABASE_SERVICE_ROLE_KEY, which expands abuse risk (storage/cost/traffic) even with malware scanning. Please move this to authenticated (or scoped with signed token validation) in the same hardening track.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/_shared/edge-authz-manifest.ts` at line 72, The manifest
entry for "secure-upload" currently sets category: "public" which allows
anonymous use of endpoints that perform privileged writes with
SUPABASE_SERVICE_ROLE_KEY; update the "secure-upload" entry in
edge-authz-manifest.ts to use a hardened category (e.g., change category from
"public" to "authenticated" or to "scoped" and add signed token validation) so
the endpoint requires authenticated/scoped access and ensure any downstream
logic that relies on this manifest enforces the new auth check.

'Access-Control-Allow-Headers': 'authorization, x-client-info, apikey, content-type',
}
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Headers": "authorization, x-client-info, apikey, content-type",
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

Allow x-request-id in CORS headers for true end-to-end correlation.

Right now browser callers cannot send X-Request-Id cross-origin, so upstream correlation propagation is blocked.

Proposed patch
-  "Access-Control-Allow-Headers": "authorization, x-client-info, apikey, content-type",
+  "Access-Control-Allow-Headers": "authorization, x-client-info, apikey, content-type, x-request-id",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/secure-upload/index.ts` at line 7, The CORS response
header list is missing "x-request-id", preventing browsers from sending
X-Request-Id cross-origin; update the Access-Control-Allow-Headers value (the
header key "Access-Control-Allow-Headers" where the string currently contains
"authorization, x-client-info, apikey, content-type") to include "x-request-id"
so browsers can forward the request ID for end-to-end correlation.

Comment on lines +41 to +44
const formData = await req.formData();
const file = formData.get("file") as File;
const folder = (formData.get("folder") as string) || "uploads";

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

Validate multipart field types before using them.

formData.get("file") as File can actually be string | null; a string will pass the truthy check and fail later at arrayBuffer() with a 500. Validate runtime types and return 400 for malformed payloads.

Proposed patch
-    const formData = await req.formData();
-    const file = formData.get("file") as File;
-    const folder = (formData.get("folder") as string) || "uploads";
+    const formData = await req.formData();
+    const filePart = formData.get("file");
+    if (!(filePart instanceof File)) {
+      return log.respond(
+        new Response(JSON.stringify({ error: "Arquivo inválido", request_id: requestId }), {
+          status: 400,
+          headers: { ...corsHeaders, "Content-Type": "application/json" },
+        }),
+      );
+    }
+    const folderPart = formData.get("folder");
+    const folder = typeof folderPart === "string" && folderPart.trim()
+      ? folderPart
+      : "uploads";
+    const file = filePart;

Also applies to: 54-57

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/secure-upload/index.ts` around lines 41 - 44, The handler
currently assumes multipart fields are the correct types; before calling
file.arrayBuffer() or using folder, validate runtime types and return a 400 for
malformed payloads. Specifically, after const formData = await req.formData()
and where you assign const file = formData.get("file") and const folder =
(formData.get("folder") as string) || "uploads", check that file is actually a
File-like object (e.g., instanceof File or has a callable arrayBuffer method)
and that folder is a string; if not, respond with status 400 and an error
message. Apply the same runtime checks at the later usage site(s) around the
code referenced on lines 54-57 where file/folder are used, and bail out early
with 400 rather than letting file.arrayBuffer() throw a 500.

Comment on lines +56 to +57
const fileBuffer = await file.arrayBuffer();
const hashBuffer = await crypto.subtle.digest("SHA-256", fileBuffer);
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

Add an explicit file-size guard before buffering into memory.

This endpoint is anonymous/public and loads the full file into memory (arrayBuffer()), which is a straightforward DoS vector without a hard byte limit.

Proposed patch
+    const MAX_FILE_BYTES = 10 * 1024 * 1024; // 10MB
+    if (file.size > MAX_FILE_BYTES) {
+      return log.respond(
+        new Response(JSON.stringify({ error: "Arquivo excede 10MB", request_id: requestId }), {
+          status: 413,
+          headers: { ...corsHeaders, "Content-Type": "application/json" },
+        }),
+      );
+    }
     const fileBuffer = await file.arrayBuffer();
📝 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 fileBuffer = await file.arrayBuffer();
const hashBuffer = await crypto.subtle.digest("SHA-256", fileBuffer);
const MAX_FILE_BYTES = 10 * 1024 * 1024; // 10MB
if (file.size > MAX_FILE_BYTES) {
return log.respond(
new Response(JSON.stringify({ error: "Arquivo excede 10MB", request_id: requestId }), {
status: 413,
headers: { ...corsHeaders, "Content-Type": "application/json" },
}),
);
}
const fileBuffer = await file.arrayBuffer();
const hashBuffer = await crypto.subtle.digest("SHA-256", fileBuffer);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/secure-upload/index.ts` around lines 56 - 57, Add a hard
byte-limit check before buffering the uploaded file: define a MAX_UPLOAD_BYTES
constant and, inside the request handler (before calling file.arrayBuffer() /
before computing hash via crypto.subtle.digest), inspect the file.size (or
Content-Length) and immediately return a 413/appropriate error if it exceeds the
limit; only call file.arrayBuffer() and continue with hashBuffer = await
crypto.subtle.digest(...) after the size check passes.

Comment on lines +191 to +193
JSON.stringify({ error: errorObj.message ?? "Erro desconhecido", request_id: requestId }),
{ headers: { ...corsHeaders, "Content-Type": "application/json" }, status: 500 },
),
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

Do not expose raw internal error messages in public responses.

Returning errorObj.message can leak internal details. Keep full details in logs, return a stable generic message with request_id.

Proposed patch
-        JSON.stringify({ error: errorObj.message ?? "Erro desconhecido", request_id: requestId }),
+        JSON.stringify({ error: "Falha no upload", request_id: requestId }),
📝 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
JSON.stringify({ error: errorObj.message ?? "Erro desconhecido", request_id: requestId }),
{ headers: { ...corsHeaders, "Content-Type": "application/json" }, status: 500 },
),
JSON.stringify({ error: "Falha no upload", request_id: requestId }),
{ headers: { ...corsHeaders, "Content-Type": "application/json" }, status: 500 },
),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/secure-upload/index.ts` around lines 191 - 193, The
response currently includes errorObj.message which may leak internal details;
change the response body to a stable generic message (e.g., "Internal server
error") and include only the requestId for correlation, while still logging the
full error (errorObj and requestId) internally; update the JSON.stringify call
that builds the 500 response (the block using requestId, errorObj, corsHeaders)
to return { error: "Internal server error", request_id: requestId } and ensure
any logging statements record errorObj.message/stack with requestId for
debugging.

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

Corrige violações de SSOT/CI para a edge function secure-upload, registrando-a no manifest de autorização e migrando o logging para o structured logger SSOT com propagação de request_id.

Changes:

  • Adiciona secure-upload ao EDGE_AUTHZ_MANIFEST como função public.
  • Migra secure-upload para createStructuredLogger + getOrCreateRequestId, usando Deno.serve e respostas via log.respond.
  • Padroniza eventos de log e inclui request_id em header e body das respostas.

Reviewed changes

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

File Description
supabase/functions/secure-upload/index.ts Migra para logger estruturado SSOT, adiciona request-id e ajusta logs/handlers.
supabase/functions/_shared/edge-authz-manifest.ts Registra secure-upload no manifest SSOT de autorização.

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

@@ -1,148 +1,196 @@
import { serve } from "https://deno.land/std@0.168.0/http/server.ts"
import { createClient } from "https://esm.sh/@supabase/supabase-js@2"
import { createClient } from "https://esm.sh/@supabase/supabase-js@2";
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

O import do @supabase/supabase-js está sem pin de versão (https://esm.sh/@supabase/supabase-js@2). No repo, esse import costuma estar fixado em uma versão exata (ex.: _shared/auth.ts:4 usa @2.49.4), para evitar mudanças não-determinísticas via esm.sh. Sugestão: fixar a versão aqui para alinhar com esse padrão.

Suggested change
import { createClient } from "https://esm.sh/@supabase/supabase-js@2";
import { createClient } from "https://esm.sh/@supabase/supabase-js@2.49.4";

Copilot uses AI. Check for mistakes.
"detect-new-device": { category: "public", rationale: "Anti-fraude pré-login" },
"bi-share-dossier": { category: "public", rationale: "Dossiê compartilhado por token" },
"dropbox-list": { category: "public", rationale: "Listagem pública de arquivos curados" },
"secure-upload": { category: "public", rationale: "Upload com scan VirusTotal anti-malware (aceita anônimo; uso real em áreas autenticadas — considerar promover a authenticated em PR futuro)" },
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

O manifest está marcando secure-upload como public, mas a função usa SUPABASE_SERVICE_ROLE_KEY para fazer upload em Storage. Isso permite que qualquer caller anônimo dispare uploads (e potencialmente abuse de custo/armazenamento) sem qualquer controle de identidade/escopo. Considerar mudar a categoria para authenticated e adicionar enforcement in-function (ex.: autenticação JWT) ou algum mecanismo equivalente (token assinado/rate limiting) antes de assumir como público no SSOT.

Suggested change
"secure-upload": { category: "public", rationale: "Upload com scan VirusTotal anti-malware (aceita anônimo; uso real em áreas autenticadas — considerar promover a authenticated em PR futuro)" },
"secure-upload": {
category: "authenticated",
rationale: "Upload aciona Storage com privilégios elevados; exige usuário autenticado para reduzir abuso de custo/armazenamento",
enforcedBy: "shared-authorize",
},

Copilot uses AI. Check for mistakes.
Comment on lines 5 to +8
const corsHeaders = {
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Headers': 'authorization, x-client-info, apikey, content-type',
}
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Headers": "authorization, x-client-info, apikey, content-type",
};
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

Este endpoint está com CORS wildcard (Access-Control-Allow-Origin: *). Como a função faz upload usando service-role, isso amplia a superfície de abuso (qualquer site pode chamar via browser). Se a intenção é uso em áreas autenticadas do frontend, preferir getCorsHeaders(req) do _shared/cors.ts (allowlist) em vez de wildcard.

Copilot uses AI. Check for mistakes.
Comment on lines +41 to +44
const formData = await req.formData();
const file = formData.get("file") as File;
const folder = (formData.get("folder") as string) || "uploads";

Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

const file = formData.get("file") as File e formData.get("folder") as string mascaram tipos inválidos vindos do client (ex.: file pode ser string ou null). Isso pode causar exceções em runtime (ex.: file.arrayBuffer()), virando 500 ao invés de erro de request. Validar explicitamente file instanceof File e que folder é string (e, idealmente, sanitizar para evitar ../ e barras).

Copilot uses AI. Check for mistakes.
Comment on lines +78 to 79
const vtApiKey = Deno.env.get("VIRUSTOTAL_API_KEY");

Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

O fluxo de segurança só roda se VIRUSTOTAL_API_KEY estiver setada; caso contrário, o upload segue sem scan, apesar do propósito/rationale sugerirem que há verificação anti-malware. Para evitar um bypass silencioso em ambientes mal configurados, considerar ao menos logar um evento (ex.: security_check_skipped) e/ou falhar o request quando a chave não existir em produção.

Suggested change
const vtApiKey = Deno.env.get("VIRUSTOTAL_API_KEY");
const vtApiKey = Deno.env.get("VIRUSTOTAL_API_KEY");
const isProduction = ["production", "prod"].includes(
(Deno.env.get("NODE_ENV") ?? Deno.env.get("ENV") ?? Deno.env.get("SUPABASE_ENV") ?? "")
.toLowerCase(),
);
if (!vtApiKey) {
scanDetails = {
...scanDetails,
source: "VirusTotal",
skipped: true,
reason: "VIRUSTOTAL_API_KEY não configurada",
};
log.error("security_check_skipped", {
reason: "missing_virustotal_api_key",
environment: isProduction ? "production" : "non-production",
bucket: auditData.bucket,
path: auditData.path,
hash: auditData.hash,
});
if (isProduction) {
throw new Error("Configuração de segurança ausente: VIRUSTOTAL_API_KEY");
}
}

Copilot uses AI. Check for mistakes.
@adm01-debug adm01-debug merged commit 0a5784e into main Apr 29, 2026
6 of 11 checks passed
adm01-debug added a commit that referenced this pull request Apr 30, 2026
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

Co-authored-by: Joaquim (via Claude Code VPS) <joaquim@promobrindes.com.br>
@adm01-debug adm01-debug deleted the claude/fix-secure-upload-authz-logging branch May 9, 2026 21:03
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