Skip to content

fix(bedrock): sign rerank requests with the shared, header-filtered SigV4 helper - #36462

Open
noahnistler wants to merge 2 commits into
BerriAI:litellm_internal_stagingfrom
Evernorth:litellm_bedrock_rerank_cross_account_signing
Open

fix(bedrock): sign rerank requests with the shared, header-filtered SigV4 helper#36462
noahnistler wants to merge 2 commits into
BerriAI:litellm_internal_stagingfrom
Evernorth:litellm_bedrock_rerank_cross_account_signing

Conversation

@noahnistler

@noahnistler noahnistler commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Bedrock rerank calls fail with a SigV4 signature mismatch when the proxy forwards client headers
  • The rerank handler signs every header instead of only the ones AWS actually includes in the signature

How it solves it:

  • Route rerank request signing through the shared get_request_headers helper other Bedrock handlers already use
  • That helper filters headers before signing, then re-attaches the rest unsigned, so forwarded headers can't corrupt the signature

User Flow

Before: an admin whose proxy forwards client headers to backend LLM APIs (forward_client_headers_to_llm_api) gets every Bedrock rerank call rejected by AWS

  1. The proxy has that setting on and a Bedrock rerank deployment, e.g. cohere.rerank-v3-5:0
  2. A user's app sends POST https://litellm-domain/v1/rerank with a query and a list of documents
  3. The response carries an AWS error: "The request signature we calculated does not match the signature you provided"

After: the same call succeeds

  1. Same proxy config
  2. The user sends the same POST https://litellm-domain/v1/rerank
  3. The response returns ranked results with relevance scores, same shape as before this bug

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to 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

I don't have access to a proxy deployment with header forwarding enabled and a live Bedrock rerank model to capture end-to-end curl output for this one, so I can't post real request/response logs. What I can show:

Deterministic before/after on the actual signing code, not a mock of it: tests/test_litellm/llms/bedrock/rerank/test_bedrock_rerank_header_forwarding.py::test_bedrock_rerank_forwarded_headers_excluded_from_sigv4_signature builds a real AWSRequest through BedrockRerankHandler._prepare_request with a forwarded x-forwarded-for header and inspects the actual computed Authorization header.

Before the fix (checked out at 3726bceb53, the commit this branch is based on): SignedHeaders includes x-forwarded-for, so any hop that rewrites that header between LiteLLM signing the request and AWS receiving it invalidates the signature.

After the fix (this branch): SignedHeaders is content-type;host;x-amz-date;x-amz-security-token, no longer including x-forwarded-for, while the header itself is still present and forwarded on the wire.

For anyone who can reproduce this against real AWS, this is the config that should trigger it (a Bedrock rerank deployment behind a proxy forwarding client headers):

general_settings:
  forward_client_headers_to_llm_api: true

model_list:
  - model_name: bedrock-rerank
    litellm_params:
      model: bedrock/cohere.rerank-v3-5:0
      aws_region_name: us-east-1
curl -s -X POST http://localhost:4000/v1/rerank \
  -H "Authorization: Bearer sk-1234" \
  -H "Content-Type: application/json" \
  -H "X-Forwarded-For: 203.0.113.5" \
  -d '{
    "model": "bedrock-rerank",
    "query": "What is the capital of the United States?",
    "documents": [
      "Carson City is the capital of Nevada.",
      "Washington, D.C. is the capital of the United States."
    ]
  }'

Type

🐛 Bug Fix

Caveats (if any)

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

Note

Medium Risk
Touches AWS request signing for Bedrock rerank. Behavior is aligned with other Bedrock handlers and covered by a signature-inspection test, but a signing bug would break all rerank calls.

Overview
Fixes Bedrock rerank SigV4 mismatches when the proxy forwards client headers (forward_client_headers_to_llm_api). Headers like x-forwarded-for were previously signed, then rewritten in transit, so AWS rejected the request.

BedrockRerankHandler._prepare_request now uses the shared get_request_headers helper instead of signing every header inline. Forwarded headers still go to Bedrock, but they are excluded from SignedHeaders.

Adds a unit test that inspects the real Authorization header and asserts x-forwarded-for is present but unsigned.

Reviewed by Cursor Bugbot for commit d80608e. Bugbot is set up for automated code reviews on this repo. Configure here.

…igV4 helper

BedrockRerankHandler._prepare_request duplicated ad-hoc SigV4 signing
instead of using BaseAWSLLM.get_request_headers, the helper every other
Bedrock handler (embeddings, converse, invoke, image) already uses.
The duplicate skipped header filtering before signing, so any forwarded
header (e.g. x-forwarded-for) got included in the signed set and could
invalidate the signature if rewritten downstream between signing and
delivery, the same class of bug fixed for the invoke path in BerriAI#19111.
@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR routes Bedrock rerank signing through the shared header-filtering helper while preserving forwarded headers unsigned

  • Replaces direct SigV4 request construction with get_request_headers
  • Adds a regression test using injected static AWS credentials without class-level monkeypatching

Confidence Score: 5/5

The PR appears safe to merge

No blocking failure remains, and the previously reported class-level credential monkeypatch has been removed

Important Files Changed

Filename Overview
litellm/llms/bedrock/rerank/handler.py Uses the shared Bedrock signing helper to exclude forwarded headers from the signature while retaining them on the request
tests/test_litellm/llms/bedrock/rerank/test_bedrock_rerank_header_forwarding.py Verifies forwarded headers remain present but unsigned, with credentials supplied through the normal instance request path

Reviews (2): Last reviewed commit: "test(bedrock): drop class-level monkeypa..." | Re-trigger Greptile

Comment thread tests/test_litellm/llms/bedrock/rerank/test_bedrock_rerank_header_forwarding.py Outdated
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Pass static AWS credentials through optional_params so the real
credential-resolution path runs locally instead of patching
BedrockRerankHandler._get_boto_credentials_from_optional_params.
@noahnistler

Copy link
Copy Markdown
Contributor Author

@greptileai please re-review, the test now injects static AWS credentials instead of monkeypatching the credential-resolution method.

@codspeed-hq

codspeed-hq Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing Evernorth:litellm_bedrock_rerank_cross_account_signing (d80608e) with litellm_internal_staging (ade805e)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_internal_staging (444b275) during the generation of this report, so ade805e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@mateo-berri

Copy link
Copy Markdown
Contributor

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d80608e. Configure here.

endpoint_url=proxy_endpoint_url,
data=body,
headers=headers,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bearer path breaks Bedrock rerank auth

High Severity

Routing rerank through get_request_headers also picks up its AWS_BEARER_TOKEN_BEDROCK branch. Bedrock API keys are not valid for Agents Runtime APIs such as /rerank, so when that env var is set the handler sends Bearer auth instead of SigV4 and AWS rejects the call. Previously this path always signed with SigV4.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d80608e. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants