mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-16 23:41:43 +00:00
fix: fail closed when key context is absent (veria-ai finding)
Confirmed real. _canonical_target_is_allowed returned True when user_api_key_dict was None, on the reasoning that no key context meant non-proxy Router use. That reasoning was wrong: of the 12 route_request call sites, only one (common_request_processing, the /chat/completions path) forwards user_api_key_dict. Image generation, rerank, moderation, speech, transcription, realtime, and the Responses WebSocket path are all authenticated endpoints that call route_request without it, so the rewrite ran with no target authorization at all on exactly those paths -- a key whose allowlist held only the stale requested spelling could reach a canonical target it was never granted. Now returns False. Declining costs those endpoints only the convenience rewrite: they behave exactly as they do today, resolution simply never engages, and no request that works now breaks. The AND-on-target guarantee becomes unconditional rather than depending on whether a given caller happens to thread the key through. Verified the primary path is unaffected: with key context present and authorized, resolution still resolves -- that is the Claude Code 403 case this PR exists to fix. Threading user_api_key_dict through the other call sites is the follow-up that re-enables resolution for them; doing it here would touch 11 unrelated endpoints in a PR that should not be changing their signatures. Updated test_no_auth_context_is_allowed -> test_absent_auth_context_fails_closed with the reasoning recorded. Gates: pytest 531 passed - ruff format/check clean - type-discipline all LIT rules at base parity - basedpyright new module 0 errors. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
9b1fdc1d8d
commit
1b8daea07c
2 changed files with 25 additions and 5 deletions
|
|
@ -262,9 +262,21 @@ async def _canonical_target_is_allowed(
|
|||
A denial returns False rather than raising, so the request falls through to
|
||||
the same 400 an unresolvable model gets today and the response reveals
|
||||
nothing about the target's existence.
|
||||
|
||||
Absent key context fails CLOSED. Most ``route_request`` callers (image
|
||||
generation, rerank, moderation, speech, transcription, realtime, Responses
|
||||
WebSocket) are authenticated but do not currently forward
|
||||
``user_api_key_dict``, so treating "no key context" as "allowed" would run
|
||||
the rewrite with no target authorization at all on exactly those paths.
|
||||
Declining instead costs those endpoints only the convenience rewrite --
|
||||
they behave as they do today, resolution simply never engages -- while
|
||||
keeping the AND-on-target guarantee unconditional. Threading the key
|
||||
through those call sites is the follow-up that re-enables resolution for
|
||||
them; until then this must not be the hole through which the check is
|
||||
skipped.
|
||||
"""
|
||||
if user_api_key_dict is None:
|
||||
return True
|
||||
return False
|
||||
from litellm.proxy.auth.auth_checks import (
|
||||
can_key_call_resolved_model, # pyright: ignore[reportUnknownVariableType] - auth_checks is partially typed
|
||||
)
|
||||
|
|
|
|||
|
|
@ -509,9 +509,17 @@ class TestCanonicalTargetReAuth:
|
|||
assert allowed is False
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_auth_context_is_allowed(self, anthropic_router: Router):
|
||||
"""No key context (non-proxy Router use) leaves the rewrite unguarded by
|
||||
key auth, matching the surrounding call path."""
|
||||
async def test_absent_auth_context_fails_closed(self, anthropic_router: Router):
|
||||
"""Regression: absent key context must DECLINE the rewrite, not allow it.
|
||||
|
||||
Most route_request callers (image generation, rerank, moderation,
|
||||
speech, transcription, realtime, Responses WebSocket) are authenticated
|
||||
but don't currently forward user_api_key_dict. Returning True here
|
||||
would run the rewrite with no target authorization at all on exactly
|
||||
those paths -- a key whose allowlist holds only the stale requested
|
||||
spelling could reach a target it was never granted. Declining costs
|
||||
those endpoints only the convenience rewrite; the AND-on-target
|
||||
guarantee stays unconditional."""
|
||||
from litellm.proxy.route_llm_request import _canonical_target_is_allowed
|
||||
|
||||
assert (
|
||||
|
|
@ -520,7 +528,7 @@ class TestCanonicalTargetReAuth:
|
|||
llm_router=anthropic_router,
|
||||
user_api_key_dict=None,
|
||||
)
|
||||
is True
|
||||
is False
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue