diff --git a/.changeset/minor-add-cli-proxy-feature-flag.md b/.changeset/minor-add-cli-proxy-feature-flag.md index f5be3e8d629..3fa1ac518b4 100644 --- a/.changeset/minor-add-cli-proxy-feature-flag.md +++ b/.changeset/minor-add-cli-proxy-feature-flag.md @@ -2,4 +2,4 @@ "gh-aw": minor --- -Add `cli-proxy` and `cli-proxy-writable` feature flags that inject `--enable-cli-proxy`, `--cli-proxy-writable`, and `--cli-proxy-policy` into the AWF command, giving agents secure `gh` CLI access without exposing `GITHUB_TOKEN` (requires firewall v0.25.14+). +Add `cli-proxy` feature flag that injects `--enable-cli-proxy` and `--cli-proxy-policy` into the AWF command, giving agents secure read-only `gh` CLI access without exposing `GITHUB_TOKEN` (requires firewall v0.25.14+). diff --git a/actions/setup/md/cli_proxy_prompt.md b/actions/setup/md/cli_proxy_prompt.md new file mode 100644 index 00000000000..f2182e3f335 --- /dev/null +++ b/actions/setup/md/cli_proxy_prompt.md @@ -0,0 +1,3 @@ + +The `gh` CLI is pre-authenticated via the CLI proxy sidecar. Use `gh` commands for all GitHub reads: listing and searching issues, pull requests, discussions, labels, milestones; reading workflow runs, jobs, and artifacts; accessing repository contents, code, and metadata. No GitHub MCP server is available. + diff --git a/actions/setup/md/cli_proxy_with_safeoutputs_prompt.md b/actions/setup/md/cli_proxy_with_safeoutputs_prompt.md new file mode 100644 index 00000000000..5bcbc0c57e1 --- /dev/null +++ b/actions/setup/md/cli_proxy_with_safeoutputs_prompt.md @@ -0,0 +1,3 @@ + +The `gh` CLI is pre-authenticated via the CLI proxy sidecar. Use `gh` commands for all GitHub reads: listing and searching issues, pull requests, discussions, labels, milestones; reading workflow runs, jobs, and artifacts; accessing repository contents, code, and metadata. Use safeoutputs tools for GitHub writes and completion signaling. No GitHub MCP server is available. + diff --git a/pkg/constants/feature_constants.go b/pkg/constants/feature_constants.go index 67420a6f8fc..5b37fea4a8d 100644 --- a/pkg/constants/feature_constants.go +++ b/pkg/constants/feature_constants.go @@ -38,17 +38,4 @@ const ( // features: // cli-proxy: true CliProxyFeatureFlag FeatureFlag = "cli-proxy" - // CliProxyWritableFeatureFlag enables write operations on the AWF CLI proxy sidecar. - // By default, the CLI proxy sidecar is read-only. When this flag is enabled, - // --cli-proxy-writable is injected into the AWF command, allowing write operations - // such as creating issues or merging PRs via gh CLI. - // - // Requires CliProxyFeatureFlag to also be enabled. - // - // Workflow frontmatter usage: - // - // features: - // cli-proxy: true - // cli-proxy-writable: true - CliProxyWritableFeatureFlag FeatureFlag = "cli-proxy-writable" ) diff --git a/pkg/workflow/awf_helpers.go b/pkg/workflow/awf_helpers.go index 1cc165f8ca3..8f763c7d501 100644 --- a/pkg/workflow/awf_helpers.go +++ b/pkg/workflow/awf_helpers.go @@ -250,12 +250,6 @@ func BuildAWFArgs(config AWFCommandConfig) []string { awfArgs = append(awfArgs, "--enable-cli-proxy") awfHelpersLog.Print("Added --enable-cli-proxy for gh CLI proxy sidecar") - // Allow write operations when cli-proxy-writable feature flag is also set - if isFeatureEnabled(constants.CliProxyWritableFeatureFlag, config.WorkflowData) { - awfArgs = append(awfArgs, "--cli-proxy-writable") - awfHelpersLog.Print("Added --cli-proxy-writable for write access via gh CLI proxy") - } - // Generate and pass the guard policy JSON for the cli-proxy. // Reuses getDIFCProxyPolicyJSON() to build the static policy from tools.github config // (min-integrity and repos fields), matching the DIFC proxy guard policy semantics. diff --git a/pkg/workflow/awf_helpers_test.go b/pkg/workflow/awf_helpers_test.go index 6ed2264fba9..e91d1ca4c80 100644 --- a/pkg/workflow/awf_helpers_test.go +++ b/pkg/workflow/awf_helpers_test.go @@ -730,8 +730,8 @@ func TestAWFSupportsExcludeEnv(t *testing.T) { } } -// TestBuildAWFArgsCliProxy tests that BuildAWFArgs correctly injects --enable-cli-proxy, -// --cli-proxy-writable, and --cli-proxy-policy based on the cli-proxy feature flags. +// TestBuildAWFArgsCliProxy tests that BuildAWFArgs correctly injects --enable-cli-proxy +// and --cli-proxy-policy based on the cli-proxy feature flag. func TestBuildAWFArgsCliProxy(t *testing.T) { baseWorkflow := func(features map[string]any, tools map[string]any) *WorkflowData { return &WorkflowData{ @@ -758,7 +758,6 @@ func TestBuildAWFArgsCliProxy(t *testing.T) { argsStr := strings.Join(args, " ") assert.NotContains(t, argsStr, "--enable-cli-proxy", "Should not include --enable-cli-proxy when feature flag is absent") - assert.NotContains(t, argsStr, "--cli-proxy-writable", "Should not include --cli-proxy-writable when feature flag is absent") assert.NotContains(t, argsStr, "--cli-proxy-policy", "Should not include --cli-proxy-policy when feature flag is absent") }) @@ -776,42 +775,6 @@ func TestBuildAWFArgsCliProxy(t *testing.T) { argsStr := strings.Join(args, " ") assert.Contains(t, argsStr, "--enable-cli-proxy", "Should include --enable-cli-proxy when cli-proxy feature flag is enabled") - assert.NotContains(t, argsStr, "--cli-proxy-writable", "Should not include --cli-proxy-writable when cli-proxy-writable feature flag is absent") - }) - - t.Run("includes --cli-proxy-writable when cli-proxy-writable feature flag is enabled", func(t *testing.T) { - config := AWFCommandConfig{ - EngineName: "copilot", - WorkflowData: baseWorkflow( - map[string]any{"cli-proxy": true, "cli-proxy-writable": true}, - nil, - ), - AllowedDomains: "github.com", - } - - args := BuildAWFArgs(config) - argsStr := strings.Join(args, " ") - - assert.Contains(t, argsStr, "--enable-cli-proxy", "Should include --enable-cli-proxy") - assert.Contains(t, argsStr, "--cli-proxy-writable", "Should include --cli-proxy-writable when feature flag is enabled") - }) - - t.Run("does not include --cli-proxy-writable without --enable-cli-proxy", func(t *testing.T) { - // cli-proxy-writable alone (without cli-proxy) should not inject any cli-proxy flags - config := AWFCommandConfig{ - EngineName: "copilot", - WorkflowData: baseWorkflow( - map[string]any{"cli-proxy-writable": true}, - nil, - ), - AllowedDomains: "github.com", - } - - args := BuildAWFArgs(config) - argsStr := strings.Join(args, " ") - - assert.NotContains(t, argsStr, "--enable-cli-proxy", "Should not include --enable-cli-proxy when cli-proxy flag is absent") - assert.NotContains(t, argsStr, "--cli-proxy-writable", "Should not include --cli-proxy-writable when cli-proxy flag is absent") }) t.Run("includes --cli-proxy-policy with guard policy when tools.github has min-integrity", func(t *testing.T) { @@ -894,8 +857,7 @@ func TestBuildAWFArgsCliProxy(t *testing.T) { }, }, Features: map[string]any{ - "cli-proxy": true, - "cli-proxy-writable": true, + "cli-proxy": true, }, Tools: map[string]any{ "github": map[string]any{ @@ -914,7 +876,6 @@ func TestBuildAWFArgsCliProxy(t *testing.T) { argsStr := strings.Join(args, " ") assert.NotContains(t, argsStr, "--enable-cli-proxy", "Should not include --enable-cli-proxy for AWF < v0.25.14") - assert.NotContains(t, argsStr, "--cli-proxy-writable", "Should not include --cli-proxy-writable for AWF < v0.25.14") assert.NotContains(t, argsStr, "--cli-proxy-policy", "Should not include --cli-proxy-policy for AWF < v0.25.14") }) } diff --git a/pkg/workflow/mcp_setup_generator.go b/pkg/workflow/mcp_setup_generator.go index e63082bdbed..f0d8e34b3a2 100644 --- a/pkg/workflow/mcp_setup_generator.go +++ b/pkg/workflow/mcp_setup_generator.go @@ -93,6 +93,13 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, } // Standard MCP tools if toolName == "github" || toolName == "playwright" || toolName == "cache-memory" || toolName == "agentic-workflows" { + // When cli-proxy is enabled, agents use the pre-authenticated gh CLI for GitHub + // reads instead of the GitHub MCP server. Skip registering the GitHub MCP server + // so it is not configured with the gateway. + if toolName == "github" && isFeatureEnabled(constants.CliProxyFeatureFlag, workflowData) { + mcpSetupGeneratorLog.Print("Skipping GitHub MCP server registration: cli-proxy feature flag is enabled") + continue + } mcpTools = append(mcpTools, toolName) } else if mcpConfig, ok := toolValue.(map[string]any); ok { // Check if it's explicitly marked as MCP type in the new format diff --git a/pkg/workflow/prompt_constants.go b/pkg/workflow/prompt_constants.go index 15879719705..24d4d41692f 100644 --- a/pkg/workflow/prompt_constants.go +++ b/pkg/workflow/prompt_constants.go @@ -26,6 +26,8 @@ const ( agenticWorkflowsGuideFile = "agentic_workflows_guide.md" githubMCPToolsPromptFile = "github_mcp_tools_prompt.md" githubMCPToolsWithSafeOutputsPromptFile = "github_mcp_tools_with_safeoutputs_prompt.md" + cliProxyPromptFile = "cli_proxy_prompt.md" + cliProxyWithSafeOutputsPromptFile = "cli_proxy_with_safeoutputs_prompt.md" ) // GitHub context prompt is kept embedded because it contains GitHub Actions expressions diff --git a/pkg/workflow/unified_prompt_step.go b/pkg/workflow/unified_prompt_step.go index 69ac21926a1..90b398a735e 100644 --- a/pkg/workflow/unified_prompt_step.go +++ b/pkg/workflow/unified_prompt_step.go @@ -222,7 +222,24 @@ func (c *Compiler) collectPromptSections(data *WorkflowData) []PromptSection { EnvVars: envVars, }) } + } + // 9b. GitHub tool-use guidance: directs the model to the correct mechanism for + // GitHub reads (and writes when safe-outputs is also enabled). + // When cli-proxy is enabled, the agent uses the pre-authenticated gh CLI for reads + // instead of a GitHub MCP server (which is not registered). Otherwise, the GitHub + // MCP server is used for reads. + if isFeatureEnabled(constants.CliProxyFeatureFlag, data) { + unifiedPromptLog.Print("Adding cli-proxy tool-use guidance (gh CLI for reads, no GitHub MCP server)") + cliProxyFile := cliProxyPromptFile + if HasSafeOutputsEnabled(data.SafeOutputs) { + cliProxyFile = cliProxyWithSafeOutputsPromptFile + } + sections = append(sections, PromptSection{ + Content: cliProxyFile, + IsFile: true, + }) + } else if hasGitHubTool(data.ParsedTools) { // GitHub MCP tool-use guidance: clarifies that the MCP server is read-only and // directs the model to use it for GitHub reads. When safe-outputs is also enabled, // the guidance explicitly separates reads (GitHub MCP) from writes (safeoutputs) so diff --git a/pkg/workflow/unified_prompt_step_test.go b/pkg/workflow/unified_prompt_step_test.go index 7f86c5ad71c..45a584b32ac 100644 --- a/pkg/workflow/unified_prompt_step_test.go +++ b/pkg/workflow/unified_prompt_step_test.go @@ -374,3 +374,127 @@ func TestCollectPromptSections_GitHubMCPAndSafeOutputsConsistency(t *testing.T) } }) } + +// TestCollectPromptSections_CliProxy tests that when the cli-proxy feature flag is +// enabled, the cli-proxy prompt is used instead of the GitHub MCP tools prompt, +// and that the GitHub MCP server guidance is never injected. +func TestCollectPromptSections_CliProxy(t *testing.T) { + t.Run("cli-proxy enabled without safe-outputs uses cli_proxy_prompt", func(t *testing.T) { + compiler := &Compiler{} + + data := &WorkflowData{ + ParsedTools: NewTools(map[string]any{"github": true}), + Features: map[string]any{"cli-proxy": true}, + SafeOutputs: nil, + } + + sections := compiler.collectPromptSections(data) + require.NotEmpty(t, sections, "Should collect sections") + + // Should include cli-proxy prompt + var cliProxySection *PromptSection + for i := range sections { + if sections[i].IsFile && sections[i].Content == cliProxyPromptFile { + cliProxySection = §ions[i] + break + } + } + require.NotNil(t, cliProxySection, "Should include cli_proxy_prompt.md when cli-proxy is enabled") + + // Should NOT include GitHub MCP tools prompt + for _, section := range sections { + assert.NotEqual(t, githubMCPToolsPromptFile, section.Content, + "Should not include github_mcp_tools_prompt.md when cli-proxy is enabled") + assert.NotEqual(t, githubMCPToolsWithSafeOutputsPromptFile, section.Content, + "Should not include github_mcp_tools_with_safeoutputs_prompt.md when cli-proxy is enabled") + } + }) + + t.Run("cli-proxy enabled with safe-outputs uses cli_proxy_with_safeoutputs_prompt", func(t *testing.T) { + compiler := &Compiler{} + + data := &WorkflowData{ + ParsedTools: NewTools(map[string]any{"github": true}), + Features: map[string]any{"cli-proxy": true}, + SafeOutputs: &SafeOutputsConfig{ + MissingData: &MissingDataConfig{}, + NoOp: &NoOpConfig{}, + }, + } + + sections := compiler.collectPromptSections(data) + require.NotEmpty(t, sections, "Should collect sections") + + // Should include the with-safeoutputs cli-proxy prompt + var cliProxySection *PromptSection + for i := range sections { + if sections[i].IsFile && sections[i].Content == cliProxyWithSafeOutputsPromptFile { + cliProxySection = §ions[i] + break + } + } + require.NotNil(t, cliProxySection, "Should include cli_proxy_with_safeoutputs_prompt.md when cli-proxy and safe-outputs are both enabled") + + // Should NOT include GitHub MCP tools prompt + for _, section := range sections { + assert.NotEqual(t, githubMCPToolsPromptFile, section.Content, + "Should not include github_mcp_tools_prompt.md when cli-proxy is enabled") + assert.NotEqual(t, githubMCPToolsWithSafeOutputsPromptFile, section.Content, + "Should not include github_mcp_tools_with_safeoutputs_prompt.md when cli-proxy is enabled") + } + }) + + t.Run("cli-proxy enabled without github tool still adds cli-proxy prompt", func(t *testing.T) { + compiler := &Compiler{} + + data := &WorkflowData{ + ParsedTools: NewTools(map[string]any{}), + Features: map[string]any{"cli-proxy": true}, + SafeOutputs: nil, + } + + sections := compiler.collectPromptSections(data) + + // Should include cli-proxy prompt even without github tool configured + var cliProxySection *PromptSection + for i := range sections { + if sections[i].IsFile && sections[i].Content == cliProxyPromptFile { + cliProxySection = §ions[i] + break + } + } + require.NotNil(t, cliProxySection, "Should include cli_proxy_prompt.md when cli-proxy is enabled regardless of tools.github") + }) + + t.Run("cli-proxy disabled uses GitHub MCP tools prompt when github tool is enabled", func(t *testing.T) { + compiler := &Compiler{} + + data := &WorkflowData{ + ParsedTools: NewTools(map[string]any{"github": true}), + Features: nil, + SafeOutputs: nil, + } + + sections := compiler.collectPromptSections(data) + + // Should include the standard GitHub MCP tools prompt + var githubMCPSection *PromptSection + for i := range sections { + if sections[i].IsFile && strings.Contains(sections[i].Content, "github_mcp_tools") { + githubMCPSection = §ions[i] + break + } + } + require.NotNil(t, githubMCPSection, "Should include github_mcp_tools file when cli-proxy is not enabled") + assert.Equal(t, githubMCPToolsPromptFile, githubMCPSection.Content, + "Should use the standard github_mcp_tools_prompt when cli-proxy is not enabled") + + // Should NOT include cli-proxy prompt + for _, section := range sections { + assert.NotEqual(t, cliProxyPromptFile, section.Content, + "Should not include cli_proxy_prompt.md when cli-proxy is not enabled") + assert.NotEqual(t, cliProxyWithSafeOutputsPromptFile, section.Content, + "Should not include cli_proxy_with_safeoutputs_prompt.md when cli-proxy is not enabled") + } + }) +}