From f06d1eda8ae43df86fc6390196ec50f70c298f3c Mon Sep 17 00:00:00 2001 From: Bob Dickinson Date: Wed, 4 Feb 2026 05:52:07 -0800 Subject: [PATCH] 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 --- CHANGES.md | 0 cmd/publisher/commands/publish.go | 62 ++-- cmd/publisher/commands/publish_test.go | 401 +++++++++++++++++----- cmd/publisher/commands/testutil_test.go | 117 +++++++ cmd/publisher/commands/validate.go | 108 +++++- cmd/publisher/commands/validate_test.go | 182 ++++++++++ internal/api/handlers/v0/edit.go | 7 + internal/api/handlers/v0/publish.go | 7 + internal/api/handlers/v0/publish_test.go | 18 + internal/api/handlers/v0/validate.go | 36 ++ internal/api/handlers/v0/validate_test.go | 173 ++++++++++ internal/api/router/v0.go | 2 + internal/validators/validators.go | 15 +- 13 files changed, 991 insertions(+), 137 deletions(-) create mode 100644 CHANGES.md create mode 100644 cmd/publisher/commands/testutil_test.go create mode 100644 cmd/publisher/commands/validate_test.go create mode 100644 internal/api/handlers/v0/validate.go create mode 100644 internal/api/handlers/v0/validate_test.go diff --git a/CHANGES.md b/CHANGES.md new file mode 100644 index 0000000..e69de29 diff --git a/cmd/publisher/commands/publish.go b/cmd/publisher/commands/publish.go index 7a34630..da7de7d 100644 --- a/cmd/publisher/commands/publish.go +++ b/cmd/publisher/commands/publish.go @@ -12,7 +12,6 @@ import ( "path/filepath" "strings" - "github.com/modelcontextprotocol/registry/internal/validators" apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0" ) @@ -38,22 +37,6 @@ func PublishCommand(args []string) error { return fmt.Errorf("invalid server.json: %w", err) } - // Validate schema version (non-empty schema, valid schema, and current schema) - // This performs schema version checks without full schema validation - // Note: When we enable full validation, use validators.ValidationAll instead - result, formattedErrorMsg := runValidationAndPrintIssues(&serverJSON, validators.ValidationSchemaVersionOnly) - if !result.Valid { - // Return error after printing (all errors already printed by validateServerJSON) - // Prefer formatted error message for schema validation errors, otherwise use first error - if formattedErrorMsg != "" { - return fmt.Errorf("%s", formattedErrorMsg) - } - if firstErr := result.FirstError(); firstErr != nil { - return firstErr - } - return fmt.Errorf("validation failed") - } - // Load saved token homeDir, err := os.UserHomeDir() if err != nil { @@ -82,8 +65,33 @@ func PublishCommand(args []string) error { // Publish to registry _, _ = fmt.Fprintf(os.Stdout, "Publishing to %s...\n", registryURL) - response, err := publishToRegistry(registryURL, serverData, token) + response, statusCode, err := publishToRegistry(registryURL, serverData, token) if err != nil { + // If publish failed with 422, call validate endpoint to show detailed errors + if statusCode == http.StatusUnprocessableEntity { + _, _ = fmt.Fprintln(os.Stdout, "Validation failed. Checking detailed validation errors...") + _, _ = fmt.Fprintln(os.Stdout) + + // Call validate endpoint (same as validate command does) + result, validateErr := validateViaAPI(registryURL, serverData) + if validateErr != nil { + // If validate also fails, return original publish error + return fmt.Errorf("publish failed: %w", err) + } + + // Print validation results using shared formatting logic + formattedErrorMsg := printValidationIssues(result, &serverJSON) + + if !result.Valid { + // Return error with formatted message if available + if formattedErrorMsg != "" { + return fmt.Errorf("%s", formattedErrorMsg) + } + return fmt.Errorf("validation failed") + } + } + + // For non-422 errors, return the original error return fmt.Errorf("publish failed: %w", err) } @@ -93,18 +101,18 @@ func PublishCommand(args []string) error { return nil } -func publishToRegistry(registryURL string, serverData []byte, token string) (*apiv0.ServerResponse, error) { +func publishToRegistry(registryURL string, serverData []byte, token string) (*apiv0.ServerResponse, int, error) { // Parse the server JSON data var serverJSON apiv0.ServerJSON err := json.Unmarshal(serverData, &serverJSON) if err != nil { - return nil, fmt.Errorf("error parsing server.json file: %w", err) + return nil, 0, fmt.Errorf("error parsing server.json file: %w", err) } // Convert to JSON jsonData, err := json.Marshal(serverJSON) if err != nil { - return nil, fmt.Errorf("error serializing request: %w", err) + return nil, 0, fmt.Errorf("error serializing request: %w", err) } // Ensure URL ends with the publish endpoint @@ -116,7 +124,7 @@ func publishToRegistry(registryURL string, serverData []byte, token string) (*ap // Create and send request req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, publishURL, bytes.NewBuffer(jsonData)) if err != nil { - return nil, fmt.Errorf("error creating request: %w", err) + return nil, 0, fmt.Errorf("error creating request: %w", err) } req.Header.Set("Content-Type", "application/json") req.Header.Set("Authorization", "Bearer "+token) @@ -124,24 +132,24 @@ func publishToRegistry(registryURL string, serverData []byte, token string) (*ap client := &http.Client{} resp, err := client.Do(req) if err != nil { - return nil, fmt.Errorf("error sending request: %w", err) + return nil, 0, fmt.Errorf("error sending request: %w", err) } defer resp.Body.Close() // Read response body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("error reading response: %w", err) + return nil, resp.StatusCode, fmt.Errorf("error reading response: %w", err) } if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("server returned status %d: %s", resp.StatusCode, body) + return nil, resp.StatusCode, fmt.Errorf("server returned status %d: %s", resp.StatusCode, body) } var serverResponse apiv0.ServerResponse if err := json.Unmarshal(body, &serverResponse); err != nil { - return nil, err + return nil, resp.StatusCode, err } - return &serverResponse, nil + return &serverResponse, resp.StatusCode, nil } diff --git a/cmd/publisher/commands/publish_test.go b/cmd/publisher/commands/publish_test.go index e7af688..27fcdfa 100644 --- a/cmd/publisher/commands/publish_test.go +++ b/cmd/publisher/commands/publish_test.go @@ -2,132 +2,361 @@ package commands_test import ( "encoding/json" - "os" - "path/filepath" - "strings" + "io" + "net/http" "testing" "github.com/modelcontextprotocol/registry/cmd/publisher/commands" + "github.com/modelcontextprotocol/registry/internal/validators" apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0" "github.com/modelcontextprotocol/registry/pkg/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestPublishCommand_Success(t *testing.T) { + // Setup mock server that returns success + server := SetupMockRegistryServer(t, nil, nil) + + // Setup token + SetupTestToken(t, server.URL, "test-token") + + // Create valid server.json + serverJSON := apiv0.ServerJSON{ + Schema: model.CurrentSchemaURL, + Name: "com.example/test-server", + Description: "A test server", + Version: "1.0.0", + } + CreateTestServerJSON(t, serverJSON) + + // Run publish command + err := commands.PublishCommand([]string{}) + + // Should succeed + assert.NoError(t, err) +} + +func TestPublishCommand_422ValidationFlow(t *testing.T) { + validateCallCount := 0 + publishCallCount := 0 + + // Setup mock server + server := SetupMockRegistryServer(t, + // Publish handler: return 422 for invalid schema + func(w http.ResponseWriter, _ *http.Request) { + publishCallCount++ + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message":"Failed to publish server, invalid schema: call /validate for details"}`)) + }, + // Validate handler: return validation errors + func(w http.ResponseWriter, r *http.Request) { + validateCallCount++ + w.Header().Set("Content-Type", "application/json") + + body, _ := io.ReadAll(r.Body) + var req apiv0.ServerJSON + _ = json.Unmarshal(body, &req) + + // Return validation result with deprecated schema error + result := validators.ValidationResult{ + Valid: false, + Issues: []validators.ValidationIssue{ + { + Type: validators.ValidationIssueTypeSemantic, + Path: "schema", + Message: "schema version 2025-07-09 is not the current version", + Severity: validators.ValidationIssueSeverityWarning, + Reference: "schema-version-deprecated", + }, + }, + } + + _ = json.NewEncoder(w).Encode(result) + }, + ) + + // Setup token + SetupTestToken(t, server.URL, "test-token") + + // Create server.json with deprecated schema + serverJSON := apiv0.ServerJSON{ + Schema: "https://static.modelcontextprotocol.io/schemas/2025-07-09/server.schema.json", + Name: "com.example/test-server", + Description: "A test server", + Version: "1.0.0", + } + CreateTestServerJSON(t, serverJSON) + + // Run publish command + err := commands.PublishCommand([]string{}) + + // Should fail with validation error + require.Error(t, err) + assert.Contains(t, err.Error(), "schema version 2025-07-09") + assert.Contains(t, err.Error(), "Migration checklist:") + assert.Contains(t, err.Error(), "Full changelog with examples:") + + // Verify both endpoints were called + assert.Equal(t, 1, publishCallCount, "publish endpoint should be called once") + assert.Equal(t, 1, validateCallCount, "validate endpoint should be called once after 422") +} + +func TestPublishCommand_422WithMultipleIssues(t *testing.T) { + validateCallCount := 0 + + // Setup mock server + server := SetupMockRegistryServer(t, + func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message":"Failed to publish server, invalid schema"}`)) + }, + func(w http.ResponseWriter, _ *http.Request) { + validateCallCount++ + w.Header().Set("Content-Type", "application/json") + + result := validators.ValidationResult{ + Valid: false, + Issues: []validators.ValidationIssue{ + { + Type: validators.ValidationIssueTypeSemantic, + Path: "version", + Message: "version must be a specific version, not a range", + Severity: validators.ValidationIssueSeverityError, + Reference: "semantic-version-range", + }, + { + Type: validators.ValidationIssueTypeSchema, + Path: "name", + Message: "name is required", + Severity: validators.ValidationIssueSeverityError, + Reference: "schema-field-required", + }, + }, + } + + _ = json.NewEncoder(w).Encode(result) + }, + ) + + SetupTestToken(t, server.URL, "test-token") + + serverJSON := apiv0.ServerJSON{ + Schema: model.CurrentSchemaURL, + Name: "com.example/test-server", + Description: "A test server", + Version: "^1.0.0", // Invalid version range + } + CreateTestServerJSON(t, serverJSON) + + err := commands.PublishCommand([]string{}) + + require.Error(t, err) + assert.Equal(t, 1, validateCallCount, "validate endpoint should be called") +} + +func TestPublishCommand_NoToken(t *testing.T) { + // Don't create a token file + serverJSON := apiv0.ServerJSON{ + Schema: model.CurrentSchemaURL, + Name: "com.example/test-server", + Description: "A test server", + Version: "1.0.0", + } + CreateTestServerJSON(t, serverJSON) + + err := commands.PublishCommand([]string{}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "not authenticated") +} + +func TestPublishCommand_Non422Error(t *testing.T) { + publishCallCount := 0 + + server := SetupMockRegistryServer(t, + func(w http.ResponseWriter, _ *http.Request) { + publishCallCount++ + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message":"Unauthorized"}`)) + }, + nil, // No validate handler needed + ) + + SetupTestToken(t, server.URL, "invalid-token") + + serverJSON := apiv0.ServerJSON{ + Schema: model.CurrentSchemaURL, + Name: "com.example/test-server", + Description: "A test server", + Version: "1.0.0", + } + CreateTestServerJSON(t, serverJSON) + + err := commands.PublishCommand([]string{}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "publish failed") + assert.Equal(t, 1, publishCallCount, "publish endpoint should be called") +} + func TestPublishCommand_DeprecatedSchema(t *testing.T) { - // Create a temporary directory for test files - tempDir, err := os.MkdirTemp("", "mcp-publisher-test") - if err != nil { - t.Fatalf("Failed to create temp dir: %v", err) - } - defer os.RemoveAll(tempDir) - - // Change to temp directory - originalDir, err := os.Getwd() - if err != nil { - t.Fatalf("Failed to get current dir: %v", err) - } - defer func() { _ = os.Chdir(originalDir) }() - - if err := os.Chdir(tempDir); err != nil { - t.Fatalf("Failed to change to temp dir: %v", err) - } - tests := []struct { - name string - schema string - expectError bool - errorSubstr string + name string + schema string + publishStatus int + validationOpts func(req apiv0.ServerJSON) validators.ValidationResult + expectError bool + errorSubstr string + checkLinks bool }{ { - name: "deprecated 2025-07-09 schema should show warning", - schema: "https://static.modelcontextprotocol.io/schemas/2025-07-09/server.schema.json", + name: "deprecated 2025-07-09 schema should show warning", + schema: "https://static.modelcontextprotocol.io/schemas/2025-07-09/server.schema.json", + publishStatus: http.StatusUnprocessableEntity, + validationOpts: func(_ apiv0.ServerJSON) validators.ValidationResult { + return validators.ValidationResult{ + Valid: false, + Issues: []validators.ValidationIssue{ + { + Type: validators.ValidationIssueTypeSemantic, + Path: "schema", + Message: "schema version 2025-07-09 is not the current version", + Severity: validators.ValidationIssueSeverityWarning, + Reference: "schema-version-deprecated", + }, + }, + } + }, expectError: true, - errorSubstr: "schema version 2025-07-09 is not the current version", + errorSubstr: "schema version 2025-07-09", + checkLinks: true, }, { - name: "current 2025-12-11 schema should pass validation", - schema: "https://static.modelcontextprotocol.io/schemas/2025-12-11/server.schema.json", + name: "current 2025-12-11 schema should pass validation", + schema: "https://static.modelcontextprotocol.io/schemas/2025-12-11/server.schema.json", + publishStatus: http.StatusCreated, + validationOpts: func(_ apiv0.ServerJSON) validators.ValidationResult { + // Should not be called since publish succeeds + return validators.ValidationResult{Valid: true} + }, expectError: false, }, { - name: "empty schema should pass validation", - schema: "", - expectError: false, + name: "empty schema should fail validation", + schema: "", + publishStatus: http.StatusUnprocessableEntity, + validationOpts: func(_ apiv0.ServerJSON) validators.ValidationResult { + return validators.ValidationResult{ + Valid: false, + Issues: []validators.ValidationIssue{ + { + Type: validators.ValidationIssueTypeSemantic, + Path: "schema", + Message: "$schema field is required", + Severity: validators.ValidationIssueSeverityError, + Reference: "schema-field-required", + }, + }, + } + }, + expectError: true, + errorSubstr: "$schema field is required", + checkLinks: true, }, { - name: "custom schema without 2025-07-09 should fail validation", - schema: "https://example.com/custom.schema.json", + name: "custom schema without valid version should fail validation", + schema: "https://example.com/custom.schema.json", + publishStatus: http.StatusUnprocessableEntity, + validationOpts: func(_ apiv0.ServerJSON) validators.ValidationResult { + return validators.ValidationResult{ + Valid: false, + Issues: []validators.ValidationIssue{ + { + Type: validators.ValidationIssueTypeSchema, + Path: "schema", + Message: "failed to extract schema version from URL", + Severity: validators.ValidationIssueSeverityError, + Reference: "schema-version-extraction-error", + }, + }, + } + }, expectError: true, errorSubstr: "failed to extract schema version from URL", + checkLinks: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create test server.json with specific schema + validateCallCount := 0 + publishCallCount := 0 + + // Setup mock server + server := SetupMockRegistryServer(t, + // Publish handler + func(w http.ResponseWriter, _ *http.Request) { + publishCallCount++ + if tt.publishStatus == http.StatusCreated { + w.WriteHeader(http.StatusCreated) + response := apiv0.ServerResponse{ + Server: apiv0.ServerJSON{ + Name: "com.example/test-server", + Version: "1.0.0", + }, + } + _ = json.NewEncoder(w).Encode(response) + } else { + w.WriteHeader(tt.publishStatus) + _, _ = w.Write([]byte(`{"message":"Failed to publish server, invalid schema: call /validate for details"}`)) + } + }, + // Validate handler (only called on 422) + func(w http.ResponseWriter, r *http.Request) { + validateCallCount++ + w.Header().Set("Content-Type", "application/json") + + body, _ := io.ReadAll(r.Body) + var req apiv0.ServerJSON + _ = json.Unmarshal(body, &req) + + result := tt.validationOpts(req) + _ = json.NewEncoder(w).Encode(result) + }, + ) + + SetupTestToken(t, server.URL, "test-token") + + // Create server.json with specific schema serverJSON := apiv0.ServerJSON{ Schema: tt.schema, Name: "com.example/test-server", Description: "A test server", Version: "1.0.0", - Repository: &model.Repository{ - URL: "https://github.com/example/test", - Source: "github", - }, - Packages: []model.Package{ - { - RegistryType: model.RegistryTypeNPM, - RegistryBaseURL: model.RegistryURLNPM, - Identifier: "@example/test-server", - Version: "1.0.0", - Transport: model.Transport{ - Type: model.TransportTypeStdio, - }, - }, - }, } + CreateTestServerJSON(t, serverJSON) - jsonData, err := json.MarshalIndent(serverJSON, "", " ") - if err != nil { - t.Fatalf("Failed to marshal test JSON: %v", err) - } - - // Write server.json to temp directory - serverFile := filepath.Join(tempDir, "server.json") - if err := os.WriteFile(serverFile, jsonData, 0o600); err != nil { - t.Fatalf("Failed to write server.json: %v", err) - } - - err = commands.PublishCommand([]string{}) + err := commands.PublishCommand([]string{}) if tt.expectError { - if err == nil { - t.Errorf("Expected error for deprecated schema, but got none") - return + require.Error(t, err, "Expected error for test case: %s", tt.name) + if tt.errorSubstr != "" { + assert.Contains(t, err.Error(), tt.errorSubstr, "Error should contain expected substring") } - if !strings.Contains(err.Error(), tt.errorSubstr) { - t.Errorf("Expected error containing '%s', got: %v", tt.errorSubstr, err) + if tt.checkLinks { + assert.Contains(t, err.Error(), "Migration checklist:", "Error should contain migration checklist link") + assert.Contains(t, err.Error(), "Full changelog with examples:", "Error should contain changelog link") } - // Check that the error contains the migration links - if !strings.Contains(err.Error(), "Migration checklist:") { - t.Errorf("Expected error to contain migration checklist link") - } - if !strings.Contains(err.Error(), "Full changelog with examples:") { - t.Errorf("Expected error to contain changelog link") + if tt.publishStatus == http.StatusUnprocessableEntity { + assert.Equal(t, 1, publishCallCount, "publish endpoint should be called once") + assert.Equal(t, 1, validateCallCount, "validate endpoint should be called after 422") } } else { - // For non-deprecated schemas, we expect the command to fail at auth step, not schema validation - if err != nil && strings.Contains(err.Error(), "deprecated schema detected") { - t.Errorf("Unexpected deprecated schema error for schema '%s': %v", tt.schema, err) - } - - // We expect auth errors for valid schemas since we don't have a token - if err != nil && !strings.Contains(err.Error(), "not authenticated") && !strings.Contains(err.Error(), "failed to read token") { - t.Logf("Expected auth error for valid schema, got: %v", err) - } + assert.NoError(t, err, "Expected success for test case: %s", tt.name) + assert.Equal(t, 1, publishCallCount, "publish endpoint should be called once") + assert.Equal(t, 0, validateCallCount, "validate endpoint should not be called on success") } - - // Clean up for next test - os.Remove(serverFile) }) } } diff --git a/cmd/publisher/commands/testutil_test.go b/cmd/publisher/commands/testutil_test.go new file mode 100644 index 0000000..51c6687 --- /dev/null +++ b/cmd/publisher/commands/testutil_test.go @@ -0,0 +1,117 @@ +package commands_test + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/modelcontextprotocol/registry/cmd/publisher/commands" + "github.com/modelcontextprotocol/registry/internal/validators" + apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0" + "github.com/stretchr/testify/require" +) + +// SetupMockRegistryServer creates an httptest.Server that mocks the registry API +func SetupMockRegistryServer(t *testing.T, publishHandler func(w http.ResponseWriter, r *http.Request), validateHandler func(w http.ResponseWriter, r *http.Request)) *httptest.Server { + t.Helper() + + mux := http.NewServeMux() + + // Default handlers if not provided + if publishHandler == nil { + publishHandler = func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusCreated) + response := apiv0.ServerResponse{ + Server: apiv0.ServerJSON{ + Name: "com.example/test", + Version: "1.0.0", + }, + } + _ = json.NewEncoder(w).Encode(response) + } + } + + if validateHandler == nil { + validateHandler = func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + result := validators.ValidationResult{Valid: true} + _ = json.NewEncoder(w).Encode(result) + } + } + + mux.HandleFunc("/v0/publish", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + w.WriteHeader(http.StatusMethodNotAllowed) + return + } + publishHandler(w, r) + }) + + mux.HandleFunc("/v0/validate", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + w.WriteHeader(http.StatusMethodNotAllowed) + return + } + validateHandler(w, r) + }) + + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + + return server +} + +// SetupTestToken creates a token file pointing to the test server +func SetupTestToken(t *testing.T, registryURL, token string) string { + t.Helper() + + homeDir, err := os.UserHomeDir() + require.NoError(t, err) + + tokenPath := filepath.Join(homeDir, commands.TokenFileName) + tokenData := map[string]string{ + "token": token, + "registry": registryURL, + } + + data, err := json.Marshal(tokenData) + require.NoError(t, err) + + err = os.WriteFile(tokenPath, data, 0600) + require.NoError(t, err) + + t.Cleanup(func() { + _ = os.Remove(tokenPath) + }) + + return tokenPath +} + +// CreateTestServerJSON creates a server.json file in a temp directory and changes to it +func CreateTestServerJSON(t *testing.T, serverJSON apiv0.ServerJSON) (string, string) { + t.Helper() + + tempDir, err := os.MkdirTemp("", "mcp-publisher-test") + require.NoError(t, err) + t.Cleanup(func() { os.RemoveAll(tempDir) }) + + jsonData, err := json.MarshalIndent(serverJSON, "", " ") + require.NoError(t, err) + + serverFile := filepath.Join(tempDir, "server.json") + err = os.WriteFile(serverFile, jsonData, 0600) + require.NoError(t, err) + + // Change to temp directory + originalDir, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(originalDir) }) + + err = os.Chdir(tempDir) + require.NoError(t, err) + + return tempDir, serverFile +} diff --git a/cmd/publisher/commands/validate.go b/cmd/publisher/commands/validate.go index ba6bcfa..b68c2b4 100644 --- a/cmd/publisher/commands/validate.go +++ b/cmd/publisher/commands/validate.go @@ -1,9 +1,14 @@ package commands import ( + "bytes" + "context" "encoding/json" "fmt" + "io" + "net/http" "os" + "path/filepath" "strings" "github.com/modelcontextprotocol/registry/internal/validators" @@ -72,17 +77,14 @@ func printSchemaValidationErrors(result *validators.ValidationResult, serverJSON return "" } -// runValidationAndPrintIssues validates the server JSON, prints schema validation errors, and prints all issues. -// Validation failures are always printed (for both validate and publish commands). -// Returns the validation result and a formatted error message string for schema validation errors. -func runValidationAndPrintIssues(serverJSON *apiv0.ServerJSON, opts validators.ValidationOptions) (*validators.ValidationResult, string) { - result := validators.ValidateServerJSON(serverJSON, opts) - +// printValidationIssues prints schema validation errors and all other validation issues. +// Returns the formatted error message string for schema validation errors (empty string if none). +func printValidationIssues(result *validators.ValidationResult, serverJSON *apiv0.ServerJSON) string { // Print schema validation errors/warnings with friendly messages formattedErrorMsg := printSchemaValidationErrors(result, serverJSON) if result.Valid { - return result, "" + return formattedErrorMsg } // Print all issues @@ -108,7 +110,7 @@ func runValidationAndPrintIssues(serverJSON *apiv0.ServerJSON, opts validators.V issueNum++ } - return result, formattedErrorMsg + return formattedErrorMsg } func ValidateCommand(args []string) error { @@ -148,15 +150,97 @@ func ValidateCommand(args []string) error { return fmt.Errorf("invalid JSON: %w", err) } - // Run detailed validation (this is the whole point of the validate command) - // Include schema validation for comprehensive validation - // Warn about non-current schemas (don't error, just inform) - result, _ := runValidationAndPrintIssues(&serverJSON, validators.ValidationAll) + // Get registry URL (same pattern as publish) + homeDir, err := os.UserHomeDir() + if err != nil { + return fmt.Errorf("failed to get home directory: %w", err) + } + + tokenPath := filepath.Join(homeDir, TokenFileName) + registryURL := DefaultRegistryURL + // Try to read registry URL from token file (if it exists) + if tokenData, err := os.ReadFile(tokenPath); err == nil { + var tokenInfo map[string]string + if err := json.Unmarshal(tokenData, &tokenInfo); err == nil { + if url := tokenInfo["registry"]; url != "" { + registryURL = url + } + } + } + + // Validate via API + _, _ = fmt.Fprintf(os.Stdout, "Validating against %s...\n", registryURL) + result, err := validateViaAPI(registryURL, serverData) + if err != nil { + return fmt.Errorf("validation failed: %w", err) + } + + // Print validation results using shared formatting logic + formattedErrorMsg := printValidationIssues(result, &serverJSON) if result.Valid { _, _ = fmt.Fprintln(os.Stdout, "✅ server.json is valid") return nil } + // Return error with formatted message if available + if formattedErrorMsg != "" { + return fmt.Errorf("%s", formattedErrorMsg) + } + return fmt.Errorf("validation failed") } + +// validateViaAPI calls the /validate endpoint on the registry +func validateViaAPI(registryURL string, serverData []byte) (*validators.ValidationResult, error) { + // Parse the server JSON data to ensure it's valid JSON + var serverJSON apiv0.ServerJSON + err := json.Unmarshal(serverData, &serverJSON) + if err != nil { + return nil, fmt.Errorf("error parsing server.json file: %w", err) + } + + // Convert to JSON + jsonData, err := json.Marshal(serverJSON) + if err != nil { + return nil, fmt.Errorf("error serializing request: %w", err) + } + + // Ensure URL ends with / and add validate endpoint + if !strings.HasSuffix(registryURL, "/") { + registryURL += "/" + } + validateURL := registryURL + "v0/validate" + + // Create and send request + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, validateURL, bytes.NewBuffer(jsonData)) + if err != nil { + return nil, fmt.Errorf("error creating request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{} + resp, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("error sending request: %w", err) + } + defer resp.Body.Close() + + // Read response + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("error reading response: %w", err) + } + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("server returned status %d: %s", resp.StatusCode, body) + } + + // Parse response - Huma returns ValidationResult directly + var result validators.ValidationResult + if err := json.Unmarshal(body, &result); err != nil { + return nil, fmt.Errorf("error parsing response: %w", err) + } + + return &result, nil +} diff --git a/cmd/publisher/commands/validate_test.go b/cmd/publisher/commands/validate_test.go new file mode 100644 index 0000000..b56b22d --- /dev/null +++ b/cmd/publisher/commands/validate_test.go @@ -0,0 +1,182 @@ +package commands_test + +import ( + "encoding/json" + "io" + "net/http" + "os" + "testing" + + "github.com/modelcontextprotocol/registry/cmd/publisher/commands" + "github.com/modelcontextprotocol/registry/internal/validators" + apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0" + "github.com/modelcontextprotocol/registry/pkg/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateCommand_Success(t *testing.T) { + validateCallCount := 0 + + server := SetupMockRegistryServer(t, + nil, // No publish handler + func(w http.ResponseWriter, _ *http.Request) { + validateCallCount++ + w.Header().Set("Content-Type", "application/json") + + result := validators.ValidationResult{ + Valid: true, + Issues: []validators.ValidationIssue{}, + } + + _ = json.NewEncoder(w).Encode(result) + }, + ) + + SetupTestToken(t, server.URL, "test-token") + + serverJSON := apiv0.ServerJSON{ + Schema: model.CurrentSchemaURL, + Name: "com.example/test-server", + Description: "A test server", + Version: "1.0.0", + } + CreateTestServerJSON(t, serverJSON) + + err := commands.ValidateCommand([]string{}) + + assert.NoError(t, err) + assert.Equal(t, 1, validateCallCount, "validate endpoint should be called") +} + +func TestValidateCommand_WithErrors(t *testing.T) { + validateCallCount := 0 + + server := SetupMockRegistryServer(t, + nil, + func(w http.ResponseWriter, _ *http.Request) { + validateCallCount++ + w.Header().Set("Content-Type", "application/json") + + result := validators.ValidationResult{ + Valid: false, + Issues: []validators.ValidationIssue{ + { + Type: validators.ValidationIssueTypeSemantic, + Path: "version", + Message: "version must be a specific version, not a range", + Severity: validators.ValidationIssueSeverityError, + Reference: "semantic-version-range", + }, + }, + } + + _ = json.NewEncoder(w).Encode(result) + }, + ) + + SetupTestToken(t, server.URL, "test-token") + + serverJSON := apiv0.ServerJSON{ + Schema: model.CurrentSchemaURL, + Name: "com.example/test-server", + Description: "A test server", + Version: "^1.0.0", // Invalid + } + CreateTestServerJSON(t, serverJSON) + + err := commands.ValidateCommand([]string{}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "validation failed") + assert.Equal(t, 1, validateCallCount, "validate endpoint should be called") +} + +func TestValidateCommand_DeprecatedSchema(t *testing.T) { + server := SetupMockRegistryServer(t, + nil, + func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + body, _ := io.ReadAll(r.Body) + var req apiv0.ServerJSON + _ = json.Unmarshal(body, &req) + + result := validators.ValidationResult{ + Valid: false, + Issues: []validators.ValidationIssue{ + { + Type: validators.ValidationIssueTypeSemantic, + Path: "schema", + Message: "schema version 2025-07-09 is not the current version", + Severity: validators.ValidationIssueSeverityWarning, + Reference: "schema-version-deprecated", + }, + }, + } + + _ = json.NewEncoder(w).Encode(result) + }, + ) + + SetupTestToken(t, server.URL, "test-token") + + serverJSON := apiv0.ServerJSON{ + Schema: "https://static.modelcontextprotocol.io/schemas/2025-07-09/server.schema.json", + Name: "com.example/test-server", + Description: "A test server", + Version: "1.0.0", + } + CreateTestServerJSON(t, serverJSON) + + err := commands.ValidateCommand([]string{}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "schema version 2025-07-09") + assert.Contains(t, err.Error(), "Migration checklist:") +} + +func TestValidateCommand_NoServerFile(t *testing.T) { + server := SetupMockRegistryServer(t, nil, nil) + SetupTestToken(t, server.URL, "test-token") + + // Don't create server.json + tempDir, err := os.MkdirTemp("", "mcp-publisher-test") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + originalDir, err := os.Getwd() + require.NoError(t, err) + defer func() { _ = os.Chdir(originalDir) }() + + _ = os.Chdir(tempDir) + + err = commands.ValidateCommand([]string{}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "not found") +} + +func TestValidateCommand_InvalidJSON(t *testing.T) { + server := SetupMockRegistryServer(t, nil, nil) + SetupTestToken(t, server.URL, "test-token") + + tempDir, err := os.MkdirTemp("", "mcp-publisher-test") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + originalDir, err := os.Getwd() + require.NoError(t, err) + defer func() { _ = os.Chdir(originalDir) }() + + _ = os.Chdir(tempDir) + + // Create invalid JSON file + err = os.WriteFile("server.json", []byte("{ invalid json }"), 0600) + require.NoError(t, err) + + err = commands.ValidateCommand([]string{}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid JSON") +} diff --git a/internal/api/handlers/v0/edit.go b/internal/api/handlers/v0/edit.go index 9c0286d..5c519e2 100644 --- a/internal/api/handlers/v0/edit.go +++ b/internal/api/handlers/v0/edit.go @@ -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) diff --git a/internal/api/handlers/v0/publish.go b/internal/api/handlers/v0/publish.go index 7631ae3..5bd371e 100644 --- a/internal/api/handlers/v0/publish.go +++ b/internal/api/handlers/v0/publish.go @@ -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 { diff --git a/internal/api/handlers/v0/publish_test.go b/internal/api/handlers/v0/publish_test.go index 0a00659..94502be 100644 --- a/internal/api/handlers/v0/publish_test.go +++ b/internal/api/handlers/v0/publish_test.go @@ -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 { diff --git a/internal/api/handlers/v0/validate.go b/internal/api/handlers/v0/validate.go new file mode 100644 index 0000000..0601f14 --- /dev/null +++ b/internal/api/handlers/v0/validate.go @@ -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 + }) +} diff --git a/internal/api/handlers/v0/validate_test.go b/internal/api/handlers/v0/validate_test.go new file mode 100644 index 0000000..4436d99 --- /dev/null +++ b/internal/api/handlers/v0/validate_test.go @@ -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) + } + } + }) + } +} diff --git a/internal/api/router/v0.go b/internal/api/router/v0.go index eae032d..62bc46e 100644 --- a/internal/api/router/v0.go +++ b/internal/api/router/v0.go @@ -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") } diff --git a/internal/validators/validators.go b/internal/validators/validators.go index 2081f0b..4da756d 100644 --- a/internal/validators/validators.go +++ b/internal/validators/validators.go @@ -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