From 5d937e0a57fb32f135a1a3b3597489680c58cda9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 16:37:26 +0000 Subject: [PATCH 1/8] feat(workflow): support sandbox.agent.version for awf resolution Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d204dbf8-0c30-4183-91ab-662ce23a4ebc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../docs/reference/frontmatter-full.md | 4 ++ pkg/parser/schemas/main_workflow_schema.json | 4 ++ pkg/workflow/aw_info_versions_test.go | 18 ++++++++ pkg/workflow/compiler_yaml.go | 12 +++--- pkg/workflow/firewall.go | 20 +++++++++ pkg/workflow/firewall_version_pinning_test.go | 42 +++++++++++++++++++ .../frontmatter_extraction_security.go | 7 ++++ .../frontmatter_extraction_security_test.go | 15 +++++++ pkg/workflow/sandbox.go | 1 + 9 files changed, 117 insertions(+), 6 deletions(-) diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index a0032ecaf02..f700d66b2b6 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -1309,6 +1309,10 @@ sandbox: # (optional) type: "awf" + # AWF version override used to install and run the matching firewall version. + # (optional) + version: "example-value" + # Container mounts to add when using AWF. Each mount is specified using Docker # mount syntax: 'source:destination:mode' where mode can be 'ro' (read-only) or # 'rw' (read-write). Example: '/host/path:/container/path:ro' diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 2d483965db7..edc502944de 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -3023,6 +3023,10 @@ "enum": ["awf"], "description": "Legacy: Sandbox type to use (use 'id' instead)" }, + "version": { + "type": "string", + "description": "AWF version override used to install and run the matching firewall version." + }, "command": { "type": "string", "x-internal": true, diff --git a/pkg/workflow/aw_info_versions_test.go b/pkg/workflow/aw_info_versions_test.go index 9c85cb105dc..23360cd212a 100644 --- a/pkg/workflow/aw_info_versions_test.go +++ b/pkg/workflow/aw_info_versions_test.go @@ -164,6 +164,7 @@ func TestAwfVersionInAwInfo(t *testing.T) { name string firewallEnabled bool firewallVersion string + agentVersion string expectedAwfVersion string description string }{ @@ -185,9 +186,18 @@ func TestAwfVersionInAwInfo(t *testing.T) { name: "Firewall disabled", firewallEnabled: false, firewallVersion: "", + agentVersion: "", expectedAwfVersion: "", description: "Should have empty awf_version when firewall is disabled", }, + { + name: "sandbox.agent.version overrides firewall version", + firewallEnabled: true, + firewallVersion: "", + agentVersion: "v0.30.1", + expectedAwfVersion: "v0.30.1", + description: "Should prefer sandbox.agent.version override", + }, } for _, tt := range tests { @@ -211,6 +221,14 @@ func TestAwfVersionInAwInfo(t *testing.T) { }, } } + if tt.agentVersion != "" { + workflowData.SandboxConfig = &SandboxConfig{ + Agent: &AgentSandboxConfig{ + Type: SandboxTypeAWF, + Version: tt.agentVersion, + }, + } + } var yaml strings.Builder compiler.generateCreateAwInfo(&yaml, workflowData, engine) diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index f3777d7c793..3e5b5bb8d31 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -722,12 +722,12 @@ func (c *Compiler) generateCreateAwInfo(yaml *strings.Builder, data *WorkflowDat firewallVersion := "" if data.NetworkPermissions != nil { allowedDomains = data.NetworkPermissions.Allowed - if data.NetworkPermissions.Firewall != nil { - firewallEnabled = data.NetworkPermissions.Firewall.Enabled - firewallVersion = data.NetworkPermissions.Firewall.Version - if firewallEnabled && firewallVersion == "" { - firewallVersion = string(constants.DefaultFirewallVersion) - } + } + if firewallConfig := getFirewallConfig(data); firewallConfig != nil { + firewallEnabled = firewallConfig.Enabled + firewallVersion = firewallConfig.Version + if firewallEnabled && firewallVersion == "" { + firewallVersion = string(constants.DefaultFirewallVersion) } } diff --git a/pkg/workflow/firewall.go b/pkg/workflow/firewall.go index a04060a36d5..6aa6c2c2bc4 100644 --- a/pkg/workflow/firewall.go +++ b/pkg/workflow/firewall.go @@ -65,9 +65,21 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { return nil } + agentConfig := getAgentConfig(workflowData) + agentVersion := "" + if agentConfig != nil { + agentVersion = agentConfig.Version + } + // Check network.firewall configuration if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil { config := workflowData.NetworkPermissions.Firewall + if agentVersion != "" && config.Version != agentVersion { + overrideConfig := *config + overrideConfig.Version = agentVersion + firewallLog.Printf("Overriding firewall version %s with sandbox.agent.version %s", config.Version, agentVersion) + return &overrideConfig + } if firewallLog.Enabled() { firewallLog.Printf("Retrieved firewall config: enabled=%v, version=%s, logLevel=%s", config.Enabled, config.Version, config.LogLevel) @@ -75,6 +87,14 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { return config } + if agentVersion != "" { + firewallLog.Printf("Using sandbox.agent.version override: %s", agentVersion) + return &FirewallConfig{ + Enabled: true, + Version: agentVersion, + } + } + return nil } diff --git a/pkg/workflow/firewall_version_pinning_test.go b/pkg/workflow/firewall_version_pinning_test.go index dacb807be51..d3e424d08a1 100644 --- a/pkg/workflow/firewall_version_pinning_test.go +++ b/pkg/workflow/firewall_version_pinning_test.go @@ -158,6 +158,48 @@ func TestCopilotEngineFirewallInstallation(t *testing.T) { } }) + t.Run("uses sandbox.agent.version when firewall version is not specified", func(t *testing.T) { + engine := NewCopilotEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + Type: SandboxTypeAWF, + Version: "v0.30.2", + }, + }, + } + + steps := engine.GetInstallationSteps(workflowData) + + var foundAWFStep bool + var awfStepStr string + for _, step := range steps { + stepStr := strings.Join(step, "\n") + if strings.Contains(stepStr, "Install AWF binary") { + foundAWFStep = true + awfStepStr = stepStr + break + } + } + + if !foundAWFStep { + t.Fatal("Expected to find AWF installation step when firewall is enabled") + } + + if !strings.Contains(awfStepStr, "v0.30.2") { + t.Errorf("AWF installation step should pass sandbox.agent.version %s to script", "v0.30.2") + } + }) + t.Run("does not include AWF installation when firewall disabled", func(t *testing.T) { engine := NewCopilotEngine() workflowData := &WorkflowData{ diff --git a/pkg/workflow/frontmatter_extraction_security.go b/pkg/workflow/frontmatter_extraction_security.go index 6ec9a1015be..8b716a6d484 100644 --- a/pkg/workflow/frontmatter_extraction_security.go +++ b/pkg/workflow/frontmatter_extraction_security.go @@ -272,6 +272,13 @@ func (c *Compiler) extractAgentSandboxConfig(agentVal any) *AgentSandboxConfig { } } + // Extract version (AWF version override) + if versionVal, hasVersion := agentObj["version"]; hasVersion { + if versionStr, ok := versionVal.(string); ok { + agentConfig.Version = versionStr + } + } + // Extract config for SRT if configVal, hasConfig := agentObj["config"]; hasConfig { agentConfig.Config = c.extractSRTConfig(configVal) diff --git a/pkg/workflow/frontmatter_extraction_security_test.go b/pkg/workflow/frontmatter_extraction_security_test.go index 4bc37cbdaf9..68aec9991fc 100644 --- a/pkg/workflow/frontmatter_extraction_security_test.go +++ b/pkg/workflow/frontmatter_extraction_security_test.go @@ -118,6 +118,21 @@ func TestExtractFirewallConfig(t *testing.T) { }) } +func TestExtractAgentSandboxConfigVersion(t *testing.T) { + compiler := &Compiler{} + + t.Run("extracts sandbox.agent.version from object format", func(t *testing.T) { + agentObj := map[string]any{ + "id": "awf", + "version": "v0.30.1", + } + + config := compiler.extractAgentSandboxConfig(agentObj) + require.NotNil(t, config, "Should extract agent sandbox config") + assert.Equal(t, "v0.30.1", config.Version, "Should extract sandbox.agent.version") + }) +} + // TestExtractMCPGatewayConfigPayloadFields tests extraction of payload-related fields // from MCP gateway frontmatter configuration func TestExtractMCPGatewayConfigPayloadFields(t *testing.T) { diff --git a/pkg/workflow/sandbox.go b/pkg/workflow/sandbox.go index 69b023b1e41..b4fc2cf90e0 100644 --- a/pkg/workflow/sandbox.go +++ b/pkg/workflow/sandbox.go @@ -44,6 +44,7 @@ type SandboxConfig struct { type AgentSandboxConfig struct { ID string `yaml:"id,omitempty"` // Agent ID: "awf" or "srt" (replaces Type in new object format) Type SandboxType `yaml:"type,omitempty"` // Sandbox type: "awf" or "srt" (legacy, use ID instead) + Version string `yaml:"version,omitempty"` // AWF version override used to install and run the matching firewall version Disabled bool `yaml:"-"` // True when agent is explicitly set to false (disables firewall). This is a runtime flag, not serialized to YAML. Config *SandboxRuntimeConfig `yaml:"config,omitempty"` // Custom SRT config (optional) Command string `yaml:"command,omitempty"` // Custom command to replace AWF or SRT installation From f9e7fafce37fbf6f61029db86e99bd05ba6e9e83 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 16:40:25 +0000 Subject: [PATCH 2/8] fix(workflow): refine sandbox agent awf version override handling Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d204dbf8-0c30-4183-91ab-662ce23a4ebc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/firewall.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/workflow/firewall.go b/pkg/workflow/firewall.go index 6aa6c2c2bc4..996bd53f644 100644 --- a/pkg/workflow/firewall.go +++ b/pkg/workflow/firewall.go @@ -74,10 +74,12 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { // Check network.firewall configuration if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil { config := workflowData.NetworkPermissions.Firewall - if agentVersion != "" && config.Version != agentVersion { + if agentVersion != "" { overrideConfig := *config overrideConfig.Version = agentVersion - firewallLog.Printf("Overriding firewall version %s with sandbox.agent.version %s", config.Version, agentVersion) + if config.Version != agentVersion { + firewallLog.Printf("Overriding firewall version %s with sandbox.agent.version %s", config.Version, agentVersion) + } return &overrideConfig } if firewallLog.Enabled() { @@ -90,7 +92,7 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { if agentVersion != "" { firewallLog.Printf("Using sandbox.agent.version override: %s", agentVersion) return &FirewallConfig{ - Enabled: true, + Enabled: isFirewallEnabled(workflowData), Version: agentVersion, } } From 2eb90d0bf8c335e4aa0553ca75f2645eed25c4f6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 16:42:47 +0000 Subject: [PATCH 3/8] chore(workflow): improve sandbox agent version override logging Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d204dbf8-0c30-4183-91ab-662ce23a4ebc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/firewall.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/workflow/firewall.go b/pkg/workflow/firewall.go index 996bd53f644..74d77fbbc06 100644 --- a/pkg/workflow/firewall.go +++ b/pkg/workflow/firewall.go @@ -78,7 +78,11 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { overrideConfig := *config overrideConfig.Version = agentVersion if config.Version != agentVersion { - firewallLog.Printf("Overriding firewall version %s with sandbox.agent.version %s", config.Version, agentVersion) + if config.Version == "" { + firewallLog.Printf("Using sandbox.agent.version %s for firewall version", agentVersion) + } else { + firewallLog.Printf("Overriding firewall version %s with sandbox.agent.version %s", config.Version, agentVersion) + } } return &overrideConfig } From 940737dab80f9388eee0986be52e1be0a7c66fe1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 16:45:23 +0000 Subject: [PATCH 4/8] chore(workflow): polish awf version override handling and tests Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d204dbf8-0c30-4183-91ab-662ce23a4ebc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/firewall.go | 14 ++++++++------ pkg/workflow/firewall_version_pinning_test.go | 7 ++++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/pkg/workflow/firewall.go b/pkg/workflow/firewall.go index 74d77fbbc06..a5625ae527d 100644 --- a/pkg/workflow/firewall.go +++ b/pkg/workflow/firewall.go @@ -75,14 +75,16 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil { config := workflowData.NetworkPermissions.Firewall if agentVersion != "" { + if config.Version == agentVersion { + return config + } + overrideConfig := *config overrideConfig.Version = agentVersion - if config.Version != agentVersion { - if config.Version == "" { - firewallLog.Printf("Using sandbox.agent.version %s for firewall version", agentVersion) - } else { - firewallLog.Printf("Overriding firewall version %s with sandbox.agent.version %s", config.Version, agentVersion) - } + if config.Version == "" { + firewallLog.Printf("Using sandbox.agent.version %s for firewall version", agentVersion) + } else { + firewallLog.Printf("Overriding firewall version %s with sandbox.agent.version %s", config.Version, agentVersion) } return &overrideConfig } diff --git a/pkg/workflow/firewall_version_pinning_test.go b/pkg/workflow/firewall_version_pinning_test.go index d3e424d08a1..8b30d6703ab 100644 --- a/pkg/workflow/firewall_version_pinning_test.go +++ b/pkg/workflow/firewall_version_pinning_test.go @@ -160,6 +160,7 @@ func TestCopilotEngineFirewallInstallation(t *testing.T) { t.Run("uses sandbox.agent.version when firewall version is not specified", func(t *testing.T) { engine := NewCopilotEngine() + sandboxAgentVersion := "v0.30.2" workflowData := &WorkflowData{ Name: "test-workflow", EngineConfig: &EngineConfig{ @@ -173,7 +174,7 @@ func TestCopilotEngineFirewallInstallation(t *testing.T) { SandboxConfig: &SandboxConfig{ Agent: &AgentSandboxConfig{ Type: SandboxTypeAWF, - Version: "v0.30.2", + Version: sandboxAgentVersion, }, }, } @@ -195,8 +196,8 @@ func TestCopilotEngineFirewallInstallation(t *testing.T) { t.Fatal("Expected to find AWF installation step when firewall is enabled") } - if !strings.Contains(awfStepStr, "v0.30.2") { - t.Errorf("AWF installation step should pass sandbox.agent.version %s to script", "v0.30.2") + if !strings.Contains(awfStepStr, sandboxAgentVersion) { + t.Errorf("AWF installation step should pass sandbox.agent.version %s to script", sandboxAgentVersion) } }) From 7f58a9c6edbc5328c833fc8a613e7699214e3c64 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 17:12:33 +0000 Subject: [PATCH 5/8] refactor: remove network.firewall frontmatter and migrate via codemod Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b86247d5-111e-4b6e-9574-3730e58aa0fe Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_network_firewall.go | 119 ++++++++++++------ pkg/cli/codemod_network_firewall_test.go | 63 +++++++++- pkg/cli/fix_command_test.go | 24 ++-- pkg/parser/schemas/main_workflow_schema.json | 66 ---------- pkg/workflow/compiler_permissions_test.go | 5 +- .../firewall_default_enablement_test.go | 27 ++-- pkg/workflow/firewall_log_level_test.go | 95 +------------- .../frontmatter_extraction_security.go | 8 +- pkg/workflow/network_merge_edge_cases_test.go | 1 - pkg/workflow/network_merge_import_test.go | 1 - pkg/workflow/sandbox_agent_false_test.go | 17 +-- pkg/workflow/sandbox_custom_agent_test.go | 2 - .../strict_mode_permissions_validation.go | 2 +- 13 files changed, 184 insertions(+), 246 deletions(-) diff --git a/pkg/cli/codemod_network_firewall.go b/pkg/cli/codemod_network_firewall.go index 42cd153d8a1..b5ecd0694db 100644 --- a/pkg/cli/codemod_network_firewall.go +++ b/pkg/cli/codemod_network_firewall.go @@ -1,6 +1,7 @@ package cli import ( + "fmt" "strings" "github.com/github/gh-aw/pkg/logger" @@ -20,53 +21,89 @@ func getNetworkFirewallCodemod() Codemod { LogMsg: "Applied network.firewall migration (firewall now always enabled via sandbox.agent: awf default)", Log: networkFirewallCodemodLog, PostTransform: func(lines []string, frontmatter map[string]any, fieldValue any) []string { - // Note: We no longer set sandbox.agent: false since the firewall is mandatory - // The firewall is always enabled via the default sandbox.agent: awf - _, hasSandbox := frontmatter["sandbox"] - // Add sandbox.agent if not already present AND if firewall was explicitly true - // (no need to add sandbox.agent: awf if firewall was false, since awf is now the default) - if !hasSandbox && fieldValue == true { - // Only add sandbox.agent: awf if firewall was explicitly set to true - sandboxLines := []string{ - "sandbox:", - " agent: awf # Firewall enabled (migrated from network.firewall)", + if !hasSandbox { + sandboxLines := sandboxAgentLinesFromFirewall(fieldValue) + if len(sandboxLines) > 0 { + lines = insertSandboxAfterNetworkBlock(lines, sandboxLines) + networkFirewallCodemodLog.Print("Converted deprecated network.firewall to sandbox.agent") } - - // Try to place it after network block - insertIndex := -1 - inNet := false - for i, line := range lines { - trimmed := strings.TrimSpace(line) - if strings.HasPrefix(trimmed, "network:") { - inNet = true - } else if inNet && len(trimmed) > 0 { - // Check if this is a top-level key (no leading whitespace) - if isTopLevelKey(line) { - // Found next top-level key - insertIndex = i - break - } - } - } - - if insertIndex >= 0 { - // Insert after network block - newLines := make([]string, 0, len(lines)+len(sandboxLines)) - newLines = append(newLines, lines[:insertIndex]...) - newLines = append(newLines, sandboxLines...) - newLines = append(newLines, lines[insertIndex:]...) - lines = newLines - } else { - // Append at the end - lines = append(lines, sandboxLines...) - } - - networkFirewallCodemodLog.Print("Added sandbox.agent: awf (firewall was explicitly enabled)") } return lines }, }) } + +func sandboxAgentLinesFromFirewall(fieldValue any) []string { + switch value := fieldValue.(type) { + case bool: + if value { + return []string{ + "sandbox:", + " agent: awf # Migrated from deprecated network setting", + } + } + return []string{ + "sandbox:", + " agent: false # Migrated from deprecated network setting", + } + case string: + if strings.EqualFold(strings.TrimSpace(value), "disable") { + return []string{ + "sandbox:", + " agent: false # Migrated from deprecated network setting", + } + } + case map[string]any: + versionValue, hasVersion := value["version"] + if hasVersion { + version := strings.TrimSpace(fmt.Sprintf("%v", versionValue)) + if version != "" { + return []string{ + "sandbox:", + " agent:", + " id: awf # Migrated from deprecated network setting", + fmt.Sprintf(" version: %q", version), + } + } + } + return []string{ + "sandbox:", + " agent: awf # Migrated from deprecated network setting", + } + case nil: + return []string{ + "sandbox:", + " agent: awf # Migrated from deprecated network setting", + } + } + return nil +} + +func insertSandboxAfterNetworkBlock(lines []string, sandboxLines []string) []string { + insertIndex := -1 + inNetworkBlock := false + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "network:") { + inNetworkBlock = true + continue + } + if inNetworkBlock && len(trimmed) > 0 && isTopLevelKey(line) { + insertIndex = i + break + } + } + + if insertIndex >= 0 { + newLines := make([]string, 0, len(lines)+len(sandboxLines)) + newLines = append(newLines, lines[:insertIndex]...) + newLines = append(newLines, sandboxLines...) + newLines = append(newLines, lines[insertIndex:]...) + return newLines + } + + return append(lines, sandboxLines...) +} diff --git a/pkg/cli/codemod_network_firewall_test.go b/pkg/cli/codemod_network_firewall_test.go index 4ff84e9b17c..f64d7424468 100644 --- a/pkg/cli/codemod_network_firewall_test.go +++ b/pkg/cli/codemod_network_firewall_test.go @@ -80,7 +80,8 @@ permissions: require.NoError(t, err) assert.True(t, applied) assert.NotContains(t, result, "firewall:", "Should remove firewall field") - assert.NotContains(t, result, "sandbox:", "Should not add sandbox for firewall: false") + assert.Contains(t, result, "sandbox:", "Should add sandbox block") + assert.Contains(t, result, "agent: false", "Should convert firewall false to sandbox.agent: false") } func TestNetworkFirewallCodemod_NoNetworkField(t *testing.T) { @@ -312,4 +313,64 @@ permissions: assert.NotContains(t, result, "firewall:") assert.NotContains(t, result, "enabled:") assert.NotContains(t, result, "strict:") + assert.Contains(t, result, "sandbox:", "Should add sandbox block") + assert.Contains(t, result, "agent: awf", "Should convert firewall object to sandbox.agent: awf") +} + +func TestNetworkFirewallCodemod_NullFirewallAddsSandboxAgent(t *testing.T) { + codemod := getNetworkFirewallCodemod() + + content := `--- +on: workflow_dispatch +network: + firewall: null +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "network": map[string]any{ + "firewall": nil, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.NotContains(t, result, "firewall:", "Should remove firewall field") + assert.Contains(t, result, "sandbox:", "Should add sandbox block") + assert.Contains(t, result, "agent: awf", "Should convert firewall null to sandbox.agent: awf") +} + +func TestNetworkFirewallCodemod_PreservesFirewallVersionInSandboxAgent(t *testing.T) { + codemod := getNetworkFirewallCodemod() + + content := `--- +on: workflow_dispatch +network: + firewall: + version: v1.2.3 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "network": map[string]any{ + "firewall": map[string]any{ + "version": "v1.2.3", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.NotContains(t, result, "firewall:", "Should remove firewall field") + assert.Contains(t, result, "sandbox:", "Should add sandbox block") + assert.Contains(t, result, "id: awf", "Should create sandbox.agent object") + assert.Contains(t, result, `version: "v1.2.3"`, "Should preserve firewall.version as sandbox.agent.version") } diff --git a/pkg/cli/fix_command_test.go b/pkg/cli/fix_command_test.go index 8a70135203d..40087399b43 100644 --- a/pkg/cli/fix_command_test.go +++ b/pkg/cli/fix_command_test.go @@ -186,9 +186,11 @@ This is a test workflow. t.Error("Expected firewall field to be removed, but it still exists") } - // firewall: null should NOT add sandbox.agent (only true values do) - if strings.Contains(updatedStr, "sandbox:") { - t.Errorf("Expected sandbox field NOT to be added for null firewall, got:\n%s", updatedStr) + if !strings.Contains(updatedStr, "sandbox:") { + t.Errorf("Expected sandbox field to be added for null firewall, got:\n%s", updatedStr) + } + if !strings.Contains(updatedStr, "agent: awf") { + t.Errorf("Expected sandbox.agent: awf to be added for null firewall, got:\n%s", updatedStr) } } @@ -261,9 +263,14 @@ This is a test workflow. t.Error("Expected version field to be removed, but it still exists") } - // firewall with nested properties (non-boolean) should NOT add sandbox.agent - if strings.Contains(updatedStr, "sandbox:") { - t.Errorf("Expected sandbox field NOT to be added for nested firewall, got:\n%s", updatedStr) + if !strings.Contains(updatedStr, "sandbox:") { + t.Errorf("Expected sandbox field to be added for nested firewall, got:\n%s", updatedStr) + } + if !strings.Contains(updatedStr, "agent:") { + t.Errorf("Expected sandbox.agent object to be added for nested firewall, got:\n%s", updatedStr) + } + if !strings.Contains(updatedStr, `version: "v1.0.0"`) { + t.Errorf("Expected firewall version to be migrated to sandbox.agent.version, got:\n%s", updatedStr) } // Verify compilation works @@ -358,9 +365,8 @@ This workflow tests comment and empty line handling. t.Error("Expected comment within firewall block to be removed, but it still exists") } - // firewall with nested properties should NOT add sandbox.agent - if strings.Contains(updatedStr, "sandbox:") { - t.Errorf("Expected sandbox field NOT to be added for nested firewall, got:\n%s", updatedStr) + if !strings.Contains(updatedStr, "sandbox:") { + t.Errorf("Expected sandbox field to be added for nested firewall, got:\n%s", updatedStr) } // Verify other network fields are preserved diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index edc502944de..6d1047b858b 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -2877,13 +2877,6 @@ }, { "allowed": ["defaults", "python", "node", "*.example.com"] - }, - { - "allowed": ["api.openai.com", "*.github.com"], - "firewall": { - "version": "v1.0.0", - "log-level": "debug" - } } ], "oneOf": [ @@ -2913,65 +2906,6 @@ "description": "Domain name or ecosystem identifier to block. Supports wildcards like '*.example.com' (matches sub.example.com, deep.nested.example.com, and example.com itself) and ecosystem names like 'python', 'node'." }, "$comment": "Blocked domains are subtracted from the allowed list. Useful for blocking specific domains or ecosystems within broader allowed categories." - }, - "firewall": { - "description": "AWF (Agent Workflow Firewall) configuration for network egress control. Supported for copilot, claude, codex, gemini, and crush engines.", - "deprecated": true, - "x-deprecation-message": "The firewall is now always enabled. Use 'sandbox.agent' to configure the sandbox type.", - "oneOf": [ - { - "type": "null", - "description": "Enable AWF with default settings (equivalent to empty object)" - }, - { - "type": "boolean", - "description": "Enable (true) or explicitly disable (false) AWF firewall" - }, - { - "type": "string", - "enum": ["disable"], - "description": "Disable AWF firewall (triggers warning if allowed != *, error in strict mode if allowed is not * or engine does not support firewall)" - }, - { - "type": "object", - "description": "Custom AWF configuration with version and arguments", - "properties": { - "args": { - "type": "array", - "description": "Optional additional arguments to pass to AWF wrapper", - "items": { - "type": "string" - } - }, - "version": { - "type": ["string", "number"], - "description": "AWF version to use (empty = latest release). Can be a string (e.g., 'v1.0.0', 'latest') or number (e.g., 20, 3.11). Numeric values are automatically converted to strings at runtime.", - "examples": ["v1.0.0", "latest", 20, 3.11] - }, - "log-level": { - "type": "string", - "description": "AWF log level (default: info). Valid values: debug, info, warn, error", - "enum": ["debug", "info", "warn", "error"] - }, - "ssl-bump": { - "type": "boolean", - "description": "Enable SSL Bump for HTTPS content inspection. When enabled, AWF can filter HTTPS traffic by URL patterns instead of just domain names. This feature is specific to AWF. Default: false", - "default": false - }, - "allow-urls": { - "type": "array", - "description": "URL patterns to allow for HTTPS traffic (requires ssl-bump: true). Supports wildcards for flexible path matching. Must include https:// scheme. This feature is specific to AWF.", - "items": { - "type": "string", - "pattern": "^https://.*", - "description": "HTTPS URL pattern with optional wildcards (e.g., 'https://github.com/githubnext/*')" - }, - "examples": [["https://github.com/githubnext/*", "https://api.github.com/repos/*"]] - } - }, - "additionalProperties": false - } - ] } }, "additionalProperties": false diff --git a/pkg/workflow/compiler_permissions_test.go b/pkg/workflow/compiler_permissions_test.go index 4cbe7fabb4c..25a2577cf74 100644 --- a/pkg/workflow/compiler_permissions_test.go +++ b/pkg/workflow/compiler_permissions_test.go @@ -219,7 +219,7 @@ This is a test workflow with empty network permissions (deny all). } }) - t.Run("network with allowed domains and firewall enabled should use AWF", func(t *testing.T) { + t.Run("network with allowed domains should use AWF", func(t *testing.T) { testContent := `--- on: push strict: false @@ -227,7 +227,6 @@ engine: id: claude network: allowed: ["example.com", "api.github.com"] - firewall: true --- # Test Workflow @@ -254,7 +253,7 @@ This is a test workflow with explicit network permissions. // Should contain AWF wrapper with --allow-domains if !strings.Contains(string(lockContent), "sudo -E awf") { - t.Error("Should contain AWF wrapper with explicit network permissions and firewall: true") + t.Error("Should contain AWF wrapper with explicit network permissions") } if !strings.Contains(string(lockContent), "--allow-domains") { t.Error("Should contain --allow-domains flag in AWF command") diff --git a/pkg/workflow/firewall_default_enablement_test.go b/pkg/workflow/firewall_default_enablement_test.go index 479cbb320e2..32ed3cf9df6 100644 --- a/pkg/workflow/firewall_default_enablement_test.go +++ b/pkg/workflow/firewall_default_enablement_test.go @@ -192,7 +192,7 @@ func TestCopilotFirewallDefaultIntegration(t *testing.T) { } }) - t.Run("copilot workflow with explicit firewall:false does not include AWF", func(t *testing.T) { + t.Run("copilot workflow with sandbox.agent:false does not include AWF", func(t *testing.T) { frontmatter := map[string]any{ "on": "workflow_dispatch", "permissions": map[string]any{ @@ -200,8 +200,10 @@ func TestCopilotFirewallDefaultIntegration(t *testing.T) { }, "engine": "copilot", "network": map[string]any{ - "allowed": []any{"example.com"}, - "firewall": false, + "allowed": []any{"example.com"}, + }, + "sandbox": map[string]any{ + "agent": false, }, } @@ -221,28 +223,23 @@ func TestCopilotFirewallDefaultIntegration(t *testing.T) { t.Fatal("Expected network permissions to be extracted") } - // Verify firewall is explicitly disabled - if networkPerms.Firewall == nil { - t.Error("Expected firewall config to be present") + sandboxConfig := c.extractSandboxConfig(frontmatter) + if sandboxConfig == nil || sandboxConfig.Agent == nil { + t.Fatal("Expected sandbox.agent configuration to be extracted") } - - if networkPerms.Firewall.Enabled { - t.Error("Expected firewall.Enabled to be false") + if !sandboxConfig.Agent.Disabled { + t.Fatal("Expected sandbox.agent to be explicitly disabled") } // Enable firewall by default (should not override explicit config) - enableFirewallByDefaultForCopilot(engineConfig.ID, networkPerms, nil) - - // Verify firewall is still disabled - if networkPerms.Firewall.Enabled { - t.Error("Expected firewall to remain disabled when explicitly set to false") - } + enableFirewallByDefaultForCopilot(engineConfig.ID, networkPerms, sandboxConfig) // Create workflow data workflowData := &WorkflowData{ Name: "test-workflow", EngineConfig: engineConfig, NetworkPermissions: networkPerms, + SandboxConfig: sandboxConfig, } // Get installation steps diff --git a/pkg/workflow/firewall_log_level_test.go b/pkg/workflow/firewall_log_level_test.go index ba42dcf6e5f..0cc06821fc1 100644 --- a/pkg/workflow/firewall_log_level_test.go +++ b/pkg/workflow/firewall_log_level_test.go @@ -7,12 +7,12 @@ import ( "testing" ) -// TestFirewallLogLevelParsing tests that the log-level field is correctly parsed +// TestFirewallLogLevelParsing tests handling of deprecated network.firewall parsing. func TestFirewallLogLevelParsing(t *testing.T) { compiler := NewCompiler() compiler.SetSkipValidation(true) - t.Run("log-level is parsed from network.firewall object", func(t *testing.T) { + t.Run("network.firewall object is ignored during extraction", func(t *testing.T) { frontmatter := map[string]any{ "network": map[string]any{ "firewall": map[string]any{ @@ -26,95 +26,8 @@ func TestFirewallLogLevelParsing(t *testing.T) { t.Fatal("Network permissions should not be nil") } - if networkPerms.Firewall == nil { - t.Fatal("Firewall config should not be nil") - } - - if networkPerms.Firewall.LogLevel != "debug" { - t.Errorf("Expected log-level 'debug', got '%s'", networkPerms.Firewall.LogLevel) - } - }) - - t.Run("log-level defaults to empty string when not specified", func(t *testing.T) { - frontmatter := map[string]any{ - "network": map[string]any{ - "firewall": map[string]any{ - "version": "v1.0.0", - }, - }, - } - - networkPerms := compiler.extractNetworkPermissions(frontmatter) - if networkPerms == nil { - t.Fatal("Network permissions should not be nil") - } - - if networkPerms.Firewall == nil { - t.Fatal("Firewall config should not be nil") - } - - if networkPerms.Firewall.LogLevel != "" { - t.Errorf("Expected log-level to be empty string, got '%s'", networkPerms.Firewall.LogLevel) - } - }) - - t.Run("log-level works with other firewall fields", func(t *testing.T) { - frontmatter := map[string]any{ - "network": map[string]any{ - "firewall": map[string]any{ - "version": "v1.0.0", - "log-level": "info", - "args": []any{"--custom-arg"}, - }, - }, - } - - networkPerms := compiler.extractNetworkPermissions(frontmatter) - if networkPerms == nil { - t.Fatal("Network permissions should not be nil") - } - - if networkPerms.Firewall == nil { - t.Fatal("Firewall config should not be nil") - } - - if networkPerms.Firewall.LogLevel != "info" { - t.Errorf("Expected log-level 'info', got '%s'", networkPerms.Firewall.LogLevel) - } - - if networkPerms.Firewall.Version != "v1.0.0" { - t.Errorf("Expected version 'v1.0.0', got '%s'", networkPerms.Firewall.Version) - } - - if len(networkPerms.Firewall.Args) != 1 { - t.Errorf("Expected 1 arg, got %d", len(networkPerms.Firewall.Args)) - } - }) - - t.Run("different log-level values are parsed correctly", func(t *testing.T) { - logLevels := []string{"debug", "info", "warn", "error"} - - for _, level := range logLevels { - frontmatter := map[string]any{ - "network": map[string]any{ - "firewall": map[string]any{ - "log-level": level, - }, - }, - } - - networkPerms := compiler.extractNetworkPermissions(frontmatter) - if networkPerms == nil { - t.Fatalf("Network permissions should not be nil for log-level '%s'", level) - } - - if networkPerms.Firewall == nil { - t.Fatalf("Firewall config should not be nil for log-level '%s'", level) - } - - if networkPerms.Firewall.LogLevel != level { - t.Errorf("Expected log-level '%s', got '%s'", level, networkPerms.Firewall.LogLevel) - } + if networkPerms.Firewall != nil { + t.Fatalf("Expected network.firewall to be ignored, got: %+v", networkPerms.Firewall) } }) } diff --git a/pkg/workflow/frontmatter_extraction_security.go b/pkg/workflow/frontmatter_extraction_security.go index 8b716a6d484..a9f8ceeead3 100644 --- a/pkg/workflow/frontmatter_extraction_security.go +++ b/pkg/workflow/frontmatter_extraction_security.go @@ -23,7 +23,7 @@ func (c *Compiler) extractNetworkPermissions(frontmatter map[string]any) *Networ return nil } - // Handle object format: { allowed: [...], firewall: ... } or {} + // Handle object format: { allowed: [...], blocked: [...] } or {} if networkObj, ok := network.(map[string]any); ok { frontmatterExtractionSecurityLog.Printf("Network permissions object format with %d fields", len(networkObj)) permissions := &NetworkPermissions{ @@ -54,12 +54,6 @@ func (c *Compiler) extractNetworkPermissions(frontmatter map[string]any) *Networ } } - // Extract firewall configuration if present - if firewall, hasFirewall := networkObj["firewall"]; hasFirewall { - frontmatterExtractionSecurityLog.Print("Extracting firewall configuration") - permissions.Firewall = c.extractFirewallConfig(firewall) - } - // Empty object {} means no network access (empty allowed list) return permissions } diff --git a/pkg/workflow/network_merge_edge_cases_test.go b/pkg/workflow/network_merge_edge_cases_test.go index ca36a581244..fd6fa1060ac 100644 --- a/pkg/workflow/network_merge_edge_cases_test.go +++ b/pkg/workflow/network_merge_edge_cases_test.go @@ -46,7 +46,6 @@ network: allowed: - github.com - api.github.com - firewall: true imports: - shared.md --- diff --git a/pkg/workflow/network_merge_import_test.go b/pkg/workflow/network_merge_import_test.go index c535247329f..f82a0a800a7 100644 --- a/pkg/workflow/network_merge_import_test.go +++ b/pkg/workflow/network_merge_import_test.go @@ -51,7 +51,6 @@ network: allowed: - defaults - github.com - firewall: true imports: - shared-network.md --- diff --git a/pkg/workflow/sandbox_agent_false_test.go b/pkg/workflow/sandbox_agent_false_test.go index 46f5c031db1..bd8a914dc0e 100644 --- a/pkg/workflow/sandbox_agent_false_test.go +++ b/pkg/workflow/sandbox_agent_false_test.go @@ -157,8 +157,8 @@ Test workflow to verify default sandbox.agent behavior (awf). }) } -func TestNetworkFirewallDeprecationWarning(t *testing.T) { - t.Run("network.firewall compiles successfully (deprecated)", func(t *testing.T) { +func TestNetworkFirewallFrontmatterRejected(t *testing.T) { + t.Run("network.firewall is rejected by schema", func(t *testing.T) { // Create temp directory for test workflows workflowsDir := t.TempDir() @@ -172,7 +172,7 @@ strict: false on: workflow_dispatch --- -Test workflow to verify network.firewall still works (deprecated). + Test workflow to verify network.firewall is rejected. ` workflowPath := filepath.Join(workflowsDir, "test-firewall-deprecated.md") @@ -181,13 +181,14 @@ Test workflow to verify network.firewall still works (deprecated). t.Fatalf("Failed to write workflow file: %v", err) } - // Compile the workflow compiler := NewCompiler() - compiler.SetSkipValidation(true) - // The compilation should succeed (deprecated fields should still work) - if err := compiler.CompileWorkflow(workflowPath); err != nil { - t.Fatalf("Compilation failed: %v", err) + err = compiler.CompileWorkflow(workflowPath) + if err == nil { + t.Fatal("Expected compilation to fail for deprecated network.firewall frontmatter, but got nil error") + } + if !strings.Contains(err.Error(), "firewall") { + t.Fatalf("Expected error to reference firewall field, got: %v", err) } }) } diff --git a/pkg/workflow/sandbox_custom_agent_test.go b/pkg/workflow/sandbox_custom_agent_test.go index 1a00ef9830b..5aac673e419 100644 --- a/pkg/workflow/sandbox_custom_agent_test.go +++ b/pkg/workflow/sandbox_custom_agent_test.go @@ -127,7 +127,6 @@ strict: false network: allowed: - "example.com" - firewall: true sandbox: agent: id: awf @@ -202,7 +201,6 @@ strict: false network: allowed: - "example.com" - firewall: true sandbox: agent: type: awf diff --git a/pkg/workflow/strict_mode_permissions_validation.go b/pkg/workflow/strict_mode_permissions_validation.go index a164ac002d5..edd38ea0fac 100644 --- a/pkg/workflow/strict_mode_permissions_validation.go +++ b/pkg/workflow/strict_mode_permissions_validation.go @@ -182,7 +182,7 @@ func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *N // In strict mode, firewall MUST be enabled if networkPermissions.Firewall == nil || !networkPermissions.Firewall.Enabled { strictModeValidationLog.Printf("Firewall validation failed: firewall not enabled in strict mode") - return fmt.Errorf("strict mode: firewall must be enabled for %s engine with network restrictions. The firewall should be enabled by default, but if you've explicitly disabled it with 'network.firewall: false' or 'sandbox.agent: false', this is not allowed in strict mode for security reasons. See: https://github.github.com/gh-aw/reference/network/", engineID) + return fmt.Errorf("strict mode: firewall must be enabled for %s engine with network restrictions. The firewall should be enabled by default, but if you've explicitly disabled it with 'sandbox.agent: false', this is not allowed in strict mode for security reasons. See: https://github.github.com/gh-aw/reference/network/", engineID) } strictModeValidationLog.Printf("Firewall validation passed") From 627f931212b60ed18f2c2e92f1691c7ddaa9b543 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 17:15:37 +0000 Subject: [PATCH 6/8] chore(cli): tighten firewall version migration normalization Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b86247d5-111e-4b6e-9574-3730e58aa0fe Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_network_firewall.go | 47 ++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/pkg/cli/codemod_network_firewall.go b/pkg/cli/codemod_network_firewall.go index b5ecd0694db..82575c6f516 100644 --- a/pkg/cli/codemod_network_firewall.go +++ b/pkg/cli/codemod_network_firewall.go @@ -1,7 +1,7 @@ package cli import ( - "fmt" + "strconv" "strings" "github.com/github/gh-aw/pkg/logger" @@ -59,13 +59,12 @@ func sandboxAgentLinesFromFirewall(fieldValue any) []string { case map[string]any: versionValue, hasVersion := value["version"] if hasVersion { - version := strings.TrimSpace(fmt.Sprintf("%v", versionValue)) - if version != "" { + if version, ok := normalizeFirewallVersion(versionValue); ok { return []string{ "sandbox:", " agent:", " id: awf # Migrated from deprecated network setting", - fmt.Sprintf(" version: %q", version), + " version: " + formatSandboxVersionYAML(version), } } } @@ -82,6 +81,46 @@ func sandboxAgentLinesFromFirewall(fieldValue any) []string { return nil } +func normalizeFirewallVersion(versionValue any) (string, bool) { + switch value := versionValue.(type) { + case string: + trimmed := strings.TrimSpace(value) + return trimmed, trimmed != "" + case int: + return strconv.Itoa(value), true + case int8: + return strconv.FormatInt(int64(value), 10), true + case int16: + return strconv.FormatInt(int64(value), 10), true + case int32: + return strconv.FormatInt(int64(value), 10), true + case int64: + return strconv.FormatInt(value, 10), true + case uint: + return strconv.FormatUint(uint64(value), 10), true + case uint8: + return strconv.FormatUint(uint64(value), 10), true + case uint16: + return strconv.FormatUint(uint64(value), 10), true + case uint32: + return strconv.FormatUint(uint64(value), 10), true + case uint64: + return strconv.FormatUint(value, 10), true + case float32: + return strconv.FormatFloat(float64(value), 'f', -1, 64), true + case float64: + return strconv.FormatFloat(value, 'f', -1, 64), true + default: + return "", false + } +} + +func formatSandboxVersionYAML(version string) string { + // Always quote because sandbox.agent.version is a string field, and this prevents + // YAML from interpreting numeric-like versions as numbers. + return strconv.Quote(version) +} + func insertSandboxAfterNetworkBlock(lines []string, sandboxLines []string) []string { insertIndex := -1 inNetworkBlock := false From 3e3e135141b1beb760b53e1693db749f7f3cbc2d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 21 Apr 2026 17:32:47 +0000 Subject: [PATCH 7/8] docs(adr): add draft ADR-27626 for sandbox.agent.version and network.firewall migration Co-Authored-By: Claude Sonnet 4.6 --- ...-version-and-network-firewall-migration.md | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 docs/adr/27626-sandbox-agent-version-and-network-firewall-migration.md diff --git a/docs/adr/27626-sandbox-agent-version-and-network-firewall-migration.md b/docs/adr/27626-sandbox-agent-version-and-network-firewall-migration.md new file mode 100644 index 00000000000..884dfa9810e --- /dev/null +++ b/docs/adr/27626-sandbox-agent-version-and-network-firewall-migration.md @@ -0,0 +1,76 @@ +# ADR-27626: Introduce sandbox.agent.version and Remove Deprecated network.firewall Field + +**Date**: 2026-04-21 +**Status**: Draft +**Deciders**: Unknown (copilot-swe-agent / pelikhan) + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +The `network.firewall` frontmatter field was previously used to configure the Agent Workflow Firewall (AWF) in agentic workflow definitions. It was deprecated in favor of the unified `sandbox.agent` configuration block, but the migration codemod only handled the `true` case, leaving `false`, `null`, `"disable"`, and object-with-version forms unmigrated. Additionally, there was no mechanism to pin a specific AWF version via frontmatter — users who needed to run a particular AWF release had no stable, documented way to express that constraint. This PR addresses both gaps simultaneously: removing the deprecated field from the schema and expanding the codemod to cover all value variants. + +### Decision + +We will remove `network.firewall` from the frontmatter schema entirely and add `sandbox.agent.version` as a first-class string field for pinning the AWF version used during installation and runtime. The codemod will be expanded to migrate all `network.firewall` value forms — `true`, `false`, `null`, `"disable"`, and `{version: ...}` objects — to their `sandbox.agent` equivalents. This consolidates AWF configuration under a single `sandbox.agent` surface and ensures the migration path covers every variant that appears in the wild. + +### Alternatives Considered + +#### Alternative 1: Retain network.firewall as a Deprecated Alias + +Keep `network.firewall` in the schema with a deprecation warning, parsing it alongside `sandbox.agent` and merging the values at runtime. This avoids a hard removal and gives teams more migration runway, but perpetuates two competing configuration surfaces indefinitely, increases parser complexity, and makes it harder to reason about precedence when both fields are set. + +#### Alternative 2: Introduce a Flat sandbox.awf-version Field + +Add a top-level sibling key `sandbox.awf-version` (or similar) rather than nesting the version under `sandbox.agent`. This is marginally more ergonomic to type but diverges from the `sandbox.agent` object model already established, creates a second place where AWF version information lives, and complicates the precedence rules for effective version resolution. + +### Consequences + +#### Positive +- All `network.firewall` value forms now have a deterministic, tested migration path to `sandbox.agent`. +- Users can pin an explicit AWF version via `sandbox.agent.version`, enabling reproducible builds without relying on the latest release. +- The schema surface is reduced by removing a deprecated field and its associated validation rules. + +#### Negative +- Behavioral change: `network.firewall: false` previously produced no `sandbox` block; it now migrates to `sandbox.agent: false`. Workflows relying on the old no-op behavior will see a new block added by the codemod. +- The `normalizeFirewallVersion` helper must handle all numeric YAML types (int8 through uint64, float32/float64) because YAML parsers may unmarshal numeric version values into any of these types, increasing codemod surface area. + +#### Neutral +- The codemod expansion requires updating existing tests that asserted `sandbox:` was *not* added for `false` and nested object cases; these expectations were inverted. +- The `aw-info` AWF version reporting now reads `sandbox.agent.version` in addition to the legacy `firewallVersion` field, requiring both paths to be tested. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### Frontmatter Schema + +1. Implementations **MUST NOT** include `network.firewall` in the frontmatter schema as a valid, non-deprecated field. +2. Implementations **MUST** expose `sandbox.agent.version` as an optional string field in the frontmatter schema for specifying an AWF version override. +3. The `sandbox.agent.version` field **MUST** be treated as a string type; numeric-like values written in YAML **MUST** be quoted at generation time to prevent YAML parsers from interpreting them as numbers. + +### Codemod Migration + +1. The `network.firewall` codemod **MUST** produce a `sandbox.agent` block for every non-absent value of `network.firewall`, including `true`, `false`, `null`, `"disable"`, and object forms. +2. When `network.firewall` is `true` or `null`, the codemod **MUST** emit `sandbox.agent: awf`. +3. When `network.firewall` is `false` or `"disable"`, the codemod **MUST** emit `sandbox.agent: false`. +4. When `network.firewall` is an object containing a `version` key, the codemod **MUST** emit a `sandbox.agent` object block with `id: awf` and `version: ""`. +5. The codemod **MUST NOT** add a `sandbox` block when one already exists in the frontmatter. +6. Numeric version values encountered during migration **MUST** be normalized to their string representation before being written as `sandbox.agent.version`. + +### AWF Version Resolution + +1. When `sandbox.agent.version` is set, it **MUST** take precedence over any version derived from the legacy `network.firewall` configuration when resolving the effective AWF version for installation and runtime. +2. Implementations **SHOULD** surface the resolved AWF version in `aw-info` metadata so that workflow authors can verify which version is active. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24736713102) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* From 6e2022f92914174fa5715d2193f3e355399a86f7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 17:53:07 +0000 Subject: [PATCH 8/8] fix(codemod): merge firewall disable/version into existing sandbox Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3917e2dd-b6b3-4753-a749-56d561d560fa Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_network_firewall.go | 116 ++++++++++++++++++++++- pkg/cli/codemod_network_firewall_test.go | 77 +++++++++++++++ pkg/workflow/firewall.go | 2 +- 3 files changed, 193 insertions(+), 2 deletions(-) diff --git a/pkg/cli/codemod_network_firewall.go b/pkg/cli/codemod_network_firewall.go index 82575c6f516..395c109d0c5 100644 --- a/pkg/cli/codemod_network_firewall.go +++ b/pkg/cli/codemod_network_firewall.go @@ -29,8 +29,13 @@ func getNetworkFirewallCodemod() Codemod { lines = insertSandboxAfterNetworkBlock(lines, sandboxLines) networkFirewallCodemodLog.Print("Converted deprecated network.firewall to sandbox.agent") } + return lines } + lines, merged := mergeFirewallIntoExistingSandbox(lines, fieldValue) + if merged { + networkFirewallCodemodLog.Print("Merged deprecated network.firewall into existing sandbox.agent") + } return lines }, }) @@ -107,7 +112,7 @@ func normalizeFirewallVersion(versionValue any) (string, bool) { case uint64: return strconv.FormatUint(value, 10), true case float32: - return strconv.FormatFloat(float64(value), 'f', -1, 64), true + return strconv.FormatFloat(float64(value), 'f', -1, 32), true case float64: return strconv.FormatFloat(value, 'f', -1, 64), true default: @@ -146,3 +151,112 @@ func insertSandboxAfterNetworkBlock(lines []string, sandboxLines []string) []str return append(lines, sandboxLines...) } + +func mergeFirewallIntoExistingSandbox(lines []string, fieldValue any) ([]string, bool) { + agentLines := sandboxAgentLinesForExistingSandbox(fieldValue) + if len(agentLines) == 0 { + return lines, false + } + + sandboxIdx := -1 + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if isTopLevelKey(line) && strings.HasPrefix(trimmed, "sandbox:") { + sandboxIdx = i + break + } + } + if sandboxIdx == -1 { + return lines, false + } + + sandboxIndent := getIndentation(lines[sandboxIdx]) + agentIndent := sandboxIndent + " " + sandboxEnd := len(lines) + for i := sandboxIdx + 1; i < len(lines); i++ { + if isTopLevelKey(lines[i]) { + sandboxEnd = i + break + } + } + + agentStart := -1 + for i := sandboxIdx + 1; i < sandboxEnd; i++ { + trimmed := strings.TrimSpace(lines[i]) + if strings.HasPrefix(trimmed, "agent:") && getIndentation(lines[i]) == agentIndent { + agentStart = i + break + } + } + + indentedAgentLines := indentLines(agentLines, agentIndent) + if agentStart == -1 { + newLines := make([]string, 0, len(lines)+len(indentedAgentLines)) + newLines = append(newLines, lines[:sandboxIdx+1]...) + newLines = append(newLines, indentedAgentLines...) + newLines = append(newLines, lines[sandboxIdx+1:]...) + return newLines, true + } + + agentEnd := agentStart + 1 + agentFieldIndent := getIndentation(lines[agentStart]) + for agentEnd < sandboxEnd { + trimmed := strings.TrimSpace(lines[agentEnd]) + if trimmed == "" { + agentEnd++ + continue + } + if strings.HasPrefix(trimmed, "#") { + if len(getIndentation(lines[agentEnd])) > len(agentFieldIndent) { + agentEnd++ + continue + } + break + } + if len(getIndentation(lines[agentEnd])) > len(agentFieldIndent) { + agentEnd++ + continue + } + break + } + + newLines := make([]string, 0, len(lines)-((agentEnd-agentStart)-len(indentedAgentLines))) + newLines = append(newLines, lines[:agentStart]...) + newLines = append(newLines, indentedAgentLines...) + newLines = append(newLines, lines[agentEnd:]...) + return newLines, true +} + +func sandboxAgentLinesForExistingSandbox(fieldValue any) []string { + switch value := fieldValue.(type) { + case bool: + if !value { + return []string{"agent: false # Migrated from deprecated network setting"} + } + case string: + if strings.EqualFold(strings.TrimSpace(value), "disable") { + return []string{"agent: false # Migrated from deprecated network setting"} + } + case map[string]any: + versionValue, hasVersion := value["version"] + if hasVersion { + if version, ok := normalizeFirewallVersion(versionValue); ok { + return []string{ + "agent:", + " id: awf # Migrated from deprecated network setting", + " version: " + formatSandboxVersionYAML(version), + } + } + } + } + + return nil +} + +func indentLines(lines []string, indent string) []string { + indented := make([]string, 0, len(lines)) + for _, line := range lines { + indented = append(indented, indent+line) + } + return indented +} diff --git a/pkg/cli/codemod_network_firewall_test.go b/pkg/cli/codemod_network_firewall_test.go index f64d7424468..8856d237076 100644 --- a/pkg/cli/codemod_network_firewall_test.go +++ b/pkg/cli/codemod_network_firewall_test.go @@ -177,6 +177,77 @@ permissions: assert.Contains(t, result, "agent: custom", "Should preserve existing sandbox config") } +func TestNetworkFirewallCodemod_MigratesFirewallFalseIntoExistingSandbox(t *testing.T) { + codemod := getNetworkFirewallCodemod() + + content := `--- +on: workflow_dispatch +network: + firewall: false +sandbox: + mcp: true +--- + +# Test Workflow` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "network": map[string]any{ + "firewall": false, + }, + "sandbox": map[string]any{ + "mcp": true, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.NotContains(t, result, "firewall:", "Should remove firewall field") + assert.Contains(t, result, "sandbox:", "Should preserve existing sandbox block") + assert.Contains(t, result, "mcp: true", "Should preserve existing sandbox settings") + assert.Contains(t, result, "agent: false", "Should migrate firewall false to sandbox.agent: false") +} + +func TestNetworkFirewallCodemod_MigratesFirewallVersionIntoExistingSandbox(t *testing.T) { + codemod := getNetworkFirewallCodemod() + + content := `--- +on: workflow_dispatch +network: + firewall: + version: 0.9 +sandbox: + mcp: true +--- + +# Test Workflow` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "network": map[string]any{ + "firewall": map[string]any{ + "version": 0.9, + }, + }, + "sandbox": map[string]any{ + "mcp": true, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.NotContains(t, result, "firewall:", "Should remove firewall field") + assert.Contains(t, result, "sandbox:", "Should preserve existing sandbox block") + assert.Contains(t, result, "mcp: true", "Should preserve existing sandbox settings") + assert.Contains(t, result, "agent:", "Should add sandbox.agent object") + assert.Contains(t, result, "id: awf", "Should set sandbox.agent.id") + assert.Contains(t, result, `version: "0.9"`, "Should migrate firewall.version to sandbox.agent.version") +} + func TestNetworkFirewallCodemod_PreservesOtherNetworkFields(t *testing.T) { codemod := getNetworkFirewallCodemod() @@ -374,3 +445,9 @@ network: assert.Contains(t, result, "id: awf", "Should create sandbox.agent object") assert.Contains(t, result, `version: "v1.2.3"`, "Should preserve firewall.version as sandbox.agent.version") } + +func TestNormalizeFirewallVersion_Float32Uses32BitPrecision(t *testing.T) { + version, ok := normalizeFirewallVersion(float32(0.9)) + require.True(t, ok) + assert.Equal(t, "0.9", version) +} diff --git a/pkg/workflow/firewall.go b/pkg/workflow/firewall.go index a5625ae527d..73599f09fe6 100644 --- a/pkg/workflow/firewall.go +++ b/pkg/workflow/firewall.go @@ -71,7 +71,7 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { agentVersion = agentConfig.Version } - // Check network.firewall configuration + // Check resolved firewall configuration from network permissions if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil { config := workflowData.NetworkPermissions.Firewall if agentVersion != "" {