mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix(auth): match a :path placeholder the way the router's converter does
A team id may carry a slash, a colon, or both. The gate expanded {x:path}
to "[^:]+", so an id with a colon in it matched no self_managed_routes
entry and its team admin got the proxy-admin-only denial on a route the
router had already resolved for them. Listing a second {x} spelling covered
a colon or a slash but never both.
Expand {x:path} to ".+" instead, except when the template puts a ":"
literal of its own after the placeholder, which is where the narrower form
was earning its keep: the Google routes end in ":generateContent" and
friends, and there the value has to stop before that suffix rather than
swallow it and match a different verb.
That lets self_managed_routes drop back to the two :path spellings the
router itself mounts.
This commit is contained in:
parent
2e0d81a9da
commit
db80501232
3 changed files with 62 additions and 14 deletions
|
|
@ -834,14 +834,10 @@ class LiteLLMRoutes(enum.Enum):
|
|||
# handler calls _verify_team_access, which admits only a proxy admin, an
|
||||
# org admin for the team, or an admin of this team.
|
||||
#
|
||||
# Two spellings per route because neither placeholder alone covers every
|
||||
# team id the router accepts: the gate expands {x:path} to "[^:]+", which
|
||||
# takes a slash but not a colon, and {x} to "[^/]+", which takes a colon
|
||||
# but not a slash. team_id is a free-form string, so both are reachable.
|
||||
# team_id is a free-form string, so it spells these with the same path
|
||||
# converter the router uses; the gate matches that converter.
|
||||
"/team/{team_id:path}/callback",
|
||||
"/team/{team_id:path}/callback/{callback_name}",
|
||||
"/team/{team_id}/callback",
|
||||
"/team/{team_id}/callback/{callback_name}",
|
||||
"/model/new",
|
||||
"/model/update",
|
||||
"/model/delete",
|
||||
|
|
|
|||
|
|
@ -497,10 +497,17 @@ class RouteChecks:
|
|||
|
||||
def _placeholder_to_regex(match: re.Match) -> str:
|
||||
placeholder: Final = match.group(0).strip("{}")
|
||||
if placeholder.endswith(":path"):
|
||||
# allow "/" in the placeholder value, but don't eat the route suffix after ":"
|
||||
return r"[^:]+"
|
||||
return r"[^/]+"
|
||||
if not placeholder.endswith(":path"):
|
||||
return r"[^/]+"
|
||||
# A ":path" placeholder takes whatever the router's own path
|
||||
# converter takes, slashes and colons alike, so an id spelled with
|
||||
# either (or both) still matches the template it was mounted under.
|
||||
#
|
||||
# Unless the template puts a ":" literal of its own after the
|
||||
# placeholder: the Google routes end in ":generateContent" and
|
||||
# friends, and there the value has to stop before that suffix
|
||||
# rather than swallow it and match a different verb.
|
||||
return r"[^:]+" if ":" in match.string[match.end() :] else r".+"
|
||||
|
||||
pattern = re.sub(r"\{[^}]+\}", _placeholder_to_regex, pattern)
|
||||
# Anchor the pattern to match the entire string
|
||||
|
|
|
|||
|
|
@ -3551,10 +3551,13 @@ TEAM_CALLBACK_ROUTES = (
|
|||
# contain a slash
|
||||
"/team/tenant/06bda574/callback",
|
||||
"/team/tenant/06bda574/callback/langfuse",
|
||||
# team_id is a free-form string, so it may also contain a colon, which the
|
||||
# gate's :path expansion excludes
|
||||
# team_id is a free-form string, so it may also contain a colon
|
||||
"/team/tenant:06bda574/callback",
|
||||
"/team/tenant:06bda574/callback/langfuse",
|
||||
# or both, which is the shape neither a "[^:]+" nor a "[^/]+" expansion
|
||||
# of the placeholder reaches on its own
|
||||
"/team/tenant:acme/prod/callback",
|
||||
"/team/tenant:acme/prod/callback/langfuse",
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -3597,8 +3600,6 @@ def test_team_callback_routes_are_self_managed():
|
|||
for template in (
|
||||
"/team/{team_id:path}/callback",
|
||||
"/team/{team_id:path}/callback/{callback_name}",
|
||||
"/team/{team_id}/callback",
|
||||
"/team/{team_id}/callback/{callback_name}",
|
||||
):
|
||||
assert template in LiteLLMRoutes.self_managed_routes.value
|
||||
|
||||
|
|
@ -3625,6 +3626,50 @@ def test_team_callback_routes_reach_their_handler_for_non_admins(route, role):
|
|||
assert _gate(route, role) == "allowed"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"pattern, route, matches",
|
||||
[
|
||||
# a :path placeholder takes what the router's path converter takes
|
||||
("/team/{team_id:path}/callback", "/team/plain/callback", True),
|
||||
("/team/{team_id:path}/callback", "/team/tenant/acme/callback", True),
|
||||
("/team/{team_id:path}/callback", "/team/tenant:acme/callback", True),
|
||||
("/team/{team_id:path}/callback", "/team/tenant:acme/prod/callback", True),
|
||||
# and still has to reach the template's own suffix
|
||||
("/team/{team_id:path}/callback", "/team/tenant:acme/disable_logging", False),
|
||||
# a template with a ":" literal after the placeholder keeps the suffix
|
||||
(
|
||||
"/v1beta/models/{model_name:path}:generateContent",
|
||||
"/v1beta/models/gemini-2.5-flash:generateContent",
|
||||
True,
|
||||
),
|
||||
(
|
||||
"/v1beta/models/{model_name:path}:generateContent",
|
||||
"/v1beta/models/publishers/google/gemini-2.5-flash:generateContent",
|
||||
True,
|
||||
),
|
||||
# the value must not swallow that suffix and match a different verb
|
||||
(
|
||||
"/v1beta/models/{model_name:path}:generateContent",
|
||||
"/v1beta/models/gemini-2.5-flash:countTokens",
|
||||
False,
|
||||
),
|
||||
# an ordinary placeholder stays one segment
|
||||
("/team/{team_id}/members/me", "/team/abc/members/me", True),
|
||||
("/team/{team_id}/members/me", "/team/tenant/abc/members/me", False),
|
||||
],
|
||||
)
|
||||
def test_path_placeholder_matches_what_the_router_accepts(pattern, route, matches):
|
||||
"""The gate's placeholder expansion has to agree with the router's.
|
||||
|
||||
A team id may carry a slash, a colon, or both, and the router mounted these
|
||||
paths with the same :path converter, so an id the router routes must not be
|
||||
an id the gate fails to recognize. The one narrowing that stays is a template
|
||||
whose own suffix begins with a colon: there the value stops before it, or
|
||||
":generateContent" would also match a ":countTokens" request.
|
||||
"""
|
||||
assert RouteChecks._route_matches_pattern(route=route, pattern=pattern) is matches
|
||||
|
||||
|
||||
def test_team_disable_logging_stays_proxy_admin_only():
|
||||
"""disable_logging was left out of the grant, so it must still be rejected at
|
||||
the gate. It is the one team callback route a team admin cannot reach."""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue