Added /validate endpoint and modified mcp-publish so that all validation happens there (#896)
## Motivation and Context Per discussion with @rdimitrov - all validation now happens in a new `/validate` endpoint. For details, see below. ## Breaking Changes None. Validation on /publish only validates schema version and semantic checks (manual checks) as before. We can add schema validation by changing a flag in the called to `ValidateJSONSchema` when we're ready. ## Types of changes - [X] New feature (non-breaking change which adds functionality) ## 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 ## API Changes ### 1. New `/validate` Endpoint A new public endpoint was added for validating `server.json` files without publishing them. **Details:** - **Paths:** `/v0/validate` and `/v0.1/validate` - **Method:** `POST` - **Authentication:** None required (public endpoint) - **Request:** JSON body containing `ServerJSON` - **Response:** `200 OK` with `ValidationResult` containing: - `valid` (boolean) - `issues` (array of validation issues with type, path, message, severity, reference) **Implementation:** - New handler file: `internal/api/handlers/v0/validate.go` - Performs comprehensive validation using `ValidationAll` option: - Schema version validation - Full schema validation - Semantic validation - Always returns `200 OK` - validity is indicated in `result.Valid` field - Registered in router for both `/v0` and `/v0.1` prefixes ### 2. `/publish` Endpoint Validation Flow Changes The publish endpoint now validates earlier in the request flow and provides clearer error responses. **Before:** Validation was mixed into `ValidatePublishRequest()` (which also handled publisher extensions and registry ownership). **After:** 1. Validates `ServerJSON` structure first using `ValidateServerJSON()` (schema version + semantic validation) 2. Returns `422 Unprocessable Entity` with message: "Failed to publish server, invalid schema: call /validate for details" 3. Only proceeds to publisher extensions/registry validation if schema validation passes 4. No longer calls `ValidatePublishRequest()` internally for schema validation **Key Changes:** - Validation happens before `CreateServer()` call - Returns `422` on schema/semantic validation failures - Error message directs users to `/validate` endpoint for details - `ValidatePublishRequest()` no longer performs schema validation (added note in comments) ### 3. `/edit` Endpoint Validation Flow Changes Similar validation changes were applied to the edit endpoint. **Changes:** - Added `ValidateServerJSON()` call before server update - Returns `422` with same error message format on validation failure - Matches the `/publish` endpoint behavior ### 4. Validator Functions Refactoring Updated validator functions to reflect that schema validation is now done separately. **Changes to `ValidatePublishRequest()`:** - Removed internal `ValidateServerJSON()` call - Added note: "ValidateServerJSON should be called separately before this function" - Now only validates publisher extensions and registry ownership **Changes to `ValidateUpdateRequest()`:** - Same change (removed schema validation, added note) - Now only validates registry ownership ## CLI Changes ### 1. `validate` Command: Now Uses API Endpoint **Before:** Performed local validation using `validators.ValidateServerJSON()`. **After:** Calls the `/v0/validate` endpoint on the registry. **Details:** - Reads `server.json` and sends it to the registry API - Displays validation results returned by the server - Uses the same error formatting as before - Registry URL comes from token file (or default) - No authentication required (public endpoint) **Implementation:** - Added `validateViaAPI()` function that POSTs to `/v0/validate` - Parses `ValidationResult` response from server - Uses shared `printValidationIssues()` for formatting ### 2. `publish` Command: Validation Only on 422 Errors **Before:** Performed upfront local schema validation before publishing. **After:** Attempts publish first, then validates only if server returns 422. **Details:** - Attempts publish immediately (no upfront validation) - If publish returns 422 (Unprocessable Entity): - Calls `/v0/validate` endpoint to get detailed validation errors - Formats and displays them using same logic as `validate` command - Returns error with migration links - For non-422 errors (401, 403, etc.), returns original error (no validation call) **Implementation Changes:** - Removed upfront `runValidationAndPrintIssues()` call - Modified `publishToRegistry()` to return HTTP status code alongside response/error - Added 422 detection logic that calls `validateViaAPI()` ### 3. Shared Validation Error Formatting **Before:** `runValidationAndPrintIssues()` function handled both validation and printing together. **After:** Refactored to separate printing logic. **Details:** - Created `printValidationIssues()` function - just prints validation issues (no validation logic) - Removed `runValidationAndPrintIssues()` function (no longer used) - Both `validate` and `publish` commands use `printValidationIssues()` for consistent output **Behavior:** - Prints schema validation errors with migration guidance - Prints all other validation issues with formatting - Returns formatted error message string for schema issues ## Summary Both CLI commands now use the `/validate` API endpoint for validation instead of local validation. The `publish` command validates only after receiving a 422 error response, and both commands share the same error formatting via `printValidationIssues()`. ### Benefits - **Centralized validation logic** - All validation happens on the server - **Always up-to-date** - CLI always uses current server validation rules - **Consistent experience** - Same validation behavior whether using CLI or API directly - **Better separation of concerns** - Schema validation separated from publisher-specific validation
This commit is contained in:
Bob Dickinson
GitHub
parent
dc73689210
commit
f06d1eda8a
7 changed files with 986 additions and 132 deletions
|
|
@ -12,6 +12,7 @@ import (
|
|||
"github.com/modelcontextprotocol/registry/internal/config"
|
||||
"github.com/modelcontextprotocol/registry/internal/database"
|
||||
"github.com/modelcontextprotocol/registry/internal/service"
|
||||
"github.com/modelcontextprotocol/registry/internal/validators"
|
||||
apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0"
|
||||
"github.com/modelcontextprotocol/registry/pkg/model"
|
||||
)
|
||||
|
|
@ -91,6 +92,12 @@ func RegisterEditEndpoints(api huma.API, pathPrefix string, registry service.Reg
|
|||
return nil, huma.Error400BadRequest("Version in request body must match URL path parameter")
|
||||
}
|
||||
|
||||
// Validate server JSON structure and schema (returns 422 on validation failure)
|
||||
validationResult := validators.ValidateServerJSON(&input.Body, validators.ValidationSchemaVersionAndSemantic)
|
||||
if !validationResult.Valid {
|
||||
return nil, huma.Error422UnprocessableEntity("Failed to edit server, invalid schema: call /validate for details")
|
||||
}
|
||||
|
||||
// Handle status changes with proper permission validation
|
||||
if input.Status != "" {
|
||||
newStatus := model.Status(input.Status)
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import (
|
|||
"github.com/modelcontextprotocol/registry/internal/auth"
|
||||
"github.com/modelcontextprotocol/registry/internal/config"
|
||||
"github.com/modelcontextprotocol/registry/internal/service"
|
||||
"github.com/modelcontextprotocol/registry/internal/validators"
|
||||
apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0"
|
||||
)
|
||||
|
||||
|
|
@ -53,6 +54,12 @@ func RegisterPublishEndpoint(api huma.API, pathPrefix string, registry service.R
|
|||
return nil, huma.Error403Forbidden(buildPermissionErrorMessage(input.Body.Name, claims.Permissions))
|
||||
}
|
||||
|
||||
// Validate server JSON structure and schema (returns 422 on validation failure)
|
||||
validationResult := validators.ValidateServerJSON(&input.Body, validators.ValidationSchemaVersionAndSemantic)
|
||||
if !validationResult.Valid {
|
||||
return nil, huma.Error422UnprocessableEntity("Failed to publish server, invalid schema: call /validate for details")
|
||||
}
|
||||
|
||||
// Publish the server with extensions
|
||||
publishedServer, err := registry.CreateServer(ctx, &input.Body)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -366,6 +366,24 @@ func TestPublishEndpoint(t *testing.T) {
|
|||
expectedStatus: http.StatusUnprocessableEntity,
|
||||
expectedError: "expected string to match pattern",
|
||||
},
|
||||
{
|
||||
name: "invalid schema - version range instead of specific version",
|
||||
requestBody: apiv0.ServerJSON{
|
||||
Schema: model.CurrentSchemaURL,
|
||||
Name: "com.example/test-server",
|
||||
Description: "A test server",
|
||||
Version: "^1.0.0", // Version range, not allowed
|
||||
},
|
||||
tokenClaims: &auth.JWTClaims{
|
||||
AuthMethod: auth.MethodNone,
|
||||
Permissions: []auth.Permission{
|
||||
{Action: auth.PermissionActionPublish, ResourcePattern: "*"},
|
||||
},
|
||||
},
|
||||
setupRegistryService: func(_ service.RegistryService) {},
|
||||
expectedStatus: http.StatusUnprocessableEntity,
|
||||
expectedError: "Failed to publish server, invalid schema: call /validate for details",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
|
|
|
|||
36
internal/api/handlers/v0/validate.go
Normal file
36
internal/api/handlers/v0/validate.go
Normal file
|
|
@ -0,0 +1,36 @@
|
|||
package v0
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"github.com/danielgtaylor/huma/v2"
|
||||
"github.com/modelcontextprotocol/registry/internal/validators"
|
||||
apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0"
|
||||
)
|
||||
|
||||
// ValidateServerInput represents the input for validating a server JSON
|
||||
type ValidateServerInput struct {
|
||||
Body apiv0.ServerJSON `body:""`
|
||||
}
|
||||
|
||||
// RegisterValidateEndpoint registers the validate endpoint with a custom path prefix
|
||||
func RegisterValidateEndpoint(api huma.API, pathPrefix string) {
|
||||
huma.Register(api, huma.Operation{
|
||||
OperationID: "validate-server" + strings.ReplaceAll(pathPrefix, "/", "-"),
|
||||
Method: http.MethodPost,
|
||||
Path: pathPrefix + "/validate",
|
||||
Summary: "Validate MCP server JSON",
|
||||
Description: "Validate a server.json file without publishing it to the registry",
|
||||
Tags: []string{"validate"},
|
||||
}, func(_ context.Context, input *ValidateServerInput) (*Response[validators.ValidationResult], error) {
|
||||
// Perform comprehensive validation (schema version, full schema validation, and semantic)
|
||||
result := validators.ValidateServerJSON(&input.Body, validators.ValidationAll)
|
||||
|
||||
// Return validation result (always 200 OK, validity indicated in result.Valid)
|
||||
return &Response[validators.ValidationResult]{
|
||||
Body: *result,
|
||||
}, nil
|
||||
})
|
||||
}
|
||||
173
internal/api/handlers/v0/validate_test.go
Normal file
173
internal/api/handlers/v0/validate_test.go
Normal file
|
|
@ -0,0 +1,173 @@
|
|||
package v0_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/danielgtaylor/huma/v2"
|
||||
"github.com/danielgtaylor/huma/v2/adapters/humago"
|
||||
v0 "github.com/modelcontextprotocol/registry/internal/api/handlers/v0"
|
||||
apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0"
|
||||
"github.com/modelcontextprotocol/registry/pkg/model"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
type issueStruct struct {
|
||||
Type string `json:"type"`
|
||||
Path string `json:"path"`
|
||||
Message string `json:"message"`
|
||||
Severity string `json:"severity"`
|
||||
Reference string `json:"reference"`
|
||||
}
|
||||
|
||||
func TestValidateEndpoint(t *testing.T) {
|
||||
// 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.RegisterValidateEndpoint(api, "/v0")
|
||||
|
||||
testCases := []struct {
|
||||
name string
|
||||
serverJSON apiv0.ServerJSON
|
||||
expectedValid bool
|
||||
expectedStatus int
|
||||
description string
|
||||
validateIssues func(t *testing.T, issues []issueStruct)
|
||||
}{
|
||||
{
|
||||
name: "valid server json",
|
||||
serverJSON: apiv0.ServerJSON{
|
||||
Schema: model.CurrentSchemaURL,
|
||||
Name: "com.example/test-server",
|
||||
Description: "A test server",
|
||||
Version: "1.0.0",
|
||||
},
|
||||
expectedValid: true,
|
||||
expectedStatus: http.StatusOK,
|
||||
description: "Should return valid for a properly formatted server JSON",
|
||||
validateIssues: func(t *testing.T, issues []issueStruct) {
|
||||
t.Helper()
|
||||
assert.Empty(t, issues, "Valid server JSON should have no issues")
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "version range should be invalid",
|
||||
serverJSON: apiv0.ServerJSON{
|
||||
Schema: model.CurrentSchemaURL,
|
||||
Name: "com.example/test-server",
|
||||
Description: "A test server",
|
||||
Version: "^1.0.0", // Version range, not allowed
|
||||
},
|
||||
expectedValid: false,
|
||||
expectedStatus: http.StatusOK,
|
||||
description: "Should return invalid when version is a range instead of specific version",
|
||||
validateIssues: func(t *testing.T, issues []issueStruct) {
|
||||
t.Helper()
|
||||
require.Greater(t, len(issues), 0, "Should have at least one issue")
|
||||
issue := issues[0]
|
||||
assert.Equal(t, "semantic", issue.Type, "Issue type should be semantic")
|
||||
assert.Equal(t, "version", issue.Path, "Issue path should be 'version'")
|
||||
assert.Equal(t, "error", issue.Severity, "Issue severity should be error")
|
||||
assert.NotEmpty(t, issue.Message, "Issue message should not be empty")
|
||||
assert.Contains(t, issue.Message, "^1.0.0", "Issue message should contain the version range")
|
||||
assert.NotEmpty(t, issue.Reference, "Issue reference should not be empty")
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "latest version should be invalid",
|
||||
serverJSON: apiv0.ServerJSON{
|
||||
Schema: model.CurrentSchemaURL,
|
||||
Name: "com.example/test-server",
|
||||
Description: "A test server",
|
||||
Version: "latest", // "latest" is reserved
|
||||
},
|
||||
expectedValid: false,
|
||||
expectedStatus: http.StatusOK,
|
||||
description: "Should return invalid when version is 'latest' (reserved word)",
|
||||
validateIssues: func(t *testing.T, issues []issueStruct) {
|
||||
t.Helper()
|
||||
require.Greater(t, len(issues), 0, "Should have at least one issue")
|
||||
issue := issues[0]
|
||||
assert.Equal(t, "semantic", issue.Type, "Issue type should be semantic")
|
||||
assert.Equal(t, "version", issue.Path, "Issue path should be 'version'")
|
||||
assert.Equal(t, "error", issue.Severity, "Issue severity should be error")
|
||||
assert.NotEmpty(t, issue.Message, "Issue message should not be empty")
|
||||
assert.Contains(t, strings.ToLower(issue.Message), "latest", "Issue message should mention 'latest'")
|
||||
assert.NotEmpty(t, issue.Reference, "Issue reference should not be empty")
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
// Create request body
|
||||
bodyBytes, err := json.Marshal(tc.serverJSON)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create request
|
||||
req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "/v0/validate", bytes.NewBuffer(bodyBytes))
|
||||
require.NoError(t, err)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
|
||||
// Perform request
|
||||
rr := httptest.NewRecorder()
|
||||
mux.ServeHTTP(rr, req)
|
||||
|
||||
// Assert status code
|
||||
assert.Equal(t, tc.expectedStatus, rr.Code, "%s: expected status %d, got %d", tc.description, tc.expectedStatus, rr.Code)
|
||||
|
||||
// Parse response - Huma returns ValidationResult directly
|
||||
var result struct {
|
||||
Valid bool `json:"valid"`
|
||||
Issues []issueStruct `json:"issues"`
|
||||
}
|
||||
err = json.Unmarshal(rr.Body.Bytes(), &result)
|
||||
require.NoError(t, err, "Failed to parse response: %s", rr.Body.String())
|
||||
|
||||
// Debug output for failed tests
|
||||
if tc.expectedValid != result.Valid {
|
||||
t.Logf("Response body: %s", rr.Body.String())
|
||||
t.Logf("Validation issues: %+v", result.Issues)
|
||||
}
|
||||
|
||||
// Always log response for first test case to show structure
|
||||
if tc.name == "valid server json" {
|
||||
var prettyJSON bytes.Buffer
|
||||
if err := json.Indent(&prettyJSON, rr.Body.Bytes(), "", " "); err == nil {
|
||||
t.Logf("Example valid response:\n%s", prettyJSON.String())
|
||||
}
|
||||
}
|
||||
if tc.name == "version range should be invalid" {
|
||||
var prettyJSON bytes.Buffer
|
||||
if err := json.Indent(&prettyJSON, rr.Body.Bytes(), "", " "); err == nil {
|
||||
t.Logf("Example invalid response:\n%s", prettyJSON.String())
|
||||
}
|
||||
}
|
||||
|
||||
// Assert validity
|
||||
assert.Equal(t, tc.expectedValid, result.Valid, "%s: expected valid=%v, got %v. Issues: %+v", tc.description, tc.expectedValid, result.Valid, result.Issues)
|
||||
|
||||
// Validate issues structure and content
|
||||
if tc.validateIssues != nil {
|
||||
tc.validateIssues(t, result.Issues)
|
||||
} else if !tc.expectedValid {
|
||||
// Default validation: if expected invalid, verify there are issues
|
||||
assert.Greater(t, len(result.Issues), 0, "%s: expected validation issues but got none. Response: %s", tc.description, rr.Body.String())
|
||||
// Validate all issues have required fields
|
||||
for i, issue := range result.Issues {
|
||||
assert.NotEmpty(t, issue.Type, "Issue %d: type should not be empty", i)
|
||||
assert.NotEmpty(t, issue.Severity, "Issue %d: severity should not be empty", i)
|
||||
assert.NotEmpty(t, issue.Message, "Issue %d: message should not be empty", i)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -21,6 +21,7 @@ func RegisterV0Routes(
|
|||
v0.RegisterEditEndpoints(api, "/v0", registry, cfg)
|
||||
v0auth.RegisterAuthEndpoints(api, "/v0", cfg)
|
||||
v0.RegisterPublishEndpoint(api, "/v0", registry, cfg)
|
||||
v0.RegisterValidateEndpoint(api, "/v0")
|
||||
}
|
||||
|
||||
func RegisterV0_1Routes(
|
||||
|
|
@ -33,4 +34,5 @@ func RegisterV0_1Routes(
|
|||
v0.RegisterEditEndpoints(api, "/v0.1", registry, cfg)
|
||||
v0auth.RegisterAuthEndpoints(api, "/v0.1", cfg)
|
||||
v0.RegisterPublishEndpoint(api, "/v0.1", registry, cfg)
|
||||
v0.RegisterValidateEndpoint(api, "/v0.1")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -626,18 +626,13 @@ func validateRemoteTransport(ctx *ValidationContext, obj *model.Transport) *Vali
|
|||
}
|
||||
|
||||
// ValidatePublishRequest validates a complete publish request including extensions
|
||||
// Note: ValidateServerJSON should be called separately before this function
|
||||
func ValidatePublishRequest(ctx context.Context, req apiv0.ServerJSON, cfg *config.Config) error {
|
||||
// Validate publisher extensions in _meta
|
||||
if err := validatePublisherExtensions(req); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Validate the server detail (includes all nested validation)
|
||||
result := ValidateServerJSON(&req, ValidationSchemaVersionAndSemantic)
|
||||
if err := result.FirstError(); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Validate registry ownership for all packages if validation is enabled
|
||||
if cfg.EnableRegistryValidation {
|
||||
if err := validateRegistryOwnership(ctx, req); err != nil {
|
||||
|
|
@ -648,13 +643,9 @@ func ValidatePublishRequest(ctx context.Context, req apiv0.ServerJSON, cfg *conf
|
|||
return nil
|
||||
}
|
||||
|
||||
// ValidateUpdateRequest validates an update request including registry ownership
|
||||
// Note: ValidateServerJSON should be called separately before this function
|
||||
func ValidateUpdateRequest(ctx context.Context, req apiv0.ServerJSON, cfg *config.Config, skipRegistryValidation bool) error {
|
||||
// Validate the server detail (includes all nested validation)
|
||||
result := ValidateServerJSON(&req, ValidationSchemaVersionAndSemantic)
|
||||
if err := result.FirstError(); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if cfg.EnableRegistryValidation && !skipRegistryValidation {
|
||||
if err := validateRegistryOwnership(ctx, req); err != nil {
|
||||
return err
|
||||
|
|
|
|||
Loading…
Reference in a new issue