chore(proxy): contain UI_LOGO_PATH / LITELLM_FAVICON_URL on unauthenticated asset endpoints - #26815
Merged
yuneng-berri merged 12 commits intoMay 1, 2026
Merged
chore(proxy): contain UI_LOGO_PATH / LITELLM_FAVICON_URL on unauthenticated asset endpoints#26815yuneng-berri merged 12 commits into
yuneng-berri merged 12 commits into
Conversation
…sset roots The unauthenticated ``/get_image`` and ``/get_favicon`` endpoints accept the admin-set env vars ``UI_LOGO_PATH`` and ``LITELLM_FAVICON_URL`` and return whatever bytes they resolve to, with a hard-coded ``image/jpeg`` or ``image/x-icon`` content-type. Two attack shapes: * ``UI_LOGO_PATH=/etc/passwd`` (or any other readable file path) — any unauthenticated caller exfiltrates the file via ``GET /get_image``. The previous gate was ``os.path.exists(logo_path)`` which fires on every readable file. Same shape for the favicon endpoint. * ``UI_LOGO_PATH=http://169.254.169.254/iam`` (or any internal HTTP service the admin pointed at) — the proxy fetches it server-side and streams the response body to the unauthenticated caller. No URL validation, no Content-Type validation; ``application/json`` AWS metadata gets tunneled out under the ``image/jpeg`` wrapper. New helper module ``litellm/proxy/common_utils/static_asset_utils.py``: * ``resolve_local_asset_path(candidate, allowed_roots)`` — returns the resolved absolute path only if it lives within one of the allowed asset roots. Uses ``realpath`` so symlinks pointing outside the roots are caught. * ``fetch_validated_image_bytes(url)`` — runs the URL through ``validate_url`` (rejecting private / cloud-metadata / loopback targets) and only returns the response body if the upstream Content-Type is in a small allowlist of image MIME types. Both ``/get_image`` and ``/get_favicon`` are wired through the helpers. The SSRF gate 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 shouldn't also expose ``/get_image`` to SSRF). Tests: - ``TestResolveLocalAssetPath``: 10 cases covering legitimate paths, ``/etc/passwd``, ``/proc/self/environ``, symlink-out, ``..`` traversal, directories, missing files, and root list edge cases. - ``TestFetchValidatedImageBytes``: 7 cases covering SSRF block, non- image content-type rejection, valid image passthrough, non-200 response, fetch exception, empty URL, and parametrized coverage of every allowed image MIME type. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The unauthenticated ``/get_logo_url`` endpoint returned the ``UI_LOGO_PATH`` env var verbatim. For HTTP(S) URLs this is intended — the dashboard loads the logo directly from a public/internal CDN. For local filesystem paths it was an information disclosure: any caller could fetch ``/get_logo_url`` and read admin-only filesystem details like ``UI_LOGO_PATH=/etc/litellm/secret-config.json``. Now the endpoint returns the URL only when it begins with ``http://`` or ``https://``. For local paths (or unset) it returns an empty string — the dashboard falls back to ``/get_image`` which serves the file via the path-containment guard added in the previous commit. Tests parametrize the disclosure-blocked cases (``/etc/...``, ``/proc/self/environ``, relative paths) and confirm HTTP / HTTPS URLs still pass through unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Align ``/get_favicon``'s allowed-root list with ``/get_image``'s. Both endpoints now accept paths under any of: * ``LITELLM_ASSETS_PATH`` (or its default — ``/var/lib/litellm/assets`` for non-root, the package dir otherwise) * the package's bundled-asset dir (``proxy/_experimental/out`` for the default favicon, ``proxy/`` for the default logo) * the proxy package dir (``current_dir``) as a final fallback Without this, an admin who put a custom favicon under ``LITELLM_ASSETS_PATH`` (e.g. mounted into the container at ``/var/lib/litellm/assets/favicon.ico``) would have the favicon endpoint silently fall back to the default after the previous commit's path-containment guard. The logo endpoint already accepted this root. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
Greptile SummaryThis PR hardens the three unauthenticated branding endpoints ( Confidence Score: 5/5Safe to merge — focused security hardening with no P1/P0 findings. All findings are P2 or lower. The SSRF fix (redirect instead of server-fetch), path-traversal guard (magic-byte validation), and info-disclosure fix (/get_logo_url filtering) are all correctly implemented and well covered by tests. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/common_utils/static_asset_utils.py | New helper module for magic-byte image validation and safe local path resolution; logic is sound with one loose GIF check (byte 4 not verified). |
| litellm/proxy/proxy_server.py | SSRF and path-traversal hardening for /get_image, /get_favicon, and /get_logo_url — remote URLs now redirect rather than being server-fetched; local paths validated by magic byte before serving. |
| tests/test_litellm/proxy/common_utils/test_static_asset_utils.py | New unit tests cover image signature acceptance, /etc/passwd rejection, symlink resolution, and path traversal — good coverage of the new helper. |
| tests/proxy_unit_tests/test_get_image.py | Tests correctly rewritten to assert redirect behavior (no server-side fetch) and stale-cache rejection; no weakening of existing coverage. |
| tests/test_litellm/proxy/test_proxy_server.py | New /get_logo_url tests verify local-path filtering and HTTP(S) pass-through; updated /get_image tests replace brittle os.path.exists mocks with real tmp_path fixtures. |
Reviews (2): Last reviewed commit: "fix(static-assets): stop serving stale l..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
Contributor
Hardening unauthenticated asset endpoints against LFI, SSRF, and path disclosureThis PR prevents three classes of issues on the unauthenticated Status: 0 open |
…pdate 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>
…on cache miss 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>
…eaner test fixture
Two cleanups from the /simplify review pass:
* ``Response`` was imported inside the ``except OSError`` branch in
``/get_image`` and at the top of ``/get_favicon``. Per the project's
no-inline-imports rule (CLAUDE.md), hoisted to the existing
``from fastapi.responses import (...)`` block at the top of
``proxy_server.py``.
* The test class's ``_patches()`` helper returned a 2-element list of
patch context managers and tests indexed into them via
``self._patches(...)[0], self._patches()[1]`` — two distinct calls
with confusing aliasing semantics. Restructured to:
- module-level ``_patch_async_safe_get(...)`` that returns a single
patch context manager
- autouse fixture that patches ``get_async_httpx_client`` for every
test in the file (it's the same patch in every case)
- small ``_image_response(...)`` factory to deduplicate Mock setup
Tests now read as ``with _patch_async_safe_get(return_value=...):``
with no list-indexing or duplicate Mock construction.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…async_safe_get
Variant analysis on the unauthenticated /get_image SSRF surfaced one
related sink in an admin-only endpoint:
``test_hashicorp_vault_connection`` in
``config_override_endpoints.py:402`` calls
``async_client.get(f"{vault_addr}/v1/auth/token/lookup-self")`` with
no SSRF guard. ``vault_addr`` is admin-set, so the threat model is
"admin misconfig (or attacker with admin creds) pivots Vault calls
to cloud metadata or another internal IP."
Same fix shape as the unauthenticated endpoints: wrap in
``async_safe_get`` so each redirect hop is re-validated and private
networks are rejected. Admins running against a legitimate internal
Vault should add the host to ``litellm.user_url_allowed_hosts`` —
the existing escape hatch already used elsewhere in the codebase.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
``Response`` is already imported from the top-level ``fastapi`` package via the multi-line ``from fastapi import (...)`` block at the top of the file (along with ``Depends``, ``HTTPException``, etc.) — ``fastapi.Response`` is the same class that ``fastapi.responses`` re-exports. The earlier ``from fastapi.responses import Response`` addition triggered ruff F811 for redefinition. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
yuneng-berri
approved these changes
May 1, 2026
yuneng-berri
merged commit May 1, 2026
15b7386
into
BerriAI:litellm_internal_staging
42 checks passed
fzowl
pushed a commit
to fzowl/litellm
that referenced
this pull request
Jun 24, 2026
chore(proxy): contain UI_LOGO_PATH / LITELLM_FAVICON_URL on unauthenticated asset endpoints
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relevant issues
Pre-Submission checklist
tests/test_litellm/directory.make test-unit.@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewType
🐛 Bug Fix
Changes
Hardens the unauthenticated logo / favicon endpoints without requiring operators to move branding assets or add new env vars.
/get_imageand/get_favicon— remote HTTP(S)UI_LOGO_PATH/LITELLM_FAVICON_URLvalues are browser-loaded via redirect instead of being fetched by the proxy. This preserves existing remote URL workflows while removing the server-side SSRF primitive from these unauthenticated endpoints./get_imageand/get_favicon— local file paths still work from their existing locations, but the resolved file must have a supported image signature (jpeg,png,gif,webp, orico) before it is served. Non-image files such as/etc/passwdor/proc/self/environfall back to the bundled default asset./get_logo_url— only returns HTTP(S) values, so local filesystem paths are not disclosed to unauthenticated callers.cached_logo.jpgfiles are no longer served by/get_image, so old pre-fix cache entries cannot keep exposing server-fetched bytes.Files
litellm/proxy/common_utils/static_asset_utils.py— local image signature detection and safe local image path resolution.litellm/proxy/proxy_server.py— redirects remote branding URLs, validates local image files, and validates stale cache entries before serving.tests/test_litellm/proxy/common_utils/test_static_asset_utils.py— covers supported image signatures and non-image local file rejection.tests/proxy_unit_tests/test_get_image.py,tests/proxy_unit_tests/test_get_favicon.py, andtests/test_litellm/proxy/test_proxy_server.py— covers remote redirect behavior, no proxy-side fetch, cache handling, local custom logo behavior, and/get_logo_urlfiltering.Behaviour notes for operators
LITELLM_ASSETS_PATH.