Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/smoke-copilot-playwright.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ safe-outputs:
run-success: "📰 VERDICT: [{workflow_name}]({run_url}) has concluded. All systems operational. This is a developing story. 🎤"
run-failure: "📰 DEVELOPING STORY: [{workflow_name}]({run_url}) reports {status}. Our correspondents are investigating the incident..."
timeout-minutes: 5
strict: true
strict: false
steps:
# Pre-flight Docker container test for Playwright MCP
- name: Pre-flight Playwright MCP Test
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/smoke-copilot.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ safe-outputs:
run-success: "📰 VERDICT: [{workflow_name}]({run_url}) has concluded. All systems operational. This is a developing story. 🎤"
run-failure: "📰 DEVELOPING STORY: [{workflow_name}]({run_url}) reports {status}. Our correspondents are investigating the incident..."
timeout-minutes: 5
strict: true
strict: false
---

# Smoke Test: Copilot Engine Validation
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/aw_info_steps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ permissions:
engine: copilot
network:
firewall: false
strict: false
---

# Test firewall disabled
Expand Down
19 changes: 10 additions & 9 deletions pkg/workflow/compiler_parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,9 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error)
// (unless SRT sandbox is configured, since AWF and SRT are mutually exclusive)
enableFirewallByDefaultForCopilot(engineSetting, networkPermissions, sandboxConfig)

// Validate firewall is enabled in strict mode for copilot with network restrictions
if err := c.validateStrictFirewall(engineSetting, networkPermissions, sandboxConfig); err != nil {
return nil, err
}

// Save the initial strict mode state again for network support check
// Re-evaluate strict mode for firewall and network validation
// (it was restored after validateStrictMode but we need it again)
initialStrictModeForNetwork := c.strictMode
initialStrictModeForFirewall := c.strictMode
if !c.strictMode {
// CLI flag not set, check frontmatter
if strictValue, exists := result.Frontmatter["strict"]; exists {
Expand All @@ -232,15 +227,21 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error)
}
}

// Validate firewall is enabled in strict mode for copilot with network restrictions
if err := c.validateStrictFirewall(engineSetting, networkPermissions, sandboxConfig); err != nil {
c.strictMode = initialStrictModeForFirewall
return nil, err
}

// Check if the engine supports network restrictions when they are defined
if err := c.checkNetworkSupport(agenticEngine, networkPermissions); err != nil {
// Restore strict mode before returning error
c.strictMode = initialStrictModeForNetwork
c.strictMode = initialStrictModeForFirewall
return nil, err
}

// Restore the strict mode state after network check
c.strictMode = initialStrictModeForNetwork
c.strictMode = initialStrictModeForFirewall

log.Print("Processing tools and includes...")

Expand Down
59 changes: 56 additions & 3 deletions pkg/workflow/firewall_default_enablement_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ func TestStrictModeFirewallValidation(t *testing.T) {
}
})

