feat(proxy): redact or drop individual batch records instead of rejecting the file - #37561
Conversation
Greptile SummaryThe PR now handles batch guardrail outcomes per record while distinguishing explicit content verdicts from guardrail failures
Confidence Score: 5/5The PR appears safe to merge No blocking failure remains
|
| Filename | Overview |
|---|---|
| litellm/proxy/openai_files_endpoints/batch_guardrails.py | Implements record scanning, verdict classification, spooled rewrites, record dropping, and per-record reporting |
| litellm/proxy/openai_files_endpoints/files_endpoints.py | Integrates batch scanning and rewritten upload streams into file creation with deterministic spool cleanup |
| litellm/exceptions.py | Adds an explicit blocked_content signal so batch handling can distinguish verdicts from technical failures |
| litellm/proxy/guardrails/guardrail_hooks/straiker/straiker.py | Separates Straiker content verdicts from fail-closed backend failures using the new exception signal |
| tests/test_litellm/proxy/openai_files_endpoint/test_batch_guardrails.py | Covers rewritten uploads, dropped records, fail-closed errors, reports, and exception-classification behavior |
Reviews (11): Last reviewed commit: "fix(proxy): register the scan spool befo..." | Re-trigger Greptile
|
|
||
| class _Redactor(CustomGuardrail): | ||
| async def async_pre_call_hook(self, user_api_key_dict, cache, data, call_type): |
There was a problem hiding this comment.
The new endpoint test directly patches proxy globals, callbacks, and routing rather than injecting mocked dependencies, coupling the test to implementation details and forcing unrelated test rewrites when proxy initialization or module ownership changes.
Context Used: CLAUDE.md (source)
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!
PR overviewThis pull request changes proxy batch-file handling so individual records can be redacted or dropped rather than causing the entire file to be rejected. The affected endpoint also reports guardrail scan results for batch processing. One issue has been addressed, but a low-impact log-integrity issue remains. An authenticated caller can place control characters in a filename or batch record identifier to forge lines in server logs when guardrail results are reported; the impact is limited to misleading or disrupting log output. Open issues (1)
Fixed/addressed: 1 · PR risk: 5/10 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b61af8d. Configure here.
45b549c to
abfb3d7
Compare
b61af8d to
6173989
Compare
|
Drop or abort now follows litellm's own |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6173989. Configure here.
|
Dropped records now name the guardrail that raised. @greptileai please review head 686ea1e |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 686ea1e. Configure here.
| len(scan_result.changes), | ||
| scan_result.scanned_records, | ||
| file.filename, | ||
| scan_result.summary(), |
There was a problem hiding this comment.
Low: Log injection through batch identifiers
scan_result.summary() includes each client-supplied custom_id, and the preceding argument includes the client-supplied filename. An authenticated caller can include CR/LF characters in either value and trigger a guardrail change to inject forged lines into server logs. Escape control characters and cap the rendered detail, or log only the counts here while retaining the structured report in metadata.
|
@greptileai please re-review. Latest commit fixes the metadata loss, memory retention, blocking rewrite, log injection and typing findings |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2c062b0. Configure here.
2c062b0 to
9180fb9
Compare
|
@greptileai please re-review, new head pushed with the latest round of fixes |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 47c8134. Configure here.
47c8134 to
b65623b
Compare
|
@greptileai please re-review, new head |
|
bugbot run |
142045e to
d2d124e
Compare
b65623b to
5be8c5a
Compare
|
Rebased onto current staging, which is where the missing test-quality gate script came from |
|
@greptileai please re-review, rebased head |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5be8c5a. Configure here.
…ting the file A single record tripping a guardrail rejected the whole upload, which is unusable for a file holding thousands of rows. A record a guardrail rewrites is now submitted in its rewritten form, a record it blocks is left out, and the create response reports every changed record by both custom_id and line so a caller can reconcile against the file it sent. The same outcome is written to the proxy log and to request metadata, so it is not visible only to the caller. A rewritten record goes straight to a spool and only its offset is carried, so a masking guardrail touching most rows of a large upload does not build a second copy of the file on the heap, and the rewrite runs off the event loop the way the sibling full-file validation does. Both proxy-injected metadata keys are captured from the record and restored exactly, including an explicit null, so a masked row keeps the tags that decide how it is attributed. A record is dropped only when a guardrail judged its content. `GuardrailRaisedException` now carries `blocked_content` for that, because half its raise sites in the repo signal an unreachable or unparseable backend under a fail-closed policy, and treating those as blocks would turn "refuse this request" into "drop this record and submit the rest". The default is off, so a raise that does not say what it means aborts the upload instead of silently shrinking the file.
…lly reached A guardrail that reports a technical failure as an HTTPException carrying a block status was read as a content block, so an unreachable backend under a fail-closed policy quietly shrank the file instead of failing the upload. Two in-tree integrations do exactly that, and one of them defaults to fail-closed, so the broken configuration was the default one. Such an exception is raised `from` the underlying error, which is a deliberate statement that something else caused it, and no content verdict in the repo is raised that way, so the chain now settles it. Implicit context is left alone, since a block raised inside an unrelated `except` would read as a failure. Two annotation errors in the same family: the one GuardrailRaisedException subclass in tree never opted into blocked_content, so a real block took the whole upload down with it, and straiker's block helper is reached both from its verdict and from its fail-closed handler, so it claimed a verdict for an outage. The helper now takes the flag from its caller. A record could also opt itself out of the chain. Guardrail selection reads a body-level `guardrails` key ahead of the proxy-injected list, and online that key can only add to the key and team selection, never replace it, so a batch record naming an empty list skipped every guardrail that was not default_on and was still reported as scanned. Every injected key is now stripped before dispatch and restored afterwards. A guardrail that reroutes a record to another model is honoured on the online path by rewriting the model, which the scan read as a rewrite and submitted in the same file, sending content to the provider the reroute existed to avoid. Every record of a batch file goes to one provider, so the upload is refused instead, naming the line. The scan spool is closed on the paths that never read it back.
…and close its spools The narrowed request metadata was installed under `litellm_metadata` only, but a record is scanned as the chat request it describes, and the guardrails that pick a policy from a request header read `metadata` instead. Noma choosing an application and Aim choosing a user both look there, so the header allowlist added for them did not reach either one and a batch record was still evaluated under the fallback policy. The scan metadata now goes into both bags, which are both stripped and restored, so neither survives into the record that ships. The scan spool was closed on the paths that abort, which are exactly the paths where it is empty, and left open on the one path where it holds the rewritten records. Nothing closed the rewrite output either, where before this feature the uploaded handle belonged to Starlette. The upload now owns both and closes them however it exits.
The scan spool was added to the request's cleanup list only after the rewrite returned, so a rewrite that raised, which for a spilled file can be as ordinary as the disk filling up, jumped to the handler with the list still empty and left the scan's own handle open. The rewrite also left its half-written output behind on that path, since nothing owns that handle until it is returned. Both now close.
5be8c5a to
175991c
Compare
|
Rebased onto staging now that #37519 has merged |
|
@greptileai please re-review, rebased head |
|
bugbot run |
| if isinstance(exc, GuardrailRaisedException): | ||
| return exc.blocked_content | ||
| if exc.__cause__ is not None: | ||
| return False |
There was a problem hiding this comment.
Upstream failures become content blocks
When Zscaler AI Guard propagates an upstream HTTP 400, 403, or 422 without an explicit cause, this fallback classifies the technical failure as a content verdict, causing the affected record to be dropped while the remaining batch is submitted.
Knowledge Base Used: Guardrails
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 175991c. Configure here.
|
Status-code heuristic is pre-existing staging code, and zscaler is untouched here. This PR narrows drops via blocked_content and cause |
|
@greptileai please re-review and rescore |
|
@greptileai The other 19 vendor sites conflate outage with verdict in one shape; fixing them is a separate contract change across 12 integrations. please re-review and rescore |
|
Full assurance run at head |
TLDR
Problem this solves:
How it solves it:
User Flow
Before: a platform team with a PII guardrail cannot use batch at all, because one row with a customer email rejects the whole job
purpose=batch, where row 2 contains an email address and row 3 asks for something the guardrail blocksAfter: the same upload is accepted, the offending rows are handled per record, and the response says what happened to each
purpose=batchlitellm_batch_guardrailreportingsubmitted_records: 3and naming row 2 asredactedand row 3 asdroppedRelevant issues
Linear ticket
Resolves LIT-5276
Pre-Submission checklist
uv run pytest tests/test_litellm/<your_test_file>.py -vScreenshots / Proof of Fix
Stacked on #37519, which makes guardrails run on batch uploads at all. Base for this QA is that PR's head,
8b2f6d6dd0Shared setup.
config.yamlconfigures two guardrails, one that masks and one that blocks, using the example guardrail shipped inlitellm/proxy/example_config_yaml/custom_guardrail.py:Both files hold four records: a clean row, a row containing
litellm, a row containingblockme, and another clean row.pr2_tagged.jsonlis the same file with every record carryinglitellm_metadata.tags, which is how a caller attributes rows to a cost center. Uploads go to the real OpenAI files API, and the content is read back from OpenAI rather than from the proxy's own copy. The proxy ran on port 4276 because 4000 was already in useBefore (8b2f6d6)
1. mixed file
curl -sS http://127.0.0.1:4276/v1/files -H "Authorization: Bearer sk-..." -F purpose=batch -F file=@pr2_mixed.jsonlHTTP 400and{"error":{"message":"{'error': 'Violated guardrail policy', 'guardrail': 'block-guard', 'guardrail_name': 'block-guard', 'guardrail_mode': 'pre_call'}","code":"400"}}2. mixed file whose records carry tags
curl -sS http://127.0.0.1:4276/v1/files -H "Authorization: Bearer sk-..." -F purpose=batch -F file=@pr2_tagged.jsonlHTTP 400with the same body, so the tagged rows never reach a provider either3. guardrail whose backend is unreachable under a fail-closed policy
risk2_input.jsonlholds three records; the middle one trips a guardrail that raisesGuardrailRaisedException("Singulr API unreachable (block_on_error=True): timed out"), the shape several integrations use when their backend is downcurl -sS http://127.0.0.1:4280/v1/files -H "Authorization: Bearer sk-..." -F purpose=batch -F file=@risk2_input.jsonlHTTP 400, since at this base any guardrail raising rejects the whole fileAfter (eef8331)
Full run, recorded live against real OpenAI:
1. mixed file
curl -sS http://127.0.0.1:4276/v1/files -H "Authorization: Bearer sk-..." -F purpose=batch -F file=@pr2_mixed.jsonlHTTP 200and{ "id": "file-KKVMnf3YSUCotz3Ks4Eutj", "object": "file", "purpose": "batch", "bytes": 594, "status": "processed", "litellm_batch_guardrail": { "submitted_records": 3, "modified_records": [ {"line": 2, "custom_id": "row-2", "action": "redacted", "guardrail": null}, {"line": 3, "custom_id": "row-3", "action": "dropped", "guardrail": "block-guard"} ] } }curl -sS -H "Authorization: Bearer $OPENAI_API_KEY" https://api.openai.com/v1/files/file-KKVMnf3YSUCotz3Ks4Eutj/content2. mixed file whose records carry tags
curl -sS http://127.0.0.1:4276/v1/files -H "Authorization: Bearer sk-..." -F purpose=batch -F file=@pr2_tagged.jsonlHTTP 200with the samelitellm_batch_guardrailreport,submitted_records: 3, row 2redactedand row 3droppedcurl -sS -H "Authorization: Bearer $OPENAI_API_KEY" https://api.openai.com/v1/files/file-Fp9HxXfXzorfM2BqxhzKGU/content3. guardrail whose backend is unreachable under a fail-closed policy
HTTP 400and{"error":{"message":"Guardrail raised an exception, Guardrail: down-guard, Message: Singulr API unreachable (block_on_error=True): timed out"}}686ea1e8a1, an earlier head of this PR, the same file returnedHTTP 200reporting{"line": 2, "custom_id": "scanner-down", "action": "dropped", "guardrail": "down-guard"}and onlyclean-1andclean-2reached OpenAI, so a scanner outage read as a policy block and the caller could not tell the difference4. real in-tree guardrail whose backend returns 503 for one record
xecguardguardrail against a stand-in backend that answers503only for the record containingtripwire;block_on_erroris left at its default, which is oncurl -sS http://127.0.0.1:4503/v1/files -H "Authorization: Bearer sk-..." -F purpose=batch -F file=@risk2_input.jsonlHTTP 400and{"error":{"message":"{'error': \"XecGuard API unreachable (block_on_error=True): Server error '503 Service Unavailable' ...\"}"}}9180fb97de, the previous head of this PR, the same upload returnedHTTP 200with{"line": 2, "custom_id": "scanner-down", "action": "dropped", "guardrail": "xecguard-pre"}and onlyclean-1andclean-2reached OpenAI. That guardrail reports an unreachable backend as anHTTPExceptioncarrying a block status, which the previous head read as a content verdictType
🆕 New Feature
Caveats (if any)
froman error is read as a failure to judgeguardrailskey is ignored for the scanFinal Attestation
Note
High Risk
Changes how guardrail enforcement applies to batch uploads: dropping or rewriting records instead of rejecting the file. A misclassified block vs. infrastructure failure would silently skip scanning some rows.
Overview
Batch
purpose=batchuploads no longer fail the whole file when a pre-call guardrail rewrites or blocks one row. Masked records are submitted as rewritten; blocked records are omitted; the rest go through. The create-file response (and proxy logs) includelitellm_batch_guardrailnaming each redacted or dropped line.GuardrailRaisedExceptionnow carriesblocked_contentso a real policy verdict can drop a row while a fail-closed outage, timeout, or unparseable backend still aborts the upload. Content blocks from HTTP 400/403/422, PII errors, and in-tree BLOCKED paths set the flag; sensitive-data reroutes still reject the file because a batch cannot follow a per-record model change.Rewrites are spooled off-heap, untouched lines are copied byte-for-byte, and a record cannot opt out of the team/key guardrail chain via its own
guardrailskey. If every record is dropped, the upload is rejected as empty.Reviewed by Cursor Bugbot for commit 175991c. Bugbot is set up for automated code reviews on this repo. Configure here.