stacklok / stacklok/toolhive

Priority tool resolver can annex an unlisted backend's tool name

Open
#6,097 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

needs-triage
Dominant language
Go
Stars
2.2k
Forks
300
Avg merge
1d 15h
Merged PRs (30d)
184

Description

Summary

Split out of the #6075 review (https://github.com/stacklok/toolhive/pull/6075#discussion_r3665086460), where @jhrozek flagged this as pre-existing on the tools path and worth its own issue: the same name-annexation hole that #6075's review closed for prompts exists for tools under the priority strategy today.

Details

PriorityConflictResolver drops every non-winning candidate for a conflicting tool name — including candidates from backends that are not listed in priorityOrder at all and were therefore never rank-compared (pkg/vmcp/aggregator/priority_resolver.go:125-132, selectWinner at :147-167).

Cedar's tool resource identity is name-only — Tool::"<advertised name>", with no backend or workload attribute anywhere in pkg/authz/authorizers/cedar (cedar/core.go:1049). Whoever wins an advertised name inherits every authorization policy written for it.

Combined effect: an unlisted backend's uniquely-named tool can be annexed by a listed backend that later ships a tool of the same name:

  1. Backend b2 (not in priorityOrder) advertises tool deploy; no conflict exists, the name passes through bare. An operator writes permit(... resource == Tool::"deploy") for it.
  2. Backend b1 (listed in priorityOrder) later adds a tool named deploy.
  3. b1 wins the conflict by rank; b2's tool is dropped. The existing permit now authorizes b1's different tool, with no config edit and no signal to the operator. b2's tool becomes unreachable.

The forbid direction fails closed (the loser is dropped, not re-advertised under another name), but permit policies are silently redirected to the annexing backend.

Possible directions (from the review discussion, not prescriptive)

  • Treat a collision involving a backend absent from priorityOrder differently from a rank-compared collision (e.g. prefix or drop rather than silently awarding the name to the listed backend).
  • Longer-term: give Cedar entities a backend/workload attribute so policies can bind to more than the bare name.

Related: the prompts-side counterpart of this was addressed in #6075 (loser dropped, invariant documented) with a follow-up PR tightening the ambiguity handling.

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

Start with pkg/vmcp/aggregator/priority_resolver.go:125-132 and selectWinner at :147-167, then compare the prompts-side handling in #6075. Review cedar/core.go:1049 to understand the name-only Tool identity. Done means a collision with an unlisted backend cannot silently redirect an existing permit policy to another backend, with the chosen behavior covered by resolver tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
authorization, backend-api-design, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.