mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
2f2950a626
Split out of #2852 at review request: that PR is bounded upstream calls plus measured hot-path costs, and this is an authentication/routing change that belongs on its own scope. #2852 now carries only the timeout work. ## The bug A routing extension can rewrite the model across families mid-request (`claude-opus-5` → `gpt-5-mini`). The caller's key does not travel with that rewrite, so the proxy forwards `sk-ant-...` to OpenAI and earns a guaranteed 401. Downstream that is indistinguishable from *"the cheap model failed the task"* — it scores as a quality regression against the router, not as a bug. Dropping the `api_key` kwarg instead lets litellm fall back to the target provider's own env credential, which is the only key that can work. ## Why this cut is different from the one that was rejected The first version returned `not provider.startswith("anthropic")`, so **any** non-`sk-ant-` credential was dropped against an Anthropic-class target — a plain Bearer token against an Anthropic-compatible or custom gateway lost its key and fell back to an env credential that may not exist. That direction is the dangerous one. A false refusal breaks a deployment that was working; a missed refusal just leaves today's 401. So this refuses on **positive evidence only**: | credential | target | forwarded? | |---|---|---| | `sk-ant-…` | `openai` / `azure` / `gemini` | **no** — cannot possibly authenticate | | `sk-ant-…` | anthropic | yes | | `sk-ant-…` | unrecognised / unclassifiable model | yes — pass-through | | anything else | anything | yes — pass-through, unchanged | `sk-ant-` is Anthropic's documented vendor-specific prefix, which is what makes it classifiable. `sk-` is not: a dozen vendors mint that shape. Everything the string cannot settle keeps main's behaviour. The reject list is explicit rather than inverted (`not anthropic`) because an unrecognised provider is usually a compatible or self-hosted gateway. Marked in the code as a hand-kept tuple with the registry-lookup upgrade path noted. Bedrock / Vertex / SageMaker are unaffected — all four dispatch sites already skip credential forwarding for them entirely (env-based auth). ## Verification `tests/test_litellm_caller_key.py`, 12 cases — the refusal, the Anthropic target, the unknown provider, `get_llm_provider` raising, and each unclassifiable credential shape asserted against **both** target families. Those last ones fail against the rejected version. Applied at all four dispatch sites (Anthropic non-stream/stream, OpenAI non-stream/stream). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
67 lines
2.5 KiB
Python
67 lines
2.5 KiB
Python
"""A caller's key must not be dropped unless we are certain it cannot work.
|
|
|
|
The proxy forwards the inbound credential to the upstream provider. When a
|
|
routing extension rewrites the model across families mid-request, that key stops
|
|
matching the target and the 401 that follows is indistinguishable, downstream,
|
|
from "the cheap model failed the task".
|
|
|
|
Refusing to forward is the fix, but it is also the more dangerous direction: a
|
|
false positive silently strips a credential from a deployment that was working,
|
|
and litellm then falls back to an env key that may not exist. So the rule is
|
|
positive evidence only -- an unrecognised credential always travels.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
|
|
from headroom.backends.litellm import _caller_key_travels_to
|
|
|
|
ANTHROPIC_KEY = "sk-ant-api03-abc123"
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"model",
|
|
["gpt-5-mini", "gpt-4o", "azure/gpt-4", "gemini/gemini-2.0-flash"],
|
|
)
|
|
def test_anthropic_key_is_refused_for_a_provider_that_cannot_accept_it(model: str) -> None:
|
|
"""The bug this exists for: claude-* rewritten to a non-Anthropic target."""
|
|
pytest.importorskip("litellm")
|
|
assert _caller_key_travels_to(model, ANTHROPIC_KEY) is False
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"model",
|
|
["claude-opus-4-5-20251101", "anthropic/claude-sonnet-4-5-20250929"],
|
|
)
|
|
def test_anthropic_key_travels_to_anthropic(model: str) -> None:
|
|
pytest.importorskip("litellm")
|
|
assert _caller_key_travels_to(model, ANTHROPIC_KEY) is True
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"key",
|
|
[
|
|
"sk-proj-openai-style", # a dozen vendors mint this shape
|
|
"Bearer-ish-opaque-token", # a plain gateway token
|
|
"hf_abc123",
|
|
"sk-ant", # near miss, not the prefix
|
|
"",
|
|
],
|
|
)
|
|
def test_only_the_anthropic_prefix_is_ever_classified(key: str) -> None:
|
|
"""Everything else is unclassifiable from the string, so it passes through.
|
|
|
|
This is the regression the review caught: the first version returned
|
|
`not provider.startswith("anthropic")`, which dropped every one of these
|
|
against an Anthropic-class target.
|
|
"""
|
|
assert _caller_key_travels_to("gpt-5-mini", key) is True
|
|
assert _caller_key_travels_to("claude-opus-4-5-20251101", key) is True
|
|
|
|
|
|
def test_unknown_provider_keeps_the_pass_through() -> None:
|
|
"""A compatible or self-hosted gateway we cannot classify must not lose its
|
|
key -- including when `get_llm_provider` raises on the model string."""
|
|
assert _caller_key_travels_to("some-self-hosted-thing", ANTHROPIC_KEY) is True
|
|
assert _caller_key_travels_to("", ANTHROPIC_KEY) is True
|