Skip to content

fix: _filter_headers_for_aws_signature - Bedrock KB#23571

Merged
ishaan-jaff merged 2 commits intolitellm_ishaan_march_13from
litellm_fix_vector_store_bedrock_issue
Mar 14, 2026
Merged

fix: _filter_headers_for_aws_signature - Bedrock KB#23571
ishaan-jaff merged 2 commits intolitellm_ishaan_march_13from
litellm_fix_vector_store_bedrock_issue

Conversation

@ishaan-jaff
Copy link
Member

fix: _filter_headers_for_aws_signature - Bedrock KB

Pre-Submission checklist

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

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Type

🐛 Bug Fix
✅ Test

Changes

@vercel
Copy link

vercel bot commented Mar 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Mar 13, 2026 5:50pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 13, 2026

Greptile Summary

This PR fixes a crash in the Bedrock Knowledge Base (KB) request path where botocore's SigV4Auth threw AttributeError: 'NoneType' object has no attribute 'split' because headers with None values (e.g. x-amzn-bedrock-kb-session-id) were passed directly into the signing process or merged back into the final headers dict.

Key changes:

  • _filter_headers_for_aws_signature: Added if header_value is None: continue to prevent None-valued headers from reaching SigV4Auth.
  • First signing code path (~line 1271): Added if header_value is not None guard when setting headers back on the AWSRequest object after signing.
  • _sign_request (~line 1399): Added if header_value is not None guard when merging original headers back into the returned dict.
  • Two new unit tests added (test_filter_headers_skips_none_values and test_sign_request_with_none_header_values) — both are purely mock-based with no real network calls.

Note: The PR description still shows placeholder text (Fixes #000) and all pre-submission checklist items are unchecked. Providing a link to the original issue and CI run results would help confirm the fix is working as intended.

Confidence Score: 4/5

  • Safe to merge — the fix is correct, consistent, and targeted, with only a minor test coverage gap.
  • The None-value guards are applied correctly in all three header-merging locations and the root cause (botocore SigV4Auth receiving None header values) is properly addressed. Two focused mock-only tests are included. The slight coverage gap is that the fix in get_request_headers (~line 1271) is not directly tested by the new tests, and the PR description still contains placeholder text ("Fixes #000") rather than referencing the actual issue.
  • No files require special attention — all changes are minimal and well-scoped.

Important Files Changed

Filename Overview
litellm/llms/bedrock/base_aws_llm.py Adds None-value guards in three header-merging locations so that headers with None values are never passed to botocore's SigV4Auth or returned to callers — correctly fixing the NoneType has no attribute 'split' crash for Bedrock KB.
tests/test_litellm/llms/bedrock/test_base_aws_llm.py Adds two focused unit tests: one for _filter_headers_for_aws_signature and one end-to-end for _sign_request. Both tests are mock-only (no real network calls). The end-to-end test does not exercise the separate first signing code path (around line 1271) that was also patched.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Incoming request headers] --> B{header_value is None?}
    B -- Yes --> C[Skip / continue]
    B -- No --> D{Is it an AWS-relevant header?\nhost, content-type, x-amz-*, x-amzn-*}
    D -- No --> E[Exclude from SigV4 headers]
    D -- Yes --> F[Include in aws_signature_headers]
    F --> G[AWSRequest + SigV4Auth.add_auth]
    G --> H[Signed request headers dict]
    H --> I[Merge back original headers]
    I --> J{header_value is None?}
    J -- Yes --> K[Skip / do not add to final dict]
    J -- No --> L[Add to final headers dict]
    L --> M[Return headers + body]
Loading

