diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 976215ea..d56dce49 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -165,8 +165,8 @@ func TestLoadFromStdin_UnsupportedType(t *testing.T) { // Should fail validation for unsupported type require.Error(t, err) - // Error should mention validation issue - assert.Contains(t, err.Error(), "validation error", "Expected validation error") + // Error should mention configuration error + assert.Contains(t, err.Error(), "Configuration error", "Expected configuration error") // Config should be nil on validation error assert.Nil(t, cfg, "Config should be nil when validation fails") @@ -782,11 +782,6 @@ func TestLoadFromStdin_InvalidMountFormat(t *testing.T) { mounts string errorMsg string }{ - { - name: "missing mode", - mounts: `["/host:/container"]`, - errorMsg: "validation error", - }, { name: "invalid mode", mounts: `["/host:/container:invalid"]`, diff --git a/internal/config/rules/rules.go b/internal/config/rules/rules.go index 58db5fff..b74c4d3a 100644 --- a/internal/config/rules/rules.go +++ b/internal/config/rules/rules.go @@ -8,7 +8,7 @@ import ( // Documentation URL constants const ( ConfigSpecURL = "https://github.com/githubnext/gh-aw/blob/main/docs/src/content/docs/reference/mcp-gateway.md" - SchemaURL = "https://github.com/githubnext/gh-aw/blob/main/docs/public/schemas/mcp-gateway-config.schema.json" + SchemaURL = "https://raw.githubusercontent.com/githubnext/gh-aw/main/docs/public/schemas/mcp-gateway-config.schema.json" ) // ValidationError represents a configuration validation error with context @@ -104,24 +104,29 @@ func TimeoutPositive(timeout int, fieldName, jsonPath string) *ValidationError { return nil } -// MountFormat validates a mount specification in the format "source:dest:mode" +// MountFormat validates a mount specification in the format "source:dest" or "source:dest:mode" // Returns nil if valid, *ValidationError if invalid // Per MCP Gateway specification v1.7.0 section 4.1.5: // - Host path MUST be an absolute path // - Container path MUST be an absolute path -// - Mode MUST be either "ro" (read-only) or "rw" (read-write) +// - Mode (if provided) MUST be either "ro" (read-only) or "rw" (read-write) func MountFormat(mount, jsonPath string, index int) *ValidationError { parts := strings.Split(mount, ":") - if len(parts) != 3 { + if len(parts) < 2 || len(parts) > 3 { return &ValidationError{ Field: "mounts", - Message: fmt.Sprintf("invalid mount format '%s' (expected 'source:dest:mode')", mount), + Message: fmt.Sprintf("invalid mount format '%s' (expected 'source:dest' or 'source:dest:mode')", mount), JSONPath: fmt.Sprintf("%s.mounts[%d]", jsonPath, index), - Suggestion: "Use format 'source:dest:mode' where mode is 'ro' (read-only) or 'rw' (read-write)", + Suggestion: "Use format 'source:dest' or 'source:dest:mode' where mode is 'ro' (read-only) or 'rw' (read-write)", } } - source, dest, mode := parts[0], parts[1], parts[2] + source := parts[0] + dest := parts[1] + mode := "" + if len(parts) == 3 { + mode = parts[2] + } // Validate source is not empty if source == "" { @@ -163,8 +168,8 @@ func MountFormat(mount, jsonPath string, index int) *ValidationError { } } - // Validate mode - if mode != "ro" && mode != "rw" { + // Validate mode if provided + if mode != "" && mode != "ro" && mode != "rw" { return &ValidationError{ Field: "mounts", Message: fmt.Sprintf("invalid mount mode '%s' (must be 'ro' or 'rw')", mode), diff --git a/internal/config/rules/rules_test.go b/internal/config/rules/rules_test.go index 77df07e8..8cc0ad97 100644 --- a/internal/config/rules/rules_test.go +++ b/internal/config/rules/rules_test.go @@ -189,12 +189,11 @@ func TestMountFormat(t *testing.T) { shouldErr: false, }, { - name: "invalid format - missing mode", + name: "valid mount without mode", mount: "/host/path:/container/path", jsonPath: "mcpServers.github", index: 0, - shouldErr: true, - errMsg: "invalid mount format", + shouldErr: false, }, { name: "invalid format - too many colons", diff --git a/internal/config/schema_validation.go b/internal/config/schema_validation.go index cd394fb4..4b83a671 100644 --- a/internal/config/schema_validation.go +++ b/internal/config/schema_validation.go @@ -1,24 +1,23 @@ package config import ( - _ "embed" "encoding/json" "fmt" + "io" + "net/http" "regexp" "strings" + "time" "github.com/githubnext/gh-aw-mcpg/internal/config/rules" "github.com/santhosh-tekuri/jsonschema/v5" ) -//go:embed schemas/mcp-gateway-config.schema.json -var schemaJSON []byte - var ( // Compile regex patterns from schema for additional validation containerPattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9./_-]*(:([a-zA-Z0-9._-]+|latest))?$`) urlPattern = regexp.MustCompile(`^https?://.+`) - mountPattern = regexp.MustCompile(`^[^:]+:[^:]+:(ro|rw)$`) + mountPattern = regexp.MustCompile(`^[^:]+:[^:]+(:(ro|rw))?$`) domainVarPattern = regexp.MustCompile(`^\$\{[A-Z_][A-Z0-9_]*\}$`) // gatewayVersion stores the version string to include in error messages @@ -32,25 +31,126 @@ func SetVersion(version string) { } } +// fetchAndFixSchema fetches the JSON schema from the remote URL and fixes +// regex patterns that use negative lookahead (not supported in JSON Schema Draft 7) +func fetchAndFixSchema(url string) ([]byte, error) { + client := &http.Client{ + Timeout: 10 * time.Second, + } + + resp, err := client.Get(url) + if err != nil { + return nil, fmt.Errorf("failed to fetch schema from %s: %w", url, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("failed to fetch schema: HTTP %d", resp.StatusCode) + } + + schemaBytes, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read schema response: %w", err) + } + + // Fix regex patterns that use negative lookahead + var schema map[string]interface{} + if err := json.Unmarshal(schemaBytes, &schema); err != nil { + return nil, fmt.Errorf("failed to parse schema: %w", err) + } + + // Fix the customServerConfig pattern that uses negative lookahead + // The oneOf constraint in mcpServerConfig will still ensure that stdio/http + // types are validated correctly. We replace the pattern with an enum that excludes + // stdio and http, which achieves the same validation goal without negative lookahead. + if definitions, ok := schema["definitions"].(map[string]interface{}); ok { + if customServerConfig, ok := definitions["customServerConfig"].(map[string]interface{}); ok { + if properties, ok := customServerConfig["properties"].(map[string]interface{}); ok { + if typeField, ok := properties["type"].(map[string]interface{}); ok { + // Remove the pattern entirely - the oneOf logic combined with the fact + // that stdioServerConfig has enum: ["stdio"] and httpServerConfig has + // enum: ["http"] will ensure proper validation + delete(typeField, "pattern") + // Also remove the type constraint since we want it to only match in the oneOf context + delete(typeField, "type") + // Add a not constraint to exclude stdio and http + typeField["not"] = map[string]interface{}{ + "enum": []string{"stdio", "http"}, + } + } + } + } + } + + // Fix the customSchemas patternProperties + if properties, ok := schema["properties"].(map[string]interface{}); ok { + if customSchemas, ok := properties["customSchemas"].(map[string]interface{}); ok { + if patternProps, ok := customSchemas["patternProperties"].(map[string]interface{}); ok { + // Find and replace the pattern property key with negative lookahead + for key, value := range patternProps { + if strings.Contains(key, "(?!") { + // Replace with a simple pattern that matches any lowercase word + // The validation logic will handle ensuring it's not stdio/http + delete(patternProps, key) + patternProps["^[a-z][a-z0-9-]*$"] = value + break + } + } + } + } + } + + fixedBytes, err := json.Marshal(schema) + if err != nil { + return nil, fmt.Errorf("failed to marshal fixed schema: %w", err) + } + + return fixedBytes, nil +} + // validateJSONSchema validates the raw JSON configuration against the JSON schema func validateJSONSchema(data []byte) error { + // Fetch the schema from the remote URL (source of truth) + schemaURL := "https://raw.githubusercontent.com/githubnext/gh-aw/main/docs/public/schemas/mcp-gateway-config.schema.json" + schemaJSON, err := fetchAndFixSchema(schemaURL) + if err != nil { + return fmt.Errorf("failed to fetch schema: %w", err) + } + // Parse the schema var schemaData interface{} if err := json.Unmarshal(schemaJSON, &schemaData); err != nil { - return fmt.Errorf("failed to parse embedded schema: %w", err) + return fmt.Errorf("failed to parse schema: %w", err) } // Compile the schema compiler := jsonschema.NewCompiler() compiler.Draft = jsonschema.Draft7 - // Add the schema with its $id - schemaURL := "https://github.com/githubnext/gh-aw/blob/main/docs/public/schemas/mcp-gateway-config.schema.json" + // Add the schema with its $id from the remote schema + // Note: The remote schema uses https://docs.github.com/gh-aw/schemas/mcp-gateway-config.schema.json + // as its $id, so we need to register it there as well + var schemaObj map[string]interface{} + if err := json.Unmarshal(schemaJSON, &schemaObj); err != nil { + return fmt.Errorf("failed to parse schema JSON: %w", err) + } + + schemaID, ok := schemaObj["$id"].(string) + if !ok || schemaID == "" { + schemaID = schemaURL + } + + // Add the schema with both URLs if err := compiler.AddResource(schemaURL, strings.NewReader(string(schemaJSON))); err != nil { return fmt.Errorf("failed to add schema resource: %w", err) } + if schemaID != schemaURL { + if err := compiler.AddResource(schemaID, strings.NewReader(string(schemaJSON))); err != nil { + return fmt.Errorf("failed to add schema resource with $id: %w", err) + } + } - schema, err := compiler.Compile(schemaURL) + schema, err := compiler.Compile(schemaID) if err != nil { return fmt.Errorf("failed to compile schema: %w", err) } diff --git a/internal/config/schema_validation_test.go b/internal/config/schema_validation_test.go index 62a0f936..ebd02a04 100644 --- a/internal/config/schema_validation_test.go +++ b/internal/config/schema_validation_test.go @@ -391,7 +391,7 @@ func TestValidateStringPatterns(t *testing.T) { shouldErr: false, }, { - name: "invalid mount pattern - missing mode", + name: "valid mount without mode", config: &StdinConfig{ MCPServers: map[string]*StdinServerConfig{ "test": { @@ -401,8 +401,7 @@ func TestValidateStringPatterns(t *testing.T) { }, }, }, - shouldErr: true, - errorMsg: "does not match required pattern", + shouldErr: false, }, { name: "valid http url pattern", @@ -556,7 +555,7 @@ func TestEnhancedErrorMessages(t *testing.T) { "Location:", "Error:", "Details:", - "https://github.com/githubnext/gh-aw/blob/main/docs/public/schemas/mcp-gateway-config.schema.json", + "https://raw.githubusercontent.com/githubnext/gh-aw/main/docs/public/schemas/mcp-gateway-config.schema.json", }, }, { diff --git a/internal/config/schemas/mcp-gateway-config.schema.json b/internal/config/schemas/mcp-gateway-config.schema.json deleted file mode 100644 index bf21bd07..00000000 --- a/internal/config/schemas/mcp-gateway-config.schema.json +++ /dev/null @@ -1,232 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "$id": "https://github.com/githubnext/gh-aw/blob/main/docs/public/schemas/mcp-gateway-config.schema.json", - "title": "MCP Gateway Configuration", - "description": "Configuration schema for the Model Context Protocol (MCP) Gateway as defined in the MCP Gateway Specification v1.0.0. The gateway provides transparent HTTP access to multiple MCP servers with protocol translation, server isolation, and authentication capabilities.", - "type": "object", - "properties": { - "mcpServers": { - "type": "object", - "description": "Map of MCP server configurations. Each key is a unique server identifier, and the value is the server configuration.", - "additionalProperties": { - "$ref": "#/definitions/mcpServerConfig" - } - }, - "gateway": { - "$ref": "#/definitions/gatewayConfig", - "description": "Gateway-specific configuration for the MCP Gateway service." - } - }, - "required": ["mcpServers", "gateway"], - "additionalProperties": false, - "definitions": { - "mcpServerConfig": { - "type": "object", - "description": "Configuration for an individual MCP server. Supports both containerized stdio servers and HTTP-based servers. Per MCP Gateway Specification v1.0.0 section 3.2.1, stdio-based servers MUST be containerized.", - "oneOf": [ - { - "$ref": "#/definitions/stdioServerConfig" - }, - { - "$ref": "#/definitions/httpServerConfig" - } - ] - }, - "stdioServerConfig": { - "type": "object", - "description": "Configuration for a containerized stdio-based MCP server. The gateway communicates with the server via standard input/output streams. Per MCP Gateway Specification section 3.2.1, all stdio servers MUST be containerized - direct command execution is not supported.", - "properties": { - "type": { - "type": "string", - "enum": ["stdio"], - "description": "Transport type for the MCP server. For containerized servers, use 'stdio'.", - "default": "stdio" - }, - "container": { - "type": "string", - "description": "Container image for the MCP server (e.g., 'ghcr.io/example/mcp-server:latest'). This field is required for stdio servers per MCP Gateway Specification section 4.1.2.", - "minLength": 1, - "pattern": "^[a-zA-Z0-9][a-zA-Z0-9./_-]*(:([a-zA-Z0-9._-]+|latest))?$" - }, - "entrypoint": { - "type": "string", - "description": "Optional entrypoint override for the container, equivalent to 'docker run --entrypoint'. If not specified, the container's default entrypoint is used.", - "minLength": 1 - }, - "entrypointArgs": { - "type": "array", - "description": "Arguments passed to the container entrypoint. These are executed inside the container after the entrypoint command.", - "items": { - "type": "string" - }, - "default": [] - }, - "mounts": { - "type": "array", - "description": "Volume mounts for the container. Format: 'source:dest:mode' where mode is 'ro' (read-only) or 'rw' (read-write). Example: '/host/data:/container/data:ro'", - "items": { - "type": "string", - "pattern": "^[^:]+:[^:]+:(ro|rw)$" - }, - "default": [] - }, - "env": { - "type": "object", - "description": "Environment variables for the server process. Values may contain variable expressions using '${VARIABLE_NAME}' syntax, which will be resolved from the process environment.", - "additionalProperties": { - "type": "string" - }, - "default": {} - }, - "args": { - "type": "array", - "description": "Additional Docker runtime arguments passed before the container image (e.g., '--network', 'host').", - "items": { - "type": "string" - }, - "default": [] - } - }, - "required": ["container"], - "additionalProperties": false - }, - "httpServerConfig": { - "type": "object", - "description": "Configuration for an HTTP-based MCP server. The gateway forwards requests directly to the specified HTTP endpoint.", - "properties": { - "type": { - "type": "string", - "enum": ["http"], - "description": "Transport type for the MCP server. For HTTP servers, use 'http'." - }, - "url": { - "type": "string", - "description": "HTTP endpoint URL for the MCP server (e.g., 'https://api.example.com/mcp'). This field is required for HTTP servers per MCP Gateway Specification section 4.1.2.", - "format": "uri", - "pattern": "^https?://.+", - "minLength": 1 - }, - "headers": { - "type": "object", - "description": "HTTP headers to include in requests to the server. Commonly used for authentication (e.g., Authorization: 'Bearer ${API_TOKEN}'). Values may contain variable expressions using '${VARIABLE_NAME}' syntax.", - "additionalProperties": { - "type": "string" - }, - "default": {} - } - }, - "required": ["type", "url"], - "additionalProperties": false - }, - "gatewayConfig": { - "type": "object", - "description": "Gateway-specific configuration for the MCP Gateway service.", - "properties": { - "port": { - "oneOf": [ - { - "type": "integer", - "minimum": 1, - "maximum": 65535 - }, - { - "type": "string", - "pattern": "^\\$\\{[A-Z_][A-Z0-9_]*\\}$" - } - ], - "description": "HTTP server port for the gateway. The gateway exposes endpoints at http://{domain}:{port}/. Can be an integer (1-65535) or a variable expression like '${MCP_GATEWAY_PORT}'." - }, - "apiKey": { - "type": "string", - "description": "API key for authentication. When configured, clients must include 'Authorization: {apiKey}' header in all RPC requests. Per MCP Gateway Specification section 7, API keys must not be logged in plaintext.", - "minLength": 1 - }, - "domain": { - "oneOf": [ - { - "type": "string", - "enum": ["localhost", "host.docker.internal"] - }, - { - "type": "string", - "pattern": "^\\$\\{[A-Z_][A-Z0-9_]*\\}$" - } - ], - "description": "Gateway domain for constructing URLs. Use 'localhost' for local development or 'host.docker.internal' when the gateway runs in a container and needs to access the host. Can also be a variable expression like '${MCP_GATEWAY_DOMAIN}'." - }, - "startupTimeout": { - "type": "integer", - "description": "Server startup timeout in seconds. The gateway enforces this timeout when initializing containerized stdio servers.", - "minimum": 1, - "default": 30 - }, - "toolTimeout": { - "type": "integer", - "description": "Tool invocation timeout in seconds. The gateway enforces this timeout for individual tool/method calls to MCP servers.", - "minimum": 1, - "default": 60 - } - }, - "required": ["port", "domain", "apiKey"], - "additionalProperties": false - } - }, - "examples": [ - { - "mcpServers": { - "github": { - "container": "ghcr.io/github/github-mcp-server:latest", - "env": { - "GITHUB_PERSONAL_ACCESS_TOKEN": "${GITHUB_TOKEN}" - } - } - }, - "gateway": { - "port": 8080, - "domain": "localhost", - "apiKey": "gateway-secret-token" - } - }, - { - "mcpServers": { - "data-server": { - "container": "ghcr.io/example/data-mcp:latest", - "entrypoint": "/custom/entrypoint.sh", - "entrypointArgs": ["--config", "/app/config.json"], - "mounts": [ - "/host/data:/container/data:ro", - "/host/config:/container/config:rw" - ], - "type": "stdio" - } - }, - "gateway": { - "port": 8080, - "domain": "localhost", - "startupTimeout": 60, - "toolTimeout": 120 - } - }, - { - "mcpServers": { - "local-server": { - "container": "ghcr.io/example/python-mcp:latest", - "entrypointArgs": ["--config", "/app/config.json"], - "type": "stdio" - }, - "remote-server": { - "type": "http", - "url": "https://api.example.com/mcp", - "headers": { - "Authorization": "Bearer ${API_TOKEN}" - } - } - }, - "gateway": { - "port": 8080, - "domain": "localhost", - "apiKey": "gateway-secret-token" - } - } - ] -} diff --git a/internal/config/validation_test.go b/internal/config/validation_test.go index 7399e38b..0f1b5f6b 100644 --- a/internal/config/validation_test.go +++ b/internal/config/validation_test.go @@ -287,14 +287,13 @@ func TestValidateStdioServer(t *testing.T) { shouldErr: false, }, { - name: "invalid mount format - missing mode", + name: "valid mount without mode", server: &StdinServerConfig{ Type: "stdio", Container: "test:latest", Mounts: []string{"/host:/container"}, }, - shouldErr: true, - errorMsg: "invalid mount format", + shouldErr: false, }, { name: "invalid mount format - too many parts",