LCORE-1313 Add automatic header propagation to MCP servers using an allowlist#1222
Merged
tisnik merged 3 commits intolightspeed-core:mainfrom Feb 25, 2026
Conversation
Contributor
WalkthroughAdds automatic header propagation: a new per-MCP-server Changes
Sequence DiagramsequenceDiagram
participant Client as HTTP Client
participant Endpoint as Endpoint Handler
participant Utils as Response/Tool Utils
participant MCP as MCP Server
Client->>Endpoint: HTTP request (headers: x-rh-identity, x-request-id)
Endpoint->>Endpoint: capture request.headers
Endpoint->>Utils: prepare_responses_params(request_headers=...)
Utils->>Utils: extract_propagated_headers(mcp_server, request_headers)
Utils->>Utils: merge propagated headers with authorization_headers & MCP-HEADERS
Utils->>MCP: build MCP tool definitions with merged headers
MCP-->>Utils: tool config ready
Utils-->>Endpoint: prepared responses (with MCP tools)
Endpoint-->>Client: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
@tisnik Could you PTAL? Btw, in the snippet you referenced the principle was the same (extract headers & forward to MCP servers), but it just hardcoded |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/utils/mcp_headers.py (1)
99-121: Case-insensitive lookup is one-directional; document the Starlette dependency or normalize both sides.
request_headers.get(header_name.lower())only works because Starlette'sHeadersobject stores all keys in lowercase. Ifrequest_headersis a plaindictwith mixed-case keys (e.g.,{"X-RH-Identity": "val"}), the lookup silently misses the header. The docstring claims case-insensitive matching but the function implicitly depends on the caller normalizing keys to lowercase.Either add a note to the docstring clarifying this precondition, or make both sides truly case-insensitive:
♻️ Proposed fix — normalize both sides
- propagated: dict[str, str] = {} - for header_name in mcp_server.headers: - value = request_headers.get(header_name.lower()) - if value is not None: - propagated[header_name] = value - return propagated + # Build a lowercase-keyed view of request_headers for case-insensitive lookup + lower_request_headers = {k.lower(): v for k, v in request_headers.items()} + propagated: dict[str, str] = {} + for header_name in mcp_server.headers: + value = lower_request_headers.get(header_name.lower()) + if value is not None: + propagated[header_name] = value + return propagated🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/mcp_headers.py` around lines 99 - 121, The function extract_propagated_headers currently assumes request_headers keys are lowercased (as in Starlette.Headers) so request_headers.get(header_name.lower()) can miss mixed-case keys; make the lookup truly case-insensitive by normalizing request_headers keys before iterating (e.g., build a mapping like normalized_request = {k.lower(): v for k, v in request_headers.items()}) and then use normalized_request.get(header_name.lower()) when populating propagated, or alternatively update the docstring to explicitly state the precondition that request_headers must be lowercased; refer to extract_propagated_headers, mcp_server.headers and request_headers to locate the change.tests/unit/utils/test_mcp_headers.py (1)
252-262: Add the inverse case-insensitive test — allowlist lowercase, request header mixed-case.
test_case_insensitive_lookupverifies allowlist-uppercase → request-lowercase. The inverse (allowlist-lowercase → request-uppercase/mixed plain dict) is not tested and would currently fail, which would help document the Starlette-dependency assumption noted inextract_propagated_headers.def test_case_insensitive_lookup_uppercase_request_header(self) -> None: """Test direction: allowlist lowercase, request header has uppercase key (plain dict).""" server = ModelContextProtocolServer( name="rbac", url="http://rbac:8080", headers=["x-rh-identity"], ) # NOTE: this test would currently fail with a plain dict because # request_headers.get("x-rh-identity") does not find "X-RH-Identity". # In production this is fine because Starlette always lowercases headers. request_headers = {"X-RH-Identity": "identity-value"} # mixed-case plain dict result = extract_propagated_headers(server, request_headers) # Documents current behavior: relies on caller normalizing keys to lowercase assert result == {} # or {"x-rh-identity": "identity-value"} after the fixsrc/models/config.py (1)
525-537: Consider adding a@field_validatorto reject empty strings and duplicates inheaders.The field accepts any
list[str]without validation. Empty strings ("") would silently no-op during propagation, and duplicate entries are wasteful but undetected. As per coding guidelines,@field_validatorshould be used for custom validation in Pydantic configuration models.🛡️ Proposed validator
headers: list[str] = Field( default_factory=list, title="Propagated headers", description=(...), ) + + `@field_validator`("headers") + `@classmethod` + def validate_headers(cls, value: list[str]) -> list[str]: + """Validate propagated headers list: no empty strings, no duplicates.""" + for h in value: + if not h.strip(): + raise ValueError("Header names in 'headers' must not be empty or blank") + if len(value) != len({h.lower() for h in value}): + raise ValueError("Header names in 'headers' must be unique (case-insensitive)") + return valueAs per coding guidelines: "Use
@field_validatorand@model_validatorfor custom validation in Pydantic configuration models."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/config.py` around lines 525 - 537, Add a Pydantic `@field_validator` for the headers Field (the headers attribute) that inspects the provided list[str], rejects/raises ValueError for any empty-string entries, and rejects duplicates in a case-insensitive manner (e.g., by comparing lowercased values); return the validated/normalized list (or raise on invalid input) so the model never silently accepts "" or duplicate header names. Use the `@field_validator` on "headers" and reference the headers Field in src/models/config.py; ensure the validator signature accepts the raw list and returns the cleaned list or raises ValueError with a clear message.tests/unit/utils/test_responses.py (1)
590-617: Add a mixed-case precedence regression test.Current coverage proves non-overwrite for same-case names, but it doesn’t cover
Authorization(auth header) vsauthorization(allowlist). Adding that case will lock in the precedence contract and catch case-sensitive merge regressions.✅ Suggested test addition
+ `@pytest.mark.asyncio` + async def test_get_mcp_tools_auth_precedence_is_case_insensitive( + self, tmp_path: Path + ) -> None: + secret_file = tmp_path / "token.txt" + secret_file.write_text("secret-token") + + servers = [ + ModelContextProtocolServer( + name="rbac", + url="http://rbac:8080", + authorization_headers={"Authorization": str(secret_file)}, + headers=["authorization"], # same header, different case + ), + ] + request_headers = {"authorization": "request-auth-value"} + + tools = await get_mcp_tools( + servers, token=None, mcp_headers=None, request_headers=request_headers + ) + + assert len(tools) == 1 + assert tools[0]["headers"] == {"Authorization": "secret-token"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/utils/test_responses.py` around lines 590 - 617, Add a regression test to tests/unit/utils/test_responses.py that verifies case-mixed header precedence: create a ModelContextProtocolServer with authorization_headers containing "Authorization" (pointing to a token file) and headers allowlist including "Authorization" and "x-rh-identity", then call get_mcp_tools with request_headers containing lowercase "authorization" and "x-rh-identity"; assert the resulting tool's headers keep the Authorization value from authorization_headers (the file token) and include the propagated "x-rh-identity" value from request_headers; name the test something like test_get_mcp_tools_mixed_case_precedence and use the existing patterns (tmp_path fixture, secret file, servers list) so it catches case-sensitive merge regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/responses.py`:
- Around line 434-440: The current loop in the mcp_server header propagation
uses a case-sensitive membership check which allows duplicate semantic headers
with different casing to coexist; change the check in the block that iterates
propagated headers (around extract_propagated_headers usage) to perform a
case-insensitive precedence test: compute a set of existing header names
normalized to lowercase from headers (and include any authorization_headers set
if present), then only add a propagated header when h_name.lower() is not in
that lowercase set; preserve the original propagated header casing when
inserting into headers but use the lowercase membership test to enforce "auth
headers win" precedence.
---
Nitpick comments:
In `@src/models/config.py`:
- Around line 525-537: Add a Pydantic `@field_validator` for the headers Field
(the headers attribute) that inspects the provided list[str], rejects/raises
ValueError for any empty-string entries, and rejects duplicates in a
case-insensitive manner (e.g., by comparing lowercased values); return the
validated/normalized list (or raise on invalid input) so the model never
silently accepts "" or duplicate header names. Use the `@field_validator` on
"headers" and reference the headers Field in src/models/config.py; ensure the
validator signature accepts the raw list and returns the cleaned list or raises
ValueError with a clear message.
In `@src/utils/mcp_headers.py`:
- Around line 99-121: The function extract_propagated_headers currently assumes
request_headers keys are lowercased (as in Starlette.Headers) so
request_headers.get(header_name.lower()) can miss mixed-case keys; make the
lookup truly case-insensitive by normalizing request_headers keys before
iterating (e.g., build a mapping like normalized_request = {k.lower(): v for k,
v in request_headers.items()}) and then use
normalized_request.get(header_name.lower()) when populating propagated, or
alternatively update the docstring to explicitly state the precondition that
request_headers must be lowercased; refer to extract_propagated_headers,
mcp_server.headers and request_headers to locate the change.
In `@tests/unit/utils/test_responses.py`:
- Around line 590-617: Add a regression test to
tests/unit/utils/test_responses.py that verifies case-mixed header precedence:
create a ModelContextProtocolServer with authorization_headers containing
"Authorization" (pointing to a token file) and headers allowlist including
"Authorization" and "x-rh-identity", then call get_mcp_tools with
request_headers containing lowercase "authorization" and "x-rh-identity"; assert
the resulting tool's headers keep the Authorization value from
authorization_headers (the file token) and include the propagated
"x-rh-identity" value from request_headers; name the test something like
test_get_mcp_tools_mixed_case_precedence and use the existing patterns (tmp_path
fixture, secret file, servers list) so it catches case-sensitive merge
regressions.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
README.mddocs/config.mdexamples/lightspeed-stack-mcp-servers.yamlsrc/app/endpoints/a2a.pysrc/app/endpoints/query.pysrc/app/endpoints/rlsapi_v1.pysrc/app/endpoints/streaming_query.pysrc/models/config.pysrc/utils/mcp_headers.pysrc/utils/responses.pytests/unit/utils/test_mcp_headers.pytests/unit/utils/test_responses.py
max-svistunov
force-pushed
the
lcore-1313-mcp-header-allowlist-propagation
branch
from
5d3d1d7 to
256b520
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/utils/test_responses.py (1)
624-795: Optional: reduce repeated setup in the new propagation tests.The new cases are solid; extracting a tiny server/config helper (or parameterizing common setup) would make maintenance easier as scenarios grow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/utils/test_responses.py` around lines 624 - 795, Several tests repeat the same MCP server/config setup; extract a small helper or fixture (e.g., a function like make_mcp_server or a pytest fixture create_mcp_config) to construct ModelContextProtocolServer instances and patch utils.responses.configuration once; update tests such as test_get_mcp_tools_with_propagated_headers, test_get_mcp_tools_propagated_headers_do_not_overwrite_auth_headers, test_get_mcp_tools_propagated_headers_missing_from_request, test_get_mcp_tools_propagated_headers_no_request_headers, test_get_mcp_tools_propagated_headers_additive_with_mcp_headers, and test_get_mcp_tools_mixed_case_precedence to call the helper/fixture instead of duplicating the mock_config/mocker.patch logic, preserving unique parameters like headers, authorization_headers, and names per test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/utils/test_responses.py`:
- Around line 624-795: Several tests repeat the same MCP server/config setup;
extract a small helper or fixture (e.g., a function like make_mcp_server or a
pytest fixture create_mcp_config) to construct ModelContextProtocolServer
instances and patch utils.responses.configuration once; update tests such as
test_get_mcp_tools_with_propagated_headers,
test_get_mcp_tools_propagated_headers_do_not_overwrite_auth_headers,
test_get_mcp_tools_propagated_headers_missing_from_request,
test_get_mcp_tools_propagated_headers_no_request_headers,
test_get_mcp_tools_propagated_headers_additive_with_mcp_headers, and
test_get_mcp_tools_mixed_case_precedence to call the helper/fixture instead of
duplicating the mock_config/mocker.patch logic, preserving unique parameters
like headers, authorization_headers, and names per test.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
README.mddocs/config.mdexamples/lightspeed-stack-mcp-servers.yamlsrc/app/endpoints/a2a.pysrc/app/endpoints/query.pysrc/app/endpoints/rlsapi_v1.pysrc/app/endpoints/streaming_query.pysrc/models/config.pysrc/utils/mcp_headers.pysrc/utils/responses.pytests/unit/app/endpoints/test_query.pytests/unit/app/endpoints/test_rlsapi_v1.pytests/unit/app/endpoints/test_streaming_query.pytests/unit/models/config/test_dump_configuration.pytests/unit/models/config/test_model_context_protocol_server.pytests/unit/utils/test_mcp_headers.pytests/unit/utils/test_responses.py
🚧 Files skipped from review as they are similar to previous changes (4)
- src/app/endpoints/streaming_query.py
- src/models/config.py
- examples/lightspeed-stack-mcp-servers.yaml
- src/app/endpoints/rlsapi_v1.py
jrobertboos
approved these changes
Feb 25, 2026
| no_tools: bool | None, | ||
| token: str, | ||
| mcp_headers: McpHeaders | None = None, | ||
| request_headers: Optional[Mapping[str, str]] = None, |
Contributor
There was a problem hiding this comment.
Personally I like Optional but at least in this file everything else uses pipes so it might be worth it to stay style consistent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add automatic header propagation to MCP servers via allowlist configuration.
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
headers: [x-rh-identity, x-rh-insights-request-id]for themock server
Summary by CodeRabbit
New Features
Documentation
Tests