mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-15 23:31:29 +00:00
feat(guardrails): add scan_raw_request flag so YAML order can't change enforcement
Maintainer finding on BerriAI/litellm#34940: guardrails for the same hook run sequentially over one shared, progressively-mutated request dict, so declaring a masking guardrail before a blocking one hides the violation from it (200 vs 400 depending purely on YAML order). scan_raw_request opts a guardrail into always evaluating a snapshot taken before any guardrail in the hook ran, regardless of its declared position. Same contract as run_in_parallel: block-only, its own mutations discarded. Verified live: real proxy, real Gemini call, two custom guardrails (a redactor then a blocker). Same request, same declared order -- without the flag the blocker never sees the raw secret (200); with it, the blocker correctly rejects before any provider call (400).
This commit is contained in:
parent
1cb6f99df1
commit
547535dbc5
8 changed files with 241 additions and 11 deletions
|
|
@ -3,7 +3,7 @@
|
|||
"limit": 18483
|
||||
},
|
||||
"reportArgumentType": {
|
||||
"limit": 2564
|
||||
"limit": 2561
|
||||
},
|
||||
"reportAssignmentType": {
|
||||
"limit": 319
|
||||
|
|
|
|||
|
|
@ -164,6 +164,7 @@ class CustomGuardrail(CustomLogger):
|
|||
sensitive_data_route_to_model: str | None = None,
|
||||
sticky_session_routing: bool = True,
|
||||
run_in_parallel: bool = False,
|
||||
scan_raw_request: bool = False,
|
||||
only_scan_new_messages: bool = False,
|
||||
**kwargs,
|
||||
):
|
||||
|
|
@ -186,6 +187,13 @@ class CustomGuardrail(CustomLogger):
|
|||
run_in_parallel: When True, this pre_call or post_call guardrail runs concurrently with
|
||||
other opted-in guardrails of the same hook. Only safe for block-only guardrails that
|
||||
do not mutate the request or response.
|
||||
scan_raw_request: When True, this pre_call guardrail always evaluates the request as it
|
||||
was before any guardrail in this hook ran, regardless of where it's declared in the
|
||||
guardrails list -- so an earlier guardrail that masks/rewrites content (e.g. PII
|
||||
redaction) can never hide a violation from this one. Only safe for block-only
|
||||
guardrails: any data this guardrail returns is discarded, matching run_in_parallel's
|
||||
contract, since applying its mutations on top of a stale snapshot would silently
|
||||
undo whatever later guardrails already did to the live request.
|
||||
"""
|
||||
self.guardrail_name = guardrail_name
|
||||
self.supported_event_hooks = supported_event_hooks
|
||||
|
|
@ -201,6 +209,7 @@ class CustomGuardrail(CustomLogger):
|
|||
self.sensitive_data_route_to_model: str | None = sensitive_data_route_to_model
|
||||
self.sticky_session_routing: bool = sticky_session_routing
|
||||
self.run_in_parallel: bool = run_in_parallel
|
||||
self.scan_raw_request: bool = scan_raw_request
|
||||
self.only_scan_new_messages: bool = only_scan_new_messages
|
||||
|
||||
if supported_event_hooks:
|
||||
|
|
|
|||
|
|
@ -413,6 +413,16 @@ class GuardrailRegistry:
|
|||
raise Exception(f"Error getting guardrail from DB: {e}")
|
||||
|
||||
|
||||
def _apply_configured_bool_override(instance: CustomGuardrail, litellm_params: LitellmParams, param_name: str) -> None:
|
||||
"""Override ``instance.<param_name>`` only when ``litellm_params`` explicitly
|
||||
sets it, preserving whatever default the guardrail's own constructor chose
|
||||
otherwise (its constructor default may be True, so blindly copying an
|
||||
absent/None config value would silently clobber it back to False)."""
|
||||
configured: Final = getattr(litellm_params, param_name, None)
|
||||
if configured is not None:
|
||||
setattr(instance, param_name, bool(configured))
|
||||
|
||||
|
||||
class InMemoryGuardrailHandler:
|
||||
"""
|
||||
Class that handles initializing guardrails and adding them to the CallbackManager
|
||||
|
|
@ -534,9 +544,8 @@ class InMemoryGuardrailHandler:
|
|||
"skip_tool_message_in_guardrail are enabled together, which excludes every message from "
|
||||
"scanning, so no request content would ever be scanned. Remove one of the two."
|
||||
)
|
||||
configured_run_in_parallel: Final[bool | None] = getattr(litellm_params, "run_in_parallel", None)
|
||||
if configured_run_in_parallel is not None:
|
||||
custom_guardrail_callback.run_in_parallel = bool(configured_run_in_parallel)
|
||||
for override_param in ("run_in_parallel", "scan_raw_request"):
|
||||
_apply_configured_bool_override(custom_guardrail_callback, litellm_params, override_param)
|
||||
|
||||
parsed_guardrail: Final = Guardrail(
|
||||
guardrail_id=guardrail.get("guardrail_id"),
|
||||
|
|
|
|||
|
|
@ -1387,6 +1387,43 @@ class ProxyLogging:
|
|||
|
||||
return data
|
||||
|
||||
async def _run_sequential_guardrail_callback(
|
||||
self,
|
||||
callback: CustomGuardrail,
|
||||
data: dict, # mutable-ok: matches _process_guardrail_callback's own request-payload typing
|
||||
raw_request_snapshot: dict | None, # mutable-ok: same request-payload shape as data
|
||||
user_api_key_dict: UserAPIKeyAuth,
|
||||
call_type: CallTypesLiteral,
|
||||
) -> dict: # mutable-ok: callers reassign the loop's own data from this return value
|
||||
"""
|
||||
Run one guardrail from the sequential pre_call loop and return what the
|
||||
rest of the loop should carry forward.
|
||||
|
||||
A guardrail opted into ``scan_raw_request`` always evaluates a fresh
|
||||
copy of ``raw_request_snapshot`` (taken before any guardrail in this
|
||||
hook ran) instead of ``data`` (the live, possibly already-mutated
|
||||
payload), so its block/pass decision can never depend on where it's
|
||||
declared relative to a guardrail that masks or rewrites content. It's
|
||||
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.
|
||||
"""
|
||||
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
|
||||
)
|
||||
result: Final = await self._process_guardrail_callback(
|
||||
callback=callback,
|
||||
data=input_data,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
call_type=call_type,
|
||||
event_type=GuardrailEventHooks.pre_call,
|
||||
)
|
||||
if result is None or scans_raw_request:
|
||||
return data
|
||||
return result
|
||||
|
||||
async def _process_prompt_template(
|
||||
self,
|
||||
data: dict,
|
||||
|
|
@ -1712,6 +1749,14 @@ 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:
|
||||
|
|
@ -1725,16 +1770,13 @@ class ProxyLogging:
|
|||
if getattr(_callback, "run_in_parallel", False):
|
||||
continue
|
||||
|
||||
result = await self._process_guardrail_callback(
|
||||
data = await self._run_sequential_guardrail_callback(
|
||||
callback=_callback,
|
||||
data=data,
|
||||
raw_request_snapshot=raw_request_snapshot,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
call_type=call_type,
|
||||
event_type=GuardrailEventHooks.pre_call,
|
||||
)
|
||||
if result is None:
|
||||
continue
|
||||
data = result
|
||||
|
||||
elif (
|
||||
_callback is not None
|
||||
|
|
@ -1786,6 +1828,7 @@ class ProxyLogging:
|
|||
await self._run_parallel_pre_call_guardrails(
|
||||
guardrails=parallel_guardrails,
|
||||
data=data,
|
||||
raw_request_snapshot=raw_request_snapshot,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
call_type=call_type,
|
||||
)
|
||||
|
|
@ -1806,6 +1849,7 @@ class ProxyLogging:
|
|||
self,
|
||||
guardrails: tuple[CustomGuardrail, ...],
|
||||
data: dict,
|
||||
raw_request_snapshot: dict | None, # mutable-ok: same request-payload shape as data
|
||||
user_api_key_dict: UserAPIKeyAuth,
|
||||
call_type: CallTypesLiteral,
|
||||
) -> None:
|
||||
|
|
@ -1822,12 +1866,22 @@ class ProxyLogging:
|
|||
the LLM, preserving the pre-call barrier that ``during_call`` guardrails
|
||||
cannot provide. Per-guardrail latency is recorded by
|
||||
``_process_guardrail_callback``'s own metrics.
|
||||
|
||||
A guardrail that also opted into ``scan_raw_request`` evaluates
|
||||
``raw_request_snapshot`` (taken before the sequential loop ran) instead
|
||||
of ``data`` (the sequential loop's output), for the same reason the
|
||||
sequential branch does: its block decision must not depend on what a
|
||||
sequential guardrail already masked or rewrote.
|
||||
"""
|
||||
results: Final = await asyncio.gather(
|
||||
*(
|
||||
self._process_guardrail_callback(
|
||||
callback=callback,
|
||||
data=data,
|
||||
data=(
|
||||
copy.deepcopy(raw_request_snapshot)
|
||||
if getattr(callback, "scan_raw_request", False) and raw_request_snapshot is not None
|
||||
else data
|
||||
),
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
call_type=call_type,
|
||||
event_type=GuardrailEventHooks.pre_call,
|
||||
|
|
|
|||
|
|
@ -957,6 +957,17 @@ class BaseLitellmParams(ContentFilterConfigModel): # works for new and patch up
|
|||
),
|
||||
)
|
||||
|
||||
scan_raw_request: bool | None = Field(
|
||||
default=None,
|
||||
description=(
|
||||
"When True, this pre_call guardrail always evaluates the request as it was before any "
|
||||
"guardrail in this hook ran, regardless of its position in the guardrails list -- so the "
|
||||
"YAML order of guardrails can never change whether this one blocks. Use only for "
|
||||
"block-only guardrails: any data this guardrail returns is discarded, same contract as "
|
||||
"run_in_parallel, since an earlier guardrail's masking must not be undone by this one."
|
||||
),
|
||||
)
|
||||
|
||||
@field_validator(
|
||||
"mode",
|
||||
"default_action",
|
||||
|
|
|
|||
|
|
@ -156,6 +156,31 @@ def test_initialize_presidio_forwards_analyze_chunk_size_bytes():
|
|||
assert initialized[-1].presidio_analyze_chunk_size_bytes == 250_000
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"config_value, expected",
|
||||
[(True, True), (False, False), (None, False)],
|
||||
)
|
||||
def test_initialize_guardrail_sets_scan_raw_request(config_value, expected):
|
||||
"""scan_raw_request from litellm_params must reach the built guardrail instance,
|
||||
same wiring as run_in_parallel."""
|
||||
litellm_params = {
|
||||
"guardrail": SupportedGuardrailIntegrations.PRESIDIO.value,
|
||||
"mode": "pre_call",
|
||||
"presidio_analyzer_api_base": "https://fakelink.com/v1/presidio/analyze",
|
||||
"presidio_anonymizer_api_base": "https://fakelink.com/v1/presidio/anonymize",
|
||||
}
|
||||
if config_value is not None:
|
||||
litellm_params["scan_raw_request"] = config_value
|
||||
|
||||
guardrail_handler = InMemoryGuardrailHandler()
|
||||
result = guardrail_handler.initialize_guardrail(
|
||||
guardrail={"guardrail_name": "test_scan_raw_request_flag", "litellm_params": litellm_params},
|
||||
)
|
||||
|
||||
custom_guardrail = guardrail_handler.guardrail_id_to_custom_guardrail[result["guardrail_id"]]
|
||||
assert custom_guardrail.scan_raw_request is expected
|
||||
|
||||
|
||||
def test_init_guardrails_v2_skips_invalid_guardrail_instead_of_crashing_boot():
|
||||
"""
|
||||
Regression: one guardrail with an invalid litellm_params combination (Lakera's
|
||||
|
|
|
|||
|
|
@ -10,8 +10,10 @@ from fastapi import HTTPException
|
|||
|
||||
import litellm
|
||||
from litellm.exceptions import RejectedRequestError
|
||||
from litellm.integrations.custom_guardrail import CustomGuardrail
|
||||
from litellm.integrations.custom_logger import CustomLogger
|
||||
from litellm.proxy.utils import ProxyLogging
|
||||
from litellm.types.guardrails import GuardrailEventHooks
|
||||
|
||||
|
||||
def _load(module: str, name: str):
|
||||
|
|
@ -454,3 +456,123 @@ def test_every_pre_call_customlogger_is_deliberately_classified():
|
|||
"Decide whether each judges the payload (mark it) or counts the request (leave it)."
|
||||
)
|
||||
assert CustomLogger.enforces_request_content is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# scan_raw_request: a guardrail's block decision must not depend on YAML order
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class _RedactingGuardrail(CustomGuardrail):
|
||||
"""Mirrors a real masking guardrail (e.g. Lakera's advisory mode): mutates
|
||||
``data`` in place and returns None, same as CustomGuardrail's documented
|
||||
contract for in-place mutation."""
|
||||
|
||||
def __init__(self, **kwargs):
|
||||
kwargs.setdefault("default_on", True)
|
||||
kwargs.setdefault("event_hook", GuardrailEventHooks.pre_call)
|
||||
super().__init__(guardrail_name="redactor", **kwargs)
|
||||
|
||||
async def async_pre_call_hook(self, user_api_key_dict, cache, data, call_type): # type: ignore[override]
|
||||
for msg in data.get("messages", []):
|
||||
if "SECRET" in msg.get("content", ""):
|
||||
msg["content"] = msg["content"].replace("SECRET", "[REDACTED]")
|
||||
return None
|
||||
|
||||
|
||||
class _BlockOnSecretGuardrail(CustomGuardrail):
|
||||
"""Blocks the request if any message contains the literal string SECRET."""
|
||||
|
||||
def __init__(self, **kwargs):
|
||||
kwargs.setdefault("default_on", True)
|
||||
kwargs.setdefault("event_hook", GuardrailEventHooks.pre_call)
|
||||
super().__init__(guardrail_name="blocker", **kwargs)
|
||||
|
||||
async def async_pre_call_hook(self, user_api_key_dict, cache, data, call_type): # type: ignore[override]
|
||||
if any("SECRET" in msg.get("content", "") for msg in data.get("messages", [])):
|
||||
raise HTTPException(status_code=400, detail="blocked: SECRET detected")
|
||||
return None
|
||||
|
||||
|
||||
def _secret_request() -> Dict[str, Any]:
|
||||
return {"messages": [{"role": "user", "content": "here is my SECRET"}], "model": "m"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_yaml_order_changes_enforcement_without_scan_raw_request(
|
||||
proxy_logging, make_user_api_key_auth, monkeypatch
|
||||
):
|
||||
"""Baseline (the bug): declaring the redactor before the blocker lets a
|
||||
request through that would have been blocked in the opposite order,
|
||||
because the blocker only ever sees the already-redacted content."""
|
||||
monkeypatch.setattr(litellm, "callbacks", [_RedactingGuardrail(), _BlockOnSecretGuardrail()])
|
||||
proxy_logging.slack_alerting_instance = MagicMock(alerting=None)
|
||||
out = await proxy_logging.pre_call_hook(
|
||||
user_api_key_dict=make_user_api_key_auth(),
|
||||
data=_secret_request(),
|
||||
call_type="completion",
|
||||
)
|
||||
assert "[REDACTED]" in out["messages"][0]["content"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reversed_yaml_order_blocks_the_same_request(proxy_logging, make_user_api_key_auth, monkeypatch):
|
||||
"""Same two guardrails, opposite declaration order: the blocker now runs
|
||||
first against the still-raw content and correctly rejects the request.
|
||||
Confirms the baseline test above is a real order-dependence, not a fluke."""
|
||||
monkeypatch.setattr(litellm, "callbacks", [_BlockOnSecretGuardrail(), _RedactingGuardrail()])
|
||||
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_makes_blocking_order_independent(proxy_logging, make_user_api_key_auth, monkeypatch):
|
||||
"""Maintainer finding on BerriAI/litellm#34940: with scan_raw_request=True
|
||||
on the blocker, declaring the redactor first no longer lets the request
|
||||
through -- the blocker evaluates the pre-loop snapshot regardless of its
|
||||
position in the guardrails list."""
|
||||
monkeypatch.setattr(
|
||||
litellm, "callbacks", [_RedactingGuardrail(), _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_guardrail_does_not_undo_later_masking(
|
||||
proxy_logging, make_user_api_key_auth, monkeypatch
|
||||
):
|
||||
"""A scan_raw_request guardrail that passes (its own snapshot has no
|
||||
violation) must not affect what a later guardrail in the sequence does to
|
||||
the live request -- its own discarded view of the data must not corrupt
|
||||
or reset the shared ``data`` object for the rest of the loop. Uses a
|
||||
request with no SECRET at all, so the blocker passes cleanly, and a
|
||||
separate marker (PII_TOKEN) that only the redactor reacts to."""
|
||||
|
||||
class _PiiRedactor(_RedactingGuardrail):
|
||||
async def async_pre_call_hook(self, user_api_key_dict, cache, data, call_type): # type: ignore[override]
|
||||
for msg in data.get("messages", []):
|
||||
if "PII_TOKEN" in msg.get("content", ""):
|
||||
msg["content"] = msg["content"].replace("PII_TOKEN", "[REDACTED]")
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(
|
||||
litellm, "callbacks", [_BlockOnSecretGuardrail(scan_raw_request=True), _PiiRedactor()]
|
||||
)
|
||||
proxy_logging.slack_alerting_instance = MagicMock(alerting=None)
|
||||
out = await proxy_logging.pre_call_hook(
|
||||
user_api_key_dict=make_user_api_key_auth(),
|
||||
data={"messages": [{"role": "user", "content": "my PII_TOKEN is here"}], "model": "m"},
|
||||
call_type="completion",
|
||||
)
|
||||
assert "[REDACTED]" in out["messages"][0]["content"]
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
"limit": 22733
|
||||
},
|
||||
"LIT002": {
|
||||
"limit": 26860
|
||||
"limit": 26859
|
||||
},
|
||||
"LIT003": {
|
||||
"limit": 269
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue