Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions pkg/authz/clientresponse_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc.
// SPDX-License-Identifier: Apache-2.0

package authz

import (
"net/http"
"net/http/httptest"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/stacklok/toolhive/pkg/authz/authorizers/cedar"
mcpparser "github.com/stacklok/toolhive/pkg/mcp"
)

func TestMiddlewareAcceptsClientResponse(t *testing.T) {
t.Parallel()

authorizer, err := cedar.NewCedarAuthorizer(cedar.ConfigOptions{
Policies: []string{`permit(principal, action, resource);`},
EntitiesJSON: `[]`,
}, "")
require.NoError(t, err)

for _, tt := range []struct {
name string
body string
}{
{"ping result", `{"jsonrpc":"2.0","id":1,"result":{}}`},
{"error response", `{"jsonrpc":"2.0","id":2,"error":{"code":-32601,"message":"Method not found"}}`},
} {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

var handlerCalled bool
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
handlerCalled = true
w.WriteHeader(http.StatusAccepted)
})
middleware := mcpparser.ParsingMiddleware(Middleware(authorizer, handler, nil))

req, err := http.NewRequest(http.MethodPost, "/mcp", strings.NewReader(tt.body))
require.NoError(t, err)
req.Header.Set("Content-Type", "application/json")

rr := httptest.NewRecorder()
middleware.ServeHTTP(rr, req)

assert.True(t, handlerCalled, "a client response must reach the transport")
assert.Equal(t, http.StatusAccepted, rr.Code)
})
}
}
9 changes: 9 additions & 0 deletions pkg/authz/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,15 @@ func Middleware(a authorizers.Authorizer, next http.Handler, passThroughTools ma
// Get parsed MCP request from context (set by parsing middleware)
parsedRequest := mcp.GetParsedMCPRequest(r.Context())
if parsedRequest == nil {
// A JSON-RPC response or error answers a request the SERVER
// initiated (ping, elicitation, sampling). It names no method and
// reaches no tool, so there is nothing to authorize, and the
// streamable-HTTP spec requires the transport to accept it with
// 202. Rejecting it tears the client's session down (#5009).
if mcp.IsClientResponse(r.Context()) {
next.ServeHTTP(w, r)
return
}
// Non-JSON POSTs are already rejected by the early return above,
// so a nil parsed request here means a malformed JSON body or a
// missing parsing middleware. This branch is now only a
Expand Down
35 changes: 35 additions & 0 deletions pkg/mcp/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ type contextKey string
const (
// MCPRequestContextKey is the context key for storing parsed MCP request data.
MCPRequestContextKey contextKey = "mcp_request"

// ClientResponseContextKey marks a POST body that decoded as a JSON-RPC
// response or error rather than a request, so downstream middleware can
// tell one apart from a body that failed to parse at all.
ClientResponseContextKey contextKey = "mcp_client_response"
)

// ParsedMCPRequest contains the parsed MCP request information.
Expand Down Expand Up @@ -125,6 +130,12 @@ func ParsingMiddleware(next http.Handler) http.Handler {

// Parse the MCP request and store in context
parsedRequest := parseMCPRequest(bodyBytes)
if parsedRequest == nil && isClientResponseBody(bodyBytes) {
// Not a request, but well-formed: it answers a request the server
// initiated. Record that so downstream middleware does not confuse
// it with an unparsable body.
r = r.WithContext(context.WithValue(r.Context(), ClientResponseContextKey, true))
}
if parsedRequest != nil {
parsedRequest.MCPMethodHeader = r.Header.Get("Mcp-Method")
parsedRequest.MCPNameHeader = r.Header.Get("Mcp-Name")
Expand Down Expand Up @@ -258,6 +269,30 @@ func RequestHasJSONContentType(r *http.Request) bool {
return strings.EqualFold(mediaType, "application/json")
}

// isClientResponseBody reports whether bodyBytes is a well-formed JSON-RPC
// response or error, i.e. a client's answer to a server-initiated request
// (ping, elicitation, sampling) rather than a call the client is making.
func isClientResponseBody(bodyBytes []byte) bool {
if len(bodyBytes) == 0 {
return false
}
msg, err := jsonrpc2.DecodeMessage(bodyBytes)
if err != nil {
return false
}
_, ok := msg.(*jsonrpc2.Response)
return ok
}

// IsClientResponse reports whether the request body decoded as a JSON-RPC
// response or error rather than a request. A nil result from
// [GetParsedMCPRequest] means either this or an unparsable body; callers that
// must distinguish the two use this.
func IsClientResponse(ctx context.Context) bool {
v, _ := ctx.Value(ClientResponseContextKey).(bool)
return v
}

// GetParsedMCPRequest retrieves the parsed MCP request from the request context.
// Returns nil if no parsed request is available.
func GetParsedMCPRequest(ctx context.Context) *ParsedMCPRequest {
Expand Down
110 changes: 110 additions & 0 deletions pkg/vmcp/client/getreject_realbackend_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc.
// SPDX-License-Identifier: Apache-2.0

package client

import (
"context"
"net/http"
"net/http/httptest"
"sync"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

mcpmcp "github.com/stacklok/toolhive-core/mcpcompat/mcp"
mcpserver "github.com/stacklok/toolhive-core/mcpcompat/server"
"github.com/stacklok/toolhive/pkg/vmcp"
)

// newGETRejectingEchoServer stands up the same real go-sdk streamable-HTTP
// backend as newRealEchoServer, stateful (Legacy), behind a handler that
// answers every HTTP GET with 400 and records the method of every request it
// receives.
//
// That is the Tableau MCP shape from issue #6497: a healthy backend that
// requires a session id issued by initialize, so a bare GET — which carries no
// session context — is rejected rather than upgraded to a standalone SSE
// stream.
func newGETRejectingEchoServer(t *testing.T, record func(method string)) *httptest.Server {
t.Helper()

mcpSrv := mcpserver.NewMCPServer("get-rejecting-backend", "1.0.0")
mcpSrv.AddTool(
mcpmcp.NewTool("echo",
mcpmcp.WithDescription("Echoes the input back"),
mcpmcp.WithString("input", mcpmcp.Required()),
),
func(_ context.Context, req mcpmcp.CallToolRequest) (*mcpmcp.CallToolResult, error) {
args, _ := req.Params.Arguments.(map[string]any)
input, _ := args["input"].(string)
return &mcpmcp.CallToolResult{Content: []mcpmcp.Content{mcpmcp.NewTextContent(input)}}, nil
},
)

inner := mcpserver.NewStreamableHTTPServer(mcpSrv)
mux := http.NewServeMux()
mux.Handle("/mcp", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
record(r.Method)
if r.Method == http.MethodGet {
// Tableau MCP's response to a session-less GET.
http.Error(w, "Invalid or missing session ID", http.StatusBadRequest)
return
}
inner.ServeHTTP(w, r)
}))
ts := httptest.NewServer(mux)
t.Cleanup(ts.Close)
return ts
}

// TestListCapabilities_GETRejectingBackendIsHealthy pins the health signal
// reported in issue #6497: a backend that rejects a bare HTTP GET must not be
// treated as unavailable.
//
// vMCP's health check is BackendClient.ListCapabilities (see
// health.NewHealthChecker), which reaches the backend over POST. GET is only
// ever the standalone server->client SSE stream, and in Streamable HTTP that
// stream is optional — a server without one answers 405 per spec, and one that
// requires a session id (Tableau MCP) answers 400. Neither says anything about
// whether the backend can serve MCP.
//
// The assertion is therefore two-sided: ListCapabilities must succeed against
// such a backend, AND it must not issue a GET at all. Only the tools/call
// forwarding path enables transport.WithContinuousListening (see
// newStreamableHTTPClient); a change that opened the standalone stream on the
// non-forwarding path would put every session-requiring backend back to
// "unavailable" and drop its tools from tools/list.
func TestListCapabilities_GETRejectingBackendIsHealthy(t *testing.T) {
t.Parallel()

var mu sync.Mutex
var methods []string
srv := newGETRejectingEchoServer(t, func(method string) {
mu.Lock()
defer mu.Unlock()
methods = append(methods, method)
})

h := newProbeClient(t)
target := &vmcp.BackendTarget{
WorkloadID: "get-rejecting-backend",
WorkloadName: "GET Rejecting Backend",
BaseURL: srv.URL + "/mcp",
TransportType: "streamable-http",
}

caps, err := h.ListCapabilities(context.Background(), target)
require.NoError(t, err,
"a backend that rejects a bare GET is still a healthy MCP backend: the health check speaks MCP over POST")
require.NotNil(t, caps)
assert.Len(t, caps.Tools, 1)
assert.Equal(t, "echo", caps.Tools[0].Name)

mu.Lock()
defer mu.Unlock()
require.NotEmpty(t, methods, "the backend must have been reached")
assert.NotContains(t, methods, http.MethodGet,
"the health path must not open a standalone SSE GET stream; only the tools/call forwarding path may")
}
Loading