fix(tradein/auth): bcrypt вне событийного цикла + настоящий потолок темпа логинов (#2665) #2712

Merged

bot-backend merged 3 commits from fix/2665-bcrypt-offloop-and-throttle into main 2026-08-06 09:02:11 +00:00

Conversation 1 Commits 3 Files changed 5 +464 -5

bot-backend commented 2026-08-06 08:16:53 +00:00

Collaborator

Copy link

Summary

verify_password звалась синхронно внутри async def login. Замер в прод-контейнере (2026-08-06): bcrypt cost 12 (все 13 живых хешей — \$2b\$12\$) = 282 мс медиана, и всё это время единственный event loop стоял целиком: 3.6 проверки/с, стойло цикла до 836 мс. Форма входа публична с cutover'а #2571 → любой клал весь трейд-ин без единого валидного пароля.

Та же блокировка была единственным настоящим потолком темпа. Замедление из #2571 потолком не является: await asyncio.sleep отпускает цикл, сотня соединений отспит его параллельно.

Обе половины в одной функции verify_password_bounded — состояние «вынесено, потолка нет» невыразимо:

• потолок = размер пула проверок, дефолт 1 поток → те же ~3.5/с, что случайно давала блокировка (вынос не ускоряет перебор);
• сверх очереди (login_password_verify_max_inflight, 4) — сразу 429, без ожидания: ждущий держит соединение к БД, а в QueuePool их 5+10.

Почему в процессе, а не в Redis — проверено, а не предположено: прод-бэкенд uvicorn app.main:app без --workers; REDIS_URL в окружении tradein-backend не задан вовсе (printenv | grep -c ^REDIS_URL= → 0, находка #2674). Потолок на Redis молча не работал бы.

Почему не периметр (вариант 1 из issue): в стоковом caddy:2 модуля rate_limit нет — caddy list-modules на проде выдаёт 134 модуля, ни одного с rate_limit. Это пересборка образа через xcaddy + правка инфраструктуры, и без теста. Вынесено владельцу отдельным предложением.

#2571 не ослаблен: оба лимитера, счётчик неудач на имя и растущая задержка — как были. 429 при насыщении отдаётся ДО сверки, одинаково для любого имени, бюджет неудач по имени не тратит.

Test plan

• test_login_flood_capped_by_rate_while_api_stays_responsive — 100 одновременных соединений, у каждого запроса СВОЯ пара (username, ip): сценарий, где обе защиты #2571 не срабатывают ни разу. Меряет ТЕМП (сверок/с за секунду непрерывного флуда), а не латентность одного ответа, и одновременно — что сторонний запрос обслуживается.
• Обе половины фальсифицированы на этом же тесте: убрать вынос в поток → худший сторонний запрос 1756мс — API встаёт под флудом входа; убрать потолок (asyncio.to_thread) → 203 сверок/с при потолке 20/с — потолок темпа не работает.
• tests/test_password.py — вердикт не изменился, bcrypt считается в чужом потоке, избыток отклоняется а не копится, слоты возвращаются.
• Полный прогон tradein-backend: 3230 passed. Единственная краснота — test_search_api.py::test_search_cache_hit, воспроизводится на чистом origin/main в том же окружении (не из этого PR).
• pre-commit (ruff 0.7.4 + format) — зелёный.

Миграция не потребовалась (выделенный номер 222 не израсходован).

Refs #2665

## Summary `verify_password` звалась синхронно внутри `async def login`. **Замер в прод-контейнере (2026-08-06):** bcrypt cost 12 (все 13 живых хешей — `\$2b\$12\$`) = **282 мс медиана**, и всё это время единственный event loop стоял целиком: **3.6 проверки/с, стойло цикла до 836 мс**. Форма входа публична с cutover'а #2571 → любой клал весь трейд-ин без единого валидного пароля. Та же блокировка была **единственным настоящим потолком темпа**. Замедление из #2571 потолком не является: `await asyncio.sleep` отпускает цикл, сотня соединений отспит его параллельно. **Обе половины в одной функции** `verify_password_bounded` — состояние «вынесено, потолка нет» невыразимо: - потолок = размер пула проверок, дефолт **1 поток → те же ~3.5/с**, что случайно давала блокировка (вынос не ускоряет перебор); - сверх очереди (`login_password_verify_max_inflight`, 4) — сразу **429, без ожидания**: ждущий держит соединение к БД, а в QueuePool их 5+10. **Почему в процессе, а не в Redis** — проверено, а не предположено: прод-бэкенд `uvicorn app.main:app` без `--workers`; `REDIS_URL` в окружении tradein-backend **не задан вовсе** (`printenv | grep -c ^REDIS_URL=` → 0, находка #2674). Потолок на Redis молча не работал бы. **Почему не периметр (вариант 1 из issue):** в стоковом `caddy:2` модуля rate_limit нет — `caddy list-modules` на проде выдаёт 134 модуля, ни одного с `rate_limit`. Это пересборка образа через xcaddy + правка инфраструктуры, и без теста. Вынесено владельцу отдельным предложением. **#2571 не ослаблен:** оба лимитера, счётчик неудач на имя и растущая задержка — как были. 429 при насыщении отдаётся ДО сверки, одинаково для любого имени, бюджет неудач по имени не тратит. ## Test plan - [x] `test_login_flood_capped_by_rate_while_api_stays_responsive` — 100 одновременных соединений, у каждого запроса СВОЯ пара (username, ip): сценарий, где обе защиты #2571 не срабатывают ни разу. Меряет ТЕМП (сверок/с за секунду непрерывного флуда), а не латентность одного ответа, и одновременно — что сторонний запрос обслуживается. - [x] Обе половины фальсифицированы на этом же тесте: убрать вынос в поток → `худший сторонний запрос 1756мс — API встаёт под флудом входа`; убрать потолок (`asyncio.to_thread`) → `203 сверок/с при потолке 20/с — потолок темпа не работает`. - [x] `tests/test_password.py` — вердикт не изменился, bcrypt считается в чужом потоке, избыток отклоняется а не копится, слоты возвращаются. - [x] Полный прогон tradein-backend: 3230 passed. Единственная краснота — `test_search_api.py::test_search_cache_hit`, воспроизводится на чистом origin/main в том же окружении (не из этого PR). - [x] pre-commit (ruff 0.7.4 + format) — зелёный. Миграция не потребовалась (выделенный номер 222 не израсходован). Refs #2665

bot-backend added 1 commit 2026-08-06 08:16:53 +00:00

fix(tradein/auth): bcrypt вне событийного цикла + настоящий потолок темпа логинов ... e8dda242c7

`verify_password` звалась синхронно внутри `async def login`. Замер в
прод-контейнере: bcrypt cost 12 (все живые хеши `$2b$12$`) = 282 мс медиана,
и всё это время единственный event loop backend'а стоял целиком — 3.6
проверки/с, стойло цикла до 836 мс. Форма входа публична с cutover'а #2571,
значит любой желающий клал ВЕСЬ трейд-ин, не зная ни одного пароля.

Та же блокировка была единственным настоящим потолком темпа: замедление из
#2571 (`await asyncio.sleep`) отпускает цикл, поэтому сотня соединений отспит
его параллельно — это латентность одного ответа, а не ограничение темпа.
Поэтому обе половины едут вместе и живут в ОДНОЙ функции
(`verify_password_bounded`): вынос без потолка ускорил бы перебор (замерено
16/с на дефолтном executor'е), потолок без выноса оставил бы отказ в
обслуживании. Состояние «вынесено, потолка нет» в коде невыразимо.

Потолок = размер пула проверок, дефолт 1 поток → те же ~3.5 проверки/с, что
случайно давала блокировка, но цикл свободен. Сверх очереди
(`login_password_verify_max_inflight`, 4) — сразу 429, без ожидания: ждущий
запрос держит соединение к БД, а в QueuePool их 5+10.

Потолок держится процессом, и это проверено, а не предположено: прод-бэкенд
запущен `uvicorn app.main:app` без `--workers`, а REDIS_URL в окружении
tradein-backend не задан вовсе (находка #2674) — потолок на Redis молча не
работал бы. Периметр (Caddy) не выбран: в стоковом caddy:2 модуля rate_limit
нет (`caddy list-modules` — 134 модуля, ни одного с rate_limit), это была бы
пересборка образа и правка инфраструктуры без теста.

Защиты #2571 не ослаблены: оба лимитера, счётчик неудач на имя и растущая
задержка остались как были; 429 при насыщении отдаётся ДО сверки, одинаково
для любого имени, и бюджет неудач по имени не тратит.

Тест меряет ТЕМП, а не латентность: 100 одновременных соединений, каждое со
своей парой (username, ip) — сценарий, в котором обе защиты #2571 не
срабатывают ни разу. Проверяется и потолок сверок/с, и то, что сторонний
запрос при этом обслуживается. Обе половины фальсифицированы: убрать вынос →
«худший сторонний запрос 1756мс», убрать потолок → «203 сверок/с при потолке
20/с».

Refs #2665

bot-backend referenced this pull request 2026-08-06 08:17:13 +00:00

[HIGH] tradein/auth: bcrypt блокирует единственный event loop — поток логинов кладёт весь API, и он же сейчас единственный настоящий ограничитель темпа #2665

Light1YT added 1 commit 2026-08-06 08:25:16 +00:00

fix(tradein/auth): слот проверки пароля освобождает работа, а не отмена запроса ... bce5b0c02f

Колбэк висел на обёртке `run_in_executor`: у неё «готово» наступает и при
ОТМЕНЕ корутины, а подхваченная пулом задача при этом продолжает занимать
поток свои 282 мс. Значит отваливающийся клиент получал свежий слот на каждую
отмену и мог набивать очередь пула быстрее, чем та разгребается — темп bcrypt
по-прежнему держал бы пул, но очередь и память росли бы без границы.

Колбэк перевешен на future ПУЛА (`submit`), декремент возвращается в поток
цикла через `call_soon_threadsafe` — счётчик остаётся собственностью цикла и
живёт без лока. Тест на отмену ловит ровно эту разницу: он краснел на
предыдущей реализации.

Refs #2665

bot-backend commented 2026-08-06 08:30:57 +00:00

Author

Collaborator

Copy link

Замер ДО (прод-контейнер tradein-backend, 2026-08-06)

Скрипт гонялся внутри контейнера, БД не трогал:

hash prefix: $2b$12$ | bcrypt version: 5.0.0
verify_password: median=282ms min=274ms max=296ms
INLINE   : 8 verifies in 2.23s = 3.6/s | max event-loop stall 836ms
THREADED : 8 verifies in 0.50s = 16.0/s | max event-loop stall 6ms

Строка INLINE — сегодняшний прод: 3.6 попытки/с ценой того, что цикл не обслуживает никого до 836 мс.
Строка THREADED — то, что получилось бы от «просто вынести в поток»: 16/с, перебор вчетверо быстрее.

Косвенное подтверждение снаружи кода: POST /api/v1/auth/login с несуществующим именем отвечает 401 за 362 мс (bcrypt против dummy-хеша доминирует) — это и есть та самая работа, что держала цикл.

Фальсификация теста (обе половины по очереди сломаны в рабочей копии)

что сломано	что сказал тест
вынос в поток убран (потолок оставлен)	худший сторонний запрос 1756мс — API встаёт под флудом входа
потолок убран (asyncio.to_thread)	218 сверок/с при потолке 20/с (317 за 1.45с) — потолок темпа не работает
ничего не сломано	passed

Третья фальсификация нашла реальный дефект в первой редакции этого же PR: колбэк освобождения слота висел на обёртке run_in_executor, у которой «готово» наступает и при ОТМЕНЕ корутины — отменяющий клиент получал свежий слот на каждую отмену. Перевешен на future пула (второй коммит), тест test_bounded_slot_freed_by_the_work_not_by_cancellation краснеет на прежней реализации.

Доступность хранилища, на которое опирается потолок

docker exec tradein-backend printenv | grep -c ^REDIS_URL=   →  0
docker inspect tradein-backend --format {{.Config.Cmd}}      →  [uvicorn app.main:app --host 0.0.0.0 --port 8000]   (без --workers)
docker exec gendesign-caddy-1 caddy list-modules | wc -l     →  134
docker exec gendesign-caddy-1 caddy list-modules | grep -c rate_limit  →  0

Потолок в памяти процесса — единственный, который РЕАЛЬНО работает в этом окружении: Redis не настроен (тот же класс дефекта, что #2674), в стоковом caddy:2 рейт-лимита нет вовсе.

Известный размен

Запросы, отбитые 429 при насыщении, НЕ попадают в user_events (пароль не проверялся, это не попытка входа) — они видны только как login rejected: password verify saturated ip=... в логах приложения. Порог достигается только при флуде, но если захочется видеть насыщение в аудите — это отдельная правка.

### Замер ДО (прод-контейнер tradein-backend, 2026-08-06) Скрипт гонялся внутри контейнера, БД не трогал: ``` hash prefix: $2b$12$ | bcrypt version: 5.0.0 verify_password: median=282ms min=274ms max=296ms INLINE : 8 verifies in 2.23s = 3.6/s | max event-loop stall 836ms THREADED : 8 verifies in 0.50s = 16.0/s | max event-loop stall 6ms ``` Строка INLINE — сегодняшний прод: 3.6 попытки/с ценой того, что цикл не обслуживает никого до 836 мс. Строка THREADED — то, что получилось бы от «просто вынести в поток»: 16/с, перебор вчетверо быстрее. Косвенное подтверждение снаружи кода: `POST /api/v1/auth/login` с несуществующим именем отвечает 401 за **362 мс** (bcrypt против dummy-хеша доминирует) — это и есть та самая работа, что держала цикл. ### Фальсификация теста (обе половины по очереди сломаны в рабочей копии) | что сломано | что сказал тест | |---|---| | вынос в поток убран (потолок оставлен) | `худший сторонний запрос 1756мс — API встаёт под флудом входа` | | потолок убран (`asyncio.to_thread`) | `218 сверок/с при потолке 20/с (317 за 1.45с) — потолок темпа не работает` | | ничего не сломано | passed | Третья фальсификация нашла реальный дефект в первой редакции этого же PR: колбэк освобождения слота висел на обёртке `run_in_executor`, у которой «готово» наступает и при ОТМЕНЕ корутины — отменяющий клиент получал свежий слот на каждую отмену. Перевешен на future пула (второй коммит), тест `test_bounded_slot_freed_by_the_work_not_by_cancellation` краснеет на прежней реализации. ### Доступность хранилища, на которое опирается потолок ``` docker exec tradein-backend printenv | grep -c ^REDIS_URL= → 0 docker inspect tradein-backend --format {{.Config.Cmd}} → [uvicorn app.main:app --host 0.0.0.0 --port 8000] (без --workers) docker exec gendesign-caddy-1 caddy list-modules | wc -l → 134 docker exec gendesign-caddy-1 caddy list-modules | grep -c rate_limit → 0 ``` Потолок в памяти процесса — единственный, который РЕАЛЬНО работает в этом окружении: Redis не настроен (тот же класс дефекта, что #2674), в стоковом caddy:2 рейт-лимита нет вовсе. ### Известный размен Запросы, отбитые 429 при насыщении, НЕ попадают в `user_events` (пароль не проверялся, это не попытка входа) — они видны только как `login rejected: password verify saturated ip=...` в логах приложения. Порог достигается только при флуде, но если захочется видеть насыщение в аудите — это отдельная правка.

bot-backend referenced this pull request 2026-08-06 08:53:54 +00:00

[HIGH] tradein/auth: потолок сверок бьёт и по своим — один IP на 5 запросах в секунду закрывает вход всем, бессрочно #2714

bot-backend referenced this pull request 2026-08-06 08:54:30 +00:00

tradein/auth: хвост после #2712 — насыщение без следа в аудите и с логом, выселяющим прочие логи; гейт после выборки из БД; правило только в коммент… #2715

Light1YT added 1 commit 2026-08-06 08:58:19 +00:00

fix(tradein/auth): дефолты потолка закреплены тестом, границы настроек, точные обоснования ... d5e15423d8

Ревью показало, что «разрыва вынесено-без-потолка не существует» верно только
по коду: `LOGIN_PASSWORD_VERIFY_WORKERS=32` в .env.runtime даёт ровно это
состояние, без единой правки и без ревью — и наступит оно именно так, под
предлогом «входы тормозят».

M1. Тест про темп вычисляет ожидаемый потолок из той же настройки, которую
охраняет, поэтому мутант дефолтов (workers 1→16, inflight 4→64) оставлял все
58 тестов зелёными. Дефолты теперь стережёт отдельный тест: workers==1 и
очередь строго уже пула соединений (5+10). На том же мутанте краснеет.
Комментарий у теста темпа больше не обещает того, чего тот не делает.

L2. `ge=1` на обе настройки. Проверено запуском: 0/-1 в workers роняли
ThreadPoolExecutor на импорте (crash-loop контейнера), 0 в max_inflight отдавал
429 на КАЖДЫЙ вход навсегда и молча — а «0» это естественная попытка выключить
лимит. Теперь отказ на валидации настроек, с именем поля.

Info. Комментарий объяснял фикс не той причиной: отмена не набивает очередь —
не начатую работу `cancel()` снимает. Настоящий вред прежней редакции —
освобождение слота при отмене УЖЕ НАЧАТОЙ сверки (поток занят, а слот числится
свободным), это и ловит тест. Обоснование переписано в коде и в тесте.

Плюс две оговорки: потолок множится и на `WEB_CONCURRENCY` (uvicorn читает
число процессов оттуда, а .env.runtime правится руками), а правило «из async
def только verify_password_bounded» относится к сверке — `hash_password` в
team.py оставлен на цикле сознательно как редкая аутентифицированная операция.

Refs #2665

bot-backend merged commit 9d8114158b into main 2026-08-06 09:02:11 +00:00

bot-backend deleted branch fix/2665-bcrypt-offloop-and-throttle 2026-08-06 09:02:11 +00:00

bot-backend referenced this pull request from a commit 2026-08-06 09:02:13 +00:00

fix(tradein/auth): bcrypt вне событийного цикла + настоящий потолок темпа логинов (#2665) (#2712)

Light1YT referenced this pull request from a commit 2026-08-06 09:39:02 +00:00

fix(tradein/auth): доля слотов сверки пароля на адрес — потолок перестаёт бить по своим (#2714)

bot-backend referenced this pull request 2026-08-06 09:39:47 +00:00

fix(tradein/auth): доля слотов сверки пароля на адрес — потолок перестаёт бить по своим (#2714) #2717

bot-backend referenced this pull request 2026-08-06 23:31:02 +00:00

tradein/frontend: у фронтенда trade-in ноль проверок поведения — при том что сегодня через него прошли три видимых пользователю изменения #2766

lekss361 referenced this pull request 2026-09-03 07:05:35 +00:00

Флаки: test_auth_api.py — замер «худший сторонний запрос < 500 мс» падает на нагруженном раннере и роняет деплой tradein #3343

Light1YT referenced this pull request from a commit 2026-09-05 17:48:49 +00:00

test(auth): проверять вынос bcrypt механизмом, а не секундомером

bot-backend referenced this pull request 2026-09-05 17:52:03 +00:00

test(tradein/auth): флаки-замер «< 500 мс» заменён проверкой механизма — bcrypt обязан исполняться вне потока event loop #3350

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

Fable 5 ревью

Находки полного мульти-агентного ревью (Claude Fable 5, 2026-06-09)

  

GG-форсайт

Новое ТЗ от Ген Геныча (июнь 2026): Site Finder v2 — ИИ-прогноз продуктовой стратегии девелопера. Актуальный приоритет.

  

admin

Admin UI / internal tools / pipeline tracker

  

analytics

Аналитика данных / matching

  

auth

Authentication / authorization / multi-tenancy

  

automation

Автоматизация / cron / scheduler

  

bug

Something isn't working

  

business

Коммерческая воронка / пилоты / pipeline / реестр ПО

  

chore

Operational housekeeping

  

ci

Forgejo Actions / build pipeline

  

compliance

152-ФЗ / реестр ПО / legal / TOS

  

data

Data sources, parsers, ETL

  

data-moat

Уникальные данные — sales tracker, CRM aggregation

  

docs

README / runbooks / onboarding

  

duplicate

This issue or pull request already exists

  

dx

Developer experience / build speed

  

enhancement

New feature or request

  

feedback/max

Feedback from Max (Senior partner)

  

generative

Generative Design — concept generation engine

  

needs-discussion

Pending team sync / decision

  

needs-human

Human review/decision required

  

observability

Monitoring / metrics / logs / Sentry / Uptime

  

pause-bots

Kill-switch — all autonomous bots pause

  

performance

Latency / throughput / DB query tuning

  

priority/p0

Critical / production-blocker

  

priority/p1

High priority

  

priority/p2

Normal priority

  

priority/p3

Низкий приоритет (follow-up, tech-debt)

  

scope/backend

Backend Python — FastAPI, Celery, scrapers

  

scope/db

DB migrations, indexes, SQL

  

scope/devops

docker-compose, Caddy, workflows

  

scope/frontend

Frontend — Next.js, React, TypeScript

  

scope/qa

Tests, smoke, Playwright

  

scrapers

Скрапперы (Avito/Cian/Yandex/Rosreestr)

  

security

Security audit / secrets / vulnerabilities

  

site-finder

Site Finder scoring quality & data layer

  

stage/1

Stage 1: Generative MVP (нед 2-7)

  

stage/2

Stage 2: Site Finder MVP (нед 8-12)

  

status/blocked

Blocked — needs human intervention

  

status/done

Closed, completed

  

status/needs-analysis

Raw/fuzzy task filed by human — auto-analyst подхватывает: code-archeology + декомпозиция в actionable scope/* + status/ready (или split на под-issues)

  

status/needs-fix

Reviewer/QA вернул worker на доработку — worker сам чинит свой PR (fixup), НЕ требует human (в отличие от status/blocked)

  

status/qa

PR merged, awaiting smoke test

  

status/ready

Ready for pickup by worker bot

  

status/review

PR open, awaiting code review

  

status/wip

Work in progress (worker claimed)

  

tech-debt

Технический долг

  

tradein

Trade-In MVP проект

  

ux

User experience / responsive / conversion

  

week ревью 1

Week review 1 — находки multi-agent аудита проекта (2026-06-14)

  

wontfix

This will not be worked on

  

ИРД

Исходно-разрешительная документация — слой ограничений участка для analyze-таблицы

  

вторичка

Trade-In Estimator — проект оценки вторичного жилья

No labels

Fable 5 ревью

GG-форсайт

admin

analytics

auth

automation

bug

business

chore

ci

compliance

data

data-moat

docs

duplicate

dx

enhancement

feedback/max

generative

needs-discussion

needs-human

observability

pause-bots

performance

priority/p0

priority/p1

priority/p2

priority/p3

scope/backend

scope/db

scope/devops

scope/frontend

scope/qa

scrapers

security

site-finder

stage/1

stage/2

status/blocked

status/done

status/needs-analysis

status/needs-fix

status/qa

status/ready

status/review

status/wip

tech-debt

tradein

ux

week ревью 1

wontfix

ИРД

вторичка

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#2712

No description provided.