chore(auth): require trusted proxy for header identity auth - #26825
Conversation
…ld denylist ``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) <noreply@anthropic.com>
Greptile SummaryThis PR hardens header-derived identity auth by adding a CIDR-based trusted proxy gate (
Confidence Score: 3/5Do not merge until the IPv4-mapped IPv6 TypeError in _is_ip_in_networks is fixed; dual-stack deployments will receive 500 errors on every legitimate proxy auth request. One P1 logic bug in the new core enforcement utility (TypeError escaping the except block for IPv4-mapped IPv6 addresses) pulls the score below the P1 ceiling of 4. The security intent and allowlist/gate design are correct, but the bug would break dual-stack deployments reliably. litellm/proxy/auth/trusted_proxy_utils.py — specifically _is_ip_in_networks (lines 68–77) needs a TypeError catch or IPv4-mapped address unwrapping before the addr-in-network comparison.
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/trusted_proxy_utils.py | New shared utility for CIDR-based trusted proxy validation; contains a P1 bug where comparing IPv4-mapped IPv6 addresses against IPv4 networks raises an unhandled TypeError, and re-parses CIDR ranges on every auth request. |
| litellm/proxy/auth/oauth2_proxy_hook.py | Adds trusted-proxy gate and identity-only allowlist (ALLOWED_OAUTH2_PROXY_FIELDS) before consuming mapped identity headers; dead max_budget branch cleaned up; logic is sound. |
| enterprise/litellm_enterprise/proxy/auth/custom_sso_handler.py | Adds trusted-proxy validation before invoking the custom UI SSO handler; OpenID import moved inside method; logic is correct. |
| litellm/integrations/custom_sso_handler.py | Base CustomSSOLoginHandler now validates the trusted proxy before reading x-litellm-user-* headers; creates a redundant double-check when called through EnterpriseCustomSSOHandler. |
| litellm/proxy/_types.py | Adds trusted_proxy_ranges field to ConfigGeneralSettings; clean schema-only addition. |
| tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.py | New test file covering trusted-proxy rejection, identity allowlist enforcement, and the primary privesc attack path; all mock-only, no real network calls. |
| tests/test_litellm/proxy/management_endpoints/test_ui_sso.py | Existing tests updated to configure trusted_proxy_ranges and client.host; new test verifies untrusted direct client is rejected before the custom handler is called. |
Reviews (3): Last reviewed commit: "chore(auth): require trusted proxy for h..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…RY tests
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) <noreply@anthropic.com>
…allowlist 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) <noreply@anthropic.com>
Identity-only allowlist and trusted-proxy CIDR gate for header-based authThis PR adds an allowlist restricting mappable fields to identity-only assertions and a CIDR-based trusted-proxy check that validates the direct TCP peer before trusting any identity headers. Without these controls, Status: 0 open |
…security fix
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) <noreply@anthropic.com>
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 fbcfd59. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review |
|
@greptileai review |
| 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) |
There was a problem hiding this comment.
Unhandled
TypeError for IPv4-mapped IPv6 addresses
Python's ipaddress module raises TypeError (not ValueError) when you check IPv6Address in IPv4Network or vice versa. On dual-stack deployments where uvicorn binds to :: (IPv6), the peer address for an IPv4 connection is often reported as ::ffff:127.0.0.1. If trusted_proxy_ranges is configured with IPv4 CIDRs like 127.0.0.1/32, the addr in network comparison raises TypeError, which escapes the try/except ValueError block and propagates as an unhandled exception — resulting in a 500 for every legitimate proxy request on such deployments.
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
try:
return any(addr in network for network in networks)
except TypeError:
# IPv4-mapped IPv6 (e.g. ::ffff:127.0.0.1) vs. IPv4Network —
# unwrap and retry against IPv4 networks when possible
if getattr(addr, "ipv4_mapped", None) is not None:
mapped = addr.ipv4_mapped
return any(mapped in n for n in networks if n.version == 4)
return False5614469
into
BerriAI:litellm_internal_staging
…orgery chore(auth): require trusted proxy for header identity auth
Relevant issues
Pre-Submission checklist
tests/test_litellm/directory.make test-unit.@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewType
🐛 Bug Fix
Changes
This PR hardens header-derived identity auth. These flows are intended for deployments where a trusted reverse proxy authenticates the user and injects identity headers before requests reach LiteLLM. Previously, LiteLLM trusted those headers without verifying that the direct client was actually the trusted proxy.
general_settings.trusted_proxy_ranges, a CIDR allowlist for reverse proxies allowed to supply identity headers. Header-based auth now fails closed unless the direct TCP peer is inside one of those ranges. This check intentionally uses the direct client IP, notX-Forwarded-For, because the direct peer is the actor supplying the identity headers.handle_oauth2_proxy_requeststill rejects anyoauth2_config_mappingsfield outsideALLOWED_OAUTH2_PROXY_FIELDS = {user_id, user_email, team_id, team_alias, org_id, models}. This prevents forged headers from settinguser_role, budgets, limits, permissions, metadata, or future privileged fields.CustomSSOLoginHandleralso validates before readingx-litellm-user-*headers.Files
litellm/proxy/auth/trusted_proxy_utils.py— shared trusted-proxy CIDR parsing and fail-closed request validation.litellm/proxy/auth/oauth2_proxy_hook.py— validates trusted proxy before consuming mapped identity headers.enterprise/litellm_enterprise/proxy/auth/custom_sso_handler.pyandlitellm/integrations/custom_sso_handler.py— validate trusted proxy before custom UI SSO header consumption.litellm/proxy/_types.py— documentsgeneral_settings.trusted_proxy_ranges.tests/test_litellm/proxy/auth/test_oauth2_proxy_hook.pycovers missing/untrusted/trusted proxy cases plus identity-only mapping;tests/test_litellm/proxy/management_endpoints/test_ui_sso.pycovers custom UI SSO rejecting untrusted direct clients before custom handler execution.Behaviour notes for operators
enable_oauth2_proxy_authmust setgeneral_settings.trusted_proxy_rangesto the CIDR range of the reverse proxy that injects the configured identity headers.custom_ui_sso_sign_in_handlermust do the same before the custom handler can consumex-litellm-*or other identity headers.127.0.0.1/32or::1/128as appropriate.