fix(tradein): SSRF host-allowlist на admin scrape + удалён dead ADMIN_TOKEN (Refs #756) #778
5 changed files with 140 additions and 4 deletions
|
|
@ -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 или пусто>
|
||||||
|
|
|
||||||
|
|
@ -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,
|
||||||
|
|
|
||||||
|
|
@ -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()
|
||||||
|
|
|
||||||
106
tradein-mvp/backend/tests/test_admin_ssrf.py
Normal file
106
tradein-mvp/backend/tests/test_admin_ssrf.py
Normal 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}"
|
||||||
|
|
@ -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:-}"
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue