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] 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 0f7cfa42229..1ba1b100a86 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 e51882a3b16..e73ac571d59 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