From 89e6acef4d99bddf2ef128a5d0adbd5da96463ee Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 16:16:15 +0200 Subject: [PATCH] fix(pricing): treat a non-finite rate as no price wherever pricing is ingested MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `inf > 0` is True, so an infinite rate read as a real price: it was imported, served, routed, and would bill an infinite amount. `NaN` is worse — every comparison with it is False, so it slipped past both the negative and the both-zero guards. Rates arrive from upstream catalogs as well as the admin edge, and `json.loads` accepts the bare `NaN`/`Infinity` literals and overflows `1e999` to `inf`, so the finiteness check goes in the catalog import filter and in each resolver rung that reports a resolved price rather than at one entry point. An oversized integer raises `OverflowError` instead of `ValueError`, so coercion catches that too — unhandled it unwound the whole fetch and cost the node an entire upstream catalog over one junk entry. A malformed *cache* rate costs only the cache rate: it is dropped and billing falls back to the full input rate, which is what a missing cache rate means. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM --- routstr/payment/models.py | 12 ++- routstr/upstream/pricing_resolver.py | 20 +++- tests/unit/test_pricing_rate_validation.py | 95 ++++++++++++++++++ tests/unit/test_upstream_generic.py | 107 +++++++++++++++++++++ 4 files changed, 228 insertions(+), 6 deletions(-) diff --git a/routstr/payment/models.py b/routstr/payment/models.py index 3cd4de83..dad9bbb5 100644 --- a/routstr/payment/models.py +++ b/routstr/payment/models.py @@ -168,7 +168,7 @@ def backfill_cache_pricing(model_id: str, pricing: Pricing) -> Pricing: def _has_valid_pricing(model: dict) -> bool: - """Check if model has valid pricing (not free, no negative values).""" + """Check if model has valid pricing (usable rates, and not free).""" pricing = model.get("pricing", {}) if not pricing: return False @@ -176,10 +176,16 @@ def _has_valid_pricing(model: dict) -> bool: try: prompt = float(pricing.get("prompt", 0)) completion = float(pricing.get("completion", 0)) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): + # An integer too large for a float raises OverflowError, not + # ValueError, so it escaped this coercion guard and unwound the whole + # fetch — one junk entry cost the node the entire upstream catalog. return False - if prompt < 0 or completion < 0: + # `NaN`/`±inf` are not prices, and neither is caught by the checks below: + # every comparison with `NaN` is False, and `inf` reads as a large positive + # rate that would be advertised and billed on. + if not is_usable_rate(prompt) or not is_usable_rate(completion): return False if prompt == 0 and completion == 0: diff --git a/routstr/upstream/pricing_resolver.py b/routstr/upstream/pricing_resolver.py index 3d009fdc..3064ba57 100644 --- a/routstr/upstream/pricing_resolver.py +++ b/routstr/upstream/pricing_resolver.py @@ -15,6 +15,7 @@ it into the base provider unchanged. from __future__ import annotations +import math from dataclasses import dataclass, field @@ -65,11 +66,19 @@ def estimate_context_length(model_id: str) -> int: def _as_float(value: object) -> float | None: - """OpenRouter reports prices as strings; coerce, ``None`` if unparseable.""" + """OpenRouter reports prices as strings; coerce, ``None`` if not a real number. + + Non-finite values are rejected as unparseable: ``float("Infinity")`` and + ``float("NaN")`` parse happily from a feed string, and ``json.loads`` + accepts the bare literals and overflows ``1e999`` to ``inf``. An oversized + integer raises ``OverflowError`` rather than ``ValueError``, so that is + caught too. + """ try: - return float(value) # type: ignore[arg-type] - except (TypeError, ValueError): + parsed = float(value) # type: ignore[arg-type] + except (TypeError, ValueError, OverflowError): return None + return parsed if math.isfinite(parsed) else None def _as_int(value: object) -> int | None: @@ -94,6 +103,11 @@ def _from_litellm(model_id: str) -> ResolvedPricing | None: # moderation/rerank tiers do this) — treating 0/0 as resolved would serve # the model for free. Reject it (and any negative) so the caller falls # through, mirroring async_fetch_openrouter_models' _has_valid_pricing. + # A non-finite entry is junk, not a price: `inf` would bill an infinite + # amount and `NaN` poisons every total it enters (and defeats the `< 0` and + # `== 0` guards below, since both comparisons are False for `NaN`). + if not math.isfinite(prompt) or not math.isfinite(completion): + return None if prompt < 0 or completion < 0 or (prompt == 0 and completion == 0): return None diff --git a/tests/unit/test_pricing_rate_validation.py b/tests/unit/test_pricing_rate_validation.py index db2f5100..9e4a5d6e 100644 --- a/tests/unit/test_pricing_rate_validation.py +++ b/tests/unit/test_pricing_rate_validation.py @@ -315,3 +315,98 @@ async def test_all_quotes_unusable_keeps_the_last_good_price( ) assert btc_usd_price() == pytest.approx(90000.0) + + +# --------------------------------------------------------------------------- +# Catalog ingest — a malformed rate must never become a stored price +# --------------------------------------------------------------------------- + + +class _CatalogResponse: + def __init__(self, payload: dict[str, Any]) -> None: + self._payload = payload + + def raise_for_status(self) -> None: + return None + + def json(self) -> dict[str, Any]: + return self._payload + + +class _CatalogClient: + """Stands in for ``httpx.AsyncClient`` against the OpenRouter catalog.""" + + def __init__(self, models: list[dict[str, Any]]) -> None: + self._models = models + + async def __aenter__(self) -> "_CatalogClient": + return self + + async def __aexit__(self, *exc: object) -> bool: + return False + + async def get(self, url: str, timeout: int | None = None) -> _CatalogResponse: + if url.endswith("/embeddings/models"): + return _CatalogResponse({"data": []}) + return _CatalogResponse({"data": self._models}) + + +def _catalog_entry(model_id: str, pricing: dict[str, Any]) -> dict[str, Any]: + return {"id": model_id, "name": model_id, "pricing": pricing} + + +def _patch_openrouter_catalog(models: list[dict[str, Any]]) -> Any: + return patch( + "routstr.payment.models.httpx.AsyncClient", + lambda *args, **kwargs: _CatalogClient(models), + ) + + +@pytest.mark.parametrize( + "bad_rate", + [float("nan"), float("inf"), float("-inf")], + ids=["nan", "inf", "negative-inf"], +) +@pytest.mark.asyncio +async def test_non_finite_catalog_rate_is_not_imported(bad_rate: float) -> None: + """A non-finite rate in the upstream catalog is junk, not a price. + + ``json.loads`` accepts the bare ``NaN``/``Infinity`` literals and overflows + ``1e999`` to ``inf``, so an upstream feed can deliver one. The import filter + rejects a negative and a both-zero price, but every comparison with ``NaN`` + is False and ``inf`` reads as a large positive, so both sailed through and + became a stored price the node would advertise and bill on. + """ + with _patch_openrouter_catalog( + [ + _catalog_entry("bad", {"prompt": bad_rate, "completion": "0.000002"}), + _catalog_entry("good", {"prompt": "0.000001", "completion": "0.000002"}), + ] + ): + from routstr.payment.models import async_fetch_openrouter_models + + models = await async_fetch_openrouter_models() + + assert [m["id"] for m in models] == ["good"] + + +@pytest.mark.asyncio +async def test_oversized_catalog_rate_does_not_empty_the_catalog() -> None: + """An integer too large to be a float must cost one model, not all of them. + + ``float()`` raises ``OverflowError`` — not ``ValueError`` — for such a + value, so the coercion guard in the import filter did not catch it and the + exception unwound the whole fetch. The node then imported nothing at all + from an upstream whose catalog was fine apart from one entry. + """ + with _patch_openrouter_catalog( + [ + _catalog_entry("bad", {"prompt": 10**400, "completion": 2}), + _catalog_entry("good", {"prompt": "0.000001", "completion": "0.000002"}), + ] + ): + from routstr.payment.models import async_fetch_openrouter_models + + models = await async_fetch_openrouter_models() + + assert [m["id"] for m in models] == ["good"] diff --git a/tests/unit/test_upstream_generic.py b/tests/unit/test_upstream_generic.py index 39daff5c..bf5316a1 100644 --- a/tests/unit/test_upstream_generic.py +++ b/tests/unit/test_upstream_generic.py @@ -522,3 +522,110 @@ async def test_unresolvable_model_fails_closed( for rec in caplog.records if rec.levelno >= logging.WARNING ) + + +# --------------------------------------------------------------------------- +# rate validation — a malformed rate is not a resolved price, at any rung +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_non_finite_litellm_rate_is_not_a_resolved_price() -> None: + """A non-finite entry in the cost map must not answer the resolution chain. + + The litellm rung rejects negatives and a both-zero entry, but every + comparison with ``NaN`` is False and ``inf`` reads as a large positive, so + either would be reported as a resolved price — enabling the model at a rate + the node cannot bill on. Fail closed instead: the model imports disabled, + which is what "no source knows this price" already means here. + """ + payload = { + "data": [ + {"id": "nan-priced-model", "object": "model", "owned_by": "mystery"}, + ] + } + cost_entry = { + "input_cost_per_token": float("nan"), + "output_cost_per_token": float("inf"), + "max_input_tokens": 8192, + } + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=[]) + with patch("routstr.payment.models.litellm_cost_entry", lambda _id: cost_entry): + with patch("routstr.payment.models.async_fetch_openrouter_models", or_feed): + models = await GenericUpstreamProvider( + base_url="http://x" + ).fetch_models() + + model = _model_by_id(models, "nan-priced-model") + assert model.enabled is False + assert model.pricing.prompt == 0.0 + assert model.pricing.completion == 0.0 + + +@pytest.mark.asyncio +async def test_non_finite_openrouter_rate_is_not_a_resolved_price() -> None: + """``float("Infinity")`` parses happily from a feed string, so the + OpenRouter rung's coercion accepted it and reported an infinite rate as a + resolved price. It is not a price; the model must import disabled.""" + payload = { + "data": [ + {"id": "or-nonfinite-xyz", "object": "model", "owned_by": "mystery"}, + ] + } + feed = [ + { + "id": "or-nonfinite-xyz", + "pricing": {"prompt": "Infinity", "completion": "0.000002"}, + "context_length": 8192, + } + ] + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=feed) + with patch("routstr.payment.models.async_fetch_openrouter_models", or_feed): + models = await GenericUpstreamProvider(base_url="http://x").fetch_models() + + model = _model_by_id(models, "or-nonfinite-xyz") + assert model.enabled is False + assert model.pricing.prompt == 0.0 + assert model.pricing.completion == 0.0 + + +@pytest.mark.asyncio +async def test_non_finite_openrouter_cache_rate_is_dropped_not_carried() -> None: + """A malformed *cache* rate must cost the cache rate, not the model. + + The catalog import filter only inspects prompt and completion, so an entry + with two sound token rates and an unusable ``input_cache_read`` reaches the + resolver intact. Carrying that rate through would price every cached input + token at ``inf``; dropping it falls back to the full input rate, which is + what a missing cache rate already means. + """ + payload = { + "data": [ + {"id": "or-badcache-xyz", "object": "model", "owned_by": "mystery"}, + ] + } + feed = [ + { + "id": "or-badcache-xyz", + "pricing": { + "prompt": "0.000001", + "completion": "0.000002", + "input_cache_read": "Infinity", + }, + "context_length": 8192, + } + ] + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=feed) + with patch("routstr.payment.models.async_fetch_openrouter_models", or_feed): + models = await GenericUpstreamProvider(base_url="http://x").fetch_models() + + model = _model_by_id(models, "or-badcache-xyz") + assert model.enabled is True + assert model.pricing.prompt == pytest.approx(1e-06) + assert model.pricing.input_cache_read == 0.0