[Fix] Restrict x-pass- header forwarding for credential and protocol headers#25916
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
There was a problem hiding this comment.
High: Incomplete credential header protection in x-pass- header allowlist
This PR adds a blocklist to prevent x-pass- prefixed headers from overwriting certain sensitive headers (authorization, api-key, host, content-length). However, several provider-specific credential headers are missing from the protected set. An authenticated user can send x-pass-x-api-key: <value> to overwrite the Anthropic x-api-key header, or x-pass-x-goog-api-key: <value> to overwrite Google's credential header, bypassing the operator's configured API keys and the proxy's billing/spend tracking.
| # Always process x-pass- prefixed headers (strip prefix and forward) | ||
| # Process x-pass- prefixed headers (strip prefix and forward) | ||
| # Certain protocol-level and credential headers are excluded from this mechanism. | ||
| _PROTECTED_HEADERS = {"authorization", "api-key", "host", "content-length"} |
There was a problem hiding this comment.
High: Incomplete credential header blocklist
The protected set misses provider-specific credential headers. An authenticated user can send x-pass-x-api-key: attacker-key to overwrite the Anthropic x-api-key header set by validate_environment, or x-pass-x-goog-api-key: key for Google AI Studio. Since forward_headers_from_request processes x-pass- headers after auth headers are set (overwriting them at line 68), this lets a user substitute the operator's downstream API key with their own, bypassing spend tracking and billing controls.
Consider either switching to an allowlist approach, or at minimum adding the known provider credential headers:
| _PROTECTED_HEADERS = {"authorization", "api-key", "host", "content-length"} | |
| _PROTECTED_HEADERS = {"authorization", "api-key", "host", "content-length", "x-api-key", "x-goog-api-key"} |
A prefix-based block on x-amz- would also prevent AWS SigV4 header tampering for Bedrock passthrough, though sign_request may re-sign after this point.
Greptile SummaryThis PR fixes a header-injection vulnerability in Confidence Score: 5/5Safe to merge — security fix is correct, all previous review concerns have been addressed, and a comprehensive test suite is included. All three P2 issues from the prior review cycle (no tests, no logging, per-call set allocation / casing inconsistency) are now resolved: constants are module-level frozensets, a debug log is emitted on skip, header keys are normalized to lowercase on write, and a full unit test covers every protected category plus the happy-path non-protected case. No new P0 or P1 issues found. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/passthrough/utils.py | Adds module-level _PASS_THROUGH_PROTECTED_HEADERS frozenset and _PASS_THROUGH_PROTECTED_HEADER_PREFIXES tuple; modifies forward_headers_from_request to skip protected credential/protocol headers with lowercase normalization of stripped names and a debug log on skip — addresses all prior review concerns. |
| tests/test_litellm/proxy/pass_through_endpoints/test_vertex_passthrough_load_balancing.py | Adds test_forward_headers_from_request_protected_headers_not_overwritten covering all six named protected headers, the x-amz-* prefix block, the still-works non-protected path, and lowercase key normalization — comprehensive coverage of the new guard. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming request header] --> B{Starts with x-pass- prefix?}
B -- No --> Z[Skip]
B -- Yes --> C[Strip prefix and normalize to lowercase]
C --> D{In _PASS_THROUGH_PROTECTED_HEADERS?}
D -- Yes --> E[Log debug and skip]
D -- No --> F{Starts with protected prefix e.g. x-amz-?}
F -- Yes --> E
F -- No --> G[Write to outbound headers dict]
Reviews (2): Last reviewed commit: "fix: extend x-pass- header protection to..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| actual_header_name = header_name[len(PASS_THROUGH_HEADER_PREFIX) :] | ||
| if actual_header_name.lower() in _PROTECTED_HEADERS: | ||
| continue | ||
| headers[actual_header_name] = header_value |
There was a problem hiding this comment.
No test in the committed test suite exercises the _PROTECTED_HEADERS skip path. The PR description states testing was done with standalone scripts in ~/.secfix-scratch/xpass-header/ that were not committed. This means regressions in this security fix are invisible to CI.
Per the project's testing guidelines, a test (or several) should be added to tests/test_litellm/proxy/pass_through_endpoints/test_vertex_passthrough_load_balancing.py covering:
x-pass-authorizationis silently dropped (not written intoheaders)x-pass-api-keyis silently droppedx-pass-hostandx-pass-content-lengthare silently dropped- A non-protected
x-pass-*header still works as before
| # Always process x-pass- prefixed headers (strip prefix and forward) | ||
| # Process x-pass- prefixed headers (strip prefix and forward) | ||
| # Certain protocol-level and credential headers are excluded from this mechanism. | ||
| _PROTECTED_HEADERS = {"authorization", "api-key", "host", "content-length"} |
There was a problem hiding this comment.
If a user was intentionally forwarding an authorization header to a downstream service that expects a different credential than LiteLLM's own auth (e.g., x-pass-authorization: Bearer <downstream-token>), this change silently drops it without any log message, warning, or config flag to opt out. Per the project's backwards-compatibility rule, a behaviour change that drops previously-accepted input should ideally be guarded by a feature flag or at minimum log a warning so the caller knows the header was skipped.
if actual_header_name.lower() in _PROTECTED_HEADERS:
verbose_proxy_logger.warning(
"Skipping x-pass- header %s: maps to a protected header name",
header_name,
)
continueRule Used: What: avoid backwards-incompatible changes without... (source)
| # Always process x-pass- prefixed headers (strip prefix and forward) | ||
| # Process x-pass- prefixed headers (strip prefix and forward) | ||
| # Certain protocol-level and credential headers are excluded from this mechanism. | ||
| _PROTECTED_HEADERS = {"authorization", "api-key", "host", "content-length"} |
There was a problem hiding this comment.
_PROTECTED_HEADERS should be a module-level constant
Defining _PROTECTED_HEADERS as a local variable inside forward_headers_from_request re-creates the set on every call and makes the list invisible to callers who might want to inspect or extend it. Moving it to module level matches common Python conventions and eliminates the per-call allocation.
| _PROTECTED_HEADERS = {"authorization", "api-key", "host", "content-length"} | |
| _PROTECTED_HEADERS: frozenset = frozenset( | |
| {"authorization", "api-key", "host", "content-length"} | |
| ) | |
| class BasePassthroughUtils: |
| for header_name, header_value in request_headers.items(): | ||
| if header_name.lower().startswith(PASS_THROUGH_HEADER_PREFIX): | ||
| # Strip the 'x-pass-' prefix to get the actual header name | ||
| actual_header_name = header_name[len(PASS_THROUGH_HEADER_PREFIX) :] |
There was a problem hiding this comment.
actual_header_name retains the casing of the incoming request header (e.g. x-pass-Anthropic-Beta → Anthropic-Beta), because only the .lower() comparison against _PROTECTED_HEADERS is normalized — the value written into headers on line 68 still uses actual_header_name as-is. Two callers sending x-pass-Custom-Header and x-pass-custom-header would produce distinct keys in the output dict. If the intent is case-insensitive uniqueness in the output, normalise the key on write as well:
| actual_header_name = header_name[len(PASS_THROUGH_HEADER_PREFIX) :] | |
| actual_header_name = header_name[len(PASS_THROUGH_HEADER_PREFIX):].lower() |
…headers and add tests - Move protected-headers set to module level as a frozenset - Add x-api-key, x-goog-api-key to protected set (provider credential headers) - Block x-amz- prefix to cover AWS SigV4 signing headers - Normalize forwarded header names to lowercase on write - Log at debug level when a protected header is skipped - Add unit test covering protected-header drop and non-protected forwarding
yuneng-berri
temporarily deployed
to
integration-postgres
GitHub Actions
Inactive
yuneng-berri
temporarily deployed
to
integration-postgres
GitHub Actions
Inactive
yuneng-berri
had a problem deploying
to
integration-postgres
GitHub Actions
Error
yuneng-berri
temporarily deployed
to
integration-postgres
GitHub Actions
Inactive
yuneng-berri
changed the base branch from
litellm_internal_staging
to
litellm_yj_apr16
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
1 participant
Summary
The
x-pass-header prefix mechanism inBasePassthroughUtils.forward_headers_from_requestprocessed all incoming headers with that prefix unconditionally, including headers whose stripped names resolve to credential or protocol-level header names (e.g.authorization,api-key,host,content-length). This allowed the forwarded value to overwrite entries already present in the outbound headers dict.The fix introduces a small set of protected header names that are skipped during
x-pass-processing. Non-protected headers continue to work as before.Changes
Added
_PROTECTED_HEADERS(authorization,api-key,host,content-length) toBasePassthroughUtils.forward_headers_from_request. Anyx-pass-header whose stripped name matches a protected entry is skipped rather than written into the outbound headers dict.Testing
~/.secfix-scratch/xpass-header/(not committed).uv run pytest tests/test_litellm/proxy/pass_through_endpoints/test_vertex_passthrough_load_balancing.py -v— 9 passed, no regressions.Type
🐛 Bug Fix