From 3d1057228049ebeef0efa7113909aa0b2e1fb2e2 Mon Sep 17 00:00:00 2001
From: Viktor Torstensson <viktor.t.git@gmail.com>
Date: Sun, 26 Oct 2025 22:29:00 -0400
Subject: [PATCH] session: fix empty config for features in sql mig

In the bbolt store, an empty config for a feature is represented as an
empty array, while in for the SQL store, the same config is represented
as nil. Therefore, in the scenario where a specific feature has an empty
config, we override the SQL FeatureConfig for that feature to also be
set to an empty array. This is needed to ensure that the deep equals
check in the migration validation does not fail in this scenario.
---
 session/sql_migration.go      | 27 ++++++++++++++++
 session/sql_migration_test.go | 61 +++++++++++++++++++++++++++++++++--
 2 files changed, 85 insertions(+), 3 deletions(-)

diff --git a/session/sql_migration.go b/session/sql_migration.go
index 5e280d50c..676cd5b3d 100644
--- a/session/sql_migration.go
+++ b/session/sql_migration.go
@@ -198,6 +198,7 @@ func migrateSessionsToSQLAndValidate(ctx context.Context,
 		overrideSessionTimeZone(migratedSession)
 		overrideMacaroonRecipe(kvSession, migratedSession)
 		overrideRemovedAccount(kvSession, migratedSession)
+		overrideFeatureConfig(kvSession, migratedSession)
 
 		if !reflect.DeepEqual(kvSession, migratedSession) {
 			diff := difflib.UnifiedDiff{
@@ -530,3 +531,29 @@ func overrideRemovedAccount(kvSession *Session, migratedSession *Session) {
 		},
 	)
 }
+
+// overrideFeatureConfig overrides a specific feature's config for the SQL
+// session in a certain scenario:
+//
+// In the bbolt store, an empty config for a feature is represented as an empty
+// array, while in for the SQL store, the same config is represented as nil.
+// Therefore, in the scenario where a specific feature has an empty config, we
+// override the SQL FeatureConfig for that feature to also be set to an empty
+// array. This is needed to ensure that the deep equals check in the migration
+// validation does not fail in this scenario.
+func overrideFeatureConfig(kvSession *Session, mSession *Session) {
+	// If FeatureConfig is not set for both sessions, we return early.
+	if kvSession.FeatureConfig == nil || mSession.FeatureConfig == nil {
+		return
+	}
+
+	migratedConf := *mSession.FeatureConfig
+	for featureName, config := range *kvSession.FeatureConfig {
+		// If the config is empty for the bbolt feature, and nil for the
+		// SQL version, we override the SQL version to match the bbolt
+		// version.
+		if len(config) == 0 && migratedConf[featureName] == nil {
+			migratedConf[featureName] = make([]byte, 0)
+		}
+	}
+}
diff --git a/session/sql_migration_test.go b/session/sql_migration_test.go
index 1cb7ba751..fb8763808 100644
--- a/session/sql_migration_test.go
+++ b/session/sql_migration_test.go
@@ -81,6 +81,10 @@ func TestSessionsStoreMigration(t *testing.T) {
 			// session doesn't.
 			overrideRemovedAccount(kvSession, sqlSession)
 
+			// Finally, we also need to override specific feature's
+			// config if they are empty.
+			overrideFeatureConfig(kvSession, sqlSession)
+
 			assertEqualSessions(t, kvSession, sqlSession)
 		}
 
@@ -200,6 +204,48 @@ func TestSessionsStoreMigration(t *testing.T) {
 				return getBoltStoreSessions(t, store)
 			},
 		},
+		{
+			name: "one session with a feature config with empty",
+			populateDB: func(t *testing.T, store *BoltStore,
+				_ accounts.Store) []*Session {
+
+				// Set a feature config, which has an empty
+				// entry for a specific feature.
+				featureConfig := map[string][]byte{
+					"AutoFees": {},
+				}
+
+				_, err := store.NewSession(
+					ctx, "test", TypeMacaroonAdmin,
+					time.Unix(1000, 0), "",
+					WithFeatureConfig(featureConfig),
+				)
+				require.NoError(t, err)
+
+				return getBoltStoreSessions(t, store)
+			},
+		},
+		{
+			name: "one session with a feature config with nil",
+			populateDB: func(t *testing.T, store *BoltStore,
+				_ accounts.Store) []*Session {
+
+				// Set a feature config, which has a nil entry
+				// for a specific feature.
+				featureConfig := map[string][]byte{
+					"AutoFees": nil,
+				}
+
+				_, err := store.NewSession(
+					ctx, "test", TypeMacaroonAdmin,
+					time.Unix(1000, 0), "",
+					WithFeatureConfig(featureConfig),
+				)
+				require.NoError(t, err)
+
+				return getBoltStoreSessions(t, store)
+			},
+		},
 		{
 			name: "one session with dev server",
 			populateDB: func(t *testing.T, store *BoltStore,
@@ -801,9 +847,18 @@ func randomPrivacyFlags() PrivacyFlags {
 func randomFeatureConfig() FeaturesConfig {
 	featureConfig := make(FeaturesConfig)
 	for i := range rand.Intn(10) {
-		featureName := fmt.Sprintf("feature%d", i)
-		featureValue := []byte{byte(rand.Int31())}
-		featureConfig[featureName] = featureValue
+		var (
+			featureName = fmt.Sprintf("feature%d", i)
+			configValue []byte
+		)
+
+		// For the vast majority of the features, we set a value for the
+		// feature's config. But in 1/5 of the cases, we set an empty
+		// config.
+		if rand.Intn(5) != 0 {
+			configValue = []byte{byte(rand.Int31())}
+		}
+		featureConfig[featureName] = configValue
 	}
 
 	return featureConfig