scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-15 23:31:29 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 165

fix(guardrails): harden scan_raw_request against review findings

Browse source 
- Use safe_deep_copy instead of a bare deepcopy for the raw-request
  snapshot; request payloads commonly carry unpicklable objects (e.g. an
  otel span in metadata), which previously raised on every guarded
  request when tracing was enabled (Bugbot, High).
- Only compute the snapshot when a guardrail actually opted in, and take
  it before _maybe_execute_pipelines runs, so a pipeline-mutated payload
  can't hide a violation from a scan_raw_request guardrail outside the
  pipeline (veria-ai).
- Log a warning when a scan_raw_request guardrail returns a modified
  payload, since that mutation is discarded and the combination is
  otherwise silently exploitable for a masking-capable integration
  misconfigured this way (veria-ai).
This commit is contained in:
Deepanshu 2026-08-27 13:52:16 -04:00
parent 547535dbc5
commit ec52c22aef
2 changed files with 138 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
litellm/proxy/utils.py
View file

@ -91,7 +91,11 @@ from litellm.integrations.custom_logger import CustomLogger
from litellm.integrations.prometheus import PrometheusLogger
from litellm.integrations.SlackAlerting.slack_alerting import SlackAlerting
from litellm.integrations.SlackAlerting.utils import _add_langfuse_trace_id_to_alert
from litellm.litellm_core_utils.core_helpers import coerce_token_limit, is_expected_client_error
from litellm.litellm_core_utils.core_helpers import (
coerce_token_limit,
is_expected_client_error,
safe_deep_copy,
)
from litellm.litellm_core_utils.litellm_logging import Logging
from litellm.litellm_core_utils.safe_json_dumps import safe_dumps
from litellm.litellm_core_utils.safe_json_loads import safe_json_loads
@ -1407,11 +1411,14 @@ class ProxyLogging:
declared block-only, same contract as ``run_in_parallel``: any data it
returns is discarded, since applying its view on top of a stale
snapshot would silently undo whatever a later guardrail already did to
the live request.
the live request. A guardrail that mutates content (e.g. PII masking)
should never set this flag -- if one does anyway, its returned
mutation is discarded and a warning is logged so the misconfiguration
is visible instead of silently forwarding unredacted content.
"""
scans_raw_request: Final = getattr(callback, "scan_raw_request", False)
input_data: Final = (
copy.deepcopy(raw_request_snapshot) if scans_raw_request and raw_request_snapshot is not None else data
safe_deep_copy(raw_request_snapshot) if scans_raw_request and raw_request_snapshot is not None else data
)
result: Final = await self._process_guardrail_callback(
callback=callback,
@ -1420,6 +1427,14 @@ class ProxyLogging:
call_type=call_type,
event_type=GuardrailEventHooks.pre_call,
)
if scans_raw_request and result is not None:
verbose_proxy_logger.warning(
"Guardrail '%s' has scan_raw_request=True but returned a modified payload; "
"scan_raw_request is for block-only guardrails and this mutation is being "
"discarded. Remove scan_raw_request from this guardrail's config if it needs "
"to mask/rewrite content.",
getattr(callback, "guardrail_name", None) or callback.__class__.__name__,
)
if result is None or scans_raw_request:
return data
return result
@ -1715,6 +1730,22 @@ class ProxyLogging:
call_type=call_type,
)
# Snapshotted here, before _maybe_execute_pipelines or any guardrail in
# this hook has run, so a scan_raw_request guardrail's block/pass
# decision never depends on its position in the guardrails list or on
# a pipeline that runs ahead of it: an earlier guardrail (pipelined or
# not) that masks/rewrites content can't hide a violation from a later
# one that opted into scanning the original request. Only computed
# when at least one registered guardrail actually opted in, and via
# safe_deep_copy (not a bare deepcopy) since the payload commonly
# carries unpicklable objects (e.g. an otel span in metadata) that
# would otherwise raise here on every guarded request.
needs_raw_request_snapshot: Final = any(
isinstance(cb, CustomGuardrail) and getattr(cb, "scan_raw_request", False)
for cb in ProxyLogging._callback_capabilities().resolved_callbacks
)
raw_request_snapshot: Final[dict | None] = safe_deep_copy(data) if needs_raw_request_snapshot else None
try:
# Execute guardrail pipelines before the normal callback loop
data = await self._maybe_execute_pipelines(
@ -1749,14 +1780,6 @@ class ProxyLogging:
and getattr(cb, "run_in_parallel", False)
and not (cb.guardrail_name and cb.guardrail_name in pipeline_managed)
)
# Snapshotted once, before any guardrail in this hook has run, so a
# scan_raw_request guardrail's block/pass decision never depends on
# its position in the guardrails list: an earlier guardrail that
# masks/rewrites content (e.g. PII redaction) can't hide a violation
# from a later one that opted into scanning the original request.
raw_request_snapshot: Final[dict | None] = ( # mutable-ok: same request-payload shape as data
copy.deepcopy(data) if data is not None else None
)
deferred_route_exc: SensitiveDataRouteException | None = None
for _callback in caps.resolved_callbacks:
@ -1878,7 +1901,7 @@ class ProxyLogging:
self._process_guardrail_callback(
callback=callback,
data=(
copy.deepcopy(raw_request_snapshot)
safe_deep_copy(raw_request_snapshot)
if getattr(callback, "scan_raw_request", False) and raw_request_snapshot is not None
else data
),

103
tests/test_litellm/proxy/utils/proxy_logging/test_pre_call_hook.py
View file

@ -576,3 +576,106 @@ async def test_scan_raw_request_guardrail_does_not_undo_later_masking(
call_type="completion",
)
assert "[REDACTED]" in out["messages"][0]["content"]
class _Unpicklable:
"""Mirrors a real otel span: deepcopy always raises, matching what
safe_deep_copy exists to handle (see litellm_core_utils/core_helpers.py)."""
def __deepcopy__(self, memo):
raise TypeError("cannot deepcopy this object")
@pytest.mark.asyncio
async def test_scan_raw_request_snapshot_survives_unpicklable_metadata(
proxy_logging, make_user_api_key_auth, monkeypatch
):
"""
Bugbot finding on BerriAI/litellm#34940: the scan_raw_request snapshot
used a bare copy.deepcopy, which raises on request payloads carrying
unpicklable objects (e.g. metadata["litellm_parent_otel_span"] when
tracing is enabled) -- failing every guarded request, not just ones
that actually use scan_raw_request. Must use safe_deep_copy instead.
"""
monkeypatch.setattr(litellm, "callbacks", [_BlockOnSecretGuardrail(scan_raw_request=True)])
proxy_logging.slack_alerting_instance = MagicMock(alerting=None)
data = {
"messages": [{"role": "user", "content": "hello, nothing flagged here"}],
"model": "m",
"metadata": {"litellm_parent_otel_span": _Unpicklable()},
}
out = await proxy_logging.pre_call_hook(
user_api_key_dict=make_user_api_key_auth(),
data=data,
call_type="completion",
)
assert out is not None
@pytest.mark.asyncio
async def test_scan_raw_request_snapshot_taken_before_pipelines(
proxy_logging, make_user_api_key_auth, monkeypatch
):
"""
veria-ai finding on BerriAI/litellm#34940: the raw snapshot was taken
after _maybe_execute_pipelines ran, so a pipeline that masks content
ahead of a non-pipelined scan_raw_request guardrail could still hide
the violation from it. Simulates a pipeline-style rewrite by having
_maybe_execute_pipelines itself return redacted data, and confirms the
scan_raw_request blocker still sees the pre-pipeline raw content.
"""
async def fake_pipelines(self, data, user_api_key_dict, call_type, event_hook):
for msg in data.get("messages", []):
if "SECRET" in msg.get("content", ""):
msg["content"] = msg["content"].replace("SECRET", "[REDACTED]")
return data
monkeypatch.setattr(ProxyLogging, "_maybe_execute_pipelines", fake_pipelines)
monkeypatch.setattr(litellm, "callbacks", [_BlockOnSecretGuardrail(scan_raw_request=True)])
proxy_logging.slack_alerting_instance = MagicMock(alerting=None)
with pytest.raises(HTTPException, match="blocked"):
await proxy_logging.pre_call_hook(
user_api_key_dict=make_user_api_key_auth(),
data=_secret_request(),
call_type="completion",
)
@pytest.mark.asyncio
async def test_scan_raw_request_warns_when_guardrail_mutation_discarded(
proxy_logging, make_user_api_key_auth, monkeypatch
):
"""
veria-ai finding on BerriAI/litellm#34940: scan_raw_request is accepted
even for a guardrail that mutates the request (e.g. a masking
integration), silently discarding its redaction and forwarding raw
content. Config-time rejection isn't generically possible (no marker
exists for "this guardrail mutates"), so a loud runtime warning is the
mitigation: confirm it fires when a scan_raw_request guardrail returns
a modified payload.
"""
class _MutatingScanner(_RedactingGuardrail):
def __init__(self, **kwargs):
super().__init__(**kwargs)
self.scan_raw_request = True
async def async_pre_call_hook(self, user_api_key_dict, cache, data, call_type): # type: ignore[override]
for msg in data.get("messages", []):
msg["content"] = msg["content"].replace("SECRET-VALUE-123", "[REDACTED]")
return data
from litellm.proxy import utils as proxy_utils_module
mock_logger = MagicMock()
monkeypatch.setattr(proxy_utils_module, "verbose_proxy_logger", mock_logger)
monkeypatch.setattr(litellm, "callbacks", [_MutatingScanner()])
proxy_logging.slack_alerting_instance = MagicMock(alerting=None)
await proxy_logging.pre_call_hook(
user_api_key_dict=make_user_api_key_auth(),
data=_secret_request(),
call_type="completion",
)
mock_logger.warning.assert_called_once()
assert "scan_raw_request" in str(mock_logger.warning.call_args)