scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-10-08 03:08:45 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 114

fix(static-assets): use async_safe_get; drop SVG; serve bytes inline on cache miss

Browse source 
Three review items addressed:

* **Veria (Medium): SSRF via redirect.** ``fetch_validated_image_bytes``
  was calling ``validate_url(url)`` once and then fetching with the
  default httpx client, so a 3xx to an internal IP would have been
  followed unvalidated. Switched to ``async_safe_get`` (the existing
  SSRF primitive used elsewhere in the codebase) which walks each
  redirect hop, re-validates, and rejects redirects to blocked
  networks. Default ``litellm.user_url_validation`` is True so
  protection is on out of the box.

* **Greptile (P2): SVG can embed JS.** Removed ``image/svg+xml`` from
  the allowed-Content-Type set. The hardcoded response media type
  (``image/jpeg`` / ``image/x-icon``) means a real SVG body wouldn't
  render as SVG anyway in modern browsers — the allowlist entry was
  giving up XSS surface for no actual SVG-rendering benefit. If real
  SVG support is wanted later, that's a deliberate feature PR with CSP
  / nosniff bundled.

* **Greptile (P2): cache-write OSError drops validated bytes.** When
  the upstream fetch succeeded but ``open(cache_path, "wb")`` raised
  (read-only assets dir), the bytes were discarded and the default
  logo was served — a silent regression for that deployment. Now
  serve the validated bytes inline via ``Response(...)`` as a fallback
  before falling back to default.

Tests:

- Replaced low-level mocks of ``validate_url`` with mocks of
  ``async_safe_get`` directly, exercising the helper's contract
  rather than the SSRF primitive's internals.
- New ``test_rejects_svg_content_type`` confirms SVG is blocked.
- ``test_get_image_cache_logic`` fixture now sets
  ``mock_response.is_redirect = False`` so ``async_safe_get`` doesn't
  treat the Mock's truthy attribute as a redirect.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
user 2026-04-29 21:57:22 +00:00
parent 55d393d77d
commit 75d1a0116e
No known key found for this signature in database
4 changed files with 101 additions and 126 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
litellm/proxy/common_utils/static_asset_utils.py
View file

