Tool filtering and rate limiting are enforced against the pre-mutation request

[jhrozek]

Tool filtering and rate limiting are enforced against the pre-mutation request

Labels: bug

Bug description

A mutating webhook can rename a tool call into a tool that --tools filtering excluded from the
server, because the tool-call filter decides against the raw body before the mutation happens.

The tool filter and tool-call filter middleware read r.Body directly
(pkg/mcp/tool_filter.go:249) and are appended to the chain before the MCP parser
(pkg/runner/middleware.go:148 and :155, versus the parser at :164 and the mutating webhook at
:177; the CLI path is the same, pkg/runner/config_builder.go:688 versus :707). Because the
chain wraps in reverse append order, the filters are the outer wrappers and run first — so they see
the request as it arrived, and a mutating webhook rewrites the body afterwards.

Consequence: a webhook patching /mcp_request/params/name can turn an allowed tool call into a
call for a tool that --tools filtered out, and the filter has already passed the request.
IsPatchScopedToMCPRequest (pkg/webhook/mutating/patch.go:66) permits that path.

The filters' position is deliberate — docs/middleware.md:1063 states "Tool filters should
come before MCP Parser to operate on raw requests" — so this is a known gap in an intentional
design, not a regression.

Relationship to the parse-staleness fix

This is the same bug family as the mutating-webhook parse staleness (#6133), but a
different mechanism, and the fix for that one does not fix this one. That fix republishes the
cached *ParsedMCPRequest so consumers inside the parser (authorization, audit, telemetry,
usage metrics) see post-mutation data. The tool filters sit outside the parser and read the raw
body, so there is no cached parse to refresh — they have simply already run.

Rate limiting has the same shape

Rate limiting is registered at pkg/runner/middleware.go:169, outside the mutating webhook at
:177, and makes its decision on parsed.ResourceID (pkg/ratelimit/middleware.go:138). It is
therefore consistent by position — it is not reading a stale cached parse — but the semantics
still surprise: it debits the bucket for the tool the caller requested while the backend runs
the tool the webhook produced. A per-tool quota is bypassable by a rename.

Note that the CLI path (pkg/runner/config_builder.go:666-730) has no rate limiting at all.

Expected behavior

Undecided, and that is why this needs its own issue rather than being folded into the other fix.
The options trade off against the reason the filters are positioned early:

1. Re-run tool filtering after mutation. Requires either moving the filters inside the parser
   (losing the "operate on raw requests" property they were placed early for) or running them
   twice.
2. Reject any mutation that changes the tool name, i.e. narrow IsPatchScopedToMCPRequest so
   /mcp_request/params/name and /mcp_request/method are not patchable. This closes the filter
   gap, the rate-limit gap, and the stale-Mcp-Name limitation in one move, at the cost of
   forbidding a rewrite the webhook API currently advertises.
3. Document the gap and treat the tool filter as an ingress-only control.

Option 2 is worth serious consideration: renaming is the mutation that breaks the most invariants,
and no known use case requires it.

Actual behavior

Filtering and per-tool rate limits are enforced against the requested tool, not the executed one.

Additional context

Verified against main @ 0a0cbd949. Preconditions: mutating webhooks configured together with
--tools filtering or rate limiting, plus a webhook that patches method or params/name.