sqldb: guard native-SQL migration path against silent downgrades

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

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

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