diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index 65a6e426e7b..b8d08739885 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -261,10 +261,14 @@ func GenerateMultiSecretValidationStep(secretNames []string, engineName, docsURL // Returns: // - GitHubActionStep: The validation step, or an empty step if a custom command is set func BuildDefaultSecretValidationStep(workflowData *WorkflowData, secrets []string, name, docsURL string) GitHubActionStep { - if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" { + if workflowData != nil && workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" { engineHelpersLog.Printf("Skipping secret validation step: custom command specified (%s)", workflowData.EngineConfig.Command) return GitHubActionStep{} } + if workflowData != nil && strings.TrimSpace(workflowData.Environment) != "" { + engineHelpersLog.Print("Skipping secret validation step: top-level environment is configured") + return GitHubActionStep{} + } return GenerateMultiSecretValidationStep(secrets, name, docsURL, getEngineEnvOverrides(workflowData)) } diff --git a/pkg/workflow/secret_validation_test.go b/pkg/workflow/secret_validation_test.go index c3d7d9a6940..c658a1baba6 100644 --- a/pkg/workflow/secret_validation_test.go +++ b/pkg/workflow/secret_validation_test.go @@ -6,6 +6,9 @@ import ( "fmt" "strings" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGenerateMultiSecretValidationStep(t *testing.T) { @@ -280,3 +283,54 @@ func TestValidationStepUsesEngineEnvOverride(t *testing.T) { }) } } + +func TestEngineSecretValidationSkippedWhenEnvironmentConfigured(t *testing.T) { + tests := []struct { + name string + engine CodingAgentEngine + }{ + { + name: "copilot engine skips validation with environment", + engine: NewCopilotEngine(), + }, + { + name: "claude engine skips validation with environment", + engine: NewClaudeEngine(), + }, + { + name: "codex engine skips validation with environment", + engine: NewCodexEngine(), + }, + { + name: "gemini engine skips validation with environment", + engine: NewGeminiEngine(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + workflowData := &WorkflowData{ + Environment: "production", + } + + steps := tt.engine.GetSecretValidationStep(workflowData) + if len(steps) != 0 { + t.Fatalf("expected secret validation step to be skipped when environment is configured, got:\n%s", strings.Join(steps, "\n")) + } + }) + } +} + +func TestBuildDefaultSecretValidationStepHandlesNilWorkflowData(t *testing.T) { + step := BuildDefaultSecretValidationStep( + nil, + []string{"COPILOT_GITHUB_TOKEN"}, + "GitHub Copilot CLI", + "https://github.github.com/gh-aw/reference/engines/#github-copilot-default", + ) + + require.NotEmpty(t, step, "expected non-empty validation step for nil workflowData") + + stepContent := strings.Join(step, "\n") + assert.Contains(t, stepContent, "Validate COPILOT_GITHUB_TOKEN secret", "expected validation step to include COPILOT_GITHUB_TOKEN check") +} diff --git a/pkg/workflow/secret_verification_output_test.go b/pkg/workflow/secret_verification_output_test.go index bdf7fb55c3e..46ac0e5b5ae 100644 --- a/pkg/workflow/secret_verification_output_test.go +++ b/pkg/workflow/secret_verification_output_test.go @@ -91,3 +91,42 @@ Test workflow` t.Error("Expected conclusion job to receive secret_verification_result from activation job") } } + +func TestSecretVerificationOutputSkippedWithEnvironment(t *testing.T) { + testDir := testutil.TempDir(t, "secret-verify-env-*") + workflowFile := filepath.Join(testDir, "test-workflow.md") + + workflow := `--- +on: workflow_dispatch +engine: copilot +environment: production +--- + +Test workflow` + + if err := os.WriteFile(workflowFile, []byte(workflow), 0o644); err != nil { + t.Fatalf("Failed to write test workflow: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowFile); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockFile := stringutil.MarkdownToLockFile(workflowFile) + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + lockStr := string(lockContent) + if !strings.Contains(lockStr, "environment: production") { + t.Error("Expected compiled workflow to include environment: production") + } + if strings.Contains(lockStr, "id: validate-secret") { + t.Error("Expected validate-secret step to be skipped when top-level environment is configured") + } + if strings.Contains(lockStr, "secret_verification_result: ${{ steps.validate-secret.outputs.verification_result }}") { + t.Error("Expected secret_verification_result activation output to be skipped when top-level environment is configured") + } +}