Skip to content

fix: prevent key-level metadata.tags from leaking into Bedrock passthrough body - #30666

Closed
factnn wants to merge 1 commit into
BerriAI:litellm_internal_stagingfrom
factnn:fix/bedrock-passthrough-metadata-leak
Closed

fix: prevent key-level metadata.tags from leaking into Bedrock passthrough body#30666
factnn wants to merge 1 commit into
BerriAI:litellm_internal_stagingfrom
factnn:fix/bedrock-passthrough-metadata-leak

Conversation

@factnn

@factnn factnn commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Fixes #30629

Key-level spend-tracking tags were injected into data["metadata"]
for Bedrock passthrough routes, then forwarded in the provider request
body. Bedrock rejects non-user_id metadata fields with HTTP 400.

Add "bedrock" to LITELLM_METADATA_ROUTES so tags are stored in
data["litellm_metadata"] instead. Spend tracking reads from both
locations, so tags are preserved for billing without leaking to the
upstream provider.

Consistent with the existing handling for /v1/messages and
responses routes.

Type

Bug Fix

Changes

  • litellm/proxy/litellm_pre_call_utils.py: 1 line

@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
litellm/proxy/auth/auth_checks.py 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a bug where key-level metadata.tags (set on API keys for spend tracking) were being injected into data["metadata"] for Bedrock passthrough routes and forwarded in the provider request body, causing HTTP 400 rejections from Bedrock.

  • litellm_pre_call_utils.py: Adds "bedrock" to LITELLM_METADATA_ROUTES so _get_metadata_variable_name returns "litellm_metadata" for all Bedrock passthrough paths, keeping key-level tags out of the provider-facing body.
  • auth_checks.py: Pre-seeds litellm_metadata: {} into the request body for Bedrock routes inside common_checks, so apply_key_tags_pre_auth (which uses get_metadata_variable_name_from_kwargs to detect which key to write to) correctly picks up litellm_metadata rather than metadata.
  • Tests: Two new unit tests confirm _get_metadata_variable_name returns "litellm_metadata" for Bedrock invoke/converse URLs; the existing Bedrock passthrough headers test is updated to assert the corrected location.

Confidence Score: 5/5

Safe to merge; the one-line change to LITELLM_METADATA_ROUTES and the auth_checks pre-seed are narrow and well-tested for the key-level tag path.

Both code changes are small, targeted, and consistent with existing patterns in the codebase. The new unit tests directly cover the routing function change, and the guardrails header fallback logic already reads from both metadata and litellm_metadata, so no downstream consumers are broken. The residual gap (x-litellm-tags header tags still writing to metadata for Bedrock routes) is pre-existing and out of scope for this fix.

litellm/proxy/auth/auth_checks.py — the pre-seeding runs after apply_client_tag_policy_pre_auth, leaving x-litellm-tags header tags still routed to metadata for Bedrock routes.

Important Files Changed

Filename Overview
litellm/proxy/litellm_pre_call_utils.py Adds "bedrock" to LITELLM_METADATA_ROUTES so _get_metadata_variable_name returns "litellm_metadata" for all Bedrock passthrough paths; one-line, consistent with existing entries.
litellm/proxy/auth/auth_checks.py Pre-seeds litellm_metadata in common_checks for bedrock routes so apply_key_tags_pre_auth routes key-level tags correctly; header-sourced x-litellm-tags still go to metadata (apply_client_tag_policy_pre_auth runs before this pre-seed).
tests/test_litellm/proxy/pass_through_endpoints/test_pass_through_endpoints.py Updates existing bedrock-path test to expect headers in litellm_metadata, correctly reflecting new routing behaviour; test assertions remain equivalent in strength.
tests/test_litellm/proxy/test_litellm_pre_call_utils.py Adds two new unit tests confirming _get_metadata_variable_name returns "litellm_metadata" for bedrock invoke and converse URLs; directly covers the litellm_pre_call_utils.py fix.

Reviews (4): Last reviewed commit: "fix: prevent key-level metadata.tags fro..." | Re-trigger Greptile

@Sameerlite

Copy link
Copy Markdown
Contributor

