Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds initial “guard policies” support to workflow tooling, starting with a GitHub-specific allowonly policy and a generic guard-policies field for MCP gateway server configs.
Changes:
- Added GitHub
allowonlypolicy types, parsing, and validation (repos scope + integrity level). - Added
guard-policiespassthrough support toMCPServerConfigand updated the MCP gateway JSON schema accordingly. - Wired guard policy validation into both file-based and string-based workflow compilation paths.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/tools_validation.go | Adds validation for GitHub guard policy (allowonly) including repo pattern checks and integrity enum validation. |
| pkg/workflow/tools_types.go | Introduces GitHubAllowOnlyPolicy / integrity types; adds GuardPolicies to MCPServerConfig and emits it in mcpServerConfigToMap. |
| pkg/workflow/tools_parser.go | Parses tools.github.allowonly into structured config. |
| pkg/workflow/schemas/mcp-gateway-config.schema.json | Extends MCP gateway schema to allow guard-policies on stdio/http server configs. |
| pkg/workflow/compiler_string_api.go | Calls new guard policy validation during string-based workflow parsing. |
| pkg/workflow/compiler_orchestrator_workflow.go | Calls new guard policy validation during file-based workflow parsing. |
| GUARD_POLICIES_PROPOSAL.md | Adds a design/proposal document describing the intended framework and examples. |
Comments suppressed due to low confidence (1)
pkg/workflow/tools_parser.go:240
allowonlyis not currently permitted by the frontmatter JSON schema, so workflows that try to use this new config will fail schema validation before this parser ever sees it. Inpkg/parser/schemas/main_workflow_schema.json,tools.githubis an object withadditionalProperties: falseand does not define anallowonlyproperty (see around thetools.githubschema block near line ~2868). Addallowonlyto the schema (and its nested shape) so the feature is usable.
// Parse allowonly guard policy configuration
if allowOnly, ok := configMap["allowonly"].(map[string]any); ok {
config.AllowOnly = parseGitHubAllowOnlyPolicy(allowOnly)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg/workflow/tools_parser.go
Outdated
| // Parse allowonly guard policy configuration | ||
| if allowOnly, ok := configMap["allowonly"].(map[string]any); ok { | ||
| config.AllowOnly = parseGitHubAllowOnlyPolicy(allowOnly) | ||
| } | ||
|
|
There was a problem hiding this comment.
allowonly is parsed and validated, but it is never propagated into the rendered MCP gateway config. A repo-wide search shows no references beyond parsing/types/validation, and mcp_renderer.go's GitHub MCP rendering does not emit any guard-policies / allowonly fields. If the intent is gateway-side enforcement, you likely need to translate tools.github.allowonly into a guard-policies entry in the GitHub MCP server config when rendering the MCP config.
This issue also appears on line 237 of the same file.
| // Parse allowonly guard policy configuration | |
| if allowOnly, ok := configMap["allowonly"].(map[string]any); ok { | |
| config.AllowOnly = parseGitHubAllowOnlyPolicy(allowOnly) | |
| } |
There was a problem hiding this comment.
Fixed in 94511cf. Added getGitHubGuardPolicies(githubTool) helper that extracts allow-only from the raw tool config and returns it as a map[string]any. Added GuardPolicies field to both GitHubMCPDockerOptions and GitHubMCPRemoteOptions, and a renderGuardPoliciesJSON helper that serializes it as a "guard-policies" block after the env section in both Docker and Remote render paths.
pkg/workflow/tools_validation.go
Outdated
| // validateReposScope validates the repos field in allowonly 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: 'allowonly.repos' string must be 'all' or 'public'. Got: '" + reposStr + "'") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Case 2: Array of patterns | ||
| 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: 'allowonly.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: 'allowonly.repos' array must contain only strings") | ||
| } | ||
|
|
||
| if err := validateRepoPattern(pattern, workflowName); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
validateReposScope only accepts []any, but GitHubReposScope is documented as "string or []string" and other code in this repo commonly uses []string as well as []any for parsed arrays (e.g., toolsets parsing explicitly supports both). This will incorrectly reject programmatically-constructed configs (and is inconsistent with the type comment). Consider accepting both []any and []string here.
There was a problem hiding this comment.
Fixed in 94511cf. Added a []string case to validateReposScope alongside the existing []any case, so programmatically-constructed configs using []string are accepted correctly.
pkg/workflow/tools_validation.go
Outdated
| // validateGitHubGuardPolicy validates the GitHub guard policy configuration | ||
| func validateGitHubGuardPolicy(tools *Tools, workflowName string) error { | ||
| if tools == nil || tools.GitHub == nil { | ||
| return nil | ||
| } | ||
|
|
||
| // Validate allowonly policy if present | ||
| if tools.GitHub.AllowOnly != nil { | ||
| if err := validateGitHubAllowOnlyPolicy(tools.GitHub.AllowOnly, workflowName); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // validateGitHubAllowOnlyPolicy validates the allowonly guard policy configuration | ||
| func validateGitHubAllowOnlyPolicy(policy *GitHubAllowOnlyPolicy, workflowName string) error { | ||
| if policy == nil { | ||
| return nil | ||
| } | ||
|
|
||
| // Validate repos field (required) | ||
| if policy.Repos == nil { | ||
| toolsValidationLog.Printf("Missing repos in allowonly policy for workflow: %s", workflowName) | ||
| return errors.New("invalid guard policy: 'allowonly.repos' is required. Use 'all', 'public', or an array of repository patterns (e.g., ['owner/repo', 'owner/*'])") | ||
| } | ||
|
|
||
| // Validate repos format | ||
| if err := validateReposScope(policy.Repos, workflowName); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Validate integrity field (required) | ||
| if policy.Integrity == "" { | ||
| toolsValidationLog.Printf("Missing integrity in allowonly policy for workflow: %s", workflowName) | ||
| return errors.New("invalid guard policy: 'allowonly.integrity' is required. Valid values: 'none', 'reader', 'writer', 'merged'") | ||
| } | ||
|
|
||
| // Validate integrity value | ||
| validIntegrityLevels := map[GitHubIntegrityLevel]bool{ | ||
| GitHubIntegrityNone: true, | ||
| GitHubIntegrityReader: true, | ||
| GitHubIntegrityWriter: true, | ||
| GitHubIntegrityMerged: true, | ||
| } | ||
|
|
||
| if !validIntegrityLevels[policy.Integrity] { | ||
| toolsValidationLog.Printf("Invalid integrity level '%s' in workflow: %s", policy.Integrity, workflowName) | ||
| return errors.New("invalid guard policy: 'allowonly.integrity' must be one of: 'none', 'reader', 'writer', 'merged'. Got: '" + string(policy.Integrity) + "'") | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
New guard policy validation paths were added here, but there are no corresponding unit tests in pkg/workflow/tools_validation_test.go covering the allowonly policy (valid/invalid repos scopes, invalid patterns, invalid integrity, etc.). Since this package already has validation tests, please add coverage for these new branches to prevent regressions and to lock in error message quality.
There was a problem hiding this comment.
Added in 94511cf. Added TestValidateGitHubGuardPolicy (14 cases covering valid/invalid repos values, invalid integrity, missing fields) and TestValidateReposScopeWithStringSlice (4 cases covering []string and []any input types) to tools_validation_test.go.
pkg/workflow/tools_types.go
Outdated
| // 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 []string | ||
|
|
||
| // GitHubAllowOnlyPolicy represents the allowonly guard policy configuration | ||
| type GitHubAllowOnlyPolicy struct { | ||
| // Repos defines the access scope for policy enforcement | ||
| // Supports: | ||
| // - "all": all repositories | ||
| // - "public": public repositories only | ||
| // - array of patterns: ["owner/repo", "owner/*", "owner/re*"] (lowercase) | ||
| Repos GitHubReposScope `json:"repos" yaml:"repos"` | ||
|
|
||
| // Integrity defines the minimum integrity level required | ||
| // Valid values: "none", "reader", "writer", "merged" | ||
| Integrity GitHubIntegrityLevel `json:"integrity" yaml:"integrity"` | ||
| } |
There was a problem hiding this comment.
GitHubReposScope is declared as any and the comment says "string or []string", but parsing stores the raw YAML value (any), which is typically []any for arrays. Either adjust the comment to reflect the actual runtime shapes (string or []any), or introduce a dedicated type + custom unmarshal so this is truly string | []string.
There was a problem hiding this comment.
Fixed in 94511cf — updated the comment to // string or []any (YAML-parsed arrays are []any) to reflect the actual runtime type.
pkg/workflow/tools_types.go
Outdated
|
|
||
| // Guard policies for access control at the MCP gateway level | ||
| // This is a general field that can hold server-specific policy configurations | ||
| // For GitHub: use GitHubGuardPolicy |
There was a problem hiding this comment.
The comment on GuardPolicies says "For GitHub: use GitHubGuardPolicy", but no such type exists in this PR (the GitHub policy type added is GitHubAllowOnlyPolicy, attached to GitHubToolConfig). This is misleading documentation; update the comment to reference the actual intended GitHub policy representation (or add the missing type if that's the plan).
| // For GitHub: use GitHubGuardPolicy | |
| // For GitHub: policies are represented via GitHubAllowOnlyPolicy on GitHubToolConfig |
There was a problem hiding this comment.
Fixed in 94511cf — updated the comment to // For GitHub: policies are represented via GitHubAllowOnlyPolicy on GitHubToolConfig.
…comments, add tests, propagate to renderer Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 94511cf — renamed Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This comment has been minimized.
This comment has been minimized.
|
@copilot flatten the frontmatter fields: |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 436875b — moved
Done in 436875b. Removed the tools:
github:
repos: "all"
integrity: readerRemoved Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot rename integrity to min-integrity |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 1cba6cd — renamed |
This comment has been minimized.
This comment has been minimized.
|
Hey @lpcox 👋 — nice feature addition! The guard policies implementation for the MCP gateway is well-structured: flat frontmatter syntax under
|
Adds guard policies support to workflow tooling, enabling fine-grained access control at the MCP gateway level for GitHub MCP servers.
Changes Made
Flat frontmatter syntax: Guard policy fields (
reposandmin-integrity) are specified directly undergithub:— no wrapper needed:Types: Added
GitHubReposScopeandGitHubIntegrityLeveltoGitHubToolConfigas flat fields (ReposandMinIntegrity)Validation: Validates
repos(string or array of patterns, lowercase, valid format) andmin-integrity(enum) with clear error messages; both fields are required when either is specified; accepts both[]any(YAML-parsed) and[]string(programmatic) arraysMCP gateway rendering: Guard policy fields are propagated into the rendered MCP gateway config as a
"guard-policies": { "allow-only": { "repos": ..., "min-integrity": ... } }block in both Docker and Remote GitHub MCP server render pathsSchema: Updated
mcp-gateway-config.schema.jsonto includeguard-policiesfield for stdio and HTTP server configsSpec document: Added
scratchpad/guard-policies-specification.mdalongside existing MCP gateway and safe outputs specificationsTests: Added
TestValidateGitHubGuardPolicyandTestValidateReposScopeWithStringSlicecovering valid/invalid repos scopes, min-integrity levels, pattern formats, and both[]any/[]stringinput types💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.