Skip to content

chore(ci): merge dev branch#28801

Merged
yuneng-berri merged 6 commits into
litellm_internal_stagingfrom
litellm_yj_may25
May 25, 2026
Merged

chore(ci): merge dev branch#28801
yuneng-berri merged 6 commits into
litellm_internal_stagingfrom
litellm_yj_may25

Conversation

@yuneng-berri

Copy link
Copy Markdown
Collaborator

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Screenshots / Proof of Fix

Type

🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test

Changes

shin-berri and others added 5 commits May 13, 2026 22:37
[Infra] Promote internal staging to main
[Infra] Promote internal staging to main
Replace direct ``request.url.path`` reads in auth, ACL, routing, and
audit-log decisions with ``get_request_route(request)`` — the helper
already added in ``auth/auth_utils.py`` that returns the ASGI
``scope["path"]`` with ``root_path`` stripped. Starlette reconstructs
``url.path`` from the Host header; ``scope["path"]`` is uvicorn's
parse of the request line and matches what FastAPI dispatches on, so
it's the authoritative route for any decision that should agree with
the actual handler.

Sites:
- _experimental/mcp_server/auth/user_api_key_auth_mcp.py
- management_endpoints/mcp_management_endpoints.py
- vector_store_endpoints/utils.py
- pass_through_endpoints/pass_through_endpoints.py
- auth/route_checks.py
- litellm_pre_call_utils.py
- spend_tracking/spend_management_endpoints.py
- common_utils/http_parsing_utils.py
- management_helpers/utils.py
- health_endpoints/_health_endpoints.py

Adds regression tests in tests/proxy_unit_tests/test_proxy_routes.py
that construct a Request with scope["path"] set to a benign route and
the Host header crafted so url.path would resolve differently; each
site's decision is asserted against scope["path"].
Move the ``from litellm.proxy.auth.auth_utils import get_request_route``
imports added in the prior commit back to the function bodies that use
them. The module-level form participates in a long-standing import
cycle through ``auth_utils -> _types -> ...`` and was flagged by CodeQL
on the PR; the lazy form matches the pattern the proxy already uses
for ``user_api_key_auth`` and related helpers elsewhere in these files.

Also drop the ``RouteChecks._is_assistants_api_request`` delegation in
``_get_metadata_variable_name`` introduced in the prior commit — the
delegation pulled ``RouteChecks`` into the same cycle, and the call
site reuses the resolved route for its other branches, so inlining
the substring check is both cycle-free and avoids a redundant second
``get_request_route`` call.

Comment in test_proxy_routes.py acknowledges that the two MCP table
entries exercise ``get_request_route`` directly rather than the full
production handler (which needs ASGI scope + MCP state to invoke).
chore(proxy): route path-dependent call sites through get_request_route
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ yuneng-berri
✅ stuxf
❌ shin-berri
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov

codecov Bot commented May 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...itellm/proxy/health_endpoints/_health_endpoints.py 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hardens a Host-header injection vulnerability where a malformed Host (e.g. localhost/?x=1) causes Starlette to reconstruct request.url.path as "/" while FastAPI dispatches on the real scope["path"]. All call sites that previously read request.url.path for auth, ACL, routing, or audit decisions are migrated to get_request_route(request), which reads the authoritative scope["path"] directly from the ASGI scope.

  • Security hardening: 10 call sites across auth, MCP, vector-store, spend-tracking, pass-through, health, and pre-call utils now use scope["path"] to prevent bypass via malformed Host headers.
  • Test coverage: A new parameterised suite (test_call_site_uses_scope_path) drives all migrated call sites through four crafted-host patterns to prove each one is no longer susceptible.

Confidence Score: 5/5

Safe to merge — mechanical substitution of request.url.path to get_request_route across all affected call sites with no auth logic restructuring.

Every changed call site performs a direct substitution and the helper behavior has been in production. No new code paths introduced; regression suite covers all migrated sites with four malformed-host patterns.

No files require special attention.

Important Files Changed

Filename Overview
litellm/proxy/auth/auth_utils.py Docstring updated to document the Host-header bypass risk; function body unchanged and correct.
litellm/proxy/auth/route_checks.py _is_assistants_api_request migrated from request.url.path to get_request_route; logic is unchanged, bypass is closed.
litellm/proxy/vector_store_endpoints/utils.py Four request.url.path usages in permission-type classifiers replaced with get_request_route; auth bypass closed.
litellm/proxy/pass_through_endpoints/pass_through_endpoints.py create_pass_through_route inner function now uses get_request_route to feed is_registered_pass_through_route, closing the bypass.
litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py Three request.url.path usages (well-known bypass, upstream-delegated auth, OAuth2 check) replaced; security path tightened.
litellm/proxy/management_endpoints/mcp_management_endpoints.py PKCE /token path check migrated to get_request_route; prevents Host-crafted path from bypassing grant-type validation.
litellm/proxy/common_utils/http_parsing_utils.py _add_vector_store_id_from_path migrated to get_request_route; prevents spurious vector_store_id extraction from injected host paths.
litellm/proxy/litellm_pre_call_utils.py _get_metadata_variable_name migrated; metadata key selection now uses authoritative scope path.
litellm/proxy/management_helpers/utils.py OTel span route attribution migrated to get_request_route; telemetry now records the real request route.
litellm/proxy/spend_tracking/spend_management_endpoints.py v2 spend-log path detection migrated; no longer susceptible to Host-injected /spend/logs/v2 substring.
litellm/proxy/health_endpoints/_health_endpoints.py /test health endpoint now echoes get_request_route(request) instead of url.path; consistent with the fix.
tests/proxy_unit_tests/test_proxy_routes.py Adds parameterised regression suite covering 7 call sites x 4 malformed-host patterns; test logic mirrors production predicates correctly.

Reviews (2): Last reviewed commit: "Merge branch 'litellm_internal_staging' ..." | Re-trigger Greptile

Comment on lines +268 to +336


# (label, scope_path, host_suffix_template, predicate, expected) — host_suffix_template
# receives the host_header via %s substitution. The predicate is invoked on a Request
# whose scope["path"] is scope_path and whose Host header is the formatted suffix.
#
# The MCP entries (well_known_mcp_bypass, pkce_token_suffix) call
# get_request_route directly rather than the surrounding production handler
# (MCPRequestHandler.process_mcp_request / _mcp_oauth_user_api_key_auth) —
# those handlers require an ASGI scope plus MCP state to invoke, and the call
# sites do nothing with the path except feed it to this helper. The helper-
# level assertion is the relevant signal.
_CALL_SITES = [
("assistants_classification", "/key/generate", "%s/thread", _is_assistants, False),
(
"metadata_variable_name",
"/chat/completions",
"%s/thread",
_metadata_var_name,
"metadata",
),
(
"vector_store_id_extraction",
"/key/generate",
"%s/vector_stores/x/files",
_vector_store_id_in_path,
False,
),
(
"well_known_mcp_bypass",
"/mcp/tools/call",
"/.well-known/%s",
lambda r: get_request_route(r).startswith("/.well-known/"),
False,
),
(
"pkce_token_suffix",
"/mcp/server-id/token",
"%s",
lambda r: get_request_route(r).rstrip("/").lower().endswith("/token"),
True,
),
(
"spend_logs_v2_classification",
"/spend/logs",
"%s/spend/logs/v2",
lambda r: "/spend/logs/v2" in get_request_route(r),
False,
),
("health_route_echo", "/test", "%s", lambda r: get_request_route(r), "/test"),
]


@pytest.mark.parametrize("host_header", _BYPASS_HOSTS)
@pytest.mark.parametrize(
"label,scope_path,host_suffix_template,predicate,expected",
_CALL_SITES,
ids=[c[0] for c in _CALL_SITES],
)
def test_call_site_uses_scope_path(
label, scope_path, host_suffix_template, predicate, expected, host_header
):
"""Each call site that previously read request.url.path must now make its
decision against scope["path"]. The Host header is crafted so url.path
would resolve to a value that flips the decision under the old code."""
request = _create_request_with_host_header(
path=scope_path, host_header=host_suffix_template % host_header
)
assert predicate(request) == expected

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing regression coverage for vector-store permission and pass-through route checks

_CALL_SITES covers _add_vector_store_id_from_path (path extraction) but not the two security-adjacent permission functions in vector_store_endpoints/utils.pyis_allowed_to_call_vector_store_endpoint and is_allowed_to_call_vector_store_files_endpoint — where the fixed path drives read/write permission classification. Similarly, the pass_through_endpoints.py is_registered_pass_through_route check is absent from the matrix.

A malformed Host that collapses url.path to / would have caused the endpoint loop to skip every entry (no match → permission_type = Nonereturn None, i.e. no permission check at all) while the real scope path would have matched a write endpoint. Both call sites were fixed but neither has a Host-injection regression test here.

@yuneng-berri

Copy link
Copy Markdown
Collaborator Author

@greptile

@yuneng-berri yuneng-berri enabled auto-merge (squash) May 25, 2026 20:43
@yuneng-berri yuneng-berri merged commit 5f75be5 into litellm_internal_staging May 25, 2026
114 of 116 checks passed
Sameerlite added a commit that referenced this pull request May 26, 2026