diff --git a/README.md b/README.md index a1f645d3..ed4a27a9 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,11 @@ To check without writing anything, for example in CI, run: gh actions-lock --verify ``` +`--verify` also checks that a commit locked under a branch or partial version +(e.g. `main`, `v4`) is still in that ref's history. `--verify-local` runs +offline and only checks that every ref has a lockfile entry, not that the +entries are trustworthy; use `--verify` in CI for that. + Pins to branches or partial versions (e.g. `main`, `v4`) are not advanced when upstream moves. To regenerate the lockfile from the current upstream commits, run: diff --git a/cmd/gh-actions-lock/command_test.go b/cmd/gh-actions-lock/command_test.go index d3461867..05d6759f 100644 --- a/cmd/gh-actions-lock/command_test.go +++ b/cmd/gh-actions-lock/command_test.go @@ -209,9 +209,10 @@ const nodeActionYAML = "name: Test Action\nruns:\n using: node20\n" func testRepoResponse(nameWithOwner, oid, actionYAML string) map[string]any { return map[string]any{ "nameWithOwner": nameWithOwner, - // commit and tag answer the parity query from the same fixture. + // commit, tag and head answer the parity query from the same fixture. "commit": map[string]any{"oid": oid}, "tag": map[string]any{"oid": oid}, + "head": map[string]any{"oid": oid}, "object": map[string]any{ "oid": oid, "file": map[string]any{ @@ -248,6 +249,7 @@ func parityOK(req *http.Request) (*http.Response, error) { "nameWithOwner": owner + "/" + body.Variables[fmt.Sprintf("name%d", i)].(string), "commit": oid, "tag": oid, + "head": oid, } } return httpmock.JSONResponse(map[string]any{"data": data})(req) diff --git a/cmd/gh-actions-lock/format/terminal.go b/cmd/gh-actions-lock/format/terminal.go index 0e44ab86..a5360ca9 100644 --- a/cmd/gh-actions-lock/format/terminal.go +++ b/cmd/gh-actions-lock/format/terminal.go @@ -148,7 +148,7 @@ func renderTermFindingDetail(out *ui.UI, f checks.Finding, dep string) { } out.TermDetail("%s %s %s", icon, out.TermDim(label), dep) out.TermDetail(" %s", f.Detail) - if f.Category == checks.UnreachablePin && f.Dependency != nil { + if f.Category == checks.UnreachablePin && !f.RefMissing && f.Dependency != nil { owner, repo := f.Dependency.OwnerRepo() if owner != "" { out.TermDetail(" ↳ %s", out.TermDim(DepReleaseURL(f.Dependency.Hostname, owner+"/"+repo, nil))) @@ -174,10 +174,14 @@ 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. +// findingLabel is categoryLabel with terminal-only titles for refs that +// don't exist upstream, which share the not-pinned and unreachable-pin +// categories in JSON. func findingLabel(f checks.Finding) string { - if f.RefMissing { + switch { + case f.RefMissing && f.Category == checks.UnreachablePin: + return "Ref no longer exists" + case f.RefMissing: return "Ref does not exist" } return categoryLabel(f.Category) @@ -294,7 +298,7 @@ func renderFindingDetail(out *ui.UI, f checks.Finding, dep string) { } out.Detail("%s %s %s", icon, out.Dim(label), dep) out.Detail(" %s", f.Detail) - if f.Category == checks.UnreachablePin && f.Dependency != nil { + if f.Category == checks.UnreachablePin && !f.RefMissing && f.Dependency != nil { owner, repo := f.Dependency.OwnerRepo() if owner != "" { out.Detail(" ↳ %s", out.Dim(DepReleaseURL(f.Dependency.Hostname, owner+"/"+repo, nil))) diff --git a/cmd/gh-actions-lock/parity_test.go b/cmd/gh-actions-lock/parity_test.go index c264fec8..00b38ea2 100644 --- a/cmd/gh-actions-lock/parity_test.go +++ b/cmd/gh-actions-lock/parity_test.go @@ -716,3 +716,237 @@ func TestParity_DependabotProxyFirstRunKeepsRenamedName(t *testing.T) { assert.Contains(t, lock, "'krzema12/github-actions-typing@v2.2.2':") assert.NotContains(t, lock, "typesafegithub") } + +// TestParity_VerifyMutableRefHistory: --verify checks that a commit locked +// under a mutable ref is still in that ref's history, since a fork-network +// commit exists under the upstream name. A legitimately moved ref stays +// sticky and silent. Plain runs make no extra calls. +func TestParity_VerifyMutableRefHistory(t *testing.T) { + const live = "cccccccccccccccccccccccccccccccccccccccc" + compareURL := `repos/actions/checkout/compare/` + parityTestSHA + `\.\.\.` + live + tests := []struct { + name string + compare httpmock.Responder + // want is the finding category, "" for a clean pass. + want string + }{ + {"impostor blocks", httpmock.JSONResponse(map[string]any{"status": "diverged", "merge_base_commit": map[string]any{"sha": parityTestMoved}}), "unreachable-pin"}, + {"moved forward passes", httpmock.JSONResponse(map[string]any{"status": "ahead", "merge_base_commit": map[string]any{"sha": parityTestSHA}}), ""}, + {"compare 503 is inconclusive", httpmock.StatusResponse(http.StatusServiceUnavailable), "reachability-unknown"}, + } + for _, tt := range tests { + for _, proxy := range []bool{false, true} { + t.Run(tt.name+"/"+map[bool]string{false: "graphql", true: "dependabot proxy"}[proxy], func(t *testing.T) { + reg := &httpmock.Registry{} + if proxy { + t.Setenv("GH_ACTIONS_LOCK_DEPENDABOT_PROXY", "1") + useDefaultTransport(t, reg) + 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/`+parityTestSHA+`$`), httpmock.JSONResponse(map[string]any{"sha": parityTestSHA})) + reg.Register(httpmock.REST("GET", `^/repos/actions/checkout/commits/v4$`), httpmock.JSONResponse(map[string]any{"sha": live})) + } else { + reg.Register(httpmock.GraphQL(`head: object\(expression`), httpmock.JSONResponse(map[string]any{ + "data": map[string]any{"a0": map[string]any{ + "nameWithOwner": "actions/checkout", + "commit": map[string]any{"oid": parityTestSHA}, + "head": map[string]any{"oid": live}, + }}, + })) + } + for range 10 { + reg.Register(httpmock.REST("GET", compareURL), tt.compare) + } + path := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 +`, "actions/checkout@v4=sha1-"+parityTestSHA) + + stdout, stderr, err := runCommandWithHTTP(t, reg, "--verify", "--json=valid,findings", path) + if proxy && tt.want == "reachability-unknown" { + require.Error(t, err, stderr) + assert.NotErrorIs(t, err, errSilent, "inconclusive under the proxy is a retry") + assert.Contains(t, err.Error(), "HTTP 503") + return + } + var p parityPayload + require.NoError(t, json.Unmarshal([]byte(stdout), &p), stdout+stderr) + switch tt.want { + case "": + require.NoError(t, err, stderr) + assert.Empty(t, p.Findings, "a moved mutable ref stays sticky and silent") + case "unreachable-pin": + require.ErrorIs(t, err, errSilent) + require.Len(t, p.Findings, 1) + assert.Equal(t, "unreachable-pin", p.Findings[0].Category) + assert.Equal(t, "error", p.Findings[0].Severity) + assert.Contains(t, p.Findings[0].Detail, "is not in actions/checkout's history") + assert.Contains(t, p.Findings[0].Detail, "v4 currently points at cccccccccccc") + assert.NotEmpty(t, p.Findings[0].DocURL, "the impostor keeps its doc link") + default: + require.NoError(t, err, stderr) + require.Len(t, p.Findings, 1) + assert.Equal(t, tt.want, p.Findings[0].Category) + assert.Equal(t, "warning", p.Findings[0].Severity) + } + }) + } + } + + for _, proxy := range []bool{false, true} { + t.Run("plain run makes no extra calls/"+map[bool]string{false: "graphql", true: "dependabot proxy"}[proxy], func(t *testing.T) { + reg := &httpmock.Registry{} + if proxy { + t.Setenv("GH_ACTIONS_LOCK_DEPENDABOT_PROXY", "1") + useDefaultTransport(t, reg) + 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/`+parityTestSHA+`$`), httpmock.JSONResponse(map[string]any{"sha": parityTestSHA})) + } else { + reg.Register(httpmock.GraphQL(`head: object\(expression`), func(*http.Request) (*http.Response, error) { + t.Error("a plain run must not resolve mutable refs") + return nil, io.EOF + }) + registerParity(reg) + } + path := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 +`, "actions/checkout@v4=sha1-"+parityTestSHA) + + _, stderr, err := runCommandWithHTTP(t, reg, "--no-onboard", "--no-narrow", "--no-interactive", path) + require.NoError(t, err, stderr) + for _, req := range reg.Requests { + assert.NotContains(t, req.URL.Path, "/compare/") + assert.NotContains(t, req.URL.Path, "/commits/v4") + } + }) + } +} + +// TestParity_VerifyMissingMutableRefBlocks: under --verify a mutable ref +// that no longer exists is a definitive answer the runner rejects, so it +// blocks rather than passing on the commit alone. +func TestParity_VerifyMissingMutableRefBlocks(t *testing.T) { + for _, proxy := range []bool{false, true} { + t.Run(map[bool]string{false: "graphql", true: "dependabot proxy"}[proxy], func(t *testing.T) { + reg := &httpmock.Registry{} + if proxy { + t.Setenv("GH_ACTIONS_LOCK_DEPENDABOT_PROXY", "1") + useDefaultTransport(t, reg) + } + // One pass for --json, one for the terminal. + for range 2 { + if proxy { + 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/`+parityTestSHA+`$`), httpmock.JSONResponse(map[string]any{"sha": parityTestSHA})) + reg.Register(httpmock.REST("GET", `^/repos/actions/checkout/commits/v4$`), httpmock.StatusResponse(http.StatusNotFound)) + } else { + reg.Register(httpmock.GraphQL(`head: object\(expression`), httpmock.JSONResponse(map[string]any{ + "data": map[string]any{"a0": map[string]any{ + "nameWithOwner": "actions/checkout", + "commit": map[string]any{"oid": parityTestSHA}, + "head": nil, + }}, + })) + } + } + path := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 +`, "actions/checkout@v4=sha1-"+parityTestSHA) + + stdout, stderr, err := runCommandWithHTTP(t, reg, "--verify", "--json=valid,findings", path) + require.ErrorIs(t, err, errSilent, stderr) + var p parityPayload + require.NoError(t, json.Unmarshal([]byte(stdout), &p), stdout+stderr) + require.Len(t, p.Findings, 1) + assert.Equal(t, "unreachable-pin", p.Findings[0].Category) + assert.Equal(t, "error", p.Findings[0].Severity) + assert.Equal(t, "ref v4 no longer exists in actions/checkout; the runner will reject this pin", p.Findings[0].Detail) + assert.Equal(t, "point `uses:` at a tag or branch that exists, then run `gh actions-lock`", p.Findings[0].Remediation) + assert.Empty(t, p.Findings[0].DocURL, "the hardening guide is unrelated to a missing ref") + + stdout, stderr, err = runCommandWithHTTP(t, reg, "--verify", path) + require.ErrorIs(t, err, errSilent) + term := stdout + stderr + assert.Contains(t, term, "Ref no longer exists actions/checkout@v4") + assert.Contains(t, term, "ref v4 no longer exists in actions/checkout; the runner will reject this pin") + assert.Contains(t, term, "point `uses:` at a tag or branch that exists, then run `gh actions-lock`") + assert.NotContains(t, term, "not in upstream history") + assert.NotContains(t, term, "/releases") + assert.NotContains(t, term, "see:") + }) + } +} + +// TestParity_VerifyRefCaseSensitive: refs differing only in case are +// distinct refs to the runner, so each gets its own history check. +func TestParity_VerifyRefCaseSensitive(t *testing.T) { + for _, proxy := range []bool{false, true} { + t.Run(map[bool]string{false: "graphql", true: "dependabot proxy"}[proxy], func(t *testing.T) { + reg := &httpmock.Registry{} + var heads []string + if proxy { + t.Setenv("GH_ACTIONS_LOCK_DEPENDABOT_PROXY", "1") + useDefaultTransport(t, reg) + for range 2 { + 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/`+parityTestSHA+`$`), httpmock.JSONResponse(map[string]any{"sha": parityTestSHA})) + } + for _, ref := range []string{"Main", "main"} { + reg.Register(httpmock.REST("GET", `^/repos/actions/checkout/commits/`+ref+`$`), func(req *http.Request) (*http.Response, error) { + heads = append(heads, ref) + return httpmock.JSONResponse(map[string]any{"sha": parityTestSHA})(req) + }) + } + } else { + reg.Register(httpmock.GraphQL(`head: object\(expression`), func(req *http.Request) (*http.Response, error) { + body, _ := io.ReadAll(req.Body) + for _, ref := range []string{"Main", "main"} { + if strings.Contains(string(body), `"`+ref+`^{commit}"`) { + heads = append(heads, ref) + } + } + repo := map[string]any{"nameWithOwner": "actions/checkout", "commit": map[string]any{"oid": parityTestSHA}, "head": map[string]any{"oid": parityTestSHA}} + return httpmock.JSONResponse(map[string]any{"data": map[string]any{"a0": repo, "a1": repo}})(req) + }) + } + path := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@Main + - uses: actions/checkout@main +`, "actions/checkout@Main=sha1-"+parityTestSHA, "actions/checkout@main=sha1-"+parityTestSHA) + + _, stderr, err := runCommandWithHTTP(t, reg, "--verify", path) + require.NoError(t, err, stderr) + assert.ElementsMatch(t, []string{"Main", "main"}, heads) + }) + } +} diff --git a/cmd/gh-actions-lock/run.go b/cmd/gh-actions-lock/run.go index d105a953..208c5f19 100644 --- a/cmd/gh-actions-lock/run.go +++ b/cmd/gh-actions-lock/run.go @@ -89,7 +89,8 @@ func bindCheckFlags(cmd *cobra.Command, opts *checkOptions) { cmd.Flags().BoolVar(&opts.verify, "verify", false, "Frozen check: run the same checks as the default without writing; fails if the lockfile is incomplete or the runner would reject a pin") cmd.Flags().BoolVar(&opts.verifyLocal, "verify-local", false, "Offline lockfile coverage check: verify every action ref has a lockfile entry.\n"+ - "No network calls, no authentication required — ideal for pre-commit hooks.") + "No network calls, no authentication required — ideal for pre-commit hooks.\n"+ + "It does not check that the entries are trustworthy; use --verify in CI for that.") cmd.Flags().StringVar(&opts.profileDir, "profile", "", "Enable profiling: write trace, CPU profile, and HTTP log to `dir`") cmd.Flags().BoolVar(&opts.noMigrateLocalActions, "no-migrate-local-actions", false, "Do not rewrite same-repo `uses: ./…` actions to the inherently-pinned `uses: $/…` form.\n"+ @@ -295,6 +296,7 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc) Store: store, Pool: pool, Relock: relock, + Verify: opts.verify, Profile: prof, } // Defer spinner start until actual network work begins. The fast path diff --git a/internal/ghapi/graphql_parity.go b/internal/ghapi/graphql_parity.go index 34f366dc..1bb0f582 100644 --- a/internal/ghapi/graphql_parity.go +++ b/internal/ghapi/graphql_parity.go @@ -21,6 +21,9 @@ type PinCheck struct { SHA string // Tag, when set, must still peel to SHA. Tag string + // Head, when set, is a mutable ref whose current commit is reported as + // HeadOID so the caller can check SHA is still in its history. + Head string } // PinState is the host's view of a PinCheck. Err marks an inconclusive check. @@ -37,7 +40,9 @@ type PinState struct { CommitFound bool // TagOID is the commit Tag peels to; empty when the tag is gone. TagOID string - Err error + // HeadOID is the commit Head peels to; empty when the ref is gone. + HeadOID string + Err error } // pinCheckBatchSize is larger than the action-file batch because each alias @@ -145,6 +150,11 @@ func buildPinCheckQuery(pins []PinCheck) (string, map[string]any) { fmt.Fprintf(&decl, ", $tag%d: String!", i) fmt.Fprintf(&body, " tag: object(expression: $tag%d) { oid }", i) } + if p.Head != "" { + vars[fmt.Sprintf("head%d", i)] = p.Head + "^{commit}" + fmt.Fprintf(&decl, ", $head%d: String!", i) + fmt.Fprintf(&body, " head: object(expression: $head%d) { oid }", i) + } body.WriteString(" }") } body.WriteString(" }") @@ -199,6 +209,9 @@ func parsePinCheckResponse(data map[string]json.RawMessage, pins []PinCheck, gql Tag *struct { OID string `json:"oid"` } `json:"tag"` + Head *struct { + OID string `json:"oid"` + } `json:"head"` } if err := json.Unmarshal(raw, &repo); err != nil { states[i].Err = fmt.Errorf("failed to parse: %w", err) @@ -213,6 +226,9 @@ func parsePinCheckResponse(data map[string]json.RawMessage, pins []PinCheck, gql if repo.Tag != nil { states[i].TagOID = repo.Tag.OID } + if repo.Head != nil { + states[i].HeadOID = repo.Head.OID + } } return states } @@ -236,7 +252,7 @@ func aliasError(gqlErr *api.GraphQLError, alias string) error { if len(item.Path) == 0 || item.Path[0] != alias { continue } - if item.Type == "NOT_FOUND" && len(item.Path) >= 2 && (item.Path[1] == "commit" || item.Path[1] == "tag") { + if item.Type == "NOT_FOUND" && len(item.Path) >= 2 && (item.Path[1] == "commit" || item.Path[1] == "tag" || item.Path[1] == "head") { continue } return errors.New(item.Message) @@ -246,7 +262,7 @@ func aliasError(gqlErr *api.GraphQLError, alias string) error { // checkPinsREST costs one repos/{owner}/{repo} call per unique repo plus one // commits call per intact pin: the tag when one is set (a match proves the -// commit exists), otherwise the SHA. +// commit exists), otherwise the SHA. A Head costs one more commits call. func (c *Client) checkPinsREST(ctx context.Context, pins []PinCheck) []PinState { states := make([]PinState, len(pins)) for i, p := range pins { @@ -283,6 +299,11 @@ func (c *Client) checkPinREST(ctx context.Context, p PinCheck) PinState { if err != nil { return PinState{Err: err} } + if p.Head != "" && s.CommitFound { + if s.HeadOID, _, err = c.restCommit(ctx, p.Owner, p.Repo, p.Head); err != nil { + return PinState{Err: err} + } + } return s } diff --git a/internal/ghapi/repos.go b/internal/ghapi/repos.go index a6550160..32934c91 100644 --- a/internal/ghapi/repos.go +++ b/internal/ghapi/repos.go @@ -427,7 +427,12 @@ func (c *Client) CompareRefs(ctx context.Context, owner, repo, base, head string url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(base), url.PathEscape(head)) var resp compareResponse - if err := c.rest.DoWithContext(ctx, http.MethodGet, path, nil, &resp); err != nil { + if c.restOnly { + err = c.anonGet(ctx, path, &resp) + } else { + err = c.rest.DoWithContext(ctx, http.MethodGet, path, nil, &resp) + } + if err != nil { return "", "", err } return resp.Status, resp.MergeBaseCommit.SHA, nil diff --git a/internal/pipeline/checks/finding.go b/internal/pipeline/checks/finding.go index d51f1965..5f10e3cf 100644 --- a/internal/pipeline/checks/finding.go +++ b/internal/pipeline/checks/finding.go @@ -47,9 +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 marks a not-pinned or unreachable-pin finding whose ref + // definitively does not exist upstream. Terminal output titles it + // separately and skips the release and doc links; the JSON category is + // unchanged. RefMissing bool // TrackedAction marks a not-pinned bare SHA whose owner/repo the // workflow already locks at another ref. It is a ref change on a diff --git a/internal/pipeline/parity.go b/internal/pipeline/parity.go index 36a71258..0abbd432 100644 --- a/internal/pipeline/parity.go +++ b/internal/pipeline/parity.go @@ -1,6 +1,7 @@ package pipeline import ( + "cmp" "context" "fmt" "strings" @@ -17,7 +18,10 @@ import ( // checkParity re-checks every locked pin in each workflow's recorded closure // the way the runner does at job start: same repository identity, commit // still present, exact-version tag still at the commit. Mutable refs that -// moved are not reported; the lock is sticky until --relock. +// moved are not reported; the lock is sticky until --relock. With verify, a +// commit on a mutable ref must still be in that ref's history: a fork-network +// commit resolves under the upstream name, so existence alone proves nothing, +// and the ref must still exist: the runner rejects a pin whose ref is gone. // // A pinned commit fixes the dependencies it declares, so the flattened // closure in the lockfile is checked in one batch with no recursion. @@ -32,7 +36,7 @@ import ( // // Under the Dependabot proxy any inconclusive check is an error: the run // aborts so Dependabot retries rather than shipping an unverified lock. -func checkParity(ctx context.Context, r *resolve.Resolver, parsed []checks.ParsedWorkflow, store *lockfile.State, report *checks.Report, recordedKeys map[string]bool, resolved []dep.Dependency, parents map[string][]string, keep func(checks.Category) bool) error { +func checkParity(ctx context.Context, r *resolve.Resolver, parsed []checks.ParsedWorkflow, store *lockfile.State, report *checks.Report, recordedKeys map[string]bool, resolved []dep.Dependency, parents map[string][]string, keep func(checks.Category) bool, verify bool) error { gh := r.GHClient() if gh == nil || store == nil { return nil @@ -52,15 +56,21 @@ func checkParity(ctx context.Context, r *resolve.Resolver, parsed []checks.Parse o.lp.Action.RepoID = store.RecordedRepoID(o.ref.Owner, o.ref.Repo) } } - key := strings.ToLower(o.lp.Pin.String() + ":" + lockedSHA(o.lp.Action.Commit)) + // Pin.String folds owner/repo only: refs are case-sensitive, as + // the runner keys them. + key := o.lp.Pin.String() + ":" + strings.ToLower(lockedSHA(o.lp.Action.Commit)) i, seen := pinIdx[key] if !seen { i = len(pins) pinIdx[key] = i pins = append(pins, ghapi.PinCheck{Owner: o.lp.Pin.Owner, Repo: o.lp.Pin.Repo, SHA: lockedSHA(o.lp.Action.Commit)}) } - if !o.fresh && checks.IsImmutableRef(o.lp.Pin.Ref) { + switch { + case o.fresh: + case checks.IsImmutableRef(o.lp.Pin.Ref): pins[i].Tag = o.lp.Pin.Ref + case verify && !parserlock.IsFullSha(o.lp.Pin.Ref): + pins[i].Head = o.lp.Pin.Ref } if j, dup := owned[[2]int{o.report, i}]; dup { owners[i][j].fresh = owners[i][j].fresh || o.fresh @@ -107,6 +117,18 @@ func checkParity(ctx context.Context, r *resolve.Resolver, parsed []checks.Parse if ctx.Err() != nil { return ctx.Err() } + for i := range states { + st, pc := &states[i], pins[i] + if pc.Head == "" || st.Err != nil || !st.CommitFound || st.HeadOID == "" || strings.EqualFold(st.HeadOID, pc.SHA) { + continue + } + switch ancestry, detail := r.CheckAncestry(ctx, pc.Owner, pc.Repo, pc.SHA, st.HeadOID); ancestry { + case resolve.AncestryNotAncestor: + st.CommitFound = false + case resolve.AncestryUnknown: + st.Err = fmt.Errorf("checking %s history: %s", pc.Head, detail) + } + } if ghapi.DependabotProxy() { for i, st := range states { if st.Err != nil { @@ -248,8 +270,13 @@ func parityFinding(pw checks.ParsedWorkflow, ref parserlock.ActionRef, lp lockfi } case !st.CommitFound: f.Category = checks.UnreachablePin - f.Detail = checks.NotInHistoryDetail(pc.SHA, nwo, ref.Ref, st.TagOID) + via + f.Detail = checks.NotInHistoryDetail(pc.SHA, nwo, ref.Ref, cmp.Or(st.TagOID, st.HeadOID)) + via f.Remediation = checks.AcceptMovedRemediation + case pc.Head != "" && st.HeadOID == "": + f.Category = checks.UnreachablePin + f.Detail = fmt.Sprintf("ref %s no longer exists in %s; the runner will reject this pin%s", pc.Head, nwo, via) + f.Remediation = "point `uses:` at a tag or branch that exists, then run `gh actions-lock`" + f.RefMissing = true case pc.Tag != "" && !strings.EqualFold(st.TagOID, pc.SHA): f.Category = checks.UnreachablePin f.ObservedSHA = st.TagOID @@ -275,7 +302,9 @@ func parityFinding(pw checks.ParsedWorkflow, ref parserlock.ActionRef, lp lockfi default: return checks.Finding{}, false } - f.DocURL = DocURLFor(f.Category) + if !f.RefMissing { + f.DocURL = DocURLFor(f.Category) + } return f, true } diff --git a/internal/pipeline/run.go b/internal/pipeline/run.go index b3144237..8c2c8a7d 100644 --- a/internal/pipeline/run.go +++ b/internal/pipeline/run.go @@ -22,6 +22,9 @@ type RunOptions struct { Pool *pinpool.Pool // Relock re-resolves every ref live instead of trusting the lockfile. Relock bool + // Verify also checks that each pin on a mutable ref is still in that + // ref's history, at one Compare call per ref that moved. + Verify bool // Resolver UX hooks — set these for interactive spinner mode. OnResolveProgress func(done, total int) @@ -134,7 +137,7 @@ func Run(ctx context.Context, opts RunOptions) (*RunResult, error) { } } endParity := prof.Phase(" parity check") - err := checkParity(ctx, r, parsed, opts.Store, report, recordedKeys, resolved, parents, keep) + err := checkParity(ctx, r, parsed, opts.Store, report, recordedKeys, resolved, parents, keep, opts.Verify) endParity() if err != nil { return nil, err