diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 6d7b0653e7..289e43fb88 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -73,6 +73,11 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) return nil, fmt.Errorf("%s: %w", cleanPath, err) } + // Validate GitHub guard policy configuration + if err := validateGitHubGuardPolicy(workflowData.ParsedTools, workflowData.Name); err != nil { + return nil, fmt.Errorf("%s: %w", cleanPath, err) + } + // Use shared action cache and resolver from the compiler actionCache, actionResolver := c.getSharedActionResolver() workflowData.ActionCache = actionCache diff --git a/pkg/workflow/compiler_string_api.go b/pkg/workflow/compiler_string_api.go index 30fe1c61c7..94f65371dc 100644 --- a/pkg/workflow/compiler_string_api.go +++ b/pkg/workflow/compiler_string_api.go @@ -130,6 +130,11 @@ func (c *Compiler) ParseWorkflowString(content string, virtualPath string) (*Wor return nil, fmt.Errorf("%s: %w", cleanPath, err) } + // Validate GitHub guard policy configuration + if err := validateGitHubGuardPolicy(workflowData.ParsedTools, workflowData.Name); err != nil { + return nil, fmt.Errorf("%s: %w", cleanPath, err) + } + // Setup action cache and resolver actionCache, actionResolver := c.getSharedActionResolver() workflowData.ActionCache = actionCache diff --git a/pkg/workflow/mcp_github_config.go b/pkg/workflow/mcp_github_config.go index 1e954ccaa4..9da6c1949d 100644 --- a/pkg/workflow/mcp_github_config.go +++ b/pkg/workflow/mcp_github_config.go @@ -241,6 +241,29 @@ func getGitHubAllowedTools(githubTool any) []string { return nil } +// getGitHubGuardPolicies extracts guard policies from GitHub tool configuration. +// It reads the flat repos/min-integrity fields and wraps them for MCP gateway rendering. +// Returns nil if no guard policies are configured. +func getGitHubGuardPolicies(githubTool any) map[string]any { + if toolConfig, ok := githubTool.(map[string]any); ok { + repos, hasRepos := toolConfig["repos"] + integrity, hasIntegrity := toolConfig["min-integrity"] + if hasRepos || hasIntegrity { + policy := map[string]any{} + if hasRepos { + policy["repos"] = repos + } + if hasIntegrity { + policy["min-integrity"] = integrity + } + return map[string]any{ + "allow-only": policy, + } + } + } + return nil +} + func getGitHubDockerImageVersion(githubTool any) string { githubDockerImageVersion := string(constants.DefaultGitHubMCPServerVersion) // Default Docker image version // Extract version setting from tool properties diff --git a/pkg/workflow/mcp_renderer.go b/pkg/workflow/mcp_renderer.go index 79395b84b5..f1c2605145 100644 --- a/pkg/workflow/mcp_renderer.go +++ b/pkg/workflow/mcp_renderer.go @@ -76,6 +76,7 @@ package workflow import ( + "encoding/json" "fmt" "os" "sort" @@ -168,6 +169,7 @@ func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, github IncludeToolsField: r.options.IncludeCopilotFields, AllowedTools: getGitHubAllowedTools(githubTool), IncludeEnvSection: r.options.IncludeCopilotFields, + GuardPolicies: getGitHubGuardPolicies(githubTool), }) } else { // Local mode - use Docker-based GitHub MCP server (default) @@ -186,6 +188,7 @@ func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, github IncludeTypeField: r.options.IncludeCopilotFields, AllowedTools: getGitHubAllowedTools(githubTool), EffectiveToken: "", // Token passed via env + GuardPolicies: getGitHubGuardPolicies(githubTool), }) } @@ -676,6 +679,8 @@ type GitHubMCPDockerOptions struct { EffectiveToken string // Mounts specifies volume mounts for the GitHub MCP server container (format: "host:container:mode") Mounts []string + // GuardPolicies specifies access control policies for the MCP gateway (e.g., allow-only repos/integrity) + GuardPolicies map[string]any } // RenderGitHubMCPDockerConfig renders the GitHub MCP server configuration for Docker (local mode). @@ -771,7 +776,13 @@ func RenderGitHubMCPDockerConfig(yaml *strings.Builder, options GitHubMCPDockerO fmt.Fprintf(yaml, " \"%s\": \"%s\"%s\n", key, envVars[key], comma) } - yaml.WriteString(" }\n") + // Close env section, with trailing comma if guard-policies follows + if len(options.GuardPolicies) > 0 { + yaml.WriteString(" },\n") + renderGuardPoliciesJSON(yaml, options.GuardPolicies, " ") + } else { + yaml.WriteString(" }\n") + } } // GitHubMCPRemoteOptions defines configuration for GitHub MCP remote mode rendering @@ -794,6 +805,8 @@ type GitHubMCPRemoteOptions struct { AllowedTools []string // IncludeEnvSection indicates whether to include the env section (Copilot needs it, Claude doesn't) IncludeEnvSection bool + // GuardPolicies specifies access control policies for the MCP gateway (e.g., allow-only repos/integrity) + GuardPolicies map[string]any } // RenderGitHubMCPRemoteConfig renders the GitHub MCP server configuration for remote (hosted) mode. @@ -836,7 +849,7 @@ func RenderGitHubMCPRemoteConfig(yaml *strings.Builder, options GitHubMCPRemoteO writeHeadersToYAML(yaml, headers, " ") // Close headers section - if options.IncludeToolsField || options.IncludeEnvSection { + if options.IncludeToolsField || options.IncludeEnvSection || len(options.GuardPolicies) > 0 { yaml.WriteString(" },\n") } else { yaml.WriteString(" }\n") @@ -856,7 +869,7 @@ func RenderGitHubMCPRemoteConfig(yaml *strings.Builder, options GitHubMCPRemoteO } yaml.WriteString("\n") } - if options.IncludeEnvSection { + if options.IncludeEnvSection || len(options.GuardPolicies) > 0 { yaml.WriteString(" ],\n") } else { yaml.WriteString(" ]\n") @@ -867,10 +880,38 @@ func RenderGitHubMCPRemoteConfig(yaml *strings.Builder, options GitHubMCPRemoteO if options.IncludeEnvSection { yaml.WriteString(" \"env\": {\n") yaml.WriteString(" \"GITHUB_PERSONAL_ACCESS_TOKEN\": \"\\${GITHUB_MCP_SERVER_TOKEN}\"\n") - yaml.WriteString(" }\n") + // Close env section, with trailing comma if guard-policies follows + if len(options.GuardPolicies) > 0 { + yaml.WriteString(" },\n") + } else { + yaml.WriteString(" }\n") + } + } + + // Add guard-policies if configured + if len(options.GuardPolicies) > 0 { + renderGuardPoliciesJSON(yaml, options.GuardPolicies, " ") } } +// renderGuardPoliciesJSON renders a "guard-policies" JSON field at the given indent level. +// The policies map contains policy names (e.g., "allow-only") mapped to their configurations. +// Renders as the last field (no trailing comma) with the given base indent. +func renderGuardPoliciesJSON(yaml *strings.Builder, policies map[string]any, indent string) { + if len(policies) == 0 { + return + } + + // Marshal to JSON with indentation, then re-indent to match the current indent level + jsonBytes, err := json.MarshalIndent(policies, indent, " ") + if err != nil { + mcpRendererLog.Printf("Failed to marshal guard-policies: %v", err) + return + } + + fmt.Fprintf(yaml, "%s\"guard-policies\": %s\n", indent, string(jsonBytes)) +} + // RenderJSONMCPConfig renders MCP configuration in JSON format with the common mcpServers structure. // This shared function extracts the duplicate pattern from Claude, Copilot, and Custom engines. // diff --git a/pkg/workflow/schemas/mcp-gateway-config.schema.json b/pkg/workflow/schemas/mcp-gateway-config.schema.json index c5bc9bbda8..5fc0eae654 100644 --- a/pkg/workflow/schemas/mcp-gateway-config.schema.json +++ b/pkg/workflow/schemas/mcp-gateway-config.schema.json @@ -93,6 +93,11 @@ "type": "string" }, "default": ["*"] + }, + "guard-policies": { + "type": "object", + "description": "Guard policies for access control at the MCP gateway level. The structure of guard policies is server-specific. For GitHub MCP server, see the GitHub guard policy schema. For other servers (Jira, WorkIQ), different policy schemas will apply.", + "additionalProperties": true } }, "required": ["container"], @@ -137,6 +142,11 @@ "type": "string" }, "default": {} + }, + "guard-policies": { + "type": "object", + "description": "Guard policies for access control at the MCP gateway level. The structure of guard policies is server-specific. For GitHub MCP server, see the GitHub guard policy schema. For other servers (Jira, WorkIQ), different policy schemas will apply.", + "additionalProperties": true } }, "required": ["type", "url"], diff --git a/pkg/workflow/tools_parser.go b/pkg/workflow/tools_parser.go index 8998a39044..810b458e96 100644 --- a/pkg/workflow/tools_parser.go +++ b/pkg/workflow/tools_parser.go @@ -234,6 +234,14 @@ func parseGitHubTool(val any) *GitHubToolConfig { config.App = parseAppConfig(app) } + // Parse guard policy fields (flat syntax: repos and min-integrity directly under github:) + if repos, ok := configMap["repos"]; ok { + config.Repos = repos // Store as-is, validation will happen later + } + if integrity, ok := configMap["min-integrity"].(string); ok { + config.MinIntegrity = GitHubIntegrityLevel(integrity) + } + return config } diff --git a/pkg/workflow/tools_types.go b/pkg/workflow/tools_types.go index 12c30a6bfd..65624a2181 100644 --- a/pkg/workflow/tools_types.go +++ b/pkg/workflow/tools_types.go @@ -152,6 +152,11 @@ func mcpServerConfigToMap(config MCPServerConfig) map[string]any { result["mounts"] = config.Mounts } + // Add guard policies if set + if len(config.GuardPolicies) > 0 { + result["guard-policies"] = config.GuardPolicies + } + // Add custom fields (these override standard fields if there are conflicts) maps.Copy(result, config.CustomFields) @@ -257,6 +262,24 @@ func (g GitHubToolsets) ToStringSlice() []string { return result } +// GitHubIntegrityLevel represents the minimum integrity level required for repository access +type GitHubIntegrityLevel string + +const ( + // GitHubIntegrityNone allows access with no integrity requirements + GitHubIntegrityNone GitHubIntegrityLevel = "none" + // GitHubIntegrityReader requires read-level integrity + GitHubIntegrityReader GitHubIntegrityLevel = "reader" + // GitHubIntegrityWriter requires write-level integrity + GitHubIntegrityWriter GitHubIntegrityLevel = "writer" + // GitHubIntegrityMerged requires merged-level integrity + GitHubIntegrityMerged GitHubIntegrityLevel = "merged" +) + +// GitHubReposScope represents the repository scope for guard policy enforcement +// Can be one of: "all", "public", or an array of repository patterns +type GitHubReposScope any // string or []any (YAML-parsed arrays are []any) + // GitHubToolConfig represents the configuration for the GitHub tool // Can be nil (enabled with defaults), string, or an object with specific settings type GitHubToolConfig struct { @@ -269,6 +292,13 @@ type GitHubToolConfig struct { Toolset GitHubToolsets `yaml:"toolsets,omitempty"` Lockdown bool `yaml:"lockdown,omitempty"` App *GitHubAppConfig `yaml:"app,omitempty"` // GitHub App configuration for token minting + + // Guard policy fields (flat syntax under github:) + // Repos defines the access scope for policy enforcement. + // Supports: "all", "public", or an array of patterns ["owner/repo", "owner/*"] (lowercase) + Repos GitHubReposScope `yaml:"repos,omitempty"` + // MinIntegrity defines the minimum integrity level required: "none", "reader", "writer", "merged" + MinIntegrity GitHubIntegrityLevel `yaml:"min-integrity,omitempty"` } // PlaywrightToolConfig represents the configuration for the Playwright tool @@ -339,6 +369,12 @@ type MCPServerConfig struct { Mode string `yaml:"mode,omitempty"` // MCP server mode (stdio, http, remote, local) Toolsets []string `yaml:"toolsets,omitempty"` // Toolsets to enable + // Guard policies for access control at the MCP gateway level + // This is a general field that can hold server-specific policy configurations + // For GitHub: policies are represented via GitHubAllowOnlyPolicy on GitHubToolConfig + // For Jira/WorkIQ: define similar server-specific policy types + GuardPolicies map[string]any `yaml:"guard-policies,omitempty"` + // For truly dynamic configuration (server-specific fields not covered above) CustomFields map[string]any `yaml:",inline"` } diff --git a/pkg/workflow/tools_validation.go b/pkg/workflow/tools_validation.go index 17a06f4efe..49ea7ad3d2 100644 --- a/pkg/workflow/tools_validation.go +++ b/pkg/workflow/tools_validation.go @@ -87,6 +87,171 @@ func validateGitHubToolConfig(tools *Tools, workflowName string) error { return nil } +// validateGitHubGuardPolicy validates the GitHub guard policy configuration. +// Guard policy fields (repos, min-integrity) are specified flat under github:. +// Both fields must be present if either is specified. +func validateGitHubGuardPolicy(tools *Tools, workflowName string) error { + if tools == nil || tools.GitHub == nil { + return nil + } + + github := tools.GitHub + hasRepos := github.Repos != nil + hasMinIntegrity := github.MinIntegrity != "" + + // No guard policy fields present - nothing to validate + if !hasRepos && !hasMinIntegrity { + return nil + } + + // Validate repos field (required when min-integrity is set) + if !hasRepos { + toolsValidationLog.Printf("Missing repos in guard policy for workflow: %s", workflowName) + return errors.New("invalid guard policy: 'github.repos' is required. Use 'all', 'public', or an array of repository patterns (e.g., ['owner/repo', 'owner/*'])") + } + + // Validate repos format + if err := validateReposScope(github.Repos, workflowName); err != nil { + return err + } + + // Validate min-integrity field (required when repos is set) + if !hasMinIntegrity { + toolsValidationLog.Printf("Missing min-integrity in guard policy for workflow: %s", workflowName) + return errors.New("invalid guard policy: 'github.min-integrity' is required. Valid values: 'none', 'reader', 'writer', 'merged'") + } + + // Validate min-integrity value + validIntegrityLevels := map[GitHubIntegrityLevel]bool{ + GitHubIntegrityNone: true, + GitHubIntegrityReader: true, + GitHubIntegrityWriter: true, + GitHubIntegrityMerged: true, + } + + if !validIntegrityLevels[github.MinIntegrity] { + toolsValidationLog.Printf("Invalid min-integrity level '%s' in workflow: %s", github.MinIntegrity, workflowName) + return errors.New("invalid guard policy: 'github.min-integrity' must be one of: 'none', 'reader', 'writer', 'merged'. Got: '" + string(github.MinIntegrity) + "'") + } + + return nil +} + +// validateReposScope validates the repos field in the guard policy +func validateReposScope(repos any, workflowName string) error { + // Case 1: String value ("all" or "public") + if reposStr, ok := repos.(string); ok { + if reposStr != "all" && reposStr != "public" { + toolsValidationLog.Printf("Invalid repos string '%s' in workflow: %s", reposStr, workflowName) + return errors.New("invalid guard policy: 'github.repos' string must be 'all' or 'public'. Got: '" + reposStr + "'") + } + return nil + } + + // Case 2a: Array of patterns from YAML parsing ([]any) + if reposArray, ok := repos.([]any); ok { + if len(reposArray) == 0 { + toolsValidationLog.Printf("Empty repos array in workflow: %s", workflowName) + return errors.New("invalid guard policy: 'github.repos' array cannot be empty. Provide at least one repository pattern") + } + + for i, item := range reposArray { + pattern, ok := item.(string) + if !ok { + toolsValidationLog.Printf("Non-string item in repos array at index %d in workflow: %s", i, workflowName) + return errors.New("invalid guard policy: 'github.repos' array must contain only strings") + } + + if err := validateRepoPattern(pattern, workflowName); err != nil { + return err + } + } + + return nil + } + + // Case 2b: Array of patterns from programmatic construction ([]string) + if reposArray, ok := repos.([]string); ok { + if len(reposArray) == 0 { + toolsValidationLog.Printf("Empty repos array in workflow: %s", workflowName) + return errors.New("invalid guard policy: 'github.repos' array cannot be empty. Provide at least one repository pattern") + } + + for _, pattern := range reposArray { + if err := validateRepoPattern(pattern, workflowName); err != nil { + return err + } + } + + return nil + } + + // Invalid type + toolsValidationLog.Printf("Invalid repos type in workflow: %s", workflowName) + return errors.New("invalid guard policy: 'github.repos' must be 'all', 'public', or an array of repository patterns") +} + +// validateRepoPattern validates a single repository pattern +func validateRepoPattern(pattern string, workflowName string) error { + // Pattern must be lowercase + if strings.ToLower(pattern) != pattern { + toolsValidationLog.Printf("Repository pattern '%s' is not lowercase in workflow: %s", pattern, workflowName) + return errors.New("invalid guard policy: repository pattern '" + pattern + "' must be lowercase") + } + + // Check for valid pattern formats: + // 1. owner/repo (exact match) + // 2. owner/* (owner wildcard) + // 3. owner/re* (repository prefix wildcard) + parts := strings.Split(pattern, "/") + if len(parts) != 2 { + toolsValidationLog.Printf("Invalid repository pattern '%s' in workflow: %s", pattern, workflowName) + return errors.New("invalid guard policy: repository pattern '" + pattern + "' must be in format 'owner/repo', 'owner/*', or 'owner/prefix*'") + } + + owner := parts[0] + repo := parts[1] + + // Validate owner part (must be non-empty and contain only valid characters) + if owner == "" { + return errors.New("invalid guard policy: repository pattern '" + pattern + "' has empty owner") + } + + if !isValidOwnerOrRepo(owner) { + return errors.New("invalid guard policy: repository pattern '" + pattern + "' has invalid owner. Must contain only lowercase letters, numbers, hyphens, and underscores") + } + + // Validate repo part + if repo == "" { + return errors.New("invalid guard policy: repository pattern '" + pattern + "' has empty repository name") + } + + // Allow wildcard '*' or prefix with trailing '*' + if repo != "*" && !isValidOwnerOrRepo(strings.TrimSuffix(repo, "*")) { + return errors.New("invalid guard policy: repository pattern '" + pattern + "' has invalid repository name. Must contain only lowercase letters, numbers, hyphens, underscores, or be '*' or 'prefix*'") + } + + // Validate that wildcard is only at the end (not in the middle) + if strings.Contains(strings.TrimSuffix(repo, "*"), "*") { + return errors.New("invalid guard policy: repository pattern '" + pattern + "' has wildcard in the middle. Wildcards only allowed at the end (e.g., 'prefix*')") + } + + return nil +} + +// isValidOwnerOrRepo checks if a string contains only valid GitHub owner/repo characters +func isValidOwnerOrRepo(s string) bool { + if s == "" { + return false + } + for _, ch := range s { + if (ch < 'a' || ch > 'z') && (ch < '0' || ch > '9') && ch != '-' && ch != '_' { + return false + } + } + return true +} + // Note: validateGitToolForSafeOutputs was removed because git commands are automatically // injected by the compiler when safe-outputs needs them (see compiler_safe_outputs.go). // The validation was misleading - it would fail even though the compiler would add the diff --git a/pkg/workflow/tools_validation_test.go b/pkg/workflow/tools_validation_test.go index 14097da97d..ee34344ed1 100644 --- a/pkg/workflow/tools_validation_test.go +++ b/pkg/workflow/tools_validation_test.go @@ -355,3 +355,206 @@ func TestValidateGitHubToolConfig(t *testing.T) { }) } } + +func TestValidateGitHubGuardPolicy(t *testing.T) { + tests := []struct { + name string + toolsMap map[string]any + shouldError bool + errorMsg string + }{ + { + name: "nil tools is valid", + toolsMap: nil, + shouldError: false, + }, + { + name: "no github tool is valid", + toolsMap: map[string]any{"bash": true}, + shouldError: false, + }, + { + name: "github tool without guard policy fields is valid", + toolsMap: map[string]any{"github": map[string]any{"mode": "remote"}}, + shouldError: false, + }, + { + name: "valid guard policy with repos=all", + toolsMap: map[string]any{ + "github": map[string]any{ + "repos": "all", + "min-integrity": "reader", + }, + }, + shouldError: false, + }, + { + name: "valid guard policy with repos=public", + toolsMap: map[string]any{ + "github": map[string]any{ + "repos": "public", + "min-integrity": "writer", + }, + }, + shouldError: false, + }, + { + name: "valid guard policy with repos array ([]any)", + toolsMap: map[string]any{ + "github": map[string]any{ + "repos": []any{"owner/repo", "owner/*"}, + "min-integrity": "merged", + }, + }, + shouldError: false, + }, + { + name: "valid guard policy with min-integrity=none", + toolsMap: map[string]any{ + "github": map[string]any{ + "repos": "all", + "min-integrity": "none", + }, + }, + shouldError: false, + }, + { + name: "missing repos field", + toolsMap: map[string]any{ + "github": map[string]any{ + "min-integrity": "reader", + }, + }, + shouldError: true, + errorMsg: "'github.repos' is required", + }, + { + name: "missing min-integrity field", + toolsMap: map[string]any{ + "github": map[string]any{ + "repos": "all", + }, + }, + shouldError: true, + errorMsg: "'github.min-integrity' is required", + }, + { + name: "invalid min-integrity value", + toolsMap: map[string]any{ + "github": map[string]any{ + "repos": "all", + "min-integrity": "superuser", + }, + }, + shouldError: true, + errorMsg: "'github.min-integrity' must be one of", + }, + { + name: "invalid repos string value", + toolsMap: map[string]any{ + "github": map[string]any{ + "repos": "private", + "min-integrity": "reader", + }, + }, + shouldError: true, + errorMsg: "'github.repos' string must be 'all' or 'public'", + }, + { + name: "empty repos array", + toolsMap: map[string]any{ + "github": map[string]any{ + "repos": []any{}, + "min-integrity": "reader", + }, + }, + shouldError: true, + errorMsg: "'github.repos' array cannot be empty", + }, + { + name: "repos array with uppercase pattern", + toolsMap: map[string]any{ + "github": map[string]any{ + "repos": []any{"Owner/repo"}, + "min-integrity": "reader", + }, + }, + shouldError: true, + errorMsg: "must be lowercase", + }, + { + name: "repos array with invalid pattern format", + toolsMap: map[string]any{ + "github": map[string]any{ + "repos": []any{"just-a-name"}, + "min-integrity": "reader", + }, + }, + shouldError: true, + errorMsg: "must be in format", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tools := NewTools(tt.toolsMap) + err := validateGitHubGuardPolicy(tools, "test-workflow") + + if tt.shouldError { + require.Error(t, err, "Expected error for %s", tt.name) + if tt.errorMsg != "" { + assert.Contains(t, err.Error(), tt.errorMsg, "Error message should contain expected text") + } + } else { + assert.NoError(t, err, "Expected no error for %s", tt.name) + } + }) + } +} + +func TestValidateReposScopeWithStringSlice(t *testing.T) { + tests := []struct { + name string + repos any + shouldError bool + errorMsg string + }{ + { + name: "valid []string repos array", + repos: []string{"owner/repo", "owner/*"}, + shouldError: false, + }, + { + name: "valid []any repos array", + repos: []any{"owner/repo", "owner/*"}, + shouldError: false, + }, + { + name: "empty []string repos array", + repos: []string{}, + shouldError: true, + errorMsg: "array cannot be empty", + }, + { + name: "[]string with invalid pattern", + repos: []string{"Owner/Repo"}, + shouldError: true, + errorMsg: "must be lowercase", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateReposScope(tt.repos, "test-workflow") + + if tt.shouldError { + require.Error(t, err, "Expected error for %s", tt.name) + if tt.errorMsg != "" { + assert.Contains(t, err.Error(), tt.errorMsg, "Error message should contain expected text") + } + } else { + assert.NoError(t, err, "Expected no error for %s", tt.name) + } + }) + } +} diff --git a/scratchpad/guard-policies-specification.md b/scratchpad/guard-policies-specification.md new file mode 100644 index 0000000000..f44e18e341 --- /dev/null +++ b/scratchpad/guard-policies-specification.md @@ -0,0 +1,307 @@ +# Guard Policies Integration Proposal + +## Executive Summary + +This document proposes an extensible guard policies framework for the MCP Gateway, starting with GitHub-specific policies. Guard policies enable fine-grained access control at the MCP gateway level, restricting which repositories and operations AI agents can access through MCP servers. + +## Problem Statement + +The user requested support for guard policies in the MCP gateway configuration, with the following requirements: + +1. Support GitHub-specific guard policies with flat frontmatter syntax: + - `repos` (scope): Repository access patterns + - `min-integrity` (minintegrity): Minimum min-integrity level required + +2. Design an extensible system that can support future MCP servers (Jira, WorkIQ) with different policy schemas + +3. Expose these parameters through workflow frontmatter in an intuitive way + +## Proposed Solution + +### 1. Type Hierarchy + +``` +GitHubToolConfig (GitHub-specific) + ├── Repos: GitHubReposScope (string or []any) + └── MinIntegrity: GitHubIntegrityLevel (enum) + +MCPServerConfig (general) + └── GuardPolicies: map[string]any (extensible for all servers) +``` + +### 2. GitHub Guard Policy Schema + +Based on the provided JSON schema, the implementation supports: + +**Repos Scope:** +- `"all"` - All repositories accessible by the token +- `"public"` - Public repositories only +- Array of patterns: + - `"owner/repo"` - Exact repository match + - `"owner/*"` - All repositories under owner + - `"owner/prefix*"` - Repositories with name prefix under owner + +**Integrity Levels:** +- `"none"` - No min-integrity requirements +- `"reader"` - Read-level integrity +- `"writer"` - Write-level integrity +- `"merged"` - Merged-level integrity + +### 3. Frontmatter Syntax + +**Minimal Example:** +```yaml +tools: + github: + mode: remote + toolsets: [default] + repos: "all" + min-integrity: reader +``` + +**With Repository Patterns:** +```yaml +tools: + github: + mode: remote + toolsets: [default] + repos: + - "myorg/*" + - "partner/shared-repo" + - "docs/api-*" + min-integrity: writer +``` + +**Public Repositories Only:** +```yaml +tools: + github: + repos: "public" + min-integrity: none +``` + +### 4. MCP Gateway Configuration Flow + +1. **Frontmatter Parsing** (`tools_parser.go`): + - Extracts `repos` and `min-integrity` directly from GitHub tool config + - Stores them as fields on `GitHubToolConfig` + - Validates structure and types + +2. **Validation** (`tools_validation.go`): + - Validates repos format (all/public or valid patterns) + - Validates min-integrity level (none/reader/writer/merged) + - Validates repository pattern syntax (lowercase, valid characters, wildcard placement) + - Called during workflow compilation + +3. **Compilation**: + - Guard policy fields (repos, min-integrity) included in compiled GitHub tool configuration + - Passed through to MCP Gateway configuration + +4. **Runtime (MCP Gateway)**: + - Gateway receives guard policies in server configuration + - Enforces policies on all tool invocations + - Blocks unauthorized repository access + +### 5. Extensibility for Future Servers + +The design supports future MCP servers (Jira, WorkIQ) through: + +1. **Server-Specific Policy Fields:** + ```go + type JiraToolConfig struct { + // ... other fields ... + // Guard policy fields (flat syntax under jira:) + Projects []string `yaml:"projects,omitempty"` + IssueTypes []string `yaml:"issue-types,omitempty"` + } + ``` + +2. **General MCPServerConfig Field:** + ```go + type MCPServerConfig struct { + // ... + GuardPolicies map[string]any `yaml:"guard-policies,omitempty"` + } + ``` + +3. **Frontmatter Configuration:** + ```yaml + tools: + jira: + mode: remote + projects: ["PROJ-*", "SHARED"] + issue-types: ["Bug", "Story"] + ``` + +## Implementation Details + +### Files Modified + +1. **pkg/workflow/tools_types.go** + - Added `GitHubIntegrityLevel` enum type + - Added `GitHubReposScope` type alias + - Extended `GitHubToolConfig` with flat `Repos` and `MinIntegrity` fields + - Extended `MCPServerConfig` with `GuardPolicies` field + +2. **pkg/workflow/schemas/mcp-gateway-config.schema.json** + - Added `guard-policies` field to `stdioServerConfig` + - Added `guard-policies` field to `httpServerConfig` + - Set `additionalProperties: true` for server-specific schemas + +3. **pkg/workflow/tools_parser.go** + - Extended `parseGitHubTool()` to extract `repos` and `min-integrity` directly + +4. **pkg/workflow/tools_validation.go** + - Updated `validateGitHubGuardPolicy()` function (validates flat fields) + - Added `validateReposScope()` function + - Added `validateRepoPattern()` function + - Added `isValidOwnerOrRepo()` helper function + +5. **pkg/workflow/compiler_orchestrator_workflow.go** + - Added call to `validateGitHubGuardPolicy()` + +6. **pkg/workflow/compiler_string_api.go** + - Added call to `validateGitHubGuardPolicy()` + +### Validation Rules + +**Repository Patterns:** +- Must be lowercase +- Format: `owner/repo`, `owner/*`, or `owner/prefix*` +- Owner and repo parts must contain only: lowercase letters, numbers, hyphens, underscores +- Wildcards only allowed at end of repo name +- Empty arrays not allowed + +**Integrity Levels:** +- Must be one of: `none`, `reader`, `writer`, `merged` +- Case-sensitive + +**Required Fields:** +- Both `repos` and `min-integrity` are required when either is specified under `github:` + +## Error Messages + +The implementation provides clear, actionable error messages: + +``` +invalid guard policy: 'github.repos' is required. +Use 'all', 'public', or an array of repository patterns (e.g., ['owner/repo', 'owner/*']) + +invalid guard policy: repository pattern 'Owner/Repo' must be lowercase + +invalid guard policy: repository pattern 'owner/re*po' has wildcard in the middle. +Wildcards only allowed at the end (e.g., 'prefix*') + +invalid guard policy: 'github.min-integrity' must be one of: 'none', 'reader', 'writer', 'merged'. +Got: 'admin' +``` + +## Usage Examples + +### Example 1: Restrict to Organization + +```yaml +tools: + github: + mode: remote + toolsets: [default] + repos: + - "myorg/*" + min-integrity: reader +``` + +### Example 2: Multiple Organizations + +```yaml +tools: + github: + mode: remote + toolsets: [default] + repos: + - "frontend-org/*" + - "backend-org/*" + - "shared/infrastructure" + min-integrity: writer +``` + +### Example 3: Public Repositories Only + +```yaml +tools: + github: + mode: remote + toolsets: [repos, issues] + repos: "public" + min-integrity: none +``` + +### Example 4: Prefix Matching + +```yaml +tools: + github: + mode: remote + toolsets: [default] + repos: + - "myorg/api-*" # Matches api-gateway, api-service, etc. + - "myorg/web-*" # Matches web-frontend, web-backend, etc. + min-integrity: writer +``` + +## Testing Strategy + +1. **Unit Tests** (Complete): + - `TestValidateGitHubGuardPolicy`: 14 cases covering valid/invalid repos values, invalid min-integrity, missing fields + - `TestValidateReposScopeWithStringSlice`: 4 cases covering `[]string` and `[]any` input types + - Tests live in `pkg/workflow/tools_validation_test.go` + +2. **Integration Tests** (Pending): + - Test end-to-end workflow compilation with guard policies + - Test that guard policies appear in compiled workflow YAML + - Test that guard policies are passed to MCP gateway configuration + +## Next Steps + +1. **Write Comprehensive Tests**: + - Unit tests for parsing functions + - Unit tests for validation functions + - Integration tests for end-to-end workflow compilation + +2. **Update Documentation**: + - Add guard policies section to MCP gateway documentation + - Add examples to GitHub MCP server documentation + - Update frontmatter configuration reference + +3. **Runtime Implementation** (Separate from this PR): + - MCP Gateway enforcement of guard policies + - Repository pattern matching logic + - Integrity level verification + - Access control logging + +## Benefits + +1. **Security**: Restrict AI agent access to specific repositories +2. **Compliance**: Enforce minimum min-integrity requirements +3. **Flexibility**: Support diverse repository patterns and wildcards +4. **Extensibility**: Easy to add policies for Jira, WorkIQ, etc. +5. **Clarity**: Clear error messages and validation +6. **Documentation**: Self-documenting through type system + +## Open Questions + +1. Should we support negative patterns (e.g., exclude certain repos)? +2. Should we support combining multiple policies (AND/OR logic)? +3. How should conflicts between lockdown and guard policies be resolved? +4. Should we add a "dry-run" mode to test policies before enforcement? + +## Conclusion + +This implementation provides a solid foundation for guard policies in the MCP gateway. The design is: + +- **Type-safe**: Strongly-typed structs with validation +- **Extensible**: Easy to add new servers and policy types +- **User-friendly**: Intuitive frontmatter syntax +- **Well-validated**: Comprehensive validation with clear error messages +- **Forward-compatible**: Supports future enhancements + +The implementation follows established patterns in the codebase and integrates seamlessly with the existing compilation and validation infrastructure.