lekss361/gendesign

Fork 0

ptica/observability: страховка скраба ПДн может утечь то, что защищает + проводка проверяется грепом по исходнику #2753

New issue

Closed

opened 2026-08-06 18:49:54 +00:00 by lekss361 · 2 comments

lekss361 commented

2026-08-06 18:49:54 +00:00

Owner

1. Логирование сбоя скраба может отправить неочищенное тело

app/observability/sentry_scrub.py — перехват обёрнут в logger.exception(...). Идея верная (иначе исключение внутри обработчика приводит к потере события целиком), но вызов выбран неудачно. Три звена, каждое проверено по установленной библиотеке:

include_local_variables по умолчанию True, и Птица его нигде не переопределяет — грепом подтверждено отсутствие include_local_variables, event_scrubber, ignore_logger во всём backend/app/;
список игнорируемых логгеров содержит только три служебных, наш модуль в него не входит — значит при уровне событий ERROR вызов создаёт новое событие;
защиты от рекурсии в SDK нет вовсе.

Итог: если скрабер когда-нибудь упадёт, получившееся событие понесёт трассировку, в локальных переменных которой лежит event — полное неочищенное тело. Собственный механизм затирания библиотеки его не поймает: он сопоставляет имена переменных, а не содержимое. Плюс интеграция заново прикрепляет то же тело к каждому событию на скоупе, так что детерминированный сбой может повторяться, а не затухнуть.

Почему это low, а не high: сериализация выполняется до вызова обработчика, поэтому на вход приходят обычные словари, списки и строки; присваивание существующему ключу во время обхода легально. Сконструировать падающий вход не удалось.

Фикс: logger.error("sentry_scrub: обработчик упал, событие уходит как есть") без трассировки — либо ignore_logger на этот модуль при инициализации.

2. `include_local_variables=False` не выставлен ни в одной точке входа Птицы

