From a710a7d6f75b2f9d70e36a2966c329d7ef855420 Mon Sep 17 00:00:00 2001 From: Kenneth Zhao Date: Tue, 23 Jun 2026 03:42:57 +0000 Subject: [PATCH] fix(mattermost): address review comments - Attachments now only attached to first chunk when message is split - Filename sanitized with Path(name).name to prevent path traversal - Updated streaming tests to match buffer-and-post-at-end pattern (iOS compatibility) --- nanobot/channels/mattermost.py | 12 ++++++------ tests/channels/test_mattermost_channel.py | 13 +++++++------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/nanobot/channels/mattermost.py b/nanobot/channels/mattermost.py index a9a5a02d..897839ad 100644 --- a/nanobot/channels/mattermost.py +++ b/nanobot/channels/mattermost.py @@ -459,11 +459,11 @@ class MattermostChannel(BaseChannel): if msg.content or file_ids: text = msg.content or " " chunks = split_message(text, MATTERMOST_MAX_MESSAGE_LEN) - for chunk in chunks: + for i, chunk in enumerate(chunks): await self._create_post( chat_id, chunk, root_id=root_id if self.config.reply_in_thread else None, - file_ids=file_ids or None, + file_ids=(file_ids if i == 0 else None) or None, ) if not meta.get("_progress") and meta.get("message_id"): @@ -616,10 +616,10 @@ class MattermostChannel(BaseChannel): async def _download_file(self, file_id: str) -> str | None: try: - resp = await self._http_client.get(f"/api/v4/files/{file_id}") - resp.raise_for_status() - info = resp.json() - name = info.get("name", file_id) + info_resp = await self._http_client.get(f"/api/v4/files/{file_id}/info") + info_resp.raise_for_status() + info = info_resp.json() + name = Path(info.get("name", file_id)).name out = Path(get_media_dir("mattermost")) / f"{file_id}_{name}" out.parent.mkdir(parents=True, exist_ok=True) diff --git a/tests/channels/test_mattermost_channel.py b/tests/channels/test_mattermost_channel.py index 4c2f5988..c96ee0c3 100644 --- a/tests/channels/test_mattermost_channel.py +++ b/tests/channels/test_mattermost_channel.py @@ -492,9 +492,9 @@ async def test_stream_first_delta_creates_post(): await channel.send_delta("chan_1", "Hello", {"_stream_id": "s1"}) posts = [c for c in fake.post_calls if c["path"] == "/api/v4/posts"] - assert len(posts) == 1 - assert "Hello" in posts[0]["json"]["message"] - assert channel._stream_posts["s1"] == "stream_post_1" + assert len(posts) == 0 + assert channel._stream_buffers["s1"] == "Hello" + assert channel._stream_committed["s1"] == "Hello" @pytest.mark.asyncio @@ -508,8 +508,8 @@ async def test_stream_subsequent_delta_edits_post(): await channel.send_delta("chan_1", " world", {"_stream_id": "s1"}) edits = [c for c in fake.put_calls if c["path"] == "/api/v4/posts/stream_post_1"] - assert len(edits) == 1 - assert edits[0]["json"]["message"] == "Hello world" + assert len(edits) == 0 + assert channel._stream_buffers["s1"] == "Hello world" @pytest.mark.asyncio @@ -535,7 +535,8 @@ async def test_stream_chunk_boundary_finalizes_and_creates_new(): await channel.send_delta("chan_1", "world", {"_stream_id": "s1"}) posts = [c for c in fake.post_calls if c["path"] == "/api/v4/posts"] - assert len(posts) == 2 + assert len(posts) == 0 + assert channel._stream_buffers["s1"] == "Hello world" # ---------------------------------------------------------------------------