From 037df6ada39ad7b2f65ba8d33cd0dd1dcbacd884 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Fri, 5 Jun 2026 09:10:51 -0700 Subject: [PATCH] feat(proxy): auth_v2 governs credentials via method-aware route matching Credentials are REST-style: the verb is the HTTP method (POST /credentials is create, GET is list) and the id is in the path, which the verb-in-path RPC matcher cannot express. Adds method-aware, prefix-capable matching for these resources and governs the credentials admin surface. Fixes a latent bypass while wiring this: authorize() re-matched the route without the method, so any REST route would have resolved to None inside authorize() and been loud-opened even when the entry point had classified it as governed. The method is now threaded through. Path-param ids are not extracted yet, so credential objects stay at "credential:*" (per-id policies a follow-up). --- litellm/proxy/auth/v2/authorizer.py | 5 ++- litellm/proxy/auth/v2/entry.py | 8 ++-- litellm/proxy/auth/v2/route_map.py | 44 +++++++++++++++++-- .../proxy/auth/v2/test_authorizer.py | 15 +++++++ .../proxy/auth/v2/test_route_map.py | 28 ++++++++++++ 5 files changed, 92 insertions(+), 8 deletions(-) diff --git a/litellm/proxy/auth/v2/authorizer.py b/litellm/proxy/auth/v2/authorizer.py index b6700a47a8f..cf8e03cde4b 100644 --- a/litellm/proxy/auth/v2/authorizer.py +++ b/litellm/proxy/auth/v2/authorizer.py @@ -33,13 +33,16 @@ def authorize( route: str, request_data: Optional[Dict[str, Any]], enforcer: Any, + method: Optional[str] = None, ) -> None: """Enforce policy for ``route``. No-op (loud) for routes v2 doesn't yet govern. Raises :class:`AuthorizationDenied` when a governed route is denied. ``enforcer`` is anything exposing ``enforce(subject, domain, obj, action)``. + ``method`` is required to resolve REST resources (e.g. credentials) whose verb + is the HTTP method; without it those routes would be treated as loud-open. """ - rule = match_route(route) + rule = match_route(route, method) if rule is None: logger.warning( "auth_v2: route '%s' is not yet protected by auth_v2; allowing. " diff --git a/litellm/proxy/auth/v2/entry.py b/litellm/proxy/auth/v2/entry.py index e440ecf6e02..5663ce9e1f7 100644 --- a/litellm/proxy/auth/v2/entry.py +++ b/litellm/proxy/auth/v2/entry.py @@ -51,7 +51,7 @@ async def user_api_key_auth_v2( proxy_logging_obj=proxy_logging_obj, ) - rule = match_route(route) + rule = match_route(route, request.method) if rule is not None: # Control plane: RBAC policy rows. identity = await authenticate(token, ctx) @@ -72,7 +72,7 @@ async def user_api_key_auth_v2( ) try: - authorize(principal, route, request_data, enforcer) + authorize(principal, route, request_data, enforcer, request.method) except AuthorizationDenied as e: raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(e)) @@ -80,7 +80,7 @@ async def user_api_key_auth_v2( return identity if is_inference_route(route): - # Data plane: casbin ABAC over the principal's allowed-model attribute. + # Data plane: plain allowed-model predicate over the principal's key. identity = await authenticate(token, ctx) request_data = await _read_request_body(request=request) requested_model = ( @@ -98,7 +98,7 @@ async def user_api_key_auth_v2( # Loud-open: route v2 doesn't yet govern. No identity required. identity = await _best_effort_identity(token, ctx) - authorize(build_principal(identity), route, None, _DENY_ALL) + authorize(build_principal(identity), route, None, _DENY_ALL, request.method) identity.request_route = route return identity diff --git a/litellm/proxy/auth/v2/route_map.py b/litellm/proxy/auth/v2/route_map.py index b744fde50c3..90fcab4ad7f 100644 --- a/litellm/proxy/auth/v2/route_map.py +++ b/litellm/proxy/auth/v2/route_map.py @@ -115,10 +115,48 @@ _INFERENCE_ROUTES = { } -def match_route(route: str) -> Optional[GovernedRoute]: - """Return the governance rule for ``route``, or None if v2 doesn't yet own it.""" +@dataclass(frozen=True) +class _RestRule: + method: str + path: str + is_prefix: bool + route: GovernedRoute + + +# REST-style resources encode the verb in the HTTP method (POST /credentials = +# create, GET /credentials = list) and the id in the path, so they need +# method-aware, prefix-capable matching rather than the verb-in-path lookup the +# RPC routes above use. Path-param ids are not extracted yet, so objects stay at +# ":*" (per-id credential policies are a follow-up). +_REST_RULES: List[_RestRule] = [ + _RestRule("POST", "/credentials", False, GovernedRoute("credential", "write")), + _RestRule("GET", "/credentials", False, GovernedRoute("credential", "read")), + _RestRule("GET", "/credentials/", True, GovernedRoute("credential", "read")), + _RestRule("DELETE", "/credentials/", True, GovernedRoute("credential", "delete")), +] + + +def match_route(route: str, method: Optional[str] = None) -> Optional[GovernedRoute]: + """Return the governance rule for ``route``, or None if v2 doesn't yet own it. + + ``method`` is required to resolve REST resources whose verb is the HTTP + method; the verb-in-path RPC routes are method-agnostic and match without it. + """ normalized = route.rstrip("/") or "/" - return _GOVERNED.get(normalized) + rule = _GOVERNED.get(normalized) + if rule is not None: + return rule + if method is not None: + verb = method.upper() + for rest in _REST_RULES: + if rest.method != verb: + continue + if rest.is_prefix: + if route.startswith(rest.path): + return rest.route + elif normalized == rest.path: + return rest.route + return None def is_inference_route(route: str) -> bool: diff --git a/tests/test_litellm/proxy/auth/v2/test_authorizer.py b/tests/test_litellm/proxy/auth/v2/test_authorizer.py index cee139d12d6..5ee6cb498db 100644 --- a/tests/test_litellm/proxy/auth/v2/test_authorizer.py +++ b/tests/test_litellm/proxy/auth/v2/test_authorizer.py @@ -55,3 +55,18 @@ def test_ungoverned_route_is_loud_open_and_never_enforces(caplog): authorize(PRINCIPAL, "/chat/completions", {}, _Exploding()) assert "not yet protected" in caplog.text assert "/chat/completions" in caplog.text + + +def test_rest_route_is_enforced_when_method_is_threaded(): + # Regression: a REST route (credentials) only resolves with the HTTP method. + # If authorize() dropped the method it would loud-open and bypass enforcement. + enforcer = _Enforcer(ret=True) + authorize(PRINCIPAL, "/credentials", {}, enforcer, "POST") + assert enforcer.calls == [("user:u1", "*", "credential:*", "write")] + + +def test_rest_route_without_method_is_not_silently_enforced(caplog): + # Without the method the REST route can't resolve; it must loud-open, not 500. + with caplog.at_level(logging.WARNING, logger="litellm.proxy.auth.v2"): + authorize(PRINCIPAL, "/credentials", {}, _Exploding()) + assert "not yet protected" in caplog.text diff --git a/tests/test_litellm/proxy/auth/v2/test_route_map.py b/tests/test_litellm/proxy/auth/v2/test_route_map.py index aa3727cb76a..f9ed99c7626 100644 --- a/tests/test_litellm/proxy/auth/v2/test_route_map.py +++ b/tests/test_litellm/proxy/auth/v2/test_route_map.py @@ -119,7 +119,35 @@ def test_runtime_guardrail_verbs_stay_loud_open(): assert match_route(route) is None +def test_credentials_are_method_aware(): + # Same path, different verb -> different action. + assert match_route("/credentials", "POST").action == "write" + assert match_route("/credentials", "GET").action == "read" + assert match_route("/credentials", "POST").resource == "credential" + + +def test_credentials_path_params_match_by_prefix(): + assert match_route("/credentials/by_name/openai-creds", "GET").action == "read" + assert match_route("/credentials/by_model/gpt-4o", "GET").action == "read" + assert match_route("/credentials/openai-creds", "DELETE").action == "delete" + + +def test_credentials_require_method_and_reject_wrong_verb(): + # Without a method the REST resource cannot be resolved. + assert match_route("/credentials") is None + # A verb with no rule (PUT) is not governed. + assert match_route("/credentials", "PUT") is None + + +def test_rpc_routes_remain_method_agnostic(): + # Verb-in-path routes match with or without a method passed. + assert match_route("/model/new").action == "write" + assert match_route("/model/new", "POST").action == "write" + assert match_route("/model/new", "GET").action == "write" + + def test_ungoverned_routes_return_none(): # Genuinely not yet owned by v2: loud-open. for route in ("/v1/models", "/health", "/"): assert match_route(route) is None + assert match_route(route, "GET") is None