fix(ptica): скраб ПДн на transaction-канале + company/message ключи (review #2749)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 9s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 2m15s
CI / backend-tests (pull_request) Successful in 15m14s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 9s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 2m15s
CI / backend-tests (pull_request) Successful in 15m14s
before_send_transaction в main.py/celery_app.py оставался на голом scrub_sensitive_query (только URL) — Starlette-интеграция кладёт request.data на transaction-scope так же, как на error-scope, поэтому PII продолжало течь через transaction-канал при glitchtip_traces_sample_rate > 0 (0.05 по умолчанию, config.py:19). Оба канала теперь на едином composed-хендлере scrub_event (PII-scrub + URL-secret redact), вынесенном в sentry_scrub.py. _PII_KEYS расширен до полного набора МЕРЫ (client_name/client_phone/ client_email/phone/email/name, #396) + company/message — PilotRequestInput (app/api/v1/pilot.py) несёт оба свободнотекстовых поля, куда чаще всего прилетают телефоны/имена/адреса. scrub_event обёрнут в try/except (возвращает event при сбое скраба) — sentry_sdk capture_internal_exceptions иначе только логирует и ДРОПАЕТ event целиком, если before_send бросает исключение. Убрана мёртвая ветка "if scrubbed is None: return None" — scrub_pii_event никогда не возвращает None. Тесты: новые ключи (company/message/client_phone/client_email), scrub_event composition + exception-safety, source-grep wiring-гейт на саму строку before_send_transaction=scrub_event в main.py/celery_app.py.
This commit is contained in:
parent
d7ccf48000
commit
cf7e7ec8c8
4 changed files with 216 additions and 55 deletions
|
|
@ -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_pii_event, 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,17 +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:
|
||||
|
||||
def _before_send(event: dict[str, object], hint: dict[str, object]) -> dict[str, object] | None:
|
||||
"""Композиция PII-scrub (client_name/phone/email/name из request.data/
|
||||
extra/contexts, аудит-фикс) + URL query-string secrets redact (api-key/
|
||||
token в event["request"]["url"]) — разные классы данных, `send_default_pii=
|
||||
False` ни то ни другое не закрывает (проверено на sentry-sdk 2.58)."""
|
||||
scrubbed = scrub_pii_event(event, hint) # type: ignore[arg-type]
|
||||
if scrubbed is None:
|
||||
return None
|
||||
return scrub_sensitive_query(scrubbed, hint) # type: ignore[arg-type,return-value]
|
||||
|
||||
# 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,
|
||||
|
|
@ -93,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=_before_send,
|
||||
before_send_transaction=scrub_sensitive_query,
|
||||
before_send=scrub_event,
|
||||
before_send_transaction=scrub_event,
|
||||
integrations=[
|
||||
StarletteIntegration(),
|
||||
FastApiIntegration(),
|
||||
|
|
|
|||
|
|
@ -4,25 +4,59 @@
|
|||
отправкой — чтобы секреты (apiKey=..., api_key=..., token=...) не утекали в
|
||||
GlitchTip через HttpxIntegration performance-spans.
|
||||
|
||||
`scrub_pii_event` — redact-ит consumer-PII (client_name / phone / email /
|
||||
name) из error events перед отправкой. `send_default_pii=False` в
|
||||
sentry_sdk.init (проверено на sentry-sdk 2.58) НЕ покрывает эти поля — это
|
||||
user-data, попадающий в request.data / extra / contexts (pilot-заявки,
|
||||
лиды, чат), а не PII-заголовки/cookies, которые режет сам флаг. Портировано
|
||||
из trade-in (`tradein-mvp/backend/app/observability/sentry_scrub.py`, #396) —
|
||||
тот же механизм на оба продукта, чтобы сопровождать одинаково.
|
||||
`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).
|
||||
_PII_KEYS = frozenset({"client_name", "phone", "email", "name"})
|
||||
# Ключи 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]+)",
|
||||
|
|
@ -75,7 +109,7 @@ def _scrub(obj: Any) -> None:
|
|||
|
||||
|
||||
def scrub_pii_event(event: Event, _hint: dict[str, Any]) -> Event | None:
|
||||
"""Redact consumer-PII (client_name / phone / email / name) из error event
|
||||
"""Redact consumer-PII (см. `_PII_KEYS`) из event (error ИЛИ transaction)
|
||||
перед отправкой в GlitchTip.
|
||||
|
||||
Обходит `request.data` / `extra` / `contexts` рекурсивно (dict/list),
|
||||
|
|
@ -90,3 +124,25 @@ def scrub_pii_event(event: Event, _hint: dict[str, Any]) -> Event | None:
|
|||
_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
|
||||
|
|
|
|||
|
|
@ -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_pii_event, scrub_sensitive_query
|
||||
from app.observability.sentry_scrub import scrub_event
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -23,17 +23,11 @@ logger = logging.getLogger(__name__)
|
|||
# чтобы события из тасков попадали в GlitchTip. SDK безопасен для двойного
|
||||
# вызова — повторный sentry_sdk.init() в одном процессе заменяет клиента.
|
||||
if settings.glitchtip_dsn:
|
||||
|
||||
def _before_send(event: dict[str, object], hint: dict[str, object]) -> dict[str, object] | None:
|
||||
"""Композиция PII-scrub (client_name/phone/email/name из request.data/
|
||||
extra/contexts, аудит-фикс) + URL query-string secrets redact — см.
|
||||
app/main.py._before_send (идентичная композиция; до этого фикса worker
|
||||
вообще не скрабил error-события, только transaction-spans)."""
|
||||
scrubbed = scrub_pii_event(event, hint) # type: ignore[arg-type]
|
||||
if scrubbed is None:
|
||||
return None
|
||||
return scrub_sensitive_query(scrubbed, hint) # type: ignore[arg-type,return-value]
|
||||
|
||||
# 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,
|
||||
|
|
@ -41,8 +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=_before_send,
|
||||
before_send_transaction=scrub_sensitive_query,
|
||||
before_send=scrub_event,
|
||||
before_send_transaction=scrub_event,
|
||||
integrations=[
|
||||
CeleryIntegration(monitor_beat_tasks=True),
|
||||
SqlalchemyIntegration(),
|
||||
|
|
|
|||
|
|
@ -2,16 +2,21 @@
|
|||
|
||||
Проверяем что init-блок в main.py / celery_app.py вызывает sentry_sdk.init()
|
||||
только при непустом GLITCHTIP_DSN, что release-fallback работает корректно,
|
||||
что scrub_sensitive_query redact-ит api keys из URL spans, и что
|
||||
scrub_pii_event redact-ит consumer-PII (client_name/phone/email/name) из
|
||||
request.data/extra/contexts перед отправкой в GlitchTip.
|
||||
что 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."""
|
||||
|
|
@ -191,6 +196,50 @@ def test_scrub_pii_redacts_request_data() -> None:
|
|||
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
|
||||
|
|
@ -273,12 +322,20 @@ def test_scrub_pii_returns_event_not_none() -> None:
|
|||
assert result is event
|
||||
|
||||
|
||||
def test_scrub_pii_composes_with_url_secret_scrub() -> None:
|
||||
"""Композиция, реально используемая в app.main._before_send /
|
||||
app.workers.celery_app._before_send: PII-scrub (ключ-based) и URL
|
||||
query-string secret redact (regex) применяются оба, не заменяя друг друга —
|
||||
разные классы данных."""
|
||||
from app.observability.sentry_scrub import scrub_pii_event, scrub_sensitive_query
|
||||
# ── 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": {
|
||||
|
|
@ -286,15 +343,75 @@ def test_scrub_pii_composes_with_url_secret_scrub() -> None:
|
|||
"url": "https://example.com?api_key=supersecret",
|
||||
}
|
||||
}
|
||||
|
||||
def composed_before_send(evt: dict, hint: dict) -> dict | None:
|
||||
scrubbed = scrub_pii_event(evt, hint)
|
||||
if scrubbed is None:
|
||||
return None
|
||||
return scrub_sensitive_query(scrubbed, hint)
|
||||
|
||||
result = composed_before_send(event, {})
|
||||
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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue