From 11c2931efbd7995c102f9ee465fff6ad993e9a3d Mon Sep 17 00:00:00 2001
From: Collin <89503725+CollinOS@users.noreply.github.com>
Date: Mon, 1 Jun 2026 10:12:12 -0400
Subject: [PATCH] Run auth password work off the event loop
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix: run bcrypt off the event loop in auth routes

The auth routes are async, but each bcrypt call ran synchronously on the event
loop. bcrypt (checkpw/hashpw) is intentionally CPU-expensive (~100-300 ms), so
every login / signup / setup / change-password froze the single event loop for
that window, stalling all other in-flight requests (chat streams, polling, ...).

/api/auth/login is the worst case: it is reachable unauthenticated, runs bcrypt
twice (verify_password, then create_session re-verifies), and is rate-limited
only per-IP. A burst of login attempts serializes the whole server — cheap
DoS amplification.

Offload the bcrypt-bearing AuthManager calls (setup, signup/create_user,
login's verify_password + create_session, change_password) via
asyncio.to_thread, matching how the codebase already offloads blocking work
(e.g. src/builtin_actions._run_subprocess, email summarize). The event loop
stays responsive while bcrypt runs on a worker thread.

Add tests/test_auth_event_loop.py: asserts login runs verify_password and
create_session on a worker thread, not the loop thread. Fails if those calls
are awaited inline again.

* test: isolate auth event-loop test from heavy core/* import chain

The regression test imported routes.auth_routes, which pulls in
core.auth and so triggers core/__init__.py — transitively importing
src.llm_core (hangs at import under the project venv) and the SQLAlchemy
declarative models (metaclass error on a bare core.database import / under
the conftest sqlalchemy stubs). Reported by the maintainer: collection
failed on system Python and hung under the venv.

Stub core.auth/core.database before the import, mirroring the existing
_ensure_stub pattern in test_auth_regressions.py and test_null_owner_gates.py.
AuthManager is only a type hint here and the handler is exercised with a
MagicMock, so no real core machinery is needed. Test now imports cleanly
and passes in <0.3s without bcrypt/sqlalchemy installed.
---
 routes/auth_routes.py         |  11 ++--
 tests/test_auth_event_loop.py | 113 ++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+), 5 deletions(-)
 create mode 100644 tests/test_auth_event_loop.py

diff --git a/routes/auth_routes.py b/routes/auth_routes.py
index 42ba0cb..45c86ed 100644
--- a/routes/auth_routes.py
+++ b/routes/auth_routes.py
@@ -3,6 +3,7 @@
 from fastapi import APIRouter, Request, Response, HTTPException
 from pydantic import BaseModel
 from typing import Optional
+import asyncio
 import logging
 import os
 
@@ -90,7 +91,7 @@ def setup_auth_routes(auth_manager: AuthManager) -> APIRouter:
             raise HTTPException(400, "Already configured")
         if len(body.password) < 8:
             raise HTTPException(400, "Password must be at least 8 characters")
-        ok = auth_manager.setup(body.username, body.password)
+        ok = await asyncio.to_thread(auth_manager.setup, body.username, body.password)
         if not ok:
             raise HTTPException(500, "Setup failed")
         return {"ok": True, "message": "Admin account created"}
@@ -108,7 +109,7 @@ def setup_auth_routes(auth_manager: AuthManager) -> APIRouter:
             raise HTTPException(400, "Password must be at least 8 characters")
         if len(body.username.strip()) < 1:
             raise HTTPException(400, "Username is required")
-        ok = auth_manager.create_user(body.username, body.password, is_admin=False)
+        ok = await asyncio.to_thread(auth_manager.create_user, body.username, body.password, is_admin=False)
         if not ok:
             raise HTTPException(409, "Username already taken")
         return {"ok": True, "message": "Account created"}
@@ -119,7 +120,7 @@ def setup_auth_routes(auth_manager: AuthManager) -> APIRouter:
             raise HTTPException(429, "Too many requests — try again later")
         # Verify password first
         username = body.username.strip().lower()
-        if not auth_manager.verify_password(username, body.password):
+        if not await asyncio.to_thread(auth_manager.verify_password, username, body.password):
             raise HTTPException(401, "Invalid credentials")
         # Check 2FA if enabled
         if auth_manager.totp_enabled(username):
@@ -129,7 +130,7 @@ def setup_auth_routes(auth_manager: AuthManager) -> APIRouter:
             if not auth_manager.totp_verify(username, body.totp_code):
                 raise HTTPException(401, "Invalid 2FA code")
         # All checks passed — create session
-        token = auth_manager.create_session(username, body.password)
+        token = await asyncio.to_thread(auth_manager.create_session, username, body.password)
         if not token:
             raise HTTPException(401, "Invalid credentials")
         cookie_kwargs = dict(
@@ -177,7 +178,7 @@ def setup_auth_routes(auth_manager: AuthManager) -> APIRouter:
             raise HTTPException(401, "Not authenticated")
         if len(body.new_password) < 8:
             raise HTTPException(400, "Password must be at least 8 characters")
-        ok = auth_manager.change_password(user, body.current_password, body.new_password)
+        ok = await asyncio.to_thread(auth_manager.change_password, user, body.current_password, body.new_password)
         if not ok:
             raise HTTPException(400, "Current password is incorrect")
         return {"ok": True}
diff --git a/tests/test_auth_event_loop.py b/tests/test_auth_event_loop.py
new file mode 100644
index 0000000..6131256
--- /dev/null
+++ b/tests/test_auth_event_loop.py
@@ -0,0 +1,113 @@
+"""Pin that the login handler keeps bcrypt off the event loop.
+
+`/api/auth/login` is an `async def` and is reachable unauthenticated. bcrypt
+(`checkpw`/`hashpw`) is deliberately CPU-expensive (~100-300 ms). Running it
+directly in the coroutine blocks the single event loop for that whole window,
+freezing every other in-flight request (chat streams, polling, ...). Because
+the endpoint is unauthenticated and rate-limited only per-IP, a burst of login
+attempts serializes the whole server — a cheap DoS-amplification vector.
+
+The fix offloads the bcrypt-bearing AuthManager calls via asyncio.to_thread.
+This test asserts those calls run on a worker thread, not the loop thread; it
+fails if they are awaited inline again.
+"""
+import os
+import sys
+import types
+import asyncio
+import threading
+from types import SimpleNamespace
+from unittest.mock import MagicMock
+
+
+# Stub `core.auth` / `core.database` before importing the route module.
+# `routes.auth_routes` does `from core.auth import AuthManager`, and importing
+# any `core.*` submodule first runs `core/__init__.py`, which transitively
+# imports `src.llm_core` (hangs at import under the project venv) and the
+# SQLAlchemy declarative models (metaclass blows up on a bare `core.database`
+# import / under the conftest's `sqlalchemy.*` MagicMock stubs). We only need
+# `AuthManager` as a type hint here — the handler is exercised with a MagicMock
+# — so stub the heavy modules out. Same trick as test_auth_regressions.py /
+# test_null_owner_gates.py.
+def _ensure_stub(name: str, **attrs):
+    """Create or augment a stub module, wiring it onto a stubbed parent package.
+
+    Augments existing entries because an earlier-run test may have already
+    stubbed the same module with a different attribute set. The parent package
+    gets `__path__` pointed at the real on-disk dir so genuinely-unstubbed
+    submodules still load normally, while `core/__init__.py` itself is bypassed
+    (the package is already in `sys.modules`)."""
+    if "." in name:
+        parent_name, _, child_name = name.rpartition(".")
+        if parent_name not in sys.modules:
+            parent = types.ModuleType(parent_name)
+            real_path = os.path.join(
+                os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
+                *parent_name.split("."),
+            )
+            parent.__path__ = [real_path] if os.path.isdir(real_path) else []
+            sys.modules[parent_name] = parent
+        else:
+            parent = sys.modules[parent_name]
+    else:
+        parent = None
+        child_name = None
+
+    mod = sys.modules.get(name)
+    if mod is None:
+        mod = types.ModuleType(name)
+        sys.modules[name] = mod
+    for k, v in attrs.items():
+        if not hasattr(mod, k):
+            setattr(mod, k, v)
+    if parent is not None and not hasattr(parent, child_name):
+        setattr(parent, child_name, mod)
+    return mod
+
+
+_ensure_stub("core.database", SessionLocal=MagicMock())
+_ensure_stub("core.auth", AuthManager=MagicMock())
+
+from routes.auth_routes import setup_auth_routes, LoginRequest
+
+
+def _login_endpoint(auth_manager):
+    router = setup_auth_routes(auth_manager)
+    for r in router.routes:
+        if getattr(r, "path", None) == "/api/auth/login" and "POST" in getattr(r, "methods", set()):
+            return r.endpoint
+    raise AssertionError("login route not found on the auth router")
+
+
+def test_login_runs_bcrypt_off_the_event_loop():
+    loop_thread = threading.get_ident()
+    seen = {}
+
+    auth = MagicMock()
+
+    def _verify(username, password):
+        seen["verify_thread"] = threading.get_ident()
+        return True
+
+    def _create(username, password):
+        seen["create_thread"] = threading.get_ident()
+        return "tok-123"
+
+    auth.verify_password.side_effect = _verify
+    auth.totp_enabled.return_value = False
+    auth.create_session.side_effect = _create
+
+    login = _login_endpoint(auth)
+
+    request = SimpleNamespace(client=SimpleNamespace(host="203.0.113.7"), cookies={})
+    response = MagicMock()
+    body = LoginRequest(username="alice", password="hunter2", remember=True)
+
+    result = asyncio.run(login(body=body, request=request, response=response))
+
+    assert result["ok"] is True
+    auth.verify_password.assert_called_once()
+    auth.create_session.assert_called_once()
+    # The whole point: the expensive bcrypt calls must NOT run on the loop thread.
+    assert seen["verify_thread"] != loop_thread, "verify_password ran on the event-loop thread"
+    assert seen["create_thread"] != loop_thread, "create_session ran on the event-loop thread"