Skip to content

Commit 08e112b

Browse files
authored
Disable sandbox.agent: false in strict mode for copilot engine (#6903)
1 parent a3d3d9d commit 08e112b

7 files changed

Lines changed: 80 additions & 22 deletions

.github/workflows/smoke-copilot-playwright.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ safe-outputs:
5656
run-success: "📰 VERDICT: [{workflow_name}]({run_url}) has concluded. All systems operational. This is a developing story. 🎤"
5757
run-failure: "📰 DEVELOPING STORY: [{workflow_name}]({run_url}) reports {status}. Our correspondents are investigating the incident..."
5858
timeout-minutes: 5
59-
strict: true
59+
strict: false
6060
steps:
6161
# Pre-flight Docker container test for Playwright MCP
6262
- name: Pre-flight Playwright MCP Test

.github/workflows/smoke-copilot.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ safe-outputs:
4040
run-success: "📰 VERDICT: [{workflow_name}]({run_url}) has concluded. All systems operational. This is a developing story. 🎤"
4141
run-failure: "📰 DEVELOPING STORY: [{workflow_name}]({run_url}) reports {status}. Our correspondents are investigating the incident..."
4242
timeout-minutes: 5
43-
strict: true
43+
strict: false
4444
---
4545

4646
# Smoke Test: Copilot Engine Validation

pkg/workflow/aw_info_steps_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ permissions:
4747
engine: copilot
4848
network:
4949
firewall: false
50+
strict: false
5051
---
5152
5253
# Test firewall disabled

pkg/workflow/compiler_parse.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,9 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error)
211211
// (unless SRT sandbox is configured, since AWF and SRT are mutually exclusive)
212212
enableFirewallByDefaultForCopilot(engineSetting, networkPermissions, sandboxConfig)
213213

214-
// Validate firewall is enabled in strict mode for copilot with network restrictions
215-
if err := c.validateStrictFirewall(engineSetting, networkPermissions, sandboxConfig); err != nil {
216-
return nil, err
217-
}
218-
219-
// Save the initial strict mode state again for network support check
214+
// Re-evaluate strict mode for firewall and network validation
220215
// (it was restored after validateStrictMode but we need it again)
221-
initialStrictModeForNetwork := c.strictMode
216+
initialStrictModeForFirewall := c.strictMode
222217
if !c.strictMode {
223218
// CLI flag not set, check frontmatter
224219
if strictValue, exists := result.Frontmatter["strict"]; exists {
@@ -232,15 +227,21 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error)
232227
}
233228
}
234229

230+
// Validate firewall is enabled in strict mode for copilot with network restrictions
231+
if err := c.validateStrictFirewall(engineSetting, networkPermissions, sandboxConfig); err != nil {
232+
c.strictMode = initialStrictModeForFirewall
233+
return nil, err
234+
}
235+
235236
// Check if the engine supports network restrictions when they are defined
236237
if err := c.checkNetworkSupport(agenticEngine, networkPermissions); err != nil {
237238
// Restore strict mode before returning error
238-
c.strictMode = initialStrictModeForNetwork
239+
c.strictMode = initialStrictModeForFirewall
239240
return nil, err
240241
}
241242

242243
// Restore the strict mode state after network check
243-
c.strictMode = initialStrictModeForNetwork
244+
c.strictMode = initialStrictModeForFirewall
244245

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

pkg/workflow/firewall_default_enablement_test.go

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ func TestStrictModeFirewallValidation(t *testing.T) {
402402
}
403403
})
404404

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

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

421421
err := compiler.validateStrictFirewall("copilot", networkPerms, sandboxConfig)
422-
if err != nil {
423-
t.Errorf("Expected no error when sandbox.agent is false, got: %v", err)
422+
if err == nil {
423+
t.Error("Expected error when sandbox.agent is false in strict mode for copilot")
424+
}
425+
expectedMsg := "sandbox.agent: false"
426+
if !strings.Contains(err.Error(), expectedMsg) {
427+
t.Errorf("Expected error message to contain '%s', got: %v", expectedMsg, err)
428+
}
429+
})
430+
431+
t.Run("strict mode refuses sandbox.agent: false for all engines", func(t *testing.T) {
432+
compiler := NewCompiler(false, "", "test")
433+
compiler.SetStrictMode(true)
434+
435+
networkPerms := &NetworkPermissions{
436+
Allowed: []string{"example.com"},
437+
ExplicitlyDefined: true,
438+
Firewall: nil,
439+
}
440+
441+
sandboxConfig := &SandboxConfig{
442+
Agent: &AgentSandboxConfig{
443+
Disabled: true,
444+
},
445+
}
446+
447+
// All engines should refuse sandbox.agent: false in strict mode
448+
err := compiler.validateStrictFirewall("claude", networkPerms, sandboxConfig)
449+
if err == nil {
450+
t.Error("Expected error for non-copilot engine with sandbox.agent: false in strict mode")
451+
}
452+
expectedMsg := "sandbox.agent: false"
453+
if !strings.Contains(err.Error(), expectedMsg) {
454+
t.Errorf("Expected error message to contain '%s', got: %v", expectedMsg, err)
424455
}
425456
})
426457

@@ -459,4 +490,26 @@ func TestStrictModeFirewallValidation(t *testing.T) {
459490
t.Errorf("Expected no error in non-strict mode, got: %v", err)
460491
}
461492
})
493+
494+
t.Run("non-strict mode allows sandbox.agent: false for copilot", func(t *testing.T) {
495+
compiler := NewCompiler(false, "", "test")
496+
compiler.SetStrictMode(false)
497+
498+
networkPerms := &NetworkPermissions{
499+
Allowed: []string{"example.com"},
500+
ExplicitlyDefined: true,
501+
Firewall: nil,
502+
}
503+
504+
sandboxConfig := &SandboxConfig{
505+
Agent: &AgentSandboxConfig{
506+
Disabled: true,
507+
},
508+
}
509+
510+
err := compiler.validateStrictFirewall("copilot", networkPerms, sandboxConfig)
511+
if err != nil {
512+
t.Errorf("Expected no error in non-strict mode with sandbox.agent: false, got: %v", err)
513+
}
514+
})
462515
}

pkg/workflow/sandbox_agent_false_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ network:
2020
- github.com
2121
sandbox:
2222
agent: false
23+
strict: false
2324
on: workflow_dispatch
2425
---
2526
@@ -118,6 +119,7 @@ network:
118119
- github.com
119120
sandbox:
120121
agent: false
122+
strict: false
121123
on: workflow_dispatch
122124
---
123125

pkg/workflow/strict_mode_validation.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -214,15 +214,22 @@ func (c *Compiler) validateStrictMode(frontmatter map[string]any, networkPermiss
214214
return nil
215215
}
216216

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

225-
// Only apply to copilot engine
225+
// Check if sandbox.agent: false is set (explicitly disabled)
226+
// In strict mode, this is not allowed for any engine as it disables the agent sandbox
227+
if sandboxConfig != nil && sandboxConfig.Agent != nil && sandboxConfig.Agent.Disabled {
228+
strictModeValidationLog.Printf("sandbox.agent: false is set, refusing in strict mode")
229+
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/")
230+
}
231+
232+
// Only apply firewall validation to copilot engine
226233
if engineID != "copilot" {
227234
strictModeValidationLog.Printf("Non-copilot engine, skipping firewall validation")
228235
return nil
@@ -245,12 +252,6 @@ func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *N
245252
}
246253
}
247254

248-
// Check if sandbox.agent: false is set (explicitly disabled)
249-
if sandboxConfig != nil && sandboxConfig.Agent != nil && sandboxConfig.Agent.Disabled {
250-
strictModeValidationLog.Printf("sandbox.agent: false is set, skipping firewall validation")
251-
return nil
252-
}
253-
254255
// If network permissions don't exist, that's fine (will default to "defaults")
255256
if networkPermissions == nil {
256257
strictModeValidationLog.Printf("No network permissions, skipping firewall validation")

0 commit comments

Comments
 (0)