mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix: s3_region_name always wins over aws_region_name for S3 signing (Greptile feedback)
This commit is contained in:
parent
913ccf4983
commit
c98ad605dc
2 changed files with 61 additions and 6 deletions
|
|
@ -403,12 +403,13 @@ class BedrockFilesConfig(BaseAWSLLM, BaseFilesConfig):
|
|||
data=create_file_data,
|
||||
)
|
||||
|
||||
# If s3_region_name is set and aws_region_name is not, propagate it so
|
||||
# SigV4 signing uses the correct region (e.g. GovCloud us-gov-west-1).
|
||||
# s3_region_name always wins for S3 operations (same priority as in
|
||||
# get_complete_file_url above). Overwrite aws_region_name unconditionally
|
||||
# so the SigV4 region matches the URL region, avoiding SignatureDoesNotMatch.
|
||||
s3_region_name = litellm_params.get("s3_region_name") or optional_params.get(
|
||||
"s3_region_name"
|
||||
)
|
||||
if s3_region_name and not optional_params.get("aws_region_name"):
|
||||
if s3_region_name:
|
||||
optional_params = {**optional_params, "aws_region_name": s3_region_name}
|
||||
|
||||
# Sign the request and return a pre-signed request object
|
||||
|
|
|
|||
|
|
@ -278,8 +278,6 @@ class TestBedrockFilesTransformation:
|
|||
Previously the code fell back to us-west-2 even when s3_region_name was set,
|
||||
breaking GovCloud (us-gov-west-1) deployments.
|
||||
"""
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from litellm.llms.bedrock.files.transformation import BedrockFilesConfig
|
||||
|
||||
config = BedrockFilesConfig()
|
||||
|
|
@ -329,7 +327,7 @@ class TestBedrockFilesTransformation:
|
|||
When s3_region_name is provided, transform_create_file_request must pass
|
||||
that region to _sign_s3_request so SigV4 signatures use the correct region.
|
||||
"""
|
||||
from unittest.mock import MagicMock, patch
|
||||
from unittest.mock import patch
|
||||
|
||||
from litellm.llms.bedrock.files.transformation import BedrockFilesConfig
|
||||
|
||||
|
|
@ -376,6 +374,62 @@ class TestBedrockFilesTransformation:
|
|||
"s3_region_name must be forwarded as aws_region_name for SigV4 signing"
|
||||
)
|
||||
|
||||
def test_s3_region_name_wins_over_aws_region_name_for_signing(self):
|
||||
"""
|
||||
When both s3_region_name and aws_region_name are set to different values,
|
||||
s3_region_name must win for signing (same as for the URL). Otherwise the
|
||||
SigV4 signature would be computed against a different region than the URL,
|
||||
causing SignatureDoesNotMatch from AWS.
|
||||
"""
|
||||
from unittest.mock import patch
|
||||
|
||||
from litellm.llms.bedrock.files.transformation import BedrockFilesConfig
|
||||
|
||||
config = BedrockFilesConfig()
|
||||
|
||||
jsonl_content = json.dumps(
|
||||
{
|
||||
"custom_id": "req-1",
|
||||
"method": "POST",
|
||||
"url": "/v1/chat/completions",
|
||||
"body": {
|
||||
"model": "bedrock/amazon.nova-pro-v1:0",
|
||||
"messages": [{"role": "user", "content": "Hello"}],
|
||||
"max_tokens": 10,
|
||||
},
|
||||
}
|
||||
).encode()
|
||||
|
||||
create_file_data = {
|
||||
"file": ("batch.jsonl", jsonl_content, "application/jsonl"),
|
||||
"purpose": "batch",
|
||||
}
|
||||
|
||||
litellm_params = {
|
||||
"s3_bucket_name": "litellm-batch-352026",
|
||||
"s3_region_name": "us-gov-west-1",
|
||||
}
|
||||
# aws_region_name set to something different — s3_region_name must still win
|
||||
optional_params = {"aws_region_name": "us-east-1"}
|
||||
|
||||
captured_optional_params: dict = {}
|
||||
|
||||
def fake_sign(content, api_base, optional_params):
|
||||
captured_optional_params.update(optional_params)
|
||||
return {"Authorization": "fake"}, content
|
||||
|
||||
with patch.object(config, "_sign_s3_request", side_effect=fake_sign):
|
||||
config.transform_create_file_request(
|
||||
model="amazon.nova-pro-v1:0",
|
||||
create_file_data=create_file_data,
|
||||
optional_params=optional_params,
|
||||
litellm_params=litellm_params,
|
||||
)
|
||||
|
||||
assert captured_optional_params.get("aws_region_name") == "us-gov-west-1", (
|
||||
"s3_region_name must override aws_region_name for SigV4 signing"
|
||||
)
|
||||
|
||||
def test_openai_passthrough_still_works(self):
|
||||
"""
|
||||
Regression test: ensure OpenAI-compatible models (e.g. gpt-oss)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue