common/hermes
2
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(slack): harden attachment handling

Browse Source 
Multiple overlapping Slack attachment improvements:

1. Upload retry with backoff on transient errors (429, 5xx, connection
   reset, rate_limited, service unavailable). New _is_retryable_upload_error
   helper covers three upload paths: _upload_file, send_video,
   send_document. Up to 3 attempts with 1.5s * attempt backoff.

2. Thread participation tracking: successful file uploads now add the
   thread_ts to _bot_message_ts, mirroring how text replies are tracked.
   This lets follow-up thread messages auto-trigger the bot (same
   engagement rules as replied threads).

3. Thread metadata preservation in the image redirect-guard fallback
   (send_image → send text fallback) and in two gateway.run.py send
   paths (image + document fallback calls).

4. HTML response rejection in _download_slack_file_bytes. Parallels
   the existing check in _download_slack_file. Guards against Slack
   returning a sign-in / redirect page as document bytes when scopes
   are missing, so the agent doesn't get HTML-as-a-PDF.

5. File lifecycle event acks (file_shared / file_created / file_change).
   These events arrive around snippet uploads. Acking them silences the
   slack_bolt 'Unhandled request' 404 warnings without changing behavior.

6. Post-loop message type classification so a mixed image+document upload
   classifies as PHOTO (or VOICE if no image), falling back to DOCUMENT.
   Previously, the per-file classification in the inbound loop could be
   overwritten unpredictably.

7. Expanded text-inject whitelist in inbound document handling to cover
   .csv, .json, .xml, .yaml, .yml, .toml, .ini, .cfg (up to 100KB) so
   snippets and config files are directly visible to the agent, not just
   cached as opaque uploads. Paired with new MIME entries in
   SUPPORTED_DOCUMENT_TYPES in base.py.

Squashed from two commits in #11819 so the single commit carries the
contributor's GitHub attribution (the original commits were authored
under a local dev hostname).
This commit is contained in:
ghostmfr
2026-04-26 18:16:15 -07:00
committed by Teknium
parent b16f9d438b
commit e818ec520a
5 changed files with 297 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
tests/gateway/test_media_download_retry.py
View File

