From a43ac2af65036092c3a44c8e8fadf6d492d4e753 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 22 Jul 2026 04:00:29 -0700 Subject: [PATCH] fix(slack): sanitize outbound Block Kit payloads at the API boundary Consolidated defensive layer for the invalid_blocks / msg_too_long bug class (#56615, #62054, #53693): one malformed or oversized block fails the ENTIRE chat.postMessage / chat.update call, so approval cards never update and messages silently drop. New block_kit.sanitize_blocks() is applied at every call site that attaches blocks (send/edit via _maybe_blocks, send_exec_approval, send_slash_confirm, and the approval/slash-confirm chat.update button handlers). It: - truncates section/context text to the 3000-char cap with an ellipsis (covers interaction payloads where Slack's HTML-escaping of < > & inflates text past the limit budgeted at send time) - truncates header text to its 150-char cap - drops empty blocks (no text / elements / rows) - replaces null table column_settings entries with {} and trims default trailing entries (Slack requires every entry be an object) - caps the payload at Slack's 50-block maximum - returns None when nothing valid remains so callers fall back to the plain-text payload; never raises Also registers contributor mappings for the salvaged PRs in this cluster (tw0316, kamonspecial, sowork-skills). --- contributors/emails/cjwang@sowork.tw | 2 + contributors/emails/kamon@gao-ai.com | 2 + contributors/emails/nwadwa@gmail.com | 2 + plugins/platforms/slack/adapter.py | 14 ++-- plugins/platforms/slack/block_kit.py | 115 +++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 7 deletions(-) create mode 100644 contributors/emails/cjwang@sowork.tw create mode 100644 contributors/emails/kamon@gao-ai.com create mode 100644 contributors/emails/nwadwa@gmail.com diff --git a/contributors/emails/cjwang@sowork.tw b/contributors/emails/cjwang@sowork.tw new file mode 100644 index 000000000000..4a6b262f7dfa --- /dev/null +++ b/contributors/emails/cjwang@sowork.tw @@ -0,0 +1,2 @@ +sowork-skills +# PR #33224 salvage (slack: truncate edit_message to prevent msg_too_long) diff --git a/contributors/emails/kamon@gao-ai.com b/contributors/emails/kamon@gao-ai.com new file mode 100644 index 000000000000..7d0bc2a38240 --- /dev/null +++ b/contributors/emails/kamon@gao-ai.com @@ -0,0 +1,2 @@ +kamonspecial +# PR #59621 salvage (slack: empty rich_text content guards) diff --git a/contributors/emails/nwadwa@gmail.com b/contributors/emails/nwadwa@gmail.com new file mode 100644 index 000000000000..4ff551cd51b0 --- /dev/null +++ b/contributors/emails/nwadwa@gmail.com @@ -0,0 +1,2 @@ +tw0316 +# PR #56618 salvage (slack: table column_settings + blocks rejection fallback) diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 867b8ae9c6c3..c583c106c629 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -56,9 +56,9 @@ from gateway.platforms.base import ( ) try: # sibling module; support both package and flat plugin-dir import - from .block_kit import render_blocks + from .block_kit import render_blocks, sanitize_blocks except ImportError: # pragma: no cover - plugin loaded outside package context - from block_kit import render_blocks # type: ignore + from block_kit import render_blocks, sanitize_blocks # type: ignore logger = logging.getLogger(__name__) @@ -2284,7 +2284,7 @@ class SlackAdapter(BasePlatformAdapter): return None try: blocks = render_blocks(content, mrkdwn_fn=self.format_message) - return self._append_feedback_block(blocks) + return sanitize_blocks(self._append_feedback_block(blocks)) except Exception: # pragma: no cover - renderer already guards itself logger.debug("[Slack] block render failed; using plain text", exc_info=True) return None @@ -4087,7 +4087,7 @@ class SlackAdapter(BasePlatformAdapter): kwargs: Dict[str, Any] = { "channel": chat_id, "text": f"⚠️ Command approval required: {cmd_preview[:100]}", - "blocks": blocks, + "blocks": sanitize_blocks(blocks), } if thread_ts: kwargs["thread_ts"] = thread_ts @@ -4167,7 +4167,7 @@ class SlackAdapter(BasePlatformAdapter): kwargs: Dict[str, Any] = { "channel": chat_id, "text": f"{title or 'Confirm'}: {body[:100]}", - "blocks": blocks, + "blocks": sanitize_blocks(blocks), } if thread_ts: kwargs["thread_ts"] = thread_ts @@ -4337,7 +4337,7 @@ class SlackAdapter(BasePlatformAdapter): channel=channel_id, ts=msg_ts, text=decision_text, - blocks=updated_blocks, + blocks=sanitize_blocks(updated_blocks), ) except Exception as e: logger.warning("[Slack] Failed to update slash-confirm message: %s", e) @@ -4509,7 +4509,7 @@ class SlackAdapter(BasePlatformAdapter): channel=channel_id, ts=msg_ts, text=decision_text, - blocks=updated_blocks, + blocks=sanitize_blocks(updated_blocks), ) except Exception as e: logger.warning("[Slack] Failed to update approval message: %s", e) diff --git a/plugins/platforms/slack/block_kit.py b/plugins/platforms/slack/block_kit.py index c67f26079943..5f8dc4d0c1b1 100644 --- a/plugins/platforms/slack/block_kit.py +++ b/plugins/platforms/slack/block_kit.py @@ -550,3 +550,118 @@ def _split_text(text: str, limit: int) -> List[str]: if remaining: out.append(remaining) return out + + +# ---------------------------------------------------------------------------- +# Outbound payload boundary — last-resort clamp before the Slack API +# ---------------------------------------------------------------------------- + + +def _clamp_text_obj(text_obj: Dict[str, Any], limit: int) -> Dict[str, Any]: + """Return ``text_obj`` with its ``text`` clamped to ``limit`` chars.""" + txt = text_obj.get("text") or "" + if len(txt) <= limit: + return text_obj + clamped = dict(text_obj) + clamped["text"] = txt[: limit - 1].rstrip() + "…" + return clamped + + +def sanitize_blocks(blocks: Optional[List[Block]]) -> Optional[List[Block]]: + """Clamp an outbound ``blocks`` payload to Slack's hard limits. + + Defensive boundary applied wherever the adapter attaches ``blocks`` to + ``chat.postMessage`` / ``chat.update``. One oversized or malformed block + fails the WHOLE call with ``invalid_blocks`` — approval cards then never + update and messages silently drop — so instead of trusting every builder, + the payload is normalized just before the API call: + + * ``section`` / ``context`` text objects are truncated to the 3000-char + cap with an ellipsis (Slack HTML-escapes ``< > &`` on storage, so text + echoed back through interaction payloads can exceed the limit that the + send path originally budgeted for — see #53693 / #62054). + * ``header`` text is truncated to its 150-char cap. + * Empty blocks (no text / no elements / no rows) are dropped — Slack + rejects zero-length text objects and empty element lists. + * ``table.column_settings`` entries must all be objects; ``null`` entries + (emitted by older renderers, per the "use null to skip" misreading of + the schema) are replaced with ``{}`` and default trailing entries are + trimmed (#56615). + * The payload is capped at Slack's 50-block maximum. + + Returns the sanitized list, or ``None`` when nothing valid remains — the + caller then sends the plain ``text`` fallback alone. Never raises. + """ + if not blocks: + return None + try: + out: List[Block] = [] + for block in blocks: + if not isinstance(block, dict) or not block.get("type"): + continue + btype = block["type"] + + if btype == "section": + text_obj = block.get("text") + has_body = bool(block.get("fields")) or bool(block.get("accessory")) + if isinstance(text_obj, dict): + if not (text_obj.get("text") or "").strip() and not has_body: + continue + clamped = _clamp_text_obj(text_obj, MAX_SECTION_TEXT) + if clamped is not text_obj: + block = dict(block) + block["text"] = clamped + elif not has_body: + continue + + elif btype == "header": + text_obj = block.get("text") + if not isinstance(text_obj, dict) or not (text_obj.get("text") or "").strip(): + continue + clamped = _clamp_text_obj(text_obj, MAX_HEADER_TEXT) + if clamped is not text_obj: + block = dict(block) + block["text"] = clamped + + elif btype == "context": + elements = block.get("elements") or [] + if not elements: + continue + clamped_els = [ + _clamp_text_obj(el, MAX_SECTION_TEXT) + if isinstance(el, dict) and el.get("type") in ("mrkdwn", "plain_text") + else el + for el in elements + ] + if any(c is not e for c, e in zip(clamped_els, elements)): + block = dict(block) + block["elements"] = clamped_els + + elif btype in ("rich_text", "actions", "context_actions"): + if not block.get("elements"): + continue + + elif btype == "table": + if not block.get("rows"): + continue + settings = block.get("column_settings") + if isinstance(settings, list) and any( + not isinstance(cs, dict) for cs in settings + ): + fixed = [cs if isinstance(cs, dict) else {} for cs in settings] + while fixed and not fixed[-1]: + fixed.pop() + block = dict(block) + if fixed: + block["column_settings"] = fixed + else: + block.pop("column_settings", None) + + out.append(block) + + if not out: + return None + return out[:MAX_BLOCKS] + except Exception: + # A sanitizer bug must never take down the send path. + return None