diff --git a/.changeset/patch-skip-sha-warnings.md b/.changeset/patch-skip-sha-warnings.md new file mode 100644 index 00000000000..e7400fbfbb6 --- /dev/null +++ b/.changeset/patch-skip-sha-warnings.md @@ -0,0 +1,12 @@ +--- +"gh-aw": patch +--- + +Skip dynamic resolution warnings for actions pinned to full SHAs + +Actions pinned to full 40-character commit SHAs should not emit dynamic +resolution warnings. This change updates `GetActionPinWithData()` to +detect SHA-based versions, suppress dynamic resolution warnings for +SHA-pinned actions, and preserve known SHA->version annotations when +available. Tests were added to cover SHA-pinned behavior. + diff --git a/.github/workflows/daily-fact.lock.yml b/.github/workflows/daily-fact.lock.yml index 50dbea4803f..bb8b161a6ad 100644 --- a/.github/workflows/daily-fact.lock.yml +++ b/.github/workflows/daily-fact.lock.yml @@ -49,7 +49,7 @@ jobs: comment_repo: "" steps: - name: Setup Scripts - uses: githubnext/gh-aw/actions/setup@623e612ff6a684e9a8634449508bdda21e2c178c + uses: githubnext/gh-aw/actions/setup@623e612ff6a684e9a8634449508bdda21e2c178c # 623e612ff6a684e9a8634449508bdda21e2c178c with: destination: /tmp/gh-aw/actions - name: Check workflow file timestamps @@ -86,7 +86,7 @@ jobs: output_types: ${{ steps.collect_output.outputs.output_types }} steps: - name: Setup Scripts - uses: githubnext/gh-aw/actions/setup@623e612ff6a684e9a8634449508bdda21e2c178c + uses: githubnext/gh-aw/actions/setup@623e612ff6a684e9a8634449508bdda21e2c178c # 623e612ff6a684e9a8634449508bdda21e2c178c with: destination: /tmp/gh-aw/actions - name: Checkout repository @@ -728,7 +728,7 @@ jobs: total_count: ${{ steps.missing_tool.outputs.total_count }} steps: - name: Setup Scripts - uses: githubnext/gh-aw/actions/setup@623e612ff6a684e9a8634449508bdda21e2c178c + uses: githubnext/gh-aw/actions/setup@623e612ff6a684e9a8634449508bdda21e2c178c # 623e612ff6a684e9a8634449508bdda21e2c178c with: destination: /tmp/gh-aw/actions - name: Debug job inputs @@ -815,7 +815,7 @@ jobs: success: ${{ steps.parse_results.outputs.success }} steps: - name: Setup Scripts - uses: githubnext/gh-aw/actions/setup@623e612ff6a684e9a8634449508bdda21e2c178c + uses: githubnext/gh-aw/actions/setup@623e612ff6a684e9a8634449508bdda21e2c178c # 623e612ff6a684e9a8634449508bdda21e2c178c with: destination: /tmp/gh-aw/actions - name: Download prompt artifact @@ -968,7 +968,7 @@ jobs: process_safe_outputs_temporary_id_map: ${{ steps.process_safe_outputs.outputs.temporary_id_map }} steps: - name: Setup Scripts - uses: githubnext/gh-aw/actions/setup@623e612ff6a684e9a8634449508bdda21e2c178c + uses: githubnext/gh-aw/actions/setup@623e612ff6a684e9a8634449508bdda21e2c178c # 623e612ff6a684e9a8634449508bdda21e2c178c with: destination: /tmp/gh-aw/actions - name: Download agent output artifact diff --git a/.github/workflows/playground-snapshots-refresh.lock.yml b/.github/workflows/playground-snapshots-refresh.lock.yml index 607b8264913..940f1e73c05 100644 --- a/.github/workflows/playground-snapshots-refresh.lock.yml +++ b/.github/workflows/playground-snapshots-refresh.lock.yml @@ -100,11 +100,11 @@ jobs: - name: Create gh-aw temp directory run: bash /tmp/gh-aw/actions/create_gh_aw_tmp_dir.sh - name: Checkout - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # 93cb6efe18208431cddfb8368fd83d5badbf9bfd with: persist-credentials: false - name: Setup Node.js - uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f # v6.1.0 + uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f # 395ad3262231945c25e8478fd5baf05154b1d79f with: node-version: "20" - env: diff --git a/pkg/workflow/action_pins.go b/pkg/workflow/action_pins.go index bfe744d6c52..71b7cc0eeda 100644 --- a/pkg/workflow/action_pins.go +++ b/pkg/workflow/action_pins.go @@ -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 != "" { @@ -168,7 +172,7 @@ 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) @@ -176,6 +180,19 @@ func GetActionPinWithData(actionRepo, version string, data *WorkflowData) (strin } } + // 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 @@ -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 @@ -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) diff --git a/pkg/workflow/action_pins_test.go b/pkg/workflow/action_pins_test.go index b4b6bf801fa..19d57b993e5 100644 --- a/pkg/workflow/action_pins_test.go +++ b/pkg/workflow/action_pins_test.go @@ -1,6 +1,8 @@ package workflow import ( + "bytes" + "os" "os/exec" "regexp" "strings" @@ -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