diff --git a/tests/agent/test_credential_pool_codex_singleton_isolation.py b/tests/agent/test_credential_pool_codex_singleton_isolation.py new file mode 100644 index 0000000000..f57ab0392c --- /dev/null +++ b/tests/agent/test_credential_pool_codex_singleton_isolation.py @@ -0,0 +1,106 @@ +"""Codex pool entries and the auth.json singleton: who may adopt whose tokens. + +``manual:device_code`` is ambiguous — a legacy alias of the singleton or an independent account +added with ``hermes auth add openai-codex``. Adopting the singleton into an independent account +silently turned two logins into one (both then hit the same usage limit; salvaged from #100423 and +#106788, cluster #92198 / #95297, issue #106705). +""" +from __future__ import annotations + +import base64 +import json +import time + +import pytest + +import hermes_cli.auth as auth_mod +from agent.credential_pool import load_pool + + +def _jwt(account: str, sub: str, exp: float) -> str: + def seg(obj: dict) -> str: + return base64.urlsafe_b64encode(json.dumps(obj).encode()).rstrip(b"=").decode() + + payload = {"exp": int(exp), "sub": sub, "https://api.openai.com/auth": {"chatgpt_account_id": account}} + return f"{seg({'alg': 'none'})}.{seg(payload)}.sig" + + +def _iso(ts: float) -> str: + return time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime(ts)) + + +def _write_store(home, singleton_tokens: dict, singleton_last_refresh: str, manual: dict) -> None: + home.mkdir(parents=True, exist_ok=True) + (home / "auth.json").write_text(json.dumps({ + "version": 1, + "active_provider": "openai-codex", + "providers": {"openai-codex": { + "tokens": singleton_tokens, "last_refresh": singleton_last_refresh, "auth_mode": "chatgpt"}}, + "credential_pool": {"openai-codex": [ + {"id": "seeded", "label": "device_code", "auth_type": "oauth", "priority": 0, + "source": "device_code", **singleton_tokens, "last_refresh": singleton_last_refresh}, + {"id": "manual", "label": "second", "auth_type": "oauth", "priority": 1, + "source": "manual:device_code", **manual}, + ]}, + }), encoding="utf-8") + + +@pytest.fixture +def home(tmp_path, monkeypatch): + home = tmp_path / "hermes" + monkeypatch.setenv("HERMES_HOME", str(home)) + return home + + +def _stub_refresh(monkeypatch, minted: str, minted_rt: str, posted: list) -> None: + def fake(access_token, refresh_token): + posted.append(refresh_token) + return {"access_token": minted, "refresh_token": minted_rt, "last_refresh": _iso(time.time())} + + monkeypatch.setattr(auth_mod, "refresh_codex_oauth_pure", fake) + + +def test_independent_manual_account_refreshes_with_its_own_pair(home, monkeypatch): + """An independent second account never adopts the singleton: its refresh POSTs its OWN refresh + token, and the persisted row still identifies the second principal afterwards.""" + now = time.time() + a_at = _jwt("acct-A", "user-A", now + 8 * 3600) + b_at = _jwt("acct-B", "user-B", now + 60) # expiring → the pool defers it to _refresh_entry() + _write_store(home, {"access_token": a_at, "refresh_token": "rt-A"}, _iso(now - 3600), + {"access_token": b_at, "refresh_token": "rt-B", "last_refresh": _iso(now - 7200)}) + posted: list = [] + b_new = _jwt("acct-B", "user-B", now + 8 * 3600) + _stub_refresh(monkeypatch, b_new, "rt-B2", posted) + + pool = load_pool("openai-codex") + refreshed = pool._refresh_entry(next(e for e in pool.entries() if e.id == "manual"), force=False) + + assert posted == ["rt-B"] + assert refreshed is not None and refreshed.refresh_token == "rt-B2" + on_disk = json.loads((home / "auth.json").read_text(encoding="utf-8")) + manual = next(e for e in on_disk["credential_pool"]["openai-codex"] if e["id"] == "manual") + assert (manual["access_token"], manual["refresh_token"]) == (b_new, "rt-B2") + # The singleton (account A) is untouched by account B's rotation. + assert on_disk["providers"]["openai-codex"]["tokens"] == {"access_token": a_at, "refresh_token": "rt-A"} + + +def test_same_account_alias_adopts_only_a_newer_singleton(home, monkeypatch): + """A legacy alias (same principal) must follow a singleton that was re-authed AFTER it, but must + not fall back onto a singleton older than its own rotation — that replays a consumed token.""" + now = time.time() + alias_at = _jwt("acct-A", "user-A", now + 8 * 3600) + stale_singleton_at = _jwt("acct-A", "user-A", now + 3600) + _write_store(home, {"access_token": stale_singleton_at, "refresh_token": "rt-consumed"}, _iso(now - 7200), + {"access_token": alias_at, "refresh_token": "rt-alias", "last_refresh": _iso(now - 60)}) + pool = load_pool("openai-codex") + alias = next(e for e in pool.entries() if e.id == "manual") + + synced = pool._sync_entry_from_auth_store(alias) + assert (synced.access_token, synced.refresh_token) == (alias_at, "rt-alias") + + # The user re-authenticates the singleton (newer stamp, same principal): the alias follows. + fresh_at = _jwt("acct-A", "user-A", now + 9 * 3600) + _write_store(home, {"access_token": fresh_at, "refresh_token": "rt-fresh"}, _iso(now + 5), + {"access_token": alias_at, "refresh_token": "rt-alias", "last_refresh": _iso(now - 60)}) + synced = load_pool("openai-codex")._sync_entry_from_auth_store(alias) + assert (synced.access_token, synced.refresh_token) == (fresh_at, "rt-fresh") diff --git a/tests/agent/test_credential_pool_singleton_replay.py b/tests/agent/test_credential_pool_singleton_replay.py deleted file mode 100644 index e69999cbc0..0000000000 --- a/tests/agent/test_credential_pool_singleton_replay.py +++ /dev/null @@ -1,313 +0,0 @@ -"""Regression tests for #106705: Codex manual:device_code pool entries must -not replay an already-consumed refresh token by re-adopting a stale singleton. - -Causal chain (agent/credential_pool.py): - -1. ``_sync_entry_from_auth_store`` adopts differing singleton tokens for BOTH - ``device_code`` and ``manual:device_code`` Codex entries with no staleness - proof. -2. A successful pool-side rotation persists the fresh chain into the - credential-pool store, but ``_sync_device_code_entry_to_auth_store`` - deliberately skips singleton write-back for ``manual:*`` sources - (independent-credential contract, #39236), so the singleton stays one - rotation behind. -3. The next refresh syncs that STALE singleton over the pool's fresh entry and - POSTs the consumed refresh token again -> ``refresh_token_reused``. - -The fix gates adoption on the singleton being provably newer (``last_refresh`` -comparison); these tests run the real production path (``load_pool`` -> -``_refresh_entry``) with only the HTTP transport boundary mocked. -""" - -from __future__ import annotations - -import base64 -import json -from datetime import datetime, timezone - -import pytest - -from agent.credential_pool import load_pool - -# Synthetic JWT-ish access tokens carrying a far-future expiry claim so -# token-expiry probes see a valid token and the refresh path is driven by -# ``force`` alone. -_FAR_FUTURE_EXP = 4102444800 # 2100-01-01 - - -def _jwt(exp: int = _FAR_FUTURE_EXP) -> str: - def _part(payload: dict) -> str: - raw = json.dumps(payload, separators=(",", ":")).encode("utf-8") - return base64.urlsafe_b64encode(raw).decode("ascii").rstrip("=") - - return f"{_part({'alg': 'none', 'typ': 'JWT'})}.{_part({'exp': exp})}.sig" - - -def _iso(seconds: float) -> str: - return datetime.fromtimestamp(seconds, tz=timezone.utc).isoformat().replace("+00:00", "Z") - - -# Rotation chains: old -> new1 -> new2. Timestamps strictly increase so the -# ``last_refresh`` ordering is provable. -_T_OLD = 1_800_000_000.0 -_T_NEW1 = _T_OLD + 600.0 - -_AT_OLD, _RT_OLD = _jwt(), "rt-old" -_AT_NEW1, _RT_NEW1 = _jwt(), "rt-new1" -_AT_NEW2, _RT_NEW2 = _jwt(), "rt-new2" - - -def _manual_entry_payload(id: str = "manual-1", access_token: str = _AT_OLD, - refresh_token: str = _RT_OLD, last_refresh=None) -> dict: - return { - "id": id, - "label": "manual codex grant", - "auth_type": "oauth", - "priority": 0, - "source": "manual:device_code", - "access_token": access_token, - "refresh_token": refresh_token, - "last_refresh": last_refresh, - } - - -def _store(provider_state: dict, pool_entries: list) -> dict: - return { - "version": 1, - "providers": {"openai-codex": provider_state}, - "credential_pool": {"openai-codex": pool_entries}, - } - - -def _tokens_state(access_token: str, refresh_token: str, last_refresh) -> dict: - return { - "tokens": {"access_token": access_token, "refresh_token": refresh_token}, - "last_refresh": last_refresh, - } - - -def _write_store(tmp_path, monkeypatch, store: dict) -> None: - home = tmp_path / "hermes" - home.mkdir(parents=True, exist_ok=True) - (home / "auth.json").write_text(json.dumps(store, indent=2)) - monkeypatch.setenv("HERMES_HOME", str(home)) - - -def _install_fake_refresh(monkeypatch, chains: dict, stamp=_T_NEW1 + 600.0): - """Mock only the HTTP transport; record every refresh_token POSTed.""" - posted = [] - - def fake_refresh(access_token, refresh_token, **kwargs): - posted.append(refresh_token) - if refresh_token not in chains: - raise AssertionError(f"unexpected refresh POST for {refresh_token!r}") - at, rt = chains[refresh_token] - return {"access_token": at, "refresh_token": rt, "last_refresh": _iso(stamp)} - - monkeypatch.setattr( - "agent.credential_pool.auth_mod.refresh_codex_oauth_pure", fake_refresh - ) - return posted - - -class TestPoolRotationDoesNotReplayConsumedRefreshToken: - """L1 + L6: after a pool-side rotation, the stale singleton must not win.""" - - def test_second_forced_refresh_uses_rotated_chain_not_stale_singleton(self, tmp_path, monkeypatch): - """L1 — the reporter's exact scenario. - - Entry holds the chain rotated by the pool (new1, stamped new1-time); - the singleton still holds the consumed old chain (stamped old-time — - write-back deliberately skipped for manual sources). A second forced - refresh must POST the entry's own new1 refresh token — NOT re-adopt - the stale singleton and replay old. - """ - _write_store(tmp_path, monkeypatch, _store( - _tokens_state(_AT_OLD, _RT_OLD, _iso(_T_OLD)), - [_manual_entry_payload( - access_token=_AT_NEW1, refresh_token=_RT_NEW1, last_refresh=_iso(_T_NEW1), - )], - )) - posted = _install_fake_refresh(monkeypatch, {_RT_NEW1: (_AT_NEW2, _RT_NEW2)}) - - pool = load_pool("openai-codex") - updated = pool._refresh_entry(pool._entries[0], force=True) - - assert updated is not None - assert updated.refresh_token == _RT_NEW2 - # The consumed old refresh token must never be re-POSTed. - assert posted == [_RT_NEW1] - - def test_recover_path_does_not_adopt_stale_singleton_after_failed_post(self, tmp_path, monkeypatch): - """L6 — failed-POST recovery must not regress the fresh chain. - - After the pool rotated to new1 (write-back skipped), a refresh POST - that fails must NOT make ``_recover_failed_refresh`` adopt the stale - old singleton as "newer tokens" — that regression is exactly the - replay loop. - """ - _write_store(tmp_path, monkeypatch, _store( - _tokens_state(_AT_OLD, _RT_OLD, _iso(_T_OLD)), - [_manual_entry_payload( - access_token=_AT_NEW1, refresh_token=_RT_NEW1, last_refresh=_iso(_T_NEW1), - )], - )) - - def failing_refresh(access_token, refresh_token, **kwargs): - raise RuntimeError("synthetic network failure") - - monkeypatch.setattr( - "agent.credential_pool.auth_mod.refresh_codex_oauth_pure", failing_refresh - ) - - pool = load_pool("openai-codex") - pool._refresh_entry(pool._entries[0], force=True) - - # The entry must not be regressed onto the stale singleton chain. - entry_now = next(e for e in pool._entries if e.id == "manual-1") - assert entry_now.refresh_token == _RT_NEW1 - assert entry_now.access_token == _AT_NEW1 - - -class TestFreshSingletonAdoptionStillWorks: - """L2 — #70111 regression guard: a provably NEWER singleton must win.""" - - def test_newer_singleton_is_adopted_over_stale_pool_entry(self, tmp_path, monkeypatch): - """The fleet-outage fix (commit 7380b48589) must keep working. - - The singleton was rotated by another process (e.g. ``hermes model`` - re-auth); the pool entry still holds the old consumed chain. The sync - must adopt the newer singleton and refresh with IT. - """ - _write_store(tmp_path, monkeypatch, _store( - _tokens_state(_AT_NEW1, _RT_NEW1, _iso(_T_NEW1)), - [_manual_entry_payload( - access_token=_AT_OLD, refresh_token=_RT_OLD, last_refresh=_iso(_T_OLD), - )], - )) - posted = _install_fake_refresh(monkeypatch, {_RT_NEW1: (_AT_NEW2, _RT_NEW2)}) - - pool = load_pool("openai-codex") - updated = pool._refresh_entry(pool._entries[0], force=True) - - assert updated is not None - assert updated.refresh_token == _RT_NEW2 - assert posted == [_RT_NEW1] - - def test_refresh_token_only_singleton_is_adopted(self, tmp_path, monkeypatch): - """L2b — refresh_token-only singleton (the consumed-access branch). - - Another process rotated and the access_token was consumed; only the - new refresh_token remains on disk. Adoption must still fire (newer - ``last_refresh``) so the consumed token is not replayed. - """ - _write_store(tmp_path, monkeypatch, _store( - _tokens_state("", _RT_NEW1, _iso(_T_NEW1)), - [_manual_entry_payload( - access_token=_AT_OLD, refresh_token=_RT_OLD, last_refresh=_iso(_T_OLD), - )], - )) - posted = _install_fake_refresh(monkeypatch, {_RT_NEW1: (_AT_NEW2, _RT_NEW2)}) - - pool = load_pool("openai-codex") - updated = pool._refresh_entry(pool._entries[0], force=True) - - assert updated is not None - assert updated.refresh_token == _RT_NEW2 - assert posted == [_RT_NEW1] - - -class TestTimestampEdgeCases: - """L3 — missing timestamps fail open to the historical adopt-on-difference.""" - - def test_missing_entry_timestamp_still_adopts_differing_singleton(self, tmp_path, monkeypatch): - """Entry carries no ``last_refresh`` (pre-stamping pool writer). - - Old behavior (adopt on difference) must be preserved so a fresh - re-auth is never stranded when the entry side has no timestamp. - """ - _write_store(tmp_path, monkeypatch, _store( - _tokens_state(_AT_NEW1, _RT_NEW1, _iso(_T_NEW1)), - [_manual_entry_payload( - access_token=_AT_OLD, refresh_token=_RT_OLD, last_refresh=None, - )], - )) - posted = _install_fake_refresh(monkeypatch, {_RT_NEW1: (_AT_NEW2, _RT_NEW2)}) - - pool = load_pool("openai-codex") - updated = pool._refresh_entry(pool._entries[0], force=True) - - assert updated is not None - assert updated.refresh_token == _RT_NEW2 - assert posted == [_RT_NEW1] - - def test_missing_singleton_timestamp_still_adopts_differing_singleton(self, tmp_path, monkeypatch): - """Singleton carries no ``last_refresh`` (legacy auth.json writer). - - Cannot prove the singleton older -> fall back to adopt-on-difference - so a fresh re-auth from a legacy writer is not stranded either. - """ - _write_store(tmp_path, monkeypatch, _store( - _tokens_state(_AT_NEW1, _RT_NEW1, None), - [_manual_entry_payload( - access_token=_AT_OLD, refresh_token=_RT_OLD, last_refresh=_iso(_T_OLD), - )], - )) - posted = _install_fake_refresh(monkeypatch, {_RT_NEW1: (_AT_NEW2, _RT_NEW2)}) - - pool = load_pool("openai-codex") - updated = pool._refresh_entry(pool._entries[0], force=True) - - assert updated is not None - assert updated.refresh_token == _RT_NEW2 - assert posted == [_RT_NEW1] - - -class TestDeviceCodeSeededEntries: - """L4 — the singleton-seeded source is unaffected by the guard.""" - - def test_device_code_entry_rotates_and_writeback_converges(self, tmp_path, monkeypatch): - _write_store(tmp_path, monkeypatch, _store( - _tokens_state(_AT_OLD, _RT_OLD, _iso(_T_OLD)), - [], - )) - posted = _install_fake_refresh(monkeypatch, {_RT_OLD: (_AT_NEW1, _RT_NEW1)}) - - pool = load_pool("openai-codex") - seeded = [e for e in pool._entries if e.source == "device_code"] - assert seeded, "device_code entry must be seeded from the singleton" - - updated = pool._refresh_entry(seeded[0], force=True) - assert updated is not None - assert updated.refresh_token == _RT_NEW1 - assert posted == [_RT_OLD] - - # Write-back must converge the singleton onto the fresh chain. - on_disk = json.loads((tmp_path / "hermes" / "auth.json").read_text()) - synced = on_disk["providers"]["openai-codex"]["tokens"] - assert synced["refresh_token"] == _RT_NEW1 - - -class TestIndependentAccountNotClobbered: - """L5 — #39236 guard: independent manual grants keep their own chain.""" - - def test_independent_manual_entry_with_newer_own_timestamp_is_not_overwritten(self, tmp_path, monkeypatch): - """An independent account's entry whose own chain is NEWER (rotated - by this pool) must never be overwritten by the older singleton — - same mechanism as L1, asserted as the #39236 no-clobber contract. - """ - _write_store(tmp_path, monkeypatch, _store( - _tokens_state(_AT_OLD, _RT_OLD, _iso(_T_OLD)), - [_manual_entry_payload( - id="indep-1", - access_token=_AT_NEW1, refresh_token=_RT_NEW1, last_refresh=_iso(_T_NEW1), - )], - )) - posted = _install_fake_refresh(monkeypatch, {_RT_NEW1: (_AT_NEW2, _RT_NEW2)}) - - pool = load_pool("openai-codex") - updated = pool._refresh_entry(pool._entries[0], force=True) - - assert updated is not None - assert updated.refresh_token == _RT_NEW2 - assert _RT_OLD not in posted