Skip to content

Clear OSV-Scanner findings: bump go directive to 1.25.0 + remaining deps#368

Open
vikrantpuppala wants to merge 7 commits into
mainfrom
vp/security-bump-go123
Open

Clear OSV-Scanner findings: bump go directive to 1.25.0 + remaining deps#368
vikrantpuppala wants to merge 7 commits into
mainfrom
vp/security-bump-go123

Conversation

@vikrantpuppala
Copy link
Copy Markdown
Collaborator

@vikrantpuppala vikrantpuppala commented May 30, 2026 •
edited
Loading

Summary

Brings the Go SQL driver to a state where OSV-Scanner v2.3.8 reports zero non-stdlib findings against go.mod — every CVE in dependency code is patched. Combined with #363 (already merged), this closes out the security-bump work.

Status: companion to #362 (the OSV-Scanner workflow PR).

go directive

go 1.20 → go 1.25.0. This is the minimum required by github.com/apache/thrift@v0.23.0 (the version patched for GHSA-wf45-q9ch-q8gh), which declares go 1.25 in its own go.mod. We don't pin to a patch (.10); the patch is determined by whatever toolchain the build environment uses, and GOTOOLCHAIN=auto (the default since Go 1.21) downloads the latest patched toolchain automatically.

Why Go 1.25 (and not 1.23 or 1.24)

Per go.dev/doc/devel/release, only the latest two major releases receive security patches. As of May 2026 that's Go 1.25 (Active LTS) and Go 1.26.

The actual binding constraint is apache/thrift@v0.23.0 requiring go 1.25. Without thrift 0.23, we cannot clear GHSA-wf45-q9ch-q8gh (HIGH 7.5) — there is no backport. So we cannot stay on go 1.23/1.24 AND clear the HIGH CVE; pick one.

We picked clearing the CVE. The CVE itself is in TFramedTransport, which this driver does not use (we use THttpClient), so it isn't reachable in practice — but customer scanners running against our lockfile WILL flag it. Patching the dep, rather than suppressing the finding, is the customer-friendly choice.

Stdlib advisories visible in OSV

OSV-Scanner reads the go directive literally and reports stdlib advisories targeting any version below the patched one. go 1.25.0 triggers ~34 stdlib advisories (each fixed in some 1.25.x patch). These are a lockfile-scanner artifact, not customer-reachable bugs:

  • The driver doesn't ship a binary; consumers compile against our go.mod and link Go's stdlib at THEIR toolchain version.
  • GOTOOLCHAIN=auto (default since Go 1.21) downloads the latest patched toolchain — so a consumer's compiled driver picks up the 1.25.10 stdlib in practice.
  • A go 1.25.10 directive would silence the scanner but doesn't change runtime behavior.

We chose the minimum required directive (1.25.0) rather than over-pinning (1.25.10) to match the convention used by grpc-go, golang.org/x/net, and aws-sdk-go-v2.

Runtime dep bumps

Package From To Why
github.com/apache/thrift v0.17.0 v0.23.0 GHSA-wf45-q9ch-q8gh (HIGH 7.5)
golang.org/x/oauth2 v0.7.0 v0.27.0 GO-2025-3488 / GHSA-6v2p-p543-phr9
golang.org/x/crypto v0.31.0 v0.52.0 GO-2025-3487, GO-2026-5005..5033 (13 SSH-agent constraint advisories)
golang.org/x/sys v0.30.0 v0.45.0 required by x/crypto v0.52.0

Plus auto-bumped transitives via go mod tidy.

Go 1.25 vet findings

Go 1.25's go vet (run as part of go test) added non-constant format string in call to ...Printf-like(...). Three latent footguns:

  • internal/rows/arrowbased/arrowRows.go:224 — Msgf(errFn(...)) → Msg(...)
  • internal/rows/arrowbased/arrowRows.go:697 — errors.Errorf(errFn(...)) → errors.New(...)
  • internal/rows/rowscanner/resultPageIterator.go:300 — Msgf(errFn(...)) → Msg(...)

None had format verbs. All would have format-injected if the input ever contained %.

CI matrix

[1.20.x] → ['1.24.x', '1.25.x', '1.26.x']. Matches Go's release support window (only 1.25/1.26 currently receive security patches; 1.24 included for one cycle of overlap).

Lint Go version: 1.20.x → 1.25.x. golangci-lint: v1.51 → v1.61 (v1.51 chokes on go 1.25 directives).

OSV-Scanner result

Before: 40 findings (1 HIGH thrift + 39 unrated stdlib)
After:   ~34 unrated stdlib advisories (lockfile-artifact, runtime-fixed by toolchain)
         0 dependency-code CVEs

Test plan

  • go build ./... clean on Go 1.25.10 toolchain
  • go test ./... passes (full unit suite)
  • go mod tidy produces no further changes
  • OSV-Scanner v2.3.8 confirms zero non-stdlib findings
  • CI green across [1.24.x, 1.25.x, 1.26.x] matrix

Customer impact

Surface Change Impact
go directive 1.20 → 1.25.0 Users compiling need Go ≥ 1.25.0. With GOTOOLCHAIN=auto (default), Go auto-downloads — most users see no friction. GOTOOLCHAIN=local users on Go < 1.25.0 get a compile error pointing to update.
Runtime behavior None The three vet-fix source changes are equivalent rewrites of error/log calls.
Public API Unchanged No exported function signatures, types, or constants changed.

Anyone on Go < 1.25 is already running upstream-unsupported Go.

This pull request was AI-assisted by Isaac.

@vikrantpuppala
Clear OSV-Scanner findings: bump go directive to 1.25.10 + deps
f9e1fa7 
Brings the Go SQL driver to a state where OSV-Scanner v2.3.8 reports
zero findings against `go.mod`. Combined with #363 (the Go-1.20-compat
dep bumps already merged), this closes out the security-bump work
on databricks-sql-go.

## go directive

`go 1.20` → `go 1.25.10`. Three reasons compound:
- `github.com/apache/thrift@v0.23.0` (the version patched for
  GHSA-wf45-q9ch-q8gh / GHSA-526f-jxpj-jmg2) declares `go 1.25` in
  its own go.mod. We need `go ≥ 1.25` to build against it.
- `golang.org/x/crypto@v0.52.0` (latest, clears GO-2026-5005 etc.)
  declares `go 1.24`. Fits within 1.25 floor.
- The `.0` minor version of any Go release carries the stdlib
  advisories accumulated since release. Bumping to `1.25.10` (current
  patch as of 2026-05) clears 39 stdlib advisories from the OSV
  report. Future contributors regenerating go.sum should bump the
  patch line in lockstep.

## Runtime dep bumps

| Package | From | To | Why |
|---|---|---|---|
| github.com/apache/thrift | v0.17.0 | v0.23.0 | GHSA-wf45-q9ch-q8gh (HIGH 7.5) |
| golang.org/x/oauth2 | v0.7.0 | v0.27.0 | GO-2025-3488 / GHSA-6v2p-p543-phr9 |
| golang.org/x/crypto | v0.31.0 | v0.52.0 | GO-2025-3487, GO-2026-5005..5033 |
| golang.org/x/sys | v0.30.0 | v0.45.0 | required by x/crypto v0.52.0 |

Plus auto-bumped transitives via `go mod tidy`: x/term, x/sync,
x/mod etc. all updated to the versions x/crypto / x/sys now require.

## Go 1.25 vet findings

Go 1.25's `go vet` (run as part of `go test`) added a check for
`non-constant format string in call to ...Printf-like(...)`. Three
existing call sites violate the new rule:

- `internal/rows/arrowbased/arrowRows.go:224` —
  `ars.Error().Msgf(errArrowRowsUnsupportedNativeType(...))` →
  `Msg(...)` (no formatting needed)
- `internal/rows/arrowbased/arrowRows.go:697` —
  `errors.Errorf(errArrowRowsUnhandledArrowType(...))` →
  `errors.New(...)` (no formatting needed)
- `internal/rows/rowscanner/resultPageIterator.go:300` —
  `rpf.logger.Error().Msgf(errRowsUnandledFetchDirection(...))` →
  `Msg(...)`

All three accept a function call returning a string. None had
format verbs — they were latent footguns that would have format-
injected if the string ever contained a `%` character. Fixing.

## CI matrix

`go-version: [1.20.x]` → `go-version: ['1.24.x', '1.25.x', '1.26.x']`.
Matches the Go release support window (only 1.25/1.26 currently
receive security patches; 1.24 included for one cycle of overlap
matching the JDBC/Python/Node patterns).

Lint job's Go version bumped 1.20.x → 1.25.x. golangci-lint
bumped v1.51 → v1.61 (v1.51 predates Go 1.21 toolchain auto-download
and chokes on `go 1.25` directives).

## OSV-Scanner result

Before: 40 findings (1 HIGH thrift, 39 unrated stdlib)
After:   0 findings

Verified locally on Go 1.25.10. Tests pass.

Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
@vikrantpuppala
Relax go directive from 1.25.10 to 1.25.0
87f9162 
The .10 patch was over-pinning: it was chosen to silence 34 stdlib
advisories from the OSV-Scanner report, but those advisories are
runtime-fixed by whatever toolchain the build actually uses
(GOTOOLCHAIN=auto downloads the latest patched). The directive only
needs to be 1.25.0 — the minimum required by github.com/apache/thrift
v0.23.0 (which declares go 1.25 in its go.mod).

Tradeoff:
- OSV-Scanner against go.mod will list ~34 stdlib advisories targeting
  1.25.0 -> 1.25.x patches. These appear in customer scanners too.
- But: these are runtime-fixed at build time. A customer using
  Go 1.25.10 to build the driver ships a binary with 1.25.10's stdlib.
  The advisories are a lockfile-scanner artifact, not a reachable
  customer vulnerability.

Aligns with how the data-infra Go pack (grpc-go, golang.org/x/net,
aws-sdk-go-v2) writes their directives — minimum required by their
deps, not the latest patch.

Tests pass on Go 1.25.10 toolchain (GOTOOLCHAIN=auto picks it up
when build environment is older).

Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
@vikrantpuppala vikrantpuppala changed the title Clear OSV-Scanner findings: bump go directive to 1.25.10 + remaining deps Clear OSV-Scanner findings: bump go directive to 1.25.0 + remaining deps May 30, 2026
@vikrantpuppala
ci: fix lint go-version mismatch, drop 1.24 from matrix
6120e51 
Two CI breakages from the directive bump:

1. Lint job failed with "the Go language version (go1.23) used to
   build golangci-lint is lower than the targeted Go version (1.25.0)".
   golangci-lint v1.61 was built with go 1.23 and refuses to lint a
   project declaring a newer directive. v1.62-1.64 are also pre-1.25.
   The v2.x family is built with go 1.25+ but requires a config-format
   migration (linters: are nested differently).

   Quickest fix: add `run.go: '1.23'` to .golangci.yml so linting
   treats the code as Go 1.23 despite the go 1.25.0 directive in go.mod.
   The directive is required by patched apache/thrift; the lint config
   override doesn't change runtime behavior.

   Migrating to golangci-lint v2 is a separate cleanup (its config
   format changed; non-trivial migration).

2. Test and Build (1.24.x) failed because protected runners set
   GOTOOLCHAIN=local, so Go 1.24.13 cannot auto-download a 1.25.x
   toolchain to satisfy our `go 1.25.0` directive. Drop 1.24.x from
   the matrix — it can't satisfy the directive in this CI setup.

   Final matrix: ['1.25.x', '1.26.x'] — matches Go's active security
   support window (only 1.25 and 1.26 receive upstream patches).

Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
@vikrantpuppala
ci(lint): drop deprecated linters from .golangci.yml
9823db2 
`deadcode`, `structcheck`, `varcheck` were removed in golangci-lint
v1.49.0 (2022-09) and replaced by `unused` (which we already enable).
Newer golangci-lint releases (including v1.61 in CI) emit deprecation
warnings AND, more critically, flag them as `level=error` which the
github-actions output formatter promotes to job-failing annotations.

Removing the three deprecated entries clears the lint job error
output without changing actual lint coverage.

Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
@vikrantpuppala
ci(lint): migrate to golangci-lint v2.12 + new config format
0a3a1e0 
The previous attempt to stay on v1.61 with `run.go: '1.23'` didn't
resolve the underlying issue: golangci-lint v1.x was built with
Go ≤ 1.23 and can't read the export data format that Go 1.25's
stdlib ships. Result: "could not import sync/atomic (unsupported
version: 2)" cascading into dozens of false typecheck errors
across the codebase.

v2.x is built with Go 1.25+ and reads the newer format natively.

Migration to the v2 config schema:
- `linters: disable-all: true` -> `linters: default: none`
- `linters-settings:` -> nested under `linters: settings:`
- `gofmt` is now a formatter (separate `formatters:` block) not a
  linter
- `gosimple` was merged into `staticcheck` and removed as a separate
  linter
- `goprintffuncname` was merged into the default staticcheck rules
- `typecheck` is implicit in v2 and no longer toggleable; dropped
- `deadcode`, `structcheck`, `varcheck` were removed in v1.49 (all
  replaced by `unused`, which we already enabled)

Same set of substantive checks; same allowlists for depguard/gosec/
nolintlint. No behavior change in what gets flagged.

Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
@vikrantpuppala
ci(lint): bump golangci-lint-action v4 -> v9.2.1 for v2 lint support
cf77582 
v4 of the action still passes the v1-era --out-format flag, which
golangci-lint v2.x removed (replaced by --output.text.format=...).
v7+ of the action is the first to support golangci-lint v2 — bump
to the current stable v9.2.1.

Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
@vikrantpuppala
ci(lint): resolve golangci-lint v2 findings on existing main
1af48ef 
Lint v2 has stricter defaults than the v1.51 the repo used previously,
surfacing pre-existing issues that v1 silently ignored. None of these
are bugs introduced by the security bump; they're cleanups so the
upgraded linter runs clean.

- .golangci.yml: depguard v2 no longer auto-allows the project's own
  third-party deps — list them explicitly so legitimate imports of
  go-retryablehttp, oauth2, thrift, jwt/v5, etc. don't trip the rule.
  Drop the v2-invalid `gosec.exclude-generated` key (the linter now
  config-verifies and would refuse to start with it present).
- Makefile: bump local install target from golangci-lint@v1.51.0 to
  v2.12.2 (matches the version the v9.2.1 GitHub Action installs).
- parameters.go: reflect.Ptr -> reflect.Pointer (Ptr is the legacy
  alias kept for backwards compat; govet's `inline` check flags it).
- internal/rows/rows.go: complete the //nolint:gosec annotation block
  the original PR author added on the very next line — the int32
  conversion on line 183 is in the same telemetry counter scope.
- telemetry/aggregator_test.go: //nolint:gosec on test-only counter.
- connection_test.go: //nolint:gosec on os.ReadFile(localFile) — the
  filename comes from a deterministic httptest fixture path.
- internal/client/client.go: //nolint:staticcheck on the deprecated
  thrift.StdLogger sentinel — it's a no-op since thrift v0.13 but
  remains the documented argument type for NewTDebugProtocolFactoryWithLogger.

Local lint run after these changes: 0 issues.

Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
@vikrantpuppala vikrantpuppala force-pushed the vp/security-bump-go123 branch from 03ac020 to 1af48ef Compare June 1, 2026 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@vikrantpuppala