From e3799829877f389d387eda966c54bba67717bb3b Mon Sep 17 00:00:00 2001 From: Jonah Kowall Date: Sat, 14 Mar 2026 12:43:13 -0400 Subject: [PATCH 1/2] jaegermcp: enforce MaxSpanDetailsPerRequest in get_span_details Signed-off-by: Jonah Kowall --- .../internal/extension/jaegermcp/config.go | 6 +++ .../extension/jaegermcp/config_test.go | 9 ++++ .../internal/handlers/get_span_details.go | 16 ++++-- .../handlers/get_span_details_test.go | 49 ++++++++++++++----- .../internal/extension/jaegermcp/server.go | 2 +- 5 files changed, 65 insertions(+), 17 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegermcp/config.go b/cmd/jaeger/internal/extension/jaegermcp/config.go index 5a1efdc304e..14c860efb69 100644 --- a/cmd/jaeger/internal/extension/jaegermcp/config.go +++ b/cmd/jaeger/internal/extension/jaegermcp/config.go @@ -4,6 +4,8 @@ package jaegermcp import ( + "fmt" + "github.com/asaskevich/govalidator" "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/confmap/xconfmap" @@ -32,6 +34,10 @@ func (cfg *Config) Validate() error { // if cfg.ServerVersion == "" { // cfg.ServerVersion = version.Get().GitVersion // } + if cfg.MaxSpanDetailsPerRequest < 1 { + return fmt.Errorf("max_span_details_per_request must be at least 1") + } + _, err := govalidator.ValidateStruct(cfg) return err } diff --git a/cmd/jaeger/internal/extension/jaegermcp/config_test.go b/cmd/jaeger/internal/extension/jaegermcp/config_test.go index 64a56cf7e4e..875f15ad4a4 100644 --- a/cmd/jaeger/internal/extension/jaegermcp/config_test.go +++ b/cmd/jaeger/internal/extension/jaegermcp/config_test.go @@ -24,6 +24,15 @@ func TestValidate(t *testing.T) { }, expectError: false, }, + { + name: "Invalid MaxSpanDetailsPerRequest (too low)", + config: &Config{ + ServerVersion: "1.0.0", + MaxSpanDetailsPerRequest: 0, + MaxSearchResults: 100, + }, + expectError: true, + }, { name: "Invalid MaxSpanDetailsPerRequest (too high)", config: &Config{ diff --git a/cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_span_details.go b/cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_span_details.go index 8694e97904c..58da081a279 100644 --- a/cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_span_details.go +++ b/cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_span_details.go @@ -30,15 +30,18 @@ type queryServiceGetTracesInterface interface { // This tool fetches full OTLP details (attributes, events, links, status) for specific spans // within a trace, allowing deep inspection of individual span data for debugging and analysis. type getSpanDetailsHandler struct { - queryService queryServiceGetTracesInterface + queryService queryServiceGetTracesInterface + maxSpanDetailsPerRequest int } // NewGetSpanDetailsHandler creates a new get_span_details handler and returns the handler function. func NewGetSpanDetailsHandler( queryService *querysvc.QueryService, + maxSpanDetailsPerRequest int, ) mcp.ToolHandlerFor[types.GetSpanDetailsInput, types.GetSpanDetailsOutput] { h := &getSpanDetailsHandler{ - queryService: queryService, + queryService: queryService, + maxSpanDetailsPerRequest: maxSpanDetailsPerRequest, } return h.handle } @@ -115,7 +118,7 @@ func (h *getSpanDetailsHandler) handle( } // buildQuery converts GetSpanDetailsInput to querysvc.GetTraceParams. -func (*getSpanDetailsHandler) buildQuery(input types.GetSpanDetailsInput) (querysvc.GetTraceParams, error) { +func (h *getSpanDetailsHandler) buildQuery(input types.GetSpanDetailsInput) (querysvc.GetTraceParams, error) { // Validate input if input.TraceID == "" { return querysvc.GetTraceParams{}, errors.New("trace_id is required") @@ -125,6 +128,13 @@ func (*getSpanDetailsHandler) buildQuery(input types.GetSpanDetailsInput) (query return querysvc.GetTraceParams{}, errors.New("span_ids is required and must not be empty") } + if len(input.SpanIDs) > h.maxSpanDetailsPerRequest { + return querysvc.GetTraceParams{}, fmt.Errorf( + "span_ids must not exceed %d items", + h.maxSpanDetailsPerRequest, + ) + } + traceID, err := parseTraceID(input.TraceID) if err != nil { return querysvc.GetTraceParams{}, fmt.Errorf("invalid trace_id: %w", err) diff --git a/cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_span_details_test.go b/cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_span_details_test.go index dc0b887f0fa..f0ed2dfa581 100644 --- a/cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_span_details_test.go +++ b/cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_span_details_test.go @@ -21,6 +21,15 @@ import ( // Helper types and functions are defined in test_helpers.go +const testMaxSpanDetailsPerRequest = 20 + +func newTestGetSpanDetailsHandler(mock queryServiceGetTracesInterface) *getSpanDetailsHandler { + return &getSpanDetailsHandler{ + queryService: mock, + maxSpanDetailsPerRequest: testMaxSpanDetailsPerRequest, + } +} + func TestGetSpanDetailsHandler_Handle_Success(t *testing.T) { traceID := testTraceID spanID1 := "span001" @@ -59,7 +68,7 @@ func TestGetSpanDetailsHandler_Handle_Success(t *testing.T) { mock := newMockYieldingTraces(testTrace) - handler := &getSpanDetailsHandler{queryService: mock} + handler := newTestGetSpanDetailsHandler(mock) input := types.GetSpanDetailsInput{ TraceID: traceID, @@ -115,7 +124,7 @@ func TestGetSpanDetailsHandler_Handle_SingleSpan(t *testing.T) { mock := newMockYieldingTraces(testTrace) - handler := &getSpanDetailsHandler{queryService: mock} + handler := newTestGetSpanDetailsHandler(mock) input := types.GetSpanDetailsInput{ TraceID: traceID, @@ -143,7 +152,7 @@ func TestGetSpanDetailsHandler_Handle_FiltersBySpanIDs(t *testing.T) { mock := newMockYieldingTraces(testTrace) - handler := &getSpanDetailsHandler{queryService: mock} + handler := newTestGetSpanDetailsHandler(mock) input := types.GetSpanDetailsInput{ TraceID: traceID, @@ -166,7 +175,7 @@ func TestGetSpanDetailsHandler_Handle_FiltersBySpanIDs(t *testing.T) { } func TestGetSpanDetailsHandler_Handle_MissingTraceID(t *testing.T) { - handler := NewGetSpanDetailsHandler(nil) + handler := NewGetSpanDetailsHandler(nil, testMaxSpanDetailsPerRequest) input := types.GetSpanDetailsInput{ SpanIDs: []string{spanIDToHex("span001")}, @@ -179,7 +188,7 @@ func TestGetSpanDetailsHandler_Handle_MissingTraceID(t *testing.T) { } func TestGetSpanDetailsHandler_Handle_MissingSpanIDs(t *testing.T) { - handler := NewGetSpanDetailsHandler(nil) + handler := NewGetSpanDetailsHandler(nil, testMaxSpanDetailsPerRequest) input := types.GetSpanDetailsInput{ TraceID: testTraceID, @@ -192,8 +201,22 @@ func TestGetSpanDetailsHandler_Handle_MissingSpanIDs(t *testing.T) { assert.Contains(t, err.Error(), "span_ids is required") } +func TestGetSpanDetailsHandler_Handle_TooManySpanIDs(t *testing.T) { + handler := NewGetSpanDetailsHandler(nil, 2) + + input := types.GetSpanDetailsInput{ + TraceID: testTraceID, + SpanIDs: []string{spanIDToHex("span001"), spanIDToHex("span002"), spanIDToHex("span003")}, + } + + _, _, err := handler(context.Background(), &mcp.CallToolRequest{}, input) + + require.Error(t, err) + assert.Contains(t, err.Error(), "span_ids must not exceed 2 items") +} + func TestGetSpanDetailsHandler_Handle_InvalidTraceID(t *testing.T) { - handler := NewGetSpanDetailsHandler(nil) + handler := NewGetSpanDetailsHandler(nil, testMaxSpanDetailsPerRequest) input := types.GetSpanDetailsInput{ TraceID: "invalid-trace-id", @@ -209,7 +232,7 @@ func TestGetSpanDetailsHandler_Handle_InvalidTraceID(t *testing.T) { func TestGetSpanDetailsHandler_Handle_TraceNotFound(t *testing.T) { mock := newMockYieldingEmpty() - handler := &getSpanDetailsHandler{queryService: mock} + handler := newTestGetSpanDetailsHandler(mock) input := types.GetSpanDetailsInput{ TraceID: testTraceID, @@ -225,7 +248,7 @@ func TestGetSpanDetailsHandler_Handle_TraceNotFound(t *testing.T) { func TestGetSpanDetailsHandler_Handle_QueryError(t *testing.T) { mock := newMockYieldingError(errors.New("database connection failed")) - handler := &getSpanDetailsHandler{queryService: mock} + handler := newTestGetSpanDetailsHandler(mock) input := types.GetSpanDetailsInput{ TraceID: testTraceID, @@ -262,7 +285,7 @@ func TestGetSpanDetailsHandler_Handle_PartialResults(t *testing.T) { }, } - handler := &getSpanDetailsHandler{queryService: mock} + handler := newTestGetSpanDetailsHandler(mock) input := types.GetSpanDetailsInput{ TraceID: traceID, @@ -300,7 +323,7 @@ func TestGetSpanDetailsHandler_Handle_MultipleIterations(t *testing.T) { }, } - handler := &getSpanDetailsHandler{queryService: mock} + handler := newTestGetSpanDetailsHandler(mock) input := types.GetSpanDetailsInput{ TraceID: traceID, @@ -335,7 +358,7 @@ func TestGetSpanDetailsHandler_Handle_WithParentSpanID(t *testing.T) { mock := newMockYieldingTraces(testTrace) - handler := &getSpanDetailsHandler{queryService: mock} + handler := newTestGetSpanDetailsHandler(mock) input := types.GetSpanDetailsInput{ TraceID: traceID, @@ -360,7 +383,7 @@ func TestGetSpanDetailsHandler_Handle_NoMatchingSpans(t *testing.T) { mock := newMockYieldingTraces(testTrace) - handler := &getSpanDetailsHandler{queryService: mock} + handler := newTestGetSpanDetailsHandler(mock) input := types.GetSpanDetailsInput{ TraceID: traceID, @@ -389,7 +412,7 @@ func TestGetSpanDetailsHandler_Handle_PartialMissingSpans(t *testing.T) { mock := newMockYieldingTraces(testTrace) - handler := &getSpanDetailsHandler{queryService: mock} + handler := newTestGetSpanDetailsHandler(mock) input := types.GetSpanDetailsInput{ TraceID: traceID, diff --git a/cmd/jaeger/internal/extension/jaegermcp/server.go b/cmd/jaeger/internal/extension/jaegermcp/server.go index 52e6e33906b..49b4aa5700c 100644 --- a/cmd/jaeger/internal/extension/jaegermcp/server.go +++ b/cmd/jaeger/internal/extension/jaegermcp/server.go @@ -163,7 +163,7 @@ func (s *server) registerTools() { }, searchTracesHandler) // Get span details tool - getSpanDetailsHandler := handlers.NewGetSpanDetailsHandler(s.queryAPI) + getSpanDetailsHandler := handlers.NewGetSpanDetailsHandler(s.queryAPI, s.config.MaxSpanDetailsPerRequest) mcp.AddTool(s.mcpServer, &mcp.Tool{ Name: "get_span_details", Description: "Fetch full details (attributes, events, links, status) for specific spans.", From c47b39255c28433875a2f7448e38534d2c795d69 Mon Sep 17 00:00:00 2001 From: Jonah Kowall Date: Sat, 14 Mar 2026 12:48:24 -0400 Subject: [PATCH 2/2] jaegermcp: keep limit validation in one place Signed-off-by: Jonah Kowall --- cmd/jaeger/internal/extension/jaegermcp/config.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/extension/jaegermcp/config.go b/cmd/jaeger/internal/extension/jaegermcp/config.go index 14c860efb69..ec800f4e4cb 100644 --- a/cmd/jaeger/internal/extension/jaegermcp/config.go +++ b/cmd/jaeger/internal/extension/jaegermcp/config.go @@ -23,7 +23,7 @@ type Config struct { ServerVersion string `mapstructure:"server_version" valid:"required"` // MaxSpanDetailsPerRequest limits the number of spans that can be fetched in a single request. - MaxSpanDetailsPerRequest int `mapstructure:"max_span_details_per_request" valid:"range(1|100)"` + MaxSpanDetailsPerRequest int `mapstructure:"max_span_details_per_request"` // MaxSearchResults limits the number of trace search results. MaxSearchResults int `mapstructure:"max_search_results" valid:"range(1|1000)"` @@ -37,6 +37,9 @@ func (cfg *Config) Validate() error { if cfg.MaxSpanDetailsPerRequest < 1 { return fmt.Errorf("max_span_details_per_request must be at least 1") } + if cfg.MaxSpanDetailsPerRequest > 100 { + return fmt.Errorf("max_span_details_per_request must not exceed 100") + } _, err := govalidator.ValidateStruct(cfg) return err