Refresh the cached MCP parse after webhook mutation

[jhrozek]

Summary

• When a mutating webhook and authorization are both configured, ToolHive authorized the
  pre-mutation MCP request while the backend executed the post-mutation body.
  ParsingMiddleware parses the body once and publishes the parse into the request context and
  into a ParsedRequestHolder, then refuses to parse again. The mutating webhook replaced
  r.Body but passed the request through unchanged, so Cedar evaluated policy against the tool
  name and arguments that arrived, not the ones that ran. docs/middleware.md:31 documented the
  opposite behavior.
• The audit half shipped in the default configuration: the event type and target.name are
  not gated on includeRequestData, so the audit trail named a request that never executed.
  Telemetry and usage metrics drifted the same way.
• Republish the parse at the mutation site when the body actually changed. One change corrects
  every consumer that reads the cached parse, rather than patching four of them. The re-parse
  routes through the same IsBatchRequest guard as ParsingMiddleware, so a patched body still
  cannot smuggle a JSON-RPC batch past controls that inspect one request per call. An unparseable
  result fails closed instead of reaching the backend.
• Also refresh r.ContentLength alongside r.Body. A patch almost always changes the body
  length, and the reverse proxy rejects a forwarded request whose declared length disagrees with
  its readable body. This was a pre-existing bug at the same lines.

Fixes #6133

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Unit tests were run per-package with the Taskfile's flags
(go test -ldflags=-extldflags=-Wl,-w -race) rather than as a full task test sweep, across
pkg/mcp, pkg/webhook/..., pkg/authz/..., pkg/audit, pkg/runner/..., pkg/telemetry/...,
pkg/usagemetrics and pkg/ratelimit/... — 18 packages green from a cleared test cache, plus
go build ./.... task lint-fix has not been run; relying on CI for that.

The three regression tests were written first and confirmed failing against unmodified code, each
on a value-mismatch assertion showing the pre-mutation value (read_file, SELECT *), not on a
compile error or panic.

Changes

File	Change
pkg/mcp/parser.go	Adds RepublishParsedMCPRequest: batch-guards, re-parses, re-attaches the Modern header fields, refreshes both the context value and the holder.
pkg/mcp/parser_test.go	Table-driven coverage: happy re-parse, header re-attach, holder present/absent, batch (incl. whitespace-prefixed), unparseable bodies, no mutation of the caller's request.
pkg/webhook/mutating/middleware.go	Calls the helper when the body changed; refreshes ContentLength; batch errors reuse WriteBatchUnsupportedError, others reuse sendErrorResponse.
pkg/webhook/mutating/middleware_test.go	Five tests: tool rename, argument rewrite, holder refresh (the audit half), method removed → conformant 500, and the no-mutation path.
pkg/runner/config_builder.go	Comment only. Its claim that the usage-metrics ordering split was "benign because usage metrics does not depend on webhook state" is falsified by this change.
docs/middleware.md	Reconciles two contradictory ordering lists, records the republish contract as a decision, documents two gaps this does not close.

Does this introduce a user-facing change?

Yes. With a mutating webhook and authorization both configured, Cedar policy is now evaluated
against the mutated request rather than the request as received, and audit events, spans and
metrics name the tool that actually executed. Operators whose policies were written against
un-normalized values — and were passing only because policy never saw the webhook's rewrite —
may see decisions change. That is the documented contract (docs/middleware.md:31) taking effect.

Special notes for reviewers

Why bytes.Equal gates the republish. Only republishing when the body changed bounds the
change to the mutation case. Without it, every no-patch and fail-open path would newly re-parse,
and five existing fail-open tests pass a body of {"jsonrpc":"2.0","id":1} that has no method
and so is not a parseable JSON-RPC request — they would start returning 500. The guard is the
correct answer rather than a workaround: in production an unmutated body provably parsed already,
because this middleware returns early when no parse exists.

Two gaps this deliberately does not close, both documented rather than fixed:

1. After a webhook renames a tool, the Mcp-Method/Mcp-Name headers forwarded to the backend
   still describe the original name, since a webhook patches only the JSON body.
   ValidateHeaderConsistency is enforced only in vMCP today, which never wires this middleware,
   so nothing here catches it — but a conformant Modern backend rejects the mismatch, so it fails
   closed. Re-rendering headers means fabricating client-supplied values with sentinel encoding,
   which is a separate decision.
2. The tool-call filter and rate limiter run outside ParsingMiddleware and so still decide
   against the request as received. Republishing cannot reach them. Filed separately as Tool filtering and rate limiting are enforced against the pre-mutation request #6134.

Coupling with #5800. That PR moves telemetry from after the webhook layers to immediately
after the parser, which would put telemetry outside the mutation and make it read the pre-mutation
parse again — it has no ParsedRequestHolder, and a derived context only flows downstream, so
republishing cannot reach it there. Whichever lands second needs to reconcile. If this merges
first, #5800 should update the ordering list and limitations bullet in docs/middleware.md, the
inline comment at the republish site, and the RepublishParsedMCPRequest doc comment, which names
the rate limiter as an uncorrected consumer. I have commented on #5800 with the details. There is
no textual conflict in either direction.

Latent, not fixed here. This makes the holder a fresher copy than the context value, where
previously both were equally stale. pkg/audit/auditor.go reads context-then-holder. Audit is
outermost in every wired chain, so its own context never holds a parse and it always falls through
to the fresh holder — the precedence would only matter if the parser ran outside audit while a
mutating webhook ran inside, which no current chain does. Left alone as out of scope.

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 86.95652% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.51%. Comparing base (0a0cbd9) to head (9a25e53).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/webhook/mutating/middleware.go	75.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6136      +/-   ##
==========================================
- Coverage   72.56%   72.51%   -0.05%     
==========================================
  Files         734      734              
  Lines       75912    75939      +27     
==========================================
- Hits        55084    55067      -17     
- Misses      16920    16980      +60     
+ Partials     3908     3892      -16     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.