Skip to content

fix(validators): name the package version in ownership-failure errors #1544

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sronix wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(validators): name the package version in ownership-failure errors #1544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/validators/registries/cargo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,8 @@ func validateCargoREADME(ctx context.Context, pkg model.Package, serverName stri
// If the token IS present but glued to a trailing character, explain that
// rather than telling the publisher to add a token they can already see.
if trailing, glued := mcpNameTokenGluedTrailing(string(body), serverName); glued {
return fmt.Errorf("cargo package '%s' ownership validation failed: found 'mcp-name: %s' in the README, but it is immediately followed by %q rather than a boundary. The token must be followed by a space, newline, or an HTML tag — put it on its own line and publish a new version", pkg.Identifier, serverName, trailing)
return fmt.Errorf("cargo package '%s' version '%s' ownership validation failed: found 'mcp-name: %s' in the README, but it is immediately followed by %q rather than a boundary. The token must be followed by a space, newline, or an HTML tag — put it on its own line and publish a new version", pkg.Identifier, pkg.Version, serverName, trailing)
}

return fmt.Errorf("cargo package '%s' ownership validation failed. The server name '%s' must appear as 'mcp-name: %s' in the package README", pkg.Identifier, serverName, serverName)
return fmt.Errorf("cargo package '%s' version '%s' ownership validation failed. The server name '%s' must appear as 'mcp-name: %s' in the package README", pkg.Identifier, pkg.Version, serverName, serverName)
}
4 changes: 2 additions & 2 deletions internal/validators/registries/cargo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ func TestValidateCargoCombinedFixture(t *testing.T) {
readmeStatus: http.StatusOK,
readmeBody: fmt.Sprintf("<html><body><p>mcp-name: %s-extended</p></body></html>", serverName),
wantErr: true,
wantContains: []string{"ownership validation failed"},
wantContains: []string{"'combined-prefix' version '0.1.0' ownership validation failed"},
},
{
// Token present but glued to a trailing period — the error must explain
Expand All @@ -446,7 +446,7 @@ func TestValidateCargoCombinedFixture(t *testing.T) {
readmeStatus: http.StatusOK,
readmeBody: fmt.Sprintf("<html><body><p>mcp-name: %s.</p></body></html>", serverName),
wantErr: true,
wantContains: []string{"immediately followed by", `"."`},
wantContains: []string{"'combined-glued' version '0.1.0' ownership validation failed", "immediately followed by", `"."`},
},
}

Expand Down
4 changes: 2 additions & 2 deletions internal/validators/registries/npm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,11 @@ func validateNPMPackage(ctx context.Context, pkg model.Package, serverName strin
}

if npmResp.MCPName == "" {
return fmt.Errorf("NPM package '%s' is missing required 'mcpName' field. Add this to your package.json: \"mcpName\": \"%s\"", pkg.Identifier, serverName)
return fmt.Errorf("NPM package '%s' version '%s' is missing required 'mcpName' field. Add this to your package.json: \"mcpName\": \"%s\"", pkg.Identifier, pkg.Version, serverName)
}

if npmResp.MCPName != serverName {
return fmt.Errorf("NPM package ownership validation failed. Expected mcpName '%s', got '%s'", serverName, npmResp.MCPName)
return fmt.Errorf("NPM package '%s' version '%s' ownership validation failed. Expected mcpName '%s', got '%s'", pkg.Identifier, pkg.Version, serverName, npmResp.MCPName)
}

return nil
Expand Down
23 changes: 23 additions & 0 deletions internal/validators/registries/npm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,29 @@ func TestValidateNPM_PositivePathMock(t *testing.T) {
assert.NoError(t, err, "a version response with the matching mcpName should validate")
}

func TestValidateNPM_OwnershipMismatchNamesVersion(t *testing.T) {
ctx := context.Background()
mock := newNPMMock(http.StatusOK, `{"mcpName":"io.github.acme/widget"}`, http.StatusOK)
defer mock.Close()

pkg := model.Package{RegistryType: model.RegistryTypeNPM, RegistryBaseURL: mock.URL, Identifier: "demo-pkg", Version: "1.2.3"}
err := registries.ValidateNPMPackage(ctx, pkg, "io.github.Acme/widget")
assert.Error(t, err)
assert.Contains(t, err.Error(), "NPM package 'demo-pkg' version '1.2.3' ownership validation failed")
assert.Contains(t, err.Error(), "Expected mcpName 'io.github.Acme/widget', got 'io.github.acme/widget'")
}

func TestValidateNPM_MissingMCPNameNamesVersion(t *testing.T) {
ctx := context.Background()
mock := newNPMMock(http.StatusOK, `{"name":"demo-pkg","version":"1.2.3"}`, http.StatusOK)
defer mock.Close()

pkg := model.Package{RegistryType: model.RegistryTypeNPM, RegistryBaseURL: mock.URL, Identifier: "demo-pkg", Version: "1.2.3"}
err := registries.ValidateNPMPackage(ctx, pkg, "io.github.test/demo")
assert.Error(t, err)
assert.Contains(t, err.Error(), "NPM package 'demo-pkg' version '1.2.3' is missing required 'mcpName' field")
}

