Skip to content

Require dangerously-disable-sandbox-agent feature flag to allow sandbox.agent: false #38205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Jun 9, 2026
Merged

Require dangerously-disable-sandbox-agent feature flag to allow sandbox.agent: false #38205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1aaf8d6
Require dangerously-disable-sandbox-agent feature flag for sandbox.ag…
Copilot Jun 9, 2026
b24b4df
Use DangerouslyDisableSandboxAgentFeatureFlag constant in error messa…
Copilot Jun 9, 2026
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
10 changes: 10 additions & 0 deletions pkg/constants/feature_constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,14 @@ const (
// features:
// group-concurrency-queue: false
GroupConcurrencyQueueFeatureFlag FeatureFlag = "group-concurrency-queue"
// DangerouslyDisableSandboxAgentFeatureFlag is required to allow sandbox.agent: false.
// Without this flag, setting sandbox.agent to false raises a validation error.
// This flag is intentionally named with "dangerously" to make the security
// implications explicit and visible in the workflow frontmatter.
//
// Workflow frontmatter usage:
//
// features:
// dangerously-disable-sandbox-agent: true
DangerouslyDisableSandboxAgentFeatureFlag FeatureFlag = "dangerously-disable-sandbox-agent"
)
3 changes: 3 additions & 0 deletions pkg/workflow/compiler_validators_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,9 @@ func TestValidateToolConfiguration_EmitsSandboxWarningBeforeThreatDetectionError

workflowData := &WorkflowData{
Name: "Test",
Features: map[string]any{
"dangerously-disable-sandbox-agent": true,
},
SandboxConfig: &SandboxConfig{
Agent: &AgentSandboxConfig{Disabled: true},
},
Expand Down
6 changes: 6 additions & 0 deletions pkg/workflow/importable_tools_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,8 @@ permissions:
issues: read
tools:
bash: true
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
imports:
Expand Down Expand Up @@ -890,6 +892,8 @@ strict: false
permissions:
contents: read
issues: read
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
imports:
Expand Down Expand Up @@ -962,6 +966,8 @@ permissions:
issues: read
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
imports:
Expand Down
8 changes: 8 additions & 0 deletions pkg/workflow/pull_request_target_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ on:
types: [opened]
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
checkout: false
Expand All @@ -58,6 +60,8 @@ on:
types: [opened]
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
---
Expand Down Expand Up @@ -103,6 +107,8 @@ on:
types: [opened]
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
---
Expand All @@ -124,6 +130,8 @@ on:
branches: [main]
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
---
Expand Down
51 changes: 45 additions & 6 deletions pkg/workflow/sandbox_agent_false_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

func TestSandboxAgentMandatory(t *testing.T) {
t.Run("sandbox.agent: false is accepted and disables agent sandbox", func(t *testing.T) {
t.Run("sandbox.agent: false with feature flag is accepted and disables agent sandbox", func(t *testing.T) {
// Create temp directory for test workflows
workflowsDir := t.TempDir()

Expand All @@ -20,13 +20,15 @@ network:
allowed:
- defaults
- github.com
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
strict: false
on: workflow_dispatch
---

Test workflow to verify sandbox.agent: false is accepted and disables agent sandbox.
Test workflow to verify sandbox.agent: false is accepted when the feature flag is set.
`

workflowPath := filepath.Join(workflowsDir, "test-agent-false.md")
Expand All @@ -35,13 +37,13 @@ Test workflow to verify sandbox.agent: false is accepted and disables agent sand
t.Fatalf("Failed to write workflow file: %v", err)
}

// Compile the workflow
// Compile the workflow (validation runs unconditionally; validateSandboxConfig
// is not gated by skipValidation, so this exercises the feature-flag check)
compiler := NewCompiler()
compiler.SetSkipValidation(true)

// Should succeed in non-strict mode
// Should succeed when the feature flag is set
if err := compiler.CompileWorkflow(workflowPath); err != nil {
t.Fatalf("Expected compilation to succeed with sandbox.agent: false in non-strict mode, but got error: %v", err)
t.Fatalf("Expected compilation to succeed with sandbox.agent: false and feature flag, but got error: %v", err)
}

// Read the compiled workflow
Expand Down Expand Up @@ -157,6 +159,43 @@ Test workflow to verify default sandbox.agent behavior (awf).
})
}

func TestSandboxAgentFalseRequiresFeatureFlag(t *testing.T) {
t.Run("sandbox.agent: false without feature flag is rejected", func(t *testing.T) {

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The new test only covers sandbox.agent: false with the flag entirely absent. An edge case worth locking in: setting dangerously-disable-sandbox-agent: false explicitly should also be rejected — parseFeatureValue returns false for a boolean false, so the code handles it correctly, but there is no test asserting that behaviour.

💡 Suggested additional sub-test 
t.Run("sandbox.agent: false with feature flag explicitly false is rejected", func(t *testing.T) {
    workflowsDir := t.TempDir()
    markdown := `---
engine: copilot
features:
  dangerously-disable-sandbox-agent: false
sandbox:
  agent: false
strict: false
on: workflow_dispatch
---
Test workflow.
`
    workflowPath := filepath.Join(workflowsDir, "test.md")
    _ = os.WriteFile(workflowPath, []byte(markdown), 0644)
    compiler := NewCompiler()
    err := compiler.CompileWorkflow(workflowPath)
    if err == nil {
        t.Fatal("Expected compilation to fail when flag is explicitly false")
    }
    if !strings.Contains(err.Error(), "dangerously-disable-sandbox-agent") {
        t.Fatalf("Expected error to reference flag, got: %v", err)
    }
})

workflowsDir := t.TempDir()

markdown := `---
engine: copilot
network:
allowed:
- defaults
- github.com
sandbox:
agent: false
strict: false
on: workflow_dispatch
---

Test workflow to verify sandbox.agent: false is rejected without the feature flag.
`

workflowPath := filepath.Join(workflowsDir, "test-agent-false-no-flag.md")
err := os.WriteFile(workflowPath, []byte(markdown), 0644)
if err != nil {
t.Fatalf("Failed to write workflow file: %v", err)
}

compiler := NewCompiler()

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fragile test: missing SetSkipValidation(true) breaks error-message assertion if the default changes.

Every other compiler := NewCompiler() in this file is immediately followed by compiler.SetSkipValidation(true) (lines 41-42, 93-94, 139-140). This test omits that call and silently inherits the current default (skipValidation: true, annotated in compiler_types.go as "for now").

💡 Why this is fragile

The test asserts the error message contains "dangerously-disable-sandbox-agent", which is only true when validateSandboxConfig is the first validator to fail. If skipValidation defaults change to false, schema validation runs first and can produce a different error (e.g. "missing required field: permissions"), causing the strings.Contains assertion to fail — masking the real reason.

Fix:

compiler := NewCompiler()
compiler.SetSkipValidation(true) // ensure schema validation is skipped so validateSandboxConfig fires first


err = compiler.CompileWorkflow(workflowPath)
if err == nil {
t.Fatal("Expected compilation to fail when sandbox.agent: false without feature flag, but got nil error")
}
if !strings.Contains(err.Error(), "dangerously-disable-sandbox-agent") {
t.Fatalf("Expected error to reference 'dangerously-disable-sandbox-agent', got: %v", err)
}
})
}

func TestNetworkFirewallFrontmatterRejected(t *testing.T) {
t.Run("network.firewall is rejected by schema", func(t *testing.T) {
// Create temp directory for test workflows
Expand Down
17 changes: 12 additions & 5 deletions pkg/workflow/sandbox_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,19 @@ func validateSandboxConfig(workflowData *WorkflowData) error {
sandboxConfig := workflowData.SandboxConfig

// Check if sandbox.agent: false was specified
// In non-strict mode, this is allowed (with a warning shown at compile time)
// The strict mode check happens in validateStrictFirewall()
// This requires the "dangerously-disable-sandbox-agent" feature flag to be enabled.
// Without the feature flag, setting sandbox.agent: false is a validation error.
if sandboxConfig.Agent != nil && sandboxConfig.Agent.Disabled {
Comment on lines 78 to 81
// sandbox.agent: false is allowed in non-strict mode, so we don't error here
// The warning is emitted in compiler.go
sandboxValidationLog.Print("sandbox.agent: false detected, will be validated by strict mode check")
if !isFeatureEnabled(constants.DangerouslyDisableSandboxAgentFeatureFlag, workflowData) {

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] isFeatureEnabled also accepts this flag via the GH_AW_FEATURES environment variable, so GH_AW_FEATURES=dangerously-disable-sandbox-agent on the runner silently bypasses the frontmatter gate. This path is untested, and for a "dangerously"-named security flag the behaviour should be explicit.

💡 Suggested options

If the env-var bypass is intentional (useful for local dev/CI runner-level override), add a test to document it:

t.Run("GH_AW_FEATURES env var enables sandbox.agent: false", func(t *testing.T) {
    t.Setenv("GH_AW_FEATURES", "dangerously-disable-sandbox-agent")
    // compile a workflow with sandbox.agent: false but no frontmatter flag
    // expect: compilation succeeds
})

If the env-var bypass is not intended (the whole point is that the opt-in must be visible in the committed workflow file), isFeatureEnabled would need a frontmatter-only mode for security-critical flags.

Either way, a one-line comment on the isFeatureEnabled call here stating the intent would help reviewers.

flag := string(constants.DangerouslyDisableSandboxAgentFeatureFlag)
return NewValidationError(
"sandbox.agent",
"false",
fmt.Sprintf("disabling the agent sandbox requires the '%s' feature flag", flag),
fmt.Sprintf("Add the feature flag to your workflow frontmatter:\n\nfeatures:\n %s: true\nsandbox:\n agent: false\n\nSee: %s", flag, constants.DocsSandboxURL),
)
}
sandboxValidationLog.Printf("sandbox.agent: false permitted by %s feature flag", constants.DangerouslyDisableSandboxAgentFeatureFlag)
}

// Validate mounts syntax if specified in agent config
Expand Down
14 changes: 14 additions & 0 deletions pkg/workflow/workflow_run_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ on:
types: [completed]
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
---
Expand Down Expand Up @@ -81,6 +83,8 @@ on:
- develop
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
---
Expand Down Expand Up @@ -186,6 +190,8 @@ on:
branches: [main]
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
---
Expand All @@ -210,6 +216,8 @@ on:
types: [completed]
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
---
Expand All @@ -233,6 +241,8 @@ on:
branches: []
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
---
Expand Down Expand Up @@ -303,6 +313,8 @@ strict: false
on: push
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
---
Expand All @@ -323,6 +335,8 @@ on:
types: [completed]
tools:
github: false
features:
dangerously-disable-sandbox-agent: true
sandbox:
agent: false
---
Expand Down
Loading