fix(mcp): prevent 307 redirects for Streamable HTTP /mcp probes#4446
Open
fix(mcp): prevent 307 redirects for Streamable HTTP /mcp probes#4446
Conversation
Signed-off-by: rich <cooleryu@outlook.com>
Signed-off-by: rich <cooleryu@outlook.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Normalize MCP mount rewrites before Starlette can emit a trailing-slash
307redirect, and keep ASGIraw_pathaligned when the middleware rewrites MCP transport paths.This prevents exact
GET /mcpprobes from being converted into a framework-level redirect before they reach the MCP transport.Motivation
Fixes #4441.
The MCP Streamable HTTP transport allows
GET /mcpto either:200withContent-Type: text/event-stream, or405 Method Not Allowedwhen the server does not offer an SSE stream at that endpoint.A
307redirect is not a valid transport response for that probe. The issue points to the Python/Starlette gateway layer as the source of the redirect, not the Rust MCP runtime.The root cause is Starlette mount behavior: an exact
/mcprequest againstapp.mount("/mcp", ...)is redirected to/mcp/. ContextForge already hasMCPPathRewriteMiddlewarein front of the mounted transport, so this patch normalizes the exact/mcppath there after authentication.I also noticed #4282, which addresses the same redirect class. This PR follows the same fix direction and adds extra coverage for the actual Starlette mount behavior,
APP_ROOT_PATH, canonical/mcp/pass-through, RFC 9728 well-known path exclusion, andraw_pathalignment.Changes
/mcpto/mcp/inMCPPathRewriteMiddleware.root_pathorAPP_ROOT_PATHis set.raw_pathaligned with rewritten/mcp/paths when present./mcp/unchanged./mcpout of the MCP transport rewrite path.Mount("/mcp", ...)and verifies/mcpno longer returns aLocationredirect./mcp,root_path,APP_ROOT_PATH, canonical/mcp/, well-known URI boundaries, andraw_pathsynchronization.Test Plan
uv run --group dev pytest tests/unit/mcpgateway/test_main_extended.py::TestMCPPathRewriteMiddleware -q22 passeduv run --group dev pytest tests/unit/mcpgateway/transports/test_streamablehttp_transport.py::test_streamable_http_auth_requires_auth_for_trailing_slash_variants tests/unit/mcpgateway/transports/test_streamablehttp_transport.py::test_streamable_http_auth_skips_well_known_rfc9728_even_when_strict -q5 passeduv run --group dev pytest tests/unit/mcpgateway/test_main_extended.py -quv run python -m py_compile mcpgateway/main.py tests/unit/mcpgateway/test_main_extended.pyuv run ruff check mcpgateway/main.pygit diff --check/mcp -> 307 Location: /mcp//mcp -> 502and/mcp/ -> 502, both withoutLocation502is expected in this local smoke test because the Rust MCP sidecar is not running; the relevant regression check is that/mcpno longer returns307.I also ran:
uv run --group dev pytest tests/protocol_compliance/test_transport_semantics.py::test_get_mcp_returns_sse_stream_or_405 -qhttp://127.0.0.1:8080.Running Ruff on the full
tests/unit/mcpgateway/test_main_extended.pyfile still reports existing findings outside this diff, so this patch avoids mixing unrelated cleanup into the bug fix.Risk
Low. The change is scoped to MCP transport paths after MCP authentication has already succeeded.
It does not broaden arbitrary path rewrites. Non-server paths such as
/foo/mcpstill pass through unchanged, and well-known OAuth metadata paths remain excluded from MCP transport rewriting.The
raw_pathupdate mirrors the rewrittenpathvalue so downstream ASGI consumers do not see inconsistent path metadata after middleware normalization.Related Issue
Fixes #4441.
Related: #4282.
Maintainer Context
This keeps the fix in the existing MCP path normalization layer instead of changing the MCP transport, Rust runtime proxy, or Nginx configuration.
The patch is intentionally small and includes boundary tests for reverse-proxy prefixes, ASGI raw path metadata, already-canonical MCP paths, and non-MCP metadata paths so the redirect fix does not expand the public route surface.
If maintainers prefer the implementation in #4282, the additional regression and boundary tests in this PR can be reused directly.