t.Run("strict mode skips validation when sandbox.agent is false", func(t *testing.T) {
t.Run("strict mode refuses sandbox.agent: false for copilot", func(t *testing.T) {
compiler := NewCompiler(false, "", "test")
compiler.SetStrictMode(true)

Expand All @@ -419,8 +419,39 @@ func TestStrictModeFirewallValidation(t *testing.T) {
}

err := compiler.validateStrictFirewall("copilot", networkPerms, sandboxConfig)
if err != nil {
t.Errorf("Expected no error when sandbox.agent is false, got: %v", err)
if err == nil {
t.Error("Expected error when sandbox.agent is false in strict mode for copilot")
}
expectedMsg := "sandbox.agent: false"
if !strings.Contains(err.Error(), expectedMsg) {
t.Errorf("Expected error message to contain '%s', got: %v", expectedMsg, err)
}
})

t.Run("strict mode refuses sandbox.agent: false for all engines", func(t *testing.T) {
compiler := NewCompiler(false, "", "test")
compiler.SetStrictMode(true)

networkPerms := &NetworkPermissions{
Allowed: []string{"example.com"},
ExplicitlyDefined: true,
Firewall: nil,
}

sandboxConfig := &SandboxConfig{
Agent: &AgentSandboxConfig{
Disabled: true,
},
}

// All engines should refuse sandbox.agent: false in strict mode
err := compiler.validateStrictFirewall("claude", networkPerms, sandboxConfig)
if err == nil {
t.Error("Expected error for non-copilot engine with sandbox.agent: false in strict mode")
}
expectedMsg := "sandbox.agent: false"
if !strings.Contains(err.Error(), expectedMsg) {
t.Errorf("Expected error message to contain '%s', got: %v", expectedMsg, err)
}
})

Expand Down Expand Up @@ -459,4 +490,26 @@ func TestStrictModeFirewallValidation(t *testing.T) {
t.Errorf("Expected no error in non-strict mode, got: %v", err)
}
})

t.Run("non-strict mode allows sandbox.agent: false for copilot", func(t *testing.T) {
compiler := NewCompiler(false, "", "test")
compiler.SetStrictMode(false)

networkPerms := &NetworkPermissions{
Allowed: []string{"example.com"},
ExplicitlyDefined: true,
Firewall: nil,
}

sandboxConfig := &SandboxConfig{
Agent: &AgentSandboxConfig{
Disabled: true,
},
}

err := compiler.validateStrictFirewall("copilot", networkPerms, sandboxConfig)
if err != nil {
t.Errorf("Expected no error in non-strict mode with sandbox.agent: false, got: %v", err)
}
})
}
2 changes: 2 additions & 0 deletions pkg/workflow/sandbox_agent_false_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ network:
- github.com
sandbox:
agent: false
strict: false
on: workflow_dispatch
---

Expand Down Expand Up @@ -118,6 +119,7 @@ network:
- github.com
sandbox:
agent: false
strict: false
on: workflow_dispatch
---

Expand Down
17 changes: 9 additions & 8 deletions pkg/workflow/strict_mode_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,15 +214,22 @@ func (c *Compiler) validateStrictMode(frontmatter map[string]any, networkPermiss
return nil
}

// validateStrictFirewall requires firewall to be enabled in strict mode for copilot engine
// validateStrictFirewall requires firewall to be enabled in strict mode
// when network domains are provided (non-wildcard)
func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *NetworkPermissions, sandboxConfig *SandboxConfig) error {
if !c.strictMode {
strictModeValidationLog.Printf("Strict mode disabled, skipping firewall validation")
return nil
}

// Only apply to copilot engine
// Check if sandbox.agent: false is set (explicitly disabled)
// In strict mode, this is not allowed for any engine as it disables the agent sandbox
if sandboxConfig != nil && sandboxConfig.Agent != nil && sandboxConfig.Agent.Disabled {
strictModeValidationLog.Printf("sandbox.agent: false is set, refusing in strict mode")
return fmt.Errorf("strict mode: 'sandbox.agent: false' is not allowed because it disables the agent sandbox. Remove 'sandbox.agent: false' or set 'strict: false' to disable strict mode. See: https://githubnext.github.io/gh-aw/reference/network/")
}

// Only apply firewall validation to copilot engine
if engineID != "copilot" {
strictModeValidationLog.Printf("Non-copilot engine, skipping firewall validation")
return nil
Expand All @@ -245,12 +252,6 @@ func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *N
}
}

// Check if sandbox.agent: false is set (explicitly disabled)
if sandboxConfig != nil && sandboxConfig.Agent != nil && sandboxConfig.Agent.Disabled {
strictModeValidationLog.Printf("sandbox.agent: false is set, skipping firewall validation")
return nil
}

// If network permissions don't exist, that's fine (will default to "defaults")
if networkPermissions == nil {
strictModeValidationLog.Printf("No network permissions, skipping firewall validation")
Expand Down