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

[tas50]

CI ran gofmt -l apps and go vet ./..., with no real linter. This adds golangci-lint on its default set (errcheck, govet, ineffassign, staticcheck, unused) as a required job and removes both steps, since golangci-lint subsumes them. Lint is one job, and nothing is checked twice.

The formatting check gets much wider

gofmt -l apps covered 2 of this repo's 19 packages. Everything under cli/, test/ and tools/ was never formatting-checked. The linter covers all of them, and names the acceptance build tag so those files load too. (The race/!race pair in cli/policyfile/rubyeval is inherently one-or-the-other and follows the default build, exactly as go vet did.)

One real bug

tarGzDir in cli/policyfile/export.go:

defer f.Close()          // os.Create
gz := gzip.NewWriter(f)
defer gz.Close()
tw := tar.NewWriter(gz)
defer tw.Close()
return filepath.Walk(...)

The tar trailer and the gzip footer are written during those Closes, which run after the return value is already set. So a failure while finalising the archive was discarded entirely and tarGzDir returned nil — a truncated or corrupt policyfile export reported as success. Closes now run innermost-first through a named return that keeps the first error.

Worth a close look, as it is the one behavioural change in the PR.

The other 30 unchecked errors were fine

They fall into three groups, all now explicit rather than silent: read handles (os.Open + gzip.NewReader), error-path cleanup where a failure is already being returned, and SSH teardown in cli/remote. The success paths were already correct — e.g. keys.go and every extraction loop already do if err := f.Close(); err != nil. errcheck additionally excludes fmt.Fprint and deferred os.RemoveAll of a temp dir.

Two config subtleties worth knowing

ST1005 is off, with the reason recorded in .golangci.yml. It wants error strings without trailing punctuation, which is right for errors meant to be wrapped; these are the CLI's top-level user-facing 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 update client_key to point at the right path."). Flattening them to satisfy a library convention would make the tool worse.

The checks list repeats golangci-lint's own defaults. Writing just ["all", "-ST1005"] silently re-enables ST1000/ST1003/ST1016/ST1020/ST1021/ST1022, which golangci-lint excludes by default. That took findings from 11 to 60 of mostly doc-comment noise before I caught it.

Also disabled: max-issues-per-linter (50), max-same-issues (3) and uniq-by-line (only the first issue per line) — three defaults that silently drop findings from what is meant to be a blocking gate.

Verification

golangci-lint run ./... reports 0 issues. go vet ./..., gofmt -l ., go build ./apps/cinc and the full go test -race ./... suite all pass, including cli/policyfile, which covers the changed export path.