From f92594f2c68a67d77d744c1344ace26e2a575efa Mon Sep 17 00:00:00 2001 From: Ryan Crabbe Date: Wed, 22 Apr 2026 14:28:58 -0700 Subject: [PATCH 01/25] fix: honor key access_group_ids when team restricts models Two model-access gates run per request in `common_checks` and they're asymmetric: `can_key_call_model` falls back to the key's `access_group_ids`, but `can_team_access_model` only looks at `team.models` + `team.access_group_ids`. A key granted a model via its own access group on a model-restricted team is silently denied at the team gate. Wrap `can_team_access_model` in try/except in `common_checks`: on `team_model_access_denied`, consult a new `_key_access_group_grants_model` helper that expands `valid_token.access_group_ids` via the existing `_get_models_from_access_groups` and checks via `_can_object_call_model`. Re-raise if the key's access groups don't grant the model. Any other exception propagates unchanged. Effect: request allowed if `team allows X` OR `key's access group grants X`, making the two gates symmetric. Test: add three unit tests for `_key_access_group_grants_model` covering: group covers model, key has no groups, group resolves but does not cover model. --- litellm/proxy/auth/auth_checks.py | 66 ++++++++++++++---- tests/proxy_unit_tests/test_auth_checks.py | 81 ++++++++++++++++++++++ 2 files changed, 133 insertions(+), 14 deletions(-) diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index 2c8299e77a..e959867091 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -494,23 +494,27 @@ async def common_checks( # noqa: PLR0915 f"Team={team_object.team_id} is blocked. Update via `/team/unblock` if you're an admin." ) - # 2. If team can call model + # 2. If team can call model (or key's access_group_ids grant it) if _model and team_object: with tracer.trace("litellm.proxy.auth.common_checks.can_team_access_model"): - if not await can_team_access_model( - model=_model, - team_object=team_object, - llm_router=llm_router, - team_model_aliases=( - valid_token.team_model_aliases if valid_token else None - ), - ): - raise ProxyException( - message=f"Team not allowed to access model. Team={team_object.team_id}, Model={_model}. Allowed team models = {team_object.models}", - type=ProxyErrorTypes.team_model_access_denied, - param="model", - code=status.HTTP_401_UNAUTHORIZED, + try: + await can_team_access_model( + model=_model, + team_object=team_object, + llm_router=llm_router, + team_model_aliases=( + valid_token.team_model_aliases if valid_token else None + ), ) + except ProxyException as team_denial: + if team_denial.type != ProxyErrorTypes.team_model_access_denied: + raise + if not await _key_access_group_grants_model( + model=_model, + valid_token=valid_token, + llm_router=llm_router, + ): + raise # 2.2. If team member has per-member model scope, enforce it if _model and team_object and valid_token and valid_token.user_id: @@ -2863,6 +2867,40 @@ async def can_team_access_model( raise +async def _key_access_group_grants_model( + model: Union[str, List[str]], + valid_token: Optional[UserAPIKeyAuth], + llm_router: Optional[Router], +) -> bool: + """ + Returns True if the key's `access_group_ids` expand to models that grant + access to `model`. Used to let a key's access group override a team's + model restriction in `common_checks`. + """ + if valid_token is None: + return False + key_access_group_ids = valid_token.access_group_ids or [] + if not key_access_group_ids: + return False + models_from_groups = await _get_models_from_access_groups( + access_group_ids=key_access_group_ids, + ) + if not models_from_groups: + return False + try: + _can_object_call_model( + model=model, + llm_router=llm_router, + models=models_from_groups, + team_model_aliases=valid_token.team_model_aliases, + team_id=valid_token.team_id, + object_type="key", + ) + return True + except ProxyException: + return False + + def can_project_access_model( model: Union[str, List[str]], project_object: LiteLLM_ProjectTableCachedObj, diff --git a/tests/proxy_unit_tests/test_auth_checks.py b/tests/proxy_unit_tests/test_auth_checks.py index 86cd5c0c41..6a404712c1 100644 --- a/tests/proxy_unit_tests/test_auth_checks.py +++ b/tests/proxy_unit_tests/test_auth_checks.py @@ -1146,3 +1146,84 @@ async def test_can_key_call_model_via_access_group_ids(): valid_token=user_api_key_object, llm_router=router, ) + + +# --------------------------------------------------------------------------- +# _key_access_group_grants_model (key access group overriding team restriction) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_key_access_group_grants_model_when_group_covers_model(): + """Key's access_group_ids expand to a set that includes the requested model.""" + from unittest.mock import AsyncMock, patch + + from litellm.proxy.auth.auth_checks import _key_access_group_grants_model + + valid_token = UserAPIKeyAuth( + token="test-token", + models=[], + access_group_ids=["ryan-access-group"], + ) + + with patch( + "litellm.proxy.auth.auth_checks._get_models_from_access_groups", + new_callable=AsyncMock, + return_value=["claude-haiku-4-5"], + ): + assert ( + await _key_access_group_grants_model( + model="claude-haiku-4-5", + valid_token=valid_token, + llm_router=None, + ) + is True + ) + + +@pytest.mark.asyncio +async def test_key_access_group_grants_model_when_key_has_no_groups(): + """Key with no access_group_ids cannot override team denial.""" + from litellm.proxy.auth.auth_checks import _key_access_group_grants_model + + valid_token = UserAPIKeyAuth( + token="test-token", + models=[], + access_group_ids=[], + ) + assert ( + await _key_access_group_grants_model( + model="claude-haiku-4-5", + valid_token=valid_token, + llm_router=None, + ) + is False + ) + + +@pytest.mark.asyncio +async def test_key_access_group_grants_model_when_group_does_not_cover_model(): + """Key's access_group_ids expand to models that do not include the request.""" + from unittest.mock import AsyncMock, patch + + from litellm.proxy.auth.auth_checks import _key_access_group_grants_model + + valid_token = UserAPIKeyAuth( + token="test-token", + models=[], + access_group_ids=["other-group"], + ) + + with patch( + "litellm.proxy.auth.auth_checks._get_models_from_access_groups", + new_callable=AsyncMock, + return_value=["gpt-4o-mini"], + ): + assert ( + await _key_access_group_grants_model( + model="claude-haiku-4-5", + valid_token=valid_token, + llm_router=None, + ) + is False + ) From 3c9a8690d1e61495916f1a0cef5e07020921e848 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Wed, 29 Apr 2026 22:18:18 +0000 Subject: [PATCH 02/25] fix(auth): gate oauth2-proxy header trust on premium + privileged-field denylist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``handle_oauth2_proxy_request`` reads HTTP request headers per the admin-set ``oauth2_config_mappings`` and constructs a ``UserAPIKeyAuth`` from the values. Two failure modes: 1. **Premium parity.** Sibling auth paths (``enable_oauth2_auth``, ``enable_jwt_auth``) require ``premium_user``; this path did not, so any open-source deployment could turn the feature on without realising it requires a hardened reverse-proxy topology. Added the ``premium_user`` gate. 2. **Privileged-field denylist.** Without a denylist, an admin who maps the wrong header to ``user_role`` (or whose reverse proxy leaks the header from upstream user input) lets any caller send ``X-User-Role: proxy_admin`` and gain full admin access — Pydantic coerces the string into the ``LitellmUserRoles.PROXY_ADMIN`` enum. Mapping any field in ``PRIVILEGED_OAUTH2_PROXY_FIELDS`` (``user_role``, ``api_key``, ``token``, ``permissions``, ``allowed_routes``, budget/limit fields, ``metadata``) raises at request time so the misconfiguration surfaces loudly rather than as a silent privesc. Operators who genuinely need a trusted upstream to assert one of these privileged fields should switch to JWT auth (signature-validated) rather than header-trust. Tests: - ``test_returns_auth_for_simple_user_id_mapping``: legitimate identity-only mapping still works. - ``test_rejects_when_not_premium``: open-source deployments get a clear enterprise-feature error. - ``test_refuses_to_map_privileged_fields``: parametrized over every entry in the denylist — each is rejected at request time. - ``test_user_role_header_forgery_attack_is_blocked``: end-to-end shape of the GHSA-5c3m-qffq-4r9m attack; rejected before auth object construction. - ``test_safe_fields_still_pass_through``: documented usage (``user_id``, ``user_email``, ``team_id``, ``models``) is unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) --- litellm/proxy/auth/oauth2_proxy_hook.py | 106 ++++++++-- .../proxy/auth/test_oauth2_proxy_hook.py | 189 ++++++++++++++++++ 2 files changed, 277 insertions(+), 18 deletions(-) create mode 100644 tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py diff --git a/litellm/proxy/auth/oauth2_proxy_hook.py b/litellm/proxy/auth/oauth2_proxy_hook.py index 0dc696bc45..341d2b477b 100644 --- a/litellm/proxy/auth/oauth2_proxy_hook.py +++ b/litellm/proxy/auth/oauth2_proxy_hook.py @@ -1,19 +1,78 @@ -from typing import Any, Dict +from typing import Any, Dict, FrozenSet from fastapi import Request from litellm._logging import verbose_proxy_logger -from litellm.proxy._types import UserAPIKeyAuth +from litellm.proxy._types import CommonProxyErrors, UserAPIKeyAuth + +# Fields on ``UserAPIKeyAuth`` that grant privileges directly (``user_role`` +# is the canonical privesc — coerced from the string ``"proxy_admin"`` into +# ``LitellmUserRoles.PROXY_ADMIN`` by Pydantic) or break trust assumptions +# (``api_key`` / ``token`` short-circuit the validated-key contract; +# ``permissions`` / ``allowed_routes`` directly grant route access; budget +# and limit fields can be set to wild values to bypass enforcement; +# ``metadata`` is too broad to safely admit from caller-controlled headers). +# +# Operators who legitimately need any of these to flow from a trusted +# upstream proxy should switch to JWT authentication, which validates a +# signature on the assertion rather than blindly trusting headers. +PRIVILEGED_OAUTH2_PROXY_FIELDS: FrozenSet[str] = frozenset( + { + "user_role", + "api_key", + "token", + "key_alias", + "key_name", + "permissions", + "allowed_routes", + "max_budget", + "spend", + "model_max_budget", + "model_spend", + "tpm_limit", + "rpm_limit", + "team_max_budget", + "team_spend", + "blocked", + "metadata", + } +) async def handle_oauth2_proxy_request(request: Request) -> UserAPIKeyAuth: """ - Handle request from oauth2 proxy. + Resolve a ``UserAPIKeyAuth`` from request headers per the admin-set + ``oauth2_config_mappings``. + + The auth model assumes the proxy is deployed behind a trusted OAuth2 + reverse proxy that injects authenticated identity headers (e.g. + oauth2-proxy, Authelia). Two safeguards above and beyond that + deployment assumption: + + 1. **Premium gate.** The sibling auth paths (``enable_oauth2_auth`` + and ``enable_jwt_auth``) require ``premium_user``; this path + previously did not, which let any open-source deployment turn + the feature on without realising it requires a hardened + deployment topology. + 2. **Privileged-field denylist.** ``oauth2_config_mappings`` maps + header names to ``UserAPIKeyAuth`` fields. Without a denylist, + an admin who maps the wrong header to ``user_role`` (or who + hasn't fully locked down their reverse proxy) lets any caller + set the ``user_role`` header to ``"proxy_admin"`` and gain full + admin privileges — Pydantic coerces the string into the enum. + Mapping any privileged field is rejected at startup-style auth + time so the misconfiguration surfaces loudly rather than as a + silent privesc. """ - from litellm.proxy.proxy_server import general_settings + from litellm.proxy.proxy_server import general_settings, premium_user + + if premium_user is not True: + raise ValueError( + "Oauth2 proxy auth is an enterprise-only feature. " + + CommonProxyErrors.not_premium_user.value + ) verbose_proxy_logger.debug("Handling oauth2 proxy request") - # Define the OAuth2 config mappings oauth2_config_mappings: Dict[str, str] = ( general_settings.get("oauth2_config_mappings") or {} ) @@ -21,21 +80,33 @@ async def handle_oauth2_proxy_request(request: Request) -> UserAPIKeyAuth: if not oauth2_config_mappings: raise ValueError("Oauth2 config mappings not found in general_settings") - # Initialize a dictionary to store the mapped values - auth_data: Dict[str, Any] = {} - # Extract values from headers based on the mappings + privileged_mapped = sorted( + set(oauth2_config_mappings.keys()) & PRIVILEGED_OAUTH2_PROXY_FIELDS + ) + if privileged_mapped: + raise ValueError( + "Oauth2 proxy auth refuses to map privileged UserAPIKeyAuth " + f"fields from request headers: {privileged_mapped}. These " + "fields would grant privileges (e.g. proxy_admin), bypass " + "budget enforcement, or short-circuit key validation if a " + "caller can spoof the corresponding header. If you need a " + "trusted upstream to assert one of these, use JWT auth " + "(signature-validated) instead of header-trust." + ) + + auth_data: Dict[str, Any] = {} for key, header in oauth2_config_mappings.items(): value = request.headers.get(header) - if value: - # Convert max_budget to float if present - if key == "max_budget": - auth_data[key] = float(value) - # Convert models to list if present - elif key == "models": - auth_data[key] = [model.strip() for model in value.split(",")] - else: - auth_data[key] = value + if not value: + continue + if key == "max_budget": + auth_data[key] = float(value) + elif key == "models": + auth_data[key] = [model.strip() for model in value.split(",")] + else: + auth_data[key] = value + verbose_proxy_logger.debug( "Auth data before creating UserAPIKeyAuth object: keys=%s", list(auth_data.keys()), @@ -45,5 +116,4 @@ async def handle_oauth2_proxy_request(request: Request) -> UserAPIKeyAuth: "UserAPIKeyAuth object created with keys: %s", list(user_api_key_auth.__fields_set__), ) - # Create and return UserAPIKeyAuth object return user_api_key_auth diff --git a/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py new file mode 100644 index 0000000000..87f8c45087 --- /dev/null +++ b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py @@ -0,0 +1,189 @@ +""" +Regression tests for the OAuth2-proxy header-forgery fix +(GHSA-5c3m-qffq-4r9m). + +The hook reads HTTP request headers per ``oauth2_config_mappings`` and +constructs a ``UserAPIKeyAuth`` from them. Two separate failure modes +the fix closes: + +1. The path was not gated on ``premium_user`` (the sibling + ``enable_oauth2_auth`` and ``enable_jwt_auth`` paths are). Open-source + deployments could enable the feature without realising it requires + a hardened deployment topology. +2. Any ``UserAPIKeyAuth`` field could be mapped from a header — including + ``user_role``, which Pydantic coerces from the string ``"proxy_admin"`` + into ``LitellmUserRoles.PROXY_ADMIN``. An attacker who reaches the + proxy directly (or via a misconfigured reverse proxy) sets the mapped + header and gains full admin privileges. +""" + +import os +import sys +from unittest.mock import patch + +import pytest +from fastapi import Request +from starlette.datastructures import Headers + +sys.path.insert(0, os.path.abspath("../../../..")) + +from litellm.proxy._types import LitellmUserRoles +from litellm.proxy.auth.oauth2_proxy_hook import ( + PRIVILEGED_OAUTH2_PROXY_FIELDS, + handle_oauth2_proxy_request, +) + + +def _request_with_headers(headers: dict) -> Request: + scope = { + "type": "http", + "headers": [(k.lower().encode(), v.encode()) for k, v in headers.items()], + } + request = Request(scope=scope) + request._headers = Headers(headers) + return request + + +@pytest.fixture +def premium_proxy_settings(monkeypatch): + """ + Patch the proxy_server module attributes the hook reads so each test + starts from "premium=True, mapping={user_id: x-user-id}". + """ + import litellm.proxy.proxy_server as proxy_server + + monkeypatch.setattr(proxy_server, "premium_user", True, raising=False) + monkeypatch.setattr( + proxy_server, + "general_settings", + {"oauth2_config_mappings": {"user_id": "x-user-id"}}, + raising=False, + ) + + +@pytest.mark.asyncio +async def test_returns_auth_for_simple_user_id_mapping(premium_proxy_settings): + request = _request_with_headers({"x-user-id": "alice"}) + + auth = await handle_oauth2_proxy_request(request) + + assert auth.user_id == "alice" + assert auth.user_role is None + + +@pytest.mark.asyncio +async def test_rejects_when_not_premium(monkeypatch): + import litellm.proxy.proxy_server as proxy_server + + monkeypatch.setattr(proxy_server, "premium_user", False, raising=False) + monkeypatch.setattr( + proxy_server, + "general_settings", + {"oauth2_config_mappings": {"user_id": "x-user-id"}}, + raising=False, + ) + request = _request_with_headers({"x-user-id": "alice"}) + + with pytest.raises(ValueError, match="enterprise"): + await handle_oauth2_proxy_request(request) + + +@pytest.mark.parametrize( + "privileged_field", + sorted(PRIVILEGED_OAUTH2_PROXY_FIELDS), +) +@pytest.mark.asyncio +async def test_refuses_to_map_privileged_fields(monkeypatch, privileged_field): + """ + The exact privesc shape from GHSA-5c3m-qffq-4r9m: an admin maps + ``user_role`` (or any other privileged field) to a header and a + caller forges ``X-User-Role: proxy_admin``. The hook must reject + this configuration outright at request time. + """ + import litellm.proxy.proxy_server as proxy_server + + monkeypatch.setattr(proxy_server, "premium_user", True, raising=False) + monkeypatch.setattr( + proxy_server, + "general_settings", + {"oauth2_config_mappings": {privileged_field: f"x-{privileged_field}"}}, + raising=False, + ) + request = _request_with_headers({f"x-{privileged_field}": "proxy_admin"}) + + with pytest.raises(ValueError) as exc: + await handle_oauth2_proxy_request(request) + assert privileged_field in str(exc.value) + + +@pytest.mark.asyncio +async def test_user_role_header_forgery_attack_is_blocked(monkeypatch): + """ + End-to-end shape from the GHSA: with ``user_role`` mapped, a forged + ``X-User-Role: proxy_admin`` header would have produced a + ``UserAPIKeyAuth`` with PROXY_ADMIN role. Now the request raises + before any auth object is constructed. + """ + import litellm.proxy.proxy_server as proxy_server + + monkeypatch.setattr(proxy_server, "premium_user", True, raising=False) + monkeypatch.setattr( + proxy_server, + "general_settings", + { + "oauth2_config_mappings": { + "user_id": "x-user-id", + "user_role": "x-user-role", + } + }, + raising=False, + ) + request = _request_with_headers( + { + "x-user-id": "attacker", + "x-user-role": LitellmUserRoles.PROXY_ADMIN.value, + } + ) + + with pytest.raises(ValueError, match="user_role"): + await handle_oauth2_proxy_request(request) + + +@pytest.mark.asyncio +async def test_safe_fields_still_pass_through(monkeypatch): + """ + Sanity check that non-privileged fields (the documented use case + for OAuth2 proxy auth — asserting identity from a trusted upstream) + still work after the fix. + """ + import litellm.proxy.proxy_server as proxy_server + + monkeypatch.setattr(proxy_server, "premium_user", True, raising=False) + monkeypatch.setattr( + proxy_server, + "general_settings", + { + "oauth2_config_mappings": { + "user_id": "x-user-id", + "user_email": "x-user-email", + "team_id": "x-team-id", + "models": "x-models", + } + }, + raising=False, + ) + request = _request_with_headers( + { + "x-user-id": "alice", + "x-user-email": "alice@example.com", + "x-team-id": "team-corp", + "x-models": "gpt-4, gpt-3.5-turbo", + } + ) + + auth = await handle_oauth2_proxy_request(request) + + assert auth.user_id == "alice" + assert auth.user_email == "alice@example.com" + assert auth.team_id == "team-corp" + assert auth.models == ["gpt-4", "gpt-3.5-turbo"] From e6867c143ae831ed9c6927034f4233048711820a Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Wed, 29 Apr 2026 22:23:29 +0000 Subject: [PATCH 03/25] =?UTF-8?q?chore(oauth2-proxy):=20/simplify=20pass?= =?UTF-8?q?=20=E2=80=94=20drop=20dead=20max=5Fbudget=20branch=20+=20DRY=20?= =?UTF-8?q?tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cleanups from the /simplify review pass: * The header-mapping loop had a special-case ``if key == "max_budget": auth_data[key] = float(value)`` branch. Since ``max_budget`` is now in ``PRIVILEGED_OAUTH2_PROXY_FIELDS``, the denylist check rejects the configuration before the loop runs — the float-conversion branch is unreachable. Removed. * Four tests independently called ``monkeypatch.setattr(proxy_server, "premium_user", ...)`` and ``monkeypatch.setattr(proxy_server, "general_settings", ...)`` with almost-identical bodies. Replaced with a ``configure_proxy`` fixture that yields a single callable — ``configure_proxy(premium=False)`` / ``configure_proxy(mappings={...})`` — so each test's setup is one line. The previously-unused ``premium_proxy_settings`` fixture is removed. Co-Authored-By: Claude Opus 4.7 (1M context) --- litellm/proxy/auth/oauth2_proxy_hook.py | 4 +- .../proxy/auth/test_oauth2_proxy_hook.py | 119 +++++++----------- 2 files changed, 43 insertions(+), 80 deletions(-) diff --git a/litellm/proxy/auth/oauth2_proxy_hook.py b/litellm/proxy/auth/oauth2_proxy_hook.py index 341d2b477b..0f7cfa4222 100644 --- a/litellm/proxy/auth/oauth2_proxy_hook.py +++ b/litellm/proxy/auth/oauth2_proxy_hook.py @@ -100,9 +100,7 @@ async def handle_oauth2_proxy_request(request: Request) -> UserAPIKeyAuth: value = request.headers.get(header) if not value: continue - if key == "max_budget": - auth_data[key] = float(value) - elif key == "models": + if key == "models": auth_data[key] = [model.strip() for model in value.split(",")] else: auth_data[key] = value diff --git a/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py index 87f8c45087..e51882a3b1 100644 --- a/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py +++ b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py @@ -45,24 +45,32 @@ def _request_with_headers(headers: dict) -> Request: @pytest.fixture -def premium_proxy_settings(monkeypatch): +def configure_proxy(monkeypatch): """ - Patch the proxy_server module attributes the hook reads so each test - starts from "premium=True, mapping={user_id: x-user-id}". + Yields a callable that sets ``premium_user`` and + ``oauth2_config_mappings`` on the proxy_server module for the + duration of one test. Default is premium=True with a single + ``user_id -> x-user-id`` mapping. """ import litellm.proxy.proxy_server as proxy_server - monkeypatch.setattr(proxy_server, "premium_user", True, raising=False) - monkeypatch.setattr( - proxy_server, - "general_settings", - {"oauth2_config_mappings": {"user_id": "x-user-id"}}, - raising=False, - ) + def _configure(*, premium=True, mappings=None): + if mappings is None: + mappings = {"user_id": "x-user-id"} + monkeypatch.setattr(proxy_server, "premium_user", premium, raising=False) + monkeypatch.setattr( + proxy_server, + "general_settings", + {"oauth2_config_mappings": mappings}, + raising=False, + ) + + return _configure @pytest.mark.asyncio -async def test_returns_auth_for_simple_user_id_mapping(premium_proxy_settings): +async def test_returns_auth_for_simple_user_id_mapping(configure_proxy): + configure_proxy() request = _request_with_headers({"x-user-id": "alice"}) auth = await handle_oauth2_proxy_request(request) @@ -72,16 +80,8 @@ async def test_returns_auth_for_simple_user_id_mapping(premium_proxy_settings): @pytest.mark.asyncio -async def test_rejects_when_not_premium(monkeypatch): - import litellm.proxy.proxy_server as proxy_server - - monkeypatch.setattr(proxy_server, "premium_user", False, raising=False) - monkeypatch.setattr( - proxy_server, - "general_settings", - {"oauth2_config_mappings": {"user_id": "x-user-id"}}, - raising=False, - ) +async def test_rejects_when_not_premium(configure_proxy): + configure_proxy(premium=False) request = _request_with_headers({"x-user-id": "alice"}) with pytest.raises(ValueError, match="enterprise"): @@ -93,22 +93,11 @@ async def test_rejects_when_not_premium(monkeypatch): sorted(PRIVILEGED_OAUTH2_PROXY_FIELDS), ) @pytest.mark.asyncio -async def test_refuses_to_map_privileged_fields(monkeypatch, privileged_field): - """ - The exact privesc shape from GHSA-5c3m-qffq-4r9m: an admin maps - ``user_role`` (or any other privileged field) to a header and a - caller forges ``X-User-Role: proxy_admin``. The hook must reject - this configuration outright at request time. - """ - import litellm.proxy.proxy_server as proxy_server - - monkeypatch.setattr(proxy_server, "premium_user", True, raising=False) - monkeypatch.setattr( - proxy_server, - "general_settings", - {"oauth2_config_mappings": {privileged_field: f"x-{privileged_field}"}}, - raising=False, - ) +async def test_refuses_to_map_privileged_fields(configure_proxy, privileged_field): + # GHSA-5c3m-qffq-4r9m attack shape: admin maps a privileged field + # to a header and a caller forges the value. The hook must reject + # the misconfiguration outright at request time. + configure_proxy(mappings={privileged_field: f"x-{privileged_field}"}) request = _request_with_headers({f"x-{privileged_field}": "proxy_admin"}) with pytest.raises(ValueError) as exc: @@ -117,26 +106,13 @@ async def test_refuses_to_map_privileged_fields(monkeypatch, privileged_field): @pytest.mark.asyncio -async def test_user_role_header_forgery_attack_is_blocked(monkeypatch): - """ - End-to-end shape from the GHSA: with ``user_role`` mapped, a forged - ``X-User-Role: proxy_admin`` header would have produced a - ``UserAPIKeyAuth`` with PROXY_ADMIN role. Now the request raises - before any auth object is constructed. - """ - import litellm.proxy.proxy_server as proxy_server - - monkeypatch.setattr(proxy_server, "premium_user", True, raising=False) - monkeypatch.setattr( - proxy_server, - "general_settings", - { - "oauth2_config_mappings": { - "user_id": "x-user-id", - "user_role": "x-user-role", - } - }, - raising=False, +async def test_user_role_header_forgery_attack_is_blocked(configure_proxy): + # End-to-end form of the privesc: with ``user_role`` mapped, the + # forged ``X-User-Role: proxy_admin`` header would have produced + # a ``UserAPIKeyAuth(user_role=PROXY_ADMIN)``. Now rejected before + # any auth object is constructed. + configure_proxy( + mappings={"user_id": "x-user-id", "user_role": "x-user-role"}, ) request = _request_with_headers( { @@ -150,27 +126,16 @@ async def test_user_role_header_forgery_attack_is_blocked(monkeypatch): @pytest.mark.asyncio -async def test_safe_fields_still_pass_through(monkeypatch): - """ - Sanity check that non-privileged fields (the documented use case - for OAuth2 proxy auth — asserting identity from a trusted upstream) - still work after the fix. - """ - import litellm.proxy.proxy_server as proxy_server - - monkeypatch.setattr(proxy_server, "premium_user", True, raising=False) - monkeypatch.setattr( - proxy_server, - "general_settings", - { - "oauth2_config_mappings": { - "user_id": "x-user-id", - "user_email": "x-user-email", - "team_id": "x-team-id", - "models": "x-models", - } +async def test_safe_fields_still_pass_through(configure_proxy): + # The documented use case for OAuth2 proxy auth: identity assertion + # from a trusted upstream. Must remain unaffected by the denylist. + configure_proxy( + mappings={ + "user_id": "x-user-id", + "user_email": "x-user-email", + "team_id": "x-team-id", + "models": "x-models", }, - raising=False, ) request = _request_with_headers( { From b35287a062dbdd99eac053223f099200b12db9c0 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Wed, 29 Apr 2026 22:28:57 +0000 Subject: [PATCH 04/25] fix(oauth2-proxy): switch privileged-field denylist to identity-only allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile flagged that the denylist was incomplete: ``user_max_budget``, ``user_tpm_limit``, ``user_rpm_limit``, and ``user_spend`` were not on it. Inspection of the auth model showed dozens more privileged fields across the ``LiteLLM_VerificationTokenView`` hierarchy (team / org / end-user / region budget / spend / limit fields, plus ``allowed_model_region``, ``rpm_limit_per_model``, etc.) — a denylist of "privileged fields" is unmaintainable here. Inverted the model. ``ALLOWED_OAUTH2_PROXY_FIELDS`` is now an identity-only allowlist: ``user_id``, ``user_email``, ``team_id``, ``team_alias``, ``org_id``, ``models``. Any mapping to a non-identity field is rejected at request time. Default-secure: a future field added to ``UserAPIKeyAuth`` is automatically blocked from header-trust. Use case for OAuth2-proxy auth is identity assertion from a trusted upstream. Anything beyond that (privileges, budgets, rate limits) is policy and should be authenticated with a signature, not a header — operators who need this should switch to JWT auth. Tests: - ``test_refuses_to_map_non_identity_fields`` parametrized over 22 fields including all four ``user_*`` Greptile flagged, plus team/org/end-user budget/limit fields, plus a fabricated field name to confirm "anything not on the allowlist" is the rule. - ``test_allowlist_is_identity_only`` locks in the allowlist's intent so future additions of budget / role / permission entries are caught in review. Co-Authored-By: Claude Opus 4.7 (1M context) --- litellm/proxy/auth/oauth2_proxy_hook.py | 87 +++++++++---------- .../proxy/auth/test_oauth2_proxy_hook.py | 59 +++++++++++-- 2 files changed, 96 insertions(+), 50 deletions(-) diff --git a/litellm/proxy/auth/oauth2_proxy_hook.py b/litellm/proxy/auth/oauth2_proxy_hook.py index 0f7cfa4222..1ba1b100a8 100644 --- a/litellm/proxy/auth/oauth2_proxy_hook.py +++ b/litellm/proxy/auth/oauth2_proxy_hook.py @@ -5,36 +5,32 @@ from fastapi import Request from litellm._logging import verbose_proxy_logger from litellm.proxy._types import CommonProxyErrors, UserAPIKeyAuth -# Fields on ``UserAPIKeyAuth`` that grant privileges directly (``user_role`` -# is the canonical privesc — coerced from the string ``"proxy_admin"`` into -# ``LitellmUserRoles.PROXY_ADMIN`` by Pydantic) or break trust assumptions -# (``api_key`` / ``token`` short-circuit the validated-key contract; -# ``permissions`` / ``allowed_routes`` directly grant route access; budget -# and limit fields can be set to wild values to bypass enforcement; -# ``metadata`` is too broad to safely admit from caller-controlled headers). +# OAuth2-proxy header trust is for **identity assertion** from a trusted +# upstream auth proxy (oauth2-proxy, Authelia, etc.). The allowlist below +# is the only safe surface — anything else (``user_role``, ``api_key``, +# ``permissions``, ``max_budget``, ``user_max_budget``, +# ``team_tpm_limit``, ``end_user_max_budget``, ``allowed_model_region``, +# and dozens of similar policy fields scattered across the +# ``LiteLLM_VerificationTokenView`` hierarchy) is a privilege grant that +# would let a caller forge their own enforcement parameters by sending +# the matching header. # -# Operators who legitimately need any of these to flow from a trusted -# upstream proxy should switch to JWT authentication, which validates a +# A denylist of "privileged fields" is unmaintainable in this codebase: +# the auth model has ~50 budget/spend/limit/permission fields and gains +# more with each release. An allowlist scoped to identity assertion is +# default-secure — new fields are blocked automatically. +# +# Operators who need a trusted upstream to assert anything beyond +# identity should switch to JWT authentication, which validates a # signature on the assertion rather than blindly trusting headers. -PRIVILEGED_OAUTH2_PROXY_FIELDS: FrozenSet[str] = frozenset( +ALLOWED_OAUTH2_PROXY_FIELDS: FrozenSet[str] = frozenset( { - "user_role", - "api_key", - "token", - "key_alias", - "key_name", - "permissions", - "allowed_routes", - "max_budget", - "spend", - "model_max_budget", - "model_spend", - "tpm_limit", - "rpm_limit", - "team_max_budget", - "team_spend", - "blocked", - "metadata", + "user_id", + "user_email", + "team_id", + "team_alias", + "org_id", + "models", } ) @@ -54,15 +50,15 @@ async def handle_oauth2_proxy_request(request: Request) -> UserAPIKeyAuth: previously did not, which let any open-source deployment turn the feature on without realising it requires a hardened deployment topology. - 2. **Privileged-field denylist.** ``oauth2_config_mappings`` maps - header names to ``UserAPIKeyAuth`` fields. Without a denylist, - an admin who maps the wrong header to ``user_role`` (or who - hasn't fully locked down their reverse proxy) lets any caller - set the ``user_role`` header to ``"proxy_admin"`` and gain full - admin privileges — Pydantic coerces the string into the enum. - Mapping any privileged field is rejected at startup-style auth - time so the misconfiguration surfaces loudly rather than as a - silent privesc. + 2. **Identity-only allowlist.** ``oauth2_config_mappings`` maps + header names to ``UserAPIKeyAuth`` fields. Without an allowlist, + an admin who maps the wrong header to ``user_role`` lets any + caller send ``X-User-Role: proxy_admin`` and gain full admin + privileges (Pydantic coerces the string into the enum). Only + fields in ``ALLOWED_OAUTH2_PROXY_FIELDS`` (identity assertion + only — see the constant's comment) may be mapped; any other + mapping is rejected at request time so the misconfiguration + surfaces loudly rather than as a silent privesc. """ from litellm.proxy.proxy_server import general_settings, premium_user @@ -81,17 +77,18 @@ async def handle_oauth2_proxy_request(request: Request) -> UserAPIKeyAuth: if not oauth2_config_mappings: raise ValueError("Oauth2 config mappings not found in general_settings") - privileged_mapped = sorted( - set(oauth2_config_mappings.keys()) & PRIVILEGED_OAUTH2_PROXY_FIELDS + disallowed = sorted( + set(oauth2_config_mappings.keys()) - ALLOWED_OAUTH2_PROXY_FIELDS ) - if privileged_mapped: + if disallowed: raise ValueError( - "Oauth2 proxy auth refuses to map privileged UserAPIKeyAuth " - f"fields from request headers: {privileged_mapped}. These " - "fields would grant privileges (e.g. proxy_admin), bypass " - "budget enforcement, or short-circuit key validation if a " - "caller can spoof the corresponding header. If you need a " - "trusted upstream to assert one of these, use JWT auth " + "Oauth2 proxy auth refuses to map non-identity UserAPIKeyAuth " + f"fields from request headers: {disallowed}. Only identity " + f"fields are accepted ({sorted(ALLOWED_OAUTH2_PROXY_FIELDS)}); " + "anything else (privileges, budgets, rate limits, metadata) " + "would let a caller forge enforcement parameters by spoofing " + "the matching header. If you need a trusted upstream to " + "assert anything beyond identity, use JWT auth " "(signature-validated) instead of header-trust." ) diff --git a/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py index e51882a3b1..e73ac571d5 100644 --- a/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py +++ b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py @@ -29,7 +29,7 @@ sys.path.insert(0, os.path.abspath("../../../..")) from litellm.proxy._types import LitellmUserRoles from litellm.proxy.auth.oauth2_proxy_hook import ( - PRIVILEGED_OAUTH2_PROXY_FIELDS, + ALLOWED_OAUTH2_PROXY_FIELDS, handle_oauth2_proxy_request, ) @@ -90,13 +90,46 @@ async def test_rejects_when_not_premium(configure_proxy): @pytest.mark.parametrize( "privileged_field", - sorted(PRIVILEGED_OAUTH2_PROXY_FIELDS), + [ + # The GHSA-5c3m-qffq-4r9m primary privesc field. + "user_role", + # Key-level enforcement bypass shapes. + "api_key", + "token", + "permissions", + "allowed_routes", + "max_budget", + "spend", + "tpm_limit", + "rpm_limit", + "model_max_budget", + "metadata", + # User-level enforcement bypass — flagged by Greptile as a denylist gap. + "user_max_budget", + "user_tpm_limit", + "user_rpm_limit", + "user_spend", + # Team / org / end-user / region — same class, all denied by the + # identity-only allowlist. + "team_max_budget", + "team_spend", + "team_member_tpm_limit", + "organization_max_budget", + "organization_tpm_limit", + "end_user_max_budget", + "allowed_model_region", + # Anything not on ALLOWED_OAUTH2_PROXY_FIELDS is blocked, even + # fabricated field names admins might try. + "definitely_not_a_real_field", + ], ) @pytest.mark.asyncio -async def test_refuses_to_map_privileged_fields(configure_proxy, privileged_field): +async def test_refuses_to_map_non_identity_fields(configure_proxy, privileged_field): # GHSA-5c3m-qffq-4r9m attack shape: admin maps a privileged field - # to a header and a caller forges the value. The hook must reject - # the misconfiguration outright at request time. + # to a header and a caller forges the value. The allowlist rejects + # any non-identity mapping at request time, regardless of whether + # the field ever appeared on a denylist — which is the whole reason + # we use an allowlist instead. configure_proxy(mappings={privileged_field: f"x-{privileged_field}"}) request = _request_with_headers({f"x-{privileged_field}": "proxy_admin"}) @@ -105,6 +138,22 @@ async def test_refuses_to_map_privileged_fields(configure_proxy, privileged_fiel assert privileged_field in str(exc.value) +@pytest.mark.parametrize("identity_field", sorted(ALLOWED_OAUTH2_PROXY_FIELDS)) +def test_allowlist_is_identity_only(identity_field): + # Lock in the allowlist's intent: only identity-assertion fields are + # safe to populate from a header. If anyone proposes adding budget / + # spend / role / permission to ``ALLOWED_OAUTH2_PROXY_FIELDS``, this + # assertion forces them to update the test deliberately. + assert identity_field in { + "user_id", + "user_email", + "team_id", + "team_alias", + "org_id", + "models", + } + + @pytest.mark.asyncio async def test_user_role_header_forgery_attack_is_blocked(configure_proxy): # End-to-end form of the privesc: with ``user_role`` mapped, the From fbcfd59b1a23edc17d6a93880726f26e308efedd Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Wed, 29 Apr 2026 23:04:05 +0000 Subject: [PATCH 05/25] fix(oauth2-proxy): drop premium gate; identity-only allowlist is the security fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile flagged the ``premium_user is not True`` check as a hard backwards-incompatible break for OSS users currently running ``enable_oauth2_proxy_auth=True``. They were right: unlike the api_base case (where the docs already required admin opt-in), this path was documented as available to OSS users. Adding the gate would have closed a documented feature, not fixed a vuln. Reframed the change: * The **identity-only allowlist** (``ALLOWED_OAUTH2_PROXY_FIELDS`` = ``{user_id, user_email, team_id, team_alias, org_id, models}``) is the actual security fix — it closes the privesc by rejecting any mapping to a non-identity field at request time. This is unchanged. * The **premium gate** was parity-with-siblings (a product decision, not a security one). Removed. BerriAI can re-add it on their own schedule with a proper deprecation cycle if they want enterprise- only gating. Tests: removed ``test_rejects_when_not_premium``; everything else (allowlist enforcement, identity passthrough, attack-shape regression) still passes — 14 tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- litellm/proxy/auth/oauth2_proxy_hook.py | 36 +++++++------------ .../proxy/auth/test_oauth2_proxy_hook.py | 19 +++------- 2 files changed, 16 insertions(+), 39 deletions(-) diff --git a/litellm/proxy/auth/oauth2_proxy_hook.py b/litellm/proxy/auth/oauth2_proxy_hook.py index 1ba1b100a8..389a5b2b9e 100644 --- a/litellm/proxy/auth/oauth2_proxy_hook.py +++ b/litellm/proxy/auth/oauth2_proxy_hook.py @@ -3,7 +3,7 @@ from typing import Any, Dict, FrozenSet from fastapi import Request from litellm._logging import verbose_proxy_logger -from litellm.proxy._types import CommonProxyErrors, UserAPIKeyAuth +from litellm.proxy._types import UserAPIKeyAuth # OAuth2-proxy header trust is for **identity assertion** from a trusted # upstream auth proxy (oauth2-proxy, Authelia, etc.). The allowlist below @@ -42,31 +42,19 @@ async def handle_oauth2_proxy_request(request: Request) -> UserAPIKeyAuth: The auth model assumes the proxy is deployed behind a trusted OAuth2 reverse proxy that injects authenticated identity headers (e.g. - oauth2-proxy, Authelia). Two safeguards above and beyond that - deployment assumption: + oauth2-proxy, Authelia). - 1. **Premium gate.** The sibling auth paths (``enable_oauth2_auth`` - and ``enable_jwt_auth``) require ``premium_user``; this path - previously did not, which let any open-source deployment turn - the feature on without realising it requires a hardened - deployment topology. - 2. **Identity-only allowlist.** ``oauth2_config_mappings`` maps - header names to ``UserAPIKeyAuth`` fields. Without an allowlist, - an admin who maps the wrong header to ``user_role`` lets any - caller send ``X-User-Role: proxy_admin`` and gain full admin - privileges (Pydantic coerces the string into the enum). Only - fields in ``ALLOWED_OAUTH2_PROXY_FIELDS`` (identity assertion - only — see the constant's comment) may be mapped; any other - mapping is rejected at request time so the misconfiguration - surfaces loudly rather than as a silent privesc. + **Identity-only allowlist.** ``oauth2_config_mappings`` maps header + names to ``UserAPIKeyAuth`` fields. Without an allowlist, an admin + who maps the wrong header to ``user_role`` lets any caller send + ``X-User-Role: proxy_admin`` and gain full admin privileges + (Pydantic coerces the string into the enum). Only fields in + ``ALLOWED_OAUTH2_PROXY_FIELDS`` (identity assertion only — see the + constant's comment) may be mapped; any other mapping is rejected at + request time so the misconfiguration surfaces loudly rather than as + a silent privesc. """ - from litellm.proxy.proxy_server import general_settings, premium_user - - if premium_user is not True: - raise ValueError( - "Oauth2 proxy auth is an enterprise-only feature. " - + CommonProxyErrors.not_premium_user.value - ) + from litellm.proxy.proxy_server import general_settings verbose_proxy_logger.debug("Handling oauth2 proxy request") oauth2_config_mappings: Dict[str, str] = ( diff --git a/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py index e73ac571d5..42af9e6f03 100644 --- a/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py +++ b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py @@ -47,17 +47,15 @@ def _request_with_headers(headers: dict) -> Request: @pytest.fixture def configure_proxy(monkeypatch): """ - Yields a callable that sets ``premium_user`` and - ``oauth2_config_mappings`` on the proxy_server module for the - duration of one test. Default is premium=True with a single - ``user_id -> x-user-id`` mapping. + Yields a callable that sets ``oauth2_config_mappings`` on the + proxy_server module for the duration of one test. Default mapping + is a single ``user_id -> x-user-id`` (identity-only). """ import litellm.proxy.proxy_server as proxy_server - def _configure(*, premium=True, mappings=None): + def _configure(*, mappings=None): if mappings is None: mappings = {"user_id": "x-user-id"} - monkeypatch.setattr(proxy_server, "premium_user", premium, raising=False) monkeypatch.setattr( proxy_server, "general_settings", @@ -79,15 +77,6 @@ async def test_returns_auth_for_simple_user_id_mapping(configure_proxy): assert auth.user_role is None -@pytest.mark.asyncio -async def test_rejects_when_not_premium(configure_proxy): - configure_proxy(premium=False) - request = _request_with_headers({"x-user-id": "alice"}) - - with pytest.raises(ValueError, match="enterprise"): - await handle_oauth2_proxy_request(request) - - @pytest.mark.parametrize( "privileged_field", [ From 722bc63e37d5a3773f95bb6c22b11f74bb3cb65e Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Wed, 29 Apr 2026 23:47:29 +0000 Subject: [PATCH 06/25] chore(oauth2-proxy): drop unused patch import + tighten docstring Greptile flagged the unused ``from unittest.mock import patch`` left over from before the ``configure_proxy`` fixture refactor (the fixture uses ``monkeypatch``, no ``patch`` calls remain). Also pruned the now-stale "premium gate" paragraph from the module docstring since that gate was removed in fbcfd59b1a. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../proxy/auth/test_oauth2_proxy_hook.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py index 42af9e6f03..9d0bdcf351 100644 --- a/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py +++ b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py @@ -3,23 +3,16 @@ Regression tests for the OAuth2-proxy header-forgery fix (GHSA-5c3m-qffq-4r9m). The hook reads HTTP request headers per ``oauth2_config_mappings`` and -constructs a ``UserAPIKeyAuth`` from them. Two separate failure modes -the fix closes: - -1. The path was not gated on ``premium_user`` (the sibling - ``enable_oauth2_auth`` and ``enable_jwt_auth`` paths are). Open-source - deployments could enable the feature without realising it requires - a hardened deployment topology. -2. Any ``UserAPIKeyAuth`` field could be mapped from a header — including - ``user_role``, which Pydantic coerces from the string ``"proxy_admin"`` - into ``LitellmUserRoles.PROXY_ADMIN``. An attacker who reaches the - proxy directly (or via a misconfigured reverse proxy) sets the mapped - header and gains full admin privileges. +constructs a ``UserAPIKeyAuth`` from them. Without the +identity-only allowlist any field could be mapped — including +``user_role``, which Pydantic coerces from the string +``"proxy_admin"`` into ``LitellmUserRoles.PROXY_ADMIN``. An attacker +who reaches the proxy directly (or via a misconfigured reverse +proxy) sets the mapped header and gains full admin privileges. """ import os import sys -from unittest.mock import patch import pytest from fastapi import Request From 2f4641752bd9f8bf325c43c1dc6cd5d85e322bcb Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Wed, 29 Apr 2026 21:20:21 -0700 Subject: [PATCH 07/25] chore(auth): require trusted proxy for header identity auth --- .../proxy/auth/custom_sso_handler.py | 54 ++++--- litellm/integrations/custom_sso_handler.py | 11 ++ litellm/proxy/_types.py | 4 + litellm/proxy/auth/oauth2_proxy_hook.py | 7 + litellm/proxy/auth/trusted_proxy_utils.py | 118 +++++++++++++++ .../proxy/auth/test_oauth2_proxy_hook.py | 50 +++++-- .../proxy/management_endpoints/test_ui_sso.py | 141 +++++++++++------- 7 files changed, 300 insertions(+), 85 deletions(-) create mode 100644 litellm/proxy/auth/trusted_proxy_utils.py diff --git a/enterprise/litellm_enterprise/proxy/auth/custom_sso_handler.py b/enterprise/litellm_enterprise/proxy/auth/custom_sso_handler.py index a368232038..e8f104c262 100644 --- a/enterprise/litellm_enterprise/proxy/auth/custom_sso_handler.py +++ b/enterprise/litellm_enterprise/proxy/auth/custom_sso_handler.py @@ -10,28 +10,21 @@ has already authenticated the user) and you need to extract user information fro custom headers or other request attributes. """ -from typing import TYPE_CHECKING, Dict, Optional, Union, cast +from typing import cast from fastapi import Request from fastapi.responses import RedirectResponse -if TYPE_CHECKING: - from fastapi_sso.sso.base import OpenID -else: - from typing import Any as OpenID - -from litellm.proxy.management_endpoints.types import CustomOpenID - class EnterpriseCustomSSOHandler: """ Enterprise Custom SSO Handler for LiteLLM Proxy - + This class provides methods for handling custom SSO authentication flows where users can implement their own authentication logic by processing request headers and returning user information in OpenID format. """ - + @staticmethod async def handle_custom_ui_sso_sign_in( request: Request, @@ -40,16 +33,16 @@ class EnterpriseCustomSSOHandler: Allow a user to execute their custom code to parse incoming request headers and return a OpenID object Use this when you have an OAuth proxy in front of LiteLLM (where the OAuth proxy has already authenticated the user) - + Args: request: The FastAPI request object containing headers and other request data - + Returns: RedirectResponse: Redirect response that sends the user to the LiteLLM UI with authentication token - + Raises: ValueError: If custom_ui_sso_sign_in_handler is not configured - + Example: This method is typically called when a user has already been authenticated by an external OAuth proxy and the proxy has added custom headers containing user information. @@ -60,27 +53,44 @@ class EnterpriseCustomSSOHandler: from litellm.integrations.custom_sso_handler import CustomSSOLoginHandler from litellm.proxy.proxy_server import ( CommonProxyErrors, + general_settings, premium_user, user_custom_ui_sso_sign_in_handler, ) + from litellm.proxy.auth.trusted_proxy_utils import ( + require_trusted_proxy_request, + ) + if premium_user is not True: raise ValueError(CommonProxyErrors.not_premium_user.value) - + if user_custom_ui_sso_sign_in_handler is None: - raise ValueError("custom_ui_sso_sign_in_handler is not configured. Please set it in general_settings.") - - custom_sso_login_handler = cast(CustomSSOLoginHandler, user_custom_ui_sso_sign_in_handler) - openid_response: OpenID = await custom_sso_login_handler.handle_custom_ui_sso_sign_in( + raise ValueError( + "custom_ui_sso_sign_in_handler is not configured. Please set it in general_settings." + ) + + require_trusted_proxy_request( request=request, + general_settings=general_settings, + feature_name="Custom UI SSO", ) - + + custom_sso_login_handler = cast( + CustomSSOLoginHandler, user_custom_ui_sso_sign_in_handler + ) + openid_response: OpenID = ( + await custom_sso_login_handler.handle_custom_ui_sso_sign_in( + request=request, + ) + ) + # Import here to avoid circular imports from litellm.proxy.management_endpoints.ui_sso import SSOAuthenticationHandler - + return await SSOAuthenticationHandler.get_redirect_response_from_openid( result=openid_response, request=request, received_response=None, generic_client_id=None, ui_access_mode=None, - ) \ No newline at end of file + ) diff --git a/litellm/integrations/custom_sso_handler.py b/litellm/integrations/custom_sso_handler.py index 7f60decabc..202e488e0e 100644 --- a/litellm/integrations/custom_sso_handler.py +++ b/litellm/integrations/custom_sso_handler.py @@ -18,6 +18,17 @@ class CustomSSOLoginHandler(CustomLogger): self, request: Request, ) -> OpenID: + from litellm.proxy.auth.trusted_proxy_utils import ( + require_trusted_proxy_request, + ) + from litellm.proxy.proxy_server import general_settings + + require_trusted_proxy_request( + request=request, + general_settings=general_settings, + feature_name="Custom UI SSO", + ) + request_headers_dict = dict(request.headers) return OpenID( id=request_headers_dict.get("x-litellm-user-id"), diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 92c920ca59..5165c7fd50 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -2374,6 +2374,10 @@ class ConfigGeneralSettings(LiteLLMPydanticObjectBase): None, description="CIDR ranges of trusted reverse proxies. When set, X-Forwarded-For headers are only trusted from these IPs.", ) + trusted_proxy_ranges: Optional[List[str]] = Field( + None, + description="CIDR ranges of trusted reverse proxies allowed to provide identity headers for header-based auth paths such as enable_oauth2_proxy_auth and custom_ui_sso_sign_in_handler.", + ) store_model_in_db: Optional[bool] = Field( None, description="If True, models and config are stored in and loaded from the database. Default is False.", diff --git a/litellm/proxy/auth/oauth2_proxy_hook.py b/litellm/proxy/auth/oauth2_proxy_hook.py index 389a5b2b9e..9fc4c4fb53 100644 --- a/litellm/proxy/auth/oauth2_proxy_hook.py +++ b/litellm/proxy/auth/oauth2_proxy_hook.py @@ -4,6 +4,7 @@ from fastapi import Request from litellm._logging import verbose_proxy_logger from litellm.proxy._types import UserAPIKeyAuth +from litellm.proxy.auth.trusted_proxy_utils import require_trusted_proxy_request # OAuth2-proxy header trust is for **identity assertion** from a trusted # upstream auth proxy (oauth2-proxy, Authelia, etc.). The allowlist below @@ -57,6 +58,12 @@ async def handle_oauth2_proxy_request(request: Request) -> UserAPIKeyAuth: from litellm.proxy.proxy_server import general_settings verbose_proxy_logger.debug("Handling oauth2 proxy request") + require_trusted_proxy_request( + request=request, + general_settings=general_settings, + feature_name="OAuth2 proxy auth", + ) + oauth2_config_mappings: Dict[str, str] = ( general_settings.get("oauth2_config_mappings") or {} ) diff --git a/litellm/proxy/auth/trusted_proxy_utils.py b/litellm/proxy/auth/trusted_proxy_utils.py new file mode 100644 index 0000000000..df7b3080f2 --- /dev/null +++ b/litellm/proxy/auth/trusted_proxy_utils.py @@ -0,0 +1,118 @@ +import ipaddress +from typing import Any, Dict, List, Optional, Union + +from fastapi import Request + +from litellm._logging import verbose_proxy_logger + +TRUSTED_PROXY_RANGES_KEY = "trusted_proxy_ranges" +TrustedProxyNetwork = Union[ipaddress.IPv4Network, ipaddress.IPv6Network] + + +def _get_proxy_general_settings() -> Dict[str, Any]: + try: + from litellm.proxy.proxy_server import general_settings + + return general_settings or {} + except ImportError: + return {} + + +def _normalize_cidr_ranges(configured_ranges: Any, *, setting_name: str) -> List[str]: + if not configured_ranges: + return [] + if isinstance(configured_ranges, str): + return [ + raw_range.strip() + for raw_range in configured_ranges.split(",") + if raw_range.strip() + ] + if isinstance(configured_ranges, (list, tuple, set)): + return [ + str(raw_range).strip() + for raw_range in configured_ranges + if str(raw_range).strip() + ] + verbose_proxy_logger.warning( + "Invalid %s value: expected a list of CIDR ranges, got %s", + setting_name, + type(configured_ranges).__name__, + ) + return [] + + +def parse_trusted_proxy_ranges( + configured_ranges: Any, + *, + setting_name: str = TRUSTED_PROXY_RANGES_KEY, +) -> List[TrustedProxyNetwork]: + networks: List[TrustedProxyNetwork] = [] + for cidr in _normalize_cidr_ranges(configured_ranges, setting_name=setting_name): + try: + networks.append(ipaddress.ip_network(cidr, strict=False)) + except ValueError: + verbose_proxy_logger.warning( + "Invalid CIDR in %s: %s, skipping", setting_name, cidr + ) + return networks + + +def _get_direct_client_ip(request: Request) -> Optional[str]: + client = getattr(request, "client", None) + client_host = getattr(client, "host", None) + if isinstance(client_host, str): + return client_host + return None + + +def _is_ip_in_networks( + client_ip: Optional[str], networks: List[TrustedProxyNetwork] +) -> bool: + if not client_ip or not networks: + return False + try: + addr = ipaddress.ip_address(client_ip.strip()) + except ValueError: + return False + return any(addr in network for network in networks) + + +def require_trusted_proxy_request( + *, + request: Request, + general_settings: Optional[Dict[str, Any]] = None, + feature_name: str, + setting_name: str = TRUSTED_PROXY_RANGES_KEY, +) -> None: + """ + Fail closed unless the direct TCP peer is one of the configured + trusted reverse proxies. + + Header-based auth paths must validate the direct peer, not + X-Forwarded-For, because the direct peer is the actor supplying the + identity headers. + """ + if general_settings is None: + general_settings = _get_proxy_general_settings() + + trusted_networks = parse_trusted_proxy_ranges( + general_settings.get(setting_name), setting_name=setting_name + ) + if not trusted_networks: + raise ValueError( + f"{feature_name} requires general_settings.{setting_name} before " + "trusting identity headers from an upstream proxy." + ) + + direct_client_ip = _get_direct_client_ip(request) + if not _is_ip_in_networks(direct_client_ip, trusted_networks): + verbose_proxy_logger.warning( + "%s rejected identity headers from untrusted direct client IP %r", + feature_name, + direct_client_ip, + ) + raise ValueError( + f"{feature_name} only accepts identity headers from configured " + f"trusted proxy ranges. Direct client IP {direct_client_ip!r} " + "is not trusted." + ) diff --git a/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py index 9d0bdcf351..dcbfd281e0 100644 --- a/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py +++ b/tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py @@ -3,12 +3,14 @@ Regression tests for the OAuth2-proxy header-forgery fix (GHSA-5c3m-qffq-4r9m). The hook reads HTTP request headers per ``oauth2_config_mappings`` and -constructs a ``UserAPIKeyAuth`` from them. Without the -identity-only allowlist any field could be mapped — including -``user_role``, which Pydantic coerces from the string -``"proxy_admin"`` into ``LitellmUserRoles.PROXY_ADMIN``. An attacker -who reaches the proxy directly (or via a misconfigured reverse -proxy) sets the mapped header and gains full admin privileges. +constructs a ``UserAPIKeyAuth`` from them. The fix has two parts: + +1. Only requests from configured trusted proxy CIDR ranges may provide + identity headers. +2. Only identity fields may be mapped from those headers. Without the + identity-only allowlist any field could be mapped — including + ``user_role``, which Pydantic coerces from the string + ``"proxy_admin"`` into ``LitellmUserRoles.PROXY_ADMIN``. """ import os @@ -27,9 +29,10 @@ from litellm.proxy.auth.oauth2_proxy_hook import ( ) -def _request_with_headers(headers: dict) -> Request: +def _request_with_headers(headers: dict, *, client_host: str = "127.0.0.1") -> Request: scope = { "type": "http", + "client": (client_host, 12345), "headers": [(k.lower().encode(), v.encode()) for k, v in headers.items()], } request = Request(scope=scope) @@ -40,19 +43,24 @@ def _request_with_headers(headers: dict) -> Request: @pytest.fixture def configure_proxy(monkeypatch): """ - Yields a callable that sets ``oauth2_config_mappings`` on the - proxy_server module for the duration of one test. Default mapping - is a single ``user_id -> x-user-id`` (identity-only). + Yields a callable that sets ``oauth2_config_mappings`` and + ``trusted_proxy_ranges`` on the proxy_server module for the duration + of one test. Defaults to a single identity mapping and localhost as + a trusted proxy. """ import litellm.proxy.proxy_server as proxy_server - def _configure(*, mappings=None): + def _configure(*, mappings=None, trusted_proxy_ranges=("127.0.0.1/32",)): if mappings is None: mappings = {"user_id": "x-user-id"} + settings = { + "oauth2_config_mappings": mappings, + "trusted_proxy_ranges": trusted_proxy_ranges, + } monkeypatch.setattr( proxy_server, "general_settings", - {"oauth2_config_mappings": mappings}, + settings, raising=False, ) @@ -70,6 +78,24 @@ async def test_returns_auth_for_simple_user_id_mapping(configure_proxy): assert auth.user_role is None +@pytest.mark.asyncio +async def test_rejects_identity_headers_without_trusted_proxy_ranges(configure_proxy): + configure_proxy(trusted_proxy_ranges=None) + request = _request_with_headers({"x-user-id": "alice"}) + + with pytest.raises(ValueError, match="trusted_proxy_ranges"): + await handle_oauth2_proxy_request(request) + + +@pytest.mark.asyncio +async def test_rejects_identity_headers_from_untrusted_direct_client(configure_proxy): + configure_proxy(trusted_proxy_ranges=["10.0.0.0/24"]) + request = _request_with_headers({"x-user-id": "alice"}, client_host="203.0.113.10") + + with pytest.raises(ValueError, match="not trusted"): + await handle_oauth2_proxy_request(request) + + @pytest.mark.parametrize( "privileged_field", [ diff --git a/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py b/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py index eecfcaa035..92759c56cb 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py +++ b/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py @@ -4,7 +4,6 @@ import os import sys from unittest.mock import AsyncMock, MagicMock, patch -import httpx import pytest from fastapi import HTTPException, Request @@ -25,7 +24,6 @@ from litellm.proxy.management_endpoints.ui_sso import ( SSOAuthenticationHandler, _setup_team_mappings, _sync_user_role_from_jwt_role_map, - determine_role_from_groups, normalize_email, process_sso_jwt_access_token, ) @@ -1849,6 +1847,7 @@ class TestCustomUISSO: "x-forwarded-for": "192.168.1.1", } mock_request.base_url = "https://test.litellm.ai/" + mock_request.client.host = "10.0.0.10" # Mock the custom handler mock_custom_handler = MagicMock(spec=CustomSSOLoginHandler) @@ -1874,36 +1873,73 @@ class TestCustomUISSO: "litellm.proxy.proxy_server.user_custom_ui_sso_sign_in_handler", mock_custom_handler, ): - with patch.object( - SSOAuthenticationHandler, - "get_redirect_response_from_openid", - return_value=mock_redirect_response, - ) as mock_get_redirect: - # Act - result = ( + with patch( + "litellm.proxy.proxy_server.general_settings", + {"trusted_proxy_ranges": ["10.0.0.0/24"]}, + ): + with patch.object( + SSOAuthenticationHandler, + "get_redirect_response_from_openid", + return_value=mock_redirect_response, + ) as mock_get_redirect: + # Act + result = await EnterpriseCustomSSOHandler.handle_custom_ui_sso_sign_in( + request=mock_request + ) + + # Assert + # Verify the custom handler was called with the request + mock_custom_handler.handle_custom_ui_sso_sign_in.assert_called_once_with( + request=mock_request + ) + + # Verify the redirect response was generated with correct OpenID + mock_get_redirect.assert_called_once_with( + result=expected_openid, + request=mock_request, + received_response=None, + generic_client_id=None, + ui_access_mode=None, + ) + + # Verify the result is the redirect response + assert result == mock_redirect_response + assert result.status_code == 303 + + @pytest.mark.asyncio + async def test_handle_custom_ui_sso_sign_in_rejects_untrusted_proxy(self): + """Custom UI SSO rejects spoofed identity headers from direct clients.""" + from enterprise.litellm_enterprise.proxy.auth.custom_sso_handler import ( + EnterpriseCustomSSOHandler, + ) + from litellm.integrations.custom_sso_handler import CustomSSOLoginHandler + + mock_request = MagicMock(spec=Request) + mock_request.headers = { + "x-litellm-user-id": "admin", + "x-litellm-user-email": "admin@example.com", + } + mock_request.base_url = "https://test.litellm.ai/" + mock_request.client.host = "203.0.113.10" + + mock_custom_handler = MagicMock(spec=CustomSSOLoginHandler) + mock_custom_handler.handle_custom_ui_sso_sign_in = AsyncMock() + + with patch("litellm.proxy.proxy_server.premium_user", True): + with patch( + "litellm.proxy.proxy_server.user_custom_ui_sso_sign_in_handler", + mock_custom_handler, + ): + with patch( + "litellm.proxy.proxy_server.general_settings", + {"trusted_proxy_ranges": ["10.0.0.0/24"]}, + ): + with pytest.raises(ValueError, match="not trusted"): await EnterpriseCustomSSOHandler.handle_custom_ui_sso_sign_in( request=mock_request ) - ) - # Assert - # Verify the custom handler was called with the request - mock_custom_handler.handle_custom_ui_sso_sign_in.assert_called_once_with( - request=mock_request - ) - - # Verify the redirect response was generated with correct OpenID - mock_get_redirect.assert_called_once_with( - result=expected_openid, - request=mock_request, - received_response=None, - generic_client_id=None, - ui_access_mode=None, - ) - - # Verify the result is the redirect response - assert result == mock_redirect_response - assert result.status_code == 303 + mock_custom_handler.handle_custom_ui_sso_sign_in.assert_not_called() @pytest.mark.asyncio async def test_custom_ui_sso_handler_execution_with_real_class(self): @@ -1954,6 +1990,7 @@ class TestCustomUISSO: "x-forwarded-for": "10.0.0.1", } mock_request.base_url = "https://custom.litellm.ai/" + mock_request.client.host = "10.0.0.20" # Mock the redirect response method mock_redirect_response = MagicMock() @@ -1964,34 +2001,36 @@ class TestCustomUISSO: "litellm.proxy.proxy_server.user_custom_ui_sso_sign_in_handler", test_handler_instance, ): - with patch.object( - SSOAuthenticationHandler, - "get_redirect_response_from_openid", - return_value=mock_redirect_response, - ) as mock_get_redirect: - # Act - result = ( - await EnterpriseCustomSSOHandler.handle_custom_ui_sso_sign_in( + with patch( + "litellm.proxy.proxy_server.general_settings", + {"trusted_proxy_ranges": ["10.0.0.0/24"]}, + ): + with patch.object( + SSOAuthenticationHandler, + "get_redirect_response_from_openid", + return_value=mock_redirect_response, + ) as mock_get_redirect: + # Act + result = await EnterpriseCustomSSOHandler.handle_custom_ui_sso_sign_in( request=mock_request ) - ) - # Assert that our custom handler was executed - assert test_handler_instance.method_called is True - assert test_handler_instance.received_request == mock_request + # Assert that our custom handler was executed + assert test_handler_instance.method_called is True + assert test_handler_instance.received_request == mock_request - # Verify the redirect response was called with the OpenID from our custom handler - mock_get_redirect.assert_called_once() - call_args = mock_get_redirect.call_args.kwargs + # Verify the redirect response was called with the OpenID from our custom handler + mock_get_redirect.assert_called_once() + call_args = mock_get_redirect.call_args.kwargs - # Verify the OpenID object has the expected values from our custom handler - openid_result = call_args["result"] - assert openid_result.id == "custom_test_user_456" - assert openid_result.email == "custom@example.com" - assert openid_result.first_name == "Custom" - assert openid_result.last_name == "Handler" - assert openid_result.display_name == "Custom Handler Test" - assert openid_result.provider == "custom" + # Verify the OpenID object has the expected values from our custom handler + openid_result = call_args["result"] + assert openid_result.id == "custom_test_user_456" + assert openid_result.email == "custom@example.com" + assert openid_result.first_name == "Custom" + assert openid_result.last_name == "Handler" + assert openid_result.display_name == "Custom Handler Test" + assert openid_result.provider == "custom" # Verify the request and other parameters were passed correctly assert call_args["request"] == mock_request From 00442e653c68d432c74071668499e7f65927b2c5 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 1 May 2026 00:19:13 +0000 Subject: [PATCH 08/25] chore(sso): bind generic SSO state to a session cookie The Generic SSO PKCE flow used the URL ``state`` parameter as the cache key for the PKCE ``code_verifier`` without binding the state to the caller's browser. An attacker who pre-minted a state and cached a verifier under it could hand the resulting login link to a victim; the victim's auth code would then be exchanged with the attacker's verifier on the callback, producing an access token under the attacker's control (Login CSRF / token theft). The non-PKCE branch is unaffected because it delegates to fastapi-sso's ``verify_and_process``, which performs its own session-cookie check. The PKCE branch bypasses that helper, which is exactly the gap this commit closes. Two-part fix in ``ui_sso.py``: - ``get_generic_sso_redirect_response`` now sets a ``litellm_oauth_state`` cookie (HttpOnly, SameSite=Lax, 10-min TTL) carrying the state value used in the redirect URL. The cookie is set on the redirect response just like the existing ``litellm_cp_return_to`` cookie a few lines earlier in the file. - ``get_generic_sso_response`` validates ``request.cookies.get( "litellm_oauth_state")`` against ``request.query_params.get( "state")`` via ``secrets.compare_digest`` before invoking the PKCE token exchange. Mismatch (or either being missing) raises a ``ProxyException`` with HTTP 400. The pre-existing TODO above the redirect logic ("state should be a random string and added to the user session with cookie") is now addressed and removed. Tests cover the redirect-side cookie set, the missing-cookie reject shape, the URL/cookie-mismatch reject shape, and the matching-cookie happy path. --- litellm/proxy/management_endpoints/ui_sso.py | 49 +++- .../proxy/management_endpoints/test_ui_sso.py | 227 ++++++++++++++++++ 2 files changed, 272 insertions(+), 4 deletions(-) diff --git a/litellm/proxy/management_endpoints/ui_sso.py b/litellm/proxy/management_endpoints/ui_sso.py index c4564a4eb0..ca9fee1a76 100644 --- a/litellm/proxy/management_endpoints/ui_sso.py +++ b/litellm/proxy/management_endpoints/ui_sso.py @@ -1155,6 +1155,30 @@ async def get_generic_sso_response( authorization_code = request.query_params.get("code") if code_verifier: + # State-to-session-cookie binding. The non-PKCE branch below + # delegates to fastapi-sso's ``verify_and_process``, which + # performs its own session-cookie check. The PKCE branch + # bypasses that helper, so we validate the URL ``state`` + # against the ``litellm_oauth_state`` cookie set on the + # redirect response — without this an attacker can pre-mint + # a state + cached PKCE verifier and hijack a victim's auth + # code (Login-CSRF / token theft). + url_state = request.query_params.get("state") + cookie_state = request.cookies.get("litellm_oauth_state") + if ( + not url_state + or not cookie_state + or not secrets.compare_digest(url_state, cookie_state) + ): + raise ProxyException( + message=( + "Invalid OAuth state parameter — does not match " + "the browser-bound state cookie." + ), + type=ProxyErrorTypes.auth_error, + param="state", + code=status.HTTP_400_BAD_REQUEST, + ) if not authorization_code: raise ProxyException( message="Missing authorization code in callback", @@ -2284,10 +2308,13 @@ class SSOAuthenticationHandler: from litellm.proxy.proxy_server import redis_usage_cache, user_api_key_cache with generic_sso: - # TODO: state should be a random string and added to the user session with cookie - # or a cryptographicly signed state that we can verify stateless - # For simplification we are using a static state, this is not perfect but some - # SSO providers do not allow stateless verification + # State is bound to the caller's browser via a ``litellm_oauth_state`` + # HttpOnly cookie set on the redirect response below; the SSO + # callback validates the URL ``state`` against that cookie before + # completing the PKCE token exchange. Without this binding, an + # attacker who pre-mints a state + a cached PKCE verifier can hand + # the link to a victim and capture the resulting access token + # (Login CSRF / token theft). ( redirect_params, code_verifier, @@ -2354,6 +2381,20 @@ class SSOAuthenticationHandler: # Update the redirect response redirect_response.headers["location"] = new_url + + # Bind state to the user's browser session. The /callback handler + # validates the URL ``state`` against this cookie via + # ``secrets.compare_digest`` before exchanging the PKCE + # code_verifier. + state_value = redirect_params.get("state") + if state_value and redirect_response is not None: + redirect_response.set_cookie( + key="litellm_oauth_state", + value=state_value, + max_age=600, + httponly=True, + samesite="lax", + ) return redirect_response @staticmethod diff --git a/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py b/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py index a0ae95df58..019d621874 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py +++ b/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py @@ -5767,3 +5767,230 @@ class TestSyncUserRoleFromJwtRoleMap: ) prisma.db.litellm_usertable.update.assert_not_called() + + +# ── VERIA-34 regression: PKCE state-to-session-cookie binding ─────────────── + + +class TestPKCEStateCookieBinding: + """The Generic SSO PKCE flow used the URL ``state`` parameter as a + cache-key for the PKCE ``code_verifier`` without binding the state to + the caller's browser. An attacker who pre-mints a state + cached + verifier could hand the link to a victim and capture the resulting + access token. Fix: set ``litellm_oauth_state`` HttpOnly cookie on + the redirect; verify the URL state matches the cookie before doing + the PKCE token exchange.""" + + @pytest.mark.asyncio + async def test_redirect_response_sets_oauth_state_cookie(self): + """``get_generic_sso_redirect_response`` must set + ``litellm_oauth_state`` on the redirect response so the callback + can verify it later.""" + from fastapi.responses import RedirectResponse + + from litellm.proxy.management_endpoints.ui_sso import ( + SSOAuthenticationHandler, + ) + + # generic_sso is a context manager + redirect-response factory. + mock_redirect = RedirectResponse( + url="https://idp.example.com/authorize?state=test-state-xyz" + ) + mock_generic_sso = MagicMock() + mock_generic_sso.__enter__ = MagicMock(return_value=mock_generic_sso) + mock_generic_sso.__exit__ = MagicMock(return_value=None) + mock_generic_sso.get_login_redirect = AsyncMock(return_value=mock_redirect) + + with patch.dict(os.environ, {"GENERIC_CLIENT_STATE": "test-state-xyz"}): + response = await SSOAuthenticationHandler.get_generic_sso_redirect_response( + generic_sso=mock_generic_sso, + state=None, + generic_authorization_endpoint="https://idp.example.com/authorize", + ) + + assert response is not None + cookie_headers = response.headers.getlist("set-cookie") + cookie_str = next( + (c for c in cookie_headers if "litellm_oauth_state=" in c), None + ) + assert ( + cookie_str is not None + ), f"litellm_oauth_state cookie not set; got: {cookie_headers}" + assert "test-state-xyz" in cookie_str + assert "HttpOnly" in cookie_str + assert "SameSite=lax" in cookie_str + + @pytest.mark.asyncio + async def test_pkce_callback_rejects_missing_cookie(self): + """When PKCE is enabled and a code_verifier is in the cache, the + callback must reject a request that has no ``litellm_oauth_state`` + cookie (browser-to-server binding missing).""" + from litellm.proxy._types import ProxyException + from litellm.proxy.management_endpoints.ui_sso import ( + SSOAuthenticationHandler, + get_generic_sso_response, + ) + + mock_request = MagicMock(spec=Request) + mock_request.query_params = { + "state": "attacker-minted-state", + "code": "auth-code", + } + # No oauth_state cookie set → request.cookies.get returns None. + mock_request.cookies = {} + + with ( + patch.object( + SSOAuthenticationHandler, + "prepare_token_exchange_parameters", + AsyncMock( + return_value={ + "code_verifier": "attacker-cached-verifier", + "_pkce_cache_key": "pkce_verifier:attacker-minted-state", + } + ), + ), + patch("fastapi_sso.sso.base.DiscoveryDocument"), + patch("fastapi_sso.sso.generic.create_provider", return_value=MagicMock()), + patch.dict( + os.environ, + { + "GENERIC_CLIENT_SECRET": "x", + "GENERIC_AUTHORIZATION_ENDPOINT": "https://idp.example.com/auth", + "GENERIC_TOKEN_ENDPOINT": "https://idp.example.com/token", + "GENERIC_USERINFO_ENDPOINT": "https://idp.example.com/userinfo", + "GENERIC_CLIENT_USE_PKCE": "true", + }, + ), + pytest.raises(ProxyException) as exc_info, + ): + await get_generic_sso_response( + request=mock_request, + jwt_handler=MagicMock(spec=JWTHandler), + generic_client_id="cid", + redirect_url="https://proxy.example.com/sso/callback", + sso_jwt_handler=None, + ) + + assert "state" in str(exc_info.value.message).lower() + + @pytest.mark.asyncio + async def test_pkce_callback_rejects_state_cookie_mismatch(self): + """The Login-CSRF shape: attacker mints state ``A``, victim's browser + carries cookie state ``B``. The callback must reject.""" + from litellm.proxy._types import ProxyException + from litellm.proxy.management_endpoints.ui_sso import ( + SSOAuthenticationHandler, + get_generic_sso_response, + ) + + mock_request = MagicMock(spec=Request) + mock_request.query_params = { + "state": "attacker-minted-state", + "code": "auth-code", + } + mock_request.cookies = {"litellm_oauth_state": "victim-browser-state"} + + with ( + patch.object( + SSOAuthenticationHandler, + "prepare_token_exchange_parameters", + AsyncMock( + return_value={ + "code_verifier": "verifier", + "_pkce_cache_key": "pkce_verifier:attacker-minted-state", + } + ), + ), + patch("fastapi_sso.sso.base.DiscoveryDocument"), + patch("fastapi_sso.sso.generic.create_provider", return_value=MagicMock()), + patch.dict( + os.environ, + { + "GENERIC_CLIENT_SECRET": "x", + "GENERIC_AUTHORIZATION_ENDPOINT": "https://idp.example.com/auth", + "GENERIC_TOKEN_ENDPOINT": "https://idp.example.com/token", + "GENERIC_USERINFO_ENDPOINT": "https://idp.example.com/userinfo", + "GENERIC_CLIENT_USE_PKCE": "true", + }, + ), + pytest.raises(ProxyException) as exc_info, + ): + await get_generic_sso_response( + request=mock_request, + jwt_handler=MagicMock(spec=JWTHandler), + generic_client_id="cid", + redirect_url="https://proxy.example.com/sso/callback", + sso_jwt_handler=None, + ) + + assert "state" in str(exc_info.value.message).lower() + + @pytest.mark.asyncio + async def test_pkce_callback_accepts_matching_state_cookie(self): + """Happy path: URL state and cookie state match (the legitimate + flow where the same browser that started the redirect lands on + the callback) → the PKCE token exchange proceeds.""" + from litellm.proxy.management_endpoints.ui_sso import ( + SSOAuthenticationHandler, + get_generic_sso_response, + ) + + mock_request = MagicMock(spec=Request) + mock_request.query_params = {"state": "matched-state", "code": "auth-code"} + mock_request.cookies = {"litellm_oauth_state": "matched-state"} + + with ( + patch.object( + SSOAuthenticationHandler, + "prepare_token_exchange_parameters", + AsyncMock( + return_value={ + "code_verifier": "verifier", + "_pkce_cache_key": "pkce_verifier:matched-state", + } + ), + ), + patch.object( + SSOAuthenticationHandler, + "_pkce_token_exchange", + AsyncMock( + return_value={ + "access_token": "tok", + "id_token": "id", + "sub": "user@example.com", + "email": "user@example.com", + } + ), + ), + patch.object( + SSOAuthenticationHandler, + "_delete_pkce_verifier", + AsyncMock(), + ), + patch("fastapi_sso.sso.base.DiscoveryDocument"), + patch("fastapi_sso.sso.generic.create_provider", return_value=MagicMock()), + patch.dict( + os.environ, + { + "GENERIC_CLIENT_SECRET": "x", + "GENERIC_AUTHORIZATION_ENDPOINT": "https://idp.example.com/auth", + "GENERIC_TOKEN_ENDPOINT": "https://idp.example.com/token", + "GENERIC_USERINFO_ENDPOINT": "https://idp.example.com/userinfo", + "GENERIC_CLIENT_USE_PKCE": "true", + }, + ), + ): + jwt_handler = MagicMock(spec=JWTHandler) + jwt_handler.get_team_ids_from_jwt.return_value = [] + result, _, _ = await get_generic_sso_response( + request=mock_request, + jwt_handler=jwt_handler, + generic_client_id="cid", + redirect_url="https://proxy.example.com/sso/callback", + sso_jwt_handler=None, + ) + + # State-cookie check passed, so the function got past the early + # ProxyException raise and produced an SSO result object. + assert result is not None From 2c852ba2b1a6729ad36e922898b670b353d111a1 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 1 May 2026 00:36:51 +0000 Subject: [PATCH 09/25] =?UTF-8?q?fix(sso):=20tighten=20oauth=5Fstate=20coo?= =?UTF-8?q?kie=20=E2=80=94=20Secure=20flag=20+=20PKCE-only=20set?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Greptile review findings addressed: 1. (P1, security) The ``litellm_oauth_state`` cookie is the sole guard against Login-CSRF in the PKCE flow but was set without the ``Secure`` attribute, so a network observer on plain HTTP could read and replay it — bypassing the protection this PR adds. Thread the originating ``Request`` down through ``get_sso_login_redirect`` and ``get_generic_sso_redirect_response`` and set ``Secure`` based on ``request.url.scheme == "https"``. When no request is supplied (programmatic callers / tests) default to ``Secure=True`` — production-safe. Local HTTP dev still works because the request scheme is observed at runtime. 2. (P2) The cookie was set unconditionally, but the callback only validates it inside the PKCE branch. Two concurrent SSO sessions (one PKCE, one plain) could overwrite each other's state cookie and produce spurious 400s for the plain-flow user. Move the ``set_cookie`` call inside the existing ``if code_verifier and "state" in redirect_params`` block so the cookie is only written when PKCE is active and the validation will actually fire. Tests cover both paths: PKCE-on (cookie set with Secure default), PKCE-off (cookie not set), and HTTP dev request (Secure dropped so the browser will actually attach the cookie on the callback hop). --- litellm/proxy/management_endpoints/ui_sso.py | 44 +++++--- .../proxy/management_endpoints/test_ui_sso.py | 104 +++++++++++++++++- 2 files changed, 130 insertions(+), 18 deletions(-) diff --git a/litellm/proxy/management_endpoints/ui_sso.py b/litellm/proxy/management_endpoints/ui_sso.py index ca9fee1a76..8225b97320 100644 --- a/litellm/proxy/management_endpoints/ui_sso.py +++ b/litellm/proxy/management_endpoints/ui_sso.py @@ -674,6 +674,7 @@ async def google_login( google_client_id=google_client_id, generic_client_id=generic_client_id, state=cli_state, + request=request, ) if return_to is not None and sso_redirect is not None: if SSOAuthenticationHandler._validate_return_to(return_to): @@ -2170,6 +2171,7 @@ class SSOAuthenticationHandler: microsoft_client_id: Optional[str] = None, generic_client_id: Optional[str] = None, state: Optional[str] = None, + request: Optional[Request] = None, ) -> Optional[RedirectResponse]: """ Step 1. Call Get Login Redirect for the SSO provider. Send the redirect response to `redirect_url` @@ -2179,6 +2181,8 @@ class SSOAuthenticationHandler: google_client_id (Optional[str], optional): The Google Client ID. Defaults to None. microsoft_client_id (Optional[str], optional): The Microsoft Client ID. Defaults to None. generic_client_id (Optional[str], optional): The Generic Client ID. Defaults to None. + request: Optional FastAPI request, used to drive the ``Secure`` + attribute on the ``litellm_oauth_state`` CSRF cookie. Returns: RedirectResponse: The redirect response from the SSO provider. @@ -2289,6 +2293,7 @@ class SSOAuthenticationHandler: generic_sso=generic_sso, state=state, generic_authorization_endpoint=generic_authorization_endpoint, + request=request, ) raise ValueError( "Unknown SSO provider. Please setup SSO with client IDs https://docs.litellm.ai/docs/proxy/admin_ui_sso" @@ -2299,6 +2304,7 @@ class SSOAuthenticationHandler: generic_sso: Any, state: Optional[str] = None, generic_authorization_endpoint: Optional[str] = None, + request: Optional[Request] = None, ) -> Optional[RedirectResponse]: """ Get the redirect response for Generic SSO @@ -2382,19 +2388,30 @@ class SSOAuthenticationHandler: # Update the redirect response redirect_response.headers["location"] = new_url - # Bind state to the user's browser session. The /callback handler - # validates the URL ``state`` against this cookie via - # ``secrets.compare_digest`` before exchanging the PKCE - # code_verifier. - state_value = redirect_params.get("state") - if state_value and redirect_response is not None: - redirect_response.set_cookie( - key="litellm_oauth_state", - value=state_value, - max_age=600, - httponly=True, - samesite="lax", - ) + # Bind state to the user's browser session. The /callback + # handler validates the URL ``state`` against this cookie via + # ``secrets.compare_digest`` before exchanging the PKCE + # code_verifier. Only set the cookie when PKCE is in use + # (i.e. inside this ``code_verifier`` branch) so two + # concurrent SSO sessions — one PKCE, one plain — cannot + # overwrite each other's state cookie. + state_value = redirect_params.get("state") + if state_value and redirect_response is not None: + # Production-safe default: require HTTPS for the + # CSRF-protection cookie unless we can prove the + # incoming request is HTTP (local dev). Without + # ``Secure`` the cookie is sent over plain HTTP, + # letting a network observer read and replay the + # state value and bypass this protection. + secure_flag = request is None or request.url.scheme == "https" + redirect_response.set_cookie( + key="litellm_oauth_state", + value=state_value, + max_age=600, + httponly=True, + samesite="lax", + secure=secure_flag, + ) return redirect_response @staticmethod @@ -4012,6 +4029,7 @@ async def debug_sso_login(request: Request): microsoft_client_id=microsoft_client_id, google_client_id=google_client_id, generic_client_id=generic_client_id, + request=request, ) diff --git a/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py b/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py index 019d621874..231fca36cf 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py +++ b/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py @@ -5782,17 +5782,18 @@ class TestPKCEStateCookieBinding: the PKCE token exchange.""" @pytest.mark.asyncio - async def test_redirect_response_sets_oauth_state_cookie(self): + async def test_redirect_response_sets_oauth_state_cookie_when_pkce_enabled(self): """``get_generic_sso_redirect_response`` must set - ``litellm_oauth_state`` on the redirect response so the callback - can verify it later.""" + ``litellm_oauth_state`` on the redirect response when PKCE is on so + the callback can verify it later. The cookie must carry HttpOnly, + SameSite=Lax, and (because no http request was supplied to the + helper) the production-safe ``Secure`` default.""" from fastapi.responses import RedirectResponse from litellm.proxy.management_endpoints.ui_sso import ( SSOAuthenticationHandler, ) - # generic_sso is a context manager + redirect-response factory. mock_redirect = RedirectResponse( url="https://idp.example.com/authorize?state=test-state-xyz" ) @@ -5801,7 +5802,13 @@ class TestPKCEStateCookieBinding: mock_generic_sso.__exit__ = MagicMock(return_value=None) mock_generic_sso.get_login_redirect = AsyncMock(return_value=mock_redirect) - with patch.dict(os.environ, {"GENERIC_CLIENT_STATE": "test-state-xyz"}): + with patch.dict( + os.environ, + { + "GENERIC_CLIENT_STATE": "test-state-xyz", + "GENERIC_CLIENT_USE_PKCE": "true", + }, + ): response = await SSOAuthenticationHandler.get_generic_sso_redirect_response( generic_sso=mock_generic_sso, state=None, @@ -5819,6 +5826,93 @@ class TestPKCEStateCookieBinding: assert "test-state-xyz" in cookie_str assert "HttpOnly" in cookie_str assert "SameSite=lax" in cookie_str + # No incoming Request supplied → ``Secure`` defaults to True so a + # network observer on plain HTTP cannot read the state value. + assert "Secure" in cookie_str + + @pytest.mark.asyncio + async def test_redirect_response_omits_oauth_state_cookie_when_pkce_disabled( + self, + ): + """Non-PKCE flows delegate to fastapi-sso's own session-cookie + binding; we do not set our cookie there because it would never be + validated (and could collide with a concurrent PKCE session in + the same browser).""" + from fastapi.responses import RedirectResponse + + from litellm.proxy.management_endpoints.ui_sso import ( + SSOAuthenticationHandler, + ) + + mock_redirect = RedirectResponse( + url="https://idp.example.com/authorize?state=test-state-xyz" + ) + mock_generic_sso = MagicMock() + mock_generic_sso.__enter__ = MagicMock(return_value=mock_generic_sso) + mock_generic_sso.__exit__ = MagicMock(return_value=None) + mock_generic_sso.get_login_redirect = AsyncMock(return_value=mock_redirect) + + with patch.dict( + os.environ, + { + "GENERIC_CLIENT_STATE": "test-state-xyz", + "GENERIC_CLIENT_USE_PKCE": "false", + }, + ): + response = await SSOAuthenticationHandler.get_generic_sso_redirect_response( + generic_sso=mock_generic_sso, + state=None, + generic_authorization_endpoint="https://idp.example.com/authorize", + ) + + assert response is not None + cookie_headers = response.headers.getlist("set-cookie") + assert not any( + "litellm_oauth_state=" in c for c in cookie_headers + ), f"litellm_oauth_state cookie set on non-PKCE flow; got: {cookie_headers}" + + @pytest.mark.asyncio + async def test_redirect_response_drops_secure_flag_for_http_dev(self): + """When the incoming request is plain HTTP (local dev), ``Secure`` + must be dropped so the browser will actually attach the cookie on + the callback hop.""" + from fastapi.responses import RedirectResponse + + from litellm.proxy.management_endpoints.ui_sso import ( + SSOAuthenticationHandler, + ) + + mock_redirect = RedirectResponse( + url="http://idp.local/authorize?state=local-dev-state" + ) + mock_generic_sso = MagicMock() + mock_generic_sso.__enter__ = MagicMock(return_value=mock_generic_sso) + mock_generic_sso.__exit__ = MagicMock(return_value=None) + mock_generic_sso.get_login_redirect = AsyncMock(return_value=mock_redirect) + + http_request = MagicMock(spec=Request) + http_request.url.scheme = "http" + + with patch.dict( + os.environ, + { + "GENERIC_CLIENT_STATE": "local-dev-state", + "GENERIC_CLIENT_USE_PKCE": "true", + }, + ): + response = await SSOAuthenticationHandler.get_generic_sso_redirect_response( + generic_sso=mock_generic_sso, + state=None, + generic_authorization_endpoint="http://idp.local/authorize", + request=http_request, + ) + + cookie_headers = response.headers.getlist("set-cookie") + cookie_str = next( + (c for c in cookie_headers if "litellm_oauth_state=" in c), None + ) + assert cookie_str is not None + assert "Secure" not in cookie_str @pytest.mark.asyncio async def test_pkce_callback_rejects_missing_cookie(self): From cc917993a918dab1d7895ad95d38a0cee34a8d0e Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Thu, 30 Apr 2026 23:51:04 -0700 Subject: [PATCH 10/25] [Fix] Release Workflow: Detect SemVer-Style Pre-Release Dev Tags The pre-release detector in create-release.yml uses `\.dev` (literal dot before `dev`), which matches PEP 440 canonical tags like `1.84.0.dev2` but misses the SemVer/Docker form `1.84.0-dev.2` (hyphen-dev). Per the release design doc's PyPI<->Docker mapping rule, both forms are valid production-track release tags and both are pre-releases (opt-in via `pip install --pre litellm`), so the workflow should mark them as GitHub pre-releases either way. Change the regex to `[-.]dev` so it accepts `.dev` and `-dev`. --- .github/workflows/create-release.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index 39d078267f..a726a921a2 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -4,7 +4,7 @@ on: workflow_dispatch: inputs: tag: - description: "Release tag (e.g. 1.84.0, 1.84.0rc1, 1.84.0.dev42, 1.84.0.post1; legacy v1.83.10-stable still accepted)" + description: "Release tag (e.g. 1.84.0, 1.84.0rc1, 1.84.0.dev42, 1.84.0-dev.2, 1.84.0.post1; legacy v1.83.10-stable still accepted)" required: true type: string commit_hash: @@ -46,9 +46,11 @@ jobs: const commitHash = process.env.COMMIT_HASH; // Mark RC / dev / nightly / alpha / beta tags as GitHub pre-releases. + // Accept both PEP 440 (`.dev`) and SemVer (`-dev`) separators so tags + // like `1.84.0.dev2` and `1.84.0-dev.2` are both detected. // PEP 440 post-releases (e.g. `1.84.0.post1`) and legacy `-stable[.patch.N]` // are stable maintenance releases, not pre-releases. - const isPrerelease = /(?:rc|nightly|alpha|beta|\.dev)/i.test(tag); + const isPrerelease = /(?:rc|nightly|alpha|beta|[-.]dev)/i.test(tag); const cosignSection = [ `## Verify Docker Image Signature`, From a5b7eeebdc180b632d9bbe91f9d9e5bfeecec697 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 1 May 2026 07:49:32 +0000 Subject: [PATCH 11/25] chore(proxy): close router-settings-override fallback smuggling path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes that together prevent a caller from smuggling unauthorized models past the API key's allowlist via per-request router overrides. 1. ``_enforce_key_and_fallback_model_access``: also walk fallback models nested inside ``router_settings_override.fallbacks`` / ``context_window_fallbacks`` / ``content_policy_fallbacks``. ``route_llm_request.py`` promotes those to per-request kwargs after auth, so without this they bypassed the model allowlist entirely. New ``iter_router_fallback_model_names`` helper extracts leaf names from both the simple top-level shape (str | {"model": str}) and the nested router-config shape ({primary: [fallbacks]}). The two fallback validation loops are unified — every name (top-level + override) is deduplicated and validated once via ``can_key_call_model`` + ``is_valid_fallback_model``. 2. ``route_request``: strip router-internal ``mock_testing_*`` flags from user-supplied data. These are testing-only flags that deterministically force the router into fallback logic by raising a synthetic ``InternalServerError`` etc. Combined with override fallbacks they made the smuggling path trivially exploitable. Test code that calls the router directly bypasses the strip and is unaffected. The strip list is derived from ``MockRouterTestingParams`` so a new ``mock_testing_*`` flag added to that dataclass is automatically covered. Co-Authored-By: Claude Opus 4.7 (1M context) --- litellm/proxy/auth/user_api_key_auth.py | 75 +++++-- litellm/proxy/route_llm_request.py | 15 ++ .../test_router_override_fallback_auth.py | 187 ++++++++++++++++++ .../proxy/test_route_llm_request.py | 32 +++ 4 files changed, 292 insertions(+), 17 deletions(-) create mode 100644 tests/test_litellm/proxy/auth/test_router_override_fallback_auth.py diff --git a/litellm/proxy/auth/user_api_key_auth.py b/litellm/proxy/auth/user_api_key_auth.py index b8db3cd2a7..e5094db0ec 100644 --- a/litellm/proxy/auth/user_api_key_auth.py +++ b/litellm/proxy/auth/user_api_key_auth.py @@ -11,7 +11,7 @@ import asyncio import re import secrets from datetime import datetime, timezone -from typing import Any, List, Optional, Tuple, cast +from typing import Any, Iterator, List, Optional, Tuple, cast import fastapi from fastapi import HTTPException, Request, WebSocket, status @@ -2131,10 +2131,6 @@ async def _enforce_key_and_fallback_model_access( pass else: model = get_model_from_request(request_data, route) - fallback_models = cast( - Optional[List[ALL_FALLBACK_MODEL_VALUES]], - request_data.get("fallbacks", None), - ) if model is not None: await can_key_call_model( @@ -2144,20 +2140,65 @@ async def _enforce_key_and_fallback_model_access( llm_router=llm_router, ) - if fallback_models is not None: - for m in fallback_models: - await can_key_call_model( - model=m["model"] if isinstance(m, dict) else m, - llm_model_list=llm_model_list, - valid_token=valid_token, - llm_router=llm_router, - ) - await is_valid_fallback_model( - model=m["model"] if isinstance(m, dict) else m, - llm_router=llm_router, - user_model=None, + # Validate every fallback model name reachable by this request: + # top-level ``fallbacks`` AND fallbacks nested inside + # ``router_settings_override`` (which ``route_llm_request.py`` + # promotes to per-request kwargs *after* this check). VERIA-44. + fallback_names: List[str] = list( + iter_router_fallback_model_names(request_data.get("fallbacks")) + ) + override_settings = request_data.get("router_settings_override") + if isinstance(override_settings, dict): + for _fb_key in ROUTER_FALLBACK_FIELDS: + fallback_names.extend( + iter_router_fallback_model_names(override_settings.get(_fb_key)) ) + for _name in dict.fromkeys(fallback_names): # dedupe, preserve order + await can_key_call_model( + model=_name, + llm_model_list=llm_model_list, + valid_token=valid_token, + llm_router=llm_router, + ) + await is_valid_fallback_model( + model=_name, + llm_router=llm_router, + user_model=None, + ) + + +ROUTER_FALLBACK_FIELDS: Tuple[str, ...] = ( + "fallbacks", + "context_window_fallbacks", + "content_policy_fallbacks", +) + + +def iter_router_fallback_model_names(fallbacks: Any) -> Iterator[str]: + """Yield leaf model names from any of the supported fallbacks shapes. + + Handles the simple top-level shape (``str`` or ``{"model": str}``) and + the nested router-config shape (``[{primary: [fallback_list]}]``). + """ + if not isinstance(fallbacks, list): + return + for entry in fallbacks: + if isinstance(entry, str): + yield entry + elif isinstance(entry, dict): + if isinstance(entry.get("model"), str): + yield entry["model"] + continue + for fallback_list in entry.values(): + if not isinstance(fallback_list, list): + continue + for m in fallback_list: + if isinstance(m, str): + yield m + elif isinstance(m, dict) and isinstance(m.get("model"), str): + yield m["model"] + async def _run_post_custom_auth_checks( valid_token: UserAPIKeyAuth, diff --git a/litellm/proxy/route_llm_request.py b/litellm/proxy/route_llm_request.py index 17cc437456..f611384b7a 100644 --- a/litellm/proxy/route_llm_request.py +++ b/litellm/proxy/route_llm_request.py @@ -1,10 +1,18 @@ import asyncio +from dataclasses import fields as _dc_fields from typing import TYPE_CHECKING, Any, Literal, Optional from fastapi import HTTPException, status import litellm from litellm.proxy._types import UserAPIKeyAuth +from litellm.types.router import MockRouterTestingParams + +# Router-internal mock_testing_* flag names. Single source of truth so a +# new flag added to ``MockRouterTestingParams`` is automatically stripped. +_MOCK_TESTING_KWARG_NAMES: tuple = tuple( + f.name for f in _dc_fields(MockRouterTestingParams) +) if TYPE_CHECKING: from litellm.router import Router as _Router @@ -322,6 +330,13 @@ async def route_request( # noqa: PLR0915 - Complex routing function, refactorin """ await add_shared_session_to_data(data) + # Strip router-internal mock_testing_* flags. Combined with an + # unauthorized fallback in ``router_settings_override`` they let a + # caller deterministically execute requests against restricted + # models. VERIA-44. + for _key in _MOCK_TESTING_KWARG_NAMES: + data.pop(_key, None) + team_id = get_team_id_from_data(data) router_model_names = llm_router.model_names if llm_router is not None else [] diff --git a/tests/test_litellm/proxy/auth/test_router_override_fallback_auth.py b/tests/test_litellm/proxy/auth/test_router_override_fallback_auth.py new file mode 100644 index 0000000000..34121d9a50 --- /dev/null +++ b/tests/test_litellm/proxy/auth/test_router_override_fallback_auth.py @@ -0,0 +1,187 @@ +""" +VERIA-44: ``router_settings_override.fallbacks`` must be validated +against the API key's model allowlist at auth time. Without this, the +override is promoted to per-request kwargs after auth and lets a caller +execute requests against models their API key cannot call. +""" + +from typing import List +from unittest.mock import AsyncMock, patch + +import pytest + +from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth +from litellm.proxy.auth.user_api_key_auth import ( + _enforce_key_and_fallback_model_access, + iter_router_fallback_model_names, +) + + +def _key_with_models(models: List[str]) -> UserAPIKeyAuth: + return UserAPIKeyAuth( + api_key="hashed", + user_id="caller", + user_role=LitellmUserRoles.INTERNAL_USER, + models=models, + ) + + +# ── iter_router_fallback_model_names ───────────────────────────────────────── + + +def testiter_router_fallback_model_names_router_config_shape(): + """Router-config shape: ``[{primary: [fallback_list]}]``.""" + assert list( + iter_router_fallback_model_names( + [{"gpt-3.5-turbo": ["gpt-4", "claude-3"]}, {"gpt-4o": ["o1"]}] + ) + ) == ["gpt-4", "claude-3", "o1"] + + +def testiter_router_fallback_model_names_simple_string_shape(): + """Simple top-level shape: list of strings.""" + assert list(iter_router_fallback_model_names(["gpt-4", "claude-3"])) == [ + "gpt-4", + "claude-3", + ] + + +def testiter_router_fallback_model_names_client_side_shape(): + """ClientSideFallbackModel shape: ``[{"model": "..."}]``.""" + assert list( + iter_router_fallback_model_names([{"model": "gpt-4"}, {"model": "claude-3"}]) + ) == ["gpt-4", "claude-3"] + + +def testiter_router_fallback_model_names_empty_or_none(): + assert list(iter_router_fallback_model_names(None)) == [] + assert list(iter_router_fallback_model_names([])) == [] + assert list(iter_router_fallback_model_names("not a list")) == [] + + +# ── _enforce_key_and_fallback_model_access ──────────────────────────────────── + + +@pytest.mark.asyncio +async def test_router_override_fallbacks_validated_against_key_allowlist(): + """A fallback nested inside ``router_settings_override`` is validated + against the API key's allowed models — not just the top-level + ``fallbacks`` field.""" + valid_token = _key_with_models(["gpt-3.5-turbo"]) + request_data = { + "model": "gpt-3.5-turbo", + "router_settings_override": { + "fallbacks": [{"gpt-3.5-turbo": ["unauthorized-model"]}], + }, + } + + seen_models: List[str] = [] + + async def fake_can_key_call_model(model, llm_model_list, valid_token, llm_router): + seen_models.append(model) + + with ( + patch( + "litellm.proxy.auth.user_api_key_auth.can_key_call_model", + side_effect=fake_can_key_call_model, + ), + patch( + "litellm.proxy.auth.user_api_key_auth.is_valid_fallback_model", + new=AsyncMock(), + ), + ): + await _enforce_key_and_fallback_model_access( + valid_token=valid_token, + request_data=request_data, + route="/v1/chat/completions", + llm_model_list=None, + llm_router=None, + ) + + # Both the primary model and the override-nested fallback must be + # checked against the API key's allowlist. + assert "gpt-3.5-turbo" in seen_models + assert "unauthorized-model" in seen_models + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "fallback_field", + [ + "fallbacks", + "context_window_fallbacks", + "content_policy_fallbacks", + ], +) +async def test_router_override_all_fallback_fields_validated(fallback_field): + """All three fallback fields the router accepts as per-request kwargs + are validated — context_window_fallbacks and content_policy_fallbacks + are promoted in route_llm_request.py too.""" + valid_token = _key_with_models(["gpt-3.5-turbo"]) + request_data = { + "model": "gpt-3.5-turbo", + "router_settings_override": { + fallback_field: [{"gpt-3.5-turbo": ["smuggled-model"]}], + }, + } + + seen: List[str] = [] + + async def fake_can_key_call_model(model, llm_model_list, valid_token, llm_router): + seen.append(model) + + with ( + patch( + "litellm.proxy.auth.user_api_key_auth.can_key_call_model", + side_effect=fake_can_key_call_model, + ), + patch( + "litellm.proxy.auth.user_api_key_auth.is_valid_fallback_model", + new=AsyncMock(), + ), + ): + await _enforce_key_and_fallback_model_access( + valid_token=valid_token, + request_data=request_data, + route="/v1/chat/completions", + llm_model_list=None, + llm_router=None, + ) + + assert "smuggled-model" in seen + + +@pytest.mark.asyncio +async def test_router_override_without_fallbacks_does_not_break_auth(): + """``router_settings_override`` set without any fallback fields is a + no-op for the auth check — only the primary model is validated.""" + valid_token = _key_with_models(["gpt-3.5-turbo"]) + request_data = { + "model": "gpt-3.5-turbo", + "router_settings_override": {"num_retries": 3, "timeout": 30}, + } + + seen: List[str] = [] + + async def fake_can_key_call_model(model, llm_model_list, valid_token, llm_router): + seen.append(model) + + with ( + patch( + "litellm.proxy.auth.user_api_key_auth.can_key_call_model", + side_effect=fake_can_key_call_model, + ), + patch( + "litellm.proxy.auth.user_api_key_auth.is_valid_fallback_model", + new=AsyncMock(), + ), + ): + await _enforce_key_and_fallback_model_access( + valid_token=valid_token, + request_data=request_data, + route="/v1/chat/completions", + llm_model_list=None, + llm_router=None, + ) + + assert seen == ["gpt-3.5-turbo"] diff --git a/tests/test_litellm/proxy/test_route_llm_request.py b/tests/test_litellm/proxy/test_route_llm_request.py index 96870b6cc7..9654c8d9c0 100644 --- a/tests/test_litellm/proxy/test_route_llm_request.py +++ b/tests/test_litellm/proxy/test_route_llm_request.py @@ -239,3 +239,35 @@ async def test_route_request_with_router_settings_override_preserves_existing(): assert call_kwargs["num_retries"] == 10 # Key/team timeout should be applied since not in request assert call_kwargs["timeout"] == 30 + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "mock_flag", + [ + "mock_testing_fallbacks", + "mock_testing_context_fallbacks", + "mock_testing_content_policy_fallbacks", + ], +) +async def test_route_request_strips_mock_testing_flags(mock_flag): + """VERIA-44: router-internal testing flags must not survive a + user-supplied request body. Without this strip, an attacker can + combine ``mock_testing_fallbacks=true`` with an unauthorized fallback + in ``router_settings_override`` to deterministically execute requests + against restricted models.""" + data = { + "model": "gpt-3.5-turbo", + "messages": [{"role": "user", "content": "Hello"}], + mock_flag: True, + } + llm_router = MagicMock() + llm_router.acompletion.return_value = "ok" + + await route_request(data, llm_router, None, "acompletion") + + call_kwargs = llm_router.acompletion.call_args[1] + assert mock_flag not in call_kwargs + # The flag is also gone from the original data dict so any subsequent + # processing (e.g. logging) doesn't see it either. + assert mock_flag not in data From e60a72ee1de40e97b48354b29d8cca856e810289 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 1 May 2026 08:06:10 +0000 Subject: [PATCH 12/25] fix(proxy): hardcode mock-testing strip list to avoid cyclic import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flagged the previous ``from litellm.types.router import MockRouterTestingParams`` at module top-level — ``litellm.types.router`` indirectly imports back into proxy modules, so the dataclass may not exist yet when ``route_llm_request`` is being imported. Hardcode the three flag names instead, with a guard test (``test_mock_testing_kwarg_names_matches_dataclass``) that asserts the hardcoded list matches ``MockRouterTestingParams.fields`` so drift is caught at test time rather than missed in production. Co-Authored-By: Claude Opus 4.7 (1M context) --- litellm/proxy/route_llm_request.py | 16 ++++++++++------ .../test_litellm/proxy/test_route_llm_request.py | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/litellm/proxy/route_llm_request.py b/litellm/proxy/route_llm_request.py index f611384b7a..bfe6b8484f 100644 --- a/litellm/proxy/route_llm_request.py +++ b/litellm/proxy/route_llm_request.py @@ -1,17 +1,21 @@ import asyncio -from dataclasses import fields as _dc_fields from typing import TYPE_CHECKING, Any, Literal, Optional from fastapi import HTTPException, status import litellm from litellm.proxy._types import UserAPIKeyAuth -from litellm.types.router import MockRouterTestingParams -# Router-internal mock_testing_* flag names. Single source of truth so a -# new flag added to ``MockRouterTestingParams`` is automatically stripped. -_MOCK_TESTING_KWARG_NAMES: tuple = tuple( - f.name for f in _dc_fields(MockRouterTestingParams) +# Router-internal mock_testing_* flag names — kept in sync with +# ``litellm.types.router.MockRouterTestingParams`` by the test +# ``test_mock_testing_kwarg_names_matches_dataclass``. Hardcoding (rather +# than deriving via ``dataclasses.fields(MockRouterTestingParams)`` at +# import time) avoids a cyclic import: ``litellm.types.router`` imports +# back into proxy modules before this module finishes loading. +_MOCK_TESTING_KWARG_NAMES: tuple = ( + "mock_testing_fallbacks", + "mock_testing_context_fallbacks", + "mock_testing_content_policy_fallbacks", ) if TYPE_CHECKING: diff --git a/tests/test_litellm/proxy/test_route_llm_request.py b/tests/test_litellm/proxy/test_route_llm_request.py index 9ef90085e4..98b0b6be02 100644 --- a/tests/test_litellm/proxy/test_route_llm_request.py +++ b/tests/test_litellm/proxy/test_route_llm_request.py @@ -241,6 +241,21 @@ async def test_route_request_with_router_settings_override_preserves_existing(): assert call_kwargs["timeout"] == 30 +def test_mock_testing_kwarg_names_matches_dataclass(): + """``_MOCK_TESTING_KWARG_NAMES`` is hardcoded to avoid a cyclic import + against ``litellm.types.router``. This test guards against drift — + if a new ``mock_testing_*`` field is added to ``MockRouterTestingParams`` + the strip list must be updated to keep covering it.""" + from dataclasses import fields + + from litellm.proxy.route_llm_request import _MOCK_TESTING_KWARG_NAMES + from litellm.types.router import MockRouterTestingParams + + assert set(_MOCK_TESTING_KWARG_NAMES) == { + f.name for f in fields(MockRouterTestingParams) + } + + @pytest.mark.asyncio @pytest.mark.parametrize( "mock_flag", From 60996ebf55796c914d39a38eef9636822e30a7d1 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 1 May 2026 14:08:53 -0700 Subject: [PATCH 13/25] chore: retrigger PR checks From e55401e39cdc6798738bee031d0e3276310f6425 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 1 May 2026 21:10:19 +0000 Subject: [PATCH 14/25] fix(auth): support JWT issuer verification, scope-warning when unscoped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When JWT auth is enabled but `JWT_AUDIENCE` is unset, `auth_jwt` disabled audience verification entirely. Tokens minted by any other application that shared the same IdP signing keys (Azure AD, Okta, etc.) were accepted as long as their signature checked out, even though their `aud` and `iss` claims pointed at unrelated apps. The proxy then fell into the no-team / no-user branch where access checks default-allow. This change: 1. Adds support for the `JWT_ISSUER` env var. When set, PyJWT verifies the token's `iss` claim — turning on the same defense for tokens that share an audience but come from a different IdP tenant. 2. Refactors the duplicated `jwt.decode` calls (RSA/EC/OKP path and x509 path) into a single `_build_decode_kwargs` helper that computes audience, issuer, and the corresponding `verify_*` opt-outs once per call. 3. Logs a single startup-time warning when JWT auth is enabled but neither `JWT_AUDIENCE` nor `JWT_ISSUER` is configured, so operators running the insecure default see a flag in their logs without getting spammed per-request. Default behavior (no env vars) is preserved for backward compatibility. Setting `JWT_AUDIENCE` and/or `JWT_ISSUER` opts into the verification. Co-Authored-By: Claude Opus 4.7 (1M context) --- litellm/proxy/auth/handle_jwt.py | 49 ++++++-- .../proxy/auth/test_handle_jwt.py | 113 ++++++++++++++++++ 2 files changed, 155 insertions(+), 7 deletions(-) diff --git a/litellm/proxy/auth/handle_jwt.py b/litellm/proxy/auth/handle_jwt.py index f50c950d74..8026c8a8ae 100644 --- a/litellm/proxy/auth/handle_jwt.py +++ b/litellm/proxy/auth/handle_jwt.py @@ -705,11 +705,48 @@ class JWTHandler: verbose_proxy_logger.error(f"Error fetching OIDC UserInfo: {str(e)}") raise Exception(f"Failed to fetch OIDC UserInfo: {str(e)}") - async def auth_jwt(self, token: str) -> dict: + _unscoped_jwt_warning_emitted = False + + @classmethod + def _build_decode_kwargs(cls) -> dict: + """Build the audience/issuer/options kwargs for ``jwt.decode``. + + Setting ``JWT_AUDIENCE`` (and optionally ``JWT_ISSUER``) turns on the + corresponding PyJWT verifications, blocking cross-tenant tokens + minted by other applications that share the same IdP signing keys. + When both are unset PyJWT only checks the signature and expiry, which + is preserved for backward compatibility but logged once as a warning. + """ audience = os.getenv("JWT_AUDIENCE") - decode_options = None + issuer = os.getenv("JWT_ISSUER") + + if ( + audience is None + and issuer is None + and not cls._unscoped_jwt_warning_emitted + ): + verbose_proxy_logger.warning( + "JWT auth is enabled but neither JWT_AUDIENCE nor JWT_ISSUER " + "is configured. Tokens minted by any application that shares " + "the same IdP signing keys will be accepted. Set JWT_AUDIENCE " + "(and ideally JWT_ISSUER) to scope this proxy." + ) + cls._unscoped_jwt_warning_emitted = True + + options: dict = {} if audience is None: - decode_options = {"verify_aud": False} + options["verify_aud"] = False + if issuer is None: + options["verify_iss"] = False + + return { + "audience": audience, + "issuer": issuer, + "options": options or None, + } + + async def auth_jwt(self, token: str) -> dict: + decode_kwargs = self._build_decode_kwargs() header = jwt.get_unverified_header(token) @@ -745,9 +782,8 @@ class JWTHandler: token, public_key_obj, # type: ignore algorithms=self.SUPPORTED_JWT_ALGORITHMS, - options=decode_options, # type: ignore[arg-type] - audience=audience, leeway=self.leeway, # allow testing of expired tokens + **decode_kwargs, ) return payload @@ -773,8 +809,7 @@ class JWTHandler: token, key, algorithms=self.SUPPORTED_JWT_ALGORITHMS, - audience=audience, - options=decode_options, + **decode_kwargs, ) return payload diff --git a/tests/test_litellm/proxy/auth/test_handle_jwt.py b/tests/test_litellm/proxy/auth/test_handle_jwt.py index 9085469268..4a3dca59a4 100644 --- a/tests/test_litellm/proxy/auth/test_handle_jwt.py +++ b/tests/test_litellm/proxy/auth/test_handle_jwt.py @@ -2565,3 +2565,116 @@ async def test_auth_builder_single_team_fallback_membership_error_skips_no_raise assert result["team_membership"] is None mock_get_team.assert_called() mock_get_membership.assert_called_once() + + +# --------------------------------------------------------------------------- +# JWTHandler._build_decode_kwargs — VERIA-27 (audience + issuer verification) +# --------------------------------------------------------------------------- + + +@pytest.fixture(autouse=False) +def _reset_unscoped_warning_flag(): + """Reset the once-per-process warning sentinel so each test sees a fresh + state.""" + JWTHandler._unscoped_jwt_warning_emitted = False + yield + JWTHandler._unscoped_jwt_warning_emitted = False + + +def test_build_decode_kwargs_no_env_disables_both_verifications( + monkeypatch, _reset_unscoped_warning_flag +): + monkeypatch.delenv("JWT_AUDIENCE", raising=False) + monkeypatch.delenv("JWT_ISSUER", raising=False) + + kwargs = JWTHandler._build_decode_kwargs() + + assert kwargs["audience"] is None + assert kwargs["issuer"] is None + assert kwargs["options"] == {"verify_aud": False, "verify_iss": False} + + +def test_build_decode_kwargs_audience_only_enables_aud_verification( + monkeypatch, _reset_unscoped_warning_flag +): + monkeypatch.setenv("JWT_AUDIENCE", "my-proxy") + monkeypatch.delenv("JWT_ISSUER", raising=False) + + kwargs = JWTHandler._build_decode_kwargs() + + assert kwargs["audience"] == "my-proxy" + assert kwargs["issuer"] is None + # verify_aud not in options means PyJWT will verify audience + assert kwargs["options"] == {"verify_iss": False} + + +def test_build_decode_kwargs_issuer_only_enables_iss_verification( + monkeypatch, _reset_unscoped_warning_flag +): + monkeypatch.delenv("JWT_AUDIENCE", raising=False) + monkeypatch.setenv("JWT_ISSUER", "https://idp.example.com/") + + kwargs = JWTHandler._build_decode_kwargs() + + assert kwargs["audience"] is None + assert kwargs["issuer"] == "https://idp.example.com/" + assert kwargs["options"] == {"verify_aud": False} + + +def test_build_decode_kwargs_both_set_enables_full_verification( + monkeypatch, _reset_unscoped_warning_flag +): + monkeypatch.setenv("JWT_AUDIENCE", "my-proxy") + monkeypatch.setenv("JWT_ISSUER", "https://idp.example.com/") + + kwargs = JWTHandler._build_decode_kwargs() + + assert kwargs["audience"] == "my-proxy" + assert kwargs["issuer"] == "https://idp.example.com/" + # No verification opt-outs — PyJWT verifies both claims by default. + assert kwargs["options"] is None + + +def test_build_decode_kwargs_warns_once_when_unscoped( + monkeypatch, _reset_unscoped_warning_flag, caplog +): + """The warning about unscoped JWT auth should fire on the first call but + not on every subsequent decode.""" + import logging + + monkeypatch.delenv("JWT_AUDIENCE", raising=False) + monkeypatch.delenv("JWT_ISSUER", raising=False) + caplog.set_level(logging.WARNING) + + JWTHandler._build_decode_kwargs() + JWTHandler._build_decode_kwargs() + JWTHandler._build_decode_kwargs() + + matching = [ + r + for r in caplog.records + if "JWT auth is enabled" in r.getMessage() + and "neither JWT_AUDIENCE nor JWT_ISSUER" in r.getMessage() + ] + assert ( + len(matching) == 1 + ), f"Expected exactly one warning across 3 calls, got {len(matching)}" + + +def test_build_decode_kwargs_no_warning_when_scoped( + monkeypatch, _reset_unscoped_warning_flag, caplog +): + import logging + + monkeypatch.setenv("JWT_AUDIENCE", "my-proxy") + monkeypatch.delenv("JWT_ISSUER", raising=False) + caplog.set_level(logging.WARNING) + + JWTHandler._build_decode_kwargs() + + matching = [ + r + for r in caplog.records + if "neither JWT_AUDIENCE nor JWT_ISSUER" in r.getMessage() + ] + assert matching == [] From 1b2756811e0118e4df1bec0b665f4a6003a3c5ad Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 1 May 2026 21:32:38 +0000 Subject: [PATCH 15/25] fix(proxy): close project hijacking and key org IDOR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related authorization gaps in management endpoints: 1. `/project/update` evaluated permission against the team_id supplied in the request body. By passing `data.team_id` pointing at a team they admin, a caller could hijack any project — `_check_user_permission_for_project` was given the attacker's team_object and happily checked admin membership against that. Drop the team_object kwarg so the helper re-fetches the existing project's team. Also require admin rights on the destination team when reassigning a project across teams, so a team admin cannot shed projects into another team's namespace. 2. `/key/update` accepted any `organization_id` and only checked that the org existed before applying limits. A caller could thereby point their key at an arbitrary org. Add `_validate_caller_can_assign_key_org` which enforces the same membership rule already applied on the `/key/list` filter path (`validate_key_list_check`); proxy admins and no-change updates skip the check. Tests cover both helpers in isolation: existing-team-admin allow, unrelated-team admin deny, proxy-admin shortcut, org-member allow, non-member deny, missing user_id deny, no-memberships deny. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../management_endpoints/project_endpoints.py | 47 +++-- .../key_management_endpoints.py | 58 +++++- .../test_project_org_authz.py | 195 ++++++++++++++++++ 3 files changed, 283 insertions(+), 17 deletions(-) create mode 100644 tests/test_litellm/proxy/management_endpoints/test_project_org_authz.py diff --git a/enterprise/litellm_enterprise/proxy/management_endpoints/project_endpoints.py b/enterprise/litellm_enterprise/proxy/management_endpoints/project_endpoints.py index f6ed7767c4..01d4fdd381 100644 --- a/enterprise/litellm_enterprise/proxy/management_endpoints/project_endpoints.py +++ b/enterprise/litellm_enterprise/proxy/management_endpoints/project_endpoints.py @@ -588,24 +588,21 @@ async def update_project( # noqa: PLR0915 param="project_id", ) - # Validate team exists and get team object for limit + permission checks - team_id_to_check = data.team_id or existing_project.team_id - team_obj_for_checks = None - if team_id_to_check is not None: - team_obj_for_checks = await _validate_team_exists( - team_id=team_id_to_check, prisma_client=prisma_client + # Permission to *edit* the project must be evaluated against the + # project's CURRENT team. Sourcing the team from `data.team_id` + # would let an admin of any team pass the check by supplying their + # own team_id, hijacking the project (VERIA-55). + target_team_id = data.team_id or existing_project.team_id + target_team_obj = None + if target_team_id is not None: + target_team_obj = await _validate_team_exists( + team_id=target_team_id, prisma_client=prisma_client ) - # Check if user has permission to update this project has_permission = await _check_user_permission_for_project( user_api_key_dict=user_api_key_dict, team_id=existing_project.team_id, prisma_client=prisma_client, - team_object=( - LiteLLM_TeamTable(**team_obj_for_checks.model_dump()) - if team_obj_for_checks - else None - ), ) if not has_permission: @@ -614,10 +611,32 @@ async def update_project( # noqa: PLR0915 detail={"error": "Only admins or team admins can update projects"}, ) + # Reassigning to a different team also requires admin rights on the + # destination team — otherwise a team admin could shed projects into + # an unsuspecting team's namespace. + if data.team_id is not None and data.team_id != existing_project.team_id: + can_assign_to_target = await _check_user_permission_for_project( + user_api_key_dict=user_api_key_dict, + team_id=data.team_id, + prisma_client=prisma_client, + team_object=( + LiteLLM_TeamTable(**target_team_obj.model_dump()) + if target_team_obj + else None + ), + ) + if not can_assign_to_target: + raise HTTPException( + status_code=403, + detail={ + "error": "Cannot reassign project to a team you are not an admin of" + }, + ) + # Validate project limits against team limits - if team_obj_for_checks is not None: + if target_team_obj is not None: _check_team_project_limits( - team_object=LiteLLM_TeamTable(**team_obj_for_checks.model_dump()), + team_object=LiteLLM_TeamTable(**target_team_obj.model_dump()), data=data, ) diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index 2485aea14f..52dbfd1ece 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -1168,6 +1168,42 @@ def check_org_key_rpm_tpm_limits( ) +async def _validate_caller_can_assign_key_org( + user_api_key_dict: UserAPIKeyAuth, + organization_id: str, + prisma_client: PrismaClient, +) -> None: + """Reject ``/key/update`` requests that point a key at an organization + the caller does not belong to. + + Mirrors the org-membership rule already enforced on ``/key/list`` in + ``validate_key_list_check``. Proxy admins are checked at the call site. + """ + if user_api_key_dict.user_id is None: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Cannot assign a key to an organization without a user_id on the caller's token", + ) + + user_row = await prisma_client.db.litellm_usertable.find_unique( + where={"user_id": user_api_key_dict.user_id}, + include={"organization_memberships": True}, + ) + memberships = ( + getattr(user_row, "organization_memberships", None) if user_row else None + ) + member_org_ids = { + membership.organization_id + for membership in (memberships or []) + if membership.organization_id is not None + } + if organization_id not in member_org_ids: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=f"Caller is not a member of organization_id={organization_id}", + ) + + async def _check_org_key_limits( org_table: LiteLLM_OrganizationTable, data: Union[GenerateKeyRequest, UpdateKeyRequest], @@ -2168,10 +2204,26 @@ async def _validate_update_key_data( user_api_key_cache=user_api_key_cache, ) + # When the caller asks to change the key's organization_id, require that + # they are a member of (or a proxy admin over) the target organization. + # Without this gate, any caller could assign their key to an arbitrary + # organization_id by passing it in the request body — VERIA-55 secondary + # IDOR. The check mirrors the membership rule already used on the + # `/key/list` filter path in `validate_key_list_check`. + _existing_org_id = getattr(existing_key_row, "organization_id", None) + if ( + data.organization_id is not None + and data.organization_id != _existing_org_id + and not _is_proxy_admin + ): + await _validate_caller_can_assign_key_org( + user_api_key_dict=user_api_key_dict, + organization_id=data.organization_id, + prisma_client=prisma_client, + ) + # Check org key limits only when throughput-related fields or organization_id change - _org_id_to_check = data.organization_id or getattr( - existing_key_row, "organization_id", None - ) + _org_id_to_check = data.organization_id or _existing_org_id _throughput_fields_changed = ( data.organization_id is not None or data.tpm_limit is not None diff --git a/tests/test_litellm/proxy/management_endpoints/test_project_org_authz.py b/tests/test_litellm/proxy/management_endpoints/test_project_org_authz.py new file mode 100644 index 0000000000..bd982480d6 --- /dev/null +++ b/tests/test_litellm/proxy/management_endpoints/test_project_org_authz.py @@ -0,0 +1,195 @@ +""" +Unit tests for the VERIA-55 fixes: + +- Project update permission must be evaluated against the project's *current* + team, not a team supplied in the request body. +- Key update may not assign a key to an organization the caller is not a + member of. +""" + +from unittest.mock import AsyncMock, MagicMock + +import pytest +from fastapi import HTTPException + +from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth + + +# --------------------------------------------------------------------------- +# /project/update — _check_user_permission_for_project +# --------------------------------------------------------------------------- + + +def _make_prisma_with_team(team_id: str, admins: list): + prisma = MagicMock() + team_row = MagicMock() + team_row.team_id = team_id + team_row.admins = admins + prisma.db.litellm_teamtable.find_unique = AsyncMock(return_value=team_row) + return prisma + + +@pytest.mark.asyncio +async def test_project_perm_check_uses_current_team_not_caller_supplied(): + """The permission check must look at the project's existing team. Even + if the caller is admin of an unrelated team, they must not pass when no + explicit team_object is forced through.""" + from enterprise.litellm_enterprise.proxy.management_endpoints.project_endpoints import ( + _check_user_permission_for_project, + ) + + # Project lives on team-A, caller is admin only of team-B. + prisma = _make_prisma_with_team(team_id="team-A", admins=["alice"]) + caller = UserAPIKeyAuth( + user_id="bob", + user_role=LitellmUserRoles.INTERNAL_USER.value, + ) + + has_perm = await _check_user_permission_for_project( + user_api_key_dict=caller, + team_id="team-A", + prisma_client=prisma, + ) + assert has_perm is False + prisma.db.litellm_teamtable.find_unique.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_project_perm_check_allows_team_admin_of_existing_team(): + from enterprise.litellm_enterprise.proxy.management_endpoints.project_endpoints import ( + _check_user_permission_for_project, + ) + + prisma = _make_prisma_with_team(team_id="team-A", admins=["alice"]) + alice = UserAPIKeyAuth( + user_id="alice", + user_role=LitellmUserRoles.INTERNAL_USER.value, + ) + + has_perm = await _check_user_permission_for_project( + user_api_key_dict=alice, + team_id="team-A", + prisma_client=prisma, + ) + assert has_perm is True + + +@pytest.mark.asyncio +async def test_project_perm_check_proxy_admin_always_allowed(): + from enterprise.litellm_enterprise.proxy.management_endpoints.project_endpoints import ( + _check_user_permission_for_project, + ) + + prisma = MagicMock() + admin = UserAPIKeyAuth( + user_id="root", + user_role=LitellmUserRoles.PROXY_ADMIN.value, + ) + + has_perm = await _check_user_permission_for_project( + user_api_key_dict=admin, + team_id="team-A", + prisma_client=prisma, + ) + assert has_perm is True + # Admin shortcut should not even hit the DB. + prisma.db.litellm_teamtable.find_unique.assert_not_called() + + +# --------------------------------------------------------------------------- +# /key/update — _validate_caller_can_assign_key_org +# --------------------------------------------------------------------------- + + +def _make_prisma_with_user_orgs(user_id: str, org_ids: list): + prisma = MagicMock() + user_row = MagicMock() + user_row.organization_memberships = [ + MagicMock(organization_id=org_id) for org_id in org_ids + ] + prisma.db.litellm_usertable.find_unique = AsyncMock(return_value=user_row) + return prisma + + +@pytest.mark.asyncio +async def test_assign_key_org_allows_member(): + from litellm.proxy.management_endpoints.key_management_endpoints import ( + _validate_caller_can_assign_key_org, + ) + + prisma = _make_prisma_with_user_orgs("alice", ["org-1", "org-2"]) + caller = UserAPIKeyAuth( + user_id="alice", + user_role=LitellmUserRoles.INTERNAL_USER.value, + ) + # Should not raise. + await _validate_caller_can_assign_key_org( + user_api_key_dict=caller, + organization_id="org-2", + prisma_client=prisma, + ) + + +@pytest.mark.asyncio +async def test_assign_key_org_blocks_non_member(): + """The IDOR: caller asks to point a key at an org they don't belong to.""" + from litellm.proxy.management_endpoints.key_management_endpoints import ( + _validate_caller_can_assign_key_org, + ) + + prisma = _make_prisma_with_user_orgs("alice", ["org-1"]) + caller = UserAPIKeyAuth( + user_id="alice", + user_role=LitellmUserRoles.INTERNAL_USER.value, + ) + with pytest.raises(HTTPException) as exc_info: + await _validate_caller_can_assign_key_org( + user_api_key_dict=caller, + organization_id="someone-elses-org", + prisma_client=prisma, + ) + assert exc_info.value.status_code == 403 + assert "someone-elses-org" in str(exc_info.value.detail) + + +@pytest.mark.asyncio +async def test_assign_key_org_blocks_caller_without_user_id(): + from litellm.proxy.management_endpoints.key_management_endpoints import ( + _validate_caller_can_assign_key_org, + ) + + prisma = MagicMock() + caller = UserAPIKeyAuth( + user_role=LitellmUserRoles.INTERNAL_USER.value, + ) + with pytest.raises(HTTPException) as exc_info: + await _validate_caller_can_assign_key_org( + user_api_key_dict=caller, + organization_id="org-1", + prisma_client=prisma, + ) + assert exc_info.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_assign_key_org_blocks_caller_with_no_memberships(): + from litellm.proxy.management_endpoints.key_management_endpoints import ( + _validate_caller_can_assign_key_org, + ) + + prisma = MagicMock() + user_row = MagicMock() + user_row.organization_memberships = None + prisma.db.litellm_usertable.find_unique = AsyncMock(return_value=user_row) + + caller = UserAPIKeyAuth( + user_id="alice", + user_role=LitellmUserRoles.INTERNAL_USER.value, + ) + with pytest.raises(HTTPException) as exc_info: + await _validate_caller_can_assign_key_org( + user_api_key_dict=caller, + organization_id="org-1", + prisma_client=prisma, + ) + assert exc_info.value.status_code == 403 From 6a5ecafdffa1cc3184c2fb72f03399f9063cd45d Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 1 May 2026 21:38:19 +0000 Subject: [PATCH 16/25] fix(prometheus): quote api_key for PromQL string literal in spend lookup `get_daily_spend_from_prometheus` was interpolating the `api_key` query parameter into a PromQL `hashed_api_key="..."` label matcher with an f-string. Any caller of `/global/spend/logs` could inject a bare `"` to terminate the matcher and append arbitrary PromQL operators or extra metric selectors, exfiltrating cross-tenant telemetry from the connected Prometheus instance. Replace the f-string with `_quote_promql_string_literal`, which uses `json.dumps` to render a complete Go-compatible double-quoted literal. PromQL string literals follow Go's escape rules per https://prometheus.io/docs/prometheus/latest/querying/basics/, and JSON's quoting is a strict subset, so the same escape covers backslash, embedded quote, and control-character cases without rolling a bespoke escape table. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../prometheus_helpers/prometheus_api.py | 24 ++- .../test_prometheus_api_promql_escape.py | 150 ++++++++++++++++++ 2 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 tests/test_litellm/integrations/test_prometheus_api_promql_escape.py diff --git a/litellm/integrations/prometheus_helpers/prometheus_api.py b/litellm/integrations/prometheus_helpers/prometheus_api.py index b25da57723..0901d7b680 100644 --- a/litellm/integrations/prometheus_helpers/prometheus_api.py +++ b/litellm/integrations/prometheus_helpers/prometheus_api.py @@ -2,6 +2,7 @@ Helper functions to query prometheus API """ +import json import time from datetime import datetime, timedelta from typing import Optional @@ -81,6 +82,24 @@ def is_prometheus_connected() -> bool: return False +def _quote_promql_string_literal(value: str) -> str: + """Render ``value`` as a PromQL double-quoted string literal. + + PromQL string literals follow Go's escape rules + (https://prometheus.io/docs/prometheus/latest/querying/basics/): a + backslash begins an escape sequence and a bare ``"`` ends the literal. + Without escaping, callers that accept arbitrary user-supplied values + (like the ``api_key`` filter on ``/global/spend/logs``) can inject extra + label matchers or selectors and read cross-tenant metrics. + + JSON's quoting rules are a strict subset of Go's, so ``json.dumps`` of + a Python string produces a literal Prometheus accepts: ``\\``, ``\\"``, + and the standard ``\\n`` / ``\\t`` / ``\\uNNNN`` control-character + escapes. The returned value already includes the surrounding quotes. + """ + return json.dumps(value, ensure_ascii=False) + + async def get_daily_spend_from_prometheus(api_key: Optional[str]): """ Expected Response Format: @@ -109,8 +128,11 @@ async def get_daily_spend_from_prometheus(api_key: Optional[str]): if api_key is None: query = "sum(delta(litellm_spend_metric_total[1d]))" else: + quoted_api_key = _quote_promql_string_literal(api_key) query = ( - f'sum(delta(litellm_spend_metric_total{{hashed_api_key="{api_key}"}}[1d]))' + "sum(delta(litellm_spend_metric_total{" + f"hashed_api_key={quoted_api_key}" + "}[1d]))" ) params = { diff --git a/tests/test_litellm/integrations/test_prometheus_api_promql_escape.py b/tests/test_litellm/integrations/test_prometheus_api_promql_escape.py new file mode 100644 index 0000000000..262ca6b692 --- /dev/null +++ b/tests/test_litellm/integrations/test_prometheus_api_promql_escape.py @@ -0,0 +1,150 @@ +""" +Tests for VERIA-53: PromQL string-literal quoting in +``get_daily_spend_from_prometheus``. + +PromQL string literals follow Go's escape rules +(https://prometheus.io/docs/prometheus/latest/querying/basics/). JSON's +quoting is a strict subset of Go's, so ``json.dumps`` produces a literal +Prometheus parses identically. +""" + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + + +def test_quote_safe_input_round_trips(): + from litellm.integrations.prometheus_helpers.prometheus_api import ( + _quote_promql_string_literal, + ) + + assert _quote_promql_string_literal("sk-abc123") == '"sk-abc123"' + assert _quote_promql_string_literal("hash:deadbeef") == '"hash:deadbeef"' + + +def test_quote_escapes_double_quote(): + from litellm.integrations.prometheus_helpers.prometheus_api import ( + _quote_promql_string_literal, + ) + + # A bare double quote would otherwise terminate the label matcher and + # let the attacker append `, foo="..."} or sum(...)`. + assert _quote_promql_string_literal('hello"injected') == '"hello\\"injected"' + + +def test_quote_escapes_backslash(): + from litellm.integrations.prometheus_helpers.prometheus_api import ( + _quote_promql_string_literal, + ) + + assert _quote_promql_string_literal('a\\"b') == '"a\\\\\\"b"' + + +def test_quote_escapes_newlines_and_control_chars(): + """Beyond the security minimum, the canonical Go/JSON escape also + handles control characters that would otherwise produce an invalid + PromQL string literal.""" + from litellm.integrations.prometheus_helpers.prometheus_api import ( + _quote_promql_string_literal, + ) + + assert _quote_promql_string_literal("a\nb") == '"a\\nb"' + assert _quote_promql_string_literal("a\tb") == '"a\\tb"' + assert _quote_promql_string_literal("a\rb") == '"a\\rb"' + + +@pytest.mark.asyncio +async def test_get_daily_spend_does_not_pass_raw_quote_into_query(): + from litellm.integrations.prometheus_helpers import prometheus_api + + captured = {} + + class _FakeResponse: + def json(self): + return {"data": {"result": []}} + + async def _capture(url, params): + captured["url"] = url + captured["params"] = params + return _FakeResponse() + + fake_client = MagicMock() + fake_client.get = AsyncMock(side_effect=_capture) + + with patch.object(prometheus_api, "PROMETHEUS_URL", "http://prom.example"): + with patch.object(prometheus_api, "async_http_handler", fake_client): + await prometheus_api.get_daily_spend_from_prometheus( + api_key='sk-victim"} or sum(other_metric{a="b' + ) + + rendered_query = captured["params"]["query"] + # The legitimate matcher framing must still be intact: one outer + # `delta()` window, one inner `hashed_api_key="..."` matcher. + assert rendered_query.startswith( + 'sum(delta(litellm_spend_metric_total{hashed_api_key="' + ) + assert rendered_query.endswith('"}[1d]))') + + # Every injected `"` from the attacker payload appears as `\"` so the + # PromQL parser treats them as literal characters inside the matcher + # value, never as the terminator that would let the rest parse as + # PromQL syntax. + inner = rendered_query[ + len('sum(delta(litellm_spend_metric_total{hashed_api_key="') : -len('"}[1d]))') + ] + assert '"' not in inner.replace('\\"', "") + + +@pytest.mark.asyncio +async def test_get_daily_spend_with_no_api_key_uses_unfiltered_query(): + from litellm.integrations.prometheus_helpers import prometheus_api + + captured = {} + + class _FakeResponse: + def json(self): + return {"data": {"result": []}} + + async def _capture(url, params): + captured["params"] = params + return _FakeResponse() + + fake_client = MagicMock() + fake_client.get = AsyncMock(side_effect=_capture) + + with patch.object(prometheus_api, "PROMETHEUS_URL", "http://prom.example"): + with patch.object(prometheus_api, "async_http_handler", fake_client): + await prometheus_api.get_daily_spend_from_prometheus(api_key=None) + + assert captured["params"]["query"] == "sum(delta(litellm_spend_metric_total[1d]))" + + +@pytest.mark.asyncio +async def test_get_daily_spend_legitimate_hashed_key_unchanged(): + """A normal hex hashed_api_key flows through `json.dumps` as itself + plus the surrounding quotes — no spurious escaping that would break + real lookups.""" + from litellm.integrations.prometheus_helpers import prometheus_api + + captured = {} + + class _FakeResponse: + def json(self): + return {"data": {"result": []}} + + async def _capture(url, params): + captured["params"] = params + return _FakeResponse() + + fake_client = MagicMock() + fake_client.get = AsyncMock(side_effect=_capture) + + legit_key = "a" * 64 # 64-char hex-ish hashed key + with patch.object(prometheus_api, "PROMETHEUS_URL", "http://prom.example"): + with patch.object(prometheus_api, "async_http_handler", fake_client): + await prometheus_api.get_daily_spend_from_prometheus(api_key=legit_key) + + assert ( + captured["params"]["query"] + == f'sum(delta(litellm_spend_metric_total{{hashed_api_key="{legit_key}"}}[1d]))' + ) From f17d7796669d83f8a4fd3ed96a7b35ca0b7227c8 Mon Sep 17 00:00:00 2001 From: Ryan Crabbe Date: Fri, 1 May 2026 15:54:03 -0700 Subject: [PATCH 17/25] fix: scope key access_group_ids override by team's assigned groups A team member could set any access_group_ids on their key (e.g. a group assigned only to a different team) and override the team's model restriction. Intersect the key's access_group_ids with team_object.access_group_ids in _key_access_group_grants_model so foreign groups are dropped before model expansion. Adds a regression test that asserts expansion is never called for foreign groups. --- litellm/proxy/auth/auth_checks.py | 17 ++++- tests/proxy_unit_tests/test_auth_checks.py | 89 ++++++++++++++++++++++ 2 files changed, 102 insertions(+), 4 deletions(-) diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index e959867091..bfcd13d909 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -512,6 +512,7 @@ async def common_checks( # noqa: PLR0915 if not await _key_access_group_grants_model( model=_model, valid_token=valid_token, + team_object=team_object, llm_router=llm_router, ): raise @@ -2870,20 +2871,28 @@ async def can_team_access_model( async def _key_access_group_grants_model( model: Union[str, List[str]], valid_token: Optional[UserAPIKeyAuth], + team_object: Optional[LiteLLM_TeamTable], llm_router: Optional[Router], ) -> bool: """ Returns True if the key's `access_group_ids` expand to models that grant access to `model`. Used to let a key's access group override a team's model restriction in `common_checks`. + + A key's access group only counts if it is also assigned to the key's team + (i.e., present in `team_object.access_group_ids`). This preserves the + team-as-owner boundary: a team member cannot escalate by naming an access + group that belongs to a different team. """ - if valid_token is None: + if valid_token is None or team_object is None: return False - key_access_group_ids = valid_token.access_group_ids or [] - if not key_access_group_ids: + key_access_group_ids = set(valid_token.access_group_ids or []) + team_access_group_ids = set(team_object.access_group_ids or []) + allowed_group_ids = key_access_group_ids & team_access_group_ids + if not allowed_group_ids: return False models_from_groups = await _get_models_from_access_groups( - access_group_ids=key_access_group_ids, + access_group_ids=list(allowed_group_ids), ) if not models_from_groups: return False diff --git a/tests/proxy_unit_tests/test_auth_checks.py b/tests/proxy_unit_tests/test_auth_checks.py index 6a404712c1..026abf99d8 100644 --- a/tests/proxy_unit_tests/test_auth_checks.py +++ b/tests/proxy_unit_tests/test_auth_checks.py @@ -1164,6 +1164,12 @@ async def test_key_access_group_grants_model_when_group_covers_model(): token="test-token", models=[], access_group_ids=["ryan-access-group"], + team_id="team-a", + ) + team_object = LiteLLM_TeamTable( + team_id="team-a", + models=["mock-success"], + access_group_ids=["ryan-access-group"], ) with patch( @@ -1175,6 +1181,7 @@ async def test_key_access_group_grants_model_when_group_covers_model(): await _key_access_group_grants_model( model="claude-haiku-4-5", valid_token=valid_token, + team_object=team_object, llm_router=None, ) is True @@ -1190,11 +1197,18 @@ async def test_key_access_group_grants_model_when_key_has_no_groups(): token="test-token", models=[], access_group_ids=[], + team_id="team-a", + ) + team_object = LiteLLM_TeamTable( + team_id="team-a", + models=["mock-success"], + access_group_ids=["ryan-access-group"], ) assert ( await _key_access_group_grants_model( model="claude-haiku-4-5", valid_token=valid_token, + team_object=team_object, llm_router=None, ) is False @@ -1212,6 +1226,12 @@ async def test_key_access_group_grants_model_when_group_does_not_cover_model(): token="test-token", models=[], access_group_ids=["other-group"], + team_id="team-a", + ) + team_object = LiteLLM_TeamTable( + team_id="team-a", + models=["mock-success"], + access_group_ids=["other-group"], ) with patch( @@ -1223,7 +1243,76 @@ async def test_key_access_group_grants_model_when_group_does_not_cover_model(): await _key_access_group_grants_model( model="claude-haiku-4-5", valid_token=valid_token, + team_object=team_object, llm_router=None, ) is False ) + + +@pytest.mark.asyncio +async def test_key_access_group_grants_model_when_group_not_assigned_to_team(): + """ + Regression test: a team member naming a foreign access group on their key + must NOT escalate to that group's models. The group expands to the requested + model, but it isn't assigned to the key's team — so the override is denied. + """ + from unittest.mock import AsyncMock, patch + + from litellm.proxy.auth.auth_checks import _key_access_group_grants_model + + valid_token = UserAPIKeyAuth( + token="test-token", + models=[], + access_group_ids=["team-b-premium"], + team_id="team-a", + ) + team_object = LiteLLM_TeamTable( + team_id="team-a", + models=["mock-success"], + access_group_ids=["team-a-basic"], + ) + + with patch( + "litellm.proxy.auth.auth_checks._get_models_from_access_groups", + new_callable=AsyncMock, + return_value=["claude-opus-4-5"], + ) as mocked_expand: + assert ( + await _key_access_group_grants_model( + model="claude-opus-4-5", + valid_token=valid_token, + team_object=team_object, + llm_router=None, + ) + is False + ) + # Foreign group must be filtered out before expansion ever runs. + mocked_expand.assert_not_called() + + +@pytest.mark.asyncio +async def test_key_access_group_grants_model_when_team_has_no_groups(): + """Team with no access_group_ids leaves the intersection empty → denied.""" + from litellm.proxy.auth.auth_checks import _key_access_group_grants_model + + valid_token = UserAPIKeyAuth( + token="test-token", + models=[], + access_group_ids=["ryan-access-group"], + team_id="team-a", + ) + team_object = LiteLLM_TeamTable( + team_id="team-a", + models=["mock-success"], + access_group_ids=[], + ) + assert ( + await _key_access_group_grants_model( + model="claude-haiku-4-5", + valid_token=valid_token, + team_object=team_object, + llm_router=None, + ) + is False + ) From bf4c250d86a492d25d71111bd6ba3e7c315b6419 Mon Sep 17 00:00:00 2001 From: Ryan Crabbe Date: Fri, 1 May 2026 16:29:33 -0700 Subject: [PATCH 18/25] fix: gate key access_group override on group's own assignment Replaces the previous intersect-with-team.access_group_ids check, which made the override unreachable in practice (the team-gate fallback already covered every case the intersection allowed). The override now resolves each of the key's access_group_ids via get_access_object and accepts the group only if its assigned_team_ids includes the key's team_id, or its assigned_key_ids includes the key's token. This fulfills the original ask (a key can extend a team's allow-list via a group the admin granted to that team or that specific key) while still rejecting foreign groups referenced by team members of other teams. --- litellm/proxy/auth/auth_checks.py | 55 +++-- tests/proxy_unit_tests/test_auth_checks.py | 233 +++++++++++++++++---- 2 files changed, 232 insertions(+), 56 deletions(-) diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index bfcd13d909..5bd489aa30 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -2879,28 +2879,57 @@ async def _key_access_group_grants_model( access to `model`. Used to let a key's access group override a team's model restriction in `common_checks`. - A key's access group only counts if it is also assigned to the key's team - (i.e., present in `team_object.access_group_ids`). This preserves the - team-as-owner boundary: a team member cannot escalate by naming an access - group that belongs to a different team. + A key's access group only counts if the access group itself authorizes the + caller as an owner — that is, the group's `assigned_team_ids` includes the + key's `team_id`, or the group's `assigned_key_ids` includes the key's + token. This preserves the team-as-owner boundary (a team member cannot + escalate by naming a group assigned to a different team) while still + letting a group reach the key without first being added to the team's + `access_group_ids` list. """ - if valid_token is None or team_object is None: + if valid_token is None: return False - key_access_group_ids = set(valid_token.access_group_ids or []) - team_access_group_ids = set(team_object.access_group_ids or []) - allowed_group_ids = key_access_group_ids & team_access_group_ids - if not allowed_group_ids: + key_access_group_ids = list(valid_token.access_group_ids or []) + if not key_access_group_ids: return False - models_from_groups = await _get_models_from_access_groups( - access_group_ids=list(allowed_group_ids), + + from litellm.proxy.proxy_server import prisma_client as _prisma_client + from litellm.proxy.proxy_server import proxy_logging_obj as _proxy_logging_obj + from litellm.proxy.proxy_server import user_api_key_cache as _user_api_key_cache + + if _prisma_client is None or _user_api_key_cache is None: + return False + + key_team_id = valid_token.team_id or ( + team_object.team_id if team_object is not None else None ) - if not models_from_groups: + key_token = valid_token.token + + authorized_models: List[str] = [] + for ag_id in key_access_group_ids: + try: + ag = await get_access_object( + access_group_id=ag_id, + prisma_client=_prisma_client, + user_api_key_cache=_user_api_key_cache, + proxy_logging_obj=_proxy_logging_obj, + ) + except Exception: + continue + team_authorized = bool( + key_team_id and key_team_id in (ag.assigned_team_ids or []) + ) + key_authorized = bool(key_token and key_token in (ag.assigned_key_ids or [])) + if team_authorized or key_authorized: + authorized_models.extend(ag.access_model_names or []) + + if not authorized_models: return False try: _can_object_call_model( model=model, llm_router=llm_router, - models=models_from_groups, + models=list(set(authorized_models)), team_model_aliases=valid_token.team_model_aliases, team_id=valid_token.team_id, object_type="key", diff --git a/tests/proxy_unit_tests/test_auth_checks.py b/tests/proxy_unit_tests/test_auth_checks.py index 026abf99d8..72914516b5 100644 --- a/tests/proxy_unit_tests/test_auth_checks.py +++ b/tests/proxy_unit_tests/test_auth_checks.py @@ -1153,9 +1153,44 @@ async def test_can_key_call_model_via_access_group_ids(): # --------------------------------------------------------------------------- +def _patch_proxy_server_globals(): + """Patch proxy_server's prisma_client and user_api_key_cache to non-None mocks + so the helper's None-guard doesn't short-circuit. The actual values don't + matter because get_access_object is patched separately to return fixtures.""" + from unittest.mock import MagicMock, patch + + return [ + patch("litellm.proxy.proxy_server.prisma_client", MagicMock()), + patch("litellm.proxy.proxy_server.user_api_key_cache", MagicMock()), + patch("litellm.proxy.proxy_server.proxy_logging_obj", MagicMock()), + ] + + +def _fake_access_group( + access_group_id: str, + access_model_names=None, + assigned_team_ids=None, + assigned_key_ids=None, +): + from litellm.proxy._types import LiteLLM_AccessGroupTable + + return LiteLLM_AccessGroupTable( + access_group_id=access_group_id, + access_group_name=access_group_id, + access_model_names=access_model_names or [], + assigned_team_ids=assigned_team_ids or [], + assigned_key_ids=assigned_key_ids or [], + ) + + @pytest.mark.asyncio -async def test_key_access_group_grants_model_when_group_covers_model(): - """Key's access_group_ids expand to a set that includes the requested model.""" +async def test_key_access_group_grants_model_when_team_authorized(): + """Group's assigned_team_ids includes the key's team and grants the model → True. + + This is the happy path equivalent of Andres's report: admin creates an + access group with assigned_team_ids=[team-a], grants claude-haiku-4-5, + attaches it to a key on team-a. Override fires. + """ from unittest.mock import AsyncMock, patch from litellm.proxy.auth.auth_checks import _key_access_group_grants_model @@ -1163,20 +1198,31 @@ async def test_key_access_group_grants_model_when_group_covers_model(): valid_token = UserAPIKeyAuth( token="test-token", models=[], - access_group_ids=["ryan-access-group"], + access_group_ids=["premium-group"], team_id="team-a", ) team_object = LiteLLM_TeamTable( team_id="team-a", models=["mock-success"], - access_group_ids=["ryan-access-group"], + access_group_ids=[], # deliberately not synced — the access group itself authorizes ) - with patch( - "litellm.proxy.auth.auth_checks._get_models_from_access_groups", - new_callable=AsyncMock, - return_value=["claude-haiku-4-5"], - ): + fake_ag = _fake_access_group( + access_group_id="premium-group", + access_model_names=["claude-haiku-4-5"], + assigned_team_ids=["team-a"], + ) + + patches = _patch_proxy_server_globals() + [ + patch( + "litellm.proxy.auth.auth_checks.get_access_object", + new_callable=AsyncMock, + return_value=fake_ag, + ), + ] + for p in patches: + p.start() + try: assert ( await _key_access_group_grants_model( model="claude-haiku-4-5", @@ -1186,11 +1232,68 @@ async def test_key_access_group_grants_model_when_group_covers_model(): ) is True ) + finally: + for p in patches: + p.stop() + + +@pytest.mark.asyncio +async def test_key_access_group_grants_model_when_key_directly_authorized(): + """Group's assigned_key_ids includes the key's token and grants the model → True. + + Per-key authorization path: an admin scopes a group directly to a key + (assigned_key_ids) without listing the team. + """ + from unittest.mock import AsyncMock, patch + + from litellm.proxy.auth.auth_checks import _key_access_group_grants_model + + valid_token = UserAPIKeyAuth( + token="test-token-hashed", + models=[], + access_group_ids=["per-key-group"], + team_id="team-a", + ) + team_object = LiteLLM_TeamTable( + team_id="team-a", + models=["mock-success"], + access_group_ids=[], + ) + + fake_ag = _fake_access_group( + access_group_id="per-key-group", + access_model_names=["claude-haiku-4-5"], + assigned_team_ids=[], + assigned_key_ids=["test-token-hashed"], + ) + + patches = _patch_proxy_server_globals() + [ + patch( + "litellm.proxy.auth.auth_checks.get_access_object", + new_callable=AsyncMock, + return_value=fake_ag, + ), + ] + for p in patches: + p.start() + try: + assert ( + await _key_access_group_grants_model( + model="claude-haiku-4-5", + valid_token=valid_token, + team_object=team_object, + llm_router=None, + ) + is True + ) + finally: + for p in patches: + p.stop() @pytest.mark.asyncio async def test_key_access_group_grants_model_when_key_has_no_groups(): - """Key with no access_group_ids cannot override team denial.""" + """Key with no access_group_ids → False (early return, no DB read).""" from litellm.proxy.auth.auth_checks import _key_access_group_grants_model valid_token = UserAPIKeyAuth( @@ -1202,7 +1305,7 @@ async def test_key_access_group_grants_model_when_key_has_no_groups(): team_object = LiteLLM_TeamTable( team_id="team-a", models=["mock-success"], - access_group_ids=["ryan-access-group"], + access_group_ids=["any-group"], ) assert ( await _key_access_group_grants_model( @@ -1217,7 +1320,7 @@ async def test_key_access_group_grants_model_when_key_has_no_groups(): @pytest.mark.asyncio async def test_key_access_group_grants_model_when_group_does_not_cover_model(): - """Key's access_group_ids expand to models that do not include the request.""" + """Group authorizes the team but does not grant the requested model → False.""" from unittest.mock import AsyncMock, patch from litellm.proxy.auth.auth_checks import _key_access_group_grants_model @@ -1225,20 +1328,31 @@ async def test_key_access_group_grants_model_when_group_does_not_cover_model(): valid_token = UserAPIKeyAuth( token="test-token", models=[], - access_group_ids=["other-group"], + access_group_ids=["basic-group"], team_id="team-a", ) team_object = LiteLLM_TeamTable( team_id="team-a", models=["mock-success"], - access_group_ids=["other-group"], + access_group_ids=[], ) - with patch( - "litellm.proxy.auth.auth_checks._get_models_from_access_groups", - new_callable=AsyncMock, - return_value=["gpt-4o-mini"], - ): + fake_ag = _fake_access_group( + access_group_id="basic-group", + access_model_names=["gpt-4o-mini"], + assigned_team_ids=["team-a"], + ) + + patches = _patch_proxy_server_globals() + [ + patch( + "litellm.proxy.auth.auth_checks.get_access_object", + new_callable=AsyncMock, + return_value=fake_ag, + ), + ] + for p in patches: + p.start() + try: assert ( await _key_access_group_grants_model( model="claude-haiku-4-5", @@ -1248,21 +1362,25 @@ async def test_key_access_group_grants_model_when_group_does_not_cover_model(): ) is False ) + finally: + for p in patches: + p.stop() @pytest.mark.asyncio -async def test_key_access_group_grants_model_when_group_not_assigned_to_team(): +async def test_key_access_group_grants_model_when_group_authorizes_neither(): """ - Regression test: a team member naming a foreign access group on their key - must NOT escalate to that group's models. The group expands to the requested - model, but it isn't assigned to the key's team — so the override is denied. + Bypass regression test: a team member sets a foreign access group on their + key. The group grants the requested model but its assigned_team_ids / + assigned_key_ids do not include this caller's team or token. Override is + denied — the team's 401 propagates. """ from unittest.mock import AsyncMock, patch from litellm.proxy.auth.auth_checks import _key_access_group_grants_model valid_token = UserAPIKeyAuth( - token="test-token", + token="team-a-token", models=[], access_group_ids=["team-b-premium"], team_id="team-a", @@ -1270,14 +1388,26 @@ async def test_key_access_group_grants_model_when_group_not_assigned_to_team(): team_object = LiteLLM_TeamTable( team_id="team-a", models=["mock-success"], - access_group_ids=["team-a-basic"], + access_group_ids=[], ) - with patch( - "litellm.proxy.auth.auth_checks._get_models_from_access_groups", - new_callable=AsyncMock, - return_value=["claude-opus-4-5"], - ) as mocked_expand: + fake_ag = _fake_access_group( + access_group_id="team-b-premium", + access_model_names=["claude-opus-4-5"], + assigned_team_ids=["team-b"], + assigned_key_ids=["team-b-token"], + ) + + patches = _patch_proxy_server_globals() + [ + patch( + "litellm.proxy.auth.auth_checks.get_access_object", + new_callable=AsyncMock, + return_value=fake_ag, + ), + ] + for p in patches: + p.start() + try: assert ( await _key_access_group_grants_model( model="claude-opus-4-5", @@ -1287,19 +1417,22 @@ async def test_key_access_group_grants_model_when_group_not_assigned_to_team(): ) is False ) - # Foreign group must be filtered out before expansion ever runs. - mocked_expand.assert_not_called() + finally: + for p in patches: + p.stop() @pytest.mark.asyncio -async def test_key_access_group_grants_model_when_team_has_no_groups(): - """Team with no access_group_ids leaves the intersection empty → denied.""" +async def test_key_access_group_grants_model_when_get_access_object_raises(): + """Group lookup failure (404, network, etc.) is treated as no authorization.""" + from unittest.mock import AsyncMock, patch + from litellm.proxy.auth.auth_checks import _key_access_group_grants_model valid_token = UserAPIKeyAuth( token="test-token", models=[], - access_group_ids=["ryan-access-group"], + access_group_ids=["missing-group"], team_id="team-a", ) team_object = LiteLLM_TeamTable( @@ -1307,12 +1440,26 @@ async def test_key_access_group_grants_model_when_team_has_no_groups(): models=["mock-success"], access_group_ids=[], ) - assert ( - await _key_access_group_grants_model( - model="claude-haiku-4-5", - valid_token=valid_token, - team_object=team_object, - llm_router=None, + + patches = _patch_proxy_server_globals() + [ + patch( + "litellm.proxy.auth.auth_checks.get_access_object", + new_callable=AsyncMock, + side_effect=Exception("not found"), + ), + ] + for p in patches: + p.start() + try: + assert ( + await _key_access_group_grants_model( + model="claude-haiku-4-5", + valid_token=valid_token, + team_object=team_object, + llm_router=None, + ) + is False ) - is False - ) + finally: + for p in patches: + p.stop() From b484c51a1c9548339d77f73ff9e3d99cc9ba63a4 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 1 May 2026 17:48:51 -0700 Subject: [PATCH 19/25] [Fix] Proxy: Repair Merge Fallout In Router-Override Fallback Auth Conflict resolution for #26968 dropped the `Iterator` typing import (NameError at module load), left a dead `fallback_models = cast(...)` block, and the new tests called `_enforce_key_and_fallback_model_access` without the now-required `request` kwarg. --- litellm/proxy/auth/user_api_key_auth.py | 6 +----- .../proxy/auth/test_router_override_fallback_auth.py | 4 ++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/litellm/proxy/auth/user_api_key_auth.py b/litellm/proxy/auth/user_api_key_auth.py index 682d57082d..9159a8ff9d 100644 --- a/litellm/proxy/auth/user_api_key_auth.py +++ b/litellm/proxy/auth/user_api_key_auth.py @@ -11,7 +11,7 @@ import asyncio import re import secrets from datetime import datetime, timezone -from typing import Any, List, Optional, Tuple, Union, cast +from typing import Any, Iterator, List, Optional, Tuple, Union, cast import fastapi from fastapi import HTTPException, Request, WebSocket, status @@ -2271,10 +2271,6 @@ async def _enforce_key_and_fallback_model_access( route=route, request=request, ) - fallback_models = cast( - Optional[List[ALL_FALLBACK_MODEL_VALUES]], - request_data.get("fallbacks", None), - ) if model is not None: await can_key_call_model( diff --git a/tests/test_litellm/proxy/auth/test_router_override_fallback_auth.py b/tests/test_litellm/proxy/auth/test_router_override_fallback_auth.py index 28808ffad8..fc0e9aec50 100644 --- a/tests/test_litellm/proxy/auth/test_router_override_fallback_auth.py +++ b/tests/test_litellm/proxy/auth/test_router_override_fallback_auth.py @@ -94,6 +94,7 @@ async def test_router_override_fallbacks_validated_against_key_allowlist(): valid_token=valid_token, request_data=request_data, route="/v1/chat/completions", + request=None, llm_model_list=None, llm_router=None, ) @@ -144,6 +145,7 @@ async def test_router_override_all_fallback_fields_validated(fallback_field): valid_token=valid_token, request_data=request_data, route="/v1/chat/completions", + request=None, llm_model_list=None, llm_router=None, ) @@ -190,6 +192,7 @@ async def test_top_level_fallback_fields_validated(fallback_field): valid_token=valid_token, request_data=request_data, route="/v1/chat/completions", + request=None, llm_model_list=None, llm_router=None, ) @@ -226,6 +229,7 @@ async def test_router_override_without_fallbacks_does_not_break_auth(): valid_token=valid_token, request_data=request_data, route="/v1/chat/completions", + request=None, llm_model_list=None, llm_router=None, ) From 10659e725f3c31820ab135020722d03ac29cec1c Mon Sep 17 00:00:00 2001 From: Michael Riad Zaky Date: Fri, 1 May 2026 17:50:39 -0700 Subject: [PATCH 20/25] isolate dual OTEL handlers --- litellm/integrations/opentelemetry.py | 53 +++-- .../integrations/test_opentelemetry.py | 183 ++++++++++++++++++ 2 files changed, 218 insertions(+), 18 deletions(-) diff --git a/litellm/integrations/opentelemetry.py b/litellm/integrations/opentelemetry.py index b6d91d0b76..77833e5de0 100644 --- a/litellm/integrations/opentelemetry.py +++ b/litellm/integrations/opentelemetry.py @@ -69,6 +69,8 @@ class OpenTelemetryConfig: deployment_environment: Optional[str] = None model_id: Optional[str] = None ignore_context_propagation: Optional[bool] = None + # When True, create a private TracerProvider instead of reusing or setting the global one. + skip_set_global: bool = False def __post_init__(self) -> None: # If endpoint is specified but exporter is still the default "console", @@ -259,16 +261,21 @@ class OpenTelemetry(CustomLogger): try: existing_provider = get_existing_provider_fn() - # If a real SDK provider exists (set by another SDK like Langfuse), use it - # This uses a positive check for SDK providers instead of a negative check for proxy providers if isinstance(existing_provider, sdk_provider_class): - verbose_logger.debug( - "OpenTelemetry: Using existing %s: %s", - provider_name, - type(existing_provider).__name__, - ) - provider = existing_provider - # Don't call set_provider to preserve existing context + if skip_set_global: + verbose_logger.debug( + "OpenTelemetry: existing %s found but skip_set_global=True; creating private %s for isolation", + provider_name, + provider_name, + ) + provider = create_new_provider_fn() + else: + verbose_logger.debug( + "OpenTelemetry: Using existing %s: %s", + provider_name, + type(existing_provider).__name__, + ) + provider = existing_provider else: # Default proxy provider or unknown type, create our own verbose_logger.debug("OpenTelemetry: Creating new %s", provider_name) @@ -293,6 +300,12 @@ class OpenTelemetry(CustomLogger): return provider + def _skip_set_global(self) -> bool: + # langfuse_otel relies on the Langfuse SDK's providers; don't overwrite them. + return self.config.skip_set_global or ( + hasattr(self, "callback_name") and self.callback_name == "langfuse_otel" + ) + def _init_tracing(self, tracer_provider): from opentelemetry import trace from opentelemetry.sdk.trace import TracerProvider @@ -303,11 +316,6 @@ class OpenTelemetry(CustomLogger): provider.add_span_processor(self._get_span_processor()) return provider - # CRITICAL FIX: For Langfuse OTEL, skip setting global provider to prevent interference - skip_global = ( - hasattr(self, "callback_name") and self.callback_name == "langfuse_otel" - ) - tracer_provider = self._get_or_create_provider( provider=tracer_provider, provider_name="TracerProvider", @@ -315,16 +323,18 @@ class OpenTelemetry(CustomLogger): sdk_provider_class=TracerProvider, create_new_provider_fn=create_tracer_provider, set_provider_fn=trace.set_tracer_provider, - skip_set_global=skip_global, + skip_set_global=self._skip_set_global(), ) # Grab our tracer from the TracerProvider (not from global context) # This ensures we use the provided TracerProvider (e.g., for testing) self.tracer = tracer_provider.get_tracer(LITELLM_TRACER_NAME) + self._tracer_provider = tracer_provider self.span_kind = SpanKind def _init_metrics(self, meter_provider): if not self.config.enable_metrics: + self._meter_provider = None self._operation_duration_histogram = None self._token_usage_histogram = None self._cost_histogram = None @@ -350,7 +360,9 @@ class OpenTelemetry(CustomLogger): sdk_provider_class=MeterProvider, create_new_provider_fn=create_meter_provider, set_provider_fn=metrics.set_meter_provider, + skip_set_global=self._skip_set_global(), ) + self._meter_provider = meter_provider meter = meter_provider.get_meter(__name__) @@ -388,6 +400,7 @@ class OpenTelemetry(CustomLogger): def _init_logs(self, logger_provider): # nothing to do if events disabled if not self.config.enable_events: + self._logger_provider = None return from opentelemetry._logs import get_logger_provider, set_logger_provider @@ -404,13 +417,14 @@ class OpenTelemetry(CustomLogger): ) return provider - self._get_or_create_provider( + self._logger_provider = self._get_or_create_provider( provider=logger_provider, provider_name="LoggerProvider", get_existing_provider_fn=get_logger_provider, sdk_provider_class=OTLoggerProvider, create_new_provider_fn=create_logger_provider, set_provider_fn=set_logger_provider, + skip_set_global=self._skip_set_global(), ) def log_success_event(self, kwargs, response_obj, start_time, end_time): @@ -1073,7 +1087,7 @@ class OpenTelemetry(CustomLogger): # See: https://github.com/open-telemetry/opentelemetry-python/pull/4676 # TODO: Refactor to use the proper OTEL Logs API instead of directly creating SDK LogRecords - from opentelemetry._logs import SeverityNumber, get_logger + from opentelemetry._logs import SeverityNumber try: from opentelemetry.sdk._logs import ( # type: ignore[attr-defined] # OTEL < 1.39.0 @@ -1084,7 +1098,10 @@ class OpenTelemetry(CustomLogger): LogRecord as SdkLogRecord, # type: ignore[attr-defined] # OTEL >= 1.39.0 ) - otel_logger = get_logger(LITELLM_LOGGER_NAME) + # Resolve through the handler's own LoggerProvider (which may be a + # private one when skip_set_global=True) rather than the module-level + # get_logger() which always goes through the global provider. + otel_logger = self._logger_provider.get_logger(LITELLM_LOGGER_NAME) parent_ctx = span.get_span_context() provider = (kwargs.get("litellm_params") or {}).get( diff --git a/tests/test_litellm/integrations/test_opentelemetry.py b/tests/test_litellm/integrations/test_opentelemetry.py index b31bbca889..962806c5b5 100644 --- a/tests/test_litellm/integrations/test_opentelemetry.py +++ b/tests/test_litellm/integrations/test_opentelemetry.py @@ -259,6 +259,189 @@ class TestOpenTelemetryProviderInitialization(unittest.TestCase): ), "Existing LoggerProvider should be respected and not overridden" +class TestOpenTelemetryDualHandlerIsolation(unittest.TestCase): + """Two OpenTelemetry handlers coexisting via skip_set_global=True + must each get their own provider for every signal (tracer/meter/logger).""" + + @staticmethod + def _wire_span_processor(exporter): + """Context manager: while active, the next OpenTelemetry instance + wires its TracerProvider to `exporter`.""" + return patch.object( + OpenTelemetry, + "_get_span_processor", + lambda self, dynamic_headers=None: SimpleSpanProcessor(exporter), + ) + + def test_skip_set_global_creates_isolated_tracer_provider(self): + from opentelemetry.sdk.trace import TracerProvider as SDKTracerProvider + + fake_existing = SDKTracerProvider() + own_exporter = InMemorySpanExporter() + cfg = OpenTelemetryConfig( + exporter="console", service_name="iso-test", skip_set_global=True + ) + with ( + patch.object(trace, "get_tracer_provider", return_value=fake_existing), + patch.object(trace, "set_tracer_provider") as mock_set, + self._wire_span_processor(own_exporter), + ): + handler = OpenTelemetry(config=cfg) + + self.assertIsNot(handler._tracer_provider, fake_existing) + mock_set.assert_not_called() + + handler.tracer.start_span("isolation_check").end() + handler._tracer_provider.force_flush(2000) + self.assertEqual( + [s.name for s in own_exporter.get_finished_spans()], + ["isolation_check"], + ) + + def test_skip_set_global_via_callback_name_back_compat(self): + from opentelemetry.sdk.trace import TracerProvider as SDKTracerProvider + + fake_existing = SDKTracerProvider() + cfg = OpenTelemetryConfig(exporter="console", service_name="lf-back-compat") + with ( + patch.object(trace, "get_tracer_provider", return_value=fake_existing), + patch.object(trace, "set_tracer_provider"), + self._wire_span_processor(InMemorySpanExporter()), + ): + handler = OpenTelemetry(config=cfg, callback_name="langfuse_otel") + + self.assertIsNot(handler._tracer_provider, fake_existing) + + def test_default_behavior_reuses_existing_sdk_tracer_provider(self): + from opentelemetry.sdk.trace import TracerProvider as SDKTracerProvider + + fake_existing = SDKTracerProvider() + with patch.object(trace, "get_tracer_provider", return_value=fake_existing): + handler = OpenTelemetry(config=OpenTelemetryConfig(service_name="shared")) + self.assertIs(handler._tracer_provider, fake_existing) + + def test_skip_set_global_creates_isolated_meter_provider(self): + from opentelemetry import metrics + from opentelemetry.sdk.metrics import MeterProvider as SDKMeterProvider + + fake_existing = SDKMeterProvider() + cfg = OpenTelemetryConfig( + exporter="console", + service_name="meter-iso-test", + enable_metrics=True, + skip_set_global=True, + ) + with ( + patch.object(metrics, "get_meter_provider", return_value=fake_existing), + patch.object(metrics, "set_meter_provider") as mock_set, + self._wire_span_processor(InMemorySpanExporter()), + ): + handler = OpenTelemetry(config=cfg) + + self.assertIsNot(handler._meter_provider, fake_existing) + mock_set.assert_not_called() + + def test_skip_set_global_creates_isolated_logger_provider(self): + from opentelemetry import _logs + from opentelemetry.sdk._logs import LoggerProvider as SDKLoggerProvider + + fake_existing = SDKLoggerProvider() + cfg = OpenTelemetryConfig( + exporter="console", + service_name="logger-iso-test", + enable_events=True, + skip_set_global=True, + ) + with ( + patch.object(_logs, "get_logger_provider", return_value=fake_existing), + patch.object(_logs, "set_logger_provider") as mock_set, + self._wire_span_processor(InMemorySpanExporter()), + ): + handler = OpenTelemetry(config=cfg) + + self.assertIsNot(handler._logger_provider, fake_existing) + mock_set.assert_not_called() + + def test_emitted_logs_route_to_isolated_logger_provider(self): + # End-to-end: emitted logs land in the handler's private LoggerProvider, + # not the global one. Guards against get_logger() bypassing self._logger_provider. + from opentelemetry import _logs + from opentelemetry.sdk._logs import LoggerProvider as SDKLoggerProvider + + global_exporter = InMemoryLogExporter() + fake_existing = SDKLoggerProvider() + fake_existing.add_log_record_processor( + SimpleLogRecordProcessor(global_exporter) + ) + + private_exporter = InMemoryLogExporter() + cfg = OpenTelemetryConfig( + exporter="console", + service_name="logger-emit-test", + enable_events=True, + skip_set_global=True, + ) + with ( + patch.object(_logs, "get_logger_provider", return_value=fake_existing), + patch.object(_logs, "set_logger_provider"), + patch.object( + OpenTelemetry, "_get_log_exporter", return_value=private_exporter + ), + self._wire_span_processor(InMemorySpanExporter()), + ): + handler = OpenTelemetry(config=cfg) + + span = handler.tracer.start_span("emit-test") + handler._emit_semantic_logs( + kwargs={"messages": [{"role": "user", "content": "hi"}]}, + response_obj={"choices": []}, + span=span, + ) + span.end() + handler._logger_provider.force_flush(2000) + + self.assertGreater(len(private_exporter.get_finished_logs()), 0) + self.assertEqual(len(global_exporter.get_finished_logs()), 0) + + def test_two_handlers_each_receive_their_own_spans(self): + # Handler A gets explicit injection (production-ish: claims the global). + exporter_a = InMemorySpanExporter() + provider_a = TracerProvider() + provider_a.add_span_processor(SimpleSpanProcessor(exporter_a)) + handler_a = OpenTelemetry( + config=OpenTelemetryConfig(service_name="handler-a"), + tracer_provider=provider_a, + ) + + # Handler B comes along with the global appearing to be A's provider. + exporter_b = InMemorySpanExporter() + cfg_b = OpenTelemetryConfig( + exporter="console", service_name="handler-b", skip_set_global=True + ) + with ( + patch.object(trace, "get_tracer_provider", return_value=provider_a), + patch.object(trace, "set_tracer_provider"), + self._wire_span_processor(exporter_b), + ): + handler_b = OpenTelemetry(config=cfg_b) + + self.assertIsNot(handler_a._tracer_provider, handler_b._tracer_provider) + + handler_a.tracer.start_span("from_handler_a").end() + handler_b.tracer.start_span("from_handler_b").end() + provider_a.force_flush(2000) + handler_b._tracer_provider.force_flush(2000) + + self.assertEqual( + sorted(s.name for s in exporter_a.get_finished_spans()), + ["from_handler_a"], + ) + self.assertEqual( + sorted(s.name for s in exporter_b.get_finished_spans()), + ["from_handler_b"], + ) + + class TestOpenTelemetry(unittest.TestCase): POLL_INTERVAL = 0.05 POLL_TIMEOUT = 2.0 From 92d3bdbb27c034891b657fbc21845f0e1515172d Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 1 May 2026 18:19:24 -0700 Subject: [PATCH 21/25] [Fix] Proxy/Key Management: Align Key-Org Membership Checks On Generate And Regenerate Mirrors the membership rule on /key/update so that /key/generate and /key/{key}/regenerate apply the same `_validate_caller_can_assign_key_org` gate when the caller specifies an `organization_id`. Proxy admins bypass. The check no-ops when `organization_id` is not being set. --- .../key_management_endpoints.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index 7a934787e0..1f66d9fec8 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -809,6 +809,20 @@ async def _common_key_generation_helper( # noqa: PLR0915 from litellm.proxy.proxy_server import prisma_client, user_api_key_cache if prisma_client: + # Mirror the membership rule applied to /key/update: when the + # caller specifies an organization_id, require that they are a + # member of (or proxy admin over) the target organization. + _is_proxy_admin = ( + user_api_key_dict.user_role is not None + and user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value + ) + if not _is_proxy_admin: + await _validate_caller_can_assign_key_org( + user_api_key_dict=user_api_key_dict, + organization_id=data.organization_id, + prisma_client=prisma_client, + ) + org_table = await get_org_object( org_id=data.organization_id, user_api_key_cache=user_api_key_cache, @@ -3920,6 +3934,22 @@ async def _execute_virtual_key_regeneration( """Generate new token, update DB, invalidate cache, and return response.""" from litellm.proxy.proxy_server import hash_token + # Apply the same membership rule used on /key/update: when the caller + # asks to point the regenerated key at a different organization_id, + # require they are a member of (or proxy admin over) the target org. + if data is not None and data.organization_id is not None: + _existing_org_id = getattr(key_in_db, "organization_id", None) + _is_proxy_admin = ( + user_api_key_dict.user_role is not None + and user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value + ) + if data.organization_id != _existing_org_id and not _is_proxy_admin: + await _validate_caller_can_assign_key_org( + user_api_key_dict=user_api_key_dict, + organization_id=data.organization_id, + prisma_client=prisma_client, + ) + new_token = await get_new_token(data=data) new_token_hash = hash_token(new_token) new_token_key_name = f"sk-...{new_token[-4:]}" From e3917c9d08bc40bf42d687078a54ab8d49981d84 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 1 May 2026 19:10:27 -0700 Subject: [PATCH 22/25] [Test] Anthropic: Replace Legacy Claude-4-Sonnet Alias With Haiku 4.5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three live-API tests pinned to claude-4-sonnet-20250514, which is a non-canonical alias of claude-sonnet-4-20250514. Anthropic's main API no longer resolves the legacy form under freshly issued keys, so the tests fail with not_found_error. The token counter test pinned to claude-sonnet-4-20250514 itself (deprecation_date 2026-05-14, two weeks out) was on borrowed time too. Bump all four to claude-haiku-4-5-20251001 — capability superset for what these tests exercise (streaming, parallel tool calling, extended thinking, token counting), no upcoming deprecation, cheaper per-token. --- tests/litellm_utils_tests/test_anthropic_token_counter.py | 2 +- tests/local_testing/test_function_calling.py | 2 +- tests/local_testing/test_streaming.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/litellm_utils_tests/test_anthropic_token_counter.py b/tests/litellm_utils_tests/test_anthropic_token_counter.py index d099eb4f8e..028586203a 100644 --- a/tests/litellm_utils_tests/test_anthropic_token_counter.py +++ b/tests/litellm_utils_tests/test_anthropic_token_counter.py @@ -26,7 +26,7 @@ class TestAnthropicTokenCounter(BaseTokenCounterTest): return AnthropicTokenCounter() def get_test_model(self) -> str: - return "claude-sonnet-4-20250514" + return "claude-haiku-4-5-20251001" def get_test_messages(self) -> List[Dict[str, Any]]: return [{"role": "user", "content": "Hello, how are you today?"}] diff --git a/tests/local_testing/test_function_calling.py b/tests/local_testing/test_function_calling.py index 02affa1d57..6fd253ee29 100644 --- a/tests/local_testing/test_function_calling.py +++ b/tests/local_testing/test_function_calling.py @@ -158,7 +158,7 @@ def test_aaparallel_function_call(model): @pytest.mark.parametrize( "model", [ - "anthropic/claude-4-sonnet-20250514", + "anthropic/claude-haiku-4-5-20251001", "bedrock/us.anthropic.claude-sonnet-4-5-20250929-v1:0", ], ) diff --git a/tests/local_testing/test_streaming.py b/tests/local_testing/test_streaming.py index bc3c34b1a5..7fe42ed546 100644 --- a/tests/local_testing/test_streaming.py +++ b/tests/local_testing/test_streaming.py @@ -1270,7 +1270,7 @@ def test_bedrock_claude_3_streaming(): @pytest.mark.parametrize( "model", [ - "claude-4-sonnet-20250514", + "claude-haiku-4-5-20251001", "cohere.command-r-plus-v1:0", # bedrock "gpt-3.5-turbo", ], @@ -2696,7 +2696,7 @@ def test_completion_claude_3_function_call_with_streaming(): try: # test without max tokens response = completion( - model="claude-4-sonnet-20250514", + model="claude-haiku-4-5-20251001", messages=messages, tools=tools, tool_choice="required", From 95ccfee7ca9b320a70759ac5de4f5b92f2af2fba Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 1 May 2026 19:22:30 -0700 Subject: [PATCH 23/25] [Chore] Proxy/UI: Drop stray _experimental/out/chat/index.html This file is a regenerable UI build artifact that should not be tracked in source. Removing so the merge into litellm_internal_staging stays clean. --- litellm/proxy/_experimental/out/chat/index.html | 1 - 1 file changed, 1 deletion(-) delete mode 100644 litellm/proxy/_experimental/out/chat/index.html diff --git a/litellm/proxy/_experimental/out/chat/index.html b/litellm/proxy/_experimental/out/chat/index.html deleted file mode 100644 index dc68814825..0000000000 --- a/litellm/proxy/_experimental/out/chat/index.html +++ /dev/null @@ -1 +0,0 @@ -LiteLLM Dashboard \ No newline at end of file From 1e63be7a72518a7a670ea5648b303da8b65d1106 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 1 May 2026 19:22:39 -0700 Subject: [PATCH 24/25] [Test] Anthropic Passthrough: Bump Streaming Cost-Injection Test To Haiku 4.5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_anthropic_messages_streaming_cost_injection hits the proxy's /v1/messages route, which routes via the anthropic/* wildcard to api.anthropic.com. The 404 surfaced in the test was Anthropic's own not_found_error propagated back through the proxy (visible from the x-litellm-model-id hash on the response — the proxy did route). Same root cause as the prior commit: the legacy claude-4-sonnet-20250514 alias is no longer recognized by Anthropic's main API under the new key. Swap to claude-haiku-4-5-20251001 — same routing path, canonical model. --- tests/pass_through_tests/test_anthropic_passthrough.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pass_through_tests/test_anthropic_passthrough.py b/tests/pass_through_tests/test_anthropic_passthrough.py index c4ae00768c..d42e06937d 100644 --- a/tests/pass_through_tests/test_anthropic_passthrough.py +++ b/tests/pass_through_tests/test_anthropic_passthrough.py @@ -333,7 +333,7 @@ async def test_anthropic_messages_streaming_cost_injection(): } payload = { - "model": "claude-4-sonnet-20250514", + "model": "claude-haiku-4-5-20251001", "max_tokens": 10, "stream": True, "messages": [{"role": "user", "content": "Say 'Hi'"}], From abfaab5dc348b957c5ff0f5938e1effaf2abc8ca Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 1 May 2026 19:33:08 -0700 Subject: [PATCH 25/25] [Test] Anthropic Passthrough: Bump Thinking Tests Off Legacy Sonnet 4 Alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit base_anthropic_messages_test.test_anthropic_messages_with_thinking and test_anthropic_streaming_with_thinking still pinned to claude-4-sonnet-20250514 — the same legacy alias Anthropic no longer recognizes under freshly issued keys. The other four tests in this base class already use claude-sonnet-4-5-20250929; these two were missed. Bump to claude-haiku-4-5-20251001 (supports_reasoning=true, no upcoming deprecation). Subclasses including TestAnthropicPassthroughBasic inherit these methods. --- tests/pass_through_tests/base_anthropic_messages_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/pass_through_tests/base_anthropic_messages_test.py b/tests/pass_through_tests/base_anthropic_messages_test.py index e86e58de33..95f709e388 100644 --- a/tests/pass_through_tests/base_anthropic_messages_test.py +++ b/tests/pass_through_tests/base_anthropic_messages_test.py @@ -54,7 +54,7 @@ class BaseAnthropicMessagesTest(ABC): print("making request to anthropic passthrough with thinking") client = self.get_client() response = client.messages.create( - model="claude-4-sonnet-20250514", + model="claude-haiku-4-5-20251001", max_tokens=20000, thinking={"type": "enabled", "budget_tokens": 16000}, messages=[ @@ -75,7 +75,7 @@ class BaseAnthropicMessagesTest(ABC): collected_response = [] client = self.get_client() with client.messages.stream( - model="claude-4-sonnet-20250514", + model="claude-haiku-4-5-20251001", max_tokens=20000, thinking={"type": "enabled", "budget_tokens": 16000}, messages=[