From a5220847ee83eb69b4375dac61444da6e7f53ed8 Mon Sep 17 00:00:00 2001 From: Maxim Svistunov Date: Thu, 5 Mar 2026 11:49:03 +0100 Subject: [PATCH 1/5] Fix MCP authorization loss during model_dump() serialization Inject authorization field for MCP tools after it is stripped by Pydantic's exclude=True in ResponsesApiParams.model_dump() https://github.com/lightspeed-core/lightspeed-stack/issues/1269 https://issues.redhat.com/browse/LCORE-1414 --- src/utils/types.py | 17 +++++ tests/unit/utils/test_types.py | 112 +++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+) diff --git a/src/utils/types.py b/src/utils/types.py index 220a85239..c20f81a1b 100644 --- a/src/utils/types.py +++ b/src/utils/types.py @@ -222,6 +222,23 @@ class ResponsesApiParams(BaseModel): description="Extra HTTP headers to send with the request (e.g. x-llamastack-provider-data)", ) + def model_dump(self, *args: Any, **kwargs: Any) -> dict[str, Any]: + """Serialize params, re-injecting MCP authorization stripped by exclude=True. + + llama-stack-api marks ``InputToolMCP.authorization`` with + ``Field(exclude=True)`` to prevent token leakage in API responses. + The base ``model_dump()`` therefore strips the field, but we need it + in the request payload so llama-stack server can authenticate with + MCP servers. See LCORE-1414 / GitHub issue #1269. + """ + result = super().model_dump(*args, **kwargs) + if self.tools and result.get("tools"): + for i, tool in enumerate(self.tools): + authorization = getattr(tool, "authorization", None) + if authorization is not None: + result["tools"][i]["authorization"] = authorization + return result + class ToolCallSummary(BaseModel): """Model representing a tool call made during response generation (for tool_calls list).""" diff --git a/tests/unit/utils/test_types.py b/tests/unit/utils/test_types.py index 882a83711..1eb989d1f 100644 --- a/tests/unit/utils/test_types.py +++ b/tests/unit/utils/test_types.py @@ -2,6 +2,10 @@ import pytest from llama_stack_api import ImageContentItem, TextContentItem, URL, _URLOrData +from llama_stack_api.openai_responses import ( + OpenAIResponseInputToolFileSearch as InputToolFileSearch, + OpenAIResponseInputToolMCP as InputToolMCP, +) from pydantic import AnyUrl, ValidationError from pytest_mock import MockerFixture @@ -9,6 +13,7 @@ from utils.types import ( GraniteToolParser, ReferencedDocument, + ResponsesApiParams, ToolCallSummary, ToolResultSummary, content_to_str, @@ -194,3 +199,110 @@ def test_constructor_partial_fields(self) -> None: doc = ReferencedDocument(doc_title="Test Title") assert doc.doc_url is None assert doc.doc_title == "Test Title" + + +class TestResponsesApiParamsModelDump: + """Tests for ResponsesApiParams.model_dump() MCP authorization fix. + + Regression tests for LCORE-1414 / GitHub issue #1269: llama-stack-api's + InputToolMCP.authorization has Field(exclude=True), causing the base + model_dump() to silently strip authorization tokens. + """ + + def _make_params(self, tools: list) -> ResponsesApiParams: + """Build minimal ResponsesApiParams with given tools.""" + return ResponsesApiParams( + input="test question", + model="provider/model", + conversation="conv-id", + store=False, + stream=False, + tools=tools, + ) + + def test_mcp_authorization_survives_model_dump(self) -> None: + """Test that MCP authorization is re-injected after model_dump().""" + tool = InputToolMCP( + server_label="auth-server", + server_url="http://localhost:3000", + require_approval="never", + authorization="my-secret-token", + ) + assert tool.authorization == "my-secret-token" + assert "authorization" not in tool.model_dump() + + params = self._make_params([tool]) + dumped = params.model_dump(exclude_none=True) + assert dumped["tools"][0]["authorization"] == "my-secret-token" + + def test_mcp_authorization_none_not_injected(self) -> None: + """Test that None authorization is not added to the dump.""" + tool = InputToolMCP( + server_label="no-auth-server", + server_url="http://localhost:3000", + require_approval="never", + ) + params = self._make_params([tool]) + dumped = params.model_dump(exclude_none=True) + assert "authorization" not in dumped["tools"][0] + + def test_non_mcp_tools_unaffected(self) -> None: + """Test that non-MCP tools are not modified by the override.""" + tool = InputToolFileSearch( + type="file_search", + vector_store_ids=["vs-1"], + ) + params = self._make_params([tool]) + dumped = params.model_dump(exclude_none=True) + assert "authorization" not in dumped["tools"][0] + + def test_mixed_tools_only_mcp_gets_authorization(self) -> None: + """Test mixed tool list: only MCP tools get authorization re-injected.""" + mcp_tool = InputToolMCP( + server_label="auth-server", + server_url="http://localhost:3000", + require_approval="never", + authorization="secret", + ) + file_tool = InputToolFileSearch( + type="file_search", + vector_store_ids=["vs-1"], + ) + params = self._make_params([file_tool, mcp_tool]) + dumped = params.model_dump(exclude_none=True) + + assert "authorization" not in dumped["tools"][0] + assert dumped["tools"][1]["authorization"] == "secret" + + def test_multiple_mcp_tools_each_preserves_authorization(self) -> None: + """Test that each MCP tool gets its own authorization re-injected.""" + tool_a = InputToolMCP( + server_label="server-a", + server_url="http://a:3000", + require_approval="never", + authorization="token-a", + ) + tool_b = InputToolMCP( + server_label="server-b", + server_url="http://b:3000", + require_approval="never", + authorization="token-b", + ) + params = self._make_params([tool_a, tool_b]) + dumped = params.model_dump(exclude_none=True) + + assert dumped["tools"][0]["authorization"] == "token-a" + assert dumped["tools"][1]["authorization"] == "token-b" + + def test_no_tools_does_not_error(self) -> None: + """Test that model_dump() works when tools is None.""" + params = ResponsesApiParams( + input="test", + model="provider/model", + conversation="conv-id", + store=False, + stream=False, + tools=None, + ) + dumped = params.model_dump(exclude_none=True) + assert "tools" not in dumped From 8b2759b07afaf885889c5bf81f596bbf1ec8fded Mon Sep 17 00:00:00 2001 From: Maxim Svistunov Date: Thu, 5 Mar 2026 12:42:53 +0100 Subject: [PATCH 2/5] Guard model_dump() auth re-injection against tool list shape mismatch --- src/utils/types.py | 14 +++++++++----- tests/unit/utils/test_types.py | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/utils/types.py b/src/utils/types.py index c20f81a1b..a78370a0b 100644 --- a/src/utils/types.py +++ b/src/utils/types.py @@ -232,11 +232,15 @@ def model_dump(self, *args: Any, **kwargs: Any) -> dict[str, Any]: MCP servers. See LCORE-1414 / GitHub issue #1269. """ result = super().model_dump(*args, **kwargs) - if self.tools and result.get("tools"): - for i, tool in enumerate(self.tools): - authorization = getattr(tool, "authorization", None) - if authorization is not None: - result["tools"][i]["authorization"] = authorization + dumped_tools = result.get("tools") + if not self.tools or not isinstance(dumped_tools, list): + return result + if len(dumped_tools) != len(self.tools): + return result + for tool, dumped_tool in zip(self.tools, dumped_tools): + authorization = getattr(tool, "authorization", None) + if authorization is not None and isinstance(dumped_tool, dict): + dumped_tool["authorization"] = authorization return result diff --git a/tests/unit/utils/test_types.py b/tests/unit/utils/test_types.py index 1eb989d1f..232f31834 100644 --- a/tests/unit/utils/test_types.py +++ b/tests/unit/utils/test_types.py @@ -294,6 +294,26 @@ def test_multiple_mcp_tools_each_preserves_authorization(self) -> None: assert dumped["tools"][0]["authorization"] == "token-a" assert dumped["tools"][1]["authorization"] == "token-b" + def test_exclude_changing_tool_list_shape_skips_reinjection(self) -> None: + """Test that exclude removing tool indices does not mis-assign authorization.""" + tool_a = InputToolMCP( + server_label="server-a", + server_url="http://a:3000", + require_approval="never", + authorization="token-a", + ) + tool_b = InputToolMCP( + server_label="server-b", + server_url="http://b:3000", + require_approval="never", + authorization="token-b", + ) + params = self._make_params([tool_a, tool_b]) + dumped = params.model_dump(exclude={"tools": {0}}) + assert len(dumped["tools"]) == 1 + assert dumped["tools"][0]["server_label"] == "server-b" + assert "authorization" not in dumped["tools"][0] + def test_no_tools_does_not_error(self) -> None: """Test that model_dump() works when tools is None.""" params = ResponsesApiParams( From 45e49d547ebb7bdff2230bccec36aa9d6ba36538 Mon Sep 17 00:00:00 2001 From: Maxim Svistunov Date: Thu, 5 Mar 2026 13:37:51 +0100 Subject: [PATCH 3/5] Add e2e test for file-based MCP authorization --- docker-compose.yaml | 1 + .../lightspeed-stack-mcp-file-auth.yaml | 26 +++++++++++++++++++ tests/e2e/features/environment.py | 15 +++++++++++ tests/e2e/features/mcp_file_auth.feature | 18 +++++++++++++ tests/e2e/secrets/mcp-token | 1 + tests/e2e/test_list.txt | 1 + 6 files changed, 62 insertions(+) create mode 100644 tests/e2e/configuration/server-mode/lightspeed-stack-mcp-file-auth.yaml create mode 100644 tests/e2e/features/mcp_file_auth.feature create mode 100644 tests/e2e/secrets/mcp-token diff --git a/docker-compose.yaml b/docker-compose.yaml index 85a38bc62..03bd6450d 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -81,6 +81,7 @@ services: - "8080:8080" volumes: - ./lightspeed-stack.yaml:/app-root/lightspeed-stack.yaml:z + - ./tests/e2e/secrets/mcp-token:/tmp/mcp-secret-token:ro environment: - OPENAI_API_KEY=${OPENAI_API_KEY} # Azure Entra ID credentials (AZURE_API_KEY is obtained dynamically) diff --git a/tests/e2e/configuration/server-mode/lightspeed-stack-mcp-file-auth.yaml b/tests/e2e/configuration/server-mode/lightspeed-stack-mcp-file-auth.yaml new file mode 100644 index 000000000..d39f55399 --- /dev/null +++ b/tests/e2e/configuration/server-mode/lightspeed-stack-mcp-file-auth.yaml @@ -0,0 +1,26 @@ +name: Lightspeed Core Service (LCS) +service: + host: 0.0.0.0 + port: 8080 + auth_enabled: false + workers: 1 + color_log: true + access_log: true +llama_stack: + # Server mode - connects to separate llama-stack service + use_as_library_client: false + url: http://llama-stack:8321 + api_key: xyzzy +user_data_collection: + feedback_enabled: true + feedback_storage: "/tmp/data/feedback" + transcripts_enabled: true + transcripts_storage: "/tmp/data/transcripts" +authentication: + module: "noop" +mcp_servers: + - name: "mcp-file-auth" + provider_id: "model-context-protocol" + url: "http://mock-mcp:3001" + authorization_headers: + Authorization: "/tmp/mcp-secret-token" diff --git a/tests/e2e/features/environment.py b/tests/e2e/features/environment.py index bf434f4b4..0ca8781d0 100644 --- a/tests/e2e/features/environment.py +++ b/tests/e2e/features/environment.py @@ -53,6 +53,10 @@ "tests/e2e/configuration/{mode_dir}/lightspeed-stack-auth-rh-identity.yaml", "tests/e2e-prow/rhoai/configs/lightspeed-stack-auth-rh-identity.yaml", ), + "mcp-file-auth": ( + "tests/e2e/configuration/{mode_dir}/lightspeed-stack-mcp-file-auth.yaml", + "tests/e2e-prow/rhoai/configs/lightspeed-stack-mcp-file-auth.yaml", + ), } @@ -383,6 +387,12 @@ def before_feature(context: Context, feature: Feature) -> None: switch_config(context.feature_config) restart_container("lightspeed-stack") + if "MCPFileAuth" in feature.tags: + context.feature_config = _get_config_path("mcp-file-auth", mode_dir) + context.default_config_backup = create_config_backup("lightspeed-stack.yaml") + switch_config(context.feature_config) + restart_container("lightspeed-stack") + def after_feature(context: Context, feature: Feature) -> None: """Run after each feature file is exercised. @@ -415,3 +425,8 @@ def after_feature(context: Context, feature: Feature) -> None: switch_config(context.default_config_backup) restart_container("lightspeed-stack") remove_config_backup(context.default_config_backup) + + if "MCPFileAuth" in feature.tags: + switch_config(context.default_config_backup) + restart_container("lightspeed-stack") + remove_config_backup(context.default_config_backup) diff --git a/tests/e2e/features/mcp_file_auth.feature b/tests/e2e/features/mcp_file_auth.feature new file mode 100644 index 000000000..9f2a200a8 --- /dev/null +++ b/tests/e2e/features/mcp_file_auth.feature @@ -0,0 +1,18 @@ +@MCPFileAuth +Feature: MCP file-based authorization tests + + Regression tests for LCORE-1414: MCP authorization tokens configured via + file-based authorization_headers must survive model_dump() serialization + and reach the MCP server as a valid Bearer token. + + Background: + Given The service is started locally + And REST API service prefix is /v1 + + Scenario: Query succeeds with file-based MCP authorization + Given The system is in default state + When I use "query" to ask question + """ + {"query": "Say hello", "model": "{MODEL}", "provider": "{PROVIDER}"} + """ + Then The status code of the response is 200 diff --git a/tests/e2e/secrets/mcp-token b/tests/e2e/secrets/mcp-token new file mode 100644 index 000000000..c05b9cec9 --- /dev/null +++ b/tests/e2e/secrets/mcp-token @@ -0,0 +1 @@ +test-secret-token diff --git a/tests/e2e/test_list.txt b/tests/e2e/test_list.txt index 42dc6b22a..3f94d09e0 100644 --- a/tests/e2e/test_list.txt +++ b/tests/e2e/test_list.txt @@ -15,4 +15,5 @@ features/rlsapi_v1_errors.feature features/streaming_query.feature features/rest_api.feature features/mcp.feature +features/mcp_file_auth.feature features/models.feature From febd2523c57b786270eac5642510bbba5083df69 Mon Sep 17 00:00:00 2001 From: Maxim Svistunov Date: Thu, 5 Mar 2026 14:43:32 +0100 Subject: [PATCH 4/5] Add body assertion and library-mode config for MCP file auth e2e --- .../lightspeed-stack-mcp-file-auth.yaml | 25 +++++++++++++++++++ tests/e2e/features/mcp_file_auth.feature | 1 + 2 files changed, 26 insertions(+) create mode 100644 tests/e2e/configuration/library-mode/lightspeed-stack-mcp-file-auth.yaml diff --git a/tests/e2e/configuration/library-mode/lightspeed-stack-mcp-file-auth.yaml b/tests/e2e/configuration/library-mode/lightspeed-stack-mcp-file-auth.yaml new file mode 100644 index 000000000..1ff0d425e --- /dev/null +++ b/tests/e2e/configuration/library-mode/lightspeed-stack-mcp-file-auth.yaml @@ -0,0 +1,25 @@ +name: Lightspeed Core Service (LCS) +service: + host: 0.0.0.0 + port: 8080 + auth_enabled: false + workers: 1 + color_log: true + access_log: true +llama_stack: + # Library mode - embeds llama-stack as library + use_as_library_client: true + library_client_config_path: run.yaml +user_data_collection: + feedback_enabled: true + feedback_storage: "/tmp/data/feedback" + transcripts_enabled: true + transcripts_storage: "/tmp/data/transcripts" +authentication: + module: "noop" +mcp_servers: + - name: "mcp-file-auth" + provider_id: "model-context-protocol" + url: "http://mock-mcp:3001" + authorization_headers: + Authorization: "/tmp/mcp-secret-token" diff --git a/tests/e2e/features/mcp_file_auth.feature b/tests/e2e/features/mcp_file_auth.feature index 9f2a200a8..c6e6ab386 100644 --- a/tests/e2e/features/mcp_file_auth.feature +++ b/tests/e2e/features/mcp_file_auth.feature @@ -16,3 +16,4 @@ Feature: MCP file-based authorization tests {"query": "Say hello", "model": "{MODEL}", "provider": "{PROVIDER}"} """ Then The status code of the response is 200 + And The body of the response contains mock_tool_e2e From 41a7a5895c6ced95727765a26185b016f954c140 Mon Sep 17 00:00:00 2001 From: Maxim Svistunov Date: Thu, 5 Mar 2026 15:42:18 +0100 Subject: [PATCH 5/5] Skip MCP file auth e2e in library mode, use directive query --- tests/e2e/features/mcp_file_auth.feature | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/e2e/features/mcp_file_auth.feature b/tests/e2e/features/mcp_file_auth.feature index c6e6ab386..455f0740c 100644 --- a/tests/e2e/features/mcp_file_auth.feature +++ b/tests/e2e/features/mcp_file_auth.feature @@ -9,11 +9,12 @@ Feature: MCP file-based authorization tests Given The service is started locally And REST API service prefix is /v1 + @skip-in-library-mode Scenario: Query succeeds with file-based MCP authorization Given The system is in default state When I use "query" to ask question """ - {"query": "Say hello", "model": "{MODEL}", "provider": "{PROVIDER}"} + {"query": "Use the mock_tool_e2e tool to send the message 'hello'", "model": "{MODEL}", "provider": "{PROVIDER}"} """ Then The status code of the response is 200 And The body of the response contains mock_tool_e2e