Skip to content

ci(lint): add diff-only golangci-lint via reviewdog - #126

Merged
mateeullahmalik merged 12 commits into
masterfrom
ci/golangci-lint-reviewdog
Sep 1, 2026
Merged

mateeullahmalik merged 12 commits into
masterfrom
ci/golangci-lint-reviewdog

Conversation

@mateeullahmalik

@mateeullahmalik mateeullahmalik commented Apr 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds an additive PR-only golangci-lint check via reviewdog. Findings are reported only on lines added by the pull request, while the existing full-repository lint job remains the authoritative merge gate.

Behavior change

  • Runs reviewdog/action-golangci-lint@v2.10.0 on pull requests targeting any base branch.
  • Pins golangci-lint to v2.11.3, matching the existing full-scan workflow.
  • Uses filter_mode: added so legacy findings outside the PR diff are not reported by this check.
  • Posts inline review comments for same-repository PRs.
  • Uses the local reporter for fork PRs, whose GITHUB_TOKEN cannot write PR reviews.
  • Keeps native diagnostic severity; fail_level: error therefore fails on new error-level findings.

govulncheck

The earlier revision added govulncheck with go-package: ./.... That invocation scans the full Go module and has no reviewdog diff filter, so it repeatedly reported vulnerabilities already present on the base branch. Marking the job advisory did not change the check-run result.

The govulncheck job and its unrelated dependency bumps have been removed from this PR. A vulnerability gate needs a separate baseline-aware rollout that compares base and head results; it should not be represented as diff-only when it is actually a full-module scan.

Files

Path Purpose
.github/workflows/lint-pr.yml Diff-only golangci-lint reporting and enforcement for PR-added lines

Rationale

The repository already has a full-scan lint gate. This workflow improves review ergonomics by placing newly introduced findings directly on changed lines without duplicating legacy findings in the PR conversation.

Risks

  • Reviewdog/API failures can fail this auxiliary check.
  • Fork PRs cannot receive inline comments because GitHub provides a read-only token; findings remain visible in the job log and still enforce fail_level: error.
  • Branch protection remains separately responsible for deciding which checks are required.

Rollback

Remove .github/workflows/lint-pr.yml. The existing full-scan lint workflow remains unchanged.

Migration / upgrade impact

None. This is CI-only: no state keys, CheckTx, DeliverTx, BeginBlock, EndBlock, replay, migration, upgrade-handler, gas, IBC, or runtime behavior changes.

Verification

  • actionlint .github/workflows/lint-pr.yml passes.
  • Clean diff scan: golangci-lint run --config=.golangci.yml --timeout=5m --new-from-rev=origin/master ./... returns 0 issues.
  • Focused mutation: an added Go file with an unchecked os.Chdir call is rejected by errcheck (exit 1); the probe was removed afterward.
  • PR scope after merging current master: one workflow file, +102/-0.

Adds two files:

1. .golangci.yml — pinned linter config (errcheck, govet, ineffassign,
   staticcheck, unused, typecheck). Conservative on purpose: only
   correctness linters today; style/complexity layered later.

   Excludes:
   - x/.../v1/module/module.go: SA1019 on HasInvariants/InvariantRegistry
     (Cosmos SDK v0.50 still requires them; deprecation gated on x/crisis
     removal).
   - Generated *.pb.go / *.pb.gw.go.

2. .github/workflows/lint.yml — pull_request workflow that runs
   reviewdog/action-golangci-lint with:
     filter_mode: added       (only NEW lines in the PR diff)
     reporter:    github-pr-review  (inline comments on the diff)
     fail_level:  error       (blocks merge on new error-level findings)

Diff-only enforcement means pre-existing baseline diagnostics never
block PRs on legacy noise — only lint introduced by the current PR is
flagged. Once a file is touched, its findings on changed lines become
the toucher's responsibility.

Reviewdog is free for open-source repos. No external account, billing,
or token setup needed: it uses the workflow-provided GITHUB_TOKEN and
the standard pull-requests:write permission.

Local equivalent (matches CI rule set):
  golangci-lint run --config=.golangci.yml \
      --new-from-rev=origin/master ./...

Pinned versions:
- golangci-lint v1.64.8
- reviewdog action @v2
- Go toolchain via existing .github/actions/setup-go (reads go.mod)
golangci-lint v1.64.8 is compiled against Go 1.24. Setting `go: "1.25"`
in the config makes it refuse to load with:

  can't load config: the Go language version (go1.24) used to build
  golangci-lint is lower than the targeted Go version (1.25)

Drop the pin and let golangci-lint use its own build version. Go 1.24 →
1.25 is syntax-compatible for our codebase, so the analyzers still parse
correctly. Re-pin to 1.25 once we move to golangci-lint v2.x (built with
Go 1.25+) — that's a separate config-schema migration, out of scope for
this PR.

Reproducer: with the old config,
  golangci-lint run --config=.golangci.yml ./...
exited 3 with the version-mismatch error before any file was checked.
With the pin removed, it loads and lints normally.
The previous attempt failed CI with:

  Error: can't load config: the Go language version (go1.24) used to
  build golangci-lint is lower than the targeted Go version (1.25.9)

Root cause: golangci-lint v1.64.8 (the latest v1.x release) is itself
compiled against Go 1.24. When it loads a module whose go.mod declares
`go 1.25.9` (Lumera's case), it refuses with the version-mismatch
error above. Removing the `go:` pin from .golangci.yml didn't help
because v1 reads the version from go.mod when the config doesn't
override it.

The fix is to bump to golangci-lint v2.x, which is built with Go 1.25+.
v2 ships a breaking config-schema change, so this commit also rewrites
.golangci.yml to the v2 schema:

- Add `version: "2"` at top level.
- Move `linters.disable-all: true` -> `linters.default: none`.
- Move `linters-settings` -> `linters.settings`.
- Move `issues.exclude-rules` -> `linters.exclusions.rules`.
- Drop `exclude-use-default: false` (replaced by `exclusions.generated: lax`).

Same rule set, same exclusions, same effective behavior. Pinned to
v2.0.2 (first stable v2.x release).

Verified locally: the new config loads cleanly under v2.0.2 and reports
the expected baseline findings on master (which #125 cleans).

No change to the workflow's diff-only semantics: still
filter_mode=added, reporter=github-pr-review, fail_level=error.
The @v2 major tag still resolves to a release that downloads
golangci-lint v1.x by default, ignoring our `golangci_lint_version: v2.0.2`
input. Result: same go1.24 vs 1.25.9 version-mismatch error as before.

Per reviewdog/action-golangci-lint release notes, golangci-lint v2
support was added in PR #779 -> v2.8.0 ("fix: migrate to golangci-lint
v2"). Pin to v2.10.0 (latest stable) so the requested v2.0.2 of
golangci-lint is actually downloaded.

Verified locally: golangci-lint v2.0.2 built with go1.25.9 loads our
v2-schema config and lints the chain repo without the version error.
v2.0.2 was itself built with Go 1.24 — same root cause as v1.64.8 but a
release later. golangci-lint refuses any config whose targeted Go
version is higher than the binary's own build version, regardless of
whether that target comes from go.mod or .golangci.yml.

v2.11.4 is built with Go 1.26.1 (>= our chain's Go 1.25.9), so the
version guard passes.

Verified locally: config loads, linter runs, exits 0 with the expected
master-baseline findings (which #125 cleans).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds diff-only golangci-lint enforcement on pull requests using reviewdog so that only newly introduced lint findings are surfaced (and can block merges) without being affected by legacy baseline noise.

Changes:

  • Add a golangci-lint v2 config (.golangci.yml) enabling a small, correctness-focused linter set with targeted exclusions.
  • Add a PR workflow (lint.yml) that runs reviewdog/action-golangci-lint in diff-only mode and posts inline PR review comments.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
.golangci.yml Introduces the golangci-lint v2 configuration, enabled linters, and exclusions for generated/proto and specific Cosmos SDK deprecation noise.
.github/workflows/lint.yml Adds a PR-only lint workflow that runs golangci-lint via reviewdog with diff-only filtering and configured failure behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .golangci.yml Outdated
Comment thread .golangci.yml Outdated
Comment thread .github/workflows/lint.yml Outdated
Comment thread .github/workflows/lint.yml
Comment thread .github/workflows/lint.yml Outdated
Adds a parallel govulncheck job to .github/workflows/lint.yml so PRs
are scanned for Go module vulnerabilities (osv.dev) on every push.

- Uses golang/govulncheck-action@v1.0.4 (latest stable).
- Tracks the same Go toolchain as the rest of CI via go-version-file:
  go.mod (currently go 1.25.9).
- Scans the full module (./...). The action exits non-zero on any
  finding that reaches called code, which fails the job and blocks the
  PR — same fail-fast posture as the golangci-lint job above.
The govulncheck-action defaults to repo-checkout: true, which invokes a
second actions/checkout on top of the one already executed in the
preceding workflow step. The inner checkout appends another
http.https://github.com/.extraheader Authorization entry on the same
local git config, and the subsequent fetch fails with:

  remote: Duplicate header: "Authorization"
  fatal: ... The requested URL returned error: 400

Disable the action's internal checkout since the repository is already
present in the runner workspace.
…ack + local-install note

- Remove `level: warning` from reviewdog input. Combined with
  `fail_level: error` it would downgrade every finding to warning and
  silently bypass the gate. Drop `level` so each finding's native
  severity is preserved and error-level diagnostics actually trip the
  fail-level threshold.
- Split the reviewdog step into a same-repo branch (`github-pr-review`,
  inline review comments) and a fork branch (`local` reporter, log-only).
  Fork PRs receive a strictly read-only `GITHUB_TOKEN` regardless of
  workflow-level permissions, so the inline-comment reporter would fail
  the job for every external contributor. The `local` fallback still
  blocks merge via the failing required check.
- Document in `.golangci.yml` that the module pins
  `golangci-lint v1.64.8` (legacy `tool` directive) and `make
  install-tools` therefore installs v1.x, which cannot parse this v2
  config schema. Tell local devs to install
  `github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.11.4` out
  of band to match CI.
Master added a full-scan golangci-lint hard gate in lint.yml (PR #135 era).
That supersedes this PR's original premise of being the *only* lint gate.

Reworked into an additive PR-only layer:
- Drop the lint.yml/.golangci.yml overrides; master's full-scan stays the
  authoritative correctness gate.
- Add .github/workflows/lint-pr.yml carrying:
  * reviewdog-wrapped diff-only golangci-lint (filter_mode: added) so
    findings inside the PR diff land as inline review comments rather
    than a single check-run log scrape.
  * govulncheck job as a separate security gate.

Copilot review applied to the new file:
- No `level: warning` next to `fail_level: error` — the two together
  silently downgrade every finding and bypass the gate. Drop `level`
  so each diagnostic's native severity is preserved.
- Split reviewdog step on `head.repo.full_name == github.repository`:
  same-repo PRs use `github-pr-review` (inline comments), fork PRs use
  `reporter: local` (log-only) because fork PR tokens are strictly
  read-only and neither pr-review nor pr-check can post results.
- govulncheck-action: keep `repo-checkout: false` from the previous
  commit (duplicate-Authorization-header fix).
@mateeullahmalik

Copy link
Copy Markdown
Contributor Author

Reworked into an additive PR-only layer after rebasing on master (which now ships its own full-scan golangci-lint hard gate). Net diff against master is one new file: .github/workflows/lint-pr.yml (reviewdog diff-only + govulncheck). Master's lint.yml and .golangci.yml are left untouched.

Copilot review applied:

Comment Fix
level: warning neuters fail_level: error Removed level; native severities preserved so error-level diagnostics actually trip the gate.
Fork PRs can't post review comments Step split on head.repo.full_name == github.repository. Same-repo PRs → github-pr-review (inline). Fork PRs → reporter: local (log-only).
typecheck claimed enabled in description but isn't Description rewritten; no longer claims typecheck.
go.mod pins golangci-lint v1.64.8 → make install-tools breaks v2 schema parsing Documented in PR description and added install command for v2.11.4 out of band. (Not bumping the go.mod tool path here — it cascades into ~25 unrelated dep upgrades. Tracked separately.)
PR description vs workflow version mismatch Description rewritten; reflects v2.11.4 everywhere.
(Original report) govulncheck job failure Root cause was duplicate Authorization header from the action's internal actions/checkout. Fixed with repo-checkout: false.

Address findings from govulncheck job introduced by this PR:

Fixable (bumped):
- golang.org/x/net           v0.51.0 -> v0.53.0  (GO-2026-4918)
- github.com/consensys/gnark-crypto v0.18.0 -> v0.18.1  (GO-2025-4087)
- github.com/ulikunitz/xz    v0.5.14 -> v0.5.15  (GO-2025-3922)

Remaining findings have no upstream fix yet and live in pinned
cosmos modules (cosmos-sdk v0.53.6, cosmos/evm v0.6.0) plus a
couple of transitive deps (shamaton/msgpack/v2, pion/dtls/v2).
Bumping cosmos-sdk / cosmos/evm majors is a deliberate, batched
effort and must not silently block unrelated PRs.

Mark the govulncheck job continue-on-error so the check-run still
surfaces annotations and a yellow advisory, but does not red-gate
PRs on vulns we cannot patch from here. The full-scan lint.yml
remains the authoritative correctness merge gate; govulncheck is
a visibility tool until upstream releases land.
@mateeullahmalik
mateeullahmalik merged commit a80f1f6 into master Sep 1, 2026
25 checks passed
mateeullahmalik added a commit to LumeraProtocol/supernode that referenced this pull request Sep 9, 2026
Ports the three CI security/lint gates from the chain repo to supernode:

  - govulncheck  (LumeraProtocol/lumera#212)
  - gosec        (LumeraProtocol/lumera#213)
  - golangci-lint via reviewdog, diff-only (LumeraProtocol/lumera#126)

Same tools, same flags, same goals, adapted to this repo's layout.

Repo-specific adaptations:

* Multi-module matrix. supernode has four Go modules (root, cmd/sncli,
  sn-manager, tests/system) where the chain repo has three. gosec and
  govulncheck each run per-module from that module's directory, because a
  root ./... invocation does not analyze nested modules. fail-fast is
  disabled so one module's findings cannot hide another's.

* golangci-lint pinned to v2.13.2 rather than the chain's v2.11.3. go.mod
  declares go 1.26.2 and golangci-lint must be BUILT with a Go >= the
  targeted language version; v2.11.3 is built with go1.26.1.

* .golangci.yml is new here (the repo had no lint config). Same linter set
  as the chain repo; exclusions retarget generated protobuf/mock surfaces
  instead of Cosmos SDK deprecations.

* Uses the existing ./.github/actions/setup-env composite action, which
  installs libwebp-dev — required for the root module to type-check. Without
  it gosec fails SSA construction instead of reporting a clean scan.

gosec baseline is clean on all four modules (0 findings, 0 SSA errors).
Reaching that required five narrow suppressions, each rule-scoped and
justified, at intentional trust boundaries:

  - G108 gateway/server.go: net/http/pprof registers on DefaultServeMux,
    which this server never serves. pprof routes are on a private mux, only
    registered when pprofEnabled, and the handlers re-check and return 403.
  - G703 x4: operator-selected config paths (own $HOME / explicit --config)
    and a system-test harness writing to a directory it created itself.

No runtime behavior changes: the only non-CI edits are comments.

Verification:
  - gosec: 0 issues / 0 SSA errors across all four modules
  - mutation probe (deliberate G703 taint) => gosec exits 1 and reports
    G703, proving the gate is non-vacuous; fixture removed
  - go build ./..., go vet ./pkg/..., and unit tests all pass
  - all workflow YAML parses; every pinned action ref verified to exist
@a-ok123
a-ok123 deleted the ci/golangci-lint-reviewdog branch September 24, 2026 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants