From 3d6951476b3abc6d957d7a0910b5016f2a6d6a46 Mon Sep 17 00:00:00 2001 From: Yucheng Zhu Date: Fri, 24 Jul 2026 17:59:51 -0700 Subject: [PATCH] fix(credentials): deny non-admin patches that would widen an auto-enabled destination An auto_enable destination with no explicit grants falls back to proxy-wide visibility, so a team-admin emptying the last team grant turned a team-scoped destination into one receiving every tenant's traces. The decider now refuses a non-admin patch when the post-merge access would have no grant left on an auto_enable destination; revoking with a surviving org or global grant, revoking on manual destinations, and proxy-admin edits are unchanged. The deny reason is safe to surface because the path is only reachable by a caller whose own teams hold every grant being removed, so the destination is already visible to them --- .../credential_endpoints/access_decision.py | 13 ++++- .../test_access_decision.py | 57 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/litellm/proxy/credential_endpoints/access_decision.py b/litellm/proxy/credential_endpoints/access_decision.py index e5ab909bfe0..34a5d3623a1 100644 --- a/litellm/proxy/credential_endpoints/access_decision.py +++ b/litellm/proxy/credential_endpoints/access_decision.py @@ -70,7 +70,8 @@ def decide_credential_patch( modify any immutable ``credential_info`` field, and (c) limits its ``access`` change to appending team_ids the caller is team-admin of to ``access.teams`` (no removals, no foreign ids, no ``global``/``orgs`` - edits). + edits), and (d) does not remove the last explicit grant from an + ``auto_enable`` destination, whose empty-grant fallback is proxy-wide. """ if is_proxy_admin: return Allow() @@ -129,4 +130,14 @@ def decide_credential_patch( from_user_input=True, ) + auto_enable = existing_info.auto_enable if existing_info is not None else False + had_explicit_grants = bool(existing_global or existing_teams or existing_orgs) + grants_after_merge = bool(existing_global or existing_orgs or patch_teams) + if auto_enable and had_explicit_grants and not grants_after_merge: + return Deny( + "removing the last grant would make this auto-enabled destination " + "proxy-wide; only the proxy admin can remove it", + from_user_input=True, + ) + return Allow() diff --git a/tests/test_litellm/proxy/credential_endpoints/test_access_decision.py b/tests/test_litellm/proxy/credential_endpoints/test_access_decision.py index 295efb8f512..535b93311e6 100644 --- a/tests/test_litellm/proxy/credential_endpoints/test_access_decision.py +++ b/tests/test_litellm/proxy/credential_endpoints/test_access_decision.py @@ -293,6 +293,63 @@ class TestTeamAdminRevoke: assert "team-B" not in d.reason +class TestAutoEnableWideningGuard: + """Emptying the last grant of an auto_enable destination falls through to + the documented empty-grants-means-proxy-wide fallback, so a narrowing edit + by a team-admin would widen the destination to every tenant. The decider + must refuse to cross that boundary for non-admins.""" + + def test_emptying_sole_grant_on_auto_enable_destination_is_denied(self): + existing = { + **_EXISTING_INFO, + "auto_enable": True, + "access": {"global": False, "teams": ["team-T"], "orgs": []}, + } + d = _decision( + existing_info=existing, + patch_info={"access": {"teams": []}}, + ) + assert isinstance(d, Deny) + assert "proxy-wide" in d.reason + + def test_emptying_teams_with_surviving_org_grant_is_allowed(self): + existing = { + **_EXISTING_INFO, + "auto_enable": True, + "access": {"global": False, "teams": ["team-T"], "orgs": ["org-1"]}, + } + d = _decision( + existing_info=existing, + patch_info={"access": {"teams": []}}, + ) + assert isinstance(d, Allow) + + def test_emptying_sole_grant_without_auto_enable_is_allowed(self): + existing = { + **_EXISTING_INFO, + "auto_enable": False, + "access": {"global": False, "teams": ["team-T"], "orgs": []}, + } + d = _decision( + existing_info=existing, + patch_info={"access": {"teams": []}}, + ) + assert isinstance(d, Allow) + + def test_proxy_admin_may_empty_sole_grant_on_auto_enable_destination(self): + existing = { + **_EXISTING_INFO, + "auto_enable": True, + "access": {"global": False, "teams": ["team-T"], "orgs": []}, + } + d = _decision( + is_proxy_admin=True, + existing_info=existing, + patch_info={"access": {"teams": []}}, + ) + assert isinstance(d, Allow) + + class TestTeamAdminMultipleTeams: def test_can_add_multiple_own_team_ids(self): d = _decision(