Merge remote-tracking branch 'forgejo/main' into feat/tradein-praktika-unlimited
All checks were successful
CI / changes (pull_request) Successful in 11s
CI Trade-In / changes (pull_request) Successful in 11s
CI Trade-In / frontend-checks (pull_request) Has been skipped
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 / backend-tests (pull_request) Successful in 1m5s
All checks were successful
CI / changes (pull_request) Successful in 11s
CI Trade-In / changes (pull_request) Successful in 11s
CI Trade-In / frontend-checks (pull_request) Has been skipped
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 / backend-tests (pull_request) Successful in 1m5s
This commit is contained in:
commit
466928b0b5
27 changed files with 2103 additions and 280 deletions
|
|
@ -592,6 +592,97 @@ jobs:
|
|||
fi
|
||||
echo "→ backend healthy на /health."
|
||||
|
||||
# Frontend health check — раньше проверялся ТОЛЬКО backend: сломанный
|
||||
# фронт (500/белый экран после build, или контейнер упавший на старте)
|
||||
# помечался успешным деплоем, отката не происходило (см. заголовок
|
||||
# секции выше). Проверяем изнутри backend-контейнера — он в одной
|
||||
# tradein-net сети с frontend, и curl там уже есть (в отличие от
|
||||
# node:alpine рантайм-образа frontend, где нет ни curl, ни wget —
|
||||
# добавлять их туда ради healthcheck не стали, backend достаточно).
|
||||
# Путь ОБЯЗАН включать /trade-in: basePath запечён в prod-образ на
|
||||
# build (NEXT_PUBLIC_BASE_PATH=/trade-in, см. build-frontend job) —
|
||||
# голый "/" внутри Next вернёт 404, а не что-то живое. "/trade-in/"
|
||||
# редиректит (307) на /trade-in/v2 — curl -f не считает 3xx ошибкой,
|
||||
# так что это чистая liveness-проверка (процесс жив и роутит),
|
||||
# без привязки к тому, что именно сейчас показывает витрина.
|
||||
frontend_healthy=""
|
||||
for i in $(seq 1 30); do
|
||||
if docker compose -p gendesign-tradein -f /opt/gendesign/tradein-mvp/docker-compose.prod.yml \
|
||||
exec -T backend curl -fsS http://frontend:3000/trade-in/ >/dev/null 2>&1; then
|
||||
frontend_healthy="yes"; break
|
||||
fi
|
||||
sleep 1
|
||||
done
|
||||
if [ -z "$frontend_healthy" ]; then
|
||||
echo "ERROR: frontend не ответил на /trade-in/ за 30s — деплой FAILED"
|
||||
exit 1
|
||||
fi
|
||||
echo "→ frontend healthy на /trade-in/."
|
||||
|
||||
# Browser health check — /health в browser/server.py всегда 200, пока
|
||||
# жив сам aiohttp-процесс (см. health_handler: "compose НЕ имеет
|
||||
# healthcheck на browser, только depends_on: service_started" — до
|
||||
# этой правки browser вообще не проверялся никаким деплой-шагом).
|
||||
# Это liveness процесса, НЕ readiness camoufox-инстансов конкретных
|
||||
# источников (те поднимаются лениво на первый /fetch) — но упавший
|
||||
# при старте контейнер (например, битый образ) здесь ловится сразу,
|
||||
# а не молча остаётся мёртвым до первого реального /fetch scraper'ом.
|
||||
browser_healthy=""
|
||||
for i in $(seq 1 30); do
|
||||
if docker compose -p gendesign-tradein -f /opt/gendesign/tradein-mvp/docker-compose.prod.yml \
|
||||
exec -T backend curl -fsS http://browser:3000/health >/dev/null 2>&1; then
|
||||
browser_healthy="yes"; break
|
||||
fi
|
||||
sleep 1
|
||||
done
|
||||
if [ -z "$browser_healthy" ]; then
|
||||
echo "ERROR: browser не ответил на /health за 30s — деплой FAILED"
|
||||
exit 1
|
||||
fi
|
||||
echo "→ browser healthy на /health."
|
||||
|
||||
# tgbot/scraper — те же backend-образ и Dockerfile, но bare python-
|
||||
# процессы БЕЗ ASGI/HTTP-сервера (см. комментарии в tgbot_main.py /
|
||||
# scheduler_main.py: "здесь нет ASGI-приложения"), поэтому HTTP-
|
||||
# healthcheck для них невозможен в принципе. Liveness проверяем по
|
||||
# состоянию контейнера через docker inspect: упавший на старте
|
||||
# процесс (например, ImportError в новом коде) restart-policy
|
||||
# unless-stopped уводит в бесконечный crash-loop — раньше это НИКАК
|
||||
# не блокировало деплой (маркер писался, даже если tgbot/scraper
|
||||
# были мертвы). Двойная проверка (running → пауза → снова running)
|
||||
# снижает шанс поймать контейнер ровно в момент between-restarts
|
||||
# промежуточного "running" внутри crash-loop.
|
||||
# tgbot пересоздаётся на КАЖДОМ деплое (безусловно в $SERVICES);
|
||||
# scraper — только когда SCRAPER_CHANGED (см. блок выше) — поэтому
|
||||
# проверяем только то, что реально входит в текущий $SERVICES.
|
||||
for svc in tgbot scraper; do
|
||||
case " $SERVICES " in
|
||||
*" $svc "*) ;;
|
||||
*) continue ;;
|
||||
esac
|
||||
container_ok=""
|
||||
state="unknown"
|
||||
for i in $(seq 1 15); do
|
||||
state=$(docker inspect -f '{{.State.Status}}' "tradein-$svc" 2>/dev/null || echo "unknown")
|
||||
if [ "$state" = "running" ]; then
|
||||
container_ok="yes"; break
|
||||
fi
|
||||
sleep 1
|
||||
done
|
||||
if [ -n "$container_ok" ]; then
|
||||
sleep 3
|
||||
state=$(docker inspect -f '{{.State.Status}}' "tradein-$svc" 2>/dev/null || echo "unknown")
|
||||
if [ "$state" != "running" ]; then
|
||||
container_ok=""
|
||||
fi
|
||||
fi
|
||||
if [ -z "$container_ok" ]; then
|
||||
echo "ERROR: tradein-$svc не в стабильном состоянии running (state='$state') — деплой FAILED"
|
||||
exit 1
|
||||
fi
|
||||
echo "→ tradein-$svc running."
|
||||
done
|
||||
|
||||
# Cleanup старых образов
|
||||
for repo in ghcr.io/lekss361/gendesign-tradein-backend \
|
||||
ghcr.io/lekss361/gendesign-tradein-frontend; do
|
||||
|
|
|
|||
|
|
@ -2394,9 +2394,14 @@ async def rotate_proxy_ip(
|
|||
data = resp.json()
|
||||
except Exception:
|
||||
data = {}
|
||||
except Exception as exc:
|
||||
except Exception:
|
||||
# НЕ отдавать str(exc) клиенту (аудит-фикс, #security-audit): httpx-исключения
|
||||
# несут полный request URL, а rotate_url — mobileproxy changeip-ссылка с API-
|
||||
# ключом провайдера в query-string (?...&proxy_key=...). str(exc) с этим URL в
|
||||
# HTTP-ответе — прямая утечка секрета вызывающему клиенту. Причина сбоя остаётся
|
||||
# в логах (exc_info=True) для диагностики; наружу — только нейтральный reason.
|
||||
logger.warning("rotate-ip: changeip failed source=%s", source, exc_info=True)
|
||||
return RotateIpResponse(ok=False, reason=f"changeip error: {exc}")
|
||||
return RotateIpResponse(ok=False, reason="changeip request failed")
|
||||
|
||||
# changeip отдаёт новый IP в одном из полей (формат провайдер-зависимый).
|
||||
new_ip = None
|
||||
|
|
|
|||
|
|
@ -5,6 +5,16 @@ POST /api/v1/trade-in/lead — контактная заявка с резуль
|
|||
trade_in_leads. Notification (Telegram/email) — вне scope: нет существующей
|
||||
SMTP/Telegram интеграции в коде (подтверждено при разборе issue), только
|
||||
persist + log; `notified_at` в таблице зарезервирован под будущую доставку.
|
||||
|
||||
IDOR-фикс (security-audit): `estimate_id` раньше только проверялся на
|
||||
СУЩЕСТВОВАНИЕ (`SELECT 1 ... WHERE id = ...`), без проверки владельца — любой
|
||||
аутентифицированный пилот мог привязать свою заявку к чужой оценке (утечка через
|
||||
последующий просмотр лида: чужой адрес/телефон/оценка в заявке, которую видит не
|
||||
её владелец). Гвард переиспользует `_assert_estimate_access` из
|
||||
`app.api.v1.trade_in` — тот же owner-or-admin подход, что и `GET /estimate/{id}`
|
||||
(#690, `tests/test_estimate_idor.py`): 401 без `X-Authenticated-User`, 403 —
|
||||
неизвестная роль, 404 — оценка не найдена ИЛИ принадлежит не этому пользователю
|
||||
(существование чужой оценки не подтверждаем).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -14,11 +24,12 @@ import re
|
|||
from typing import Annotated, Any, Literal
|
||||
from uuid import UUID
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, Request
|
||||
from fastapi import APIRouter, Depends, Header, HTTPException, Request
|
||||
from pydantic import BaseModel, Field, field_validator
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.api.v1.trade_in import _assert_estimate_access
|
||||
from app.core.db import get_db
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -75,15 +86,20 @@ async def create_trade_in_lead(
|
|||
payload: TradeInLeadInput,
|
||||
request: Request,
|
||||
db: Annotated[Session, Depends(get_db)],
|
||||
x_authenticated_user: Annotated[str | None, Header(alias="X-Authenticated-User")] = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Сохраняет лид (телефон + согласие) в trade_in_leads."""
|
||||
if payload.estimate_id is not None:
|
||||
exists = db.execute(
|
||||
text("SELECT 1 FROM trade_in_estimates WHERE id = CAST(:id AS uuid)"),
|
||||
estimate_row = db.execute(
|
||||
text("SELECT created_by FROM trade_in_estimates WHERE id = CAST(:id AS uuid)"),
|
||||
{"id": str(payload.estimate_id)},
|
||||
).fetchone()
|
||||
if exists is None:
|
||||
if estimate_row is None:
|
||||
raise HTTPException(status_code=404, detail="estimate not found")
|
||||
# IDOR guard (security-audit, зеркалит #690): нельзя привязать лид к
|
||||
# чужой оценке. 404 и на "не найдено", и на "чужая" — не подтверждаем
|
||||
# существование чужого estimate_id.
|
||||
_assert_estimate_access(estimate_row.created_by, x_authenticated_user)
|
||||
|
||||
user_agent = request.headers.get("user-agent")
|
||||
# 152-ФЗ audit trail: реальный клиентский IP из X-Forwarded-For (его ставит
|
||||
|
|
|
|||
136
tradein-mvp/backend/app/core/rbac.py
Normal file
136
tradein-mvp/backend/app/core/rbac.py
Normal file
|
|
@ -0,0 +1,136 @@
|
|||
"""RBAC guard middleware — extracted from ``app/main.py``.
|
||||
|
||||
Historically ``rbac_guard`` lived inline in ``app/main.py`` and the test suite
|
||||
(``tests/test_rbac.py``, ``tests/test_internal_auth_secret.py``) kept a
|
||||
hand-maintained *copy* of it, labelled "MIRROR of app.main — keep in sync
|
||||
manually". The copy drifted: it was missing the #2213
|
||||
``X-Internal-Auth-Secret`` defense-in-depth check that the real guard has,
|
||||
so a regression in that check would NOT have failed CI.
|
||||
|
||||
This module holds the real guard with no DB/lifespan/scheduler side effects
|
||||
(only ``app.core.auth`` + ``app.core.config``, both side-effect-free at
|
||||
import time beyond requiring ``DATABASE_URL`` in the environment for
|
||||
``Settings()``). ``app/main.py`` and the test apps both import THIS module,
|
||||
so tests exercise the exact production code path instead of a copy that can
|
||||
silently fall out of sync.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import re
|
||||
import secrets
|
||||
from collections.abc import Awaitable, Callable
|
||||
|
||||
from fastapi import Request
|
||||
from fastapi.responses import JSONResponse, Response
|
||||
|
||||
from app.core.auth import get_role, is_path_allowed
|
||||
from app.core.config import settings
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# RBAC: defense-in-depth поверх Caddy basic_auth + X-Authenticated-User
|
||||
# (см. app/core/auth.py + auth/roles.yaml). Правила:
|
||||
# 1) Любой non-public path требует X-Authenticated-User — иначе 401.
|
||||
# 2) Юзер должен быть в roles.yaml — иначе 403 («неизвестный юзер ничего
|
||||
# не видит» — decided 2026-05-25).
|
||||
# 3) /api/v1/admin/* (= внешний /trade-in/api/v1/admin/* после Caddy
|
||||
# `uri strip_prefix /trade-in`) — только role=admin, иначе 403.
|
||||
# Public paths без auth (/health, /docs, /openapi.json) пропускаем —
|
||||
# X-Authenticated-User там не приходит из Caddy.
|
||||
_ADMIN_API_RE = re.compile(r"^/api/v1/admin/")
|
||||
_PUBLIC_PATHS = frozenset({"/health", "/docs", "/redoc", "/openapi.json"})
|
||||
# #R2-H3: Caddy срезает внешний префикс /trade-in (uri strip_prefix) перед
|
||||
# tradein-backend, а globs в roles.yaml — ВНЕШНИЕ (/trade-in/api/v1/**). Для
|
||||
# scope-проверки восстанавливаем внешний путь.
|
||||
_EXTERNAL_PREFIX = "/trade-in"
|
||||
# Bootstrap-пути, доступные ЛЮБОМУ известному юзеру независимо от роли: /me отдаёт
|
||||
# роль (expired → trial-экран), /brand/* — брендинг login/trial-экрана. Без них
|
||||
# expired (roles.yaml paths:[] deny:/**) не получил бы роль и не увидел trial-экран.
|
||||
_RBAC_BOOTSTRAP_EXEMPT = ("/api/v1/me", "/api/v1/brand")
|
||||
|
||||
|
||||
async def rbac_guard(
|
||||
request: Request,
|
||||
call_next: Callable[[Request], Awaitable[Response]],
|
||||
) -> Response:
|
||||
path = request.url.path
|
||||
if path in _PUBLIC_PATHS:
|
||||
return await call_next(request)
|
||||
|
||||
username = request.headers.get("X-Authenticated-User")
|
||||
if not username:
|
||||
return JSONResponse(
|
||||
status_code=401,
|
||||
content={"detail": "no authenticated user (Caddy basic_auth required)"},
|
||||
)
|
||||
|
||||
# #2213 defense-in-depth: если общий секрет задан — запрос с X-Authenticated-User
|
||||
# ОБЯЗАН нести валидный X-Internal-Auth-Secret (его добавляет Caddy из env).
|
||||
# Иначе это подделка заголовка мимо Caddy (напр. изнутри gendesign_shared) → 401.
|
||||
# Constant-time compare против timing-атак. Пусто = защита не активна (fail-open).
|
||||
secret = settings.tradein_internal_auth_secret
|
||||
if secret:
|
||||
provided = request.headers.get("X-Internal-Auth-Secret", "")
|
||||
if not secrets.compare_digest(provided, secret):
|
||||
logger.warning(
|
||||
"RBAC: X-Authenticated-User=%r без валидного X-Internal-Auth-Secret "
|
||||
"на %s — возможная подделка заголовка мимо Caddy",
|
||||
username,
|
||||
path,
|
||||
)
|
||||
return JSONResponse(
|
||||
status_code=401,
|
||||
content={"detail": "invalid or missing internal auth secret"},
|
||||
)
|
||||
|
||||
try:
|
||||
role = get_role(username)
|
||||
except KeyError:
|
||||
logger.warning("RBAC: unknown user %r tried %s", username, path)
|
||||
return JSONResponse(
|
||||
status_code=403,
|
||||
content={"detail": "user not in roles config"},
|
||||
)
|
||||
|
||||
if _ADMIN_API_RE.match(path) and role != "admin":
|
||||
logger.info("RBAC: blocked %s (role=%s) from %s", username, role, path)
|
||||
return JSONResponse(
|
||||
status_code=403,
|
||||
content={"detail": "admin only"},
|
||||
)
|
||||
|
||||
# #R2-H3: энфорсим roles.yaml scope (paths/deny) для ВСЕХ non-admin путей, а не
|
||||
# только /admin/*. Иначе revoked (role=expired, paths:[] deny:/**) или узко-
|
||||
# скоупленный аккаунт достаёт non-admin API (напр. POST /api/v1/search —
|
||||
# экспорт листингов), который roles.yaml ему запрещает. Bootstrap-пути (/me,
|
||||
# /brand) исключены выше по списку. roles.yaml globs внешние → восстанавливаем
|
||||
# внешний путь (Caddy срезал /trade-in). На сбой парса — fail-open + громкий
|
||||
# лог: не лочим платящего pilot из-за конфиг-бага (admin-гейт выше остаётся).
|
||||
if not path.startswith(_RBAC_BOOTSTRAP_EXEMPT):
|
||||
external_path = _EXTERNAL_PREFIX + path
|
||||
try:
|
||||
allowed = is_path_allowed(role, external_path)
|
||||
except Exception:
|
||||
logger.exception(
|
||||
"RBAC scope-check raised for %s %s (ext=%s) — fail-open",
|
||||
username,
|
||||
path,
|
||||
external_path,
|
||||
)
|
||||
allowed = True
|
||||
if not allowed:
|
||||
logger.info(
|
||||
"RBAC: scope-blocked %s (role=%s) from %s (ext=%s)",
|
||||
username,
|
||||
role,
|
||||
path,
|
||||
external_path,
|
||||
)
|
||||
return JSONResponse(
|
||||
status_code=403,
|
||||
content={"detail": "forbidden for role"},
|
||||
)
|
||||
|
||||
return await call_next(request)
|
||||
|
|
@ -1,10 +1,27 @@
|
|||
"""RequestAuditMiddleware — пишет `api_request` (+ дедуплицированный `login`)
|
||||
события в `user_events` для каждого аутентифицированного `/api/*` запроса.
|
||||
"""RequestAuditMiddleware — пишет `api_request` / `admin_action` (+ дедуплицированный
|
||||
`login`/`login_failed`) события в `user_events` для каждого аутентифицированного
|
||||
`/api/*` запроса.
|
||||
|
||||
Foundation для Feature 2 (login/IP audit) и базы Feature 3 (behavior analytics).
|
||||
Логирование выполняется ПОСЛЕ `call_next` (не задерживает и не ветвит реальный
|
||||
ответ клиенту) и через fire-and-forget `schedule_event` — сбой аудита никогда
|
||||
не влияет на HTTP-ответ.
|
||||
|
||||
Порядок middleware-стека (см. `app/main.py`: `rbac_guard` — `@app.middleware("http")`,
|
||||
объявлен ДО `app.add_middleware(RequestAuditMiddleware)`) делает `RequestAudit`
|
||||
ВНЕШНИМ по отношению к `rbac_guard` (Starlette строит стек в обратном порядке
|
||||
регистрации — последний `add_middleware` оборачивает предыдущие). Поэтому к моменту,
|
||||
когда код ниже читает `response.status_code`, в нём уже отражён исход rbac_guard
|
||||
(401/403 short-circuit) ИЛИ реального хендлера — статус несёт реальный смысл
|
||||
"успех/отказ", а не только "запрос дошёл до хендлера".
|
||||
|
||||
Заведомо неаутентифицированный трафик (сканеры, долбящиеся в /wp-login.php и т.п.
|
||||
без валидного basic_auth) сюда вообще не попадает: Caddy гейтит basic_auth ПЕРЕД
|
||||
проксированием, так что `X-Authenticated-User` в таких запросах нет — условие
|
||||
`if username and ...` ниже их уже отсекает. Поэтому шум сканеров не нужно
|
||||
дополнительно фильтровать в этом файле — тот класс проблемы («сигнал тонет в шуме
|
||||
сканера») здесь структурно невозможен: событие может появиться только для
|
||||
запроса, прошедшего Caddy basic_auth.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -25,6 +42,13 @@ logger = logging.getLogger(__name__)
|
|||
# циклическую зависимость.
|
||||
_PUBLIC_PATHS = frozenset({"/health", "/docs", "/redoc", "/openapi.json"})
|
||||
|
||||
# Методы, меняющие состояние — для /api/v1/admin/* именно они должны попадать в
|
||||
# аудит с атрибуцией (кто именно загрузил куки / включил авто-логин / поправил
|
||||
# прокси / изменил настройки скрапера / выполнил bulk-операцию). GET/HEAD/OPTIONS
|
||||
# на /admin/* остаются вне аудита (см. комментарий ниже — это просмотр дашбордов,
|
||||
# не действие).
|
||||
_MUTATING_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"})
|
||||
|
||||
|
||||
class RequestAuditMiddleware(BaseHTTPMiddleware):
|
||||
"""Логирует активность аутентифицированных пользователей в `user_events`."""
|
||||
|
|
@ -38,32 +62,67 @@ class RequestAuditMiddleware(BaseHTTPMiddleware):
|
|||
if username and path.startswith("/api/") and path not in _PUBLIC_PATHS:
|
||||
ip = _client_ip(request)
|
||||
ua = request.headers.get("user-agent")
|
||||
method = request.method
|
||||
success = response.status_code < 400
|
||||
is_admin_path = path.startswith("/api/v1/admin/")
|
||||
|
||||
# Общий behavior/activity-поток — каждый authenticated API-запрос.
|
||||
# /api/v1/admin/* исключаем: это ops-действия (просмотр самих
|
||||
# дашбордов аудита/аналитики), а не поведение пилота — иначе
|
||||
# запросы дашборда зашумляют top_paths и счётчики активности.
|
||||
# login ниже логируем всегда (вход админа с IP — валидный аудит).
|
||||
if not path.startswith("/api/v1/admin/"):
|
||||
# /api/v1/admin/* исключаем из `api_request`: это ops-действия
|
||||
# (просмотр дашбордов аудита/аналитики), а не поведение пилота —
|
||||
# иначе запросы дашборда зашумляют top_paths и счётчики активности.
|
||||
if not is_admin_path:
|
||||
schedule_event(
|
||||
event_type="api_request",
|
||||
username=username,
|
||||
ip=ip,
|
||||
user_agent=ua,
|
||||
path=path,
|
||||
method=request.method,
|
||||
method=method,
|
||||
payload={"status_code": response.status_code},
|
||||
)
|
||||
|
||||
# Дедуплицированный login/IP-audit сигнал — максимум раз в день
|
||||
# на (юзер, IP, устройство).
|
||||
if should_log_login(username, ip, ua):
|
||||
elif method in _MUTATING_METHODS:
|
||||
# Admin-аудит (security-audit fix): раньше ЛЮБОЙ запрос под
|
||||
# /api/v1/admin/* (включая меняющие состояние — загрузка кук,
|
||||
# авто-логин, правка прокси, настройки скраперов, bulk-операции)
|
||||
# полностью исключался из `user_events` тем же условием, что и
|
||||
# шумные GET-дашборды — установить, КТО совершил действие, было
|
||||
# невозможно. Пишем факт действия + атрибуцию (username/ip/path/
|
||||
# method/статус) — БЕЗ тела запроса (там куки/пароли/секреты
|
||||
# правки прокси), это НЕ payload-лог, а событие "что произошло".
|
||||
schedule_event(
|
||||
event_type="login",
|
||||
event_type="admin_action",
|
||||
username=username,
|
||||
ip=ip,
|
||||
user_agent=ua,
|
||||
path=path,
|
||||
method=request.method,
|
||||
method=method,
|
||||
payload={"status_code": response.status_code, "success": success},
|
||||
)
|
||||
|
||||
# Дедуплицированный login/IP-audit сигнал — максимум раз в день
|
||||
# на (юзер, IP, устройство). Security-audit fix: раньше событие
|
||||
# всегда писалось как `login` независимо от исхода запроса —
|
||||
# отражённая RBAC-попытка (валидный Caddy basic_auth, но
|
||||
# 401/403 от rbac_guard: протухший X-Internal-Auth-Secret,
|
||||
# неизвестная роль, scope-блок) была неотличима от настоящего
|
||||
# входа. Теперь тип события расходится по `response.status_code`:
|
||||
# `login` — успех, `login_failed` — otказ. Дедуп-бакет (once per
|
||||
# user+ip+ua+day) НЕ разбит отдельно на success/fail (это
|
||||
# потребовало бы менять `should_log_login` в user_events.py —
|
||||
# вне scope этого фикса): если в рамках одного дня с этого же
|
||||
# устройства сначала случился отказ, а затем реальный успешный
|
||||
# вход, второе событие в тот же день не запишется — тот же
|
||||
# компромисс дедупа, что был и раньше, разница только в том, что
|
||||
# теперь ЕДИНСТВЕННОЕ событие дня корректно отражает, чем оно было.
|
||||
if should_log_login(username, ip, ua):
|
||||
schedule_event(
|
||||
event_type="login" if success else "login_failed",
|
||||
username=username,
|
||||
ip=ip,
|
||||
user_agent=ua,
|
||||
path=path,
|
||||
method=method,
|
||||
payload={"status_code": response.status_code},
|
||||
)
|
||||
except Exception:
|
||||
logger.warning("RequestAuditMiddleware: failed to record event", exc_info=True)
|
||||
|
|
|
|||
|
|
@ -8,15 +8,12 @@ from __future__ import annotations
|
|||
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import secrets
|
||||
from collections.abc import AsyncGenerator, Awaitable, Callable
|
||||
from collections.abc import AsyncGenerator
|
||||
from contextlib import asynccontextmanager
|
||||
|
||||
import sentry_sdk
|
||||
from fastapi import FastAPI, Request
|
||||
from fastapi import FastAPI
|
||||
from fastapi.middleware.cors import CORSMiddleware
|
||||
from fastapi.responses import JSONResponse, Response
|
||||
from sentry_sdk.integrations.fastapi import FastApiIntegration
|
||||
from sentry_sdk.integrations.httpx import HttpxIntegration
|
||||
from sentry_sdk.integrations.logging import LoggingIntegration
|
||||
|
|
@ -35,11 +32,11 @@ from app.api.v1 import (
|
|||
support,
|
||||
trade_in,
|
||||
)
|
||||
from app.core.auth import get_role, is_path_allowed
|
||||
from app.core.config import settings
|
||||
from app.core.db import SessionLocal
|
||||
from app.core.fdw import ensure_fdw_user_mapping
|
||||
from app.core.ratelimit import RateLimitMiddleware
|
||||
from app.core.rbac import rbac_guard
|
||||
from app.core.request_audit import RequestAuditMiddleware
|
||||
from app.observability.sentry_scrub import scrub_pii_event
|
||||
|
||||
|
|
@ -138,104 +135,9 @@ app = FastAPI(
|
|||
# не видит» — decided 2026-05-25).
|
||||
# 3) /api/v1/admin/* (= внешний /trade-in/api/v1/admin/* после Caddy
|
||||
# `uri strip_prefix /trade-in`) — только role=admin, иначе 403.
|
||||
# Public paths без auth (/health, /docs, /openapi.json) пропускаем —
|
||||
# X-Authenticated-User там не приходит из Caddy.
|
||||
_ADMIN_API_RE = re.compile(r"^/api/v1/admin/")
|
||||
_PUBLIC_PATHS = frozenset({"/health", "/docs", "/redoc", "/openapi.json"})
|
||||
# #R2-H3: Caddy срезает внешний префикс /trade-in (uri strip_prefix) перед
|
||||
# tradein-backend, а globs в roles.yaml — ВНЕШНИЕ (/trade-in/api/v1/**). Для
|
||||
# scope-проверки восстанавливаем внешний путь.
|
||||
_EXTERNAL_PREFIX = "/trade-in"
|
||||
# Bootstrap-пути, доступные ЛЮБОМУ известному юзеру независимо от роли: /me отдаёт
|
||||
# роль (expired → trial-экран), /brand/* — брендинг login/trial-экрана. Без них
|
||||
# expired (roles.yaml paths:[] deny:/**) не получил бы роль и не увидел trial-экран.
|
||||
_RBAC_BOOTSTRAP_EXEMPT = ("/api/v1/me", "/api/v1/brand")
|
||||
|
||||
|
||||
@app.middleware("http")
|
||||
async def rbac_guard(
|
||||
request: Request,
|
||||
call_next: Callable[[Request], Awaitable[Response]],
|
||||
) -> Response:
|
||||
path = request.url.path
|
||||
if path in _PUBLIC_PATHS:
|
||||
return await call_next(request)
|
||||
|
||||
username = request.headers.get("X-Authenticated-User")
|
||||
if not username:
|
||||
return JSONResponse(
|
||||
status_code=401,
|
||||
content={"detail": "no authenticated user (Caddy basic_auth required)"},
|
||||
)
|
||||
|
||||
# #2213 defense-in-depth: если общий секрет задан — запрос с X-Authenticated-User
|
||||
# ОБЯЗАН нести валидный X-Internal-Auth-Secret (его добавляет Caddy из env).
|
||||
# Иначе это подделка заголовка мимо Caddy (напр. изнутри gendesign_shared) → 401.
|
||||
# Constant-time compare против timing-атак. Пусто = защита не активна (fail-open).
|
||||
secret = settings.tradein_internal_auth_secret
|
||||
if secret:
|
||||
provided = request.headers.get("X-Internal-Auth-Secret", "")
|
||||
if not secrets.compare_digest(provided, secret):
|
||||
logger.warning(
|
||||
"RBAC: X-Authenticated-User=%r без валидного X-Internal-Auth-Secret "
|
||||
"на %s — возможная подделка заголовка мимо Caddy",
|
||||
username,
|
||||
path,
|
||||
)
|
||||
return JSONResponse(
|
||||
status_code=401,
|
||||
content={"detail": "invalid or missing internal auth secret"},
|
||||
)
|
||||
|
||||
try:
|
||||
role = get_role(username)
|
||||
except KeyError:
|
||||
logger.warning("RBAC: unknown user %r tried %s", username, path)
|
||||
return JSONResponse(
|
||||
status_code=403,
|
||||
content={"detail": "user not in roles config"},
|
||||
)
|
||||
|
||||
if _ADMIN_API_RE.match(path) and role != "admin":
|
||||
logger.info("RBAC: blocked %s (role=%s) from %s", username, role, path)
|
||||
return JSONResponse(
|
||||
status_code=403,
|
||||
content={"detail": "admin only"},
|
||||
)
|
||||
|
||||
# #R2-H3: энфорсим roles.yaml scope (paths/deny) для ВСЕХ non-admin путей, а не
|
||||
# только /admin/*. Иначе revoked (role=expired, paths:[] deny:/**) или узко-
|
||||
# скоупленный аккаунт достаёт non-admin API (напр. POST /api/v1/search —
|
||||
# экспорт листингов), который roles.yaml ему запрещает. Bootstrap-пути (/me,
|
||||
# /brand) исключены выше по списку. roles.yaml globs внешние → восстанавливаем
|
||||
# внешний путь (Caddy срезал /trade-in). На сбой парса — fail-open + громкий
|
||||
# лог: не лочим платящего pilot из-за конфиг-бага (admin-гейт выше остаётся).
|
||||
if not path.startswith(_RBAC_BOOTSTRAP_EXEMPT):
|
||||
external_path = _EXTERNAL_PREFIX + path
|
||||
try:
|
||||
allowed = is_path_allowed(role, external_path)
|
||||
except Exception:
|
||||
logger.exception(
|
||||
"RBAC scope-check raised for %s %s (ext=%s) — fail-open",
|
||||
username,
|
||||
path,
|
||||
external_path,
|
||||
)
|
||||
allowed = True
|
||||
if not allowed:
|
||||
logger.info(
|
||||
"RBAC: scope-blocked %s (role=%s) from %s (ext=%s)",
|
||||
username,
|
||||
role,
|
||||
path,
|
||||
external_path,
|
||||
)
|
||||
return JSONResponse(
|
||||
status_code=403,
|
||||
content={"detail": "forbidden for role"},
|
||||
)
|
||||
|
||||
return await call_next(request)
|
||||
# Guard body живёт в app/core/rbac.py (без DB/lifespan side effects), чтобы
|
||||
# тесты могли импортировать РЕАЛЬНЫЙ guard вместо hand-maintained копии.
|
||||
app.middleware("http")(rbac_guard)
|
||||
|
||||
|
||||
app.add_middleware(
|
||||
|
|
|
|||
|
|
@ -47,6 +47,31 @@ _TG_BOT_TOKEN_REPLACEMENT = "/bot[REDACTED]"
|
|||
# `id:value` в логах, напр. `chat_id:12345`).
|
||||
_TG_BOT_TOKEN_BARE_RE = re.compile(r"\b\d{6,12}:[A-Za-z0-9_-]{30,}\b")
|
||||
|
||||
# Query-string секреты в исходящих URL сторонних API (аудит-фикс, #security-audit):
|
||||
# mobileproxy changeip-ссылка (`AVITO_PROXY_ROTATE_URL` и др., admin.py
|
||||
# rotate_proxy_ip) несёт провайдерский API-ключ в query (`?...&proxy_key=...`).
|
||||
# Два независимых пути утечки в GlitchTip, зеркалящих TG-токен выше:
|
||||
# 1. `HttpxIntegration.send()` парсит URL через `parse_url(str(request.url),
|
||||
# sanitize=False)` (ЯВНЫЙ opt-out из sentry_sdk `sanitize_url`, который иначе
|
||||
# сам вырезал бы query-параметры) и кладёт полный URL в span `data["url"]` —
|
||||
# сейчас неактивно (`traces_sample_rate=0.0` в app/main.py/scheduler_main.py →
|
||||
# span не сэмплится/не уходит), но молча перестанет спасать, если трейсинг
|
||||
# когда-нибудь включат.
|
||||
# 2. `include_local_variables=True` (sentry_sdk default в app/main.py — в отличие
|
||||
# от tgbot_main.py, где явно False) кладёт stack-frame locals (`rotate_url`,
|
||||
# `exc` в rotate_proxy_ip) в traceback открытым текстом.
|
||||
# Как и TG-токен — full-text regex по КАЖДОЙ строке event (не ключ-based): секрет
|
||||
# может всплыть где угодно (frame locals, breadcrumb, exception message). НЕ
|
||||
# завязано на конкретного провайдера — покрывает любой query-параметр из
|
||||
# общеупотребимого набора секретных имён (api_key/proxy_key/token/secret/password/
|
||||
# access_token/auth), т.к. cian/yandex у нас имеют СВОИ rotate-URL (потенциально
|
||||
# другой провайдер, другое имя параметра).
|
||||
_URL_SECRET_QUERY_RE = re.compile(
|
||||
r"(?i)([?&](?:api[_-]?key|proxy[_-]?key|token|secret|password|pwd|"
|
||||
r"access[_-]?token|auth)=)[^&\s\"'<>]+"
|
||||
)
|
||||
_URL_SECRET_QUERY_REPLACEMENT = r"\g<1>" + _REDACTED
|
||||
|
||||
|
||||
def _scrub(obj: Any) -> None:
|
||||
"""Рекурсивно заменить значения PII-ключей в dict на [REDACTED] (in-place)."""
|
||||
|
|
@ -61,8 +86,50 @@ def _scrub(obj: Any) -> None:
|
|||
_scrub(item)
|
||||
|
||||
|
||||
def _redact_url_secrets_inplace(obj: Any) -> None:
|
||||
"""Рекурсивно (IN-PLACE, как `_scrub`) заменяет значения секрет-подобных
|
||||
query-параметров (`?token=...`, `?proxy_key=...` и т.п.) на [REDACTED] в
|
||||
КАЖДОЙ строке event — не ключ-based: секрет утекает через httpx span
|
||||
`url`/`query` data и через текст исключений (`str(exc)` httpx содержит полный
|
||||
request URL), а не только через известные PII-поля формы. Мутирует dict/list
|
||||
на месте (НЕ пересоздаёт структуру, в отличие от `_redact_strings`) —
|
||||
сохраняет identity верхнеуровневого `event`, на что опирается контракт
|
||||
`scrub_pii_event`/`before_send` и существующие тесты (`out is event`).
|
||||
"""
|
||||
if isinstance(obj, dict):
|
||||
for key, value in obj.items():
|
||||
if isinstance(value, str):
|
||||
redacted = _URL_SECRET_QUERY_RE.sub(_URL_SECRET_QUERY_REPLACEMENT, value)
|
||||
if redacted != value:
|
||||
obj[key] = redacted
|
||||
else:
|
||||
_redact_url_secrets_inplace(value)
|
||||
elif isinstance(obj, list):
|
||||
for i, value in enumerate(obj):
|
||||
if isinstance(value, str):
|
||||
redacted = _URL_SECRET_QUERY_RE.sub(_URL_SECRET_QUERY_REPLACEMENT, value)
|
||||
if redacted != value:
|
||||
obj[i] = redacted
|
||||
else:
|
||||
_redact_url_secrets_inplace(value)
|
||||
# tuple намеренно не обрабатываем: sentry_sdk event — это JSON-совместимая
|
||||
# структура (dict/list/str/int/...), tuple там не встречается, а даже если бы
|
||||
# встретился — он immutable, in-place правка невозможна (см. `_scrub`, тот же
|
||||
# выбор для dict/list).
|
||||
|
||||
|
||||
def scrub_pii_event(event: Event, _hint: dict[str, Any]) -> Event | None:
|
||||
"""Redact consumer-PII из error event перед отправкой. Возвращает event (не None)."""
|
||||
"""Redact consumer-PII + URL query-string секретов из error event перед отправкой.
|
||||
|
||||
Композиция (обе — in-place, сохраняют identity `event`): (1) ключ-based
|
||||
dict-scrub consumer-PII полей формы (как раньше), (2) full-text regex-проход
|
||||
по ВСЕМУ event, вырезающий значения секрет-подобных query-параметров в любой
|
||||
строке (proxy/API-ключи в исходящих URL сторонних сервисов, напр. mobileproxy
|
||||
changeip — #security-audit). Второй шаг не завязан на конкретные ключи полей —
|
||||
ловит секрет в frame locals, breadcrumb, exception message и т.д., где он может
|
||||
оказаться независимо от include_local_variables/traces_sample_rate. Возвращает
|
||||
event (не None).
|
||||
"""
|
||||
if not isinstance(event, dict):
|
||||
return event
|
||||
request = event.get("request")
|
||||
|
|
@ -70,6 +137,7 @@ def scrub_pii_event(event: Event, _hint: dict[str, Any]) -> Event | None:
|
|||
_scrub(request.get("data"))
|
||||
_scrub(event.get("extra"))
|
||||
_scrub(event.get("contexts"))
|
||||
_redact_url_secrets_inplace(event)
|
||||
return event
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -71,6 +71,24 @@ _MFE_AUTH = "header-frontend"
|
|||
# Callers that need to distinguish ban from valid auth should check state.get("_ban").
|
||||
VERIFY_BAN_SENTINEL: dict[str, Any] = {"_ban": True}
|
||||
|
||||
# audit-scrapers finding 4: verify_session раньше сводило 5xx / сетевой сбой /
|
||||
# смену вёрстки к тому же None, что и реальный логаут (401 / isAuthenticated=false) —
|
||||
# вызывающие (_cian_pre_claim, admin upload/auto-login) реагировали "куки протухли,
|
||||
# перезалей" там, где куки ни при чём (Cian недоступен ИЛИ scraper_kit.cian_state_parser
|
||||
# больше не находит header-frontend initialState). Два отдельных сигнала ниже НЕ
|
||||
# триггерят "cookies expired" алерт у вызывающих.
|
||||
|
||||
# Cian источник недоступен прямо сейчас (5xx-ответ ИЛИ сетевой/транспортный сбой —
|
||||
# timeout, DNS, connection reset). Cookies могут быть абсолютно валидны — просто
|
||||
# нечем было их проверить. Retry позже, БЕЗ пометки session invalid.
|
||||
VERIFY_SOURCE_UNAVAILABLE_SENTINEL: dict[str, Any] = {"_source_unavailable": True}
|
||||
|
||||
# HTTP 200 получен, но ожидаемый auth-state (header-frontend/initialState с
|
||||
# user.isAuthenticated) не найден/не распарсился — Cian изменил вёрстку/MFE-схему.
|
||||
# Это engineering-проблема (extract_state/_MFE_AUTH нужно обновить), НЕ протухшие
|
||||
# cookies — переставлять куки здесь бесполезно.
|
||||
VERIFY_MARKUP_CHANGED_SENTINEL: dict[str, Any] = {"_markup_changed": True}
|
||||
|
||||
|
||||
def _classify_verify_response(
|
||||
status_code: int,
|
||||
|
|
@ -79,19 +97,25 @@ def _classify_verify_response(
|
|||
"""Pure classifier — maps (status_code, html) to verify_session outcome.
|
||||
|
||||
Returns:
|
||||
VERIFY_BAN_SENTINEL — 403/TLS ban (cookies may be fine, server is blocking)
|
||||
None — 401 or isAuthenticated=false (cookies genuinely expired)
|
||||
state dict — authenticated successfully
|
||||
VERIFY_BAN_SENTINEL — 403/TLS ban (cookies могут быть в порядке,
|
||||
блокирует сервер)
|
||||
VERIFY_SOURCE_UNAVAILABLE_SENTINEL — 5xx/иной non-200 без содержимого —
|
||||
источник недоступен, НЕ cookies
|
||||
VERIFY_MARKUP_CHANGED_SENTINEL — HTTP 200, но auth-state не найден/не
|
||||
распарсился — вёрстка/схема изменилась
|
||||
None — 401 ИЛИ isAuthenticated=false — cookies
|
||||
ДЕЙСТВИТЕЛЬНО протухли/разлогинены
|
||||
state dict — authenticated successfully
|
||||
"""
|
||||
if status_code == 403:
|
||||
return VERIFY_BAN_SENTINEL
|
||||
if status_code == 401:
|
||||
return None
|
||||
if html is None:
|
||||
return None
|
||||
if status_code != 200 or html is None:
|
||||
return VERIFY_SOURCE_UNAVAILABLE_SENTINEL
|
||||
state = extract_state(html, mfe=_MFE_AUTH, key="initialState")
|
||||
if state is None:
|
||||
return None
|
||||
return VERIFY_MARKUP_CHANGED_SENTINEL
|
||||
user = state.get("user", {}) or {}
|
||||
if not user.get("isAuthenticated"):
|
||||
return None
|
||||
|
|
@ -104,11 +128,18 @@ async def verify_session(cookies: dict[str, str]) -> dict[str, Any] | None:
|
|||
Uses curl_cffi with impersonate='chrome120' (same as prod scrapers) to avoid
|
||||
TLS-fingerprint bans that httpx would trigger.
|
||||
|
||||
Returns:
|
||||
state dict — authenticated (contains user.isAuthenticated + userId)
|
||||
VERIFY_BAN_SENTINEL — HTTP 403 TLS/bot ban; cookies may still be valid —
|
||||
callers should NOT trigger a cookie-refresh alert
|
||||
None — HTTP 401 or isAuthenticated=false; cookies expired
|
||||
Returns (проверяй через `is`, НЕ `==` — это sentinel-объекты):
|
||||
state dict — authenticated (user.isAuthenticated + userId)
|
||||
VERIFY_BAN_SENTINEL — HTTP 403 TLS/bot ban; cookies могут быть
|
||||
валидны — НЕ триггерить cookie-refresh alert
|
||||
VERIFY_SOURCE_UNAVAILABLE_SENTINEL — 5xx/network/timeout; источник недоступен,
|
||||
НЕ триггерить cookie-refresh alert, retry позже
|
||||
VERIFY_MARKUP_CHANGED_SENTINEL — HTTP 200 но auth-state не распарсился;
|
||||
Cian изменил вёрстку — НЕ cookie-проблема,
|
||||
нужен engineering-фикс extract_state/_MFE_AUTH
|
||||
None — HTTP 401 или isAuthenticated=false; cookies
|
||||
ДЕЙСТВИТЕЛЬНО протухли — здесь и только здесь
|
||||
имеет смысл просить re-upload
|
||||
|
||||
Никогда не логирует сырые значения cookies.
|
||||
"""
|
||||
|
|
@ -134,6 +165,18 @@ async def verify_session(cookies: dict[str, str]) -> dict[str, Any] | None:
|
|||
logger.warning(
|
||||
"Cian cookies verify: HTTP 403 TLS/bot ban — cookies NOT marked expired"
|
||||
)
|
||||
elif result is VERIFY_SOURCE_UNAVAILABLE_SENTINEL:
|
||||
logger.warning(
|
||||
"Cian cookies verify: source unavailable (status=%d) — "
|
||||
"cookies NOT marked expired, retry later",
|
||||
status,
|
||||
)
|
||||
elif result is VERIFY_MARKUP_CHANGED_SENTINEL:
|
||||
logger.error(
|
||||
"Cian cookies verify: HTTP 200 but auth-state not found/parseable "
|
||||
"(mfe=%s) — markup/schema changed, cookies NOT marked expired",
|
||||
_MFE_AUTH,
|
||||
)
|
||||
elif result is None:
|
||||
logger.warning("Cian cookies verify: expired/unauthenticated (status=%d)", status)
|
||||
else:
|
||||
|
|
@ -142,8 +185,11 @@ async def verify_session(cookies: dict[str, str]) -> dict[str, Any] | None:
|
|||
|
||||
return result
|
||||
except Exception as exc:
|
||||
logger.warning("Cian cookies verify failed: %s", exc)
|
||||
return None
|
||||
# Сетевой/транспортный сбой (timeout, DNS, connection reset и т.п.) — источник
|
||||
# недоступен, НЕ признак протухших cookies (finding 4). Раньше здесь везде
|
||||
# возвращался None, конфлируя с реальным логаутом.
|
||||
logger.warning("Cian cookies verify: transport/network error — %s", exc)
|
||||
return VERIFY_SOURCE_UNAVAILABLE_SENTINEL
|
||||
|
||||
|
||||
def save_session(
|
||||
|
|
|
|||
|
|
@ -36,6 +36,21 @@
|
|||
(entrypoint), не здесь.
|
||||
E) /start клиенту → короткое приветствие МЕРЫ, без зеркалирования в топик
|
||||
(команда — не содержательное обращение, не должна засорять топик).
|
||||
F) Флуд-лимит на отправителя (низкий приоритет, per-chat_id): воркер
|
||||
long-polling однопоточный и обрабатывает апдейты СТРОГО последовательно, а
|
||||
Telegram ограничивает саму support-группу ~20 сообщениями/минуту — ОДНИМ
|
||||
бюджетом на ВСЕХ клиентов разом (зеркала + шапки + ответы оператора).
|
||||
Превышение — 429 с ожиданием 30-60с, на которые воркер не может обработать
|
||||
НИ ОДНОГО следующего апдейта — один флудящий клиент подвешивает доставку
|
||||
всем остальным. `_flood_limiter` (тот же `SlidingWindowLimiter`, что и
|
||||
веб-чат поддержки, ключ — TELEGRAM chat_id) режет per-sender поток заметно
|
||||
ниже группового лимита; сообщения сверх бюджета НЕ зеркалируются (иначе
|
||||
сам факт мирроринга уже съедает групповой бюджет, который мы и защищаем) и
|
||||
НЕ пишутся в tg_support_messages (нечего маршрутизировать без
|
||||
topic_message_id). Клиент получает уведомление, что сообщение НЕ
|
||||
доставлено (молчать нельзя — иначе клиент решит, что оператор его получил),
|
||||
но не чаще ОДНОГО РАЗА за то же окно (`_flood_notify_limiter`, limit=1) —
|
||||
иначе само уведомление стало бы вторым источником флуда.
|
||||
|
||||
Персистентность вынесена за `BridgeStorage`-протокол — маршрутизирующая логика
|
||||
(`process_update` и приватные `_handle_*`) не завязана на реальную БД, тестируется
|
||||
|
|
@ -58,6 +73,7 @@ from sqlalchemy.exc import SQLAlchemyError
|
|||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.core.config import settings
|
||||
from app.core.ratelimit import SlidingWindowLimiter
|
||||
from app.core.shutdown import shutdown_requested
|
||||
from app.services.tgbot import web_support_storage
|
||||
from app.services.tgbot.client import TelegramApiError, TelegramClient
|
||||
|
|
@ -87,6 +103,29 @@ SERVICE_UNAVAILABLE_TEXT = (
|
|||
# tg_support_messages.kind): "text | photo | document | video | voice | other".
|
||||
_KNOWN_KINDS = ("text", "photo", "document", "video", "voice")
|
||||
|
||||
# (низкий приоритет, флуд-защита) — см. пункт F) в докстринге модуля. Порог
|
||||
# НАМЕРЕННО заметно ниже группового лимита Telegram (~20 msg/min): бюджет
|
||||
# делится с шапками-идентификациями и ответами оператора, и с другими
|
||||
# одновременными клиентами — щедрый лимит одного отправителя всё равно упёрся
|
||||
# бы в общий групповой 429. Тот же примитив, что и веб-чат поддержки
|
||||
# (app/api/v1/support.py `_send_limiter`), ключ здесь — TELEGRAM chat_id
|
||||
# отправителя (не username — у Telegram-клиента username может отсутствовать).
|
||||
_FLOOD_LIMIT = 5
|
||||
_FLOOD_WINDOW_S = 60.0
|
||||
_flood_limiter = SlidingWindowLimiter(limit=_FLOOD_LIMIT, window_s=_FLOOD_WINDOW_S)
|
||||
|
||||
# Уведомление о флуде — не чаще ОДНОГО раза за то же окно, иначе само
|
||||
# уведомление стало бы вторым источником флуда. Отдельный лимитер с limit=1 на
|
||||
# то же окно: `check()` возвращает None (и фиксирует попытку) ровно один раз за
|
||||
# окно, дальше молчит до его истечения — без отдельной структуры "когда в
|
||||
# последний раз уведомляли".
|
||||
_flood_notify_limiter = SlidingWindowLimiter(limit=1, window_s=_FLOOD_WINDOW_S)
|
||||
|
||||
FLOOD_LIMITED_TEXT = (
|
||||
"Сообщение не доставлено — вы отправляете сообщения слишком часто. "
|
||||
"Пожалуйста, подождите немного и напишите ещё раз."
|
||||
)
|
||||
|
||||
# #tgsupport-web review M2: реплай оператора медиа-типом (в т.ч. фото С ПОДПИСЬЮ)
|
||||
# на веб-зеркало НЕ доставляется частично — веб-чат текстовый MVP, оператор
|
||||
# получает это уведомление в топике вместо тихого игнора (иначе уверен, что ответил).
|
||||
|
|
@ -407,6 +446,30 @@ async def _handle_private_message(
|
|||
logger.warning("tgbot bridge: приватное сообщение без message_id — игнор")
|
||||
return
|
||||
|
||||
# F) Флуд-лимит на отправителя — peek БЕЗ расхода бюджета (тот же паттерн,
|
||||
# что `_send_limiter` в app/api/v1/support.py: под лимитом ниже сразу
|
||||
# `.record()`-им попытку). Над лимитом — НЕ зеркалируем (иначе сам мирроринг
|
||||
# уже съедает групповой Telegram-бюджет, который лимит и защищает) и НЕ
|
||||
# пишем в tg_support_messages (без topic_message_id маршрутизировать ответ
|
||||
# всё равно нечего).
|
||||
flood_key = str(chat_id) # SlidingWindowLimiter — ключ str (см. app/core/ratelimit.py)
|
||||
if _flood_limiter.retry_after(flood_key) is not None:
|
||||
logger.warning(
|
||||
"tgbot bridge: chat_id=%d превысил флуд-лимит (%d msg/%.0fs) — "
|
||||
"сообщение НЕ зеркалируется в топик (защита группового Telegram-лимита)",
|
||||
chat_id,
|
||||
_FLOOD_LIMIT,
|
||||
_FLOOD_WINDOW_S,
|
||||
)
|
||||
# Уведомляем клиента, что сообщение НЕ доставлено (молчать нельзя —
|
||||
# иначе клиент решит, что оператор его получил), но не чаще одного раза
|
||||
# за окно — `_flood_notify_limiter.check()` возвращает None (и сам
|
||||
# фиксирует попытку) ровно один раз за окно.
|
||||
if _flood_notify_limiter.check(flood_key) is None:
|
||||
await client.send_message(chat_id=chat_id, text=FLOOD_LIMITED_TEXT)
|
||||
return
|
||||
_flood_limiter.record(flood_key)
|
||||
|
||||
# Шапка — только на первое сообщение клиента за окно, иначе топик засоряется.
|
||||
if not storage.had_recent_inbound(chat_id, window_seconds=_HEADER_THROTTLE_WINDOW_S):
|
||||
header = _format_topic_header(chat_id, username, first_name, last_name)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,231 @@
|
|||
-- 190_sale_share_price_bucket_signature.sql
|
||||
--
|
||||
-- CONTEXT: аудит МЕРЫ. Числитель v_building_sale_share (мигр. 148) дедупит
|
||||
-- листинги по сигнатуре (rooms, round(area_m2), floor) — убирает кросс-
|
||||
-- площадочные дубли одной физической квартиры (avito+cian+domclick). Но в
|
||||
-- типовом секционном доме 4 РАЗНЫЕ квартиры на одном этаже в разных
|
||||
-- подъездах имеют ТУ ЖЕ тройку признаков (подъезда в данных нет) → ложно
|
||||
-- схлопываются в одну.
|
||||
--
|
||||
-- Прод-замер (снят вручную, до этой миграции; не переснят в рамках неё —
|
||||
-- нет доступа к БД из этой сессии, см. ниже):
|
||||
-- · без дедупа (активные вторичные, house_id_fk/rooms/area_m2/floor/
|
||||
-- price_rub все NOT NULL): 15 424 записи;
|
||||
-- · текущая сигнатура (rooms, round(area_m2), floor): 11 324 «квартиры»
|
||||
-- (−4 100 против raw — почти весь эффект дедупа, но и false-merge тоже);
|
||||
-- · та же сигнатура + price_bucket round(price_rub/100000): 12 497
|
||||
-- (+1 173 против текущей, +10.4%) — возвращает часть false-merge'ов.
|
||||
-- Внутри 3 220 групп, схлопнутых текущей сигнатурой:
|
||||
-- · 1 252 группы (1 756 записей) имеют РАЗНЫЕ цены — почти наверняка
|
||||
-- разные квартиры, не кросс-пост;
|
||||
-- · 233 группы (249 записей) пришли с ОДНОЙ площадки — одна площадка
|
||||
-- редко публикует одну и ту же квартиру дважды, тоже почти наверняка
|
||||
-- разные квартиры (см. "residual risk" ниже — этот класс НЕ решается
|
||||
-- одним лишь price_bucket, если у них к тому же совпала цена).
|
||||
--
|
||||
-- РЕШЕНИЕ ВЛАДЕЛЬЦА ПРОДУКТА: схлопывать записи, только если они совпадают
|
||||
-- ЕЩЁ И по цене (round(price_rub/100000) — тот же бакет, что уже
|
||||
-- используется в backend/app/services/estimator.py::_DEDUP_PRICE_BUCKET_RUB
|
||||
-- для кросс-source физ-дедупа аналогов; ~±0.5% допуска при 21М, ~±2% при
|
||||
-- 2.5М — см. app/core/config.py:265). Разные квартиры в одном доме
|
||||
-- почти никогда не стоят ровно одинаково, кросс-пост одного лота — стоит.
|
||||
--
|
||||
-- ЧТО НЕ ВОШЛО (source-distinctness) и почему:
|
||||
-- Продуктовое решение также просило требовать "с разных площадок". Честно
|
||||
-- выразить это внутри count(DISTINCT ...) НЕЛЬЗЯ без перестройки CTE
|
||||
-- listing_agg в двухуровневую агрегацию (сначала GROUP BY house_id +
|
||||
-- расширенная сигнатура + count(DISTINCT source) per группа, потом per-house
|
||||
-- SUM(CASE WHEN distinct_sources>=2 THEN 1 ELSE listing_count END)) — это
|
||||
-- затронуло бы ВСЕ 6 агрегатов CTE (active_secondary, listings_45d,
|
||||
-- median_price_rub, median_price_per_m2, avg_days_on_market,
|
||||
-- listings_med_floors), которые сейчас делят один плоский FILTER-паттерн,
|
||||
-- накопленный за 6 миграций (148-153). Риск регрессии от такой перестройки
|
||||
-- в одной миграции выше, чем ценность второго guard'а поверх уже сильно
|
||||
-- сузившего false-merge price_bucket. Берём только price-часть.
|
||||
--
|
||||
-- RESIDUAL RISK (направление ошибки после этой миграции):
|
||||
-- 1) НЕ решено — 233 группы/249 записей с ОДНОЙ площадкой: если у них
|
||||
-- внутри группы цена ТОЖЕ совпадает (не проверено, нет прод-доступа
|
||||
-- в этой сессии), они останутся ложно схлопнуты (недосчёт числителя,
|
||||
-- sale_share_pct ЗАНИЖЕН для этих домов) — тот же вид ошибки, что и
|
||||
-- раньше, но у существенно меньшего подмножества.
|
||||
-- 2) НОВЫЙ вид ошибки, которого не было: настоящий кросс-пост одного
|
||||
-- физлота, где цена УСПЕЛА измениться между скрейпами разных площадок
|
||||
-- (снизили цену на avito, domclick ещё не досканирован) — теперь НЕ
|
||||
-- схлопнется (разные price_bucket) → числитель ЗАВЫШЕН для этих домов.
|
||||
-- Раньше такая пара схлопывалась верно (без price в ключе). Прямого
|
||||
-- прод-замера размера этого класса нет.
|
||||
-- Итого: миграция МЕНЯЕТ баланс ошибки с «сильный недосчёт от false-merge
|
||||
-- по этажу/подъезду» на «слабый недосчёт по одноплощадочным совпадениям +
|
||||
-- небольшой new-пересчёт по кросс-постам с ценовым дрейфом» — чище, но не
|
||||
-- идеально в обе стороны.
|
||||
--
|
||||
-- price_rub NULL/0 handling: listings.price_rub объявлена `bigint NOT NULL`
|
||||
-- (002_core_tables.sql), но код уже трактует её defensively как потенциально
|
||||
-- отсутствующую (146/148: `l.price_rub IS NOT NULL` в median FILTER) — то же
|
||||
-- делаем здесь. price_bucket-компонент = NULL, когда price_rub IS NULL ИЛИ
|
||||
-- <= 0 (0/отрицательное — sentinel нераспарсенной цены, не реальная цена).
|
||||
-- Партиально-NULL кортеж (rooms/area/floor есть, price_bucket NULL)
|
||||
-- count(DISTINCT ROW(...)) трактует как СВОЙ отдельный кортеж (см. NULL-
|
||||
-- handling мигр. 148) — т.е. листинг без подтверждённой цены НЕ схлопывается
|
||||
-- ни с чем, считается один. Консервативно (не создаёт ложных совпадений по
|
||||
-- цене) и совпадает с философией estimator.py::_lot_dedup_components
|
||||
-- (`if not price: composite = None` → лот не участвует в физ-дедупе).
|
||||
--
|
||||
-- WHAT: в CTE listing_agg расширяем сигнатуру DISTINCT В ОБОИХ числителях
|
||||
-- (active_secondary, listings_45d) с (rooms, round(area_m2), floor) до
|
||||
-- (rooms, round(area_m2), floor, price_bucket), где price_bucket = CASE
|
||||
-- WHEN l.price_rub IS NULL OR l.price_rub <= 0 THEN NULL
|
||||
-- ELSE round(l.price_rub / 100000.0) END.
|
||||
-- Остальные 4 агрегата CTE (median_price_rub, median_price_per_m2,
|
||||
-- avg_days_on_market, listings_med_floors) — НЕ дедуп-based (считают по
|
||||
-- сырым листингам, прошедшим FILTER), не трогаем. Весь top-level SELECT /
|
||||
-- WHERE / плаузибилити-гейт (мигр. 145/153) / appended-колонки
|
||||
-- (zhkh_flat_count, flat_count_source) / гео(≤300м, мигр.150) / floors-guard
|
||||
-- (±3, мигр.152) — БАЙТ-В-БАЙТ как в мигр. 153.
|
||||
--
|
||||
-- DEPENDENCIES: 143 (view + houses.gar_*), 144 (canon match → gar_flat_count),
|
||||
-- 145 (плаузибилити-гейт знаменателя), 146 (listings_45d + sale_share_pct_45d
|
||||
-- + zhkh в COALESCE), 147 (canon strip geo-prefixes), 148 (дедуп
|
||||
-- кросс-площадочных дублей — база сигнатуры, которую здесь расширяем), 149
|
||||
-- (ЖКХ-приоритет знаменателя), 150 (гео-фильтр ≤300м в CTE), 151 (bare-street
|
||||
-- aliases — view не трогала), 152 (floors-guard ±3), 153 (плаузибилити по
|
||||
-- листинговой медианной этажности). Базируется на текущем (153) определении
|
||||
-- view — меняем ТОЛЬКО DISTINCT-выражение в active_secondary/listings_45d.
|
||||
--
|
||||
-- SAFETY / IDEMPOTENCY: CREATE OR REPLACE VIEW ONLY (структура top-level
|
||||
-- колонок не меняется — те же позиции/типы/имена, что в 153) + COMMENT.
|
||||
-- Никакого DDL над таблицами. Повторный прогон — no-op (REPLACE на
|
||||
-- идентичное определение). Деплой-раннер гонит файл через
|
||||
-- psql -v ON_ERROR_STOP=on БЕЗ --single-transaction → транзакцию открывает
|
||||
-- САМ файл (BEGIN/COMMIT ниже), как 146/148/149/150/152/153.
|
||||
--
|
||||
-- CONSUMERS (грепнуто по backend+frontend, не тронуты этой миграцией):
|
||||
-- backend/app/services/buildings_query.py — SELECT * колонок view (список,
|
||||
-- summary, гистограмма) — тот же набор колонок, не ломается;
|
||||
-- backend/app/schemas/buildings.py, backend/app/api/v1/buildings.py —
|
||||
-- Pydantic-схема поверх тех же колонок, не ломается;
|
||||
-- backend/tests/test_buildings_api.py — тестирует ТОЛЬКО текст SQL-билдеров
|
||||
-- (строку "FROM v_building_sale_share" и т.п.), не внутренний DISTINCT view
|
||||
-- → не ломается этой миграцией;
|
||||
-- ⚠ backend/app/services/buildings_query.py::build_listings_query — ОТДЕЛЬНЫЙ
|
||||
-- SQL (не читает view), реализует ТУ ЖЕ (rooms, round(area_m2), floor)
|
||||
-- сигнатуру САМОСТОЯТЕЛЬНО (DISTINCT ON) для панели листингов одного дома.
|
||||
-- После этой миграции сигнатуры /buildings/sale-share (список, через view,
|
||||
-- теперь +price_bucket) и /buildings/{id}/listings (панель, старая 3-тройка)
|
||||
-- РАСХОДЯТСЯ — на детальной панели дома возможен чуть меньший count уникальных
|
||||
-- квартир, чем active_secondary в списке. НЕ трогаем buildings_query.py в
|
||||
-- этой миграции (вне границ задачи) — фиксируем расхождение как known
|
||||
-- follow-up для отдельной задачи.
|
||||
--
|
||||
-- NB по нумерации: последний занятый = 188 (187/188 заняты веб-чатом);
|
||||
-- следующий свободный sequential = 189 (проверено `ls tradein-mvp/backend/
|
||||
-- data/sql | grep '^18'` — 187, 188 заняты, 189 свободен; дубля basename нет).
|
||||
--
|
||||
-- Deploy order: после 188_tg_support_chat_id_scope.sql.
|
||||
|
||||
BEGIN;
|
||||
|
||||
CREATE OR REPLACE VIEW v_building_sale_share AS
|
||||
WITH listing_agg AS (
|
||||
SELECT l.house_id_fk AS house_id,
|
||||
count(DISTINCT (l.rooms, round(l.area_m2), l.floor,
|
||||
CASE WHEN l.price_rub IS NULL OR l.price_rub <= 0 THEN NULL
|
||||
ELSE round(l.price_rub / 100000.0) END))
|
||||
FILTER (WHERE l.is_active AND l.listing_segment = 'vtorichka'::text AND (l.geom IS NULL OR hg.geom IS NULL OR ST_DistanceSphere(l.geom, hg.geom) <= 300) AND (l.total_floors IS NULL OR COALESCE(hg.zhkh_floors, hg.total_floors) IS NULL OR abs(l.total_floors - COALESCE(hg.zhkh_floors, hg.total_floors)) <= 3)) AS active_secondary,
|
||||
count(DISTINCT (l.rooms, round(l.area_m2), l.floor,
|
||||
CASE WHEN l.price_rub IS NULL OR l.price_rub <= 0 THEN NULL
|
||||
ELSE round(l.price_rub / 100000.0) END)) FILTER (
|
||||
WHERE l.listing_segment = 'vtorichka'::text
|
||||
AND l.last_seen_at >= (now() - interval '45 days')
|
||||
AND (l.geom IS NULL OR hg.geom IS NULL OR ST_DistanceSphere(l.geom, hg.geom) <= 300)
|
||||
AND (l.total_floors IS NULL OR COALESCE(hg.zhkh_floors, hg.total_floors) IS NULL OR abs(l.total_floors - COALESCE(hg.zhkh_floors, hg.total_floors)) <= 3)
|
||||
) AS listings_45d,
|
||||
percentile_cont(0.5::double precision) WITHIN GROUP (ORDER BY (l.price_rub::double precision))
|
||||
FILTER (WHERE l.is_active AND l.listing_segment = 'vtorichka'::text AND l.price_rub IS NOT NULL AND (l.geom IS NULL OR hg.geom IS NULL OR ST_DistanceSphere(l.geom, hg.geom) <= 300) AND (l.total_floors IS NULL OR COALESCE(hg.zhkh_floors, hg.total_floors) IS NULL OR abs(l.total_floors - COALESCE(hg.zhkh_floors, hg.total_floors)) <= 3)) AS median_price_rub,
|
||||
percentile_cont(0.5::double precision) WITHIN GROUP (ORDER BY (l.price_per_m2::double precision))
|
||||
FILTER (WHERE l.is_active AND l.listing_segment = 'vtorichka'::text AND l.price_per_m2 IS NOT NULL AND (l.geom IS NULL OR hg.geom IS NULL OR ST_DistanceSphere(l.geom, hg.geom) <= 300) AND (l.total_floors IS NULL OR COALESCE(hg.zhkh_floors, hg.total_floors) IS NULL OR abs(l.total_floors - COALESCE(hg.zhkh_floors, hg.total_floors)) <= 3)) AS median_price_per_m2,
|
||||
avg(l.days_on_market)
|
||||
FILTER (WHERE l.is_active AND l.listing_segment = 'vtorichka'::text AND l.days_on_market IS NOT NULL AND (l.geom IS NULL OR hg.geom IS NULL OR ST_DistanceSphere(l.geom, hg.geom) <= 300) AND (l.total_floors IS NULL OR COALESCE(hg.zhkh_floors, hg.total_floors) IS NULL OR abs(l.total_floors - COALESCE(hg.zhkh_floors, hg.total_floors)) <= 3)) AS avg_days_on_market,
|
||||
percentile_cont(0.5) WITHIN GROUP (ORDER BY l.total_floors)
|
||||
FILTER (WHERE l.is_active AND l.listing_segment = 'vtorichka'::text AND (l.geom IS NULL OR hg.geom IS NULL OR ST_DistanceSphere(l.geom, hg.geom) <= 300) AND (l.total_floors IS NULL OR COALESCE(hg.zhkh_floors, hg.total_floors) IS NULL OR abs(l.total_floors - COALESCE(hg.zhkh_floors, hg.total_floors)) <= 3)) AS listings_med_floors
|
||||
FROM listings l
|
||||
JOIN houses hg ON hg.id = l.house_id_fk
|
||||
WHERE l.house_id_fk IS NOT NULL
|
||||
GROUP BY l.house_id_fk
|
||||
)
|
||||
SELECT h.id AS house_id,
|
||||
h.short_address,
|
||||
h.full_address,
|
||||
h.address,
|
||||
h.lat,
|
||||
h.lon,
|
||||
h.year_built,
|
||||
h.house_type,
|
||||
h.total_floors,
|
||||
h.series_name,
|
||||
h.is_emergency,
|
||||
COALESCE(h.zhkh_flat_count, h.gar_flat_count, NULLIF(h.total_units, 0), NULLIF(h.flat_count, 0)) AS flat_count_effective,
|
||||
h.gar_flat_count,
|
||||
h.gar_match_method,
|
||||
la.active_secondary,
|
||||
la.median_price_rub,
|
||||
la.median_price_per_m2,
|
||||
la.avg_days_on_market,
|
||||
CASE
|
||||
WHEN COALESCE(h.zhkh_flat_count, h.gar_flat_count, NULLIF(h.total_units, 0), NULLIF(h.flat_count, 0))
|
||||
>= GREATEST(COALESCE(h.total_floors, 0), COALESCE(la.listings_med_floors, 0)::int, 8)
|
||||
AND la.active_secondary <= COALESCE(h.zhkh_flat_count, h.gar_flat_count, NULLIF(h.total_units, 0), NULLIF(h.flat_count, 0))
|
||||
THEN round(100.0 * la.active_secondary::numeric
|
||||
/ COALESCE(h.zhkh_flat_count, h.gar_flat_count, NULLIF(h.total_units, 0), NULLIF(h.flat_count, 0))::numeric, 1)
|
||||
ELSE NULL::numeric
|
||||
END AS sale_share_pct,
|
||||
la.listings_45d,
|
||||
CASE
|
||||
WHEN COALESCE(h.zhkh_flat_count, h.gar_flat_count, NULLIF(h.total_units, 0), NULLIF(h.flat_count, 0))
|
||||
>= GREATEST(COALESCE(h.total_floors, 0), COALESCE(la.listings_med_floors, 0)::int, 8)
|
||||
AND la.listings_45d <= COALESCE(h.zhkh_flat_count, h.gar_flat_count, NULLIF(h.total_units, 0), NULLIF(h.flat_count, 0))
|
||||
THEN round(100.0 * la.listings_45d::numeric
|
||||
/ COALESCE(h.zhkh_flat_count, h.gar_flat_count, NULLIF(h.total_units, 0), NULLIF(h.flat_count, 0))::numeric, 1)
|
||||
ELSE NULL::numeric
|
||||
END AS sale_share_pct_45d,
|
||||
h.zhkh_flat_count,
|
||||
CASE
|
||||
WHEN h.zhkh_flat_count IS NOT NULL THEN 'zhkh'
|
||||
WHEN h.gar_flat_count IS NOT NULL THEN 'gar'
|
||||
WHEN NULLIF(h.total_units, 0) IS NOT NULL THEN 'total_units'
|
||||
WHEN NULLIF(h.flat_count, 0) IS NOT NULL THEN 'flat_count'
|
||||
ELSE NULL::text
|
||||
END AS flat_count_source
|
||||
FROM houses h
|
||||
JOIN listing_agg la ON la.house_id = h.id
|
||||
WHERE h.geom IS NOT NULL AND (la.active_secondary > 0 OR la.listings_45d > 0);
|
||||
|
||||
COMMENT ON VIEW v_building_sale_share IS
|
||||
'Per-building rollup вторички для «доли квартир дома в продаже» (мигр. 143; знаменатель — '
|
||||
'ГАР canon-match мигр. 144; 2-й источник ЖКХ + окно 45д мигр. 146; дедуп кросс-площадочных '
|
||||
'дублей мигр. 148 + price_bucket мигр. 189; ЖКХ-приоритет знаменателя мигр. 149; гео-фильтр '
|
||||
'числителя ≤300м мигр. 150). flat_count_effective = '
|
||||
'COALESCE(zhkh_flat_count, gar_flat_count, NULLIF(total_units,0), NULLIF(flat_count,0)) — '
|
||||
'ЖКХ ПРИОРИТЕТ (ГИС ЖКХ точнее ГАР, который дико недосчитывает квартиры в МКД; мигр. 149). '
|
||||
'Колонки zhkh_flat_count (сырой ЖКХ-счёт) + flat_count_source (zhkh|gar|total_units|flat_count|'
|
||||
'NULL — какой источник реально дал знаменатель) добавлены для прозрачности. Оба числителя '
|
||||
'считают УНИКАЛЬНЫЕ КВАРТИРЫ по сигнатуре count(DISTINCT (rooms, round(area_m2), floor, '
|
||||
'price_bucket)), где price_bucket = round(price_rub/100000) ИЛИ NULL при price_rub NULL/<=0 '
|
||||
'(мигр. 189: одна тройка rooms/area/floor не отличает соседние квартиры на одном этаже в разных '
|
||||
'подъездах — совпадение ЕЩЁ И по цене резко снижает false-merge; NULL-цена не схлопывается ни с '
|
||||
'чем, считается отдельно — та же партиально-NULL философия, что и в мигр. 148 для rooms/area/'
|
||||
'floor, и что в estimator.py::_lot_dedup_components для физ-дедупа аналогов). Требование '
|
||||
'«разных площадок» из продуктового решения НЕ выражено в SQL (потребовало бы двухуровневой '
|
||||
'агрегации across всех 6 FILTER-агрегатов CTE) — residual risk: однисточниковые группы с '
|
||||
'совпавшей ценой остаются ложно схлопнуты; кросс-посты с ценовым дрейфом между скрейпами '
|
||||
'перестают схлопываться (см. комментарий мигр. 189 в файле). active_secondary = FILTER '
|
||||
'(is_active AND vtorichka); listings_45d = FILTER (vtorichka AND last_seen_at>=now()-45d). '
|
||||
'sale_share_pct = active_secondary/denom; sale_share_pct_45d = listings_45d/denom. Оба под '
|
||||
'плаузибилити-гейтом (denom>=GREATEST(total_floors, листинговая-медианная-этажность, 8) AND '
|
||||
'числитель<=denom; мигр. 145 + 153), иначе NULL. Фильтр: geom NOT NULL AND (active_secondary>0 '
|
||||
'OR listings_45d>0) — churn-only дома тоже видны. active_secondary/listings_45d/медианы цены и '
|
||||
'срока считают ТОЛЬКО листинги ≤300м от geom своего дома (мигр. 150) с floors-guard ±3 (мигр. '
|
||||
'152). Листинги/дома без geom — кепим. Знаменатель НЕ изменён мигр. 189.';
|
||||
|
||||
COMMIT;
|
||||
|
|
@ -176,3 +176,31 @@
|
|||
169_osm_poi_ekb_local.sql
|
||||
170_scrape_schedules_seed_osm_poi_ekb_refresh.sql
|
||||
172_trade_in_leads.sql
|
||||
173_scrape_proxies_add_domclick_affinity.sql
|
||||
174_domclick_session_cookies.sql
|
||||
175_scrape_schedules_seed_domclick_detail_backfill.sql
|
||||
176_domrf_kapremont.sql
|
||||
177_deals_city_region.sql
|
||||
178_deal_city_price_bands.sql
|
||||
179_scrape_schedules_seed_oblast_city_sweeps.sql
|
||||
180_seed_sber_freshness_monitor.sql
|
||||
181_clamp_bad_listing_dates.sql
|
||||
182_trade_in_leads_consent_proof.sql
|
||||
183_reenable_deactivate_stale_domklik.sql
|
||||
184_user_events.sql
|
||||
185_account_quota_overrides.sql
|
||||
186_tg_support.sql
|
||||
#
|
||||
# 187_web_support_chat.sql / 188_tg_support_chat_id_scope.sql — НАМЕРЕННО НЕ
|
||||
# добавлены (2026-07-27, devops-аудит). Прецедент из ЭТОГО же репо:
|
||||
# commit 5eadae1e (fix(tradein/support): address deep-review ... L5) добавил
|
||||
# и тут же убрал "187_web_support_chat.sql" из этого файла с формулировкой
|
||||
# "keeping an unmerged migration name out of it preserves the option to
|
||||
# rename before merge without tripping the "can't rename applied
|
||||
# migrations" test". Обе миграции — часть веб-чата поддержки (#2532/#2533),
|
||||
# который на момент этой правки ещё активно дорабатывается в параллельной
|
||||
# сессии/окне (тот же фиче-набор, соседняя задача). Дописывать их сюда сейчас
|
||||
# повторило бы именно ту ошибку, которую L5 исправил: заморозить имя файла
|
||||
# ДО того как он гарантированно осел на проде в финальном виде. Когда фича
|
||||
# стабилизируется и подтверждено, что 187/188 применены (_schema_migrations
|
||||
# на проде) — дописать одной строкой в отдельном PR.
|
||||
|
|
|
|||
18
tradein-mvp/backend/tests/conftest.py
Normal file
18
tradein-mvp/backend/tests/conftest.py
Normal file
|
|
@ -0,0 +1,18 @@
|
|||
"""Repo-wide test config for tradein-mvp/backend.
|
||||
|
||||
Currently only registers custom pytest markers so they don't emit
|
||||
PytestUnknownMarkWarning when used (`--strict-markers` is not enabled in
|
||||
pyproject.toml, so an unregistered marker would only warn, not fail — this
|
||||
just keeps output clean and documents intent in one place).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
|
||||
def pytest_configure(config) -> None:
|
||||
config.addinivalue_line(
|
||||
"markers",
|
||||
"pdf_render: real (non-mocked) WeasyPrint render — needs native "
|
||||
"Pango/cairo/GObject libs, self-skips where unavailable (see "
|
||||
"tests/test_pdf_real_render.py docstring for how to run it for real).",
|
||||
)
|
||||
|
|
@ -243,6 +243,25 @@ def _support_chat_settings(monkeypatch: pytest.MonkeyPatch) -> None:
|
|||
monkeypatch.setattr(bridge.settings, "telegram_support_topic_id", SUPPORT_TOPIC_ID)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_flood_limiters(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""F) `_flood_limiter`/`_flood_notify_limiter` — module-level singletons (тот же
|
||||
паттерн, что `_send_limiter` в app/api/v1/support.py); большинство тестов в
|
||||
этом файле шлют сообщения от одного и того же chat_id=555, поэтому без сброса
|
||||
накопленные хиты одного теста бы протекали в следующий и ломали его
|
||||
предположения (тест флуда должен видеть ЧИСТЫЙ бюджет)."""
|
||||
monkeypatch.setattr(
|
||||
bridge,
|
||||
"_flood_limiter",
|
||||
bridge.SlidingWindowLimiter(limit=bridge._FLOOD_LIMIT, window_s=bridge._FLOOD_WINDOW_S),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
bridge,
|
||||
"_flood_notify_limiter",
|
||||
bridge.SlidingWindowLimiter(limit=1, window_s=bridge._FLOOD_WINDOW_S),
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _stop_patches():
|
||||
"""Останавливает httpx.AsyncClient monkeypatch после каждого теста (unittest.mock.patch.start()
|
||||
|
|
@ -427,6 +446,100 @@ async def test_private_message_notifies_client_when_support_chat_unset(
|
|||
assert storage.get_offset() == 14
|
||||
|
||||
|
||||
# ── F) флуд-лимит на отправителя (низкий приоритет) ─────────────────────────
|
||||
|
||||
|
||||
async def test_private_message_flood_limit_blocks_excess_and_notifies_once() -> None:
|
||||
"""Больше `_FLOOD_LIMIT` сообщений от ОДНОГО chat_id за окно — зеркалирование
|
||||
сверх лимита отключается (никакого copyMessage, никакой записи в
|
||||
tg_support_messages — маршрутизировать ответ всё равно нечего без
|
||||
topic_message_id). Клиент получает уведомление о недоставке РОВНО один раз
|
||||
за окно, а не на каждое следующее превышение — иначе само уведомление стало
|
||||
бы вторым источником флуда."""
|
||||
calls: list[tuple[str, dict[str, Any]]] = []
|
||||
client = _make_client({"copyMessage": {"message_id": 900}}, calls)
|
||||
storage = FakeBridgeStorage()
|
||||
|
||||
update_id = 100
|
||||
for i in range(bridge._FLOOD_LIMIT):
|
||||
update = {"update_id": update_id, "message": _private_message(message_id=i + 1)}
|
||||
await bridge.process_update(update, client, storage)
|
||||
update_id += 1
|
||||
|
||||
# Ровно _FLOOD_LIMIT сообщений прошли мирроринг: первое — шапка + зеркало,
|
||||
# остальные — только зеркало.
|
||||
mirrored_calls = [m for m, _ in calls if m == "copyMessage"]
|
||||
assert len(mirrored_calls) == bridge._FLOOD_LIMIT
|
||||
assert len(storage.messages) == bridge._FLOOD_LIMIT
|
||||
|
||||
calls.clear()
|
||||
over_limit_update = {
|
||||
"update_id": update_id,
|
||||
"message": _private_message(message_id=bridge._FLOOD_LIMIT + 1),
|
||||
}
|
||||
await bridge.process_update(over_limit_update, client, storage)
|
||||
update_id += 1
|
||||
|
||||
# Сверх лимита — НЕ зеркалируется, НЕ пишется в лог переписки, клиент
|
||||
# получает уведомление о недоставке (не тихий игнор — клиент не должен
|
||||
# решить, что оператор получил сообщение).
|
||||
assert len(calls) == 1
|
||||
method, payload = calls[0]
|
||||
assert method == "sendMessage"
|
||||
assert payload["chat_id"] == 555
|
||||
assert payload["text"] == bridge.FLOOD_LIMITED_TEXT
|
||||
assert len(storage.messages) == bridge._FLOOD_LIMIT
|
||||
|
||||
calls.clear()
|
||||
second_over_limit_update = {
|
||||
"update_id": update_id,
|
||||
"message": _private_message(message_id=bridge._FLOOD_LIMIT + 2),
|
||||
}
|
||||
await bridge.process_update(second_over_limit_update, client, storage)
|
||||
|
||||
# Повторное превышение в ТОМ ЖЕ окне — уведомление подавлено (не второй
|
||||
# источник флуда), никаких Telegram-вызовов вообще.
|
||||
assert calls == []
|
||||
assert len(storage.messages) == bridge._FLOOD_LIMIT
|
||||
|
||||
|
||||
async def test_private_message_flood_limit_does_not_block_other_client() -> None:
|
||||
"""Флуд-лимит — per-chat_id: клиент А исчерпал свой бюджет, но клиент Б
|
||||
(другой chat_id) продолжает получать зеркалирование как обычно — один
|
||||
флудящий клиент не блокирует доставку сообщений остальным (сама суть
|
||||
задачи — воркер однопоточный, но лимит не даёт флудеру монополизировать
|
||||
его через Telegram 429)."""
|
||||
calls: list[tuple[str, dict[str, Any]]] = []
|
||||
client = _make_client({"copyMessage": {"message_id": 901}}, calls)
|
||||
storage = FakeBridgeStorage()
|
||||
|
||||
flooding_chat_id = 555
|
||||
update_id = 300
|
||||
for i in range(bridge._FLOOD_LIMIT + 2):
|
||||
update = {
|
||||
"update_id": update_id,
|
||||
"message": _private_message(chat_id=flooding_chat_id, message_id=i + 1),
|
||||
}
|
||||
await bridge.process_update(update, client, storage)
|
||||
update_id += 1
|
||||
|
||||
calls.clear()
|
||||
|
||||
other_chat_id = 777001
|
||||
other_update = {
|
||||
"update_id": update_id,
|
||||
"message": _private_message(chat_id=other_chat_id, message_id=1, username="another_client"),
|
||||
}
|
||||
await bridge.process_update(other_update, client, storage)
|
||||
|
||||
methods = [m for m, _ in calls]
|
||||
# Другой клиент получает шапку (первое обращение) + зеркало как обычно —
|
||||
# флуд первого клиента на него не влияет.
|
||||
assert methods == ["sendMessage", "copyMessage"]
|
||||
mirror_call = calls[1][1]
|
||||
assert mirror_call["from_chat_id"] == other_chat_id
|
||||
|
||||
|
||||
# ── B) реплай оператора → user ──────────────────────────────────────────────
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,94 @@
|
|||
"""Audit-scrapers finding 3: Avito detail publish_date year-boundary rollover.
|
||||
|
||||
Avito не показывает год для дат текущего года («20 декабря в 15:30»). Раньше
|
||||
`_extract_meta` всегда брал ТЕКУЩИЙ год момента парсинга — объявлению, опубликованному
|
||||
в декабре и прочитанному в январе следующего года, ставился год парсинга (будущая
|
||||
дата), завышая свежесть лота. Фикс: если получившаяся дата оказалась в будущем
|
||||
относительно момента парсинга — откатываем на год назад.
|
||||
|
||||
Refs: audit-scrapers 2026-07-26, finding 3 (low).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from datetime import date as real_date
|
||||
|
||||
import pytest
|
||||
from selectolax.parser import HTMLParser
|
||||
|
||||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||||
|
||||
from scraper_kit.providers.avito import detail as kit_detail
|
||||
|
||||
|
||||
def _freeze_today(monkeypatch: pytest.MonkeyPatch, frozen: real_date) -> None:
|
||||
"""Подменяет `date` в scraper_kit.providers.avito.detail так, что date.today()
|
||||
детерминированно возвращает `frozen` (date — immutable C-тип, .today нельзя
|
||||
monkeypatch'нуть напрямую — подменяем ссылку на класс в модуле)."""
|
||||
|
||||
class _FrozenDate(real_date):
|
||||
@classmethod
|
||||
def today(cls) -> real_date: # type: ignore[override]
|
||||
return frozen
|
||||
|
||||
monkeypatch.setattr(kit_detail, "date", _FrozenDate)
|
||||
|
||||
|
||||
def _tree_with_publish_text(text: str) -> HTMLParser:
|
||||
html = f'<html><body><div data-marker="item-view/item-id">{text}</div></body></html>'
|
||||
return HTMLParser(html)
|
||||
|
||||
|
||||
def test_december_publish_date_read_in_january_rolls_back_a_year(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Объявление '20 декабря' парсится 5 января СЛЕДУЮЩЕГО года: без фикса
|
||||
дата была бы 2027-12-20 (в будущем относительно today=2027-01-05) — теперь
|
||||
откатывается на 2026-12-20."""
|
||||
_freeze_today(monkeypatch, real_date(2027, 1, 5))
|
||||
tree = _tree_with_publish_text("№ 4291500000 · 20 декабря в 15:30")
|
||||
|
||||
publish_date, _, _ = kit_detail._extract_meta(tree)
|
||||
|
||||
assert publish_date == real_date(2026, 12, 20)
|
||||
|
||||
|
||||
def test_same_year_past_publish_date_not_rolled_back(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Control: дата в прошлом (не будущем) в том же году — год НЕ откатывается."""
|
||||
_freeze_today(monkeypatch, real_date(2027, 1, 5))
|
||||
tree = _tree_with_publish_text("№ 4291500001 · 3 января в 09:00")
|
||||
|
||||
publish_date, _, _ = kit_detail._extract_meta(tree)
|
||||
|
||||
assert publish_date == real_date(2027, 1, 3)
|
||||
|
||||
|
||||
def test_publish_date_equal_to_today_not_rolled_back(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Control: дата ровно = today (не строго будущее) — год НЕ откатывается."""
|
||||
_freeze_today(monkeypatch, real_date(2027, 1, 5))
|
||||
tree = _tree_with_publish_text("№ 4291500002 · 5 января в 12:00")
|
||||
|
||||
publish_date, _, _ = kit_detail._extract_meta(tree)
|
||||
|
||||
assert publish_date == real_date(2027, 1, 5)
|
||||
|
||||
|
||||
def test_mid_year_publish_date_not_rolled_back(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Обычный случай вдали от границы года — поведение не меняется."""
|
||||
_freeze_today(monkeypatch, real_date(2027, 6, 15))
|
||||
tree = _tree_with_publish_text("№ 4291500003 · 20 марта в 10:00")
|
||||
|
||||
publish_date, _, _ = kit_detail._extract_meta(tree)
|
||||
|
||||
assert publish_date == real_date(2027, 3, 20)
|
||||
|
||||
|
||||
def test_no_publish_date_in_text_returns_none(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Regression guard: отсутствие даты в тексте по-прежнему даёт None (не падает)."""
|
||||
_freeze_today(monkeypatch, real_date(2027, 1, 5))
|
||||
tree = _tree_with_publish_text("№ 4291500004")
|
||||
|
||||
publish_date, _, _ = kit_detail._extract_meta(tree)
|
||||
|
||||
assert publish_date is None
|
||||
206
tradein-mvp/backend/tests/test_avito_sweep_dom_drift.py
Normal file
206
tradein-mvp/backend/tests/test_avito_sweep_dom_drift.py
Normal file
|
|
@ -0,0 +1,206 @@
|
|||
"""Audit-scrapers finding 1: Avito citywide/byrooms/exhaustive sweep DOM-drift detection.
|
||||
|
||||
Раньше 0 карточек на page=1 (обход всего города / категории комнатности / ценового
|
||||
бакета exhaustive-сбора) молча трактовалось как «объявлений действительно нет» —
|
||||
неотличимо от content-block/captcha или дрейфа DOM-маркера карточки (`data-marker=
|
||||
"item-*"`). Фикс переиспользует существующий механизм `AvitoContentBlockedError`
|
||||
(см. `fetch_around`, #754/#779) + новый `_is_unexpected_empty_page()` — независимый
|
||||
сигнал `_extract_total_count` (счётчик `page-title/count` либо no-results маркер):
|
||||
|
||||
- page=1, 0 карточек, НЕТ no-results маркера/счётчика → аномалия → raise.
|
||||
- page=1, 0 карточек, ЕСТЬ no-results маркер (total=0) → валидная пустая выборка.
|
||||
- page>1, 0 карточек → всегда graceful end-of-pagination (не regressed).
|
||||
- exhaustive leaf-бакет: probe независимо утверждал total>0, но после пагинации
|
||||
всех страниц собрано 0 карточек → аномалия → raise (даже без per-page проверки
|
||||
внутри _paginate_leaf_bucket, т.к. там нет break-on-empty цикла).
|
||||
|
||||
Refs: audit-scrapers 2026-07-26, finding 1 (medium).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||||
|
||||
from scraper_kit.avito_exceptions import AvitoContentBlockedError
|
||||
from scraper_kit.base import ScrapedLot
|
||||
from scraper_kit.providers.avito.serp import ROOM_SLUGS, AvitoScraper
|
||||
|
||||
from app.services.scraper_adapters import RealScraperConfig
|
||||
|
||||
# HTML "успешно получен, разумного размера", но БЕЗ data-marker="item-*" карточек
|
||||
# И без no-results маркера/счётчика — неотличимо от content-block/DOM-drift.
|
||||
_NO_MARKER_HTML = "<html><body>" + ("x" * 500) + "</body></html>"
|
||||
|
||||
# Валидная пустая выборка: no-results маркер присутствует (_AVITO_NO_RESULTS_MARKERS).
|
||||
_NO_RESULTS_HTML = (
|
||||
"<html><body>По вашему запросу ничего не найдено. Попробуйте изменить фильтры."
|
||||
+ ("y" * 200)
|
||||
+ "</body></html>"
|
||||
)
|
||||
|
||||
# Firewall/captcha-страница (переиспользуем существующий fixture-паттерн из #754) —
|
||||
# используется только для проверки, что page>1 остаётся graceful независимо от
|
||||
# содержимого (проверка применяется ТОЛЬКО к page==1).
|
||||
_BLOCKPAGE_HTML = "<html><body><h1>Доступ ограничен</h1></body></html>"
|
||||
|
||||
|
||||
def _make_lot(source_id: str) -> ScrapedLot:
|
||||
return ScrapedLot(
|
||||
source="avito",
|
||||
source_url=f"https://www.avito.ru/ekaterinburg/kvartiry/{source_id}",
|
||||
source_id=source_id,
|
||||
price_rub=6_000_000,
|
||||
)
|
||||
|
||||
|
||||
# ── fetch_city_wide (_paginate_sweep) ────────────────────────────────────────
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_citywide_page1_zero_cards_no_marker_raises() -> None:
|
||||
s = AvitoScraper(RealScraperConfig())
|
||||
with patch.object(s, "_fetch_serp_html", AsyncMock(return_value=_NO_MARKER_HTML)):
|
||||
with pytest.raises(AvitoContentBlockedError):
|
||||
await s.fetch_city_wide(pages=5, delay_override_sec=0)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_citywide_page1_zero_cards_with_no_results_marker_is_valid_empty() -> None:
|
||||
s = AvitoScraper(RealScraperConfig())
|
||||
with patch.object(s, "_fetch_serp_html", AsyncMock(return_value=_NO_RESULTS_HTML)):
|
||||
result = await s.fetch_city_wide(pages=5, delay_override_sec=0)
|
||||
assert result == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_citywide_page_gt1_zero_cards_stays_graceful() -> None:
|
||||
"""page=1 реально возвращает карточки (mock _parse_html) — page=2 пустой
|
||||
firewall-текст без карточек НЕ должен поднимать исключение (только page==1)."""
|
||||
s = AvitoScraper(RealScraperConfig())
|
||||
call_n = 0
|
||||
|
||||
async def _fetch(url: str, page: int) -> str:
|
||||
return "<html>page1</html>" if page == 1 else _BLOCKPAGE_HTML
|
||||
|
||||
def _parse(html: str, source_url_base: str) -> list[ScrapedLot]:
|
||||
nonlocal call_n
|
||||
call_n += 1
|
||||
return [_make_lot("A"), _make_lot("B")] if call_n == 1 else []
|
||||
|
||||
with patch.object(s, "_fetch_serp_html", AsyncMock(side_effect=_fetch)):
|
||||
with patch.object(s, "_parse_html", side_effect=_parse):
|
||||
with patch.object(s, "sleep_between_requests", AsyncMock(return_value=None)):
|
||||
result = await s.fetch_city_wide(pages=5, delay_override_sec=0)
|
||||
|
||||
assert len(result) == 2
|
||||
assert call_n == 2 # page1(2 lots) + page2(0 lots) → stop, no raise
|
||||
|
||||
|
||||
# ── fetch_by_rooms ────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_byrooms_category_page1_zero_cards_no_marker_raises() -> None:
|
||||
s = AvitoScraper(RealScraperConfig())
|
||||
with patch.object(s, "_fetch_serp_html", AsyncMock(return_value=_NO_MARKER_HTML)):
|
||||
with pytest.raises(AvitoContentBlockedError):
|
||||
await s.fetch_by_rooms(pages=5, delay_override_sec=0, room_slugs=ROOM_SLUGS[:1])
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_byrooms_category_page1_zero_cards_with_marker_is_valid_empty() -> None:
|
||||
s = AvitoScraper(RealScraperConfig())
|
||||
with patch.object(s, "_fetch_serp_html", AsyncMock(return_value=_NO_RESULTS_HTML)):
|
||||
result = await s.fetch_by_rooms(pages=5, delay_override_sec=0, room_slugs=ROOM_SLUGS[:1])
|
||||
assert result == []
|
||||
|
||||
|
||||
# ── _paginate_leaf_bucket (exhaustive/fetch_all_secondary) ───────────────────
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_leaf_bucket_expected_total_positive_but_zero_parsed_raises() -> None:
|
||||
"""Probe независимо утверждал total=5 (bucket не может быть легитимно пустым),
|
||||
но парсинг всех страниц дал 0 карточек — DOM-drift, не пустой бакет."""
|
||||
s = AvitoScraper(RealScraperConfig())
|
||||
seen: dict[str, ScrapedLot] = {}
|
||||
|
||||
with patch.object(s, "_parse_html", return_value=[]):
|
||||
with pytest.raises(AvitoContentBlockedError):
|
||||
await s._paginate_leaf_bucket(
|
||||
room_slug="studii-ASgBAgICAUSSA8YQ",
|
||||
room_label="studio",
|
||||
lo=0,
|
||||
hi=3_000_000,
|
||||
html="<html>probe-page-1</html>",
|
||||
max_pages=1,
|
||||
seen=seen,
|
||||
price_cap_per_bucket=1400,
|
||||
max_pages_per_bucket=100,
|
||||
concurrency=5,
|
||||
secondary_only=True,
|
||||
on_bucket=None,
|
||||
skip_buckets=None,
|
||||
expected_total=5,
|
||||
)
|
||||
assert seen == {}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_leaf_bucket_expected_total_none_zero_parsed_no_raise() -> None:
|
||||
"""Probe провалился (expected_total=None, best-effort пагинация) — 0 карточек
|
||||
здесь НЕ аномалия (мы не знаем, есть ли реально данные в бакете)."""
|
||||
s = AvitoScraper(RealScraperConfig())
|
||||
seen: dict[str, ScrapedLot] = {}
|
||||
|
||||
with patch.object(s, "_parse_html", return_value=[]):
|
||||
# Не должно поднимать исключение.
|
||||
await s._paginate_leaf_bucket(
|
||||
room_slug="studii-ASgBAgICAUSSA8YQ",
|
||||
room_label="studio",
|
||||
lo=0,
|
||||
hi=3_000_000,
|
||||
html=None,
|
||||
max_pages=1,
|
||||
seen=seen,
|
||||
price_cap_per_bucket=1400,
|
||||
max_pages_per_bucket=100,
|
||||
concurrency=5,
|
||||
secondary_only=True,
|
||||
on_bucket=None,
|
||||
skip_buckets=None,
|
||||
expected_total=None,
|
||||
)
|
||||
assert seen == {}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_leaf_bucket_expected_total_matches_collected_no_raise() -> None:
|
||||
"""Нормальный путь: probe total=2, парсинг реально даёт 2 карточки — не аномалия."""
|
||||
s = AvitoScraper(RealScraperConfig())
|
||||
seen: dict[str, ScrapedLot] = {}
|
||||
lots = [_make_lot("L1"), _make_lot("L2")]
|
||||
|
||||
with patch.object(s, "_parse_html", return_value=lots):
|
||||
await s._paginate_leaf_bucket(
|
||||
room_slug="studii-ASgBAgICAUSSA8YQ",
|
||||
room_label="studio",
|
||||
lo=0,
|
||||
hi=3_000_000,
|
||||
html="<html>probe-page-1</html>",
|
||||
max_pages=1,
|
||||
seen=seen,
|
||||
price_cap_per_bucket=1400,
|
||||
max_pages_per_bucket=100,
|
||||
concurrency=5,
|
||||
secondary_only=True,
|
||||
on_bucket=None,
|
||||
skip_buckets=None,
|
||||
expected_total=2,
|
||||
)
|
||||
assert set(seen.keys()) == {"L1", "L2"}
|
||||
|
|
@ -0,0 +1,106 @@
|
|||
"""Audit-scrapers finding 2: Cian totalOffers vs results.offers length mismatch.
|
||||
|
||||
`_parse_serp_html` извлекает `totalOffers` и `results.offers` из ОДНОГО Redux
|
||||
state-блоба (одна SSR-выдача). Раньше `results.offers` пустой при `totalOffers>0`
|
||||
логировался WARNING'ом и тихо возвращался `[]` — не считался schema-regression, не
|
||||
попадал в мониторинг (`_report_schema_regression`/Glitchtip).
|
||||
|
||||
Порог: 0 vs >0 — единственный позиционно-независимый сигнал, который можно
|
||||
проверить без номера страницы внутри `_parse_serp_html` (эта функция не знает,
|
||||
какая это страница пагинации — дробный порог типа "< 50% от totalOffers" ложно
|
||||
сработал бы на легитимной последней частичной странице exhaustive-пагинации,
|
||||
которую эта функция не различает). totalOffers=0 (реально пустой поиск) НЕ
|
||||
считается регрессией.
|
||||
|
||||
Refs: audit-scrapers 2026-07-26, finding 2 (low).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||||
|
||||
from scraper_kit.providers.cian.serp import CianScraper
|
||||
|
||||
from app.services.scraper_adapters import RealScraperConfig
|
||||
|
||||
|
||||
def _scraper() -> CianScraper:
|
||||
return CianScraper(RealScraperConfig())
|
||||
|
||||
|
||||
def test_total_offers_positive_but_offers_empty_reports_regression() -> None:
|
||||
"""totalOffers=5, results.offers=[] — internal contradiction, must report."""
|
||||
s = _scraper()
|
||||
state = {"results": {"totalOffers": 5, "offers": []}}
|
||||
with patch("scraper_kit.providers.cian.serp.extract_state", return_value=state):
|
||||
with patch.object(s, "_report_schema_regression") as mock_report:
|
||||
lots = s._parse_serp_html("<html>irrelevant</html>")
|
||||
|
||||
assert lots == []
|
||||
mock_report.assert_called_once()
|
||||
(msg,), _ = mock_report.call_args
|
||||
assert "totalOffers=5" in msg
|
||||
|
||||
|
||||
def test_total_offers_zero_and_offers_empty_is_valid_empty_search() -> None:
|
||||
"""totalOffers=0, offers=[] — легитимная пустая выборка, НЕ регрессия."""
|
||||
s = _scraper()
|
||||
state = {"results": {"totalOffers": 0, "offers": []}}
|
||||
with patch("scraper_kit.providers.cian.serp.extract_state", return_value=state):
|
||||
with patch.object(s, "_report_schema_regression") as mock_report:
|
||||
lots = s._parse_serp_html("<html>irrelevant</html>")
|
||||
|
||||
assert lots == []
|
||||
mock_report.assert_not_called()
|
||||
|
||||
|
||||
def test_total_offers_none_and_offers_empty_is_not_reported_as_regression() -> None:
|
||||
"""totalOffers отсутствует/None в state — недостаточно сигнала для regression-репорта
|
||||
(могла быть частично битая state-структура без явного totalOffers>0 контр-сигнала)."""
|
||||
s = _scraper()
|
||||
state = {"results": {"offers": []}}
|
||||
with patch("scraper_kit.providers.cian.serp.extract_state", return_value=state):
|
||||
with patch.object(s, "_report_schema_regression") as mock_report:
|
||||
lots = s._parse_serp_html("<html>irrelevant</html>")
|
||||
|
||||
assert lots == []
|
||||
mock_report.assert_not_called()
|
||||
|
||||
|
||||
def test_offers_present_normal_path_unaffected() -> None:
|
||||
"""totalOffers=1, offers содержит 1 запись без cianId/id — не проходит
|
||||
_offer_to_lot, но это уже существующая (0/N offer-level) охрана, не finding 2."""
|
||||
s = _scraper()
|
||||
state = {"results": {"totalOffers": 1, "offers": [{"noId": True}]}}
|
||||
with patch("scraper_kit.providers.cian.serp.extract_state", return_value=state):
|
||||
with patch.object(s, "_report_schema_regression") as mock_report:
|
||||
lots = s._parse_serp_html("<html>irrelevant</html>")
|
||||
|
||||
# offers_data непустой → finding 2 guard не участвует; существующая offer-level
|
||||
# охрана (raw_count>0 and saved_count==0) должна отработать вместо неё.
|
||||
assert lots == []
|
||||
mock_report.assert_called_once()
|
||||
(msg,), _ = mock_report.call_args
|
||||
assert "_offer_to_lot" in msg
|
||||
|
||||
|
||||
def test_state_none_extraction_failed_no_regression_report() -> None:
|
||||
"""extract_state вернул None (captcha/структура целиком не найдена) — уже
|
||||
существующая ветка, НЕ должна триггерить finding-2 regression report."""
|
||||
s = _scraper()
|
||||
with patch("scraper_kit.providers.cian.serp.extract_state", return_value=None):
|
||||
with patch.object(s, "_report_schema_regression") as mock_report:
|
||||
lots = s._parse_serp_html("<html>irrelevant</html>")
|
||||
|
||||
assert lots == []
|
||||
mock_report.assert_not_called()
|
||||
|
||||
|
||||
def test_report_schema_regression_swallows_missing_glitchtip_dsn() -> None:
|
||||
"""_report_schema_regression не должен падать, если glitchtip_dsn не настроен."""
|
||||
s = _scraper()
|
||||
s._config = MagicMock(glitchtip_dsn=None)
|
||||
s._report_schema_regression("test message") # не должно бросить исключение
|
||||
|
|
@ -10,6 +10,8 @@ import pytest
|
|||
from app.services.cian_session import (
|
||||
CIAN_REQUIRED_COOKIES,
|
||||
VERIFY_BAN_SENTINEL,
|
||||
VERIFY_MARKUP_CHANGED_SENTINEL,
|
||||
VERIFY_SOURCE_UNAVAILABLE_SENTINEL,
|
||||
_classify_verify_response,
|
||||
load_session,
|
||||
mark_session_invalid,
|
||||
|
|
@ -211,14 +213,40 @@ def test_classify_200_authenticated_returns_state(monkeypatch: pytest.MonkeyPatc
|
|||
assert result == expected
|
||||
|
||||
|
||||
def test_classify_200_state_missing_returns_none(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""200 but extract_state returns None → None."""
|
||||
def test_classify_200_state_missing_returns_markup_changed_sentinel(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""audit-scrapers finding 4: HTTP 200 но extract_state не нашёл auth-state
|
||||
(Cian сменил вёрстку/MFE-схему header-frontend) → VERIFY_MARKUP_CHANGED_SENTINEL,
|
||||
НЕ None. Раньше это конфлировалось с "cookies expired" (реальный логаут)."""
|
||||
monkeypatch.setattr(
|
||||
"app.services.cian_session.extract_state",
|
||||
lambda html, mfe, key: None,
|
||||
)
|
||||
result = _classify_verify_response(200, "<html></html>")
|
||||
assert result is None
|
||||
assert result is VERIFY_MARKUP_CHANGED_SENTINEL
|
||||
assert result is not None # НЕ должно триггерить cookie-refresh alert
|
||||
|
||||
|
||||
def test_classify_5xx_returns_source_unavailable_sentinel() -> None:
|
||||
"""audit-scrapers finding 4: HTTP 500 (источник недоступен) →
|
||||
VERIFY_SOURCE_UNAVAILABLE_SENTINEL, НЕ None (cookies тут ни при чём)."""
|
||||
result = _classify_verify_response(500, None)
|
||||
assert result is VERIFY_SOURCE_UNAVAILABLE_SENTINEL
|
||||
assert result is not None
|
||||
|
||||
|
||||
def test_classify_502_returns_source_unavailable_sentinel() -> None:
|
||||
"""Любой non-200/403/401 статус (напр. 502 bad gateway) — источник недоступен."""
|
||||
result = _classify_verify_response(502, None)
|
||||
assert result is VERIFY_SOURCE_UNAVAILABLE_SENTINEL
|
||||
|
||||
|
||||
def test_classify_status_200_html_none_returns_source_unavailable_sentinel() -> None:
|
||||
"""Defensive: status=200 но html=None (не должно случаться в проде, но
|
||||
classifier не должен молча вернуть None='expired') → source-unavailable."""
|
||||
result = _classify_verify_response(200, None)
|
||||
assert result is VERIFY_SOURCE_UNAVAILABLE_SENTINEL
|
||||
|
||||
|
||||
def test_classify_403_is_distinct_from_401() -> None:
|
||||
|
|
@ -230,6 +258,28 @@ def test_classify_403_is_distinct_from_401() -> None:
|
|||
assert expired is None
|
||||
|
||||
|
||||
def test_classify_all_four_outcomes_are_mutually_distinct() -> None:
|
||||
"""audit-scrapers finding 4: expired (401) / ban (403) / source-unavailable (5xx)
|
||||
/ markup-changed (200+extract_state=None) — четыре РАЗНЫХ сигнала, ни один не
|
||||
коллапсирует в другой. Только expired (None) должен триггерить re-login alert."""
|
||||
expired = _classify_verify_response(401, None)
|
||||
ban = _classify_verify_response(403, None)
|
||||
source_down = _classify_verify_response(500, None)
|
||||
|
||||
with pytest.MonkeyPatch.context() as mp:
|
||||
mp.setattr("app.services.cian_session.extract_state", lambda html, mfe, key: None)
|
||||
markup_changed = _classify_verify_response(200, "<html></html>")
|
||||
|
||||
outcomes = [expired, ban, source_down, markup_changed]
|
||||
# None встречается ровно один раз (только expired) — остальные три truthy sentinel'а
|
||||
# и все различны между собой (identity, не equality — это разные dict-объекты).
|
||||
assert outcomes.count(None) == 1
|
||||
assert expired is None
|
||||
non_none = [o for o in outcomes if o is not None]
|
||||
assert len(non_none) == 3
|
||||
assert len({id(o) for o in non_none}) == 3
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# verify_session (async) — integration with curl_cffi mock
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -317,10 +367,12 @@ async def test_verify_session_not_authenticated_returns_none(
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_verify_session_state_missing_returns_none(
|
||||
async def test_verify_session_state_missing_returns_markup_changed_sentinel(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""200 + extract_state returns None → None."""
|
||||
"""audit-scrapers finding 4: 200 + extract_state returns None (markup changed)
|
||||
→ VERIFY_MARKUP_CHANGED_SENTINEL, НЕ None. Раньше ложно триггерило "cookies
|
||||
expired, please re-upload" для реальной причины "Cian сменил вёрстку"."""
|
||||
monkeypatch.setattr(
|
||||
"app.services.cian_session.extract_state",
|
||||
lambda html, mfe, key: None,
|
||||
|
|
@ -334,7 +386,42 @@ async def test_verify_session_state_missing_returns_none(
|
|||
with patch("app.services.cian_session.AsyncSession", return_value=mock_session):
|
||||
result = await verify_session({"DMIR_AUTH": "x"})
|
||||
|
||||
assert result is None
|
||||
assert result is VERIFY_MARKUP_CHANGED_SENTINEL
|
||||
assert result is not None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_verify_session_5xx_returns_source_unavailable_sentinel() -> None:
|
||||
"""audit-scrapers finding 4: HTTP 500 → VERIFY_SOURCE_UNAVAILABLE_SENTINEL,
|
||||
НЕ None. Источник временно недоступен — cookies тут ни при чём, вызывающий
|
||||
не должен помечать сессию invalid / просить re-upload."""
|
||||
mock_session = AsyncMock()
|
||||
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
||||
mock_session.__aexit__ = AsyncMock(return_value=None)
|
||||
mock_session.get = AsyncMock(return_value=_make_cffi_resp(500))
|
||||
|
||||
with patch("app.services.cian_session.AsyncSession", return_value=mock_session):
|
||||
result = await verify_session({"DMIR_AUTH": "x"})
|
||||
|
||||
assert result is VERIFY_SOURCE_UNAVAILABLE_SENTINEL
|
||||
assert result is not None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_verify_session_network_error_returns_source_unavailable_sentinel() -> None:
|
||||
"""audit-scrapers finding 4: сетевой/транспортный сбой (timeout, connection
|
||||
reset и т.п.) → VERIFY_SOURCE_UNAVAILABLE_SENTINEL, НЕ None. Раньше generic
|
||||
except возвращал None — конфлировал сетевой сбой с протухшими cookies."""
|
||||
mock_session = AsyncMock()
|
||||
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
||||
mock_session.__aexit__ = AsyncMock(return_value=None)
|
||||
mock_session.get = AsyncMock(side_effect=ConnectionError("connection reset by peer"))
|
||||
|
||||
with patch("app.services.cian_session.AsyncSession", return_value=mock_session):
|
||||
result = await verify_session({"DMIR_AUTH": "x"})
|
||||
|
||||
assert result is VERIFY_SOURCE_UNAVAILABLE_SENTINEL
|
||||
assert result is not None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -2,13 +2,16 @@
|
|||
|
||||
Backend доверяет X-Authenticated-User (его ставит Caddy). На общей docker-сети
|
||||
gendesign_shared любой контейнер мог бы отправить поддельный
|
||||
`X-Authenticated-User: admin` напрямую на tradein-backend:8000. Общий секрет
|
||||
X-Internal-Auth-Secret закрывает дыру: если TRADEIN_INTERNAL_AUTH_SECRET задан,
|
||||
каждый запрос с X-Authenticated-User обязан нести валидный секрет (constant-time),
|
||||
`X-Authenticated-User: admin` напрямую на tradein-backend:8000, минуя Caddy.
|
||||
Общий секрет X-Internal-Auth-Secret закрывает дыру: если TRADEIN_INTERNAL_AUTH_SECRET
|
||||
задан, каждый запрос с X-Authenticated-User обязан нести валидный секрет (constant-time),
|
||||
иначе 401. Пусто = fail-open (backward-compat до провижининга).
|
||||
|
||||
MIRROR of rbac_guard из app/main.py — включая #2213 secret-gate. Держим копию
|
||||
здесь (как test_rbac.py), чтобы не тянуть тяжёлый app.main (lifespan/DB/scheduler).
|
||||
Использует РЕАЛЬНЫЙ rbac_guard (app/core/rbac.py) — тот же, что регистрирует
|
||||
app/main.py в проде. Раньше здесь была hand-maintained копия ("MIRROR of
|
||||
rbac_guard из app/main.py"); соседний test_rbac.py держал СВОЮ отдельную
|
||||
копию, которая успела отстать (потеряла именно этот secret-gate) — регрессия
|
||||
в реальном guard'е могла бы пройти CI незамеченной. См. app/core/rbac.py.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -17,20 +20,13 @@ import os
|
|||
|
||||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||||
|
||||
import re
|
||||
import secrets
|
||||
from collections.abc import Awaitable, Callable
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI, Request
|
||||
from fastapi.responses import JSONResponse, Response
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app.core import auth as auth_mod
|
||||
from app.core import config
|
||||
|
||||
_ADMIN_API_RE = re.compile(r"^/api/v1/admin/")
|
||||
_PUBLIC_PATHS = frozenset({"/health", "/docs", "/redoc", "/openapi.json"})
|
||||
from app.core.rbac import rbac_guard
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
|
|
@ -39,44 +35,9 @@ def _reset_auth_cache() -> None:
|
|||
|
||||
|
||||
def _build_test_app() -> FastAPI:
|
||||
"""Копия rbac_guard из app/main.py (с #2213 secret-gate)."""
|
||||
"""Test app используя РЕАЛЬНЫЙ rbac_guard (с #2213 secret-gate)."""
|
||||
app = FastAPI()
|
||||
|
||||
@app.middleware("http")
|
||||
async def rbac_guard(
|
||||
request: Request,
|
||||
call_next: Callable[[Request], Awaitable[Response]],
|
||||
) -> Response:
|
||||
path = request.url.path
|
||||
if path in _PUBLIC_PATHS:
|
||||
return await call_next(request)
|
||||
|
||||
username = request.headers.get("X-Authenticated-User")
|
||||
if not username:
|
||||
return JSONResponse(
|
||||
status_code=401,
|
||||
content={"detail": "no authenticated user (Caddy basic_auth required)"},
|
||||
)
|
||||
|
||||
secret = config.settings.tradein_internal_auth_secret
|
||||
if secret:
|
||||
provided = request.headers.get("X-Internal-Auth-Secret", "")
|
||||
if not secrets.compare_digest(provided, secret):
|
||||
return JSONResponse(
|
||||
status_code=401,
|
||||
content={"detail": "invalid or missing internal auth secret"},
|
||||
)
|
||||
|
||||
try:
|
||||
role = auth_mod.get_role(username)
|
||||
except KeyError:
|
||||
return JSONResponse(
|
||||
status_code=403,
|
||||
content={"detail": "user not in roles config"},
|
||||
)
|
||||
if _ADMIN_API_RE.match(path) and role != "admin":
|
||||
return JSONResponse(status_code=403, content={"detail": "admin only"})
|
||||
return await call_next(request)
|
||||
app.middleware("http")(rbac_guard)
|
||||
|
||||
@app.get("/api/v1/ping")
|
||||
async def ping() -> dict:
|
||||
|
|
|
|||
209
tradein-mvp/backend/tests/test_pdf_real_render.py
Normal file
209
tradein-mvp/backend/tests/test_pdf_real_render.py
Normal file
|
|
@ -0,0 +1,209 @@
|
|||
"""Real (non-mocked) WeasyPrint render invariants for the Trade-In PDF report.
|
||||
|
||||
tests/test_pdf_security.py exercises only the HTML-builder layer with
|
||||
WeasyPrint stubbed out (``sys.modules['weasyprint'] = MagicMock()``) — by
|
||||
design, so those tests run everywhere without needing WeasyPrint's native
|
||||
GTK/Pango/cairo libs. That design has a blind spot: it can NEVER catch a real
|
||||
WeasyPrint pagination regression — e.g. the bug fixed in commit 42a50cf8
|
||||
("running @page header/footer → ровно 4 страницы (без пустых)"), where the
|
||||
persistent running header/footer produced an extra TRAILING BLANK 5th page
|
||||
instead of the documented "Структура отчёта — 4 страницы" invariant (see
|
||||
app/services/exporters/trade_in_pdf.py module docstring). A mocked
|
||||
WeasyPrint can never lay out/paginate anything, so it structurally cannot see
|
||||
that class of bug — only a real render can.
|
||||
|
||||
Requires WeasyPrint's native dependencies (Pango/cairo/GObject). These are
|
||||
NOT installed in this dev sandbox (Windows, no libgobject-2.0-0) and NOT
|
||||
installed on the bare ``ubuntu-latest`` runner used by
|
||||
.forgejo/workflows/ci-tradein.yml (no apt-get step there — see that file).
|
||||
This whole module self-skips via ``pytest.skip(..., allow_module_level=True)``
|
||||
the moment the real `import weasyprint` fails for ANY reason (missing native
|
||||
lib, etc.), so it is a harmless no-op everywhere it can't actually run.
|
||||
|
||||
How to run it for real:
|
||||
- Inside the prod/dev docker image — the `runner` stage of
|
||||
tradein-mvp/backend/Dockerfile installs libcairo2 + libpango-1.0-0 +
|
||||
libpangoft2-1.0-0 + fonts-dejavu-core, so WeasyPrint's native deps are
|
||||
present there:
|
||||
docker exec tradein-backend python -m pytest -q -m pdf_render \
|
||||
tests/test_pdf_real_render.py
|
||||
- Locally on a Linux box with WeasyPrint's system deps installed (see
|
||||
https://doc.courtbouillon.org/weasyprint/stable/first_steps.html):
|
||||
uv run pytest -q -m pdf_render tests/test_pdf_real_render.py
|
||||
- Marked ``@pytest.mark.pdf_render`` (registered in tests/conftest.py) so it
|
||||
can be explicitly selected/excluded once a CI runner with the native libs
|
||||
exists; today ci-tradein.yml's `ubuntu-latest` runner doesn't have them,
|
||||
so this module simply self-skips there — nothing to deselect.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
|
||||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||||
|
||||
# test_pdf_security.py deliberately does `sys.modules['weasyprint'] = MagicMock()`
|
||||
# for its own unit tests. sys.modules is a process-global cache: if that module
|
||||
# was imported before this one in the same pytest run, `import weasyprint` below
|
||||
# would silently return the Mock instead of the real package. Purge any
|
||||
# existing stub before attempting the real import — independent of collection
|
||||
# order across test modules.
|
||||
for _name in [n for n in sys.modules if n == "weasyprint" or n.startswith("weasyprint.")]:
|
||||
del sys.modules[_name]
|
||||
|
||||
import pytest # noqa: E402
|
||||
|
||||
try:
|
||||
import weasyprint
|
||||
except Exception as exc: # pragma: no cover - env-dependent (native GTK/Pango/cairo libs)
|
||||
pytest.skip(
|
||||
f"WeasyPrint native deps unavailable, skipping real-render tests: {exc}",
|
||||
allow_module_level=True,
|
||||
)
|
||||
|
||||
pytestmark = pytest.mark.pdf_render
|
||||
|
||||
import unittest.mock as _mock # noqa: E402
|
||||
from datetime import UTC, datetime, timedelta # noqa: E402
|
||||
from uuid import uuid4 # noqa: E402
|
||||
|
||||
from app.schemas.trade_in import AggregatedEstimate # noqa: E402
|
||||
from app.services.brand import Brand # noqa: E402
|
||||
from app.services.exporters import trade_in_pdf as mod # noqa: E402
|
||||
|
||||
_GENERIC = Brand(
|
||||
slug="generic",
|
||||
name="Trade-In",
|
||||
logo_url=None,
|
||||
primary_color="#1d4ed8",
|
||||
accent_color="#f59e0b",
|
||||
footer_text=None,
|
||||
pdf_disclaimer=None,
|
||||
)
|
||||
|
||||
_SNAPSHOT = {
|
||||
"address": "Екатеринбург, ул. Ленина, 1",
|
||||
"area_m2": 50.0,
|
||||
"rooms": 2,
|
||||
"floor": 3,
|
||||
"total_floors": 9,
|
||||
"year_built": 2010,
|
||||
"house_type": "panel",
|
||||
"repair_state": "standard",
|
||||
"has_balcony": True,
|
||||
}
|
||||
|
||||
|
||||
def _estimate(**overrides) -> AggregatedEstimate:
|
||||
base = dict(
|
||||
estimate_id=uuid4(),
|
||||
median_price_rub=10_000_000,
|
||||
range_low_rub=9_000_000,
|
||||
range_high_rub=11_000_000,
|
||||
median_price_per_m2=200_000,
|
||||
confidence="high",
|
||||
n_analogs=15,
|
||||
period_months=24,
|
||||
analogs=[],
|
||||
actual_deals=[],
|
||||
expires_at=datetime.now(UTC) + timedelta(days=30),
|
||||
)
|
||||
base.update(overrides)
|
||||
return AggregatedEstimate(**base)
|
||||
|
||||
|
||||
def _zero_estimate(**overrides) -> AggregatedEstimate:
|
||||
"""Оценка с median=0 — insufficient_data=True (одностраничный empty-state)."""
|
||||
base = dict(
|
||||
estimate_id=uuid4(),
|
||||
median_price_rub=0,
|
||||
range_low_rub=0,
|
||||
range_high_rub=0,
|
||||
median_price_per_m2=0,
|
||||
confidence="low",
|
||||
n_analogs=0,
|
||||
period_months=24,
|
||||
analogs=[],
|
||||
actual_deals=[],
|
||||
expires_at=datetime.now(UTC) + timedelta(days=30),
|
||||
)
|
||||
base.update(overrides)
|
||||
return AggregatedEstimate(**base)
|
||||
|
||||
|
||||
def _render_real_document(estimate, snapshot, brand):
|
||||
"""Call the REAL generate_trade_in_pdf (no mocking of WeasyPrint), capturing
|
||||
the actual weasyprint.HTML/CSS instances + html string it constructs via
|
||||
thin capturing subclasses — so we can additionally call .render() on the
|
||||
exact same HTML instance to get a weasyprint.Document (whose .pages is the
|
||||
public, documented page-count API), without duplicating trade_in_pdf.py's
|
||||
own html_str/css_str assembly logic here (that would drift out of sync
|
||||
with the real function, defeating the point of this test).
|
||||
|
||||
Returns (document, pdf_bytes, html_string).
|
||||
"""
|
||||
html_instances: list[weasyprint.HTML] = []
|
||||
css_instances: list[weasyprint.CSS] = []
|
||||
html_strings: list[str] = []
|
||||
|
||||
class _CapturingHTML(weasyprint.HTML):
|
||||
def __init__(self, *a, **kw):
|
||||
html_strings.append(kw.get("string") if "string" in kw else (a[0] if a else None))
|
||||
super().__init__(*a, **kw)
|
||||
html_instances.append(self)
|
||||
|
||||
class _CapturingCSS(weasyprint.CSS):
|
||||
def __init__(self, *a, **kw):
|
||||
super().__init__(*a, **kw)
|
||||
css_instances.append(self)
|
||||
|
||||
with (
|
||||
_mock.patch.object(weasyprint, "HTML", _CapturingHTML),
|
||||
_mock.patch.object(weasyprint, "CSS", _CapturingCSS),
|
||||
):
|
||||
pdf_bytes = mod.generate_trade_in_pdf(estimate, snapshot, brand=brand)
|
||||
|
||||
assert html_instances, "HTML() was never constructed by generate_trade_in_pdf"
|
||||
assert css_instances, "CSS() was never constructed by generate_trade_in_pdf"
|
||||
document = html_instances[-1].render(stylesheets=[css_instances[-1]])
|
||||
return document, pdf_bytes, html_strings[-1]
|
||||
|
||||
|
||||
def test_real_pdf_has_exactly_4_pages_no_trailing_blank() -> None:
|
||||
"""Regression for commit 42a50cf8: the running @page header/footer must
|
||||
NOT produce a 5th trailing blank page. trade_in_pdf.py's own module
|
||||
docstring documents "Структура отчёта — 4 страницы" as the fixed
|
||||
cover/listings/deals/offer composition — that count IS the authoritative
|
||||
"no empty pages" invariant for this report (any blank page shows up as an
|
||||
extra page beyond these 4 named sections)."""
|
||||
est = _estimate(n_analogs=12, sources_used=["avito"])
|
||||
document, pdf_bytes, _html_str = _render_real_document(est, _SNAPSHOT, _GENERIC)
|
||||
assert len(document.pages) == 4, (
|
||||
f"expected exactly 4 pages (cover/listings/deals/offer), got "
|
||||
f"{len(document.pages)} — likely a trailing/leading blank-page regression"
|
||||
)
|
||||
assert pdf_bytes.startswith(b"%PDF-"), "write_pdf must produce a real PDF"
|
||||
|
||||
|
||||
def test_real_pdf_insufficient_data_is_single_page() -> None:
|
||||
"""insufficient_data=True → one-page empty-state, not the 4-page report."""
|
||||
est = _zero_estimate()
|
||||
document, _pdf_bytes, html_str = _render_real_document(est, _SNAPSHOT, _GENERIC)
|
||||
assert len(document.pages) == 1
|
||||
assert "Недостаточно данных" in html_str
|
||||
|
||||
|
||||
def test_real_pdf_key_blocks_present_exactly_once() -> None:
|
||||
"""Key section headings for each of the 4 pages are present in the actual
|
||||
composed HTML that WeasyPrint rendered (integration-level check — the
|
||||
mocked tests in test_pdf_security.py already verify each builder function
|
||||
in isolation; this catches a composition bug where wiring them together
|
||||
in generate_trade_in_pdf silently drops or duplicates a section)."""
|
||||
est = _estimate(n_analogs=12, sources_used=["avito"])
|
||||
document, _pdf_bytes, html_str = _render_real_document(est, _SNAPSHOT, _GENERIC)
|
||||
assert len(document.pages) == 4
|
||||
markers = ["РЫНОК КВАРТИР", "ФОРМИРОВАНИЕ ВЫКУПНОЙ СТОИМОСТИ"]
|
||||
for marker in markers:
|
||||
found = html_str.count(marker)
|
||||
assert found == 1, f"expected exactly one {marker!r} block, found {found}"
|
||||
|
|
@ -1,7 +1,5 @@
|
|||
"""RBAC unit + integration tests for tradein backend.
|
||||
|
||||
MIRROR of backend/tests/test_rbac.py — kept in sync manually.
|
||||
|
||||
Coverage:
|
||||
- get_role / get_user_scope happy + error paths
|
||||
- is_path_allowed glob semantics (admin everywhere, pilot blocked from /admin/**)
|
||||
|
|
@ -14,22 +12,29 @@ Coverage:
|
|||
|
||||
Тесты используют изолированный FastAPI app (см. _build_test_app), чтобы не
|
||||
тянуть тяжёлые модули из app.main (lifespan task + DB session + scheduler).
|
||||
RBAC middleware и /me router импортируются напрямую — это даёт нам ровно
|
||||
тот же поведение что в проде.
|
||||
Guard больше НЕ мокается/копируется вручную — импортируется тот же
|
||||
``app.core.rbac.rbac_guard``, что регистрирует app/main.py в проде. Раньше
|
||||
здесь была hand-maintained "MIRROR of app.main" копия, которая незаметно
|
||||
отстала (не имела #2213 X-Internal-Auth-Secret check) — правки реального
|
||||
guard'а тесты бы не заметили. См. app/core/rbac.py.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from collections.abc import Awaitable, Callable
|
||||
import os
|
||||
|
||||
# Settings (app.core.config, импортируемый через app.core.rbac) требует
|
||||
# DATABASE_URL на конструирование — stub перед любым app-импортом (тот же
|
||||
# паттерн что и в остальных tests/*.py).
|
||||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI, Request
|
||||
from fastapi.responses import JSONResponse, Response
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app.api.v1 import me as me_router
|
||||
from app.core import auth as auth_mod
|
||||
from app.core.rbac import rbac_guard
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
|
|
@ -38,60 +43,13 @@ def _reset_auth_cache() -> None:
|
|||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Test app — копия rbac_guard из app/main.py.
|
||||
# Test app — использует РЕАЛЬНЫЙ rbac_guard (app/core/rbac.py), а не копию.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
_ADMIN_API_RE = re.compile(r"^/api/v1/admin/")
|
||||
_PUBLIC_PATHS = frozenset({"/health", "/docs", "/redoc", "/openapi.json"})
|
||||
# MIRROR of app.main (#R2-H3): keep in sync with the real rbac_guard.
|
||||
_EXTERNAL_PREFIX = "/trade-in"
|
||||
_RBAC_BOOTSTRAP_EXEMPT = ("/api/v1/me", "/api/v1/brand")
|
||||
|
||||
|
||||
def _build_test_app() -> FastAPI:
|
||||
app = FastAPI()
|
||||
|
||||
@app.middleware("http")
|
||||
async def rbac_guard(
|
||||
request: Request,
|
||||
call_next: Callable[[Request], Awaitable[Response]],
|
||||
) -> Response:
|
||||
path = request.url.path
|
||||
if path in _PUBLIC_PATHS:
|
||||
return await call_next(request)
|
||||
|
||||
username = request.headers.get("X-Authenticated-User")
|
||||
if not username:
|
||||
return JSONResponse(
|
||||
status_code=401,
|
||||
content={"detail": "no authenticated user (Caddy basic_auth required)"},
|
||||
)
|
||||
try:
|
||||
role = auth_mod.get_role(username)
|
||||
except KeyError:
|
||||
return JSONResponse(
|
||||
status_code=403,
|
||||
content={"detail": "user not in roles config"},
|
||||
)
|
||||
if _ADMIN_API_RE.match(path) and role != "admin":
|
||||
return JSONResponse(
|
||||
status_code=403,
|
||||
content={"detail": "admin only"},
|
||||
)
|
||||
# #R2-H3 mirror: enforce roles.yaml scope on non-bootstrap paths.
|
||||
if not path.startswith(_RBAC_BOOTSTRAP_EXEMPT):
|
||||
external_path = _EXTERNAL_PREFIX + path
|
||||
try:
|
||||
allowed = auth_mod.is_path_allowed(role, external_path)
|
||||
except Exception:
|
||||
allowed = True
|
||||
if not allowed:
|
||||
return JSONResponse(
|
||||
status_code=403,
|
||||
content={"detail": "forbidden for role"},
|
||||
)
|
||||
return await call_next(request)
|
||||
app.middleware("http")(rbac_guard)
|
||||
|
||||
app.include_router(me_router.router, prefix="/api/v1", tags=["me"])
|
||||
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:
|
|||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
from fastapi import FastAPI, Response
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app.core.request_audit import RequestAuditMiddleware
|
||||
|
|
@ -138,3 +138,152 @@ def test_admin_path_excluded_from_api_request_but_login_kept() -> None:
|
|||
assert resp.status_code == 200
|
||||
event_types = [c.kwargs["event_type"] for c in mock_schedule.call_args_list]
|
||||
assert event_types == ["login"]
|
||||
|
||||
|
||||
# ── Admin audit (security-audit fix): mutating /admin/* -> admin_action ────────
|
||||
|
||||
|
||||
def test_admin_mutating_post_schedules_admin_action_with_attribution() -> None:
|
||||
"""POST на /api/v1/admin/* (напр. правка прокси / настройки скрапера) должен
|
||||
писать `admin_action` с атрибуцией (кто), а НЕ игнорироваться целиком, как
|
||||
раньше (security-audit: не было возможности установить, кто это сделал)."""
|
||||
app = FastAPI()
|
||||
app.add_middleware(RequestAuditMiddleware)
|
||||
|
||||
@app.post("/api/v1/admin/scraper/avito/rotate-ip")
|
||||
def rotate_ip() -> dict[str, bool]:
|
||||
return {"ok": True}
|
||||
|
||||
with (
|
||||
patch("app.core.request_audit.schedule_event") as mock_schedule,
|
||||
patch("app.core.request_audit.should_log_login", return_value=False),
|
||||
):
|
||||
resp = TestClient(app).post(
|
||||
"/api/v1/admin/scraper/avito/rotate-ip",
|
||||
headers={"X-Authenticated-User": "admin"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert mock_schedule.call_count == 1
|
||||
kwargs = mock_schedule.call_args.kwargs
|
||||
assert kwargs["event_type"] == "admin_action"
|
||||
assert kwargs["username"] == "admin"
|
||||
assert kwargs["path"] == "/api/v1/admin/scraper/avito/rotate-ip"
|
||||
assert kwargs["method"] == "POST"
|
||||
assert kwargs["payload"] == {"status_code": 200, "success": True}
|
||||
|
||||
|
||||
def test_admin_get_does_not_schedule_admin_action() -> None:
|
||||
"""GET на /admin/* (просмотр дашборда) НЕ должен писать admin_action — только
|
||||
мутирующие методы считаются "действием"."""
|
||||
app = FastAPI()
|
||||
app.add_middleware(RequestAuditMiddleware)
|
||||
|
||||
@app.get("/api/v1/admin/scraper/health")
|
||||
def health() -> dict[str, bool]:
|
||||
return {"ok": True}
|
||||
|
||||
with (
|
||||
patch("app.core.request_audit.schedule_event") as mock_schedule,
|
||||
patch("app.core.request_audit.should_log_login", return_value=False),
|
||||
):
|
||||
TestClient(app).get(
|
||||
"/api/v1/admin/scraper/health", headers={"X-Authenticated-User": "admin"}
|
||||
)
|
||||
|
||||
mock_schedule.assert_not_called()
|
||||
|
||||
|
||||
def test_admin_action_payload_excludes_request_body() -> None:
|
||||
"""security-audit: тело запроса (куки/пароли/секреты правки прокси) НЕ должно
|
||||
попадать в audit-payload — только факт действия + атрибуция."""
|
||||
app = FastAPI()
|
||||
app.add_middleware(RequestAuditMiddleware)
|
||||
|
||||
@app.post("/api/v1/admin/scraper/cookies")
|
||||
def upload_cookies() -> dict[str, bool]:
|
||||
# Хендлер намеренно НЕ объявляет body-параметр — middleware проверяет
|
||||
# только headers/path/method/status, JSON-тело запроса ниже (куки) в
|
||||
# audit-payload попасть не может структурно, не только "по договорённости".
|
||||
return {"ok": True}
|
||||
|
||||
with (
|
||||
patch("app.core.request_audit.schedule_event") as mock_schedule,
|
||||
patch("app.core.request_audit.should_log_login", return_value=False),
|
||||
):
|
||||
TestClient(app).post(
|
||||
"/api/v1/admin/scraper/cookies",
|
||||
headers={"X-Authenticated-User": "admin"},
|
||||
json={"cookies": "super-secret-session-cookie"},
|
||||
)
|
||||
|
||||
kwargs = mock_schedule.call_args.kwargs
|
||||
assert kwargs["event_type"] == "admin_action"
|
||||
assert "super-secret-session-cookie" not in repr(kwargs)
|
||||
|
||||
|
||||
def test_admin_mutating_failure_status_recorded_in_payload() -> None:
|
||||
"""admin_action на неуспешный ответ (напр. 500 от нижестоящего сервиса) должен
|
||||
нести success=False + реальный status_code — не маскироваться под успех."""
|
||||
app = FastAPI()
|
||||
app.add_middleware(RequestAuditMiddleware)
|
||||
|
||||
@app.post("/api/v1/admin/scraper/pacing")
|
||||
def pacing() -> Response:
|
||||
return Response(status_code=502)
|
||||
|
||||
with (
|
||||
patch("app.core.request_audit.schedule_event") as mock_schedule,
|
||||
patch("app.core.request_audit.should_log_login", return_value=False),
|
||||
):
|
||||
TestClient(app).post(
|
||||
"/api/v1/admin/scraper/pacing", headers={"X-Authenticated-User": "admin"}
|
||||
)
|
||||
|
||||
kwargs = mock_schedule.call_args.kwargs
|
||||
assert kwargs["event_type"] == "admin_action"
|
||||
assert kwargs["payload"] == {"status_code": 502, "success": False}
|
||||
|
||||
|
||||
# ── login vs login_failed (security-audit fix) ──────────────────────────────────
|
||||
|
||||
|
||||
def test_login_event_type_when_request_succeeds(client: TestClient) -> None:
|
||||
"""Ответ < 400 -> event_type='login' (успешный вход/активность), payload несёт
|
||||
status_code."""
|
||||
with (
|
||||
patch("app.core.request_audit.schedule_event") as mock_schedule,
|
||||
patch("app.core.request_audit.should_log_login", return_value=True),
|
||||
):
|
||||
client.get("/api/v1/ping", headers={"X-Authenticated-User": "alice"})
|
||||
|
||||
login_calls = [c for c in mock_schedule.call_args_list if c.kwargs["event_type"] == "login"]
|
||||
assert len(login_calls) == 1
|
||||
assert login_calls[0].kwargs["payload"] == {"status_code": 200}
|
||||
|
||||
|
||||
def test_login_failed_event_type_when_rbac_rejects_request() -> None:
|
||||
"""Ответ >= 400 (напр. RBAC-отказ downstream: неизвестная роль / протухший
|
||||
внутренний секрет) -> event_type='login_failed', а НЕ 'login' — раньше эти
|
||||
два случая были неразличимы в журнале (security-audit)."""
|
||||
app = FastAPI()
|
||||
app.add_middleware(RequestAuditMiddleware)
|
||||
|
||||
@app.get("/api/v1/ping")
|
||||
def ping() -> Response:
|
||||
return Response(status_code=403, content="forbidden")
|
||||
|
||||
with (
|
||||
patch("app.core.request_audit.schedule_event") as mock_schedule,
|
||||
patch("app.core.request_audit.should_log_login", return_value=True),
|
||||
):
|
||||
TestClient(app).get("/api/v1/ping", headers={"X-Authenticated-User": "alice"})
|
||||
|
||||
login_calls = [
|
||||
c
|
||||
for c in mock_schedule.call_args_list
|
||||
if c.kwargs["event_type"] in ("login", "login_failed")
|
||||
]
|
||||
assert len(login_calls) == 1
|
||||
assert login_calls[0].kwargs["event_type"] == "login_failed"
|
||||
assert login_calls[0].kwargs["payload"] == {"status_code": 403}
|
||||
|
|
|
|||
|
|
@ -305,7 +305,44 @@ def test_rotate_ip_changeip_error(client: TestClient) -> None:
|
|||
assert r.status_code == 200
|
||||
body = r.json()
|
||||
assert body["ok"] is False
|
||||
assert "changeip error" in body["reason"]
|
||||
# security-audit: нейтральный reason, БЕЗ текста исходного исключения
|
||||
# (str(exc) httpx мог нести rotate_url с proxy-ключом в query — см. тест ниже).
|
||||
assert body["reason"] == "changeip request failed"
|
||||
|
||||
|
||||
def test_rotate_ip_changeip_error_does_not_leak_proxy_key(client: TestClient) -> None:
|
||||
"""security-audit: секретный API-ключ провайдера в rotate_url НЕ должен попасть
|
||||
в HTTP-ответ клиенту через текст httpx-исключения (раньше
|
||||
`reason=f"changeip error: {exc}"` отдавал str(exc) с полным URL, включая
|
||||
query-параметр ключа, наружу)."""
|
||||
from app.api.v1 import admin as admin_module
|
||||
|
||||
secret_url = "http://ch/changeip?proxy_key=TOP-SECRET-KEY-1234"
|
||||
|
||||
class _BoomClient:
|
||||
def __init__(self, *a: Any, **k: Any) -> None:
|
||||
pass
|
||||
|
||||
async def __aenter__(self) -> _BoomClient:
|
||||
return self
|
||||
|
||||
async def __aexit__(self, *a: Any) -> None:
|
||||
return None
|
||||
|
||||
async def get(self, *a: Any, **k: Any) -> Any:
|
||||
raise RuntimeError(f"All connection attempts failed for {secret_url}&format=json")
|
||||
|
||||
with (
|
||||
patch.object(admin_module.httpx, "AsyncClient", _BoomClient),
|
||||
patch.object(admin_module.settings, "avito_proxy_rotate_url", secret_url),
|
||||
):
|
||||
r = client.post("/api/v1/admin/scraper/avito/rotate-ip")
|
||||
|
||||
assert r.status_code == 200
|
||||
assert "TOP-SECRET-KEY-1234" not in r.text
|
||||
body = r.json()
|
||||
assert body["ok"] is False
|
||||
assert "TOP-SECRET-KEY-1234" not in (body["reason"] or "")
|
||||
|
||||
|
||||
# ── API 4: GET /scraper/pacing ───────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -7,6 +7,9 @@
|
|||
- телефон без цифр / слишком мало цифр -> 422 (digit-guard, #2376 hardening)
|
||||
- source="landing" (мёртвая воронка) -> 422 (литерал убран из схемы)
|
||||
- estimate_id, которого нет в trade_in_estimates -> 404
|
||||
- IDOR guard (security-audit, зеркалит #690/test_estimate_idor.py): estimate_id
|
||||
чужого пользователя -> 404; admin может привязать любой; нет
|
||||
X-Authenticated-User -> 401; неизвестная роль -> 403
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -16,6 +19,7 @@ import os
|
|||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||||
|
||||
from datetime import UTC, datetime
|
||||
from types import SimpleNamespace
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
from uuid import uuid4
|
||||
|
|
@ -45,6 +49,16 @@ def client(db: MagicMock) -> TestClient:
|
|||
return TestClient(app)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _restore_get_role():
|
||||
"""Restore app.core.auth.get_role after each test (mirror test_estimate_idor.py)."""
|
||||
from app.core import auth as auth_mod
|
||||
|
||||
original = auth_mod.get_role
|
||||
yield
|
||||
auth_mod.get_role = original
|
||||
|
||||
|
||||
def _insert_result(lead_id: str) -> MagicMock:
|
||||
result = MagicMock()
|
||||
result.mappings.return_value.one.return_value = {
|
||||
|
|
@ -135,9 +149,14 @@ def test_lead_with_unknown_estimate_id_404(client: TestClient, db: MagicMock) ->
|
|||
|
||||
|
||||
def test_lead_with_known_estimate_id_200(client: TestClient, db: MagicMock) -> None:
|
||||
"""Owner привязывает лид к своей же оценке -> 200."""
|
||||
from app.core import auth as auth_mod
|
||||
|
||||
auth_mod.get_role = lambda _u: "pilot" # type: ignore[assignment]
|
||||
|
||||
lead_id = str(uuid4())
|
||||
exists_result = MagicMock()
|
||||
exists_result.fetchone.return_value = (1,)
|
||||
exists_result.fetchone.return_value = SimpleNamespace(created_by="kopylov")
|
||||
db.execute.side_effect = [exists_result, _insert_result(lead_id)]
|
||||
|
||||
estimate_id = str(uuid4())
|
||||
|
|
@ -148,6 +167,7 @@ def test_lead_with_known_estimate_id_200(client: TestClient, db: MagicMock) -> N
|
|||
"consent": True,
|
||||
"estimate_id": estimate_id,
|
||||
},
|
||||
headers={"X-Authenticated-User": "kopylov"},
|
||||
)
|
||||
assert r.status_code == 200, r.text
|
||||
params = db.execute.call_args.args[1]
|
||||
|
|
@ -155,6 +175,86 @@ def test_lead_with_known_estimate_id_200(client: TestClient, db: MagicMock) -> N
|
|||
assert params["source"] == "result"
|
||||
|
||||
|
||||
# ── IDOR guard (security-audit): estimate_id ownership ─────────────────────────
|
||||
|
||||
|
||||
def test_lead_estimate_id_owned_by_other_user_gets_404(client: TestClient, db: MagicMock) -> None:
|
||||
"""Чужой estimate_id -> 404 (существование не подтверждаем), лид НЕ создаётся."""
|
||||
from app.core import auth as auth_mod
|
||||
|
||||
auth_mod.get_role = lambda _u: "pilot" # type: ignore[assignment]
|
||||
|
||||
exists_result = MagicMock()
|
||||
exists_result.fetchone.return_value = SimpleNamespace(created_by="victim")
|
||||
db.execute.return_value = exists_result
|
||||
|
||||
r = client.post(
|
||||
"/api/v1/trade-in/lead",
|
||||
json={"phone": "+79123456789", "consent": True, "estimate_id": str(uuid4())},
|
||||
headers={"X-Authenticated-User": "attacker"},
|
||||
)
|
||||
assert r.status_code == 404, r.text
|
||||
assert not db.commit.called
|
||||
|
||||
|
||||
def test_lead_estimate_id_admin_can_attach_any_200(client: TestClient, db: MagicMock) -> None:
|
||||
"""Admin может привязать лид к чужой оценке (owner-or-admin, зеркалит #690)."""
|
||||
from app.core import auth as auth_mod
|
||||
|
||||
auth_mod.get_role = lambda _u: "admin" # type: ignore[assignment]
|
||||
|
||||
lead_id = str(uuid4())
|
||||
exists_result = MagicMock()
|
||||
exists_result.fetchone.return_value = SimpleNamespace(created_by="someone_else")
|
||||
db.execute.side_effect = [exists_result, _insert_result(lead_id)]
|
||||
|
||||
r = client.post(
|
||||
"/api/v1/trade-in/lead",
|
||||
json={"phone": "+79123456789", "consent": True, "estimate_id": str(uuid4())},
|
||||
headers={"X-Authenticated-User": "admin"},
|
||||
)
|
||||
assert r.status_code == 200, r.text
|
||||
|
||||
|
||||
def test_lead_estimate_id_requires_authenticated_user_401(
|
||||
client: TestClient, db: MagicMock
|
||||
) -> None:
|
||||
"""estimate_id задан, но нет X-Authenticated-User -> 401 (defense-in-depth: в
|
||||
проде rbac_guard уже требует заголовок раньше, см. app/main.py)."""
|
||||
exists_result = MagicMock()
|
||||
exists_result.fetchone.return_value = SimpleNamespace(created_by="kopylov")
|
||||
db.execute.return_value = exists_result
|
||||
|
||||
r = client.post(
|
||||
"/api/v1/trade-in/lead",
|
||||
json={"phone": "+79123456789", "consent": True, "estimate_id": str(uuid4())},
|
||||
)
|
||||
assert r.status_code == 401, r.text
|
||||
assert not db.commit.called
|
||||
|
||||
|
||||
def test_lead_estimate_id_unknown_role_403(client: TestClient, db: MagicMock) -> None:
|
||||
"""Аутентифицирован через Caddy, но роль отсутствует в roles.yaml -> 403."""
|
||||
from app.core import auth as auth_mod
|
||||
|
||||
def _raise_keyerror(_u: str):
|
||||
raise KeyError(_u)
|
||||
|
||||
auth_mod.get_role = _raise_keyerror # type: ignore[assignment]
|
||||
|
||||
exists_result = MagicMock()
|
||||
exists_result.fetchone.return_value = SimpleNamespace(created_by="kopylov")
|
||||
db.execute.return_value = exists_result
|
||||
|
||||
r = client.post(
|
||||
"/api/v1/trade-in/lead",
|
||||
json={"phone": "+79123456789", "consent": True, "estimate_id": str(uuid4())},
|
||||
headers={"X-Authenticated-User": "ghost"},
|
||||
)
|
||||
assert r.status_code == 403, r.text
|
||||
assert not db.commit.called
|
||||
|
||||
|
||||
def test_lead_digit_free_phone_422(client: TestClient, db: MagicMock) -> None:
|
||||
# "(()) -- .." проходит regex-маску (только +/скобки/дефисы/точки/пробелы),
|
||||
# но содержит 0 цифр -> должно отклоняться digit-валидатором.
|
||||
|
|
|
|||
|
|
@ -44,6 +44,20 @@ services:
|
|||
# (грубо ~2.5g на каждую доп. параллельную страницу).
|
||||
mem_limit: 2560m
|
||||
memswap_limit: 3g
|
||||
# stop_grace_period: browser/server.py — bare aiohttp web.run_app(), которое
|
||||
# само ловит SIGTERM (aiohttp.web.GracefulExit) и даёт себе внутренний
|
||||
# shutdown_timeout=60s (aiohttp default, здесь не переопределён) на закрытие
|
||||
# in-flight соединений ПЕРЕД тем как _on_cleanup закроет camoufox-инстансы.
|
||||
# Без stop_grace_period Docker бы SIGKILL'ил через дефолтные 10s — это убивало
|
||||
# бы headless-страницу (комментарий выше: /fetch карточка ~15-27s, из
|
||||
# scraper stop_grace_period #1951) на середине навигации/скрейпа задолго до
|
||||
# того как aiohttp вообще успеет начать свой собственный graceful-путь.
|
||||
# 90s = 60s aiohttp shutdown_timeout + ~30s запас на закрытие Firefox-
|
||||
# инстансов в _on_cleanup (дороже обычного process.kill — camoufox — полноценный
|
||||
# Firefox-профиль). Не 120s как у scraper/tgbot: у browser нет
|
||||
# многочасовых unit'ов (единица работы — одна страница, секунды-десятки
|
||||
# секунд), 120s был бы избыточным запасом без code-level обоснования.
|
||||
stop_grace_period: 90s
|
||||
logging: *default-logging
|
||||
env_file:
|
||||
- path: ./backend/.env.runtime
|
||||
|
|
@ -97,6 +111,20 @@ services:
|
|||
# наложение export + бэкфилл при 640m было бы впритык)
|
||||
mem_limit: 768m
|
||||
memswap_limit: 768m
|
||||
# stop_grace_period: uvicorn command ниже не задаёт --timeout-graceful-shutdown,
|
||||
# т.е. используется uvicorn-дефолт None (безлимитно ждёт in-flight запросы на
|
||||
# SIGTERM — verified в uvicorn docs, Server.shutdown() без timeout зависает до
|
||||
# завершения задач). Единственный реальный backstop — Docker'овский
|
||||
# stop_grace_period; дефолтные 10s SIGKILL'или бы синхронный PDF-экспорт
|
||||
# (/estimate/{id}/pdf — sync-def route, значит выполняется в Starlette
|
||||
# threadpool: WeasyPrint write_pdf() + url_fetcher timeout=10s на встроенные
|
||||
# SVG/шрифты, см. app/services/exporters/trade_in_pdf.py) прямо посреди
|
||||
# рендера. 60s — щедрый запас над этим (fetcher максимум 10s + рендер
|
||||
# исторически секунды, не минуты); не 120s как у scraper/tgbot — там код
|
||||
# сам ограничивает свой drain через _DRAIN_TIMEOUT_S=100s (cooperative
|
||||
# shutdown handler), здесь такого code-level таймера нет и заводить его
|
||||
# ради одного PDF-эндпоинта — за рамками этого fix'а.
|
||||
stop_grace_period: 60s
|
||||
logging: *default-logging
|
||||
# Prod: uvicorn БЕЗ --reload (Dockerfile CMD несёт --reload только для dev hot-reload,
|
||||
# где app/ bind-mount'ится). В prod --reload = лишний WatchFiles-наблюдатель + риск
|
||||
|
|
|
|||
|
|
@ -994,12 +994,18 @@ def _extract_meta(tree: HTMLParser) -> tuple[date | None, int | None, int | None
|
|||
day = int(m_date.group(1))
|
||||
month_word = m_date.group(2).lower()
|
||||
month = RUS_MONTHS.get(month_word)
|
||||
# Год — текущий (Avito не показывает год для свежих объявлений)
|
||||
import datetime
|
||||
|
||||
current_year = datetime.date.today().year
|
||||
if month:
|
||||
publish_date = date(current_year, month, day)
|
||||
# Avito не показывает год для свежих объявлений — берём текущий.
|
||||
# audit-scrapers finding 3: если объявление опубликовано в декабре,
|
||||
# а страница парсится в январе СЛЕДУЮЩЕГО года, "текущий год" даёт
|
||||
# дату в будущем (завышает свежесть лота). Если получившаяся дата
|
||||
# оказалась в будущем относительно момента парсинга — откатываем
|
||||
# на год назад (это дата из прошлого года).
|
||||
today = date.today()
|
||||
candidate = date(today.year, month, day)
|
||||
if candidate > today:
|
||||
candidate = date(today.year - 1, month, day)
|
||||
publish_date = candidate
|
||||
except (ValueError, KeyError):
|
||||
pass
|
||||
|
||||
|
|
|
|||
|
|
@ -953,6 +953,29 @@ class AvitoScraper(BaseScraper):
|
|||
return 0
|
||||
return None
|
||||
|
||||
def _is_unexpected_empty_page(self, html: str) -> bool:
|
||||
"""Отличить «в выборке реально 0 объявлений» от DOM-дрейфа/content-block.
|
||||
|
||||
Вызывается ТОЛЬКО когда `_parse_html` уже вернул 0 карточек на page=1
|
||||
(обход всего города/категории/бакета) — само по себе это неотличимо от
|
||||
«объявлений действительно нет» (#audit-scrapers finding 1).
|
||||
|
||||
`_extract_total_count(html)` даёт независимый от DOM-карточек сигнал
|
||||
(счётчик `page-title/count` либо no-results-маркер):
|
||||
- total_hint == 0 → валидный no-results-маркер найден — НЕ аномалия.
|
||||
- total_hint > 0 → счётчик утверждает, что результаты есть, но карточки
|
||||
(`data-marker="item-*"`) не распознаны — DOM-маркер
|
||||
карточки разошёлся со счётчиком (drift).
|
||||
- total_hint is None → ни счётчика, ни no-results-маркера — тоже
|
||||
подозрительно (captcha/firewall без ожидаемой
|
||||
структуры страницы).
|
||||
|
||||
Returns:
|
||||
True — 0 карточек считается аномалией (нужно поднять
|
||||
``AvitoContentBlockedError``); False — валидная пустая выборка.
|
||||
"""
|
||||
return self._extract_total_count(html) != 0
|
||||
|
||||
async def _fetch_rooms_page_html(
|
||||
self,
|
||||
room_slug: str,
|
||||
|
|
@ -1282,6 +1305,7 @@ class AvitoScraper(BaseScraper):
|
|||
secondary_only=secondary_only,
|
||||
on_bucket=on_bucket,
|
||||
skip_buckets=skip_buckets,
|
||||
expected_total=total,
|
||||
)
|
||||
|
||||
await walk_price_range(
|
||||
|
|
@ -1309,6 +1333,7 @@ class AvitoScraper(BaseScraper):
|
|||
secondary_only: bool,
|
||||
on_bucket: Callable[..., Any] | None,
|
||||
skip_buckets: set[str] | None,
|
||||
expected_total: int | None = None,
|
||||
) -> None:
|
||||
"""Параллельная пагинация одного leaf-бакета + фильтр + дедуп + on_bucket.
|
||||
|
||||
|
|
@ -1320,6 +1345,14 @@ class AvitoScraper(BaseScraper):
|
|||
bucket_key = "room_label:lo:hi" (закрытый) либо "room_label:lo:open" (открытый).
|
||||
skip_buckets: если bucket_key в skip_buckets — пагинация и on_bucket пропускаются.
|
||||
AvitoBlockedError/AvitoRateLimitedError из page-фетчей пробрасываются наверх.
|
||||
|
||||
expected_total: total из probe (``_extract_total_count``), известный ДО вызова
|
||||
(см. finding 1 audit-scrapers). Если задан и > 0, а после пагинации всех
|
||||
``max_pages`` страниц собрано 0 карточек — это противоречие (тот же probe-html
|
||||
независимо утверждал total>0), т.е. DOM-маркер карточки разошёлся со счётчиком
|
||||
(drift), а не легитимно пустой бакет (тот даёт expected_total=0 и сюда даже не
|
||||
доходит — вызывающий _leaf не паджинирует пустые бакеты). None — probe провалился
|
||||
(best-effort пагинация, отсутствие данных ожидаемо, проверка пропускается).
|
||||
"""
|
||||
_lo_param = lo if lo > 0 else None
|
||||
_hi_param = hi # None → _build_rooms_url не ставит pmax
|
||||
|
|
@ -1378,6 +1411,23 @@ class AvitoScraper(BaseScraper):
|
|||
|
||||
collected_this_bucket = len(bucket_lots)
|
||||
|
||||
# ── Guard: probe утверждал total>0, но парсинг всех страниц дал 0 карточек ──
|
||||
# (finding 1 audit-scrapers). Тот же probe-html независимо подтвердил, что
|
||||
# результаты есть (_extract_total_count) — 0 карточек здесь не может быть
|
||||
# легитимной пустой выдачей, значит DOM-маркер карточки разошёлся со счётчиком.
|
||||
if expected_total is not None and expected_total > 0 and collected_this_bucket == 0:
|
||||
logger.error(
|
||||
"avito: bucket %s probe expected_total=%d but 0 cards parsed across "
|
||||
"%d page(s) — content-block/DOM-drift suspected",
|
||||
bucket_key,
|
||||
expected_total,
|
||||
max_pages,
|
||||
)
|
||||
raise AvitoContentBlockedError(
|
||||
f"Avito bucket {bucket_key}: probe total={expected_total} but 0 cards "
|
||||
"parsed — content-block/DOM-drift suspected"
|
||||
)
|
||||
|
||||
# ── Фильтр новостроек (secondary_only) ────────────────────────────────
|
||||
dropped_nb = 0
|
||||
if secondary_only:
|
||||
|
|
@ -1596,6 +1646,18 @@ class AvitoScraper(BaseScraper):
|
|||
|
||||
lots = self._parse_html(html, source_url_base=url)
|
||||
if not lots:
|
||||
if page == 1 and self._is_unexpected_empty_page(html):
|
||||
logger.error(
|
||||
"avito %s SERP page=1 returned HTTP 200 but 0 cards "
|
||||
"(no no-results marker) — likely content-block/captcha or "
|
||||
"DOM-marker drift url=%s",
|
||||
label,
|
||||
url,
|
||||
)
|
||||
raise AvitoContentBlockedError(
|
||||
f"Avito {label} sweep: HTTP 200 with 0 cards on page=1 — "
|
||||
"content-block/DOM-drift suspected"
|
||||
)
|
||||
logger.info("avito %s page=%d: 0 lots — end of pagination", label, page)
|
||||
break
|
||||
|
||||
|
|
@ -1706,6 +1768,18 @@ class AvitoScraper(BaseScraper):
|
|||
|
||||
lots = self._parse_html(html, source_url_base=url)
|
||||
if not lots:
|
||||
if page == 1 and self._is_unexpected_empty_page(html):
|
||||
logger.error(
|
||||
"avito byrooms category=%s page=1 returned HTTP 200 but 0 cards "
|
||||
"(no no-results marker) — likely content-block/captcha or "
|
||||
"DOM-marker drift url=%s",
|
||||
name,
|
||||
url,
|
||||
)
|
||||
raise AvitoContentBlockedError(
|
||||
f"Avito byrooms category={name}: HTTP 200 with 0 cards on "
|
||||
"page=1 — content-block/DOM-drift suspected"
|
||||
)
|
||||
logger.info(
|
||||
"avito byrooms category=%s page=%d: 0 lots — end of category",
|
||||
name,
|
||||
|
|
|
|||
|
|
@ -639,6 +639,25 @@ class CianScraper(BaseScraper):
|
|||
if inspect.isawaitable(res_cb):
|
||||
await res_cb
|
||||
|
||||
def _report_schema_regression(self, message: str) -> None:
|
||||
"""Отправить сигнал schema-regression в Glitchtip (если настроен).
|
||||
|
||||
Общий механизм для silent-failure guard'ов `_parse_serp_html` (offer-level
|
||||
parse-failure и totalOffers/results.offers mismatch, audit-scrapers finding 2)
|
||||
— переиспользуется, чтобы обе проверки одинаково попадали в мониторинг, а не
|
||||
только в текстовый лог.
|
||||
"""
|
||||
try:
|
||||
if self._config.glitchtip_dsn:
|
||||
# Ленивый импорт: sentry_sdk — app-side error-reporting, не dep
|
||||
# scraper_kit. Только когда glitchtip настроен И случилась
|
||||
# schema-regression.
|
||||
import sentry_sdk
|
||||
|
||||
sentry_sdk.capture_message(message, level="error")
|
||||
except Exception:
|
||||
logger.debug("sentry_sdk report failed (not installed/initialised)", exc_info=True)
|
||||
|
||||
def _parse_serp_html(self, html: str) -> list[ScrapedLot]:
|
||||
"""Извлечь offers из Cian Redux state.
|
||||
|
||||
|
|
@ -655,18 +674,41 @@ class CianScraper(BaseScraper):
|
|||
)
|
||||
return []
|
||||
|
||||
offers_data: list[dict[str, Any]] = state.get("results", {}).get("offers", [])
|
||||
results = state.get("results", {})
|
||||
offers_data: list[dict[str, Any]] = results.get("offers", [])
|
||||
total_offers = results.get("totalOffers")
|
||||
|
||||
if not offers_data:
|
||||
logger.warning(
|
||||
"cian state found but results.offers пуст (totalOffers=%s)",
|
||||
state.get("results", {}).get("totalOffers", "?"),
|
||||
)
|
||||
# audit-scrapers finding 2: totalOffers и results.offers приходят из ОДНОГО
|
||||
# state-блоба (одна SSR-выдача) — если totalOffers>0, а offers пуст, это
|
||||
# внутреннее противоречие payload'а, а не легитимная пагинация (probe/leaf
|
||||
# запрашивают только max_pages = ceil(totalOffers/28), посчитанные из ТОГО ЖЕ
|
||||
# totalOffers, так что «сходили за последнюю страницу» здесь не объясняет 0).
|
||||
# Порог: 0 vs >0 — единственный позиционно-независимый сигнал, который можно
|
||||
# проверить без номера страницы; дробный порог (напр. «< 50% от expected»)
|
||||
# ложно сработал бы на легитимной последней частичной странице пагинации,
|
||||
# которую эта функция не различает.
|
||||
if isinstance(total_offers, int) and total_offers > 0:
|
||||
logger.error(
|
||||
"cian SERP: totalOffers=%d но results.offers пуст — schema "
|
||||
"regression suspected (counter/offers mismatch, not empty search)",
|
||||
total_offers,
|
||||
)
|
||||
self._report_schema_regression(
|
||||
f"cian SERP: totalOffers={total_offers} but results.offers is "
|
||||
"empty — possible schema regression (counter/offers mismatch)"
|
||||
)
|
||||
else:
|
||||
logger.warning(
|
||||
"cian state found but results.offers пуст (totalOffers=%s)",
|
||||
total_offers if total_offers is not None else "?",
|
||||
)
|
||||
return []
|
||||
|
||||
logger.info(
|
||||
"cian SERP state ok: %d offers (totalOffers=%s)",
|
||||
len(offers_data),
|
||||
state.get("results", {}).get("totalOffers", "?"),
|
||||
total_offers if total_offers is not None else "?",
|
||||
)
|
||||
|
||||
lots: list[ScrapedLot] = []
|
||||
|
|
@ -684,20 +726,10 @@ class CianScraper(BaseScraper):
|
|||
"cian SERP: 0/%d offers прошли _offer_to_lot — возможна schema regression",
|
||||
raw_count,
|
||||
)
|
||||
try:
|
||||
if self._config.glitchtip_dsn:
|
||||
# Ленивый импорт: sentry_sdk — app-side error-reporting, не dep
|
||||
# scraper_kit. Только когда glitchtip настроен И случилась
|
||||
# schema-regression. ImportError глотается общим except ниже.
|
||||
import sentry_sdk
|
||||
|
||||
sentry_sdk.capture_message(
|
||||
f"cian SERP: {raw_count}/{raw_count} offers failed _offer_to_lot"
|
||||
" — possible schema regression",
|
||||
level="error",
|
||||
)
|
||||
except Exception:
|
||||
pass # sentry_sdk not installed/initialised in dev
|
||||
self._report_schema_regression(
|
||||
f"cian SERP: {raw_count}/{raw_count} offers failed _offer_to_lot"
|
||||
" — possible schema regression"
|
||||
)
|
||||
|
||||
return lots
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue