From 4caeb02735cdbbe302d0f28bf1a0ef14aa1af3e8 Mon Sep 17 00:00:00 2001 From: Mariano Nicolini Date: Thu, 27 Aug 2026 19:12:58 -0300 Subject: [PATCH] fix(models): key the pricing cache on auth state, not just the base URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fetch_models_with_pricing` checked its cache above the point where the Authorization header is built, and keyed that cache on the base URL alone. Whichever read of a given base URL landed first in a process therefore answered every later read, whatever key it passed — a non-empty result is held for the life of the process. That is wrong for any endpoint whose answer depends on who is asking. The Nous inference gateway filters `GET /v1/models` by the caller's org model policy, so an anonymous read landing first makes a later authenticated read return the full, unfiltered catalog without a request going out. Separate the URL root from the cache key and fold auth state into the latter. Only whether a key was supplied participates, never its value, so no secret reaches the key. `credits_tracker` peeked into the private `_pricing_cache` and duplicated the key shape to do it; it now calls `peek_cached_pricing`, which owns both the /v1-suffix normalization and the preference for the authenticated catalog. Co-Authored-By: Claude Opus 5 (1M context) --- agent/credits_tracker.py | 14 +- hermes_cli/models.py | 41 +++++- .../hermes_cli/test_pricing_cache_auth_key.py | 127 ++++++++++++++++++ 3 files changed, 171 insertions(+), 11 deletions(-) create mode 100644 tests/hermes_cli/test_pricing_cache_auth_key.py diff --git a/agent/credits_tracker.py b/agent/credits_tracker.py index 39c74ea58b..2d0873c563 100644 --- a/agent/credits_tracker.py +++ b/agent/credits_tracker.py @@ -252,15 +252,13 @@ def is_free_tier_model(model: str, base_url: str = "") -> bool: if not base_url: return False try: - from hermes_cli.models import _is_model_free, _pricing_cache + from hermes_cli.models import _is_model_free, peek_cached_pricing - # Mirror get_pricing_for_provider's key normalization: the agent's - # Nous base_url is /v1-suffixed (https://inference-api.nousresearch.com/v1) - # but the picker keys _pricing_cache on the pre-/v1 root. - key = base_url.rstrip("/") - if key.endswith("/v1"): - key = key[:-3].rstrip("/") - pricing = _pricing_cache.get(key) + # The agent's Nous base_url is /v1-suffixed + # (https://inference-api.nousresearch.com/v1) but the catalog fetchers + # key on the pre-/v1 root, and on auth state besides; peek_cached_pricing + # owns both details. + pricing = peek_cached_pricing(base_url) if not pricing: return False return _is_model_free(model, pricing) diff --git a/hermes_cli/models.py b/hermes_cli/models.py index 389cbaed72..cc2ba908b8 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -2255,6 +2255,39 @@ def _cache_catalog( return result +# A governed endpoint answers an authenticated read with a policy-filtered +# catalog and an anonymous read with the full one, so auth state is part of the +# cache identity. NUL cannot appear in a URL, so the suffix cannot collide with +# a base URL that happens to end this way. +_PRICING_AUTH_KEY_SUFFIX = "\x00auth" + + +def _pricing_cache_key(url_root: str, api_key: str | None) -> str: + """The ``_pricing_cache`` key for a read of *url_root*. + + Only *whether* a key was supplied participates — never its value, so no + secret reaches the cache key. + """ + return url_root + _PRICING_AUTH_KEY_SUFFIX if api_key else url_root + + +def peek_cached_pricing(base_url: str) -> dict[str, dict[str, Any]]: + """Pricing already cached for *base_url*, or ``{}``. Never fetches. + + Accepts a ``/v1``-suffixed URL as well as the pre-``/v1`` root the + catalog fetchers key on. Prefers the authenticated catalog, which is the + one scoped to the caller's org. + """ + root = (base_url or "").rstrip("/") + if root.endswith("/v1"): + root = root[:-3].rstrip("/") + for key in (root + _PRICING_AUTH_KEY_SUFFIX, root): + cached = _pricing_cache.get(key) + if cached: + return cached + return {} + + def _format_price_per_mtok(per_token_str: str) -> str: """Convert a per-token price string to a human-friendly $/Mtok string. @@ -2391,7 +2424,8 @@ def fetch_models_with_pricing( ) -> dict[str, dict[str, Any]]: """Fetch ``/v1/models`` and return ``{model_id: {prompt, completion, ...}}``. - Results are cached per *base_url* so repeated calls are free. + Results are cached per *base_url* and per auth state, so repeated calls + are free and an authenticated read never answers an anonymous one. Works with any OpenRouter-compatible endpoint (OpenRouter, Nous Portal). When *include_sale_original* is true (Nous Portal only) and the gateway @@ -2402,13 +2436,14 @@ def fetch_models_with_pricing( ``{prompt, completion}`` shape even if a response happens to nest ``original``. """ - cache_key = (base_url or "").rstrip("/") + url_root = (base_url or "").rstrip("/") + cache_key = _pricing_cache_key(url_root, api_key) if not force_refresh: cached = _cached_catalog(cache_key) if cached is not None: return cached - url = cache_key + "/v1/models" + url = url_root + "/v1/models" headers: dict[str, str] = { "Accept": "application/json", "User-Agent": _HERMES_USER_AGENT, diff --git a/tests/hermes_cli/test_pricing_cache_auth_key.py b/tests/hermes_cli/test_pricing_cache_auth_key.py new file mode 100644 index 0000000000..a1ae120365 --- /dev/null +++ b/tests/hermes_cli/test_pricing_cache_auth_key.py @@ -0,0 +1,127 @@ +"""``_pricing_cache`` keys on auth state, not just the base URL. + +A governed endpoint (Nous ``/v1/models`` filtered by an org's model policy) +answers an authenticated read with a narrower catalog than an anonymous one. +Keyed on the base URL alone, whichever read landed first in a process answered +every later one — so an authenticated caller could be handed the full, +unfiltered catalog without a request going out. +""" + +from __future__ import annotations + +import json +from unittest.mock import MagicMock + +import pytest + +import hermes_cli.models as models_mod +from hermes_cli.models import fetch_models_with_pricing, peek_cached_pricing + +BASE = "https://inference-api.example.com" + +# What the endpoint serves anonymously vs. to a policy-restricted caller. +_FULL = ["vendor/allowed", "vendor/blocked"] +_FILTERED = ["vendor/allowed"] + + +@pytest.fixture(autouse=True) +def _clear_pricing_cache(): + models_mod._pricing_cache.clear() + models_mod._pricing_cache_retry_after.clear() + yield + models_mod._pricing_cache.clear() + models_mod._pricing_cache_retry_after.clear() + + +@pytest.fixture +def catalog(monkeypatch): + """Serve the filtered catalog to an authenticated read, the full one to an + anonymous read, and record every request.""" + requests: list[str | None] = [] + + def _fake_urlopen(req, timeout=8.0): + auth = req.get_header("Authorization") + requests.append(auth) + ids = _FILTERED if auth else _FULL + payload = { + "data": [ + {"id": mid, "pricing": {"prompt": "0.000002", "completion": "0.00001"}} + for mid in ids + ] + } + resp = MagicMock() + resp.read.return_value = json.dumps(payload).encode() + resp.__enter__ = lambda self: self + resp.__exit__ = lambda *a: False + return resp + + monkeypatch.setattr(models_mod, "_urlopen_model_catalog_request", _fake_urlopen) + return requests + + +def test_authenticated_read_is_not_answered_by_an_anonymous_one(catalog): + """The bug: an anonymous read landing first must not answer the next + authenticated read out of cache.""" + anon = fetch_models_with_pricing(api_key="", base_url=BASE) + authed = fetch_models_with_pricing(api_key="sk-test", base_url=BASE) + + assert sorted(anon) == sorted(_FULL) + assert sorted(authed) == sorted(_FILTERED) + assert len(catalog) == 2, "the authenticated read must reach the network" + assert catalog[0] is None and catalog[1] == "Bearer sk-test" + + +def test_anonymous_read_is_not_answered_by_an_authenticated_one(catalog): + """And the reverse direction, so neither entry can shadow the other.""" + authed = fetch_models_with_pricing(api_key="sk-test", base_url=BASE) + anon = fetch_models_with_pricing(api_key="", base_url=BASE) + + assert sorted(authed) == sorted(_FILTERED) + assert sorted(anon) == sorted(_FULL) + assert len(catalog) == 2 + + +@pytest.mark.parametrize("api_key", ["sk-test", ""]) +def test_repeated_read_still_hits_the_cache(catalog, api_key): + """Widening the key must not cost the caching it was there for.""" + first = fetch_models_with_pricing(api_key=api_key, base_url=BASE) + second = fetch_models_with_pricing(api_key=api_key, base_url=BASE) + + assert first == second + assert len(catalog) == 1, "second read should be served from cache" + + +def test_force_refresh_replaces_only_its_own_entry(catalog): + """A forced authenticated re-read must leave the anonymous entry intact.""" + fetch_models_with_pricing(api_key="", base_url=BASE) + fetch_models_with_pricing(api_key="sk-test", base_url=BASE) + fetch_models_with_pricing(api_key="sk-test", base_url=BASE, force_refresh=True) + + assert len(catalog) == 3 + anon = fetch_models_with_pricing(api_key="", base_url=BASE) + assert sorted(anon) == sorted(_FULL) + assert len(catalog) == 3, "the anonymous entry should have survived" + + +class TestPeekCachedPricing: + def test_returns_empty_when_nothing_cached(self): + assert peek_cached_pricing(BASE) == {} + + def test_accepts_a_v1_suffixed_url(self, catalog): + """The agent holds a /v1-suffixed base URL; the fetchers key on the root.""" + fetch_models_with_pricing(api_key="sk-test", base_url=BASE) + assert sorted(peek_cached_pricing(BASE + "/v1")) == sorted(_FILTERED) + + def test_prefers_the_authenticated_catalog(self, catalog): + """It is the one scoped to the caller's org.""" + fetch_models_with_pricing(api_key="", base_url=BASE) + fetch_models_with_pricing(api_key="sk-test", base_url=BASE) + assert sorted(peek_cached_pricing(BASE)) == sorted(_FILTERED) + + def test_falls_back_to_the_anonymous_catalog(self, catalog): + fetch_models_with_pricing(api_key="", base_url=BASE) + assert sorted(peek_cached_pricing(BASE)) == sorted(_FULL) + + def test_never_fetches(self, catalog): + peek_cached_pricing(BASE) + assert catalog == []