feat: implement DNS namespace validation and remote URL uniqueness checks (#306)
## Summary - Add reverse-DNS namespace format validation for server names (namespace/name) - Validate remote URLs match the DNS namespace derived from hostname - Prevent duplicate remote URLs across different servers - Support localhost/development URLs without namespace restrictions - Add comprehensive test coverage for validation logic - Update database layer to support remote_url filtering Fixes #180 by ensuring proper namespace governance and preventing URL conflicts. --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
d663406280
commit
923b2e07b9
2 changed files with 497 additions and 29 deletions
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue