Skip to content

Commit c30900d

Browse files
authored
fix: use superuser DSN for applying migrations (#1215)
* fix: use superuser DSN for applying migrations The migration runner was using buildServiceDSN which replaced superuser credentials with per-service user credentials that have empty passwords. PostgreSQL with scram-sha-256 authentication rejects passwordless connections, causing authentication failures. Add buildSuperuserDSN that preserves superuser credentials while only changing the target database name. Update runMigrations to use it. Per-service users are still created in provisionDatabases for future RBAC enforcement. Closes #1214 * fix: increase outer context timeout in scram-sha-256 test Bump from 120s to 180s to leave sufficient time for RunMigrations after container startup (which waits up to 90s). --------- Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
1 parent b0b93fc commit c30900d

4 files changed

Lines changed: 207 additions & 3 deletions

File tree

internal/migrations/driver_test.go

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
package migrations_test
22

33
import (
4+
"net/url"
45
"strings"
56
"testing"
67

78
"github.com/meridianhub/meridian/internal/migrations"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
811
)
912

1013
func TestDriverFromEnv(t *testing.T) {
@@ -118,3 +121,101 @@ func TestBuildServiceDSN_DatabaseAndUser(t *testing.T) {
118121
t.Errorf("expected simple_protocol query param in DSN, got %q", got)
119122
}
120123
}
124+
125+
func TestBuildSuperuserDSN_PreservesCredentials(t *testing.T) {
126+
tests := []struct {
127+
name string
128+
superDSN string
129+
dbName string
130+
driver migrations.Driver
131+
wantUser string
132+
wantPass string
133+
wantDB string
134+
wantPort string
135+
}{
136+
{
137+
name: "CockroachDB preserves root credentials",
138+
superDSN: "postgres://root:secretpass@localhost:26257/defaultdb?sslmode=disable",
139+
dbName: "meridian_tenant",
140+
driver: migrations.DriverCockroachDB,
141+
wantUser: "root",
142+
wantPass: "secretpass",
143+
wantDB: "meridian_tenant",
144+
wantPort: "26257",
145+
},
146+
{
147+
name: "PostgreSQL preserves postgres credentials",
148+
superDSN: "postgres://postgres:pgpass@db.example.com:5432/postgres",
149+
dbName: "meridian_party",
150+
driver: migrations.DriverPostgres,
151+
wantUser: "postgres",
152+
wantPass: "pgpass",
153+
wantDB: "meridian_party",
154+
wantPort: "5432",
155+
},
156+
{
157+
name: "adds default CockroachDB port when missing",
158+
superDSN: "postgres://root@localhost/defaultdb",
159+
dbName: "meridian_platform",
160+
driver: migrations.DriverCockroachDB,
161+
wantUser: "root",
162+
wantDB: "meridian_platform",
163+
wantPort: "26257",
164+
},
165+
{
166+
name: "adds default PostgreSQL port when missing",
167+
superDSN: "postgres://postgres:pass@localhost/postgres",
168+
dbName: "meridian_current_account",
169+
driver: migrations.DriverPostgres,
170+
wantUser: "postgres",
171+
wantPass: "pass",
172+
wantDB: "meridian_current_account",
173+
wantPort: "5432",
174+
},
175+
}
176+
177+
for _, tt := range tests {
178+
t.Run(tt.name, func(t *testing.T) {
179+
result := migrations.BuildSuperuserDSN(tt.superDSN, tt.dbName, tt.driver)
180+
181+
parsed, err := url.Parse(result)
182+
require.NoError(t, err)
183+
184+
assert.Equal(t, tt.wantUser, parsed.User.Username(), "user mismatch")
185+
if tt.wantPass != "" {
186+
pass, _ := parsed.User.Password()
187+
assert.Equal(t, tt.wantPass, pass, "password mismatch")
188+
}
189+
assert.Equal(t, "/"+tt.wantDB, parsed.Path, "database mismatch")
190+
assert.Equal(t, tt.wantPort, parsed.Port(), "port mismatch")
191+
assert.Equal(t, "simple_protocol", parsed.Query().Get("default_query_exec_mode"))
192+
})
193+
}
194+
}
195+
196+
func TestBuildSuperuserDSN_DiffersFromServiceDSN(t *testing.T) {
197+
superDSN := "postgres://postgres:secretpass@localhost:5432/postgres"
198+
sdb := migrations.ServiceDatabase{
199+
Database: "meridian_party",
200+
User: "meridian_party_user",
201+
Password: "",
202+
}
203+
204+
serviceDSN := migrations.BuildServiceDSN(superDSN, sdb, migrations.DriverPostgres)
205+
superuserTargetDSN := migrations.BuildSuperuserDSN(superDSN, "meridian_party", migrations.DriverPostgres)
206+
207+
// Service DSN uses service user (passwordless).
208+
serviceParsed, err := url.Parse(serviceDSN)
209+
require.NoError(t, err)
210+
assert.Equal(t, "meridian_party_user", serviceParsed.User.Username())
211+
_, hasPass := serviceParsed.User.Password()
212+
assert.False(t, hasPass, "service DSN should not have password")
213+
214+
// Superuser DSN preserves superuser credentials.
215+
superParsed, err := url.Parse(superuserTargetDSN)
216+
require.NoError(t, err)
217+
assert.Equal(t, "postgres", superParsed.User.Username())
218+
pass, hasPass := superParsed.User.Password()
219+
assert.True(t, hasPass, "superuser DSN should preserve password")
220+
assert.Equal(t, "secretpass", pass)
221+
}

internal/migrations/export_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,8 @@ package migrations
55
func BuildServiceDSN(superuserDSN string, sdb ServiceDatabase, driver Driver) string {
66
return buildServiceDSN(superuserDSN, sdb, driver)
77
}
8+
9+
// BuildSuperuserDSN is the exported test alias for buildSuperuserDSN.
10+
func BuildSuperuserDSN(superuserDSN string, dbName string, driver Driver) string {
11+
return buildSuperuserDSN(superuserDSN, dbName, driver)
12+
}

internal/migrations/runner.go

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,11 @@ func runMigrations(ctx context.Context, migrationFS fs.FS, superuserDSN string,
135135
// Group migrations by target database.
136136
byDB := groupByDatabase(migrations)
137137

138-
// Apply migrations to each database.
138+
// Apply migrations to each database using superuser credentials.
139+
// Service users may not have password-based auth configured (scram-sha-256),
140+
// so we preserve the superuser credentials and only change the database name.
139141
for dbName, dbMigrations := range byDB {
140-
sdb := dbMigrations[0].sdb
141-
dsn := buildServiceDSN(superuserDSN, sdb, driver)
142+
dsn := buildSuperuserDSN(superuserDSN, dbName, driver)
142143

143144
if err := applyDatabaseMigrations(ctx, dsn, dbName, dbMigrations, driver, logger); err != nil {
144145
return fmt.Errorf("apply migrations to %s: %w", dbName, err)
@@ -502,6 +503,39 @@ func buildServiceDSN(superuserDSN string, sdb ServiceDatabase, driver Driver) st
502503
return parsed.String()
503504
}
504505

506+
// buildSuperuserDSN modifies a superuser DSN to target a specific database while
507+
// preserving the superuser credentials. This is used for applying migrations where
508+
// the service user may not have password-based authentication configured.
509+
//
510+
// The default port is 26257 for CockroachDB and 5432 for PostgreSQL.
511+
func buildSuperuserDSN(superuserDSN string, dbName string, driver Driver) string {
512+
parsed, err := url.Parse(superuserDSN)
513+
if err != nil {
514+
return superuserDSN
515+
}
516+
517+
// Preserve superuser credentials, only replace database.
518+
parsed.Path = "/" + dbName
519+
520+
// Ensure default port if not specified.
521+
if parsed.Port() == "" {
522+
defaultPort := "26257"
523+
if driver == DriverPostgres {
524+
defaultPort = "5432"
525+
}
526+
parsed.Host = parsed.Hostname() + ":" + defaultPort
527+
}
528+
529+
// Enable simple protocol so multi-statement migration SQL files work with pgx v5.
530+
q := parsed.Query()
531+
if q.Get("default_query_exec_mode") == "" {
532+
q.Set("default_query_exec_mode", "simple_protocol")
533+
}
534+
parsed.RawQuery = q.Encode()
535+
536+
return parsed.String()
537+
}
538+
505539
// quoteIdent wraps a SQL identifier in double quotes, escaping any embedded double quotes.
506540
func quoteIdent(s string) string {
507541
return `"` + strings.ReplaceAll(s, `"`, `""`) + `"`

internal/migrations/runner_postgres_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,70 @@ func TestRunMigrations_Postgres(t *testing.T) {
128128
}
129129
}
130130

131+
func TestRunMigrations_Postgres_ScramAuth(t *testing.T) {
132+
if os.Getenv("CI") == "" && testing.Short() {
133+
t.Skip("skipping integration test; use -short=false or set CI=true")
134+
}
135+
136+
t.Setenv("DB_DRIVER", "postgres")
137+
138+
ctx, cancel := context.WithTimeout(context.Background(), 180*time.Second)
139+
defer cancel()
140+
141+
// Start PostgreSQL WITHOUT trust auth (uses default scram-sha-256).
142+
// This simulates production where service users have no passwords.
143+
container, err := tcpostgres.Run(ctx,
144+
"postgres:16-alpine",
145+
tcpostgres.WithDatabase("defaultdb"),
146+
tcpostgres.WithUsername("postgres"),
147+
tcpostgres.WithPassword("secretpassword"),
148+
// NO POSTGRES_HOST_AUTH_METHOD=trust - uses default scram-sha-256
149+
testcontainers.WithWaitStrategy(
150+
wait.ForLog("database system is ready to accept connections").
151+
WithOccurrence(2).
152+
WithStartupTimeout(90*time.Second)),
153+
)
154+
if err != nil {
155+
t.Fatalf("start PostgreSQL container: %v", err)
156+
}
157+
defer func() {
158+
cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 10*time.Second)
159+
defer cleanupCancel()
160+
_ = container.Terminate(cleanupCtx)
161+
}()
162+
163+
connStr, err := container.ConnectionString(ctx, "sslmode=disable")
164+
if err != nil {
165+
t.Fatalf("get PostgreSQL connection string: %v", err)
166+
}
167+
168+
logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug}))
169+
testFS := testMigrationFS()
170+
171+
// This should succeed using superuser credentials for migrations.
172+
err = migrations.RunMigrations(ctx, testFS, connStr, logger)
173+
if err != nil {
174+
t.Fatalf("migrations should succeed with scram-sha-256 auth: %v", err)
175+
}
176+
177+
// Verify migrations were applied.
178+
caDSN := replaceDSNDatabasePG(t, connStr, "meridian_current_account")
179+
caConn, err := pgx.Connect(ctx, caDSN+"&default_query_exec_mode=simple_protocol")
180+
if err != nil {
181+
t.Fatalf("connect to meridian_current_account: %v", err)
182+
}
183+
defer caConn.Close(ctx)
184+
185+
var count int
186+
err = caConn.QueryRow(ctx, `SELECT count(*) FROM _meridian_migrations`).Scan(&count)
187+
if err != nil {
188+
t.Fatalf("count migrations: %v", err)
189+
}
190+
if count != 2 {
191+
t.Errorf("expected 2 migrations recorded, got %d", count)
192+
}
193+
}
194+
131195
func TestRunMigrations_Postgres_Idempotent(t *testing.T) {
132196
if os.Getenv("CI") == "" && testing.Short() {
133197
t.Skip("skipping integration test; use -short=false or set CI=true")

0 commit comments

Comments
 (0)