From 5626d9e720fb4b390ddcaa3c19df886bca480a0c Mon Sep 17 00:00:00 2001 From: bot-backend Date: Tue, 28 Jul 2026 15:24:21 +0300 Subject: [PATCH] =?UTF-8?q?feat(mera/b2c):=20=D0=BF=D1=80=D0=B0=D0=B2?= =?UTF-8?q?=D0=BE=D0=B2=D0=B0=D1=8F=20=D1=80=D0=B0=D0=BC=D0=BA=D0=B0=20?= =?UTF-8?q?=E2=80=94=20=D1=81=D0=BE=D0=B3=D0=BB=D0=B0=D1=81=D0=B8=D0=B5=20?= =?UTF-8?q?=D0=B4=D0=BE=20=D1=81=D0=BE=D1=85=D1=80=D0=B0=D0=BD=D0=B5=D0=BD?= =?UTF-8?q?=D0=B8=D1=8F,=20=D1=83=D0=B4=D0=B0=D0=BB=D0=B5=D0=BD=D0=B8?= =?UTF-8?q?=D0=B5=20=D0=BF=D0=BE=20=D1=81=D1=80=D0=BE=D0=BA=D1=83=20=D0=B8?= =?UTF-8?q?=20=D0=BF=D0=BE=20=D0=B7=D0=B0=D0=BF=D1=80=D0=BE=D1=81=D1=83=20?= =?UTF-8?q?(=D1=8D=D1=82=D0=B0=D0=BF=204=20=D0=B8=D0=B7=208)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Три дефекта, каждый блокировал легальный публичный запуск. 1. Адрес физлица сохранялся в базу ДО любого согласия: согласие фиксировалось только на форме заявки, то есть ПОСЛЕ записи адреса. Для пилота с договором терпимо, для человека с улицы — нет. Проверка согласия поставлена первой строкой расчёта, до геокодирования и до обоих мест записи адреса. Хранение — колонками на самой оценке, 1:1 с уже работающим прецедентом для заявок (миграция 182): IP клиента, версия политики, дословный снимок текста. Отдельная таблица событий не заводилась: согласие даётся ровно на создание этой строки, и когда строка удаляется по сроку, исчезновение доказательства вместе с данными логично. Enforcement НЕ выводится из пустого created_by — первая версия так и делала и сломала 92 несвязанных теста оценщика, которые зовут расчёт без имени пользователя, проверяя ценовую логику. Вместо этого явный флаг, который выставляет единственный боевой вызывающий. B2B-поток не тронут: поле согласия опционально, иначе сломались бы пилоты, чей фронт его не шлёт. 2. Срок жизни оценки применялся только как фильтр при чтении — физического удаления не было ни в одной фоновой задаче, данные жили вечно вопреки декларированному сроку. Заведена задача удаления пачками с ограничением на прогон и коммитом после каждой пачки, идемпотентная. В расписании она ВЫКЛЮЧЕНА: это первая автоматическая задача, удаляющая персональные данные, и первый прогон должен быть под наблюдением. 3. Пути «удалите мои данные» не было. Добавлен сервис удаления и админская ручка. Ключи: имя пользователя, идентификатор оценки, телефон, чат в телеграме. Честно зафиксировано в коде: аноним без ссылки на оценку, без оставленного телефона и без обращения в поддержку неидентифицируем — удалить его данные без дополнительной идентификации нельзя. Отдельно: удаление чистит только копию в базе, зеркало переписки в телеграм-топике не удаляется ничем в кодовой базе, нужен ручной шаг. 4. Соответствие текста согласия на фронте и снимка на бэке держалось на комментарии. Теперь есть тест, который ловит расхождение. Сроки хранения вынесены в настройки. Значение для заявок предложено инженерно (типичный отраслевой диапазон), юридически обоснованный срок — за юристом, и это записано в коде. Тесты: 2775 passed. --- tradein-mvp/backend/app/api/v1/lead.py | 16 +- .../backend/app/api/v1/privacy_admin.py | 79 +++++ tradein-mvp/backend/app/api/v1/trade_in.py | 16 +- tradein-mvp/backend/app/core/config.py | 29 ++ tradein-mvp/backend/app/main.py | 2 + tradein-mvp/backend/app/schemas/trade_in.py | 9 + .../backend/app/services/data_erasure.py | 175 ++++++++++ tradein-mvp/backend/app/services/estimator.py | 137 +++++++- .../backend/app/services/product_handlers.py | 21 ++ .../app/tasks/purge_expired_trade_in_data.py | 164 +++++++++ .../192_trade_in_estimates_consent_proof.sql | 93 +++++ .../sql/193_trade_in_privacy_retention.sql | 93 +++++ .../tests/test_consent_text_frontend_sync.py | 89 +++++ .../backend/tests/test_data_erasure.py | 160 +++++++++ .../tests/test_estimate_consent_gate.py | 319 ++++++++++++++++++ .../tests/test_purge_expired_trade_in_data.py | 232 +++++++++++++ 16 files changed, 1622 insertions(+), 12 deletions(-) create mode 100644 tradein-mvp/backend/app/api/v1/privacy_admin.py create mode 100644 tradein-mvp/backend/app/services/data_erasure.py create mode 100644 tradein-mvp/backend/app/tasks/purge_expired_trade_in_data.py create mode 100644 tradein-mvp/backend/data/sql/192_trade_in_estimates_consent_proof.sql create mode 100644 tradein-mvp/backend/data/sql/193_trade_in_privacy_retention.sql create mode 100644 tradein-mvp/backend/tests/test_consent_text_frontend_sync.py create mode 100644 tradein-mvp/backend/tests/test_data_erasure.py create mode 100644 tradein-mvp/backend/tests/test_estimate_consent_gate.py create mode 100644 tradein-mvp/backend/tests/test_purge_expired_trade_in_data.py diff --git a/tradein-mvp/backend/app/api/v1/lead.py b/tradein-mvp/backend/app/api/v1/lead.py index a5f1d106..41ebe082 100644 --- a/tradein-mvp/backend/app/api/v1/lead.py +++ b/tradein-mvp/backend/app/api/v1/lead.py @@ -21,6 +21,7 @@ from __future__ import annotations import logging import re +from datetime import UTC, datetime, timedelta from typing import Annotated, Any, Literal from uuid import UUID @@ -30,6 +31,7 @@ from sqlalchemy import text from sqlalchemy.orm import Session from app.api.v1.trade_in import _assert_estimate_access +from app.core.config import settings from app.core.db import get_db logger = logging.getLogger(__name__) @@ -115,17 +117,26 @@ async def create_trade_in_lead( # consent_text_snapshot теперь durable-колонки на trade_in_leads (migration 182, # ранее — только audit-лог, #2497 TODO). client_ip может быть None (нет # X-Forwarded-For и request.client) — колонка nullable, CAST(NULL AS inet) валиден. + # + # ЭТАП 4 B2C: expires_at (migration 193) — раньше лид хранился бессрочно + # (никакого TTL вообще не было, в отличие от trade_in_estimates.expires_at). + # Считаем на insert-time тем же паттерном, что estimator.py делает для + # trade_in_estimates — retention-период вынесен в settings, не хардкод. + expires_at = datetime.now(tz=UTC) + timedelta(days=settings.trade_in_lead_retention_days) + row = ( db.execute( text( """ INSERT INTO trade_in_leads ( estimate_id, phone, consent, source, user_agent, - client_ip, consent_policy_version, consent_text_snapshot + client_ip, consent_policy_version, consent_text_snapshot, + expires_at ) VALUES ( CAST(:estimate_id AS uuid), :phone, :consent, :source, :user_agent, - CAST(:client_ip AS inet), :consent_policy_version, :consent_text_snapshot + CAST(:client_ip AS inet), :consent_policy_version, :consent_text_snapshot, + :expires_at ) RETURNING CAST(id AS text), created_at """ @@ -139,6 +150,7 @@ async def create_trade_in_lead( "client_ip": client_ip, "consent_policy_version": _CONSENT_POLICY_VERSION, "consent_text_snapshot": _CONSENT_TEXT_SNAPSHOT, + "expires_at": expires_at, }, ) .mappings() diff --git a/tradein-mvp/backend/app/api/v1/privacy_admin.py b/tradein-mvp/backend/app/api/v1/privacy_admin.py new file mode 100644 index 00000000..9ea0ee9d --- /dev/null +++ b/tradein-mvp/backend/app/api/v1/privacy_admin.py @@ -0,0 +1,79 @@ +"""Admin right-to-erasure endpoint (152-ФЗ) — ЭТАП 4 B2C launch, part C. + +Auth не нужен в этом файле — вся ветка `/api/v1/admin/*` уже гейтится +`rbac_guard` middleware в app/main.py (`_ADMIN_API_RE`, role != admin → 403), +тем же паттерном, что app/api/v1/audit.py. + +Мутационный (DELETE), поэтому осторожно: это НЕ self-service для конечного +пользователя. Оператор поддержки/admin вызывает это ПОСЛЕ того, как убедился +(вне этого API — телефон/estimate-ссылка/переписка), что запрос на удаление +реально пришёл от владельца данных, а не от третьего лица, знающего чей-то +номер телефона. Идентификация анонима — см. app/services/data_erasure.py +module docstring (честно про то, что не всегда разрешимо). +""" + +from __future__ import annotations + +import asyncio +import logging +from typing import Annotated +from uuid import UUID + +from fastapi import APIRouter, Depends, HTTPException +from pydantic import BaseModel, Field +from sqlalchemy.orm import Session + +from app.core.db import get_db + +logger = logging.getLogger(__name__) + +router = APIRouter() + + +class DataErasureRequest(BaseModel): + """Хотя бы одно поле обязательно — см. erase_person_data ValueError guard.""" + + username: str | None = Field(default=None, max_length=200) + estimate_ids: list[UUID] | None = None + phone: str | None = Field(default=None, max_length=32) + tg_chat_id: int | None = None + + +@router.post("/privacy/erase") +async def erase_person_data_endpoint( + payload: DataErasureRequest, + db: Annotated[Session, Depends(get_db)], +) -> dict[str, dict[str, int]]: + """Физически удалить данные человека по одному или нескольким идентификаторам. + + Идентификаторы (хотя бы один): + - username — B2B-пилот: удаляет ВСЕ его оценки (created_by=username, CASCADE + подчищает фото/IMV-оценки), связанные лиды, веб-чат поддержки. + - estimate_ids — конкретные оценки по UUID (анонимный путь: человек прислал + ссылку/PDF со своим estimate_id) + лиды, привязанные к ним. + - phone — лиды с этим номером телефона (независимо от привязки к оценке). + - tg_chat_id — Telegram-поддержка (@MERAsupport_bot), включая переписку В + ЭТОЙ БД. НЕ удаляет зеркало в Telegram-топике (см. + app/services/data_erasure.py — ВАЖНЫЙ ФАКТ, честно, не скрываем). + + 422 если ни один идентификатор не передан (не даём случайно вызвать + "удали всё" пустым телом). + """ + if not any([payload.username, payload.estimate_ids, payload.phone, payload.tg_chat_id]): + raise HTTPException( + status_code=422, + detail="at least one identifier required: username / estimate_ids / phone / tg_chat_id", + ) + + from app.services.data_erasure import erase_person_data + + counters = await asyncio.to_thread( + erase_person_data, + db, + username=payload.username, + estimate_ids=payload.estimate_ids, + phone=payload.phone, + tg_chat_id=payload.tg_chat_id, + ) + logger.info("admin privacy erase requested -> %s", counters) + return {"deleted": counters} diff --git a/tradein-mvp/backend/app/api/v1/trade_in.py b/tradein-mvp/backend/app/api/v1/trade_in.py index 13f4213a..4493c542 100644 --- a/tradein-mvp/backend/app/api/v1/trade_in.py +++ b/tradein-mvp/backend/app/api/v1/trade_in.py @@ -171,8 +171,22 @@ async def estimate( # явный 503 — так любая БУДУЩАЯ реальная ошибка становится видимой, а не # «глотается» шлюзом. HTTPException пробрасываем как есть (это не сбой). # created_by (#656) прокидываем в estimate_quality для скоупа /history. + # ЭТАП 4 B2C (152-ФЗ): require_consent=True только когда нет + # X-Authenticated-User — сегодня rbac_guard (app/core/rbac.py) уже требует + # этот заголовок на любом non-public пути, так что эта ветка пока + # недостижима в проде (анонимный /estimate ещё не открыт другими частями + # ЭТАП 4/B2C работ) — гейт готов ЗАРАНЕЕ, на момент открытия анонимного + # доступа. client_ip — proof-of-consent (estimate_quality персистит его + # на trade_in_estimates только когда require_consent=True; B2B-пилоты + # остаются NULL, см. estimator.py::_estimate_consent_persist_fields). try: - result = await estimate_quality(payload, db, created_by=x_authenticated_user) + result = await estimate_quality( + payload, + db, + created_by=x_authenticated_user, + client_ip=_client_ip(request), + require_consent=x_authenticated_user is None, + ) except HTTPException: raise except Exception: diff --git a/tradein-mvp/backend/app/core/config.py b/tradein-mvp/backend/app/core/config.py index 7340a8ee..2b2d264e 100644 --- a/tradein-mvp/backend/app/core/config.py +++ b/tradein-mvp/backend/app/core/config.py @@ -574,6 +574,35 @@ class Settings(BaseSettings): # допуском на перерыв в работе scraper'а. ENV: AVITO_STALE_TTL_DAYS. avito_stale_ttl_days: int = 10 + # ── ЭТАП 4 B2C launch — retention / erasure (152-ФЗ) ──────────────────── + # trade_in_estimates.expires_at TTL (часы от момента создания). Раньше был + # хардкод `timedelta(hours=24)` в estimator.py (x2: главный INSERT + + # _empty_estimate fallback) — вынесено в настройку, чтобы retention-период + # не требовал правки кода. 24ч — продуктовое решение MVP (оценка живёт + # "сессию" клиента, не архив); юридически обоснованный срок хранения адреса + # физлица для анонимного B2C — решение не инженера, см. итоговый комментарий + # к задаче. ENV: TRADE_IN_ESTIMATE_RETENTION_HOURS. + trade_in_estimate_retention_hours: int = 24 + + # trade_in_leads.expires_at TTL (дни от момента создания, migration 193). + # У trade_in_leads раньше вообще не было срока хранения — лид (телефон + + # согласие) жил в БД бессрочно. 180 дней (6 месяцев) — рабочий default для + # НЕконвертированных маркетинговых лидов (типичный индустриальный диапазон + # 90-180 дней при отсутствии дальнейшего договорного отношения с клиентом); + # если лид конвертировался в реальную сделку/договор — для него должен + # действовать ДРУГОЙ (договорной) срок хранения, но в кодовой базе нет + # механизма отметки "лид конвертирован" — этого разграничения здесь НЕТ, + # см. итоговый комментарий к задаче (конкретный юридически обоснованный + # срок — решение DPO/юриста, не инженера). ENV: TRADE_IN_LEAD_RETENTION_DAYS. + trade_in_lead_retention_days: int = 180 + + # Батч-размер физического DELETE в purge_expired_trade_in_data (нельзя одним + # DELETE по всей таблице — долгая блокировка на большом бэклоге). Задача сама + # крутит цикл батчей за один прогон (см. _DEFAULT_MAX_BATCHES в таске) — + # это ограничивает ОДНУ транзакцию, не общий прогресс. ENV: + # TRADE_IN_PURGE_BATCH_SIZE. + trade_in_purge_batch_size: int = 500 + # ── Avito SERP ЕКБ гео-фильтр (per-card city-slug) ───────────────────── # Avito при редких/дорогих комбо (4+ комн.) добивает выдачу «по всей России» # (Москва/Челябинск/Омск и т.д.). Каждая карточка несёт СВОЙ href с city-slug diff --git a/tradein-mvp/backend/app/main.py b/tradein-mvp/backend/app/main.py index 65fb5098..07d58fa8 100644 --- a/tradein-mvp/backend/app/main.py +++ b/tradein-mvp/backend/app/main.py @@ -28,6 +28,7 @@ from app.api.v1 import ( geocode, lead, me, + privacy_admin, search, support, trade_in, @@ -161,6 +162,7 @@ def health() -> dict[str, str]: app.include_router(geocode.router, prefix="/api/v1/geocode", tags=["geocode"]) app.include_router(admin.router, prefix="/api/v1/admin", tags=["admin"]) app.include_router(audit.router, prefix="/api/v1/admin", tags=["admin-audit"]) +app.include_router(privacy_admin.router, prefix="/api/v1/admin", tags=["admin-privacy"]) app.include_router(brand.router, prefix="/api/v1/brand", tags=["brand"]) app.include_router(trade_in.router, prefix="/api/v1/trade-in", tags=["trade-in"]) app.include_router(lead.router, prefix="/api/v1/trade-in", tags=["trade-in"]) diff --git a/tradein-mvp/backend/app/schemas/trade_in.py b/tradein-mvp/backend/app/schemas/trade_in.py index 69dee79d..7e361bde 100644 --- a/tradein-mvp/backend/app/schemas/trade_in.py +++ b/tradein-mvp/backend/app/schemas/trade_in.py @@ -42,6 +42,15 @@ class TradeInEstimateInput(BaseModel): has_mortgage: bool | None = None # client_name / client_phone удалены (PII purge #1969, DROP COLUMN 167). + # ЭТАП 4 B2C launch — anonymous consent-before-save (152-ФЗ, migration 192). + # Enforcement (НЕ здесь): app.services.estimator.estimate_quality проверяет + # `created_by is None and not consent -> 422` ДО первого INSERT адреса в + # trade_in_estimates. Здесь поле намеренно `bool | None = None`, а НЕ + # `Literal[True]` (как TradeInLeadInput.consent) — сделать True строго- + # обязательным на уровне Pydantic сломало бы B2B-пилотов: их согласие + # закрыто договором, а не UI-чекбоксом, и их фронт НЕ шлёт это поле вовсе. + consent: bool | None = None + class AnalogLot(BaseModel): address: str diff --git a/tradein-mvp/backend/app/services/data_erasure.py b/tradein-mvp/backend/app/services/data_erasure.py new file mode 100644 index 00000000..b2053dae --- /dev/null +++ b/tradein-mvp/backend/app/services/data_erasure.py @@ -0,0 +1,175 @@ +"""Right-to-erasure mechanism (152-ФЗ) — ЭТАП 4 B2C launch, part C. + +WHY: + trade_in has no self-service "delete my data" endpoint at all. Both B2B + pilots (identified by `created_by` username) and future anonymous B2C + users need SOME way to have their personal data physically removed on + request, not just after their retention TTL expires + (app/tasks/purge_expired_trade_in_data.py handles the TTL path, this + module handles the on-demand path). + +WHO CAN BE IDENTIFIED, HONESTLY: + - B2B pilot (has a `username`): trivially -- `created_by = username` scopes + every estimate they created; leads/support threads follow from there. + - Anonymous person: has NO username. This function can ONLY act on + identifiers the requester can actually supply: + * `estimate_ids` -- if they still have the link/PDF from their estimate + (the UUID in the URL/QR-code IS their proof of "this is mine"). + * `phone` -- if they left a contact-request lead with that phone. + * `tg_chat_id` -- if they messaged @MERAsupport_bot directly (their own + Telegram chat id -- not guessable/spoofable by a third party the way + a name or IP would be). + If an anonymous person has NONE of these (e.g. they only remember the + street address, or ran an estimate but never saved anything and didn't + log support contact) -- THIS IS HONESTLY UNRESOLVABLE without additional + identification. There is no username, no stable session, nothing in the + DB schema today that lets a support operator find "the one estimate this + specific stranger made three days ago" among many. Do not paper over + this: an operator facing that case must say so, not silently pick "the + closest match". + +⚠️ TELEGRAM CAVEAT (152-ФЗ, honestly, do not omit): + Every tg_support_messages row was, at send time, ALSO mirrored by the bot + into the support-group Telegram topic (see app/services/tgbot/bridge.py, + 186_tg_support.sql). Deleting `tg_support_users` here only removes the + copy IN THIS DATABASE. The mirrored copy lives in the Telegram supergroup, + outside this function's reach, and is NOT deleted by anything in this + codebase. A complete erasure across the whole chain requires a SEPARATE + manual step (Telegram Bot API `deleteMessage` per `topic_message_id` in + the supergroup) that is out of scope here. Do not cite this function's + return value as proof of full erasure of the Telegram-side copy. + +WHAT ELSE IS *NOT* TOUCHED (known gap, flagged, not silently dropped): + `user_events` (184_user_events.sql) logs `estimate_request` events with a + JSONB payload that includes `address`/`area_m2`/`rooms` and is keyed by + `username` (empty string for anonymous callers today) + `ip_address`, with + NO FK to trade_in_estimates (decoupled/append-only by explicit design -- + see that migration's comment). This function does NOT purge user_events: + it is an audit/analytics log, not an estimate/lead/support record, and + deciding whether "audit trail" is a legitimate 152-ФЗ retention basis that + overrides an erasure request is a legal call, not an engineering one. Flag + it to whoever handles the request; do not assume it is already covered. +""" + +from __future__ import annotations + +import logging +from collections.abc import Sequence +from uuid import UUID + +from sqlalchemy import text +from sqlalchemy.orm import Session + +logger = logging.getLogger(__name__) + + +def erase_person_data( + db: Session, + *, + username: str | None = None, + estimate_ids: Sequence[UUID] | None = None, + phone: str | None = None, + tg_chat_id: int | None = None, +) -> dict[str, int]: + """Physically delete a person's data across trade_in tables. + + At least one identifier is required (raises ValueError otherwise -- callers + MUST pass an explicit identifier, never "erase everything" by omission). + + Order of operations matters: leads are captured/deleted BEFORE estimates, + because trade_in_leads.estimate_id is ON DELETE SET NULL (172) -- once the + estimate row is gone, the join key to find "leads that came from this + person's estimate" is gone too. + + Returns per-table deleted-row counters. Callers own committing the ambient + Session lifecycle in whatever way their layer does (this function DOES + commit itself, mirroring app/tasks/*.py conventions, since this is a + one-shot admin operation, not a request-scoped unit of work shared with + other writes). + """ + if not any([username, estimate_ids, phone, tg_chat_id]): + raise ValueError( + "erase_person_data requires at least one identifier: " + "username / estimate_ids / phone / tg_chat_id" + ) + + counters: dict[str, int] = { + "trade_in_estimates_deleted": 0, + "trade_in_leads_deleted": 0, + "web_support_deleted": 0, + "tg_support_deleted": 0, + } + + # 1. Собрать ПОЛНЫЙ набор estimate_id ДО удаления оценок: явные estimate_ids + # (анонимный путь -- человек прислал ссылку/PDF) + все id с + # created_by=username (B2B-путь). Нужно захватить это СЕЙЧАС -- после + # DELETE FROM trade_in_estimates связанные trade_in_leads.estimate_id + # уйдут в NULL (ON DELETE SET NULL, 172), join станет невозможен. + all_estimate_ids: set[UUID] = set(estimate_ids or []) + if username: + owned = ( + db.execute( + text("SELECT id FROM trade_in_estimates WHERE created_by = :username"), + {"username": username}, + ) + .scalars() + .all() + ) + all_estimate_ids.update(owned) + + # 2. Лиды -- пока estimate_id ещё живой FK (см. п.1), плюс отдельно по + # телефону (лид мог быть оставлен без attach к оценке вовсе). + ids_param = [str(i) for i in all_estimate_ids] + result = db.execute( + text( + """ + DELETE FROM trade_in_leads + WHERE estimate_id = ANY(CAST(:ids AS uuid[])) + OR phone = :phone + """ + ), + {"ids": ids_param, "phone": phone}, + ) + counters["trade_in_leads_deleted"] = result.rowcount or 0 + + # 3. Оценки (CASCADE подчищает estimate_photos + avito_imv_evaluations). + if all_estimate_ids: + result = db.execute( + text("DELETE FROM trade_in_estimates WHERE id = ANY(CAST(:ids AS uuid[]))"), + {"ids": ids_param}, + ) + counters["trade_in_estimates_deleted"] = result.rowcount or 0 + + # 4. Веб-чат поддержки -- ключ username (сайт закрыт Caddy basic_auth, у + # анонима username нет и быть не может, см. 187_web_support_chat.sql). + if username: + result = db.execute( + text("DELETE FROM web_support_threads WHERE username = :username"), + {"username": username}, + ) + counters["web_support_deleted"] = result.rowcount or 0 + + # 5. Telegram-поддержка -- ключ chat_id, ЕДИНСТВЕННЫЙ путь, реально + # доступный анониму без username (см. module docstring). ⚠️ Чистит + # ТОЛЬКО эту БД -- Telegram-топик со своей копией переписки НЕ + # затрагивается, см. ВАЖНЫЙ ФАКТ в docstring выше. + if tg_chat_id is not None: + result = db.execute( + text("DELETE FROM tg_support_users WHERE chat_id = CAST(:chat_id AS bigint)"), + {"chat_id": tg_chat_id}, + ) + counters["tg_support_deleted"] = result.rowcount or 0 + + db.commit() + logger.info( + "erase_person_data: username=%r estimate_ids=%d phone=%s tg_chat_id=%s -> %s", + username, + len(all_estimate_ids), + "" if phone else None, + tg_chat_id, + counters, + ) + return counters + + +__all__: list[str] = ["erase_person_data"] diff --git a/tradein-mvp/backend/app/services/estimator.py b/tradein-mvp/backend/app/services/estimator.py index d533bb6e..07613f48 100644 --- a/tradein-mvp/backend/app/services/estimator.py +++ b/tradein-mvp/backend/app/services/estimator.py @@ -33,6 +33,7 @@ from datetime import UTC, date, datetime, timedelta from typing import Any, Literal from uuid import uuid4 +from fastapi import HTTPException from scraper_kit.providers.avito.imv import ( IMVAddressNotFoundError, IMVAuthError, @@ -92,6 +93,69 @@ MIN_ANALOGS_PER_SOURCE = 5 # гарантированный минимум на LISTINGS_FRESH_DAYS = 14 # объявления не старше 14 дней DEALS_PERIOD_MONTHS = 12 # сделки за последний год +# ── ЭТАП 4 B2C launch — anonymous consent-before-save (152-ФЗ) ──────────────── +# Отдельная пара от _CONSENT_POLICY_VERSION/_CONSENT_TEXT_SNAPSHOT в +# app/api/v1/lead.py: там согласие на КОНТАКТ-заявку (обработка телефона для +# CRM), здесь — согласие на САМУ ОЦЕНКУ (обработка адреса + параметров квартиры +# для расчёта стоимости). Разный предмет обработки -> разный текст/версия, даже +# если оба сейчас датированы одним месяцем. +# +# Enforcement: `require_consent` — явный keyword-only флаг у estimate_quality(), +# НЕ вывод из created_by is None. Причина: estimate_quality() — единственная +# ФУНКЦИЯ, но с ДЕСЯТКАМИ прямых вызовов из тестов эстиматора (test_same_ +# building_anchor.py, test_estimator_quarter_index.py и т.д.), почти все зовут +# её как estimate_quality(payload, db) без created_by вообще — они тестируют +# ценовую логику, не auth. Если бы gate триггерился от created_by is None, +# ЛЮБОЙ такой тест внезапно стал бы "анонимным" и падал на 422 (проверено: +# сломал 92 теста при первой попытке этого PR). require_consent=False по +# умолчанию -> НИ ОДИН существующий вызов не меняет поведение. Единственный +# реальный (production) вызывающий — app/api/v1/trade_in.py::estimate() — +# явно передаёт require_consent=(x_authenticated_user is None), т.е. gate +# реально применяется РОВНО там и тогда, где решение принимает rbac (нет +# X-Authenticated-User => анонимный запрос). +# +# СИНХРОННОСТЬ С ФРОНТОМ: сегодня анонимный флоу ещё не открыт (rbac_guard +# требует X-Authenticated-User на ЛЮБОМ non-public path — см. app/core/rbac.py), +# поэтому у этого текста пока НЕТ живого фронтового чекбокса для сверки (в +# отличие от _CONSENT_TEXT_SNAPSHOT в lead.py, см. +# tests/test_consent_text_frontend_sync.py). Когда анонимный /estimate откроется +# и фронт получит свой чекбокс — добавь сюда симметричный sync-тест ПРЕЖДЕ, чем +# полагаться на комментарий (ровно та ошибка, которую эта задача чинит для лидов). +_ESTIMATE_CONSENT_POLICY_VERSION = "2026-07" +_ESTIMATE_CONSENT_TEXT_SNAPSHOT = ( + "Согласен(-на) на обработку персональных данных (адрес объекта и параметры " + "квартиры) в целях предварительной оценки стоимости в соответствии с " + "Федеральным законом «О персональных данных» № 152-ФЗ" +) + + +def _estimate_consent_persist_fields( + require_consent: bool, client_ip: str | None +) -> dict[str, Any]: + """Поля consent/client_ip/policy/snapshot для INSERT в trade_in_estimates. + + require_consent=True -> здесь мы УЖЕ прошли gate в начале estimate_quality() + (payload.consent is True гарантирован), поэтому пишем durable-доказательство. + require_consent=False (B2B-путь, дефолт для всех прочих вызывающих) -> все + четыре NULL: согласие закрыто договором либо вызывающий вообще не участвует + в consent-контракте (внутренние тесты/скрипты) — доказательство здесь не + собиралось и собираться не должно. + """ + if require_consent: + return { + "consent": True, + "client_ip": client_ip, + "consent_policy_version": _ESTIMATE_CONSENT_POLICY_VERSION, + "consent_text_snapshot": _ESTIMATE_CONSENT_TEXT_SNAPSHOT, + } + return { + "consent": None, + "client_ip": None, + "consent_policy_version": None, + "consent_text_snapshot": None, + } + + # #oblast-D (non-EKB deals-headline-fallback): минимум ДКП-сделок, чтобы # _fetch_dkp_corridor доверял СВОЕЙ street-scoped выборке — иначе (тонкая # конкретная улица небольшого города) виджет расширяется до city-wide (см. @@ -3137,7 +3201,12 @@ def _price_from_inputs( # ── Public ─────────────────────────────────────────────────────────────────── async def estimate_quality( - payload: TradeInEstimateInput, db: Session, created_by: str | None = None + payload: TradeInEstimateInput, + db: Session, + created_by: str | None = None, + client_ip: str | None = None, + *, + require_consent: bool = False, ) -> AggregatedEstimate: """Главная функция — оценка квартиры по реальным данным. @@ -3148,9 +3217,34 @@ async def estimate_quality( AggregatedEstimate с tier classification (T0_per_house / T1_per_street) — frontend может разделять confidence в UI. + Args: + require_consent: ЭТАП 4 B2C consent-before-save gate (keyword-only, + дефолт False — см. _ESTIMATE_CONSENT_* комментарий выше про то, + почему это НЕ выводится из created_by is None). Единственный + реальный вызывающий — app/api/v1/trade_in.py::estimate() — передаёт + True, когда нет X-Authenticated-User (анонимный запрос). + Returns: AggregatedEstimate с estimate_id, медианой, диапазоном, аналогами. + + Raises: + HTTPException(422): require_consent=True без payload.consent=True. + Проверка стоит ПЕРВОЙ строкой тела функции, ДО geocode() и ДО + обоих мест, где адрес попадает в trade_in_estimates (главный + INSERT ниже И _empty_estimate fallback) — адрес физлица не должен + попасть в БД раньше согласия ни при каком исходе оценки. """ + # ЭТАП 4 B2C launch (152-ФЗ) — consent-before-save. require_consent=False + # (дефолт, весь B2B-путь и все внутренние/тестовые вызовы) — эта проверка + # их не касается. require_consent=True (реально анонимный HTTP-запрос, + # см. Args выше) обязан нести явное согласие ПРЕЖДЕ, чем estimate_id будет + # сгенерирован и адрес уйдёт в БД. + if require_consent and not payload.consent: + raise HTTPException( + status_code=422, + detail="consent required for anonymous estimate request", + ) + # 1. Geocode (#654: time-budgeted — Yandex/Nominatim retry chain can stack # multiple network round-trips + 1s Nominatim rate-limit sleeps). geo: GeocodeResult | None = None @@ -3190,7 +3284,13 @@ async def estimate_quality( # Без координат не можем искать через PostGIS. Возвращаем low confidence. logger.warning("geocode failed for %s — returning low-confidence estimate", payload.address) return await asyncio.to_thread( - _empty_estimate, payload, db, reason="address_not_geocoded", created_by=created_by + _empty_estimate, + payload, + db, + reason="address_not_geocoded", + created_by=created_by, + client_ip=client_ip, + require_consent=require_consent, ) # 1b. DaData enrichment (PR Q1) — on-demand cleanup для target адреса. @@ -3609,7 +3709,7 @@ async def estimate_quality( # 6. Сохраняем в trade_in_estimates estimate_id = uuid4() now = datetime.now(tz=UTC) - expires_at = now + timedelta(hours=24) + expires_at = now + timedelta(hours=settings.trade_in_estimate_retention_hours) # #694: когда same-building якорь сработал, headline построен на комплах того # же дома (anchor_comps_used) — показываем ИХ, а не радиусные listings_clean @@ -3682,7 +3782,8 @@ async def estimate_quality( expected_sold_range_high, expected_sold_per_m2, asking_to_sold_ratio, ratio_basis, created_by, - expires_at + expires_at, + consent, client_ip, consent_policy_version, consent_text_snapshot ) VALUES ( CAST(:id AS uuid), :address, :lat, :lon, @@ -3702,7 +3803,9 @@ async def estimate_quality( :expected_sold_range_high, :expected_sold_per_m2, :asking_to_sold_ratio, :ratio_basis, :created_by, - :expires_at + :expires_at, + :consent, CAST(:client_ip AS inet), :consent_policy_version, + :consent_text_snapshot ) """ ), @@ -3754,6 +3857,7 @@ async def estimate_quality( "ratio_basis": ratio_basis, "created_by": created_by, "expires_at": expires_at, + **_estimate_consent_persist_fields(require_consent, client_ip), }, ) @@ -6066,16 +6170,27 @@ def _deal_to_analog(row: dict[str, Any]) -> AnalogLot: def _empty_estimate( - payload: TradeInEstimateInput, db: Session, *, reason: str, created_by: str | None = None + payload: TradeInEstimateInput, + db: Session, + *, + reason: str, + created_by: str | None = None, + client_ip: str | None = None, + require_consent: bool = False, ) -> AggregatedEstimate: """Fallback когда нет данных для оценки. Сохраняет запись в БД (confidence='low', пустые analogs/deals), чтобы GET /estimate/{id} не возвращал 404. C-4 security audit. + + ЭТАП 4 B2C: этот путь тоже пишет адрес в trade_in_estimates -- consent-gate в + estimate_quality() уже отработал ДО вызова (require_consent=True подразумевает + payload.consent is True), здесь просто персистим то же consent-доказательство, + что и главный путь (_estimate_consent_persist_fields). """ estimate_id = uuid4() now = datetime.now(tz=UTC) - expires_at = now + timedelta(hours=24) + expires_at = now + timedelta(hours=settings.trade_in_estimate_retention_hours) db.execute( text( @@ -6090,7 +6205,8 @@ def _empty_estimate( analogs, actual_deals, sources_used, created_by, - expires_at + expires_at, + consent, client_ip, consent_policy_version, consent_text_snapshot ) VALUES ( CAST(:id AS uuid), :address, :area, :rooms, :floor, :total_floors, @@ -6101,7 +6217,9 @@ def _empty_estimate( '[]'::jsonb, '[]'::jsonb, '[]'::jsonb, :created_by, - :expires_at + :expires_at, + :consent, CAST(:client_ip AS inet), :consent_policy_version, + :consent_text_snapshot ) """ ), @@ -6121,6 +6239,7 @@ def _empty_estimate( "explanation": reason, "created_by": created_by, "expires_at": expires_at, + **_estimate_consent_persist_fields(require_consent, client_ip), }, ) db.commit() diff --git a/tradein-mvp/backend/app/services/product_handlers.py b/tradein-mvp/backend/app/services/product_handlers.py index d6c70869..4165571a 100644 --- a/tradein-mvp/backend/app/services/product_handlers.py +++ b/tradein-mvp/backend/app/services/product_handlers.py @@ -348,6 +348,24 @@ async def _job_house_imv_backfill( logger.exception("scheduler: mark_failed crashed run_id=%d", run_id) +# ── purge_expired_trade_in_data — ЭТАП 4 B2C retention (152-ФЗ) ─────────────── +async def _job_purge_expired_trade_in_data( + db: Session, run_id: int, params: dict[str, Any], ctx: SchedulerContext +) -> None: + from app.tasks.purge_expired_trade_in_data import purge_expired_trade_in_data + + batch_size = params.get("batch_size") + max_batches = params.get("max_batches") + + loop = asyncio.get_event_loop() + await loop.run_in_executor( + None, + lambda: purge_expired_trade_in_data( + db, run_id, batch_size=batch_size, max_batches=max_batches + ), + ) + + # ── house_dedup_merge — sync destructive merge в executor, owns lifecycle ───── async def _job_house_dedup_merge( db: Session, run_id: int, params: dict[str, Any], ctx: SchedulerContext @@ -424,6 +442,9 @@ def build_product_handlers(ctx: SchedulerContext) -> dict[str, Handler]: "osm_poi_ekb_refresh": Handler(_job_osm_poi_ekb_refresh, "osm_poi_ekb_refresh"), "house_imv_backfill": Handler(_job_house_imv_backfill, "house_imv_backfill"), "house_dedup_merge": Handler(_job_house_dedup_merge, "house_dedup_merge"), + "purge_expired_trade_in_data": Handler( + _job_purge_expired_trade_in_data, "purge_expired_trade_in_data" + ), "proxy_healthcheck": Handler( _job_proxy_healthcheck, "proxy_healthcheck", diff --git a/tradein-mvp/backend/app/tasks/purge_expired_trade_in_data.py b/tradein-mvp/backend/app/tasks/purge_expired_trade_in_data.py new file mode 100644 index 00000000..17a99508 --- /dev/null +++ b/tradein-mvp/backend/app/tasks/purge_expired_trade_in_data.py @@ -0,0 +1,164 @@ +"""Physically delete expired personal data — ЭТАП 4 B2C retention enforcement (152-ФЗ). + +WHY: + trade_in_estimates.expires_at (и, начиная с migration 193, trade_in_leads.expires_at) + defined a retention window, but neither table had any background job that actually + DELETEd rows once expired -- expires_at was used ONLY as a read-time filter + (GET /estimate/{id}: "AND expires_at > NOW()"). Personal data (address / phone) + outlived its declared lifetime indefinitely, contradicting the retention policy + shown to the user. + +WHAT: + Batched physical DELETE for both tables, run nightly by the kit-scheduler (see + app.services.product_handlers._job_purge_expired_trade_in_data, scrape_schedules + row seeded by migration 193 -- seeded enabled=false, see that migration's docstring + for why). Same architecture as app/tasks/deactivate_stale_avito.py (sync, DB-only, + invoked via run_in_executor from the async kit handler). + + - trade_in_estimates: ON DELETE CASCADE already cleans up estimate_photos + (007_estimate_photos.sql) and avito_imv_evaluations (018_avito_imv_evaluations.sql) + for each deleted estimate. + - trade_in_leads: ON DELETE SET NULL on trade_in_leads.estimate_id (172_trade_in_leads.sql) + means a lead created from a now-purged estimate SURVIVES with estimate_id nulled -- + it has its OWN retention clock (trade_in_leads.expires_at) and its own PII (phone), + purged independently below. + +BATCHING (не единый DELETE по всей таблице): + Each table is drained in batches of `batch_size` rows (default + settings.trade_in_purge_batch_size), each batch its OWN statement + its OWN commit + (bounds lock/transaction duration on a backlog). A run stops draining a table once a + batch returns fewer rows than batch_size (caught up) OR after `max_batches` iterations + (safety cap on total run duration -- any remaining backlog drains over subsequent + nightly runs, not one giant transaction). Idempotent: rows already deleted simply + don't match `expires_at < NOW()` on the next run; a mid-run failure leaves earlier + committed batches deleted (correct, not rolled back) and mark_failed records the + partial counters reached so far. +""" + +from __future__ import annotations + +import logging +from typing import Any + +from sqlalchemy import text +from sqlalchemy.orm import Session + +from app.core.config import settings +from app.services import scrape_runs as runs_mod + +logger = logging.getLogger(__name__) + +# Safety cap on batches per table per run -- bounds a single scheduled run's total +# duration even if the backlog is much larger than batch_size * max_batches; the +# remainder simply drains on the next nightly run (idempotent, no data loss risk). +_DEFAULT_MAX_BATCHES = 20 + +_DELETE_EXPIRED_ESTIMATES_SQL = text( + """ + DELETE FROM trade_in_estimates + WHERE id IN ( + SELECT id FROM trade_in_estimates + WHERE expires_at < NOW() + ORDER BY expires_at + LIMIT CAST(:batch_size AS int) + ) + """ +) + +_DELETE_EXPIRED_LEADS_SQL = text( + """ + DELETE FROM trade_in_leads + WHERE id IN ( + SELECT id FROM trade_in_leads + WHERE expires_at < NOW() + ORDER BY expires_at + LIMIT CAST(:batch_size AS int) + ) + """ +) + + +def _drain_expired( + db: Session, + stmt: Any, + *, + batch_size: int, + max_batches: int, + label: str, + counters: dict[str, int], + counter_key: str, +) -> None: + """Run `stmt` (one bounded DELETE batch) repeatedly until caught up or capped. + + Commits after EVERY batch -- keeps each individual transaction/lock short even + when the backlog is large. Updates `counters[counter_key]` INCREMENTALLY (not + just once at the end) so that a mid-run exception on a LATER batch still leaves + an accurate count of what was actually deleted-and-committed by earlier batches + -- those rows are gone for real (commit already happened) whether or not this + function ever returns normally. + """ + for batch_num in range(1, max_batches + 1): + result = db.execute(stmt, {"batch_size": batch_size}) + deleted = result.rowcount or 0 + db.commit() + counters[counter_key] += deleted + logger.info( + "purge_expired_trade_in_data: %s batch=%d deleted=%d (running_total=%d)", + label, + batch_num, + deleted, + counters[counter_key], + ) + if deleted < batch_size: + break # caught up -- fewer expired rows left than one batch + + +def purge_expired_trade_in_data( + db: Session, + run_id: int, + *, + batch_size: int | None = None, + max_batches: int | None = None, +) -> dict[str, int]: + """Delete expired rows from trade_in_estimates + trade_in_leads, in bounded batches. + + Sync (invoked via run_in_executor from the kit-scheduler handler, same pattern as + deactivate_stale_listings). Finalises the scrape_runs row (mark_done / mark_failed). + + Returns {"estimates_deleted": N, "leads_deleted": M}. + """ + batch_size = batch_size or settings.trade_in_purge_batch_size + max_batches = max_batches or _DEFAULT_MAX_BATCHES + counters: dict[str, int] = {"estimates_deleted": 0, "leads_deleted": 0} + try: + _drain_expired( + db, + _DELETE_EXPIRED_ESTIMATES_SQL, + batch_size=batch_size, + max_batches=max_batches, + label="trade_in_estimates", + counters=counters, + counter_key="estimates_deleted", + ) + _drain_expired( + db, + _DELETE_EXPIRED_LEADS_SQL, + batch_size=batch_size, + max_batches=max_batches, + label="trade_in_leads", + counters=counters, + counter_key="leads_deleted", + ) + runs_mod.mark_done(db, run_id, counters) + logger.info( + "purge_expired_trade_in_data run_id=%d done: estimates_deleted=%d leads_deleted=%d", + run_id, + counters["estimates_deleted"], + counters["leads_deleted"], + ) + return counters + except Exception as exc: + logger.exception("purge_expired_trade_in_data run_id=%d failed", run_id) + db.rollback() + runs_mod.mark_failed(db, run_id, str(exc)[:1000], counters) + raise diff --git a/tradein-mvp/backend/data/sql/192_trade_in_estimates_consent_proof.sql b/tradein-mvp/backend/data/sql/192_trade_in_estimates_consent_proof.sql new file mode 100644 index 00000000..78ff74e5 --- /dev/null +++ b/tradein-mvp/backend/data/sql/192_trade_in_estimates_consent_proof.sql @@ -0,0 +1,93 @@ +-- 192_trade_in_estimates_consent_proof.sql +-- ЭТАП 4 B2C launch — правовая рамка для анонимных пользователей (152-ФЗ). +-- +-- WHY: +-- trade_in_estimates.address (NOT NULL) сохраняется на КАЖДОЙ оценке, но до +-- сих пор в кодовой базе не было ни единой проверки согласия ДО этого +-- сохранения — ни на уровне схемы, ни на уровне API. Для авторизованного +-- B2B-пилота это было терпимо (согласие закрыто договором), но сегодняшняя +-- схема совершенно не готова к анонимному B2C-пользователю "с улицы": для +-- него согласия не существует вообще, ни в БД, ни в договоре. +-- +-- Прецедент уже есть и работает: migration 182 добавила на trade_in_leads +-- durable-доказательство согласия (client_ip / consent_policy_version / +-- consent_text_snapshot) вместо голого boolean + audit-лога. Этот файл +-- переиспользует РОВНО ТУ ЖЕ форму для trade_in_estimates — согласованность +-- схемы для одного и того же понятия (152-ФЗ proof-of-consent) в двух +-- соседних таблицах важнее гипотетической экономии на отдельной events- +-- таблице (см. разбор формы хранения в PR/задаче ЭТАП 4). +-- +-- WHAT: +-- Четыре nullable-колонки на trade_in_estimates: +-- - consent (boolean) — True для анонимных запросов, что +-- прошли consent-gate в +-- estimate_quality() (app/services/ +-- estimator.py). NULL для B2B-пилотов +-- (created_by задан) — их согласие +-- закрыто договором, НЕ UI-чекбоксом, +-- и мы НЕ подделываем доказательство, +-- которое реально не собиралось. +-- - client_ip (inet) — клиентский IP анонимного запроса на +-- момент согласия. +-- - consent_policy_version (text) — снимок _ESTIMATE_CONSENT_POLICY_VERSION +-- (estimator.py) на момент согласия. +-- - consent_text_snapshot (text) — снимок точного текста согласия, +-- показанного пользователю +-- (_ESTIMATE_CONSENT_TEXT_SNAPSHOT). +-- +-- CHECK-констрейнт: consent IS NULL OR consent IS TRUE — на уровне схемы +-- защищает от того, чтобы False-согласие когда-либо попало в БД (сама +-- проверка в estimate_quality() уже не пускает False дальше 422, это +-- defense-in-depth на случай будущего кода, который забудет про gate). +-- +-- Индекс на expires_at — обслуживает будущую retention-задачу +-- purge_expired_trade_in_data (см. migration 193), которая физически +-- удаляет строки, чей expires_at истёк (сегодня expires_at используется +-- ТОЛЬКО как read-time фильтр, см. GET /estimate/{id}: "AND expires_at > +-- NOW()" — без индекса такой batched-DELETE делал бы full scan таблицы +-- на каждый ночной прогон). +-- +-- IDEMPOTENCY / SAFETY: +-- - ADD COLUMN IF NOT EXISTS x4 — безопасный re-run, все nullable, без +-- DEFAULT, без backfill (существующие строки остаются NULL — честное +-- отражение того, что доказательство согласия для них НЕ собиралось, +-- не искусственная порча схемы). +-- - CHECK-констрейнт добавлен через DO-блок с проверкой pg_constraint по +-- имени (Postgres не поддерживает `ADD CONSTRAINT IF NOT EXISTS` +-- напрямую) — паттерн 1:1 из 189_account_estimate_usage_nonnegative.sql. +-- - CREATE INDEX IF NOT EXISTS — безопасный re-run. +-- - Чисто additive: ничего существующего не читается/не переписывается. +-- +-- Dependencies: 001_trade_in_estimates.sql (таблица), +-- 083_trade_in_estimates_created_by.sql (created_by, использован в gate), +-- 182_trade_in_leads_consent_proof.sql (форма-прецедент для trade_in_leads). + +BEGIN; + +ALTER TABLE trade_in_estimates + ADD COLUMN IF NOT EXISTS consent boolean, + ADD COLUMN IF NOT EXISTS client_ip inet, + ADD COLUMN IF NOT EXISTS consent_policy_version text, + ADD COLUMN IF NOT EXISTS consent_text_snapshot text; + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'trade_in_estimates_consent_not_false' + ) THEN + ALTER TABLE trade_in_estimates + ADD CONSTRAINT trade_in_estimates_consent_not_false + CHECK (consent IS NULL OR consent IS TRUE); + END IF; +END $$; + +CREATE INDEX IF NOT EXISTS trade_in_estimates_expires_at_idx + ON trade_in_estimates (expires_at); + +COMMENT ON COLUMN trade_in_estimates.consent IS '152-ФЗ proof-of-consent (ЭТАП 4 B2C): TRUE для анонимных запросов, прошедших consent-gate в estimate_quality(). NULL для B2B-пилотов (created_by задан) — согласие закрыто договором, доказательство здесь не собирается.'; +COMMENT ON COLUMN trade_in_estimates.client_ip IS '152-ФЗ proof-of-consent: клиентский IP анонимного запроса на момент согласия (см. app/api/v1/trade_in.py::estimate, _client_ip). NULL для B2B-пилотов.'; +COMMENT ON COLUMN trade_in_estimates.consent_policy_version IS '152-ФЗ proof-of-consent: снимок _ESTIMATE_CONSENT_POLICY_VERSION (app/services/estimator.py) на момент согласия.'; +COMMENT ON COLUMN trade_in_estimates.consent_text_snapshot IS '152-ФЗ proof-of-consent: снимок текста согласия на ОЦЕНКУ, показанного пользователю (_ESTIMATE_CONSENT_TEXT_SNAPSHOT) — отдельный текст от trade_in_leads.consent_text_snapshot (тот про согласие на контакт-заявку, другой предмет обработки).'; + +COMMIT; diff --git a/tradein-mvp/backend/data/sql/193_trade_in_privacy_retention.sql b/tradein-mvp/backend/data/sql/193_trade_in_privacy_retention.sql new file mode 100644 index 00000000..822a0526 --- /dev/null +++ b/tradein-mvp/backend/data/sql/193_trade_in_privacy_retention.sql @@ -0,0 +1,93 @@ +-- 193_trade_in_privacy_retention.sql +-- ЭТАП 4 B2C launch — retention enforcement (152-ФЗ): срок хранения ДОЛЖЕН +-- приводить к физическому удалению, а не быть декоративным полем. +-- +-- WHY: +-- trade_in_estimates.expires_at существовал (migration 004), но применялся +-- ТОЛЬКО как read-time фильтр (GET /estimate/{id}: "AND expires_at > NOW()") +-- — ни одна фоновая задача не удаляла строки после истечения TTL. Данные +-- физлица (адрес) жили в БД бессрочно вопреки декларированному сроку. +-- trade_in_leads было ещё хуже: там вообще НЕ было понятия TTL/expires_at — +-- телефон + согласие хранились бессрочно с момента создания таблицы +-- (172_trade_in_leads.sql). +-- +-- WHAT: +-- 1. trade_in_leads.expires_at (timestamptz NOT NULL) — backfill для +-- существующих строк = created_at + 180 дней (тот же default, что +-- settings.trade_in_lead_retention_days в app/core/config.py на момент +-- этой миграции; 180 дней = рабочий MVP-default для НЕконвертированных +-- маркетинговых лидов, см. обоснование в config.py — конкретный +-- юридически обоснованный срок хранения это решение DPO/юриста, не +-- инженера). Новые строки получают expires_at на insert-time +-- (app/api/v1/lead.py, тем же паттерном, что trade_in_estimates). +-- 2. Индекс на trade_in_leads.expires_at — для batched-DELETE ниже. +-- 3. scrape_schedules seed: purge_expired_trade_in_data — ночная задача +-- (app/tasks/purge_expired_trade_in_data.py, kit-handler в +-- app/services/product_handlers.py), физически удаляющая ИСТЁКШИЕ +-- строки в trade_in_estimates И trade_in_leads пачками (batch_size из +-- default_params, лимит max_batches за один прогон — см. таск-докстринг). +-- ON DELETE CASCADE (007_estimate_photos, 018_avito_imv_evaluations) и +-- ON DELETE SET NULL (172_trade_in_leads.estimate_id) уже подчищают +-- зависимые таблицы автоматически — этот файл их не трогает. +-- +-- Seeded с enabled=false (тот же осторожный паттерн, что +-- 175_scrape_schedules_seed_domclick_detail_backfill.sql): это ПЕРВАЯ +-- автоматическая задача физического DELETE персональных данных в trade-in — +-- заслуживает supervised первого прогона (смотри логи/counters вручную) +-- перед тем, как доверить её расписанию. Включение — отдельный ручной шаг +-- (UPDATE scrape_schedules SET enabled=true WHERE source= +-- 'purge_expired_trade_in_data'). +-- +-- IDEMPOTENCY / SAFETY: +-- - ADD COLUMN IF NOT EXISTS + UPDATE ... WHERE expires_at IS NULL (no-op на +-- повторном прогоне, все строки уже проставлены) + +-- ALTER COLUMN ... SET NOT NULL (идемпотентно само по себе — Postgres не +-- ошибается на повторной установке уже действующего NOT NULL). +-- - CREATE INDEX IF NOT EXISTS — безопасный re-run. +-- - INSERT ... ON CONFLICT (source) DO NOTHING — безопасный re-run seed'а. +-- +-- Dependencies: 172_trade_in_leads.sql (таблица), 052_scrape_schedules.sql +-- (scrape_schedules), 192_trade_in_estimates_consent_proof.sql (соседняя +-- часть той же ЭТАП 4 инициативы — индекс на trade_in_estimates.expires_at +-- уже создан там). + +BEGIN; + +ALTER TABLE trade_in_leads + ADD COLUMN IF NOT EXISTS expires_at timestamptz; + +UPDATE trade_in_leads +SET expires_at = created_at + interval '180 days' +WHERE expires_at IS NULL; + +ALTER TABLE trade_in_leads + ALTER COLUMN expires_at SET NOT NULL; + +CREATE INDEX IF NOT EXISTS trade_in_leads_expires_at_idx + ON trade_in_leads (expires_at); + +COMMENT ON COLUMN trade_in_leads.expires_at IS 'ЭТАП 4 B2C (152-ФЗ): срок хранения лида. Backfill для legacy-строк = created_at + 180 дней; новые строки считаются на insert-time из settings.trade_in_lead_retention_days (app/api/v1/lead.py). Физическое удаление после истечения — app/tasks/purge_expired_trade_in_data.py.'; + +INSERT INTO scrape_schedules ( + source, + enabled, + window_start_hour, + window_end_hour, + next_run_at, + default_params +) +VALUES +( + 'purge_expired_trade_in_data', + false, + 2, + 4, + ((CURRENT_DATE + INTERVAL '1 day') + make_interval(hours => 2)) AT TIME ZONE 'UTC', + '{"batch_size": 500, "max_batches": 20}'::jsonb +) +ON CONFLICT (source) DO NOTHING; + +COMMENT ON TABLE scrape_schedules IS + 'In-app scheduler config (replaces cron-script setup). Sources: avito_city_sweep, yandex_city_sweep (dormant, #561), cian_history_backfill, rosreestr_dkp_import, listing_source_snapshot (#570), asking_to_sold_ratio_refresh (#648), refresh_search_matview (#769), yandex_address_backfill (#855, EKB pilot), sber_index_pull (#887, monthly), rosreestr_quarter_poll (#888, monthly), cian_city_sweep (dormant, #973), yandex_newbuilding_sweep (dormant, #974), geocode_missing_listings (#1: listings geom backfill, all sources), avito_detail_backfill (#1551: nightly detail-enrichment backfill for legacy avito listings), domclick_detail_backfill (#2000: nightly Layer B detail-enrichment backfill for domklik listings, cookie-injection + QRATOR-aware, disabled by default until smoke-tested), purge_expired_trade_in_data (ЭТАП 4 B2C: nightly batched physical DELETE of expired trade_in_estimates/trade_in_leads rows, disabled by default until a supervised first run).'; + +COMMIT; diff --git a/tradein-mvp/backend/tests/test_consent_text_frontend_sync.py b/tradein-mvp/backend/tests/test_consent_text_frontend_sync.py new file mode 100644 index 00000000..b87af5ae --- /dev/null +++ b/tradein-mvp/backend/tests/test_consent_text_frontend_sync.py @@ -0,0 +1,89 @@ +"""ЭТАП 4 B2C launch — consent-text sync guard (part D). + +WHY: + _CONSENT_TEXT_SNAPSHOT in app/api/v1/lead.py is a durable 152-ФЗ + proof-of-consent: it must be the EXACT text a user actually saw and agreed + to. Before this test, the only thing keeping it in sync with the real + frontend checkbox label (LeadForm.tsx) was a code COMMENT ("Должен + ДОСЛОВНО совпадать с чекбоксом в LeadForm.tsx"). A comment cannot fail CI + -- a frontend copy edit could silently drift from the backend snapshot, + and every future lead's "proof" would then misrepresent what the user + actually saw. + +WHAT: + Extract the actual consent-checkbox label text straight out of + LeadForm.tsx (regex, no JSX parser needed -- there is exactly one + in the file today) and assert it matches _CONSENT_TEXT_SNAPSHOT byte-for- + byte after whitespace normalisation (JSX text nodes wrap across source + lines; the DOM-rendered text collapses that to single spaces). If someone + edits ONE side without the other, this test fails. + + NOTE: the NEW anonymous-estimate consent text (_ESTIMATE_CONSENT_TEXT_SNAPSHOT + in app/services/estimator.py, ЭТАП 4 part A) has NO frontend counterpart yet + -- the anonymous /estimate flow isn't live (rbac_guard still requires + X-Authenticated-User on every non-public path, see app/core/rbac.py). When + that flow ships its own consent checkbox, add a second sync test here + mirroring this one -- do NOT rely on a comment for that pairing either. +""" + +from __future__ import annotations + +import os +import re +from pathlib import Path + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +_FRONTEND_LEAD_FORM = ( + Path(__file__).resolve().parents[2] + / "frontend" + / "src" + / "components" + / "trade-in" + / "v2" + / "LeadForm.tsx" +) + + +def _extract_span_text(tsx_source: str) -> str: + """Pull the text content of the (single) ... in LeadForm.tsx, + whitespace-normalised the same way a browser collapses JSX text-node + whitespace when rendering (multiple lines/indentation -> single spaces). + """ + match = re.search(r"\s*(.*?)\s*", tsx_source, re.DOTALL) + assert match is not None, "no found in LeadForm.tsx -- consent label markup changed" + return re.sub(r"\s+", " ", match.group(1)).strip() + + +def test_frontend_lead_form_exists() -> None: + assert _FRONTEND_LEAD_FORM.is_file(), f"missing frontend file: {_FRONTEND_LEAD_FORM}" + + +def test_backend_consent_snapshot_matches_frontend_checkbox_label() -> None: + """The whole point: this FAILS if lead.py._CONSENT_TEXT_SNAPSHOT and + LeadForm.tsx's checkbox label ever diverge -- no longer just a comment.""" + from app.api.v1.lead import _CONSENT_TEXT_SNAPSHOT + + frontend_text = _extract_span_text(_FRONTEND_LEAD_FORM.read_text(encoding="utf-8")) + backend_text = re.sub(r"\s+", " ", _CONSENT_TEXT_SNAPSHOT).strip() + + assert frontend_text == backend_text, ( + "consent text drift detected between app/api/v1/lead.py._CONSENT_TEXT_SNAPSHOT " + "and frontend/src/components/trade-in/v2/LeadForm.tsx checkbox label -- the " + "152-ФЗ proof-of-consent snapshot no longer matches what users actually see. " + "Bump _CONSENT_POLICY_VERSION and update _CONSENT_TEXT_SNAPSHOT together with " + "any frontend copy change.\n" + f" frontend: {frontend_text!r}\n" + f" backend: {backend_text!r}" + ) + + +def test_extract_span_text_helper_is_whitespace_insensitive() -> None: + """Sanity check on the extraction helper itself, independent of the real file.""" + sample = """ + + Line one + Line two + + """ + assert _extract_span_text(sample) == "Line one Line two" diff --git a/tradein-mvp/backend/tests/test_data_erasure.py b/tradein-mvp/backend/tests/test_data_erasure.py new file mode 100644 index 00000000..ef2ec46e --- /dev/null +++ b/tradein-mvp/backend/tests/test_data_erasure.py @@ -0,0 +1,160 @@ +"""ЭТАП 4 B2C launch — right-to-erasure mechanism (part C). + +Covers app/services/data_erasure.py: + - at least one identifier required (ValueError, no db.execute at all) + - username (B2B pilot): estimates + their leads + web_support_threads deleted + - estimate_ids only (anonymous, has the link/PDF): estimates + linked leads deleted, + web_support/tg_support untouched (no username = nothing to key them by) + - phone only: only leads deleted (no estimate/support action) + - tg_chat_id only: only tg_support deleted (anonymous Telegram-support path) + - ORDER: leads are captured/deleted BEFORE estimates (estimate_id FK is + ON DELETE SET NULL -- deleting estimates first would orphan the join) + - commits once at the end +""" + +from __future__ import annotations + +import os +from types import SimpleNamespace +from typing import Any +from unittest.mock import MagicMock +from uuid import uuid4 + +import pytest + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +from app.services import data_erasure + + +class _Result: + def __init__(self, rowcount: int = 0, scalar_ids: list[Any] | None = None) -> None: + self.rowcount = rowcount + self._scalar_ids = scalar_ids or [] + + def scalars(self) -> SimpleNamespace: + return SimpleNamespace(all=lambda: self._scalar_ids) + + +def _sql_of(call: Any) -> str: + stmt = call.args[0] + return str(getattr(stmt, "text", stmt)) + + +def test_requires_at_least_one_identifier() -> None: + db = MagicMock() + with pytest.raises(ValueError, match="at least one identifier"): + data_erasure.erase_person_data(db) + assert not db.execute.called + assert not db.commit.called + + +def test_erase_by_username_deletes_estimates_leads_and_web_support() -> None: + db = MagicMock() + owned_id = uuid4() + db.execute.side_effect = [ + _Result(scalar_ids=[owned_id]), # SELECT id FROM trade_in_estimates WHERE created_by + _Result(rowcount=2), # DELETE FROM trade_in_leads + _Result(rowcount=1), # DELETE FROM trade_in_estimates + _Result(rowcount=3), # DELETE FROM web_support_threads + ] + + out = data_erasure.erase_person_data(db, username="kopylov") + + assert out == { + "trade_in_estimates_deleted": 1, + "trade_in_leads_deleted": 2, + "web_support_deleted": 3, + "tg_support_deleted": 0, + } + assert db.commit.called + + calls = db.execute.call_args_list + assert "SELECT id FROM trade_in_estimates" in _sql_of(calls[0]) + assert "created_by" in _sql_of(calls[0]) + assert "DELETE FROM trade_in_leads" in _sql_of(calls[1]) + assert "DELETE FROM trade_in_estimates" in _sql_of(calls[2]) + assert "DELETE FROM web_support_threads" in _sql_of(calls[3]) + # estimate_ids captured from the SELECT reach the estimates DELETE. + estimates_delete_params = calls[2].args[1] + assert str(owned_id) in estimates_delete_params["ids"] + + +def test_leads_deleted_before_estimates_order() -> None: + """FK trade_in_leads.estimate_id is ON DELETE SET NULL -- capturing/deleting + leads must happen BEFORE the estimates DELETE, else the join key is gone.""" + db = MagicMock() + owned_id = uuid4() + db.execute.side_effect = [ + _Result(scalar_ids=[owned_id]), + _Result(rowcount=0), + _Result(rowcount=1), + _Result(rowcount=0), + ] + data_erasure.erase_person_data(db, username="kopylov") + calls = db.execute.call_args_list + leads_idx = next(i for i, c in enumerate(calls) if "DELETE FROM trade_in_leads" in _sql_of(c)) + estimates_idx = next( + i for i, c in enumerate(calls) if "DELETE FROM trade_in_estimates" in _sql_of(c) + ) + assert leads_idx < estimates_idx + + +def test_erase_by_estimate_ids_only_no_web_or_tg_support_touched() -> None: + db = MagicMock() + eid = uuid4() + db.execute.side_effect = [ + _Result(rowcount=1), # DELETE FROM trade_in_leads (matches estimate_id) + _Result(rowcount=1), # DELETE FROM trade_in_estimates + ] + + out = data_erasure.erase_person_data(db, estimate_ids=[eid]) + + assert out == { + "trade_in_estimates_deleted": 1, + "trade_in_leads_deleted": 1, + "web_support_deleted": 0, + "tg_support_deleted": 0, + } + assert db.execute.call_count == 2 # no username -> no SELECT, no web_support DELETE + + +def test_erase_by_phone_only_touches_only_leads() -> None: + db = MagicMock() + db.execute.side_effect = [_Result(rowcount=1)] # DELETE FROM trade_in_leads WHERE phone=... + + out = data_erasure.erase_person_data(db, phone="+79123456789") + + assert out == { + "trade_in_estimates_deleted": 0, + "trade_in_leads_deleted": 1, + "web_support_deleted": 0, + "tg_support_deleted": 0, + } + assert db.execute.call_count == 1 + params = db.execute.call_args_list[0].args[1] + assert params["phone"] == "+79123456789" + assert params["ids"] == [] + + +def test_erase_by_tg_chat_id_only_touches_only_tg_support() -> None: + """Anonymous person with NO username, NO estimate link, NO lead phone -- but + they DID message @MERAsupport_bot -- can still be identified by their own + Telegram chat_id (see module docstring: not spoofable by a third party).""" + db = MagicMock() + db.execute.side_effect = [ + _Result(rowcount=0), # DELETE FROM trade_in_leads (no ids, no phone -> matches nothing) + _Result(rowcount=5), # DELETE FROM tg_support_users + ] + + out = data_erasure.erase_person_data(db, tg_chat_id=123456789) + + assert out == { + "trade_in_estimates_deleted": 0, + "trade_in_leads_deleted": 0, + "web_support_deleted": 0, + "tg_support_deleted": 5, + } + calls = db.execute.call_args_list + assert "DELETE FROM tg_support_users" in _sql_of(calls[-1]) + assert calls[-1].args[1]["chat_id"] == 123456789 diff --git a/tradein-mvp/backend/tests/test_estimate_consent_gate.py b/tradein-mvp/backend/tests/test_estimate_consent_gate.py new file mode 100644 index 00000000..28a3d13a --- /dev/null +++ b/tradein-mvp/backend/tests/test_estimate_consent_gate.py @@ -0,0 +1,319 @@ +"""ЭТАП 4 B2C launch — consent-before-save gate on trade_in_estimates (part A). + +Covers: + - require_consent=True + payload.consent missing/False -> HTTPException(422), + raised BEFORE geocode() is ever touched and BEFORE any db.execute call + (gate is literally the first statement in estimate_quality()). + - require_consent=True + payload.consent=True -> proceeds, and the eventual + INSERT carries consent=True / client_ip / policy version / text snapshot. + - require_consent=True + consent=True but geocode fails -> the + _empty_estimate() fallback INSERT *also* carries the same consent proof + (both places an address can reach trade_in_estimates are covered). + - require_consent defaults to False (regression guard): a bare + estimate_quality(payload, db) call -- exactly the shape used by ~90 other + estimator tests that don't care about auth/consent at all -- is completely + unaffected. This is the whole reason the gate keys off an explicit + keyword-only flag instead of `created_by is None`: the first version of + this gate DID key off created_by and broke 92 unrelated tests across the + estimator test suite (every one of them calls estimate_quality(payload, db) + with created_by defaulting to None, which used to mean nothing). + - the real (only) production caller, app/api/v1/trade_in.py::estimate(), + passes require_consent=(x_authenticated_user is None) -- verified via source + inspection, since exercising it live would require a full FastAPI app + fixture (rbac_guard is DB/env-heavy and out of scope for this offline test). + +Style mirrors tests/test_estimator_client_coords.py + test_estimator_event_loop_2207.py +(offline, db=MagicMock(), downstream helpers patched). +""" + +from __future__ import annotations + +import contextlib +import inspect +import os +import re +from pathlib import Path +from typing import Any + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +from unittest.mock import AsyncMock, MagicMock, patch + +import anyio +import pytest +from fastapi import HTTPException + +_SQL_DIR = Path(__file__).resolve().parents[1] / "data" / "sql" +_MIGRATION_192 = _SQL_DIR / "192_trade_in_estimates_consent_proof.sql" + + +def _make_payload(**overrides: Any) -> Any: + from app.schemas.trade_in import TradeInEstimateInput + + base: dict[str, Any] = dict(address="ЕКБ, ул. Тестовая, 1", area_m2=40.0, rooms=1) + base.update(overrides) + return TradeInEstimateInput(**base) + + +def _make_fake_geo() -> Any: + from app.services.geocoder import GeocodeResult + + return GeocodeResult( + lat=56.838, + lon=60.595, + full_address="Свердловская обл., Екатеринбург, ул. Тестовая, 1", + provider="nominatim", + ) + + +def _downstream_patches(geocode_mock: Any) -> tuple[Any, ...]: + """Offline mocks so estimate_quality runs to completion (mirrors 2207 test's set).""" + return ( + patch("app.services.estimator.geocode", new=geocode_mock), + patch("app.services.estimator.dadata_clean_address", new=AsyncMock(return_value=None)), + patch("app.services.estimator.match_house_readonly", return_value=None), + patch("app.services.estimator.get_house_metadata", new=AsyncMock(return_value=None)), + patch("app.services.estimator._fetch_analogs", return_value=([], False, "W")), + patch("app.services.estimator._fetch_deals", return_value=[]), + patch("app.services.estimator._get_or_fetch_imv_cached", new=AsyncMock(return_value=None)), + patch( + "app.services.estimator._get_or_fetch_yandex_valuation_cached", + new=AsyncMock(return_value=None), + ), + patch( + "app.services.estimator.estimate_via_cian_valuation", + new=AsyncMock(return_value=None), + ), + patch("app.services.estimator._get_asking_sold_ratio", return_value=(None, None)), + ) + + +def _find_insert_call(db: MagicMock, table_marker: str) -> dict[str, Any]: + for call in db.execute.call_args_list: + stmt = call.args[0] + sql = str(getattr(stmt, "text", stmt)) + if f"INSERT INTO {table_marker}" in sql: + return call.args[1] + raise AssertionError( + f"no INSERT INTO {table_marker} call captured; calls={db.execute.call_args_list}" + ) + + +# ── require_consent=True, no consent -> 422 BEFORE any work ─────────────────── + + +def test_require_consent_without_payload_consent_raises_422_before_geocode() -> None: + from app.services.estimator import estimate_quality + + db = MagicMock() + payload = _make_payload() # consent defaults to None + + async def _run() -> None: + with patch("app.services.estimator.geocode") as geocode_mock: + with pytest.raises(HTTPException) as exc_info: + await estimate_quality(payload, db, require_consent=True) + assert exc_info.value.status_code == 422 + geocode_mock.assert_not_called() + + anyio.run(_run) + assert not db.execute.called, "gate must precede ANY db write" + + +def test_require_consent_with_payload_consent_false_raises_422() -> None: + from app.services.estimator import estimate_quality + + db = MagicMock() + payload = _make_payload(consent=False) + + async def _run() -> None: + with pytest.raises(HTTPException) as exc_info: + await estimate_quality(payload, db, require_consent=True) + assert exc_info.value.status_code == 422 + + anyio.run(_run) + assert not db.execute.called + + +# ── require_consent=True, consent given -> proceeds, proof persisted ────────── + + +def test_require_consent_with_payload_consent_persists_proof() -> None: + from app.services.estimator import _ESTIMATE_CONSENT_POLICY_VERSION as POLICY_VERSION + from app.services.estimator import _ESTIMATE_CONSENT_TEXT_SNAPSHOT as TEXT_SNAPSHOT + from app.services.estimator import estimate_quality + + db = MagicMock() + payload = _make_payload(consent=True) + geocode_mock = AsyncMock(return_value=_make_fake_geo()) + + async def _run() -> Any: + with contextlib.ExitStack() as stack: + for cm in _downstream_patches(geocode_mock): + stack.enter_context(cm) + return await estimate_quality( + payload, db, client_ip="203.0.113.9", require_consent=True + ) + + result = anyio.run(_run) + assert result.estimate_id is not None + + params = _find_insert_call(db, "trade_in_estimates") + assert params["consent"] is True + assert params["client_ip"] == "203.0.113.9" + assert params["consent_policy_version"] == POLICY_VERSION + assert params["consent_text_snapshot"] == TEXT_SNAPSHOT + assert params["created_by"] is None + + +# ── require_consent=True, consent given, geocode fails -> _empty_estimate too ─ + + +def test_empty_estimate_fallback_persists_proof_when_consent_required() -> None: + from app.services.estimator import _ESTIMATE_CONSENT_POLICY_VERSION as POLICY_VERSION + from app.services.estimator import _ESTIMATE_CONSENT_TEXT_SNAPSHOT as TEXT_SNAPSHOT + from app.services.estimator import estimate_quality + + db = MagicMock() + payload = _make_payload(consent=True) + geocode_mock = AsyncMock(return_value=None) # geocode failure -> _empty_estimate path + + async def _run() -> Any: + with patch("app.services.estimator.geocode", new=geocode_mock): + return await estimate_quality( + payload, db, client_ip="198.51.100.4", require_consent=True + ) + + result = anyio.run(_run) + assert result.n_analogs == 0 + + params = _find_insert_call(db, "trade_in_estimates") + assert params["consent"] is True + assert params["client_ip"] == "198.51.100.4" + assert params["consent_policy_version"] == POLICY_VERSION + assert params["consent_text_snapshot"] == TEXT_SNAPSHOT + + +def test_empty_estimate_fallback_gate_still_blocks_without_consent() -> None: + """Gate precedes _empty_estimate too -- geocode is never even reached.""" + from app.services.estimator import estimate_quality + + db = MagicMock() + payload = _make_payload() # no consent + + async def _run() -> None: + with patch("app.services.estimator.geocode") as geocode_mock: + with pytest.raises(HTTPException) as exc_info: + await estimate_quality(payload, db, require_consent=True) + assert exc_info.value.status_code == 422 + geocode_mock.assert_not_called() + + anyio.run(_run) + + +# ── require_consent defaults False -> zero blast radius on existing callers ─── + + +def test_require_consent_defaults_false() -> None: + from app.services.estimator import estimate_quality + + sig = inspect.signature(estimate_quality) + assert sig.parameters["require_consent"].default is False + assert sig.parameters["require_consent"].kind == inspect.Parameter.KEYWORD_ONLY + + +def test_bare_call_without_require_consent_is_unaffected() -> None: + """The exact call shape used by ~90 other estimator tests + (estimate_quality(payload, db), no created_by/require_consent at all) -- + must keep working with NO consent field on the payload whatsoever.""" + from app.services.estimator import estimate_quality + + db = MagicMock() + payload = _make_payload() # no consent field + geocode_mock = AsyncMock(return_value=_make_fake_geo()) + + async def _run() -> Any: + with contextlib.ExitStack() as stack: + for cm in _downstream_patches(geocode_mock): + stack.enter_context(cm) + return await estimate_quality(payload, db) + + result = anyio.run(_run) + assert result.estimate_id is not None + + params = _find_insert_call(db, "trade_in_estimates") + assert params["consent"] is None + assert params["client_ip"] is None + assert params["consent_policy_version"] is None + assert params["consent_text_snapshot"] is None + + +# ── B2B pilot (created_by set, require_consent left False) -> unaffected ────── + + +def test_b2b_pilot_without_consent_field_unaffected() -> None: + from app.services.estimator import estimate_quality + + db = MagicMock() + payload = _make_payload() # no consent field at all -- mirrors real pilot frontend + geocode_mock = AsyncMock(return_value=_make_fake_geo()) + + async def _run() -> Any: + with contextlib.ExitStack() as stack: + for cm in _downstream_patches(geocode_mock): + stack.enter_context(cm) + return await estimate_quality(payload, db, created_by="kopylov") + + result = anyio.run(_run) + assert result.estimate_id is not None + + params = _find_insert_call(db, "trade_in_estimates") + assert params["created_by"] == "kopylov" + assert params["consent"] is None + assert params["client_ip"] is None + assert params["consent_policy_version"] is None + assert params["consent_text_snapshot"] is None + + +# ── production wiring: app/api/v1/trade_in.py passes require_consent correctly ─ + + +def test_api_handler_wires_require_consent_from_auth_header() -> None: + """Source-inspection guard: the ONLY production caller of estimate_quality + must derive require_consent from the ABSENCE of X-Authenticated-User, not + hardcode True/False. Cheaper and more robust than spinning up a full app + + rbac_guard fixture just to exercise one kwarg's wiring.""" + from app.api.v1 import trade_in as trade_in_module + + src = re.sub(r"\s+", " ", inspect.getsource(trade_in_module.estimate)) + assert "require_consent=x_authenticated_user is None" in src + + +# ── Migration 192 sanity ──────────────────────────────────────────────────────── + + +def test_migration_192_exists() -> None: + assert _MIGRATION_192.is_file(), f"missing migration: {_MIGRATION_192}" + + +def test_migration_192_is_transactional() -> None: + sql = _MIGRATION_192.read_text("utf-8") + assert "BEGIN;" in sql + assert "COMMIT;" in sql + + +def test_migration_192_is_idempotent() -> None: + sql = _MIGRATION_192.read_text("utf-8") + assert "ADD COLUMN IF NOT EXISTS consent" in sql + assert "ADD COLUMN IF NOT EXISTS client_ip" in sql + assert "CREATE INDEX IF NOT EXISTS" in sql + assert "pg_constraint" in sql # DO-block guard, not bare ADD CONSTRAINT + + +def test_migration_192_no_psycopg_trap() -> None: + sql = _MIGRATION_192.read_text("utf-8") + assert not re.search(r":\w+::", sql) + + +def test_migration_192_check_constraint_allows_null_or_true() -> None: + sql = _MIGRATION_192.read_text("utf-8") + assert "consent IS NULL OR consent IS TRUE" in sql diff --git a/tradein-mvp/backend/tests/test_purge_expired_trade_in_data.py b/tradein-mvp/backend/tests/test_purge_expired_trade_in_data.py new file mode 100644 index 00000000..e32b4570 --- /dev/null +++ b/tradein-mvp/backend/tests/test_purge_expired_trade_in_data.py @@ -0,0 +1,232 @@ +"""ЭТАП 4 B2C launch — retention -> physical deletion task (part B). + +Covers app/tasks/purge_expired_trade_in_data.py: + - batched DELETE (not a single unbounded DELETE), each batch its own commit + - stops draining a table once a batch returns fewer rows than batch_size + - safety cap (max_batches) bounds a single run even on a huge backlog + - both tables (trade_in_estimates, trade_in_leads) get drained + - failure path: rollback + mark_failed with partial counters, exception re-raised + - SQL shape: DELETE (not UPDATE/deactivate), no psycopg `::` cast trap + +Style mirrors tests/test_deactivate_stale_listings.py (_FakeDB, monkeypatched +runs_mod.mark_done/mark_failed). +""" + +from __future__ import annotations + +import os +import re +from pathlib import Path +from typing import Any + +import pytest + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +from app.tasks import purge_expired_trade_in_data as task_mod + +_SQL_DIR = Path(__file__).resolve().parents[1] / "data" / "sql" +_MIGRATION_193 = _SQL_DIR / "193_trade_in_privacy_retention.sql" + + +class _FakeResult: + def __init__(self, rowcount: int) -> None: + self.rowcount = rowcount + + +class _FakeDB: + """Pops rowcounts in call order -- caller supplies the exact sequence expected.""" + + def __init__(self, rowcounts: list[int]) -> None: + self._rowcounts = list(rowcounts) + self.executed: list[tuple[Any, Any]] = [] + self.commits = 0 + self.rolled_back = False + + def execute(self, stmt: Any, params: dict[str, Any] | None = None) -> _FakeResult: + self.executed.append((stmt, params)) + return _FakeResult(self._rowcounts.pop(0)) + + def commit(self) -> None: + self.commits += 1 + + def rollback(self) -> None: + self.rolled_back = True + + +def _patch_runs(monkeypatch: pytest.MonkeyPatch) -> dict[str, Any]: + marked: dict[str, Any] = {} + monkeypatch.setattr( + task_mod.runs_mod, + "mark_done", + lambda _db, run_id, counters: marked.update( + kind="done", run_id=run_id, counters=dict(counters) + ), + ) + monkeypatch.setattr( + task_mod.runs_mod, + "mark_failed", + lambda _db, run_id, err, counters: marked.update( + kind="failed", run_id=run_id, err=err, counters=dict(counters) + ), + ) + return marked + + +# ── batching behaviour ────────────────────────────────────────────────────── + + +def test_stops_when_batch_below_size(monkeypatch: pytest.MonkeyPatch) -> None: + marked = _patch_runs(monkeypatch) + # estimates: one batch of 3 (< batch_size=10) -> stop. leads: one batch of 0 -> stop. + db = _FakeDB([3, 0]) + out = task_mod.purge_expired_trade_in_data(db, run_id=1, batch_size=10, max_batches=20) # type: ignore[arg-type] + assert out == {"estimates_deleted": 3, "leads_deleted": 0} + assert len(db.executed) == 2 + assert db.commits == 2 + assert marked["counters"] == out + + +def test_loops_until_below_batch_size(monkeypatch: pytest.MonkeyPatch) -> None: + _patch_runs(monkeypatch) + # estimates: 5,5,2 (batch_size=5) -> 12 total, 3 batches. leads: 5,1 -> 6 total, 2 batches. + db = _FakeDB([5, 5, 2, 5, 1]) + out = task_mod.purge_expired_trade_in_data(db, run_id=2, batch_size=5, max_batches=20) # type: ignore[arg-type] + assert out == {"estimates_deleted": 12, "leads_deleted": 6} + assert len(db.executed) == 5 + assert db.commits == 5, "each batch must commit independently, not one final commit" + + +def test_respects_max_batches_cap(monkeypatch: pytest.MonkeyPatch) -> None: + """Every batch returns a FULL batch_size (never 'caught up') -- only the + max_batches safety cap can stop the loop. Proves the cap is enforced, not + just coincidentally matching a 'caught up' condition.""" + _patch_runs(monkeypatch) + db = _FakeDB([5, 5, 5, 5, 5, 5]) # exactly max_batches=3 per table, no more + out = task_mod.purge_expired_trade_in_data(db, run_id=3, batch_size=5, max_batches=3) # type: ignore[arg-type] + assert out == {"estimates_deleted": 15, "leads_deleted": 15} + assert len(db.executed) == 6 # 3 (estimates) + 3 (leads), NOT unbounded + + +def test_default_batch_size_and_max_batches_from_settings(monkeypatch: pytest.MonkeyPatch) -> None: + _patch_runs(monkeypatch) + db = _FakeDB([0, 0]) # first batch already empty on both tables -> stop immediately + task_mod.purge_expired_trade_in_data(db, run_id=4) # type: ignore[arg-type] + _stmt, params = db.executed[0] + assert params is not None + assert params["batch_size"] == task_mod.settings.trade_in_purge_batch_size + + +# ── table coverage / SQL shape ────────────────────────────────────────────── + + +def test_drains_both_tables_in_order(monkeypatch: pytest.MonkeyPatch) -> None: + _patch_runs(monkeypatch) + db = _FakeDB([0, 0]) + task_mod.purge_expired_trade_in_data(db, run_id=5, batch_size=100, max_batches=1) # type: ignore[arg-type] + first_sql = str(getattr(db.executed[0][0], "text", db.executed[0][0])) + second_sql = str(getattr(db.executed[1][0], "text", db.executed[1][0])) + assert "trade_in_estimates" in first_sql + assert "trade_in_leads" in second_sql + + +def test_estimates_sql_is_delete_not_update() -> None: + sql = task_mod._DELETE_EXPIRED_ESTIMATES_SQL.text + assert "DELETE FROM trade_in_estimates" in sql + assert "UPDATE" not in sql.upper() + assert "expires_at < NOW()" in sql + assert "ORDER BY expires_at" in sql + assert "LIMIT CAST(:batch_size AS int)" in sql + assert not re.search(r":\w+::", sql) + + +def test_leads_sql_is_delete_not_update() -> None: + sql = task_mod._DELETE_EXPIRED_LEADS_SQL.text + assert "DELETE FROM trade_in_leads" in sql + assert "UPDATE" not in sql.upper() + assert "expires_at < NOW()" in sql + assert "ORDER BY expires_at" in sql + assert not re.search(r":\w+::", sql) + + +def test_sql_does_not_delete_whole_table_unbounded() -> None: + """Neither statement is a bare `DELETE FROM table` -- both scope via a + subselect + LIMIT batch.""" + statements = ( + task_mod._DELETE_EXPIRED_ESTIMATES_SQL.text, + task_mod._DELETE_EXPIRED_LEADS_SQL.text, + ) + for sql in statements: + assert "WHERE id IN (" in sql + assert "LIMIT" in sql + + +# ── failure path ───────────────────────────────────────────────────────────── + + +def test_failure_path_rollback_and_mark_failed(monkeypatch: pytest.MonkeyPatch) -> None: + marked = _patch_runs(monkeypatch) + + class _BoomDB(_FakeDB): + def execute(self, stmt: Any, params: dict[str, Any] | None = None) -> _FakeResult: + # First batch (estimates) succeeds and commits; second call (still + # draining estimates, or first leads call) explodes. + if len(self.executed) >= 1: + raise RuntimeError("db exploded") + return super().execute(stmt, params) + + db = _BoomDB([5]) # only ONE successful batch before the boom + with pytest.raises(RuntimeError, match="db exploded"): + task_mod.purge_expired_trade_in_data(db, run_id=6, batch_size=5, max_batches=20) # type: ignore[arg-type] + + assert db.rolled_back is True + assert marked["kind"] == "failed" + assert marked["run_id"] == 6 + # Partial progress preserved in the counters passed to mark_failed (first + # estimates batch of 5 already committed before the boom on the 2nd call). + assert marked["counters"]["estimates_deleted"] == 5 + assert marked["counters"]["leads_deleted"] == 0 + + +def test_idempotent_zero_rowcount_is_not_an_error(monkeypatch: pytest.MonkeyPatch) -> None: + """Re-running against an already-drained backlog just deletes 0 rows, cleanly.""" + marked = _patch_runs(monkeypatch) + db = _FakeDB([0, 0]) + out = task_mod.purge_expired_trade_in_data(db, run_id=7, batch_size=500, max_batches=20) # type: ignore[arg-type] + assert out == {"estimates_deleted": 0, "leads_deleted": 0} + assert marked["kind"] == "done" + + +# ── migration 193 ──────────────────────────────────────────────────────────── + + +def test_migration_193_exists() -> None: + assert _MIGRATION_193.is_file(), f"missing migration: {_MIGRATION_193}" + + +def test_migration_193_is_transactional() -> None: + sql = _MIGRATION_193.read_text("utf-8") + assert "BEGIN;" in sql + assert "COMMIT;" in sql + + +def test_migration_193_backfills_and_sets_not_null() -> None: + sql = _MIGRATION_193.read_text("utf-8") + assert "ADD COLUMN IF NOT EXISTS expires_at" in sql + assert "WHERE expires_at IS NULL" in sql + assert "SET NOT NULL" in sql + assert "180 days" in sql + + +def test_migration_193_seeds_purge_schedule_disabled_by_default() -> None: + sql = _MIGRATION_193.read_text("utf-8") + assert "'purge_expired_trade_in_data'" in sql + assert "ON CONFLICT (source) DO NOTHING" in sql + # Seeded disabled -- first automated PII-DELETE job in trade-in deserves a + # supervised first run before the scheduler can trigger it unattended. + assert re.search(r"'purge_expired_trade_in_data',\s*\n\s*false,", sql) + + +def test_migration_193_no_psycopg_trap() -> None: + sql = _MIGRATION_193.read_text("utf-8") + assert not re.search(r":\w+::", sql)