diff --git a/internal/config/config.go b/internal/config/config.go new file mode 100644 index 00000000..1663a872 --- /dev/null +++ b/internal/config/config.go @@ -0,0 +1,105 @@ +// Package config loads CLI configuration from file and environment. +package config + +import ( + "os" + "path/filepath" + "strconv" + + "github.com/github/gh-actions-pin/internal/tag" + "gopkg.in/yaml.v3" +) + +// Config holds process-wide settings loaded from the config file and +// environment variables. Create one via LoadConfig at startup and pass +// it through the pipeline. +type Config struct { + // Path is the resolved config file path (empty if none found). + Path string + // Cooldown controls how recently a tag must have been released to + // be excluded from upgrade suggestions. + Cooldown tag.CooldownConfig + // Workers is the concurrency limit for pool-parallelized phases. + // Defaults to 8; overridden by GH_ACTIONS_PIN_WORKERS. + Workers int + // StallHintMS is the stall-detection threshold in milliseconds. + // 0 disables the watcher. Overridden by GH_ACTIONS_PIN_STALL_HINT_MS. + StallHintMS int + // DebugProgress enables per-phase progress tracing. + DebugProgress bool +} + +// Load reads the config file and environment, returning a Config +// with sensible defaults for any unset values. +func Load() Config { + p := configPath() + c := Config{ + Path: p, + Workers: 8, + StallHintMS: -1, // sentinel: use pinpool default + DebugProgress: envBool("GH_ACTIONS_PIN_DEBUG_PROGRESS"), + } + + if v, err := strconv.Atoi(os.Getenv("GH_ACTIONS_PIN_WORKERS")); err == nil && v > 0 { + c.Workers = v + } + if v := os.Getenv("GH_ACTIONS_PIN_STALL_HINT_MS"); v != "" { + if ms, err := strconv.Atoi(v); err == nil { + c.StallHintMS = ms + } + } + + c.Cooldown = loadCooldownFromFile(p) + return c +} + +func envBool(key string) bool { + v := os.Getenv(key) + return v == "1" || v == "true" || v == "yes" +} + +// loadCooldownFromFile reads cooldown settings from the config file. +func loadCooldownFromFile(path string) tag.CooldownConfig { + cfg := tag.CooldownConfig{ + DefaultDays: 3, + RepoOverrides: make(map[string]int), + } + if path == "" { + return cfg + } + data, err := os.ReadFile(path) + if err != nil { + return cfg + } + var file struct { + CooldownDays int `yaml:"cooldown_days"` + Repos map[string]struct { + CooldownDays int `yaml:"cooldown_days"` + } `yaml:"repos"` + } + if err := yaml.Unmarshal(data, &file); err != nil { + return cfg + } + if file.CooldownDays > 0 { + cfg.DefaultDays = file.CooldownDays + } + for nwo, repoCfg := range file.Repos { + if repoCfg.CooldownDays >= 0 { + cfg.RepoOverrides[nwo] = repoCfg.CooldownDays + } + } + return cfg +} + +// configPath returns the path to the config file, respecting +// GH_ACTIONS_PIN_CONFIG for testing/demos. +func configPath() string { + if p := os.Getenv("GH_ACTIONS_PIN_CONFIG"); p != "" { + return p + } + home, err := os.UserHomeDir() + if err != nil { + return "" + } + return filepath.Join(home, ".config", "gh-actions-pin", "config.yml") +} diff --git a/internal/doctor/apply.go b/internal/doctor/apply.go deleted file mode 100644 index 8f68455b..00000000 --- a/internal/doctor/apply.go +++ /dev/null @@ -1,198 +0,0 @@ -package doctor - -import ( - "fmt" - "os" - - "github.com/github/gh-actions-pin/internal/lockfile" - "github.com/github/gh-actions-pin/internal/resolver" -) - -// isSHARef returns true if ref looks like a full commit SHA (40 or 64 hex chars). -func isSHARef(ref string) bool { - return lockfile.IsFullSHA(ref) -} - -// LooksLikeVersion returns true if ref starts with "v" followed by a digit. -func LooksLikeVersion(ref string) bool { - if len(ref) < 2 { - return false - } - return ref[0] == 'v' && ref[1] >= '0' && ref[1] <= '9' -} - -// applyPin runs the full pin flow on an unpinned workflow. -func (rem *Remediator) applyPin(wr WorkflowReport) error { - wf, err := lockfile.Load(wr.Path) - if err != nil { - return err - } - - deps, err := rem.resolver.ResolveAllRecursive(wr.ActionRefs) - if err != nil { - return fmt.Errorf("resolving actions: %w", err) - } - - // Check for impostor commits at pin time — don't let fork-network - // commits get pinned in the first place. Fail closed: if we can't - // verify reachability (e.g. branch_commits returns 429), refuse to - // pin rather than silently accepting a potentially poisoned commit. - // - // When reachability is disabled via config, skip this gate entirely — - // the branch_commits endpoint would return "disabled" for every dep, - // which would block all pinning. - if !rem.resolver.DisableReachability { - reachResults := rem.resolver.CheckReachabilityAll(deps) - for _, rr := range reachResults { - switch rr.Status { - case resolver.Unreachable: - rem.output.Error("%s/%s@%s: %s", rr.Owner, rr.Repo, rr.Ref, rr.Detail) - rem.Alerted++ - return fmt.Errorf("refusing to pin: impostor commit detected for %s/%s@%s", rr.Owner, rr.Repo, rr.Ref) - case resolver.ReachabilityUnknown: - rem.output.Error("%s/%s@%s: could not verify commit reachability — %s", rr.Owner, rr.Repo, rr.Ref, rr.Detail) - rem.Alerted++ - return fmt.Errorf("refusing to pin: cannot verify reachability for %s/%s@%s (try again later)", rr.Owner, rr.Repo, rr.Ref) - } - } - } - - // Narrow mutable version tags (v4, v4.2) to specific patch tags (v4.2.1) - // so that REF_MOVED signals are meaningful — patch tags should never move. - // Skip narrowing for same-owner internal repos — broad tags are fine - // within your own org's private actions. Public repos always narrow. - rewrites := make(map[string]string) - parentRewrites := make(map[string]string) - for i := range deps { - dep := &deps[i] - if !IsMutableVersionTag(dep.Ref) { - continue - } - owner, repo := dep.OwnerRepo() - if owner == "" { - continue - } - if rem.isSameOwner(owner) { - info, err := rem.tagLister.GetRepoInfo(owner, repo) - if err == nil && info.IsInternal() { - continue - } - } - patchTag, err := rem.tagLister.BestPatchTagForSHA(owner, repo, dep.SHA) - if err != nil || patchTag == "" { - continue - } - oldUses := dep.NWO + "@" + dep.Ref - newUses := dep.NWO + "@" + patchTag - rewrites[oldUses] = newUses - parentRewrites[dep.Key()] = dep.NWO + "@" + patchTag - rem.output.Detail(" %s → %s (pinning to patch version)", dep.Ref, patchTag) - dep.Ref = patchTag - } - - // Update parent map keys to reflect narrowed refs. - rem.resolver.RekeyParentMap(parentRewrites) - - // If we have rewrites, update the uses: lines in the workflow first. - if len(rewrites) > 0 { - content, _, err := wf.RewriteActionRefs(rewrites) - if err != nil { - return fmt.Errorf("rewriting refs to patch versions: %w", err) - } - if err := os.WriteFile(wr.Path, content, 0o644); err != nil { - return fmt.Errorf("writing file: %w", err) - } - // Re-load after rewrite so WriteDependencies sees the updated content. - wf, err = lockfile.Load(wr.Path) - if err != nil { - return err - } - } - - written, err := wf.WriteDependencies(deps, rem.resolver.ParentMap()) - if err != nil { - return fmt.Errorf("writing dependencies: %w", err) - } - if err := os.WriteFile(wr.Path, written, 0o644); err != nil { - return fmt.Errorf("writing file: %w", err) - } - - rem.output.Success("Pinned %d dependencies in %s", len(deps), wr.Path) - rem.Fixed++ - return nil -} - -// applySHAToTag rewrites a uses: line from @SHA to @tag and re-resolves. -func (rem *Remediator) applySHAToTag(wr WorkflowReport, dep *lockfile.Dependency, owner, repo, tag string) error { - wf, err := lockfile.Load(wr.Path) - if err != nil { - return err - } - - // Build replacement: old "owner/repo@sha" → "owner/repo@tag" - oldUses := dep.NWO + "@" + dep.Ref - newUses := dep.NWO + "@" + tag - - content, changed, err := wf.RewriteActionRefs(map[string]string{oldUses: newUses}) - if err != nil { - return fmt.Errorf("rewriting action refs: %w", err) - } - if changed == 0 { - rem.output.Warning("could not find %s in workflow to rewrite", oldUses) - rem.Skipped++ - return nil - } - - // Write the rewritten content, then re-parse and re-resolve to get correct lockfile. - if err := os.WriteFile(wr.Path, content, 0o644); err != nil { - return fmt.Errorf("writing file: %w", err) - } - - // Re-load, re-extract, re-resolve, re-write lockfile. - wf2, err := lockfile.Load(wr.Path) - if err != nil { - return err - } - refs, _, _ := wf2.ExtractActionRefs() - deps, err := rem.resolver.ResolveAllRecursive(refs) - if err != nil { - return fmt.Errorf("re-resolving after ref change: %w", err) - } - written, err := wf2.WriteDependencies(deps, rem.resolver.ParentMap()) - if err != nil { - return fmt.Errorf("writing dependencies: %w", err) - } - if err := os.WriteFile(wr.Path, written, 0o644); err != nil { - return fmt.Errorf("writing file: %w", err) - } - - rem.output.Success("Converted %s from SHA to %s and re-pinned", dep.NWO, tag) - rem.Fixed++ - return nil -} - -// applyReResolve re-resolves a single stale dependency. -func (rem *Remediator) applyReResolve(wr WorkflowReport, dep *lockfile.Dependency) error { - wf, err := lockfile.Load(wr.Path) - if err != nil { - return err - } - - refs, _, _ := wf.ExtractActionRefs() - deps, err := rem.resolver.ResolveAllRecursive(refs) - if err != nil { - return fmt.Errorf("resolving actions: %w", err) - } - - written, err := wf.WriteDependencies(deps, rem.resolver.ParentMap()) - if err != nil { - return fmt.Errorf("writing dependencies: %w", err) - } - if err := os.WriteFile(wr.Path, written, 0o644); err != nil { - return fmt.Errorf("writing file: %w", err) - } - - rem.output.Success("Updated %s to latest resolution", dep.Key()) - rem.Fixed++ - return nil -} diff --git a/internal/doctor/compare.go b/internal/doctor/compare.go deleted file mode 100644 index d9d71cc3..00000000 --- a/internal/doctor/compare.go +++ /dev/null @@ -1,105 +0,0 @@ -package doctor - -import ( - "fmt" - "strings" - - "github.com/github/gh-actions-pin/internal/lockfile" -) - -// compareSnapshots compares pinned dependencies against a live resolution and -// returns findings for any discrepancies: ref moves, ref changes, and stale deps. -func compareSnapshots(path string, existing, live []lockfile.Dependency, directNWOs map[string]bool) []Finding { - liveByKey := make(map[string]lockfile.Dependency, len(live)) - liveByNWO := make(map[string][]lockfile.Dependency, len(live)) - for _, dep := range live { - liveByKey[dep.Key()] = dep - liveByNWO[dep.NWO] = append(liveByNWO[dep.NWO], dep) - } - - var findings []Finding - for _, pinned := range existing { - if lockfile.IsFullSHA(pinned.Ref) { - continue - } - - resolved, ok := matchLiveDep(pinned, liveByKey, liveByNWO) - if !ok { - findings = append(findings, unmatchedFindings(path, pinned, liveByNWO, directNWOs)...) - continue - } - if !strings.EqualFold(pinned.SHA, resolved.SHA) { - resolvedCopy := resolved - findings = append(findings, Finding{ - WorkflowPath: path, - Category: CategoryRefMoved, - Severity: SeverityWarning, - Dependency: &pinned, - Detail: fmt.Sprintf("pinned %s but ref now resolves to %s", pinned.SHA[:12], resolvedCopy.SHA[:12]), - Remediation: fmt.Sprintf("update to %s with `gh actions-pin upgrade`", resolvedCopy.SHA[:12]), - LiveSHA: resolvedCopy.SHA, - }) - } - } - return findings -} - -// matchLiveDep tries to find a live dependency matching the pinned one. -// First by exact key, then fuzzy by NWO (same SHA or narrowed version). -func matchLiveDep(pinned lockfile.Dependency, byKey map[string]lockfile.Dependency, byNWO map[string][]lockfile.Dependency) (lockfile.Dependency, bool) { - if dep, ok := byKey[pinned.Key()]; ok { - return dep, true - } - if candidates, has := byNWO[pinned.NWO]; has { - for _, cand := range candidates { - if strings.EqualFold(cand.SHA, pinned.SHA) { - return cand, true - } - if IsNarrowedVersion(cand.Ref, pinned.Ref) { - return cand, true - } - } - } - return lockfile.Dependency{}, false -} - -// unmatchedFindings produces findings for a pinned dep that has no live match: -// either a ref change (direct dep with different ref) or stale (orphaned). -func unmatchedFindings(path string, pinned lockfile.Dependency, liveByNWO map[string][]lockfile.Dependency, directNWOs map[string]bool) []Finding { - if directNWOs[pinned.NWO] { - if candidates, has := liveByNWO[pinned.NWO]; has && len(candidates) > 0 { - newDep := candidates[0] - if newDep.Ref != pinned.Ref { - refOwner, refRepo := pinned.OwnerRepo() - return []Finding{{ - WorkflowPath: path, - Category: CategoryRefChanged, - Severity: SeverityWarning, - Dependency: &pinned, - ActionRef: &lockfile.ActionRef{ - Owner: refOwner, - Repo: refRepo, - Ref: newDep.Ref, - }, - Detail: fmt.Sprintf("ref changed from %s to %s in workflow — re-pin to update", pinned.Ref, newDep.Ref), - Remediation: "re-pin to match the new ref", - }} - } - } - } - - detail := "no longer in workflow — will be cleaned up" - remediation := "re-resolve to remove orphaned dependency" - if !directNWOs[pinned.NWO] { - detail = "transitive dependency no longer discovered from upstream composite action" - remediation = "re-resolve to clean up" - } - return []Finding{{ - WorkflowPath: path, - Category: CategoryStale, - Severity: SeverityInfo, - Dependency: &pinned, - Detail: detail, - Remediation: remediation, - }} -} diff --git a/internal/doctor/config.go b/internal/doctor/config.go deleted file mode 100644 index 1cc91a3b..00000000 --- a/internal/doctor/config.go +++ /dev/null @@ -1,52 +0,0 @@ -package doctor - -import ( - "os" - "path/filepath" - - "gopkg.in/yaml.v3" -) - -// configPath returns the path to the config file, respecting -// GH_ACTIONS_PIN_CONFIG for testing/demos. -func configPath() string { - if p := os.Getenv("GH_ACTIONS_PIN_CONFIG"); p != "" { - return p - } - home, err := os.UserHomeDir() - if err != nil { - return "" - } - return filepath.Join(home, ".config", "gh-actions-pin", "config.yml") -} - -// ReachabilityEnabled reads the reachability_check flag from the config file. -// Defaults to false (disabled). -// -// The reachability check uses the undocumented /{owner}/{repo}/branch_commits/{sha} -// web endpoint to detect fork-network injection (impostor commits). This endpoint -// is aggressive with 429 rate limits and has no public API equivalent. Until GitHub -// exposes a documented "commit reachability" or "commit contains" API, this feature -// is dead in the water for most users and disabled by default. -// -// To enable: -// -// # ~/.config/gh-actions-pin/config.yml -// reachability_check: true -func ReachabilityEnabled() bool { - p := configPath() - if p == "" { - return false - } - data, err := os.ReadFile(p) - if err != nil { - return false - } - var file struct { - ReachabilityCheck bool `yaml:"reachability_check"` - } - if err := yaml.Unmarshal(data, &file); err != nil { - return false - } - return file.ReachabilityCheck -} diff --git a/internal/doctor/diagnose.go b/internal/doctor/diagnose.go deleted file mode 100644 index 3147fb38..00000000 --- a/internal/doctor/diagnose.go +++ /dev/null @@ -1,316 +0,0 @@ -package doctor - -import ( - "fmt" - - "github.com/github/gh-actions-pin/internal/lockfile" - "github.com/github/gh-actions-pin/internal/resolver" - "github.com/github/gh-actions-pin/internal/ui" -) - -// Diagnose scans a set of workflows and produces findings for each. -// It performs no output — purely analytical. -func Diagnose(paths []string, r *resolver.Resolver) *Report { - report := &Report{} - for _, path := range paths { - wr := diagnoseOneWorkflow(path, r) - report.Workflows = append(report.Workflows, wr) - } - return report -} - -func diagnoseOneWorkflow(path string, r *resolver.Resolver) WorkflowReport { - wr := WorkflowReport{Path: path} - - wf, err := lockfile.Load(path) - if err != nil { - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategoryNotPinned, - Severity: SeverityError, - Detail: fmt.Sprintf("failed to load workflow: %s", err), - }) - return wr - } - - refs, _, parseWarnings := wf.ExtractActionRefs() - wr.ActionRefs = refs - wr.ParseWarnings = parseWarnings - - // No action refs → run-only workflow, nothing to do. - if len(refs) == 0 { - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategoryRunOnly, - Severity: SeverityOK, - Detail: "no action references found", - }) - return wr - } - - existingDeps, depsErr := wf.ReadDependencies() - if depsErr != nil { - // Malformed dependencies: section — report as error, don't fold into "not pinned". - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategoryNotPinned, - Severity: SeverityError, - Detail: fmt.Sprintf("failed to read dependencies: %s", depsErr), - Remediation: "fix or regenerate the dependencies: section with `gh actions-pin`", - }) - return wr - } - - if len(existingDeps) == 0 { - // No lockfile. Check if any action refs are already SHA-pinned — - // those should be SHAAsRef, not NotPinned. - var shaRefs, tagRefs []lockfile.ActionRef - for _, ref := range refs { - if lockfile.IsFullSHA(ref.Ref) { - shaRefs = append(shaRefs, ref) - } else { - tagRefs = append(tagRefs, ref) - } - } - - for _, ref := range shaRefs { - nwo := ref.NWO() - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategorySHAAsRef, - Severity: SeverityWarning, - ActionRef: &ref, - Dependency: &lockfile.Dependency{ - NWO: nwo, - Ref: ref.Ref, - SHA: ref.Ref, - }, - Detail: "pinned to a bare SHA without a tag ref — weakens supply-chain security", - Remediation: fmt.Sprintf( - "pin to a tag instead: https://github.com/%s/releases", nwo), - }) - } - - if len(tagRefs) > 0 { - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategoryNotPinned, - Severity: SeverityError, - Detail: fmt.Sprintf("%d %s not pinned", len(tagRefs), ui.Pluralize(len(tagRefs), "action", "actions")), - Remediation: "pin with `gh actions-pin`", - }) - } - - if len(wr.Findings) == 0 { - return wr - } - } - wr.Deps = existingDeps - - // Build direct NWO set from workflow uses: lines. - directNWOs := make(map[string]bool) - for _, ref := range refs { - directNWOs[ref.NWO()] = true - } - - // Build dependency inventory with direct/transitive classification. - for _, dep := range existingDeps { - wr.Inventory = append(wr.Inventory, InventoryEntry{ - Dep: dep, - File: path, - Direct: directNWOs[dep.NWO], - }) - } - - // Check for SHA-as-ref anti-pattern in existing deps (direct only). - for i := range existingDeps { - dep := &existingDeps[i] - if !directNWOs[dep.NWO] { - continue - } - if lockfile.IsFullSHA(dep.Ref) { - owner, repo := dep.OwnerRepo() - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategorySHAAsRef, - Severity: SeverityWarning, - Dependency: dep, - Detail: "pinned to a bare SHA without a tag ref — weakens supply-chain security", - Remediation: fmt.Sprintf( - "pin to a tag instead: https://github.com/%s/%s/releases", owner, repo), - }) - } - } - - // Re-resolve to check for staleness and tampering. - liveDeps, err := r.ResolveAllRecursive(refs) - if err != nil { - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategoryValid, - Severity: SeverityWarning, - Detail: fmt.Sprintf("could not re-resolve actions: %s", err), - }) - return wr - } - - // MISLEADING_SHA: detect refs that look like SHAs but resolve to different commits. - for _, mismatch := range lockfile.CheckSHARefMismatches(liveDeps) { - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategoryMisleadingSHA, - Severity: SeverityError, - Dependency: &mismatch.Dep, - Detail: fmt.Sprintf("ref %s resolved to %s", mismatch.Dep.Ref, mismatch.ResolvedAs), - }) - } - - // Compare pinned deps against live resolution. - wr.Findings = append(wr.Findings, compareSnapshots(path, existingDeps, liveDeps, directNWOs)...) - - // LOCKFILE_FORGERY: for each REF_MOVED finding, check if the pinned SHA - // is actually an ancestor of the live SHA. If not, the lockfile entry was - // likely injected or tampered with (or upstream rewrote history). - liveByKey := make(map[string]lockfile.Dependency, len(liveDeps)) - for _, dep := range liveDeps { - liveByKey[dep.Key()] = dep - } - for i := range wr.Findings { - f := &wr.Findings[i] - if f.Category != CategoryRefMoved || f.Dependency == nil { - continue - } - live, ok := liveByKey[f.Dependency.Key()] - if !ok { - continue - } - owner, repo := f.Dependency.OwnerRepo() - status, detail := r.CheckAncestry(owner, repo, f.Dependency.SHA, live.SHA) - switch status { - case resolver.AncestryNotAncestor: - f.Category = CategoryLockfileForgery - f.Severity = SeverityError - f.Detail = fmt.Sprintf("pinned %s is not an ancestor of %s — %s", - f.Dependency.SHA[:12], live.SHA[:12], detail) - f.Remediation = "investigate immediately — the lockfile may have been tampered with" - case resolver.AncestryUnknown: - // Fail open: keep as REF_MOVED, add a note about the ancestry check. - f.Detail += fmt.Sprintf(" (ancestry check inconclusive: %s)", detail) - } - // AncestryConfirmed: leave as REF_MOVED — legitimate tag movement. - } - - // Build set of dep keys already promoted to LOCKFILE_FORGERY so we don't - // also flag them as IMPOSTER_COMMIT — the two are mutually exclusive. - forgeryKeys := make(map[string]bool) - for _, f := range wr.Findings { - if f.Category == CategoryLockfileForgery && f.Dependency != nil { - forgeryKeys[f.Dependency.Key()] = true - } - } - - // Build set of NWOs with ref-changed findings to avoid duplicate "not pinned" findings. - refChangedNWOs := make(map[string]bool) - for _, f := range wr.Findings { - if f.Category == CategoryRefChanged && f.Dependency != nil { - refChangedNWOs[f.Dependency.NWO] = true - } - } - - // Check for missing deps (action in workflow but not pinned). - depsByKey := make(map[string]lockfile.Dependency, len(existingDeps)) - for _, dep := range existingDeps { - depsByKey[dep.Key()] = dep - } - for _, ref := range refs { - key := ref.FullName() + "@" + ref.Ref - if _, ok := depsByKey[key]; !ok { - if refChangedNWOs[ref.NWO()] { - continue - } - refCopy := ref - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategoryNotPinned, - Severity: SeverityError, - ActionRef: &refCopy, - Detail: "used in workflow but not pinned", - Remediation: "pin with `gh actions-pin`", - }) - } - } - - // Reachability checks (skip entirely when disabled — no warnings, no findings). - if !r.DisableReachability { - reachResults := r.CheckReachabilityAll(existingDeps) - // Build dep lookup by key for attaching to reachability findings. - depByKey := make(map[string]lockfile.Dependency, len(existingDeps)) - for _, d := range existingDeps { - depByKey[d.Key()] = d - } - for _, rr := range reachResults { - var depPtr *lockfile.Dependency - if d, ok := depByKey[rr.DepKey]; ok { - depPtr = &d - } - switch rr.Status { - case resolver.Unreachable: - if forgeryKeys[rr.DepKey] { - continue // already flagged as LOCKFILE_FORGERY — reachability is implied - } - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategoryImposterCommit, - Severity: SeverityError, - Dependency: depPtr, - Detail: rr.Detail, - }) - case resolver.ReachabilityUnknown: - isTransitive := !directNWOs[rr.Owner+"/"+rr.Repo] - parentNWO := "" - if isTransitive { - if parents := r.ParentMap()[rr.DepKey]; len(parents) > 0 { - parentNWO = parents[0] - } - } - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategoryValid, - Severity: SeverityWarning, - Detail: rr.Detail, - Dependency: &lockfile.Dependency{ - NWO: rr.Owner + "/" + rr.Repo, - Ref: rr.Ref, - SHA: rr.SHA, - }, - ParentNWO: parentNWO, - Remediation: func() string { - if isTransitive { - return "transitive dependency pinned to a bare SHA — reachability cannot be verified" - } - return "reachability check inconclusive" - }(), - }) - } - } - } - - // If no issues found, mark as valid. - hasIssues := false - for _, f := range wr.Findings { - if f.Severity == SeverityError || (f.Category != CategoryValid && f.Category != CategoryRunOnly && f.Severity == SeverityWarning) { - hasIssues = true - break - } - } - if !hasIssues { - wr.Findings = append(wr.Findings, Finding{ - WorkflowPath: path, - Category: CategoryValid, - Severity: SeverityOK, - Detail: "all dependencies pinned and verified", - }) - } - - return wr -} diff --git a/internal/doctor/is_upgrade_test.go b/internal/doctor/is_upgrade_test.go deleted file mode 100644 index d6b3fbbb..00000000 --- a/internal/doctor/is_upgrade_test.go +++ /dev/null @@ -1,59 +0,0 @@ -package doctor - -import ( - "fmt" - "testing" -) - -func TestIsUpgrade_Cases(t *testing.T) { - cases := []struct { - current, latest string - want bool - }{ - {"v4.0.0", "v4", false}, - {"v4", "v4", false}, - {"v3.1.1", "v3", false}, - {"v3.0.1", "v3", false}, - {"v2.2.0", "v2", false}, - {"v2.0.0", "v2", false}, - {"v2.2.1", "v2.2.2", true}, - {"v5", "v6", true}, - {"main", "v1.1.0", true}, - {"v4", "codeql-bundle-v2.6.0-beta.1", false}, - {"v4.0.0", "v4.0.0", false}, - {"v1.1.0", "v1.1.0", false}, - {"v3", "v3.35.2", true}, - } - for _, tc := range cases { - got := IsUpgrade(tc.current, tc.latest) - if got != tc.want { - t.Errorf("IsUpgrade(%q, %q) = %v, want %v", tc.current, tc.latest, got, tc.want) - } else { - fmt.Printf("✓ %-20s → %-30s upgrade=%v\n", tc.current, tc.latest, got) - } - } -} - -func TestIsNarrowedVersion(t *testing.T) { - cases := []struct { - mutable, narrowed string - want bool - }{ - {"v4", "v4.1.0", true}, - {"v4", "v4.0.0", true}, - {"v4.2", "v4.2.1", true}, - {"v4.2", "v4.2.0", true}, - {"v4", "v5.0.0", false}, // different major - {"v4.2", "v4.3.0", false}, // different minor - {"v4.1.0", "v4.1.0", true}, // identity (full semver is its own narrowing) - {"v4", "v4", false}, // mutable→mutable, not narrowed - {"main", "v4.1.0", false}, // non-version - {"v4", "v4.1.0-beta", false}, // pre-release - } - for _, tc := range cases { - got := IsNarrowedVersion(tc.mutable, tc.narrowed) - if got != tc.want { - t.Errorf("IsNarrowedVersion(%q, %q) = %v, want %v", tc.mutable, tc.narrowed, got, tc.want) - } - } -} diff --git a/internal/doctor/picker.go b/internal/doctor/picker.go deleted file mode 100644 index 819e9131..00000000 --- a/internal/doctor/picker.go +++ /dev/null @@ -1,133 +0,0 @@ -package doctor - -import ( - "errors" - "fmt" -) - -// pickerTag is a tag option in a tag picker, with display metadata. -type pickerTag struct { - Name string - IsInstalled bool // currently pinned SHA points to this tag - IsImmutable bool - IsRelease bool - IsMajor bool -} - -// pickerAction is the result of a tag picker selection. -type pickerAction int - -const ( - pickerApply pickerAction = iota // user selected a tag - pickerSkip // user chose "Skip" - pickerShowAll // user chose "Show all tags" - pickerOpenReleases // user chose "Open releases" - pickerDefaultBranch // user chose the default branch -) - -// pickerResult holds the outcome of a picker prompt. -type pickerResult struct { - Action pickerAction - Tag string // populated for pickerDefaultBranch - TagIndex int // index into the tag slice for pickerApply -} - -// tagLabel renders a single tag option with hyperlinks and decorators. -func (rem *Remediator) tagLabel(owner, repo string, tag pickerTag, recommend bool) string { - tagURL := TagURL(owner, repo, tag.Name) - label := rem.output.Hyperlink(tag.Name, tagURL) - if tag.IsInstalled { - label += " 📌 current" - } - if !rem.isSameOwner(owner) { - if tag.IsImmutable { - label += " 🔒 immutable" - } else if tag.IsRelease { - label += " (release)" - } - } - if recommend { - label += " (recommended)" - } - if age := FormatTagAge(rem.tagLister.ReleaseDate(owner, repo, tag.Name)); age != "" { - label += " " + age - } - return label -} - -// defaultBranchOption appends a default branch entry if this is a same-owner repo. -// Returns the index of the default branch option (-1 if not added) and the updated options slice. -func (rem *Remediator) defaultBranchOption(options []string, owner, repo string) ([]string, int) { - if !rem.isSameOwner(owner) { - return options, -1 - } - info, err := rem.tagLister.GetRepoInfo(owner, repo) - if err != nil { - return options, -1 - } - branchURL := fmt.Sprintf("https://github.com/%s/%s/tree/%s", owner, repo, info.DefaultBranch) - label := rem.output.Hyperlink(info.DefaultBranch, branchURL) + " (default branch)" - if age := FormatTagAge(info.PushedAt); age != "" { - label += " last push " + age - } - options = append(options, label) - return options, len(options) - 1 -} - -// sentinel options appended after tags + default branch. -type pickerSentinels struct { - ShowAll bool // "Show all tags" - OpenReleases string // "Open releases → URL" (empty = disabled) -} - -// runPicker shows a tag selection prompt and returns the user's choice. -// tagCount is the number of tag options at the front of the options slice -// (before default branch and sentinel options). -func (rem *Remediator) runPicker(title string, options []string, tagCount int, defaultBranchIdx int, sentinels pickerSentinels) (pickerResult, error) { - // Append sentinel options. - if sentinels.ShowAll { - options = append(options, "Show all tags") - } - if sentinels.OpenReleases != "" { - options = append(options, fmt.Sprintf("Open releases → %s", sentinels.OpenReleases)) - } - options = append(options, "Skip this action") - - idx, err := rem.prompter.Select(title, options) - if err != nil { - if errors.Is(err, ErrAborted) { - return pickerResult{}, ErrAborted - } - return pickerResult{Action: pickerSkip}, nil - } - if idx < 0 || idx >= len(options) { - return pickerResult{Action: pickerSkip}, nil - } - - // Skip is always last. - if idx == len(options)-1 { - return pickerResult{Action: pickerSkip}, nil - } - - // Second-to-last sentinel. - if idx == len(options)-2 { - if sentinels.OpenReleases != "" { - return pickerResult{Action: pickerOpenReleases}, nil - } - if sentinels.ShowAll { - return pickerResult{Action: pickerShowAll}, nil - } - } - - // Third-to-last when both sentinels are present. - if sentinels.ShowAll && sentinels.OpenReleases != "" && idx == len(options)-3 { - return pickerResult{Action: pickerShowAll}, nil - } - - // Default branch. - if idx == defaultBranchIdx { - return pickerResult{Action: pickerDefaultBranch}, nil - } - - return pickerResult{Action: pickerApply, TagIndex: idx}, nil -} diff --git a/internal/doctor/prompt.go b/internal/doctor/prompt.go deleted file mode 100644 index 0c5bbb90..00000000 --- a/internal/doctor/prompt.go +++ /dev/null @@ -1,193 +0,0 @@ -package doctor - -import ( - "errors" - "fmt" - "io" - "os" - - "charm.land/huh/v2" - "golang.org/x/term" -) - -// ErrAborted is returned when the user presses Ctrl+C to abort. -var ErrAborted = errors.New("aborted by user") - -// Prompter abstracts interactive user prompts for testing and non-TTY fallback. -type Prompter interface { - // Confirm asks a yes/no question. - Confirm(message string, defaultVal bool) (bool, error) - // Select presents a single-choice menu. Returns the selected index. - Select(message string, options []string) (int, error) - // MultiSelect presents a multi-choice menu. Returns selected indices. - MultiSelect(message string, options []string) ([]int, error) - // IsInteractive returns true if this prompter can ask questions. - IsInteractive() bool -} - -// HuhPrompter implements Prompter using the huh library (same as gh CLI). -type HuhPrompter struct { - out io.Writer - isTerminal func() bool -} - -// NewHuhPrompter creates an interactive prompter that writes to stderr. -func NewHuhPrompter() *HuhPrompter { - return &HuhPrompter{ - out: os.Stderr, - isTerminal: func() bool { return term.IsTerminal(int(os.Stderr.Fd())) }, - } -} - -// NewHuhPrompterWithWriter creates a prompter that writes to the given writer -// and uses the provided function for TTY detection. -func NewHuhPrompterWithWriter(w io.Writer, isTerminal func() bool) *HuhPrompter { - return &HuhPrompter{out: w, isTerminal: isTerminal} -} - -func (p *HuhPrompter) IsInteractive() bool { - return p.isTerminal() -} - -func (p *HuhPrompter) Confirm(message string, defaultVal bool) (bool, error) { - result := defaultVal - err := huh.NewForm( - huh.NewGroup( - huh.NewConfirm(). - Title(message). - Value(&result). - Affirmative("Yes"). - Negative("No"), - ), - ).WithOutput(p.out).Run() - if err != nil { - if errors.Is(err, huh.ErrUserAborted) { - return false, ErrAborted - } - return false, err - } - return result, nil -} - -func (p *HuhPrompter) Select(message string, options []string) (int, error) { - if len(options) == 0 { - return -1, fmt.Errorf("no options provided") - } - var selected int - huhOptions := make([]huh.Option[int], len(options)) - for i, opt := range options { - huhOptions[i] = huh.NewOption(opt, i) - } - - err := huh.NewForm( - huh.NewGroup( - huh.NewSelect[int](). - Title(message). - Options(huhOptions...). - Value(&selected), - ), - ).WithOutput(p.out).Run() - if err != nil { - if errors.Is(err, huh.ErrUserAborted) { - return -1, ErrAborted - } - return -1, err - } - return selected, nil -} - -func (p *HuhPrompter) MultiSelect(message string, options []string) ([]int, error) { - if len(options) == 0 { - return nil, fmt.Errorf("no options provided") - } - var selected []int - huhOptions := make([]huh.Option[int], len(options)) - for i, opt := range options { - huhOptions[i] = huh.NewOption(opt, i) - } - - err := huh.NewForm( - huh.NewGroup( - huh.NewMultiSelect[int](). - Title(message). - Options(huhOptions...). - Value(&selected), - ), - ).WithOutput(p.out).Run() - if err != nil { - if errors.Is(err, huh.ErrUserAborted) { - return nil, ErrAborted - } - return nil, err - } - return selected, nil -} - -// TestPrompter is a non-interactive prompter for tests. -// It plays back pre-configured responses in order. -type TestPrompter struct { - confirmResponses []bool - selectResponses []int - multiSelectResponses [][]int - confirmIdx int - selectIdx int - multiSelectIdx int -} - -// NewTestPrompter creates a test prompter with canned responses. -func NewTestPrompter(confirms []bool, selects []int) *TestPrompter { - return &TestPrompter{ - confirmResponses: confirms, - selectResponses: selects, - } -} - -// NewTestPrompterFull creates a test prompter with all response types. -func NewTestPrompterFull(confirms []bool, selects []int, multiSelects [][]int) *TestPrompter { - return &TestPrompter{ - confirmResponses: confirms, - selectResponses: selects, - multiSelectResponses: multiSelects, - } -} - -func (p *TestPrompter) IsInteractive() bool { return true } - -func (p *TestPrompter) Confirm(message string, defaultVal bool) (bool, error) { - if p.confirmIdx >= len(p.confirmResponses) { - return defaultVal, nil - } - result := p.confirmResponses[p.confirmIdx] - p.confirmIdx++ - return result, nil -} - -func (p *TestPrompter) Select(message string, options []string) (int, error) { - if p.selectIdx >= len(p.selectResponses) { - return 0, nil - } - result := p.selectResponses[p.selectIdx] - p.selectIdx++ - return result, nil -} - -func (p *TestPrompter) MultiSelect(message string, options []string) ([]int, error) { - if p.multiSelectIdx >= len(p.multiSelectResponses) { - return nil, nil - } - result := p.multiSelectResponses[p.multiSelectIdx] - p.multiSelectIdx++ - return result, nil -} - -// NoopPrompter always returns defaults — used in non-interactive mode. -type NoopPrompter struct{} - -func (p *NoopPrompter) IsInteractive() bool { return false } -func (p *NoopPrompter) Confirm(message string, defaultVal bool) (bool, error) { return defaultVal, nil } -func (p *NoopPrompter) Select(message string, options []string) (int, error) { - return -1, fmt.Errorf("non-interactive") -} -func (p *NoopPrompter) MultiSelect(message string, options []string) ([]int, error) { - return nil, fmt.Errorf("non-interactive") -} diff --git a/internal/doctor/remediate.go b/internal/doctor/remediate.go deleted file mode 100644 index 70acde55..00000000 --- a/internal/doctor/remediate.go +++ /dev/null @@ -1,744 +0,0 @@ -package doctor - -import ( - "errors" - "fmt" - "os" - "os/exec" - "strings" - - "github.com/cli/go-gh/v2/pkg/api" - "github.com/github/gh-actions-pin/internal/lockfile" - "github.com/github/gh-actions-pin/internal/resolver" - "github.com/github/gh-actions-pin/internal/ui" -) - -// RemediateOptions controls the remediation flow. -type RemediateOptions struct { - Interactive bool // true when stderr is a TTY - RepoOwner string // owner of the repo being scanned (for same-owner detection) -} - -// Remediator walks through findings and applies fixes interactively. -type Remediator struct { - prompter Prompter - resolver *resolver.Resolver - tagLister *TagLister - output *ui.UI - opts RemediateOptions - - state sessionState - - // How many remaining occurrences of each choiceKey across all workflows. - remaining map[string]int - - // Counters for summary. - Fixed int - Skipped int - Alerted int - SkippedDeps []string // unique dep keys that were skipped (for summary) - AlertedDeps []string // dep keys that triggered security alerts (deduplicated) -} - -// NewRemediator creates a new Remediator. -func NewRemediator(p Prompter, r *resolver.Resolver, client *api.RESTClient, out *ui.UI, opts RemediateOptions) *Remediator { - return &Remediator{ - prompter: p, - resolver: r, - tagLister: NewTagLister(client), - output: out, - opts: opts, - state: newSessionState(), - } -} - -// isSameOwner returns true if the action's owner matches the repo being scanned, -// meaning it's an internal/first-party action where default-branch pinning is sensible. -func (rem *Remediator) isSameOwner(actionOwner string) bool { - return rem.opts.RepoOwner != "" && strings.EqualFold(rem.opts.RepoOwner, actionOwner) -} - -// offerApplyAll checks if this dep appears in more workflows and auto-applies -// the same choice everywhere. No prompt needed — same dep, same tag, just do it. -func (rem *Remediator) offerApplyAll(dep *lockfile.Dependency, tag string) { - key := choiceKey(dep) - rem.remaining[key]-- - others := rem.remaining[key] - if others <= 0 { - return - } - - rem.output.Detail(" ↳ applying %s to %d remaining %s", tag, others, ui.Pluralize(others, "file", "files")) - rem.state.recordChoice(dep, tag) -} - -// Remediate walks through a report and handles each workflow that needs attention. -func (rem *Remediator) Remediate(report *Report) error { - actionable := report.WorkflowsNeedingAttention() - if len(actionable) == 0 { - return nil - } - - // Pre-scan: count how many times each dep appears so we can offer "apply to all". - rem.remaining = make(map[string]int) - for _, wr := range actionable { - for _, f := range wr.Findings { - if f.Category == CategorySHAAsRef && f.Dependency != nil { - rem.remaining[choiceKey(f.Dependency)]++ - } - } - } - - for _, wr := range actionable { - if err := rem.remediateWorkflow(wr); err != nil { - return err - } - } - return nil -} - -func (rem *Remediator) depKey(f Finding) string { - if f.Dependency != nil { - return f.Dependency.Key() - } - if f.ActionRef != nil { - return f.ActionRef.FullName() + "@" + f.ActionRef.Ref - } - return "" -} - -func (rem *Remediator) addAlertedDep(f Finding) { - key := rem.depKey(f) - for _, k := range rem.AlertedDeps { - if k == key { - return - } - } - rem.AlertedDeps = append(rem.AlertedDeps, key) -} - -// skipDep records a dependency as skipped (needs interactive resolution). -func (rem *Remediator) skipDep(dep *lockfile.Dependency) { - key := dep.Key() - rem.output.Skip("%s: requires interactive tag selection", key) - rem.state.choices[key] = "skipped" - rem.SkippedDeps = append(rem.SkippedDeps, key) - rem.Skipped++ -} - -func (rem *Remediator) repoNWO(f Finding) string { - if f.Dependency != nil { - owner, repo := f.Dependency.OwnerRepo() - if owner != "" { - return owner + "/" + repo - } - } - if f.ActionRef != nil { - return f.ActionRef.Owner + "/" + f.ActionRef.Repo - } - return "" -} - -// pinPromptTitle returns the Select prompt title annotated with repo visibility. -func (rem *Remediator) pinPromptTitle(nwo, owner, repo string) string { - title := fmt.Sprintf("Pin %s to which tag?", nwo) - if info, err := rem.tagLister.GetRepoInfo(owner, repo); err == nil { - title += fmt.Sprintf(" (%s)", info.VisibilityLabel()) - } - return title -} - -func (rem *Remediator) remediateWorkflow(wr WorkflowReport) error { - headerPrinted := false - ensureHeader := func() { - if !headerPrinted { - rem.output.Header("%s", wr.Path) - headerPrinted = true - } - } - - // In interactive mode, always show the header. - if rem.prompter.IsInteractive() { - ensureHeader() - } - - first := true - for _, finding := range wr.Findings { - if finding.Category == CategoryValid || finding.Category == CategoryRunOnly || finding.Category == CategoryRefMoved { - continue - } - - // For non-interactive SHA_AS_REF, check if this dep was already printed. - // If so, skip silently (no header, no blank line). - if !rem.prompter.IsInteractive() && finding.Category == CategorySHAAsRef { - if finding.Dependency != nil { - if _, seen := rem.state.choices[finding.Dependency.Key()]; seen { - rem.Skipped++ - continue - } - } - } - - ensureHeader() - if !first { - rem.output.Blank() - } - first = false - - switch finding.Category { - case CategoryNotPinned: - // Re-read workflow from disk — earlier SHA→tag conversions may have - // changed refs since diagnosis time. - if wf, err := lockfile.Load(wr.Path); err == nil { - if freshRefs, _, _ := wf.ExtractActionRefs(); len(freshRefs) > 0 { - wr.ActionRefs = freshRefs - } - } - if err := rem.handleNotPinned(wr); err != nil { - return err - } - return nil // NotPinned is workflow-level, one pass is enough. - - case CategorySHAAsRef: - if err := rem.handleSHAAsRef(wr, finding); err != nil { - return err - } - - case CategoryStale: - if err := rem.handleStale(wr, finding); err != nil { - return err - } - - case CategoryRefChanged: - if err := rem.handleRefChanged(wr, finding); err != nil { - return err - } - - case CategoryImposterCommit: - rem.output.Error("%s", finding.Detail) - rem.output.Hint("This may indicate a fork-network injection attack. Do not auto-fix.") - rem.Alerted++ - rem.addAlertedDep(finding) - - case CategoryLockfileForgery: - rem.output.Error("LOCKFILE_FORGERY %s: %s", rem.depKey(finding), finding.Detail) - rem.output.Hint("The pinned SHA was never in this ref's lineage — possible lockfile tampering.") - rem.Alerted++ - rem.addAlertedDep(finding) - - case CategoryMisleadingSHA: - rem.output.Error("MISLEADING_SHA %s: %s", rem.depKey(finding), finding.Detail) - rem.output.Hint("This ref may be a deceptive branch or tag name masquerading as a commit hash.") - rem.Alerted++ - rem.addAlertedDep(finding) - } - } - - if headerPrinted { - rem.output.Blank() - } - return nil -} - -func (rem *Remediator) handleNotPinned(wr WorkflowReport) error { - rem.output.Warning("%d %s found but not pinned", len(wr.ActionRefs), ui.Pluralize(len(wr.ActionRefs), "action", "actions")) - - if !rem.prompter.IsInteractive() { - // Non-interactive: auto-pin all refs (ref→SHA is deterministic). - rem.state.markRefsApproved(wr.ActionRefs) - return rem.applyPin(wr) - } - - // For internal repos, offer the default branch as an alternative ref. - wr = rem.offerDefaultBranch(wr) - - // If all refs in this workflow were already approved in a prior workflow, auto-apply. - if rem.state.allRefsApproved(wr.ActionRefs) { - rem.output.Detail(" ↳ all actions already approved — auto-pinning") - return rem.applyPin(wr) - } - - // Resolve all refs to show the SHAs they'll pin to. - resolved, _ := rem.resolver.ResolveAllRecursive(wr.ActionRefs) - shaByKey := make(map[string]string) - for _, dep := range resolved { - shaByKey[dep.Key()] = dep.SHA - } - - // Review each action one at a time. Auto-apply prior choices and internal - // actions silently; prompt for each external action. - var approved []lockfile.ActionRef - for _, ref := range wr.ActionRefs { - key := ref.FullName() + "@" + ref.Ref - - // Prior choice — auto-apply without prompting. - if rem.state.approvedRefs[refKey(ref)] { - sha := shaByKey[key] - rem.output.Detail(" %s → %s %s", key, sha[:12], rem.output.Dim("↩ prior choice")) - approved = append(approved, ref) - continue - } - - // Internal (same-owner) action — auto-apply without prompting. - if rem.isSameOwner(ref.Owner) { - sha := shaByKey[key] - label := "" - if info, err := rem.tagLister.GetRepoInfo(ref.Owner, ref.Repo); err == nil { - label = info.VisibilityLabel() - if ref.Ref == info.DefaultBranch { - label += " · default branch" - } - if age := FormatTagAge(info.PushedAt); age != "" { - label += " · last push " + age - } - } - rem.output.Detail(" %s → %s %s", key, sha[:12], rem.output.Dim(label)) - approved = append(approved, ref) - continue - } - - // External action — auto-pin when there's a clear default, prompt otherwise. - sha, ok := shaByKey[key] - if !ok { - rem.output.Detail(" %s (could not resolve)", key) - continue - } - - displayTag := ref.Ref - autoPin := false - - // Case 1: Already a full semver tag (v4.3.1) — good default, verify it's a real tag. - if sv, svOK := lockfile.ParseSemver(ref.Ref); svOK && sv.IsFullSemver() { - if rem.tagLister.LookupTag(ref.Owner, ref.Repo, ref.Ref) != nil { - autoPin = true - } - } - - // Case 2: Mutable tag (v4, v4.2) — auto-pin if there's exactly one matching patch tag. - if !autoPin && IsMutableVersionTag(ref.Ref) { - if uniqueTag, err := rem.tagLister.UniquePatchTagForRef(ref.Owner, ref.Repo, sha, ref.Ref); err == nil && uniqueTag != "" { - displayTag = uniqueTag - autoPin = true - } - } - - if autoPin { - tagURL := fmt.Sprintf("https://github.com/%s/%s/releases/tag/%s", ref.Owner, ref.Repo, displayTag) - tagLink := rem.output.Dim(rem.output.Hyperlink("release", tagURL)) - if ti := rem.tagLister.LookupTag(ref.Owner, ref.Repo, displayTag); ti != nil && ti.IsImmutable { - tagLink = rem.output.Dim("🔒 " + rem.output.Hyperlink("immutable release", tagURL)) - } - // Show verifiable SHA match: tag resolves to the same commit. - commitURL := fmt.Sprintf("https://github.com/%s/%s/commit/%s", ref.Owner, ref.Repo, sha) - shaLabel := rem.output.Hyperlink(sha[:12], commitURL) - if displayTag != ref.Ref { - rem.output.Detail(" %s → %s → %s %s", key, displayTag, shaLabel, tagLink) - } else { - rem.output.Detail(" %s → %s %s", key, shaLabel, tagLink) - } - // Record both original and narrowed ref for cascade. - rem.state.approvedRefs[refKey(ref)] = true - if displayTag != ref.Ref { - narrowedRef := ref - narrowedRef.Ref = displayTag - rem.state.approvedRefs[refKey(narrowedRef)] = true - } - approved = append(approved, ref) - continue - } - - // Ambiguous case — prompt the user. - narrowHint := "" - if IsMutableVersionTag(ref.Ref) { - if patchTag, err := rem.tagLister.BestPatchTagForSHA(ref.Owner, ref.Repo, sha); err == nil && patchTag != "" { - narrowHint = fmt.Sprintf(" → %s", patchTag) - displayTag = patchTag - } - } - - tagLink := "" - tagURL := fmt.Sprintf("https://github.com/%s/%s/releases/tag/%s", ref.Owner, ref.Repo, displayTag) - if ti := rem.tagLister.LookupTag(ref.Owner, ref.Repo, displayTag); ti != nil && ti.IsImmutable { - tagLink = " " + rem.output.Dim("🔒 "+rem.output.Hyperlink("immutable release", tagURL)) - } else { - tagLink = " " + rem.output.Dim(rem.output.Hyperlink("release", tagURL)) - } - - rem.output.Detail(" %s%s → %s%s", key, narrowHint, sha[:12], tagLink) - - ok, err := rem.prompter.Confirm(fmt.Sprintf("Pin %s?", ref.FullName()+"@"+displayTag), true) - if err != nil { - if errors.Is(err, ErrAborted) { - return ErrAborted - } - continue - } - if ok { - approved = append(approved, ref) - } else { - rem.output.Skip("skipped %s", ref.FullName()) - } - } - - if len(approved) == 0 { - rem.Skipped++ - return nil - } - - wr.ActionRefs = approved - rem.state.markRefsApproved(approved) - return rem.applyPin(wr) -} - -// offerDefaultBranch checks each action ref for same-owner repos (internal -// actions) and switches bare SHA refs to the default branch. Named refs -// (tags, branches, versions) are preserved as-is. -// Returns a (possibly modified) copy of the WorkflowReport with updated refs. -func (rem *Remediator) offerDefaultBranch(wr WorkflowReport) WorkflowReport { - updated := make([]lockfile.ActionRef, 0, len(wr.ActionRefs)) - for _, ref := range wr.ActionRefs { - if !rem.isSameOwner(ref.Owner) { - updated = append(updated, ref) - continue - } - - info, err := rem.tagLister.GetRepoInfo(ref.Owner, ref.Repo) - if err != nil { - updated = append(updated, ref) - continue - } - - // Already targeting the default branch — nothing to offer. - if ref.Ref == info.DefaultBranch { - updated = append(updated, ref) - continue - } - - // Bare SHA → swap to default branch. Named refs stay as-is. - if isSHARef(ref.Ref) { - rem.output.Detail(" %s: using %s (default branch) instead of %s", - ref.FullName(), info.DefaultBranch, ref.Ref) - ref.Ref = info.DefaultBranch - updated = append(updated, ref) - continue - } - - // Named ref (tag, branch, version) — preserve what the user wrote. - updated = append(updated, ref) - } - - wr.ActionRefs = updated - return wr -} - -func (rem *Remediator) handleSHAAsRef(wr WorkflowReport, finding Finding) error { - dep := finding.Dependency - - owner, repo := dep.OwnerRepo() - - // Make the SHA a clickable link to the commit on GitHub. - commitURL := fmt.Sprintf("https://github.com/%s/%s/commit/%s", owner, repo, dep.SHA) - depLabel := dep.NWO + "@" + rem.output.Hyperlink(dep.SHA[:12], commitURL) - - if !rem.prompter.IsInteractive() && owner == "" { - rem.output.Warning("%s: %s", depLabel, finding.Detail) - rem.skipDep(dep) - return nil - } - - rem.output.Warning("%s: %s", depLabel, finding.Detail) - - if owner == "" { - rem.Skipped++ - return nil - } - - // Session memory: reuse prior internal ref choice for same-owner repos (any SHA). - if rem.isSameOwner(owner) { - nwo := owner + "/" + repo - if priorRef, ok := rem.state.internalRefChoices[nwo]; ok { - rem.output.Detail(" ↳ reusing prior choice for %s: %s", nwo, priorRef) - return rem.applySHAToTag(wr, dep, owner, repo, priorRef) - } - } - - // Session memory: if we already chose a tag for this exact dep, auto-apply. - if priorTag, ok := rem.state.recallChoice(dep); ok { - rem.output.Detail(" ↳ reusing prior choice: %s", priorTag) - return rem.applySHAToTag(wr, dep, owner, repo, priorTag) - } - - // Try to find which tags this SHA already belongs to. - suggestions, err := rem.tagLister.SuggestTagsForSHA(owner, repo, dep.SHA) - if err != nil { - rem.output.Warning("could not fetch tags: %s", err) - rem.Skipped++ - return nil - } - - // Smart default for internal (same-owner) repos: if the SHA already - // belongs to a tag, auto-pick it — no need to prompt. If no tag match, - // fall back to the default branch. - if rem.isSameOwner(owner) { - // Prefer a tag that directly points at this SHA. - for _, s := range suggestions { - if s.Preferred { - tag := s.Tag - tagURL := TagURL(owner, repo, tag.Name) - tagLink := rem.output.Dim(rem.output.Hyperlink("tag", tagURL)) - commitURL := fmt.Sprintf("https://github.com/%s/%s/commit/%s", owner, repo, dep.SHA) - shaLabel := rem.output.Hyperlink(dep.SHA[:12], commitURL) - rem.output.Detail(" ↳ already installed to %s (%s) %s", tag.Name, shaLabel, tagLink) - nwo := owner + "/" + repo - rem.state.internalRefChoices[nwo] = tag.Name - rem.state.recordChoice(dep, tag.Name) - return rem.applySHAToTag(wr, dep, owner, repo, tag.Name) - } - } - // No tag match — use default branch. - if info, err := rem.tagLister.GetRepoInfo(owner, repo); err == nil { - rem.output.Detail(" ↳ using %s (default branch) for %s/%s", info.DefaultBranch, owner, repo) - nwo := owner + "/" + repo - rem.state.internalRefChoices[nwo] = info.DefaultBranch - return rem.applySHAToTag(wr, dep, owner, repo, info.DefaultBranch) - } - } - - // Smart default: for external repos, if exactly one full-semver tag points - // at this SHA, auto-pick it. - if len(suggestions) > 0 && !rem.isSameOwner(owner) { - var fullSemverTags []TagSuggestion - for _, s := range suggestions { - sv, ok := lockfile.ParseSemver(s.Tag.Name) - if ok && sv.IsFullSemver() { - fullSemverTags = append(fullSemverTags, s) - } - } - if len(fullSemverTags) == 1 { - tag := fullSemverTags[0].Tag - tagURL := TagURL(owner, repo, tag.Name) - tagLink := rem.output.Dim(rem.output.Hyperlink("release", tagURL)) - if tag.IsImmutable { - tagLink = rem.output.Dim("🔒 " + rem.output.Hyperlink("immutable release", tagURL)) - } - // Show verifiable SHA match: tag points at the same commit. - commitURL := fmt.Sprintf("https://github.com/%s/%s/commit/%s", owner, repo, dep.SHA) - shaLabel := rem.output.Hyperlink(dep.SHA[:12], commitURL) - rem.output.Detail(" ↳ auto-pinning to %s (%s) %s", tag.Name, shaLabel, tagLink) - rem.state.recordChoice(dep, tag.Name) - return rem.applySHAToTag(wr, dep, owner, repo, tag.Name) - } - } - - // If we found tags for this SHA, present smart suggestions. - if len(suggestions) > 0 { - if !rem.prompter.IsInteractive() { - // Multiple tags match — can't auto-pick, need human choice. - rem.skipDep(dep) - return nil - } - return rem.handleSHAWithSuggestions(wr, finding, suggestions, owner, repo) - } - - // No tag matches this SHA — this is an unreleased commit. Be loud. - noTagCommitURL := fmt.Sprintf("https://github.com/%s/%s/commit/%s", owner, repo, dep.SHA) - shaLink := rem.output.Hyperlink(dep.SHA[:12], noTagCommitURL) - releasesURL := fmt.Sprintf("https://github.com/%s/%s/releases", owner, repo) - releasesLink := rem.output.Hyperlink("releases", releasesURL) - rem.output.Error(" commit %s does not belong to any release — you are running unreleased code", shaLink) - rem.output.Detail(" ↳ pin to a tagged release instead: %s", releasesLink) - if !rem.prompter.IsInteractive() { - rem.skipDep(dep) - return nil - } - return rem.handleSHATagPicker(wr, finding, owner, repo) -} - -func (rem *Remediator) handleSHAWithSuggestions(wr WorkflowReport, finding Finding, suggestions []TagSuggestion, owner, repo string) error { - dep := finding.Dependency - - // Build picker — full semver first (recommended), then major tags. - reordered := reorderSuggestions(suggestions) - if !rem.isSameOwner(owner) { - var filtered []TagSuggestion - for _, s := range reordered { - if !s.Tag.IsMajor { - filtered = append(filtered, s) - } - } - reordered = filtered - } - - options := make([]string, 0, len(reordered)+3) - for i, s := range reordered { - recommend := i == 0 && !s.Tag.IsMajor && !rem.isSameOwner(owner) - options = append(options, rem.tagLabel(owner, repo, pickerTag{ - Name: s.Tag.Name, - IsInstalled: s.Preferred, - IsImmutable: s.Tag.IsImmutable, - IsRelease: s.Tag.IsRelease, - IsMajor: s.Tag.IsMajor, - }, recommend)) - } - - var defaultBranchIdx int - options, defaultBranchIdx = rem.defaultBranchOption(options, owner, repo) - tagCount := len(reordered) - - result, err := rem.runPicker( - rem.pinPromptTitle(dep.NWO, owner, repo), - options, tagCount, defaultBranchIdx, - pickerSentinels{ShowAll: true}, - ) - if err != nil { - return err - } - - switch result.Action { - case pickerSkip: - rem.Skipped++ - return nil - case pickerShowAll: - return rem.handleSHATagPicker(wr, finding, owner, repo) - case pickerDefaultBranch: - info, _ := rem.tagLister.GetRepoInfo(owner, repo) - rem.state.internalRefChoices[owner+"/"+repo] = info.DefaultBranch - if err := rem.applySHAToTag(wr, dep, owner, repo, info.DefaultBranch); err != nil { - return err - } - rem.offerApplyAll(dep, info.DefaultBranch) - return nil - default: - selectedTag := reordered[result.TagIndex].Tag - if rem.isSameOwner(owner) { - rem.state.internalRefChoices[owner+"/"+repo] = selectedTag.Name - } - if err := rem.applySHAToTag(wr, dep, owner, repo, selectedTag.Name); err != nil { - return err - } - rem.offerApplyAll(dep, selectedTag.Name) - return nil - } -} - -func (rem *Remediator) handleSHATagPicker(wr WorkflowReport, finding Finding, owner, repo string) error { - dep := finding.Dependency - - curated, err := rem.tagLister.CuratePickerTags(owner, repo, dep.SHA) - if err != nil { - rem.output.Warning("could not fetch tags: %s", err) - rem.Skipped++ - return nil - } - - if len(curated) == 0 { - rem.output.Warning("no tags found for %s/%s", owner, repo) - rem.Skipped++ - return nil - } - - options := make([]string, 0, len(curated)+3) - for _, pt := range curated { - options = append(options, rem.tagLabel(owner, repo, pickerTag{ - Name: pt.Tag.Name, - IsInstalled: pt.Installed, - IsImmutable: pt.Tag.IsImmutable, - IsRelease: pt.Tag.IsRelease, - IsMajor: pt.Tag.IsMajor, - }, false)) - } - - var defaultBranchIdx int - options, defaultBranchIdx = rem.defaultBranchOption(options, owner, repo) - tagCount := len(curated) - - releasesURL := fmt.Sprintf("https://github.com/%s/%s/releases", owner, repo) - result, err := rem.runPicker( - rem.pinPromptTitle(owner+"/"+repo, owner, repo), - options, tagCount, defaultBranchIdx, - pickerSentinels{OpenReleases: releasesURL}, - ) - if err != nil { - return err - } - - switch result.Action { - case pickerSkip: - rem.Skipped++ - return nil - case pickerOpenReleases: - rem.output.Info("Opening releases page...") - openBrowser(releasesURL) - rem.Skipped++ - return nil - case pickerDefaultBranch: - info, _ := rem.tagLister.GetRepoInfo(owner, repo) - rem.state.internalRefChoices[owner+"/"+repo] = info.DefaultBranch - if err := rem.applySHAToTag(wr, dep, owner, repo, info.DefaultBranch); err != nil { - return err - } - rem.offerApplyAll(dep, info.DefaultBranch) - return nil - default: - selectedTag := curated[result.TagIndex].Tag - if rem.isSameOwner(owner) { - rem.state.internalRefChoices[owner+"/"+repo] = selectedTag.Name - } - if err := rem.applySHAToTag(wr, dep, owner, repo, selectedTag.Name); err != nil { - return err - } - rem.offerApplyAll(dep, selectedTag.Name) - return nil - } -} - -func (rem *Remediator) handleRefChanged(wr WorkflowReport, finding Finding) error { - dep := finding.Dependency - newRef := "" - if finding.ActionRef != nil { - newRef = finding.ActionRef.Ref - } - rem.output.Warning("%s: %s", dep.Key(), finding.Detail) - - if !rem.prompter.IsInteractive() { - // Non-interactive: auto-apply ref change — deterministic re-resolve. - return rem.applyReResolve(wr, dep) - } - - prompt := fmt.Sprintf("Re-pin %s to %s?", dep.NWO, newRef) - ok, err := rem.prompter.Confirm(prompt, true) - if err != nil { - if errors.Is(err, ErrAborted) { - return ErrAborted - } - rem.Skipped++ - return nil - } - if !ok { - rem.Skipped++ - return nil - } - - return rem.applyReResolve(wr, dep) -} - -func (rem *Remediator) handleStale(wr WorkflowReport, finding Finding) error { - dep := finding.Dependency - rem.output.Detail("%s: no longer in workflow — cleaning up", dep.Key()) - - // Auto-clean: re-resolve rewrites the lockfile without orphaned deps. - return rem.applyReResolve(wr, dep) -} - -// openBrowser attempts to open a URL in the user's browser. -func openBrowser(url string) { - // Use the open command on macOS, xdg-open on Linux. - // Best-effort — don't fail the doctor flow if it doesn't work. - cmd := "open" - if _, err := os.Stat("/usr/bin/xdg-open"); err == nil { - cmd = "xdg-open" - } - // #nosec G204 — URL is constructed from known repo owner/name, not user input. - proc := exec.Command(cmd, url) - _ = proc.Start() -} diff --git a/internal/doctor/session.go b/internal/doctor/session.go deleted file mode 100644 index ab192e6c..00000000 --- a/internal/doctor/session.go +++ /dev/null @@ -1,66 +0,0 @@ -package doctor - -import "github.com/github/gh-actions-pin/internal/lockfile" - -// sessionState tracks user decisions across multiple workflows during -// an interactive remediation session. It allows auto-applying a prior -// choice when the same dependency appears in another workflow file. -type sessionState struct { - // "owner/repo@SHA" → chosen tag name. - choices map[string]string - - // "owner/repo" → chosen ref (e.g. "main" or "v2") for same-owner repos. - internalRefChoices map[string]string - - // "owner/repo@ref" → true for refs the user already approved for pinning. - approvedRefs map[string]bool -} - -func newSessionState() sessionState { - return sessionState{ - choices: make(map[string]string), - internalRefChoices: make(map[string]string), - approvedRefs: make(map[string]bool), - } -} - -// choiceKey returns a stable key for session memory: "owner/repo@SHA". -func choiceKey(dep *lockfile.Dependency) string { - return dep.NWO + "@" + dep.SHA -} - -// recordChoice saves a tag choice for a dep so it can be auto-applied later. -func (s *sessionState) recordChoice(dep *lockfile.Dependency, tag string) { - s.choices[choiceKey(dep)] = tag -} - -// recallChoice returns (tag, true) if we already made a choice for this dep. -func (s *sessionState) recallChoice(dep *lockfile.Dependency) (string, bool) { - tag, ok := s.choices[choiceKey(dep)] - return tag, ok -} - -// refKey returns a session memory key for an unpinned action ref: "owner/repo@ref". -func refKey(ref lockfile.ActionRef) string { - return ref.FullName() + "@" + ref.Ref -} - -// markRefsApproved records all action refs as approved for auto-pinning. -func (s *sessionState) markRefsApproved(refs []lockfile.ActionRef) { - for _, ref := range refs { - s.approvedRefs[refKey(ref)] = true - } -} - -// allRefsApproved returns true if every ref was already approved in a prior workflow. -func (s *sessionState) allRefsApproved(refs []lockfile.ActionRef) bool { - if len(refs) == 0 { - return false - } - for _, ref := range refs { - if !s.approvedRefs[refKey(ref)] { - return false - } - } - return true -} diff --git a/internal/doctor/tagging.go b/internal/doctor/tagging.go deleted file mode 100644 index 3ffbf9fc..00000000 --- a/internal/doctor/tagging.go +++ /dev/null @@ -1,341 +0,0 @@ -package doctor - -import ( - "fmt" - - "github.com/github/gh-actions-pin/internal/lockfile" - "strings" - "time" -) - -// TagSuggestion is a tag paired with why it's being suggested. -type TagSuggestion struct { - Tag TagInfo - Reason string - Preferred bool // true if this tag points directly at the pinned SHA -} - -// PickerTag is a tag formatted for the interactive picker. -type PickerTag struct { - Tag TagInfo - Label string // formatted display label - Installed bool // true if this tag matches the currently pinned SHA -} - -// BestPatchTagForSHA returns the highest full-semver patch tag pointing at the -// given SHA, or "" if none exists. This is used to narrow mutable version refs -// (like "v4") to a specific patch version (like "v4.2.1") when pinning. -func (tl *TagLister) BestPatchTagForSHA(owner, repo, sha string) (string, error) { - matching, err := tl.TagsForSHA(owner, repo, sha) - if err != nil { - return "", err - } - - var best lockfile.Semver - bestFound := false - for _, t := range matching { - if t.IsMajor { - continue - } - sv, ok := lockfile.ParseSemver(t.Name) - if !ok || !sv.IsFullSemver() { - continue - } - if !bestFound || sv.Major > best.Major || - (sv.Major == best.Major && sv.Minor > best.Minor) || - (sv.Major == best.Major && sv.Minor == best.Minor && sv.Patch > best.Patch) { - best = sv - bestFound = true - } - } - - if !bestFound { - return "", nil - } - return best.Raw, nil -} - -// UniquePatchTagForRef returns the sole full-semver patch tag that matches the -// given ref's family, or "" if the choice is ambiguous (0 or 2+ candidates). -// For "v9" it only considers v9.x.y tags; for "v4.2" only v4.2.x tags. -// This is used for auto-pinning: if there's exactly one obvious patch tag, -// we can pin without prompting. -func (tl *TagLister) UniquePatchTagForRef(owner, repo, sha, ref string) (string, error) { - refSV, refOK := lockfile.ParseSemver(ref) - if !refOK { - return "", nil - } - - matching, err := tl.TagsForSHA(owner, repo, sha) - if err != nil { - return "", err - } - - var candidates []lockfile.Semver - for _, t := range matching { - if t.IsMajor { - continue - } - sv, ok := lockfile.ParseSemver(t.Name) - if !ok || !sv.IsFullSemver() { - continue - } - // Must be in the same family as the original ref. - if sv.Major != refSV.Major { - continue - } - // If original ref specifies minor (e.g. "v4.2"), patch must match that minor. - if ref != refSV.MajorTag() && sv.Minor != refSV.Minor { - continue - } - candidates = append(candidates, sv) - } - - if len(candidates) != 1 { - return "", nil - } - return candidates[0].Raw, nil -} - -// TagsForSHA returns all tags whose commit SHA matches the given SHA. -func (tl *TagLister) TagsForSHA(owner, repo, sha string) ([]TagInfo, error) { - all, err := tl.ListTags(owner, repo) - if err != nil { - return nil, err - } - var matched []TagInfo - for _, t := range all { - if strings.EqualFold(t.SHA, sha) { - matched = append(matched, t) - } - } - return matched, nil -} - -// SuggestTagsForSHA returns a curated set of tag suggestions for a pinned SHA. -// It includes exact-match tags, plus major/minor family alternatives when the -// match is version-like. Returns at most 5 suggestions. -func (tl *TagLister) SuggestTagsForSHA(owner, repo, sha string) ([]TagSuggestion, error) { - matching, err := tl.TagsForSHA(owner, repo, sha) - if err != nil { - return nil, err - } - - if len(matching) == 0 { - return nil, nil - } - - var suggestions []TagSuggestion - - // Find the best semver match to derive family tags. - var bestSV lockfile.Semver - bestFound := false - for _, t := range matching { - if sv, ok := lockfile.ParseSemver(t.Name); ok && sv.Rest == "" && !t.IsMajor { - if !bestFound || sv.Major > bestSV.Major || - (sv.Major == bestSV.Major && sv.Minor > bestSV.Minor) || - (sv.Major == bestSV.Major && sv.Minor == bestSV.Minor && sv.Patch > bestSV.Patch) { - bestSV = sv - bestFound = true - } - } - } - - seen := make(map[string]bool) - - // Add exact-match tags (non-major, non-minor-only) first. - for _, t := range matching { - if t.IsMajor { - continue - } - if len(suggestions) >= 3 { - break - } - suggestions = append(suggestions, TagSuggestion{ - Tag: t, - Reason: "exact match for pinned SHA", - Preferred: true, - }) - seen[t.Name] = true - } - - // Suggest major/minor family tags if the best match is semver-ish. - if bestFound { - allTags, _ := tl.ListTags(owner, repo) - - // Look for the major tag (e.g. v4). - majorName := bestSV.MajorTag() - if !seen[majorName] { - for _, t := range allTags { - if t.Name == majorName { - suggestions = append(suggestions, TagSuggestion{ - Tag: t, - Reason: fmt.Sprintf("major tag (tracks latest %s.x.x)", majorName), - }) - seen[majorName] = true - break - } - } - } - - // Look for a minor tag (e.g. v4.2) — not all repos have these. - minorName := bestSV.MinorTag() - if !seen[minorName] { - for _, t := range allTags { - if t.Name == minorName { - suggestions = append(suggestions, TagSuggestion{ - Tag: t, - Reason: fmt.Sprintf("minor tag (tracks latest %s.x)", minorName), - }) - seen[minorName] = true - break - } - } - } - } - - // Cap at 5 total. - if len(suggestions) > 5 { - suggestions = suggestions[:5] - } - - return suggestions, nil -} - -// CuratePickerTags returns a short list of the most useful tags for a picker. -// Shows the latest patch per major version (up to 3 majors), marks the one -// matching pinnedSHA as "installed", and only puts 📦 on that one. -func (tl *TagLister) CuratePickerTags(owner, repo, pinnedSHA string) ([]PickerTag, error) { - all, err := tl.ListTags(owner, repo) - if err != nil { - return nil, err - } - if len(all) == 0 { - return nil, nil - } - - // Pick the latest full-version tag per major (skip major-only tags, pre-releases). - type majorBucket struct { - major int - tag TagInfo - } - seen := make(map[int]bool) - var buckets []majorBucket - - for _, t := range all { - if t.IsMajor { - continue - } - sv, ok := lockfile.ParseSemver(t.Name) - if !ok || sv.Rest != "" { - continue - } - // Skip tags younger than the cooldown period. - if tl.isTagTooNew(owner, repo, t.Name) && !strings.EqualFold(t.SHA, pinnedSHA) { - continue - } - if !seen[sv.Major] { - seen[sv.Major] = true - buckets = append(buckets, majorBucket{major: sv.Major, tag: t}) - } - if len(buckets) >= 3 { - break - } - } - - // Build picker entries. - var result []PickerTag - for _, b := range buckets { - installed := strings.EqualFold(b.tag.SHA, pinnedSHA) - result = append(result, PickerTag{ - Tag: b.tag, - Label: b.tag.Name, - Installed: installed, - }) - } - - // If pinnedSHA matches a tag that isn't in our buckets, prepend it. - pinnedFound := false - for _, pt := range result { - if pt.Installed { - pinnedFound = true - break - } - } - if !pinnedFound { - for _, t := range all { - if strings.EqualFold(t.SHA, pinnedSHA) && !t.IsMajor { - label := t.Name + " 📦 installed" - result = append([]PickerTag{{ - Tag: t, - Label: label, - Installed: true, - }}, result...) - break - } - } - } - - return result, nil -} - -// FormatTagAge returns a relative age string like "3d ago" from an ISO 8601 timestamp. -func FormatTagAge(isoDate string) string { - if isoDate == "" { - return "" - } - t, err := time.Parse(time.RFC3339, isoDate) - if err != nil { - return "" - } - d := time.Since(t) - switch { - case d < time.Hour: - return fmt.Sprintf("%dm ago", int(d.Minutes())) - case d < 24*time.Hour: - return fmt.Sprintf("%dh ago", int(d.Hours())) - case d < 30*24*time.Hour: - return fmt.Sprintf("%dd ago", int(d.Hours()/24)) - case d < 365*24*time.Hour: - return fmt.Sprintf("%dmo ago", int(d.Hours()/(24*30))) - default: - return fmt.Sprintf("%dy ago", int(d.Hours()/(24*365))) - } -} - -// TagURL returns a clickable GitHub releases tag URL. -func TagURL(owner, repo, tag string) string { - return fmt.Sprintf("https://github.com/%s/%s/releases/tag/%s", owner, repo, tag) -} - -// reorderSuggestions puts full semver tags first (recommended for pinning), -// then major-only tags last. Within full semver: immutable releases first, -// then regular releases, then plain tags. -func reorderSuggestions(suggestions []TagSuggestion) []TagSuggestion { - out := make([]TagSuggestion, 0, len(suggestions)) - // Immutable releases first. - for _, s := range suggestions { - if !s.Tag.IsMajor && s.Tag.IsImmutable { - out = append(out, s) - } - } - // Regular releases. - for _, s := range suggestions { - if !s.Tag.IsMajor && s.Tag.IsRelease && !s.Tag.IsImmutable { - out = append(out, s) - } - } - // Non-release full version tags. - for _, s := range suggestions { - if !s.Tag.IsMajor && !s.Tag.IsRelease { - out = append(out, s) - } - } - // Major-only tags last. - for _, s := range suggestions { - if s.Tag.IsMajor { - out = append(out, s) - } - } - return out -} diff --git a/internal/doctor/tags.go b/internal/doctor/tags.go deleted file mode 100644 index b7657ea9..00000000 --- a/internal/doctor/tags.go +++ /dev/null @@ -1,334 +0,0 @@ -package doctor - -import ( - "fmt" - "net/url" - "os" - "sort" - "time" - - "github.com/cli/go-gh/v2/pkg/api" - "github.com/github/gh-actions-pin/internal/lockfile" - "gopkg.in/yaml.v3" -) - -// TagInfo represents a tag with optional release metadata. -type TagInfo struct { - Name string // e.g. "v4.2.2" - SHA string // commit SHA the tag points to (dereferenced for annotated tags) - IsRelease bool // true if a GitHub Release exists for this tag - IsImmutable bool // true if the release is marked immutable (tag can't be moved/deleted) - IsMajor bool // true if this looks like a major-only tag (e.g. "v4") -} - -// RepoInfo holds repository metadata relevant for pinning decisions. -type RepoInfo struct { - DefaultBranch string // e.g. "main" - Visibility string // "public", "private", or "internal" - PushedAt string // ISO 8601 timestamp of last push -} - -// VisibilityLabel returns a human-readable label for display in prompts. -func (ri RepoInfo) VisibilityLabel() string { - switch ri.Visibility { - case "private", "internal": - return "🏠 internal" - default: - return "public" - } -} - -// IsInternal returns true for private or internal repos — those where -// pinning to the default branch is a reasonable option. -func (ri RepoInfo) IsInternal() bool { - return ri.Visibility == "private" || ri.Visibility == "internal" -} - -// TagLister fetches tags and release metadata for action repos. -type TagLister struct { - client *api.RESTClient - cache map[string][]TagInfo - repoCache map[string]*RepoInfo - releaseDates map[string]map[string]string // owner/repo → tag → published_at - cooldown CooldownConfig -} - -// CooldownConfig controls how old a tag must be before we recommend it. -// Tags with a known release date younger than the threshold are excluded -// from suggestions and curated picks. -type CooldownConfig struct { - DefaultDays int // global default (0 = no filtering) - RepoOverrides map[string]int // "owner/repo" → days override -} - -// CooldownDays returns the cooldown period for a given repo. -func (c CooldownConfig) CooldownDays(owner, repo string) int { - if days, ok := c.RepoOverrides[owner+"/"+repo]; ok { - return days - } - return c.DefaultDays -} - -// NewTagLister creates a TagLister with the given REST client. -func NewTagLister(client *api.RESTClient) *TagLister { - return &TagLister{ - client: client, - cache: make(map[string][]TagInfo), - repoCache: make(map[string]*RepoInfo), - releaseDates: make(map[string]map[string]string), - cooldown: LoadCooldownConfig(), - } -} - -// ListTags fetches tags for an action repo, enriched with release metadata. -// Results are cached per owner/repo. -func (tl *TagLister) ListTags(owner, repo string) ([]TagInfo, error) { - key := owner + "/" + repo - if cached, ok := tl.cache[key]; ok { - return cached, nil - } - - tags, err := tl.fetchTags(owner, repo) - if err != nil { - return nil, err - } - - releaseTagSet, err := tl.fetchReleaseTags(owner, repo) - if err != nil { - // Non-fatal — releases are optional enrichment. - releaseTagSet = make(map[string]releaseInfo) - } - - for i := range tags { - if ri, ok := releaseTagSet[tags[i].Name]; ok { - tags[i].IsRelease = ri.IsRelease - tags[i].IsImmutable = ri.IsImmutable - } - tags[i].IsMajor = isMajorTag(tags[i].Name) - } - - // Sort: latest semver first, major tags last. - sort.Slice(tags, func(i, j int) bool { - if tags[i].IsMajor != tags[j].IsMajor { - return !tags[i].IsMajor - } - return tags[i].Name > tags[j].Name - }) - - tl.cache[key] = tags - return tags, nil -} - -// LookupTag returns the TagInfo for a specific tag name, or nil if not found. -// Uses the cached tag list from ListTags. -func (tl *TagLister) LookupTag(owner, repo, tagName string) *TagInfo { - all, err := tl.ListTags(owner, repo) - if err != nil { - return nil - } - for i := range all { - if all[i].Name == tagName { - return &all[i] - } - } - return nil -} - -// LatestStableTag returns the latest non-major stable tag that passes cooldown. -// It skips major-only tags (e.g. "v4"), pre-release tags, and tags younger -// than the cooldown period. Returns ("", nil) if no suitable tag is found. -func (tl *TagLister) LatestStableTag(owner, repo string) (string, error) { - all, err := tl.ListTags(owner, repo) - if err != nil { - return "", err - } - for _, t := range all { - if t.IsMajor { - continue - } - sv, ok := lockfile.ParseSemver(t.Name) - if !ok || sv.Rest != "" { - continue // skip pre-release, non-semver - } - if tl.isTagTooNew(owner, repo, t.Name) { - continue - } - return t.Name, nil - } - return "", nil -} - -func (tl *TagLister) fetchTags(owner, repo string) ([]TagInfo, error) { - // Use the repos/tags endpoint — it dereferences annotated tags automatically. - path := fmt.Sprintf("repos/%s/%s/tags?per_page=100", - url.PathEscape(owner), url.PathEscape(repo)) - - var apiTags []struct { - Name string `json:"name"` - Commit struct { - SHA string `json:"sha"` - } `json:"commit"` - } - - if err := tl.client.Get(path, &apiTags); err != nil { - return nil, fmt.Errorf("fetching tags for %s/%s: %w", owner, repo, err) - } - - tags := make([]TagInfo, 0, len(apiTags)) - for _, t := range apiTags { - tags = append(tags, TagInfo{ - Name: t.Name, - SHA: t.Commit.SHA, - }) - } - return tags, nil -} - -// ReleaseInfo holds release metadata for a tag. -type ReleaseInfo struct { - TagName string - PublishedAt string // ISO 8601 date -} - -// releaseInfo holds the release/immutable status for a tag. -type releaseInfo struct { - IsRelease bool - IsImmutable bool -} - -func (tl *TagLister) fetchReleaseTags(owner, repo string) (map[string]releaseInfo, error) { - path := fmt.Sprintf("repos/%s/%s/releases?per_page=30", - url.PathEscape(owner), url.PathEscape(repo)) - - var releases []struct { - TagName string `json:"tag_name"` - PublishedAt string `json:"published_at"` - Immutable bool `json:"immutable"` - } - - if err := tl.client.Get(path, &releases); err != nil { - return nil, err - } - - set := make(map[string]releaseInfo, len(releases)) - for _, rel := range releases { - set[rel.TagName] = releaseInfo{IsRelease: true, IsImmutable: rel.Immutable} - } - - // Cache release dates. - key := owner + "/" + repo - if _, ok := tl.releaseDates[key]; !ok { - tl.releaseDates[key] = make(map[string]string) - } - for _, rel := range releases { - if rel.PublishedAt != "" { - tl.releaseDates[key][rel.TagName] = rel.PublishedAt - } - } - return set, nil -} - -// ReleaseDate returns the published_at date for a tag, if available. -func (tl *TagLister) ReleaseDate(owner, repo, tag string) string { - key := owner + "/" + repo - if dates, ok := tl.releaseDates[key]; ok { - return dates[tag] - } - return "" -} - -// LoadCooldownConfig reads cooldown settings from ~/.config/gh-actions-pin/config.yml. -// Returns sensible defaults (3 days) if the file doesn't exist or is malformed. -func LoadCooldownConfig() CooldownConfig { - cfg := CooldownConfig{ - DefaultDays: 3, - RepoOverrides: make(map[string]int), - } - - p := configPath() - if p == "" { - return cfg - } - data, err := os.ReadFile(p) - if err != nil { - return cfg - } - - var file struct { - CooldownDays int `yaml:"cooldown_days"` - Repos map[string]struct { - CooldownDays int `yaml:"cooldown_days"` - } `yaml:"repos"` - } - if err := yaml.Unmarshal(data, &file); err != nil { - return cfg - } - if file.CooldownDays > 0 { - cfg.DefaultDays = file.CooldownDays - } - for nwo, repoCfg := range file.Repos { - if repoCfg.CooldownDays >= 0 { - cfg.RepoOverrides[nwo] = repoCfg.CooldownDays - } - } - return cfg -} - -// isTagTooNew returns true if the tag's release date is younger than the cooldown period. -// Tags without a known release date are never filtered (we can't determine their age). -func (tl *TagLister) isTagTooNew(owner, repo, tag string) bool { - days := tl.cooldown.CooldownDays(owner, repo) - if days <= 0 { - return false - } - isoDate := tl.ReleaseDate(owner, repo, tag) - if isoDate == "" { - return false - } - t, err := time.Parse(time.RFC3339, isoDate) - if err != nil { - return false - } - return time.Since(t) < time.Duration(days)*24*time.Hour -} - -// GetRepoInfo fetches repository visibility and default branch. Cached per owner/repo. -func (tl *TagLister) GetRepoInfo(owner, repo string) (*RepoInfo, error) { - key := owner + "/" + repo - if cached, ok := tl.repoCache[key]; ok { - return cached, nil - } - - path := fmt.Sprintf("repos/%s/%s", - url.PathEscape(owner), url.PathEscape(repo)) - - var result struct { - DefaultBranch string `json:"default_branch"` - Visibility string `json:"visibility"` - PushedAt string `json:"pushed_at"` - } - if err := tl.client.Get(path, &result); err != nil { - return nil, err - } - - info := &RepoInfo{ - DefaultBranch: result.DefaultBranch, - Visibility: result.Visibility, - PushedAt: result.PushedAt, - } - tl.repoCache[key] = info - return info, nil -} - -// BranchHeadSHA returns the latest commit SHA on the given branch. -func (tl *TagLister) BranchHeadSHA(owner, repo, branch string) (string, error) { - path := fmt.Sprintf("repos/%s/%s/commits/%s", - url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(branch)) - var result struct { - SHA string `json:"sha"` - } - if err := tl.client.Get(path, &result); err != nil { - return "", err - } - return result.SHA, nil -} diff --git a/internal/doctor/version.go b/internal/doctor/version.go deleted file mode 100644 index d5f9b267..00000000 --- a/internal/doctor/version.go +++ /dev/null @@ -1,87 +0,0 @@ -package doctor - -import ( - "strings" - - "github.com/github/gh-actions-pin/internal/lockfile" -) - -// IsMutableVersionTag returns true if ref looks like a mutable version tag -// (major-only like "v4" or minor-only like "v4.2") that should be narrowed -// to a specific patch version for pinning. -func IsMutableVersionTag(ref string) bool { - sv, ok := lockfile.ParseSemver(ref) - if !ok { - return false - } - return !sv.IsFullSemver() -} - -// IsNarrowedVersion returns true if narrowed is a more specific patch version -// of mutable. For example: mutable="v4", narrowed="v4.1.0" → true. -// mutable="v4.2", narrowed="v4.2.1" → true. mutable="v4", narrowed="v5.0.0" → false. -func IsNarrowedVersion(mutable, narrowed string) bool { - mv, mOK := lockfile.ParseSemver(mutable) - nv, nOK := lockfile.ParseSemver(narrowed) - if !mOK || !nOK { - return false - } - if !nv.IsFullSemver() { - return false - } - if mv.Major != nv.Major { - return false - } - if mutable != mv.MajorTag() && mv.Minor != nv.Minor { - return false - } - return true -} - -// IsUpgrade returns true if moving from currentRef to latestRef is a real -// version upgrade. Returns false for noops where the current ref is already -// at or more specific than the latest (e.g. v4.0.0 → v4, v3.1.1 → v3). -func IsUpgrade(currentRef, latestRef string) bool { - if currentRef == latestRef { - return false - } - cur, curOK := lockfile.ParseSemver(currentRef) - lat, latOK := lockfile.ParseSemver(latestRef) - if !curOK || !latOK { - if !latOK { - return false - } - return true - } - if lat.Rest != "" { - return false - } - if lat.Major < cur.Major { - return false - } - if lat.Major == cur.Major { - if lat.Minor == 0 && lat.Patch == 0 && cur.Minor >= 0 { - if latestRef == lat.MajorTag() { - return false - } - } - if lat.Minor == cur.Minor && lat.Patch <= cur.Patch { - return false - } - if lat.Minor < cur.Minor { - return false - } - } - return true -} - -// isMajorTag returns true if the tag looks like a major-only version (e.g. "v4", "v12"). -func isMajorTag(tag string) bool { - tag = strings.TrimPrefix(tag, "v") - for _, c := range tag { - if c < '0' || c > '9' { - return false - } - } - return len(tag) > 0 -} diff --git a/internal/doctor/version_test.go b/internal/doctor/version_test.go deleted file mode 100644 index 4501d815..00000000 --- a/internal/doctor/version_test.go +++ /dev/null @@ -1,39 +0,0 @@ -package doctor - -import "testing" - -func TestIsMutableVersionTag(t *testing.T) { - cases := []struct { - ref string - want bool - }{ - // Mutable version tags — should be narrowed - {"v4", true}, - {"v4.2", true}, - {"v1", true}, - {"v10", true}, - - // Full semver — not mutable - {"v4.2.1", false}, - {"v1.0.0", false}, - - // SHA refs — must never be treated as mutable version tags - {"1e7e51e771db61008b38414a730f564565cf7c20", false}, - {"de0fac2e4500dabe0009e67214ff5f5447ce83dd", false}, - {"0000000000000000000000000000000000000000", false}, - {"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", false}, - {"1E7E51E771DB61008B38414A730F564565CF7C20", false}, // uppercase - - // Non-version refs - {"main", false}, - {"develop", false}, - {"", false}, - {"v", false}, - } - for _, tc := range cases { - got := IsMutableVersionTag(tc.ref) - if got != tc.want { - t.Errorf("IsMutableVersionTag(%q) = %v, want %v", tc.ref, got, tc.want) - } - } -} diff --git a/internal/pipeline/checks/category.go b/internal/pipeline/checks/category.go new file mode 100644 index 00000000..394c4aaa --- /dev/null +++ b/internal/pipeline/checks/category.go @@ -0,0 +1,110 @@ +// Package checks implements the structural, misleading-sha, and +// resolver-bound validators run against parsed workflows. +package checks + +// Category classifies the state of a workflow or action dependency. The string +// values are part of the schema surfaced to consumers (SARIF rule IDs, JSON +// output, doc URL slugs); the frozen-strings test guards against accidental +// renames. +type Category string + +const ( + // NotPinned means the workflow has action refs but no + // corresponding dependencies entry in the lockfile. + NotPinned Category = "not-pinned" + // ShaAsRef means a dependency is pinned to a bare SHA with no + // human-readable tag ref alongside it. + ShaAsRef Category = "sha-as-ref" + // RefChanged means the workflow uses: ref was manually changed + // (e.g. v6.2.0 → v6) and the lockfile no longer matches. + RefChanged Category = "ref-changed" + // 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 Category = "stale" + // ImpostorCommit means the pinned SHA is not in the ref's git + // history (possible fork-network commit). Matches zizmor's + // impostor-commit audit ID. + ImpostorCommit Category = "impostor-commit" + // MisleadingSHA means a ref looks like a SHA but resolves to a + // different commit. + MisleadingSHA Category = "misleading-sha" + // LockfileForgery means the pinned SHA is not an ancestor of the + // current ref — the lockfile entry was likely injected or + // tampered with. + LockfileForgery Category = "lockfile-forgery" + // Valid means the dependency is pinned and verified. + Valid Category = "valid" + // RunOnly means the workflow has no action refs (only run: + // steps), so pinning is not applicable. + RunOnly Category = "run-only" + // AncestryUnknown means the Compare API couldn't decide whether + // the pinned SHA is in the ref's history (typically rate-limited + // or transient error). Non-blocking diagnostic: we know the SHAs + // differ but can't classify the move as benign-but-known + // (ref-moved) vs. tampered (lockfile-forgery). + AncestryUnknown Category = "ancestry-unknown" + // ReachabilityUnknown means branch_commits couldn't decide + // whether the pinned SHA is still reachable from any branch in + // the upstream repo (resolver failure, GraphQL rate limit, etc). + // Non-blocking diagnostic: surfaced so consumers can retry rather + // than treating the dep as verified. + ReachabilityUnknown Category = "reachability-unknown" + // OnboardingRequired means an `upgrade --no-onboard` run targeted + // a workflow that has no existing entry in `lockfile.workflows{}`. + // The CLI refuses to silently add it during a dependency-update + // run; the operator must onboard the workflow explicitly before + // re-running upgrade. + OnboardingRequired Category = "onboarding-required" +) + +// IsInconclusive reports whether c represents a diagnostic that +// couldn't reach a verdict (network/rate-limit fallback). These are +// surfaced as warnings but are not blocking: consumers (e.g. +// Dependabot FindingMapper) treat them as "scan inconclusive, retry" +// rather than "lockfile is bad". +func (c Category) IsInconclusive() bool { + switch c { + case AncestryUnknown, ReachabilityUnknown: + return true + } + return false +} + +// Severity indicates how serious a finding is if it represents a real +// problem. Pair with Confidence to express how strongly the tool stands +// behind the call. +type Severity string + +const ( + // SeverityOK means the finding represents a clean state — no + // action needed. + SeverityOK Severity = "ok" + // SeverityInfo is purely informational and does not require + // action. + SeverityInfo Severity = "info" + // SeverityWarning indicates a concern worth surfacing but not + // blocking on. + SeverityWarning Severity = "warning" + // SeverityError indicates a blocking issue the operator must + // resolve. + SeverityError Severity = "error" +) + +// Confidence is how certain the producer is the finding is real, +// modeled on zizmor's audit output. +type Confidence string + +const ( + // ConfidenceLow marks a signal that could not be fully verified + // (resolver failure, reachability inconclusive). + ConfidenceLow Confidence = "low" + // ConfidenceMedium marks a signal inferred from a fallback + // (tag-object peel, ancestry unknown due to rate limit). + ConfidenceMedium Confidence = "medium" + // ConfidenceHigh marks a signal resting on authoritative data + // (exact SHA comparison, upstream reachability answer). + ConfidenceHigh Confidence = "high" +) diff --git a/internal/pipeline/checks/category_test.go b/internal/pipeline/checks/category_test.go new file mode 100644 index 00000000..24acf920 --- /dev/null +++ b/internal/pipeline/checks/category_test.go @@ -0,0 +1,90 @@ +package checks + +import "testing" + +// TestCategoryStringsAreFrozen pins the Category string vocabulary. +// These string values are surfaced to consumers (SARIF rule IDs, JSON +// output, doc URL slugs). Renaming a constant's string is a breaking +// change; this test fails loudly so the breakage is intentional. +func TestCategoryStringsAreFrozen(t *testing.T) { + cases := []struct { + got Category + want string + }{ + {NotPinned, "not-pinned"}, + {ShaAsRef, "sha-as-ref"}, + {RefChanged, "ref-changed"}, + {RefMoved, "ref-moved"}, + {Stale, "stale"}, + {ImpostorCommit, "impostor-commit"}, + {MisleadingSHA, "misleading-sha"}, + {LockfileForgery, "lockfile-forgery"}, + {Valid, "valid"}, + {RunOnly, "run-only"}, + {AncestryUnknown, "ancestry-unknown"}, + {ReachabilityUnknown, "reachability-unknown"}, + {OnboardingRequired, "onboarding-required"}, + } + for _, c := range cases { + if string(c.got) != c.want { + t.Errorf("Category renamed: got %q, want %q (this is a breaking change to the schema)", string(c.got), c.want) + } + } +} + +// TestCategoryIsInconclusive guards the inconclusive partition so a +// new diagnostic category isn't silently treated as blocking by +// consumers that key off this predicate. +func TestCategoryIsInconclusive(t *testing.T) { + inconclusive := []Category{AncestryUnknown, ReachabilityUnknown} + for _, c := range inconclusive { + if !c.IsInconclusive() { + t.Errorf("%q must be inconclusive", string(c)) + } + } + blocking := []Category{ + NotPinned, ShaAsRef, RefChanged, RefMoved, Stale, + ImpostorCommit, MisleadingSHA, LockfileForgery, + Valid, RunOnly, OnboardingRequired, + } + for _, c := range blocking { + if c.IsInconclusive() { + t.Errorf("%q must not be inconclusive", string(c)) + } + } +} + +// TestSeverityStringsAreFrozen pins the Severity string vocabulary. +func TestSeverityStringsAreFrozen(t *testing.T) { + cases := []struct { + got Severity + want string + }{ + {SeverityOK, "ok"}, + {SeverityInfo, "info"}, + {SeverityWarning, "warning"}, + {SeverityError, "error"}, + } + for _, c := range cases { + if string(c.got) != c.want { + t.Errorf("Severity renamed: got %q, want %q (this is a breaking change to the schema)", string(c.got), c.want) + } + } +} + +// TestConfidenceStringsAreFrozen pins the Confidence string vocabulary. +func TestConfidenceStringsAreFrozen(t *testing.T) { + cases := []struct { + got Confidence + want string + }{ + {ConfidenceLow, "low"}, + {ConfidenceMedium, "medium"}, + {ConfidenceHigh, "high"}, + } + for _, c := range cases { + if string(c.got) != c.want { + t.Errorf("Confidence renamed: got %q, want %q (this is a breaking change to the schema)", string(c.got), c.want) + } + } +} diff --git a/internal/doctor/finding.go b/internal/pipeline/checks/finding.go similarity index 58% rename from internal/doctor/finding.go rename to internal/pipeline/checks/finding.go index d4280f7f..cf3fb505 100644 --- a/internal/doctor/finding.go +++ b/internal/pipeline/checks/finding.go @@ -1,42 +1,8 @@ -package doctor +package checks -import "github.com/github/gh-actions-pin/internal/lockfile" - -// Category classifies the state of a workflow or individual action dependency. -type Category string - -const ( - // CategoryNotPinned means the workflow has action refs but no dependencies: section. - CategoryNotPinned Category = "not_pinned" - // CategorySHAAsRef means a dependency is pinned to a bare SHA with no tag ref. - CategorySHAAsRef Category = "sha_as_ref" - // CategoryStale means the pinned SHA no longer matches what the ref resolves to. - CategoryStale Category = "stale" - // CategoryRefChanged means the uses: ref was manually changed (e.g. v6.2.0 → v6). - CategoryRefChanged Category = "ref_changed" - // CategoryImposterCommit means the pinned SHA is not in the ref's git history (possible fork-network commit). - CategoryImposterCommit Category = "imposter_commit" - // CategoryMisleadingSHA means a ref looks like a SHA but resolves to a different commit. - CategoryMisleadingSHA Category = "misleading_sha" - // CategoryLockfileForgery means the pinned SHA is not an ancestor of the - // current ref — the lockfile entry was likely injected or tampered with. - CategoryLockfileForgery Category = "lockfile_forgery" - // CategoryRefMoved means the upstream tag now resolves to a different SHA than what's locked. - CategoryRefMoved Category = "ref_moved" - // CategoryValid means the dependency is pinned and verified. - CategoryValid Category = "valid" - // CategoryRunOnly means the workflow has no action refs (only run: steps). - CategoryRunOnly Category = "run_only" -) - -// Severity indicates how serious a finding is. -type Severity string - -const ( - SeverityOK Severity = "ok" - SeverityInfo Severity = "info" - SeverityWarning Severity = "warning" - SeverityError Severity = "error" +import ( + parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" + "github.com/github/gh-actions-pin/internal/dep" ) // Finding represents a single diagnosed issue (or clean bill) for a workflow. @@ -47,25 +13,48 @@ type Finding struct { Category Category // Severity of the finding. Severity Severity + // Confidence of the finding — see the Confidence type docs. Always + // populated at construction; an empty value is a bug and the + // no-empty-confidence test will catch it. + Confidence Confidence // ActionRef is the action reference this finding relates to (nil for workflow-level findings). - ActionRef *lockfile.ActionRef + ActionRef *parserlock.ActionRef // Dependency is the existing pinned dep if any. - Dependency *lockfile.Dependency + Dependency *dep.Dependency // ParentNWO is the dep key of the direct action that pulls in this transitive dep (empty if direct). ParentNWO string // Detail is a human-readable explanation. Detail string - // Remediation describes what doctor can do about it. + // Remediation describes what the check command can do about it. Remediation string - // LiveSHA is the current upstream SHA when it differs from the pinned SHA (e.g. REF_MOVED). - LiveSHA string + // ObservedSHA is the SHA the resolver got at scan time, recorded when + // it differs from the pinned SHA (e.g. ref-moved, misleading-sha, + // lockfile-forgery). + ObservedSHA string + // DocURL points to docs explaining the finding. Populated by the + // engine adapter so it's parity-aligned with the editor's + // codeDescription link; "" when no URL is mapped. + DocURL string + // RecommendedTag is the most recent stable tag whose commit is + // reachable from a branch, populated for unreachable-SHA findings + // (ImpostorCommit) when one can be found. Empty otherwise. + RecommendedTag string + // RecommendedSHA is the commit SHA the recommended tag points to. + RecommendedSHA string + // RecommendedSearched is true when the release walk ran for this + // finding (regardless of outcome). Lets renderers distinguish + // "we didn't look" from "we looked and found nothing." + RecommendedSearched bool } // InventoryEntry describes a single dependency with context. type InventoryEntry struct { - Dep lockfile.Dependency + Dep dep.Dependency File string Direct bool + // Parents lists the dep keys of parent composite actions that pull in this + // transitive dependency. Empty for direct dependencies. + Parents []string } // WorkflowReport aggregates all findings for a single workflow file. @@ -73,9 +62,9 @@ type WorkflowReport struct { Path string Findings []Finding // ActionRefs are all action references found in the workflow. - ActionRefs []lockfile.ActionRef + ActionRefs []parserlock.ActionRef // Deps are the existing pinned dependencies (nil if not pinned). - Deps []lockfile.Dependency + Deps []dep.Dependency // Inventory lists all dependencies with direct/transitive classification. Inventory []InventoryEntry // ParseWarnings from ExtractActionRefs (e.g. malformed uses: lines). @@ -85,8 +74,11 @@ type WorkflowReport struct { // NeedsAttention returns true if this workflow has any non-OK findings. func (r *WorkflowReport) NeedsAttention() bool { for _, f := range r.Findings { + if f.Category.IsInconclusive() { + continue + } switch f.Category { - case CategoryValid, CategoryRunOnly, CategoryMisleadingSHA, CategoryRefMoved: + case Valid, RunOnly, MisleadingSHA, RefMoved: continue default: return true @@ -108,10 +100,16 @@ func (r *WorkflowReport) CountByCategory(c Category) int { // IsValid returns true for findings that don't represent integrity violations. func (f *Finding) IsValid() bool { + if f.Severity == SeverityError { + return false + } + if f.Category.IsInconclusive() { + return true + } switch f.Category { - case CategoryValid, CategoryRunOnly, CategorySHAAsRef, CategoryRefMoved: + case Valid, RunOnly, ShaAsRef, RefMoved: return true - case CategoryNotPinned: + case NotPinned: return f.ActionRef == nil // workflow-level is a warning default: return false @@ -121,13 +119,13 @@ func (f *Finding) IsValid() bool { // IsWarning returns true for findings that should render as warnings (not errors). func (f *Finding) IsWarning() bool { switch { - case f.Category == CategorySHAAsRef: + case f.Category == ShaAsRef: return true - case f.Category == CategoryRefMoved: + case f.Category == RefMoved: return true - case f.Category == CategoryValid && f.Severity == SeverityWarning: + case f.Category.IsInconclusive(): return true - case f.Category == CategoryNotPinned && f.ActionRef == nil: + case f.Category == NotPinned && f.ActionRef == nil: return true default: return false @@ -145,9 +143,12 @@ func (f *Finding) DepKey() string { return "" } -// Report aggregates all workflow reports for a doctor run. +// Report aggregates all workflow reports for a check run. type Report struct { Workflows []WorkflowReport + // RepoFindings are findings that apply to the repository as a whole + // (not to any individual workflow). + RepoFindings []Finding } // IsValid returns true if all workflows in the report pass validation. @@ -157,6 +158,11 @@ func (r *Report) IsValid() bool { return false } } + for _, f := range r.RepoFindings { + if !f.IsValid() { + return false + } + } return true } diff --git a/internal/pipeline/checks/impostor.go b/internal/pipeline/checks/impostor.go new file mode 100644 index 00000000..8b9f7f33 --- /dev/null +++ b/internal/pipeline/checks/impostor.go @@ -0,0 +1,139 @@ +package checks + +import ( + "context" + + parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" + "github.com/github/gh-actions-pin/internal/ghapi" + "github.com/github/gh-actions-pin/internal/pinpool" + "github.com/github/gh-actions-pin/internal/resolve" + "github.com/github/gh-actions-pin/internal/tag" +) + +// ReachabilityChecker is the subset of resolve.Resolver needed to verify +// that a tag's commit is reachable from a branch in the action repo. +// Defined as an interface so tests can stub without a real resolver. +type ReachabilityChecker interface { + CheckReachability(ctx context.Context, owner, repo, sha, ref string) resolve.ReachabilityResult +} + +// maxRecommendedTagsChecked bounds the per-finding tag walk so a repo with a +// long tail of unreachable tags doesn't trigger an unbounded reachability +// fan-out. +const maxRecommendedTagsChecked = 10 + +// FindRecommendedRelease walks the action repo's tags newest-first and returns the +// first stable release whose commit is reachable from a branch. It's the +// remediation half of the ImpostorCommit detection: when we flag a +// pinned SHA as orphaned, this answers "what should the user re-pin to?" +// +// Returns ("", "") when no qualifying tag is found within the bounded walk +// (e.g. the action has never tagged a reachable release, or all recent +// releases are also orphaned and the user should escalate to the publisher). +func FindRecommendedRelease(ctx context.Context, tl *tag.Lister, r ReachabilityChecker, pool *pinpool.Pool, owner, repo string) (recTag, sha string) { + if tl == nil || r == nil { + return "", "" + } + tags, err := tl.ListTags(ctx, owner, repo) + if err != nil { + return "", "" + } + + // Collect up to maxRecommendedTagsChecked candidate tags. + type candidate struct{ tag, sha string } + var candidates []candidate + for _, t := range tags { + if t.IsMajor { + continue + } + sv, ok := parserlock.ParseSemVer(t.Name) + if !ok || sv.Rest != "" { + continue + } + if t.SHA == "" { + continue + } + candidates = append(candidates, candidate{tag: t.Name, sha: t.SHA}) + if len(candidates) >= maxRecommendedTagsChecked { + break + } + } + if len(candidates) == 0 { + return "", "" + } + + // Check all candidates in parallel via the shared worker pool. + // Branch listings and singleflight inside CheckReachability coalesce + // across workers sharing the same NWO, so the marginal cost per extra + // SHA is roughly one GraphQL compare (~300 ms) divided by pool width. + type indexedCandidate struct { + idx int + candidate + } + indexed := make([]indexedCandidate, len(candidates)) + for i, c := range candidates { + indexed[i] = indexedCandidate{idx: i, candidate: c} + } + results := make([]resolve.ReachabilityResult, len(candidates)) + _ = pinpool.RunTyped(pool, ctx, "Checking recommended releases", + indexed, + func(ic indexedCandidate) string { return owner + "/" + repo + "@" + ic.tag }, + func(ctx context.Context, _ int, ic indexedCandidate) error { + results[ic.idx] = r.CheckReachability(ctx, owner, repo, ic.sha, ic.tag) + return nil + }, + ) + + // Return the first reachable tag in newest-first order. + for i, rr := range results { + if rr.Status == resolve.Reachable { + return candidates[i].tag, candidates[i].sha + } + } + return "", "" +} + +// EnrichImpostorFindings walks the report and attaches a recommended release +// to every ImpostorCommit finding when one is available. Mutates +// findings in place. Safe to call when tl or r is nil — becomes a no-op so +// non-network code paths (tests, --offline) don't trigger lookups. +// +// Findings that have been walked are also marked via RecommendedSearched +// so renderers can distinguish "didn't look" from "looked and found nothing" +// — the latter is itself useful signal (e.g. an action whose entire release +// flow detaches tag commits from any branch, warranting harder escalation +// to the publisher). +func EnrichImpostorFindings(ctx context.Context, report *Report, tl *tag.Lister, r ReachabilityChecker, pool *pinpool.Pool) { + if report == nil || tl == nil || r == nil { + return + } + // Cache per owner/repo so multiple impostor findings against the same + // action share a single tag walk + reachability sweep. + type suggestion struct{ tag, sha string } + cache := make(map[ghapi.Repo]suggestion) + for i := range report.Workflows { + wf := &report.Workflows[i] + for j := range wf.Findings { + f := &wf.Findings[j] + if f.Category != ImpostorCommit || f.Dependency == nil { + continue + } + owner, repo := f.Dependency.OwnerRepo() + if owner == "" || repo == "" { + continue + } + key := ghapi.ForRepo(owner, repo) + s, ok := cache[key] + if !ok { + t, sha := FindRecommendedRelease(ctx, tl, r, pool, owner, repo) + s = suggestion{tag: t, sha: sha} + cache[key] = s + } + f.RecommendedSearched = true + if s.tag != "" { + f.RecommendedTag = s.tag + f.RecommendedSHA = s.sha + } + } + } +} diff --git a/internal/pipeline/checks/impostor_test.go b/internal/pipeline/checks/impostor_test.go new file mode 100644 index 00000000..7bff2e8a --- /dev/null +++ b/internal/pipeline/checks/impostor_test.go @@ -0,0 +1,156 @@ +package checks + +import ( + "context" + "testing" + + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/ghapi/httpmock" + "github.com/github/gh-actions-pin/internal/pinpool" + "github.com/github/gh-actions-pin/internal/resolve" + "github.com/github/gh-actions-pin/internal/tag" +) + +type fakeReachabilityChecker struct { + results map[string]resolve.ReachabilityStatus +} + +func (f *fakeReachabilityChecker) CheckReachability(_ context.Context, owner, repo, sha, ref string) resolve.ReachabilityResult { + status := f.results[ref] + if status == "" { + status = resolve.Unreachable + } + return resolve.ReachabilityResult{Owner: owner, Repo: repo, SHA: sha, Ref: ref, Status: status} +} + +// registerTagWalk wires the three endpoints Lister hits during a +// publisher walk: GET /tags, GET /git/matching-refs/tags, GET /releases. +// Tests parameterize only the /tags payload; matching-refs and releases +// are registered empty so the walk completes deterministically. +func registerTagWalk(reg *httpmock.Registry, owner, repo string, tags []map[string]any) { + reg.Register( + httpmock.REST("GET", `repos/`+owner+`/`+repo+`/tags`), + httpmock.JSONResponse(tags), + ) + reg.Register( + httpmock.REST("GET", `repos/`+owner+`/`+repo+`/git/matching-refs/tags`), + httpmock.JSONResponse([]map[string]any{}), + ) + reg.Register( + httpmock.REST("GET", `repos/`+owner+`/`+repo+`/releases`), + httpmock.JSONResponse([]map[string]any{}), + ) +} + +// TestFindRecommendedRelease_PicksFirstReachable walks tags newest-first and stops at +// the first stable tag whose commit is reachable from a branch. +func TestFindRecommendedRelease_PicksFirstReachable(t *testing.T) { + reg := &httpmock.Registry{} + registerTagWalk(reg, "acme", "widget", []map[string]any{ + {"name": "v1.5.0", "commit": map[string]any{"sha": "aaaaaaa1111111111111111111111111111111aa"}}, + {"name": "v1.4.0", "commit": map[string]any{"sha": "bbbbbbb2222222222222222222222222222222bb"}}, + {"name": "v1.3.0", "commit": map[string]any{"sha": "ccccccc3333333333333333333333333333333cc"}}, + }) + + tl := tag.NewListerForTest(t, reg) + rc := &fakeReachabilityChecker{results: map[string]resolve.ReachabilityStatus{ + "v1.5.0": resolve.Unreachable, + "v1.4.0": resolve.Reachable, + }} + + tag, sha := FindRecommendedRelease(context.Background(), tl, rc, pinpool.New(0, nil), "acme", "widget") + if tag != "v1.4.0" { + t.Fatalf("expected v1.4.0, got %q", tag) + } + if sha != "bbbbbbb2222222222222222222222222222222bb" { + t.Fatalf("expected bbbb…bb, got %q", sha) + } +} + +// TestFindRecommendedRelease_NoneReachable returns empty when every recent tag is +// detached from a branch — signal for the caller to escalate to the publisher. +func TestFindRecommendedRelease_NoneReachable(t *testing.T) { + reg := &httpmock.Registry{} + registerTagWalk(reg, "acme", "widget", []map[string]any{ + {"name": "v1.2.0", "commit": map[string]any{"sha": "aaaaaaa1111111111111111111111111111111aa"}}, + {"name": "v1.1.0", "commit": map[string]any{"sha": "bbbbbbb2222222222222222222222222222222bb"}}, + }) + + tl := tag.NewListerForTest(t, reg) + rc := &fakeReachabilityChecker{} // all Unreachable + + tag, sha := FindRecommendedRelease(context.Background(), tl, rc, pinpool.New(0, nil), "acme", "widget") + if tag != "" || sha != "" { + t.Fatalf("expected empty suggestion, got tag=%q sha=%q", tag, sha) + } +} + +// TestEnrichImpostorFindings_MarksSearched flags impostor findings with the +// search outcome even when no suggestion is found so renderers can surface +// the "escalate to publisher" hint. +func TestEnrichImpostorFindings_MarksSearched(t *testing.T) { + reg := &httpmock.Registry{} + registerTagWalk(reg, "acme", "widget", []map[string]any{ + {"name": "v1.0.0", "commit": map[string]any{"sha": "aaaaaaa1111111111111111111111111111111aa"}}, + }) + + tl := tag.NewListerForTest(t, reg) + rc := &fakeReachabilityChecker{} // none reachable + + report := &Report{ + Workflows: []WorkflowReport{{ + Path: ".github/workflows/test.yml", + Findings: []Finding{{ + Category: ImpostorCommit, + Confidence: ConfidenceHigh, + Dependency: &dep.Dependency{NWO: "acme/widget", Ref: "v1"}, + }}, + }}, + } + + EnrichImpostorFindings(context.Background(), report, tl, rc, pinpool.New(0, nil)) + + f := report.Workflows[0].Findings[0] + if !f.RecommendedSearched { + t.Error("expected RecommendedSearched=true after walk") + } + if f.RecommendedTag != "" { + t.Errorf("expected no suggestion when nothing reachable, got %q", f.RecommendedTag) + } +} + +// TestEnrichImpostorFindings_PopulatesSuggestion attaches the discovered tag +// to the finding so downstream renderers (presentCheckResults, summary) can +// surface a concrete re-pin target. +func TestEnrichImpostorFindings_PopulatesSuggestion(t *testing.T) { + reg := &httpmock.Registry{} + registerTagWalk(reg, "acme", "widget", []map[string]any{ + {"name": "v1.0.0", "commit": map[string]any{"sha": "aaaaaaa1111111111111111111111111111111aa"}}, + }) + + tl := tag.NewListerForTest(t, reg) + rc := &fakeReachabilityChecker{results: map[string]resolve.ReachabilityStatus{ + "v1.0.0": resolve.Reachable, + }} + + report := &Report{ + Workflows: []WorkflowReport{{ + Path: ".github/workflows/test.yml", + Findings: []Finding{{ + Category: ImpostorCommit, + Confidence: ConfidenceHigh, + Dependency: &dep.Dependency{NWO: "acme/widget", Ref: "v1"}, + }}, + }}, + } + + EnrichImpostorFindings(context.Background(), report, tl, rc, pinpool.New(0, nil)) + + f := report.Workflows[0].Findings[0] + if f.RecommendedTag != "v1.0.0" { + t.Errorf("expected v1.0.0, got %q", f.RecommendedTag) + } + if f.RecommendedSHA != "aaaaaaa1111111111111111111111111111111aa" { + t.Errorf("unexpected sha %q", f.RecommendedSHA) + } +} diff --git a/internal/pipeline/checks/misleading.go b/internal/pipeline/checks/misleading.go new file mode 100644 index 00000000..66a1bb95 --- /dev/null +++ b/internal/pipeline/checks/misleading.go @@ -0,0 +1,183 @@ +package checks + +import ( + "context" + "fmt" + "strings" + + parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" + "github.com/github/gh-actions-pin/internal/resolve" +) + +// checkMisleadingSha emits MisleadingSHA when a uses: ref looks +// like a SHA but the resolver maps it to a different commit. Independent +// of whether a lock entry exists. An annotated-tag-object SHA pin +// (possibly through a chain of tag-of-tag) is content-addressed and +// safe even though the peeled commit differs from the ref string — those +// are skipped via PeelTagObject. +func checkMisleadingSha(ctx context.Context, pw ParsedWorkflow, r CheckResolver) []Finding { + var out []Finding + for _, ref := range pw.Refs { + if !parserlock.IsFullSha(ref.Ref) { + continue + } + sha, ok := r.ResolveRef(ref.Owner, ref.Repo, ref.Ref) + if !ok || sha == "" { + continue + } + if strings.EqualFold(sha, ref.Ref) { + continue + } + if peeled, ok := r.PeelTagObject(ctx, ref.Owner, ref.Repo, ref.Ref); ok && strings.EqualFold(peeled, sha) { + continue + } + // Ref string is SHA-shaped, resolver returned a different commit, + // and PeelTagObject ruled out the legitimate tag-object shape. + f := newRefFinding(pw, ref, MisleadingSHA, SeverityError, ConfidenceHigh) + f.ObservedSHA = sha + f.Dependency = synthDep(ref, ref.Ref) + f.Detail = fmt.Sprintf("ref %s resolves to %s — the ref string looks like a SHA but isn't this commit", parserlock.ShortSHA(ref.Ref), parserlock.ShortSHA(sha)) + f.Remediation = "investigate — the ref may be a tag named after a SHA, not the SHA itself" + out = append(out, f) + } + return out +} + +// checkRefMovedAndForgery emits RefMoved when the upstream ref +// resolves to a different SHA than the lockfile. If CheckAncestry confirms +// the locked SHA is NOT an ancestor of the observed SHA, the finding is +// upgraded to LockfileForgery (mutually exclusive with ref-moved). +// When the observed SHA is itself unreachable from any branch of the +// upstream repo (tag-moved-to-fork-network), an additional +// ImpostorCommit finding is emitted alongside ref-moved / +// ancestry-unknown. Forgery suppresses the observed-SHA impostor: the +// lockfile-tampering claim is stronger. +func checkRefMovedAndForgery(ctx context.Context, pw ParsedWorkflow, depIndex map[string]parserlock.Pin, r CheckResolver) []Finding { + var out []Finding + for _, ref := range pw.Refs { + if parserlock.IsFullSha(ref.Ref) { + continue + } + pin, ok := depIndex[parserlock.IndexKey(ref.Owner, ref.Repo, ref.Ref)] + if !ok { + continue + } + sha, ok := r.ResolveRef(ref.Owner, ref.Repo, ref.Ref) + if !ok || sha == "" { + continue + } + if strings.EqualFold(sha, pin.Hex) { + continue + } + ancestry, ancestryDetail := r.CheckAncestry(ctx, ref.Owner, ref.Repo, pin.Hex, sha) + f := newRefFinding(pw, ref, "", "", "") + f.ObservedSHA = sha + f.Dependency = synthDep(ref, pin.Hex) + switch ancestry { + case resolve.AncestryNotAncestor: + // Compare API gave an authoritative not-an-ancestor verdict. + // Forgery wins: don't double-flag with an observed-SHA + // impostor finding. + f.Category = LockfileForgery + f.Severity = SeverityError + f.Confidence = ConfidenceHigh + f.Detail = fmt.Sprintf("pinned %s is not an ancestor of %s — lockfile may have been tampered with", parserlock.ShortSHA(pin.Hex), parserlock.ShortSHA(sha)) + f.Remediation = "investigate immediately — verify the lockfile entry against upstream history" + out = append(out, f) + case resolve.AncestryUnknown: + // Compare API didn't reach a verdict — typically rate-limited + // even after CheckAncestry's bounded retry. Surface as its + // own category so consumers don't conflate inconclusive with + // benign, and append the resolver's detail so the operator + // sees why. + f.Category = AncestryUnknown + f.Severity = SeverityWarning + f.Confidence = ConfidenceMedium + f.Detail = fmt.Sprintf("ref %s now resolves to %s, lockfile pins %s (ancestry check inconclusive%s)", ref.Ref, parserlock.ShortSHA(sha), parserlock.ShortSHA(pin.Hex), suffixWith(ancestryDetail)) + f.Remediation = "retry when the Compare API is available to classify this as ref-moved or lockfile-forgery" + out = append(out, f) + // Inconclusive ancestry doesn't block a branch_commits check + // on the observed SHA. + if imp, ok := liveRefImpostorFinding(pw, ref, sha, r); ok { + out = append(out, imp) + } + default: + // AncestryConfirmed: routine release. + f.Category = RefMoved + f.Severity = SeverityWarning + f.Confidence = ConfidenceHigh + f.Detail = fmt.Sprintf("ref %s now resolves to %s, lockfile pins %s", ref.Ref, parserlock.ShortSHA(sha), parserlock.ShortSHA(pin.Hex)) + f.Remediation = "re-run `gh actions-pin` to refresh the lock entry" + out = append(out, f) + if imp, ok := liveRefImpostorFinding(pw, ref, sha, r); ok { + out = append(out, imp) + } + } + } + return out +} + +// liveRefImpostorFinding returns an impostor-commit finding when the +// observed SHA is not reachable from any branch of the upstream repo +// (the tag-hijacked-to-fork-network shape). Unknown reachability fails +// open. Caller must suppress this in the forgery branch. +func liveRefImpostorFinding(pw ParsedWorkflow, ref parserlock.ActionRef, observedSHA string, r CheckResolver) (Finding, bool) { + status := r.CheckReachability(ref.Owner, ref.Repo, observedSHA, ref.Ref) + if status != resolve.Unreachable { + return Finding{}, false + } + f := newRefFinding(pw, ref, ImpostorCommit, SeverityError, ConfidenceHigh) + f.ObservedSHA = observedSHA + f.Dependency = synthDep(ref, observedSHA) + f.Detail = fmt.Sprintf("ref %s now resolves to %s — not on any branch of %s/%s (fork-network injection)", ref.Ref, parserlock.ShortSHA(observedSHA), ref.Owner, ref.Repo) + f.Remediation = "investigate immediately — the upstream ref has been moved to a commit that is not in this repo's branch history" + return f, true +} + +// suffixWith renders an optional detail as ": " for inline +// concatenation, returning empty when detail is empty. +func suffixWith(detail string) string { + if detail == "" { + return "" + } + return ": " + detail +} + +// checkImpostorCommit emits ImpostorCommit when the locked SHA is +// not reachable from the ref's history. Skips entries already covered by +// a forgery finding (forgery is the stronger signal). +func checkImpostorCommit(pw ParsedWorkflow, depIndex map[string]parserlock.Pin, r CheckResolver, forgeryKeys map[string]bool) []Finding { + if len(depIndex) == 0 { + return nil + } + var out []Finding + for _, ref := range pw.Refs { + pin, ok := depIndex[parserlock.IndexKey(ref.Owner, ref.Repo, ref.Ref)] + if !ok { + continue + } + if parserlock.IsFullSha(ref.Ref) { + continue + } + if forgeryKeys[parserlock.IndexKey(ref.Owner, ref.Repo, ref.Ref)] { + continue + } + status := r.CheckReachability(ref.Owner, ref.Repo, pin.Hex, ref.Ref) + if status != resolve.Unreachable { + // Fail open on ReachabilityUnknown by design: only an + // authoritative Unreachable is an impostor. The inconclusive + // case is surfaced as a ReachabilityUnknown warning by the + // pipeline's reachabilityComplementFindings — emitting it here + // too would double-report every direct dep on an API hiccup. + continue + } + // branch_commits gave an authoritative answer: the locked SHA is + // not on any branch of the upstream repo (fork-network impostor). + f := newRefFinding(pw, ref, ImpostorCommit, SeverityError, ConfidenceHigh) + f.Dependency = synthDep(ref, pin.Hex) + f.Detail = fmt.Sprintf("locked %s is not reachable from %s — classic fork-network impostor-commit shape", parserlock.ShortSHA(pin.Hex), ref.Ref) + f.Remediation = "investigate immediately — the lockfile entry may have been injected" + out = append(out, f) + } + return out +} diff --git a/internal/pipeline/checks/parsed.go b/internal/pipeline/checks/parsed.go new file mode 100644 index 00000000..30c9b005 --- /dev/null +++ b/internal/pipeline/checks/parsed.go @@ -0,0 +1,37 @@ +package checks + +import ( + parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" + "github.com/github/gh-actions-pin/internal/dep" +) + +// ParsedWorkflow holds the per-workflow parse result that both phases need. +// LoadErr / DepsErr capture early failures so DiagnoseParsed can surface them +// as findings without re-loading the file. +type ParsedWorkflow struct { + Path string + Refs []parserlock.ActionRef + ExistingDeps []dep.Dependency + ParseWarnings []string + LoadErr error + DepsErr error + // Resolved, when true, instructs DiagnoseParsed to run this + // workflow's diagnostics with a nil resolver. Network-bound checks + // (ref-moved, impostor-commit) are skipped and the engine relies on + // purely structural validation against the on-disk lockfile. Caller + // is asserting "this workflow is already fully resolved" — typically + // set on the fast path when every direct ref in the workflow is + // already recorded in the lockfile. + Resolved bool + // SkipReachWhenUnchanged, when true, instructs DiagnoseParsed to skip + // the per-dep reachability network call for any ExistingDep whose + // (NWO, Ref, SHA) matches an entry in the freshly-resolved live deps + // for this workflow. A Reachable result is synthesized in place. This + // is the per-workflow analogue of the cmd-level fast path: when at + // least one direct ref is new/changed (so the workflow couldn't be + // fully trusted), the remaining unchanged pins still don't need a + // fresh network reachability sweep on every run. Callers should leave + // this false when --rescan or an equivalent "verify everything" flag + // is in effect. + SkipReachWhenUnchanged bool +} diff --git a/internal/pipeline/checks/resolver.go b/internal/pipeline/checks/resolver.go new file mode 100644 index 00000000..d2b8ad2e --- /dev/null +++ b/internal/pipeline/checks/resolver.go @@ -0,0 +1,93 @@ +package checks + +import ( + "context" + + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/ghapi" + "github.com/github/gh-actions-pin/internal/resolve" +) + +// CheckResolver is the surface the resolver-bound checks need. The +// production implementation is *prewarmedResolver; tests use stubs. +type CheckResolver interface { + // ResolveRef returns the live SHA for owner/repo@ref. ok=false means + // the resolver could not answer (network failure, unknown ref); checks + // fail open on that. + ResolveRef(owner, repo, ref string) (sha string, ok bool) + // PeelTagObject reports whether a hex SHA names an annotated tag + // object (or chain of tag-of-tag) and, if so, returns the commit OID + // it ultimately points at. + PeelTagObject(ctx context.Context, owner, repo, sha string) (commit string, ok bool) + // CheckAncestry asks whether candidate is an ancestor of head and + // returns a short human-readable detail alongside the status — the + // rate-limit or compare-base detail callers surface to operators. + CheckAncestry(ctx context.Context, owner, repo, candidate, head string) (resolve.AncestryStatus, string) + // CheckReachability asks whether sha is reachable from ref's history. + CheckReachability(owner, repo, sha, ref string) resolve.ReachabilityStatus +} + +// prewarmedResolver adapts *resolve.Resolver to CheckResolver. Ref +// resolutions and reachability are pre-computed; ancestry and tag-object +// peels stay on-demand and delegate to the resolver's own cache. +type prewarmedResolver struct { + inner *resolve.Resolver + refs map[ghapi.NWORef]string // (owner/repo, ref) -> sha + reach map[ghapi.Reach]resolve.ReachabilityStatus // (owner/repo, sha, ref) -> status +} + +// NewPrewarmedResolver primes the adapter with the live resolution of +// refs and a pre-computed reachability sweep. Pass live==nil when +// ResolveAllRecursive failed; checks that need a ref will fail open. +// extraReach carries reach results for SHAs outside the canonical +// lockfile sweep — typically the observed SHA of a moved ref. +func NewPrewarmedResolver(r *resolve.Resolver, live []dep.Dependency, reach []resolve.ReachabilityResult, extraReach ...[]resolve.ReachabilityResult) *prewarmedResolver { + extras := 0 + for _, e := range extraReach { + extras += len(e) + } + a := &prewarmedResolver{ + inner: r, + refs: make(map[ghapi.NWORef]string, len(live)), + reach: make(map[ghapi.Reach]resolve.ReachabilityStatus, len(reach)+extras), + } + for _, d := range live { + owner, repo := d.OwnerRepo() + a.refs[ghapi.ForNWORef(owner, repo, d.Ref)] = d.SHA + } + for _, rr := range reach { + a.reach[ghapi.ForReach(rr.Owner, rr.Repo, rr.SHA, rr.Ref)] = rr.Status + } + for _, batch := range extraReach { + for _, rr := range batch { + a.reach[ghapi.ForReach(rr.Owner, rr.Repo, rr.SHA, rr.Ref)] = rr.Status + } + } + return a +} + +func (a *prewarmedResolver) ResolveRef(owner, repo, ref string) (string, bool) { + sha, ok := a.refs[ghapi.ForNWORef(owner, repo, ref)] + return sha, ok +} + +func (a *prewarmedResolver) PeelTagObject(ctx context.Context, owner, repo, sha string) (string, bool) { + if a.inner == nil { + return "", false + } + return a.inner.PeelTagObject(ctx, owner, repo, sha) +} + +func (a *prewarmedResolver) CheckAncestry(ctx context.Context, owner, repo, candidate, head string) (resolve.AncestryStatus, string) { + if a.inner == nil { + return resolve.AncestryUnknown, "" + } + return a.inner.CheckAncestry(ctx, owner, repo, candidate, head) +} + +func (a *prewarmedResolver) CheckReachability(owner, repo, sha, ref string) resolve.ReachabilityStatus { + if s, ok := a.reach[ghapi.ForReach(owner, repo, sha, ref)]; ok { + return s + } + return resolve.ReachabilityUnknown +} diff --git a/internal/pipeline/checks/run.go b/internal/pipeline/checks/run.go new file mode 100644 index 00000000..e06b47f9 --- /dev/null +++ b/internal/pipeline/checks/run.go @@ -0,0 +1,116 @@ +package checks + +import ( + "context" + "strings" + + parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/workflowfile" +) + +// RunChecks evaluates all enabled validators against the given parsed +// workflow and returns findings in catalog order. The lockfile snapshot +// scopes the structural checks; the resolver enables the resolver-bound +// checks (misleading-sha, ref-moved, forgery, impostor). When r is nil, +// resolver-bound checks are skipped silently. +// +// Returned findings have their primitive fields populated, plus +// ActionRef (for direct uses) and Dependency (for ref-tied entries). +// 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)) + depPins, depIndex := parseWorkflowDeps(wfEntry) + + var out []Finding + out = append(out, checkNotPinned(pw, depPins, depIndex)...) + out = append(out, checkShaAsRef(pw, depIndex)...) + out = append(out, checkRefChanged(pw, depPins)...) + out = append(out, checkStale(pw, depPins)...) + + if r != nil { + out = append(out, checkMisleadingSha(ctx, pw, r)...) + refMoved := checkRefMovedAndForgery(ctx, pw, depIndex, r) + out = append(out, refMoved...) + out = append(out, checkImpostorCommit(pw, depIndex, r, collectForgeryKeys(refMoved))...) + } + return out +} + +// parseWorkflowDeps decodes a workflow's dependency pin strings into Pins +// plus an index keyed by "owner/repo@ref". Unparseable entries are +// dropped silently — they're surfaced separately by workflowfile.Parse callers. +func parseWorkflowDeps(rawDeps []string) ([]parserlock.Pin, map[string]parserlock.Pin) { + pins := make([]parserlock.Pin, 0, len(rawDeps)) + idx := make(map[string]parserlock.Pin, len(rawDeps)) + for _, raw := range rawDeps { + pin, ok := parserlock.ParsePin(raw) + if !ok { + continue + } + pins = append(pins, pin) + idx[pin.IndexKey()] = pin + } + return pins, idx +} + +// collectForgeryKeys returns the set of IndexKeys flagged as forgery so +// the impostor check can skip them. +func collectForgeryKeys(ff []Finding) map[string]bool { + if len(ff) == 0 { + return nil + } + out := make(map[string]bool) + for _, f := range ff { + if f.Category != LockfileForgery || f.ActionRef == nil { + continue + } + ar := f.ActionRef + out[parserlock.IndexKey(ar.Owner, ar.Repo, ar.Ref)] = true + } + return out +} + +// newRefFinding builds a Finding with the common header fields populated +// from a uses: ref. Category/Severity can be empty when the caller fills +// them in based on a downstream branch (e.g. ref-moved vs forgery). +// Confidence is required at construction — see Finding.Confidence. +func newRefFinding(pw ParsedWorkflow, ref parserlock.ActionRef, cat Category, sev Severity, conf Confidence) Finding { + refCopy := ref + return Finding{ + WorkflowPath: pw.Path, + Category: cat, + Severity: sev, + Confidence: conf, + ActionRef: &refCopy, + } +} + +// synthDep builds a dep.Dependency from an ActionRef + locked SHA. +// Used by checks that surface lock-state but don't have a real +// Dependency pointer from the store. +func synthDep(ref parserlock.ActionRef, sha string) *dep.Dependency { + return &dep.Dependency{ + NWO: ref.Owner + "/" + ref.Repo, + Path: ref.Path, + Ref: ref.Ref, + SHA: sha, + } +} + +// nwoLower returns lowercased "owner/repo". Trivial but used in several +// validators; centralized so the casing rule lives in one place. +func nwoLower(owner, repo string) string { + return strings.ToLower(owner) + "/" + strings.ToLower(repo) +} + +// formatUseName renders "owner/repo" or "owner/repo/path" for human +// messages. Index/lookup keys are always at owner/repo granularity. +func formatUseName(owner, repo, path string) string { + s := nwoLower(owner, repo) + if path != "" { + s += "/" + path + } + return s +} diff --git a/internal/pipeline/checks/run_test.go b/internal/pipeline/checks/run_test.go new file mode 100644 index 00000000..62b215ec --- /dev/null +++ b/internal/pipeline/checks/run_test.go @@ -0,0 +1,721 @@ +package checks + +import ( + "context" + "sort" + "strings" + "testing" + + parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" + "github.com/github/gh-actions-pin/internal/resolve" +) + +// Typed map keys for the test stub: a small struct per lookup tuple so +// callers can't drift on delimiter choice. +type ( + stubRefKey struct{ owner, repo, ref string } + stubAncestryKey struct{ owner, repo, cand, head string } + stubReachKey struct{ owner, repo, sha, ref string } + stubTagObjectKey struct{ owner, repo, sha string } +) + +// stubCheckResolver scripts every CheckResolver call from test fixtures. +// Missing entries return *Unknown values (fail-open). +type stubCheckResolver struct { + refs map[stubRefKey]string // resolved ref → sha; absence = unknown + ancestry map[stubAncestryKey]resolve.AncestryStatus // (cand, head) ancestry decision + ancestryDetails map[stubAncestryKey]string // optional per-key detail string; absence = "" + reach map[stubReachKey]resolve.ReachabilityStatus // sha-reachable-from-ref decision + tagObjects map[stubTagObjectKey]string // sha → peeled commit +} + +func (s *stubCheckResolver) ResolveRef(owner, repo, ref string) (string, bool) { + if s == nil { + return "", false + } + sha, ok := s.refs[stubRefKey{owner, repo, ref}] + return sha, ok +} + +func (s *stubCheckResolver) CheckAncestry(_ context.Context, owner, repo, cand, head string) (resolve.AncestryStatus, string) { + if s == nil { + return resolve.AncestryUnknown, "" + } + key := stubAncestryKey{owner, repo, cand, head} + v, ok := s.ancestry[key] + if !ok { + return resolve.AncestryUnknown, s.ancestryDetails[key] + } + return v, s.ancestryDetails[key] +} + +func (s *stubCheckResolver) CheckReachability(owner, repo, sha, ref string) resolve.ReachabilityStatus { + if s == nil { + return resolve.ReachabilityUnknown + } + v, ok := s.reach[stubReachKey{owner, repo, sha, ref}] + if !ok { + return resolve.ReachabilityUnknown + } + return v +} + +func (s *stubCheckResolver) PeelTagObject(_ context.Context, owner, repo, sha string) (string, bool) { + if s == nil { + return "", false + } + commit, ok := s.tagObjects[stubTagObjectKey{owner, repo, sha}] + return commit, ok +} + +const ( + shaCheckoutV4 = "8e8c483db84b4bee98b60c0593521ed34d9990e8" + shaCheckoutV3 = "11bd71901bbe5b1630ceea73d27597364c9af683" + shaSetupGoV5 = "0aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + shaImpostor = "ffffffffffffffffffffffffffffffffffffffff" +) + +func checkPinKey(owner, repo, ref, sha string) string { + return owner + "/" + repo + "@" + ref + ":sha1-" + sha +} + +func checkNewLockfile(workflows map[string][]string) parserlock.File { + return parserlock.File{ + Version: parserlock.Version, + Workflows: workflows, + } +} + +func checkParsedWF(path string, uses ...parserlock.ActionRef) ParsedWorkflow { + return ParsedWorkflow{Path: path, Refs: uses} +} + +func checkRef(owner, repo, ref string) parserlock.ActionRef { + return parserlock.ActionRef{Owner: owner, Repo: repo, Ref: ref} +} + +func findingCategories(fs []Finding) []string { + out := make([]string, 0, len(fs)) + for _, f := range fs { + out = append(out, string(f.Category)) + } + sort.Strings(out) + return out +} + +// TestRunChecks groups per-category RunChecks integration cases that share +// the same lockfile + parsed-workflow + resolver setup. Cases that need to +// assert non-category aspects (Severity, ObservedSHA, Dependency.SHA, +// Confidence) hang those off the optional `extra` hook. +// +// TestRunChecks_AllFindingsCarryConfidence stays separate: it is a +// structural fail-fast guard that exercises every check path in one +// fixture. +func TestRunChecks(t *testing.T) { + const wfPath = ".github/workflows/ci.yml" + + cases := []struct { + name string + lockfile map[string][]string + workflowRefs []parserlock.ActionRef + resolver *stubCheckResolver + noResolver bool // when true, RunChecks gets nil instead of resolver + wantCategories []Category + // extra runs after category assertions for case-specific checks + // (Severity, ObservedSHA, Dependency.SHA, Confidence, etc). + extra func(t *testing.T, got []Finding) + }{ + { + name: "not-pinned: ref used but absent from lockfile", + lockfile: map[string][]string{}, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + wantCategories: []Category{NotPinned}, + extra: func(t *testing.T, got []Finding) { + if got[0].Severity != SeverityError { + t.Fatalf("expected error severity, got %s", got[0].Severity) + } + }, + }, + { + name: "sha-as-ref: workflow ref is a bare SHA", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", shaCheckoutV4, shaCheckoutV4)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", shaCheckoutV4)}, + wantCategories: []Category{ShaAsRef}, + }, + { + name: "ref-changed + stale: lockfile pins v4, workflow uses v3", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV4)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v3")}, + wantCategories: []Category{RefChanged, Stale}, + extra: func(t *testing.T, got []Finding) { + var refChanged *Finding + for i := range got { + if got[i].Category == RefChanged { + refChanged = &got[i] + } + } + if refChanged == nil || refChanged.Dependency == nil || refChanged.Dependency.SHA != shaCheckoutV4 { + t.Fatalf("expected ref-changed with locked sha %s, got %#v", shaCheckoutV4, refChanged) + } + }, + }, + { + name: "stale: lockfile pins ref no workflow uses", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV4)}, + }, + workflowRefs: nil, + wantCategories: []Category{Stale}, + }, + { + name: "no findings: pinned and current with reachable sha", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV4)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaCheckoutV4, + }, + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaCheckoutV4, "v4"}: resolve.Reachable, + }, + }, + wantCategories: nil, + }, + { + name: "ref-moved: pinned sha drifted but ancestry confirms", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV3)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaCheckoutV4, + }, + ancestry: map[stubAncestryKey]resolve.AncestryStatus{ + {"actions", "checkout", shaCheckoutV3, shaCheckoutV4}: resolve.AncestryConfirmed, + }, + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaCheckoutV3, "v4"}: resolve.Reachable, + }, + }, + wantCategories: []Category{RefMoved}, + extra: func(t *testing.T, got []Finding) { + if got[0].ObservedSHA != shaCheckoutV4 || got[0].Dependency == nil || got[0].Dependency.SHA != shaCheckoutV3 { + t.Fatalf("unexpected sha pair on finding: %#v", got[0]) + } + }, + }, + { + name: "lockfile-forgery: pinned sha is not an ancestor of upstream", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaImpostor)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaCheckoutV4, + }, + ancestry: map[stubAncestryKey]resolve.AncestryStatus{ + {"actions", "checkout", shaImpostor, shaCheckoutV4}: resolve.AncestryNotAncestor, + }, + }, + extra: func(t *testing.T, got []Finding) { + hasForgery := false + for _, f := range got { + if f.Category == LockfileForgery { + hasForgery = true + if f.Severity != SeverityError { + t.Fatalf("expected error severity, got %s", f.Severity) + } + if f.ObservedSHA != shaCheckoutV4 { + t.Fatalf("ObservedSHA: got %q, want %q (resolver output, makes claim falsifiable)", f.ObservedSHA, shaCheckoutV4) + } + if f.Dependency == nil || f.Dependency.SHA != shaImpostor { + t.Fatalf("Dependency.SHA: want pinned %s, got %#v", shaImpostor, f.Dependency) + } + } + } + if !hasForgery { + t.Fatalf("expected a lockfile-forgery finding, got %v", findingCategories(got)) + } + }, + }, + { + name: "impostor-commit: sha unreachable from ref and resolver doesn't know ref", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaImpostor)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + // Resolver doesn't know the ref → no ref-moved / forgery path. + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaImpostor, "v4"}: resolve.Unreachable, + }, + }, + wantCategories: []Category{ImpostorCommit}, + }, + { + name: "misleading-sha: sha-shaped ref resolves to different commit", + lockfile: map[string][]string{}, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", shaCheckoutV4)}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", shaCheckoutV4}: shaSetupGoV5, + }, + }, + extra: func(t *testing.T, got []Finding) { + hasMisleading := false + for _, f := range got { + if f.Category == MisleadingSHA { + hasMisleading = true + if f.ObservedSHA != shaSetupGoV5 { + t.Fatalf("ObservedSHA: got %q, want %q (resolver output, makes claim falsifiable)", f.ObservedSHA, shaSetupGoV5) + } + if f.Dependency == nil || f.Dependency.SHA != shaCheckoutV4 { + t.Fatalf("Dependency.SHA: want pinned %s (the SHA-shaped ref), got %#v", shaCheckoutV4, f.Dependency) + } + } + } + if !hasMisleading { + t.Fatalf("expected misleading-sha finding, got %v", findingCategories(got)) + } + }, + }, + { + // Covers the legitimate annotated-tag-object pin pattern (e.g. + // actions/github-script@): the resolver + // peels via ^{commit} so res.Sha is the underlying commit, not + // the pinned ref, but the pin is still immutable and must not + // trip misleading-sha. + name: "misleading-sha negative: tag-object SHA pin must not false-positive", + lockfile: map[string][]string{}, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "github-script", "d746ffe35508b1917358783b479e04febd2b8f71")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "github-script", "d746ffe35508b1917358783b479e04febd2b8f71"}: shaSetupGoV5, + }, + tagObjects: map[stubTagObjectKey]string{ + {"actions", "github-script", "d746ffe35508b1917358783b479e04febd2b8f71"}: shaSetupGoV5, + }, + }, + extra: func(t *testing.T, got []Finding) { + for _, f := range got { + if f.Category == MisleadingSHA { + t.Fatalf("did not expect misleading-sha for tag-object SHA pin, got %v", findingCategories(got)) + } + } + }, + }, + { + name: "no resolver: resolver-dependent checks are skipped", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV4)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + noResolver: true, + wantCategories: nil, + }, + { + // Compare API rate-limit fallback: ancestry is unknown so + // the SHA mismatch can't be classified as ref-moved or + // lockfile-forgery. Emit AncestryUnknown so + // consumers don't conflate "scan inconclusive" with valid. + name: "ancestry unknown emits ancestry-unknown, not ref-moved", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV3)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaCheckoutV4, + }, + // No ancestry entry → stub returns AncestryUnknown. + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaCheckoutV3, "v4"}: resolve.Reachable, + }, + }, + wantCategories: []Category{AncestryUnknown}, + extra: func(t *testing.T, got []Finding) { + if got[0].Category == Valid { + t.Fatalf("Category: ancestry-unknown must not regress to valid (Dependabot FindingMapper treats valid as clean)") + } + if got[0].Confidence != ConfidenceMedium { + t.Errorf("Confidence: got %q, want %q (AncestryUnknown is the rate-limit fallback path)", got[0].Confidence, ConfidenceMedium) + } + if got[0].Severity != SeverityWarning { + t.Errorf("Severity: got %q, want %q (inconclusive findings stay warnings)", got[0].Severity, SeverityWarning) + } + }, + }, + { + // Positive counterpart to the AncestryUnknown→medium case: + // when the Compare API gives us AncestryConfirmed the + // ref-moved finding is High-confidence because we have + // authoritative upstream data. + name: "ref-moved confidence: AncestryConfirmed is high", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV3)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaCheckoutV4, + }, + ancestry: map[stubAncestryKey]resolve.AncestryStatus{ + {"actions", "checkout", shaCheckoutV3, shaCheckoutV4}: resolve.AncestryConfirmed, + }, + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaCheckoutV3, "v4"}: resolve.Reachable, + }, + }, + wantCategories: []Category{RefMoved}, + extra: func(t *testing.T, got []Finding) { + if got[0].Confidence != ConfidenceHigh { + t.Errorf("Confidence: got %q, want %q (AncestryConfirmed is authoritative)", got[0].Confidence, ConfidenceHigh) + } + }, + }, + { + // Detail plumbing: when CheckAncestry returns a non-empty + // rate-limit detail (e.g. "rate limited (HTTP 429); resets + // at 1717552800"), the AncestryUnknown finding must surface + // it inside the parenthetical so operators don't see a + // generic "ancestry check inconclusive". + name: "ancestry unknown surfaces resolver detail", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV3)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaCheckoutV4, + }, + ancestryDetails: map[stubAncestryKey]string{ + {"actions", "checkout", shaCheckoutV3, shaCheckoutV4}: "rate limited (HTTP 429); resets at 1717552800; retry budget exhausted after 3 attempts", + }, + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaCheckoutV3, "v4"}: resolve.Reachable, + }, + }, + wantCategories: []Category{AncestryUnknown}, + extra: func(t *testing.T, got []Finding) { + if !strings.Contains(got[0].Detail, "rate limited (HTTP 429)") { + t.Errorf("Detail: expected resolver rate-limit detail, got %q", got[0].Detail) + } + if !strings.Contains(got[0].Detail, "resets at 1717552800") { + t.Errorf("Detail: expected reset timestamp in finding, got %q", got[0].Detail) + } + }, + }, + { + // Tag-hijacked-to-fork-network: locked SHA stays legit + // (still on a branch); live tag has been moved to a + // fork-network commit not on any upstream branch. + // AncestryConfirmed means CompareCommits returns + // "ahead": the live SHA descends from the lockfile + // commit (its parent is a real descendant), so ref-moved + // would otherwise be the only finding. The new + // liveRefImpostorFinding catches the live-SHA branch + // unreachability and escalates with a parallel + // impostor-commit error. + name: "ref-moved + impostor-commit: tag hijacked to fork-network commit", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV3)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaImpostor, + }, + ancestry: map[stubAncestryKey]resolve.AncestryStatus{ + {"actions", "checkout", shaCheckoutV3, shaImpostor}: resolve.AncestryConfirmed, + }, + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaCheckoutV3, "v4"}: resolve.Reachable, + {"actions", "checkout", shaImpostor, "v4"}: resolve.Unreachable, + }, + }, + wantCategories: []Category{ImpostorCommit, RefMoved}, + extra: func(t *testing.T, got []Finding) { + var impostor, refMoved *Finding + for i := range got { + switch got[i].Category { + case ImpostorCommit: + impostor = &got[i] + case RefMoved: + refMoved = &got[i] + } + } + if impostor == nil || refMoved == nil { + t.Fatalf("expected both impostor-commit and ref-moved, got %v", findingCategories(got)) + } + if impostor.Severity != SeverityError { + t.Errorf("impostor severity: got %s, want error", impostor.Severity) + } + if impostor.ObservedSHA != shaImpostor { + t.Errorf("impostor ObservedSHA: got %q, want %q (live SHA, the actual impostor)", impostor.ObservedSHA, shaImpostor) + } + if impostor.Dependency == nil || impostor.Dependency.SHA != shaImpostor { + t.Errorf("impostor Dependency.SHA: want live %q, got %#v (must differ from ref-moved finding so consumers can tell them apart)", shaImpostor, impostor.Dependency) + } + if refMoved.Dependency == nil || refMoved.Dependency.SHA != shaCheckoutV3 { + t.Errorf("ref-moved Dependency.SHA: want locked %q, got %#v", shaCheckoutV3, refMoved.Dependency) + } + if !strings.Contains(impostor.Detail, "fork-network injection") { + t.Errorf("impostor Detail: want fork-network wording, got %q", impostor.Detail) + } + }, + }, + { + // Negative: when the live SHA *is* reachable from a + // branch, the move is benign (release-train style). + // Only ref-moved should fire — no parallel impostor. + name: "ref-moved only: live SHA reachable means benign move", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV3)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaCheckoutV4, + }, + ancestry: map[stubAncestryKey]resolve.AncestryStatus{ + {"actions", "checkout", shaCheckoutV3, shaCheckoutV4}: resolve.AncestryConfirmed, + }, + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaCheckoutV3, "v4"}: resolve.Reachable, + {"actions", "checkout", shaCheckoutV4, "v4"}: resolve.Reachable, + }, + }, + wantCategories: []Category{RefMoved}, + }, + { + // Fail-open: reach result Unknown for live SHA (cache miss, + // rate limit) must not escalate to impostor-commit. Same + // fallback policy as the locked-SHA path. + name: "ref-moved only: live-SHA reachability unknown stays benign", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV3)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaCheckoutV4, + }, + ancestry: map[stubAncestryKey]resolve.AncestryStatus{ + {"actions", "checkout", shaCheckoutV3, shaCheckoutV4}: resolve.AncestryConfirmed, + }, + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaCheckoutV3, "v4"}: resolve.Reachable, + // no live-SHA entry → ReachabilityUnknown + }, + }, + wantCategories: []Category{RefMoved}, + }, + { + // Forgery suppression: when ancestry says + // AncestryNotAncestor the lockfile is forged. Do NOT + // emit a parallel impostor-commit even if the live SHA + // is also unreachable — forgery is the stronger claim + // and double-flagging clutters without adding action. + name: "forgery suppresses live impostor", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaImpostor)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaCheckoutV4, + }, + ancestry: map[stubAncestryKey]resolve.AncestryStatus{ + {"actions", "checkout", shaImpostor, shaCheckoutV4}: resolve.AncestryNotAncestor, + }, + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaCheckoutV4, "v4"}: resolve.Unreachable, + }, + }, + extra: func(t *testing.T, got []Finding) { + cats := findingCategories(got) + for _, c := range cats { + if c == string(ImpostorCommit) { + t.Fatalf("forgery branch must not emit parallel impostor-commit, got %v", cats) + } + } + hasForgery := false + for _, c := range cats { + if c == string(LockfileForgery) { + hasForgery = true + } + } + if !hasForgery { + t.Fatalf("expected lockfile-forgery, got %v", cats) + } + }, + }, + { + // AncestryUnknown + live SHA unreachable: both signals + // must surface. ancestry-unknown says "we can't tell if + // this is a release move or a forgery", impostor-commit + // says "live SHA is on no branch — investigate". They + // answer different questions, so emit both. + name: "ancestry-unknown + impostor: independent signals coexist", + lockfile: map[string][]string{ + wfPath: {checkPinKey("actions", "checkout", "v4", shaCheckoutV3)}, + }, + workflowRefs: []parserlock.ActionRef{checkRef("actions", "checkout", "v4")}, + resolver: &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaImpostor, + }, + // No ancestry entry → AncestryUnknown. + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaCheckoutV3, "v4"}: resolve.Reachable, + {"actions", "checkout", shaImpostor, "v4"}: resolve.Unreachable, + }, + }, + wantCategories: []Category{AncestryUnknown, ImpostorCommit}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + lf := checkNewLockfile(tc.lockfile) + pw := checkParsedWF(wfPath, tc.workflowRefs...) + var r CheckResolver + if !tc.noResolver && tc.resolver != nil { + r = tc.resolver + } + got := RunChecks(context.Background(), pw, lf, r) + + if tc.wantCategories != nil { + wantStrs := make([]string, len(tc.wantCategories)) + for i, c := range tc.wantCategories { + wantStrs[i] = string(c) + } + sort.Strings(wantStrs) + gotStrs := findingCategories(got) + if len(gotStrs) != len(wantStrs) { + t.Fatalf("findings: got %v, want %v (full: %#v)", gotStrs, wantStrs, got) + } + for i := range wantStrs { + if gotStrs[i] != wantStrs[i] { + t.Fatalf("findings: got %v, want %v (full: %#v)", gotStrs, wantStrs, got) + } + } + } + + if tc.extra != nil { + tc.extra(t, got) + } + }) + } +} + +// TestRunChecks_AllFindingsCarryConfidence is the fail-fast guard the +// confidence-axis card requires: every finding emitted by any check +// path must carry a non-empty Confidence. A zero value here would mean +// a new check (or an edit to an existing one) forgot to set the field, +// and the JSON/SARIF surface would leak `""`. +func TestRunChecks_AllFindingsCarryConfidence(t *testing.T) { + // Cover every check path RunChecks dispatches to. + lf := checkNewLockfile(map[string][]string{ + ".github/workflows/ci.yml": { + checkPinKey("actions", "checkout", "v4", shaCheckoutV3), // ref-moved/forgery seed + checkPinKey("actions", "unused", "v1", shaSetupGoV5), // stale seed + }, + }) + pw := checkParsedWF(".github/workflows/ci.yml", + checkRef("actions", "checkout", "v4"), // ref-moved or forgery + checkRef("actions", "setup-node", "v3"), // not-pinned + checkRef("actions", "bare-sha", shaImpostor), // sha-as-ref + misleading + ) + r := &stubCheckResolver{ + refs: map[stubRefKey]string{ + {"actions", "checkout", "v4"}: shaCheckoutV4, + {"actions", "bare-sha", shaImpostor}: shaSetupGoV5, + }, + ancestry: map[stubAncestryKey]resolve.AncestryStatus{ + {"actions", "checkout", shaCheckoutV3, shaCheckoutV4}: resolve.AncestryConfirmed, + }, + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaCheckoutV3, "v4"}: resolve.Reachable, + }, + } + got := RunChecks(context.Background(), pw, lf, r) + if len(got) == 0 { + t.Fatal("expected findings to exercise the confidence guard") + } + for i, f := range got { + if f.Confidence == "" { + t.Errorf("finding[%d] category=%s has empty Confidence — every construction site must set it", i, f.Category) + } + } +} + +// --- impostor-commit fail-open guards ------------------------------------- +// +// The locked-SHA impostor check (checkImpostorCommit) must only fire on an +// authoritative Unreachable verdict. When reachability is Unknown (rate +// limit / transient API failure) it must fail open: a scanner that cried +// "injected lockfile entry" every time the GitHub API hiccupped would be +// worse than useless. These tests pin that contract. + +func impostorFixture(reach resolve.ReachabilityStatus) (ParsedWorkflow, map[string]parserlock.Pin, *stubCheckResolver) { + ref := checkRef("actions", "checkout", "v4") + pw := checkParsedWF(".github/workflows/ci.yml", ref) + depIndex := map[string]parserlock.Pin{ + parserlock.IndexKey("actions", "checkout", "v4"): { + NWO: "actions/checkout", Owner: "actions", Repo: "checkout", + Ref: "v4", Algo: "sha1", Hex: shaImpostor, + }, + } + r := &stubCheckResolver{ + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaImpostor, "v4"}: reach, + }, + } + return pw, depIndex, r +} + +func TestCheckImpostorCommit_UnreachableEmitsFinding(t *testing.T) { + pw, depIndex, r := impostorFixture(resolve.Unreachable) + + out := checkImpostorCommit(pw, depIndex, r, nil) + if len(out) != 1 { + t.Fatalf("Unreachable: got %d findings, want 1 (%v)", len(out), findingCategories(out)) + } + if out[0].Category != ImpostorCommit { + t.Fatalf("category = %v, want ImpostorCommit", out[0].Category) + } +} + +func TestCheckImpostorCommit_ReachabilityUnknownFailsOpen(t *testing.T) { + pw, depIndex, r := impostorFixture(resolve.ReachabilityUnknown) + + out := checkImpostorCommit(pw, depIndex, r, nil) + for _, f := range out { + if f.Category == ImpostorCommit { + t.Fatalf("Unknown reachability must not emit ImpostorCommit (got %v)", findingCategories(out)) + } + } +} + +func TestLiveRefImpostorFinding_ReachabilityUnknownFailsOpen(t *testing.T) { + ref := checkRef("actions", "checkout", "v4") + pw := checkParsedWF(".github/workflows/ci.yml", ref) + r := &stubCheckResolver{ + reach: map[stubReachKey]resolve.ReachabilityStatus{ + {"actions", "checkout", shaImpostor, "v4"}: resolve.ReachabilityUnknown, + }, + } + + if _, ok := liveRefImpostorFinding(pw, ref, shaImpostor, r); ok { + t.Fatal("Unknown reachability must not produce a live-ref impostor finding") + } +} diff --git a/internal/pipeline/checks/structural.go b/internal/pipeline/checks/structural.go new file mode 100644 index 00000000..bf70f259 --- /dev/null +++ b/internal/pipeline/checks/structural.go @@ -0,0 +1,150 @@ +package checks + +import ( + "fmt" + "strings" + + parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" + "github.com/github/gh-actions-pin/internal/dep" +) + +// checkNotPinned emits NotPinned for any uses: ref that has no +// matching lockfile entry. SHA-shaped refs are reported under their own +// category. When the lockfile has an entry for the same action at a +// different ref, RefChanged wins. +func checkNotPinned(pw ParsedWorkflow, depPins []parserlock.Pin, depIndex map[string]parserlock.Pin) []Finding { + if len(pw.Refs) == 0 { + return nil + } + knownAction := make(map[string]bool, len(depPins)) + for _, p := range depPins { + knownAction[nwoLower(p.Owner, p.Repo)] = true + } + var out []Finding + for _, ref := range pw.Refs { + if parserlock.IsFullSha(ref.Ref) { + continue + } + if _, ok := depIndex[parserlock.IndexKey(ref.Owner, ref.Repo, ref.Ref)]; ok { + continue + } + if knownAction[nwoLower(ref.Owner, ref.Repo)] { + continue + } + f := newRefFinding(pw, ref, NotPinned, SeverityError, ConfidenceHigh) + f.Detail = fmt.Sprintf("used in workflow but not pinned in lockfile (%s@%s)", formatUseName(ref.Owner, ref.Repo, ref.Path), ref.Ref) + f.Remediation = "pin with `gh actions-pin`" + out = append(out, f) + } + return out +} + +// checkShaAsRef emits ShaAsRef for any uses: ref that is a bare +// commit SHA — both bare-SHA uses with no lock entry and bare-SHA uses +// whose lock entry just mirrors the same SHA. The anti-pattern (no +// human-readable ref) is the same in both cases. +func checkShaAsRef(pw ParsedWorkflow, depIndex map[string]parserlock.Pin) []Finding { + var out []Finding + for _, ref := range pw.Refs { + if !parserlock.IsFullSha(ref.Ref) { + continue + } + f := newRefFinding(pw, ref, ShaAsRef, SeverityWarning, ConfidenceHigh) + f.Detail = "pinned to a bare SHA without a symbolic ref — weakens supply-chain traceability" + f.Remediation = fmt.Sprintf("pin to a tag instead: https://github.com/%s/releases", nwoLower(ref.Owner, ref.Repo)) + lockedSha := ref.Ref + if locked, ok := depIndex[parserlock.IndexKey(ref.Owner, ref.Repo, ref.Ref)]; ok { + lockedSha = locked.Hex + } + f.Dependency = synthDep(ref, lockedSha) + out = append(out, f) + } + return out +} + +// checkRefChanged emits RefChanged when the workflow's uses: ref +// differs from the lockfile entry's ref for the same action (owner/repo). +// A single action may legitimately have multiple pinned refs across +// workflows, so this only fires when no pin matches the workflow's ref. +func checkRefChanged(pw ParsedWorkflow, depPins []parserlock.Pin) []Finding { + if len(depPins) == 0 { + return nil + } + pinsByAction := make(map[string][]parserlock.Pin, len(depPins)) + for _, p := range depPins { + k := nwoLower(p.Owner, p.Repo) + pinsByAction[k] = append(pinsByAction[k], p) + } + var out []Finding + for _, ref := range pw.Refs { + if parserlock.IsFullSha(ref.Ref) { + continue + } + key := nwoLower(ref.Owner, ref.Repo) + candidates, ok := pinsByAction[key] + if !ok { + continue + } + match := false + for _, p := range candidates { + if p.Ref == ref.Ref { + match = true + break + } + } + if match { + continue + } + p := candidates[0] + f := newRefFinding(pw, ref, RefChanged, SeverityError, ConfidenceHigh) + f.Detail = fmt.Sprintf("workflow uses ref %q but lockfile pins %q", ref.Ref, p.Ref) + f.Remediation = "re-run `gh actions-pin` to refresh the lockfile, or revert the uses: line" + f.Dependency = synthDep(ref, p.Hex) + out = append(out, f) + } + return out +} + +// checkStale emits Stale for lockfile dep entries that no uses: +// ref in the workflow references. If the workflow has already been +// rewritten to pin by SHA, the lockfile entry (keyed by the original tag) +// is still valid — surface keys both ways so we don't false-flag. +func checkStale(pw ParsedWorkflow, depPins []parserlock.Pin) []Finding { + if len(depPins) == 0 { + return nil + } + used := make(map[string]bool, len(pw.Refs)) + usedBySHA := make(map[string]bool, len(pw.Refs)) + for _, ref := range pw.Refs { + used[parserlock.IndexKey(ref.Owner, ref.Repo, ref.Ref)] = true + nwo := strings.ToLower(ref.Owner + "/" + ref.Repo) + usedBySHA[nwo+"@"+strings.ToLower(ref.Ref)] = true + } + var out []Finding + for _, p := range depPins { + if used[p.IndexKey()] { + continue + } + if p.Hex != "" { + nwo := strings.ToLower(p.NWO) + if usedBySHA[nwo+"@"+strings.ToLower(p.Hex)] { + continue + } + } + f := Finding{ + WorkflowPath: pw.Path, + Category: Stale, + Severity: SeverityWarning, + Confidence: ConfidenceHigh, + Detail: fmt.Sprintf("lockfile pins %s@%s but no uses: in this workflow references it", nwoLower(p.Owner, p.Repo), p.Ref), + Remediation: "remove the entry or re-run `gh actions-pin`", + Dependency: &dep.Dependency{ + NWO: strings.ToLower(p.NWO), + Ref: p.Ref, + SHA: p.Hex, + }, + } + out = append(out, f) + } + return out +} diff --git a/internal/pipeline/diagnose.go b/internal/pipeline/diagnose.go new file mode 100644 index 00000000..d771b659 --- /dev/null +++ b/internal/pipeline/diagnose.go @@ -0,0 +1,222 @@ +// Package pipeline orchestrates the scan, resolve, check, and report +// flow for a single run. +package pipeline + +import ( + "context" + "fmt" + + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/ghapi" + "github.com/github/gh-actions-pin/internal/lockfile" + "github.com/github/gh-actions-pin/internal/pinpool" + "github.com/github/gh-actions-pin/internal/pipeline/checks" + "github.com/github/gh-actions-pin/internal/resolve" +) + +// DiagnoseParsed runs the engine diagnostics for each pre-parsed workflow. +// Assumes the resolver caches have already been warmed (calls into the +// resolver will hit cache and stay silent). Returns a checks.Report aggregating per- +// workflow findings in input order. +func DiagnoseParsed(ctx context.Context, parsed []checks.ParsedWorkflow, r *resolve.Resolver, store *lockfile.State, pool *pinpool.Pool) *checks.Report { + type indexedPW struct { + idx int + pw checks.ParsedWorkflow + } + items := make([]indexedPW, len(parsed)) + for i, pw := range parsed { + items[i] = indexedPW{idx: i, pw: pw} + } + + results := make([]checks.WorkflowReport, len(parsed)) + _ = pinpool.RunTyped(pool, ctx, "Diagnosing workflows", + items, + func(ipw indexedPW) string { return ipw.pw.Path }, + func(ctx context.Context, _ int, ipw indexedPW) error { + effR := r + if ipw.pw.Resolved { + effR = nil + } + results[ipw.idx] = diagnoseOneParsed(ctx, ipw.pw, effR, store, pool) + return nil + }, + ) + + return &checks.Report{Workflows: results} +} + +func diagnoseOneParsed(ctx context.Context, pw checks.ParsedWorkflow, r *resolve.Resolver, store *lockfile.State, pool *pinpool.Pool) checks.WorkflowReport { + wr := checks.WorkflowReport{Path: pw.Path} + + if pw.LoadErr != nil { + wr.Findings = append(wr.Findings, checks.Finding{ + WorkflowPath: pw.Path, + Category: checks.NotPinned, + Severity: checks.SeverityError, + // High: the YAML failed to load — concrete, file-level fact. + Confidence: checks.ConfidenceHigh, + Detail: fmt.Sprintf("failed to load workflow: %s", pw.LoadErr), + DocURL: DocURLFor(checks.NotPinned), + }) + return wr + } + + wr.ActionRefs = pw.Refs + wr.ParseWarnings = pw.ParseWarnings + + if len(pw.Refs) == 0 { + wr.Findings = append(wr.Findings, checks.Finding{ + WorkflowPath: pw.Path, + Category: checks.RunOnly, + Severity: checks.SeverityOK, + Confidence: checks.ConfidenceHigh, + Detail: "no action references found", + }) + return wr + } + + if pw.DepsErr != nil { + wr.Findings = append(wr.Findings, checks.Finding{ + WorkflowPath: pw.Path, + Category: checks.NotPinned, + Severity: checks.SeverityError, + Confidence: checks.ConfidenceHigh, + Detail: fmt.Sprintf("failed to read dependencies: %s", pw.DepsErr), + Remediation: "fix or regenerate the dependencies: section with `gh actions-pin`", + DocURL: DocURLFor(checks.NotPinned), + }) + return wr + } + wr.Deps = pw.ExistingDeps + + directNWOs := make(map[ghapi.Repo]bool, len(pw.Refs)) + for _, ref := range pw.Refs { + directNWOs[ghapi.ForRepo(ref.Owner, ref.Repo)] = true + } + + // Resolve live state: hits cache when ParseAll's caller pre-warmed the + // resolver. Failure degrades to structural-only checks for any refs that + // couldn't be resolved — partial results are kept. + var liveDeps []dep.Dependency + var resolvedParents dep.ParentMap + if r != nil { + var resolveErr error + liveDeps, resolvedParents, resolveErr = r.ResolveAllRecursive(ctx, pw.Refs) + if resolveErr != nil { + // Low: we're surfacing the resolver failure itself, not a + // verdict about any specific dependency. + wr.Findings = append(wr.Findings, checks.Finding{ + WorkflowPath: pw.Path, + Category: checks.ReachabilityUnknown, + Severity: checks.SeverityWarning, + Confidence: checks.ConfidenceLow, + Detail: fmt.Sprintf("could not re-resolve actions: %s", resolveErr), + }) + } + } + + for _, dep := range pw.ExistingDeps { + owner, repo := dep.OwnerRepo() + wr.Inventory = append(wr.Inventory, checks.InventoryEntry{ + Dep: dep, + File: pw.Path, + Direct: directNWOs[ghapi.ForRepo(owner, repo)], + }) + } + parentMap := map[string][]string{} + if r != nil { + parentMap = resolvedParents + populateInventoryParents(wr.Inventory, parentMap) + } + + var reach []resolve.ReachabilityResult + if r != nil && len(pw.ExistingDeps) > 0 { + toCheck, trusted := partitionReachByLive(pw.ExistingDeps, liveDeps, pw.SkipReachWhenUnchanged) + reach = trusted + if len(toCheck) > 0 { + reach = append(reach, r.CheckReachabilityAll(ctx, toCheck)...) + } + } + // Independent sweep for LIVE SHAs whose tag has moved: the + // tag-hijacked-to-fork-network shape is invisible to the locked-SHA + // sweep above (the lockfile entry is still legitimate; the live + // SHA is the impostor). Kept separate so the result map's + // (NWO, Ref, SHA) keys don't shadow the lockfile sweep — they + // share NWO@Ref dep keys, which would confuse + // reachabilityComplementFindings if mixed into `reach`. + var liveMovedReach []resolve.ReachabilityResult + if r != nil && len(liveDeps) > 0 && len(pw.ExistingDeps) > 0 { + if moved := liveMovedDeps(pw.ExistingDeps, liveDeps); len(moved) > 0 { + liveMovedReach = r.CheckReachabilityAll(ctx, moved) + } + } + // Pin-time parity sweep: any (NWO, Ref, LIVE SHA) that neither the + // locked-SHA sweep nor the tag-moved sweep covers gets a fresh reach + // check here. Catches the NotPinned-direct impostor case and any + // transitive composite live dep that isn't in the lockfile yet. With + // this in place, applyPin's reach-loop Unreachable branch becomes a + // fail-loud invariant rather than a primary detection path. + var liveDirectReach []resolve.ReachabilityResult + if r != nil && len(liveDeps) > 0 { + if extra := liveDirectReachDeps(pw, liveDeps); len(extra) > 0 { + liveDirectReach = r.CheckReachabilityAll(ctx, extra) + } + } + var checkR checks.CheckResolver + if r != nil && liveDeps != nil { + checkR = checks.NewPrewarmedResolver(r, liveDeps, reach, liveMovedReach, liveDirectReach) + } + rawFindings := checks.RunChecks(ctx, pw, store.File(), checkR) + + depByKey := indexDeps(pw.ExistingDeps) + for _, f := range rawFindings { + if f.Category == checks.Stale && isTransitivePin(f, depByKey, parentMap) { + continue + } + attachParent(&f, depByKey, directNWOs, parentMap) + f.DocURL = DocURLFor(f.Category) + wr.Findings = append(wr.Findings, f) + } + + if len(reach) > 0 { + wr.Findings = append(wr.Findings, reachabilityComplementFindings(pw.Path, reach, pw.ExistingDeps, directNWOs, parentMap, wr.Findings)...) + } + if len(liveDirectReach) > 0 { + wr.Findings = append(wr.Findings, liveReachImpostorFindings(pw.Path, liveDirectReach, liveDeps, directNWOs, parentMap, wr.Findings)...) + } + + if !hasIssues(wr.Findings) { + wr.Findings = append(wr.Findings, checks.Finding{ + WorkflowPath: pw.Path, + Category: checks.Valid, + Severity: checks.SeverityOK, + Confidence: checks.ConfidenceHigh, + Detail: "all dependencies pinned and verified", + }) + } + + return wr +} + +func indexDeps(deps []dep.Dependency) map[string]dep.Dependency { + out := make(map[string]dep.Dependency, len(deps)) + for _, dep := range deps { + out[dep.Key()] = dep + } + return out +} + +func hasIssues(ff []checks.Finding) bool { + for _, f := range ff { + if f.Severity == checks.SeverityError { + return true + } + if f.Category.IsInconclusive() { + continue + } + if f.Category != checks.Valid && f.Category != checks.RunOnly && f.Severity == checks.SeverityWarning { + return true + } + } + return false +} diff --git a/internal/pipeline/doc_urls.go b/internal/pipeline/doc_urls.go new file mode 100644 index 00000000..a5eded25 --- /dev/null +++ b/internal/pipeline/doc_urls.go @@ -0,0 +1,89 @@ +package pipeline + +import "github.com/github/gh-actions-pin/internal/pipeline/checks" + +// Documentation URLs for each finding Category. +// +// Parity twin of the TypeScript engine's doc URL table at +// languageservices/workflow-parser/src/lockfile/diagnostics/doc-urls.ts. +// Strings here MUST stay in sync with that table — the CLI and editor +// present the same link to users so the experience is identical. +// +// Categories that don't yet have a dedicated docs anchor fall back to +// the canonical "using third-party actions" page so users always land +// somewhere actionable. + +const securityHardeningBase = "https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions" + +// PublisherTagReleasesDocURL points to GitHub's guidance for action publishers +// on tagging releases from a branch. It's surfaced alongside impostor-commit +// findings to help users escalate to the action's maintainer when the pinned +// SHA is orphaned (off any branch) — a publisher behavior the consumer can't +// fix locally beyond re-pinning to a sane release. +const PublisherTagReleasesDocURL = "https://docs.github.com/en/actions/how-tos/create-and-publish-actions/manage-custom-actions#using-tags-for-release-management" + +// PublisherEscalationCopy is the standardized one-liner shown in any block +// where a SHA fell off-branch on the publisher side. Phrased as a direct +// instruction to maintainers so users have a copy-paste sentence to drop +// into an issue or release-process discussion when escalating. +const PublisherEscalationCopy = "Actions publishers should ensure released actions are reachable from a branch. Otherwise, they are indistinguishable from impostor commits" + +// docURLs maps every Category that can appear on a checks.Finding to its +// documentation URL. Categories representing "no issue" (Valid, RunOnly) +// have no URL — they aren't rendered as findings. +var docURLs = map[checks.Category]string{ + checks.NotPinned: securityHardeningBase + "#using-third-party-actions", + checks.ShaAsRef: securityHardeningBase + "#using-third-party-actions", + checks.RefChanged: securityHardeningBase + "#using-third-party-actions", + checks.Stale: securityHardeningBase + "#using-third-party-actions", + checks.MisleadingSHA: securityHardeningBase + "#using-third-party-actions", + checks.RefMoved: securityHardeningBase + "#using-third-party-actions", + checks.LockfileForgery: securityHardeningBase + "#using-third-party-actions", + checks.ImpostorCommit: securityHardeningBase + "#using-third-party-actions", + checks.OnboardingRequired: securityHardeningBase + "#using-third-party-actions", + checks.AncestryUnknown: securityHardeningBase + "#using-third-party-actions", + checks.ReachabilityUnknown: securityHardeningBase + "#using-third-party-actions", +} + +// DocURLFor returns the documentation URL for a finding category, or "" +// when the category has no associated URL (e.g. checks.Valid). +func DocURLFor(c checks.Category) string { + return docURLs[c] +} + +// ReleasesURL returns the GitHub releases URL for an action. When ref +// looks like a tag, links to the specific release; otherwise links to +// the releases index so users can pick one. +func ReleasesURL(owner, repo, ref string) string { + base := "https://github.com/" + owner + "/" + repo + "/releases" + if isLikelyTag(ref) { + return base + "/tag/" + ref + } + return base +} + +// isLikelyTag mirrors the heuristic in the TS doc-urls module: anything +// that isn't a full SHA and isn't a well-known branch name is treated as +// a tag. Worst case the user gets a 404 and falls back to /releases. +func isLikelyTag(ref string) bool { + if ref == "" || ref == "main" || ref == "master" || ref == "trunk" { + return false + } + if len(ref) == 40 && isHex(ref) { + return false + } + return true +} + +func isHex(s string) bool { + for _, c := range s { + switch { + case c >= '0' && c <= '9': + case c >= 'a' && c <= 'f': + case c >= 'A' && c <= 'F': + default: + return false + } + } + return true +} diff --git a/internal/pipeline/finding_enrich.go b/internal/pipeline/finding_enrich.go new file mode 100644 index 00000000..83587e76 --- /dev/null +++ b/internal/pipeline/finding_enrich.go @@ -0,0 +1,62 @@ +package pipeline + +import ( + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/ghapi" + "github.com/github/gh-actions-pin/internal/pipeline/checks" +) + +// attachParent looks up the dep's composite-expansion parents (if any) +// and surfaces the first one as checks.Finding.ParentNWO. Direct (workflow-level) +// uses don't get a parent attached even if one exists in the graph. +// +// Findings emitted by RunChecks already carry an ActionRef for direct uses +// and a Dependency synthesized from the workflow ref / lockfile pin. This +// is purely about pointing the user at the composite that pulled in a +// transitively-pinned dep. +func attachParent(f *checks.Finding, depByKey map[string]dep.Dependency, directNWOs map[ghapi.Repo]bool, parentMap map[string][]string) { + if f.Dependency == nil { + return + } + owner, repo := f.Dependency.OwnerRepo() + if directNWOs[ghapi.ForRepo(owner, repo)] { + return + } + // Prefer the dep snapshot from the workflow's ExistingDeps (it has the + // canonical NWO casing the parent map keys with). Synthesised deps + // already match — but the indexed lookup is cheap regardless. + key := f.Dependency.Key() + if dep, ok := depByKey[key]; ok { + key = dep.Key() + } + if parents := parentMap[key]; len(parents) > 0 { + f.ParentNWO = parents[0] + } +} + +// isTransitivePin reports whether the finding refers to a dep reached via +// composite expansion (i.e. has parents in the parent map). +func isTransitivePin(f checks.Finding, depByKey map[string]dep.Dependency, parentMap map[string][]string) bool { + if f.Dependency == nil { + return false + } + if _, ok := depByKey[f.Dependency.Key()]; !ok { + return false + } + return len(parentMap[f.Dependency.Key()]) > 0 +} + +// populateInventoryParents fills in the Parents field for transitive inventory +// entries (those not marked Direct and without parents yet) by looking up each +// entry's dep key in parentMap. +func populateInventoryParents(inventory []checks.InventoryEntry, parentMap map[string][]string) { + for i := range inventory { + if inventory[i].Direct || len(inventory[i].Parents) > 0 { + continue + } + parents := parentMap[inventory[i].Dep.Key()] + if len(parents) > 0 { + inventory[i].Parents = append([]string(nil), parents...) + } + } +} diff --git a/internal/pipeline/impostor_parity_test.go b/internal/pipeline/impostor_parity_test.go new file mode 100644 index 00000000..0a240f34 --- /dev/null +++ b/internal/pipeline/impostor_parity_test.go @@ -0,0 +1,298 @@ +package pipeline + +import ( + "testing" + + "github.com/github/gh-actions-pin/internal/pipeline/checks" + + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/ghapi" + "github.com/github/gh-actions-pin/internal/resolve" +) + +const ( + testShaImpostor = "ffffffffffffffffffffffffffffffffffffffff" + testShaCheckoutV4 = "8e8c483db84b4bee98b60c0593521ed34d9990e8" + testShaCheckoutV3 = "11bd71901bbe5b1630ceea73d27597364c9af683" + testShaSetupGoV5 = "0aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +) + +// TestLiveReachImpostorFindings_Parity proves the pre-pin live-direct sweep +// emits checks.ImpostorCommit for every shape that today's post-pin +// alertImpostor sweep would catch (and only those shapes). Each case is the +// pre-condition for deleting AutoFixAlertedImposters from apply.go. +func TestLiveReachImpostorFindings_Parity(t *testing.T) { + const wfPath = ".github/workflows/ci.yml" + + directNWO := map[ghapi.Repo]bool{ + ghapi.ForRepo("actions", "checkout"): true, + } + transitiveDirectNWO := map[ghapi.Repo]bool{ + ghapi.ForRepo("actions", "cache"): true, + // someorg/helper is transitive, no entry here + } + + tests := []struct { + name string + reach []resolve.ReachabilityResult + live []dep.Dependency + directNWOs map[ghapi.Repo]bool + parentMap map[string][]string + existing []checks.Finding + wantCount int + wantCategory checks.Category + wantParentSet bool + wantSHA string + }{ + { + name: "unpinned direct ref resolves to unreachable SHA", + reach: []resolve.ReachabilityResult{{ + Owner: "actions", Repo: "checkout", Ref: "v4", SHA: testShaImpostor, + DepKey: "actions/checkout@v4", + Status: resolve.Unreachable, + Detail: "no branch contains commit", + }}, + live: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaImpostor}, + }, + directNWOs: directNWO, + wantCount: 1, + wantCategory: checks.ImpostorCommit, + wantSHA: testShaImpostor, + }, + { + name: "unpinned transitive dep (different NWO) resolves to unreachable SHA", + reach: []resolve.ReachabilityResult{{ + Owner: "someorg", Repo: "helper", Ref: "v1", SHA: testShaImpostor, + DepKey: "someorg/helper@v1", + Status: resolve.Unreachable, + }}, + live: []dep.Dependency{ + {NWO: "someorg/helper", Ref: "v1", SHA: testShaImpostor}, + }, + directNWOs: transitiveDirectNWO, + parentMap: map[string][]string{ + "someorg/helper@v1": {"actions/cache@v4"}, + }, + wantCount: 1, + wantCategory: checks.ImpostorCommit, + wantParentSet: true, + wantSHA: testShaImpostor, + }, + { + name: "reachable live SHA emits nothing", + reach: []resolve.ReachabilityResult{{ + Owner: "actions", Repo: "checkout", Ref: "v4", SHA: testShaCheckoutV4, + DepKey: "actions/checkout@v4", + Status: resolve.Reachable, + }}, + live: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV4}, + }, + directNWOs: directNWO, + wantCount: 0, + }, + { + name: "suppressed when prior checks.ImpostorCommit already covers dep", + reach: []resolve.ReachabilityResult{{ + Owner: "actions", Repo: "checkout", Ref: "v4", SHA: testShaImpostor, + DepKey: "actions/checkout@v4", + Status: resolve.Unreachable, + }}, + live: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaImpostor}, + }, + directNWOs: directNWO, + existing: []checks.Finding{{ + WorkflowPath: wfPath, + Category: checks.ImpostorCommit, + Dependency: &dep.Dependency{NWO: "actions/checkout", Ref: "v4", SHA: testShaImpostor}, + }}, + wantCount: 0, + }, + { + name: "suppressed when prior checks.LockfileForgery covers dep", + reach: []resolve.ReachabilityResult{{ + Owner: "actions", Repo: "checkout", Ref: "v4", SHA: testShaImpostor, + DepKey: "actions/checkout@v4", + Status: resolve.Unreachable, + }}, + live: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaImpostor}, + }, + directNWOs: directNWO, + existing: []checks.Finding{{ + WorkflowPath: wfPath, + Category: checks.LockfileForgery, + Dependency: &dep.Dependency{NWO: "actions/checkout", Ref: "v4", SHA: testShaImpostor}, + }}, + wantCount: 0, + }, + { + name: "unknown status emits nothing (only Unreachable fires)", + reach: []resolve.ReachabilityResult{{ + Owner: "actions", Repo: "checkout", Ref: "v4", SHA: testShaImpostor, + DepKey: "actions/checkout@v4", + Status: resolve.ReachabilityUnknown, + }}, + live: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaImpostor}, + }, + directNWOs: directNWO, + wantCount: 0, + }, + { + name: "deduplicates second reach result for same dep", + reach: []resolve.ReachabilityResult{ + { + Owner: "actions", Repo: "checkout", Ref: "v4", SHA: testShaImpostor, + DepKey: "actions/checkout@v4", + Status: resolve.Unreachable, + }, + { + Owner: "actions", Repo: "checkout", Ref: "v4", SHA: testShaImpostor, + DepKey: "actions/checkout@v4", + Status: resolve.Unreachable, + }, + }, + live: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaImpostor}, + }, + directNWOs: directNWO, + wantCount: 1, + wantCategory: checks.ImpostorCommit, + wantSHA: testShaImpostor, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := liveReachImpostorFindings(wfPath, tc.reach, tc.live, tc.directNWOs, tc.parentMap, tc.existing) + if len(got) != tc.wantCount { + t.Fatalf("got %d findings, want %d: %+v", len(got), tc.wantCount, got) + } + if tc.wantCount == 0 { + return + } + f := got[0] + if f.Category != tc.wantCategory { + t.Errorf("category = %s, want %s", f.Category, tc.wantCategory) + } + if f.Severity != checks.SeverityError { + t.Errorf("severity = %s, want %s", f.Severity, checks.SeverityError) + } + if f.Confidence != checks.ConfidenceHigh { + t.Errorf("confidence = %s, want %s", f.Confidence, checks.ConfidenceHigh) + } + if f.Dependency == nil || f.Dependency.SHA != tc.wantSHA { + t.Errorf("dependency SHA = %v, want %s", f.Dependency, tc.wantSHA) + } + if tc.wantParentSet && f.ParentNWO == "" { + t.Error("expected ParentNWO to be set for transitive case") + } + if !tc.wantParentSet && f.ParentNWO != "" { + t.Errorf("ParentNWO = %q, want empty for direct case", f.ParentNWO) + } + }) + } +} + +// TestLiveDirectReachDeps_Coverage proves the synthesis function: +// - emits nothing when every live dep is already covered by ExistingDeps +// - emits nothing when every live dep is covered by the live-moved sweep +// (existing dep at the same key, different SHA) +// - emits one entry per uncovered live dep, deduped by (NWO, ref, SHA) +func TestLiveDirectReachDeps_Coverage(t *testing.T) { + tests := []struct { + name string + existing []dep.Dependency + live []dep.Dependency + wantCount int + }{ + { + name: "unpinned: all live deps need a fresh check", + existing: nil, + live: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV4}, + {NWO: "actions/setup-go", Ref: "v5", SHA: testShaSetupGoV5}, + }, + wantCount: 2, + }, + { + name: "existing locked SHA covers reach key — skipped", + existing: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV4}, + }, + live: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV4}, + }, + wantCount: 0, + }, + { + name: "live-moved: existing at same dep key but different SHA — skipped (live-moved sweep handles it)", + existing: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV3}, + }, + live: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV4}, + }, + wantCount: 0, + }, + { + name: "partial coverage: existing covers one, live-extra needs check", + existing: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV4}, + }, + live: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV4}, + {NWO: "actions/setup-go", Ref: "v5", SHA: testShaSetupGoV5}, + }, + wantCount: 1, + }, + { + name: "dedups by reach key", + existing: nil, + live: []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV4}, + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV4}, + }, + wantCount: 1, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + pw := checks.ParsedWorkflow{Path: "wf", ExistingDeps: tc.existing} + got := liveDirectReachDeps(pw, tc.live) + if len(got) != tc.wantCount { + t.Fatalf("got %d deps, want %d: %+v", len(got), tc.wantCount, got) + } + }) + } +} + +// TestCollectLiveDirectReachDeps_UnionDedup proves the cmd-level pre-warm +// helper unions per-workflow results and dedupes across workflows. +func TestCollectLiveDirectReachDeps_UnionDedup(t *testing.T) { + parsed := []checks.ParsedWorkflow{ + {Path: "a.yml", ExistingDeps: nil}, + {Path: "b.yml", ExistingDeps: nil}, + } + live := []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV4}, + {NWO: "actions/setup-go", Ref: "v5", SHA: testShaSetupGoV5}, + } + got := CollectLiveDirectReachDeps(parsed, live) + if len(got) != 2 { + t.Fatalf("got %d deps, want 2: %+v", len(got), got) + } + + // With one workflow already pinning checkout, only setup-go remains. + parsed[0].ExistingDeps = []dep.Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: testShaCheckoutV4}, + } + got = CollectLiveDirectReachDeps(parsed, live) + if len(got) != 1 || got[0].NWO != "actions/setup-go" { + t.Fatalf("got %+v, want only setup-go", got) + } +} diff --git a/internal/pipeline/parse.go b/internal/pipeline/parse.go new file mode 100644 index 00000000..16cd5648 --- /dev/null +++ b/internal/pipeline/parse.go @@ -0,0 +1,105 @@ +package pipeline + +import ( + "context" + + parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/ghapi" + "github.com/github/gh-actions-pin/internal/lockfile" + "github.com/github/gh-actions-pin/internal/pinpool" + "github.com/github/gh-actions-pin/internal/pipeline/checks" + "github.com/github/gh-actions-pin/internal/resolve" + "github.com/github/gh-actions-pin/internal/workflowfile" +) + +// Diagnose scans workflows and produces findings for each. +// +// It is a backward-compatible wrapper around ParseAll, resolver pre-warming, +// and DiagnoseParsed. Newer callers can drive those phases directly to control +// UI progress. +func Diagnose(ctx context.Context, paths []string, r *resolve.Resolver, store *lockfile.State, pool *pinpool.Pool, onWorkflow ...func(done, total int, path string)) *checks.Report { + var onScan func(done, total int, path string) + if len(onWorkflow) > 0 { + onScan = onWorkflow[0] + } + parsed := ParseAll(paths, store, onScan) + if r != nil { + refs, deps := CollectResolvable(parsed) + if len(refs) > 0 { + _, _, _ = r.ResolveAllRecursive(ctx, refs) + } + if len(deps) > 0 { + _ = r.CheckReachabilityAll(ctx, deps) + } + } + return DiagnoseParsed(ctx, parsed, r, store, pool) +} + +// ParseAll loads and parses every workflow path, returning a slice in input +// order. onScan, if non-nil, fires with 1-based progress before each workflow +// is parsed so the UI can render [i/N] without leaking resolver detail. +func ParseAll(paths []string, store *lockfile.State, onScan func(done, total int, path string)) []checks.ParsedWorkflow { + total := len(paths) + out := make([]checks.ParsedWorkflow, 0, total) + for i, path := range paths { + if onScan != nil { + onScan(i+1, total, path) + } + pw := checks.ParsedWorkflow{Path: path} + wf, err := workflowfile.Load(path) + if err != nil { + pw.LoadErr = err + out = append(out, pw) + continue + } + pw.Refs, _, pw.ParseWarnings = wf.ExtractActionRefs() + if len(pw.Refs) > 0 { + wfKey := workflowfile.KeyFromPath(path) + deps, depsErr := store.Get(wfKey) + if depsErr != nil { + pw.DepsErr = depsErr + } else { + pw.ExistingDeps = deps + } + } + out = append(out, pw) + } + return out +} + +// CollectResolvable returns the deduplicated union of refs and existing deps +// across all parsed workflows. Use the returned slices to pre-warm the +// resolver caches once before per-workflow diagnostics. +func CollectResolvable(parsed []checks.ParsedWorkflow) ([]parserlock.ActionRef, []dep.Dependency) { + seenRef := make(map[ghapi.ActionRef]bool) + var refs []parserlock.ActionRef + for _, pw := range parsed { + for _, ref := range pw.Refs { + key := ghapi.ForActionRef(ref.Owner, ref.Repo, ref.Path, ref.Ref) + if seenRef[key] { + continue + } + seenRef[key] = true + refs = append(refs, ref) + } + } + seenDep := make(map[string]bool) + var deps []dep.Dependency + for _, pw := range parsed { + for _, dep := range pw.ExistingDeps { + key := dep.Key() + if seenDep[key] { + continue + } + seenDep[key] = true + deps = append(deps, dep) + } + } + return refs, deps +} + +// DiagnoseParsed runs the engine diagnostics for each pre-parsed workflow. +// Assumes the resolver caches have already been warmed (calls into the +// resolver will hit cache and stay silent). Returns a checks.Report aggregating per- +// workflow findings in input order. diff --git a/internal/pipeline/reach_findings.go b/internal/pipeline/reach_findings.go new file mode 100644 index 00000000..42f3a746 --- /dev/null +++ b/internal/pipeline/reach_findings.go @@ -0,0 +1,171 @@ +package pipeline + +import ( + "fmt" + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/ghapi" + "github.com/github/gh-actions-pin/internal/pipeline/checks" + "github.com/github/gh-actions-pin/internal/resolve" +) + +// reachabilityComplementFindings covers the cases the engine doesn't: +// - Impostor for transitive (composite-expanded) deps the engine never +// visits because they aren't in workflow uses. +// - Reachability-Unknown warnings for all deps (engine fails open on +// Unknown). Direct + transitive both get a warning so the user knows +// the check was inconclusive. +func reachabilityComplementFindings( + path string, + reach []resolve.ReachabilityResult, + deps []dep.Dependency, + directNWOs map[ghapi.Repo]bool, + parentMap map[string][]string, + existing []checks.Finding, +) []checks.Finding { + if len(reach) == 0 { + return nil + } + + forgeryKeys := map[string]bool{} + for _, f := range existing { + if f.Category == checks.LockfileForgery && f.Dependency != nil { + forgeryKeys[f.Dependency.Key()] = true + } + } + + depByKey := make(map[string]dep.Dependency, len(deps)) + for _, d := range deps { + depByKey[d.Key()] = d + } + + var out []checks.Finding + for _, rr := range reach { + dep, ok := depByKey[rr.DepKey] + if !ok { + continue + } + depCopy := dep + owner, repo := dep.OwnerRepo() + direct := directNWOs[ghapi.ForRepo(owner, repo)] + parent := "" + if parents := parentMap[rr.DepKey]; len(parents) > 0 { + parent = parents[0] + } + switch rr.Status { + case resolve.Unreachable: + if direct { + continue // engine emits impostor for direct uses + } + if forgeryKeys[rr.DepKey] { + continue + } + // High: branch_commits returned an authoritative + // "unreachable" for this transitive pin. + out = append(out, checks.Finding{ + WorkflowPath: path, + Category: checks.ImpostorCommit, + Severity: checks.SeverityError, + Confidence: checks.ConfidenceHigh, + Dependency: &depCopy, + ParentNWO: parent, + Detail: rr.Detail, + Remediation: "investigate immediately — the lockfile entry may have been injected", + DocURL: DocURLFor(checks.ImpostorCommit), + }) + case resolve.ReachabilityUnknown: + remediation := "transitive dependency pinned to a bare SHA — reachability cannot be verified" + if direct { + remediation = "reachability check inconclusive — retry when network/API is available" + } + // Low: we couldn't get a reachability answer at all. + out = append(out, checks.Finding{ + WorkflowPath: path, + Category: checks.ReachabilityUnknown, + Severity: checks.SeverityWarning, + Confidence: checks.ConfidenceLow, + Dependency: &depCopy, + ParentNWO: parent, + Detail: rr.Detail, + Remediation: remediation, + }) + } + } + return out +} + +// liveReachImpostorFindings emits checks.ImpostorCommit for live-resolved +// SHAs that come back Unreachable from the live-direct sweep. Operates on +// synthetic live deps (not pw.ExistingDeps), so it fires for unpinned and +// transitive-not-in-lockfile cases that reachabilityComplementFindings +// (keyed on existing deps) can't see. +// +// Suppresses duplicates against any prior impostor/forgery finding for the +// same dep key — the engine's checkImpostorCommit may have already emitted +// for a direct ref via the live-ref-vs-locked compare in check_misleading. +func liveReachImpostorFindings( + path string, + reach []resolve.ReachabilityResult, + live []dep.Dependency, + directNWOs map[ghapi.Repo]bool, + parentMap map[string][]string, + existing []checks.Finding, +) []checks.Finding { + if len(reach) == 0 { + return nil + } + covered := map[string]bool{} + for _, f := range existing { + if f.Dependency == nil { + continue + } + switch f.Category { + case checks.ImpostorCommit, checks.LockfileForgery: + covered[f.Dependency.Key()] = true + } + } + liveByReachKey := make(map[ghapi.Reach]dep.Dependency, len(live)) + for _, d := range live { + owner, repo := d.OwnerRepo() + liveByReachKey[ghapi.ForReach(owner, repo, d.SHA, d.Ref)] = d + } + var out []checks.Finding + for _, rr := range reach { + if rr.Status != resolve.Unreachable { + continue + } + dep, ok := liveByReachKey[ghapi.ForReach(rr.Owner, rr.Repo, rr.SHA, rr.Ref)] + if !ok { + continue + } + if covered[dep.Key()] { + continue + } + depCopy := dep + owner, repo := dep.OwnerRepo() + direct := directNWOs[ghapi.ForRepo(owner, repo)] + parent := "" + if !direct { + if parents := parentMap[dep.Key()]; len(parents) > 0 { + parent = parents[0] + } + } + detail := rr.Detail + if detail == "" { + detail = fmt.Sprintf("live resolve of %s/%s@%s → %s is not reachable from any branch", owner, repo, dep.Ref, dep.SHA) + } + out = append(out, checks.Finding{ + WorkflowPath: path, + Category: checks.ImpostorCommit, + Severity: checks.SeverityError, + Confidence: checks.ConfidenceHigh, + Dependency: &depCopy, + ParentNWO: parent, + Detail: detail, + Remediation: "investigate immediately — the live ref resolves to a commit that is not reachable from any branch", + DocURL: DocURLFor(checks.ImpostorCommit), + }) + // Mark covered so a second reach result for the same dep doesn't double-emit. + covered[dep.Key()] = true + } + return out +} diff --git a/internal/pipeline/reach_findings_test.go b/internal/pipeline/reach_findings_test.go new file mode 100644 index 00000000..bd219b77 --- /dev/null +++ b/internal/pipeline/reach_findings_test.go @@ -0,0 +1,129 @@ +package pipeline + +import ( + "sort" + "testing" + + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/ghapi" + "github.com/github/gh-actions-pin/internal/pipeline/checks" + "github.com/github/gh-actions-pin/internal/resolve" +) + +func reachResult(d dep.Dependency, status resolve.ReachabilityStatus, detail string) resolve.ReachabilityResult { + owner, repo := d.OwnerRepo() + return resolve.ReachabilityResult{ + Owner: owner, + Repo: repo, + Ref: d.Ref, + SHA: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + DepKey: d.Key(), + Status: status, + Detail: detail, + } +} + +func reachCategories(fs []checks.Finding) []checks.Category { + out := make([]checks.Category, 0, len(fs)) + for _, f := range fs { + out = append(out, f.Category) + } + sort.Slice(out, func(i, j int) bool { return out[i] < out[j] }) + return out +} + +// TestReachabilityComplementFindings locks the division of labor between the +// check engine (checkImpostorCommit) and the pipeline-level complement sweep. +// The engine owns the authoritative direct-Unreachable -> ImpostorCommit +// emission and stays SILENT on Unknown; the complement path must therefore +// (a) suppress direct Unreachable to avoid double-emitting impostor, and +// (b) own the Unknown warning for every dep so a rate-limit hiccup is still +// surfaced as inconclusive rather than swallowed. +func TestReachabilityComplementFindings(t *testing.T) { + d := dep.Dependency{NWO: "actions/checkout", Ref: "v4"} + directNWOs := map[ghapi.Repo]bool{ghapi.ForRepo("actions", "checkout"): true} + transitiveNWOs := map[ghapi.Repo]bool{} + + cases := []struct { + name string + direct bool + status resolve.ReachabilityStatus + forgery bool + want []checks.Category + }{ + { + name: "direct unknown fails open to a warning", + direct: true, status: resolve.ReachabilityUnknown, + want: []checks.Category{checks.ReachabilityUnknown}, + }, + { + name: "direct unreachable is silent (engine owns impostor)", + direct: true, status: resolve.Unreachable, + want: nil, + }, + { + name: "direct reachable emits nothing", + direct: true, status: resolve.Reachable, + want: nil, + }, + { + name: "transitive unreachable emits impostor", + direct: false, status: resolve.Unreachable, + want: []checks.Category{checks.ImpostorCommit}, + }, + { + name: "transitive unreachable under forgery is suppressed", + direct: false, status: resolve.Unreachable, forgery: true, + want: nil, + }, + { + name: "transitive unknown fails open to a warning", + direct: false, status: resolve.ReachabilityUnknown, + want: []checks.Category{checks.ReachabilityUnknown}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + nwos := transitiveNWOs + if tc.direct { + nwos = directNWOs + } + var existing []checks.Finding + if tc.forgery { + dc := d + existing = []checks.Finding{{ + Category: checks.LockfileForgery, + Dependency: &dc, + }} + } + + reach := []resolve.ReachabilityResult{reachResult(d, tc.status, "compare status: diverged")} + got := reachabilityComplementFindings( + ".github/workflows/ci.yml", + reach, + []dep.Dependency{d}, + nwos, + nil, + existing, + ) + + gotCats := reachCategories(got) + if len(gotCats) != len(tc.want) { + t.Fatalf("categories = %v, want %v", gotCats, tc.want) + } + for i := range gotCats { + if gotCats[i] != tc.want[i] { + t.Fatalf("categories = %v, want %v", gotCats, tc.want) + } + } + + // Every emitted finding must carry a confidence (schema invariant). + for _, f := range got { + if f.Confidence == "" { + t.Errorf("finding %s missing confidence", f.Category) + } + } + }) + } +} diff --git a/internal/pipeline/reach_partition.go b/internal/pipeline/reach_partition.go new file mode 100644 index 00000000..5dc2d5b0 --- /dev/null +++ b/internal/pipeline/reach_partition.go @@ -0,0 +1,270 @@ +package pipeline + +import ( + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/ghapi" + "github.com/github/gh-actions-pin/internal/pipeline/checks" + "github.com/github/gh-actions-pin/internal/resolve" + "strings" +) + +// CollectReachDeps returns the deduplicated union of existing deps across the +// given parsed workflows that will need a fresh reachability network check +// once diagnostics runs. It mirrors the per-workflow partition diagnose +// performs internally (see partitionReachByLive) but operates over the union, +// so callers can pre-warm CheckReachabilityAll once across every unresolved +// workflow instead of paying the per-workflow repo-warmup + per-dep +// concurrency cost serially. Pass live as the result of a single +// ResolveAllRecursive over the union of refs (the resolver cache makes the +// per-workflow re-lookups inside diagnose free). +func CollectReachDeps(parsed []checks.ParsedWorkflow, live []dep.Dependency) []dep.Dependency { + if len(parsed) == 0 { + return nil + } + liveSHA := make(map[string]string, len(live)) + for _, d := range live { + liveSHA[d.Key()] = d.SHA + } + seen := make(map[string]bool) + var out []dep.Dependency + for _, pw := range parsed { + if !pw.SkipReachWhenUnchanged { + // When unchanged-skip isn't active (e.g. --rescan), the per- + // workflow path will check every existing dep. Mirror that so + // pre-warm sees the full set. + for _, d := range pw.ExistingDeps { + if seen[d.Key()] { + continue + } + seen[d.Key()] = true + out = append(out, d) + } + continue + } + for _, d := range pw.ExistingDeps { + sha, ok := liveSHA[d.Key()] + if ok && strings.EqualFold(sha, d.SHA) { + continue + } + if seen[d.Key()] { + continue + } + seen[d.Key()] = true + out = append(out, d) + } + } + return out +} + +// CollectLiveMovedReachDeps returns the deduplicated set of synthetic +// dependencies (NWO, Ref + LIVE SHA) for which a reachability check +// should be pre-warmed. Each entry pairs an existing lockfile dep with +// the LIVE SHA it currently resolves to, when they differ — the input +// that lets the engine emit checks.ImpostorCommit for the +// tag-hijacked-to-fork-network shape. Pass live as the result of a +// single ResolveAllRecursive over the union of refs. +func CollectLiveMovedReachDeps(parsed []checks.ParsedWorkflow, live []dep.Dependency) []dep.Dependency { + if len(parsed) == 0 || len(live) == 0 { + return nil + } + liveSHA := make(map[string]string, len(live)) + liveDep := make(map[string]dep.Dependency, len(live)) + for _, d := range live { + liveSHA[d.Key()] = d.SHA + liveDep[d.Key()] = d + } + seen := make(map[ghapi.Reach]bool) + var out []dep.Dependency + for _, pw := range parsed { + for _, d := range pw.ExistingDeps { + ls, ok := liveSHA[d.Key()] + if !ok || strings.EqualFold(ls, d.SHA) { + continue + } + synthetic := d + synthetic.SHA = ls + // Prefer the live dep's NWO casing if the live resolve has + // one — it's the canonical one returned by the API. + if ld, ok := liveDep[d.Key()]; ok && ld.NWO != "" { + synthetic.NWO = ld.NWO + } + owner, repo := synthetic.OwnerRepo() + k := ghapi.ForReach(owner, repo, synthetic.SHA, synthetic.Ref) + if seen[k] { + continue + } + seen[k] = true + out = append(out, synthetic) + } + } + return out +} + +// liveDirectReachDeps returns live-resolved deps whose (NWO, Ref, SHA) +// isn't already covered by the locked-SHA sweep (partitionReachByLive) or +// the tag-moved sweep (liveMovedDeps), so the engine can give them a +// fresh reachability check before pinning. Covers two pin-time impostor +// shapes that the existing diagnose paths miss: +// +// - NotPinned workflow: no ExistingDep at all, so the locked-SHA sweep +// never runs. Without this, applyPin's reach loop is the only thing +// catching these — diagnose now fires the checks.ImpostorCommit +// finding pre-pin so the auto-fix runs via tryAutoFixImpostors. +// - Transitive composite dep that ResolveAllRecursive discovered but +// isn't yet in the lockfile. The locked-SHA sweep can't see it; the +// live-moved sweep only fires when an ExistingDep exists for the same +// dep key with a different SHA. +// +// Dedup by ghapi.Reach across direct + transitive entries. +func liveDirectReachDeps(pw checks.ParsedWorkflow, live []dep.Dependency) []dep.Dependency { + if len(live) == 0 { + return nil + } + covered := make(map[ghapi.Reach]bool, len(pw.ExistingDeps)+len(live)) + existingByDepKey := make(map[string]dep.Dependency, len(pw.ExistingDeps)) + for _, d := range pw.ExistingDeps { + owner, repo := d.OwnerRepo() + covered[ghapi.ForReach(owner, repo, d.SHA, d.Ref)] = true + existingByDepKey[d.Key()] = d + } + for _, d := range live { + ed, ok := existingByDepKey[d.Key()] + if !ok || strings.EqualFold(ed.SHA, d.SHA) { + continue + } + owner, repo := d.OwnerRepo() + covered[ghapi.ForReach(owner, repo, d.SHA, d.Ref)] = true + } + seen := make(map[ghapi.Reach]bool, len(live)) + var out []dep.Dependency + for _, d := range live { + owner, repo := d.OwnerRepo() + k := ghapi.ForReach(owner, repo, d.SHA, d.Ref) + if covered[k] || seen[k] { + continue + } + seen[k] = true + out = append(out, d) + } + return out +} + +// CollectLiveDirectReachDeps is the cmd-level pre-warm analogue of +// liveDirectReachDeps. Returns the deduplicated set of synthetic live +// deps across all parsed workflows that need a fresh reachability check +// because they're outside both the locked-SHA and live-moved sweeps. On +// a fully steady-state lockfile this is empty; on a brand-new repo (no +// lockfile yet) it's the full live set. +func CollectLiveDirectReachDeps(parsed []checks.ParsedWorkflow, live []dep.Dependency) []dep.Dependency { + if len(parsed) == 0 || len(live) == 0 { + return nil + } + covered := make(map[ghapi.Reach]bool) + existingByDepKey := make(map[string]dep.Dependency) + for _, pw := range parsed { + for _, d := range pw.ExistingDeps { + owner, repo := d.OwnerRepo() + covered[ghapi.ForReach(owner, repo, d.SHA, d.Ref)] = true + existingByDepKey[d.Key()] = d + } + } + for _, d := range live { + ed, ok := existingByDepKey[d.Key()] + if !ok || strings.EqualFold(ed.SHA, d.SHA) { + continue + } + owner, repo := d.OwnerRepo() + covered[ghapi.ForReach(owner, repo, d.SHA, d.Ref)] = true + } + seen := make(map[ghapi.Reach]bool, len(live)) + var out []dep.Dependency + for _, d := range live { + owner, repo := d.OwnerRepo() + k := ghapi.ForReach(owner, repo, d.SHA, d.Ref) + if covered[k] || seen[k] { + continue + } + seen[k] = true + out = append(out, d) + } + return out +} + +// liveMovedDeps is the per-workflow analogue of CollectLiveMovedReachDeps. +// Returns synthetic (NWO, Ref, LIVE SHA) deps for any existing dep whose +// live resolve differs from the recorded SHA. +func liveMovedDeps(existing, live []dep.Dependency) []dep.Dependency { + if len(existing) == 0 || len(live) == 0 { + return nil + } + liveSHA := make(map[string]string, len(live)) + liveDep := make(map[string]dep.Dependency, len(live)) + for _, d := range live { + liveSHA[d.Key()] = d.SHA + liveDep[d.Key()] = d + } + seen := make(map[ghapi.Reach]bool) + var out []dep.Dependency + for _, d := range existing { + ls, ok := liveSHA[d.Key()] + if !ok || strings.EqualFold(ls, d.SHA) { + continue + } + synthetic := d + synthetic.SHA = ls + if ld, ok := liveDep[d.Key()]; ok && ld.NWO != "" { + synthetic.NWO = ld.NWO + } + owner, repo := synthetic.OwnerRepo() + k := ghapi.ForReach(owner, repo, synthetic.SHA, synthetic.Ref) + if seen[k] { + continue + } + seen[k] = true + out = append(out, synthetic) + } + return out +} + +// partitionReachByLive splits existing deps into the set that needs a fresh +// reachability network check and the set that can be synthesized as +// Reachable because the freshly-resolved live deps confirm the recorded +// (NWO, Ref, SHA) is still what the ref resolves to right now. +// +// When skipUnchanged is false, every existing dep goes to toCheck. This +// is the --rescan path: re-verify every recorded pin against current +// upstream branches. +func partitionReachByLive(existing, live []dep.Dependency, skipUnchanged bool) (toCheck []dep.Dependency, trusted []resolve.ReachabilityResult) { + if !skipUnchanged || len(live) == 0 { + return existing, nil + } + liveSHA := make(map[string]string, len(live)) + for _, d := range live { + liveSHA[d.Key()] = d.SHA + } + for _, d := range existing { + sha, ok := liveSHA[d.Key()] + if !ok || !strings.EqualFold(sha, d.SHA) { + toCheck = append(toCheck, d) + continue + } + owner, repo := d.OwnerRepo() + trusted = append(trusted, resolve.ReachabilityResult{ + Owner: owner, + Repo: repo, + Ref: d.Ref, + SHA: d.SHA, + DepKey: d.Key(), + Status: resolve.Reachable, + Detail: "lockfile entry unchanged and live resolve confirms SHA — prior reachability verification retained", + }) + } + return toCheck, trusted +} + +// reachabilityComplementFindings covers the cases the engine doesn't: +// - Impostor for transitive (composite-expanded) deps the engine never +// visits because they aren't in workflow uses. +// - Reachability-Unknown warnings for all deps (engine fails open on +// Unknown). Direct + transitive both get a warning so the user knows +// the check was inconclusive. diff --git a/internal/pipeline/resolver_test.go b/internal/pipeline/resolver_test.go new file mode 100644 index 00000000..304e4f14 --- /dev/null +++ b/internal/pipeline/resolver_test.go @@ -0,0 +1,102 @@ +package pipeline + +import ( + "testing" + + "github.com/github/gh-actions-pin/internal/pipeline/checks" + + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/resolve" +) + +// TestPrewarmedResolver_LockedAndLiveCoexist verifies that locked-SHA +// and observed-SHA reach results for the same NWO@Ref both survive in +// the prewarmedResolver cache (the cache key includes the SHA). +func TestPrewarmedResolver_LockedAndLiveCoexist(t *testing.T) { + const ( + owner = "owner" + repo = "repo" + ref = "tampered" + locked = "ea53476fdc172d8552df5af9658a45a367e4f41d" + live = "7b403c9ec14b00000000000000000000deadbeef" + ) + locks := []resolve.ReachabilityResult{ + {Owner: owner, Repo: repo, Ref: ref, SHA: locked, Status: resolve.Reachable}, + } + lives := []resolve.ReachabilityResult{ + {Owner: owner, Repo: repo, Ref: ref, SHA: live, Status: resolve.Unreachable}, + } + pw := checks.NewPrewarmedResolver(nil, nil, locks, lives) + if got := pw.CheckReachability(owner, repo, locked, ref); got != resolve.Reachable { + t.Errorf("locked SHA: got %v, want Reachable", got) + } + if got := pw.CheckReachability(owner, repo, live, ref); got != resolve.Unreachable { + t.Errorf("observed SHA: got %v, want Unreachable", got) + } +} + +func TestCollectLiveMovedReachDeps(t *testing.T) { + mkDep := func(nwo, ref, sha string) dep.Dependency { + return dep.Dependency{NWO: nwo, Ref: ref, SHA: sha} + } + existing := []dep.Dependency{ + mkDep("owner/repo", "v4", "aaaa000000000000000000000000000000000000"), // moved → in output + mkDep("owner/repo", "v3", "bbbb000000000000000000000000000000000000"), // unchanged → skipped + mkDep("owner/repo", "v5", "cccc000000000000000000000000000000000000"), // no live entry → skipped + mkDep("owner/repo", "main", "dddd000000000000000000000000000000000000"), // moved → in output + mkDep("owner/repo", "main", "dddd000000000000000000000000000000000000"), // dup → dedup'd + } + live := []dep.Dependency{ + mkDep("owner/repo", "v4", "1111000000000000000000000000000000000000"), + mkDep("owner/repo", "v3", "bbbb000000000000000000000000000000000000"), + mkDep("owner/repo", "main", "2222000000000000000000000000000000000000"), + } + parsed := []checks.ParsedWorkflow{{Path: ".github/workflows/a.yml", ExistingDeps: existing}} + got := CollectLiveMovedReachDeps(parsed, live) + + if len(got) != 2 { + t.Fatalf("got %d synthetic deps, want 2: %+v", len(got), got) + } + wantSHAs := map[string]bool{ + "1111000000000000000000000000000000000000": false, + "2222000000000000000000000000000000000000": false, + } + for _, d := range got { + if d.Ref == "" || d.SHA == "" { + t.Errorf("synthetic dep missing fields: %#v", d) + } + if _, ok := wantSHAs[d.SHA]; !ok { + t.Errorf("unexpected SHA in output: %s", d.SHA) + continue + } + wantSHAs[d.SHA] = true + } + for sha, seen := range wantSHAs { + if !seen { + t.Errorf("expected live SHA %s in output, missing", sha) + } + } +} + +// TestLiveMovedDeps mirrors TestCollectLiveMovedReachDeps for the +// per-workflow path used inside diagnoseOneParsed. +func TestLiveMovedDeps(t *testing.T) { + mkDep := func(nwo, ref, sha string) dep.Dependency { + return dep.Dependency{NWO: nwo, Ref: ref, SHA: sha} + } + existing := []dep.Dependency{ + mkDep("owner/repo", "v4", "aaaa000000000000000000000000000000000000"), + mkDep("owner/repo", "v3", "bbbb000000000000000000000000000000000000"), + } + live := []dep.Dependency{ + mkDep("owner/repo", "v4", "1111000000000000000000000000000000000000"), + mkDep("owner/repo", "v3", "bbbb000000000000000000000000000000000000"), + } + got := liveMovedDeps(existing, live) + if len(got) != 1 { + t.Fatalf("got %d synthetic deps, want 1: %+v", len(got), got) + } + if got[0].Ref != "v4" || got[0].SHA != "1111000000000000000000000000000000000000" { + t.Errorf("unexpected synthetic dep: %#v", got[0]) + } +} diff --git a/internal/pipeline/run.go b/internal/pipeline/run.go new file mode 100644 index 00000000..8f8762d9 --- /dev/null +++ b/internal/pipeline/run.go @@ -0,0 +1,188 @@ +package pipeline + +import ( + "context" + + "github.com/github/gh-actions-pin/internal/dep" + "github.com/github/gh-actions-pin/internal/lockfile" + "github.com/github/gh-actions-pin/internal/pinpool" + "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" +) + +// RunOptions configures the Run pipeline. +type RunOptions struct { + WorkflowPaths []string + Resolver *resolve.Resolver + Tagger *tag.Lister + Store *lockfile.State + Pool *pinpool.Pool + Rescan bool // re-verify all pins end-to-end + + // OnScan fires with 1-based progress before each workflow is parsed. + OnScan func(done, total int, path string) + // OnProgress fires at each pipeline phase boundary. + OnProgress func(phase string) + // Resolver UX hooks — set these for interactive spinner mode. + OnResolveProgress func(done, total int) + OnVerifyProgress func(done, total int) + // Profile receives phase timing when profiling is enabled. + Profile *profile.Session +} + +// RunResult bundles the pipeline output. +type RunResult struct { + Report *checks.Report + Valid bool + SkippedRescan int +} + +// Run executes the full diagnostic pipeline: parse → trust-check → +// resolve → reachability pre-warm → diagnose → enrich impostors. +func Run(ctx context.Context, opts RunOptions) (*RunResult, error) { + progress := opts.OnProgress + if progress == nil { + progress = func(string) {} + } + r := opts.Resolver + prof := opts.Profile + + // Phase 1: Parse. + endParse := prof.Phase(" parse workflows") + parsed := ParseAll(opts.WorkflowPaths, opts.Store, opts.OnScan) + endParse() + + // Fast path: trust fully-recorded workflows. + skippedRescan := 0 + if !opts.Rescan { + for i := range parsed { + if isFullyRecorded(parsed[i]) { + parsed[i].Resolved = true + skippedRescan++ + } else { + parsed[i].SkipReachWhenUnchanged = true + } + } + } + + // Collect unresolved workflows for network work. + var unresolved []checks.ParsedWorkflow + for _, pw := range parsed { + if !pw.Resolved { + unresolved = append(unresolved, pw) + } + } + refs, deps := CollectResolvable(unresolved) + + // Phase 2: Resolve. + if r == nil { + // No resolver means no network resolution or reachability. + // Diagnose will still flag structural issues (not-pinned, etc.). + } else { + // Wire resolver progress hooks. + if opts.OnResolveProgress != nil { + r.OnResolveProgress = opts.OnResolveProgress + } + if opts.OnVerifyProgress != nil { + r.OnVerifyProgress = opts.OnVerifyProgress + } + + if len(refs) > 0 || (opts.Rescan && len(deps) > 0) { + progress("Resolving actions") + } + if len(refs) > 0 { + endResolve := prof.Phase(" resolve refs") + // First call warms the resolver's cache; results are consumed + // indirectly by the reachability phase and diagnose via cache + // lookups. The live deps are re-fetched from cache below. + _, _, _ = r.ResolveAllRecursive(ctx, refs) + endResolve() + } + + // Phase 3: Pre-warm reachability across all unresolved workflows. + var reachDeps, liveMoved, liveDirect []dep.Dependency + if opts.Rescan { + reachDeps = deps + if len(unresolved) > 0 { + live, _, _ := r.ResolveAllRecursive(ctx, refs) + liveMoved = CollectLiveMovedReachDeps(unresolved, live) + liveDirect = CollectLiveDirectReachDeps(unresolved, live) + } + } else { + live, _, _ := r.ResolveAllRecursive(ctx, refs) + reachDeps = CollectReachDeps(unresolved, live) + liveMoved = CollectLiveMovedReachDeps(unresolved, live) + liveDirect = CollectLiveDirectReachDeps(unresolved, live) + } + if len(reachDeps) > 0 || len(liveMoved) > 0 || len(liveDirect) > 0 { + progress("Verifying reachability") + endReach := prof.Phase(" reachability pre-warm") + if len(reachDeps) > 0 { + _ = r.CheckReachabilityAll(ctx, reachDeps) + } + if len(liveMoved) > 0 { + _ = r.CheckReachabilityAll(ctx, liveMoved) + } + if len(liveDirect) > 0 { + _ = r.CheckReachabilityAll(ctx, liveDirect) + } + endReach() + } + + // Quiet resolver hooks before diagnostics (cache-only, no progress). + r.OnResolveProgress = nil + r.OnVerifyProgress = nil + } + + // Phase 4: Diagnose. + progress("Analyzing") + endDiag := prof.Phase(" diagnose (parallel)") + report := DiagnoseParsed(ctx, parsed, r, opts.Store, opts.Pool) + endDiag() + valid := report.IsValid() + + // Phase 5: Enrich impostor findings with recommended release suggestions. + if opts.Tagger != nil && hasImpostorFindings(report) { + checks.EnrichImpostorFindings(ctx, report, opts.Tagger, r, opts.Pool) + } + + return &RunResult{ + Report: report, + Valid: valid, + SkippedRescan: skippedRescan, + }, nil +} + +// isFullyRecorded returns true when every direct ref in the workflow has a +// matching lockfile entry — the steady-state happy path. +func isFullyRecorded(pw checks.ParsedWorkflow) bool { + if pw.LoadErr != nil || pw.DepsErr != nil { + return false + } + if len(pw.Refs) == 0 { + return true + } + haveDep := make(map[string]bool, len(pw.ExistingDeps)) + for _, d := range pw.ExistingDeps { + haveDep[d.NWO+"@"+d.Ref] = true + } + for _, r := range pw.Refs { + if !haveDep[r.Owner+"/"+r.Repo+"@"+r.Ref] { + return false + } + } + return true +} + +func hasImpostorFindings(r *checks.Report) bool { + for _, wr := range r.Workflows { + for _, f := range wr.Findings { + if f.Category == checks.ImpostorCommit { + return true + } + } + } + return false +} diff --git a/internal/profile/profile.go b/internal/profile/profile.go new file mode 100644 index 00000000..6001eefb --- /dev/null +++ b/internal/profile/profile.go @@ -0,0 +1,327 @@ +// Package profile captures phase timing, CPU profiles, and HTTP +// round-trip logs for performance analysis. +package profile + +import ( + "fmt" + "io" + "net/http" + "os" + "runtime/pprof" + "runtime/trace" + "sort" + "strings" + "sync" + "sync/atomic" + "time" +) + +// Session holds all profiling state for one CLI invocation. +// Create with Start, tear down with Stop (writes summaries). +type Session struct { + traceFile *os.File + cpuFile *os.File + w io.Writer // summary output (stderr) + spans []span + mu sync.Mutex + httpLog *HTTPLog + startTime time.Time +} + +type span struct { + name string + start time.Time + duration time.Duration +} + +// Options configures what profiling to enable. +type Options struct { + TracePath string // runtime/trace output file (viewable with `go tool trace`) + CPUProfilePath string // pprof CPU profile output file + HTTPLog bool // log every HTTP round-trip + Output io.Writer +} + +// Start begins profiling. Returns a no-op session if nothing is enabled. +func Start(opts Options) (*Session, error) { + s := &Session{ + w: opts.Output, + startTime: time.Now(), + } + if s.w == nil { + s.w = os.Stderr + } + + if opts.TracePath != "" { + f, err := os.Create(opts.TracePath) + if err != nil { + return nil, fmt.Errorf("creating trace file: %w", err) + } + s.traceFile = f + if err := trace.Start(f); err != nil { + f.Close() + return nil, fmt.Errorf("starting trace: %w", err) + } + } + + if opts.CPUProfilePath != "" { + f, err := os.Create(opts.CPUProfilePath) + if err != nil { + s.cleanupTrace() + return nil, fmt.Errorf("creating cpu profile: %w", err) + } + s.cpuFile = f + if err := pprof.StartCPUProfile(f); err != nil { + f.Close() + s.cleanupTrace() + return nil, fmt.Errorf("starting cpu profile: %w", err) + } + } + + if opts.HTTPLog { + s.httpLog = &HTTPLog{} + } + + return s, nil +} + +// Phase records wall-clock timing for a named phase. Call the returned +// function when the phase ends. +func (s *Session) Phase(name string) func() { + if s == nil { + return func() {} + } + start := time.Now() + return func() { + dur := time.Since(start) + s.mu.Lock() + s.spans = append(s.spans, span{name: name, start: start, duration: dur}) + s.mu.Unlock() + } +} + +// WrapTransport wraps an http.RoundTripper with request logging. +// Returns the original transport if HTTP logging is disabled. +func (s *Session) WrapTransport(rt http.RoundTripper) http.RoundTripper { + if s == nil || s.httpLog == nil { + return rt + } + return &loggingTransport{inner: rt, log: s.httpLog} +} + +// cleanupTrace stops and closes the trace file on error paths during Start. +func (s *Session) cleanupTrace() { + if s.traceFile != nil { + trace.Stop() + s.traceFile.Close() + s.traceFile = nil + } +} + +// Enabled reports whether any profiling is active. +func (s *Session) Enabled() bool { + return s != nil && (s.traceFile != nil || s.cpuFile != nil || s.httpLog != nil) +} + +// Stop ends profiling, writes summary, closes files. +func (s *Session) Stop() { + if s == nil { + return + } + var cpuPath, tracePath string + if s.cpuFile != nil { + pprof.StopCPUProfile() + cpuPath = s.cpuFile.Name() + s.cpuFile.Close() + fmt.Fprintf(s.w, " cpu profile: %s\n", cpuPath) + } + if s.traceFile != nil { + trace.Stop() + tracePath = s.traceFile.Name() + s.traceFile.Close() + s.traceFile = nil + fmt.Fprintf(s.w, " trace: %s\n", tracePath) + } + + if len(s.spans) > 0 || (s.httpLog != nil && s.httpLog.Total() > 0) { + fmt.Fprintf(s.w, "\n── profile summary (%s total) ──\n", time.Since(s.startTime).Round(time.Millisecond)) + } + + if len(s.spans) > 0 { + fmt.Fprintf(s.w, "\nPhases:\n") + for _, sp := range s.spans { + fmt.Fprintf(s.w, " %-40s %s\n", sp.name, sp.duration.Round(time.Millisecond)) + } + } + + if s.httpLog != nil && s.httpLog.Total() > 0 { + s.httpLog.WriteSummary(s.w) + } + + // Print ready-to-paste commands for interactive visualization. + if cpuPath != "" || tracePath != "" { + fmt.Fprintf(s.w, "\nVisualize:\n") + if cpuPath != "" { + fmt.Fprintf(s.w, " go tool pprof -http=:8080 %s\n", cpuPath) + } + if tracePath != "" { + fmt.Fprintf(s.w, " go tool trace %s\n", tracePath) + } + } +} + +// ── HTTP logging ── + +// HTTPLog collects per-request timing data. +type HTTPLog struct { + mu sync.Mutex + entries []httpEntry + total atomic.Int64 +} + +type httpEntry struct { + method string + path string + status int + duration time.Duration + ts time.Time +} + +// Total returns the number of HTTP round trips recorded. +func (h *HTTPLog) Total() int64 { return h.total.Load() } + +func (h *HTTPLog) record(e httpEntry) { + h.total.Add(1) + h.mu.Lock() + h.entries = append(h.entries, e) + h.mu.Unlock() +} + +// WriteSummary prints aggregated HTTP stats. +func (h *HTTPLog) WriteSummary(w io.Writer) { + h.mu.Lock() + entries := make([]httpEntry, len(h.entries)) + copy(entries, h.entries) + h.mu.Unlock() + + if len(entries) == 0 { + return + } + + // Aggregate by path pattern. + type bucket struct { + pattern string + count int + total time.Duration + min time.Duration + max time.Duration + errors int + } + byPattern := map[string]*bucket{} + var totalDuration time.Duration + var totalErrors int + + for _, e := range entries { + pat := classifyPath(e.method, e.path) + b, ok := byPattern[pat] + if !ok { + b = &bucket{pattern: pat, min: e.duration} + byPattern[pat] = b + } + b.count++ + b.total += e.duration + if e.duration < b.min { + b.min = e.duration + } + if e.duration > b.max { + b.max = e.duration + } + if e.status >= 400 { + b.errors++ + totalErrors++ + } + totalDuration += e.duration + } + + sorted := make([]*bucket, 0, len(byPattern)) + for _, b := range byPattern { + sorted = append(sorted, b) + } + sort.Slice(sorted, func(i, j int) bool { return sorted[i].total > sorted[j].total }) + + fmt.Fprintf(w, "\nHTTP requests: %d total, %d errors, %s cumulative\n\n", + len(entries), totalErrors, totalDuration.Round(time.Millisecond)) + fmt.Fprintf(w, " %-50s %5s %8s %8s %8s %8s\n", "ENDPOINT", "COUNT", "TOTAL", "AVG", "MIN", "MAX") + fmt.Fprintf(w, " %s\n", strings.Repeat("─", 100)) + for _, b := range sorted { + avg := b.total / time.Duration(b.count) + errSuffix := "" + if b.errors > 0 { + errSuffix = fmt.Sprintf(" (%d err)", b.errors) + } + fmt.Fprintf(w, " %-50s %5d %8s %8s %8s %8s%s\n", + b.pattern, b.count, + b.total.Round(time.Millisecond), + avg.Round(time.Millisecond), + b.min.Round(time.Millisecond), + b.max.Round(time.Millisecond), + errSuffix) + } +} + +// classifyPath normalizes API paths into patterns for aggregation. +func classifyPath(method, path string) string { + parts := strings.Split(strings.TrimPrefix(path, "/"), "/") + if len(parts) < 3 { + return method + " " + path + } + // repos/{owner}/{repo}/... → repos/*/compare, repos/*/branches, etc. + if parts[0] == "repos" && len(parts) >= 4 { + nwo := parts[1] + "/" + parts[2] + // Collapse SHA-like segments. + normalized := make([]string, 0, len(parts)-3) + for _, p := range parts[3:] { + if len(p) >= 40 && isHex(p) { + normalized = append(normalized, "{sha}") + } else { + normalized = append(normalized, p) + } + } + return fmt.Sprintf("%s repos/%s/%s", method, nwo, strings.Join(normalized, "/")) + } + return method + " " + path +} + +func isHex(s string) bool { + for _, c := range s { + if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F')) { + return false + } + } + return true +} + +// loggingTransport wraps an http.RoundTripper to log every request. +type loggingTransport struct { + inner http.RoundTripper + log *HTTPLog +} + +func (lt *loggingTransport) RoundTrip(req *http.Request) (*http.Response, error) { + start := time.Now() + resp, err := lt.inner.RoundTrip(req) + dur := time.Since(start) + + status := 0 + if resp != nil { + status = resp.StatusCode + } + lt.log.record(httpEntry{ + method: req.Method, + path: req.URL.Path, + status: status, + duration: dur, + ts: start, + }) + return resp, err +} diff --git a/internal/ui/ui.go b/internal/ui/ui.go index 8554980e..71928e48 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -4,10 +4,14 @@ package ui import ( + "encoding/json" "fmt" "io" "os" + "strings" + "sync" "time" + "unicode/utf8" "github.com/briandowns/spinner" "github.com/muesli/termenv" @@ -24,11 +28,205 @@ const ( ) // UI writes human-readable output to an io.Writer with optional ANSI styling. +// +// When a log sink is attached via SetLog, all narration (Success, Detail, +// Warning, …) is redirected as plain text to the log and the terminal is left +// to spinners, prompts, and the Term* summary methods. This keeps the +// interactive output clean and avoids spinner/scrollback interleaving. +// +// Headless mode (set when stderr isn't a TTY or CI=true) takes precedence +// over the log sink: every narration call writes one plain `text\n` line to +// the writer with no icons, no indentation, and no color. Progress methods +// emit a single line per phase boundary (label stem change) and otherwise +// no-op — no spinner, no [N/M] churn. type UI struct { - w io.Writer - output *termenv.Output - noColor bool - spinner *spinner.Spinner + w io.Writer + logw io.Writer + output *termenv.Output + noColor bool + isTTY bool + headless bool + spinner *spinner.Spinner + + // headlessLabelStem is the last printed phase label stem (everything + // before the first '[' in an UpdateLabel call). Used in headless mode to + // deduplicate `Resolving actions [1/42]` … `[42/42]` into one line. + headlessLabelStem string + + // progLabel and progDetail hold the two halves of the active spinner line + // (the per-workflow label and the resolver's current-action detail). They + // are recombined and truncated to one terminal row on every update so the + // spinner never wraps — a wrapped spinner breaks the library's + // backspace-based erase and causes line jumping/leftover fragments. + progLabel string + progDetail string + + // progLast holds the most recent non-empty rendered line. If an update + // transiently leaves both label and detail empty (e.g. detail cleared + // between phases before the next label is set), we keep showing progLast + // so the spinner never flashes a bare, label-less glyph. + progLast string + + // progPaused is set while the spinner is temporarily halted (e.g. to let an + // interactive prompt own the terminal). The spinner object is retained so + // ResumeProgress can restart it with the same label/detail. + progPaused bool + + // progHasDetail tracks whether the last renderProgress call rendered a + // second detail line. clearSpinnerLines uses this to know whether to also + // erase line 2 after stopping the spinner. + progHasDetail bool + + // spinWriter is a thin io.Writer wrapper set while a spinner is active. + // It intercepts each spinner tick write (which starts with '\r') and + // appends the detail line below the spinner WITHOUT putting the detail text + // in the spinner Suffix — keeping the suffix short so the library's + // byte-count wrap detection never triggers on the second line's content. + spinWriter *spinnerWriter +} + +// SetLog attaches a narration sink. Once set, narration methods write plain +// text to w instead of the terminal. Pass nil to detach. +func (u *UI) SetLog(w io.Writer) { + u.logw = w +} + +// MarkHeadless forces the UI into plain-text streaming mode after +// construction. Used when a flag like --no-interactive signals headless +// intent that the auto-detection (TTY + CI env) at construction time +// couldn't see. Idempotent and one-way: once headless, the UI stays +// headless for the rest of its lifetime. Also disables color so any +// already-cached output profile is consistent with the new mode. +func (u *UI) MarkHeadless() { + if u.headless { + return + } + u.headless = true + u.noColor = true + u.output = termenv.NewOutput(u.w, termenv.WithProfile(termenv.Ascii)) +} + +// progressTrace caches the result of GH_ACTIONS_PIN_DEBUG_PROGRESS once. When +// set, every UpdateLabel and SetWorkerStatus call writes a timestamped JSONL +// line to a dedicated trace file so we can audit phase transitions and verify +// the worker pool is actually fanning out without depending on visual +// inspection of the spinner. The path is resolved from the env var: "1" or +// "true" maps to $TMPDIR/gh-actions-pin-progress.log, anything else is treated +// as a literal path. +var ( + progressTraceMu sync.Mutex + progressTraceFile *os.File + progressTracePath = resolveProgressTracePath() +) + +func resolveProgressTracePath() string { + v := os.Getenv("GH_ACTIONS_PIN_DEBUG_PROGRESS") + switch v { + case "": + return "" + case "1", "true", "yes": + dir := os.TempDir() + return dir + "/gh-actions-pin-progress.log" + default: + return v + } +} + +// CloseProgressTrace closes the progress trace file if one was opened. It is +// safe to call unconditionally; it no-ops when tracing is off or already closed. +func CloseProgressTrace() { + progressTraceMu.Lock() + defer progressTraceMu.Unlock() + if progressTraceFile != nil { + progressTraceFile.Close() + progressTraceFile = nil + } +} + +// traceProgress emits a structured trace event to the progress trace file when +// progress tracing is enabled. kind is "label" or "slot[N]"; payload is the new +// value. No-op when tracing is off. +func (u *UI) traceProgress(kind, payload string) { + if progressTracePath == "" { + return + } + progressTraceMu.Lock() + defer progressTraceMu.Unlock() + if progressTraceFile == nil { + f, err := os.OpenFile(progressTracePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644) + if err != nil { + return + } + progressTraceFile = f + fmt.Fprintf(f, "# gh-actions-pin progress trace %s\n", time.Now().Format(time.RFC3339)) + } + rec := struct { + Time string `json:"time"` + Kind string `json:"kind"` + Payload string `json:"payload"` + }{ + Time: time.Now().Format(time.RFC3339Nano), + Kind: kind, + Payload: payload, + } + b, err := json.Marshal(rec) + if err != nil { + return + } + fmt.Fprintln(progressTraceFile, string(b)) +} + +// logging reports whether a narration log sink is attached. +func (u *UI) logging() bool { + return u.logw != nil +} + +// emit writes a fully-formatted narration line. With a log sink attached the +// line goes there (already plain, since color helpers no-op while logging); +// otherwise it goes to the terminal, pausing any active spinner. +func (u *UI) emit(line string) { + if u.logw != nil { + fmt.Fprint(u.logw, line) + return + } + u.printLine(func() { fmt.Fprint(u.w, line) }) +} + +// logTagged writes one structured JSON record to the log sink (JSONL: one +// object per line). level classifies the line (success, error, warning, skip, +// info, detail, hint, header); an empty level falls back to "detail" so +// continuation lines stay machine-readable. The message is plain text — color +// and hyperlink helpers no-op while a log sink is attached. +func (u *UI) logTagged(level, text string) { + if u.logw == nil { + return + } + if level == "" { + level = "detail" + } + rec := struct { + Time string `json:"time"` + Level string `json:"level"` + Msg string `json:"msg"` + }{ + Time: time.Now().Format(time.RFC3339Nano), + Level: level, + Msg: text, + } + b, err := json.Marshal(rec) + if err != nil { + return + } + fmt.Fprintf(u.logw, "%s\n", b) +} + +// paint applies an ANSI foreground color regardless of the log sink. Used by +// the Term* summary methods, which always target the terminal. +func (u *UI) paint(colorCode, s string) string { + if u.noColor { + return s + } + return u.output.String(s).Foreground(u.output.Color(colorCode)).String() } // New creates a UI that writes to stderr with color auto-detected from the @@ -36,7 +234,8 @@ type UI struct { func New() *UI { isTTY := term.IsTerminal(int(os.Stderr.Fd())) noColor := isColorDisabled() - colorEnabled := isTTY && !noColor + headless := isHeadless(isTTY) + colorEnabled := isTTY && !noColor && !headless profile := termenv.Ascii if colorEnabled { @@ -44,9 +243,11 @@ func New() *UI { } return &UI{ - w: os.Stderr, - output: termenv.NewOutput(os.Stderr, termenv.WithProfile(profile)), - noColor: !colorEnabled, + w: os.Stderr, + output: termenv.NewOutput(os.Stderr, termenv.WithProfile(profile)), + noColor: !colorEnabled, + isTTY: isTTY, + headless: headless, } } @@ -59,7 +260,8 @@ func NewWithWriter(w io.Writer) *UI { if f, ok := w.(*os.File); ok { isTTY = term.IsTerminal(int(f.Fd())) } - colorEnabled := isTTY && !noColor + headless := isHeadless(isTTY) + colorEnabled := isTTY && !noColor && !headless profile := termenv.Ascii if colorEnabled { @@ -67,19 +269,24 @@ func NewWithWriter(w io.Writer) *UI { } return &UI{ - w: w, - output: termenv.NewOutput(w, termenv.WithProfile(profile)), - noColor: !colorEnabled, + w: w, + output: termenv.NewOutput(w, termenv.WithProfile(profile)), + noColor: !colorEnabled, + isTTY: isTTY, + headless: headless, } } // NewPlain creates a UI with no color and writes to the given writer. -// Useful for tests. +// Useful for tests. The result is in headless mode: narration writes plain +// `text\n` lines to w with no icons or color, progress methods don't spawn +// a spinner, and Blank/TermBlank are no-ops. func NewPlain(w io.Writer) *UI { return &UI{ - w: w, - output: termenv.NewOutput(w, termenv.WithProfile(termenv.Ascii)), - noColor: true, + w: w, + output: termenv.NewOutput(w, termenv.WithProfile(termenv.Ascii)), + noColor: true, + headless: true, } } @@ -93,65 +300,596 @@ func isColorDisabled() bool { return false } +// isHeadless reports whether the UI should run in plain-text streaming mode: +// any non-TTY writer, or any environment where CI is set (most providers set +// CI=true; GitHub Actions and others honor this convention). Headless mode +// suppresses spinners and ANSI styling so CI logs stay greppable. +func isHeadless(isTTY bool) bool { + if !isTTY { + return true + } + if v := os.Getenv("CI"); v != "" && v != "0" && v != "false" { + return true + } + return false +} + +// headlessEmit writes one plain-text line (no icons, no color, no log +// routing) to the UI writer. Used by every narration and Term* method when +// headless mode is on so CI logs are flat, greppable, and stdout-safe. Any +// trailing newlines in text are normalized so callers can pass either pre- +// terminated strings (e.g. from Detail's " msg\n" pattern) or bare text. +func (u *UI) headlessEmit(text string) { + text = strings.TrimRight(text, "\n") + fmt.Fprintln(u.w, text) +} + +// spinnerWriter wraps the terminal writer used by the spinner. It intercepts +// each write from the spinner goroutine (every write starts with '\r') and, +// when worker status lines are set, appends them as dim lines below the spinner +// after the spinner has written its own content. Worker text is kept entirely +// out of the spinner Suffix so the library's byte-count wrap detection never +// sees the extra lines — eliminating the runaway multi-line erase bug. +type spinnerWriter struct { + mu sync.Mutex + w io.Writer + workers []string // per-slot status; empty string = idle + hints []string // per-slot dim suffix appended after the status + nRendered int // number of worker lines written in the last tick + noColor bool + output *termenv.Output + // stop closes to signal the independent worker-redraw ticker to exit. + // done is closed once the ticker goroutine has returned. The ticker + // keeps worker glyphs animating even when the spinner library coalesces, + // throttles, or briefly stalls its own writes (e.g. under network + // contention) — without the ticker, animation freezes whenever Write + // isn't called. + stop chan struct{} + done chan struct{} + + // deferredWrites buffers setWorkerStatus calls that happen while + // printLine has snapshotted-and-cleared the workers slice for an + // inline message print. When non-nil, setWorkerStatus writes into + // this map instead of workers; printLine merges the buffered writes + // back onto its snapshot on restore so concurrent clears/updates + // from the pin pool aren't clobbered by the restore. Nil during + // normal operation. + deferredWrites map[int]string + // deferredHints mirrors deferredWrites for hint state so concurrent + // stall-watcher updates aren't clobbered by printLine's restore. + deferredHints map[int]string +} + +// workerSpinFrames is the rotating glyph shown next to each ACTIVE worker row +// (rows starting with "→") so subtasks visibly pulse instead of looking +// frozen. Matches the main spinner's braille charset (CharSets[11]) so the +// motion stays cohesive. +var workerSpinFrames = []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} + +// workerFrameInterval is how often (wall-clock) each worker glyph advances one +// step. Matches the main spinner's 120ms tick so motion feels cohesive. +const workerFrameInterval = 120 * time.Millisecond + +func (sw *spinnerWriter) Write(p []byte) (n int, err error) { + sw.mu.Lock() + defer sw.mu.Unlock() + + n, err = sw.w.Write(p) + if err != nil || len(p) == 0 || p[0] != '\r' { + return + } + sw.renderWorkersLocked() + return +} + +// renderWorkersLocked redraws the worker rows below the spinner line, leaving +// the cursor back on the spinner line. Caller must hold sw.mu and the cursor +// must currently be on the spinner line. Glyph frames are picked from the wall +// clock so animation continues even when triggered by the independent ticker +// instead of by a spinner Write. +func (sw *spinnerWriter) renderWorkersLocked() { + step := int(time.Now().UnixNano() / int64(workerFrameInterval)) + // Per-slot phase offset so rows visibly cascade instead of all hitting + // the same frame in lockstep — that lockstep was what made them look + // frozen even when they weren't. + width := termWidthOf(sw.w) + var lines []string + for slot, w := range sw.workers { + if w == "" { + continue + } + body := w + if len(w) >= len("→ ") && w[:len("→ ")] == "→ " { + frame := workerSpinFrames[(step+slot)%len(workerSpinFrames)] + body = frame + " " + w[len("→ "):] + } + hint := "" + if slot < len(sw.hints) { + hint = sw.hints[slot] + } + // Combined width budget: " " indent + body + " " + hint must + // fit one terminal row. Hint loses first if there isn't room. + if width > 4 { + budget := width - 2 // " " indent + if len(body)+1+len(hint) > budget { + if len(body) >= budget { + body = truncateBytes(body, budget) + hint = "" + } else if hint != "" { + hintBudget := budget - len(body) - 1 + if hintBudget < 4 { + hint = "" + } else { + hint = truncateBytes(hint, hintBudget) + } + } + } + } + var line string + if !sw.noColor { + line = sw.output.String(" " + body).Faint().String() + if hint != "" { + line += " " + sw.output.String(hint).Faint().String() + } + } else { + line = " " + body + if hint != "" { + line += " " + hint + } + } + lines = append(lines, line) + } + nLines := len(lines) + for _, line := range lines { + // Use \n (newline) rather than \033[1B (cursor-down) so the + // buffer scrolls when we're at the bottom of the viewport. ESC[1B + // is a no-op at the last row and the subsequent ESC[NA cursor-up + // would then overshoot, landing on (and clobbering) lines above + // the spinner — including the user's typed command line. + fmt.Fprintf(sw.w, "\n\r\033[2K%s", line) + } + // Erase stale lines left over from a previous render that had more + // active workers. Without this, ghost "→ dep" rows from finished + // workers persist below the current set. + for i := nLines; i < sw.nRendered; i++ { + fmt.Fprintf(sw.w, "\n\r\033[2K") + } + totalDown := nLines + if sw.nRendered > nLines { + totalDown = sw.nRendered + } + if totalDown > 0 { + fmt.Fprintf(sw.w, "\033[%dA\r", totalDown) + } + sw.nRendered = nLines +} + +// startAnimator launches a goroutine that periodically redraws the worker +// rows so their glyphs keep pulsing even when the spinner library's own +// writes stall or coalesce. Caller MUST NOT hold sw.mu. +func (sw *spinnerWriter) startAnimator() { + sw.mu.Lock() + if sw.stop != nil { + sw.mu.Unlock() + return + } + sw.stop = make(chan struct{}) + sw.done = make(chan struct{}) + stop := sw.stop + done := sw.done + sw.mu.Unlock() + + go func() { + defer close(done) + t := time.NewTicker(workerFrameInterval) + defer t.Stop() + for { + select { + case <-stop: + return + case <-t.C: + sw.mu.Lock() + hasActive := false + for _, w := range sw.workers { + if len(w) >= len("→ ") && w[:len("→ ")] == "→ " { + hasActive = true + break + } + } + if hasActive { + sw.renderWorkersLocked() + } + sw.mu.Unlock() + } + } + }() +} + +// stopAnimator signals the redraw ticker to exit and waits for it. Caller +// MUST NOT hold sw.mu. +func (sw *spinnerWriter) stopAnimator() { + sw.mu.Lock() + stop := sw.stop + done := sw.done + sw.stop = nil + sw.done = nil + sw.mu.Unlock() + if stop == nil { + return + } + close(stop) + <-done +} + +// setDetail is a backward-compat shim that sets a single worker slot (slot 0). +func (sw *spinnerWriter) setDetail(det string) { + sw.setWorkerStatus(0, det) +} + +// setWorkerStatus sets or clears one worker's status slot. While printLine +// has the workers slice snapshotted (deferredWrites != nil) the write is +// buffered into the deferred map so printLine's restore phase can merge it +// onto the snapshot — preventing the restore from clobbering clears and +// updates issued by the pin pool during the print window. Setting a slot to +// any value also clears that slot's hint so a stale "(still working…)" +// suffix from a previous job can't bleed into the next one. +func (sw *spinnerWriter) setWorkerStatus(slot int, status string) { + sw.mu.Lock() + if sw.deferredWrites != nil { + sw.deferredWrites[slot] = status + if sw.deferredHints != nil { + sw.deferredHints[slot] = "" + } + sw.mu.Unlock() + return + } + for len(sw.workers) <= slot { + sw.workers = append(sw.workers, "") + } + sw.workers[slot] = status + if slot < len(sw.hints) { + sw.hints[slot] = "" + } + sw.mu.Unlock() +} + +// setWorkerHint sets or clears one worker's dim suffix without touching the +// status text. Defers like setWorkerStatus while printLine has the slices +// snapshotted. +func (sw *spinnerWriter) setWorkerHint(slot int, hint string) { + sw.mu.Lock() + defer sw.mu.Unlock() + if sw.deferredHints != nil { + sw.deferredHints[slot] = hint + return + } + for len(sw.hints) <= slot { + sw.hints = append(sw.hints, "") + } + sw.hints[slot] = hint +} + +// clearSpinnerLines erases the spinner line and any worker lines that were +// rendered below it on the last tick. Uses \033[J (erase to end of screen) +// rather than the per-line cursor-down/clear/cursor-up dance the previous +// implementation used: ESC[1B doesn't scroll the buffer at the bottom of the +// viewport, so the followup ESC[NA could overshoot into already-rendered +// rows above and clobber them on the next spinner tick. +func (u *UI) clearSpinnerLines() { + if u.spinWriter != nil { + u.spinWriter.mu.Lock() + u.spinWriter.nRendered = 0 + u.spinWriter.mu.Unlock() + } + fmt.Fprint(u.w, "\r\033[J") + u.progHasDetail = false +} + +// printLine writes one line of output, transparently pausing an active +// spinner so its animation frame doesn't interleave with (and corrupt) the +// text. The spinner resumes after the write. When no spinner is active it +// just runs write. +func (u *UI) printLine(write func()) { + if u.spinner != nil && u.spinner.Active() { + // Snapshot and zero all worker slots so the stop-write doesn't render + // extra lines that we'd then fail to clear. Buffer any concurrent + // setWorkerStatus calls into deferredWrites so the restore merges + // them instead of clobbering — without the buffer, pin-pool workers + // that clear or repaint their slot during the write window lose + // those updates, which is what leaves stale "→ path" rows visible + // after their owner worker has exited. + var savedWorkers []string + var savedHints []string + if u.spinWriter != nil { + u.spinWriter.mu.Lock() + savedWorkers = make([]string, len(u.spinWriter.workers)) + copy(savedWorkers, u.spinWriter.workers) + savedHints = make([]string, len(u.spinWriter.hints)) + copy(savedHints, u.spinWriter.hints) + u.spinWriter.workers = nil + u.spinWriter.hints = nil + u.spinWriter.deferredWrites = map[int]string{} + u.spinWriter.deferredHints = map[int]string{} + u.spinWriter.mu.Unlock() + } + u.spinner.Stop() + u.clearSpinnerLines() + write() + if u.spinWriter != nil { + u.spinWriter.mu.Lock() + for slot, status := range u.spinWriter.deferredWrites { + for len(savedWorkers) <= slot { + savedWorkers = append(savedWorkers, "") + } + savedWorkers[slot] = status + // A status update implicitly clears the hint, mirroring + // setWorkerStatus's normal-path semantics. + for len(savedHints) <= slot { + savedHints = append(savedHints, "") + } + savedHints[slot] = "" + } + for slot, hint := range u.spinWriter.deferredHints { + for len(savedHints) <= slot { + savedHints = append(savedHints, "") + } + // Don't overwrite a hint clear caused by a same-window + // status update: if deferredWrites also touched this slot, + // the status reset already cleared the hint above. + if _, statusUpdated := u.spinWriter.deferredWrites[slot]; statusUpdated { + continue + } + savedHints[slot] = hint + } + u.spinWriter.workers = savedWorkers + u.spinWriter.hints = savedHints + u.spinWriter.deferredWrites = nil + u.spinWriter.deferredHints = nil + u.spinWriter.mu.Unlock() + } + u.spinner.Start() + return + } + write() +} + // Success prints a green "✓" prefixed message. func (u *UI) Success(msg string, args ...any) { text := fmt.Sprintf(msg, args...) - fmt.Fprintf(u.w, "%s %s\n", u.Green(IconSuccess), text) + if u.logging() { + u.logTagged("success", text) + return + } + if u.headless { + u.headlessEmit(text) + return + } + u.emit(fmt.Sprintf("%s %s\n", u.Green(IconSuccess), text)) } // Error prints a red "✗" prefixed message. func (u *UI) Error(msg string, args ...any) { text := fmt.Sprintf(msg, args...) - fmt.Fprintf(u.w, "%s %s\n", u.Red(IconError), text) + if u.logging() { + u.logTagged("error", text) + return + } + if u.headless { + u.headlessEmit(text) + return + } + u.emit(fmt.Sprintf("%s %s\n", u.Red(IconError), text)) } // Warning prints a yellow "!" prefixed message. func (u *UI) Warning(msg string, args ...any) { text := fmt.Sprintf(msg, args...) - fmt.Fprintf(u.w, "%s %s\n", u.Yellow(IconWarning), text) + if u.logging() { + u.logTagged("warning", text) + return + } + if u.headless { + u.headlessEmit(text) + return + } + u.emit(fmt.Sprintf("%s %s\n", u.Yellow(IconWarning), text)) } // Skip prints a gray "-" prefixed message. func (u *UI) Skip(msg string, args ...any) { text := fmt.Sprintf(msg, args...) - fmt.Fprintf(u.w, "%s %s\n", u.Dim(IconSkip), u.Dim(text)) + if u.logging() { + u.logTagged("skip", text) + return + } + if u.headless { + u.headlessEmit(text) + return + } + u.emit(fmt.Sprintf("%s %s\n", u.Dim(IconSkip), u.Dim(text))) } // Info prints a message with no prefix. func (u *UI) Info(msg string, args ...any) { - fmt.Fprintf(u.w, msg+"\n", args...) + if u.logging() { + u.logTagged("info", fmt.Sprintf(msg, args...)) + return + } + if u.headless { + u.headlessEmit(fmt.Sprintf(msg, args...)) + return + } + u.emit(fmt.Sprintf(msg+"\n", args...)) } // Infof prints a message with no prefix and no trailing newline. func (u *UI) Infof(msg string, args ...any) { - fmt.Fprintf(u.w, msg, args...) + if u.logging() { + u.logTagged("info", fmt.Sprintf(msg, args...)) + return + } + if u.headless { + u.headlessEmit(fmt.Sprintf(msg, args...)) + return + } + u.emit(fmt.Sprintf(msg, args...)) } // Header prints a bold message, used for file/section headers. func (u *UI) Header(msg string, args ...any) { text := fmt.Sprintf(msg, args...) - fmt.Fprintf(u.w, "\n%s\n", u.Bold(text)) + if u.logging() { + u.logTagged("header", text) + return + } + if u.headless { + u.headlessEmit(text) + return + } + u.emit(fmt.Sprintf("\n%s\n", u.Bold(text))) } // Hint prints a dim, indented message — typically a suggested command. func (u *UI) Hint(msg string, args ...any) { text := fmt.Sprintf(msg, args...) - fmt.Fprintf(u.w, " %s\n", u.Dim(text)) + if u.logging() { + u.logTagged("hint", text) + return + } + if u.headless { + u.headlessEmit(text) + return + } + u.emit(fmt.Sprintf(" %s\n", u.Dim(text))) } // Detail prints an indented detail line (2-space indent). func (u *UI) Detail(msg string, args ...any) { - fmt.Fprintf(u.w, " "+msg+"\n", args...) + if u.logging() { + u.logTagged("", fmt.Sprintf(msg, args...)) + return + } + if u.headless { + u.headlessEmit(fmt.Sprintf(msg, args...)) + return + } + u.emit(fmt.Sprintf(" "+msg+"\n", args...)) } -// Blank prints an empty line. +// Blank prints an empty line. In log mode it is a no-op so the JSONL transcript +// stays one valid object per line. In headless mode it is also a no-op so CI +// logs stay flat — phase boundaries do the visual separating instead. func (u *UI) Blank() { + if u.headless || u.logging() { + return + } + u.emit("\n") +} + +// TermSuccess prints a green "✓" summary line directly to the terminal, +// bypassing the narration log. Use for the final run summary. +func (u *UI) TermSuccess(msg string, args ...any) { + if u.headless { + u.headlessEmit(fmt.Sprintf(msg, args...)) + return + } + fmt.Fprintf(u.w, "%s %s\n", u.paint("2", IconSuccess), fmt.Sprintf(msg, args...)) +} + +// TermError prints a red "✗" summary line directly to the terminal. +func (u *UI) TermError(msg string, args ...any) { + if u.headless { + u.headlessEmit(fmt.Sprintf(msg, args...)) + return + } + fmt.Fprintf(u.w, "%s %s\n", u.paint("1", IconError), fmt.Sprintf(msg, args...)) +} + +// TermWarn prints a yellow "!" summary line directly to the terminal. +func (u *UI) TermWarn(msg string, args ...any) { + if u.headless { + u.headlessEmit(fmt.Sprintf(msg, args...)) + return + } + fmt.Fprintf(u.w, "%s %s\n", u.paint("3", IconWarning), fmt.Sprintf(msg, args...)) +} + +// TermCaution prints a red "!" summary line directly to the terminal. Use for +// non-fatal but attention-worthy signals (e.g. a commit pinned only after a +// full-branch-scan fallback) that warrant red without the "✗ failure" framing. +func (u *UI) TermCaution(msg string, args ...any) { + if u.headless { + u.headlessEmit(fmt.Sprintf(msg, args...)) + return + } + fmt.Fprintf(u.w, "%s %s\n", u.paint("1", IconWarning), fmt.Sprintf(msg, args...)) +} + +// TermDetail prints an indented summary detail line directly to the terminal. +func (u *UI) TermDetail(msg string, args ...any) { + if u.headless { + u.headlessEmit(fmt.Sprintf(msg, args...)) + return + } + fmt.Fprintf(u.w, " "+msg+"\n", args...) +} + +// TermNeutral prints a dimmed, neutral "-" summary line directly to the +// terminal. Per cli/cli iconography, "-" denotes neutral/informational +// status (not success, alert, or failure). Used for footer pointers such +// as the resolution-record path. +func (u *UI) TermNeutral(msg string, args ...any) { + text := fmt.Sprintf(msg, args...) + if u.headless { + u.headlessEmit(text) + return + } + fmt.Fprintf(u.w, "%s %s\n", u.TermDim(IconSkip), u.TermDim(text)) +} + +// TermYellow returns s in yellow for use in Term* output. Unlike Yellow, this +// does not suppress color when a narration log sink is attached, since Term* +// methods write directly to the terminal rather than the log. +func (u *UI) TermYellow(s string) string { + return u.paint("3", s) +} + +// TermDim returns s in dim/faint for use in Term* output. +func (u *UI) TermDim(s string) string { + if u.noColor { + return s + } + return u.output.String(s).Faint().String() +} + +// TermBold returns s in bold for use in Term* output. +func (u *UI) TermBold(s string) string { + if u.noColor { + return s + } + return u.output.String(s).Bold().String() +} + +// TermLink wraps text in an OSC 8 hyperlink for use in Term* output. Falls +// back to plain text when color is disabled or url is empty. +func (u *UI) TermLink(text, url string) string { + if u.noColor || url == "" { + return text + } + return u.output.Hyperlink(url, text) +} + +// TermBlank prints an empty line directly to the terminal. +func (u *UI) TermBlank() { + if u.headless { + return + } fmt.Fprintln(u.w) } // Bold returns s in bold if color is enabled. func (u *UI) Bold(s string) string { - if u.noColor { + if u.noColor || u.logging() { return s } return u.output.String(s).Bold().String() @@ -159,7 +897,7 @@ func (u *UI) Bold(s string) string { // Dim returns s in dim/faint if color is enabled. func (u *UI) Dim(s string) string { - if u.noColor { + if u.noColor || u.logging() { return s } return u.output.String(s).Faint().String() @@ -167,7 +905,7 @@ func (u *UI) Dim(s string) string { // Red returns s in red if color is enabled. func (u *UI) Red(s string) string { - if u.noColor { + if u.noColor || u.logging() { return s } return u.output.String(s).Foreground(u.output.Color("1")).String() @@ -175,7 +913,7 @@ func (u *UI) Red(s string) string { // Green returns s in green if color is enabled. func (u *UI) Green(s string) string { - if u.noColor { + if u.noColor || u.logging() { return s } return u.output.String(s).Foreground(u.output.Color("2")).String() @@ -183,7 +921,7 @@ func (u *UI) Green(s string) string { // Yellow returns s in yellow if color is enabled. func (u *UI) Yellow(s string) string { - if u.noColor { + if u.noColor || u.logging() { return s } return u.output.String(s).Foreground(u.output.Color("3")).String() @@ -191,7 +929,7 @@ func (u *UI) Yellow(s string) string { // Cyan returns s in cyan if color is enabled. func (u *UI) Cyan(s string) string { - if u.noColor { + if u.noColor || u.logging() { return s } return u.output.String(s).Foreground(u.output.Color("6")).String() @@ -201,46 +939,367 @@ func (u *UI) Cyan(s string) string { // supports it, otherwise returns text as-is. Most modern terminals (iTerm2, // WezTerm, kitty, GNOME Terminal, Windows Terminal) support this. func (u *UI) Hyperlink(text, url string) string { - if u.noColor { + if u.noColor || u.logging() { return text } return u.output.Hyperlink(url, text) } +// DocLink renders a documentation reference: the bare URL when writing to the +// log (so the transcript stays actionable), otherwise a dim "docs" hyperlink +// for the terminal. +func (u *UI) DocLink(url string) string { + if u.logging() { + return url + } + return u.Dim(u.Hyperlink("docs", url)) +} + // IsTTY returns true if the output is a terminal. func (u *UI) IsTTY() bool { - return !u.noColor + return u.isTTY +} + +// Headless reports whether the UI is running in plain-text streaming mode +// (non-TTY writer, or CI environment). Callers can use this to gate behavior +// that should differ between interactive and machine-consumable output — for +// example, leaving the narration log attached so per-action lines stream to +// stderr rather than being discarded. +func (u *UI) Headless() bool { + return u.headless +} + +// ProgressActive reports whether a spinner is currently running. Callers use +// this to adopt an already-running spinner (keeping it continuous across +// phases) instead of stopping and restarting one, which would leave a visible +// gap on the terminal. +func (u *UI) ProgressActive() bool { + return u.spinner != nil } // StartProgress starts an animated spinner with the given label on stderr. // On non-TTY outputs, prints a static label instead. Matches gh CLI's Primer // progress indicator: braille dots, 120ms, cyan. func (u *UI) StartProgress(label string) { - if u.noColor { + if u.headless { if label != "" { - fmt.Fprintf(u.w, "%s...\n", label) + u.headlessEmit(label) + u.headlessLabelStem = labelStem(label) } return } - sp := spinner.New(spinner.CharSets[11], 120*time.Millisecond, - spinner.WithWriter(u.w), - spinner.WithColor("fgCyan"), - ) - if label != "" { - sp.Prefix = label + " " + sw := &spinnerWriter{ + w: u.w, + noColor: u.noColor, + output: u.output, } + u.spinWriter = sw + opts := []spinner.Option{spinner.WithWriter(sw)} + if !u.noColor { + opts = append(opts, spinner.WithColor("fgCyan")) + } + sp := spinner.New(spinner.CharSets[11], 120*time.Millisecond, opts...) u.spinner = sp + u.progLabel = label + u.progDetail = "" + u.progLast = "" + u.progPaused = false + u.renderProgress() sp.Start() + sw.startAnimator() +} + +// PauseProgress temporarily halts the spinner and clears its line so other +// output (typically an interactive prompt) can render cleanly. The label and +// detail are retained; ResumeProgress restarts the spinner where it left off. +// Safe to call when no spinner is active or one is already paused. +func (u *UI) PauseProgress() { + if u.spinner == nil || u.progPaused { + return + } + if u.spinWriter != nil { + u.spinWriter.stopAnimator() + u.spinWriter.mu.Lock() + u.spinWriter.workers = nil + u.spinWriter.hints = nil + u.spinWriter.mu.Unlock() + } + u.spinner.Stop() + u.progPaused = true + if u.isTTY { + u.clearSpinnerLines() + } +} + +// ResumeProgress restarts a spinner previously paused by PauseProgress, +// redrawing the retained label/detail. Safe to call when no spinner is active +// or one is not paused. +func (u *UI) ResumeProgress() { + if u.spinner == nil || !u.progPaused { + return + } + u.progPaused = false + u.renderProgress() + u.spinner.Start() + if u.spinWriter != nil { + u.spinWriter.startAnimator() + } } // StopProgress stops the spinner. Safe to call if no spinner is active. func (u *UI) StopProgress() { if u.spinner != nil { + if u.spinWriter != nil { + u.spinWriter.stopAnimator() + u.spinWriter.mu.Lock() + u.spinWriter.workers = nil + u.spinWriter.hints = nil + u.spinWriter.mu.Unlock() + } u.spinner.Stop() + u.clearSpinnerLines() u.spinner = nil - // Clear the spinner line. - fmt.Fprintf(u.w, "\r\033[2K") + u.spinWriter = nil + u.progLabel = "" + u.progDetail = "" + u.progLast = "" + u.progPaused = false + } +} + +// UpdateProgress sets a detail string in worker slot 0 (backward-compat +// single-detail shim). No-op when no spinner is active. +func (u *UI) UpdateProgress(detail string) { + if u.spinner == nil { + return + } + u.progDetail = detail + u.renderProgress() +} + +// SetWorkerStatus sets or clears one worker slot's status line, shown as a +// subdued line below the spinner. slot indexes from 0. No-op when no spinner +// is active. +func (u *UI) SetWorkerStatus(slot int, status string) { + u.traceProgress(fmt.Sprintf("slot[%d]", slot), status) + if u.spinWriter == nil { + return + } + if !u.noColor { + width := u.termWidth() + if width > 4 { + status = truncateBytes(status, width-4) + } + } + u.spinWriter.setWorkerStatus(slot, status) +} + +// SetWorkerHint sets or clears a dim suffix appended after the worker slot's +// status text (e.g. "→ workflow.yml (still working…)"). Used by pinpool's +// stall watcher to surface that a worker has been on the same job for longer +// than the stall threshold without clobbering the slot's main status. No-op +// when no spinner is active. +func (u *UI) SetWorkerHint(slot int, hint string) { + u.traceProgress(fmt.Sprintf("hint[%d]", slot), hint) + if u.spinWriter == nil { + return + } + u.spinWriter.setWorkerHint(slot, hint) +} + +// ClearWorkerStatuses wipes every worker slot so stale "✓ NWO" rows from a +// completed phase don't carry into the next one. No-op when no spinner is +// active. +func (u *UI) ClearWorkerStatuses() { + if u.spinWriter == nil { + return + } + u.spinWriter.mu.Lock() + for i := range u.spinWriter.workers { + u.spinWriter.workers[i] = "" + } + for i := range u.spinWriter.hints { + u.spinWriter.hints[i] = "" + } + u.spinWriter.mu.Unlock() +} + +// UpdateLabel changes the spinner prefix label (e.g. to show per-workflow +// "[i/N] path" progress). No-op when no spinner is active. +func (u *UI) UpdateLabel(label string) { + u.traceProgress("label", label) + if u.headless { + stem := labelStem(label) + if stem != "" && stem != u.headlessLabelStem { + u.headlessEmit(stem) + u.headlessLabelStem = stem + } + return + } + if u.spinner == nil { + return + } + u.progLabel = label + u.renderProgress() +} + +// labelStem reduces a progress label to a phase identifier used for headless +// dedup. It strips both a trailing " [N/M]" and a leading "[N/M] " progress +// counter, then takes the leading "verb" portion (everything before the first +// digit) so labels like "Scanning 78 workflows", "Scanning [1/78] foo.yml", +// "[1/78] Pinning dependencies", and "Scanning" all collapse to the same +// stem. A label without recognizable structure is trimmed and returned as-is. +// Stripping the leading bracket form matters: without it, headlessEmit prints +// a stray "[" line for parallel-worker labels like "[1/78] Pinning workflows" +// because the first-digit scan returns the bare "[" prefix. +func labelStem(label string) string { + label = strings.TrimSpace(label) + if strings.HasPrefix(label, "[") { + if end := strings.Index(label, "]"); end > 0 { + label = strings.TrimSpace(label[end+1:]) + } + } + if i := strings.LastIndex(label, " ["); i >= 0 { + label = label[:i] + } + label = strings.TrimSpace(label) + for i, r := range label { + if r >= '0' && r <= '9' { + return strings.TrimSpace(label[:i]) + } + } + return label +} + +// renderProgress recombines the label and detail into a single line that is +// truncated to fit the terminal width (leaving room for the spinner glyph and +// a space). The spinner glyph is anchored at the left edge (column 0): the +// combined line is assigned to the spinner Suffix with an empty Prefix, so the +// library always renders "\r{glyph} {label} — {detail}". Keeping the glyph +// fixed on the left stops it from drifting as the detail text changes width. +// The whole string is truncated to one terminal row — wrapping would defeat +// the library's backspace-based erase and cause the line jumping the user +// sees. +func (u *UI) renderProgress() { + if u.spinner == nil { + return + } + + label := u.progLabel + detail := u.progDetail + + if label == "" { + if u.progLast == "" { + return + } + label = u.progLast + } else { + u.progLast = label + } + + width := u.termWidth() + if width > 8 { + budget := width - 6 + if !u.noColor { + budget -= 7 // bold escape open+close + } + label = truncateBytes(label, budget) + } + + if detail != "" { + // Worker rows render a pulsing glyph only when the text starts + // with "→ "; UpdateProgress callers (resolver progress hooks) + // pass plain strings like "resolving foo@bar". Prepend the + // arrow so the slot animates instead of looking frozen. + if !strings.HasPrefix(detail, "→ ") { + detail = "→ " + detail + } + if width > 4 { + budget := width - 4 // " " indent + faint open+close + if !u.noColor { + budget -= 7 + } + detail = truncateBytes(detail, budget) + } + } + + // Pass detail (slot 0) to the writer; it appends worker lines on every + // spinner tick without inflating the Suffix byte count. + // Only write when detail is non-empty: calling setDetail("") would + // overwrite slot 0 that the pool's worker status may have set. + if u.spinWriter != nil && detail != "" { + u.spinWriter.setDetail(detail) + } + u.progHasDetail = detail != "" + + var suffix string + if !u.noColor { + suffix = u.output.String(label).Bold().String() + } else { + suffix = label + } + + u.spinner.Prefix = "" + if suffix != "" { + u.spinner.Suffix = " " + suffix + } else { + u.spinner.Suffix = "" + } +} + +// termWidth returns the terminal column count for the spinner writer, or 0 if +// it cannot be determined (in which case callers skip truncation). +func (u *UI) termWidth() int { + return termWidthOf(u.w) +} + +// termWidthOf returns the terminal width of w, or 0 when it isn't a TTY. +func termWidthOf(w io.Writer) int { + f, ok := w.(*os.File) + if !ok { + return 0 + } + cols, _, err := term.GetSize(int(f.Fd())) + if err != nil || cols <= 0 { + return 0 + } + return cols +} + +// truncateBytes shortens s so its UTF-8 byte length is at most max, never +// splitting a multibyte rune. When truncation occurs the tail is replaced with +// a single ellipsis ("…", 3 bytes). The budget is byte-based because the +// spinner library measures wrap width in bytes; a rune/column budget lets +// multibyte characters (the "—" separator, non-ASCII paths) push the real byte +// width past the terminal edge and trigger its two-line erase. +func truncateBytes(s string, max int) string { + if max <= 0 { + return "" + } + if len(s) <= max { + return s + } + const ellipsis = "…" // 3 bytes + if max < len(ellipsis) { + return trimToRuneBoundary(s, max) + } + return trimToRuneBoundary(s, max-len(ellipsis)) + ellipsis +} + +// trimToRuneBoundary returns the longest prefix of s whose byte length is at +// most max, cut on a rune boundary so multibyte characters aren't split. +func trimToRuneBoundary(s string, max int) string { + if max <= 0 { + return "" + } + if len(s) <= max { + return s + } + end := max + for end > 0 && !utf8.RuneStart(s[end]) { + end-- } + return s[:end] } // Pluralize returns singular when n==1, plural otherwise. diff --git a/internal/ui/ui_test.go b/internal/ui/ui_test.go new file mode 100644 index 00000000..71b52a48 --- /dev/null +++ b/internal/ui/ui_test.go @@ -0,0 +1,215 @@ +package ui + +import ( + "bytes" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/muesli/termenv" +) + +func newTestSpinWriter() *spinnerWriter { + var buf bytes.Buffer + return &spinnerWriter{ + w: &buf, + noColor: true, + output: termenv.NewOutput(&buf, termenv.WithProfile(termenv.Ascii)), + } +} + +// TestSpinnerWriter_DeferredWritesBufferClearAndUpdate verifies the buffer +// path setWorkerStatus takes while printLine has the workers slice +// snapshotted. Without buffering, those writes would land in a slice that +// printLine throws away on restore. +func TestSpinnerWriter_DeferredWritesBufferClearAndUpdate(t *testing.T) { + sw := newTestSpinWriter() + sw.workers = []string{"→ a.yml", "→ b.yml"} + sw.deferredWrites = map[int]string{} + + sw.setWorkerStatus(0, "") + sw.setWorkerStatus(1, "→ c.yml") + sw.setWorkerStatus(3, "→ d.yml") + + if len(sw.workers) != 2 || sw.workers[0] != "→ a.yml" || sw.workers[1] != "→ b.yml" { + t.Errorf("workers slice mutated while deferred: %#v", sw.workers) + } + if got, want := sw.deferredWrites[0], ""; got != want { + t.Errorf("deferred[0]: got %q, want %q", got, want) + } + if got, want := sw.deferredWrites[1], "→ c.yml"; got != want { + t.Errorf("deferred[1]: got %q, want %q", got, want) + } + if got, want := sw.deferredWrites[3], "→ d.yml"; got != want { + t.Errorf("deferred[3]: got %q, want %q", got, want) + } +} + +// TestPrintLine_MergesDeferredWritesOverSnapshot exercises the full +// snapshot → buffered writes → restore-with-merge dance via the public +// printLine entry point. +func TestPrintLine_MergesDeferredWritesOverSnapshot(t *testing.T) { + u := &UI{w: &bytes.Buffer{}, noColor: true} + u.spinWriter = newTestSpinWriter() + u.spinWriter.workers = []string{"→ a.yml", "→ b.yml", "→ c.yml"} + + // Simulate the snapshot-and-clear that printLine performs when the + // spinner is active. We bypass the actual spinner because tests run + // headless; the buffered-write semantics under test live entirely in + // spinWriter, independent of the spinner library. + u.spinWriter.mu.Lock() + savedWorkers := append([]string(nil), u.spinWriter.workers...) + u.spinWriter.workers = nil + u.spinWriter.deferredWrites = map[int]string{} + u.spinWriter.mu.Unlock() + + // Concurrent pin-pool activity during the write window: slot 0 + // cleared (worker exited), slot 1 repainted (new job grabbed), slot + // 2 untouched. + u.SetWorkerStatus(0, "") + u.SetWorkerStatus(1, "→ new.yml") + + // Apply the merge-on-restore the way printLine does. + u.spinWriter.mu.Lock() + for slot, status := range u.spinWriter.deferredWrites { + for len(savedWorkers) <= slot { + savedWorkers = append(savedWorkers, "") + } + savedWorkers[slot] = status + } + u.spinWriter.workers = savedWorkers + u.spinWriter.deferredWrites = nil + u.spinWriter.mu.Unlock() + + got := u.spinWriter.workers + want := []string{"", "→ new.yml", "→ c.yml"} + if len(got) != len(want) { + t.Fatalf("workers length: got %d, want %d (%#v)", len(got), len(want), got) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("workers[%d]: got %q, want %q", i, got[i], want[i]) + } + } +} + +// TestSpinnerWriter_HintAppendedAfterStatus verifies a hint set via +// setWorkerHint renders as a suffix on the worker row, after the status text. +func TestSpinnerWriter_HintAppendedAfterStatus(t *testing.T) { + sw := newTestSpinWriter() + buf := sw.w.(*bytes.Buffer) + + sw.setWorkerStatus(0, "→ workflow.yml") + sw.setWorkerHint(0, "(still working…)") + + sw.mu.Lock() + sw.renderWorkersLocked() + sw.mu.Unlock() + + out := buf.String() + if !strings.Contains(out, "workflow.yml") { + t.Fatalf("expected workflow.yml in output; got %q", out) + } + if !strings.Contains(out, "(still working…)") { + t.Fatalf("expected hint suffix in output; got %q", out) + } + statusIdx := strings.Index(out, "workflow.yml") + hintIdx := strings.Index(out, "(still working…)") + if hintIdx < statusIdx { + t.Fatalf("hint should come after status; got hint@%d status@%d in %q", hintIdx, statusIdx, out) + } +} + +// TestSpinnerWriter_SetWorkerStatusClearsHint guards the defensive-clear +// contract: any setWorkerStatus call must wipe that slot's hint so a stale +// "(still working…)" from the previous job can't bleed into the next one. +func TestSpinnerWriter_SetWorkerStatusClearsHint(t *testing.T) { + sw := newTestSpinWriter() + sw.setWorkerStatus(0, "→ a.yml") + sw.setWorkerHint(0, "(still working…)") + sw.setWorkerStatus(0, "→ b.yml") + + sw.mu.Lock() + defer sw.mu.Unlock() + if len(sw.hints) > 0 && sw.hints[0] != "" { + t.Fatalf("hint not cleared on status update: %q", sw.hints[0]) + } +} + +// TestSpinnerWriter_HintSurvivesPrintLineDeferral mirrors the printLine +// snapshot/restore dance for hints. Without deferredHints, a hint set +// concurrently while printLine has the slices snapshotted would be lost on +// restore. +func TestSpinnerWriter_HintSurvivesPrintLineDeferral(t *testing.T) { + sw := newTestSpinWriter() + sw.workers = []string{"→ a.yml"} + sw.hints = []string{""} + + // Begin snapshot, like printLine does. + sw.mu.Lock() + savedHints := append([]string(nil), sw.hints...) + savedWorkers := append([]string(nil), sw.workers...) + sw.workers = nil + sw.hints = nil + sw.deferredWrites = map[int]string{} + sw.deferredHints = map[int]string{} + sw.mu.Unlock() + + // Concurrent stall-watcher fires hint while snapshot is active. + sw.setWorkerHint(0, "(still working…)") + + // Restore, merging deferred hints onto the snapshot. + sw.mu.Lock() + for slot, hint := range sw.deferredHints { + for len(savedHints) <= slot { + savedHints = append(savedHints, "") + } + savedHints[slot] = hint + } + sw.workers = savedWorkers + sw.hints = savedHints + sw.deferredWrites = nil + sw.deferredHints = nil + sw.mu.Unlock() + + if len(sw.hints) == 0 || sw.hints[0] != "(still working…)" { + t.Fatalf("hint lost across snapshot/restore: %v", sw.hints) + } +} + +// TestSpinnerWriter_AnimatorHeartbeat guards Thing 2's regression test: +// even when the worker status text never changes, the independent animator +// goroutine must keep redrawing the row (advancing the spinner glyph) so +// users don't perceive the spinner as frozen. +func TestSpinnerWriter_AnimatorHeartbeat(t *testing.T) { + sw := newTestSpinWriter() + cw := &countingWriter{inner: sw.w} + sw.w = cw + sw.output = termenv.NewOutput(cw, termenv.WithProfile(termenv.Ascii)) + + sw.setWorkerStatus(0, "→ blocked.yml") // active "→" row triggers animator + sw.startAnimator() + defer sw.stopAnimator() + + // Wait three frame intervals plus slack. The animator must have + // written at least twice — proving the row is still being redrawn + // even though setWorkerStatus hasn't been called again. + time.Sleep(4 * workerFrameInterval) + + if cw.writes.Load() < 2 { + t.Fatalf("animator should have redrawn ≥2 times after %s; saw %d writes", + 4*workerFrameInterval, cw.writes.Load()) + } +} + +// countingWriter wraps an io.Writer and counts Write calls. +type countingWriter struct { + inner interface{ Write(p []byte) (int, error) } + writes atomic.Int64 +} + +func (c *countingWriter) Write(p []byte) (int, error) { + c.writes.Add(1) + return c.inner.Write(p) +} diff --git a/internal/workflowfile/rewrite.go b/internal/workflowfile/rewrite.go new file mode 100644 index 00000000..ef77944d --- /dev/null +++ b/internal/workflowfile/rewrite.go @@ -0,0 +1,115 @@ +package workflowfile + +import ( + "strings" + + "gopkg.in/yaml.v3" +) + +// RewriteActionRefs rewrites targeted uses: refs in the original workflow +// content while preserving the surrounding formatting and comments. +func (f *File) RewriteActionRefs(replacements map[string]string) ([]byte, int, error) { + if len(replacements) == 0 { + return append([]byte(nil), f.Content...), 0, nil + } + + lines := strings.Split(string(f.Content), "\n") + changed := 0 + + walkYAMLNodes(&f.root, func(keyNode, valueNode *yaml.Node) { + if keyNode == nil || valueNode == nil || keyNode.Value != "uses" || valueNode.Kind != yaml.ScalarNode { + return + } + // Skip aliases / nodes that came from anchors. Replacing one + // anchor reference would silently change every other use site. + if valueNode.Alias != nil || valueNode.Anchor != "" { + return + } + + oldValue := strings.TrimSpace(valueNode.Value) + newValue, ok := replacements[oldValue] + if !ok { + // Sub-path actions: uses: owner/repo/path@ref should match + // a rewrite keyed on owner/repo@ref (NWO-level granularity). + newValue, ok = subpathRewriteLookup(oldValue, replacements) + } + if !ok || newValue == "" || newValue == oldValue { + return + } + + // Anchor the rewrite at the YAML node's reported (line, column) + // rather than scanning the line for the first occurrence of + // oldValue. The previous strings.Index(...) approach would + // happily substitute matching text inside a YAML comment that + // preceded the value. + lineIndex := valueNode.Line - 1 + colIndex := valueNode.Column - 1 + if lineIndex < 0 || lineIndex >= len(lines) || colIndex < 0 { + return + } + line := lines[lineIndex] + if colIndex+len(oldValue) > len(line) { + return + } + // Quoted scalars report Column at the opening quote; the actual + // value sits one byte further in. + if valueNode.Style == yaml.SingleQuotedStyle || valueNode.Style == yaml.DoubleQuotedStyle { + if colIndex+1+len(oldValue) > len(line) { + return + } + if line[colIndex+1:colIndex+1+len(oldValue)] != oldValue { + return + } + lines[lineIndex] = line[:colIndex+1] + newValue + stripTrailingComment(line[colIndex+1+len(oldValue):]) + changed++ + return + } + if line[colIndex:colIndex+len(oldValue)] != oldValue { + return + } + lines[lineIndex] = line[:colIndex] + newValue + stripTrailingComment(line[colIndex+len(oldValue):]) + changed++ + }) + + return []byte(strings.Join(lines, "\n")), changed, nil +} + +// subpathRewriteLookup handles sub-path actions like actions/cache/restore@ref. +func subpathRewriteLookup(usesValue string, replacements map[string]string) (string, bool) { + atIdx := strings.LastIndex(usesValue, "@") + if atIdx < 0 { + return "", false + } + fullName := usesValue[:atIdx] + ref := usesValue[atIdx+1:] + + parts := strings.SplitN(fullName, "/", 3) + if len(parts) < 3 { + return "", false // no sub-path + } + nwo := parts[0] + "/" + parts[1] + subPath := parts[2] + + nwoKey := nwo + "@" + ref + newNWOValue, ok := replacements[nwoKey] + if !ok { + return "", false + } + + newAtIdx := strings.LastIndex(newNWOValue, "@") + if newAtIdx < 0 { + return "", false + } + newRef := newNWOValue[newAtIdx+1:] + return nwo + "/" + subPath + "@" + newRef, true +} + +// stripTrailingComment removes a trailing YAML comment (# ...) from the +// remainder of a line after the uses: value has been replaced. +func stripTrailingComment(tail string) string { + idx := strings.Index(tail, "#") + if idx < 0 { + return tail + } + return strings.TrimRight(tail[:idx], " \t") +} diff --git a/internal/workflowfile/rewrite_test.go b/internal/workflowfile/rewrite_test.go new file mode 100644 index 00000000..1286eaec --- /dev/null +++ b/internal/workflowfile/rewrite_test.go @@ -0,0 +1,209 @@ +package workflowfile + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSubpathRewriteLookup(t *testing.T) { + replacements := map[string]string{ + "actions/cache@27d5ce7": "actions/cache@v5.0.5", + } + + tests := []struct { + name string + input string + want string + wantOK bool + }{ + { + name: "sub-path restore", + input: "actions/cache/restore@27d5ce7", + want: "actions/cache/restore@v5.0.5", + wantOK: true, + }, + { + name: "sub-path save", + input: "actions/cache/save@27d5ce7", + want: "actions/cache/save@v5.0.5", + wantOK: true, + }, + { + name: "no sub-path (exact match, not handled here)", + input: "actions/cache@27d5ce7", + want: "", + wantOK: false, + }, + { + name: "no match in replacements", + input: "actions/checkout/sub@abc1234", + want: "", + wantOK: false, + }, + { + name: "no @ separator", + input: "actions/cache/restore", + want: "", + wantOK: false, + }, + { + name: "deeply nested sub-path", + input: "actions/cache/restore/deep/nested@27d5ce7", + want: "actions/cache/restore/deep/nested@v5.0.5", + wantOK: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := subpathRewriteLookup(tt.input, replacements) + assert.Equal(t, tt.wantOK, ok) + if ok { + assert.Equal(t, tt.want, got) + } + }) + } +} + +func TestRewriteActionRefs(t *testing.T) { + f, err := Load("testdata/simple.yml") + require.NoError(t, err) + + output, changed, err := f.RewriteActionRefs(map[string]string{ + "actions/checkout@v4": "actions/checkout@v5", + "actions/setup-go@v5": "actions/setup-go@v6", + }) + require.NoError(t, err) + assert.Equal(t, 2, changed) + + s := string(output) + assert.Contains(t, s, "uses: actions/checkout@v5") + assert.Contains(t, s, "uses: actions/setup-go@v6") + assert.NotContains(t, s, "uses: actions/checkout@v4") + assert.NotContains(t, s, "uses: actions/setup-go@v5") +} + +func TestRewriteActionRefs_DropsTrailingComments(t *testing.T) { + content := []byte(`name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 # pinned for stability + - uses: actions/setup-go@v5 +`) + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "with_comment.yml") + require.NoError(t, os.WriteFile(path, content, 0o644)) + + f, err := Load(path) + require.NoError(t, err) + + output, changed, err := f.RewriteActionRefs(map[string]string{ + "actions/checkout@v4": "actions/checkout@v4.2.1", + }) + require.NoError(t, err) + assert.Equal(t, 1, changed) + + s := string(output) + assert.Contains(t, s, "uses: actions/checkout@v4.2.1\n") + assert.NotContains(t, s, "pinned for stability") +} + +func TestRewriteActionRefs_OnlyMatchesYAMLUses(t *testing.T) { + content := []byte(`name: ci +on: push +# DO NOT USE actions/checkout@v4 - see docs +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 +`) + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "comment_first.yml") + require.NoError(t, os.WriteFile(path, content, 0o644)) + + f, err := Load(path) + require.NoError(t, err) + + output, changed, err := f.RewriteActionRefs(map[string]string{ + "actions/checkout@v4": "actions/checkout@v4.2.1", + }) + require.NoError(t, err) + assert.Equal(t, 1, changed) + + s := string(output) + assert.Contains(t, s, "uses: actions/checkout@v4.2.1") + assert.Contains(t, s, "# DO NOT USE actions/checkout@v4 - see docs") +} + +func TestRewriteActionRefs_AnchoredAtColumn(t *testing.T) { + content := []byte("jobs:\n a:\n steps:\n # bumped from actions/checkout@v3 — do not revert\n - uses: actions/checkout@v3\n") + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "anchor.yml") + require.NoError(t, os.WriteFile(path, content, 0o644)) + + f, err := Load(path) + require.NoError(t, err) + + output, changed, err := f.RewriteActionRefs(map[string]string{ + "actions/checkout@v3": "actions/checkout@v4", + }) + require.NoError(t, err) + assert.Equal(t, 1, changed) + + s := string(output) + assert.Contains(t, s, " - uses: actions/checkout@v4\n") + assert.Contains(t, s, "# bumped from actions/checkout@v3 — do not revert") +} + +func TestRewriteActionRefs_SkipsAnchorsAndAliases(t *testing.T) { + content := []byte("jobs:\n a:\n steps:\n - uses: &pinned actions/checkout@v3\n b:\n steps:\n - uses: *pinned\n") + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "anchored.yml") + require.NoError(t, os.WriteFile(path, content, 0o644)) + + f, err := Load(path) + require.NoError(t, err) + + _, changed, err := f.RewriteActionRefs(map[string]string{ + "actions/checkout@v3": "actions/checkout@v4", + }) + require.NoError(t, err) + assert.Equal(t, 0, changed) +} + +func TestRewriteActionRefs_SubPathActions(t *testing.T) { + content := []byte(`jobs: + build: + steps: + - uses: actions/cache/restore@27d5ce7 # restore cache + - uses: actions/cache/save@27d5ce7 + - uses: actions/cache@27d5ce7 + - uses: actions/checkout@v3 +`) + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "subpath.yml") + require.NoError(t, os.WriteFile(path, content, 0o644)) + + f, err := Load(path) + require.NoError(t, err) + + output, changed, err := f.RewriteActionRefs(map[string]string{ + "actions/cache@27d5ce7": "actions/cache@v5.0.5", + }) + require.NoError(t, err) + assert.Equal(t, 3, changed) + + out := string(output) + assert.Contains(t, out, "actions/cache/restore@v5.0.5\n") + assert.Contains(t, out, "actions/cache/save@v5.0.5") + assert.Contains(t, out, "actions/cache@v5.0.5") + assert.Contains(t, out, "actions/checkout@v3") +} diff --git a/internal/workflowfile/testdata/mixed_refs.yml b/internal/workflowfile/testdata/mixed_refs.yml new file mode 100644 index 00000000..5a86a329 --- /dev/null +++ b/internal/workflowfile/testdata/mixed_refs.yml @@ -0,0 +1,15 @@ +name: mixed +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: ./local-action + - uses: docker://alpine:3.18 + - uses: actions/cache/save@v4 + - uses: ${{ matrix.action }} + - uses: owner/repo/.github/workflows/called.yml@v1 + - uses: owner/repo/.github/workflows/called.yaml@main + - uses: actions/setup-node@v4 + - uses: ./.github/workflows/reusable.yml diff --git a/internal/workflowfile/testdata/simple.yml b/internal/workflowfile/testdata/simple.yml new file mode 100644 index 00000000..fb09b850 --- /dev/null +++ b/internal/workflowfile/testdata/simple.yml @@ -0,0 +1,9 @@ +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + - run: go test ./... diff --git a/internal/workflowfile/workflowfile.go b/internal/workflowfile/workflowfile.go new file mode 100644 index 00000000..c32dd7b4 --- /dev/null +++ b/internal/workflowfile/workflowfile.go @@ -0,0 +1,276 @@ +// Package workflowfile owns the parsed workflow YAML representation: loading, +// extraction of action refs, local composite discovery, and comment-preserving +// rewriting. It intentionally has no dependency on the lockfile or resolver +// packages. +package workflowfile + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" + "gopkg.in/yaml.v3" +) + +// File is the parsed workflow YAML the CLI rewrites in-place. +// It carries the original byte content alongside the parsed node tree so +// RewriteActionRefs can do anchored, comment-preserving substitution. +type File struct { + Path string + Content []byte + root yaml.Node +} + +// Load reads and parses a workflow file. +func Load(path string) (*File, error) { + content, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("reading workflow: %w", err) + } + + return Parse(path, content) +} + +// Parse builds a File from already-loaded workflow content. +func Parse(path string, content []byte) (*File, error) { + f := &File{ + Path: path, + Content: content, + } + if err := yaml.Unmarshal(content, &f.root); err != nil { + return nil, fmt.Errorf("parsing workflow YAML: %w", err) + } + + return f, nil +} + +// ExtractActionRefs finds all uses: references to repository actions in the workflow. +func (f *File) ExtractActionRefs() ([]parserlock.ActionRef, []string, []string) { + var refs []parserlock.ActionRef + var warnings []string + var localPaths []string + seen := make(map[string]bool) + seenLocal := make(map[string]bool) + + walkYAML(&f.root, func(key, value string) { + if key != "uses" { + return + } + value = strings.TrimSpace(value) + if strings.Contains(value, "${") { + warnings = append(warnings, fmt.Sprintf("can't pin expression-based uses: %s", value)) + return + } + if strings.HasPrefix(value, "./") { + if parserlock.IsLocalReusableWorkflow(value) { + return + } + if !seenLocal[value] { + seenLocal[value] = true + localPaths = append(localPaths, value) + } + return + } + actionRef := parserlock.ParseActionRef(value) + if actionRef != nil { + dedupKey := actionRef.FullName() + "@" + actionRef.Ref + if !seen[dedupKey] { + seen[dedupKey] = true + refs = append(refs, *actionRef) + } + } + }) + + return refs, localPaths, warnings +} + +// DiscoverWorkflows finds all workflow files in .github/workflows/ relative to +// the current directory. Returns nil if the directory doesn't exist. +func DiscoverWorkflows() ([]string, error) { + dir := filepath.Join(".github", "workflows") + entries, err := os.ReadDir(dir) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("reading %s: %w", dir, err) + } + + var paths []string + for _, entry := range entries { + if entry.IsDir() { + continue + } + ext := filepath.Ext(entry.Name()) + if ext == ".yml" || ext == ".yaml" { + paths = append(paths, filepath.Join(dir, entry.Name())) + } + } + sort.Strings(paths) + return paths, nil +} + +// ExtractLocalCompositeRefs reads action.yml files from local paths relative +// to the workflow file's directory and returns any repository action refs +// found in their steps. +func ExtractLocalCompositeRefs(workflowPath string, localPaths []string) ([]parserlock.ActionRef, []string) { + var refs []parserlock.ActionRef + var warnings []string + seen := make(map[string]bool) + + repoRoot := findRepoRoot(workflowPath) + if repoRoot == "" { + if len(localPaths) > 0 { + warnings = append(warnings, "can't resolve local action paths: not in a git repository") + } + return nil, warnings + } + + for _, localPath := range localPaths { + relPath := strings.TrimPrefix(localPath, "./") + actionDir := filepath.Join(repoRoot, relPath) + if !isWithinRoot(repoRoot, actionDir) { + warnings = append(warnings, fmt.Sprintf("refusing to read action file outside repo root: %s", localPath)) + continue + } + + actionContent, err := os.ReadFile(filepath.Join(actionDir, "action.yml")) + if err != nil { + actionContent, err = os.ReadFile(filepath.Join(actionDir, "action.yaml")) + if err != nil { + warnings = append(warnings, fmt.Sprintf("can't read action file for %s: %v", localPath, err)) + continue + } + } + + uses, parseErr := parseActionYAMLForUses(actionContent) + if parseErr != nil { + warnings = append(warnings, fmt.Sprintf("can't parse action file for %s: %v", localPath, parseErr)) + continue + } + + for _, use := range uses { + actionRef := parserlock.ParseActionRef(use) + if actionRef != nil { + dedupKey := actionRef.FullName() + "@" + actionRef.Ref + if !seen[dedupKey] { + seen[dedupKey] = true + refs = append(refs, *actionRef) + } + } + } + } + + return refs, warnings +} + +func walkYAML(node *yaml.Node, fn func(key, value string)) { + walkYAMLNodes(node, func(keyNode, valueNode *yaml.Node) { + if keyNode.Kind == yaml.ScalarNode && valueNode.Kind == yaml.ScalarNode { + fn(keyNode.Value, valueNode.Value) + } + }) +} + +// maxYAMLWalkDepth bounds recursion in walkYAMLNodes so a hostile or +// pathological workflow tree cannot stack-overflow the parser. +const maxYAMLWalkDepth = 100 + +func walkYAMLNodes(node *yaml.Node, fn func(keyNode, valueNode *yaml.Node)) { + walkYAMLNodesDepth(node, fn, 0) +} + +func walkYAMLNodesDepth(node *yaml.Node, fn func(keyNode, valueNode *yaml.Node), depth int) { + if node == nil || depth > maxYAMLWalkDepth { + return + } + switch node.Kind { + case yaml.DocumentNode: + for _, child := range node.Content { + walkYAMLNodesDepth(child, fn, depth+1) + } + case yaml.MappingNode: + for i := 0; i < len(node.Content)-1; i += 2 { + key := node.Content[i] + val := node.Content[i+1] + fn(key, val) + walkYAMLNodesDepth(val, fn, depth+1) + } + case yaml.SequenceNode: + for _, child := range node.Content { + walkYAMLNodesDepth(child, fn, depth+1) + } + } +} + +func findRepoRoot(startPath string) string { + absPath, err := filepath.Abs(filepath.Dir(startPath)) + if err != nil { + return "" + } + for { + if _, err := os.Stat(filepath.Join(absPath, ".git")); err == nil { + return absPath + } + parent := filepath.Dir(absPath) + if parent == absPath { + return "" + } + absPath = parent + } +} + +func isWithinRoot(root, candidate string) bool { + absRoot, err := filepath.Abs(root) + if err != nil { + return false + } + absCandidate, err := filepath.Abs(candidate) + if err != nil { + return false + } + rel, err := filepath.Rel(absRoot, absCandidate) + if err != nil { + return false + } + if rel == "." { + return true + } + return !strings.HasPrefix(rel, ".."+string(filepath.Separator)) && rel != ".." +} + +func parseActionYAMLForUses(content []byte) ([]string, error) { + var action struct { + Runs struct { + Using string `yaml:"using"` + Steps []struct { + Uses string `yaml:"uses"` + } `yaml:"steps"` + } `yaml:"runs"` + } + if err := yaml.Unmarshal(content, &action); err != nil { + return nil, err + } + if action.Runs.Using != "composite" { + return nil, nil + } + + var uses []string + for _, step := range action.Runs.Steps { + if step.Uses != "" { + uses = append(uses, step.Uses) + } + } + return uses, nil +} + +// KeyFromPath converts a workflow path discovered on disk (relative to the +// repo root or cwd) into the repo-relative key used inside the lockfile. +func KeyFromPath(workflowPath string) string { + cleaned := filepath.ToSlash(filepath.Clean(workflowPath)) + cleaned = strings.TrimPrefix(cleaned, "./") + return cleaned +} diff --git a/internal/workflowfile/workflowfile_test.go b/internal/workflowfile/workflowfile_test.go new file mode 100644 index 00000000..95be9e79 --- /dev/null +++ b/internal/workflowfile/workflowfile_test.go @@ -0,0 +1,62 @@ +package workflowfile + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoadAndExtractActionRefs(t *testing.T) { + f, err := Load("testdata/simple.yml") + require.NoError(t, err) + + refs, localPaths, warnings := f.ExtractActionRefs() + assert.Len(t, refs, 2) + assert.Empty(t, localPaths) + assert.Empty(t, warnings) + + assert.Equal(t, "actions/checkout", refs[0].NWO()) + assert.Equal(t, "v4", refs[0].Ref) + assert.Equal(t, "actions/setup-go", refs[1].NWO()) + assert.Equal(t, "v5", refs[1].Ref) +} + +func TestExtractActionRefsMixed(t *testing.T) { + f, err := Load("testdata/mixed_refs.yml") + require.NoError(t, err) + + refs, localPaths, warnings := f.ExtractActionRefs() + assert.Len(t, refs, 3) + assert.Equal(t, "actions/checkout", refs[0].NWO()) + assert.Equal(t, "actions/cache", refs[1].NWO()) + assert.Equal(t, "save", refs[1].Path) + assert.Equal(t, "actions/setup-node", refs[2].NWO()) + + assert.Len(t, localPaths, 1) + assert.Equal(t, "./local-action", localPaths[0]) + + assert.Len(t, warnings, 1) + assert.Contains(t, warnings[0], "expression-based") +} + +func TestExtractLocalCompositeRefs_RejectsPathTraversal(t *testing.T) { + repoRoot := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(repoRoot, ".git"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".github", "workflows"), 0o755)) + workflowPath := filepath.Join(repoRoot, ".github", "workflows", "ci.yml") + require.NoError(t, os.WriteFile(workflowPath, []byte("name: ci\n"), 0o644)) + + _, warnings := ExtractLocalCompositeRefs(workflowPath, []string{"./../../etc"}) + + var sawRefusal bool + for _, w := range warnings { + if strings.Contains(w, "refusing to read action file outside repo root") { + sawRefusal = true + } + } + assert.True(t, sawRefusal, "expected refusal warning, got: %#v", warnings) +}