diff --git a/pkg/workflow/mcp_github_config.go b/pkg/workflow/mcp_github_config.go index 06d05b996c..bb9d783179 100644 --- a/pkg/workflow/mcp_github_config.go +++ b/pkg/workflow/mcp_github_config.go @@ -258,11 +258,16 @@ func getGitHubGuardPolicies(githubTool any) map[string]any { // deriveSafeOutputsGuardPolicyFromGitHub generates a safeoutputs guard-policy from GitHub guard-policy. // When the GitHub MCP server has a guard-policy with repos, the safeoutputs MCP must also have -// a linked guard-policy. Each entry in the GitHub MCP server's "repos" must have a corresponding -// entry in safeoutputs "accept" with the prefix "private:". +// a linked guard-policy with accept field derived from repos according to these rules: +// +// Rules by repos value: +// - repos="all" or repos="public": returns nil (write-sink not required, agent secrecy is empty) +// - repos=["O/*"]: accept=["private:O"] (owner wildcard → strip wildcard) +// - repos=["O/P*"]: accept=["private:O/P*"] (prefix wildcard → keep as-is) +// - repos=["O/R"]: accept=["private:O/R"] (specific repo → keep as-is) // // This allows the gateway to read private data from the GitHub MCP server and still write to safeoutputs. -// Returns nil if no GitHub guard policies are configured. +// Returns nil if no GitHub guard policies are configured or if repos="all" or repos="public". func deriveSafeOutputsGuardPolicyFromGitHub(githubTool any) map[string]any { githubPolicies := getGitHubGuardPolicies(githubTool) if githubPolicies == nil { @@ -281,32 +286,33 @@ func deriveSafeOutputsGuardPolicyFromGitHub(githubTool any) map[string]any { return nil } - // Convert repos to accept list with "private:" prefix + // Convert repos to accept list according to the specification var acceptList []string switch r := repos.(type) { case string: // Single string value (e.g., "all", "public", or a pattern) - if r == "all" || r == "public" { - // For "all" or "public", add "private:*" to accept all private repos - acceptList = []string{"private:*"} - } else { - // Single pattern - add with private: prefix - acceptList = []string{"private:" + r} + switch r { + case "all", "public": + // For "all" or "public", agent secrecy is empty, so write-sink not required + return nil + default: + // Single pattern - transform according to rules + acceptList = []string{transformRepoPattern(r)} } case []any: // Array of patterns acceptList = make([]string, 0, len(r)) for _, item := range r { if pattern, ok := item.(string); ok { - acceptList = append(acceptList, "private:"+pattern) + acceptList = append(acceptList, transformRepoPattern(pattern)) } } case []string: // Array of patterns (already strings) acceptList = make([]string, 0, len(r)) for _, pattern := range r { - acceptList = append(acceptList, "private:"+pattern) + acceptList = append(acceptList, transformRepoPattern(pattern)) } default: // Unknown type, return nil @@ -322,6 +328,21 @@ func deriveSafeOutputsGuardPolicyFromGitHub(githubTool any) map[string]any { } } +// transformRepoPattern transforms a repos pattern to the corresponding accept pattern. +// Rules: +// - "O/*" → "private:O" (owner wildcard → strip wildcard) +// - "O/P*" → "private:O/P*" (prefix wildcard → keep as-is) +// - "O/R" → "private:O/R" (specific repo → keep as-is) +func transformRepoPattern(pattern string) string { + // Check if pattern ends with "/*" (owner wildcard) + if owner, found := strings.CutSuffix(pattern, "/*"); found { + // Strip the wildcard: "owner/*" → "private:owner" + return "private:" + owner + } + // All other patterns (including "O/P*" prefix wildcards): add "private:" prefix + return "private:" + pattern +} + func getGitHubDockerImageVersion(githubTool any) string { githubDockerImageVersion := string(constants.DefaultGitHubMCPServerVersion) // Default Docker image version // Extract version setting from tool properties diff --git a/pkg/workflow/safeoutputs_guard_policy_test.go b/pkg/workflow/safeoutputs_guard_policy_test.go index 8ccf781264..fbd3d1c433 100644 --- a/pkg/workflow/safeoutputs_guard_policy_test.go +++ b/pkg/workflow/safeoutputs_guard_policy_test.go @@ -33,32 +33,41 @@ func TestDeriveSafeOutputsGuardPolicyFromGitHub(t *testing.T) { description: "Single repo pattern should get private: prefix", }, { - name: "wildcard repo pattern", + name: "owner wildcard pattern", githubTool: map[string]any{ "repos": "github/*", "min-integrity": "approved", }, expectedPolicies: map[string]any{ "write-sink": map[string]any{ - "accept": []string{"private:github/*"}, + "accept": []string{"private:github"}, }, }, expectNil: false, - description: "Wildcard pattern should get private: prefix", + description: "Owner wildcard (github/*) should strip wildcard → private:github", }, { - name: "repos set to all", + name: "repo prefix wildcard pattern", githubTool: map[string]any{ - "repos": "all", + "repos": "github/gh-aw*", "min-integrity": "approved", }, expectedPolicies: map[string]any{ "write-sink": map[string]any{ - "accept": []string{"private:*"}, + "accept": []string{"private:github/gh-aw*"}, }, }, expectNil: false, - description: "repos='all' should map to private:*", + description: "Repo prefix wildcard should keep as-is with private: prefix", + }, + { + name: "repos set to all", + githubTool: map[string]any{ + "repos": "all", + "min-integrity": "approved", + }, + expectNil: true, + description: "repos='all' should return nil (write-sink not required, agent secrecy is empty)", }, { name: "repos set to public", @@ -66,13 +75,8 @@ func TestDeriveSafeOutputsGuardPolicyFromGitHub(t *testing.T) { "repos": "public", "min-integrity": "none", }, - expectedPolicies: map[string]any{ - "write-sink": map[string]any{ - "accept": []string{"private:*"}, - }, - }, - expectNil: false, - description: "repos='public' should map to private:*", + expectNil: true, + description: "repos='public' should return nil (write-sink not required, agent secrecy is empty)", }, { name: "multiple repo patterns as []any", @@ -92,7 +96,7 @@ func TestDeriveSafeOutputsGuardPolicyFromGitHub(t *testing.T) { }, }, expectNil: false, - description: "Array of patterns should all get private: prefix", + description: "Array of prefix patterns should all get private: prefix", }, { name: "multiple repo patterns as []string", @@ -114,6 +118,70 @@ func TestDeriveSafeOutputsGuardPolicyFromGitHub(t *testing.T) { expectNil: false, description: "[]string array should all get private: prefix", }, + { + name: "mixed patterns with owner wildcard", + githubTool: map[string]any{ + "repos": []string{ + "github/*", + "microsoft/copilot", + }, + "min-integrity": "approved", + }, + expectedPolicies: map[string]any{ + "write-sink": map[string]any{ + "accept": []string{ + "private:github", + "private:microsoft/copilot", + }, + }, + }, + expectNil: false, + description: "Owner wildcard (github/*) should transform to private:github, specific repo should keep pattern", + }, + { + name: "array with all three pattern types", + githubTool: map[string]any{ + "repos": []string{ + "github/*", // owner wildcard + "microsoft/copilot*", // prefix wildcard + "google/gemini", // specific repo + }, + "min-integrity": "approved", + }, + expectedPolicies: map[string]any{ + "write-sink": map[string]any{ + "accept": []string{ + "private:github", + "private:microsoft/copilot*", + "private:google/gemini", + }, + }, + }, + expectNil: false, + description: "Array with owner wildcard, prefix wildcard, and specific repo should all transform correctly", + }, + { + name: "array with multiple owner wildcards", + githubTool: map[string]any{ + "repos": []any{ + "github/*", + "microsoft/*", + "google/*", + }, + "min-integrity": "approved", + }, + expectedPolicies: map[string]any{ + "write-sink": map[string]any{ + "accept": []string{ + "private:github", + "private:microsoft", + "private:google", + }, + }, + }, + expectNil: false, + description: "Multiple owner wildcards should all strip the wildcard suffix", + }, { name: "no repos configured", githubTool: map[string]any{