fix(auth): close admin bootstrap race and cover store SQL paths

internal/auth/store_postgres.go

@@ -20,6 +20,7 @@ type DBConnection interface {
 	QueryRow(ctx context.Context, sql string, args ...any) pgx.Row
 	Query(ctx context.Context, sql string, args ...any) (pgx.Rows, error)
 	Exec(ctx context.Context, sql string, args ...any) (pgconn.CommandTag, error)
+	Begin(ctx context.Context) (pgx.Tx, error)
 	Ping(ctx context.Context) error
 }
 
@@ -353,6 +354,14 @@ func (s *PostgresStore) CountGroupMembers(ctx context.Context, groupID string) (
 	return count, nil
 }
 
+// adminInvariantAdvisoryLockKey is the transaction-scoped advisory lock key
+// serializing writes that affect the "at least/at most the right number of
+// admins" invariants. It MUST stay equal to the key used by
+// check_min_one_admin() in migration 000065 so bootstrap inserts and
+// admin-demoting commits serialize against each other as well as among
+// themselves.
+const adminInvariantAdvisoryLockKey = 8059058058580001
+
 // CreateAdminIfNone atomically inserts user as the first admin in the
 // system. Returns (true, nil) when the insert succeeded; (false, nil)
 // when an admin already existed (TOCTOU race — both callers passed
@@ -361,10 +370,18 @@ func (s *PostgresStore) CountGroupMembers(ctx context.Context, groupID string) (
 // for any other failure.
 //
 // The conditional INSERT closes the bootstrap race without the
-// users_one_admin partial unique index (dropped in migration 000050).
-// Postgres guarantees atomicity of the SELECT … WHERE NOT EXISTS …
-// INSERT in a single statement — no advisory lock or transaction is
-// needed.
+// users_one_admin partial unique index (dropped in migration 000050),
+// but a single INSERT … WHERE NOT EXISTS statement is NOT race-free on
+// its own: under READ COMMITTED each statement's snapshot is taken at
+// statement start, so two concurrent calls can both see "no admin" and
+// both insert (reproduced by
+// TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce). To close
+// that window the insert runs in a transaction that first takes the
+// transaction-scoped advisory lock shared with the min-one-admin
+// trigger (migration 000065): the second caller blocks until the first
+// commits, and its INSERT statement then takes a fresh snapshot that
+// sees the committed admin, so the NOT EXISTS guard suppresses the
+// duplicate. The lock auto-releases at COMMIT or ROLLBACK.
 func (s *PostgresStore) CreateAdminIfNone(ctx context.Context, user *User) (bool, error) {
 	if user.ID == "" {
 		user.ID = uuid.New().String()
@@ -411,7 +428,22 @@ func (s *PostgresStore) CreateAdminIfNone(ctx context.Context, user *User) (bool
 		groupIDs = append(append([]string(nil), groupIDs...), DefaultAdminGroupID)
 	}
 
-	tag, err := s.db.Exec(ctx, query,
+	tx, err := s.db.Begin(ctx)
+	if err != nil {
+		return false, fmt.Errorf("failed to begin admin bootstrap transaction: %w", err)
+	}
+	// Rollback is a no-op after a successful Commit; on any earlier return
+	// it also releases the advisory lock. Matches the project convention
+	// (e.g. internal/config/store_postgres.go) for deferred rollback.
+	defer tx.Rollback(ctx) //nolint:errcheck
+
+	// Serialize against concurrent bootstrap calls and against the
+	// min-one-admin deferred trigger (see adminInvariantAdvisoryLockKey).
+	if _, err = tx.Exec(ctx, "SELECT pg_advisory_xact_lock($1)", adminInvariantAdvisoryLockKey); err != nil {
+		return false, fmt.Errorf("failed to acquire admin bootstrap lock: %w", err)
+	}
+
+	tag, err := tx.Exec(ctx, query,
 		user.ID, user.Email, user.PasswordHash, user.Salt,
 		groupIDs, user.Active, user.MFAEnabled, user.MFASecret,
 		user.MFAPendingSecret, user.MFAPendingSecretExpiresAt, recoveryCodes,
@@ -431,6 +463,10 @@ func (s *PostgresStore) CreateAdminIfNone(ctx context.Context, user *User) (bool
 		return false, fmt.Errorf("failed to create admin: %w", err)
 	}
 
+	if err := tx.Commit(ctx); err != nil {
+		return false, fmt.Errorf("failed to commit admin bootstrap transaction: %w", err)
+	}
+
 	return tag.RowsAffected() > 0, nil
 }
 

internal/auth/store_postgres_db_test.go

@@ -0,0 +1,215 @@
+//go:build integration
+// +build integration
+
+package auth
+
+// store_postgres_db_test.go - DB-backed integration tests for the
+// security-critical PostgresStore bootstrap paths (TEST-01, issue #1153).
+// These run the real SQL against a migrated PostgreSQL testcontainer,
+// proving the 19-column bootstrap INSERT, the NOT EXISTS guard and its
+// agreement with AdminExists (active-membership semantics), and the
+// insert-once guarantee under two concurrent CreateAdminIfNone calls.
+
+import (
+	"context"
+	"fmt"
+	"path/filepath"
+	"runtime"
+	"testing"
+	"time"
+
+	"github.com/LeanerCloud/CUDly/internal/database"
+	"github.com/LeanerCloud/CUDly/internal/database/postgres/migrations"
+	"github.com/LeanerCloud/CUDly/internal/database/postgres/testhelpers"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// getAuthTestMigrationsPath returns the absolute path to the migrations directory.
+func getAuthTestMigrationsPath() string {
+	_, filename, _, _ := runtime.Caller(0)
+	return filepath.Join(filepath.Dir(filename), "..", "database", "postgres", "migrations")
+}
+
+// setupAuthTestDB starts a PostgreSQL container and runs all migrations
+// without seeding a bootstrap admin (empty adminEmail), so tests start from
+// the "fresh deployment, no admin yet" state CreateAdminIfNone exists for.
+func setupAuthTestDB(t *testing.T) *database.Connection {
+	t.Helper()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
+	defer cancel()
+
+	container, err := testhelpers.SetupPostgresContainer(ctx, t)
+	if err != nil {
+		t.Skipf("Skipping DB test: cannot start PostgreSQL container: %v", err)
+		return nil
+	}
+
+	if err := migrations.RunMigrations(ctx, container.DB.Pool(), getAuthTestMigrationsPath(), "", ""); err != nil {
+		container.Cleanup(ctx)
+		t.Skipf("Skipping DB test: cannot run migrations: %v", err)
+		return nil
+	}
+
+	t.Cleanup(func() {
+		container.Cleanup(context.Background())
+	})
+
+	return container.DB
+}
+
+// resetUsers wipes the users table between scenarios. TRUNCATE is used
+// deliberately: the trg_min_one_admin_delete row trigger (migration 000065)
+// would otherwise veto deleting the last admin, and TRUNCATE does not fire
+// row-level triggers. CASCADE clears dependent rows (sessions, api_keys).
+func resetUsers(t *testing.T, db *database.Connection) {
+	t.Helper()
+	_, err := db.Exec(context.Background(), "TRUNCATE users CASCADE")
+	require.NoError(t, err)
+}
+
+func newBootstrapCandidate(email string) *User {
+	return &User{
+		Email:        email,
+		PasswordHash: "test-hash",
+		Salt:         "test-salt",
+		Active:       true,
+	}
+}
+
+// TestIntegration_CreateAdminIfNone_Bootstrap exercises the real bootstrap
+// INSERT end to end: column ordering, the Administrators-group forcing logic,
+// and the NOT EXISTS guard's agreement with AdminExists.
+func TestIntegration_CreateAdminIfNone_Bootstrap(t *testing.T) {
+	db := setupAuthTestDB(t)
+	store := NewPostgresStore(db)
+	ctx := context.Background()
+
+	t.Run("inserts first admin and forces Administrators group", func(t *testing.T) {
+		resetUsers(t, db)
+
+		user := newBootstrapCandidate("first-admin@example.com")
+		created, err := store.CreateAdminIfNone(ctx, user)
+		require.NoError(t, err)
+		assert.True(t, created)
+
+		// The inserted row must satisfy the membership predicate AdminExists uses.
+		exists, err := store.AdminExists(ctx)
+		require.NoError(t, err)
+		assert.True(t, exists, "bootstrap admin must be visible to AdminExists")
+
+		stored, err := store.GetUserByEmail(ctx, "first-admin@example.com")
+		require.NoError(t, err)
+		assert.Contains(t, stored.GroupIDs, DefaultAdminGroupID,
+			"bootstrap admin must carry the Administrators group even though the caller did not supply it")
+
+		count, err := store.CountGroupMembers(ctx, DefaultAdminGroupID)
+		require.NoError(t, err)
+		assert.Equal(t, 1, count)
+	})
+
+	t.Run("second sequential call is a no-op", func(t *testing.T) {
+		resetUsers(t, db)
+
+		created, err := store.CreateAdminIfNone(ctx, newBootstrapCandidate("admin-a@example.com"))
+		require.NoError(t, err)
+		require.True(t, created)
+
+		created, err = store.CreateAdminIfNone(ctx, newBootstrapCandidate("admin-b@example.com"))
+		require.NoError(t, err)
+		assert.False(t, created, "an active admin already exists, so the guarded insert must not fire")
+
+		count, err := store.CountGroupMembers(ctx, DefaultAdminGroupID)
+		require.NoError(t, err)
+		assert.Equal(t, 1, count, "the losing call must not have inserted a row")
+	})
+
+	t.Run("inactive admin counts as no admin, matching AdminExists", func(t *testing.T) {
+		resetUsers(t, db)
+
+		// Insert an INACTIVE Administrators-group member through the regular,
+		// unguarded create path (INSERT does not fire the min-one-admin trigger).
+		inactive := newBootstrapCandidate("inactive-admin@example.com")
+		inactive.Active = false
+		inactive.GroupIDs = []string{DefaultAdminGroupID}
+		require.NoError(t, store.CreateUser(ctx, inactive))
+
+		exists, err := store.AdminExists(ctx)
+		require.NoError(t, err)
+		require.False(t, exists, "inactive admin must not count as an existing admin")
+
+		// The NOT EXISTS guard must agree: bootstrap proceeds.
+		created, err := store.CreateAdminIfNone(ctx, newBootstrapCandidate("real-admin@example.com"))
+		require.NoError(t, err)
+		assert.True(t, created,
+			"CreateAdminIfNone must treat an inactive-only admin set as 'no admin', mirroring AdminExists")
+
+		count, err := store.CountGroupMembers(ctx, DefaultAdminGroupID)
+		require.NoError(t, err)
+		assert.Equal(t, 2, count, "CountGroupMembers counts members regardless of active flag")
+	})
+
+	t.Run("email collision with existing non-admin surfaces ErrEmailInUse", func(t *testing.T) {
+		resetUsers(t, db)
+
+		regular := newBootstrapCandidate("taken@example.com")
+		// users_min_one_group (migration 000057) requires at least one group;
+		// any seeded non-admin group satisfies it.
+		regular.GroupIDs = []string{DefaultPurchaserGroupID}
+		require.NoError(t, store.CreateUser(ctx, regular))
+
+		created, err := store.CreateAdminIfNone(ctx, newBootstrapCandidate("taken@example.com"))
+		assert.False(t, created)
+		assert.ErrorIs(t, err, ErrEmailInUse)
+	})
+}
+
+// TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce proves the
+// insert-once semantics under two concurrent CreateAdminIfNone calls: exactly
+// one caller wins and exactly one admin row exists afterwards, across
+// repeated barrier-synchronized attempts.
+func TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce(t *testing.T) {
+	db := setupAuthTestDB(t)
+	store := NewPostgresStore(db)
+	ctx := context.Background()
+
+	const iterations = 25
+
+	for i := 0; i < iterations; i++ {
+		resetUsers(t, db)
+
+		type outcome struct {
+			created bool
+			err     error
+		}
+		results := make(chan outcome, 2)
+		start := make(chan struct{})
+
+		for n := 0; n < 2; n++ {
+			user := newBootstrapCandidate(fmt.Sprintf("admin-%d-%d@example.com", i, n))
+			go func(u *User) {
+				<-start
+				created, err := store.CreateAdminIfNone(ctx, u)
+				results <- outcome{created: created, err: err}
+			}(user)
+		}
+		close(start)
+
+		winners := 0
+		for n := 0; n < 2; n++ {
+			r := <-results
+			require.NoError(t, r.err, "iteration %d", i)
+			if r.created {
+				winners++
+			}
+		}
+		require.Equal(t, 1, winners,
+			"iteration %d: exactly one of two concurrent bootstrap calls must win", i)
+
+		count, err := store.CountGroupMembers(ctx, DefaultAdminGroupID)
+		require.NoError(t, err)
+		require.Equal(t, 1, count,
+			"iteration %d: exactly one admin row must exist after concurrent bootstrap", i)
+	}
+}