From 07eb764096359eafbf70d9f2c92222e5e97c499c Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Tue, 20 Oct 2026 12:28:06 -0700 Subject: [PATCH] fix(proxy): read model_access_group_deny_by_default like the other deny flags A quoted "true" in the config now turns the setting on and an invalid value fails closed. The grant helpers return values and the router raises the 403 at its own boundary, and the auth test runs the real common checks. --- litellm/proxy/auth/auth_checks.py | 19 ++++++++++++++ litellm/proxy/auth/user_api_key_auth.py | 6 ++--- litellm/router.py | 25 ++++++++++++------ .../router_utils/model_access_group_grants.py | 19 +++++--------- .../test_user_api_key_auth_request_flow.py | 26 +++++++------------ 5 files changed, 55 insertions(+), 40 deletions(-) diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index b897293ebce..5dabee3abf8 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -5931,6 +5931,25 @@ def personal_key_team_model_access_applies(valid_token: UserAPIKeyAuth | None) - _MODEL_NAMES: Final[TypeAdapter[tuple[str, ...]]] = TypeAdapter(tuple[str, ...]) +def model_access_group_deny_by_default(general_settings: Mapping[str, object]) -> bool: + """ + Startup rejects a non-boolean value from the config file. A non-boolean value that reaches + general_settings another way enables the policy, so only access-group models are denied. + """ + try: + return ConfigGeneralSettings.model_validate( + MappingProxyType( + { + "model_access_group_deny_by_default": general_settings.get( + "model_access_group_deny_by_default", False + ) + } + ) + ).model_access_group_deny_by_default + except ValidationError: + return True + + async def model_access_group_grants( deny_by_default: bool, valid_token: UserAPIKeyAuth, diff --git a/litellm/proxy/auth/user_api_key_auth.py b/litellm/proxy/auth/user_api_key_auth.py index 9062a8db2a2..9d0decb03f5 100644 --- a/litellm/proxy/auth/user_api_key_auth.py +++ b/litellm/proxy/auth/user_api_key_auth.py @@ -84,6 +84,7 @@ from litellm.proxy.auth.auth_checks import ( # noqa: F401 # legacy module expo jwt_key_mapping_cache_key, key_access_group_grants_model, key_model_aliases_for_auth_check, + model_access_group_deny_by_default, model_access_group_grants, resolve_and_validate_end_user_id, resolve_default_end_user_budget, @@ -3794,10 +3795,7 @@ async def run_centralized_common_checks( bind_admission_counter_keys(user_api_key_auth_obj, end_user_id=end_user_id) grants: Final = await model_access_group_grants( - deny_by_default=( - "model_access_group_deny_by_default" in general_settings - and general_settings["model_access_group_deny_by_default"] is True - ), + deny_by_default=model_access_group_deny_by_default(_GENERAL_SETTINGS_ADAPTER.validate_python(general_settings)), valid_token=user_api_key_auth_obj, team_object=typed_team_object, user_object=user_object if isinstance(user_object, LiteLLM_UserTable) else None, diff --git a/litellm/router.py b/litellm/router.py index dd46e56179e..0d24088d667 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -296,7 +296,11 @@ from litellm.router_utils.kubernetes_pod_discovery import ( resolve_pods_after, resolve_pods_after_bound, ) -from litellm.router_utils.model_access_group_grants import drop_ungranted_deployments, require_granted_deployment +from litellm.router_utils.model_access_group_grants import ( + deployment_is_granted, + granted_deployments, + model_access_denied, +) from litellm.router_utils.pre_call_checks.continuation_prefill_check import ( MID_STREAM_CONTINUATION_MARKER, ContinuationOutput, @@ -14328,11 +14332,16 @@ class Router: resolved_model, deployments = self._resolve_available_deployments( model=model, specific_deployment=specific_deployment, request_kwargs=request_kwargs ) - if isinstance(deployments, Sequence): - return resolved_model, list(drop_ungranted_deployments(deployments, model, request_kwargs)) - return resolved_model, cast( # cast-ok: contract requires a plain dict for a single deployment - dict[str, object], require_granted_deployment(deployments, model, request_kwargs) - ) + if not isinstance(deployments, Sequence): + if not deployment_is_granted(deployments, model, request_kwargs): + raise model_access_denied(model) + return resolved_model, cast( # cast-ok: contract requires a plain dict for a single deployment + dict[str, object], deployments + ) + granted: Final = granted_deployments(deployments, model, request_kwargs) + if deployments and not granted: + raise model_access_denied(model) + return resolved_model, list(granted) def _resolve_available_deployments( self, @@ -15552,8 +15561,8 @@ class Router: ) definition: Final = selected_strategy.deployment strategy_source: Final = marker if marker is not None else definition.model_dump() if definition else None - if strategy_source is not None: - _ = require_granted_deployment(strategy_source, model, request_kwargs) + if strategy_source is not None and not deployment_is_granted(strategy_source, model, request_kwargs): + raise model_access_denied(model) config_id: Final = definition.model_info.id if definition else None updated_at: Final = definition.model_info.updated_at if definition else None router_kind: Final = classify_strategy_router_model(definition.litellm_params.model) if definition else None diff --git a/litellm/router_utils/model_access_group_grants.py b/litellm/router_utils/model_access_group_grants.py index 270eacd10df..55534a4508a 100644 --- a/litellm/router_utils/model_access_group_grants.py +++ b/litellm/router_utils/model_access_group_grants.py @@ -52,7 +52,7 @@ def _is_granted(deployment: Mapping[str, object], requested_model: str, grants: return not groups or requested_model in grants or row.model_name in grants or not grants.isdisjoint(groups) -def _denied(model: str) -> litellm.PermissionDeniedError: +def model_access_denied(model: str) -> litellm.PermissionDeniedError: from litellm.proxy.auth.model_access_denied import model_access_denied_client_message return litellm.PermissionDeniedError( @@ -63,22 +63,17 @@ def _denied(model: str) -> litellm.PermissionDeniedError: ) -def require_granted_deployment( - deployment: _Deployment, requested_model: str, request_kwargs: Mapping[str, object] | None -) -> _Deployment: +def deployment_is_granted( + deployment: Mapping[str, object], requested_model: str, request_kwargs: Mapping[str, object] | None +) -> bool: grants: Final = request_model_access_group_grants(request_kwargs) - if grants is None or _is_granted(deployment, requested_model, grants): - return deployment - raise _denied(requested_model) + return grants is None or _is_granted(deployment, requested_model, grants) -def drop_ungranted_deployments( +def granted_deployments( deployments: Sequence[_Deployment], requested_model: str, request_kwargs: Mapping[str, object] | None ) -> tuple[_Deployment, ...]: grants: Final = request_model_access_group_grants(request_kwargs) if grants is None: return tuple(deployments) - granted: Final = tuple(deployment for deployment in deployments if _is_granted(deployment, requested_model, grants)) - if deployments and not granted: - raise _denied(requested_model) - return granted + return tuple(deployment for deployment in deployments if _is_granted(deployment, requested_model, grants)) diff --git a/tests/unit/proxy/auth/test_user_api_key_auth_request_flow.py b/tests/unit/proxy/auth/test_user_api_key_auth_request_flow.py index 22a94d14077..91819d3a65f 100644 --- a/tests/unit/proxy/auth/test_user_api_key_auth_request_flow.py +++ b/tests/unit/proxy/auth/test_user_api_key_auth_request_flow.py @@ -5207,10 +5207,12 @@ async def test_centralized_common_checks_enforces_team_model_max_budget_from_the "general_settings, expected_grants", [ ({"model_access_group_deny_by_default": True}, frozenset({"group-x"})), - ({"model_access_group_deny_by_default": "true"}, None), + ({"model_access_group_deny_by_default": "true"}, frozenset({"group-x"})), + ({"model_access_group_deny_by_default": "not-a-bool"}, frozenset({"group-x"})), + ({"model_access_group_deny_by_default": False}, None), ({}, None), ], - ids=["deny-by-default", "setting-not-true", "setting-absent"], + ids=["deny-by-default", "quoted-true", "invalid-value-fails-closed", "setting-false", "setting-absent"], ) async def test_centralized_common_checks_stamps_the_team_model_access_group_grants_on_the_token( monkeypatch: pytest.MonkeyPatch, @@ -5238,20 +5240,12 @@ async def test_centralized_common_checks_stamps_the_team_model_access_group_gran }.items(): monkeypatch.setattr(proxy_server, name, value) - with ( - patch( - "litellm.proxy.auth.user_api_key_auth.common_checks", new_callable=AsyncMock - ), # test-quality-ok: stubs the sibling check so only the grant stamping is under test - patch( # test-quality-ok: stubs the budget reservation so only the grant stamping is under test - "litellm.proxy.auth.user_api_key_auth._reserve_budget_after_common_checks", new_callable=AsyncMock - ), - ): - await run_centralized_common_checks( - user_api_key_auth_obj=token, - request=request, - request_data={"model": "group-x"}, - route="/chat/completions", - ) + await run_centralized_common_checks( + user_api_key_auth_obj=token, + request=request, + request_data={"model": "group-x"}, + route="/chat/completions", + ) assert token.model_access_group_grants == expected_grants