fix(api): detect duplicate registrations by SQLSTATE, not error text

internal/api/handler_registrations.go

@@ -102,11 +102,8 @@ func (h *Handler) submitRegistration(ctx context.Context, req *events.LambdaFunc
 	}
 	reg.RegCredentialPayload = encrypted
 
-	if err := h.config.CreateAccountRegistration(ctx, reg); err != nil {
-		if strings.Contains(err.Error(), "duplicate") {
-			return nil, NewClientError(409, "a pending registration already exists for this account")
-		}
-		return nil, fmt.Errorf("registrations: %w", err)
+	if err := h.insertRegistration(ctx, reg); err != nil {
+		return nil, err
 	}
 
 	// Notify admins (synchronous, errors logged but not propagated).
@@ -127,11 +124,37 @@ func (h *Handler) submitRegistration(ctx context.Context, req *events.LambdaFunc
 	}
 
 	return map[string]string{
-		"reference_token": token,
+		"reference_token": reg.ReferenceToken,
 		"status":          "pending",
 	}, nil
 }
 
+// maxReferenceTokenAttempts bounds how often insertRegistration regenerates a
+// colliding reference token before giving up.
+const maxReferenceTokenAttempts = 3
+
+// insertRegistration stores reg, mapping a duplicate pending registration to
+// 409 and regenerating reg.ReferenceToken on a token collision. Any other
+// store error is returned wrapped, so it surfaces as a generic 500.
+func (h *Handler) insertRegistration(ctx context.Context, reg *config.AccountRegistration) error {
+	for attempt := 1; ; attempt++ {
+		err := h.config.CreateAccountRegistration(ctx, reg)
+		switch {
+		case err == nil:
+			return nil
+		case errors.Is(err, config.ErrDuplicatePendingRegistration):
+			return NewClientError(409, "a pending registration already exists for this account")
+		case !errors.Is(err, config.ErrReferenceTokenCollision) || attempt >= maxReferenceTokenAttempts:
+			return fmt.Errorf("registrations: %w", err)
+		}
+		token, tokErr := generateReferenceToken()
+		if tokErr != nil {
+			return fmt.Errorf("registrations: generate token: %w", tokErr)
+		}
+		reg.ReferenceToken = token
+	}
+}
+
 // getRegistrationStatus handles GET /api/register/:token.
 func (h *Handler) getRegistrationStatus(ctx context.Context, token string) (any, error) {
 	reg, err := h.config.GetAccountRegistrationByToken(ctx, token)

internal/api/handler_registrations_submit_test.go

@@ -0,0 +1,131 @@
+package api
+
+// Tests for how submitRegistration maps store errors (issue #137): the 409
+// "already registered" answer comes only from the typed duplicate sentinel,
+// never from the word "duplicate" in an error message.
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"testing"
+
+	"github.com/aws/aws-lambda-go/events"
+	"github.com/jackc/pgx/v5/pgconn"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/email"
+)
+
+// countingRegistrationNotifier counts admin "registration received" emails.
+type countingRegistrationNotifier struct {
+	stubEmailNotifier
+	received int
+}
+
+func (n *countingRegistrationNotifier) SendRegistrationReceivedNotification(_ context.Context, _ email.RegistrationNotificationData) error {
+	n.received++
+	return nil
+}
+
+const submitRegistrationBody = `{"provider":"aws","external_id":"123456789012",` +
+	`"account_name":"Prod","contact_email":"owner@example.com"}`
+
+func submitRegistrationRequest() *events.LambdaFunctionURLRequest {
+	return &events.LambdaFunctionURLRequest{Body: submitRegistrationBody}
+}
+
+func TestSubmitRegistration_DuplicatePending_Returns409WithoutEmail(t *testing.T) {
+	ctx := context.Background()
+	store := new(MockConfigStore)
+	store.On("CreateAccountRegistration", ctx, mock.Anything).
+		Return(fmt.Errorf("%w: %w", config.ErrDuplicatePendingRegistration,
+			&pgconn.PgError{Code: "23505", ConstraintName: "idx_account_registrations_pending_unique"})).Once()
+	notifier := &countingRegistrationNotifier{}
+	h := &Handler{config: store, emailNotifier: notifier}
+
+	_, err := h.submitRegistration(ctx, submitRegistrationRequest())
+
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "duplicate must be a client error, got %v", err)
+	assert.Equal(t, 409, ce.code)
+	assert.Equal(t, "a pending registration already exists for this account", ce.message)
+	assert.Zero(t, notifier.received, "a duplicate must not email admins")
+	store.AssertNumberOfCalls(t, "CreateAccountRegistration", 1)
+}
+
+// Errors that are not the duplicate sentinel must surface as a generic 500,
+// even when their text contains "duplicate", and must not leak the DB error.
+func TestSubmitRegistration_NonDuplicateErrors_Return500(t *testing.T) {
+	cases := map[string]error{
+		"text mentions duplicate": errors.New(
+			`duplicate: duplicate key value violates unique constraint "idx_account_registrations_pending_unique"`),
+		"unique violation on another constraint": fmt.Errorf("failed to create account registration: %w",
+			&pgconn.PgError{Code: "23505", ConstraintName: "account_registrations_pkey",
+				Message: "duplicate key value violates unique constraint"}),
+		"other database error": fmt.Errorf("failed to create account registration: %w",
+			&pgconn.PgError{Code: "53300", Message: "too many connections"}),
+	}
+	for name, storeErr := range cases {
+		t.Run(name, func(t *testing.T) {
+			ctx := context.Background()
+			store := new(MockConfigStore)
+			store.On("CreateAccountRegistration", ctx, mock.Anything).Return(storeErr).Once()
+			notifier := &countingRegistrationNotifier{}
+			h := &Handler{config: store, emailNotifier: notifier}
+
+			_, err := h.submitRegistration(ctx, submitRegistrationRequest())
+
+			require.Error(t, err)
+			_, isClient := IsClientError(err)
+			assert.False(t, isClient, "must not be reported as a client (409) error")
+			status, body := h.handleRequestError(err)
+			assert.Equal(t, 500, status)
+			assert.Equal(t, map[string]string{"error": "Internal server error"}, body)
+			assert.Zero(t, notifier.received, "a duplicate must not email admins")
+			store.AssertNumberOfCalls(t, "CreateAccountRegistration", 1)
+		})
+	}
+}
+
+// A reference-token collision is retryable: the handler regenerates the token
+// and re-inserts instead of telling the submitter the account is registered.
+func TestSubmitRegistration_TokenCollision_RetriesWithNewToken(t *testing.T) {
+	ctx := context.Background()
+	store := new(MockConfigStore)
+	var tokens []string
+	collision := fmt.Errorf("%w: %w", config.ErrReferenceTokenCollision,
+		&pgconn.PgError{Code: "23505", ConstraintName: "account_registrations_reference_token_key"})
+	record := func(args mock.Arguments) {
+		tokens = append(tokens, args.Get(1).(*config.AccountRegistration).ReferenceToken)
+	}
+	store.On("CreateAccountRegistration", ctx, mock.Anything).Run(record).Return(collision).Once()
+	store.On("CreateAccountRegistration", ctx, mock.Anything).Run(record).Return(nil).Once()
+	h := &Handler{config: store}
+
+	resp, err := h.submitRegistration(ctx, submitRegistrationRequest())
+
+	require.NoError(t, err)
+	require.Len(t, tokens, 2)
+	assert.NotEqual(t, tokens[0], tokens[1], "retry must use a fresh token")
+	assert.Equal(t, map[string]string{"reference_token": tokens[1], "status": "pending"}, resp)
+}
+
+func TestSubmitRegistration_TokenCollision_GivesUpAs500(t *testing.T) {
+	ctx := context.Background()
+	store := new(MockConfigStore)
+	collision := fmt.Errorf("%w: %w", config.ErrReferenceTokenCollision,
+		&pgconn.PgError{Code: "23505", ConstraintName: "account_registrations_reference_token_key"})
+	store.On("CreateAccountRegistration", ctx, mock.Anything).Return(collision)
+	h := &Handler{config: store}
+
+	_, err := h.submitRegistration(ctx, submitRegistrationRequest())
+
+	require.ErrorIs(t, err, config.ErrReferenceTokenCollision)
+	_, isClient := IsClientError(err)
+	assert.False(t, isClient)
+	store.AssertNumberOfCalls(t, "CreateAccountRegistration", maxReferenceTokenAttempts)
+}

internal/config/store_postgres_registrations.go

@@ -9,15 +9,53 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+	"github.com/jackc/pgx/v5/pgconn"
 )
 
 // ErrRegistrationConflict is returned when a registration status transition fails
 // because another request already changed the status (concurrent modification).
 var ErrRegistrationConflict = errors.New("registration status conflict: already processed")
 
-// CreateAccountRegistration inserts a new registration request. Returns an error
-// wrapping "duplicate" when the partial unique index rejects a second pending
-// registration for the same provider+external_id.
+// ErrDuplicatePendingRegistration is returned by CreateAccountRegistration when
+// a pending registration already exists for the same provider+external_id.
+var ErrDuplicatePendingRegistration = errors.New("a pending registration already exists for this account")
+
+// ErrReferenceTokenCollision is returned by CreateAccountRegistration when the
+// generated reference_token is already taken. The caller should regenerate the
+// token and retry; it says nothing about whether the account is registered.
+var ErrReferenceTokenCollision = errors.New("registration reference token collision")
+
+// Unique constraints on account_registrations (migration 000017). Matched by
+// name so a new unique constraint cannot silently fold into an existing branch.
+const (
+	pgUniqueViolation                    = "23505"
+	registrationPendingUniqueIndex       = "idx_account_registrations_pending_unique"
+	registrationReferenceTokenConstraint = "account_registrations_reference_token_key"
+)
+
+// classifyRegistrationInsertError maps a unique violation on a known
+// account_registrations constraint to its sentinel, using the driver's SQLSTATE
+// and constraint name rather than the message text (which varies with
+// lc_messages and driver versions). The driver error stays wrapped. Every
+// other error, including unique violations on other constraints, is returned
+// as a generic insert failure.
+func classifyRegistrationInsertError(err error) error {
+	var pgErr *pgconn.PgError
+	if errors.As(err, &pgErr) && pgErr.Code == pgUniqueViolation {
+		switch pgErr.ConstraintName {
+		case registrationPendingUniqueIndex:
+			return fmt.Errorf("%w: %w", ErrDuplicatePendingRegistration, err)
+		case registrationReferenceTokenConstraint:
+			return fmt.Errorf("%w: %w", ErrReferenceTokenCollision, err)
+		}
+	}
+	return fmt.Errorf("failed to create account registration: %w", err)
+}
+
+// CreateAccountRegistration inserts a new registration request. It returns an
+// error wrapping ErrDuplicatePendingRegistration when the partial unique index
+// rejects a second pending registration for the same provider+external_id, and
+// one wrapping ErrReferenceTokenCollision when reg.ReferenceToken is taken.
 func (s *PostgresStore) CreateAccountRegistration(ctx context.Context, reg *AccountRegistration) error {
 	if reg.ID == "" {
 		reg.ID = uuid.New().String()
@@ -61,11 +99,7 @@ func (s *PostgresStore) CreateAccountRegistration(ctx context.Context, reg *Acco
 		reg.CreatedAt, reg.UpdatedAt,
 	)
 	if err != nil {
-		if strings.Contains(err.Error(), "idx_account_registrations_pending_unique") ||
-			strings.Contains(err.Error(), "duplicate key") {
-			return fmt.Errorf("duplicate: a pending registration already exists for this account")
-		}
-		return fmt.Errorf("failed to create account registration: %w", err)
+		return classifyRegistrationInsertError(err)
 	}
 	return nil
 }

internal/config/store_postgres_registrations_integration_test.go

@@ -0,0 +1,70 @@
+//go:build integration
+// +build integration
+
+package config
+
+// Real-DB test for registration uniqueness (issue #137). The constraint names
+// the store matches on come from migration 000017, so they are checked against
+// the errors Postgres actually raises rather than hand-built ones.
+
+import (
+	"context"
+	"testing"
+
+	"github.com/jackc/pgx/v5/pgconn"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers"
+)
+
+func newPendingRegistration(token, externalID string) *AccountRegistration {
+	return &AccountRegistration{
+		ReferenceToken: token, Status: "pending", Provider: "aws", ExternalID: externalID,
+		AccountName: "Prod", ContactEmail: "owner@example.com",
+	}
+}
+
+func TestCreateAccountRegistration_UniquenessPostgres(t *testing.T) {
+	ctx := context.Background()
+	container := testhelpers.RequirePostgresContainer(ctx, t)
+	t.Cleanup(func() { assert.NoError(t, container.Cleanup(context.Background())) })
+	require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), getTestMigrationsPath(), "", ""))
+	store := NewPostgresStore(container.DB)
+
+	first := newPendingRegistration("tok-137-a", "111111111111")
+	require.NoError(t, store.CreateAccountRegistration(ctx, first))
+
+	// Same account submitted twice while the first is pending.
+	err := store.CreateAccountRegistration(ctx, newPendingRegistration("tok-137-b", "111111111111"))
+	require.ErrorIs(t, err, ErrDuplicatePendingRegistration)
+	assert.NotErrorIs(t, err, ErrReferenceTokenCollision)
+	var pgErr *pgconn.PgError
+	require.ErrorAs(t, err, &pgErr)
+	assert.Equal(t, pgUniqueViolation, pgErr.Code)
+	assert.Equal(t, registrationPendingUniqueIndex, pgErr.ConstraintName)
+
+	// A different account reusing an existing token is a token collision,
+	// not a duplicate registration.
+	err = store.CreateAccountRegistration(ctx, newPendingRegistration("tok-137-a", "222222222222"))
+	require.ErrorIs(t, err, ErrReferenceTokenCollision)
+	assert.NotErrorIs(t, err, ErrDuplicatePendingRegistration)
+	require.ErrorAs(t, err, &pgErr)
+	assert.Equal(t, registrationReferenceTokenConstraint, pgErr.ConstraintName)
+
+	// A unique violation on any other constraint (here the primary key) is
+	// neither sentinel.
+	dupID := newPendingRegistration("tok-137-c", "333333333333")
+	dupID.ID = first.ID
+	err = store.CreateAccountRegistration(ctx, dupID)
+	require.Error(t, err)
+	assert.NotErrorIs(t, err, ErrDuplicatePendingRegistration)
+	assert.NotErrorIs(t, err, ErrReferenceTokenCollision)
+
+	// Once the first registration leaves pending, the account may register again.
+	_, err = container.DB.Pool().Exec(ctx,
+		`UPDATE account_registrations SET status = 'rejected' WHERE id = $1`, first.ID)
+	require.NoError(t, err)
+	require.NoError(t, store.CreateAccountRegistration(ctx, newPendingRegistration("tok-137-d", "111111111111")))
+}