From d238e602203cafe0582eb0003255e4c6958d8858 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 7 Sep 2026 18:38:25 -0700 Subject: [PATCH] fix(bedrock): answer 400 for a file id outside the configured bucket and keep S3 error bodies --- litellm/llms/bedrock/files/transformation.py | 45 ++++++-- .../test_bedrock_files_transformation.py | 100 +++++++++++++++++- .../test_files_endpoint.py | 49 +++++++++ 3 files changed, 178 insertions(+), 16 deletions(-) diff --git a/litellm/llms/bedrock/files/transformation.py b/litellm/llms/bedrock/files/transformation.py index 1266397636a..a7f1b380fe2 100644 --- a/litellm/llms/bedrock/files/transformation.py +++ b/litellm/llms/bedrock/files/transformation.py @@ -263,6 +263,35 @@ def _validate_file_id_against_configured_buckets( return validate_against(configured_bucket_names[-1]) +_REJECTED_FILE_ID_REQUEST_URL: Final = "https://docs.litellm.ai/docs" + + +def _rejected_file_id(reason: ValueError) -> BedrockError: + message: Final = str(reason) + return BedrockError( + status_code=400, + message=message, + response=httpx.Response( + status_code=400, + text=message, + request=httpx.Request(method="GET", url=_REJECTED_FILE_ID_REQUEST_URL), + ), + ) + + +def _resolve_managed_s3_object(file_id: str, litellm_params: Mapping[str, object]) -> tuple[str, str]: + configured_bucket_names: Final = get_configured_s3_bucket_names(litellm_params) + allow_legacy_cloud_file_ids: Final = should_allow_legacy_cloud_file_ids(litellm_params) + try: + return _validate_file_id_against_configured_buckets( + s3_uri=extract_s3_uri_from_file_id(file_id), + configured_bucket_names=configured_bucket_names, + allow_legacy_cloud_file_ids=allow_legacy_cloud_file_ids, + ) + except ValueError as reason: + raise _rejected_file_id(reason) from reason + + _ANY_MANAGED_LISTING_PREFIX: Final = os.path.commonprefix(BEDROCK_MANAGED_S3_PREFIXES) _MANAGED_LISTING_PREFIX_BY_PURPOSE: Final = MappingProxyType( { @@ -1279,11 +1308,7 @@ class BedrockFilesConfig(BaseAWSLLM, BaseFilesConfig): ) -> tuple[str, dict[str, str]]: if not file_id: raise ValueError("file_id is required for Bedrock file deletion") - bucket_name, object_key = _validate_file_id_against_configured_buckets( - s3_uri=extract_s3_uri_from_file_id(file_id), - configured_bucket_names=get_configured_s3_bucket_names(litellm_params), - allow_legacy_cloud_file_ids=should_allow_legacy_cloud_file_ids(litellm_params), - ) + bucket_name, object_key = _resolve_managed_s3_object(file_id=file_id, litellm_params=litellm_params) target: Final = self._s3_request_target(optional_params=optional_params, litellm_params=litellm_params) url: Final = f"{target.endpoint_url}/{bucket_name}/{encode_s3_object_key_for_url(object_key)}" signed_headers: Final = self._sign_s3_empty_body_request( @@ -1307,6 +1332,7 @@ class BedrockFilesConfig(BaseAWSLLM, BaseFilesConfig): status_code=raw_response.status_code, message=raw_response.text, headers=raw_response.headers, + response=raw_response, ) return FileDeleted(id=str(litellm_params.get(DELETED_FILE_ID_PARAM, "")), deleted=True, object="file") @@ -1371,6 +1397,7 @@ class BedrockFilesConfig(BaseAWSLLM, BaseFilesConfig): status_code=raw_response.status_code, message=raw_response.text, headers=raw_response.headers, + response=raw_response, ) purpose: Final = _requested_listing_purpose(litellm_params) configured_bucket_name: Final = _listing_bucket_name(litellm_params, purpose) @@ -1406,12 +1433,7 @@ class BedrockFilesConfig(BaseAWSLLM, BaseFilesConfig): if not file_id: raise ValueError("file_id is required for Bedrock file content retrieval") - s3_uri: Final = extract_s3_uri_from_file_id(file_id) - bucket_name, object_key = _validate_file_id_against_configured_buckets( - s3_uri=s3_uri, - configured_bucket_names=get_configured_s3_bucket_names(litellm_params), - allow_legacy_cloud_file_ids=should_allow_legacy_cloud_file_ids(litellm_params), - ) + bucket_name, object_key = _resolve_managed_s3_object(file_id=file_id, litellm_params=litellm_params) target: Final = self._s3_request_target(optional_params=optional_params, litellm_params=litellm_params) url: Final = f"{target.endpoint_url}/{bucket_name}/{encode_s3_object_key_for_url(object_key)}" @@ -1502,6 +1524,7 @@ class BedrockFilesConfig(BaseAWSLLM, BaseFilesConfig): status_code=raw_response.status_code, message=raw_response.text, headers=raw_response.headers, + response=raw_response, ) return HttpxBinaryResponseContent(response=raw_response) diff --git a/tests/test_litellm/llms/bedrock/files/test_bedrock_files_transformation.py b/tests/test_litellm/llms/bedrock/files/test_bedrock_files_transformation.py index 9bf9e468dca..12b9f63c99b 100644 --- a/tests/test_litellm/llms/bedrock/files/test_bedrock_files_transformation.py +++ b/tests/test_litellm/llms/bedrock/files/test_bedrock_files_transformation.py @@ -1930,11 +1930,12 @@ class TestBedrockFileContentTransformation: assert url == self.EXPECTED_URL def test_transform_file_content_request_rejects_foreign_bucket(self, monkeypatch): + from litellm.llms.bedrock.common_utils import BedrockError from litellm.llms.bedrock.files.transformation import BedrockFilesConfig monkeypatch.setenv("AWS_S3_BUCKET_NAME", "my-bucket") - with pytest.raises(ValueError, match="configured storage bucket"): + with pytest.raises(BedrockError, match="configured storage bucket") as rejection: BedrockFilesConfig().transform_file_content_request( file_content_request={ "file_id": "s3://other-bucket/litellm-batch-outputs/job/x.jsonl.out" @@ -1943,18 +1944,25 @@ class TestBedrockFileContentTransformation: litellm_params=self._litellm_params(), ) + assert rejection.value.status_code == 400 + + def test_transform_file_content_request_rejects_unmanaged_key(self, monkeypatch): + from litellm.llms.bedrock.common_utils import BedrockError from litellm.llms.bedrock.files.transformation import BedrockFilesConfig monkeypatch.setenv("AWS_S3_BUCKET_NAME", "my-bucket") - with pytest.raises(ValueError, match="LiteLLM-managed"): + with pytest.raises(BedrockError, match="LiteLLM-managed") as rejection: BedrockFilesConfig().transform_file_content_request( file_content_request={"file_id": "s3://my-bucket/private/x.jsonl"}, optional_params={}, litellm_params=self._litellm_params(), ) + assert rejection.value.status_code == 400 + + def test_extract_s3_uri_rejects_non_managed_file_id(self): """A file id that is neither an s3:// URI nor a unified id must be rejected.""" from litellm.llms.bedrock.files.transformation import ( @@ -2083,12 +2091,13 @@ class TestBedrockFileContentTransformation: def test_rejects_bucket_outside_input_and_output(self, monkeypatch): """A file id whose bucket is neither the input nor the output bucket is still rejected (SSRF / bucket-confusion guard).""" + from litellm.llms.bedrock.common_utils import BedrockError from litellm.llms.bedrock.files.transformation import BedrockFilesConfig monkeypatch.delenv("AWS_S3_BUCKET_NAME", raising=False) monkeypatch.delenv("AWS_S3_OUTPUT_BUCKET_NAME", raising=False) - with pytest.raises(ValueError, match="configured storage bucket"): + with pytest.raises(BedrockError, match="configured storage bucket") as rejection: BedrockFilesConfig().transform_file_content_request( file_content_request={ "file_id": "s3://other-bucket/litellm-batch-outputs/job/x.jsonl.out" @@ -2099,6 +2108,9 @@ class TestBedrockFileContentTransformation: ), ) + assert rejection.value.status_code == 400 + + def test_sign_request_without_botocore_raises_helpful_error(self, monkeypatch): """A missing botocore must surface an actionable 'install boto3' error rather than a raw import failure.""" @@ -2622,29 +2634,37 @@ class TestBedrockFileDeletionTransformation: assert url == self.EXPECTED_URL def test_transform_delete_file_request_rejects_foreign_bucket(self, monkeypatch): + from litellm.llms.bedrock.common_utils import BedrockError from litellm.llms.bedrock.files.transformation import BedrockFilesConfig monkeypatch.setenv("AWS_S3_BUCKET_NAME", "my-bucket") - with pytest.raises(ValueError, match="configured storage bucket"): + with pytest.raises(BedrockError, match="configured storage bucket") as rejection: BedrockFilesConfig().transform_delete_file_request( file_id="s3://other-bucket/litellm-bedrock-files/job-123/input.jsonl", optional_params={}, litellm_params=_bedrock_s3_params(), ) + assert rejection.value.status_code == 400 + + def test_transform_delete_file_request_rejects_unmanaged_key(self, monkeypatch): + from litellm.llms.bedrock.common_utils import BedrockError from litellm.llms.bedrock.files.transformation import BedrockFilesConfig monkeypatch.setenv("AWS_S3_BUCKET_NAME", "my-bucket") - with pytest.raises(ValueError, match="LiteLLM-managed"): + with pytest.raises(BedrockError, match="LiteLLM-managed") as rejection: BedrockFilesConfig().transform_delete_file_request( file_id="s3://my-bucket/private/x.jsonl", optional_params={}, litellm_params=_bedrock_s3_params(), ) + assert rejection.value.status_code == 400 + + def test_transform_delete_file_response_echoes_the_deleted_id(self): import httpx @@ -2728,6 +2748,57 @@ class TestBedrockFileDeletionTransformation: assert response.id == self.S3_URI assert response.deleted is True + def test_file_delete_end_to_end_answers_400_for_a_foreign_bucket(self, monkeypatch): + import respx + + import litellm + from litellm.llms.bedrock.common_utils import BedrockError + + monkeypatch.setenv("AWS_S3_BUCKET_NAME", "my-bucket") + + with respx.mock, pytest.raises(BedrockError) as rejection: + litellm.file_delete( + file_id="s3://other-bucket/litellm-bedrock-files/job-123/input.jsonl", + custom_llm_provider="bedrock", + **_bedrock_s3_params(), + ) + + assert rejection.value.status_code == 400 + assert "configured storage bucket" in rejection.value.message + + def test_file_delete_end_to_end_answers_400_for_a_non_managed_id(self, monkeypatch): + import respx + + import litellm + from litellm.llms.bedrock.common_utils import BedrockError + + monkeypatch.setenv("AWS_S3_BUCKET_NAME", "my-bucket") + + with respx.mock, pytest.raises(BedrockError) as rejection: + litellm.file_delete(file_id="file-1234567890", custom_llm_provider="bedrock", **_bedrock_s3_params()) + + assert rejection.value.status_code == 400 + assert "managed LiteLLM S3 file id" in rejection.value.message + + def test_file_delete_end_to_end_surfaces_the_s3_error_body(self, monkeypatch): + import httpx + import respx + + import litellm + from litellm.llms.bedrock.common_utils import BedrockError + + monkeypatch.setenv("AWS_S3_BUCKET_NAME", "my-bucket") + + with respx.mock: + respx.delete(self.EXPECTED_URL).mock( + return_value=httpx.Response(403, text="AccessDenied") + ) + with pytest.raises(BedrockError) as denied: + litellm.file_delete(file_id=self.S3_URI, custom_llm_provider="bedrock", **_bedrock_s3_params()) + + assert denied.value.status_code == 403 + assert "AccessDenied" in denied.value.message + class TestBedrockFileListTransformation: """SigV4-signed S3 ListObjectsV2 over the LiteLLM-managed key prefixes.""" @@ -3305,3 +3376,22 @@ class TestBedrockFileListTransformation: ) self._assert_capped_listing(route, files) + + def test_file_list_end_to_end_surfaces_the_s3_error_body(self, monkeypatch): + import httpx + import respx + + import litellm + from litellm.llms.bedrock.common_utils import BedrockError + + monkeypatch.setenv("AWS_S3_BUCKET_NAME", "my-bucket") + + with respx.mock: + respx.get(self.BUCKET_URL, params__contains=self.BATCH_QUERY).mock( + return_value=httpx.Response(403, text="AccessDenied") + ) + with pytest.raises(BedrockError) as denied: + litellm.file_list(custom_llm_provider="bedrock", purpose="batch", **_bedrock_s3_params()) + + assert denied.value.status_code == 403 + assert "AccessDenied" in denied.value.message 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 a4b36487330..824170e6b3d 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 @@ -4733,3 +4733,52 @@ def test_list_files_target_model_names_passes_trusted_bedrock_credentials( assert isinstance(trusted_credentials, MappingProxyType) assert trusted_credentials["s3_bucket_name"] == "my-bucket" proxy_logging_obj.post_call_failure_hook.assert_not_called() + + +def test_delete_file_answers_400_for_an_id_outside_the_configured_bucket(mocker: MockerFixture, monkeypatch): + from urllib.parse import quote + + import litellm.proxy.proxy_server as ps + from litellm.proxy._types import LitellmUserRoles + + bedrock_router = Router( + model_list=[ + { + "model_name": "bedrock-claude", + "litellm_params": { + "model": "bedrock/anthropic.claude-3-5-sonnet-20240620-v1:0", + "aws_access_key_id": "AKIAEXAMPLE", + "aws_secret_access_key": "secret", + "aws_region_name": "us-west-2", + "s3_bucket_name": "my-bucket", + }, + }, + ] + ) + + proxy_logging_obj = setup_proxy_logging_object(monkeypatch, bedrock_router) + monkeypatch.setattr("litellm.proxy.proxy_server.master_key", None) + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", None) + monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", bedrock_router) + proxy_logging_obj.update_request_status = mocker.AsyncMock() + proxy_logging_obj.post_call_success_hook = mocker.AsyncMock(return_value=[]) + proxy_logging_obj.post_call_failure_hook = mocker.AsyncMock() + + app.dependency_overrides[ps.user_api_key_auth] = lambda: UserAPIKeyAuth( + api_key="test-key", + user_role=LitellmUserRoles.PROXY_ADMIN, + user_id="test-user", + ) + foreign_file_id: Final = quote("s3://other-bucket/litellm-bedrock-files/job-123/input.jsonl", safe="") + + try: + with respx.mock: + response = client.delete( + f"/v1/files/{foreign_file_id}?model=bedrock-claude", + headers={"Authorization": "Bearer test-key"}, + ) + finally: + app.dependency_overrides.pop(ps.user_api_key_auth, None) + + assert response.status_code == 400, response.text + assert "configured storage bucket" in response.json()["error"]["message"]