chore(ci): merge dev branch (#28801)

* chore(proxy): route path-dependent call sites through get_request_route

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"].

* chore(proxy): make get_request_route imports lazy at call sites

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).

---------

Co-authored-by: shin-berri <shin-laptop@berri.ai>
Co-authored-by: user <70670632+stuxf@users.noreply.github.com>
This commit is contained in:
yuneng-jiang 2026-05-25 13:44:49 -07:00 committed by GitHub
parent 30551de371
commit 5f75be5c1c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 184 additions and 24 deletions

View File

@ -118,15 +118,19 @@ class MCPRequestHandler:
return b"{}"
request.body = mock_body # type: ignore
# Inline import — auth_utils participates in a proxy import cycle.
from litellm.proxy.auth.auth_utils import ( # noqa: PLC0415
get_request_route,
)
request_route = get_request_route(request)
# Only OAuth metadata routes registered under /.well-known/ are public.
# Match on request.url.path (path-only, exact prefix) so the substring
# cannot be smuggled via query string, hostname, or a deeper URL segment.
if request.url.path.startswith("/.well-known/"):
if request_route.startswith("/.well-known/"):
validated_user_api_key_auth = UserAPIKeyAuth()
elif (
not litellm_api_key
and MCPRequestHandler._target_servers_delegate_auth_to_upstream( # noqa: E501
path=request.url.path, mcp_servers=mcp_servers
path=request_route, mcp_servers=mcp_servers
)
):
# Operator opted this oauth2 server into upstream-delegated auth
@ -174,7 +178,7 @@ class MCPRequestHandler:
"401",
"403",
) and MCPRequestHandler._target_servers_use_oauth2(
path=request.url.path, mcp_servers=mcp_servers
path=request_route, mcp_servers=mcp_servers
):
verbose_logger.debug(
"MCP OAuth2: target server is OAuth2-mode, treating "

View File

@ -498,9 +498,18 @@ def route_in_additonal_public_routes(current_route: str):
def get_request_route(request: Request) -> str:
"""
Helper to get the route from the request
Resolve the request route from the ASGI scope, with ``root_path`` stripped.
remove base url from path if set e.g. `/genai/chat/completions` -> `/chat/completions
Prefer this over ``request.url.path`` for any auth, ACL, routing, or
audit-log decision: Starlette reconstructs ``url.path`` by interpolating
the Host header into a URL string and re-parsing with ``urlsplit``, so a
malformed Host (e.g. ``localhost/?x=1``) collapses ``url.path`` to ``"/"``
while FastAPI continues to dispatch on ``scope["path"]``. ``scope["path"]``
is uvicorn's parse of the HTTP request line and matches the actual
handler, so it's the authoritative route.
Also normalizes sub-path deployments by stripping ``scope["root_path"]``
e.g. ``/genai/chat/completions`` -> ``/chat/completions``.
"""
try:
scope = request.scope

View File

@ -627,7 +627,11 @@ class RouteChecks:
Returns:
bool: True if `thread` or `assistant` is in the request path, False otherwise
"""
if "thread" in request.url.path or "assistant" in request.url.path:
# Inline import — auth_utils participates in a proxy import cycle.
from .auth_utils import get_request_route # noqa: PLC0415
route = get_request_route(request)
if "thread" in route or "assistant" in route:
return True
return False

View File

@ -546,7 +546,10 @@ def _add_vector_store_id_from_path(request_data: dict, request: Request) -> None
request_data: The request data dictionary to populate
request: The FastAPI Request object
"""
path = request.url.path
# Inline import — auth_utils participates in a proxy import cycle.
from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415
path = get_request_route(request)
vector_store_match = re.search(r"/vector_stores/([^/]+)/", path)
if vector_store_match:
vector_store_id = vector_store_match.group(1)

View File

@ -151,7 +151,10 @@ async def test_endpoint(request: Request):
dict: A dictionary containing the route of the request URL.
"""
# ping the proxy server to check if its healthy
return {"route": request.url.path}
# Inline import — auth_utils participates in a proxy import cycle.
from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415
return {"route": get_request_route(request)}
@router.get(

View File

@ -333,8 +333,10 @@ def _get_metadata_variable_name(request: Request) -> str:
For ALL other endpoints we call this "metadata"
"""
path = request.url.path
# Inline imports — auth_utils/route_checks participate in a proxy import cycle.
from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415
path = get_request_route(request)
if "thread" in path or "assistant" in path:
return "litellm_metadata"

View File

@ -1568,6 +1568,9 @@ if MCP_AVAILABLE:
from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( # noqa: PLC0415
global_mcp_server_manager,
)
from litellm.proxy.auth.auth_utils import ( # noqa: PLC0415
get_request_route,
)
server_id = request.path_params.get("server_id", "")
if server_id:
@ -1584,7 +1587,7 @@ if MCP_AVAILABLE:
):
# For /token, require PKCE authorization_code; refresh_token
# grants must NOT bypass auth (see comment above).
path_lower = (request.url.path or "").rstrip("/").lower()
path_lower = get_request_route(request).rstrip("/").lower()
if path_lower.endswith("/token"):
body_data = await _read_request_body(request=request)
grant_type = (body_data or {}).get("grant_type", "")

View File

@ -460,7 +460,12 @@ async def _emit_management_endpoint_otel_span(
http_request: Optional[Request] = kwargs.get("http_request")
if http_request is not None:
route = http_request.url.path
# Inline import — auth_utils participates in a proxy import cycle.
from litellm.proxy.auth.auth_utils import ( # noqa: PLC0415
get_request_route,
)
route = get_request_route(http_request)
request_body: dict = await _read_request_body(request=http_request)
else:
route = func.__name__

View File

@ -1307,11 +1307,14 @@ def create_pass_through_route(
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
subpath: str = "", # captures sub-paths when include_subpath=True
):
from litellm.proxy.auth.auth_utils import ( # noqa: PLC0415
get_request_route,
)
from litellm.proxy.pass_through_endpoints.pass_through_endpoints import (
InitPassThroughEndpointHelpers,
)
path = request.url.path
path = get_request_route(request)
# Parse request data based on content type
(

View File

@ -1817,7 +1817,10 @@ async def ui_view_spend_logs( # noqa: PLR0915
)
try:
is_v2 = "/spend/logs/v2" in request.url.path
# Inline import — auth_utils participates in a proxy import cycle.
from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415
is_v2 = "/spend/logs/v2" in get_request_route(request)
formats = ["%Y-%m-%d %H:%M:%S", "%Y-%m-%d"] if is_v2 else ["%Y-%m-%d %H:%M:%S"]
def parse_date(date_str: str) -> datetime:

View File

@ -330,11 +330,16 @@ def is_allowed_to_call_vector_store_endpoint(
provider_config.get_vector_store_endpoints_by_type()
)
# Inline import — auth_utils participates in a proxy import cycle.
from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415
request_route = get_request_route(request)
# Determine the permission type based on the request
permission_type = None
for endpoint in provider_vector_store_endpoints["read"]:
if request.method == endpoint[0] and _does_endpoint_match(
endpoint[1], request.url.path
endpoint[1], request_route
):
permission_type = "read"
break
@ -342,7 +347,7 @@ def is_allowed_to_call_vector_store_endpoint(
if permission_type is None:
for endpoint in provider_vector_store_endpoints["write"]:
if request.method == endpoint[0] and _does_endpoint_match(
endpoint[1], request.url.path
endpoint[1], request_route
):
permission_type = "write"
break
@ -392,10 +397,15 @@ def is_allowed_to_call_vector_store_files_endpoint(
provider_config.get_vector_store_file_endpoints_by_type()
)
# Inline import — auth_utils participates in a proxy import cycle.
from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415
request_route = get_request_route(request)
permission_type: Optional[str] = None
for endpoint in provider_vector_store_endpoints.get("read", ()):
if request.method == endpoint[0] and _does_endpoint_match(
endpoint[1], request.url.path
endpoint[1], request_route
):
permission_type = "read"
break
@ -403,7 +413,7 @@ def is_allowed_to_call_vector_store_files_endpoint(
if permission_type is None:
for endpoint in provider_vector_store_endpoints.get("write", ()):
if request.method == endpoint[0] and _does_endpoint_match(
endpoint[1], request.url.path
endpoint[1], request_route
):
permission_type = "write"
break

View File

@ -217,9 +217,120 @@ def _create_request_with_host_header(path: str, host_header: str) -> Request:
],
)
def test_get_request_route_not_bypassed_by_malformed_host(host_header: str):
for protected_path in ["/health", "/user/new", "/key/generate", "/get/internal_user_settings"]:
request = _create_request_with_host_header(path=protected_path, host_header=host_header)
result = get_request_route(request)
assert result == protected_path, (
f"Host: {host_header!r} caused route {protected_path!r} to resolve as {result!r}"
for protected_path in [
"/health",
"/user/new",
"/key/generate",
"/get/internal_user_settings",
]:
request = _create_request_with_host_header(
path=protected_path, host_header=host_header
)
result = get_request_route(request)
assert (
result == protected_path
), f"Host: {host_header!r} caused route {protected_path!r} to resolve as {result!r}"
# ---------------------------------------------------------------------------
# Regression tests for variant call sites that previously read request.url.path
# (Host-derived) instead of the ASGI scope path. Each test sends a Host header
# crafted to collapse url.path to a substring the call site's decision logic
# would match on, while scope["path"] is the real (unmatching) route.
# ---------------------------------------------------------------------------
_BYPASS_HOSTS = [
"localhost/?x=1",
"localhost:4000/?x=1",
"localhost/#test",
"localhost:4000/#test",
]
def _is_assistants(req):
return RouteChecks._is_assistants_api_request(req)
def _metadata_var_name(req):
from litellm.proxy.litellm_pre_call_utils import _get_metadata_variable_name
return _get_metadata_variable_name(req)
def _vector_store_id_in_path(req):
from litellm.proxy.common_utils.http_parsing_utils import (
_add_vector_store_id_from_path,
)
data: dict = {}
_add_vector_store_id_from_path(request_data=data, request=req)
return "vector_store_id" in data
# (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