fix(db/migrate): preserve verify-ca/verify-full TLS mode in migration DSN

[cristim]

What

buildMigrateDSN (used to build the golang-migrate connection string) inferred the migration sslmode solely from whether the pgx TLSConfig was nil:

sslMode := "require"
if config.ConnConfig.TLSConfig == nil {
    sslMode = "disable"
}

This collapses every TLS-enabled configuration to sslmode=require, silently downgrading stricter modes (verify-ca, verify-full, both supported in internal/database/config.go). During migrations the connection then performs no certificate-chain or hostname verification that the application enforces in normal operation, so a deployment configured for verify-full runs its migrations with require-level (no verification) TLS.

Fix

Recover the exact mode from the parsed pgx TLSConfig via a new sslModeFromTLSConfig, inverting pgx v5's sslmode -> *tls.Config mapping:

sslmode	pgx TLSConfig
disable	nil
require	InsecureSkipVerify=true, no VerifyPeerCertificate
verify-ca	InsecureSkipVerify=true + a VerifyPeerCertificate callback
verify-full	InsecureSkipVerify=false + ServerName

Added 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 (both downgraded to require) and passes after the fix.

Provenance

Extracted from #1276, which is now closed as superseded by main's independent golangci-lint sweep (that branch reached the same lint-clean state via root-cause fixes, ~30 nolints, rather than #1276's ~291 suppressions). This TLS-mode preservation is the one unique, non-lint fix on that branch and does not belong to the lint theme, so it lands here on its own.

Verification

• go build ./... passes.
• New test passes on this branch; confirmed it fails on the pre-fix code.
• golangci-lint adds 0 new issues over origin/main (--new-from-rev origin/main clean); the pre-existing findings in migrate.go are main's baseline, untouched.
• .golangci.yml byte-identical to origin/main.
• Diff is 2 files, +58/-7.

Summary by CodeRabbit

• Bug Fixes

  • Database migrations now preserve configured PostgreSQL SSL modes, including certificate verification settings.
  • Strengthened TLS configurations are retained during migration runs instead of being downgraded.

• Tests

  • Added coverage to verify SSL mode preservation across supported PostgreSQL connection settings.

[coderabbitai]

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 18cd3897-6db2-440d-874c-6a9d627b79fe

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Migration DSN construction now derives PostgreSQL sslmode from the parsed pgx TLS configuration, preserving strict verification modes. A regression test covers disable, require, verify-ca, and verify-full.

Changes

PostgreSQL migration TLS handling

Layer / File(s)	Summary
Map pgx TLS settings to migration sslmode internal/database/postgres/migrations/migrate.go	buildMigrateDSN now uses sslModeFromTLSConfig to map TLS settings to disable, require, verify-ca, or verify-full.
Validate SSL mode preservation internal/database/postgres/migrations/migrate_security_test.go	Tests verify that each supported SSL mode remains present in the generated migration DSN.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: preserving PostgreSQL TLS mode during migration DSN construction.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/migrate-preserve-sslmode

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/database/postgres/migrations/migrate.go (1)

532-544: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Preserve the CA path for migration connections
internal/database/postgres/migrations/migrate.go:520-544 only carries sslmode; it drops any sslrootcert/CA source, and internal/database/config.go doesn’t expose one elsewhere. verify-ca/verify-full will only work if lib/pq can find its default trust anchor, so strict-TLS deployments can fail here. Thread the root cert through to the migration DSN as well.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/database/postgres/migrations/migrate.go` around lines 532 - 544,
Update the migration DSN construction in the shown migration connection flow to
preserve the TLS root certificate/CA source alongside sslmode. Thread the
configured CA path into the generated postgres URL using lib/pq’s sslrootcert
option, while retaining the existing behavior for connections without a
configured CA.

🧹 Nitpick comments (2)

internal/database/postgres/migrations/migrate_security_test.go (1)

136-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider covering require + sslrootcert.

The table only exercises bare sslmode values, so it can't catch that pgx maps require+sslrootcert to verify-ca (see the sslModeFromTLSConfig comment), nor whether strict modes actually connect during migration. A subtest with sslrootcert set would pin that behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/database/postgres/migrations/migrate_security_test.go` around lines
136 - 154, Extend TestBuildMigrateDSN_PreservesSSLMode with a subtest or table
case combining sslmode=require and an sslrootcert value, then assert that
buildMigrateDSN preserves the resulting pgx mapping as sslmode=verify-ca. Use a
valid certificate path or test fixture accepted by pgxpool.ParseConfig, and
retain the existing bare-mode coverage.

internal/database/postgres/migrations/migrate.go (1)

547-578: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Clarify the require + sslrootcert case in the TLS mapping.
pgx treats that combination as verify-ca, so the require bullet should be narrowed to require without a root cert. Add a test case for sslmode=require&sslrootcert=... to pin the round-trip behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/database/postgres/migrations/migrate.go` around lines 547 - 578,
Clarify the sslModeFromTLSConfig mapping documentation so the require case
explicitly excludes configurations with a root certificate, since pgx maps
require plus sslrootcert to verify-ca. Add a focused test covering
sslmode=require with sslrootcert and assert that the round-trip preserves the
resulting verify-ca mode.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@internal/database/postgres/migrations/migrate.go`:
- Around line 532-544: Update the migration DSN construction in the shown
migration connection flow to preserve the TLS root certificate/CA source
alongside sslmode. Thread the configured CA path into the generated postgres URL
using lib/pq’s sslrootcert option, while retaining the existing behavior for
connections without a configured CA.

---

Nitpick comments:
In `@internal/database/postgres/migrations/migrate_security_test.go`:
- Around line 136-154: Extend TestBuildMigrateDSN_PreservesSSLMode with a
subtest or table case combining sslmode=require and an sslrootcert value, then
assert that buildMigrateDSN preserves the resulting pgx mapping as
sslmode=verify-ca. Use a valid certificate path or test fixture accepted by
pgxpool.ParseConfig, and retain the existing bare-mode coverage.

In `@internal/database/postgres/migrations/migrate.go`:
- Around line 547-578: Clarify the sslModeFromTLSConfig mapping documentation so
the require case explicitly excludes configurations with a root certificate,
since pgx maps require plus sslrootcert to verify-ca. Add a focused test
covering sslmode=require with sslrootcert and assert that the round-trip
preserves the resulting verify-ca mode.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d6016c00-ea3e-4329-b8ee-f33c90c70d75

📥 Commits

Reviewing files that changed from the base of the PR and between f3134a7 and a32b927.

📒 Files selected for processing (2)

• internal/database/postgres/migrations/migrate.go
• internal/database/postgres/migrations/migrate_security_test.go