fix(backend): prevent double voting by jwidan · Pull Request #1257 · Equal-Vote/bettervoting

jwidan · 2026-03-01T06:11:31Z

Description

This PR stops a race condition that let users cast the same ballot more than once. Before this update, the backend synchronously checked voter permissions but passed the database writes to an asynchronous background worker, leaving a gap that overlapping requests could abuse.

To fix this flaw, the database writes are now done synchronously. A strict row-level lock is established on the voter's roll entry during the submit ballot event. This lock forces overlapping requests to wait for the first write to finish before they can move forward. When the waiting duplicate request reads the database, it sees the updated state and drops the vote.

Main changes:

Moved the ballot casts and roll updates out of the asynchronous event queue and into the synchronous controller.
Updated the core data models to accept database transaction locks, ensuring all queries execute safely inside the locked context without breaking backward compatibility for older methods.
Ensured the system does not attempt to send email receipts for duplicate ballots rejected by the database lock.

Local tests ran successfully and my previously successful attempts to double vote no longer work with this PR.

netlify · 2026-03-01T06:11:36Z

✅ Deploy Preview for bettervoting ready!

Name	Link
🔨 Latest commit	`38c9f29`
🔍 Latest deploy log	https://app.netlify.com/projects/bettervoting/deploys/69af568be089be0008c1f737
😎 Deploy Preview	https://deploy-preview-1257--bettervoting.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

jacksonloper

This the right pattern I think, we should do this or something close to it. Left a few comments.

jacksonloper · 2026-03-03T13:44:40Z

packages/backend/src/Models/CastVoteStore.ts

+                    throw new Error("ALREADY_VOTED");
+                }
+
+                if (!currentRoll) {


Why do we have to do this twice?

READ COMMITTED makes it necessary: https://www.postgresql.org/docs/current/transaction-iso.html#XACT-READ-COMMITTED

When a transaction uses this isolation level, a SELECT query (without a FOR UPDATE/SHARE clause) sees only data committed before the query began; it never sees either uncommitted data or changes committed by concurrent transactions during the query's execution. In effect, a SELECT query sees a snapshot of the database as of the instant the query begins to run.

To give an example,

Request A & Request B execute a query at roughly the same time. A new data snapshot is made at the start for each of them. Both snapshots see the current active row where head=true.

Request A gets the row lock, B attempts to get it but is blocked by A. B is now in a wait queue.

A finishes, updates the locked row (head=false), inserts a new row (w/ head=true), and commits.

After A releases the lock, B goes to lock the previously modified row (the new row doesn't exist in its snapshot), but the row no longer satisfies B's head=true. B's query returns undefined and interpreted as proof that voter has not voted yet and introduces another race condition where a duplicate ballot could be processed (but the window is probably smaller than what it was before this PR)

The second query makes a new snapshot so it can see the new state made by Request A.

Ah I see. I don't think this will fix the race though, because there are other reasons we update the voter rows, not just to mark submitted. Let's see if I can get this right 😂.

A locks the row for a vote.

B wants to send an email to the voter, but A has the row locked. It waits.

A commits, sets head=false, inserts a new row with head=true submitted=true.

B unblocks. B is confused because the row it was waiting for has vanished (its head is false now). It says "whatever" and inserts a new row with head=true and submitted=false

Now there are two head=true rows 😂 . I have no idea what happens then.

You could certainly argue this is really a problem with the un-transactified-nature of email roll update and similar.

Its also not terribly concerning because realistically for nefariousness it would require coordination of the admin and the voter, and broadly we tend to trust the admins somewhat. Ofc if we add something in the future that the voter could contorl like "request that voter_id is resent to my email" and use the same patterns we have been using, then nefariousness could become a legit problem.

Still... hmm...

jacksonloper · 2026-03-03T13:48:33Z

packages/backend/src/Controllers/Ballot/castVoteController.ts

+                    await ServiceLocator.castVoteStore().submitBallotEvent(event, ctx);
+                    successfullySavedEvents.push(event);
+                } catch (e: any) {
+                    Logger.error(req, `Could not upload ballot for ${event.roll?.voter_id}: ${e.message}`);


This log isn't visible to an election admin. So, from their point of view, this will fail silently. I suppose we need some ux plan for handling this.

jacksonloper · 2026-03-03T14:24:53Z

packages/backend/src/Models/__mocks__/CastVoteStore.ts

+            }
+        }
+
+        if (event.isBallotUpdate) {


What if not event.roll? I still don't want duplicate ballots even if there's no roll. You can vote as much as you like really, its not a security thing (mostly (always?) roll-less elections are in draft state) but still... at minimum I think we want something here that checks if the ballot is a duplicate and logs it. Not because we think nefariousness, but in case we do something silly later we want to notice that we've done something silly.

jacksonloper · 2026-03-05T15:09:39Z

I think this PR would benefit from an index that ensures the "exactly one head=true row" invariant for (election_id, voter_id). So then if a transaction tries to insert head=true while another row has head=true, postgres will at least keep that from happening. What do you think @jwidan ?

maybe something like packages/backend/src/Migrations/2026_03_05_unique_head_constraint.ts with

  import { Kysely, sql } from 'kysely'                                                                                                            
                             
  export async function up(db: Kysely<any>): Promise<void> {
      await sql`CREATE UNIQUE INDEX "electionRollDB_one_head" ON "electionRollDB" (election_id, voter_id) WHERE head = true`.execute(db)
  }

  export async function down(db: Kysely<any>): Promise<void> {
      await sql`DROP INDEX "electionRollDB_one_head"`.execute(db)
  }

jacksonloper · 2026-03-06T18:02:15Z

On some consideration I think its possible we should go with optimistic concurrency here instead of trying to wrap the check-if-they've-voted inside the write transaction. Like our project seems to be geared around the pattern:

read in one transaction
then edit in memory
then write in another transaction.
This pattern has a pitfall: we might be writing based on old news. And this PR addresses one aspect of that pitfall. But there's a general problem here we can clean up pretty generally by requiring most writes to provide the update_time at which you got your news (and aborting if you're stale). In other words, optimistic concurrency.

