fix(security): salvage six P1 hardening PRs — browser guards, MEDIA anchoring, .env lockdown, delegate ACP transport, matrix sync isolation

gateway/platforms/api_server.py

@@ -54,9 +54,11 @@
 
 from gateway.config import Platform, PlatformConfig
 from gateway.platforms.base import (
+    MEDIA_TAG_CLEANUP_RE,
     BasePlatformAdapter,
     SendResult,
     is_network_accessible,
+    validate_media_delivery_path,
 )
 from agent.redact import redact_sensitive_text
 
@@ -581,9 +583,6 @@ async def cors_middleware(request, handler):
     ".webp": "image/webp",
     ".bmp": "image/bmp",
 }
-_MEDIA_TAG_RE = re.compile(
-    r"[`\"']?MEDIA:\s*(`[^`\n]+`|\"[^\"\n]+\"|'[^'\n]+'|\S+)[`\"']?"
-)
 _MEDIA_DATA_URL_MAX_BYTES = 5 * 1024 * 1024  # skip images larger than 5MB
 
 
@@ -594,29 +593,46 @@ def _resolve_media_to_data_urls(text: str) -> str:
     ``MEDIA:`` tags referencing images on the server are useless to them.
     Inline small local images as markdown data URLs; non-image or unreadable
     paths are left untouched.
+
+    Uses the same anchored ``MEDIA_TAG_CLEANUP_RE`` matcher and
+    ``validate_media_delivery_path`` safety check every other platform
+    adapter's media delivery already goes through (gateway/platforms/base.py)
+    — an absolute-path anchor plus a known-extension requirement, and a
+    resolved-path check against the credential/system-path denylist. The
+    prior pattern here matched any bare token after ``MEDIA:`` (including a
+    relative/traversal path like ``../../etc/passwd.png``) and read the file
+    directly with no denylist, so any image-suffixed, readable file the
+    process could see was base64-exfiltrated to the API caller if its path
+    merely appeared in the model's own final reply text.
     """
     if not text or "MEDIA:" not in text:
         return text
     import base64
 
     def _to_data_url(path_str: str) -> Optional[str]:
-        p = Path(path_str.strip().strip("`\"'")).expanduser()
+        # validate_media_delivery_path() strips wrapping quotes/backticks
+        # and trailing punctuation internally, same as MEDIA_TAG_CLEANUP_RE's
+        # other callers (extract_media / _strip_media_tag_directives) rely on.
+        safe_path = validate_media_delivery_path(path_str)
+        if not safe_path:
+            return None
+        p = Path(safe_path)
         suffix = p.suffix.lower()
         if suffix not in _MEDIA_IMG_EXT:
             return None
         try:
-            if not p.is_file() or p.stat().st_size > _MEDIA_DATA_URL_MAX_BYTES:
+            if p.stat().st_size > _MEDIA_DATA_URL_MAX_BYTES:
                 return None
             b64 = base64.b64encode(p.read_bytes()).decode()
         except OSError:
             return None
         return f"![image](data:{_MEDIA_MIME[suffix]};base64,{b64})"
 
     def _repl(m: "re.Match[str]") -> str:
-        return _to_data_url(m.group(1)) or m.group(0)
+        return _to_data_url(m.group("path")) or m.group(0)
 
     try:
-        return _MEDIA_TAG_RE.sub(_repl, text)
+        return MEDIA_TAG_CLEANUP_RE.sub(_repl, text)
     except Exception:
         return text
 

hermes_cli/web_server.py

@@ -1186,6 +1186,19 @@ class ManagedFilesPolicy:
     "target",
     "venv",
 }
+
+# Filenames that must never be listed, read, or downloaded through the
+# managed-files API.  These typically contain credentials (API keys, tokens)
+# and exposing them through the dashboard file browser is a security leak —
+# see issue #57505.
+def _is_sensitive_filename(name: str) -> bool:
+    """Return True for ``.env`` and any ``.env.<suffix>`` variant.
+
+    Case-insensitive so ``.ENV`` / ``.Env.local`` on case-insensitive
+    filesystems (macOS/Windows mounts) can't slip past the guard.
+    """
+    lowered = name.lower()
+    return lowered == ".env" or lowered.startswith(".env.")
 _FS_DATA_URL_MAX_BYTES = 16 * 1024 * 1024
 _FS_TEXT_SOURCE_MAX_BYTES = 64 * 1024 * 1024
 _FS_TEXT_PREVIEW_MAX_BYTES = 512 * 1024
@@ -1616,7 +1629,11 @@ async def list_managed_files(request: Request, path: Optional[str] = None):
         raise HTTPException(status_code=400, detail="Path is not a directory")
 
     try:
-        entries = [_managed_file_entry(policy, child) for child in target.iterdir()]
+        entries = [
+            _managed_file_entry(policy, child)
+            for child in target.iterdir()
+            if not _is_sensitive_filename(child.name)
+        ]
     except PermissionError:
         raise HTTPException(status_code=403, detail="Directory is not readable")
     except OSError as exc:
@@ -1642,6 +1659,8 @@ async def read_managed_file(request: Request, path: str):
         raise HTTPException(status_code=404, detail="File not found")
     if not target.is_file():
         raise HTTPException(status_code=400, detail="Path is not a file")
+    if _is_sensitive_filename(target.name):
+        raise HTTPException(status_code=403, detail="Access to sensitive files is not allowed")
 
     try:
         size = target.stat().st_size
@@ -1684,6 +1703,8 @@ async def download_managed_file(request: Request, path: str):
         raise HTTPException(status_code=404, detail="File not found")
     if not target.is_file():
         raise HTTPException(status_code=400, detail="Path is not a file")
+    if _is_sensitive_filename(target.name):
+        raise HTTPException(status_code=403, detail="Access to sensitive files is not allowed")
 
     try:
         size = target.stat().st_size

plugins/platforms/matrix/adapter.py

@@ -2351,7 +2351,18 @@ async def _dispatch_sync(self, sync_data: Dict[str, Any]) -> None:
         if inspect.isawaitable(tasks):
             tasks = await tasks
         if tasks:
-            await asyncio.gather(*tasks)
+            # return_exceptions=True so one failing event handler doesn't abort
+            # the whole gather and silently drop the SIBLING events in the same
+            # sync response (a bare gather re-raises the first exception, leaving
+            # the rest of the batch unprocessed). Mirrors the invite/redaction
+            # gathers above. Surface each failure instead of swallowing it.
+            results = await asyncio.gather(*tasks, return_exceptions=True)
+            for result in results:
+                if isinstance(result, Exception):
+                    logger.warning(
+                        "Matrix: event handler failed during sync dispatch: %s",
+                        result,
+                    )
 
     def _is_self_sender(self, sender: str) -> bool:
         """Return True if the sender refers to the bot's own account.

