hard: registration wallet-existence checks are TOCTOU — concurrent registers create duplicate identities

[Godfrey-Delight]

🔗 Related Issue

Closes #120

---

🔖 Title

fix(auth): enforce database-level uniqueness in register and eliminate race conditions

---

📝 Description

Eliminated application-side pre-check queries in AuthService.register() (findByWallet and checkUsernameExists) which allowed race conditions during concurrent user registrations (e.g., double-click or retry storms). Uniqueness is now enforced directly by DB-level UNIQUE constraints on users.wallet_address and users.username. Unique constraint violations (Postgres 23505) are caught and mapped to existing structured 409 ConflictException error responses (AUTH_WALLET_EXISTS, AUTH_USERNAME_TAKEN). In addition, failed registration attempts clean up uploaded avatar objects and partial user records to prevent orphaned state.

---

🔄 Changes Made

• Database Migration: Added migration 20260826130000_ensure_users_unique_constraints.sql ensuring UNIQUE indexes exist on users.wallet_address and users.username.
• Repository Layer: Updated UsersRepository.createProfile() to catch Postgres error 23505 and map wallet/username collisions to structured ConflictException responses. Added deleteAvatar and deleteUserById methods.
• Auth Service Rewrite: Rewrote AuthService.register() to attempt direct insertion via createProfile(), eliminating racy pre-checks. Wrapped profile creation, avatar upload, and token issuance in a try...catch block with rollback cleanup.
• Unit & Race Tests: Added unit tests covering PostgreSQL 23505 error mapping, parallel duplicate-wallet and duplicate-username race tests (Promise.allSettled), sequential re-registration compatibility, and avatar/user cleanup on failure.
• Documentation: Updated context/progress-tracker.md.

---

📸 Screenshots (if applicable)

N/A (Backend API logic change)

---

🗒️ Additional Notes

• npm run build passed cleanly with 0 TypeScript errors.
• npm test passed with 30/30 test suites green (355/355 total tests passing).
• API error format and status codes remain 100% backward compatible for legitimate callers.

[EmeditWeb]

⚠️ Automated Audit: partial

@Godfrey-Delight Good start — please look into the gaps identified below.

The PR correctly eliminates the TOCTOU race by removing application-side pre-checks (findByWallet, checkUsernameExists) and relying on DB-level UNIQUE constraints enforced at insert time. The migration, repository error mapping (23505 → structured 409s), try/catch rollback with avatar/user cleanup, and comprehensive regression tests (parallel wallet race, parallel username race, sequential re-register, cleanup-on-failure) all directly address the root cause described in Issue #120. CI passed; tests are substantive and cover the vulnerability path. The only blemish is merge conflict markers left in context/progress-tracker.md.

⚖️ Adjusted by bot policy: gaps were still identified.

Gaps identified:

• Merge conflict markers (<<<<<<<, =======, >>>>>>>) present in context/progress-tracker.md — must be resolved before merge
• Migration assumes no pre-existing duplicate rows in users table; if duplicates exist, CREATE UNIQUE INDEX will fail at deploy time — consider a data-cleanup step or conditional logic in the migration

---

CI checks: ✅ PASSED: build-test

_{Audited by stepfi-audit-bot 🤖}

[EmeditWeb]

✅ Automated Audit: solves

@Godfrey-Delight Excellent work, thank you! 🎉

The PR directly addresses all four requirements of Issue #120: (1) migration adds DB-level UNIQUE indexes on wallet_address and username with deduplication of existing rows, (2) register() pre-checks (findByWallet, checkUsernameExists) are removed and replaced with direct insert + error-code mapping in createProfile(), (3) try/catch cleanup prevents orphaned avatars and partial user records on failure, (4) tests cover parallel race conditions via Promise.allSettled, sequential re-registration, and cleanup paths. CI passes (355/355 tests). The fixes target the actual root cause (application-level TOCTOU) rather than symptoms.

---

CI checks: ✅ PASSED: build-test
Merge conflicts: ✅ none — but the PR is blocked (failing/missing required checks or reviews).

_{Audited by stepfi-audit-bot 🤖}