From 10350ea01a8fc570678550af25e0efd0a982a06b Mon Sep 17 00:00:00 2001 From: Ossama Lafhel Date: Sun, 14 Sep 2025 13:41:03 +0200 Subject: [PATCH] fix: validate server names to allow only single slash (#471) --- internal/api/handlers/v0/publish_test.go | 230 +++++++++++++++++++++++ internal/validators/constants.go | 5 +- internal/validators/validators.go | 8 +- internal/validators/validators_test.go | 104 +++++++++- 4 files changed, 342 insertions(+), 5 deletions(-) diff --git a/internal/api/handlers/v0/publish_test.go b/internal/api/handlers/v0/publish_test.go index 61932769..1811301b 100644 --- a/internal/api/handlers/v0/publish_test.go +++ b/internal/api/handlers/v0/publish_test.go @@ -224,6 +224,134 @@ func TestPublishEndpoint(t *testing.T) { setupRegistryService: func(_ service.RegistryService) {}, 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 { @@ -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) + } + }) + } +} \ No newline at end of file diff --git a/internal/validators/constants.go b/internal/validators/constants.go index e7a36ea1..b7279dd9 100644 --- a/internal/validators/constants.go +++ b/internal/validators/constants.go @@ -25,6 +25,9 @@ var ( ErrInvalidNamedArgumentName = errors.New("invalid named argument name format") ErrArgumentValueStartsWithName = errors.New("argument value 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 @@ -33,4 +36,4 @@ type RepositorySource string const ( SourceGitHub RepositorySource = "github" SourceGitLab RepositorySource = "gitlab" -) +) \ No newline at end of file diff --git a/internal/validators/validators.go b/internal/validators/validators.go index 95f73ba2..4ce45acf 100644 --- a/internal/validators/validators.go +++ b/internal/validators/validators.go @@ -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')") } + // Check for multiple slashes - reject if found + slashCount := strings.Count(name, "/") + if slashCount > 1 { + return "", ErrMultipleSlashesInServerName + } + 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") @@ -504,4 +510,4 @@ func isValidHostForDomain(hostname, publisherDomain string) bool { } return false -} +} \ No newline at end of file diff --git a/internal/validators/validators_test.go b/internal/validators/validators_test.go index ea2b65af..04133524 100644 --- a/internal/validators/validators_test.go +++ b/internal/validators/validators_test.go @@ -171,6 +171,25 @@ func TestValidate(t *testing.T) { }, 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", serverDetail: apiv0.ServerJSON{ @@ -847,11 +866,12 @@ func TestValidate_ServerNameFormat(t *testing.T) { errorMsg: "non-empty namespace and name parts", }, { - name: "multiple slashes - uses first as separator", + name: "multiple slashes - should be rejected", serverDetail: apiv0.ServerJSON{ 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) { validCases := []model.Argument{ { @@ -1492,4 +1590,4 @@ func createValidServerWithArgument(arg model.Argument) apiv0.ServerJSON { }, }, } -} +} \ No newline at end of file