diff --git a/.github/aw/create-agentic-workflow.md b/.github/aw/create-agentic-workflow.md index a878049b0cd..ecf99f175d4 100644 --- a/.github/aw/create-agentic-workflow.md +++ b/.github/aw/create-agentic-workflow.md @@ -148,6 +148,7 @@ Rules: - always restrict `create-pull-request.allowed-files` - prefer the dedicated safe output instead of shelling out to `gh` for the same mutation - include `noop` guidance in the prompt so successful no-op runs are explicit +- when using `create-issue`, instruct the agent to provide a meaningful body (20-65000 characters; avoid placeholder-only text) ### 7. Decide who can trigger the workflow diff --git a/.github/aw/experiments.md b/.github/aw/experiments.md index 8235cdc7750..3413d3d244e 100644 --- a/.github/aw/experiments.md +++ b/.github/aw/experiments.md @@ -292,5 +292,5 @@ experiments: - ❌ **Interpreting early results** (<~20 runs/variant) — chance variation dominates. - ❌ **Experiments as feature flags** — use `features:` for deterministic switches. - ❌ **Engine experiments in one file** — `engine:` cannot switch mid-run; use two parallel files. -- ❌ **Nesting `{{#if experiments. }}` inside `{{#runtime-import? }}`** — evaluation order not guaranteed across import boundaries. +- ❌ **Nesting `{{#if experiments. }}` inside `{{#runtime-import? }}`** — evaluation order is brittle across import boundaries. Prefer explicit branching in the main workflow prompt or separate workflow files per variant. - ❌ **Writing the internal env-var form** `__GH_AW_EXPERIMENTS__*` — implementation detail, may change. diff --git a/.github/aw/safe-outputs-content.md b/.github/aw/safe-outputs-content.md index 15ba8b985b9..394f96959a3 100644 --- a/.github/aw/safe-outputs-content.md +++ b/.github/aw/safe-outputs-content.md @@ -26,6 +26,10 @@ description: Safe-output reference for issue, discussion, comment, and pull requ allowed-repos: [owner/other] # Optional: additional repos agent can target (agent uses `repo` field in output) ``` + `create_issue` output validation requires: + - `body` minimum length: **20** characters + - `body` maximum length: **65000** characters + **Auto-Expiration**: The `expires` field auto-closes issues after a time period. Supports integers (days) or relative formats (2h, 7d, 2w, 1m, 1y). Generates `agentics-maintenance.yml` workflow that runs at minimum required frequency based on shortest expiration time: 1 day or less → every 2 hours, 2 days → every 6 hours, 3-4 days → every 12 hours, 5+ days → daily. **Deduplication for Scheduled Workflows**: When a `schedule:` trigger is combined with `create-issue`, use `skip-if-match:` in the `on:` block to prevent opening a duplicate issue on every run. Pair with `expires:` so stale issues are cleaned up automatically: diff --git a/.github/aw/syntax-agentic.md b/.github/aw/syntax-agentic.md index a360d3e5f77..f39d3662605 100644 --- a/.github/aw/syntax-agentic.md +++ b/.github/aw/syntax-agentic.md @@ -62,6 +62,7 @@ description: Agentic workflow specific frontmatter fields for GitHub Agentic Wor - `cli-proxy: true` - Enable AWF CLI proxy sidecar for secure read-only `gh` CLI access without exposing `GITHUB_TOKEN` (requires AWF v0.26.0+). Prerequisite for `integrity-reactions`; the compiler enables it automatically when `integrity-reactions: true` is set. - `integrity-reactions: true` - Enable reaction-based integrity promotion/demotion. Maintainers can use 👍/❤️ reactions to promote content to `approved` and 👎/😕 to demote it to `none`. Compiler automatically enables `cli-proxy`. Requires `tools.github.min-integrity` to be set and MCPG >= v0.2.18. Defaults: endorsement reactions THUMBS_UP/HEART, disapproval reactions THUMBS_DOWN/CONFUSED, endorser-min-integrity: approved, disapproval-integrity: none. - `mcp-cli: true` - Deprecated. This flag has been removed; MCP CLI mounting is now always enabled when `tools.cli-proxy: true` is set. + - `dangerously-disable-sandbox-agent: ""` - Required when `sandbox.agent: false` is set. Must be a plain string justification (minimum 20 characters; expressions are not allowed) that explains why disabling the sandbox is safe for this workflow. - **`experiments:`** - A/B testing experiments for balanced variant selection (object) - Maps experiment names to variant lists (bare array) or full config objects @@ -343,9 +344,11 @@ description: Agentic workflow specific frontmatter fields for GitHub Agentic Wor model-fallback: false # Optional: disable model fallback (default true); set false for BYOK Azure OpenAI to prevent deployment-name rewriting ``` - - To disable the agent firewall while keeping MCP gateway enabled (not allowed in strict mode): + - To disable the agent firewall while keeping MCP gateway enabled, you must provide the dangerous-disable justification feature: ```yaml + features: + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false ``` diff --git a/.github/aw/upgrade-agentic-workflows.md b/.github/aw/upgrade-agentic-workflows.md index f25a7be6caa..3ecfa940f94 100644 --- a/.github/aw/upgrade-agentic-workflows.md +++ b/.github/aw/upgrade-agentic-workflows.md @@ -102,7 +102,13 @@ Before attempting to compile, apply automatic codemods: This will automatically update workflow files with changes like: - Replacing 'timeout_minutes' with 'timeout-minutes' - - Replacing 'network.firewall' with 'sandbox.agent: false' + - Replacing `network.firewall: false` with: + ```yaml + features: + dangerously-disable-sandbox-agent: "controlled environment with no internet access" + sandbox: + agent: false + ``` - Removing deprecated 'mcp-scripts.mode' field 2. **Review the Changes** diff --git a/docs/src/content/docs/reference/safe-outputs.md b/docs/src/content/docs/reference/safe-outputs.md index c438c9ea63b..d7c56b1993f 100644 --- a/docs/src/content/docs/reference/safe-outputs.md +++ b/docs/src/content/docs/reference/safe-outputs.md @@ -129,6 +129,8 @@ See [Cross-Repository Operations](/gh-aw/reference/cross-repository/) for compre #### `create_issue` tool field schema (`fields`) +`create_issue.body` must be between **20** and **65000** characters. + | Parameter | Type | Required | Description | Example | |-----------|------|----------|-------------|---------| | `fields` | `array` | No | Optional issue field updates to apply immediately after issue creation. | `[{"name":"Priority","value":"P1"}]` | diff --git a/pkg/cli/codemod_network_firewall.go b/pkg/cli/codemod_network_firewall.go index 395c109d0c5..554f4c0c2ec 100644 --- a/pkg/cli/codemod_network_firewall.go +++ b/pkg/cli/codemod_network_firewall.go @@ -1,6 +1,7 @@ package cli import ( + "fmt" "strconv" "strings" @@ -9,6 +10,8 @@ import ( var networkFirewallCodemodLog = logger.New("cli:codemod_network_firewall") +const migratedSandboxDisableJustification = "migrated from deprecated network.firewall disable setting" + // getNetworkFirewallCodemod creates a codemod for migrating network.firewall to sandbox.agent func getNetworkFirewallCodemod() Codemod { return newFieldRemovalCodemod(fieldRemovalCodemodConfig{ @@ -21,12 +24,16 @@ 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 { + requiresDisableFlag := requiresSandboxDisableFeatureFlag(fieldValue) _, hasSandbox := frontmatter["sandbox"] if !hasSandbox { sandboxLines := sandboxAgentLinesFromFirewall(fieldValue) if len(sandboxLines) > 0 { lines = insertSandboxAfterNetworkBlock(lines, sandboxLines) + if requiresDisableFlag { + lines = ensureSandboxDisableFeatureFlag(lines) + } networkFirewallCodemodLog.Print("Converted deprecated network.firewall to sandbox.agent") } return lines @@ -34,6 +41,9 @@ func getNetworkFirewallCodemod() Codemod { lines, merged := mergeFirewallIntoExistingSandbox(lines, fieldValue) if merged { + if requiresDisableFlag { + lines = ensureSandboxDisableFeatureFlag(lines) + } networkFirewallCodemodLog.Print("Merged deprecated network.firewall into existing sandbox.agent") } return lines @@ -41,6 +51,17 @@ func getNetworkFirewallCodemod() Codemod { }) } +func requiresSandboxDisableFeatureFlag(fieldValue any) bool { + switch value := fieldValue.(type) { + case bool: + return !value + case string: + return strings.EqualFold(strings.TrimSpace(value), "disable") + default: + return false + } +} + func sandboxAgentLinesFromFirewall(fieldValue any) []string { switch value := fieldValue.(type) { case bool: @@ -260,3 +281,59 @@ func indentLines(lines []string, indent string) []string { } return indented } + +func ensureSandboxDisableFeatureFlag(lines []string) []string { + featureKey := "dangerously-disable-sandbox-agent:" + + featuresIdx := -1 + featuresIndent := "" + featuresEnd := len(lines) + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if isTopLevelKey(line) && strings.HasPrefix(trimmed, "features:") { + featuresIdx = i + featuresIndent = getIndentation(line) + for j := i + 1; j < len(lines); j++ { + if isTopLevelKey(lines[j]) { + featuresEnd = j + break + } + } + break + } + } + + if featuresIdx >= 0 { + featureIndent := featuresIndent + " " + for i := featuresIdx + 1; i < featuresEnd; i++ { + if strings.HasPrefix(strings.TrimSpace(lines[i]), featureKey) { + return lines + } + } + featureLine := fmt.Sprintf("%s%s %s", featureIndent, featureKey, strconv.Quote(migratedSandboxDisableJustification)) + newLines := make([]string, 0, len(lines)+1) + newLines = append(newLines, lines[:featuresEnd]...) + newLines = append(newLines, featureLine) + newLines = append(newLines, lines[featuresEnd:]...) + return newLines + } + + insertIndex := len(lines) + for i, line := range lines { + if isTopLevelKey(line) && strings.HasPrefix(strings.TrimSpace(line), "sandbox:") { + insertIndex = i + break + } + } + + featureLines := []string{ + "features:", + " " + featureKey + " " + strconv.Quote(migratedSandboxDisableJustification), + } + + newLines := make([]string, 0, len(lines)+len(featureLines)) + newLines = append(newLines, lines[:insertIndex]...) + newLines = append(newLines, featureLines...) + newLines = append(newLines, lines[insertIndex:]...) + return newLines +} diff --git a/pkg/cli/codemod_network_firewall_test.go b/pkg/cli/codemod_network_firewall_test.go index 8856d237076..a3012d15e1b 100644 --- a/pkg/cli/codemod_network_firewall_test.go +++ b/pkg/cli/codemod_network_firewall_test.go @@ -82,6 +82,7 @@ permissions: assert.NotContains(t, result, "firewall:", "Should remove firewall field") assert.Contains(t, result, "sandbox:", "Should add sandbox block") assert.Contains(t, result, "agent: false", "Should convert firewall false to sandbox.agent: false") + assert.Contains(t, result, `dangerously-disable-sandbox-agent: "migrated from deprecated network.firewall disable setting"`, "Should add required sandbox-disable justification feature") } func TestNetworkFirewallCodemod_NoNetworkField(t *testing.T) { @@ -208,6 +209,43 @@ sandbox: 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") + assert.Contains(t, result, `dangerously-disable-sandbox-agent: "migrated from deprecated network.firewall disable setting"`, "Should add required sandbox-disable justification feature") +} + +func TestNetworkFirewallCodemod_PreservesExistingSandboxDisableJustification(t *testing.T) { + codemod := getNetworkFirewallCodemod() + + content := `--- +on: workflow_dispatch +network: + firewall: false +features: + dangerously-disable-sandbox-agent: "already documented justification string with enough detail" +sandbox: + mcp: true +--- + +# Test Workflow` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "network": map[string]any{ + "firewall": false, + }, + "features": map[string]any{ + "dangerously-disable-sandbox-agent": "already documented justification string with enough detail", + }, + "sandbox": map[string]any{ + "mcp": true, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, `dangerously-disable-sandbox-agent: "already documented justification string with enough detail"`) + assert.NotContains(t, result, migratedSandboxDisableJustification, "Should not overwrite existing justification") } func TestNetworkFirewallCodemod_MigratesFirewallVersionIntoExistingSandbox(t *testing.T) { diff --git a/pkg/constants/feature_constants.go b/pkg/constants/feature_constants.go index 59587f0f792..e13933af1d6 100644 --- a/pkg/constants/feature_constants.go +++ b/pkg/constants/feature_constants.go @@ -84,6 +84,6 @@ const ( // Workflow frontmatter usage: // // features: - // dangerously-disable-sandbox-agent: true + // dangerously-disable-sandbox-agent: "controlled environment with no internet access" DangerouslyDisableSandboxAgentFeatureFlag FeatureFlag = "dangerously-disable-sandbox-agent" ) diff --git a/pkg/workflow/compiler_validators_test.go b/pkg/workflow/compiler_validators_test.go index 340b24f3729..c2f610b3f82 100644 --- a/pkg/workflow/compiler_validators_test.go +++ b/pkg/workflow/compiler_validators_test.go @@ -385,7 +385,7 @@ func TestValidateToolConfiguration_EmitsSandboxWarningBeforeThreatDetectionError workflowData := &WorkflowData{ Name: "Test", Features: map[string]any{ - "dangerously-disable-sandbox-agent": true, + "dangerously-disable-sandbox-agent": "controlled environment with no internet access", }, SandboxConfig: &SandboxConfig{ Agent: &AgentSandboxConfig{Disabled: true}, diff --git a/pkg/workflow/importable_tools_test.go b/pkg/workflow/importable_tools_test.go index 364f380da4e..051bd34ada8 100644 --- a/pkg/workflow/importable_tools_test.go +++ b/pkg/workflow/importable_tools_test.go @@ -802,7 +802,7 @@ permissions: tools: bash: true features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false imports: @@ -893,7 +893,7 @@ permissions: contents: read issues: read features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false imports: @@ -967,7 +967,7 @@ permissions: tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false imports: diff --git a/pkg/workflow/imports_test.go b/pkg/workflow/imports_test.go index aa6d38311c0..fc5dbb9c2d4 100644 --- a/pkg/workflow/imports_test.go +++ b/pkg/workflow/imports_test.go @@ -174,6 +174,69 @@ This is a test workflow with multiple imports. } } +func TestCompileWorkflowWithConditionalImport(t *testing.T) { + tempDir := testutil.TempDir(t, "test-*") + + sharedPath := filepath.Join(tempDir, "shared-conditional.md") + sharedContent := `--- +steps: + - name: Conditional Imported Step + run: echo "from import" +--- + +Imported conditional instructions. +` + if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { + t.Fatalf("Failed to write shared conditional file: %v", err) + } + + workflowPath := filepath.Join(tempDir, "test-workflow.md") + workflowContent := `--- +on: issues +permissions: + contents: read + issues: read + pull-requests: read +engine: copilot +experiments: + strategy: [eager, lazy] +imports: + - path: shared-conditional.md + if: "experiments.strategy == 'eager'" +--- + +# Test Workflow + +Main workflow body. +` + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := workflow.NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("CompileWorkflow failed: %v", err) + } + + lockFilePath := stringutil.MarkdownToLockFile(workflowPath) + lockFileContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + compiled := string(lockFileContent) + assertions := []string{ + `{{#if experiments.strategy == "eager"}}`, + "{{/if}}", + "needs.activation.outputs.strategy == 'eager'", + } + for _, expected := range assertions { + if !strings.Contains(compiled, expected) { + t.Errorf("Expected compiled workflow to contain %q", expected) + } + } +} + func TestCompileWorkflowWithMCPServersImport(t *testing.T) { // Create a temporary directory for test files tempDir := testutil.TempDir(t, "test-*") diff --git a/pkg/workflow/pull_request_target_validation_test.go b/pkg/workflow/pull_request_target_validation_test.go index 3668bdc4a4e..c5de575852e 100644 --- a/pkg/workflow/pull_request_target_validation_test.go +++ b/pkg/workflow/pull_request_target_validation_test.go @@ -37,7 +37,7 @@ on: tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false checkout: false @@ -61,7 +61,7 @@ on: tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false --- @@ -108,7 +108,7 @@ on: tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false --- @@ -131,7 +131,7 @@ on: tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false --- diff --git a/pkg/workflow/sandbox_agent_false_test.go b/pkg/workflow/sandbox_agent_false_test.go index 7fc26de1a23..cfbe6a25199 100644 --- a/pkg/workflow/sandbox_agent_false_test.go +++ b/pkg/workflow/sandbox_agent_false_test.go @@ -21,7 +21,7 @@ network: - defaults - github.com features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false strict: false @@ -194,6 +194,78 @@ Test workflow to verify sandbox.agent: false is rejected without the feature fla t.Fatalf("Expected error to reference 'dangerously-disable-sandbox-agent', got: %v", err) } }) + + t.Run("sandbox.agent: false with short justification is rejected", func(t *testing.T) { + workflowsDir := t.TempDir() + + markdown := `--- +engine: copilot +network: + allowed: + - defaults + - github.com +features: + dangerously-disable-sandbox-agent: "too short" +sandbox: + agent: false +strict: false +on: workflow_dispatch +--- + +Test workflow to verify sandbox.agent: false is rejected when feature justification is too short. +` + + workflowPath := filepath.Join(workflowsDir, "test-agent-false-short-flag.md") + err := os.WriteFile(workflowPath, []byte(markdown), 0644) + if err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + err = compiler.CompileWorkflow(workflowPath) + if err == nil { + t.Fatal("Expected compilation to fail when justification is too short, but got nil error") + } + if !strings.Contains(err.Error(), "at least 20 characters") { + t.Fatalf("Expected error to mention minimum length, got: %v", err) + } + }) + + t.Run("sandbox.agent: false with expression justification is rejected", func(t *testing.T) { + workflowsDir := t.TempDir() + + markdown := `--- +engine: copilot +network: + allowed: + - defaults + - github.com +features: + dangerously-disable-sandbox-agent: "${{ inputs.reason }}" +sandbox: + agent: false +strict: false +on: workflow_dispatch +--- + +Test workflow to verify sandbox.agent: false is rejected when feature uses an expression. +` + + workflowPath := filepath.Join(workflowsDir, "test-agent-false-expression-flag.md") + err := os.WriteFile(workflowPath, []byte(markdown), 0644) + if err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + err = compiler.CompileWorkflow(workflowPath) + if err == nil { + t.Fatal("Expected compilation to fail when justification uses an expression, but got nil error") + } + if !strings.Contains(err.Error(), "expressions") { + t.Fatalf("Expected error to mention expressions are not allowed, got: %v", err) + } + }) } func TestNetworkFirewallFrontmatterRejected(t *testing.T) { diff --git a/pkg/workflow/sandbox_test.go b/pkg/workflow/sandbox_test.go index 47ac4f3d2dd..a18806bfb86 100644 --- a/pkg/workflow/sandbox_test.go +++ b/pkg/workflow/sandbox_test.go @@ -41,6 +41,61 @@ func TestValidateSandboxConfig(t *testing.T) { }, }, }, + { + name: "sandbox.agent false with valid justification", + data: &WorkflowData{ + Features: map[string]any{ + "dangerously-disable-sandbox-agent": "controlled environment with no internet access", + }, + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + Disabled: true, + }, + }, + }, + }, + { + name: "sandbox.agent false without justification", + data: &WorkflowData{ + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + Disabled: true, + }, + }, + }, + expectError: true, + errorMsg: "dangerously-disable-sandbox-agent", + }, + { + name: "sandbox.agent false with short justification", + data: &WorkflowData{ + Features: map[string]any{ + "dangerously-disable-sandbox-agent": "too short", + }, + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + Disabled: true, + }, + }, + }, + expectError: true, + errorMsg: "at least 20 characters", + }, + { + name: "sandbox.agent false with expression justification", + data: &WorkflowData{ + Features: map[string]any{ + "dangerously-disable-sandbox-agent": "${{ inputs.reason }}", + }, + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + Disabled: true, + }, + }, + }, + expectError: true, + errorMsg: "expressions", + }, } for _, tt := range tests { diff --git a/pkg/workflow/sandbox_validation.go b/pkg/workflow/sandbox_validation.go index c606fe276ac..22da1a5c43a 100644 --- a/pkg/workflow/sandbox_validation.go +++ b/pkg/workflow/sandbox_validation.go @@ -11,13 +11,20 @@ package workflow import ( + "errors" "fmt" + "regexp" + "strings" "github.com/github/gh-aw/pkg/constants" ) var sandboxValidationLog = newValidationLogger("sandbox") +const minSandboxDisableJustificationLength = 20 + +var githubActionsExpressionPattern = regexp.MustCompile(`\$\{\{[\s\S]*\}\}`) + // validateMountsSyntax validates that mount strings follow the correct syntax // Expected format: "source:destination:mode" where mode is either "ro" or "rw" func validateMountsSyntax(mounts []string) error { @@ -76,19 +83,21 @@ func validateSandboxConfig(workflowData *WorkflowData) error { sandboxConfig := workflowData.SandboxConfig // Check if sandbox.agent: false was specified - // This requires the "dangerously-disable-sandbox-agent" feature flag to be enabled. - // Without the feature flag, setting sandbox.agent: false is a validation error. + // This requires the "dangerously-disable-sandbox-agent" feature to include a + // justification string. Without a valid justification, disabling the sandbox + // is a validation error. if sandboxConfig.Agent != nil && sandboxConfig.Agent.Disabled { - if !isFeatureEnabled(constants.DangerouslyDisableSandboxAgentFeatureFlag, workflowData) { + justification, err := getSandboxDisableJustification(workflowData) + if err != nil { 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), + fmt.Sprintf("disabling the agent sandbox requires '%s' to be a justification string (%d+ chars, expressions not allowed): %v", flag, minSandboxDisableJustificationLength, err), + fmt.Sprintf("Add the feature value to your workflow frontmatter:\n\nfeatures:\n %s: \"controlled environment with no internet access\"\nsandbox:\n agent: false\n\nSee: %s", flag, constants.DocsSandboxURL), ) } - sandboxValidationLog.Printf("sandbox.agent: false permitted by %s feature flag", constants.DangerouslyDisableSandboxAgentFeatureFlag) + sandboxValidationLog.Printf("sandbox.agent: false permitted by %s justification: %q", constants.DangerouslyDisableSandboxAgentFeatureFlag, justification) } // Validate mounts syntax if specified in agent config @@ -136,3 +145,43 @@ func validateSandboxConfig(workflowData *WorkflowData) error { return nil } + +func getSandboxDisableJustification(workflowData *WorkflowData) (string, error) { + if workflowData == nil || workflowData.Features == nil { + return "", errors.New("dangerously-disable-sandbox-agent feature is missing") + } + + flagName := string(constants.DangerouslyDisableSandboxAgentFeatureFlag) + value, found := getFeatureValueCaseInsensitive(workflowData.Features, flagName) + if !found { + return "", errors.New("dangerously-disable-sandbox-agent feature is missing") + } + + justification, ok := value.(string) + if !ok { + return "", fmt.Errorf("feature must be a string, got %T", value) + } + + trimmed := strings.TrimSpace(justification) + if len(trimmed) < minSandboxDisableJustificationLength { + return "", fmt.Errorf("feature must be at least %d characters", minSandboxDisableJustificationLength) + } + + if githubActionsExpressionPattern.MatchString(trimmed) { + return "", errors.New("feature cannot use GitHub Actions expressions") + } + + return trimmed, nil +} + +func getFeatureValueCaseInsensitive(features map[string]any, flagName string) (any, bool) { + if value, exists := features[flagName]; exists { + return value, true + } + for key, value := range features { + if strings.EqualFold(key, flagName) { + return value, true + } + } + return nil, false +} diff --git a/pkg/workflow/workflow_run_validation_test.go b/pkg/workflow/workflow_run_validation_test.go index e200ba46d71..c6a25fbd359 100644 --- a/pkg/workflow/workflow_run_validation_test.go +++ b/pkg/workflow/workflow_run_validation_test.go @@ -37,7 +37,7 @@ on: tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false --- @@ -84,7 +84,7 @@ on: tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false --- @@ -191,7 +191,7 @@ on: tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false --- @@ -217,7 +217,7 @@ on: tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false --- @@ -242,7 +242,7 @@ on: tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false --- @@ -314,7 +314,7 @@ on: push tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false --- @@ -336,7 +336,7 @@ on: tools: github: false features: - dangerously-disable-sandbox-agent: true + dangerously-disable-sandbox-agent: "controlled environment with no internet access" sandbox: agent: false ---