From 0764163f804a2aaebff5f1fe76ed0a71960bbc8d Mon Sep 17 00:00:00 2001 From: Shannon Sands Date: Mon, 13 Jul 2026 13:53:38 +1000 Subject: [PATCH] fix(auth): stop JWT refreshes from status polling --- hermes_cli/auth.py | 49 ++--- tests/hermes_cli/test_auth_nous_provider.py | 7 +- .../hermes_cli/test_nous_session_validity.py | 198 +++++++++++------- 3 files changed, 154 insertions(+), 100 deletions(-) diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 81bf0e897bd5..037bf04034ec 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -2328,7 +2328,7 @@ def _log_nous_invoke_jwt_selected( access_token: Any, sequence_id: Optional[str] = None, ) -> None: - logger.info("Nous inference auth: using NAS invoke JWT") + logger.debug("Nous inference auth: using NAS invoke JWT") _oauth_trace( "nous_invoke_jwt_selected", sequence_id=sequence_id, @@ -6540,7 +6540,9 @@ def get_nous_session_validity() -> str: non-terminal error). Never triggers a re-mint. Determinable with NO working token — it reads local auth-store state only, - which is exactly the condition a dead hosted box is in. + which is exactly the condition a dead hosted box is in. This function is + called by the frequently-polled public ``/api/status`` endpoint, so it must + never resolve credentials or perform an OAuth refresh. ANTI-FLAP CONTRACT: only a *terminal* failure maps to "terminal". A normal mid-rotation blip, a transient network error, or a merely-expiring token @@ -6557,34 +6559,29 @@ def get_nous_session_validity() -> str: try: state = get_provider_auth_state("nous") except Exception: - state = None - - if state: - last_err = state.get("last_auth_error") - if isinstance(last_err, dict) and last_err.get("relogin_required"): - # Only terminal while there is no usable credential left. If a later - # successful login repopulated tokens, the stale marker must not - # keep reporting terminal. - if not (state.get("access_token") or state.get("refresh_token")): - return NOUS_SESSION_TERMINAL - - try: - status = get_nous_auth_status() - except Exception: - # Status computation itself failed — indeterminate, not terminal. return NOUS_SESSION_UNKNOWN - if status.get("logged_in"): + if not state: + return NOUS_SESSION_UNKNOWN + + last_err = state.get("last_auth_error") + if isinstance(last_err, dict) and last_err.get("relogin_required"): + # Only terminal while there is no usable credential left. If a later + # successful login repopulated tokens, the stale marker must not + # keep reporting terminal. + if not (state.get("access_token") or state.get("refresh_token")): + return NOUS_SESSION_TERMINAL + + if _nous_invoke_jwt_status( + state.get("access_token"), + scope=state.get("scope"), + expires_at=state.get("expires_at"), + ) is None: return NOUS_SESSION_VALID - # Not logged in. Distinguish a terminal (relogin-required) failure from a - # transient / indeterminate one. Only the former is actionable by NAS. - if status.get("relogin_required"): - return NOUS_SESSION_TERMINAL - - # No Nous provider state at all, or a non-terminal not-logged-in condition - # (e.g. a transient refresh error that did not set relogin_required). Treat - # as unknown so a healthy box mid-blip never triggers a re-mint. + # Missing, malformed, expired, or merely expiring credentials are not proof + # of a terminal session. Runtime inference/keepalive paths own refreshes; + # the health endpoint remains side-effect free and reports indeterminate. return NOUS_SESSION_UNKNOWN diff --git a/tests/hermes_cli/test_auth_nous_provider.py b/tests/hermes_cli/test_auth_nous_provider.py index a6505dd589ab..20867a8c1585 100644 --- a/tests/hermes_cli/test_auth_nous_provider.py +++ b/tests/hermes_cli/test_auth_nous_provider.py @@ -613,13 +613,18 @@ def test_nous_inference_auth_logs_do_not_include_secret_values( monkeypatch.setattr(auth_mod, "_refresh_access_token", _fake_refresh_access_token) - caplog.set_level(logging.INFO, logger="hermes_cli.auth") + caplog.set_level(logging.DEBUG, logger="hermes_cli.auth") auth_mod.resolve_nous_runtime_credentials( force_refresh=True, ) logged = caplog.text assert "using NAS invoke JWT" in logged + assert not any( + record.levelno >= logging.INFO + and "using NAS invoke JWT" in record.getMessage() + for record in caplog.records + ) assert token not in logged assert refreshed_token not in logged assert refresh_token not in logged diff --git a/tests/hermes_cli/test_nous_session_validity.py b/tests/hermes_cli/test_nous_session_validity.py index 02673d6ad783..2c5adaf68bf4 100644 --- a/tests/hermes_cli/test_nous_session_validity.py +++ b/tests/hermes_cli/test_nous_session_validity.py @@ -1,11 +1,8 @@ -"""Tests for get_nous_session_validity — the /api/status classifier NAS reads -to decide whether to re-mint a hosted-agent bootstrap session. +"""Tests for the local-only Nous session classifier exposed on /api/status.""" -The anti-flap contract is the load-bearing property: only a *terminal* auth -failure may report "terminal" (a spurious "terminal" triggers an unnecessary -NAS re-mint + machine restart on a healthy box). A mid-rotation blip, a -transient error, or a merely-expiring token must NOT report "terminal". -""" +import base64 +import json +import time import hermes_cli.auth as auth from hermes_cli.auth import ( @@ -16,89 +13,144 @@ from hermes_cli.auth import ( ) -def _clear_cache(): - auth.invalidate_nous_auth_status_cache() +def _invoke_jwt(*, seconds: int = 3600) -> str: + def _encode(value: dict) -> str: + raw = json.dumps(value, separators=(",", ":")).encode() + return base64.urlsafe_b64encode(raw).rstrip(b"=").decode() + + return ".".join( + ( + _encode({"alg": "none", "typ": "JWT"}), + _encode( + { + "sub": "test-user", + "scope": auth.DEFAULT_NOUS_SCOPE, + "exp": int(time.time() + seconds), + } + ), + "signature", + ) + ) -def test_valid_when_logged_in(monkeypatch): - """A healthy login → 'valid'.""" - monkeypatch.setattr(auth, "get_provider_auth_state", lambda p: { - "access_token": "at", "refresh_token": "rt", - }) - monkeypatch.setattr(auth, "get_nous_auth_status", lambda: {"logged_in": True}) +def _fail_if_live_auth_is_used(*args, **kwargs): + raise AssertionError("session validity must not resolve or refresh credentials") + + +def _block_live_auth(monkeypatch): + monkeypatch.setattr(auth, "get_nous_auth_status", _fail_if_live_auth_is_used) + monkeypatch.setattr( + auth, + "resolve_nous_runtime_credentials", + _fail_if_live_auth_is_used, + ) + + +def test_valid_when_local_invoke_jwt_is_usable(monkeypatch): + monkeypatch.setattr( + auth, + "get_provider_auth_state", + lambda provider: { + "access_token": _invoke_jwt(), + "refresh_token": "rt", + "scope": auth.DEFAULT_NOUS_SCOPE, + }, + ) + _block_live_auth(monkeypatch) + assert get_nous_session_validity() == NOUS_SESSION_VALID +def test_repeated_status_checks_never_use_live_auth_resolution(monkeypatch): + state = { + "access_token": _invoke_jwt(), + "refresh_token": "rt", + "scope": auth.DEFAULT_NOUS_SCOPE, + } + monkeypatch.setattr(auth, "get_provider_auth_state", lambda provider: state) + _block_live_auth(monkeypatch) + + assert [get_nous_session_validity() for _ in range(10)] == [ + NOUS_SESSION_VALID + ] * 10 + + def test_terminal_on_persisted_quarantine_marker(monkeypatch): - """A persisted last_auth_error.relogin_required with tokens cleared → - 'terminal'. This is the exact on-disk state the incident produced.""" - monkeypatch.setattr(auth, "get_provider_auth_state", lambda p: { - # tokens cleared by the quarantine path - "last_auth_error": {"relogin_required": True, "code": "invalid_grant"}, - }) - # status would also say not-logged-in, but the marker short-circuits first - monkeypatch.setattr(auth, "get_nous_auth_status", lambda: {"logged_in": False}) + monkeypatch.setattr( + auth, + "get_provider_auth_state", + lambda provider: { + "last_auth_error": { + "relogin_required": True, + "code": "invalid_grant", + }, + }, + ) + _block_live_auth(monkeypatch) + assert get_nous_session_validity() == NOUS_SESSION_TERMINAL -def test_terminal_on_relogin_required_status(monkeypatch): - """Not logged in + relogin_required from the live status → 'terminal'.""" - monkeypatch.setattr(auth, "get_provider_auth_state", lambda p: { - "refresh_token": "rt", # present, but status resolution fails terminally - }) - monkeypatch.setattr(auth, "get_nous_auth_status", lambda: { - "logged_in": False, "relogin_required": True, "error_code": "invalid_grant", - }) - assert get_nous_session_validity() == NOUS_SESSION_TERMINAL - - -def test_unknown_when_no_provider_state(monkeypatch): - """No Nous provider state at all → 'unknown' (never terminal).""" - monkeypatch.setattr(auth, "get_provider_auth_state", lambda p: None) - monkeypatch.setattr(auth, "get_nous_auth_status", lambda: {"logged_in": False}) - assert get_nous_session_validity() == NOUS_SESSION_UNKNOWN - - -def test_anti_flap_transient_not_logged_in_is_unknown(monkeypatch): - """ANTI-FLAP: not-logged-in WITHOUT relogin_required (a transient/network - blip) must be 'unknown', NOT 'terminal' — otherwise a healthy box mid-blip - triggers a spurious re-mint.""" - monkeypatch.setattr(auth, "get_provider_auth_state", lambda p: { - "access_token": "at", "refresh_token": "rt", - }) - monkeypatch.setattr(auth, "get_nous_auth_status", lambda: { - "logged_in": False, "error": "connection reset", # no relogin_required - }) - assert get_nous_session_validity() == NOUS_SESSION_UNKNOWN - - def test_stale_quarantine_marker_ignored_after_relogin(monkeypatch): - """ANTI-FLAP: a leftover last_auth_error marker must NOT report 'terminal' - once a subsequent login has repopulated tokens.""" - monkeypatch.setattr(auth, "get_provider_auth_state", lambda p: { - "access_token": "new-at", "refresh_token": "new-rt", - "last_auth_error": {"relogin_required": True, "code": "invalid_grant"}, - }) - monkeypatch.setattr(auth, "get_nous_auth_status", lambda: {"logged_in": True}) + monkeypatch.setattr( + auth, + "get_provider_auth_state", + lambda provider: { + "access_token": _invoke_jwt(), + "refresh_token": "new-rt", + "scope": auth.DEFAULT_NOUS_SCOPE, + "last_auth_error": { + "relogin_required": True, + "code": "invalid_grant", + }, + }, + ) + _block_live_auth(monkeypatch) + assert get_nous_session_validity() == NOUS_SESSION_VALID -def test_status_exception_is_unknown_not_terminal(monkeypatch): - """If status computation itself throws, that's indeterminate → 'unknown'.""" - monkeypatch.setattr(auth, "get_provider_auth_state", lambda p: {"refresh_token": "rt"}) +def test_expiring_token_is_unknown_without_refreshing(monkeypatch): + monkeypatch.setattr( + auth, + "get_provider_auth_state", + lambda provider: { + "access_token": _invoke_jwt(seconds=30), + "refresh_token": "rt", + "scope": auth.DEFAULT_NOUS_SCOPE, + }, + ) + _block_live_auth(monkeypatch) - def _boom(): - raise RuntimeError("boom") - - monkeypatch.setattr(auth, "get_nous_auth_status", _boom) assert get_nous_session_validity() == NOUS_SESSION_UNKNOWN -def test_provider_state_exception_falls_through_to_status(monkeypatch): - """If reading provider state throws, fall through to status (don't crash).""" - def _boom(p): +def test_invalid_token_is_unknown_without_refreshing(monkeypatch): + monkeypatch.setattr( + auth, + "get_provider_auth_state", + lambda provider: { + "access_token": "not-a-jwt", + "refresh_token": "rt", + }, + ) + _block_live_auth(monkeypatch) + + assert get_nous_session_validity() == NOUS_SESSION_UNKNOWN + + +def test_no_provider_state_is_unknown(monkeypatch): + monkeypatch.setattr(auth, "get_provider_auth_state", lambda provider: None) + _block_live_auth(monkeypatch) + + assert get_nous_session_validity() == NOUS_SESSION_UNKNOWN + + +def test_provider_state_exception_is_unknown(monkeypatch): + def _boom(provider): raise RuntimeError("disk error") monkeypatch.setattr(auth, "get_provider_auth_state", _boom) - monkeypatch.setattr(auth, "get_nous_auth_status", lambda: {"logged_in": True}) - assert get_nous_session_validity() == NOUS_SESSION_VALID + _block_live_auth(monkeypatch) + + assert get_nous_session_validity() == NOUS_SESSION_UNKNOWN