From 0099f250c2d166e96495f47cd94c02fdd8f0bd76 Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Wed, 26 Aug 2026 13:38:26 -0300 Subject: [PATCH] fix(auth): close Anthropic OAuth review gaps --- ANTHROPIC_ISSUES.md | 94 ++++--- RELATORIO_ANTHROPIC_OAUTH_BUGS.md | 66 ++--- agent/anthropic_adapter.py | 132 +++++++--- agent/credential_pool.py | 18 ++ hermes_cli/web_server.py | 36 +-- tests/agent/test_anthropic_keychain.py | 88 ++++++- tests/agent/test_anthropic_oauth_stress.py | 238 +++++++++++++++++- ..._credential_pool_anthropic_refresh_race.py | 25 +- ...test_credential_pool_oauth_writethrough.py | 110 ++++++++ .../test_auth_store_lock_concurrent.py | 1 + tests/hermes_cli/test_web_oauth_dispatch.py | 100 +++++++- web/src/lib/api.test.ts | 29 ++- web/src/lib/api.ts | 4 + 13 files changed, 788 insertions(+), 153 deletions(-) diff --git a/ANTHROPIC_ISSUES.md b/ANTHROPIC_ISSUES.md index 05c692c188..b0293ed9e4 100644 --- a/ANTHROPIC_ISSUES.md +++ b/ANTHROPIC_ISSUES.md @@ -1,12 +1,14 @@ # Anthropic OAuth — Issues encontrados no hermes-agent -> **Atualização:** todos os 5 bugs abaixo (incluindo o Bug 5, achado pelo teste de esforço) foram **corrigidos** nesta sessão. +> **Estado atual do PR #87891:** os achados abaixo são históricos e devem ser lidos junto com o estado atual. O fluxo OAuth Anthropic do dashboard foi removido, não corrigido em linha: o catálogo expõe `flow: "external"` e as rotas `start`/`submit` rejeitam o fluxo. O fluxo interativo de terminal (`hermes auth add anthropic`) permanece explicitamente fora do escopo desta remoção. > Issues: [#87887](https://github.com/NousResearch/hermes-agent/issues/87887) (bugs 1-2) · [#87888](https://github.com/NousResearch/hermes-agent/issues/87888) (bugs 3-4) · [#87889](https://github.com/NousResearch/hermes-agent/issues/87889) (bug 5). > -> **Atualização 2 (review do PR #87891):** em vez de manter os patches dos bugs 1, 2 e 4 abaixo, o fluxo de login OAuth do **dashboard web** (`_start_anthropic_pkce` / `_submit_anthropic_pkce` / `_save_anthropic_oauth_creds` em `hermes_cli/web_server.py`) foi **removido por completo**. Um endpoint HTTP não-supervisionado emitindo tokens de assinatura Claude Pro/Max fora do cliente oficial da Anthropic esbarra nas políticas de uso da Anthropic para credenciais OAuth, então a correção mais segura é eliminar essa superfície em vez de só corrigi-la. O catálogo de providers do dashboard agora marca `anthropic` como `flow: "external"`, apontando para `hermes auth add anthropic` (o fluxo PKCE via terminal, fora do escopo desta mudança e não afetado). Bugs 3 e 5 (race condition no refresh cross-processo e o `PermissionError` no lock do Windows) continuam corrigidos como descrito abaixo — são bugs do fluxo de login `claude_code`/CLI, que permanece. +> **Decisão de segurança:** um endpoint HTTP não-supervisionado emitindo tokens de assinatura Claude Pro/Max fora do cliente oficial da Anthropic fica fora da política aceita para este produto. A correção é eliminar essa superfície, não mantê-la com patches de CSRF ou de persistência. +> +> **Controles que permanecem:** a serialização de refresh inclui `anthropic`; a fonte compartilhada `claude_code` usa um lock baseado no arquivo compartilhado tanto no `CredentialPool` quanto no resolver direto; o write-through de `hermes_pkce` atualiza `.anthropic_oauth.json`; e o lock Windows trata a inicialização concorrente do arquivo. A cobertura atual inclui teste com processos independentes e perfis distintos. **Data:** 2026-08-16 -**Branch:** `fix/update-orphan-history-guard-87694` +**Branch do PR:** `fix/anthropic-oauth-csrf-race-apikey-shadow` **Pedido original:** investigar se o OAuth da Anthropic tem bug real — rotas, race conditions, zombie processes, segurança — e provar com testes. --- @@ -15,34 +17,32 @@ **Sim, funciona.** Nenhum dos bugs encontrados impede login ou uso normal. São dois problemas distintos, ambos reais e comprovados por teste, mas nenhum é "quebrado/não autentica": -| Fluxo de login | Afetado por quê | +| Fluxo de login | Estado no PR #87891 | |---|---| -| `claude setup-token` (CLI oficial Anthropic, credencial lida de `~/.claude/.credentials.json`, `source="claude_code"`) | Nenhum dos bugs 1-3 — é o único caminho protegido nos dois casos | -| `hermes model` no terminal, opção 1 (OAuth via CLI) | Bug de race condition (só sob múltiplos processos Hermes concorrentes); limpa `ANTHROPIC_API_KEY` corretamente, então **não** sofre o Bug 4 | -| Login via **dashboard web** do Hermes | Bug de segurança (CSRF/PKCE leak, bugs 1-2) + race condition (bug 3) + **Bug 4: não limpa `ANTHROPIC_API_KEY` antiga, que continua vencendo o OAuth** | +| `claude setup-token` (CLI oficial Anthropic, credencial lida de `~/.claude/.credentials.json`, `source="claude_code"`) | Permanece disponível; o refresh compartilhado é serializado e revalidado | +| `hermes auth add anthropic` no terminal | Permanece disponível e escreve a credencial Hermes; o refresh de `hermes_pkce` tem write-through | +| Login Anthropic no **dashboard web** do Hermes | Removido; o catálogo retorna `flow: "external"` e não há emissão de tokens por HTTP | -**Atualização:** o sintoma relatado pelo usuário ("configuro com OAuth e o Hermes usa API key mesmo assim") foi diagnosticado como o **Bug 4** — ver seção dedicada abaixo. +**Nota sobre o sintoma original:** o caminho dashboard que podia deixar uma API key antiga sombrear o OAuth não existe mais. A prioridade de uma API key explícita na resolução continua sendo deliberada para credenciais configuradas pelo usuário. -- **Bug de segurança:** não impede login, mas expõe o fluxo do dashboard a um vetor de CSRF já corrigido no CLI e nunca replicado lá. +- **Bug de segurança histórico:** não impede login, mas expunha o fluxo do dashboard a um vetor de CSRF. A superfície HTTP foi removida no PR; o fluxo de terminal continua separado. - **Bug de race condition:** só aparece com **múltiplos processos Hermes rodando ao mesmo tempo** (fleet workers, cron + sessão interativa) disputando o refresh do mesmo token no exato instante em que ele expira. Uso single-process (maioria dos usuários, a maior parte do tempo) nunca bate nisso. --- ## O que foi feito nesta investigação -1. Localizado o código real do fluxo Anthropic (não documentação): `agent/anthropic_adapter.py`, `agent/credential_pool.py`, `hermes_cli/auth.py`, `hermes_cli/auth_commands.py`, `hermes_cli/web_server.py`. -2. Comparado o fluxo de login via CLI (`run_hermes_oauth_login_pure`) com o fluxo paralelo do dashboard web (`_start_anthropic_pkce` / `_submit_anthropic_pkce`) — achado o bug de CSRF por essa comparação. -3. Comparado a proteção cross-processo que Codex/xAI recebem no refresh (`_auth_store_lock`) com o que a Anthropic recebe — achado o bug de race condition por essa comparação. -4. Escritos dois arquivos de teste novos que **provam** os bugs (testes que devem falhar contra o código atual, e falham): - - `tests/hermes_cli/test_anthropic_dashboard_pkce_csrf.py` (5 testes) - - `tests/agent/test_credential_pool_anthropic_refresh_race.py` (3 testes) -5. Rodado `pytest` nos dois arquivos: **7 failed, 1 passed** — confirma os bugs objetivamente (não é opinião). +1. Localizado o código real nos módulos `agent/anthropic_adapter.py`, `agent/credential_pool.py`, `hermes_cli/auth.py`, `hermes_cli/auth_commands.py` e `hermes_cli/web_server.py`. +2. Comparado o fluxo interativo de terminal com a implementação paralela do dashboard e confirmado que os bugs de CSRF/PKCE pertenciam à superfície HTTP removida. +3. Comparado o lock cross-processo de Codex/xAI com a Anthropic e alinhado a autoridade do lock à fonte real: `auth.json` por perfil e `~/.claude/.credentials.json` para `claude_code`. +4. Mantida a regressão de refresh em `tests/agent/test_credential_pool_anthropic_refresh_race.py`, ampliada a carga em `tests/agent/test_anthropic_oauth_stress.py` e coberto o lock Windows em `tests/hermes_cli/test_auth_store_lock_concurrent.py`. +5. Adicionada cobertura de dispatcher para provar que o dashboard não cria sessão nem emite URL/código OAuth Anthropic. 6. Investigado zombie process especificamente no fluxo Anthropic — **não encontrado**. -7. Nenhum código de produção foi alterado — só testes de regressão + este relatório. +7. A documentação desta investigação foi atualizada para refletir a remoção do fluxo dashboard e os testes atuais; não representa uma reprodução contra o código atual quando descreve o estado anterior. --- -## Bug 1 — PKCE `code_verifier` vazado como `state` (só no dashboard web) +## Bug 1 (histórico) — PKCE `code_verifier` vazado como `state` (só no dashboard web) **Onde:** `hermes_cli/web_server.py`, função `_start_anthropic_pkce()` (~linha 10637-10661) @@ -59,7 +59,7 @@ params = { } ``` -Isso é **exatamente** o bug que já existiu no fluxo de CLI e foi corrigido (histórico documentado em `tests/agent/test_anthropic_oauth_pkce.py`: PR #1775 corrigiu → PR #2647 reintroduziu → PR #3107 removeu a função antiga → PR #10699/issue #10693 corrigiu de vez na função sobrevivente). O CLI (`agent/anthropic_adapter.py::run_hermes_oauth_login_pure`) gera um `state` independente (`secrets.token_urlsafe(32)`). O **dashboard web nunca recebeu essa correção** — é uma implementação paralela que reintroduz o problema numa rota diferente. +Isso era **exatamente** o bug que já existiu no fluxo de CLI e foi corrigido (histórico documentado em `tests/agent/test_anthropic_oauth_pkce.py`: PR #1775 corrigiu → PR #2647 reintroduziu → PR #3107 removeu a função antiga → PR #10699/issue #10693 corrigiu de vez na função sobrevivente). O CLI (`agent/anthropic_adapter.py::run_hermes_oauth_login_pure`) gera um `state` independente (`secrets.token_urlsafe(32)`). O dashboard web tinha uma implementação paralela; no PR #87891 essa implementação foi removida, portanto o código vulnerável abaixo não é mais um call path ativo. **Consequência:** o `code_verifier` (que por RFC 7636 §7.2 deve ficar confidencial) vaza via histórico do navegador, cabeçalho `Referer`, e logs de acesso da Anthropic (`platform.claude.com`). @@ -69,11 +69,11 @@ assert 'dg7ihLbmv6ooNu2-V_dKcVPKp29kNJewr4s8UBSr1gE' != 'dg7ihLbmv6ooNu2-V_dKcVP ``` state e verifier são literalmente o mesmo valor. -**Status:** ✅ **corrigido** — `_start_anthropic_pkce()` agora gera `state` independente via `secrets.token_urlsafe(32)`. +**Status no PR:** ✅ **neutralizado por remoção** — `_start_anthropic_pkce()` não existe mais e o catálogo Anthropic é `flow: "external"`. O teste atual verifica que a rota `start` rejeita o fluxo, em vez de testar uma função removida. --- -## Bug 2 — Callback do dashboard nunca valida o `state` (zero proteção CSRF) +## Bug 2 (histórico) — Callback do dashboard nunca valida o `state` (zero proteção CSRF) **Onde:** `hermes_cli/web_server.py`, função `_submit_anthropic_pkce()` (~linha 10664-10692) @@ -86,11 +86,11 @@ exchange_data = json.dumps({ }).encode() ``` -O código nunca faz `if state_from_callback != sess["state"]: reject`. Qualquer valor (ou nenhum) é aceito e a troca de token prossegue. O CLI faz essa checagem (`if received_state != oauth_state: abort`); o dashboard não faz nenhuma. +O código antigo nunca fazia `if state_from_callback != sess["state"]: reject`. Qualquer valor (ou nenhum) era aceito e a troca de token prosseguia. O CLI faz essa checagem (`if received_state != oauth_state: abort`); o dashboard não tem mais esse endpoint. **Evidência (teste real rodado):** payload POST real capturado mesmo com `state` adulterado (`attacker-controlled-state`) — a troca de token aconteceu do mesmo jeito. -**Status:** ✅ **corrigido** — `_submit_anthropic_pkce()` agora rejeita a troca de token quando o `state` não bate. +**Status no PR:** ✅ **neutralizado por remoção** — `_submit_anthropic_pkce()` não existe mais; a rota `submit` genérica rejeita o provider Anthropic e não cria nem completa uma sessão OAuth. --- @@ -114,7 +114,7 @@ O único caminho de recuperação em caso de falha (`_sync_anthropic_entry_from_ if self.provider != "anthropic" or entry.source != "claude_code": return entry ``` -Credenciais de login nativo do Hermes (`hermes_pkce`) ou do dashboard (`manual:dashboard_pkce`) **não têm recuperação nenhuma**. Se perdem a corrida, caem em `self._mark_exhausted(entry, None)` (linha 1705) — mesmo com um token válido existindo em disco, escrito pelo processo que ganhou. +Credenciais de login nativo do Hermes (`manual:hermes_pkce`, gravadas em `~/.hermes/.anthropic_oauth.json`, e a forma sem prefixo usada pelo seeding) ou as antigas emitidas pelo dashboard (`manual:dashboard_pkce`) **não tinham recuperação nenhuma**. Ao perder a corrida, o processo caía em `self._mark_exhausted(entry, None)` — mesmo com um token válido existindo em disco, escrito pelo processo vencedor. **Cenário real (não ataque, uso normal):** dois processos Hermes concorrentes (ex: fleet worker + sessão CLI, ou dois cron jobs) compartilhando a mesma conta Anthropic via login nativo. Token expira; ambos tentam refresh ao mesmo tempo; servidor da Anthropic aceita só o primeiro; o segundo recebe `invalid_grant` e fica marcado como exhausted — pedindo reautenticação mesmo com credencial válida disponível. @@ -130,7 +130,7 @@ PASSED test_concurrent_claude_code_refresh_recovers_via_credentials_file # contraste: fonte 'claude_code' SE recupera — prova a assimetria ``` -**Status:** ✅ **corrigido**. `"anthropic"` foi incluído na tupla protegida por `_auth_store_lock`, e um novo método `_sync_anthropic_entry_from_pool_store()` (espelhando o padrão já usado pelo xAI) resincroniza a partir do próprio credential-pool store — funciona pra **todas** as fontes (`claude_code`, `hermes_pkce`, `manual:dashboard_pkce`), não só `claude_code`. +**Status no PR:** ✅ **corrigido para os fluxos que permanecem**. `"anthropic"` foi incluído na tupla protegida por `_auth_store_lock`; `claude_code` também usa um lock keyed ao arquivo compartilhado `~/.claude/.credentials.json`; e `_sync_anthropic_entry_from_pool_store()` cobre fontes persistidas do pool. O dashboard `manual:dashboard_pkce` foi removido, não é mais uma fonte emitida. --- @@ -152,7 +152,7 @@ Ordem de prioridade na resolução de credencial: O comentário do próprio código é explícito sobre a intenção: "An explicit user-configured key must not be shadowed by auto-discovered [...] credential-pool OAuth credentials." — ou seja, se `ANTHROPIC_API_KEY` estiver preenchida (mesmo que antiga/esquecida), ela sempre ganha da credencial OAuth do pool (prioridade 3 bate prioridade 5), mesmo depois de um login OAuth bem-sucedido. -**A causa raiz específica:** o fluxo de login OAuth do **dashboard web** (`hermes_cli/web_server.py::_save_anthropic_oauth_creds`) grava a credencial no credential pool, mas **nunca limpa** a variável `ANTHROPIC_API_KEY`. Isso é diferente do fluxo de OAuth via **CLI** (`hermes model` → opção 1 → `save_anthropic_oauth_token()`), que limpa a API key antiga automaticamente ao salvar o token OAuth. Resultado: se em qualquer momento anterior uma `ANTHROPIC_API_KEY` ficou configurada (setup antigo, teste, auto-detecção), fazer login OAuth pelo dashboard não a remove, e ela continua vencendo pra sempre na resolução de credencial. +**A causa raiz específica:** o fluxo de login OAuth do **dashboard web** (`hermes_cli/web_server.py::_save_anthropic_oauth_creds`) gravava a credencial no credential pool, mas **não limpava** a variável `ANTHROPIC_API_KEY`. Esse fluxo foi removido no PR. O fluxo de OAuth via **CLI** (`hermes model` → opção 1 → `save_anthropic_oauth_token()`), que limpa a API key antiga automaticamente ao salvar o token OAuth, é separado e permanece fora do escopo. **Correção recomendada:** em `_save_anthropic_oauth_creds()` (web_server.py), limpar `ANTHROPIC_API_KEY` do mesmo jeito que `save_anthropic_oauth_token()` já faz no fluxo de CLI — ou, na resolução (`resolve_anthropic_token`), preferir a credencial OAuth mais recente quando o usuário acabou de completar um login explícito, em vez de uma API key estática que pode estar obsoleta. @@ -163,13 +163,13 @@ hermes doctor ``` Se estiver preenchida, é essa a causa. Correção rápida: esvaziar `ANTHROPIC_API_KEY` no `.env` do Hermes, ou refazer o login OAuth pela opção 1 do `hermes model` no terminal (que já limpa corretamente). -**Status:** ✅ **corrigido**. `_save_anthropic_oauth_creds()` agora limpa `ANTHROPIC_API_KEY` ao salvar o login OAuth do dashboard, igual o fluxo de CLI já fazia. +**Status no PR:** ✅ **neutralizado por remoção** — não existe mais `_save_anthropic_oauth_creds()` nem login Anthropic no dashboard. A prioridade de `ANTHROPIC_API_KEY` na resolução continua deliberada para uma chave explicitamente configurada. --- ## Bug 5 — PermissionError sob concorrência real no lock cross-processo (achado pelo teste de esforço) -**Como foi achado:** ao escrever um teste de carga (`tests/agent/test_anthropic_oauth_stress.py`) simulando 20 "processos Hermes" concorrentes disputando o refresh (necessário pra validar o Bug 3 sob estresse, não só com 2 threads), o teste **falhou de verdade** — 16 de 20 threads levantaram `PermissionError: [Errno 13] Permission denied` vindo de dentro do próprio `_auth_store_lock()`. +**Como foi achado:** ao escrever um teste de carga (`tests/agent/test_anthropic_oauth_stress.py`) simulando concorrência real no refresh (necessário pra validar o Bug 3 sob estresse, não só com uma chamada isolada), a inicialização do lock no Windows podia levantar `PermissionError: [Errno 13] Permission denied` vindo de dentro do próprio `_auth_store_lock()`. **Causa raiz:** `hermes_cli/auth.py::_file_lock()` — a checagem "garante que o arquivo de lock tem pelo menos 1 byte" (necessária pro `msvcrt.locking()` do Windows) fazia um `write_text()` **sem tratamento de exceção**, fora do loop de retry que existe logo depois: @@ -191,7 +191,7 @@ if msvcrt and (not lock_path.exists() or lock_path.stat().st_size == 0): pass # outro holder já garantiu conteúdo; segue pro loop de retry ``` -**Status:** ✅ **corrigido**. Testes: `tests/agent/test_anthropic_oauth_stress.py` (reproduz de forma confiável, 16/20 falhas antes do fix, 0 depois) e `tests/hermes_cli/test_auth_store_lock_concurrent.py` (cobertura dedicada e genérica do lock, independente da Anthropic). +**Status no PR:** ✅ **corrigido**. O write de inicialização do lock é best-effort e qualquer contenção segue para o loop de retry. A cobertura do lock é `windows_only`, e o teste de refresh com perfis distintos usa processos independentes e exige exatamente um POST do refresh token compartilhado. --- @@ -206,31 +206,43 @@ Checado especificamente no fluxo Anthropic: --- -## Arquivos criados nesta investigação +## Arquivos e cobertura atuais do PR | Arquivo | O que é | |---|---| -| `tests/hermes_cli/test_anthropic_dashboard_pkce_csrf.py` | 5 testes provando bugs 1 e 2 | -| `tests/agent/test_credential_pool_anthropic_refresh_race.py` | 3 testes provando bug 3 | -| `RELATORIO_ANTHROPIC_OAUTH_BUGS.md` | Relatório técnico detalhado (versão anterior desta investigação, bugs 1-3) | -| `ANTHROPIC_ISSUES.md` | Este arquivo — resumo consolidado, inclui bug 4 (causa real do "não funciona" relatado) | +| `tests/hermes_cli/test_web_oauth_dispatch.py` | Dispatcher: Anthropic external; `start`/`submit` dashboard rejeitados | +| `tests/agent/test_credential_pool_anthropic_refresh_race.py` | 3 testes de lock/sincronização Anthropic | +| `tests/agent/test_anthropic_oauth_stress.py` | Carga em threads + processo cross-profile com POST único | +| `tests/agent/test_anthropic_keychain.py` | Resolver direto Claude Code sob lock compartilhado | +| `tests/hermes_cli/test_auth_store_lock_concurrent.py` | Concorrência do lock real, marcada `windows_only` | +| `tests/agent/test_credential_pool_oauth_writethrough.py` | Rotação `hermes_pkce` preservada no `.anthropic_oauth.json` | +| `RELATORIO_ANTHROPIC_OAUTH_BUGS.md` | Relatório técnico histórico e estado da implementação | +| `ANTHROPIC_ISSUES.md` | Este arquivo — resumo consolidado e matriz de escopo | **Nota sobre dados sensíveis:** este relatório foi verificado com busca por regex de chaves reais (`sk-ant-...`, `ANTHROPIC_API_KEY=`, `ANTHROPIC_TOKEN=`) — nenhuma encontrada. Só há valores sintéticos usados nos testes automatizados (ex: `sk-ant-oat-rotated-1`), nunca uma chave real. O diagnóstico do Bug 4 pede pro usuário rodar `hermes doctor` / checar o próprio `.env` localmente, mas o resultado desse comando **não foi colado neste relatório** — só a explicação da causa raiz no código. -Nenhum arquivo de produção foi alterado. +Arquivos de produção alterados pelo PR: `agent/anthropic_adapter.py`, `agent/credential_pool.py`, `hermes_cli/auth.py` e `hermes_cli/web_server.py`. O frontend também recebe a correção de escopo em `web/src/lib/api.ts` para que operações OAuth sigam o perfil selecionado. -## Como reproduzir +## Como validar o estado atual ```bash -python -m pytest tests/hermes_cli/test_anthropic_dashboard_pkce_csrf.py tests/agent/test_credential_pool_anthropic_refresh_race.py -v +scripts/run_tests.sh tests/hermes_cli/test_web_oauth_dispatch.py tests/agent/test_credential_pool_anthropic_refresh_race.py tests/agent/test_anthropic_oauth_stress.py tests/agent/test_credential_pool_oauth_writethrough.py tests/agent/test_anthropic_keychain.py tests/hermes_cli/test_auth_store_lock_concurrent.py -q ``` -Resultado com o código atual (não corrigido): **7 failed, 1 passed**. +Para a cobertura frontend de perfil OAuth: + +```bash +cd web +npx vitest run src/lib/api.test.ts +``` + +Os resultados dependem do ambiente e devem ser registrados no PR junto com a cabeça testada; os blocos históricos acima não são resultados do código atual. ## Referência rápida de linhas | Arquivo | Linhas | |---|---| -| `hermes_cli/web_server.py` | 10637-10661 (`_start_anthropic_pkce`), 10664-10728 (`_submit_anthropic_pkce`) | -| `agent/credential_pool.py` | 1307-1344 (`_refresh_entry`), 855-905 (`_sync_anthropic_entry_from_credentials_file`), 1442-1483 (recuperação), 1705 (`_mark_exhausted`) | -| `agent/anthropic_adapter.py` | 1125-1186 (`refresh_anthropic_oauth_pure`), 1189-1239 (`_refresh_oauth_token`), 1531-1658 (`run_hermes_oauth_login_pure`, fluxo CLI correto) | +| `hermes_cli/web_server.py` | Catálogo `anthropic` como `flow: "external"`; dispatcher rejeita `start`/`submit` | +| `agent/credential_pool.py` | `_refresh_entry`, `_sync_anthropic_entry_from_pool_store`, lock compartilhado Claude Code e fallback de recuperação | +| `agent/anthropic_adapter.py` | `claude_code_credentials_path`, refresh puro, fluxo CLI PKCE separado e write-through Hermes | +| `web/src/lib/api.ts` | `/api/providers/oauth` incluído no escopo de perfil do dashboard | diff --git a/RELATORIO_ANTHROPIC_OAUTH_BUGS.md b/RELATORIO_ANTHROPIC_OAUTH_BUGS.md index 460c78b8e8..ed18f221a4 100644 --- a/RELATORIO_ANTHROPIC_OAUTH_BUGS.md +++ b/RELATORIO_ANTHROPIC_OAUTH_BUGS.md @@ -1,15 +1,17 @@ # Relatório de Investigação — Bugs no fluxo OAuth da Anthropic (hermes-agent) -> **Atualização (review do PR #87891):** o fluxo de login OAuth do **dashboard web** (`_start_anthropic_pkce` / `_submit_anthropic_pkce` / `_save_anthropic_oauth_creds` em `hermes_cli/web_server.py`) foi **removido por completo**, em vez de mantido com os patches dos bugs 1, 2 e 4 descritos abaixo. Um endpoint HTTP não-supervisionado emitindo tokens de assinatura Claude Pro/Max fora do cliente oficial da Anthropic esbarra nas políticas de uso da Anthropic para credenciais OAuth. O catálogo de providers do dashboard agora marca `anthropic` como `flow: "external"`, apontando para `hermes auth add anthropic` (fluxo PKCE via terminal, fora do escopo desta mudança). Os testes `tests/hermes_cli/test_anthropic_dashboard_pkce_csrf.py` e `tests/hermes_cli/test_web_server_oauth_write.py` foram removidos por testarem exclusivamente código agora inexistente. Bugs 3 e 5 (race condition no refresh cross-processo e o `PermissionError` no lock do Windows) seguem corrigidos como descrito abaixo — afetam o fluxo de login `claude_code`/CLI, que permanece. +> **Estado atual do PR #87891:** este arquivo preserva os achados contra a implementação anterior. O fluxo de login OAuth do **dashboard web** (`_start_anthropic_pkce` / `_submit_anthropic_pkce` / `_save_anthropic_oauth_creds`) foi removido por completo, em vez de mantido com patches de CSRF ou de shadowing. Um endpoint HTTP não-supervisionado emitindo tokens de assinatura Claude Pro/Max fora do cliente oficial da Anthropic fica fora da política aceita para este produto. O catálogo marca `anthropic` como `flow: "external"`, e as rotas `start`/`submit` rejeitam esse fluxo. O fluxo PKCE interativo de terminal (`hermes auth add anthropic`) permanece explicitamente fora do escopo desta remoção. +> +> Os testes `tests/hermes_cli/test_anthropic_dashboard_pkce_csrf.py` e `tests/hermes_cli/test_web_server_oauth_write.py` foram removidos por testarem exclusivamente código inexistente na cabeça atual. Bugs 3 e 5 continuam cobertos: refresh Anthropic com lock cross-processo, lock adicional para o arquivo compartilhado `claude_code` (inclusive no resolver direto), write-through de `hermes_pkce`, e cobertura nativa Windows. -**Data:** 2026-08-16 -**Branch:** `fix/update-orphan-history-guard-87694` +**Data do achado original:** 2026-08-16 +**Branch do PR:** `fix/anthropic-oauth-csrf-race-apikey-shadow` **Escopo investigado:** `agent/anthropic_adapter.py`, `agent/credential_pool.py`, `hermes_cli/auth.py`, `hermes_cli/auth_commands.py`, `hermes_cli/web_server.py` -**Testes novos:** `tests/hermes_cli/test_anthropic_dashboard_pkce_csrf.py`, `tests/agent/test_credential_pool_anthropic_refresh_race.py` +**Cobertura atual:** `tests/hermes_cli/test_web_oauth_dispatch.py`, `tests/agent/test_credential_pool_anthropic_refresh_race.py`, `tests/agent/test_anthropic_oauth_stress.py`, `tests/agent/test_credential_pool_oauth_writethrough.py`, `tests/agent/test_anthropic_keychain.py`, `tests/hermes_cli/test_auth_store_lock_concurrent.py` e `web/src/lib/api.test.ts` ## Resumo executivo -Foram encontrados **dois bugs reais e reproduzíveis** no fluxo OAuth da Anthropic, ambos confirmados por testes automatizados que falham contra o código atual (evidência objetiva, não especulação): +Foram encontrados **dois bugs reais e reproduzíveis** na implementação anterior do fluxo OAuth da Anthropic, ambos confirmados por testes automatizados que falhavam contra aquela versão (evidência objetiva, não especulação): | # | Bug | Categoria | Severidade | |---|-----|-----------|------------| @@ -21,7 +23,7 @@ Não foi encontrada evidência de **zombie processes** no fluxo Anthropic especi --- -## 1. Bug de segurança — PKCE `code_verifier` vazado como `state` (dashboard web) +## 1. Bug histórico de segurança — PKCE `code_verifier` vazado como `state` (dashboard web) ### Onde @@ -88,7 +90,7 @@ params = {..., "state": oauth_state} --- -## 2. Bug de segurança — ausência total de validação de `state` no callback do dashboard +## 2. Bug histórico de segurança — ausência total de validação de `state` no callback do dashboard ### Onde @@ -148,7 +150,9 @@ if not state_from_callback or state_from_callback != sess["state"]: ### Onde -`agent/credential_pool.py`, método `CredentialPool._refresh_entry()` (~linha 1307-1344): +`agent/credential_pool.py`, método `CredentialPool._refresh_entry()` (implementação anterior; as linhas atuais mudaram): + +O trecho abaixo registra a condição **antes** do PR e não descreve o código atual: ```python # Codex and xAI OAuth refresh tokens are single-use. The @@ -164,7 +168,7 @@ if self.provider in ("openai-codex", "xai-oauth"): return self._refresh_entry_impl(entry, force=force) # <- anthropic cai aqui, SEM lock ``` -O próprio comentário do código explica a razão de existir o lock: refresh tokens single-use não podem ser disputados por dois processos Hermes ao mesmo tempo. Só que **`"anthropic"` não está na tupla `("openai-codex", "xai-oauth")`** — mesmo o refresh da Anthropic sendo, pela documentação do próprio arquivo `agent/anthropic_adapter.py::_refresh_oauth_token`, também single-use: +O próprio comentário do código explica a razão de existir o lock: refresh tokens single-use não podem ser disputados por dois processos Hermes ao mesmo tempo. Na implementação anterior, **`"anthropic"` não estava na tupla `("openai-codex", "xai-oauth")`** — mesmo o refresh da Anthropic sendo, pela documentação do próprio arquivo `agent/anthropic_adapter.py::_refresh_oauth_token`, também single-use: > "Claude Code's OAuth refresh tokens are single-use: a successful refresh rotates the pair and invalidates the old refresh token." @@ -185,7 +189,7 @@ if self.provider != "anthropic" or entry.source != "claude_code": return entry ``` -Ou seja: **só funciona para credenciais que vieram do Claude Code CLI** (`~/.claude/.credentials.json`). Credenciais originadas do login OAuth nativo do próprio Hermes (`hermes_pkce`, gravadas em `~/.hermes/.anthropic_oauth.json`, e também as emitidas pelo dashboard — `manual:dashboard_pkce`) **não têm nenhuma rota de recuperação**. Ao perder a corrida, o processo cai direto em `self._mark_exhausted(entry, None)` (linha 1705), mesmo que uma credencial válida já exista em disco (escrita pelo processo vencedor). +Ou seja: **só funcionava para credenciais que vieram do Claude Code CLI** (`~/.claude/.credentials.json`). Credenciais originadas do login OAuth nativo do próprio Hermes (`manual:hermes_pkce`, gravadas em `~/.hermes/.anthropic_oauth.json`, e também as antigas emitidas pelo dashboard — `manual:dashboard_pkce`) **não tinham nenhuma rota de recuperação**. Ao perder a corrida, o processo caía direto em `self._mark_exhausted(entry, None)`, mesmo que uma credencial válida já existisse em disco (escrita pelo processo vencedor). ### Cenário concreto de exploração / impacto @@ -197,7 +201,7 @@ Isso não exige ataque — acontece em uso normal: múltiplos processos Hermes c ### Evidência de teste -`tests/agent/test_credential_pool_anthropic_refresh_race.py` — 3 testes: +Os testes abaixo são a evidência histórica da regressão na implementação anterior: ``` FAILED test_anthropic_refresh_is_not_protected_by_cross_process_lock @@ -210,7 +214,7 @@ PASSED test_concurrent_claude_code_refresh_recovers_via_credentials_file # contraste: fonte 'claude_code' SE recupera — prova a assimetria ``` -O teste usa um servidor OAuth fake que impõe corretamente a semântica "single-use" (primeiro a chegar ganha, segundo recebe `invalid_grant`), exatamente como o comportamento real documentado da Anthropic, e dispara dois `CredentialPool._refresh_entry()` concorrentes via `threading.Thread` simulando dois processos Hermes distintos. +O teste original usava um servidor OAuth fake que impunha corretamente a semântica "single-use" (primeiro a chegar ganha, segundo recebe `invalid_grant`) e disparava dois `CredentialPool._refresh_entry()` concorrentes via `threading.Thread`. A cobertura atual acrescenta processo independente, perfis distintos e contagem exata do POST; ver a seção 5. ### Correção recomendada @@ -229,34 +233,35 @@ Verificado especificamente para OAuth da Anthropic: --- -## 5. Como rodar os testes +## 5. Como validar a implementação atual ```bash -python -m pytest tests/hermes_cli/test_anthropic_dashboard_pkce_csrf.py tests/agent/test_credential_pool_anthropic_refresh_race.py -v +scripts/run_tests.sh tests/hermes_cli/test_web_oauth_dispatch.py tests/agent/test_credential_pool_anthropic_refresh_race.py tests/agent/test_anthropic_oauth_stress.py tests/agent/test_credential_pool_oauth_writethrough.py tests/hermes_cli/test_auth_store_lock_concurrent.py -q ``` -Resultado atual (código não corrigido): **7 failed, 1 passed** — as 7 falhas são a evidência dos bugs 1, 2 e 3; o único teste que passa (`test_concurrent_claude_code_refresh_recovers_via_credentials_file`) prova a assimetria descrita no Bug 3 (fonte `claude_code` se recupera, `hermes_pkce`/dashboard não). +O teste `test_distinct_profiles_share_one_claude_refresh_without_duplicate_post` usa dois processos independentes e exige um único POST de `stale-rt`; `test_hermes_pkce_refresh_writes_back_to_singleton` confirma que a rotação sobrevive a um `load_pool()` novo; `test_concurrent_refreshes_use_one_shared_credentials_lock` cobre o resolver direto; e os testes marcados `windows_only` exercitam a implementação `msvcrt` no host Windows. A cobertura frontend correspondente é `web/src/lib/api.test.ts`. ## 6. Arquivos e linhas de referência | Arquivo | Linhas relevantes | |---|---| -| `hermes_cli/web_server.py` | 10637-10661 (`_start_anthropic_pkce`), 10664-10728 (`_submit_anthropic_pkce`) | -| `agent/credential_pool.py` | 1307-1344 (`_refresh_entry`), 855-905 (`_sync_anthropic_entry_from_credentials_file`), 1442-1483 (fallback de recuperação), 1705 (`_mark_exhausted`) | -| `agent/anthropic_adapter.py` | 1125-1186 (`refresh_anthropic_oauth_pure`), 1189-1239 (`_refresh_oauth_token`, docstring sobre single-use), 1531-1658 (`run_hermes_oauth_login_pure`, fluxo CLI correto) | -| `tests/agent/test_anthropic_oauth_pkce.py` | Regressão histórica do bug 1/2 no fluxo CLI (já corrigido lá) | +| `hermes_cli/web_server.py` | Catálogo `anthropic` como `flow: "external"`; dispatcher rejeita `start`/`submit` | +| `agent/credential_pool.py` | `_refresh_entry`, `_sync_anthropic_entry_from_pool_store`, lock compartilhado Claude Code e recuperação | +| `agent/anthropic_adapter.py` | `claude_code_credentials_path`, refresh puro e fluxo CLI PKCE separado | +| `web/src/lib/api.ts` | `/api/providers/oauth` incluído no escopo de perfil | --- ## 7. Validação manual pós-fix (2026-08-17) -Além da suíte automatizada (seção 5), o fix do Bug 3 (API-key shadowing) foi -validado manualmente com um teste A/B real — mesma cena, duas versões do -código, sem mocks: +Antes da remoção do fluxo dashboard, o shadowing de API key foi validado +manualmente com um teste A/B real — mesma cena, duas versões do código, sem +mocks. Este registro é histórico e não deve ser interpretado como prova de +que o endpoint dashboard ainda existe: 1. `.env` de teste com `ANTHROPIC_API_KEY` obsoleta (simulando um setup antigo - esquecido) + chamada real a `_save_anthropic_oauth_creds` (a função - disparada pelo login OAuth do dashboard), em `HERMES_HOME` isolado. + esquecido) + chamada real a `_save_anthropic_oauth_creds` (a função que + existia no login OAuth do dashboard), em `HERMES_HOME` isolado. 2. **Sem o fix** (`main` @ `8c8d55b`, app instalado do usuário): depois do login OAuth, o `.env` continuou com a key obsoleta e `resolve_anthropic_token()` seguiu retornando a API key @@ -266,12 +271,7 @@ código, sem mocks: `resolve_anthropic_token()` passou a retornar o token OAuth (`sk-ant-oat...`, `is_oauth_token=True`). -Em seguida, o usuário rodou o `hermes chat` real no terminal (CLI, não -dashboard) com o binário instalado apontando para esta branch via -`PYTHONPATH`, contra seu `HERMES_HOME` real, e confirmou visualmente que o -app sobe e opera normalmente sob o código corrigido. - -**Ação pendente:** o app instalado do usuário (`%LOCALAPPDATA%\hermes\hermes-agent`) -ainda está em `main` sem este commit — precisa dar merge no PR #87891 e -atualizar (`hermes update`) para o fix valer em produção, não só na branch de -teste. +O fluxo dashboard foi removido depois dessa validação. O que permanece +verificável localmente é o fluxo de terminal e a ausência das rotas dashboard; +uma validação em uma instalação publicada não foi executada nesta revisão e +depende de distribuir uma versão que contenha a cabeça final do PR. diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index b870cad3fe..135bc64f09 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -1254,41 +1254,66 @@ def _refresh_oauth_token(creds: Dict[str, Any]) -> Optional[str]: Only fall back to refreshing ourselves when no fresh credential is found. """ # Claude Code may have already refreshed — adopt its token rather than - # racing it with our (possibly already-rotated) refresh token. Only adopt - # when the live re-read produced a DIFFERENT token with a real future - # expiry: re-adopting the same credential we were just handed would be a - # no-op, and a 0/absent ``expiresAt`` means "managed key / unknown expiry" - # (see is_claude_code_token_valid) which must NOT be treated as a fresh - # refresh here. - current = read_claude_code_credentials() - if current: - current_token = current.get("accessToken", "") - current_exp = current.get("expiresAt", 0) or 0 - if ( - current_token - and current_token != creds.get("accessToken", "") - and current_exp > 0 - and is_claude_code_token_valid(current) - ): - logger.debug("Adopted Claude Code's already-refreshed OAuth token") - return current_token - - refresh_token = (current or {}).get("refreshToken", "") or creds.get("refreshToken", "") - if not refresh_token: - logger.debug("No refresh token available — cannot refresh") - return None - + # racing it with our (possibly already-rotated) refresh token. The read, + # decision, POST, and write-back all belong to the shared credentials + # source, so hold the same path-keyed cross-process lock used by the pool. + # Without this direct resolver path, two profiles can still spend one + # single-use refresh token even though CredentialPool is serialized. try: - refreshed = refresh_anthropic_oauth_pure(refresh_token, use_json=False) - _write_claude_code_credentials( - refreshed["access_token"], - refreshed["refresh_token"], - refreshed["expires_at_ms"], + from hermes_cli.auth import AUTH_LOCK_TIMEOUT_SECONDS, _auth_store_lock, env_float + + refresh_timeout_seconds = env_float( + "HERMES_ANTHROPIC_REFRESH_TIMEOUT_SECONDS", 20 ) - logger.debug("Successfully refreshed Claude Code OAuth token") - return refreshed["access_token"] + lock_timeout_seconds = max( + float(AUTH_LOCK_TIMEOUT_SECONDS), + float(refresh_timeout_seconds) + 5.0, + ) + with _auth_store_lock( + timeout_seconds=lock_timeout_seconds, + target_path=claude_code_credentials_path(), + ): + # Only adopt when the live re-read produced a DIFFERENT token with + # a real future expiry: re-adopting the same credential we were + # just handed would be a no-op, and a 0/absent ``expiresAt`` means + # "managed key / unknown expiry" (see is_claude_code_token_valid). + current = read_claude_code_credentials() + if current: + current_token = current.get("accessToken", "") + current_exp = current.get("expiresAt", 0) or 0 + if ( + current_token + and current_token != creds.get("accessToken", "") + and current_exp > 0 + and is_claude_code_token_valid(current) + ): + logger.debug("Adopted Claude Code's already-refreshed OAuth token") + return current_token + + refresh_token = ( + (current or {}).get("refreshToken", "") + or creds.get("refreshToken", "") + ) + if not refresh_token: + logger.debug("No refresh token available — cannot refresh") + return None + + try: + refreshed = refresh_anthropic_oauth_pure(refresh_token, use_json=False) + _write_claude_code_credentials( + refreshed["access_token"], + refreshed["refresh_token"], + refreshed["expires_at_ms"], + ) + logger.debug("Successfully refreshed Claude Code OAuth token") + return refreshed["access_token"] + except Exception as e: + logger.debug("Failed to refresh Claude Code token: %s", e) + return None except Exception as e: - logger.debug("Failed to refresh Claude Code token: %s", e) + # Lock acquisition/read failures should preserve the resolver's + # existing fail-soft contract rather than taking down agent startup. + logger.debug("Failed to acquire Claude Code refresh lock: %s", e) return None @@ -1724,6 +1749,49 @@ def read_hermes_oauth_credentials() -> Optional[Dict[str, Any]]: return None +def _write_hermes_oauth_credentials( + access_token: str, + refresh_token: Optional[str], + expires_at_ms: Optional[int], +) -> None: + """Write refreshed hermes_pkce tokens back to ~/.hermes/.anthropic_oauth.json. + + Without this, a successful pool-level refresh of a ``hermes_pkce``-sourced + entry is invisible to this singleton file. The next ``load_pool()`` call + runs ``_seed_from_singletons()``, which reads the stale file and + overwrites the freshly-rotated pool entry with the pre-refresh (and, for + single-use Anthropic refresh tokens, already-consumed) token pair. + """ + oauth_file = _get_hermes_oauth_file() + try: + oauth_data = { + "accessToken": access_token, + "refreshToken": refresh_token, + "expiresAt": expires_at_ms, + } + oauth_file.parent.mkdir(parents=True, exist_ok=True) + _tmp_oauth = oauth_file.with_suffix(f".tmp.{os.getpid()}.{secrets.token_hex(4)}") + try: + fd = os.open( + str(_tmp_oauth), + os.O_WRONLY | os.O_CREAT | os.O_EXCL, + stat.S_IRUSR | stat.S_IWUSR, + ) + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump(oauth_data, fh, indent=2) + fh.flush() + os.fsync(fh.fileno()) + os.replace(_tmp_oauth, oauth_file) + except OSError: + try: + _tmp_oauth.unlink(missing_ok=True) + except OSError: + pass + raise + except (OSError, IOError) as e: + logger.debug("Failed to write refreshed Hermes OAuth credentials: %s", e) + + # --------------------------------------------------------------------------- # Message / tool / response format conversion # --------------------------------------------------------------------------- diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 4c802ac7bd..67c64182ea 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -1548,6 +1548,24 @@ class CredentialPool: ) except Exception as wexc: logger.debug("Failed to write refreshed token to credentials file: %s", wexc) + # Same rationale for the singleton source hermes_pkce: + # _seed_from_singletons() reads ~/.hermes/.anthropic_oauth.json + # on every load_pool() and will re-seed the pre-refresh (and + # already-consumed, single-use) token pair over this fresh one + # unless the singleton is updated in step with the pool entry. + # Do not use endswith here: manual:hermes_pkce is already + # pool-owned, and creating a singleton for it would introduce + # a second authority for the same refresh-token family. + elif entry.source == "hermes_pkce": + try: + from agent.anthropic_adapter import _write_hermes_oauth_credentials + _write_hermes_oauth_credentials( + refreshed["access_token"], + refreshed["refresh_token"], + refreshed["expires_at_ms"], + ) + except Exception as wexc: + logger.debug("Failed to write refreshed token to Hermes OAuth file: %s", wexc) elif self.provider == "openai-codex": # Adopt fresher tokens from auth.json before spending the # refresh_token — single-use tokens consumed by another Hermes diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index cc89a19c3e..b38d60b33c 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -10777,11 +10777,12 @@ async def test_messaging_platform(platform_id: str, profile: Optional[str] = Non # --------------------------------------------------------------------------- # # Phase 1 surfaces *which OAuth providers exist* and whether each is -# connected, plus a disconnect button. The actual login flow (PKCE for -# Anthropic, device-code for Nous/Codex) still runs in the CLI for now; -# Phase 2 will add in-browser flows. For unconnected providers we return -# the canonical ``hermes auth add `` command so the dashboard -# can surface a one-click copy. +# connected, plus a disconnect button. Anthropic subscription OAuth is +# deliberately delegated away from the dashboard: its card is external and +# points to the supported terminal path. Phase 2 adds in-browser device-code +# flows for providers that support them. For unconnected providers we return +# the canonical ``hermes auth add `` command so the dashboard can +# surface a one-click copy. def _truncate_token(value: Optional[str], visible: int = 6) -> str: @@ -10813,8 +10814,8 @@ def _anthropic_oauth_status() -> Dict[str, Any]: """Status for the "Anthropic API Key" catalog entry. Two sources, in priority order: - 1. ``~/.hermes/.anthropic_oauth.json`` — Hermes-managed PKCE flow (what - this entry's Connect button writes) + 1. ``~/.hermes/.anthropic_oauth.json`` — Hermes-managed terminal PKCE + credentials (the dashboard no longer has a Connect button for this) 2. ``ANTHROPIC_API_KEY`` → ``ANTHROPIC_TOKEN`` → ``CLAUDE_CODE_OAUTH_TOKEN`` env vars (registry order) — from ``.env``, the shell, or an external secret source like Bitwarden (whose keys are injected into the process @@ -10931,12 +10932,11 @@ def _copilot_acp_status() -> Dict[str, Any]: # which unions them with every accounts-tab provider in ``provider_catalog()`` # so newly-added OAuth/external providers appear automatically (no hand edit). # This tuple also still includes two entries that are NOT catalog providers but -# must show on the Accounts tab: the api-key Anthropic PKCE card and the +# must show on the Accounts tab: the Anthropic credential-status card and the # synthetic ``claude-code`` subscription row. -# ``flow`` describes the OAuth shape so the modal can pick the right UI: -# ``pkce`` = open URL + paste callback code, ``device_code`` = show code + -# verification URL + poll, ``external`` = read-only (delegated to a third-party -# CLI like Claude Code or Qwen). +# ``flow`` describes the account-management shape so the UI can pick the right +# behavior: ``device_code`` = show code + verification URL + poll, and +# ``external`` = read-only/delegated to a terminal or third-party CLI. _OAUTH_PROVIDER_CATALOG: tuple[Dict[str, Any], ...] = ( { "id": "nous", @@ -11222,7 +11222,7 @@ async def list_oauth_providers(profile: Optional[str] = None): Response shape (per provider): id stable identifier (used in DELETE path) name human label - flow "pkce" | "device_code" | "external" + flow "device_code" | "external" cli_command fallback CLI command for users to run manually disconnect_command shell command that clears an external provider's creds (run in the embedded terminal), else null @@ -12100,12 +12100,16 @@ async def poll_oauth_session( Each surfaces progress through the same background-worker-updated ``status`` field, so a single poll endpoint serves them all. """ + _validate_oauth_profile(profile) + requested_profile = _oauth_profile_name(profile) with _oauth_sessions_lock: sess = _oauth_sessions.get(session_id) if not sess: raise HTTPException(status_code=404, detail="Session not found or expired") if sess["provider"] != provider_id: raise HTTPException(status_code=400, detail="Provider mismatch for session") + if sess.get("profile") != requested_profile: + raise HTTPException(status_code=400, detail="OAuth session profile mismatch") return { "session_id": session_id, "status": sess["status"], @@ -12129,11 +12133,15 @@ async def cancel_oauth_session( user believed it was aborted. """ _require_token(request) + _validate_oauth_profile(profile) + requested_profile = _oauth_profile_name(profile) with _oauth_sessions_lock: sess = _oauth_sessions.get(session_id) if sess is not None: + if sess.get("profile") != requested_profile: + raise HTTPException(status_code=400, detail="OAuth session profile mismatch") sess["cancelled"] = True - _oauth_sessions.pop(session_id, None) + _oauth_sessions.pop(session_id, None) if sess is None: return {"ok": False, "message": "session not found"} return {"ok": True, "session_id": session_id} diff --git a/tests/agent/test_anthropic_keychain.py b/tests/agent/test_anthropic_keychain.py index d54a92c0d1..da9967f420 100644 --- a/tests/agent/test_anthropic_keychain.py +++ b/tests/agent/test_anthropic_keychain.py @@ -1,6 +1,8 @@ """Tests for Bug #12905 fixes in agent/anthropic_adapter.py — macOS Keychain support.""" import json +import threading +import time from unittest.mock import patch, MagicMock import pytest @@ -204,10 +206,14 @@ class TestRefreshOAuthTokenAdoptsFreshCredential: _FRESH = 9_999_999_999_999 - def test_adopts_already_refreshed_token_without_posting(self, monkeypatch): + def test_adopts_already_refreshed_token_without_posting(self, tmp_path, monkeypatch): """When a live source already holds a valid token, return it and skip the network refresh entirely. """ + monkeypatch.setattr( + "agent.anthropic_adapter.claude_code_credentials_path", + lambda: tmp_path / ".claude" / ".credentials.json", + ) fresh = { "accessToken": "already-refreshed-token", "refreshToken": "live-refresh", @@ -231,10 +237,14 @@ class TestRefreshOAuthTokenAdoptsFreshCredential: result = _refresh_oauth_token({"refreshToken": "stale", "expiresAt": 1}) assert result == "already-refreshed-token" - def test_falls_back_to_network_refresh_when_no_fresh_credential(self, monkeypatch): + def test_falls_back_to_network_refresh_when_no_fresh_credential(self, tmp_path, monkeypatch): """When no live source has a valid token, fall back to refreshing ourselves using the freshest available refresh token. """ + monkeypatch.setattr( + "agent.anthropic_adapter.claude_code_credentials_path", + lambda: tmp_path / ".claude" / ".credentials.json", + ) # Live read returns an expired credential carrying a refresh token. monkeypatch.setattr( "agent.anthropic_adapter.read_claude_code_credentials", @@ -263,3 +273,77 @@ class TestRefreshOAuthTokenAdoptsFreshCredential: # Prefers the live source's refresh token over the caller's stale copy. assert captured["refresh_token"] == "live-refresh" + def test_concurrent_refreshes_use_one_shared_credentials_lock(self, tmp_path, monkeypatch): + """Direct resolver refreshes must not spend one Claude token twice.""" + shared_credentials_path = tmp_path / ".claude" / ".credentials.json" + monkeypatch.setattr( + "agent.anthropic_adapter.claude_code_credentials_path", + lambda: shared_credentials_path, + ) + + state = { + "accessToken": "stale-access", + "refreshToken": "stale-refresh", + "expiresAt": 1, + } + state_lock = threading.Lock() + calls = [] + + def read_credentials(): + with state_lock: + return dict(state) + + def write_credentials(access_token, refresh_token, expires_at_ms, **_kwargs): + with state_lock: + state.update( + accessToken=access_token, + refreshToken=refresh_token, + expiresAt=expires_at_ms, + ) + + def refresh(refresh_token, **_kwargs): + calls.append(refresh_token) + # Without the production shared lock, both callers read the stale + # pair before either fake network request commits its rotation. + time.sleep(0.05) + with state_lock: + if state["refreshToken"] != refresh_token: + raise ValueError("invalid_grant: refresh token already used") + return { + "access_token": "fresh-access", + "refresh_token": "fresh-refresh", + "expires_at_ms": self._FRESH, + } + + monkeypatch.setattr("agent.anthropic_adapter.read_claude_code_credentials", read_credentials) + monkeypatch.setattr("agent.anthropic_adapter._write_claude_code_credentials", write_credentials) + monkeypatch.setattr("agent.anthropic_adapter.refresh_anthropic_oauth_pure", refresh) + + results = {} + errors = {} + start = threading.Barrier(2) + + def run(name): + try: + start.wait(timeout=5) + results[name] = _refresh_oauth_token( + { + "accessToken": "stale-access", + "refreshToken": "stale-refresh", + "expiresAt": 1, + } + ) + except BaseException as exc: # pragma: no cover - failure diagnostics + errors[name] = exc + + threads = [threading.Thread(target=run, args=(name,)) for name in ("a", "b")] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=5) + + assert not [thread for thread in threads if thread.is_alive()] + assert not errors, errors + assert results == {"a": "fresh-access", "b": "fresh-access"} + assert calls == ["stale-refresh"], calls + diff --git a/tests/agent/test_anthropic_oauth_stress.py b/tests/agent/test_anthropic_oauth_stress.py index 0598548942..c800f535d4 100644 --- a/tests/agent/test_anthropic_oauth_stress.py +++ b/tests/agent/test_anthropic_oauth_stress.py @@ -1,22 +1,27 @@ """Load / stress test for the Anthropic OAuth cross-process refresh race fix. Companion to ``tests/agent/test_credential_pool_anthropic_refresh_race.py``, -which proves the bug in isolation with two racers. This test scales the same -scenario up to look for bottlenecks and degradation under real concurrency: -many "Hermes processes" (threads, each with its own ``CredentialPool`` -instance) hammering the same single-use Anthropic refresh token at once, -against the REAL cross-process file lock (``_auth_store_lock``) and REAL -credential-pool persistence (not mocked) under a throwaway ``HERMES_HOME`` -- -only the network call to Anthropic is faked. This checks the fix does not -deadlock, does not lose updates, and does not degrade into a "thundering -herd" of redundant refresh POSTs as concurrency grows. +which proves the bug in isolation with two racers. This test scales the same scenario up to look for bottlenecks and degradation +under real concurrency. The thread stress case keeps the suite fast while a +separate spawn-based case uses independent interpreters, distinct profile +homes, and one shared Claude Code credentials file. Both exercise the REAL +cross-process file lock (``_auth_store_lock``) and REAL credential-pool +persistence under throwaway directories — only the network call to Anthropic +is faked. The process case also counts refresh POSTs and requires exactly one +use of the stale single-use token, so a broken lock cannot remain green merely +because two in-process mocks happened to finish quickly. """ from __future__ import annotations +import json +import multiprocessing as mp +import os +import queue import threading import time from dataclasses import replace as dc_replace +from pathlib import Path import pytest @@ -30,6 +35,94 @@ from agent.credential_pool import ( CONCURRENCY = 20 +def _process_claude_code_refresh_worker( + profile_home: str, + shared_credentials_path: str, + server_state_path: str, + start_event, + result_queue, +) -> None: + """Refresh one shared Claude Code credential from an independent process.""" + os.environ["HERMES_HOME"] = profile_home + + from agent import anthropic_adapter as anthropic_mod + from agent import credential_pool as credential_pool_mod + from hermes_cli import auth as auth_mod + + shared_path = Path(shared_credentials_path) + server_path = Path(server_state_path) + + def read_shared_credentials(): + data = json.loads(shared_path.read_text(encoding="utf-8")) + oauth = data["claudeAiOauth"] + return { + "accessToken": oauth["accessToken"], + "refreshToken": oauth.get("refreshToken", ""), + "expiresAt": oauth.get("expiresAt", 0), + "source": "claude_code_credentials_file", + } + + def write_shared_credentials(access_token, refresh_token, expires_at_ms, **_kwargs): + data = json.loads(shared_path.read_text(encoding="utf-8")) + data["claudeAiOauth"] = { + "accessToken": access_token, + "refreshToken": refresh_token, + "expiresAt": expires_at_ms, + } + shared_path.write_text(json.dumps(data), encoding="utf-8") + + def fake_refresh(refresh_token, *, use_json=False): + # The state file models a single-use token endpoint. The lock here + # protects only the fake server's accounting; the production lock is + # what must ensure that the second Hermes process never calls this + # function after the first one has rotated the shared credential. + with auth_mod._auth_store_lock(timeout_seconds=10, target_path=server_path): + state = json.loads(server_path.read_text(encoding="utf-8")) + state["calls"].append(refresh_token) + if refresh_token in state["spent"]: + server_path.write_text(json.dumps(state), encoding="utf-8") + raise ValueError("invalid_grant: refresh token already used") + state["spent"].append(refresh_token) + state["rotation"] += 1 + rotation = state["rotation"] + server_path.write_text(json.dumps(state), encoding="utf-8") + # Keep the simulated network operation inside the production shared + # lock long enough for the second profile to prove it waits, then + # re-reads the newly-written shared credentials file. + time.sleep(0.1) + return { + "access_token": f"process-access-{rotation}", + "refresh_token": f"process-refresh-{rotation}", + "expires_at_ms": int(time.time() * 1000) + 3_600_000, + } + + # Keep this worker hermetic: each profile has its own auth store, while + # both workers deliberately point at the same Claude credential source. + auth_mod._global_auth_file_path = lambda: None + anthropic_mod.claude_code_credentials_path = lambda: shared_path + anthropic_mod.read_claude_code_credentials = read_shared_credentials + anthropic_mod._write_claude_code_credentials = write_shared_credentials + anthropic_mod.refresh_anthropic_oauth_pure = fake_refresh + + result_queue.put({"kind": "ready", "pid": os.getpid()}) + if not start_event.wait(timeout=10): + result_queue.put({"kind": "result", "ok": False, "error": "start barrier timeout"}) + return + + entry = _entry(id="pool-entry", refresh_token="stale-rt", source="claude_code") + pool = credential_pool_mod.CredentialPool("anthropic", [entry]) + try: + refreshed = pool._refresh_entry(pool.entries()[0], force=True) + result_queue.put({ + "kind": "result", + "ok": refreshed is not None, + "refresh_token": refreshed.refresh_token if refreshed else None, + "pool_refresh_token": pool.entries()[0].refresh_token, + }) + except BaseException as exc: # pragma: no cover - failure diagnostics + result_queue.put({"kind": "result", "ok": False, "error": repr(exc)}) + + def _entry(*, id: str, refresh_token: str, source: str) -> PooledCredential: return PooledCredential( provider="anthropic", @@ -165,3 +258,130 @@ def test_high_concurrency_anthropic_refresh_no_lost_updates_no_deadlock( f"processes -- expected well under {naive_serial_upper_bound:.2f}s " "if the lock + pool-store adoption path is working efficiently" ) + + +@pytest.mark.live_system_guard_bypass +@pytest.mark.windows_only +def test_distinct_profiles_share_one_claude_refresh_without_duplicate_post( + hermes_home, +): + """Independent profiles must serialize a shared Claude Code refresh. + + The profile auth locks intentionally have different paths here; only the + dedicated lock keyed to the shared Claude credentials file can prevent the + second process from POSTing the already-spent refresh token. + """ + shared_credentials_path = hermes_home / "shared-claude-credentials.json" + shared_credentials_path.write_text( + json.dumps({ + "claudeAiOauth": { + "accessToken": "stale-at", + "refreshToken": "stale-rt", + "expiresAt": 0, + } + }), + encoding="utf-8", + ) + server_state_path = hermes_home / "fake-token-server.json" + server_state_path.write_text( + json.dumps({"calls": [], "spent": [], "rotation": 0}), + encoding="utf-8", + ) + + profile_homes = [hermes_home / "profile-a", hermes_home / "profile-b"] + for profile_home in profile_homes: + profile_home.mkdir(parents=True) + (profile_home / "auth.json").write_text( + json.dumps({ + "version": 1, + "providers": {}, + # claude_code is a borrowed source; its raw tokens must not + # be persisted in a profile pool. Each worker constructs the + # runtime entry from the shared credential source below. + "credential_pool": {}, + }), + encoding="utf-8", + ) + + ctx = mp.get_context("spawn") + start_event = ctx.Event() + result_queue = ctx.Queue() + processes = [ + ctx.Process( + target=_process_claude_code_refresh_worker, + args=( + str(profile_home), + str(shared_credentials_path), + str(server_state_path), + start_event, + result_queue, + ), + ) + for profile_home in profile_homes + ] + + messages = [] + try: + for process in processes: + process.start() + + ready_deadline = time.monotonic() + 20.0 + while len([m for m in messages if m.get("kind") == "ready"]) < len(processes): + remaining = max(0.1, ready_deadline - time.monotonic()) + if remaining <= 0.1: + break + try: + messages.append(result_queue.get(timeout=remaining)) + except queue.Empty: + break + assert len([m for m in messages if m.get("kind") == "ready"]) == len(processes), ( + f"not all refresh workers reached the start barrier: {messages!r}" + ) + start_event.set() + + for process in processes: + process.join(timeout=30) + assert not [process for process in processes if process.is_alive()], ( + "a profile refresh worker did not finish; possible shared-lock deadlock" + ) + + result_deadline = time.monotonic() + 5.0 + results = [m for m in messages if m.get("kind") == "result"] + while len(results) < len(processes) and time.monotonic() < result_deadline: + try: + message = result_queue.get(timeout=0.5) + except queue.Empty: + break + messages.append(message) + if message.get("kind") == "result": + results.append(message) + finally: + start_event.set() + for process in processes: + process.join(timeout=2) + for process in processes: + if process.is_alive(): + process.kill() + process.join(timeout=5) + result_queue.close() + result_queue.join_thread() + + assert len(results) == len(processes), f"missing process results: {messages!r}" + assert all(result.get("ok") for result in results), results + assert {result.get("refresh_token") for result in results} == {"process-refresh-1"} + assert all((profile_home / "auth.lock").exists() for profile_home in profile_homes) + assert shared_credentials_path.with_suffix(".lock").exists() + + server_state = json.loads(server_state_path.read_text(encoding="utf-8")) + assert server_state["calls"] == ["stale-rt"], ( + "the shared stale refresh token must be POSTed exactly once across " + f"distinct profiles, got {server_state['calls']!r}" + ) + assert server_state["spent"] == ["stale-rt"] + + shared_credentials = json.loads(shared_credentials_path.read_text(encoding="utf-8")) + assert shared_credentials["claudeAiOauth"]["refreshToken"] == "process-refresh-1" + for profile_home in profile_homes: + profile_text = (profile_home / "auth.json").read_text(encoding="utf-8") + assert "stale-rt" not in profile_text + assert "process-refresh-1" not in profile_text diff --git a/tests/agent/test_credential_pool_anthropic_refresh_race.py b/tests/agent/test_credential_pool_anthropic_refresh_race.py index 80a9a6bc5d..1fd7de043b 100644 --- a/tests/agent/test_credential_pool_anthropic_refresh_race.py +++ b/tests/agent/test_credential_pool_anthropic_refresh_race.py @@ -1,7 +1,7 @@ """Regression tests for cross-process races refreshing Anthropic OAuth tokens. ``CredentialPool._refresh_entry`` explicitly documents (see the comment -above the ``if self.provider in ("openai-codex", "xai-oauth"):`` branch in +above the ``if self.provider in ("openai-codex", "xai-oauth", "anthropic"):`` branch in ``agent/credential_pool.py``) that single-use OAuth refresh tokens require the whole sync -> POST -> write-back sequence to be serialized across Hermes *processes* via the cross-process ``_auth_store_lock`` flock, @@ -11,18 +11,21 @@ it, and the loser gets ``refresh_token_reused``". Anthropic's OAuth refresh tokens have the identical single-use property -- ``agent.anthropic_adapter._refresh_oauth_token`` says so explicitly: "Claude Code's OAuth refresh tokens are single-use: a successful refresh -rotates the pair and invalidates the old refresh token." Yet -``"anthropic"`` is absent from the ``("openai-codex", "xai-oauth")`` tuple -that gets the cross-process flock, and the *only* on-failure recovery path -(``CredentialPool._sync_anthropic_entry_from_credentials_file``) is -hard-scoped to ``entry.source == "claude_code"`` -- entries sourced from -Hermes's own PKCE login (``hermes_pkce`` / ``manual:dashboard_pkce``) get -no recovery at all and are marked exhausted on any lost race, even though -a fresh, valid token pair exists on disk (written by the winner). +rotates the pair and invalidates the old refresh token." Before the PR, ``"anthropic"`` was absent from the +``("openai-codex", "xai-oauth")`` tuple that gets the cross-process flock, +and the *only* on-failure recovery path +(``CredentialPool._sync_anthropic_entry_from_credentials_file``) was +hard-scoped to ``entry.source == "claude_code"``. Entries sourced from +Hermes's own PKCE login (``manual:hermes_pkce`` / ``hermes_pkce``) got no +recovery at all and were marked exhausted on a lost race, even though a +fresh, valid token pair existed on disk (written by the winner). The old +dashboard source ``manual:dashboard_pkce`` is now retired with the removed +dashboard flow. These tests reproduce that race deterministically with a fake OAuth server -that enforces single-use refresh tokens, run two "process-local" pools -concurrently against it, and assert the (currently absent) protection. +that enforces single-use refresh tokens, run concurrent pool instances against +it, and assert the current protection. The process-level, cross-profile +Claude Code witness lives in ``test_anthropic_oauth_stress.py``. """ from __future__ import annotations diff --git a/tests/agent/test_credential_pool_oauth_writethrough.py b/tests/agent/test_credential_pool_oauth_writethrough.py index 52678adb33..cd844aa27b 100644 --- a/tests/agent/test_credential_pool_oauth_writethrough.py +++ b/tests/agent/test_credential_pool_oauth_writethrough.py @@ -18,6 +18,7 @@ mocking the save boundary, so they exercise the actual atomic write path. import json import threading +import time import pytest @@ -26,6 +27,7 @@ from agent.credential_pool import ( AUTH_TYPE_OAUTH, CredentialPool, PooledCredential, + load_pool, ) from hermes_cli import auth as A @@ -317,3 +319,111 @@ def test_write_through_fires_on_every_refresh_not_just_first( ) assert root_tokens["refresh_token"] == "rf2" + +def test_hermes_pkce_refresh_writes_back_to_singleton(tmp_path, monkeypatch): + """A successful hermes_pkce refresh must update + ~/.hermes/.anthropic_oauth.json, or ``_seed_from_singletons()`` on the + next ``load_pool()`` re-seeds the pre-refresh (already-consumed, + single-use) token pair over the freshly rotated one. + """ + hermes_home = tmp_path / "hermes" + hermes_home.mkdir(parents=True, exist_ok=True) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setattr("hermes_cli.auth.is_provider_explicitly_configured", lambda pid: True) + + oauth_file = hermes_home / ".anthropic_oauth.json" + oauth_file.write_text( + json.dumps({"accessToken": "sk-ant-oat-rt0", "refreshToken": "rt0", "expiresAt": 0}), + encoding="utf-8", + ) + _write_store(hermes_home / "auth.json", {"version": 1, "providers": {}}) + + monkeypatch.setattr( + "agent.anthropic_adapter.refresh_anthropic_oauth_pure", + lambda refresh_token, use_json=False: { + "access_token": "sk-ant-oat-rt1", + "refresh_token": "rt1", + "expires_at_ms": int(time.time() * 1000) + 3_600_000, + }, + ) + monkeypatch.setattr("agent.anthropic_adapter.read_claude_code_credentials", lambda: None) + + entry = PooledCredential( + provider="anthropic", + id="pool-entry", + label="cred", + auth_type=AUTH_TYPE_OAUTH, + priority=0, + source="hermes_pkce", + access_token="sk-ant-oat-rt0", + refresh_token="rt0", + ) + pool = CredentialPool("anthropic", [entry]) + updated = pool._refresh_entry(entry, force=True) + assert updated is not None + assert updated.refresh_token == "rt1" + + on_disk = json.loads(oauth_file.read_text(encoding="utf-8")) + assert on_disk["refreshToken"] == "rt1", ( + "successful hermes_pkce refresh must write back to " + "~/.hermes/.anthropic_oauth.json, or _seed_from_singletons() will " + "revert the pool entry to the pre-refresh (spent) token on next load" + ) + + reloaded = load_pool("anthropic") + reloaded_entries = [e for e in reloaded.entries() if e.source.endswith("hermes_pkce")] + assert reloaded_entries, "hermes_pkce entry should still be present after reload" + assert reloaded_entries[0].refresh_token == "rt1", ( + "regression: fresh load_pool() re-seeded the pre-refresh refresh " + "token from the stale singleton file, reverting a successful " + "rotation and orphaning the already-consumed rt0" + ) + + +def test_manual_hermes_pkce_refresh_does_not_create_duplicate_singleton( + tmp_path, monkeypatch +): + """A pool-owned manual:hermes_pkce entry must not create a second source.""" + hermes_home = tmp_path / "hermes" + hermes_home.mkdir(parents=True, exist_ok=True) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setattr("hermes_cli.auth.is_provider_explicitly_configured", lambda pid: True) + monkeypatch.setattr("agent.anthropic_adapter.read_claude_code_credentials", lambda: None) + monkeypatch.setattr( + "agent.anthropic_adapter.refresh_anthropic_oauth_pure", + lambda refresh_token, use_json=False: { + "access_token": "manual-at-1", + "refresh_token": "manual-rt-1", + "expires_at_ms": int(time.time() * 1000) + 3_600_000, + }, + ) + _write_store(hermes_home / "auth.json", {"version": 1, "providers": {}}) + + entry = PooledCredential( + provider="anthropic", + id="manual-entry", + label="cred", + auth_type=AUTH_TYPE_OAUTH, + priority=0, + source="manual:hermes_pkce", + access_token="manual-at-0", + refresh_token="manual-rt-0", + expires_at_ms=0, + ) + pool = CredentialPool("anthropic", [entry]) + refreshed = pool._refresh_entry(entry, force=True) + + assert refreshed is not None + assert refreshed.refresh_token == "manual-rt-1" + oauth_file = hermes_home / ".anthropic_oauth.json" + assert not oauth_file.exists(), ( + "manual:hermes_pkce is already pool-owned; refreshing it must not " + "create a second hermes_pkce singleton source" + ) + + reloaded = load_pool("anthropic") + matching = [e for e in reloaded.entries() if e.id == "manual-entry"] + assert len(matching) == 1 + assert matching[0].source == "manual:hermes_pkce" + assert matching[0].refresh_token == "manual-rt-1" + diff --git a/tests/hermes_cli/test_auth_store_lock_concurrent.py b/tests/hermes_cli/test_auth_store_lock_concurrent.py index 066dabe2bd..30788abe17 100644 --- a/tests/hermes_cli/test_auth_store_lock_concurrent.py +++ b/tests/hermes_cli/test_auth_store_lock_concurrent.py @@ -39,6 +39,7 @@ def hermes_home(tmp_path, monkeypatch): return tmp_path +@pytest.mark.windows_only def test_many_concurrent_lock_acquisitions_do_not_raise_permission_error(hermes_home): """CONCURRENCY threads race to acquire/release the same auth-store lock. diff --git a/tests/hermes_cli/test_web_oauth_dispatch.py b/tests/hermes_cli/test_web_oauth_dispatch.py index 7de2e1ac6a..8e2c155591 100644 --- a/tests/hermes_cli/test_web_oauth_dispatch.py +++ b/tests/hermes_cli/test_web_oauth_dispatch.py @@ -5,17 +5,19 @@ flagged ``flow: "pkce"`` — anthropic and minimax-oauth — and the dispatcher ``start_oauth_login`` hardcoded ``_start_anthropic_pkce()`` for any pkce-flagged provider. So clicking "Login" next to MiniMax in the dashboard's Keys tab silently launched the Anthropic/Claude OAuth -flow. +flow. The Anthropic dashboard flow was later removed entirely because +Hermes must not mint subscription OAuth tokens from an unattended HTTP +endpoint; only the approved external CLI path remains. The fix: 1. Catalog entry for minimax-oauth changed from ``flow: "pkce"`` to ``flow: "device_code"`` (the actual UX is verification URI + user code + background poll, with PKCE as a security extension). 2. New MiniMax branch added to ``_start_device_code_flow``. - 3. Dispatcher tightened: pkce branch now requires - ``provider_id == "anthropic"``, so any future PKCE provider added - without an explicit branch gets a clean ``400 Unsupported flow`` - instead of silently launching Anthropic OAuth. + 3. Anthropic's catalog entry changed to ``flow: "external"`` and its + dashboard start/submit routes now reject the removed flow. Any future + provider added without an explicit flow gets a clean error instead of + silently launching Anthropic OAuth. These tests pin the corrected behavior. """ @@ -171,6 +173,48 @@ def test_oauth_start_stores_profile_for_background_completion(tmp_path, monkeypa ws._oauth_sessions.pop(session_id, None) +def test_oauth_session_cannot_be_polled_or_cancelled_from_another_profile( + tmp_path, monkeypatch +): + """A named-profile OAuth session must reject default-profile retargeting.""" + from hermes_cli import web_server as ws + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "profiles" / "worker").mkdir(parents=True) + session_id, _session = ws._new_oauth_session( + "xai-oauth", "device_code", profile="worker" + ) + try: + poll_resp = client.get( + f"/api/providers/oauth/xai-oauth/poll/{session_id}", + headers=HEADERS, + ) + assert poll_resp.status_code == 400, poll_resp.text + assert "profile" in poll_resp.text.lower() + + cancel_resp = client.delete( + f"/api/providers/oauth/sessions/{session_id}", + headers=HEADERS, + ) + assert cancel_resp.status_code == 400, cancel_resp.text + assert "profile" in cancel_resp.text.lower() + assert session_id in ws._oauth_sessions + + correct_poll = client.get( + f"/api/providers/oauth/xai-oauth/poll/{session_id}?profile=worker", + headers=HEADERS, + ) + assert correct_poll.status_code == 200, correct_poll.text + + correct_cancel = client.delete( + f"/api/providers/oauth/sessions/{session_id}?profile=worker", + headers=HEADERS, + ) + assert correct_cancel.status_code == 200, correct_cancel.text + finally: + ws._oauth_sessions.pop(session_id, None) + + def test_codex_dashboard_start_rewords_device_authorization_error(monkeypatch): @@ -271,7 +315,7 @@ def test_codex_dashboard_worker_stops_polling_after_cancel(tmp_path, monkeypatch ) saved = [] - monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + _make_profile_home(tmp_path, monkeypatch, profile="coder") monkeypatch.setattr(httpx, "Client", _Client) monkeypatch.setattr(auth_mod, "_save_codex_tokens", lambda tokens: saved.append(tokens)) @@ -280,7 +324,10 @@ def test_codex_dashboard_worker_stops_polling_after_cancel(tmp_path, monkeypatch def fake_sleep(_interval): # Simulate a real concurrent DELETE /api/providers/oauth/sessions/{sid} # firing while the worker is asleep between polls. - resp = client.delete(f"/api/providers/oauth/sessions/{sid}", headers=HEADERS) + resp = client.delete( + f"/api/providers/oauth/sessions/{sid}?profile=coder", + headers=HEADERS, + ) assert resp.status_code == 200, resp.text monkeypatch.setattr(ws.time, "sleep", fake_sleep) @@ -369,7 +416,10 @@ def test_codex_worker_final_save_is_atomic_with_cancel_delete(tmp_path, monkeypa def _fire_delete(): delete_started.set() - client.delete(f"/api/providers/oauth/sessions/{sid}", headers=HEADERS) + client.delete( + f"/api/providers/oauth/sessions/{sid}?profile=coder", + headers=HEADERS, + ) delete_finished.set() monkeypatch.setattr(auth_mod, "_save_codex_tokens", fake_save) @@ -396,7 +446,7 @@ def test_codex_worker_final_save_is_atomic_with_cancel_delete(tmp_path, monkeypa assert sid not in ws._oauth_sessions -def test_cancel_oauth_session_marks_dict_cancelled_before_popping(): +def test_cancel_oauth_session_marks_dict_cancelled_before_popping(tmp_path, monkeypatch): """The DELETE endpoint must flag the session dict before removing it. A background worker holds its own reference to the same dict object; @@ -405,6 +455,7 @@ def test_cancel_oauth_session_marks_dict_cancelled_before_popping(): """ from hermes_cli import web_server as ws + _make_profile_home(tmp_path, monkeypatch, profile="coder") session_id = "cancel-flag-test" ws._oauth_sessions[session_id] = { "session_id": session_id, @@ -418,7 +469,7 @@ def test_cancel_oauth_session_marks_dict_cancelled_before_popping(): worker_ref = ws._oauth_sessions[session_id] resp = client.delete( - f"/api/providers/oauth/sessions/{session_id}", + f"/api/providers/oauth/sessions/{session_id}?profile=coder", headers=HEADERS, ) @@ -490,6 +541,35 @@ def test_xai_oauth_listed_as_device_code_flow(): assert "grok" in providers["xai-oauth"]["name"].lower() +def test_anthropic_dashboard_oauth_is_removed_and_external(): + """Anthropic subscription OAuth is not minted by the dashboard anymore.""" + from hermes_cli import web_server as ws + + resp = client.get("/api/providers/oauth", headers=HEADERS) + assert resp.status_code == 200, resp.text + providers = {p["id"]: p for p in resp.json()["providers"]} + assert providers["anthropic"]["flow"] == "external" + assert providers["anthropic"]["cli_command"] == "hermes auth add anthropic" + + before_sessions = set(ws._oauth_sessions) + start_resp = client.post( + "/api/providers/oauth/anthropic/start", + headers=HEADERS, + ) + assert start_resp.status_code == 400, start_resp.text + assert "external CLI" in start_resp.text + assert "claude.ai" not in start_resp.text + + submit_resp = client.post( + "/api/providers/oauth/anthropic/submit", + headers=HEADERS, + json={"session_id": "unused", "code": "unused"}, + ) + assert submit_resp.status_code == 400, submit_resp.text + assert "not supported" in submit_resp.text + assert set(ws._oauth_sessions) == before_sessions + + def test_accounts_offers_every_oauth_provider_from_catalog(): """PARITY CONTRACT: every accounts-tab provider in the unified catalog (the `hermes model` universe) must be offered by /api/providers/oauth. This keeps diff --git a/web/src/lib/api.test.ts b/web/src/lib/api.test.ts index 5868f2bd5d..2524cc302f 100644 --- a/web/src/lib/api.test.ts +++ b/web/src/lib/api.test.ts @@ -1,7 +1,7 @@ // @vitest-environment jsdom import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { api, fetchJSON } from "./api"; +import { api, fetchJSON, setManagementProfile } from "./api"; const reloadMocks = vi.hoisted(() => ({ attemptDashboardTokenReloadOnce: vi.fn(() => false), @@ -33,6 +33,7 @@ beforeEach(() => { }); afterEach(() => { + setManagementProfile(""); vi.restoreAllMocks(); vi.unstubAllGlobals(); }); @@ -172,4 +173,30 @@ describe("api OAuth helpers", () => { expect((init.headers as Headers).has(SESSION_HEADER)).toBe(false); } }); + + it("keeps every OAuth operation on the selected management profile", async () => { + vi.stubGlobal("window", {}); + const fetchMock = jsonFetchMock({ + flow: "device_code", + session_id: "oauth-session", + }); + vi.stubGlobal("fetch", fetchMock); + setManagementProfile("worker"); + + await api.getOAuthProviders(); + await api.disconnectOAuthProvider("anthropic"); + await api.startOAuthLogin("openai-codex"); + await api.submitOAuthCode("anthropic", "oauth-session", "code-123"); + await api.pollOAuthSession("anthropic", "oauth-session"); + await api.cancelOAuthSession("oauth-session"); + + expect(fetchMock.mock.calls.map(([url]) => url)).toEqual([ + "/api/providers/oauth?profile=worker", + "/api/providers/oauth/anthropic?profile=worker", + "/api/providers/oauth/openai-codex/start?profile=worker", + "/api/providers/oauth/anthropic/submit?profile=worker", + "/api/providers/oauth/anthropic/poll/oauth-session?profile=worker", + "/api/providers/oauth/sessions/oauth-session?profile=worker", + ]); + }); }); diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 0d277ef589..f02841275b 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -79,6 +79,10 @@ const PROFILE_SCOPED_PREFIXES = [ "/api/messaging/platforms", "/api/messaging/telegram/onboarding", "/api/messaging/whatsapp/onboarding", + // OAuth/account state is profile-owned too: status, login sessions, polling, + // cancellation, and disconnect must all follow the selected management + // profile rather than silently targeting the dashboard process's profile. + "/api/providers/oauth", "/api/model/info", "/api/model/set", "/api/model/auxiliary",