POST /api/posts/[id]/vote in app/api/posts/[id]/vote/route.ts decides what to do by reading the caller's existing vote and then writing:
const { data: existing } = await supabase
.from("votes")
.select("id, direction")
.eq("post_id", postId)
.eq("ip_hash", ipHash)
.maybeSingle();
if (direction === null) { ... } else if (existing) { ... } else {
await supabase.from("votes").insert({ post_id: postId, ip_hash: ipHash, direction });
}
Two problems.
Read-modify-write is not atomic. Two requests carrying the same ip_hash (double-click, retry, two tabs) can both observe existing === null and both reach the insert branch. Whether that yields two rows depends entirely on whether votes has a unique constraint on (post_id, ip_hash), which is not visible from this repository, is not asserted anywhere in code, and is not covered by a test. If it does exist, the second insert throws and the error is swallowed because none of these writes check error. If it does not exist, one IP quietly gets two votes on one case and vote_score is wrong.
Every write in this handler ignores its error. The delete, update and insert calls are all awaited without inspecting the returned error. The handler then re-reads vote_score and returns 200 with whatever the database happens to hold, so a failed write is indistinguishable from a successful one to the caller.
Suggested direction:
- Confirm and, if missing, add a unique index on
votes (post_id, ip_hash).
- Replace the read-then-write with a single upsert on that conflict target, or move the whole toggle into a
SECURITY DEFINER RPC the way consume_rate_limit is handled in lib/rate-limit/shared.ts.
- Check the
error on every write and return 500 with a logEvent entry, matching how app/api/posts/route.ts handles its insert failure.
- Add a route test next to the existing ones under
app/api/.
If you would like to take this on, comment here to claim it. Contributors can hold two open claims at a time.
POST /api/posts/[id]/voteinapp/api/posts/[id]/vote/route.tsdecides what to do by reading the caller's existing vote and then writing:Two problems.
Read-modify-write is not atomic. Two requests carrying the same
ip_hash(double-click, retry, two tabs) can both observeexisting === nulland both reach theinsertbranch. Whether that yields two rows depends entirely on whethervoteshas a unique constraint on(post_id, ip_hash), which is not visible from this repository, is not asserted anywhere in code, and is not covered by a test. If it does exist, the second insert throws and the error is swallowed because none of these writes checkerror. If it does not exist, one IP quietly gets two votes on one case andvote_scoreis wrong.Every write in this handler ignores its error. The
delete,updateandinsertcalls are all awaited without inspecting the returnederror. The handler then re-readsvote_scoreand returns 200 with whatever the database happens to hold, so a failed write is indistinguishable from a successful one to the caller.Suggested direction:
votes (post_id, ip_hash).SECURITY DEFINERRPC the wayconsume_rate_limitis handled inlib/rate-limit/shared.ts.erroron every write and return 500 with alogEvententry, matching howapp/api/posts/route.tshandles its insert failure.app/api/.If you would like to take this on, comment here to claim it. Contributors can hold two open claims at a time.