mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
fix(static-assets): unblock CI — pass headers explicitly + harden + update legacy tests
Three CI failures from the previous push, all addressed:
* ``lint`` (mypy): ``async_client.get(url, **request_kwargs)`` confused
mypy because ``AsyncHTTPHandler.get``'s second positional arg is typed
``bool | None``. Switched to an explicit branch:
``await async_client.get(rewritten_url, headers={"host": host_header})``
for the HTTP-rewritten case, plain ``get(rewritten_url)`` otherwise.
* ``proxy-infra`` /
``test_get_image_custom_local_logo_bypasses_cache``: the existing
test set ``UI_LOGO_PATH=/app/custom_logo.jpg`` with no
``LITELLM_ASSETS_PATH``, asserting the path was served verbatim. That
was the LFI behaviour the new path-containment guard closes. Updated
the test to set ``LITELLM_ASSETS_PATH=/app`` so the path is inside an
allowed root, and patched the helper's ``realpath`` / ``isfile`` to
go along with the mocked filesystem. Test intent (bypass cache when
``UI_LOGO_PATH`` is local) is preserved.
* ``auth-and-jwt`` / ``test_get_image_cache_logic``: existing test
built a ``Mock`` response without ``headers``, so the new
Content-Type check tripped on ``Mock().split(";")[0]``. Two fixes:
1. Set ``mock_response.headers = {"content-type": "image/jpeg"}``
on the test (matches the real upstream contract — a logo CDN
always sets a Content-Type).
2. Make ``fetch_validated_image_bytes`` defensive: if the
Content-Type header is missing or non-string, treat as non-image
and fall back to default. Closes a subtle hole — pre-fix, an
upstream that omits Content-Type entirely would have served
arbitrary bytes under the ``image/jpeg`` wrapper.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
4f751fdcc6
commit
55d393d77d
3 changed files with 32 additions and 9 deletions
|
|
@ -101,16 +101,17 @@ async def fetch_validated_image_bytes(
|
|||
# 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).
|
||||
request_kwargs = {}
|
||||
if rewritten_url != url:
|
||||
request_kwargs["headers"] = {"host": host_header}
|
||||
|
||||
async_client = get_async_httpx_client(
|
||||
llm_provider=httpxSpecialProvider.UI,
|
||||
params={"timeout": timeout_s},
|
||||
)
|
||||
try:
|
||||
response = await async_client.get(rewritten_url, **request_kwargs)
|
||||
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
|
||||
|
|
@ -118,9 +119,15 @@ async def fetch_validated_image_bytes(
|
|||
if response.status_code != 200:
|
||||
return None
|
||||
|
||||
content_type = (
|
||||
(response.headers.get("content-type") or "").split(";")[0].strip().lower()
|
||||
raw_content_type = (
|
||||
response.headers.get("content-type") if hasattr(response, "headers") else None
|
||||
)
|
||||
if not isinstance(raw_content_type, str):
|
||||
# Defensive: if upstream omits Content-Type entirely, treat as
|
||||
# non-image. (Also keeps ``Mock`` responses without a configured
|
||||
# ``headers`` from blowing up the content-type check.)
|
||||
return None
|
||||
content_type = raw_content_type.split(";")[0].strip().lower()
|
||||
if content_type not in ALLOWED_IMAGE_CONTENT_TYPES:
|
||||
verbose_proxy_logger.warning(
|
||||
"Asset fetch from %r returned non-image content-type %r — refusing to serve.",
|
||||
|
|
|
|||
|
|
@ -64,10 +64,13 @@ async def test_get_image_cache_logic():
|
|||
if os.path.exists(cache_path):
|
||||
os.remove(cache_path)
|
||||
|
||||
# Mock response
|
||||
# Mock response — set headers explicitly so the Content-Type
|
||||
# validation added for GHSA-pjc9-2hw6-78rr accepts the response
|
||||
# as a legitimate image.
|
||||
mock_response = mock.Mock()
|
||||
mock_response.status_code = 200
|
||||
mock_response.content = b"fake image data"
|
||||
mock_response.headers = {"content-type": "image/jpeg"}
|
||||
|
||||
with mock.patch(
|
||||
"litellm.llms.custom_httpx.http_handler.AsyncHTTPHandler.get"
|
||||
|
|
|
|||
|
|
@ -4047,9 +4047,12 @@ async def test_get_image_custom_local_logo_bypasses_cache(monkeypatch):
|
|||
|
||||
from litellm.proxy.proxy_server import get_image
|
||||
|
||||
# Use a path inside the allowlisted ``LITELLM_ASSETS_PATH`` — the
|
||||
# path-containment guard added for GHSA-3pcp-536p-ghjc rejects any
|
||||
# local UI_LOGO_PATH outside the allowed asset roots.
|
||||
monkeypatch.setenv("LITELLM_ASSETS_PATH", "/app")
|
||||
monkeypatch.setenv("UI_LOGO_PATH", "/app/custom_logo.jpg")
|
||||
monkeypatch.delenv("LITELLM_NON_ROOT", raising=False)
|
||||
monkeypatch.delenv("LITELLM_ASSETS_PATH", raising=False)
|
||||
|
||||
calls_to_file_response = []
|
||||
|
||||
|
|
@ -4063,6 +4066,16 @@ async def test_get_image_custom_local_logo_bypasses_cache(monkeypatch):
|
|||
patch(
|
||||
"litellm.proxy.proxy_server.FileResponse", side_effect=fake_file_response
|
||||
),
|
||||
# The path-containment helper calls ``os.path.realpath`` and
|
||||
# ``os.path.isfile`` — make them play along for the test path.
|
||||
patch(
|
||||
"litellm.proxy.common_utils.static_asset_utils.os.path.realpath",
|
||||
side_effect=lambda p: p,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.common_utils.static_asset_utils.os.path.isfile",
|
||||
return_value=True,
|
||||
),
|
||||
):
|
||||
await get_image()
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue