fix(auth): close Anthropic OAuth review gaps
This commit is contained in:
joaomarcos
+53
-41
@@ -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 |
|
||||
|
||||
@@ -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.
|
||||
|
||||
+100
-32
@@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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
|
||||
|
||||
+22
-14
@@ -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 <provider>`` 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 <provider>`` 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}
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
+28
-1
@@ -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",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user