Tool filtering and rate limiting are enforced against the pre-mutation request
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
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:
- 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. - Reject any mutation that changes the tool name, i.e. narrow
IsPatchScopedToMCPRequestso
/mcp_request/params/nameand/mcp_request/methodare not patchable. This closes the filter
gap, the rate-limit gap, and the stale-Mcp-Namelimitation in one move, at the cost of
forbidding a rewrite the webhook API currently advertises. - 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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the middleware ordering in pkg/runner/middleware.go and pkg/runner/config_builder.go, then read pkg/mcp/tool_filter.go, pkg/ratelimit/middleware.go, and IsPatchScopedToMCPRequest in pkg/webhook/mutating/patch.go. Decide which stated behavior should govern mutations of method or params/name, and make filtering and rate limiting consistent with that decision without overlooking the documented raw-request ordering.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100