Merge pull request #1319 from ViktorT-11/2026-06-sql-table-fixes

[sql-72] Preserve SQL ordering and uniqueness parity with kvdb

db/sqlc/actions_custom.go

@@ -175,7 +175,7 @@ func buildListActionsQuery(params ListActionsParams) (string, []interface{}) {
 	if params.Reversed {
 		order = "DESC"
 	}
-	query += " ORDER BY a.created_at " + order
+	query += " ORDER BY a.created_at " + order + ", a.id " + order
 
 	// Maybe paginate.
 	if params.Pagination != nil {

db/sqlc/migrations/000002_sessions.up.sql

@@ -84,9 +84,13 @@ CREATE TABLE IF NOT EXISTS session_macaroon_permissions (
     entity TEXT NOT NULL,
 
     -- The action that this permission is for.
-    action TEXT NOT NULL
+    action TEXT NOT NULL,
+
+    -- The original position of the permission in the session recipe.
+    position INTEGER NOT NULL
 );
-CREATE INDEX IF NOT EXISTS sessions_mac_perms_idx ON session_macaroon_permissions(session_id);
+CREATE INDEX IF NOT EXISTS sessions_mac_perms_idx
+    ON session_macaroon_permissions(session_id, position);
 
 -- The session_macaroon_caveats table contains the macaroon caveats that are
 -- associated with a session.
@@ -105,10 +109,14 @@ CREATE TABLE IF NOT EXISTS session_macaroon_caveats (
     verification_id BLOB,
 
     -- The location hint for third party caveats.
-    location TEXT
+    location TEXT,
+
+    -- The original position of the caveat in the session recipe.
+    position INTEGER NOT NULL
 );
 
-CREATE INDEX IF NOT EXISTS sessions_mac_caveats_idx ON session_macaroon_caveats(session_id);
+CREATE INDEX IF NOT EXISTS sessions_mac_caveats_idx
+    ON session_macaroon_caveats(session_id, position);
 
 -- The session_feature_configs table contains the feature configs that are
 -- associated with a session.

db/sqlc/migrations/000003_kvstores.down.sql

@@ -1,5 +1,7 @@
 -- Drop indexes first.
-DROP INDEX IF EXISTS kvstores_lookup_idx;
+DROP INDEX IF EXISTS kvstores_feature_lookup_idx;
+DROP INDEX IF EXISTS kvstores_group_lookup_idx;
+DROP INDEX IF EXISTS kvstores_global_lookup_idx;
 DROP INDEX IF EXISTS features_name_idx;
 DROP INDEX IF EXISTS rules_name_idx;
 

db/sqlc/migrations/000003_kvstores.up.sql

@@ -50,8 +50,29 @@ CREATE TABLE IF NOT EXISTS kvstores (
     entry_key TEXT NOT NULL,
 
     -- The value of the entry.
-    value BLOB NOT NULL
+    value BLOB NOT NULL,
+
+    -- Feature-scoped kv stores must always belong to a session group.
+    CHECK (feature_id IS NULL OR group_id IS NOT NULL)
 );
 
-CREATE UNIQUE INDEX IF NOT EXISTS kvstores_lookup_idx
-    ON kvstores (entry_key, rule_id, perm, group_id, feature_id);
+-- Mirror the legacy KVDB namespace semantics precisely. A kv store record is
+-- uniquely identified by one of three namespace shapes:
+--   1. Global: entry_key + rule_id + perm
+--   2. Group scoped: entry_key + rule_id + perm + group_id
+--   3. Feature scoped: entry_key + rule_id + perm + group_id + feature_id
+--
+-- A single UNIQUE index across nullable columns is not sufficient here, as
+-- SQL NULL handling can allow duplicates for the global and group-scoped
+-- cases that the KVDB bucket layout would never permit.
+CREATE UNIQUE INDEX IF NOT EXISTS kvstores_global_lookup_idx
+    ON kvstores (entry_key, rule_id, perm)
+    WHERE group_id IS NULL AND feature_id IS NULL;
+
+CREATE UNIQUE INDEX IF NOT EXISTS kvstores_group_lookup_idx
+    ON kvstores (entry_key, rule_id, perm, group_id)
+    WHERE group_id IS NOT NULL AND feature_id IS NULL;
+
+CREATE UNIQUE INDEX IF NOT EXISTS kvstores_feature_lookup_idx
+    ON kvstores (entry_key, rule_id, perm, group_id, feature_id)
+    WHERE feature_id IS NOT NULL;

db/sqlc/migrations/000005_actions.down.sql

@@ -1,5 +1,5 @@
-DROP INDEX IF NOT EXISTS actions_state_idx;
-DROP INDEX IF NOT EXISTS actions_session_id_idx;
-DROP INDEX IF NOT EXISTS actions_feature_name_idx;
-DROP INDEX IF NOT EXISTS actions_created_at_idx;
+DROP INDEX IF EXISTS actions_state_idx;
+DROP INDEX IF EXISTS actions_session_id_idx;
+DROP INDEX IF EXISTS actions_feature_name_idx;
+DROP INDEX IF EXISTS actions_created_at_id_idx;
 DROP TABLE IF EXISTS actions;

db/sqlc/migrations/000005_actions.up.sql

@@ -49,4 +49,8 @@ CREATE TABLE IF NOT EXISTS actions(
 CREATE INDEX IF NOT EXISTS actions_state_idx ON actions(action_state);
 CREATE INDEX IF NOT EXISTS actions_session_id_idx ON actions(session_id);
 CREATE INDEX IF NOT EXISTS actions_feature_name_idx ON actions(feature_name);
-CREATE INDEX IF NOT EXISTS actions_created_at_idx ON actions(created_at);
+-- Actions are commonly queried in chronological order. Include the primary key
+-- as a stable tie-breaker so callers can order by (created_at, id) and still
+-- get deterministic results when multiple actions share the same timestamp.
+CREATE INDEX IF NOT EXISTS actions_created_at_id_idx
+    ON actions(created_at, id);

db/sqlc/models.go

@@ -113,6 +113,7 @@ type SessionMacaroonCaveat struct {
 	CaveatID       []byte
 	VerificationID []byte
 	Location       sql.NullString
+	Position       int32
 }
 
 type SessionMacaroonPermission struct {
@@ -120,6 +121,7 @@ type SessionMacaroonPermission struct {
 	SessionID int64
 	Entity    string
 	Action    string
+	Position  int32
 }
 
 type SessionPrivacyFlag struct {

db/sqlc/queries/sessions.sql

@@ -81,25 +81,27 @@ WHERE id = $2;
 
 -- name: InsertSessionMacaroonPermission :exec
 INSERT INTO session_macaroon_permissions (
-    session_id, entity, action
-) VALUES (
-    $1, $2, $3
-);
-
--- name: GetSessionMacaroonPermissions :many
-SELECT * FROM session_macaroon_permissions
-WHERE session_id = $1;
-
--- name: InsertSessionMacaroonCaveat :exec
-INSERT INTO session_macaroon_caveats (
-    session_id, caveat_id, verification_id, location
+    session_id, entity, action, position
 ) VALUES (
     $1, $2, $3, $4
 );
 
+-- name: GetSessionMacaroonPermissions :many
+SELECT * FROM session_macaroon_permissions
+WHERE session_id = $1
+ORDER BY position ASC;
+
+-- name: InsertSessionMacaroonCaveat :exec
+INSERT INTO session_macaroon_caveats (
+    session_id, caveat_id, verification_id, location, position
+) VALUES (
+    $1, $2, $3, $4, $5
+);
+
 -- name: GetSessionMacaroonCaveats :many
 SELECT * FROM session_macaroon_caveats
-WHERE session_id = $1;
+WHERE session_id = $1
+ORDER BY position ASC;
 
 -- name: InsertSessionFeatureConfig :exec
 INSERT INTO session_feature_configs (
@@ -121,4 +123,4 @@ INSERT INTO session_privacy_flags (
 
 -- name: GetSessionPrivacyFlags :many
 SELECT * FROM session_privacy_flags
-WHERE session_id = $1;
+WHERE session_id = $1;

db/sqlc/sessions.sql.go

@@ -205,8 +205,9 @@ func (q *Queries) GetSessionIDByAlias(ctx context.Context, alias []byte) (int64,
 }
 
 const getSessionMacaroonCaveats = `-- name: GetSessionMacaroonCaveats :many
-SELECT id, session_id, caveat_id, verification_id, location FROM session_macaroon_caveats
+SELECT id, session_id, caveat_id, verification_id, location, position FROM session_macaroon_caveats
 WHERE session_id = $1
+ORDER BY position ASC
 `
 
 func (q *Queries) GetSessionMacaroonCaveats(ctx context.Context, sessionID int64) ([]SessionMacaroonCaveat, error) {
@@ -224,6 +225,7 @@ func (q *Queries) GetSessionMacaroonCaveats(ctx context.Context, sessionID int64
 			&i.CaveatID,
 			&i.VerificationID,
 			&i.Location,
+			&i.Position,
 		); err != nil {
 			return nil, err
 		}
@@ -239,8 +241,9 @@ func (q *Queries) GetSessionMacaroonCaveats(ctx context.Context, sessionID int64
 }
 
 const getSessionMacaroonPermissions = `-- name: GetSessionMacaroonPermissions :many
-SELECT id, session_id, entity, action FROM session_macaroon_permissions
+SELECT id, session_id, entity, action, position FROM session_macaroon_permissions
 WHERE session_id = $1
+ORDER BY position ASC
 `
 
 func (q *Queries) GetSessionMacaroonPermissions(ctx context.Context, sessionID int64) ([]SessionMacaroonPermission, error) {
@@ -257,6 +260,7 @@ func (q *Queries) GetSessionMacaroonPermissions(ctx context.Context, sessionID i
 			&i.SessionID,
 			&i.Entity,
 			&i.Action,
+			&i.Position,
 		); err != nil {
 			return nil, err
 		}
@@ -422,9 +426,9 @@ func (q *Queries) InsertSessionFeatureConfig(ctx context.Context, arg InsertSess
 
 const insertSessionMacaroonCaveat = `-- name: InsertSessionMacaroonCaveat :exec
 INSERT INTO session_macaroon_caveats (
-    session_id, caveat_id, verification_id, location
+    session_id, caveat_id, verification_id, location, position
 ) VALUES (
-    $1, $2, $3, $4
+    $1, $2, $3, $4, $5
 )
 `
 
@@ -433,6 +437,7 @@ type InsertSessionMacaroonCaveatParams struct {
 	CaveatID       []byte
 	VerificationID []byte
 	Location       sql.NullString
+	Position       int32
 }
 
 func (q *Queries) InsertSessionMacaroonCaveat(ctx context.Context, arg InsertSessionMacaroonCaveatParams) error {
@@ -441,15 +446,16 @@ func (q *Queries) InsertSessionMacaroonCaveat(ctx context.Context, arg InsertSes
 		arg.CaveatID,
 		arg.VerificationID,
 		arg.Location,
+		arg.Position,
 	)
 	return err
 }
 
 const insertSessionMacaroonPermission = `-- name: InsertSessionMacaroonPermission :exec
 INSERT INTO session_macaroon_permissions (
-    session_id, entity, action
+    session_id, entity, action, position
 ) VALUES (
-    $1, $2, $3
+    $1, $2, $3, $4
 )
 `
 
@@ -457,10 +463,16 @@ type InsertSessionMacaroonPermissionParams struct {
 	SessionID int64
 	Entity    string
 	Action    string
+	Position  int32
 }
 
 func (q *Queries) InsertSessionMacaroonPermission(ctx context.Context, arg InsertSessionMacaroonPermissionParams) error {
-	_, err := q.db.ExecContext(ctx, insertSessionMacaroonPermission, arg.SessionID, arg.Entity, arg.Action)
+	_, err := q.db.ExecContext(ctx, insertSessionMacaroonPermission,
+		arg.SessionID,
+		arg.Entity,
+		arg.Action,
+		arg.Position,
+	)
 	return err
 }
 

db/sqlcmig6/actions_custom.go

@@ -175,7 +175,7 @@ func buildListActionsQuery(params ListActionsParams) (string, []interface{}) {
 	if params.Reversed {
 		order = "DESC"
 	}
-	query += " ORDER BY a.created_at " + order
+	query += " ORDER BY a.created_at " + order + ", a.id " + order
 
 	// Maybe paginate.
 	if params.Pagination != nil {

db/sqlcmig6/models.go

@@ -109,6 +109,7 @@ type SessionMacaroonCaveat struct {
 	CaveatID       []byte
 	VerificationID []byte
 	Location       sql.NullString
+	Position       int64
 }
 
 type SessionMacaroonPermission struct {
@@ -116,6 +117,7 @@ type SessionMacaroonPermission struct {
 	SessionID int64
 	Entity    string
 	Action    string
+	Position  int64
 }
 
 type SessionPrivacyFlag struct {

db/sqlcmig6/sessions.sql.go

@@ -200,8 +200,9 @@ func (q *Queries) GetSessionIDByAlias(ctx context.Context, alias []byte) (int64,
 }
 
 const getSessionMacaroonCaveats = `-- name: GetSessionMacaroonCaveats :many
-SELECT id, session_id, caveat_id, verification_id, location FROM session_macaroon_caveats
+SELECT id, session_id, caveat_id, verification_id, location, position FROM session_macaroon_caveats
 WHERE session_id = $1
+ORDER BY position ASC
 `
 
 func (q *Queries) GetSessionMacaroonCaveats(ctx context.Context, sessionID int64) ([]SessionMacaroonCaveat, error) {
@@ -219,6 +220,7 @@ func (q *Queries) GetSessionMacaroonCaveats(ctx context.Context, sessionID int64
 			&i.CaveatID,
 			&i.VerificationID,
 			&i.Location,
+			&i.Position,
 		); err != nil {
 			return nil, err
 		}
@@ -234,8 +236,9 @@ func (q *Queries) GetSessionMacaroonCaveats(ctx context.Context, sessionID int64
 }
 
 const getSessionMacaroonPermissions = `-- name: GetSessionMacaroonPermissions :many
-SELECT id, session_id, entity, action FROM session_macaroon_permissions
+SELECT id, session_id, entity, action, position FROM session_macaroon_permissions
 WHERE session_id = $1
+ORDER BY position ASC
 `
 
 func (q *Queries) GetSessionMacaroonPermissions(ctx context.Context, sessionID int64) ([]SessionMacaroonPermission, error) {
@@ -252,6 +255,7 @@ func (q *Queries) GetSessionMacaroonPermissions(ctx context.Context, sessionID i
 			&i.SessionID,
 			&i.Entity,
 			&i.Action,
+			&i.Position,
 		); err != nil {
 			return nil, err
 		}
@@ -417,9 +421,9 @@ func (q *Queries) InsertSessionFeatureConfig(ctx context.Context, arg InsertSess
 
 const insertSessionMacaroonCaveat = `-- name: InsertSessionMacaroonCaveat :exec
 INSERT INTO session_macaroon_caveats (
-    session_id, caveat_id, verification_id, location
+    session_id, caveat_id, verification_id, location, position
 ) VALUES (
-    $1, $2, $3, $4
+    $1, $2, $3, $4, $5
 )
 `
 
@@ -428,6 +432,7 @@ type InsertSessionMacaroonCaveatParams struct {
 	CaveatID       []byte
 	VerificationID []byte
 	Location       sql.NullString
+	Position       int64
 }
 
 func (q *Queries) InsertSessionMacaroonCaveat(ctx context.Context, arg InsertSessionMacaroonCaveatParams) error {
@@ -436,15 +441,16 @@ func (q *Queries) InsertSessionMacaroonCaveat(ctx context.Context, arg InsertSes
 		arg.CaveatID,
 		arg.VerificationID,
 		arg.Location,
+		arg.Position,
 	)
 	return err
 }
 
 const insertSessionMacaroonPermission = `-- name: InsertSessionMacaroonPermission :exec
 INSERT INTO session_macaroon_permissions (
-    session_id, entity, action
+    session_id, entity, action, position
 ) VALUES (
-    $1, $2, $3
+    $1, $2, $3, $4
 )
 `
 
@@ -452,10 +458,13 @@ type InsertSessionMacaroonPermissionParams struct {
 	SessionID int64
 	Entity    string
 	Action    string
+	Position  int64
 }
 
 func (q *Queries) InsertSessionMacaroonPermission(ctx context.Context, arg InsertSessionMacaroonPermissionParams) error {
-	_, err := q.db.ExecContext(ctx, insertSessionMacaroonPermission, arg.SessionID, arg.Entity, arg.Action)
+	_, err := q.db.ExecContext(ctx, insertSessionMacaroonPermission,
+		arg.SessionID, arg.Entity, arg.Action, arg.Position,
+	)
 	return err
 }
 

firewalldb/sql_migration.go

@@ -930,32 +930,42 @@ func migrateActionsToSQL(ctx context.Context, kvStore *bbolt.DB,
 			return fmt.Errorf("actions bucket not found")
 		}
 
+		actionsIndexBucket := actionsBucket.Bucket(actionsIndex)
+		if actionsIndexBucket == nil {
+			return fmt.Errorf("actions->actions-index bucket not " +
+				"found")
+		}
+
 		sessionsBucket := actionsBucket.Bucket(actionsKey)
 		if sessionsBucket == nil {
 			return fmt.Errorf("actions->sessions bucket not found")
 		}
 
-		// Iterate over session ID buckets (i.e. what we should name
-		// macaroon IDs).
-		//
-		// nolint:ll
-		return sessionsBucket.ForEach(func(macID []byte, v []byte) error {
-			if v != nil {
-				return fmt.Errorf("expected only sub-buckets " +
-					"in sessions bucket")
+		// Iterate over the global actions index so that SQL insertion
+		// order follows the legacy KVDB action order rather than bucket
+		// traversal order.
+		return actionsIndexBucket.ForEach(func(seqNo []byte,
+			locatorBytes []byte) error {
+
+			if locatorBytes == nil {
+				return fmt.Errorf("unexpected nested bucket " +
+					"under actions-index")
 			}
 
-			sessBucket := sessionsBucket.Bucket(macID)
-			if sessBucket == nil {
-				return fmt.Errorf("session bucket for %x not "+
-					"found", macID)
+			locator, err := deserializeActionLocator(
+				bytes.NewReader(locatorBytes),
+			)
+			if err != nil {
+				return fmt.Errorf("unable to deserialize "+
+					"action locator for seq %x: %w",
+					seqNo, err)
 			}
 
 			// fetch the full macaroon root key ID based on the
 			// macaroon identifier for the action (the last 4 bytes
 			// of the root key ID).
 			var macIDArr [4]byte
-			copy(macIDArr[:], macID)
+			copy(macIDArr[:], locator.sessionID[:])
 
 			macRootKeyID, ok := macMap[macIDArr]
 			if !ok {
@@ -970,65 +980,43 @@ func migrateActionsToSQL(ctx context.Context, kvStore *bbolt.DB,
 				// backends.
 				log.Warnf("No macaroon root key ID found for "+
 					"macaroon ID %x, using zeroes for "+
-					"the first 4 bytes", macID)
+					"the first 4 bytes",
+					locator.sessionID[:])
 
 				macRootKeyID = make([]byte, 8)
 				copy(macRootKeyID[4:], macIDArr[:])
 			}
 
-			// Iterate over the actions inside each session/macaroon
-			// ID.
-			return sessBucket.ForEach(func(actionID,
-				actionBytes []byte) error {
+			action, err := getAction(sessionsBucket, locator)
+			if err != nil {
+				return fmt.Errorf("unable to deserialize "+
+					"action for locator %+v: %w",
+					locator, err)
+			}
 
-				if actionBytes == nil {
-					return fmt.Errorf("unexpected nested "+
-						"bucket under session %x",
-						macID)
-				}
+			log.Tracef("Migrated Action: SeqNo: %x, Macaroon ID: "+
+				"%x, ActionID: %d, Actor: %s, Feature: %s",
+				seqNo, locator.sessionID[:], locator.actionID,
+				action.ActorName, action.FeatureName)
 
-				sessionID, err := session.IDFromBytes(macID)
-				if err != nil {
-					// This should be unreachable, as the
-					// macID should always be 4 bytes long.
-					return fmt.Errorf("invalid session ID "+
-						"format %x: %v", macID, err)
-				}
+			// Now proceed to migrate the action, and also validate
+			// that the action was correctly migrated.
+			err = migrateActionToSQL(
+				ctx, sqlTx, acctsMap, sessMap, action,
+				macRootKeyID,
+			)
+			if err != nil {
+				return fmt.Errorf("migrating action to SQL "+
+					"failed: %w", err)
+			}
 
-				action, err := DeserializeAction(
-					bytes.NewReader(actionBytes), sessionID,
-				)
-				if err != nil {
-					return fmt.Errorf("unable to "+
-						"deserialize action in "+
-						"session %x: %w", macID, err)
-				}
+			migCount++
+			if migCount%migrationProgressLogInterval == 0 {
+				log.Infof("Migrated %d actions from KV to SQL",
+					migCount)
+			}
 
-				log.Tracef("Migrated Action: Macaroon ID: %x, "+
-					"ActionID: %x, Actor: %s, Feature: %s",
-					macID, actionID, action.ActorName,
-					action.FeatureName)
-
-				// Now proceed to migrate the action, and also
-				// validate that the action was correctly
-				// migrated.
-				err = migrateActionToSQL(
-					ctx, sqlTx, acctsMap, sessMap, action,
-					macRootKeyID,
-				)
-				if err != nil {
-					return fmt.Errorf("migrating action "+
-						"to SQL failed: %w", err)
-				}
-
-				migCount++
-				if migCount%migrationProgressLogInterval == 0 {
-					log.Infof("Migrated %d actions from "+
-						"KV to SQL", migCount)
-				}
-
-				return nil
-			})
+			return nil
 		})
 	})
 	if err != nil {

firewalldb/sql_migration_test.go

@@ -479,6 +479,10 @@ func TestFirewallDBMigration(t *testing.T) {
 			name:       "action with no session or account",
 			populateDB: actionNoSessionOrAccount,
 		},
+		{
+			name:       "actions with same timestamp",
+			populateDB: actionsWithSameTimestamp,
+		},
 		{
 			name:       "action with empty RPCParamsJson",
 			populateDB: actionEmptyRPCParamsJson,
@@ -1281,6 +1285,36 @@ func actionNoSessionOrAccount(t *testing.T, ctx context.Context,
 	}
 }
 
+// actionsWithSameTimestamp adds multiple actions at the same timestamp and
+// returns them in insertion order. This makes sure migration validation also
+// covers the SQL (created_at, id) tie-breaker.
+func actionsWithSameTimestamp(t *testing.T, ctx context.Context,
+	boltDB *BoltDB, _ session.Store, _ accounts.Store,
+	rStore *rootKeyMockStore) *expectedResult {
+
+	actions := make([]*Action, 0, 3)
+	for i := range 3 {
+		rootKey := rStore.addRandomRootKey()
+
+		actionReq := testActionReq
+		actionReq.MacaroonRootKeyID = fn.Some(rootKey)
+		actionReq.SessionID = fn.None[session.ID]()
+		actionReq.AccountID = fn.None[accounts.AccountID]()
+		actionReq.FeatureName = fmt.Sprintf("same-time-%d", i+1)
+
+		actions = append(
+			actions,
+			addActionAtCurrentTime(t, ctx, boltDB, &actionReq),
+		)
+	}
+
+	return &expectedResult{
+		kvEntries: []*kvEntry{},
+		privPairs: make(privacyPairs),
+		actions:   actions,
+	}
+}
+
 // actionEmptyRPCParamsJson adds an action which has no RPCParamsJson set.
 func actionEmptyRPCParamsJson(t *testing.T, ctx context.Context,
 	boltDB *BoltDB, _ session.Store, _ accounts.Store,
@@ -1986,6 +2020,14 @@ func addAction(t *testing.T, ctx context.Context, boltDB *BoltDB,
 	// of a session or account that it might be linked to.
 	boltDB.clock = clock.NewTestClock(boltDB.clock.Now().Add(time.Second))
 
+	return addActionAtCurrentTime(t, ctx, boltDB, actionReq)
+}
+
+// addActionAtCurrentTime adds an action using the bolt DB's current clock
+// value without advancing it first.
+func addActionAtCurrentTime(t *testing.T, ctx context.Context, boltDB *BoltDB,
+	actionReq *AddActionReq) *Action {
+
 	aLocator, err := boltDB.AddAction(ctx, actionReq)
 	require.NoError(t, err)
 

session/sql_migration.go

@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
-	"sort"
 	"time"
 
 	"github.com/btcsuite/btcd/btcec/v2"
@@ -547,12 +546,13 @@ func migrateSingleSessionToSQL(ctx context.Context, tx *s6.Queries,
 	// into the linked child tables.
 	if session.MacaroonRecipe != nil {
 		// We start by inserting the macaroon permissions.
-		for _, sessionPerm := range session.MacaroonRecipe.Permissions {
+		for i, sessionPerm := range session.MacaroonRecipe.Permissions {
 			err = tx.InsertSessionMacaroonPermission(
 				ctx, s6.InsertSessionMacaroonPermissionParams{
 					SessionID: sqlId,
 					Entity:    sessionPerm.Entity,
 					Action:    sessionPerm.Action,
+					Position:  int64(i),
 				},
 			)
 			if err != nil {
@@ -561,7 +561,7 @@ func migrateSingleSessionToSQL(ctx context.Context, tx *s6.Queries,
 		}
 
 		// Next we insert the macaroon caveats.
-		for _, caveat := range session.MacaroonRecipe.Caveats {
+		for i, caveat := range session.MacaroonRecipe.Caveats {
 			err = tx.InsertSessionMacaroonCaveat(
 				ctx, s6.InsertSessionMacaroonCaveatParams{
 					SessionID:      sqlId,
@@ -570,6 +570,7 @@ func migrateSingleSessionToSQL(ctx context.Context, tx *s6.Queries,
 					Location: sqldb.SQLStr(
 						caveat.Location,
 					),
+					Position: int64(i),
 				},
 			)
 			if err != nil {
@@ -650,15 +651,18 @@ func overrideSessionTimeZone(session *Session) {
 // as nil in the bbolt store. Therefore, we also override the permissions
 // or caveats to nil for the migrated session in that scenario, so that the
 // deep equals check does not fail in this scenario either.
-//
-// Additionally, we sort the caveats & permissions of both the kv and sql
-// sessions by their ID, so that they are always comparable in a deterministic
-// way with deep equals.
 func overrideMacaroonRecipe(kvSession *Session, migratedSession *Session) {
 	if kvSession.MacaroonRecipe != nil {
 		kvPerms := kvSession.MacaroonRecipe.Permissions
 		kvCaveats := kvSession.MacaroonRecipe.Caveats
 
+		// If the migratedSession.MacaroonRecipe is nil, we set it to
+		// an empty MacaroonRecipe, as that can be correct when both
+		// kvPerms and kvCaveats are nil.
+		if migratedSession.MacaroonRecipe == nil {
+			migratedSession.MacaroonRecipe = &MacaroonRecipe{}
+		}
+
 		// If the kvSession has a MacaroonRecipe with nil set for any
 		// of the fields, we need to override the migratedSession
 		// MacaroonRecipe to match that.
@@ -671,30 +675,10 @@ func overrideMacaroonRecipe(kvSession *Session, migratedSession *Session) {
 		}
 
 		sqlCaveats := migratedSession.MacaroonRecipe.Caveats
-		sqlPerms := migratedSession.MacaroonRecipe.Permissions
-
-		// If there have been caveats set for the MacaroonRecipe,
-		// the order of the postgres db caveats will in very rare cases
-		// differ from the kv store caveats. Therefore, we sort
-		// both the kv and sql caveats by their ID, so that we can
-		// compare them in a deterministic way.
-		if kvCaveats != nil {
-			sort.Slice(kvCaveats, func(i, j int) bool {
-				return bytes.Compare(
-					kvCaveats[i].Id, kvCaveats[j].Id,
-				) < 0
-			})
-
-			sort.Slice(sqlCaveats, func(i, j int) bool {
-				return bytes.Compare(
-					sqlCaveats[i].Id, sqlCaveats[j].Id,
-				) < 0
-			})
-		}
 
 		// Empty caveat verification IDs can be persisted as nil by SQL
 		// backends, while the KV store can retain them as empty slices.
-		// After sorting, we only normalize caveats that still line up
+		// We only normalize caveats that still line up
 		// by ID. If the lengths or IDs differ, we leave the slices
 		// as-is and let the subsequent DeepEqual report the migration
 		// mismatch, hence let the migration fail.
@@ -718,29 +702,6 @@ func overrideMacaroonRecipe(kvSession *Session, migratedSession *Session) {
 				}
 			}
 		}
-
-		// Similarly, we sort the macaroon permissions for both the kv
-		// and sql sessions, so that we can compare them in a
-		// deterministic way.
-		if kvPerms != nil {
-			sort.Slice(kvPerms, func(i, j int) bool {
-				if kvPerms[i].Entity == kvPerms[j].Entity {
-					return kvPerms[i].Action <
-						kvPerms[j].Action
-				}
-
-				return kvPerms[i].Entity < kvPerms[j].Entity
-			})
-
-			sort.Slice(sqlPerms, func(i, j int) bool {
-				if sqlPerms[i].Entity == sqlPerms[j].Entity {
-					return sqlPerms[i].Action <
-						sqlPerms[j].Action
-				}
-
-				return sqlPerms[i].Entity < sqlPerms[j].Entity
-			})
-		}
 	}
 }
 

session/sql_store.go

@@ -240,13 +240,14 @@ func (s *SQLStore) NewSession(ctx context.Context, label string, typ Type,
 
 		// Write mac perms and caveats.
 		if sess.MacaroonRecipe != nil {
-			for _, perm := range sess.MacaroonRecipe.Permissions {
+			for i, perm := range sess.MacaroonRecipe.Permissions {
 				// nolint:ll
 				err := db.InsertSessionMacaroonPermission(
 					ctx, sqlc.InsertSessionMacaroonPermissionParams{
 						SessionID: dbID,
 						Entity:    perm.Entity,
 						Action:    perm.Action,
+						Position:  int32(i),
 					},
 				)
 				if err != nil {
@@ -255,7 +256,7 @@ func (s *SQLStore) NewSession(ctx context.Context, label string, typ Type,
 				}
 			}
 
-			for _, caveat := range sess.MacaroonRecipe.Caveats {
+			for i, caveat := range sess.MacaroonRecipe.Caveats {
 				// nolint:ll
 				err := db.InsertSessionMacaroonCaveat(
 					ctx, sqlc.InsertSessionMacaroonCaveatParams{
@@ -268,6 +269,7 @@ func (s *SQLStore) NewSession(ctx context.Context, label string, typ Type,
 							Valid: caveat.
 								Location != "",
 						},
+						Position: int32(i),
 					},
 				)
 				if err != nil {