Skip to content

Commit d235d51

Browse files
MohanLakshmadhav165brian-hussey
authored
fix: handle root_path in MCPPathRewriteMiddleware for reverse proxy deployments (#4217)
* fix: handle root_path in MCPPathRewriteMiddleware for reverse proxy deployments Fixes #4215 The /servers/{id}/mcp endpoint was returning 404 in reverse proxy deployments with path prefixes because MCPPathRewriteMiddleware checked original_path without stripping the root_path prefix. - Extract root_path from scope or settings before pattern matching - Strip root_path to get app-relative path for /servers/ detection - Preserve root_path when rewriting to /mcp/ - Add 8 unit tests covering prefix scenarios and security checks Pattern follows streamablehttp_transport.py:831 and token_scoping.py:354 Signed-off-by: Mohan Lakshmaiah <mohan.economist@gmail.com> * refactor: use _normalize_scope_path in MCPPathRewriteMiddleware Replace inline root_path stripping with existing _normalize_scope_path() which provides full-segment boundary checks and root_path="/" safety. Closes #4215 Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Update .secrets.baseline Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com> --------- Signed-off-by: Mohan Lakshmaiah <mohan.economist@gmail.com> Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com> Co-authored-by: Madhav Kandukuri <madhav165@gmail.com> Co-authored-by: Brian Hussey <brian.hussey@ie.ibm.com>
1 parent 0fc878c commit d235d51

File tree

3 files changed

+171
-9
lines changed

3 files changed

+171
-9
lines changed

‎.secrets.baseline‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"files": "^.secrets.baseline|package-lock.json|Cargo.lock|scripts/sign_image.sh|scripts/zap|sonar-project.properties|uv.lock|go.sum|mcpgateway/sri_hashes.json|^.secrets.baseline$",
44
"lines": null
55
},
6-
"generated_at": "2026-04-15T13:04:45Z",
6+
"generated_at": "2026-04-15T14:38:50Z",
77
"plugins_used": [
88
{
99
"name": "AWSKeyDetector"

‎mcpgateway/main.py‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1434,9 +1434,7 @@ def jsonpath_modifier(data: Any, jsonpath: str = "$[*]", mappings: Optional[Dict
14341434
# Log jsonpath_modifier invocation with structured data (only if debug enabled)
14351435
if logger.isEnabledFor(logging.DEBUG):
14361436
data_length = len(data) if isinstance(data, list) else None
1437-
logger.debug(
1438-
f"jsonpath_modifier: path='{SecurityValidator.sanitize_log_message(jsonpath)}', has_mappings={mappings is not None}, " f"data_type={type(data).__name__}, data_length={data_length}"
1439-
)
1437+
logger.debug(f"jsonpath_modifier: path='{SecurityValidator.sanitize_log_message(jsonpath)}', has_mappings={mappings is not None}, data_type={type(data).__name__}, data_length={data_length}")
14401438

14411439
try:
14421440
main_expr: JSONPath = _parse_jsonpath(jsonpath)
@@ -3005,20 +3003,27 @@ async def _call_streamable_http(self, scope, receive, send):
30053003
original_path = scope.get("path", "")
30063004
scope["modified_path"] = original_path
30073005

3006+
# Strip root_path prefix before pattern matching.
3007+
# In reverse proxy deployments, scope["path"] may contain the full path
3008+
# including the proxy prefix (e.g., "/dev/mcp-gateway/service/gateway/servers/123/mcp").
3009+
# We need to strip this prefix to correctly match the /servers/ pattern.
3010+
root_path = (scope.get("root_path") or settings.app_root_path or "").rstrip("/")
3011+
app_path = _normalize_scope_path(original_path, root_path)
3012+
30083013
# Skip rewriting for well-known URIs (RFC 9728 OAuth metadata, etc.)
30093014
# These paths may end with /mcp but should not be rewritten to the MCP transport
3010-
if not original_path.startswith("/.well-known/"):
3011-
if (original_path.endswith("/mcp") and original_path != "/mcp") or (original_path.endswith("/mcp/") and original_path != "/mcp/"):
3015+
if not app_path.startswith("/.well-known/"):
3016+
if (app_path.endswith("/mcp") and app_path != "/mcp") or (app_path.endswith("/mcp/") and app_path != "/mcp/"):
30123017
# SECURITY: Only rewrite recognised MCP paths — /servers/{id}/mcp.
30133018
# Arbitrary prefixes (e.g. /foo/mcp) must NOT be rewritten to
30143019
# /mcp/ as that would expose the global MCP transport under
30153020
# undocumented aliases, broadening the externally reachable
30163021
# route surface.
3017-
if original_path.startswith("/servers/"):
3022+
if app_path.startswith("/servers/"):
30183023
# Validate that a non-empty server_id segment is present.
30193024
# Without this check, paths like /servers//mcp (empty ID)
30203025
# would be rewritten and silently fall through (#3891).
3021-
_srv_match = re.match(r"/servers/([^/]+)/mcp", original_path)
3026+
_srv_match = re.match(r"/servers/([^/]+)/mcp", app_path)
30223027
if not _srv_match:
30233028
response = ORJSONResponse({"detail": "Invalid server identifier"}, status_code=404)
30243029
await response(scope, receive, send)
@@ -3028,7 +3033,8 @@ async def _call_streamable_http(self, scope, receive, send):
30283033
await self.application(scope, receive, send)
30293034
return
30303035
# Rewrite to /mcp/ and continue through middleware (lets CORSMiddleware handle preflight)
3031-
scope["path"] = "/mcp/"
3036+
# Preserve root_path prefix when rewriting
3037+
scope["path"] = f"{root_path}/mcp/" if root_path else "/mcp/"
30323038
await self.application(scope, receive, send)
30333039
return
30343040
await self.application(scope, receive, send)

‎tests/unit/mcpgateway/test_main_extended.py‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2833,6 +2833,162 @@ async def send(msg):
28332833
# ORJSONResponse sends http.response.start + http.response.body
28342834
assert any(m.get("status") == 404 for m in sent if m.get("type") == "http.response.start")
28352835

2836+
@pytest.mark.asyncio
2837+
async def test_rewrite_servers_mcp_with_root_path_in_scope(self):
2838+
"""Documented pattern works when root_path is set in scope."""
2839+
app_mock = AsyncMock()
2840+
middleware = MCPPathRewriteMiddleware(app_mock)
2841+
scope = {
2842+
"type": "http",
2843+
"path": "/gateway/servers/123/mcp",
2844+
"root_path": "/gateway",
2845+
"headers": []
2846+
}
2847+
receive, send = AsyncMock(), AsyncMock()
2848+
2849+
with patch("mcpgateway.main.streamable_http_auth", return_value=True):
2850+
await middleware._call_streamable_http(scope, receive, send)
2851+
2852+
assert scope["path"] == "/gateway/mcp/" # Rewritten with prefix preserved
2853+
app_mock.assert_called_once()
2854+
2855+
@pytest.mark.asyncio
2856+
async def test_rewrite_servers_mcp_with_multilevel_prefix(self):
2857+
"""Handle complex multi-level prefixes."""
2858+
app_mock = AsyncMock()
2859+
middleware = MCPPathRewriteMiddleware(app_mock)
2860+
scope = {
2861+
"type": "http",
2862+
"path": "/dev/mcp-gateway/service/gateway/servers/123/mcp",
2863+
"root_path": "/dev/mcp-gateway/service/gateway",
2864+
"headers": []
2865+
}
2866+
receive, send = AsyncMock(), AsyncMock()
2867+
2868+
with patch("mcpgateway.main.streamable_http_auth", return_value=True):
2869+
await middleware._call_streamable_http(scope, receive, send)
2870+
2871+
assert scope["path"] == "/dev/mcp-gateway/service/gateway/mcp/"
2872+
app_mock.assert_called_once()
2873+
2874+
@pytest.mark.asyncio
2875+
async def test_rewrite_servers_mcp_with_prefix_in_path_no_scope_root(self):
2876+
"""Handle prefix in path when scope['root_path'] is empty but APP_ROOT_PATH is set."""
2877+
app_mock = AsyncMock()
2878+
middleware = MCPPathRewriteMiddleware(app_mock)
2879+
scope = {
2880+
"type": "http",
2881+
"path": "/gateway/servers/123/mcp",
2882+
"root_path": "", # Not set in scope
2883+
"headers": []
2884+
}
2885+
receive, send = AsyncMock(), AsyncMock()
2886+
2887+
with patch('mcpgateway.config.settings.app_root_path', '/gateway'):
2888+
with patch("mcpgateway.main.streamable_http_auth", return_value=True):
2889+
await middleware._call_streamable_http(scope, receive, send)
2890+
2891+
assert scope["path"] == "/gateway/mcp/"
2892+
app_mock.assert_called_once()
2893+
2894+
@pytest.mark.asyncio
2895+
async def test_workaround_mcp_servers_passes_through(self):
2896+
"""Undocumented workaround /mcp/servers/{id} passes through without rewrite."""
2897+
app_mock = AsyncMock()
2898+
middleware = MCPPathRewriteMiddleware(app_mock)
2899+
scope = {"type": "http", "path": "/mcp/servers/123", "headers": []}
2900+
receive, send = AsyncMock(), AsyncMock()
2901+
2902+
with patch("mcpgateway.main.streamable_http_auth", return_value=True):
2903+
await middleware._call_streamable_http(scope, receive, send)
2904+
2905+
assert scope["path"] == "/mcp/servers/123" # NOT rewritten
2906+
app_mock.assert_called_once()
2907+
2908+
@pytest.mark.asyncio
2909+
async def test_workaround_mcp_servers_with_prefix(self):
2910+
"""Workaround /mcp/servers/{id} with prefix passes through."""
2911+
app_mock = AsyncMock()
2912+
middleware = MCPPathRewriteMiddleware(app_mock)
2913+
scope = {
2914+
"type": "http",
2915+
"path": "/gateway/mcp/servers/123",
2916+
"root_path": "/gateway",
2917+
"headers": []
2918+
}
2919+
receive, send = AsyncMock(), AsyncMock()
2920+
2921+
with patch("mcpgateway.main.streamable_http_auth", return_value=True):
2922+
await middleware._call_streamable_http(scope, receive, send)
2923+
2924+
assert scope["path"] == "/gateway/mcp/servers/123" # NOT rewritten
2925+
app_mock.assert_called_once()
2926+
2927+
@pytest.mark.asyncio
2928+
async def test_security_arbitrary_prefix_not_rewritten(self):
2929+
"""PR #3892 security: Arbitrary paths ending with /mcp are NOT rewritten."""
2930+
app_mock = AsyncMock()
2931+
middleware = MCPPathRewriteMiddleware(app_mock)
2932+
scope = {
2933+
"type": "http",
2934+
"path": "/arbitrary/prefix/mcp",
2935+
"root_path": "",
2936+
"headers": []
2937+
}
2938+
receive, send = AsyncMock(), AsyncMock()
2939+
2940+
with patch("mcpgateway.main.streamable_http_auth", return_value=True):
2941+
await middleware._call_streamable_http(scope, receive, send)
2942+
2943+
# Should pass through WITHOUT rewrite (security check)
2944+
assert scope["path"] == "/arbitrary/prefix/mcp"
2945+
app_mock.assert_called_once()
2946+
2947+
@pytest.mark.asyncio
2948+
async def test_security_arbitrary_prefix_with_root_path(self):
2949+
"""Security: Arbitrary paths rejected even with root_path."""
2950+
app_mock = AsyncMock()
2951+
middleware = MCPPathRewriteMiddleware(app_mock)
2952+
scope = {
2953+
"type": "http",
2954+
"path": "/gateway/arbitrary/prefix/mcp",
2955+
"root_path": "/gateway",
2956+
"headers": []
2957+
}
2958+
receive, send = AsyncMock(), AsyncMock()
2959+
2960+
with patch("mcpgateway.main.streamable_http_auth", return_value=True):
2961+
await middleware._call_streamable_http(scope, receive, send)
2962+
2963+
# After stripping root_path, path is "/arbitrary/prefix/mcp"
2964+
# Should pass through WITHOUT rewrite
2965+
assert scope["path"] == "/gateway/arbitrary/prefix/mcp"
2966+
app_mock.assert_called_once()
2967+
2968+
@pytest.mark.asyncio
2969+
async def test_security_empty_server_id_with_prefix_rejected(self):
2970+
"""Security: Empty server ID with prefix is rejected."""
2971+
app_mock = AsyncMock()
2972+
middleware = MCPPathRewriteMiddleware(app_mock)
2973+
scope = {
2974+
"type": "http",
2975+
"path": "/gateway/servers//mcp",
2976+
"root_path": "/gateway",
2977+
"headers": []
2978+
}
2979+
receive, send = AsyncMock(), AsyncMock()
2980+
sent = []
2981+
2982+
async def mock_send(msg):
2983+
sent.append(msg)
2984+
2985+
with patch("mcpgateway.main.streamable_http_auth", return_value=True):
2986+
await middleware._call_streamable_http(scope, receive, mock_send)
2987+
2988+
app_mock.assert_not_called()
2989+
# ORJSONResponse sends http.response.start + http.response.body
2990+
assert any(m.get("status") == 404 for m in sent if m.get("type") == "http.response.start")
2991+
28362992

28372993
class TestServerEndpointCoverage:
28382994
"""Exercise server endpoints and SSE coverage."""

0 commit comments

Comments
 (0)