diff --git a/litellm/proxy/credential_endpoints/access_decision.py b/litellm/proxy/credential_endpoints/access_decision.py index 34a5d3623a1..d052a89cf2b 100644 --- a/litellm/proxy/credential_endpoints/access_decision.py +++ b/litellm/proxy/credential_endpoints/access_decision.py @@ -70,8 +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), and (d) does not remove the last explicit grant from an - ``auto_enable`` destination, whose empty-grant fallback is proxy-wide. + edits). Emptying the last team grant is allowed: empty access is deny-all, + so it disables the destination rather than widening it. """ if is_proxy_admin: return Allow() @@ -130,14 +130,4 @@ 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/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index 1bdb34a7481..31e4e1694b0 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -701,15 +701,15 @@ async def _resolve_logging_exporters( ) -> "tuple[list, list]": """Resolve the destinations this request fans out to, as (destinations, backends). - ``credential_info.access`` is visibility, not enablement: a granted destination - does NOT fire just because the caller can see it. A destination is selected only - when it is an explicit global/default (``auto_enable``) OR it is named in the - identity chain's ``logging_exporters`` (key + team + org) AND its ``access`` grants - the caller. The visibility re-check is defensive: a name that points at a - destination no longer visible to this identity is ignored, so a stale or - cross-tenant assignment can never route traffic out. Each survivor is built via - ``build_destination`` and deduped on (endpoint, headers, resource attributes). - Returns ([], []) when nothing is selected (default-deny). + ``credential_info.access`` gates every destination: empty access grants no one, so + an empty-access destination never fires (proxy-wide requires ``access.global``). A + destination is selected when its ``access`` grants the caller AND either it is + ``auto_enable`` (fires without being named) or it is named in the identity chain's + ``logging_exporters`` (key + team + org). The access check is also the defensive + re-check on a named destination, so a stale or cross-tenant assignment can never + route traffic out. Each survivor is built via ``build_destination`` and deduped on + (endpoint, headers, resource attributes). Returns ([], []) when nothing is selected + (default-deny). """ from litellm.integrations.otel.presets.destinations import build_destination from litellm.proxy.management_endpoints.logging_exporter_access import ( @@ -727,19 +727,9 @@ async def _resolve_logging_exporters( info = parse_credential_info(credential.credential_info) if info is None or info.credential_type != "logging": return False - if info.auto_enable: - # auto_enable is scoped by access: if access has explicit grants, - # the request identity must fall within them. Empty access = proxy-wide. - from litellm.proxy.management_endpoints.logging_exporter_access import ( - _has_explicit_access_grants, - ) - - if _has_explicit_access_grants(info.access): - return access_grants(info.access, team_ids, org_ids) - return True # no grants = proxy-wide auto - if credential.credential_name not in names: + if not access_grants(info.access, team_ids, org_ids): return False - return access_grants(info.access, team_ids, org_ids) + return info.auto_enable or credential.credential_name in names def _build( credential: "CredentialItem", diff --git a/litellm/proxy/management_endpoints/logging_exporter_access.py b/litellm/proxy/management_endpoints/logging_exporter_access.py index 774fb20fa54..80af7b8a869 100644 --- a/litellm/proxy/management_endpoints/logging_exporter_access.py +++ b/litellm/proxy/management_endpoints/logging_exporter_access.py @@ -66,17 +66,6 @@ def access_grants( return not org_ids.isdisjoint(access.orgs) -def _has_explicit_access_grants(access: CredentialAccess | None) -> bool: - """True when ``access`` contains at least one explicit grant (global, team, or org). - - Used to distinguish "access intentionally left empty" (proxy-wide fallback) from - "access scoped to specific teams or orgs". - """ - if access is None: - return False - return access.global_ or bool(access.teams) or bool(access.orgs) - - def is_destination_visible( info: CredentialInfo, team_ids: frozenset[str], @@ -85,21 +74,8 @@ def is_destination_visible( """Whether a caller admin-scoped to ``team_ids`` / ``org_ids`` may see and assign this destination. - ``auto_enable`` is scoped by ``access``: - - If ``access`` has explicit grants (global / teams / orgs), the caller must - fall within those grants — even for auto-enabled destinations. - - If ``access`` is empty (no grants at all), the destination is treated as - proxy-wide and is visible to every admin caller. This preserves backward - compatibility for ``auto_enable=True`` destinations created without an - ``access`` block. - - A destination with ``auto_enable=False`` follows the same access check; the - only difference is that ``auto_enable=True`` without any explicit grants is - visible to all admins, while ``auto_enable=False`` without grants is visible - to nobody. + Visibility is decided entirely by ``access``: empty access grants no one, so an + empty-access destination is invisible regardless of ``auto_enable``. Proxy-wide + visibility must be requested explicitly with ``access.global = true``. """ - if _has_explicit_access_grants(info.access): - return access_grants(info.access, team_ids, org_ids) - # No explicit grants: auto_enable=True → proxy-wide (visible to all admins); - # auto_enable=False → invisible (no grants = not reachable by any non-admin). - return info.auto_enable + return access_grants(info.access, team_ids, org_ids) 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 535b93311e6..de7f9e0dd03 100644 --- a/tests/test_litellm/proxy/credential_endpoints/test_access_decision.py +++ b/tests/test_litellm/proxy/credential_endpoints/test_access_decision.py @@ -293,13 +293,13 @@ 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.""" +class TestEmptyingGrantsIsAllowed: + """Empty access is deny-all (a destination with no grants routes to no one), + so a team-admin emptying their own grant merely DISABLES the destination and + can never widen it. The decider allows it for every combination; there is no + special auto_enable case, because empty access is not proxy-wide.""" - def test_emptying_sole_grant_on_auto_enable_destination_is_denied(self): + def test_emptying_sole_own_grant_on_auto_enable_destination_is_allowed(self): existing = { **_EXISTING_INFO, "auto_enable": True, @@ -309,22 +309,9 @@ class TestAutoEnableWideningGuard: 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): + def test_emptying_sole_own_grant_without_auto_enable_is_allowed(self): existing = { **_EXISTING_INFO, "auto_enable": False, @@ -336,18 +323,20 @@ class TestAutoEnableWideningGuard: ) assert isinstance(d, Allow) - def test_proxy_admin_may_empty_sole_grant_on_auto_enable_destination(self): + def test_emptying_teams_still_cannot_drop_a_foreign_grant(self): + """Allowing empty-out does not weaken the foreign-revoke guard: a team-admin + still cannot remove a team they do not administer, auto_enable or not.""" existing = { **_EXISTING_INFO, "auto_enable": True, - "access": {"global": False, "teams": ["team-T"], "orgs": []}, + "access": {"global": False, "teams": ["team-T", "team-A"], "orgs": []}, } d = _decision( - is_proxy_admin=True, existing_info=existing, - patch_info={"access": {"teams": []}}, + patch_info={"access": {"teams": []}}, # drops team-A (foreign) too ) - assert isinstance(d, Allow) + assert isinstance(d, Deny) + assert "team-A" not in d.reason class TestTeamAdminMultipleTeams: diff --git a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py index 8ad30a3b9df..4de81a42d48 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py +++ b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py @@ -99,29 +99,28 @@ def test_access_grants_not_global_when_false(): assert access_grants(a, frozenset({"t1"}), frozenset({"o1"})) is False -# --- is_destination_visible: auto_enable scoped by access ------------------ +# --- is_destination_visible: decided entirely by access -------------------- # -# auto_enable=True means "selected automatically" but the scope of that -# automatic selection is controlled by access: -# - explicit grants (global/teams/orgs) → caller must be within the grant -# - empty access (no grants at all) → proxy-wide fallback (visible to all) -# This lets admins create a team-scoped auto-exporter without it leaking to -# every other team on the proxy. +# Visibility is access-only. auto_enable does not affect it: an empty-access +# destination is invisible regardless of auto_enable (empty access = deny-all). +# Proxy-wide visibility must be requested explicitly with access.global=True. -def test_visible_auto_enable_empty_access_is_proxy_wide(): - """auto_enable=True with no access grants is proxy-wide: visible to all admins.""" +def test_visible_empty_access_is_deny_all_even_with_auto_enable(): + """Empty access grants no one, even when auto_enable=True: not proxy-wide.""" info = CredentialInfo(credential_type="logging", auto_enable=True) - assert is_destination_visible(info, frozenset(), frozenset()) is True - assert is_destination_visible(info, frozenset({"any-team"}), frozenset()) is True - assert is_destination_visible(info, frozenset(), frozenset({"any-org"})) is True + assert is_destination_visible(info, frozenset(), frozenset()) is False + assert is_destination_visible(info, frozenset({"any-team"}), frozenset()) is False + assert is_destination_visible(info, frozenset(), frozenset({"any-org"})) is False -def test_visible_auto_enable_global_access_is_proxy_wide(): - """auto_enable=True + access.global=True is proxy-wide.""" +def test_visible_global_access_is_proxy_wide(): + """access.global=True is proxy-wide regardless of auto_enable.""" info = CredentialInfo(credential_type="logging", auto_enable=True, access=_access(global_=True)) assert is_destination_visible(info, frozenset({"t1"}), frozenset()) is True assert is_destination_visible(info, frozenset(), frozenset()) is True + manual = CredentialInfo(credential_type="logging", access=_access(global_=True)) + assert is_destination_visible(manual, frozenset(), frozenset()) is True def test_visible_auto_enable_team_scoped(): diff --git a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_validation.py b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_validation.py index 6609fb6703a..a0af9de508e 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_validation.py +++ b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_validation.py @@ -50,7 +50,8 @@ def _registry(): "access": {"teams": ["ds-team"], "orgs": ["ds-org"]}, }, ), - # explicit global/default: assignable by anyone via the auto_enable escape. + # proxy-wide auto default: access.global makes it visible to every scope, + # auto_enable makes it fire without being named. CredentialItem( credential_name="central-default", credential_values={}, @@ -58,6 +59,7 @@ def _registry(): "credential_type": "logging", "description": "arize", "auto_enable": True, + "access": {"global": True}, }, ), CredentialItem( @@ -195,8 +197,8 @@ def test_proxy_admin_can_assign_any_destination(_registry): def test_team_admin_can_assign_auto_enable_default(_registry): - """An explicit global/default (auto_enable) is assignable by any admin scope, - the way a global destination is.""" + """A proxy-wide auto default (access.global + auto_enable) is visible to every + scope, so a team admin in any team may name it.""" validate_logging_exporter_assignment( _ok(["central-default"]), _non_admin(), diff --git a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py index 2f2271514f1..570a25e840a 100644 --- a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py +++ b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py @@ -5442,10 +5442,11 @@ _ARIZE_ENDPOINT = "https://otlp.arize.com/v1" @pytest.fixture def _seeded_logging_credentials_with_access(): - """``access`` is visibility, not enablement. ``langfuse-eu`` is granted to - ``team-eu``/``org-eu`` but never auto-fires; ``arize-global`` carries - ``access.global`` to prove global visibility alone STILL does not auto-fire; - ``arize-default`` is the explicit ``auto_enable`` global/default.""" + """``access`` gates enablement. ``langfuse-eu`` is granted to + ``team-eu``/``org-eu`` but never auto-fires (not auto_enable, not named); + ``arize-global`` carries ``access.global`` to prove global visibility alone + STILL does not auto-fire; ``arize-default`` is the proxy-wide auto default + (``auto_enable`` + ``access.global``). Empty access would be deny-all.""" from litellm.models.credentials import CredentialItem original = litellm.credential_list @@ -5479,6 +5480,7 @@ def _seeded_logging_credentials_with_access(): "credential_type": "logging", "description": "arize", "auto_enable": True, + "access": {"global": True}, }, ), ] @@ -5535,6 +5537,34 @@ async def test_resolve_name_without_visibility_is_dropped( assert {d["endpoint"] for d in destinations} == {_ARIZE_ENDPOINT} +@pytest.mark.asyncio +async def test_resolve_auto_enable_empty_access_is_deny_all(monkeypatch): + """The core of the empty-access hardening: an auto_enable destination with no + access grants fires for NO ONE (empty access = deny-all, not proxy-wide). + Mutating the resolver to treat empty access as proxy-wide re-fires it here.""" + from litellm.models.credentials import CredentialItem + from litellm.proxy.litellm_pre_call_utils import _resolve_logging_exporters + + original = litellm.credential_list + litellm.credential_list = [ + CredentialItem( + credential_name="arize-empty-auto", + credential_values={"arize_space_id": "E", "arize_api_key": "K"}, + credential_info={ + "credential_type": "logging", + "description": "arize", + "auto_enable": True, + }, + ), + ] + try: + for auth in (_auth(team_id="team-x"), _auth(org_id="org-y"), _auth()): + destinations, _ = await _resolve_logging_exporters(auth) + assert destinations == [] + finally: + litellm.credential_list = original + + @pytest.mark.asyncio async def test_resolve_access_global_alone_does_not_fire( _seeded_logging_credentials_with_access, diff --git a/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTable.test.tsx b/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTable.test.tsx index 0c4abfbcdcb..74b82f36702 100644 --- a/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTable.test.tsx +++ b/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTable.test.tsx @@ -177,6 +177,26 @@ describe("LoggingCallbacksTable", () => { expect(screen.getByText("Auto-enabled")).toBeInTheDocument(); }); + it("renders disabled mode for an auto-enable destination with no access grants", () => { + render( + , + ); + expect(screen.getByText("Disabled")).toBeInTheDocument(); + expect(screen.queryByText("Auto-enabled")).not.toBeInTheDocument(); + }); + it("a destination row edits access and deletes without exposing callback actions", async () => { const user = userEvent.setup(); const onEditAccess = vi.fn(); diff --git a/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTableColumns.tsx b/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTableColumns.tsx index 48d77c406c4..3be6b95726c 100644 --- a/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTableColumns.tsx +++ b/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTableColumns.tsx @@ -55,10 +55,22 @@ function destinationMode(record: AlertingObject) { (access?.teams?.length ?? 0) > 0, (access?.orgs?.length ?? 0) > 0, ].some(Boolean); - const tooltip = hasExplicitGrants - ? "Exports automatically for all identities within the access scope without requiring explicit assignment." - : "No explicit access grants. Treated as proxy-wide automatic export for backward compatibility. Add access.global=true or access.teams/orgs to scope this destination."; - return ; + if (!hasExplicitGrants) { + return ( + + ); + } + return ( + + ); } function ScopeCell({ callback }: { callback: AlertingObject }) {