modelcontextprotocol/registry
Watch
1
0
Fork
You've already forked registry
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: add server name validation and clean invalid data before migration 009 (#591)

Browse source 
## Summary

This PR adds two important improvements to ensure data integrity:

1. **Adds server name format validation at the API layer** to match the
database constraint from migration 008
2. **Includes migration 008 to clean up invalid data** before applying
strict constraints in migration 009

## Changes

### Server Name Validation
- Enforces validation at the API layer matching the database constraint
- **Namespace pattern**: `[a-zA-Z0-9][a-zA-Z0-9.-]*[a-zA-Z0-9]` 
  - Must start and end with alphanumeric characters
  - Can contain dots and hyphens in the middle
- **Name pattern**: `[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9]`
  - Must start and end with alphanumeric characters  
  - Can contain dots, underscores, and hyphens in the middle
- Provides clear, user-friendly error messages indicating which part is
invalid
- Uses DRY approach with composed regex patterns

### Migration 008: Data Cleanup
- Removes servers with invalid names or empty versions (5 servers
affected in production)
- Fixes invalid status values by setting them to 'active' (1 server
affected)
- Removes duplicate name+version combinations
- Includes comprehensive safety checks to prevent accidental data loss
- Must run before migration 009 which adds strict constraints

## Testing
- All validator tests pass
- Migration includes safety checks that match production data
- CI verification included

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Tadas Antanavicius 2025-09-30 10:07:42 -07:00 • committed by GitHub
commit 272ee9322d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 230 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

1
go.mod
View file

@ -32,6 +32,7 @@ require (
github.com/jackc/pgpassfile v1.0.0 // indirect
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect
github.com/jackc/puddle/v2 v2.2.2 // indirect
github.com/lib/pq v1.10.9 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_model v0.6.2 // indirect

2
go.sum
View file

@ -42,6 +42,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw=
github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=

197
internal/database/migrations/008_clean_invalid_data.sql Normal file
View file

@ -0,0 +1,197 @@
-- Migration 008: Clean up invalid data before applying stricter constraints in migration 009
-- This migration removes or fixes data that would violate constraints introduced in the next migration
BEGIN;
-- Safety check: Count exactly what we'll be modifying
DO $$
DECLARE
invalid_name_count INTEGER;
empty_version_count INTEGER;
invalid_status_count INTEGER;
duplicate_count INTEGER;
total_to_delete INTEGER;
total_servers INTEGER;
has_known_bad_server BOOLEAN;
r RECORD;
BEGIN
-- Get total server count
SELECT COUNT(*) INTO total_servers FROM servers;
-- Check if we have the specific known problematic server from production
-- This server only exists in production data, not in test fixtures
SELECT EXISTS(
SELECT 1 FROM servers
WHERE value->>'name' = 'io.github.joelverhagen/Knapcode.SampleMcpServer/aot'
) INTO has_known_bad_server;
-- Skip migration if we don't have the known bad data (indicates test environment)
IF NOT has_known_bad_server THEN
RAISE NOTICE 'Migration 008: Skipping cleanup - no known problematic data found (likely test/dev environment)';
RETURN; -- Exit early, don't run any cleanup
END IF;
RAISE NOTICE 'Migration 008: Found known problematic data, proceeding with cleanup';
-- Count servers with invalid name format
SELECT COUNT(*) INTO invalid_name_count
FROM servers
WHERE value->>'name' NOT SIMILAR TO '[a-zA-Z0-9][a-zA-Z0-9.-]*[a-zA-Z0-9]/[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9]';
-- Count servers with empty or NULL versions
SELECT COUNT(*) INTO empty_version_count
FROM servers
WHERE value->>'version' IS NULL OR value->>'version' = '';
-- Count servers with invalid status
SELECT COUNT(*) INTO invalid_status_count
FROM servers
WHERE value->>'status' IS NOT NULL
AND value->>'status' != ''
AND value->>'status' NOT IN ('active', 'deprecated', 'deleted');
-- Count duplicate name+version combinations
SELECT COUNT(*) INTO duplicate_count
FROM (
SELECT value->>'name', value->>'version', COUNT(*) as cnt
FROM servers
GROUP BY value->>'name', value->>'version'
HAVING COUNT(*) > 1
) dups;
-- Calculate total deletions (some servers might have both invalid name AND empty version)
SELECT COUNT(*) INTO total_to_delete
FROM servers
WHERE value->>'name' NOT SIMILAR TO '[a-zA-Z0-9][a-zA-Z0-9.-]*[a-zA-Z0-9]/[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9]'
OR value->>'version' IS NULL
OR value->>'version' = '';
-- Log the cleanup operations with safety check
RAISE NOTICE 'Migration 008 Data Cleanup Plan:';
RAISE NOTICE ' Total servers in database: %', total_servers;
IF total_servers > 0 THEN
RAISE NOTICE ' Servers to DELETE: % (% percent of total)', total_to_delete, ROUND((total_to_delete::numeric / total_servers * 100), 2);
ELSE
RAISE NOTICE ' Servers to DELETE: %', total_to_delete;
END IF;
RAISE NOTICE ' - Invalid names: %', invalid_name_count;
RAISE NOTICE ' - Empty versions: %', empty_version_count;
RAISE NOTICE ' Servers to UPDATE (fix status): %', invalid_status_count;
RAISE NOTICE ' Duplicate name+version pairs: %', duplicate_count;
-- Always log the specific servers that will be deleted for transparency
IF total_to_delete > 0 THEN
RAISE NOTICE '';
RAISE NOTICE 'Servers that will be deleted:';
FOR r IN
SELECT value->>'name' as name, value->>'version' as version,
CASE
WHEN value->>'name' NOT SIMILAR TO '[a-zA-Z0-9][a-zA-Z0-9.-]*[a-zA-Z0-9]/[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9]' THEN 'Invalid name format'
WHEN value->>'version' IS NULL THEN 'NULL version'
WHEN value->>'version' = '' THEN 'Empty version'
END as reason
FROM servers
WHERE value->>'name' NOT SIMILAR TO '[a-zA-Z0-9][a-zA-Z0-9.-]*[a-zA-Z0-9]/[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9]'
OR value->>'version' IS NULL
OR value->>'version' = ''
ORDER BY value->>'name'
LOOP
RAISE NOTICE ' - % @ % (reason: %)', r.name, COALESCE(r.version, '<NULL>'), r.reason;
END LOOP;
END IF;
-- Always log the specific servers that will have status updated for transparency
IF invalid_status_count > 0 THEN
RAISE NOTICE '';
RAISE NOTICE 'Servers that will have status updated:';
FOR r IN
SELECT value->>'name' as name, value->>'version' as version, value->>'status' as status
FROM servers
WHERE value->>'status' IS NOT NULL
AND value->>'status' != ''
AND value->>'status' NOT IN ('active', 'deprecated', 'deleted')
ORDER BY value->>'name'
LOOP
RAISE NOTICE ' - % @ % (current status: %)', r.name, r.version, r.status;
END LOOP;
END IF;
-- SAFETY CHECK: Ensure we're deleting exactly the expected data
-- We only reach this point if we found the known bad server
-- In production (2025-09-30), we expect exactly 5 deletions and 1 status update
IF total_to_delete != 5 THEN
RAISE EXCEPTION 'Safety check failed: Expected to delete exactly 5 servers but would delete %. Check the log above for details. Aborting to prevent data loss.', total_to_delete;
END IF;
IF invalid_status_count != 1 THEN
RAISE EXCEPTION 'Safety check failed: Expected to update exactly 1 server status but would update %. Check the log above for details. Aborting to prevent data corruption.', invalid_status_count;
END IF;
END $$;
-- Delete servers with invalid names or empty versions
-- These cannot be reasonably fixed and would violate primary key constraints
DELETE FROM servers
WHERE value->>'name' NOT SIMILAR TO '[a-zA-Z0-9][a-zA-Z0-9.-]*[a-zA-Z0-9]/[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9]'
OR value->>'version' IS NULL
OR value->>'version' = '';
-- Fix invalid status values by setting them to 'active'
-- These can be reasonably defaulted to a valid value
UPDATE servers
SET value = jsonb_set(value, '{status}', '"active"')
WHERE value->>'status' IS NOT NULL
AND value->>'status' != ''
AND value->>'status' NOT IN ('active', 'deprecated', 'deleted');
-- Remove duplicate name+version combinations
-- Keep the one with the most recent publishedAt date
DELETE FROM servers s1
WHERE EXISTS (
SELECT 1 FROM servers s2
WHERE s2.value->>'name' = s1.value->>'name'
AND s2.value->>'version' = s1.value->>'version'
AND (s2.value->>'publishedAt')::timestamp > (s1.value->>'publishedAt')::timestamp
);
-- Verify the operations completed as expected
DO $$
DECLARE
remaining_count INTEGER;
actual_deleted INTEGER;
actual_updated INTEGER;
still_invalid_names INTEGER;
still_empty_versions INTEGER;
still_invalid_status INTEGER;
BEGIN
SELECT COUNT(*) INTO remaining_count FROM servers;
-- Check if any invalid data remains
SELECT COUNT(*) INTO still_invalid_names
FROM servers
WHERE value->>'name' NOT SIMILAR TO '[a-zA-Z0-9][a-zA-Z0-9.-]*[a-zA-Z0-9]/[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9]';
SELECT COUNT(*) INTO still_empty_versions
FROM servers
WHERE value->>'version' IS NULL OR value->>'version' = '';
SELECT COUNT(*) INTO still_invalid_status
FROM servers
WHERE value->>'status' IS NOT NULL
AND value->>'status' != ''
AND value->>'status' NOT IN ('active', 'deprecated', 'deleted');
RAISE NOTICE 'Data cleanup complete:';
RAISE NOTICE ' Servers remaining: %', remaining_count;
RAISE NOTICE ' Invalid names remaining: %', still_invalid_names;
RAISE NOTICE ' Empty versions remaining: %', still_empty_versions;
RAISE NOTICE ' Invalid status remaining: %', still_invalid_status;
-- Final safety check: Ensure we cleaned everything we intended to
IF still_invalid_names > 0 OR still_empty_versions > 0 OR still_invalid_status > 0 THEN
RAISE EXCEPTION 'Cleanup incomplete! Invalid data still remains. Aborting.';
END IF;
END $$;
COMMIT;

220
internal/database/migrations/009_separate_official_metadata.sql Normal file
View file

@ -0,0 +1,220 @@
-- Separate official metadata from JSON to dedicated columns for better performance and consistency
-- This migration transforms the database from JSON-heavy to columnar structure for registry-managed data
BEGIN;
-- Add new columns for official metadata and core identifiers
ALTER TABLE servers ADD COLUMN server_name VARCHAR(255);
ALTER TABLE servers ADD COLUMN version VARCHAR(255);
ALTER TABLE servers ADD COLUMN status VARCHAR(50) DEFAULT 'active';
ALTER TABLE servers ADD COLUMN published_at TIMESTAMP WITH TIME ZONE;
ALTER TABLE servers ADD COLUMN updated_at TIMESTAMP WITH TIME ZONE;
ALTER TABLE servers ADD COLUMN is_latest BOOLEAN;
-- Create a function to migrate existing data from JSON to columns
CREATE OR REPLACE FUNCTION migrate_official_metadata()
RETURNS VOID AS $$
DECLARE
rec RECORD;
official_meta JSONB;
updated_value JSONB;
server_meta JSONB;
publisher_provided JSONB;
BEGIN
-- Iterate through all existing records
FOR rec IN SELECT version_id, value FROM servers ORDER BY version_id LOOP
-- Extract core identifiers
UPDATE servers
SET
server_name = rec.value->>'name',
version = rec.value->>'version'
WHERE version_id = rec.version_id;
-- Extract official metadata from nested JSON structure
official_meta := rec.value->'_meta'->'io.modelcontextprotocol.registry/official';
IF official_meta IS NOT NULL THEN
-- Update columns with extracted metadata
-- Note: status is at top level in old format, not in official_meta
UPDATE servers
SET
status = COALESCE(
NULLIF(NULLIF(rec.value->>'status', ''), 'null'),
'active'
),
published_at = COALESCE((official_meta->>'publishedAt')::TIMESTAMP WITH TIME ZONE, NOW()),
updated_at = COALESCE((official_meta->>'updatedAt')::TIMESTAMP WITH TIME ZONE, NOW()),
is_latest = COALESCE((official_meta->>'isLatest')::BOOLEAN, true)
WHERE version_id = rec.version_id;
ELSE
-- Handle records without official metadata (set defaults)
UPDATE servers
SET
status = COALESCE(
NULLIF(NULLIF(rec.value->>'status', ''), 'null'),
'active'
),
published_at = NOW(),
updated_at = NOW(),
is_latest = true
WHERE version_id = rec.version_id;
END IF;
-- Clean up the JSON: remove status field and official metadata
updated_value := rec.value - 'status';
-- Reconstruct _meta with only publisher-provided data
publisher_provided := rec.value->'_meta'->'io.modelcontextprotocol.registry/publisher-provided';
IF publisher_provided IS NOT NULL THEN
server_meta := jsonb_build_object('io.modelcontextprotocol.registry/publisher-provided', publisher_provided);
updated_value := jsonb_set(updated_value, '{_meta}', server_meta);
ELSE
-- Remove _meta entirely if no publisher-provided data
updated_value := updated_value - '_meta';
END IF;
-- Update the JSON with cleaned structure (keeping name and version for immutable server.json)
UPDATE servers
SET value = updated_value
WHERE version_id = rec.version_id;
END LOOP;
END;
$$ LANGUAGE plpgsql;
-- Execute the migration
SELECT migrate_official_metadata();
-- Drop the migration function
DROP FUNCTION migrate_official_metadata();
-- Safety check: Normalize invalid status values before adding NOT NULL constraints
-- This handles: NULL (missing field), empty string, string "null", and any invalid values
-- First, log how many records have problematic status values for debugging
DO $$
DECLARE
null_count INTEGER;
empty_count INTEGER;
invalid_count INTEGER;
BEGIN
SELECT COUNT(*) INTO null_count FROM servers WHERE status IS NULL;
SELECT COUNT(*) INTO empty_count FROM servers WHERE status = '' OR status = 'null';
SELECT COUNT(*) INTO invalid_count FROM servers WHERE status IS NOT NULL AND status NOT IN ('active', 'deprecated', 'deleted', '', 'null');
IF null_count > 0 OR empty_count > 0 OR invalid_count > 0 THEN
RAISE NOTICE 'Status cleanup: NULL=%, empty/null string=%, invalid=%', null_count, empty_count, invalid_count;
END IF;
END $$;
-- Fix all problematic status values
UPDATE servers
SET status = 'active'
WHERE status IS NULL
OR status = ''
OR status = 'null'
OR status NOT IN ('active', 'deprecated', 'deleted');
-- Double-check that no NULLs remain (this should never fail after the above)
DO $$
DECLARE
remaining_nulls INTEGER;
BEGIN
SELECT COUNT(*) INTO remaining_nulls FROM servers WHERE status IS NULL;
IF remaining_nulls > 0 THEN
RAISE EXCEPTION 'Still have % NULL status values after cleanup!', remaining_nulls;
END IF;
END $$;
UPDATE servers SET published_at = NOW() WHERE published_at IS NULL;
-- Safety check: Ensure updated_at is never NULL
UPDATE servers SET updated_at = NOW() WHERE updated_at IS NULL;
-- For is_latest: preserve existing true values, handle NULLs deterministically
-- First, for servers where NO version has is_latest=true, mark the most recent as latest
WITH servers_without_latest AS (
SELECT DISTINCT s1.server_name
FROM servers s1
WHERE NOT EXISTS (
SELECT 1 FROM servers s2
WHERE s2.server_name = s1.server_name
AND s2.is_latest = true
)
),
latest_versions AS (
SELECT s.server_name, MAX(s.published_at) as max_published
FROM servers s
INNER JOIN servers_without_latest swl ON s.server_name = swl.server_name
GROUP BY s.server_name
)
UPDATE servers s
SET is_latest = true
FROM latest_versions lv
WHERE s.server_name = lv.server_name
AND s.published_at = lv.max_published
AND s.is_latest IS NULL;
-- Finally, set remaining NULLs to false
UPDATE servers SET is_latest = false WHERE is_latest IS NULL;
-- Make the new columns NOT NULL now that all records have values
ALTER TABLE servers ALTER COLUMN server_name SET NOT NULL;
ALTER TABLE servers ALTER COLUMN version SET NOT NULL;
ALTER TABLE servers ALTER COLUMN status SET NOT NULL;
ALTER TABLE servers ALTER COLUMN published_at SET NOT NULL;
ALTER TABLE servers ALTER COLUMN updated_at SET NOT NULL;
ALTER TABLE servers ALTER COLUMN is_latest SET NOT NULL;
-- Drop the old primary key constraint
ALTER TABLE servers DROP CONSTRAINT servers_pkey;
-- Create new composite primary key using name + version (natural key)
ALTER TABLE servers ADD CONSTRAINT servers_pkey PRIMARY KEY (server_name, version);
-- Drop the old version_id column since we're using name-based keys now
ALTER TABLE servers DROP COLUMN version_id;
-- Drop old indexes that used JSON paths and UUIDs
DROP INDEX IF EXISTS idx_servers_version_id;
DROP INDEX IF EXISTS idx_servers_name_latest;
DROP INDEX IF EXISTS idx_servers_updated_at;
DROP INDEX IF EXISTS idx_servers_remotes;
DROP INDEX IF EXISTS idx_unique_server_version;
DROP INDEX IF EXISTS idx_unique_latest_version;
-- Create new efficient indexes on the dedicated columns
CREATE INDEX idx_servers_name ON servers (server_name);
CREATE INDEX idx_servers_name_version ON servers (server_name, version);
CREATE INDEX idx_servers_name_latest ON servers (server_name, is_latest) WHERE is_latest = true;
CREATE INDEX idx_servers_status ON servers (status);
CREATE INDEX idx_servers_published_at ON servers (published_at DESC);
CREATE INDEX idx_servers_updated_at ON servers (updated_at DESC);
-- Ensure only one version per server can be marked as latest
CREATE UNIQUE INDEX idx_unique_latest_per_server
ON servers (server_name)
WHERE is_latest = true;
-- Create GIN index for remaining JSON queries (remotes, packages, etc.)
CREATE INDEX idx_servers_json_remotes ON servers USING GIN((value->'remotes'));
CREATE INDEX idx_servers_json_packages ON servers USING GIN((value->'packages'));
-- Update $schema field to latest version for all entries
UPDATE servers
SET value = jsonb_set(value, '{$schema}', '"https://static.modelcontextprotocol.io/schemas/2025-09-29/server.schema.json"')
WHERE value ? '$schema' AND value IS NOT NULL;
-- Add check constraints for data integrity
ALTER TABLE servers ADD CONSTRAINT check_status_valid
CHECK (status IN ('active', 'deprecated', 'deleted'));
ALTER TABLE servers ADD CONSTRAINT check_server_name_format
CHECK (server_name ~ '^[a-zA-Z0-9][a-zA-Z0-9.-]*[a-zA-Z0-9]/[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9]$');
ALTER TABLE servers ADD CONSTRAINT check_version_not_empty
CHECK (length(trim(version)) > 0);
ALTER TABLE servers ADD CONSTRAINT check_published_at_reasonable
CHECK (published_at >= '2020-01-01'::timestamp AND published_at <= NOW() + interval '1 day');
COMMIT;

1
internal/validators/constants.go
View file

@ -28,6 +28,7 @@ var (
// Server name validation errors
ErrMultipleSlashesInServerName = errors.New("server name cannot contain multiple slashes")
ErrInvalidServerNameFormat = errors.New("server name format is invalid")
)
// RepositorySource represents valid repository sources

29
internal/validators/validators.go
View file

@ -14,6 +14,18 @@ import (
"github.com/modelcontextprotocol/registry/pkg/model"
)
// Server name validation patterns
var (
// Component patterns for namespace and name parts
namespacePattern = `[a-zA-Z0-9][a-zA-Z0-9.-]*[a-zA-Z0-9]`
namePartPattern = `[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9]`
// Compiled regexes
namespaceRegex = regexp.MustCompile(`^` + namespacePattern + `$`)
namePartRegex = regexp.MustCompile(`^` + namePartPattern + `$`)
serverNameRegex = regexp.MustCompile(`^` + namespacePattern + `/` + namePartPattern + `$`)
)
// Regexes to detect semver range syntaxes
var (
// Case 1: comparator ranges
@ -403,11 +415,28 @@ func parseServerName(serverJSON apiv0.ServerJSON) (string, error) {
return "", ErrMultipleSlashesInServerName
}
// Split and check for empty parts
parts := strings.SplitN(name, "/", 2)
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
return "", fmt.Errorf("server name must be in format 'dns-namespace/name' with non-empty namespace and name parts")
}
// Validate name format using regex
if !serverNameRegex.MatchString(name) {
namespace := parts[0]
serverName := parts[1]
// Check which part is invalid for a better error message
if !namespaceRegex.MatchString(namespace) {
return "", fmt.Errorf("%w: namespace '%s' is invalid. Namespace must start and end with alphanumeric characters, and may contain dots and hyphens in the middle", ErrInvalidServerNameFormat, namespace)
}
if !namePartRegex.MatchString(serverName) {
return "", fmt.Errorf("%w: name '%s' is invalid. Name must start and end with alphanumeric characters, and may contain dots, underscores, and hyphens in the middle", ErrInvalidServerNameFormat, serverName)
}
// Fallback in case both somehow pass individually but not together
return "", fmt.Errorf("%w: invalid format for '%s'", ErrInvalidServerNameFormat, name)
}
return name, nil
}