Skip to content

Commit 63cf08e

Browse files
authored
fix: validate server names to allow only single slash (#471) (#476)
## 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"
1 parent b77e382 commit 63cf08e

File tree

4 files changed

+342
-5
lines changed

4 files changed

+342
-5
lines changed

internal/api/handlers/v0/publish_test.go

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,134 @@ func TestPublishEndpoint(t *testing.T) {
224224
setupRegistryService: func(_ service.RegistryService) {},
225225
expectedStatus: http.StatusOK,
226226
},
227+
{
228+
name: "invalid server name - multiple slashes (two slashes)",
229+
requestBody: apiv0.ServerJSON{
230+
Name: "com.example/server/path",
231+
Description: "Server with multiple slashes in name",
232+
Version: "1.0.0",
233+
Repository: model.Repository{
234+
URL: "https://github.com/example/test-server",
235+
Source: "github",
236+
ID: "example/test-server",
237+
},
238+
},
239+
tokenClaims: &auth.JWTClaims{
240+
AuthMethod: auth.MethodNone,
241+
Permissions: []auth.Permission{
242+
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
243+
},
244+
},
245+
setupRegistryService: func(_ service.RegistryService) {},
246+
expectedStatus: http.StatusBadRequest,
247+
expectedError: "server name cannot contain multiple slashes",
248+
},
249+
{
250+
name: "invalid server name - multiple slashes (three slashes)",
251+
requestBody: apiv0.ServerJSON{
252+
Name: "org.company/dept/team/project",
253+
Description: "Server with three slashes in name",
254+
Version: "1.0.0",
255+
},
256+
tokenClaims: &auth.JWTClaims{
257+
AuthMethod: auth.MethodNone,
258+
Permissions: []auth.Permission{
259+
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
260+
},
261+
},
262+
setupRegistryService: func(_ service.RegistryService) {},
263+
expectedStatus: http.StatusBadRequest,
264+
expectedError: "server name cannot contain multiple slashes",
265+
},
266+
{
267+
name: "invalid server name - consecutive slashes",
268+
requestBody: apiv0.ServerJSON{
269+
Name: "com.example//double-slash",
270+
Description: "Server with consecutive slashes",
271+
Version: "1.0.0",
272+
},
273+
tokenClaims: &auth.JWTClaims{
274+
AuthMethod: auth.MethodNone,
275+
Permissions: []auth.Permission{
276+
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
277+
},
278+
},
279+
setupRegistryService: func(_ service.RegistryService) {},
280+
expectedStatus: http.StatusBadRequest,
281+
expectedError: "server name cannot contain multiple slashes",
282+
},
283+
{
284+
name: "invalid server name - URL-like path",
285+
requestBody: apiv0.ServerJSON{
286+
Name: "com.example/servers/v1/api",
287+
Description: "Server with URL-like path structure",
288+
Version: "1.0.0",
289+
},
290+
tokenClaims: &auth.JWTClaims{
291+
AuthMethod: auth.MethodNone,
292+
Permissions: []auth.Permission{
293+
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
294+
},
295+
},
296+
setupRegistryService: func(_ service.RegistryService) {},
297+
expectedStatus: http.StatusBadRequest,
298+
expectedError: "server name cannot contain multiple slashes",
299+
},
300+
{
301+
name: "invalid server name - many slashes",
302+
requestBody: apiv0.ServerJSON{
303+
Name: "a/b/c/d/e/f",
304+
Description: "Server with many slashes",
305+
Version: "1.0.0",
306+
},
307+
tokenClaims: &auth.JWTClaims{
308+
AuthMethod: auth.MethodNone,
309+
Permissions: []auth.Permission{
310+
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
311+
},
312+
},
313+
setupRegistryService: func(_ service.RegistryService) {},
314+
expectedStatus: http.StatusBadRequest,
315+
expectedError: "server name cannot contain multiple slashes",
316+
},
317+
{
318+
name: "invalid server name - with packages and remotes",
319+
requestBody: apiv0.ServerJSON{
320+
Name: "com.example/test/server/v2",
321+
Description: "Complex server with invalid name",
322+
Version: "2.0.0",
323+
Repository: model.Repository{
324+
URL: "https://github.com/example/test-server",
325+
Source: "github",
326+
ID: "example/test-server",
327+
},
328+
Packages: []model.Package{
329+
{
330+
RegistryType: model.RegistryTypeNPM,
331+
Identifier: "test-package",
332+
Version: "2.0.0",
333+
Transport: model.Transport{
334+
Type: model.TransportTypeStdio,
335+
},
336+
},
337+
},
338+
Remotes: []model.Transport{
339+
{
340+
Type: model.TransportTypeStreamableHTTP,
341+
URL: "https://example.com/api",
342+
},
343+
},
344+
},
345+
tokenClaims: &auth.JWTClaims{
346+
AuthMethod: auth.MethodNone,
347+
Permissions: []auth.Permission{
348+
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
349+
},
350+
},
351+
setupRegistryService: func(_ service.RegistryService) {},
352+
expectedStatus: http.StatusBadRequest,
353+
expectedError: "server name cannot contain multiple slashes",
354+
},
227355
}
228356

229357
for _, tc := range testCases {
@@ -279,3 +407,105 @@ func TestPublishEndpoint(t *testing.T) {
279407
})
280408
}
281409
}
410+
411+
// TestPublishEndpoint_MultipleSlashesEdgeCases tests additional edge cases for multi-slash validation
412+
func TestPublishEndpoint_MultipleSlashesEdgeCases(t *testing.T) {
413+
testSeed := make([]byte, ed25519.SeedSize)
414+
_, err := rand.Read(testSeed)
415+
require.NoError(t, err)
416+
testConfig := &config.Config{
417+
JWTPrivateKey: hex.EncodeToString(testSeed),
418+
EnableRegistryValidation: false,
419+
}
420+
421+
testCases := []struct {
422+
name string
423+
serverName string
424+
expectedStatus int
425+
description string
426+
}{
427+
{
428+
name: "valid - single slash",
429+
serverName: "com.example/server",
430+
expectedStatus: http.StatusOK,
431+
description: "Valid server name with single slash should succeed",
432+
},
433+
{
434+
name: "invalid - trailing slash after valid name",
435+
serverName: "com.example/server/",
436+
expectedStatus: http.StatusBadRequest,
437+
description: "Trailing slash creates multiple slashes",
438+
},
439+
{
440+
name: "invalid - leading and middle slash",
441+
serverName: "/com.example/server",
442+
expectedStatus: http.StatusBadRequest,
443+
description: "Leading slash with middle slash",
444+
},
445+
{
446+
name: "invalid - file system style path",
447+
serverName: "usr/local/bin/server",
448+
expectedStatus: http.StatusBadRequest,
449+
description: "File system style paths should be rejected",
450+
},
451+
{
452+
name: "invalid - version-like suffix",
453+
serverName: "com.example/server/v1.0.0",
454+
expectedStatus: http.StatusBadRequest,
455+
description: "Version suffixes with slash should be rejected",
456+
},
457+
}
458+
459+
for _, tc := range testCases {
460+
t.Run(tc.name, func(t *testing.T) {
461+
// Create registry service
462+
registryService := service.NewRegistryService(database.NewMemoryDB(), testConfig)
463+
464+
// Create a new ServeMux and Huma API
465+
mux := http.NewServeMux()
466+
api := humago.New(mux, huma.DefaultConfig("Test API", "1.0.0"))
467+
468+
// Register the endpoint
469+
v0.RegisterPublishEndpoint(api, registryService, testConfig)
470+
471+
// Create request body
472+
requestBody := apiv0.ServerJSON{
473+
Name: tc.serverName,
474+
Description: "Test server",
475+
Version: "1.0.0",
476+
}
477+
478+
bodyBytes, err := json.Marshal(requestBody)
479+
require.NoError(t, err)
480+
481+
// Create request
482+
req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "/v0/publish", bytes.NewBuffer(bodyBytes))
483+
require.NoError(t, err)
484+
req.Header.Set("Content-Type", "application/json")
485+
486+
// Set auth header with permissions
487+
tokenClaims := auth.JWTClaims{
488+
AuthMethod: auth.MethodNone,
489+
Permissions: []auth.Permission{
490+
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
491+
},
492+
}
493+
token, err := generateTestJWTToken(testConfig, tokenClaims)
494+
require.NoError(t, err)
495+
req.Header.Set("Authorization", "Bearer "+token)
496+
497+
// Perform request
498+
rr := httptest.NewRecorder()
499+
mux.ServeHTTP(rr, req)
500+
501+
// Assertions
502+
assert.Equal(t, tc.expectedStatus, rr.Code,
503+
"%s: expected status %d, got %d", tc.description, tc.expectedStatus, rr.Code)
504+
505+
if tc.expectedStatus == http.StatusBadRequest {
506+
assert.Contains(t, rr.Body.String(), "server name cannot contain multiple slashes",
507+
"%s: should contain specific error message", tc.description)
508+
}
509+
})
510+
}
511+
}

internal/validators/constants.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ var (
2525
ErrInvalidNamedArgumentName = errors.New("invalid named argument name format")
2626
ErrArgumentValueStartsWithName = errors.New("argument value cannot start with the argument name")
2727
ErrArgumentDefaultStartsWithName = errors.New("argument default cannot start with the argument name")
28+
29+
// Server name validation errors
30+
ErrMultipleSlashesInServerName = errors.New("server name cannot contain multiple slashes")
2831
)
2932

3033
// RepositorySource represents valid repository sources
@@ -33,4 +36,4 @@ type RepositorySource string
3336
const (
3437
SourceGitHub RepositorySource = "github"
3538
SourceGitLab RepositorySource = "gitlab"
36-
)
39+
)

internal/validators/validators.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,12 @@ func parseServerName(serverJSON apiv0.ServerJSON) (string, error) {
402402
return "", fmt.Errorf("server name must be in format 'dns-namespace/name' (e.g., 'com.example.api/server')")
403403
}
404404

405+
// Check for multiple slashes - reject if found
406+
slashCount := strings.Count(name, "/")
407+
if slashCount > 1 {
408+
return "", ErrMultipleSlashesInServerName
409+
}
410+
405411
parts := strings.SplitN(name, "/", 2)
406412
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
407413
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 {
504510
}
505511

506512
return false
507-
}
513+
}

0 commit comments

Comments
 (0)