fix(signup): return 409 instead of raw 500 on concurrent signup race (fixes #2675)

[deepshekhardas]

fix(signup): return 409 instead of raw 500 on concurrent signup race (fixes #2675)

Two concurrent /otp requests for the same new email can both pass the
user-existence check, then collide on the users_email_partial_key
partial unique index during user creation. The losing request surfaced a
raw 500 (unexpected_failure, aborted transaction) indistinguishable
from a server outage.

signupNewUser now maps a unique-constraint violation (23505) from the
user insert to a structured 409 conflict with
error_code=user_already_exists, so a client can distinguish the race
from an outage. A retry a second later would hit the existing-user OTP
path anyway.

[depthfirst-app]

🟡 Severity: MEDIUM

An unauthenticated caller controls the signup email and X-JWT-AUD; when the audience-scoped lookup misses an email belonging to another audience, the global users_email_partial_key reaches this branch and returns user_already_exists. On structured-error API versions, that response exposes cross-audience account existence.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: In signupNewUser, the unique-constraint violation on tx.Create(user) is currently mapped unconditionally to 409 user_already_exists. This leaks cross-audience email existence because IsDuplicatedEmail filters by audience and misses a user in a different audience, but the global users_email_partial_key index still fires.

The fix is to add a same-audience confirmation lookup before returning user_already_exists. After the constraint violation, tx (and the closure's conn, since they may be the same aborted connection when called from within an outer transaction) cannot be used for subsequent queries. Instead, use a.db — the non-transactional connection held by the API struct — to call models.FindUserByEmailAndAudience(a.db, user.GetEmail(), user.Aud). Only return the user_already_exists 409 if a user is confirmed to exist in the SAME audience (a legitimate concurrent race). Otherwise fall through to the existing generic NewInternalServerError on line 391, which avoids exposing cross-audience account existence to the unauthenticated caller.

⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.

Suggested change

	// A concurrent request created the same user first
	return apierrors.NewConflictError(apierrors.ErrorCodeUserAlreadyExists, "User already exists").WithInternalError(terr)
	// Only surface user_already_exists for a same-audience concurrent race;
	// a cross-audience email collision must not reveal cross-audience existence.
	// Use a.db (non-transactional) because conn/tx is aborted after the violation.
	if existing, _ := models.FindUserByEmailAndAudience(a.db, user.GetEmail(), user.Aud); existing != nil {
	return apierrors.NewConflictError(apierrors.ErrorCodeUserAlreadyExists, "User already exists").WithInternalError(terr)
	}