Of course we need a single transaction for writing to the ballot and writing to the election rolls, which is what CastVoteStore was originally all about, but weirdly it seems like we're not using it at all (and the version on main seems to be out of date in terms of how the database actually works now).

So, tldr:

This PR is definitely right to revive CastVoteStore, which puts ballot-update and rolls-update in one transaction (and it was was dead code before)!
This PR is definitely right to not send the confirmation email (or return a 200) until AFTER the database update completes successfully
It seems less important to me to do the check-if-they've-voted in the same transaction, and it requires this funny double scan. It seems more important to have all write transactions require an are-you-stale check (like run UPDATE electionRollDB SET head=false WHERE election_id=X AND voter_id=Y AND head=true AND update_date=<the update_date I read> and abort if that affects 0 zeros).
Its unclear to me that we need to thread optional trx through a bunch of functions. The CastVoteStore already has the database and can write the ballot and roll updates directly. There's some duplication involved in this but that is aok with me because casting votes is The Central Operation of voting and it makes sense to me that other ways of writing to the tables would be Different.

jacksonloper · 2026-03-06T18:05:54Z

packages/backend/src/Models/CastVoteStore.ts

+        });
+    }
+
    submitBallot(


This submitBallot is dead code I guess. And was before your PR if I'm understanding things correctly.

- Replaced pessimistic locking transactions with optimistic concurrency control (OCC) for ballot casting. - CastVoteStore.ts now validates the update_date of the electionRollDB snapshot to prevent race conditions. - Added a PostgreSQL electionRollDB_one_head constraint to ensure background jobs cannot duplicate roll state. - Roll-less draft elections log duplicate votes but are permitted to bypass constraints for admin testing. - Hardened ViewBallots.tsx and castVoteController to enforce and handle missing status payloads. - Removed dead code left behind.

- Avoid running unneeded duplicate ballot checks during known ballot updates. - Prevent duplicate ballots from being saved if an update happens exactly at the same time. - Stop altering the timestamp on old ballots. - Ensure test votes for anonymous elections are safely saved without falsely triggering "Already Voted" errors (Fixes Playwright E2E suite failure).

jwidan · 2026-03-07T08:09:20Z

Let me know if these commits cover the concerns brought up before. Included your constraint + implemented OCC and some other checks if perfect timing managed to happen. Cleaned up the dead code as well.

Also threw in a quick fix for ballots with missing statuses (my test attack that prompted this PR didn't have statuses and it would crash the admin dashboard)

jacksonloper

Looking good. My remaining thoughts are all minor. I think @JonBlauvelt you could take a look.

jacksonloper · 2026-03-09T15:54:58Z

packages/backend/src/Models/CastVoteStore.ts

+                        .execute();
+
+                    if (Number(updateBallotResult[0].numUpdatedRows) === 0) {
+                        throw new Error("ALREADY_VOTED"); 


I guess the error here is that they're trying to update twice at once, not really that they've already voted. Maybe a different sentinel?

jacksonloper · 2026-03-09T18:32:53Z

packages/backend/src/Models/CastVoteStore.ts

+                        .executeTakeFirst();
+
+                    if (duplicateBallot) {
+                        Logger.info(ctx, `Duplicate ballot detected for roll-less election user_id: ${event.inputBallot.user_id}`);


Its hard to see why we shouldn't throw an error here

Erroring would break the "Allows multiple votes per device" setting in an open election + the ability to submit multiple test ballots in draft

Ohh I misread this entirely. I thought you were addressing the we-dont-want-identical-ballot-ids issue. Ok. We can address that later anyway.

jacksonloper · 2026-03-09T18:36:38Z

packages/backend/src/Models/CastVoteStore.ts

+                            .executeTakeFirst();
+
+                        if (existingCount) {
+                            throw new Error("ALREADY_VOTED"); 


Or already trying to vote 😂 . Or actually maybe the admin is just changing something about the roll. Maybe the error message should be something about concurrent roll editing detected, aborting, please try again.

fix(backend): prevent double voting

538657c

Fix build

a4a7c1c

jacksonloper reviewed Mar 3, 2026

View reviewed changes

jacksonloper reviewed Mar 6, 2026

View reviewed changes

jwidan added 3 commits March 6, 2026 23:38

Merge branch 'Equal-Vote:main' into fix/prevent-double-voting

a74a75e

jacksonloper reviewed Mar 9, 2026

View reviewed changes

jacksonloper requested a review from JonBlauvelt March 9, 2026 18:48

refactor: throw distinct errors

38c9f29

Conversation

jwidan commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

netlify bot commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for bettervoting ready!

Uh oh!

jacksonloper left a comment

Choose a reason for hiding this comment

Uh oh!

jacksonloper Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

jwidan Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

jacksonloper Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacksonloper Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

jacksonloper Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacksonloper commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jacksonloper commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jacksonloper Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jwidan commented Mar 7, 2026

Uh oh!

jacksonloper left a comment

Choose a reason for hiding this comment

Uh oh!

jacksonloper Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

jacksonloper Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

jwidan Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

jacksonloper Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

jacksonloper Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jwidan commented Mar 1, 2026 •

edited

Loading

netlify bot commented Mar 1, 2026 •

edited

Loading

jacksonloper Mar 5, 2026 •

edited

Loading

jacksonloper Mar 3, 2026 •

edited

Loading

jacksonloper commented Mar 5, 2026 •

edited

Loading

jacksonloper commented Mar 6, 2026 •

edited

Loading

jacksonloper Mar 6, 2026 •

edited

Loading

jacksonloper Mar 9, 2026 •

edited

Loading