fix(proxy): invoke post-call guardrails on pass-through endpoint responses (#20270) - #26262
Conversation
|
|
Congrats! CodSpeed is installed 🎉
You will start to see performance impacts in the reports once the benchmarks are run from your default branch.
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR wires post-call guardrails into non-streaming pass-through endpoint responses ( Confidence Score: 4/5Safe to merge with one small logging-accuracy fix worth addressing before or after merge. The three previously flagged P0/P1 issues (400 vs 200 status, stale content-length, ModifyResponseException placement) are all resolved in the current commit. The remaining open item is the logging payload recording litellm/proxy/pass_through_endpoints/pass_through_endpoints.py — the
|
| Filename | Overview |
|---|---|
| litellm/exceptions.py | Adds ModifyResponseException as a first-class exception; clean move from custom_guardrail.py, no issues. |
| litellm/integrations/custom_guardrail.py | Replaces inline ModifyResponseException definition with a re-export from litellm.exceptions; backward-compatible and correct. |
| litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py | Adds call_type fallback from litellm_logging_obj for pass-through routes; correctly keyed on CallTypes.pass_through.value == "pass_through_endpoint" and the handler exists in load_guardrail_translation_mappings(). |
| litellm/proxy/pass_through_endpoints/pass_through_endpoints.py | Core change: adds opt-in post_call_success_hook invocation, ModifyResponseException handler returning HTTP 200, and content-length strip on body rewrite. _content_modified is set to True whenever hook returns any dict (including unchanged), causing unconditional re-encoding for all guardrail-enabled requests. |
| tests/test_litellm/proxy/pass_through_endpoints/test_passthrough_post_call_guardrails.py | New mock-only test file; covers hook firing, no-op path, ModifyResponseException→200, and unified guardrail call_type resolution. Compliant with repo mock-only test policy. |
Sequence Diagram
sequenceDiagram
participant Client
participant PassThrough as pass_through_request()
participant PreHook as proxy_logging_obj.pre_call_hook
participant Upstream as Upstream Provider
participant PostHook as proxy_logging_obj.post_call_success_hook
participant UnifiedGuardrail as UnifiedLLMGuardrails
participant PTHandler as PassThroughEndpointHandler
Client->>PassThrough: POST /vertex_ai/* (with guardrails_config)
PassThrough->>PassThrough: collect_guardrails() → guardrails_to_run
PassThrough->>PassThrough: init logging_obj (call_type=pass_through_endpoint)
PassThrough->>PreHook: pre_call_hook(data=_parsed_body)
PreHook-->>PassThrough: possibly stripped data
PassThrough->>Upstream: HTTP POST
Upstream-->>PassThrough: JSON response
alt guardrails_to_run non-empty AND response is JSON
PassThrough->>PostHook: post_call_success_hook(hook_data, response_body)
PostHook->>UnifiedGuardrail: async_post_call_success_hook()
UnifiedGuardrail->>UnifiedGuardrail: resolve call_type
UnifiedGuardrail->>PTHandler: process_output_response()
PTHandler-->>UnifiedGuardrail: possibly modified response dict
UnifiedGuardrail-->>PostHook: response dict
PostHook-->>PassThrough: response dict
alt guardrail raised ModifyResponseException
PassThrough->>PassThrough: catch ModifyResponseException
PassThrough->>Client: 200 + error envelope
else normal path
PassThrough->>PassThrough: re-encode if dict, strip content-length if modified
PassThrough->>Client: 200 + response
end
else no guardrails or non-JSON
PassThrough->>Client: 200 + original response bytes
end
Reviews (9): Last reviewed commit: "test: add unit tests for pass-through po..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if response_body is not None and guardrails_to_run: | ||
| response_body = await proxy_logging_obj.post_call_success_hook( | ||
| data=_parsed_body, | ||
| user_api_key_dict=user_api_key_dict, | ||
| response=response_body, # type: ignore[arg-type] | ||
| ) |
There was a problem hiding this comment.
post_call_success_hook also fires non-guardrail CustomLogger callbacks
proxy_logging_obj.post_call_success_hook() iterates litellm.callbacks and calls async_post_call_success_hook on every non-guardrail CustomLogger instance unconditionally (see utils.py lines 2086-2091). The PR description and backwards-compatibility note state that "CustomLogger callbacks" won't activate on pass-through routes, but any CustomLogger in the global callback list will actually fire whenever guardrails_to_run is non-empty — this is a broader behavior change than the documentation implies. Users who have both custom logging callbacks and a guardrail configured on a pass-through route may observe unexpected side effects from those logging callbacks. Consider documenting this, or filtering to only the guardrail code path if that was the intent.
There was a problem hiding this comment.
Acknowledged — this is consistent with how post_call_success_hook works on all other endpoints (/chat/completions, /v1/messages, etc.) where both guardrail and non-guardrail CustomLogger callbacks fire in the same hook. Since pass-through guardrails are opt-in only (user must explicitly configure guardrails_config), enabling the hook is an intentional decision. Filtering to guardrail-only callbacks would diverge from the standard endpoint behavior and could surprise users who expect their logging callbacks to fire when guardrails are active. Happy to add a note in the PR description clarifying this.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
d85481e to
0b89fd4
Compare
| @@ -34,6 +34,7 @@ | |||
| from litellm._logging import verbose_proxy_logger | |||
| from litellm._uuid import uuid | |||
| from litellm.constants import MAXIMUM_TRACEBACK_LINES_TO_LOG | |||
| from litellm.integrations.custom_guardrail import ModifyResponseException | |||
| @@ -635,6 +635,7 @@ async def pass_through_request( # noqa: PLR0915 | |||
| custom_llm_provider: Optional field - custom LLM provider for the endpoint | |||
| guardrails_config: Optional field - guardrails configuration for passthrough endpoint | |||
| """ | |||
| from litellm.integrations.custom_guardrail import ModifyResponseException | |||
| @@ -635,6 +635,7 @@ | |||
| custom_llm_provider: Optional field - custom LLM provider for the endpoint | |||
| guardrails_config: Optional field - guardrails configuration for passthrough endpoint | |||
| """ | |||
| from litellm.integrations.custom_guardrail import ModifyResponseException | |||
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
9e09b19 to
4d1da0a
Compare
| error_body = { | ||
| "error": { | ||
| "message": e.message or "Response blocked by guardrail", | ||
| "type": "content_filter", | ||
| "guardrail_name": e.guardrail_name, | ||
| "model": e.model, | ||
| } | ||
| } | ||
| return Response( | ||
| content=json.dumps(error_body), | ||
| status_code=400, | ||
| media_type="application/json", | ||
| ) | ||
| except Exception as e: |
There was a problem hiding this comment.
ModifyResponseException returns 400, but the contract requires 200
ModifyResponseException's own docstring says "It should be caught by the proxy and returned with a 200 status code", and both proxy_server.py (line 7168, comment: "return 200 with violation message") and anthropic_endpoints/endpoints.py (line 72, same comment) honour that. Returning 400 breaks clients that rely on the semantic guarantee that guardrail substitutions are surfaced as valid (200) responses, not as HTTP errors.
The PR description even names the test test_modify_response_exception_returns_200, but the committed test asserts result.status_code == 400 — confirming the intent was 200 and this is an unintended divergence.
return Response(
content=json.dumps(error_body),
status_code=200, # ← should be 200, not 400
media_type="application/json",
)There was a problem hiding this comment.
Fixed in a5c7abe — changed back to status_code=200 to match the ModifyResponseException contract (consistent with proxy_server.py and anthropic_endpoints.py). The response body uses a provider-agnostic error envelope instead of ModelResponse since pass-through clients may be any provider (Gemini, Bedrock, etc.).
| litellm_logging_obj is not None | ||
| and getattr(litellm_logging_obj, "call_type", None) | ||
| == CallTypes.pass_through.value | ||
| ): |
There was a problem hiding this comment.
This is a pre-existing log line in unified_guardrail.py (line 237: verbose_proxy_logger.debug("async_post_call_success_hook response: %s", response)) — not introduced by this PR. Our changes start at line 248 (the call_type fallback block). The CodeQL finding is on existing code that happens to be in the diff context.
| self.guardrail_name = guardrail_name | ||
| self.detection_info = detection_info or {} | ||
| super().__init__(message) | ||
| from litellm.exceptions import ModifyResponseException as ModifyResponseException |
There was a problem hiding this comment.
This is a pre-existing cyclic import through litellm/__init__.py — not introduced by this PR. Our change actually improved the situation: we moved the import target from custom_guardrail (which chains into custom_logger → proxy code) to exceptions (a leaf module with only typing, httpx, openai, litellm.types.utils imports). The cycle CodeQL detects is the top-level litellm package re-export chain that affects virtually every module in the codebase.
| @@ -635,6 +635,7 @@ async def pass_through_request( # noqa: PLR0915 | |||
| custom_llm_provider: Optional field - custom LLM provider for the endpoint | |||
| guardrails_config: Optional field - guardrails configuration for passthrough endpoint | |||
| """ | |||
| from litellm.exceptions import ModifyResponseException | |||
There was a problem hiding this comment.
Same as above — this is the same pre-existing litellm/__init__.py cycle, not introduced by this PR. The function-level import ensures no module-load-time issue; CodeQL is flagging the broader package-level cycle that exists for virtually every litellm.* module.
| # Re-apply guardrails metadata after pre_call_hook (which may return a stripped dict) | ||
| if guardrails_to_run: | ||
| if _parsed_body is None: | ||
| _parsed_body = {} | ||
| if "metadata" not in _parsed_body: | ||
| _parsed_body["metadata"] = {} | ||
| _parsed_body["metadata"]["guardrails"] = guardrails_to_run |
There was a problem hiding this comment.
litellm_logging_obj not re-applied after pre_call_hook, making the unified_guardrail fallback dead code
logging_obj is stored in _parsed_body["litellm_logging_obj"] at line 743, but when pre_call_hook returns a stripped dict (the comment at line 751 explicitly acknowledges this risk for metadata.guardrails), litellm_logging_obj is not re-applied. The re-apply block at lines 751–757 only restores metadata.guardrails.
As a result, when post_call_success_hook(data=_parsed_body, ...) is called at line 930, _parsed_body won't contain litellm_logging_obj, so data.get("litellm_logging_obj") in unified_guardrail.py line 232 always returns None — and the entire call_type resolution fallback added in this PR silently does nothing, causing the unified guardrail to fall through to return response unmodified.
# Re-apply guardrails metadata after pre_call_hook (which may return a stripped dict)
if guardrails_to_run:
if _parsed_body is None:
_parsed_body = {}
if "metadata" not in _parsed_body:
_parsed_body["metadata"] = {}
_parsed_body["metadata"]["guardrails"] = guardrails_to_run
+ _parsed_body["litellm_logging_obj"] = logging_objThere was a problem hiding this comment.
Fixed in 059815f — hook_data now explicitly includes litellm_logging_obj = logging_obj, ensuring the unified guardrail fallback can resolve call_type from data.get("litellm_logging_obj") regardless of what pre_call_hook does to _parsed_body.
032ed13 to
a2d1d95
Compare
…onses (BerriAI#20270) Wire post_call_success_hook into non-streaming pass-through response path, gated on explicit guardrail config (opt-in only, no backwards-compat break). - Call post_call_success_hook after reading non-streaming response body - Build enriched hook_data with guardrails metadata and litellm_logging_obj at call site (avoids mutation of _parsed_body which is shared by logging) - Handle ModifyResponseException with provider-agnostic error envelope, post_call_failure_hook, and defensive try/except - Strip stale content-length when guardrail modifies response body - Move ModifyResponseException to litellm.exceptions to break cyclic import; re-export from custom_guardrail for backwards compat - Add call_type fallback in UnifiedLLMGuardrails for pass-through endpoints using CallTypes.pass_through.value enum
5 tests covering the post-call guardrail invocation on pass-through endpoints: - post_call_success_hook fires when guardrails configured - post_call_success_hook skipped when no guardrails (backwards compat) - ModifyResponseException returns 200 with provider-agnostic error - UnifiedLLMGuardrails resolves call_type from logging_obj for pass-through - ModifyResponseException re-export from custom_guardrail stays in sync
a2d1d95 to
32cdace
Compare
Low: No security issues foundThis PR enables post-call guardrail invocation on pass-through endpoint responses, which is a security improvement — guardrails that previously didn't run on pass-through responses now get invoked. The Status: 0 open Posted by Veria AI · 2026-04-23T20:09:46.114Z |
|
Hi @ishaan-jaff @krrish-berri-2 - this PR is ready for review:
Happy to address any feedback. Thanks! |
…onses (#20270) (#26262) * fix(proxy): invoke post-call guardrails on pass-through endpoint responses (#20270) Wire post_call_success_hook into non-streaming pass-through response path, gated on explicit guardrail config (opt-in only, no backwards-compat break). - Call post_call_success_hook after reading non-streaming response body - Build enriched hook_data with guardrails metadata and litellm_logging_obj at call site (avoids mutation of _parsed_body which is shared by logging) - Handle ModifyResponseException with provider-agnostic error envelope, post_call_failure_hook, and defensive try/except - Strip stale content-length when guardrail modifies response body - Move ModifyResponseException to litellm.exceptions to break cyclic import; re-export from custom_guardrail for backwards compat - Add call_type fallback in UnifiedLLMGuardrails for pass-through endpoints using CallTypes.pass_through.value enum * test: add unit tests for pass-through post-call guardrails 5 tests covering the post-call guardrail invocation on pass-through endpoints: - post_call_success_hook fires when guardrails configured - post_call_success_hook skipped when no guardrails (backwards compat) - ModifyResponseException returns 200 with provider-agnostic error - UnifiedLLMGuardrails resolves call_type from logging_obj for pass-through - ModifyResponseException re-export from custom_guardrail stays in sync
…onses (BerriAI#20270) (BerriAI#26262) * fix(proxy): invoke post-call guardrails on pass-through endpoint responses (BerriAI#20270) Wire post_call_success_hook into non-streaming pass-through response path, gated on explicit guardrail config (opt-in only, no backwards-compat break). - Call post_call_success_hook after reading non-streaming response body - Build enriched hook_data with guardrails metadata and litellm_logging_obj at call site (avoids mutation of _parsed_body which is shared by logging) - Handle ModifyResponseException with provider-agnostic error envelope, post_call_failure_hook, and defensive try/except - Strip stale content-length when guardrail modifies response body - Move ModifyResponseException to litellm.exceptions to break cyclic import; re-export from custom_guardrail for backwards compat - Add call_type fallback in UnifiedLLMGuardrails for pass-through endpoints using CallTypes.pass_through.value enum * test: add unit tests for pass-through post-call guardrails 5 tests covering the post-call guardrail invocation on pass-through endpoints: - post_call_success_hook fires when guardrails configured - post_call_success_hook skipped when no guardrails (backwards compat) - ModifyResponseException returns 200 with provider-agnostic error - UnifiedLLMGuardrails resolves call_type from logging_obj for pass-through - ModifyResponseException re-export from custom_guardrail stays in sync
4 participants
Summary
Fixes #20270 — post-call guardrails are never invoked on pass-through endpoint responses.
Pass-through endpoints (
/vertex_ai/*,/openai/*,/bedrock/*) currently fire pre-call hooks but skippost-call guardrails entirely for non-streaming responses. The handler infrastructure already exists
(
PassThroughEndpointHandler.process_output_response()) but is never reached because:pass_through_request()never callspost_call_success_hookafter reading the responseUnifiedLLMGuardrails.async_post_call_success_hook()can't resolvecall_typefor pass-through routes(not in
API_ROUTE_TO_CALL_TYPES, response is a rawdictnot a typedLLMResponseTypes)Changes
pass_through_endpoints.pyproxy_logging_obj.post_call_success_hook()after reading non-streaming response bodychange for existing users without guardrails)
ModifyResponseExceptionwithpost_call_failure_hook+ 200 response (matchesproxy_server.pyand
anthropic_endpointspatterns)unified_guardrail.pycall_typefromlogging_obj.call_typewhen route-based and response-type-basedresolution both return
NoneBackwards Compatibility
Post-call hooks are gated on
guardrails_to_runbeing non-empty — they only fire when guardrails areexplicitly configured for the pass-through endpoint. Existing pass-through users without guardrail
configuration see zero behavior change. This prevents
default_onguardrails andCustomLoggercallbacksfrom unexpectedly activating on pass-through routes.
Test Plan
Manual end-to-end verification
Tested this PR against real Vertex AI (not mocks) using a local LiteLLM proxy built from this branch. Both the allow and block paths in the new code work as designed.
Setup
Results
post_call_success_hook fires exactly once per request (verified via inserted diagnostic); the original upstream body is forwarded byte-equivalent (content-length left intact since _content_modified stays False).
Matches the new error envelope exactly (type, guardrail_name, model, message). post_call_failure_hook is invoked once (verified). 200 status returned per the PR's design choice for guardrail violations.
UTs
test_post_call_success_hook_called_when_guardrails_configured— verifies hook fires with guardrailstest_post_call_success_hook_skipped_when_no_guardrails— verifies no-op without guardrailstest_modify_response_exception_returns_200— verifies guardrail violation returns 200 with message,calls
post_call_failure_hook, includes usagetest_pass_through_call_type_resolved_from_logging_obj— verifies unified guardrail resolvescall_type for pass-through endpoints
Limitations (follow-up)
PassThroughEndpointHandler.process_output_response()extractstextsbut nottool_callsfrom theresponse — guardrails that inspect tool calls from native provider formats need a separate enhancement