@@ -735,6 +735,7 @@ class TestSlackDownloadSlackFileBytes:
fake_response = MagicMock()
fake_response.content = b"raw bytes here"
fake_response.raise_for_status = MagicMock()
fake_response.headers = {"content-type": "application/pdf"}
mock_client = AsyncMock()
mock_client.get = AsyncMock(return_value=fake_response)
@@ -750,6 +751,29 @@ class TestSlackDownloadSlackFileBytes:
result = asyncio.run(run())
assert result == b"raw bytes here"
def test_rejects_html_response(self):
"""Slack HTML sign-in pages should not be accepted as file bytes."""
adapter = _make_slack_adapter()
fake_response = MagicMock()
fake_response.content = b"<!DOCTYPE html><html><title>Slack</title></html>"
fake_response.raise_for_status = MagicMock()
fake_response.headers = {"content-type": "text/html; charset=utf-8"}
mock_client = AsyncMock()
mock_client.get = AsyncMock(return_value=fake_response)
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
mock_client.__aexit__ = AsyncMock(return_value=False)
async def run():
with patch("httpx.AsyncClient", return_value=mock_client):
await adapter._download_slack_file_bytes(
"https://files.slack.com/file.bin"
)
with pytest.raises(ValueError, match="HTML instead of file bytes"):
asyncio.run(run())
def test_retries_on_429_then_succeeds(self):
"""429 on first attempt is retried; raw bytes returned on second."""
adapter = _make_slack_adapter()
@@ -757,6 +781,7 @@ class TestSlackDownloadSlackFileBytes:
ok_response = MagicMock()
ok_response.content = b"final bytes"
ok_response.raise_for_status = MagicMock()
ok_response.headers = {"content-type": "application/pdf"}
mock_client = AsyncMock()
mock_client.get = AsyncMock(

106
tests/gateway/test_slack.py
View File

@@ -287,6 +287,40 @@ class TestSendDocument:
call_kwargs = adapter._app.client.files_upload_v2.call_args[1]
assert call_kwargs["thread_ts"] == "1234567890.123456"
@pytest.mark.asyncio
async def test_send_document_thread_upload_marks_bot_participation(self, adapter, tmp_path):
test_file = tmp_path / "notes.txt"
test_file.write_bytes(b"some notes")
adapter._app.client.files_upload_v2 = AsyncMock(return_value={"ok": True})
await adapter.send_document(
chat_id="C123",
file_path=str(test_file),
metadata={"thread_id": "1234567890.123456"},
)
assert "1234567890.123456" in adapter._bot_message_ts
@pytest.mark.asyncio
async def test_send_document_retries_transient_upload_error(self, adapter, tmp_path):
test_file = tmp_path / "notes.txt"
test_file.write_bytes(b"some notes")
adapter._app.client.files_upload_v2 = AsyncMock(
side_effect=[RuntimeError("Connection reset by peer"), {"ok": True}]
)
with patch("asyncio.sleep", new_callable=AsyncMock) as sleep_mock:
result = await adapter.send_document(
chat_id="C123",
file_path=str(test_file),
)
assert result.success
assert adapter._app.client.files_upload_v2.await_count == 2
sleep_mock.assert_awaited_once()
# ---------------------------------------------------------------------------
# TestSendVideo
@@ -430,6 +464,36 @@ class TestIncomingDocumentHandling:
msg_event = adapter.handle_message.call_args[0][0]
assert "# Title" in msg_event.text
@pytest.mark.asyncio
async def test_json_snippet_injects_content(self, adapter):
"""A .json snippet should be treated as a text document and injected."""
content = b'{"hello": "world", "count": 2}'
with patch.object(adapter, "_download_slack_file_bytes", new_callable=AsyncMock) as dl:
dl.return_value = content
event = self._make_event(
text="can you parse this",
files=[{
"mimetype": "text/plain",
"name": "zapfile.json",
"filetype": "json",
"pretty_type": "JSON",
"mode": "snippet",
"editable": True,
"url_private_download": "https://files.slack.com/zapfile.json",
"size": len(content),
}],
)
await adapter._handle_slack_message(event)
msg_event = adapter.handle_message.call_args[0][0]
assert msg_event.message_type == MessageType.DOCUMENT
assert len(msg_event.media_urls) == 1
assert msg_event.media_types == ["application/json"]
assert '[Content of zapfile.json]' in msg_event.text
assert '"hello": "world"' in msg_event.text
assert 'can you parse this' in msg_event.text
@pytest.mark.asyncio
async def test_large_txt_not_injected(self, adapter):
"""A .txt file over 100KB should be cached but NOT injected."""
@@ -2090,6 +2154,48 @@ class TestSendImageSSRFGuards:
assert "see this" in call_kwargs["text"]
assert "https://public.example/image.png" in call_kwargs["text"]
@pytest.mark.asyncio
async def test_send_image_fallback_preserves_thread_metadata(self, adapter):
redirect_response = MagicMock()
redirect_response.is_redirect = True
redirect_response.next_request = MagicMock(
url="http://169.254.169.254/latest/meta-data"
)
client_kwargs = {}
mock_client = AsyncMock()
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
mock_client.__aexit__ = AsyncMock(return_value=False)
async def fake_get(_url):
for hook in client_kwargs["event_hooks"]["response"]:
await hook(redirect_response)
mock_client.get = AsyncMock(side_effect=fake_get)
adapter._app.client.files_upload_v2 = AsyncMock(return_value={"ok": True})
adapter._app.client.chat_postMessage = AsyncMock(return_value={"ts": "reply_ts"})
def fake_async_client(*args, **kwargs):
client_kwargs.update(kwargs)
return mock_client
def fake_is_safe_url(url):
return url == "https://public.example/image.png"
with (
patch("tools.url_safety.is_safe_url", side_effect=fake_is_safe_url),
patch("httpx.AsyncClient", side_effect=fake_async_client),
):
await adapter.send_image(
chat_id="C123",
image_url="https://public.example/image.png",
caption="see this",
metadata={"thread_id": "parent_ts_789"},
)
call_kwargs = adapter._app.client.chat_postMessage.call_args.kwargs
assert call_kwargs.get("thread_ts") == "parent_ts_789"
# ---------------------------------------------------------------------------
# TestProgressMessageThread