mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-09 13:21:42 +00:00
fix(mcp): add POST probe fallback in preflight content-type check
Some MCP servers (e.g. DocuSeal) serve their web UI on HEAD/GET but speak Streamable HTTP only via POST. The preflight probe now tries a lightweight JSON-RPC `initialize` POST before rejecting endpoints whose HEAD/GET returns a non-MCP content type (e.g. `text/html`). If the POST returns `application/json` or `text/event-stream` with a 2xx status, the endpoint is accepted. Otherwise the original rejection behaviour is preserved. Adds 5 new test cases covering the POST probe path: - POST rescues HTML HEAD with JSON response - POST rescues HTML HEAD with event-stream response - POST still rejects when it also returns HTML - POST still rejects on non-2xx status - POST not attempted when HEAD already returns valid MCP content type
This commit is contained in:
parent
18e840469f
commit
32c1c47eef
2 changed files with 152 additions and 9 deletions
|
|
@ -5,8 +5,8 @@ HTTP server (via httpx's ASGI/transport plumbing through a stdlib server),
|
|||
rather than reimplementing the probe inline. That distinction matters: the
|
||||
production probe must run on its own httpx client outside the MCP SDK's anyio
|
||||
task group, and a faithful test must exercise that actual method so the
|
||||
content-type allow-list, HEAD->GET fallback, and best-effort pass-through are
|
||||
all covered as shipped.
|
||||
content-type allow-list, HEAD->GET fallback, POST probe fallback, and
|
||||
best-effort pass-through are all covered as shipped.
|
||||
|
||||
OAuth note
|
||||
----------
|
||||
|
|
@ -56,12 +56,19 @@ def _serve(handler_cls):
|
|||
|
||||
def _handler(status: int = 200,
|
||||
content_type: "str | None" = "text/html; charset=utf-8",
|
||||
body: bytes = b"<html>x</html>", head_status=None, record=None):
|
||||
body: bytes = b"<html>x</html>", head_status=None, record=None,
|
||||
post_content_type: "str | None" = None,
|
||||
post_body: bytes = b"",
|
||||
post_status: "int | None" = None):
|
||||
"""Build a BaseHTTPRequestHandler that replies with the given shape.
|
||||
|
||||
``head_status`` lets HEAD return a different status than GET (to exercise
|
||||
the HEAD->GET fallback). ``record`` is an optional list that captures the
|
||||
HTTP methods the server actually saw.
|
||||
|
||||
``post_content_type`` / ``post_body`` / ``post_status`` let POST return a
|
||||
different response than HEAD/GET (to exercise the POST probe fallback for
|
||||
servers that serve HTML on GET but speak MCP via POST).
|
||||
"""
|
||||
|
||||
class _H(http.server.BaseHTTPRequestHandler):
|
||||
|
|
@ -85,6 +92,18 @@ def _handler(status: int = 200,
|
|||
record.append("GET")
|
||||
self._write(status, content_type, body)
|
||||
|
||||
def do_POST(self):
|
||||
if record is not None:
|
||||
record.append("POST")
|
||||
# Read and discard request body to avoid broken pipe.
|
||||
length = int(self.headers.get("Content-Length", 0))
|
||||
if length:
|
||||
self.rfile.read(length)
|
||||
sc = post_status if post_status is not None else status
|
||||
ct = post_content_type if post_content_type is not None else content_type
|
||||
pb = post_body if post_body else body
|
||||
self._write(sc, ct, pb)
|
||||
|
||||
def log_message(self, format, *args): # noqa: A002
|
||||
pass
|
||||
|
||||
|
|
@ -190,6 +209,7 @@ def test_cancelled_error_is_not_swallowed():
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_head_405_falls_back_to_get_and_rejects_html():
|
||||
"""HEAD→405, GET→html, POST probe also returns html → reject."""
|
||||
task = _make_task("fallback_srv")
|
||||
record: list[str] = []
|
||||
with _serve(_handler(
|
||||
|
|
@ -198,7 +218,8 @@ def test_head_405_falls_back_to_get_and_rejects_html():
|
|||
)) as base:
|
||||
with pytest.raises(NonMcpEndpointError):
|
||||
asyncio.run(task._preflight_content_type(f"{base}/", timeout=5.0))
|
||||
assert record == ["HEAD", "GET"]
|
||||
# HEAD → 405, falls back to GET (html), then POST probe (also html) → reject.
|
||||
assert record == ["HEAD", "GET", "POST"]
|
||||
|
||||
|
||||
def test_head_501_falls_back_to_get_and_passes_json():
|
||||
|
|
@ -313,3 +334,78 @@ def test_ssl_verify_and_cert_forwarded(monkeypatch):
|
|||
assert captured.get("verify") is False
|
||||
assert captured.get("cert") == "/path/to/cert.pem"
|
||||
assert captured.get("follow_redirects") is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# POST probe fallback for POST-only MCP servers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_post_probe_rescues_html_head_with_json_post():
|
||||
"""HEAD returns text/html but POST returns application/json → pass."""
|
||||
task = _make_task()
|
||||
record: list[str] = []
|
||||
with _serve(_handler(
|
||||
status=200, content_type="text/html",
|
||||
post_content_type="application/json; charset=utf-8",
|
||||
post_body=b'{"jsonrpc":"2.0","id":"_probe","result":{}}',
|
||||
record=record,
|
||||
)) as base:
|
||||
# Must not raise — the POST probe should rescue this.
|
||||
asyncio.run(task._preflight_content_type(f"{base}/mcp", timeout=5.0))
|
||||
assert "HEAD" in record
|
||||
assert "POST" in record
|
||||
|
||||
|
||||
def test_post_probe_rescues_html_head_with_event_stream_post():
|
||||
"""HEAD returns text/html but POST returns text/event-stream → pass."""
|
||||
task = _make_task()
|
||||
with _serve(_handler(
|
||||
status=200, content_type="text/html",
|
||||
post_content_type="text/event-stream",
|
||||
post_body=b"data: {}\n\n",
|
||||
)) as base:
|
||||
asyncio.run(task._preflight_content_type(f"{base}/mcp", timeout=5.0))
|
||||
|
||||
|
||||
def test_post_probe_still_rejects_when_post_also_returns_html():
|
||||
"""HEAD and POST both return text/html → reject."""
|
||||
task = _make_task("both_html")
|
||||
with _serve(_handler(
|
||||
status=200, content_type="text/html",
|
||||
post_content_type="text/html",
|
||||
post_body=b"<html>nope</html>",
|
||||
)) as base:
|
||||
with pytest.raises(NonMcpEndpointError):
|
||||
asyncio.run(task._preflight_content_type(f"{base}/", timeout=5.0))
|
||||
|
||||
|
||||
def test_post_probe_still_rejects_when_post_returns_non_2xx():
|
||||
"""HEAD returns HTML, POST returns 401 with JSON → reject.
|
||||
|
||||
A non-2xx POST does not prove MCP capability; the original HEAD/GET
|
||||
response is used and should still trigger rejection.
|
||||
"""
|
||||
task = _make_task("post_401")
|
||||
with _serve(_handler(
|
||||
status=200, content_type="text/html",
|
||||
post_content_type="application/json",
|
||||
post_body=b'{"error":"unauthorized"}',
|
||||
post_status=401,
|
||||
)) as base:
|
||||
with pytest.raises(NonMcpEndpointError):
|
||||
asyncio.run(task._preflight_content_type(f"{base}/", timeout=5.0))
|
||||
|
||||
|
||||
def test_post_probe_not_attempted_for_valid_head():
|
||||
"""When HEAD already returns application/json, no POST probe is needed."""
|
||||
task = _make_task()
|
||||
record: list[str] = []
|
||||
with _serve(_handler(
|
||||
status=200, content_type="application/json", body=b"{}",
|
||||
post_content_type="application/json",
|
||||
post_body=b'{}',
|
||||
record=record,
|
||||
)) as base:
|
||||
asyncio.run(task._preflight_content_type(f"{base}/mcp", timeout=5.0))
|
||||
assert record == ["HEAD"]
|
||||
assert "POST" not in record
|
||||
|
|
|
|||
|
|
@ -2067,11 +2067,17 @@ class MCPServerTask:
|
|||
|
||||
Detection is allow-list based: a 2xx response is rejected only when it
|
||||
carries a definite content type that is NOT one an MCP endpoint uses
|
||||
(``application/json`` / ``text/event-stream``). A missing or empty
|
||||
content type, non-2xx status, or any network/transport error passes
|
||||
through silently — the probe is strictly best-effort, and the real
|
||||
handshake remains the source of truth for everything except the
|
||||
unambiguous "this is a web page, not MCP" case.
|
||||
(``application/json`` / ``text/event-stream``). When HEAD/GET returns
|
||||
a non-MCP content type (e.g. ``text/html``), a lightweight JSON-RPC
|
||||
``initialize`` POST is attempted before giving up — some servers
|
||||
(e.g. DocuSeal) serve a web UI on GET but speak Streamable HTTP only
|
||||
via POST.
|
||||
|
||||
A missing or empty content type, non-2xx status, or any
|
||||
network/transport error passes through silently — the probe is
|
||||
strictly best-effort, and the real handshake remains the source of
|
||||
truth for everything except the unambiguous "this is a web page,
|
||||
not MCP" case.
|
||||
|
||||
Runs on its own httpx client OUTSIDE the SDK's anyio task group, so the
|
||||
raised error propagates as itself rather than being wrapped in an
|
||||
|
|
@ -2099,6 +2105,47 @@ class MCPServerTask:
|
|||
resp = await client.head(url, headers=probe_headers)
|
||||
if resp.status_code in (405, 501):
|
||||
resp = await client.get(url, headers=probe_headers)
|
||||
|
||||
# Some MCP servers (e.g. DocuSeal) serve their web UI on
|
||||
# HEAD/GET but speak Streamable HTTP only via POST. Before
|
||||
# rejecting the endpoint, try a lightweight JSON-RPC POST
|
||||
# probe so we don't false-positive on POST-only servers.
|
||||
ct = (
|
||||
resp.headers.get("content-type", "")
|
||||
.split(";")[0]
|
||||
.strip()
|
||||
.lower()
|
||||
)
|
||||
if (
|
||||
ct
|
||||
and ct not in self._MCP_CONTENT_TYPES
|
||||
and 200 <= resp.status_code < 300
|
||||
):
|
||||
post_resp = await client.post(
|
||||
url,
|
||||
headers={
|
||||
**probe_headers,
|
||||
"Content-Type": "application/json",
|
||||
"Accept": "application/json, text/event-stream",
|
||||
},
|
||||
content=(
|
||||
'{"jsonrpc":"2.0","id":"_probe",'
|
||||
'"method":"initialize",'
|
||||
'"params":{"protocolVersion":"2025-03-26",'
|
||||
'"capabilities":{},'
|
||||
'"clientInfo":{"name":"hermes-probe",'
|
||||
'"version":"0.1"}}}'
|
||||
),
|
||||
)
|
||||
if 200 <= post_resp.status_code < 300:
|
||||
post_ct = (
|
||||
post_resp.headers.get("content-type", "")
|
||||
.split(";")[0]
|
||||
.strip()
|
||||
.lower()
|
||||
)
|
||||
if post_ct in self._MCP_CONTENT_TYPES:
|
||||
resp = post_resp
|
||||
except _httpx.HTTPError:
|
||||
return # DNS/connect/timeout/transport error — let the SDK try.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue