diff --git a/reme/components/service/http_service.py b/reme/components/service/http_service.py index 177fec33..029db215 100644 --- a/reme/components/service/http_service.py +++ b/reme/components/service/http_service.py @@ -11,6 +11,7 @@ from fastapi import FastAPI, HTTPException from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import FileResponse, StreamingResponse from fastapi.staticfiles import StaticFiles +from starlette.routing import Route from .base_service import BaseService from ..component_registry import R @@ -92,9 +93,31 @@ class HttpService(BaseService): allow_headers=["*"], ) if self.mcp_app is not None: - # Mounting at /mcp makes Starlette redirect to /mcp/. Merge the - # generated routes so the configured path remains canonical. - self.service.router.routes.extend(self.mcp_app.routes) + # Forward the exact path to the complete FastMCP ASGI app. Copying + # only its routes would bypass its middleware and application state; + # mounting it would make the trailing-slash path canonical instead. + self.service.router.routes.append( + Route( + self.mcp_path, + endpoint=self.mcp_app, + include_in_schema=False, + ), + ) + + def add_jobs(self, app: "Application") -> None: + """Validate reserved routes before the shared tolerant registration loop.""" + if self.mcp_enabled: + conflicts = sorted( + job.name + for name, job in app.context.jobs.items() + if job.enable_serve and (self.jobs is None or name in self.jobs) and f"/{job.name}" == self.mcp_path + ) + if conflicts: + names = ", ".join(conflicts) + raise ValueError( + f"Job name conflicts with the MCP endpoint {self.mcp_path!r}: {names}", + ) + super().add_jobs(app) def add_job(self, job: BaseJob) -> bool: """Register HTTP routes for every job and MCP tools for non-stream jobs.""" @@ -180,6 +203,12 @@ class HttpService(BaseService): raise ValueError( "mcp_path must start with '/', must not be '/', and must not end with '/'", ) + if "//" in path or any(segment in {".", ".."} for segment in path.split("/")): + raise ValueError("mcp_path must use non-empty literal path segments") + if any(char in path for char in "{}?#\\") or any( + char.isspace() or ord(char) < 32 or ord(char) == 127 for char in path + ): + raise ValueError("mcp_path must be a literal URL path without route, query, or fragment syntax") if path in {"/assets", "/docs", "/redoc", "/openapi.json"}: raise ValueError(f"mcp_path conflicts with reserved HTTP path {path!r}") return path diff --git a/tests/unit/test_http_web_workspace.py b/tests/unit/test_http_web_workspace.py index 57a5a7f8..4d6fc994 100644 --- a/tests/unit/test_http_web_workspace.py +++ b/tests/unit/test_http_web_workspace.py @@ -7,6 +7,7 @@ from types import ModuleType, SimpleNamespace import pytest from fastapi.testclient import TestClient +from starlette.middleware.base import BaseHTTPMiddleware from reme.components.service.http_service import HttpService from reme.components.job import BaseJob, StreamJob @@ -133,6 +134,19 @@ def test_http_service_uses_exact_mcp_path_and_runs_one_application_lifespan() -> app = CountingApplication() service = HttpService(web_enabled=False) service.build_service(app) # type: ignore[arg-type] + mcp_server = service.mcp_server + + class VerifyFastMCPAppMiddleware(BaseHTTPMiddleware): + """Prove requests retain FastMCP middleware and application state.""" + + async def dispatch(self, request, call_next): + """Verify the child app context and mark its response.""" + assert request.app.state.fastmcp_server is mcp_server + response = await call_next(request) + response.headers["X-FastMCP-Middleware"] = "preserved" + return response + + service.mcp_app.add_middleware(VerifyFastMCPAppMiddleware) with TestClient(service.service, follow_redirects=False) as client: response = client.post( @@ -154,6 +168,7 @@ def test_http_service_uses_exact_mcp_path_and_runs_one_application_lifespan() -> ) assert response.status_code == 200 assert '"serverInfo"' in response.text + assert response.headers["X-FastMCP-Middleware"] == "preserved" assert client.post("/mcp/mcp", json={}).status_code == 404 assert app.start_count == 1 @@ -171,7 +186,18 @@ def test_http_service_can_disable_mcp() -> None: def test_http_service_rejects_invalid_or_conflicting_mcp_paths() -> None: """Reject paths that shadow built-ins and jobs that shadow the MCP endpoint.""" - for path in ("mcp", "/", "/mcp/", "/docs"): + for path in ( + "mcp", + "/", + "/mcp/", + "/docs", + "/{rest:path}", + "/mcp?mode=test", + "/mcp#fragment", + "/mcp path", + "/mcp//nested", + "/mcp/../nested", + ): with pytest.raises(ValueError, match="mcp_path"): HttpService(mcp_path=path) @@ -181,6 +207,18 @@ def test_http_service_rejects_invalid_or_conflicting_mcp_paths() -> None: service.add_job(BaseJob(name="mcp")) +def test_http_service_fails_startup_preflight_for_mcp_job_conflict() -> None: + """Do not let BaseService's tolerant registration hide reserved-route conflicts.""" + service = HttpService(web_enabled=False) + service.build_service(_FakeApplication()) # type: ignore[arg-type] + app = SimpleNamespace( + context=SimpleNamespace(jobs={"mcp": BaseJob(name="mcp")}), + ) + + with pytest.raises(ValueError, match="conflicts with the MCP endpoint"): + service.add_jobs(app) + + def test_http_service_does_not_serve_symlinks_outside_static_dir( tmp_path: Path, ) -> None: