mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
fix: _filter_headers_for_aws_signature - Bedrock KB (#23571)
* fix: _filter_headers_for_aws_signature * fix: filter None header values in all post-signing re-merge paths Addresses Greptile feedback: None-valued headers were being filtered during SigV4 signing but re-merged back into the final headers dict afterward, which would cause downstream HTTP client failures. Made-with: Cursor
This commit is contained in:
parent
961d1d1a6c
commit
64f9e9df9a
2 changed files with 86 additions and 4 deletions
|
|
@ -1268,7 +1268,8 @@ class BaseAWSLLM:
|
|||
|
||||
# Add back all original headers (including forwarded ones) after signature calculation
|
||||
for header_name, header_value in headers.items():
|
||||
request.headers[header_name] = header_value
|
||||
if header_value is not None:
|
||||
request.headers[header_name] = header_value
|
||||
|
||||
if (
|
||||
extra_headers is not None and "Authorization" in extra_headers
|
||||
|
|
@ -1298,6 +1299,8 @@ class BaseAWSLLM:
|
|||
}
|
||||
|
||||
for header_name, header_value in headers.items():
|
||||
if header_value is None:
|
||||
continue
|
||||
header_lower = header_name.lower()
|
||||
if (
|
||||
header_lower in aws_headers
|
||||
|
|
@ -1393,7 +1396,8 @@ class BaseAWSLLM:
|
|||
# Add back original headers after signing. Only headers in SignedHeaders
|
||||
# are integrity-protected; forwarded headers (x-forwarded-*) must remain unsigned.
|
||||
for header_name, header_value in headers.items():
|
||||
request_headers_dict[header_name] = header_value
|
||||
if header_value is not None:
|
||||
request_headers_dict[header_name] = header_value
|
||||
if (
|
||||
headers is not None and "Authorization" in headers
|
||||
): # prevent sigv4 from overwriting the auth header
|
||||
|
|
|
|||
|
|
@ -14,15 +14,16 @@ from datetime import datetime, timedelta, timezone
|
|||
from typing import Any, Dict
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from botocore.awsrequest import AWSPreparedRequest, AWSRequest
|
||||
from botocore.credentials import Credentials
|
||||
from botocore.awsrequest import AWSRequest, AWSPreparedRequest
|
||||
|
||||
import litellm
|
||||
from litellm.caching.caching import DualCache
|
||||
from litellm.llms.bedrock.base_aws_llm import (
|
||||
AwsAuthError,
|
||||
BaseAWSLLM,
|
||||
Boto3CredentialsInfo,
|
||||
)
|
||||
from litellm.caching.caching import DualCache
|
||||
|
||||
# Global variable for the base_aws_llm.py file path
|
||||
|
||||
|
|
@ -1519,6 +1520,83 @@ def test_is_already_running_as_role_invalid_target_arn():
|
|||
assert base_aws_llm._is_already_running_as_role("not-a-valid-arn") is False
|
||||
|
||||
|
||||
def test_filter_headers_skips_none_values():
|
||||
"""
|
||||
Test that _filter_headers_for_aws_signature skips headers with None values.
|
||||
|
||||
Reproduces the issue where botocore's SigV4Auth crashes with
|
||||
'NoneType' object has no attribute 'split' when a header value is None.
|
||||
"""
|
||||
llm = BaseAWSLLM()
|
||||
|
||||
headers = {
|
||||
"Content-Type": "application/json",
|
||||
"x-amz-security-token": None,
|
||||
"x-amzn-bedrock-kb-session-id": None,
|
||||
"host": None,
|
||||
"x-amz-date": "20240101T000000Z",
|
||||
"x-custom-header": None,
|
||||
}
|
||||
|
||||
filtered = llm._filter_headers_for_aws_signature(headers)
|
||||
|
||||
assert filtered["Content-Type"] == "application/json"
|
||||
assert filtered["x-amz-date"] == "20240101T000000Z"
|
||||
assert "x-amz-security-token" not in filtered
|
||||
assert "x-amzn-bedrock-kb-session-id" not in filtered
|
||||
assert "host" not in filtered
|
||||
# Non-AWS headers are excluded regardless
|
||||
assert "x-custom-header" not in filtered
|
||||
|
||||
|
||||
def test_sign_request_with_none_header_values():
|
||||
"""
|
||||
End-to-end test that _sign_request does not crash when headers contain
|
||||
None values for x-amz-* keys.
|
||||
|
||||
This reproduces the Bedrock KB GovCloud issue where SigV4 signing failed
|
||||
with 'NoneType' object has no attribute 'split'.
|
||||
|
||||
Also verifies that None-valued headers are NOT re-merged into the
|
||||
returned headers dict (which would cause downstream HTTP client failures).
|
||||
"""
|
||||
llm = BaseAWSLLM()
|
||||
|
||||
mock_credentials = Credentials("test_key", "test_secret")
|
||||
|
||||
headers_with_nones = {
|
||||
"Content-Type": "application/json",
|
||||
"x-amzn-trace-id": None,
|
||||
"x-forwarded-for": None,
|
||||
}
|
||||
|
||||
with patch.object(
|
||||
llm, "get_credentials", return_value=mock_credentials
|
||||
), patch.object(
|
||||
llm, "_get_aws_region_name", return_value="us-gov-west-1"
|
||||
):
|
||||
result_headers, result_body = llm._sign_request(
|
||||
service_name="bedrock",
|
||||
headers=headers_with_nones,
|
||||
optional_params={
|
||||
"aws_access_key_id": "test_key",
|
||||
"aws_secret_access_key": "test_secret",
|
||||
"aws_region_name": "us-gov-west-1",
|
||||
},
|
||||
request_data={"retrievalQuery": {"text": "test query"}},
|
||||
api_base="https://bedrock-agent-runtime.us-gov-west-1.amazonaws.com/knowledgebases/KB123/retrieve",
|
||||
)
|
||||
|
||||
assert "Authorization" in result_headers
|
||||
assert result_body is not None
|
||||
|
||||
# None-valued headers must NOT appear in the returned headers
|
||||
for header_name, header_value in result_headers.items():
|
||||
assert header_value is not None, (
|
||||
f"Header '{header_name}' has None value in returned headers"
|
||||
)
|
||||
|
||||
|
||||
def test_is_already_running_as_role_ssl_verify_passed():
|
||||
"""
|
||||
Test that ssl_verify parameter is correctly passed to the STS client.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue