From a32b9270e61feeaa2af4574065d5388d4027330b Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 11 Jul 2026 15:33:16 +0300 Subject: [PATCH] fix(db/migrate): preserve verify-ca/verify-full TLS mode in migration DSN buildMigrateDSN inferred the migration sslmode solely from whether TLSConfig was nil, collapsing every TLS-enabled configuration to sslmode=require. This silently downgraded stricter modes (verify-ca, verify-full, both supported in internal/database/config.go) during migrations, dropping the certificate validation the application enforces in normal operation. Recover the exact mode from the parsed pgx TLSConfig via sslModeFromTLSConfig, which inverts pgx's sslmode -> *tls.Config mapping (nil -> disable; InsecureSkipVerify without a VerifyPeerCertificate callback -> require; InsecureSkipVerify with the callback -> verify-ca; InsecureSkipVerify=false -> verify-full). Add TestBuildMigrateDSN_PreservesSSLMode, which round-trips each mode DSN -> pgx TLSConfig -> migration DSN and pins the version-specific mapping; it fails on the pre-fix inference for verify-ca and verify-full. --- .../database/postgres/migrations/migrate.go | 45 ++++++++++++++++--- .../migrations/migrate_security_test.go | 20 +++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/internal/database/postgres/migrations/migrate.go b/internal/database/postgres/migrations/migrate.go index d605d1f6f..7f1396183 100644 --- a/internal/database/postgres/migrations/migrate.go +++ b/internal/database/postgres/migrations/migrate.go @@ -2,6 +2,7 @@ package migrations import ( "context" + "crypto/tls" "errors" "fmt" "log" @@ -513,8 +514,9 @@ func GetMigrationVersion(ctx context.Context, pool *pgxpool.Pool, migrationsPath } // buildMigrateDSN builds a connection string for golang-migrate from pgx config. -// sslmode is inferred from the pgx TLSConfig ("require" when TLS is configured, -// "disable" otherwise). +// The SSL mode is recovered from the parsed TLSConfig so strict modes +// (verify-ca / verify-full) are preserved rather than silently downgraded to +// require. See sslModeFromTLSConfig for the exact mapping. func buildMigrateDSN(config *pgxpool.Config) string { // Extract connection details from pgx config host := config.ConnConfig.Host @@ -527,11 +529,7 @@ func buildMigrateDSN(config *pgxpool.Config) string { encodedUser := url.QueryEscape(user) encodedPassword := url.QueryEscape(password) - // Infer sslmode from the TLS config. - sslMode := "require" - if config.ConnConfig.TLSConfig == nil { - sslMode = "disable" - } + sslMode := sslModeFromTLSConfig(config.ConnConfig.TLSConfig) // Build DSN (golang-migrate uses postgres:// format) // Don't add connection options - RDS Proxy doesn't support them @@ -546,6 +544,39 @@ func buildMigrateDSN(config *pgxpool.Config) string { ) } +// sslModeFromTLSConfig recovers the libpq sslmode string that pgx derived from +// the original DSN, so the migration DSN matches the security policy the +// application enforces in normal operation. pgx (v5) maps sslmode -> *tls.Config +// as follows, which we invert here: +// +// - disable -> nil +// - require -> InsecureSkipVerify=true, no custom chain/host verification +// - verify-ca -> InsecureSkipVerify=true + a VerifyPeerCertificate callback +// (verifies the chain but not the hostname) +// - verify-full -> InsecureSkipVerify=false + ServerName set +// (verifies both chain and hostname) +// +// Collapsing every TLS-enabled mode to "require" would silently drop the +// certificate-validation guarantees of verify-ca / verify-full during +// migrations, so we distinguish them explicitly. +func sslModeFromTLSConfig(tlsConfig *tls.Config) string { + if tlsConfig == nil { + return "disable" + } + // verify-full is the only mode where pgx leaves InsecureSkipVerify=false: + // the standard library then verifies both the chain and the hostname. + if !tlsConfig.InsecureSkipVerify { + return "verify-full" + } + // verify-ca skips the standard hostname check but installs a custom + // VerifyPeerCertificate callback to validate the chain; plain require + // installs neither. + if tlsConfig.VerifyPeerCertificate != nil { + return "verify-ca" + } + return "require" +} + // ValidateMigrationsPath checks if migrations directory exists. func ValidateMigrationsPath(path string) error { info, err := os.Stat(path) diff --git a/internal/database/postgres/migrations/migrate_security_test.go b/internal/database/postgres/migrations/migrate_security_test.go index e67a20b27..29a15bae3 100644 --- a/internal/database/postgres/migrations/migrate_security_test.go +++ b/internal/database/postgres/migrations/migrate_security_test.go @@ -133,6 +133,26 @@ func TestBuildMigrateDSN_PasswordNotInLogs(t *testing.T) { "buildMigrateDSN must not emit the database password to the log output") } +// TestBuildMigrateDSN_PreservesSSLMode guards against silently downgrading the +// migration DSN's sslmode: a strict mode (verify-ca / verify-full) must survive +// the DSN -> pgx *tls.Config -> migration DSN round-trip rather than collapsing +// to require. This also pins the pgx-version-specific TLSConfig mapping in +// sslModeFromTLSConfig. +func TestBuildMigrateDSN_PreservesSSLMode(t *testing.T) { + for _, mode := range []string{"disable", "require", "verify-ca", "verify-full"} { + t.Run(mode, func(t *testing.T) { + rawDSN := fmt.Sprintf("postgres://user:pass@localhost:5432/db?sslmode=%s", mode) + poolCfg, err := pgxpool.ParseConfig(rawDSN) + require.NoError(t, err, "pgxpool.ParseConfig must accept sslmode=%s", mode) + + result := buildMigrateDSN(poolCfg) + + assert.Contains(t, result, "sslmode="+mode, + "buildMigrateDSN must preserve the configured sslmode, not downgrade it") + }) + } +} + // TestMaybeForceVersion_NonNumericError ensures a non-numeric // CUDLY_FORCE_MIGRATION_VERSION produces an error without logging the // bad value to stdout.