Skip to content

fix(mcp): prevent 307 redirects for Streamable HTTP /mcp probes#4446

Open
cooleryu wants to merge 2 commits intoIBM:mainfrom
cooleryu:fix-mcp-trailing-slash-4441
Open

fix(mcp): prevent 307 redirects for Streamable HTTP /mcp probes#4446
cooleryu wants to merge 2 commits intoIBM:mainfrom
cooleryu:fix-mcp-trailing-slash-4441

Conversation

@cooleryu
Copy link
Copy Markdown

@cooleryu cooleryu commented Apr 26, 2026

Summary

Normalize MCP mount rewrites before Starlette can emit a trailing-slash 307 redirect, and keep ASGI raw_path aligned when the middleware rewrites MCP transport paths.

This prevents exact GET /mcp probes from being converted into a framework-level redirect before they reach the MCP transport.

Motivation

Fixes #4441.

The MCP Streamable HTTP transport allows GET /mcp to either:

  • return 200 with Content-Type: text/event-stream, or
  • return 405 Method Not Allowed when the server does not offer an SSE stream at that endpoint.

A 307 redirect 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 /mcp request against app.mount("/mcp", ...) is redirected to /mcp/. ContextForge already has MCPPathRewriteMiddleware in front of the mounted transport, so this patch normalizes the exact /mcp path 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, and raw_path alignment.

Changes

  • Normalize exact /mcp to /mcp/ in MCPPathRewriteMiddleware.
  • Preserve reverse-proxy prefixes when root_path or APP_ROOT_PATH is set.
  • Keep ASGI raw_path aligned with rewritten /mcp/ paths when present.
  • Leave already-canonical /mcp/ unchanged.
  • Keep RFC 9728 well-known URLs ending in /mcp out of the MCP transport rewrite path.
  • Add a regression test that wires the middleware in front of a real Starlette Mount("/mcp", ...) and verifies /mcp no longer returns a Location redirect.
  • Add unit coverage for exact /mcp, root_path, APP_ROOT_PATH, canonical /mcp/, well-known URI boundaries, and raw_path synchronization.

Test Plan

  • uv run --group dev pytest tests/unit/mcpgateway/test_main_extended.py::TestMCPPathRewriteMiddleware -q
    • Result: 22 passed
  • uv 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 -q
    • Result: 5 passed
  • uv run --group dev pytest tests/unit/mcpgateway/test_main_extended.py -q
    • Result: passed
  • uv run python -m py_compile mcpgateway/main.py tests/unit/mcpgateway/test_main_extended.py
    • Result: passed
  • uv run ruff check mcpgateway/main.py
    • Result: passed
  • git diff --check
    • Result: passed
  • ASGI smoke test with Edge-mode environment variables and no Rust sidecar:
    • Before this patch: /mcp -> 307 Location: /mcp/
    • After this patch: /mcp -> 502 and /mcp/ -> 502, both without Location
    • The 502 is expected in this local smoke test because the Rust MCP sidecar is not running; the relevant regression check is that /mcp no longer returns 307.

I also ran:

  • uv run --group dev pytest tests/protocol_compliance/test_transport_semantics.py::test_get_mcp_returns_sse_stream_or_405 -q
    • Result: skipped because no live gateway is running at http://127.0.0.1:8080.

Running Ruff on the full tests/unit/mcpgateway/test_main_extended.py file 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/mcp still pass through unchanged, and well-known OAuth metadata paths remain excluded from MCP transport rewriting.

The raw_path update mirrors the rewritten path value 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.

Signed-off-by: rich <cooleryu@outlook.com>
Signed-off-by: rich <cooleryu@outlook.com>
@cooleryu cooleryu changed the title fix: normalize /mcp before Starlette mount redirect fix: normalize MCP mount rewrites before redirect Apr 26, 2026
@cooleryu cooleryu marked this pull request as ready for review April 26, 2026 09:47
@cooleryu cooleryu changed the title fix: normalize MCP mount rewrites before redirect fix(mcp): prevent 307 redirects for Streamable HTTP /mcp probes Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: GET /mcp/ returns 307 redirect in Edge mode instead of spec-required 200 (SSE) or 405

1 participant