test(tradein/auth): правило про синхронную сверку — сторожем, а не комментарием (#2715)
All checks were successful
CI / changes (pull_request) Successful in 9s
CI Trade-In / changes (pull_request) Successful in 11s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 3m15s
All checks were successful
CI / changes (pull_request) Successful in 9s
CI Trade-In / changes (pull_request) Successful in 11s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 3m15s
Пункт 3 хвоста #2715. Правило «из async def зови ТОЛЬКО verify_password_bounded» жило в docstring app/core/password.py, а синхронная verify_password оставалась публичной и импортируемой. Одной строчки asyncio.to_thread(verify_password, …) в будущем коде хватит, чтобы получить вынос в поток вообще без учёта слотов: внешне быстро, а потолок перебора (#2665) и доля на ключ (#2714) тихо исчезают. Сторож по AST, а не по grep: verify_password поминается в комментариях и docstring'ах (auth.py, config.py) — текстовый поиск краснел бы на них, а форму без скобок (to_thread(verify_password, …)) — ровно ту, ради которой всё и пишется — не поймал бы вовсе. Сам детектор проверен на себе (test_detector_actually_detects): пустой детектор прошёл бы все файлы и выглядел бы рабочим сторожем. Проверено мутацией: импорт verify_password в app/api/v1/team.py красит тест. Прецедент такого сторожа: backend/tests/sql/test_auth_sql_migrations.py. Refs #2715
This commit is contained in:
parent
2a1577738a
commit
1dc3a08563
1 changed files with 83 additions and 0 deletions
83
tradein-mvp/backend/tests/test_password_call_sites.py
Normal file
83
tradein-mvp/backend/tests/test_password_call_sites.py
Normal file
|
|
@ -0,0 +1,83 @@
|
|||
"""Правило «из `async def` зови ТОЛЬКО ограниченную сверку» — проверяемое (#2715).
|
||||
|
||||
Правило живёт в docstring `app/core/password.py`: синхронный `verify_password`
|
||||
блокирует поток на ~282 мс (bcrypt cost 12), поэтому из кода приложения его
|
||||
зовёт РОВНО ОДНА функция — `verify_password_bounded`, и она же единственная,
|
||||
кто считает слоты (потолок темпа #2665 + доля на ключ #2714).
|
||||
|
||||
Комментарий это правило не удерживает. Синхронная функция остаётся публичной и
|
||||
импортируемой, и достаточно одной строчки `asyncio.to_thread(verify_password,
|
||||
…)` в будущем коде, чтобы получить вынос в поток ВООБЩЕ БЕЗ учёта слотов:
|
||||
внешне всё работает, вход отвечает быстро, а потолок перебора тихо исчезает.
|
||||
Ревью такое ловит ровно до тех пор, пока помнит, что правило есть.
|
||||
|
||||
Прецедент такого сторожа в репозитории: backend/tests/sql/test_auth_sql_migrations.py.
|
||||
|
||||
ПОЧЕМУ AST, А НЕ GREP. `verify_password` упоминается в комментариях и docstring'ах
|
||||
(app/api/v1/auth.py, app/core/config.py) — текстовый поиск краснел бы на них, и
|
||||
сторож пришлось бы ослаблять исключениями до бессмысленности. AST видит только
|
||||
ССЫЛКИ НА СИМВОЛ и ловит форму без скобок (`to_thread(verify_password, …)`),
|
||||
которую `grep 'verify_password('` не поймал бы вовсе — то есть ровно ту, ради
|
||||
которой сторож и написан.
|
||||
|
||||
Без БД и без сети — только чтение файлов.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
_BACKEND_ROOT = Path(__file__).resolve().parents[1]
|
||||
_APP_DIR = _BACKEND_ROOT / "app"
|
||||
# Единственное место, которому синхронная сверка разрешена: там она и определена,
|
||||
# и оттуда её забирает пул внутри `verify_password_bounded`.
|
||||
_OWNER = _APP_DIR / "core" / "password.py"
|
||||
|
||||
|
||||
def _references_verify_password(source: str) -> bool:
|
||||
"""Ссылается ли модуль на символ `verify_password` (в любой форме)."""
|
||||
for node in ast.walk(ast.parse(source)):
|
||||
if isinstance(node, ast.Name) and node.id == "verify_password":
|
||||
return True
|
||||
if isinstance(node, ast.Attribute) and node.attr == "verify_password":
|
||||
return True
|
||||
if isinstance(node, ast.ImportFrom) and any(
|
||||
alias.name == "verify_password" for alias in node.names
|
||||
):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def test_detector_actually_detects() -> None:
|
||||
"""Сторож обязан уметь краснеть — иначе он зелен вхолостую.
|
||||
|
||||
Проверка на самого себя: пустой детектор (`return False`) прошёл бы все
|
||||
файлы приложения и выглядел бы работающим сторожем ровно до первого
|
||||
настоящего нарушения.
|
||||
"""
|
||||
# Формы, которые обязан ловить.
|
||||
assert _references_verify_password("from app.core.password import verify_password")
|
||||
assert _references_verify_password("asyncio.to_thread(verify_password, plain, hashed)")
|
||||
assert _references_verify_password("password.verify_password(plain, hashed)")
|
||||
assert _references_verify_password("ok = verify_password(plain, hashed)")
|
||||
|
||||
# Формы, на которые краснеть НЕЛЬЗЯ (иначе сторож потребуют выключить).
|
||||
assert not _references_verify_password("await verify_password_bounded(p, h, key=ip)")
|
||||
assert not _references_verify_password('"""Зови verify_password только из пула."""')
|
||||
assert not _references_verify_password("# verify_password тут только в комментарии")
|
||||
|
||||
|
||||
def test_sync_verify_password_is_called_from_one_place_only() -> None:
|
||||
"""В `app/` синхронную сверку не поминает никто, кроме её собственного модуля."""
|
||||
offenders = [
|
||||
str(path.relative_to(_BACKEND_ROOT))
|
||||
for path in sorted(_APP_DIR.rglob("*.py"))
|
||||
if path != _OWNER and _references_verify_password(path.read_text(encoding="utf-8"))
|
||||
]
|
||||
|
||||
assert offenders == [], (
|
||||
f"{offenders}: синхронный verify_password блокирует поток на ~282 мс и НЕ считает "
|
||||
"слоты. Из кода приложения зови verify_password_bounded (app/core/password.py) — "
|
||||
"она единственная точка выноса в пул и единственная точка учёта потолка"
|
||||
)
|
||||
Loading…
Add table
Reference in a new issue