ci: replace the vet and gofmt steps with a golangci-lint job

.github/workflows/test.yml

@@ -10,7 +10,7 @@ permissions:
 
 jobs:
   test:
-    name: Unit tests, vet & build
+    name: Unit tests & build
     runs-on: ubuntu-latest
     steps:
       - name: Check out code
@@ -22,18 +22,6 @@ jobs:
           go-version-file: go.mod
           cache: true
 
-      - name: Check formatting
-        run: |
-          unformatted=$(gofmt -l apps)
-          if [ -n "$unformatted" ]; then
-            echo "These files are not gofmt-formatted:"
-            echo "$unformatted"
-            exit 1
-          fi
-
-      - name: Vet
-        run: go vet ./...
-
       - name: Install gotestsum
         run: go install gotest.tools/gotestsum@v1.13.0
 
@@ -45,3 +33,23 @@ jobs:
 
       - name: Build
         run: go build ./apps/cinc
+
+  lint:
+    name: Lint
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out code
+        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+
+      - name: Set up Go
+        uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
+        with:
+          go-version-file: go.mod
+          cache: true
+
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0
+        with:
+          # Pinned so a new linter release cannot turn a green branch red
+          # without a commit.
+          version: v2.13.2

.golangci.yml

@@ -0,0 +1,79 @@
+version: "2"
+
+run:
+  # test/acceptance sits behind a build tag; without naming it here the linter
+  # never loads those files. (The race/!race pair in cli/policyfile/rubyeval
+  # is inherently one-or-the-other and follows the default build, as `go vet`
+  # did.)
+  build-tags:
+    - acceptance
+
+# The default set: errcheck, govet, ineffassign, staticcheck, unused.
+# govet makes a separate `go vet` step redundant, and the gofmt formatter
+# below makes a separate `gofmt -l` step redundant, so CI runs this alone.
+linters:
+  default: standard
+
+  settings:
+    staticcheck:
+      # ST1005 wants error strings without trailing punctuation. That fits
+      # errors meant to be wrapped; these are a CLI's top-level messages,
+      # written as full sentences on purpose ("can't find your client key at
+      # %s — it's set as client_key in %s. Create the key file, or ..."). The
+      # style is deliberate and documented at each site, so the check is off
+      # rather than the messages flattened.
+      # This repeats golangci-lint's own default exclusions; listing only
+      # "all" and -ST1005 would silently re-enable ST1000/ST1003/ST1016/
+      # ST1020/ST1021/ST1022 and bury the real findings in doc-comment noise.
+      checks:
+        - all
+        - -ST1000
+        - -ST1003
+        - -ST1016
+        - -ST1020
+        - -ST1021
+        - -ST1022
+        - -ST1005
+
+    errcheck:
+      # Errors that are genuinely nothing to act on. Everything else must be
+      # handled, including the ones that look boring.
+      exclude-functions:
+        # Deferred close on a read-only handle: the error reports a flush
+        # failure there is no write to lose.
+        - (io.Closer).Close
+        - (*database/sql.Rows).Close
+        # Rollback after a committed or already-failed transaction returns
+        # sql.ErrTxDone by design; the deferred call is the safety net.
+        - (*database/sql.Tx).Rollback
+        # Draining a response body to make the connection reusable, and
+        # writing usage text to stderr. A failure changes nothing we can do.
+        - io.Copy
+        - fmt.Fprintf
+        - fmt.Fprintln
+        - fmt.Fprint
+        # Deferred cleanup of a temp dir the process is done with.
+        - os.RemoveAll
+
+  exclusions:
+    rules:
+      # Test helpers close temp handles and ignore returns on purpose; the
+      # test failing IS the error handling.
+      - path: _test\.go
+        linters: [errcheck]
+
+formatters:
+  # Reported as issues by `golangci-lint run`, so this replaces a standalone
+  # gofmt check. Applied (rather than reported) by `golangci-lint fmt`.
+  enable:
+    - gofmt
+
+issues:
+  # Defaults are max-issues-per-linter: 50 and max-same-issues: 3, which
+  # truncate repeated findings — fix three and three more appear. A blocking
+  # gate has to report everything at once.
+  max-issues-per-linter: 0
+  max-same-issues: 0
+  # Also on by default: only the first issue on any given line is reported.
+  # Two distinct problems on one line means one of them is silently dropped.
+  uniq-by-line: false

Makefile

@@ -15,7 +15,7 @@ LDFLAGS := -X $(LDFLAGS_PKG).version=$(VERSION) \
            -X $(LDFLAGS_PKG).commit=$(COMMIT) \
            -X $(LDFLAGS_PKG).buildDate=$(BUILD_DATE)
 
-.PHONY: all build dist install test test-acceptance vet fmt tidy clean run docs help
+.PHONY: all build dist install test test-acceptance vet lint fmt tidy clean run docs help
 
 all: build
 
@@ -59,6 +59,12 @@ test-acceptance:
 vet:
 	go vet ./...
 
+## lint: run golangci-lint (config in .golangci.yml). Subsumes vet and a gofmt
+## check, and unlike `gofmt -l apps` it covers every package.
+## Install: go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.13.2
+lint:
+	golangci-lint run ./...
+
 ## fmt: format all Go source
 fmt:
 	gofmt -w apps

apps/cinc/cmd/keys.go

@@ -274,11 +274,11 @@ func writePrivateKey(out io.Writer, priv, keyFile, fileMsg string) error {
 			return fmt.Errorf("cinc: write key file: %w", err)
 		}
 		if err := f.Chmod(0o600); err != nil {
-			f.Close()
+			_ = f.Close() // already returning an error
 			return fmt.Errorf("cinc: write key file: %w", err)
 		}
 		if _, err := f.WriteString(priv); err != nil {
-			f.Close()
+			_ = f.Close() // already returning an error
 			return fmt.Errorf("cinc: write key file: %w", err)
 		}
 		if err := f.Close(); err != nil {

cli/cookbook/cookbook.go

@@ -570,7 +570,7 @@ func writeFileToTar(tw *tar.Writer, dir, entryPath string) error {
 	if err != nil {
 		return fmt.Errorf("open %s: %w", entryPath, err)
 	}
-	defer f.Close()
+	defer func() { _ = f.Close() }() // read handle
 	if _, err := io.Copy(tw, f); err != nil {
 		return fmt.Errorf("write tar entry %s: %w", entryPath, err)
 	}
@@ -745,7 +745,7 @@ func ExtractArchiveFiles(data []byte) ([]string, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer gz.Close()
+	defer func() { _ = gz.Close() }() // read handle
 	tr := tar.NewReader(gz)
 	var files []string
 	for {

cli/cookbook/extract.go

@@ -37,7 +37,7 @@ func ExtractArchive(r io.Reader, destDir string) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("open gzip: %w", err)
 	}
-	defer gz.Close()
+	defer func() { _ = gz.Close() }() // read handle
 
 	tr := tar.NewReader(gz)
 	roots := map[string]struct{}{}
@@ -119,7 +119,7 @@ func writeFile(r io.Reader, target, name string, total *int64) error {
 		return fmt.Errorf("create %s: %w", target, err)
 	}
 	if err := boundedCopy(f, r, name, total); err != nil {
-		f.Close()
+		_ = f.Close() // already returning an error
 		return fmt.Errorf("write %s: %w", target, err)
 	}
 	if err := f.Close(); err != nil {

cli/explore/actions_test.go

@@ -39,7 +39,7 @@ func TestEditFlowSavesEditedObject(t *testing.T) {
 
 	// e starts an edit of the selected node; the seed is fetched async.
 	m, cmd = pressRune(t, m, 'e')
-	m, cmd = step(t, m, drain(cmd)) // editSeedMsg → editor opens
+	m, _ = step(t, m, drain(cmd)) // editSeedMsg → editor opens
 	if m.screen != screenEditor {
 		t.Fatalf("screen = %v, want editor", m.screen)
 	}
@@ -74,11 +74,11 @@ func TestCreateWithSecretShowsResultModal(t *testing.T) {
 	m.cur = newUserKind()
 	m.screen = screenList
 
-	m, cmd := m2(m.startCreate()) // n: Creatable → editor with template
+	m, _ = m2(m.startCreate()) // n: Creatable → editor with template
 	if m.screen != screenEditor {
 		t.Fatalf("screen = %v, want editor", m.screen)
 	}
-	m, cmd = commitEditor(t, m)
+	m, cmd := commitEditor(t, m)
 	m, _ = step(t, m, drain(cmd)) // mutationDoneMsg with secret
 
 	if m.screen != screenResult {

cli/explore/search_test.go

@@ -55,7 +55,7 @@ func TestSearchNarrowsListToServerMatches(t *testing.T) {
 	m, _ := openNodes(t, searchMux(t))
 
 	// Open the search modal; it defaults to the current list's index.
-	m, cmd := pressRune(t, m, 's')
+	m, _ = pressRune(t, m, 's')
 	if m.screen != screenSearch {
 		t.Fatalf("screen = %v, want search", m.screen)
 	}
@@ -65,7 +65,7 @@ func TestSearchNarrowsListToServerMatches(t *testing.T) {
 
 	// Type a query and run it.
 	m, _ = step(t, m, keyRunes("role:web"))
-	m, cmd = pressKey(t, m, 13) // enter
+	m, cmd := pressKey(t, m, 13) // enter
 	msg := drain(cmd)
 	sl, ok := msg.(searchLoadedMsg)
 	if !ok {

cli/nodeedit/convert_test.go

@@ -39,7 +39,8 @@ func TestAttributesSeedShowsEditableBagsInOrderAndExcludesAutomatic(t *testing.T
 	iNormal := strings.Index(s, "normal")
 	iDefault := strings.Index(s, "default")
 	iOverride := strings.Index(s, "override")
-	if !(iNormal >= 0 && iNormal < iDefault && iDefault < iOverride) {
+	ordered := iNormal >= 0 && iNormal < iDefault && iDefault < iOverride
+	if !ordered {
 		t.Errorf("editable bags out of order in seed:\n%s", s)
 	}
 	if strings.Contains(s, "automatic") {

cli/policyfile/archive.go

@@ -86,12 +86,12 @@ func extractBundleTarball(archivePath, dest string) error {
 	if err != nil {
 		return err
 	}
-	defer f.Close()
+	defer func() { _ = f.Close() }() // read handle
 	gz, err := gzip.NewReader(f)
 	if err != nil {
 		return fmt.Errorf("policyfile: open archive %s: %w", archivePath, err)
 	}
-	defer gz.Close()
+	defer func() { _ = gz.Close() }() // read handle
 
 	tr := tar.NewReader(gz)
 	var total int64
@@ -122,7 +122,7 @@ func extractBundleTarball(archivePath, dest string) error {
 				return err
 			}
 			if err := boundedCopy(out, tr, hdr.Name, &total); err != nil {
-				out.Close()
+				_ = out.Close() // already returning an error
 				return fmt.Errorf("policyfile: %w", err)
 			}
 			if err := out.Close(); err != nil {

cli/policyfile/export.go

@@ -129,14 +129,14 @@ func copyTree(src, dst string) error {
 		if err != nil {
 			return err
 		}
-		defer in.Close()
+		defer func() { _ = in.Close() }() // read handle
 		// Clamp to a safe mode rather than trusting the source file's bits.
 		out, err := os.OpenFile(target, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, extractFileMode)
 		if err != nil {
 			return err
 		}
 		if _, err := io.Copy(out, in); err != nil {
-			out.Close()
+			_ = out.Close() // already returning an error
 			return err
 		}
 		return out.Close()
@@ -145,16 +145,24 @@ func copyTree(src, dst string) error {
 
 // tarGzDir writes dir as a gzip-compressed tarball at archivePath, with entries
 // rooted at the directory's base name.
-func tarGzDir(dir, archivePath string) error {
+func tarGzDir(dir, archivePath string) (err error) {
 	f, err := os.Create(archivePath)
 	if err != nil {
 		return err
 	}
-	defer f.Close()
 	gz := gzip.NewWriter(f)
-	defer gz.Close()
 	tw := tar.NewWriter(gz)
-	defer tw.Close()
+	// The tar trailer and the gzip footer are written by Close, so a failure
+	// there means a truncated archive. Closing innermost first and keeping the
+	// first error is what makes that surface instead of being reported as a
+	// successful export.
+	defer func() {
+		for _, c := range []io.Closer{tw, gz, f} {
+			if cerr := c.Close(); err == nil {
+				err = cerr
+			}
+		}
+	}()
 
 	root := filepath.Base(dir)
 	return filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
@@ -183,7 +191,7 @@ func tarGzDir(dir, archivePath string) error {
 		if err != nil {
 			return err
 		}
-		defer in.Close()
+		defer func() { _ = in.Close() }() // read handle
 		_, err = io.Copy(tw, in)
 		return err
 	})

cli/policyfile/extract.go

@@ -64,7 +64,7 @@ func extractCookbookTarball(r io.Reader, dest string) error {
 	if err != nil {
 		return fmt.Errorf("supermarket: open gzip: %w", err)
 	}
-	defer gz.Close()
+	defer func() { _ = gz.Close() }() // read handle
 
 	tr := tar.NewReader(gz)
 	var total int64
@@ -98,7 +98,7 @@ func extractCookbookTarball(r io.Reader, dest string) error {
 				return err
 			}
 			if err := boundedCopy(f, tr, hdr.Name, &total); err != nil {
-				f.Close()
+				_ = f.Close() // already returning an error
 				return fmt.Errorf("supermarket: %w", err)
 			}
 			if err := f.Close(); err != nil {

cli/policyfile/rubyeval/loader.go

@@ -191,7 +191,7 @@ func verifyFileSHA256(path, wantHex string) error {
 	if err != nil {
 		return err
 	}
-	defer f.Close()
+	defer func() { _ = f.Close() }() // read handle
 	h := sha256.New()
 	if _, err := io.Copy(h, f); err != nil {
 		return err
@@ -210,7 +210,7 @@ func extractTarGz(archive []byte, dest string) error {
 	if err != nil {
 		return err
 	}
-	defer gz.Close()
+	defer func() { _ = gz.Close() }() // read handle
 	tr := tar.NewReader(gz)
 	for {
 		hdr, err := tr.Next()
@@ -238,7 +238,7 @@ func extractTarGz(archive []byte, dest string) error {
 				return err
 			}
 			if _, err := io.Copy(f, tr); err != nil { //nolint:gosec // pinned, checksum-verified archive
-				f.Close()
+				_ = f.Close() // already returning an error
 				return err
 			}
 			if err := f.Close(); err != nil {

cli/policyfile/rubyeval/rubyeval.go

@@ -324,7 +324,7 @@ func readCapped(path string, max int64) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer f.Close()
+	defer func() { _ = f.Close() }() // read handle
 	if info, err := f.Stat(); err == nil && info.Size() > max {
 		return nil, fmt.Errorf("policyfile: engine result is %d bytes, over the %d-byte cap", info.Size(), max)
 	}