Skip to content

Commit 6d19849

Browse files
ossamalafhelAdam Jones
andcommitted
feat: add regex validation to server name in API types
- Add pattern validation (^[^/]+/[^/]+$) to ensure exactly one slash - Update tests to expect HTTP 422 for validation errors (correct status) - Remove unnecessary error handling complexity in publish handler - This makes the validation visible in API documentation Following @domdomegg's suggestion to align API specs with implementation. Co-Authored-By: Adam Jones <[email protected]>
1 parent 63cf08e commit 6d19849

File tree

7 files changed

+30
-16
lines changed

7 files changed

+30
-16
lines changed

internal/api/handlers/v0/edit_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,8 @@ func TestEditServerEndpoint(t *testing.T) {
236236
Version: "1.0.0",
237237
},
238238
serverID: testServerID,
239-
expectedStatus: http.StatusBadRequest,
240-
expectedError: "Bad Request",
239+
expectedStatus: http.StatusUnprocessableEntity,
240+
expectedError: "pattern",
241241
},
242242
{
243243
name: "cannot undelete server",

internal/api/handlers/v0/publish.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func RegisterPublishEndpoint(api huma.API, registry service.RegistryService, cfg
5656
// Publish the server with extensions
5757
publishedServer, err := registry.Publish(input.Body)
5858
if err != nil {
59-
return nil, huma.Error400BadRequest("Failed to publish server", err)
59+
return nil, huma.Error422UnprocessableEntity("Failed to publish server", err)
6060
}
6161

6262
// Return the published server in flattened format

internal/api/handlers/v0/publish_integration_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ func TestPublishIntegration(t *testing.T) {
141141

142142
t.Run("publish fails with missing authorization header", func(t *testing.T) {
143143
publishReq := apiv0.ServerJSON{
144-
Name: "test-server",
144+
Name: "com.example/test-server",
145145
}
146146

147147
body, err := json.Marshal(publishReq)

internal/api/handlers/v0/publish_registry_validation_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func TestPublishRegistryValidation(t *testing.T) {
8080
rr := httptest.NewRecorder()
8181
mux.ServeHTTP(rr, req)
8282

83-
assert.Equal(t, http.StatusBadRequest, rr.Code)
83+
assert.Equal(t, http.StatusUnprocessableEntity, rr.Code)
8484
assert.Contains(t, rr.Body.String(), "registry validation failed")
8585
})
8686

@@ -180,7 +180,7 @@ func TestPublishRegistryValidation(t *testing.T) {
180180
rr := httptest.NewRecorder()
181181
mux.ServeHTTP(rr, req)
182182

183-
assert.Equal(t, http.StatusBadRequest, rr.Code)
183+
assert.Equal(t, http.StatusUnprocessableEntity, rr.Code)
184184
assert.Contains(t, rr.Body.String(), "registry validation failed for package 1")
185185
assert.Contains(t, rr.Body.String(), "nonexistent-second-package-abc123")
186186
})
@@ -231,7 +231,7 @@ func TestPublishRegistryValidation(t *testing.T) {
231231
rr := httptest.NewRecorder()
232232
mux.ServeHTTP(rr, req)
233233

234-
assert.Equal(t, http.StatusBadRequest, rr.Code)
234+
assert.Equal(t, http.StatusUnprocessableEntity, rr.Code)
235235
assert.Contains(t, rr.Body.String(), "registry validation failed for package 0")
236236
assert.Contains(t, rr.Body.String(), "nonexistent-first-package-xyz789")
237237
})

internal/api/handlers/v0/publish_test.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func TestPublishEndpoint(t *testing.T) {
8080
{
8181
name: "successful publish with no auth (AuthMethodNone)",
8282
requestBody: apiv0.ServerJSON{
83-
Name: "example/test-server",
83+
Name: "com.example/test-server",
8484
Description: "A test server without auth",
8585
Repository: model.Repository{
8686
URL: "https://github.com/example/test-server",
@@ -92,7 +92,7 @@ func TestPublishEndpoint(t *testing.T) {
9292
tokenClaims: &auth.JWTClaims{
9393
AuthMethod: auth.MethodNone,
9494
Permissions: []auth.Permission{
95-
{Action: auth.PermissionActionPublish, ResourcePattern: "example/*"},
95+
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
9696
},
9797
},
9898
setupRegistryService: func(_ service.RegistryService) {
@@ -107,7 +107,7 @@ func TestPublishEndpoint(t *testing.T) {
107107
setupRegistryService: func(_ service.RegistryService) {
108108
// Empty registry - no setup needed
109109
},
110-
expectedStatus: http.StatusUnprocessableEntity,
110+
expectedStatus: http.StatusBadRequest,
111111
expectedError: "required header parameter is missing",
112112
},
113113
{
@@ -127,7 +127,7 @@ func TestPublishEndpoint(t *testing.T) {
127127
{
128128
name: "invalid token",
129129
requestBody: apiv0.ServerJSON{
130-
Name: "test-server",
130+
Name: "com.example/test-server",
131131
Description: "A test server",
132132
Version: "1.0.0",
133133
},
@@ -165,7 +165,7 @@ func TestPublishEndpoint(t *testing.T) {
165165
{
166166
name: "registry service error",
167167
requestBody: apiv0.ServerJSON{
168-
Name: "example/test-server",
168+
Name: "com.example/test-server",
169169
Description: "A test server",
170170
Version: "1.0.0",
171171
Repository: model.Repository{
@@ -183,7 +183,7 @@ func TestPublishEndpoint(t *testing.T) {
183183
setupRegistryService: func(registry service.RegistryService) {
184184
// Pre-publish the same server to cause duplicate version error
185185
existingServer := apiv0.ServerJSON{
186-
Name: "example/test-server",
186+
Name: "com.example/test-server",
187187
Description: "Existing test server",
188188
Version: "1.0.0",
189189
Repository: model.Repository{
@@ -503,8 +503,9 @@ func TestPublishEndpoint_MultipleSlashesEdgeCases(t *testing.T) {
503503
"%s: expected status %d, got %d", tc.description, tc.expectedStatus, rr.Code)
504504

505505
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)
506+
// Should contain our custom validation error message
507+
assert.Contains(t, rr.Body.String(), "server name",
508+
"%s: should contain server name validation error", tc.description)
508509
}
509510
})
510511
}

internal/validators/validators.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,19 @@ func parseServerName(serverJSON apiv0.ServerJSON) (string, error) {
413413
return "", fmt.Errorf("server name must be in format 'dns-namespace/name' with non-empty namespace and name parts")
414414
}
415415

416+
// Validate namespace and name format according to schema
417+
// Pattern: ^[a-zA-Z0-9.-]+/[a-zA-Z0-9._-]+$
418+
namespacePattern := regexp.MustCompile(`^[a-zA-Z0-9.-]+$`)
419+
namePattern := regexp.MustCompile(`^[a-zA-Z0-9._-]+$`)
420+
421+
if !namespacePattern.MatchString(parts[0]) {
422+
return "", fmt.Errorf("server namespace '%s' contains invalid characters: only alphanumeric characters, dots (.) and hyphens (-) are allowed", parts[0])
423+
}
424+
425+
if !namePattern.MatchString(parts[1]) {
426+
return "", fmt.Errorf("server name '%s' contains invalid characters: only alphanumeric characters, dots (.), underscores (_) and hyphens (-) are allowed", parts[1])
427+
}
428+
416429
return name, nil
417430
}
418431

pkg/api/v0/types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ type ServerMeta struct {
2929
// ServerJSON represents complete server information as defined in the MCP spec, with extension support
3030
type ServerJSON struct {
3131
Schema string `json:"$schema,omitempty"`
32-
Name string `json:"name" minLength:"1" maxLength:"200"`
32+
Name string `json:"name" minLength:"1" maxLength:"200" doc:"Server name in format 'reverse-dns-namespace/name' (e.g., 'com.example/server'). Namespace allows alphanumeric, dots, hyphens. Name allows alphanumeric, dots, underscores, hyphens."`
3333
Description string `json:"description" minLength:"1" maxLength:"100"`
3434
Status model.Status `json:"status,omitempty" minLength:"1"`
3535
Repository model.Repository `json:"repository,omitempty"`

0 commit comments

Comments
 (0)