Skip to content

fix(deps): bump Go modules with critical and high CVEs - #15

Merged
dmakarenko merged 1 commit into
goodnotesfrom
bump-go-deps-cve-20261001
Oct 1, 2026
Merged

dmakarenko merged 1 commit into
goodnotesfrom
bump-go-deps-cve-20261001

Conversation

@dmakarenko

@dmakarenko dmakarenko commented Oct 1, 2026 •

Copy link
Copy Markdown

Bumps 14 Go modules to versions with the CVE fixes (pgx v5.10.0, grpc v1.84.0, golang.org/x/*, go-jose, jwt, otel/sdk), raises the go directive to 1.26.0, moves RunTestSMTP to internal/testhelpers so that dockertest, docker/cli, and runc leave the release binary, and makes cve-scan.yaml run on goodnotes without the broken Anchore steps.

Trivy on v1.3.0-gn-v10 found 3 CRITICAL and 36 HIGH findings in usr/bin/kratos, and each fork image from v8 has them; pgx stays at v5.10.0 because v5.11.0 reads a DSN differently, and make sdk sets GODEBUG=gotypesalias=0 because go-swagger v0.31.0 panics on type aliases (this holds until Go 1.27).

Tested: Trivy on the make docker image reports 0 findings, locally and in the scanners check; 464 migrations, serve, identity create, and password login pass against CockroachDB v22.2.6; golangci-lint reports 0 issues; the generated spec is identical to the committed spec/swagger.json; the short suite gives 5,179 pass, 191 fail, 55 skip on this branch and on the base commit, with identical failed tests.

Not fixed here: the CI test and end-to-end jobs were red on goodnotes before this PR, and Trivy cannot see CVE-2026-33503 (Kratos itself, HIGH) in the image, because the image build gives the binary no module version.

Do not merge before a human review, and merge with a merge commit.

https://claude.ai/code/session_01A5FnxmDnKJUUrV6ZBFMCHU

Trivy on v1.3.0-gn-v10 reported 39 findings (3 CRITICAL, 36 HIGH) in
Go modules that are linked into usr/bin/kratos. A local Trivy scan of
the image from this commit reports 0.

Version changes:
- go directive: 1.22 -> 1.26.0 (the new module versions need it; the
  builder image and ci.yaml already use Go 1.26)
- github.com/jackc/pgx/v5: v5.6.0 -> v5.10.0
- google.golang.org/grpc: v1.65.0 -> v1.84.0
- golang.org/x/crypto: v0.25.0 -> v0.57.0
- golang.org/x/net: v0.27.0 -> v0.59.0
- golang.org/x/oauth2: v0.21.0 -> v0.37.0
- golang.org/x/text: v0.16.0 -> v0.42.0
- golang.org/x/mod: v0.19.0 -> v0.41.0
- github.com/go-jose/go-jose/v3: v3.0.3 -> v3.0.5
- github.com/go-jose/go-jose/v4: v4.0.2 -> v4.1.5
- github.com/golang-jwt/jwt/v4: v4.5.0 -> v4.5.2
- github.com/golang-jwt/jwt/v5: v5.2.1 -> v5.3.1
- go.opentelemetry.io/otel/sdk: v1.28.0 -> v1.46.0
- github.com/ory/dockertest/v3: v3.11.0 -> v3.12.0

pgx stays at v5.10.0 and not at v5.11.0. v5.11.0 replaces the
connection URI parser: it ends the user part at the first "@" and keeps
"+" in a query value as a literal character, so a DSN that works today
can fail to connect. v5.10.0 has the CVE fixes (Trivy needs 5.9.0).

RunTestSMTP and CleanUpTestSMTP move from package x to
internal/testhelpers. Package x is part of the server, so the helper
linked dockertest, docker/cli and runc into the release binary. The
bump of dockertest does not reach the fixed versions of docker/cli
(29.2.0) and runc (1.2.8), and the server does not use these modules.

Effects of the go directive, and their fixes:
- Go 1.26 vet rejects a non-constant format string, and go test runs
  that check. Eight calls change to a constant format or to the
  non-format function, as upstream Ory did.
- golang.org/x/net v0.59.0 marks its context.Context alias for
  inlining, and govet reports each use, so the password settings
  strategy imports the standard context package.
- go-swagger v0.31.0 panics on a type alias, and Go 1.23 and later
  materialize aliases by default. The sdk make target sets
  GODEBUG=gotypesalias=0 for the spec generation step. With it, the
  generated spec is identical to the committed spec/swagger.json. Go
  1.27 removes this setting, so go-swagger must change before that
  toolchain move. Newer go-swagger releases and the upstream fork
  change the generated spec, so they are not used here.
- licenses.yml and format.yml: Go 1.22 -> 1.26. With Go 1.22 the
  toolchain switch made go-licenses reject each standard library
  package.

Scan workflow (cve-scan.yaml) gate changes:
- It now also runs on a push to goodnotes and on a pull request into
  goodnotes. Before, a fork pull request got no scan until the tag.
- The three Anchore steps are removed. The Anchore step failed on every
  run before it scanned: scan-action v3 pins grype v0.74.4, and grype
  refuses a vulnerability database that is more than 5 days old. Its
  SARIF upload used the deprecated CodeQL v2 action.
- .grype.yaml is deleted, because the Anchore step was its only reader.
- The Trivy step stays as the vulnerability gate (CRITICAL and HIGH,
  exit code 42).

Claude-Session: https://claude.ai/code/session_01A5FnxmDnKJUUrV6ZBFMCHU
@dmakarenko
dmakarenko force-pushed the bump-go-deps-cve-20261001 branch from 77d78c5 to ffc99c0 Compare October 1, 2026 21:34
@dmakarenko

Copy link
Copy Markdown
Author

Check before the rollout of the next tag: database pool size.

The go 1.26.0 directive turns on the Go 1.25 and 1.26 runtime defaults, and one of them is container-aware GOMAXPROCS: the runtime now sets GOMAXPROCS from the CPU limit of the container, and not from the CPU count of the node.

Kratos sizes two things from GOMAXPROCS:

  • the default database pool (ory/x sqlcon: max_conns = 2 * GOMAXPROCS, max_idle_conns = GOMAXPROCS), when the DSN does not set them
  • the Jsonnet worker pool (cmd/root.go)

Example: a pod with CPU limit 1 on a 16-core node, and a DSN with no max_conns. Before: 32 connections, 16 idle, 16 Jsonnet workers. After: 4 connections, 2 idle, 2 Jsonnet workers.

There is no effect if the pod has no CPU limit, or if the DSN sets max_conns and max_idle_conns. The production pod spec and DSN are not in this repository, so this is not verified.

If the limit is set and the DSN does not size the pool, set max_conns and max_idle_conns in the DSN. The alternative is godebug containermaxprocs=0 in go.mod, which keeps the runtime behavior of the current images.

Source: review round 01-initial, open question from the bugs+impl agent.

@dmakarenko
dmakarenko merged commit e9a07c5 into goodnotes Oct 1, 2026
13 of 22 checks passed
@dmakarenko
dmakarenko deleted the bump-go-deps-cve-20261001 branch October 1, 2026 22:06
@dmakarenko

Copy link
Copy Markdown
Author

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.

1 participant