scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-08-28 05:25:59 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 36

fix(s3_v2): percent-encode object keys once so signed and sent URLs match (#38005)

Browse source 
Co-authored-by: yucheng <yucheng@berri.ai>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
devin-ai-integration[bot] 2026-08-24 18:16:17 -07:00 • committed by GitHub
parent 82b0069f83
commit 8f6de53c92
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 141 additions and 74 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

99
litellm/integrations/s3_v2.py
View file

@ -11,6 +11,7 @@ import time
from collections.abc import Mapping
from datetime import datetime
from typing import Final, cast
from urllib.parse import quote
import litellm
from litellm._logging import print_verbose, verbose_logger
@ -206,6 +207,23 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
params.get("s3_sse_kms_key_id") or s3_sse_kms_key_id,
)
def _build_object_url(self, s3_object_key: str) -> str:
"""
Build the exact URL that is both signed and sent, with the key percent-encoded once.
S3SigV4Auth signs the path verbatim while S3 canonicalizes the received path with reserved
characters encoded, so an unencoded `=`, `+`, `&`, `#`, `?`, `%` or space in the key makes
the two signatures disagree (403 SignatureDoesNotMatch).
"""
encoded_key: Final = quote(s3_object_key, safe="/")
if self.s3_endpoint_url and self.s3_bucket_name:
if self.s3_use_virtual_hosted_style:
endpoint_host: Final = self.s3_endpoint_url.replace("https://", "").replace("http://", "")
protocol: Final = "https://" if self.s3_endpoint_url.startswith("https://") else "http://"
return f"{protocol}{self.s3_bucket_name}.{endpoint_host}/{encoded_key}"
return f"{self.s3_endpoint_url}/{self.s3_bucket_name}/{encoded_key}"
return f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{encoded_key}"
def _sse_headers(self) -> Mapping[str, str]:
candidates: Final = {
"x-amz-server-side-encryption": self.s3_server_side_encryption,
@ -292,7 +310,6 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
import base64
import hashlib
import requests
from botocore.auth import S3SigV4Auth
from botocore.awsrequest import AWSRequest
except ImportError:
@ -316,18 +333,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
verbose_logger.debug("s3_v2 logger - uploading data to s3 - %s", batch_logging_element.s3_object_key)
verbose_logger.debug("s3_v2 logger - s3_verify setting: %s", self.s3_verify)
# Prepare the URL
url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{batch_logging_element.s3_object_key}"
if self.s3_endpoint_url and self.s3_bucket_name:
if self.s3_use_virtual_hosted_style:
# Virtual-hosted-style: bucket.endpoint/key
endpoint_host: Final = self.s3_endpoint_url.replace("https://", "").replace("http://", "")
protocol: Final = "https://" if self.s3_endpoint_url.startswith("https://") else "http://"
url = f"{protocol}{self.s3_bucket_name}.{endpoint_host}/{batch_logging_element.s3_object_key}"
else:
# Path-style: endpoint/bucket/key
url = self.s3_endpoint_url + "/" + self.s3_bucket_name + "/" + batch_logging_element.s3_object_key
url: Final = self._build_object_url(batch_logging_element.s3_object_key)
# Convert JSON to string
json_string: Final = safe_dumps(batch_logging_element.payload)
@ -348,29 +354,19 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
"Cache-Control": "private, immutable, max-age=31536000, s-maxage=0",
**self._sse_headers(),
}
req: Final = requests.Request("PUT", url, data=json_string, headers=headers)
prepped: Final = req.prepare()
# Sign the request
aws_request: Final = AWSRequest(
method=prepped.method,
url=prepped.url,
data=prepped.body,
headers=prepped.headers,
)
aws_request: Final = AWSRequest(method="PUT", url=url, data=json_string, headers=headers)
aws_region_name: Final = self.get_aws_region_name_for_non_llm_api_calls(aws_region_name=self.s3_region_name)
S3SigV4Auth(credentials, "s3", aws_region_name).add_auth(aws_request)
# Prepare the signed headers
signed_headers: Final = dict(aws_request.headers.items())
# Use prepared URL so path segments match SigV4 canonical request (e.g. %20 for spaces).
request_url: Final = prepped.url or url
# Make the request with retry for transient S3 errors (500/503)
max_retries: Final = 3
for attempt in range(max_retries):
response = await self.async_httpx_client.put(request_url, data=json_string, headers=signed_headers)
response = await self.async_httpx_client.put(url, data=json_string, headers=signed_headers)
if response.status_code in (500, 503) and attempt < max_retries - 1:
wait_time = 2**attempt # 1s, 2s
verbose_logger.warning(
@ -478,7 +474,6 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
import base64
import hashlib
import requests
from botocore.auth import S3SigV4Auth
from botocore.awsrequest import AWSRequest
from botocore.credentials import Credentials
@ -493,18 +488,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
aws_region_name=self.s3_region_name,
)
# Prepare the URL
url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{batch_logging_element.s3_object_key}"
if self.s3_endpoint_url and self.s3_bucket_name:
if self.s3_use_virtual_hosted_style:
# Virtual-hosted-style: bucket.endpoint/key
endpoint_host: Final = self.s3_endpoint_url.replace("https://", "").replace("http://", "")
protocol: Final = "https://" if self.s3_endpoint_url.startswith("https://") else "http://"
url = f"{protocol}{self.s3_bucket_name}.{endpoint_host}/{batch_logging_element.s3_object_key}"
else:
# Path-style: endpoint/bucket/key
url = self.s3_endpoint_url + "/" + self.s3_bucket_name + "/" + batch_logging_element.s3_object_key
url: Final = self._build_object_url(batch_logging_element.s3_object_key)
# Convert JSON to string
json_string: Final = safe_dumps(batch_logging_element.payload)
@ -525,32 +509,22 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
"Cache-Control": "private, immutable, max-age=31536000, s-maxage=0",
**self._sse_headers(),
}
req: Final = requests.Request("PUT", url, data=json_string, headers=headers)
prepped: Final = req.prepare()
# Sign the request
aws_request: Final = AWSRequest(
method=prepped.method,
url=prepped.url,
data=prepped.body,
headers=prepped.headers,
)
aws_request: Final = AWSRequest(method="PUT", url=url, data=json_string, headers=headers)
aws_region_name: Final = self.get_aws_region_name_for_non_llm_api_calls(aws_region_name=self.s3_region_name)
S3SigV4Auth(credentials, "s3", aws_region_name).add_auth(aws_request)
# Prepare the signed headers
signed_headers: Final = dict(aws_request.headers.items())
# Use prepared URL so path segments match SigV4 canonical request (e.g. %20 for spaces).
request_url: Final = prepped.url or url
httpx_client: Final = _get_httpx_client(
params=({"ssl_verify": self.s3_verify} if self.s3_verify is not None else None)
)
# Make the request with retry for transient S3 errors (500/503)
max_retries: Final = 3
for attempt in range(max_retries):
response = httpx_client.put(request_url, data=json_string, headers=signed_headers)
response = httpx_client.put(url, data=json_string, headers=signed_headers)
if response.status_code in (500, 503) and attempt < max_retries - 1:
wait_time = 2**attempt # 1s, 2s
verbose_logger.warning(
@ -582,7 +556,6 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
try:
import hashlib
import requests
from botocore.auth import S3SigV4Auth
from botocore.awsrequest import AWSRequest
except ImportError:
@ -607,18 +580,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
verbose_logger.debug("s3_v2 logger - downloading data from s3 - %s", s3_object_key)
# Prepare the URL
url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{s3_object_key}"
if self.s3_endpoint_url and self.s3_bucket_name:
if self.s3_use_virtual_hosted_style:
# Virtual-hosted-style: bucket.endpoint/key
endpoint_host: Final = self.s3_endpoint_url.replace("https://", "").replace("http://", "")
protocol: Final = "https://" if self.s3_endpoint_url.startswith("https://") else "http://"
url = f"{protocol}{self.s3_bucket_name}.{endpoint_host}/{s3_object_key}"
else:
# Path-style: endpoint/bucket/key
url = self.s3_endpoint_url + "/" + self.s3_bucket_name + "/" + s3_object_key
url: Final = self._build_object_url(s3_object_key)
# Prepare the request for GET operation
# For GET requests, we need x-amz-content-sha256 with hash of empty string
@ -626,22 +588,15 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
headers: Final = {
"x-amz-content-sha256": empty_string_hash,
}
req: Final = requests.Request("GET", url, headers=headers)
prepped: Final = req.prepare()
# Sign the request
aws_request: Final = AWSRequest(
method=prepped.method,
url=prepped.url,
headers=prepped.headers,
)
aws_request: Final = AWSRequest(method="GET", url=url, headers=headers)
S3SigV4Auth(credentials, "s3", self.s3_region_name).add_auth(aws_request)
# Prepare the signed headers
signed_headers: Final = dict(aws_request.headers.items())
request_url: Final = prepped.url or url
response: Final = await self.async_httpx_client.get(request_url, headers=signed_headers)
response: Final = await self.async_httpx_client.get(url, headers=signed_headers)
if response.status_code != 200:
verbose_logger.exception("S3 object not found, saw response=", response.text)

116
tests/test_litellm/integrations/test_s3_v2.py
View file

@ -1647,15 +1647,27 @@ def _signature_for(signer_cls, url: str, method: str, body: bytes | None, header
return signer.signature(signer.string_to_sign(request, canonical_request), request)
def _as_s3_canonicalizes(url: str) -> str:
"""
The path S3 rebuilds from the wire path: percent-encode everything outside the unreserved
set, without normalizing or double-encoding. `=` becomes `%3D`, `%20` stays `%20`.
"""
from urllib.parse import quote, unquote, urlsplit, urlunsplit
split = urlsplit(url)
return urlunsplit(split._replace(path=quote(unquote(split.path), safe="/~")))
def _assert_signed_for_s3_canonicalization(url: str, method: str, body: bytes | None, headers: dict[str, str]) -> None:
"""
S3 rebuilds the canonical request from the wire path with single percent-encoding, which
botocore models as S3SigV4Auth; plain SigV4Auth double-encodes it (%2520 for a space) and S3
answers 403 SignatureDoesNotMatch. Assert we signed the path the way S3 reads it.
answers 403 SignatureDoesNotMatch. Assert we sent an already-encoded path and signed it the
way S3 reads it.
"""
from botocore.auth import S3SigV4Auth, SigV4Auth
assert "%20" in url
assert url == _as_s3_canonicalizes(url)
sent_signature = headers["Authorization"].split("Signature=")[1].strip()
assert sent_signature == _signature_for(S3SigV4Auth, url, method, body, headers)
assert sent_signature != _signature_for(SigV4Auth, url, method, body, headers)
@ -1744,3 +1756,103 @@ async def test_download_signs_object_key_with_space_the_way_s3_does():
body=None,
headers=call.kwargs["headers"],
)
_RESERVED_CHAR_KEYS = (
"2026-08-21/time-05-29-36_resp_bGl0ZWxsbTpjdXN0b20=.json",
"session=logs/2026-08-21/time-05-29-36_abc.json",
"a+b/2026-08-21/time-05-29-36_abc.json",
"a&b/2026-08-21/time-05-29-36_abc.json",
"a#b/2026-08-21/time-05-29-36_abc.json",
"a?b/2026-08-21/time-05-29-36_abc.json",
"a%b/2026-08-21/time-05-29-36_abc.json",
_KEY_WITH_SPACE,
)
def _element_for(s3_object_key: str):
from litellm.types.integrations.s3_v2 import s3BatchLoggingElement
return s3BatchLoggingElement(
s3_object_key=s3_object_key,
payload={"test": "sigv4"},
s3_object_download_filename="log.json",
)
def _expected_wire_url(s3_object_key: str) -> str:
"""The URL boto3 itself would put on the wire for this key."""
from urllib.parse import quote
return f"https://logs-bucket.s3.us-east-1.amazonaws.com/{quote(s3_object_key, safe='/')}"
@pytest.mark.parametrize("s3_object_key", _RESERVED_CHAR_KEYS)
@pytest.mark.asyncio
async def test_async_upload_percent_encodes_reserved_characters_in_object_key(s3_object_key):
from unittest.mock import AsyncMock, MagicMock
logger = _logger_for_signing()
response = MagicMock()
response.status_code = 200
response.raise_for_status = MagicMock()
logger.async_httpx_client = AsyncMock()
logger.async_httpx_client.put.return_value = response
await logger.async_upload_data_to_s3(_element_for(s3_object_key))
call = logger.async_httpx_client.put.call_args
assert call[0][0] == _expected_wire_url(s3_object_key)
_assert_signed_for_s3_canonicalization(
url=call[0][0],
method="PUT",
body=call.kwargs["data"].encode("utf-8"),
headers=call.kwargs["headers"],
)
@pytest.mark.parametrize("s3_object_key", _RESERVED_CHAR_KEYS)
def test_sync_upload_percent_encodes_reserved_characters_in_object_key(s3_object_key):
from unittest.mock import MagicMock
logger = _logger_for_signing()
response = MagicMock()
response.status_code = 200
response.raise_for_status = MagicMock()
mock_sync_client = MagicMock()
mock_sync_client.put.return_value = response
with patch("litellm.integrations.s3_v2._get_httpx_client", return_value=mock_sync_client):
logger.upload_data_to_s3(_element_for(s3_object_key))
call = mock_sync_client.put.call_args
assert call[0][0] == _expected_wire_url(s3_object_key)
_assert_signed_for_s3_canonicalization(
url=call[0][0],
method="PUT",
body=call.kwargs["data"].encode("utf-8"),
headers=call.kwargs["headers"],
)
@pytest.mark.parametrize("s3_object_key", _RESERVED_CHAR_KEYS)
@pytest.mark.asyncio
async def test_download_percent_encodes_reserved_characters_in_object_key(s3_object_key):
from unittest.mock import AsyncMock, MagicMock
logger = _logger_for_signing()
response = MagicMock()
response.status_code = 200
response.json = MagicMock(return_value={"downloaded": "data"})
logger.async_httpx_client = AsyncMock()
logger.async_httpx_client.get.return_value = response
assert await logger._download_object_from_s3(s3_object_key) == {"downloaded": "data"}
call = logger.async_httpx_client.get.call_args
assert call[0][0] == _expected_wire_url(s3_object_key)
_assert_signed_for_s3_canonicalization(
url=call[0][0],
method="GET",
body=None,
headers=call.kwargs["headers"],
)