diff --git a/.github/workflows/golangci.yml b/.github/workflows/golangci.yml new file mode 100644 index 0000000..b44810a --- /dev/null +++ b/.github/workflows/golangci.yml @@ -0,0 +1,83 @@ +name: golangci-lint + +# Runs golangci-lint (with the gosec security linter) against the +# repo's .golangci.yml on every PR + push to develop/main. This is the +# "one tool, one config" successor being sized up for the standalone +# lint steps in build.yml, and the first thing in this repo that scans +# our own code for insecure patterns — govulncheck (build.yml + +# vulncheck.yml) only covers known CVEs in dependencies. +# +# Why the action is safe now: the golangci-lint-action timeout story +# (#6) was the SSA linters (staticcheck, unused) choking on the +# k8s.io/* dep tree. The current .golangci.yml enables no SSA linters; +# a full run measures ~11s wall locally / ~62s on the runner (#423). +# +# Part of backend#1305 (epic #930, Layer 1). + +on: + push: + branches: [develop, main] + pull_request: + branches: [develop, main] + +permissions: + contents: read + +concurrency: + group: golangci-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + golangci: + timeout-minutes: 10 + name: golangci-lint (advisory) + runs-on: ubuntu-latest + # ============================ ADVISORY ============================ + # DELIBERATELY not a required check, and `--issues-exit-code=0` on + # the run step keeps findings from red-Xing the job: golangci-lint + # exits non-zero on any finding by default, and the pre-existing + # gosec backlog (8 findings at introduction: 4x G204, 3x G304, + # 1x G115 — sized on #423, the PR that added this job) would + # otherwise fail every PR for issues it didn't introduce. Findings + # still surface as inline annotations and in the job log. + # + # Known blind spot while advisory: typecheck (compile) errors ride + # the same issues exit path as lint findings, so under this flag + # they exit 0 too (verified empirically on #423 — this is NOT a + # separate exit code). No real signal is lost: a non-compiling PR + # reds the required Test / Lint / Build jobs anyway, and a broken + # .golangci.yml still fails THIS job via the action's + # `golangci-lint config verify` pre-step, which --issues-exit-code + # does not touch. + # + # (Job-level `continue-on-error: true` is NOT the tool for this — + # it greens the workflow run but still shows the job itself as + # failed in the PR checks list; see the first run on #423.) + # + # REMOVE the `--issues-exit-code=0` arg when the backlog hits zero + # and this check flips to required (the advisory -> required + # pattern from backend#1303; tracked under backend#1305 / epic + # #930). A required check that can't fail is worse than no check. + # ================================================================== + steps: + - uses: actions/checkout@v7 + + - name: Set up Go + uses: actions/setup-go@v7 + with: + go-version-file: go.mod + cache: true + + # Both versions pinned for reproducibility, same policy as the + # standalone tools in build.yml (#127). golangci-lint v2.12.2 is + # built with Go 1.26 (required: go.mod says `go 1.26.0`; v1-era + # binaries can't typecheck this module). Bump deliberately, and + # keep the version in step with the format expectations noted in + # .golangci.yml. + - name: golangci-lint run (.golangci.yml) + uses: golangci/golangci-lint-action@v9.3.0 + with: + version: v2.12.2 + # ADVISORY MODE — see the block comment above (incl. the + # typecheck caveat). Remove at the required-flip. + args: --issues-exit-code=0 diff --git a/.golangci.yml b/.golangci.yml index a6d52e7..d5976f5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -4,9 +4,24 @@ # bugs without flooding PRs with style noise. Tune up over time # rather than turning everything on day one and quarantining half of # them. +# +# Format: golangci-lint v2 (`version: "2"`). The v1-format file died +# with golangci-lint v1: v1 binaries are EOL and predate Go 1.26, so +# they can't typecheck this module at all, and v2 binaries (what +# `brew install golangci-lint` ships) refuse v1 configs. Migrated via +# `golangci-lint migrate` (backend#1305, epic #930 Layer 1); needs +# golangci-lint >= v2. +# +# CI: run by the advisory `golangci-lint` job in +# .github/workflows/golangci.yml (pinned version there; keep it in +# lockstep when the format needs a newer binary). The old +# action-times-out story (#6) was about the SSA linters (staticcheck, +# unused) on the k8s.io dep tree — this set has none, so the action +# is safe with it. + +version: "2" run: - timeout: 5m # Track go.mod's `go` directive. Go's release cadence + `go mod # tidy`'s aggressive bumping (especially when k8s.io/* deps want # newer Go) keep dragging go.mod's minimum up; pinning a stale @@ -17,42 +32,67 @@ run: go: "1.26" linters: - disable-all: true + default: none enable: # The set is trimmed to the linters that DON'T do full whole-program # SSA analysis. `staticcheck` and `unused` were in the original set # and reproducibly caused the GitHub-hosted runner to time-budget # the job (~2 min then shutdown signal) on this module — k8s.io/* # transitive deps inflate the analysis graph enough to OOM-or-stall - # the standard 4-CPU/16GB runner. Re-enabling them is a v0.2 - # follow-up that needs either a larger runner, a much narrower - # scope (e.g. only `./internal/...`), or a faster successor like - # govulncheck for the security-only subset. + # the standard 4-CPU/16GB runner (#6). staticcheck now runs + # standalone in build.yml's Lint job instead. # Cheap, per-file checks — catch real bugs without SSA. - errcheck # unchecked error returns - govet # `go vet` - ineffassign # assignments that go nowhere + # Security: insecure code patterns (G1xx-G6xx) — command injection, + # path traversal, weak crypto, world-writable files. Complements + # govulncheck (known CVEs in deps) with our-own-code checks; this is + # a customer-installed binary that shells out and writes to disk, so + # both halves matter (backend#1305, epic #930 Layer 1). + - gosec + # Style / hygiene that pays for itself in code review time. - - gofmt - - goimports - misspell - unconvert # unnecessary type conversions -linters-settings: - goimports: - # Group imports: stdlib, third-party, our own. Keeps diffs - # readable when adding new imports. - local-prefixes: github.com/tracebloc/cli - -issues: - # Default exclusions hide a lot of real findings — opt back in. - exclude-use-default: false - - exclude-rules: - # Test files often deliberately ignore err returns from - # bytes.Buffer / strings.Builder / fmt.Fprintf, which never fail. - - path: _test\.go - linters: - - errcheck + exclusions: + # v1 had `exclude-use-default: false` — the default exclusions hide + # a lot of real findings, so keep opting back in (no presets). + generated: lax + rules: + # Test files often deliberately ignore err returns from + # bytes.Buffer / strings.Builder / fmt.Fprintf, which never fail. + # gosec is likewise test-exempt per convention: tests use fixed + # temp paths, os.Setenv, and relaxed perms that G-rules flag but + # that never ship in the binary. + - path: _test\.go + linters: + - errcheck + - gosec + paths: + - third_party$ + - builtin$ + - examples$ + +# v2 moved the formatters out of `linters`. Same tools as before — +# build.yml's Lint job runs the standalone equivalents (`gofmt -s`, +# `goimports -local`). +formatters: + enable: + - gofmt + - goimports + settings: + goimports: + # Group imports: stdlib, third-party, our own. Keeps diffs + # readable when adding new imports. + local-prefixes: + - github.com/tracebloc/cli + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$