Block on missing refs and commits, and say how to fix them

cmd/gh-actions-lock/format/terminal.go

@@ -122,7 +122,7 @@ func PresentReadOnlyFailures(out *ui.UI, report *checks.Report) (hasFixable bool
 		g := groups[key]
 		out.TermBlank()
 		for _, f := range g.findings {
-			if IsAutoFixable(f.Category) {
+			if IsAutoFixable(f.Category) && !f.RefMissing {
 				hasFixable = true
 			} else if f.Severity == checks.SeverityError {
 				blocked = true
@@ -141,7 +141,7 @@ func PresentReadOnlyFailures(out *ui.UI, report *checks.Report) (hasFixable bool
 // prints a single non-valid finding to the terminal (bypassing the
 // narration log) so read-only modes surface the same detail as fix mode.
 func renderTermFindingDetail(out *ui.UI, f checks.Finding, dep string) {
-	label := categoryLabel(f.Category)
+	label := findingLabel(f)
 	icon := out.TermYellow(ui.IconWarning)
 	if f.Severity == checks.SeverityError {
 		icon = out.TermRed(ui.IconError)
@@ -154,7 +154,7 @@ func renderTermFindingDetail(out *ui.UI, f checks.Finding, dep string) {
 			out.TermDetail("  ↳ %s", out.TermDim(DepReleaseURL(f.Dependency.Hostname, owner+"/"+repo, nil)))
 		}
 	}
-	if IsAlertedCategory(f.Category) && f.Remediation != "" {
+	if (IsAlertedCategory(f.Category) || f.RefMissing) && f.Remediation != "" {
 		out.TermDetail("  %s %s", out.TermYellow(ui.IconWarning), f.Remediation)
 	}
 	if f.RecommendedTag != "" {
@@ -174,6 +174,15 @@ func renderTermFindingDetail(out *ui.UI, f checks.Finding, dep string) {
 	}
 }
 
+// findingLabel is categoryLabel with a terminal-only title for refs that
+// don't exist upstream, which share the not-pinned category in JSON.
+func findingLabel(f checks.Finding) string {
+	if f.RefMissing {
+		return "Ref does not exist"
+	}
+	return categoryLabel(f.Category)
+}
+
 // categoryLabel returns a human-readable label for a finding category,
 // e.g. "repo-renamed" -> "Repository renamed". Falls back to a
 // hyphen-to-space title for unmapped categories so new slugs still read
@@ -278,7 +287,7 @@ func renderErrorFindings(out *ui.UI, report *checks.Report, failedCount, checked
 // renderFindingDetail prints a single non-valid finding with its category
 // icon, detail text, remediation, and doc links.
 func renderFindingDetail(out *ui.UI, f checks.Finding, dep string) {
-	label := categoryLabel(f.Category)
+	label := findingLabel(f)
 	icon := out.Yellow(ui.IconWarning)
 	if f.Severity == checks.SeverityError {
 		icon = out.Red(ui.IconError)
@@ -291,7 +300,7 @@ func renderFindingDetail(out *ui.UI, f checks.Finding, dep string) {
 			out.Detail("  ↳ %s", out.Dim(DepReleaseURL(f.Dependency.Hostname, owner+"/"+repo, nil)))
 		}
 	}
-	if IsAlertedCategory(f.Category) && f.Remediation != "" {
+	if (IsAlertedCategory(f.Category) || f.RefMissing) && f.Remediation != "" {
 		out.Detail("  %s %s", out.Yellow(ui.IconWarning), f.Remediation)
 	}
 	if f.RecommendedTag != "" {

cmd/gh-actions-lock/format/terminal_test.go

@@ -661,6 +661,31 @@ func TestPresentReadOnlyFailures_LocalActionNotFixable(t *testing.T) {
 	}
 }
 
+// TestPresentReadOnlyFailures_MissingRefNotFixable keeps the "Re-run
+// without --no-fix" hint off a ref that doesn't exist: it shares the fixable
+// not-pinned category, but a re-run can't create the ref.
+func TestPresentReadOnlyFailures_MissingRefNotFixable(t *testing.T) {
+	u, _ := newTestUI()
+	report := &checks.Report{
+		Workflows: []checks.WorkflowReport{{
+			Path: ".github/workflows/ci.yml",
+			Findings: []checks.Finding{{
+				WorkflowPath: ".github/workflows/ci.yml",
+				Category:     checks.NotPinned,
+				Severity:     checks.SeverityError,
+				Confidence:   checks.ConfidenceHigh,
+				Dependency:   &dep.Dependency{NWO: "actions/checkout", Ref: "v999.0.0"},
+				Detail:       "ref v999.0.0 does not exist in actions/checkout",
+				RefMissing:   true,
+			}},
+		}},
+	}
+
+	if PresentReadOnlyFailures(u, report) {
+		t.Error("a missing ref is not auto-fixable; hasFixable should be false")
+	}
+}
+
 // TestPresentReadOnlyFailures_UnreachablePinShowsReleases verifies the
 // read-only renderer includes the upstream releases link for an
 // unreachable-pin finding, matching the detailed (fix-mode) renderer.

cmd/gh-actions-lock/missing_ref_test.go

@@ -0,0 +1,117 @@
+package main
+
+import (
+	"encoding/json"
+	"net/http"
+	"os"
+	"regexp"
+	"strings"
+	"testing"
+
+	"github.com/github/gh-actions-lock/internal/ghapi/httpmock"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestMissingRefNamesTheCause proves a ref that does not exist fails with an
+// error finding that names it, rather than a fixable ref-changed plus a
+// reachability warning.
+func TestMissingRefNamesTheCause(t *testing.T) {
+	refs := []struct {
+		name, ref, detail, remedy string
+	}{
+		{"tag", "v999.0.0", "ref v999.0.0 does not exist in actions/checkout",
+			"point `uses:` at a tag or branch that exists"},
+		{"commit", "7b403c9ec14bd3ae0bbf793c2bee8815a7ac920b", "commit 7b403c9ec14b does not exist or is not reachable in actions/checkout",
+			"pin a commit that exists in actions/checkout, or point `uses:` at a tag or branch"},
+		{"short sha", "7b403c9", "ref 7b403c9 does not resolve in actions/checkout",
+			"if this is a commit, use the full 40-character SHA; otherwise point `uses:` at a tag or branch that exists"},
+	}
+	for _, tt := range refs {
+		for _, proxy := range []bool{false, true} {
+			t.Run(tt.name+"/"+map[bool]string{false: "graphql", true: "dependabot proxy"}[proxy], func(t *testing.T) {
+				testMissingRef(t, tt.ref, tt.detail, tt.remedy, proxy)
+			})
+		}
+	}
+}
+
+func testMissingRef(t *testing.T, ref, detail, remedy string, proxy bool) {
+	t.Helper()
+	reg := &httpmock.Registry{}
+	// Each resolve pass (plan, then diagnosis) asks again, for both
+	// the JSON and the terminal run.
+	for range 6 {
+		registerMissingRef(reg, ref, proxy)
+	}
+	if proxy {
+		t.Setenv("GH_ACTIONS_LOCK_DEPENDABOT_PROXY", "1")
+		useDefaultTransport(t, reg)
+	}
+	path := writeTempWorkflow(t, `
+name: ci
+on: push
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4.2.2
+`, "actions/checkout@v4.2.2")
+	wf, err := os.ReadFile(path)
+	require.NoError(t, err)
+	wf = []byte(strings.Replace(string(wf), "@v4.2.2", "@"+ref, 1))
+	require.NoError(t, os.WriteFile(path, wf, 0o600))
+	lockBefore := readTempLockfilePins(t)
+
+	stdout, _, err := runCommandWithHTTP(t, reg,
+		"--no-onboard", "--no-narrow", "--no-interactive", "--json=findings", path)
+	require.ErrorIs(t, err, errSilent)
+	var out struct {
+		Findings []struct {
+			Category    string `json:"category"`
+			Severity    string `json:"severity"`
+			Detail      string `json:"detail"`
+			Remediation string `json:"remediation"`
+			DocURL      string `json:"doc_url"`
+		} `json:"findings"`
+	}
+	require.NoError(t, json.Unmarshal([]byte(stdout), &out), stdout)
+	found := false
+	for _, f := range out.Findings {
+		if f.Category == "not-pinned" && f.Severity == "error" {
+			found = true
+			assert.Equal(t, detail, f.Detail)
+			assert.Equal(t, remedy, f.Remediation)
+			assert.Empty(t, f.DocURL, "the security-hardening page doesn't explain a missing ref")
+		}
+	}
+	assert.True(t, found, stdout)
+	assert.Equal(t, lockBefore, readTempLockfilePins(t))
+	after, err := os.ReadFile(path)
+	require.NoError(t, err)
+	assert.Equal(t, string(wf), string(after))
+
+	_, stderr, err := runCommandWithHTTP(t, reg,
+		"--no-onboard", "--no-narrow", "--no-interactive", path)
+	require.ErrorIs(t, err, errSilent)
+	assert.Contains(t, stderr, "Ref does not exist actions/checkout@"+ref)
+	assert.Contains(t, stderr, detail)
+	assert.Contains(t, stderr, remedy)
+	assert.NotContains(t, stderr, "Re-run without")
+	assert.NotContains(t, stderr, "Not pinned")
+	assert.NotContains(t, stderr, "security-hardening")
+}
+
+func registerMissingRef(reg *httpmock.Registry, ref string, proxy bool) {
+	if !proxy {
+		reg.Register(httpmock.GraphQL(`repository\(owner:`), httpmock.JSONResponse(map[string]any{
+			"data": map[string]any{"a0": map[string]any{"nameWithOwner": "actions/checkout", "object": nil}},
+		}))
+		return
+	}
+	reg.Register(httpmock.REST("GET", `^/repos/actions/checkout$`), httpmock.JSONResponse(map[string]any{
+		"full_name": "actions/checkout", "id": 1, "owner": map[string]any{"id": 1},
+	}))
+	reg.Register(httpmock.REST("GET", `^/repos/actions/checkout/commits/`+regexp.QuoteMeta(ref)+`$`),
+		httpmock.StatusResponse(http.StatusUnprocessableEntity))
+}

cmd/gh-actions-lock/parity_test.go

@@ -584,3 +584,12 @@ jobs:
 	assert.Equal(t, 2, countGraphQL(ct), "resolve + parity: %v", ct.urls)
 	assert.Contains(t, readTempLockfilePins(t), "actions/checkout@v4.2.1")
 }
+
+// useDefaultTransport routes anonymous REST calls, which use
+// http.DefaultTransport, through rt for the rest of the test.
+func useDefaultTransport(t *testing.T, rt http.RoundTripper) {
+	t.Helper()
+	orig := http.DefaultTransport
+	http.DefaultTransport = rt
+	t.Cleanup(func() { http.DefaultTransport = orig })
+}

internal/ghapi/errors.go

@@ -52,3 +52,13 @@ func IsRateLimited(err error) bool {
 	return httpErr.StatusCode == http.StatusForbidden &&
 		httpErr.Headers.Get("X-RateLimit-Remaining") == "0"
 }
+
+// RefNotFoundError reports a ref that does not exist in a repository that
+// does. Unlike a transport or rate-limit failure, retrying will not help.
+type RefNotFoundError struct {
+	Owner, Repo, Ref string
+	Err              error
+}
+
+func (e *RefNotFoundError) Error() string { return e.Err.Error() }
+func (e *RefNotFoundError) Unwrap() error { return e.Err }

internal/ghapi/graphql_action_files.go

@@ -310,17 +310,19 @@ func parseActionFileResponse(data map[string]json.RawMessage, refs []ActionFileR
 
 		if repo.Object == nil || repo.Object.OID == "" {
 			n := len(ref.Ref)
+			var err error
 			switch {
 			case isHexString(ref.Ref) && (n == 40 || n == 64):
 				// Full SHA that doesn't resolve — commit is unreachable/orphaned
-				results[idx].Err = fmt.Errorf("commit %s does not exist or is not reachable in %s/%s",
+				err = fmt.Errorf("commit %s does not exist or is not reachable in %s/%s",
 					ref.Ref[:12], ref.Owner, ref.Repo)
 			case isHexString(ref.Ref):
 				// Short hex — ambiguous, might be a truncated SHA
-				results[idx].Err = fmt.Errorf("version %q does not resolve — if this is a commit, use the full 40-character SHA", ref.Ref)
+				err = fmt.Errorf("version %q does not resolve — if this is a commit, use the full 40-character SHA", ref.Ref)
 			default:
-				results[idx].Err = fmt.Errorf("version %q does not exist", ref.Ref)
+				err = fmt.Errorf("version %q does not exist", ref.Ref)
 			}
+			results[idx].Err = &RefNotFoundError{Owner: ref.Owner, Repo: ref.Repo, Ref: ref.Ref, Err: err}
 			continue
 		}
 

internal/ghapi/rest_fallback.go

@@ -3,6 +3,7 @@ package ghapi
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -338,6 +339,10 @@ func (c *Client) resolveAnonymous(ctx context.Context, ref ActionFileRequest) Ac
 	)
 	sha, err := c.anonGetCommitSHA(ctx, commitURL)
 	if err != nil {
+		var missing *RefNotFoundError
+		if errors.As(err, &missing) {
+			missing.Owner, missing.Repo, missing.Ref = ref.Owner, ref.Repo, ref.Ref
+		}
 		result.Err = fmt.Errorf("anonymous fallback: %w", err)
 		return result
 	}
@@ -386,6 +391,9 @@ func (c *Client) anonGetCommitSHA(ctx context.Context, url string) (string, erro
 	}
 	defer resp.Body.Close()
 
+	if resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusUnprocessableEntity {
+		return "", &RefNotFoundError{Err: fmt.Errorf("HTTP %d resolving commit", resp.StatusCode)}
+	}
 	if resp.StatusCode != http.StatusOK {
 		return "", fmt.Errorf("HTTP %d resolving commit", resp.StatusCode)
 	}

internal/pipeline/checks/finding.go

@@ -47,6 +47,10 @@ type Finding struct {
 	// finding (regardless of outcome). Lets renderers distinguish
 	// "we didn't look" from "we looked and found nothing."
 	RecommendedSearched bool
+	// RefMissing marks a not-pinned finding whose ref definitively does not
+	// exist upstream. Terminal output titles it separately; the JSON
+	// category stays not-pinned.
+	RefMissing bool
 }
 
 // InventoryEntry describes a single dependency with context.

internal/pipeline/diagnose.go

@@ -101,6 +101,20 @@ func diagnoseOneParsed(ctx context.Context, pw checks.ParsedWorkflow, r *resolve
 				})
 				blockingResolverError = true
 			}
+			for _, missing := range missingRefs(resolveErr) {
+				nwo := missing.Owner + "/" + missing.Repo
+				wr.Findings = append(wr.Findings, checks.Finding{
+					WorkflowPath: pw.Path,
+					Category:     checks.NotPinned,
+					Severity:     checks.SeverityError,
+					Confidence:   checks.ConfidenceHigh,
+					Dependency:   &dep.Dependency{NWO: nwo, Ref: missing.Ref},
+					Detail:       missingRefDetail(missing.Ref, nwo),
+					Remediation:  missingRefRemediation(missing.Ref, nwo),
+					RefMissing:   true,
+				})
+				blockingResolverError = true
+			}
 			if blockingResolverError {
 				wr.BlockingResolverError = true
 				return wr
@@ -332,3 +346,48 @@ func hasIssues(ff []checks.Finding) bool {
 	}
 	return false
 }
+
+// missingRefs collects every ref the resolver definitively failed to find.
+func missingRefs(err error) []*ghapi.RefNotFoundError {
+	switch e := err.(type) {
+	case nil:
+		return nil
+	case *ghapi.RefNotFoundError:
+		return []*ghapi.RefNotFoundError{e}
+	case interface{ Unwrap() []error }:
+		var out []*ghapi.RefNotFoundError
+		for _, inner := range e.Unwrap() {
+			out = append(out, missingRefs(inner)...)
+		}
+		return out
+	}
+	return missingRefs(errors.Unwrap(err))
+}
+
+// missingRefDetail names a full commit SHA by its short form so the message
+// reads like the rest of the CLI's commit references.
+func missingRefDetail(ref, nwo string) string {
+	switch {
+	case isFullSHA(ref):
+		return fmt.Sprintf("commit %s does not exist or is not reachable in %s", ref[:12], nwo)
+	case isHex(ref):
+		return fmt.Sprintf("ref %s does not resolve in %s", ref, nwo)
+	}
+	return fmt.Sprintf("ref %s does not exist in %s", ref, nwo)
+}
+
+// missingRefRemediation keeps the short-SHA hint: a truncated commit is a
+// different mistake from a tag that was never published.
+func missingRefRemediation(ref, nwo string) string {
+	switch {
+	case isFullSHA(ref):
+		return fmt.Sprintf("pin a commit that exists in %s, or point `uses:` at a tag or branch", nwo)
+	case isHex(ref):
+		return "if this is a commit, use the full 40-character SHA; otherwise point `uses:` at a tag or branch that exists"
+	}
+	return "point `uses:` at a tag or branch that exists"
+}
+
+func isFullSHA(ref string) bool {
+	return (len(ref) == 40 || len(ref) == 64) && isHex(ref)
+}

test/scenarios/catalog.yml

@@ -1965,7 +1965,7 @@ scenarios:
 
   - name: dbot_impostor_blocks
     category: dependabot
-    description: "Orphaned commit (no tag or branch) produces reachability-unknown warning in JSON findings"
+    description: "Commit that does not exist upstream blocks with a not-pinned error"
     needs_token: true
     tags: [smoke]
     flags: ["--no-onboard", "--no-narrow", "--no-interactive", "--no-fix", "--json=findings"]
@@ -1975,14 +1975,16 @@ scenarios:
           name: CI
           actions: ["nodeselector/actions-test-fixtures@7b403c9ec14bd3ae0bbf793c2bee8815a7ac920b"]
     expect:
-      exit: 0
+      exit: 1
       stdout_is_json: true
-      stdout_contains: ["does not exist or is not reachable"]
+      stdout_contains: ["does not exist or is not reachable in nodeselector/actions-test-fixtures"]
       jq:
         - expr: '.findings | length'
           gt: 0
         - expr: '.findings[0].category'
-          equals: 'reachability-unknown'
+          equals: 'not-pinned'
+        - expr: '.findings[0].severity'
+          equals: 'error'
 
   - name: dbot_forgery_blocks
     category: dependabot