From 7667fb3843636e6384c97f004a9ec307ea7a1ac9 Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Fri, 16 Oct 2026 16:16:19 -0400 Subject: [PATCH 1/2] fix(mcpcompat/server): recover AddResource panic in syncSessionResources go-sdk's AddResource panics when the resource URI fails url.Parse. Since syncSessionResources can run on the live session goroutine, a bad URI in a per-session overlay would crash the session. Register each resource via addSessionResource, which recovers, logs and skips the offending resource, mirroring addSessionTool and addSessionResourceTemplate. Also correct the sibling comments that claimed AddResource cannot panic. Co-Authored-By: Claude Sonnet 5.5 --- mcpcompat/server/server_internal_test.go | 32 ++++++++++++++++++ mcpcompat/server/session.go | 42 ++++++++++++++++++++---- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/mcpcompat/server/server_internal_test.go b/mcpcompat/server/server_internal_test.go index 778cd8b..ffeebb7 100644 --- a/mcpcompat/server/server_internal_test.go +++ b/mcpcompat/server/server_internal_test.go @@ -164,6 +164,38 @@ func TestSetSessionTools_NonObjectSchemaDoesNotPanic(t *testing.T) { assert.Contains(t, registered, "good-schema", "the well-formed sibling tool must still register") } +// TestSyncSessionResources_UnparseableURIDoesNotPanic verifies that an overlay +// resource whose URI fails url.Parse does not crash the session when synced onto +// the go-sdk server. go-sdk's AddResource panics on such a URI, so the panic must +// be recovered by MCPServer.addSessionResource and the offending resource skipped +// while other resources in the same overlay are still processed. +func TestSyncSessionResources_UnparseableURIDoesNotPanic(t *testing.T) { + t.Parallel() + s := NewMCPServer("s", "1") + srv, err := s.buildServer(nil, false) + require.NoError(t, err) + + const badName = "broken" + handler := func(context.Context, mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { return nil, nil } + cs := s.sessionFor("sid-bad-resource") + cs.SetSessionResources(map[string]ServerResource{ + badName: { + Resource: mcp.Resource{URI: "%zz", Name: badName}, + Handler: handler, + }, + "good": { + Resource: mcp.Resource{URI: "file:///good", Name: "good"}, + Handler: handler, + }, + }) + + assert.NotPanics(t, func() { s.syncSessionResources(srv, cs) }) + + // The helper reports false for the bad URI and true for a well-formed one. + assert.False(t, s.addSessionResource(srv, &gosdk.Resource{URI: "%zz", Name: badName}, ServerResource{Handler: handler})) + assert.True(t, s.addSessionResource(srv, &gosdk.Resource{URI: "file:///ok", Name: "ok"}, ServerResource{Handler: handler})) +} + func TestHooks_Fire(t *testing.T) { t.Parallel() h := &Hooks{} diff --git a/mcpcompat/server/session.go b/mcpcompat/server/session.go index 1b912df..e2c3e36 100644 --- a/mcpcompat/server/session.go +++ b/mcpcompat/server/session.go @@ -472,23 +472,53 @@ func (s *MCPServer) addSessionTool(srv *gosdk.Server, gt *gosdk.Tool, h ToolHand // syncSessionResources adds the session's resource overlay onto its go-sdk // server. Resources are add-only here (ToolHive sets them once at registration). +// +// go-sdk's AddResource calls url.Parse on the resource URI and PANICS if it +// fails. This path can run on the live session goroutine, so each resource is +// registered via addSessionResource, which recovers such a panic, logs it, and +// skips the offending resource so one bad URI does not take down the session. func (s *MCPServer) syncSessionResources(srv *gosdk.Server, cs *clientSession) { cs.mu.RLock() defer cs.mu.RUnlock() - for _, sr := range cs.resources { + for uri, sr := range cs.resources { gr := &gosdk.Resource{} if err := jsonConvert(sr.Resource, gr); err != nil { continue } - srv.AddResource(gr, s.wrapServerResource(sr)) + if !s.addSessionResource(srv, gr, sr) { + if s.logger != nil { + s.logger.Warn("skipping per-session resource: AddResource rejected its URI", "uri", uri) + } + } } } +// addSessionResource registers a resource on a per-session go-sdk server, +// recovering the panic go-sdk's AddResource raises when the resource URI does +// not parse. It returns false (and logs) when the resource could not be +// registered, so the caller skips it rather than crashing the session. Mirrors +// addSessionResourceTemplate. +func (s *MCPServer) addSessionResource( + srv *gosdk.Server, gr *gosdk.Resource, sr ServerResource, +) (ok bool) { + defer func() { + if r := recover(); r != nil { + if s.logger != nil { + s.logger.Error("per-session AddResource panicked; skipping resource", + "uri", gr.URI, "panic", r) + } + ok = false + } + }() + srv.AddResource(gr, s.wrapServerResource(sr)) + return true +} + // syncSessionResourceTemplates adds the session's resource-template overlay onto // its go-sdk server. Templates are add-only here (ToolHive sets them once at -// registration), mirroring syncSessionResources. Unlike AddResource, go-sdk's -// AddResourceTemplate parses the URI template and PANICS if it is invalid or not -// absolute; this path can run on the live session goroutine, so each template is +// registration), mirroring syncSessionResources. Like AddResource, go-sdk's +// AddResourceTemplate PANICS on a bad URI (here, an invalid or non-absolute URI +// template); this path can run on the live session goroutine, so each template is // registered via addSessionResourceTemplate, which recovers such a panic, logs // it, and skips the offending template so one bad template does not take down // the session. @@ -540,7 +570,7 @@ func (s *MCPServer) addSessionResourceTemplate( // without any shim-side dispatch. Prompts are add-only here (ToolHive sets them // once at registration). Unlike AddTool, go-sdk's AddPrompt performs no // schema/URI validation and cannot panic, so no recover wrapper is needed -// (mirroring syncSessionResources rather than the tool path). +// (unlike the tool, resource and resource-template paths). func (s *MCPServer) syncSessionPrompts(srv *gosdk.Server, cs *clientSession) { cs.mu.RLock() defer cs.mu.RUnlock() From 675d6b45c30152322eb163597e7a8cf8c20e484a Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Fri, 16 Oct 2026 16:46:29 -0400 Subject: [PATCH 2/2] refactor(mcpcompat/server): pre-validate resource URI instead of recovering go-sdk's AddResource only panics when url.Parse fails, so run the same check first and skip the resource, avoiding the recover/defer helper. Co-Authored-By: Claude Sonnet 5.5 --- mcpcompat/server/server_internal_test.go | 10 +++---- mcpcompat/server/session.go | 34 ++++++------------------ 2 files changed, 11 insertions(+), 33 deletions(-) diff --git a/mcpcompat/server/server_internal_test.go b/mcpcompat/server/server_internal_test.go index ffeebb7..0cb2689 100644 --- a/mcpcompat/server/server_internal_test.go +++ b/mcpcompat/server/server_internal_test.go @@ -166,9 +166,9 @@ func TestSetSessionTools_NonObjectSchemaDoesNotPanic(t *testing.T) { // TestSyncSessionResources_UnparseableURIDoesNotPanic verifies that an overlay // resource whose URI fails url.Parse does not crash the session when synced onto -// the go-sdk server. go-sdk's AddResource panics on such a URI, so the panic must -// be recovered by MCPServer.addSessionResource and the offending resource skipped -// while other resources in the same overlay are still processed. +// the go-sdk server. go-sdk's AddResource panics on such a URI, so the +// panic must be avoided by the url.Parse pre-check in syncSessionResources and the offending +// resource skipped while other resources in the same overlay are still processed. func TestSyncSessionResources_UnparseableURIDoesNotPanic(t *testing.T) { t.Parallel() s := NewMCPServer("s", "1") @@ -190,10 +190,6 @@ func TestSyncSessionResources_UnparseableURIDoesNotPanic(t *testing.T) { }) assert.NotPanics(t, func() { s.syncSessionResources(srv, cs) }) - - // The helper reports false for the bad URI and true for a well-formed one. - assert.False(t, s.addSessionResource(srv, &gosdk.Resource{URI: "%zz", Name: badName}, ServerResource{Handler: handler})) - assert.True(t, s.addSessionResource(srv, &gosdk.Resource{URI: "file:///ok", Name: "ok"}, ServerResource{Handler: handler})) } func TestHooks_Fire(t *testing.T) { diff --git a/mcpcompat/server/session.go b/mcpcompat/server/session.go index e2c3e36..abade8a 100644 --- a/mcpcompat/server/session.go +++ b/mcpcompat/server/session.go @@ -5,6 +5,7 @@ package server import ( "context" + "net/url" "sync" "sync/atomic" @@ -474,9 +475,9 @@ func (s *MCPServer) addSessionTool(srv *gosdk.Server, gt *gosdk.Tool, h ToolHand // server. Resources are add-only here (ToolHive sets them once at registration). // // go-sdk's AddResource calls url.Parse on the resource URI and PANICS if it -// fails. This path can run on the live session goroutine, so each resource is -// registered via addSessionResource, which recovers such a panic, logs it, and -// skips the offending resource so one bad URI does not take down the session. +// fails. This path can run on the live session goroutine, so the same check is +// run first and a resource with an unparseable URI is logged and skipped, so one +// bad URI does not take down the session. func (s *MCPServer) syncSessionResources(srv *gosdk.Server, cs *clientSession) { cs.mu.RLock() defer cs.mu.RUnlock() @@ -485,35 +486,16 @@ func (s *MCPServer) syncSessionResources(srv *gosdk.Server, cs *clientSession) { if err := jsonConvert(sr.Resource, gr); err != nil { continue } - if !s.addSessionResource(srv, gr, sr) { + if _, err := url.Parse(gr.URI); err != nil { if s.logger != nil { - s.logger.Warn("skipping per-session resource: AddResource rejected its URI", "uri", uri) + s.logger.Warn("skipping per-session resource: unparseable URI", "uri", uri, "error", err) } + continue } + srv.AddResource(gr, s.wrapServerResource(sr)) } } -// addSessionResource registers a resource on a per-session go-sdk server, -// recovering the panic go-sdk's AddResource raises when the resource URI does -// not parse. It returns false (and logs) when the resource could not be -// registered, so the caller skips it rather than crashing the session. Mirrors -// addSessionResourceTemplate. -func (s *MCPServer) addSessionResource( - srv *gosdk.Server, gr *gosdk.Resource, sr ServerResource, -) (ok bool) { - defer func() { - if r := recover(); r != nil { - if s.logger != nil { - s.logger.Error("per-session AddResource panicked; skipping resource", - "uri", gr.URI, "panic", r) - } - ok = false - } - }() - srv.AddResource(gr, s.wrapServerResource(sr)) - return true -} - // syncSessionResourceTemplates adds the session's resource-template overlay onto // its go-sdk server. Templates are add-only here (ToolHive sets them once at // registration), mirroring syncSessionResources. Like AddResource, go-sdk's