From 414a5bc924ea94b96c46497b68ccfbc01faee406 Mon Sep 17 00:00:00 2001 From: JohnC1009 Date: Mon, 25 May 2026 07:14:19 -0400 Subject: [PATCH] fix(auth): fall back to global auth.json in _load_provider_state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In profile mode, _load_provider_state previously returned None when a provider was absent from the profile's auth.json — even if the user had authenticated at the global root. This broke runtime credential resolvers that read state directly (resolve_nous_access_token, resolve_nous_runtime_credentials), causing profiles without their own nous login to fail with 'Hermes is not logged into Nous Portal' despite a valid global session. Push the existing read-only global fallback (already used by get_provider_auth_state and read_credential_pool) into _load_provider_state so every caller benefits, and simplify get_provider_auth_state into a thin wrapper. Writes still target the profile only — profile state continues to shadow global state on the next read after a per-profile login. Behavior in classic (non-profile) mode is unchanged because _load_global_auth_store returns an empty dict. Adds 5 tests covering the new contract on _load_provider_state directly. Existing 770 auth/credential/nous tests still pass. --- hermes_cli/auth.py | 46 +++++++--- .../hermes_cli/test_auth_profile_fallback.py | 92 +++++++++++++++++++ 2 files changed, 123 insertions(+), 15 deletions(-) diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 87069b3de8d..dd2a17e5f44 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -1125,11 +1125,32 @@ def _save_auth_store(auth_store: Dict[str, Any]) -> Path: def _load_provider_state(auth_store: Dict[str, Any], provider_id: str) -> Optional[Dict[str, Any]]: + """Return a provider's persisted state. + + In profile mode, falls back to the global-root ``auth.json`` when the + profile has no entry for ``provider_id``. This mirrors the per-provider + shadowing already used by ``read_credential_pool``: workers spawned in a + profile can see providers (e.g. ``nous``) that were only authenticated at + global scope. Once the user runs ``hermes auth login `` inside + the profile, the profile state fully shadows the global state on the next + read. See issue #18594 follow-up. + """ providers = auth_store.get("providers") - if not isinstance(providers, dict): - return None - state = providers.get(provider_id) - return dict(state) if isinstance(state, dict) else None + if isinstance(providers, dict): + state = providers.get(provider_id) + if isinstance(state, dict): + return dict(state) + + # Read-only fallback to the global-root auth store (profile mode only; + # returns empty dict in classic mode so this is a no-op). + global_store = _load_global_auth_store() + if global_store: + global_providers = global_store.get("providers") + if isinstance(global_providers, dict): + global_state = global_providers.get(provider_id) + if isinstance(global_state, dict): + return dict(global_state) + return None def _save_provider_state(auth_store: Dict[str, Any], provider_id: str, state: Dict[str, Any]) -> None: @@ -1283,23 +1304,18 @@ def unsuppress_credential_source(provider_id: str, source: str) -> bool: def get_provider_auth_state(provider_id: str) -> Optional[Dict[str, Any]]: """Return persisted auth state for a provider, or None. - In profile mode, falls back to the global-root ``auth.json`` when the - profile has no state for this provider. Profile state always wins when - present. Writes (``_save_auth_store`` / ``persist_*_credentials``) are - unchanged — they still target the profile only. This mirrors + In profile mode, ``_load_provider_state`` already falls back to the + global-root ``auth.json`` per-provider when the profile has no entry — + so this is now a thin convenience wrapper. Profile state always wins + when present. Writes (``_save_auth_store`` / ``persist_*_credentials``) + are unchanged — they still target the profile only. This mirrors ``read_credential_pool``'s per-provider shadowing semantics so that ``_seed_from_singletons`` can reseed a profile's credential pool from global-scope provider state (e.g. a globally-authenticated Anthropic OAuth or Nous device-code session). See issue #18594 follow-up. """ auth_store = _load_auth_store() - state = _load_provider_state(auth_store, provider_id) - if state is not None: - return state - global_store = _load_global_auth_store() - if not global_store: - return None - return _load_provider_state(global_store, provider_id) + return _load_provider_state(auth_store, provider_id) def get_active_provider() -> Optional[str]: diff --git a/tests/hermes_cli/test_auth_profile_fallback.py b/tests/hermes_cli/test_auth_profile_fallback.py index 2063517d28c..5210404c40e 100644 --- a/tests/hermes_cli/test_auth_profile_fallback.py +++ b/tests/hermes_cli/test_auth_profile_fallback.py @@ -275,6 +275,98 @@ def test_provider_auth_state_returns_none_when_neither_has_it(profile_env): assert get_provider_auth_state("nous") is None +# --------------------------------------------------------------------------- +# _load_provider_state — internal global fallback (issue #18594 follow-up) +# +# Several runtime helpers (notably ``resolve_nous_runtime_credentials`` and +# ``resolve_nous_access_token``) call ``_load_provider_state`` directly with +# a profile-loaded auth store rather than going through +# ``get_provider_auth_state``. Without the fallback wired into +# ``_load_provider_state`` itself, those helpers raise ``"Hermes is not +# logged into Nous Portal"`` even though the user has a valid global Nous +# login. These tests pin the per-provider shadowing into the helper. +# --------------------------------------------------------------------------- + + +def test_load_provider_state_falls_back_to_global(profile_env): + """When the loaded profile store has no provider entry, fall back to global.""" + from hermes_cli.auth import _load_auth_store, _load_provider_state + + _write(profile_env["global"] / "auth.json", _make_auth_store(providers={ + "nous": {"access_token": "global-nous-token", "refresh_token": "rt"}, + })) + _write(profile_env["profile"] / "auth.json", _make_auth_store(providers={})) + + auth_store = _load_auth_store() + state = _load_provider_state(auth_store, "nous") + assert state is not None + assert state["access_token"] == "global-nous-token" + + +def test_load_provider_state_profile_wins_over_global(profile_env): + from hermes_cli.auth import _load_auth_store, _load_provider_state + + _write(profile_env["global"] / "auth.json", _make_auth_store(providers={ + "nous": {"access_token": "global-token"}, + })) + _write(profile_env["profile"] / "auth.json", _make_auth_store(providers={ + "nous": {"access_token": "profile-token"}, + })) + + auth_store = _load_auth_store() + state = _load_provider_state(auth_store, "nous") + assert state is not None + assert state["access_token"] == "profile-token" + + +def test_load_provider_state_returns_none_when_neither_has_it(profile_env): + from hermes_cli.auth import _load_auth_store, _load_provider_state + + _write(profile_env["global"] / "auth.json", _make_auth_store(providers={})) + _write(profile_env["profile"] / "auth.json", _make_auth_store(providers={})) + + auth_store = _load_auth_store() + assert _load_provider_state(auth_store, "nous") is None + + +def test_load_provider_state_classic_mode_no_fallback(tmp_path, monkeypatch): + """In classic mode there is no global to fall back to; behavior is unchanged.""" + fake_home = tmp_path / "home" + fake_home.mkdir() + monkeypatch.setattr(Path, "home", lambda: fake_home) + hermes_home = tmp_path / "classic" + hermes_home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + + _write(hermes_home / "auth.json", _make_auth_store(providers={ + "nous": {"access_token": "classic-token"}, + })) + + from hermes_cli.auth import _load_auth_store, _load_provider_state + + auth_store = _load_auth_store() + state = _load_provider_state(auth_store, "nous") + assert state is not None + assert state["access_token"] == "classic-token" + # Absent providers still return None. + assert _load_provider_state(auth_store, "anthropic") is None + + +def test_load_provider_state_malformed_global_does_not_break_profile(profile_env): + """A corrupt global auth.json must not break profile reads.""" + (profile_env["global"] / "auth.json").write_text("{not valid json") + _write(profile_env["profile"] / "auth.json", _make_auth_store(providers={ + "nous": {"access_token": "profile-token"}, + })) + + from hermes_cli.auth import _load_auth_store, _load_provider_state + + auth_store = _load_auth_store() + state = _load_provider_state(auth_store, "nous") + assert state is not None + assert state["access_token"] == "profile-token" + + # --------------------------------------------------------------------------- # Classic mode — no fallback path should ever trigger # ---------------------------------------------------------------------------