scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-10-10 03:28:53 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 85

Merge pull request #26819 from stuxf/fix/team-callback-idor

Browse source 
chore(team): require team-management role on /team/{id}/callback endpoints
This commit is contained in:
yuneng-jiang 2026-05-04 14:31:59 -07:00 • committed by GitHub
commit c064170a18
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 406 additions and 72 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
litellm/proxy/management_endpoints/organization_endpoints.py
View file

@ -500,6 +500,21 @@ async def update_organization(
if data.updated_by is None:
data.updated_by = user_api_key_dict.user_id
if data.organization_id is None:
raise HTTPException(
status_code=400,
detail={"error": "organization_id is required"},
)
# IDOR guard: only proxy admins / org admins of THIS org may update
# it. Without this, any authenticated key holder could rewrite
# another organization's metadata, budgets, and object permissions.
await _verify_org_access(
organization_id=data.organization_id,
user_api_key_dict=user_api_key_dict,
prisma_client=prisma_client,
)
existing_organization_row = (
await prisma_client.db.litellm_organizationtable.find_unique(
where={"organization_id": data.organization_id},
@ -909,6 +924,16 @@ async def organization_member_add(
if prisma_client is None:
raise HTTPException(status_code=500, detail={"error": "No db connected"})
# IDOR guard: docstring says "Only proxy_admin or org_admin of
# organization, allowed to access this endpoint" — but the code
# never enforced that. Any authenticated key holder could add
# members to any org. Now gated explicitly.
await _verify_org_access(
organization_id=data.organization_id,
user_api_key_dict=user_api_key_dict,
prisma_client=prisma_client,
)
# Check if organization exists
existing_organization_row = (
await prisma_client.db.litellm_organizationtable.find_unique(
@ -1018,6 +1043,16 @@ async def organization_member_update(
detail={"error": CommonProxyErrors.db_not_connected_error.value},
)
# IDOR guard: only proxy admins / org admins of THIS org may
# update member roles. The PROXY_ADMIN-target check below was
# the only access control; without this, any authenticated user
# could change any non-admin member's role in any org.
await _verify_org_access(
organization_id=data.organization_id,
user_api_key_dict=user_api_key_dict,
prisma_client=prisma_client,
)
# Check if organization exists
existing_organization_row = (
await prisma_client.db.litellm_organizationtable.find_unique(
@ -1179,6 +1214,15 @@ async def organization_member_delete(
detail={"error": CommonProxyErrors.db_not_connected_error.value},
)
# IDOR guard: only proxy admins / org admins of THIS org may
# delete members. Without this, any authenticated key holder
# could remove any user from any org.
await _verify_org_access(
organization_id=data.organization_id,
user_api_key_dict=user_api_key_dict,
prisma_client=prisma_client,
)
if data.user_email is not None and data.user_id is None:
existing_user_email_row = await find_member_if_email(
data.user_email, prisma_client

50
litellm/proxy/management_endpoints/team_callback_endpoints.py
View file

@ -19,6 +19,7 @@ from litellm._uuid import uuid
from litellm.proxy._types import (
AddTeamCallback,
LiteLLM_AuditLogs,
LiteLLM_TeamTable,
LitellmTableNames,
ProxyErrorTypes,
ProxyException,
@ -26,6 +27,7 @@ from litellm.proxy._types import (
UserAPIKeyAuth,
)
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
from litellm.proxy.management_endpoints.team_endpoints import _verify_team_access
from litellm.proxy.management_helpers.utils import management_endpoint_wrapper
router = APIRouter()
@ -207,6 +209,15 @@ async def add_team_callbacks(
},
)
# IDOR guard: only proxy admins / org admins / team admins of THIS
# team may write callback credentials. Without this, any
# authenticated key holder could overwrite another team's logging
# config (and read back the credentials they wrote).
await _verify_team_access(
team_obj=LiteLLM_TeamTable(**_existing_team.model_dump()),
user_api_key_dict=user_api_key_dict,
)
# store team callback settings in metadata
team_metadata = _existing_team.metadata
team_callback_settings: List[dict] = team_metadata.get(
@ -316,6 +327,14 @@ async def disable_team_logging(
detail={"error": f"Team id = {team_id} does not exist."},
)
# IDOR guard: only proxy admins / org admins / team admins of THIS
# team may disable its logging — otherwise any authenticated key
# holder can silence audit logging for any team.
await _verify_team_access(
team_obj=LiteLLM_TeamTable(**_existing_team.model_dump()),
user_api_key_dict=user_api_key_dict,
)
# Update team metadata to disable logging
team_metadata = _existing_team.metadata
before_metadata = copy.deepcopy(team_metadata)
@ -364,20 +383,18 @@ async def disable_team_logging(
},
}
except HTTPException:
# Legitimate 4xx (e.g. 403 from the access guard, 404 for an
# unknown team). Re-raise without the error-level log noise that
# the catch-all branch below would produce.
raise
except ProxyException:
raise
except Exception as e:
verbose_proxy_logger.error(
f"litellm.proxy.proxy_server.disable_team_logging(): Exception occurred - {str(e)}"
)
verbose_proxy_logger.debug(traceback.format_exc())
if isinstance(e, HTTPException):
raise ProxyException(
message=getattr(e, "detail", f"Internal Server Error({str(e)})"),
type=ProxyErrorTypes.internal_server_error.value,
param=getattr(e, "param", "None"),
code=getattr(e, "status_code", status.HTTP_500_INTERNAL_SERVER_ERROR),
)
elif isinstance(e, ProxyException):
raise e
raise ProxyException(
message="Internal Server Error, " + str(e),
type=ProxyErrorTypes.internal_server_error.value,
@ -437,6 +454,14 @@ async def get_team_callbacks(
detail={"error": f"Team id = {team_id} does not exist."},
)
# IDOR guard: callback metadata holds third-party API credentials
# (Langfuse / Langsmith / GCS). Only proxy admins / org admins /
# team admins of THIS team may read them.
await _verify_team_access(
team_obj=LiteLLM_TeamTable(**_existing_team.model_dump()),
user_api_key_dict=user_api_key_dict,
)
# Retrieve team callback settings from metadata
team_metadata = _existing_team.metadata
team_callback_settings = team_metadata.get("callback_settings", {})
@ -454,6 +479,13 @@ async def get_team_callbacks(
},
}
except HTTPException:
# Legitimate 4xx (e.g. 403 from the access guard) — re-raise
# without the error-level log noise that the catch-all below
# would produce.
raise
except ProxyException:
raise
except Exception as e:
verbose_proxy_logger.error(
"litellm.proxy.proxy_server.get_team_callbacks(): Exception occurred - {}".format(

130
tests/test_litellm/proxy/management_endpoints/test_organization_endpoints.py
View file

@ -565,3 +565,133 @@ async def test_organization_info_includes_user_email(monkeypatch):
membership = LiteLLM_OrganizationMembershipTable(**raw_membership)
assert membership.user_email == "alice@example.com"
# Regression tests for IDOR fixes on org-scoped endpoints. Sibling cluster
# to GHSA-xxv2-fprq-9x93 (team callback IDOR): the same shape of "any
# authenticated key holder reaches an endpoint that takes an
# organization_id from the request body without an access guard." The
# fix routes ``update_organization``, ``organization_member_add``,
# ``organization_member_update``, and ``organization_member_delete``
# through the existing ``_verify_org_access`` helper.
@pytest.fixture
def unauthorized_caller():
from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth
return UserAPIKeyAuth(
user_role=LitellmUserRoles.INTERNAL_USER,
user_id="random_authenticated_user",
api_key="sk-random",
)
@pytest.fixture
def patched_org_prisma():
"""Mock prisma so that find_unique returns a victim org and
get_user_object reports the caller has no org membership — so
_verify_org_access raises 403."""
victim_row = MagicMock()
victim_row.organization_id = "org-victim"
victim_row.metadata = {}
victim_row.model_dump.return_value = {"organization_id": "org-victim"}
caller_user = MagicMock()
caller_user.organization_memberships = [] # no admin role anywhere
with (
patch("litellm.proxy.proxy_server.prisma_client") as mock_prisma,
patch(
"litellm.proxy.management_endpoints.organization_endpoints.get_user_object",
new_callable=AsyncMock,
return_value=caller_user,
),
patch(
"litellm.proxy.proxy_server.user_api_key_cache",
),
patch("litellm.proxy.proxy_server.proxy_logging_obj"),
):
mock_prisma.db.litellm_organizationtable.find_unique = AsyncMock(
return_value=victim_row
)
yield mock_prisma
@pytest.mark.asyncio
async def test_organization_member_add_rejects_unauthorized_caller(
patched_org_prisma, unauthorized_caller
):
# ``organization_member_add`` catches HTTPException in its
# catch-all and re-wraps as ProxyException with the original status
# code preserved.
from litellm.proxy._types import (
OrganizationMemberAddRequest,
OrgMember,
ProxyException,
)
from litellm.proxy.management_endpoints.organization_endpoints import (
organization_member_add,
)
from unittest.mock import Mock
from fastapi import Request
data = OrganizationMemberAddRequest(
organization_id="org-victim",
member=OrgMember(role="internal_user", user_id="attacker-user"),
)
with pytest.raises((HTTPException, ProxyException)) as exc:
await organization_member_add(
data=data,
http_request=Mock(spec=Request),
user_api_key_dict=unauthorized_caller,
)
code = getattr(exc.value, "status_code", None) or getattr(exc.value, "code", None)
assert int(code) == 403
@pytest.mark.asyncio
async def test_organization_member_update_rejects_unauthorized_caller(
patched_org_prisma, unauthorized_caller
):
from litellm.proxy._types import OrganizationMemberUpdateRequest
from litellm.proxy.management_endpoints.organization_endpoints import (
organization_member_update,
)
data = OrganizationMemberUpdateRequest(
organization_id="org-victim",
user_id="some-other-user",
role="org_admin",
)
with pytest.raises(HTTPException) as exc:
await organization_member_update(
data=data,
user_api_key_dict=unauthorized_caller,
)
assert exc.value.status_code == 403
@pytest.mark.asyncio
async def test_organization_member_delete_rejects_unauthorized_caller(
patched_org_prisma, unauthorized_caller
):
from litellm.proxy._types import OrganizationMemberDeleteRequest
from litellm.proxy.management_endpoints.organization_endpoints import (
organization_member_delete,
)
data = OrganizationMemberDeleteRequest(
organization_id="org-victim",
user_id="some-other-user",
)
with pytest.raises(HTTPException) as exc:
await organization_member_delete(
data=data,
user_api_key_dict=unauthorized_caller,
)
assert exc.value.status_code == 403

254
tests/test_litellm/proxy/management_endpoints/test_team_callback_endpoints.py
View file

@ -1,71 +1,205 @@
"""
Audit-log emission for the team-callback admin endpoints.
Regression tests for team callback endpoint access control and audit logging.
The endpoints in ``team_callback_endpoints.py`` mutate a team's logging
callbacks (``add_team_callbacks``) or zero them out entirely
(``disable_team_logging``). Both are admin-only mutations, and the
disable variant is itself a logging-control action, so when the operator
has Enterprise audit logging enabled (``litellm.store_audit_logs = True``)
each call must emit a row that captures who did it and what the metadata
looked like before/after.
The team callback endpoints mutate or expose callback credentials. They must
enforce target-team management access and, when audit logging is enabled, emit
redacted audit rows for callback mutations.
"""
import json
from unittest.mock import AsyncMock, MagicMock, patch
from unittest.mock import AsyncMock, MagicMock, Mock, patch
import pytest
from fastapi import Request
from fastapi import HTTPException, Request
import litellm
from litellm.proxy._types import (
AddTeamCallback,
LitellmTableNames,
LitellmUserRoles,
UserAPIKeyAuth,
)
from litellm.proxy.management_endpoints.team_callback_endpoints import (
add_team_callbacks,
disable_team_logging,
get_team_callbacks,
)
def _team_row(
*,
team_id: str = "team-victim",
metadata: dict | None = None,
admin_user_id: str = "victim_admin",
organization_id: str = "org-victim",
) -> MagicMock:
row = MagicMock()
row.team_id = team_id
row.metadata = metadata or {}
row.model_dump.return_value = {
"team_id": team_id,
"team_alias": "victim-team",
"members_with_roles": [
{"role": "admin", "user_id": admin_user_id},
],
"organization_id": organization_id,
"metadata": row.metadata,
}
return row
def _patch_prisma(existing_team: MagicMock):
mock_prisma = MagicMock()
mock_prisma.get_data = AsyncMock(return_value=existing_team)
updated_row = MagicMock()
updated_row.team_id = existing_team.team_id
mock_prisma.db.litellm_teamtable.update = AsyncMock(return_value=updated_row)
return mock_prisma
def _admin_auth() -> UserAPIKeyAuth:
return UserAPIKeyAuth(
api_key="hashed",
user_id="admin-user",
user_role="proxy_admin",
user_role=LitellmUserRoles.PROXY_ADMIN,
)
def _existing_team_row(metadata: dict) -> MagicMock:
row = MagicMock()
row.team_id = "team-1"
row.metadata = metadata
return row
@pytest.fixture
def unauthorized_caller():
return UserAPIKeyAuth(
user_role=LitellmUserRoles.INTERNAL_USER,
user_id="random_authenticated_user",
api_key="sk-random",
)
def _patch_prisma(existing_metadata: dict):
"""Build a context-manager that patches the proxy's ``prisma_client``
to return ``existing_metadata`` from ``get_data`` and a stub team row
from ``litellm_teamtable.update``."""
mock_prisma = MagicMock()
mock_prisma.get_data = AsyncMock(return_value=_existing_team_row(existing_metadata))
@pytest.fixture
def patched_prisma():
with (
patch("litellm.proxy.proxy_server.prisma_client") as mock_client,
patch(
"litellm.proxy.management_endpoints.team_endpoints._is_user_org_admin_for_team",
new_callable=AsyncMock,
return_value=False,
),
):
mock_client.get_data = AsyncMock(return_value=_team_row())
mock_client.db.litellm_teamtable.update = AsyncMock()
yield mock_client
updated_row = MagicMock()
updated_row.team_id = "team-1"
mock_prisma.db.litellm_teamtable.update = AsyncMock(return_value=updated_row)
return mock_prisma
@pytest.mark.asyncio
async def test_add_team_callbacks_rejects_unauthorized_caller(
patched_prisma, unauthorized_caller
):
data = AddTeamCallback(
callback_name="langfuse",
callback_type="success",
callback_vars={
"langfuse_public_key": "pk-attacker",
"langfuse_secret_key": "sk-attacker",
},
)
with pytest.raises(HTTPException) as exc:
await add_team_callbacks(
data=data,
http_request=Mock(spec=Request),
team_id="team-victim",
user_api_key_dict=unauthorized_caller,
)
assert exc.value.status_code == 403
patched_prisma.db.litellm_teamtable.update.assert_not_called()
@pytest.mark.asyncio
async def test_disable_team_logging_rejects_unauthorized_caller(
patched_prisma, unauthorized_caller
):
with pytest.raises(HTTPException) as exc:
await disable_team_logging(
http_request=Mock(spec=Request),
team_id="team-victim",
user_api_key_dict=unauthorized_caller,
)
assert exc.value.status_code == 403
patched_prisma.db.litellm_teamtable.update.assert_not_called()
@pytest.mark.asyncio
async def test_get_team_callbacks_rejects_unauthorized_caller(
patched_prisma, unauthorized_caller
):
with pytest.raises(HTTPException) as exc:
await get_team_callbacks(
http_request=Mock(spec=Request),
team_id="team-victim",
user_api_key_dict=unauthorized_caller,
)
assert exc.value.status_code == 403
@pytest.mark.asyncio
async def test_proxy_admin_can_add_team_callbacks(patched_prisma):
data = AddTeamCallback(
callback_name="langfuse",
callback_type="success",
callback_vars={
"langfuse_public_key": "pk-admin",
"langfuse_secret_key": "sk-admin",
},
)
await add_team_callbacks(
data=data,
http_request=Mock(spec=Request),
team_id="team-victim",
user_api_key_dict=_admin_auth(),
)
patched_prisma.db.litellm_teamtable.update.assert_awaited_once()
@pytest.mark.asyncio
async def test_team_admin_of_target_team_can_add_callbacks(patched_prisma):
patched_prisma.get_data = AsyncMock(
return_value=_team_row(admin_user_id="team_admin_user")
)
team_admin = UserAPIKeyAuth(
user_role=LitellmUserRoles.INTERNAL_USER,
user_id="team_admin_user",
api_key="sk-team-admin",
)
data = AddTeamCallback(
callback_name="langfuse",
callback_type="success",
callback_vars={
"langfuse_public_key": "pk-team",
"langfuse_secret_key": "sk-team",
},
)
await add_team_callbacks(
data=data,
http_request=Mock(spec=Request),
team_id="team-victim",
user_api_key_dict=team_admin,
)
patched_prisma.db.litellm_teamtable.update.assert_awaited_once()
@pytest.mark.asyncio
async def test_disable_team_logging_emits_audit_log_when_enabled(monkeypatch):
monkeypatch.setattr(litellm, "store_audit_logs", True)
mock_prisma = _patch_prisma(
{
"callback_settings": {
"success_callback": ["langfuse"],
"failure_callback": [],
}
}
_team_row(
team_id="team-1",
metadata={
"callback_settings": {
"success_callback": ["langfuse"],
"failure_callback": [],
}
},
)
)
audit_calls = []
@ -87,8 +221,6 @@ async def test_disable_team_logging_emits_audit_log_when_enabled(monkeypatch):
user_api_key_dict=_admin_auth(),
litellm_changed_by=None,
)
# asyncio.create_task fires the coroutine eagerly; await one tick to let
# the audit-log emit run before the test exits.
import asyncio
for _ in range(3):
@ -103,9 +235,7 @@ async def test_disable_team_logging_emits_audit_log_when_enabled(monkeypatch):
before = json.loads(log.before_value)
after = json.loads(log.updated_values)
# Before: the team's pre-existing success_callback survives in the snapshot.
assert before["metadata"]["callback_settings"]["success_callback"] == ["langfuse"]
# After: callbacks zeroed out by the endpoint.
assert after["metadata"]["callback_settings"]["success_callback"] == []
assert after["metadata"]["callback_settings"]["failure_callback"] == []
@ -114,12 +244,15 @@ async def test_disable_team_logging_emits_audit_log_when_enabled(monkeypatch):
async def test_disable_team_logging_no_audit_when_disabled(monkeypatch):
monkeypatch.setattr(litellm, "store_audit_logs", False)
mock_prisma = _patch_prisma(
{
"callback_settings": {
"success_callback": ["langfuse"],
"failure_callback": [],
}
}
_team_row(
team_id="team-1",
metadata={
"callback_settings": {
"success_callback": ["langfuse"],
"failure_callback": [],
}
},
)
)
audit_calls = []
@ -147,7 +280,7 @@ async def test_disable_team_logging_no_audit_when_disabled(monkeypatch):
@pytest.mark.asyncio
async def test_add_team_callbacks_emits_audit_log_when_enabled(monkeypatch):
monkeypatch.setattr(litellm, "store_audit_logs", True)
mock_prisma = _patch_prisma({"logging": []})
mock_prisma = _patch_prisma(_team_row(team_id="team-1", metadata={"logging": []}))
audit_calls = []
@ -186,7 +319,6 @@ async def test_add_team_callbacks_emits_audit_log_when_enabled(monkeypatch):
assert log.table_name == LitellmTableNames.TEAM_TABLE_NAME
assert log.object_id == "team-1"
assert log.action == "updated"
# ``litellm_changed_by`` header takes precedence over the auth user_id.
assert log.changed_by == "ops-on-call"
before = json.loads(log.before_value)
@ -195,34 +327,32 @@ async def test_add_team_callbacks_emits_audit_log_when_enabled(monkeypatch):
assert len(after["metadata"]["logging"]) == 1
assert after["metadata"]["logging"][0]["callback_name"] == "langfuse"
# Callback secrets MUST NOT leak into the audit log payload.
callback_vars = after["metadata"]["logging"][0]["callback_vars"]
assert callback_vars["langfuse_public_key"] != "pk"
assert callback_vars["langfuse_secret_key"] != "sk"
# Key names are preserved so the auditor can see which fields changed.
assert "langfuse_public_key" in callback_vars
assert "langfuse_secret_key" in callback_vars
# And no plaintext secret should appear anywhere in the serialized row.
assert "sk" not in log.updated_values.replace("sk-", "") # crude leak check
assert "sk" not in log.updated_values.replace("sk-", "")
assert "pk" not in (log.updated_values.replace("pk-", "").replace("public_key", ""))
@pytest.mark.asyncio
async def test_disable_team_logging_redacts_existing_callback_secrets(monkeypatch):
monkeypatch.setattr(litellm, "store_audit_logs", True)
# Existing team has populated callback_vars containing secrets — redaction
# must apply to the BEFORE snapshot too.
mock_prisma = _patch_prisma(
{
"callback_settings": {
"success_callback": ["langfuse"],
"failure_callback": [],
"callback_vars": {
"langfuse_public_key": "pk-real",
"langfuse_secret_key": "sk-real-secret",
},
}
}
_team_row(
team_id="team-1",
metadata={
"callback_settings": {
"success_callback": ["langfuse"],
"failure_callback": [],
"callback_vars": {
"langfuse_public_key": "pk-real",
"langfuse_secret_key": "sk-real-secret",
},
}
},
)
)
audit_calls = []
@ -251,8 +381,6 @@ async def test_disable_team_logging_redacts_existing_callback_secrets(monkeypatc
assert len(audit_calls) == 1
log = audit_calls[0]
# The pre-existing secret_key value must NOT appear in the serialized
# before_value or updated_values.
assert "sk-real-secret" not in log.before_value
assert "sk-real-secret" not in log.updated_values
assert "pk-real" not in log.before_value
@ -262,7 +390,7 @@ async def test_disable_team_logging_redacts_existing_callback_secrets(monkeypatc
@pytest.mark.asyncio
async def test_add_team_callbacks_no_audit_when_disabled(monkeypatch):
monkeypatch.setattr(litellm, "store_audit_logs", False)
mock_prisma = _patch_prisma({"logging": []})
mock_prisma = _patch_prisma(_team_row(team_id="team-1", metadata={"logging": []}))
audit_calls = []