chore(proxy): cherry-pick #28547 onto patch/v1.86.1 - #28969
Conversation
Backport of #28547 (`d480ffda3c`) onto the `patch/v1.86.1` branch. Routes the remaining path-dependent call sites in auth, ACL, routing, and audit-log decisions through `get_request_route(request)` so they read from the ASGI `scope["path"]` instead of `request.url.path`. The helper itself already exists on v1.86.1 (added by #27878); this PR extends the helper's usage to the additional sites listed below. Sites routed through get_request_route: - _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 Regression tests in tests/proxy_unit_tests/test_proxy_routes.py 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"]. Conflict resolution ------------------- Cherry-pick applied cleanly with no conflicts. All 11 files plus the test file are pure `request.url.path` → `get_request_route(request)` swaps with the lazy auth_utils import (no feature drift).
Greptile SummaryThis cherry-pick backports #28547 onto
Confidence Score: 4/5The auth and ACL fixes are correct and consistent across all changed files; the cherry-pick is clean with no feature drift from the original PR. All 10 production sites are correctly migrated to tests/proxy_unit_tests/test_proxy_routes.py — missing a _CALL_SITES entry for the vector store ACL bypass; litellm/proxy/vector_store_endpoints/utils.py is otherwise correct.
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/auth_utils.py | Docstring updated to explain the security motivation for using scope["path"] over request.url.path; no functional logic change. |
| litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py | Routes all three path-dependent checks (/.well-known/ bypass, upstream-delegate, OAuth2 mode) through get_request_route; inline import avoids import cycle. |
| litellm/proxy/auth/route_checks.py | _is_assistants_api_request now reads the route from scope instead of url.path, preventing thread/assistant substring injection via Host header. |
| litellm/proxy/management_endpoints/mcp_management_endpoints.py | PKCE /token suffix check migrated to get_request_route; the original or "" guard on url.path was unnecessary since the helper always returns str. |
| litellm/proxy/vector_store_endpoints/utils.py | ACL permission checks (read/write) for vector store endpoints now use scope path; the new test suite has no direct parametric test for is_allowed_to_call_vector_store_endpoint bypass. |
| litellm/proxy/common_utils/http_parsing_utils.py | _add_vector_store_id_from_path now extracts the vector_store_id from the scope path rather than url.path. |
| litellm/proxy/litellm_pre_call_utils.py | _get_metadata_variable_name uses scope path for thread/assistant detection; covered by a parametric regression test. |
| litellm/proxy/management_helpers/utils.py | Both OTel route-tagging sites inside management_endpoint_wrapper now use get_request_route; no test for these but they are non-security audit-log paths. |
| litellm/proxy/pass_through_endpoints/pass_through_endpoints.py | Pass-through path determination uses get_request_route, which also strips root_path; change is consistent with the rest of the proxy. |
| litellm/proxy/spend_tracking/spend_management_endpoints.py | v2 spend-log classification uses scope path; tested by spend_logs_v2_classification parametric case. |
| litellm/proxy/health_endpoints/_health_endpoints.py | test_endpoint echo now returns scope path; covered by health_route_echo parametric test. |
| tests/proxy_unit_tests/test_proxy_routes.py | New parametric test suite covers 7 call sites against 4 bypass host patterns; vector store ACL (is_allowed_to_call_vector_store_endpoint) and pass-through path are not covered by a direct parametric test. |
Reviews (1): Last reviewed commit: "chore(proxy): cherry-pick #28547 onto pa..." | Re-trigger Greptile
| _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, |
There was a problem hiding this comment.
Missing direct test coverage for vector store ACL bypass
is_allowed_to_call_vector_store_endpoint and is_allowed_to_call_vector_store_files_endpoint in vector_store_endpoints/utils.py are security-relevant permission checks (read/write ACL decisions) that were migrated to get_request_route, but no entry in _CALL_SITES exercises them. A crafted Host header that injects a read-matching endpoint pattern into url.path while scope["path"] is an unrelated route would have previously bypassed the write guard; the fix is correct but the parametric suite doesn't confirm it. Adding a case analogous to vector_store_id_extraction would close the gap.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Backport of #28547 (
d480ffda3c) ontopatch/v1.86.1.Routes the remaining path-dependent call sites in auth, ACL, routing, and audit-log decisions through
get_request_route(request)so they read from the ASGIscope["path"]instead ofrequest.url.path. The helper itself already exists on v1.86.1 (added by #27878); this PR extends the helper's usage to the additional sites listed below.Sites routed through
get_request_route_experimental/mcp_server/auth/user_api_key_auth_mcp.pymanagement_endpoints/mcp_management_endpoints.pyvector_store_endpoints/utils.pypass_through_endpoints/pass_through_endpoints.pyauth/route_checks.pylitellm_pre_call_utils.pyspend_tracking/spend_management_endpoints.pycommon_utils/http_parsing_utils.pymanagement_helpers/utils.pyhealth_endpoints/_health_endpoints.pyConflict resolution
Cherry-pick applied cleanly with no conflicts. All 11 files plus the test file are pure
request.url.path→get_request_route(request)swaps with the lazyauth_utilsimport (no feature drift).Test plan
uv run pytest tests/proxy_unit_tests/test_proxy_routes.py -vmake test-unit