fix(tradein): SSRF host-allowlist на admin scrape + удалён dead ADMIN_TOKEN (Refs #756)

admin scrape-эндпоинты фетчили произвольный *_url без host-проверки (SSRF
defense-in-depth поверх RBAC). _assert_allowed_url(url): абсолютный URL с
не-allowlist хостом → 400; относительный путь → ok (хост подставляется
источником). Guard в 5 хендлерах (avito_house/avito_detail/yandex_detail/
cian_detail/cian_newbuilding). scrape_allowed_hosts в config (сверено со
scrapers/*.py — добавлены ekb.cian.ru, ekaterinburg.n1.ru). ADMIN_TOKEN
(0 чтений в коде) убран из docker-compose.prod.yml + DEPLOY.md. main.py RBAC
не тронут. 11 тестов pass, ruff clean.
This commit is contained in:
bot-backend 2026-05-30 19:21:31 +03:00
parent 79a79004cf
commit 5c559ed0e3
5 changed files with 140 additions and 4 deletions

View file

@ -57,7 +57,7 @@ import /opt/gendesign/tradein-mvp/deploy/Caddyfile.tradein-fragment
shell-скриптом deploy через `source .env.runtime` перед `compose up`. shell-скриптом deploy через `source .env.runtime` перед `compose up`.
2. `/opt/gendesign/tradein-mvp/backend/.env.runtime` — переменные внутри 2. `/opt/gendesign/tradein-mvp/backend/.env.runtime` — переменные внутри
контейнера `tradein-backend` (читаются через `env_file:` в compose). Сюда контейнера `tradein-backend` (читаются через `env_file:` в compose). Сюда
попадают `YANDEX_GEOCODER_API_KEY`, `ADMIN_TOKEN`, `COOKIE_ENCRYPTION_KEY` попадают `YANDEX_GEOCODER_API_KEY`, `COOKIE_ENCRYPTION_KEY`
всё, что нужно scripts/backfill_house_coords.py и application code внутри всё, что нужно scripts/backfill_house_coords.py и application code внутри
контейнера. контейнера.
@ -81,7 +81,6 @@ COOKIE_ENCRYPTION_KEY=<64-char hex>
# Может быть симлинком на ../.env.runtime если переменные совпадают: # Может быть симлинком на ../.env.runtime если переменные совпадают:
# ln -s ../.env.runtime /opt/gendesign/tradein-mvp/backend/.env.runtime # ln -s ../.env.runtime /opt/gendesign/tradein-mvp/backend/.env.runtime
YANDEX_GEOCODER_API_KEY=<key или пусто> YANDEX_GEOCODER_API_KEY=<key или пусто>
ADMIN_TOKEN=<token или пусто>
COOKIE_ENCRYPTION_KEY=<64-char hex> COOKIE_ENCRYPTION_KEY=<64-char hex>
GENDESIGN_FDW_PASSWORD=<password или пусто> GENDESIGN_FDW_PASSWORD=<password или пусто>
GLITCHTIP_DSN=<dsn или пусто> GLITCHTIP_DSN=<dsn или пусто>

View file

@ -9,6 +9,7 @@ import json
import logging import logging
import time import time
from typing import Annotated, Any, Literal from typing import Annotated, Any, Literal
from urllib.parse import urlparse
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Query from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Query
from pydantic import BaseModel, Field from pydantic import BaseModel, Field
@ -46,6 +47,17 @@ logger = logging.getLogger(__name__)
router = APIRouter() router = APIRouter()
def _assert_allowed_url(url: str) -> None:
"""SSRF guard: отклоняем абсолютные URL с хостом не из allowlist (#756).
Относительные пути (urlparse().netloc пустой) пропускаем хост
будет подставлен фиксированным AVITO_BASE / BASE_URL в самом scraper'е.
"""
parsed = urlparse(url)
if parsed.netloc and parsed.netloc not in settings.scrape_allowed_hosts:
raise HTTPException(status_code=400, detail="host not allowed")
_ALL_SOURCES = ["avito", "cian", "yandex", "n1"] _ALL_SOURCES = ["avito", "cian", "yandex", "n1"]
@ -453,6 +465,7 @@ async def scrape_avito_house(
Flow: fetch_house_catalog save_house_catalog_enrichment. Flow: fetch_house_catalog save_house_catalog_enrichment.
Returns counters {'house_id', 'reviews', 'sellers', 'listings_linked', 'placement_history'}. Returns counters {'house_id', 'reviews', 'sellers', 'listings_linked', 'placement_history'}.
""" """
_assert_allowed_url(house_url)
try: try:
enrichment = await fetch_house_catalog(house_url) enrichment = await fetch_house_catalog(house_url)
except Exception as e: except Exception as e:
@ -481,6 +494,7 @@ async def scrape_avito_detail(
Flow: fetch_detail save_detail_enrichment (UPDATE listings WHERE source='avito'). Flow: fetch_detail save_detail_enrichment (UPDATE listings WHERE source='avito').
Returns {'ok', 'item_id', 'updated'}. Returns {'ok', 'item_id', 'updated'}.
""" """
_assert_allowed_url(item_url)
try: try:
enrichment = await fetch_detail(item_url) enrichment = await fetch_detail(item_url)
except Exception as e: except Exception as e:
@ -929,6 +943,7 @@ async def scrape_yandex_detail(
Returns a snapshot of the extracted DetailEnrichment (no DB write - read-only Returns a snapshot of the extracted DetailEnrichment (no DB write - read-only
debug; main pipeline writes via estimator on /estimate flow). debug; main pipeline writes via estimator on /estimate flow).
""" """
_assert_allowed_url(offer_url)
async with YandexDetailScraper() as scraper: async with YandexDetailScraper() as scraper:
result = await scraper.fetch_detail(offer_url) result = await scraper.fetch_detail(offer_url)
if result is None: if result is None:
@ -1047,6 +1062,7 @@ async def scrape_cian_detail(
If `listing_id` provided save enrichment (offer_price_history + listings updates). If `listing_id` provided save enrichment (offer_price_history + listings updates).
Without it debug-only (no DB write). Without it debug-only (no DB write).
""" """
_assert_allowed_url(offer_url)
from app.services.scrapers.cian_detail import fetch_detail, save_detail_enrichment from app.services.scrapers.cian_detail import fetch_detail, save_detail_enrichment
enrichment = await fetch_detail(offer_url) enrichment = await fetch_detail(offer_url)
@ -1090,6 +1106,7 @@ async def scrape_cian_newbuilding(
If `house_id` provided persist (houses + houses_price_dynamics + management_companies). If `house_id` provided persist (houses + houses_price_dynamics + management_companies).
Without it debug-only (no DB write). Without it debug-only (no DB write).
""" """
_assert_allowed_url(zhk_url)
from app.services.scrapers.cian_newbuilding import ( from app.services.scrapers.cian_newbuilding import (
fetch_newbuilding, fetch_newbuilding,
save_newbuilding_enrichment, save_newbuilding_enrichment,

View file

@ -108,5 +108,21 @@ class Settings(BaseSettings):
# Конфигурируется через env ESTIMATE_QUOTA_LIMIT. Default 15. # Конфигурируется через env ESTIMATE_QUOTA_LIMIT. Default 15.
estimate_quota_limit: int = 15 estimate_quota_limit: int = 15
# SSRF-защита для admin scrape endpoints (#756).
# Список хостов которым разрешено передавать абсолютные URL в параметрах *_url.
# Относительные пути (без netloc) проходят без проверки — хост подставляется
# фиксированным в самом scraper'е. Задаётся через env SCRAPE_ALLOWED_HOSTS
# (comma-separated). По умолчанию — все хосты используемые scrapers/*.py.
scrape_allowed_hosts: set[str] = {
"www.avito.ru",
"avito.ru",
"www.cian.ru",
"cian.ru",
"ekb.cian.ru",
"realty.yandex.ru",
"ekaterinburg.n1.ru",
"n1.ru",
}
settings = Settings() settings = Settings()

View file

@ -0,0 +1,106 @@
"""Unit tests for SSRF allowlist guard (_assert_allowed_url) — issue #756.
Approach: unit-tests on the helper directly (no DB/scraper fixtures needed).
The guard is a pure synchronous function; TestClient integration is not required
because the guard raises before any async scraper is called.
"""
from __future__ import annotations
import os
import sys
from unittest.mock import MagicMock
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
_wp_mock = MagicMock()
sys.modules.setdefault("weasyprint", _wp_mock)
sys.modules.setdefault("weasyprint.CSS", _wp_mock)
sys.modules.setdefault("weasyprint.HTML", _wp_mock)
import pytest # noqa: E402
from fastapi import HTTPException # noqa: E402
from app.api.v1.admin import _assert_allowed_url # noqa: E402
from app.core.config import settings # noqa: E402
# ---------------------------------------------------------------------------
# Absolute URL — host NOT in allowlist → 400
# ---------------------------------------------------------------------------
def test_absolute_ssrf_metadata_endpoint_blocked() -> None:
"""http://169.254.169.254/... (AWS metadata) должен быть отклонён с 400."""
with pytest.raises(HTTPException) as exc_info:
_assert_allowed_url("http://169.254.169.254/latest/meta-data/")
assert exc_info.value.status_code == 400
assert "host not allowed" in exc_info.value.detail
def test_absolute_localhost_blocked() -> None:
with pytest.raises(HTTPException) as exc_info:
_assert_allowed_url("http://localhost/internal/admin")
assert exc_info.value.status_code == 400
def test_absolute_arbitrary_host_blocked() -> None:
with pytest.raises(HTTPException) as exc_info:
_assert_allowed_url("https://evil.example.com/scrape")
assert exc_info.value.status_code == 400
# ---------------------------------------------------------------------------
# Absolute URL — host IN allowlist → passes
# ---------------------------------------------------------------------------
def test_absolute_avito_allowed() -> None:
_assert_allowed_url("https://www.avito.ru/catalog/houses/some-slug/12345")
def test_absolute_cian_allowed() -> None:
_assert_allowed_url("https://ekb.cian.ru/sale/flat/327830237/")
def test_absolute_yandex_realty_allowed() -> None:
_assert_allowed_url("https://realty.yandex.ru/ekaterinburg/kupit/kvartira/123/")
def test_absolute_n1_allowed() -> None:
_assert_allowed_url("https://ekaterinburg.n1.ru/kupit/kvartiry/vtorichka/")
# ---------------------------------------------------------------------------
# Relative path — no netloc → always passes
# ---------------------------------------------------------------------------
def test_relative_path_passes_the_guard() -> None:
"""Путь без схемы/хоста (авито-стиль /ekaterinburg/...) разрешается."""
_assert_allowed_url("/ekaterinburg/kvartiry/prodam-123")
def test_relative_path_house_catalog_passes() -> None:
_assert_allowed_url("/catalog/houses/ekb-slug/99999")
def test_empty_string_passes() -> None:
"""Пустая строка — нет netloc → guard не блокирует (upstream validation задача scraper'а)."""
_assert_allowed_url("")
# ---------------------------------------------------------------------------
# Allowlist integrity: verify expected hosts present
# ---------------------------------------------------------------------------
def test_allowlist_contains_required_scraper_hosts() -> None:
required = {
"www.avito.ru",
"ekb.cian.ru",
"www.cian.ru",
"realty.yandex.ru",
"ekaterinburg.n1.ru",
}
missing = required - settings.scrape_allowed_hosts
assert not missing, f"Missing hosts in scrape_allowed_hosts: {missing}"

View file

@ -58,8 +58,6 @@ services:
# Yandex Geocoder API key — читается из backend/.env.runtime (env_file). # Yandex Geocoder API key — читается из backend/.env.runtime (env_file).
# Раньше дублировалось здесь как ${YANDEX_GEOCODER_API_KEY:-} — убрано # Раньше дублировалось здесь как ${YANDEX_GEOCODER_API_KEY:-} — убрано
# т.к. environment: overrides env_file и при пустой host-env стирает значение. # т.к. environment: overrides env_file и при пустой host-env стирает значение.
# Защита /api/v1/admin/* — задаётся в .env.runtime. Пусто = открыто (dev).
ADMIN_TOKEN: "${ADMIN_TOKEN:-}"
# GlitchTip DSN — мониторинг ошибок (#396). Пусто = выключено. # GlitchTip DSN — мониторинг ошибок (#396). Пусто = выключено.
GLITCHTIP_DSN: "${GLITCHTIP_DSN:-}" GLITCHTIP_DSN: "${GLITCHTIP_DSN:-}"
GENDESIGN_FDW_PASSWORD: "${GENDESIGN_FDW_PASSWORD:-}" GENDESIGN_FDW_PASSWORD: "${GENDESIGN_FDW_PASSWORD:-}"