From 6431f23107ea42eafc42faa54f89194f7c0daa82 Mon Sep 17 00:00:00 2001 From: adam jones Date: Wed, 27 Aug 2025 21:28:04 +0100 Subject: [PATCH] feat: add admin edit endpoint for server moderation (#315) ## Summary Implements `PUT /v0/servers/{id}` admin edit endpoint to address part of issue #182, providing an API-based solution for server moderation without requiring direct database access. - **New endpoint**: `PUT /v0/servers/{id}` for editing existing servers - **Security**: Uses existing JWT auth with `PermissionActionEdit` permissions - **Anti-undelete protection**: Added `ServerStatusDeleted` status that cannot be reverted - **Full implementation**: Database, service, API, and comprehensive test coverage ## Key Use Cases - Mark malicious servers as `deleted` for takedown - Update server metadata for moderation actions Related to: https://github.com/modelcontextprotocol/registry/issues/182 (but that is not _done_ yet because we need admins to be able to login (probably with #283), and need to add docs on how to do the takedown, and probably guidelines on what we will/won't takedown) --- docs/server-json/server.schema.json | 4 +- internal/api/handlers/v0/edit.go | 110 +++++++ internal/api/handlers/v0/edit_test.go | 348 +++++++++++++++++++++++ internal/api/handlers/v0/publish_test.go | 8 + internal/api/router/v0.go | 1 + internal/database/database.go | 2 + internal/database/memory.go | 23 ++ internal/database/postgres.go | 90 ++++++ internal/model/model.go | 23 +- internal/service/fake_service.go | 29 ++ internal/service/registry_service.go | 62 +++- internal/service/service.go | 4 +- 12 files changed, 681 insertions(+), 23 deletions(-) create mode 100644 internal/api/handlers/v0/edit.go create mode 100644 internal/api/handlers/v0/edit_test.go diff --git a/docs/server-json/server.schema.json b/docs/server-json/server.schema.json index 0135ba5..3575e42 100644 --- a/docs/server-json/server.schema.json +++ b/docs/server-json/server.schema.json @@ -66,9 +66,9 @@ }, "status": { "type": "string", - "enum": ["active", "deprecated"], + "enum": ["active", "deprecated", "deleted"], "default": "active", - "description": "Server lifecycle status. 'deprecated' indicates the server is no longer recommended for new usage." + "description": "Server lifecycle status. 'deprecated' indicates the server is no longer recommended for new usage. 'deleted' indicates the server should never be installed and existing installations should be uninstalled - this is rare, and usually indicates malware or a legal takedown." }, "repository": { "$ref": "#/$defs/Repository" diff --git a/internal/api/handlers/v0/edit.go b/internal/api/handlers/v0/edit.go new file mode 100644 index 0000000..8307e21 --- /dev/null +++ b/internal/api/handlers/v0/edit.go @@ -0,0 +1,110 @@ +package v0 + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "strings" + + "github.com/danielgtaylor/huma/v2" + "github.com/modelcontextprotocol/registry/internal/auth" + "github.com/modelcontextprotocol/registry/internal/config" + "github.com/modelcontextprotocol/registry/internal/database" + "github.com/modelcontextprotocol/registry/internal/model" + "github.com/modelcontextprotocol/registry/internal/service" + "github.com/modelcontextprotocol/registry/internal/validators" +) + +// EditServerInput represents the input for editing a server +type EditServerInput struct { + Authorization string `header:"Authorization" doc:"Registry JWT token with edit permissions" required:"true"` + ID string `path:"id" doc:"Server ID (UUID)" format:"uuid"` + RawBody []byte `body:"raw"` +} + +// RegisterEditEndpoints registers the edit endpoint +func RegisterEditEndpoints(api huma.API, registry service.RegistryService, cfg *config.Config) { + jwtManager := auth.NewJWTManager(cfg) + + // Edit server endpoint + huma.Register(api, huma.Operation{ + OperationID: "edit-server", + Method: http.MethodPut, + Path: "/v0/servers/{id}", + Summary: "Edit MCP server", + Description: "Update an existing MCP server (admin only)", + Tags: []string{"admin"}, + Security: []map[string][]string{ + {"bearer": {}}, + }, + }, func(ctx context.Context, input *EditServerInput) (*Response[model.ServerResponse], error) { + // Extract bearer token + const bearerPrefix = "Bearer " + authHeader := input.Authorization + if len(authHeader) < len(bearerPrefix) || !strings.EqualFold(authHeader[:len(bearerPrefix)], bearerPrefix) { + return nil, huma.Error401Unauthorized("Invalid Authorization header format. Expected 'Bearer '") + } + token := authHeader[len(bearerPrefix):] + + // Validate Registry JWT token + claims, err := jwtManager.ValidateToken(ctx, token) + if err != nil { + return nil, huma.Error401Unauthorized("Invalid or expired Registry JWT token", err) + } + + // Validate that only allowed extension fields are present + if err := model.ValidatePublishRequestExtensions(input.RawBody); err != nil { + return nil, huma.Error400BadRequest("Invalid request format", err) + } + + // Get current server to check permissions against existing name + currentServer, err := registry.GetByID(input.ID) + if err != nil { + if errors.Is(err, database.ErrNotFound) { + return nil, huma.Error404NotFound("Server not found") + } + return nil, huma.Error500InternalServerError("Failed to get current server", err) + } + + // Verify edit permissions for this server using the existing server name + if !jwtManager.HasPermission(currentServer.Server.Name, auth.PermissionActionEdit, claims.Permissions) { + return nil, huma.Error403Forbidden("You do not have edit permissions for this server") + } + + // Parse the validated request body + var editRequest model.PublishRequest + if err := json.Unmarshal(input.RawBody, &editRequest); err != nil { + return nil, huma.Error400BadRequest("Invalid JSON format", err) + } + + // Validate the server detail + validator := validators.NewObjectValidator() + if err := validator.Validate(&editRequest.Server); err != nil { + return nil, huma.Error400BadRequest(err.Error()) + } + + // Prevent renaming servers + if currentServer.Server.Name != editRequest.Server.Name { + return nil, huma.Error400BadRequest("Cannot rename server") + } + + // Prevent undeleting servers - once deleted, they stay deleted + if currentServer.Server.Status == model.ServerStatusDeleted && editRequest.Server.Status != model.ServerStatusDeleted { + return nil, huma.Error400BadRequest("Cannot change status of deleted server. Deleted servers cannot be undeleted.") + } + + // Edit the server + updatedServer, err := registry.EditServer(input.ID, editRequest) + if err != nil { + if errors.Is(err, database.ErrNotFound) { + return nil, huma.Error404NotFound("Server not found") + } + return nil, huma.Error400BadRequest("Failed to edit server", err) + } + + return &Response[model.ServerResponse]{ + Body: *updatedServer, + }, nil + }) +} diff --git a/internal/api/handlers/v0/edit_test.go b/internal/api/handlers/v0/edit_test.go new file mode 100644 index 0000000..820deae --- /dev/null +++ b/internal/api/handlers/v0/edit_test.go @@ -0,0 +1,348 @@ +package v0_test + +import ( + "bytes" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/danielgtaylor/huma/v2" + "github.com/danielgtaylor/huma/v2/adapters/humago" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + v0 "github.com/modelcontextprotocol/registry/internal/api/handlers/v0" + "github.com/modelcontextprotocol/registry/internal/auth" + "github.com/modelcontextprotocol/registry/internal/config" + "github.com/modelcontextprotocol/registry/internal/database" + "github.com/modelcontextprotocol/registry/internal/model" +) + +func TestEditServerEndpoint(t *testing.T) { + testCases := []struct { + name string + serverID string + authHeader string + requestBody interface{} + setupMocks func(*MockRegistryService) + expectedStatus int + expectedError string + }{ + { + name: "successful edit with valid token and permissions", + serverID: "550e8400-e29b-41d4-a716-446655440001", + authHeader: func() string { + cfg := &config.Config{JWTPrivateKey: "bb2c6b424005acd5df47a9e2c87f446def86dd740c888ea3efb825b23f7ef47c"} + token, _ := generateTestJWTToken(cfg, auth.JWTClaims{ + AuthMethod: model.AuthMethodGitHubAT, + AuthMethodSubject: "domdomegg", + Permissions: []auth.Permission{ + {Action: auth.PermissionActionEdit, ResourcePattern: "io.github.domdomegg/*"}, + }, + }) + return "Bearer " + token + }(), + requestBody: model.PublishRequest{ + Server: model.ServerDetail{ + Name: "io.github.domdomegg/test-server", + Description: "Updated test server", + Status: model.ServerStatusDeprecated, + Repository: model.Repository{ + URL: "https://github.com/domdomegg/test-server", + Source: "github", + ID: "domdomegg/test-server", + }, + VersionDetail: model.VersionDetail{ + Version: "1.0.1", + }, + }, + }, + setupMocks: func(registry *MockRegistryService) { + // Current server (not deleted) + currentServer := &model.ServerResponse{ + Server: model.ServerDetail{ + Name: "io.github.domdomegg/test-server", + Description: "Original server", + Status: model.ServerStatusActive, + }, + } + registry.On("GetByID", "550e8400-e29b-41d4-a716-446655440001").Return(currentServer, nil) + + expectedResponse := &model.ServerResponse{ + Server: model.ServerDetail{ + Name: "io.github.domdomegg/test-server", + Description: "Updated test server", + Status: model.ServerStatusDeprecated, + }, + } + registry.On("EditServer", "550e8400-e29b-41d4-a716-446655440001", mock.AnythingOfType("model.PublishRequest")).Return(expectedResponse, nil) + }, + expectedStatus: http.StatusOK, + }, + { + name: "missing authorization header", + serverID: "550e8400-e29b-41d4-a716-446655440001", + authHeader: "", + requestBody: model.PublishRequest{}, + setupMocks: func(_ *MockRegistryService) {}, + expectedStatus: 422, + expectedError: "required header parameter is missing", + }, + { + name: "invalid authorization header format", + serverID: "550e8400-e29b-41d4-a716-446655440001", + authHeader: "InvalidFormat token123", + requestBody: model.PublishRequest{}, + setupMocks: func(_ *MockRegistryService) {}, + expectedStatus: http.StatusUnauthorized, + expectedError: "Unauthorized", + }, + { + name: "invalid token", + serverID: "550e8400-e29b-41d4-a716-446655440001", + authHeader: "Bearer invalid-token", + requestBody: model.PublishRequest{}, + setupMocks: func(_ *MockRegistryService) {}, + expectedStatus: http.StatusUnauthorized, + expectedError: "Unauthorized", + }, + { + name: "permission denied - no edit permissions", + serverID: "550e8400-e29b-41d4-a716-446655440001", + authHeader: func() string { + cfg := &config.Config{JWTPrivateKey: "bb2c6b424005acd5df47a9e2c87f446def86dd740c888ea3efb825b23f7ef47c"} + token, _ := generateTestJWTToken(cfg, auth.JWTClaims{ + AuthMethod: model.AuthMethodGitHubAT, + AuthMethodSubject: "domdomegg", + Permissions: []auth.Permission{ + {Action: auth.PermissionActionPublish, ResourcePattern: "io.github.domdomegg/*"}, + }, + }) + return "Bearer " + token + }(), + requestBody: model.PublishRequest{ + Server: model.ServerDetail{ + Name: "io.github.domdomegg/test-server", + Description: "Updated test server", + }, + }, + setupMocks: func(registry *MockRegistryService) { + // Need to mock GetByID since we check permissions against existing server name + currentServer := &model.ServerResponse{ + Server: model.ServerDetail{ + Name: "io.github.domdomegg/test-server", + Description: "Original server", + Status: model.ServerStatusActive, + }, + } + registry.On("GetByID", "550e8400-e29b-41d4-a716-446655440001").Return(currentServer, nil) + }, + expectedStatus: http.StatusForbidden, + expectedError: "Forbidden", + }, + { + name: "permission denied - wrong resource", + serverID: "550e8400-e29b-41d4-a716-446655440001", + authHeader: func() string { + cfg := &config.Config{JWTPrivateKey: "bb2c6b424005acd5df47a9e2c87f446def86dd740c888ea3efb825b23f7ef47c"} + token, _ := generateTestJWTToken(cfg, auth.JWTClaims{ + AuthMethod: model.AuthMethodGitHubAT, + AuthMethodSubject: "domdomegg", + Permissions: []auth.Permission{ + {Action: auth.PermissionActionEdit, ResourcePattern: "io.github.domdomegg/*"}, + }, + }) + return "Bearer " + token + }(), + requestBody: model.PublishRequest{ + Server: model.ServerDetail{ + Name: "io.github.other/test-server", + Description: "Updated test server", + }, + }, + setupMocks: func(registry *MockRegistryService) { + // Need to mock GetByID since we check permissions against existing server name + // This test case shows a different scenario: existing server is "other" but user only has perms for "domdomegg" + currentServer := &model.ServerResponse{ + Server: model.ServerDetail{ + Name: "io.github.other/test-server", + Description: "Original server", + Status: model.ServerStatusActive, + }, + } + registry.On("GetByID", "550e8400-e29b-41d4-a716-446655440001").Return(currentServer, nil) + }, + expectedStatus: http.StatusForbidden, + expectedError: "Forbidden", + }, + { + name: "server not found", + serverID: "550e8400-e29b-41d4-a716-446655440001", + authHeader: func() string { + cfg := &config.Config{JWTPrivateKey: "bb2c6b424005acd5df47a9e2c87f446def86dd740c888ea3efb825b23f7ef47c"} + token, _ := generateTestJWTToken(cfg, auth.JWTClaims{ + AuthMethod: model.AuthMethodGitHubAT, + AuthMethodSubject: "domdomegg", + Permissions: []auth.Permission{ + {Action: auth.PermissionActionEdit, ResourcePattern: "io.github.domdomegg/*"}, + }, + }) + return "Bearer " + token + }(), + requestBody: model.PublishRequest{ + Server: model.ServerDetail{ + Name: "io.github.domdomegg/test-server", + Description: "Updated test server", + }, + }, + setupMocks: func(registry *MockRegistryService) { + registry.On("GetByID", "550e8400-e29b-41d4-a716-446655440001").Return(nil, database.ErrNotFound) + }, + expectedStatus: http.StatusNotFound, + expectedError: "Not Found", + }, + { + name: "service error", + serverID: "550e8400-e29b-41d4-a716-446655440001", + authHeader: func() string { + cfg := &config.Config{JWTPrivateKey: "bb2c6b424005acd5df47a9e2c87f446def86dd740c888ea3efb825b23f7ef47c"} + token, _ := generateTestJWTToken(cfg, auth.JWTClaims{ + AuthMethod: model.AuthMethodGitHubAT, + AuthMethodSubject: "domdomegg", + Permissions: []auth.Permission{ + {Action: auth.PermissionActionEdit, ResourcePattern: "*"}, + }, + }) + return "Bearer " + token + }(), + requestBody: model.PublishRequest{ + Server: model.ServerDetail{ + Name: "io.github.domdomegg/test-server", + Description: "Updated test server", + }, + }, + setupMocks: func(registry *MockRegistryService) { + // Current server (not deleted) + currentServer := &model.ServerResponse{ + Server: model.ServerDetail{ + Name: "io.github.domdomegg/test-server", + Description: "Original server", + Status: model.ServerStatusActive, + }, + } + registry.On("GetByID", "550e8400-e29b-41d4-a716-446655440001").Return(currentServer, nil) + registry.On("EditServer", "550e8400-e29b-41d4-a716-446655440001", mock.AnythingOfType("model.PublishRequest")).Return(nil, errors.New("database error")) + }, + expectedStatus: http.StatusBadRequest, + expectedError: "Bad Request", + }, + { + name: "invalid request body", + serverID: "550e8400-e29b-41d4-a716-446655440001", + authHeader: func() string { + cfg := &config.Config{JWTPrivateKey: "bb2c6b424005acd5df47a9e2c87f446def86dd740c888ea3efb825b23f7ef47c"} + token, _ := generateTestJWTToken(cfg, auth.JWTClaims{ + AuthMethod: model.AuthMethodGitHubAT, + AuthMethodSubject: "domdomegg", + Permissions: []auth.Permission{ + {Action: auth.PermissionActionEdit, ResourcePattern: "*"}, + }, + }) + return "Bearer " + token + }(), + requestBody: "invalid json", + setupMocks: func(_ *MockRegistryService) {}, + expectedStatus: http.StatusBadRequest, + expectedError: "Bad Request", + }, + { + name: "cannot undelete server", + serverID: "550e8400-e29b-41d4-a716-446655440001", + authHeader: func() string { + cfg := &config.Config{JWTPrivateKey: "bb2c6b424005acd5df47a9e2c87f446def86dd740c888ea3efb825b23f7ef47c"} + token, _ := generateTestJWTToken(cfg, auth.JWTClaims{ + AuthMethod: model.AuthMethodGitHubAT, + AuthMethodSubject: "domdomegg", + Permissions: []auth.Permission{ + {Action: auth.PermissionActionEdit, ResourcePattern: "*"}, + }, + }) + return "Bearer " + token + }(), + requestBody: model.PublishRequest{ + Server: model.ServerDetail{ + Name: "io.github.domdomegg/test-server", + Description: "Trying to undelete server", + Status: model.ServerStatusActive, + }, + }, + setupMocks: func(registry *MockRegistryService) { + // Current server is deleted + currentServer := &model.ServerResponse{ + Server: model.ServerDetail{ + Name: "io.github.domdomegg/test-server", + Description: "Original server", + Status: model.ServerStatusDeleted, + }, + } + registry.On("GetByID", "550e8400-e29b-41d4-a716-446655440001").Return(currentServer, nil) + }, + expectedStatus: http.StatusBadRequest, + expectedError: "Cannot change status of deleted server", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create mock registry + mockRegistry := new(MockRegistryService) + tc.setupMocks(mockRegistry) + + // Create Huma API + mux := http.NewServeMux() + humaConfig := huma.DefaultConfig("Test API", "1.0.0") + api := humago.New(mux, humaConfig) + + // Register edit endpoints + cfg := &config.Config{ + JWTPrivateKey: "bb2c6b424005acd5df47a9e2c87f446def86dd740c888ea3efb825b23f7ef47c", + } + v0.RegisterEditEndpoints(api, mockRegistry, cfg) + + // Create request body + var requestBody []byte + var err error + if str, ok := tc.requestBody.(string); ok { + requestBody = []byte(str) + } else { + requestBody, err = json.Marshal(tc.requestBody) + assert.NoError(t, err) + } + + // Create request + req := httptest.NewRequest(http.MethodPut, "/v0/servers/"+tc.serverID, bytes.NewReader(requestBody)) + req.Header.Set("Content-Type", "application/json") + if tc.authHeader != "" { + req.Header.Set("Authorization", tc.authHeader) + } + + // Create response recorder + w := httptest.NewRecorder() + + // Call the endpoint + mux.ServeHTTP(w, req) + + // Check status code + assert.Equal(t, tc.expectedStatus, w.Code) + + // Check error message if expected + if tc.expectedError != "" { + assert.Contains(t, w.Body.String(), tc.expectedError) + } + + mockRegistry.AssertExpectations(t) + }) + } +} \ No newline at end of file diff --git a/internal/api/handlers/v0/publish_test.go b/internal/api/handlers/v0/publish_test.go index ed7d910..48b2b69 100644 --- a/internal/api/handlers/v0/publish_test.go +++ b/internal/api/handlers/v0/publish_test.go @@ -49,6 +49,14 @@ func (m *MockRegistryService) Publish(request model.PublishRequest) (*model.Serv return args.Get(0).(*model.ServerResponse), args.Error(1) } +func (m *MockRegistryService) EditServer(id string, request model.PublishRequest) (*model.ServerResponse, error) { + args := m.Called(id, request) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*model.ServerResponse), args.Error(1) +} + // Helper function to generate a valid JWT token for testing func generateTestJWTToken(cfg *config.Config, claims auth.JWTClaims) (string, error) { jwtManager := auth.NewJWTManager(cfg) diff --git a/internal/api/router/v0.go b/internal/api/router/v0.go index d35df48..d1735a4 100644 --- a/internal/api/router/v0.go +++ b/internal/api/router/v0.go @@ -17,6 +17,7 @@ func RegisterV0Routes( v0.RegisterHealthEndpoint(api, cfg, metrics) v0.RegisterPingEndpoint(api) v0.RegisterServersEndpoints(api, registry) + v0.RegisterEditEndpoints(api, registry, cfg) v0auth.RegisterAuthEndpoints(api, cfg) v0.RegisterPublishEndpoint(api, registry, cfg) } diff --git a/internal/database/database.go b/internal/database/database.go index 8e07ce5..904720b 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -28,6 +28,8 @@ type Database interface { Publish(ctx context.Context, serverDetail model.ServerDetail, publisherExtensions map[string]interface{}, registryMetadata model.RegistryMetadata) (*model.ServerRecord, error) // UpdateLatestFlag updates the is_latest flag for a specific server record UpdateLatestFlag(ctx context.Context, id string, isLatest bool) error + // UpdateServer updates an existing server record with new server details + UpdateServer(ctx context.Context, id string, serverDetail model.ServerDetail, publisherExtensions map[string]interface{}) (*model.ServerRecord, error) // ImportSeed imports initial data from a seed file ImportSeed(ctx context.Context, seedFilePath string) error // Close closes the database connection diff --git a/internal/database/memory.go b/internal/database/memory.go index 72790f9..2f36dba 100644 --- a/internal/database/memory.go +++ b/internal/database/memory.go @@ -246,6 +246,29 @@ func (db *MemoryDB) UpdateLatestFlag(ctx context.Context, id string, isLatest bo return ErrNotFound } +// UpdateServer updates an existing server record with new server details +func (db *MemoryDB) UpdateServer(ctx context.Context, id string, serverDetail model.ServerDetail, publisherExtensions map[string]interface{}) (*model.ServerRecord, error) { + if ctx.Err() != nil { + return nil, ctx.Err() + } + + db.mu.Lock() + defer db.mu.Unlock() + + entry, exists := db.entries[id] + if !exists { + return nil, ErrNotFound + } + + // Update the server details + entry.ServerJSON = serverDetail + entry.PublisherExtensions = publisherExtensions + entry.RegistryMetadata.UpdatedAt = time.Now() + + // Return the updated record + return entry, nil +} + // Close closes the database connection // For an in-memory database, this is a no-op func (db *MemoryDB) Close() error { diff --git a/internal/database/postgres.go b/internal/database/postgres.go index 7088699..e012f08 100644 --- a/internal/database/postgres.go +++ b/internal/database/postgres.go @@ -531,6 +531,96 @@ func (db *PostgreSQL) UpdateLatestFlag(ctx context.Context, id string, isLatest return nil } +// UpdateServer updates an existing server record with new server details +func (db *PostgreSQL) UpdateServer(ctx context.Context, id string, serverDetail model.ServerDetail, publisherExtensions map[string]interface{}) (*model.ServerRecord, error) { + if ctx.Err() != nil { + return nil, ctx.Err() + } + + // First check if the server exists + _, err := db.GetByID(ctx, id) + if err != nil { + return nil, err + } + + // Start a transaction for atomic updates + tx, err := db.conn.Begin(ctx) + if err != nil { + return nil, fmt.Errorf("failed to begin transaction: %w", err) + } + defer func() { + if rollbackErr := tx.Rollback(ctx); rollbackErr != nil && !errors.Is(rollbackErr, pgx.ErrTxClosed) { + log.Printf("Failed to rollback transaction: %v", rollbackErr) + } + }() + + // Prepare JSON data + repositoryJSON, err := json.Marshal(serverDetail.Repository) + if err != nil { + return nil, fmt.Errorf("failed to marshal repository: %w", err) + } + + packagesJSON, err := json.Marshal(serverDetail.Packages) + if err != nil { + return nil, fmt.Errorf("failed to marshal packages: %w", err) + } + + remotesJSON, err := json.Marshal(serverDetail.Remotes) + if err != nil { + return nil, fmt.Errorf("failed to marshal remotes: %w", err) + } + + publisherExtensionsJSON, err := json.Marshal(publisherExtensions) + if err != nil { + return nil, fmt.Errorf("failed to marshal publisher extensions: %w", err) + } + + now := time.Now() + + // Update servers table + serverQuery := ` + UPDATE servers + SET name = $1, description = $2, status = $3, repository = $4, + version = $5, packages = $6, remotes = $7, updated_at = $8 + WHERE id = $9 + ` + + _, err = tx.Exec(ctx, serverQuery, + serverDetail.Name, + serverDetail.Description, + serverDetail.Status, + repositoryJSON, + serverDetail.VersionDetail.Version, + packagesJSON, + remotesJSON, + now, + id, + ) + if err != nil { + return nil, fmt.Errorf("failed to update server: %w", err) + } + + // Update server_extensions table + extQuery := ` + UPDATE server_extensions + SET updated_at = $1, publisher_extensions = $2 + WHERE server_id = $3 + ` + + _, err = tx.Exec(ctx, extQuery, now, publisherExtensionsJSON, id) + if err != nil { + return nil, fmt.Errorf("failed to update server extensions: %w", err) + } + + // Commit transaction + if err := tx.Commit(ctx); err != nil { + return nil, fmt.Errorf("failed to commit transaction: %w", err) + } + + // Return the updated record + return db.GetByID(ctx, id) +} + // Close closes the database connection func (db *PostgreSQL) Close() error { return db.conn.Close(context.Background()) diff --git a/internal/model/model.go b/internal/model/model.go index 0298ecb..adca232 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -31,10 +31,9 @@ const ( type ServerStatus string const ( - // ServerStatusActive represents an actively maintained server (as asserted by the publisher) - ServerStatusActive ServerStatus = "active" - // ServerStatusDeprecated represents a server that is no longer actively maintained + ServerStatusActive ServerStatus = "active" ServerStatusDeprecated ServerStatus = "deprecated" + ServerStatusDeleted ServerStatus = "deleted" ) // Repository represents a source code repository as defined in the spec @@ -91,17 +90,17 @@ type Argument struct { type Package struct { // RegistryType indicates how to download packages (e.g., "npm", "pypi", "docker-hub", "mcpb") - RegistryType string `json:"registry_type,omitempty" bson:"registry_type,omitempty"` + RegistryType string `json:"registry_type,omitempty" bson:"registry_type,omitempty"` // RegistryBaseURL is the base URL of the package registry - RegistryBaseURL string `json:"registry_base_url,omitempty" bson:"registry_base_url,omitempty"` + RegistryBaseURL string `json:"registry_base_url,omitempty" bson:"registry_base_url,omitempty"` // Identifier is the package identifier - either a package name (for registries) or URL (for direct downloads) - Identifier string `json:"identifier,omitempty" bson:"identifier,omitempty"` - Version string `json:"version,omitempty" bson:"version,omitempty"` - FileSHA256 string `json:"file_sha256,omitempty" bson:"file_sha256,omitempty"` - RunTimeHint string `json:"runtime_hint,omitempty" bson:"runtime_hint,omitempty"` - RuntimeArguments []Argument `json:"runtime_arguments,omitempty" bson:"runtime_arguments,omitempty"` - PackageArguments []Argument `json:"package_arguments,omitempty" bson:"package_arguments,omitempty"` - EnvironmentVariables []KeyValueInput `json:"environment_variables,omitempty" bson:"environment_variables,omitempty"` + Identifier string `json:"identifier,omitempty" bson:"identifier,omitempty"` + Version string `json:"version,omitempty" bson:"version,omitempty"` + FileSHA256 string `json:"file_sha256,omitempty" bson:"file_sha256,omitempty"` + RunTimeHint string `json:"runtime_hint,omitempty" bson:"runtime_hint,omitempty"` + RuntimeArguments []Argument `json:"runtime_arguments,omitempty" bson:"runtime_arguments,omitempty"` + PackageArguments []Argument `json:"package_arguments,omitempty" bson:"package_arguments,omitempty"` + EnvironmentVariables []KeyValueInput `json:"environment_variables,omitempty" bson:"environment_variables,omitempty"` } // Remote represents a remote connection endpoint diff --git a/internal/service/fake_service.go b/internal/service/fake_service.go index 3f33322..53906e6 100644 --- a/internal/service/fake_service.go +++ b/internal/service/fake_service.go @@ -146,6 +146,35 @@ func (s *fakeRegistryService) Publish(req model.PublishRequest) (*model.ServerRe return &response, nil } +// EditServer updates an existing server with new details (admin operation) +func (s *fakeRegistryService) EditServer(id string, req model.PublishRequest) (*model.ServerResponse, error) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // Validate the request + if err := model.ValidatePublisherExtensions(req); err != nil { + return nil, err + } + + // Validate server name exists and format + if _, err := model.ParseServerName(req.Server); err != nil { + return nil, err + } + + // Extract publisher extensions from request + publisherExtensions := model.ExtractPublisherExtensions(req) + + // Update server in database + serverRecord, err := s.db.UpdateServer(ctx, id, req.Server, publisherExtensions) + if err != nil { + return nil, err + } + + // Convert ServerRecord to ServerResponse format + response := serverRecord.ToServerResponse() + return &response, nil +} + // Close closes the in-memory database connection func (s *fakeRegistryService) Close() error { return s.db.Close() diff --git a/internal/service/registry_service.go b/internal/service/registry_service.go index 3752bf7..7f99bbc 100644 --- a/internal/service/registry_service.go +++ b/internal/service/registry_service.go @@ -90,36 +90,36 @@ func validateMCPBPackage(host string) error { if !isAllowed { return fmt.Errorf("MCPB packages must be hosted on allowlisted providers (GitHub or GitLab). Host '%s' is not allowed", host) } - + return nil } // validatePackage validates packages to ensure they meet requirements func validatePackage(pkg *model.Package) error { registryType := strings.ToLower(pkg.RegistryType) - + // For direct download packages (mcpb or direct URLs) - if registryType == "mcpb" || - strings.HasPrefix(pkg.Identifier, "http://") || strings.HasPrefix(pkg.Identifier, "https://") { + if registryType == "mcpb" || + strings.HasPrefix(pkg.Identifier, "http://") || strings.HasPrefix(pkg.Identifier, "https://") { parsedURL, err := url.Parse(pkg.Identifier) if err != nil { return fmt.Errorf("invalid package URL: %w", err) } - + host := strings.ToLower(parsedURL.Host) - + // For MCPB packages, validate they're from allowed hosts if registryType == "mcpb" || strings.HasSuffix(strings.ToLower(pkg.Identifier), ".mcpb") { return validateMCPBPackage(host) } - + // For other URL-based packages, just ensure it's valid if parsedURL.Scheme == "" || parsedURL.Host == "" { return fmt.Errorf("package URL must be a valid absolute URL") } return nil } - + // For registry-based packages, no special validation needed // Registry types like "npm", "pypi", "docker-hub", "nuget" are all valid return nil @@ -254,3 +254,49 @@ func (s *registryServiceImpl) validateNoDuplicateRemoteURLs(ctx context.Context, return nil } + +// EditServer updates an existing server with new details (admin operation) +func (s *registryServiceImpl) EditServer(id string, req model.PublishRequest) (*model.ServerResponse, error) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // Validate the request + if err := model.ValidatePublisherExtensions(req); err != nil { + return nil, err + } + + // Validate server name exists and format + if _, err := model.ParseServerName(req.Server); err != nil { + return nil, err + } + + // Validate all packages + for _, pkg := range req.Server.Packages { + if err := validatePackage(&pkg); err != nil { + return nil, fmt.Errorf("validation failed: %w", err) + } + } + + // Validate reverse-DNS namespace matching for remote URLs + if err := model.ValidateRemoteNamespaceMatch(req.Server); err != nil { + return nil, err + } + + // Check for duplicate remote URLs + if err := s.validateNoDuplicateRemoteURLs(ctx, req.Server); err != nil { + return nil, err + } + + // Extract publisher extensions from request + publisherExtensions := model.ExtractPublisherExtensions(req) + + // Update server in database + serverRecord, err := s.db.UpdateServer(ctx, id, req.Server, publisherExtensions) + if err != nil { + return nil, err + } + + // Convert ServerRecord to ServerResponse format + response := serverRecord.ToServerResponse() + return &response, nil +} diff --git a/internal/service/service.go b/internal/service/service.go index 454727b..6826506 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -6,8 +6,10 @@ import "github.com/modelcontextprotocol/registry/internal/model" type RegistryService interface { // List retrieves servers with extension wrapper format List(cursor string, limit int) ([]model.ServerResponse, string, error) - // GetByID retrieves a single server by registry metadata ID with extension wrapper format + // GetByID retrieves a single server by registry metadata ID with extension wrapper format GetByID(id string) (*model.ServerResponse, error) // Publish publishes a server with separated extensions Publish(req model.PublishRequest) (*model.ServerResponse, error) + // EditServer updates an existing server with new details + EditServer(id string, req model.PublishRequest) (*model.ServerResponse, error) }