fix(rbac): честный докстринг про размен оракулов + трейлинг-слэш больше не выдаёт маршрут
All checks were successful
CI Trade-In / changes (pull_request) Successful in 16s
CI / changes (pull_request) Successful in 19s
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 5m51s

Ревью #3352: комментарий у _path_is_routed утверждал «ослабления нет» — неверно.
Раньше 401 был ПРЕФИКСНЫМ оракулом (таблицу маршрутов по нему не перечислить),
теперь 401/404 — оракул СУЩЕСТВОВАНИЯ маршрута, включая имена admin-ручек.
Докстринг переписан честно, с проверенным по caddy/sites/apps.caddy фактом:
блок handle /trade-in/api/* стоит выше import users.caddy.snippet, внешнего
basic_auth у trade-in нет — значит перебор имён выполним и снаружи.

Второй канал того же оракула закрыт: /api/v1/me/ не матчил ни один маршрут,
guard пропускал, а роутер отвечал 307 на существующий путь. FastAPI получил
redirect_slashes=False (проверено: ни одного route с трейлинг-слэшем, ни одного
такого вызова во фронте; deny-правила уже на глоб-форме).

Тесты: PARTIAL-кейс (POST на GET-путь анониму → 401, не 404/405), трейлинг-слэш
на РЕАЛЬНОМ app.main (тест на копии был бы тавтологией), admin-гейт сверяется по
тексту 'admin only' — scope-ветка отвечает тем же 403, но 'forbidden for role'.
This commit is contained in:
bot-backend 2026-09-05 23:12:17 +05:00
parent 0ef1880e55
commit 8c4be70193
4 changed files with 71 additions and 4 deletions

View file

@ -194,10 +194,40 @@ def _path_is_routed(request: Request) -> bool:
префикса, поэтому такие запросы пропускаются дальше 404 отдаёт роутер, тот
же самый, что и на любой другой мусор.
Ослабления нет: ``Match.NONE`` по ВСЕМ маршрутам значит, что выполнять
нечего хендлера, до которого можно было бы дотянуться, не существует.
``Match.PARTIAL`` (путь есть, метод другой) считается маршрутом и идёт в
guard как раньше там путь реально существует, скрывать нечего.
ЧТО ИМЕННО РАЗМЕНЯНО (это НЕ «ослабления нет»). Раньше аноним получал 401 на
ЛЮБОЙ non-public путь то есть оракул был ПРЕФИКСНЫЙ: он говорил «префикс
закрыт», но перечислить по нему таблицу маршрутов было нельзя. Теперь 401 =
«такой маршрут есть», 404 = «нет», и это уже оракул СУЩЕСТВОВАНИЯ маршрута:
перебором аноним восстанавливает список всех ручек приложения, включая имена
под ``/api/v1/admin/*``. Доступа это не даёт (закрытая ручка по-прежнему
отвечает 401/403), но карту периметра даёт.
Почему размен принят. Точечно: 404 на несуществующее норма HTTP, а
подобранное ИМЯ ручки без креденшелов бесполезно; исчезает же реальный
признак «этот префикс что-то охраняет». Это защита в глубину, и её глубина
здесь честно меньше, чем была.
Периметр admin-путей снаружи НЕ срезан проверено по конфигу, а не по
предположению: ``caddy/sites/apps.caddy`` блок ``handle /trade-in/api/*``
делает ``uri strip_prefix /trade-in`` + ``reverse_proxy tradein-backend:8000``
и стоит ЦЕЛИКОМ ВЫШЕ ``import caddy/users.caddy.snippet`` (basic_auth), т.е.
у trade-in своя авторизация и внешнего barrier'а нет. Значит внешний
``https://gendsgn.ru/trade-in/api/v1/admin/...`` доходит до этого guard'а
анонимно, и перебор имён admin-ручек выполним снаружи, не только изнутри
docker-сети. Хочется убрать резать надо в Caddy (отдельный issue), guard
этого не сделает: он про роли, а не про сетевой периметр.
``Match.NONE`` по ВСЕМ маршрутам значит, что выполнять нечего хендлера, до
которого можно было бы дотянуться, не существует. ``Match.PARTIAL`` (путь
есть, метод другой) считается маршрутом и идёт в guard как раньше там путь
реально существует, скрывать нечего.
Трейлинг-слэш был вторым каналом того же оракула в обход guard'а:
``/api/v1/me/`` не матчит ни один маршрут (``Match.NONE``) guard пропускает
Starlette-роутер отвечал 307 на ``/api/v1/me``, то есть «маршрут есть»
сообщал редирект, а не 401. Закрыто в ``app/main.py``:
``FastAPI(redirect_slashes=False)`` теперь такой путь даёт тот же 404, что
и любой другой мусор.
Неизвестное приложение (``scope["app"]`` не выставлен) ведём себя как
раньше, то есть отдаём запрос в guard.

View file

@ -218,6 +218,16 @@ app = FastAPI(
description="Оценка вторичного жилья (выкупная стоимость) — копия trade-in feature из gendesign", # noqa: E501
version="0.1.0",
lifespan=lifespan,
# #3324: трейлинг-слэш обходил rbac_guard как канал разведки периметра.
# `/api/v1/me/` не матчит ни один маршрут → guard пропускает (см.
# rbac._path_is_routed) → роутер отвечал 307 на `/api/v1/me`, т.е. «маршрут
# существует» сообщал редирект вместо 401. Выключение проверено на предмет
# поломок: ни один route не объявлен с трейлинг-слэшем (нет `@router.get("/")`
# и пустых путей), ни один из 173 вызовов `api/v1` во фронте
# (tradein-mvp/frontend/src) не заканчивается слэшем, mount/StaticFiles нет.
# Deny-правила на слэш тоже не зависят от редиректа — они переведены на
# глоб-форму специально ради этого (см. auth_session.DB_ROLE_PATHS).
redirect_slashes=False,
)

View file

@ -1316,7 +1316,11 @@ def test_session_user_can_reach_tradein_but_not_admin(client: TestClient, store:
# Роут зарегистрирован (см. _build_test_app), поэтому 403 приходит именно от
# admin-гейта rbac_guard'а: role=employee != admin. Не 404 — иначе «any
# non-2xx» маскировал бы опечатку в guard'е; и не 200 — иначе гейт не сработал.
# Один статус этого не доказывает: scope-check (deny-список employee) отвечает
# тем же 403, поэтому сверяем ТЕКСТ — 'admin only' пишет только admin-гейт,
# scope-ветка пишет 'forbidden for role' (app/core/rbac.py).
assert denied.status_code == 403
assert denied.json()["detail"] == "admin only", denied.text
# ---------------------------------------------------------------------------

View file

@ -411,10 +411,33 @@ def test_rbac_guard_unrouted_path_answers_the_same_everywhere(client: TestClient
# Защита не ослаблена: РЕАЛЬНЫЙ закрытый маршрут анониму по-прежнему 401.
assert client.get("/api/v1/trade-in/support/unread").status_code == 401
assert client.get("/api/v1/me").status_code == 401
# Match.PARTIAL — путь есть, метод чужой (/api/v1/me зарегистрирован как GET).
# Такой запрос обязан идти в guard (401), а НЕ проваливаться в роутер за 405:
# иначе «метод не тот» стало бы ещё одним способом обойти проверку личности.
assert client.post("/api/v1/me").status_code == 401
# Реальный публичный маршрут по-прежнему работает без идентичности.
assert client.get("/api/v1/trade-in/support/anon/unread").status_code == 200
def test_real_app_does_not_redirect_trailing_slash() -> None:
"""#3324: `/api/v1/me/` анониму → 404, а не 307 на существующий путь.
Трейлинг-слэш был вторым каналом оракула существования: путь со слэшем не
матчит ни один маршрут (Match.NONE) guard пропускает Starlette отвечал
307 на `/api/v1/me`, т.е. сообщал «маршрут есть» в обход 401.
Проверяется РЕАЛЬНОЕ приложение (app.main), а не локальный _build_test_app:
флаг `redirect_slashes=False` живёт именно там, и тест на своей копии app
был бы тавтологией он подтверждал бы настройку фикстуры, а не прода.
Импорт локальный: модуль этого файла намеренно не тянет app.main (см. шапку).
"""
from app.main import app as real_app
# TestClient без `with` не запускает lifespan — в БД никто не ходит.
resp = TestClient(real_app).get("/api/v1/me/", follow_redirects=False)
assert resp.status_code == 404, f"{resp.status_code} {resp.headers.get('location')}"
# ---------------------------------------------------------------------------
# 2026-07-31: внутренние разделы («Доля в продаже» / «Кэш») закрыты от клиентов
# ---------------------------------------------------------------------------