Harden extraction, attestation, and install orchestration

.github/workflows/test.yml

@@ -5,13 +5,45 @@ on:
   pull_request:
     branches: [main]
 
+permissions:
+  contents: read
+
 jobs:
   test:
-    runs-on: ubuntu-latest
+    name: test (${{ matrix.os }})
+    strategy:
+      fail-fast: false
+      matrix:
+        # gh-tool is a Unix-targeted CLI: installs are symlink-based, paths
+        # follow the XDG layout, and shell integration covers bash/zsh/fish.
+        # Test on the platforms it actually supports as a runtime.
+        os: [ubuntu-latest, macos-latest]
+    runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
           go-version-file: go.mod
       - run: go build ./...
-      - run: go test ./...
+      - run: go vet ./...
+      - run: go test -race ./...
+
+  lint:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: go.mod
+      - name: gofmt
+        run: |
+          files=$(gofmt -l .)
+          if [ -n "$files" ]; then
+            echo "The following files are not gofmt-formatted:"
+            echo "$files"
+            echo "Run 'gofmt -w .' to fix."
+            exit 1
+          fi
+      - name: staticcheck
+        run: go run honnef.co/go/tools/cmd/staticcheck@latest ./...

cmd/add.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -45,10 +46,12 @@ func init() {
 
 func runAdd(cmd *cobra.Command, args []string) error {
 	if !term.IsTerminal(int(os.Stdin.Fd())) || !term.IsTerminal(int(os.Stdout.Fd())) {
-		return fmt.Errorf(`gh tool add requires an interactive terminal.
+		fmt.Fprintf(os.Stderr, `gh tool add requires an interactive terminal.
 For non-interactive use, run:
   gh tool install %s --pattern '...' --bin '...'
-or edit your manifest directly.`, args[0])
+or edit your manifest directly.
+`, args[0])
+		return errors.New("interactive terminal required")
 	}
 
 	repo := args[0]
@@ -70,7 +73,7 @@ or edit your manifest directly.`, args[0])
 	if err != nil {
 		return err
 	}
-	fmt.Printf("✓ %s @ %s — %d classified assets, %d skipped\n", rel.Repo, rel.Tag, len(rel.All), len(rel.Skipped))
+	fmt.Printf("%s %s @ %s — %d classified assets, %d skipped\n", ui.Success(ui.IconSuccess), rel.Repo, rel.Tag, len(rel.All), len(rel.Skipped))
 
 	platforms := rel.Platforms()
 	if len(platforms) == 0 {
@@ -183,9 +186,9 @@ or edit your manifest directly.`, args[0])
 		return fmt.Errorf("saving manifest: %w", err)
 	}
 	if existing {
-		fmt.Printf("✓ Updated %s in %s\n", repo, mfPath)
+		fmt.Printf("%s Updated %s in %s\n", ui.Success(ui.IconSuccess), repo, mfPath)
 	} else {
-		fmt.Printf("✓ Saved %s to %s\n", repo, mfPath)
+		fmt.Printf("%s Saved %s to %s\n", ui.Success(ui.IconSuccess), repo, mfPath)
 	}
 
 	if !flagAddInstall {
@@ -229,7 +232,7 @@ func refineDarwinArchs(chosen map[discover.PlatformKey]string, inspectAssetName
 
 func chooseAddPlatforms(platforms []discover.PlatformKey) ([]discover.PlatformKey, error) {
 	if len(platforms) == 1 {
-		fmt.Printf("· Only one platform detected: %s\n", platforms[0])
+		fmt.Printf("%s Only one platform detected: %s\n", ui.IconBullet, platforms[0])
 		return platforms, nil
 	}
 	options := make([]string, len(platforms))
@@ -425,17 +428,15 @@ func chooseAddBins(layout *discover.Layout, repo, inspectAssetName, foldedPatter
 
 	var picked []string
 	if match := layout.MatchBinName(name); match != "" && len(layout.Executables) == 1 {
-		fmt.Printf("· Auto-detected bin: %s\n", match)
+		fmt.Printf("%s Auto-detected bin: %s\n", ui.IconBullet, match)
 		picked = []string{match}
 	} else {
 		options := make([]string, len(layout.Executables))
 		// Default to selecting every detected executable. Multi-binary
 		// releases (uv: uv+uvx, git: many) almost always want all of
 		// them, and a single-executable case where the name doesn't
 		// match the repo (handled below) still wants the binary.
-		for i, e := range layout.Executables {
-			options[i] = e
-		}
+		copy(options, layout.Executables)
 		if err := promptMultiSelect("Select binaries to symlink:", options, options, &picked); err != nil {
 			return nil, err
 		}

cmd/cache.go

@@ -2,13 +2,16 @@ package cmd
 
 import (
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"strings"
 
 	"github.com/cli/go-gh/v2/pkg/tableprinter"
 	"github.com/cli/go-gh/v2/pkg/term"
 	"github.com/spf13/cobra"
+
+	"github.com/ascarter/gh-tool/internal/ui"
 )
 
 var cacheCmd = &cobra.Command{
@@ -137,20 +140,24 @@ func runCacheClean(cmd *cobra.Command, args []string) error {
 		if err := os.RemoveAll(cacheDir); err != nil {
 			return err
 		}
-		fmt.Printf("✓ Cleaned cache for %s\n", args[0])
+		fmt.Printf("%s Cleaned cache for %s\n", ui.Success(ui.IconSuccess), args[0])
 		return nil
 	}
 
 	if err := os.RemoveAll(dirs.Cache); err != nil {
 		return err
 	}
-	fmt.Println("✓ Cleaned all cached downloads")
+	fmt.Printf("%s Cleaned all cached downloads\n", ui.Success(ui.IconSuccess))
 	return nil
 }
 
 func dirStats(dir string) (totalSize int64, fileCount int) {
-	_ = filepath.Walk(dir, func(_ string, info os.FileInfo, err error) error {
-		if err != nil || info.IsDir() {
+	_ = filepath.WalkDir(dir, func(_ string, d fs.DirEntry, err error) error {
+		if err != nil || d.IsDir() {
+			return nil
+		}
+		info, err := d.Info()
+		if err != nil {
 			return nil
 		}
 		totalSize += info.Size()

cmd/install.go

@@ -26,17 +26,18 @@ ad-hoc installs not in the manifest).`,
 }
 
 var (
-	flagPattern    string
-	flagTag        string
-	flagBin        []string
-	flagMan        []string
-	flagComp       []string
-	flagNoVerify   bool
-	flagForce      bool
-	flagFile       string
-	flagJobs       int
-	flagNoProgress bool
-	flagVerbose    bool
+	flagPattern            string
+	flagTag                string
+	flagBin                []string
+	flagMan                []string
+	flagComp               []string
+	flagNoVerify           bool
+	flagRequireAttestation bool
+	flagForce              bool
+	flagFile               string
+	flagJobs               int
+	flagNoProgress         bool
+	flagVerbose            bool
 )
 
 func init() {
@@ -46,11 +47,13 @@ func init() {
 	installCmd.Flags().StringSliceVar(&flagMan, "man", nil, "man page path(s) in archive")
 	installCmd.Flags().StringSliceVar(&flagComp, "completion", nil, "completion path(s) in archive")
 	installCmd.Flags().BoolVar(&flagNoVerify, "no-verify", false, "skip attestation verification")
+	installCmd.Flags().BoolVar(&flagRequireAttestation, "require-attestation", false, "fail if an attestation exists but does not verify")
 	installCmd.Flags().BoolVar(&flagForce, "force", false, "reinstall even if up-to-date")
 	installCmd.Flags().StringVarP(&flagFile, "file", "f", "", "manifest path (default: $XDG_CONFIG_HOME/gh-tool/config.toml)")
 	installCmd.Flags().IntVarP(&flagJobs, "jobs", "j", 0, "parallel installs (default: min(8, NumCPU))")
 	installCmd.Flags().BoolVar(&flagNoProgress, "no-progress", false, "disable the live progress UI")
 	installCmd.Flags().BoolVarP(&flagVerbose, "verbose", "v", false, "log every step (download, verify, extract)")
+	rootCmd.AddCommand(installCmd)
 }
 
 // manifestPath returns the manifest path honoring --file, falling back to the
@@ -65,6 +68,7 @@ func manifestPath(dirs paths.Dirs) string {
 func runInstall(cmd *cobra.Command, args []string) error {
 	dirs := resolveDirs()
 	mgr := tool.NewManager(dirs)
+	mgr.RequireAttestation = flagRequireAttestation
 	mfPath := manifestPath(dirs)
 
 	cfg, err := config.Load(mfPath)
@@ -112,39 +116,90 @@ func runInstall(cmd *cobra.Command, args []string) error {
 
 	if flagForce {
 		mgr.CleanupInstall(t.Name())
-	} else if isUpToDate(mgr, t) {
-		return nil
+	} else if targetTag, err := resolveTargetTag(t); err == nil {
+		if state := mgr.ReadState(t.Name()); state != nil && upToDate(state, t, targetTag) {
+			fmt.Printf("%s %s up to date (%s)\n", ui.IconBullet, t.Name(), targetTag)
+			return nil
+		}
+		// Thread the resolved tag through so Install does not resolve it
+		// a second time.
+		t.Tag = targetTag
 	}
 
 	return mgr.Install(t, !flagNoVerify)
 }
 
 // runInstallReconcile reconciles the local install set against the manifest
 // in parallel. Tools filtered out by ShouldInstallOn or already at the
-// target version are short-circuited before the worker pool spawns.
+// target version are short-circuited before the worker pool spawns. The
+// target tag for each eligible tool is resolved once, up front (in
+// parallel), and threaded into Install so the latest tag is not resolved a
+// second time during download.
 func runInstallReconcile(mgr *tool.Manager, cfg *config.Config) error {
 	if len(cfg.Tools) == 0 {
 		fmt.Println("No tools in manifest. Use: gh tool add <owner/repo>")
 		return nil
 	}
 
 	verify := !flagNoVerify
-	type pending struct {
-		t config.Tool
-	}
-	var queue []pending
 
+	// Phase 1: filter by OS before making any network calls.
+	type item struct {
+		t          config.Tool
+		targetTag  string
+		resolveErr error
+	}
+	items := make([]*item, 0, len(cfg.Tools))
 	for _, t := range cfg.Tools {
 		if !t.ShouldInstallOn(runtime.GOOS) {
 			fmt.Printf("%s %s skipped on %s\n", ui.IconBullet, t.Name(), runtime.GOOS)
 			continue
 		}
+		items = append(items, &item{t: t})
+	}
+	if len(items) == 0 {
+		return nil
+	}
+
+	// Phase 2: resolve each eligible tool's target tag in parallel. This is
+	// the slow part of reconcile (one `gh release view` per unpinned tool).
+	// Resolving once here lets us both compare against installed state and
+	// thread the resolved tag into Install, avoiding a second resolution.
+	resolveJobs := make([]ui.Job, 0, len(items))
+	for _, it := range items {
+		it := it
+		resolveJobs = append(resolveJobs, ui.Job{
+			Name: it.t.Name(),
+			Run: func() error {
+				it.targetTag, it.resolveErr = resolveTargetTag(it.t)
+				return nil
+			},
+		})
+	}
+	_, _ = ui.Run(resolveJobs, ui.ResolveJobs(flagJobs))
+
+	// Phase 3: compare, print, and queue serially so output stays ordered.
+	queue := make([]config.Tool, 0, len(items))
+	for _, it := range items {
+		name := it.t.Name()
 		if flagForce {
-			mgr.CleanupInstall(t.Name())
-		} else if isUpToDate(mgr, t) {
+			mgr.CleanupInstall(name)
+		}
+		if it.resolveErr != nil {
+			// Resolution failed; queue with the original spec so Install
+			// re-resolves and surfaces the error through its normal path.
+			queue = append(queue, it.t)
 			continue
 		}
-		queue = append(queue, pending{t: t})
+		if !flagForce {
+			if state := mgr.ReadState(name); state != nil && upToDate(state, it.t, it.targetTag) {
+				fmt.Printf("%s %s up to date (%s)\n", ui.IconBullet, name, it.targetTag)
+				continue
+			}
+		}
+		t := it.t
+		t.Tag = it.targetTag
+		queue = append(queue, t)
 	}
 
 	if len(queue) == 0 {
@@ -165,8 +220,8 @@ func runInstallReconcile(mgr *tool.Manager, cfg *config.Config) error {
 	}
 
 	jobs := make([]ui.Job, 0, len(queue))
-	for _, p := range queue {
-		t := p.t
+	for _, t := range queue {
+		t := t
 		jobs = append(jobs, ui.Job{
 			Name: t.Name(),
 			Run:  func() error { return mgr.Install(t, verify) },
@@ -191,39 +246,42 @@ func runInstallReconcile(mgr *tool.Manager, cfg *config.Config) error {
 	return nil
 }
 
-// isUpToDate reports whether the installed copy already matches the
-// target version AND the manifest's asset spec. The spec check covers
-// the case where the user added or renamed a bin/man/completion entry
-// after the tool was already installed at the current release tag —
-// without it, `gh tool install` would print "up to date" and never
-// create the new symlinks.
-func isUpToDate(mgr *tool.Manager, t config.Tool) bool {
-	name := t.Name()
-	state := mgr.ReadState(name)
-	if state == nil {
-		return false
+// resolveTargetTag returns the tag a tool should be installed at: the pinned
+// tag when one is set, otherwise the repo's latest release tag resolved via
+// the GitHub API. An empty latest tag is treated as an error so callers don't
+// accidentally thread an empty tag (which would silently fall back to
+// re-resolution downstream).
+func resolveTargetTag(t config.Tool) (string, error) {
+	if t.Tag != "" && t.Tag != "latest" {
+		return t.Tag, nil
 	}
-
-	targetTag := t.Tag
-	if targetTag == "" || targetTag == "latest" {
-		latest, err := tool.LatestTag(t.Repo)
-		if err != nil {
-			return false
-		}
-		targetTag = latest
+	tag, err := tool.LatestTag(t.Repo)
+	if err != nil {
+		return "", err
+	}
+	if tag == "" {
+		return "", fmt.Errorf("no latest release tag for %s", t.Repo)
 	}
+	return tag, nil
+}
 
-	if state.Tag != targetTag {
+// upToDate reports whether the installed copy already matches the target tag
+// AND the manifest's asset spec. The spec check covers the case where the
+// user added or renamed a bin/man/completion entry after the tool was already
+// installed at the current release tag — without it, `gh tool install` would
+// print "up to date" and never create the new symlinks. This function is pure:
+// it performs no I/O and prints nothing, so the resolution of targetTag is the
+// caller's responsibility.
+func upToDate(state *tool.InstalledState, t config.Tool, targetTag string) bool {
+	if state == nil {
 		return false
 	}
-	if !stringSlicesEqual(state.Bin, t.Bin) ||
-		!stringSlicesEqual(state.Man, t.Man) ||
-		!stringSlicesEqual(state.Completions, t.Completions) {
+	if state.Tag != targetTag {
 		return false
 	}
-
-	fmt.Printf("%s %s up to date (%s)\n", ui.IconBullet, name, targetTag)
-	return true
+	return stringSlicesEqual(state.Bin, t.Bin) &&
+		stringSlicesEqual(state.Man, t.Man) &&
+		stringSlicesEqual(state.Completions, t.Completions)
 }
 
 // stringSlicesEqual compares two slices as unordered sets, treating