diff --git a/docs/release-notes/release-notes-0.22.0.md b/docs/release-notes/release-notes-0.22.0.md index 85626b68f41..bff456ab68f 100644 --- a/docs/release-notes/release-notes-0.22.0.md +++ b/docs/release-notes/release-notes-0.22.0.md @@ -55,6 +55,15 @@ the reported network statistics such as total network capacity, channel count and max out degree. +* [Guarded the native-SQL store's migration path against silent + downgrades](https://github.com/lightningnetwork/lnd/pull/10964). If the + database's tracked migration version was ever higher than the versions + known to the running binary (e.g. an older `lnd` binary started against a + database already migrated forward by a newer release), every migration + would be silently skipped instead of erroring, letting `lnd` operate + against a schema it doesn't understand. `ApplyMigrations` now fails with + `ErrDBDowngrade` in that case. + # New Features ## Functional Enhancements diff --git a/sqldb/migrations.go b/sqldb/migrations.go index 241e5c0d683..30555c91c6b 100644 --- a/sqldb/migrations.go +++ b/sqldb/migrations.go @@ -142,6 +142,14 @@ var ( // match the original record. ErrMigrationMismatch = fmt.Errorf("migrated record does not match " + "original record") + + // ErrDBDowngrade is returned when the database's tracked migration + // version is higher than the highest version known to the running + // binary. This indicates the database was previously migrated + // forward by a newer lnd release, and the binary currently running + // is older than that release. + ErrDBDowngrade = fmt.Errorf("database version is newer than this " + + "binary supports, refusing to downgrade") ) // MigrationConfig is a configuration struct that describes SQL migrations. Each @@ -447,6 +455,29 @@ func ApplyMigrations(ctx context.Context, db *BaseDB, "(dirty=%v) as base version", currentVersion, dirty) } + // Guard against downgrades. If the database's tracked migration + // version is higher than anything this binary knows about, the + // database was already migrated forward by a newer lnd release and + // an older binary is now being run against it. Without this check, + // the loop below would treat every migration as "already applied" + // (since each migration's Version would be <= currentVersion) and + // silently skip all of them, letting lnd operate against a schema it + // doesn't understand and risking data corruption. We fail loudly + // instead. + // + // NOTE: highestKnownVersion equals len(migrations) because the + // version-ordering check above guarantees migrations are numbered + // contiguously starting at 1. + highestKnownVersion := len(migrations) + if currentVersion > highestKnownVersion { + return fmt.Errorf("%w: database is at migration version %d "+ + "but this binary only knows about migrations up to "+ + "version %d; this can happen if an older lnd binary "+ + "is started against a database that was already "+ + "migrated by a newer lnd release", + ErrDBDowngrade, currentVersion, highestKnownVersion) + } + // Due to an a migration issue in v0.19.0-rc1 we may be at version 2 and // have a dirty schema due to failing migration 3. If this is indeed the // case, we need to reset the dirty flag to be able to apply the fixed diff --git a/sqldb/migrations_test.go b/sqldb/migrations_test.go index 4e2b68e0ffa..7cb727b9d4c 100644 --- a/sqldb/migrations_test.go +++ b/sqldb/migrations_test.go @@ -618,6 +618,127 @@ func TestMigrationSucceedsAfterDirtyStateMigrationFailure19RC1(t *testing.T) { }) } +// TestApplyMigrationsRefusesDowngrade asserts that ApplyMigrations refuses to +// proceed and returns ErrDBDowngrade when the database's tracked migration +// version is higher than the highest version known to the migrations list +// passed in. This simulates an operator starting an older lnd binary against +// a database that was already migrated forward by a newer lnd release, which +// must fail loudly instead of silently skipping every migration and running +// against an unrecognized schema. +func TestApplyMigrationsRefusesDowngrade(t *testing.T) { + ctxb := t.Context() + + // A "newer" binary that knows about 3 migrations. + newerMigrations := []MigrationConfig{ + { + Name: "000001_invoices", + Version: 1, + SchemaVersion: 1, + }, + { + Name: "000002_amp_invoices", + Version: 2, + SchemaVersion: 2, + }, + { + Name: "000003_invoice_events", + Version: 3, + SchemaVersion: 3, + }, + } + + // An "older" binary that only knows about the first 2 migrations. + olderMigrations := newerMigrations[:2] + + t.Run("SQLite", func(t *testing.T) { + dbFileName := filepath.Join(t.TempDir(), "tmp.db") + + // The "newer" binary starts up first and migrates the + // database forward through all 3 versions it knows about. + dbNewer, err := NewSqliteStore(&SqliteConfig{ + SkipMigrations: false, + }, dbFileName) + require.NoError(t, err) + + require.NoError( + t, dbNewer.ApplyAllMigrations(ctxb, newerMigrations), + ) + + version, err := dbNewer.GetDatabaseVersion(ctxb) + require.NoError(t, err) + require.EqualValues(t, 3, version) + require.NoError(t, dbNewer.DB.Close()) + + // Now simulate a downgrade: an "older" binary that only + // knows about the first 2 migrations is started against the + // same, already-migrated database file. + dbOlder, err := NewSqliteStore(&SqliteConfig{ + SkipMigrations: false, + }, dbFileName) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, dbOlder.DB.Close()) + }) + + err = dbOlder.ApplyAllMigrations(ctxb, olderMigrations) + require.ErrorIs(t, err, ErrDBDowngrade) + + // The tracked version must remain untouched; no migration or + // schema change should have been attempted. + version, err = dbOlder.GetDatabaseVersion(ctxb) + require.NoError(t, err) + require.EqualValues(t, 3, version) + }) + + t.Run("Postgres", func(t *testing.T) { + fixture := NewTestPgFixture(t, DefaultPostgresFixtureLifetime) + t.Cleanup(func() { + fixture.TearDown(t) + }) + + dbName := randomDBName(t) + _, err := fixture.db.ExecContext( + ctxb, "CREATE DATABASE "+dbName, + ) + require.NoError(t, err) + + cfg := fixture.GetConfig(dbName) + cfg.SkipMigrations = false + + // The "newer" binary starts up first and migrates the + // database forward through all 3 versions it knows about. + dbNewer, err := NewPostgresStore(cfg) + require.NoError(t, err) + + require.NoError( + t, dbNewer.ApplyAllMigrations(ctxb, newerMigrations), + ) + + version, err := dbNewer.GetDatabaseVersion(ctxb) + require.NoError(t, err) + require.EqualValues(t, 3, version) + require.NoError(t, dbNewer.DB.Close()) + + // Now simulate a downgrade: an "older" binary that only + // knows about the first 2 migrations is started against the + // same, already-migrated database. + dbOlder, err := NewPostgresStore(cfg) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, dbOlder.DB.Close()) + }) + + err = dbOlder.ApplyAllMigrations(ctxb, olderMigrations) + require.ErrorIs(t, err, ErrDBDowngrade) + + // The tracked version must remain untouched; no migration or + // schema change should have been attempted. + version, err = dbOlder.GetDatabaseVersion(ctxb) + require.NoError(t, err) + require.EqualValues(t, 3, version) + }) +} + // TestMigrationConfigConsistency verifies that the migration configuration in // migrationConfig is consistent with the actual SQL schema files embedded in // the binary. This catches version collisions (e.g. two migrations claiming