Add more validation for free-form server fields (#272)
<!-- Provide a brief summary of your changes --> The changes are intended to create validators for all the free-form fields. Please find the details of the validation of different field below. - Repository.Source: only github and gitlab is accepted. - Repository.Url - It should be a valid github or gitlab URL. Created regex for both. - Server.Name - Max 200 char can be there. Reverse domain name verification is part of another PR (not owned by me). - Package.Name - Max 200 char and no spaces. - Server.Description - Max 100 char. - Remote.Url - It should be valid URL. ## Motivation and Context <!-- Why is this change needed? What problem does it solve? --> The motivation was to make the code clean and add base code for more validator for different objects like Repository etc. https://github.com/modelcontextprotocol/registry/issues/97 ## How Has This Been Tested? It's tested via UTs. Added UT's for all the validations. ## Breaking Changes Yes ## 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) - [ ] New feature (non-breaking change which adds functionality) - [x] 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) - [ ] 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 Co-authored-by: Avish Porwal <avishporwal@microsoft.com> Co-authored-by: Adam Jones <adamj+git@anthropic.com>
This commit is contained in:
Avish Porwal
GitHub
parent
5fb6ee2982
commit
c3b72ef3a6
9 changed files with 484 additions and 12 deletions
|
|
@ -53,12 +53,16 @@
|
|||
"name": {
|
||||
"type": "string",
|
||||
"description": "Server name/identifier",
|
||||
"example": "io.modelcontextprotocol/filesystem"
|
||||
"example": "io.modelcontextprotocol/filesystem",
|
||||
"minLength": 1,
|
||||
"maxLength": 200
|
||||
},
|
||||
"description": {
|
||||
"type": "string",
|
||||
"description": "Human-readable description of the server's functionality",
|
||||
"example": "Node.js server implementing Model Context Protocol (MCP) for filesystem operations."
|
||||
"example": "Node.js server implementing Model Context Protocol (MCP) for filesystem operations.",
|
||||
"minLength": 1,
|
||||
"maxLength": 100
|
||||
},
|
||||
"status": {
|
||||
"type": "string",
|
||||
|
|
@ -95,7 +99,8 @@
|
|||
"version": {
|
||||
"type": "string",
|
||||
"description": "Package version",
|
||||
"example": "1.0.2"
|
||||
"example": "1.0.2",
|
||||
"minLength": 1
|
||||
},
|
||||
"runtime_hint": {
|
||||
"type": "string",
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import (
|
|||
"github.com/modelcontextprotocol/registry/internal/config"
|
||||
"github.com/modelcontextprotocol/registry/internal/model"
|
||||
"github.com/modelcontextprotocol/registry/internal/service"
|
||||
"github.com/modelcontextprotocol/registry/internal/validators"
|
||||
)
|
||||
|
||||
// PublishServerInput represents the input for publishing a server
|
||||
|
|
@ -63,6 +64,12 @@ func RegisterPublishEndpoint(api huma.API, registry service.RegistryService, cfg
|
|||
// Get server details from request body
|
||||
serverDetail := publishRequest.Server
|
||||
|
||||
// Validate the server detail
|
||||
validator := validators.NewObjectValidator()
|
||||
if err := validator.Validate(&serverDetail); err != nil {
|
||||
return nil, huma.Error400BadRequest(err.Error())
|
||||
}
|
||||
|
||||
// Verify that the token's repository matches the server being published
|
||||
if !jwtManager.HasPermission(serverDetail.Name, auth.PermissionActionPublish, claims.Permissions) {
|
||||
return nil, huma.Error403Forbidden("You do not have permission to publish this server")
|
||||
|
|
|
|||
|
|
@ -105,9 +105,9 @@ func TestPublishIntegration(t *testing.T) {
|
|||
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",
|
||||
Source: "example",
|
||||
ID: "test-mcp-server",
|
||||
URL: "https://github.com/example/test-server",
|
||||
Source: "github",
|
||||
ID: "example/test-server",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
|
|
@ -194,6 +194,11 @@ func TestPublishIntegration(t *testing.T) {
|
|||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/example/test-server",
|
||||
Source: "github",
|
||||
ID: "example/test-server",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -112,8 +112,8 @@ func TestPublishEndpoint(t *testing.T) {
|
|||
Name: "example/test-server",
|
||||
Description: "A test server without auth",
|
||||
Repository: model.Repository{
|
||||
URL: "https://example.com/test-server",
|
||||
Source: "example",
|
||||
URL: "https://github.com/example/test-server",
|
||||
Source: "github",
|
||||
ID: "example/test-server",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
|
|
@ -173,6 +173,11 @@ func TestPublishEndpoint(t *testing.T) {
|
|||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/example/test-server",
|
||||
Source: "github",
|
||||
ID: "example/test-server",
|
||||
},
|
||||
},
|
||||
},
|
||||
tokenClaims: &auth.JWTClaims{
|
||||
|
|
@ -194,6 +199,11 @@ func TestPublishEndpoint(t *testing.T) {
|
|||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/example/test-server",
|
||||
Source: "github",
|
||||
ID: "example/test-server",
|
||||
},
|
||||
},
|
||||
},
|
||||
tokenClaims: &auth.JWTClaims{
|
||||
|
|
|
|||
|
|
@ -100,7 +100,7 @@ type Package struct {
|
|||
// Remote represents a remote connection endpoint
|
||||
type Remote struct {
|
||||
TransportType string `json:"transport_type" bson:"transport_type"`
|
||||
URL string `json:"url" bson:"url"`
|
||||
URL string `json:"url" format:"uri" bson:"url"`
|
||||
Headers []KeyValueInput `json:"headers,omitempty" bson:"headers,omitempty"`
|
||||
}
|
||||
|
||||
|
|
@ -112,9 +112,9 @@ type VersionDetail struct {
|
|||
// ServerDetail represents complete server information as defined in the MCP spec (pure, no registry metadata)
|
||||
type ServerDetail struct {
|
||||
Schema string `json:"$schema,omitempty" bson:"$schema,omitempty"`
|
||||
Name string `json:"name" bson:"name"`
|
||||
Description string `json:"description" bson:"description"`
|
||||
Status ServerStatus `json:"status,omitempty" bson:"status,omitempty"`
|
||||
Name string `json:"name" minLength:"1" maxLength:"200" bson:"name"`
|
||||
Description string `json:"description" minLength:"1" maxLength:"100" bson:"description"`
|
||||
Status ServerStatus `json:"status,omitempty" minLength:"1" bson:"status,omitempty"`
|
||||
Repository Repository `json:"repository,omitempty" bson:"repository"`
|
||||
VersionDetail VersionDetail `json:"version_detail" bson:"version_detail"`
|
||||
Packages []Package `json:"packages,omitempty" bson:"packages,omitempty"`
|
||||
|
|
|
|||
23
internal/validators/constants.go
Normal file
23
internal/validators/constants.go
Normal file
|
|
@ -0,0 +1,23 @@
|
|||
package validators
|
||||
|
||||
import "errors"
|
||||
|
||||
// Error messages for validation
|
||||
var (
|
||||
// Repository validation errors
|
||||
ErrInvalidRepositoryURL = errors.New("invalid repository URL")
|
||||
|
||||
// Package validation errors
|
||||
ErrPackageNameHasSpaces = errors.New("package name cannot contain spaces")
|
||||
|
||||
// Remote validation errors
|
||||
ErrInvalidRemoteURL = errors.New("invalid remote URL")
|
||||
)
|
||||
|
||||
// RepositorySource represents valid repository sources
|
||||
type RepositorySource string
|
||||
|
||||
const (
|
||||
SourceGitHub RepositorySource = "github"
|
||||
SourceGitLab RepositorySource = "gitlab"
|
||||
)
|
||||
50
internal/validators/utils.go
Normal file
50
internal/validators/utils.go
Normal file
|
|
@ -0,0 +1,50 @@
|
|||
package validators
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
"regexp"
|
||||
"strings"
|
||||
)
|
||||
|
||||
var (
|
||||
// Regular expressions for validating repository URLs
|
||||
// These regex patterns ensure the URL is in the format of a valid GitHub or GitLab repository
|
||||
// For example: // - GitHub: https://github.com/user/repo
|
||||
githubURLRegex = regexp.MustCompile(`^https?://(www\.)?github\.com/[\w.-]+/[\w.-]+/?$`)
|
||||
gitlabURLRegex = regexp.MustCompile(`^https?://(www\.)?gitlab\.com/[\w.-]+/[\w.-]+/?$`)
|
||||
)
|
||||
|
||||
// IsValidRepositoryURL checks if the given URL is valid for the specified repository source
|
||||
func IsValidRepositoryURL(source RepositorySource, url string) bool {
|
||||
switch source {
|
||||
case SourceGitHub:
|
||||
return githubURLRegex.MatchString(url)
|
||||
case SourceGitLab:
|
||||
return gitlabURLRegex.MatchString(url)
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// HasNoSpaces checks if a string contains no spaces
|
||||
func HasNoSpaces(s string) bool {
|
||||
return !strings.Contains(s, " ")
|
||||
}
|
||||
|
||||
// IsValidURL checks if a URL is in valid format
|
||||
func IsValidURL(rawURL string) bool {
|
||||
// Parse the URL
|
||||
u, err := url.Parse(rawURL)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
|
||||
// Check if scheme is present (http or https)
|
||||
if u.Scheme != "http" && u.Scheme != "https" {
|
||||
return false
|
||||
}
|
||||
|
||||
if u.Host == "" {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
124
internal/validators/validators.go
Normal file
124
internal/validators/validators.go
Normal file
|
|
@ -0,0 +1,124 @@
|
|||
package validators
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
|
||||
"github.com/modelcontextprotocol/registry/internal/model"
|
||||
)
|
||||
|
||||
// ServerValidator validates server details
|
||||
type ServerValidator struct {
|
||||
*RepositoryValidator // Embedded RepositoryValidator for repository validation
|
||||
}
|
||||
|
||||
// Validate checks if the server details are valid
|
||||
func (v *ServerValidator) Validate(obj *model.ServerDetail) error {
|
||||
if err := v.RepositoryValidator.Validate(&obj.Repository); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// NewServerValidator creates a new ServerValidator instance
|
||||
func NewServerValidator() *ServerValidator {
|
||||
return &ServerValidator{
|
||||
RepositoryValidator: NewRepositoryValidator(),
|
||||
}
|
||||
}
|
||||
|
||||
// RepositoryValidator validates repository details
|
||||
type RepositoryValidator struct {
|
||||
validSources map[RepositorySource]bool
|
||||
}
|
||||
|
||||
// Validate checks if the repository details are valid
|
||||
func (rv *RepositoryValidator) Validate(obj *model.Repository) error {
|
||||
// Skip validation for empty repository (optional field)
|
||||
if obj.URL == "" && obj.Source == "" {
|
||||
return nil
|
||||
}
|
||||
|
||||
// validate the repository source
|
||||
repoSource := RepositorySource(obj.Source)
|
||||
if !IsValidRepositoryURL(repoSource, obj.URL) {
|
||||
return fmt.Errorf("%w: %s", ErrInvalidRepositoryURL, obj.URL)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// NewRepositoryValidator creates a new RepositoryValidator instance
|
||||
func NewRepositoryValidator() *RepositoryValidator {
|
||||
return &RepositoryValidator{
|
||||
validSources: map[RepositorySource]bool{SourceGitHub: true, SourceGitLab: true},
|
||||
}
|
||||
}
|
||||
|
||||
// PackageValidator validates package details
|
||||
type PackageValidator struct{}
|
||||
|
||||
// Validate checks if the package details are valid
|
||||
func (pv *PackageValidator) Validate(obj *model.Package) error {
|
||||
if !HasNoSpaces(obj.Name) {
|
||||
return ErrPackageNameHasSpaces
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// NewPackageValidator creates a new PackageValidator instance
|
||||
func NewPackageValidator() *PackageValidator {
|
||||
return &PackageValidator{}
|
||||
}
|
||||
|
||||
// RemoteValidator validates remote connection details
|
||||
type RemoteValidator struct{}
|
||||
|
||||
// Validate checks if the remote connection details are valid
|
||||
func (rv *RemoteValidator) Validate(obj *model.Remote) error {
|
||||
if !IsValidURL(obj.URL) {
|
||||
return fmt.Errorf("%w: %s", ErrInvalidRemoteURL, obj.URL)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// NewRemoteValidator creates a new RemoteValidator instance
|
||||
func NewRemoteValidator() *RemoteValidator {
|
||||
return &RemoteValidator{}
|
||||
}
|
||||
|
||||
// ObjectValidator aggregates multiple validators for different object types
|
||||
// This allows for a single entry point to validate complex objects that may contain multiple fields
|
||||
// that need validation.
|
||||
type ObjectValidator struct {
|
||||
ServerValidator *ServerValidator
|
||||
PackageValidator *PackageValidator
|
||||
RemoteValidator *RemoteValidator
|
||||
}
|
||||
|
||||
func NewObjectValidator() *ObjectValidator {
|
||||
return &ObjectValidator{
|
||||
ServerValidator: NewServerValidator(),
|
||||
PackageValidator: NewPackageValidator(),
|
||||
RemoteValidator: NewRemoteValidator(),
|
||||
}
|
||||
}
|
||||
|
||||
func (ov *ObjectValidator) Validate(obj *model.ServerDetail) error {
|
||||
if err := ov.ServerValidator.Validate(obj); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
for _, pkg := range obj.Packages {
|
||||
if err := ov.PackageValidator.Validate(&pkg); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
for _, remote := range obj.Remotes {
|
||||
if err := ov.RemoteValidator.Validate(&remote); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
248
internal/validators/validators_test.go
Normal file
248
internal/validators/validators_test.go
Normal file
|
|
@ -0,0 +1,248 @@
|
|||
package validators_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/modelcontextprotocol/registry/internal/model"
|
||||
"github.com/modelcontextprotocol/registry/internal/validators"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestObjectValidator_Validate(t *testing.T) {
|
||||
validator := validators.NewObjectValidator()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
serverDetail model.ServerDetail
|
||||
expectedError string
|
||||
}{
|
||||
{
|
||||
name: "valid server detail with all fields",
|
||||
serverDetail: model.ServerDetail{
|
||||
Name: "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{
|
||||
{
|
||||
Name: "test-package",
|
||||
RegistryName: "npm",
|
||||
},
|
||||
},
|
||||
Remotes: []model.Remote{
|
||||
{
|
||||
URL: "https://example.com/remote",
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: "",
|
||||
},
|
||||
{
|
||||
name: "server with invalid repository source",
|
||||
serverDetail: model.ServerDetail{
|
||||
Name: "test-server",
|
||||
Description: "A test server",
|
||||
Repository: model.Repository{
|
||||
URL: "https://bitbucket.org/owner/repo",
|
||||
Source: "bitbucket", // Not in validSources
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
},
|
||||
expectedError: validators.ErrInvalidRepositoryURL.Error(),
|
||||
},
|
||||
{
|
||||
name: "server with invalid GitHub URL format",
|
||||
serverDetail: model.ServerDetail{
|
||||
Name: "test-server",
|
||||
Description: "A test server",
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/owner", // Missing repo name
|
||||
Source: "github",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
},
|
||||
expectedError: validators.ErrInvalidRepositoryURL.Error(),
|
||||
},
|
||||
{
|
||||
name: "server with invalid GitLab URL format",
|
||||
serverDetail: model.ServerDetail{
|
||||
Name: "test-server",
|
||||
Description: "A test server",
|
||||
Repository: model.Repository{
|
||||
URL: "https://gitlab.com", // Missing owner and repo
|
||||
Source: "gitlab",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
},
|
||||
expectedError: validators.ErrInvalidRepositoryURL.Error(),
|
||||
},
|
||||
{
|
||||
name: "package with spaces in name",
|
||||
serverDetail: model.ServerDetail{
|
||||
Name: "test-server",
|
||||
Description: "A test server",
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/owner/repo",
|
||||
Source: "github",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
Packages: []model.Package{
|
||||
{
|
||||
Name: "test package with spaces",
|
||||
RegistryName: "npm",
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: validators.ErrPackageNameHasSpaces.Error(),
|
||||
},
|
||||
{
|
||||
name: "multiple packages with one invalid",
|
||||
serverDetail: model.ServerDetail{
|
||||
Name: "test-server",
|
||||
Description: "A test server",
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/owner/repo",
|
||||
Source: "github",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
Packages: []model.Package{
|
||||
{
|
||||
Name: "valid-package",
|
||||
RegistryName: "npm",
|
||||
},
|
||||
{
|
||||
Name: "invalid package", // Has space
|
||||
RegistryName: "pip",
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: validators.ErrPackageNameHasSpaces.Error(),
|
||||
},
|
||||
{
|
||||
name: "remote with invalid URL",
|
||||
serverDetail: model.ServerDetail{
|
||||
Name: "test-server",
|
||||
Description: "A test server",
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/owner/repo",
|
||||
Source: "github",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
Remotes: []model.Remote{
|
||||
{
|
||||
URL: "not-a-valid-url",
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: validators.ErrInvalidRemoteURL.Error(),
|
||||
},
|
||||
{
|
||||
name: "remote with missing scheme",
|
||||
serverDetail: model.ServerDetail{
|
||||
Name: "test-server",
|
||||
Description: "A test server",
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/owner/repo",
|
||||
Source: "github",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
Remotes: []model.Remote{
|
||||
{
|
||||
URL: "example.com/remote",
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: validators.ErrInvalidRemoteURL.Error(),
|
||||
},
|
||||
{
|
||||
name: "multiple remotes with one invalid",
|
||||
serverDetail: model.ServerDetail{
|
||||
Name: "test-server",
|
||||
Description: "A test server",
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/owner/repo",
|
||||
Source: "github",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
Remotes: []model.Remote{
|
||||
{
|
||||
URL: "https://valid.com/remote",
|
||||
},
|
||||
{
|
||||
URL: "invalid-url",
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: validators.ErrInvalidRemoteURL.Error(),
|
||||
},
|
||||
{
|
||||
name: "server detail with nil packages and remotes",
|
||||
serverDetail: model.ServerDetail{
|
||||
Name: "test-server",
|
||||
Description: "A test server",
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/owner/repo",
|
||||
Source: "github",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
Packages: nil,
|
||||
Remotes: nil,
|
||||
},
|
||||
expectedError: "",
|
||||
},
|
||||
{
|
||||
name: "server detail with empty packages and remotes slices",
|
||||
serverDetail: model.ServerDetail{
|
||||
Name: "test-server",
|
||||
Description: "A test server",
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/owner/repo",
|
||||
Source: "github",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
Packages: []model.Package{},
|
||||
Remotes: []model.Remote{},
|
||||
},
|
||||
expectedError: "",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
err := validator.Validate(&tt.serverDetail)
|
||||
|
||||
if tt.expectedError == "" {
|
||||
assert.NoError(t, err)
|
||||
} else {
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), tt.expectedError)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue