From 7e958dafc2185678532137314596083b9f588c68 Mon Sep 17 00:00:00 2001 From: Robin Fernandes Date: Thu, 28 May 2026 19:26:30 +1000 Subject: [PATCH] fix(auth): address Nous JWT fallback review --- agent/credential_pool.py | 16 ++++++++- hermes_cli/auth.py | 37 ++++++--------------- hermes_cli/proxy/adapters/nous_portal.py | 18 ++++++++-- tests/agent/test_credential_pool.py | 23 +++++++++++++ tests/hermes_cli/test_auth_nous_provider.py | 19 ++++++----- tests/hermes_cli/test_proxy.py | 24 ++++++++----- 6 files changed, 90 insertions(+), 47 deletions(-) diff --git a/agent/credential_pool.py b/agent/credential_pool.py index f5ebafd8e..8dd5472f2 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -204,7 +204,21 @@ class PooledCredential: if self.provider == "nous": # Nous stores the runtime inference credential in agent_key for # compatibility. It must be a NAS invoke JWT. - return str(self.agent_key or self.access_token or "") + for token, expires_at in ( + (self.agent_key, self.agent_key_expires_at), + (self.access_token, self.expires_at), + ): + if ( + isinstance(token, str) + and token.strip() + and auth_mod._nous_invoke_jwt_is_usable( + token, + scope=getattr(self, "scope", None), + expires_at=expires_at, + ) + ): + return token.strip() + return "" return str(self.access_token or "") @property diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 38f9e604a..4beade005 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -1791,30 +1791,19 @@ def _nous_invoke_jwt_is_usable( ) -def _choose_nous_inference_auth_path( +def _assert_nous_inference_jwt_usable( state: Dict[str, Any], *, access_token: Any = None, - min_key_ttl_seconds: int = DEFAULT_AGENT_KEY_MIN_TTL_SECONDS, - inference_auth_mode: str = NOUS_INFERENCE_AUTH_MODE_AUTO, -) -> Tuple[str, Optional[str]]: - del min_key_ttl_seconds - _normalize_nous_inference_auth_mode(inference_auth_mode) +) -> None: token = state.get("access_token") if access_token is None else access_token - if _nous_invoke_jwt_is_usable( + reason = _nous_invoke_jwt_status( token, scope=state.get("scope"), expires_at=state.get("expires_at"), - ): - return NOUS_AUTH_PATH_INVOKE_JWT, None - reason = ( - _nous_invoke_jwt_status( - token, - scope=state.get("scope"), - expires_at=state.get("expires_at"), - ) - or "invoke_jwt_unavailable" ) + if reason is None: + return raise AuthError( "Nous Portal access token is not a usable inference JWT " f"({reason}). Re-authenticate with: hermes auth add nous", @@ -5058,12 +5047,8 @@ def refresh_nous_oauth_pure( if on_state_update is not None: on_state_update(dict(state), "post_refresh_access_token") - selected_auth_path, _ = _choose_nous_inference_auth_path( - state, - inference_auth_mode=inference_auth_mode, - ) - if selected_auth_path == NOUS_AUTH_PATH_INVOKE_JWT: - _select_nous_invoke_jwt(state) + _assert_nous_inference_jwt_usable(state) + _select_nous_invoke_jwt(state) return state @@ -5266,7 +5251,6 @@ def resolve_nous_runtime_credentials( refresh_token_fp=_token_fingerprint(state.get("refresh_token")), ) - selected_auth_path = NOUS_AUTH_PATH_INVOKE_JWT with httpx.Client(timeout=timeout, headers={"Accept": "application/json"}, verify=verify) as client: access_token = state.get("access_token") refresh_token = state.get("refresh_token") @@ -5357,10 +5341,9 @@ def resolve_nous_runtime_credentials( # Persist immediately so validation failures cannot drop rotated refresh tokens. _persist_state("post_refresh_access_token") - selected_auth_path, _ = _choose_nous_inference_auth_path( + _assert_nous_inference_jwt_usable( state, access_token=access_token, - inference_auth_mode=inference_auth_mode, ) _select_nous_invoke_jwt( state, @@ -5403,7 +5386,7 @@ def resolve_nous_runtime_credentials( "expires_at": expires_at, "expires_in": expires_in, "source": NOUS_AUTH_PATH_INVOKE_JWT, - "auth_path": selected_auth_path, + "auth_path": NOUS_AUTH_PATH_INVOKE_JWT, } @@ -5448,7 +5431,7 @@ def _snapshot_nous_pool_status() -> Dict[str, Any]: return (agent_exp, access_exp, -priority) entry = max(entries, key=_entry_sort_key) - runtime_key = getattr(entry, "runtime_api_key", None) or getattr(entry, "access_token", "") + runtime_key = getattr(entry, "runtime_api_key", None) if not runtime_key: return _empty_nous_auth_status() access_token = getattr(entry, "access_token", None) diff --git a/hermes_cli/proxy/adapters/nous_portal.py b/hermes_cli/proxy/adapters/nous_portal.py index 0d06fd545..064708a67 100644 --- a/hermes_cli/proxy/adapters/nous_portal.py +++ b/hermes_cli/proxy/adapters/nous_portal.py @@ -84,10 +84,21 @@ class NousPortalAdapter(UpstreamAdapter): failed_credential: UpstreamCredential, status_code: int, ) -> Optional[UpstreamCredential]: - _ = failed_credential, status_code - return None + _ = failed_credential + if status_code != 401: + return None + logger.info("proxy: Nous upstream rejected bearer; force-refreshing invoke JWT") + return self._get_credential( + inference_auth_mode=NOUS_INFERENCE_AUTH_MODE_AUTO, + force_refresh=True, + ) - def _get_credential(self, *, inference_auth_mode: str) -> UpstreamCredential: + def _get_credential( + self, + *, + inference_auth_mode: str, + force_refresh: bool = False, + ) -> UpstreamCredential: with self._lock: state = self._read_state() if state is None: @@ -98,6 +109,7 @@ class NousPortalAdapter(UpstreamAdapter): try: refreshed = resolve_nous_runtime_credentials( inference_auth_mode=inference_auth_mode, + force_refresh=force_refresh, ) except AuthError as exc: if _is_terminal_nous_refresh_error(exc): diff --git a/tests/agent/test_credential_pool.py b/tests/agent/test_credential_pool.py index 2f46b273b..22a4de6d5 100644 --- a/tests/agent/test_credential_pool.py +++ b/tests/agent/test_credential_pool.py @@ -1293,6 +1293,29 @@ def test_load_pool_mirrors_nous_invoke_jwt_agent_key_runtime_api_key(tmp_path, m assert pool_entry["agent_key_expires_at"] == expires_at +def test_nous_runtime_api_key_rejects_opaque_agent_key(): + from agent.credential_pool import PooledCredential + + entry = PooledCredential( + provider="nous", + id="nous-opaque", + label="opaque", + auth_type="oauth", + priority=0, + source="device_code", + access_token="opaque-access-token", + refresh_token="refresh-token", + agent_key="opaque-agent-key", + agent_key_expires_at=datetime.fromtimestamp( + time.time() + 3600, + tz=timezone.utc, + ).isoformat(), + extra={"scope": "inference:invoke"}, + ) + + assert entry.runtime_api_key == "" + + def test_nous_pool_terminal_refresh_removes_device_code_entry(tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) monkeypatch.setenv("HERMES_SHARED_AUTH_DIR", str(tmp_path / "shared")) diff --git a/tests/hermes_cli/test_auth_nous_provider.py b/tests/hermes_cli/test_auth_nous_provider.py index 453a0d19e..efb5ce47f 100644 --- a/tests/hermes_cli/test_auth_nous_provider.py +++ b/tests/hermes_cli/test_auth_nous_provider.py @@ -485,7 +485,7 @@ def test_nous_device_code_login_does_not_retry_legacy_scope_when_invoke_refused( assert scopes == [auth_mod.DEFAULT_NOUS_SCOPE] -def test_legacy_session_env_is_ignored_for_invoke_scope_and_jwt_storage(tmp_path, monkeypatch): +def test_removed_legacy_session_env_var_does_not_change_jwt_auth(tmp_path, monkeypatch): import hermes_cli.auth as auth_mod hermes_home = tmp_path / "hermes" @@ -609,13 +609,16 @@ def test_get_nous_auth_status_checks_credential_pool(tmp_path, monkeypatch): # Seed the credential pool with a Nous entry from agent.credential_pool import PooledCredential, load_pool pool = load_pool("nous") + token = _invoke_jwt(seconds=3600) + expires_at = _future_iso(3600) entry = PooledCredential.from_dict("nous", { - "access_token": "test-access-token", + "access_token": token, "refresh_token": "test-refresh-token", "portal_base_url": "https://portal.example.com", "inference_base_url": "https://inference.example.com/v1", - "agent_key": "test-agent-key", - "agent_key_expires_at": "2099-01-01T00:00:00+00:00", + "agent_key": token, + "agent_key_expires_at": expires_at, + "scope": "inference:invoke", "label": "dashboard device_code", "auth_type": "oauth", "source": "manual:dashboard_device_code", @@ -628,7 +631,7 @@ def test_get_nous_auth_status_checks_credential_pool(tmp_path, monkeypatch): assert "example.com" in str(status.get("portal_base_url", "")) -def test_get_nous_auth_status_pool_opaque_key_is_not_portal_login(tmp_path, monkeypatch): +def test_get_nous_auth_status_pool_opaque_key_is_not_inference_credential(tmp_path, monkeypatch): from hermes_cli.auth import get_nous_auth_status, invalidate_nous_auth_status_cache hermes_home = tmp_path / "hermes" @@ -656,11 +659,11 @@ def test_get_nous_auth_status_pool_opaque_key_is_not_portal_login(tmp_path, monk status = get_nous_auth_status() assert status["logged_in"] is False - assert status["inference_credential_present"] is True - assert status["credential_source"] == "pool:manual opaque key" + assert status["inference_credential_present"] is False + assert status["credential_source"] is None assert status.get("access_token") is None assert status.get("portal_base_url") is None - assert status.get("inference_base_url") == "https://inference.example.com/v1" + assert status.get("inference_base_url") is None invalidate_nous_auth_status_cache() diff --git a/tests/hermes_cli/test_proxy.py b/tests/hermes_cli/test_proxy.py index a9eca5b6d..0e2a4efde 100644 --- a/tests/hermes_cli/test_proxy.py +++ b/tests/hermes_cli/test_proxy.py @@ -105,7 +105,7 @@ def test_nous_adapter_authenticated_with_agent_key(tmp_path, monkeypatch): def test_nous_adapter_authenticated_with_refresh_token_only(tmp_path, monkeypatch): - """If access_token+refresh_token exist but no agent_key yet, we can still mint.""" + """If access_token+refresh_token exist but no agent_key yet, we can still refresh.""" monkeypatch.setenv("HERMES_HOME", str(tmp_path)) _write_auth_store(tmp_path, { "access_token": "access-tok", @@ -125,7 +125,7 @@ def test_nous_adapter_get_credential_uses_runtime_resolver(tmp_path, monkeypatch }) refreshed_state = { - "api_key": "minted-bearer", + "api_key": "jwt-bearer", "base_url": "https://inference-api.nousresearch.com/v1", "expires_at": "2099-01-01T00:00:00Z", } @@ -138,13 +138,13 @@ def test_nous_adapter_get_credential_uses_runtime_resolver(tmp_path, monkeypatch cred = adapter.get_credential() mock_resolve.assert_called_once() - assert cred.bearer == "minted-bearer" + assert cred.bearer == "jwt-bearer" assert cred.base_url == "https://inference-api.nousresearch.com/v1" assert cred.expires_at == "2099-01-01T00:00:00Z" assert cred.token_type == "Bearer" -def test_nous_adapter_retry_credential_does_not_fallback_on_jwt_401(tmp_path, monkeypatch): +def test_nous_adapter_retry_credential_force_refreshes_on_jwt_401(tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path)) _write_auth_store(tmp_path, { "access_token": "jwt-access", @@ -154,9 +154,15 @@ def test_nous_adapter_retry_credential_does_not_fallback_on_jwt_401(tmp_path, mo "inference_base_url": "https://inference-api.nousresearch.com/v1", "agent_key": "jwt-access", }) + refreshed_state = { + "api_key": "fresh-jwt-bearer", + "base_url": "https://inference-api.nousresearch.com/v1", + "expires_at": "2099-01-01T00:00:00Z", + } with patch( "hermes_cli.proxy.adapters.nous_portal.resolve_nous_runtime_credentials", + return_value=refreshed_state, ) as mock_resolve: adapter = NousPortalAdapter() cred = adapter.get_retry_credential( @@ -167,11 +173,13 @@ def test_nous_adapter_retry_credential_does_not_fallback_on_jwt_401(tmp_path, mo status_code=401, ) - assert cred is None - mock_resolve.assert_not_called() + assert cred is not None + assert cred.bearer == "fresh-jwt-bearer" + assert mock_resolve.call_args.kwargs["force_refresh"] is True + assert mock_resolve.call_args.kwargs["inference_auth_mode"] == "auto" -def test_nous_adapter_retry_credential_skips_opaque_bearer(tmp_path, monkeypatch): +def test_nous_adapter_retry_credential_skips_non_401(tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path)) _write_auth_store(tmp_path, { "access_token": "jwt-access", @@ -188,7 +196,7 @@ def test_nous_adapter_retry_credential_skips_opaque_bearer(tmp_path, monkeypatch bearer="opaque-bearer", base_url="https://inference-api.nousresearch.com/v1", ), - status_code=401, + status_code=403, ) assert cred is None