func TestValidateNPM_RealPackages(t *testing.T) {
ctx := context.Background()

Expand Down
2 changes: 1 addition & 1 deletion internal/validators/registries/oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func ValidateOCI(ctx context.Context, pkg model.Package, serverName string) erro
}

if mcpName != serverName {
return fmt.Errorf("OCI image ownership validation failed. Expected annotation 'io.modelcontextprotocol.server.name' = '%s', got '%s'", serverName, mcpName)
return fmt.Errorf("OCI image '%s' ownership validation failed. Expected annotation 'io.modelcontextprotocol.server.name' = '%s', got '%s'", pkg.Identifier, serverName, mcpName)
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/validators/registries/oci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,6 @@ func TestValidateOCI_LabelMismatch(t *testing.T) {

err := registries.ValidateOCI(ctx, pkg, "io.github.github/github-mcp-server-mismatch")
assert.Error(t, err)
assert.Contains(t, err.Error(), "ownership validation failed")
assert.Contains(t, err.Error(), "OCI image 'ghcr.io/github/github-mcp-server:latest' ownership validation failed")
assert.Contains(t, err.Error(), "Expected annotation")
}
4 changes: 2 additions & 2 deletions internal/validators/registries/pypi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,10 @@ func validatePyPIPackage(ctx context.Context, pkg model.Package, serverName stri
// If the token IS present but glued to a trailing character, say so — otherwise
// the publisher sees "must appear as mcp-name: X" while looking at exactly that.
if trailing, glued := mcpNameTokenGluedTrailing(description, serverName); glued {
return fmt.Errorf("PyPI package '%s' ownership validation failed: found 'mcp-name: %s' in the README, but it is immediately followed by %q rather than a boundary. The token must be followed by a space, newline, an HTML tag, or a comment close ('-->') — put it on its own line and republish", pkg.Identifier, serverName, trailing)
return fmt.Errorf("PyPI package '%s' version '%s' ownership validation failed: found 'mcp-name: %s' in the README, but it is immediately followed by %q rather than a boundary. The token must be followed by a space, newline, an HTML tag, or a comment close ('-->') — put it on its own line and republish", pkg.Identifier, pkg.Version, serverName, trailing)
}

return fmt.Errorf("PyPI package '%s' ownership validation failed. The server name '%s' must appear as 'mcp-name: %s' in the package README", pkg.Identifier, serverName, serverName)
return fmt.Errorf("PyPI package '%s' version '%s' ownership validation failed. The server name '%s' must appear as 'mcp-name: %s' in the package README", pkg.Identifier, pkg.Version, serverName, serverName)
}

// pypiPackageState is the outcome of probing the package-level PyPI metadata
Expand Down
27 changes: 27 additions & 0 deletions internal/validators/registries/pypi_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,33 @@ func TestValidatePyPI_PositivePathMock(t *testing.T) {
assert.NoError(t, err, "a version README containing the exact mcp-name token should validate")
}

func TestValidatePyPI_TokenMissingNamesVersion(t *testing.T) {
ctx := context.Background()
body := `{"info":{"description":"# Demo\n\nNo ownership token here.\n"}}`
mock := newPyPIMock(http.StatusOK, body, http.StatusOK)
defer mock.Close()

pkg := model.Package{RegistryType: model.RegistryTypePyPI, RegistryBaseURL: mock.URL, Identifier: "demo-pkg", Version: "1.0.0"}
err := registries.ValidatePyPIPackage(ctx, pkg, "io.github.test/demo")
assert.Error(t, err)
assert.Contains(t, err.Error(), "PyPI package 'demo-pkg' version '1.0.0' ownership validation failed")
assert.Contains(t, err.Error(), "must appear as 'mcp-name: io.github.test/demo'")
}

func TestValidatePyPI_TokenGluedNamesVersion(t *testing.T) {
ctx := context.Background()
const serverName = "io.github.test/demo"
body := fmt.Sprintf(`{"info":{"description":"# Demo\n\nmcp-name: %s.\n"}}`, serverName)
mock := newPyPIMock(http.StatusOK, body, http.StatusOK)
defer mock.Close()

pkg := model.Package{RegistryType: model.RegistryTypePyPI, RegistryBaseURL: mock.URL, Identifier: "demo-pkg", Version: "1.0.0"}
err := registries.ValidatePyPIPackage(ctx, pkg, serverName)
assert.Error(t, err)
assert.Contains(t, err.Error(), "PyPI package 'demo-pkg' version '1.0.0' ownership validation failed")
assert.Contains(t, err.Error(), "immediately followed by")
}

func TestValidatePyPI_RealPackages(t *testing.T) {
ctx := context.Background()

Expand Down