Run auth password work off the event loop

* 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.
This commit is contained in:
Collin
2026-06-01 10:12:12 -04:00
committed by GitHub
parent a51a1fc4fc
commit 11c2931efb
2 changed files with 119 additions and 5 deletions
+6 -5
View File
@@ -3,6 +3,7 @@
from fastapi import APIRouter, Request, Response, HTTPException from fastapi import APIRouter, Request, Response, HTTPException
from pydantic import BaseModel from pydantic import BaseModel
from typing import Optional from typing import Optional
import asyncio
import logging import logging
import os import os
@@ -90,7 +91,7 @@ def setup_auth_routes(auth_manager: AuthManager) -> APIRouter:
raise HTTPException(400, "Already configured") raise HTTPException(400, "Already configured")
if len(body.password) < 8: if len(body.password) < 8:
raise HTTPException(400, "Password must be at least 8 characters") 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: if not ok:
raise HTTPException(500, "Setup failed") raise HTTPException(500, "Setup failed")
return {"ok": True, "message": "Admin account created"} 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") raise HTTPException(400, "Password must be at least 8 characters")
if len(body.username.strip()) < 1: if len(body.username.strip()) < 1:
raise HTTPException(400, "Username is required") 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: if not ok:
raise HTTPException(409, "Username already taken") raise HTTPException(409, "Username already taken")
return {"ok": True, "message": "Account created"} 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") raise HTTPException(429, "Too many requests — try again later")
# Verify password first # Verify password first
username = body.username.strip().lower() 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") raise HTTPException(401, "Invalid credentials")
# Check 2FA if enabled # Check 2FA if enabled
if auth_manager.totp_enabled(username): 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): if not auth_manager.totp_verify(username, body.totp_code):
raise HTTPException(401, "Invalid 2FA code") raise HTTPException(401, "Invalid 2FA code")
# All checks passed — create session # 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: if not token:
raise HTTPException(401, "Invalid credentials") raise HTTPException(401, "Invalid credentials")
cookie_kwargs = dict( cookie_kwargs = dict(
@@ -177,7 +178,7 @@ def setup_auth_routes(auth_manager: AuthManager) -> APIRouter:
raise HTTPException(401, "Not authenticated") raise HTTPException(401, "Not authenticated")
if len(body.new_password) < 8: if len(body.new_password) < 8:
raise HTTPException(400, "Password must be at least 8 characters") 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: if not ok:
raise HTTPException(400, "Current password is incorrect") raise HTTPException(400, "Current password is incorrect")
return {"ok": True} return {"ok": True}
+113
View File
@@ -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"