Comments Outside Diff (1)

  1. tests/test_litellm/llms/bedrock/test_base_aws_llm.py, line 1552-1597 (link)

    Missing coverage for get_request_headers None-value guard

    The PR patches three locations, but the new end-to-end test only exercises _sign_request. The identical post-signing header merge-back guard added to get_request_headers (around base_aws_llm.py:1270-1272) is not covered by any new test.

    Consider adding a parallel test for get_request_headers with None-valued headers (e.g. x-amzn-bedrock-kb-session-id: None) to ensure the fix there is also regression-protected. For example:

    def test_get_request_headers_skips_none_header_values():
        llm = BaseAWSLLM()
        mock_credentials = Credentials("test_key", "test_secret")
        headers_with_nones = {
            "Content-Type": "application/json",
            "x-amzn-bedrock-kb-session-id": None,
        }
        prepped = llm.get_request_headers(
            credentials=mock_credentials,
            aws_region_name="us-gov-west-1",
            extra_headers=None,
            endpoint_url="https://bedrock-agent-runtime.us-gov-west-1.amazonaws.com/knowledgebases/KB123/retrieve",
            data=b'{"test": "body"}',
            headers=headers_with_nones,
        )
        for key, value in prepped.headers.items():
            assert value is not None, f"Header '{key}' must not be None"

Last reviewed commit: ac4d982

Addresses Greptile feedback: None-valued headers were being filtered
during SigV4 signing but re-merged back into the final headers dict
afterward, which would cause downstream HTTP client failures.

Made-with: Cursor
@ishaan-jaff
Copy link
Member Author

@greptile review again

@ishaan-jaff ishaan-jaff changed the base branch from main to litellm_ishaan_march_13 March 14, 2026 03:06
@ishaan-jaff ishaan-jaff merged commit 64f9e9d into litellm_ishaan_march_13 Mar 14, 2026
74 of 97 checks passed
ishaan-jaff added a commit that referenced this pull request Mar 14, 2026
* fix(bedrock): respect s3_region_name for batch file uploads (#23569)

* fix(bedrock): respect s3_region_name for batch file uploads (GovCloud fix)

* fix: s3_region_name always wins over aws_region_name for S3 signing (Greptile feedback)

* fix: _filter_headers_for_aws_signature - Bedrock KB (#23571)

* fix: _filter_headers_for_aws_signature

* fix: filter None header values in all post-signing re-merge paths

Addresses Greptile feedback: None-valued headers were being filtered
during SigV4 signing but re-merged back into the final headers dict
afterward, which would cause downstream HTTP client failures.

Made-with: Cursor

* feat(router): tag_regex routing — route by User-Agent regex without per-developer tag config (#23594)

* feat(router): add tag_regex support for header-based routing

Adds a new `tag_regex` field to litellm_params that lets operators route
requests based on regex patterns matched against request headers — primarily
User-Agent — without requiring per-developer tag configuration.

Use case: route all Claude Code traffic (User-Agent: claude-code/x.y.z) to
a dedicated deployment by setting:

  tag_regex:
    - "^User-Agent: claude-code\\/"

in the deployment's litellm_params. Works alongside existing `tags` routing;
exact tag match takes precedence over regex match. Unmatched requests fall
through to deployments tagged `default`.

The matched deployment, pattern, and user_agent are recorded in
`metadata["tag_routing"]` so they flow through to SpendLogs automatically.

* fix(tag_regex): address backwards-compat, metadata overwrite, and warning noise

Three issues from code review:

1. Backwards-compat: `has_tag_filter` was widened to activate on any non-empty
   User-Agent, which would raise ValueError for existing deployments using plain
   tags without a `default` fallback. Fix: only activate header-based regex
   filtering when at least one candidate deployment has `tag_regex` configured.

2. Metadata overwrite: `metadata["tag_routing"]` was overwritten for every
   matching deployment in the loop, leaving inaccurate provenance when multiple
   deployments match. Fix: write only for the first match.

3. Warning noise: an invalid regex pattern logged one warning per header string
   rather than once per pattern. Fix: compile first (catching re.error once),
   then iterate over header strings.

Also adds two new tests covering these cases, and adds docs page for
tag_regex routing with a Claude Code walk-through.

* refactor(tag_regex): remove unnecessary _healthy_list copy

* docs: merge tag_regex section into tag_routing.md, remove standalone page

- Add ## Regex-based tag routing (tag_regex) section to existing
  tag_routing.md instead of a separate page
- Remove tag_regex_routing.md standalone doc (odd UX to have a separate
  page for a sub-feature)
- Remove proxy/tag_regex_routing from sidebars.js
- Add match_any=False debug warning in tag_based_routing.py when regex
  routing fires under strict mode (regex always uses OR semantics)

* fix(tag_regex): address greptile review - security docs, strict-mode enforcement, validation order

- Strengthen security note in tag_routing.md: explicitly state User-Agent
  is client-supplied and can be set to any value; frame tag_regex as a
  traffic classification hint, not an access-control mechanism
- Move tag_regex startup validation before _add_deployment() so an invalid
  pattern never leaves partial router state
- Enforce match_any=False strict-tag policy: when a deployment has both
  tags and tag_regex and the strict tag check fails, skip the regex fallback
  rather than silently bypassing the operator's intent
- Extract per-deployment match logic into _match_deployment() helper to
  keep get_deployments_for_tag() readable
- Add two new tests: strict-mode blocks regex fallback, regex-only
  deployment still matches under match_any=False

* fix(ci): apply Black formatting to 14 files and stabilize flaky caplog tests

- Run Black formatter on 14 files that were failing the lint check
- Replace caplog-based assertions in TestAliasConflicts with
  unittest.mock.patch on verbose_logger.warning for xdist compatibility
- The caplog fixture can produce empty text in pytest-xdist workers
  in certain CI environments, causing flaky test failures

Co-authored-by: Ishaan Jaff <ishaan-jaff@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Ishaan Jaff <ishaan-jaff@users.noreply.github.com>
RheagalFire pushed a commit that referenced this pull request Mar 15, 2026
…der (#23663)

* fix: forward extra_headers to HuggingFace embedding calls (#23525)

Fixes #23502

The huggingface_embed.embedding() call was not receiving the headers
parameter, causing extra_headers (e.g., X-HF-Bill-To) to be silently
dropped. Other providers (openrouter, vercel_ai_gateway, bedrock) already
pass headers correctly. This fix adds headers=headers to match the
behavior of other providers.

Co-authored-by: Jah-yee <sparklab@outlook.com>

* fix: add getPopupContainer to Select components in fallback modal to fix z-index issue (#23516)

The model dropdown menus in the Add Fallbacks modal were rendering behind
the modal overlay because Ant Design portals Select dropdowns to document.body
by default. By setting getPopupContainer to attach the dropdown to its parent
element, the dropdown inherits the modal's stacking context and renders above
the modal.

Fixes #17895

* PR #22867 added _remove_scope_from_cache_control for Bedrock and Azur… (#23183)

* PR #22867 added _remove_scope_from_cache_control for Bedrock and Azure AI but omitted Vertex AI. This applies the same pattern to VertexAIPartnerModelsAnthropicMessagesConfig."

* PR #22867 added _remove_scope_from_cache_control for Bedrock and Azure AI but omitted Vertex AI. This applies the same pattern to VertexAIPartnerModelsAnthropicMessagesConfig."

* PR #22867 added _remove_scope_from_cache_control to AzureAnthropicMessagesConfig
 but missed VertexAIPartnerModelsAnthropicMessagesConfi Rather than duplicating the method again, moved it up to the base AnthropicMessagesConfig so all providers
  inherit it, and removed the now-redundant copy from the Azure AI subclass.

* PR #22867 added _remove_scope_from_cache_control to AzureAnthropicMessagesConfig
 but missed VertexAIPartnerModelsAnthropicMessagesConfi Rather than duplicating the method again, moved it up to the base AnthropicMessagesConfig so all providers
  inherit it, and removed the now-redundant copy from the Azure AI subclass.

---------

Co-authored-by: Krish Dholakia <krrishdholakia@gmail.com>

* fix: auto-fill reasoning_content for moonshot kimi reasoning models in multi-turn tool calling (#23580)

* Handle response.failed, response.incomplete, and response.cancelled (#23492)

* Handle response.failed, response.incomplete, and response.cancelled terminal events in background streaming

Previously the background streaming task only handled response.completed and
hardcoded the final status to "completed". This missed three other terminal
event types from the OpenAI streaming spec, causing failed/incomplete/cancelled
responses to be incorrectly marked as completed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude

* Remove unused terminal_response_data variable

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude

* Address code review: derive fallback status from event type, rewrite tests as integration tests

1. Replace hardcoded "completed" fallback in response_data.get("status")
   with _event_to_status lookup so that response.incomplete and
   response.cancelled events get the correct fallback if the response
   body ever omits the status field.

2. Replace duplicated-logic unit tests with integration tests that
   exercise background_streaming_task directly using mocked streaming
   responses and assert on the final update_state call arguments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude

* Remove dead mock_processor and unused mock_response parameter from test helper

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude

* Remove FastAPI and UserAPIKeyAuth imports from test file

These types were only used as Mock(spec=...) arguments. Drop the spec
constraints and remove the top-level imports to avoid pulling FastAPI
into test files outside litellm/proxy/.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude

* Log warning when streaming response has no body_iterator

If base_process_llm_request returns a non-streaming response (no
body_iterator), log a warning since this likely indicates a
misconfiguration or provider error rather than a successful completion.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* fix(security): bump tar to 7.5.11 and tornado to 6.5.5 (#23602)

* fix(security): bump tar to 7.5.11 and tornado to 6.5.5

- tar >=7.5.11: fixes CVE-2026-31802 (HIGH) in node-pkg
- tornado >=6.5.5: fixes CVE-2026-31958 (HIGH) and GHSA-78cv-mqj4-43f7 (MEDIUM) in python-pkg

Addresses vulnerabilities found in ghcr.io/berriai/litellm:main-v1.82.0-stable Trivy scan.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: document tar override is enforced via Dockerfile, not npm

* fix: revert invalid JSON comment in package.json tar override

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* [Feat] - Ishaan main merge branch  (#23596)

* fix(bedrock): respect s3_region_name for batch file uploads (#23569)

* fix(bedrock): respect s3_region_name for batch file uploads (GovCloud fix)

* fix: s3_region_name always wins over aws_region_name for S3 signing (Greptile feedback)

* fix: _filter_headers_for_aws_signature - Bedrock KB (#23571)

* fix: _filter_headers_for_aws_signature

* fix: filter None header values in all post-signing re-merge paths

Addresses Greptile feedback: None-valued headers were being filtered
during SigV4 signing but re-merged back into the final headers dict
afterward, which would cause downstream HTTP client failures.

Made-with: Cursor

* feat(router): tag_regex routing — route by User-Agent regex without per-developer tag config (#23594)

* feat(router): add tag_regex support for header-based routing

Adds a new `tag_regex` field to litellm_params that lets operators route
requests based on regex patterns matched against request headers — primarily
User-Agent — without requiring per-developer tag configuration.

Use case: route all Claude Code traffic (User-Agent: claude-code/x.y.z) to
a dedicated deployment by setting:

  tag_regex:
    - "^User-Agent: claude-code\\/"

in the deployment's litellm_params. Works alongside existing `tags` routing;
exact tag match takes precedence over regex match. Unmatched requests fall
through to deployments tagged `default`.

The matched deployment, pattern, and user_agent are recorded in
`metadata["tag_routing"]` so they flow through to SpendLogs automatically.

* fix(tag_regex): address backwards-compat, metadata overwrite, and warning noise

Three issues from code review:

1. Backwards-compat: `has_tag_filter` was widened to activate on any non-empty
   User-Agent, which would raise ValueError for existing deployments using plain
   tags without a `default` fallback. Fix: only activate header-based regex
   filtering when at least one candidate deployment has `tag_regex` configured.

2. Metadata overwrite: `metadata["tag_routing"]` was overwritten for every
   matching deployment in the loop, leaving inaccurate provenance when multiple
   deployments match. Fix: write only for the first match.

3. Warning noise: an invalid regex pattern logged one warning per header string
   rather than once per pattern. Fix: compile first (catching re.error once),
   then iterate over header strings.

Also adds two new tests covering these cases, and adds docs page for
tag_regex routing with a Claude Code walk-through.

* refactor(tag_regex): remove unnecessary _healthy_list copy

* docs: merge tag_regex section into tag_routing.md, remove standalone page

- Add ## Regex-based tag routing (tag_regex) section to existing
  tag_routing.md instead of a separate page
- Remove tag_regex_routing.md standalone doc (odd UX to have a separate
  page for a sub-feature)
- Remove proxy/tag_regex_routing from sidebars.js
- Add match_any=False debug warning in tag_based_routing.py when regex
  routing fires under strict mode (regex always uses OR semantics)

* fix(tag_regex): address greptile review - security docs, strict-mode enforcement, validation order

- Strengthen security note in tag_routing.md: explicitly state User-Agent
  is client-supplied and can be set to any value; frame tag_regex as a
  traffic classification hint, not an access-control mechanism
- Move tag_regex startup validation before _add_deployment() so an invalid
  pattern never leaves partial router state
- Enforce match_any=False strict-tag policy: when a deployment has both
  tags and tag_regex and the strict tag check fails, skip the regex fallback
  rather than silently bypassing the operator's intent
- Extract per-deployment match logic into _match_deployment() helper to
  keep get_deployments_for_tag() readable
- Add two new tests: strict-mode blocks regex fallback, regex-only
  deployment still matches under match_any=False

* fix(ci): apply Black formatting to 14 files and stabilize flaky caplog tests

- Run Black formatter on 14 files that were failing the lint check
- Replace caplog-based assertions in TestAliasConflicts with
  unittest.mock.patch on verbose_logger.warning for xdist compatibility
- The caplog fixture can produce empty text in pytest-xdist workers
  in certain CI environments, causing flaky test failures

Co-authored-by: Ishaan Jaff <ishaan-jaff@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Ishaan Jaff <ishaan-jaff@users.noreply.github.com>

* fix: tiktoken cache nonroot offline (#23498)

* fix: restore offline tiktoken cache for non-root envs

Made-with: Cursor

* chore: mkdir for custom tiktoken cache dir

Made-with: Cursor

* test: patch tiktoken.get_encoding in custom-dir test to avoid network

Made-with: Cursor

* test: clear CUSTOM_TIKTOKEN_CACHE_DIR in helper for test isolation

Made-with: Cursor

* test: restore default_encoding module state after custom-dir test

Made-with: Cursor

* fix: normalize content_filtered finish_reason (#23564)

Map provider finish_reason "content_filtered" to the OpenAI-compatible "content_filter" and extend core_helpers tests to cover this case.

Made-with: Cursor

* fix: Fixes #23185 (#23647)

* fix: merge annotations from all streaming chunks in stream_chunk_builder

Previously, stream_chunk_builder only took annotations from the first
chunk that contained them, losing any annotations from later chunks.

This is a problem because providers like Gemini/Vertex AI send grounding
metadata (converted to annotations) in the final streaming chunk, while
other providers may spread annotations across multiple chunks.

Changes:
- Collect and merge annotations from ALL annotation-bearing chunks
  instead of only using the first one

---------

Co-authored-by: RoomWithOutRoof <166608075+Jah-yee@users.noreply.github.com>
Co-authored-by: Jah-yee <sparklab@outlook.com>
Co-authored-by: Ethan T. <ethanchang32@gmail.com>
Co-authored-by: Awais Qureshi <awais.qureshi@arbisoft.com>
Co-authored-by: Krish Dholakia <krrishdholakia@gmail.com>
Co-authored-by: Pradyumna Yadav <pradyumna.aky@gmail.com>
Co-authored-by: xianzongxie-stripe <87151258+xianzongxie-stripe@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Joe Reyna <joseph.reyna@gmail.com>
Co-authored-by: Ishaan Jaff <ishaanjaffer0324@gmail.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Ishaan Jaff <ishaan-jaff@users.noreply.github.com>
Co-authored-by: milan-berri <milan@berri.ai>
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.

1 participant