diff --git a/.claude/rules/tradein.md b/.claude/rules/tradein.md index f54f8d34..3eb19d62 100644 --- a/.claude/rules/tradein.md +++ b/.claude/rules/tradein.md @@ -41,9 +41,21 @@ In-app scheduler (`scrape_schedules`, tick 60s, `python -m app.scheduler_main`, `tradein-mvp/backend/data/sql/NN_*.sql` применяется автоматически на деплое через `_schema_migrations` в `.forgejo/workflows/deploy-tradein.yml` (НЕ init-only, strict exit-1). Idempotency критична — -деструктивный DDL хитит прод на деплое. NN-нумерация уже 3-значная и ИМЕЕТ коллизии (`108_*` ×2, -`084_*` ×2) → перед новым файлом `ls tradein-mvp/backend/data/sql | grep '^NN'` на дубль basename, -не доверяй `tail`. +деструктивный DDL хитит прод на деплое. + +**Номер новой миграции сверяй с `origin/main`, не с локальным `ls`** — локальное дерево не видит +миграций, смерженных после ветвления (так разъехались 212 в #2682 и 234 в #2754): + +```bash +git fetch origin main +git ls-tree -r --name-only origin/main -- tradein-mvp/backend/data/sql | tail +``` + +`-r` обязателен — без него `ls-tree` печатает сам каталог одной строкой, а не файлы. + +Правило целиком — в докстринге `tradein-mvp/backend/tests/test_migration_numbering.py` (единственная +формулировка контракта, #2683); он же гейтит его в CI. Дописывать имя в какой-либо список НЕ надо: +`_manifest_applied.txt` удалён — он отставал и по построению не мог покраснеть. ## Rapid-merge trap diff --git a/.forgejo/workflows/ci-tradein.yml b/.forgejo/workflows/ci-tradein.yml index ed60c692..7b5cd38b 100644 --- a/.forgejo/workflows/ci-tradein.yml +++ b/.forgejo/workflows/ci-tradein.yml @@ -94,6 +94,21 @@ jobs: CI_PG: ci-pg-tradein-${{ github.run_id }} steps: - uses: actions/checkout@v4 + with: + # ПОЛНАЯ история, а не дефолтный depth=1 (#2683). + # tests/test_migration_numbering.py сверяет номер новой миграции с + # origin/main и с точкой ветвления. Ровно этот флаг их и даёт: при + # depth=0 checkout идёт refspec'ом `+refs/heads/*:refs/remotes/origin/*` + # (видно в логе прогона), при depth=1 — только `+:refs/remotes/ + # pull/N/head`, то есть ни ветки main, ни общего предка в клоне нет. + # Дотянуть main отдельным `git fetch` НЕЛЬЗЯ: из job-контейнера + # git.gendsgn.ru:443 недостижим (проверено, run 6977 — connection + # refused), сеть есть только у самого checkout. + # + # Гейт при отсутствии эталона краснеет, а не пропускается: молча + # пропущенная проверка и есть тот зелёный, который ничего не проверяет. + # Пак репозитория ~33 MiB — полный fetch дешевле разбора коллизии на проде. + fetch-depth: 0 - name: Поднять Postgres и собрать схему tradein working-directory: . @@ -192,13 +207,22 @@ jobs: restore-keys: | uv-tradein-${{ runner.os }}- - - name: Sync deps (incl. dev group — pytest) + - name: Sync deps (incl. dev group — pytest, ruff) # Workspace-лок tradein-mvp/uv.lock TRACKED (с воркспейса #2137; gitignored # только старый backend/uv.lock) → --frozen детерминирован и зеркалит # Dockerfile (uv sync --frozen --no-dev там). uv находит workspace root # вверх от cwd. run: uv sync --frozen + - name: Lint (ruff check) + # Правила выбраны в tradein-mvp/backend/pyproject.toml ([tool.ruff.lint] + # select = E F I B UP N RUF), но до этого шага их никто не гонял в CI — + # "дерево чистое" было непроверенным утверждением, а не гарантией. + # Версия ruff — та же, что в tradein-mvp/uv.lock (--frozen из шага выше), + # т.е. ровно то, что видит `uv sync --frozen` в Dockerfile. + # Blocking: любое нарушение → job RED (не декоративно). + run: uv run ruff check . + - name: Run pytest (tradein-mvp/backend) # БЕЗ deselect'ов — сьют гоняется целиком (#2722). # diff --git a/.forgejo/workflows/deploy-tradein.yml b/.forgejo/workflows/deploy-tradein.yml index 640d12ff..c53f67c0 100644 --- a/.forgejo/workflows/deploy-tradein.yml +++ b/.forgejo/workflows/deploy-tradein.yml @@ -176,6 +176,11 @@ jobs: DATABASE_URL: postgresql+psycopg://test:test@localhost:5432/test steps: - uses: actions/checkout@v4 + with: + # Как в ci-tradein.yml: tests/test_migration_numbering.py (#2683) требует + # origin/main и общего предка с HEAD, а даёт их именно depth=0 — при + # depth=1 checkout тянет один sha и ветки main в клоне нет. + fetch-depth: 0 - name: Install uv # Официальный standalone-инсталлер: системный `pip install uv` на @@ -679,8 +684,9 @@ jobs: # Tracking через _schema_migrations (порт паттерна из deploy.yml): # каждый .sql применяется РОВНО один раз, failed migration → exit 1 # (никаких swallowed errors). cwd = /opt/gendesign/tradein-mvp. - # NB: цикл берёт только *.sql — data/sql/_manifest_applied.txt (инвариант - # #2216) glob'ом не подхватывается. + # ИМЕННО ЭТОТ цикл делает main эталоном применённого: всё, что доехало + # до main, здесь и применяется, а имя закрепляется в _schema_migrations. + # На этом стоит гейт номеров — tests/test_migration_numbering.py (#2683). # Pre-existence detection ДО CREATE TABLE: если таблицы ещё нет, это # первый deploy после внедрения tracking на уже-наполненной prod-БД diff --git a/.forgejo/workflows/deploy.yml b/.forgejo/workflows/deploy.yml index 4485dd82..451a6bb4 100644 --- a/.forgejo/workflows/deploy.yml +++ b/.forgejo/workflows/deploy.yml @@ -20,6 +20,14 @@ on: # деплоя ниже — без этого триггера правка bootstrap-файла молча не доезжала бы # до прода до следующего чужого коммита в backend/. - "ops/db-bootstrap/**" + # RBAC roles config (auth/roles.yaml, bind-mounted read-only ТОЛЬКО в backend — + # см. docker-compose.prod.yml; worker монтирует лишь ./data и ./reports). + # app.core.auth кэширует парсинг на весь lifetime процесса (@lru_cache) — без + # этого триггера правка ролей вступала бы в силу в случайный момент, только на + # следующий чужой деплой (`up -d --force-recreate --no-deps backend worker beat` + # ниже сбрасывает кэш перезапуском процесса; сам файл в образ не запекается, + # ребилда картинок для этого не нужно). + - "auth/**" # То же самое, ровно тот же класс бага (#2887): скрипт запускается на VM # по cron из /opt/gendesign/ops/, куда попадает только через `git reset --hard` # шага деплоя. Без этой строки правка скрипта лежала бы в main, а cron месяцами diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml deleted file mode 100644 index a2c3ed7e..00000000 --- a/.github/workflows/ci.yml +++ /dev/null @@ -1,91 +0,0 @@ -name: CI - -on: - push: - branches: - - main - - 'feat/**' - - 'fix/**' - - 'refactor/**' - - 'chore/**' - - 'docs/**' - - 'perf/**' - - 'test/**' - - 'hotfix/**' - pull_request: - branches: [main] - -concurrency: - group: ci-${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -jobs: - backend: - runs-on: ubuntu-latest - services: - postgres: - image: postgis/postgis:16-3.4 - env: - POSTGRES_DB: gendesign - POSTGRES_USER: gendesign - POSTGRES_PASSWORD: gendesign - ports: - - 5432:5432 - options: >- - --health-cmd "pg_isready -U gendesign" - --health-interval 5s - --health-timeout 5s - --health-retries 10 - defaults: - run: - working-directory: backend - steps: - - uses: actions/checkout@v4 - - - name: Install uv - uses: astral-sh/setup-uv@v3 - with: - enable-cache: true - - - name: Set up Python - run: uv python install 3.12 - - - name: Install system deps for geo + WeasyPrint - run: | - sudo apt-get update - sudo apt-get install -y libpq-dev libgdal-dev libproj-dev libgeos-dev \ - libcairo2 libpango-1.0-0 libpangoft2-1.0-0 - - - name: Install Python deps - run: uv sync - - - name: Lint (ruff) - run: uv run ruff check . - - - name: Type check (mypy strict on core) - run: | - uv run mypy \ - app/services/generative \ - app/services/site_finder/scorer.py - - - name: Test (pytest) - run: uv run pytest -q - env: - DATABASE_URL: postgresql+psycopg://gendesign:gendesign@localhost:5432/gendesign - - frontend: - runs-on: ubuntu-latest - defaults: - run: - working-directory: frontend - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 - with: - node-version: "20" - cache: "npm" - cache-dependency-path: frontend/package-lock.json - - run: npm ci || npm install - - run: npm run lint - - run: npm run type-check - - run: npm run build diff --git a/Caddyfile b/Caddyfile index 6ae40a86..e02a49d7 100644 --- a/Caddyfile +++ b/Caddyfile @@ -222,8 +222,7 @@ www.gendsgn.ru { # резолвится на этот сервер → HTTP-01/TLS-ALPN challenge недостижим) и продолжит # ретраить с backoff, ПОКА запись не появится. Остальные site-блоки в этом же # Caddyfile (gendsgn.ru, obsidian.gendsgn.ru и т.д.) не затрагиваются — -# автоматический HTTPS в Caddy изолирован per-hostname (тот же принцип, что -# уже описан для status.gendsgn.ru ниже). Повторные неудачные попытки ДО +# автоматический HTTPS в Caddy изолирован per-hostname. Повторные неудачные попытки ДО # появления DNS могут исчерпать rate-limit Let's Encrypt (5 failed # validations/hostname/hour) — не критично, просто подождать; `docker volume # rm gendesign_caddy_data` для этого НЕ нужен (и вообще требует user-approval). @@ -423,25 +422,6 @@ errors.gendsgn.ru { } } -# Uptime Kuma — self-hosted uptime monitoring + public status page (#75 B6-1). -# DNS: A-record status.gendsgn.ru → IP VPS (добавить перед деплоем стека). -# Контейнер из docker-compose.uptime.yml (project gendesign-uptime) на shared -# gendesign_shared network. Если стек не запущен — Caddy отдаёт 502 ТОЛЬКО на -# этом домене, main-сайт не страдает (как obsidian.gendsgn.ru). -# -# ВНИМАНИЕ: status-page НАМЕРЕННО публичен (trust-building для пилотов, issue #75). -# Admin-панель Kuma (/dashboard, /manage-*) защищена собственным логином Kuma — -# НЕ кладём её за caddy/users.caddy.snippet, иначе double-auth сломает setup. -status.gendsgn.ru { - encode zstd gzip - - reverse_proxy uptime-kuma:3001 - - log { - output file /var/log/caddy/status.gendsgn.ru.log - } -} - # Forgejo — self-hosted git (migration 2026-05-16). # DNS: A-record git.gendsgn.ru → IP VPS. # Forgejo container из forgejo-migration/docker-compose.yml на shared diff --git a/README.md b/README.md index 36f5925d..2a1acaa7 100644 --- a/README.md +++ b/README.md @@ -85,12 +85,10 @@ docker-compose.prod.yml main стек (backend, frontend, postgres, redis, work docker-compose.obsidian.yml obsidian-стек (CouchDB) — деплоится отдельно docker-compose.uptime.yml Uptime Kuma мониторинг (status.gendsgn.ru) — отдельный стек, запуск вручную .forgejo/workflows/ (Forgejo Actions — основной CI/CD после миграции 16.05.2026) - ├── ci.yml lint (ruff) + mypy + pytest на PR + ├── ci.yml lint (ruff) + pytest на PR ├── deploy.yml main → пересборка backend/frontend образов + auto-apply data/sql/*.sql + SSH deploy ├── deploy-tradein.yml tradein-mvp стек (отдельный пайплайн + свой _schema_migrations) └── stale-claims.yml авто-снятие протухших claim-меток в bot-пайплайне -.github/workflows/ (остаточные — только obsidian-стек на GitHub) - └── deploy-obsidian.yml obsidian-стек (CouchDB compose changes + bootstrap) ``` --- @@ -158,7 +156,7 @@ docker-compose.uptime.yml Uptime Kuma мониторинг (status.gendsgn.ru **Forgejo Actions deploys** (self-hosted `git.gendsgn.ru`, мигрировано с GitHub Actions 16.05.2026): -- [`.forgejo/workflows/ci.yml`](.forgejo/workflows/ci.yml) — на PR: ruff lint + mypy (selective strict) + pytest. Блокирует merge при провале. +- [`.forgejo/workflows/ci.yml`](.forgejo/workflows/ci.yml) — на PR: ruff lint + pytest (coverage gate ≥65%). mypy strict в гейте не гоняется (доступен вручную — `uv run mypy app/services/generative app/services/site_finder/scorer.py`). Блокирует merge при провале. - [`.forgejo/workflows/deploy.yml`](.forgejo/workflows/deploy.yml) — main: триггер на `backend/**`, `frontend/**`, `Caddyfile`, `docker-compose.prod.yml`, `data/sql/**`. Build backend lean + worker-with-chromium + frontend → push в приватный GHCR → SSH `git reset --hard`, **auto-apply pending `data/sql/NN_*.sql` через `_schema_migrations`** (idempotent, см. ниже про миграции), sed `SENTRY_RELEASE=$IMAGE_TAG` в `backend/.env.runtime`, `compose pull && up -d`, `caddy reload`, `curl /health`. - [`.forgejo/workflows/deploy-tradein.yml`](.forgejo/workflows/deploy-tradein.yml) — tradein-mvp стек (отдельный пайплайн). - [`.forgejo/workflows/deploy-obsidian.yml`](.forgejo/workflows/deploy-obsidian.yml) — obsidian: триггер на `docker-compose.obsidian.yml`, `scripts/setup-couchdb.sh`, `docs/obsidian-livesync.md`. Без сборки образов (couchdb:3 с DockerHub), SSH `compose up -d` + idempotent bootstrap (CORS, DB, лимиты). *(до 2026-07-05 ошибочно лежал в `.github/workflows/` — там ни разу не исполнился, см. issue #2416; контейнер держался вручную.)* diff --git a/backend/app/services/site_finder/eesk_reserve_loader.py b/backend/app/services/site_finder/eesk_reserve_loader.py index c396321b..937e3b29 100644 --- a/backend/app/services/site_finder/eesk_reserve_loader.py +++ b/backend/app/services/site_finder/eesk_reserve_loader.py @@ -169,7 +169,16 @@ def _cell(row: tuple, idx: int) -> object: def _pct_share_to_percent(value: object) -> float | None: """Доля загрузки (0.41) → проценты (41.0). Уже-проценты (>1) не трогаем. - В xlsx ЕЭСК степень загрузки хранится ДОЛЕЙ (0..1). Храним в процентах. + В xlsx ЕЭСК степень загрузки хранится ДОЛЕЙ (0..1). + + #2464-B: продакшен-вызывающих у функции СЕЙЧАС НЕТ. Значение колонки E + раньше писалось в `load_index`, но это категориальная колонка + ('open'|'limited'|'closed'|NULL) — число в ней фронт отбрасывает в + «неизвестно» и плодит мусорный бакет в `power_summary.by_load_index`. + Функцию оставляю с тестами: она описывает формат листа, и она понадобится + в тот момент, когда под процент загрузки заведут числовую колонку. + Если такого решения не будет — удалить вместе с тестом, а не держать молча. + None/мусор → None. """ num = parse_reserve_number(value) @@ -214,7 +223,9 @@ def load_ps_35_220(db: Session, xlsx_bytes: bytes, reserve_asof: date | None) -> rows_seen += 1 district = _cell(row, 1) # B - load_pct = _pct_share_to_percent(_cell(row, 4)) # E (доля → %) + # Колонку E (степень загрузки ЦП долей) НЕ читаем и не храним: места + # под неё в power_supply_centers нет — load_index категориальный, + # current_load_mva в мегавольт-амперах (#2464-B, см. UPDATE ниже). reserve = parse_reserve_number(_cell(row, 6)) # G (свободная МВт) name_norm = normalize_sc_name(str(sc_name)) @@ -223,7 +234,6 @@ def load_ps_35_220(db: Session, xlsx_bytes: bytes, reserve_asof: date | None) -> "reserve": reserve, "asof": reserve_asof, "district": str(district).strip() if district else None, - "load_pct": load_pct, "name_norm": name_norm, } @@ -236,10 +246,22 @@ def load_ps_35_220(db: Session, xlsx_bytes: bytes, reserve_asof: date | None) -> reserve_unit = 'МВт', installed_capacity_mva = :installed, district = :district, - load_index = COALESCE( - load_index, - CAST(:load_pct AS text) - ), + -- #2464-B: сюда БОЛЬШЕ НЕ пишем степень загрузки. + -- load_index — категориальная колонка + -- ('open'|'limited'|'closed'|NULL, см. + -- data/sql/180_connection_capacity.sql:35), её + -- заполняет rosseti_wfs_loader._map_load_index. + -- Раньше тут стоял COALESCE(load_index, + -- CAST(:load_pct AS text)) — при пустой ячейке + -- в колонку легло бы число строкой ("72.5"), + -- а фронтовый classifyLoadIndex такое значение + -- отбрасывает в null («неизвестно»), и в + -- power_summary.by_load_index появился бы + -- бакет с именем "72.5". + -- Сегодня не стреляло только потому, что у всех + -- 3416 строк load_index уже заполнен + -- (open 2741 / limited 346 / closed 329, NULL 0) + -- и COALESCE не проваливался. capacity_source = 'eesk_35_220', reserve_asof = :asof WHERE sc_name_norm = :name_norm diff --git a/backend/tests/sql/test_auth_sql_migrations.py b/backend/tests/sql/test_auth_sql_migrations.py index 3c02f7b9..29a14105 100644 --- a/backend/tests/sql/test_auth_sql_migrations.py +++ b/backend/tests/sql/test_auth_sql_migrations.py @@ -1,11 +1,11 @@ """Инварианты миграций БД `auth` (data/sql/auth/*.sql) + её bootstrap (ops/db-bootstrap/*.sql). -Прецедента manifest-теста для КОРНЕВОГО data/sql в этом репозитории нет (он есть только -в tradein: tradein-mvp/backend/tests/test_migrations_manifest.py по -tradein-mvp/backend/data/sql/_manifest_applied.txt). Заводить manifest на 154 legacy-файла -корневого каталога — не задача этого PR, поэтому здесь проверяются инварианты, которые -можно проверить БЕЗ снимка «уже применённого»: они выполнимы на новом каталоге с первого -дня и ловят регрессии, которые иначе всплывают только на проде во время деплоя. +Снимка «уже применённого» здесь нет и не нужно: у соседнего стека такой файл-список был +(tradein data/sql/_manifest_applied.txt) и его удалили в #2683 — он отставал от каталога +и по построению не мог покраснеть. Аналог гейта для tradein теперь берёт эталон из git: +tradein-mvp/backend/tests/test_migration_numbering.py. Здесь же проверяются инварианты, +выполнимые БЕЗ всякого эталона: они верны на новом каталоге с первого дня и ловят +регрессии, которые иначе всплывают только на проде во время деплоя. Тест не требует БД — только чтение файлов. """ diff --git a/backend/tests/test_eesk_reserve_loader.py b/backend/tests/test_eesk_reserve_loader.py index df4dff70..3a1c9032 100644 --- a/backend/tests/test_eesk_reserve_loader.py +++ b/backend/tests/test_eesk_reserve_loader.py @@ -184,10 +184,36 @@ def test_load_ps_35_220_parse_and_match() -> None: assert first["installed"] == 40.0 assert first["reserve"] == 15.0 assert first["district"] == "Ленинский" - assert first["load_pct"] == 41.0 # доля 0.41 → 41.0% assert first["asof"] == date(2026, 6, 30) +def test_load_ps_35_220_does_not_write_load_percent() -> None: + """#2464-B: степень загрузки НЕ уходит в UPDATE и не попадает в load_index. + + Раньше значение колонки E писалось как + `load_index = COALESCE(load_index, CAST(:load_pct AS text))`. load_index — + категориальная колонка ('open'|'limited'|'closed'|NULL, + data/sql/180_connection_capacity.sql:35): число строкой фронт отбрасывает + в «неизвестно» (classifyLoadIndex), а в power_summary.by_load_index + появлялся бы бакет с именем вроде "41.0". + + На проде не стреляло только потому, что load_index заполнен у всех строк + (open 2741 / limited 346 / closed 329, NULL 0 — замер верификации 13.08), + и COALESCE не проваливался. + """ + from datetime import date + + db = _FakeSession(scalar_value=None, rowcount=1) + ee.load_ps_35_220(db, _build_ps_workbook(), date(2026, 6, 30)) + + # Комментарии из SQL убираем: слово load_index встречается в пояснении, + # а проверять надо ИСПОЛНЯЕМЫЙ текст, а не прозу вокруг него. + sql_code = "\n".join(line.split("--", 1)[0] for line in str(db.calls[0][0]).splitlines()) + assert "load_index" not in sql_code, sql_code + for _sql, params in db.calls: + assert "load_pct" not in params, params + + def test_load_ps_35_220_unmatched_counted() -> None: """ПС без совпадения (rowcount=0 — напр. не ЕЭСК) → unmatched, не падаем.""" from datetime import date diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 9e04bd6b..8ee650cb 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -78,6 +78,16 @@ services: image: postgis/postgis:16-3.4 logging: *default-logging restart: unless-stopped + # #2812: /dev/shm под dynamic_shared_memory_type=posix. Умолчание Docker — 64 МБ, + # и параллельные планы кладут туда свои DSM-сегменты. Прод-замер 2026-08-10: + # база постоянно держит ~9.8 МиБ (DSA кумулятивной статистики pgstat), один + # параллельный запрос Объектива берёт ~15.4 МиБ → 4-й одновременный не влезает + # в 64 МиБ и падает `DiskFull: could not resize shared memory segment`. Ровно это + # и случилось: 6 отказов за 1.2 с (market_metrics / sales_series / special_indices). + # 1 ГиБ = ~65 таких запросов; потолок celery (--concurrency=8) + request-path ≈ 12. + # tmpfs выделяется ПО ФАКТУ: значение — потолок, не резерв (0 Б до первого запроса). + # Rollback = убрать строку (снова 64 МиБ) + пересоздать контейнер. + shm_size: 1gb environment: POSTGRES_DB: ${POSTGRES_DB} POSTGRES_USER: ${POSTGRES_USER} @@ -331,9 +341,19 @@ services: REDIS_URL: redis://redis:6379/2 SECRET_KEY: ${GLITCHTIP_SECRET} PORT: "8080" - EMAIL_URL: consolemail:// + # Почта отключена по умолчанию: consolemail:// печатает письмо в stdout и + # никуда его не отправляет. Реальный адрес приходит из /opt/gendesign/.env + # (GLITCHTIP_EMAIL_URL) — в репозитории пароля почтового ящика быть не должно. + # + # ВАЖНО про схему DSN (django-environ, парсер GlitchTip): для порта 465 с + # implicit SSL нужна схема smtp+ssl://, а НЕ smtps:// — вторая помечена + # deprecated и включает STARTTLS (EMAIL_USE_TLS), то есть 465 с ней рвёт + # соединение. Для 587/STARTTLS схема — smtp+tls://. + # smtp+ssl://alerts%40meraocenka.ru:ПАРОЛЬ@smtp.beget.com:465 + # Логин — почтовый адрес целиком, @ в нём кодируется как %40. + EMAIL_URL: ${GLITCHTIP_EMAIL_URL:-consolemail://} GLITCHTIP_DOMAIN: https://errors.gendsgn.ru - DEFAULT_FROM_EMAIL: errors@gendsgn.ru + DEFAULT_FROM_EMAIL: ${GLITCHTIP_FROM_EMAIL:-errors@gendsgn.ru} ENABLE_USER_REGISTRATION: "true" ENABLE_ORGANIZATION_CREATION: "false" restart: always @@ -365,9 +385,27 @@ services: REDIS_URL: redis://redis:6379/2 SECRET_KEY: ${GLITCHTIP_SECRET} CELERY_WORKER_AUTOSCALE: "1,3" + # Письма и веб-хуки шлёт celery, то есть ИМЕННО этот контейнер, а не web. + # До этой правки почтовых переменных здесь не было вовсе: настройка одного + # glitchtip-web не дала бы ни одного отправленного письма — worker брал + # умолчания образа. Значения обязаны совпадать с web (см. комментарий там). + EMAIL_URL: ${GLITCHTIP_EMAIL_URL:-consolemail://} + DEFAULT_FROM_EMAIL: ${GLITCHTIP_FROM_EMAIL:-errors@gendsgn.ru} + # Нужен для абсолютных ссылок внутри писем и веб-хуков: без него + # уведомление приходит со ссылкой в никуда. + GLITCHTIP_DOMAIN: https://errors.gendsgn.ru restart: always mem_limit: 384m - networks: [default] + # GlitchTip → Telegram алерты (мониторинг был нем, аудит 2026-08-15, + # см. tradein-mvp/backend/app/api/v1/glitchtip.py): вебхуки шлёт РЕАЛЬНО + # этот celery-воркер (apps/alerts/webhooks.py send_webhook — не glitchtip-web), + # получателю `webhook` нужен доступ к http://tradein-backend:8000/... — + # tradein-backend сидит на gendesign_shared, у glitchtip-* её раньше не было + # вообще (та же грабля, что #2709 у redis: сеть должна быть общей ДО того, + # как переменная окружения с URL вообще имеет смысл). default — обязательно + # явно, иначе воркер потеряет Postgres/Redis-брокер (см. комментарий у redis + # выше про неявную привязку к default). + networks: [default, shared] caddy: image: caddy:2 diff --git a/ops/docker-prune.sh b/ops/docker-prune.sh index 31ca32d1..34a0d30d 100755 --- a/ops/docker-prune.sh +++ b/ops/docker-prune.sh @@ -18,7 +18,15 @@ # FORGEJO-ACTIONS-TASK-*. Именованные тома со смыслом (gendesign_postgres_data, # tradein-postgres-data, *_caddy_*, couchdb, redis и любые будущие) не трогаются # НИКОГДА — даже если в моменте оказались отцеплены. Голый `docker volume prune` -# такой разницы не делает, поэтому здесь он намеренно не используется. +# такой разницы не делает, поэтому здесь он намеренно не используется; +# - зависшие (running, но фактически брошенные) job-контейнеры раннера Forgejo +# Actions старше JOB_CONTAINER_MAX_AGE_HOURS. 2026-08-15: живьём на проде +# обнаружены три штуки в статусе Up 4-8 недель (раннер не убрал контейнер +# после прерванного/упавшего workflow — task killed, рестарт раннера в +# процессе job'а и т.п.). CI job физически не идёт сутками, поэтому что +# угодно с этим именем старше порога — гарантированный мусор, а не активная +# задача. `docker container prune` их не видит: тот фильтрует только +# status=exited, а эти контейнеры формально Up. # # Usage (cron на прод-VM; `bash <путь>`, а не голый путь — тогда снятый +x не ломает). # Лог в /tmp — как у соседних записей в том же crontab (backup.sh, backfill'ы): @@ -35,6 +43,9 @@ set -euo pipefail DRY_RUN="${DRY_RUN:-0}" STOPPED_AGE="${STOPPED_AGE:-24h}" IMAGE_AGE="${IMAGE_AGE:-168h}" +# Job CI никогда не идёт сутками — что угодно с именем job-контейнера раннера +# старше этого порога снимается безусловно (см. секцию 4 ниже). +JOB_CONTAINER_MAX_AGE_HOURS="${JOB_CONTAINER_MAX_AGE_HOURS:-24}" log() { printf '%s %s\n' "$(date -u +'%Y-%m-%dT%H:%M:%SZ')" "$*"; } @@ -93,6 +104,73 @@ else log "томов удалено: ${removed} из ${#candidates[@]}" fi +# ── 4. зависшие job-контейнеры раннера Forgejo Actions ─────────────────────── +# Фильтр по имени — ЯКОРЬ на начало (`^FORGEJO-ACTIONS-TASK-`), не "содержит +# подстроку": `docker ps --filter name=` матчит как regex, поэтому `^...` +# гарантирует точный префикс, а не случайное совпадение где-то в середине +# имени сервисного контейнера. Долгоживущие сервисные контейнеры (forgejo, +# forgejo-runner*, gendesign-*, tradein-*, couchdb) под этот префикс не +# подпадают вообще — но ниже всё равно есть explicit-skip как страховка на +# случай будущего переименования, а не молчаливая надежда на то, что фильтр +# никогда не ошибётся. +# +# Возраст — из `docker inspect .State.StartedAt` (RFC3339), НЕ из текстового +# "Up 4 weeks" в выводе `docker ps`: тот округляет к ближайшей крупной единице +# и не пригоден для сравнения с порогом в часах. +mapfile -t job_ids < <(docker ps -aq --filter "name=^FORGEJO-ACTIONS-TASK-" || true) + +job_removed=0 +job_candidates=0 +if [[ "${#job_ids[@]}" -eq 0 ]]; then + log "зависших job-контейнеров нет" +else + for id in "${job_ids[@]}"; do + name="$(docker inspect --format '{{.Name}}' "$id" 2>/dev/null | sed 's#^/##' || true)" + [[ -z "$name" ]] && continue + + case "$name" in + forgejo | forgejo-runner* | gendesign-* | tradein-* | couchdb) + log "job-контейнеры: ПРОПУЩЕН сервисный '${name}' (не должен был пройти фильтр имени)" + continue + ;; + esac + + started_at="$(docker inspect --format '{{.State.StartedAt}}' "$id" 2>/dev/null || true)" + [[ -z "$started_at" || "$started_at" == "0001-01-01T00:00:00Z" ]] && continue + + started_epoch="$(date -u -d "$started_at" +%s 2>/dev/null || echo 0)" + [[ "$started_epoch" -eq 0 ]] && continue + + now_epoch="$(date -u +%s)" + age_hours=$(((now_epoch - started_epoch) / 3600)) + [[ "$age_hours" -lt "$JOB_CONTAINER_MAX_AGE_HOURS" ]] && continue + + job_candidates=$((job_candidates + 1)) + size="$(docker ps -a --filter "id=${id}" --size --format '{{.Size}}' 2>/dev/null \ + | awk '{print $1}' || true)" + + if [[ "$DRY_RUN" == "1" ]]; then + log "job-контейнеры: [dry-run] снял бы '${name}' (возраст ${age_hours}ч, writable-слой ${size:-?})" + continue + fi + + if docker rm -f "$id" >/dev/null 2>&1; then + job_removed=$((job_removed + 1)) + log "job-контейнеры: снят '${name}' (возраст ${age_hours}ч, writable-слой ${size:-?} освобождён)" + else + log "job-контейнеры: НЕ удалось снять '${name}' (id ${id:0:12})" + fi + done + + if [[ "$job_candidates" -eq 0 ]]; then + log "job-контейнеры: ${#job_ids[@]} шт., ни один не старше порога ${JOB_CONTAINER_MAX_AGE_HOURS}ч" + elif [[ "$DRY_RUN" == "1" ]]; then + log "job-контейнеры: к снятию ${job_candidates} из ${#job_ids[@]}" + else + log "job-контейнеры: снято ${job_removed} из ${job_candidates} кандидатов (порог ${JOB_CONTAINER_MAX_AGE_HOURS}ч)" + fi +fi + after_pct="$(disk_used_pct)" log "готово: диск занят ${after_pct}% (было ${before_pct}%)" diff --git a/ops/journald-gendesign.conf.example b/ops/journald-gendesign.conf.example new file mode 100644 index 00000000..414f8236 --- /dev/null +++ b/ops/journald-gendesign.conf.example @@ -0,0 +1,36 @@ +# systemd-journald drop-in — cap persistent journal disk usage on prod VPS. +# +# ЗАМЕР 2026-08-15 (ssh gendesign, read-only): `/var/log` занимал 3.1G. Наивная +# первая проверка `journalctl --disk-usage` показала только 174M и навела на +# ложный след «основной объём — не journald». На деле `journalctl --disk-usage`, +# запущенный НЕ из группы systemd-journal/adm, недосчитывает — он не может +# полноценно перечислить архивные *.journal файлы без прав на чтение. Прямой +# `du -sh /var/log/journal` дал 2.5G — это ~80% всего `/var/log`, ровно 100 +# файлов по ~48M в /var/log/journal//. Второй по размеру вклад — +# традиционный rsyslog (syslog/syslog.1/auth.log/kern.log/ufw.log/dmesg/btmp, +# ~0.6G) — те уже ротируются через logrotate (видны .1/.4.gz копии), отдельного +# вмешательства не требуют и вне scope этого файла. +# +# В /etc/systemd/journald.conf на проде НЕТ SystemMaxUse (все ключи закомменчены +# дефолтами) — без явного лимита journald довольствуется default-правилом +# «до 10% файловой системы», на VPS с диском ~145G это фактически безлимит. +# +# УСТАНОВКА НА СЕРВЕРЕ (руками, deploy.yml этот файл НЕ подхватывает — +# systemd-конфиги вне /opt/gendesign, деплой синкает только сам репозиторий): +# sudo mkdir -p /etc/systemd/journald.conf.d +# sudo cp /opt/gendesign/ops/journald-gendesign.conf.example \ +# /etc/systemd/journald.conf.d/gendesign-max-use.conf +# sudo systemctl restart systemd-journald +# +# `restart systemd-journald` применяет лимит немедленно — journald сам +# провакуумит существующие архивные файлы вниз до SystemMaxUse (ожидаемый +# эффект: /var/log/journal схлопнется примерно с 2.5G до ~500M). Это НЕ +# `docker volume rm` / `caddy reload` — под общий deploy-guard не подпадает, +# но всё равно на живом проде: делает user сам после ревью PR. +# +# Значение 500M — консервативный запас на 4 vCPU/4-16G VPS с активным CI +# (docker/forgejo-runner логи в journald тоже льются). При необходимости +# больше retention для дебага — поднять SystemMaxUse, не удалять файл. + +[Journal] +SystemMaxUse=500M diff --git a/scripts/smoke-mera-perimeter.sh b/scripts/smoke-mera-perimeter.sh index 6c433b67..ea45311a 100644 --- a/scripts/smoke-mera-perimeter.sh +++ b/scripts/smoke-mera-perimeter.sh @@ -154,6 +154,27 @@ check "gendsgn.ru/api/v1/admin/* — 401 anonymous" "$BASE_MAIN/api/v1/admin/use check "merahome.ru — 301 to canonical" "https://merahome.ru/" 301 check "meraotsenka.ru — 301 to canonical" "https://meraotsenka.ru/" 301 +# 6. Платёжный периметр (PR-D2) — готовит почву под PR-D3 (роутер) и PR-D4 +# (Caddy), но САМ НИЧЕГО НЕ ОТКРЫВАЕТ. Ожидаем закрытое состояние С ОБЕИХ +# СТОРОН прямо сейчас: +# - meraocenka.ru вообще не проксирует /trade-in/api/* (allowlist-by-default, +# см. проверку 2) — 404 от Caddy, до бэкенда не доходит; +# - gendsgn.ru проксирует /trade-in/api/* в tradein-backend, но rbac_guard +# (`_PUBLIC_PATHS` в app/core/rbac.py — ЭТОТ PR её не трогает) не знает +# платёжные пути и требует X-Authenticated-User → 401 анониму. +# Если один из этих чек-ов вдруг перестанет быть 404/401 РАНЬШЕ мержа +# PR-D3/PR-D4 — это и есть преждевременная утечка периметра, которую ловит +# этот смоук (канарейка: осознанно станет красной, когда PR-D3/PR-D4 явно +# откроют эти пути — тогда ожидания здесь надо обновить вместе с ними). +check "meraocenka.ru payments/notify — must 404 (Caddy не проксирует, PR-D4)" \ + "$BASE_MERA/trade-in/api/v1/trade-in/payments/notify" 404 +check "meraocenka.ru payments/checkout — must 404 (Caddy не проксирует, PR-D4)" \ + "$BASE_MERA/trade-in/api/v1/trade-in/payments/checkout" 404 +check "trade-in payments/notify — 401 anonymous (rbac закрыт до PR-D3)" \ + "$BASE_MAIN/trade-in/api/v1/trade-in/payments/notify" 401 +check "trade-in payments/checkout — 401 anonymous (rbac закрыт до PR-D3)" \ + "$BASE_MAIN/trade-in/api/v1/trade-in/payments/checkout" 401 + echo "========================================" if [ "$fail" -eq 0 ]; then echo "ALL CHECKS PASSED" diff --git a/tradein-mvp/backend/app/api/v1/glitchtip.py b/tradein-mvp/backend/app/api/v1/glitchtip.py new file mode 100644 index 00000000..0f051ee7 --- /dev/null +++ b/tradein-mvp/backend/app/api/v1/glitchtip.py @@ -0,0 +1,219 @@ +"""GlitchTip → Telegram алерты (мониторинг сейчас нем: `alerts_projectalert`/ +`alerts_alertrecipient` пусты, `EMAIL_URL=consolemail://` печатает письма в +stdout и никуда их не доставляет — аудит на проде 2026-08-15). + +GlitchTip (self-hosted, `errors.gendsgn.ru`, образ `glitchtip/glitchtip:6.1.6`) +умеет слать получателю типа `webhook` (``RecipientType.GENERAL_WEBHOOK`` — +"General Slack-compatible webhook"). И issue-алерты (``apps/alerts/webhooks.py +send_issue_as_webhook``), и uptime-алерты (``apps/uptime/webhooks.py +_send_uptime_generic``) в итоге идут через ОДНУ И ТУ ЖЕ низкоуровневую +``send_webhook()`` — ``aiohttp.ClientSession.post(url, json=asdict(WebhookPayload +(text=..., attachments=[...])))``, БЕЗ каких-либо заголовков (ни Authorization, +ни подписи, ни X-*). Значит: + 1) тело запроса для issue и uptime алертов структурно ОДИНАКОВОЕ — + ``{"text": str, "attachments": [{"title","title_link","text","color", + "fields",...}]}`` — просто у uptime пустые/отсутствующие ``fields``/``color``; + 2) единственный канал для аутентификации — сам URL (как и Slack-вебхуки). + Секрет ОБЯЗАН ехать query-параметром, HTTP-заголовок здесь поставить + нечем (GlitchTip-сторона его не добавляет). + +Переиспользуем существующий ``TRADEIN_INTERNAL_AUTH_SECRET`` (#2213 +defense-in-depth, см. ``app.core.rbac``) вместо нового секрета — тот же +``secrets.compare_digest`` constant-time compare, тот же env. Отличие от +rbac-паттерна: ТАМ пустой секрет — fail-open (есть второй рубеж, roles.yaml). +ЗДЕСЬ секрет — единственный рубеж вообще, поэтому пустой секрет ИЛИ +несконфигурированный Telegram-бот → 503 "не настроено", а не тихий +fail-open настежь. + +Путь ФИКСИРОВАННЫЙ (не несёт секрет в себе) — так его можно добавить в +``app.core.rbac._PUBLIC_PATHS`` одной строкой (точное совпадение, без +regex/prefix-веток в ``rbac_guard``). Сам путь — не секрет, секрет — только +значение query-параметра. + +Сетевая связность (docker-compose.prod.yml, корневой стек): вебхуки шлёт +``glitchtip-worker`` (celery-таска), НЕ ``glitchtip-web`` — оба сейчас сидят +только в ``gendesign_default``. tradein-backend слушает на ``gendesign_shared`` +(алиас неявный — Docker embedded DNS резолвит по ``container_name``, тот же +приём уже используется Caddy → ``tradein-backend:8000``, см. Caddyfile). +Значит ``glitchtip-worker`` тоже должен быть подписан на ``gendesign_shared``, +иначе имя ``tradein-backend`` не резолвится — общей сети нет. +""" + +from __future__ import annotations + +import json +import logging +import secrets +from datetime import UTC, datetime +from typing import Annotated, Any + +from fastapi import APIRouter, HTTPException, Query, Request +from pydantic import BaseModel, ConfigDict, ValidationError + +from app.core.config import settings +from app.services.tgbot.client import TelegramApiError, TelegramClient + +logger = logging.getLogger(__name__) + +router = APIRouter() + +# Telegram sendMessage лимит — 4096 символов (см. support.py MAX_MESSAGE_LENGTH +# для исходящих сообщений пользователя; здесь лимит на ИСХОДЯЩЕЕ в Telegram, тот +# же потолок). Суффикс обрезки учтён в _truncate. +_TELEGRAM_MAX_LEN = 4096 +_TRUNCATE_SUFFIX = "\n… (обрезано)" + +# Узкий интерактивный бюджет (тот же принцип, что #tgsupport-web review H1 в +# support.py): GlitchTip-таска ждёт HTTP-ответ синхронно (её собственный aiohttp +# timeout=10s), поэтому наш путь не может тянуть воркерные 5 ретраев/минуты. +_INTERACTIVE_SEND_TIMEOUT_S = 8.0 +_INTERACTIVE_SEND_MAX_RETRIES = 1 + + +class GlitchTipAttachment(BaseModel): + """Slack-совместимый attachment. Issue- и uptime-алерты заполняют РАЗНЫЕ + подмножества полей (uptime не шлёт ``fields``/``color``) — все опциональны, + ``extra="allow"`` на случай будущих версий GlitchTip.""" + + model_config = ConfigDict(extra="allow") + + title: str | None = None + title_link: str | None = None + text: str | None = None + color: str | None = None + fields: list[dict[str, Any]] | None = None + + +class GlitchTipWebhookPayload(BaseModel): + """Тело POST от GlitchTip ``send_webhook()`` — одинаковое для issue- и + uptime-алертов (см. docstring модуля).""" + + model_config = ConfigDict(extra="allow") + + text: str | None = None + attachments: list[GlitchTipAttachment] | None = None + + +def _truncate(text: str, limit: int = _TELEGRAM_MAX_LEN) -> str: + if len(text) <= limit: + return text + return text[: limit - len(_TRUNCATE_SUFFIX)] + _TRUNCATE_SUFFIX + + +def _field_value(attachment: GlitchTipAttachment, label: str) -> str | None: + """Ищет значение поля attachment.fields по title (issue-алерты кладут туда + "Project" литералом — см. apps/alerts/webhooks.py send_issue_as_webhook).""" + for field in attachment.fields or []: + if str(field.get("title", "")).strip().lower() == label.lower(): + value = field.get("value") + return str(value) if value is not None else None + return None + + +def _format_known_payload(payload: GlitchTipWebhookPayload, received_at: datetime) -> str: + lines = [f"GlitchTip: {payload.text or 'Alert'}"] + for attachment in payload.attachments or []: + block: list[str] = [] + project = _field_value(attachment, "Project") + if project: + block.append(f"Проект: {project}") + if attachment.title: + block.append(attachment.title) + if attachment.text: + block.append(attachment.text) + if attachment.title_link: + block.append(f"Ссылка: {attachment.title_link}") + if block: + lines.append("") + lines.extend(block) + lines.append("") + lines.append(f"Получено: {received_at.strftime('%Y-%m-%d %H:%M:%S')} UTC") + return "\n".join(lines) + + +def _format_unknown_payload(raw_body: bytes, received_at: datetime) -> str: + """Payload не распознан ни как issue-, ни как uptime-алерт (нет ни `text`, + ни `attachments`, либо тело — не JSON-объект вовсе) — не роняем запрос, + пересылаем как есть с пометкой (см. требование задачи: неизвестная форма + payload не должна давать 500).""" + text_repr = raw_body.decode("utf-8", errors="replace") + header = "GlitchTip webhook: неизвестный формат payload, пересылаю как есть" + return _truncate( + f"{header}\n\n{text_repr}\n\nПолучено: {received_at.strftime('%Y-%m-%d %H:%M:%S')} UTC" + ) + + +def _build_message(raw_body: bytes, received_at: datetime) -> str: + try: + data = json.loads(raw_body) + except (json.JSONDecodeError, UnicodeDecodeError): + return _format_unknown_payload(raw_body, received_at) + + if not isinstance(data, dict): + return _format_unknown_payload(raw_body, received_at) + + try: + payload = GlitchTipWebhookPayload.model_validate(data) + except ValidationError: + return _format_unknown_payload(raw_body, received_at) + + if payload.text is None and not payload.attachments: + return _format_unknown_payload(raw_body, received_at) + + return _truncate(_format_known_payload(payload, received_at)) + + +def _alerts_configured() -> bool: + """Все три части ОБЯЗАНЫ быть заданы: секрет (auth), токен бота, chat_id + темы алертов. Отсутствие любой — 503, а не тихий no-op и не fail-open.""" + return bool( + settings.tradein_internal_auth_secret + and settings.telegram_bot_token + and settings.telegram_alerts_chat_id + ) + + +def _verify_secret(provided: str) -> None: + expected = settings.tradein_internal_auth_secret + if not secrets.compare_digest(provided or "", expected): + logger.warning("glitchtip webhook: invalid or missing secret query param") + raise HTTPException(status_code=401, detail="invalid or missing secret") + + +@router.post("/ops/glitchtip-webhook") +async def glitchtip_webhook( + request: Request, + secret: Annotated[str, Query()] = "", +) -> dict[str, str]: + """Приёмник GlitchTip webhook-алертов (issue + uptime) → пересылка в + Telegram-тему алертов (``TELEGRAM_ALERTS_CHAT_ID``/``TELEGRAM_ALERTS_TOPIC_ID`` + — ОТДЕЛЬНАЯ тема от support-топика, см. docstring модуля). + + Путь публичный в ``rbac_guard`` (``app.core.rbac._PUBLIC_PATHS``) — этот + хендлер сам делает единственную проверку (``secret`` query-параметр). + """ + if not _alerts_configured(): + raise HTTPException(status_code=503, detail="glitchtip alerts webhook not configured") + + _verify_secret(secret) + + raw_body = await request.body() + received_at = datetime.now(UTC) + text = _build_message(raw_body, received_at) + + client = TelegramClient(settings.telegram_bot_token) + try: + await client.send_message( + chat_id=settings.telegram_alerts_chat_id, + text=text, + message_thread_id=settings.telegram_alerts_topic_id or None, + # review H1-style бюджет (см. support.py) — синхронный HTTP-путь не + # может легально висеть воркерные минуты ретраев. + timeout=_INTERACTIVE_SEND_TIMEOUT_S, + max_retries=_INTERACTIVE_SEND_MAX_RETRIES, + ) + except TelegramApiError: + logger.exception("glitchtip webhook: не удалось переслать алерт в Telegram") + raise HTTPException(status_code=502, detail="failed to forward alert to telegram") from None + + return {"status": "ok"} diff --git a/tradein-mvp/backend/app/core/config.py b/tradein-mvp/backend/app/core/config.py index 8d6cf935..cac916e8 100644 --- a/tradein-mvp/backend/app/core/config.py +++ b/tradein-mvp/backend/app/core/config.py @@ -997,6 +997,15 @@ class Settings(BaseSettings): # message_thread_id топика внутри support-группы, в который идут зеркала. telegram_support_topic_id: int = Field(default=0, validation_alias="TELEGRAM_SUPPORT_TOPIC_ID") + # ── GlitchTip → Telegram алерты (мониторинг был нем, аудит 2026-08-15) ── + # Отдельная тема от TELEGRAM_SUPPORT_TOPIC_ID выше — алерты об ошибках прода + # НЕ должны литься в топик, куда пишут живые клиенты. См. app/api/v1/glitchtip.py. + # Пусто/0 = вебхук отвечает 503 "not configured" (fail-closed, не fail-open — + # это единственный auth-рубеж эндпоинта, в отличие от rbac-путей). + # ENV: TELEGRAM_ALERTS_CHAT_ID, TELEGRAM_ALERTS_TOPIC_ID. + telegram_alerts_chat_id: int = Field(default=0, validation_alias="TELEGRAM_ALERTS_CHAT_ID") + telegram_alerts_topic_id: int = Field(default=0, validation_alias="TELEGRAM_ALERTS_TOPIC_ID") + # ── Платёжный контур МЕРЫ (Т-Банк эквайринг) — схема-only PR-B ────────── # См. `mera-tbank-acquiring-recon.md` в корне репо. Этот PR НЕ содержит # роутеров/httpx-клиента/подписи Token — только поля конфига и kill-switch. diff --git a/tradein-mvp/backend/app/core/ratelimit.py b/tradein-mvp/backend/app/core/ratelimit.py index f5f3fe04..cda736bb 100644 --- a/tradein-mvp/backend/app/core/ratelimit.py +++ b/tradein-mvp/backend/app/core/ratelimit.py @@ -32,6 +32,18 @@ from starlette.middleware.base import BaseHTTPMiddleware from app.core.config import settings +# Платёжная нотификация Т-Банка (PR-D2, готовит почву под PR-D3 — путь ещё +# закрыт rbac до того момента). Сервер-к-серверу, без сессии/X-Authenticated-User +# → в общем лимитере попал бы в один и тот же per-IP ключ с любым другим +# анонимным трафиком с той же исходящей сети банка. Мотив НЕ «банк упрётся в +# лимит» — 300/60с и так щедро — а «429 никогда не должен стать причиной, по +# которой денежное состояние разъехалось»: для банка недоставленная нотификация +# = «доставка не удалась», альтернативного канала нет, а очередь ретраев +# растягивается на сутки. Только точный путь notify — НЕ checkout (тот +# инициирует пользователь с сессией/курсором в браузере, абуз там штатно +# лимитируем как любой другой API-путь). +_PAYMENTS_NOTIFY_PATH = "/api/v1/trade-in/payments/notify" + class RateLimitMiddleware(BaseHTTPMiddleware): """Sliding-window rate limit на /api/v1/*. Health и статика — без лимита.""" @@ -42,6 +54,23 @@ class RateLimitMiddleware(BaseHTTPMiddleware): async def dispatch(self, request: Request, call_next): # type: ignore[no-untyped-def] path = request.url.path + # Платёжная нотификация — мимо ОБЩЕГО (per-user/per-IP shared) лимитера, + # но НЕ без лимита вовсе: idiom `_notify_limiter` (`SlidingWindowLimiter`, + # тот же приём, что `support.py:92`/`:319` — узкий per-feature бюджет + # ВМЕСТО общего, не полное отключение защиты). Порог заведомо выше любого + # штатного трафика банка (документированное расписание ретраев неизвестно, + # см. mera-tbank-acquiring-recon.md — берём с кратным запасом), но конечен: + # полное отключение оставило бы путь без backstop против шторма запросов — + # подпись отсекает мусор ПОСЛЕ разбора тела (PR-D3), не до. + if path == _PAYMENTS_NOTIFY_PATH: + retry_after = _notify_limiter.check(_client_ip(request)) + if retry_after is not None: + return JSONResponse( + status_code=429, + content={"detail": "Слишком много запросов. Попробуйте позже."}, + headers={"Retry-After": str(int(retry_after) + 1)}, + ) + return await call_next(request) # Лимитируем только API; health и прочее — пропускаем. if not path.startswith("/api/"): return await call_next(request) @@ -143,6 +172,16 @@ class SlidingWindowLimiter: return None +# Щедрый бюджет для платёжной нотификации (PR-D2): 3000/60с (50 req/s) — на два +# порядка выше любого правдоподобного трафика банка (тест 400/60с проходит с +# запасом в 7.5×), но конечен — backstop против шторма запросов на путь, где +# подпись проверяется уже ПОСЛЕ разбора тела. Ключ — client IP (у сервер-к- +# серверу вызова нет сессии/X-Authenticated-User). +_NOTIFY_RATE_LIMIT = 3000 +_NOTIFY_RATE_WINDOW_S = 60.0 +_notify_limiter = SlidingWindowLimiter(limit=_NOTIFY_RATE_LIMIT, window_s=_NOTIFY_RATE_WINDOW_S) + + def _client_ip(request: Request) -> str: """Честный клиентский IP при РОВНО ОДНОМ доверенном прокси (Caddy) перед нами. diff --git a/tradein-mvp/backend/app/core/rbac.py b/tradein-mvp/backend/app/core/rbac.py index bf7e7ead..56833013 100644 --- a/tradein-mvp/backend/app/core/rbac.py +++ b/tradein-mvp/backend/app/core/rbac.py @@ -86,6 +86,12 @@ _PUBLIC_PATHS = frozenset( # не секрет, читает только process env — быстрая справка для клиента/ # поддержки/смоук-теста, не должна требовать сессию. "/api/v1/trade-in/version", + # GlitchTip webhook → Telegram (app/api/v1/glitchtip.py): вызывается + # ИЗ glitchtip-worker (docker-сеть gendesign_shared), не может нести + # X-Authenticated-User/сессию. Путь фиксированный и не секрет — секрет + # это query-параметр `secret`, который проверяет сам хендлер + # (secrets.compare_digest против TRADEIN_INTERNAL_AUTH_SECRET). + "/api/v1/trade-in/ops/glitchtip-webhook", # Публичный B2C-периметр МЕРЫ (meraocenka.ru): у посетителя лендинга # идентичности нет и не будет — Caddy на этом домене вообще без # basic_auth. Обе ручки только читают (SELECT/прокси автокомплита) и не diff --git a/tradein-mvp/backend/app/core/request_audit.py b/tradein-mvp/backend/app/core/request_audit.py index 7eafefbc..9416b556 100644 --- a/tradein-mvp/backend/app/core/request_audit.py +++ b/tradein-mvp/backend/app/core/request_audit.py @@ -40,7 +40,29 @@ logger = logging.getLogger(__name__) # Зеркалит app.main._PUBLIC_PATHS. Не импортируем напрямую из app.main — оно # импортирует этот модуль (регистрирует middleware), обратный импорт дал бы # циклическую зависимость. -_PUBLIC_PATHS = frozenset({"/health", "/docs", "/redoc", "/openapi.json"}) +# +# PR-D2: `/api/v1/trade-in/payments/notify` — заранее в skip-набор (defense-in- +# depth), хотя rbac ещё закрывает этот путь до PR-D3. Причины две: +# 1) сам путь не должен попадать в аудит вообще — тело нотификации содержит +# `Token`/`Pan`/`ExpDate` (см. `app/main.py._before_send`, тот же мотив, что +# и вырезание тела из мониторинга); хоть это middleware само по себе тело +# запроса в payload не пишет (только status_code/path/method), путь не +# должен зависеть от того, что кто-то потом добавит поле "body" в событие; +# 2) НЕ авторизующая проверка: `RequestAuditMiddleware` внешний относительно +# `rbac_guard` и читает сырой `X-Authenticated-User` (см. `main.py` порядок +# middleware) — анонимный POST на notify с подделанным заголовком +# `X-Authenticated-User: admin` иначе писал бы фальшивые события в +# `user_events` с атрибуцией admin, при этом rbac при этом ничего не знает +# (сам гейт отдельно, 401 всё равно вернёт до PR-D3). +_PUBLIC_PATHS = frozenset( + { + "/health", + "/docs", + "/redoc", + "/openapi.json", + "/api/v1/trade-in/payments/notify", + } +) # Методы, меняющие состояние — для /api/v1/admin/* именно они должны попадать в # аудит с атрибуцией (кто именно загрузил куки / включил авто-логин / поправил diff --git a/tradein-mvp/backend/app/main.py b/tradein-mvp/backend/app/main.py index 8b5dbab2..a3d16343 100644 --- a/tradein-mvp/backend/app/main.py +++ b/tradein-mvp/backend/app/main.py @@ -28,6 +28,7 @@ from app.api.v1 import ( brand, buildings, geocode, + glitchtip, lead, me, privacy_admin, @@ -69,20 +70,32 @@ logging.getLogger("httpx").setLevel(logging.WARNING) if settings.glitchtip_dsn: from app.observability.sentry_scrub import ( redact_telegram_bot_token, + scrub_payment_request_body, stabilize_retry_error_fingerprint, ) def _before_send(event: dict[str, object], hint: dict[str, object]) -> dict[str, object] | None: - """Композиция PII-scrub + Telegram bot-токен redaction (#tgsupport-web) + - RetryError fingerprint-стабилизация (glitchtip-noise) — см. - app/tgbot_main.py._before_send (та же композиция без последнего шага, - тот бот geocoder не зовёт). PII/token — тот же риск: теперь этот процесс - тоже держит TelegramClient в стек-фреймах при ошибке sendMessage, а - include_local_variables=False ниже — первый рубеж защиты. RetryError — - этот процесс обслуживает /api/v1/geocode/* (suggest/lookup/reverse), - которые ретраят Nominatim через tenacity; см. + """Композиция платёжный body-wipe + PII-scrub + Telegram bot-токен redaction + + RetryError fingerprint-стабилизация (#tgsupport-web, PR-D2, glitchtip-noise) — + см. app/tgbot_main.py._before_send (идентичная композиция без последнего шага, + тот бот geocoder не зовёт). Тот же риск: теперь этот процесс тоже держит + TelegramClient в стек-фреймах при ошибке sendMessage, а + include_local_variables=False ниже — первый рубеж защиты. + + PR-D2: платёжный body-wipe идёт ПЕРВЫМ шагом, а не заменяет остальные — + режет `request.data` целиком только для `/payments/*`, остальные пути + (extra/contexts/traceback) по-прежнему проходят ключ-based scrub и + token-redaction. Тот же обработчик передан ОБОИМ каналам ниже + (before_send и before_send_transaction) — вчерашний баг в Птице закрыл + только error-канал, transaction-канал остался вообще без обработчика. + + RetryError-стабилизация — этот процесс обслуживает /api/v1/geocode/* + (suggest/lookup/reverse), которые ретраят Nominatim через tenacity; см. sentry_scrub.stabilize_retry_error_fingerprint.""" - scrubbed = scrub_pii_event(event, hint) # type: ignore[arg-type] + scrubbed = scrub_payment_request_body(event, hint) # type: ignore[arg-type] + if scrubbed is None: + return None + scrubbed = scrub_pii_event(scrubbed, hint) # type: ignore[arg-type] if scrubbed is None: return None detokened = redact_telegram_bot_token(scrubbed, hint) # type: ignore[arg-type] @@ -100,6 +113,10 @@ if settings.glitchtip_dsn: # держит base URL с токеном в локальных переменных стек-фрейма — default # sentry_sdk (True) приложил бы их к traceback открытым текстом. before_send=_before_send, + # PR-D2: тот же обработчик на transaction-канал — traces_sample_rate=0.0 + # сегодня не шлёт трейсы вообще, но это belt-and-suspenders на случай, + # если трейсинг когда-нибудь включат (см. docstring _before_send выше). + before_send_transaction=_before_send, integrations=[ StarletteIntegration(), FastApiIntegration(), @@ -252,6 +269,7 @@ app.include_router(trade_in.router, prefix="/api/v1/trade-in", tags=["trade-in"] app.include_router(version.router, prefix="/api/v1/trade-in", tags=["trade-in-version"]) app.include_router(lead.router, prefix="/api/v1/trade-in", tags=["trade-in"]) app.include_router(support.router, prefix="/api/v1/trade-in", tags=["trade-in-support"]) +app.include_router(glitchtip.router, prefix="/api/v1/trade-in", tags=["trade-in-ops"]) app.include_router(buildings.router, prefix="/api/v1/buildings", tags=["buildings"]) app.include_router(search.router, prefix="/api/v1", tags=["search"]) app.include_router(me.router, prefix="/api/v1", tags=["me"]) diff --git a/tradein-mvp/backend/app/observability/sentry_scrub.py b/tradein-mvp/backend/app/observability/sentry_scrub.py index 7cb78fd0..0920486e 100644 --- a/tradein-mvp/backend/app/observability/sentry_scrub.py +++ b/tradein-mvp/backend/app/observability/sentry_scrub.py @@ -29,7 +29,36 @@ from tenacity import RetryError _REDACTED = "[REDACTED]" # Ключи consumer-PII (нижний регистр; сверка case-insensitive). -_PII_KEYS = frozenset({"client_name", "client_phone", "client_email", "phone", "email", "name"}) +# PR-D2 (payments perimeter hardening): + платёжные поля Т-Банка (customer_email/ +# customer_phone из checkout, pan/expdate/cardid/rebillid/token/terminalkey из +# notify) — belt-and-suspenders поверх `scrub_payment_request_body` ниже, которая +# вырезает `request.data` для /payments/* целиком: этот словарь всё равно нужен +# для extra/contexts И на случай, если платёжное поле когда-нибудь попадёт в +# error event НЕ через request.data (напр. кто-то положит его в extra вручную). +_PII_KEYS = frozenset( + { + "client_name", + "client_phone", + "client_email", + "phone", + "email", + "name", + "customer_email", + "customer_phone", + "pan", + "expdate", + "cardid", + "rebillid", + "token", + "terminalkey", + } +) + +# Сегмент пути платёжного периметра (notify + checkout + любой будущий +# /payments/* суб-путь) — PR-D2, готовит почву под PR-D3 (эндпоинты ещё не +# существуют). Матчим по сегменту, не по конкретному эндпоинту, чтобы не +# требовать правки этого файла на каждый новый платёжный путь. +_PAYMENTS_URL_SEGMENT = "/api/v1/trade-in/payments/" # Telegram Bot API токен в пути URL: /bot:/. # Матчим ровно этот сегмент (не весь URL) — сохраняет остальной путь/query @@ -179,6 +208,39 @@ def scrub_pii_event(event: Event, _hint: dict[str, Any]) -> Event | None: return event +def scrub_payment_request_body(event: Event, _hint: dict[str, Any]) -> Event | None: + """Вырезать `event['request']['data']` целиком для платёжных путей (PR-D2). + + Ключ-based `scrub_pii_event` НЕ спасает платёжную нотификацию: sentry_sdk + 2.64 (`integrations/starlette.py`) кладёт ПОЛНОЕ тело запроса в + `event.request.data`, и `send_default_pii=False` этот путь не гейтит — тот + флаг управляет только куками, не телом запроса (проверено живьём на соседнем + продукте). Тело нотификации Т-Банка несёт `Token`/`Pan`/`ExpDate`/`CardId`/ + `RebillId`/`DATA` — банк сам выбирает имена полей, перечислить их все заранее + нельзя, поэтому единственная безопасная стратегия для этого пути — не + отправлять тело целиком, а не пытаться вычистить отдельные ключи. + + Матчим по сегменту `/api/v1/trade-in/payments/` (не по конкретному + эндпоинту) — покрывает notify, checkout и любой будущий суб-путь одним + фильтром, без правки этого файла на каждое расширение платёжного API. + Сравнение регистронезависимое: `_PUBLIC_PATHS` (rbac) — точное множество без + учёта регистра только у Caddy, не у Python, так что нестандартный регистр + пути технически может долететь до обработчика и породить событие. + + Композировать с `scrub_pii_event`/`redact_telegram_bot_token`, а не вместо + них — этот шаг закрывает только `request.data`, extra/contexts и + traceback-locals остаются на ответственности остальных шагов композиции. + """ + if not isinstance(event, dict): + return event + request = event.get("request") + if isinstance(request, dict): + url = request.get("url") + if isinstance(url, str) and _PAYMENTS_URL_SEGMENT in url.lower(): + request.pop("data", None) + return event + + def _redact_strings(obj: Any) -> Any: """Рекурсивно проходит dict/list/tuple и прогоняет обе токен-регулярки по КАЖДОЙ строке (не только по конкретным ключам) — токен может оказаться в locals diff --git a/tradein-mvp/backend/app/scheduler_main.py b/tradein-mvp/backend/app/scheduler_main.py index e5f93ed0..210b30fa 100644 --- a/tradein-mvp/backend/app/scheduler_main.py +++ b/tradein-mvp/backend/app/scheduler_main.py @@ -45,20 +45,31 @@ if settings.glitchtip_dsn: from sentry_sdk.integrations.sqlalchemy import SqlalchemyIntegration from app.observability.sentry_scrub import ( + scrub_payment_request_body, scrub_pii_event, stabilize_retry_error_fingerprint, ) def _before_send(event: dict, hint: dict) -> dict | None: # type: ignore[type-arg] - """PII-scrub + RetryError fingerprint-стабилизация (glitchtip-noise). + """PR-D2: этот процесс не держит ASGI-приложения (нет `request` в event + сегодня), но payments_confirm/payments_reconcile (PR-E, тот же + `tradein-scraper` контейнер) будут звать Т-Банк API отсюда — belt-and- + suspenders на случай, если платёжные данные когда-нибудь попадут в + `request`/`extra`. Тот же обработчик на оба канала ниже — см. + app/main.py._before_send (идентичный мотив, не дублировать без причины). - Этот процесс гоняет `geocode_missing_listings` (ночной batch, сотни - адресов за прогон) — @retry-декорированные Nominatim-хелперы - (app/services/geocoder.py) на исчерпанных ретраях исторически плодили - по отдельному GlitchTip issue на КАЖДЫЙ адрес (RetryError.__str__() - тащит нестабильный repr() Future). См. sentry_scrub docstring. + PII-scrub + RetryError fingerprint-стабилизация (glitchtip-noise) идут + следом за платёжным body-wipe: этот процесс гоняет + `geocode_missing_listings` (ночной batch, сотни адресов за прогон) — + @retry-декорированные Nominatim-хелперы (app/services/geocoder.py) на + исчерпанных ретраях исторически плодили по отдельному GlitchTip issue + на КАЖДЫЙ адрес (RetryError.__str__() тащит нестабильный repr() Future). + См. sentry_scrub docstring. """ - scrubbed = scrub_pii_event(event, hint) + scrubbed = scrub_payment_request_body(event, hint) # type: ignore[arg-type] + if scrubbed is None: + return None + scrubbed = scrub_pii_event(scrubbed, hint) if scrubbed is None: return None return stabilize_retry_error_fingerprint(scrubbed, hint) @@ -70,6 +81,7 @@ if settings.glitchtip_dsn: traces_sample_rate=0.0, send_default_pii=False, before_send=_before_send, + before_send_transaction=_before_send, integrations=[ SqlalchemyIntegration(), HttpxIntegration(), diff --git a/tradein-mvp/backend/app/tgbot_main.py b/tradein-mvp/backend/app/tgbot_main.py index 5f731a86..48eaf32e 100644 --- a/tradein-mvp/backend/app/tgbot_main.py +++ b/tradein-mvp/backend/app/tgbot_main.py @@ -58,12 +58,17 @@ if settings.glitchtip_dsn: from sentry_sdk.integrations.httpx import HttpxIntegration from sentry_sdk.integrations.logging import LoggingIntegration - from app.observability.sentry_scrub import redact_telegram_bot_token, scrub_pii_event + from app.observability.sentry_scrub import ( + redact_telegram_bot_token, + scrub_payment_request_body, + scrub_pii_event, + ) def _before_send(event: Any, hint: dict[str, Any]) -> Any: - """Композиция PII-scrub (form-данные) + Telegram bot-токен redaction - (#tgsupport review). Токен утекает ДВУМЯ независимыми векторами, которые - `include_local_variables=False` ниже и этот хук закрывают вместе: + """Композиция платёжный body-wipe (PR-D2) + PII-scrub (form-данные) + + Telegram bot-токен redaction (#tgsupport review). Токен утекает ДВУМЯ + независимыми векторами, которые `include_local_variables=False` ниже и + этот хук закрывают вместе: 1. `include_local_variables=True` (sentry_sdk default) кладёт stack-frame locals (`self._base`/`url` в `TelegramClient._request`) в traceback — закрыто через `include_local_variables=False` в `sentry_sdk.init`. @@ -72,8 +77,16 @@ if settings.glitchtip_dsn: перестанет спасать, если трейсинг когда-нибудь включат. Regex-редактор — belt-and-suspenders на случай #1 (если include_local_variables случайно вернут) И на span data. + + Платёжный body-wipe — belt-and-suspenders: этот процесс не держит ASGI- + приложения (нет `request` в event сегодня), но тот же обработчик передан + ОБОИМ каналам ниже (before_send/before_send_transaction) ради единообразия + со всеми точками инициализации sentry_sdk в проекте (см. app/main.py). """ - scrubbed = scrub_pii_event(event, hint) + scrubbed = scrub_payment_request_body(event, hint) + if scrubbed is None: + return None + scrubbed = scrub_pii_event(scrubbed, hint) if scrubbed is None: return None return redact_telegram_bot_token(scrubbed, hint) @@ -86,6 +99,7 @@ if settings.glitchtip_dsn: send_default_pii=False, include_local_variables=False, before_send=_before_send, + before_send_transaction=_before_send, integrations=[ HttpxIntegration(), LoggingIntegration(level=logging.INFO, event_level=logging.ERROR), diff --git a/tradein-mvp/backend/data/sql/_manifest_applied.txt b/tradein-mvp/backend/data/sql/_manifest_applied.txt deleted file mode 100644 index 276864a0..00000000 --- a/tradein-mvp/backend/data/sql/_manifest_applied.txt +++ /dev/null @@ -1,257 +0,0 @@ -# _manifest_applied.txt — CONTRACT (issue #2216) -# -# Отсортированный список ВСЕХ имён миграций (bare filename) в data/sql/, -# которые на момент коммита уже применены/забейслайнены на проде. -# Прод трекает миграции по bare-filename в public._schema_migrations — -# переименование или удаление применённого файла => повторный прогон на -# проде (новый filename считается неприменённым) => дубль-эффекты/ошибки. -# -# ПРАВИЛА (enforced tests/test_migrations_manifest.py): -# 1. Каждое имя здесь ОБЯЗАНО существовать в data/sql/ (нельзя rename/rm applied). -# 2. Новый .sql-файл => НЕ переиспользуй NN-префикс (кроме 6 grandfathered дублей). -# 3. Добавляя новую миграцию — допиши её имя сюда В ТОМ ЖЕ PR (список отсортирован). -# -# Комментарии (# ...) и пустые строки тест игнорирует. -001_trade_in_estimates.sql -002_core_tables.sql -003_seed_deals.sql -004_extend_trade_in_estimates.sql -005_geocode_tracking.sql -007_estimate_photos.sql -008_crm_fields.sql -009_houses.sql -010_houses_alter.sql -011_listings_alter.sql -012_sellers.sql -013_listings_alter_seller.sql -014_house_reviews.sql -015_scrape_runs.sql -016_listings_snapshots.sql -017_house_placement_history.sql -018_avito_imv_evaluations.sql -019_listings_alter_cian.sql -020_houses_alter_cian.sql -021_management_companies.sql -022_agents_table.sql -023_offer_price_history.sql -024_houses_price_dynamics.sql -025_house_reliability_checks.sql -026_external_valuations.sql -027_cian_session_cookies.sql -028_matching_tables.sql -029_extend_matching_valuation_dynamics.sql -030_avito_imv_cache_key_unique.sql -031_houses_alter_yandex.sql -032_yandex_history.sql -033_listings_alter_yandex.sql -034_trade_in_estimates_geom.sql -035_drop_duplicate_indexes.sql -040_houses_extend.sql -041_house_sources_noop.sql -042_listing_sources_price_divergence_idx.sql -043_house_reviews_extend.sql -044_external_valuations_link.sql -045_house_placement_history_extend.sql -046_views.sql -047_cian_history_sanitize.sql -050_search_optimization.sql -051_scrape_runs_extend.sql -052_scrape_schedules.sql -053_scraper_settings.sql -054_scraper_settings_global.sql -060_postgres_fdw_extension.sql -061_drop_legacy_cad_buildings.sql -062_clean_avito_addresses.sql -063_backfill_houses_and_link_listings.sql -064_house_imv_phase_c.sql -065_trade_in_estimates_floor_optional.sql -066_address_mismatch_audit.sql -067_v_street_sales_vs_listings.sql -068_drop_v_street_sales_vs_listings.sql -069_trade_in_estimates_dadata_fields.sql -070_houses_dadata_enrichment.sql -071_houses_cian_zhk_url.sql -072_scrape_schedules_seed_cian_rosreestr.sql -073_normalize_repair_state.sql -075_backfill_repair_state_from_description.sql -076_account_estimate_quota.sql -077_dedup_hash_plain_key_backfill.sql -078_scrape_schedules_seed_yandex_sweep.sql -079_listing_source_history.sql -080_asking_to_sold_ratios.sql -081_trade_in_estimates_expected_sold.sql -082_scrape_schedules_seed_ratio_refresh.sql -083_trade_in_estimates_created_by.sql -084_brand_praktika_fill.sql -084_scrape_schedules_seed_n1_sweep.sql -085_quarter_price_index_fdw.sql -086_deals_address_trgm_index.sql -087_fdw_server_options.sql -088_scrape_schedules_seed_search_matview_refresh.sql -089_listings_geo_precision.sql -090_scrape_schedules_seed_deactivate_stale_avito.sql -091_scrape_schedules_seed_yandex_address_backfill.sql -092_sber_price_index.sql -093_scrape_schedules_seed_sber_index_pull.sql -094_cadastral_unify.sql -095_dead_schema.sql -096_scrape_schedules_seed_rosreestr_quarter_poll.sql -097_index_hygiene.sql -098_asking_to_sold_ratios_tiered.sql -099_brand_praktika_logo_wordmark.sql -100_enable_deactivate_stale_avito.sql -101_gendesign_reader_role.sql -102_grant_listings_gendesign_reader.sql -103_scrape_schedules_seed_newbuilding_enrich.sql -104_index_hygiene_geom_dedup.sql -105_market_schema_yandex_enrichment.sql -106_scrape_schedules_seed_yandex_newbuilding_sweep.sql -107_scrape_schedules_seed_cian_city_sweep.sql -108_clean_avito_addresses_v2.sql -108_merge_duplicate_houses.sql -109_asking_to_sold_ratio_segment_filter.sql -110_scrape_schedules_seed_geocode_missing_listings.sql -111_listings_avito_detail_fields.sql -112_scrape_schedules_seed_avito_detail_backfill.sql -113_deactivate_ghost_duplicate_listings.sql -113_yandex_detail_backfill.sql -114_disable_n1_sweep.sql -115_scrape_schedules_seed_deactivate_stale_yandex_cian.sql -116_offer_price_history_change_trigger.sql -117_listings_last_seen_deactivate_index.sql -118_enable_cian_city_sweep.sql -119_yandex_city_sweep_center_combos.sql -120_restore_partial_active_indexes.sql -121_remove_brand_praktika.sql -121_yandex_rich_fields.sql -122_enable_domclick_city_sweep.sql -123_avito_newbuilding_sweep_schedule.sql -124_cad_buildings_local.sql -124_deglue_avito_addresses.sql -125_scrape_schedules_seed_cadastral_geo_match.sql -126_scrape_schedules_seed_cian_full_load.sql -127_scrape_schedules_seed_avito_full_load.sql -128_listings_card_hash.sql -129_avito_full_load_incremental_split.sql -130_backfill_listings_house_id_fk.sql -130_ekb_geoportal_buildings.sql -131_fix_diff_percent_overflow.sql -132_scrape_schedules_seed_house_imv.sql -133_listings_uq_source_source_id.sql -134_listings_geom_geography_gist.sql -135_scrape_schedules_seed_house_dedup_merge.sql -136_backfill_listings_house_id_fk_source_identity.sql -137_listings_addr_norm_trgm.sql -138_domclick_bff_rewrite_schedule.sql -139_premium_houses.sql -140_yandex_house_type_backfill.sql -141_cian_promote_house_type.sql -142_premium_buildings_curated.sql -143_building_sale_share_schema.sql -144_gar_canon_addr_match.sql -145_building_sale_share_plausible_denom.sql -146_sale_share_45d_and_zhkh_denom.sql -147_canon_strip_geo_prefixes.sql -148_dedup_apartments_in_sale_share.sql -149_zhkh_priority_denominator.sql -150_sale_share_listing_geo_filter.sql -151_clean_bare_street_aliases.sql -152_sale_share_floors_guard.sql -153_sale_share_listings_floors_plausibility.sql -154_market_contract_views.sql -155_reader_grants_to_contract_views.sql -156_revoke_raw_from_reader.sql -157_scrape_proxies.sql -158_seed_proxy_healthcheck_schedule.sql -159_houses_fias_idx.sql -160_seed_deactivate_stale_domklik_n1.sql -161_backfill_scraped_at_active_recent.sql -162_seed_deals_freshness_monitor.sql -163_disable_deactivate_stale_domklik.sql -164_yandex_url_canonicalize_active_dups.sql -165_remove_n1_source.sql -166_purge_listings_phones.sql -167_drop_client_pii.sql -168_fdw_osm_poi_ekb.sql -169_osm_poi_ekb_local.sql -170_scrape_schedules_seed_osm_poi_ekb_refresh.sql -171_scrape_schedules_seed_geoportal_coords_backfill.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 -189_account_estimate_usage_nonnegative.sql -190_sale_share_price_bucket_signature.sql -191_account_quota_unlimited_flag.sql -192_tradein_users_auth.sql -193_tradein_users_seed.sql -194_deal_city_price_bands_tiers.sql -195_scrape_schedules_seed_deal_city_price_bands_refresh.sql -196_listings_city.sql -197_backfill_listings_city_from_url.sql -198_scrape_proxy_rotations.sql -199_scrape_proxies_asocks_rotate_url.sql -200_region_code_foreign_cities.sql -201_purge_dead_mobileproxy_proxies.sql -202_listing_source_snapshot_budget_sec.sql -203_purge_geocode_cache_house_letter.sql -204_cian_oblast_sweeps_secondary.sql -205_sales_vs_listings_city_filter.sql -206_scrape_schedules_cut_wasteful_load.sql -207_backfill_yandex_cian_city_geo_cleanup.sql -208_reenable_domclick_detail_backfill.sql -209_scrape_proxies_disabled_reason.sql -210_scrape_proxy_source_bans.sql -211_sales_vs_listings_segment_guard.sql -212_sber_index_pull_weekly.sql -213_listings_snapshots_status_vocab.sql -214_drop_dead_run_metrics.sql -215_avito_full_load_window_matches_cadence.sql -216_dead_code_sweep.sql -# -# 2026-08-06: список догнан до факта прода. Проверка перед правкой — -# _schema_migrations на tradein-postgres: 209 применённых имён, здесь было -# 178; расхождение — 31 имя, все в одну сторону (применено, но не заморожено). -# Обратного расхождения нет: ни одной строки, которой не было бы на проде. -# -# Тем самым снято отложенное условие из прошлой редакции: 187/188 (веб-чат -# поддержки, #2532/#2533) откладывались до подтверждения, что они осели на -# проде в финальном виде. Они в _schema_migrations — условие выполнено. -# -# 217-232 сюда намеренно не дописаны этой миграцией (222/225): в момент -# правки они уже слиты в main и применены на проде (см. _schema_migrations), -# но их авторы не дописали имена в тот же PR — это чужой пробел, не наш; -# self-maintenance-контракт (см. докстринг test_migrations_manifest.py) -# требует дописывать только СВОЙ файл в СВОЁМ PR, что и сделано ниже для -# 222/225 по прецеденту 233_payments.sql. -222_db_audit_cleanup.sql -225_listing_source_snapshots_run_id_idx.sql -233_payments.sql -234_scrape_runs_ban_kind_unknown.sql -240_trade_in_estimates_retain_until.sql -250_drop_duplicate_expires_at_index.sql -251_listings_drop_ceiling_height.sql -254_listings_backfill_avito_rating_glued_address.sql -257_listings_backfill_yandex_source_url.sql -258_houses_imv_transient_attempts.sql -259_data_quality_drop_pct_cadastr.sql -260_houses_drop_has_panorama.sql -261_listings_search_mv_drop_placeholder_columns.sql -262_scrape_schedules_seed_oblast_city_sweeps_wave2.sql -263_scrape_schedules_wave2_cian_newbuilding_only_false.sql -264_deactivate_stale_avito_cap_mult.sql -265_deactivate_stale_yandex_cap_mult.sql -266_seed_deactivate_stale_null_segment_yandex_cian.sql diff --git a/tradein-mvp/backend/tests/skip_allowlist.txt b/tradein-mvp/backend/tests/skip_allowlist.txt index d97627f7..ee3c6e89 100644 --- a/tradein-mvp/backend/tests/skip_allowlist.txt +++ b/tradein-mvp/backend/tests/skip_allowlist.txt @@ -60,6 +60,16 @@ tests/test_purge_expired_trade_in_data.py::test_real_purge_not_wedged_by_healthy # на мок-лэйне (deploy-tradein.yml, DSN-заглушка) констрейнта нет вовсе. tests/test_2764_ban_kind_no_default.py::test_real_default_ban_kind_survives_the_check_constraint +# Гейт номеров миграций (#2683) сверяется с origin/main и точкой ветвления. Где +# git-эталона нет — прогон внутри prod-образа, экспорт исходников без .git — +# проверять не с чем, и тест это ГОВОРИТ вслух вместо тихого зелёного. +# В CI пропуска не бывает: при CI/GITHUB_ACTIONS та же ветка делает pytest.fail +# (отсутствие эталона в пайплайне — сломанный гейт, а не «нечего проверять»), +# а ci-tradein.yml/deploy-tradein.yml берут checkout с fetch-depth: 0 — при нём +# checkout сам приносит refs/remotes/origin/*, отдельный git fetch не нужен и +# из job-контейнера всё равно не проходит (run 6977, connection refused). +tests/test_migration_numbering.py::test_applied_migration_is_not_renamed_or_deleted +tests/test_migration_numbering.py::test_new_migration_takes_a_free_number # Повтор застрявших transient_error (#2674, PR #2843) — тот же `_live_session()`. # Проверяют ВЫБОРКУ очереди на живой схеме (кто попал в пакет прогона), а не текст # SQL: на мок-лэйне deploy-tradein.yml БД нет вовсе. В ci-tradein.yml они бегут diff --git a/tradein-mvp/backend/tests/test_estimator_pure_units.py b/tradein-mvp/backend/tests/test_estimator_pure_units.py index 1170e4c2..b2f64d2c 100644 --- a/tradein-mvp/backend/tests/test_estimator_pure_units.py +++ b/tradein-mvp/backend/tests/test_estimator_pure_units.py @@ -378,7 +378,7 @@ def test_corridor_clamp_above_corridor_tier_c_clamps() -> None: def test_corridor_clamp_inside_corridor_is_noop() -> None: # Эконом/комфорт: headline в коридоре (с учётом slack) → ничего не меняется. - new_ppm2, new_price, new_low, new_high, clamped = _clamp( + new_ppm2, _, _, _, clamped = _clamp( median_ppm2=140_000, corridor_high=130_000, count=20, tier="C" ) assert clamped is False diff --git a/tradein-mvp/backend/tests/test_glitchtip_webhook.py b/tradein-mvp/backend/tests/test_glitchtip_webhook.py new file mode 100644 index 00000000..2a777597 --- /dev/null +++ b/tradein-mvp/backend/tests/test_glitchtip_webhook.py @@ -0,0 +1,291 @@ +"""Offline-тесты приёмника GlitchTip webhook-алертов +(POST /api/v1/trade-in/ops/glitchtip-webhook) — app/api/v1/glitchtip.py. + +Проверяет HTTP-контракт: успешная пересылка issue-/uptime-алертов в +Telegram (клиент замокан), отказ без валидного секрета, отказ при +несконфигурированных настройках, обрезка длинного текста под лимит Telegram +(4096), graceful-обработка неизвестной формы payload (НЕ 500). + +NEVER touches real DB / real Telegram API. +""" + +from __future__ import annotations + +import os + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +import json +from typing import Any, ClassVar + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from app.api.v1 import glitchtip as glitchtip_module +from app.services.tgbot.client import TelegramApiError + +_SECRET = "test-shared-secret" +_ENDPOINT = "/api/v1/trade-in/ops/glitchtip-webhook" + + +@pytest.fixture(autouse=True) +def _configured(monkeypatch: pytest.MonkeyPatch) -> None: + """По умолчанию вебхук полностью сконфигурирован — отдельные тесты + переопределяют конкретные поля.""" + monkeypatch.setattr(glitchtip_module.settings, "tradein_internal_auth_secret", _SECRET) + monkeypatch.setattr(glitchtip_module.settings, "telegram_bot_token", "fake-token") + monkeypatch.setattr(glitchtip_module.settings, "telegram_alerts_chat_id", -1004443088679) + monkeypatch.setattr(glitchtip_module.settings, "telegram_alerts_topic_id", 158) + + +class _FakeTelegramClient: + """Подменяет `TelegramClient` внутри модуля `glitchtip` — никакого httpx/сети.""" + + calls: ClassVar[list[dict[str, Any]]] = [] + _response: ClassVar[dict[str, Any] | Exception] = {"message_id": 1} + + def __init__(self, _token: str) -> None: + pass + + async def send_message(self, **kwargs: Any) -> dict[str, Any]: + _FakeTelegramClient.calls.append(kwargs) + if isinstance(_FakeTelegramClient._response, Exception): + raise _FakeTelegramClient._response + return _FakeTelegramClient._response + + +@pytest.fixture(autouse=True) +def _fake_telegram_client(monkeypatch: pytest.MonkeyPatch) -> Any: + _FakeTelegramClient.calls = [] + _FakeTelegramClient._response = {"message_id": 1} + monkeypatch.setattr(glitchtip_module, "TelegramClient", _FakeTelegramClient) + return _FakeTelegramClient + + +@pytest.fixture +def client() -> TestClient: + app = FastAPI() + app.include_router(glitchtip_module.router, prefix="/api/v1/trade-in") + return TestClient(app) + + +_ISSUE_PAYLOAD = { + "text": "GlitchTip Alert", + "attachments": [ + { + "title": "ValueError: something broke", + "title_link": "https://errors.gendsgn.ru/organizations/gendesign/issues/123/", + "text": "app/services/foo.py in bar", + "color": "#e03131", + "fields": [ + {"title": "Project", "value": "tradein-backend", "short": True}, + {"title": "Environment", "value": "production", "short": True}, + ], + "mrkdown_in": ["text"], + } + ], +} + +_UPTIME_PAYLOAD = { + "text": "GlitchTip Uptime Alert", + "attachments": [ + { + "title": "gendsgn.ru", + "title_link": "https://errors.gendsgn.ru/organizations/gendesign/uptime/1/", + "text": "The monitored site has gone down.", + "image_url": None, + "color": None, + "fields": None, + "mrkdown_in": None, + } + ], +} + + +# ── успешная пересылка ────────────────────────────────────────────────────── + + +def test_issue_alert_forwarded_to_telegram(client: TestClient, _fake_telegram_client: Any) -> None: + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json=_ISSUE_PAYLOAD) + + assert r.status_code == 200, r.text + assert r.json() == {"status": "ok"} + assert len(_fake_telegram_client.calls) == 1 + call = _fake_telegram_client.calls[0] + assert call["chat_id"] == -1004443088679 + assert call["message_thread_id"] == 158 + assert "ValueError: something broke" in call["text"] + assert "tradein-backend" in call["text"] # Project field + assert "errors.gendsgn.ru" in call["text"] + + +def test_uptime_alert_forwarded_to_telegram(client: TestClient, _fake_telegram_client: Any) -> None: + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json=_UPTIME_PAYLOAD) + + assert r.status_code == 200, r.text + assert len(_fake_telegram_client.calls) == 1 + call = _fake_telegram_client.calls[0] + assert "GlitchTip Uptime Alert" in call["text"] + assert "gone down" in call["text"] + assert call["message_thread_id"] == 158 + + +def test_telegram_failure_returns_502_not_500( + client: TestClient, _fake_telegram_client: Any +) -> None: + _FakeTelegramClient._response = TelegramApiError("sendMessage", 400, "chat not found") + + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json=_ISSUE_PAYLOAD) + + assert r.status_code == 502 + assert r.status_code != 500 + + +# ── auth ───────────────────────────────────────────────────────────────────── + + +def test_missing_secret_401(client: TestClient, _fake_telegram_client: Any) -> None: + r = client.post(_ENDPOINT, json=_ISSUE_PAYLOAD) + + assert r.status_code == 401 + assert _fake_telegram_client.calls == [] + + +def test_wrong_secret_401(client: TestClient, _fake_telegram_client: Any) -> None: + r = client.post(f"{_ENDPOINT}?secret=wrong-value", json=_ISSUE_PAYLOAD) + + assert r.status_code == 401 + assert _fake_telegram_client.calls == [] + + +def test_secret_not_configured_returns_503_not_500( + client: TestClient, monkeypatch: pytest.MonkeyPatch, _fake_telegram_client: Any +) -> None: + monkeypatch.setattr(glitchtip_module.settings, "tradein_internal_auth_secret", "") + + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json=_ISSUE_PAYLOAD) + + assert r.status_code == 503 + assert r.status_code != 500 + assert _fake_telegram_client.calls == [] + + +def test_bot_not_configured_returns_503( + client: TestClient, monkeypatch: pytest.MonkeyPatch, _fake_telegram_client: Any +) -> None: + monkeypatch.setattr(glitchtip_module.settings, "telegram_bot_token", "") + + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json=_ISSUE_PAYLOAD) + + assert r.status_code == 503 + assert _fake_telegram_client.calls == [] + + +def test_alerts_chat_id_not_configured_returns_503( + client: TestClient, monkeypatch: pytest.MonkeyPatch, _fake_telegram_client: Any +) -> None: + monkeypatch.setattr(glitchtip_module.settings, "telegram_alerts_chat_id", 0) + + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json=_ISSUE_PAYLOAD) + + assert r.status_code == 503 + assert _fake_telegram_client.calls == [] + + +# ── обрезка длинного текста ───────────────────────────────────────────────── + + +def test_long_payload_truncated_to_telegram_limit( + client: TestClient, _fake_telegram_client: Any +) -> None: + huge_payload = { + "text": "GlitchTip Alert", + "attachments": [ + { + "title": "Huge issue", + "title_link": "https://errors.gendsgn.ru/x", + "text": "x" * 10000, + } + ], + } + + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json=huge_payload) + + assert r.status_code == 200, r.text + sent_text = _fake_telegram_client.calls[0]["text"] + assert len(sent_text) <= 4096 + assert sent_text.endswith("(обрезано)") + + +def test_unknown_form_huge_raw_body_truncated( + client: TestClient, _fake_telegram_client: Any +) -> None: + r = client.post( + f"{_ENDPOINT}?secret={_SECRET}", + content=("x" * 10000).encode(), + headers={"content-type": "application/json"}, + ) + + assert r.status_code == 200, r.text + sent_text = _fake_telegram_client.calls[0]["text"] + assert len(sent_text) <= 4096 + + +# ── неизвестная форма payload — НЕ 500 ────────────────────────────────────── + + +def test_unknown_json_shape_forwarded_with_marker( + client: TestClient, _fake_telegram_client: Any +) -> None: + """Ни `text`, ни `attachments` — форма, которую GlitchTip НЕ шлёт сегодня, + но контракт задачи требует не падать 500, а переслать как есть.""" + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json={"some_field": "some_value", "n": 42}) + + assert r.status_code == 200, r.text + sent_text = _fake_telegram_client.calls[0]["text"] + assert "неизвестный формат" in sent_text + assert "some_value" in sent_text + + +def test_non_json_body_forwarded_not_500(client: TestClient, _fake_telegram_client: Any) -> None: + r = client.post( + f"{_ENDPOINT}?secret={_SECRET}", + content=b"not-json-at-all {{{", + headers={"content-type": "text/plain"}, + ) + + assert r.status_code == 200, r.text + sent_text = _fake_telegram_client.calls[0]["text"] + assert "неизвестный формат" in sent_text + assert "not-json-at-all" in sent_text + + +def test_json_array_body_forwarded_not_500(client: TestClient, _fake_telegram_client: Any) -> None: + """Валидный JSON, но не объект (top-level list) — тоже неизвестная форма.""" + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json=[1, 2, 3]) + + assert r.status_code == 200, r.text + assert len(_fake_telegram_client.calls) == 1 + + +def test_empty_body_forwarded_not_500(client: TestClient, _fake_telegram_client: Any) -> None: + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", content=b"") + + assert r.status_code == 200, r.text + assert len(_fake_telegram_client.calls) == 1 + + +# ── формат сообщения ───────────────────────────────────────────────────────── + + +def test_message_format_json_roundtrip(client: TestClient, _fake_telegram_client: Any) -> None: + """Sanity: убеждаемся, что тестовый payload реально валиден как JSON (не + полагаемся на literal dict без проверки сериализации).""" + body = json.dumps(_ISSUE_PAYLOAD) + r = client.post( + f"{_ENDPOINT}?secret={_SECRET}", + content=body.encode(), + headers={"content-type": "application/json"}, + ) + assert r.status_code == 200, r.text diff --git a/tradein-mvp/backend/tests/test_listing_segment_upsert_selfheal.py b/tradein-mvp/backend/tests/test_listing_segment_upsert_selfheal.py new file mode 100644 index 00000000..fcea6062 --- /dev/null +++ b/tradein-mvp/backend/tests/test_listing_segment_upsert_selfheal.py @@ -0,0 +1,215 @@ +"""listing_segment upsert self-heal: строка не должна вечно застревать с NULL-сегментом. + +Баг: `scraper_kit.base.save_listings` писал `listing_segment` ТОЛЬКО в INSERT-ветке +upsert'а — колонки не было ни в `ON CONFLICT (dedup_hash) DO UPDATE SET`, ни в +reconcile-UPDATE (dedup_hash-drift fallback, срабатывает при UniqueViolation по +(source, source_id)). Итог: если первый скрейп объявления не смог определить сегмент +(классификатор вернул None), строка рождалась с `listing_segment IS NULL` и +НИКОГДА не самочинялась на последующих пересборах, даже когда сегмент становился +определим. Симптом лечили отдельной джобой деактивации +(`deactivate_stale_{cian,yandex}_null_segment`, миграция 266, PR #2908) — чистит +мусор в `is_active`, но не лечит саму запись сегмента. + +Fix: `listing_segment = COALESCE(EXCLUDED.listing_segment, listings.listing_segment)` +в ON CONFLICT DO UPDATE + `listing_segment = COALESCE(:listing_segment, listing_segment)` +в reconcile UPDATE — тот же идиом, что уже применён для `city` (#2594, +test_listings_city_from_sweep.py) и `kitchen_area_m2`/`ceiling_height_m` (#2007): +новое значение обновляет строку, но НЕ затирает уже известное пустым. + +Тесты здесь, как и соседний test_listings_city_from_sweep.py, мокают db.execute и +проверяют SQL-текст + bind-параметры (unit-уровень, без реальной Postgres) — +COALESCE-семантику "новое непустое побеждает / пустое не затирает старое" исполняет +сама база при выполнении запроса. +""" + +from __future__ import annotations + +import os +from contextlib import contextmanager +from typing import Any +from unittest.mock import MagicMock, patch + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db") + +from scraper_kit.base import ScrapedLot as KitLot +from scraper_kit.base import save_listings as kit_save_listings + + +@contextmanager +def _nested_ctx() -> Any: + yield MagicMock() + + +def _mock_db_insert_path(listing_id: int = 42) -> MagicMock: + """Session mock для fresh INSERT path (xmax = 0 → inserted).""" + insert_row = MagicMock() + insert_row.id = listing_id + insert_row.inserted = True + + db = MagicMock() + + def _execute(sql: Any, params: dict[str, Any] | None = None) -> MagicMock: + s = str(sql) + res = MagicMock() + if "SELECT card_hash" in s and "WHERE dedup_hash" in s: + res.fetchone.return_value = None + elif "FROM listings_snapshots" in s: + res.fetchone.return_value = None + elif "INSERT INTO listings (" in s: + res.fetchone.return_value = insert_row + else: + res.fetchone.return_value = None + return res + + db.execute.side_effect = _execute + db.begin_nested.side_effect = _nested_ctx + return db + + +def _find_call(db: MagicMock, needle: str) -> tuple[str, dict[str, Any]]: + for call in db.execute.call_args_list: + sql = str(call.args[0]) + if needle in sql: + params = call.args[1] if len(call.args) > 1 else {} + return sql, params + raise AssertionError(f"SQL containing {needle!r} not found") + + +def _kit_matcher() -> MagicMock: + matcher = MagicMock() + matcher.match_or_create_house.return_value = (101, 1.0, "new") + matcher.upsert_listing_source.return_value = None + return matcher + + +def _lot( + source: str = "avito", + source_id: str = "1", + listing_segment: str | None = None, +) -> KitLot: + return KitLot( + source=source, + source_url=f"https://www.{source}.ru/item/{source_id}", + source_id=source_id, + address="ул. Победы, 30", + listing_segment=listing_segment, + price_rub=3_000_000, + ) + + +# ── save_listings(...) — INSERT path ──────────────────────────────────────── + + +def test_save_listings_writes_listing_segment_into_insert_sql() -> None: + """listing_segment передаётся в SQL params И колонка есть в INSERT-списке.""" + db = _mock_db_insert_path() + lot = _lot(listing_segment="vtorichka") + + with patch("scraper_kit.base.upsert_listing_snapshot", return_value=None): + kit_save_listings(db, [lot], matcher=_kit_matcher(), region_code=66) + + sql, params = _find_call(db, "INSERT INTO listings (") + assert "listing_segment" in sql, "listing_segment column must be in INSERT column list" + assert params["listing_segment"] == "vtorichka" + + +def test_save_listings_listing_segment_none_backward_compat() -> None: + """Классификатор не определил сегмент (None) — INSERT всё равно проходит, NULL.""" + db = _mock_db_insert_path() + lot = _lot(listing_segment=None) + + with patch("scraper_kit.base.upsert_listing_snapshot", return_value=None): + kit_save_listings(db, [lot], matcher=_kit_matcher(), region_code=66) + + _sql, params = _find_call(db, "INSERT INTO listings (") + assert params["listing_segment"] is None + + +# ── ON CONFLICT DO UPDATE — COALESCE self-heal (главный фикс) ────────────── + + +def test_save_listings_on_conflict_coalesces_listing_segment() -> None: + """ON CONFLICT DO UPDATE — listing_segment = COALESCE(EXCLUDED.listing_segment, + listings.listing_segment), не blind overwrite и не "никогда не обновляется".""" + db = _mock_db_insert_path() + lot = _lot(listing_segment="vtorichka") + + with patch("scraper_kit.base.upsert_listing_snapshot", return_value=None): + kit_save_listings(db, [lot], matcher=_kit_matcher(), region_code=66) + + sql, params = _find_call(db, "INSERT INTO listings (") + assert "listing_segment = COALESCE(" in sql + assert "EXCLUDED.listing_segment, listings.listing_segment" in sql + # Повторный скрейп с ОПРЕДЕЛЁННЫМ сегментом — новое значение уходит в EXCLUDED, + # COALESCE на стороне Postgres применит его к прежде-NULL строке (self-heal). + assert params["listing_segment"] == "vtorichka" + + +def test_save_listings_on_conflict_listing_segment_none_does_not_blind_overwrite() -> None: + """Повторный скрейп БЕЗ сегмента (classifier снова None) — SQL всё равно + несёт COALESCE (не голый EXCLUDED), значит уже известный сегмент строки + в БД НЕ будет затёрт пустым при выполнении запроса.""" + db = _mock_db_insert_path() + lot = _lot(listing_segment=None) + + with patch("scraper_kit.base.upsert_listing_snapshot", return_value=None): + kit_save_listings(db, [lot], matcher=_kit_matcher(), region_code=66) + + sql, params = _find_call(db, "INSERT INTO listings (") + assert "listing_segment = COALESCE(" in sql + assert "EXCLUDED.listing_segment, listings.listing_segment" in sql + assert params["listing_segment"] is None + + +# ── Reconcile UPDATE (dedup_hash drift) — тот же self-heal ───────────────── + + +def test_save_listings_reconcile_update_coalesces_listing_segment() -> None: + """dedup_hash-drift reconcile UPDATE path — тоже COALESCE(:listing_segment, + listing_segment), не blind overwrite. Без этого фикса строки, дошедшие до + reconcile (content дрейфит, старый dedup_hash не находится, INSERT ловит + UniqueViolation по (source, source_id)), остались бы незалеченными.""" + import psycopg.errors + from sqlalchemy.exc import IntegrityError + + uv_orig = psycopg.errors.UniqueViolation() + integrity_err = IntegrityError("INSERT INTO listings ...", {}, uv_orig) + rec_row = MagicMock() + rec_row.id = 88 + + db = MagicMock() + + def _execute(sql: Any, params: dict[str, Any] | None = None) -> MagicMock: + s = str(sql) + res = MagicMock() + if "SELECT card_hash" in s and "WHERE dedup_hash" in s: + res.fetchone.return_value = None + elif "FROM listings_snapshots" in s: + res.fetchone.return_value = None + elif "INSERT INTO listings (" in s: + raise integrity_err + elif "UPDATE listings" in s and "SET dedup_hash" in s: + res.fetchone.return_value = rec_row + else: + res.fetchone.return_value = None + return res + + db.execute.side_effect = _execute + + @contextmanager + def _nested() -> Any: + try: + yield MagicMock() + except IntegrityError: + raise + + db.begin_nested.side_effect = _nested + + lot = _lot(source="avito", source_id="7960764619", listing_segment="novostroyki") + + with patch("scraper_kit.base.upsert_listing_snapshot", return_value=None): + kit_save_listings(db, [lot], matcher=_kit_matcher(), region_code=66) + + sql, params = _find_call(db, "SET dedup_hash") + assert "listing_segment = COALESCE(:listing_segment, listing_segment)" in sql + assert params["listing_segment"] == "novostroyki" diff --git a/tradein-mvp/backend/tests/test_migration_numbering.py b/tradein-mvp/backend/tests/test_migration_numbering.py new file mode 100644 index 00000000..a97ebcc8 --- /dev/null +++ b/tradein-mvp/backend/tests/test_migration_numbering.py @@ -0,0 +1,215 @@ +"""Инварианты нумерации миграций trade-in (issues #2216, #2683). + +КОНТРАКТ. ОДНА ФОРМУЛИРОВКА, И ОНА ЗДЕСЬ — больше нигде её дублировать не надо. + + Эталон «что уже закреплено на проде» — origin/main, а не файл-список. + deploy-tradein.yml прогоняет КАЖДЫЙ data/sql/*.sql из main под ON_ERROR_STOP + (падение миграции => красный деплой) и трекает применённое по bare filename в + public._schema_migrations. То есть «файл доехал до main» == «имя закреплено на + проде», и вести это знание отдельно от git незачем: git и есть журнал. + + Отсюда ровно два инварианта, и ниже проверяются именно они. + + 1. Имя, существовавшее в точке ветвления, нельзя переименовать или удалить. + Прод помнит СТАРОЕ имя; новое считается неприменённым и прогоняется + повторно — дубль-INSERT / повторный DDL / PK violation под ON_ERROR_STOP, + то есть либо красный деплой, либо тихо задвоенные данные. Нужно изменить + уже применённую миграцию — заводи НОВЫЙ файл, старый оставь как есть. + + 2. НОВЫЙ файл обязан нести NN-префикс, свободный не только в рабочем дереве, + но и в origin/main. Порядок применения — `ls | sort`, два файла с одним NN + дают неопределённый порядок. Шесть исторических дублей (084/108/113/121/ + 124/130) не «новые» и не флагаются. + + ДОПИСЫВАТЬ НИЧЕГО НЕ НАДО. Автор миграции кладёт файл со свободным номером — и + всё. Списка, который можно забыть обновить, здесь больше нет: до #2683 таким + списком был data/sql/_manifest_applied.txt, и он по построению не мог + покраснеть — «файл, которого нет в списке» и «новый файл этого PR» были для + теста одним и тем же, поэтому забытое имя навсегда оставалось зелёным + (замер 2026-08-07: 15 забытых имён, сьют зелёный). + + ПОЧЕМУ ТОЧКА ВЕТВЛЕНИЯ, А НЕ САМ origin/main. Ветка, отведённая неделю назад, + не содержит миграций, смерженных после неё. Правило «origin/main ⊆ рабочее + дерево» красило бы каждую такую ветку без вины автора — и его отключили бы + через неделю. Сверка с merge-base ловит ровно то, что удалила или + переименовала ЭТА ветка, а номера при этом сверяются с ПОЛНЫМ origin/main, + чтобы коллизия с миграцией, смерженной после ветвления, всё-таки нашлась. + + КАК УБЕДИТЬСЯ, ЧТО СТОРОЖ УМЕЕТ КРАСНЕТЬ (не на слово): + test_collision_rule_flags_a_taken_number ниже проверяет само правило на + литеральных входах, а сквозной прогон воспроизводится так — + git worktree add --detach /tmp/wt + touch /tmp/wt/tradein-mvp/backend/data/sql/<занятый-NN>_probe.sql + (cd /tmp/wt/tradein-mvp/backend && pytest tests/test_migration_numbering.py) +""" + +from __future__ import annotations + +import os +import re +import subprocess +from collections.abc import Iterable +from pathlib import Path, PurePosixPath + +import pytest + +_TESTS_DIR = Path(__file__).resolve().parent +_SQL_DIR = _TESTS_DIR.parent / "data" / "sql" +_REPO_ROOT = _TESTS_DIR.parents[2] +# Путь каталога ОТ КОРНЯ РЕПО — им адресуем дерево коммита через git ls-tree. +_SQL_PATHSPEC = "tradein-mvp/backend/data/sql" + +_NN_PREFIX = re.compile(r"^(\d+)_") +# origin — штатный remote; forgejo/main оставлен как исторический алиас. +_MAIN_REFS = ("origin/main", "forgejo/main", "main") + + +def _git(*args: str) -> str | None: + """stdout git-команды, либо None если git недоступен/команда упала.""" + try: + done = subprocess.run( + # Фиксированный argv, без shell — args приходят только из этого модуля. + ["git", "-C", str(_REPO_ROOT), *args], + capture_output=True, + text=True, + timeout=30, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return None + return done.stdout if done.returncode == 0 else None + + +def _sql_names_at(rev: str) -> set[str]: + """Bare-имена *.sql в data/sql на ревизии rev.""" + out = _git("ls-tree", "-r", "-z", "--name-only", rev, "--", _SQL_PATHSPEC) or "" + return {PurePosixPath(p).name for p in out.split("\0") if p.endswith(".sql")} + + +def _sql_names_on_disk() -> set[str]: + """Bare-имена *.sql в рабочем дереве — включая ещё не закоммиченные.""" + return {p.name for p in _SQL_DIR.glob("*.sql")} + + +def _prefix(name: str) -> str | None: + m = _NN_PREFIX.match(name) + return m.group(1) if m else None + + +def _collisions(new_names: Iterable[str], universe: Iterable[str]) -> list[str]: + """Для каждого НОВОГО имени — чужие имена с тем же NN-префиксом.""" + by_prefix: dict[str, set[str]] = {} + for name in universe: + p = _prefix(name) + if p is not None: + by_prefix.setdefault(p, set()).add(name) + + found: list[str] = [] + for name in sorted(new_names): + p = _prefix(name) + if p is None: + continue + others = sorted(by_prefix.get(p, set()) - {name}) + if others: + found.append(f"{name} — номер {p} уже занят: {', '.join(others)}") + return found + + +def _baseline() -> tuple[set[str], set[str]]: + """(имена в точке ветвления, имена в main). Без git-эталона проверять нечего.""" + main_ref = next( + (r for r in _MAIN_REFS if _git("rev-parse", "--verify", "--quiet", f"{r}^{{commit}}")), + None, + ) + merge_base = None + if main_ref is not None: + out = _git("merge-base", main_ref, "HEAD") + merge_base = out.strip() if out else None + + if main_ref is None or merge_base is None: + why = ( + f"нет git-эталона миграций: ни один из {_MAIN_REFS} не резолвится либо у него " + f"нет общего предка с HEAD (repo={_REPO_ROOT}). Лечится " + "`git fetch --no-tags origin +refs/heads/main:refs/remotes/origin/main` " + "и НЕ shallow-клоном (нужен общий предок)." + ) + # В CI это не «нечего проверять», а сломанный гейт: пропуск здесь и есть + # тот зелёный, который ничего не проверяет. Поэтому красим. + if os.environ.get("CI") or os.environ.get("GITHUB_ACTIONS"): + pytest.fail(why) + pytest.skip(why) + + base_names = _sql_names_at(merge_base) + main_names = _sql_names_at(main_ref) + # Анти-вакуум: пустой эталон сделал бы обе проверки зелёными всегда. + # Ровно так ломается сторож, если _SQL_PATHSPEC разъедется с раскладкой репо. + assert base_names, ( + f"эталон пуст: git ls-tree {merge_base} -- {_SQL_PATHSPEC} не вернул ни одного " + "*.sql. Проверка номеров была бы вакуумно-зелёной — почини путь." + ) + assert main_names, f"в {main_ref} не найдено *.sql по пути {_SQL_PATHSPEC} — то же самое." + return base_names, main_names + + +def test_applied_migration_is_not_renamed_or_deleted() -> None: + """Файл, существовавший в точке ветвления, обязан существовать и сейчас. + + Red => эта ветка переименовала или удалила миграцию, которую прод уже + применил и помнит по СТАРОМУ имени. Верни исходное имя; нужно поправить + поведение — заводи новый файл с новым номером. + """ + on_disk = _sql_names_on_disk() + assert on_disk, f"не найдено *.sql в {_SQL_DIR}" + base_names, _ = _baseline() + + gone = sorted(base_names - on_disk) + assert not gone, ( + f"миграции пропали из {_SQL_PATHSPEC}/ (переименованы или удалены): {gone}. " + "Прод трекает их по bare-filename в _schema_migrations — под новым именем " + "миграция прогонится повторно. Верни имена как были." + ) + + +def test_new_migration_takes_a_free_number() -> None: + """Новый файл не переиспользует NN, занятый в origin/main или в этой ветке. + + Red => номер уже занят. Возьми следующий свободный, сверяясь с origin/main: + + git fetch origin main + git ls-tree -r --name-only origin/main -- tradein-mvp/backend/data/sql | tail + + `-r` обязателен: без него ls-tree печатает сам каталог, а не файлы (в этом + виде рецепт и ходил по issue #2683 — и молча возвращал одну строку). + Локального `ls` недостаточно: он не видит миграций, смерженных после + ветвления — ровно так разъехались 212 в #2682 и 234 в #2754. + """ + on_disk = _sql_names_on_disk() + base_names, main_names = _baseline() + + new_names = on_disk - base_names + problems = _collisions(new_names, main_names | on_disk) + assert not problems, "коллизия номеров миграций: " + "; ".join(problems) + + +def test_collision_rule_flags_a_taken_number() -> None: + """Проверка самого правила — сторож обязан уметь краснеть (#2683 п.6). + + Ожидания здесь — ЛИТЕРАЛЫ, а не производные от содержимого data/sql: тест, + который берёт ожидание из охраняемой настройки, зелен при любой настройке. + """ + # Реальный случай #2754: ветка отвелась до того, как в main приехал 234_scrape. + assert _collisions( + ["234_trade_in_estimates_retain_until.sql"], + { + "233_payments.sql", + "234_scrape_runs_ban_kind_unknown.sql", + "234_trade_in_estimates_retain_until.sql", + }, + ) == [ + "234_trade_in_estimates_retain_until.sql — номер 234 уже занят: " + "234_scrape_runs_ban_kind_unknown.sql" + ] + # Два новых файла с одним номером внутри одной ветки — оба названы. + assert len(_collisions(["300_a.sql", "300_b.sql"], {"300_a.sql", "300_b.sql"})) == 2 + # Свободный номер — тишина; исторические дубли не новые и не флагаются. + assert _collisions(["300_a.sql"], {"084_x.sql", "084_y.sql", "300_a.sql"}) == [] diff --git a/tradein-mvp/backend/tests/test_migrations_manifest.py b/tradein-mvp/backend/tests/test_migrations_manifest.py deleted file mode 100644 index 26e3375f..00000000 --- a/tradein-mvp/backend/tests/test_migrations_manifest.py +++ /dev/null @@ -1,138 +0,0 @@ -"""Invariants over data/sql migrations (issue #2216). - -Прод применяет миграции по BARE FILENAME: deploy-tradein.yml трекает каждый -`data/sql/*.sql` в таблице `public._schema_migrations` (PRIMARY KEY = filename). -Из этого следуют два хрупких инварианта, которые этот тест защищает от регрессии: - -1. Переименование / удаление УЖЕ ПРИМЕНЁННОЙ миграции ломает прод: новый - filename считается неприменённым и прогоняется повторно (дубль-INSERT, - повторный DDL, PK violation под ON_ERROR_STOP => деплой падает или, хуже, - молча дублирует данные). Manifest `data/sql/_manifest_applied.txt` — это - слепок применённых имён на момент коммита; любой из них ОБЯЗАН существовать. - -2. Два разных файла с одинаковым NN-префиксом ("дубль-префикс") — источник - двусмысленного порядка применения (`ls | sort` детерминирован, но человек - легко создаёт коллизию). 6 исторических дублей grandfathered'ы (оба в - manifest). Любой НОВЫЙ файл обязан нести уникальный префикс. - -Self-maintenance: добавляя новую миграцию, допиши её имя в manifest В ТОМ ЖЕ PR -(см. assert-сообщения ниже). Тест требует data/sql ⊇ manifest и уникальность -префикса у новых файлов; сам manifest дополняет автор миграции. -""" - -from __future__ import annotations - -import re -from pathlib import Path - -_BACKEND_ROOT = Path(__file__).resolve().parents[1] -_SQL_DIR = _BACKEND_ROOT / "data" / "sql" -_MANIFEST = _SQL_DIR / "_manifest_applied.txt" - -_NN_PREFIX = re.compile(r"^(\d+)_") - - -def _read_manifest() -> list[str]: - """Имена миграций из manifest; # comments и пустые строки игнорируются.""" - names: list[str] = [] - for raw in _MANIFEST.read_text(encoding="utf-8").splitlines(): - line = raw.strip() - if not line or line.startswith("#"): - continue - names.append(line) - return names - - -def _actual_sql_files() -> set[str]: - return {p.name for p in _SQL_DIR.glob("*.sql")} - - -def _prefix(name: str) -> str | None: - m = _NN_PREFIX.match(name) - return m.group(1) if m else None - - -def test_manifest_entries_all_exist() -> None: - """Каждый файл из manifest СУЩЕСТВУЕТ в data/sql/. - - Red => применённая миграция переименована или удалена. Прод трекает по - bare-filename: старое имя остаётся в _schema_migrations, НОВОЕ имя считается - неприменённым и прогоняется повторно. Восстанови исходное имя файла (или, - если переименование намеренное — так делать НЕЛЬЗЯ для уже-применённых - миграций: заведи НОВЫЙ файл, а старый оставь как есть). - """ - actual = _actual_sql_files() - manifest = _read_manifest() - missing = sorted(n for n in manifest if n not in actual) - assert not missing, ( - "Миграции из _manifest_applied.txt отсутствуют в data/sql/ " - f"(переименованы/удалены?): {missing}. Эти имена уже применены на проде " - "(tracking по bare-filename в _schema_migrations) — их нельзя " - "переименовывать/удалять. Верни исходные имена файлов." - ) - - -def test_manifest_is_sorted_and_unique() -> None: - """Manifest отсортирован (codepoint) и без дублей — детерминированный слепок.""" - manifest = _read_manifest() - assert manifest == sorted(manifest), ( - "_manifest_applied.txt не отсортирован. Пересортируй записи " - "(LC_ALL=C sort / Python sorted())." - ) - dupes = sorted({n for n in manifest if manifest.count(n) > 1}) - assert not dupes, f"Дублирующиеся строки в _manifest_applied.txt: {dupes}" - - -def test_new_files_do_not_reuse_prefix() -> None: - """Новые (не в manifest) .sql-файлы НЕ переиспользуют существующий NN-префикс. - - Grandfathered дубли (084/108/113/121/124/130) — оба файла в manifest, поэтому - не флагаются: считаются "существующими", а не "новыми". - - Red => новый файл взял префикс уже присутствующей миграции. Присвой - следующий свободный NN и допиши имя в _manifest_applied.txt (тот же PR). - """ - actual = _actual_sql_files() - manifest = set(_read_manifest()) - - # Префиксы, «занятые» уже-применёнными (manifest) миграциями. - baseline_prefixes: set[str] = set() - for name in manifest: - p = _prefix(name) - if p is not None: - baseline_prefixes.add(p) - - new_files = sorted(actual - manifest) - collisions: list[str] = [] - # Внутри новых файлов префикс тоже обязан быть уникален (два новых с одним NN). - seen_new_prefix: dict[str, str] = {} - for name in new_files: - p = _prefix(name) - if p is None: - continue - if p in baseline_prefixes: - collisions.append(f"{name} (префикс {p} занят применённой миграцией)") - elif p in seen_new_prefix: - collisions.append(f"{name} (префикс {p} уже у нового {seen_new_prefix[p]})") - else: - seen_new_prefix[p] = name - - assert not collisions, ( - "Новые миграции переиспользуют NN-префикс: " + "; ".join(collisions) + ". " - "Присвой следующий свободный номер и добавь имя файла в " - "_manifest_applied.txt в ЭТОМ ЖЕ PR." - ) - - -def test_manifest_covers_all_but_new_files() -> None: - """data/sql ⊇ manifest, и каждый новый файл имеет уникальный префикс — - напоминание о self-maintenance: manifest дополняется вместе с миграцией. - - Этот тест НЕ требует, чтобы новый файл уже был в manifest (иначе PR с новой - миграцией всегда красный). Он лишь гарантирует, что manifest не отстал от - реальности В ЧАСТИ применённых имён (см. test_manifest_entries_all_exist) и - что новые файлы не создают префикс-коллизий (см. предыдущий тест). - """ - # Sanity: manifest непустой и в data/sql есть файлы — защита от битых путей. - assert _actual_sql_files(), f"Не найдено *.sql в {_SQL_DIR}" - assert _read_manifest(), f"_manifest_applied.txt пуст: {_MANIFEST}" diff --git a/tradein-mvp/backend/tests/test_ratelimit.py b/tradein-mvp/backend/tests/test_ratelimit.py index 4d1cd8dc..b9b71c1a 100644 --- a/tradein-mvp/backend/tests/test_ratelimit.py +++ b/tradein-mvp/backend/tests/test_ratelimit.py @@ -149,6 +149,98 @@ def test_sliding_window_limiter_per_key_isolation(): assert limiter.retry_after("bob") is None # свой ключ — не задет alice +# ── Платёжная нотификация — мимо ОБЩЕГО лимитера (PR-D2, критерий приёмки #3) ── + + +@pytest.fixture +def notify_client(monkeypatch): + """То же минимальное приложение, что `client`, но лимит анонима искусственно + крошечный (1/60с) — если бы notify-путь шёл через общий лимитер, 2-й запрос + уже получил бы 429. Плюс контрольный `/api/v1/ping` — доказывает, что + лимитер в принципе активен (не выключен целиком), просто notify мимо него.""" + monkeypatch.setattr(config.settings, "rate_limit", 1) + monkeypatch.setattr(config.settings, "rate_limit_window_s", 60.0) + monkeypatch.setattr(config.settings, "rate_limit_authenticated_multiplier", 2) + + app = FastAPI() + app.add_middleware(RateLimitMiddleware) + + @app.get("/api/v1/ping") + def ping() -> dict[str, bool]: + return {"ok": True} + + @app.post("/api/v1/trade-in/payments/notify") + def notify() -> dict[str, bool]: + return {"ok": True} + + return TestClient(app) + + +def test_notify_path_bypasses_general_limiter_400_requests_zero_429(notify_client): + """PR-D2 acceptance criteria: 400 запросов к notify с одного адреса за минуту + не дают ни одного отказа по частоте — даже с общим лимитом искусственно + зажатым до 1/60с (см. фикстуру).""" + statuses = [ + notify_client.post("/api/v1/trade-in/payments/notify").status_code for _ in range(400) + ] + assert all( + code == 200 for code in statuses + ), f"notify получил 429 хотя бы раз: {[c for c in statuses if c != 200]}" + + +def test_general_limiter_still_active_for_other_paths(notify_client): + """Контроль: общий лимитер НЕ выключен целиком — обычный /api/v1/ping с тем + же крошечным лимитом (1/60с) отбивается на 2-м запросе как обычно. Доказывает, + что notify-bypass узкий (точный путь), а не побочный эффект общей поломки.""" + assert notify_client.get("/api/v1/ping").status_code == 200 + assert notify_client.get("/api/v1/ping").status_code == 429 + + +def test_notify_bypass_has_own_dedicated_limiter_not_unlimited(): + """notify НЕ отключён от лимитера вовсе — своя щедрая, но конечная корзина + (`_notify_limiter`, идиома `SlidingWindowLimiter` из `support.py`). Проверяем + напрямую: исчерпать маленький искусственный лимит и убедиться, что backstop + таки срабатывает (защита от полного disable вместо узкого бюджета).""" + from app.core import ratelimit as ratelimit_module + + limiter = ratelimit_module.SlidingWindowLimiter(limit=2, window_s=60.0) + assert limiter.check("1.2.3.4") is None + assert limiter.check("1.2.3.4") is None + # 3-й запрос того же ключа — уже за лимитом (backstop жив). + assert limiter.check("1.2.3.4") is not None + + +def test_notify_limiter_key_is_per_ip_not_global(monkeypatch): + """Бюджет notify — per-IP (не общий на все входящие сразу), тот же принцип + ключа, что общий лимитер использует для анонимного трафика.""" + monkeypatch.setattr(config.settings, "rate_limit", 300) + monkeypatch.setattr(config.settings, "rate_limit_window_s", 60.0) + + from app.core import ratelimit as ratelimit_module + + monkeypatch.setattr( + ratelimit_module, + "_notify_limiter", + ratelimit_module.SlidingWindowLimiter(limit=1, window_s=60.0), + ) + + app = FastAPI() + app.add_middleware(RateLimitMiddleware) + + @app.post("/api/v1/trade-in/payments/notify") + def notify() -> dict[str, bool]: + return {"ok": True} + + client = TestClient(app) + # Первый запрос с IP #1 — проходит, второй с тем же IP — 429 (лимит=1). + headers_ip1 = {"X-Forwarded-For": "1.1.1.1"} + headers_ip2 = {"X-Forwarded-For": "2.2.2.2"} + assert client.post("/api/v1/trade-in/payments/notify", headers=headers_ip1).status_code == 200 + assert client.post("/api/v1/trade-in/payments/notify", headers=headers_ip1).status_code == 429 + # Другой IP — своя, независимая корзина. + assert client.post("/api/v1/trade-in/payments/notify", headers=headers_ip2).status_code == 200 + + def test_sliding_window_limiter_prunes_empty_buckets_past_threshold(): """review L2: пустые корзины чистятся при накоплении >10000 ключей (тот же паттерн, что `RateLimitMiddleware.dispatch`) — не бесконечная утечка памяти. diff --git a/tradein-mvp/backend/tests/test_request_audit.py b/tradein-mvp/backend/tests/test_request_audit.py index 06bbd2d5..1e3f04c8 100644 --- a/tradein-mvp/backend/tests/test_request_audit.py +++ b/tradein-mvp/backend/tests/test_request_audit.py @@ -262,6 +262,37 @@ def test_login_event_type_when_request_succeeds(client: TestClient) -> None: assert login_calls[0].kwargs["payload"] == {"status_code": 200} +# ── Платёжная нотификация — вне аудита (PR-D2, критерий приёмки #4) ───────── + + +def test_payments_notify_path_skips_audit_even_with_spoofed_admin_header() -> None: + """PR-D2 acceptance criteria: запрос к нотификации не создаёт записей в + журнале аудита — даже с заголовком `X-Authenticated-User: admin`. Middleware + — ВНЕШНИЙ относительно rbac_guard и читает сырой заголовок напрямую (см. + docstring `request_audit.py`), так что анонимный POST со спуфнутым + заголовком иначе писал бы фальшивое `login`/`api_request` событие с + атрибуцией admin, хотя rbac этот путь пока (до PR-D3) закрывает 401'ом + отдельно и независимо от этого middleware.""" + app = FastAPI() + app.add_middleware(RequestAuditMiddleware) + + @app.post("/api/v1/trade-in/payments/notify") + def notify() -> dict[str, bool]: + return {"ok": True} + + with ( + patch("app.core.request_audit.schedule_event") as mock_schedule, + patch("app.core.request_audit.should_log_login", return_value=True), + ): + resp = TestClient(app).post( + "/api/v1/trade-in/payments/notify", + headers={"X-Authenticated-User": "admin"}, + ) + + assert resp.status_code == 200 + mock_schedule.assert_not_called() + + def test_login_failed_event_type_when_rbac_rejects_request() -> None: """Ответ >= 400 (напр. RBAC-отказ downstream: неизвестная роль / протухший внутренний секрет) -> event_type='login_failed', а НЕ 'login' — раньше эти diff --git a/tradein-mvp/backend/tests/test_sentry_init_wiring.py b/tradein-mvp/backend/tests/test_sentry_init_wiring.py new file mode 100644 index 00000000..46b86499 --- /dev/null +++ b/tradein-mvp/backend/tests/test_sentry_init_wiring.py @@ -0,0 +1,111 @@ +"""PR-D2 (платёжный периметр): каждая точка инициализации `sentry_sdk.init(...)` +в проекте обязана проводить ОБА канала мониторинга — `before_send` (error-события) +и `before_send_transaction` (performance-трейсы). Мотивирующий инцидент (соседний +продукт, Птица, вчера): закрыли только error-канал через `before_send`, а +`before_send_transaction` остался вообще без обработчика — очистка body/PII там +не применялась. + +Инициализация происходит на module-level внутри `if settings.glitchtip_dsn:` — +поведенческий тест потребовал бы реального импорта модуля с DSN, выставленным +ДО импорта (модуль кэшируется, monkeypatch settings после импорта на init уже не +влияет), плюс `sentry_sdk.init` — процесс-глобальный singleton (повторные вызовы +из разных тестов друг друга затирают). Вместо этого — статический разбор AST: +детерминирован, не трогает process-global state, не зависит от порядка тестов. + +НЕ grep/substring по тексту файла: `before_send_transaction` уже упоминается в +docstring-комментариях этих же файлов (объясняющих МОТИВ) — substring-поиск дал +бы ложный PASS без единой реальной проводки в `sentry_sdk.init(...)`. Разбор +именно keyword-аргументов AST Call-узла `sentry_sdk.init(...)` не подвержен +этому false positive. +""" + +from __future__ import annotations + +import ast +from pathlib import Path + +import pytest + +_APP_DIR = Path(__file__).resolve().parent.parent / "app" + +# Все известные точки инициализации sentry_sdk в проекте (backend API, scraper +# scheduler, telegram support-bridge). Список сверяется отдельным тестом ниже +# против грепа по всему `app/`, чтобы новая точка инициализации не прошла мимо +# этого файла молча. +_SENTRY_INIT_FILES = ["main.py", "scheduler_main.py", "tgbot_main.py"] + + +def _sentry_init_calls(source: str, filename: str) -> list[ast.Call]: + """Все AST Call-узлы вида `sentry_sdk.init(...)` в модуле.""" + tree = ast.parse(source, filename=filename) + calls = [] + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + func = node.func + if ( + isinstance(func, ast.Attribute) + and func.attr == "init" + and isinstance(func.value, ast.Name) + and func.value.id == "sentry_sdk" + ): + calls.append(node) + return calls + + +@pytest.mark.parametrize("filename", _SENTRY_INIT_FILES) +def test_sentry_init_wires_both_channels(filename: str) -> None: + source = (_APP_DIR / filename).read_text(encoding="utf-8") + calls = _sentry_init_calls(source, filename) + assert calls, f"{filename}: sentry_sdk.init(...) call not found (файл переехал?)" + for call in calls: + kwarg_names = {kw.arg for kw in call.keywords if kw.arg is not None} + assert "before_send" in kwarg_names, ( + f"{filename}: sentry_sdk.init(...) не передаёт before_send — " + "error-канал уходит в GlitchTip без scrub" + ) + assert "before_send_transaction" in kwarg_names, ( + f"{filename}: sentry_sdk.init(...) не передаёт before_send_transaction — " + "transaction-канал уходит в GlitchTip без scrub (ровно вчерашний баг Птицы)" + ) + + +def test_sentry_init_before_send_and_transaction_use_same_handler() -> None: + """`before_send` и `before_send_transaction` обязаны указывать на ОДИН и тот + же обработчик (одинаковое имя переменной/функции в keyword-значении) — иначе + возможен регресс, при котором кто-то поправит один канал и забудет второй, + хотя формально оба параметра присутствуют.""" + for filename in _SENTRY_INIT_FILES: + source = (_APP_DIR / filename).read_text(encoding="utf-8") + calls = _sentry_init_calls(source, filename) + for call in calls: + kwargs = {kw.arg: kw.value for kw in call.keywords if kw.arg is not None} + before_send = kwargs.get("before_send") + before_send_txn = kwargs.get("before_send_transaction") + assert before_send is not None and before_send_txn is not None + # Оба значения — ссылки на имя (ast.Name), сравниваем идентификатор. + assert isinstance(before_send, ast.Name) + assert isinstance(before_send_txn, ast.Name) + assert before_send.id == before_send_txn.id, ( + f"{filename}: before_send={before_send.id!r} != " + f"before_send_transaction={before_send_txn.id!r} — разные обработчики " + "на двух каналах, ровно тот класс бага, что и голый пропуск канала" + ) + + +def test_all_sentry_init_call_sites_are_enumerated() -> None: + """Если кто-то добавит НОВУЮ точку инициализации sentry_sdk.init(...) где-то + ещё в app/ — этот тест должен упасть, а не молча пропустить её мимо теста + выше (список `_SENTRY_INIT_FILES` — руками поддерживаемый allowlist).""" + found_files = set() + for py_file in _APP_DIR.rglob("*.py"): + source = py_file.read_text(encoding="utf-8") + if _sentry_init_calls(source, str(py_file)): + found_files.add(py_file.relative_to(_APP_DIR).as_posix()) + + expected = set(_SENTRY_INIT_FILES) + assert found_files == expected, ( + f"Точки инициализации sentry_sdk.init(...) разошлись со списком в тесте: " + f"найдено {sorted(found_files)}, ожидалось {sorted(expected)}. Новую точку " + "нужно добавить в _SENTRY_INIT_FILES ЭТОГО файла и проверить оба канала." + ) diff --git a/tradein-mvp/backend/tests/test_sentry_scrub.py b/tradein-mvp/backend/tests/test_sentry_scrub.py index 67d26650..4925c48d 100644 --- a/tradein-mvp/backend/tests/test_sentry_scrub.py +++ b/tradein-mvp/backend/tests/test_sentry_scrub.py @@ -14,6 +14,7 @@ os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost: from app.observability.sentry_scrub import ( redact_telegram_bot_token, + scrub_payment_request_body, scrub_pii_event, stabilize_retry_error_fingerprint, ) @@ -232,6 +233,128 @@ def test_bare_token_redaction_leaves_benign_colon_strings_untouched(benign: str) assert out["logentry"]["message"] == benign +# ── Платёжный body-wipe (PR-D2, критерий приёмки #1) ───────────────────────── + + +def test_scrub_payment_request_body_removes_data_for_payments_path() -> None: + """Событие мониторинга с адресом платёжного пути и телом, содержащим `Token` + и `Pan`, уходит БЕЗ ключа с телом (PR-D2 acceptance criteria).""" + event = { + "request": { + "url": "https://gendsgn.ru/api/v1/trade-in/payments/notify", + "data": { + "Token": "deadbeefdeadbeefdeadbeef", + "Pan": "220000******0000", + "ExpDate": "1230", + "CardId": "123456", + "RebillId": "987654", + "DATA": {"Email": "someone@example.com"}, + }, + "method": "POST", + } + } + out = scrub_payment_request_body(event, {}) + assert out is not None + assert "data" not in out["request"] + # Остальные поля request не тронуты. + assert out["request"]["method"] == "POST" + assert out["request"]["url"] == "https://gendsgn.ru/api/v1/trade-in/payments/notify" + + +def test_scrub_payment_request_body_covers_checkout_too() -> None: + """Матч по сегменту пути, не по конкретному эндпоинту — checkout тоже режется.""" + event = { + "request": { + "url": "https://gendsgn.ru/api/v1/trade-in/payments/checkout", + "data": {"consent": True, "product_code": "report_pdf"}, + } + } + out = scrub_payment_request_body(event, {}) + assert out is not None + assert "data" not in out["request"] + + +def test_scrub_payment_request_body_case_insensitive_url_match() -> None: + """Регистр URL не должен позволять данным проскочить — Caddy/rbac регистр + трактуют по-разному, страховка на случай, если событие всё же породилось.""" + event = { + "request": { + "url": "https://gendsgn.ru/API/V1/Trade-In/Payments/Notify", + "data": {"Token": "secret"}, + } + } + out = scrub_payment_request_body(event, {}) + assert out is not None + assert "data" not in out["request"] + + +def test_scrub_payment_request_body_leaves_other_paths_untouched() -> None: + """Не платёжный путь — тело остаётся (это не общий kill-switch на request.data).""" + event = { + "request": { + "url": "https://gendsgn.ru/api/v1/trade-in/estimate", + "data": {"area_sqm": 50, "region": "66"}, + } + } + out = scrub_payment_request_body(event, {}) + assert out is not None + assert out["request"]["data"] == {"area_sqm": 50, "region": "66"} + + +def test_scrub_payment_request_body_handles_missing_request() -> None: + out = scrub_payment_request_body({"level": "error"}, {}) + assert out == {"level": "error"} + + +def test_scrub_payment_request_body_handles_non_dict_event() -> None: + assert scrub_payment_request_body(None, {}) is None # type: ignore[arg-type] + + +def test_scrub_payment_request_body_handles_missing_url() -> None: + """`request` без `url` (нестандартный event) — не бросает, тело не трогает.""" + event = {"request": {"data": {"Token": "x"}}} + out = scrub_payment_request_body(event, {}) + assert out is not None + assert out["request"]["data"] == {"Token": "x"} + + +# ── Расширенный набор платёжных PII-ключей (PR-D2, критерий приёмки #2) ────── + + +def test_pii_keys_scrub_payment_fields_at_arbitrary_depth() -> None: + """Скрабер вычищает `customer_email`/`customer_phone`/платёжные поля на + произвольной глубине вложенности (PR-D2 acceptance criteria).""" + event = { + "extra": { + "checkout_context": { + "buyer": { + "customer_email": "buyer@example.com", + "customer_phone": "+79991234567", + "nested_list": [ + {"pan": "220000******1111", "expdate": "0129"}, + {"cardid": "abc123", "rebillid": "xyz789"}, + ], + }, + "token": "sensitive-token-value", + "terminalkey": "TinkoffBankTest", + "order_id": "ord_123", + } + } + } + out = scrub_pii_event(event, {}) + ctx = out["extra"]["checkout_context"] + assert ctx["buyer"]["customer_email"] == "[REDACTED]" + assert ctx["buyer"]["customer_phone"] == "[REDACTED]" + assert ctx["buyer"]["nested_list"][0]["pan"] == "[REDACTED]" + assert ctx["buyer"]["nested_list"][0]["expdate"] == "[REDACTED]" + assert ctx["buyer"]["nested_list"][1]["cardid"] == "[REDACTED]" + assert ctx["buyer"]["nested_list"][1]["rebillid"] == "[REDACTED]" + assert ctx["token"] == "[REDACTED]" + assert ctx["terminalkey"] == "[REDACTED]" + # non-PII поле остаётся. + assert ctx["order_id"] == "ord_123" + + def test_composed_before_send_scrubs_pii_and_token_together() -> None: """Композиция, реально используемая в `app.tgbot_main._before_send`: PII-scrub (ключ-based) И token-redaction (regex full-text) применяются оба, не заменяя @@ -262,6 +385,37 @@ def test_composed_before_send_scrubs_pii_and_token_together() -> None: assert "8663867262:AAExampleSecretPartAbCdEf123" not in frame_url +def test_composed_before_send_payment_wipe_pii_and_token_together() -> None: + """Полная композиция `app.main._before_send` (PR-D2): body-wipe для платёжного + пути → PII-scrub → token-redaction, в этом порядке, все три применяются.""" + event = { + "request": { + "url": "https://gendsgn.ru/api/v1/trade-in/payments/notify", + "data": {"Token": "deadbeef", "Pan": "220000******0000"}, + }, + "extra": {"client_phone": "+79991234567"}, + "exception": { + "values": [{"stacktrace": {"frames": [{"vars": {"url": _LEAKED_TOKEN_URL}}]}}] + }, + } + + def composed_before_send(evt, hint): + scrubbed = scrub_payment_request_body(evt, hint) + if scrubbed is None: + return None + scrubbed = scrub_pii_event(scrubbed, hint) + if scrubbed is None: + return None + return redact_telegram_bot_token(scrubbed, hint) + + out = composed_before_send(event, {}) + assert out is not None + assert "data" not in out["request"] + assert out["extra"]["client_phone"] == "[REDACTED]" + frame_url = out["exception"]["values"][0]["stacktrace"]["frames"][0]["vars"]["url"] + assert "8663867262:AAExampleSecretPartAbCdEf123" not in frame_url + + # ── RetryError fingerprint stabilization (glitchtip-noise, #) ─ # # tenacity.RetryError.__str__() тащит repr() последнего Future — memory address diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/base.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/base.py index 36f8e6e3..58ad9b0d 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/base.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/base.py @@ -636,6 +636,19 @@ def save_listings( is_rosreestr_checked = COALESCE( EXCLUDED.is_rosreestr_checked, listings.is_rosreestr_checked ), + -- listing_segment раньше писался ТОЛЬКО при INSERT — строка, единожды + -- родившаяся с NULL-сегментом (классификатор не сработал в первый + -- скрейп), никогда не самочинилась на повторных сборах, даже когда + -- сегмент становился определим. Симптом лечили отдельной джобой + -- деактивации (deactivate_stale_{cian,yandex}_null_segment, + -- миграция 266, PR #2908) — это чистит мусор, но не устраняет причину. + -- COALESCE, а не голый EXCLUDED: если СЕЙЧАС скрейп снова не смог + -- определить сегмент (EXCLUDED.listing_segment IS NULL), нельзя затирать + -- уже известное значение пустым — та же защита, что и для + -- city/kitchen/ceiling выше. + listing_segment = COALESCE( + EXCLUDED.listing_segment, listings.listing_segment + ), -- Yandex rich fields (+ shared description/agency/publish_date): -- COALESCE so a source that does not provide them never wipes -- a value previously written by another source / scrape. @@ -738,6 +751,11 @@ def save_listings( is_rosreestr_checked = COALESCE( :is_rosreestr_checked, is_rosreestr_checked ), + -- см. ON CONFLICT DO UPDATE выше — тот же self-heal для + -- listing_segment, теперь и на reconcile-пути (dedup_hash + -- drift). Без этого строки, прошедшие через reconcile, + -- остались бы с тем же незалеченным NULL-сегментом. + listing_segment = COALESCE(:listing_segment, listing_segment), publish_date = COALESCE(:publish_date, publish_date), days_on_market = COALESCE(:days_on_market, days_on_market), description = COALESCE(:description, description),