From f53e8d680304a59db27ceb6ec1d9df27ec86ea9a Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 1 May 2026 16:01:19 -0700 Subject: [PATCH] harden bedrock file bucket validation --- .../cloud_storage_security.py | 5 +++- litellm/llms/bedrock/files/handler.py | 6 ++--- .../files/test_bedrock_files_handler.py | 25 +++++++++++++++++++ 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/litellm/litellm_core_utils/cloud_storage_security.py b/litellm/litellm_core_utils/cloud_storage_security.py index 3c60e0cd523..b47a07abb91 100644 --- a/litellm/litellm_core_utils/cloud_storage_security.py +++ b/litellm/litellm_core_utils/cloud_storage_security.py @@ -69,7 +69,10 @@ def _validate_cloud_object_path(object_name: str) -> None: raise ValueError("Cloud storage object name must be relative") if any(ord(char) < 32 or ord(char) == 127 for char in object_name): raise ValueError("Cloud storage object name contains control characters") - if any(segment in {".", ".."} for segment in object_name.split("/")): + segments = object_name.split("/") + if any(segment in {".", ".."} for segment in segments): + raise ValueError("Cloud storage object name contains an invalid path segment") + if "" in segments[:-1]: raise ValueError("Cloud storage object name contains an invalid path segment") diff --git a/litellm/llms/bedrock/files/handler.py b/litellm/llms/bedrock/files/handler.py index 6061fc8c0c9..b3cd77cff72 100644 --- a/litellm/llms/bedrock/files/handler.py +++ b/litellm/llms/bedrock/files/handler.py @@ -90,12 +90,10 @@ class BedrockFilesHandler(BaseAWSLLM): ) def _get_configured_s3_bucket_name(self, optional_params: dict) -> str: - bucket_name = optional_params.get("s3_bucket_name") or os.getenv( - "AWS_S3_BUCKET_NAME" - ) + bucket_name = os.getenv("AWS_S3_BUCKET_NAME") if not bucket_name: raise ValueError( - "S3 bucket_name is required. Set 's3_bucket_name' or AWS_S3_BUCKET_NAME." + "S3 bucket_name is required. Set AWS_S3_BUCKET_NAME for Bedrock file content retrieval." ) return bucket_name diff --git a/tests/test_litellm/llms/bedrock/files/test_bedrock_files_handler.py b/tests/test_litellm/llms/bedrock/files/test_bedrock_files_handler.py index b8375748295..d1622008b0e 100644 --- a/tests/test_litellm/llms/bedrock/files/test_bedrock_files_handler.py +++ b/tests/test_litellm/llms/bedrock/files/test_bedrock_files_handler.py @@ -1,4 +1,6 @@ import base64 +import os +from unittest.mock import patch import pytest @@ -60,6 +62,13 @@ class TestBedrockFilesHandler: configured_bucket_name="safe-bucket", ) + def test_should_reject_empty_middle_path_segment(self): + with pytest.raises(ValueError, match="invalid path segment"): + self.handler._parse_s3_uri( + s3_uri="s3://safe-bucket/litellm-bedrock-files//secret.jsonl", + configured_bucket_name="safe-bucket", + ) + def test_should_extract_unified_managed_s3_uri(self): file_id = _encode_unified_file_id( "s3://safe-bucket/litellm-batch-outputs/job/output.jsonl" @@ -83,3 +92,19 @@ class TestBedrockFilesHandler: s3_uri=s3_uri, configured_bucket_name="safe-bucket", ) + + def test_should_not_trust_request_s3_bucket_name_for_expected_bucket(self): + with patch.dict(os.environ, {"AWS_S3_BUCKET_NAME": "safe-bucket"}): + assert ( + self.handler._get_configured_s3_bucket_name( + {"s3_bucket_name": "attacker-bucket"} + ) + == "safe-bucket" + ) + + def test_should_require_server_s3_bucket_name(self): + with patch.dict(os.environ, {}, clear=True): + with pytest.raises(ValueError, match="AWS_S3_BUCKET_NAME"): + self.handler._get_configured_s3_bucket_name( + {"s3_bucket_name": "attacker-bucket"} + )