(fix) lcore-1461

uncommented tests

fixed k8s config

fixed black

fixed library e2e

addressed coderabbit

addressed comments

fixed "is not None"

Author: jrobertboos

docker-compose-library.yaml

@@ -38,6 +38,8 @@ services:
       - ./run.yaml:/app-root/run.yaml:Z
       - ${GCP_KEYS_PATH:-./tmp/.gcp-keys-dummy}:/opt/app-root/.gcp-keys:ro
       - ./tests/e2e/rag:/opt/app-root/src/.llama/storage/rag:Z
+      - ./tests/e2e/secrets/mcp-token:/tmp/mcp-token:ro
+      - ./tests/e2e/secrets/invalid-mcp-token:/tmp/invalid-mcp-token:ro
     environment:
       # LLM Provider API Keys
       - BRAVE_SEARCH_API_KEY=${BRAVE_SEARCH_API_KEY:-}

docker-compose.yaml

@@ -81,8 +81,8 @@ services:
       - "8080:8080"
     volumes:
       - ./lightspeed-stack.yaml:/app-root/lightspeed-stack.yaml:z
-      - ./tests/e2e/secrets/mcp-token:/tmp/mcp-secret-token:ro
-      - ./tests/e2e/secrets/invalid-mcp-token:/tmp/invalid-mcp-secret-token:ro
+      - ./tests/e2e/secrets/mcp-token:/tmp/mcp-token:ro
+      - ./tests/e2e/secrets/invalid-mcp-token:/tmp/invalid-mcp-token:ro
     environment:
       - OPENAI_API_KEY=${OPENAI_API_KEY}
       # Azure Entra ID credentials (AZURE_API_KEY is obtained dynamically)

src/app/endpoints/tools.py

@@ -20,7 +20,7 @@
     UnauthorizedResponse,
 )
 from utils.endpoints import check_configuration_loaded
-from utils.mcp_headers import McpHeaders, mcp_headers_dependency
+from utils.mcp_headers import McpHeaders, build_mcp_headers, mcp_headers_dependency
 from utils.mcp_oauth_probe import check_mcp_auth
 from utils.tool_formatter import format_tools_list
 
@@ -115,15 +115,18 @@ async def tools_endpoint_handler(  # pylint: disable=too-many-locals,too-many-st
         ToolsResponse: An object containing the consolidated list of available tools
         with metadata including tool name, description, parameters, and server source.
     """
-    # Used only by the middleware
-    _ = auth
+    _, _, _, token = auth
 
     # Nothing interesting in the request
     _ = request
 
     check_configuration_loaded(configuration)
 
-    await check_mcp_auth(configuration, mcp_headers)
+    complete_mcp_headers = build_mcp_headers(
+        configuration, mcp_headers, request.headers, token
+    )
+
+    await check_mcp_auth(configuration, complete_mcp_headers)
 
     toolgroups_response = []
     try:
@@ -145,7 +148,7 @@ async def tools_endpoint_handler(  # pylint: disable=too-many-locals,too-many-st
     for toolgroup in toolgroups_response:
         try:
             # Get tools for each toolgroup
-            headers = mcp_headers.get(toolgroup.identifier, {})
+            headers = complete_mcp_headers.get(toolgroup.identifier, {})
             authorization = headers.pop("Authorization", None)
 
             tools_response = await client.tools.list(

src/utils/mcp_headers.py

@@ -2,10 +2,12 @@
 
 import json
 from collections.abc import Mapping
+from typing import Optional
 from urllib.parse import urlparse
 
 from fastapi import Request
 
+import constants
 from configuration import AppConfig
 from log import get_logger
 from models.config import ModelContextProtocolServer
@@ -121,3 +123,132 @@ def extract_propagated_headers(
         if value is not None:
             propagated[header_name] = value
     return propagated
+
+
+def find_unresolved_auth_headers(
+    configured: Mapping[str, str],
+    resolved: Mapping[str, str],
+) -> list[str]:
+    """Return configured auth header names that are absent from the resolved headers.
+
+    Comparison is case-insensitive so that ``Authorization`` and ``authorization``
+    are treated as the same header name.
+
+    Args:
+        configured: The server's ``authorization_headers`` configuration mapping
+            (header name → secret path or keyword).
+        resolved: The fully resolved headers that will be sent to the MCP server.
+
+    Returns:
+        List of header names from ``configured`` that could not be resolved, i.e.
+        are not present as a key in ``resolved``. An empty list means all headers
+        were resolved successfully.
+    """
+    resolved_lower = {k.lower() for k in resolved}
+    return [h for h in configured if h.lower() not in resolved_lower]
+
+
+def build_server_headers(
+    mcp_server: ModelContextProtocolServer,
+    client_headers: dict[str, str],
+    request_headers: Optional[Mapping[str, str]],
+    token: Optional[str],
+) -> dict[str, str]:
+    """Build the complete set of headers for a single MCP server.
+
+    Merges client-supplied headers, resolved authorization headers, and propagated
+    request headers in priority order (highest first):
+
+    1. Client-supplied headers (already present in ``client_headers``).
+    2. Statically resolved authorization headers from configuration.
+    3. Kubernetes Bearer token for headers configured with the ``kubernetes`` keyword.
+       ``client`` and ``oauth`` keywords are skipped — those values are already
+       provided by the client in source 1.
+    4. Headers propagated from the incoming request via the server's allowlist.
+
+    Args:
+        mcp_server: MCP server configuration.
+        client_headers: Headers already supplied by the client for this server.
+        request_headers: Headers from the incoming HTTP request, or ``None``.
+        token: Optional Kubernetes service-account token.
+
+    Returns:
+        Merged headers dictionary for the server. May be empty if no headers apply.
+    """
+    server_headers: dict[str, str] = dict(client_headers)
+    existing_lower = {k.lower() for k in server_headers}
+
+    for (
+        header_name,
+        resolved_value,
+    ) in mcp_server.resolved_authorization_headers.items():
+        if header_name.lower() in existing_lower:
+            continue
+        match resolved_value:
+            case constants.MCP_AUTH_KUBERNETES:
+                if token:
+                    server_headers[header_name] = f"Bearer {token}"
+                    existing_lower.add(header_name.lower())
+            case constants.MCP_AUTH_CLIENT | constants.MCP_AUTH_OAUTH:
+                pass  # client-provided; already included via the initial client_headers copy
+            case _:
+                server_headers[header_name] = resolved_value
+                existing_lower.add(header_name.lower())
+
+    if mcp_server.headers and request_headers is not None:
+        propagated = extract_propagated_headers(mcp_server, request_headers)
+        for h_name, h_value in propagated.items():
+            if h_name.lower() not in existing_lower:
+                server_headers[h_name] = h_value
+                existing_lower.add(h_name.lower())
+
+    return server_headers
+
+
+def build_mcp_headers(
+    config: AppConfig,
+    mcp_headers: McpHeaders,
+    request_headers: Optional[Mapping[str, str]],
+    token: Optional[str] = None,
+) -> McpHeaders:
+    """Build complete MCP headers by merging all header sources for each MCP server.
+
+    For each configured MCP server, combines four header sources (in priority order,
+    highest first):
+
+    1. Client-supplied headers from the ``MCP-HEADERS`` request header (keyed by server name).
+    2. Statically resolved authorization headers from configuration (e.g. file-based secrets).
+    3. Kubernetes Bearer token: when a header is configured with the ``kubernetes`` keyword,
+       the supplied ``token`` is formatted as ``Bearer <token>`` and used as its value.
+       ``client`` and ``oauth`` keywords are not resolved here — those values are already
+       provided by the client in source 1.
+    4. Headers propagated from the incoming request via the server's configured allowlist.
+
+    Args:
+        config: Application configuration containing mcp_servers.
+        mcp_headers: Per-request headers from the client, keyed by MCP server name.
+        request_headers: Headers from the incoming HTTP request used for allowlist
+            propagation, or ``None`` when not available.
+        token: Optional Kubernetes service-account token used to resolve headers
+            configured with the ``kubernetes`` keyword.
+
+    Returns:
+        McpHeaders keyed by MCP server name with the complete merged set of headers.
+        Servers that end up with no headers are omitted from the result.
+    """
+    if not config.mcp_servers:
+        return {}
+
+    complete: McpHeaders = {}
+
+    for mcp_server in config.mcp_servers:
+        server_headers = build_server_headers(
+            mcp_server,
+            dict(mcp_headers.get(mcp_server.name, {})),
+            request_headers,
+            token,
+        )
+        if server_headers:
+            complete[mcp_server.name] = server_headers
+
+    return complete

src/utils/mcp_oauth_probe.py

@@ -40,7 +40,16 @@ async def check_mcp_auth(configuration: AppConfig, mcp_headers: McpHeaders) -> N
     probes = []
     for mcp_server in configuration.mcp_servers:
         headers = mcp_headers.get(mcp_server.name, {})
-        authorization = headers.get("Authorization", None)
+        auth_header = headers.get("Authorization")
+        if auth_header is not None:
+            authorization = (
+                auth_header
+                if auth_header.startswith("Bearer ")
+                else f"Bearer {auth_header}"
+            )
+        else:
+            authorization = None
+
         if (
             authorization
             or constants.MCP_AUTH_OAUTH

src/utils/responses.py

@@ -96,7 +96,11 @@
     NotFoundResponse,
     ServiceUnavailableResponse,
 )
-from utils.mcp_headers import McpHeaders, extract_propagated_headers
+from utils.mcp_headers import (
+    McpHeaders,
+    build_mcp_headers,
+    find_unresolved_auth_headers,
+)
 from utils.prompts import get_system_prompt, get_topic_summary_system_prompt
 from utils.query import (
     extract_provider_and_model_from_model_id,
@@ -483,17 +487,21 @@ def get_rag_tools(vector_store_ids: list[str]) -> Optional[list[InputToolFileSea
     ]
 
 
-async def get_mcp_tools(  # pylint: disable=too-many-return-statements,too-many-locals
+async def get_mcp_tools(
     token: Optional[str] = None,
     mcp_headers: Optional[McpHeaders] = None,
     request_headers: Optional[Mapping[str, str]] = None,
 ) -> list[InputToolMCP]:
     """Convert MCP servers to tools format for Responses API.
 
+    Fully delegates header assembly to ``build_mcp_headers``, which handles static
+    config tokens, the kubernetes Bearer token, client/oauth client-provided headers,
+    and propagated request headers.
+
     Args:
-        token: Optional authentication token for MCP server authorization
-        mcp_headers: Optional per-request headers for MCP servers, keyed by server URL
-        request_headers: Optional incoming HTTP request headers for allowlist propagation
+        token: Optional Kubernetes service-account token for ``kubernetes`` auth headers.
+        mcp_headers: Optional per-request headers for MCP servers, keyed by server name.
+        request_headers: Optional incoming HTTP request headers for allowlist propagation.
 
     Returns:
         List of MCP tool definitions with server details and optional auth. When
@@ -504,68 +512,27 @@ async def get_mcp_tools(  # pylint: disable=too-many-return-statements,too-many-
         HTTPException: 401 with WWW-Authenticate header when an MCP server uses OAuth,
             no headers are passed, and the server responds with 401 and WWW-Authenticate.
     """
-
-    def _get_token_value(original: str, header: str) -> Optional[str]:
-        """Convert to header value."""
-        match original:
-            case constants.MCP_AUTH_KUBERNETES:
-                # use k8s token
-                if token is None or token == "":
-                    return None
-                return f"Bearer {token}"
-            case constants.MCP_AUTH_CLIENT:
-                # use client provided token
-                if mcp_headers is None:
-                    return None
-                c_headers = mcp_headers.get(mcp_server.name, None)
-                if c_headers is None:
-                    return None
-                return c_headers.get(header, None)
-            case constants.MCP_AUTH_OAUTH:
-                # use oauth token
-                if mcp_headers is None:
-                    return None
-                c_headers = mcp_headers.get(mcp_server.name, None)
-                if c_headers is None:
-                    return None
-                return c_headers.get(header, None)
-            case _:
-                # use provided
-                return original
+    complete_headers = build_mcp_headers(
+        configuration, mcp_headers or {}, request_headers, token
+    )
 
     tools: list[InputToolMCP] = []
     for mcp_server in configuration.mcp_servers:
-        # Build headers
-        headers: dict[str, str] = {}
-        for name, value in mcp_server.resolved_authorization_headers.items():
-            # for each defined header
-            h_value = _get_token_value(value, name)
-            # only add the header if we got value
-            if h_value is not None:
-                headers[name] = h_value
-
-        # Skip server if auth headers were configured but not all could be resolved
-        if mcp_server.authorization_headers and len(headers) != len(
-            mcp_server.authorization_headers
-        ):
+        headers: dict[str, str] = dict(complete_headers.get(mcp_server.name, {}))
+
+        # Skip server if any configured auth header could not be resolved.
+        unresolved = find_unresolved_auth_headers(
+            mcp_server.authorization_headers, headers
+        )
+        if unresolved:
             logger.warning(
                 "Skipping MCP server %s: required %d auth headers but only resolved %d",
                 mcp_server.name,
                 len(mcp_server.authorization_headers),
-                len(headers),
+                len(mcp_server.authorization_headers) - len(unresolved),
             )
             continue
 
-        # Propagate allowlisted headers from the incoming request
-        if mcp_server.headers and request_headers is not None:
-            propagated = extract_propagated_headers(mcp_server, request_headers)
-            existing_lower = {name.lower() for name in headers}
-            for h_name, h_value in propagated.items():
-                if h_name.lower() not in existing_lower:
-                    headers[h_name] = h_value
-                    existing_lower.add(h_name.lower())
-
-        # Build Authorization header
         authorization = headers.pop("Authorization", None)
         tools.append(
             InputToolMCP(

tests/e2e/configuration/library-mode/lightspeed-stack-invalid-mcp-file-auth.yaml

@@ -21,4 +21,4 @@ mcp_servers:
   - name: "mcp-file"
     url: "http://mock-mcp:3001"
     authorization_headers:
-      Authorization: "/tmp/invalid-mcp-secret-token"
+      Authorization: "/tmp/invalid-mcp-token"

tests/e2e/configuration/library-mode/lightspeed-stack-mcp-file-auth.yaml

@@ -21,4 +21,4 @@ mcp_servers:
   - name: "mcp-file"
     url: "http://mock-mcp:3001"
     authorization_headers:
-      Authorization: "/tmp/mcp-secret-token"
+      Authorization: "/tmp/mcp-token"

tests/e2e/configuration/library-mode/lightspeed-stack-mcp-kubernetes-auth.yaml

@@ -16,7 +16,7 @@ user_data_collection:
   transcripts_enabled: true
   transcripts_storage: "/tmp/data/transcripts"
 authentication:
-  module: "noop"
+  module: "noop-with-token"
 mcp_servers:
   - name: "mcp-kubernetes"
     url: "http://mock-mcp:3001"

tests/e2e/configuration/library-mode/lightspeed-stack-mcp.yaml

@@ -29,7 +29,7 @@ mcp_servers:
   - name: "mcp-file"
     url: "http://mock-mcp:3001"
     authorization_headers:
-      Authorization: "/tmp/mcp-secret-token"
+      Authorization: "/tmp/mcp-token"
   - name: "mcp-client"
     url: "http://mock-mcp:3001"
     authorization_headers: