Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ func TestGuardUnknownSessionFiresDespiteForgedModernRevision(t *testing.T) {
body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
assert.Contains(t, string(body), `"code":-32001`)
// The 404 echoes the request's JSON-RPC id so a client can correlate it,
// matching the classification-error path (#5945).
assert.Contains(t, string(body), `"id":1`)
assert.False(t, backendCalled.Load(),
"backend must not be contacted: the unknown-session guard must fire regardless of the classified revision")
}
Expand Down
7 changes: 6 additions & 1 deletion pkg/transport/proxy/transparent/transparent_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -598,12 +598,17 @@ func (t *tracingTransport) RoundTrip(req *http.Request) (*http.Response, error)
isJSON := strings.Contains(req.Header.Get("Content-Type"), "application/json")
sawInitialize := false
revision := mcp.RevisionLegacy
// Echoed by the unknown-session guard below so its 404 correlates with the
// request, like every sibling error path (#5945). Stays nil for bodies that
// carry no JSON-RPC id, which NotFoundBody renders as "id":null.
var rpcID json.RawMessage

if len(reqBody) > 0 &&
((isMCP && isJSON) ||
t.p.transportType == types.TransportTypeStreamableHTTP.String()) {
method, params, id, singleRequest, isInit := t.parseRPCRequest(reqBody)
sawInitialize = isInit
rpcID = id
if singleRequest {
meta := mcp.ExtractMeta(params)
rev, cerr := mcp.ClassifyRevision(method, meta, req.Header.Get("MCP-Protocol-Version"))
Expand All @@ -628,7 +633,7 @@ func (t *tracingTransport) RoundTrip(req *http.Request) (*http.Response, error)
slog.Error("session store lookup failed", "error", err)
return plainResponse(req, http.StatusServiceUnavailable, "session store unavailable"), nil
}
return session.NotFoundResponse(req), nil
return session.NotFoundResponse(req, rpcID), nil
}
}

Expand Down
11 changes: 7 additions & 4 deletions pkg/transport/session/jsonrpc_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,14 @@ func WriteNotFound(w http.ResponseWriter, requestID any) {
_, _ = w.Write(NotFoundBody(requestID))
}

// NotFoundResponse constructs an *http.Response with HTTP 404 and a
// JSON-RPC error body. Use this in httputil.ReverseProxy.ModifyResponse
// NotFoundResponse constructs an *http.Response with HTTP 404 and a JSON-RPC
// error body echoing requestID. Use this in httputil.ReverseProxy.ModifyResponse
// (transparent proxy) where no http.ResponseWriter is available.
func NotFoundResponse(req *http.Request) *http.Response {
body := NotFoundBody(nil)
//
// Pass nil when the incoming request carries no JSON-RPC id: a GET or DELETE
// (no body), or a body that did not parse as a single JSON-RPC request.
func NotFoundResponse(req *http.Request, requestID any) *http.Response {
body := NotFoundBody(requestID)
hdr := make(http.Header)
hdr.Set("Content-Type", "application/json")
return &http.Response{
Expand Down
55 changes: 46 additions & 9 deletions pkg/transport/session/jsonrpc_errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,52 @@ func TestWriteNotFound(t *testing.T) {
func TestNotFoundResponse(t *testing.T) {
t.Parallel()

req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
resp := NotFoundResponse(req)
tests := []struct {
name string
requestID any
wantID string
}{
{
name: "no request id echoes null",
requestID: nil,
wantID: `"id":null`,
},
{
name: "string request id is echoed",
requestID: "req-1",
wantID: `"id":"req-1"`,
},
{
// The transparent proxy passes the id straight through as a
// json.RawMessage; a nil one must still render as null rather
// than producing an invalid body.
name: "raw json id is echoed verbatim",
requestID: json.RawMessage(`42`),
wantID: `"id":42`,
},
{
name: "nil raw json id echoes null",
requestID: json.RawMessage(nil),
wantID: `"id":null`,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

assert.Equal(t, http.StatusNotFound, resp.StatusCode)
assert.Equal(t, "application/json", resp.Header.Get("Content-Type"))
assert.Equal(t, req, resp.Request)
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
resp := NotFoundResponse(req, tt.requestID)

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
assert.Contains(t, string(body), `"code":-32001`)
assert.Contains(t, string(body), `"id":null`)
assert.Equal(t, http.StatusNotFound, resp.StatusCode)
assert.Equal(t, "application/json", resp.Header.Get("Content-Type"))
assert.Equal(t, req, resp.Request)

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
assert.Contains(t, string(body), `"code":-32001`)
assert.Contains(t, string(body), tt.wantID)
assert.Equal(t, int64(len(body)), resp.ContentLength)
})
}
}
19 changes: 4 additions & 15 deletions test/e2e/hostile_input_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,7 @@ var _ = Describe("Hostile Input Proxy", Label("proxy", "stateless", "hostile-inp
resp, err := client.Send(context.Background(), proxyURL, req)
Expect(err).ToNot(HaveOccurred(), tc.name)
Expect(resp.StatusCode).To(Equal(tc.wantStatus), tc.name)
if tc.idNotEchoed {
Expect(resp.ID).To(BeNil(), "%s: echoed id", tc.name)
} else {
Expect(resp.ID).To(Equal(json.Number(fmt.Sprintf("%d", tc.id))), "%s: echoed id", tc.name)
}
Expect(resp.ID).To(Equal(json.Number(fmt.Sprintf("%d", tc.id))), "%s: echoed id", tc.name)
Expect(resp.Error).ToNot(BeNil(), tc.name)
Expect(resp.Error.Code).To(Equal(tc.wantCode), tc.name)
if tc.wantData != nil {
Expand Down Expand Up @@ -188,12 +184,6 @@ type hostileCase struct {
wantStatus int
wantCode int64
wantData map[string]any // nil means the error must carry no "data" field at all
// idNotEchoed marks a rejection path that -- unlike ClassificationErrorResponse
// -- doesn't echo the request id at all: session.NotFoundResponse hardcodes a
// nil id (see jsonrpc_errors.go's NotFoundBody(nil) call). A real minor
// asymmetry in the current code, left flagged rather than fixed (out of scope
// for a test-only step).
idNotEchoed bool
}

// hostileRejectionCases enumerates the classification-error and session-guard
Expand Down Expand Up @@ -273,10 +263,9 @@ func hostileRejectionCases() []hostileCase {
// in revision_guard_regression_test.go.
req.WithSessionID("foreign-" + uuid.NewString())
},
wantStatus: http.StatusNotFound,
wantCode: session.CodeSessionNotFound,
wantData: nil,
idNotEchoed: true,
wantStatus: http.StatusNotFound,
wantCode: session.CodeSessionNotFound,
wantData: nil,
},
}
}
Loading