Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 75 additions & 14 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -147,25 +147,86 @@ linters:

exclusions:
# Path-scoped exclusions consolidate repeated per-site nolint
# waivers into one place so reviewers grep the config, not 20
# scattered comments. Issue #499 lane A: gosec G304 (potential file
# inclusion via variable) is the canonical noise in test files —
# every detector test reads "testdata/<x>.schema.json" through
# filepath.Join, which is by definition a variable path. The path
# is checked into the repo, the working dir is set by `go test`,
# and the file is not operator-controlled at runtime. Waiving G304
# on `_test.go` collapses ~22 inline waivers across
# module/pkg/patterns/** and the integration / SDK test trees with
# no loss of coverage on production code (gosec still runs G304 on
# non-test files; see module/pkg/replay/runner.go and
# module/receiver/ncclfrreceiver/nccl_fr.go which keep their inline
# rationale comments because they read paths derived from operator
# config).
# waivers into one place so reviewers grep the config, not 20+
# scattered comments. Issue #499: progressive centralization waves.
#
# G304 in `_test.go` — every detector test reads
# "testdata/<x>.schema.json" through filepath.Join, which is by
# definition a variable path. The path is checked into the repo,
# the working dir is set by `go test`, and the file is not
# operator-controlled at runtime.
#
# Lane K (this PR): extend to remaining structural patterns —
# subprocess/network exec in integration tests, hermetic loaders
# in module/pkg/replay, fixture-generation tools, and pattern-
# detector test vocabularies. After this wave, the only inline
# nolint waivers left carry truly point-specific rationale (unsafe
# aliasing, weak-rng for chaos injection, pre-deprecation parity
# asserts) that don't generalize to a path scope.
rules:
# Lane A: G304 (potential file inclusion via variable) on test
# files — every _test.go in the tree reads fixture/testdata
# paths via variables.
- linters:
- gosec
path: _test\.go
text: "G304"
# Integration tests legitimately invoke subprocesses (G204) and
# hit localhost endpoints (G107) — they orchestrate built
# binaries against ephemeral servers under t.TempDir control.
- linters:
- gosec
path: internal/integration/.*_test\.go
text: "G204|G107"
# Component integration helpers spawn child processes
# (e.g. py-spy) under test-controlled args.
- linters:
- gosec
path: components/.*/integration_helper_test\.go
text: "G204"
# Replay corpus loader is hermetic by design: the fixture dir
# passed in is test-controlled and the loader never sees
# operator-derived paths. Production callers go through the
# receiver's config-validated path, which carries its own
# inline G304 waiver pinpointing the operator boundary.
- linters:
- gosec
path: module/pkg/replay/runner\.go
text: "G304"
# Schema-test helper only reachable from tests; schemaPath is a
# relative path baked into the test corpus.
- linters:
- gosec
path: module/pkg/testutil/jsonschematest/.*\.go
text: "G304"
# genfixtures is a dev tool that writes 0644 JSON/Pkl fixtures
# for checkin under tools/genfixtures/testdata. The output is
# public test data by design — G306 (file perms) does not apply.
- linters:
- gosec
path: tools/genfixtures/.*\.go
text: "G306"
# coverage-check is a developer-only CLI: it ingests a coverage
# profile path supplied by the operator running the tool. Not a
# server; G304 is not the relevant threat model.
- linters:
- gosec
path: tools/coverage-check/.*\.go
text: "G304"
# failure-inject is a chaos-testing CLI that opens operator-
# supplied target paths with 0o600 perms. G304 on the open and
# G404 on the deterministic-seed RNG (chaos repro needs
# reproducibility, not crypto) are intentional.
- linters:
- gosec
path: tools/failure-inject/.*\.go
text: "G304|G404"
# Pattern-detector test corpora reuse the same handful of
# vocabulary literals ("Xid 79", "throttle", etc.) across many
# cases by design — see the "input vocabulary" docstrings.
- linters:
- goconst
path: module/pkg/patterns/.*_test\.go

