fix(stability): fail CLOSED on feed-sanitize exception + cover bulk-SQL/flush (review round 1)

#1 [security] The final feed-sanitize try/except was FAIL-OPEN: since 4.4 removed
the per-fragment passes, post['html'] / the concatenated feed html is now raw
channel-controlled HTML, and this is its ONLY sanitize. If bleach itself throws
(RecursionError/OOM on pathological nested HTML — a class already seen in this
project), the except branch returned the RAW payload = stored XSS. Both branches
(RSS per-post, HTML whole-feed) now fail CLOSED via html.escape, so the content
survives as inert text and no live tag ever reaches the client. Added a test that
forces bleach to raise and asserts no live <script>/<img>/<a> reaches the feed
(adversarially validated: reverting to fail-open makes it fail).

#2 [test] Direct test of the new upsert_media_file_ids_bulk_sync on a temp DB:
empty no-op, multi-row insert, and re-upsert-updates-added (the real executemany +
ON CONFLICT, previously only mocked).

#3 [test] Test that media ids collected before a render exception are still
flushed (the flush is in a finally) — removing the finally now turns a test red.

#4 [cleanup] Removed the dead  in rss_generator (the
flush is delegated to post_parser._flush_pending_media_ids).

#5 [doc] Corrected the sanitize-map comments: RSS sanitizes per-post, only HTML is
the whole-feed pass.

233 passed (214 baseline + 19).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

post_parser.py

@@ -526,7 +526,8 @@ class PostParser:
             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 whole-feed sanitize in rss_generator, so no
