fix(guardrails): match the downstream remote-url predicate exactly

The remote-image refusal tested `startswith(("http://", "https://"))`
while BedrockImageProcessor.process_image_async decides with
`"http://" in image_url or "https://" in image_url`. A url carrying
leading whitespace failed the prefix test, then matched downstream and
was fetched -- the fail-closed policy turned into an uncapped
server-side download, which is exactly the remote-fetch work this PR
defers (async_safe_get still has no size cap on the base).

Aligned the two predicates and parametrized the rejection test over
leading space, tab and newline, asserting the decoder is never awaited.
Reverting the predicate fails the three whitespace cases.

Reported by Veria AI on #38175.
This commit is contained in:
feng.tsai 2026-09-08 10:35:07 +08:00
parent f084815cf1
commit 288f4aeafc
2 changed files with 33 additions and 11 deletions

View file

@ -606,8 +606,16 @@ class BedrockGuardrail(CustomGuardrail, BaseAWSLLM):
same bytes to the model), so the operator gets "not supported" instead of
the decoder's "could not be read". Anything else that is not a data URI is
an unrecognized payload and falls through to the decoder, which rejects it.
The test is a substring, not a prefix, deliberately: it has to reject
everything `BedrockImageProcessor.process_image_async` would treat as
remote, and that check is `"http://" in image_url or "https://" in
image_url`. A prefix test reads more naturally but leaves a hole -- a url
carrying leading whitespace fails it, then matches downstream and is
fetched, so the fail-closed policy here would be bypassed into an
uncapped server-side download. Keep the two predicates identical.
"""
if image_url.startswith(("http://", "https://")):
if "http://" in image_url or "https://" in image_url:
self._handle_unscannable_image(reason="remote image URLs are not supported")
try:

View file

@ -5589,24 +5589,38 @@ class TestBedrockGuardrailImageInput:
assert "could not be read" in str(exc_info.value.detail) or "not a png/jpeg" in str(exc_info.value.detail)
@pytest.mark.parametrize(
"url",
[
pytest.param("https://example.com/pic.png", id="plain https url"),
pytest.param("http://example.com/pic.png", id="plain http url"),
# The rejection used to be a prefix test, which these slip past while
# `process_image_async` still treats them as remote and fetches them --
# an uncapped server-side download straight through the fail-closed path.
pytest.param(" https://example.com/pic.png", id="leading space"),
pytest.param("\thttps://example.com/pic.png", id="leading tab"),
pytest.param("\nhttps://example.com/pic.png", id="leading newline"),
],
)
@pytest.mark.asyncio
async def test_a_remote_image_url_is_rejected_without_being_fetched(self):
async def test_a_remote_image_url_is_rejected_without_being_fetched(self, url):
"""A remote url is named as its own rejection rather than left to the decoder.
Fetching one safely (size cap, SSRF/redirect validation) is separate work; this
PR only scans inline images, so a url has to fail closed rather than be ignored
or silently forwarded to the model unscanned.
or silently forwarded to the model unscanned. The refusal has to catch every
shape the decoder would fetch, not just the well-formed ones.
"""
messages = [
{
"role": "user",
"content": [{"type": "image_url", "image_url": {"url": "https://example.com/pic.png"}}],
}
]
messages = [{"role": "user", "content": [{"type": "image_url", "image_url": {"url": url}}]}]
with pytest.raises(HTTPException) as exc_info:
await self._guardrail().convert_to_bedrock_format(source="INPUT", messages=messages)
with patch( # test-quality-ok: the fetch is the thing under assertion -- it must never be reached
"litellm.proxy.guardrails.guardrail_hooks.bedrock_guardrails.BedrockImageProcessor.process_image_async",
new_callable=AsyncMock,
) as decode:
with pytest.raises(HTTPException) as exc_info:
await self._guardrail().convert_to_bedrock_format(source="INPUT", messages=messages)
decode.assert_not_awaited()
assert "remote image URLs are not supported" in str(exc_info.value.detail)
def test_file_backed_part_counting_skips_non_mapping_entries(self):