mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
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).
This commit is contained in:
parent
72da4f8657
commit
a43ac2af65
5 changed files with 128 additions and 7 deletions
2
contributors/emails/cjwang@sowork.tw
Normal file
2
contributors/emails/cjwang@sowork.tw
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
sowork-skills
|
||||
# PR #33224 salvage (slack: truncate edit_message to prevent msg_too_long)
|
||||
2
contributors/emails/kamon@gao-ai.com
Normal file
2
contributors/emails/kamon@gao-ai.com
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
kamonspecial
|
||||
# PR #59621 salvage (slack: empty rich_text content guards)
|
||||
2
contributors/emails/nwadwa@gmail.com
Normal file
2
contributors/emails/nwadwa@gmail.com
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
tw0316
|
||||
# PR #56618 salvage (slack: table column_settings + blocks rejection fallback)
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue