Research digest: Go static-analysis audit (2026-05-30)¶
Scope¶
Static-analysis sweep across all fork-added Go code under cmd/ + pkg/ on master tip bbcaa8d127, using the three industry-standard analysers beyond the default go vet:
nilness(golang.org/x/tools/go/analysis/passes/nilness/cmd/nilness) — possible nil-pointer dereference detection.staticcheck(honnef.co/go/tools) — SA-prefixed correctness rules, S-prefixed simplification rules, U-prefixed unused-symbol checks.gosec(github.com/securego/gosec/v2) — security-focused linter (CWE-mapped rules, e.g. G118 / G122 / G304).
Companion to PRs #330 (cmd test coverage), #347 (pkg test coverage), and
341 (Go dependency audit). This sweep deliberately avoids those files¶
and avoids any go.mod / go.sum change.
Findings (pre-fix)¶
| Tool | Findings | Severity breakdown |
|---|---|---|
| nilness | 0 | (clean) |
| staticcheck | 13 | SA1019 ×4, U1000 ×8, S1009 ×1 |
| gosec -high | 3 | G118 ×2, G122 ×1 |
SA1019 (deprecated symbol) — 4 sites¶
prometheus.NewGoCollector() and prometheus.NewProcessCollector() have been deprecated in favour of the prometheus/collectors subpackage since client_golang v1.12 (mid-2022). Affected files:
cmd/vmafx-server/main.go:98-99cmd/vmafx-controller/main.go:98-99
The replacement subpackage is already pulled in transitively; no go.mod change required.
U1000 (unused) — 8 sites¶
cmd/vmafx-node/main_test.go— 7 symbols (mockControllerstruct,newMockController,RegisterNode,Heartbeat,PullWork,ReportResult,startMockController). The file's own header notes that the controller-registration path was replaced by the ffmpeg-probe / gRPC server model; the scaffolding was orphaned during that refactor.pkg/bisect/bisect_test.go—mockScoreFunchelper. All currentTestBisect_*tests inline theirScoreFuncvia closures overlinearVMAF, so the helper had no remaining callers.
S1009 (redundant nil guard) — 1 site¶
pkg/ai/infer_test.go:124 — len() on a nil slice is zero by spec, so the models != nil && len(models) != 0 guard simplifies to len(models) != 0.
G118 (goroutine uses Background ctx) — 2 sites (false positives)¶
cmd/vmafx-server/http_server.go:182 and cmd/vmafx-controller/http_server.go:183 both implement the canonical net/http graceful-shutdown pattern: a goroutine that waits on the parent ctx.Done(), then builds a fresh timeout context to bound the in-flight-request drain. Propagating the request-scoped ctx to httpSrv.Shutdown would cancel it immediately — the very condition the goroutine just observed — defeating the entire drain. Annotated with #nosec G118 + an explanatory comment.
G122 (filepath.Walk TOCTOU) — 1 site¶
cmd/vmafx-mcp/impl.go:1034 reads .c files under the repo's own core/src/feature/ (or legacy libvmaf/src/feature/) tree to enumerate extractors for the MCP list_extractors tool. The input root is trusted (repository-owned source), so the symlink-TOCTOU risk is minimal — but we still added a filepath.Abs + strings.HasPrefix containment check defending against the (hypothetical) case where the trusted root itself contains a hostile symlink leaving the tree. Annotated with #nosec G122 G304 on the resulting os.ReadFile.
Decisions¶
- Do not upgrade
go.mod— PR #341 owns dependency motion. Migrating tocollectors.*is possible without a version bump because the subpackage is part of the same already-pinned module. - Delete dead test scaffolding, do not "mark intentionally unused" via
_ = symbol— the symbols correspond to obsolete code paths (per the in-file comments); deleting them is the correctness fix. - Annotate G118 / G122 as false positives, do not refactor — restructuring graceful shutdown to use the cancelled parent context would introduce a real bug. The
#nosecform preserves the lint gate's signal-to-noise for future contributors. - No upstream-port impact — every modified file is fork-original (
cmd/vmafx-*,pkg/*); no rebase delta against Netflix master.
Post-fix scoreboard¶
| Tool | Findings |
|---|---|
| nilness | 0 |
| staticcheck | 0 |
| gosec -high | 0 |
Verification¶
nilness ./... # silent
staticcheck ./... # silent
gosec -severity high ./... # Issues: 0
go build ./... # clean
go test -race ./cmd/vmafx-{server,controller,mcp,node}/... \
./pkg/{ai,bisect}/... # all pass
The cmd/vmafx-operator/internal/controller package fails on this host because /usr/local/kubebuilder/bin/etcd is not installed — pre-existing environmental gap, unrelated to this sweep.
References¶
req— user direction (2026-05-30): "run Go static analysis on fork-added Go code incmd/+pkg/and flag and fix real findings; avoid PR-overlap with #330 / #347 / #341".- ADR-0108 — deep-dive deliverables rule (this digest discharges item 1).
- ADR-0703 — vmafx-server scope (covers the prometheus metrics surface that the SA1019 fix touches).
- ADR-0713 — vmafx-node scope (covers the test file whose dead mock was removed).