mirror of
https://github.com/Skyvern-AI/skyvern.git
synced 2026-07-09 16:09:13 +00:00
fix(SKY-12003): restore hosted /mcp/ connectivity and freeze Docker deps to uv.lock (#7166)
Co-authored-by: AronPerez <aperez0295@gmail.com>
This commit is contained in:
parent
e7a57dd653
commit
caf7420309
2 changed files with 35 additions and 13 deletions
|
|
@ -24,6 +24,7 @@ client surfaces emerge; do not expand it to third-party integrators.
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from collections.abc import Collection
|
||||||
from urllib.parse import urlsplit
|
from urllib.parse import urlsplit
|
||||||
|
|
||||||
import structlog
|
import structlog
|
||||||
|
|
@ -34,20 +35,20 @@ LOG = structlog.get_logger(__name__)
|
||||||
_MAX_LOGGED_ORIGIN_CHARS = 200
|
_MAX_LOGGED_ORIGIN_CHARS = 200
|
||||||
|
|
||||||
|
|
||||||
_ALLOWED_MCP_ORIGIN_HOSTS = frozenset(
|
MCP_ORIGIN_HOSTS: tuple[str, ...] = (
|
||||||
{
|
"claude.ai",
|
||||||
"claude.ai",
|
"www.claude.ai",
|
||||||
"www.claude.ai",
|
"claude.com",
|
||||||
"claude.com",
|
"www.claude.com",
|
||||||
"www.claude.com",
|
|
||||||
}
|
|
||||||
)
|
)
|
||||||
|
_ALLOWED_MCP_ORIGIN_HOSTS = frozenset(MCP_ORIGIN_HOSTS)
|
||||||
|
|
||||||
# `urlsplit("http://[::1]:3000").hostname` returns bare `::1` (no brackets).
|
# `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."""
|
"""Return True if `origin` is permitted to invoke the MCP endpoint."""
|
||||||
if origin is None or origin == "":
|
if origin is None or origin == "":
|
||||||
return True
|
return True
|
||||||
|
|
@ -61,7 +62,9 @@ def is_allowed_origin(origin: str | None) -> bool:
|
||||||
return False
|
return False
|
||||||
if host in _ALLOWED_LOOPBACK_HOSTS:
|
if host in _ALLOWED_LOOPBACK_HOSTS:
|
||||||
return True
|
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:
|
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.
|
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.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:
|
async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
|
||||||
# FastMCP currently mounts streamable-HTTP only, but WebSocket scopes
|
# FastMCP currently mounts streamable-HTTP only, but WebSocket scopes
|
||||||
|
|
@ -101,7 +105,7 @@ class OriginValidationMiddleware:
|
||||||
origin = value.decode("latin-1")
|
origin = value.decode("latin-1")
|
||||||
break
|
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
|
# The offending origin is captured in the structured log; do not
|
||||||
# echo it back in the response body to avoid reflecting
|
# echo it back in the response body to avoid reflecting
|
||||||
# attacker-controlled input.
|
# attacker-controlled input.
|
||||||
|
|
@ -127,4 +131,4 @@ class OriginValidationMiddleware:
|
||||||
await self.app(scope, receive, send)
|
await self.app(scope, receive, send)
|
||||||
|
|
||||||
|
|
||||||
__all__ = ["OriginValidationMiddleware", "is_allowed_origin"]
|
__all__ = ["MCP_LOOPBACK_HOSTS", "MCP_ORIGIN_HOSTS", "OriginValidationMiddleware", "is_allowed_origin"]
|
||||||
|
|
|
||||||
|
|
@ -70,6 +70,13 @@ def test_is_allowed_origin_random_is_rejected() -> None:
|
||||||
assert is_allowed_origin("https://attacker.com") is False
|
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:
|
def test_is_allowed_origin_prefix_spoofing_rejected() -> None:
|
||||||
# Subdomain claiming claude.ai must not pass (only hostname exact matches).
|
# Subdomain claiming claude.ai must not pass (only hostname exact matches).
|
||||||
assert is_allowed_origin("https://claude.ai.attacker.com") is False
|
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
|
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:
|
def test_middleware_rejects_unknown_origin(client: TestClient) -> None:
|
||||||
response = client.post("/mcp/", headers={"origin": "https://evil.example"})
|
response = client.post("/mcp/", headers={"origin": "https://evil.example"})
|
||||||
assert response.status_code == 403
|
assert response.status_code == 403
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue