The race condition in SkillMatch’s attend route

SkillMatch shows how full a workshop is and disables the Attend button once every seat is taken. It looks correct when you click through it alone. The problem only appears when two people act at nearly the same moment.

What the route does today

const workshop = await Workshop.findById(req.params.id);   // 1. read
if (workshop.attendees.includes(req.user._id))
  return res.status(400).json({ error: 'Already attending' });

workshop.attendees.push(req.user._id);                        // 2. change in memory
await workshop.save();                                        // 3. write

The route reads the workshop, decides in JavaScript, then writes. Between step 1 and step 3, another request can run the same three steps. The server never checks the seat count at all; the only capacity check lives in the React component that disables the button.

How two people both get the last seat

• The workshop has 20 seats and 19 attendees. Alice and Bob both see an enabled Attend button.
• Both requests run findById and load a document with 19 attendees.
• Neither request finds its user in the list, so both push and save.
• The workshop now has 21 attendees for 20 seats.

Anyone can also skip the button entirely and call POST /:id/attend with a valid token, so the UI check was never a guarantee. The rule has to live where the data is.

The fix: let MongoDB decide in one step

MongoDB applies a single-document update atomically. If the conditions and the change are in one findOneAndUpdate, there is no gap for another request to slip into: the update only happens if the document still matches at the moment it is written.

const workshop = await Workshop.findOneAndUpdate(
  {
    _id: req.params.id,
    hostedBy: { $ne: req.user._id },      // can't attend your own workshop
    attendees: { $ne: req.user._id },     // not already attending
    // seats is stored as a String, so convert it before comparing
    $expr: { $lt: [{ $size: '$attendees' }, { $toInt: '$seats' }] },
  },
  { $push: { attendees: req.user._id } },
  { new: true },
);

if (!workshop) {
  return res.status(409).json({ error: 'Workshop is full or you are already attending' });
}
res.json(workshop);

The $toInt matters. seats is declared as String in the Mongoose schema, and MongoDB compares a number to a string by type order, not value: every number sorts before every string. Without the conversion, $lt would always be true and the check would silently pass.

A second bug the rewrite fixes

The current owner check is workshop.hostedBy.toString() === req.user._id. The left side is a string and the right side is an ObjectId, so strict equality is always false and hosts can attend their own workshops. Moving the check into the query as hostedBy: { $ne: req.user._id } lets MongoDB compare ObjectIds properly.

How I would test it

Create a workshop with one seat, then fire two attend requests with Promise.all from two different users. Exactly one should get 200 and the other 409. Before the fix, both get 200. A test like that is worth more than any amount of reasoning about timing, because it fails loudly when someone later “simplifies” the query back into read-then-write.