From 55d393d77d34b58e802e00c9d645f9aa0a338300 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Wed, 29 Apr 2026 21:47:41 +0000 Subject: [PATCH] =?UTF-8?q?fix(static-assets):=20unblock=20CI=20=E2=80=94?= =?UTF-8?q?=20pass=20headers=20explicitly=20+=20harden=20+=20update=20lega?= =?UTF-8?q?cy=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../proxy/common_utils/static_asset_utils.py | 21 ++++++++++++------- tests/proxy_unit_tests/test_get_image.py | 5 ++++- tests/test_litellm/proxy/test_proxy_server.py | 15 ++++++++++++- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/litellm/proxy/common_utils/static_asset_utils.py b/litellm/proxy/common_utils/static_asset_utils.py index 0643572118b..ffead95dcfe 100644 --- a/litellm/proxy/common_utils/static_asset_utils.py +++ b/litellm/proxy/common_utils/static_asset_utils.py @@ -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.", diff --git a/tests/proxy_unit_tests/test_get_image.py b/tests/proxy_unit_tests/test_get_image.py index ad8c2672754..f14c6da5539 100644 --- a/tests/proxy_unit_tests/test_get_image.py +++ b/tests/proxy_unit_tests/test_get_image.py @@ -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" diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index da3f963f9de..1a77e39aa80 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -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()