From e4dc29f98e3a642009843ced6b28ca0bcbb5d9c2 Mon Sep 17 00:00:00 2001 From: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:52:30 +0800 Subject: [PATCH] fix(api): make the 10-minute email IP first-strike window take effect (#39479) Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> --- api/services/account_service.py | 10 +-- .../services/test_account_service.py | 63 +++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/api/services/account_service.py b/api/services/account_service.py index 85816d36106..432a2484ed6 100644 --- a/api/services/account_service.py +++ b/api/services/account_service.py @@ -1229,12 +1229,12 @@ class AccountService: if hour_limit_count >= 1: redis_client.setex(freeze_key, 60 * 60, 1) return True - else: - redis_client.setex(hour_limit_key, 60 * 10, hour_limit_count + 1) # first time limit 10 minutes - # add hour limit count - redis_client.incr(hour_limit_key) - redis_client.expire(hour_limit_key, 60 * 60) + # First strike claims a 10-minute window atomically; a concurrent + # over-limit request that loses the claim is the second strike and + # freezes the IP for an hour. + if not redis_client.set(hour_limit_key, 1, ex=60 * 10, nx=True): + redis_client.setex(freeze_key, 60 * 60, 1) return True diff --git a/api/tests/unit_tests/services/test_account_service.py b/api/tests/unit_tests/services/test_account_service.py index 6c758445c2d..13715f02820 100644 --- a/api/tests/unit_tests/services/test_account_service.py +++ b/api/tests/unit_tests/services/test_account_service.py @@ -2697,3 +2697,66 @@ def test_get_account_by_email_with_case_fallback_uses_lowercase(sqlite_session: result = AccountService.get_account_by_email_with_case_fallback("Case@Test.com", session=sqlite_session) assert result is account + + +class TestIsEmailSendIpLimit: + """The 10-minute first-strike window must actually take effect (#39477).""" + + def _mock_redis(self, *, minute_count: int, hour_count: int | None, frozen: bool = False) -> MagicMock: + values = { + "email_send_ip_limit_freeze:1.2.3.4": "1" if frozen else None, + "email_send_ip_limit_minute:1.2.3.4": str(minute_count), + "email_send_ip_limit_hour:1.2.3.4": None if hour_count is None else str(hour_count), + } + redis_client = MagicMock() + redis_client.get.side_effect = lambda key: values.get(key) + return redis_client + + def test_frozen_ip_is_limited(self): + redis_client = self._mock_redis(minute_count=0, hour_count=None, frozen=True) + with patch("services.account_service.redis_client", redis_client): + assert AccountService.is_email_send_ip_limit("1.2.3.4") is True + + def test_first_strike_sets_ten_minute_window(self): + redis_client = self._mock_redis(minute_count=999, hour_count=None) + redis_client.set.return_value = True + with ( + patch("services.account_service.redis_client", redis_client), + patch.object(dify_config, "EMAIL_SEND_IP_LIMIT_PER_MINUTE", 1), + ): + assert AccountService.is_email_send_ip_limit("1.2.3.4") is True + + redis_client.set.assert_called_once_with("email_send_ip_limit_hour:1.2.3.4", 1, ex=60 * 10, nx=True) + # No non-atomic setex/incr/expire may widen or shrink the window. + redis_client.setex.assert_not_called() + redis_client.incr.assert_not_called() + redis_client.expire.assert_not_called() + + def test_first_strike_lost_claim_freezes_immediately(self): + redis_client = self._mock_redis(minute_count=999, hour_count=None) + redis_client.set.return_value = None # another worker claimed the strike first + with ( + patch("services.account_service.redis_client", redis_client), + patch.object(dify_config, "EMAIL_SEND_IP_LIMIT_PER_MINUTE", 1), + ): + assert AccountService.is_email_send_ip_limit("1.2.3.4") is True + + redis_client.setex.assert_called_once_with("email_send_ip_limit_freeze:1.2.3.4", 60 * 60, 1) + + def test_second_strike_inside_window_freezes_for_an_hour(self): + redis_client = self._mock_redis(minute_count=999, hour_count=1) + with ( + patch("services.account_service.redis_client", redis_client), + patch.object(dify_config, "EMAIL_SEND_IP_LIMIT_PER_MINUTE", 1), + ): + assert AccountService.is_email_send_ip_limit("1.2.3.4") is True + + redis_client.setex.assert_called_once_with("email_send_ip_limit_freeze:1.2.3.4", 60 * 60, 1) + + def test_under_limit_not_limited(self): + redis_client = self._mock_redis(minute_count=0, hour_count=None) + with ( + patch("services.account_service.redis_client", redis_client), + patch.object(dify_config, "EMAIL_SEND_IP_LIMIT_PER_MINUTE", 60), + ): + assert AccountService.is_email_send_ip_limit("1.2.3.4") is False