Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions .changeset/patch-skip-sha-warnings.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions .github/workflows/daily-fact.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions .github/workflows/playground-snapshots-refresh.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 33 additions & 7 deletions pkg/workflow/action_pins.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,12 @@ func GetActionPin(actionRepo string) string {
// The returned reference includes a comment with the version tag (e.g., "repo@sha # v1")
func GetActionPinWithData(actionRepo, version string, data *WorkflowData) (string, error) {
actionPinsLog.Printf("Resolving action pin: repo=%s, version=%s, strict_mode=%t", actionRepo, version, data.StrictMode)
// First try dynamic resolution if resolver is available
if data.ActionResolver != nil {

// Check if version is already a full 40-character SHA
isAlreadySHA := isValidFullSHA(version)

// First try dynamic resolution if resolver is available (but not for SHAs, as they can't be resolved)
if data.ActionResolver != nil && !isAlreadySHA {
actionPinsLog.Printf("Attempting dynamic resolution for %s@%s", actionRepo, version)
sha, err := data.ActionResolver.ResolveSHA(actionRepo, version)
if err == nil && sha != "" {
Expand Down Expand Up @@ -168,14 +172,27 @@ func GetActionPinWithData(actionRepo, version string, data *WorkflowData) (strin
// Sort matching pins by version (descending - highest first)
sortPinsByVersion(matchingPins)

// First, try to find an exact version match
// First, try to find an exact version match (for version tags)
for _, pin := range matchingPins {
if pin.Version == version {
actionPinsLog.Printf("Exact version match: requested=%s, found=%s", version, pin.Version)
return actionRepo + "@" + pin.SHA + " # " + pin.Version, nil
}
}

// If version is a SHA, check if it matches any hardcoded pin's SHA
if isAlreadySHA {
for _, pin := range matchingPins {
if pin.SHA == version {
actionPinsLog.Printf("Exact SHA match: requested=%s, found version=%s", version, pin.Version)
return actionRepo + "@" + pin.SHA + " # " + pin.Version, nil
}
}
// SHA provided but doesn't match any hardcoded pin - return it as-is without warnings
actionPinsLog.Printf("SHA %s not found in hardcoded pins, returning as-is", version)
return actionRepo + "@" + version + " # " + version, nil
}

// No exact match found
// In non-strict mode, use the first pin we found (which is the highest due to sorting above)
// This matches the behavior of GetActionPin and GetActionPinByRepo, which return
Expand All @@ -185,9 +202,12 @@ func GetActionPinWithData(actionRepo, version string, data *WorkflowData) (strin
if !data.StrictMode && len(matchingPins) > 0 {
highestPin := matchingPins[0]
actionPinsLog.Printf("No exact match for version %s, using highest available: %s", version, highestPin.Version)
warningMsg := fmt.Sprintf("Unable to resolve %s@%s dynamically, using hardcoded pin for %s@%s",
actionRepo, version, actionRepo, highestPin.Version)
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warningMsg))
// Only emit warning if the version is not a SHA (SHAs shouldn't generate warnings)
if !isAlreadySHA {
warningMsg := fmt.Sprintf("Unable to resolve %s@%s dynamically, using hardcoded pin for %s@%s",
actionRepo, version, actionRepo, highestPin.Version)
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warningMsg))
}
actionPinsLog.Printf("Using highest version in non-strict mode: %s@%s (requested) → %s@%s (used)",
actionRepo, version, actionRepo, highestPin.Version)
return actionRepo + "@" + highestPin.SHA + " # " + highestPin.Version, nil
Expand All @@ -204,7 +224,13 @@ func GetActionPinWithData(actionRepo, version string, data *WorkflowData) (strin
return "", fmt.Errorf("%s", errMsg)
}

// In non-strict mode, emit warning and return empty string
// In non-strict mode, emit warning and return empty string (unless it's already a SHA)
if isAlreadySHA {
// If version is already a SHA and we couldn't find it in pins, return it as-is without warnings
actionPinsLog.Printf("SHA %s not found in hardcoded pins, returning as-is", version)
return actionRepo + "@" + version + " # " + version, nil
}

warningMsg := fmt.Sprintf("Unable to pin action %s@%s", actionRepo, version)
if data.ActionResolver != nil {
warningMsg = fmt.Sprintf("Unable to pin action %s@%s: resolution failed", actionRepo, version)
Expand Down
79 changes: 79 additions & 0 deletions pkg/workflow/action_pins_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package workflow

import (
"bytes"
"os"
"os/exec"
"regexp"
"strings"
Expand Down Expand Up @@ -720,6 +722,83 @@ func TestGetActionPinWithData_SemverPreference(t *testing.T) {
}
}

// TestGetActionPinWithData_AlreadySHA tests that no warnings are issued when
// the version is already a full 40-character SHA
func TestGetActionPinWithData_AlreadySHA(t *testing.T) {
tests := []struct {
name string
repo string
sha string
expectError bool
}{
{
name: "actions/checkout with full SHA",
repo: "actions/checkout",
sha: "93cb6efe18208431cddfb9bfd000000000000000", // 40-char SHA
},
{
name: "actions/setup-node with full SHA",
repo: "actions/setup-node",
sha: "395ad3262231945c25e8478fd5baf05154b1d79f", // 40-char SHA from the issue
},
{
name: "different action with full SHA",
repo: "actions/upload-artifact",
sha: "1234567890abcdef1234567890abcdef12345678", // 40-char SHA
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
data := &WorkflowData{
StrictMode: false,
}

// Capture stderr to verify no warnings are issued
oldStderr := os.Stderr
r, w, _ := os.Pipe()
os.Stderr = w

result, err := GetActionPinWithData(tt.repo, tt.sha, data)

w.Close()
os.Stderr = oldStderr

var buf bytes.Buffer
buf.ReadFrom(r)
stderr := buf.String()

// Should not error for full SHAs
if err != nil {
t.Errorf("GetActionPinWithData() unexpected error = %v", err)
return
}

// Should return the SHA as-is
if result == "" {
t.Errorf("GetActionPinWithData() returned empty result")
return
}

// Result should contain the original SHA
if !strings.Contains(result, tt.sha) {
t.Errorf("GetActionPinWithData() = %s, expected to contain SHA %s", result, tt.sha)
}

// IMPORTANT: Should NOT emit any warnings for actions already pinned to SHAs
if strings.Contains(stderr, "⚠") || strings.Contains(stderr, "Unable to resolve") {
t.Errorf("Expected NO warnings for action already pinned to SHA, but got: %s", stderr)
}

// Log the resolution for debugging
t.Logf("Resolution: %s@%s → %s", tt.repo, tt.sha, result)
if stderr != "" {
t.Logf("Stderr (should be empty): %s", strings.TrimSpace(stderr))
}
})
}
}

func TestSortPinsByVersion(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading