fix(auth): keep the borrowed claude_code row out of token authority and carry the spent-rotation verdict through resolution
Two runtime blockers from the exact-head review of c057ef5.
1. A sanitized `claude_code` pool row was treated as token authority.
`claude_code` is a borrowed source: it is absent from the owned-source
allowlist, so `sanitize_borrowed_credential_payload` strips `access_token`
and `refresh_token` before the row reaches `auth.json`. `load_pool()`
re-hydrates the live pair from the singleton on every load, which is what
makes `~/.claude/.credentials.json` — not the pool store — authoritative
for this source.
`_sync_anthropic_entry_from_pool_store()` re-read that persisted row during
refresh. Being token-less, it "differed" from the live entry, so it was
adopted as a rotation performed by another process: `_refresh_entry()`
replaced a usable credential with an empty one and returned it before
`_claude_code_credentials_lock()` and the authoritative re-read were ever
entered. The empty OAuth entry then stayed selectable, because the
empty-runtime-key guard in `_available_entries()` covered API-key rows only.
Repairs: the pool-store sync refuses borrowed sources outright (plus a
defensive refusal of any token-less row, for future sources that sanitize on
write); the `claude_code` branch of `_refresh_entry()` now runs before the
generic adopt-and-return shortcut, so the path-keyed lock and the
authoritative re-read are always entered before deciding to POST or adopt;
and an OAuth entry with no access token is never leased.
2. A failed commit still fell through to the same spent credential.
`_refresh_oauth_token()` correctly returns None when the refresh POST
rotated the single-use token but the replacement could not be committed.
That verdict did not survive the caller: `resolve_anthropic_token()`
continued to `_resolve_anthropic_pool_token()`, which enumerates read-only
(`clear_expired=False, refresh=False`) over a pool that `load_pool()` had
just re-seeded from the unchanged singleton — so the pair whose refresh half
was already spent came back as a healthy token, and
`_refresh_provider_credentials("anthropic")` reported success and evicted
its cached clients.
Repair: every commit-failure path records the consumed pre-rotation pair as
non-reversible fingerprints (bounded, process-local), and both the Claude
Code file resolver and the pool resolver refuse a credential whose
fingerprint is on that list. `_refresh_provider_credentials("anthropic")`
consequently returns False when the spent family is the only credential,
while genuinely independent pool credentials stay eligible.
Coverage: `test_anthropic_borrowed_row_authority.py` starts from
`load_pool()` reading an actually persisted, actually sanitized row, forces
a refresh, and asserts the full pair survives with exactly one POST and one
commit, that the shared-file lock is entered, and that no empty OAuth entry
can be leased. `test_anthropic_spent_rotation_verdict.py` takes the full
resolver path: successful POST plus failed commit must make
`resolve_anthropic_token()` return None, make
`_refresh_provider_credentials("anthropic")` return False, and keep the
spent fingerprint out of every lease — with a control proving a successful
commit quarantines nothing and an independent credential still resolving.
Five of the seven new borrowed-row tests fail on the previous head, and the
three resolution tests fail with the verdict disabled.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gcoy6nLTg5R6FHHhjcLZEC
This commit is contained in:
@@ -102,6 +102,8 @@ from agent.anthropic_credentials import ( # noqa: F401
|
||||
_write_hermes_oauth_credentials,
|
||||
claude_code_credentials_path,
|
||||
is_claude_code_token_valid,
|
||||
is_rotation_consumed_uncommitted,
|
||||
mark_rotation_consumed_uncommitted,
|
||||
read_claude_code_credentials,
|
||||
read_hermes_oauth_credentials,
|
||||
refresh_anthropic_oauth_pure,
|
||||
|
||||
@@ -31,6 +31,8 @@ import platform
|
||||
import secrets
|
||||
import stat
|
||||
import subprocess
|
||||
import threading
|
||||
from collections import OrderedDict
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict, Optional
|
||||
|
||||
@@ -103,6 +105,59 @@ class CredentialPersistError(RuntimeError):
|
||||
self.path = path
|
||||
|
||||
|
||||
# Fingerprints of Anthropic secrets whose refresh POST succeeded (so the
|
||||
# server-side pair was rotated and the old refresh token is spent) but whose
|
||||
# replacement never reached its authoritative store. The pre-rotation pair
|
||||
# survives on disk and is re-seeded on the next ``load_pool()``, so without an
|
||||
# explicit verdict the resolver happily hands that already-consumed credential
|
||||
# back from a later source and the caller reads a silent success.
|
||||
#
|
||||
# Kept as non-reversible digests, process-local, and bounded: a spent secret is
|
||||
# spent forever, so entries never need clearing (a re-auth mints new tokens
|
||||
# with new fingerprints).
|
||||
_SPENT_ROTATION_LOCK = threading.Lock()
|
||||
_SPENT_ROTATION_FINGERPRINTS: "OrderedDict[str, None]" = OrderedDict()
|
||||
_SPENT_ROTATION_MAX_TRACKED = 64
|
||||
|
||||
|
||||
def mark_rotation_consumed_uncommitted(*secrets: Any) -> None:
|
||||
"""Record secrets consumed by a refresh whose replacement never committed.
|
||||
|
||||
Called from every commit-failure path (the direct resolver here and
|
||||
``CredentialPool._fail_closed_unpersisted_rotation``). Recording the
|
||||
*pre-rotation* pair is what lets later resolution steps recognise the stale
|
||||
copy they read back off disk as unusable rather than as a working token.
|
||||
"""
|
||||
from agent.credential_persistence import fingerprint_secret_value
|
||||
|
||||
with _SPENT_ROTATION_LOCK:
|
||||
for secret in secrets:
|
||||
value = str(secret or "").strip()
|
||||
if not value:
|
||||
continue
|
||||
fingerprint = fingerprint_secret_value(value)
|
||||
if not fingerprint:
|
||||
continue
|
||||
_SPENT_ROTATION_FINGERPRINTS.pop(fingerprint, None)
|
||||
_SPENT_ROTATION_FINGERPRINTS[fingerprint] = None
|
||||
while len(_SPENT_ROTATION_FINGERPRINTS) > _SPENT_ROTATION_MAX_TRACKED:
|
||||
_SPENT_ROTATION_FINGERPRINTS.popitem(last=False)
|
||||
|
||||
|
||||
def is_rotation_consumed_uncommitted(secret: Any) -> bool:
|
||||
"""True when *secret* belongs to a rotation that was spent but not committed."""
|
||||
from agent.credential_persistence import fingerprint_secret_value
|
||||
|
||||
value = str(secret or "").strip()
|
||||
if not value:
|
||||
return False
|
||||
fingerprint = fingerprint_secret_value(value)
|
||||
if not fingerprint:
|
||||
return False
|
||||
with _SPENT_ROTATION_LOCK:
|
||||
return fingerprint in _SPENT_ROTATION_FINGERPRINTS
|
||||
|
||||
|
||||
def _read_claude_code_credentials_from_keychain() -> Optional[Dict[str, Any]]:
|
||||
"""Read Claude Code OAuth credentials from the macOS Keychain.
|
||||
|
||||
@@ -406,6 +461,16 @@ def _refresh_oauth_token(creds: Dict[str, Any]) -> Optional[str]:
|
||||
claude_code_credentials_path(),
|
||||
e,
|
||||
)
|
||||
# The POST already spent ``refresh_token`` server-side and the
|
||||
# replacement is gone. The pre-rotation pair is still on disk,
|
||||
# so mark it: without this, source 5 re-reads it through the
|
||||
# pool and returns the consumed credential as a success.
|
||||
mark_rotation_consumed_uncommitted(
|
||||
refresh_token,
|
||||
creds.get("accessToken", ""),
|
||||
(current or {}).get("accessToken", ""),
|
||||
(current or {}).get("refreshToken", ""),
|
||||
)
|
||||
return None
|
||||
|
||||
logger.debug("Successfully refreshed Claude Code OAuth token")
|
||||
@@ -498,6 +563,15 @@ def _write_claude_code_credentials(
|
||||
def _resolve_claude_code_token_from_credentials(creds: Optional[Dict[str, Any]] = None) -> Optional[str]:
|
||||
"""Resolve a token from Claude Code credential files, refreshing if needed."""
|
||||
creds = creds or read_claude_code_credentials()
|
||||
if creds and is_rotation_consumed_uncommitted(creds.get("accessToken", "")):
|
||||
# This process already rotated this pair and failed to commit the
|
||||
# replacement. The file still holds the spent copy; treating it as
|
||||
# usable is exactly the silent success this transaction fails closed
|
||||
# to prevent.
|
||||
logger.debug(
|
||||
"Claude Code credentials hold a rotated-but-uncommitted token - refusing"
|
||||
)
|
||||
return None
|
||||
if creds and is_claude_code_token_valid(creds):
|
||||
logger.debug("Using Claude Code credentials (auto-detected)")
|
||||
return creds["accessToken"]
|
||||
@@ -566,8 +640,22 @@ def _resolve_anthropic_pool_token() -> Optional[str]:
|
||||
# and crash the whole resolver, taking down the source #5 fallback too.
|
||||
# Matches the aux-client analog (auxiliary_client.py: str(key or "")).
|
||||
token = (getattr(entry, "access_token", None) or "").strip()
|
||||
if token:
|
||||
return token
|
||||
if not token:
|
||||
continue
|
||||
# ``load_pool()`` re-seeds pool rows from the singleton files, so a
|
||||
# rotation that was consumed upstream but never committed comes back
|
||||
# here looking healthy. Enumeration is deliberately read-only
|
||||
# (refresh=False), which means nothing on this path would otherwise
|
||||
# notice that the credential is spent.
|
||||
if is_rotation_consumed_uncommitted(token) or is_rotation_consumed_uncommitted(
|
||||
getattr(entry, "refresh_token", None)
|
||||
):
|
||||
logger.debug(
|
||||
"Skipping Anthropic pool entry %s: rotated-but-uncommitted credential",
|
||||
getattr(entry, "id", "?"),
|
||||
)
|
||||
continue
|
||||
return token
|
||||
|
||||
return None
|
||||
|
||||
|
||||
@@ -991,13 +991,26 @@ class CredentialPool:
|
||||
``~/.claude/.credentials.json``), this re-reads the exact persisted
|
||||
row from the credential-pool store itself
|
||||
(``~/.hermes/auth.json`` / profile equivalent), so it works for
|
||||
every Anthropic source — ``claude_code``, ``hermes_pkce``, and
|
||||
every *pool-owned* Anthropic source - ``hermes_pkce`` and
|
||||
dashboard-issued ``manual:dashboard_pkce`` entries alike. Called
|
||||
while the shared cross-process auth-store lock is held, mirroring
|
||||
``_sync_xai_oauth_entry_from_pool_store``.
|
||||
|
||||
Borrowed sources (``claude_code``) are deliberately excluded: they
|
||||
are reference-only rows, so ``sanitize_borrowed_credential_payload``
|
||||
strips ``access_token``/``refresh_token`` before the row reaches
|
||||
``auth.json``. Re-reading such a row yields an entry whose tokens
|
||||
are empty, which differs from the live in-memory pair and would
|
||||
otherwise be adopted as a rotation performed by another process --
|
||||
replacing a usable credential with a blank one, and returning
|
||||
before the authoritative ``~/.claude/.credentials.json`` re-read
|
||||
ever happens. The pool store is not token authority for those
|
||||
sources; the singleton file is.
|
||||
"""
|
||||
if self.provider != "anthropic":
|
||||
return entry
|
||||
if is_borrowed_credential_source(entry.source, self.provider):
|
||||
return entry
|
||||
try:
|
||||
persisted = next(
|
||||
(
|
||||
@@ -1010,6 +1023,15 @@ class CredentialPool:
|
||||
if not isinstance(persisted, dict):
|
||||
return entry
|
||||
stored = PooledCredential.from_dict(self.provider, persisted)
|
||||
if not (stored.access_token or "").strip() and not (
|
||||
stored.refresh_token or ""
|
||||
).strip():
|
||||
# A row carrying no token material at all cannot be a
|
||||
# rotation performed by another process; adopting it would
|
||||
# blank the live entry. Belt-and-braces behind the
|
||||
# borrowed-source refusal above, for any future source that
|
||||
# sanitizes its secrets on write.
|
||||
return entry
|
||||
if (
|
||||
stored.access_token != entry.access_token
|
||||
or stored.refresh_token != entry.refresh_token
|
||||
@@ -1466,11 +1488,10 @@ class CredentialPool:
|
||||
if not force and not self._entry_needs_refresh(entry):
|
||||
return entry
|
||||
return self._refresh_entry_impl(entry, force=force)
|
||||
if (
|
||||
synced.access_token != entry.access_token
|
||||
or synced.refresh_token != entry.refresh_token
|
||||
):
|
||||
return synced
|
||||
# claude_code first: the shared credentials file - not the
|
||||
# pool store - is this source's token authority, so the
|
||||
# path-keyed lock and the authoritative re-read must be
|
||||
# entered before any adopt-and-return shortcut can fire.
|
||||
if self.provider == "anthropic" and synced.source == "claude_code":
|
||||
# claude_code entries are NOT profile-owned: the refresh
|
||||
# token lives in a single shared ~/.claude/.credentials.json
|
||||
@@ -1492,6 +1513,11 @@ class CredentialPool:
|
||||
if synced.refresh_token != entry.refresh_token:
|
||||
return synced
|
||||
return self._refresh_entry_impl(synced, force=force)
|
||||
if (
|
||||
synced.access_token != entry.access_token
|
||||
or synced.refresh_token != entry.refresh_token
|
||||
):
|
||||
return synced
|
||||
return self._refresh_entry_impl(synced, force=force)
|
||||
return self._refresh_entry_impl(entry, force=force)
|
||||
|
||||
@@ -1545,6 +1571,17 @@ class CredentialPool:
|
||||
store,
|
||||
exc,
|
||||
)
|
||||
try:
|
||||
from agent.anthropic_credentials import mark_rotation_consumed_uncommitted
|
||||
|
||||
# Quarantining the row is not enough on its own: the singleton file
|
||||
# still holds the spent pair, ``load_pool()`` re-seeds it, and the
|
||||
# read-only resolver (``_resolve_anthropic_pool_token``) would hand
|
||||
# it back as a working token. Record the fingerprints so every
|
||||
# resolution step in this process recognises it as consumed.
|
||||
mark_rotation_consumed_uncommitted(entry.access_token, entry.refresh_token)
|
||||
except Exception: # pragma: no cover - never block the quarantine
|
||||
logger.debug("Failed to record consumed rotation fingerprints", exc_info=True)
|
||||
self._mark_exhausted(
|
||||
entry,
|
||||
None,
|
||||
@@ -2244,6 +2281,14 @@ class CredentialPool:
|
||||
if refreshed is None:
|
||||
continue
|
||||
entry = refreshed
|
||||
if entry.auth_type == AUTH_TYPE_OAUTH and not (
|
||||
entry.access_token or ""
|
||||
).strip():
|
||||
# A borrowed OAuth row that failed to hydrate (or a
|
||||
# sanitized row read straight off disk) carries no access
|
||||
# token. The API-key guard at the top of the loop does not
|
||||
# cover it, and leasing it would send an empty bearer.
|
||||
continue
|
||||
available.append(entry)
|
||||
if entries_to_prune:
|
||||
pruned_ids = set(entries_to_prune)
|
||||
|
||||
281
tests/agent/test_anthropic_borrowed_row_authority.py
Normal file
281
tests/agent/test_anthropic_borrowed_row_authority.py
Normal file
@@ -0,0 +1,281 @@
|
||||
"""The borrowed ``claude_code`` row is a reference, never a token authority.
|
||||
|
||||
``claude_code`` is absent from ``_PERSISTABLE_PROVIDER_SOURCES``, so
|
||||
``sanitize_borrowed_credential_payload`` strips ``access_token`` and
|
||||
``refresh_token`` before the pool row reaches ``auth.json``: what survives on
|
||||
disk is provenance, status and a ``secret_fingerprint``. ``load_pool()``
|
||||
re-hydrates the live pair from ``~/.claude/.credentials.json`` on every load,
|
||||
which is what makes the singleton -- not the pool store -- authoritative for
|
||||
this source.
|
||||
|
||||
Two failure modes follow from forgetting that, and both are covered here:
|
||||
|
||||
1. ``_sync_anthropic_entry_from_pool_store()`` re-reads the persisted row
|
||||
during refresh. For a borrowed source that row has *no* tokens, so it
|
||||
"differs" from the live entry and was adopted as though another process had
|
||||
rotated the pair -- blanking a usable credential and returning before
|
||||
``_claude_code_credentials_lock()`` and the authoritative re-read were ever
|
||||
entered.
|
||||
2. ``_available_entries()`` only refused to lease empty *API-key* rows, so the
|
||||
blanked OAuth entry stayed selectable and would have been sent as an empty
|
||||
bearer.
|
||||
|
||||
The existing race/write-through suites build ``CredentialPool`` objects
|
||||
directly or back the store with unsanitized in-memory rows, so neither crosses
|
||||
the real persistence boundary. Every test below starts from ``load_pool()``
|
||||
reading an actually persisted, actually sanitized row.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import time
|
||||
from dataclasses import replace as dc_replace
|
||||
|
||||
import pytest
|
||||
|
||||
from agent import anthropic_credentials as AA
|
||||
from agent.credential_persistence import sanitize_borrowed_credential_payload
|
||||
from agent.credential_pool import (
|
||||
AUTH_TYPE_OAUTH,
|
||||
CredentialPool,
|
||||
PooledCredential,
|
||||
load_pool,
|
||||
)
|
||||
|
||||
_EXPIRED_MS = 1_000
|
||||
|
||||
_STALE_ACCESS = "sk-ant-oat01-borrowed-stale"
|
||||
_STALE_REFRESH = "sk-ant-ort01-borrowed-stale"
|
||||
_ROTATED_ACCESS = "sk-ant-oat01-borrowed-rotated"
|
||||
_ROTATED_REFRESH = "sk-ant-ort01-borrowed-rotated"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def hermes_home(tmp_path, monkeypatch):
|
||||
"""Real on-disk HERMES_HOME so ``load_pool()`` re-reads what it persisted."""
|
||||
home = tmp_path / "hermes"
|
||||
home.mkdir(parents=True, exist_ok=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
for var in ("ANTHROPIC_API_KEY", "ANTHROPIC_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN"):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
(home / "auth.json").write_text(
|
||||
json.dumps({"version": 1, "providers": {}}), encoding="utf-8"
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.auth.is_provider_explicitly_configured", lambda pid: True
|
||||
)
|
||||
return home
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def claude_credentials(tmp_path, monkeypatch):
|
||||
"""Point the ``claude_code`` singleton at a tmp file holding a stale pair."""
|
||||
cred_path = tmp_path / "claude" / ".credentials.json"
|
||||
cred_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
cred_path.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"claudeAiOauth": {
|
||||
"accessToken": _STALE_ACCESS,
|
||||
"refreshToken": _STALE_REFRESH,
|
||||
"expiresAt": _EXPIRED_MS,
|
||||
"scopes": ["user:inference", "user:profile"],
|
||||
}
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
monkeypatch.setattr(AA, "claude_code_credentials_path", lambda: cred_path)
|
||||
monkeypatch.setattr(AA, "_read_claude_code_credentials_from_keychain", lambda: None)
|
||||
return cred_path
|
||||
|
||||
|
||||
def _persisted_rows(home):
|
||||
store = json.loads((home / "auth.json").read_text(encoding="utf-8"))
|
||||
return store.get("credential_pool", {}).get("anthropic", [])
|
||||
|
||||
|
||||
def _claude_pair(cred_path):
|
||||
data = json.loads(cred_path.read_text(encoding="utf-8"))["claudeAiOauth"]
|
||||
return data["accessToken"], data["refreshToken"]
|
||||
|
||||
|
||||
def _rotating_refresh(refresh_token, **_kw):
|
||||
return {
|
||||
"access_token": _ROTATED_ACCESS,
|
||||
"refresh_token": _ROTATED_REFRESH,
|
||||
"expires_at_ms": int(time.time() * 1000) + 3_600_000,
|
||||
}
|
||||
|
||||
|
||||
def test_persisted_claude_code_row_carries_no_token_material(
|
||||
hermes_home, claude_credentials
|
||||
):
|
||||
"""Baseline: the row the refresh path re-reads really is sanitized.
|
||||
|
||||
Every other test in this file only means something if the disk row is
|
||||
token-less, so assert the boundary rather than assuming it.
|
||||
"""
|
||||
pool = load_pool("anthropic")
|
||||
|
||||
live = [e for e in pool._entries if e.source == "claude_code"]
|
||||
assert len(live) == 1
|
||||
assert live[0].access_token == _STALE_ACCESS, (
|
||||
"load_pool must hydrate the live pair from the singleton"
|
||||
)
|
||||
|
||||
rows = [r for r in _persisted_rows(hermes_home) if r.get("source") == "claude_code"]
|
||||
assert len(rows) == 1
|
||||
assert not rows[0].get("access_token")
|
||||
assert not rows[0].get("refresh_token")
|
||||
assert str(rows[0].get("secret_fingerprint", "")).startswith("sha256:")
|
||||
assert sanitize_borrowed_credential_payload(rows[0], "anthropic") == rows[0]
|
||||
|
||||
|
||||
def test_pool_store_sync_never_adopts_a_borrowed_row(hermes_home, claude_credentials):
|
||||
"""The sanitized row must not be mistaken for a rotation by another process."""
|
||||
pool = load_pool("anthropic")
|
||||
entry = next(e for e in pool._entries if e.source == "claude_code")
|
||||
|
||||
synced = pool._sync_anthropic_entry_from_pool_store(entry)
|
||||
|
||||
assert synced is entry, "a borrowed row is a reference, not token authority"
|
||||
assert synced.access_token == _STALE_ACCESS
|
||||
assert synced.refresh_token == _STALE_REFRESH
|
||||
|
||||
|
||||
def test_refresh_from_persisted_sanitized_row_keeps_the_full_pair(
|
||||
hermes_home, claude_credentials, monkeypatch
|
||||
):
|
||||
"""The production ``load -> sanitize -> refresh`` path refreshes, not blanks.
|
||||
|
||||
Exactly one POST and one authoritative write, the returned entry carries
|
||||
the complete rotated pair, and the shared credentials file is the copy that
|
||||
was updated.
|
||||
"""
|
||||
posts = []
|
||||
writes = []
|
||||
|
||||
def _counting_refresh(refresh_token, **kwargs):
|
||||
posts.append(refresh_token)
|
||||
return _rotating_refresh(refresh_token, **kwargs)
|
||||
|
||||
real_write = AA._write_claude_code_credentials
|
||||
|
||||
def _counting_write(access_token, refresh_token, expires_at_ms):
|
||||
writes.append(refresh_token)
|
||||
return real_write(access_token, refresh_token, expires_at_ms)
|
||||
|
||||
monkeypatch.setattr(AA, "refresh_anthropic_oauth_pure", _counting_refresh)
|
||||
monkeypatch.setattr(AA, "_write_claude_code_credentials", _counting_write)
|
||||
|
||||
pool = load_pool("anthropic")
|
||||
entry = next(e for e in pool._entries if e.source == "claude_code")
|
||||
|
||||
refreshed = pool._refresh_entry(entry, force=True)
|
||||
|
||||
assert refreshed is not None, "the refresh must not be abandoned"
|
||||
assert refreshed.access_token == _ROTATED_ACCESS
|
||||
assert refreshed.refresh_token == _ROTATED_REFRESH
|
||||
assert posts == [_STALE_REFRESH], f"expected exactly one POST, got {posts}"
|
||||
assert writes == [_ROTATED_REFRESH], f"expected exactly one commit, got {writes}"
|
||||
assert _claude_pair(claude_credentials) == (_ROTATED_ACCESS, _ROTATED_REFRESH)
|
||||
|
||||
|
||||
def test_refresh_reaches_the_shared_credentials_lock(
|
||||
hermes_home, claude_credentials, monkeypatch
|
||||
):
|
||||
"""``claude_code`` must always take the path-keyed lock before deciding.
|
||||
|
||||
That lock is what serializes profiles sharing one
|
||||
``~/.claude/.credentials.json``; an adopt-and-return shortcut firing first
|
||||
would leave the cross-profile race exactly where it was.
|
||||
"""
|
||||
taken = []
|
||||
real_lock = CredentialPool._claude_code_credentials_lock
|
||||
|
||||
def _tracking_lock(self):
|
||||
taken.append(True)
|
||||
return real_lock(self)
|
||||
|
||||
monkeypatch.setattr(CredentialPool, "_claude_code_credentials_lock", _tracking_lock)
|
||||
monkeypatch.setattr(AA, "refresh_anthropic_oauth_pure", _rotating_refresh)
|
||||
|
||||
pool = load_pool("anthropic")
|
||||
entry = next(e for e in pool._entries if e.source == "claude_code")
|
||||
pool._refresh_entry(entry, force=True)
|
||||
|
||||
assert taken, "the authoritative re-read must happen under the shared-file lock"
|
||||
|
||||
|
||||
def test_empty_oauth_entry_is_never_leased(hermes_home, claude_credentials):
|
||||
"""A token-less OAuth row must not be selectable as an empty bearer.
|
||||
|
||||
The pre-existing guard covered ``AUTH_TYPE_API_KEY`` only, so an OAuth row
|
||||
that failed to hydrate went straight into the available list.
|
||||
"""
|
||||
pool = load_pool("anthropic")
|
||||
entry = next(e for e in pool._entries if e.source == "claude_code")
|
||||
blanked = dc_replace(entry, access_token="", refresh_token="")
|
||||
pool._replace_entry(entry, blanked)
|
||||
|
||||
available, _pending = pool._available_entries(clear_expired=False, refresh=False)
|
||||
|
||||
assert all(e.access_token for e in available), (
|
||||
"an OAuth entry with no access token must never be leased"
|
||||
)
|
||||
assert blanked.id not in {e.id for e in available}
|
||||
|
||||
|
||||
def test_selection_after_refresh_leases_only_hydrated_entries(
|
||||
hermes_home, claude_credentials, monkeypatch
|
||||
):
|
||||
"""End-to-end: refresh through selection leaves a usable, non-empty lease."""
|
||||
monkeypatch.setattr(AA, "refresh_anthropic_oauth_pure", _rotating_refresh)
|
||||
|
||||
pool = load_pool("anthropic")
|
||||
available, _pending = pool._available_entries(clear_expired=True, refresh=True)
|
||||
|
||||
assert available, "the credential must survive the refresh, not be dropped"
|
||||
assert all(e.access_token for e in available)
|
||||
assert any(e.access_token == _ROTATED_ACCESS for e in available)
|
||||
|
||||
|
||||
def test_hermes_pkce_row_still_syncs_from_the_pool_store(monkeypatch):
|
||||
"""The borrowed-source refusal must not disable pool-owned adoption.
|
||||
|
||||
``hermes_pkce`` *is* pool-owned, so its persisted row keeps its tokens and
|
||||
stays a legitimate rotation witness for another pool instance.
|
||||
"""
|
||||
rotated = {
|
||||
"id": "anthropic-pkce",
|
||||
"label": "anthropic oauth",
|
||||
"auth_type": AUTH_TYPE_OAUTH,
|
||||
"priority": 0,
|
||||
"source": "hermes_pkce",
|
||||
"access_token": _ROTATED_ACCESS,
|
||||
"refresh_token": _ROTATED_REFRESH,
|
||||
"expires_at_ms": int(time.time() * 1000) + 3_600_000,
|
||||
}
|
||||
monkeypatch.setattr(
|
||||
"agent.credential_pool.read_credential_pool", lambda provider=None: [rotated]
|
||||
)
|
||||
|
||||
entry = PooledCredential(
|
||||
provider="anthropic",
|
||||
id="anthropic-pkce",
|
||||
label="anthropic oauth",
|
||||
auth_type=AUTH_TYPE_OAUTH,
|
||||
priority=0,
|
||||
source="hermes_pkce",
|
||||
access_token=_STALE_ACCESS,
|
||||
refresh_token=_STALE_REFRESH,
|
||||
expires_at_ms=_EXPIRED_MS,
|
||||
)
|
||||
pool = CredentialPool("anthropic", [entry])
|
||||
|
||||
synced = pool._sync_anthropic_entry_from_pool_store(entry)
|
||||
|
||||
assert synced.access_token == _ROTATED_ACCESS
|
||||
assert synced.refresh_token == _ROTATED_REFRESH
|
||||
@@ -54,6 +54,19 @@ _ROTATED_ACCESS = "sk-ant-oat01-rotated"
|
||||
_ROTATED_REFRESH = "sk-ant-ort01-rotated"
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clean_spent_registry():
|
||||
"""Isolate the process-global consumed-rotation registry between tests.
|
||||
|
||||
Every failure injection here records the spent pair (see
|
||||
``mark_rotation_consumed_uncommitted``), and those fingerprints would
|
||||
otherwise leak into unrelated tests that reuse the same token literals.
|
||||
"""
|
||||
AA._SPENT_ROTATION_FINGERPRINTS.clear()
|
||||
yield
|
||||
AA._SPENT_ROTATION_FINGERPRINTS.clear()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def hermes_home(tmp_path, monkeypatch):
|
||||
"""Real on-disk HERMES_HOME so ``load_pool()`` re-reads what we persisted."""
|
||||
|
||||
254
tests/agent/test_anthropic_spent_rotation_verdict.py
Normal file
254
tests/agent/test_anthropic_spent_rotation_verdict.py
Normal file
@@ -0,0 +1,254 @@
|
||||
"""A rotation that was consumed but never committed must stay unusable.
|
||||
|
||||
``_refresh_oauth_token()`` already refuses to return an access token whose
|
||||
refresh half was lost to a failed write. That verdict was local: the caller
|
||||
above it (``resolve_anthropic_token()``) simply continued to the next source,
|
||||
and source 5 (``_resolve_anthropic_pool_token``) enumerates read-only
|
||||
(``clear_expired=False, refresh=False``) over a pool that ``load_pool()`` has
|
||||
just re-seeded from the *unchanged* singleton file. So the very pair whose
|
||||
refresh token the POST had already spent came back as a healthy token, and
|
||||
``_refresh_provider_credentials("anthropic")`` reported the refresh as a
|
||||
success and evicted its cached clients.
|
||||
|
||||
That is the same silent-transition failure the fail-closed path exists to
|
||||
prevent, one layer up: no ``invalid_grant`` is raised until the *next* refresh,
|
||||
by which point the provenance of the failure is gone.
|
||||
|
||||
These tests take the full resolver path, not just the writer: successful POST +
|
||||
failed commit must make ``resolve_anthropic_token()`` return ``None`` (or a
|
||||
genuinely independent credential), must make
|
||||
``_refresh_provider_credentials("anthropic")`` return ``False`` when the spent
|
||||
family is the only credential, and must keep the spent fingerprint out of every
|
||||
lease.
|
||||
|
||||
Companion: ``test_anthropic_credential_persist_failure.py`` covers the writers
|
||||
and the pool quarantine; this file covers what resolution does afterwards.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
from agent import anthropic_credentials as AA
|
||||
from agent.auxiliary_client import _refresh_provider_credentials
|
||||
from agent.credential_pool import AUTH_TYPE_OAUTH, load_pool
|
||||
|
||||
_EXPIRED_MS = 1_000
|
||||
|
||||
_STALE_ACCESS = "sk-ant-oat01-spent-stale"
|
||||
_STALE_REFRESH = "sk-ant-ort01-spent-stale"
|
||||
_ROTATED_ACCESS = "sk-ant-oat01-spent-rotated"
|
||||
_ROTATED_REFRESH = "sk-ant-ort01-spent-rotated"
|
||||
_INDEPENDENT_ACCESS = "sk-ant-oat01-independent"
|
||||
_INDEPENDENT_REFRESH = "sk-ant-ort01-independent"
|
||||
|
||||
_SINGLETON_FILENAMES = frozenset({".credentials.json", ".anthropic_oauth.json"})
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clean_spent_registry():
|
||||
"""The consumed-rotation registry is process-global; isolate each test."""
|
||||
AA._SPENT_ROTATION_FINGERPRINTS.clear()
|
||||
yield
|
||||
AA._SPENT_ROTATION_FINGERPRINTS.clear()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def hermes_home(tmp_path, monkeypatch):
|
||||
home = tmp_path / "hermes"
|
||||
home.mkdir(parents=True, exist_ok=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
for var in ("ANTHROPIC_API_KEY", "ANTHROPIC_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN"):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
(home / "auth.json").write_text(
|
||||
json.dumps({"version": 1, "providers": {}}), encoding="utf-8"
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.auth.is_provider_explicitly_configured", lambda pid: True
|
||||
)
|
||||
return home
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def claude_credentials(tmp_path, monkeypatch):
|
||||
cred_path = tmp_path / "claude" / ".credentials.json"
|
||||
cred_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
cred_path.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"claudeAiOauth": {
|
||||
"accessToken": _STALE_ACCESS,
|
||||
"refreshToken": _STALE_REFRESH,
|
||||
"expiresAt": _EXPIRED_MS,
|
||||
"scopes": ["user:inference", "user:profile"],
|
||||
}
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
monkeypatch.setattr(AA, "claude_code_credentials_path", lambda: cred_path)
|
||||
monkeypatch.setattr(AA, "_read_claude_code_credentials_from_keychain", lambda: None)
|
||||
return cred_path
|
||||
|
||||
|
||||
def _rotating_refresh(*_a, **_kw):
|
||||
"""Stand-in for the token endpoint: the POST always succeeds and rotates."""
|
||||
return {
|
||||
"access_token": _ROTATED_ACCESS,
|
||||
"refresh_token": _ROTATED_REFRESH,
|
||||
"expires_at_ms": int(time.time() * 1000) + 3_600_000,
|
||||
}
|
||||
|
||||
|
||||
def _break_durable_write(monkeypatch):
|
||||
"""Only the authoritative singletons fail; auth.json must stay writable."""
|
||||
real_replace = os.replace
|
||||
|
||||
def _failing_replace(src, dst):
|
||||
if os.path.basename(os.fspath(dst)) in _SINGLETON_FILENAMES:
|
||||
raise OSError(13, "Permission denied")
|
||||
return real_replace(src, dst)
|
||||
|
||||
monkeypatch.setattr(AA.os, "replace", _failing_replace)
|
||||
|
||||
|
||||
def _add_independent_pool_entry(home):
|
||||
"""Persist a second, unrelated Anthropic OAuth credential in the pool."""
|
||||
path = home / "auth.json"
|
||||
store = json.loads(path.read_text(encoding="utf-8"))
|
||||
pool = store.setdefault("credential_pool", {})
|
||||
pool.setdefault("anthropic", []).append(
|
||||
{
|
||||
"id": "anthropic-independent",
|
||||
"label": "second subscription",
|
||||
"auth_type": AUTH_TYPE_OAUTH,
|
||||
"priority": 10,
|
||||
"source": "manual",
|
||||
"access_token": _INDEPENDENT_ACCESS,
|
||||
"refresh_token": _INDEPENDENT_REFRESH,
|
||||
"expires_at_ms": int(time.time() * 1000) + 3_600_000,
|
||||
}
|
||||
)
|
||||
path.write_text(json.dumps(store), encoding="utf-8")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The registry itself
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_registry_matches_only_the_recorded_secret():
|
||||
AA.mark_rotation_consumed_uncommitted(_STALE_REFRESH, "", None)
|
||||
|
||||
assert AA.is_rotation_consumed_uncommitted(_STALE_REFRESH)
|
||||
assert not AA.is_rotation_consumed_uncommitted(_INDEPENDENT_REFRESH)
|
||||
assert not AA.is_rotation_consumed_uncommitted("")
|
||||
assert not AA.is_rotation_consumed_uncommitted(None)
|
||||
|
||||
|
||||
def test_registry_stays_bounded():
|
||||
for i in range(AA._SPENT_ROTATION_MAX_TRACKED * 2):
|
||||
AA.mark_rotation_consumed_uncommitted(f"sk-ant-ort01-{i}")
|
||||
|
||||
assert len(AA._SPENT_ROTATION_FINGERPRINTS) == AA._SPENT_ROTATION_MAX_TRACKED
|
||||
assert AA.is_rotation_consumed_uncommitted(
|
||||
f"sk-ant-ort01-{AA._SPENT_ROTATION_MAX_TRACKED * 2 - 1}"
|
||||
), "the most recent rotation must survive eviction"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Full resolution
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_failed_commit_marks_the_consumed_pair(
|
||||
hermes_home, claude_credentials, monkeypatch
|
||||
):
|
||||
"""The pre-rotation pair - the copy left on disk - is what gets recorded."""
|
||||
monkeypatch.setattr(AA, "refresh_anthropic_oauth_pure", _rotating_refresh)
|
||||
_break_durable_write(monkeypatch)
|
||||
|
||||
assert AA._refresh_oauth_token(AA.read_claude_code_credentials()) is None
|
||||
|
||||
assert AA.is_rotation_consumed_uncommitted(_STALE_REFRESH)
|
||||
assert AA.is_rotation_consumed_uncommitted(_STALE_ACCESS)
|
||||
|
||||
|
||||
def test_resolve_returns_none_when_the_rotation_could_not_commit(
|
||||
hermes_home, claude_credentials, monkeypatch
|
||||
):
|
||||
"""Full resolver: source 5 must not hand back the pair source 4 refused.
|
||||
|
||||
``load_pool()`` re-seeds the claude_code row straight from the unchanged
|
||||
credentials file, so without the consumed-rotation verdict this returns the
|
||||
already-spent access token and the caller sees a success.
|
||||
"""
|
||||
monkeypatch.setattr(AA, "refresh_anthropic_oauth_pure", _rotating_refresh)
|
||||
_break_durable_write(monkeypatch)
|
||||
|
||||
assert AA.resolve_anthropic_token() is None, (
|
||||
"a consumed-but-uncommitted rotation must not resolve to a usable token"
|
||||
)
|
||||
|
||||
|
||||
def test_spent_fingerprint_is_never_leased_from_the_pool(
|
||||
hermes_home, claude_credentials, monkeypatch
|
||||
):
|
||||
"""Direct witness on source 5 alone, after the rotation was spent."""
|
||||
monkeypatch.setattr(AA, "refresh_anthropic_oauth_pure", _rotating_refresh)
|
||||
_break_durable_write(monkeypatch)
|
||||
|
||||
assert AA._refresh_oauth_token(AA.read_claude_code_credentials()) is None
|
||||
|
||||
# The pool still holds the pre-rotation pair: nothing rewrote the file.
|
||||
pool = load_pool("anthropic")
|
||||
seeded = next(e for e in pool._entries if e.source == "claude_code")
|
||||
assert seeded.access_token == _STALE_ACCESS
|
||||
|
||||
assert AA._resolve_anthropic_pool_token() is None, (
|
||||
"the spent credential must not be leased just because it is on disk"
|
||||
)
|
||||
|
||||
|
||||
def test_auxiliary_refresh_reports_failure_for_a_lost_commit(
|
||||
hermes_home, claude_credentials, monkeypatch
|
||||
):
|
||||
"""``_refresh_provider_credentials`` must fail when this is the only credential.
|
||||
|
||||
Returning True here evicts the cached clients and tells the retry loop the
|
||||
provider recovered, which is the point at which the failure stops being
|
||||
visible anywhere.
|
||||
"""
|
||||
monkeypatch.setattr(AA, "refresh_anthropic_oauth_pure", _rotating_refresh)
|
||||
_break_durable_write(monkeypatch)
|
||||
|
||||
assert _refresh_provider_credentials("anthropic") is False
|
||||
|
||||
|
||||
def test_independent_pool_credential_stays_eligible(
|
||||
hermes_home, claude_credentials, monkeypatch
|
||||
):
|
||||
"""Failing closed is scoped to the spent family, not to Anthropic as a whole."""
|
||||
_add_independent_pool_entry(hermes_home)
|
||||
monkeypatch.setattr(AA, "refresh_anthropic_oauth_pure", _rotating_refresh)
|
||||
_break_durable_write(monkeypatch)
|
||||
|
||||
resolved = AA.resolve_anthropic_token()
|
||||
|
||||
assert resolved == _INDEPENDENT_ACCESS, (
|
||||
"an unrelated credential must still be selectable after the quarantine"
|
||||
)
|
||||
|
||||
|
||||
def test_successful_commit_leaves_the_credential_usable(
|
||||
hermes_home, claude_credentials, monkeypatch
|
||||
):
|
||||
"""Control: nothing is quarantined when the commit actually lands."""
|
||||
monkeypatch.setattr(AA, "refresh_anthropic_oauth_pure", _rotating_refresh)
|
||||
|
||||
assert AA.resolve_anthropic_token() == _ROTATED_ACCESS
|
||||
assert AA._SPENT_ROTATION_FINGERPRINTS == {}
|
||||
Reference in New Issue
Block a user