From 789769c20e7e1cb78c91d0f70e0b7f71051a9185 Mon Sep 17 00:00:00 2001 From: Developer Date: Sat, 10 Oct 2026 16:35:35 -0400 Subject: [PATCH] fix(alerts): enforce atomic count pairing and eliminate orphan vote loophole in firestore.rules --- .../emulator-tests/firestore.rules.test.js | 254 ++++++++++++++++-- firestore.rules | 9 +- 2 files changed, 236 insertions(+), 27 deletions(-) diff --git a/firebase/emulator-tests/firestore.rules.test.js b/firebase/emulator-tests/firestore.rules.test.js index 5f33515..22241e6 100644 --- a/firebase/emulator-tests/firestore.rules.test.js +++ b/firebase/emulator-tests/firestore.rules.test.js @@ -6325,10 +6325,18 @@ describe('MotorCloud Rider Swarm Road Alerts Security Rules', () => { })); const voteRefBob = doc(bob, `road_alerts/${alertId}/votes/bob`); - await assertSucceeds(setDoc(voteRefBob, { - voterUid: 'bob', - type: 'confirm', - votedAt: Date.now(), + await assertSucceeds(runTransaction(bob, async (tx) => { + const alertRef = doc(bob, `road_alerts/${alertId}`); + const snap = await tx.get(alertRef); + tx.update(alertRef, { + confirmations: snap.data().confirmations + 1, + status: 'confirmed', + }); + tx.set(voteRefBob, { + voterUid: 'bob', + type: 'confirm', + votedAt: Date.now(), + }); })); // Signed-in riders can read the vote subcollection @@ -6352,10 +6360,18 @@ describe('MotorCloud Rider Swarm Road Alerts Security Rules', () => { })); const voteRefBob = doc(bob, `road_alerts/${alertId}/votes/bob`); - await assertSucceeds(setDoc(voteRefBob, { - voterUid: 'bob', - type: 'dismiss', - votedAt: Date.now(), + await assertSucceeds(runTransaction(bob, async (tx) => { + const alertRef = doc(bob, `road_alerts/${alertId}`); + const snap = await tx.get(alertRef); + tx.update(alertRef, { + dismissals: snap.data().dismissals + 1, + status: 'active', + }); + tx.set(voteRefBob, { + voterUid: 'bob', + type: 'dismiss', + votedAt: Date.now(), + }); })); }); @@ -6372,10 +6388,18 @@ describe('MotorCloud Rider Swarm Road Alerts Security Rules', () => { })); const voteRefBob = doc(bob, `road_alerts/${alertId}/votes/bob`); - await assertSucceeds(setDoc(voteRefBob, { - voterUid: 'bob', - type: 'confirm', - votedAt: Date.now(), + await assertSucceeds(runTransaction(bob, async (tx) => { + const alertRef = doc(bob, `road_alerts/${alertId}`); + const snap = await tx.get(alertRef); + tx.update(alertRef, { + confirmations: snap.data().confirmations + 1, + status: 'confirmed', + }); + tx.set(voteRefBob, { + voterUid: 'bob', + type: 'confirm', + votedAt: Date.now(), + }); })); // Trying to overwrite via setDoc fails (update blocked) @@ -6389,6 +6413,21 @@ describe('MotorCloud Rider Swarm Road Alerts Security Rules', () => { await assertFails(updateDoc(voteRefBob, { votedAt: Date.now(), })); + + // Second transaction also fails + await assertFails(runTransaction(bob, async (tx) => { + const alertRef = doc(bob, `road_alerts/${alertId}`); + const snap = await tx.get(alertRef); + tx.update(alertRef, { + confirmations: snap.data().confirmations + 1, + status: 'confirmed', + }); + tx.set(voteRefBob, { + voterUid: 'bob', + type: 'confirm', + votedAt: Date.now(), + }); + })); }); test('confirmation followed by dismissal from same user (Bob tries to change vote) fails', async () => { @@ -6404,10 +6443,18 @@ describe('MotorCloud Rider Swarm Road Alerts Security Rules', () => { })); const voteRefBob = doc(bob, `road_alerts/${alertId}/votes/bob`); - await assertSucceeds(setDoc(voteRefBob, { - voterUid: 'bob', - type: 'confirm', - votedAt: Date.now(), + await assertSucceeds(runTransaction(bob, async (tx) => { + const alertRef = doc(bob, `road_alerts/${alertId}`); + const snap = await tx.get(alertRef); + tx.update(alertRef, { + confirmations: snap.data().confirmations + 1, + status: 'confirmed', + }); + tx.set(voteRefBob, { + voterUid: 'bob', + type: 'confirm', + votedAt: Date.now(), + }); })); // Trying to change vote to dismiss via updateDoc fails @@ -6425,6 +6472,21 @@ describe('MotorCloud Rider Swarm Road Alerts Security Rules', () => { // Trying to retract/delete vote fails await assertFails(deleteDoc(voteRefBob)); + + // Trying to change vote via transaction fails + await assertFails(runTransaction(bob, async (tx) => { + const alertRef = doc(bob, `road_alerts/${alertId}`); + const snap = await tx.get(alertRef); + tx.update(alertRef, { + dismissals: snap.data().dismissals + 1, + status: 'active', + }); + tx.set(voteRefBob, { + voterUid: 'bob', + type: 'dismiss', + votedAt: Date.now(), + }); + })); }); test('voting on one\'s own report (Alice tries to vote on road_alerts/alert_by_alice/votes/alice) fails', async () => { @@ -6583,16 +6645,32 @@ describe('MotorCloud Rider Swarm Road Alerts Security Rules', () => { const voteRefBob = doc(bob, `road_alerts/${alertId}/votes/bob`); const voteRefCharlie = doc(charlie, `road_alerts/${alertId}/votes/charlie`); - await assertSucceeds(setDoc(voteRefBob, { - voterUid: 'bob', - type: 'confirm', - votedAt: Date.now(), + await assertSucceeds(runTransaction(bob, async (tx) => { + const alertRef = doc(bob, `road_alerts/${alertId}`); + const snap = await tx.get(alertRef); + tx.update(alertRef, { + confirmations: snap.data().confirmations + 1, + status: 'confirmed', + }); + tx.set(voteRefBob, { + voterUid: 'bob', + type: 'confirm', + votedAt: Date.now(), + }); })); - await assertSucceeds(setDoc(voteRefCharlie, { - voterUid: 'charlie', - type: 'dismiss', - votedAt: Date.now(), + await assertSucceeds(runTransaction(charlie, async (tx) => { + const alertRef = doc(charlie, `road_alerts/${alertId}`); + const snap = await tx.get(alertRef); + tx.update(alertRef, { + dismissals: snap.data().dismissals + 1, + status: 'confirmed', + }); + tx.set(voteRefCharlie, { + voterUid: 'charlie', + type: 'dismiss', + votedAt: Date.now(), + }); })); }); @@ -7126,5 +7204,133 @@ describe('MotorCloud Rider Swarm Road Alerts Security Rules', () => { dismissals: 1, })); }); + + test('standalone vote creation without a parent update fails', async () => { + const alice = testEnv.authenticatedContext('alice').firestore(); + const bob = testEnv.authenticatedContext('bob').firestore(); + + const alertId = 'alert_standalone_vote_fails'; + await assertSucceeds(setDoc(doc(alice, `road_alerts/${alertId}`), { + ...baseAlert, + id: alertId, + status: 'active', + confirmations: 1, + dismissals: 0, + reportedAt: Date.now() - 1000, + expiresAt: Date.now() + 3600000, + })); + + const voteRefBob = doc(bob, `road_alerts/${alertId}/votes/bob`); + + // Standalone confirmation vote without parent update fails + await assertFails(setDoc(voteRefBob, { + voterUid: 'bob', + type: 'confirm', + votedAt: Date.now(), + })); + + // Standalone dismissal vote without parent update fails + await assertFails(setDoc(voteRefBob, { + voterUid: 'bob', + type: 'dismiss', + votedAt: Date.now(), + })); + }); + + test('vote creation with unchanged totals fails', async () => { + const alice = testEnv.authenticatedContext('alice').firestore(); + const bob = testEnv.authenticatedContext('bob').firestore(); + + const alertId = 'alert_unchanged_totals_fails'; + await assertSucceeds(setDoc(doc(alice, `road_alerts/${alertId}`), { + ...baseAlert, + id: alertId, + status: 'active', + confirmations: 1, + dismissals: 0, + reportedAt: Date.now() - 1000, + expiresAt: Date.now() + 3600000, + })); + + // Transaction attempts to create vote while leaving parent counts unchanged + await assertFails(runTransaction(bob, async (tx) => { + const alertRef = doc(bob, `road_alerts/${alertId}`); + const snap = await tx.get(alertRef); + tx.update(alertRef, { + confirmations: snap.data().confirmations, + dismissals: snap.data().dismissals, + status: 'active', + }); + tx.set(doc(bob, `road_alerts/${alertId}/votes/bob`), { + voterUid: 'bob', + type: 'confirm', + votedAt: Date.now(), + }); + })); + }); + + test('confirmation vote paired with a dismissal increment fails', async () => { + const alice = testEnv.authenticatedContext('alice').firestore(); + const bob = testEnv.authenticatedContext('bob').firestore(); + + const alertId = 'alert_mismatched_conf_dism_fails'; + await assertSucceeds(setDoc(doc(alice, `road_alerts/${alertId}`), { + ...baseAlert, + id: alertId, + status: 'active', + confirmations: 1, + dismissals: 0, + reportedAt: Date.now() - 1000, + expiresAt: Date.now() + 3600000, + })); + + // Confirmation vote paired with dismissals + 1 + await assertFails(runTransaction(bob, async (tx) => { + const alertRef = doc(bob, `road_alerts/${alertId}`); + const snap = await tx.get(alertRef); + tx.update(alertRef, { + dismissals: snap.data().dismissals + 1, + confirmations: snap.data().confirmations, + status: 'active', + }); + tx.set(doc(bob, `road_alerts/${alertId}/votes/bob`), { + voterUid: 'bob', + type: 'confirm', + votedAt: Date.now(), + }); + })); + }); + + test('dismissal vote paired with a confirmation increment fails', async () => { + const alice = testEnv.authenticatedContext('alice').firestore(); + const bob = testEnv.authenticatedContext('bob').firestore(); + + const alertId = 'alert_mismatched_dism_conf_fails'; + await assertSucceeds(setDoc(doc(alice, `road_alerts/${alertId}`), { + ...baseAlert, + id: alertId, + status: 'active', + confirmations: 1, + dismissals: 0, + reportedAt: Date.now() - 1000, + expiresAt: Date.now() + 3600000, + })); + + // Dismissal vote paired with confirmations + 1 + await assertFails(runTransaction(bob, async (tx) => { + const alertRef = doc(bob, `road_alerts/${alertId}`); + const snap = await tx.get(alertRef); + tx.update(alertRef, { + confirmations: snap.data().confirmations + 1, + dismissals: snap.data().dismissals, + status: 'confirmed', + }); + tx.set(doc(bob, `road_alerts/${alertId}/votes/bob`), { + voterUid: 'bob', + type: 'dismiss', + votedAt: Date.now(), + }); + })); + }); }); }); diff --git a/firestore.rules b/firestore.rules index 4d2158b..a821685 100644 --- a/firestore.rules +++ b/firestore.rules @@ -1607,9 +1607,12 @@ service cloud.firestore { && request.resource.data.type in ['confirm', 'dismiss'] && (request.resource.data.votedAt is int || request.resource.data.votedAt is timestamp) && ( - (request.resource.data.type == 'confirm' && alertAfter.data.confirmations == alertDoc.data.confirmations + 1) - || (request.resource.data.type == 'dismiss' && alertAfter.data.dismissals == alertDoc.data.dismissals + 1) - || (alertAfter.data.confirmations == alertDoc.data.confirmations && alertAfter.data.dismissals == alertDoc.data.dismissals) + (request.resource.data.type == 'confirm' + && alertAfter.data.confirmations == alertDoc.data.confirmations + 1 + && alertAfter.data.dismissals == alertDoc.data.dismissals) + || (request.resource.data.type == 'dismiss' + && alertAfter.data.dismissals == alertDoc.data.dismissals + 1 + && alertAfter.data.confirmations == alertDoc.data.confirmations) ); }