Merge pull request #48 from github/nodeselector/add-no-narrow-flag

Tag narrowing, dependabot contract, and integration hardening

Author: nodeselector

.gitignore

@@ -4,3 +4,4 @@
 # VHS demo recordings
 /demo/vhs/out/
 profiles/
+dist/

cmd/gh-actions-pin/check.go

@@ -13,11 +13,13 @@ import (
 
 	"github.com/MakeNowJust/heredoc"
 	"github.com/cli/go-gh/v2/pkg/repository"
+	parserlock "github.com/github/actions-lockfile/go/pkg/lockfile"
 	"github.com/github/gh-actions-pin/cmd/gh-actions-pin/format"
 	"github.com/github/gh-actions-pin/internal/config"
 	"github.com/github/gh-actions-pin/internal/pin"
 	"github.com/github/gh-actions-pin/internal/pinpool"
 	"github.com/github/gh-actions-pin/internal/pipeline"
+	"github.com/github/gh-actions-pin/internal/pipeline/checks"
 	"github.com/github/gh-actions-pin/internal/profile"
 	"github.com/github/gh-actions-pin/internal/resolve"
 	"github.com/github/gh-actions-pin/internal/tag"
@@ -40,6 +42,10 @@ type checkOptions struct {
 	// rewriting workflows or updating the lockfile. Orthogonal to the
 	// renderer choice (--json).
 	noFix bool
+	// noNarrow disables tag narrowing: mutable version refs like "v4"
+	// are kept as-is in the lock comment instead of being resolved to
+	// the full patch tag (e.g. "v4.2.1").
+	noNarrow bool
 }
 
 func newCheckCmd(newResolver resolverFunc) *cobra.Command {
@@ -124,6 +130,7 @@ func bindCheckFlags(cmd *cobra.Command, opts *checkOptions) {
 	cmd.Flags().StringVar(&opts.hostname, "hostname", "", "GitHub hostname to query (defaults to GH_HOST, current repo host, or github.com)")
 	cmd.Flags().BoolVar(&opts.rescan, "rescan", false, "Re-verify reachability for every recorded pin (bypasses the lockfile fast path)")
 	cmd.Flags().BoolVar(&opts.noFix, "no-fix", false, "Read-only: report findings without modifying workflows or the lockfile")
+	cmd.Flags().BoolVar(&opts.noNarrow, "no-narrow", false, "Keep mutable version refs (e.g. v4) instead of narrowing to full patch tags (e.g. v4.2.1)")
 	cmd.Flags().StringVar(&opts.profileDir, "profile", "", "Enable profiling: write trace, CPU profile, and HTTP log to `dir`")
 }
 
@@ -173,7 +180,11 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
 	}
 
 	endSetup := prof.Phase("setup (discover + lockfile)")
-	paths, r, store, err := newRun(opts.workflowPaths, opts.hostname, pool, newResolver)
+	// check fix mode can rebuild a deleted lockfile, so interactive sessions
+	// may delete-and-recreate an unreadable one. --no-fix is read-only and
+	// must not delete; it fails instead.
+	recoverLock := newLockRecovery(noInteractiveFlag(cmd), console, confirmFactoryHook, !opts.noFix)
+	paths, r, store, err := newRun(opts.workflowPaths, opts.hostname, pool, newResolver, recoverLock)
 	if err != nil {
 		return err
 	}
@@ -250,6 +261,20 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
 	valid := result.Valid
 	skippedRescan := result.SkippedRescan
 
+	// --no-onboard: refuse to onboard new workflows or actions. Rewrite the
+	// relevant not-pinned findings to onboarding-required and drop their refs
+	// so Plan/Commit never pins them; already-tracked refs that were bumped
+	// (ref-changed) are left to re-pin as usual.
+	onboardingRefused := 0
+	var refusedLabels []string
+	if noOnboardFlag(cmd) {
+		refusedLabels = gateNoOnboard(report)
+		onboardingRefused = len(refusedLabels)
+		if onboardingRefused > 0 {
+			valid = report.IsValid()
+		}
+	}
+
 	// Render the read-only diagnosis. --json selects the renderer; it does
 	// not decide whether fixes are applied. Terminal output is shown up front
 	// (the human narrative). JSON is emitted later, after any fixes land, so
@@ -274,6 +299,14 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
 				return err
 			}
 		}
