diff --git a/internal/api/handlers/v0/servers.go b/internal/api/handlers/v0/servers.go index 059095d28..60d0d2b98 100644 --- a/internal/api/handlers/v0/servers.go +++ b/internal/api/handlers/v0/servers.go @@ -58,7 +58,7 @@ type ListServersInput struct { Cursor string `query:"cursor" doc:"Pagination cursor" required:"false" example:"server-cursor-123"` Limit int `query:"limit" doc:"Number of items per page" default:"30" minimum:"1" maximum:"100" example:"50"` UpdatedSince string `query:"updated_since" doc:"Filter servers updated since timestamp (RFC3339 datetime)" required:"false" example:"2025-08-07T13:15:04.280Z"` - Search string `query:"search" doc:"Search servers by name (substring match)" required:"false" example:"filesystem"` + Search string `query:"search" doc:"Search servers by name or description (substring match)" required:"false" example:"filesystem"` Version string `query:"version" doc:"Filter by version ('latest' for latest version, or an exact version like '1.2.3')" required:"false" example:"latest"` IncludeDeleted OptionalBool `query:"include_deleted" doc:"Include deleted servers in results (default: false, but always true when updated_since is provided)" required:"false"` } @@ -105,9 +105,10 @@ func RegisterServersEndpoints(api huma.API, pathPrefix string, registry service. } } - // Handle search parameter + // Handle search parameter - matches against both name and description if input.Search != "" { filter.SubstringName = &input.Search + filter.SubstringDescription = &input.Search } // Handle version parameter diff --git a/internal/api/handlers/v0/servers_test.go b/internal/api/handlers/v0/servers_test.go index 16c763c4e..b611b7203 100644 --- a/internal/api/handlers/v0/servers_test.go +++ b/internal/api/handlers/v0/servers_test.go @@ -36,7 +36,7 @@ func TestListServersEndpoint(t *testing.T) { _, err = registryService.CreateServer(ctx, &apiv0.ServerJSON{ Schema: model.CurrentSchemaURL, Name: "com.example/server-beta", - Description: "Beta test server", + Description: "Beta test server for weather forecasting", Version: "2.0.0", }) require.NoError(t, err) @@ -71,6 +71,18 @@ func TestListServersEndpoint(t *testing.T) { expectedStatus: http.StatusOK, expectedCount: 1, }, + { + name: "search matches description when name does not match", + queryParams: "?search=weather", + expectedStatus: http.StatusOK, + expectedCount: 1, + }, + { + name: "search term absent from both name and description matches nothing", + queryParams: "?search=nonexistent-term-xyz", + expectedStatus: http.StatusOK, + expectedCount: 0, + }, { name: "filter latest only", queryParams: "?version=latest", diff --git a/internal/database/database.go b/internal/database/database.go index b4a797fae..a45a9537b 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -22,13 +22,14 @@ var ( // ServerFilter defines filtering options for server queries type ServerFilter struct { - Name *string // for finding versions of same server - RemoteURL *string // for duplicate URL detection - UpdatedSince *time.Time // for incremental sync filtering - SubstringName *string // for substring search on name - Version *string // for exact version matching - IsLatest *bool // for filtering latest versions only - IncludeDeleted *bool // for including deleted packages in results (default: exclude) + Name *string // for finding versions of same server + RemoteURL *string // for duplicate URL detection + UpdatedSince *time.Time // for incremental sync filtering + SubstringName *string // for substring search on name + SubstringDescription *string // for substring search on description + Version *string // for exact version matching + IsLatest *bool // for filtering latest versions only + IncludeDeleted *bool // for including deleted packages in results (default: exclude) } // Database defines the interface for database operations diff --git a/internal/database/postgres.go b/internal/database/postgres.go index b0b2d983b..efd04d7a3 100644 --- a/internal/database/postgres.go +++ b/internal/database/postgres.go @@ -117,14 +117,26 @@ func buildFilterConditions(filter *ServerFilter, argIndex int) ([]string, []any, args = append(args, *filter.UpdatedSince) argIndex++ } - if filter.SubstringName != nil { - // Escape LIKE metacharacters so that user input cannot expand into - // wildcard matches (e.g. `?search=_` matching every single-char name, - // `?search=%` matching everything). Order matters: backslashes must be - // escaped first so subsequent escape backslashes are not double-escaped. - escaped := strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`).Replace(*filter.SubstringName) + switch { + case filter.SubstringName != nil && filter.SubstringDescription != nil: + // The ?search= query param sets both fields to the same term so that + // a server matches on name OR description. These must be combined into + // a single OR condition here rather than left as two independent + // per-field checks below (which the outer AND-join would turn into + // "name matches AND description matches"). + conditions = append(conditions, fmt.Sprintf( + "(server_name ILIKE $%d ESCAPE '\\' OR value ->> 'description' ILIKE $%d ESCAPE '\\')", + argIndex, argIndex+1, + )) + args = append(args, likePattern(*filter.SubstringName), likePattern(*filter.SubstringDescription)) + argIndex += 2 + case filter.SubstringName != nil: conditions = append(conditions, fmt.Sprintf("server_name ILIKE $%d ESCAPE '\\'", argIndex)) - args = append(args, "%"+escaped+"%") + args = append(args, likePattern(*filter.SubstringName)) + argIndex++ + case filter.SubstringDescription != nil: + conditions = append(conditions, fmt.Sprintf("value ->> 'description' ILIKE $%d ESCAPE '\\'", argIndex)) + args = append(args, likePattern(*filter.SubstringDescription)) argIndex++ } if filter.Version != nil { @@ -144,6 +156,17 @@ func buildFilterConditions(filter *ServerFilter, argIndex int) ([]string, []any, return conditions, args, argIndex } +// likePattern escapes LIKE/ILIKE metacharacters in s and wraps it for a +// substring match, so that user input cannot expand into unintended wildcard +// matches (e.g. `?search=_` matching every single-char name, `?search=%` +// matching everything). Order matters: backslashes must be escaped first so +// subsequent escape backslashes are not double-escaped. Callers must pair +// this with `ESCAPE '\\'` in the ILIKE clause. +func likePattern(s string) string { + escaped := strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`).Replace(s) + return "%" + escaped + "%" +} + // addCursorCondition adds pagination cursor condition to WHERE clause. // // The compound cursor uses a row-constructor comparison so PostgreSQL can seek diff --git a/internal/database/postgres_internal_test.go b/internal/database/postgres_internal_test.go new file mode 100644 index 000000000..3f0879ac8 --- /dev/null +++ b/internal/database/postgres_internal_test.go @@ -0,0 +1,104 @@ +package database + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func ptr[T any](v T) *T { return &v } + +// TestBuildFilterConditions_SearchMatchesNameOrDescription covers the +// SubstringName/SubstringDescription combination directly against the SQL +// fragments buildFilterConditions produces. This is white-box on purpose: +// ListServers-level tests in postgres_test.go require a live PostgreSQL +// instance and can only assert on results, not on the generated WHERE +// clause. The behavior under test here is specifically that the ?search= +// query param (which the handler maps to both SubstringName and +// SubstringDescription set to the same term) must be OR'd together - every +// other pair of filter fields in ServerFilter is combined with AND, so this +// is the one case buildFilterConditions has to special-case to avoid +// silently requiring a match on both name and description at once. +func TestBuildFilterConditions_SearchMatchesNameOrDescription(t *testing.T) { + tests := []struct { + name string + filter *ServerFilter + wantConditions []string + wantArgs []any + }{ + { + name: "nil filter produces no conditions", + filter: nil, + wantConditions: nil, + wantArgs: nil, + }, + { + name: "SubstringName and SubstringDescription both set (the ?search= case) are OR'd", + filter: &ServerFilter{ + SubstringName: ptr("weather"), + SubstringDescription: ptr("weather"), + }, + wantConditions: []string{ + "(server_name ILIKE $1 ESCAPE '\\' OR value ->> 'description' ILIKE $2 ESCAPE '\\')", + "status != 'deleted'", + }, + wantArgs: []any{"%weather%", "%weather%"}, + }, + { + name: "SubstringName alone still matches only the name column", + filter: &ServerFilter{ + SubstringName: ptr("weather"), + }, + wantConditions: []string{ + "server_name ILIKE $1 ESCAPE '\\'", + "status != 'deleted'", + }, + wantArgs: []any{"%weather%"}, + }, + { + name: "SubstringDescription alone matches only the JSONB description field", + filter: &ServerFilter{ + SubstringDescription: ptr("weather"), + }, + wantConditions: []string{ + "value ->> 'description' ILIKE $1 ESCAPE '\\'", + "status != 'deleted'", + }, + wantArgs: []any{"%weather%"}, + }, + { + name: "LIKE metacharacters in the search term are escaped on both sides of the OR", + filter: &ServerFilter{ + SubstringName: ptr("100%_free"), + SubstringDescription: ptr("100%_free"), + }, + wantConditions: []string{ + "(server_name ILIKE $1 ESCAPE '\\' OR value ->> 'description' ILIKE $2 ESCAPE '\\')", + "status != 'deleted'", + }, + wantArgs: []any{`%100\%\_free%`, `%100\%\_free%`}, + }, + { + name: "combined search alongside other filters keeps AND semantics for the rest", + filter: &ServerFilter{ + SubstringName: ptr("weather"), + SubstringDescription: ptr("weather"), + IsLatest: ptr(true), + }, + wantConditions: []string{ + "(server_name ILIKE $1 ESCAPE '\\' OR value ->> 'description' ILIKE $2 ESCAPE '\\')", + "is_latest = $3", + "status != 'deleted'", + }, + wantArgs: []any{"%weather%", "%weather%", true}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotConditions, gotArgs, _ := buildFilterConditions(tt.filter, 1) + assert.Equal(t, tt.wantConditions, gotConditions) + assert.Equal(t, tt.wantArgs, gotArgs) + }) + } +} diff --git a/internal/database/postgres_test.go b/internal/database/postgres_test.go index 6784ae52d..e9904d9dd 100644 --- a/internal/database/postgres_test.go +++ b/internal/database/postgres_test.go @@ -246,6 +246,7 @@ func TestPostgreSQL_ListServers(t *testing.T) { remoteURL string isLatest bool publishedAt time.Time + description string }{ { name: "com.example/server-a", @@ -254,6 +255,7 @@ func TestPostgreSQL_ListServers(t *testing.T) { remoteURL: "https://api-a.example.com/mcp", isLatest: true, publishedAt: time.Now().Add(-2 * time.Hour), + description: "Provides real-time weather forecasts", }, { name: "com.example/server-b", @@ -262,6 +264,7 @@ func TestPostgreSQL_ListServers(t *testing.T) { remoteURL: "https://api-b.example.com/mcp", isLatest: true, publishedAt: time.Now().Add(-1 * time.Hour), + description: "Translates text between languages", }, { name: "com.example/server-c", @@ -270,6 +273,7 @@ func TestPostgreSQL_ListServers(t *testing.T) { remoteURL: "https://api-c.example.com/mcp", isLatest: true, publishedAt: time.Now().Add(-30 * time.Minute), + description: "A deprecated utility server", }, } @@ -277,7 +281,7 @@ func TestPostgreSQL_ListServers(t *testing.T) { for _, server := range testServers { serverJSON := &apiv0.ServerJSON{ Name: server.name, - Description: "Test server for listing", + Description: server.description, Version: server.version, Remotes: []model.Transport{ {Type: "http", URL: server.remoteURL}, @@ -337,6 +341,44 @@ func TestPostgreSQL_ListServers(t *testing.T) { limit: 10, expectedCount: 3, }, + { + name: "filter by substring description", + filter: &database.ServerFilter{ + SubstringDescription: stringPtr("weather"), + }, + limit: 10, + expectedCount: 1, + expectedNames: []string{"com.example/server-a"}, + }, + { + name: "substring description filter is case-insensitive substring, not exact match", + filter: &database.ServerFilter{ + SubstringDescription: stringPtr("LANGUAGES"), + }, + limit: 10, + expectedCount: 1, + expectedNames: []string{"com.example/server-b"}, + }, + { + name: "combined name+description search (the ?search= case) matches on description alone", + filter: &database.ServerFilter{ + SubstringName: stringPtr("weather"), // does not match any server_name + SubstringDescription: stringPtr("weather"), // matches server-a's description + }, + limit: 10, + expectedCount: 1, + expectedNames: []string{"com.example/server-a"}, + }, + { + name: "combined name+description search matches on name alone", + filter: &database.ServerFilter{ + SubstringName: stringPtr("server-b"), // matches server-b's name + SubstringDescription: stringPtr("no-such-phrase"), // matches nothing + }, + limit: 10, + expectedCount: 1, + expectedNames: []string{"com.example/server-b"}, + }, { name: "filter by version", filter: &database.ServerFilter{