Skip to content

fix: SHA-pin the runtime setup-cli step emitted for custom steps: workflows #38344

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 3 commits into from
Jun 10, 2026
Merged

fix: SHA-pin the runtime setup-cli step emitted for custom steps: workflows #38344

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
101aaec
Initial plan
Copilot Jun 10, 2026
8019bb8
fix: thread WorkflowData into runtime setup-cli step for SHA-pinned a…
Copilot Jun 10, 2026
184476d
test: cover WorkflowData pin path for gh-aw setup step
Copilot Jun 10, 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
2 changes: 1 addition & 1 deletion pkg/workflow/compiler_yaml_main_job.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func (c *Compiler) generateRuntimeAndWorkspaceSetupSteps(yaml *strings.Builder,
}

// Generate runtime setup steps (after filtering out user-customized ones)
runtimeSetupSteps := GenerateRuntimeSetupSteps(runtimeRequirements)
runtimeSetupSteps := GenerateRuntimeSetupSteps(runtimeRequirements, data)
compilerYamlLog.Printf("Detected runtime requirements: %d runtimes, %d setup steps", len(runtimeRequirements), len(runtimeSetupSteps))

// Decision logic for where to place runtime steps:
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/runtime_definitions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ var knownRuntimes = []*Runtime{
ID: "gh-aw",
Name: "gh-aw CLI",
ActionRepo: "github/gh-aw/actions/setup-cli",
ActionVersion: "v0.72.1",
ActionVersion: "", // Derived at compile time from the compiler version; see runtime_step_generator.go
VersionField: "version",
// Default version is computed at generation time from the current gh-aw build.
DefaultVersion: "",
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/runtime_gh_aw_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestGenerateRuntimeSetupSteps_GhAw_DevBuildsFromSource(t *testing.T) {
steps := GenerateRuntimeSetupSteps([]RuntimeRequirement{{
Runtime: ghAwRuntime,
Version: "",
}})
}}, nil)
require.NotEmpty(t, steps)

content := strings.Join(steps[0], "\n")
Expand Down Expand Up @@ -134,7 +134,7 @@ func TestGenerateRuntimeSetupSteps_GhAw_ReleaseUsesSetupCLI(t *testing.T) {
steps := GenerateRuntimeSetupSteps([]RuntimeRequirement{{
Runtime: ghAwRuntime,
Version: "",
}})
}}, nil)

@github-actions github-actions Bot Jun 10, 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.

The actual bug fix has zero test coverage: every call to GenerateRuntimeSetupSteps in this file passes nil, so resolveGhAwSetupActionRef never exercises the workflowData != nil branch that performs SHA-pinning via getActionPinWithData.

💡 Suggested addition

Add a test that passes a *WorkflowData with a mock resolver that returns a known SHA for setup-cli@version and asserts the generated step uses the SHA-pinned ref. Without this, a regression in the lock-file pin path — the security-relevant code added by this PR — would go undetected by the test suite.

require.NotEmpty(t, steps)

content := strings.Join(steps[0], "\n")
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/runtime_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ func TestRuntimeIfConditionIntegration(t *testing.T) {
reqSlice = append(reqSlice, *req)
}

steps := GenerateRuntimeSetupSteps(reqSlice)
steps := GenerateRuntimeSetupSteps(reqSlice, nil)
allSteps := ""
for _, step := range steps {
for _, line := range step {
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/runtime_setup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ func TestGenerateRuntimeSetupSteps(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
steps := GenerateRuntimeSetupSteps(tt.requirements)
steps := GenerateRuntimeSetupSteps(tt.requirements, nil)

if len(steps) != tt.expectSteps {
t.Errorf("Expected %d steps, got %d", tt.expectSteps, len(steps))
Expand Down Expand Up @@ -917,7 +917,7 @@ func TestGenerateRuntimeSetupStepsWithIfCondition(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
steps := GenerateRuntimeSetupSteps(tt.requirements)
steps := GenerateRuntimeSetupSteps(tt.requirements, nil)

if len(steps) != tt.expectSteps {
t.Errorf("Expected %d steps, got %d", tt.expectSteps, len(steps))
Expand Down
15 changes: 10 additions & 5 deletions pkg/workflow/runtime_step_generator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,18 @@ import (

var runtimeStepGeneratorLog = logger.New("workflow:runtime_step_generator")

// GenerateRuntimeSetupSteps creates GitHub Actions steps for runtime setup
func GenerateRuntimeSetupSteps(requirements []RuntimeRequirement) []GitHubActionStep {
// GenerateRuntimeSetupSteps creates GitHub Actions steps for runtime setup.
// data is the WorkflowData for the workflow being compiled; it is forwarded to
// generateSetupStep so that the gh-aw setup-cli step can use the lock-file-aware
// pin resolver (getActionPinWithData) rather than the static embedded-pins fallback.
// Pass nil when WorkflowData is not available (e.g. in tests).

@github-actions github-actions Bot Jun 10, 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.

Doc comment actively discourages exercising the new path: "Pass nil when WorkflowData is not available (e.g. in tests)" directly caused every test author in this PR to skip the getActionPinWithData code path. That guidance is the root cause of the test coverage gap.

💡 Suggested replacement 
// data is the WorkflowData for the workflow being compiled; pass nil only in
// tests that specifically target non-gh-aw runtimes. Tests for the gh-aw
// setup-cli step should pass a populated *WorkflowData to exercise the
// lock-file pin path.

Or, if nil must be supported for breadth-of-test reasons, add a nil-guard that makes the degraded behaviour explicit rather than silently falling back to the static-pin path.

func GenerateRuntimeSetupSteps(requirements []RuntimeRequirement, data *WorkflowData) []GitHubActionStep {
runtimeStepGeneratorLog.Printf("Generating runtime setup steps: requirement_count=%d", len(requirements))
runtimeSetupLog.Printf("Generating runtime setup steps for %d requirements", len(requirements))
var steps []GitHubActionStep

for _, req := range requirements {
steps = append(steps, generateSetupStep(&req))
steps = append(steps, generateSetupStep(&req, data))
Comment on lines 24 to +25

// Add environment variable capture steps after setup actions for AWF chroot mode.
// Most env vars are inherited via AWF_HOST_PATH, but Go is special.
Expand Down Expand Up @@ -50,7 +54,7 @@ func generateEnvCaptureStep(envVar string, captureCmd string) GitHubActionStep {
}

// generateSetupStep creates a setup step for a given runtime requirement
func generateSetupStep(req *RuntimeRequirement) GitHubActionStep {
func generateSetupStep(req *RuntimeRequirement, data *WorkflowData) GitHubActionStep {
runtime := req.Runtime
version := req.Version
runtimeStepGeneratorLog.Printf("Generating setup step for runtime: %s, version=%s, if=%s", runtime.ID, version, req.IfCondition)
Expand All @@ -74,7 +78,8 @@ func generateSetupStep(req *RuntimeRequirement) GitHubActionStep {
ifCondition: req.IfCondition,
cliVersion: version,
actionRepo: runtime.ActionRepo,
fallbackActionRefTag: runtime.ActionVersion,
fallbackActionRefTag: version,

@github-actions github-actions Bot Jun 10, 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.

fallbackActionRefTag is silently unused when workflowData != nil: in resolveGhAwSetupActionRef, the non-nil branch builds its fallback as fmt.Sprintf("%s@%s", config.actionRepo, config.cliVersion) — fallbackActionRefTag is never read there. Only the workflowData == nil branch (static pin path) uses it.

💡 Why this matters

Today cliVersion == fallbackActionRefTag == version at the only call site, so there is no observable bug. But the struct API is a trap: a future caller (or a test) that sets fallbackActionRefTag to something different from cliVersion expecting it to govern the "no pin found" fallback in the non-nil path would see it silently ignored, and the step would still emit repo@cliVersion instead. Consider renaming the field to noDataFallbackActionRefTag or adding a comment to resolveGhAwSetupActionRef that makes the scoping explicit.

workflowData: data,
withFields: allExtraFields,
Comment on lines 80 to 84
})
if err != nil {
Expand Down
Loading