mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
fix(mcp): redact global env var secrets from management telemetry
Management endpoints that create or update an MCP server return the server with decrypted scope=global env var values so the admin edit form can be pre-filled. management_endpoint_wrapper serializes that response into an OTEL span and only filtered top-level credential fields, so the nested env_vars list reached the trace verbatim and an observability user could read upstream API keys. Blank env_vars[].value in the telemetry response while keeping names and scopes; the endpoint's own return value is untouched so the admin still receives the decrypted values.
This commit is contained in:
parent
43394c198d
commit
86cf341c3c
2 changed files with 101 additions and 0 deletions
|
|
@ -436,6 +436,33 @@ async def send_management_endpoint_alert(
|
|||
)
|
||||
|
||||
|
||||
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.
|
||||
"""
|
||||
env_vars = response.get("env_vars")
|
||||
if not isinstance(env_vars, list):
|
||||
return
|
||||
|
||||
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]
|
||||
|
||||
|
||||
async def _emit_management_endpoint_otel_span(
|
||||
func: Callable,
|
||||
kwargs: dict,
|
||||
|
|
@ -497,6 +524,7 @@ async def _emit_management_endpoint_otel_span(
|
|||
try:
|
||||
raw = dict(result)
|
||||
_response = {k: v for k, v in raw.items() if k not in _CREDENTIAL_FIELDS}
|
||||
_redact_env_var_values(_response)
|
||||
except Exception:
|
||||
_response = None
|
||||
|
||||
|
|
|
|||
|
|
@ -19,6 +19,79 @@ from litellm.proxy._types import (
|
|||
from litellm.proxy.management_helpers.utils import add_new_member
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_management_otel_span_redacts_mcp_global_env_var_secrets(monkeypatch):
|
||||
"""A decrypted MCP global env var secret must never reach telemetry.
|
||||
|
||||
MCP create/update endpoints return the server with decrypted
|
||||
``scope="global"`` env var values so the admin UI can pre-fill the edit
|
||||
form. ``management_endpoint_wrapper`` serializes the response into an OTEL
|
||||
span, and that span is readable by observability users, so the secret value
|
||||
must be blanked there while names/scopes stay for usefulness. The endpoint's
|
||||
own return value must keep the decrypted value for the admin.
|
||||
"""
|
||||
import datetime
|
||||
|
||||
from litellm.proxy._types import (
|
||||
LiteLLM_MCPServerTable,
|
||||
MCPEnvVar,
|
||||
MCPEnvVarScope,
|
||||
)
|
||||
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-p@ss"
|
||||
result = LiteLLM_MCPServerTable(
|
||||
server_id="srv-1",
|
||||
alias="echo",
|
||||
url="http://localhost:8765/mcp",
|
||||
transport="http",
|
||||
env_vars=[
|
||||
MCPEnvVar(name="DB_PASSWORD", value=secret, scope=MCPEnvVarScope.global_),
|
||||
MCPEnvVar(
|
||||
name="CORP_USER",
|
||||
value="",
|
||||
scope=MCPEnvVarScope.user,
|
||||
description="Your DB username",
|
||||
),
|
||||
],
|
||||
created_at=datetime.datetime.now(),
|
||||
updated_at=datetime.datetime.now(),
|
||||
)
|
||||
|
||||
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,
|
||||
)
|
||||
|
||||
serialized = captured["response"]["env_vars"]
|
||||
# The secret must not appear anywhere the span serializer would stringify.
|
||||
assert secret not in str(captured["response"])
|
||||
assert all(entry["value"] == "" for entry in serialized)
|
||||
# Names and scopes survive so the trace stays useful.
|
||||
assert {entry["name"] for entry in serialized} == {"DB_PASSWORD", "CORP_USER"}
|
||||
assert any(entry["scope"] == MCPEnvVarScope.global_ for entry in serialized)
|
||||
# The endpoint's own return value is untouched: the admin still gets the
|
||||
# decrypted value to pre-fill the edit form.
|
||||
assert result.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