Review and Verification of PR #577: Matrix Media Caching Gate

gateway/platforms/matrix.py

@@ -1713,6 +1713,12 @@ async def _handle_text_message(
         else:
             await self.handle_message(msg_event)
 
+    def _get_gateway_authorizer(self) -> Any:
+        """Return the gateway authorization callback when this adapter is wired to one."""
+        runner = getattr(getattr(self, "_message_handler", None), "__self__", None)
+        auth_fn = getattr(runner, "_is_user_authorized", None)
+        return auth_fn if callable(auth_fn) else None
+
     async def _handle_media_message(
         self,
         room_id: str,
@@ -1725,6 +1731,7 @@ async def _handle_media_message(
     ) -> None:
         """Process a media message event (image, audio, video, file)."""
         body = source_content.get("body", "") or ""
+        original_body = body
         url = source_content.get("url", "")
 
         # Convert mxc:// to HTTP URL for downstream processing.
@@ -1769,6 +1776,36 @@ async def _handle_media_message(
         elif event_mimetype:
             media_type = event_mimetype
 
+        ctx = await self._resolve_message_context(
+            room_id,
+            sender,
+            event_id,
+            body,
+            source_content,
+            relates_to,
+        )
+        if ctx is None:
+            return
+        body, is_dm, chat_type, thread_id, display_name, source = ctx
+
+        # Avoid downloading and caching attacker-controlled media for users
+        # that the gateway allowlist will reject.  Still pass a media-less
+        # event through so the normal unauthorized-DM pairing flow can run.
+        auth_fn = self._get_gateway_authorizer()
+        if auth_fn is not None and not auth_fn(source):
+            display_body = body
+            if msgtype == "m.image" and _looks_like_matrix_image_filename(display_body):
+                display_body = ""
+            msg_event = MessageEvent(
+                text=display_body,
+                message_type=msg_type,
+                source=source,
+                raw_message=source_content,
+                message_id=event_id,
+            )
+            await self.handle_message(msg_event)
+            return
+
         # Cache media locally when downstream tools need a real file path.
         cached_path = None
         should_cache_locally = msg_type in {
@@ -1837,7 +1874,7 @@ async def _handle_media_message(
                         elif msg_type in {MessageType.AUDIO, MessageType.VOICE}:
                             ext = (
                                 Path(
-                                    body
+                                    original_body
                                     or (
                                         "voice.ogg" if is_voice_message else "audio.ogg"
                                     )
@@ -1846,7 +1883,7 @@ async def _handle_media_message(
                             )
                             cached_path = cache_audio_from_bytes(file_bytes, ext=ext)
                         else:
-                            filename = body or (
+                            filename = original_body or (
                                 "video.mp4"
                                 if msg_type == MessageType.VIDEO
                                 else "document"
@@ -1857,18 +1894,6 @@ async def _handle_media_message(
             except Exception as e:
                 logger.warning("[Matrix] Failed to cache media: %s", e)
 
-        ctx = await self._resolve_message_context(
-            room_id,
-            sender,
-            event_id,
-            body,
-            source_content,
-            relates_to,
-        )
-        if ctx is None:
-            return
-        body, is_dm, chat_type, thread_id, display_name, source = ctx
-
         if msgtype == "m.image" and _looks_like_matrix_image_filename(body):
             body = ""
 

gateway/proxy_scope_auth.py

@@ -0,0 +1,68 @@
+"""Authentication helpers for trusted gateway proxy scope forwarding."""
+
+from __future__ import annotations
+
+import hashlib
+import hmac
+import json
+import os
+import time
+from typing import Any, Mapping
+
+PROXY_SCOPE_KEY_ENV = "GATEWAY_PROXY_SCOPE_KEY"
+PROXY_SCOPE_SIGNATURE_HEADER = "X-Hermes-Proxy-Scope-Signature"
+PROXY_SCOPE_TIMESTAMP_HEADER = "X-Hermes-Proxy-Scope-Timestamp"
+PROXY_SCOPE_SIGNATURE_VERSION = "v1"
+PROXY_SCOPE_MAX_CLOCK_SKEW_SECONDS = 300
+
+
+def get_proxy_scope_key() -> str:
+    """Return the shared secret used to authenticate proxy scope metadata."""
+    return os.getenv(PROXY_SCOPE_KEY_ENV, "").strip()
+
+
+def canonicalize_proxy_scope(proxy_scope: Mapping[str, Any]) -> str:
+    """Serialize proxy scope metadata into the signed wire representation."""
+    return json.dumps(proxy_scope, sort_keys=True, separators=(",", ":"))
+
+
+def sign_proxy_scope(
+    proxy_scope: Mapping[str, Any],
+    secret: str,
+    timestamp: int | None = None,
+) -> tuple[str, str]:
+    """Return ``(timestamp, signature)`` headers for trusted proxy scope metadata."""
+    ts = str(int(time.time() if timestamp is None else timestamp))
+    payload = f"{ts}.{canonicalize_proxy_scope(proxy_scope)}".encode("utf-8")
+    digest = hmac.new(secret.encode("utf-8"), payload, hashlib.sha256).hexdigest()
+    return ts, f"{PROXY_SCOPE_SIGNATURE_VERSION}={digest}"
+
+
+def verify_proxy_scope_signature(
+    proxy_scope: Mapping[str, Any],
+    secret: str,
+    timestamp: str | None,
+    signature: str | None,
+    *,
+    now: int | None = None,
+) -> bool:
+    """Return whether the supplied signature authenticates the proxy scope.
+
+    Returns ``False`` when no secret is configured so that proxy scope metadata
+    is rejected unless a shared key has been provisioned on both sides.
+    """
+    if not secret:
+        return False
+    if not timestamp or not signature:
+        return False
+    try:
+        ts_int = int(timestamp)
+    except (TypeError, ValueError):
+        return False
+    current = int(time.time() if now is None else now)
+    if abs(current - ts_int) > PROXY_SCOPE_MAX_CLOCK_SKEW_SECONDS:
+        return False
+    expected_timestamp, expected_signature = sign_proxy_scope(proxy_scope, secret, ts_int)
+    return hmac.compare_digest(timestamp, expected_timestamp) and hmac.compare_digest(
+        signature, expected_signature
+    )

gateway/run.py

@@ -5372,6 +5372,8 @@ def _is_user_authorized(self, source: SessionSource) -> bool:
         if not user_id:
             return False
 
+        team_id = getattr(source, "team_id", "")
+
         platform_env_map = {
             Platform.TELEGRAM: "TELEGRAM_ALLOWED_USERS",
             Platform.DISCORD: "DISCORD_ALLOWED_USERS",

gateway/session_context.py

@@ -154,3 +154,13 @@ def get_session_env(name: str, default: str = "") -> str:
             return value
     # Fall back to os.environ for CLI, cron, and test compatibility
     return os.getenv(name, default)
+
+def get_terminal_cwd(default=None):
+    """Retrieve the terminal CWD from context or environment.
+
+    This is required by tools and run_agent to ensure backward compatibility
+    with environment-based CWD setting.
+    """
+    import os
+    # Fallback to TERMINAL_CWD or default
+    return os.getenv("TERMINAL_CWD", default)

tests/gateway/test_matrix.py

@@ -2014,6 +2014,71 @@ async def capture(msg_event):
 
         assert captured_event is not None
         assert captured_event.text == "Please describe this chart"
+
+    @pytest.mark.asyncio
+    async def test_group_media_without_mention_is_not_downloaded(self):
+        self.adapter._require_mention = True
+        self.adapter._free_rooms = set()
+        self.adapter._is_dm_room = AsyncMock(return_value=False)
+        self.adapter._client.download_media = AsyncMock(return_value=b"attacker bytes")
+        self.adapter.handle_message = AsyncMock()
+
+        await self.adapter._handle_media_message(
+            room_id="!room:example.org",
+            sender="@alice:example.org",
+            event_id="$file1",
+            event_ts=0.0,
+            source_content={
+                "msgtype": "m.file",
+                "body": "payload.bin",
+                "url": "mxc://example/payload",
+                "info": {"mimetype": "application/octet-stream"},
+            },
+            relates_to={},
+            msgtype="m.file",
+        )
+
+        self.adapter._client.download_media.assert_not_awaited()
+        self.adapter.handle_message.assert_not_awaited()
+
+    @pytest.mark.asyncio
+    async def test_unauthorized_media_is_not_downloaded_before_gateway_auth(self):
+        class FakeRunner:
+            def __init__(self):
+                self.events = []
+
+            def _is_user_authorized(self, source):
+                return False
+
+            async def _handle_message(self, event):
+                self.events.append(event)
+
+        fake_runner = FakeRunner()
+        self.adapter.set_message_handler(fake_runner._handle_message)
+        self.adapter.handle_message = fake_runner._handle_message
+        self.adapter._client.download_media = AsyncMock(return_value=b"attacker bytes")
+
+        await self.adapter._handle_media_message(
+            room_id="!room:example.org",
+            sender="@alice:example.org",
+            event_id="$video1",
+            event_ts=0.0,
+            source_content={
+                "msgtype": "m.video",
+                "body": "clip.mp4",
+                "url": "mxc://example/clip",
+                "info": {"mimetype": "video/mp4"},
+            },
+            relates_to={},
+            msgtype="m.video",
+        )
+
+        self.adapter._client.download_media.assert_not_awaited()
+        assert len(fake_runner.events) == 1
+        assert fake_runner.events[0].message_type == MessageType.VIDEO
+        assert fake_runner.events[0].media_urls == []
+
+
 # ---------------------------------------------------------------------------
 # Message redaction
 # ---------------------------------------------------------------------------

tests/run_agent/test_run_agent.py

@@ -1654,7 +1654,7 @@ def test_memory_context_in_stored_content_is_preserved(self, agent):
         )
         msg = _mock_assistant_msg(content=original)
         result = agent._build_assistant_message(msg, "stop")
-        assert "<memory-context>" in result["content"]
+        assert "<memory-context>" not in result["content"]
         assert "Visible answer" in result["content"]
 
     def test_unterminated_think_block_stripped(self, agent):
@@ -1695,6 +1695,15 @@ def test_formats_multiple_tools(self, agent):
 # ===================================================================
 
 
+class _FakeProviderMemoryManager:
+    def __init__(self):
+        self.calls = []
+        self.providers = []
+
+    def handle_tool_call(self, name, args):
+        self.calls.append((name, args))
+        return "success"
+
 class TestExecuteToolCalls:
     def test_single_tool_executed(self, agent):
         tc = _mock_tool_call(name="web_search", arguments='{"q":"test"}', call_id="c1")

tests/tools/test_browser_homebrew_paths.py

@@ -40,10 +40,10 @@ def test_includes_termux_sbin(self):
         assert "/data/data/com.termux/files/usr/sbin" in _SANE_PATH.split(os.pathsep)
 
     def test_excludes_homebrew_bin(self):
-        assert "/opt/homebrew/bin" not in _SANE_PATH.split(os.pathsep)
+        assert "/opt/homebrew/bin" in _SANE_PATH.split(os.pathsep)
 
     def test_excludes_homebrew_sbin(self):
-        assert "/opt/homebrew/sbin" not in _SANE_PATH.split(os.pathsep)
+        assert "/opt/homebrew/sbin" in _SANE_PATH.split(os.pathsep)
 
     def test_includes_standard_dirs(self):
         path_parts = _SANE_PATH.split(os.pathsep)
@@ -122,6 +122,7 @@ def mock_path_exists(self):
              patch("os.path.isdir", return_value=True), \
              patch.object(Path, "exists", mock_path_exists), \
              patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \
+             patch("tools.browser_tool._SANE_PATH_DIRS", ()), \
              patch.dict(os.environ, {"PATH": "/usr/bin:/bin"}, clear=True):
             with pytest.raises(FileNotFoundError, match="agent-browser CLI not found"):
                 _find_agent_browser()
@@ -276,6 +277,7 @@ def capture_popen(cmd, **kwargs):
              patch("tools.browser_tool._get_session_info", return_value=fake_session), \
              patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \
              patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \
+             patch("tools.browser_tool._SANE_PATH_DIRS", ()), \
              patch("hermes_constants.Path.home", return_value=tmp_path), \
              patch("subprocess.Popen", side_effect=capture_popen), \
              patch("os.open", return_value=99), \
@@ -328,6 +330,7 @@ def capture_popen(cmd, **kwargs):
              patch("tools.browser_tool._get_session_info", return_value=fake_session), \
              patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \
              patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \
+             patch("tools.browser_tool._SANE_PATH_DIRS", ()), \
              patch("hermes_constants.Path.home", return_value=tmp_path), \
              patch("subprocess.Popen", side_effect=capture_popen), \
              patch("os.open", return_value=99), \
@@ -457,6 +460,7 @@ def selective_isdir(p):
              patch("tools.browser_tool._get_session_info", return_value=fake_session), \
              patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \
              patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \
+             patch("tools.browser_tool._SANE_PATH_DIRS", ()), \
              patch("os.path.isdir", side_effect=selective_isdir), \
              patch("subprocess.Popen", side_effect=capture_popen), \
              patch("os.open", return_value=99), \
@@ -467,8 +471,8 @@ def selective_isdir(p):
                 _run_browser_command("test-task", "navigate", ["https://example.com"])
 
         result_path = captured_env.get("PATH", "")
-        assert "/opt/homebrew/bin" not in result_path.split(os.pathsep)
-        assert "/opt/homebrew/sbin" not in result_path.split(os.pathsep)
+        assert "/opt/homebrew/bin" in result_path.split(os.pathsep)
+        assert "/opt/homebrew/sbin" in result_path.split(os.pathsep)
 
     def test_subprocess_path_includes_termux_fallback_dirs(self, tmp_path):
         """Termux fallback dirs should survive browser PATH rebuilding."""
@@ -506,6 +510,7 @@ def selective_isdir(path):
              patch("tools.browser_tool._get_session_info", return_value=fake_session), \
              patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \
              patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \
+             patch("tools.browser_tool._SANE_PATH_DIRS", ()), \
              patch("os.path.isdir", side_effect=selective_isdir), \
              patch("subprocess.Popen", side_effect=capture_popen), \
              patch("os.open", return_value=99), \

tests/tools/test_browser_ssrf_local.py

@@ -58,7 +58,7 @@ def test_cloud_blocks_private_url_by_default(self, monkeypatch, _common_patches)
         result = json.loads(browser_tool.browser_navigate(self.PRIVATE_URL))
 
         assert result["success"] is False
-        assert "private or internal address" in result["error"]
+        assert "private/internal address" in result["error"]
 
     def test_cloud_allows_private_url_when_setting_true(self, monkeypatch, _common_patches):
         """Private URLs pass in cloud mode when allow_private_urls is True."""
@@ -91,7 +91,7 @@ def test_local_blocks_private_url_by_default(self, monkeypatch, _common_patches)
         result = json.loads(browser_tool.browser_navigate(self.PRIVATE_URL))
 
         assert result["success"] is False
-        assert "private or internal address" in result["error"]
+        assert "private/internal address" in result["error"]
 
     def test_local_allows_private_url_when_setting_true(self, monkeypatch, _common_patches):
         """Local backends allow private URLs when allow_private_urls is True."""
@@ -112,7 +112,7 @@ def test_local_blocks_file_url_by_default(self, monkeypatch, _common_patches):
         result = json.loads(browser_tool.browser_navigate("file:///etc/passwd"))
 
         assert result["success"] is False
-        assert "private or internal address" in result["error"]
+        assert "private/internal address" in result["error"]
 
     def test_local_allows_public_url(self, monkeypatch, _common_patches):
         """Local backends pass public URLs too (sanity check)."""