Compare commits

...

1 Commits

Author SHA1 Message Date
kshitijk4poor
3eadb3dd89 fix(signal): implement send_image_file, send_voice, and send_video for MEDIA: tag delivery
The Signal adapter inherited base class defaults for send_image_file(),
send_voice(), and send_video() which only sent the file path as text
(e.g. '🖼️ Image: /tmp/chart.png') instead of actually delivering the file
as a Signal attachment.

When agent responses contain MEDIA:/path/to/file tags, the gateway
media pipeline extracts them and routes through these methods by file
type. Without proper overrides, image/audio/video files were never
actually delivered to Signal users.

Extract a shared _send_attachment() helper that handles all file
validation, size checking, group/DM routing, and RPC dispatch. The four
public methods (send_document, send_image_file, send_voice, send_video)
now delegate to this helper, following the same pattern used by WhatsApp
(_send_media_to_bridge) and Discord (_send_file_attachment).

The helper also uses a single stat() call with try/except FileNotFoundError
instead of the previous exists() + stat() two-syscall pattern, eliminating
a TOCTOU race. As a bonus, send_document() now gains the 100MB size check
that was previously missing (inconsistency with send_image).

Add 25 tests covering all methods plus MEDIA: tag extraction integration,
method-override guards, and send_document's new size check.

Fixes #5105
2026-04-06 11:40:43 -07:00
2 changed files with 406 additions and 7 deletions

View File

