Merge pull request 'fix(tradein/payments): тело нотификации не течёт в мониторинг и аудит, повторы банка не отбиваются лимитом' (#2794) from feat/tradein-payments-perimeter-hardening into main
All checks were successful
Deploy Trade-In / changes (push) Successful in 19s
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 3m46s
Deploy Trade-In / build-backend (push) Successful in 1m24s
Deploy Trade-In / deploy (push) Successful in 2m4s
Deploy Trade-In / deploy-status (push) Successful in 1s
All checks were successful
Deploy Trade-In / changes (push) Successful in 19s
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 3m46s
Deploy Trade-In / build-backend (push) Successful in 1m24s
Deploy Trade-In / deploy (push) Successful in 2m4s
Deploy Trade-In / deploy-status (push) Successful in 1s
This commit is contained in:
commit
58f04087bb
11 changed files with 597 additions and 23 deletions
|
|
@ -101,6 +101,27 @@ check "gendsgn.ru/api/v1/admin/* — 401 anonymous" "$BASE_MAIN/api/v1/admin/use
|
|||
check "merahome.ru — 301 to canonical" "https://merahome.ru/" 301
|
||||
check "meraotsenka.ru — 301 to canonical" "https://meraotsenka.ru/" 301
|
||||
|
||||
# 6. Платёжный периметр (PR-D2) — готовит почву под PR-D3 (роутер) и PR-D4
|
||||
# (Caddy), но САМ НИЧЕГО НЕ ОТКРЫВАЕТ. Ожидаем закрытое состояние С ОБЕИХ
|
||||
# СТОРОН прямо сейчас:
|
||||
# - meraocenka.ru вообще не проксирует /trade-in/api/* (allowlist-by-default,
|
||||
# см. проверку 2) — 404 от Caddy, до бэкенда не доходит;
|
||||
# - gendsgn.ru проксирует /trade-in/api/* в tradein-backend, но rbac_guard
|
||||
# (`_PUBLIC_PATHS` в app/core/rbac.py — ЭТОТ PR её не трогает) не знает
|
||||
# платёжные пути и требует X-Authenticated-User → 401 анониму.
|
||||
# Если один из этих чек-ов вдруг перестанет быть 404/401 РАНЬШЕ мержа
|
||||
# PR-D3/PR-D4 — это и есть преждевременная утечка периметра, которую ловит
|
||||
# этот смоук (канарейка: осознанно станет красной, когда PR-D3/PR-D4 явно
|
||||
# откроют эти пути — тогда ожидания здесь надо обновить вместе с ними).
|
||||
check "meraocenka.ru payments/notify — must 404 (Caddy не проксирует, PR-D4)" \
|
||||
"$BASE_MERA/trade-in/api/v1/trade-in/payments/notify" 404
|
||||
check "meraocenka.ru payments/checkout — must 404 (Caddy не проксирует, PR-D4)" \
|
||||
"$BASE_MERA/trade-in/api/v1/trade-in/payments/checkout" 404
|
||||
check "trade-in payments/notify — 401 anonymous (rbac закрыт до PR-D3)" \
|
||||
"$BASE_MAIN/trade-in/api/v1/trade-in/payments/notify" 401
|
||||
check "trade-in payments/checkout — 401 anonymous (rbac закрыт до PR-D3)" \
|
||||
"$BASE_MAIN/trade-in/api/v1/trade-in/payments/checkout" 401
|
||||
|
||||
echo "========================================"
|
||||
if [ "$fail" -eq 0 ]; then
|
||||
echo "ALL CHECKS PASSED"
|
||||
|
|
|
|||
|
|
@ -32,6 +32,18 @@ from starlette.middleware.base import BaseHTTPMiddleware
|
|||
|
||||
from app.core.config import settings
|
||||
|
||||
# Платёжная нотификация Т-Банка (PR-D2, готовит почву под PR-D3 — путь ещё
|
||||
# закрыт rbac до того момента). Сервер-к-серверу, без сессии/X-Authenticated-User
|
||||
# → в общем лимитере попал бы в один и тот же per-IP ключ с любым другим
|
||||
# анонимным трафиком с той же исходящей сети банка. Мотив НЕ «банк упрётся в
|
||||
# лимит» — 300/60с и так щедро — а «429 никогда не должен стать причиной, по
|
||||
# которой денежное состояние разъехалось»: для банка недоставленная нотификация
|
||||
# = «доставка не удалась», альтернативного канала нет, а очередь ретраев
|
||||
# растягивается на сутки. Только точный путь notify — НЕ checkout (тот
|
||||
# инициирует пользователь с сессией/курсором в браузере, абуз там штатно
|
||||
# лимитируем как любой другой API-путь).
|
||||
_PAYMENTS_NOTIFY_PATH = "/api/v1/trade-in/payments/notify"
|
||||
|
||||
|
||||
class RateLimitMiddleware(BaseHTTPMiddleware):
|
||||
"""Sliding-window rate limit на /api/v1/*. Health и статика — без лимита."""
|
||||
|
|
@ -42,6 +54,23 @@ class RateLimitMiddleware(BaseHTTPMiddleware):
|
|||
|
||||
async def dispatch(self, request: Request, call_next): # type: ignore[no-untyped-def]
|
||||
path = request.url.path
|
||||
# Платёжная нотификация — мимо ОБЩЕГО (per-user/per-IP shared) лимитера,
|
||||
# но НЕ без лимита вовсе: idiom `_notify_limiter` (`SlidingWindowLimiter`,
|
||||
# тот же приём, что `support.py:92`/`:319` — узкий per-feature бюджет
|
||||
# ВМЕСТО общего, не полное отключение защиты). Порог заведомо выше любого
|
||||
# штатного трафика банка (документированное расписание ретраев неизвестно,
|
||||
# см. mera-tbank-acquiring-recon.md — берём с кратным запасом), но конечен:
|
||||
# полное отключение оставило бы путь без backstop против шторма запросов —
|
||||
# подпись отсекает мусор ПОСЛЕ разбора тела (PR-D3), не до.
|
||||
if path == _PAYMENTS_NOTIFY_PATH:
|
||||
retry_after = _notify_limiter.check(_client_ip(request))
|
||||
if retry_after is not None:
|
||||
return JSONResponse(
|
||||
status_code=429,
|
||||
content={"detail": "Слишком много запросов. Попробуйте позже."},
|
||||
headers={"Retry-After": str(int(retry_after) + 1)},
|
||||
)
|
||||
return await call_next(request)
|
||||
# Лимитируем только API; health и прочее — пропускаем.
|
||||
if not path.startswith("/api/"):
|
||||
return await call_next(request)
|
||||
|
|
@ -143,6 +172,16 @@ class SlidingWindowLimiter:
|
|||
return None
|
||||
|
||||
|
||||
# Щедрый бюджет для платёжной нотификации (PR-D2): 3000/60с (50 req/s) — на два
|
||||
# порядка выше любого правдоподобного трафика банка (тест 400/60с проходит с
|
||||
# запасом в 7.5×), но конечен — backstop против шторма запросов на путь, где
|
||||
# подпись проверяется уже ПОСЛЕ разбора тела. Ключ — client IP (у сервер-к-
|
||||
# серверу вызова нет сессии/X-Authenticated-User).
|
||||
_NOTIFY_RATE_LIMIT = 3000
|
||||
_NOTIFY_RATE_WINDOW_S = 60.0
|
||||
_notify_limiter = SlidingWindowLimiter(limit=_NOTIFY_RATE_LIMIT, window_s=_NOTIFY_RATE_WINDOW_S)
|
||||
|
||||
|
||||
def _client_ip(request: Request) -> str:
|
||||
"""Честный клиентский IP при РОВНО ОДНОМ доверенном прокси (Caddy) перед нами.
|
||||
|
||||
|
|
|
|||
|
|
@ -40,7 +40,29 @@ logger = logging.getLogger(__name__)
|
|||
# Зеркалит app.main._PUBLIC_PATHS. Не импортируем напрямую из app.main — оно
|
||||
# импортирует этот модуль (регистрирует middleware), обратный импорт дал бы
|
||||
# циклическую зависимость.
|
||||
_PUBLIC_PATHS = frozenset({"/health", "/docs", "/redoc", "/openapi.json"})
|
||||
#
|
||||
# PR-D2: `/api/v1/trade-in/payments/notify` — заранее в skip-набор (defense-in-
|
||||
# depth), хотя rbac ещё закрывает этот путь до PR-D3. Причины две:
|
||||
# 1) сам путь не должен попадать в аудит вообще — тело нотификации содержит
|
||||
# `Token`/`Pan`/`ExpDate` (см. `app/main.py._before_send`, тот же мотив, что
|
||||
# и вырезание тела из мониторинга); хоть это middleware само по себе тело
|
||||
# запроса в payload не пишет (только status_code/path/method), путь не
|
||||
# должен зависеть от того, что кто-то потом добавит поле "body" в событие;
|
||||
# 2) НЕ авторизующая проверка: `RequestAuditMiddleware` внешний относительно
|
||||
# `rbac_guard` и читает сырой `X-Authenticated-User` (см. `main.py` порядок
|
||||
# middleware) — анонимный POST на notify с подделанным заголовком
|
||||
# `X-Authenticated-User: admin` иначе писал бы фальшивые события в
|
||||
# `user_events` с атрибуцией admin, при этом rbac при этом ничего не знает
|
||||
# (сам гейт отдельно, 401 всё равно вернёт до PR-D3).
|
||||
_PUBLIC_PATHS = frozenset(
|
||||
{
|
||||
"/health",
|
||||
"/docs",
|
||||
"/redoc",
|
||||
"/openapi.json",
|
||||
"/api/v1/trade-in/payments/notify",
|
||||
}
|
||||
)
|
||||
|
||||
# Методы, меняющие состояние — для /api/v1/admin/* именно они должны попадать в
|
||||
# аудит с атрибуцией (кто именно загрузил куки / включил авто-логин / поправил
|
||||
|
|
|
|||
|
|
@ -68,20 +68,32 @@ logging.getLogger("httpx").setLevel(logging.WARNING)
|
|||
if settings.glitchtip_dsn:
|
||||
from app.observability.sentry_scrub import (
|
||||
redact_telegram_bot_token,
|
||||
scrub_payment_request_body,
|
||||
stabilize_retry_error_fingerprint,
|
||||
)
|
||||
|
||||
def _before_send(event: dict[str, object], hint: dict[str, object]) -> dict[str, object] | None:
|
||||
"""Композиция PII-scrub + Telegram bot-токен redaction (#tgsupport-web) +
|
||||
RetryError fingerprint-стабилизация (glitchtip-noise) — см.
|
||||
app/tgbot_main.py._before_send (та же композиция без последнего шага,
|
||||
тот бот geocoder не зовёт). PII/token — тот же риск: теперь этот процесс
|
||||
тоже держит TelegramClient в стек-фреймах при ошибке sendMessage, а
|
||||
include_local_variables=False ниже — первый рубеж защиты. RetryError —
|
||||
этот процесс обслуживает /api/v1/geocode/* (suggest/lookup/reverse),
|
||||
которые ретраят Nominatim через tenacity; см.
|
||||
"""Композиция платёжный body-wipe + PII-scrub + Telegram bot-токен redaction +
|
||||
RetryError fingerprint-стабилизация (#tgsupport-web, PR-D2, glitchtip-noise) —
|
||||
см. app/tgbot_main.py._before_send (идентичная композиция без последнего шага,
|
||||
тот бот geocoder не зовёт). Тот же риск: теперь этот процесс тоже держит
|
||||
TelegramClient в стек-фреймах при ошибке sendMessage, а
|
||||
include_local_variables=False ниже — первый рубеж защиты.
|
||||
|
||||
PR-D2: платёжный body-wipe идёт ПЕРВЫМ шагом, а не заменяет остальные —
|
||||
режет `request.data` целиком только для `/payments/*`, остальные пути
|
||||
(extra/contexts/traceback) по-прежнему проходят ключ-based scrub и
|
||||
token-redaction. Тот же обработчик передан ОБОИМ каналам ниже
|
||||
(before_send и before_send_transaction) — вчерашний баг в Птице закрыл
|
||||
только error-канал, transaction-канал остался вообще без обработчика.
|
||||
|
||||
RetryError-стабилизация — этот процесс обслуживает /api/v1/geocode/*
|
||||
(suggest/lookup/reverse), которые ретраят Nominatim через tenacity; см.
|
||||
sentry_scrub.stabilize_retry_error_fingerprint."""
|
||||
scrubbed = scrub_pii_event(event, hint) # type: ignore[arg-type]
|
||||
scrubbed = scrub_payment_request_body(event, hint) # type: ignore[arg-type]
|
||||
if scrubbed is None:
|
||||
return None
|
||||
scrubbed = scrub_pii_event(scrubbed, hint) # type: ignore[arg-type]
|
||||
if scrubbed is None:
|
||||
return None
|
||||
detokened = redact_telegram_bot_token(scrubbed, hint) # type: ignore[arg-type]
|
||||
|
|
@ -99,6 +111,10 @@ if settings.glitchtip_dsn:
|
|||
# держит base URL с токеном в локальных переменных стек-фрейма — default
|
||||
# sentry_sdk (True) приложил бы их к traceback открытым текстом.
|
||||
before_send=_before_send,
|
||||
# PR-D2: тот же обработчик на transaction-канал — traces_sample_rate=0.0
|
||||
# сегодня не шлёт трейсы вообще, но это belt-and-suspenders на случай,
|
||||
# если трейсинг когда-нибудь включат (см. docstring _before_send выше).
|
||||
before_send_transaction=_before_send,
|
||||
integrations=[
|
||||
StarletteIntegration(),
|
||||
FastApiIntegration(),
|
||||
|
|
|
|||
|
|
@ -29,7 +29,36 @@ from tenacity import RetryError
|
|||
|
||||
_REDACTED = "[REDACTED]"
|
||||
# Ключи consumer-PII (нижний регистр; сверка case-insensitive).
|
||||
_PII_KEYS = frozenset({"client_name", "client_phone", "client_email", "phone", "email", "name"})
|
||||
# PR-D2 (payments perimeter hardening): + платёжные поля Т-Банка (customer_email/
|
||||
# customer_phone из checkout, pan/expdate/cardid/rebillid/token/terminalkey из
|
||||
# notify) — belt-and-suspenders поверх `scrub_payment_request_body` ниже, которая
|
||||
# вырезает `request.data` для /payments/* целиком: этот словарь всё равно нужен
|
||||
# для extra/contexts И на случай, если платёжное поле когда-нибудь попадёт в
|
||||
# error event НЕ через request.data (напр. кто-то положит его в extra вручную).
|
||||
_PII_KEYS = frozenset(
|
||||
{
|
||||
"client_name",
|
||||
"client_phone",
|
||||
"client_email",
|
||||
"phone",
|
||||
"email",
|
||||
"name",
|
||||
"customer_email",
|
||||
"customer_phone",
|
||||
"pan",
|
||||
"expdate",
|
||||
"cardid",
|
||||
"rebillid",
|
||||
"token",
|
||||
"terminalkey",
|
||||
}
|
||||
)
|
||||
|
||||
# Сегмент пути платёжного периметра (notify + checkout + любой будущий
|
||||
# /payments/* суб-путь) — PR-D2, готовит почву под PR-D3 (эндпоинты ещё не
|
||||
# существуют). Матчим по сегменту, не по конкретному эндпоинту, чтобы не
|
||||
# требовать правки этого файла на каждый новый платёжный путь.
|
||||
_PAYMENTS_URL_SEGMENT = "/api/v1/trade-in/payments/"
|
||||
|
||||
# Telegram Bot API токен в пути URL: /bot<numeric_id>:<secret-part>/<method>.
|
||||
# Матчим ровно этот сегмент (не весь URL) — сохраняет остальной путь/query
|
||||
|
|
@ -179,6 +208,39 @@ def scrub_pii_event(event: Event, _hint: dict[str, Any]) -> Event | None:
|
|||
return event
|
||||
|
||||
|
||||
def scrub_payment_request_body(event: Event, _hint: dict[str, Any]) -> Event | None:
|
||||
"""Вырезать `event['request']['data']` целиком для платёжных путей (PR-D2).
|
||||
|
||||
Ключ-based `scrub_pii_event` НЕ спасает платёжную нотификацию: sentry_sdk
|
||||
2.64 (`integrations/starlette.py`) кладёт ПОЛНОЕ тело запроса в
|
||||
`event.request.data`, и `send_default_pii=False` этот путь не гейтит — тот
|
||||
флаг управляет только куками, не телом запроса (проверено живьём на соседнем
|
||||
продукте). Тело нотификации Т-Банка несёт `Token`/`Pan`/`ExpDate`/`CardId`/
|
||||
`RebillId`/`DATA` — банк сам выбирает имена полей, перечислить их все заранее
|
||||
нельзя, поэтому единственная безопасная стратегия для этого пути — не
|
||||
отправлять тело целиком, а не пытаться вычистить отдельные ключи.
|
||||
|
||||
Матчим по сегменту `/api/v1/trade-in/payments/` (не по конкретному
|
||||
эндпоинту) — покрывает notify, checkout и любой будущий суб-путь одним
|
||||
фильтром, без правки этого файла на каждое расширение платёжного API.
|
||||
Сравнение регистронезависимое: `_PUBLIC_PATHS` (rbac) — точное множество без
|
||||
учёта регистра только у Caddy, не у Python, так что нестандартный регистр
|
||||
пути технически может долететь до обработчика и породить событие.
|
||||
|
||||
Композировать с `scrub_pii_event`/`redact_telegram_bot_token`, а не вместо
|
||||
них — этот шаг закрывает только `request.data`, extra/contexts и
|
||||
traceback-locals остаются на ответственности остальных шагов композиции.
|
||||
"""
|
||||
if not isinstance(event, dict):
|
||||
return event
|
||||
request = event.get("request")
|
||||
if isinstance(request, dict):
|
||||
url = request.get("url")
|
||||
if isinstance(url, str) and _PAYMENTS_URL_SEGMENT in url.lower():
|
||||
request.pop("data", None)
|
||||
return event
|
||||
|
||||
|
||||
def _redact_strings(obj: Any) -> Any:
|
||||
"""Рекурсивно проходит dict/list/tuple и прогоняет обе токен-регулярки по КАЖДОЙ
|
||||
строке (не только по конкретным ключам) — токен может оказаться в locals
|
||||
|
|
|
|||
|
|
@ -45,20 +45,31 @@ if settings.glitchtip_dsn:
|
|||
from sentry_sdk.integrations.sqlalchemy import SqlalchemyIntegration
|
||||
|
||||
from app.observability.sentry_scrub import (
|
||||
scrub_payment_request_body,
|
||||
scrub_pii_event,
|
||||
stabilize_retry_error_fingerprint,
|
||||
)
|
||||
|
||||
def _before_send(event: dict, hint: dict) -> dict | None: # type: ignore[type-arg]
|
||||
"""PII-scrub + RetryError fingerprint-стабилизация (glitchtip-noise).
|
||||
"""PR-D2: этот процесс не держит ASGI-приложения (нет `request` в event
|
||||
сегодня), но payments_confirm/payments_reconcile (PR-E, тот же
|
||||
`tradein-scraper` контейнер) будут звать Т-Банк API отсюда — belt-and-
|
||||
suspenders на случай, если платёжные данные когда-нибудь попадут в
|
||||
`request`/`extra`. Тот же обработчик на оба канала ниже — см.
|
||||
app/main.py._before_send (идентичный мотив, не дублировать без причины).
|
||||
|
||||
Этот процесс гоняет `geocode_missing_listings` (ночной batch, сотни
|
||||
адресов за прогон) — @retry-декорированные Nominatim-хелперы
|
||||
(app/services/geocoder.py) на исчерпанных ретраях исторически плодили
|
||||
по отдельному GlitchTip issue на КАЖДЫЙ адрес (RetryError.__str__()
|
||||
тащит нестабильный repr() Future). См. sentry_scrub docstring.
|
||||
PII-scrub + RetryError fingerprint-стабилизация (glitchtip-noise) идут
|
||||
следом за платёжным body-wipe: этот процесс гоняет
|
||||
`geocode_missing_listings` (ночной batch, сотни адресов за прогон) —
|
||||
@retry-декорированные Nominatim-хелперы (app/services/geocoder.py) на
|
||||
исчерпанных ретраях исторически плодили по отдельному GlitchTip issue
|
||||
на КАЖДЫЙ адрес (RetryError.__str__() тащит нестабильный repr() Future).
|
||||
См. sentry_scrub docstring.
|
||||
"""
|
||||
scrubbed = scrub_pii_event(event, hint)
|
||||
scrubbed = scrub_payment_request_body(event, hint) # type: ignore[arg-type]
|
||||
if scrubbed is None:
|
||||
return None
|
||||
scrubbed = scrub_pii_event(scrubbed, hint)
|
||||
if scrubbed is None:
|
||||
return None
|
||||
return stabilize_retry_error_fingerprint(scrubbed, hint)
|
||||
|
|
@ -70,6 +81,7 @@ if settings.glitchtip_dsn:
|
|||
traces_sample_rate=0.0,
|
||||
send_default_pii=False,
|
||||
before_send=_before_send,
|
||||
before_send_transaction=_before_send,
|
||||
integrations=[
|
||||
SqlalchemyIntegration(),
|
||||
HttpxIntegration(),
|
||||
|
|
|
|||
|
|
@ -58,12 +58,17 @@ if settings.glitchtip_dsn:
|
|||
from sentry_sdk.integrations.httpx import HttpxIntegration
|
||||
from sentry_sdk.integrations.logging import LoggingIntegration
|
||||
|
||||
from app.observability.sentry_scrub import redact_telegram_bot_token, scrub_pii_event
|
||||
from app.observability.sentry_scrub import (
|
||||
redact_telegram_bot_token,
|
||||
scrub_payment_request_body,
|
||||
scrub_pii_event,
|
||||
)
|
||||
|
||||
def _before_send(event: Any, hint: dict[str, Any]) -> Any:
|
||||
"""Композиция PII-scrub (form-данные) + Telegram bot-токен redaction
|
||||
(#tgsupport review). Токен утекает ДВУМЯ независимыми векторами, которые
|
||||
`include_local_variables=False` ниже и этот хук закрывают вместе:
|
||||
"""Композиция платёжный body-wipe (PR-D2) + PII-scrub (form-данные) +
|
||||
Telegram bot-токен redaction (#tgsupport review). Токен утекает ДВУМЯ
|
||||
независимыми векторами, которые `include_local_variables=False` ниже и
|
||||
этот хук закрывают вместе:
|
||||
1. `include_local_variables=True` (sentry_sdk default) кладёт stack-frame
|
||||
locals (`self._base`/`url` в `TelegramClient._request`) в traceback —
|
||||
закрыто через `include_local_variables=False` в `sentry_sdk.init`.
|
||||
|
|
@ -72,8 +77,16 @@ if settings.glitchtip_dsn:
|
|||
перестанет спасать, если трейсинг когда-нибудь включат. Regex-редактор
|
||||
— belt-and-suspenders на случай #1 (если include_local_variables
|
||||
случайно вернут) И на span data.
|
||||
|
||||
Платёжный body-wipe — belt-and-suspenders: этот процесс не держит ASGI-
|
||||
приложения (нет `request` в event сегодня), но тот же обработчик передан
|
||||
ОБОИМ каналам ниже (before_send/before_send_transaction) ради единообразия
|
||||
со всеми точками инициализации sentry_sdk в проекте (см. app/main.py).
|
||||
"""
|
||||
scrubbed = scrub_pii_event(event, hint)
|
||||
scrubbed = scrub_payment_request_body(event, hint)
|
||||
if scrubbed is None:
|
||||
return None
|
||||
scrubbed = scrub_pii_event(scrubbed, hint)
|
||||
if scrubbed is None:
|
||||
return None
|
||||
return redact_telegram_bot_token(scrubbed, hint)
|
||||
|
|
@ -86,6 +99,7 @@ if settings.glitchtip_dsn:
|
|||
send_default_pii=False,
|
||||
include_local_variables=False,
|
||||
before_send=_before_send,
|
||||
before_send_transaction=_before_send,
|
||||
integrations=[
|
||||
HttpxIntegration(),
|
||||
LoggingIntegration(level=logging.INFO, event_level=logging.ERROR),
|
||||
|
|
|
|||
|
|
@ -149,6 +149,98 @@ def test_sliding_window_limiter_per_key_isolation():
|
|||
assert limiter.retry_after("bob") is None # свой ключ — не задет alice
|
||||
|
||||
|
||||
# ── Платёжная нотификация — мимо ОБЩЕГО лимитера (PR-D2, критерий приёмки #3) ──
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def notify_client(monkeypatch):
|
||||
"""То же минимальное приложение, что `client`, но лимит анонима искусственно
|
||||
крошечный (1/60с) — если бы notify-путь шёл через общий лимитер, 2-й запрос
|
||||
уже получил бы 429. Плюс контрольный `/api/v1/ping` — доказывает, что
|
||||
лимитер в принципе активен (не выключен целиком), просто notify мимо него."""
|
||||
monkeypatch.setattr(config.settings, "rate_limit", 1)
|
||||
monkeypatch.setattr(config.settings, "rate_limit_window_s", 60.0)
|
||||
monkeypatch.setattr(config.settings, "rate_limit_authenticated_multiplier", 2)
|
||||
|
||||
app = FastAPI()
|
||||
app.add_middleware(RateLimitMiddleware)
|
||||
|
||||
@app.get("/api/v1/ping")
|
||||
def ping() -> dict[str, bool]:
|
||||
return {"ok": True}
|
||||
|
||||
@app.post("/api/v1/trade-in/payments/notify")
|
||||
def notify() -> dict[str, bool]:
|
||||
return {"ok": True}
|
||||
|
||||
return TestClient(app)
|
||||
|
||||
|
||||
def test_notify_path_bypasses_general_limiter_400_requests_zero_429(notify_client):
|
||||
"""PR-D2 acceptance criteria: 400 запросов к notify с одного адреса за минуту
|
||||
не дают ни одного отказа по частоте — даже с общим лимитом искусственно
|
||||
зажатым до 1/60с (см. фикстуру)."""
|
||||
statuses = [
|
||||
notify_client.post("/api/v1/trade-in/payments/notify").status_code for _ in range(400)
|
||||
]
|
||||
assert all(
|
||||
code == 200 for code in statuses
|
||||
), f"notify получил 429 хотя бы раз: {[c for c in statuses if c != 200]}"
|
||||
|
||||
|
||||
def test_general_limiter_still_active_for_other_paths(notify_client):
|
||||
"""Контроль: общий лимитер НЕ выключен целиком — обычный /api/v1/ping с тем
|
||||
же крошечным лимитом (1/60с) отбивается на 2-м запросе как обычно. Доказывает,
|
||||
что notify-bypass узкий (точный путь), а не побочный эффект общей поломки."""
|
||||
assert notify_client.get("/api/v1/ping").status_code == 200
|
||||
assert notify_client.get("/api/v1/ping").status_code == 429
|
||||
|
||||
|
||||
def test_notify_bypass_has_own_dedicated_limiter_not_unlimited():
|
||||
"""notify НЕ отключён от лимитера вовсе — своя щедрая, но конечная корзина
|
||||
(`_notify_limiter`, идиома `SlidingWindowLimiter` из `support.py`). Проверяем
|
||||
напрямую: исчерпать маленький искусственный лимит и убедиться, что backstop
|
||||
таки срабатывает (защита от полного disable вместо узкого бюджета)."""
|
||||
from app.core import ratelimit as ratelimit_module
|
||||
|
||||
limiter = ratelimit_module.SlidingWindowLimiter(limit=2, window_s=60.0)
|
||||
assert limiter.check("1.2.3.4") is None
|
||||
assert limiter.check("1.2.3.4") is None
|
||||
# 3-й запрос того же ключа — уже за лимитом (backstop жив).
|
||||
assert limiter.check("1.2.3.4") is not None
|
||||
|
||||
|
||||
def test_notify_limiter_key_is_per_ip_not_global(monkeypatch):
|
||||
"""Бюджет notify — per-IP (не общий на все входящие сразу), тот же принцип
|
||||
ключа, что общий лимитер использует для анонимного трафика."""
|
||||
monkeypatch.setattr(config.settings, "rate_limit", 300)
|
||||
monkeypatch.setattr(config.settings, "rate_limit_window_s", 60.0)
|
||||
|
||||
from app.core import ratelimit as ratelimit_module
|
||||
|
||||
monkeypatch.setattr(
|
||||
ratelimit_module,
|
||||
"_notify_limiter",
|
||||
ratelimit_module.SlidingWindowLimiter(limit=1, window_s=60.0),
|
||||
)
|
||||
|
||||
app = FastAPI()
|
||||
app.add_middleware(RateLimitMiddleware)
|
||||
|
||||
@app.post("/api/v1/trade-in/payments/notify")
|
||||
def notify() -> dict[str, bool]:
|
||||
return {"ok": True}
|
||||
|
||||
client = TestClient(app)
|
||||
# Первый запрос с IP #1 — проходит, второй с тем же IP — 429 (лимит=1).
|
||||
headers_ip1 = {"X-Forwarded-For": "1.1.1.1"}
|
||||
headers_ip2 = {"X-Forwarded-For": "2.2.2.2"}
|
||||
assert client.post("/api/v1/trade-in/payments/notify", headers=headers_ip1).status_code == 200
|
||||
assert client.post("/api/v1/trade-in/payments/notify", headers=headers_ip1).status_code == 429
|
||||
# Другой IP — своя, независимая корзина.
|
||||
assert client.post("/api/v1/trade-in/payments/notify", headers=headers_ip2).status_code == 200
|
||||
|
||||
|
||||
def test_sliding_window_limiter_prunes_empty_buckets_past_threshold():
|
||||
"""review L2: пустые корзины чистятся при накоплении >10000 ключей (тот же
|
||||
паттерн, что `RateLimitMiddleware.dispatch`) — не бесконечная утечка памяти.
|
||||
|
|
|
|||
|
|
@ -262,6 +262,37 @@ def test_login_event_type_when_request_succeeds(client: TestClient) -> None:
|
|||
assert login_calls[0].kwargs["payload"] == {"status_code": 200}
|
||||
|
||||
|
||||
# ── Платёжная нотификация — вне аудита (PR-D2, критерий приёмки #4) ─────────
|
||||
|
||||
|
||||
def test_payments_notify_path_skips_audit_even_with_spoofed_admin_header() -> None:
|
||||
"""PR-D2 acceptance criteria: запрос к нотификации не создаёт записей в
|
||||
журнале аудита — даже с заголовком `X-Authenticated-User: admin`. Middleware
|
||||
— ВНЕШНИЙ относительно rbac_guard и читает сырой заголовок напрямую (см.
|
||||
docstring `request_audit.py`), так что анонимный POST со спуфнутым
|
||||
заголовком иначе писал бы фальшивое `login`/`api_request` событие с
|
||||
атрибуцией admin, хотя rbac этот путь пока (до PR-D3) закрывает 401'ом
|
||||
отдельно и независимо от этого middleware."""
|
||||
app = FastAPI()
|
||||
app.add_middleware(RequestAuditMiddleware)
|
||||
|
||||
@app.post("/api/v1/trade-in/payments/notify")
|
||||
def notify() -> dict[str, bool]:
|
||||
return {"ok": True}
|
||||
|
||||
with (
|
||||
patch("app.core.request_audit.schedule_event") as mock_schedule,
|
||||
patch("app.core.request_audit.should_log_login", return_value=True),
|
||||
):
|
||||
resp = TestClient(app).post(
|
||||
"/api/v1/trade-in/payments/notify",
|
||||
headers={"X-Authenticated-User": "admin"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
mock_schedule.assert_not_called()
|
||||
|
||||
|
||||
def test_login_failed_event_type_when_rbac_rejects_request() -> None:
|
||||
"""Ответ >= 400 (напр. RBAC-отказ downstream: неизвестная роль / протухший
|
||||
внутренний секрет) -> event_type='login_failed', а НЕ 'login' — раньше эти
|
||||
|
|
|
|||
111
tradein-mvp/backend/tests/test_sentry_init_wiring.py
Normal file
111
tradein-mvp/backend/tests/test_sentry_init_wiring.py
Normal file
|
|
@ -0,0 +1,111 @@
|
|||
"""PR-D2 (платёжный периметр): каждая точка инициализации `sentry_sdk.init(...)`
|
||||
в проекте обязана проводить ОБА канала мониторинга — `before_send` (error-события)
|
||||
и `before_send_transaction` (performance-трейсы). Мотивирующий инцидент (соседний
|
||||
продукт, Птица, вчера): закрыли только error-канал через `before_send`, а
|
||||
`before_send_transaction` остался вообще без обработчика — очистка body/PII там
|
||||
не применялась.
|
||||
|
||||
Инициализация происходит на module-level внутри `if settings.glitchtip_dsn:` —
|
||||
поведенческий тест потребовал бы реального импорта модуля с DSN, выставленным
|
||||
ДО импорта (модуль кэшируется, monkeypatch settings после импорта на init уже не
|
||||
влияет), плюс `sentry_sdk.init` — процесс-глобальный singleton (повторные вызовы
|
||||
из разных тестов друг друга затирают). Вместо этого — статический разбор AST:
|
||||
детерминирован, не трогает process-global state, не зависит от порядка тестов.
|
||||
|
||||
НЕ grep/substring по тексту файла: `before_send_transaction` уже упоминается в
|
||||
docstring-комментариях этих же файлов (объясняющих МОТИВ) — substring-поиск дал
|
||||
бы ложный PASS без единой реальной проводки в `sentry_sdk.init(...)`. Разбор
|
||||
именно keyword-аргументов AST Call-узла `sentry_sdk.init(...)` не подвержен
|
||||
этому false positive.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
_APP_DIR = Path(__file__).resolve().parent.parent / "app"
|
||||
|
||||
# Все известные точки инициализации sentry_sdk в проекте (backend API, scraper
|
||||
# scheduler, telegram support-bridge). Список сверяется отдельным тестом ниже
|
||||
# против грепа по всему `app/`, чтобы новая точка инициализации не прошла мимо
|
||||
# этого файла молча.
|
||||
_SENTRY_INIT_FILES = ["main.py", "scheduler_main.py", "tgbot_main.py"]
|
||||
|
||||
|
||||
def _sentry_init_calls(source: str, filename: str) -> list[ast.Call]:
|
||||
"""Все AST Call-узлы вида `sentry_sdk.init(...)` в модуле."""
|
||||
tree = ast.parse(source, filename=filename)
|
||||
calls = []
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, ast.Call):
|
||||
continue
|
||||
func = node.func
|
||||
if (
|
||||
isinstance(func, ast.Attribute)
|
||||
and func.attr == "init"
|
||||
and isinstance(func.value, ast.Name)
|
||||
and func.value.id == "sentry_sdk"
|
||||
):
|
||||
calls.append(node)
|
||||
return calls
|
||||
|
||||
|
||||
@pytest.mark.parametrize("filename", _SENTRY_INIT_FILES)
|
||||
def test_sentry_init_wires_both_channels(filename: str) -> None:
|
||||
source = (_APP_DIR / filename).read_text(encoding="utf-8")
|
||||
calls = _sentry_init_calls(source, filename)
|
||||
assert calls, f"{filename}: sentry_sdk.init(...) call not found (файл переехал?)"
|
||||
for call in calls:
|
||||
kwarg_names = {kw.arg for kw in call.keywords if kw.arg is not None}
|
||||
assert "before_send" in kwarg_names, (
|
||||
f"{filename}: sentry_sdk.init(...) не передаёт before_send — "
|
||||
"error-канал уходит в GlitchTip без scrub"
|
||||
)
|
||||
assert "before_send_transaction" in kwarg_names, (
|
||||
f"{filename}: sentry_sdk.init(...) не передаёт before_send_transaction — "
|
||||
"transaction-канал уходит в GlitchTip без scrub (ровно вчерашний баг Птицы)"
|
||||
)
|
||||
|
||||
|
||||
def test_sentry_init_before_send_and_transaction_use_same_handler() -> None:
|
||||
"""`before_send` и `before_send_transaction` обязаны указывать на ОДИН и тот
|
||||
же обработчик (одинаковое имя переменной/функции в keyword-значении) — иначе
|
||||
возможен регресс, при котором кто-то поправит один канал и забудет второй,
|
||||
хотя формально оба параметра присутствуют."""
|
||||
for filename in _SENTRY_INIT_FILES:
|
||||
source = (_APP_DIR / filename).read_text(encoding="utf-8")
|
||||
calls = _sentry_init_calls(source, filename)
|
||||
for call in calls:
|
||||
kwargs = {kw.arg: kw.value for kw in call.keywords if kw.arg is not None}
|
||||
before_send = kwargs.get("before_send")
|
||||
before_send_txn = kwargs.get("before_send_transaction")
|
||||
assert before_send is not None and before_send_txn is not None
|
||||
# Оба значения — ссылки на имя (ast.Name), сравниваем идентификатор.
|
||||
assert isinstance(before_send, ast.Name)
|
||||
assert isinstance(before_send_txn, ast.Name)
|
||||
assert before_send.id == before_send_txn.id, (
|
||||
f"{filename}: before_send={before_send.id!r} != "
|
||||
f"before_send_transaction={before_send_txn.id!r} — разные обработчики "
|
||||
"на двух каналах, ровно тот класс бага, что и голый пропуск канала"
|
||||
)
|
||||
|
||||
|
||||
def test_all_sentry_init_call_sites_are_enumerated() -> None:
|
||||
"""Если кто-то добавит НОВУЮ точку инициализации sentry_sdk.init(...) где-то
|
||||
ещё в app/ — этот тест должен упасть, а не молча пропустить её мимо теста
|
||||
выше (список `_SENTRY_INIT_FILES` — руками поддерживаемый allowlist)."""
|
||||
found_files = set()
|
||||
for py_file in _APP_DIR.rglob("*.py"):
|
||||
source = py_file.read_text(encoding="utf-8")
|
||||
if _sentry_init_calls(source, str(py_file)):
|
||||
found_files.add(py_file.relative_to(_APP_DIR).as_posix())
|
||||
|
||||
expected = set(_SENTRY_INIT_FILES)
|
||||
assert found_files == expected, (
|
||||
f"Точки инициализации sentry_sdk.init(...) разошлись со списком в тесте: "
|
||||
f"найдено {sorted(found_files)}, ожидалось {sorted(expected)}. Новую точку "
|
||||
"нужно добавить в _SENTRY_INIT_FILES ЭТОГО файла и проверить оба канала."
|
||||
)
|
||||
|
|
@ -14,6 +14,7 @@ os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:
|
|||
|
||||
from app.observability.sentry_scrub import (
|
||||
redact_telegram_bot_token,
|
||||
scrub_payment_request_body,
|
||||
scrub_pii_event,
|
||||
stabilize_retry_error_fingerprint,
|
||||
)
|
||||
|
|
@ -232,6 +233,128 @@ def test_bare_token_redaction_leaves_benign_colon_strings_untouched(benign: str)
|
|||
assert out["logentry"]["message"] == benign
|
||||
|
||||
|
||||
# ── Платёжный body-wipe (PR-D2, критерий приёмки #1) ─────────────────────────
|
||||
|
||||
|
||||
def test_scrub_payment_request_body_removes_data_for_payments_path() -> None:
|
||||
"""Событие мониторинга с адресом платёжного пути и телом, содержащим `Token`
|
||||
и `Pan`, уходит БЕЗ ключа с телом (PR-D2 acceptance criteria)."""
|
||||
event = {
|
||||
"request": {
|
||||
"url": "https://gendsgn.ru/api/v1/trade-in/payments/notify",
|
||||
"data": {
|
||||
"Token": "deadbeefdeadbeefdeadbeef",
|
||||
"Pan": "220000******0000",
|
||||
"ExpDate": "1230",
|
||||
"CardId": "123456",
|
||||
"RebillId": "987654",
|
||||
"DATA": {"Email": "someone@example.com"},
|
||||
},
|
||||
"method": "POST",
|
||||
}
|
||||
}
|
||||
out = scrub_payment_request_body(event, {})
|
||||
assert out is not None
|
||||
assert "data" not in out["request"]
|
||||
# Остальные поля request не тронуты.
|
||||
assert out["request"]["method"] == "POST"
|
||||
assert out["request"]["url"] == "https://gendsgn.ru/api/v1/trade-in/payments/notify"
|
||||
|
||||
|
||||
def test_scrub_payment_request_body_covers_checkout_too() -> None:
|
||||
"""Матч по сегменту пути, не по конкретному эндпоинту — checkout тоже режется."""
|
||||
event = {
|
||||
"request": {
|
||||
"url": "https://gendsgn.ru/api/v1/trade-in/payments/checkout",
|
||||
"data": {"consent": True, "product_code": "report_pdf"},
|
||||
}
|
||||
}
|
||||
out = scrub_payment_request_body(event, {})
|
||||
assert out is not None
|
||||
assert "data" not in out["request"]
|
||||
|
||||
|
||||
def test_scrub_payment_request_body_case_insensitive_url_match() -> None:
|
||||
"""Регистр URL не должен позволять данным проскочить — Caddy/rbac регистр
|
||||
трактуют по-разному, страховка на случай, если событие всё же породилось."""
|
||||
event = {
|
||||
"request": {
|
||||
"url": "https://gendsgn.ru/API/V1/Trade-In/Payments/Notify",
|
||||
"data": {"Token": "secret"},
|
||||
}
|
||||
}
|
||||
out = scrub_payment_request_body(event, {})
|
||||
assert out is not None
|
||||
assert "data" not in out["request"]
|
||||
|
||||
|
||||
def test_scrub_payment_request_body_leaves_other_paths_untouched() -> None:
|
||||
"""Не платёжный путь — тело остаётся (это не общий kill-switch на request.data)."""
|
||||
event = {
|
||||
"request": {
|
||||
"url": "https://gendsgn.ru/api/v1/trade-in/estimate",
|
||||
"data": {"area_sqm": 50, "region": "66"},
|
||||
}
|
||||
}
|
||||
out = scrub_payment_request_body(event, {})
|
||||
assert out is not None
|
||||
assert out["request"]["data"] == {"area_sqm": 50, "region": "66"}
|
||||
|
||||
|
||||
def test_scrub_payment_request_body_handles_missing_request() -> None:
|
||||
out = scrub_payment_request_body({"level": "error"}, {})
|
||||
assert out == {"level": "error"}
|
||||
|
||||
|
||||
def test_scrub_payment_request_body_handles_non_dict_event() -> None:
|
||||
assert scrub_payment_request_body(None, {}) is None # type: ignore[arg-type]
|
||||
|
||||
|
||||
def test_scrub_payment_request_body_handles_missing_url() -> None:
|
||||
"""`request` без `url` (нестандартный event) — не бросает, тело не трогает."""
|
||||
event = {"request": {"data": {"Token": "x"}}}
|
||||
out = scrub_payment_request_body(event, {})
|
||||
assert out is not None
|
||||
assert out["request"]["data"] == {"Token": "x"}
|
||||
|
||||
|
||||
# ── Расширенный набор платёжных PII-ключей (PR-D2, критерий приёмки #2) ──────
|
||||
|
||||
|
||||
def test_pii_keys_scrub_payment_fields_at_arbitrary_depth() -> None:
|
||||
"""Скрабер вычищает `customer_email`/`customer_phone`/платёжные поля на
|
||||
произвольной глубине вложенности (PR-D2 acceptance criteria)."""
|
||||
event = {
|
||||
"extra": {
|
||||
"checkout_context": {
|
||||
"buyer": {
|
||||
"customer_email": "buyer@example.com",
|
||||
"customer_phone": "+79991234567",
|
||||
"nested_list": [
|
||||
{"pan": "220000******1111", "expdate": "0129"},
|
||||
{"cardid": "abc123", "rebillid": "xyz789"},
|
||||
],
|
||||
},
|
||||
"token": "sensitive-token-value",
|
||||
"terminalkey": "TinkoffBankTest",
|
||||
"order_id": "ord_123",
|
||||
}
|
||||
}
|
||||
}
|
||||
out = scrub_pii_event(event, {})
|
||||
ctx = out["extra"]["checkout_context"]
|
||||
assert ctx["buyer"]["customer_email"] == "[REDACTED]"
|
||||
assert ctx["buyer"]["customer_phone"] == "[REDACTED]"
|
||||
assert ctx["buyer"]["nested_list"][0]["pan"] == "[REDACTED]"
|
||||
assert ctx["buyer"]["nested_list"][0]["expdate"] == "[REDACTED]"
|
||||
assert ctx["buyer"]["nested_list"][1]["cardid"] == "[REDACTED]"
|
||||
assert ctx["buyer"]["nested_list"][1]["rebillid"] == "[REDACTED]"
|
||||
assert ctx["token"] == "[REDACTED]"
|
||||
assert ctx["terminalkey"] == "[REDACTED]"
|
||||
# non-PII поле остаётся.
|
||||
assert ctx["order_id"] == "ord_123"
|
||||
|
||||
|
||||
def test_composed_before_send_scrubs_pii_and_token_together() -> None:
|
||||
"""Композиция, реально используемая в `app.tgbot_main._before_send`: PII-scrub
|
||||
(ключ-based) И token-redaction (regex full-text) применяются оба, не заменяя
|
||||
|
|
@ -262,6 +385,37 @@ def test_composed_before_send_scrubs_pii_and_token_together() -> None:
|
|||
assert "8663867262:AAExampleSecretPartAbCdEf123" not in frame_url
|
||||
|
||||
|
||||
def test_composed_before_send_payment_wipe_pii_and_token_together() -> None:
|
||||
"""Полная композиция `app.main._before_send` (PR-D2): body-wipe для платёжного
|
||||
пути → PII-scrub → token-redaction, в этом порядке, все три применяются."""
|
||||
event = {
|
||||
"request": {
|
||||
"url": "https://gendsgn.ru/api/v1/trade-in/payments/notify",
|
||||
"data": {"Token": "deadbeef", "Pan": "220000******0000"},
|
||||
},
|
||||
"extra": {"client_phone": "+79991234567"},
|
||||
"exception": {
|
||||
"values": [{"stacktrace": {"frames": [{"vars": {"url": _LEAKED_TOKEN_URL}}]}}]
|
||||
},
|
||||
}
|
||||
|
||||
def composed_before_send(evt, hint):
|
||||
scrubbed = scrub_payment_request_body(evt, hint)
|
||||
if scrubbed is None:
|
||||
return None
|
||||
scrubbed = scrub_pii_event(scrubbed, hint)
|
||||
if scrubbed is None:
|
||||
return None
|
||||
return redact_telegram_bot_token(scrubbed, hint)
|
||||
|
||||
out = composed_before_send(event, {})
|
||||
assert out is not None
|
||||
assert "data" not in out["request"]
|
||||
assert out["extra"]["client_phone"] == "[REDACTED]"
|
||||
frame_url = out["exception"]["values"][0]["stacktrace"]["frames"][0]["vars"]["url"]
|
||||
assert "8663867262:AAExampleSecretPartAbCdEf123" not in frame_url
|
||||
|
||||
|
||||
# ── RetryError fingerprint stabilization (glitchtip-noise, #<GlitchTip triage>) ─
|
||||
#
|
||||
# tenacity.RetryError.__str__() тащит repr() последнего Future — memory address
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue