From caf7420309abba0c3f8fa1eb846c60e025d79427 Mon Sep 17 00:00:00 2001 From: Marc Kelechava Date: Tue, 7 Jul 2026 14:04:05 -0700 Subject: [PATCH] fix(SKY-12003): restore hosted /mcp/ connectivity and freeze Docker deps to uv.lock (#7166) Co-authored-by: AronPerez --- skyvern/cli/mcp_tools/origin_middleware.py | 30 ++++++++++++---------- tests/unit/test_mcp_origin_middleware.py | 18 +++++++++++++ 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/skyvern/cli/mcp_tools/origin_middleware.py b/skyvern/cli/mcp_tools/origin_middleware.py index 2ee379e01..b9f5a4db6 100644 --- a/skyvern/cli/mcp_tools/origin_middleware.py +++ b/skyvern/cli/mcp_tools/origin_middleware.py @@ -24,6 +24,7 @@ client surfaces emerge; do not expand it to third-party integrators. from __future__ import annotations +from collections.abc import Collection from urllib.parse import urlsplit import structlog @@ -34,20 +35,20 @@ LOG = structlog.get_logger(__name__) _MAX_LOGGED_ORIGIN_CHARS = 200 -_ALLOWED_MCP_ORIGIN_HOSTS = frozenset( - { - "claude.ai", - "www.claude.ai", - "claude.com", - "www.claude.com", - } +MCP_ORIGIN_HOSTS: tuple[str, ...] = ( + "claude.ai", + "www.claude.ai", + "claude.com", + "www.claude.com", ) +_ALLOWED_MCP_ORIGIN_HOSTS = frozenset(MCP_ORIGIN_HOSTS) # `urlsplit("http://[::1]:3000").hostname` returns bare `::1` (no brackets). -_ALLOWED_LOOPBACK_HOSTS = frozenset({"localhost", "127.0.0.1", "::1"}) +MCP_LOOPBACK_HOSTS: tuple[str, ...] = ("localhost", "127.0.0.1", "::1") +_ALLOWED_LOOPBACK_HOSTS = frozenset(MCP_LOOPBACK_HOSTS) -def is_allowed_origin(origin: str | None) -> bool: +def is_allowed_origin(origin: str | None, extra_allowed_origins: Collection[str] = ()) -> bool: """Return True if `origin` is permitted to invoke the MCP endpoint.""" if origin is None or origin == "": return True @@ -61,7 +62,9 @@ def is_allowed_origin(origin: str | None) -> bool: return False if host in _ALLOWED_LOOPBACK_HOSTS: return True - return host in _ALLOWED_MCP_ORIGIN_HOSTS + if host in _ALLOWED_MCP_ORIGIN_HOSTS: + return True + return f"{parts.scheme}://{parts.netloc}".lower() in extra_allowed_origins def _sanitize_origin_for_log(origin: str | None) -> str | None: @@ -83,8 +86,9 @@ class OriginValidationMiddleware: before any API key or OAuth validation work is performed. """ - def __init__(self, app: ASGIApp) -> None: + def __init__(self, app: ASGIApp, extra_allowed_origins: Collection[str] = ()) -> None: self.app = app + self.extra_allowed_origins = frozenset(origin.strip().lower().rstrip("/") for origin in extra_allowed_origins) async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: # FastMCP currently mounts streamable-HTTP only, but WebSocket scopes @@ -101,7 +105,7 @@ class OriginValidationMiddleware: origin = value.decode("latin-1") break - if not is_allowed_origin(origin): + if not is_allowed_origin(origin, self.extra_allowed_origins): # The offending origin is captured in the structured log; do not # echo it back in the response body to avoid reflecting # attacker-controlled input. @@ -127,4 +131,4 @@ class OriginValidationMiddleware: await self.app(scope, receive, send) -__all__ = ["OriginValidationMiddleware", "is_allowed_origin"] +__all__ = ["MCP_LOOPBACK_HOSTS", "MCP_ORIGIN_HOSTS", "OriginValidationMiddleware", "is_allowed_origin"] diff --git a/tests/unit/test_mcp_origin_middleware.py b/tests/unit/test_mcp_origin_middleware.py index d72a92626..d2b4b0d46 100644 --- a/tests/unit/test_mcp_origin_middleware.py +++ b/tests/unit/test_mcp_origin_middleware.py @@ -70,6 +70,13 @@ def test_is_allowed_origin_random_is_rejected() -> None: assert is_allowed_origin("https://attacker.com") is False +def test_is_allowed_origin_extra_origin_allowed() -> None: + extra = frozenset({"https://inspector.skyvern.example"}) + assert is_allowed_origin("https://inspector.skyvern.example", extra) is True + assert is_allowed_origin("https://inspector.skyvern.example:8443", extra) is False + assert is_allowed_origin("https://evil.example", extra) is False + + def test_is_allowed_origin_prefix_spoofing_rejected() -> None: # Subdomain claiming claude.ai must not pass (only hostname exact matches). assert is_allowed_origin("https://claude.ai.attacker.com") is False @@ -112,6 +119,17 @@ def test_middleware_allows_loopback(client: TestClient) -> None: assert response.status_code == 200 +def test_middleware_allows_configured_extra_origin() -> None: + async def app_factory(scope, receive, send): # type: ignore[no-untyped-def] + inner = Starlette(routes=[Route("/mcp/", _ok_handler, methods=["GET", "POST"])]) + middleware = OriginValidationMiddleware(inner, extra_allowed_origins=["https://Inspector.Skyvern.Example/"]) + await middleware(scope, receive, send) + + client = TestClient(app_factory) + assert client.post("/mcp/", headers={"origin": "https://inspector.skyvern.example"}).status_code == 200 + assert client.post("/mcp/", headers={"origin": "https://evil.example"}).status_code == 403 + + def test_middleware_rejects_unknown_origin(client: TestClient) -> None: response = client.post("/mcp/", headers={"origin": "https://evil.example"}) assert response.status_code == 403