@ -18,7 +18,7 @@ import os
from typing import List, Optional
from litellm._logging import verbose_proxy_logger
from litellm.litellm_core_utils.url_utils import SSRFError, validate_url
from litellm.litellm_core_utils.url_utils import SSRFError, async_safe_get
from litellm.llms.custom_httpx.http_handler import get_async_httpx_client
from litellm.types.llms.custom_http import httpxSpecialProvider
@ -26,13 +26,19 @@ from litellm.types.llms.custom_http import httpxSpecialProvider
# without this, an admin-configured URL whose upstream returns
# ``application/json`` (e.g. cloud metadata, internal API) would still be
# served back to the caller verbatim.
#
# ``image/svg+xml`` is intentionally NOT in this list: SVG is the only
# common image format that can embed JavaScript, and the endpoint is
# unauthenticated. An admin-configured CDN serving a crafted SVG would
# otherwise reach unauthenticated callers; removing SVG closes the
# residual XSS surface even though the response is served with a
# hardcoded ``image/jpeg`` / ``image/x-icon`` media type.
ALLOWED_IMAGE_CONTENT_TYPES = frozenset(
{
"image/jpeg",
"image/jpg",
"image/png",
"image/gif",
"image/svg+xml",
"image/webp",
"image/x-icon",
"image/vnd.microsoft.icon",
@ -76,19 +82,27 @@ async def fetch_validated_image_bytes(
url: str, *, timeout_s: float = 5.0
) -> Optional[bytes]:
"""
Fetch ``url`` with SSRF protection (always-on) and Content-Type
validation. Returns the raw bytes on success, ``None`` on any
failure (blocked target, non-200, or non-image response).
Fetch ``url`` with SSRF protection and Content-Type validation.
Returns the raw bytes on success, ``None`` on any failure (blocked
target, redirect to a blocked target, non-200, or non-image
response).
The SSRF guard is enforced unconditionally — these endpoints are
unauthenticated, so the admin-facing ``litellm.user_url_validation``
toggle does not apply. An admin who opted out of URL validation for
LLM provider paths should not also expose ``/get_image`` to SSRF.
Delegates to ``async_safe_get`` so each redirect hop is re-validated
against ``BLOCKED_NETWORKS`` (a 3xx to ``169.254.169.254`` is
rejected, not followed). Honours ``litellm.user_url_validation``
like every other SSRF-aware fetch in the codebase; the toggle
defaults to True, and an admin who has explicitly disabled URL
validation has opted out of SSRF protection globally.
"""
if not url:
return None
async_client = get_async_httpx_client(
llm_provider=httpxSpecialProvider.UI,
params={"timeout": timeout_s},
)
try:
rewritten_url, host_header = validate_url(url)
response = await async_safe_get(async_client, url)
except SSRFError as exc:
verbose_proxy_logger.warning(
"Blocked unauthenticated asset fetch — SSRF guard rejected %r: %s",
@ -96,22 +110,6 @@ async def fetch_validated_image_bytes(
exc,
)
return None
# ``validate_url`` rewrites HTTP URLs to point at a validated IP and
# returns the original hostname for the Host header. For HTTPS with
# ssl_verify enabled, it returns the URL unchanged (TLS hostname
# validation handles DNS rebinding).
async_client = get_async_httpx_client(
llm_provider=httpxSpecialProvider.UI,
params={"timeout": timeout_s},
)
try:
if rewritten_url != url:
response = await async_client.get(
rewritten_url, headers={"host": host_header}
)
else:
response = await async_client.get(rewritten_url)
except Exception as exc:
verbose_proxy_logger.debug("Asset fetch failed for %r: %s", url, exc)
return None

25
litellm/proxy/proxy_server.py
View file

@ -12312,16 +12312,21 @@ async def get_image():
# SSRF + content-type validation — the helper rejects
# private/internal/cloud-metadata targets and non-image responses.
image_bytes = await fetch_validated_image_bytes(logo_path)
if image_bytes is not None:
try:
with open(cache_path, "wb") as f:
f.write(image_bytes)
return FileResponse(cache_path, media_type="image/jpeg")
except OSError as e:
verbose_proxy_logger.debug(
"Could not write logo cache to %s: %s", cache_path, e
)
return FileResponse(default_logo, media_type="image/jpeg")
if image_bytes is None:
return FileResponse(default_logo, media_type="image/jpeg")
try:
with open(cache_path, "wb") as f:
f.write(image_bytes)
return FileResponse(cache_path, media_type="image/jpeg")
except OSError as e:
# Read-only assets dir: serve the validated bytes inline
# rather than dropping them and returning the default logo.
from fastapi.responses import Response
verbose_proxy_logger.debug(
"Could not write logo cache to %s: %s — serving inline", cache_path, e
)
return Response(content=image_bytes, media_type="image/jpeg")
else:
# Default logo (resolved from the bundled asset, not user-controlled).
return FileResponse(logo_path, media_type="image/jpeg")

6
tests/proxy_unit_tests/test_get_image.py
View file

@ -65,12 +65,14 @@ async def test_get_image_cache_logic():
os.remove(cache_path)
# Mock response — set headers explicitly so the Content-Type
# validation added for GHSA-pjc9-2hw6-78rr accepts the response
# as a legitimate image.
# validation accepts the response as a legitimate image, and set
# ``is_redirect=False`` so ``async_safe_get`` doesn't try to walk
# a redirect chain.
mock_response = mock.Mock()
mock_response.status_code = 200
mock_response.content = b"fake image data"
mock_response.headers = {"content-type": "image/jpeg"}
mock_response.is_redirect = False
with mock.patch(
"litellm.llms.custom_httpx.http_handler.AsyncHTTPHandler.get"

146
tests/test_litellm/proxy/common_utils/test_static_asset_utils.py
View file

@ -101,121 +101,89 @@ class TestResolveLocalAssetPath:
class TestFetchValidatedImageBytes:
@pytest.fixture
def mock_async_client(self):
client = MagicMock()
client.get = AsyncMock()
return client
"""
The helper now delegates to ``async_safe_get`` for the SSRF guard +
redirect handling. Tests mock ``async_safe_get`` directly so they
exercise the helper's contract (Content-Type validation, status code
handling, exception fallthrough) without depending on the SSRF
primitive's internals.
"""
@pytest.mark.asyncio
async def test_blocks_private_ip_via_validate_url(self, mock_async_client):
# The SSRF half of GHSA-pjc9-2hw6-78rr — admin sets logo URL to
# http://169.254.169.254/iam, attacker hits /get_image, exfils creds.
with (
@staticmethod
def _patches(*, async_safe_get_return=None, async_safe_get_side_effect=None):
return [
patch(
"litellm.proxy.common_utils.static_asset_utils.validate_url",
side_effect=SSRFError("blocked: 169.254.169.254"),
"litellm.proxy.common_utils.static_asset_utils.async_safe_get",
new_callable=AsyncMock,
return_value=async_safe_get_return,
side_effect=async_safe_get_side_effect,
),
patch(
"litellm.proxy.common_utils.static_asset_utils.get_async_httpx_client",
return_value=mock_async_client,
return_value=MagicMock(),
),
):
result = await fetch_validated_image_bytes("http://169.254.169.254/iam")
assert result is None
# The fetch must not be attempted when the URL is rejected.
mock_async_client.get.assert_not_called()
]
@pytest.mark.asyncio
async def test_rejects_non_image_content_type(self, mock_async_client):
async def test_blocks_ssrf_target(self):
# The SSRF half of GHSA-pjc9-2hw6-78rr — admin sets logo URL to
# http://169.254.169.254/iam, attacker hits /get_image, exfils creds.
# ``async_safe_get`` raises SSRFError on private/metadata targets
# and on redirect hops to those targets (covers the redirect
# bypass Veria flagged on the previous iteration).
with (
self._patches(
async_safe_get_side_effect=SSRFError("blocked: 169.254.169.254")
)[0],
self._patches()[1],
):
result = await fetch_validated_image_bytes("http://169.254.169.254/iam")
assert result is None
@pytest.mark.asyncio
async def test_rejects_non_image_content_type(self):
# Even when the URL passes SSRF, the upstream response must be an
# image. Otherwise an attacker could redirect to an upstream that
# image. Otherwise an attacker could point at an upstream that
# returns ``application/json`` AWS creds and have them tunneled
# through the ``image/jpeg`` response wrapper.
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.headers = {"content-type": "application/json"}
mock_response.content = b'{"AccessKeyId": "..."}'
mock_async_client.get.return_value = mock_response
with (
patch(
"litellm.proxy.common_utils.static_asset_utils.validate_url",
return_value=("http://cdn.example/logo", "cdn.example"),
),
patch(
"litellm.proxy.common_utils.static_asset_utils.get_async_httpx_client",
return_value=mock_async_client,
),
):
with self._patches(async_safe_get_return=mock_response)[0], self._patches()[1]:
result = await fetch_validated_image_bytes("http://cdn.example/logo")
assert result is None
@pytest.mark.asyncio
async def test_returns_bytes_for_valid_image_response(self, mock_async_client):
async def test_returns_bytes_for_valid_image_response(self):
png_bytes = b"\x89PNG\r\n\x1a\nfake png body"
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.headers = {"content-type": "image/png; charset=binary"}
mock_response.content = png_bytes
mock_async_client.get.return_value = mock_response
with (
patch(
"litellm.proxy.common_utils.static_asset_utils.validate_url",
return_value=(
"https://cdn.example/logo.png",
"cdn.example",
),
),
patch(
"litellm.proxy.common_utils.static_asset_utils.get_async_httpx_client",
return_value=mock_async_client,
),
):
with self._patches(async_safe_get_return=mock_response)[0], self._patches()[1]:
result = await fetch_validated_image_bytes("https://cdn.example/logo.png")
assert result == png_bytes
@pytest.mark.asyncio
async def test_returns_none_on_non_200_response(self, mock_async_client):
async def test_returns_none_on_non_200_response(self):
mock_response = MagicMock()
mock_response.status_code = 404
mock_response.headers = {"content-type": "image/png"}
mock_async_client.get.return_value = mock_response
with (
patch(
"litellm.proxy.common_utils.static_asset_utils.validate_url",
return_value=("https://cdn.example/logo", "cdn.example"),
),
patch(
"litellm.proxy.common_utils.static_asset_utils.get_async_httpx_client",
return_value=mock_async_client,
),
):
with self._patches(async_safe_get_return=mock_response)[0], self._patches()[1]:
result = await fetch_validated_image_bytes("https://cdn.example/logo")
assert result is None
@pytest.mark.asyncio
async def test_returns_none_on_fetch_exception(self, mock_async_client):
mock_async_client.get.side_effect = Exception("connection reset")
async def test_returns_none_on_fetch_exception(self):
with (
patch(
"litellm.proxy.common_utils.static_asset_utils.validate_url",
return_value=("https://cdn.example/logo", "cdn.example"),
),
patch(
"litellm.proxy.common_utils.static_asset_utils.get_async_httpx_client",
return_value=mock_async_client,
),
self._patches(async_safe_get_side_effect=Exception("connection reset"))[0],
self._patches()[1],
):
result = await fetch_validated_image_bytes("https://cdn.example/logo")
assert result is None
@pytest.mark.asyncio
@ -223,30 +191,32 @@ class TestFetchValidatedImageBytes:
result = await fetch_validated_image_bytes("")
assert result is None
@pytest.mark.asyncio
async def test_rejects_svg_content_type(self):
# ``image/svg+xml`` is intentionally NOT in the allowlist for
# unauthenticated endpoints — SVG is the only common image
# format that can embed JavaScript.
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.headers = {"content-type": "image/svg+xml"}
mock_response.content = b"<svg><script>alert(1)</script></svg>"
with self._patches(async_safe_get_return=mock_response)[0], self._patches()[1]:
result = await fetch_validated_image_bytes("https://cdn.example/x.svg")
assert result is None
@pytest.mark.parametrize(
"content_type",
sorted(ALLOWED_IMAGE_CONTENT_TYPES),
)
@pytest.mark.asyncio
async def test_accepts_each_allowed_image_content_type(
self, mock_async_client, content_type
):
async def test_accepts_each_allowed_image_content_type(self, content_type):
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.headers = {"content-type": content_type}
mock_response.content = b"image-bytes"
mock_async_client.get.return_value = mock_response
with (
patch(
"litellm.proxy.common_utils.static_asset_utils.validate_url",
return_value=("https://cdn.example/logo", "cdn.example"),
),
patch(
"litellm.proxy.common_utils.static_asset_utils.get_async_httpx_client",
return_value=mock_async_client,
),
):
with self._patches(async_safe_get_return=mock_response)[0], self._patches()[1]:
result = await fetch_validated_image_bytes("https://cdn.example/logo")
assert result == b"image-bytes"