fix(qa): correção de 11 bugs reais — MFA bypass, fail-open RBAC, carrinho, autosave + 7#332
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughPR corrige bugs críticos de race conditions, segurança de acesso e cache em múltiplos hooks. Implementa default-deny em permissões, cancela respostas assíncronas fora de ordem, deduplica carrinho por cor, corrige chaves de cache, valida auto-save e popula tabela de preços dinamicamente. TypeScript baseline reduzido em 10 erros. ChangesCorreções de bugs e segurança
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🎯 PLANO MASTER — coordenação das 5 PRs abertas
Decisões cravadas (25/05/2026)
Estado atual
Próximo passo para ESTE PR (#332)
Checklist de validação local antes do merge
🤖 Coordenação: Claude (sessão 25/05/2026). Para reentrar no contexto em outra sessão, leia este bloco primeiro. |
|
Tip For best results, initiate chat on the files or code changes.
Estado de PR
Quando pronto para review: Aguardarei o ✏️ Learnings added
|
…nte no carrinho, autosave sobrescrevendo edições + 5 Auditoria exaustiva de QA. Correções verificadas (testes verdes, tsc baseline 508→498, zero regressão): P0 segurança - useAuthMFA: lia data.currentLevel/nextLevel mas authService.fetchAAL() retorna currentAAL/nextAAL → currentAAL sempre undefined, então o gate de step-up (mfaRequired && currentAAL==='aal1') NUNCA disparava. Usuários com MFA em sessão AAL1 entravam em /admin/* e rotas dev sem o 2º fator. Corrige nomes + mock que mascarava. - access-policy.checkAccess: requiredRole fora de 'supervisor'/'dev' (ex.: 'admin', 'manager') caía em allowed:true (fail-open). Adiciona default-deny. P1 integridade de dados / features quebradas - useSellerCarts.addItem: dedup só por product_id mesclava a 2ª cor na linha da 1ª (perda de variante) e estourava .maybeSingle() com 2+ linhas. Casa por variante (+ .is null). - useAutoSaveQuote: efeito de restauração re-rodava a cada render (onRestore inline) e re-aplicava o rascunho salvo por cima de edições ao vivo. Guard de restauração única. - useTecnicasUnificadas: priceTables hardcoded [] fazia QuantityComparisonTable (/simulador-precos) renderizar "N/D" em toda célula. Deriva de techniques. P2 correção - bridge-status-events: Omit não-distributivo sobre união discriminada descartava reason/attempt/attempts. DistributiveOmit restaura type-safety (−5 erros tsc). - useColorEnrichment: queryKey por .length (não conteúdo) servia enrichment de conjunto de IDs errado em colisão de tamanho. Chave por conteúdo. - useMagicUpState: resposta async fora-de-ordem sobrescrevia cores/imagens do produto errado. Adiciona guard de cancelamento. P3 - useProductImages: .sort() mutava o array do caller em render. Copia antes. - migratePayload: type-safe (acesso a unknown). kill-switch test: tipa mock.
2c6de8e to
175c333
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
✅ Rebase em main concluído — pronto para reviewO que mudou
Resultado após rebase
Validação pre-pushOutras PRs do plano master — atualização
@coderabbitai review 🤖 Rebase + force-push executados via Claude Code VPS. Pronto para CodeRabbit avaliar. |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
Este PR reúne um pacote de correções de QA focado em segurança (MFA/AAL e RBAC) e em integridade/robustez de features (carrinho, autosave, comparação de preços e estabilidade de hooks), além de reduzir o baseline de erros do TypeScript.
Changes:
- Security hardening: corrige leitura de AAL no
useAuthMFAe reforçacheckAccesscom default-deny para evitar fail-open de papéis não tratados. - Fixes de integridade de UX/dados: dedupe de itens do carrinho por variante (produto+cor) e restauração do autosave limitada a uma única vez por montagem.
- Robustez/perf: priceTables derivado das técnicas ativas, chaves de cache mais estáveis e guard contra resposta async fora de ordem no Magic Up.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/access/access-policy.ts | Endurece RBAC no checkAccess com default-deny e tratamento explícito de roles de gestão. |
| src/hooks/simulation/useTecnicasUnificadas.ts | Passa a derivar priceTables de techniques para restaurar a tabela de comparação de preços. |
| src/hooks/quotes/useAutoSaveQuote.ts | Torna migração mais defensiva e evita re-restore por re-render (guard de restauração única). |
| src/hooks/products/useSellerCarts.ts | Deduplicação de item no carrinho considerando variante (incluindo NULL via .is). |
| src/hooks/products/useProductImages.ts | Evita mutação do array de IDs ao montar queryKey (cópia antes do .sort()). |
| src/hooks/products/useColorEnrichment.ts | Corrige colisão de cache alterando queryKey para depender do conteúdo dos IDs. |
| src/hooks/intelligence/useMagicUpState.ts | Adiciona cancelamento para impedir resposta fora de ordem sobrescrever estado ao trocar de produto. |
| src/hooks/auth/useAuthMFA.ts | Corrige campos retornados por fetchAAL (currentAAL/nextAAL). |
| src/contexts/AuthContext.test.tsx | Ajusta mock de fetchAAL para refletir o shape correto. |
| .tsc-baseline.json | Atualiza baseline (redução de totalErrors e remoção de entradas corrigidas). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const colorName = item.color_name ?? null; | ||
| let lookup = supabase | ||
| .from('seller_cart_items') | ||
| .select('id, quantity') | ||
| .eq('cart_id', cartId) | ||
| .eq('product_id', item.product_id) | ||
| .maybeSingle(); | ||
| .eq('product_id', item.product_id); | ||
| lookup = | ||
| colorName === null ? lookup.is('color_name', null) : lookup.eq('color_name', colorName); | ||
|
|
| @@ -298,6 +298,9 @@ export function useMagicUpState() { | |||
| setSelectedTechnique(null); | |||
| const isSupervisorOrAbove = safeRoles.some((r) => | ||
| ['dev', 'supervisor', 'admin', 'manager'].includes(r), | ||
| ); |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hooks/intelligence/useMagicUpState.ts (1)
293-300:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResetar
loadingColorsao limparselectedProductpara evitar loading preso.Se o produto for limpo com a requisição anterior ainda em voo, o cleanup marca
cancelled = truee ofinallydeixa de executarsetLoadingColors(false). Como o branch!selectedProductretorna sem resetar esse estado, o loading pode ficar indefinidamente emtrue.💡 Patch sugerido
useEffect(() => { if (!selectedProduct) { + setLoadingColors(false); setColors([]); setProductImages([]); setSelectedColor(null); setSelectedLocationId(null); setSelectedTechnique(null); return; }Also applies to: 352-357
🤖 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 `@src/hooks/intelligence/useMagicUpState.ts` around lines 293 - 300, When clearing selectedProduct in useMagicUpState, also reset the loadingColors flag to avoid a stuck spinner: add setLoadingColors(false) alongside setColors([]), setProductImages([]), setSelectedColor(null), setSelectedLocationId(null), and setSelectedTechnique(null) in the branch that returns early when !selectedProduct; apply the same change to the other similar cleanup branch (the one around the 352-357 area) so both paths explicitly clear loadingColors in addition to relying on the request cancellation/finally.src/hooks/products/useProductImages.ts (1)
172-172:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMutação de array de entrada.
getImageUrlsordena o arrayimagesin-place (.sort()), mutando o argumento do chamador. Isso é inconsistente com o fix feito na linha 199 e pode causar bugs se o array for reutilizado.🛡️ Fix sugerido
export function getImageUrls(images: ProductImage[]): string[] { - return images.sort((a, b) => a.display_order - b.display_order).map((img) => img.url_cdn); + return [...images].sort((a, b) => a.display_order - b.display_order).map((img) => img.url_cdn); }🤖 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 `@src/hooks/products/useProductImages.ts` at line 172, getImageUrls currently calls images.sort(...) which mutates the caller's array; instead create a shallow copy before sorting to avoid in-place mutation (e.g., copy images then sort by display_order and map to url_cdn). Update the implementation of getImageUrls to operate on the copied array so the original images argument is not modified; reference the getImageUrls function, the images parameter, the display_order comparator and the url_cdn mapping when locating the change.
🧹 Nitpick comments (3)
src/hooks/products/useColorEnrichment.ts (2)
34-41: ⚖️ Poor tradeoffCache de módulo nunca é invalidado.
cachedColorGroupsecachedColorVariationssão carregados uma vez e nunca resetados. Se cores forem adicionadas/atualizadas durante a sessão do usuário (ex: admin cria nova cor e usuário continua navegando), o cache vai servir dados desatualizados.Se isso é aceitável (cores mudam raramente e só após deploy), está ok. Caso contrário, considere usar React Query para essas referências também ou expor uma função de invalidação.
🤖 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 `@src/hooks/products/useColorEnrichment.ts` around lines 34 - 41, The module-level caches cachedColorGroups and cachedColorVariations are never invalidated; update useColorEnrichment.ts to provide a clear invalidation strategy: either migrate these lookups to React Query (so they auto-refetch and can be invalidated via queryClient.invalidateQueries) or add and export an explicit invalidateColors() function that sets cachedColorGroups = null and cachedColorVariations = null and call it after mutating color data (e.g., from admin flows). Ensure references to cachedColorGroups and cachedColorVariations remain consistent and documented so callers know to trigger invalidation when colors change.
116-127: ⚡ Quick winType assertions sem validação de runtime.
As conversões
as Array<{...}>nas linhas 117 e 120 assumem queinvokeBatchBridgeretorna exatamente a estrutura esperada. Se a API externa mudar ou retornar dados inesperados, isso pode causar erros em runtime ou comportamento silencioso incorreto.Considere adicionar validação básica (ex: verificar se
recordsé array e tem campos esperados) ou usar um schema validator (Zod, etc.).🤖 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 `@src/hooks/products/useColorEnrichment.ts` around lines 116 - 127, The current casts for cachedColorGroups and cachedColorVariations assume refResults from invokeBatchBridge contain the exact shape; replace the unchecked "as Array<...>" casts with runtime validation: check refResults[0]?.data?.records and refResults[1]?.data?.records are arrays (Array.isArray) and filter/map items to ensure required properties (id, slug for groups; id, name, slug, group_id, hex_code for variations) exist and have expected types, or integrate a schema validator (e.g., Zod) to parse/validate the payload before assigning to cachedColorGroups and cachedColorVariations so malformed API responses are rejected/normalized rather than blindly cast.src/hooks/products/useProductImages.ts (1)
88-110: 🏗️ Heavy liftPerformance: fetch de todas as imagens do BD.
fetchProductImagesBatchbusca até 5000 imagens de todos os produtos (filters: { is_active: true }) e filtra em memória, mesmo quando apenas algunsproductIdssão solicitados. Se o catálogo tem milhares de produtos, isso transfere dados desnecessários.O comentário na linha 89 indica limitação do bridge (não suporta IN), mas considere:
- Usar
invokeBatchBridgecom múltiplas queries (uma por produto) se a API suportar batching eficiente- Ou adicionar suporte a filtros IN no bridge
Se essa limitação é conhecida e aceita, pode ignorar.
🤖 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 `@src/hooks/products/useProductImages.ts` around lines 88 - 110, The current fetch logic issues invokeExternalDb({ table: 'product_images', filters: { is_active: true }, limit: 5000 }) then filters by productIds in memory (see result.records and productIdSet), causing huge unnecessary transfers; fix by pushing the product filter into the DB call instead of in-memory: either update the bridge call to accept an IN filter for product_id (e.g., filters: { is_active: true, product_id: { in: productIds } }) or replace the single invokeExternalDb call with a batched approach (use invokeBatchBridge or similar to send one query per productId) so only matching images are returned; adjust the code that builds imagesByProduct to consume the reduced result set unchanged.
🤖 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 `@src/hooks/auth/useAuthMFA.ts`:
- Around line 16-19: O bloco catch do método que chama fetchAAL está apenas
logando o erro e deve falhar-fechado: ao capturar qualquer exceção de fetchAAL
limpe explicitamente o estado de autenticação step-up removendo/zerando
currentAAL, nextAAL e hasMFA (garantir que sejam definidos como nulos/false
conforme os tipos do hook) para evitar que um valor antigo (por exemplo aal2)
mantenha o gate local em estado MFA-validado; atualize o catch existente (onde
logger.warn é chamado) para também resetar esses campos e, se houver, disparar
quaisquer efeitos/updates necessários para propagar a falha de MFA ao resto do
hook/componente.
In `@src/hooks/products/useSellerCarts.ts`:
- Line 178: A desestruturação atual em useSellerCarts que faz const { data:
existing } = await lookup.limit(1).maybeSingle() ignora o campo error retornado
por .maybeSingle(); modifique para desestruturar { data: existing, error } a
partir de lookup.limit(1).maybeSingle(), verifique se error existe e trate-o
(lançando, retornando um erro ou logando e abortando a operação) antes de
prosseguir com a inserção; garanta também que a lógica que decide criar um novo
registro diferencia entre existing === null (nenhum registro) e undefined (falha
na query) para evitar duplicatas ou mascaramento de falhas.
In `@src/hooks/quotes/useAutoSaveQuote.ts`:
- Around line 29-31: The payload read in useAutoSaveQuote (variable payload) is
only checked with typeof object then blindly cast to { version?: number } and to
T/AutoSavePayload<T>, which can throw at runtime for malformed localStorage
data; update the validation in the top of the function to assert that payload is
non-null, typeof payload === 'object', and that required keys exist (e.g.,
'version' and/or 'data' present) before casting to versioned /
AutoSavePayload<T>, log a warning via the module logger when structure is
invalid, and return null to avoid unsafe type assertions in functions like
useAutoSaveQuote and any code paths that rely on the versioned/data shape.
In `@src/lib/access/access-policy.ts`:
- Around line 32-40: The management-role check in access-policy.ts (the branch
handling requiredRole === 'supervisor' || 'admin' || 'manager') is out of sync
with the server-side helper in supabase/functions/_shared/auth.ts — the server
includes 'simulation' in the supervisor-or-above set and treats 'manager'
differently; update the frontend logic so the same helper/role-set is used:
either change the local isSupervisorOrAbove implementation to match the server
helper's exact role membership (including 'simulation' and the same
inclusion/exclusion of 'manager') or replace the inline condition in
access-policy.ts to call the shared helper/function used by the backend so both
sides evaluate the same set of roles. Ensure you reference and align the symbols
isSupervisorOrAbove and the server helper in supabase/functions/_shared/auth.ts.
---
Outside diff comments:
In `@src/hooks/intelligence/useMagicUpState.ts`:
- Around line 293-300: When clearing selectedProduct in useMagicUpState, also
reset the loadingColors flag to avoid a stuck spinner: add
setLoadingColors(false) alongside setColors([]), setProductImages([]),
setSelectedColor(null), setSelectedLocationId(null), and
setSelectedTechnique(null) in the branch that returns early when
!selectedProduct; apply the same change to the other similar cleanup branch (the
one around the 352-357 area) so both paths explicitly clear loadingColors in
addition to relying on the request cancellation/finally.
In `@src/hooks/products/useProductImages.ts`:
- Line 172: getImageUrls currently calls images.sort(...) which mutates the
caller's array; instead create a shallow copy before sorting to avoid in-place
mutation (e.g., copy images then sort by display_order and map to url_cdn).
Update the implementation of getImageUrls to operate on the copied array so the
original images argument is not modified; reference the getImageUrls function,
the images parameter, the display_order comparator and the url_cdn mapping when
locating the change.
---
Nitpick comments:
In `@src/hooks/products/useColorEnrichment.ts`:
- Around line 34-41: The module-level caches cachedColorGroups and
cachedColorVariations are never invalidated; update useColorEnrichment.ts to
provide a clear invalidation strategy: either migrate these lookups to React
Query (so they auto-refetch and can be invalidated via
queryClient.invalidateQueries) or add and export an explicit invalidateColors()
function that sets cachedColorGroups = null and cachedColorVariations = null and
call it after mutating color data (e.g., from admin flows). Ensure references to
cachedColorGroups and cachedColorVariations remain consistent and documented so
callers know to trigger invalidation when colors change.
- Around line 116-127: The current casts for cachedColorGroups and
cachedColorVariations assume refResults from invokeBatchBridge contain the exact
shape; replace the unchecked "as Array<...>" casts with runtime validation:
check refResults[0]?.data?.records and refResults[1]?.data?.records are arrays
(Array.isArray) and filter/map items to ensure required properties (id, slug for
groups; id, name, slug, group_id, hex_code for variations) exist and have
expected types, or integrate a schema validator (e.g., Zod) to parse/validate
the payload before assigning to cachedColorGroups and cachedColorVariations so
malformed API responses are rejected/normalized rather than blindly cast.
In `@src/hooks/products/useProductImages.ts`:
- Around line 88-110: The current fetch logic issues invokeExternalDb({ table:
'product_images', filters: { is_active: true }, limit: 5000 }) then filters by
productIds in memory (see result.records and productIdSet), causing huge
unnecessary transfers; fix by pushing the product filter into the DB call
instead of in-memory: either update the bridge call to accept an IN filter for
product_id (e.g., filters: { is_active: true, product_id: { in: productIds } })
or replace the single invokeExternalDb call with a batched approach (use
invokeBatchBridge or similar to send one query per productId) so only matching
images are returned; adjust the code that builds imagesByProduct to consume the
reduced result set unchanged.
🪄 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: 5bc52080-ae00-416f-b4b4-46a12a45f630
📒 Files selected for processing (10)
.tsc-baseline.jsonsrc/contexts/AuthContext.test.tsxsrc/hooks/auth/useAuthMFA.tssrc/hooks/intelligence/useMagicUpState.tssrc/hooks/products/useColorEnrichment.tssrc/hooks/products/useProductImages.tssrc/hooks/products/useSellerCarts.tssrc/hooks/quotes/useAutoSaveQuote.tssrc/hooks/simulation/useTecnicasUnificadas.tssrc/lib/access/access-policy.ts
| } catch (e) { | ||
| if (import.meta.env.DEV) logger.warn('AAL fetch failed', e instanceof Error ? e.message : String(e)); | ||
| if (import.meta.env.DEV) | ||
| logger.warn('AAL fetch failed', e instanceof Error ? e.message : String(e)); | ||
| } |
There was a problem hiding this comment.
Faça fail-closed quando fetchAAL falhar.
Se fetchAAL já tiver retornado aal2 antes, uma falha posterior mantém o estado antigo e o gate local continua tratando a sessão como MFA-validada. Para um fluxo de step-up, isso vira bypass por estado stale; no catch, limpe currentAAL, nextAAL e hasMFA.
Diff sugerido
} catch (e) {
+ setCurrentAAL(null);
+ setNextAAL(null);
+ setHasMFA(false);
if (import.meta.env.DEV)
logger.warn('AAL fetch failed', e instanceof Error ? e.message : String(e));
}📝 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.
| } catch (e) { | |
| if (import.meta.env.DEV) logger.warn('AAL fetch failed', e instanceof Error ? e.message : String(e)); | |
| if (import.meta.env.DEV) | |
| logger.warn('AAL fetch failed', e instanceof Error ? e.message : String(e)); | |
| } | |
| } catch (e) { | |
| setCurrentAAL(null); | |
| setNextAAL(null); | |
| setHasMFA(false); | |
| if (import.meta.env.DEV) | |
| logger.warn('AAL fetch failed', e instanceof Error ? e.message : String(e)); | |
| } |
🤖 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 `@src/hooks/auth/useAuthMFA.ts` around lines 16 - 19, O bloco catch do método
que chama fetchAAL está apenas logando o erro e deve falhar-fechado: ao capturar
qualquer exceção de fetchAAL limpe explicitamente o estado de autenticação
step-up removendo/zerando currentAAL, nextAAL e hasMFA (garantir que sejam
definidos como nulos/false conforme os tipos do hook) para evitar que um valor
antigo (por exemplo aal2) mantenha o gate local em estado MFA-validado; atualize
o catch existente (onde logger.warn é chamado) para também resetar esses campos
e, se houver, disparar quaisquer efeitos/updates necessários para propagar a
falha de MFA ao resto do hook/componente.
| lookup = | ||
| colorName === null ? lookup.is('color_name', null) : lookup.eq('color_name', colorName); | ||
|
|
||
| const { data: existing } = await lookup.limit(1).maybeSingle(); |
There was a problem hiding this comment.
Erro de lookup não está sendo verificado.
A desestruturação ignora o campo error retornado por .maybeSingle(). Se a query falhar (erro de rede, timeout, constraint violation), existing será undefined e o código prosseguirá para inserção, potencialmente criando duplicatas ou mascarando falhas do banco.
🛡️ Fix proposto para tratar erro de lookup
- const { data: existing } = await lookup.limit(1).maybeSingle();
+ const { data: existing, error: lookupError } = await lookup.limit(1).maybeSingle();
+ if (lookupError) throw lookupError;📝 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 { data: existing } = await lookup.limit(1).maybeSingle(); | |
| const { data: existing, error: lookupError } = await lookup.limit(1).maybeSingle(); | |
| if (lookupError) throw lookupError; |
🤖 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 `@src/hooks/products/useSellerCarts.ts` at line 178, A desestruturação atual em
useSellerCarts que faz const { data: existing } = await
lookup.limit(1).maybeSingle() ignora o campo error retornado por .maybeSingle();
modifique para desestruturar { data: existing, error } a partir de
lookup.limit(1).maybeSingle(), verifique se error existe e trate-o (lançando,
retornando um erro ou logando e abortando a operação) antes de prosseguir com a
inserção; garanta também que a lógica que decide criar um novo registro
diferencia entre existing === null (nenhum registro) e undefined (falha na
query) para evitar duplicatas ou mascaramento de falhas.
| if (!payload || typeof payload !== 'object') return null; | ||
|
|
||
| const versioned = payload as { version?: number }; |
There was a problem hiding this comment.
Validação incompleta de unknown com type assertions.
O payload é validado apenas como objeto (typeof payload !== 'object'), mas depois usa type assertions (as { version?: number }, linha 38 as T, linha 55 as AutoSavePayload<T>) sem verificar se a estrutura realmente existe. Se o localStorage contiver dados corrompidos ou malformados, o código pode falhar em runtime.
Considere adicionar validações explícitas:
if (!payload || typeof payload !== 'object') return null;
const versioned = payload as { version?: number; data?: unknown };
// Validar presença de campos críticos
if (!('version' in versioned) && !('data' in versioned)) {
logger.warn('[AutoSave] Invalid payload structure');
return null;
}🤖 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 `@src/hooks/quotes/useAutoSaveQuote.ts` around lines 29 - 31, The payload read
in useAutoSaveQuote (variable payload) is only checked with typeof object then
blindly cast to { version?: number } and to T/AutoSavePayload<T>, which can
throw at runtime for malformed localStorage data; update the validation in the
top of the function to assert that payload is non-null, typeof payload ===
'object', and that required keys exist (e.g., 'version' and/or 'data' present)
before casting to versioned / AutoSavePayload<T>, log a warning via the module
logger when structure is invalid, and return null to avoid unsafe type
assertions in functions like useAutoSaveQuote and any code paths that rely on
the versioned/data shape.
| } else if ( | ||
| requiredRole === 'supervisor' || | ||
| requiredRole === 'admin' || | ||
| requiredRole === 'manager' | ||
| ) { | ||
| // Papéis de gestão exigem supervisor-ou-acima. | ||
| if (!isSupervisorOrAbove) { | ||
| return { allowed: false, reason: 'insufficient_role' }; | ||
| } |
There was a problem hiding this comment.
Alinhe a hierarquia de gestão com a regra do backend.
Nas Lines 32-40, admin/manager passam a depender de isSupervisorOrAbove, mas o helper equivalente em supabase/functions/_shared/auth.ts:84-91 usa outro conjunto de papéis (simulation entra e manager não). Isso cria drift de autorização entre cliente e servidor: o frontend pode liberar rota que o backend não reconhece, ou bloquear um papel que o backend aceita.
🤖 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 `@src/lib/access/access-policy.ts` around lines 32 - 40, The management-role
check in access-policy.ts (the branch handling requiredRole === 'supervisor' ||
'admin' || 'manager') is out of sync with the server-side helper in
supabase/functions/_shared/auth.ts — the server includes 'simulation' in the
supervisor-or-above set and treats 'manager' differently; update the frontend
logic so the same helper/role-set is used: either change the local
isSupervisorOrAbove implementation to match the server helper's exact role
membership (including 'simulation' and the same inclusion/exclusion of
'manager') or replace the inline condition in access-policy.ts to call the
shared helper/function used by the backend so both sides evaluate the same set
of roles. Ensure you reference and align the symbols isSupervisorOrAbove and the
server helper in supabase/functions/_shared/auth.ts.
…argem latente Itens de follow-up da auditoria de QA (PR #332). 1. Undo/redo do kit-builder agora FUNCIONA (antes: botões sempre desabilitados) - pushSnapshot nunca era chamado → history vazio → canUndo sempre false; e undo()/redo() devolviam snapshots que ninguém aplicava de volta. - Snapshot passa a guardar estado COMPLETO restaurável (KitSnapshot) em vez da versão lossy (boxId/keys). - Novo useKitBuilder.restoreKitSnapshot aplica o snapshot SEM forçar o passo "summary" (diferente do loadKit). useKitBuilderPageState faz push-on-change (com guard isRestoring) e aplica o snapshot no undo/redo. - Testes unitários cobrindo push/dedup/undo/redo (15/15 verdes). 2. Remove componentes mortos PriceResultV51 + QuantityAndResult (+ exports do barrel). Eram os ÚNICOS lugares que exibiam o margin=markup e o preco_minimo_unitario=0; sem renderizadores (confirmado: nenhum importador). NOTA: o resto da "cadeia" (useGravacaoV2.ts, tipo CustomizationPriceV2, useGravacaoPriceV2) é ALCANÇÁVEL (via /simulador-precos → MultiEngravingResult) e/ou testado — mantido. 3. Corrige conflação margem×markup no adapter ALCANÇÁVEL (price-response.adapter): margin_percent agora é margem real derivada (markupToMargin), não mais um alias do markup. Não é exibido hoje, mas deixa o marginPercent do wizard correto caso seja exibido.
…nt + recaptura eslint baseline A branch (rebased em main) tropeçava em drift pré-existente de baseline ao rodar os gates, nenhum funcionalmente causado por este PR: 1. useUserManagement.ts (TS2589 + TS2352): o embed `user_roles(role)` depende da relação profiles↔user_roles, AUSENTE no types.ts gerado → SelectQueryError + instanciação profunda. Mesma família de drift do types.ts do #332. Corrigido na raiz com untypedFrom('profiles') (idiomático do projeto; query idêntica em runtime) — elimina os 2 erros de forma determinística, sem baselinar TS2589. 2. .eslint-baseline.json recapturado (127→124): absorve drift pré-existente (CoverageInsightsDashboardPage no-non-null-assertion) e remove entradas dos 2 componentes mortos deletados neste PR. CoverageInsights é de um arquivo NÃO tocado aqui — drift de merges anteriores sem regen do baseline.
…ções de gate (#403) * feat(kit-builder): liga undo/redo + remove gravação morta + corrige margem latente Itens de follow-up da auditoria de QA (PR #332). 1. Undo/redo do kit-builder agora FUNCIONA (antes: botões sempre desabilitados) - pushSnapshot nunca era chamado → history vazio → canUndo sempre false; e undo()/redo() devolviam snapshots que ninguém aplicava de volta. - Snapshot passa a guardar estado COMPLETO restaurável (KitSnapshot) em vez da versão lossy (boxId/keys). - Novo useKitBuilder.restoreKitSnapshot aplica o snapshot SEM forçar o passo "summary" (diferente do loadKit). useKitBuilderPageState faz push-on-change (com guard isRestoring) e aplica o snapshot no undo/redo. - Testes unitários cobrindo push/dedup/undo/redo (15/15 verdes). 2. Remove componentes mortos PriceResultV51 + QuantityAndResult (+ exports do barrel). Eram os ÚNICOS lugares que exibiam o margin=markup e o preco_minimo_unitario=0; sem renderizadores (confirmado: nenhum importador). NOTA: o resto da "cadeia" (useGravacaoV2.ts, tipo CustomizationPriceV2, useGravacaoPriceV2) é ALCANÇÁVEL (via /simulador-precos → MultiEngravingResult) e/ou testado — mantido. 3. Corrige conflação margem×markup no adapter ALCANÇÁVEL (price-response.adapter): margin_percent agora é margem real derivada (markupToMargin), não mais um alias do markup. Não é exibido hoje, mas deixa o marginPercent do wizard correto caso seja exibido. * fix(types): destrava gates — corrige TS2589/TS2352 em useUserManagement + recaptura eslint baseline A branch (rebased em main) tropeçava em drift pré-existente de baseline ao rodar os gates, nenhum funcionalmente causado por este PR: 1. useUserManagement.ts (TS2589 + TS2352): o embed `user_roles(role)` depende da relação profiles↔user_roles, AUSENTE no types.ts gerado → SelectQueryError + instanciação profunda. Mesma família de drift do types.ts do #332. Corrigido na raiz com untypedFrom('profiles') (idiomático do projeto; query idêntica em runtime) — elimina os 2 erros de forma determinística, sem baselinar TS2589. 2. .eslint-baseline.json recapturado (127→124): absorve drift pré-existente (CoverageInsightsDashboardPage no-non-null-assertion) e remove entradas dos 2 componentes mortos deletados neste PR. CoverageInsights é de um arquivo NÃO tocado aqui — drift de merges anteriores sem regen do baseline. --------- Co-authored-by: Claude <noreply@anthropic.com>
Auditoria exaustiva de QA
Varredura sistemática (camadas auth/RBAC, pricing, carrinho/orçamentos, hooks React, data-layer) cruzando build/typecheck/lint/testes com leitura manual. Cada achado foi verificado contra o código real antes de corrigir — incluindo descartar 1 falso-positivo (ver abaixo).
Verificação: tsc baseline
508→498(10 erros eliminados, zero regressão) · ESLint gate verde · testes das áreas tocadas: 221 passando.🔴 P0 — Segurança
1. Bypass de MFA/AAL2 em todas as rotas admin/dev (
src/hooks/auth/useAuthMFA.ts)useAuthMFAliadata.currentLevel/data.nextLevel, masauthService.fetchAAL()retornacurrentAAL/nextAAL. LogocurrentAALera sempreundefined, e o gate de step-upmfaRequired && currentAAL === 'aal1'(AdminRoute/DevRoute) nunca disparava. Usuário com MFA cadastrado mas sessão em AAL1 entrava em/admin/*e rotas dev sem o 2º fator. Os 2 errosTS2339no baseline eram exatamente esses acessos errados. Corrigido + o mock de teste que mascarava o bug.2. Fail-open em
checkAccess(src/lib/access/access-policy.ts)Qualquer
requiredRolediferente de'supervisor'/'dev'(ex.:'admin','manager','agente') caía emallowed: true. Adicionado default-deny + tratamento explícito de papéis de gestão.🟠 P1 — Integridade de dados / features quebradas
3. Perda de variante de cor no carrinho (
src/hooks/products/useSellerCarts.ts)addItemdeduplicava só porproduct_id. Adicionar o mesmo produto numa 2ª cor mesclava na linha da 1ª cor (variante perdida) e, com 2+ linhas, estourava.maybeSingle(). Passa a casar pela identidade da variante (com.is/.eqp/ NULL) +.limit(1).4. Autosave sobrescrevendo edições ao vivo (
src/hooks/quotes/useAutoSaveQuote.ts)O efeito de restauração tinha
onRestore(closure inline, identidade nova a cada render) na lista de deps → re-rodava a cada render e re-aplicava o rascunho salvo por cima das edições do usuário (ex.: 2º item adicionado revertia). Adicionado guard de restauração única. Bônus:migratePayloadtype-safe.5. Tabela de comparação de preços 100% quebrada (
src/hooks/simulation/useTecnicasUnificadas.ts)useCustomizationPricing().priceTablesestava hardcoded[]→QuantityComparisonTable(rota/simulador-precos) renderizava "N/D" em toda célula técnica×quantidade. Agora derivado detechniques.🟡 P2/P3 — Correção e robustez
6. Type-safety do event bus do bridge (
src/lib/external-db/bridge-status-events.ts) —Omitnão-distributivo sobre união discriminada descartavareason/attempt/attempts.DistributiveOmitrestaura a checagem (−5 erros tsc) no módulo do external-db-bridge.7. Colisão de cache no enrichment de cores (
src/hooks/products/useColorEnrichment.ts) —queryKeypor.length(não conteúdo) servia dados de conjunto de IDs errado. Chave por conteúdo.8. Race condition no Magic Up (
src/hooks/intelligence/useMagicUpState.ts) — resposta async fora-de-ordem sobrescrevia cores/imagens do produto errado. Guard de cancelamento.9. Mutação em render (
src/hooks/products/useProductImages.ts) —.sort()mutava o array do caller. Copia antes.10–11. Higiene — erro de tipo no mock do teste do kill-switch + destrava do gate ESLint (eqeqeq real
!= null→!== nullemObservabilityDashboard,import typeemuseKillSwitchBanner; baseline recapturado p/ drift pré-existente dos commits de observabilidade).⚪ Verificados e NÃO corrigidos (com justificativa)
activeItemIndex" emQuoteBuilderSummaryColumn— o ajuste recalcula a partir do valor capturado, produzindo o mesmo resultado doremoveItem. Redundante, não bug.useQuoteBuilderState): a inconsistência falha de forma segura (exige aprovação a mais), não é bypass.pushSnapshotnunca é chamado → botões sempre desabilitados. Inofensivo, mas requer trabalho de feature (não é fix de bug).price-response.adapter/useGravacaoV2):margin_percentrecebe o markup e é exibido como "Margem". Decisão de semântica de negócio (relabel vs. derivar margem) — reportado para definição.preco_minimo_unitario: 0hardcoded (hook legado),base_priceinexistente emProductSearch,contactId = client_idno modo edição (não hácontact_idpersistido),useProfileRolesmantém papéis em refetch vazio,quoteServiceescopo team/all depende de RLS (confirmar server-side),useCategoryDescendantscom storm de fetch latente (sem consumidor). Detalhes nos commits/relatório.🤖 Auditoria assistida por Claude Code. Cada correção tem teste verde e justificativa; achados de semântica de negócio foram reportados, não adivinhados.
Generated by Claude Code
Summary by cubic
Corrige bypass de MFA (AAL2) e fail-open no RBAC em rotas sensíveis, além de falhas no carrinho, autosave e comparação de preços. Evita corridas/cache incorretos e ajusta baseline do TypeScript no CI para maior estabilidade.
useAuthMFAusacurrentAAL/nextAAL; step-up reativado; mock de teste ajustado.checkAccessadota default-deny e trataadmin/manager..ispara NULL; evita mescla de cores e falha em.maybeSingle().migratePayloadlida comunknowncom segurança.priceTablesderivado detechniques; a comparação em/simulador-precosvolta a exibir valores..sort()sem mutar o array de entrada no batch de imagens.src/integrations/supabase/types.ts(TS2300+10,TS2717+1); ESLint verde; 221 testes relevantes passando.Written for commit 58ab5a7. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Release Notes
Bug Fixes
Melhorias de Performance
Segurança