Skip to content

Vote toggle is a read-modify-write that ignores every write error #52

Description

@royalpinto007

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinghelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions