Refresh the cached MCP parse after webhook mutation

docs/middleware.md

@@ -31,6 +31,8 @@ Two webhook types are supported:
 1. **Mutating webhooks**: Transform the parsed MCP request before later policy evaluation.
 2. **Validating webhooks**: Approve or deny the request after mutation has completed.
 
+A mutating webhook therefore steers what authorization (and audit, telemetry, and usage metrics) evaluates, including the feature/operation mapping used to pick a Cedar policy. This is intended: an operator who configures a mutating webhook — including a fail-open one — is deliberately giving it influence over policy input, not introducing an accident.
+
 When configured together, the effective order is:
 
 1. Audit (wraps everything below, so webhook denials are audited)
@@ -58,6 +60,11 @@ Example config files:
 - [`docs/examples/webhooks.yaml`](examples/webhooks.yaml)
 - [`docs/examples/webhooks.json`](examples/webhooks.json)
 
+### Known limitations
+
+- **Stale Modern headers.** A mutating webhook patches only the JSON body, so after it renames a tool the `Mcp-Method`/`Mcp-Name` headers forwarded to the backend still describe the original name. `ValidateHeaderConsistency` (`pkg/mcp/revision.go:512`) requires the header and body to agree and returns a `RequestHeaderMismatchError` (`CodeHeaderMismatch`) if they don't, but today that check is only wired into vMCP (`pkg/vmcp/server/classification.go:72`), which never runs the mutating webhook — so nothing in ToolHive catches this yet. A spec-conformant Modern (2026-07-28) backend will reject the mismatched request itself, so this fails closed rather than silently mis-authorizing. Practical guidance: a mutating webhook must not rename tools on the Modern path.
+- **Controls outside the parser see the pre-mutation request.** The tool-call filter (`pkg/mcp/tool_filter.go`) reads the raw request body directly, and the rate limiter runs before the mutating webhook in the chain (see the ordering rules below) — both decide against the request as received, not as mutated. A mutating webhook can therefore rename a call into a tool that `--tools` filtering excluded, and the rate limiter debits the bucket for the requested tool rather than the executed one. This is a known gap, not a regression: the tool filter's position ahead of the MCP parser is deliberate (it needs the raw request), as already noted in the ordering rules below.
+
 ## Architecture Diagram
 
 ```mermaid
@@ -1048,11 +1055,14 @@ The middleware chain execution order is critical and controlled by the order in
 5. **Tool Filter Middleware** (if enabled) - Filters available tools in list responses
 6. **Tool Call Filter Middleware** (if enabled) - Filters tool call requests
 7. **MCP Parser Middleware** (always present) - Parses JSON-RPC MCP requests
-8. **Usage Metrics Middleware** (if enabled) - Tracks tool call counts
-9. **Telemetry Middleware** (if enabled) - OpenTelemetry instrumentation
-10. **Authorization Middleware** (if enabled) - Cedar policy evaluation
-11. **Header Forward Middleware** (if configured for remote servers) - Injects custom headers
-12. **Recovery Middleware** (always present) - Catches panics
+8. **Rate Limit Middleware** (if configured) - Enforces per-identity/tool limits using the parsed request
+9. **Mutating Webhook Middleware** (if configured) - Patches the parsed MCP request and republishes the parse
+10. **Validating Webhook Middleware** (if configured) - Approves or denies the (possibly mutated) request
+11. **Usage Metrics Middleware** (if enabled) - Tracks tool call counts
+12. **Telemetry Middleware** (if enabled) - OpenTelemetry instrumentation
+13. **Authorization Middleware** (if enabled) - Cedar policy evaluation
+14. **Header Forward Middleware** (if configured for remote servers) - Injects custom headers
+15. **Recovery Middleware** (always present) - Catches panics
 
 **Important Ordering Rules**:
 - Audit wraps the whole chain (directly inside the body-size limit): every request that passes the size cap produces an audit event no matter which middleware rejects it. It does not need to run inside auth or the parser — those publish the identity and parsed MCP data back to it via `auth.IdentityHolder` and `mcp.ParsedRequestHolder`.
@@ -1062,9 +1072,12 @@ The middleware chain execution order is critical and controlled by the order in
 - Token Exchange must come after Upstream Swap if both are used (can further transform the upstream IdP token)
 - Tool filters should come before MCP Parser to operate on raw requests
 - MCP Parser must come before Authorization (provides structured MCP data)
+- Mutating webhooks must come before Validating webhooks and before Authorization, so policy evaluation sees the patched request
+- Middleware that rewrites the request body must republish the parsed request via `mcp.RepublishParsedMCPRequest` and refresh `r.ContentLength` — `ParsingMiddleware` deliberately parses only once, so every later consumer (authorization, audit, telemetry, usage metrics) reads the cached parse rather than re-reading the body
 - Header Forward executes close to the backend handler (innermost position)
 - Recovery is always last in config, making it the innermost wrapper (the chain wraps in reverse config order, so the first entry is the outermost and runs first)
 - Body-size limit and Origin validation stay OUTSIDE audit: oversized bodies must be rejected before audit buffers request data, and origin validation is a pre-auth DNS-rebind guard. Their rejections (413/403) are the only ones not audited.
+- The list above (steps 1-15) describes the operator/proxyrunner path (`PopulateMiddlewareConfigs` in `pkg/runner/middleware.go`). The CLI flag path (`WithMiddlewareFromFlags` in `pkg/runner/config_builder.go`) has no rate limiting at all, and orders Usage Metrics before the webhooks rather than after (see the comment at `pkg/runner/config_builder.go:700`) — both paths still run Mutating before Validating webhooks and both before Authorization.
 
 ### Custom Authorization Policies
 

pkg/mcp/parser.go

@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"io"
 	"net/http"
 	"strconv"
@@ -141,6 +142,46 @@ func ParsingMiddleware(next http.Handler) http.Handler {
 	})
 }
 
+// RepublishParsedMCPRequest refreshes the cached parse after middleware has
+// rewritten the request body, returning the request to pass downstream.
+// ParsingMiddleware deliberately parses only once, so any middleware that
+// replaces r.Body MUST call this or downstream consumers (authorization,
+// audit, telemetry) will decide on the bytes that arrived rather than the
+// bytes the backend executes.
+//
+// On error, the returned request is nil and must not be passed downstream:
+// the caller is responsible for terminating the request (e.g. writing an
+// error response) instead of proceeding with a stale or absent parse.
+//
+// This only refreshes consumers that read the parse from the request context or
+// from a ParsedRequestHolder. Middleware that inspects the raw body from OUTSIDE
+// ParsingMiddleware — the tool-call filter and the rate limiter — has already
+// decided against the pre-rewrite body and is not corrected by republishing.
+//
+// The caller must also refresh r.ContentLength when it replaces r.Body, or the
+// reverse proxy will reject the forwarded request.
+func RepublishParsedMCPRequest(r *http.Request, body []byte) (*http.Request, error) {
+	// Batch-reject before parsing, using the same guard ParsingMiddleware uses,
+	// so a mutation can never smuggle a batch past authz/audit by rewriting a
+	// single request into an array (see IsBatchRequest's doc comment).
+	if IsBatchRequest(body) {
+		return nil, &BatchUnsupportedError{}
+	}
+
+	parsed := parseMCPRequest(body)
+	if parsed == nil {
+		return nil, errors.New("republished body is not a valid JSON-RPC request")
+	}
+	parsed.MCPMethodHeader = r.Header.Get("Mcp-Method")
+	parsed.MCPNameHeader = r.Header.Get("Mcp-Name")
+
+	if holder, ok := ParsedRequestHolderFromContext(r.Context()); ok {
+		holder.Parsed = parsed
+	}
+
+	return r.WithContext(context.WithValue(r.Context(), MCPRequestContextKey, parsed)), nil
+}
+
 // parsedRequestHolderContextKey is the context key for ParsedRequestHolder.
 type parsedRequestHolderContextKey struct{}
 

pkg/mcp/parser_test.go

@@ -1871,3 +1871,142 @@ func TestParsingMiddlewareIntegration(t *testing.T) {
 		})
 	}
 }
+
+func TestRepublishParsedMCPRequest(t *testing.T) {
+	t.Parallel()
+
+	oldBody := `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"old-tool","arguments":{"a":1}}}`
+	newBody := `{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"new-tool","arguments":{"b":2}}}`
+
+	tests := []struct {
+		name          string
+		body          []byte
+		expectErr     bool
+		expectBatch   bool
+		expectNilReq  bool
+		checkResponse func(t *testing.T, req *http.Request, err error)
+	}{
+		{
+			name: "happy re-parse reflects new body",
+			body: []byte(newBody),
+			checkResponse: func(t *testing.T, req *http.Request, err error) {
+				t.Helper()
+				require.NoError(t, err)
+				parsed := GetParsedMCPRequest(req.Context())
+				require.NotNil(t, parsed)
+				assert.Equal(t, "tools/call", parsed.Method)
+				assert.Equal(t, "new-tool", parsed.ResourceID)
+				assert.Equal(t, map[string]interface{}{"b": float64(2)}, parsed.Arguments)
+				assert.NotNil(t, parsed.Params)
+				assert.Equal(t, int64(2), parsed.ID)
+				assert.False(t, parsed.IsBatch)
+			},
+		},
+		{
+			name:        "batch body rejected before parsing",
+			body:        []byte(`[{"jsonrpc":"2.0","method":"tools/call","id":1}]`),
+			expectErr:   true,
+			expectBatch: true,
+		},
+		{
+			name:        "whitespace-prefixed batch still detected",
+			body:        []byte(" \t[{\"jsonrpc\":\"2.0\",\"method\":\"tools/call\",\"id\":1}]"),
+			expectErr:   true,
+			expectBatch: true,
+		},
+		{
+			name:      "valid JSON that is not a request",
+			body:      []byte(`{"jsonrpc":"2.0","result":{}}`),
+			expectErr: true,
+		},
+		{
+			name:      "empty object is not a request",
+			body:      []byte(`{}`),
+			expectErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			req := httptest.NewRequest(http.MethodPost, "/messages", bytes.NewBufferString(oldBody))
+			req.Header.Set("Content-Type", "application/json")
+			var capturedOldCtx context.Context
+			testHandler := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+				capturedOldCtx = r.Context()
+			})
+			ParsingMiddleware(testHandler).ServeHTTP(httptest.NewRecorder(), req)
+			require.NotNil(t, GetParsedMCPRequest(capturedOldCtx), "precondition: old body must have parsed")
+			req = req.WithContext(capturedOldCtx)
+
+			republished, err := RepublishParsedMCPRequest(req, tt.body)
+
+			if tt.expectErr {
+				require.Error(t, err)
+				assert.Nil(t, republished)
+				if tt.expectBatch {
+					var batchErr *BatchUnsupportedError
+					assert.ErrorAs(t, err, &batchErr)
+				}
+				return
+			}
+
+			require.NoError(t, err)
+			require.NotNil(t, republished)
+			if tt.checkResponse != nil {
+				tt.checkResponse(t, republished, err)
+			}
+
+			// The original request's context must still yield the OLD parse:
+			// RepublishParsedMCPRequest must not mutate the caller's request.
+			oldParsed := GetParsedMCPRequest(req.Context())
+			require.NotNil(t, oldParsed)
+			assert.Equal(t, "old-tool", oldParsed.ResourceID)
+		})
+	}
+}
+
+func TestRepublishParsedMCPRequestHeaders(t *testing.T) {
+	t.Parallel()
+
+	req := httptest.NewRequest(http.MethodPost, "/messages", bytes.NewBufferString(""))
+	req.Header.Set("Mcp-Method", "tools/call")
+	req.Header.Set("Mcp-Name", "some-tool")
+
+	body := []byte(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"weather"}}`)
+	republished, err := RepublishParsedMCPRequest(req, body)
+	require.NoError(t, err)
+
+	parsed := GetParsedMCPRequest(republished.Context())
+	require.NotNil(t, parsed)
+	assert.Equal(t, "tools/call", parsed.MCPMethodHeader)
+	assert.Equal(t, "some-tool", parsed.MCPNameHeader)
+}
+
+func TestRepublishParsedMCPRequestHolder(t *testing.T) {
+	t.Parallel()
+	body := []byte(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"weather"}}`)
+
+	t.Run("holder present is refreshed", func(t *testing.T) {
+		t.Parallel()
+		req := httptest.NewRequest(http.MethodPost, "/messages", bytes.NewBufferString(""))
+		holder := &ParsedRequestHolder{}
+		req = req.WithContext(WithParsedRequestHolder(req.Context(), holder))
+
+		republished, err := RepublishParsedMCPRequest(req, body)
+		require.NoError(t, err)
+		require.NotNil(t, republished)
+		require.NotNil(t, holder.Parsed)
+		assert.Equal(t, "weather", holder.Parsed.ResourceID)
+	})
+
+	t.Run("holder absent does not panic", func(t *testing.T) {
+		t.Parallel()
+		req := httptest.NewRequest(http.MethodPost, "/messages", bytes.NewBufferString(""))
+
+		republished, err := RepublishParsedMCPRequest(req, body)
+		require.NoError(t, err)
+		require.NotNil(t, republished)
+	})
+}

pkg/runner/config_builder.go

@@ -699,8 +699,11 @@ func WithMiddlewareFromFlags(
 		//
 		// NOTE: addCoreMiddlewares also injects usage metrics before webhook insertion here,
 		// which differs slightly from PopulateMiddlewareConfigs where usage metrics is added
-		// after webhooks. This is currently benign because usage metrics does not depend on
-		// webhook state, and the broader ordering TODO remains to unify these paths.
+		// after webhooks. Since mutating webhooks republish the parsed request
+		// (mcp.RepublishParsedMCPRequest), the two paths now disagree when a webhook rewrites
+		// the JSON-RPC method: this path counts the method as received, the operator path counts
+		// the method as mutated. Only tool-call counts are affected, and only for a webhook that
+		// patches "method" itself. The broader ordering TODO remains to unify these paths.
 
 		// Add Mutating webhooks before Validating webhooks
 		var err error

pkg/webhook/mutating/middleware.go

@@ -129,8 +129,30 @@ func createMutatingHandler(executors []clientExecutor, serverName, transport str
 				return
 			}
 
+			// A mutating webhook rewrote the body, so the parse cached by ParsingMiddleware
+			// now describes a request the backend will not execute. Refresh it before any
+			// downstream consumer (authorization, audit, telemetry) reads it.
+			if !bytes.Equal(bodyBytes, mutatedBody) {
+				republished, err := mcp.RepublishParsedMCPRequest(r, mutatedBody)
+				if err != nil {
+					var batchErr *mcp.BatchUnsupportedError
+					if errors.As(err, &batchErr) {
+						mcp.WriteBatchUnsupportedError(w)
+						return
+					}
+					slog.Error("Mutating webhook produced an unparsable MCP request", "error", err)
+					sendErrorResponse(w, http.StatusInternalServerError, "Webhook produced an invalid MCP request", parsedMCP.ID)
+					return
+				}
+				r = republished
+			}
+
 			// Replace the request body with the (potentially mutated) MCP body for downstream handlers.
+			// ContentLength must be updated alongside it: a patch almost always changes the body
+			// length, and the reverse proxy rejects a forwarded request whose declared length
+			// disagrees with the body it can actually read.
 			r.Body = io.NopCloser(bytes.NewBuffer(mutatedBody))
+			r.ContentLength = int64(len(mutatedBody))
 			next.ServeHTTP(w, r)
 		})
 	}