Skip to content

Commit 00a795e

Browse files
author
root
committed
fix: gate MCP App UI resources on backing tool availability
Closes #2519 UI resources for write tools were registered unconditionally even when --read-only mode filtered out the backing tools. Register each UI resource only when its companion tool passes inventory filters.
1 parent 457f599 commit 00a795e

3 files changed

Lines changed: 107 additions & 20 deletions

File tree

pkg/github/server.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,14 @@ func NewMCPServer(ctx context.Context, cfg *MCPServerConfig, deps ToolDependenci
103103
inv.RegisterAll(ctx, ghServer, deps)
104104

105105
// Register MCP App UI resources whenever the embedded UI assets are
106-
// available. The resources are static HTML and are only referenced by
107-
// tools when the remote_mcp_ui_apps feature flag is enabled for the
108-
// request (the inventory strips the _meta.ui block otherwise via
109-
// stripMCPAppsMetadata), so registering them unconditionally is safe.
110-
// Registering here — rather than in the stdio bootstrap — ensures the
111-
// remote/HTTP server also serves them, fixing the "-32002 Resource not
112-
// found" error clients hit after the tool returns a ui:// URI.
106+
// available. Each resource is registered only if its backing write/read
107+
// tool is in the inventory (see RegisterUIResources), so read-only mode
108+
// does not expose write-tool UI surfaces. Registering here — rather than
109+
// in the stdio bootstrap — ensures the remote/HTTP server also serves
110+
// them, fixing the "-32002 Resource not found" error clients hit after
111+
// the tool returns a ui:// URI.
113112
if UIAssetsAvailable() {
114-
RegisterUIResources(ghServer)
113+
RegisterUIResources(ctx, ghServer, inv)
115114
}
116115

117116
return ghServer, nil

pkg/github/ui_resources.go

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,46 @@ package github
33
import (
44
"context"
55

6+
"github.com/github/github-mcp-server/pkg/inventory"
67
"github.com/modelcontextprotocol/go-sdk/mcp"
78
)
89

10+
type uiResourceSpec struct {
11+
toolName string
12+
register func(s *mcp.Server)
13+
}
14+
915
// RegisterUIResources registers MCP App UI resources with the server.
1016
// These are static resources (not templates) that serve HTML content for
1117
// MCP App-enabled tools. The HTML is built from React/Primer components
1218
// in the ui/ directory using `script/build-ui`.
1319
//
20+
// UI resources are registered only when their backing tool is present in
21+
// the inventory's available tools (respecting read-only mode and other filters).
22+
//
1423
// Resource metadata follows the stable 2026-01-26 MCP Apps spec:
1524
// https://github.com/modelcontextprotocol/ext-apps/blob/main/specification/2026-01-26/apps.mdx
16-
func RegisterUIResources(s *mcp.Server) {
17-
// Register the get_me UI resource
25+
func RegisterUIResources(ctx context.Context, s *mcp.Server, inv *inventory.Inventory) {
26+
tools := inv.AvailableTools(ctx)
27+
available := make(map[string]struct{}, len(tools))
28+
for _, tool := range tools {
29+
available[tool.Tool.Name] = struct{}{}
30+
}
31+
32+
specs := []uiResourceSpec{
33+
{toolName: "get_me", register: registerGetMeUIResource},
34+
{toolName: "issue_write", register: registerIssueWriteUIResource},
35+
{toolName: "create_pull_request", register: registerPullRequestWriteUIResource},
36+
}
37+
38+
for _, spec := range specs {
39+
if _, ok := available[spec.toolName]; ok {
40+
spec.register(s)
41+
}
42+
}
43+
}
44+
45+
func registerGetMeUIResource(s *mcp.Server) {
1846
s.AddResource(
1947
&mcp.Resource{
2048
URI: GetMeUIResourceURI,
@@ -45,8 +73,9 @@ func RegisterUIResources(s *mcp.Server) {
4573
}, nil
4674
},
4775
)
76+
}
4877

49-
// Register the issue_write UI resource
78+
func registerIssueWriteUIResource(s *mcp.Server) {
5079
s.AddResource(
5180
&mcp.Resource{
5281
URI: IssueWriteUIResourceURI,
@@ -75,8 +104,9 @@ func RegisterUIResources(s *mcp.Server) {
75104
}, nil
76105
},
77106
)
107+
}
78108

79-
// Register the create_pull_request UI resource
109+
func registerPullRequestWriteUIResource(s *mcp.Server) {
80110
s.AddResource(
81111
&mcp.Resource{
82112
URI: PullRequestWriteUIResourceURI,

pkg/github/ui_resources_test.go

Lines changed: 66 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ func TestRegisterUIResources_ReadableViaClient(t *testing.T) {
2525
t.Skip("UI assets not built; run script/build-ui to enable this test")
2626
}
2727

28+
ctx := context.Background()
2829
srv := mcp.NewServer(&mcp.Implementation{Name: "test", Version: "0.0.1"}, nil)
29-
RegisterUIResources(srv)
30+
RegisterUIResources(ctx, srv, mustInventoryWithUIAppTools(t))
3031

3132
// Connect an in-memory client/server pair and read each advertised URI.
3233
st, ct := mcp.NewInMemoryTransports()
@@ -68,6 +69,52 @@ func TestRegisterUIResources_ReadableViaClient(t *testing.T) {
6869
}
6970
}
7071

72+
// TestRegisterUIResources_ReadOnlyExcludesWriteUIResources verifies that write
73+
// tool UI resources are not registered when the server runs in read-only mode,
74+
// while read-only tool UI (get_me) remains available.
75+
func TestRegisterUIResources_ReadOnlyExcludesWriteUIResources(t *testing.T) {
76+
t.Parallel()
77+
78+
if !UIAssetsAvailable() {
79+
t.Skip("UI assets not built; run script/build-ui to enable this test")
80+
}
81+
82+
ctx := context.Background()
83+
srv := mcp.NewServer(&mcp.Implementation{Name: "test", Version: "0.0.1"}, nil)
84+
RegisterUIResources(ctx, srv, mustReadOnlyInventoryWithUIAppToolsets(t))
85+
86+
st, ct := mcp.NewInMemoryTransports()
87+
88+
type clientResult struct {
89+
session *mcp.ClientSession
90+
err error
91+
}
92+
clientCh := make(chan clientResult, 1)
93+
go func() {
94+
client := mcp.NewClient(&mcp.Implementation{Name: "test-client"}, nil)
95+
cs, err := client.Connect(context.Background(), ct, nil)
96+
clientCh <- clientResult{session: cs, err: err}
97+
}()
98+
99+
ss, err := srv.Connect(context.Background(), st, nil)
100+
require.NoError(t, err)
101+
t.Cleanup(func() { _ = ss.Close() })
102+
103+
got := <-clientCh
104+
require.NoError(t, got.err)
105+
t.Cleanup(func() { _ = got.session.Close() })
106+
107+
res, err := got.session.ReadResource(context.Background(), &mcp.ReadResourceParams{URI: GetMeUIResourceURI})
108+
require.NoError(t, err)
109+
require.NotEmpty(t, res.Contents)
110+
111+
_, err = got.session.ReadResource(context.Background(), &mcp.ReadResourceParams{URI: IssueWriteUIResourceURI})
112+
require.Error(t, err, "issue_write UI should not be registered in read-only mode")
113+
114+
_, err = got.session.ReadResource(context.Background(), &mcp.ReadResourceParams{URI: PullRequestWriteUIResourceURI})
115+
require.Error(t, err, "pr_write UI should not be registered in read-only mode")
116+
}
117+
71118
// TestNewMCPServer_RegistersUIResources verifies that NewMCPServer — the
72119
// shared constructor used by both the stdio and HTTP entry points — registers
73120
// the UI resources when UI assets are embedded. Previously this registration
@@ -80,9 +127,10 @@ func TestNewMCPServer_RegistersUIResources(t *testing.T) {
80127
}
81128

82129
srv, err := NewMCPServer(context.Background(), &MCPServerConfig{
83-
Version: "test",
84-
Translator: stubTranslator,
85-
}, stubDeps{t: stubTranslator}, mustEmptyInventory(t))
130+
Version: "test",
131+
Translator: stubTranslator,
132+
EnabledToolsets: []string{"issues"},
133+
}, stubDeps{t: stubTranslator}, mustInventoryWithUIAppTools(t))
86134
require.NoError(t, err)
87135

88136
st, ct := mcp.NewInMemoryTransports()
@@ -113,11 +161,21 @@ func TestNewMCPServer_RegistersUIResources(t *testing.T) {
113161
assert.Equal(t, MCPAppMIMEType, res.Contents[0].MIMEType)
114162
}
115163

116-
// mustEmptyInventory builds an empty inventory for tests that only care about
117-
// resources/prompts registered outside the inventory (such as the UI resources).
118-
func mustEmptyInventory(t *testing.T) *inventory.Inventory {
164+
func mustInventoryWithUIAppTools(t *testing.T) *inventory.Inventory {
165+
t.Helper()
166+
inv, err := NewInventory(stubTranslator).
167+
WithToolsets([]string{"context", "issues", "pull_requests"}).
168+
Build()
169+
require.NoError(t, err)
170+
return inv
171+
}
172+
173+
func mustReadOnlyInventoryWithUIAppToolsets(t *testing.T) *inventory.Inventory {
119174
t.Helper()
120-
inv, err := NewInventory(stubTranslator).WithToolsets([]string{}).Build()
175+
inv, err := NewInventory(stubTranslator).
176+
WithToolsets([]string{"context", "issues", "pull_requests"}).
177+
WithReadOnly(true).
178+
Build()
121179
require.NoError(t, err)
122180
return inv
123181
}

0 commit comments

Comments
 (0)