run_agent.py

@@ -5623,7 +5623,10 @@ def _dispatch_delegate_task(self, function_args: dict) -> str:
         New DELEGATE_TASK_SCHEMA fields only need to be added here to reach all
         invocation paths (concurrent, sequential, inline).
         """
-        from tools.delegate_tool import delegate_task as _delegate_task
+        from tools.delegate_tool import (
+            _strip_model_hidden_task_fields,
+            delegate_task as _delegate_task,
+        )
         # Delegations from the top-level MODEL always run in the background —
         # the model does not get to choose. delegate_task returns immediately
         # with a handle (one per task) and each subagent's result re-enters the
@@ -5639,10 +5642,8 @@ def _dispatch_delegate_task(self, function_args: dict) -> str:
         return _delegate_task(
             goal=function_args.get("goal"),
             context=function_args.get("context"),
-            tasks=function_args.get("tasks"),
+            tasks=_strip_model_hidden_task_fields(function_args.get("tasks")),
             max_iterations=function_args.get("max_iterations"),
-            acp_command=function_args.get("acp_command"),
-            acp_args=function_args.get("acp_args"),
             role=function_args.get("role"),
             background=(not _is_subagent),
             parent_agent=self,

tests/gateway/test_api_server_media_data_urls.py

@@ -72,6 +72,40 @@ def test_multiple_tags(self):
         out = _resolve_media_to_data_urls(f"MEDIA:{p1}\nand MEDIA:{p2}")
         self.assertEqual(out.count("data:image/png;base64,"), 2)
 
+    def test_relative_traversal_path_not_inlined(self):
+        """A relative/traversal path must never be inlined — the anchored
+        MEDIA_TAG_CLEANUP_RE matcher requires an absolute-path prefix
+        (~/, /, or a Windows drive letter), so a bare relative token after
+        MEDIA: is left as literal text rather than resolved against cwd."""
+        text = "MEDIA:../../../../etc/passwd.png"
+        self.assertEqual(_resolve_media_to_data_urls(text), text)
+
+    def test_credential_path_not_inlined_even_with_image_extension(self):
+        """An absolute path under the credential/system-path denylist
+        (validate_media_delivery_path) must not be inlined even though it
+        has an allowed image extension and the tag matcher's shape."""
+        text = "MEDIA:~/.ssh/id_rsa.png"
+        self.assertEqual(_resolve_media_to_data_urls(text), text)
+
+    def test_symlink_escaping_to_denylisted_target_not_inlined(self):
+        """A symlink whose resolved target lands under a denylisted system
+        prefix (/etc) must not be inlined — validate_media_delivery_path
+        resolves symlinks before the containment/denylist check runs, so
+        the traversal can't be laundered through an innocuous-looking
+        image-suffixed symlink name."""
+        import os
+        import tempfile
+        from pathlib import Path
+
+        d = Path(tempfile.mkdtemp(prefix="hermes_media_test_symlink"))
+        link = d / "shot.png"
+        try:
+            os.symlink("/etc/hosts", link)
+        except OSError:
+            self.skipTest("symlink creation not supported in this environment")
+        text = f"MEDIA:{link}"
+        self.assertEqual(_resolve_media_to_data_urls(text), text)
+
 
 if __name__ == "__main__":
     unittest.main()

tests/gateway/test_matrix.py

@@ -5276,3 +5276,36 @@ async def test_flag_clears_when_second_connect_resolves_device_id(self):
         assert None not in _verify_call.args[0]["@bot:example.org"]
 
         await adapter.disconnect()
+
+
+class TestMatrixDispatchSyncIsolation:
+    """A failing mautrix event handler must not abort the whole sync batch.
+
+    ``_dispatch_sync`` gathers the per-event handler tasks. Without
+    ``return_exceptions=True`` the first exception aborts the gather and the
+    sibling events in the same sync response are silently dropped.
+    """
+
+    @pytest.mark.asyncio
+    async def test_dispatch_sync_isolates_failing_handler(self, caplog):
+        import logging
+
+        adapter = _make_adapter()
+        ran = {"ok": False}
+
+        async def _boom():
+            raise RuntimeError("handler boom")
+
+        async def _ok():
+            ran["ok"] = True
+
+        client = MagicMock()
+        client.handle_sync = MagicMock(return_value=[_boom(), _ok()])
+        adapter._client = client
+
+        with caplog.at_level(logging.WARNING):
+            # Must not raise despite the failing handler.
+            await adapter._dispatch_sync({"next_batch": "s1"})
+
+        assert ran["ok"] is True  # the sibling handler still ran
+        assert "event handler failed" in caplog.text  # failure surfaced, not swallowed

tests/hermes_cli/test_web_server_files.py

@@ -488,3 +488,75 @@ async def close(self):
     # ... and no .upload temp file was left behind.
     leftovers = [p.name for p in target.parent.iterdir() if ".upload" in p.name]
     assert leftovers == [], f"temp upload files leaked on cancellation: {leftovers}"
+
+
+def test_sensitive_env_files_hidden_from_listing(forced_files_client):
+    """Regression test for #57505: .env files must not appear in directory listings."""
+    client, root = forced_files_client
+
+    # Create a regular file and .env variants including shorthand suffixes.
+    root.mkdir(parents=True, exist_ok=True)
+    regular = root / "config.txt"
+    regular.write_text("safe content")
+    env_file = root / ".env"
+    env_file.write_text("SECRET_KEY=abc123")
+    env_local = root / ".env.local"
+    env_local.write_text("LOCAL_SECRET=def456")
+    env_prod = root / ".env.prod"
+    env_prod.write_text("PROD_SECRET=ghi789")
+
+    listing = client.get("/api/files", params={"path": str(root)})
+    assert listing.status_code == 200
+    names = [e["name"] for e in listing.json()["entries"]]
+    assert "config.txt" in names
+    assert ".env" not in names
+    assert ".env.local" not in names
+    assert ".env.prod" not in names
+
+
+def test_sensitive_env_files_blocked_read(forced_files_client):
+    """Regression test for #57505: .env files must not be readable."""
+    client, root = forced_files_client
+
+    root.mkdir(parents=True, exist_ok=True)
+    env_file = root / ".env"
+    env_file.write_text("SECRET_KEY=abc123")
+
+    resp = client.get("/api/files/read", params={"path": str(env_file)})
+    assert resp.status_code == 403
+
+
+def test_sensitive_env_files_blocked_download(forced_files_client):
+    """Regression test for #57505: .env files must not be downloadable."""
+    client, root = forced_files_client
+
+    root.mkdir(parents=True, exist_ok=True)
+    env_file = root / ".env"
+    env_file.write_text("SECRET_KEY=abc123")
+
+    resp = client.get("/api/files/download", params={"path": str(env_file)})
+    assert resp.status_code == 403
+
+
+def test_sensitive_env_suffix_variants_blocked(forced_files_client):
+    """Regression: .env.<suffix> shorthand variants (e.g. .env.prod) must also be blocked."""
+    client, root = forced_files_client
+
+    root.mkdir(parents=True, exist_ok=True)
+    for suffix in ("prod", "dev", "staging.local", "ci"):
+        p = root / f".env.{suffix}"
+        p.write_text(f"SECRET_{suffix}=abc123")
+        assert client.get("/api/files/read", params={"path": str(p)}).status_code == 403
+        assert client.get("/api/files/download", params={"path": str(p)}).status_code == 403
+
+
+def test_sensitive_env_case_insensitive_blocked(forced_files_client):
+    """Regression: .ENV / .Env.local casings must be blocked too (case-insensitive FS mounts)."""
+    client, root = forced_files_client
+
+    root.mkdir(parents=True, exist_ok=True)
+    for name in (".ENV", ".Env.local", ".eNv.PROD"):
+        p = root / name
+        p.write_text("SECRET=abc123")
+        assert client.get("/api/files/read", params={"path": str(p)}).status_code == 403
+        assert client.get("/api/files/download", params={"path": str(p)}).status_code == 403

tests/tools/test_browser_camofox_private_page_guard.py

@@ -83,6 +83,34 @@ def fail_http(*_args, **_kwargs):
     assert action_phrase in out["error"]
 
 
+@pytest.mark.parametrize(
+    ("tool_call", "action_phrase"),
+    [
+        (lambda: browser_camofox.camofox_click("@e1", task_id="t1"), "click"),
+        (
+            lambda: browser_camofox.camofox_type("@e1", "do-not-send-this", task_id="t1"),
+            "type",
+        ),
+        (lambda: browser_camofox.camofox_press("Enter", task_id="t1"), "press"),
+    ],
+)
+def test_private_page_blocks_camofox_input_actions(monkeypatch, _session, tool_call, action_phrase):
+    _block_active(monkeypatch)
+
+    def fail_post(*_args, **_kwargs):
+        raise AssertionError("Camofox action HTTP call should not run on a private page")
+
+    monkeypatch.setattr(browser_camofox, "_post", fail_post)
+
+    out = json.loads(tool_call())
+
+    assert out["success"] is False
+    assert PRIVATE_URL in out["error"]
+    assert "private or internal address" in out["error"]
+    assert action_phrase in out["error"]
+    assert "do-not-send-this" not in json.dumps(out)
+
+
 def test_snapshot_still_runs_when_page_is_public(monkeypatch, _session):
     _public_page(monkeypatch)
 
@@ -98,6 +126,29 @@ def test_snapshot_still_runs_when_page_is_public(monkeypatch, _session):
     assert out["element_count"] == 1
 
 
+def test_camofox_click_still_runs_when_page_is_public(monkeypatch, _session):
+    _public_page(monkeypatch)
+    calls = []
+
+    def fake_post(path, body=None, timeout=None):
+        calls.append((path, body, timeout))
+        return {"url": "https://example.test/"}
+
+    monkeypatch.setattr(browser_camofox, "_post", fake_post)
+
+    out = json.loads(browser_camofox.camofox_click("@e1", task_id="t1"))
+
+    assert out["success"] is True
+    assert out["clicked"] == "e1"
+    assert calls == [
+        (
+            "/tabs/tab-1/click",
+            {"userId": "user-1", "ref": "e1"},
+            None,
+        )
+    ]
+
+
 def test_guard_inactive_does_not_probe(monkeypatch, _session):
     """When the SSRF guard is inactive the read proceeds WITHOUT probing the URL.