From 0295613578108af43044d927eae34082971f60fa Mon Sep 17 00:00:00 2001
From: Christian Krakau-Louis <christianlouis@gmail.com>
Date: Thu, 21 May 2026 19:00:19 +0200
Subject: [PATCH] fix: address remote MCP auth review feedback

---
 musicround/mcp_http.py | 20 ++++++++++++--------
 tests/test_mcp_http.py | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/musicround/mcp_http.py b/musicround/mcp_http.py
index eba0c9d..5aabb3c 100644
--- a/musicround/mcp_http.py
+++ b/musicround/mcp_http.py
@@ -11,6 +11,10 @@ from starlette.types import ASGIApp, Receive, Scope, Send
 from musicround.mcp_server import mcp
 
 
+_BASE_ALLOWED_HOSTS = tuple(mcp.settings.transport_security.allowed_hosts)
+_BASE_ALLOWED_ORIGINS = tuple(mcp.settings.transport_security.allowed_origins)
+
+
 class BearerAuthMiddleware:
     """Require a bearer token for MCP HTTP traffic."""
 
@@ -38,9 +42,11 @@ class BearerAuthMiddleware:
         authorization = headers.get(b"authorization", b"").decode("latin1")
         scheme, _, token = authorization.partition(" ")
         if scheme.lower() != "bearer" or not compare_digest(token.strip(), expected):
-            await JSONResponse({"error": "Unauthorized."}, status_code=401)(
-                scope, receive, send
-            )
+            await JSONResponse(
+                {"error": "Unauthorized."},
+                headers={"WWW-Authenticate": "Bearer"},
+                status_code=401,
+            )(scope, receive, send)
             return
 
         await self.app(scope, receive, send)
@@ -65,11 +71,9 @@ def _configure_server() -> None:
         if value.strip()
     ]
     security = mcp.settings.transport_security
-    security.allowed_hosts.extend(
-        host for host in allowed_hosts if host not in security.allowed_hosts
-    )
-    security.allowed_origins.extend(
-        origin for origin in allowed_origins if origin not in security.allowed_origins
+    security.allowed_hosts = list(dict.fromkeys([*_BASE_ALLOWED_HOSTS, *allowed_hosts]))
+    security.allowed_origins = list(
+        dict.fromkeys([*_BASE_ALLOWED_ORIGINS, *allowed_origins])
     )
 
 
diff --git a/tests/test_mcp_http.py b/tests/test_mcp_http.py
index d282526..02195ac 100644
--- a/tests/test_mcp_http.py
+++ b/tests/test_mcp_http.py
@@ -8,7 +8,7 @@ from starlette.testclient import TestClient
 os.environ.setdefault("SECRET_KEY", "test-secret-key-for-testing-only")
 os.environ.setdefault("AUTOMATION_TOKEN", "test-automation-token-for-testing")
 
-from musicround.mcp_http import BearerAuthMiddleware, build_app  # noqa: E402
+from musicround.mcp_http import BearerAuthMiddleware, build_app, mcp  # noqa: E402
 
 
 async def _dummy_app(scope, receive, send):
@@ -34,5 +34,36 @@ def test_mcp_requires_bearer_token(monkeypatch):
     accepted = client.get("/mcp", headers={"Authorization": "Bearer test-mcp-token"})
 
     assert missing.status_code == 401
+    assert missing.headers["WWW-Authenticate"] == "Bearer"
     assert wrong.status_code == 401
     assert accepted.status_code != 401
+
+
+def test_mcp_falls_back_to_automation_token(monkeypatch):
+    monkeypatch.delenv("MCP_BEARER_TOKEN", raising=False)
+    monkeypatch.setenv("AUTOMATION_TOKEN", "automation-token")
+    client = TestClient(BearerAuthMiddleware(_dummy_app))
+
+    accepted = client.get("/mcp", headers={"Authorization": "Bearer automation-token"})
+
+    assert accepted.status_code == 204
+
+
+def test_mcp_reports_missing_bearer_configuration(monkeypatch):
+    monkeypatch.delenv("MCP_BEARER_TOKEN", raising=False)
+    monkeypatch.delenv("AUTOMATION_TOKEN", raising=False)
+    client = TestClient(BearerAuthMiddleware(_dummy_app))
+
+    response = client.get("/mcp", headers={"Authorization": "Bearer anything"})
+
+    assert response.status_code == 500
+
+
+def test_allowed_hosts_are_replaced_between_builds(monkeypatch):
+    monkeypatch.setenv("MCP_ALLOWED_HOSTS", "first.example")
+    build_app()
+    monkeypatch.setenv("MCP_ALLOWED_HOSTS", "second.example")
+    build_app()
+
+    assert "first.example" not in mcp.settings.transport_security.allowed_hosts
+    assert "second.example" in mcp.settings.transport_security.allowed_hosts