Remove namespace match validation for websiteUrl and remote URL (#769)
<!-- Provide a brief summary of your changes --> ## Motivation and Context <!-- Why is this change needed? What problem does it solve? --> Resolve https://github.com/modelcontextprotocol/registry/issues/768 and https://github.com/modelcontextprotocol/registry/issues/494. Require HTTPS for remote URLs. Currently all remote URLs on the MCP Registry are HTTPS. Also fix a test bug when running on Windows (bad temp path). ## How Has This Been Tested? <!-- Have you tested this in a real application? Which scenarios were tested? --> Unit tests. Published a server.json locally with a website URL not matching the server name. ## Breaking Changes <!-- Will users need to update their code or configurations? --> ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [x] I have added appropriate error handling - [ ] ~I have added or updated documentation as needed~ N/A ## Additional context <!-- Add any other context, implementation notes, or design decisions -->
This commit is contained in:
parent
d9c2e506a9
commit
45bf43f9d1
5 changed files with 30 additions and 118 deletions
|
|
@ -280,7 +280,7 @@ This will essentially instruct the MCP client to execute `dnx Knapcode.SampleMcp
|
||||||
"remotes": [
|
"remotes": [
|
||||||
{
|
{
|
||||||
"type": "streamable-http",
|
"type": "streamable-http",
|
||||||
"url": "http://mcp-fs.anonymous.modelcontextprotocol.io/http"
|
"url": "https://mcp-fs.anonymous.modelcontextprotocol.io/http"
|
||||||
}
|
}
|
||||||
],
|
],
|
||||||
"_meta": {
|
"_meta": {
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,7 @@ import (
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"os"
|
"os"
|
||||||
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/modelcontextprotocol/registry/internal/config"
|
"github.com/modelcontextprotocol/registry/internal/config"
|
||||||
|
|
@ -20,7 +21,7 @@ import (
|
||||||
|
|
||||||
func TestImportService_LocalFile(t *testing.T) {
|
func TestImportService_LocalFile(t *testing.T) {
|
||||||
// Create a temporary seed file
|
// Create a temporary seed file
|
||||||
tempFile := "/tmp/test_import_seed.json"
|
tempFile := filepath.Join(os.TempDir(), "test_import_seed.json")
|
||||||
seedData := []*apiv0.ServerJSON{
|
seedData := []*apiv0.ServerJSON{
|
||||||
{
|
{
|
||||||
Schema: model.CurrentSchemaURL,
|
Schema: model.CurrentSchemaURL,
|
||||||
|
|
|
||||||
|
|
@ -144,6 +144,10 @@ func IsValidRemoteURL(rawURL string) bool {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if u.Scheme != "https" {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -6,7 +6,6 @@ import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/url"
|
"net/url"
|
||||||
"regexp"
|
"regexp"
|
||||||
"slices"
|
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"github.com/modelcontextprotocol/registry/internal/config"
|
"github.com/modelcontextprotocol/registry/internal/config"
|
||||||
|
|
@ -108,16 +107,6 @@ func ValidateServerJSON(serverJSON *apiv0.ServerJSON) error {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Validate reverse-DNS namespace matching for remote URLs
|
|
||||||
if err := validateRemoteNamespaceMatch(*serverJSON); err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
// Validate reverse-DNS namespace matching for website URL
|
|
||||||
if err := validateWebsiteURLNamespaceMatch(*serverJSON); err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -509,99 +498,3 @@ func parseServerName(serverJSON apiv0.ServerJSON) (string, error) {
|
||||||
|
|
||||||
return name, nil
|
return name, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// validateRemoteNamespaceMatch validates that remote URLs match the reverse-DNS namespace
|
|
||||||
func validateRemoteNamespaceMatch(serverJSON apiv0.ServerJSON) error {
|
|
||||||
namespace := serverJSON.Name
|
|
||||||
|
|
||||||
for _, remote := range serverJSON.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
|
|
||||||
}
|
|
||||||
|
|
||||||
// validateWebsiteURLNamespaceMatch validates that website URL matches the reverse-DNS namespace
|
|
||||||
func validateWebsiteURLNamespaceMatch(serverJSON apiv0.ServerJSON) error {
|
|
||||||
// Skip validation if website URL is not provided
|
|
||||||
if serverJSON.WebsiteURL == "" {
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
|
|
||||||
namespace := serverJSON.Name
|
|
||||||
if err := validateRemoteURLMatchesNamespace(serverJSON.WebsiteURL, namespace); err != nil {
|
|
||||||
return fmt.Errorf("websiteUrl %s does not match namespace %s: %w", serverJSON.WebsiteURL, 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
|
|
||||||
}
|
|
||||||
|
|
|
||||||
|
|
@ -454,6 +454,21 @@ func TestValidate(t *testing.T) {
|
||||||
},
|
},
|
||||||
expectedError: "websiteUrl must use https scheme: ftp://example.com/docs",
|
expectedError: "websiteUrl must use https scheme: ftp://example.com/docs",
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "server with invalid websiteUrl - required HTTPS",
|
||||||
|
serverDetail: apiv0.ServerJSON{
|
||||||
|
Schema: model.CurrentSchemaURL,
|
||||||
|
Name: "com.example/test-server",
|
||||||
|
Description: "A test server",
|
||||||
|
Repository: &model.Repository{
|
||||||
|
URL: "https://github.com/owner/repo",
|
||||||
|
Source: "github",
|
||||||
|
},
|
||||||
|
Version: "1.0.0",
|
||||||
|
WebsiteURL: "http://example.com/docs",
|
||||||
|
},
|
||||||
|
expectedError: "websiteUrl must use https scheme: http://example.com/docs",
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "server with malformed websiteUrl",
|
name: "server with malformed websiteUrl",
|
||||||
serverDetail: apiv0.ServerJSON{
|
serverDetail: apiv0.ServerJSON{
|
||||||
|
|
@ -512,7 +527,7 @@ func TestValidate(t *testing.T) {
|
||||||
Version: "1.0.0",
|
Version: "1.0.0",
|
||||||
WebsiteURL: "https://different.com/docs",
|
WebsiteURL: "https://different.com/docs",
|
||||||
},
|
},
|
||||||
expectedError: "websiteUrl https://different.com/docs does not match namespace com.example/test-server",
|
expectedError: "",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "package with spaces in name",
|
name: "package with spaces in name",
|
||||||
|
|
@ -797,7 +812,7 @@ func TestValidate_RemoteNamespaceMatch(t *testing.T) {
|
||||||
expectError: false,
|
expectError: false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "invalid - wrong domain",
|
name: "valid - different domain",
|
||||||
serverDetail: apiv0.ServerJSON{
|
serverDetail: apiv0.ServerJSON{
|
||||||
Schema: model.CurrentSchemaURL,
|
Schema: model.CurrentSchemaURL,
|
||||||
Name: "com.example/test-server",
|
Name: "com.example/test-server",
|
||||||
|
|
@ -808,23 +823,22 @@ func TestValidate_RemoteNamespaceMatch(t *testing.T) {
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
expectError: true,
|
expectError: false,
|
||||||
errorMsg: "remote URL host google.com does not match publisher domain example.com",
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "invalid - different domain entirely",
|
name: "invalid - not HTTPS",
|
||||||
serverDetail: apiv0.ServerJSON{
|
serverDetail: apiv0.ServerJSON{
|
||||||
Schema: model.CurrentSchemaURL,
|
Schema: model.CurrentSchemaURL,
|
||||||
Name: "com.microsoft/server",
|
Name: "com.microsoft/server",
|
||||||
Remotes: []model.Transport{
|
Remotes: []model.Transport{
|
||||||
{
|
{
|
||||||
Type: "streamable-http",
|
Type: "streamable-http",
|
||||||
URL: "https://api.github.com/endpoint",
|
URL: "http://api.github.com/endpoint",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
expectError: true,
|
expectError: true,
|
||||||
errorMsg: "remote URL host api.github.com does not match publisher domain microsoft.com",
|
errorMsg: "invalid remote URL: http://api.github.com/endpoint",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "invalid URL format",
|
name: "invalid URL format",
|
||||||
|
|
@ -880,12 +894,12 @@ func TestValidate_RemoteNamespaceMatch(t *testing.T) {
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
Type: "streamable-http",
|
Type: "streamable-http",
|
||||||
URL: "https://google.com/websocket",
|
URL: "http://example.com/sse",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
expectError: true,
|
expectError: true,
|
||||||
errorMsg: "remote URL host google.com does not match publisher domain example.com",
|
errorMsg: "invalid remote URL: http://example.com/sse",
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue