diff --git a/backend/app/main.py b/backend/app/main.py index ee3969ec..7162bfa0 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -48,7 +48,7 @@ from app.core import auth_db from app.core.audit_middleware import audit_log_middleware from app.core.auth import get_role from app.core.config import settings -from app.observability.sentry_scrub import scrub_sensitive_query +from app.observability.sentry_scrub import scrub_event from app.services.auth_session import resolve_session_token logger = logging.getLogger(__name__) @@ -75,6 +75,11 @@ if not any(getattr(_h, "_gd_app_stream", False) for _h in _app_logger.handlers): # (middleware, маршруты) видели активный client с самого старта процесса. # GlitchTip не поддерживает profiling — profiles_sample_rate=0.0. if settings.glitchtip_dsn: + # before_send И before_send_transaction — ОБА на scrub_event (#2457-review): + # Starlette-интеграция кладёт request.data на transaction-scope так же, как + # на error-scope, поэтому голый scrub_sensitive_query (только URL) на + # before_send_transaction оставлял бы PII-канал открытым при любом + # glitchtip_traces_sample_rate > 0 (см. sentry_scrub.py module docstring). sentry_sdk.init( dsn=settings.glitchtip_dsn, environment=settings.environment, @@ -82,8 +87,8 @@ if settings.glitchtip_dsn: traces_sample_rate=settings.glitchtip_traces_sample_rate, profiles_sample_rate=0.0, send_default_pii=False, - before_send=scrub_sensitive_query, - before_send_transaction=scrub_sensitive_query, + before_send=scrub_event, + before_send_transaction=scrub_event, integrations=[ StarletteIntegration(), FastApiIntegration(), diff --git a/backend/app/observability/sentry_scrub.py b/backend/app/observability/sentry_scrub.py index af936c31..d40911ae 100644 --- a/backend/app/observability/sentry_scrub.py +++ b/backend/app/observability/sentry_scrub.py @@ -1,17 +1,63 @@ -"""Хук before_send_transaction для GlitchTip/Sentry SDK. +"""Хуки before_send / before_send_transaction для GlitchTip/Sentry SDK. -Redact-ит api keys / tokens из URL-spans перед отправкой — чтобы -секреты (apiKey=..., api_key=..., token=...) не утекали в GlitchTip -через HttpxIntegration performance-spans. +`scrub_sensitive_query` — redact-ит api keys / tokens из URL-spans перед +отправкой — чтобы секреты (apiKey=..., api_key=..., token=...) не утекали в +GlitchTip через HttpxIntegration performance-spans. + +`scrub_pii_event` — redact-ит consumer-PII (client_name / client_phone / +client_email / phone / email / name / company / message) из events перед +отправкой. `send_default_pii=False` в sentry_sdk.init (проверено на +sentry-sdk 2.58) НЕ покрывает эти поля — это user-data, попадающий в +request.data / extra / contexts (pilot-заявки — `PilotRequestInput` в +`app/api/v1/pilot.py` несёт все 6 полей включая свободный текст `company`/ +`message`, куда чаще всего прилетают телефоны/имена/адреса; чат — свободный +вопрос в `app/schemas/chat.py`), а не PII-заголовки/cookies, которые режет +сам флаг. Портировано из trade-in (`tradein-mvp/backend/app/observability/ +sentry_scrub.py`, #396) — тот же набор ключей (client_name/client_phone/ +client_email — Птица их не использует сегодня, но одинаковый механизм на +оба продукта проще сопровождать), плюс `company`/`message`, специфичные для +`PilotRequestInput` (#2457-review). + +`scrub_event` — composed-хендлер (PII-scrub + URL-secret redact), которым +надо вешать ОБА канала — `before_send` И `before_send_transaction`. +Starlette-интеграция кладёт тело запроса в `request_info["data"]` на +transaction-scope точно так же, как на error-scope (scope-обработчики для +transactions НЕ пропускаются — пропуск бывает только на availability-чеках). +Если повесить PII-scrub только на `before_send`, а `before_send_transaction` +оставить на голом `scrub_sensitive_query` — PII продолжит течь через +transaction-канал при любом `glitchtip_traces_sample_rate > 0` (#2457-review, +воспроизведено: pilot-заявка с реальными данными → ~1/20 попадает в +транзакцию с полным телом). """ from __future__ import annotations +import logging import re from typing import Any from sentry_sdk.types import Event +logger = logging.getLogger(__name__) + +_REDACTED = "[REDACTED]" +# Ключи consumer-PII (нижний регистр; сверка case-insensitive). Набор МЕРЫ +# (client_name/client_phone/client_email/phone/email/name, #396) + company/ +# message — специфичные для PilotRequestInput (app/api/v1/pilot.py) поля +# свободного текста (#2457-review). +_PII_KEYS = frozenset( + { + "client_name", + "client_phone", + "client_email", + "phone", + "email", + "name", + "company", + "message", + } +) + _SENSITIVE_PARAM_RE = re.compile( r"((?:api[_-]?[Kk]ey|token|access[_-]?token|secret)=)([^&\s]+)", re.IGNORECASE, @@ -47,3 +93,56 @@ def scrub_sensitive_query(event: Event, _hint: dict[str, Any]) -> Event | None: request["url"] = _redact(request["url"]) return event + + +def _scrub(obj: Any) -> None: + """Рекурсивно заменить значения PII-ключей в dict на [REDACTED] (in-place).""" + if isinstance(obj, dict): + for key, value in obj.items(): + if isinstance(key, str) and key.lower() in _PII_KEYS: + obj[key] = _REDACTED + else: + _scrub(value) + elif isinstance(obj, list): + for item in obj: + _scrub(item) + + +def scrub_pii_event(event: Event, _hint: dict[str, Any]) -> Event | None: + """Redact consumer-PII (см. `_PII_KEYS`) из event (error ИЛИ transaction) + перед отправкой в GlitchTip. + + Обходит `request.data` / `extra` / `contexts` рекурсивно (dict/list), + заменяет значения PII-ключей на [REDACTED] in-place. Возвращает event + (не None) — иначе SDK дропнет отчёт целиком. + """ + if not isinstance(event, dict): + return event + request = event.get("request") + if isinstance(request, dict): + _scrub(request.get("data")) + _scrub(event.get("extra")) + _scrub(event.get("contexts")) + return event + + +def scrub_event(event: Event, hint: dict[str, Any]) -> Event | None: + """Composed `before_send` / `before_send_transaction` handler: PII-scrub + + URL query-secret redact. Вешать ОДИНАКОВО на оба канала — см. module + docstring (#2457-review): transaction-scope несёт `request.data` точно так + же, как error-scope. + + try/except — предохранитель: sentry_sdk оборачивает вызов `before_send` в + `capture_internal_exceptions`, который при исключении внутри хендлера + ТОЛЬКО логирует и ДРОПАЕТ event целиком (SDK никогда не узнает, что + редактор упал, — event просто не уйдёт). Наблюдаемость важнее полноты + покрытия редактора: лучше отправить событие в состоянии "сколько успели + отредактировать до сбоя", чем не отправить вообще и молча остаться без + сигнала в мониторинге. + """ + try: + scrub_pii_event(event, hint) + scrub_sensitive_query(event, hint) + except Exception: + logger.exception("sentry_scrub.scrub_event: handler failed, sending event as-is") + return event diff --git a/backend/app/services/exporters/full_report_docx.py b/backend/app/services/exporters/full_report_docx.py index d85d24ca..6d6f139e 100644 --- a/backend/app/services/exporters/full_report_docx.py +++ b/backend/app/services/exporters/full_report_docx.py @@ -869,7 +869,7 @@ def _build_financial_cascade(doc: _DocxDocument, financial: dict[str, Any]) -> N ["Земля", _fmt_money_signed(financial.get("land_rub"))], ["Итого затраты", _fmt_money_signed(financial.get("cost_rub"))], ["Валовая маржа", _fmt_money_signed(financial.get("gross_margin_rub"))], - ["НДС (паркинг)", _fmt_money_signed(financial.get("vat_rub"))], + ["НДС (паркинг + коммерция)", _fmt_money_signed(financial.get("vat_rub"))], ["Прибыль до налога", _fmt_money_signed(financial.get("profit_before_tax_rub"))], ["Налог на прибыль", _fmt_money_signed(financial.get("profit_tax_rub"))], ["Чистая прибыль", _fmt_money_signed(financial.get("net_profit_rub"))], diff --git a/backend/app/services/exporters/full_report_html.py b/backend/app/services/exporters/full_report_html.py index c71725af..6087e4a4 100644 --- a/backend/app/services/exporters/full_report_html.py +++ b/backend/app/services/exporters/full_report_html.py @@ -1338,7 +1338,7 @@ def _build_financial_cascade(financial: dict[str, Any]) -> str: ["Земля", _fmt_money_signed(financial.get("land_rub"))], ["Итого затраты", _fmt_money_signed(financial.get("cost_rub"))], ["Валовая маржа", _fmt_money_signed(financial.get("gross_margin_rub"))], - ["НДС (паркинг)", _fmt_money_signed(financial.get("vat_rub"))], + ["НДС (паркинг + коммерция)", _fmt_money_signed(financial.get("vat_rub"))], ["Прибыль до налога", _fmt_money_signed(financial.get("profit_before_tax_rub"))], ["Налог на прибыль", _fmt_money_signed(financial.get("profit_tax_rub"))], ["Чистая прибыль", _fmt_money_signed(financial.get("net_profit_rub"))], diff --git a/backend/app/workers/celery_app.py b/backend/app/workers/celery_app.py index 1268bfe6..e52b8f5b 100644 --- a/backend/app/workers/celery_app.py +++ b/backend/app/workers/celery_app.py @@ -15,7 +15,7 @@ from sentry_sdk.integrations.logging import LoggingIntegration from sentry_sdk.integrations.sqlalchemy import SqlalchemyIntegration from app.core.config import settings -from app.observability.sentry_scrub import scrub_sensitive_query +from app.observability.sentry_scrub import scrub_event logger = logging.getLogger(__name__) @@ -23,6 +23,11 @@ logger = logging.getLogger(__name__) # чтобы события из тасков попадали в GlitchTip. SDK безопасен для двойного # вызова — повторный sentry_sdk.init() в одном процессе заменяет клиента. if settings.glitchtip_dsn: + # before_send И before_send_transaction — ОБА на scrub_event (#2457-review, + # см. app/main.py и sentry_scrub.py module docstring): до этого фикса worker + # вообще не скрабил error-события (тут before_send не было), а + # before_send_transaction был на голом scrub_sensitive_query (только URL) — + # оба канала пропускали PII. sentry_sdk.init( dsn=settings.glitchtip_dsn, environment=settings.environment, @@ -30,7 +35,8 @@ if settings.glitchtip_dsn: traces_sample_rate=settings.glitchtip_traces_sample_rate, profiles_sample_rate=0.0, send_default_pii=False, - before_send_transaction=scrub_sensitive_query, + before_send=scrub_event, + before_send_transaction=scrub_event, integrations=[ CeleryIntegration(monitor_beat_tasks=True), SqlalchemyIntegration(), diff --git a/backend/tests/test_sentry_init.py b/backend/tests/test_sentry_init.py index 75538a31..73dd80c4 100644 --- a/backend/tests/test_sentry_init.py +++ b/backend/tests/test_sentry_init.py @@ -2,14 +2,21 @@ Проверяем что init-блок в main.py / celery_app.py вызывает sentry_sdk.init() только при непустом GLITCHTIP_DSN, что release-fallback работает корректно, -и что scrub_sensitive_query redact-ит api keys из URL spans. +что scrub_sensitive_query redact-ит api keys из URL spans, что scrub_pii_event +redact-ит consumer-PII (client_name/client_phone/client_email/phone/email/name/ +company/message) из request.data/extra/contexts, и что composed-хендлер +scrub_event реально повешен на ОБА канала (before_send И +before_send_transaction) в main.py/celery_app.py (#2457-review). """ import os +import pathlib from unittest.mock import patch import sentry_sdk +_BACKEND_ROOT = pathlib.Path(__file__).resolve().parents[1] + def test_sdk_imports_without_error() -> None: """Все интеграции импортируются без ModuleNotFoundError.""" @@ -156,3 +163,255 @@ def test_scrub_handles_missing_spans() -> None: event: dict = {"request": {"url": "https://example.com"}} result = scrub_sensitive_query(event, {}) assert result["request"]["url"] == "https://example.com" + + +# ── scrub_pii_event (портировано из tradein-mvp, #396 / аудит-фикс) ──────────── +# +# send_default_pii=False в sentry_sdk.init НЕ закрывает consumer-PII, попадающий +# в request.data / extra / contexts (лиды pilot.py, chat.py) — это user-data, +# а не PII-заголовки/cookies, которые режет сам флаг (проверено на sentry-sdk +# 2.58). scrub_pii_event закрывает этот вектор ключ-based редактором. + + +def test_scrub_pii_redacts_request_data() -> None: + """scrub_pii_event заменяет client_name/phone/email в request.data.""" + from app.observability.sentry_scrub import scrub_pii_event + + event: dict = { + "request": { + "data": { + "client_name": "Иван Иванов", + "phone": "+79991234567", + "email": "ivan@example.com", + "address": "Екатеринбург, ул. Ленина 1", + } + } + } + result = scrub_pii_event(event, {}) + data = result["request"]["data"] + assert data["client_name"] == "[REDACTED]" + assert data["phone"] == "[REDACTED]" + assert data["email"] == "[REDACTED]" + # non-PII поле не трогаем + assert data["address"] == "Екатеринбург, ул. Ленина 1" + + +def test_scrub_pii_redacts_pilot_request_company_and_message() -> None: + """scrub_pii_event заменяет company/message — свободный текст + PilotRequestInput (app/api/v1/pilot.py), куда чаще всего прилетают + телефоны/имена/адреса, а не только фиксированные name/phone/email + (#2457-review).""" + from app.observability.sentry_scrub import scrub_pii_event + + event: dict = { + "request": { + "data": { + "company": "ООО Ромашка", + "message": "Меня зовут Иван, звоните на +79991234567", + "source": "landing", + } + } + } + result = scrub_pii_event(event, {}) + data = result["request"]["data"] + assert data["company"] == "[REDACTED]" + assert data["message"] == "[REDACTED]" + # non-PII поле не трогаем + assert data["source"] == "landing" + + +def test_scrub_pii_redacts_client_prefixed_keys() -> None: + """Полный набор ключей МЕРЫ (client_name/client_phone/client_email, #396) — + Птица их сегодня не использует, но одинаковый механизм на оба продукта + проще сопровождать (#2457-review).""" + from app.observability.sentry_scrub import scrub_pii_event + + event: dict = { + "extra": { + "client_name": "Иван", + "client_phone": "+79991234567", + "client_email": "ivan@example.com", + } + } + result = scrub_pii_event(event, {}) + extra = result["extra"] + assert extra["client_name"] == "[REDACTED]" + assert extra["client_phone"] == "[REDACTED]" + assert extra["client_email"] == "[REDACTED]" + + +def test_scrub_pii_redacts_extra() -> None: + """scrub_pii_event заменяет PII-ключи в extra, не трогая остальное.""" + from app.observability.sentry_scrub import scrub_pii_event + + event: dict = { + "extra": { + "phone": "+79990000000", + "email": "x@y.ru", + "name": "Пётр", + "lead_id": 42, + } + } + result = scrub_pii_event(event, {}) + extra = result["extra"] + assert extra["phone"] == "[REDACTED]" + assert extra["email"] == "[REDACTED]" + assert extra["name"] == "[REDACTED]" + assert extra["lead_id"] == 42 + + +def test_scrub_pii_is_case_insensitive() -> None: + """Сверка PII-ключей case-insensitive.""" + from app.observability.sentry_scrub import scrub_pii_event + + event: dict = {"extra": {"Client_Name": "Анна", "PHONE": "+7900"}} + result = scrub_pii_event(event, {}) + assert result["extra"]["Client_Name"] == "[REDACTED]" + assert result["extra"]["PHONE"] == "[REDACTED]" + + +def test_scrub_pii_redacts_nested_contexts() -> None: + """scrub_pii_event проходит вложенные dict в contexts, не трогая non-PII.""" + from app.observability.sentry_scrub import scrub_pii_event + + event: dict = {"contexts": {"trace": {"op": "http"}, "lead": {"email": "z@z.ru"}}} + result = scrub_pii_event(event, {}) + assert result["contexts"]["lead"]["email"] == "[REDACTED]" + assert result["contexts"]["trace"]["op"] == "http" + + +def test_scrub_pii_leaves_non_pii_untouched() -> None: + """scrub_pii_event не трогает event без PII-ключей.""" + from app.observability.sentry_scrub import scrub_pii_event + + event: dict = { + "request": {"data": {"region": "66", "area_sqm": 50}}, + "extra": {"job": "geocode"}, + "level": "error", + } + result = scrub_pii_event(event, {}) + assert result["request"]["data"] == {"region": "66", "area_sqm": 50} + assert result["extra"] == {"job": "geocode"} + assert result["level"] == "error" + + +def test_scrub_pii_handles_missing_sections() -> None: + """scrub_pii_event не падает на пустом event.""" + from app.observability.sentry_scrub import scrub_pii_event + + result = scrub_pii_event({}, {}) + assert result == {} + + +def test_scrub_pii_handles_none_and_non_dict_sections() -> None: + """scrub_pii_event не бросает исключений на None/не-dict секциях.""" + from app.observability.sentry_scrub import scrub_pii_event + + event: dict = {"request": None, "extra": None, "contexts": "not-a-dict"} + result = scrub_pii_event(event, {}) + assert result is event + + +def test_scrub_pii_returns_event_not_none() -> None: + """before_send должен вернуть event (не None) — иначе SDK дропнет отчёт.""" + from app.observability.sentry_scrub import scrub_pii_event + + event: dict = {"request": {"data": {"client_name": "X"}}} + result = scrub_pii_event(event, {}) + assert result is not None + assert result is event + + +# ── scrub_event (composed before_send / before_send_transaction handler) ─────── +# +# scrub_event — ЕДИНЫЙ хендлер, которым в main.py/celery_app.py вешаются ОБА +# канала (before_send И before_send_transaction). До #2457-review composed-хук +# висел только на before_send, а before_send_transaction оставался на голом +# scrub_sensitive_query (только URL) — Starlette-интеграция кладёт request.data +# на transaction-scope так же, как на error-scope, поэтому PII продолжало течь +# через transaction-канал при glitchtip_traces_sample_rate > 0. + + +def test_scrub_event_composes_pii_and_url_secret_scrub() -> None: + """scrub_event применяет PII-scrub (ключ-based) И URL query-string secret + redact (regex) оба разом, не заменяя друг друга — разные классы данных.""" + from app.observability.sentry_scrub import scrub_event + + event: dict = { + "request": { + "data": {"client_name": "Иван"}, + "url": "https://example.com?api_key=supersecret", + } + } + result = scrub_event(event, {}) + assert result is not None + assert result["request"]["data"]["client_name"] == "[REDACTED]" + assert "[REDACTED]" in result["request"]["url"] + assert "supersecret" not in result["request"]["url"] + + +def test_scrub_event_returns_event_not_none() -> None: + """scrub_event всегда возвращает event (не None) — иначе SDK дропнет отчёт.""" + from app.observability.sentry_scrub import scrub_event + + event: dict = {"request": {"data": {"name": "X"}}} + result = scrub_event(event, {}) + assert result is not None + assert result is event + + +def test_scrub_event_survives_scrub_pii_event_exception() -> None: + """try/except в scrub_event — предохранитель: sentry_sdk оборачивает + before_send в capture_internal_exceptions, который при исключении ТОЛЬКО + логирует и ДРОПАЕТ event целиком (SDK никогда не узнает, что редактор упал). + Если scrub_pii_event падает — scrub_event обязан вернуть event, а не + пробросить исключение дальше (#2457-review).""" + from app.observability.sentry_scrub import scrub_event + + event: dict = {"request": {"data": {"client_name": "X"}}} + with patch( + "app.observability.sentry_scrub.scrub_pii_event", + side_effect=RuntimeError("boom"), + ): + result = scrub_event(event, {}) + assert result is not None + assert result is event + + +def test_scrub_event_survives_scrub_sensitive_query_exception() -> None: + """То же самое для второго шага композиции (URL-secret redact).""" + from app.observability.sentry_scrub import scrub_event + + event: dict = {"request": {"data": {"name": "X"}}} + with patch( + "app.observability.sentry_scrub.scrub_sensitive_query", + side_effect=RuntimeError("boom"), + ): + result = scrub_event(event, {}) + assert result is not None + assert result is event + + +# ── wiring: before_send/before_send_transaction реально используют scrub_event ── +# +# Source-grep вместо мока sentry_sdk.init: main.py/celery_app.py вызывают +# sentry_sdk.init() на module-level import, поэтому мок пришлось бы ставить ДО +# импорта app.main — фрагильно и не переиспользуемо между тестами (модуль уже +# закэширован в sys.modules к моменту первого теста). Прямая проверка исходника +# — детерминированный, дешёвый и точный регрессионный гейт на саму строку, +# которую правил review (#2457). + + +def test_main_wires_scrub_event_to_both_channels() -> None: + """app/main.py: before_send И before_send_transaction ОБА на scrub_event.""" + text = (_BACKEND_ROOT / "app" / "main.py").read_text(encoding="utf-8") + assert "before_send=scrub_event" in text + assert "before_send_transaction=scrub_event" in text + + +def test_celery_app_wires_scrub_event_to_both_channels() -> None: + """app/workers/celery_app.py: before_send И before_send_transaction ОБА на + scrub_event (раньше before_send не было вообще).""" + text = (_BACKEND_ROOT / "app" / "workers" / "celery_app.py").read_text(encoding="utf-8") + assert "before_send=scrub_event" in text + assert "before_send_transaction=scrub_event" in text