fix(bedrock/claude-platform): normalize content-type header case to fix SigV4 401 on aws-external-anthropic - #28257
Conversation
…re SigV4 signing When the caller passes headers with a lowercase "content-type" key (as get_anthropic_headers() does), and _sign_request prepends an uppercase "Content-Type" key, both survive as separate entries in a plain Python dict. botocore's AWSRequest uses a case-insensitive HeadersDict, so it sees two values for the same header and joins them into "application/json, application/json" in the SigV4 canonical string. The actual HTTP request only sends one value, so the signature never matches → 401 authentication_error from aws-external-anthropic. Fix: normalise all header keys to lowercase before signing with setdefault() to add content-type only if not already present. Adds a regression test that captures the AWSRequest headers and asserts exactly one content-type key reaches the signer. Fixes: duplicate Content-Type in aws-external-anthropic SigV4 canonical string
|
|
Congrats! CodSpeed is installed 🎉
You will start to see performance impacts in the reports once the benchmarks are run from your default branch.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
🤖 litellm-agent: This PR is currently BLOCKED from merge. Score: 2/5 ❌ Why blocked:
Details: Score docked for: 1 PR-related CI failure (Greptile gate: score not yet reviewed below required 4/5 — request a Greptile review ( Fix the issues above and push an update — the bot will re-review automatically.
|
Greptile SummaryThis PR fixes a SigV4 signing bug on the
Confidence Score: 5/5Safe to merge — the change is confined to the header-normalization step in _sign_request, the root cause is well-understood, and targeted regression tests cover both the content-type deduplication and the caller-Authorization-override path. The fix is minimal and surgical: it addresses the exact dict key-case collision that produced the malformed SigV4 canonical string. _filter_headers_for_aws_signature already lowercases keys internally before comparing, so it is unaffected. The Authorization handling is correct — the pop-then-uppercase-set pattern eliminates the intermediate duplicate before the method returns. No existing tests were modified, and both new tests are mock-only and run cleanly in CI. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/llms/bedrock/base_aws_llm.py | Normalizes incoming header keys to lowercase before SigV4 signing, preventing duplicate content-type entries; also fixes the Authorization guard to use the post-normalization lowercase key and pops the lowercase entry before re-adding it uppercase. |
| tests/test_litellm/llms/bedrock/test_claude_platform_provider.py | Adds regression test that captures headers passed to AWSRequest and asserts exactly one content-type key is present, directly covering the canonical-string duplication bug. |
| tests/test_litellm/llms/bedrock/test_base_aws_llm.py | Adds test verifying that a caller-supplied Authorization header wins over SigV4's generated value, and that no duplicate authorization keys appear in the output. |
Reviews (3): Last reviewed commit: "fix(bedrock/base_aws_llm): remove lowerc..." | Re-trigger Greptile
…der normalization After lowercasing all header keys, the Authorization override guard was checking for uppercase "Authorization" (always False) and the key lookup would also fail. Update both to use lowercase "authorization" to match the post-normalization dict. Caught by Greptile review on PR BerriAI#28257.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
🤖 litellm-agent: This PR is currently BLOCKED from merge. Score: 4/5 ❌ Why blocked:
Details: Score docked for: 1 unresolved reviewer concern (greptile). Fix the issues above and push an update — the bot will re-review automatically.
|
…fter SigV4 override After the header normalization, the merge loop writes lowercase 'authorization' into request_headers_dict and the Authorization-override guard then writes uppercase 'Authorization' — both keys survive as separate entries in the plain dict. Fix by popping the lowercase key before writing the canonical uppercase one so HTTP clients only see a single Authorization header. Add test_sign_request_caller_authorization_overrides_sigv4 to cover the override branch (fixes Codecov missing-line report) and assert no duplicate key survives in the returned headers dict.
|
Addressed the remaining Greptile concern (commit 53832e3): Problem: the merge loop wrote lowercase Fix: Coverage: added All 90 unit tests pass locally. |
|
🤖 litellm-agent: This PR is currently BLOCKED from merge. Score: 0/5 ❌ Why blocked:
Details: Score docked for: karpathy needs_human — The normalization Fix the issues above and push an update — the bot will re-review automatically.
|
…d header normalization
The previous normalization ({k.lower() for k in headers}) lowercased every
header key for all 8+ callers of _sign_request, not just the claude_platform
route that triggered the bug. That was a broader change than necessary.
Replace with a targeted guard: only prepend Content-Type when no caller has
already set it under any casing. This leaves all existing header keys
untouched for every other route while still preventing the duplicate
content-type entry in the SigV4 canonical string for claude_platform.
The Authorization override guard is restored to its original form since it
relied on the caller using uppercase "Authorization" — unchanged behavior.
|
Addressed the Problem with the previous approach: New approach: replace the blanket normalization with a targeted case-insensitive guard: if not any(k.lower() == "content-type" for k in headers):
headers = {"Content-Type": "application/json", **headers}This only skips the prepend when 91 unit tests pass locally. |
|
Closing in favor of a simpler fix — see replacement PR. |
2 participants
Relevant issues
Fixes #28256
Pre-Submission checklist
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unitType
🐛 Bug Fix
Changes
litellm/llms/bedrock/base_aws_llm.py—_sign_request()(lines 1502-1505)Before:
After:
tests/test_litellm/llms/bedrock/test_claude_platform_provider.py— addedtest_sigv4_no_duplicate_content_type_in_canonical_stringwhich captures the headers dict passed toAWSRequestand asserts exactly onecontent-typekey is present.Root cause
get_anthropic_headers()sets"content-type": "application/json"(lowercase)._sign_request()then prepends"Content-Type": "application/json"(uppercase). Python dicts are case-sensitive, so both keys survive. botocore'sAWSRequestuses a case-insensitiveHeadersDictand joins both values into"application/json, application/json"in the SigV4 canonical string. The actual wire request sends only"application/json", so the signatures never match → 401.This affects all requests to the
bedrock/claude_platform/<model>route (aws-external-anthropic.<region>.api.aws), making the feature unusable since it was introduced in #27678.Screenshots / Proof of Fix
Before (canonical string from AWS error response):
After:
content-type:application/json(single value, signature matches).All 12 unit tests in
test_claude_platform_provider.pypass.