From ee4954a2e1d12f1f764bc13ef44479211d65e099 Mon Sep 17 00:00:00 2001 From: Emre K <110906681+kocaemre@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:25:14 +0200 Subject: [PATCH 1/2] Avoid priority annexing unlisted tools Signed-off-by: Emre K <110906681+kocaemre@users.noreply.github.com> --- pkg/vmcp/aggregator/conflict_resolver_test.go | 47 +++++++++++++ pkg/vmcp/aggregator/priority_resolver.go | 68 ++++++++++++------- 2 files changed, 90 insertions(+), 25 deletions(-) diff --git a/pkg/vmcp/aggregator/conflict_resolver_test.go b/pkg/vmcp/aggregator/conflict_resolver_test.go index 247b29b794..d41c2798b0 100644 --- a/pkg/vmcp/aggregator/conflict_resolver_test.go +++ b/pkg/vmcp/aggregator/conflict_resolver_test.go @@ -207,6 +207,53 @@ func TestPriorityConflictResolver(t *testing.T) { "teams_send_message": vmcp.ConflictStrategyPrefix, // Prefix fallback used }, }, + { + name: "mixed listed and unlisted conflict uses prefix fallback", + priorityOrder: []string{"github"}, + toolsByBackend: map[string][]vmcp.Tool{ + "github": { + {Name: "deploy", Description: "GitHub deploy"}, + }, + "prod": { + {Name: "deploy", Description: "Production deploy"}, + }, + }, + wantCount: 2, + wantWinners: map[string]string{ + "github_deploy": "github", + "prod_deploy": "prod", + }, + wantStrategies: map[string]vmcp.ConflictResolutionStrategy{ + "github_deploy": vmcp.ConflictStrategyPrefix, + "prod_deploy": vmcp.ConflictStrategyPrefix, + }, + }, + { + name: "three-way mixed listed and unlisted conflict uses prefix fallback", + priorityOrder: []string{"github", "staging"}, + toolsByBackend: map[string][]vmcp.Tool{ + "github": { + {Name: "deploy", Description: "GitHub deploy"}, + }, + "staging": { + {Name: "deploy", Description: "Staging deploy"}, + }, + "prod": { + {Name: "deploy", Description: "Production deploy"}, + }, + }, + wantCount: 3, + wantWinners: map[string]string{ + "github_deploy": "github", + "staging_deploy": "staging", + "prod_deploy": "prod", + }, + wantStrategies: map[string]vmcp.ConflictResolutionStrategy{ + "github_deploy": vmcp.ConflictStrategyPrefix, + "staging_deploy": vmcp.ConflictStrategyPrefix, + "prod_deploy": vmcp.ConflictStrategyPrefix, + }, + }, { name: "empty priority order", priorityOrder: []string{}, diff --git a/pkg/vmcp/aggregator/priority_resolver.go b/pkg/vmcp/aggregator/priority_resolver.go index afa0e88d3c..2d9b7fa601 100644 --- a/pkg/vmcp/aggregator/priority_resolver.go +++ b/pkg/vmcp/aggregator/priority_resolver.go @@ -12,11 +12,12 @@ import ( ) // PriorityConflictResolver implements priority-based conflict resolution. -// The first backend in the priority order wins; conflicting tools from -// lower-priority backends are dropped. +// When every conflicting backend is listed in the priority order, the first +// backend in that order wins and lower-priority tools are dropped. // -// For backends not in the priority list, conflicts are resolved using -// prefix strategy as a fallback (prevents data loss). +// When any conflicting backend is absent from the priority list, all candidates +// in that conflict use the prefix strategy as a fallback to prevent a listed +// backend from annexing the bare tool name. type PriorityConflictResolver struct { // PriorityOrder defines the priority of backends (first has highest priority). PriorityOrder []string @@ -82,35 +83,23 @@ func (r *PriorityConflictResolver) ResolveToolConflicts( continue } - // Conflict detected - choose the highest priority backend - winner := r.selectWinner(candidates) - if winner == nil { - // All candidates are from backends not in priority list - // Use prefix strategy as fallback to avoid data loss + if r.hasUnlistedCandidate(candidates) { + // A collision involving a backend outside priorityOrder cannot be safely + // rank-compared. Prefix every candidate instead of awarding the bare name + // to a listed backend, which could silently redirect name-only policies. backendIDs := make([]string, len(candidates)) for i, c := range candidates { backendIDs[i] = c.BackendID } - slog.Debug("tool exists in backends not in priority order, using prefix fallback", + slog.Warn("tool conflict includes backend not in priority order, using prefix fallback", "tool", toolName, "backends", backendIDs) - // Apply prefix strategy to these unmapped backends - for _, candidate := range candidates { - prefixedName := r.prefixResolver.applyPrefix(candidate.BackendID, toolName) - resolved[prefixedName] = &ResolvedTool{ - ResolvedName: prefixedName, - OriginalName: toolName, - Description: candidate.Tool.Description, - InputSchema: candidate.Tool.InputSchema, - OutputSchema: candidate.Tool.OutputSchema, - Annotations: candidate.Tool.Annotations, - BackendID: candidate.BackendID, - ConflictResolutionApplied: vmcp.ConflictStrategyPrefix, // Fallback used prefix - } - } + r.addPrefixedCandidates(resolved, toolName, candidates) continue } + // Conflict detected among only listed backends; choose the highest priority backend. + winner := r.selectWinner(candidates) resolved[toolName] = &ResolvedTool{ ResolvedName: toolName, OriginalName: toolName, @@ -142,8 +131,37 @@ func (r *PriorityConflictResolver) ResolveToolConflicts( return resolved, nil } +func (r *PriorityConflictResolver) hasUnlistedCandidate(candidates []toolWithBackend) bool { + for _, candidate := range candidates { + if _, exists := r.priorityMap[candidate.BackendID]; !exists { + return true + } + } + return false +} + +func (r *PriorityConflictResolver) addPrefixedCandidates( + resolved map[string]*ResolvedTool, + toolName string, + candidates []toolWithBackend, +) { + for _, candidate := range candidates { + prefixedName := r.prefixResolver.applyPrefix(candidate.BackendID, toolName) + resolved[prefixedName] = &ResolvedTool{ + ResolvedName: prefixedName, + OriginalName: toolName, + Description: candidate.Tool.Description, + InputSchema: candidate.Tool.InputSchema, + OutputSchema: candidate.Tool.OutputSchema, + Annotations: candidate.Tool.Annotations, + BackendID: candidate.BackendID, + ConflictResolutionApplied: vmcp.ConflictStrategyPrefix, // Fallback used prefix + } + } +} + // selectWinner chooses the tool from the highest-priority backend. -// Returns nil if none of the candidates are in the priority list. +// Callers should only pass candidates from backends that are in the priority list. func (r *PriorityConflictResolver) selectWinner(candidates []toolWithBackend) *toolWithBackend { var winner *toolWithBackend winnerPriority := -1 From 834d834666befef7d461456d0988f45a052c5331 Mon Sep 17 00:00:00 2001 From: Emre K <110906681+kocaemre@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:20:13 +0200 Subject: [PATCH 2/2] Drop unrankable priority conflicts Signed-off-by: Emre K <110906681+kocaemre@users.noreply.github.com> --- pkg/vmcp/aggregator/conflict_resolver_test.go | 63 ++++++++++--------- pkg/vmcp/aggregator/priority_resolver.go | 42 +++---------- 2 files changed, 42 insertions(+), 63 deletions(-) diff --git a/pkg/vmcp/aggregator/conflict_resolver_test.go b/pkg/vmcp/aggregator/conflict_resolver_test.go index d41c2798b0..659b5fd13f 100644 --- a/pkg/vmcp/aggregator/conflict_resolver_test.go +++ b/pkg/vmcp/aggregator/conflict_resolver_test.go @@ -123,6 +123,7 @@ func TestPriorityConflictResolver(t *testing.T) { wantCount int wantWinners map[string]string // tool name -> expected backend ID wantStrategies map[string]vmcp.ConflictResolutionStrategy // tool name -> expected strategy (optional) + wantMissing []string // tool names that must not be advertised wantErr bool }{ { @@ -182,7 +183,7 @@ func TestPriorityConflictResolver(t *testing.T) { }, }, { - name: "backends not in priority with conflict use prefix fallback", + name: "unlisted backends with conflict are dropped", priorityOrder: []string{"github"}, toolsByBackend: map[string][]vmcp.Tool{ "github": { @@ -195,20 +196,13 @@ func TestPriorityConflictResolver(t *testing.T) { {Name: "send_message", Description: "Teams message"}, }, }, - wantCount: 3, // All tools included, conflicting ones prefixed + wantCount: 1, // Conflicting unrankable tools are dropped wantWinners: map[string]string{ - "create_issue": "github", // In priority list - "slack_send_message": "slack", // Not in priority, prefixed - "teams_send_message": "teams", // Not in priority, prefixed - }, - wantStrategies: map[string]vmcp.ConflictResolutionStrategy{ - "create_issue": vmcp.ConflictStrategyPriority, // Priority strategy used - "slack_send_message": vmcp.ConflictStrategyPrefix, // Prefix fallback used - "teams_send_message": vmcp.ConflictStrategyPrefix, // Prefix fallback used + "create_issue": "github", // In priority list, no conflict }, }, { - name: "mixed listed and unlisted conflict uses prefix fallback", + name: "mixed listed and unlisted conflict drops all candidates", priorityOrder: []string{"github"}, toolsByBackend: map[string][]vmcp.Tool{ "github": { @@ -218,18 +212,12 @@ func TestPriorityConflictResolver(t *testing.T) { {Name: "deploy", Description: "Production deploy"}, }, }, - wantCount: 2, - wantWinners: map[string]string{ - "github_deploy": "github", - "prod_deploy": "prod", - }, - wantStrategies: map[string]vmcp.ConflictResolutionStrategy{ - "github_deploy": vmcp.ConflictStrategyPrefix, - "prod_deploy": vmcp.ConflictStrategyPrefix, - }, + wantCount: 0, + wantWinners: map[string]string{}, + wantMissing: []string{"deploy", "github_deploy", "prod_deploy"}, }, { - name: "three-way mixed listed and unlisted conflict uses prefix fallback", + name: "three-way mixed conflict drops all candidates", priorityOrder: []string{"github", "staging"}, toolsByBackend: map[string][]vmcp.Tool{ "github": { @@ -242,17 +230,24 @@ func TestPriorityConflictResolver(t *testing.T) { {Name: "deploy", Description: "Production deploy"}, }, }, - wantCount: 3, - wantWinners: map[string]string{ - "github_deploy": "github", - "staging_deploy": "staging", - "prod_deploy": "prod", - }, - wantStrategies: map[string]vmcp.ConflictResolutionStrategy{ - "github_deploy": vmcp.ConflictStrategyPrefix, - "staging_deploy": vmcp.ConflictStrategyPrefix, - "prod_deploy": vmcp.ConflictStrategyPrefix, + wantCount: 0, + wantWinners: map[string]string{}, + wantMissing: []string{"deploy", "github_deploy", "staging_deploy", "prod_deploy"}, + }, + { + name: "drop prevents forbid bypass via prefixed names", + priorityOrder: []string{"github"}, + toolsByBackend: map[string][]vmcp.Tool{ + "github": { + {Name: "deploy", Description: "GitHub deploy"}, + }, + "prod": { + {Name: "deploy", Description: "Production deploy"}, + }, }, + wantCount: 0, + wantWinners: map[string]string{}, + wantMissing: []string{"deploy", "github_deploy", "prod_deploy"}, }, { name: "empty priority order", @@ -314,6 +309,12 @@ func TestPriorityConflictResolver(t *testing.T) { } } } + + for _, toolName := range tt.wantMissing { + if _, exists := resolved[toolName]; exists { + t.Errorf("tool %q should have been dropped", toolName) + } + } }) } } diff --git a/pkg/vmcp/aggregator/priority_resolver.go b/pkg/vmcp/aggregator/priority_resolver.go index 2d9b7fa601..cfee481b47 100644 --- a/pkg/vmcp/aggregator/priority_resolver.go +++ b/pkg/vmcp/aggregator/priority_resolver.go @@ -16,17 +16,15 @@ import ( // backend in that order wins and lower-priority tools are dropped. // // When any conflicting backend is absent from the priority list, all candidates -// in that conflict use the prefix strategy as a fallback to prevent a listed -// backend from annexing the bare tool name. +// in that conflict are dropped because the conflict cannot be safely ranked. +// Dropping preserves the fail-closed policy invariant: a renamed loser would be +// advertised under a name that existing name-scoped forbid policies do not cover. type PriorityConflictResolver struct { // PriorityOrder defines the priority of backends (first has highest priority). PriorityOrder []string // priorityMap is a map from backend ID to its priority index. priorityMap map[string]int - - // prefixResolver is used as fallback for backends not in priority list. - prefixResolver *PrefixConflictResolver } // NewPriorityConflictResolver creates a new priority-based conflict resolver. @@ -45,9 +43,8 @@ func NewPriorityConflictResolver(priorityOrder []string) (*PriorityConflictResol } return &PriorityConflictResolver{ - PriorityOrder: priorityOrder, - priorityMap: priorityMap, - prefixResolver: NewPrefixConflictResolver(defaultPrefixFormat), // Fallback for unmapped backends + PriorityOrder: priorityOrder, + priorityMap: priorityMap, }, nil } @@ -85,16 +82,17 @@ func (r *PriorityConflictResolver) ResolveToolConflicts( if r.hasUnlistedCandidate(candidates) { // A collision involving a backend outside priorityOrder cannot be safely - // rank-compared. Prefix every candidate instead of awarding the bare name - // to a listed backend, which could silently redirect name-only policies. + // rank-compared. Drop every candidate instead of awarding the bare name + // to a listed backend or re-advertising losers under names outside + // existing name-scoped policies. backendIDs := make([]string, len(candidates)) for i, c := range candidates { backendIDs[i] = c.BackendID } - slog.Warn("tool conflict includes backend not in priority order, using prefix fallback", + slog.Error("dropped tool conflict involving backend not in priority order", "tool", toolName, "backends", backendIDs) - r.addPrefixedCandidates(resolved, toolName, candidates) + droppedTools += len(candidates) continue } @@ -140,26 +138,6 @@ func (r *PriorityConflictResolver) hasUnlistedCandidate(candidates []toolWithBac return false } -func (r *PriorityConflictResolver) addPrefixedCandidates( - resolved map[string]*ResolvedTool, - toolName string, - candidates []toolWithBackend, -) { - for _, candidate := range candidates { - prefixedName := r.prefixResolver.applyPrefix(candidate.BackendID, toolName) - resolved[prefixedName] = &ResolvedTool{ - ResolvedName: prefixedName, - OriginalName: toolName, - Description: candidate.Tool.Description, - InputSchema: candidate.Tool.InputSchema, - OutputSchema: candidate.Tool.OutputSchema, - Annotations: candidate.Tool.Annotations, - BackendID: candidate.BackendID, - ConflictResolutionApplied: vmcp.ConflictStrategyPrefix, // Fallback used prefix - } - } -} - // selectWinner chooses the tool from the highest-priority backend. // Callers should only pass candidates from backends that are in the priority list. func (r *PriorityConflictResolver) selectWinner(candidates []toolWithBackend) *toolWithBackend {