Skip to content

ci(security): add govulncheck, gosec and diff-only golangci-lint - #324

Open
mateeullahmalik wants to merge 1 commit into
masterfrom
ci/security-scanners-and-lint
Open

ci(security): add govulncheck, gosec and diff-only golangci-lint#324
mateeullahmalik wants to merge 1 commit into
masterfrom
ci/security-scanners-and-lint

Conversation

@mateeullahmalik

Copy link
Copy Markdown
Collaborator

Ports the three CI gates from the chain repo to supernode — same tools, same flags, same goals, adapted to this repo's layout.

Chain PR Tool This PR
LumeraProtocol/lumera#212 govulncheck .github/workflows/govulncheck.yml
LumeraProtocol/lumera#213 gosec .github/workflows/gosec.yml
LumeraProtocol/lumera#126 golangci-lint + reviewdog (diff-only) .github/workflows/lint-pr.yml

Behavior change

Three new workflows run on every PR and on pushes to master. No runtime behavior changes — the only non-CI edits are comments (#nosec annotations).

Repo-specific adaptations

These are the places where a literal copy of the chain workflows would have been wrong:

1. Four Go modules, not one. supernode has ., cmd/sncli, sn-manager, tests/system. gosec and govulncheck each run per-module from that module's directory via a matrix — a root ./... invocation does not analyze nested modules. fail-fast: false so one module's findings can't hide another's.

2. golangci-lint pinned to v2.13.2, not 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 or it hard-errors. v2.11.3 is built with go1.26.1. Verified locally:

version built with usable here
v2.11.3 (chain pin) go1.26.1 ✗ below module go directive
v2.12.2 go1.26.2
v2.13.2 go1.27.0 chosen

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

4. Uses the existing ./.github/actions/setup-env, which installs libwebp-dev. The root module needs it to type-check; without it gosec fails SSA construction rather than reporting a clean scan.

5. Diff-only lint is the only lint gate here. In the chain repo, lint-pr.yml is an additive layer over an authoritative full-scan lint.yml. supernode has no such gate and a legacy baseline of 89 findings (50 errcheck, 23 unused, 8 staticcheck, 4 ineffassign, 3 gofmt, 1 govet). A full-repo gate would be red on day one, so enforcement is scoped to the PR diff (filter_mode: added): new code must be clean, legacy debt is not retroactively blocking.

gosec: baseline is clean ✅

0 findings, 0 SSA/type errors, all four modules:

MODULE=.            EXIT=0 ISSUES=0 NOSEC=1 SSA_ERRORS=0
MODULE=cmd/sncli    EXIT=0 ISSUES=0 NOSEC=1 SSA_ERRORS=0
MODULE=sn-manager   EXIT=0 ISSUES=0 NOSEC=2 SSA_ERRORS=0
MODULE=tests/system EXIT=0 ISSUES=0 NOSEC=1 SSA_ERRORS=0

Reaching that required five narrow, rule-scoped, justified suppressions at intentional trust boundaries — no exclude-dir, no global rule exclusion, no continue-on-error:

Rule Location Justification
G108 gateway/server.go:8 net/http/pprof registers on DefaultServeMux, which this server never serves. pprof routes are on a private mux, only registered when pprofEnabled (testnet chain-id or ENABLE_PPROF=true), and the handlers re-check the flag and return 403.
G703 sn-manager/cmd/init.go:240 Path from operator's own $HOME; only stat'ed.
G703 sn-manager/cmd/helpers.go:53 Same.
G703 cmd/sncli/cli/utils.go:28 Explicit operator-supplied --config, already cleaned.
G703 tests/system/system.go:208 Test harness writing to a dir it created itself.

Mutation-tested (non-vacuous). Injected a deliberate G703 taint shape → gosec exited 1 and reported G703 in SARIF. Fixture deleted; no probe artifacts remain in the tree.

Note: the chain repo's gosec (devnet) job is currently red on master from an SSA/type-error coverage failure (package ibcutil has type errors, skipping SSA analysis). That's a coverage defect worth fixing there — this port does not inherit it.

govulncheck: ⚠️ this job will be RED on merge — please read

This is the one place I could not make green without a judgement call that belongs to you.

Reachable-vulnerability baseline on current master:

Module Reachable Fixable No upstream fix
. 18 16 2
cmd/sncli 16 14 2
sn-manager 10 10 0
tests/system 10 9 1

The overwhelming majority are Go stdlib advisories against go1.26.2 (net/http, crypto/tls, crypto/x509, net/url, html/template, encoding/asn1, net/textproto, net). They are fixed in later go1.26.x patches. I measured three options on a throwaway copy:

Option Change Root module result
A. Merge as-is none 18 reachable — job red immediately
B. Toolchain only go 1.26.21.26.6, one line per module, zero dependency changes 18 → 5
C. Toolchain + fixable deps B, plus grpc v1.80.0→v1.82.1 and x/image v0.41.0→v0.43.0 18 → 2

Under C, the only survivors repo-wide are the two advisories with no upstream fix, which are exactly the class the chain repo already accepts:

  • GO-2026-5932golang.org/x/crypto@v0.52.0 (Fixed in: N/A)
  • GO-2025-3684github.com/cosmos/evm@v0.6.0 (Fixed in: N/A)

I verified option C builds and passes: go build ./... ✅, go vet ./pkg/... ✅, unit tests ✅.

I deliberately did not include B or C in this PR. Bumping gRPC on a P2P daemon with custom ALTS credentials is a real production risk that deserves its own reviewed, devnet-validated PR — not a silent rider on a CI change. Note also that the chain repo's own govulncheck is red on master today for the same reason (5 vulns, all Fixed in: N/A), so shipping red here is at least consistent with current chain behavior.

Recommendation: merge this PR for the workflows, then land the toolchain bump (B, one line per module, zero dep churn) as an immediate follow-up to take root 18 → 5. Do C separately with devnet validation. Tell me which you want and I'll open it.

Risks

  • lint-pr.yml posts inline review comments on same-repo PRs; fork PRs fall back to reporter: local (log-only) because GITHUB_TOKEN is read-only there. The job still fails on new errors either way.
  • level is deliberately not set on the reviewdog steps: it rewrites every finding's severity rather than defaulting it, so level: warning + fail_level: error would silently neuter the gate.
  • govulncheck job will be red until the toolchain question above is resolved.
  • These workflows do not block merge until added to required status checks in repo settings.

Rollback

Delete the three workflow files and .golangci.yml. The five #nosec comments are inert and can stay or go independently.

Migration / upgrade impact

None. CI-only. No state keys, no ABCI phases, no replay/migration/upgrade-handler behavior touched.

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
@github-advanced-security

Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

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.

2 participants