fix(channels): escape Slack reserved chars before mrkdwn conversion (#4197)

* fix(channels): escape &/</> in Slack outbound text before mrkdwn conversion

Slack requires callers to replace &, <, and > with their HTML entity
equivalents before sending message text, since an unescaped <...>
triggers Slack's own mention/link syntax (e.g. <@USERID>,
<http://url|label>). SlackChannel.send() ran msg.text through the
markdown-to-mrkdwn converter and sent the result straight to
chat_postMessage with no escaping step, so technical/code content like
"if a < b && b > c:" would arrive as literal Slack markup and render
broken or misinterpreted.

Escaping must happen before the mrkdwn conversion, not after: the
converter emits its own <url|label> syntax for real markdown links
([text](url)), and that generated syntax must reach Slack unescaped.
Escaping raw input first (via html.escape(text, quote=False), which
replaces & before </>) and leaving the converter's output alone
satisfies both requirements.

* fix(channels): preserve a line-leading > as Slack's blockquote marker

_escape_slack_text's html.escape(text, quote=False) replaces every ">"
with "&gt;", including a ">" at the start of a line -- Slack's own
mrkdwn blockquote marker, which the converter otherwise passes through
unchanged. Agent output using markdown blockquotes for quoting/callouts
lost its blockquote styling and arrived as a literal "&gt; " prefix
instead of a rendered blockquote.

Only "&" and "<" neutralize Slack's "<...>" mention/link syntax; ">" is
special to Slack only at the start of a line. Restore a line-leading
">" to a literal character after escaping (re.sub with MULTILINE), so
the mrkdwn converter still renders it as a blockquote; a "<"/"&"
anywhere, and a ">" that is not at a line start, still escape.

New tests cover the line-start preservation, that the exemption does
not widen into "never escape '>'" (a mid-line/non-leading ">" still
escapes), and that restoration applies per line in multiline text; all
three revert cleanly against the unconditional html.escape() to
reproduce the blockquote-corruption bug. Full test_channels.py (259
tests) plus ruff check/format are clean.

backend/app/channels/slack.py

@@ -3,7 +3,9 @@
 from __future__ import annotations
 
 import asyncio
+import html
 import logging
+import re
 from typing import Any
 
 from markdown_to_mrkdwn import SlackMarkdownConverter
@@ -18,6 +20,35 @@ logger = logging.getLogger(__name__)
 _slack_md_converter = SlackMarkdownConverter()
 
 
+def _escape_slack_text(text: str) -> str:
+    """Escape Slack's reserved characters (``&``, ``<``, ``>``) in raw message text,
+    except a ``>`` at the very start of a line -- Slack's own blockquote marker.
+
+    Slack requires callers to replace these with their HTML entity equivalents
+    (``&amp;``, ``&lt;``, ``&gt;``) before sending message text -- an unescaped
+    ``<...>`` triggers Slack's own mention/link syntax (e.g. ``<@USERID>``,
+    ``<http://url|label>``). See:
+    https://api.slack.com/reference/surfaces/formatting#escaping
+
+    This MUST run before ``_slack_md_converter.convert()``, not after: the
+    converter emits its own mrkdwn link syntax (``<url|label>``) for real
+    markdown links, and that generated syntax must reach Slack unescaped.
+    Escaping the raw input first -- and leaving the converter's own output
+    alone -- satisfies both requirements. ``html.escape(..., quote=False)``
+    replaces ``&`` before ``<``/``>``, so the entities it introduces are never
+    re-escaped.
+
+    Only ``&`` and ``<`` neutralize Slack's ``<...>`` mention/link syntax; a
+    ``>`` is special to Slack only at the start of a line, where the mrkdwn
+    converter passes it through unchanged as a blockquote marker. Escaping
+    every ``>`` would turn a quoted line into visible ``&gt;`` text instead of
+    a rendered blockquote, so a line-leading ``>`` is restored to a literal
+    ``>`` after escaping; a ``>`` anywhere else in the text still escapes.
+    """
+    escaped = html.escape(text, quote=False)
+    return re.sub(r"(?m)^&gt;", ">", escaped)
+
+
 def _normalize_allowed_users(allowed_users: Any) -> set[str]:
     if allowed_users is None:
         return set()
@@ -128,7 +159,7 @@ class SlackChannel(Channel):
 
         kwargs: dict[str, Any] = {
             "channel": msg.chat_id,
-            "text": _slack_md_converter.convert(msg.text),
+            "text": _slack_md_converter.convert(_escape_slack_text(msg.text)),
         }
         if msg.thread_ts:
             kwargs["thread_ts"] = msg.thread_ts

backend/tests/test_channels.py

@@ -7153,6 +7153,110 @@ class TestSlackMarkdownConversion:
         assert "*Title*" in result
         assert "#" not in result
 
+    def test_converter_passes_reserved_characters_through_unchanged(self):
+        # The library itself never escapes Slack's reserved characters -- this
+        # pins that assumption so SlackChannel.send() knows it must do so itself.
+        from app.channels.slack import _slack_md_converter
+
+        result = _slack_md_converter.convert("if a < b && b > c:")
+        assert result == "if a < b && b > c:"
+
+
+# ---------------------------------------------------------------------------
+# Slack outbound text escaping tests (Slack's &/</> HTML-entity requirement)
+#
+# Slack requires callers to replace &, <, and > with their HTML entity
+# equivalents before sending message text, because an unescaped `<...>`
+# triggers Slack's own mention/link syntax (e.g. `<@USERID>`,
+# `<http://url|label>`). See:
+# https://api.slack.com/reference/surfaces/formatting#escaping
+# ---------------------------------------------------------------------------
+
+
+class TestSlackTextEscaping:
+    @staticmethod
+    def _sent_text(text: str) -> str:
+        """Send *text* through SlackChannel.send() and return the resulting
+        Slack API ``text`` kwarg, without actually hitting the network."""
+        from app.channels.slack import SlackChannel
+
+        captured: dict[str, object] = {}
+
+        async def go():
+            bus = MessageBus()
+            ch = SlackChannel(bus=bus, config={"bot_token": "xoxb-test", "app_token": "xapp-test"})
+
+            mock_web = MagicMock()
+
+            def post_message(**kwargs):
+                captured.update(kwargs)
+                return MagicMock()
+
+            mock_web.chat_postMessage = post_message
+            ch._web_client = mock_web
+
+            msg = OutboundMessage(channel_name="slack", chat_id="C123", thread_id="t1", text=text)
+            await ch.send(msg)
+
+        _run(go())
+        return captured["text"]
+
+    def test_raw_angle_brackets_and_ampersand_are_escaped(self):
+        # Realistic technical/code content containing all three reserved
+        # characters must arrive escaped, so Slack renders it as literal text
+        # instead of attempting to parse a broken mention/link.
+        sent = self._sent_text("if a < b && b > c:")
+        assert sent == "if a &lt; b &amp;&amp; b &gt; c:"
+        assert "<" not in sent
+        assert ">" not in sent
+
+    def test_bot_mention_syntax_is_neutralized_not_interpreted(self):
+        # Raw text that happens to look like a mention must not survive as
+        # live `<@...>` syntax -- Slack would otherwise try to resolve it.
+        sent = self._sent_text("please ask <@U12345> for review")
+        assert sent == "please ask &lt;@U12345&gt; for review"
+
+    def test_real_markdown_link_still_converts_without_double_escaping(self):
+        # Critical non-regression case: escaping must run BEFORE mrkdwn
+        # conversion, not after. The converter's own generated `<url|label>`
+        # syntax for a real markdown link must survive untouched -- if
+        # escaping ran after conversion instead, this would corrupt into
+        # `&lt;url|label&gt;` and Slack would render a dead link.
+        sent = self._sent_text("See [DeerFlow docs](https://example.com/docs) for more.")
+        assert "<https://example.com/docs|DeerFlow docs>" in sent
+        assert "&lt;" not in sent
+        assert "&gt;" not in sent
+
+    def test_ampersand_in_link_url_is_escaped_before_conversion(self):
+        # & must be escaped first (before < and >) so it doesn't double-escape
+        # the &amp;/&lt;/&gt; entities being introduced, and a literal '&' in a
+        # URL must still come through as &amp; per Slack's escaping rule --
+        # even inside the converter's own generated <url|label> syntax.
+        sent = self._sent_text("[Search](https://example.com?a=1&b=2)")
+        assert "<https://example.com?a=1&amp;b=2|Search>" in sent
+
+    def test_blockquote_marker_at_line_start_is_preserved(self):
+        # A ">" at the very start of a line is Slack's own blockquote marker
+        # (the mrkdwn converter passes it through unchanged), not part of the
+        # <...> mention/link syntax that & and < neutralize. Escaping it would
+        # turn a quoted line into visible "&gt;" text instead of a rendered
+        # blockquote.
+        sent = self._sent_text("> quoted text")
+        assert sent == "> quoted text"
+
+    def test_blockquote_marker_exemption_is_line_start_only(self):
+        # The line-start exemption must not widen into "never escape '>'":
+        # a "<"/"&" anywhere, and a ">" that is NOT at the start of a line,
+        # still escape -- only the leading marker is restored.
+        sent = self._sent_text("> a < b & c > d")
+        assert sent == "> a &lt; b &amp; c &gt; d"
+
+    def test_blockquote_marker_restored_on_every_line(self):
+        # The restoration must apply per-line (re.MULTILINE), not just once
+        # at the start of the whole string.
+        sent = self._sent_text("intro\n> first quote\nmiddle\n> second quote")
+        assert sent == "intro\n> first quote\nmiddle\n> second quote"
+
 
 # ---------------------------------------------------------------------------
 # Telegram streaming tests