Skip to content

fix(financial): sob o bypass do RBAC, a alçada deixa de recusar quem o /me acabou de liberar - #1015

Merged
GabrielAderaldo merged 3 commits into
devfrom
fix/alcada-aprovador-sob-rbac-bypass
Sep 10, 2026
Merged

fix(financial): sob o bypass do RBAC, a alçada deixa de recusar quem o /me acabou de liberar#1015
GabrielAderaldo merged 3 commits into
devfrom
fix/alcada-aprovador-sob-rbac-bypass

Conversation

@GabrielAderaldo

@GabrielAderaldo GabrielAderaldo commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Sob AUTH_RBAC_MODE=bypass (ADR-0052) existem três camadas que decidem sobre permissão, e só duas o honravam: authorize vira no-op ✅, o GET /me anuncia o catálogo inteiro (list-user-permissions.ts:35) ✅, e a approval-policy do domínio (approval-policy.ts:27) lia canApprove do banco cru e recusava ❌.

Medido em 09/09, ambiente local: o /me de um usuário provisionado pelo ETL legado devolvia 47 permissões, incluindo payable:approve; o banco lhe dava uma, e não era essa. Ao aprovar, ele tomava um approver-missing-permission que contradizia o que o /me acabara de lhe dizer — depois de o authorize no-op o ter deixado entrar até o fundo para morrer lá. Do lado do usuário, prometer e negar a mesma coisa não se distingue de defeito.

A decisão está no ADR-0069, supersedes parcial do 0052.

O que muda

Um decorator — withRbacBypass sobre o ApproverAuthorityReader — composto num ponto só, no composition root do financial. Embutir no user-read.drizzle.ts faria o auth mentir sobre os próprios papéis para todo consumidor, inclusive a tela de gestão de acessos.

⚠️ Alcança o reader do approveDocument, e só ele (depsForApprove). O mesmo port responde a duas perguntas, e apenas a primeira é controle de acesso:

