mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-14 09:03:16 +00:00
fix(proxy): accept virtual keys on include_subpath pass-through sub-paths
A pass-through endpoint declared with include_subpath and no auth flag forwards its exact path for any valid virtual key, but sub-path requests were rejected with a management-route 401 (Only proxy admin can be used ...). The unauthenticated-forward carve-out in the centralized auth gate matched only the exact configured path, so wildcard sub-paths fell through to admin-only authz. Recognize sub-paths of an include_subpath endpoint the same way as the exact path, using the same str(auth).lower() == true spelling the route registration uses to decide enforcement, so auth:true still enforces on both. Fixes #36280
This commit is contained in:
parent
5e4b3838aa
commit
d4b4df9ebd
2 changed files with 88 additions and 1 deletions
|
|
@ -2363,6 +2363,34 @@ def _token_can_vouch_for_team(valid_token: UserAPIKeyAuth, lookup_error: BaseExc
|
|||
return PrismaDBExceptionHandler.should_allow_request_on_db_unavailable()
|
||||
|
||||
|
||||
_PASS_THROUGH_SUBPATH_SEPARATOR: Final = "/"
|
||||
|
||||
|
||||
def _request_matches_unauthenticated_pass_through(endpoint: object, route: str) -> bool:
|
||||
"""Match config pass-through routes that forward without proxy authz.
|
||||
|
||||
``include_subpath`` wildcards must inherit the exact path's auth treatment,
|
||||
else a valid virtual key is 401'd as an admin-only route on every sub-path
|
||||
(issue #36280). ``auth`` uses the same ``str(...).lower() == "true"`` spelling
|
||||
the route registration uses to decide enforcement."""
|
||||
if not isinstance(endpoint, dict):
|
||||
return False
|
||||
endpoint_data: Final = cast("dict[str, object]", endpoint)
|
||||
auth: Final = endpoint_data.get("auth")
|
||||
if auth is not None and str(auth).lower() == "true":
|
||||
return False
|
||||
path: Final = endpoint_data.get("path", "")
|
||||
if not isinstance(path, str) or not path:
|
||||
return False
|
||||
if route == path:
|
||||
return True
|
||||
if endpoint_data.get("include_subpath", False) is True:
|
||||
return route.startswith(
|
||||
path.rstrip(_PASS_THROUGH_SUBPATH_SEPARATOR) + _PASS_THROUGH_SUBPATH_SEPARATOR
|
||||
)
|
||||
return False
|
||||
|
||||
|
||||
@tracer.wrap()
|
||||
async def _run_centralized_common_checks(
|
||||
user_api_key_auth_obj: UserAPIKeyAuth,
|
||||
|
|
@ -2413,7 +2441,7 @@ async def _run_centralized_common_checks(
|
|||
pass_through_endpoints: Final = general_settings.get("pass_through_endpoints", None)
|
||||
if pass_through_endpoints is not None:
|
||||
for endpoint in pass_through_endpoints:
|
||||
if isinstance(endpoint, dict) and endpoint.get("path", "") == route and endpoint.get("auth") is not True:
|
||||
if _request_matches_unauthenticated_pass_through(endpoint=endpoint, route=route):
|
||||
return
|
||||
|
||||
# No-auth dev mode: master_key unset AND no JWT/OAuth2 auth
|
||||
|
|
|
|||
|
|
@ -4099,6 +4099,65 @@ async def test_centralized_common_checks_runs_for_custom_auth_with_flag():
|
|||
setattr(_proxy_server_mod, k, v)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"endpoint, route, expect_common_checks",
|
||||
[
|
||||
# #36280: an include_subpath endpoint without auth forwards its
|
||||
# sub-paths without proxy authz, exactly like its exact path — so a
|
||||
# virtual key is accepted instead of 401'd as an admin-only route.
|
||||
({"path": "/orx", "include_subpath": True}, "/orx/rerank", False),
|
||||
({"path": "/orx", "include_subpath": True}, "/orx", False),
|
||||
# No include_subpath: the sub-path is not a registered route, so authz
|
||||
# must still run — the bypass must not widen to arbitrary prefixes.
|
||||
({"path": "/orx"}, "/orx/rerank", True),
|
||||
# A path that only shares a prefix is not a sub-path.
|
||||
({"path": "/orx", "include_subpath": True}, "/orxfoo", True),
|
||||
# auth enforced (any spelling) keeps authz on sub-paths.
|
||||
({"path": "/orx", "include_subpath": True, "auth": "true"}, "/orx/rerank", True),
|
||||
({"path": "/orx", "include_subpath": True, "auth": True}, "/orx/rerank", True),
|
||||
# auth enforced also keeps authz on the exact path (any spelling).
|
||||
({"path": "/orx", "auth": "true"}, "/orx", True),
|
||||
({"path": "/orx", "auth": True}, "/orx", True),
|
||||
],
|
||||
)
|
||||
async def test_centralized_common_checks_include_subpath_pass_through_authz(
|
||||
endpoint, route, expect_common_checks
|
||||
):
|
||||
"""#36280: pass-through sub-paths registered via include_subpath must be
|
||||
treated like the exact path for auth — the auth:false forward bypasses the
|
||||
centralized gate on both, and auth:true keeps enforcing on both."""
|
||||
from fastapi import Request
|
||||
from starlette.datastructures import URL
|
||||
|
||||
import litellm.proxy.proxy_server as _proxy_server_mod
|
||||
|
||||
token = UserAPIKeyAuth(api_key="sk-test", user_id="u1")
|
||||
request = Request(scope={"type": "http", "method": "POST", "headers": [], "query_string": b""})
|
||||
request._url = URL(url=route)
|
||||
|
||||
attrs = _proxy_attrs_for_centralized_checks(user_custom_auth=None)
|
||||
attrs["general_settings"] = {"pass_through_endpoints": [endpoint]}
|
||||
originals = {a: getattr(_proxy_server_mod, a, None) for a in attrs}
|
||||
try:
|
||||
for k, v in attrs.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
with patch(
|
||||
"litellm.proxy.auth.user_api_key_auth.common_checks",
|
||||
new_callable=AsyncMock,
|
||||
) as mock_checks:
|
||||
await _run_centralized_common_checks(
|
||||
user_api_key_auth_obj=token,
|
||||
request=request,
|
||||
request_data={},
|
||||
route=route,
|
||||
)
|
||||
assert bool(mock_checks.await_count) is expect_common_checks
|
||||
finally:
|
||||
for k, v in originals.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_centralized_common_checks_runs_for_oauth2_fallback_token():
|
||||
"""VERIA-18 regression: an OAuth2 token that would previously early-
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue