fix(security): harden password recovery with usable eight-character codes (#2477)

Use eight securely random recovery-code characters with concurrency-safe online throttling. Preserve existing keys and verify recovery through browser and real SQLite integration tests.

Co-authored-by: zhangjinpeng@mail.tuchong.com <zhangjinpeng@mail.tuchong.com>
Co-authored-by: dadachann <185672915+dadachann@users.noreply.github.com>

src/langbot/pkg/api/http/controller/groups/user.py

@@ -2,6 +2,8 @@ import quart
 import argon2
 import asyncio
 import datetime
+import hmac
+import time
 import uuid
 from urllib.parse import parse_qs, urlsplit
 
@@ -11,6 +13,33 @@ from ...context import RequestContext
 from .....cloud.launch import SpaceLaunchError
 from ...service.user import ControlPlaneDirectoryRequiredError, PublicRegistrationClosedError
 
+# Fixed-window admission quota for the unauthenticated reset-password endpoint (#2392).
+# The admission check and slot bump share ONE synchronous critical section with no await
+# points, so concurrent bursts within a single event loop cannot slip past accounting.
+# Every admitted attempt consumes quota (regardless of success), which throttles both the
+# legacy 24-bit keyspace exhaustion and brute-force on modern high-entropy keys.
+# NOTE: this state is process-local; multi-worker deployments need a shared limiter upstream.
+_MAX_RESET_ATTEMPTS_PER_WINDOW = 5
+_RESET_WINDOW_SECONDS = 15 * 60
+
+_reset_password_state: dict = {'window_started_at': 0.0, 'attempts': 0}
+
+
+def _admit_reset_attempt(now: float) -> bool:
+    """Atomically reserve one reset-password admission slot.
+
+    Must stay await-free: running to completion without suspension makes the
+    check-and-increment atomic under the single-threaded event loop.
+    """
+    st = _reset_password_state
+    if now - st['window_started_at'] >= _RESET_WINDOW_SECONDS:
+        st['window_started_at'] = now
+        st['attempts'] = 0
+    if st['attempts'] >= _MAX_RESET_ATTEMPTS_PER_WINDOW:
+        return False
+    st['attempts'] += 1
+    return True
+
 
 @group.group_class('user', '/api/v1/user')
 class UserRouterGroup(group.RouterGroup):
@@ -81,6 +110,12 @@ class UserRouterGroup(group.RouterGroup):
 
         @self.route('/reset-password', methods=['POST'], auth_type=group.AuthType.NONE)
         async def _() -> str:
+            # Admit (or reject) BEFORE touching the body or any service call (#2392):
+            # rejecting requests never reach the slow path, and quota accounting happens
+            # synchronously at entry, closing the post-await race of burst requests.
+            if not _admit_reset_attempt(time.monotonic()):
+                return self.http_status(429, -1, 'Too many attempts, try again later')
+
             json_data = await quart.request.json
 
             user_email = json_data['user']
@@ -98,7 +133,18 @@ class UserRouterGroup(group.RouterGroup):
             if user_obj is None:
                 return self.http_status(400, -1, 'User not found')
 
-            if recovery_key != self.ap.instance_config.data['system']['recovery_key']:
+            stored_key = self.ap.instance_config.data['system']['recovery_key']
+            try:
+                key_matches = (
+                    isinstance(recovery_key, str)
+                    and isinstance(stored_key, str)
+                    and hmac.compare_digest(recovery_key.encode(), stored_key.encode())
+                )
+            except UnicodeEncodeError:
+                # JSON can contain lone surrogates, which are not valid UTF-8.
+                key_matches = False
+
+            if not key_matches:
                 return self.http_status(403, -1, 'Invalid recovery key')
 
             await self.ap.user_service.reset_password(user_email, new_password)

src/langbot/pkg/core/stages/genkeys.py

@@ -1,9 +1,18 @@
 from __future__ import annotations
 
+import logging
 import secrets
 
 from .. import stage, app
 
+# This stage runs before SetupLoggerStage, so ap.logger is still None here;
+# the module logger falls back to the stderr lastResort handler.
+_logger = logging.getLogger(__name__)
+
+# 32 symbols without 0/O or 1/I; eight independent draws provide 40 random bits.
+_RECOVERY_KEY_ALPHABET = '23456789ABCDEFGHJKLMNPQRSTUVWXYZ'
+_RECOVERY_KEY_LENGTH = 8
+
 
 @stage.stage_class('GenKeysStage')
 class GenKeysStage(stage.BootingStage):
@@ -20,5 +29,15 @@ class GenKeysStage(stage.BootingStage):
             ap.instance_config.data['system']['recovery_key'] = ''
 
         if not ap.instance_config.data['system']['recovery_key']:
-            ap.instance_config.data['system']['recovery_key'] = secrets.token_hex(3).upper()
+            # Keep recovery practical to type. Security also requires the reset
+            # endpoint's concurrency-safe quota (five admissions per 15 minutes).
+            ap.instance_config.data['system']['recovery_key'] = ''.join(
+                secrets.choice(_RECOVERY_KEY_ALPHABET) for _ in range(_RECOVERY_KEY_LENGTH)
+            )
             await ap.instance_config.dump_config()
+        elif len(ap.instance_config.data['system']['recovery_key']) < _RECOVERY_KEY_LENGTH:
+            _logger.warning(
+                'Low-entropy legacy recovery key detected (length < 8); '
+                'regenerate system.recovery_key in the configuration file '
+                'with a strong random value (#2392)'
+            )