From 43687653744f3fd112e9229f96360a43765a063a Mon Sep 17 00:00:00 2001 From: samtsai15 <6171228+samtsai15@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:40:39 +0800 Subject: [PATCH] fix(guardrails): refuse a file-backed image instead of documenting that it passes An Anthropic `{"type": "image", "source": {"type": "file", "file_id": ...}}` block carries no bytes, so the extractor yields nothing for it while the provider still forwards the file to the model. The docstring called that a known gap. Documented is not the same as safe, and a silent pass is the exact failure this whole path exists to remove, so it is now refused: blocked by default, allowed only where the operator sets on_unscannable_image. Resolving the file id would need a Files API client this guardrail does not have, which is a larger change than the one this PR is making. The check reads inputs["structured_messages"], which on /v1/messages carries the raw Anthropic blocks, so the file source is visible to the guardrail without altering GenericGuardrailAPIInputs. Putting a marker in inputs["images"] was the alternative and would have reached five other guardrails that consume that field, trading one gap for four new unknowns. It detects the file shape specifically rather than comparing an image count against inputs["images"]. structured_messages is already narrowed by the skip and scope flags, so a mismatch is not by itself evidence of a dropped image, and refusing a legitimate request would be worse than the gap being closed. A test pins that base64 and url sources are still accepted. Placed before the incremental and latest-message-only shortcuts, so a file-backed image cannot be skipped by them either. The first draft of the refusal test passed for the wrong reason: without the check the request died on absent AWS credentials rather than on the bypass. The Bedrock call is now stubbed, so removing the check makes it fail with DID NOT RAISE -- the silent pass itself. Co-Authored-By: Claude Opus 5 (1M context) --- .../guardrail_hooks/bedrock_guardrails.py | 39 ++++++ .../test_bedrock_guardrails.py | 129 ++++++++++++++++++ 2 files changed, 168 insertions(+) diff --git a/litellm/proxy/guardrails/guardrail_hooks/bedrock_guardrails.py b/litellm/proxy/guardrails/guardrail_hooks/bedrock_guardrails.py index 2e9971697cd..5b7ee1196ce 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/bedrock_guardrails.py +++ b/litellm/proxy/guardrails/guardrail_hooks/bedrock_guardrails.py @@ -539,6 +539,38 @@ class BedrockGuardrail(CustomGuardrail, BaseAWSLLM): # on_unscannable_image policy decides rather than this helper. return value + @staticmethod + def _file_backed_image_count(structured_messages: object) -> int: + """Count image parts whose bytes live behind a provider Files API. + + An Anthropic `{"type": "image", "source": {"type": "file", "file_id": ...}}` + block carries no data, so the guardrail translation yields nothing for it + while the provider still forwards the file to the model. Left alone that is + an image the policy never sees, which is the failure this whole path exists + to remove -- so it is counted here and refused rather than documented. + + Detects that one shape rather than comparing counts against + `inputs["images"]`: structured_messages is already narrowed by the + skip/scope flags, so a mismatch is not by itself evidence of a dropped + image, and blocking a legitimate request is worse than the gap. + """ + if not isinstance(structured_messages, list): + return 0 + found = 0 # rebind-ok: running count over the message list + for message in structured_messages: + if not isinstance(message, dict): + continue + content = message.get("content") + if not isinstance(content, list): + continue + for part in content: + if not isinstance(part, dict) or part.get("type") != "image": + continue + source = part.get("source") + if isinstance(source, dict) and source.get("type") == "file": + found += 1 + return found + async def _build_image_content_item( self, image_url: str, budget: "_ImageFetchBudget | None" = None ) -> BedrockContentItem | None: @@ -3404,6 +3436,13 @@ class BedrockGuardrail(CustomGuardrail, BaseAWSLLM): # from `image`/`source` blocks. Five other guardrails already consume this # field; Bedrock was the one that dropped it on the floor. image_urls: Final = tuple(inputs.get("images") or ()) if input_type == "request" else () + if input_type == "request": + file_images: Final = self._file_backed_image_count(inputs.get("structured_messages")) + if file_images: + # Before the shortcuts below, so this cannot be skipped either. + self._handle_unscannable_image( + reason=f"{file_images} image(s) reference a provider file id, whose bytes are not available here" + ) try: verbose_proxy_logger.debug( "Bedrock Guardrail: Applying guardrail to %s text(s) and %s image(s)", len(texts), len(image_urls) diff --git a/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_bedrock_guardrails.py b/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_bedrock_guardrails.py index 56be71885da..b29103551cd 100644 --- a/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_bedrock_guardrails.py +++ b/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_bedrock_guardrails.py @@ -5746,6 +5746,135 @@ class TestBedrockGuardrailImageInput: assert _retained_image_bytes({"image": {"format": "png", "source": {"bytes": 123}}}) == 0 assert _retained_image_bytes({"image": {"format": "png", "source": {"bytes": "AAAA"}}}) == 3 + @pytest.mark.asyncio + async def test_a_file_backed_image_is_refused_rather_than_ignored(self): + """`{"type": "file"}` carries no bytes, so nothing reaches inputs["images"]. + + The provider still forwards the file to the model, so ignoring it is exactly + the silent pass this path exists to remove. Documented is not the same as + safe; under the default policy the request is refused. + """ + inputs = { + "texts": ["what does this say?"], + "images": [], + "structured_messages": [ + { + "role": "user", + "content": [ + {"type": "text", "text": "what does this say?"}, + {"type": "image", "source": {"type": "file", "file_id": "file_abc"}}, + ], + } + ], + } + + g = self._guardrail() + sent: list = [] + + async def spy(**kwargs): + sent.append(kwargs["messages"]) + return {"action": "NONE", "outputs": []} + + # Stubbed so that without the refusal this request would simply succeed: + # the failure mode being pinned is a silent pass, not an AWS error. + with patch.object(g, "make_bedrock_api_request", new=spy): + with pytest.raises(HTTPException) as exc_info: + await g.apply_guardrail(inputs=inputs, request_data={}, input_type="request") + + assert "file id" in str(exc_info.value.detail) + assert sent == [], "refused before any scan was attempted" + + @pytest.mark.asyncio + async def test_a_file_backed_image_is_let_through_under_the_allow_policy(self): + """An operator who would rather serve it unscanned can still say so.""" + g = self._guardrail(on_unscannable_image="allow") + sent: list = [] + + async def spy(**kwargs): + sent.append(kwargs["messages"]) + return {"action": "NONE", "outputs": []} + + with patch.object(g, "make_bedrock_api_request", new=spy): + await g.apply_guardrail( + inputs={ + "texts": ["hello"], + "images": [], + "structured_messages": [ + { + "role": "user", + "content": [{"type": "image", "source": {"type": "file", "file_id": "file_abc"}}], + } + ], + }, + request_data={}, + input_type="request", + ) + + assert sent, "the text alongside the file image still has to be scanned" + + @pytest.mark.asyncio + async def test_the_scannable_source_shapes_are_not_refused(self): + """The refusal has to be specific to the shape that cannot be read. + + structured_messages is already narrowed by the skip and scope flags, so a + count mismatch against inputs["images"] is not evidence of a dropped image. + Blocking a legitimate request would be worse than the gap being closed. + """ + g = self._guardrail() + sent: list = [] + + async def spy(**kwargs): + sent.append(await g.convert_to_bedrock_format(source="INPUT", messages=kwargs["messages"])) + return {"action": "NONE", "outputs": []} + + with patch.object(g, "make_bedrock_api_request", new=spy): + await g.apply_guardrail( + inputs={ + "texts": ["hello"], + "images": [self._PNG_DATA_URI], + "structured_messages": [ + { + "role": "user", + "content": [ + {"type": "text", "text": "hello"}, + {"type": "image", "source": {"type": "base64", "data": "AAAA"}}, + {"type": "image", "source": {"type": "url", "url": "https://example.com/a.png"}}, + ], + } + ], + }, + request_data={}, + input_type="request", + ) + + kinds = [k for item in sent[0]["content"] for k in item] + assert "image" in kinds + + @pytest.mark.asyncio + async def test_a_file_backed_image_on_the_response_side_is_not_refused(self): + """Images are a request-side concern; an OUTPUT scan takes generated text.""" + g = self._guardrail() + + async def spy(**kwargs): + return {"action": "NONE", "outputs": []} + + with patch.object(g, "make_bedrock_api_request", new=spy): + result = await g.apply_guardrail( + inputs={ + "texts": ["the model said this"], + "structured_messages": [ + { + "role": "user", + "content": [{"type": "image", "source": {"type": "file", "file_id": "file_abc"}}], + } + ], + }, + request_data={}, + input_type="response", + ) + + assert result is not None + @pytest.mark.asyncio async def test_apply_guardrail_scans_images_from_inputs(self): """The proxy reaches BedrockGuardrail through `apply_guardrail`, not the native hook.