## Summary Added validation to reject server names with multiple slashes, ensuring consistency between JSON schema pattern and API validation. - Count slashes in parseServerName() and reject if > 1 - Add ErrMultipleSlashesInServerName error constant - Add unit tests in validators_test.go - Add integration test in publish_test.go ## Motivation and Context Fixes #471. The JSON schema regex pattern `^[^/]+/[^/]+$` only allows a single slash in server names, but the API validation was not enforcing this constraint. This inconsistency allowed invalid server names like `com.example/server/path` to be published. ## How Has This Been Tested? - Unit tests added to verify slash validation logic - Integration tests added to verify the publish endpoint rejects multi-slash names - Tested various edge cases: trailing slashes, consecutive slashes, URL-like paths - All existing tests pass ## Breaking Changes None. This change only adds validation to reject previously invalid server names that shouldn't have been accepted. ## Types of changes - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist - [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 - [x] I have added or updated documentation as needed ## Additional context - The fix ensures server names follow the intended format: `namespace/name` (e.g., `com.example/my-server`) - This validation aligns the API behavior with the existing JSON schema regex pattern - Test coverage includes edge cases like `com.example//server`, `com.example/server/`, and `a/b/c/d/e/f` - The error message clearly indicates the issue: "server name cannot contain multiple slashes"
This commit is contained in:
parent
b77e382b98
commit
63cf08efad
4 changed files with 342 additions and 5 deletions
|
|
@ -224,6 +224,134 @@ func TestPublishEndpoint(t *testing.T) {
|
||||||
setupRegistryService: func(_ service.RegistryService) {},
|
setupRegistryService: func(_ service.RegistryService) {},
|
||||||
expectedStatus: http.StatusOK,
|
expectedStatus: http.StatusOK,
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "invalid server name - multiple slashes (two slashes)",
|
||||||
|
requestBody: apiv0.ServerJSON{
|
||||||
|
Name: "com.example/server/path",
|
||||||
|
Description: "Server with multiple slashes in name",
|
||||||
|
Version: "1.0.0",
|
||||||
|
Repository: model.Repository{
|
||||||
|
URL: "https://github.com/example/test-server",
|
||||||
|
Source: "github",
|
||||||
|
ID: "example/test-server",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
tokenClaims: &auth.JWTClaims{
|
||||||
|
AuthMethod: auth.MethodNone,
|
||||||
|
Permissions: []auth.Permission{
|
||||||
|
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
setupRegistryService: func(_ service.RegistryService) {},
|
||||||
|
expectedStatus: http.StatusBadRequest,
|
||||||
|
expectedError: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "invalid server name - multiple slashes (three slashes)",
|
||||||
|
requestBody: apiv0.ServerJSON{
|
||||||
|
Name: "org.company/dept/team/project",
|
||||||
|
Description: "Server with three slashes in name",
|
||||||
|
Version: "1.0.0",
|
||||||
|
},
|
||||||
|
tokenClaims: &auth.JWTClaims{
|
||||||
|
AuthMethod: auth.MethodNone,
|
||||||
|
Permissions: []auth.Permission{
|
||||||
|
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
setupRegistryService: func(_ service.RegistryService) {},
|
||||||
|
expectedStatus: http.StatusBadRequest,
|
||||||
|
expectedError: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "invalid server name - consecutive slashes",
|
||||||
|
requestBody: apiv0.ServerJSON{
|
||||||
|
Name: "com.example//double-slash",
|
||||||
|
Description: "Server with consecutive slashes",
|
||||||
|
Version: "1.0.0",
|
||||||
|
},
|
||||||
|
tokenClaims: &auth.JWTClaims{
|
||||||
|
AuthMethod: auth.MethodNone,
|
||||||
|
Permissions: []auth.Permission{
|
||||||
|
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
setupRegistryService: func(_ service.RegistryService) {},
|
||||||
|
expectedStatus: http.StatusBadRequest,
|
||||||
|
expectedError: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "invalid server name - URL-like path",
|
||||||
|
requestBody: apiv0.ServerJSON{
|
||||||
|
Name: "com.example/servers/v1/api",
|
||||||
|
Description: "Server with URL-like path structure",
|
||||||
|
Version: "1.0.0",
|
||||||
|
},
|
||||||
|
tokenClaims: &auth.JWTClaims{
|
||||||
|
AuthMethod: auth.MethodNone,
|
||||||
|
Permissions: []auth.Permission{
|
||||||
|
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
setupRegistryService: func(_ service.RegistryService) {},
|
||||||
|
expectedStatus: http.StatusBadRequest,
|
||||||
|
expectedError: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "invalid server name - many slashes",
|
||||||
|
requestBody: apiv0.ServerJSON{
|
||||||
|
Name: "a/b/c/d/e/f",
|
||||||
|
Description: "Server with many slashes",
|
||||||
|
Version: "1.0.0",
|
||||||
|
},
|
||||||
|
tokenClaims: &auth.JWTClaims{
|
||||||
|
AuthMethod: auth.MethodNone,
|
||||||
|
Permissions: []auth.Permission{
|
||||||
|
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
setupRegistryService: func(_ service.RegistryService) {},
|
||||||
|
expectedStatus: http.StatusBadRequest,
|
||||||
|
expectedError: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "invalid server name - with packages and remotes",
|
||||||
|
requestBody: apiv0.ServerJSON{
|
||||||
|
Name: "com.example/test/server/v2",
|
||||||
|
Description: "Complex server with invalid name",
|
||||||
|
Version: "2.0.0",
|
||||||
|
Repository: model.Repository{
|
||||||
|
URL: "https://github.com/example/test-server",
|
||||||
|
Source: "github",
|
||||||
|
ID: "example/test-server",
|
||||||
|
},
|
||||||
|
Packages: []model.Package{
|
||||||
|
{
|
||||||
|
RegistryType: model.RegistryTypeNPM,
|
||||||
|
Identifier: "test-package",
|
||||||
|
Version: "2.0.0",
|
||||||
|
Transport: model.Transport{
|
||||||
|
Type: model.TransportTypeStdio,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Remotes: []model.Transport{
|
||||||
|
{
|
||||||
|
Type: model.TransportTypeStreamableHTTP,
|
||||||
|
URL: "https://example.com/api",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
tokenClaims: &auth.JWTClaims{
|
||||||
|
AuthMethod: auth.MethodNone,
|
||||||
|
Permissions: []auth.Permission{
|
||||||
|
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
setupRegistryService: func(_ service.RegistryService) {},
|
||||||
|
expectedStatus: http.StatusBadRequest,
|
||||||
|
expectedError: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tc := range testCases {
|
for _, tc := range testCases {
|
||||||
|
|
@ -279,3 +407,105 @@ func TestPublishEndpoint(t *testing.T) {
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestPublishEndpoint_MultipleSlashesEdgeCases tests additional edge cases for multi-slash validation
|
||||||
|
func TestPublishEndpoint_MultipleSlashesEdgeCases(t *testing.T) {
|
||||||
|
testSeed := make([]byte, ed25519.SeedSize)
|
||||||
|
_, err := rand.Read(testSeed)
|
||||||
|
require.NoError(t, err)
|
||||||
|
testConfig := &config.Config{
|
||||||
|
JWTPrivateKey: hex.EncodeToString(testSeed),
|
||||||
|
EnableRegistryValidation: false,
|
||||||
|
}
|
||||||
|
|
||||||
|
testCases := []struct {
|
||||||
|
name string
|
||||||
|
serverName string
|
||||||
|
expectedStatus int
|
||||||
|
description string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "valid - single slash",
|
||||||
|
serverName: "com.example/server",
|
||||||
|
expectedStatus: http.StatusOK,
|
||||||
|
description: "Valid server name with single slash should succeed",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "invalid - trailing slash after valid name",
|
||||||
|
serverName: "com.example/server/",
|
||||||
|
expectedStatus: http.StatusBadRequest,
|
||||||
|
description: "Trailing slash creates multiple slashes",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "invalid - leading and middle slash",
|
||||||
|
serverName: "/com.example/server",
|
||||||
|
expectedStatus: http.StatusBadRequest,
|
||||||
|
description: "Leading slash with middle slash",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "invalid - file system style path",
|
||||||
|
serverName: "usr/local/bin/server",
|
||||||
|
expectedStatus: http.StatusBadRequest,
|
||||||
|
description: "File system style paths should be rejected",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "invalid - version-like suffix",
|
||||||
|
serverName: "com.example/server/v1.0.0",
|
||||||
|
expectedStatus: http.StatusBadRequest,
|
||||||
|
description: "Version suffixes with slash should be rejected",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range testCases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
// Create registry service
|
||||||
|
registryService := service.NewRegistryService(database.NewMemoryDB(), testConfig)
|
||||||
|
|
||||||
|
// Create a new ServeMux and Huma API
|
||||||
|
mux := http.NewServeMux()
|
||||||
|
api := humago.New(mux, huma.DefaultConfig("Test API", "1.0.0"))
|
||||||
|
|
||||||
|
// Register the endpoint
|
||||||
|
v0.RegisterPublishEndpoint(api, registryService, testConfig)
|
||||||
|
|
||||||
|
// Create request body
|
||||||
|
requestBody := apiv0.ServerJSON{
|
||||||
|
Name: tc.serverName,
|
||||||
|
Description: "Test server",
|
||||||
|
Version: "1.0.0",
|
||||||
|
}
|
||||||
|
|
||||||
|
bodyBytes, err := json.Marshal(requestBody)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
// Create request
|
||||||
|
req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "/v0/publish", bytes.NewBuffer(bodyBytes))
|
||||||
|
require.NoError(t, err)
|
||||||
|
req.Header.Set("Content-Type", "application/json")
|
||||||
|
|
||||||
|
// Set auth header with permissions
|
||||||
|
tokenClaims := auth.JWTClaims{
|
||||||
|
AuthMethod: auth.MethodNone,
|
||||||
|
Permissions: []auth.Permission{
|
||||||
|
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
token, err := generateTestJWTToken(testConfig, tokenClaims)
|
||||||
|
require.NoError(t, err)
|
||||||
|
req.Header.Set("Authorization", "Bearer "+token)
|
||||||
|
|
||||||
|
// Perform request
|
||||||
|
rr := httptest.NewRecorder()
|
||||||
|
mux.ServeHTTP(rr, req)
|
||||||
|
|
||||||
|
// Assertions
|
||||||
|
assert.Equal(t, tc.expectedStatus, rr.Code,
|
||||||
|
"%s: expected status %d, got %d", tc.description, tc.expectedStatus, rr.Code)
|
||||||
|
|
||||||
|
if tc.expectedStatus == http.StatusBadRequest {
|
||||||
|
assert.Contains(t, rr.Body.String(), "server name cannot contain multiple slashes",
|
||||||
|
"%s: should contain specific error message", tc.description)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -25,6 +25,9 @@ var (
|
||||||
ErrInvalidNamedArgumentName = errors.New("invalid named argument name format")
|
ErrInvalidNamedArgumentName = errors.New("invalid named argument name format")
|
||||||
ErrArgumentValueStartsWithName = errors.New("argument value cannot start with the argument name")
|
ErrArgumentValueStartsWithName = errors.New("argument value cannot start with the argument name")
|
||||||
ErrArgumentDefaultStartsWithName = errors.New("argument default cannot start with the argument name")
|
ErrArgumentDefaultStartsWithName = errors.New("argument default cannot start with the argument name")
|
||||||
|
|
||||||
|
// Server name validation errors
|
||||||
|
ErrMultipleSlashesInServerName = errors.New("server name cannot contain multiple slashes")
|
||||||
)
|
)
|
||||||
|
|
||||||
// RepositorySource represents valid repository sources
|
// RepositorySource represents valid repository sources
|
||||||
|
|
@ -33,4 +36,4 @@ type RepositorySource string
|
||||||
const (
|
const (
|
||||||
SourceGitHub RepositorySource = "github"
|
SourceGitHub RepositorySource = "github"
|
||||||
SourceGitLab RepositorySource = "gitlab"
|
SourceGitLab RepositorySource = "gitlab"
|
||||||
)
|
)
|
||||||
|
|
@ -402,6 +402,12 @@ func parseServerName(serverJSON apiv0.ServerJSON) (string, error) {
|
||||||
return "", fmt.Errorf("server name must be in format 'dns-namespace/name' (e.g., 'com.example.api/server')")
|
return "", fmt.Errorf("server name must be in format 'dns-namespace/name' (e.g., 'com.example.api/server')")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check for multiple slashes - reject if found
|
||||||
|
slashCount := strings.Count(name, "/")
|
||||||
|
if slashCount > 1 {
|
||||||
|
return "", ErrMultipleSlashesInServerName
|
||||||
|
}
|
||||||
|
|
||||||
parts := strings.SplitN(name, "/", 2)
|
parts := strings.SplitN(name, "/", 2)
|
||||||
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
|
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 "", fmt.Errorf("server name must be in format 'dns-namespace/name' with non-empty namespace and name parts")
|
||||||
|
|
@ -504,4 +510,4 @@ func isValidHostForDomain(hostname, publisherDomain string) bool {
|
||||||
}
|
}
|
||||||
|
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
@ -171,6 +171,25 @@ func TestValidate(t *testing.T) {
|
||||||
},
|
},
|
||||||
expectedError: validators.ErrVersionLooksLikeRange.Error(),
|
expectedError: validators.ErrVersionLooksLikeRange.Error(),
|
||||||
},
|
},
|
||||||
|
// Server name validation - multiple slashes
|
||||||
|
{
|
||||||
|
name: "server name with two slashes",
|
||||||
|
serverDetail: apiv0.ServerJSON{
|
||||||
|
Name: "com.example/server/path",
|
||||||
|
Description: "A test server",
|
||||||
|
Version: "1.0.0",
|
||||||
|
},
|
||||||
|
expectedError: validators.ErrMultipleSlashesInServerName.Error(),
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "server name with three slashes",
|
||||||
|
serverDetail: apiv0.ServerJSON{
|
||||||
|
Name: "com.example/server/path/deep",
|
||||||
|
Description: "A test server",
|
||||||
|
Version: "1.0.0",
|
||||||
|
},
|
||||||
|
expectedError: validators.ErrMultipleSlashesInServerName.Error(),
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "valid server detail with all fields",
|
name: "valid server detail with all fields",
|
||||||
serverDetail: apiv0.ServerJSON{
|
serverDetail: apiv0.ServerJSON{
|
||||||
|
|
@ -847,11 +866,12 @@ func TestValidate_ServerNameFormat(t *testing.T) {
|
||||||
errorMsg: "non-empty namespace and name parts",
|
errorMsg: "non-empty namespace and name parts",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "multiple slashes - uses first as separator",
|
name: "multiple slashes - should be rejected",
|
||||||
serverDetail: apiv0.ServerJSON{
|
serverDetail: apiv0.ServerJSON{
|
||||||
Name: "com.example/server/path",
|
Name: "com.example/server/path",
|
||||||
},
|
},
|
||||||
expectError: false,
|
expectError: true,
|
||||||
|
errorMsg: "server name cannot contain multiple slashes",
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -869,6 +889,84 @@ func TestValidate_ServerNameFormat(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestValidate_MultipleSlashesInServerName(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
serverName string
|
||||||
|
expectError bool
|
||||||
|
errorMsg string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "single slash - valid",
|
||||||
|
serverName: "com.example/my-server",
|
||||||
|
expectError: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "two slashes - invalid",
|
||||||
|
serverName: "com.example/my-server/extra",
|
||||||
|
expectError: true,
|
||||||
|
errorMsg: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "three slashes - invalid",
|
||||||
|
serverName: "com.example/my/server/name",
|
||||||
|
expectError: true,
|
||||||
|
errorMsg: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "many slashes - invalid",
|
||||||
|
serverName: "com.example/a/b/c/d/e",
|
||||||
|
expectError: true,
|
||||||
|
errorMsg: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "double slash - invalid",
|
||||||
|
serverName: "com.example//server",
|
||||||
|
expectError: true,
|
||||||
|
errorMsg: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "trailing slash counts as two - invalid",
|
||||||
|
serverName: "com.example/server/",
|
||||||
|
expectError: true,
|
||||||
|
errorMsg: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "no slash - still invalid for different reason",
|
||||||
|
serverName: "com.example.server",
|
||||||
|
expectError: true,
|
||||||
|
errorMsg: "server name must be in format 'dns-namespace/name'",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "complex valid namespace with single slash",
|
||||||
|
serverName: "com.microsoft.azure.service/webapp-server",
|
||||||
|
expectError: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "complex namespace with multiple slashes - invalid",
|
||||||
|
serverName: "com.microsoft.azure/service/webapp-server",
|
||||||
|
expectError: true,
|
||||||
|
errorMsg: "server name cannot contain multiple slashes",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
serverDetail := apiv0.ServerJSON{
|
||||||
|
Name: tt.serverName,
|
||||||
|
}
|
||||||
|
err := validators.ValidateServerJSON(&serverDetail)
|
||||||
|
|
||||||
|
if tt.expectError {
|
||||||
|
assert.Error(t, err)
|
||||||
|
assert.Contains(t, err.Error(), tt.errorMsg)
|
||||||
|
} else {
|
||||||
|
assert.NoError(t, err)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestValidateArgument_ValidNamedArguments(t *testing.T) {
|
func TestValidateArgument_ValidNamedArguments(t *testing.T) {
|
||||||
validCases := []model.Argument{
|
validCases := []model.Argument{
|
||||||
{
|
{
|
||||||
|
|
@ -1492,4 +1590,4 @@ func createValidServerWithArgument(arg model.Argument) apiv0.ServerJSON {
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Loading…
Reference in a new issue