fix: resolve critical race condition in signup endpoint

[riddhima25bet10005-a11y]

Summary

Fixes a critical race condition in the signup endpoint. The synchronous array read (getUsers()) previously occurred before the asynchronous bcrypt.hash() which yields the event loop, causing concurrent signups to overwrite each other leading to silent data loss.

The fix fetches the user array twice: once before hashing to quickly prevent duplicates and DoS attacks, and once immediately before the synchronous append-and-write to ensure thread safety.

Related issue

Closes #522

Testing

• I ran the relevant checks locally
• I verified the app still starts
• I tested the affected flow end-to-end

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas

Screenshots / recordings

N/A

Notes

None

Security

• No sensitive data included

Summary by CodeRabbit

• Bug Fixes
  • Improved error handling during user signup when attempting to register with an existing account.

[vercel]

@riddhima25bet10005-a11y is attempting to deploy a commit to the firefistisdead's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

📝 Walkthrough

Walkthrough

In exports.signup within authController.js, the declarations for users and existingUser are changed from const to let. Inside the duplicate-user guard branch, both variables are re-fetched and reassigned before returning a 400 response with a simplified { message: "User already exists" } JSON payload.

Changes

Signup duplicate-user guard refactor

Layer / File(s)	Summary
Duplicate-user check and 400 response src/controllers/authController.js	users and existingUser are declared with let instead of const; the existing-user branch re-fetches both variables and returns a compact { message: "User already exists" } payload with a 400 status.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related issues

• #503: The re-fetch of users and existingUser inside the duplicate-user guard is a partial mitigation of the concurrent write synchronization problem described in this issue, which affects the same signup() function in authController.js during race conditions with parallel user registrations.

🐇 With let in place of const,
The user list gets read once more,
Before we say "already exists" —
A fresher check than before!
🥕 Small hops, safer doors.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: resolving a critical race condition in the signup endpoint, matching the PR's primary objective.
Description check	✅ Passed	The description comprehensively covers the problem, solution, and testing checklist following the repository template with all required sections completed.
Linked Issues check	✅ Passed	The PR directly addresses issue #522 by implementing the dual-fetch strategy to prevent race conditions during concurrent signups, matching the stated objective.
Out of Scope Changes check	✅ Passed	All changes in authController.js are focused on fixing the race condition in the signup endpoint, with no unrelated modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/controllers/authController.js (1)

65-69: ⚡ Quick win

Race condition fix correctly implemented.

The second fetch and duplicate-check after the async bcrypt operation is the critical race-prevention mechanism. Since Node.js executes lines 65-76 atomically (all synchronous operations), any concurrent request that reaches line 65 after this request completes line 76 will see the newly-saved user and correctly exit at line 68.

💡 Optional: Add clarifying comment for the dual-check pattern

Consider adding a brief comment to explain why the duplicate-user check happens twice, since the pattern may not be immediately obvious to future maintainers:

 const hashedPassword = await bcrypt.hash(
   password,
   10
 );
 
+// Re-fetch and re-check after async bcrypt to prevent race condition:
+// concurrent requests may have saved the same email while we were hashing.
 users = getUsers();
 existingUser = users.find((u) => u.email === email);
 if (existingUser) {
   return res.status(400).json({ message: "User already exists" });
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/controllers/authController.js` around lines 65 - 69, The race condition
fix is correctly implemented with a dual-check pattern, but add a clarifying
comment before the second getUsers() call in the authController.js file to
explain why the duplicate-user check happens twice. The comment should explain
that the first check (before async operations) and second check (after bcrypt)
together prevent race conditions by ensuring no concurrent request creates a
user with the same email between the initial validation and the save operation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/controllers/authController.js`:
- Around line 65-69: The race condition fix is correctly implemented with a
dual-check pattern, but add a clarifying comment before the second getUsers()
call in the authController.js file to explain why the duplicate-user check
happens twice. The comment should explain that the first check (before async
operations) and second check (after bcrypt) together prevent race conditions by
ensuring no concurrent request creates a user with the same email between the
initial validation and the save operation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8df2f4f5-7eb1-4f5f-b8a7-b45a6fb29eb2

📥 Commits

Reviewing files that changed from the base of the PR and between 5590b87 and 40c157c.

📒 Files selected for processing (1)

• src/controllers/authController.js

[riddhima25bet10005-a11y]

Hi @FireFistisDead
Kindly review.