fix(tradein/payments): строгий разбор нотификации и отказ вместо догадок на враждебном входе #2737

Merged

bot-reviewer merged 1 commit from feat/tradein-payments-notification-hardening into main

2026-08-06 16:10:30 +00:00

bot-backend commented

2026-08-06 12:49:51 +00:00

Collaborator

Summary

Усиление слоя платежей МЕРЫ (app/services/payments/*) перед публичным эндпоинтом нотификации Т-Банка (PR-D). Без изменений роутеров/RBAC/config — только сам платёжный слой.

Свойство алгоритма подписи Т-Банка (важно, зафиксировано намеренно)

Алгоритм Token конкатенирует значения полей payload без разделителя между ними (см. token.py). Из-за этого символы могут "перекладываться" между лексикографически соседними ключами так, что итоговая подпись всё равно сходится — это свойство алгоритма банка, которое нельзя изменить (мы не выбираем формат Token, который реально пришлёт банк на проде).

Проверено живым расчётом на официальном эталонном векторе документации:
Amount=1111, CardId="000000" даёт тот же Token, что и Amount=11, CardId="11000000" (лишняя 1 "перетекает" из конца Amount в начало CardId, потому что Amount < CardId лексикографически и оба поля стоят рядом в конкатенации).

Следствие: подпись сама по себе не гарантирует, что банк прислал именно ту сумму, которую он в реальности захолдировал/списал. Единственная реальная защита от этого — обработчик нотификации (PR-D) обязан сверять amount_kopecks из нотификации с уже сохранённым payments.amount_kopecks в БД (запись, созданная на init_payment(), найденная по order_id/payment_id) до того, как нотификация принимается как валидное событие. Расхождение = отказ, а не «примерно похоже — примем». Это зафиксировано docstring'ами в notification.py, чтобы следующий автор (PR-D) не пропустил.

Что сделано

token.py — verify_notification_token больше не кидает исключение на враждебном входе.
- payload не dict (например список) → False, а не AttributeError.
- Token с не-ASCII символами → False, а не TypeError из hmac.compare_digest.
- Любая ошибка внутри sign() на мусорном payload (например float-поле после п.3) → False, а не проброс исключения.
- Причина: после появления публичной ручки нотификации необработанное исключение = неаутентифицированный HTTP 500 в ответ банку, а любой ответ кроме "OK" банк трактует как временный сбой и ретраит уведомление почасово в течение суток.
Новый модуль notification.py — parse_notification().
Строгий типизированный разбор dict → TBankNotification (frozen dataclass) после успешной проверки подписи. Сырой dict дальше в бизнес-логику не уходит.
- Success — только настоящий bool (не "true", не 1).
- Amount — только int; bool (подкласс int в Python) отсекается отдельно ДО общей int-проверки.
- Status/OrderId/PaymentId/TerminalKey — только непустой str.
- Любое несоответствие → NotificationParseError с указанием поля и того, что реально пришло.
Убрана угадывающая сериализация float в sign().
Раньше float сериализовался через format(value, "f") + rstrip нулей — недокументированный формат. Расхождение: 0.1 + 0.2 подписывалось бы как "0.3", а json.dumps(0.1 + 0.2) реально даёт "0.30000000000000004" — Token не соответствовал бы факту, что уходит в JSON-теле. Теперь float в подписываемых полях → явный TokenSigningError. Тесты, закреплявшие float-поведение как спецификацию, переписаны под новое поведение (падение вместо угадывания).
Докстринги-контракты для PR-D:
- parse_notification/TBankNotification — сверка суммы с payments.amount_kopecks в БД обязательна (см. выше).
- confirm() — после TBankApiError слепой cancel() запрещён: таймаут мог прийти на уже успевшем Confirm, cancel() вернёт уже захваченные банком деньги. Сначала get_state().
- init_payment() — после сетевой ошибки разбираться через check_order(), не повторять init_payment() вслепую (риск второго холда на тот же OrderId).
- Module docstring tbank_client.py — worst case одного вызова ~74с (4 попытки × 15с таймаут + backoff 2+4+8=14с), а бюджет ответа банку на нотификацию ~10с → исходящий HTTP внутри обработчика нотификации запрещён.
Тесты на ретраи Confirm/Cancel. Раньше retry-покрытие шло только через init_payment/get_state — денежные вызовы не были покрыты 5xx/network-error/4xx/gives-up сценариями.

Границы (соблюдены)

Роутеров/эндпоинтов не создавал, rbac.py/roles.yaml/Caddyfile/config.py/main.py/миграции не трогал. Ничего не деплоил. Сеть в тестах не используется — только httpx.MockTransport.

Test plan

uv run pytest tests/test_payments_token.py tests/test_payments_notification.py tests/test_payments_receipt.py tests/services/payments/ — 108 passed
ruff check — all checks passed
ruff format --check — all files already formatted
Существующие тесты не менялись, кроме двух float-тестов в test_payments_token.py (явно предписано задачей) — остальные (эталонные векторы, verify-тесты) не тронуты

## Summary Усиление слоя платежей МЕРЫ (`app/services/payments/*`) перед публичным эндпоинтом нотификации Т-Банка (PR-D). Без изменений роутеров/RBAC/config — только сам платёжный слой. ### Свойство алгоритма подписи Т-Банка (важно, зафиксировано намеренно) Алгоритм `Token` конкатенирует значения полей payload **без разделителя** между ними (см. `token.py`). Из-за этого символы могут "перекладываться" между лексикографически соседними ключами так, что итоговая подпись всё равно сходится — это **свойство алгоритма банка, которое нельзя изменить** (мы не выбираем формат Token, который реально пришлёт банк на проде). Проверено живым расчётом на официальном эталонном векторе документации: `Amount=1111, CardId="000000"` даёт **тот же** Token, что и `Amount=11, CardId="11000000"` (лишняя `1` "перетекает" из конца `Amount` в начало `CardId`, потому что `Amount < CardId` лексикографически и оба поля стоят рядом в конкатенации). Следствие: **подпись сама по себе не гарантирует, что банк прислал именно ту сумму, которую он в реальности захолдировал/списал**. Единственная реальная защита от этого — обработчик нотификации (PR-D) обязан сверять `amount_kopecks` из нотификации с уже сохранённым `payments.amount_kopecks` в БД (запись, созданная на `init_payment()`, найденная по `order_id`/`payment_id`) **до** того, как нотификация принимается как валидное событие. Расхождение = отказ, а не «примерно похоже — примем». Это зафиксировано docstring'ами в `notification.py`, чтобы следующий автор (PR-D) не пропустил. ### Что сделано 1. **`token.py` — `verify_notification_token` больше не кидает исключение на враждебном входе.** - `payload` не `dict` (например список) → `False`, а не `AttributeError`. - `Token` с не-ASCII символами → `False`, а не `TypeError` из `hmac.compare_digest`. - Любая ошибка внутри `sign()` на мусорном payload (например float-поле после п.3) → `False`, а не проброс исключения. - Причина: после появления публичной ручки нотификации необработанное исключение = неаутентифицированный HTTP 500 в ответ банку, а любой ответ кроме `"OK"` банк трактует как временный сбой и ретраит уведомление почасово в течение суток. 2. **Новый модуль `notification.py` — `parse_notification()`.** Строгий типизированный разбор `dict` → `TBankNotification` (frozen dataclass) **после** успешной проверки подписи. Сырой `dict` дальше в бизнес-логику не уходит. - `Success` — только настоящий `bool` (не `"true"`, не `1`). - `Amount` — только `int`; `bool` (подкласс `int` в Python) отсекается отдельно ДО общей `int`-проверки. - `Status`/`OrderId`/`PaymentId`/`TerminalKey` — только непустой `str`. - Любое несоответствие → `NotificationParseError` с указанием поля и того, что реально пришло. 3. **Убрана угадывающая сериализация float в `sign()`.** Раньше `float` сериализовался через `format(value, "f")` + rstrip нулей — недокументированный формат. Расхождение: `0.1 + 0.2` подписывалось бы как `"0.3"`, а `json.dumps(0.1 + 0.2)` реально даёт `"0.30000000000000004"` — Token не соответствовал бы факту, что уходит в JSON-теле. Теперь `float` в подписываемых полях → явный `TokenSigningError`. Тесты, закреплявшие float-поведение как спецификацию, переписаны под новое поведение (падение вместо угадывания). 4. **Докстринги-контракты для PR-D:** - `parse_notification`/`TBankNotification` — сверка суммы с `payments.amount_kopecks` в БД обязательна (см. выше). - `confirm()` — после `TBankApiError` слепой `cancel()` запрещён: таймаут мог прийти на уже успевшем `Confirm`, `cancel()` вернёт уже захваченные банком деньги. Сначала `get_state()`. - `init_payment()` — после сетевой ошибки разбираться через `check_order()`, не повторять `init_payment()` вслепую (риск второго холда на тот же `OrderId`). - Module docstring `tbank_client.py` — worst case одного вызова ~74с (4 попытки × 15с таймаут + backoff 2+4+8=14с), а бюджет ответа банку на нотификацию ~10с → исходящий HTTP внутри обработчика нотификации запрещён. 5. **Тесты на ретраи `Confirm`/`Cancel`.** Раньше retry-покрытие шло только через `init_payment`/`get_state` — денежные вызовы не были покрыты 5xx/network-error/4xx/gives-up сценариями. ### Границы (соблюдены) Роутеров/эндпоинтов не создавал, `rbac.py`/`roles.yaml`/`Caddyfile`/`config.py`/`main.py`/миграции не трогал. Ничего не деплоил. Сеть в тестах не используется — только `httpx.MockTransport`. ## Test plan - [x] `uv run pytest tests/test_payments_token.py tests/test_payments_notification.py tests/test_payments_receipt.py tests/services/payments/` — 108 passed - [x] `ruff check` — all checks passed - [x] `ruff format --check` — all files already formatted - [x] Существующие тесты не менялись, кроме двух float-тестов в `test_payments_token.py` (явно предписано задачей) — остальные (эталонные векторы, verify-тесты) не тронуты

bot-backend added 1 commit 2026-08-06 12:49:52 +00:00