Thanks for the PR! A couple of things to get this over the finish line:

  • Greptile's code review came back with a score below 5/5 — could you take a look at its feedback and address the comments? Once it reaches 5/5 with no open threads we'll take another look.
  • The CI checks are currently failing — could you take a look? If any failures are pre-existing or unrelated to your change, a quick note in a comment helps us move faster.
  • Could you add proof of the change working (screenshots, test output, or a sample request/response)? Even a quick curl before/after really speeds up the review.

Once those are addressed we'll take another look — appreciate the contribution!

@factnn

factnn commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@Sameerlite Hi there, thanks for your review!

The only CI failure is the same pre-existing infrastructure issue as other litellm PRs — lint fails because type_discipline_gate.py is missing from the CI environment. Not related to this change.

I've also added regression tests for bedrock routes and updated the existing passthrough test. Greptile score should improve on the next run.

@Sameerlite

Copy link
Copy Markdown
Contributor

@greptileai

@Sameerlite

Copy link
Copy Markdown
Contributor

@factnn Can you rebase with latest litellm_internal_staging?

@factnn
factnn force-pushed the fix/bedrock-passthrough-metadata-leak branch from b0d28d1 to 034bf8f Compare June 18, 2026 13:31
@factnn

factnn commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@Sameerlite Done, my pleasure.

Comment thread litellm/proxy/litellm_pre_call_utils.py
@veria-ai

veria-ai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

@Sameerlite

Copy link
Copy Markdown
Contributor

Thanks for the rebase and for the updated tests! A couple of things still needed before we can merge:

  • Could you add proof of the change working (e.g. a curl showing the Bedrock passthrough no longer returns HTTP 400, or test output confirming tags are routed to litellm_metadata and not leaked to the provider body)?

Triggering a fresh Greptile review on the updated commit:

@greptileai

Once those are addressed, we'll take another look!

@factnn
factnn force-pushed the fix/bedrock-passthrough-metadata-leak branch from d7a348d to e496883 Compare June 21, 2026 06:43
@factnn

factnn commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the rebase and for the updated tests! A couple of things still needed before we can merge:

  • Could you add proof of the change working (e.g. a curl showing the Bedrock passthrough no longer returns HTTP 400, or test output confirming tags are routed to litellm_metadata and not leaked to the provider body)?

Triggering a fresh Greptile review on the updated commit:

@greptileai

Once those are addressed, we'll take another look!

@Sameerlite two updates:

1.All three tests pass in CI: https://github.com/BerriAI/litellm/actions/runs/27896275028/job/82548278630. The 3 CI tests are as below and screenshots are attached.
PASSED test_returns_litellm_metadata_for_bedrock_converse
PASSED test_returns_litellm_metadata_for_bedrock_invoke
PASSED test_add_litellm_data_to_request_adds_headers_to_metadata

2.Adopted the Veria AI suggestion — added a litellm_metadata pre-seed in auth_checks.py before apply_key_tags_pre_auth. That one extra line is what drags down codecov/patch, since the auth-stage path isn't covered by unit tests.

截屏2026-06-21 15 30 39 截屏2026-06-21 15 31 08

@greptile-apps

greptile-apps Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

The PR looks good. The changes are correct and the two-part approach is sound:

  1. litellm_pre_call_utils.py — adding "bedrock" to LITELLM_METADATA_ROUTES handles the pre-call path via _get_metadata_variable_name.
  2. auth_checks.py — the setdefault("litellm_metadata", {}) pre-seed is necessary because apply_key_tags_pre_auth runs at auth time before the pre-call phase, so it can't rely on LITELLM_METADATA_ROUTES alone to steer tags to litellm_metadata.

The CI passing on all three targeted tests (test_returns_litellm_metadata_for_bedrock_converse, test_returns_litellm_metadata_for_bedrock_invoke, test_add_litellm_data_to_request_adds_headers_to_metadata) is sufficient evidence that the routing is correct. The codecov/patch drop on the auth-stage line is expected since that path requires an integration test with a live/mocked token flow — it's not a blocker.

This is ready to merge from a technical standpoint.

@Sameerlite

Copy link
Copy Markdown
Contributor

@greptileai

@mateo-berri

mateo-berri commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

This has been merged (via a mirror branch to run CircleCI). Thank you so much for the contribution!

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.

[Bug]: Virtual-key metadata.tags leak into Bedrock invoke passthrough body → "metadata.tags: Extra inputs are not permitted"

3 participants