refactor(render): #28 этап 1 ревью раунд 1 — тест изоляции §3.4 + актуализация комментов
Docker Image CI / build (pull_request) Has been cancelled
Docker Image CI / build (pull_request) Has been cancelled
Do 1 [test-coverage]: залочил security-инвариант §3.4 (per-post изоляция фрагмента) именованным тестом test_unbalanced_post_does_not_swallow_next_post_html_feed (в test_stage4_eventloop.py — гоняет реальный per-post HTML-пайплайн на истории из 2 сообщений). Пост A несёт висячий `<div><b>DANGLING_A` без закрывашек, пост B целый с маркером. Ассертит: фид бьётся на 2 фрагмента по `<hr post-divider>`; теги A сбалансированы В ПРЕДЕЛАХ фрагмента A; маркер B в фрагменте B и НЕ в A (не заглочен). Проверено, что тест краснеет на регрессии «join-before-sanitize»: при склейке raw-постов до единого whole-feed bleach.clean невайтлистнутый `<hr>` стрипается (strip=True) → 0 разделителей → 1 фрагмент → первый ассерт падает. То есть будущий этап, вернувший склейку до санитайза, ловится этим тестом. Do 2 [documentation]: 4 устаревших коммента про удалённый «whole-feed sanitize для HTML» обновлены на «per-post в _render_pipeline для обоих (RSS и HTML)» (post_parser.py process_message docstring / _generate_html_body NOTE / generate_html_footer coverage-map; rss_generator.py feed-path коммент). Вывод «санитайзится ровно один раз» сохранён. Исполняемый код не менялся. Полный сюит 347 passed (+1 тест изоляции); оракул этапа 0 — 11 passed, golden не менялся. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+7
-6
@@ -637,10 +637,10 @@ class PostParser:
|
||||
serializing every post.
|
||||
sanitize: when True, run the html body and footer through a single
|
||||
bleach pass each. Single-post HTML and JSON need this (there is no
|
||||
whole-feed pass on those paths). Feed generation passes False and
|
||||
relies on the final sanitize in rss_generator (per-post for RSS,
|
||||
whole-feed for HTML), so no
|
||||
fragment is sanitized more than once.
|
||||
feed-level pass on those paths). Feed generation passes False and
|
||||
relies on the per-post sanitize in rss_generator._render_pipeline
|
||||
(per-post for BOTH RSS and HTML), so no fragment is sanitized more
|
||||
than once.
|
||||
"""
|
||||
# Compute html body once — avoids triple _generate_html_body calls.
|
||||
# The internal per-fragment sanitize passes were removed (4.4); sanitize
|
||||
@@ -799,7 +799,7 @@ class PostParser:
|
||||
|
||||
# NOTE: sanitize is NOT applied here. Sanitization happens exactly once per
|
||||
# output boundary (process_message for single-post/JSON; in rss_generator the
|
||||
# per-post pass for RSS and the whole-feed pass for HTML). See the map (4.4).
|
||||
# per-post pass in _render_pipeline for BOTH RSS and HTML). See the map (4.4).
|
||||
html_body = '\n'.join(content_body)
|
||||
return html_body
|
||||
|
||||
@@ -1004,7 +1004,8 @@ class PostParser:
|
||||
content_footer.append('<br>' + flags_html)
|
||||
|
||||
# Not sanitized here — sanitized once at the output boundary (4.4 coverage map):
|
||||
# process_message for single-post/JSON; per-post (RSS) / whole-feed (HTML) for feeds.
|
||||
# process_message for single-post/JSON; per-post in _render_pipeline for feeds
|
||||
# (both RSS and HTML).
|
||||
html_footer = '\n'.join(content_footer)
|
||||
return html_footer
|
||||
|
||||
|
||||
+3
-2
@@ -220,8 +220,9 @@ def _render_messages_groups(messages_groups: list[list[Message]],
|
||||
try:
|
||||
if len(group) == 1: # Single message - simple case
|
||||
one_message = group[0]
|
||||
# Feed path: raw_message not needed and sanitize deferred to the final
|
||||
# whole-feed pass, so each fragment is sanitized exactly once.
|
||||
# Feed path: raw_message not needed and sanitize deferred to the per-post
|
||||
# pass in _render_pipeline (both RSS and HTML), so each fragment is
|
||||
# sanitized exactly once.
|
||||
message_data = post_parser.process_message(one_message, include_raw=False, sanitize=False)
|
||||
html_parts = [
|
||||
f'<div class="message-body">{message_data["html"]["body"]}</div>',
|
||||
|
||||
@@ -383,6 +383,56 @@ async def test_xss_in_media_caption_stripped_in_feeds(monkeypatch):
|
||||
_assert_clean(html_feed, "html feed (media caption)")
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# §3.4 — per-post sanitize ISOLATION (stage-1 load-bearing invariant, PR #36).
|
||||
# A dangling/unbalanced tag in post A must be normalized WITHIN A's own fragment and
|
||||
# cannot swallow post B (cross-post DOM/XSS bleed). This holds because _render_pipeline
|
||||
# runs a SEPARATE bleach.clean per post and the formatter joins the <hr> divider AFTER
|
||||
# sanitize. A FUTURE stage that rejoins BEFORE sanitizing would reintroduce the bleed
|
||||
# and pass every single-message XSS test — so this 2-post case MUST turn it red.
|
||||
# --------------------------------------------------------------------------- #
|
||||
@pytest.mark.asyncio
|
||||
async def test_unbalanced_post_does_not_swallow_next_post_html_feed(monkeypatch):
|
||||
# Post A carries a dangling <div><b> with NO closers; post B is well-formed and
|
||||
# carries a unique marker. _Str.html feeds the raw tags into the pre-sanitize body
|
||||
# exactly as a real message with entities would.
|
||||
msg_a = make_message(50, text="<div><b>DANGLING_A no closers here",
|
||||
date=datetime(2024, 1, 1, 12, 5, 0, tzinfo=timezone.utc))
|
||||
msg_b = make_message(51, text="INTACT_B_MARKER",
|
||||
date=datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc))
|
||||
|
||||
async def fake_get_chat(client, channel):
|
||||
return SimpleNamespace(title="Test", username="testchan", id=-1001234567890)
|
||||
|
||||
async def fake_get_history(client, channel, limit=20):
|
||||
return [msg_a, msg_b] # A is newer than B -> A rendered first
|
||||
|
||||
monkeypatch.setattr("tg_cache.cached_get_chat", fake_get_chat, raising=False)
|
||||
monkeypatch.setattr("tg_cache.cached_get_chat_history", fake_get_history, raising=False)
|
||||
|
||||
html = await generate_channel_html("testchan", client=SimpleNamespace(), limit=5)
|
||||
|
||||
# The divider survives (joined AFTER per-post sanitize) and splits the feed into
|
||||
# exactly two TOP-LEVEL fragments. A join-before-sanitize regression strips the
|
||||
# non-whitelisted <hr> entirely (strip=True) -> this alone already goes red.
|
||||
parts = html.split('<hr class="post-divider">')
|
||||
assert len(parts) == 2, "expected exactly one top-level post-divider between the two posts"
|
||||
frag_a, frag_b = parts
|
||||
|
||||
# A's dangling tags were balanced WITHIN A's own fragment, NOT deferred past the
|
||||
# divider. A rejoin-before-sanitize regression closes them only at the very end of
|
||||
# the whole feed (after B), leaving frag_a with more opens than closes.
|
||||
assert "DANGLING_A" in frag_a
|
||||
assert frag_a.count("<div") == frag_a.count("</div>"), "post A's <div> not closed within its own fragment"
|
||||
assert frag_a.count("<b>") == frag_a.count("</b>"), "post A's <b> not closed within its own fragment"
|
||||
|
||||
# B is intact, lives at top level, and is itself balanced — never nested inside A
|
||||
# (its marker must not have been trapped before A's divider).
|
||||
assert "INTACT_B_MARKER" in frag_b, "post B content missing/swallowed by post A"
|
||||
assert "INTACT_B_MARKER" not in frag_a, "post B content bled into post A's fragment"
|
||||
assert frag_b.count("<div") == frag_b.count("</div>"), "post B fragment not self-contained"
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Review round-1: the new bulk-upsert SQL executed for real (not mocked).
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
Reference in New Issue
Block a user