From 6990d1494fa377261d05631dd599efb496e16968 Mon Sep 17 00:00:00 2001
From: Rezha Julio <rezha.pradana@gojek.com>
Date: Sat, 4 Jul 2026 11:46:09 +0700
Subject: [PATCH] fix(captcha): guard against duplicate callback delivery race

Double-tapping the verify button raced two callback deliveries through
check_user_profile before either delivery removed the pending captcha
row, risking a double unrestrict_user + start_new_user_probation call.
remove_pending_captcha's existing delete-rowcount now gates finalization
so only the delivery that actually removes the row proceeds.

Also cleans trailing whitespace in user_checker.py and ignores the
local .tokensave/ cache directory.
---
 .gitignore                       |  2 ++
 src/bot/handlers/captcha.py      |  8 +++++++-
 src/bot/services/user_checker.py |  4 ++--
 tests/test_captcha.py            | 31 +++++++++++++++++++++++++++++++
 4 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/.gitignore b/.gitignore
index 7409ff5..b4aa9a5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -18,3 +18,5 @@ docs/
 
 # Local review artifacts (parallel-reviewer outputs)
 reviews/
+
+.tokensave/
diff --git a/src/bot/handlers/captcha.py b/src/bot/handlers/captcha.py
index c377cee..1910b60 100644
--- a/src/bot/handlers/captcha.py
+++ b/src/bot/handlers/captcha.py
@@ -308,7 +308,13 @@ async def captcha_callback_handler(
     # DB finalization first (reversible). If it fails, user stays restricted
     # in Telegram, DB row preserved, timeout still armed — user can retry.
     try:
-        db.remove_pending_captcha(target_user_id, group_config.group_id)
+        removed = db.remove_pending_captcha(target_user_id, group_config.group_id)
+        if not removed:
+            # Duplicate callback delivery (e.g. double-tap) raced us here —
+            # the other delivery already finalized this user. Ack and stop.
+            logger.info(f"Captcha for user {target_user_id} already finalized, ignoring duplicate callback")
+            await query.answer()
+            return
         db.start_new_user_probation(target_user_id, group_config.group_id)
     except Exception:
         logger.error(f"DB finalization failed for user {target_user_id}", exc_info=True)
diff --git a/src/bot/services/user_checker.py b/src/bot/services/user_checker.py
index 601bf40..2794f64 100644
--- a/src/bot/services/user_checker.py
+++ b/src/bot/services/user_checker.py
@@ -73,7 +73,7 @@ async def check_user_profile(bot: Bot, user: User) -> ProfileCheckResult:
         ProfileCheckResult: Result containing photo and username status.
     """
     logger.info(f"Checking profile for user_id={user.id}")
-    
+
     has_username = user.username is not None
 
     db = get_database()
@@ -96,5 +96,5 @@ async def check_user_profile(bot: Bot, user: User) -> ProfileCheckResult:
     logger.info(
         f"Profile check for user_id={user.id}: has_photo={has_profile_photo}, has_username={has_username}"
     )
-    
+
     return result
diff --git a/tests/test_captcha.py b/tests/test_captcha.py
index 8f0271e..1f46513 100644
--- a/tests/test_captcha.py
+++ b/tests/test_captcha.py
@@ -455,6 +455,37 @@ async def test_db_finalization_failure_keeps_user_restricted(
         # Telegram unrestrict was NOT called (DB failure short-circuits).
         mock_unrestrict.assert_not_called()
 
+    async def test_duplicate_callback_delivery_ignored(
+        self, mock_context, mock_registry, temp_db
+    ):
+        """remove_pending_captcha returns False when another concurrent
+        delivery of the same double-tap already finalized this user.
+        The duplicate must ack quietly and never re-run unrestrict/probation."""
+        from bot.database.service import get_database
+
+        db = get_database()
+        db.add_pending_captcha(12345, -1001234567890, -1001234567890, 999, "Test User")
+
+        query = self._make_callback_query()
+
+        update = MagicMock()
+        update.callback_query = query
+
+        with (
+            patch("bot.handlers.captcha.get_group_registry", return_value=mock_registry),
+            patch("bot.handlers.captcha.check_user_profile", return_value=ProfileCheckResult(has_profile_photo=True, has_username=True)),
+            patch("bot.handlers.captcha.unrestrict_user") as mock_unrestrict,
+            patch.object(db, "remove_pending_captcha", return_value=False),
+            patch.object(db, "start_new_user_probation") as mock_probation,
+        ):
+            await captcha_callback_handler(update, mock_context)
+
+        # Bare ack so the client's spinner clears — no alert, no success edit.
+        query.answer.assert_called_once_with()
+        query.edit_message_text.assert_not_called()
+        mock_unrestrict.assert_not_called()
+        mock_probation.assert_not_called()
+
     async def test_edit_message_failure_in_callback_continues_gracefully(
         self, mock_context, mock_registry, temp_db
     ):