diff --git a/docs/src/content/docs/agent-factory-status.mdx b/docs/src/content/docs/agent-factory-status.mdx index 950a8d5f72..946cd1956e 100644 --- a/docs/src/content/docs/agent-factory-status.mdx +++ b/docs/src/content/docs/agent-factory-status.mdx @@ -132,7 +132,7 @@ These are experimental agentic workflows used by the GitHub Next team to learn, | [Smoke Codex](https://github.com/github/gh-aw/blob/main/.github/workflows/smoke-codex.md) | codex | [![Smoke Codex](https://github.com/github/gh-aw/actions/workflows/smoke-codex.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/smoke-codex.lock.yml) | - | - | | [Smoke Copilot](https://github.com/github/gh-aw/blob/main/.github/workflows/smoke-copilot.md) | copilot | [![Smoke Copilot](https://github.com/github/gh-aw/actions/workflows/smoke-copilot.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/smoke-copilot.lock.yml) | - | - | | [Smoke OpenCode](https://github.com/github/gh-aw/blob/main/.github/workflows/smoke-opencode.md) | copilot | [![Smoke OpenCode](https://github.com/github/gh-aw/actions/workflows/smoke-opencode.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/smoke-opencode.lock.yml) | - | - | -| [Smoke Project](https://github.com/github/gh-aw/blob/main/.github/workflows/smoke-project.md) | codex | [![Smoke Project](https://github.com/github/gh-aw/actions/workflows/smoke-project.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/smoke-project.lock.yml) | - | - | +| [Smoke Project](https://github.com/github/gh-aw/blob/main/.github/workflows/smoke-project.md) | copilot | [![Smoke Project](https://github.com/github/gh-aw/actions/workflows/smoke-project.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/smoke-project.lock.yml) | - | - | | [Stale Repository Identifier](https://github.com/github/gh-aw/blob/main/.github/workflows/stale-repo-identifier.md) | copilot | [![Stale Repository Identifier](https://github.com/github/gh-aw/actions/workflows/stale-repo-identifier.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/stale-repo-identifier.lock.yml) | - | - | | [Static Analysis Report](https://github.com/github/gh-aw/blob/main/.github/workflows/static-analysis-report.md) | claude | [![Static Analysis Report](https://github.com/github/gh-aw/actions/workflows/static-analysis-report.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/static-analysis-report.lock.yml) | - | - | | [Step Name Alignment](https://github.com/github/gh-aw/blob/main/.github/workflows/step-name-alignment.md) | claude | [![Step Name Alignment](https://github.com/github/gh-aw/actions/workflows/step-name-alignment.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/step-name-alignment.lock.yml) | `daily` | - | diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 1e8bb9aef6..ffb9374183 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -261,6 +261,9 @@ const ( // DocsPermissionsURL is the documentation URL for GitHub permissions configuration DocsPermissionsURL DocURL = "https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/permissions.md" + // DocsNetworkURL is the documentation URL for network configuration + DocsNetworkURL DocURL = "https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/network.md" + // DocsSandboxURL is the documentation URL for sandbox configuration DocsSandboxURL DocURL = "https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/sandbox.md" ) diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 39a3828957..330e57c095 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -174,6 +174,12 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath return formatCompilerError(markdownPath, "error", err.Error(), err) } + // Validate network firewall configuration + log.Printf("Validating network firewall configuration") + if err := validateNetworkFirewallConfig(workflowData.NetworkPermissions); err != nil { + return formatCompilerError(markdownPath, "error", err.Error(), err) + } + // Emit experimental warning for sandbox-runtime feature if isSRTEnabled(workflowData) { fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Using experimental feature: sandbox-runtime firewall")) diff --git a/pkg/workflow/network_firewall_validation.go b/pkg/workflow/network_firewall_validation.go new file mode 100644 index 0000000000..0ba6453fb9 --- /dev/null +++ b/pkg/workflow/network_firewall_validation.go @@ -0,0 +1,49 @@ +// This file provides network firewall validation functions for agentic workflow compilation. +// +// This file contains domain-specific validation functions for network firewall configuration: +// - validateNetworkFirewallConfig() - Validates firewall configuration dependencies +// +// These validation functions are organized in a dedicated file following the validation +// architecture pattern where domain-specific validation belongs in domain validation files. +// See validation.go for the complete validation architecture documentation. + +package workflow + +import ( + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/logger" +) + +var networkFirewallValidationLog = logger.New("workflow:network_firewall_validation") + +// validateNetworkFirewallConfig validates network firewall configuration dependencies +// Returns an error if the configuration is invalid +func validateNetworkFirewallConfig(networkPermissions *NetworkPermissions) error { + if networkPermissions == nil { + return nil + } + + firewallConfig := networkPermissions.Firewall + if firewallConfig == nil { + return nil + } + + networkFirewallValidationLog.Print("Validating network firewall configuration") + + // Validate allow-urls requires ssl-bump + if len(firewallConfig.AllowURLs) > 0 && !firewallConfig.SSLBump { + networkFirewallValidationLog.Printf("Validation error: allow-urls specified without ssl-bump: %d URLs", len(firewallConfig.AllowURLs)) + return NewValidationError( + "network.firewall.allow-urls", + "requires ssl-bump: true", + "allow-urls requires ssl-bump: true to function. SSL Bump enables HTTPS content inspection, which is necessary for URL path filtering", + "Enable SSL Bump in your firewall configuration:\n\nnetwork:\n firewall:\n ssl-bump: true\n allow-urls:\n - \"https://github.com/githubnext/*\"\n\nSee: "+string(constants.DocsNetworkURL), + ) + } + + if len(firewallConfig.AllowURLs) > 0 { + networkFirewallValidationLog.Printf("Validated allow-urls: %d URLs with ssl-bump enabled", len(firewallConfig.AllowURLs)) + } + + return nil +} diff --git a/pkg/workflow/network_firewall_validation_test.go b/pkg/workflow/network_firewall_validation_test.go new file mode 100644 index 0000000000..870dee7ec6 --- /dev/null +++ b/pkg/workflow/network_firewall_validation_test.go @@ -0,0 +1,203 @@ +//go:build !integration + +package workflow + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateNetworkFirewallConfig_AllowURLsRequiresSSLBump(t *testing.T) { + t.Run("allow-urls without ssl-bump fails validation", func(t *testing.T) { + networkPermissions := &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + SSLBump: false, + AllowURLs: []string{"https://github.com/githubnext/*"}, + }, + } + + err := validateNetworkFirewallConfig(networkPermissions) + + require.Error(t, err, "Expected validation error when allow-urls is specified without ssl-bump") + assert.Contains(t, err.Error(), "allow-urls requires ssl-bump: true", "Error should mention the ssl-bump requirement") + assert.Contains(t, err.Error(), "network.firewall.allow-urls", "Error should identify the field") + }) + + t.Run("allow-urls with ssl-bump passes validation", func(t *testing.T) { + networkPermissions := &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + SSLBump: true, + AllowURLs: []string{"https://github.com/githubnext/*"}, + }, + } + + err := validateNetworkFirewallConfig(networkPermissions) + + assert.NoError(t, err, "Should not return error when ssl-bump is enabled with allow-urls") + }) + + t.Run("multiple allow-urls without ssl-bump fails validation", func(t *testing.T) { + networkPermissions := &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + SSLBump: false, + AllowURLs: []string{ + "https://github.com/githubnext/*", + "https://api.github.com/repos/*", + "https://example.com/api/*", + }, + }, + } + + err := validateNetworkFirewallConfig(networkPermissions) + + require.Error(t, err, "Expected validation error when multiple allow-urls are specified without ssl-bump") + assert.Contains(t, err.Error(), "allow-urls requires ssl-bump: true", "Error should mention the ssl-bump requirement") + }) + + t.Run("multiple allow-urls with ssl-bump passes validation", func(t *testing.T) { + networkPermissions := &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + SSLBump: true, + AllowURLs: []string{ + "https://github.com/githubnext/*", + "https://api.github.com/repos/*", + "https://example.com/api/*", + }, + }, + } + + err := validateNetworkFirewallConfig(networkPermissions) + + assert.NoError(t, err, "Should not return error when ssl-bump is enabled with multiple allow-urls") + }) + + t.Run("ssl-bump without allow-urls passes validation", func(t *testing.T) { + networkPermissions := &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + SSLBump: true, + AllowURLs: nil, + }, + } + + err := validateNetworkFirewallConfig(networkPermissions) + + assert.NoError(t, err, "Should not return error when ssl-bump is enabled without allow-urls") + }) + + t.Run("empty allow-urls with ssl-bump passes validation", func(t *testing.T) { + networkPermissions := &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + SSLBump: true, + AllowURLs: []string{}, + }, + } + + err := validateNetworkFirewallConfig(networkPermissions) + + assert.NoError(t, err, "Should not return error when allow-urls is empty") + }) + + t.Run("empty allow-urls without ssl-bump passes validation", func(t *testing.T) { + networkPermissions := &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + SSLBump: false, + AllowURLs: []string{}, + }, + } + + err := validateNetworkFirewallConfig(networkPermissions) + + assert.NoError(t, err, "Should not return error when allow-urls is empty") + }) + + t.Run("no firewall config passes validation", func(t *testing.T) { + networkPermissions := &NetworkPermissions{ + Firewall: nil, + } + + err := validateNetworkFirewallConfig(networkPermissions) + + assert.NoError(t, err, "Should not return error when firewall is not configured") + }) + + t.Run("nil network permissions passes validation", func(t *testing.T) { + err := validateNetworkFirewallConfig(nil) + + assert.NoError(t, err, "Should not return error when network permissions is nil") + }) + + t.Run("firewall enabled false with allow-urls and no ssl-bump fails validation", func(t *testing.T) { + networkPermissions := &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: false, + SSLBump: false, + AllowURLs: []string{"https://github.com/*"}, + }, + } + + err := validateNetworkFirewallConfig(networkPermissions) + + require.Error(t, err, "Expected validation error even when firewall is disabled") + assert.Contains(t, err.Error(), "allow-urls requires ssl-bump: true", "Error should mention the ssl-bump requirement") + }) +} + +func TestValidateNetworkFirewallConfig_Integration(t *testing.T) { + t.Run("compiler rejects workflow with allow-urls but no ssl-bump", func(t *testing.T) { + compiler := NewCompiler() + compiler.SetStrictMode(false) // Test in non-strict mode + + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + SSLBump: false, + AllowURLs: []string{"https://github.com/githubnext/*"}, + }, + }, + } + + // Manually call validation (simulating what happens in CompileWorkflowData) + err := validateNetworkFirewallConfig(workflowData.NetworkPermissions) + + require.Error(t, err, "Compiler should reject workflow with allow-urls but no ssl-bump") + assert.Contains(t, err.Error(), "allow-urls requires ssl-bump: true", "Error should explain the requirement") + }) + + t.Run("compiler accepts workflow with allow-urls and ssl-bump", func(t *testing.T) { + compiler := NewCompiler() + compiler.SetStrictMode(false) + + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + SSLBump: true, + AllowURLs: []string{"https://github.com/githubnext/*"}, + }, + }, + } + + // Manually call validation + err := validateNetworkFirewallConfig(workflowData.NetworkPermissions) + + assert.NoError(t, err, "Compiler should accept workflow with allow-urls and ssl-bump enabled") + }) +}