Enhanced server.json validation (phase 1) (#636)
Added schema validation and support for exhaustive and more detailed validation to existing validators. Added new `mcp-publisher validate` command. No change to any existing tests or behavior other than the new command. ## Motivation and Context Many currently published servers fail schema validation. Of those that pass, many have semantic/logical errors or other errors that make them impossible to configure, or their configuration when applied doesn't generate correct MCP server configuration (per their own documentation). To address this, we need better tooling to validate servers and to inform server publishers of errors, issues, and best practices in a way that is clear and actionable, both during server development and at time of publishing. I have started a discussion about the broader topic: https://github.com/modelcontextprotocol/registry/discussions/635 There is also a fairly detailed document describing the design, architecture, implementation, future plans, etc. See [Enhanced Validation Design](docs/explanations/proposed-enhanced-validation.md) This PR is the first step in the process of improving validation. It adds schema validation and updates all existing validators to use a new model that allows them to track context and return rich and exhaustive results. This has been done in a way that is backward compatible (ValidateServerJSON is unchanged as are all existing validation unit tests). There is a new `mcp-publisher` command called `validate` that is currenty the only place that schema validation and enhanced (exhaustive/rich) validation results are exposed. ### Changes Internal Validation improvements - Schema validation has been implemented and is available as an option in validation methods (defaults to off) - Existing validators track the JSON path (context) of the attribute they're evaluating, passing it to any child validators - Existing validators are exhaustive (return all issues, not failing fast on the first error) - Existing validators return rich validation results, including: - type (schema, semantic, linter) - severity (error, warn, info) - path (JSON path to server.json element) - reference (to the schema element or rule that triggered the result) - message (currently the same as previous error message) A new `validate` command has been added to `mcp-publisher` so publishers can evaluate their server.json before publishing - Includes new schema validation and previous semantic validation - Shows full details of all issues encountered during validation ### Usage Given a server.json that looks like this: ```json { "$schema": "https://static.modelcontextprotocol.io/schemas/2025-09-29/server.schema.json", "name": "io.github.BobDickinson/registry", "description": "An MCP server that provides [describe what your server does]", "repository": { "url": "", "source": "" }, "version": "=1.0.0", "packages": [ { "registryType": "oci", "registryBaseUrl": "https://docker.io", "identifier": "registry", "version": "1.0.0", "transport": { "type": "sse" }, "packageArguments": [ { "type": "named", "name": "--mode", "description": "Operation mode", "format": "foo" } ], "environmentVariables": [ { "description": "Your API key for the service", "isRequired": true, "format": "string", "isSecret": true, "name": "YOUR_API_KEY" } ] } ] } ``` Run the command: `mcp-publisher validate server.json` Which will produce the output: ```bash ❌ Validation failed with 5 issue(s): 1. [error] packages.0.packageArguments.0.format (schema) value must be one of "string", "number", "boolean", "filepath" Reference: #/definitions/Input/properties/format/enum from: [#/definitions/ServerDetail]/properties/packages/items/[#/definitions/Package]/properties/packageArguments/items/[#/definitions/Argument]/else/[#/definitions/NamedArgument]/allOf/0/[#/definitions/InputWithVariables]/allOf/0/[#/definitions/Input]/properties/format/enum 2. [error] packages.0.transport (schema) missing required fields: 'url' Reference: #/definitions/SseTransport/required from: [#/definitions/ServerDetail]/properties/packages/items/[#/definitions/Package]/properties/transport/else/else/[#/definitions/SseTransport]/required 3. [error] repository.url (schema) '' has invalid format 'uri' Reference: #/definitions/Repository/properties/url/format from: [#/definitions/ServerDetail]/properties/repository/[#/definitions/Repository]/properties/url/format 4. [error] version (semantic) version must be a specific version, not a range: "=1.0.0" Reference: version-looks-like-range 5. [error] packages[0].transport.url (semantic) url is required for sse transport type Reference: streamable-transport-url-required Error: validation failed ``` Note in the results above: - All errors contain the JSON path of the triggering element from server.json. - The first three errors are schema errors detected during schema validation. The `Reference` provided contains an absolute path to the schema rule that was violated, followed by the full schema path that enforced that rule (with $ref redirect values substited and enclosed in square brackets). - The error messages provided by the existing semantic validators are exactly the same as the previous error messages those validators produced. In the final solution we would remove semantic validators that are redundant to schema validation (as in number 5 above). We have the option to use the structured data produced by schema validation to replace the generic schema validation message with a more descriptive message if that is an issue (in particular, if the only argument for an ad-hoc validator is that is produces a better/cleaner message, we would just move that message creation code to the schema error result formatter and special case it as opposed to having a redundant validator). ### What's Next If this general direction is supported and this PR is accepted, a fast follow would include: - Add new validators (infliuenced by observed issues) for both semantic errors and best practices - Require passing schema validation (in addition to improved semantic validation) in order to publish - Remove existing validation code that is redundant to schema validation (ensuring error messages remain intelligible) - Update validation client code and tests to handle the new rich return types natively ### Issues Schema validation requires embedding the `server.schema.json` file into the validators package via `go:embed`, which is restricted from embedding files outside of the package. For this reason we copy the schema file into a schema subdir (via a prep target in the makefile) and we .gitignore it. I tried cleaning up the copy in the make clean target, but then the linter would complain about the embed target being missing if it wasn't there during development. I also considered just checking in the schema file and assuming a separate workflow for updating the embedded schema file, but it's not clear what that workflow would be or how it would be maintained. I'm open to a different approach, but the schema file does need to be embedded somehow for schema validation to work. ## How Has This Been Tested? All existing validation tests pass, new tests implemented and all pass. ## Breaking Changes No breaking changes, no behavior change except new `validate` command ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [X] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [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 --------- Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
parent
d3d137e170
commit
fc88cf0870
1 changed files with 6107 additions and 179 deletions
|
|
@ -51,206 +51,299 @@ var (
|
|||
dottedVersionLikeRe = regexp.MustCompile(`^\s*(?:v?\d+|x|X|\*)(?:\.(?:\d+|x|X|\*)){1,2}(?:-[0-9A-Za-z.-]+)?\s*$`)
|
||||
)
|
||||
|
||||
func ValidateServerJSON(serverJSON *apiv0.ServerJSON) error {
|
||||
// Validate schema version is provided and supported
|
||||
// Note: Schema field is also marked as required in the ServerJSON struct definition
|
||||
// for API-level validation and documentation
|
||||
if serverJSON.Schema == "" {
|
||||
return fmt.Errorf("$schema field is required")
|
||||
// ValidateServerJSON performs exhaustive validation and returns all issues found
|
||||
// opts specifies which types of validation to perform. ValidateSchema implies ValidateSchemaVersion.
|
||||
// Empty schema is always checked and always produces an error when schema validation is performed.
|
||||
func ValidateServerJSON(serverJSON *apiv0.ServerJSON, opts ValidationOptions) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
ctx := &ValidationContext{}
|
||||
|
||||
// Schema validation (version check and/or full validation)
|
||||
if opts.ValidateSchemaVersion || opts.ValidateSchema {
|
||||
schemaResult := validateServerJSONSchema(serverJSON, opts.ValidateSchema, opts.NonCurrentSchemaPolicy)
|
||||
result.Merge(schemaResult)
|
||||
}
|
||||
if !strings.Contains(serverJSON.Schema, model.CurrentSchemaVersion) {
|
||||
return fmt.Errorf("schema version %s is not supported. Please use schema version %s", serverJSON.Schema, model.CurrentSchemaVersion)
|
||||
|
||||
// Semantic validation (only if requested)
|
||||
if !opts.ValidateSemantic {
|
||||
return result
|
||||
}
|
||||
|
||||
// Validate server name exists and format
|
||||
if _, err := parseServerName(*serverJSON); err != nil {
|
||||
return err
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("name").String(),
|
||||
err,
|
||||
"invalid-server-name",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
// Validate top-level server version is a specific version (not a range) & not "latest"
|
||||
if err := validateVersion(serverJSON.Version); err != nil {
|
||||
return err
|
||||
}
|
||||
versionResult := validateVersion(ctx.Field("version"), serverJSON.Version)
|
||||
result.Merge(versionResult)
|
||||
|
||||
// Validate repository
|
||||
if err := validateRepository(serverJSON.Repository); err != nil {
|
||||
return err
|
||||
}
|
||||
repoResult := validateRepository(ctx.Field("repository"), serverJSON.Repository)
|
||||
result.Merge(repoResult)
|
||||
|
||||
// Validate website URL if provided
|
||||
if err := validateWebsiteURL(serverJSON.WebsiteURL); err != nil {
|
||||
return err
|
||||
}
|
||||
websiteResult := validateWebsiteURL(ctx.Field("websiteUrl"), serverJSON.WebsiteURL)
|
||||
result.Merge(websiteResult)
|
||||
|
||||
// Validate title if provided
|
||||
if err := validateTitle(serverJSON.Title); err != nil {
|
||||
return err
|
||||
}
|
||||
titleResult := validateTitle(ctx.Field("title"), serverJSON.Title)
|
||||
result.Merge(titleResult)
|
||||
|
||||
// Validate icons if provided
|
||||
if err := validateIcons(serverJSON.Icons); err != nil {
|
||||
return err
|
||||
}
|
||||
iconsResult := validateIcons(ctx.Field("icons"), serverJSON.Icons)
|
||||
result.Merge(iconsResult)
|
||||
|
||||
// Validate all packages (basic field validation)
|
||||
// Detailed package validation (including registry checks) is done during publish
|
||||
for _, pkg := range serverJSON.Packages {
|
||||
if err := validatePackageField(&pkg); err != nil {
|
||||
return err
|
||||
}
|
||||
for i, pkg := range serverJSON.Packages {
|
||||
pkgResult := validatePackageField(ctx.Field("packages").Index(i), &pkg)
|
||||
result.Merge(pkgResult)
|
||||
}
|
||||
|
||||
// Validate all remotes
|
||||
for _, remote := range serverJSON.Remotes {
|
||||
if err := validateRemoteTransport(&remote); err != nil {
|
||||
return err
|
||||
}
|
||||
for i, remote := range serverJSON.Remotes {
|
||||
remoteResult := validateRemoteTransport(ctx.Field("remotes").Index(i), &remote)
|
||||
result.Merge(remoteResult)
|
||||
}
|
||||
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
func validateRepository(obj *model.Repository) error {
|
||||
func validateRepository(ctx *ValidationContext, obj *model.Repository) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
// Skip validation if repository is nil or empty (optional field)
|
||||
if obj == nil || (obj.URL == "" && obj.Source == "") {
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
// validate the repository source
|
||||
repoSource := RepositorySource(obj.Source)
|
||||
if !IsValidRepositoryURL(repoSource, obj.URL) {
|
||||
return fmt.Errorf("%w: %s", ErrInvalidRepositoryURL, obj.URL)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("url").String(),
|
||||
fmt.Errorf("%w: %s", ErrInvalidRepositoryURL, obj.URL),
|
||||
"invalid-repository-url",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
// validate subfolder if present
|
||||
if obj.Subfolder != "" && !IsValidSubfolderPath(obj.Subfolder) {
|
||||
return fmt.Errorf("%w: %s", ErrInvalidSubfolderPath, obj.Subfolder)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("subfolder").String(),
|
||||
fmt.Errorf("%w: %s", ErrInvalidSubfolderPath, obj.Subfolder),
|
||||
"invalid-subfolder-path",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
func validateWebsiteURL(websiteURL string) error {
|
||||
func validateWebsiteURL(ctx *ValidationContext, websiteURL string) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
// Skip validation if website URL is not provided (optional field)
|
||||
if websiteURL == "" {
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
// Parse the URL to ensure it's valid
|
||||
parsedURL, err := url.Parse(websiteURL)
|
||||
if err != nil {
|
||||
return fmt.Errorf("invalid websiteUrl: %w", err)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.String(),
|
||||
fmt.Errorf("invalid websiteUrl: %w", err),
|
||||
"invalid-website-url",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
return result
|
||||
}
|
||||
|
||||
// Ensure it's an absolute URL with valid scheme
|
||||
if !parsedURL.IsAbs() {
|
||||
return fmt.Errorf("websiteUrl must be absolute (include scheme): %s", websiteURL)
|
||||
issue := NewValidationIssue(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.String(),
|
||||
fmt.Sprintf("websiteUrl must be absolute (include scheme): %s", websiteURL),
|
||||
ValidationIssueSeverityError,
|
||||
"website-url-must-be-absolute",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
// Only allow HTTPS scheme for security
|
||||
if parsedURL.Scheme != SchemeHTTPS {
|
||||
return fmt.Errorf("websiteUrl must use https scheme: %s", websiteURL)
|
||||
issue := NewValidationIssue(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.String(),
|
||||
fmt.Sprintf("websiteUrl must use https scheme: %s", websiteURL),
|
||||
ValidationIssueSeverityError,
|
||||
"website-url-invalid-scheme",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
func validateTitle(title string) error {
|
||||
func validateTitle(ctx *ValidationContext, title string) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
// Skip validation if title is not provided (optional field)
|
||||
if title == "" {
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
// Check that title is not only whitespace
|
||||
if strings.TrimSpace(title) == "" {
|
||||
return fmt.Errorf("title cannot be only whitespace")
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.String(),
|
||||
fmt.Errorf("title cannot be only whitespace"),
|
||||
"title-whitespace-only",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
func validateIcons(icons []model.Icon) error {
|
||||
func validateIcons(ctx *ValidationContext, icons []model.Icon) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
// Skip validation if no icons are provided (optional field)
|
||||
if len(icons) == 0 {
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
// Validate each icon
|
||||
for i, icon := range icons {
|
||||
if err := validateIcon(&icon); err != nil {
|
||||
return fmt.Errorf("invalid icon at index %d: %w", i, err)
|
||||
}
|
||||
iconResult := validateIcon(ctx.Index(i), &icon)
|
||||
result.Merge(iconResult)
|
||||
}
|
||||
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
func validateIcon(icon *model.Icon) error {
|
||||
func validateIcon(ctx *ValidationContext, icon *model.Icon) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
// Parse the URL to ensure it's valid
|
||||
parsedURL, err := url.Parse(icon.Src)
|
||||
if err != nil {
|
||||
return fmt.Errorf("invalid icon src URL: %w", err)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("src").String(),
|
||||
fmt.Errorf("invalid icon src URL: %w", err),
|
||||
"icon-src-invalid-url",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
return result
|
||||
}
|
||||
|
||||
// Ensure it's an absolute URL
|
||||
if !parsedURL.IsAbs() {
|
||||
return fmt.Errorf("icon src must be an absolute URL (include scheme): %s", icon.Src)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("src").String(),
|
||||
fmt.Errorf("icon src must be an absolute URL (include scheme): %s", icon.Src),
|
||||
"icon-src-not-absolute",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
// Only allow HTTPS scheme for security (no HTTP or data: URIs)
|
||||
if parsedURL.Scheme != SchemeHTTPS {
|
||||
return fmt.Errorf("icon src must use https scheme (got %s): %s", parsedURL.Scheme, icon.Src)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("src").String(),
|
||||
fmt.Errorf("icon src must use https scheme (got %s): %s", parsedURL.Scheme, icon.Src),
|
||||
"icon-src-invalid-scheme",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
func validatePackageField(obj *model.Package) error {
|
||||
func validatePackageField(ctx *ValidationContext, obj *model.Package) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
// Validate identifier has no spaces
|
||||
if !HasNoSpaces(obj.Identifier) {
|
||||
return ErrPackageNameHasSpaces
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("identifier").String(),
|
||||
ErrPackageNameHasSpaces,
|
||||
"package-name-has-spaces",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
// Validate version string
|
||||
if err := validateVersion(obj.Version); err != nil {
|
||||
return err
|
||||
}
|
||||
versionResult := validateVersion(ctx.Field("version"), obj.Version)
|
||||
result.Merge(versionResult)
|
||||
|
||||
// Validate runtime arguments
|
||||
for _, arg := range obj.RuntimeArguments {
|
||||
if err := validateArgument(&arg); err != nil {
|
||||
return fmt.Errorf("invalid runtime argument: %w", err)
|
||||
}
|
||||
for i, arg := range obj.RuntimeArguments {
|
||||
argResult := validateArgument(ctx.Field("runtimeArguments").Index(i), &arg)
|
||||
result.Merge(argResult)
|
||||
}
|
||||
|
||||
// Validate package arguments
|
||||
for _, arg := range obj.PackageArguments {
|
||||
if err := validateArgument(&arg); err != nil {
|
||||
return fmt.Errorf("invalid package argument: %w", err)
|
||||
}
|
||||
for i, arg := range obj.PackageArguments {
|
||||
argResult := validateArgument(ctx.Field("packageArguments").Index(i), &arg)
|
||||
result.Merge(argResult)
|
||||
}
|
||||
|
||||
// Validate transport with template variable support
|
||||
availableVariables := collectAvailableVariables(obj)
|
||||
if err := validatePackageTransport(&obj.Transport, availableVariables); err != nil {
|
||||
return fmt.Errorf("invalid transport: %w", err)
|
||||
}
|
||||
transportResult := validatePackageTransport(ctx.Field("transport"), &obj.Transport, availableVariables)
|
||||
result.Merge(transportResult)
|
||||
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
// validateVersion validates the version string.
|
||||
// NB: we decided that we would not enforce strict semver for version strings
|
||||
func validateVersion(version string) error {
|
||||
func validateVersion(ctx *ValidationContext, version string) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
if version == "latest" {
|
||||
return ErrReservedVersionString
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.String(),
|
||||
ErrReservedVersionString,
|
||||
"reserved-version-string",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
return result
|
||||
}
|
||||
|
||||
// Reject semver range-like inputs
|
||||
if looksLikeVersionRange(version) {
|
||||
return fmt.Errorf("%w: %q", ErrVersionLooksLikeRange, version)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.String(),
|
||||
fmt.Errorf("%w: %q", ErrVersionLooksLikeRange, version),
|
||||
"version-looks-like-range",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
// looksLikeVersionRange detects common semver range syntaxes and wildcard patterns.
|
||||
|
|
@ -286,25 +379,34 @@ func looksLikeVersionRange(version string) bool {
|
|||
}
|
||||
|
||||
// validateArgument validates argument details
|
||||
func validateArgument(obj *model.Argument) error {
|
||||
func validateArgument(ctx *ValidationContext, obj *model.Argument) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
if obj.Type == model.ArgumentTypeNamed {
|
||||
// Validate named argument name format
|
||||
if err := validateNamedArgumentName(obj.Name); err != nil {
|
||||
return err
|
||||
}
|
||||
nameResult := validateNamedArgumentName(ctx.Field("name"), obj.Name)
|
||||
result.Merge(nameResult)
|
||||
|
||||
// Validate value and default don't start with the name
|
||||
if err := validateArgumentValueFields(obj.Name, obj.Value, obj.Default); err != nil {
|
||||
return err
|
||||
}
|
||||
valueResult := validateArgumentValueFields(ctx, obj.Name, obj.Value, obj.Default)
|
||||
result.Merge(valueResult)
|
||||
}
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
func validateNamedArgumentName(name string) error {
|
||||
func validateNamedArgumentName(ctx *ValidationContext, name string) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
// Check if name is required for named arguments
|
||||
if name == "" {
|
||||
return ErrNamedArgumentNameRequired
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.String(),
|
||||
ErrNamedArgumentNameRequired,
|
||||
"named-argument-name-required",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
return result
|
||||
}
|
||||
|
||||
// Check for invalid characters that suggest embedded values or descriptions
|
||||
|
|
@ -312,23 +414,43 @@ func validateNamedArgumentName(name string) error {
|
|||
// Invalid: "--directory <absolute_path_to_adfin_mcp_folder>", "--port 8080"
|
||||
if strings.Contains(name, "<") || strings.Contains(name, ">") ||
|
||||
strings.Contains(name, " ") || strings.Contains(name, "$") {
|
||||
return fmt.Errorf("%w: %s", ErrInvalidNamedArgumentName, name)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.String(),
|
||||
fmt.Errorf("%w: %s", ErrInvalidNamedArgumentName, name),
|
||||
"invalid-named-argument-name",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
func validateArgumentValueFields(name, value, defaultValue string) error {
|
||||
func validateArgumentValueFields(ctx *ValidationContext, name, value, defaultValue string) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
// Check if value starts with the argument name (using startsWith, not contains)
|
||||
if value != "" && strings.HasPrefix(value, name) {
|
||||
return fmt.Errorf("%w: value starts with argument name '%s': %s", ErrArgumentValueStartsWithName, name, value)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("value").String(),
|
||||
fmt.Errorf("%w: value starts with argument name '%s': %s", ErrArgumentValueStartsWithName, name, value),
|
||||
"argument-value-starts-with-name",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
if defaultValue != "" && strings.HasPrefix(defaultValue, name) {
|
||||
return fmt.Errorf("%w: default starts with argument name '%s': %s", ErrArgumentDefaultStartsWithName, name, defaultValue)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("default").String(),
|
||||
fmt.Errorf("%w: default starts with argument name '%s': %s", ErrArgumentDefaultStartsWithName, name, defaultValue),
|
||||
"argument-default-starts-with-name",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
return nil
|
||||
return result
|
||||
}
|
||||
|
||||
// collectAvailableVariables collects all available template variables from a package
|
||||
|
|
@ -378,44 +500,92 @@ func collectRemoteTransportVariables(transport *model.Transport) []string {
|
|||
}
|
||||
|
||||
// validatePackageTransport validates a package's transport with templating support
|
||||
func validatePackageTransport(transport *model.Transport, availableVariables []string) error {
|
||||
func validatePackageTransport(ctx *ValidationContext, transport *model.Transport, availableVariables []string) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
// Validate transport type is supported
|
||||
switch transport.Type {
|
||||
case model.TransportTypeStdio:
|
||||
// Validate that URL is empty for stdio transport
|
||||
if transport.URL != "" {
|
||||
return fmt.Errorf("url must be empty for %s transport type, got: %s", transport.Type, transport.URL)
|
||||
issue := NewValidationIssue(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("url").String(),
|
||||
fmt.Sprintf("url must be empty for %s transport type, got: %s", transport.Type, transport.URL),
|
||||
ValidationIssueSeverityError,
|
||||
"stdio-transport-url-not-empty",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
return nil
|
||||
case model.TransportTypeStreamableHTTP, model.TransportTypeSSE:
|
||||
// URL is required for streamable-http and sse
|
||||
if transport.URL == "" {
|
||||
return fmt.Errorf("url is required for %s transport type", transport.Type)
|
||||
}
|
||||
// Validate URL format with template variable support
|
||||
if !IsValidTemplatedURL(transport.URL, availableVariables) {
|
||||
issue := NewValidationIssue(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("url").String(),
|
||||
fmt.Sprintf("url is required for %s transport type", transport.Type),
|
||||
ValidationIssueSeverityError,
|
||||
"streamable-transport-url-required",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
} else if !IsValidTemplatedURL(transport.URL, availableVariables) {
|
||||
// Check if it's a template variable issue or basic URL issue
|
||||
templateVars := extractTemplateVariables(transport.URL)
|
||||
var err error
|
||||
if len(templateVars) > 0 {
|
||||
return fmt.Errorf("%w: template variables in URL %s reference undefined variables. Available variables: %v",
|
||||
err = fmt.Errorf("%w: template variables in URL %s reference undefined variables. Available variables: %v",
|
||||
ErrInvalidPackageTransportURL, transport.URL, availableVariables)
|
||||
} else {
|
||||
err = fmt.Errorf("%w: %s", ErrInvalidPackageTransportURL, transport.URL)
|
||||
}
|
||||
return fmt.Errorf("%w: %s", ErrInvalidPackageTransportURL, transport.URL)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("url").String(),
|
||||
err,
|
||||
"invalid-templated-url",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
return nil
|
||||
default:
|
||||
return fmt.Errorf("unsupported transport type: %s", transport.Type)
|
||||
issue := NewValidationIssue(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("type").String(),
|
||||
fmt.Sprintf("unsupported transport type: %s", transport.Type),
|
||||
ValidationIssueSeverityError,
|
||||
"unsupported-transport-type",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
return result
|
||||
}
|
||||
|
||||
// validateRemoteTransport validates a remote transport with optional templating
|
||||
func validateRemoteTransport(obj *model.Transport) error {
|
||||
func validateRemoteTransport(ctx *ValidationContext, obj *model.Transport) *ValidationResult {
|
||||
result := &ValidationResult{Valid: true, Issues: []ValidationIssue{}}
|
||||
|
||||
// Validate transport type is supported - remotes only support streamable-http and sse
|
||||
switch obj.Type {
|
||||
case model.TransportTypeStreamableHTTP, model.TransportTypeSSE:
|
||||
// URL is required for streamable-http and sse
|
||||
if obj.URL == "" {
|
||||
return fmt.Errorf("url is required for %s transport type", obj.Type)
|
||||
issue := NewValidationIssue(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("url").String(),
|
||||
fmt.Sprintf("url is required for %s transport type", obj.Type),
|
||||
ValidationIssueSeverityError,
|
||||
"remote-transport-url-required",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
} else if !IsValidRemoteURL(obj.URL) {
|
||||
// Validate URL format (no templates allowed for remotes, no localhost)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("url").String(),
|
||||
fmt.Errorf("%w: %s", ErrInvalidRemoteURL, obj.URL),
|
||||
"invalid-remote-url",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
// Collect available variables from the transport's Variables field
|
||||
|
|
@ -425,21 +595,34 @@ func validateRemoteTransport(obj *model.Transport) error {
|
|||
if !IsValidTemplatedURL(obj.URL, availableVariables) {
|
||||
// Check if it's a template variable issue or basic URL issue
|
||||
templateVars := extractTemplateVariables(obj.URL)
|
||||
var err error
|
||||
if len(templateVars) > 0 {
|
||||
return fmt.Errorf("%w: template variables in URL %s reference undefined variables. Available variables: %v",
|
||||
err = fmt.Errorf("%w: template variables in URL %s reference undefined variables. Available variables: %v",
|
||||
ErrInvalidRemoteURL, obj.URL, availableVariables)
|
||||
} else {
|
||||
err = fmt.Errorf("%w: %s", ErrInvalidRemoteURL, obj.URL)
|
||||
}
|
||||
return fmt.Errorf("%w: %s", ErrInvalidRemoteURL, obj.URL)
|
||||
issue := NewValidationIssueFromError(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("url").String(),
|
||||
err,
|
||||
"invalid-templated-url",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
// Additional check: reject localhost URLs for remotes (like the old IsValidRemoteURL did)
|
||||
if !IsValidRemoteURL(obj.URL) {
|
||||
return fmt.Errorf("%w: %s", ErrInvalidRemoteURL, obj.URL)
|
||||
}
|
||||
return nil
|
||||
return result
|
||||
default:
|
||||
return fmt.Errorf("unsupported transport type for remotes: %s (only streamable-http and sse are supported)", obj.Type)
|
||||
issue := NewValidationIssue(
|
||||
ValidationIssueTypeSemantic,
|
||||
ctx.Field("type").String(),
|
||||
fmt.Sprintf("unsupported transport type for remotes: %s (only streamable-http and sse are supported)", obj.Type),
|
||||
ValidationIssueSeverityError,
|
||||
"unsupported-remote-transport-type",
|
||||
)
|
||||
result.AddIssue(issue)
|
||||
}
|
||||
|
||||
return result
|
||||
}
|
||||
|
||||
// ValidatePublishRequest validates a complete publish request including extensions
|
||||
|
|
@ -450,22 +633,46 @@ func ValidatePublishRequest(ctx context.Context, req apiv0.ServerJSON, cfg *conf
|
|||
}
|
||||
|
||||
// Validate the server detail (includes all nested validation)
|
||||
if err := ValidateServerJSON(&req); err != nil {
|
||||
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 {
|
||||
for i, pkg := range req.Packages {
|
||||
if err := ValidatePackage(ctx, pkg, req.Name); err != nil {
|
||||
return fmt.Errorf("registry validation failed for package %d (%s): %w", i, pkg.Identifier, err)
|
||||
}
|
||||
if err := validateRegistryOwnership(ctx, req); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func validateRegistryOwnership(ctx context.Context, req apiv0.ServerJSON) error {
|
||||
for i, pkg := range req.Packages {
|
||||
if err := ValidatePackage(ctx, pkg, req.Name); err != nil {
|
||||
return fmt.Errorf("registry validation failed for package %d (%s): %w", i, pkg.Identifier, err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func validatePublisherExtensions(req apiv0.ServerJSON) error {
|
||||
const maxExtensionSize = 4 * 1024 // 4KB limit
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue