vMCP authz-gate hardening: unify parse source, reserved-name guard, verb parity
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
Hardening follow-ups split out of the vMCP authz-gate PR (#5841, fixes #5827) so they don't hold up the 403 fix. All three are reviewer notes that don't affect current correctness.
1. Unify the gate's parse source with the enforced parse (argument-conditional policies)
pkg/vmcp/server/call_gate.go authorizes tools/call on parsed.Arguments (decoded by pkg/mcp's parser), while dispatch re-authorizes on the SDK's decode of the same request bytes. Both are encoding/json over identical bytes, so they agree for plain JSON today. If the decoders ever diverge (json.Number vs float64, a future typed-params path, duplicate-key handling), an argument-conditional Cedar policy could have the gate ALLOW while the call path DENIES — re-introducing the exact 200 + IsError this PR removed (or a spurious 403). Make the gated decision and the enforced decision derive from a single parse (e.g. carry the gate's decision forward, or share one decoded argument map). Raised by @rdimitrov.
2. Fail-loud collision check for the execute_tool_script reserved name
pkg/vmcp/codemode/decorator.go's CheckToolCall admits execute_tool_script unconditionally by name. If a backend ever advertised a real tool literally named execute_tool_script, it would be shadowed at ListTools/LookupTool and its Cedar policy would never run. The name is effectively reserved for the codemode virtual tool, but nothing enforces that. Add a fail-loud collision check at aggregation/ListTools time so a real backend tool with that name is a loud error, not a silent shadow. Raised by @jhrozek.
3. Verb parity between the vMCP gate and thv run's authz middleware
thv run's authz middleware denies elicitation/create, sampling/createMessage, and tasks/* by default (pkg/authz/middleware.go). The vMCP pre-dispatch gate admits them, because the core admission seam has no Cedar entity for these method kinds yet (its default is deliberately fail-open — see the gate's default case). When vMCP grows an admission decision for these verbs, add the matching gate cases so the two paths share one posture. Raised by @jhrozek.
Context: split out of stacklok/toolhive#5841 (fixes #5827).
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
Trace the gate in pkg/vmcp/server/call_gate.go alongside the parser in pkg/mcp, then inspect CheckToolCall in pkg/vmcp/codemode/decorator.go and the defaults in pkg/authz/middleware.go. Done means the gate and dispatch share one parse, reserved-name collisions fail loudly, and future verb handling has documented parity without changing current admission behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend, security
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100