From 1d407c2f26d7587bf184f7cf19dfcaede7e860d7 Mon Sep 17 00:00:00 2001 From: Kent Date: Fri, 26 Jun 2026 17:24:21 +0800 Subject: [PATCH 1/6] fix(bedrock): validate file-content retrieval against the configured output bucket Bedrock batch jobs write their results to s3_output_bucket_name when it differs from the input bucket, but the file-content retrieval path validated the file id only against the input bucket (s3_bucket_name). A deployment that configures a separate output bucket therefore could not retrieve its own batch outputs: the id validated against the input bucket and was rejected as a foreign bucket. Resolve the trusted output bucket alongside the input bucket from the immutable credential snapshot (or AWS_S3_OUTPUT_BUCKET_NAME), and try the file id against each configured bucket, returning the first that validates. The SSRF guard is preserved: only server-configured buckets are tried, never a request param, and an id outside both is still rejected. --- litellm/llms/bedrock/files/transformation.py | 67 +++++++++++-- .../test_bedrock_files_transformation.py | 94 +++++++++++++++++++ 2 files changed, 151 insertions(+), 10 deletions(-) diff --git a/litellm/llms/bedrock/files/transformation.py b/litellm/llms/bedrock/files/transformation.py index 6cfaa88275d..df06c333d1f 100644 --- a/litellm/llms/bedrock/files/transformation.py +++ b/litellm/llms/bedrock/files/transformation.py @@ -81,11 +81,12 @@ class _BedrockS3RequestParams(BaseModel): class _TrustedS3ModelCredentials(BaseModel): - """The S3 bucket the server trusts file ids against, from the deployment snapshot.""" + """The S3 buckets the server trusts file ids against, from the deployment snapshot.""" model_config = ConfigDict(extra="ignore") s3_bucket_name: str | None = None + s3_output_bucket_name: str | None = None def extract_s3_uri_from_file_id(file_id: str) -> str: @@ -135,6 +136,41 @@ def get_configured_s3_bucket_name(litellm_params: Mapping[str, object]) -> str: return bucket_name +def get_configured_s3_bucket_names( + litellm_params: Mapping[str, object], +) -> tuple[str, ...]: + """ + Resolve the server-configured S3 buckets a Bedrock file id may live in. + + Bedrock batch outputs land in ``s3_output_bucket_name`` when it differs from + the input bucket, so retrieval validates against both. Same trust rules as + ``get_configured_s3_bucket_name``: only the immutable credential snapshot or + the environment, never a request param. + """ + trusted_model_credentials = litellm_params.get( + "_litellm_internal_model_credentials" + ) + input_bucket: str | None = None + output_bucket: str | None = None + if isinstance(trusted_model_credentials, MappingProxyType): + snapshot: dict[str, object] = {} + snapshot.update(trusted_model_credentials) # any-ok: untyped snapshot + trusted = _TrustedS3ModelCredentials.model_validate(snapshot) + input_bucket = trusted.s3_bucket_name + output_bucket = trusted.s3_output_bucket_name + input_bucket = input_bucket or os.getenv("AWS_S3_BUCKET_NAME") + output_bucket = output_bucket or os.getenv("AWS_S3_OUTPUT_BUCKET_NAME") + + buckets = tuple( + dict.fromkeys(bucket for bucket in (input_bucket, output_bucket) if bucket) + ) + if not buckets: + raise ValueError( + "S3 bucket_name is required. Set 's3_bucket_name' in proxy config or AWS_S3_BUCKET_NAME for Bedrock file content retrieval." + ) + return buckets + + class BedrockFilesConfig(BaseAWSLLM, BaseFilesConfig): """ Config for Bedrock Files - handles S3 uploads for Bedrock batch processing @@ -1042,15 +1078,26 @@ class BedrockFilesConfig(BaseAWSLLM, BaseFilesConfig): raise ValueError("file_id is required for Bedrock file content retrieval") s3_uri = extract_s3_uri_from_file_id(file_id) - bucket_name, object_key = validate_managed_cloud_file_id( - file_id=s3_uri, - scheme="s3://", - configured_bucket_name=get_configured_s3_bucket_name(litellm_params), - allowed_object_prefixes=BEDROCK_MANAGED_S3_PREFIXES, - allow_legacy_cloud_file_ids=should_allow_legacy_cloud_file_ids( - litellm_params - ), - ) + allow_legacy = should_allow_legacy_cloud_file_ids(litellm_params) + last_error: ValueError | None = None + bucket_name: str | None = None + object_key: str | None = None + for configured_bucket in get_configured_s3_bucket_names(litellm_params): + try: + bucket_name, object_key = validate_managed_cloud_file_id( + file_id=s3_uri, + scheme="s3://", + configured_bucket_name=configured_bucket, + allowed_object_prefixes=BEDROCK_MANAGED_S3_PREFIXES, + allow_legacy_cloud_file_ids=allow_legacy, + ) + break + except ValueError as e: + last_error = e + if bucket_name is None or object_key is None: + raise last_error or ValueError( + "file_id must reference a LiteLLM-managed storage object" + ) # The shared file-content handler passes optional_params={}, so AWS # credentials/region arrive via litellm_params here (unlike the upload 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 c548fe53e15..dd111969555 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 @@ -1308,6 +1308,100 @@ class TestBedrockFileContentTransformation: litellm_params=self._litellm_params(), ) + def _trusted(self, **creds) -> dict: + from types import MappingProxyType + + params = self._litellm_params() + params["_litellm_internal_model_credentials"] = MappingProxyType(dict(creds)) + return params + + def test_retrieves_from_distinct_output_bucket(self, monkeypatch): + """Batch outputs can land in a separate s3_output_bucket_name. Retrieval + must validate the file id against the output bucket too, not just the + input bucket, or the very outputs the feature serves are unreachable.""" + 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) + + url, _ = BedrockFilesConfig().transform_file_content_request( + file_content_request={ + "file_id": "s3://out-bucket/litellm-batch-outputs/job/in.jsonl.out" + }, + optional_params={}, + litellm_params=self._trusted( + s3_bucket_name="in-bucket", s3_output_bucket_name="out-bucket" + ), + ) + + assert ( + url + == "https://s3.us-west-2.amazonaws.com/out-bucket/litellm-batch-outputs/job/in.jsonl.out" + ) + + def test_output_bucket_falls_back_to_env(self, monkeypatch): + """The output bucket resolves from AWS_S3_OUTPUT_BUCKET_NAME when not in + the trusted snapshot, mirroring the input-bucket env fallback.""" + from litellm.llms.bedrock.files.transformation import BedrockFilesConfig + + monkeypatch.setenv("AWS_S3_BUCKET_NAME", "in-bucket") + monkeypatch.setenv("AWS_S3_OUTPUT_BUCKET_NAME", "env-out-bucket") + + url, _ = BedrockFilesConfig().transform_file_content_request( + file_content_request={ + "file_id": "s3://env-out-bucket/litellm-batch-outputs/job/in.jsonl.out" + }, + optional_params={}, + litellm_params=self._litellm_params(), + ) + + assert ( + url + == "https://s3.us-west-2.amazonaws.com/env-out-bucket/litellm-batch-outputs/job/in.jsonl.out" + ) + + def test_input_bucket_still_validates_when_output_bucket_set(self, monkeypatch): + """Adding output-bucket support must not break retrieval of input-bucket + objects when both buckets are configured.""" + 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) + + url, _ = BedrockFilesConfig().transform_file_content_request( + file_content_request={ + "file_id": "s3://in-bucket/litellm-batch-outputs/job/in.jsonl.out" + }, + optional_params={}, + litellm_params=self._trusted( + s3_bucket_name="in-bucket", s3_output_bucket_name="out-bucket" + ), + ) + + assert ( + url + == "https://s3.us-west-2.amazonaws.com/in-bucket/litellm-batch-outputs/job/in.jsonl.out" + ) + + 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.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"): + BedrockFilesConfig().transform_file_content_request( + file_content_request={ + "file_id": "s3://other-bucket/litellm-batch-outputs/job/x.jsonl.out" + }, + optional_params={}, + litellm_params=self._trusted( + s3_bucket_name="in-bucket", s3_output_bucket_name="out-bucket" + ), + ) + 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.""" From a9a322d63f26e48a517d84ad428e77fcb4cbca03 Mon Sep 17 00:00:00 2001 From: Kent Date: Fri, 26 Jun 2026 19:03:10 +0800 Subject: [PATCH 2/6] fix(router): keep s3_output_bucket_name in the trusted credential snapshot The output-bucket retrieval fix only worked via the AWS_S3_OUTPUT_BUCKET_NAME env var, never via per-model s3_output_bucket_name config. The proxy builds the trusted snapshot that retrieval validates against by round-tripping a deployment's litellm_params through CredentialLiteLLMParams in get_deployment_credentials_with_provider, and that strict allowlist did not declare s3_output_bucket_name, so the field was silently dropped before retrieval saw it (same trap as azure_ad_token in #30235). The snapshot branch of get_configured_s3_bucket_names was therefore dead in the model-routing path and output-bucket file ids were rejected as foreign. Declaring s3_output_bucket_name on CredentialLiteLLMParams lets it survive into the snapshot, so the existing multi-bucket validation works for per-model output buckets too. The PR's tests injected the field straight into the MappingProxyType, bypassing this filter, so they passed despite the live gap. _trusted now builds the snapshot through CredentialLiteLLMParams the way the proxy does, and a router-level regression test pins that get_deployment_credentials_with_provider preserves the output bucket. Both fail without this change. --- litellm/types/router.py | 6 ++ .../test_bedrock_files_transformation.py | 19 +++++- ...st_azure_ad_token_credential_resolution.py | 68 +++++++++++++++++++ 3 files changed, 90 insertions(+), 3 deletions(-) diff --git a/litellm/types/router.py b/litellm/types/router.py index a1c571ed7f7..333ec6d9b05 100644 --- a/litellm/types/router.py +++ b/litellm/types/router.py @@ -188,6 +188,12 @@ class CredentialLiteLLMParams(BaseModel): aws_bedrock_runtime_endpoint: Optional[str] = None aws_bedrock_project_id: Optional[str] = None s3_bucket_name: Optional[str] = None + # Like the fields above, must be declared here or the strict dump in + # ``get_deployment_credentials_with_provider`` drops it from the trusted + # snapshot, so per-model output-bucket config never reaches Bedrock + # file-content retrieval and output-bucket file ids are wrongly rejected + # (#26335). + s3_output_bucket_name: Optional[str] = None ## IBM WATSONX ## watsonx_region_name: Optional[str] = None 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 dd111969555..448132f20b1 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 @@ -1308,17 +1308,30 @@ class TestBedrockFileContentTransformation: litellm_params=self._litellm_params(), ) - def _trusted(self, **creds) -> dict: + def _trusted(self, **deployment_litellm_params) -> dict: + """Build the trusted snapshot the way the proxy does: deployment + litellm_params funneled through ``CredentialLiteLLMParams`` (the strict + allowlist ``get_deployment_credentials_with_provider`` applies) before + retrieval ever sees them. Injecting a raw ``MappingProxyType`` would + bypass that filter and hide whether a bucket field actually survives + into the snapshot in production.""" from types import MappingProxyType + from litellm.types.router import CredentialLiteLLMParams + + snapshot = CredentialLiteLLMParams(**deployment_litellm_params).model_dump( + exclude_none=True + ) params = self._litellm_params() - params["_litellm_internal_model_credentials"] = MappingProxyType(dict(creds)) + params["_litellm_internal_model_credentials"] = MappingProxyType(snapshot) return params def test_retrieves_from_distinct_output_bucket(self, monkeypatch): """Batch outputs can land in a separate s3_output_bucket_name. Retrieval must validate the file id against the output bucket too, not just the - input bucket, or the very outputs the feature serves are unreachable.""" + input bucket, or the very outputs the feature serves are unreachable. + The snapshot is built through the production credential filter, so this + fails if s3_output_bucket_name is dropped from that allowlist.""" from litellm.llms.bedrock.files.transformation import BedrockFilesConfig monkeypatch.delenv("AWS_S3_BUCKET_NAME", raising=False) diff --git a/tests/test_litellm/test_azure_ad_token_credential_resolution.py b/tests/test_litellm/test_azure_ad_token_credential_resolution.py index 958b236c9b3..f6f47b3f4c4 100644 --- a/tests/test_litellm/test_azure_ad_token_credential_resolution.py +++ b/tests/test_litellm/test_azure_ad_token_credential_resolution.py @@ -143,3 +143,71 @@ class TestRouterCredentialResolution: assert credentials is not None assert credentials.get("api_key") == "sk-static-key" assert "azure_ad_token" not in credentials + + +class TestRouterCredentialResolutionS3OutputBucket: + """Same strict-dump trap as azure_ad_token (#30235), for Bedrock batch + file retrieval (#26335). Bedrock batch outputs land in a per-model + ``s3_output_bucket_name`` when it differs from the input bucket. The + file-content retrieval path validates a file id against the buckets in the + trusted credential snapshot, and that snapshot is built by round-tripping + the deployment's ``litellm_params`` through ``CredentialLiteLLMParams``. If + the field is undeclared it is dropped, so the output bucket never reaches + retrieval and output-bucket file ids are rejected as foreign.""" + + def test_credentials_preserve_s3_output_bucket_name(self): + from litellm import Router + + deployment_id = "bedrock-batch-output-bucket-fixed-uuid" + router = Router( + model_list=[ + { + "model_name": "bedrock-batch", + "litellm_params": { + "model": "bedrock/anthropic.claude-3-sonnet-20240229-v1:0", + "s3_bucket_name": "in-bucket", + "s3_output_bucket_name": "out-bucket", + "aws_region_name": "us-west-2", + }, + "model_info": {"id": deployment_id}, + } + ] + ) + + credentials = router.get_deployment_credentials_with_provider( + model_id=deployment_id + ) + assert credentials is not None + assert credentials.get("s3_output_bucket_name") == "out-bucket", ( + "Router credential resolution dropped s3_output_bucket_name; " + "Bedrock batch file-content retrieval will reject output-bucket " + "file ids as foreign for model-routed deployments (#26335)" + ) + assert credentials.get("s3_bucket_name") == "in-bucket" + + def test_credentials_without_output_bucket_unaffected(self): + """A deployment that configures only the input bucket keeps it and does + not gain a phantom output bucket in the resolved credentials.""" + from litellm import Router + + deployment_id = "bedrock-batch-input-only-fixed-uuid" + router = Router( + model_list=[ + { + "model_name": "bedrock-batch-input-only", + "litellm_params": { + "model": "bedrock/anthropic.claude-3-sonnet-20240229-v1:0", + "s3_bucket_name": "in-bucket", + "aws_region_name": "us-west-2", + }, + "model_info": {"id": deployment_id}, + } + ] + ) + + credentials = router.get_deployment_credentials_with_provider( + model_id=deployment_id + ) + assert credentials is not None + assert credentials.get("s3_bucket_name") == "in-bucket" + assert "s3_output_bucket_name" not in credentials From 6e6a508e0e46da5af34b9ccaa89602a2acf2adc1 Mon Sep 17 00:00:00 2001 From: Kent Date: Fri, 26 Jun 2026 19:18:09 +0800 Subject: [PATCH 3/6] chore(ui): regenerate schema.d.ts for s3_output_bucket_name Adding s3_output_bucket_name to CredentialLiteLLMParams changes the proxy OpenAPI spec, so the generated dashboard types need regenerating to match (Check UI API Types Sync). --- ui/litellm-dashboard/src/lib/http/schema.d.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index b53acf930f2..f4c124659a4 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -25410,6 +25410,8 @@ export interface components { s3_bucket_name?: string | null; /** S3 Encryption Key Id */ s3_encryption_key_id?: string | null; + /** S3 Output Bucket Name */ + s3_output_bucket_name?: string | null; /** Search Context Cost Per Query */ search_context_cost_per_query?: { [key: string]: unknown; @@ -33118,6 +33120,8 @@ export interface components { s3_bucket_name?: string | null; /** S3 Encryption Key Id */ s3_encryption_key_id?: string | null; + /** S3 Output Bucket Name */ + s3_output_bucket_name?: string | null; /** Search Context Cost Per Query */ search_context_cost_per_query?: { [key: string]: unknown; From 6a940ef3f4c3a6c89698d5fb082bd3a99c4841bd Mon Sep 17 00:00:00 2001 From: Kent Date: Tue, 30 Jun 2026 02:10:44 +0800 Subject: [PATCH 4/6] chore(types): type the dict-shim helpers to offset the budget gate Adding s3_output_bucket_name to CredentialLiteLLMParams adds one reportUnknownArgumentType error at each untyped **kwargs construction site of GenericLiteLLMParams repo-wide (~114 sites), which pushed the basedpyright budget just over its ceiling. Typing the key parameter of the get/__getitem__/ __setitem__/__contains__ dict-shim helpers on ModelInfo, GenericLiteLLMParams, LiteLLM_Params, and Deployment removes the unknown-argument errors at the getattr/setattr/hasattr calls in those bodies, bringing the repo total back under the cap without raising any other rule. --- litellm/types/router.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/litellm/types/router.py b/litellm/types/router.py index 87e3364ac7e..75370d94895 100644 --- a/litellm/types/router.py +++ b/litellm/types/router.py @@ -154,19 +154,19 @@ class ModelInfo(BaseModel): model_config = ConfigDict(extra="allow") - def __contains__(self, key): + def __contains__(self, key: str) -> bool: # Define custom behavior for the 'in' operator return hasattr(self, key) - def get(self, key, default=None): + def get(self, key: str, default=None): # Custom .get() method to access attributes with a default value if the attribute doesn't exist return getattr(self, key, default) - def __getitem__(self, key): + def __getitem__(self, key: str): # Allow dictionary-style access to attributes return getattr(self, key) - def __setitem__(self, key, value): + def __setitem__(self, key: str, value) -> None: # Allow dictionary-style assignment of attributes setattr(self, key, value) @@ -303,19 +303,19 @@ class GenericLiteLLMParams(CredentialLiteLLMParams, CustomPricingLiteLLMParams): return filtered return data - def __contains__(self, key): + def __contains__(self, key: str) -> bool: # Define custom behavior for the 'in' operator return hasattr(self, key) - def get(self, key, default=None): + def get(self, key: str, default=None): # Custom .get() method to access attributes with a default value if the attribute doesn't exist return getattr(self, key, default) - def __getitem__(self, key): + def __getitem__(self, key: str): # Allow dictionary-style access to attributes return getattr(self, key) - def __setitem__(self, key, value): + def __setitem__(self, key: str, value) -> None: # Allow dictionary-style assignment of attributes setattr(self, key, value) @@ -328,19 +328,19 @@ class LiteLLM_Params(GenericLiteLLMParams): model: str model_config = ConfigDict(extra="allow", arbitrary_types_allowed=True) - def __contains__(self, key): + def __contains__(self, key: str) -> bool: # Define custom behavior for the 'in' operator return hasattr(self, key) - def get(self, key, default=None): + def get(self, key: str, default=None): # Custom .get() method to access attributes with a default value if the attribute doesn't exist return getattr(self, key, default) - def __getitem__(self, key): + def __getitem__(self, key: str): # Allow dictionary-style access to attributes return getattr(self, key) - def __setitem__(self, key, value): + def __setitem__(self, key: str, value) -> None: # Allow dictionary-style assignment of attributes setattr(self, key, value) @@ -473,19 +473,19 @@ class Deployment(BaseModel): # if using pydantic v1 return self.dict(**kwargs) - def __contains__(self, key): + def __contains__(self, key: str) -> bool: # Define custom behavior for the 'in' operator return hasattr(self, key) - def get(self, key, default=None): + def get(self, key: str, default=None): # Custom .get() method to access attributes with a default value if the attribute doesn't exist return getattr(self, key, default) - def __getitem__(self, key): + def __getitem__(self, key: str): # Allow dictionary-style access to attributes return getattr(self, key) - def __setitem__(self, key, value): + def __setitem__(self, key: str, value) -> None: # Allow dictionary-style assignment of attributes setattr(self, key, value) From 7bea4def27cb8c05833afd2944248f844ed18ca0 Mon Sep 17 00:00:00 2001 From: Kent Date: Fri, 26 Jun 2026 17:24:21 +0800 Subject: [PATCH 5/6] fix(bedrock): validate file-content retrieval against the configured output bucket Bedrock batch jobs write their results to s3_output_bucket_name when it differs from the input bucket, but the file-content retrieval path validated the file id only against the input bucket (s3_bucket_name). A deployment that configures a separate output bucket therefore could not retrieve its own batch outputs: the id validated against the input bucket and was rejected as a foreign bucket. Resolve the trusted output bucket alongside the input bucket from the immutable credential snapshot (or AWS_S3_OUTPUT_BUCKET_NAME), and try the file id against each configured bucket, returning the first that validates. The SSRF guard is preserved: only server-configured buckets are tried, never a request param, and an id outside both is still rejected. (cherry picked from commit 1d407c2f26d7587bf184f7cf19dfcaede7e860d7) --- litellm/llms/bedrock/files/transformation.py | 74 +++++++++++---- .../test_bedrock_files_transformation.py | 94 +++++++++++++++++++ 2 files changed, 152 insertions(+), 16 deletions(-) diff --git a/litellm/llms/bedrock/files/transformation.py b/litellm/llms/bedrock/files/transformation.py index 0f0a0f91024..b034696594a 100644 --- a/litellm/llms/bedrock/files/transformation.py +++ b/litellm/llms/bedrock/files/transformation.py @@ -3,6 +3,7 @@ import json import os import time from collections.abc import Iterable, Mapping, MutableMapping, Sequence +from contextlib import suppress from functools import cache from itertools import chain from types import MappingProxyType @@ -149,11 +150,12 @@ class _BedrockS3RequestParams(BaseModel): class _TrustedS3ModelCredentials(BaseModel): - """The S3 bucket the server trusts file ids against, from the deployment snapshot.""" + """The S3 buckets the server trusts file ids against, from the deployment snapshot.""" model_config = ConfigDict(extra="ignore") s3_bucket_name: str | None = None + s3_output_bucket_name: str | None = None def extract_s3_uri_from_file_id(file_id: str) -> str: @@ -179,6 +181,18 @@ def extract_s3_uri_from_file_id(file_id: str) -> str: raise ValueError("file_id must be a managed LiteLLM S3 file id") +_S3_BUCKET_REQUIRED_ERROR: Final = "S3 bucket_name is required. Set 's3_bucket_name' in proxy config or AWS_S3_BUCKET_NAME for Bedrock file content retrieval." + + +def _trusted_s3_model_credentials(litellm_params: Mapping[str, object]) -> _TrustedS3ModelCredentials: + trusted_model_credentials: Final = litellm_params.get("_litellm_internal_model_credentials") + if not isinstance(trusted_model_credentials, MappingProxyType): + return _TrustedS3ModelCredentials() + snapshot: Final[dict[str, object]] = {} + snapshot.update(trusted_model_credentials) # any-ok: untyped snapshot + return _TrustedS3ModelCredentials.model_validate(snapshot) + + def get_configured_s3_bucket_name(litellm_params: Mapping[str, object]) -> str: """ Resolve the server-configured S3 bucket for Bedrock file operations. @@ -187,20 +201,50 @@ def get_configured_s3_bucket_name(litellm_params: Mapping[str, object]) -> str: environment; never a request-supplied param, since the bucket is what `validate_managed_cloud_file_id` checks file ids against. """ - trusted_model_credentials: Final = litellm_params.get("_litellm_internal_model_credentials") - bucket_name: str | None = None - if isinstance(trusted_model_credentials, MappingProxyType): - snapshot: Final[dict[str, object]] = {} - snapshot.update(trusted_model_credentials) # any-ok: untyped snapshot - bucket_name = _TrustedS3ModelCredentials.model_validate(snapshot).s3_bucket_name - bucket_name = bucket_name or os.getenv("AWS_S3_BUCKET_NAME") + bucket_name: Final = _trusted_s3_model_credentials(litellm_params).s3_bucket_name or os.getenv("AWS_S3_BUCKET_NAME") if not bucket_name: - raise ValueError( - "S3 bucket_name is required. Set 's3_bucket_name' in proxy config or AWS_S3_BUCKET_NAME for Bedrock file content retrieval." - ) + raise ValueError(_S3_BUCKET_REQUIRED_ERROR) return bucket_name +def get_configured_s3_bucket_names(litellm_params: Mapping[str, object]) -> tuple[str, ...]: + """ + Resolve the server-configured S3 buckets a Bedrock file id may live in. + + Bedrock batch outputs land in ``s3_output_bucket_name`` when it differs from + the input bucket, so retrieval validates against both. Same trust rules as + ``get_configured_s3_bucket_name``: only the immutable credential snapshot or + the environment, never a request param. + """ + trusted: Final = _trusted_s3_model_credentials(litellm_params) + input_bucket: Final = trusted.s3_bucket_name or os.getenv("AWS_S3_BUCKET_NAME") + output_bucket: Final = trusted.s3_output_bucket_name or os.getenv("AWS_S3_OUTPUT_BUCKET_NAME") + buckets: Final = tuple(dict.fromkeys(bucket for bucket in (input_bucket, output_bucket) if bucket)) + if not buckets: + raise ValueError(_S3_BUCKET_REQUIRED_ERROR) + return buckets + + +def _validate_file_id_against_configured_buckets( + s3_uri: str, + configured_bucket_names: tuple[str, ...], + allow_legacy_cloud_file_ids: bool, +) -> tuple[str, str]: + def validate_against(configured_bucket_name: str) -> tuple[str, str]: + return validate_managed_cloud_file_id( + file_id=s3_uri, + scheme="s3://", + configured_bucket_name=configured_bucket_name, + allowed_object_prefixes=BEDROCK_MANAGED_S3_PREFIXES, + allow_legacy_cloud_file_ids=allow_legacy_cloud_file_ids, + ) + + for candidate_bucket_name in configured_bucket_names[:-1]: + with suppress(ValueError): + return validate_against(candidate_bucket_name) + return validate_against(configured_bucket_names[-1]) + + def _uploaded_object_size(litellm_params: Mapping[str, object], raw_response: Response) -> int: """ S3 answers PutObject with an empty body, so the stored object size comes from the @@ -1186,11 +1230,9 @@ class BedrockFilesConfig(BaseAWSLLM, BaseFilesConfig): 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_managed_cloud_file_id( - file_id=s3_uri, - scheme="s3://", - configured_bucket_name=get_configured_s3_bucket_name(litellm_params), - allowed_object_prefixes=BEDROCK_MANAGED_S3_PREFIXES, + 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), ) 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 305ce7139da..e297c92782b 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 @@ -1992,6 +1992,100 @@ class TestBedrockFileContentTransformation: litellm_params=self._litellm_params(), ) + def _trusted(self, **creds) -> dict: + from types import MappingProxyType + + params = self._litellm_params() + params["_litellm_internal_model_credentials"] = MappingProxyType(dict(creds)) + return params + + def test_retrieves_from_distinct_output_bucket(self, monkeypatch): + """Batch outputs can land in a separate s3_output_bucket_name. Retrieval + must validate the file id against the output bucket too, not just the + input bucket, or the very outputs the feature serves are unreachable.""" + 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) + + url, _ = BedrockFilesConfig().transform_file_content_request( + file_content_request={ + "file_id": "s3://out-bucket/litellm-batch-outputs/job/in.jsonl.out" + }, + optional_params={}, + litellm_params=self._trusted( + s3_bucket_name="in-bucket", s3_output_bucket_name="out-bucket" + ), + ) + + assert ( + url + == "https://s3.us-west-2.amazonaws.com/out-bucket/litellm-batch-outputs/job/in.jsonl.out" + ) + + def test_output_bucket_falls_back_to_env(self, monkeypatch): + """The output bucket resolves from AWS_S3_OUTPUT_BUCKET_NAME when not in + the trusted snapshot, mirroring the input-bucket env fallback.""" + from litellm.llms.bedrock.files.transformation import BedrockFilesConfig + + monkeypatch.setenv("AWS_S3_BUCKET_NAME", "in-bucket") + monkeypatch.setenv("AWS_S3_OUTPUT_BUCKET_NAME", "env-out-bucket") + + url, _ = BedrockFilesConfig().transform_file_content_request( + file_content_request={ + "file_id": "s3://env-out-bucket/litellm-batch-outputs/job/in.jsonl.out" + }, + optional_params={}, + litellm_params=self._litellm_params(), + ) + + assert ( + url + == "https://s3.us-west-2.amazonaws.com/env-out-bucket/litellm-batch-outputs/job/in.jsonl.out" + ) + + def test_input_bucket_still_validates_when_output_bucket_set(self, monkeypatch): + """Adding output-bucket support must not break retrieval of input-bucket + objects when both buckets are configured.""" + 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) + + url, _ = BedrockFilesConfig().transform_file_content_request( + file_content_request={ + "file_id": "s3://in-bucket/litellm-batch-outputs/job/in.jsonl.out" + }, + optional_params={}, + litellm_params=self._trusted( + s3_bucket_name="in-bucket", s3_output_bucket_name="out-bucket" + ), + ) + + assert ( + url + == "https://s3.us-west-2.amazonaws.com/in-bucket/litellm-batch-outputs/job/in.jsonl.out" + ) + + 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.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"): + BedrockFilesConfig().transform_file_content_request( + file_content_request={ + "file_id": "s3://other-bucket/litellm-batch-outputs/job/x.jsonl.out" + }, + optional_params={}, + litellm_params=self._trusted( + s3_bucket_name="in-bucket", s3_output_bucket_name="out-bucket" + ), + ) + 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.""" From 9ff9f771377d5ce0a47db8e979ab08d628375d25 Mon Sep 17 00:00:00 2001 From: Kent Date: Fri, 26 Jun 2026 19:03:10 +0800 Subject: [PATCH 6/6] test(router): cover s3_output_bucket_name surviving the trusted credential snapshot The field itself landed on staging via 0c5c9c79d7; these are the regression tests from PR #31435 for the retrieval-facing half. (cherry picked from commit a9a322d63f6d4658b1f28d1622335775e94736a4) --- .../test_bedrock_files_transformation.py | 19 +++++- ...st_azure_ad_token_credential_resolution.py | 68 +++++++++++++++++++ 2 files changed, 84 insertions(+), 3 deletions(-) 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 e297c92782b..da13f265ee4 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 @@ -1992,17 +1992,30 @@ class TestBedrockFileContentTransformation: litellm_params=self._litellm_params(), ) - def _trusted(self, **creds) -> dict: + def _trusted(self, **deployment_litellm_params) -> dict: + """Build the trusted snapshot the way the proxy does: deployment + litellm_params funneled through ``CredentialLiteLLMParams`` (the strict + allowlist ``get_deployment_credentials_with_provider`` applies) before + retrieval ever sees them. Injecting a raw ``MappingProxyType`` would + bypass that filter and hide whether a bucket field actually survives + into the snapshot in production.""" from types import MappingProxyType + from litellm.types.router import CredentialLiteLLMParams + + snapshot = CredentialLiteLLMParams(**deployment_litellm_params).model_dump( + exclude_none=True + ) params = self._litellm_params() - params["_litellm_internal_model_credentials"] = MappingProxyType(dict(creds)) + params["_litellm_internal_model_credentials"] = MappingProxyType(snapshot) return params def test_retrieves_from_distinct_output_bucket(self, monkeypatch): """Batch outputs can land in a separate s3_output_bucket_name. Retrieval must validate the file id against the output bucket too, not just the - input bucket, or the very outputs the feature serves are unreachable.""" + input bucket, or the very outputs the feature serves are unreachable. + The snapshot is built through the production credential filter, so this + fails if s3_output_bucket_name is dropped from that allowlist.""" from litellm.llms.bedrock.files.transformation import BedrockFilesConfig monkeypatch.delenv("AWS_S3_BUCKET_NAME", raising=False) diff --git a/tests/test_litellm/test_azure_ad_token_credential_resolution.py b/tests/test_litellm/test_azure_ad_token_credential_resolution.py index 958b236c9b3..f6f47b3f4c4 100644 --- a/tests/test_litellm/test_azure_ad_token_credential_resolution.py +++ b/tests/test_litellm/test_azure_ad_token_credential_resolution.py @@ -143,3 +143,71 @@ class TestRouterCredentialResolution: assert credentials is not None assert credentials.get("api_key") == "sk-static-key" assert "azure_ad_token" not in credentials + + +class TestRouterCredentialResolutionS3OutputBucket: + """Same strict-dump trap as azure_ad_token (#30235), for Bedrock batch + file retrieval (#26335). Bedrock batch outputs land in a per-model + ``s3_output_bucket_name`` when it differs from the input bucket. The + file-content retrieval path validates a file id against the buckets in the + trusted credential snapshot, and that snapshot is built by round-tripping + the deployment's ``litellm_params`` through ``CredentialLiteLLMParams``. If + the field is undeclared it is dropped, so the output bucket never reaches + retrieval and output-bucket file ids are rejected as foreign.""" + + def test_credentials_preserve_s3_output_bucket_name(self): + from litellm import Router + + deployment_id = "bedrock-batch-output-bucket-fixed-uuid" + router = Router( + model_list=[ + { + "model_name": "bedrock-batch", + "litellm_params": { + "model": "bedrock/anthropic.claude-3-sonnet-20240229-v1:0", + "s3_bucket_name": "in-bucket", + "s3_output_bucket_name": "out-bucket", + "aws_region_name": "us-west-2", + }, + "model_info": {"id": deployment_id}, + } + ] + ) + + credentials = router.get_deployment_credentials_with_provider( + model_id=deployment_id + ) + assert credentials is not None + assert credentials.get("s3_output_bucket_name") == "out-bucket", ( + "Router credential resolution dropped s3_output_bucket_name; " + "Bedrock batch file-content retrieval will reject output-bucket " + "file ids as foreign for model-routed deployments (#26335)" + ) + assert credentials.get("s3_bucket_name") == "in-bucket" + + def test_credentials_without_output_bucket_unaffected(self): + """A deployment that configures only the input bucket keeps it and does + not gain a phantom output bucket in the resolved credentials.""" + from litellm import Router + + deployment_id = "bedrock-batch-input-only-fixed-uuid" + router = Router( + model_list=[ + { + "model_name": "bedrock-batch-input-only", + "litellm_params": { + "model": "bedrock/anthropic.claude-3-sonnet-20240229-v1:0", + "s3_bucket_name": "in-bucket", + "aws_region_name": "us-west-2", + }, + "model_info": {"id": deployment_id}, + } + ] + ) + + credentials = router.get_deployment_credentials_with_provider( + model_id=deployment_id + ) + assert credentials is not None + assert credentials.get("s3_bucket_name") == "in-bucket" + assert "s3_output_bucket_name" not in credentials