test(tradein/auth): правило про синхронную сверку — сторожем, а не комментарием (#2715) (#2735)
All checks were successful
Deploy Trade-In / changes (push) Successful in 9s
Deploy Trade-In / build-frontend (push) Has been skipped
Deploy Trade-In / build-browser (push) Has been skipped
Deploy Trade-In / test (push) Successful in 3m0s
Deploy Trade-In / build-backend (push) Successful in 32s
Deploy Trade-In / deploy (push) Successful in 1m24s
All checks were successful
Deploy Trade-In / changes (push) Successful in 9s
Deploy Trade-In / build-frontend (push) Has been skipped
Deploy Trade-In / build-browser (push) Has been skipped
Deploy Trade-In / test (push) Successful in 3m0s
Deploy Trade-In / build-backend (push) Successful in 32s
Deploy Trade-In / deploy (push) Successful in 1m24s
This commit is contained in:
parent
90e328df66
commit
f0968c8513
1 changed files with 110 additions and 0 deletions
110
tradein-mvp/backend/tests/test_password_call_sites.py
Normal file
110
tradein-mvp/backend/tests/test_password_call_sites.py
Normal file
|
|
@ -0,0 +1,110 @@
|
||||||
|
"""Правило «из `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('` не поймал бы вовсе — то есть ровно ту, ради
|
||||||
|
которой сторож и написан.
|
||||||
|
|
||||||
|
ЧЕГО СТОРОЖ НЕ ВИДИТ, и это записано тут, а не подразумевается: строкового
|
||||||
|
доступа (`getattr(mod, "verify_password")`) и обхода модуля целиком (прямой
|
||||||
|
`bcrypt.checkpw`). От НАМЕРЕННОГО обхода он не защищает и не может — только от
|
||||||
|
нечаянного, а нечаянный и есть частый случай. Обе непойманные формы закреплены
|
||||||
|
исполняемо (`test_detector_blind_spots_are_known`), чтобы «не ловим» было
|
||||||
|
проверенным фактом, а не обещанием в тексте.
|
||||||
|
|
||||||
|
Без БД и без сети — только чтение файлов.
|
||||||
|
"""
|
||||||
|
|
||||||
|
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_detector_blind_spots_are_known() -> None:
|
||||||
|
"""Слепые зоны — зафиксированы, а не забыты.
|
||||||
|
|
||||||
|
Обе формы обходят сторож НАМЕРЕННЫМ усилием: строковый доступ к атрибуту и
|
||||||
|
обход модуля целиком. Ловить их AST'ом можно было бы только ценой ложняков
|
||||||
|
(любой `getattr` с любой строкой, любой вызов bcrypt), а цена ложняка —
|
||||||
|
требование выключить сторож. Тест держит это знание исполняемым: захочет
|
||||||
|
однажды детектор их ловить — покраснеет здесь и заставит осознанно
|
||||||
|
переписать и этот тест, и текст модуля.
|
||||||
|
"""
|
||||||
|
assert not _references_verify_password('fn = getattr(password_mod, "verify_password")')
|
||||||
|
assert not _references_verify_password("bcrypt.checkpw(plain.encode(), hashed.encode())")
|
||||||
|
|
||||||
|
|
||||||
|
def test_sync_verify_password_is_called_from_one_place_only() -> None:
|
||||||
|
"""В `app/` синхронную сверку не поминает никто, кроме её собственного модуля."""
|
||||||
|
# Область сканирования жива. `rglob` по несуществующему каталогу не падает —
|
||||||
|
# отдаёт пусто, нарушителей ноль, сторож зелен НАВСЕГДА. Достаточно
|
||||||
|
# переложить этот файл в подкаталог tests/ (их уже восемь, и прецедент
|
||||||
|
# такого сторожа лежит именно в подкаталоге), чтобы `parents[1]` уехал.
|
||||||
|
assert _OWNER.exists(), f"область сканирования съехала: {_APP_DIR}"
|
||||||
|
|
||||||
|
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