From 8d84c543d22b3e91b506440d7ac0798a71095d9c Mon Sep 17 00:00:00 2001 From: Ben Date: Thu, 2 Jul 2026 16:39:41 +1000 Subject: [PATCH] fix(auth): make HERMES_PORTAL_BASE_URL/NOUS_PORTAL_BASE_URL bypass the Portal host allowlist Ben caught that the initial approach (widening _NOUS_PORTAL_ALLOWED_HOSTS to include the staging host) was the wrong fix -- env vars are supposed to override the allowlist, mirroring how NOUS_INFERENCE_BASE_URL already bypasses _ALLOWED_NOUS_INFERENCE_HOSTS via _nous_inference_env_override(). The actual bug: both resolve_nous_access_token and resolve_nous_runtime_credentials read `_optional_base_url(state.get("portal_base_url")) or os.getenv(...) or ...` -- a plain `or` chain where the STORED state value wins first (short-circuits before the env vars are even read), and then whichever value won gets run through the same _NOUS_PORTAL_ALLOWED_HOSTS gate regardless of its source. So a hosted agent stamped with HERMES_PORTAL_BASE_URL= in its env AND a staging portal_base_url already persisted to auth.json would still get silently rewritten to prod on every refresh, because the env var never even got a chance to be consulted. Revert the previous _NOUS_PORTAL_ALLOWED_HOSTS widening entirely -- staying prod-only preserves the allowlist's actual job (rejecting an untrusted network-provided portal_base_url persisted to auth.json by a compromised Portal response). Add _nous_portal_env_override() (mirrors _nous_inference_env_override()) and restructure both call sites so the env override is checked FIRST and, when set, wins outright and skips the allowlist gate entirely -- the allowlist only ever runs against the fallback (stored-state-or-default) path now. Rewrote tests/hermes_cli/test_nous_portal_staging_allowlist.py to test the actual fix: the helper function, and an end-to-end resolve_nous_access_token proof that the env override wins even when state ALSO has the staging host stored (the exact incident shape), that it wins over a stored PROD host too, and that the allowlist's heal-to-prod behaviour for an untrusted stored value is preserved when no override is set. --- hermes_cli/auth.py | 92 +++++--- .../test_nous_portal_staging_allowlist.py | 206 ++++++++++++------ 2 files changed, 202 insertions(+), 96 deletions(-) diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 101224cf82d..fdd099bfa46 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -1833,18 +1833,6 @@ _NOUS_STALE_PORTAL_HOSTS: FrozenSet[str] = frozenset({ # "localhost" / "127.0.0.1" are valid for local development and testing. _NOUS_PORTAL_ALLOWED_HOSTS: FrozenSet[str] = frozenset({ "portal.nousresearch.com", - # Staging portal — hosted agents provisioned by nous-account-service on the - # `staging` Vercel environment persist this host to auth.json's - # portal_base_url (see buildBootstrapAuthJson / env.BASE_URL there). Without - # it, resolve_nous_access_token's allowlist guard silently rewrites - # portal_base_url back to prod on the very first refresh, so a - # staging-issued refresh token gets replayed against the PROD token - # endpoint. Prod correctly rejects it with invalid_grant, which then - # triggers _quarantine_nous_oauth_state and wipes the credential pool - # entirely — turning a config mismatch into a full relogin requirement. - # Same failure shape as the NOUS_INFERENCE_BASE_URL/_ALLOWED_NOUS_INFERENCE_HOSTS - # fix below; this closes the sibling gap on the portal host. - "portal.staging-nousresearch.com", "localhost", "127.0.0.1", }) @@ -1938,6 +1926,28 @@ def _nous_inference_env_override() -> Optional[str]: return _optional_base_url(os.getenv("NOUS_INFERENCE_BASE_URL")) +def _nous_portal_env_override() -> Optional[str]: + """Return the user/deployment-set Portal base URL override, if any. + + Mirrors ``_nous_inference_env_override()``: ``HERMES_PORTAL_BASE_URL`` / + ``NOUS_PORTAL_BASE_URL`` are the documented dev/staging escape hatch for + pointing Hermes at a non-production Nous Portal (e.g. a hosted agent + provisioned on nous-account-service's `staging` environment, which stamps + ``HERMES_PORTAL_BASE_URL=https://portal.staging-nousresearch.com`` into + the container env). The env source is trusted (the OS user/deployment + set it themselves), so — like the inference override — it must NOT be + gated by ``_NOUS_PORTAL_ALLOWED_HOSTS``: that allowlist exists to reject + an untrusted NETWORK-provided value (a poisoned portal_base_url + persisted to auth.json), not a value the operator explicitly configured. + + Returns a trailing-slash-stripped non-empty string, or ``None`` when + neither env var is set/blank. + """ + return _optional_base_url( + os.getenv("HERMES_PORTAL_BASE_URL") or os.getenv("NOUS_PORTAL_BASE_URL") + ) + + def _decode_jwt_claims(token: Any) -> Dict[str, Any]: if not isinstance(token, str) or token.count(".") != 2: return {} @@ -5398,20 +5408,29 @@ def resolve_nous_access_token( relogin_required=True, ) - portal_base_url = ( - _optional_base_url(state.get("portal_base_url")) - or os.getenv("HERMES_PORTAL_BASE_URL") - or os.getenv("NOUS_PORTAL_BASE_URL") - or DEFAULT_NOUS_PORTAL_URL - ).rstrip("/") + # HERMES_PORTAL_BASE_URL / NOUS_PORTAL_BASE_URL is the trusted + # operator/deployment override (mirrors NOUS_INFERENCE_BASE_URL) and + # must win OUTRIGHT — including over a stored value — and bypass the + # host allowlist entirely, since the allowlist exists to reject an + # untrusted network-provided value, not one the operator configured. + # Only fall through to the stored/default value + allowlist gate when + # no override is set. + env_portal_override = _nous_portal_env_override() + if env_portal_override: + portal_base_url = env_portal_override.rstrip("/") + else: + portal_base_url = ( + _optional_base_url(state.get("portal_base_url")) + or DEFAULT_NOUS_PORTAL_URL + ).rstrip("/") - parsed_portal_url = urlparse(portal_base_url) - if parsed_portal_url.hostname and parsed_portal_url.hostname not in _NOUS_PORTAL_ALLOWED_HOSTS: - logger.warning( - "auth: ignoring invalid portal_base_url %r (host %r not in allowlist), using default", - portal_base_url, parsed_portal_url.hostname, - ) - portal_base_url = DEFAULT_NOUS_PORTAL_URL + parsed_portal_url = urlparse(portal_base_url) + if parsed_portal_url.hostname and parsed_portal_url.hostname not in _NOUS_PORTAL_ALLOWED_HOSTS: + logger.warning( + "auth: ignoring invalid portal_base_url %r (host %r not in allowlist), using default", + portal_base_url, parsed_portal_url.hostname, + ) + portal_base_url = DEFAULT_NOUS_PORTAL_URL client_id = str(state.get("client_id") or DEFAULT_NOUS_CLIENT_ID) verify = _resolve_verify(insecure=insecure, ca_bundle=ca_bundle, auth_state=state) @@ -5730,13 +5749,22 @@ def resolve_nous_runtime_credentials( # A persisted/stale portal_base_url is where the refresh token gets # POSTed on refresh — reject any host outside the allowlist so a # poisoned value can't exfiltrate the bearer, healing to the default. - parsed_portal_url = urlparse(portal_base_url) - if parsed_portal_url.hostname and parsed_portal_url.hostname not in _NOUS_PORTAL_ALLOWED_HOSTS: - logger.warning( - "auth: ignoring invalid portal_base_url %r (host %r not in allowlist), using default", - portal_base_url, parsed_portal_url.hostname, - ) - portal_base_url = DEFAULT_NOUS_PORTAL_URL + # The trusted operator/deployment env override (HERMES_PORTAL_BASE_URL / + # NOUS_PORTAL_BASE_URL) bypasses this gate entirely — mirrors + # NOUS_INFERENCE_BASE_URL's treatment below; the allowlist exists to + # reject an untrusted NETWORK-provided value, not one the operator + # explicitly configured. + env_portal_override = _nous_portal_env_override() + if env_portal_override: + portal_base_url = env_portal_override.rstrip("/") + else: + parsed_portal_url = urlparse(portal_base_url) + if parsed_portal_url.hostname and parsed_portal_url.hostname not in _NOUS_PORTAL_ALLOWED_HOSTS: + logger.warning( + "auth: ignoring invalid portal_base_url %r (host %r not in allowlist), using default", + portal_base_url, parsed_portal_url.hostname, + ) + portal_base_url = DEFAULT_NOUS_PORTAL_URL # Persisted value: validated network-provenance only. The stored # inference_base_url is re-validated on read so a poisoned/stale diff --git a/tests/hermes_cli/test_nous_portal_staging_allowlist.py b/tests/hermes_cli/test_nous_portal_staging_allowlist.py index 0b2958cc482..3153db4ec5f 100644 --- a/tests/hermes_cli/test_nous_portal_staging_allowlist.py +++ b/tests/hermes_cli/test_nous_portal_staging_allowlist.py @@ -1,75 +1,86 @@ -"""Regression tests for the Nous Portal host allowlist accepting staging. +"""Regression tests for the Nous Portal env-override bypassing the host +allowlist, mirroring the existing NOUS_INFERENCE_BASE_URL / +_ALLOWED_NOUS_INFERENCE_HOSTS treatment. -Real incident (2026-07): a hosted agent provisioned on nous-account-service's -`staging` Vercel environment is stamped with -``portal_base_url=https://portal.staging-nousresearch.com`` in its bootstrap -``auth.json`` (see ``buildBootstrapAuthJson`` / ``env.BASE_URL`` in -nous-account-service). ``resolve_nous_access_token``'s host-allowlist guard -(``_NOUS_PORTAL_ALLOWED_HOSTS``) only recognised the production portal host, -so on the very first refresh it silently rewrote ``portal_base_url`` back to -prod and then replayed a staging-issued refresh token against the PROD token -endpoint. Prod correctly rejected it with ``invalid_grant``, which triggered -``_quarantine_nous_oauth_state`` and wiped the entire credential pool — -turning a simple config mismatch into a full relogin requirement on every -staging hosted-agent instance. +Real incident (2026-07): a hosted agent provisioned by nous-account-service +on the `staging` Vercel environment is stamped with +``HERMES_PORTAL_BASE_URL=https://portal.staging-nousresearch.com`` in its +container env (the documented dev/staging override), while its bootstrap +``auth.json`` ALSO persists ``portal_base_url`` to the same staging host. -These tests verify the staging portal host is accepted by the allowlist and -is NOT silently rewritten to the production default. +Before this fix, ``resolve_nous_access_token`` / ``resolve_nous_runtime_ +credentials`` read ``state.get("portal_base_url")`` FIRST via a plain ``or`` +chain, so whenever the stored state had ANY value the env vars were never +even consulted — and whichever value won (state or env) was then run through +``_NOUS_PORTAL_ALLOWED_HOSTS``, which only recognised the production host. +The staging host was silently rewritten back to prod on every refresh, so a +staging-issued refresh token got replayed against the PROD token endpoint. +Prod correctly rejected that with ``invalid_grant``, which triggered +``_quarantine_nous_oauth_state`` and wiped the entire credential pool. + +The correct fix (mirroring ``_nous_inference_env_override()``): the env +override is a TRUSTED value the operator/deployment set themselves — it must +win outright (even over a stored value) and bypass the allowlist entirely. +The allowlist exists only to reject an untrusted NETWORK-provided value +(a poisoned portal_base_url written to auth.json by a compromised Portal +response), never a value the operator explicitly configured. """ from __future__ import annotations +import json +import logging + from hermes_cli.auth import ( DEFAULT_NOUS_PORTAL_URL, _NOUS_PORTAL_ALLOWED_HOSTS, + _nous_portal_env_override, ) -class TestPortalAllowlistIncludesStaging: - def test_prod_host_in_allowlist(self): - assert "portal.nousresearch.com" in _NOUS_PORTAL_ALLOWED_HOSTS +class TestPortalEnvOverrideHelper: + def test_none_when_unset(self, monkeypatch): + monkeypatch.delenv("HERMES_PORTAL_BASE_URL", raising=False) + monkeypatch.delenv("NOUS_PORTAL_BASE_URL", raising=False) + assert _nous_portal_env_override() is None - def test_staging_host_in_allowlist(self): - """The staging Nous Portal host must be accepted — hosted agents on - the `staging` NAS environment persist this host to auth.json and - must be able to refresh against it without being force-rewritten to - prod (which would replay a staging refresh token against the prod - token endpoint and fail with invalid_grant).""" - assert "portal.staging-nousresearch.com" in _NOUS_PORTAL_ALLOWED_HOSTS + def test_hermes_portal_base_url_wins(self, monkeypatch): + monkeypatch.setenv( + "HERMES_PORTAL_BASE_URL", "https://portal.staging-nousresearch.com/" + ) + monkeypatch.delenv("NOUS_PORTAL_BASE_URL", raising=False) + assert ( + _nous_portal_env_override() == "https://portal.staging-nousresearch.com" + ) - def test_localhost_dev_hosts_still_present(self): - """Guard against accidentally dropping the existing dev entries - while adding the staging host.""" - assert "localhost" in _NOUS_PORTAL_ALLOWED_HOSTS - assert "127.0.0.1" in _NOUS_PORTAL_ALLOWED_HOSTS + def test_nous_portal_base_url_used_as_fallback(self, monkeypatch): + monkeypatch.delenv("HERMES_PORTAL_BASE_URL", raising=False) + monkeypatch.setenv( + "NOUS_PORTAL_BASE_URL", "https://portal.staging-nousresearch.com" + ) + assert ( + _nous_portal_env_override() == "https://portal.staging-nousresearch.com" + ) - def test_default_portal_url_host_is_allowlisted(self): - """Sanity check mirroring the inference-host allowlist's own sanity - test: the default portal URL's host must itself be in the - allowlist, otherwise every install would break.""" - from urllib.parse import urlparse - - host = urlparse(DEFAULT_NOUS_PORTAL_URL).hostname - assert host in _NOUS_PORTAL_ALLOWED_HOSTS - - def test_attacker_host_not_allowlisted(self): - """The allowlist must stay tight — only the documented hosts.""" - assert "attacker.com" not in _NOUS_PORTAL_ALLOWED_HOSTS - assert "evil.portal.nousresearch.com" not in _NOUS_PORTAL_ALLOWED_HOSTS + def test_env_override_not_gated_by_allowlist(self, monkeypatch): + """The whole point: an env-set staging host is NOT in + _NOUS_PORTAL_ALLOWED_HOSTS, and the helper must return it anyway — + gating happens only for network-provenance values.""" + monkeypatch.setenv( + "HERMES_PORTAL_BASE_URL", "https://portal.staging-nousresearch.com" + ) + assert "portal.staging-nousresearch.com" not in _NOUS_PORTAL_ALLOWED_HOSTS + assert ( + _nous_portal_env_override() == "https://portal.staging-nousresearch.com" + ) -class TestResolveAccessTokenAcceptsStagingPortal: - """End-to-end: resolve_nous_access_token must refresh against a stored - staging portal_base_url rather than silently rewriting it to prod.""" +class TestResolveAccessTokenEnvOverrideWins: + """End-to-end: resolve_nous_access_token must use the env override for + the refresh call, bypassing the allowlist, even when state also has a + portal_base_url set (the exact incident shape).""" - def test_staging_portal_url_not_rewritten_on_refresh(self, monkeypatch, tmp_path): - import json - import logging - - import hermes_cli.auth as auth - - staging_portal = "https://portal.staging-nousresearch.com" - monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + def _write_auth_file(self, tmp_path, *, stored_portal_url): auth_file = tmp_path / "auth.json" auth_file.write_text( json.dumps( @@ -78,7 +89,7 @@ class TestResolveAccessTokenAcceptsStagingPortal: "active_provider": "nous", "providers": { "nous": { - "portal_base_url": staging_portal, + "portal_base_url": stored_portal_url, "access_token": "expired-access", "refresh_token": "staging-refresh", "client_id": "hermes-cli-vps", @@ -88,7 +99,9 @@ class TestResolveAccessTokenAcceptsStagingPortal: } ) ) + return auth_file + def _run_and_capture(self, monkeypatch, auth): seen_portal_urls = [] def _fake_refresh(*, client, portal_base_url, client_id, refresh_token): @@ -102,19 +115,84 @@ class TestResolveAccessTokenAcceptsStagingPortal: monkeypatch.setattr(auth, "_refresh_access_token", _fake_refresh) caplog_records = [] - with_caplog = logging.getLogger("hermes_cli.auth") + logger = logging.getLogger("hermes_cli.auth") handler = logging.Handler() handler.emit = lambda record: caplog_records.append(record.getMessage()) - with_caplog.addHandler(handler) + logger.addHandler(handler) try: auth.resolve_nous_access_token() finally: - with_caplog.removeHandler(handler) + logger.removeHandler(handler) + return seen_portal_urls, caplog_records - assert seen_portal_urls == [staging_portal], ( - "refresh must target the stored staging portal, not be " - f"silently rewritten to prod; saw {seen_portal_urls!r}" - ) + def test_env_override_wins_even_with_staging_state_stored( + self, monkeypatch, tmp_path + ): + """The real incident: state ALSO has the staging host stored (from + a prior HERMES_AUTH_JSON_BOOTSTRAP seed), and the env var is set to + the same staging host. Both must resolve to staging, and the + allowlist-rejection warning must never fire.""" + import hermes_cli.auth as auth + + staging_portal = "https://portal.staging-nousresearch.com" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("HERMES_PORTAL_BASE_URL", staging_portal) + self._write_auth_file(tmp_path, stored_portal_url=staging_portal) + + seen_portal_urls, records = self._run_and_capture(monkeypatch, auth) + + assert seen_portal_urls == [staging_portal] assert not any( - "ignoring invalid portal_base_url" in msg for msg in caplog_records - ), "staging portal host must not trip the allowlist-rejection warning" + "ignoring invalid portal_base_url" in msg for msg in records + ), "env override must bypass the allowlist gate entirely" + + def test_env_override_wins_over_prod_state(self, monkeypatch, tmp_path): + """Even when the STORED state is the prod host (e.g. a stale/healed + value from before the env var was set), the env override must still + win for the actual refresh call.""" + import hermes_cli.auth as auth + + staging_portal = "https://portal.staging-nousresearch.com" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("HERMES_PORTAL_BASE_URL", staging_portal) + self._write_auth_file(tmp_path, stored_portal_url=DEFAULT_NOUS_PORTAL_URL) + + seen_portal_urls, _records = self._run_and_capture(monkeypatch, auth) + + assert seen_portal_urls == [staging_portal] + + def test_no_env_override_stored_staging_host_heals_to_prod( + self, monkeypatch, tmp_path + ): + """Without the env override set, a stored staging host is untrusted + network provenance and correctly heals to prod (this is the + allowlist's actual job — preserved, not regressed, by this fix).""" + import hermes_cli.auth as auth + + staging_portal = "https://portal.staging-nousresearch.com" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.delenv("HERMES_PORTAL_BASE_URL", raising=False) + monkeypatch.delenv("NOUS_PORTAL_BASE_URL", raising=False) + self._write_auth_file(tmp_path, stored_portal_url=staging_portal) + + seen_portal_urls, records = self._run_and_capture(monkeypatch, auth) + + assert seen_portal_urls == [DEFAULT_NOUS_PORTAL_URL] + assert any("ignoring invalid portal_base_url" in msg for msg in records) + + def test_no_env_no_staging_state_prod_url_used_unmodified( + self, monkeypatch, tmp_path + ): + """Baseline: no override, no staging state — prod is used and the + allowlist never even logs a warning (nothing was rejected).""" + import hermes_cli.auth as auth + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.delenv("HERMES_PORTAL_BASE_URL", raising=False) + monkeypatch.delenv("NOUS_PORTAL_BASE_URL", raising=False) + self._write_auth_file(tmp_path, stored_portal_url=DEFAULT_NOUS_PORTAL_URL) + + seen_portal_urls, records = self._run_and_capture(monkeypatch, auth) + + assert seen_portal_urls == [DEFAULT_NOUS_PORTAL_URL] + assert not any("ignoring invalid portal_base_url" in msg for msg in records)