stacklok / stacklok/toolhive

vMCP authz-gate hardening: unify parse source, reserved-name guard, verb parity

Open
#5,845 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.