From 703fa31f9037a2ae3d6c08cfd75de0f372d4ab95 Mon Sep 17 00:00:00 2001 From: Thomas Thomsen Date: Tue, 18 Aug 2026 20:57:09 +0900 Subject: [PATCH] fix(validators): name the package version in ownership-failure errors An ownership failure (mcpName mismatch, missing mcpName, missing or glued mcp-name token, OCI label mismatch) reported which value was found but not which package version it was read from. A publisher whose server.json still names an older release then reads the value as belonging to the release they just published and looks for the fault in the wrong place (#1525). NuGet already says "ownership validation for version X failed". This brings npm, PyPI, cargo and OCI in line: npm, PyPI and cargo name the version, OCI names the full image reference, which carries the tag or digest. Message text only, no behavior change. Existing tests match on substrings and stay green; new hermetic cases assert the version is present. --- internal/validators/registries/cargo.go | 4 +-- internal/validators/registries/cargo_test.go | 4 +-- internal/validators/registries/npm.go | 4 +-- internal/validators/registries/npm_test.go | 23 +++++++++++++++++ internal/validators/registries/oci.go | 2 +- internal/validators/registries/oci_test.go | 2 +- internal/validators/registries/pypi.go | 4 +-- internal/validators/registries/pypi_test.go | 27 ++++++++++++++++++++ 8 files changed, 60 insertions(+), 10 deletions(-) diff --git a/internal/validators/registries/cargo.go b/internal/validators/registries/cargo.go index 5aa72b109..950832490 100644 --- a/internal/validators/registries/cargo.go +++ b/internal/validators/registries/cargo.go @@ -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) } diff --git a/internal/validators/registries/cargo_test.go b/internal/validators/registries/cargo_test.go index 5328f4523..e370dc7c2 100644 --- a/internal/validators/registries/cargo_test.go +++ b/internal/validators/registries/cargo_test.go @@ -434,7 +434,7 @@ func TestValidateCargoCombinedFixture(t *testing.T) { readmeStatus: http.StatusOK, readmeBody: fmt.Sprintf("

mcp-name: %s-extended

", 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 @@ -446,7 +446,7 @@ func TestValidateCargoCombinedFixture(t *testing.T) { readmeStatus: http.StatusOK, readmeBody: fmt.Sprintf("

mcp-name: %s.

", serverName), wantErr: true, - wantContains: []string{"immediately followed by", `"."`}, + wantContains: []string{"'combined-glued' version '0.1.0' ownership validation failed", "immediately followed by", `"."`}, }, } diff --git a/internal/validators/registries/npm.go b/internal/validators/registries/npm.go index a6ed17023..fdd7fb725 100644 --- a/internal/validators/registries/npm.go +++ b/internal/validators/registries/npm.go @@ -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 diff --git a/internal/validators/registries/npm_test.go b/internal/validators/registries/npm_test.go index 7adb82827..0264e72c7 100644 --- a/internal/validators/registries/npm_test.go +++ b/internal/validators/registries/npm_test.go @@ -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() diff --git a/internal/validators/registries/oci.go b/internal/validators/registries/oci.go index f3032677c..7214a00e6 100644 --- a/internal/validators/registries/oci.go +++ b/internal/validators/registries/oci.go @@ -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 diff --git a/internal/validators/registries/oci_test.go b/internal/validators/registries/oci_test.go index fddf44601..aabc6ee6f 100644 --- a/internal/validators/registries/oci_test.go +++ b/internal/validators/registries/oci_test.go @@ -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") } diff --git a/internal/validators/registries/pypi.go b/internal/validators/registries/pypi.go index 339ed911e..83dfdc54e 100644 --- a/internal/validators/registries/pypi.go +++ b/internal/validators/registries/pypi.go @@ -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 diff --git a/internal/validators/registries/pypi_test.go b/internal/validators/registries/pypi_test.go index e240082a9..b30b764c3 100644 --- a/internal/validators/registries/pypi_test.go +++ b/internal/validators/registries/pypi_test.go @@ -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()