-
Notifications
You must be signed in to change notification settings - Fork 0
fix(edge): registra secure-upload no manifest e migra para structured logger SSOT #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -69,6 +69,7 @@ export const EDGE_AUTHZ_MANIFEST: Record<string, AuthzEntry> = { | |||||||||||||
| "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)" }, | ||||||||||||||
|
||||||||||||||
| "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", | |
| }, |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -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"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { createClient } from "https://esm.sh/@supabase/supabase-js@2"; | |
| import { createClient } from "https://esm.sh/@supabase/supabase-js@2.49.4"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Copilot
AI
Apr 29, 2026
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Copilot
AI
Apr 29, 2026
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
Copilot
AI
Apr 29, 2026
There was a problem hiding this comment.
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.
| 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"); | |
| } | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secure-uploadshould not remain classified aspublicwith 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 toauthenticated(orscopedwith signed token validation) in the same hardening track.🤖 Prompt for AI Agents