From 64d9460711aec1dc7424f4c2ded44728bf6c3bc6 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 7 Jul 2026 12:24:01 +0200 Subject: [PATCH] fix(upstream): validate native model_spec prices before trusting them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generic provider treated any Venice-style model_spec.pricing as authoritative after only a None check, so a native both-zero price served the model free, a negative one credited the caller on every request, and a non-numeric string threw while parsing — the outer catch then dropped the provider's entire catalog. Coerce both native prices through the resolver's _as_float and reject absent / non-numeric / negative / both-zero values, falling through to the shared litellm→OpenRouter→fail-closed chain instead. This extends the same money-safety guard the litellm and OpenRouter rungs already apply to the native source, and keeps one malformed entry from emptying the catalog. Co-Authored-By: Claude Opus 4.8 --- routstr/upstream/generic.py | 18 ++++-- tests/unit/test_upstream_generic.py | 95 +++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 6 deletions(-) diff --git a/routstr/upstream/generic.py b/routstr/upstream/generic.py index 0a040b9c..3faf80e9 100644 --- a/routstr/upstream/generic.py +++ b/routstr/upstream/generic.py @@ -8,6 +8,7 @@ from .base import BaseUpstreamProvider from .pricing_resolver import ( FallbackPricingResolver, ResolvedPricing, + _as_float, estimate_context_length, ) @@ -74,16 +75,21 @@ class GenericUpstreamProvider(BaseUpstreamProvider): ) -> ResolvedPricing | None: """Read pricing/metadata from Venice's bespoke ``model_spec`` schema. - Returns ``None`` when the upstream reported no native price (the common - case for bare OpenAI-compatible ``/models`` responses), so the caller - falls through to the shared resolution chain instead of fabricating a - number. + Returns ``None`` when the upstream reported no *usable* native price — + absent, non-numeric, negative, or both-zero — so the caller falls + through to the shared resolution chain instead of fabricating a number + or trusting a bogus one. This mirrors the money-safety guards the + litellm and OpenRouter rungs already apply: a both-zero price would + serve the model free, a negative one would credit the caller, and a + non-numeric string would otherwise throw and drop the whole catalog. """ pricing_info = model_spec.get("pricing", {}) - input_usd = pricing_info.get("input", {}).get("usd") - output_usd = pricing_info.get("output", {}).get("usd") + input_usd = _as_float(pricing_info.get("input", {}).get("usd")) + output_usd = _as_float(pricing_info.get("output", {}).get("usd")) if input_usd is None or output_usd is None: return None + if input_usd < 0 or output_usd < 0 or (input_usd == 0 and output_usd == 0): + return None capabilities = model_spec.get("capabilities", {}) input_modalities = ["text"] diff --git a/tests/unit/test_upstream_generic.py b/tests/unit/test_upstream_generic.py index 1bef8867..4ff6f589 100644 --- a/tests/unit/test_upstream_generic.py +++ b/tests/unit/test_upstream_generic.py @@ -111,6 +111,101 @@ async def test_native_model_spec_resolves_and_captures_metadata() -> None: or_feed.assert_not_awaited() +# --------------------------------------------------------------------------- +# native model_spec validation — a bogus native price is not authoritative +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_native_both_zero_price_falls_through_to_litellm() -> None: + """A native ``model_spec`` that prices both tokens at 0 is not a real price + (the same free-tier trap the litellm/OpenRouter rungs already reject). It + must not be treated as authoritative and served free; the resolver falls + through, so a litellm-known model lands on litellm's real rate instead.""" + payload = { + "data": [ + { + "id": "deepseek-chat", + "owned_by": "deepseek", + "model_spec": { + "pricing": {"input": {"usd": 0}, "output": {"usd": 0}}, + }, + } + ] + } + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=[]) + 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, "deepseek-chat") + assert model.enabled is True + assert model.pricing.prompt == pytest.approx(2.8e-07) + assert model.pricing.completion == pytest.approx(4.2e-07) + + +@pytest.mark.asyncio +async def test_native_negative_price_falls_through_to_litellm() -> None: + """A negative native price would credit the caller's balance on every + request (a fund drain, not a discount). Reject it like any other unusable + price and fall through to the chain.""" + payload = { + "data": [ + { + "id": "deepseek-chat", + "owned_by": "deepseek", + "model_spec": { + "pricing": {"input": {"usd": -0.5}, "output": {"usd": -1.5}}, + }, + } + ] + } + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=[]) + 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, "deepseek-chat") + assert model.enabled is True + assert model.pricing.prompt == pytest.approx(2.8e-07) + assert model.pricing.completion == pytest.approx(4.2e-07) + + +@pytest.mark.asyncio +async def test_native_non_numeric_price_does_not_break_catalog() -> None: + """A non-numeric native price (``"free"``) must not raise while parsing — + an unguarded ``"free" / 1_000_000`` throws and the outer catch drops the + *entire* provider catalog. It has to fail closed for that one model while + every other model in the same response still resolves.""" + payload = { + "data": [ + { + "id": "broken-price", + "owned_by": "mystery", + "model_spec": { + "pricing": {"input": {"usd": "free"}, "output": {"usd": "free"}}, + }, + }, + {"id": "deepseek-chat", "object": "model", "owned_by": "deepseek"}, + ] + } + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=[]) + with patch("routstr.payment.models.async_fetch_openrouter_models", or_feed): + models = await GenericUpstreamProvider(base_url="http://x").fetch_models() + + # One malformed entry must not empty the catalog. + assert {m.id for m in models} == {"broken-price", "deepseek-chat"} + broken = _model_by_id(models, "broken-price") + assert broken.enabled is False + healthy = _model_by_id(models, "deepseek-chat") + assert healthy.enabled is True + assert healthy.pricing.prompt == pytest.approx(2.8e-07) + + # --------------------------------------------------------------------------- # litellm rescue — the money-critical case (DeepSeek bare /models) # ---------------------------------------------------------------------------