From 646f3404a556806198e1ccdede5890836a89b185 Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Wed, 2 Sep 2026 14:27:46 -0700 Subject: [PATCH] fix(security): restrict and validate file uploads at /v1/files and /upload/logo (#39379) * fix(security): restrict and validate file uploads at /v1/files and /upload/logo Extends fast-fail upload validation to every purpose at POST /v1/files, not just purpose=batch: a configurable max_file_size_mb size cap and a blocked_file_extensions denylist, plus rejection of filenames carrying a directory-traversal component before anything is read, stored, or forwarded to a provider. Also fixes two concrete gaps found while auditing every upload surface: the Azure Blob Storage backend derived a blob path's extension with filename.split(".")[-1], which does not parse path structure and let a crafted filename embed a directory traversal sequence into the stored blob path; and POST /upload/logo (the admin UI logo upload) had no role check at all, so any authenticated API key, not just a proxy admin, could write a file to the server's disk. * fix(lint): drop cast()/mutation from settings coercion, sync blocked_file_extensions on reload Replaces the TypeAdapter+cast() reads of max_file_size_mb and blocked_file_extensions with small isinstance-based validators, since the codebase's cast() budget (LIT006) had no headroom left. Also adds the blocked_file_extensions reload block that was missing from _update_general_settings: it was registered as an editable setting but never re-synced into runtime state, so a value set through the DB-backed settings editor would silently never take effect (Greptile finding). * fix(security): declare max_file_size_mb and blocked_file_extensions on ConfigGeneralSettings The DB-backed general-settings update endpoints validate every field through ConfigGeneralSettings.model_fields before persisting it, so without these declarations an operator could never actually set either setting through that path even though both were registered for the Admin UI's settings editor and reloaded on config refresh (Greptile finding). blocked_file_extensions is typed as a tuple, not a list, to stay out of the immutable-collections lint budget; the stored JSON value is unaffected since the raw request payload, not the validated model, is what gets persisted. * chore: regenerate schema.d.ts for the new ConfigGeneralSettings fields * fix(security): normalize configured blocked_file_extensions casing check_blocked_extension lowercased the uploaded filename's extension before comparing but compared it against blocked_extensions verbatim, so an admin-configured blocked_file_extensions: ['.EXE'] would never match an uploaded payload.exe (Greptile finding). Normalizes the configured values the same way at comparison time, and adds the missing case (mismatched-case config, lowercase upload) as a regression test, mutation-checked against the unfixed comparison. * fix(security): restore caller-owned stream position after size inspection _file_size_bytes unconditionally seeked back to 0 after measuring a BinaryIO's length, discarding wherever the caller had actually positioned it (Greptile finding). Saves and restores the original position instead. Rewrites the existing test that had encoded the old "always resets to 0" behavior as its expectation, and adds a sibling case for the under-cap path; both are mutation-checked against the unfixed always-reset-to-0 behavior. --- .../files/azure_blob_storage_backend.py | 27 +++- litellm/proxy/_types.py | 8 + .../openai_files_endpoints/files_endpoints.py | 30 +++- .../general_upload_validation.py | 150 ++++++++++++++++++ litellm/proxy/proxy_server.py | 8 + .../proxy_setting_endpoints.py | 19 ++- .../files/test_azure_blob_storage_backend.py | 44 +++++ .../test_files_endpoint.py | 111 +++++++++++++ .../test_general_upload_validation.py | 134 ++++++++++++++++ .../test_proxy_setting_endpoints.py | 50 ++++++ ui/litellm-dashboard/src/lib/http/schema.d.ts | 10 ++ 11 files changed, 582 insertions(+), 9 deletions(-) create mode 100644 litellm/proxy/openai_files_endpoints/general_upload_validation.py create mode 100644 tests/test_litellm/proxy/openai_files_endpoint/test_general_upload_validation.py diff --git a/litellm/llms/base_llm/files/azure_blob_storage_backend.py b/litellm/llms/base_llm/files/azure_blob_storage_backend.py index e22cf528856..a192226110c 100644 --- a/litellm/llms/base_llm/files/azure_blob_storage_backend.py +++ b/litellm/llms/base_llm/files/azure_blob_storage_backend.py @@ -7,16 +7,36 @@ to reuse all authentication and Azure Storage operations. """ import time +from pathlib import Path from typing import Final from urllib.parse import quote, urlparse from litellm._logging import verbose_logger from litellm._uuid import uuid from litellm.integrations.azure_storage.azure_storage import AzureBlobStorageLogger +from litellm.proxy.common_utils.path_utils import safe_filename from .storage_backend import BaseFileStorageBackend +def _safe_basename(original_filename: str) -> str: + try: + return safe_filename(original_filename) + except ValueError: + return "file" + + +def _safe_extension(original_filename: str) -> str: + """The extension off a basename, with no path separators or traversal sequences. + + original_filename.split(".")[-1] does not parse path structure, so a filename + like "a.jsonl/../../etc/cron.d/x" would put "../../etc/cron.d/x" straight into + the blob path built below. Path.suffix only ever looks at the last path + component, so routing through safe_filename() first closes that off. + """ + return Path(_safe_basename(original_filename)).suffix.lstrip(".") + + class AzureBlobStorageBackend(BaseFileStorageBackend, AzureBlobStorageLogger): """ Azure Blob Storage backend implementation. @@ -81,16 +101,15 @@ class AzureBlobStorageBackend(BaseFileStorageBackend, AzureBlobStorageLogger): def _generate_file_name(self, original_filename: str, file_naming_strategy: str) -> str: """Generate file name based on naming strategy.""" if file_naming_strategy == "original_filename": - # Use original filename, but sanitize it - return quote(original_filename, safe="") + return quote(_safe_basename(original_filename), safe="") elif file_naming_strategy == "timestamp": # Use timestamp - extension = original_filename.split(".")[-1] if "." in original_filename else "" + extension = _safe_extension(original_filename) timestamp: Final = int(time.time() * 1000) # milliseconds return f"{timestamp}.{extension}" if extension else str(timestamp) else: # default to "uuid" # Use UUID - extension = original_filename.split(".")[-1] if "." in original_filename else "" + extension = _safe_extension(original_filename) file_uuid: Final = str(uuid.uuid4()) return f"{file_uuid}.{extension}" if extension else file_uuid diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 18714256a8f..3c67297b5c7 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -2508,6 +2508,14 @@ class ConfigGeneralSettings(LiteLLMPydanticObjectBase): None, description="max batch input file size in MB for /v1/files uploads with purpose=batch, if a file is larger than this size it will be rejected before being forwarded to the provider", ) + max_file_size_mb: int | None = Field( + None, + description="max file size in MB for /v1/files uploads, for any purpose, if a file is larger than this size it will be rejected before being forwarded to the provider", + ) + blocked_file_extensions: tuple[str, ...] | None = Field( + None, + description="file extensions (e.g. ['.exe', '.sh']) rejected on /v1/files uploads, for any purpose, matched case-insensitively against the uploaded filename", + ) max_response_size_mb: int | None = Field( None, description="max response size in MB, if a response is larger than this size it will be rejected", diff --git a/litellm/proxy/openai_files_endpoints/files_endpoints.py b/litellm/proxy/openai_files_endpoints/files_endpoints.py index 9bc90260de1..bf07f4748ef 100644 --- a/litellm/proxy/openai_files_endpoints/files_endpoints.py +++ b/litellm/proxy/openai_files_endpoints/files_endpoints.py @@ -70,6 +70,15 @@ from litellm.proxy.openai_files_endpoints.common_utils import ( validate_managed_files_requirement, validate_managed_id_requirement, ) +from litellm.proxy.openai_files_endpoints.general_upload_validation import ( + MB, + check_blocked_extension, + check_unsafe_filename, + check_upload_file_size, + coerce_optional_int_setting, + coerce_optional_str_list_setting, + raise_upload_validation_failure, +) from litellm.proxy.utils import ProxyLogging, is_known_model from litellm.repositories.table_repositories import ManagedFileRepository from litellm.router import Router @@ -397,13 +406,23 @@ async def create_file( # descriptor and its disk blocks until the collector runs. spools: Final[list[BinaryIO]] = [] # mutable-ok: filled as the scan opens handles try: + unsafe_filename_failure: Final = check_unsafe_filename(file.filename) + if unsafe_filename_failure is not None: + raise_upload_validation_failure(unsafe_filename_failure) + + max_file_size_mb: Final = coerce_optional_int_setting(general_settings.get("max_file_size_mb")) + # Batch uploads can be gigabytes. Starlette has already spooled the upload # to disk, so stream from that handle instead of reading it into memory. - # Other uploads are small and stay in-memory bytes. + # Other uploads stay in-memory bytes, bounded to max_file_size_mb (plus one + # byte, to still tell "exactly at the limit" from "over it") when it is set, + # so an oversized upload cannot be read to completion before it is rejected. file_source: bytes | BinaryIO if purpose == "batch": await file.seek(0) file_source = file.file + elif max_file_size_mb is not None and max_file_size_mb > 0: + file_source = await file.read(max_file_size_mb * MB + 1) else: file_source = await file.read() custom_llm_provider = ( @@ -442,6 +461,15 @@ async def create_file( # Cast purpose to OpenAIFilesPurpose type purpose = cast(OpenAIFilesPurpose, purpose) + general_size_failure: Final = check_upload_file_size(file_source, max_file_size_mb) + if general_size_failure is not None: + raise_upload_validation_failure(general_size_failure) + + blocked_extensions: Final = coerce_optional_str_list_setting(general_settings.get("blocked_file_extensions")) + blocked_extension_failure: Final = check_blocked_extension(file.filename, blocked_extensions) + if blocked_extension_failure is not None: + raise_upload_validation_failure(blocked_extension_failure) + if purpose == "batch": batch_file_failure: Final = await asyncio.to_thread( check_batch_file_upload, diff --git a/litellm/proxy/openai_files_endpoints/general_upload_validation.py b/litellm/proxy/openai_files_endpoints/general_upload_validation.py new file mode 100644 index 00000000000..9d450cb5b8d --- /dev/null +++ b/litellm/proxy/openai_files_endpoints/general_upload_validation.py @@ -0,0 +1,150 @@ +""" +Upload validation applied to every purpose at POST /v1/files. + +batch_file_validation.py checks the JSONL shape of purpose="batch" uploads; this +module applies the same fast-fail-before-forwarding shape (size cap, blocked +extensions, path-traversal filenames) regardless of purpose. +""" + +from dataclasses import dataclass +from pathlib import Path +from typing import BinaryIO, Final, NoReturn, assert_never + +from litellm.proxy._types import ProxyException +from litellm.proxy.common_utils.path_utils import safe_filename + +MB: Final = 1024 * 1024 + + +def coerce_optional_int_setting(raw: object) -> int | None: + """A general_settings value declared as an optional integer, e.g. max_file_size_mb. + + bool is an int subclass, so an explicit isinstance(raw, bool) exclusion is needed + or a YAML `true`/`false` would silently pass as 1/0. + """ + if raw is None: + return None + if isinstance(raw, int) and not isinstance(raw, bool): + return raw + raise TypeError(f"expected an integer, got {raw!r}") + + +def coerce_optional_str_list_setting(raw: object) -> tuple[str, ...]: + """A general_settings value declared as an optional list of strings, e.g. blocked_file_extensions.""" + if raw is None: + return () + if not isinstance(raw, list) or not all(isinstance(item, str) for item in raw): + raise TypeError(f"expected a list of strings, got {raw!r}") + return tuple(raw) + + +@dataclass(frozen=True, slots=True) +class UploadedFileTooLarge: + size_bytes: int + limit_mb: int + + +@dataclass(frozen=True, slots=True) +class UploadedFileBlockedExtension: + extension: str + + +@dataclass(frozen=True, slots=True) +class UploadedFileUnsafeFilename: + filename: str + + +UploadValidationFailure = UploadedFileTooLarge | UploadedFileBlockedExtension | UploadedFileUnsafeFilename + + +def _file_size_bytes(file_source: bytes | BinaryIO) -> int: + if isinstance(file_source, bytes): + return len(file_source) + original_position: Final = file_source.tell() + file_source.seek(0, 2) + size: Final = file_source.tell() + file_source.seek(original_position) + return size + + +def check_upload_file_size( + file_source: bytes | BinaryIO, + max_file_size_mb: int | None, +) -> UploadedFileTooLarge | None: + if max_file_size_mb is None or max_file_size_mb <= 0: + return None + size_bytes: Final = _file_size_bytes(file_source) + if size_bytes > max_file_size_mb * MB: + return UploadedFileTooLarge(size_bytes=size_bytes, limit_mb=max_file_size_mb) + return None + + +def check_blocked_extension( + filename: str | None, + blocked_extensions: tuple[str, ...], +) -> UploadedFileBlockedExtension | None: + if not blocked_extensions or not filename: + return None + try: + extension: Final = Path(safe_filename(filename)).suffix.lower() + except ValueError: + return None + # The uploaded name's extension is normalized above; blocked_extensions comes + # straight from config.yaml or the DB and is normalized here too, so a + # differently-cased entry (".EXE") still catches a lowercase upload. + normalized_blocked: Final = frozenset(item.lower() for item in blocked_extensions) + if extension and extension in normalized_blocked: + return UploadedFileBlockedExtension(extension=extension) + return None + + +def check_unsafe_filename(filename: str | None) -> UploadedFileUnsafeFilename | None: + """Reject a filename before it can influence any storage path or backend call. + + Only flags a genuine traversal component ("..") or a null byte, so an ordinary + name like "report.v2.pdf" or ".env" is never rejected. + """ + if not filename: + return None + if "\x00" in filename: + return UploadedFileUnsafeFilename(filename=filename) + normalized: Final = filename.replace("\\", "/") + if any(part == ".." for part in normalized.split("/")): + return UploadedFileUnsafeFilename(filename=filename) + return None + + +def raise_upload_validation_failure(failure: UploadValidationFailure) -> NoReturn: + match failure: + case UploadedFileTooLarge(size_bytes=size_bytes, limit_mb=limit_mb): + raise ProxyException( + message=( + f"Uploaded file exceeds the configured max_file_size_mb of {limit_mb} MB " + f"(read stopped at {size_bytes / MB:.1f} MB). The file was not forwarded to the provider." + ), + type="invalid_request_error", + param="file", + code=413, + ) + case UploadedFileBlockedExtension(extension=extension): + raise ProxyException( + message=( + f"File extension '{extension}' is blocked by this proxy's blocked_file_extensions " + "setting. The file was not forwarded to the provider." + ), + type="invalid_request_error", + param="file", + code=400, + ) + case UploadedFileUnsafeFilename(filename=filename): + raise ProxyException( + message=( + f"Filename '{filename}' is not allowed: directory traversal sequences are not " + "permitted in uploaded file names. The file was not forwarded to the provider." + ), + type="invalid_request_error", + param="file", + code=400, + ) + case _: + assert_never(failure) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index d52f05f6166..9cc3882ee09 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -6644,6 +6644,12 @@ class ProxyConfig: if "max_batch_file_size_mb" not in self._yaml_general_settings_keys: general_settings["max_batch_file_size_mb"] = _general_settings.get("max_batch_file_size_mb") + if "max_file_size_mb" not in self._yaml_general_settings_keys: + general_settings["max_file_size_mb"] = _general_settings.get("max_file_size_mb") + + if "blocked_file_extensions" not in self._yaml_general_settings_keys: + general_settings["blocked_file_extensions"] = _general_settings.get("blocked_file_extensions") + ## ALERTING ARGS ## if "alerting_args" in _general_settings: general_settings["alerting_args"] = _general_settings["alerting_args"] @@ -16412,6 +16418,8 @@ _GENERAL_SETTINGS_CONFIG_LIST_FIELD_TYPES: Final[Mapping[str, str]] = MappingPro "global_max_parallel_requests": "Integer", "max_request_size_mb": "Integer", "max_batch_file_size_mb": "Integer", + "max_file_size_mb": "Integer", + "blocked_file_extensions": "List", "max_response_size_mb": "Integer", "proxy_config_reload_interval_seconds": "Integer", "pass_through_endpoints": "PydanticModel", diff --git a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py index 52258602581..91fcdbd34dd 100644 --- a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py +++ b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py @@ -1594,7 +1594,10 @@ async def update_ui_settings( tags=["UI Theme Settings"], dependencies=[Depends(user_api_key_auth)], ) -async def upload_logo(file: UploadFile = File(...)): +async def upload_logo( + file: UploadFile = File(...), + user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), +): """ Upload a custom logo for the admin UI. Accepts image files (PNG, JPG, JPEG, SVG) and stores them for use in the UI. @@ -1602,6 +1605,12 @@ async def upload_logo(file: UploadFile = File(...)): import os from pathlib import Path + if user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN: + raise HTTPException( + status_code=403, + detail="Only proxy admins can upload a UI logo.", + ) + # Validate file type allowed_extensions: Final = {".png", ".jpg", ".jpeg", ".svg"} file_extension: Final = Path(file.filename or "").suffix.lower() @@ -1612,9 +1621,11 @@ async def upload_logo(file: UploadFile = File(...)): detail=f"Invalid file type. Allowed types: {', '.join(allowed_extensions)}", ) - # Validate file size (max 5MB) - file_content: Final = await file.read() - if len(file_content) > 5 * 1024 * 1024: # 5MB + # Read bounded to one byte past the limit, so an oversized upload is never + # fully buffered in memory before being rejected. + max_logo_size_bytes: Final = 5 * 1024 * 1024 + file_content: Final = await file.read(max_logo_size_bytes + 1) + if len(file_content) > max_logo_size_bytes: raise HTTPException(status_code=400, detail="File size too large. Maximum size is 5MB.") # Create uploads directory if it doesn't exist diff --git a/tests/test_litellm/llms/base_llm/files/test_azure_blob_storage_backend.py b/tests/test_litellm/llms/base_llm/files/test_azure_blob_storage_backend.py index b924ea8f93f..7e1139ca79a 100644 --- a/tests/test_litellm/llms/base_llm/files/test_azure_blob_storage_backend.py +++ b/tests/test_litellm/llms/base_llm/files/test_azure_blob_storage_backend.py @@ -184,6 +184,50 @@ async def test_download_file_drops_query_string_from_the_stored_url(mock_env_var ) +@pytest.mark.parametrize( + "malicious_filename", + [ + "report.jsonl/../../etc/cron.d/evil", + "a.b/../../../root/.ssh/authorized_keys", + ], +) +@pytest.mark.parametrize("strategy", ["uuid", "timestamp"]) +@pytest.mark.asyncio +async def test_generate_file_name_strips_path_traversal_from_extension(mock_env_vars, malicious_filename, strategy): + """ + original_filename.split(".")[-1] does not parse path structure, so a filename whose + last "." is followed by a directory traversal sequence used to put that sequence + straight into the blob path built from this name. The mutant this pins is reverting + _safe_extension() back to that bare split. + """ + backend = _make_backend() + generated = backend._generate_file_name(malicious_filename, strategy) + assert "/" not in generated + assert ".." not in generated + + +@pytest.mark.asyncio +async def test_generate_file_name_uuid_strategy_preserves_ordinary_extension(mock_env_vars): + backend = _make_backend() + generated = backend._generate_file_name("data.jsonl", "uuid") + assert generated.endswith(".jsonl") + + +@pytest.mark.asyncio +async def test_generate_file_name_original_filename_strategy_strips_directory_components(mock_env_vars): + """The blob name must never carry a directory the caller supplied, traversal or not.""" + backend = _make_backend() + generated = backend._generate_file_name("../../etc/passwd", "original_filename") + assert generated == "passwd" + + +@pytest.mark.asyncio +async def test_generate_file_name_null_byte_filename_falls_back_to_safe_default(mock_env_vars): + backend = _make_backend() + generated = backend._generate_file_name("report.pdf\x00.exe", "uuid") + assert "\x00" not in generated + + @pytest.mark.parametrize( "env_fixture, expected_suffix", [("mock_env_vars", "core.windows.net"), ("mock_gov_env_vars", GOV_SUFFIX)], diff --git a/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py b/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py index 87e0319f6a1..bca97915347 100644 --- a/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py +++ b/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py @@ -4555,3 +4555,114 @@ def test_create_file_async_pre_call_hook_rejection_blocks_upload(monkeypatch, ll assert response.status_code == 400, response.text assert "file upload not allowed" in response.text assert provider_route.call_count == 0 + + +def test_create_file_non_batch_over_max_file_size_mb_rejected_before_forwarding(monkeypatch, llm_router: Router): + """max_file_size_mb applies to every purpose, unlike the batch-only max_batch_file_size_mb.""" + import litellm.proxy.proxy_server as ps + + forwarded_calls = _setup_batch_upload_endpoint(monkeypatch, llm_router) + monkeypatch.setitem(ps.general_settings, "max_file_size_mb", 1) + + oversized = b"x" * (2 * 1024 * 1024) + try: + response = client.post( + "/v1/files", + files={"file": ("labels.jsonl", oversized, "application/octet-stream")}, + data={"purpose": "user_data"}, + headers={"Authorization": "Bearer test-key"}, + ) + finally: + _teardown_batch_upload_endpoint() + + assert response.status_code == 413, response.text + error = response.json()["error"] + assert error["type"] == "invalid_request_error" + assert error["param"] == "file" + assert "max_file_size_mb" in error["message"] + assert "1 MB" in error["message"] + assert forwarded_calls == [] + + +def test_create_file_non_batch_under_max_file_size_mb_forwards(monkeypatch, llm_router: Router): + import litellm.proxy.proxy_server as ps + + forwarded_calls = _setup_batch_upload_endpoint(monkeypatch, llm_router) + monkeypatch.setitem(ps.general_settings, "max_file_size_mb", 1) + + try: + response = client.post( + "/v1/files", + files={"file": ("labels.jsonl", b"small content", "application/octet-stream")}, + data={"purpose": "user_data"}, + headers={"Authorization": "Bearer test-key"}, + ) + finally: + _teardown_batch_upload_endpoint() + + assert response.status_code == 200, response.text + assert len(forwarded_calls) == 1 + + +def test_create_file_blocked_extension_rejected_before_forwarding(monkeypatch, llm_router: Router): + import litellm.proxy.proxy_server as ps + + forwarded_calls = _setup_batch_upload_endpoint(monkeypatch, llm_router) + monkeypatch.setitem(ps.general_settings, "blocked_file_extensions", [".exe", ".sh"]) + + try: + response = client.post( + "/v1/files", + files={"file": ("payload.exe", b"MZ\x90\x00", "application/octet-stream")}, + data={"purpose": "user_data"}, + headers={"Authorization": "Bearer test-key"}, + ) + finally: + _teardown_batch_upload_endpoint() + + assert response.status_code == 400, response.text + error = response.json()["error"] + assert error["type"] == "invalid_request_error" + assert error["param"] == "file" + assert ".exe" in error["message"] + assert "blocked_file_extensions" in error["message"] + assert forwarded_calls == [] + + +def test_create_file_blocked_extension_unset_allows_everything(monkeypatch, llm_router: Router): + forwarded_calls = _setup_batch_upload_endpoint(monkeypatch, llm_router) + + try: + response = client.post( + "/v1/files", + files={"file": ("payload.exe", b"MZ\x90\x00", "application/octet-stream")}, + data={"purpose": "user_data"}, + headers={"Authorization": "Bearer test-key"}, + ) + finally: + _teardown_batch_upload_endpoint() + + assert response.status_code == 200, response.text + assert len(forwarded_calls) == 1 + + +def test_create_file_path_traversal_filename_rejected_before_forwarding(monkeypatch, llm_router: Router): + """A filename carrying a directory-traversal component must never reach storage or the provider.""" + forwarded_calls = _setup_batch_upload_endpoint(monkeypatch, llm_router) + + try: + response = client.post( + "/v1/files", + files={"file": ("../../etc/passwd", b"malicious content", "text/plain")}, + data={"purpose": "user_data"}, + headers={"Authorization": "Bearer test-key"}, + ) + finally: + _teardown_batch_upload_endpoint() + + assert response.status_code == 400, response.text + error = response.json()["error"] + assert error["type"] == "invalid_request_error" + assert error["param"] == "file" + assert "traversal" in error["message"].lower() + assert forwarded_calls == [] diff --git a/tests/test_litellm/proxy/openai_files_endpoint/test_general_upload_validation.py b/tests/test_litellm/proxy/openai_files_endpoint/test_general_upload_validation.py new file mode 100644 index 00000000000..9f7dd914e4a --- /dev/null +++ b/tests/test_litellm/proxy/openai_files_endpoint/test_general_upload_validation.py @@ -0,0 +1,134 @@ +import io + +import pytest + +from litellm.proxy._types import ProxyException +from litellm.proxy.openai_files_endpoints.general_upload_validation import ( + MB, + UploadedFileBlockedExtension, + UploadedFileTooLarge, + UploadedFileUnsafeFilename, + check_blocked_extension, + check_unsafe_filename, + check_upload_file_size, + raise_upload_validation_failure, +) + + +def test_size_under_cap_allowed(): + assert check_upload_file_size(b"x" * 100, 1) is None + + +def test_size_over_cap_rejected_for_bytes(): + content = b"x" * (2 * MB) + assert check_upload_file_size(content, 1) == UploadedFileTooLarge(size_bytes=len(content), limit_mb=1) + + +def test_size_over_cap_rejected_for_binaryio_and_restores_caller_position(): + """The handle is caller-owned; inspecting its size must not discard where the caller had it.""" + content = b"x" * (2 * MB) + handle = io.BytesIO(content) + handle.seek(17) + assert check_upload_file_size(handle, 1) == UploadedFileTooLarge(size_bytes=len(content), limit_mb=1) + assert handle.tell() == 17 + + +def test_size_under_cap_allowed_for_binaryio_restores_caller_position(): + handle = io.BytesIO(b"x" * 100) + handle.seek(42) + assert check_upload_file_size(handle, 1) is None + assert handle.tell() == 42 + + +def test_size_exactly_at_cap_allowed(): + content = b"x" * MB + assert check_upload_file_size(content, 1) is None + + +def test_no_cap_skips_size_check(): + assert check_upload_file_size(b"x" * (10 * MB), None) is None + + +@pytest.mark.parametrize("cap", [0, -3]) +def test_nonpositive_cap_disables_size_check(cap): + assert check_upload_file_size(b"x" * (10 * MB), cap) is None + + +def test_blocked_extension_rejected(): + assert check_blocked_extension("payload.exe", (".exe", ".sh")) == UploadedFileBlockedExtension(extension=".exe") + + +def test_blocked_extension_match_is_case_insensitive(): + assert check_blocked_extension("payload.EXE", (".exe",)) == UploadedFileBlockedExtension(extension=".exe") + + +def test_blocked_extension_match_is_case_insensitive_for_configured_value(): + """A config entry like blocked_file_extensions: ['.EXE'] must still catch a lowercase upload.""" + assert check_blocked_extension("payload.exe", (".EXE",)) == UploadedFileBlockedExtension(extension=".exe") + + +def test_extension_not_in_blocklist_allowed(): + assert check_blocked_extension("report.pdf", (".exe", ".sh")) is None + + +def test_empty_blocklist_allows_everything(): + assert check_blocked_extension("payload.exe", ()) is None + + +def test_no_filename_skips_extension_check(): + assert check_blocked_extension(None, (".exe",)) is None + + +def test_path_traversal_filename_rejected(): + assert check_unsafe_filename("../../etc/passwd") == UploadedFileUnsafeFilename(filename="../../etc/passwd") + + +def test_windows_style_path_traversal_filename_rejected(): + assert check_unsafe_filename("..\\..\\windows\\system32\\config") == UploadedFileUnsafeFilename( + filename="..\\..\\windows\\system32\\config" + ) + + +def test_traversal_embedded_after_extension_rejected(): + assert check_unsafe_filename("report.jsonl/../../etc/cron.d/evil") == UploadedFileUnsafeFilename( + filename="report.jsonl/../../etc/cron.d/evil" + ) + + +def test_null_byte_filename_rejected(): + assert check_unsafe_filename("report.pdf\x00.exe") == UploadedFileUnsafeFilename(filename="report.pdf\x00.exe") + + +@pytest.mark.parametrize("filename", ["report.pdf", ".env", "a.b.c.jsonl", "my file (1).csv", None]) +def test_ordinary_filenames_allowed(filename): + assert check_unsafe_filename(filename) is None + + +@pytest.mark.parametrize( + "failure, expected_code, expected_fragments", + [ + ( + UploadedFileTooLarge(size_bytes=15728640, limit_mb=10), + "413", + ("15.0 MB", "max_file_size_mb", "10 MB", "not forwarded"), + ), + ( + UploadedFileBlockedExtension(extension=".exe"), + "400", + (".exe", "blocked_file_extensions", "not forwarded"), + ), + ( + UploadedFileUnsafeFilename(filename="../../etc/passwd"), + "400", + ("../../etc/passwd", "traversal", "not forwarded"), + ), + ], +) +def test_failures_map_to_openai_shaped_proxy_exceptions(failure, expected_code, expected_fragments): + with pytest.raises(ProxyException) as exc_info: + raise_upload_validation_failure(failure) + assert exc_info.value.code == expected_code + assert exc_info.value.type == "invalid_request_error" + assert exc_info.value.param == "file" + for fragment in expected_fragments: + assert fragment in exc_info.value.message diff --git a/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py b/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py index dc256ccf718..860a3e4ee53 100644 --- a/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py +++ b/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py @@ -3006,6 +3006,56 @@ def test_update_mcp_semantic_filter_settings_requires_proxy_admin(monkeypatch): app.dependency_overrides.pop(user_api_key_auth, None) +def test_upload_logo_requires_proxy_admin(monkeypatch): + """Any authenticated key could previously write a file to the server's disk here.""" + from litellm.proxy._types import UserAPIKeyAuth + from litellm.proxy.auth.user_api_key_auth import user_api_key_auth + + async def _internal_user_auth(): + return UserAPIKeyAuth( + user_id="internal-user-1", + api_key="hashed-internal-key", + user_role=LitellmUserRoles.INTERNAL_USER, + ) + + app.dependency_overrides[user_api_key_auth] = _internal_user_auth + try: + resp = client.post( + "/upload/logo", + files={"file": ("logo.png", b"\x89PNG\r\n\x1a\n" + b"x" * 32, "image/png")}, + ) + assert resp.status_code == 403 + assert "proxy admin" in resp.json()["detail"].lower() + finally: + app.dependency_overrides.pop(user_api_key_auth, None) + + +def test_upload_logo_allows_proxy_admin(monkeypatch, tmp_path): + from litellm.proxy._types import UserAPIKeyAuth + from litellm.proxy.auth.user_api_key_auth import user_api_key_auth + + async def _admin_auth(): + return UserAPIKeyAuth( + user_id="admin-1", + api_key="hashed-admin-key", + user_role=LitellmUserRoles.PROXY_ADMIN, + ) + + app.dependency_overrides[user_api_key_auth] = _admin_auth + try: + resp = client.post( + "/upload/logo", + files={"file": ("logo.png", b"\x89PNG\r\n\x1a\n" + b"x" * 32, "image/png")}, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["status"] == "success" + finally: + app.dependency_overrides.pop(user_api_key_auth, None) + uploaded_path = resp.json().get("file_path") + if uploaded_path and os.path.exists(uploaded_path): + os.remove(uploaded_path) + + class TestPtuCostAttributionUISetting: """``enable_ptu_cost_attribution`` is derived from the environment on every GET. diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index a1b27070003..d7d5ad58f35 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -25438,6 +25438,11 @@ export interface components { * @description run health checks in background */ background_health_checks?: boolean | null; + /** + * Blocked File Extensions + * @description file extensions (e.g. ['.exe', '.sh']) rejected on /v1/files uploads, for any purpose, matched case-insensitively against the uploaded filename + */ + blocked_file_extensions?: string[] | null; /** * Cancel On Disconnect * @description cancel the in-flight upstream LLM request (non-streaming) when the client disconnects, freeing backend capacity (e.g. a vLLM GPU slot); the request is logged as a 499 failure @@ -25577,6 +25582,11 @@ export interface components { * @description max batch input file size in MB for /v1/files uploads with purpose=batch, if a file is larger than this size it will be rejected before being forwarded to the provider */ max_batch_file_size_mb?: number | null; + /** + * Max File Size Mb + * @description max file size in MB for /v1/files uploads, for any purpose, if a file is larger than this size it will be rejected before being forwarded to the provider + */ + max_file_size_mb?: number | null; /** * Max Parallel Requests * @description maximum parallel requests for each api key