fix(mera/public): Infinity/NaN во входе — 422, и бюджет считает такие запросы
Аудит живого сайта 30.08.2026: POST /api/public/mera/coverage с
{"lat":56.8,"lon":1e400,...} отвечал 500, и двенадцать таких запросов подряд
дали двенадцать пятисоток и ни одного 429. Две независимые поломки в одном
месте, обе воспроизведены локально до правки.
1. 500 вместо 422. json.loads принимает Infinity/-Infinity/NaN, а 1e400 даёт
inf переполнением. Pydantic отбивает такое поле по границам и кладёт
значение в input ошибки, а ответ об ошибке сериализуется
json.dumps(allow_nan=False) и падает уже после входа в ответ. Ломается не
поле, а сборка ответа об ошибке — одна на всё приложение, поэтому и
обработчик один (app/core/http_errors.py), а не валидатор на lon.
2. Лимитер мимо. _enforce стоял первой строкой тела хендлера, а FastAPI
валидирует тело позже зависимостей, но раньше тела — до проверки просто не
доходило. Та же поправка места, что уже сделана сегодня у
_require_public_estimate_enabled: перенос в dependencies. Сделано для всех
ручек файла, не только coverage. У /estimate и /estimate/read флаг остаётся
первой зависимостью — 429 на выключенной ручке подтверждал бы её
существование.
Тесты двусторонние: снятие обработчика роняет 4 проверки 422, возврат лимитера
в тело роняет проверку бюджета (проверено).
This commit is contained in:
parent
ba35c68eb2
commit
df9dd52996
4 changed files with 190 additions and 21 deletions
|
|
@ -70,6 +70,7 @@ import asyncio
|
|||
import hashlib
|
||||
import logging
|
||||
import secrets
|
||||
from collections.abc import Callable
|
||||
from datetime import datetime
|
||||
from typing import Annotated, Literal
|
||||
|
||||
|
|
@ -185,6 +186,33 @@ def _enforce(limiter: SlidingWindowLimiter, request: Request, what: str) -> None
|
|||
limiter.record(ip)
|
||||
|
||||
|
||||
def _budget(limiter: SlidingWindowLimiter, what: str) -> Callable[[Request], None]:
|
||||
"""Per-IP бюджет КАК ЗАВИСИМОСТЬ, а не первой строкой тела.
|
||||
|
||||
Ровно та же поправка места, что уже сделана у
|
||||
`_require_public_estimate_enabled` (см. его докстринг): FastAPI решает
|
||||
зависимости РАНЬШЕ, чем валидирует тело, поэтому проверка в теле не
|
||||
срабатывает на запросах, которые падают на разборе тела — до неё просто не
|
||||
доходит.
|
||||
|
||||
Для флага это стоило утечки схемы, для лимитера — неограниченного потока
|
||||
отказов: 12 запросов подряд с некорректным телом на `/coverage` дали
|
||||
двенадцать ответов и ни одного 429 (замер 30.08.2026). Пока такой вход
|
||||
ронял сериализацию 422 (чинится в app/core/http_errors.py), это был поток
|
||||
500 с одного адреса; после починки — поток 422, но всё так же мимо бюджета.
|
||||
|
||||
ЧЕГО ЭТА ЗАВИСИМОСТЬ НЕ ЛОВИТ: тело, которое не разбирается как JSON
|
||||
вообще — там `RequestValidationError` летит до решения зависимостей. Такой
|
||||
запрос стоит один `json.loads` и остаётся под общим `RateLimitMiddleware`
|
||||
(300/60с на IP); заводить ради него middleware поверх middleware смысла нет.
|
||||
"""
|
||||
|
||||
def _dep(request: Request) -> None:
|
||||
_enforce(limiter, request, what)
|
||||
|
||||
return _dep
|
||||
|
||||
|
||||
class PublicSuggestInput(BaseModel):
|
||||
"""Вход публичного автокомплита.
|
||||
|
||||
|
|
@ -231,9 +259,12 @@ def _query_with_city(query: str, city_hint: str | None) -> str:
|
|||
return f"{city_hint}, {query}"
|
||||
|
||||
|
||||
@router.post("/suggest", response_model=SuggestResponse)
|
||||
@router.post(
|
||||
"/suggest",
|
||||
response_model=SuggestResponse,
|
||||
dependencies=[Depends(_budget(_suggest_limiter, "suggest"))],
|
||||
)
|
||||
async def public_suggest(
|
||||
request: Request,
|
||||
payload: PublicSuggestInput,
|
||||
db: Annotated[Session, Depends(get_db)],
|
||||
) -> SuggestResponse:
|
||||
|
|
@ -260,8 +291,6 @@ async def public_suggest(
|
|||
десятка в публичном UI не показывается, а каждый лишний кандидат может
|
||||
стоить внешнего вызова.
|
||||
"""
|
||||
_enforce(_suggest_limiter, request, "suggest")
|
||||
|
||||
# Суточный потолок — ПОСЛЕ per-IP: сначала отсекаем одиночного абузера его
|
||||
# собственным лимитом, и только оставшееся считаем в общий бюджет.
|
||||
daily_retry = _daily_suggest_limiter.retry_after(_GLOBAL_KEY)
|
||||
|
|
@ -302,9 +331,12 @@ async def public_suggest(
|
|||
_suggest_slots.release()
|
||||
|
||||
|
||||
@router.post("/coverage", response_model=CoverageProbeResponse)
|
||||
@router.post(
|
||||
"/coverage",
|
||||
response_model=CoverageProbeResponse,
|
||||
dependencies=[Depends(_budget(_coverage_limiter, "coverage"))],
|
||||
)
|
||||
def public_coverage(
|
||||
request: Request,
|
||||
payload: CoverageProbeInput,
|
||||
db: Annotated[Session, Depends(get_db)],
|
||||
) -> CoverageProbeResponse:
|
||||
|
|
@ -318,7 +350,6 @@ def public_coverage(
|
|||
Ответ не содержит ни одной цены (см. `CoverageProbeResponse`) — бесплатный
|
||||
шаг доказывает наличие данных, цену продаёт платный.
|
||||
"""
|
||||
_enforce(_coverage_limiter, request, "coverage")
|
||||
return coverage_probe(payload=payload, db=db)
|
||||
|
||||
|
||||
|
|
@ -351,9 +382,12 @@ _STATS_SQL = text("""
|
|||
""")
|
||||
|
||||
|
||||
@router.get("/stats", response_model=dict[str, LandingStat])
|
||||
@router.get(
|
||||
"/stats",
|
||||
response_model=dict[str, LandingStat],
|
||||
dependencies=[Depends(_budget(_stats_limiter, "stats"))],
|
||||
)
|
||||
def public_stats(
|
||||
request: Request,
|
||||
db: Annotated[Session, Depends(get_db)],
|
||||
) -> dict[str, LandingStat]:
|
||||
"""Витринные метрики лэндинга — готовый ночной срез (issue: числа по проду).
|
||||
|
|
@ -374,8 +408,6 @@ def public_stats(
|
|||
`value` — числовое value_num, если оно есть; иначе value_text (для метрик,
|
||||
у которых значение не число). Оба NULL — отдаём null, а не выдуманный ноль.
|
||||
"""
|
||||
_enforce(_stats_limiter, request, "stats")
|
||||
|
||||
rows = db.execute(_STATS_SQL).fetchall()
|
||||
return {
|
||||
row.metric: LandingStat(
|
||||
|
|
@ -499,9 +531,12 @@ _SHOWCASE_SQL = text(
|
|||
)
|
||||
|
||||
|
||||
@router.get("/showcase", response_model=ShowcaseResponse)
|
||||
@router.get(
|
||||
"/showcase",
|
||||
response_model=ShowcaseResponse,
|
||||
dependencies=[Depends(_budget(_showcase_limiter, "showcase"))],
|
||||
)
|
||||
def public_showcase(
|
||||
request: Request,
|
||||
db: Annotated[Session, Depends(get_db)],
|
||||
) -> ShowcaseResponse:
|
||||
"""Витрина лэндинга: реальные ДКП-сделки против прогноза МЕРЫ.
|
||||
|
|
@ -518,7 +553,6 @@ def public_showcase(
|
|||
годных строк не поместилось и по какому правилу отсеяно остальное. Числа
|
||||
считает пересчёт; без них витрина не имеет права подписаться честно.
|
||||
"""
|
||||
_enforce(_showcase_limiter, request, "showcase")
|
||||
run = db.execute(_SHOWCASE_RUN_SQL).mappings().first()
|
||||
if run is None:
|
||||
return ShowcaseResponse(computed_at=None, deals=[], stats=None)
|
||||
|
|
@ -693,7 +727,13 @@ def _coverage_for(
|
|||
@router.post(
|
||||
"/estimate",
|
||||
response_model=PublicEstimateResult,
|
||||
dependencies=[Depends(_require_public_estimate_enabled)],
|
||||
# Порядок несущий: флаг ПЕРВЫМ. Лимитер впереди него отвечал бы 429 на
|
||||
# выключенной ручке, а несуществующий путь даёт 401 — то есть 429 снова
|
||||
# подтверждал бы существование ручки, ровно то, что чинил флаг-гейт.
|
||||
dependencies=[
|
||||
Depends(_require_public_estimate_enabled),
|
||||
Depends(_budget(_estimate_limiter, "estimate")),
|
||||
],
|
||||
)
|
||||
async def public_estimate(
|
||||
request: Request,
|
||||
|
|
@ -714,8 +754,6 @@ async def public_estimate(
|
|||
контур (сосед, `/api/v1/trade-in/r/{token}`) открывает его после оплаты по
|
||||
своему токену. Наружу здесь уезжает только `PublicEstimateResult`.
|
||||
"""
|
||||
_enforce(_estimate_limiter, request, "estimate")
|
||||
|
||||
daily_retry = _daily_estimate_limiter.retry_after(_ESTIMATE_GLOBAL_KEY)
|
||||
if daily_retry is not None:
|
||||
logger.error(
|
||||
|
|
@ -773,10 +811,13 @@ async def public_estimate(
|
|||
@router.post(
|
||||
"/estimate/read",
|
||||
response_model=PublicEstimateResult,
|
||||
dependencies=[Depends(_require_public_estimate_enabled)],
|
||||
# Флаг первым — по той же причине, что у `/estimate`.
|
||||
dependencies=[
|
||||
Depends(_require_public_estimate_enabled),
|
||||
Depends(_budget(_estimate_read_limiter, "estimate-read")),
|
||||
],
|
||||
)
|
||||
def public_estimate_read(
|
||||
request: Request,
|
||||
payload: PublicEstimateTokenInput,
|
||||
db: Annotated[Session, Depends(get_db)],
|
||||
) -> PublicEstimateResult:
|
||||
|
|
@ -791,8 +832,6 @@ def public_estimate_read(
|
|||
Просрочка и «нет такого токена» отвечают ОДИНАКОВО (404): различать их
|
||||
значит подтверждать существование расчёта тому, кто угадал токен.
|
||||
"""
|
||||
_enforce(_estimate_read_limiter, request, "estimate-read")
|
||||
|
||||
row = db.execute(
|
||||
text(
|
||||
"""
|
||||
|
|
|
|||
59
tradein-mvp/backend/app/core/http_errors.py
Normal file
59
tradein-mvp/backend/app/core/http_errors.py
Normal file
|
|
@ -0,0 +1,59 @@
|
|||
"""Ответ об ошибке валидации, который собирается при ЛЮБОМ входе.
|
||||
|
||||
ЧТО СЛОМАЛОСЬ
|
||||
-------------
|
||||
Аудит живого сайта 30.08.2026: публичная проба покрытия отвечала 500 на входе,
|
||||
который обязан отсеиваться валидацией::
|
||||
|
||||
POST /trade-in/api/public/mera/coverage
|
||||
{"lat":56.8,"lon":1e400,"rooms":2,"area_m2":50} → 500
|
||||
|
||||
Разбор. `json.loads` принимает то, чего нет в стандарте JSON: литералы
|
||||
`Infinity`, `-Infinity`, `NaN`, а `1e400` даёт `inf` переполнением. Pydantic
|
||||
такое поле честно отбивает по границам (`lon: le=180`) и кладёт значение в
|
||||
`input` ошибки. Дальше штатный обработчик FastAPI отдаёт перечень ошибок через
|
||||
`JSONResponse`, а тот сериализует `json.dumps(..., allow_nan=False)` — и падает
|
||||
уже ПОСЛЕ входа в ответ. Наружу это 500, то есть отказ сервера там, где
|
||||
корректный ответ — 422.
|
||||
|
||||
ПОЧЕМУ ОДИН ОБРАБОТЧИК, А НЕ ВАЛИДАТОР НА ПОЛЕ
|
||||
----------------------------------------------
|
||||
Чинить по одному полю значит починить `lon` и оставить `lat`, `area_m2`,
|
||||
`limit` и каждое число каждой будущей схемы. Ломается не поле: ломается
|
||||
сборка ОТВЕТА об ошибке, одна на всё приложение. Здесь она и чинится.
|
||||
|
||||
Отдельным модулем (а не строкой в `app/main.py`) ровно затем, чтобы тест мог
|
||||
поставить ТОТ ЖЕ обработчик на своё маленькое приложение, не втягивая весь
|
||||
граф роутеров: иначе тестовое приложение отвечало бы иначе, чем прод, и
|
||||
проверка «422, а не 500» была бы зелёной по построению.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import math
|
||||
|
||||
from fastapi import FastAPI, Request
|
||||
from fastapi.encoders import jsonable_encoder
|
||||
from fastapi.exceptions import RequestValidationError
|
||||
from fastapi.responses import JSONResponse
|
||||
|
||||
|
||||
def _json_safe_float(value: float) -> float | str:
|
||||
"""inf/nan → строка. JSON их не умеет, а на ВХОД они приходят законно."""
|
||||
return value if math.isfinite(value) else str(value)
|
||||
|
||||
|
||||
async def validation_error_handler(request: Request, exc: RequestValidationError) -> JSONResponse:
|
||||
"""Тот же стандартный `{"detail": [...]}`, но нефинитное число в `input`
|
||||
едет строкой ("inf"/"nan") вместо того, чтобы ронять ответ."""
|
||||
return JSONResponse(
|
||||
status_code=422,
|
||||
content=jsonable_encoder(
|
||||
{"detail": exc.errors()},
|
||||
custom_encoder={float: _json_safe_float},
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
def install_validation_error_handler(app: FastAPI) -> None:
|
||||
app.add_exception_handler(RequestValidationError, validation_error_handler)
|
||||
|
|
@ -43,6 +43,7 @@ from app.core.auth_db import get_auth_engine
|
|||
from app.core.config import settings
|
||||
from app.core.db import SessionLocal
|
||||
from app.core.fdw import ensure_fdw_user_mapping
|
||||
from app.core.http_errors import install_validation_error_handler
|
||||
from app.core.ratelimit import RateLimitMiddleware
|
||||
from app.core.rbac import rbac_guard
|
||||
from app.core.request_audit import RequestAuditMiddleware
|
||||
|
|
@ -219,6 +220,10 @@ app = FastAPI(
|
|||
lifespan=lifespan,
|
||||
)
|
||||
|
||||
|
||||
# 422 вместо 500 на Infinity/NaN во входе — разбор в app/core/http_errors.py.
|
||||
install_validation_error_handler(app)
|
||||
|
||||
# RBAC: defense-in-depth поверх Caddy basic_auth + X-Authenticated-User
|
||||
# (см. app/core/auth.py + auth/roles.yaml). Правила:
|
||||
# 1) Любой non-public path требует X-Authenticated-User — иначе 401.
|
||||
|
|
|
|||
|
|
@ -47,6 +47,7 @@ from fastapi.testclient import TestClient # noqa: E402
|
|||
from app.api.public import mera as public_mera # noqa: E402
|
||||
from app.api.v1.geocode import SuggestResponse # noqa: E402
|
||||
from app.core.db import get_db # noqa: E402
|
||||
from app.core.http_errors import install_validation_error_handler # noqa: E402
|
||||
from app.core.rbac import _PUBLIC_PATHS, rbac_guard # noqa: E402
|
||||
from app.schemas.trade_in import CoverageProbeResponse # noqa: E402
|
||||
|
||||
|
|
@ -94,6 +95,10 @@ def client() -> TestClient:
|
|||
"""
|
||||
app = FastAPI()
|
||||
app.middleware("http")(rbac_guard)
|
||||
# Тот же обработчик 422, что вешает app/main.py. Без него тестовое
|
||||
# приложение отвечало бы на нефинитные числа иначе, чем прод, и проверка
|
||||
# «422, а не 500» была бы зелёной по построению.
|
||||
install_validation_error_handler(app)
|
||||
app.include_router(public_mera.router, prefix=PREFIX)
|
||||
|
||||
@app.get("/api/v1/trade-in/coverage")
|
||||
|
|
@ -635,3 +640,64 @@ def test_suggest_passes_city_prefixed_query_downstream(client: TestClient) -> No
|
|||
assert captured["q"] == "Серов, Ленина 1"
|
||||
# Сам хинт продолжаем передавать: от него зависит гейт кадастрового тира.
|
||||
assert captured["city_hint"] == "Серов"
|
||||
|
||||
|
||||
# ── 9. Невалидный вход: 422 и всё тот же бюджет ──────────────────────────────
|
||||
#
|
||||
# Аудит живого сайта 30.08.2026: POST /coverage с `"lon":1e400` отвечал 500, и
|
||||
# двенадцать таких запросов подряд дали двенадцать пятисоток и ни одного 429.
|
||||
# Две разные поломки в одном месте, поэтому и проверок здесь две.
|
||||
|
||||
_JSON = {"content-type": "application/json"}
|
||||
|
||||
# `json.loads` принимает нестандартные литералы Infinity/NaN, а `1e400` — это
|
||||
# переполнение float. Ни одно из этих чисел не сериализуется обратно в JSON,
|
||||
# поэтому они и роняли ответ об ошибке. Поля берём разные намеренно: чинить
|
||||
# должно не поле, а сериализацию перечня ошибок.
|
||||
_NON_FINITE_BODIES = [
|
||||
b'{"lat":56.838,"lon":1e400,"rooms":2,"area_m2":54.0}',
|
||||
b'{"lat":56.838,"lon":NaN,"rooms":2,"area_m2":54.0}',
|
||||
b'{"lat":Infinity,"lon":60.597,"rooms":2,"area_m2":54.0}',
|
||||
b'{"lat":56.838,"lon":60.597,"rooms":2,"area_m2":-Infinity}',
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("body", _NON_FINITE_BODIES)
|
||||
def test_non_finite_number_is_422_not_500(client: TestClient, body: bytes) -> None:
|
||||
"""Infinity/NaN во входе — это невалидный вход, а не отказ сервера.
|
||||
|
||||
Красный вид этого теста без починки — не «assert 500 != 422», а
|
||||
необработанный ValueError из `json.dumps(..., allow_nan=False)`: он летит
|
||||
сквозь TestClient. Оба исхода одинаково красные и оба про одно: ответ об
|
||||
ошибке не собрался.
|
||||
"""
|
||||
resp = client.post(f"{PREFIX}/coverage", content=body, headers=_JSON)
|
||||
|
||||
assert resp.status_code == 422, resp.text
|
||||
# Форма ответа остаётся стандартной, иначе фронт разбирает её иначе.
|
||||
assert isinstance(resp.json()["detail"], list)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("route", ["/coverage", "/suggest"])
|
||||
def test_invalid_body_still_spends_the_per_ip_budget(client: TestClient, route: str) -> None:
|
||||
"""Бюджет обязан срабатывать РАНЬШЕ разбора тела.
|
||||
|
||||
Иначе он не защищает ровно от того, что на разборе тела и падает: клиент
|
||||
льёт неограниченный поток отказов с одного адреса.
|
||||
|
||||
Двусторонность: верните `_enforce(...)` первой строкой тела хендлера — и
|
||||
все ответы станут 422, ни одного 429, тест покраснеет.
|
||||
"""
|
||||
limit = public_mera._COVERAGE_LIMIT if route == "/coverage" else public_mera._SUGGEST_LIMIT
|
||||
codes = [
|
||||
client.post(
|
||||
f"{PREFIX}{route}", content=b'{"lat":56.838,"lon":1e400}', headers=_JSON
|
||||
).status_code
|
||||
for _ in range(limit + 3)
|
||||
]
|
||||
|
||||
assert 429 in codes, f"бюджет не сработал: {codes}"
|
||||
# Ничего третьего быть не должно — ни 500, ни внезапной 200 на мусоре.
|
||||
assert set(codes) <= {422, 429}, codes
|
||||
# Отказ начинается ровно после исчерпания окна, а не «когда-нибудь».
|
||||
assert codes.index(429) == limit, codes
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue