mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(credentials): empty access is deny-all; proxy-wide requires access.global
An empty access grant on a logging destination now means no one can use it, for both visibility and request-time routing, regardless of auto_enable. Previously an auto_enable destination with empty access was treated as proxy-wide, an implicit allow-all that let a team-admin widen a scoped destination by emptying its grants and let a client-invisible destination fan out to every tenant. Proxy-wide export now requires an explicit access.global=true (already proxy-admin-only). Removes the auto_enable empty-access fallback from is_destination_visible and the request resolver, deletes the now-unused _has_explicit_access_grants helper, and drops the last-grant-removal guard in decide_credential_patch that only existed to contain the old widening behavior. Emptying the last grant is now allowed and simply disables the destination. auto_enable is unchanged in meaning (fires without being named, scoped by access) and the logging_exporters assignment path is untouched. UI Mode cell renders Disabled for an auto_enable destination with no access grants.
This commit is contained in:
parent
3d6951476b
commit
65dd6b8376
9 changed files with 119 additions and 111 deletions
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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():
|
||||
|
|
|
|||
|
|
@ -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(),
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
<LoggingCallbacksTable
|
||||
callbacks={[
|
||||
{
|
||||
name: "otel-empty",
|
||||
variables: baseVars,
|
||||
credentialName: "otel-empty",
|
||||
access: { global: false, teams: [], orgs: [] },
|
||||
autoEnable: true,
|
||||
resolvedScope: { global: false, teams: [], orgs: [] },
|
||||
},
|
||||
]}
|
||||
availableCallbacks={{}}
|
||||
/>,
|
||||
);
|
||||
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();
|
||||
|
|
|
|||
|
|
@ -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 <StatusBadge tone="warning" label="Auto-enabled" tooltip={tooltip} />;
|
||||
if (!hasExplicitGrants) {
|
||||
return (
|
||||
<StatusBadge
|
||||
tone="neutral"
|
||||
label="Disabled"
|
||||
tooltip="No access grants, so this destination receives nothing. Add Access (Global, or specific Teams/Orgs) to enable it."
|
||||
/>
|
||||
);
|
||||
}
|
||||
return (
|
||||
<StatusBadge
|
||||
tone="warning"
|
||||
label="Auto-enabled"
|
||||
tooltip="Exports automatically for all identities within the access scope without requiring explicit assignment."
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
function ScopeCell({ callback }: { callback: AlertingObject }) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue