ci(lint): path-scope gosec G304 to non-test code (#499)#507
Merged
Conversation
7 tasks
trilamsr
added a commit
that referenced
this pull request
Jun 3, 2026
## Summary Bumps the Go toolchain pin from **1.26.3 -> 1.26.4** to pick up the stdlib fix for [GO-2026-5037](https://pkg.go.dev/vuln/GO-2026-5037) (`crypto/x509.HostnameError.Error`), which `govulncheck` flags via `tools/pyspy-lint/main.go:106:14` (reachable through `fmt.Fprintln` on an error path). This was failing the `verify-static` job on every recent PR. ## Root cause `crypto/x509.HostnameError.Error` shipped vulnerable in Go 1.26.3. Patched in Go 1.26.4. There is no in-repo workaround — the call site in `tools/pyspy-lint` is legitimate error formatting; the only correct fix is bumping the toolchain pin. Confirmed locally: ``` $ govulncheck ./tools/pyspy-lint/... # with GOTOOLCHAIN=go1.26.4 No vulnerabilities found. ``` ## Files touched (5) - `go.mod` — `go 1.26.3` -> `go 1.26.4` - `go.work` — `go 1.26.3` -> `go 1.26.4` (+ updated header comments) - `.go-version` — `1.26.3` -> `1.26.4` (drives `actions/setup-go` via `go-version-file`) - `install/kubernetes/tracecore/Dockerfile` — base image bumped to `golang:1.26.4-alpine` with refreshed sha256 digest (`f23e8b22…2a17f`, fetched via `crane digest`) - `docs/SUPPORT-MATRIX.md` — Go-toolchain row updated to `1.26.4` `module/go.mod` is intentionally untouched — it pins `go 1.22.0` to track the OTel collector v0.110.0 OCB-distribution baseline (see existing comment), and the workspace `go` directive (`1.26.4`) remains `>=` the member-module floor (`1.22.0`), so workspace mode is unaffected. ## Test plan - [x] `govulncheck ./tools/pyspy-lint/...` -> No vulnerabilities found - [x] `go build ./...` (root, GOTOOLCHAIN=go1.26.4) -> clean - [x] `go test ./tools/... ./internal/...` -> all green (incl. `tools/pyspy-lint`, the file containing the flagged call site) - [x] `module/` `go test ./...` -> matches `main` (one pre-existing failure in `processor/patterndetectorprocessor` `TestPatternDetector_NegativeFixturesEmitNoVerdicts/synthetic-2026-06-multi-rank-disk-pressure`, reproducible on `main` at the same SHA — unrelated to this bump, out-of-scope here) - [x] `make lint` -> 0 issues - [ ] CI `verify-static` job passes (the gate this PR exists to fix) - [ ] CI `build` / kind install bench builds against new pinned-digest golang base image ## Unblocks Should clear `verify-static` for PRs #504, #505, #507 (and #506 once its own `action.yml` fix lands). ```release-notes chore: bump Go toolchain pin to 1.26.4 to pick up the stdlib fix for GO-2026-5037 (crypto/x509.HostnameError.Error). No behavior change. ``` Signed-off-by: Tri Lam <tree@lumalabs.ai>
Collapses ~22 inline gosec G304 waivers across the test tree into one path-scoped exclusion in .golangci.yml plus a shared jsonschematest.Compile helper for the load-parse-compile dance every detector schema test was open-coding. - .golangci.yml: linters.exclusions.rules waives gosec G304 on _test.go - module/pkg/testutil/jsonschematest: new helper + its own unit test - module/pkg/patterns/*_test.go: 22 schema-compile blocks => one-liner - Strips redundant gosec waivers from all *_test.go (path exclusion now covers them) - Keeps the 4 non-test waivers (runner.go + nccl_fr.go) with their inline rationale; production code still gets gosec G304 coverage Net: -274 / +86 lines. Signed-off-by: Tri Lam <tree@lumalabs.ai>
ba44555 to
a1bb3fa
Compare
4 tasks
trilamsr
added a commit
that referenced
this pull request
Jun 4, 2026
## Summary Centralize the remaining structural `nolint` patterns into `.golangci.yml` path-scoped exclude-rules. Builds on PR #507 (lane A: `_test.go` G304) by covering the next four structural classes that were each generating multiple inline waivers. **Inline `nolint:` waivers: 35 → 13 (-63%).** Untouched: `module/processor/patterndetectorprocessor/` (lane J overlap; deferred to next wave) and 11 point-specific load-bearing keepers. ## Before / after by category | Category | Before | After | Mechanism | | --------------------------------------- | -----: | ----: | ------------------------------------------------------------------------ | | `gosec` (G304 in `_test.go`) | 8 | 0 | already covered by PR #507's existing exclude-rule (redundant comments) | | `gosec` (G204 / G107 integration tests) | 3 | 0 | new path scope `internal/integration/.*_test\.go`, `components/.*/integration_helper_test\.go` | | `gosec` (G304 hermetic replay loader) | 3 | 0 | new path scope `module/pkg/replay/runner\.go` | | `gosec` (G304 jsonschematest helper) | 1 | 0 | new path scope `module/pkg/testutil/jsonschematest/.*\.go` | | `gosec` (G306 / G304 / G404 dev tools) | 5 | 0 | new path scopes for `tools/genfixtures`, `tools/coverage-check`, `tools/failure-inject` | | `goconst` (patterns test vocabularies) | 2 | 0 | new path scope `module/pkg/patterns/.*_test\.go` | | **Centralized subtotal** | **22** | **0** | | | `gocyclo` (kept inline, load-bearing) | 7 | 7 | flat-table dispatch / pattern-match rationale; point-specific | | `gosec` (G103 / G304 prod, load-bearing)| 2 | 2 | `xid_correlation.go` unsafe aliasing; `nccl_fr.go` operator-config path | | `unused` (doc / export refs) | 2 | 2 | `pyspy.go`, `steady_state_linux_test.go` — referenced from doc.go | | `wrapcheck` (stutter rationale) | 1 | 1 | `tools/coverage-check/main.go` os.Open already includes path | | `staticcheck` (SA1019 parity assert) | 1 | 1 | `xid_correlation_test.go` — issue #277 pre-deprecation parity | | **Inline keepers subtotal** | **13** |**13** | | | **TOTAL inline** | **35** |**13** | **-22 (-63%)** | ## Root cause Each inline waiver was treating a *class* of safe-by-design call sites as a point exception. The class-level fixes (path-scoped exclude-rules) move the rationale into one searchable location so future reviewers grep `.golangci.yml` instead of `git log`-archaeologying 22 scattered comments. ## What's NOT in this PR - `module/processor/patterndetectorprocessor/config.go` (2 gocyclo waivers) and `xid_correlation_test.go` (1 staticcheck) — lane J (#497) touches `patterndetectorprocessor/`; deferred to the next wave to avoid merge conflicts. - The 11 load-bearing inline keepers above — each carries a point-specific rationale that does not generalize to a path scope (unsafe aliasing site, weak-RNG-for-chaos-repro, pre-deprecation parity asserts, etc.). ## Test plan - [x] `make lint` green pre-change (baseline: 0 issues). - [x] `make lint` green post-change (0 issues). - [x] Pre-commit hook ran root `golangci-lint` + `module/` `golangci-lint` + `go vet` + `go mod verify` + attribute-namespace-check — all green. - [x] `git diff --stat` confirms no `patterndetectorprocessor/` files touched. Closes #499. ```release-notes ci(lint): centralize 22 inline `nolint` waivers (gosec G204/G107/G304/G306/G404, goconst) into `.golangci.yml` path-scoped exclude-rules. 13 load-bearing point-specific waivers remain inline. ``` --------- Signed-off-by: Tri Lam <tree@lumalabs.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue #499 lane A. Collapses ~22 inline
gosec G304waivers across the test tree into one path-scoped exclusion in.golangci.yml, plus a sharedjsonschematest.Compilehelper for the load-parse-compile dance every detector schema test was open-coding.Root cause.
gosec G304flags variable-pathos.ReadFile. Detector schema tests by construction usefilepath.Join("testdata", "<pattern>_verdict.schema.json")— a variable path — and each carried a per-site//nolint:gosecrationale comment. The linter rule does not fit test code; the right scope is non-test code. Path-scoped exclusion + helper extraction is the structural fix, not a workaround.Changes
.golangci.yml:linters.exclusions.ruleswaivesgosec G304on_test.go, with inline rationale referencing the inline waivers it replaces.module/pkg/testutil/jsonschematest/: new helper package + its own unit test (TestCompile_ValidSchema,TestCompile_FailsOnMissingFile) + fixture.module/pkg/patterns/*_test.go(12 files): 22 schema-compile blocks reduced to one-liners viajsonschematest.Compile(t, …).//nolint:gosecdirectives stripped from*_test.goacross the module (path exclusion now covers them).module/pkg/replay/runner.go:80,95,108(hermetic fixture loader) andmodule/receiver/ncclfrreceiver/nccl_fr.go:225(operator-configured dump dir).Net: -274 / +86 lines.
Test plan
grep -rn 'nolint:gosec' module/ --include='*_test.go' | wc -lreturns 0cd module && go test ./pkg/... ./processor/rankjoinprocessor/... ./receiver/... ./sdk/...greencd module && go test ./pkg/testutil/jsonschematest/...green (helper has unit test)make lintgreen (0 issues)make lint-module-fullgreen (0 issues)Note:
module/processor/patterndetectorprocessor.TestPatternDetector_NegativeFixturesEmitNoVerdicts/synthetic-2026-06-multi-rank-disk-pressurefails on this branch AND onmainat de5b201 — pre-existing, unrelated to this PR (verified viagit stash+ re-run on main HEAD).