From bed2b7bca9eb62ccca09ddebc454681820a822fa Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 1 Aug 2026 22:12:52 +0300 Subject: [PATCH] =?UTF-8?q?fix(tradein/proxy):=20pin=20ASocks=20rotate=5Fu?= =?UTF-8?q?rl=20host=20=E2=80=94=20=D0=BD=D0=B5=20=D1=81=D0=BB=D0=B0=D1=82?= =?UTF-8?q?=D1=8C=20=D1=82=D0=BE=D0=BA=D0=B5=D0=BD=20=D0=BD=D0=B0=20=D1=87?= =?UTF-8?q?=D1=83=D0=B6=D0=BE=D0=B9=20=D0=BF=D1=80=D0=BE=D0=BA=D1=81=D0=B8?= =?UTF-8?q?=20(#2600)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scrape_proxies.rotate_url колонка неоднородна: прод несёт и mobileproxy changeip-ссылки (id 3/4/5), и ASocks-ссылки (id 1/9/10/11). Без явной проверки хоста Authorization: Bearer ушёл бы на чужой провайдер — security review PR #2611. Добавлен ALLOWED_ROTATE_HOST-пиннинг (https-only, хост == api.asocks.com) ДО HTTP-вызова; несовпадение — отказ, не безголовый запрос без Authorization (смысл ручной ротации — конкретный провайдер). Заодно: класс исключения (не секрет) в note сетевой ошибки — отличить ConnectError от ReadTimeout; расширено leak-покрытие на текст log/Sentry сообщений (не только reason/note). --- .../backend/app/services/proxy_rotation.py | 71 +++++++++- .../tests/services/test_proxy_rotation.py | 123 +++++++++++++++++- 2 files changed, 187 insertions(+), 7 deletions(-) diff --git a/tradein-mvp/backend/app/services/proxy_rotation.py b/tradein-mvp/backend/app/services/proxy_rotation.py index 9ff922c8..ad711655 100644 --- a/tradein-mvp/backend/app/services/proxy_rotation.py +++ b/tradein-mvp/backend/app/services/proxy_rotation.py @@ -41,6 +41,21 @@ str(exc) — см. комментарий в app.api.v1.admin.rotate_proxy_ip (~ httpx-исключения несут полный request URL/детали, поэтому наружу — только нейтральный reason, полные детали — в лог с exc_info=True. +⛔ Хост-пиннинг (security review PR #2611): scrape_proxies.rotate_url колонка +НЕОДНОРОДНА — часть строк пула (id 3/4/5 на проде) несёт mobileproxy changeip- +ссылки (`https://changeip.mobileproxy.space/?proxy_key=<секрет mobileproxy>`, +см. app.api.v1.admin._provider_rotate_url / avito_proxy_rotate_url), не ASocks. +Без явной проверки хоста наш `Authorization: Bearer ` ушёл бы +на ЧУЖОЙ провайдер (mobileproxy) — плюс сам GET/POST по их changeip, вероятно, +реально ротирует ИХ IP и тратит ИХ суточный лимит, а мы бы записали это как +успех ASocks. rotate_proxy ПЕРЕД любым HTTP-вызовом проверяет +urlparse(rotate_url).hostname == ALLOWED_ROTATE_HOST (https-only) — несовпадение +это ОТКАЗ (ok=False, нейтральный reason), а НЕ попытка безголового запроса без +Authorization: смысл ручной ротации — конкретный провайдер (ASocks), молчаливый +вызов чужой ручки без авторизации — это сюрприз оператору (он думает "ASocks +ротировал", а фактически задел mobileproxy), которого проще не допустить, чем +потом объяснять админу расхождение счётчиков. + psycopg v3 / SQLAlchemy text(): все параметры через CAST(:x AS type), НЕ :x::type. """ @@ -49,6 +64,7 @@ from __future__ import annotations import logging from dataclasses import dataclass from typing import Any +from urllib.parse import urlparse import httpx from sqlalchemy import text @@ -59,6 +75,7 @@ from app.core.config import settings logger = logging.getLogger(__name__) __all__ = [ + "ALLOWED_ROTATE_HOST", "DAILY_ROTATION_LIMIT", "RotationResult", "rotate_proxy", @@ -70,6 +87,21 @@ DAILY_ROTATION_LIMIT = 3 # Таймаут POST refresh-ip. Пункт задачи требует "~30с". _ROTATE_TIMEOUT_S = 30.0 +# Единственный хост, на который разрешено уходить с ASOCKS_API_TOKEN в заголовке +# (см. "⛔ Хост-пиннинг" в docstring модуля). scrape_proxies.rotate_url может +# нести ЧУЖИЕ changeip-ссылки (mobileproxy и т.п.) — сравнение ДО HTTP-вызова. +ALLOWED_ROTATE_HOST = "api.asocks.com" + + +def _is_allowed_rotate_url(url: str) -> bool: + """https-only + hostname точно ALLOWED_ROTATE_HOST (регистронезависимо — + urlparse().hostname уже лоуеркейзит). Не бросает исключений на кривом url.""" + try: + parsed = urlparse(url) + except ValueError: + return False + return parsed.scheme == "https" and parsed.hostname == ALLOWED_ROTATE_HOST + @dataclass class RotationResult: @@ -194,11 +226,14 @@ async def rotate_proxy(db: Session, proxy_id: int) -> RotationResult: Порядок: 1. proxy_id не найден в scrape_proxies → ok=False, reason нейтральный. 2. rotate_url пусто → ok=False, "ротация не поддерживается" (НЕ ошибка). - 3. ASOCKS_API_TOKEN не задан (settings.asocks_api_token) → ok=False, + 3. rotate_url хост != ALLOWED_ROTATE_HOST (https://api.asocks.com) → ok=False + ДО HTTP-вызова — токен не должен уйти на чужой провайдер (mobileproxy + changeip и т.п. в этой же колонке пула, см. "⛔ Хост-пиннинг" в модуле). + 4. ASOCKS_API_TOKEN не задан (settings.asocks_api_token) → ok=False, внятный отказ, ничего не ломается. - 4. Суточный лимит (см. _quota_used_today) исчерпан → ok=False, отказ БЕЗ + 5. Суточный лимит (см. _quota_used_today) исчерпан → ok=False, отказ БЕЗ обращения к API. - 5. POST rotate_url с Authorization: Bearer , timeout ~30с. + 6. POST rotate_url с Authorization: Bearer , timeout ~30с. - Сетевая ошибка (нет ответа) → ok=False, аудит-запись http_status=NULL (НЕ считается в лимите), нейтральный reason, детали в лог exc_info=True. - 401 → громкий отказ (_alert_stale_token) + аудит-запись (НЕ считается @@ -229,6 +264,24 @@ async def rotate_proxy(db: Session, proxy_id: int) -> RotationResult: ok=False, reason="rotation not supported for this proxy (no rotate_url configured)" ) + if not _is_allowed_rotate_url(rotate_url): + # scrape_proxies.rotate_url колонка неоднородна (другие строки пула несут + # mobileproxy changeip-ссылки с ИХ секретом) — отправлять наш + # Authorization: Bearer на непроверенный хост нельзя. + # Логируем ТОЛЬКО hostname (не полный url — на других провайдерах он + # несёт их собственный секрет в query-string, тот же класс утечки, что + # и в rotate_proxy_ip, см. модуль docstring). + logger.warning( + "proxy_rotation: proxy_id=%d rotate_url host=%r is not the allowed ASocks host " + "(%s) — refusing before any HTTP call to avoid leaking the token to it", + proxy_id, + urlparse(rotate_url).hostname, + ALLOWED_ROTATE_HOST, + ) + return RotationResult( + ok=False, reason="rotation not supported for this proxy (unexpected rotate host)" + ) + token = settings.asocks_api_token if not token: logger.warning( @@ -254,15 +307,21 @@ async def rotate_proxy(db: Session, proxy_id: int) -> RotationResult: try: async with httpx.AsyncClient(timeout=_ROTATE_TIMEOUT_S) as client: resp = await client.post(rotate_url, headers={"Authorization": f"Bearer {token}"}) - except Exception: + except Exception as exc: # Ответа не было вообще — не подтверждено, что запрос дошёл до провайдера, # значит квота НЕ тратится. str(exc) НИКОГДА не идёт наружу (может нести - # служебные детали соединения) — только exc_info=True в лог. + # служебные детали соединения) — только exc_info=True в лог. type(exc).__name__ + # секрета не несёт (это имя класса — ConnectError/ReadTimeout/…) и в note + # ПОЛЕЗЕН оператору: отличить "не дозвонились" от "дозвонились, зависли". logger.warning( "proxy_rotation: request failed (no response) proxy_id=%d", proxy_id, exc_info=True ) _record_attempt( - db, proxy_id, success=False, http_status=None, note="request failed (no response)" + db, + proxy_id, + success=False, + http_status=None, + note=f"request failed: {type(exc).__name__}", ) return RotationResult( ok=False, diff --git a/tradein-mvp/backend/tests/services/test_proxy_rotation.py b/tradein-mvp/backend/tests/services/test_proxy_rotation.py index a51adbd4..a5762f9a 100644 --- a/tradein-mvp/backend/tests/services/test_proxy_rotation.py +++ b/tradein-mvp/backend/tests/services/test_proxy_rotation.py @@ -10,8 +10,12 @@ FakeSession эмулирует scrape_proxies (одна строка) + scrape_p - Успешная ротация пишет запись в scrape_proxy_rotations (success=True). - 401 → logger.error (громкий отказ) + sentry_sdk.capture_message (мониторинг), аудит-запись пишется, но НЕ считается против суточного лимита. - - Токен не появляется ни в RotationResult.reason, ни в note аудит-записи — + - Токен не появляется ни в RotationResult.reason, ни в note аудит-записи, ни в + тексте log-сообщений (caplog.getMessage()), ни в тексте, ушедшем в Sentry — ни в одном из сценариев (сеть-ошибка, 401, provider 5xx, success). + - rotate_url на ЧУЖОМ хосте (не ALLOWED_ROTATE_HOST) → отказ ДО HTTP-вызова — + scrape_proxies.rotate_url колонка неоднородна (несёт и mobileproxy changeip- + ссылки), наш ASOCKS_API_TOKEN не должен уйти на них (security review PR #2611). """ from __future__ import annotations @@ -167,6 +171,10 @@ def _no_http_allowed(): _DEFAULT_ROTATE_URL = "https://api.asocks.com/unlimited-proxy/1/refresh-ip" +# (name, rotate_url, response=(status, json_body)|None, exception|None) — ровно один +# из response/exception задан, либо оба None (локальный отказ, HTTP не идёт). +_LogScenario = tuple[str, str | None, tuple[int, dict[str, Any] | None] | None, Exception | None] + def _proxy_row(rotate_url: str | None = _DEFAULT_ROTATE_URL) -> dict[str, Any]: return {"id": 1, "rotate_url": rotate_url} @@ -202,6 +210,64 @@ async def test_no_rotate_url_is_not_an_error(monkeypatch: pytest.MonkeyPatch) -> assert db.rotations == [] # ничего не писалось — попытки не было +# ── host pinning (security review PR #2611) ───────────────────────────────── +# +# scrape_proxies.rotate_url колонка неоднородна: прод сейчас несёт mobileproxy +# changeip-ссылки (id 3/4/5) БОК О БОК с ASocks-ссылками (id 1/9/10/11, миграция +# 199). Без host-пиннинга наш Authorization: Bearer ушёл бы +# на чужой провайдер. + + +async def test_rotate_url_on_foreign_host_refused_before_http_call( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(proxy_rotation.settings, "asocks_api_token", SECRET_TOKEN) + monkeypatch.setattr(proxy_rotation.httpx, "AsyncClient", _no_http_allowed()) + + foreign_url = "https://changeip.mobileproxy.space/?proxy_key=mobileproxy-own-secret" + db = FakeSession(_proxy_row(rotate_url=foreign_url)) + result = await proxy_rotation.rotate_proxy(db, 1) # type: ignore[arg-type] + + assert result.ok is False + assert result.reason is not None + # _no_http_allowed() would have raised AssertionError from within rotate_proxy + # if the code had tried an HTTP call (i.e. sent our token) — reaching this + # line means it refused first. Belt-and-suspenders: no audit row either + # (this is a local rejection, same as no-rotate_url/no-token/limit). + assert db.rotations == [] + assert SECRET_TOKEN not in result.reason + + +async def test_allowed_host_case_insensitive_still_proceeds( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Хост сверяется без учёта регистра (urlparse().hostname лоуеркейзит) — тот + же ALLOWED_ROTATE_HOST в другом регистре ДОЛЖЕН проходить, иначе пиннинг + превратился бы в ложный отказ на легитимном rotate_url.""" + monkeypatch.setattr(proxy_rotation.settings, "asocks_api_token", SECRET_TOKEN) + fake_client, calls = _fake_async_client(response=(200, {"ip": "1.2.3.4"}), exception=None) + monkeypatch.setattr(proxy_rotation.httpx, "AsyncClient", fake_client) + + db = FakeSession(_proxy_row(rotate_url="https://API.ASOCKS.COM/unlimited-proxy/1/refresh-ip")) + result = await proxy_rotation.rotate_proxy(db, 1) # type: ignore[arg-type] + + assert result.ok is True + assert len(calls) == 1 + + +async def test_allowed_host_over_plain_http_is_refused(monkeypatch: pytest.MonkeyPatch) -> None: + """http:// (не https://) на тот же хост — отказ (защита от даунгрейда + транспорта, которым Authorization ушёл бы в открытом виде).""" + monkeypatch.setattr(proxy_rotation.settings, "asocks_api_token", SECRET_TOKEN) + monkeypatch.setattr(proxy_rotation.httpx, "AsyncClient", _no_http_allowed()) + + db = FakeSession(_proxy_row(rotate_url="http://api.asocks.com/unlimited-proxy/1/refresh-ip")) + result = await proxy_rotation.rotate_proxy(db, 1) # type: ignore[arg-type] + + assert result.ok is False + assert db.rotations == [] + + # ── missing token → neutral refusal, no crash ─────────────────────────────── @@ -354,6 +420,9 @@ async def test_token_never_appears_in_reason_on_network_error( # сетевая ошибка не подтверждает, что провайдер обработал попытку → квота не тратится assert db.rotations[0]["http_status"] is None assert proxy_rotation._quota_used_today(db, 1) == 0 # type: ignore[arg-type] + # exception class name (не секрет) в note — оператор отличит "не дозвонились" + # (ConnectError) от "дозвонились, зависли" (ReadTimeout). + assert "ConnectError" in (db.rotations[0]["note"] or "") async def test_token_never_appears_on_provider_error_status( @@ -375,6 +444,58 @@ async def test_token_never_appears_on_provider_error_status( assert proxy_rotation._quota_used_today(db, 1) == 1 # type: ignore[arg-type] +async def test_token_never_appears_in_log_messages_or_sentry_text( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """Расширенное leak-покрытие (security review PR #2611): предыдущие тесты + проверяли только reason/note. Здесь — текст, реально уходящий в logging и в + Sentry (не exc_info-traceback, который по дизайну МОЖЕТ нести детали + исключения — см. модуль docstring; это осознанно разрешённое место). + caplog.records[i].getMessage() возвращает форматированный msg %% args, БЕЗ + exc_text — то есть эта проверка ловит именно "секрет попал в аргумент + logger.*()", а не в traceback. + """ + sentry_texts: list[str] = [] + monkeypatch.setattr( + "sentry_sdk.capture_message", + lambda msg, level=None: sentry_texts.append(msg), + ) + monkeypatch.setattr(proxy_rotation.settings, "asocks_api_token", SECRET_TOKEN) + + scenarios: list[_LogScenario] = [ + ("success", _DEFAULT_ROTATE_URL, (200, {"ip": "1.1.1.1"}), None), + ("401", _DEFAULT_ROTATE_URL, (401, {"message": "Unauthenticated"}), None), + ("provider_500", _DEFAULT_ROTATE_URL, (500, {"message": "err"}), None), + ( + "network_error", + _DEFAULT_ROTATE_URL, + None, + httpx.ConnectError(f"boom token={SECRET_TOKEN}"), + ), + ("foreign_host", "https://changeip.mobileproxy.space/?proxy_key=x", None, None), + ] + + for name, rotate_url, response, exception in scenarios: + if response is not None or exception is not None: + fake_client, _ = _fake_async_client(response=response, exception=exception) + monkeypatch.setattr(proxy_rotation.httpx, "AsyncClient", fake_client) + else: + monkeypatch.setattr(proxy_rotation.httpx, "AsyncClient", _no_http_allowed()) + + db = FakeSession(_proxy_row(rotate_url=rotate_url)) + with caplog.at_level(logging.DEBUG): + caplog.clear() + await proxy_rotation.rotate_proxy(db, 1) # type: ignore[arg-type] + + for record in caplog.records: + assert ( + SECRET_TOKEN not in record.getMessage() + ), f"scenario={name}: token leaked into log message args" + + assert sentry_texts, "expected at least one Sentry capture (401 scenario)" + assert all(SECRET_TOKEN not in text for text in sentry_texts) + + # ── proxy not found ──────────────────────────────────────────────────────────