+                relies on the final sanitize in rss_generator (per-post for RSS,
+                whole-feed for HTML), so no
                 fragment is sanitized more than once.
         """
         # Compute html body once — avoids triple _generate_html_body calls.
@@ -715,8 +716,8 @@ class PostParser:
         content_body.append(f'</div><br>')
 
         # NOTE: sanitize is NOT applied here. Sanitization happens exactly once per
-        # output boundary (process_message for single-post/JSON, final whole-feed pass
-        # in rss_generator for feeds). See the sanitize coverage map (4.4).
+        # 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).
         html_body = '\n'.join(content_body)
         return html_body
 
@@ -879,7 +880,7 @@ 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, final whole-feed pass for feeds.
+        # process_message for single-post/JSON; per-post (RSS) / whole-feed (HTML) for feeds.
         html_footer = '\n'.join(content_footer)
         return html_footer
 

rss_generator.py

@@ -14,6 +14,7 @@ import logging
 import asyncio
 import re
 import time
+from html import escape as html_escape
 from datetime import datetime, timezone
 from types import SimpleNamespace
 from typing import Optional
@@ -23,7 +24,6 @@ from pyrogram.types import Message
 from post_parser import PostParser
 from config import get_settings
 from tg_throttle import tg_rpc_bounded
-from file_io import upsert_media_file_ids_bulk_sync, DB_PATH
 from bleach.css_sanitizer import CSSSanitizer
 from bleach import clean as HTMLSanitizer
 
@@ -487,8 +487,13 @@ async def generate_channel_rss(channel: str | int,
                     )
                 sanitized_html = await asyncio.to_thread(_sanitize_sync, post['html'])
             except Exception as e:
+                # FAIL CLOSED: since 4.4 removed the per-fragment passes, post['html'] is
+                # raw channel-controlled HTML, and this is its ONLY sanitize. If bleach
+                # itself throws (e.g. RecursionError on pathological nested HTML), do NOT
+                # emit the raw payload (that would be stored XSS) — html.escape it so the
+                # content survives as inert text.
                 logger.error(f"rss_html_sanitization_error: channel {channel}, message_id {post['message_id']}, error {str(e)}")
-                sanitized_html = post['html']
+                sanitized_html = html_escape(post['html'])
             fe.content(content=sanitized_html, type='CDATA')
             
             if post['date'] is not None:
@@ -693,7 +698,11 @@ async def generate_channel_html(channel: str | int,
                 )
             html = await asyncio.to_thread(_sanitize_sync, html)
         except Exception as e:
+            # FAIL CLOSED (see the RSS path): `html` is now the raw concatenated feed
+            # (4.4 removed the per-fragment passes). If bleach throws, html.escape the
+            # whole feed rather than returning raw channel HTML/JS to the client.
             logger.error(f"html_final_sanitization_error: channel {channel}, error {str(e)}")
+            html = html_escape(html)
         
         html_gen_elapsed = time.time() - html_gen_start_time
         logger.debug(f"html_generation_timing: channel {channel}, HTML generated in {html_gen_elapsed:.3f} seconds")

tests/test_stage4_eventloop.py

@@ -365,3 +365,107 @@ async def test_xss_in_media_caption_stripped_in_feeds(monkeypatch):
     monkeypatch.setattr("tg_cache.cached_get_chat_history", fake_get_history2, raising=False)
     html_feed = await generate_channel_html("testchan", client=SimpleNamespace(), limit=5)
     _assert_clean(html_feed, "html feed (media caption)")
+
+
+# --------------------------------------------------------------------------- #
+# Review round-1: the new bulk-upsert SQL executed for real (not mocked).
+# --------------------------------------------------------------------------- #
+def test_bulk_upsert_media_file_ids_real_sql(tmp_path):
+    import sqlite3
+    from file_io import upsert_media_file_ids_bulk_sync, init_db_sync
+
+    db = str(tmp_path / "t.db")
+    init_db_sync(db)
+
+    # Empty list is a no-op (no crash).
+    upsert_media_file_ids_bulk_sync(db, [])
+
+    # Multi-row insert.
+    upsert_media_file_ids_bulk_sync(db, [
+        ("chA", 1, "fidA", 100.0),
+        ("chB", 2, "fidB", 200.0),
+    ])
+    conn = sqlite3.connect(db)
+    rows = dict(((c, p, f), a) for c, p, f, a in
+                conn.execute("SELECT channel, post_id, file_unique_id, added FROM media_file_ids"))
+    assert rows[("chA", 1, "fidA")] == 100.0
+    assert rows[("chB", 2, "fidB")] == 200.0
+
+    # Re-upsert the SAME key updates `added` (ON CONFLICT ... DO UPDATE SET added=excluded.added).
+    upsert_media_file_ids_bulk_sync(db, [("chA", 1, "fidA", 999.0)])
+    a = conn.execute(
+        "SELECT added FROM media_file_ids WHERE channel='chA' AND post_id=1 AND file_unique_id='fidA'"
+    ).fetchone()[0]
+    assert a == 999.0
+    conn.close()
+
+
+# --------------------------------------------------------------------------- #
+# Review round-1: media ids collected before a render exception are still
+# flushed (the flush is in a finally). Removing the finally must break this.
+# --------------------------------------------------------------------------- #
+@pytest.mark.asyncio
+async def test_pending_media_flushed_on_render_exception(monkeypatch):
+    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 [make_message(40, text="hi")]
+
+    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)
+
+    # A render pipeline that collects a pending id then raises mid-render.
+    def boom_pipeline(messages, post_parser, *a, **k):
+        post_parser._pending_media_ids.append(("chZ", 9, "fidZ", 1.0))
+        raise RuntimeError("render blew up")
+
+    monkeypatch.setattr(rss_module, "_render_pipeline", boom_pipeline)
+
+    flushed = {}
+    async def fake_bulk(db, entries):
+        flushed["entries"] = list(entries)
+    monkeypatch.setattr("post_parser.upsert_media_file_ids_bulk_sync",
+                        lambda db, entries: flushed.__setitem__("entries", list(entries)),
+                        raising=False)
+
+    with pytest.raises(Exception):
+        await generate_channel_rss("testchan", client=SimpleNamespace(), limit=5)
+
+    # The collected id was persisted despite the render exception (flush in finally).
+    assert flushed.get("entries") == [("chZ", 9, "fidZ", 1.0)]
+
+
+# --------------------------------------------------------------------------- #
+# Review round-1 [security]: if the ONLY sanitize pass throws, the feed must
+# FAIL CLOSED (html.escape the raw content), never emit the raw XSS payload.
+# --------------------------------------------------------------------------- #
+@pytest.mark.asyncio
+async def test_rss_fails_closed_when_sanitizer_raises(monkeypatch):
+    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 [make_message(41, text=XSS_PAYLOAD)]
+
+    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)
+
+    # Force bleach to blow up (e.g. the RecursionError class already seen in prod).
+    # rss_generator imports it as `from bleach import clean as HTMLSanitizer`.
+    def boom(*a, **k):
+        raise RecursionError("bleach exploded")
+    monkeypatch.setattr(rss_module, "HTMLSanitizer", boom, raising=True)
+
+    rss = await generate_channel_rss("testchan", client=SimpleNamespace(), limit=5)
+    chunks = re.findall(r"<!\[CDATA\[(.*?)\]\]>", rss, re.DOTALL)
+    assert chunks, "expected CDATA content"
+    for chunk in chunks:
+        # Fail-closed: the raw payload was html.escaped, so NO live tag survived — every
+        # `<` became `&lt;`. (The letters "javascript:" still appear, but as inert text
+        # inside an escaped &quot;…&quot;, not a live href.)
+        assert "<script" not in chunk, "RSS fail-open: raw <script> reached the feed"
+        assert "<img" not in chunk, "RSS fail-open: raw <img onerror> reached the feed"
+        assert "<a " not in chunk, "RSS fail-open: raw <a href> reached the feed"
+        # ...and the escaping actually ran (payload present as escaped text, not dropped).
+        assert "&lt;script&gt;" in chunk, "expected the payload html-escaped, not dropped"