Consumidor Pergunta Sob bypass
approveDocument o chamador autenticado pode aprovar este valor? (#609) afrouxada
saveDocument · submitDraft o approverRef indicado tem alçada? (#289/#297) enforçada

A segunda é roteamento — o mesmo motivo que deixa list intacto. Compor no deps compartilhado (o caminho óbvio, e o primeiro que escrevi) afrouxa também ela, e o efeito não é simétrico: a indicação grava approverRef apontando para quem não é aprovador, e a linha sobrevive ao religar da flag. Prova invertida, medida: devolvendo o decorator ao deps, o POST /documents responde 201 com o approverRef inválido persistido, e o caso novo da borda acusa.

Limites fixados por teste

Os cinco casos de approver-authority-reader.rbac-bypass.test.ts: null continua null (approver-not-found sobrevive — o bypass afrouxa permissão, nunca a existência do sujeito); o teto passa intacto (#299/#609); falha de leitura propaga sem máscara; list passa intacto.

Na borda, o par bypass ligado × desligado sobre a mesma autoridade (canApprove: false) prova que o flag é a variável: ligado dá 200 e Approved; desligado dá 422, o documento permanece Open, e o slug interno não vaza no body.

⚠️ Custo aceito

Sob bypass, todo autenticado aprova qualquer valor. Quem não tem papel aprovador tem teto null (user-read.drizzle.ts:42), e null é SEM TETO pela regra binária do #299. "O teto continua valendo" não protege a população que esta mudança libera. Reabre, enquanto o bypass durar, o buraco que o #609 fechou — e vale em produção, com o risco assumido por escrito no #634.

Reversão custa DUAS linhas, não zero. Duas marcas ← religar no server.ts: o rbacMode fixado e o rbacBypass: true. Apagar só a primeira religa a rota e o /me e deixa a policy do domínio afrouxada — e nada mecânico acusa. O literal não deriva de rbacMode porque o ESLint recusa comparação que o compilador prova sempre verdadeira.

🔴 Risco calculado que o revisor precisa pesar antes do merge

O custo acima está escrito como "sob bypass todo autenticado aprova qualquer valor". Essa frase só descreve um risco contido se "autenticado" for um conjunto controlado — e hoje não é.

POST /api/v2/auth/register (auth/adapters/http/plugin.ts:46-74) não tem preHandler: nem requireAuth, nem authorize. E server.ts monta authHttpPlugin(authDeps) incondicionalmente, sem gate de ambiente — diferente do vanSandbox, que é fail-closed no composition root. A rota é pública onde o binário subir.

Somando com este PR: anônimo → autenticado → aprova pagamento de qualquer valor. Q.A. mediu a cadeia ponta a ponta na stack local e chegou de conta anônima a documento aprovado em 4 minutos, com as 47 permissões idênticas às do admin.

Isso é pré-existente e independente deste diff — está na dev hoje. O que este PR muda é que ele remove o último gate que ainda separava quem pode de quem não pode aprovar. Fica registrado aqui como risco calculado, com issue própria, porque consertar /register neste PR misturaria auth num diff de financial.

Achados menores também fora de escopo, com issue própria: undo-approval.ts:55 grava actor: null literal e o UndoApprovalCommand nem carrega usuário, embora req.userId esteja disponível no handler e o approve vizinho o use — PayableApproved traz o ator, ApprovalUndone vem None.

Gate

typecheck + format:check + lint + test — 11.844 testes, 0 falhas, 20 skip esperados.

Revisão: /code-review high (5 achados; os 3 de dentro do diff corrigidos aqui, ver abaixo) e /security-review (nenhum High/Medium).

Achados do code-review tratados neste PR: o decorator alcançava três call sites e não um (corrigido + teste com prova invertida); o ADR afirmava que religar não custava código (corrigido — são duas linhas); as citações server.ts:165 derivaram para :166 por causa do próprio commit e passaram a ser por marcador, não por número. O ADR-0052 ganhou o ponteiro para o 0069 — sem ele, quem abre o 0052 lê como norma corrente um alcance que já mudou, que é o defeito que a auth-module.md registra para o par 0024/0055.

O segundo commit é independente: chore(ci) fixa --test-concurrency=6 (28 CPUs para 15 GiB derrubavam a sessão gráfica; 6 workers, 2,2 GB de pico).

Refs: #634, #609, #299

https://claude.ai/code/session_01T3DKgMk376thR2ygRa3rA7

@GabrielAderaldo
fix(financial): sob o bypass do RBAC, a alçada deixa de recusar quem …
1863cc5 
…o /me acabou de liberar

Sob `AUTH_RBAC_MODE=bypass` (ADR-0052) existem **três** camadas que decidem sobre permissão, e só
duas o honravam: `authorize` vira no-op ✅, o `GET /me` anuncia o catálogo inteiro
(`list-user-permissions.ts:35`) ✅, e a `approval-policy` do domínio (`approval-policy.ts:27`) lia
`canApprove` do banco cru e recusava ❌.

**Medido em 09/09, ambiente local:** o `/me` de um usuário provisionado pelo ETL legado devolvia
**47 permissões, incluindo `payable:approve`**; o banco lhe dava **uma**, e não era essa. Ao aprovar,
ele tomava um `approver-missing-permission` que contradizia o que o `/me` acabara de lhe dizer —
depois de o `authorize` no-op o ter deixado entrar até o fundo para morrer lá. Do lado do usuário,
prometer e negar a mesma coisa não se distingue de defeito.

**A decisão está no ADR-0069**, `supersedes` parcial do 0052: sob bypass a `approval-policy` deixa de
barrar por PERMISSÃO. Aplicada como **decorator** — `withRbacBypass` sobre o
`ApproverAuthorityReader` —, composto num ponto só, no composition root do `financial`. Embutir isto
no `user-read.drizzle.ts` faria o `auth` mentir sobre os próprios papéis para **todo** consumidor,
inclusive a tela de gestão de acessos; comportamento transversal é decorator e nunca código dentro do
provedor. O `server.ts` traduz o modo num booleano — o `financial` não passa a conhecer `RbacMode`,
que é vocabulário do `auth`.

⚠️ **O decorator envolve o reader que vai ao `approveDocument`, e SÓ ele** (`depsForApprove`). O
mesmo port responde a duas perguntas, e só a primeira é controle de acesso:

- `approveDocument` — o **chamador autenticado** pode aprovar este valor? (#609) → **afrouxada**;
- `saveDocument`/`submitDraft` — o `approverRef` **indicado** tem alçada? (#289/#297) → **enforçada**.

Compor no `deps` compartilhado é o caminho óbvio, e foi o primeiro que escrevi. Afrouxa também a
segunda — e o efeito não é simétrico: a indicação **grava** `approverRef` apontando para quem não é
aprovador, e a linha **sobrevive ao religar da flag**. Seria dano que o #634 não desfaz. **Prova
invertida (medida, não suposta):** devolvendo o decorator ao `deps`, o `POST /documents` responde
**201 com o `approverRef` inválido persistido**, e o caso novo da borda acusa.

**Três limites são parte da decisão, e é por eles que ela se desfaz por engano.** Os cinco casos de
`approver-authority-reader.rbac-bypass.test.ts` existem para fixá-los:

- `null` continua `null` — o bypass afrouxa **permissão**, nunca a existência do sujeito, e
  `approver-not-found` sobrevive;
- o **teto** passa intacto — quem tem papel com alçada continua limitado por ele (#299/#609);
- `list` passa intacto — `escalate` é **roteamento de negócio**, não controle de acesso: mexer nele
  mudaria para quem o documento é encaminhado, não quem pode aprová-lo.

Na borda, o par bypass ligado × desligado sobre a **mesma** autoridade (`canApprove: false`) é o que
prova que o flag é a variável, e não outra diferença de montagem: ligado dá 200 e `Approved`;
desligado dá 422, o documento permanece `Open` e o slug interno não vaza no body.

⚠️ **O custo aceito, e é o principal: sob bypass todo autenticado aprova qualquer valor.** Quem não
tem papel aprovador tem teto `null` — `maxLimit` devolve `null` para conjunto vazio
(`user-read.drizzle.ts:42`) — e `null` é **SEM TETO** pela regra binária do #299
(`approval-policy.ts:28-31`). Então "o teto continua valendo" **não protege** exatamente a população
que esta mudança libera. Reabre, enquanto o bypass durar, o buraco que o #609 fechou — e **vale em
produção**, porque `server.ts:165` fixa o modo por código, com o risco assumido por escrito no #634.

**O gatilho de reversão é o #634 — e ele custa DUAS linhas, não zero.** São duas marcas `← religar`
no `server.ts`: o `rbacMode` fixado e o `rbacBypass: true` da composição do `financial`. Apagar só a
primeira religa a rota e o `/me` e **deixa a policy do domínio afrouxada**, com o RBAC já enforçado —
o pior dos dois mundos, e **nada mecânico acusa**: não há erro de tipo, teste vermelho nem lint. O
literal não deriva de `rbacMode` porque o ESLint recusa comparação que o compilador prova sempre
verdadeira (a mesma razão que impede envolver o banner num `if`); o preço é a nota, que é o único
guarda existente. O ADR-0052 ganha o ponteiro para cá — sem ele, quem abrir o 0052 lê como norma
corrente um alcance que já mudou, que é o defeito que a `auth-module.md` já registra para o par
0024/0055.

As citações passam a ser por **marcador**, não por número de linha: a primeira versão deste ADR dizia
`server.ts:165`, e as duas linhas que este mesmo commit acrescentou empurraram o alvo para a `:166` —
a `:165` virou justamente o `resolveRbacMode` que a instrução manda voltar a usar. Quem seguisse a
citação apagaria a leitura da env e deixaria o hardcode de pé.

**Alternativa recusada pelo dono:** consertar o `/me`, subtraindo `payable:approve` do catálogo
anunciado sob bypass e mantendo a policy a barrar. Também elimina a contradição, e tinha precedente
no próprio repositório (`revoke-role.ts:88` chama `authorize` direto para sobreviver ao bypass). Foi
recusada porque, sob bypass, a promessa é *"todo autenticado é super-usuário"*, e uma regra de
domínio que segue cobrando permissão é **exceção** à promessa, não correção dela. A análise ficou
escrita no ADR para quem for reabrir.

⚠️ Esta decisão **não alcança** outras policies de domínio que leiam permissão do banco. Hoje esta é
a única identificada, e o `financial` já recebe o booleano — seguir é barato, mas é deliberado.

Gate verde: typecheck + format:check + lint + test (11.844 testes, 0 falhas, 20 skip esperados).

Refs: #634, #609, #299

Assisted-by: Claude-Code:claude-opus-5
Claude-Session: https://claude.ai/code/session_01T3DKgMk376thR2ygRa3rA7
@GabrielAderaldo
chore(ci): a suíte de unidade fixa a concorrência em 6 processos
52b7d52 
`node --test` sem `--test-concurrency` dimensiona o paralelismo por `os.availableParallelism()`. Na
máquina de desenvolvimento isso devolve **28**, contra **15 GiB** de RAM — e cada worker do runner
custa ~370 MB medidos (Node completo com `--experimental-strip-types` + `--enable-source-maps`). O
default pedia ~10,4 GB num box que já roda a sessão gráfica: o gate empurrava desktop, `mysqld` e
navegador para a swap de 4 GiB, que não voltava.

Não é micro-otimização de estilo. `pnpm test` é chamado pelo hook `Stop`
(`stop-quality-gate.sh:97`) e pelo pre-commit, então o pico acontece a **cada turno encerrado**, não
só quando alguém roda a suíte à mão.

Medido depois do cap: 6 workers, **2,2 GB de pico**, ~155s, suíte verde.

O cap não muda o que é executado, só quantos arquivos concorrem. No CI (`ubuntu-latest`, 4 vCPU /
16 GiB) ele **sobe** a concorrência de 4 para 6, a ~2,2 GB de pico — folgado dentro do orçamento do
runner. A suíte de integração continua em `--test-concurrency=1`, por outro motivo: isolamento de
banco (`.claude/rules/testing.md`).

⚠️ Não elevar nem remover "para acelerar" sem antes medir `free -h` contra `nproc`. A razão RAM/core
aqui é ~0,54 GB e o worker quer 0,37 GB. Suíte lenta demais se resolve reduzindo o custo por worker
ou dividindo a suíte, não soltando o paralelismo.

Assisted-by: Claude-Code:claude-opus-5
Claude-Session: https://claude.ai/code/session_01T3DKgMk376thR2ygRa3rA7
Copilot AI lite review requested due to automatic review settings September 10, 2026 01:59
Copilot AI reviewed Sep 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This was referenced Sep 10, 2026
Open
Open
@GabrielAderaldo

GabrielAderaldo commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Os dois achados fora do escopo deste PR, registrados em vez de consertados aqui:

Consertar qualquer um deles aqui misturaria auth num diff de financial.

@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@GabrielAderaldo
chore(deps): nodemailer sai da 9.0.1, que o audit de produção passou …
37575b5 
…a recusar

GHSA-2x7j-588g-ccc2 (`high`, CVSS 7.5 `AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H`) foi publicada em
**08/09/2026 21:33 UTC**: complexidade quadrática no `addressparser` do nodemailer permite DoS
remoto por lista de endereços forjada. Vulneráveis `<9.1.0`; corrigido em `>=9.1.0`.

O pin era `9.0.1` (`package.json:124`, de c66ff54), e nodemailer é `dependencies` — então quem
acusa é o job **bloqueante** `audit (produção — blocking)`, não o report-only:

    pnpm audit --prod --audit-level=high --ignore-registry-errors  →  exit 1
    │ Paths │ .>nodemailer │      4 vulnerabilities found · 3 moderate | 1 high

O vermelho não veio de diff nenhum: é o relógio. A advisory saiu 28h antes do run do PR #1015, e a
`dev` só está verde porque o último `audit` dela é de **06/09** — dois dias antes de a advisory
existir. Qualquer branch que rodar o gate a partir de agora pega o mesmo exit 1.

Sobe para **9.1.1**, a última da linha 9.x (publicada em 01/09, fora da quarentena de
`minimumReleaseAge: 1440`). A `10.0.2` existe, mas saiu em **09/09 20:19 UTC** — major de menos de
24h, que a quarentena recusaria de todo modo, e que exigiria revisar a API antes: o adapter em
`src/modules/notifications/adapters/email/nodemailer.ts:15-17` importa `createTransport` mais os
tipos de `nodemailer/lib/smtp-pool` e `smtp-transport`, caminhos internos que major move sem aviso.

Pin exato porque `dependencies` carrega o que serve tráfego (`.claude/rules/supply-chain.md`), e a
graça do pin é exatamente esta: subir de versão vira ato deliberado, visível em diff de PR.

Os demais `high` do relatório — axios, form-data, undici — entram por `@usebruno/cli`, que é
devDependency; ficam no job report-only, por desenho, e este commit não os toca.

Verificado: `pnpm audit --prod --audit-level=high` responde `No known vulnerabilities found`.
Gate completo verde — typecheck, format:check, lint e 11.844 testes, 0 falhas, 20 skip esperados.

Assisted-by: Claude-Code:claude-opus-5
Claude-Session: https://claude.ai/code/session_01G17M4M3A3c6bd6jmCPAk2W
@GabrielAderaldo
GabrielAderaldo merged commit 72cf83a into dev Sep 10, 2026
20 checks passed
This was referenced Sep 10, 2026
Merged
Merged
@GabrielAderaldo
GabrielAderaldo deleted the fix/alcada-aprovador-sob-rbac-bypass branch October 5, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@GabrielAderaldo