У МЕРЫ он есть (пришёл с PR #2737), у Птицы — ни в main.py, ни в workers/celery_app.py. Это шире, чем п. 1: при любом исключении в любом обработчике локальные переменные кадра уходят в мониторинг. Именно этот флаг сегодня спас пароль платёжного терминала от утечки в соседнем PR — там он выставлен.

3. Проводка обработчиков проверяется грепом по тексту исходника

Два теста в backend/tests/test_sentry_init.py ищут строку before_send=scrub_event в файлах. Причина уважительная: инициализация происходит при импорте модуля, к моменту первого теста модуль уже в sys.modules, а блок стоит за проверкой наличия ключа мониторинга, которого в тестах нет — то есть поведенческого пути к этой проверке действительно не существует, и альтернатива это «не проверять проводку вовсе».

Слабость конкретная: подстрока может совпасть с комментарием. И это не гипотетика — оба файла сейчас несут многострочные комментарии, содержащие и before_send, и before_send_transaction, и scrub_event. Достаточно переформулировать комментарий, удалив при этом сам аргумент, и гейт останется зелёным на сломанной проводке.

Фикс: разбор синтаксического дерева вместо поиска подстроки — около восьми строк, невосприимчиво к комментариям и докстрокам, остаётся проверкой по исходнику:

tree = ast.parse((_BACKEND_ROOT / "app" / "main.py").read_text(encoding="utf-8"))
call = next(n for n in ast.walk(tree)
            if isinstance(n, ast.Call) and getattr(n.func, "attr", None) == "init")
kw = {k.arg: k.value for k in call.keywords}
for ch in ("before_send", "before_send_transaction"):
    assert isinstance(kw[ch], ast.Name) and kw[ch].id == "scrub_event"

4. Формулировка про НДС во фронтенде (хвост #2457)

frontend/src/components/concept/ConceptVariantsResult.tsx — два места, не одно:

строка 659 — подпись «НДС (паркинг)», backend-часть уже поправлена на «паркинг + коммерция»;
строки 528-529 — текст «НДС начисляется только на паркинг (нежилые машиноместа)». Это утверждение сильнее подписи и прямо отрицает коммерческую составляющую, которую services/generative/financial.py:749 заведомо включает.

Образец корректной формулировки уже есть рядом: MassingEconomics.tsx:175 говорит «НДС на нежилое».

Найдено при ревью PR #2749.

Хвост после PR #2749 (скраб персональных данных перед отправкой в мониторинг). Ничего не горит, но изменение затевалось ради 152-ФЗ, и оставлять эти три вещи неисправленными в такой теме — плохой размен. ## 1. Логирование сбоя скраба может отправить неочищенное тело `app/observability/sentry_scrub.py` — перехват обёрнут в `logger.exception(...)`. Идея верная (иначе исключение внутри обработчика приводит к потере события целиком), но вызов выбран неудачно. Три звена, каждое проверено по установленной библиотеке: - `include_local_variables` по умолчанию `True`, и Птица его нигде не переопределяет — грепом подтверждено отсутствие `include_local_variables`, `event_scrubber`, `ignore_logger` во всём `backend/app/`; - список игнорируемых логгеров содержит только три служебных, наш модуль в него не входит — значит при уровне событий `ERROR` вызов **создаёт новое событие**; - защиты от рекурсии в SDK нет вовсе. Итог: если скрабер когда-нибудь упадёт, получившееся событие понесёт трассировку, в локальных переменных которой лежит `event` — полное неочищенное тело. Собственный механизм затирания библиотеки его не поймает: он сопоставляет **имена** переменных, а не содержимое. Плюс интеграция заново прикрепляет то же тело к каждому событию на скоупе, так что детерминированный сбой может повторяться, а не затухнуть. **Почему это low, а не high:** сериализация выполняется до вызова обработчика, поэтому на вход приходят обычные словари, списки и строки; присваивание существующему ключу во время обхода легально. Сконструировать падающий вход не удалось. **Фикс:** `logger.error("sentry_scrub: обработчик упал, событие уходит как есть")` без трассировки — либо `ignore_logger` на этот модуль при инициализации. ## 2. `include_local_variables=False` не выставлен ни в одной точке входа Птицы У МЕРЫ он есть (пришёл с PR #2737), у Птицы — ни в `main.py`, ни в `workers/celery_app.py`. Это шире, чем п. 1: при любом исключении в любом обработчике локальные переменные кадра уходят в мониторинг. Именно этот флаг сегодня спас пароль платёжного терминала от утечки в соседнем PR — там он выставлен. ## 3. Проводка обработчиков проверяется грепом по тексту исходника Два теста в `backend/tests/test_sentry_init.py` ищут строку `before_send=scrub_event` в файлах. Причина уважительная: инициализация происходит при импорте модуля, к моменту первого теста модуль уже в `sys.modules`, а блок стоит за проверкой наличия ключа мониторинга, которого в тестах нет — то есть поведенческого пути к этой проверке действительно не существует, и альтернатива это «не проверять проводку вовсе». Слабость конкретная: подстрока **может совпасть с комментарием**. И это не гипотетика — оба файла сейчас несут многострочные комментарии, содержащие и `before_send`, и `before_send_transaction`, и `scrub_event`. Достаточно переформулировать комментарий, удалив при этом сам аргумент, и гейт останется зелёным на сломанной проводке. **Фикс:** разбор синтаксического дерева вместо поиска подстроки — около восьми строк, невосприимчиво к комментариям и докстрокам, остаётся проверкой по исходнику: ```python tree = ast.parse((_BACKEND_ROOT / "app" / "main.py").read_text(encoding="utf-8")) call = next(n for n in ast.walk(tree) if isinstance(n, ast.Call) and getattr(n.func, "attr", None) == "init") kw = {k.arg: k.value for k in call.keywords} for ch in ("before_send", "before_send_transaction"): assert isinstance(kw[ch], ast.Name) and kw[ch].id == "scrub_event" ``` ## 4. Формулировка про НДС во фронтенде (хвост #2457) `frontend/src/components/concept/ConceptVariantsResult.tsx` — **два** места, не одно: - строка `659` — подпись «НДС (паркинг)», backend-часть уже поправлена на «паркинг + коммерция»; - строки `528-529` — текст «НДС начисляется **только** на паркинг (нежилые машиноместа)». Это утверждение сильнее подписи и прямо отрицает коммерческую составляющую, которую `services/generative/financial.py:749` заведомо включает. Образец корректной формулировки уже есть рядом: `MassingEconomics.tsx:175` говорит «НДС на нежилое». Найдено при ревью PR #2749.

bot-backend commented

2026-08-07 09:10:13 +00:00

Collaborator

НЕ ЗАКРЫТО — ни один из четырёх пунктов не сделан. Проверка на проде 2026-08-07 09:0x UTC

PR на эту задачу нет вообще — ни по API Forgejo, ни в истории коммитов. Ноль имеет причину: никто не брался.

Проверял не по main, а в живом контейнере gendesign-backend-1.

п.1 — страховка скраба по-прежнему может утечь то, что защищает

/app/app/observability/sentry_scrub.py:147
    logger.exception("sentry_scrub.scrub_event: handler failed, sending event as-is")

logger.exception на месте. Трассировка с event в локальных переменных по-прежнему уйдёт в мониторинг, если скрабер упадёт.

п.2 — `include_local_variables` не выставлен нигде

grep -rn include_local_variables /app/app/  →  ничего

Ни в main.py, ни в workers/celery_app.py. У МЕРЫ флаг есть (#2737), у Птицы нет — то есть при любом исключении в любом обработчике локальные переменные кадра уходят в мониторинг.

Это тот самый флаг, который в соседнем PR того же дня спас пароль платёжного терминала.

п.3 — проводка всё ещё проверяется поиском подстроки

backend/tests/test_sentry_init.py — тот же поиск строк по тексту исходника. В живом контейнере видно, почему это опасно именно здесь: искомые подстроки лежат в комментариях над вызовом.

/app/app/main.py:78            # before_send И before_send_transaction — ОБА на scrub_event
/app/app/main.py:90                before_send=scrub_event,
/app/app/workers/celery_app.py:26  # before_send И before_send_transaction — ОБА на scrub_event
/app/app/workers/celery_app.py:38      before_send=scrub_event,

Удалить строки 90/38 и оставить комментарии 78/26 — гейт останется зелёным на разорванной проводке. Разбор AST (восемь строк, приведены в теле задачи) это снимает.

п.4 — формулировка про НДС

frontend/src/components/concept/ConceptVariantsResult.tsx:

528-529  «НДС начисляется только на паркинг (нежилые машиноместа)»   ← не поправлено
659      label="НДС (паркинг)"                                       ← не поправлено

Оба места на месте, включая то, которое прямо отрицает коммерческую составляющую, заведомо включаемую services/generative/financial.py:749.

Итог

Закрывать нечего. Оба пункта, ради которых задача заводилась (утечка через страховку и проверяемость проводки), открыты, плюс два хвоста.

## НЕ ЗАКРЫТО — ни один из четырёх пунктов не сделан. Проверка на проде 2026-08-07 09:0x UTC PR на эту задачу нет вообще — ни по API Forgejo, ни в истории коммитов. Ноль имеет причину: **никто не брался**. Проверял не по main, а в живом контейнере `gendesign-backend-1`. ### п.1 — страховка скраба по-прежнему может утечь то, что защищает ``` /app/app/observability/sentry_scrub.py:147 logger.exception("sentry_scrub.scrub_event: handler failed, sending event as-is") ``` `logger.exception` на месте. Трассировка с `event` в локальных переменных по-прежнему уйдёт в мониторинг, если скрабер упадёт. ### п.2 — `include_local_variables` не выставлен нигде ``` grep -rn include_local_variables /app/app/ → ничего ``` Ни в `main.py`, ни в `workers/celery_app.py`. У МЕРЫ флаг есть (#2737), у Птицы нет — то есть при любом исключении в любом обработчике локальные переменные кадра уходят в мониторинг. Это тот самый флаг, который в соседнем PR того же дня спас пароль платёжного терминала. ### п.3 — проводка всё ещё проверяется поиском подстроки `backend/tests/test_sentry_init.py` — тот же поиск строк по тексту исходника. В живом контейнере видно, почему это опасно именно здесь: искомые подстроки лежат в **комментариях** над вызовом. ``` /app/app/main.py:78 # before_send И before_send_transaction — ОБА на scrub_event /app/app/main.py:90 before_send=scrub_event, /app/app/workers/celery_app.py:26 # before_send И before_send_transaction — ОБА на scrub_event /app/app/workers/celery_app.py:38 before_send=scrub_event, ``` Удалить строки 90/38 и оставить комментарии 78/26 — гейт останется зелёным на разорванной проводке. Разбор AST (восемь строк, приведены в теле задачи) это снимает. ### п.4 — формулировка про НДС `frontend/src/components/concept/ConceptVariantsResult.tsx`: ``` 528-529 «НДС начисляется только на паркинг (нежилые машиноместа)» ← не поправлено 659 label="НДС (паркинг)" ← не поправлено ``` Оба места на месте, включая то, которое прямо отрицает коммерческую составляющую, заведомо включаемую `services/generative/financial.py:749`. ### Итог Закрывать нечего. Оба пункта, ради которых задача заводилась (утечка через страховку и проверяемость проводки), открыты, плюс два хвоста.

Light1YT referenced this issue from a commit

2026-08-07 09:45:33 +00:00

fix(ptica/observability): страховка скраба перестаёт утекать то, что защищает (#2753)

Light1YT referenced this issue from a commit

2026-08-07 09:45:33 +00:00

fix(ptica/concept): подписи НДС во фронте перестают отрицать коммерцию (#2753)

bot-backend referenced this issue from a pull request that will close it,

2026-08-07 09:46:04 +00:00

fix(ptica): скраб ПДн перестаёт утекать то, что защищает + проводка проверяется поведением (#2753) #2787

bot-backend commented

2026-08-07 09:46:38 +00:00

Collaborator

Взял в работу, PR #2787 — все четыре пункта.

Две поправки к постановке, обе проверены на актуальном коде:

п.3 сформулирован сильнее, чем есть. Комментарии в main.py/celery_app.py сегодня содержат before_send, before_send_transaction и scrub_event по отдельности, но НЕ искомую подстроку before_send=scrub_event целиком (со знаком равенства). Проверено исполнением: удалить аргументы, оставив комментарии как есть → старый гейт КРАСНЕЕТ. Зелёным на разорванной проводке он становится только если комментарий переформулировать, назвав в нём аргумент вместе со знаком равенства — это естественная формулировка, так что дыра реальна, но не открыта прямо сейчас. Настоящий довод против грепа другой и он сильнее: подстрока ничего не говорит о том, дошли ли ПДн до транспорта. Они доходили — через локальные переменные, при живой проводке и зелёном гейте.
п.4 — три места, а не два. Кроме 528-529 и 659 та же сноска содержала «Коммерческие и офисные площади не учитываются», хотя двадцатью строками выше сам компонент рисует «Выручка — нежилое (1-й этаж)» из revenue_office_rub. Это утверждение сильнее двух названных; без него правка была бы самопротиворечивой в пределах одного абзаца. Образец из тела задачи (MassingEconomics.tsx:175) больше не существует — файл удалён в #2747.

Рекурсия из п.1 проверена не рассуждением, а исполнением: на коде до фикса процесс не завершается, 1000+ вложенных трассировок за минуту.

Взял в работу, PR #2787 — все четыре пункта. Две поправки к постановке, обе проверены на актуальном коде: 1. **п.3 сформулирован сильнее, чем есть.** Комментарии в `main.py`/`celery_app.py` сегодня содержат `before_send`, `before_send_transaction` и `scrub_event` по отдельности, но НЕ искомую подстроку `before_send=scrub_event` целиком (со знаком равенства). Проверено исполнением: удалить аргументы, оставив комментарии как есть → старый гейт КРАСНЕЕТ. Зелёным на разорванной проводке он становится только если комментарий переформулировать, назвав в нём аргумент вместе со знаком равенства — это естественная формулировка, так что дыра реальна, но не открыта прямо сейчас. Настоящий довод против грепа другой и он сильнее: подстрока ничего не говорит о том, дошли ли ПДн до транспорта. Они доходили — через локальные переменные, при живой проводке и зелёном гейте. 2. **п.4 — три места, а не два.** Кроме 528-529 и 659 та же сноска содержала «Коммерческие и офисные площади не учитываются», хотя двадцатью строками выше сам компонент рисует «Выручка — нежилое (1-й этаж)» из `revenue_office_rub`. Это утверждение сильнее двух названных; без него правка была бы самопротиворечивой в пределах одного абзаца. Образец из тела задачи (`MassingEconomics.tsx:175`) больше не существует — файл удалён в #2747. Рекурсия из п.1 проверена не рассуждением, а исполнением: на коде до фикса процесс не завершается, 1000+ вложенных трассировок за минуту.

bot-backend closed this issue

2026-08-07 10:11:38 +00:00

bot-backend referenced this issue from a commit

2026-08-07 10:11:38 +00:00

fix(ptica): скраб ПДн перестаёт утекать то, что защищает + проводка проверяется поведением (#2753) (#2787)