From 2b0db3d243da3c6d3b0667e6b2d5d1cc52e4375a Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 18 Aug 2026 20:35:04 +0200 Subject: [PATCH 1/5] fix(ci): drop lib/pq by moving golang-migrate onto the pgx/v5 driver govulncheck fails repo-wide on three github.com/lib/pq advisories, GO-2026-6170 / 6171 / 6172 (CVE-2026-56871 / 2 / 3). All three were introduced at lib/pq 1.0.0 with no fix event in any release, so no version bump clears them, and all three list Driver.Open, conn.Exec, conn.Query and rows.Next among their affected symbols, which is why source-mode govulncheck reaches them rather than reporting an import-level-only finding. Every pull request has been red since the advisories were published. lib/pq entered the build through a single blank import: golang-migrate's database/postgres driver, which this repo uses only to run migrations. golang-migrate v4.19.1 also ships database/pgx/v5, built on jackc/pgx v5, which the application already uses everywhere else through pgxpool. The pgx/v5 driver registers the URL scheme "pgx5" only, where the lib/pq driver registered "postgres" and "postgresql", and golang-migrate resolves the driver by scheme at Open() time. So the scheme has to move with the import, in the library and in every place that hands a URL to the migrate CLI. A mismatch fails at migration time, not compile time, and migrations run on every container start when DB_AUTO_MIGRATE=true. Both advisory-lock derivation (GenerateAdvisoryLockId over database, schema and migrations table) and the schema_migrations table name are identical between the two drivers, so migration state carries over and a partially rolled deployment still mutually excludes. Changes: - internal/database/postgres/migrations/migrate.go: import database/pgx/v5 instead of database/postgres, and build the DSN from a named migrateURLScheme constant rather than a hardcoded "postgres://". - Dockerfile, Makefile, ci.yml, database-migration.yml: build the migrate CLI with -tags pgx5, so the binary executed on every container start stops carrying lib/pq too. - scripts/entrypoint.sh, database-migration.yml, ci.yml: emit pgx5:// URLs to match. - Two regression tests: one pins the DSN scheme to a driver the package actually imports (asserting against database.List(), so it fails in both directions), one asserts the lib/pq-backed driver is not registered at all. Proof the dependency left the build rather than just go.mod: packages matching github.com/lib/pq in `go list -deps ./...` go from 3 to 0 in the root module, and from 3 to 0 in the `-test` closure. It remains reachable in `go list -m all` only as a test dependency of testcontainers-go/modules/postgres, which nothing here compiles. Closes #1849 --- .github/workflows/ci.yml | 7 +++- .github/workflows/database-migration.yml | 22 +++++++---- Dockerfile | 10 ++++- Makefile | 2 +- go.mod | 2 +- go.sum | 2 + .../database/postgres/migrations/migrate.go | 17 ++++++-- .../migrations/migrate_security_test.go | 39 +++++++++++++++++++ scripts/entrypoint.sh | 5 ++- 9 files changed, 89 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 57a5de163..51b23bddb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -236,7 +236,10 @@ jobs: - name: Install golang-migrate run: | - go install -tags 'postgres' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 + # -tags 'pgx5', matching the Dockerfile and the library import in + # internal/database/postgres/migrations: the postgres tag links + # lib/pq, which carries three unfixable advisories (issue #1849). + go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 - name: Run database migrations env: @@ -248,7 +251,7 @@ jobs: run: | if [ -d "internal/database/postgres/migrations" ]; then migrate -path internal/database/postgres/migrations \ - -database "postgresql://${DB_USER}:${DB_PASSWORD}@${DB_HOST}:${DB_PORT}/${DB_NAME}?sslmode=disable" \ + -database "pgx5://${DB_USER}:${DB_PASSWORD}@${DB_HOST}:${DB_PORT}/${DB_NAME}?sslmode=disable" \ up fi diff --git a/.github/workflows/database-migration.yml b/.github/workflows/database-migration.yml index 429e2648e..de884d4df 100644 --- a/.github/workflows/database-migration.yml +++ b/.github/workflows/database-migration.yml @@ -276,7 +276,7 @@ jobs: go-version-file: go.mod - name: Install golang-migrate - run: go install -tags 'postgres' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 + run: go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@d979d5b3a71173a29b74b5b88418bfda9437d885 # v6.1.1 @@ -304,7 +304,9 @@ jobs: STEPS: ${{ inputs.steps }} run: | set -uo pipefail - DB_URL="postgresql://cudly:${DB_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require" + # pgx5://, not postgresql://: migrate is installed with -tags 'pgx5' + # above and golang-migrate dispatches on the URL scheme (issue #1849). + DB_URL="pgx5://cudly:${DB_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require" # `validate` already rejects a malformed `steps`/`direction` before this # job is ever reached, but this job sits behind an environment gate and @@ -339,7 +341,9 @@ jobs: DB_ENDPOINT: ${{ steps.get-endpoint.outputs.endpoint }} run: | set -uo pipefail - DB_URL="postgresql://cudly:${DB_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require" + # pgx5://, not postgresql://: migrate is installed with -tags 'pgx5' + # above and golang-migrate dispatches on the URL scheme (issue #1849). + DB_URL="pgx5://cudly:${DB_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require" VERSION=$(migrate -path "$MIGRATIONS_PATH" -database "$DB_URL" version 2>&1 || echo "unknown") echo "Current migration version: $VERSION" echo "MIGRATION_VERSION=$VERSION" >> "$GITHUB_ENV" @@ -378,7 +382,7 @@ jobs: go-version-file: go.mod - name: Install golang-migrate - run: go install -tags 'postgres' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 + run: go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 - name: Authenticate to Google Cloud uses: google-github-actions/auth@7c6bc770dae815cd3e89ee6cdf493a5fab2cc093 # v3.0.0 @@ -409,7 +413,9 @@ jobs: STEPS: ${{ inputs.steps }} run: | set -uo pipefail - DB_URL="postgresql://cudly:${DB_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require" + # pgx5://, not postgresql://: migrate is installed with -tags 'pgx5' + # above and golang-migrate dispatches on the URL scheme (issue #1849). + DB_URL="pgx5://cudly:${DB_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require" STEPS="${STEPS:-0}" @@ -465,7 +471,7 @@ jobs: go-version-file: go.mod - name: Install golang-migrate - run: go install -tags 'postgres' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 + run: go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 - name: Azure Login uses: azure/login@532459ea530d8321f2fb9bb10d1e0bcf23869a43 # v3.0.0 @@ -494,7 +500,9 @@ jobs: STEPS: ${{ inputs.steps }} run: | set -uo pipefail - DB_URL="postgresql://cudly:${DB_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require" + # pgx5://, not postgresql://: migrate is installed with -tags 'pgx5' + # above and golang-migrate dispatches on the URL scheme (issue #1849). + DB_URL="pgx5://cudly:${DB_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require" STEPS="${STEPS:-0}" diff --git a/Dockerfile b/Dockerfile index 8600ea228..4d867d10e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -47,8 +47,16 @@ SHELL ["/bin/ash", "-eo", "pipefail", "-c"] # bin/${GOOS}_${GOARCH}/ instead, so resolve both layouts; the final `mv` fails # the build if neither produced a binary. # Keep this version in step with MIGRATE_VERSION in the Makefile. +# -tags=pgx5, not postgres: the postgres tag links the lib/pq driver, which +# carries three unfixable advisories (GO-2026-6170/6171/6172, no fixed version +# in any release) reached through Driver.Open and conn.Exec on every container +# start. The pgx5 tag builds the same driver on jackc/pgx v5, which the +# application already uses. It registers the "pgx5" URL scheme only, so +# scripts/entrypoint.sh and .github/workflows/database-migration.yml must build +# pgx5:// URLs - a postgres:// URL against this binary fails at runtime with +# "unknown driver". See issue #1849. RUN CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} \ - go install -tags=postgres -ldflags="-s -w -X main.Version=v4.19.1" \ + go install -tags=pgx5 -ldflags="-s -w -X main.Version=v4.19.1" \ github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 && \ GOPATH_BIN="$(go env GOPATH)/bin" && \ MIGRATE_BIN="${GOPATH_BIN}/${TARGETOS}_${TARGETARCH}/migrate" && \ diff --git a/Makefile b/Makefile index 4d3a65848..d58863031 100644 --- a/Makefile +++ b/Makefile @@ -247,7 +247,7 @@ install-dev-tools: @echo "Installing gocyclo $(GOCYCLO_VERSION)..." @go install github.com/fzipp/gocyclo/cmd/gocyclo@$(GOCYCLO_VERSION) @echo "Installing golang-migrate $(MIGRATE_VERSION)..." - @go install -tags 'postgres' github.com/golang-migrate/migrate/v4/cmd/migrate@$(MIGRATE_VERSION) + @go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate@$(MIGRATE_VERSION) @echo "✓ Development tools installed" @echo "" @echo "Additional tools to install manually:" diff --git a/go.mod b/go.mod index 00be2c581..9e823889c 100644 --- a/go.mod +++ b/go.mod @@ -149,11 +149,11 @@ require ( github.com/envoyproxy/go-control-plane/envoy v1.37.0 // indirect github.com/envoyproxy/protoc-gen-validate v1.3.3 // indirect github.com/go-ole/go-ole v1.2.6 // indirect + github.com/jackc/pgerrcode v0.0.0-20220416144525-469b46aa5efa // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect github.com/klauspost/compress v1.18.5 // indirect - github.com/lib/pq v1.10.9 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect github.com/magiconair/properties v1.8.10 // indirect github.com/microsoft/kiota-abstractions-go v1.9.4 // indirect diff --git a/go.sum b/go.sum index 137b0dc16..2e70038ec 100644 --- a/go.sum +++ b/go.sum @@ -236,6 +236,8 @@ github.com/googleapis/gax-go/v2 v2.21.0 h1:h45NjjzEO3faG9Lg/cFrBh2PgegVVgzqKzuZl github.com/googleapis/gax-go/v2 v2.21.0/go.mod h1:But/NJU6TnZsrLai/xBAQLLz+Hc7fHZJt/hsCz3Fih4= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/jackc/pgerrcode v0.0.0-20220416144525-469b46aa5efa h1:s+4MhCQ6YrzisK6hFJUX53drDT4UsSW3DEhKn0ifuHw= +github.com/jackc/pgerrcode v0.0.0-20220416144525-469b46aa5efa/go.mod h1:a/s9Lp5W7n/DD0VrVoyJ00FbP2ytTPDVOivvn2bMlds= github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM= github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg= github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo= diff --git a/internal/database/postgres/migrations/migrate.go b/internal/database/postgres/migrations/migrate.go index 59a433bcf..5958b899a 100644 --- a/internal/database/postgres/migrations/migrate.go +++ b/internal/database/postgres/migrations/migrate.go @@ -11,8 +11,8 @@ import ( "strconv" "github.com/golang-migrate/migrate/v4" - _ "github.com/golang-migrate/migrate/v4/database/postgres" // postgres driver - _ "github.com/golang-migrate/migrate/v4/source/file" // file source + _ "github.com/golang-migrate/migrate/v4/database/pgx/v5" // postgres driver (pgx/v5) + _ "github.com/golang-migrate/migrate/v4/source/file" // file source "github.com/jackc/pgx/v5/pgxpool" "golang.org/x/crypto/bcrypt" ) @@ -540,6 +540,15 @@ func GetMigrationVersion(ctx context.Context, pool *pgxpool.Pool, migrationsPath return version, dirty, nil } +// migrateURLScheme selects which golang-migrate database driver handles the +// DSN. golang-migrate dispatches purely on the URL scheme, and the pgx/v5 +// driver registers itself as "pgx5" only - "postgres"/"postgresql" belong to +// the lib/pq-backed driver this package deliberately does not import (issue +// #1849). A mismatch between this constant and the imported driver fails at +// migration time, not at compile time, which is why +// TestBuildMigrateDSN_SchemeMatchesRegisteredDriver pins the two together. +const migrateURLScheme = "pgx5" + // buildMigrateDSN builds a connection string for golang-migrate from pgx config. // The SSL mode is recovered from the parsed TLSConfig so strict modes // (verify-ca / verify-full) are preserved rather than silently downgraded to @@ -558,10 +567,10 @@ func buildMigrateDSN(config *pgxpool.Config) string { sslMode := sslModeFromTLSConfig(config.ConnConfig.TLSConfig) - // Build DSN (golang-migrate uses postgres:// format) // Don't add connection options - RDS Proxy doesn't support them return fmt.Sprintf( - "postgres://%s:%s@%s:%d/%s?sslmode=%s", + "%s://%s:%s@%s:%d/%s?sslmode=%s", + migrateURLScheme, encodedUser, encodedPassword, host, diff --git a/internal/database/postgres/migrations/migrate_security_test.go b/internal/database/postgres/migrations/migrate_security_test.go index 29a15bae3..3f6401ce7 100644 --- a/internal/database/postgres/migrations/migrate_security_test.go +++ b/internal/database/postgres/migrations/migrate_security_test.go @@ -5,9 +5,11 @@ import ( "context" "fmt" "log" + "net/url" "os" "testing" + "github.com/golang-migrate/migrate/v4/database" "github.com/jackc/pgx/v5/pgxpool" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -153,6 +155,43 @@ func TestBuildMigrateDSN_PreservesSSLMode(t *testing.T) { } } +// TestBuildMigrateDSN_SchemeMatchesRegisteredDriver pins the migration DSN's +// URL scheme to a driver this package actually imports. golang-migrate resolves +// the driver by scheme at Open() time, so a scheme naming no registered driver +// fails at migration time -- on every container start, since entrypoint.sh runs +// migrations with DB_AUTO_MIGRATE=true -- and not at compile time. Swapping the +// driver import without swapping the scheme (or the reverse) is exactly the +// mistake this guards: the pgx/v5 driver registers "pgx5" only, where the +// lib/pq-backed driver registered "postgres"/"postgresql". +func TestBuildMigrateDSN_SchemeMatchesRegisteredDriver(t *testing.T) { + poolCfg, err := pgxpool.ParseConfig("postgres://user:pass@localhost:5432/db?sslmode=require") + require.NoError(t, err, "pgxpool.ParseConfig must accept the fixture DSN") + + parsed, err := url.Parse(buildMigrateDSN(poolCfg)) + require.NoError(t, err, "buildMigrateDSN must return a parseable URL") + + assert.Equal(t, migrateURLScheme, parsed.Scheme, + "buildMigrateDSN must emit the scheme named by migrateURLScheme") + assert.Contains(t, database.List(), migrateURLScheme, + "migrateURLScheme must name a golang-migrate driver this package imports; "+ + "registered drivers: %v", database.List()) +} + +// TestLibPqDriverNotRegistered asserts the lib/pq-backed golang-migrate driver +// is not linked into this package. lib/pq carries three advisories with no +// fixed version in any release (GO-2026-6170/6171/6172, CVE-2026-56871/2/3), +// reached through Driver.Open and conn.Exec, so importing it fails the +// repo-wide govulncheck gate with no bump available to clear it (issue #1849). +// Re-adding the import is a one-line change that would otherwise only surface +// as a red CI run on an unrelated pull request. +func TestLibPqDriverNotRegistered(t *testing.T) { + for _, scheme := range []string{"postgres", "postgresql"} { + assert.NotContains(t, database.List(), scheme, + "golang-migrate driver %q is registered, which means the lib/pq-backed "+ + "database/postgres driver was imported somewhere in this package", scheme) + } +} + // TestMaybeForceVersion_NonNumericError ensures a non-numeric // CUDLY_FORCE_MIGRATION_VERSION produces an error without logging the // bad value to stdout. diff --git a/scripts/entrypoint.sh b/scripts/entrypoint.sh index 568241008..5fe9baf84 100644 --- a/scripts/entrypoint.sh +++ b/scripts/entrypoint.sh @@ -64,7 +64,10 @@ if [ "$DB_AUTO_MIGRATE" = "true" ]; then # Run migrations if password is available # URL-encode password to handle special characters (generated passwords contain =,[,$, etc.) ENCODED_PASSWORD=$(printf '%s' "${DB_PASSWORD:-}" | awk 'BEGIN{split("",hex); for(i=0;i<256;i++){c=sprintf("%c",i); hex[c]=sprintf("%%%02X",i)}} {n=length($0); for(i=1;i<=n;i++){c=substr($0,i,1); if(c~/[A-Za-z0-9._~-]/)printf "%s",c; else printf "%s",hex[c]}}') - DB_URL="postgresql://${DB_USER}:${ENCODED_PASSWORD}@${DB_HOST}:${DB_PORT}/${DB_NAME}?sslmode=${DB_SSL_MODE}" + # pgx5://, not postgresql://: the migrate binary in this image is built + # with -tags=pgx5 (see Dockerfile), and golang-migrate dispatches on the + # URL scheme. The pgx/v5 driver registers "pgx5" only. See issue #1849. + DB_URL="pgx5://${DB_USER}:${ENCODED_PASSWORD}@${DB_HOST}:${DB_PORT}/${DB_NAME}?sslmode=${DB_SSL_MODE}" migrate -path "$DB_MIGRATIONS_PATH" -database "$DB_URL" up 2>&1 MIGRATE_EXIT_CODE=$? From 441c2c7aba2468e39e42079b71c2ff05ce6b606d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 18 Aug 2026 21:00:53 +0200 Subject: [PATCH 2/5] fix(build): build the migrate CLI from this module so its deps match ours Building the CLI with -tags=pgx5 removed lib/pq from the shipped image but replaced it with something worse: `go install ...@v4.19.1` resolves dependencies from golang-migrate's own go.mod, which pins jackc/pgx v5.5.4, golang.org/x/crypto v0.45.0 and golang.org/x/text v0.31.0. The postgres tag never linked any of those, so the swap took /usr/local/bin/migrate from five advisories with no published fix to seventeen with one, and scripts/scan-shipped-image.sh failed the build exactly as it should have. Build cmd/migrate as a package of this module instead, with no @version suffix, so it resolves against the repo's own pins: pgx v5.9.2, x/crypto v0.53.0, x/text v0.39.0, every one of them above the fixed version the advisories name. Measured with `govulncheck -mode=binary` on the resulting binary: no vulnerabilities found, against seventeen fixable ones for the @v4.19.1 build of the same source at the same tag. In the Dockerfile this moves the build below `go mod download`, where go.mod and go.sum exist, and lets `go build -o` replace the GOPATH and cross-compilation-subdirectory resolution that `go install` needed. The version now comes from go.mod rather than being restated: MIGRATE_VERSION is gone from the Makefile, and the -X main.Version ldflag reads `go list -m -f '{{.Version}}' github.com/golang-migrate/migrate/v4`. Refs #1849 --- .github/workflows/ci.yml | 6 ++- .github/workflows/database-migration.yml | 21 +++++++-- Dockerfile | 57 +++++++++++++----------- Makefile | 8 ++-- 4 files changed, 59 insertions(+), 33 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51b23bddb..00b274639 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -238,8 +238,10 @@ jobs: run: | # -tags 'pgx5', matching the Dockerfile and the library import in # internal/database/postgres/migrations: the postgres tag links - # lib/pq, which carries three unfixable advisories (issue #1849). - go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 + # lib/pq, which carries unfixable advisories (issue #1849). No + # @version suffix: installed as a package of this module, so the + # version and dependency set come from go.mod. + go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate - name: Run database migrations env: diff --git a/.github/workflows/database-migration.yml b/.github/workflows/database-migration.yml index de884d4df..252ca3d5a 100644 --- a/.github/workflows/database-migration.yml +++ b/.github/workflows/database-migration.yml @@ -276,7 +276,12 @@ jobs: go-version-file: go.mod - name: Install golang-migrate - run: go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 + # No @version suffix: installed as a package of this module, so the + # migrate version and its dependency set come from go.mod, the same way + # the Dockerfile and `make install-tools` build it. -tags 'pgx5' keeps + # lib/pq out of the binary that talks to the production database. + # See issue #1849. + run: go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@d979d5b3a71173a29b74b5b88418bfda9437d885 # v6.1.1 @@ -382,7 +387,12 @@ jobs: go-version-file: go.mod - name: Install golang-migrate - run: go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 + # No @version suffix: installed as a package of this module, so the + # migrate version and its dependency set come from go.mod, the same way + # the Dockerfile and `make install-tools` build it. -tags 'pgx5' keeps + # lib/pq out of the binary that talks to the production database. + # See issue #1849. + run: go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate - name: Authenticate to Google Cloud uses: google-github-actions/auth@7c6bc770dae815cd3e89ee6cdf493a5fab2cc093 # v3.0.0 @@ -471,7 +481,12 @@ jobs: go-version-file: go.mod - name: Install golang-migrate - run: go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 + # No @version suffix: installed as a package of this module, so the + # migrate version and its dependency set come from go.mod, the same way + # the Dockerfile and `make install-tools` build it. -tags 'pgx5' keeps + # lib/pq out of the binary that talks to the production database. + # See issue #1849. + run: go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate - name: Azure Login uses: azure/login@532459ea530d8321f2fb9bb10d1e0bcf23869a43 # v3.0.0 diff --git a/Dockerfile b/Dockerfile index 4d867d10e..990e64bc5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -38,31 +38,6 @@ RUN apk add --no-cache \ # Set shell with pipefail for safer pipe operations SHELL ["/bin/ash", "-eo", "pipefail", "-c"] -# Build golang-migrate from source on this stage's pinned Go toolchain, the same -# way `make install-tools` does. Upstream's prebuilt release tarballs carry -# whatever toolchain upstream built them with (v4.19.1 ships go1.25.4), which is -# how issue #1833's stdlib CVEs reached the runtime image, where entrypoint.sh -# runs `migrate up` against the database on every container start. -# `go install` refuses GOBIN when cross-compiling and writes to -# bin/${GOOS}_${GOARCH}/ instead, so resolve both layouts; the final `mv` fails -# the build if neither produced a binary. -# Keep this version in step with MIGRATE_VERSION in the Makefile. -# -tags=pgx5, not postgres: the postgres tag links the lib/pq driver, which -# carries three unfixable advisories (GO-2026-6170/6171/6172, no fixed version -# in any release) reached through Driver.Open and conn.Exec on every container -# start. The pgx5 tag builds the same driver on jackc/pgx v5, which the -# application already uses. It registers the "pgx5" URL scheme only, so -# scripts/entrypoint.sh and .github/workflows/database-migration.yml must build -# pgx5:// URLs - a postgres:// URL against this binary fails at runtime with -# "unknown driver". See issue #1849. -RUN CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} \ - go install -tags=pgx5 -ldflags="-s -w -X main.Version=v4.19.1" \ - github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1 && \ - GOPATH_BIN="$(go env GOPATH)/bin" && \ - MIGRATE_BIN="${GOPATH_BIN}/${TARGETOS}_${TARGETARCH}/migrate" && \ - { [ -x "${MIGRATE_BIN}" ] || MIGRATE_BIN="${GOPATH_BIN}/migrate"; } && \ - mv "${MIGRATE_BIN}" /usr/local/bin/migrate - WORKDIR /app # Copy go module files @@ -77,6 +52,38 @@ COPY providers/gcp/go.mod providers/gcp/go.sum providers/gcp/ # Download dependencies RUN go mod download +# Build golang-migrate from source on this stage's pinned Go toolchain, the same +# way `make install-tools` does. Upstream's prebuilt release tarballs carry +# whatever toolchain upstream built them with (v4.19.1 ships go1.25.4), which is +# how issue #1833's stdlib CVEs reached the runtime image, where entrypoint.sh +# runs `migrate up` against the database on every container start. +# +# Built as a package of THIS module (no @version suffix) rather than +# `go install ...@v4.19.1`, and therefore placed after `go mod download`. +# The @version form resolves dependencies from golang-migrate's own go.mod, +# which pins jackc/pgx v5.5.4, x/crypto v0.45.0 and x/text v0.31.0 - all +# superseded, and together 17 advisories with published fixes that +# scripts/scan-shipped-image.sh correctly fails on. Building from the main +# module instead resolves them at this repo's own versions, which are already +# above every one of those fixes. The migrate version therefore comes from +# go.mod, and is read back out of the build list rather than restated. +# +# -tags=pgx5, not postgres: the postgres tag links the lib/pq driver, which +# carries advisories with no fixed version in any release (GO-2026-6166, 6168, +# 6170, 6171, 6172) reached through Driver.Open and conn.Exec on every +# container start. The pgx5 tag builds the same driver on jackc/pgx v5, which +# the application already uses. It registers the "pgx5" URL scheme only, so +# scripts/entrypoint.sh and .github/workflows/database-migration.yml must build +# pgx5:// URLs - a postgres:// URL against this binary fails at runtime with +# "unknown driver". See issue #1849. +RUN MIGRATE_VERSION="$(go list -m -f '{{.Version}}' github.com/golang-migrate/migrate/v4)" && \ + echo "Building golang-migrate ${MIGRATE_VERSION} for ${TARGETOS}/${TARGETARCH}" && \ + CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build \ + -tags=pgx5 \ + -ldflags="-s -w -X main.Version=${MIGRATE_VERSION}" \ + -o /usr/local/bin/migrate \ + github.com/golang-migrate/migrate/v4/cmd/migrate + # Copy source code COPY . . diff --git a/Makefile b/Makefile index d58863031..e34ce114e 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,9 @@ GIT_SHA?=$(shell git rev-parse --short HEAD 2>/dev/null || echo unknown) GOLANGCI_LINT_VERSION?=v2.10.1 GOSEC_VERSION?=v2.28.0 GOCYCLO_VERSION?=v0.6.0 -MIGRATE_VERSION?=v4.19.1 +# golang-migrate deliberately has no version variable: it is installed as a +# package of this module (see install-tools), so its version and its whole +# dependency set come from go.mod. See issue #1849. # staticcheck has no CI pin; it is used by scripts/security-scan.sh STATICCHECK_VERSION?=v0.7.0 LDFLAGS=-ldflags "-s -w -X main.Version=$(VERSION) -X main.BuildTime=$(BUILD_TIME) -X main.GitSHA=$(GIT_SHA)" @@ -246,8 +248,8 @@ install-dev-tools: @go install honnef.co/go/tools/cmd/staticcheck@$(STATICCHECK_VERSION) @echo "Installing gocyclo $(GOCYCLO_VERSION)..." @go install github.com/fzipp/gocyclo/cmd/gocyclo@$(GOCYCLO_VERSION) - @echo "Installing golang-migrate $(MIGRATE_VERSION)..." - @go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate@$(MIGRATE_VERSION) + @echo "Installing golang-migrate $$(go list -m -f '{{.Version}}' github.com/golang-migrate/migrate/v4)..." + @go install -tags 'pgx5' github.com/golang-migrate/migrate/v4/cmd/migrate @echo "✓ Development tools installed" @echo "" @echo "Additional tools to install manually:" From 3559c97c65667bd068fff6e6e277185755279275 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 18 Aug 2026 21:08:49 +0200 Subject: [PATCH 3/5] docs: use pgx5:// in the copy-pasteable migrate commands DEVELOPMENT.md and DEPLOYMENT.md hand operators seven `migrate -database "postgresql://..."` commands, including the manual production migration run against RDS. With migrate now built -tags pgx5, every one of them fails with `unknown driver postgres` at the point someone is already reaching for a runbook. The dev image's binary (the upstream v4.17.0 release tarball, which registers all drivers including pgx5) and the pgx5-only binaries built from this repo both accept pgx5://, so the same scheme is correct in every one of these contexts. Each file gets one note saying why the scheme is what it is, so the next reader does not correct it back. Refs #1849 --- docs/DEPLOYMENT.md | 9 +++++++-- docs/DEVELOPMENT.md | 16 +++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 764ab5c85..1a040c85f 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -637,12 +637,17 @@ Multi-account support requires an AES-256-GCM encryption key for stored cloud ac If `DB_AUTO_MIGRATE=true` (the default), migration 000011 runs automatically on Lambda cold start. To run manually: +> The URL scheme is `pgx5://`, not `postgres://`. golang-migrate selects its +> driver by scheme, and `migrate` is built here with `-tags pgx5` so it does not +> link `lib/pq` (issue #1849). A `postgres://` URL fails with +> `unknown driver postgres`. + ```bash DB_PASSWORD=$(aws secretsmanager get-secret-value --secret-id cudly-dev-db-password-* --query SecretString --output text | jq -r .password) RDS_ENDPOINT=$(cd terraform/environments/aws && terraform output -raw database_proxy_endpoint) migrate -path internal/database/postgres/migrations \ - -database "postgresql://cudly:${DB_PASSWORD}@${RDS_ENDPOINT}:5432/cudly?sslmode=require" up + -database "pgx5://cudly:${DB_PASSWORD}@${RDS_ENDPOINT}:5432/cudly?sslmode=require" up ``` --- @@ -656,7 +661,7 @@ DB_PASSWORD=$(aws secretsmanager get-secret-value --secret-id cudly-dev-db-passw RDS_ENDPOINT=$(cd terraform/environments/aws && terraform output -raw database_proxy_endpoint) migrate -path internal/database/postgres/migrations \ - -database "postgresql://cudly:${DB_PASSWORD}@${RDS_ENDPOINT}:5432/cudly?sslmode=require" up + -database "pgx5://cudly:${DB_PASSWORD}@${RDS_ENDPOINT}:5432/cudly?sslmode=require" up ``` ### Terraform State Lock diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 3dafeaa65..92a77ec4c 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -36,17 +36,23 @@ docker-compose down ### 3. Database Access +> Migration URLs use the `pgx5://` scheme, not `postgres://`. golang-migrate +> picks its database driver by URL scheme, and this repo builds `migrate` with +> `-tags pgx5` so the binary does not link `lib/pq`, which carries advisories +> with no published fix (issue #1849). A `postgres://` URL fails against it with +> `unknown driver postgres`. + ```bash # Connect to PostgreSQL using psql docker-compose exec postgres psql -U cudly -d cudly # Run migrations manually docker-compose exec app migrate -path /app/internal/database/postgres/migrations \ - -database "postgresql://cudly:cudly_local_dev@postgres:5432/cudly?sslmode=disable" up + -database "pgx5://cudly:cudly_local_dev@postgres:5432/cudly?sslmode=disable" up # Check migration status docker-compose exec app migrate -path /app/internal/database/postgres/migrations \ - -database "postgresql://cudly:cudly_local_dev@postgres:5432/cudly?sslmode=disable" version + -database "pgx5://cudly:cudly_local_dev@postgres:5432/cudly?sslmode=disable" version ``` ## Development Workflow @@ -68,11 +74,11 @@ migrate create -ext sql -dir internal/database/postgres/migrations -seq add_new_ # Run migrations docker-compose exec app migrate -path /app/internal/database/postgres/migrations \ - -database "postgresql://cudly:cudly_local_dev@postgres:5432/cudly?sslmode=disable" up + -database "pgx5://cudly:cudly_local_dev@postgres:5432/cudly?sslmode=disable" up # Rollback last migration docker-compose exec app migrate -path /app/internal/database/postgres/migrations \ - -database "postgresql://cudly:cudly_local_dev@postgres:5432/cudly?sslmode=disable" down 1 + -database "pgx5://cudly:cudly_local_dev@postgres:5432/cudly?sslmode=disable" down 1 ``` ## Environment Variables @@ -374,7 +380,7 @@ docker-compose exec postgres psql -U cudly -d cudly -c "SELECT * FROM schema_mig # Force migration version (use with caution) migrate -path internal/database/postgres/migrations \ - -database "postgresql://cudly:cudly_local_dev@localhost:5432/cudly?sslmode=disable" \ + -database "pgx5://cudly:cudly_local_dev@localhost:5432/cudly?sslmode=disable" \ force ``` From 64e5dcdd6659694f4135e1152e71f1968205f468 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 18 Aug 2026 21:21:58 +0200 Subject: [PATCH 4/5] docs(test): correct the lib/pq advisory count in the guard's comment The test comment cited three advisories, matching issue #1849 as filed. The baseline govulncheck run over origin/main reports five: GO-2026-6166 (GSS authentication completes without mutual proof) and GO-2026-6168 (unbounded iteration count, CPU denial of service in lib/pq/scram) were published after the issue, and are equally unfixable. A comment that undercounts what the guard covers invites someone to weigh the tradeoff against the wrong number. Refs #1849 --- .../postgres/migrations/migrate_security_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/database/postgres/migrations/migrate_security_test.go b/internal/database/postgres/migrations/migrate_security_test.go index 3f6401ce7..505322ecb 100644 --- a/internal/database/postgres/migrations/migrate_security_test.go +++ b/internal/database/postgres/migrations/migrate_security_test.go @@ -178,10 +178,11 @@ func TestBuildMigrateDSN_SchemeMatchesRegisteredDriver(t *testing.T) { } // TestLibPqDriverNotRegistered asserts the lib/pq-backed golang-migrate driver -// is not linked into this package. lib/pq carries three advisories with no -// fixed version in any release (GO-2026-6170/6171/6172, CVE-2026-56871/2/3), -// reached through Driver.Open and conn.Exec, so importing it fails the -// repo-wide govulncheck gate with no bump available to clear it (issue #1849). +// is not linked into this package. lib/pq carries five advisories with no fixed +// version in any release (GO-2026-6166, 6168, 6170, 6171 and 6172; the issue +// was filed when only the last three existed), reached through Driver.Open and +// conn.Exec, so importing it fails the repo-wide govulncheck gate with no bump +// available to clear it (issue #1849). // Re-adding the import is a one-line change that would otherwise only surface // as a red CI run on an unrelated pull request. func TestLibPqDriverNotRegistered(t *testing.T) { From 5222f3304cd306c1df8903b4c9ef7d048f5b178c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 18 Aug 2026 21:37:16 +0200 Subject: [PATCH 5/5] test(db): assert lib/pq is absent from the build closures, not only unregistered Addresses the CodeRabbit review on #1850. Registration is the weaker of the two axes on its own: a future change could link lib/pq into the binary without registering a golang-migrate driver, and `database.List()` would stay clean while govulncheck went red on someone else's pull request. The verification for this PR measured the build closures by hand (`go list -deps`, three lib/pq packages down to zero, same for the -test closure) but nothing encoded it. This makes the guard reach as far as the property that was actually checked. Both targets are covered, since they are separate builds and covering one leaves the other unguarded: the root module (`go list -deps ./...`) and the migrate CLI as this repo builds it (`-tags=pgx5`, the binary entrypoint.sh runs against the database on every container start). A failed command and an empty package list are hard failures rather than an empty closure. "The list does not contain lib/pq" is trivially true of a list that came back empty because the toolchain was missing or `go list` errored, which would leave a guard that passes for the wrong reason. Verified in both directions rather than assumed: - On the exact pre-fix state (postgres import + postgres scheme), the closure subtest fails BY ASSERTION, naming the packages: `Should be empty, but was [github.com/lib/pq/oid github.com/lib/pq/scram github.com/lib/pq]`. Not a panic, not a shelling-out error. - With the test binary run against a PATH that has no `go` on it, both closure subtests FAIL rather than silently passing. - The registration subtest is unchanged and still fails on that same mutation. The CLI subtest's limit is stated in the comment rather than left implied: it pins the build as configured and cannot notice someone changing the tag back to `postgres` in the Dockerfile, which is a build-configuration change no Go test observes. Runtime is 0.6s for all three subtests. Refs #1849 --- .../migrations/migrate_security_test.go | 133 ++++++++++++++++-- 1 file changed, 121 insertions(+), 12 deletions(-) diff --git a/internal/database/postgres/migrations/migrate_security_test.go b/internal/database/postgres/migrations/migrate_security_test.go index 505322ecb..0f5f1015a 100644 --- a/internal/database/postgres/migrations/migrate_security_test.go +++ b/internal/database/postgres/migrations/migrate_security_test.go @@ -7,6 +7,9 @@ import ( "log" "net/url" "os" + "os/exec" + "path/filepath" + "strings" "testing" "github.com/golang-migrate/migrate/v4/database" @@ -177,20 +180,126 @@ func TestBuildMigrateDSN_SchemeMatchesRegisteredDriver(t *testing.T) { "registered drivers: %v", database.List()) } -// TestLibPqDriverNotRegistered asserts the lib/pq-backed golang-migrate driver -// is not linked into this package. lib/pq carries five advisories with no fixed -// version in any release (GO-2026-6166, 6168, 6170, 6171 and 6172; the issue -// was filed when only the last three existed), reached through Driver.Open and -// conn.Exec, so importing it fails the repo-wide govulncheck gate with no bump -// available to clear it (issue #1849). -// Re-adding the import is a one-line change that would otherwise only surface -// as a red CI run on an unrelated pull request. +// libPqModulePath is the module whose absence the guards below assert. lib/pq +// carries five advisories with no fixed version in any release (GO-2026-6166, +// 6168, 6170, 6171 and 6172; issue #1849 was filed when only the last three +// existed), reached through Driver.Open and conn.Exec, so linking it fails the +// repo-wide govulncheck gate with no bump available to clear it. +const libPqModulePath = "github.com/lib/pq" + +// migrateCLIPackage and migrateCLIBuildTag mirror how the Dockerfile, the +// Makefile and the ci / database-migration workflows build the migrate binary +// that runs `migrate up` against the database on every container start. +const ( + migrateCLIPackage = "github.com/golang-migrate/migrate/v4/cmd/migrate" + migrateCLIBuildTag = "pgx5" +) + +// TestLibPqDriverNotRegistered asserts lib/pq is not reachable from anything +// this repo builds, along two independent axes. +// +// Registration is the weaker axis on its own: a change could link lib/pq +// without registering a golang-migrate driver, and the registration assertions +// would stay green. So the build closure is asserted directly, which is the +// same property `go list -deps` measures and the one govulncheck's source mode +// analyses. Both targets are covered, because the module and the CLI are +// separate builds and covering one leaves the other unguarded. +// +// Known limit, stated so nobody reads more into it: the CLI subtest pins the +// build as configured, with migrateCLIBuildTag. It asserts that build stays +// lib/pq-free; it cannot notice someone changing the tag back to `postgres` in +// the Dockerfile, which is a build-configuration change no Go test observes. func TestLibPqDriverNotRegistered(t *testing.T) { - for _, scheme := range []string{"postgres", "postgresql"} { - assert.NotContains(t, database.List(), scheme, - "golang-migrate driver %q is registered, which means the lib/pq-backed "+ - "database/postgres driver was imported somewhere in this package", scheme) + t.Run("driver not registered", func(t *testing.T) { + for _, scheme := range []string{"postgres", "postgresql"} { + assert.NotContains(t, database.List(), scheme, + "golang-migrate driver %q is registered, which means the lib/pq-backed "+ + "database/postgres driver was imported somewhere in this package", scheme) + } + }) + + t.Run("absent from the root module build closure", func(t *testing.T) { + deps := goListDeps(t, "./...") + assert.Empty(t, packagesFromModule(deps, libPqModulePath), + "%s packages are reachable from `go list -deps ./...`; the module is linked "+ + "into this repo's build even if no golang-migrate driver registered it", + libPqModulePath) + }) + + t.Run("absent from the migrate CLI build closure", func(t *testing.T) { + deps := goListDeps(t, "-tags="+migrateCLIBuildTag, migrateCLIPackage) + assert.Empty(t, packagesFromModule(deps, libPqModulePath), + "%s packages are reachable from the migrate CLI built with -tags=%s; that is "+ + "the binary scripts/entrypoint.sh runs against the database on every "+ + "container start", libPqModulePath, migrateCLIBuildTag) + }) +} + +// goListDeps returns the build closure `go list -deps ` reports, +// resolved from the main module's root so `./...` means the whole module rather +// than whichever package directory the test happens to run in. +// +// A failed command and an empty result are both hard failures rather than an +// empty closure: "the package list does not contain lib/pq" is trivially true +// of a list that is empty because the toolchain was missing, the module root +// could not be resolved, or `go list` errored. That would turn this guard into +// one that passes for the wrong reason, which is worse than not having it. +func goListDeps(t *testing.T, args ...string) []string { + t.Helper() + + root := mainModuleRoot(t) + + cmd := exec.CommandContext(t.Context(), "go", append([]string{"list", "-deps"}, args...)...) + cmd.Dir = root + var stderr bytes.Buffer + cmd.Stderr = &stderr + + out, err := cmd.Output() + require.NoErrorf(t, err, "go list -deps %v in %s failed: %s", args, root, stderr.String()) + + pkgs := strings.Fields(string(out)) + require.NotEmptyf(t, pkgs, "go list -deps %v in %s returned no packages", args, root) + + return pkgs +} + +// mainModuleRoot resolves the directory holding the main module's go.mod. +// `go env GOMOD` reports the module the test's own directory belongs to, which +// is the root module even under the go.work workspace. +func mainModuleRoot(t *testing.T) string { + t.Helper() + + cmd := exec.CommandContext(t.Context(), "go", "env", "GOMOD") + var stderr bytes.Buffer + cmd.Stderr = &stderr + + out, err := cmd.Output() + require.NoErrorf(t, err, "go env GOMOD failed: %s", stderr.String()) + + // `go env GOMOD` reports os.DevNull, not an empty string, when the toolchain + // is in GOPATH mode. Both mean there is no module root to resolve, and + // neither may be allowed to reach filepath.Dir and produce a plausible-looking + // directory that `go list` would then run in. + goMod := strings.TrimSpace(string(out)) + require.NotEmpty(t, goMod, "go env GOMOD returned no path; the test is not running inside a module") + require.NotEqual(t, os.DevNull, goMod, "go env GOMOD reported %s; the toolchain is not in module mode", os.DevNull) + + return filepath.Dir(goMod) +} + +// packagesFromModule returns the entries of pkgs that belong to module, matching +// the module path itself and any package beneath it (lib/pq's vulnerable symbols +// span both `github.com/lib/pq` and `github.com/lib/pq/scram`). Prefix matching +// is anchored on a trailing slash so a same-prefixed but unrelated module such +// as `github.com/lib/pqfoo` is not counted. +func packagesFromModule(pkgs []string, module string) []string { + var found []string + for _, pkg := range pkgs { + if pkg == module || strings.HasPrefix(pkg, module+"/") { + found = append(found, pkg) + } } + return found } // TestMaybeForceVersion_NonNumericError ensures a non-numeric