Repository navigation
fix(bedrock)!: stream Converse passthrough frames through post_call guardrails instead of buffering - #46307
fix(bedrock)!: stream Converse passthrough frames through post_call guardrails instead of buffering#46307yucheng-berri wants to merge 18 commits into
Conversation
|
@litereview code-review |
✅ code-review5/5 · Ready for human reviewScope: full · Full PR review. Completed · Some checks not run. No correctness findings. The latest commit, The finding about the recursive Checks and execution details
It also changes Evidence:
View review · Reviewed |
There was a problem hiding this comment.
Devin Review found 1 potential issue.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
Merging this PR will not alter performance
|
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 2 · PR risk: 0/10 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@litereview code-review |
|
bugbot run |
|
@litereview full |
|
@litereview code-review |
|
bugbot run |
156d8c1 to
f945704
Compare
|
@litereview code-review |
|
bugbot run |
…as they arrive (LIT-9515)
…ardrails instead of buffering (LIT-9515) A post_call guardrail that scans through apply_guardrail now runs on /bedrock/.../converse-stream the way it runs on /v1/messages: frames reach the caller as the upstream sends them, every fifth frame and the end of the stream are scanned, and a block after the first frame ends the stream with an AWS exception frame. streaming_buffer_until_moderated still holds the stream and applies masks per content block. Guardrails that need the whole response (own async_post_call_success_hook, mask_response_content, native hooks, policy pipelines) keep the buffered path.
… with an exception frame (LIT-9515)
…tream failure hook (LIT-9515)
…ds (LIT-9515) - The framer ends the stream with an empty item once the upstream closes, so held masks are applied whether the upstream ends with messageStop, an exception or error frame, or a dropped connection - A guardrail HTTPException after the first frame reaches the relay, which logs the stream's spend, runs the failure hook and ends the stream with the exception frame, instead of ending inside the guardrail with no failure row - A failure before the first frame waits for the stream's spend to be logged, as a mid-stream failure does - Closing the response before its body starts closes the guardrail layers and the upstream - Each stacked guardrail layer gets its own copy of every frame, so an earlier layer's end-of-stream rewrite never reaches frames a later layer still holds - Deltas whose text ends in half a surrogate pair are decoded with the standard JSON parser
…ad of ending the stream (LIT-9515)
…-only, stacked, disconnect and burst cells (LIT-9515)
…ore Bedrock reports usage (LIT-9515) Bedrock sends usage only in the last frame of a stream, so a stream ended by a guardrail block or a client disconnect logged 0 prompt tokens while AWS still charges for the prompt. The stream collector now estimates them from the request the way chat completions streams do
…rough prompt estimate (LIT-9515)
|
bugbot run |
| ): | ||
| continue | ||
| leaf_attrs = type(cb).__dict__ | ||
| holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs |
There was a problem hiding this comment.
Medium · Default sampled relay drops guardrail masks on Bedrock converse-stream that the buffered path used to apply (PII leak)
Still open, and the code is unchanged since the last decision.
_passthrough_stream_guardrails sends every guardrail that defines apply_guardrail to the new relay path. A guardrail without a leaf-class async_post_call_streaming_iterator_hook gets holds_until_moderated = False, so it runs in sampled mode. In that mode, BedrockPassthroughGuardrailHandler.process_output_streaming_response writes the guardrail's rewritten texts back only once the stream has ended (handler.py:745-746). By then every frame has already been relayed. Before this PR, the same request was buffered through de_anonymize_event_stream → post_call_success_hook, and the rewritten text was re-encoded into the frames.
Concrete failure: attach a text-rewriting post_call guardrail to a key that calls /bedrock/model/<id>/converse-stream. Examples:
- enterprise secret detection (
apply_guardrailreturnsredact_text(text)) - Noma v2 anonymization
- Generic Guardrail API
- custom code
After the upgrade, secrets and PII reach the client unredacted, with no config change and nothing logged.
The PR now carries fix(bedrock)!: and a named approver. That records acceptance of the risk but doesn't fix it. Operators have to opt in to streaming_buffer_until_moderated: true to get masking back, and per the PR text that doesn't work for Noma v2 or custom code.
Possible fixes: keep these guardrails on the buffered path, or default them to held mode, when their apply_guardrail can rewrite output.
Evidence
common_request_processing.py:3466: holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs.
common_request_processing.py:3467-3473: the fallback to the buffered path only triggers on a missing apply_guardrail, use_native_lifecycle_hooks, mask_response_content, run_in_parallel or release-on-scan.
bedrock/passthrough/guardrail_translation/handler.py:745-746: if self._check_streaming_has_ended(frames): _write_back_stream_texts(...).
enterprise/litellm_enterprise/enterprise_callbacks/secret_detection.py:~606: return {**inputs, "texts": [self.redact_text(text) for text in texts]}, with no streaming iterator override, so it runs in sampled relay.
Leaf-class sweep: about 30 guardrails (Noma v2, generic_guardrail_api, custom_code, secret_detection, ...) define apply_guardrail without async_post_call_streaming_iterator_hook.
Latest summary by full · View this review · Reviewed 8f532fd9a93f
There was a problem hiding this comment.
Same accepted risk the owner signed off on 2026-10-14, unchanged in this commit, and sampled chat, messages and responses streams drop these masks too
There was a problem hiding this comment.
Finding still applies
The reply cites the owner's sign-off and consistency with other streaming routes. Neither is technical evidence that the defect is wrong or fixed. At head, the routing logic and the write-back-only-at-stream-end behaviour are unchanged. A guardrail that defines apply_guardrail but no leaf-class async_post_call_streaming_iterator_hook (for example enterprise secret detection) is still relayed in sampled mode on Bedrock converse-stream. Its rewrites never reach the client, whereas the base buffered path applied them. This remains an accepted but real backwards-incompatible regression.
Evidence
litellm/proxy/common_request_processing.py:3466: holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs is unchanged.
litellm/proxy/common_request_processing.py:3467-3474: the buffered fallback triggers only on a missing apply_guardrail, use_native_lifecycle_hooks, mask_response_content, run_in_parallel or release-on-scan. Nothing routes a text-rewriting guardrail to buffered or held mode.
litellm/llms/bedrock/passthrough/guardrail_translation/handler.py:745-746: if self._check_streaming_has_ended(frames): _write_back_stream_texts(...). Rewrites apply only after every frame has been relayed in sampled mode.
enterprise/litellm_enterprise/enterprise_callbacks/secret_detection.py:592-606: apply_guardrail returns redacted texts, and the file defines no async_post_call_streaming_iterator_hook.
The reply (comment 4286828014) states the risk is accepted and the code is unchanged in this commit. That confirms the defect remains.
Reviewed 8f532fd9a93f · View follow-up
…am_guardrail_relay # Conflicts: # litellm/llms/bedrock/passthrough/transformation.py # litellm/llms/pass_through/guardrail_translation/handler.py
…rock when a held stream's client leaves - Noma v2 scans on the converse-stream relay carry stream, and the last one carries the finished Converse response with its stopReason, as chat sends them, so Noma keeps one record per reply. Other guardrails get the request unchanged. When the last sampled scan already covered the whole answer, the end-of-stream scan still runs for Noma so it gets the finished answer - A held relay stream whose client leaves before the first frame stops reading Bedrock and returns 499, like chat. When Bedrock sent nothing yet, the budget reservation and the parallel-request slot are released - A cancelled wait for the first streamed chunk now stops the upstream read instead of leaving it running - A Noma v2 scan cancelled before Noma answers records not_run instead of success with an empty response
|
@litereview full |
|
bugbot run |
| ): | ||
| continue | ||
| leaf_attrs = type(cb).__dict__ | ||
| holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs |
There was a problem hiding this comment.
Medium · Default sampled relay drops guardrail masks on Bedrock converse-stream that the buffered path used to apply (PII leak)
Still open. The code is unchanged since the last two decisions upheld this finding.
How it happens
_passthrough_stream_guardrails sends every guardrail with a leaf-class apply_guardrail down the new frame relay. If the class has no leaf async_post_call_streaming_iterator_hook, it gets holds_until_moderated = False, so the unified guardrail runs it in sampled mode and forwards each frame as it arrives.
BedrockPassthroughGuardrailHandler.process_output_streaming_response writes rewritten texts back into the frames only once _check_streaming_has_ended(frames) is true. By then every content frame has already reached the client.
On base, the same request was buffered. It went through de_anonymize_event_stream and then post_call_success_hook, and the rewritten text was re-encoded into the frames before anything was sent.
Failure scenario
- Attach enterprise secret detection to a key as a post_call guardrail. Its
apply_guardrailreturnsredact_text(text), and the class has no streaming iterator override. - Call
/bedrock/model/<id>/converse-streamand get the model to echo an API key. - Base: the client receives
[REDACTED]inside thecontentBlockDeltaframes. - Head: the client receives the raw secret. Nothing is logged and no config changed.
Noma v2 anonymization, the Generic Guardrail API and custom-code guardrails fail the same way. The PR text says Noma v2 and custom code ignore streaming_buffer_until_moderated, so the documented opt-out doesn't restore masking for them.
Status
The PR title has ! and the body names an approver, so shipping this is a recorded decision. It is still a client-observable data-exposure regression, not a fix.
Possible fix
Keep guardrails whose apply_guardrail can rewrite output on the buffered path, or default them to held mode (holds_until_moderated=True) on this route.
Evidence
litellm/proxy/common_request_processing.py:3490: holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs.
litellm/proxy/common_request_processing.py:3491-3498: the buffered fallback (return None) triggers only on a missing leaf apply_guardrail, use_native_lifecycle_hooks, mask_response_content, run_in_parallel or release-on-scan.
litellm/llms/bedrock/passthrough/guardrail_translation/handler.py:775-776: if ended: _write_back_stream_texts(frames, groups, ...). Rewrites land only after the stream has ended.
unified_guardrail.py sampled branch: when not buffer_until_moderated, original_items = (copy.deepcopy(item),) are yielded right after each scan, so the original text is sent.
enterprise/litellm_enterprise/enterprise_callbacks/secret_detection.py:592: defines apply_guardrail, and a grep finds no async_post_call_streaming_iterator_hook in the file.
Latest summary by full · View this review · Reviewed c694684a0ae4
There was a problem hiding this comment.
Matches chat, messages and responses streaming, accepted by the ticket owner. Held mode still masks. I added secret detection to the PR body's list.
There was a problem hiding this comment.
Finding still applies
The reply accepts the behaviour; it doesn't change it. At head, the guardrail routing and the stream-end-only write-back are unchanged.
Guardrails that rewrite text in apply_guardrail but have no streaming iterator override run in sampled mode by default on /bedrock/.../converse-stream. In that mode the original frames reach the client before any rewrite is applied. Before this PR, the same request was buffered and masked.
Held mode does mask, but it is opt-in. The PR says Noma v2 and custom code ignore that setting, so for them the opt-out doesn't help. Matching other routes and getting product sign-off is a recorded decision to ship the regression, not technical evidence that the regression is gone.
Evidence
litellm/proxy/common_request_processing.py:3489-3490: holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs, so a guardrail that defines only apply_guardrail defaults to sampled relay.
litellm/proxy/common_request_processing.py:3491-3498: the buffered fallback still triggers only on a missing leaf apply_guardrail, use_native_lifecycle_hooks, mask_response_content, run_in_parallel or release-on-scan. Nothing keeps a rewriting guardrail on the buffered or held path.
litellm/llms/bedrock/passthrough/guardrail_translation/handler.py:816 and :832-833: ended = self._check_streaming_has_ended(frames) and then if ended: _write_back_stream_texts(...). The docstring at :806-808 says rewrites reach the caller only when the stream is held until moderated.
enterprise/litellm_enterprise/enterprise_callbacks/secret_detection.py:592 defines apply_guardrail, and grep finds no async_post_call_streaming_iterator_hook in that file. Secret detection therefore takes the default sampled relay and its redactions don't reach the wire.
Reviewed fde82b354b3b · View follow-up
…reasoning as Converse blocks
|
@litereview full |
|
bugbot run |
| ): | ||
| continue | ||
| leaf_attrs = type(cb).__dict__ | ||
| holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs |
There was a problem hiding this comment.
Medium · Default sampled relay on Bedrock converse-stream drops masks from rewriting guardrails that the buffered path used to apply
This finding is unchanged from earlier rounds. I re-checked it at head fde82b35, and none of the code involved has changed.
What happens
_passthrough_stream_guardrails sets holds_until_moderated only when the leaf class defines async_post_call_streaming_iterator_hook. It falls back to the buffered path only for these cases:
- the leaf class has no
apply_guardrail use_native_lifecycle_hooksmask_response_contentrun_in_parallel- release-on-scan
- policy pipelines
Any other guardrail that defines apply_guardrail is relayed in sampled mode. In that mode, BedrockPassthroughGuardrailHandler.process_output_streaming_response calls _write_back_stream_texts only if ended. By then the unified guardrail has already sent the original frames to the client.
Failure scenario
- Attach a post_call guardrail that rewrites text (secret detection, Noma v2 anonymization, the Generic Guardrail API, or custom code) to a key.
- Call
/bedrock/model/<id>/converse-streamwith a prompt that makes the model echo a secret. - Base: the stream is buffered, and
de_anonymize_event_streamre-encodes the masked text into thecontentBlockDeltaframes. - Head: the frames reach the client with the raw secret. Nothing is logged and no config changed.
The documented opt-out is streaming_buffer_until_moderated: true. The PR says Noma v2 and custom code ignore it, so those users have no way to get masking back.
Status
The author replied with sign-off and the argument that other routes behave the same way. Neither changes the code, so this is still a client-visible data-exposure regression.
Possible fix
Keep guardrails that can rewrite output on the buffered or held path for this route, or default them to holds_until_moderated=True.
Evidence
litellm/proxy/common_request_processing.py:3489-3490: holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs
litellm/proxy/common_request_processing.py:3491-3498: the only conditions that return None (buffered path) are a missing apply_guardrail, native hooks, mask_response_content, run_in_parallel and release-on-scan
litellm/llms/bedrock/passthrough/guardrail_translation/handler.py process_output_streaming_response: if ended: _write_back_stream_texts(...), and its docstring says rewrites reach the caller only when the stream is held until moderated
enterprise/litellm_enterprise/enterprise_callbacks/secret_detection.py defines apply_guardrail and no async_post_call_streaming_iterator_hook, so it takes the sampled relay by default
Latest summary by full · View this review · Reviewed fde82b354b3b
…calls a tool without input
|
@litereview full |
|
bugbot run |
| ): | ||
| continue | ||
| leaf_attrs = type(cb).__dict__ | ||
| holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs |
There was a problem hiding this comment.
Medium · Default sampled relay on Bedrock converse-stream sends rewriting guardrails' unmasked text to the client (redaction regression)
Still open. The code is unchanged since the earlier decisions, and I reran the reproducer at head 11bf9185.
How it happens
_passthrough_stream_guardrailssends any guardrail with a leaf-classapply_guardraildown the new frame relay.- If the class has no leaf
async_post_call_streaming_iterator_hook, it getsholds_until_moderated=False. unified_guardrailtherefore runs it in sampled mode and forwards the original frames right after each scan.BedrockPassthroughGuardrailHandler.process_output_streaming_responsewrites the guardrail's rewrittentextsback into the frames only whenendedis true. By then everycontentBlockDeltahas already gone to the client.
On base, the same request was buffered whole and went through _handle_event_stream_allm_passthrough_route, which re-encoded the rewritten text into the frames.
Failure scenario
- Attach a post_call guardrail whose
apply_guardrailreturns rewritten text. Examples: enterprise secret detection (redact_text), Noma v2 anonymization, the Generic Guardrail API, custom code. - Call
/bedrock/model/<id>/converse-streamand have the model echo a secret. - Base: the client gets
[REDACTED]. - Head: the client gets the raw secret. No config changed and nothing is logged.
The documented opt-out (streaming_buffer_until_moderated: true) has to be turned on by each operator. Per the PR text, Noma v2 and custom code ignore it.
Status
The ! marker and the named approver record that the risk was accepted. They are not a fix.
Possible fix
On this route, keep guardrails whose apply_guardrail can rewrite output on the buffered path, or make held mode their default.
Evidence
litellm/proxy/common_request_processing.py:3489-3490: leaf_attrs = type(cb).__dict__; holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs.
litellm/proxy/common_request_processing.py:3491-3498: the buffered fallback runs only when the leaf has no apply_guardrail, or on use_native_lifecycle_hooks, mask_response_content, run_in_parallel or release-on-scan. Nothing covers guardrails that rewrite text.
litellm/llms/bedrock/passthrough/guardrail_translation/handler.py:852-853: if ended: _write_back_stream_texts(...). The docstring at :819-820 says the rewrites reach the caller only when the stream is held until moderated.
Local reproducer /tmp/repro.py (real ProxyLogging and the real Bedrock framer/encoder, a guardrail that replaces sk-SECRET in apply_guardrail, the secret split across three deltas). Output at head: selected [('Redact', False)] then raw secret on wire: True redacted: False.
Latest summary by full · View this review · Reviewed 11bf9185a4a1
There was a problem hiding this comment.
Kept on purpose to match chat, messages and responses, per the ticket owner. Held mode still masks, and the PR body marks it breaking.
There was a problem hiding this comment.
Finding still applies
The reply says the behaviour was kept on purpose, that held mode still masks, and that the PR is marked breaking. All three are true, but none shows the defect is wrong or fixed.
At head, the code still works like this:
_passthrough_stream_guardrailssends a guardrail down the frame relay when its class definesapply_guardrail. If the class doesn't also defineasync_post_call_streaming_iterator_hook, the guardrail getsholds_until_moderated=False.- None of the buffered-path exclusions covers guardrails that rewrite text.
- In sampled mode,
process_output_streaming_responsewrites rewrites back only onceendedis true. By then the originalcontentBlockDeltaframes have already reached the client.
On base, the same request was buffered and the rewrite (for example [REDACTED]) reached the client. With no config change, the raw text now does. Held mode is opt-in, and the PR itself says Noma v2 and custom code ignore that setting, so for them the opt-in doesn't help.
The ! marker and the owner's sign-off record that the risk was accepted. They don't remove it, so I'm keeping the regression open.
Evidence
litellm/proxy/common_request_processing.py:3489-3490: leaf_attrs = type(cb).__dict__; holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs.
litellm/proxy/common_request_processing.py:3491-3499: the buffered fallback runs only when there is no leaf apply_guardrail, or with use_native_lifecycle_hooks, mask_response_content, run_in_parallel or release-on-scan. Otherwise the guardrail is appended to relayed.
litellm/llms/bedrock/passthrough/guardrail_translation/handler.py:852-853: if ended: _write_back_stream_texts(...). The docstring at :817-819 says rewrites "reach the caller only when the stream is held until moderated".
The reply doesn't point to a code change, a test or a different observed result. It only acknowledges the behaviour and says it was approved.
Reviewed 11bf9185a4a1 · View follow-up
[High impact] Refactors streaming response handling for guardrails across multiple providers. This PR should not merge yet because a stacked held guardrail can still release text without its masks FindingsSummaryThis PR relays eligible Bedrock
New code also needs the repository’s required variable annotations
Reviews (2) · Last reviewed commit: "test(bedrock): expect the ValidationExce..." · Reviewed by Greptile |
| *(_converse_event("contentBlockStop", {"contentBlockIndex": index}) for index in sorted(started - stopped)), | ||
| _converse_event("contentBlockDelta", {"contentBlockIndex": block_index, "delta": {"text": exc.message}}), | ||
| _converse_event("contentBlockStop", {"contentBlockIndex": block_index}), | ||
| _converse_event("messageStop", {"stopReason": "guardrail_intervened"}), |
There was a problem hiding this comment.
Held masks disappear after blocking
When a sampled Bedrock guardrail with disable_exception_on_block: true blocks mid-answer, these replacement frames reach the next guardrail as raw bytes. A later held masking guardrail ignores those bytes when checking whether the stream ended. It scans the earlier text but skips writing its masks back, then releases that text unmasked. Convert the replacement bytes back into ConverseStreamFrame items before passing them to later guardrails.
How this was verified: Later guardrails exclude raw bytes from their frame list, and mask write-back requires a typed messageStop or ends_stream frame.
…am_guardrail_relay
…held block before the first frame Main's AWS-shaped passthrough errors (#46522) answer a guardrail block before the first frame with x-amzn-errortype ValidationException, so boto3 reads the code and the guardrail message instead of a bare 400.
|
bugbot run |
|
@litereview full |
| ): | ||
| continue | ||
| leaf_attrs = type(cb).__dict__ | ||
| holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs |
There was a problem hiding this comment.
High · Default sampled relay on Bedrock converse-stream sends rewriting guardrails' unmasked text to the client (still present at 8ab2027)
This finding is unchanged from the open, upheld history entry. I re-checked it at this head.
What happens
_passthrough_stream_guardrails sends any guardrail that defines apply_guardrail and isn't mask_response_content, run_in_parallel, native-hooks or release-on-scan to the frame-by-frame relay. Unless the guardrail class itself defines async_post_call_streaming_iterator_hook, it runs in sampled mode: holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs.
In sampled mode, the unified guardrail yields copy.deepcopy(item) copies of the original frames. BedrockPassthroughGuardrailHandler.process_output_streaming_response writes rewrites back only if ended: (handler.py around line 852), after those frames have already gone out.
On base, the same request went through de_anonymize_event_stream, which calls post_call_success_hook on the buffered body and re-encodes the rewritten text. The client got masked output.
Failure scenario
- Attach a post_call guardrail on a key whose
apply_guardrailredacts text. Examples: enterprise secret detection (secret_detection.pydefinesapply_guardrailwith no streaming iterator override), Noma v2 anonymization, Generic Guardrail API, or custom code. - Call
/bedrock/model/<id>/converse-streamwith a prompt that makes the model echo a card number or secret. - Base: the client's
contentBlockDeltaframes carry the redacted text. - Head: the client gets the raw text. Nothing is logged, and no config changed.
The documented opt-out, streaming_buffer_until_moderated: true, doesn't help secret detection or custom code, because the PR says those ignore the setting.
Possible fix
Keep guardrails whose apply_guardrail can rewrite output on the buffered path for this route, or default them to held mode (holds_until_moderated=True).
Evidence
Reproduced at head 8ab2027 with a real ProxyLogging.async_post_call_event_stream_iterator_hook over BedrockPassthroughGuardrailHandler.event_stream_items (the PR's own _relay_through_post_call_guardrail helper). The guardrail's apply_guardrail replaces 4111 with [X]. Output: CLIENT GOT: ['Card: 4111 1111 ', '1111 1111.', 'Thanks!'], so the redaction never reached the wire.
litellm/proxy/common_request_processing.py:3490: holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs.
litellm/proxy/common_request_processing.py:3491-3498: the buffered fallback triggers only on a missing leaf apply_guardrail, use_native_lifecycle_hooks, mask_response_content, run_in_parallel or release-on-scan.
litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py:1324-1326: original_items = (... else (copy.deepcopy(item),)) holds the copies that are released after each sampled scan.
litellm/llms/bedrock/passthrough/guardrail_translation/handler.py process_output_streaming_response: if ended: _write_back_stream_texts(...), so rewrites land only after the stream ends.
The route-level test test_bedrock_converse_stream_relays_frames_for_an_apply_guardrail_guardrail uses _apply_guardrail, whose apply_guardrail returns inputs unchanged, so no test covers a rewriting guardrail on the relay.
Latest summary by full · View this review · Reviewed 8ab2027da160
There was a problem hiding this comment.
Kept on purpose to match chat, per the ticket owner. Held mode still masks, and Noma v2 now honors it after main's #46606.
There was a problem hiding this comment.
Finding still applies
The reply says the change is intentional and that held mode still masks. That describes a known limitation; it isn't evidence that the defect is gone. At head, a rewriting guardrail that defines apply_guardrail without a leaf streaming-iterator hook, such as CustomCodeGuardrail, still goes to the sampled relay with holds_until_moderated=False. Unredacted frames reach the client, where base buffered the response and re-encoded the masked text. Custom code can't opt into held mode per guardrail: its initializer never forwards streaming_buffer_until_moderated, and the class has no such attribute. The secret-detection example in the original finding was wrong, because that class sets use_native_lifecycle_hooks=True and stays buffered. That correction narrows the affected set but doesn't remove the defect.
Evidence
litellm/proxy/common_request_processing.py:3489-3499: holds_until_moderated = "async_post_call_streaming_iterator_hook" in leaf_attrs. The relay falls back to buffering only when the leaf lacks apply_guardrail, or when native hooks, mask_response_content, run_in_parallel or release-on-scan apply.
I ran this at head: CustomCodeGuardrail(custom_code=<modify replacing 4111>, event_hook='post_call') reports 'apply_guardrail' in leaf=True, 'async_post_call_streaming_iterator_hook' in leaf=False, use_native_lifecycle_hooks=False, mask_response_content=False, run_in_parallel=False, and streaming_buffer_until_moderated is not set. So it is relayed in sampled mode.
litellm/proxy/guardrails/guardrail_hooks/custom_code/__init__.py: initialize_guardrail passes only name, code, timeout, event_hook and default_on, never streaming_buffer_until_moderated.
litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py:1062-1068: resolve_streaming_flag reads the guardrail attribute, then guardrail_config, then the global optional params. Custom code has no attribute or config, so the default False applies.
litellm/proxy/common_request_processing.py:3748: the buffered de_anonymize_event_stream path, which re-encodes rewritten text, is now used only when _passthrough_stream_guardrails returns None.
Correction: enterprise/litellm_enterprise/enterprise_callbacks/secret_detection.py:538 sets use_native_lifecycle_hooks = True, so secret detection stays buffered and was a wrong example in the original finding.
Reviewed 8ab2027da160 · View follow-up
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 8ab2027. Configure here.
| current = frame.guardrailed_payload if frame.guardrailed_payload is not None else frame.payload | ||
| if frame.event_type != "contentBlockDelta" or current is None: | ||
| continue | ||
| rewritten = copy.deepcopy(current) | ||
| changed = False |
There was a problem hiding this comment.
AGENTS.md requires Final annotations and reasons for reassignment. These locals lack both. Satisfy that requirement before merging
Context Used: AGENTS.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!
TLDR
Problem this solves:
/bedrock/.../converse-streamheld the whole stream until Bedrock finishedClientErrorstreammark and no finished answerHow it solves it:
/v1/messagesstreaming_buffer_until_moderated: trueto hold and maskmask_response_content, a held stream withstreaming_buffer_release_on_scan,run_in_parallel, native hooks, policy pipelines) keep the buffered pathstream: true, and the last one carries the finished answer with its tool calls, one record per reply like chatIntentional product change: in the default sampled mode, frames reach the caller before the guardrail's verdict and masks are not applied, exactly as on
/v1/chat/completions,/v1/messagesand/v1/responses. Setstreaming_buffer_until_moderated: trueto keep masking. Noma v2 honors that setting since #46606 merged to main, and custom code still ignores it on every routeUser Flow
Before: an app using the AWS SDK
ConverseStreamthrough the proxy stalls, then times out, once a post_call guardrail is onconverse_streamwithendpoint_url=https://litellm-domain/bedrockand a 5 s read timeoutReadTimeoutErrorClientError: An error occurred (400) ...with an empty messageAfter: the same app streams normally and sees blocks as readable SDK errors
converse_streamwith the same 5 s read timeoutEventStreamError (validationException)with the guardrail's message, after the text already receivedLinear ticket
Refs LIT-9515
When this PR opened, a block before the first frame (held mode) returned LiteLLM's JSON error, and Noma v2 ignored the streaming settings. Both are now fixed on main, by #46522 (LIT-9617) and #46606 (LIT-9633), and this branch has both since main was merged in at 8ab2027. invoke-with-response-stream is LIT-9618
#46242 (an earlier attempt at this ticket) was closed by its author before this PR opened
Docs: BerriAI/litellm-docs#2340
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv 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 moreDelays 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
Setup for the boto3 cases. One model
claude-haiku=bedrock/us.anthropic.claude-haiku-4-5-20251001-v1:0(real Bedrock, us-east-1), and a real AWS Bedrock guardrail (guardrail: bedrock, wordzzzforbiddenblocked, EMAIL anonymized) attached as apost_callguardrail to the caller's key, once withstreaming_buffer_until_moderated: false(sampled), once with that plusstreaming_end_of_stream_only: true, and once with the default (held). The client is boto3bedrock-runtimeconverse_streamwithendpoint_url=http://<proxy>/bedrock, Postgres-backed proxy, 4 workersSetup for the Noma cases. Real Noma v2 (
guardrail: noma,use_v2: true,api.noma.security) behind a recorder that logs each scan request and forwards it, so the request the proxy sent is visible. The held-stream disconnect case stacks a real Straiker guardrail, which holds the stream by defaultBefore (464bb5a)
no guardrail, read_timeout=5
client.converse_stream(modelId="claude-haiku", ...)with a key with no guardrail, asking for a 700-word story,read_timeout=5stopReason: end_turn, text:# The Keeper's Vigil Samuel had kept the lighthouse for for ... atient vigil above the rocks and the restless sea.post_call guardrail, read_timeout=5
client.converse_stream(modelId="claude-haiku", ...)with the key with the Bedrock guardrail (sampled), asking for a 700-word story,read_timeout=5ReadTimeoutError: Read timeout on endpoint URL: "http://127.0.0.1:4971/bedrock/model/claude-haiku/converse-stream"post_call guardrail scanning at end of stream only, read_timeout=5
client.converse_stream(modelId="claude-haiku", ...)with the key with the Bedrock guardrail (streaming_end_of_stream_only: true), asking for a 700-word story,read_timeout=5ReadTimeoutError: Read timeout on endpoint URL: "http://127.0.0.1:4971/bedrock/model/claude-haiku/converse-stream"post_call guardrail blocks mid-stream
client.converse_stream(modelId="claude-haiku", ...)with the key with the Bedrock guardrail (sampled), asking for about 120 words whose third sentence containszzzforbidden,read_timeout=60ClientError: An error occurred (400) when calling the ConverseStream operation:post_call guardrail masks (held stream)
client.converse_stream(modelId="claude-haiku", ...)with the key with the Bedrock guardrail (held, the default), asking for a note asking to emailjane.doe@example.com,read_timeout=60stopReason: end_turn, text:# Note Could you please email Jane Doe at {EMAIL} regarding the meeting? Thank you for following up on this.Client disconnect on a 3.5k-token prompt
curl -sN http://<proxy>/bedrock/model/claude-haiku/converse-stream -H "x-litellm-api-key: $KEY" -d @big.json | head -c 400, wherebig.jsonis a 3.5k-token prompt asking for a 250-word story and$KEYhas the sampled guardrail, so the client hangs up after the first frames9227e0ce-4125-46be-a861-bff321aa9f5cClient leaves a held stream before the first frame
POST /bedrock/model/claude-haiku/converse-streamasking for a 300-word answer, with a key whose guardrail holds the stream (Straiker), and the client gives up after 1.5 s, before any frame68456d48-ab27-4b76-984e-9e6ce558bac8shows one Success row for the whole answer, 25 prompt + 495 completion tokens, $0.002750, 6.71 sAdmin UI Logs for a blocked stream (custom_code guardrail)
POST /bedrock/model/claude-haiku/converse-streamstream is blocked by acustom_codepost_call guardrail, HTTP 400 JSONcode says noa9c470fb-b8f6-4942-84e1-1e67a3d2a0e8shows the Before screenshots under User Flow (one Success row, 36 + 170 tokens, $0.000975, no Failure row)Noma v2 scans of a streamed answer
converse_streamwith a Noma v2 key, five prompts (card number, email, prompt injection, PII, a long answer), three runs eachstreammark and no finishedresponseinrequest_dataNoma v2 scan of a reply that only calls a tool
converse_streamwithtoolChoiceforcingget_time, a tool with no parameters, on a Noma v2 key, sampled and held, two runs eachAfter (11bf918)
no guardrail, read_timeout=5
client.converse_stream(modelId="claude-haiku", ...)with a key with no guardrail, asking for a 700-word story,read_timeout=5stopReason: end_turn, text:# The Keeper's Watch Marcus had tended the lighthouse for t ... s his light. In the morning, he would climb again.post_call guardrail, read_timeout=5
client.converse_stream(modelId="claude-haiku", ...)with the key with the Bedrock guardrail (sampled), asking for a 700-word story,read_timeout=5stopReason: end_turn, text:# The Keeper's Watch Thomas had been keeping the lighthouse ... one. That was enough. That had always been enough.post_call guardrail scanning at end of stream only, read_timeout=5
client.converse_stream(modelId="claude-haiku", ...)with the key with the Bedrock guardrail (streaming_end_of_stream_only: true), asking for a 700-word story,read_timeout=5stopReason: end_turn, text:# The Lighthouse Keeper Thomas had been keeping the lightho ... his responsibility, his conversation with the sea.post_call guardrail blocks mid-stream
client.converse_stream(modelId="claude-haiku", ...)with the key with the Bedrock guardrail (sampled), asking for about 120 words whose third sentence containszzzforbidden,read_timeout=60EventStreamError validationException: Violated guardrail policy# The Joy of Gardening Gardening is a rewarding hobby that ... ony, there's always space to grow something. The zpost_call guardrail masks (held stream)
client.converse_stream(modelId="claude-haiku", ...)with the key with the Bedrock guardrail (held, the default), asking for a note asking to emailjane.doe@example.com,read_timeout=60stopReason: end_turn, text:# Note Please email {EMAIL} to confirm your attendance at t ... if you have any questions or scheduling conflicts.Client disconnect on a 3.5k-token prompt
curlwith the same prompt and keye9dc30df-4e10-4293-a754-e4ffd872c65d/v1/chat/completionsstream gets) + 12 completion tokens, $0.003578, 1.14 sClient leaves a held stream before the first frame
62b20d65-1d39-4efe-9de7-e4a9d4d48daeshows one Success row for what Bedrock sent before the stop, 25 prompt + 27 completion tokens, $0.000176, 1.47 s. A chat completions stream on the same key gave 25 + 40 tokens and stopped 0.01 s after the client leftAdmin UI Logs for a blocked stream (custom_code guardrail)
validationExceptionframe withcode says nobf848796-84ea-46e3-aee5-8cc02cd29666shows the After screenshots under User Flow (Success row for 36 prompt + 60 relayed tokens, $0.000370, plus a Failure row)Noma v2 scans of a streamed answer
stream: trueon every Noma scan and the finishedresponseon the last scan only, as chat sends it. A card-number reply got 2 scans, a long answer 34get_weathertool call ended with a Noma scan whoseresponseholds{"text": "It's a beautiful day to check the weather!"}and{"toolUse": {"toolUseId": "tooluse_JQN1...", "name": "get_weather", "type": "tool_use", "input": {"city": "Paris"}}},stopReason: tool_useNoma v2 scan of a reply that only calls a tool
get_timecall, sampled and held, two runs eachtexts: [],stream: trueandresponsecontent[{"toolUse": {"toolUseId": "tooluse_mqty...", "name": "get_time", "type": "tool_use", "input": {}}}]. Chat completions on the same key also gives one scan, with the tool call intool_callsWith main's #46606 (11bf918 merged with main ecbea93, the same code as head 8ab2027)
Noma v2 masks in held mode
converse_streamon real Bedrock Haiku with a real Noma v2 guardrail (use_v2: true), asking for a line with a test card number, alone and at the end of a 150-word note, two runs each, on main and on this branch merged with mainstreaming_buffer_until_moderated: true: 4 of 4 replies masked on both (Card on file: *******, exp 12/29.), with one Noma scan per reply. On the merge that scan carriesstream: trueand the finishedresponsestreaming_end_of_stream_only: true: main masked 8 of 8 because it buffers the whole reply. The merge sent the first frame at 0.7-0.8 s and delivered the card unmasked 8 of 8, as chat does (the breaking change below)Integration tests and audit
tests/integration/observability/test_bedrock_converse_stream_post_call_guardrail.pydrives a real proxy with Postgres, Redis, a scripted Bedrock upstream and a scripted guardrail vendor through boto3 and raw HTTP. Its 28 cells cover streaming before the upstream finishes, mid-stream blocks, held-mode masks (including a held Bedrock guardrail withstreaming_buffer_release_on_scan: true), upstream and vendor failures, end-of-stream-only mode, stacked guardrails (including arun_in_parallelguardrail listed before a masking one), client disconnects in sampled and held mode, a parallel request slot freed by a held disconnect, spend and failure rows, Noma v2 stream marks (including a reply that only calls a tool), and bursts. Base 464bb5a fails 16 of them, and head 11bf918 passes all 28 on two runs. The tool-only Noma cell was red on this PR's earlier head fde82b3, and the parallel-slot cell was red on e82e15f (the follow-up request got 429)Main was merged in at 8ab2027. Main's #46522 now answers a held block before the first frame as
ValidationExceptionwith the guardrail message instead of a bare 400, so that cell checks for it. At 8ab2027 this file plus #46606'stests/integration/streaming/test_noma_v2_streaming_settings.pypass on two runs (277 passed, identical selections), and the new merge base ecbea93 fails the same 16 relay cellsTwo earlier regression sweeps are folded in. A held guardrail with
streaming_buffer_release_on_scan: trueand arun_in_parallelguardrail listed before a masking one both keep the buffered path, so they mask and scan the masked text as main does (real Bedrock A/B, base and head masked on every run)After merging main, 8ab2027 re-ran the integration files, the live A/B batch against the new merge base ecbea93, worker-kill chaos and billing, with the same results as below, and the final audit at 8ab2027 is PASS. The audit at 11bf918 re-ran the integration file (28 of 28 twice on head, 16 failing on base), the live A/B batch, worker-kill chaos and billing on both legs, and the Noma v2 and held-disconnect cells on real Bedrock and real Noma. Every behavior change below was signed off by the ticket owner, including the 5xx exception frame and the metrics and hooks the relay skips
Type
🐛 Bug Fix
Behavior changes
Removed / renamed
Changed on
/bedrock/.../converse-streamwith a post_call guardrail that scans throughapply_guardrailstreaming_buffer_until_moderated: truekeeps per-content-block masking for guardrails that read it, such as the Generic Guardrail API and, since fix(guardrails): honor streaming settings on the Noma v2 guardrail #46606 merged to main, Noma v2 (live, see Proof). Custom code still ignores that setting on every route, so it can't keep masking here. Signed off by the ticket owner, @yucheng-berri, in the LIT-9515 design review on 2026-10-13, and accepted again on 2026-10-14 as a known risk after LiteReview flagged it, to match chatvalidationExceptionfor 4xx,internalServerExceptionfor 5xx) instead of HTTP 400/500 JSON, and an end-of-stream block arrives after the textinternalServerExceptioninstead of HTTP 500 JSONdisable_exception_on_blockon a held stream returns 200 with the guardrail message andstopReason: guardrail_intervenedinstead of HTTP 500streaming_end_of_stream_only: truekeeps it at about 12 sguardrail_informationholds one entry per scan round, as on chatcustom_code)x-amzn-requestid, instead of a body withcontent-lengthuse_v2: true) getsstream: trueon every scan and the finished answer as a Converseresponse(text, reasoning and tool-call blocks,stopReason) on the scan after the stream ends, as chat sends it. When the last sampled scan already covered every text, Noma gets one more scan at the end. A reply that only calls a tool gets one scan at the end (base sent none), so Noma can now block it, or fail it closed during a Noma outage, as on chat. Other guardrails get the request as beforeChanged on
/bedrock/.../converse-streamwith a held post_call guardrailChanged on every streaming route
not_runin the spend row instead of a verdict, on chat tooChanged on every Bedrock streaming passthrough
/v1/chat/completionsstreams use. Unguardedconverse-streamdisconnects get it tooUnchanged
apply_guardrail(for example Noma withoutuse_v2: true),mask_response_content, a held stream withstreaming_buffer_release_on_scan: true, post_call guardrails withrun_in_parallel: true, native-lifecycle guardrails, and post_call policy pipelines keep the whole stream buffered, as beforestreamorresponsemark, from this PR before and after the Noma change (e82e15f vs c694684, 15 live cells)invoke-with-response-stream, non-streaming/converse, chat/messages/responses streaming apart from the two items aboveCaveats (if any)
Medium
streaming_end_of_stream_only: trueavoids it/v1/messages). OTEL does get aguardrailspan per scan round on the relay, as on/v1/messages, where the buffered path emitted noneLow
guardContentblocks and tool definitions are not counted (chat's estimate leaves out tools too), so document-heavy or tool-heavy requests log a low estimate, still above base's 0messageStop(same order as/v1/messages), so a client that stops reading at the firstmessageStopmisses itGUARDRAIL_INTERVENEDverdict on a reply that only calls a tool is logged but changes nothing, because tool calls are not sent as texts (text plus tool replies behave the same)Final Attestation
Ran /live-pr-risk at 8ab2027 (real Bedrock, 4 workers, base ecbea93 vs head, plus real Noma v2 cells): no regressions. The backward-incompatible changes it found are exactly the ones listed in Behavior changes, and the base vs head differences are the same 30 cells as at 11bf918
REVIEWER MUST KNOW BEFORE APPROVING
streaming_buffer_until_moderated: true. Noma v2 honors it since fix(guardrails): honor streaming settings on the Noma v2 guardrail #46606 merged to main (a held Noma v2 guardrail masked a card in 4 of 4 live replies on this branch merged with main). Custom code still ignores it on every route, so custom code can't keep masking here/v1/chat/completions. Recommendstreaming_end_of_stream_only: trueto customers who only need a final verdict. Noma v2 honors it since fix(guardrails): honor streaming settings on the Noma v2 guardrail #46606 merged to main (one Noma scan per reply, live)not_runfor a scan cut by a disconnect on every route. The unified guardrail gainedneeds_ended_stream_scan(default False), and only the Bedrock Converse translation and Noma v2 opt in, so no other route or guardrail gets the extra end scanuse_v2: true(NomaV2, which scans throughapply_guardrail) streams. The v1 Noma guardrail runs its own hooks and still holds the whole stream, so the customer's POC needsuse_v2: trueNote
High Risk
Changes guardrail streaming semantics on a security-sensitive path with intentional breaking behavior (unmasked sampled relay), new billing/disconnect handling, and complex frame rewrite logic.
Overview
Bedrock
converse-streampassthrough no longer buffers the full AWS event stream forapply_guardrailpost_call guardrails. Eligible guardrails scan frame-by-frame (same sampling/hold semantics as chat), with guardrails that need the whole body still on the buffered path.The Bedrock translation decodes/re-encodes Converse event-stream frames, groups text per content block, applies rewrites when the stream ends (held mode), and emits in-stream
validationException/internalServerExceptionframes for mid-stream blocks/failures. The passthrough route dispatches viaLlmPassthroughRouteHandler; the proxy adds_relay_guarded_passthrough_event_streamandasync_post_call_event_stream_iterator_hook.Noma v2 sets
reads_stream_marksso scans getstream: trueand a finished Converseresponseon the last round; unified guardrail skips redundant end scans unlessneeds_ended_stream_scanapplies. Disconnect/cleanup: first-chunk buffering cancels upstream on caller cancel; held-stream early disconnect returns 499 and stops upstream; passthrough spend flush is awaitable. Logging: cut Converse streams estimate prompt tokens from the request body.Breaking (default sampled mode): frames reach clients before verdict and masks are not applied unless
streaming_buffer_until_moderated: true(chat parity).Reviewed by Cursor Bugbot for commit 8ab2027. Bugbot is set up for automated code reviews on this repo. Configure here.