Validate server registry urls are allowlisted for that registry type (#348)

## Summary
Closes a validation gap that allowed servers to be published with
unsupported registry types and base URLs.

## Changes
- **Registry Type Validation**: Only allow `npm`, `pypi`, `oci`,
`nuget`, `mcpb`
- **Registry Base URL Validation**: Enforce strict allowlist of official
registry URLs
- **Required Fields**: Registry type and base URL required when package
identifier provided
- **Comprehensive Tests**: Clear test structure showing valid vs invalid
cases side-by-side

## Validation Rules
- `npm` → `https://registry.npmjs.org`
- `pypi` → `https://pypi.org`
- `oci` → `https://docker.io` 
- `nuget` → `https://api.nuget.org`
- `mcpb` → `https://github.com` or `https://gitlab.com`

All other combinations are rejected with clear error messages.
This commit is contained in:
adam jones 2025-09-03 16:01:56 +01:00 • committed by GitHub
commit e67997c177
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 430 additions and 5 deletions

View file

@ -13,6 +13,11 @@ var (
// Remote validation errors
ErrInvalidRemoteURL = errors.New("invalid remote URL")
// Registry validation errors
ErrUnsupportedRegistryType = errors.New("unsupported registry type")
ErrUnsupportedRegistryBaseURL = errors.New("unsupported registry base URL")
ErrMismatchedRegistryTypeAndURL = errors.New("registry type and base URL do not match")
// Argument validation errors
ErrNamedArgumentNameRequired = errors.New("named argument name is required")
ErrInvalidNamedArgumentName = errors.New("invalid named argument name format")

View file

@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"net/url"
"regexp"
"slices"
"strings"
@ -141,7 +142,13 @@ func validateRemote(obj *model.Remote) error {
return nil
}
func validateMCPBPackage(host string) error {
func validateMCPBPackage(fullURL string) error {
parsedURL, err := url.Parse(fullURL)
if err != nil {
return fmt.Errorf("invalid MCPB package URL: %w", err)
}
host := strings.ToLower(parsedURL.Host)
allowedHosts := []string{
"github.com",
"www.github.com",
@ -161,12 +168,178 @@ func validateMCPBPackage(host string) error {
return fmt.Errorf("MCPB packages must be hosted on allowlisted providers (GitHub or GitLab). Host '%s' is not allowed", host)
}
// Validate URL path is a proper release URL with strict structure validation
path := parsedURL.Path
switch host {
case "github.com", "www.github.com":
// GitHub release URLs must match: /owner/repo/releases/download/tag/filename
if !isValidGitHubReleaseURL(path) {
return fmt.Errorf("GitHub MCPB packages must be release assets following the pattern '/owner/repo/releases/download/tag/filename'")
}
case "gitlab.com", "www.gitlab.com":
// GitLab release URLs must match specific patterns
if !isValidGitLabReleaseURL(path) {
return fmt.Errorf("GitLab MCPB packages must be release assets following patterns '/owner/repo/-/releases/tag/downloads/filename' or '/owner/repo/-/package_files/id/download'")
}
}
return nil
}
// getDefaultRegistryBaseURL returns the default registry base URL for a given registry type
func getDefaultRegistryBaseURL(registryType string) string {
defaultURLs := map[string]string{
model.RegistryTypeNPM: model.RegistryURLNPM,
model.RegistryTypePyPI: model.RegistryURLPyPI,
model.RegistryTypeOCI: model.RegistryURLDocker,
model.RegistryTypeNuGet: model.RegistryURLNuGet,
}
return defaultURLs[registryType]
}
// isValidGitHubReleaseURL validates that a path follows the GitHub release asset pattern
// Pattern: /owner/repo/releases/download/tag/filename
func isValidGitHubReleaseURL(path string) bool {
// GitHub release URL pattern: /owner/repo/releases/download/tag/filename
// - owner: username or organization (1-39 chars, alphanumeric + hyphens, no consecutive hyphens)
// - repo: repository name (similar rules to owner)
// - tag: release tag (can contain various characters but not empty)
// - filename: asset filename (not empty)
pattern := `^/([a-zA-Z0-9]([a-zA-Z0-9\-]{0,37}[a-zA-Z0-9])?)/([a-zA-Z0-9._\-]+)/releases/download/([^/]+)/([^/]+)$`
matched, _ := regexp.MatchString(pattern, path)
return matched
}
// isValidGitLabReleaseURL validates that a path follows GitLab release asset patterns
func isValidGitLabReleaseURL(path string) bool {
// GitLab release URL patterns:
// 1. /owner/repo/-/releases/tag/downloads/filename
// 2. /owner/repo/-/package_files/id/download
// 3. /group/subgroup/repo/-/releases/tag/downloads/filename (nested groups)
// The key insight is that GitLab URLs have "/-/" as a delimiter that separates the
// project path from the GitLab-specific routes. Everything before "/-/" is the project path.
// Pattern 1: Release downloads with /-/releases/tag/downloads/filename
releasePattern := `^/([a-zA-Z0-9._\-]+(?:/[a-zA-Z0-9._\-]+)*)/-/releases/([^/]+)/downloads/([^/]+)$`
if matched, _ := regexp.MatchString(releasePattern, path); matched {
return true
}
// Pattern 2: Package files with /-/package_files/id/download
packagePattern := `^/([a-zA-Z0-9._\-]+(?:/[a-zA-Z0-9._\-]+)*)/-/package_files/([0-9]+)/download$`
if matched, _ := regexp.MatchString(packagePattern, path); matched {
return true
}
return false
}
// inferMCPBRegistryBaseURL infers the registry base URL from an MCPB identifier
func inferMCPBRegistryBaseURL(identifier string) string {
parsedURL, err := url.Parse(identifier)
if err != nil {
return ""
}
host := strings.ToLower(parsedURL.Host)
switch host {
case "github.com", "www.github.com":
return model.RegistryURLGitHub
case "gitlab.com", "www.gitlab.com":
return model.RegistryURLGitLab
default:
return ""
}
}
// validateRegistryType checks if the registry type is supported
func validateRegistryType(registryType string) error {
// Registry type is required
if registryType == "" {
return fmt.Errorf("%w: registry type is required", ErrUnsupportedRegistryType)
}
supportedTypes := []string{
model.RegistryTypeNPM,
model.RegistryTypePyPI,
model.RegistryTypeOCI,
model.RegistryTypeNuGet,
model.RegistryTypeMCPB,
}
for _, supported := range supportedTypes {
if registryType == supported {
return nil
}
}
return fmt.Errorf("%w: '%s'. Supported types: %v", ErrUnsupportedRegistryType, registryType, supportedTypes)
}
// validateRegistryBaseURL checks if the registry base URL is valid for the given registry type
func validateRegistryBaseURL(registryType, baseURL string) error {
// Base URL is required for all registry types except MCPB (which uses direct URLs)
if baseURL == "" {
if registryType == model.RegistryTypeMCPB {
return nil // MCPB packages use direct URLs in the identifier
}
return fmt.Errorf("%w: registry base URL is required for registry type '%s'", ErrUnsupportedRegistryBaseURL, registryType)
}
// Define expected base URLs for each registry type
expectedURLs := map[string][]string{
model.RegistryTypeNPM: {model.RegistryURLNPM},
model.RegistryTypePyPI: {model.RegistryURLPyPI},
model.RegistryTypeOCI: {model.RegistryURLDocker},
model.RegistryTypeNuGet: {model.RegistryURLNuGet},
model.RegistryTypeMCPB: {model.RegistryURLGitHub, model.RegistryURLGitLab},
}
// Check if the base URL is valid for the registry type
if expectedURLsForType, exists := expectedURLs[registryType]; exists {
for _, expected := range expectedURLsForType {
if baseURL == expected {
return nil
}
}
return fmt.Errorf("%w: '%s' is not valid for registry type '%s'. Expected: %v",
ErrMismatchedRegistryTypeAndURL, baseURL, registryType, expectedURLsForType)
}
// If registry type is not in our expected URLs map but base URL is provided,
// it's likely an unsupported base URL
return fmt.Errorf("%w: '%s'", ErrUnsupportedRegistryBaseURL, baseURL)
}
func validatePackage(pkg *model.Package) error {
registryType := strings.ToLower(pkg.RegistryType)
// Only validate if package has an identifier (i.e., it's a real package reference)
if pkg.Identifier != "" {
// Validate registry type is supported
if err := validateRegistryType(registryType); err != nil {
return err
}
// Apply defaults for registry base URL if empty
if pkg.RegistryBaseURL == "" {
if registryType == model.RegistryTypeMCPB {
// For MCPB, infer from identifier
pkg.RegistryBaseURL = inferMCPBRegistryBaseURL(pkg.Identifier)
} else {
// For other types, use default
pkg.RegistryBaseURL = getDefaultRegistryBaseURL(registryType)
}
}
// Validate registry base URL matches the registry type
if err := validateRegistryBaseURL(registryType, pkg.RegistryBaseURL); err != nil {
return err
}
}
// For direct download packages (mcpb or direct URLs)
if registryType == model.RegistryTypeMCPB ||
strings.HasPrefix(pkg.Identifier, "http://") || strings.HasPrefix(pkg.Identifier, "https://") {
@ -175,11 +348,9 @@ func validatePackage(pkg *model.Package) error {
return fmt.Errorf("invalid package URL: %w", err)
}
host := strings.ToLower(parsedURL.Host)
// For MCPB packages, validate they're from allowed hosts
// For MCPB packages, validate they're from allowed hosts and are release URLs
if registryType == model.RegistryTypeMCPB {
return validateMCPBPackage(host)
return validateMCPBPackage(pkg.Identifier)
}
// For other URL-based packages, just ensure it's valid

View file

@ -2,6 +2,7 @@ package validators_test
import (
"fmt"
"strings"
"testing"
"github.com/modelcontextprotocol/registry/internal/validators"
@ -648,6 +649,185 @@ func TestValidateArgument_ValidValueFields(t *testing.T) {
}
// Helper function to create a valid server with a specific argument for testing
func TestValidate_RegistryTypes(t *testing.T) {
testCases := []struct {
name string
registryType string
baseURL string
identifier string
expectError bool
}{
// Valid registry types (should pass)
{"valid_npm", model.RegistryTypeNPM, model.RegistryURLNPM, "test-package", false},
{"valid_pypi", model.RegistryTypePyPI, model.RegistryURLPyPI, "test-package", false},
{"valid_oci", model.RegistryTypeOCI, model.RegistryURLDocker, "test-package", false},
{"valid_nuget", model.RegistryTypeNuGet, model.RegistryURLNuGet, "test-package", false},
{"valid_mcpb_github", model.RegistryTypeMCPB, model.RegistryURLGitHub, "https://github.com/owner/repo/releases/download/v1.0.0/package.mcpb", false},
{"valid_mcpb_gitlab", model.RegistryTypeMCPB, model.RegistryURLGitLab, "https://gitlab.com/owner/repo/-/releases/v1.0.0/downloads/package.mcpb", false},
// Invalid registry types (should fail)
{"invalid_maven", "maven", "https://example.com/registry", "test-package", true},
{"invalid_cargo", "cargo", "https://example.com/registry", "test-package", true},
{"invalid_gem", "gem", "https://example.com/registry", "test-package", true},
{"invalid_invalid", "invalid", "https://example.com/registry", "test-package", true},
{"invalid_unknown", "UNKNOWN", "https://example.com/registry", "test-package", true},
{"invalid_custom", "custom-registry", "https://example.com/registry", "test-package", true},
{"invalid_github", "github", "https://example.com/registry", "test-package", true}, // This is a source, not a registry type
{"invalid_docker", "docker", "https://example.com/registry", "test-package", true}, // Should be "oci"
{"invalid_empty", "", "https://example.com/registry", "test-package", true}, // Empty registry type
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
serverDetail := apiv0.ServerJSON{
Name: "com.example/test-server",
Description: "A test server",
Repository: model.Repository{
URL: "https://github.com/owner/repo",
Source: "github",
ID: "owner/repo",
},
VersionDetail: model.VersionDetail{
Version: "1.0.0",
},
Packages: []model.Package{
{
Identifier: tc.identifier,
RegistryType: tc.registryType,
RegistryBaseURL: tc.baseURL,
},
},
Remotes: []model.Remote{
{
URL: "https://example.com/remote",
},
},
}
err := validators.ValidateServerJSON(&serverDetail)
if tc.expectError {
assert.Error(t, err)
assert.Contains(t, err.Error(), validators.ErrUnsupportedRegistryType.Error())
} else {
assert.NoError(t, err)
}
})
}
}
func TestValidate_RegistryBaseURLs(t *testing.T) {
testCases := []struct {
name string
registryType string
baseURL string
identifier string
expectError bool
}{
// Invalid base URLs for specific registry types
{"npm_wrong_url", model.RegistryTypeNPM, "https://pypi.org", "test-package", true},
{"pypi_wrong_url", model.RegistryTypePyPI, "https://registry.npmjs.org", "test-package", true},
{"oci_wrong_url", model.RegistryTypeOCI, "https://registry.npmjs.org", "test-package", true},
{"nuget_wrong_url", model.RegistryTypeNuGet, "https://docker.io", "test-package", true},
{"mcpb_wrong_url", model.RegistryTypeMCPB, "https://evil.com", "https://github.com/owner/repo", true},
{"mismatched_base_url_1", model.RegistryTypeNPM, model.RegistryURLDocker, "test-package", true},
{"mismatched_base_url_2", model.RegistryTypeOCI, model.RegistryTypeNuGet, "test-package", true},
// Localhost URLs should be rejected - no development exceptions
{"localhost_npm", model.RegistryTypeNPM, "http://localhost:3000", "test-package", true},
{"localhost_ip", model.RegistryTypePyPI, "http://127.0.0.1:8080", "test-package", true},
// Valid combinations (should pass)
{"valid_npm", model.RegistryTypeNPM, model.RegistryURLNPM, "test-package", false},
{"valid_pypi", model.RegistryTypePyPI, model.RegistryURLPyPI, "test-package", false},
{"valid_oci", model.RegistryTypeOCI, model.RegistryURLDocker, "test-package", false},
{"valid_nuget", model.RegistryTypeNuGet, model.RegistryURLNuGet, "test-package", false},
{"valid_mcpb_github", model.RegistryTypeMCPB, model.RegistryURLGitHub, "https://github.com/owner/repo/releases/download/v1.0.0/package.mcpb", false},
{"valid_mcpb_gitlab", model.RegistryTypeMCPB, model.RegistryURLGitLab, "https://gitlab.com/owner/repo/-/releases/v1.0.0/downloads/package.mcpb", false},
{"empty_base_url_npm", model.RegistryTypeNPM, "", "test-package", false}, // should be inferred
{"empty_base_url_nuget", model.RegistryTypeNuGet, "", "test-package", false}, // should be inferred
{"empty_base_url_mcpb", model.RegistryTypeMCPB, "", "https://github.com/owner/repo/releases/download/v1.0.0/package.mcpb", false},
// Trailing slash URLs should be rejected - strict exact match only
{"npm_trailing_slash", model.RegistryTypeNPM, "https://registry.npmjs.org/", "test-package", true},
{"pypi_trailing_slash", model.RegistryTypePyPI, "https://pypi.org/", "test-package", true},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
serverDetail := apiv0.ServerJSON{
Name: "com.example/test-server",
Description: "A test server",
Repository: model.Repository{
URL: "https://github.com/owner/repo",
Source: "github",
ID: "owner/repo",
},
VersionDetail: model.VersionDetail{
Version: "1.0.0",
},
Packages: []model.Package{
{
Identifier: tc.identifier,
RegistryType: tc.registryType,
RegistryBaseURL: tc.baseURL,
},
},
Remotes: []model.Remote{
{
URL: "https://example.com/remote",
},
},
}
err := validators.ValidateServerJSON(&serverDetail)
if tc.expectError {
assert.Error(t, err)
// Check that the error is related to registry validation
errStr := err.Error()
assert.True(t,
strings.Contains(errStr, validators.ErrUnsupportedRegistryBaseURL.Error()) ||
strings.Contains(errStr, validators.ErrMismatchedRegistryTypeAndURL.Error()),
"Expected registry validation error, got: %s", errStr)
} else {
assert.NoError(t, err)
}
})
}
}
func TestValidate_EmptyRegistryType(t *testing.T) {
// Test that empty registry type is rejected
serverDetail := apiv0.ServerJSON{
Name: "com.example/test-server",
Description: "A test server",
Repository: model.Repository{
URL: "https://github.com/owner/repo",
Source: "github",
ID: "owner/repo",
},
VersionDetail: model.VersionDetail{
Version: "1.0.0",
},
Packages: []model.Package{
{
Identifier: "test-package",
RegistryType: "", // Empty registry type
RegistryBaseURL: "",
},
},
Remotes: []model.Remote{
{
URL: "https://example.com/remote",
},
},
}
err := validators.ValidateServerJSON(&serverDetail)
assert.Error(t, err)
assert.Contains(t, err.Error(), validators.ErrUnsupportedRegistryType.Error())
assert.Contains(t, err.Error(), "registry type is required")
}
func createValidServerWithArgument(arg model.Argument) apiv0.ServerJSON {
return apiv0.ServerJSON{
Name: "com.example/test-server",
@ -675,3 +855,72 @@ func createValidServerWithArgument(arg model.Argument) apiv0.ServerJSON {
},
}
}
func TestValidate_MCPBReleaseURLs(t *testing.T) {
testCases := []struct {
name string
identifier string
expectError bool
errorMsg string
}{
// Valid GitHub release URLs
{"valid_github_release", "https://github.com/owner/repo/releases/download/v1.0.0/package.mcpb", false, ""},
{"valid_github_release_with_path", "https://github.com/org/project/releases/download/v2.1.0/my-server.mcpb", false, ""},
{"valid_github_complex_tag", "https://github.com/owner/repo/releases/download/v1.0.0-alpha.1+build.123/package.mcpb", false, ""},
{"valid_github_single_char_owner", "https://github.com/a/b/releases/download/v1.0.0/package.mcpb", false, ""},
// Valid GitLab release URLs
{"valid_gitlab_releases", "https://gitlab.com/owner/repo/-/releases/v1.0.0/downloads/package.mcpb", false, ""},
{"valid_gitlab_package_files", "https://gitlab.com/owner/repo/-/package_files/123/download", false, ""},
{"valid_gitlab_nested_group", "https://gitlab.com/group/subgroup/repo/-/releases/v1.0.0/downloads/package.mcpb", false, ""},
{"valid_gitlab_deep_nested", "https://gitlab.com/org/team/project/repo/-/releases/v2.0.0/downloads/server.mcpb", false, ""},
// Invalid GitHub URLs (not release URLs)
{"invalid_github_root", "https://github.com/owner/repo", true, "GitHub MCPB packages must be release assets"},
{"invalid_github_tree", "https://github.com/owner/repo/tree/main", true, "GitHub MCPB packages must be release assets"},
{"invalid_github_blob", "https://github.com/owner/repo/blob/main/file.mcpb", true, "GitHub MCPB packages must be release assets"},
{"invalid_github_fake_release_path", "https://github.com/owner/repo/fake/releases/download/v1.0.0/file.mcpb", true, "GitHub MCPB packages must be release assets"},
{"invalid_github_missing_tag", "https://github.com/owner/repo/releases/download//file.mcpb", true, "GitHub MCPB packages must be release assets"},
{"invalid_github_missing_filename", "https://github.com/owner/repo/releases/download/v1.0.0/", true, "GitHub MCPB packages must be release assets"},
// Invalid GitLab URLs (not release URLs)
{"invalid_gitlab_root", "https://gitlab.com/owner/repo", true, "GitLab MCPB packages must be release assets"},
{"invalid_gitlab_tree", "https://gitlab.com/owner/repo/-/tree/main", true, "GitLab MCPB packages must be release assets"},
{"invalid_gitlab_blob", "https://gitlab.com/owner/repo/-/blob/main/file.mcpb", true, "GitLab MCPB packages must be release assets"},
{"invalid_gitlab_missing_dash_prefix", "https://gitlab.com/owner/repo/releases/v1.0.0/downloads/file.mcpb", true, "GitLab MCPB packages must be release assets"},
{"invalid_gitlab_missing_downloads", "https://gitlab.com/owner/repo/-/releases/v1.0.0/file.mcpb", true, "GitLab MCPB packages must be release assets"},
{"invalid_gitlab_invalid_package_files", "https://gitlab.com/owner/repo/-/package_files/abc/download", true, "GitLab MCPB packages must be release assets"},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
server := apiv0.ServerJSON{
Name: "com.example/test-server",
Description: "Test server",
VersionDetail: model.VersionDetail{
Version: "1.0.0",
},
Packages: []model.Package{
{
RegistryType: model.RegistryTypeMCPB,
RegistryBaseURL: model.RegistryURLGitHub,
Identifier: tc.identifier,
},
},
Remotes: []model.Remote{
{
URL: "https://example.com/remote",
},
},
}
err := validators.ValidateServerJSON(&server)
if tc.expectError {
assert.Error(t, err)
assert.Contains(t, err.Error(), tc.errorMsg)
} else {
assert.NoError(t, err)
}
})
}
}