checks: report unused lockfile entries and untracked workflows as errors

cmd/gh-actions-lock/command_test.go

@@ -1204,18 +1204,16 @@ jobs:
 	require.NoError(t, os.WriteFile(second, workflow, 0o600))
 	require.NoError(t, os.WriteFile(ignored, []byte("not a workflow\n"), 0o600))
 
+	// No lockfile, so both workflows are untracked and the run fails.
 	stdout, _, err := runCommandWithHTTP(t, reg, "--no-fix", "--json=valid,workflows", dir)
-	require.NoError(t, err)
+	require.ErrorIs(t, err, errSilent)
 
 	var payload struct {
-		Valid     bool `json:"valid"`
 		Workflows []struct {
-			Path  string `json:"path"`
-			Valid bool   `json:"valid"`
+			Path string `json:"path"`
 		} `json:"workflows"`
 	}
 	require.NoError(t, json.Unmarshal([]byte(stdout), &payload))
-	assert.True(t, payload.Valid)
 	require.Len(t, payload.Workflows, 2)
 	assert.Equal(t, first, payload.Workflows[0].Path)
 	assert.Equal(t, second, payload.Workflows[1].Path)

cmd/gh-actions-lock/onboard_gate.go

@@ -27,7 +27,18 @@ func gateNoOnboard(report *checks.Report) []string {
 		refusedKeys := make(map[string]bool)
 		for fi := range wr.Findings {
 			f := &wr.Findings[fi]
-			if f.Category != checks.NotPinned || f.ActionRef == nil || f.TrackedAction {
+			if f.Category != checks.NotPinned || f.TrackedAction {
+				continue
+			}
+			if f.ActionRef == nil {
+				// A dependency-free workflow missing from the lockfile.
+				if f.Remediable && len(wr.ActionRefs) == 0 {
+					f.Category = checks.OnboardingRequired
+					f.Severity = checks.SeverityInfo
+					f.Detail = "workflow has no lockfile entry; --no-onboard refuses to add new workflows or actions"
+					f.Remediation = "onboard it first with `gh actions-lock` (without --no-onboard)"
+					refused = append(refused, wr.Path)
+				}
 				continue
 			}
 			ar := f.ActionRef

cmd/gh-actions-lock/testdata/golden-json/expected.json

@@ -57,8 +57,8 @@
       "dependency": "old/dead@v1",
       "detail": "lockfile pins old/dead@v1 but no uses: in this workflow references it",
       "doc_url": "https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-third-party-actions",
-      "remediation": "remove the entry or re-run `gh actions-lock`",
-      "severity": "warning",
+      "remediation": "run `gh actions-lock` to prune the unused entry",
+      "severity": "error",
       "workflow": ".github/workflows/ci.yml"
     }
   ],
@@ -123,8 +123,8 @@
           "dependency": "old/dead@v1",
           "detail": "lockfile pins old/dead@v1 but no uses: in this workflow references it",
           "doc_url": "https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-third-party-actions",
-          "remediation": "remove the entry or re-run `gh actions-lock`",
-          "severity": "warning",
+          "remediation": "run `gh actions-lock` to prune the unused entry",
+          "severity": "error",
           "workflow": ".github/workflows/ci.yml"
         }
       ],

cmd/gh-actions-lock/verify_test.go

@@ -4,10 +4,13 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 	"testing"
 
+	"github.com/github/gh-actions-lock/cmd/gh-actions-lock/format"
 	"github.com/github/gh-actions-lock/internal/ghapi/httpmock"
+	"github.com/github/gh-actions-lock/internal/pipeline/checks"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -159,3 +162,214 @@ jobs:
 		}
 	}
 }
+
+// TestStaleEntry_SeverityMatchesVerdict pins the contract consumers rely on:
+// `valid` is false, and read-only modes exit non-zero, exactly when a finding
+// has error severity. An unused lockfile entry is an error in every mode; a
+// fix run prunes it and succeeds.
+func TestStaleEntry_SeverityMatchesVerdict(t *testing.T) {
+	const workflow = `
+name: ci
+on: push
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4.2.1
+`
+	for _, tc := range []struct {
+		name    string
+		mode    []string
+		wantErr bool
+	}{
+		{name: "verify fails", mode: []string{"--verify"}, wantErr: true},
+		{name: "verify-local fails", mode: []string{"--verify-local"}, wantErr: true},
+		{name: "fix run prunes", wantErr: false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			path := writeTempWorkflow(t, workflow,
+				"actions/checkout@v4.2.1=sha1-"+parityTestSHA,
+				"actions/setup-go@v5=sha1-"+parityTestSHA,
+			)
+			reg := &httpmock.Registry{}
+			args := append(slices.Clone(tc.mode), "--no-interactive", path)
+
+			p, err := runParity(t, reg, args...)
+			if tc.wantErr {
+				require.ErrorIs(t, err, errSilent)
+			} else {
+				require.NoError(t, err)
+			}
+
+			var stale []format.Finding
+			hasError := false
+			for _, f := range p.Findings {
+				hasError = hasError || f.Severity == string(checks.SeverityError)
+				if f.Category == string(checks.Stale) {
+					stale = append(stale, f)
+				}
+			}
+			require.Len(t, stale, 1, "%+v", p.Findings)
+			assert.Equal(t, "actions/setup-go@v5", stale[0].Dependency)
+			assert.Equal(t, string(checks.SeverityError), stale[0].Severity)
+			assert.Equal(t, !hasError, p.Valid, "valid must agree with severities")
+
+			pins := readTempLockfilePins(t)
+			if tc.wantErr {
+				assert.Contains(t, pins, "actions/setup-go@v5", "read-only modes must not prune")
+			} else {
+				assert.NotContains(t, pins, "actions/setup-go@v5", "fix run prunes the unused entry")
+			}
+		})
+	}
+}
+
+// TestStaleEntry_TerminalShowsRemediation checks a human sees the finding
+// and how to clear it in both read-only modes.
+func TestStaleEntry_TerminalShowsRemediation(t *testing.T) {
+	for _, mode := range []string{"--verify", "--verify-local"} {
+		t.Run(mode, func(t *testing.T) {
+			path := writeTempWorkflow(t, `
+name: ci
+on: push
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4.2.1
+`,
+				"actions/checkout@v4.2.1=sha1-"+parityTestSHA,
+				"actions/setup-go@v5=sha1-"+parityTestSHA,
+			)
+			_, stderr, err := runCommandWithHTTP(t, &httpmock.Registry{}, mode, "--no-interactive", path)
+			require.ErrorIs(t, err, errSilent)
+			assert.Contains(t, stderr, "Unused lockfile entry actions/setup-go@v5")
+			assert.Contains(t, stderr, "without "+mode)
+		})
+	}
+}
+
+// TestVerifyLocal_LocalActionVerdictFollowsSeverity: a local-action warning
+// on a workflow the lockfile doesn't cover yet is not a failure; once the
+// workflow is onboarded the same step is an error. JSON omits the warning
+// (skipFindingInJSON), so the terminal run proves it is still surfaced.
+func TestVerifyLocal_LocalActionVerdictFollowsSeverity(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		body    string
+		pins    []string
+		other   bool // lockfile exists but covers another workflow
+		wantErr bool
+	}{
+		{
+			name:  "not onboarded warns and passes",
+			body:  "      - uses: ./my-action\n",
+			other: true,
+		},
+		{
+			name:    "onboarded errors and fails",
+			body:    "      - uses: actions/checkout@v4.2.1\n      - uses: ./my-action\n",
+			pins:    []string{"actions/checkout@v4.2.1=sha1-" + parityTestSHA},
+			wantErr: true,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			writeTempWorkflow(t, "name: ci\non: push\njobs:\n  test:\n    runs-on: ubuntu-latest\n    steps:\n"+tc.body, tc.pins...)
+			if tc.other {
+				writeTempLockfile(t, ".", "other.yml", []string{"actions/checkout@v4.2.1=sha1-" + parityTestSHA})
+			}
+			require.NoError(t, os.Mkdir(".git", 0o755))
+			require.NoError(t, os.Mkdir("my-action", 0o755))
+			require.NoError(t, os.WriteFile(filepath.Join("my-action", "action.yml"), []byte("runs:\n  using: composite\n  steps: []\n"), 0o644))
+			checkErr := func(err error) {
+				if tc.wantErr {
+					require.ErrorIs(t, err, errSilent)
+				} else {
+					require.NoError(t, err)
+				}
+			}
+
+			p, err := runParity(t, &httpmock.Registry{}, "--verify-local", "--no-interactive")
+			checkErr(err)
+			assert.Equal(t, !tc.wantErr, p.Valid)
+			if tc.wantErr {
+				require.Len(t, p.Findings, 1, "%+v", p.Findings)
+				assert.Equal(t, string(checks.LocalAction), p.Findings[0].Category)
+				assert.Equal(t, string(checks.SeverityError), p.Findings[0].Severity)
+			} else {
+				assert.Empty(t, p.Findings, "non-error local-action is omitted from JSON")
+			}
+
+			stdout, stderr, err := runCommandWithHTTP(t, &httpmock.Registry{}, "--verify-local", "--no-interactive")
+			checkErr(err)
+			assert.Contains(t, stdout+stderr, "local path actions")
+		})
+	}
+}
+
+// TestEmptyWorkflow_LockfileEntryRequired: enforcement requires every
+// workflow to be listed, so a workflow with no uses: still needs an entry
+// and that entry must not carry pins.
+func TestEmptyWorkflow_LockfileEntryRequired(t *testing.T) {
+	const workflow = `
+name: ci
+on: push
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo ok
+`
+	const pin = "actions/checkout@v4=sha1-" + parityTestSHA
+	for _, tc := range []struct {
+		name         string
+		mode         []string
+		otherPins    []string // lockfile covers only other.yml
+		ownPins      []string // lockfile covers workflow.yml
+		wantCategory checks.Category
+		wantErr      bool
+		wantEntry    string
+	}{
+		{name: "verify untracked", mode: []string{"--verify"}, otherPins: []string{pin}, wantCategory: checks.NotPinned, wantErr: true},
+		{name: "verify-local untracked", mode: []string{"--verify-local"}, otherPins: []string{pin}, wantCategory: checks.NotPinned, wantErr: true},
+		{name: "fix run adds entry", otherPins: []string{pin}, wantCategory: checks.NotPinned, wantEntry: "'.github/workflows/workflow.yml': []"},
+		{name: "no-onboard leaves it", mode: []string{"--no-onboard"}, otherPins: []string{pin}, wantCategory: checks.OnboardingRequired},
+		{name: "verify no-onboard opts out", mode: []string{"--verify", "--no-onboard"}, otherPins: []string{pin}, wantCategory: checks.OnboardingRequired},
+		{name: "verify unused pin", mode: []string{"--verify"}, ownPins: []string{pin}, wantCategory: checks.Stale, wantErr: true},
+		{name: "verify-local unused pin", mode: []string{"--verify-local"}, ownPins: []string{pin}, wantCategory: checks.Stale, wantErr: true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			path := writeTempWorkflow(t, workflow, tc.ownPins...)
+			if len(tc.otherPins) > 0 {
+				writeTempLockfile(t, ".", "other.yml", tc.otherPins)
+			}
+			args := append(slices.Clone(tc.mode), "--no-interactive", path)
+
+			p, err := runParity(t, &httpmock.Registry{}, args...)
+			if tc.wantErr {
+				require.ErrorIs(t, err, errSilent)
+			} else {
+				require.NoError(t, err)
+			}
+
+			hasError := false
+			var cats []string
+			for _, f := range p.Findings {
+				hasError = hasError || f.Severity == string(checks.SeverityError)
+				cats = append(cats, f.Category)
+			}
+			// Fix runs report the pre-fix diagnosis, so only the read-only
+			// modes tie errors to the exit code.
+			if len(tc.mode) > 0 {
+				assert.Equal(t, tc.wantErr, hasError, "%+v", p.Findings)
+			}
+			assert.Equal(t, !hasError, p.Valid, "valid must agree with severities")
+			if tc.wantCategory != "" {
+				assert.Contains(t, cats, string(tc.wantCategory))
+			}
+			if tc.wantEntry != "" {
+				assert.Contains(t, readTempLockfilePins(t), tc.wantEntry)
+			}
+		})
+	}
+}

internal/pipeline/checks/category.go

@@ -20,8 +20,8 @@ const (
 	// RefMoved means the upstream tag now resolves to a different
 	// SHA than what the lockfile has recorded.
 	RefMoved Category = "ref-moved"
-	// Stale means the pinned SHA no longer matches what the ref
-	// resolves to today.
+	// Stale means the lockfile pins a dependency for a workflow that no
+	// uses: in that workflow references.
 	Stale Category = "stale"
 	// MisleadingSHA means a ref looks like a SHA but resolves to a
 	// different commit.

internal/pipeline/checks/finding.go

@@ -122,22 +122,11 @@ func (r *WorkflowReport) CountByCategory(c Category) int {
 	return n
 }
 
-// IsValid returns true for findings that don't represent integrity violations.
+// IsValid reports whether f leaves the verdict valid. Severity alone decides:
+// the JSON `valid` field and the exit code must agree with the severities
+// consumers see, so a blocking finding has to be emitted as an error.
 func (f *Finding) IsValid() bool {
-	if f.Severity == SeverityError {
-		return false
-	}
-	if f.Category.IsInconclusive() {
-		return true
-	}
-	switch f.Category {
-	case Valid, RunOnly, LocalAction, RepoRenamed, ShaAsRef, RefMoved, VersionRef, OnboardingRequired, StaleWorkflow, SelfRepositoryAction, FreshTag, CooldownConfigIgnored:
-		return true
-	case NotPinned:
-		return f.ActionRef == nil // workflow-level is a warning
-	default:
-		return false
-	}
+	return f.Severity != SeverityError
 }
 
 // IsWarning returns true for findings that should render as warnings (not errors).

internal/pipeline/checks/finding_test.go

@@ -0,0 +1,23 @@
+package checks
+
+import (
+	"testing"
+
+	parserlock "github.com/github/actions-lockfile/go/pkg/lockfile"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFindingIsValidFollowsSeverity(t *testing.T) {
+	categories := []Category{Stale, NotPinned, RefChanged, UnreachablePin, ShaAsRef, RefMoved, ReachabilityUnknown, StaleWorkflow, Valid}
+	for _, sev := range []Severity{SeverityOK, SeverityInfo, SeverityWarning, SeverityError} {
+		for _, cat := range categories {
+			t.Run(string(cat)+"/"+string(sev), func(t *testing.T) {
+				f := Finding{Category: cat, Severity: sev, ActionRef: &parserlock.ActionRef{Owner: "o", Repo: "r", Ref: "v1"}}
+				want := sev != SeverityError
+				assert.Equal(t, want, f.IsValid())
+				r := Report{Workflows: []WorkflowReport{{Findings: []Finding{f}}}}
+				assert.Equal(t, want, r.IsValid())
+			})
+		}
+	}
+}

internal/pipeline/checks/run.go

@@ -20,10 +20,11 @@ import (
 // DocURL and ParentNWO are attached by the caller (diagnoseOneParsed)
 // because they need lookup tables runChecks doesn't carry.
 func RunChecks(ctx context.Context, pw ParsedWorkflow, lf parserlock.File, r CheckResolver) []Finding {
-	wfEntry, _ := lf.LookupWorkflow(workflowfile.KeyFromPath(pw.Path))
+	wfEntry, tracked := lf.LookupWorkflow(workflowfile.KeyFromPath(pw.Path))
 	depPins, depIndex := parseWorkflowDeps(wfEntry, lf.Dependencies)
 
 	var out []Finding
+	out = append(out, checkUntracked(pw, tracked)...)
 	out = append(out, checkNotPinned(pw, depPins, depIndex)...)
 	out = append(out, checkShaAsRef(pw, depIndex)...)
 	out = append(out, checkRefChanged(pw, depPins)...)