fix(stability): стадия 4 — гигиена event loop (рендер фида в поток, ленивый raw_message, один sanitize на выход) #12
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/pyrogram-bridge#12
Reference in New Issue
Block a user
Delete Branch "fix/stage-4-eventloop"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Стадия 4 плана — гигиена event loop при генерации фидов. closes #4
Генерация фида блокировала луп (deepcopy сотен Message + bleach ×4 на сообщение +
str(message)на пост). Выносим CPU-работу с лупа и сокращаем её.raw_messageлениво:process_message(include_raw=False)вообще не считаетstr(message); только/jsonи debug-HTML._save_media_file_idsне трогает asyncio/БД — складывает вPostParser._pending_media_ids(инстанс на запрос), вызывающий флашит один раз черезto_thread(upsert_media_file_ids_bulk_sync)(новаяexecutemany-функция в file_io.py). Удалены_persist_pending_count+ create_task-механика. Обязательно ДО 4.3, иначеget_running_loop()внутри render-треда бросил бы и молча убил персистентность media-id.to_thread(_render_pipeline)из обоих генераторов; deepcopy уехал в тред. Render-путь подтверждён свободным от asyncio-примитивов./html+/json— body+footer один раз в process_message; debug-<pre>—html.escape. media встраивается в body, reactions в footer → оба под покрывающим проходом.How verified
Venv под requirements:
python -m pytest tests/→ 230 passed (214 baseline + 16 новых)./html→ process_message body+footer;/json→ body+footer. Ни один фрагмент не достигает клиента без ровно одного прохода.<script>/onerror=/javascript:): вычищено во всех 4 выходах — вкл. НОВЫЕ тесты с payload в media-caption (тот фрагмент, у которого убрали внутренний проход). Адверсариально проверил: занулил санитайзер → media-caption XSS-тесты ПАДАЮТ, значит реально ловят дыру.create_task/get_running_loop/asyncio.— чисто.Внутренний цикл: 1 проход ревью (APPROVE with suggestions, критических XSS-дыр нет — ревьюер независимо оттрассировал карту; asyncio из треда убран полностью; bulk-SQL байт-в-байт со старым). По замечаниям: задокументировал смену входа извлечения флагов на pre-sanitize body (следствие 4.4, non-security, легитимные ссылки не задеты); добавил media-caption XSS-тесты; flush media-id теперь в
finally(частичный рендер всё равно персистит собранное).База / стек
PR в
fix/stage-3-fileresponse. Ретаргечу по мере мержа предыдущих стадий.⚠️ DoD «/ping во время генерации < 100мс» зависит от
/pingстадии 6 — здесь не проверяем. Деплой/наблюдение (падениеdiag_sanitize_slow) — стадия 7.Checklist
Feed generation blocked the loop (deepcopy of hundreds of Messages + per-message bleach x4 + str(message) per post). This moves the CPU work off the loop and cuts it down. 4.1 raw_message is lazy: process_message(include_raw=False) omits str(message) entirely; only /json and debug-HTML compute it. 4.2 (done BEFORE 4.3) _save_media_file_ids no longer touches asyncio/DB — it appends to the per-request PostParser._pending_media_ids, and the caller flushes once via to_thread(upsert_media_file_ids_bulk_sync) (a new executemany fn in file_io.py). Removed _persist_pending_count + the create_task machinery. This had to precede 4.3 or get_running_loop() inside the render thread would raise and silently kill media-id persistence. 4.3 The render pipeline (_create_time_based_media_groups, _create_messages_groups, _trim_messages_groups, _render_messages_groups) is now plain-sync and runs in ONE asyncio.to_thread(_render_pipeline) from both feed generators; deepcopy moved into the thread. The render path is verified free of asyncio primitives. 4.4 Sanitize is now ONE pass per output boundary (removed the 4 internal per-fragment bleach passes): RSS/HTML feeds sanitize once at the whole-feed pass; /html and /json sanitize body+footer once in process_message; the debug <pre> raw dump is html.escape'd. Media embeds in body, reactions in footer, so both are covered by the boundary pass. XSS tests (<script>/onerror=/javascript:) confirm all four outputs are clean. Review round-1: documented that flags are now extracted from the pre-sanitize body (a 4.4 consequence — non-security, legitimate links unaffected); added media-caption XSS tests for the exact fragments whose internal passes were removed (adversarially validated: neutering the sanitizer makes them fail); the media-id flush is now in a finally so a partial render still persists what it collected. 230 passed (214 baseline + 16). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Открыл PR по стадии 4 (гигиена event loop — самая рисковая: XSS-санитайз + thread-рефактор). Внутренний цикл: 1 проход, ревьюер НЕЗАВИСИМО оттрассировал карту санитайза (media→body, reactions→footer, все 4 выхода ровно один проход, ноль — нигде), asyncio из render-треда убран полностью (grep чист), bulk-SQL байт-в-байт. По замечаниям добавил media-caption XSS-тесты (адверсариально проверил: занулил санитайзер → падают) + flush в finally + задокументировал смену входа флагов. 230 passed. Порядок 4.2-до-4.3 соблюдён. Стек: база fix/stage-3-fileresponse.
Ревью — #12 (fix(stability): стадия 4 — гигиена event loop (рендер фида в поток, ленивый raw_message, один sanitize на выход)), round 1. Вердикт: CHANGES
Сильная, хорошо структурированная работа — веер 9 аспектов + мои проверки. Ядро безопасности ГЕНУИННО ПОДТВЕРЖДЕНО адверсариально (не по прозе): карта sanitize-once держится на happy-path — security занулил ОБА санитайзера и payload (
<script>/onerror=/javascript:в тексте, media-caption И реакции) выжил во всех 4 выходах, с реальным санитайзером — вычищен везде; whitelist покрывающего прохода не шире удалённого (даже строже — фид-проход роняетs/del); title/description XML-эскейпит feedgen; debug-<pre>теперьhtml.escape(фикс). test-coverage мутационно подтвердил, что 7 XSS-тестов РЕАЛЬНО кусают (занулил санитайзер → все 7 падают). Render-путь асинхронно-чист (reachability-проверено: asyncio только вget_post/_flush, не на тред-пути);PostParserper-request (нет гонки_pending_media_ids); bulk-upsert байт-в-байт с single (executemany, дубликаты в батче безопасны). 4.2→4.3 порядок соблюдён, старая create_task-механика удалена полностью.НО холистический взгляд (coherence) + regressions НЕЗАВИСИМО поймали реальную регрессию безопасности, которую по-аспектный happy-path-взгляд пропускает: fail-OPEN на except-ветке единственного оставшегося санитайза (я подтвердил по коду). Это блокер.
Объективка ЗЕЛЁНАЯ — ВОСПРОИЗВЁЛ сам (venv,
pytest tests/→ 230 passed = 214+16). Гейт зелён, но fail-open — дефект пути в except-ветке, которую гейт не прогоняет; отсюда CHANGES по security-DO, а не по красному тесту. Эскалаций нет (фикс ограниченный и очевидный → DO).📋 Do (1 security + 4 low) + ⛔ DROP + Вне scope + что сверено
Do — apply these, then re-review
[security · high] Fail-CLOSED на except-ветках финального sanitize фидов —
rss_generator.py:489-491(RSS) и:694-696(HTML).ДО 4.4 эти fail-open ветки были безопасны: фрагменты уже были просанитайзены per-fragment внутри
process_message, так что даже при исключении bleach выход был чист. 4.4 убрал внутренние проходы → на фид-путиprocess_message(sanitize=False), поэтомуpost['html']/ конкатенированныйhtmlтеперь СЫРОЙ, channel-controlled HTML/JS. Единственный оставшийся защитный проход обёрнут вtry/except, который на сбое отдаёт СЫРОЙ payload: RSSsanitized_html = post['html']→fe.content(CDATA); HTMLexcept:оставляетhtml= сырая конкатенация → возвращается. bleach/html5lib МОГУТ бросить (RecursionError/OOM на глубоко-вложенном или патологическом HTML из канала — а в этом проекте класс bleach/regex-крашей уже наблюдался) → сейчас это отдаёт НЕсанитайзенный XSS в фид. Автор сам добавил except, т.е. сбой bleach ожидаем — значит ветка достижима. Fix: fail closed — на исключении дропнуть/пропустить пост, либо подставитьhtml.escape()/пусто, либо повторить per-post_sanitize_htmlв except; НЕ отдавать сырой фрагмент. (Single-post_sanitize_html-fallback — пре-существующий, не менялся; вне этого DO, но подумай о том же харденинге.)[test-coverage · low] Покрыть новый bulk-SQL —
file_io.py:72(upsert_media_file_ids_bulk_sync). Все тесты мокают её заглушкой → реальныйexecutemany/empty-guard/ON CONFLICT(...) DO UPDATE SET added=excluded.addedне исполняется ни одним тестом (это ядро новой персистентности стадии-4). Near-verbatim с проверенной single + ON CONFLICT совпадает с PRIMARY KEY, так что риск низкий, но битый conflict-clause/порядок колонок уедет молча. Fix: прямой тест на temp-БД — empty no-op, multi-row insert, re-upsert того же ключа обновляетadded.[test-coverage · low] Покрыть инвариант finally-flush при частичном рендере —
rss_generator.py:442и:659. PR явно опирается на «flush в finally, чтобы частичный рендер всё равно сохранил собранное», но ни один тест этого не стережёт (сними finally — никто не заметит). Fix: замокать_render_pipelineтак, чтобы он добавил pending-id и бросил; ассертить, что bulk-upsert-заглушка всё равно получила батч, а_pending_media_idsочищен.[simplification+conventions · low] Мёртвый импорт —
rss_generator.py:26(from file_io import upsert_media_file_ids_bulk_sync, DB_PATH). Оба имени не используются в файле (флаш делегированpost_parser._flush_pending_media_ids, у него свой импорт). Введён этим PR. Fix: убрать строку 26.[documentation · low] Неточность в sanitize-map комментариях —
post_parser.py:715,797,879,967. Пишут «final whole-feed pass in rss_generator», но RSS санитайзит per-post (:488), whole-feed — только HTML (:694). Инвариант («каждый фрагмент ровно один раз») держится — это лишь неточность формулировки в security-map комментарии. Fix: «final per-post/whole-feed pass in rss_generator».⛔ DROP — кодеру НЕ делать · калибровочный лог (оператору)
[below-threshold]low[coherence] Мёртвый код: singleupsert_media_file_id_sync(file_io.py:60) после перехода на bulk имеет ноль вызовов (сверил grep'ом). Безвредно; удаление опционально.[below-threshold]low[stability] Устаревшие докстрингиgenerate_channel_rss/generate_channel_html(rss_generator.py:358,584) упоминают параметрpost_parser=, которого в сигнатуре нет. Косметика.[below-threshold]low[regressions] Single-postget_post-flush (post_parser.py:118) не вfinally: еслиprocess_messageбросит на середине, собранный id теряется — но одиночный рендер при сбое и так не даёт выхода. Пренебрежимо.[by-design]info[regressions] Флаги теперь из PRE-sanitize body — non-security, дивергенция лишь для URL-подобного текста, который bleach бы срезал в запрещённом атрибуте (наш генератор такого не эмитит); задокументировано, принято.[speculative]info[regressions] Байт-эквивалентность sanitize(concat) vs concat(sanitize) — подтверждено идентично для реального контента (bleach идемпотентен), дивергенция требует адверсариального ввода; принято.Вне scope — отдельная задача (НЕ часть этого вердикта)
[security bug · low]Пре-существующий reflected-XSS: несанитайзенныйtitleв debug-<div>—post_parser.py:489, рендерится ТОЛЬКО при?debug=true, эндпоинт/post/htmlза токеном (api_server.py:924). Security сверил — идентично в базе (origin/fix/stage-3-fileresponse), стадия-4 НЕ трогала эту строку и НЕ убирала санитайз title. Низкая severity (debug+token-gated). Fix:html.escape(data["html"]["title"]), как стадия-4 сделала с raw_message. Завёл отдельным issue.Сверено (9 аспектов, голова
9f9091f6, дифф three-dot от merge-base1e24fd53=утв. голова стадии-3): objective gate ВОСПРОИЗВЁЛ сам (230 passed); карта sanitize адверсариально доказана (занулены оба санитайзера → payload выживает везде; реальный → вычищен); XSS-тесты мутационно кусают (7/7 падают при зануленном санитайзере); render-путь asyncio-чист (reachability); PostParser per-request; bulk-upsert байт-в-байт + дубликаты-в-батче безопасны; 4.2→4.3 порядок и полнота (нет остаточного get_running_loop/create_task); флаги-из-pre-sanitize non-security; feedgen XML-эскейпит title/description. Единственный блокер — fail-open except-ветки фид-санитайза (coherence+regressions независимо, я подтвердил). Эскалаций нет.Round 1 — все закрыты. Отличный улов по #1: холистический взгляд поймал реальную дыру, которую happy-path-мутация пропускала.
#1 [security high] Fail CLOSED на except-ветках финального sanitize. Ты прав — 4.4 убрал внутренние проходы, поэтому на фид-пути
post['html']теперь сырой channel-controlled HTML, а единственный оставшийся проход был обёрнут в fail-open except (отдавал сырой payload при сбое bleach; RecursionError/OOM на патологическом HTML достижим). Обе ветки (RSS per-post, HTML whole-feed) теперь fail closed черезhtml.escape— контент выживает как инертный текст, живой тег не доходит до клиента. Тест: форсирую RecursionError в bleach, ассертю что ни<script/<img/<aне в фиде, а payload присутствует только в<script>-форме. Адверсариально проверил: откатил на fail-open → тест ПАДАЕТ.#2 [test] Прямой тест
upsert_media_file_ids_bulk_syncна temp-БД: empty no-op, multi-row, re-upsert обновляетadded(реальный executemany + ON CONFLICT, раньше только мок).#3 [test] Тест что media id, собранные до исключения рендера, всё равно флашатся (flush в finally) — снятие finally красит тест.
#4 [cleanup] Убрал мёртвый импорт
from file_io import upsert_media_file_ids_bulk_sync, DB_PATHв rss_generator (флаш делегирован post_parser). (В коммит-мессадж эта строка попала покорёженной — бэктик отработал шелл-подстановкой; в диффе всё чисто.)#5 [doc] Поправил sanitize-map комментарии: RSS санитайзит per-post, whole-feed — только HTML.
Вне scope (debug title reflected-XSS) — согласен, пре-существует, заведи отдельным issue (или я заведу, скажи).
Прогон: 233 passed (214 baseline + 19). Внутренний цикл: 1 проход + адверсариальная валидация fail-closed-теста.
Ревью — #12 (fix(stability): стадия 4 — гигиена event loop), round 2. Вердикт: PASS ✅
Блокер round-1 (security fail-open) ГЕНУИННО ЗАКРЫТ И АДВЕРСАРИАЛЬНО ПОДТВЕРЖДЁН на обоих sink'ах, все 5 пунктов разобраны:
html_escape(post['html'])(RSSrss_generator.py:496) иhtml_escape(html)(HTML:703), а не сырой payload. Адверсариально проверено (занулил санитайзер вraise RecursionError, прогнал<script>/<img onerror>/javascript:через реальное сообщение канала в оба генератора): RSS-CDATA — контент выходит&lt;script&gt;…, ноль живых тегов, и]]>-breakout невозможен (html.escapeпревращает каждый>→>, терминатор CDATA из ввода не собрать); HTML-фид — ноль символов<→ живого тега нет. Fix не может бросить второе исключение (post['html']/htmlвсегда str черезjoin; dict-форма'html'сюда не доходит). Кодер добавил кусающий тестtest_rss_fails_closed_when_sanitizer_raises(реверт фикса → красный; sanitize-error реально достигается).test_bulk_upsert_media_file_ids_real_sqlгоняет РЕАЛЬНЫЙexecutemany+ON CONFLICT DO UPDATEна temp-sqlite (empty/multi-row/conflict-update); мутация clause→DO NOTHINGроняет тест.test_pending_media_flushed_on_render_exceptionчерез реальныйgenerate_channel_rss(рендер бросает →finallyвсё равно флашит батч); снятие finally роняет тест.from file_io import upsert_media_file_ids_bulk_sync, DB_PATHубран из rss_generator.py (ссылок нет, модуль импортится чисто).Объективка ЗЕЛЁНАЯ — ВОСПРОИЗВЁЛ сам (venv):
pytest tests/→ 233 passed (230 + 3 новых), stage4 19 passed. 3 новых теста мутационно кусают (проверено реальными реверт-прогонами). Регрессий нет.📋 Что сверено + ⛔ DROP (мелкие хвосты — кодеру НЕ обязательно)
Сверено (голова
8d213902, дельта round-1→29f9091f6..8d21390294): objective gate ВОСПРОИЗВЁЛ сам (233 passed); fail-closed адверсариально подтверждён на обоих sink'ах (форсированный bleach-краш → выход только escaped, ни одного живого тега, CDATA-breakout невозможен, второе исключение невозможно); 3 новых теста мутационно кусают (реверт фикса/finally/ON-CONFLICT → красный); dead import убран без остаточных ссылок; doc-формулировка поправлена в 3/4 местах. Эскалаций нет.⛔ DROP — кодеру НЕ делать · калибровочный лог (оператору)
[below-threshold]low[documentation] Осталась старая формулировка «sanitize deferred to the final whole-feed pass, so each fragment is sanitized exactly once» в общем render-коде —rss_generator.py:224-226(_render_messages_groups). Та же неточность, что #5 поправил в post_parser (RSS — per-post, не whole-feed); инвариант держится, это лишь текст. Можно снять попутно; не блокер (третий round-trip ради одной строки комментария не оправдан).[below-threshold]low[test-coverage] HTML-фид fail-closed ветка (rss_generator.py:700-705) без отдельного теста — симметрична покрытой (и мутационно-проверенной) RSS-ветке, и security адверсариально подтвердил её корректность руками. Опц.: твинtest_rss_fails_closed_when_sanitizer_raisesдляgenerate_channel_html. Регрессия-страховка на маловероятную будущую расхождение, не блокер.