Repository navigation
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The request model validates list message content lazily, so the dump that builds the POST body consumes it and a later read of the model rows sees empty content. apply_guardrail compared the guardrail's returned rows with those consumed rows, so no multipart row ever matched its echo. A guardrail that echoed every row unchanged and masked through texts had its rows taken as a rewrite, and the unmasked rows reached the LLM Compare against the JSON rows actually posted instead. An unchanged echo now falls back to texts again, and a partial echo restores the caller's row at each echoed index. structured_messages_from_response is renamed to structured_messages_from_json since it now reads request rows as well The dump also sends a row as content [] when one of its parts fails validation (guarded_text, a base64 document, flac audio), so the guardrail never saw that row's text. An echo of it restored the caller's unmasked row while another row's rewrite made the rows path win, which dropped the masked texts. Such a row now carries the caller's own content, so the guardrail sees it in full. Rows the model dumps in full are posted exactly as before, and the rest of the body is unchanged That content goes through a new as_json_value helper in _content_utils. It round-trips through the stdlib json codec because pydantic's serializer silently replaces anything nested past 254 levels with "..." GenericGuardrailAPI also takes an optional async_handler so tests can inject the HTTP client. The regression tests drive the real OpenAI chat and Anthropic Messages guardrail handlers through it
A row whose part list holds a part the request model rejects is now posted with the caller's content as is. The as_json_value round trip it went through is gone: the whole request is already dumped by pydantic before that point, so it never kept extra nesting depth, and no test needed it The two JSON builds in the row helpers carry the same mutable-ok as the POST body, and a new test covers /v1/responses input_text rows, which the fix already handled
A rejected part holding a non-JSON-native value, such as a tuple, was posted as a JSON array but compared against the caller's tuple. An unchanged echo then looked rewritten, so the masked texts were dropped and the original prompt reached the LLM Caller content now goes through the same stdlib codec httpx encodes the body with before it is posted and compared, and a regression test covers the tuple case
…drail_api Main dropped the LIT002 rule, so the mutable-ok comments on the JSON row builds no longer suppress anything and LIT013 now flags them. They are removed. The part-list guard narrows to Sequence[object] instead of list[object], since its callers only need len() and a read, which also keeps it clear of LIT001
117c37d to
d11066e
Compare
| yield from _iter_text_parts_in_content(message.get("content")) | ||
|
|
||
|
|
||
| def image_part_url(part: object) -> str | None: |
|
@greptileai please review the latest tip d11066e, which is rebased onto #43679 and adds the live proof and config-only fields |
|
|
@greptileai please re-review 3465bfb, which fixes the response rewrite P1 by counting tools and rows only when the endpoint posted them |
A guardrail that re-serializes the rows it was sent often drops null fields, such as the thinking_blocks: null an Anthropic assistant row is posted with or the content: null of a chat tool-call row. The echo check compared rows exactly, so such an echo looked like a rewrite of every row: the masked texts were ignored and the unmasked rows written back, and the raw value reached the model. Rows now count as an echo when they match apart from null fields, in both the every-row check and the per-row restore. A field the guardrail sets to null where the caller had a value still counts as a change
…ic_guardrail_api send_images=False drops the top-level images field and replaces the URL of every image_url part in the structured_messages rows sent to the guardrail with "[omitted]", keeping the part so the guardrail still knows an image was there. That covers remote URLs as well as inline data, and on /v1/messages Anthropic documents too, since they reach the guardrail as image parts. exclude_payload_fields drops named top-level request fields. Unknown names warn and are ignored, and input_type and litellm_call_id are always sent. An invalid value for either option is ignored with a warning and its default is kept The guardrail can no longer rewrite content it did not see. Returned texts, structured_messages, images and tools are ignored with a warning when that field was not sent. An intervention whose only real change went to such a field is rejected instead of letting the call through unchanged, and the error names the request or response side. Changes are compared with what the guardrail was posted, and only texts, tools and written-back rows count, since no endpoint writes returned images back. A returned row whose image was withheld is accepted with the caller's image put back into each part still holding the placeholder, and a rewrite that changes, moves or copies a placeholder is rejected exclude_payload_fields is config-only: fields marked ui_hidden are left out of the Admin UI form, which has no list input for it. initialize_guardrail accepts an optional async_handler, so tests inject the HTTP client
…nt posted them A guardrail that was not sent texts could mask them and also return tools or structured_messages. The refusal check counted those as an applied change, but every response handler and most request handlers (embeddings, completion, image generation and others) never supply or apply either field, so the call went through with the original, unmasked text Tools and written-back rows now count only when the endpoint posted them to the guardrail, which covers both sides without a per-endpoint list. A tool the guardrail adds to a request that posted none no longer stands in for a refused rewrite either, so that case now fails closed
The repo keeps sentinel values in litellm/constants.py, so the "[omitted]" image placeholder and the marker for a rewrite that cannot be applied move there as GENERIC_GUARDRAIL_IMAGE_OMITTED_PLACEHOLDER and GENERIC_GUARDRAIL_UNAPPLIABLE_REWRITE. The marker stays a frozen dataclass so the restore helpers can still narrow their unions with isinstance It also adds a regression test for a guardrail that is not sent texts, masks them anyway and echoes Anthropic rows without their null fields. With the null fields now ignored in the echo check, nothing applies and the request is rejected instead of reaching the model unmasked
3465bfb to
d761026
Compare
|
@greptileai please review d761026, rebased onto #43679's null-echo fix with the payload sentinels moved into constants.py |
| if original_rows is None or shown_rows is None or len(returned_rows) != len(original_rows): | ||
| return tuple(returned_rows) | ||
| if all(returned == shown for shown, returned in zip(shown_rows, returned_rows)): | ||
| if all(same_json_ignoring_nulls(returned, shown) for shown, returned in zip(shown_rows, returned_rows)): |
There was a problem hiding this comment.
Repeated row serialization When a guardrail echoes structured messages containing large base64 images, this comparison serializes and parses both the returned and posted rows. If any row differs, the code repeats that work for each row. This adds latency and raises peak memory use on image-bearing requests.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Measured about 0.06s per call on a 20 MB base64 row, the same order as the request's existing JSON serialization, so leaving it as is
There was a problem hiding this comment.
That measurement addresses the concern: ~0.06 s for a 20 MB row is comparable to the request’s existing JSON serialization, so the additional serialization does not appear to introduce a material regression. Leaving this implementation as-is is reasonable; no change needed.
TLDR
Problem this solves:
How it solves it:
send_images: falsewithholds image data from the guardrail call[omitted]exclude_payload_fieldsdrops named top-level fields from the guardrail callRelevant issues
Split out of #37055. Depends on #43679, so review only the top commit until it merges. Docs: BerriAI/litellm-docs#2075
User Flow
Before: an admin whose guardrail only reads text has every image and header sent to it on every request
streaming_transform_mode, with no option to stop sending imagesimage_urlpart, or the same content to /v1/responses or /v1/messagesstructured_messages, plusrequest_headers,toolsand every other fieldAfter: the same guardrail gets only what it needs, and the model still sees the image
send_imagesas its last field, or setssend_images: falseandexclude_payload_fields: ["request_headers", "tools"]in config.yamlimages,request_headersortools, with each image URL reading[omitted]The screenshots were taken at 3465bfb, and no commit since then touches the Admin UI form. To reproduce them, open http://localhost:4000/ui/?page=guardrails, click Add New Guardrail, then Add Provider Guardrail, pick Generic Guardrail API, type any name, click Next and scroll the form to the bottom.
exclude_payload_fieldsis config-only, so the form does not show itPre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
textsexcluded, a guardrail that masks them and echoes Anthropic rows without their null fields is now rejected, and the test fails if fix(guardrails): keep masked text when a guardrail echoes multipart rows #43679's echo fix is revertedsend_imagesis unset or invalid, parsingsend_imagesstrictly, keeping unknown or protected field names in the exclude list, still posting excluded fields, leaving image URLs inside messages or the top-levelimagesfield when images are withheld, skipping the refusal check or running it for every action, counting returned images as an applied change, not counting returned texts, tools or written-back rows, counting tools or rows the endpoint never posted, treating an unchanged echo as a change, accepting rewrites to fields that were not sent, not restoring the caller's image into an echoed placeholder part, accepting a newly added placeholder, raising on an invalid exclude list, showing config-only fields in the Admin UI form, not forwarding either option from the config, and naming the wrong side in the refusal messageuv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or morecode-qualityandcodecov/patchmake checkpasses locally withBASE_REF=upstream/main. A fork whosemainlags upstream fails the budget gates against that stale base, so setBASE_REFwhen checking a fork_send_imagesandimage_part_urlfor mixing implicit and explicit returns. Both are false positives: every path through the try/except returns, and thematchends incase _: return None@greptileaito re-request a review after pushing changes)constants.py, done in d761026. Greptile scored 5/5 on d761026, and its one note, repeated row serialization on large image rows, is answered in the thread with a measurement (about 0.06s for a 20 MB row)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
All runs are against a live proxy on
localhost:4000started withpython litellm/proxy/proxy_cli.py --config proof_config.yaml --detailed_debug --use_v2_migration_resolver, backed by a real Postgres, and every completion is a real, billed call to Bedrockus.anthropic.claude-haiku-4-5-20251001-v1:0.$PROOF_KEYis a virtual key fromPOST /key/generatewith access tobedrock-haiku-4-5, and$REDis a base64 PNG of a solid red square. Each request picks its guardrail with"guardrails": [...]At the base commit neither option exists, so the proxy ignores both and sends the guardrail everything. The guardrail endpoint is a small FastAPI app on
127.0.0.1:8787that logs which fields it received, how many top-level images it got, and the first 40 characters of every image URL insidestructured_messages. It allows everything, except that whentextsis missing it answersGUARDRAIL_INTERVENEDwith masked texts, which is the rewrite a PII guardrail would send. On the response side it also returns a tool list, which no response handler applies. Nothing else is stubbedThe model still answers "Red" with
send_images: false, because the setting only changes what the guardrail is sent, never what the model receivesBefore (5b61f8b)
Case 1: /v1/chat/completions with an image and send_images: false
Case 2: /v1/responses with an input_image and send_images: false
Case 3: /v1/messages with an image block and send_images: false
Case 4: exclude_payload_fields: [request_headers, tools]
Case 5: the guardrail rewrites texts it was not sent
Case 6: a post_call guardrail masks response texts it was not sent and also returns tools
Case 7: send_images: "maybe" is ignored with a warning and images are still sent
Case 8: startup warning for a guardrail that is not sent everything
After (d761026)
Case 1: /v1/chat/completions with an image and send_images: false
Case 2: /v1/responses with an input_image and send_images: false
Case 3: /v1/messages with an image block and send_images: false
Case 4: exclude_payload_fields: [request_headers, tools]
Case 5: the guardrail rewrites texts it was not sent
Case 6: a post_call guardrail masks response texts it was not sent and also returns tools
Case 7: send_images: "maybe" is ignored with a warning and images are still sent
Case 8: startup warning for a guardrail that is not sent everything
Type
🆕 New Feature
Caveats (if any)
Severe
send_images: false, text written inside an image is not checkedinput_typeandlitellm_call_idare always sentMedium
[omitted]partsend_images: falseonly covers image parts/v1/messagesPDFs are withheld toofile, are still sentLow
exclude_payload_fieldsis config-only, the Admin UI form hides itui_hiddenmarker as feat(guardrails): add system prompt and first role skip filters to generic_guardrail_api #43684 does thisexclude_payload_fieldsare also ignored with a warningpartsand nocontent, so the guardrail request fails validation and the call fails closedimage_part_urloverlaps the private_image_part_urlinprompt_templates/common_utils.pyconfig_parsing.pyis also added by feat(guardrails): add system prompt and first role skip filters to generic_guardrail_api #43684, feat(guardrails): add key alias and team skip filters to generic_guardrail_api #43757 and feat(guardrails): add call-type filters to generic_guardrail_api #43765, keep this copy'sfallbackargumentgeneric_guardrail_api.pywith feat(guardrails): add guardrail_information_scope to generic_guardrail_api #43680, feat(guardrails): add fire_and_forget dispatch to generic_guardrail_api #43681, feat(guardrails): add system prompt and first role skip filters to generic_guardrail_api #43684, feat(guardrails): add key alias and team skip filters to generic_guardrail_api #43757 and feat(guardrails): add call-type filters to generic_guardrail_api #43765_content_utils.pyFinal Attestation