Repository navigation
Conversation
Greptile SummaryThe PR registers sensitive_data_routing as a built-in pre-call guardrail and adds detector-based rerouting with configurable sticky-session lifetime
Confidence Score: 3/5The non-positive session TTL path must be rejected before merge because it can disable persistence of sensitive-session routing The new configuration accepts zero and negative TTLs, then forwards them unchanged to cache backends that immediately expire or reject the routing override Files Needing Attention: litellm/types/proxy/guardrails/guardrail_hooks/sensitive_data_routing.py and litellm/proxy/hooks/sensitive_data_routing.py
|
| Filename | Overview |
|---|---|
| litellm/proxy/guardrails/guardrail_hooks/sensitive_data_routing/init.py | Registers and initializes the new guardrail through the repository's automatic package discovery mechanism |
| litellm/proxy/guardrails/guardrail_hooks/sensitive_data_routing/sensitive_data_routing.py | Implements pre-call pattern detection and raises a model-routing exception carrying sticky-session configuration |
| litellm/types/proxy/guardrails/guardrail_hooks/sensitive_data_routing.py | Defines the guardrail configuration but fails to constrain session_ttl_seconds to positive values |
| litellm/proxy/hooks/sensitive_data_routing.py | Applies per-guardrail TTL overrides directly to Redis and local routing caches |
| litellm/proxy/utils.py | Propagates the guardrail-specific TTL when handling sensitive-data routing exceptions |
| tests/test_litellm/proxy/guardrails/guardrail_hooks/test_sensitive_data_routing.py | Covers registration, initialization, detector matching, rerouting, and sticky-session flags, but not invalid TTL values |
Reviews (1): Last reviewed commit: "fix: BerriAI/litellm#36535 - [Bug]: sens..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| session_ttl_seconds: int = Field( | ||
| default=DEFAULT_SESSION_TTL_SECONDS, | ||
| description="How long a session stays pinned to the on-premise model after detection.", | ||
| ) |
There was a problem hiding this comment.
When session_ttl_seconds is non-positive, cache entries expire immediately or Redis rejects them, leaving later session requests unpinned.
How this was verified: The configured value flows unchanged into both cache TTL arguments.
| session_ttl_seconds: int = Field( | |
| default=DEFAULT_SESSION_TTL_SECONDS, | |
| description="How long a session stays pinned to the on-premise model after detection.", | |
| ) | |
| session_ttl_seconds: int = Field( | |
| default=DEFAULT_SESSION_TTL_SECONDS, | |
| ge=1, | |
| description="How long a session stays pinned to the on-premise model after detection.", | |
| ) |
Knowledge Base Used: Guardrails
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
|
|
||
| session_id: Final = get_session_id_from_request_data(request_data) | ||
| raise SensitiveDataRouteException( | ||
| route_to_model=self.on_premise_model, |
There was a problem hiding this comment.
Medium: Rerouted model bypasses model-access controls
Model authorization runs against the caller-supplied model before pre-call guardrails execute, while the exception handler replaces it with on_premise_model without repeating those checks. A caller allowed only the original model can include a configured keyword or sensitive-data pattern and invoke the restricted target model; authorize the resolved target against the key, team, user, project, and member scopes before applying either the immediate or sticky routing override.
| for name in prebuilt_patterns or () | ||
| ), | ||
| *( | ||
| SensitiveDataDetector("regex_pattern", pattern, re.compile(pattern, re.IGNORECASE)) |
There was a problem hiding this comment.
Low: Custom regex permits request-driven denial of service
These operator-supplied patterns are evaluated with Python's backtracking regex engine synchronously in the async request path. If a supported configuration contains a nested-quantifier pattern such as (a+)+$, a caller can submit a crafted prompt that monopolizes the worker; reject unsafe expressions or evaluate custom patterns with a bounded matching engine and timeout.
PR overviewThe PR registers sensitive_data_routing as a built-in guardrail that routes requests to an on-premise model when configured keywords or sensitive-data patterns are detected. One issue has been addressed, but three remain open. Sensitive-data routing can invoke a target model outside the caller’s authorized scope, and caller-supplied cloud fallbacks can send detected sensitive content off-premise when the on-premise model’s context limit is exceeded. Custom regex matching also allows a crafted request to monopolize a worker if an operator configures a vulnerable pattern. Open issues (3)
Fixed/addressed: 1 · PR risk: 7/10 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Merging this PR will not alter performance
|
|
went through the three findings, took one and am pushing back on the other two. non-positive TTL (P1) — real, and mine. rerouted model bypasses model-access controls — the concern is fair but it isnt this PR. the two places the model gets swapped, theres also a design question underneath it. custom regex ReDoS — patterns come from |
pjdurden
changed the title
|
Status check on this one, since it has been quiet since Aug 21. The only red check is the Vertex AI job, and it is not from this PR. The four failures are all embedding-cost assertions in Byte-identical to the four failing on my other open PR #37291, which shares no code with this one, so this looks like base-branch pricing drift rather than anything either branch did. This PR touches the guardrail registration, The three review findings from Aug were resolved in the thread above: I took the non-positive TTL one, since So this is waiting on a first human review rather than on anything from my side. @krrishdholakia @ishaan-jaff when one of you has a moment. |
|
|
|
merged current main in ( |
… recognized as guardrail Signed-off-by: pjdurden <prajjwalchittori1@gmail.com>
A non-positive TTL either expires the in-memory pin immediately or makes Redis reject the SETEX, so sticky_session silently never pinned. Bound the field at config-parse time instead of failing quietly at request time. Signed-off-by: pjdurden <prajjwalchittori1@gmail.com>
The config-model pattern lists become Sequence[str], which the guardrail already accepted and which generates an identical JSON schema. The remaining four are fixed by CustomGuardrail's own signatures, so they carry a mutable-ok reason instead. Comments are kept short enough that ruff format leaves them on the annotation line, which is where the gate reads them. Signed-off-by: pjdurden <prajjwalchittori1@gmail.com>
Assistant tool_calls reach the guardrail through inputs["tool_calls"], not texts, so an SSN inside function.arguments was forwarded to the cloud model without triggering a reroute. Scan the raw argument string and its decoded JSON string values alongside the message text. Also replaces the two-for generator in detect() with a plain loop and declares SensitiveDataDetectorKind as a TypeAlias, for LIT014/LIT010.
| return inputs | ||
|
|
||
| session_id: Final = get_session_id_from_request_data(request_data) | ||
| raise SensitiveDataRouteException( |
There was a problem hiding this comment.
Medium: Cloud fallbacks bypass sensitive-data routing
When configured globally with default_on: true, a caller can submit sensitive text with fallbacks: ["cloud-model"] and exceed the on-premise model's smaller context window: the exception handler changes only model, so the router forwards the unchanged prompt to the cloud fallback without rerunning the global guardrail. Disable fallbacks after sensitive-data detection, or restrict every fallback to approved on-premise targets, including requests restored from sticky-session pins.
Automated draft from oss-work.