diff --git a/docs/server-json/examples.md b/docs/server-json/examples.md index 453a371..897906d 100644 --- a/docs/server-json/examples.md +++ b/docs/server-json/examples.md @@ -186,7 +186,7 @@ This will essentially instruct the MCP client to execute `dnx Knapcode.SampleMcp ```json { "server": { - "name": "com.example/mcp-fs", + "name": "io.modelcontextprotocol.anonymous/mcp-fs", "description": "Cloud-hosted MCP filesystem server", "repository": { "url": "https://github.com/example/remote-fs", @@ -199,7 +199,7 @@ This will essentially instruct the MCP client to execute `dnx Knapcode.SampleMcp "remotes": [ { "transport_type": "sse", - "url": "http://mcp-fs.example.com/sse" + "url": "http://mcp-fs.anonymous.modelcontextprotocol.io/sse" } ] }, @@ -418,7 +418,7 @@ The `dnx` tool ships with the .NET 10 SDK, starting with Preview 6. ```json { "server": { - "name": "com.example/hybrid-mcp", + "name": "io.modelcontextprotocol.anonymous/hybrid-mcp", "description": "MCP server available as both local package and remote service", "repository": { "url": "https://github.com/example/hybrid-mcp", @@ -448,7 +448,7 @@ The `dnx` tool ships with the .NET 10 SDK, starting with Preview 6. "remotes": [ { "transport_type": "sse", - "url": "https://mcp.example.com/sse", + "url": "https://mcp.anonymous.modelcontextprotocol.io/sse", "headers": [ { "name": "X-API-Key", @@ -466,7 +466,7 @@ The `dnx` tool ships with the .NET 10 SDK, starting with Preview 6. }, { "transport_type": "streamable", - "url": "https://mcp.example.com/http" + "url": "https://mcp.anonymous.modelcontextprotocol.io/http" } ] }, diff --git a/internal/api/handlers/v0/publish.go b/internal/api/handlers/v0/publish.go index 78302f2..2332e7d 100644 --- a/internal/api/handlers/v0/publish.go +++ b/internal/api/handlers/v0/publish.go @@ -71,7 +71,7 @@ func RegisterPublishEndpoint(api huma.API, registry service.RegistryService, cfg // Publish the server with extensions publishedServer, err := registry.Publish(publishRequest) if err != nil { - return nil, huma.Error500InternalServerError("Failed to publish server", err) + return nil, huma.Error400BadRequest("Failed to publish server", err) } // Return the published server in extension wrapper format diff --git a/internal/api/handlers/v0/publish_integration_test.go b/internal/api/handlers/v0/publish_integration_test.go index 60ffb97..ece2173 100644 --- a/internal/api/handlers/v0/publish_integration_test.go +++ b/internal/api/handlers/v0/publish_integration_test.go @@ -99,10 +99,10 @@ func TestPublishIntegration(t *testing.T) { assert.Equal(t, publishReq.Server.VersionDetail.Version, response.Server.VersionDetail.Version) }) - t.Run("successful publish without auth (no prefix)", func(t *testing.T) { + t.Run("successful publish with none auth (no prefix)", func(t *testing.T) { publishReq := model.PublishRequest{ Server: model.ServerDetail{ - Name: "test-mcp-server-no-auth", + Name: "com.example/test-mcp-server-no-auth", Description: "A test MCP server without authentication", Repository: model.Repository{ URL: "https://example.com/test-mcp-server", diff --git a/internal/api/handlers/v0/publish_test.go b/internal/api/handlers/v0/publish_test.go index 54befd7..3554151 100644 --- a/internal/api/handlers/v0/publish_test.go +++ b/internal/api/handlers/v0/publish_test.go @@ -203,9 +203,9 @@ func TestPublishEndpoint(t *testing.T) { }, }, setupMocks: func(registry *MockRegistryService) { - registry.On("Publish", mock.AnythingOfType("model.PublishRequest")).Return(nil, errors.New("database error")) + registry.On("Publish", mock.AnythingOfType("model.PublishRequest")).Return(nil, errors.New("cannot publish duplicate version")) }, - expectedStatus: http.StatusInternalServerError, + expectedStatus: http.StatusBadRequest, expectedError: "Failed to publish server", }, } diff --git a/internal/database/database.go b/internal/database/database.go index b9a6aa2..8e07ce5 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -13,7 +13,7 @@ var ( ErrAlreadyExists = errors.New("record already exists") ErrInvalidInput = errors.New("invalid input") ErrDatabase = errors.New("database error") - ErrInvalidVersion = errors.New("invalid version: cannot publish older version after newer version") + ErrInvalidVersion = errors.New("invalid version: cannot publish duplicate version") ErrMaxServersReached = errors.New("maximum number of versions for this server reached (10000): please reach out at https://github.com/modelcontextprotocol/registry to explain your use case") ) diff --git a/internal/database/memory.go b/internal/database/memory.go index f34c810..72790f9 100644 --- a/internal/database/memory.go +++ b/internal/database/memory.go @@ -42,7 +42,7 @@ func NewMemoryDB(e map[string]*model.ServerDetail) *MemoryDB { } -// List retrieves ServerRecord entries with optional filtering and pagination +//nolint:cyclop // Complexity from filtering logic is acceptable for memory implementation func (db *MemoryDB) List( ctx context.Context, filter map[string]any, @@ -89,6 +89,18 @@ func (db *MemoryDB) List( if string(entry.ServerJSON.Status) != value.(string) { include = false } + case "remote_url": + found := false + remoteURL := value.(string) + for _, remote := range entry.ServerJSON.Remotes { + if remote.URL == remoteURL { + found = true + break + } + } + if !found { + include = false + } } } diff --git a/internal/database/postgres.go b/internal/database/postgres.go index a3e57b9..7088699 100644 --- a/internal/database/postgres.go +++ b/internal/database/postgres.go @@ -76,6 +76,10 @@ func (db *PostgreSQL) List( whereClause += fmt.Sprintf(" AND s.status = $%d", argIndex) args = append(args, v) argIndex++ + case "remote_url": + whereClause += fmt.Sprintf(" AND EXISTS (SELECT 1 FROM jsonb_array_elements(CASE WHEN jsonb_typeof(s.remotes) = 'array' THEN s.remotes ELSE '[]'::jsonb END) AS remote WHERE remote->>'url' = $%d)", argIndex) + args = append(args, v) + argIndex++ } } diff --git a/internal/model/model.go b/internal/model/model.go index e98e9fb..dbcc23b 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -3,6 +3,9 @@ package model import ( "encoding/json" "fmt" + "net/url" + "slices" + "strings" "time" ) @@ -32,7 +35,6 @@ const ( ServerStatusDeprecated ServerStatus = "deprecated" ) - // Repository represents a source code repository as defined in the spec type Repository struct { URL string `json:"url" bson:"url"` @@ -40,7 +42,6 @@ type Repository struct { ID string `json:"id,omitempty" bson:"id,omitempty"` } - // create an enum for Format type Format string @@ -108,8 +109,7 @@ type VersionDetail struct { Version string `json:"version" bson:"version"` } - -// ServerDetail represents complete server information as defined in the MCP spec (pure, no registry metadata) +// ServerDetail represents complete server information as defined in the MCP spec (pure, no registry metadata) type ServerDetail struct { Name string `json:"name" bson:"name"` Description string `json:"description" bson:"description"` @@ -131,8 +131,8 @@ type RegistryMetadata struct { // ServerRecord represents the complete storage model that separates server.json from registry metadata type ServerRecord struct { - ServerJSON ServerDetail `json:"server" bson:"server"` // Pure MCP server.json - RegistryMetadata RegistryMetadata `json:"registry_metadata" bson:"registry_metadata"` // Registry-generated data + ServerJSON ServerDetail `json:"server" bson:"server"` // Pure MCP server.json + RegistryMetadata RegistryMetadata `json:"registry_metadata" bson:"registry_metadata"` // Registry-generated data PublisherExtensions map[string]interface{} `json:"publisher_extensions" bson:"publisher_extensions"` // x-publisher extensions } @@ -244,22 +244,114 @@ func ParseServerName(serverDetail ServerDetail) (string, error) { if name == "" { return "", fmt.Errorf("server name is required and must be a string") } + + // Validate format: dns-namespace/name + if !strings.Contains(name, "/") { + return "", fmt.Errorf("server name must be in format 'dns-namespace/name' (e.g., 'com.example.api/server')") + } + + parts := strings.SplitN(name, "/", 2) + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return "", fmt.Errorf("server name must be in format 'dns-namespace/name' with non-empty namespace and name parts") + } + return name, nil } +// ValidateRemoteNamespaceMatch validates that remote URLs match the reverse-DNS namespace +func ValidateRemoteNamespaceMatch(serverDetail ServerDetail) error { + namespace := serverDetail.Name + + for _, remote := range serverDetail.Remotes { + if err := validateRemoteURLMatchesNamespace(remote.URL, namespace); err != nil { + return fmt.Errorf("remote URL %s does not match namespace %s: %w", remote.URL, namespace, err) + } + } + + return nil +} + +// validateRemoteURLMatchesNamespace checks if a remote URL's hostname matches the publisher domain from the namespace +func validateRemoteURLMatchesNamespace(remoteURL, namespace string) error { + // Parse the URL to extract the hostname + parsedURL, err := url.Parse(remoteURL) + if err != nil { + return fmt.Errorf("invalid URL format: %w", err) + } + + hostname := parsedURL.Hostname() + if hostname == "" { + return fmt.Errorf("URL must have a valid hostname") + } + + // Skip validation for localhost and local development URLs + if hostname == "localhost" || strings.HasSuffix(hostname, ".localhost") || hostname == "127.0.0.1" { + return nil + } + + // Extract publisher domain from reverse-DNS namespace + publisherDomain := extractPublisherDomainFromNamespace(namespace) + if publisherDomain == "" { + return fmt.Errorf("invalid namespace format: cannot extract domain from %s", namespace) + } + + // Check if the remote URL hostname matches the publisher domain or is a subdomain + if !isValidHostForDomain(hostname, publisherDomain) { + return fmt.Errorf("remote URL host %s does not match publisher domain %s", hostname, publisherDomain) + } + + return nil +} + +// extractPublisherDomainFromNamespace converts reverse-DNS namespace to normal domain format +// e.g., "com.example" -> "example.com" +func extractPublisherDomainFromNamespace(namespace string) string { + // Extract the namespace part before the first slash + namespacePart := namespace + if slashIdx := strings.Index(namespace, "/"); slashIdx != -1 { + namespacePart = namespace[:slashIdx] + } + + // Split into parts and reverse them to get normal domain format + parts := strings.Split(namespacePart, ".") + if len(parts) < 2 { + return "" + } + + // Reverse the parts to convert from reverse-DNS to normal domain + slices.Reverse(parts) + + return strings.Join(parts, ".") +} + +// isValidHostForDomain checks if a hostname is the domain or a subdomain of the publisher domain +func isValidHostForDomain(hostname, publisherDomain string) bool { + // Exact match + if hostname == publisherDomain { + return true + } + + // Subdomain match - hostname should end with "." + publisherDomain + if strings.HasSuffix(hostname, "."+publisherDomain) { + return true + } + + return false +} + // ToServerResponse converts a ServerRecord to API response format func (sr *ServerRecord) ToServerResponse() ServerResponse { response := ServerResponse{ Server: sr.ServerJSON, } - + // Add registry metadata extension response.XIOModelContextProtocolRegistry = sr.RegistryMetadata.CreateRegistryExtensions()["x-io.modelcontextprotocol.registry"] - + // Add publisher extensions directly if len(sr.PublisherExtensions) > 0 { response.XPublisher = sr.PublisherExtensions } - + return response } diff --git a/internal/model/model_test.go b/internal/model/model_test.go index 7e28ad1..d5ad3b7 100644 --- a/internal/model/model_test.go +++ b/internal/model/model_test.go @@ -98,17 +98,17 @@ func TestExtractPublisherExtensions_DoesNotDoubleNest(t *testing.T) { request := PublishRequest{ Server: ServerDetail{Name: "test"}, XPublisher: map[string]interface{}{ - "tool": "publisher-cli", + "tool": "publisher-cli", "version": "1.0.0", }, } result := ExtractPublisherExtensions(request) - + // Verify we get the data directly, not wrapped in another "x-publisher" key assert.Equal(t, "publisher-cli", result["tool"]) assert.Equal(t, "1.0.0", result["version"]) - + // Verify we don't have double nesting assert.NotContains(t, result, "x-publisher") } @@ -143,7 +143,7 @@ func TestRegistryMetadata_CreateRegistryExtensions(t *testing.T) { func TestServerResponse_JSONSerialization(t *testing.T) { // Test that ServerResponse properly serializes to extension wrapper format publishedTime := time.Date(2023, 12, 1, 10, 30, 0, 0, time.UTC) - + response := ServerResponse{ Server: ServerDetail{ Name: "test-server", @@ -227,7 +227,7 @@ func TestPublishRequest_WithPublisherExtensions(t *testing.T) { require.NotNil(t, request.XPublisher) publisherMap := request.XPublisher.(map[string]interface{}) assert.Equal(t, "publisher-cli", publisherMap["tool"]) - + metadata := publisherMap["metadata"].(map[string]interface{}) assert.Equal(t, "2023-12-01", metadata["build_date"]) assert.Equal(t, "abc123", metadata["commit"]) @@ -256,10 +256,213 @@ func TestServerResponse_EmptyExtensions(t *testing.T) { // Should still have the structure, even with nil publisher extensions assert.Contains(t, parsed, "server") assert.Contains(t, parsed, "x-io.modelcontextprotocol.registry") - + // x-publisher should be null/nil in JSON when empty publisherValue, exists := parsed["x-publisher"] if exists { assert.Nil(t, publisherValue) } -} \ No newline at end of file +} + +func TestValidateRemoteNamespaceMatch(t *testing.T) { + tests := []struct { + name string + serverDetail ServerDetail + expectError bool + errorMsg string + }{ + { + name: "valid match - example.com domain", + serverDetail: ServerDetail{ + Name: "com.example/test-server", + Remotes: []Remote{ + {URL: "https://example.com/mcp"}, + }, + }, + expectError: false, + }, + { + name: "valid match - subdomain mcp.example.com", + serverDetail: ServerDetail{ + Name: "com.example/test-server", + Remotes: []Remote{ + {URL: "https://mcp.example.com/endpoint"}, + }, + }, + expectError: false, + }, + { + name: "valid match - api subdomain", + serverDetail: ServerDetail{ + Name: "com.example/api-server", + Remotes: []Remote{ + {URL: "https://api.example.com/mcp"}, + }, + }, + expectError: false, + }, + { + name: "invalid - wrong domain", + serverDetail: ServerDetail{ + Name: "com.example/test-server", + Remotes: []Remote{ + {URL: "https://google.com/mcp"}, + }, + }, + expectError: true, + errorMsg: "remote URL host google.com does not match publisher domain example.com", + }, + { + name: "invalid - different domain entirely", + serverDetail: ServerDetail{ + Name: "com.microsoft/server", + Remotes: []Remote{ + {URL: "https://api.github.com/endpoint"}, + }, + }, + expectError: true, + errorMsg: "remote URL host api.github.com does not match publisher domain microsoft.com", + }, + { + name: "localhost URLs allowed with any namespace", + serverDetail: ServerDetail{ + Name: "com.example/test-server", + Remotes: []Remote{ + {URL: "http://localhost:3000/sse"}, + }, + }, + expectError: false, + }, + { + name: "invalid URL format", + serverDetail: ServerDetail{ + Name: "com.example/test", + Remotes: []Remote{ + {URL: "not-a-valid-url"}, + }, + }, + expectError: true, + errorMsg: "URL must have a valid hostname", + }, + { + name: "empty remotes array", + serverDetail: ServerDetail{ + Name: "com.example/test", + Remotes: []Remote{}, + }, + expectError: false, + }, + { + name: "multiple valid remotes - different subdomains", + serverDetail: ServerDetail{ + Name: "com.example/server", + Remotes: []Remote{ + {URL: "https://api.example.com/sse"}, + {URL: "https://mcp.example.com/websocket"}, + }, + }, + expectError: false, + }, + { + name: "one valid, one invalid remote", + serverDetail: ServerDetail{ + Name: "com.example/server", + Remotes: []Remote{ + {URL: "https://example.com/sse"}, + {URL: "https://google.com/websocket"}, + }, + }, + expectError: true, + errorMsg: "remote URL host google.com does not match publisher domain example.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateRemoteNamespaceMatch(tt.serverDetail) + + if tt.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.errorMsg) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestParseServerName(t *testing.T) { + tests := []struct { + name string + serverDetail ServerDetail + expectError bool + errorMsg string + }{ + { + name: "valid namespace/name format", + serverDetail: ServerDetail{ + Name: "com.example.api/server", + }, + expectError: false, + }, + { + name: "valid complex namespace", + serverDetail: ServerDetail{ + Name: "com.github.microsoft.azure/webapp-server", + }, + expectError: false, + }, + { + name: "empty server name", + serverDetail: ServerDetail{ + Name: "", + }, + expectError: true, + errorMsg: "server name is required", + }, + { + name: "missing slash separator", + serverDetail: ServerDetail{ + Name: "com.example.server", + }, + expectError: true, + errorMsg: "server name must be in format 'dns-namespace/name'", + }, + { + name: "empty namespace part", + serverDetail: ServerDetail{ + Name: "/server-name", + }, + expectError: true, + errorMsg: "non-empty namespace and name parts", + }, + { + name: "empty name part", + serverDetail: ServerDetail{ + Name: "com.example/", + }, + expectError: true, + errorMsg: "non-empty namespace and name parts", + }, + { + name: "multiple slashes - uses first as separator", + serverDetail: ServerDetail{ + Name: "com.example/server/path", + }, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := ParseServerName(tt.serverDetail) + + if tt.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.errorMsg) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/internal/service/registry_service.go b/internal/service/registry_service.go index 6fc76c5..a2cdc59 100644 --- a/internal/service/registry_service.go +++ b/internal/service/registry_service.go @@ -3,6 +3,7 @@ package service import ( "context" "errors" + "fmt" "time" "github.com/google/uuid" @@ -78,11 +79,21 @@ func (s *registryServiceImpl) Publish(req model.PublishRequest) (*model.ServerRe return nil, err } - // Validate server name exists + // Validate server name exists and format if _, err := model.ParseServerName(req.Server); err != nil { return nil, err } + // Validate reverse-DNS namespace matching for remote URLs + if err := model.ValidateRemoteNamespaceMatch(req.Server); err != nil { + return nil, err + } + + // Check for duplicate remote URLs + if err := s.validateNoDuplicateRemoteURLs(ctx, req.Server); err != nil { + return nil, err + } + // Get the new version's details newVersion := req.Server.VersionDetail.Version newName := req.Server.Name @@ -154,3 +165,28 @@ func (s *registryServiceImpl) Publish(req model.PublishRequest) (*model.ServerRe response := serverRecord.ToServerResponse() return &response, nil } + +// validateNoDuplicateRemoteURLs checks that no other server is using the same remote URLs +func (s *registryServiceImpl) validateNoDuplicateRemoteURLs(ctx context.Context, serverDetail model.ServerDetail) error { + // Check each remote URL in the new server for conflicts + for _, remote := range serverDetail.Remotes { + // Use filter to find servers with this remote URL + filter := map[string]any{ + "remote_url": remote.URL, + } + + conflictingServers, _, err := s.db.List(ctx, filter, "", 1000) + if err != nil { + return fmt.Errorf("failed to check remote URL conflict: %w", err) + } + + // Check if any conflicting server has a different name + for _, conflictingServer := range conflictingServers { + if conflictingServer.ServerJSON.Name != serverDetail.Name { + return fmt.Errorf("remote URL %s is already used by server %s", remote.URL, conflictingServer.ServerJSON.Name) + } + } + } + + return nil +} diff --git a/internal/service/registry_service_test.go b/internal/service/registry_service_test.go new file mode 100644 index 0000000..84b33a4 --- /dev/null +++ b/internal/service/registry_service_test.go @@ -0,0 +1,121 @@ +//nolint:testpackage +package service + +import ( + "context" + "testing" + + "github.com/modelcontextprotocol/registry/internal/database" + "github.com/modelcontextprotocol/registry/internal/model" + "github.com/stretchr/testify/assert" +) + +func TestValidateNoDuplicateRemoteURLs(t *testing.T) { + // Create test data + existingServers := map[string]*model.ServerDetail{ + "existing1": { + Name: "com.example/existing-server", + Description: "An existing server", + VersionDetail: model.VersionDetail{ + Version: "1.0.0", + }, + Remotes: []model.Remote{ + {URL: "https://api.example.com/mcp"}, + {URL: "https://webhook.example.com/sse"}, + }, + }, + "existing2": { + Name: "com.microsoft/another-server", + Description: "Another existing server", + VersionDetail: model.VersionDetail{ + Version: "1.0.0", + }, + Remotes: []model.Remote{ + {URL: "https://api.microsoft.com/mcp"}, + }, + }, + } + + memDB := database.NewMemoryDB(existingServers) + service := NewRegistryServiceWithDB(memDB) + + tests := []struct { + name string + serverDetail model.ServerDetail + expectError bool + errorMsg string + }{ + { + name: "no remote URLs - should pass", + serverDetail: model.ServerDetail{ + Name: "com.example/new-server", + Description: "A new server with no remotes", + VersionDetail: model.VersionDetail{ + Version: "1.0.0", + }, + Remotes: []model.Remote{}, + }, + expectError: false, + }, + { + name: "new unique remote URLs - should pass", + serverDetail: model.ServerDetail{ + Name: "com.example/new-server", + Description: "A new server", + VersionDetail: model.VersionDetail{ + Version: "1.0.0", + }, + Remotes: []model.Remote{ + {URL: "https://new.example.com/mcp"}, + {URL: "https://unique.example.com/sse"}, + }, + }, + expectError: false, + }, + { + name: "duplicate remote URL - should fail", + serverDetail: model.ServerDetail{ + Name: "com.example/new-server", + Description: "A new server with duplicate URL", + VersionDetail: model.VersionDetail{ + Version: "1.0.0", + }, + Remotes: []model.Remote{ + {URL: "https://api.example.com/mcp"}, // This URL already exists + }, + }, + expectError: true, + errorMsg: "remote URL https://api.example.com/mcp is already used by server com.example/existing-server", + }, + { + name: "updating same server with same URLs - should pass", + serverDetail: model.ServerDetail{ + Name: "com.example/existing-server", // Same name as existing + Description: "Updated existing server", + VersionDetail: model.VersionDetail{ + Version: "1.1.0", + }, + Remotes: []model.Remote{ + {URL: "https://api.example.com/mcp"}, // Same URL as before + }, + }, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + impl := service.(*registryServiceImpl) + + err := impl.validateNoDuplicateRemoteURLs(ctx, tt.serverDetail) + + if tt.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.errorMsg) + } else { + assert.NoError(t, err) + } + }) + } +} \ No newline at end of file