+		// Surface SSO URL even in read-only mode — it's the actionable fix
+		// for SAML-gated repos and shouldn't require a --fix run to see.
+		if gc := r.GHClient(); gc != nil {
+			if ssoURL := gc.SSOURL(); ssoURL != "" {
+				console.TermBlank()
+				console.TermDetail("Authorize in your web browser:  %s", ssoURL)
+			}
+		}
 		if !valid {
 			if opts.jsonFields == "" {
 				console.TermDetail("Re-run without --no-fix to apply fixes.")
@@ -303,6 +336,7 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
 		RepoOwner: repoOwner,
 		RepoName:  repoName,
 		Version:   cliVersion(),
+		NoNarrow:  opts.noNarrow,
 	})
 	endPlan()
 	if planErr != nil {
@@ -321,6 +355,13 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
 
 	console.StopProgress()
 
+	// Inject info-severity findings for non-semver refs so they appear
+	// in --json output. Suppressed when --no-narrow is set (user chose
+	// this deliberately).
+	if !opts.noNarrow {
+		injectVersionRefFindings(report, record)
+	}
+
 	// Write the run log.
 	record.Repo = &pin.RepoInfo{Owner: repoOwner, Name: repoName, Host: resolveHostname(opts.hostname)}
 	if path, werr := record.WriteJSON(); werr == nil {
@@ -347,7 +388,7 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
 
 	// Terminal summary.
 	hasInconclusive := opts.rescan && report.HasInconclusive()
-	summaryErr := renderPinSummary(console, record, report, r, skippedRescan, hasInconclusive)
+	summaryErr := renderPinSummary(console, record, report, r, skippedRescan, hasInconclusive, refusedLabels, opts.noNarrow)
 
 	// Surface the SAML SSO authorization URL if one was captured during
 	// the run, matching cli/cli's "Authorize in your web browser:" line.
@@ -374,6 +415,60 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
 	return nil
 }
 
+// injectVersionRefFindings appends info-severity findings for entries pinned
+// with a non-full-semver ref (v4, v3.1, main, etc.). These surface in --json
+// output so machine consumers can detect imprecise refs.
+func injectVersionRefFindings(report *checks.Report, record *pin.Record) {
+	// Index which workflows each non-semver dep appears in.
+	type depInfo struct {
+		nwo string
+		ref string
+		wfs map[string]bool
+	}
+	seen := map[string]*depInfo{} // NWO@Ref → info
+	for _, e := range record.Entries {
+		if e.Resolution != pin.Pinned && e.Resolution != pin.Verified {
+			continue
+		}
+		sv, ok := parserlock.ParseSemVer(e.Ref)
+		if ok && sv.IsFull() {
+			continue
+		}
+		key := e.NWO + "@" + e.Ref
+		di, exists := seen[key]
+		if !exists {
+			di = &depInfo{nwo: e.NWO, ref: e.Ref, wfs: map[string]bool{}}
+			seen[key] = di
+		}
+		for _, wf := range e.Workflows {
+			di.wfs[wf] = true
+		}
+	}
+	if len(seen) == 0 {
+		return
+	}
+
+	// Append a finding to each affected workflow report.
+	for i := range report.Workflows {
+		wr := &report.Workflows[i]
+		for _, di := range seen {
+			if !di.wfs[wr.Path] {
+				continue
+			}
+			wr.Findings = append(wr.Findings, checks.Finding{
+				WorkflowPath: wr.Path,
+				Category:     checks.VersionRef,
+				Severity:     checks.SeverityInfo,
+				Confidence:   checks.ConfidenceHigh,
+				Detail: fmt.Sprintf(
+					"%s@%s: prefer a full semver ref (e.g. v4.2.1) — each patch tag resolves to exactly one commit",
+					di.nwo, di.ref,
+				),
+			})
+		}
+	}
+}
+
 // cliVersion returns the gh-actions-pin extension version embedded by the Go
 // build system. Returns "(devel)" for local `go build` and a real version
 // like "v0.1.2" when installed via `gh extension install`.

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

@@ -95,7 +95,7 @@ func renderErrorFindings(out *ui.UI, report *checks.Report, failedCount, checked
 	parts := []string{}
 	for _, cat := range []checks.Category{
 		checks.LockfileForgery,
-		checks.RefChanged, checks.NotPinned,
+		checks.RefChanged, checks.NotPinned, checks.OnboardingRequired,
 		checks.Stale, checks.MisleadingSHA, checks.ImpostorCommit,
 	} {
 		if n, ok := catCounts[cat]; ok {
@@ -252,7 +252,7 @@ func renderWarnings(out *ui.UI, report *checks.Report, willRemediate bool) {
 // remediator should not re-print it in non-interactive mode).
 func IsAlertedCategory(c checks.Category) bool {
 	switch c {
-	case checks.ImpostorCommit, checks.LockfileForgery, checks.MisleadingSHA:
+	case checks.ImpostorCommit, checks.LockfileForgery, checks.MisleadingSHA, checks.OnboardingRequired:
 		return true
 	}
 	return false

cmd/gh-actions-pin/lockrecovery.go

@@ -0,0 +1,97 @@
+package main
+
+import (
+	"fmt"
+	"os"
+
+	"github.com/cli/go-gh/v2/pkg/prompter"
+	"github.com/github/gh-actions-pin/internal/ui"
+	"github.com/spf13/cobra"
+	"golang.org/x/term"
+)
+
+// noInteractiveFlag reports the value of the persistent --no-interactive flag,
+// defaulting to false when it is not registered.
+func noInteractiveFlag(cmd *cobra.Command) bool {
+	v, _ := cmd.Flags().GetBool("no-interactive")
+	return v
+}
+
+// confirmer asks a yes/no question. Satisfied by *prompter.Prompter; an
+// interface so tests inject a fake without a TTY.
+type confirmer interface {
+	Confirm(prompt string, defaultValue bool) (bool, error)
+}
+
+// confirmFactory returns a confirmer and whether the session can prompt.
+// canPrompt is false in any non-interactive context (no TTY, CI), so the
+// recovery policy fails closed instead of blocking on input that never comes.
+type confirmFactory func() (confirmer, bool)
+
+// confirmFactoryHook is the confirm factory commands use to build the
+// corrupt-lockfile recovery policy. Production points at defaultConfirmFactory
+// (real terminal). Tests override it to drive the interactive delete-and-
+// recreate path without a TTY; the command tests run serially (t.Chdir) so a
+// package-level override with cleanup is safe.
+var confirmFactoryHook confirmFactory = defaultConfirmFactory
+
+// defaultConfirmFactory binds to the real terminal and renders to stderr so
+// `--json` stdout stays clean. It reports canPrompt only when both stdin and
+// stderr are TTYs and CI is unset — a CI runner with a stray TTY must never
+// be prompted.
+func defaultConfirmFactory() (confirmer, bool) {
+	if !term.IsTerminal(int(os.Stdin.Fd())) || !term.IsTerminal(int(os.Stderr.Fd())) || ciEnabled() {
+		return nil, false
+	}
+	return prompter.New(os.Stdin, os.Stderr, os.Stderr), true
+}
+
+// ciEnabled mirrors the CI convention used by internal/ui: most providers set
+// CI=true. A truthy CI value means no interactive prompts.
+func ciEnabled() bool {
+	v := os.Getenv("CI")
+	return v != "" && v != "0" && v != "false"
+}
+
+// lockRecovery decides what to do when the on-disk lockfile can't be parsed.
+// It returns (true, nil) when the lockfile was removed and loading should be
+// retried (the empty-lockfile path then recreates it), or a non-nil error to
+// abort the run (exit 2). It never silently accepts an unreadable lockfile.
+type lockRecovery func(lockPath string, parseErr error) (recovered bool, err error)
+
+// newLockRecovery builds the recovery policy. allowDelete is false for
+// read-only or relock commands that cannot rebuild a deleted lockfile
+// (`check --no-fix`, `update`); those always fail with a clear pointer. When
+// allowDelete is true (`check` fix mode), an interactive session is offered a
+// delete-and-recreate; non-interactive sessions (CI, --no-interactive) fail.
+func newLockRecovery(noInteractive bool, console *ui.UI, newConfirm confirmFactory, allowDelete bool) lockRecovery {
+	return func(lockPath string, parseErr error) (bool, error) {
+		if !allowDelete {
+			return false, fmt.Errorf("%w; run `gh actions-pin check` to rebuild it, or delete it by hand", parseErr)
+		}
+		var (
+			confirm   confirmer
+			canPrompt bool
+		)
+		if newConfirm != nil {
+			confirm, canPrompt = newConfirm()
+		}
+		if noInteractive || !canPrompt {
+			return false, fmt.Errorf("%w; delete it and re-run to recreate it, or fix it by hand", parseErr)
+		}
+		// Release the terminal so the prompt renders cleanly over any spinner.
+		console.StopProgress()
+		ok, err := confirm.Confirm(fmt.Sprintf("Lockfile %s is unreadable (%v). Delete and recreate it?", lockPath, parseErr), false)
+		if err != nil {
+			return false, err
+		}
+		if !ok {
+			return false, fmt.Errorf("%w; left in place", parseErr)
+		}
+		if err := os.Remove(lockPath); err != nil {
+			return false, fmt.Errorf("deleting unreadable lockfile %s: %w", lockPath, err)
+		}
+		console.TermNeutral("Deleted unreadable lockfile %s; it will be recreated.", lockPath)
+		return true, nil
+	}
+}