fix(bedrock): use deployment credentials for AWS requests - #36160
Conversation
Greptile SummaryThe PR routes Bedrock file and batch requests through deployment AWS credentials and preserves the complete AWS authentication configuration during router credential resolution
Confidence Score: 5/5The PR appears safe to merge No blocking failure remains
|
| Filename | Overview |
|---|---|
| litellm/llms/bedrock/common_utils.py | Adds centralized merging of deployment and request AWS parameters with deployment identity precedence |
| litellm/llms/bedrock/files/transformation.py | Uses merged deployment parameters for S3 endpoint selection, region selection, encryption resolution, and request signing |
| litellm/llms/bedrock/batches/transformation.py | Uses merged deployment parameters for batch endpoint construction and create or retrieve signing |
| litellm/proxy/auth/auth_utils.py | Expands blocked client-side AWS identity selectors unless the proxy-wide credential opt-in is enabled |
| litellm/types/router.py | Preserves additional AWS authentication fields in router credential models |
| ui/litellm-dashboard/src/lib/http/schema.d.ts | Regenerates dashboard API declarations for the expanded AWS credential schema |
Reviews (5): Last reviewed commit: "fix(proxy): ban caller-supplied aws iden..." | Re-trigger Greptile
|
|
||
| monkeypatch.setattr(config, "_sign_s3_request", capture_signing) | ||
|
|
||
| result = config.transform_create_file_request( |
There was a problem hiding this comment.
Tests monkeypatch signing internals
These tests replace signing methods on configuration objects instead of injecting mock dependencies, coupling the regression coverage to internal object structure and making refactors brittle. The same pattern occurs on lines 48 and 77.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
|
|
bugbot run |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1b488f7. Configure here.
mateo-berri
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution!
c4656d8
into
BerriAI:litellm_internal_staging
TLDR
Problem this solves:
How it solves it:
s3_region_nameauthoritative so SigV4 matches the S3 URL regionUser Flow
Before: a batch developer's upload dies because the gateway signs with the server's ambient AWS identity instead of the deployment's
bedrock/us.anthropic.claude-haiku-4-5-20251001-v1:0deployment whoselitellm_paramscarry the deployment's own AWS key, secret, session token, regionus-west-2, an SSE-KMS-locked S3 bucket, and a batch role ARNpurpose=batchand a JSONL of chat requests"Unable to locate credentials": the deployment's keys were never consulted and the server itself holds noneinput_file_idfails the same way, so no batch job is ever created"aws_profile_name"to a plain POST http://localhost:PORT/v1/chat/completions body and the gateway signs as that profile on the host rather than the deployment's own identity, and naming a profile that does not exist returns"The config profile (X) could not be found", so the host's profile names can be probed one guess at a timeAfter: the same requests succeed because the deployment's own identity signs every call
bedrock/us.anthropic.claude-haiku-4-5-20251001-v1:0deployment with the samelitellm_paramspurpose=batchfile-...id and the object lands in the deployment's SSE-KMS bucketbatch_...id whose decoded ARN names the deployment's own account andus-west-2validatingthen in progress, read back with the deployment's credentials"aws_profile_name"to any of those bodies, or to a chat completion, or nesting it underextra_body, now comes back refused with"aws_profile_name is not allowed in request body"before the request reaches AWS, so the developer can no longer choose the signing identity or probe the host's profile names, and only a proxy admin can re-enable itRelevant issues
Fixes #36155
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Live proxies against real AWS (account 439158074652, us-west-2), real Bedrock batch jobs, no mocks. Every proxy booted with zero ambient AWS identity: no
AWS_*credential env vars in the process,AWS_CONFIG_FILEandAWS_SHARED_CREDENTIALS_FILEpointed at files holding no[default]section, instance metadata disabled, and.envloading suppressed. So the deployment'slitellm_paramswere the only possible credential source: temporary STS credentials (key, secret, session token),s3_bucket_nameon a bucket whose policy denies PutObject without SSE-KMS under one specific key,s3_encryption_key_id, andaws_batch_role_arn. Three commits, each in its own worktree and venv on its own port: the merge base79d412efc2, this branch's pre-hardening commitb5eed5e526, and the tip1b488f7c2f1. The reported symptom: dead at the merge base, working at the tip
Before, at the merge base
79d412efc2:The upload produced no id at all, so the batch create above deliberately carries a bogus
input_file_idto show the same credential failure on that route; it dies before the id is ever validated. The proxy's own debug line for those requests readsaws_access_key_id=[set=False] aws_secret_access_key=[set=False] aws_session_token=[set=False]whileaws_region_name=us-west-2is set, so the deployment's params reach credential resolution but its credentials do not. A chat completion on that same proxy process, same alias params, returned 200, which rules out bad QA credentialsAfter, at the tip
1b488f7c2f:The upload only succeeds if the PUT carries both the deployment's SigV4 identity and the SSE-KMS headers, since the bucket policy denies anything else, and
head-objectconfirms the object landed under the pinned key. The batch job ARN names the deployment's own account and region, so all three handlers demonstrably used the deployment's own credentials2. Caller-chosen signing identity: honored before the hardening, refused at the tip
The escalation is easiest to see as a contrast on one proxy: the same request twice, differing only by one caller-supplied field.
At
b5eed5e526, this branch after the credential fix but before the boundary hardening, on port 45911.aws_profile_nameis not on the banned list at that commit:The proxy log for the 403 shows the caller's field surviving auth, reaching credential resolution as
aws_access_key_id=[set=True] ... aws_profile_name=attacker, and AWS answering frombedrock.us-west-2.amazonaws.com/model-invocation-job. The deployment's real key was present on that very call and the caller's profile won anyway. The bogus AWS credentials used for theattackerprofile are the public example pair from AWS's own documentationThis is not a hole this PR opened. At the merge base the same trick already worked on chat completions, where deployment credentials were being resolved correctly all along:
So fixing the credential merge without also closing the boundary would have extended a pre-existing chat-route hole onto uploads and batches. At the tip that chat-route hole is closed too, control then treatment on one proxy:
The log slice for that rejected call contains no
bedrock-runtimecall, no outbound request, and no SigV4 signing at all, against one of each in the control slice, so nothing was signed before the refusalAnd on uploads and batches at the tip, on port 41733:
The tip proxy's access log carries the whole contrast in order:
POST /v1/files 200,POST /v1/batches 200,GET /v1/batches/... 200, thenPOST /v1/files 401,POST /v1/batches 401,POST /v1/batches 401, thenPOST /v1/files 200. Exactly one Bedrock job id appears anywhere in that log, so no rejected request reached AWSNotes for the reviewer, from the runs rather than the diff
The refusal comes back as
401with"type":"auth_error", not400, because the banned-param check runs inside the auth dependency. This PR neither causes nor changes that: it is the same envelope every previously-banned param already produced, and the PR only appends three names to that listOn the uploads route the caller's extra multipart field was dropped before credential resolution even at
b5eed5e526, so the observed escalation was specific to/v1/batchesand chat completions. The banned-param check still covers the uploads route, and multipart fields do reach it, which is why the 401 above fires there tooThree things the runs surfaced that this PR leaves alone. A successful upload reports
"bytes": 0for a file thathead-objectshows as 396 bytes. Batch create returns"model": nullwhile the later retrieve fills inmetadata.model_arn. And puttingaws_batch_role_arnin a deployment'slitellm_paramsmakes plain chat on that same alias fail validation withExtra inputs are not permitted, so one alias cannot currently serve both chat and batch; that one was measured at the merge base and not re-measured at the tip, which is why the chat checks above use a batch-field-free aliasHow the earlier gaps were caught
The live run is also what caught a second gap on this branch's earlier head
f72ddedf39: the router's credential resolution round-tripslitellm_paramsthrough a strict schema that only knewaws_access_key_id,aws_secret_access_key, andaws_region_name, so the deployment'saws_session_tokenand its assume-role, profile, and web-identity settings were silently dropped and S3 answeredInvalidAccessKeyId.b63ba63655adds those fields plus a mutation-tested regression test that fails on the old schema and passes on the new oneType
🐛 Bug Fix
Caveats (if any)
anthropic_version: Field requiredwhen the JSONL names the proxy model alias #36156Final Attestation