@@ -717,19 +717,27 @@ class SignalAdapter(BasePlatformAdapter):
return SendResult(success=True) return SendResult(success=True)
return SendResult(success=False, error="RPC send with attachment failed") return SendResult(success=False, error="RPC send with attachment failed")
async def send_document( async def _send_attachment(
self, self,
chat_id: str, chat_id: str,
file_path: str, file_path: str,
media_label: str,
caption: Optional[str] = None, caption: Optional[str] = None,
filename: Optional[str] = None,
**kwargs,
) -> SendResult: ) -> SendResult:
"""Send a document/file attachment.""" """Send any file as a Signal attachment via RPC.
Shared implementation for send_document, send_image_file, send_voice,
and send_video — avoids duplicating the validation/routing/RPC logic.
"""
await self._stop_typing_indicator(chat_id) await self._stop_typing_indicator(chat_id)
if not Path(file_path).exists(): try:
return SendResult(success=False, error="File not found") file_size = Path(file_path).stat().st_size
except FileNotFoundError:
return SendResult(success=False, error=f"{media_label} file not found: {file_path}")
if file_size > SIGNAL_MAX_ATTACHMENT_SIZE:
return SendResult(success=False, error=f"{media_label} too large ({file_size} bytes)")
params: Dict[str, Any] = { params: Dict[str, Any] = {
"account": self.account, "account": self.account,
@@ -746,7 +754,59 @@ class SignalAdapter(BasePlatformAdapter):
if result is not None: if result is not None:
self._track_sent_timestamp(result) self._track_sent_timestamp(result)
return SendResult(success=True) return SendResult(success=True)
return SendResult(success=False, error="RPC send document failed") return SendResult(success=False, error=f"RPC send {media_label.lower()} failed")
async def send_document(
self,
chat_id: str,
file_path: str,
caption: Optional[str] = None,
filename: Optional[str] = None,
**kwargs,
) -> SendResult:
"""Send a document/file attachment."""
return await self._send_attachment(chat_id, file_path, "File", caption)
async def send_image_file(
self,
chat_id: str,
image_path: str,
caption: Optional[str] = None,
reply_to: Optional[str] = None,
**kwargs,
) -> SendResult:
"""Send a local image file as a native Signal attachment.
Called by the gateway media delivery flow when MEDIA: tags containing
image paths are extracted from agent responses.
"""
return await self._send_attachment(chat_id, image_path, "Image", caption)
async def send_voice(
self,
chat_id: str,
audio_path: str,
caption: Optional[str] = None,
reply_to: Optional[str] = None,
**kwargs,
) -> SendResult:
"""Send an audio file as a Signal attachment.
Signal does not distinguish voice messages from file attachments at
the API level, so this routes through the same RPC send path.
"""
return await self._send_attachment(chat_id, audio_path, "Audio", caption)
async def send_video(
self,
chat_id: str,
video_path: str,
caption: Optional[str] = None,
reply_to: Optional[str] = None,
**kwargs,
) -> SendResult:
"""Send a video file as a Signal attachment."""
return await self._send_attachment(chat_id, video_path, "Video", caption)
# ------------------------------------------------------------------ # ------------------------------------------------------------------
# Typing Indicators # Typing Indicators

View File

@@ -2,6 +2,7 @@
import base64 import base64
import json import json
import pytest import pytest
from pathlib import Path
from unittest.mock import MagicMock, patch, AsyncMock from unittest.mock import MagicMock, patch, AsyncMock
from urllib.parse import quote from urllib.parse import quote
@@ -368,3 +369,341 @@ class TestSignalSendMessage:
# Just verify the import works and Signal is a valid platform # Just verify the import works and Signal is a valid platform
from gateway.config import Platform from gateway.config import Platform
assert Platform.SIGNAL.value == "signal" assert Platform.SIGNAL.value == "signal"
# ---------------------------------------------------------------------------
# send_image_file method (#5105)
# ---------------------------------------------------------------------------
class TestSignalSendImageFile:
@pytest.mark.asyncio
async def test_send_image_file_sends_via_rpc(self, monkeypatch, tmp_path):
"""send_image_file should send image as attachment via signal-cli RPC."""
adapter = _make_signal_adapter(monkeypatch)
mock_rpc, captured = _stub_rpc({"timestamp": 1234567890})
adapter._rpc = mock_rpc
adapter._stop_typing_indicator = AsyncMock()
img_path = tmp_path / "chart.png"
img_path.write_bytes(b"\x89PNG" + b"\x00" * 100)
result = await adapter.send_image_file(chat_id="+155****4567", image_path=str(img_path))
assert result.success is True
assert len(captured) == 1
assert captured[0]["method"] == "send"
assert captured[0]["params"]["account"] == adapter.account
assert captured[0]["params"]["recipient"] == ["+155****4567"]
assert captured[0]["params"]["attachments"] == [str(img_path)]
assert captured[0]["params"]["message"] == "" # caption=None → ""
# Typing indicator must be stopped before sending
adapter._stop_typing_indicator.assert_awaited_once_with("+155****4567")
# Timestamp must be tracked for echo-back prevention
assert 1234567890 in adapter._recent_sent_timestamps
@pytest.mark.asyncio
async def test_send_image_file_to_group(self, monkeypatch, tmp_path):
"""send_image_file should route group chats via groupId."""
adapter = _make_signal_adapter(monkeypatch)
mock_rpc, captured = _stub_rpc({"timestamp": 1234567890})
adapter._rpc = mock_rpc
adapter._stop_typing_indicator = AsyncMock()
img_path = tmp_path / "photo.jpg"
img_path.write_bytes(b"\xff\xd8" + b"\x00" * 100)
result = await adapter.send_image_file(
chat_id="group:abc123==", image_path=str(img_path), caption="Here's the chart"
)
assert result.success is True
assert captured[0]["params"]["groupId"] == "abc123=="
assert captured[0]["params"]["message"] == "Here's the chart"
@pytest.mark.asyncio
async def test_send_image_file_missing(self, monkeypatch):
"""send_image_file should fail gracefully for nonexistent files."""
adapter = _make_signal_adapter(monkeypatch)
adapter._stop_typing_indicator = AsyncMock()
result = await adapter.send_image_file(chat_id="+155****4567", image_path="/nonexistent.png")
assert result.success is False
assert "not found" in result.error.lower()
@pytest.mark.asyncio
async def test_send_image_file_too_large(self, monkeypatch, tmp_path):
"""send_image_file should reject files over 100MB."""
adapter = _make_signal_adapter(monkeypatch)
adapter._stop_typing_indicator = AsyncMock()
img_path = tmp_path / "huge.png"
img_path.write_bytes(b"x")
def mock_stat(self, **kwargs):
class FakeStat:
st_size = 200 * 1024 * 1024 # 200 MB
return FakeStat()
with patch.object(Path, "stat", mock_stat):
result = await adapter.send_image_file(chat_id="+155****4567", image_path=str(img_path))
assert result.success is False
assert "too large" in result.error.lower()
@pytest.mark.asyncio
async def test_send_image_file_rpc_failure(self, monkeypatch, tmp_path):
"""send_image_file should return error when RPC returns None."""
adapter = _make_signal_adapter(monkeypatch)
mock_rpc, _ = _stub_rpc(None)
adapter._rpc = mock_rpc
adapter._stop_typing_indicator = AsyncMock()
img_path = tmp_path / "test.png"
img_path.write_bytes(b"\x89PNG" + b"\x00" * 100)
result = await adapter.send_image_file(chat_id="+155****4567", image_path=str(img_path))
assert result.success is False
assert "failed" in result.error.lower()
# ---------------------------------------------------------------------------
# send_voice method (#5105)
# ---------------------------------------------------------------------------
class TestSignalSendVoice:
@pytest.mark.asyncio
async def test_send_voice_sends_via_rpc(self, monkeypatch, tmp_path):
"""send_voice should send audio as attachment via signal-cli RPC."""
adapter = _make_signal_adapter(monkeypatch)
mock_rpc, captured = _stub_rpc({"timestamp": 1234567890})
adapter._rpc = mock_rpc
adapter._stop_typing_indicator = AsyncMock()
audio_path = tmp_path / "reply.ogg"
audio_path.write_bytes(b"OggS" + b"\x00" * 100)
result = await adapter.send_voice(chat_id="+155****4567", audio_path=str(audio_path))
assert result.success is True
assert captured[0]["method"] == "send"
assert captured[0]["params"]["attachments"] == [str(audio_path)]
assert captured[0]["params"]["message"] == "" # caption=None → ""
adapter._stop_typing_indicator.assert_awaited_once_with("+155****4567")
assert 1234567890 in adapter._recent_sent_timestamps
@pytest.mark.asyncio
async def test_send_voice_missing_file(self, monkeypatch):
"""send_voice should fail for nonexistent audio."""
adapter = _make_signal_adapter(monkeypatch)
adapter._stop_typing_indicator = AsyncMock()
result = await adapter.send_voice(chat_id="+155****4567", audio_path="/missing.ogg")
assert result.success is False
assert "not found" in result.error.lower()
@pytest.mark.asyncio
async def test_send_voice_to_group(self, monkeypatch, tmp_path):
"""send_voice should route group chats correctly."""
adapter = _make_signal_adapter(monkeypatch)
mock_rpc, captured = _stub_rpc({"timestamp": 9999})
adapter._rpc = mock_rpc
adapter._stop_typing_indicator = AsyncMock()
audio_path = tmp_path / "note.mp3"
audio_path.write_bytes(b"\xff\xe0" + b"\x00" * 100)
result = await adapter.send_voice(chat_id="group:grp1==", audio_path=str(audio_path))
assert result.success is True
assert captured[0]["params"]["groupId"] == "grp1=="
@pytest.mark.asyncio
async def test_send_voice_too_large(self, monkeypatch, tmp_path):
"""send_voice should reject files over 100MB."""
adapter = _make_signal_adapter(monkeypatch)
adapter._stop_typing_indicator = AsyncMock()
audio_path = tmp_path / "huge.ogg"
audio_path.write_bytes(b"x")
def mock_stat(self, **kwargs):
class FakeStat:
st_size = 200 * 1024 * 1024
return FakeStat()
with patch.object(Path, "stat", mock_stat):
result = await adapter.send_voice(chat_id="+155****4567", audio_path=str(audio_path))
assert result.success is False
assert "too large" in result.error.lower()
@pytest.mark.asyncio
async def test_send_voice_rpc_failure(self, monkeypatch, tmp_path):
"""send_voice should return error when RPC returns None."""
adapter = _make_signal_adapter(monkeypatch)
mock_rpc, _ = _stub_rpc(None)
adapter._rpc = mock_rpc
adapter._stop_typing_indicator = AsyncMock()
audio_path = tmp_path / "reply.ogg"
audio_path.write_bytes(b"OggS" + b"\x00" * 100)
result = await adapter.send_voice(chat_id="+155****4567", audio_path=str(audio_path))
assert result.success is False
assert "failed" in result.error.lower()
# ---------------------------------------------------------------------------
# send_video method (#5105)
# ---------------------------------------------------------------------------
class TestSignalSendVideo:
@pytest.mark.asyncio
async def test_send_video_sends_via_rpc(self, monkeypatch, tmp_path):
"""send_video should send video as attachment via signal-cli RPC."""
adapter = _make_signal_adapter(monkeypatch)
mock_rpc, captured = _stub_rpc({"timestamp": 1234567890})
adapter._rpc = mock_rpc
adapter._stop_typing_indicator = AsyncMock()
vid_path = tmp_path / "demo.mp4"
vid_path.write_bytes(b"\x00\x00\x00\x18ftyp" + b"\x00" * 100)
result = await adapter.send_video(chat_id="+155****4567", video_path=str(vid_path))
assert result.success is True
assert captured[0]["method"] == "send"
assert captured[0]["params"]["attachments"] == [str(vid_path)]
assert captured[0]["params"]["message"] == "" # caption=None → ""
adapter._stop_typing_indicator.assert_awaited_once_with("+155****4567")
assert 1234567890 in adapter._recent_sent_timestamps
@pytest.mark.asyncio
async def test_send_video_missing_file(self, monkeypatch):
"""send_video should fail for nonexistent video."""
adapter = _make_signal_adapter(monkeypatch)
adapter._stop_typing_indicator = AsyncMock()
result = await adapter.send_video(chat_id="+155****4567", video_path="/missing.mp4")
assert result.success is False
assert "not found" in result.error.lower()
@pytest.mark.asyncio
async def test_send_video_too_large(self, monkeypatch, tmp_path):
"""send_video should reject files over 100MB."""
adapter = _make_signal_adapter(monkeypatch)
adapter._stop_typing_indicator = AsyncMock()
vid_path = tmp_path / "huge.mp4"
vid_path.write_bytes(b"x")
def mock_stat(self, **kwargs):
class FakeStat:
st_size = 200 * 1024 * 1024
return FakeStat()
with patch.object(Path, "stat", mock_stat):
result = await adapter.send_video(chat_id="+155****4567", video_path=str(vid_path))
assert result.success is False
assert "too large" in result.error.lower()
@pytest.mark.asyncio
async def test_send_video_rpc_failure(self, monkeypatch, tmp_path):
"""send_video should return error when RPC returns None."""
adapter = _make_signal_adapter(monkeypatch)
mock_rpc, _ = _stub_rpc(None)
adapter._rpc = mock_rpc
adapter._stop_typing_indicator = AsyncMock()
vid_path = tmp_path / "demo.mp4"
vid_path.write_bytes(b"\x00\x00\x00\x18ftyp" + b"\x00" * 100)
result = await adapter.send_video(chat_id="+155****4567", video_path=str(vid_path))
assert result.success is False
assert "failed" in result.error.lower()
# ---------------------------------------------------------------------------
# MEDIA: tag extraction integration
# ---------------------------------------------------------------------------
class TestSignalMediaExtraction:
"""Verify the full pipeline: MEDIA: tag → extract → send_image_file/send_voice."""
def test_extract_media_finds_image_tag(self):
"""BasePlatformAdapter.extract_media should find MEDIA: image paths."""
from gateway.platforms.base import BasePlatformAdapter
media, cleaned = BasePlatformAdapter.extract_media(
"Here's the chart.\nMEDIA:/tmp/price_graph.png"
)
assert len(media) == 1
assert media[0][0] == "/tmp/price_graph.png"
assert "MEDIA:" not in cleaned
def test_extract_media_finds_audio_tag(self):
"""BasePlatformAdapter.extract_media should find MEDIA: audio paths."""
from gateway.platforms.base import BasePlatformAdapter
media, cleaned = BasePlatformAdapter.extract_media(
"[[audio_as_voice]]\nMEDIA:/tmp/reply.ogg"
)
assert len(media) == 1
assert media[0][0] == "/tmp/reply.ogg"
assert media[0][1] is True # is_voice flag
def test_signal_has_all_media_methods(self, monkeypatch):
"""SignalAdapter must override all media send methods used by gateway."""
adapter = _make_signal_adapter(monkeypatch)
from gateway.platforms.base import BasePlatformAdapter
# These methods must NOT be the base class defaults (which just send text)
assert type(adapter).send_image_file is not BasePlatformAdapter.send_image_file
assert type(adapter).send_voice is not BasePlatformAdapter.send_voice
assert type(adapter).send_video is not BasePlatformAdapter.send_video
assert type(adapter).send_document is not BasePlatformAdapter.send_document
assert type(adapter).send_image is not BasePlatformAdapter.send_image
# ---------------------------------------------------------------------------
# send_document now routes through _send_attachment (#5105 bonus)
# ---------------------------------------------------------------------------
class TestSignalSendDocumentViaHelper:
"""Verify send_document gained size check and path-in-error via _send_attachment."""
@pytest.mark.asyncio
async def test_send_document_too_large(self, monkeypatch, tmp_path):
"""send_document should now reject files over 100MB (was previously missing)."""
adapter = _make_signal_adapter(monkeypatch)
adapter._stop_typing_indicator = AsyncMock()
doc_path = tmp_path / "huge.pdf"
doc_path.write_bytes(b"x")
def mock_stat(self, **kwargs):
class FakeStat:
st_size = 200 * 1024 * 1024
return FakeStat()
with patch.object(Path, "stat", mock_stat):
result = await adapter.send_document(chat_id="+155****4567", file_path=str(doc_path))
assert result.success is False
assert "too large" in result.error.lower()
@pytest.mark.asyncio
async def test_send_document_error_includes_path(self, monkeypatch):
"""send_document error message should include the file path."""
adapter = _make_signal_adapter(monkeypatch)
adapter._stop_typing_indicator = AsyncMock()
result = await adapter.send_document(chat_id="+155****4567", file_path="/nonexistent.pdf")
assert result.success is False
assert "/nonexistent.pdf" in result.error