Some checks failed
CI Trade-In / changes (pull_request) Successful in 9s
CI / changes (pull_request) Failing after 8s
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 / browser-tests (pull_request) Successful in 46s
CI Trade-In / frontend-checks (pull_request) Successful in 1m3s
CI Trade-In / backend-tests (pull_request) Successful in 4m42s
Ревью четырьмя независимыми линзами (периметр, семантика Caddy, политика ПДн
против кода, фронт) + по два проверяющих на каждую находку. Ниже — то, что
пережило проверку и воспроизведено на живом коде, а не выведено из чтения.
## Caddy: открытый редирект и потерянные ссылки
Захват хвоста регекспом (`^/trade-in/mera-public/(.+)$` → `redir /{re…1}`) —
открытый редирект. Захват берётся из РАСКОДИРОВАННОГО пути, поэтому
`/trade-in/mera-public/%5Cevil.example/pay` даёт цель `/\evil.example/pay`, а
браузеры трактуют `/\` как `//` — Location уводит на чужой хост. Готовая
фишинговая заготовка с домена, который напечатан внутри оферты и уходит
модератору эквайера. Заменено поимённым списком путей: такой адрес просто не
матчится.
Адреса со слэшем на конце (`/oferta/`, и длинные `…/oferta/`) отдавали 404 —
ровно те ссылки, ради сохранности которых редирект и делался. Добавлена
нормализация, цепочка замкнута (проверено: 2 перехода → 200).
Query-строка терялась: размещённые ссылки с UTM приходили бы в аналитику как
прямой заход. `uri strip_prefix` + `{uri}` переносит её. Обёртка `route`
обязательна — без неё `redir` выполняется раньше `uri` и Location равен
исходному адресу (бесконечный цикл, поймано на стенде).
`/v3` — черновое превью с маркетинговыми плейсхолдерами — было открыто на
боевом домене молча. Теперь названо вслух и запинено тестом.
## Гейты, которых не было
`caddy validate` не звал НИ ОДИН workflow, а deploy применяет конфиг не через
`reload` (тот отказался бы принять битый), а через `up -d --force-recreate` —
опечатка уводит контейнер в crash-loop и роняет ВСЕ домены. Добавлен гейт в
ci.yml, тем же образом caddy:2, что и на проде.
Проверка «роут ↔ Caddy» была односторонней и пропускала обратную ошибку —
путь, открытый наружу, о котором приложение не знает. Так и уехал `/v3`.
Теперь двусторонняя, плюс проверка, что для каждой страницы есть 301.
## Бюджет внешнего геокодера
Per-IP окна ограничивают одного клиента, но не сумму: 40/мин с адреса — это
57 600 в сутки при бесплатном тире DaData в 10 000, ОБЩЕМ с закрытым контуром.
Подтверждено на проде: достаточно упомянуть не-екатеринбургский город, чтобы
локальный тир отключился и запрос гарантированно ушёл во внешний сервис. То
есть один скрипт оставлял без подсказок платящих пилотов.
Per-IP снижен до 20/мин, добавлен общий суточный потолок 2000 и потолок
одновременных подсказок (4): кадастровый тир уходит в FDW-скан чужой базы,
держит соединение около секунды, а пул общий с B2B — полтора десятка
параллельных публичных запросов клали бы закрытый контур.
## «Адрес нигде не сохраняется» — теперь правда целиком
Две утечки, обе воспроизведены:
1. ЖУРНАЛЫ. Геокодер печатает введённую строку открытым текстом на каждый
вызов, прод пишет stdout в persistent journald — адрес ложился на диск
рядом с IP того же запроса в access-логе Caddy. Закрыто фильтром логов на
время публичного запроса (contextvar, переживает await и to_thread).
Закрытый контур логи сохраняет: они нужны для разбора жалоб пилотов.
2. МОНИТОРИНГ. sentry_sdk кладёт в событие ПОЛНОЕ тело запроса — а тело
публичной ручки это ровно `{"q": "<адрес>"}`; `send_default_pii=False` тут
не гейт, он про куки. Плюс брэдкрамб httpx несёт адрес в query геокодера.
Закрыто `scrub_public_address`.
Текст п. 5.4 политики расширен до «ни в журналы веб-сервера, ни в технические
журналы, ни в мониторинг» — ровно то, что теперь обеспечено кодом.
## Фронт
- Отмена запроса подсказок откладывалась внутрь следующего debounce-такта и
не наступала вовсе, если человек переставал печатать: ответ по старой строке
долетал и ложился в список. Контроллер создаётся сразу, отменяется в cleanup.
- Список схлопывался на каждое нажатие — клик по намеченному пункту
промахивался. Старая выдача висит, пока не пришла новая.
- «Комнат» с лэндинга — свободный текст: «студия» не совпадала ни с одним
option, селект показывал пустоту, parseInt давал NaN, на сервер уходил
rooms: null → 422 с текстом «сломалось на нашей стороне». Нормализация
вынесена чистой функцией и покрыта тестами.
- У пробы покрытия не было ни таймаута, ни отмены: оборванное соединение
оставляло кнопку в «Смотрим данные…» навсегда. 15 с + понятный текст.
- Ошибка подсказок глушилась в пустой список — тупик без объяснения.
- Комбобокс: Tab проваливался в кнопки подсказок, список не закрывался по
уходу фокуса и перекрывал поля, Escape оставлял висячий aria-activedescendant.
## Проверено
Локальный стенд (реальный site-блок Caddy + заглушка): 18 маршрутов, включая
`%5C`, `//`, `%2F` — все три теперь 404. vitest 55 passed, backend 17 passed по
публичному API, tsc, lint, build, isolation guard 41 файл, caddy validate.
Мутации: снять редакцию логов → падает тест журналов; не вырезать тело запроса
→ падает тест мониторинга; убрать /estimate из Caddy → падает тест маршрутов.
97 lines
6.1 KiB
Python
97 lines
6.1 KiB
Python
"""Пометка «этот запрос пришёл из публичной формы» и её единственное следствие:
|
||
адрес, который ввёл аноним, не попадает в журналы.
|
||
|
||
ЗАЧЕМ ЭТО СУЩЕСТВУЕТ
|
||
--------------------
|
||
На `meraocenka.ru/estimate` и в политике обработки ПДн сказано, что введённый
|
||
адрес нигде не сохраняется. По базам данных это правда (обе публичные ручки
|
||
только читают), а по журналам — не было: геокодер логирует запрос открытым
|
||
текстом на каждый вызов, например
|
||
|
||
INFO app.services.dadata: dadata suggest: 'онуфриева 24' → 5 вариантов
|
||
|
||
Прод пишет stdout контейнеров в journald с persistent-хранилищем
|
||
(`tradein-mvp/docker-compose.prod.yml`), то есть строка ложится на диск и живёт
|
||
там неделями. Рядом, в access-логе Caddy, лежит IP того же запроса с той же
|
||
меткой времени — то есть адрес квартиры фактически сохранён и сопоставим с
|
||
человеком. Ровно то, что публичная страница обещает не делать.
|
||
|
||
Найдено состязательным ревью PR публичного периметра (16.08.2026) и
|
||
воспроизведено на проде, а не выведено из чтения кода.
|
||
|
||
ПОЧЕМУ ФИЛЬТР, А НЕ ПРАВКА КАЖДОГО ВЫЗОВА logger
|
||
------------------------------------------------
|
||
Мест, где адрес попадает в лог, много (`app/services/dadata.py`,
|
||
`app/services/geocoder.py` — успех, пустая выдача, сетевая ошибка, таймаут,
|
||
кадастровый фолбэк), и любое новое добавится незаметно. Обещание не должно
|
||
зависеть от того, вспомнил ли автор следующей правки про эту страницу.
|
||
Фильтр — единственная точка, которая закрывает и уже написанное, и будущее.
|
||
|
||
ПОЧЕМУ contextvar
|
||
-----------------
|
||
Публичный и закрытый контуры обслуживает ОДИН процесс, и один и тот же
|
||
`suggest()` вызывают оба. Различить их можно только по текущему запросу.
|
||
`ContextVar` — то, что переживает `await` и копируется в `asyncio.to_thread`
|
||
(им геокодер уходит в синхронный кадастровый тир), в отличие от глобального
|
||
флага, который в конкурентной обработке принадлежал бы соседнему запросу.
|
||
|
||
Для B2B-трафика ничего не меняется: там флаг не выставлен, логи прежние — они
|
||
нужны, чтобы разбирать жалобы пилотов на подсказки.
|
||
"""
|
||
|
||
from __future__ import annotations
|
||
|
||
import logging
|
||
from collections.abc import Iterator
|
||
from contextlib import contextmanager
|
||
from contextvars import ContextVar
|
||
|
||
#: Истинно, пока обрабатывается запрос анонимной публичной формы.
|
||
is_public_request: ContextVar[bool] = ContextVar("mera_is_public_request", default=False)
|
||
|
||
#: Что видно в журнале вместо сообщения. Уровень и логгер сохраняются — по ним
|
||
#: по-прежнему видно, что вызов был и чем закончился.
|
||
REDACTED_MESSAGE = "<публичный запрос МЕРЫ: содержимое скрыто>"
|
||
|
||
#: Логгеры, чьи записи могут содержать введённый адрес.
|
||
ADDRESS_LOGGERS = ("app.services.dadata", "app.services.geocoder")
|
||
|
||
|
||
@contextmanager
|
||
def public_request_scope() -> Iterator[None]:
|
||
"""Помечает текущий запрос публичным на время работы блока."""
|
||
token = is_public_request.set(True)
|
||
try:
|
||
yield
|
||
finally:
|
||
is_public_request.reset(token)
|
||
|
||
|
||
class RedactPublicAddressFilter(logging.Filter):
|
||
"""Заменяет сообщение целиком, пока обрабатывается публичный запрос.
|
||
|
||
Целиком, а не по ключам: в шаблонах сообщений адрес стоит рядом с
|
||
безобидными аргументами (`"%r → %d вариантов"`), и отличить их друг от
|
||
друга внутри фильтра нельзя. Терять текст сообщения на публичном пути
|
||
дешевле, чем хранить адреса; на закрытом контуре текст остаётся полным.
|
||
"""
|
||
|
||
def filter(self, record: logging.LogRecord) -> bool:
|
||
if is_public_request.get():
|
||
record.msg = REDACTED_MESSAGE
|
||
record.args = ()
|
||
return True
|
||
|
||
|
||
def install_address_log_redaction() -> None:
|
||
"""Вешает фильтр на логгеры, видящие адрес. Идемпотентно.
|
||
|
||
Фильтр ставится на КОНКРЕТНЫЕ логгеры, а не на корневой хендлер: фильтры
|
||
логгера применяются к записям этого логгера, а не ко всему, что через
|
||
хендлер проходит, — то есть посторонние сообщения (пул соединений, старт
|
||
приложения) во время публичного запроса не пострадают.
|
||
"""
|
||
for name in ADDRESS_LOGGERS:
|
||
logger = logging.getLogger(name)
|
||
if not any(isinstance(f, RedactPublicAddressFilter) for f in logger.filters):
|
||
logger.addFilter(RedactPublicAddressFilter())
|