issues:
max-issues-per-linter: 0
Expand Down
2 changes: 1 addition & 1 deletion components/receivers/pyspy/integration_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ except KeyboardInterrupt:

// The python binary is resolved via exec.LookPath; the script
// is a constant inline literal. No external untrusted input.
cmd := exec.CommandContext(ctx, python, "-u", "-c", script) //nolint:gosec // G204
cmd := exec.CommandContext(ctx, python, "-u", "-c", script)
cmd.Env = append(os.Environ(),
"PYTHONPATH_TRACECORE="+pythonPkgDir,
"PYSPY_DIR="+dir,
Expand Down
4 changes: 2 additions & 2 deletions internal/integration/ocb_scrape_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ service:
t.Fatalf("write config: %v", err)
}

cmd := exec.Command(bin, "--config="+cfgPath) //nolint:gosec // both args produced under test control (findBuiltBinary + t.TempDir)
cmd := exec.Command(bin, "--config="+cfgPath)
// Capture combined output so a startup failure surfaces in the
// test log instead of vanishing into the void.
cmd.Stdout = io.Discard
Expand Down Expand Up @@ -213,7 +213,7 @@ func waitForMetrics(t *testing.T, url string, required []string, budget time.Dur
var lastErr error
var lastBody string
for time.Now().Before(deadline) {
resp, err := http.Get(url) //nolint:gosec // localhost ephemeral
resp, err := http.Get(url)
switch {
case err != nil:
lastErr = err
Expand Down
5 changes: 2 additions & 3 deletions module/pkg/patterns/silent_data_corruption_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -604,9 +604,8 @@ func TestSDCVerdict_SchemaConformance(t *testing.T) {
//
// The repeated `rejected` / `maybe` / `full` literals are the test's
// input vocabulary — extracting them to package-level constants would
// obscure which row mutates which schema axis.
//
//nolint:goconst // See "input vocabulary" note above.
// obscure which row mutates which schema axis. goconst is path-scoped
// off for module/pkg/patterns/*_test.go in .golangci.yml.
func TestSDCVerdict_SchemaRejectsDrift(t *testing.T) {
t.Parallel()

Expand Down
5 changes: 2 additions & 3 deletions module/pkg/patterns/thermal_throttle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,8 @@ func TestThermalThrottleVerdict_SchemaConformance(t *testing.T) {
//
// The repeated `full` literals are the test's input vocabulary —
// extracting them to a package-level constant would obscure which row
// mutates which schema axis.
//
//nolint:goconst // See "input vocabulary" note above.
// mutates which schema axis. goconst is path-scoped off for
// module/pkg/patterns/*_test.go in .golangci.yml.
func TestThermalThrottleVerdict_SchemaRejectsDrift(t *testing.T) {
t.Parallel()

Expand Down
6 changes: 3 additions & 3 deletions module/pkg/replay/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func LoadFixture(dir string) (Fixture, error) {
name := filepath.Base(dir)
f := Fixture{Name: name}

manBytes, err := os.ReadFile(filepath.Join(dir, manifestFile)) //nolint:gosec // fixture dir is test-controlled; replay corpus loader is hermetic by design
manBytes, err := os.ReadFile(filepath.Join(dir, manifestFile))
if err != nil {
return f, fmt.Errorf("replay.LoadFixture(%s): %s: %w", name, manifestFile, err)
}
Expand All @@ -92,7 +92,7 @@ func LoadFixture(dir string) (Fixture, error) {
return f, fmt.Errorf("replay.LoadFixture(%s): %w", name, err)
}

golden, err := os.ReadFile(filepath.Join(dir, goldenFile)) //nolint:gosec // fixture dir is test-controlled; replay corpus loader is hermetic by design
golden, err := os.ReadFile(filepath.Join(dir, goldenFile))
if err != nil {
return f, fmt.Errorf("replay.LoadFixture(%s): %s: %w", name, goldenFile, err)
}
Expand All @@ -105,7 +105,7 @@ func LoadFixture(dir string) (Fixture, error) {
// Missing-file returns nil; malformed-content returns the unmarshal
// error so silent fixture drift is impossible.
func readJSONOptional(path string, target any) error {
bs, err := os.ReadFile(path) //nolint:gosec // path comes from the fixture-dir join above; replay loader is hermetic
bs, err := os.ReadFile(path)
if os.IsNotExist(err) {
return nil
}
Expand Down
11 changes: 5 additions & 6 deletions module/pkg/testutil/jsonschematest/jsonschematest.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,11 @@ import (
func Compile(t testing.TB, schemaPath string) *jsonschema.Schema {
t.Helper()

// G304: the path is test-local. The package-level gosec G304
// exclusion for *_test.go in .golangci.yml covers callers; this
// helper is non-test code but is only reachable from tests, so
// we accept one waiver here in exchange for deleting 22 inline
// waivers across the patterns suite.
schemaBytes, err := os.ReadFile(schemaPath) //nolint:gosec // schemaPath is a test-controlled relative path; helper is reached only from tests
// G304: the path is test-local. Covered by the
// module/pkg/testutil/jsonschematest path-scoped gosec G304
// exclusion in .golangci.yml — this helper is non-test code but
// is only reachable from tests.
schemaBytes, err := os.ReadFile(schemaPath)
if err != nil {
t.Fatalf("jsonschematest.Compile: read %s: %v", schemaPath, err)
return nil
Expand Down
2 changes: 1 addition & 1 deletion tools/coverage-check/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func main() {
}

func parseProfile(path string) (map[string]*pkgCov, error) {
f, err := os.Open(path) //nolint:gosec // operator-supplied profile path; this is a dev tool, not a server.
f, err := os.Open(path)
if err != nil {
return nil, err //nolint:wrapcheck // os.Open errors already include the path; wrapping would stutter.
}
Expand Down
2 changes: 1 addition & 1 deletion tools/failure-inject/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func openOutput(path string, stdout io.Writer) (io.Writer, func(), error) {
// OpenFile and keep the operator-trust assumption documented in
// the README.
flags := os.O_WRONLY | os.O_CREATE | os.O_TRUNC | openNoFollow
f, err := os.OpenFile(path, flags, 0o600) //nolint:gosec
f, err := os.OpenFile(path, flags, 0o600)
if err != nil {
return nil, nil, fmt.Errorf("open --out: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion tools/failure-inject/main_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestRun_OutRefusesSymlink(t *testing.T) {
"open should fail with ELOOP, proving O_NOFOLLOW is in the flag set; stderr=%q", stderr.String())

// The symlink target must be untouched (no truncation).
got, err := os.ReadFile(target) //nolint:gosec // path is t.TempDir-scoped
got, err := os.ReadFile(target)
require.NoError(t, err)
require.Equal(t, "sentinel\n", string(got),
"symlink target was clobbered — O_NOFOLLOW guard is broken")
Expand Down
2 changes: 1 addition & 1 deletion tools/failure-inject/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func TestRun_OutFlagWritesFile(t *testing.T) {
require.Equal(t, exitOK, rc, "stderr=%q", stderr.String())
require.Zero(t, stdout.Len(), "with --out, stdout must stay empty")

data, err := os.ReadFile(outPath) //nolint:gosec // path is t.TempDir-scoped
data, err := os.ReadFile(outPath)
require.NoError(t, err)
require.Contains(t, string(data), "reason: Evicted")
}
Expand Down
2 changes: 1 addition & 1 deletion tools/failure-inject/podevict/podevict.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func Run(ctx context.Context, opts Options, w io.Writer) error {

// Same gosec G404 rationale as xidgen: deterministic PCG IS the
// contract; this is not a key-generation surface.
rng := rand.New(rand.NewPCG(opts.Seed, opts.Seed^0xBF58476D1CE4E5B9)) //nolint:gosec
rng := rand.New(rand.NewPCG(opts.Seed, opts.Seed^0xBF58476D1CE4E5B9))
uid := newUID(rng)
podName := fmt.Sprintf("training-job-%04x", rng.Uint64N(0x10000))
nodeName := fmt.Sprintf("gpu-node-%04x", rng.Uint64N(0x10000))
Expand Down
4 changes: 2 additions & 2 deletions tools/genfixtures/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func run(outDir string) error {
return err
}
pklPath := filepath.Join(outDir, f.Slug+".pkl")
if err := os.WriteFile(pklPath, bs, 0o644); err != nil { //nolint:gosec // fixture, not sensitive
if err := os.WriteFile(pklPath, bs, 0o644); err != nil {
return fmt.Errorf("write %s: %w", pklPath, err)
}

Expand All @@ -49,7 +49,7 @@ func run(outDir string) error {
}
raw = append(raw, '\n')
jsonPath := filepath.Join(outDir, f.Slug+".golden.json")
if err := os.WriteFile(jsonPath, raw, 0o644); err != nil { //nolint:gosec // fixture, not sensitive
if err := os.WriteFile(jsonPath, raw, 0o644); err != nil {
return fmt.Errorf("write %s: %w", jsonPath, err)
}
}
Expand Down
12 changes: 6 additions & 6 deletions tools/genfixtures/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ func TestRun_HappyPath(t *testing.T) {
pklPath := filepath.Join(outDir, f.Slug+".pkl")
jsonPath := filepath.Join(outDir, f.Slug+".golden.json")

gotPkl, err := os.ReadFile(pklPath) //nolint:gosec // test-controlled path
gotPkl, err := os.ReadFile(pklPath)
require.NoError(t, err, "missing pkl output")
wantPkl, err := f.Bytes()
require.NoError(t, err)
require.Equal(t, wantPkl, gotPkl, "pkl bytes mismatch for %s", f.Slug)

gotJSON, err := os.ReadFile(jsonPath) //nolint:gosec // test-controlled path
gotJSON, err := os.ReadFile(jsonPath)
require.NoError(t, err, "missing golden json output")

// Output must be valid JSON with the expected top-level shape.
Expand Down Expand Up @@ -92,9 +92,9 @@ func TestRun_Determinism(t *testing.T) {
for _, f := range frparser.BuildFixtures() {
for _, suffix := range []string{".pkl", ".golden.json"} {
name := f.Slug + suffix
aBytes, err := os.ReadFile(filepath.Join(a, name)) //nolint:gosec // test-controlled path
aBytes, err := os.ReadFile(filepath.Join(a, name))
require.NoError(t, err)
bBytes, err := os.ReadFile(filepath.Join(b, name)) //nolint:gosec // test-controlled path
bBytes, err := os.ReadFile(filepath.Join(b, name))
require.NoError(t, err)
require.Equal(t, aBytes, bBytes, "non-deterministic output for %s", name)
}
Expand All @@ -117,9 +117,9 @@ func TestRun_MatchesCommittedGoldens(t *testing.T) {
for _, f := range frparser.BuildFixtures() {
for _, suffix := range []string{".pkl", ".golden.json"} {
name := f.Slug + suffix
fresh, err := os.ReadFile(filepath.Join(outDir, name)) //nolint:gosec // test-controlled path
fresh, err := os.ReadFile(filepath.Join(outDir, name))
require.NoError(t, err)
gold, err := os.ReadFile(filepath.Join(committed, name)) //nolint:gosec // test-controlled path
gold, err := os.ReadFile(filepath.Join(committed, name))
require.NoError(t, err, "committed golden missing: %s", name)
require.Equal(t, gold, fresh,
"committed %s differs from a fresh run(); run `make generate-fixtures`", name)
Expand Down
Loading