Skip to content

Commit df9489c

Browse files
authored
fix: filter direct tools by agent token scope (#520)
1 parent a667213 commit df9489c

4 files changed

Lines changed: 308 additions & 2 deletions

File tree

internal/server/mcp.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"regexp"
99
"strconv"
1010
"strings"
11+
"sync"
1112
"sync/atomic"
1213
"time"
1314

@@ -109,6 +110,13 @@ type MCPProxyServer struct {
109110
// Hooks shared across all routing mode servers
110111
hooks *mcpserver.Hooks
111112

113+
// directToolPerms maps direct-mode tool names (server__tool) to the
114+
// operation permission required to call them. It is populated with the
115+
// direct-mode registry and used only to filter tools/list for scoped agent
116+
// tokens; execution-time authorization remains authoritative.
117+
directToolPermsMu sync.RWMutex
118+
directToolPerms map[string]string
119+
112120
// Spec 049: in-memory only counter of retrieve_tools calls that opted into
113121
// include_disabled. Never persisted (privacy, consistent with Spec 042).
114122
includeDisabledCalls atomic.Int64
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package server
2+
3+
import (
4+
"context"
5+
6+
"github.com/mark3labs/mcp-go/mcp"
7+
8+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/auth"
9+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
10+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts"
11+
)
12+
13+
// requiredPermissionForDirectTool derives the agent-token permission a direct
14+
// tool requires from its annotations. It reuses the same variant->operation-type
15+
// mapping that call-time authorization uses (see handleDirectToolCall in
16+
// mcp_routing.go) so discovery filtering can never diverge from execution
17+
// enforcement.
18+
func requiredPermissionForDirectTool(annotations *config.ToolAnnotations) string {
19+
return contracts.ToolVariantToOperationType[contracts.DeriveCallWith(annotations)]
20+
}
21+
22+
func (p *MCPProxyServer) setDirectToolPermissions(perms map[string]string) {
23+
p.directToolPermsMu.Lock()
24+
defer p.directToolPermsMu.Unlock()
25+
26+
if len(perms) == 0 {
27+
p.directToolPerms = nil
28+
return
29+
}
30+
31+
copied := make(map[string]string, len(perms))
32+
for name, perm := range perms {
33+
copied[name] = perm
34+
}
35+
p.directToolPerms = copied
36+
}
37+
38+
func (p *MCPProxyServer) lookupDirectToolPermission(directName string) (string, bool) {
39+
p.directToolPermsMu.RLock()
40+
defer p.directToolPermsMu.RUnlock()
41+
42+
perm, ok := p.directToolPerms[directName]
43+
return perm, ok
44+
}
45+
46+
// filterDirectModeToolsForAuth filters tools/list for scoped agent tokens.
47+
//
48+
// Direct mode registers upstream tools globally as server__tool. Without this
49+
// filter, scoped agent tokens prevent execution but still disclose tool names,
50+
// descriptions, and schemas for servers outside their scope. Call-time auth is
51+
// still authoritative; this filter only removes tools that the current token
52+
// could not call from discovery responses.
53+
func (p *MCPProxyServer) filterDirectModeToolsForAuth(ctx context.Context, tools []mcp.Tool) []mcp.Tool {
54+
if len(tools) == 0 {
55+
return tools
56+
}
57+
58+
authCtx := auth.AuthContextFromContext(ctx)
59+
if authCtx == nil || authCtx.Type != auth.AuthTypeAgent {
60+
return tools
61+
}
62+
63+
filtered := make([]mcp.Tool, 0, len(tools))
64+
for _, tool := range tools {
65+
serverName, _, ok := ParseDirectToolName(tool.Name)
66+
if !ok {
67+
filtered = append(filtered, tool)
68+
continue
69+
}
70+
71+
if !authCtx.CanAccessServer(serverName) {
72+
continue
73+
}
74+
75+
requiredPerm, ok := p.lookupDirectToolPermission(tool.Name)
76+
if !ok {
77+
continue
78+
}
79+
80+
if requiredPerm != "" && !authCtx.HasPermission(requiredPerm) {
81+
continue
82+
}
83+
84+
filtered = append(filtered, tool)
85+
}
86+
87+
return filtered
88+
}

internal/server/mcp_routing.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,16 @@ func (p *MCPProxyServer) buildDirectModeTools() []mcpserver.ServerTool {
4848
// Use DiscoverTools which already filters for connected, enabled, non-quarantined servers
4949
tools, err := p.upstreamManager.DiscoverTools(ctx)
5050
if err != nil {
51+
p.setDirectToolPermissions(nil)
5152
p.logger.Error("failed to discover tools for direct mode", zap.Error(err))
5253
return nil
5354
}
5455

5556
serverTools := make([]mcpserver.ServerTool, 0, len(tools))
57+
directToolPerms := make(map[string]string, len(tools))
5658
for _, tool := range tools {
5759
directName := FormatDirectToolName(tool.ServerName, tool.Name)
60+
directToolPerms[directName] = requiredPermissionForDirectTool(tool.Annotations)
5861

5962
// Build MCP tool options
6063
opts := []mcp.ToolOption{
@@ -114,6 +117,8 @@ func (p *MCPProxyServer) buildDirectModeTools() []mcpserver.ServerTool {
114117
})
115118
}
116119

120+
p.setDirectToolPermissions(directToolPerms)
121+
117122
p.logger.Info("built direct mode tools",
118123
zap.Int("tool_count", len(serverTools)))
119124

@@ -500,9 +505,16 @@ func (p *MCPProxyServer) initRoutingModeServers() {
500505
opts = append(opts, mcpserver.WithHooks(p.hooks))
501506
}
502507

503-
// Create direct mode server
508+
// Create direct mode server. Both direct-mode tool filters are agent-scoped
509+
// discovery filters and belong only on the direct server (not the shared
510+
// code-exec / call-tool servers): filterDirectModeToolsForAuth enforces
511+
// agent-token server/permission scope, filterDirectToolsForAgentCallability
512+
// hides tools the agent could not actually invoke.
504513
directOpts := append([]mcpserver.ServerOption{}, opts...)
505-
directOpts = append(directOpts, mcpserver.WithToolFilter(p.filterDirectToolsForAgentCallability))
514+
directOpts = append(directOpts,
515+
mcpserver.WithToolFilter(p.filterDirectModeToolsForAuth),
516+
mcpserver.WithToolFilter(p.filterDirectToolsForAgentCallability),
517+
)
506518
p.directServer = mcpserver.NewMCPServer(
507519
"mcpproxy-go",
508520
mcpServerVersion(),

internal/server/mcp_routing_test.go

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,204 @@ func TestDirectModeHandler_DestructiveToolNeedsDestructivePermission(t *testing.
366366
assert.Contains(t, result.Content[0].(mcp.TextContent).Text, "destructive")
367367
}
368368

369+
func TestRequiredPermissionForDirectTool_MapsAnnotationsToAuthPermissions(t *testing.T) {
370+
readOnly := true
371+
write := false
372+
destructive := true
373+
374+
tests := []struct {
375+
name string
376+
annotations *config.ToolAnnotations
377+
want string
378+
}{
379+
{
380+
name: "nil annotations default to read",
381+
want: auth.PermRead,
382+
},
383+
{
384+
name: "read only hint maps to read",
385+
annotations: &config.ToolAnnotations{
386+
ReadOnlyHint: &readOnly,
387+
},
388+
want: auth.PermRead,
389+
},
390+
{
391+
name: "read only false maps to write",
392+
annotations: &config.ToolAnnotations{
393+
ReadOnlyHint: &write,
394+
},
395+
want: auth.PermWrite,
396+
},
397+
{
398+
name: "destructive hint maps to destructive",
399+
annotations: &config.ToolAnnotations{
400+
DestructiveHint: &destructive,
401+
},
402+
want: auth.PermDestructive,
403+
},
404+
{
405+
name: "destructive hint takes precedence over read only hint",
406+
annotations: &config.ToolAnnotations{
407+
ReadOnlyHint: &readOnly,
408+
DestructiveHint: &destructive,
409+
},
410+
want: auth.PermDestructive,
411+
},
412+
}
413+
414+
for _, tt := range tests {
415+
t.Run(tt.name, func(t *testing.T) {
416+
assert.Equal(t, tt.want, requiredPermissionForDirectTool(tt.annotations))
417+
})
418+
}
419+
}
420+
421+
func TestSetDirectToolPermissions_DefensivelyCopiesMap(t *testing.T) {
422+
proxy := &MCPProxyServer{}
423+
toolName := FormatDirectToolName("github", "get_issue")
424+
perms := map[string]string{
425+
toolName: auth.PermRead,
426+
}
427+
428+
proxy.setDirectToolPermissions(perms)
429+
perms[toolName] = auth.PermDestructive
430+
431+
got, ok := proxy.lookupDirectToolPermission(toolName)
432+
require.True(t, ok)
433+
assert.Equal(t, auth.PermRead, got)
434+
}
435+
436+
func TestFilterDirectModeToolsForAuth_DoesNotMutateInputSlice(t *testing.T) {
437+
proxy := &MCPProxyServer{}
438+
allowed := FormatDirectToolName("github", "get_issue")
439+
denied := FormatDirectToolName("gitlab", "get_issue")
440+
tools := []mcp.Tool{
441+
{Name: allowed},
442+
{Name: denied},
443+
}
444+
original := append([]mcp.Tool(nil), tools...)
445+
446+
proxy.setDirectToolPermissions(map[string]string{
447+
allowed: auth.PermRead,
448+
denied: auth.PermRead,
449+
})
450+
451+
ctx := auth.WithAuthContext(context.Background(), &auth.AuthContext{
452+
Type: auth.AuthTypeAgent,
453+
AgentName: "test-agent",
454+
AllowedServers: []string{"github"},
455+
Permissions: []string{auth.PermRead},
456+
})
457+
458+
filtered := proxy.filterDirectModeToolsForAuth(ctx, tools)
459+
460+
assert.Equal(t, []string{allowed}, directToolNamesForTest(filtered))
461+
assert.Equal(t, original, tools)
462+
}
463+
464+
func TestFilterDirectModeToolsForAuth_AgentServerAndPermissionScope(t *testing.T) {
465+
proxy := &MCPProxyServer{}
466+
467+
githubRead := FormatDirectToolName("github", "get_issue")
468+
githubWrite := FormatDirectToolName("github", "create_issue")
469+
githubDestroy := FormatDirectToolName("github", "delete_repo")
470+
gitlabRead := FormatDirectToolName("gitlab", "get_issue")
471+
472+
proxy.setDirectToolPermissions(map[string]string{
473+
githubRead: auth.PermRead,
474+
githubWrite: auth.PermWrite,
475+
githubDestroy: auth.PermDestructive,
476+
gitlabRead: auth.PermRead,
477+
})
478+
479+
tools := []mcp.Tool{
480+
{Name: githubRead},
481+
{Name: githubWrite},
482+
{Name: githubDestroy},
483+
{Name: gitlabRead},
484+
}
485+
486+
agentCtx := auth.WithAuthContext(context.Background(), &auth.AuthContext{
487+
Type: auth.AuthTypeAgent,
488+
AgentName: "test-agent",
489+
AllowedServers: []string{"github"},
490+
Permissions: []string{auth.PermRead, auth.PermWrite},
491+
})
492+
493+
filtered := proxy.filterDirectModeToolsForAuth(agentCtx, tools)
494+
495+
assert.Equal(t, []string{githubRead, githubWrite}, directToolNamesForTest(filtered))
496+
}
497+
498+
func TestFilterDirectModeToolsForAuth_NonAgentUnchanged(t *testing.T) {
499+
proxy := &MCPProxyServer{}
500+
tools := []mcp.Tool{
501+
{Name: FormatDirectToolName("github", "get_issue")},
502+
{Name: FormatDirectToolName("gitlab", "get_issue")},
503+
}
504+
505+
assert.Equal(t, tools, proxy.filterDirectModeToolsForAuth(context.Background(), tools))
506+
507+
adminCtx := auth.WithAuthContext(context.Background(), auth.AdminContext())
508+
assert.Equal(t, tools, proxy.filterDirectModeToolsForAuth(adminCtx, tools))
509+
}
510+
511+
func TestFilterDirectModeToolsForAuth_FailsClosedOnMissingPermissionMetadata(t *testing.T) {
512+
proxy := &MCPProxyServer{}
513+
514+
visible := FormatDirectToolName("github", "get_issue")
515+
missing := FormatDirectToolName("github", "unknown")
516+
proxy.setDirectToolPermissions(map[string]string{
517+
visible: auth.PermRead,
518+
})
519+
520+
ctx := auth.WithAuthContext(context.Background(), &auth.AuthContext{
521+
Type: auth.AuthTypeAgent,
522+
AgentName: "test-agent",
523+
AllowedServers: []string{"github"},
524+
Permissions: []string{auth.PermRead},
525+
})
526+
527+
filtered := proxy.filterDirectModeToolsForAuth(ctx, []mcp.Tool{
528+
{Name: visible},
529+
{Name: missing},
530+
})
531+
532+
assert.Equal(t, []string{visible}, directToolNamesForTest(filtered))
533+
}
534+
535+
func TestFilterDirectModeToolsForAuth_KeepsNonDirectTools(t *testing.T) {
536+
proxy := &MCPProxyServer{}
537+
538+
direct := FormatDirectToolName("github", "get_issue")
539+
nonDirect := "retrieve_tools"
540+
proxy.setDirectToolPermissions(map[string]string{
541+
direct: auth.PermRead,
542+
})
543+
544+
ctx := auth.WithAuthContext(context.Background(), &auth.AuthContext{
545+
Type: auth.AuthTypeAgent,
546+
AgentName: "test-agent",
547+
AllowedServers: []string{"github"},
548+
Permissions: []string{auth.PermRead},
549+
})
550+
551+
filtered := proxy.filterDirectModeToolsForAuth(ctx, []mcp.Tool{
552+
{Name: direct},
553+
{Name: nonDirect},
554+
})
555+
556+
assert.Equal(t, []string{direct, nonDirect}, directToolNamesForTest(filtered))
557+
}
558+
559+
func directToolNamesForTest(tools []mcp.Tool) []string {
560+
names := make([]string, 0, len(tools))
561+
for _, tool := range tools {
562+
names = append(names, tool.Name)
563+
}
564+
return names
565+
}
566+
369567
func TestDirectModeHandler_NoAuthContext(t *testing.T) {
370568
logger, _ := zap.NewDevelopment()
371569
proxy := &MCPProxyServer{

0 commit comments

Comments
 (0)