mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
fix(bedrock): answer 400 for a file id outside the configured bucket and keep S3 error bodies
This commit is contained in:
mateo-berri
parent
69608a29db
commit
d238e60220
3 changed files with 178 additions and 16 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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="<Error><Code>AccessDenied</Code></Error>")
|
||||
)
|
||||
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="<Error><Code>AccessDenied</Code></Error>")
|
||||
)
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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"]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue