mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
fix(mcp): redact nested env_vars in submissions telemetry response
management_endpoint_wrapper serializes management responses into OTEL spans, and _redact_env_var_values only scrubbed env_vars at the top level. GET /v1/mcp/server/submissions returns MCPSubmissionsSummary with items[].env_vars carrying decrypted scope="global" values for full admins, so those upstream credentials were stringified into the response.items span attribute and readable by observability users on the legacy OTEL path. Walk the items list too and blank each record's env_vars values, copying the record (model_copy / dict spread) so the HTTP response the admin gets back keeps the decrypted values for the edit form. Regression test asserts the nested secret never reaches the span while names and scopes survive; reverting the items walk makes it fail
This commit is contained in:
parent
82e69edd3e
commit
eaab1c5665
2 changed files with 121 additions and 20 deletions
|
|
@ -5,6 +5,7 @@ from functools import wraps
|
|||
from typing import Any, Callable, List, Optional, Tuple
|
||||
|
||||
from fastapi import HTTPException, Request
|
||||
from pydantic import BaseModel
|
||||
|
||||
import litellm
|
||||
from litellm._logging import verbose_logger
|
||||
|
|
@ -436,31 +437,56 @@ async def send_management_endpoint_alert(
|
|||
)
|
||||
|
||||
|
||||
def _redacted_env_var(entry: Any) -> dict:
|
||||
get = entry.get if isinstance(entry, dict) else lambda k: getattr(entry, k, None)
|
||||
return {
|
||||
"name": get("name"),
|
||||
"scope": get("scope"),
|
||||
"description": get("description"),
|
||||
"value": "",
|
||||
}
|
||||
|
||||
|
||||
def _redact_record_env_vars(record: Any) -> Any:
|
||||
"""Return ``record`` with its ``env_vars[].value`` blanked.
|
||||
|
||||
Copies rather than mutating, because the record aliases the live response
|
||||
object that is also returned to the caller. Records without an ``env_vars``
|
||||
list are returned unchanged.
|
||||
"""
|
||||
env_vars = (
|
||||
record.get("env_vars")
|
||||
if isinstance(record, dict)
|
||||
else getattr(record, "env_vars", None)
|
||||
)
|
||||
if not isinstance(env_vars, list):
|
||||
return record
|
||||
redacted = [_redacted_env_var(entry) for entry in env_vars]
|
||||
if isinstance(record, dict):
|
||||
return {**record, "env_vars": redacted}
|
||||
if isinstance(record, BaseModel):
|
||||
return record.model_copy(update={"env_vars": redacted})
|
||||
return record
|
||||
|
||||
|
||||
def _redact_env_var_values(response: dict) -> None:
|
||||
"""Blank ``env_vars[].value`` in a management response before telemetry.
|
||||
|
||||
MCP create/update endpoints return decrypted ``scope="global"`` env var
|
||||
values so the admin UI can pre-fill the edit form; those values are
|
||||
upstream credentials and must not be serialized verbatim into OTEL spans,
|
||||
where an observability user could read them. Names, scopes, and
|
||||
descriptions are kept so traces stay useful.
|
||||
MCP endpoints return decrypted ``scope="global"`` env var values so the admin
|
||||
UI can pre-fill the edit form; those values are upstream credentials and must
|
||||
not be serialized verbatim into OTEL spans, where an observability user could
|
||||
read them. The values surface both at the top level (single-server
|
||||
create/update) and nested under ``items`` (the submissions queue), so both are
|
||||
scrubbed. Names, scopes, and descriptions are kept so traces stay useful.
|
||||
"""
|
||||
env_vars = response.get("env_vars")
|
||||
if not isinstance(env_vars, list):
|
||||
return
|
||||
if isinstance(response.get("env_vars"), list):
|
||||
response["env_vars"] = [
|
||||
_redacted_env_var(entry) for entry in response["env_vars"]
|
||||
]
|
||||
|
||||
def _redacted(entry: Any) -> dict:
|
||||
get = (
|
||||
entry.get if isinstance(entry, dict) else lambda k: getattr(entry, k, None)
|
||||
)
|
||||
return {
|
||||
"name": get("name"),
|
||||
"scope": get("scope"),
|
||||
"description": get("description"),
|
||||
"value": "",
|
||||
}
|
||||
|
||||
response["env_vars"] = [_redacted(entry) for entry in env_vars]
|
||||
items = response.get("items")
|
||||
if isinstance(items, list):
|
||||
response["items"] = [_redact_record_env_vars(item) for item in items]
|
||||
|
||||
|
||||
async def _emit_management_endpoint_otel_span(
|
||||
|
|
|
|||
|
|
@ -92,6 +92,81 @@ async def test_management_otel_span_redacts_mcp_global_env_var_secrets(monkeypat
|
|||
assert result.env_vars[0].value == secret
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_management_otel_span_redacts_nested_submission_env_var_secrets(
|
||||
monkeypatch,
|
||||
):
|
||||
"""Decrypted global env var secrets nested under ``items`` must also be blanked.
|
||||
|
||||
``GET /v1/mcp/server/submissions`` returns ``MCPSubmissionsSummary`` whose
|
||||
``items[].env_vars`` carry decrypted ``scope="global"`` values for full admins.
|
||||
``management_endpoint_wrapper`` stringifies that nested ``items`` value into the
|
||||
OTEL span, so redaction has to walk into ``items`` and not just the top level,
|
||||
while the endpoint's own return value keeps the value for the admin UI.
|
||||
"""
|
||||
import datetime
|
||||
|
||||
from litellm.proxy._types import (
|
||||
LiteLLM_MCPServerTable,
|
||||
MCPEnvVar,
|
||||
MCPEnvVarScope,
|
||||
MCPSubmissionsSummary,
|
||||
)
|
||||
from litellm.proxy.management_helpers import utils as mgmt_utils
|
||||
|
||||
captured = {}
|
||||
|
||||
class _FakeOtelLogger:
|
||||
async def async_management_endpoint_success_hook(
|
||||
self, logging_payload, parent_otel_span
|
||||
):
|
||||
captured["response"] = logging_payload.response
|
||||
|
||||
import litellm.proxy.proxy_server as proxy_server
|
||||
|
||||
monkeypatch.setattr(proxy_server, "open_telemetry_logger", _FakeOtelLogger())
|
||||
monkeypatch.setattr(mgmt_utils, "is_otel_v2_enabled", lambda: False)
|
||||
|
||||
secret = "s3cr3t-submission"
|
||||
server = LiteLLM_MCPServerTable(
|
||||
server_id="srv-sub",
|
||||
alias="echo",
|
||||
url="http://localhost:8765/mcp",
|
||||
transport="http",
|
||||
env_vars=[
|
||||
MCPEnvVar(name="DB_PASSWORD", value=secret, scope=MCPEnvVarScope.global_),
|
||||
],
|
||||
created_at=datetime.datetime.now(),
|
||||
updated_at=datetime.datetime.now(),
|
||||
)
|
||||
result = MCPSubmissionsSummary(
|
||||
total=1, pending_review=1, active=0, rejected=0, items=[server]
|
||||
)
|
||||
|
||||
await mgmt_utils._emit_management_endpoint_otel_span(
|
||||
func=lambda: None,
|
||||
kwargs={},
|
||||
parent_otel_span=object(),
|
||||
start_time=datetime.datetime.now(),
|
||||
end_time=datetime.datetime.now(),
|
||||
result=result,
|
||||
)
|
||||
|
||||
# The nested secret must not appear anywhere the span serializer stringifies.
|
||||
assert secret not in str(captured["response"])
|
||||
|
||||
redacted_item = captured["response"]["items"][0]
|
||||
redacted_env_vars = (
|
||||
redacted_item["env_vars"]
|
||||
if isinstance(redacted_item, dict)
|
||||
else redacted_item.env_vars
|
||||
)
|
||||
assert [entry["value"] for entry in redacted_env_vars] == [""]
|
||||
assert redacted_env_vars[0]["name"] == "DB_PASSWORD"
|
||||
# The endpoint's own return value is untouched for the admin UI.
|
||||
assert result.items[0].env_vars[0].value == secret
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_new_member_clones_default_team_budget_id():
|
||||
"""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue