Skip to content

fix: resetAccount recifra feedback com a DEK nova em vez de descartá-la (closes #121) - #136

Draft
Guiroos wants to merge 4 commits into
mainfrom
claude/quirky-johnson-czr4gt
Draft

fix: resetAccount recifra feedback com a DEK nova em vez de descartá-la (closes #121)#136
Guiroos wants to merge 4 commits into
mainfrom
claude/quirky-johnson-czr4gt

Conversation

@Guiroos

@Guiroos Guiroos commented Sep 1, 2026

Copy link
Copy Markdown
Owner

O que mudou

resetAccount apagava userSettings (onde mora encryptedDek) na Fase 1 e provisionava uma DEK aleatória nova na Fase 2. feedback é a única tabela com coluna cifrada por DEK que sobrevive ao reset — suas mensagens continuavam cifradas com a chave antiga, agora destruída e sem cópia em lugar nenhum. getAllFeedbacks (lib/queries/admin.ts) tenta decriptar todas as linhas dentro de um único Promise.all; a primeira mensagem órfã lança (decipher.final() falha o auth tag do GCM) e derruba a página /admin inteira para todos os usuários.

  1. lib/actions/reset-account.ts:
    • Antes da Fase 1, lê userSettings.encryptedDek direto (não via getDekForUser) e decripta com decryptDek para capturar a DEK antiga.
    • Captura o feedback pendente do usuário (id, message) antes do delete.
    • Ao final (nova Fase 4), recifra cada linha capturada com a DEK nova (decryptField com a antiga → encryptField com a nova), envolvido em try/catch por linha — um feedback já órfão de uma ocorrência anterior deste bug não pode voltar a bloquear o reset da conta.
  2. lib/queries/admin.ts: getDecryptedMessage passa a capturar falha de decrypt por linha (try/catch + console.error), devolvendo um texto sentinela em vez de derrubar o Promise.all inteiro — protege contra linhas órfãs que já existam em produção.

Por que dessa forma

Segui a proposta da issue à risca, inclusive o helper nomeado:

  • decryptDek lendo userSettings.encryptedDek direto, nunca getDekForUser para capturar a DEK antiga. getDekForUser é cache() do React, memoizado por request/Server Action — se a captura da DEK antiga chamasse getDekForUser no topo, a chamada da Fase 2 devolveria o mesmo valor memoizado (a DEK deletada), e as categorias padrão da Fase 3 seriam cifradas com uma chave que não existe mais no banco — o reset ficaria pior do que o bug que corrige. getDekForUser é o helper "óbvio" usado no resto do repo; aqui é exatamente o errado.
  • decryptField/encryptField, não decryptOptional/encryptOptional: feedback.message é .notNull() (schema.ts:544).
  • try/catch por linha na recifragem (não pedido explicitamente pela issue, mas necessário para não regredir): sem ele, um usuário que já tenha feedback órfão de uma ocorrência anterior deste bug (antes desta correção existir) teria resetAccount() lançando e ficaria impedido de resetar a própria conta — pior que o estado atual, que ao menos completa o reset.

Não toquei deleteAccount (lib/actions/delete-account.ts) — ali feedback.userId é onDelete: 'cascade' e a intenção é apagar tudo, comportamento já correto e fora do escopo desta issue.

Como testei

  • npm run lint && npm run format:check && npm run typecheck && npm test && npm run build — todos verdes (34 arquivos, 527 testes).
  • Novo teste em __tests__/integration/actions-reset-account.test.ts ('mantém o feedback legível depois do reset'), em it() separado dos demais para não contaminar as asserções toHaveLength(0) do primeiro teste: insere feedback cifrado com a DEK atual, chama resetAccount(), decripta com a DEK nova e assere o texto original — a asserção toBe(...) (não not.toThrow()) é o que distingue a correção certa de duas erradas plausíveis (incluir feedback na Fase 1 de delete, ou só um try/catch em getAllFeedbacks sem recifrar).
  • npm run test:integration não foi rodado — exige credenciais do Neon (NEON_API_KEY, NEON_PROJECT_ID, NEON_PARENT_BRANCH_ID) indisponíveis neste ambiente, e só roda em push para main. O teste novo está no arquivo de integração existente e será validado nesse pipeline.

Risco e o que NÃO foi coberto

  • O teste de integração novo depende do pipeline de main para rodar de fato — não pôde ser executado localmente.
  • Linhas de feedback já órfãs em produção (de ocorrências passadas deste bug, se houver) não são recuperáveis por este PR — a DEK que as cifrava já não existe em lugar nenhum. O try/catch da Fase 4 e o fallback de getAllFeedbacks evitam que elas voltem a quebrar o /admin, mas o texto original dessas linhas específicas está perdido.
  • Não implementei backfill/script de reparo para linhas já órfãs — fora do escopo da issue, que trata da causa (o reset futuro), não do histórico.

Arquivos tocados

  • lib/actions/reset-account.ts
  • lib/queries/admin.ts
  • __tests__/integration/actions-reset-account.test.ts

Closes #121


Generated by Claude Code

…la (closes #121)

feedback sobrevive à Fase 1 do reset por ser dado de produto, mas a
Fase 2 provisiona uma DEK aleatória nova — as mensagens continuavam
cifradas com a chave antiga, destruída, e getAllFeedbacks derrubava o
/admin inteiro no primeiro Promise.all que topasse com uma delas.

resetAccount agora lê a DEK antiga direto de userSettings (nunca via
getDekForUser, que memoiza por request e devolveria a DEK já apagada
na Fase 2) e recifra o feedback pendente com a DEK nova ao final.
getAllFeedbacks passa a degradar por linha em vez de derrubar a
página inteira, para as linhas já órfãs em produção.

@Guiroos Guiroos left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Revisão do head 4cac97a. A abordagem está certa e a armadilha central da issue foi respeitada: a DEK antiga vem de leitura direta de userSettings + decryptDek, e getDekForUser só aparece na Fase 2 — se a captura do topo o tivesse usado, o cache() do React devolveria a DEK deletada na Fase 2 e as categorias da Fase 3 sairiam cifradas com uma chave inexistente. O achado é de site único, e reconferi: das 21 pgTable do schema, a Fase 1 apaga 16 e sobram users, accounts, sessions, verificationTokens e feedback — só esta última tem coluna cifrada por DEK, então não há segundo site pendente (exigência 8 fechada).

O teste da Fase 4 cobre de verdade: com a recifragem revertida, row.message continua cifrado com a DEK antiga e decryptField(row.message, dekNova) estoura no auth tag do GCM — a asserção toBe(...) também derruba as duas correções erradas que a issue nomeou (apagar feedback na Fase 1 deixa row undefined; só o try/catch no /admin devolve o sentinela).

Dois achados: um bloqueante (cobertura da metade do /admin), um nit. Detalhe e correção concreta em cada comentário inline.

Uma nota que não é achado, mas vale conferir quando o pipeline de main rodar de fato: getDekForUser é cache(), e o teste novo o chama antes (dekAntiga) e depois (dekNova) do reset. Fora de um request do React o dispatcher de cache é nulo e a chamada não memoiza, então dekNova é mesmo a chave nova e o teste é válido — mas não deu para executar aqui (sem node_modules nem credenciais Neon nesta sessão), e se a memoização valesse no Vitest as duas variáveis seriam o mesmo Buffer e o teste passaria verde com o bug intacto. Se quiser tirar o teste dessa dependência de detalhe interno do React, ler userSettings.encryptedDek + decryptDek para obter dekNova é uma linha e usa exatamente o mesmo motivo pelo qual a action evita getDekForUser no topo.


Generated by Claude Code

Comment thread lib/queries/admin.ts
Comment thread lib/actions/reset-account.ts Outdated
…ível

Endereça a revisão de #136:
- lib/actions/reset-account.ts: decryptDek pode lançar (DEK cifrada com
  MEK que não é mais a do ambiente); antes rodava sem guard e um throw
  ali abortaria o reset inteiro antes mesmo da Fase 1 — pior que o
  bug que este PR corrige. Agora falha para null e loga, feedback
  simplesmente não é recifrado nesse caso.
- __tests__/integration/queries-admin.test.ts (novo): cobre o try/catch
  de getAllFeedbacks, que não tinha teste nenhum — insere uma linha
  cifrada com uma DEK estrangeira e assere o sentinela de fallback em
  vez de deixar o Promise.all rejeitar a lista inteira.

@Guiroos Guiroos left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Revisão do head 1b814ce (a anterior foi no 4cac97a). Os dois achados daquela rodada estão endereçados de verdade, não só respondidos: queries-admin.test.ts cobre o try/catch do /admin com a forma que eu tinha proposto (DEK estrangeira via randomBytes(32), asserção no sentinela, filtro por id), e o guard de decryptDek entrou em reset-account.ts:79-86. O let no lugar da IIFE está correto — o gotcha de react-hooks/immutability é escopado a callback de hook, e npm run lint passa limpo.

Rodei os gates nesta sessão, sobre este head: npm run lint, npm run typecheck e npm test verdes (34 arquivos, 527 testes). Os dois testes de integração novos não rodam em PR (if: github.event_name == 'push'), como o corpo já declara.

Dois achados, ambos nit — nenhum bloqueante. Os dois são de cobertura/completude do próprio guard, não da abordagem:

  1. lib/queries/admin.ts:51getDekForUser ficou fora do try que começa em :54. Ele também chama decryptDek, também lança, e a rejeição derruba o Promise.all inteiro — o mesmo /admin fora do ar que o PR foi corrigir, uma linha acima do guard. Correção é mover a busca para dentro do try que já existe.
  2. lib/actions/reset-account.ts:82 — o guard novo não tem teste; comentá-lo mantém a suíte verde.

Detalhe, evidência e o caso de teste concreto em cada comentário inline.

Uma retratação: a ressalva sobre cache() do React que deixei em aberto na revisão anterior era falso alarme e está retirada — consegui rodar aqui e confirmei que react@19.2.6 não memoiza fora de escopo de request. O teste da Fase 4 é válido como está; ignore a sugestão de trocá-lo por leitura direta de userSettings.


Generated by Claude Code

Comment thread lib/queries/admin.ts Outdated
Comment thread lib/actions/reset-account.ts
Endereça a revisão de 1b814ce:
- lib/queries/admin.ts: getDekForUser estava fora do try/catch de
  getDecryptedMessage — uma DEK ilegível (MEK do ambiente diferente
  da que cifrou aquela DEK) ainda derrubava o Promise.all inteiro,
  o mesmo modo de falha que este catch existe para evitar. Movida a
  busca para dentro do try já existente.
- __tests__/integration/queries-admin.test.ts: novo caso cobrindo
  DEK ilegível (não só mensagem órfã) — cria um usuário com
  encryptedDek corrompido e confere que getAllFeedbacks degrada por
  linha em vez de rejeitar tudo.
- __tests__/integration/actions-reset-account.test.ts: novo caso
  cobrindo o guard de decryptDek em resetAccount — corrompe o
  encryptedDek do usuário e confere que o reset completa (17
  categorias recriadas) em vez de abortar sem apagar nem
  provisionar nada. Precisa ser o último it() do describe por
  corromper o encryptedDek do userId compartilhado pelo arquivo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NiDhufvRwKxUxbqNF4k3y

@Guiroos Guiroos left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Revisão do head 31f3019 (as anteriores foram no 4cac97a e no 1b814ce). Os dois nits daquela rodada estão fechados: getDekForUser entrou para dentro do try (lib/queries/admin.ts:51-53), e o guard de decryptDek ganhou o it() de integração que faltava — comentando o try/catch de reset-account.ts:79-86, decryptDek lança e o resolves.toBeUndefined() fica vermelho.

Reconferi a exigência 8 por conta própria, já que ela é o eixo do achado: das 21 pgTable do schema, a Fase 1 apaga 16; sobram users, accounts, sessions, verificationTokens e feedback, e só esta última tem coluna cifrada por DEK. Site único, fechado.

Rodei os gates nesta sessão, sobre este head: npm ci, npm run lint, npm run typecheck e npm test verdes (34 arquivos, 527 testes). Os testes de integração não rodam em PR (if: github.event_name == 'push'), como o corpo declara.

Dois achados: um bloqueante, um nit.

  1. Bloqueantelib/queries/admin.ts:57. Ao mover getDekForUser para dentro do try, o catch passou a cobrir também a falha de getMek(), e o sentinela afirma "chave rotacionada" para todas elas. Com uma ENCRYPTION_MASTER_KEY errada num deploy, /admin renderiza a lista inteira dizendo que as mensagens de todos os usuários se perderam numa rotação de chave que não houve — é o modo de falha da #35, nomeado na categoria 4 do .claude/audit.md. Verifiquei rodando que encryptDek/decryptDek estouram com MEK vazia e com 62 hex chars, antes de qualquer I/O. Correção de uma linha no inline.
  2. Nit__tests__/integration/actions-reset-account.test.ts:217. O comentário que manda manter o it() por último justifica com um dekAntiga = null que não acontece (o próprio resetAccount() reprovisiona uma DEK válida antes de terminar); o resíduo real é a linha de feedback órfã que ele deixa para trás.

Generated by Claude Code

Comment thread lib/queries/admin.ts Outdated
Comment thread __tests__/integration/actions-reset-account.test.ts Outdated
…este isolado por usuário

Endereça a revisão de 31f3019:
- lib/queries/admin.ts: mover getDekForUser para dentro do try (na
  rodada anterior) alargou o catch para cobrir também MEK ausente/
  malformada e falha de upsert — nenhuma delas é "chave rotacionada".
  Sentinela trocado para '[mensagem ilegível]', sem afirmar causa que
  o catch não sabe distinguir (categoria 4 do .claude/audit.md).
- __tests__/integration/queries-admin.test.ts: assertions atualizadas
  para o novo texto.
- __tests__/integration/actions-reset-account.test.ts: o último it()
  passou a usar um usuário próprio em vez de corromper o encryptedDek
  do userId compartilhado pelo arquivo — o comentário anterior
  justificava a ordem com um dekAntiga=null que resetAccount() nunca
  produz (a Fase 2 sempre reprovisiona DEK válida); o resíduo real
  era a linha de feedback do it() anterior ficando órfã. Isolamento
  por ID remove a dependência de ordem.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NiDhufvRwKxUxbqNF4k3y
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants