mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
fix(proxy): stop leaking internal exception details to clients (#39380)
* fix(proxy): stop leaking internal exception details to clients Public error responses could disclose internal details in two places. A proxy-layer exception with no recognized provider status code (a bug in a custom callback, a hook, or litellm's own code) forwarded its raw str() text verbatim on a 5xx, including any embedded credential, filesystem path, or internal hostname, or a full stack trace; the same client-facing message now runs through a redaction layer built on top of the credential redaction that already runs on log output, so it also drops an embedded traceback and scrubs path-shaped and hostname-shaped substrings. It intentionally never runs on server-side logs, which must keep full detail for debugging. exception_type(), litellm's core exception mapper, is shared by direct SDK callers (litellm.completion()) and the proxy, and it deliberately embeds a traceback into an unmapped exception's message as a debugging aid for library users; a first pass at this fix stripped that traceback inside exception_type() itself and broke that convention (caught by tests asserting on the traceback frame). The traceback stays in exception_type()'s own output; only the proxy's client-facing response boundary (and the streaming response generator, which never needs to embed one at all) strips it. Full generic-message replacement for the unclassified-exception case was tried first and reverted too: several routes deliberately raise a bare exception as an informative, secret-free validation message (e.g. the OCR endpoint's rejection of provider-native file IDs), and replacing those wholesale broke that convention; targeted redaction leaves them untouched. Also stops the default uvicorn-based proxy from sending a Server response header. Resolves LIT-6747 * refactor(proxy): drop the unrelated error-message constant and trim redaction comments Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
5ad330f620
commit
7978b9f721
9 changed files with 202 additions and 12 deletions
|
|
@ -17,7 +17,11 @@ from litellm.constants import (
|
|||
from litellm.litellm_core_utils.env_utils import get_env_int
|
||||
from litellm.litellm_core_utils.safe_json_dumps import safe_dumps
|
||||
from litellm.litellm_core_utils.safe_json_loads import safe_json_loads
|
||||
from litellm.litellm_core_utils.secret_redaction import redact_string, redact_structured_value
|
||||
from litellm.litellm_core_utils.secret_redaction import (
|
||||
redact_internal_details,
|
||||
redact_string,
|
||||
redact_structured_value,
|
||||
)
|
||||
|
||||
set_verbose = False
|
||||
|
||||
|
|
@ -89,6 +93,14 @@ def redact_secrets(value: str) -> str:
|
|||
return _redact_string(value)
|
||||
|
||||
|
||||
def redact_internal_details_from_client_message(value: str) -> str:
|
||||
"""Public API: redact_secrets() plus filesystem paths, internal hostnames, and an
|
||||
embedded traceback, for a string about to leave the process in an HTTP response."""
|
||||
if not _ENABLE_SECRET_REDACTION:
|
||||
return value
|
||||
return redact_internal_details(value)
|
||||
|
||||
|
||||
def _substituted_color_message(record: logging.LogRecord) -> str | None:
|
||||
"""Render a record's ``color_message`` against its args, or None if absent.
|
||||
|
||||
|
|
|
|||
|
|
@ -92,6 +92,27 @@ def redact_string(value: str) -> str:
|
|||
return _SECRET_RE.sub(_REDACTED, value)
|
||||
|
||||
|
||||
_UNIX_SYSTEM_PATH: Final = r"/(?:etc|var|opt|usr|home|root|private|Users|tmp|mnt|srv)/[^\s'\"\)\]}>,]+"
|
||||
_WINDOWS_DRIVE_PATH: Final = r"[A-Za-z]:\\[^\s'\"\)\]}>,]+"
|
||||
_PRIVATE_OR_LOOPBACK_IPV4: Final = (
|
||||
r"\b(?:10(?:\.\d{1,3}){3}|172\.(?:1[6-9]|2\d|3[01])(?:\.\d{1,3}){2}|192\.168(?:\.\d{1,3}){2}|127(?:\.\d{1,3}){3})\b"
|
||||
)
|
||||
_INTERNAL_SUFFIX_HOSTNAME: Final = r"\b[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*\.(?:internal|local|corp|lan|intra|private)\b"
|
||||
_INTERNAL_DETAIL_RE: Final = re.compile(
|
||||
"|".join((_UNIX_SYSTEM_PATH, _WINDOWS_DRIVE_PATH, _PRIVATE_OR_LOOPBACK_IPV4, _INTERNAL_SUFFIX_HOSTNAME)),
|
||||
re.IGNORECASE,
|
||||
)
|
||||
_TRACEBACK_MARKER: Final = "Traceback (most recent call last):"
|
||||
|
||||
|
||||
def redact_internal_details(value: str) -> str:
|
||||
"""Drop an embedded traceback and scrub filesystem paths and internal hostnames,
|
||||
on top of redact_string(). For client-facing messages only: server logs keep this detail."""
|
||||
marker_index: Final = value.find(_TRACEBACK_MARKER)
|
||||
without_traceback: Final = value[:marker_index].rstrip() if marker_index != -1 else value
|
||||
return _INTERNAL_DETAIL_RE.sub(_REDACTED, redact_string(without_traceback))
|
||||
|
||||
|
||||
def redact_structured_value(key: str | None, value: str) -> str:
|
||||
"""Scrub *value* as it appeared under *key* inside a structured record.
|
||||
|
||||
|
|
|
|||
|
|
@ -3,7 +3,6 @@ import contextlib
|
|||
import json
|
||||
import logging
|
||||
import math
|
||||
import traceback
|
||||
from collections.abc import AsyncGenerator, Awaitable, Callable, Coroutine, Mapping, Sequence
|
||||
from datetime import datetime
|
||||
from functools import lru_cache
|
||||
|
|
@ -18,7 +17,7 @@ from fastapi.responses import JSONResponse, Response, StreamingResponse
|
|||
from starlette.types import Receive, Scope, Send
|
||||
|
||||
import litellm
|
||||
from litellm._logging import _redact_string, verbose_proxy_logger
|
||||
from litellm._logging import redact_internal_details_from_client_message, verbose_proxy_logger
|
||||
from litellm._uuid import uuid
|
||||
from litellm.constants import (
|
||||
DD_TRACER_STREAMING_CHUNK_YIELD_RESOURCE,
|
||||
|
|
@ -3417,7 +3416,7 @@ class ProxyBaseLLMRequestProcessing:
|
|||
else:
|
||||
_code = status.HTTP_500_INTERNAL_SERVER_ERROR
|
||||
raise ProxyException(
|
||||
message=getattr(e, "message", error_msg),
|
||||
message=redact_internal_details_from_client_message(getattr(e, "message", error_msg)),
|
||||
type=getattr(e, "type", "None"),
|
||||
param=getattr(e, "param", "None"),
|
||||
openai_code=getattr(e, "code", None),
|
||||
|
|
@ -3629,10 +3628,8 @@ class ProxyBaseLLMRequestProcessing:
|
|||
|
||||
if isinstance(e, HTTPException):
|
||||
raise e
|
||||
error_traceback: Final = _redact_string(traceback.format_exc())
|
||||
error_msg: Final = f"{e}\n\n{error_traceback}"
|
||||
proxy_exception: Final = ProxyException(
|
||||
message=getattr(e, "message", error_msg),
|
||||
message=redact_internal_details_from_client_message(getattr(e, "message", str(e))),
|
||||
type=getattr(e, "type", "None"),
|
||||
param=getattr(e, "param", "None"),
|
||||
code=getattr(e, "status_code", 500),
|
||||
|
|
|
|||
|
|
@ -261,6 +261,7 @@ class ProxyInitializationHelpers:
|
|||
"app": "litellm.proxy.proxy_server:app",
|
||||
"host": host,
|
||||
"port": port,
|
||||
"server_header": False,
|
||||
}
|
||||
if log_config is not None:
|
||||
print(f"Using log_config: {log_config}")
|
||||
|
|
|
|||
|
|
@ -1013,6 +1013,48 @@ def test_an_unmapped_exception_with_no_model_or_provider_is_a_connection_error(q
|
|||
assert "boom" in raised.value.message
|
||||
|
||||
|
||||
def _raise_and_map(
|
||||
model: str | None, original_exception: Exception, custom_llm_provider: str | None
|
||||
) -> None:
|
||||
"""Calls exception_type() from inside the except block, as litellm/main.py does,
|
||||
so traceback.format_exc() has a real stack."""
|
||||
try:
|
||||
raise original_exception
|
||||
except type(original_exception) as caught:
|
||||
exception_type(
|
||||
model=model,
|
||||
original_exception=caught,
|
||||
custom_llm_provider=custom_llm_provider,
|
||||
)
|
||||
|
||||
|
||||
def test_an_unmapped_exception_message_keeps_traceback_for_sdk_callers(quiet_exception_mapping):
|
||||
"""Direct SDK callers debug unmapped provider exceptions with this traceback;
|
||||
only the proxy's response boundary strips it."""
|
||||
with pytest.raises(litellm.APIConnectionError) as raised:
|
||||
_raise_and_map(
|
||||
model="MiniMax-M2.5",
|
||||
original_exception=RuntimeError("socket hung up"),
|
||||
custom_llm_provider="minimax",
|
||||
)
|
||||
|
||||
assert "Traceback (most recent call last)" in raised.value.message
|
||||
assert "test_exception_mapping_utils.py" in raised.value.message
|
||||
|
||||
|
||||
def test_an_unmapped_exception_with_no_model_or_provider_message_keeps_traceback(
|
||||
quiet_exception_mapping,
|
||||
):
|
||||
with pytest.raises(litellm.APIConnectionError) as raised:
|
||||
_raise_and_map(
|
||||
model=None,
|
||||
original_exception=ValueError("boom"),
|
||||
custom_llm_provider=None,
|
||||
)
|
||||
|
||||
assert "Traceback (most recent call last)" in raised.value.message
|
||||
|
||||
|
||||
CONTEXT_WINDOW_MESSAGE = "This model's maximum context length is 4096 tokens."
|
||||
CONTENT_POLICY_MESSAGE = (
|
||||
'{"error": {"type": "invalid_request_error", "code": "content_policy_violation"}}'
|
||||
|
|
|
|||
|
|
@ -3013,7 +3013,7 @@ class TestHandleLLMApiExceptionDictDetail:
|
|||
assert "NotFoundError" in proxy_exc.message
|
||||
|
||||
async def test_exception_with_status_code_propagates(self):
|
||||
"""Exception with a statically-set status_code should propagate it."""
|
||||
"""Exception with a statically-set status_code should propagate it and its message."""
|
||||
from litellm.llms.vertex_ai.common_utils import VertexAIError
|
||||
|
||||
exc = VertexAIError(
|
||||
|
|
@ -3022,12 +3022,30 @@ class TestHandleLLMApiExceptionDictDetail:
|
|||
)
|
||||
proxy_exc = await self._invoke(exc)
|
||||
assert proxy_exc.code == "429"
|
||||
assert proxy_exc.message == "Rate limit exceeded"
|
||||
|
||||
async def test_exception_without_status_code_defaults_to_500(self):
|
||||
"""Exception with no status_code attribute defaults to 500."""
|
||||
"""Exception with no status_code attribute defaults to 500; a message with nothing
|
||||
to redact still reaches the client, since routes raise plain exceptions as validation text."""
|
||||
exc = ValueError("Something broke")
|
||||
proxy_exc = await self._invoke(exc)
|
||||
assert proxy_exc.code == "500"
|
||||
assert proxy_exc.message == "Something broke"
|
||||
|
||||
async def test_unclassified_exception_redacts_internal_details_from_client_message(self):
|
||||
"""Regression for LIT-6747: an unclassified exception's credential, path, and host
|
||||
must not reach the client."""
|
||||
exc = RuntimeError(
|
||||
"Failed to connect to postgresql://litellm_internal:S3cr3tPGPass@10.20.30.40:5432/litellm_prod "
|
||||
"(config file /etc/litellm/secrets/db.yaml)"
|
||||
)
|
||||
proxy_exc = await self._invoke(exc)
|
||||
assert proxy_exc.code == "500"
|
||||
assert "S3cr3tPGPass" not in proxy_exc.message
|
||||
assert "litellm_internal" not in proxy_exc.message
|
||||
assert "10.20.30.40" not in proxy_exc.message
|
||||
assert "/etc/litellm/secrets/db.yaml" not in proxy_exc.message
|
||||
assert "REDACTED" in proxy_exc.message
|
||||
|
||||
async def test_already_normalized_proxy_exception_is_honored(self):
|
||||
"""A ProxyException raised mid-request (e.g. a guardrail block) is already
|
||||
|
|
@ -3244,6 +3262,42 @@ class TestStreamCloseOnDisconnect:
|
|||
|
||||
assert upstream.aclosed
|
||||
|
||||
async def test_async_streaming_data_generator_redacts_internal_details_on_error(
|
||||
self,
|
||||
):
|
||||
"""Regression for LIT-6747: a mid-stream exception must not hand its raw text or a
|
||||
traceback to serialize_error."""
|
||||
|
||||
class FailingUpstream:
|
||||
def __aiter__(self):
|
||||
return self
|
||||
|
||||
async def __anext__(self):
|
||||
raise RuntimeError(
|
||||
"Failed to connect to postgresql://litellm_internal:S3cr3tPGPass@10.20.30.40:5432/litellm_prod "
|
||||
"(config file /etc/litellm/secrets/db.yaml)"
|
||||
)
|
||||
|
||||
ProxyLogging._callback_capabilities_cache.clear()
|
||||
captured: list = []
|
||||
gen = ProxyBaseLLMRequestProcessing.async_streaming_data_generator(
|
||||
response=FailingUpstream(),
|
||||
user_api_key_dict=ProxyUserAPIKeyAuth(api_key="sk-test"),
|
||||
request_data={"model": "mock-model"},
|
||||
proxy_logging_obj=ProxyLogging(user_api_key_cache=MagicMock()),
|
||||
serialize_chunk=lambda c: "data: x\n\n",
|
||||
serialize_error=lambda e: captured.append(e) or "data: error\n\n",
|
||||
)
|
||||
|
||||
await gen.__anext__()
|
||||
|
||||
assert len(captured) == 1
|
||||
message = captured[0].message
|
||||
assert "S3cr3tPGPass" not in message
|
||||
assert "10.20.30.40" not in message
|
||||
assert "/etc/litellm/secrets/db.yaml" not in message
|
||||
assert "Traceback (most recent call last)" not in message
|
||||
|
||||
@staticmethod
|
||||
def _request_that_disconnects() -> Request:
|
||||
async def receive():
|
||||
|
|
|
|||
|
|
@ -95,6 +95,7 @@ class TestProxyInitializationHelpers:
|
|||
assert args["app"] == "litellm.proxy.proxy_server:app"
|
||||
assert args["host"] == "localhost"
|
||||
assert args["port"] == 8000
|
||||
assert args["server_header"] is False
|
||||
|
||||
# Test with log_config
|
||||
args = ProxyInitializationHelpers._get_default_unvicorn_init_args(
|
||||
|
|
|
|||
|
|
@ -172,8 +172,6 @@ class TestLLMHTTPHandlerRealtimeRedaction:
|
|||
|
||||
|
||||
class TestProxyStreamingDataGeneratorRedaction:
|
||||
"""Test _redact_string on traceback.format_exc() — the pattern at common_request_processing.py:1733."""
|
||||
|
||||
def test_redact_traceback_format_exc(self):
|
||||
try:
|
||||
raise RuntimeError(
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import logging
|
|||
import logging.config
|
||||
import sys
|
||||
import time
|
||||
import traceback
|
||||
from collections.abc import Callable
|
||||
from io import StringIO
|
||||
from typing import Final
|
||||
|
|
@ -13,11 +14,12 @@ from litellm._logging import (
|
|||
JsonFormatter,
|
||||
_redact_string,
|
||||
_secret_filter,
|
||||
redact_internal_details_from_client_message,
|
||||
verbose_logger,
|
||||
verbose_proxy_logger,
|
||||
verbose_router_logger,
|
||||
)
|
||||
from litellm.litellm_core_utils.secret_redaction import redact_string
|
||||
from litellm.litellm_core_utils.secret_redaction import redact_internal_details, redact_string
|
||||
|
||||
SECRET = "sk-proj-abc123def456ghi789jklmnopqrst"
|
||||
|
||||
|
|
@ -657,3 +659,65 @@ def test_json_formatter_redacts_non_string_extra_values(extra):
|
|||
assert output.strip(), "no record captured"
|
||||
assert SECRET not in output, f"non-string extra leaked a secret: {output}"
|
||||
assert "REDACTED" in output
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"text,leaked",
|
||||
(
|
||||
("config file /etc/litellm/secrets/db.yaml", "/etc/litellm/secrets/db.yaml"),
|
||||
("home dir /Users/admin/.litellm/master_key.txt", "/Users/admin/.litellm/master_key.txt"),
|
||||
("cache at /var/cache/litellm/tokens.db", "/var/cache/litellm/tokens.db"),
|
||||
("path C:\\Users\\admin\\secrets.env", "C:\\Users\\admin\\secrets.env"),
|
||||
("connecting to host 10.20.30.40", "10.20.30.40"),
|
||||
("connecting to host 192.168.1.5", "192.168.1.5"),
|
||||
("connecting to host 172.16.0.9", "172.16.0.9"),
|
||||
("connecting to host 127.0.0.1", "127.0.0.1"),
|
||||
("connecting to db-primary.internal", "db-primary.internal"),
|
||||
("connecting to redis.corp", "redis.corp"),
|
||||
),
|
||||
)
|
||||
def test_redact_internal_details_catches_paths_and_hostnames(text, leaked):
|
||||
result = redact_internal_details(text)
|
||||
assert leaked not in result, f"{leaked!r} was not redacted"
|
||||
assert "REDACTED" in result
|
||||
|
||||
|
||||
def test_redact_internal_details_leaves_public_hostnames_and_routes_alone():
|
||||
"""litellm's own error messages rely on routes like /v1/models staying legible."""
|
||||
safe_strings = (
|
||||
"call https://api.openai.com/v1/chat/completions",
|
||||
"/chat/completions: Invalid model name passed in model=gpt-9",
|
||||
"Call `/v1/models` to view available models for your key",
|
||||
"reducto:// file IDs are not accepted through the proxy OCR API",
|
||||
)
|
||||
for text in safe_strings:
|
||||
assert redact_internal_details(text) == text
|
||||
|
||||
|
||||
def test_redact_internal_details_layers_on_top_of_credential_redaction():
|
||||
text = "postgresql://litellm_internal:S3cr3tPGPass@10.20.30.40:5432/litellm_prod"
|
||||
result = redact_internal_details(text)
|
||||
assert "S3cr3tPGPass" not in result
|
||||
assert "10.20.30.40" not in result
|
||||
|
||||
|
||||
def test_redact_internal_details_drops_embedded_traceback():
|
||||
"""Regression for LIT-6747: the traceback exception_type() embeds for SDK callers
|
||||
must never reach an HTTP client."""
|
||||
try:
|
||||
raise RuntimeError("socket hung up")
|
||||
except RuntimeError:
|
||||
raw_tb = traceback.format_exc()
|
||||
message = f"litellm.APIConnectionError: MinimaxException - socket hung up\n{raw_tb}"
|
||||
|
||||
result = redact_internal_details(message)
|
||||
|
||||
assert result == "litellm.APIConnectionError: MinimaxException - socket hung up"
|
||||
assert "Traceback (most recent call last)" not in result
|
||||
assert __file__.split("/")[-1] not in result
|
||||
|
||||
|
||||
def test_redact_internal_details_from_client_message_respects_disable_flag():
|
||||
with patch("litellm._logging._ENABLE_SECRET_REDACTION", False): # test-quality-ok: the opt-out flag is the SUT
|
||||
text = "config file /etc/litellm/secrets/db.yaml"
|
||||
assert redact_internal_details_from_client_message(text) == text
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue