diff --git a/.github/workflows/skillet.lock.yml b/.github/workflows/skillet.lock.yml index f846fac08b1..2009dcfaae5 100644 --- a/.github/workflows/skillet.lock.yml +++ b/.github/workflows/skillet.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"7b2d840cf8dce2c2bb688f7dab4a1623490d72f7c3e3f4dff947ec2764091cb5","body_hash":"e20efaa7adad4cff2d0b9e82062a586ed95a836a1c9453119e8d7ca909f3dded","strict":true,"agent_id":"copilot","engine_versions":{"copilot":"1.0.63"}} +# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"f7ca85b33fdc0e15d5aaa91aa776c97f24654da34aec81f1f6862bb62aeb7794","body_hash":"e20efaa7adad4cff2d0b9e82062a586ed95a836a1c9453119e8d7ca909f3dded","strict":true,"agent_id":"copilot","engine_versions":{"copilot":"1.0.63"}} # gh-aw-manifest: {"version":1,"secrets":["GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"27d5ce7f107fe9357f9df03efb73ab90386fccae","version":"v5.0.5"},{"repo":"actions/cache/save","sha":"27d5ce7f107fe9357f9df03efb73ab90386fccae","version":"v5.0.5"},{"repo":"actions/checkout","sha":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0","version":"v7.0.0"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"3a2844b7e9c422d3c10d287c895573f7108da1b3"},{"repo":"actions/setup-node","sha":"48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e","version":"v6.4.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.7","digest":"sha256:aae231e4635c8999d039c132f1602d3df850fe9b84a00aa2b5ac981179b5661c","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.7@sha256:aae231e4635c8999d039c132f1602d3df850fe9b84a00aa2b5ac981179b5661c"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.7","digest":"sha256:009caf2e3d88fa77b64e9a03a95a228fc58db0f1701c6d324b29ba5a3c7c79b6","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.7@sha256:009caf2e3d88fa77b64e9a03a95a228fc58db0f1701c6d324b29ba5a3c7c79b6"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.7","digest":"sha256:4757f198a3fa20f88bdbe70be7ae1a05f127d9c0a9e96a5d6460ef40c08fc83d","pinned_image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.7@sha256:4757f198a3fa20f88bdbe70be7ae1a05f127d9c0a9e96a5d6460ef40c08fc83d"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.7","digest":"sha256:deb1d4e19de62d51cee0508057a596a19315c3423ada4d675cad136dc8037c96","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.7@sha256:deb1d4e19de62d51cee0508057a596a19315c3423ada4d675cad136dc8037c96"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.3.27","digest":"sha256:fe984bddde4ec05d756d9043edb0a32912e6b7b72f6a121b1082f29221421cc7","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.3.27@sha256:fe984bddde4ec05d756d9043edb0a32912e6b7b72f6a121b1082f29221421cc7"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b","pinned_image":"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},{"image":"ghcr.io/github/github-mcp-server:v1.3.0","digest":"sha256:5c83359327a0bacc3d34db730bea6557d39d341cee0bf6c58c9a896e33150e80","pinned_image":"ghcr.io/github/github-mcp-server:v1.3.0@sha256:5c83359327a0bacc3d34db730bea6557d39d341cee0bf6c58c9a896e33150e80"}]} # This file was automatically generated by gh-aw. DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # @@ -44,7 +44,6 @@ # Custom actions used: # - actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 # - actions/cache/save@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 -# - actions/checkout # - actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 # - actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 # - actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 @@ -1685,7 +1684,7 @@ jobs: GH_AW_INFO_AWF_VERSION: "v0.27.7" GH_AW_INFO_ENGINE_ID: "copilot" - name: Checkout skills directory - uses: actions/checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: persist-credentials: false sparse-checkout: | diff --git a/.github/workflows/skillet.md b/.github/workflows/skillet.md index 4d015f077b0..627a2f61535 100644 --- a/.github/workflows/skillet.md +++ b/.github/workflows/skillet.md @@ -40,7 +40,7 @@ jobs: pre-activation: pre-steps: - name: Checkout skills directory - uses: actions/checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: sparse-checkout: | .github/skills diff --git a/pkg/workflow/action_pins.go b/pkg/workflow/action_pins.go index d1ff2ab83b7..b2cb64d41f1 100644 --- a/pkg/workflow/action_pins.go +++ b/pkg/workflow/action_pins.go @@ -1,6 +1,7 @@ package workflow import ( + "fmt" "strings" actionpins "github.com/github/gh-aw/pkg/actionpins" @@ -198,19 +199,29 @@ func getCachedActionPin(repo string, data *WorkflowData) string { // applyActionPinToTypedStep applies SHA pinning to a WorkflowStep if it uses an action. // Returns a modified copy of the step with pinned references. // If the step doesn't use an action or the action is not pinned, returns the original step. -func applyActionPinToTypedStep(step *WorkflowStep, data *WorkflowData) *WorkflowStep { +// Returns an error if the step uses an unversioned action reference with no available pin, +// because emitting such a reference would produce invalid GitHub Actions workflow syntax. +func applyActionPinToTypedStep(step *WorkflowStep, data *WorkflowData) (*WorkflowStep, error) { if step == nil || !step.IsUsesStep() { - return step + return step, nil } actionRepo := extractActionRepo(step.Uses) if actionRepo == "" { - return step + return step, nil } version := extractActionVersion(step.Uses) if version == "" { - return step + pin := getCachedActionPin(actionRepo, data) + if pin == "" { + return nil, fmt.Errorf("unversioned action %q has no available pin; add a @ref (e.g. @v1) or include it in action-pins.json", actionRepo) + } + + actionPinsLog.Printf("Pinned action: %s (no ref) -> %s", actionRepo, pin) + result := step.Clone() + result.Uses = pin + return result, nil } // Strip the comment suffix before checking if it's already a SHA. @@ -220,20 +231,21 @@ func applyActionPinToTypedStep(step *WorkflowStep, data *WorkflowData) *Workflow pinnedRef, err := getActionPinWithData(actionRepo, rawVersion, data) if err != nil || pinnedRef == "" { actionPinsLog.Printf("Skipping pin for %s@%s: no pin available", actionRepo, rawVersion) - return step + return step, nil } actionPinsLog.Printf("Pinned action: %s@%s -> %s", actionRepo, rawVersion, pinnedRef) result := step.Clone() result.Uses = pinnedRef - return result + return result, nil } // applyActionPinsToTypedSteps applies SHA pinning to a slice of typed WorkflowStep pointers. -// Returns a new slice with pinned references. -func applyActionPinsToTypedSteps(steps []*WorkflowStep, data *WorkflowData) []*WorkflowStep { +// Returns a new slice with pinned references, or an error if any step has an unversioned +// action reference with no available pin. +func applyActionPinsToTypedSteps(steps []*WorkflowStep, data *WorkflowData) ([]*WorkflowStep, error) { if steps == nil { - return nil + return nil, nil } result := make([]*WorkflowStep, 0, len(steps)) @@ -242,7 +254,11 @@ func applyActionPinsToTypedSteps(steps []*WorkflowStep, data *WorkflowData) []*W result = append(result, nil) continue } - result = append(result, applyActionPinToTypedStep(step, data)) + pinned, err := applyActionPinToTypedStep(step, data) + if err != nil { + return nil, err + } + result = append(result, pinned) } - return result + return result, nil } diff --git a/pkg/workflow/action_pins_test.go b/pkg/workflow/action_pins_test.go index 66e0a76f546..aa07da6b84a 100644 --- a/pkg/workflow/action_pins_test.go +++ b/pkg/workflow/action_pins_test.go @@ -15,6 +15,7 @@ import ( const setupNodeV6ExpectedUsesPlaceholder = "__setup_node_v6__" const checkoutV6ExpectedUsesPlaceholder = "__checkout_v6__" const checkoutSHAExpectedUsesPlaceholder = "__checkout_sha__" +const checkoutLatestExpectedUsesPlaceholder = "__checkout_latest__" func expectedPinnedUses(t *testing.T, repo, version string) string { t.Helper() @@ -29,6 +30,16 @@ func expectedPinnedUses(t *testing.T, repo, version string) string { return result } +func latestPinnedUsesForRepo(t *testing.T, repo string) string { + t.Helper() + + result := getCachedActionPin(repo, &WorkflowData{}) + if result == "" { + t.Fatalf("getCachedActionPin(%s) returned empty", repo) + } + return result +} + // TestGetActionPinFallback tests that getActionPin returns empty string for unknown actions func TestGetActionPinFallback(t *testing.T) { result := getActionPin("unknown/action") @@ -173,6 +184,15 @@ func TestApplyActionPinToStep(t *testing.T) { expectPinned: false, expectedUses: "my-org/my-action@v1", }, + { + name: "step with unversioned action", + stepMap: map[string]any{ + "name": "Checkout", + "uses": "actions/checkout", + }, + expectPinned: true, + expectedUses: checkoutLatestExpectedUsesPlaceholder, + }, { name: "step without uses field", stepMap: map[string]any{ @@ -205,7 +225,10 @@ func TestApplyActionPinToStep(t *testing.T) { } // Apply action pinning using typed version - pinnedStep := applyActionPinToTypedStep(typedStep, data) + pinnedStep, pinErr := applyActionPinToTypedStep(typedStep, data) + if pinErr != nil { + t.Fatalf("applyActionPinToTypedStep() returned error: %v", pinErr) + } if pinnedStep == nil { t.Fatal("applyActionPinToTypedStep returned nil") } @@ -231,6 +254,9 @@ func TestApplyActionPinToStep(t *testing.T) { if expectedUses == checkoutSHAExpectedUsesPlaceholder { expectedUses = expectedPinnedUses(t, "actions/checkout", "de0fac2e4500dabe0009e67214ff5f5447ce83dd") } + if expectedUses == checkoutLatestExpectedUsesPlaceholder { + expectedUses = latestPinnedUsesForRepo(t, "actions/checkout") + } if usesStr != expectedUses { t.Errorf("applyActionPinToTypedStep uses = %q, want %q", usesStr, expectedUses) } @@ -319,6 +345,7 @@ func TestApplyActionPinToTypedStep(t *testing.T) { step *WorkflowStep expectPinned bool expectedUses string + wantErr bool }{ { name: "step with pinned action (checkout)", @@ -350,6 +377,23 @@ func TestApplyActionPinToTypedStep(t *testing.T) { expectPinned: false, expectedUses: "my-org/my-action@v1", }, + { + name: "step with unversioned action in embedded pins", + step: &WorkflowStep{ + Name: "Checkout", + Uses: "actions/checkout", + }, + expectPinned: true, + expectedUses: checkoutLatestExpectedUsesPlaceholder, + }, + { + name: "step with unversioned action not in pins returns error", + step: &WorkflowStep{ + Name: "Custom action", + Uses: "my-org/my-unpinned-action", + }, + wantErr: true, + }, { name: "step without uses field", step: &WorkflowStep{ @@ -388,15 +432,29 @@ func TestApplyActionPinToTypedStep(t *testing.T) { // Create a test WorkflowData data := &WorkflowData{} - result := applyActionPinToTypedStep(tt.step, data) + result, err := applyActionPinToTypedStep(tt.step, data) if tt.step == nil { + if err != nil { + t.Errorf("applyActionPinToTypedStep(nil) returned error: %v", err) + } if result != nil { t.Errorf("applyActionPinToTypedStep(nil) = %v, want nil", result) } return } + if tt.wantErr { + if err == nil { + t.Errorf("applyActionPinToTypedStep() expected error, got nil") + } + return + } + + if err != nil { + t.Fatalf("applyActionPinToTypedStep() returned error: %v", err) + } + if result == nil { t.Fatalf("applyActionPinToTypedStep() returned nil") } @@ -409,6 +467,9 @@ func TestApplyActionPinToTypedStep(t *testing.T) { if expectedUses == checkoutV6ExpectedUsesPlaceholder { expectedUses = expectedPinnedUses(t, "actions/checkout", "v6") } + if expectedUses == checkoutLatestExpectedUsesPlaceholder { + expectedUses = latestPinnedUsesForRepo(t, "actions/checkout") + } if result.Uses != expectedUses { t.Errorf("applyActionPinToTypedStep() uses = %q, want %q", result.Uses, expectedUses) } @@ -453,7 +514,10 @@ func TestApplyActionPinToTypedStep_Immutability(t *testing.T) { originalUses := originalStep.Uses data := &WorkflowData{} - result := applyActionPinToTypedStep(originalStep, data) + result, err := applyActionPinToTypedStep(originalStep, data) + if err != nil { + t.Fatalf("applyActionPinToTypedStep() returned error: %v", err) + } // Verify the original step was not modified if originalStep.Uses != originalUses { @@ -649,9 +713,10 @@ func TestApplyActionPinsToTypedSteps(t *testing.T) { } tests := []struct { - name string - steps []*WorkflowStep - want []*WorkflowStep + name string + steps []*WorkflowStep + want []*WorkflowStep + wantErr bool }{ { name: "nil steps", @@ -725,11 +790,32 @@ func TestApplyActionPinsToTypedSteps(t *testing.T) { }, }, }, + { + name: "unversioned action not in pins returns error", + steps: []*WorkflowStep{ + { + Name: "Unknown action", + Uses: "my-org/my-unpinned-action", + }, + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := applyActionPinsToTypedSteps(tt.steps, data) + got, err := applyActionPinsToTypedSteps(tt.steps, data) + + if tt.wantErr { + if err == nil { + t.Errorf("applyActionPinsToTypedSteps() expected error, got nil") + } + return + } + + if err != nil { + t.Fatalf("applyActionPinsToTypedSteps() returned unexpected error: %v", err) + } if len(got) != len(tt.want) { t.Errorf("applyActionPinsToTypedSteps() returned %d steps, want %d", len(got), len(tt.want)) @@ -1224,7 +1310,10 @@ func TestMapToStepWithActionPinning(t *testing.T) { } // Apply action pinning using typed version - pinnedStep := applyActionPinToTypedStep(typedStep, data) + pinnedStep, pinErr := applyActionPinToTypedStep(typedStep, data) + if pinErr != nil { + t.Fatalf("applyActionPinToTypedStep() returned error: %v", pinErr) + } if pinnedStep == nil { t.Fatal("applyActionPinToTypedStep returned nil") } @@ -1328,7 +1417,10 @@ func TestSliceToStepsWithActionPinning(t *testing.T) { } // Apply action pinning using typed version - pinnedSteps := applyActionPinsToTypedSteps(typedSteps, data) + pinnedSteps, pinErr := applyActionPinsToTypedSteps(typedSteps, data) + if pinErr != nil { + t.Fatalf("applyActionPinsToTypedSteps() returned unexpected error: %v", pinErr) + } if len(pinnedSteps) != len(typedSteps) { t.Errorf("applyActionPinsToTypedSteps() returned %d steps, want %d", len(pinnedSteps), len(typedSteps)) } diff --git a/pkg/workflow/compiler_jobs.go b/pkg/workflow/compiler_jobs.go index c72493ea888..8bad50bbae0 100644 --- a/pkg/workflow/compiler_jobs.go +++ b/pkg/workflow/compiler_jobs.go @@ -1216,7 +1216,10 @@ func (c *Compiler) extractPinnedJobSteps(fieldName string, jobName string, confi return nil, fmt.Errorf("failed to convert %s to typed step for job '%s': %w", fieldName, jobName, err) } - pinnedStep := applyActionPinToTypedStep(typedStep, data) + pinnedStep, err := applyActionPinToTypedStep(typedStep, data) + if err != nil { + return nil, fmt.Errorf("failed to pin action for %s in job '%s': %w", fieldName, jobName, err) + } finalStepMap := pinnedStep.ToMap() sanitizedMap, warnings, _ := sanitizeRunStepExpressions(finalStepMap) for _, w := range warnings { diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 24e8f3eb81e..7f52317746b 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -196,10 +196,18 @@ func (c *Compiler) populateWorkflowBuildContext(ctx *workflowBuildContext) error if err := c.mergeImportedWorkflowConfiguration(ctx); err != nil { return err } - c.processAndMergeSteps(ctx.frontmatter.Frontmatter, ctx.workflowData, ctx.engineSetup.importsResult) - c.processAndMergePreSteps(ctx.frontmatter.Frontmatter, ctx.workflowData, ctx.engineSetup.importsResult) - c.processAndMergePreAgentSteps(ctx.frontmatter.Frontmatter, ctx.workflowData, ctx.engineSetup.importsResult) - c.processAndMergePostSteps(ctx.frontmatter.Frontmatter, ctx.workflowData, ctx.engineSetup.importsResult) + if err := c.processAndMergeSteps(ctx.frontmatter.Frontmatter, ctx.workflowData, ctx.engineSetup.importsResult); err != nil { + return formatCompilerError(ctx.cleanPath, "error", err.Error(), err) + } + if err := c.processAndMergePreSteps(ctx.frontmatter.Frontmatter, ctx.workflowData, ctx.engineSetup.importsResult); err != nil { + return formatCompilerError(ctx.cleanPath, "error", err.Error(), err) + } + if err := c.processAndMergePreAgentSteps(ctx.frontmatter.Frontmatter, ctx.workflowData, ctx.engineSetup.importsResult); err != nil { + return formatCompilerError(ctx.cleanPath, "error", err.Error(), err) + } + if err := c.processAndMergePostSteps(ctx.frontmatter.Frontmatter, ctx.workflowData, ctx.engineSetup.importsResult); err != nil { + return formatCompilerError(ctx.cleanPath, "error", err.Error(), err) + } c.processAndMergeServices(ctx.frontmatter.Frontmatter, ctx.workflowData, ctx.engineSetup.importsResult) ctx.workflowData.KnownActionCredentialEnvVars = DetectKnownCredentialLeakingActionsFromWorkflowData(ctx.workflowData) if err := c.extractAdditionalConfigurations(ctx.frontmatter.Frontmatter, ctx.toolsResult.tools, ctx.markdownDir, ctx.workflowData, ctx.engineSetup.importsResult, ctx.toolsResult.rawMainMarkdown, ctx.toolsResult.safeOutputs); err != nil { @@ -661,7 +669,10 @@ func (c *Compiler) processOnSectionAndFilters( } typedSteps, convErr := SliceToSteps(anySteps) if convErr == nil { - typedSteps = applyActionPinsToTypedSteps(typedSteps, workflowData) + typedSteps, convErr = applyActionPinsToTypedSteps(typedSteps, workflowData) + if convErr != nil { + return fmt.Errorf("on.steps: %w", convErr) + } for i, s := range typedSteps { onSteps[i] = s.ToMap() } diff --git a/pkg/workflow/compiler_orchestrator_workflow_test.go b/pkg/workflow/compiler_orchestrator_workflow_test.go index 7f05a479726..0864d183647 100644 --- a/pkg/workflow/compiler_orchestrator_workflow_test.go +++ b/pkg/workflow/compiler_orchestrator_workflow_test.go @@ -431,7 +431,7 @@ func TestProcessAndMergeSteps_NoSteps(t *testing.T) { frontmatter := map[string]any{} importsResult := &parser.ImportsResult{} - compiler.processAndMergeSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergeSteps(frontmatter, workflowData, importsResult)) // CustomSteps should be empty when no steps are defined assert.Empty(t, workflowData.CustomSteps) @@ -458,7 +458,7 @@ func TestProcessAndMergeSteps_MainStepsOnly(t *testing.T) { } importsResult := &parser.ImportsResult{} - compiler.processAndMergeSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergeSteps(frontmatter, workflowData, importsResult)) // CustomSteps should contain the main workflow steps assert.NotEmpty(t, workflowData.CustomSteps) @@ -499,7 +499,7 @@ func TestProcessAndMergeSteps_WithImportedSteps(t *testing.T) { MergedSteps: string(importedStepsYAML), } - compiler.processAndMergeSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergeSteps(frontmatter, workflowData, importsResult)) // CustomSteps should contain both imported and main steps assert.NotEmpty(t, workflowData.CustomSteps) @@ -544,7 +544,7 @@ func TestProcessAndMergeSteps_WithCopilotSetupSteps(t *testing.T) { CopilotSetupSteps: string(copilotSetupYAML), } - compiler.processAndMergeSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergeSteps(frontmatter, workflowData, importsResult)) // CustomSteps should contain both copilot-setup and main steps assert.NotEmpty(t, workflowData.CustomSteps) @@ -592,7 +592,7 @@ func TestProcessAndMergeSteps_AllStepTypes(t *testing.T) { MergedSteps: string(otherStepsYAML), } - compiler.processAndMergeSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergeSteps(frontmatter, workflowData, importsResult)) // All steps should be present assert.Contains(t, workflowData.CustomSteps, "Copilot setup") @@ -615,7 +615,7 @@ func TestProcessAndMergePostSteps_NoPostSteps(t *testing.T) { frontmatter := map[string]any{} importsResult := &parser.ImportsResult{} - compiler.processAndMergePostSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergePostSteps(frontmatter, workflowData, importsResult)) assert.Empty(t, workflowData.PostSteps) } @@ -645,7 +645,7 @@ func TestProcessAndMergePostSteps_WithPostSteps(t *testing.T) { } importsResult := &parser.ImportsResult{} - compiler.processAndMergePostSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergePostSteps(frontmatter, workflowData, importsResult)) assert.NotEmpty(t, workflowData.PostSteps) assert.Contains(t, workflowData.PostSteps, "Cleanup") @@ -671,7 +671,7 @@ func TestProcessAndMergePostSteps_WithImportedPostSteps(t *testing.T) { MergedPostSteps: string(importedPostStepsYAML), } - compiler.processAndMergePostSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergePostSteps(frontmatter, workflowData, importsResult)) assert.Contains(t, workflowData.PostSteps, "Main post step") assert.Contains(t, workflowData.PostSteps, "Imported post step") @@ -689,7 +689,7 @@ func TestProcessAndMergePreSteps_NoPreSteps(t *testing.T) { frontmatter := map[string]any{} importsResult := &parser.ImportsResult{} - compiler.processAndMergePreSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergePreSteps(frontmatter, workflowData, importsResult)) assert.Empty(t, workflowData.PreSteps) } @@ -706,7 +706,7 @@ func TestProcessAndMergePreSteps_WithPreSteps(t *testing.T) { } importsResult := &parser.ImportsResult{} - compiler.processAndMergePreSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergePreSteps(frontmatter, workflowData, importsResult)) assert.NotEmpty(t, workflowData.PreSteps) assert.Contains(t, workflowData.PreSteps, "Mint token") @@ -731,7 +731,7 @@ func TestProcessAndMergePreSteps_WithImportedPreSteps(t *testing.T) { MergedPreSteps: string(importedPreStepsYAML), } - compiler.processAndMergePreSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergePreSteps(frontmatter, workflowData, importsResult)) assert.Contains(t, workflowData.PreSteps, "Main pre step") assert.Contains(t, workflowData.PreSteps, "Imported pre step") @@ -749,7 +749,7 @@ func TestProcessAndMergePreAgentSteps_NoPreAgentSteps(t *testing.T) { frontmatter := map[string]any{} importsResult := &parser.ImportsResult{} - compiler.processAndMergePreAgentSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergePreAgentSteps(frontmatter, workflowData, importsResult)) assert.Empty(t, workflowData.PreAgentSteps) } @@ -766,7 +766,7 @@ func TestProcessAndMergePreAgentSteps_WithPreAgentSteps(t *testing.T) { } importsResult := &parser.ImportsResult{} - compiler.processAndMergePreAgentSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergePreAgentSteps(frontmatter, workflowData, importsResult)) assert.NotEmpty(t, workflowData.PreAgentSteps) assert.Contains(t, workflowData.PreAgentSteps, "Prepare final context") @@ -791,7 +791,7 @@ func TestProcessAndMergePreAgentSteps_WithImportedPreAgentSteps(t *testing.T) { MergedPreAgentSteps: string(importedPreAgentStepsYAML), } - compiler.processAndMergePreAgentSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergePreAgentSteps(frontmatter, workflowData, importsResult)) assert.Contains(t, workflowData.PreAgentSteps, "Main pre-agent step") assert.Contains(t, workflowData.PreAgentSteps, "Imported pre-agent step") @@ -1830,7 +1830,7 @@ func TestProcessAndMergeSteps_InvalidYAML(t *testing.T) { } // Should handle gracefully without panicking - compiler.processAndMergeSteps(frontmatter, workflowData, importsResult) + require.NoError(t, compiler.processAndMergeSteps(frontmatter, workflowData, importsResult)) // Should still have main steps assert.NotEmpty(t, workflowData.CustomSteps) diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index a8d1064b8d0..89afb024111 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -152,7 +152,10 @@ func (c *Compiler) buildSafeOutputsSetupAndDownloadSteps(data *WorkflowData, age if err != nil { return nil, fmt.Errorf("failed to convert safe-outputs step at index %d to typed step: %w", i, err) } - pinnedStep := applyActionPinToTypedStep(typedStep, data) + pinnedStep, err := applyActionPinToTypedStep(typedStep, data) + if err != nil { + return nil, fmt.Errorf("failed to pin action for safe-outputs step at index %d: %w", i, err) + } stepYAML, err := ConvertStepToYAML(pinnedStep.ToMap()) if err != nil { return nil, fmt.Errorf("failed to convert safe-outputs step at index %d to YAML: %w", i, err) diff --git a/pkg/workflow/compiler_string_api.go b/pkg/workflow/compiler_string_api.go index 7de6fe7caa7..311538f0a9d 100644 --- a/pkg/workflow/compiler_string_api.go +++ b/pkg/workflow/compiler_string_api.go @@ -204,7 +204,9 @@ func (c *Compiler) ParseWorkflowString(content string, virtualPath string) (*Wor } // Process and merge custom steps - c.processAndMergeSteps(parseResult.frontmatterResult.Frontmatter, workflowData, engineSetup.importsResult) + if err := c.processAndMergeSteps(parseResult.frontmatterResult.Frontmatter, workflowData, engineSetup.importsResult); err != nil { + return nil, err + } // Apply defaults if err := c.applyDefaults(workflowData, cleanPath); err != nil { diff --git a/pkg/workflow/safe_jobs.go b/pkg/workflow/safe_jobs.go index d2fc4f09b31..0ea1881b41f 100644 --- a/pkg/workflow/safe_jobs.go +++ b/pkg/workflow/safe_jobs.go @@ -297,7 +297,10 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool } // Apply action pinning using type-safe version - pinnedStep := applyActionPinToTypedStep(typedStep, data) + pinnedStep, err := applyActionPinToTypedStep(typedStep, data) + if err != nil { + return nil, fmt.Errorf("failed to pin action for step in safe job %s: %w", normalizedJobName, err) + } // Convert back to map for YAML generation stepYAML, err := ConvertStepToYAML(pinnedStep.ToMap()) diff --git a/pkg/workflow/workflow_builder.go b/pkg/workflow/workflow_builder.go index 86bd89b2905..fc511b9082c 100644 --- a/pkg/workflow/workflow_builder.go +++ b/pkg/workflow/workflow_builder.go @@ -2,6 +2,7 @@ package workflow import ( "encoding/json" + "fmt" "maps" "strings" @@ -382,7 +383,7 @@ func extractDispatchItemNumber(frontmatter map[string]any) bool { } // processAndMergeSteps handles the merging of imported steps with main workflow steps -func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { +func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) error { workflowBuilderLog.Print("Processing and merging custom steps") workflowData.CustomSteps = c.extractTopLevelYAMLSection(frontmatter, "steps") @@ -399,7 +400,10 @@ func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData workflowBuilderLog.Printf("Failed to convert copilot-setup steps to typed steps: %v", err) } else { // Apply action pinning to copilot-setup steps - typedCopilotSteps = applyActionPinsToTypedSteps(typedCopilotSteps, workflowData) + typedCopilotSteps, err = applyActionPinsToTypedSteps(typedCopilotSteps, workflowData) + if err != nil { + return fmt.Errorf("copilot-setup steps: %w", err) + } // Convert back to []any for YAML marshaling copilotSetupSteps = StepsToSlice(typedCopilotSteps) } @@ -416,7 +420,10 @@ func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData workflowBuilderLog.Printf("Failed to convert other imported steps to typed steps: %v", err) } else { // Apply action pinning to other imported steps - typedOtherSteps = applyActionPinsToTypedSteps(typedOtherSteps, workflowData) + typedOtherSteps, err = applyActionPinsToTypedSteps(typedOtherSteps, workflowData) + if err != nil { + return fmt.Errorf("imported steps: %w", err) + } // Convert back to []any for YAML marshaling otherImportedSteps = StepsToSlice(typedOtherSteps) } @@ -437,7 +444,10 @@ func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData workflowBuilderLog.Printf("Failed to convert main steps to typed steps: %v", err) } else { // Apply action pinning to main steps - typedMainSteps = applyActionPinsToTypedSteps(typedMainSteps, workflowData) + typedMainSteps, err = applyActionPinsToTypedSteps(typedMainSteps, workflowData) + if err != nil { + return fmt.Errorf("steps: %w", err) + } // Convert back to []any for YAML marshaling mainSteps = StepsToSlice(typedMainSteps) } @@ -464,6 +474,7 @@ func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData workflowData.CustomSteps = unquoteUsesWithComments(string(stepsYAML)) } } + return nil } // processAndMergePreSteps handles the processing and merging of pre-steps with action pinning. @@ -471,7 +482,7 @@ func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData // built-in steps, allowing users to mint tokens or perform other setup that must happen // before the repository is checked out. Imported pre-steps are merged before the main // workflow's pre-steps so that the main workflow can override or extend the imports. -func (c *Compiler) processAndMergePreSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { +func (c *Compiler) processAndMergePreSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) error { workflowBuilderLog.Print("Processing and merging pre-steps") mainPreStepsYAML := c.extractTopLevelYAMLSection(frontmatter, "pre-steps") @@ -486,7 +497,10 @@ func (c *Compiler) processAndMergePreSteps(frontmatter map[string]any, workflowD if err != nil { workflowBuilderLog.Printf("Failed to convert imported pre-steps to typed steps: %v", err) } else { - typedImported = applyActionPinsToTypedSteps(typedImported, workflowData) + typedImported, err = applyActionPinsToTypedSteps(typedImported, workflowData) + if err != nil { + return fmt.Errorf("imported pre-steps: %w", err) + } importedPreSteps = StepsToSlice(typedImported) } } @@ -504,7 +518,10 @@ func (c *Compiler) processAndMergePreSteps(frontmatter map[string]any, workflowD if err != nil { workflowBuilderLog.Printf("Failed to convert main pre-steps to typed steps: %v", err) } else { - typedMain = applyActionPinsToTypedSteps(typedMain, workflowData) + typedMain, err = applyActionPinsToTypedSteps(typedMain, workflowData) + if err != nil { + return fmt.Errorf("pre-steps: %w", err) + } mainPreSteps = StepsToSlice(typedMain) } } @@ -524,11 +541,12 @@ func (c *Compiler) processAndMergePreSteps(frontmatter map[string]any, workflowD workflowData.PreSteps = unquoteUsesWithComments(string(stepsYAML)) } } + return nil } // processAndMergePreAgentSteps handles processing and merging of pre-agent-steps with action pinning. // Imported pre-agent-steps are prepended so main workflow pre-agent-steps run last. -func (c *Compiler) processAndMergePreAgentSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { +func (c *Compiler) processAndMergePreAgentSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) error { workflowBuilderLog.Print("Processing and merging pre-agent-steps") mainPreAgentStepsYAML := c.extractTopLevelYAMLSection(frontmatter, "pre-agent-steps") @@ -542,7 +560,10 @@ func (c *Compiler) processAndMergePreAgentSteps(frontmatter map[string]any, work if err != nil { workflowBuilderLog.Printf("Failed to convert imported pre-agent-steps to typed steps: %v", err) } else { - typedImported = applyActionPinsToTypedSteps(typedImported, workflowData) + typedImported, err = applyActionPinsToTypedSteps(typedImported, workflowData) + if err != nil { + return fmt.Errorf("imported pre-agent-steps: %w", err) + } importedPreAgentSteps = StepsToSlice(typedImported) } } @@ -559,7 +580,10 @@ func (c *Compiler) processAndMergePreAgentSteps(frontmatter map[string]any, work if err != nil { workflowBuilderLog.Printf("Failed to convert main pre-agent-steps to typed steps: %v", err) } else { - typedMain = applyActionPinsToTypedSteps(typedMain, workflowData) + typedMain, err = applyActionPinsToTypedSteps(typedMain, workflowData) + if err != nil { + return fmt.Errorf("pre-agent-steps: %w", err) + } mainPreAgentSteps = StepsToSlice(typedMain) } } @@ -578,11 +602,12 @@ func (c *Compiler) processAndMergePreAgentSteps(frontmatter map[string]any, work workflowData.PreAgentSteps = unquoteUsesWithComments(string(stepsYAML)) } } + return nil } // processAndMergePostSteps handles the processing and merging of post-steps with action pinning. // Imported post-steps are appended after the main workflow's post-steps. -func (c *Compiler) processAndMergePostSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { +func (c *Compiler) processAndMergePostSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) error { workflowBuilderLog.Print("Processing and merging post-steps") mainPostStepsYAML := c.extractTopLevelYAMLSection(frontmatter, "post-steps") @@ -597,7 +622,10 @@ func (c *Compiler) processAndMergePostSteps(frontmatter map[string]any, workflow if err != nil { workflowBuilderLog.Printf("Failed to convert imported post-steps to typed steps: %v", err) } else { - typedImported = applyActionPinsToTypedSteps(typedImported, workflowData) + typedImported, err = applyActionPinsToTypedSteps(typedImported, workflowData) + if err != nil { + return fmt.Errorf("imported post-steps: %w", err) + } importedPostSteps = StepsToSlice(typedImported) } } @@ -615,7 +643,10 @@ func (c *Compiler) processAndMergePostSteps(frontmatter map[string]any, workflow if err != nil { workflowBuilderLog.Printf("Failed to convert main post-steps to typed steps: %v", err) } else { - typedMain = applyActionPinsToTypedSteps(typedMain, workflowData) + typedMain, err = applyActionPinsToTypedSteps(typedMain, workflowData) + if err != nil { + return fmt.Errorf("post-steps: %w", err) + } mainPostSteps = StepsToSlice(typedMain) } } @@ -635,4 +666,5 @@ func (c *Compiler) processAndMergePostSteps(frontmatter map[string]any, workflow workflowData.PostSteps = unquoteUsesWithComments(string(stepsYAML)) } } + return nil }