security(environments): protect modal/singularity snapshot stores with owner-only perms

agent/auxiliary_client.py

@@ -652,6 +652,17 @@ def _nous_extra_body() -> dict:
 _ANTHROPIC_DEFAULT_BASE_URL = "https://api.anthropic.com"
 _AUTH_JSON_PATH = get_hermes_home() / "auth.json"
 
+
+def _auth_json_path() -> Path:
+    """Return the active profile's auth.json path at call time.
+
+    Long-lived multi-profile runtimes (Dashboard/TUI/Desktop backend, cron)
+    import this module once and later bind different profiles.  Resolving
+    at call time ensures the live profile-scoped HERMES_HOME is always
+    respected (#40677).
+    """
+    return get_hermes_home() / "auth.json"
+
 # Codex OAuth endpoint used when a caller explicitly requests
 # provider="openai-codex".  There is deliberately no hardcoded default
 # model: the set of models OpenAI accepts on this endpoint for
@@ -1490,9 +1501,10 @@ def _read_nous_auth() -> Optional[dict]:
         }
 
     try:
-        if not _AUTH_JSON_PATH.is_file():
+        auth_path = _auth_json_path()
+        if not auth_path.is_file():
             return None
-        data = json.loads(_AUTH_JSON_PATH.read_text())
+        data = json.loads(auth_path.read_text())
         if data.get("active_provider") != "nous":
             return None
         provider = data.get("providers", {}).get("nous", {})

agent/skill_commands.py

@@ -22,6 +22,7 @@
 
 _skill_commands: Dict[str, Dict[str, Any]] = {}
 _skill_commands_platform: Optional[str] = None
+_skill_commands_skills_dir: Optional[Path] = None  # profile-scoped invalidation (#40677)
 # Patterns for sanitizing skill names into clean hyphen-separated slugs.
 _SKILL_INVALID_CHARS = re.compile(r"[^a-z0-9-]")
 _SKILL_MULTI_HYPHEN = re.compile(r"-{2,}")
@@ -323,19 +324,19 @@ def scan_skill_commands() -> Dict[str, Dict[str, Any]]:
     Returns:
         Dict mapping "/skill-name" to {name, description, skill_md_path, skill_dir}.
     """
-    global _skill_commands, _skill_commands_platform
+    global _skill_commands, _skill_commands_platform, _skill_commands_skills_dir
     _skill_commands_platform = _resolve_skill_commands_platform()
     _skill_commands = {}
     try:
-        from tools.skills_tool import SKILLS_DIR, _parse_frontmatter, skill_matches_platform, skill_matches_environment, _get_disabled_skill_names
+        from tools.skills_tool import _skills_dir, _parse_frontmatter, skill_matches_platform, skill_matches_environment, _get_disabled_skill_names
         from agent.skill_utils import get_external_skills_dirs, iter_skill_index_files
+        skills_root = _skills_dir()
+        _skill_commands_skills_dir = skills_root
         disabled = _get_disabled_skill_names()
         seen_names: set = set()
-
-        # Scan local dir first, then external dirs
         dirs_to_scan = []
-        if SKILLS_DIR.exists():
-            dirs_to_scan.append(SKILLS_DIR)
+        if skills_root.exists():
+            dirs_to_scan.append(skills_root)
         dirs_to_scan.extend(get_external_skills_dirs())
 
         for scan_dir in dirs_to_scan:
@@ -393,10 +394,19 @@ def get_skill_commands() -> Dict[str, Dict[str, Any]]:
     Rescans when the active platform scope changes (e.g. a gateway
     process serving Telegram and Discord concurrently) so each platform
     sees its own ``skills.platform_disabled`` view (#14536).
+    Also rescans when the active profile's skills directory changes
+    (e.g. a Dashboard/TUI/Desktop backend serving multiple profiles
+    from one long-lived process — #40677).
     """
+    try:
+        from tools.skills_tool import _skills_dir
+        current_skills_dir = _skills_dir()
+    except Exception:
+        current_skills_dir = None
     if (
         not _skill_commands
         or _skill_commands_platform != _resolve_skill_commands_platform()
+        or _skill_commands_skills_dir != current_skills_dir
     ):
         scan_skill_commands()
     return _skill_commands

agent/skill_utils.py

@@ -528,11 +528,14 @@ def normalize_skill_lookup_name(identifier: str) -> str:
     # (not via get_skills_dir()): callers and tests patch
     # ``tools.skills_tool.SKILLS_DIR`` and skill_view() itself resolves
     # against that module attribute, so normalization must agree with the
-    # exact root skill_view() will enforce.  Import deferred to avoid a
+    # exact root skill_view() will enforce.  Use _skills_dir() (not the
+    # module-level SKILLS_DIR) so the live profile-scoped HERMES_HOME is
+    # always respected in long-lived multi-profile runtimes (#40677).
+    # Import deferred to avoid a
     # module cycle (tools.skills_tool imports agent.skill_utils).
     try:
         from tools import skills_tool as _skills_tool
-        primary_root = Path(_skills_tool.SKILLS_DIR)
+        primary_root = Path(_skills_tool._skills_dir())
     except Exception:
         primary_root = get_skills_dir()
 

hermes_cli/web_server.py

@@ -11507,18 +11507,15 @@ def _profile_scope(profile: Optional[str]):
     1. ``load_config``/``save_config`` resolve ``get_hermes_home()`` at call
        time — the context-local override from ``set_hermes_home_override``
        reaches them (same pattern as ``_write_profile_model``).
-    2. ``tools.skills_tool`` and ``tools.skill_manager_tool`` bind
-       ``SKILLS_DIR`` at import time, so the override CANNOT reach them.
-       Like ``_call_cron_for_profile`` does for cron's module globals,
-       temporarily retarget both under a lock and restore them
-       immediately after.
+    2. ``tools.skills_tool`` and ``tools.skill_manager_tool`` resolve
+       ``SKILLS_DIR`` at call time via ``_skills_dir()`` (#60180), so the
+       ``set_hermes_home_override`` above is sufficient — no module-global
+       patching is needed.  The lock serializes concurrent skills-list
+       calls that may compete for the override.
 
     ``profile`` of None/""/"current" means "the dashboard's own profile" —
-    config resolution is untouched, but the skill-module globals are still
-    retargeted to the *current* ``get_hermes_home()`` so writes land in the
-    live home even when the import-time binding is stale (e.g. the process
-    imported the modules before a HERMES_HOME override, or under test
-    isolation).
+    config resolution is untouched; skills resolution uses the current
+    ``get_hermes_home()`` via ``_skills_dir()`` (#60180).
     """
     requested = (profile or "").strip()
 
@@ -11527,8 +11524,6 @@ def _profile_scope(profile: Optional[str]):
         set_hermes_home_override,
         reset_hermes_home_override,
     )
-    from tools import skills_tool as _skills_tool
-    from tools import skill_manager_tool as _skill_mgr
 
     token = None
     if not requested or requested.lower() == "current":
@@ -11538,21 +11533,9 @@ def _profile_scope(profile: Optional[str]):
         token = set_hermes_home_override(str(profile_dir))
 
     with _SKILLS_PROFILE_LOCK:
-        old_home = _skills_tool.HERMES_HOME
-        old_skills_dir = _skills_tool.SKILLS_DIR
-        old_mgr_home = _skill_mgr.HERMES_HOME
-        old_mgr_skills_dir = _skill_mgr.SKILLS_DIR
-        _skills_tool.HERMES_HOME = profile_dir
-        _skills_tool.SKILLS_DIR = profile_dir / "skills"
-        _skill_mgr.HERMES_HOME = profile_dir
-        _skill_mgr.SKILLS_DIR = profile_dir / "skills"
         try:
             yield profile_dir if token is not None else None
         finally:
-            _skills_tool.HERMES_HOME = old_home
-            _skills_tool.SKILLS_DIR = old_skills_dir
-            _skill_mgr.HERMES_HOME = old_mgr_home
-            _skill_mgr.SKILLS_DIR = old_mgr_skills_dir
             if token is not None:
                 reset_hermes_home_override(token)
 
@@ -11562,13 +11545,10 @@ def _config_profile_scope(profile: Optional[str]):
     """Await-safe, config-only profile scope for handlers that ``await``.
 
     Unlike ``_profile_scope`` this touches ONLY the context-local
-    ``set_hermes_home_override`` contextvar — it does NOT swap the
-    process-global ``skills_tool``/``skill_manager`` module attributes.
-    Those globals are shared across all event-loop tasks, so holding them
-    across an ``await`` lets a concurrent skills request restore THIS
-    request's profile dir on its ``finally`` (cross-contamination). The
-    contextvar override is task-local and survives an ``await`` cleanly,
-    which is all endpoints that resolve ``get_hermes_home()`` at call time
+    ``set_hermes_home_override`` contextvar — it does NOT swap
+    process-global module attributes.  Since ``_skills_dir()`` resolves
+    at call time (#60180), the contextvar override alone is sufficient
+    for all endpoints that resolve ``get_hermes_home()`` at call time.
     (config, env, gateway status) actually need.
 
     None/""/"current" means the dashboard's own profile — no override.

plugins/platforms/discord/adapter.py

@@ -2464,7 +2464,9 @@ async def send_multiple_images(
                                         self.name, resp.status, image_url[:80],
                                     )
                                     continue
-                                data = await resp.read()
+                                data = await _read_response_bytes_bounded(
+                                    resp, _DISCORD_IMAGE_DOWNLOAD_MAX_BYTES
+                                )
                                 ct = resp.headers.get("content-type", "image/png")
                                 ext = "png"
                                 if "jpeg" in ct or "jpg" in ct:
@@ -3552,7 +3554,9 @@ async def send_image(
                     if resp.status != 200:
                         raise Exception(f"Failed to download image: HTTP {resp.status}")
 
-                    image_data = await resp.read()
+                    image_data = await _read_response_bytes_bounded(
+                        resp, _DISCORD_IMAGE_DOWNLOAD_MAX_BYTES
+                    )
 
                     # Determine filename from URL or content type
                     content_type = resp.headers.get("content-type", "image/png")
@@ -3631,7 +3635,9 @@ async def send_animation(
                     if resp.status != 200:
                         raise Exception(f"Failed to download animation: HTTP {resp.status}")
 
-                    animation_data = await resp.read()
+                    animation_data = await _read_response_bytes_bounded(
+                        resp, _DISCORD_IMAGE_DOWNLOAD_MAX_BYTES
+                    )
 
                     import io
                     file = discord.File(io.BytesIO(animation_data), filename="animation.gif")
@@ -5796,7 +5802,9 @@ async def _cache_discord_document(self, att, ext: str) -> bytes:
             ) as resp:
                 if resp.status != 200:
                     raise Exception(f"HTTP {resp.status}")
-                return await resp.read()
+                return await _read_response_bytes_bounded(
+                    resp, _DISCORD_ATTACHMENT_DOWNLOAD_MAX_BYTES
+                )
 
     async def _handle_message(self, message: DiscordMessage, role_authorized: bool = False) -> None:
         """Handle incoming Discord messages."""
@@ -7436,12 +7444,25 @@ async def on_timeout(self):
 _DISCORD_CHANNEL_TYPE_PROBE_CACHE: Dict[str, bool] = {}
 _DISCORD_STANDALONE_JSON_BODY_LIMIT_BYTES = 1 * 1024 * 1024
 _DISCORD_STANDALONE_ERROR_BODY_LIMIT_BYTES = 8 * 1024
+_DISCORD_IMAGE_DOWNLOAD_MAX_BYTES = 50 * 1024 * 1024  # generous limit for images/animations
+_DISCORD_ATTACHMENT_DOWNLOAD_MAX_BYTES = 100 * 1024 * 1024  # generous for file attachments
 
 
 def _remember_channel_is_forum(chat_id: str, is_forum: bool) -> None:
     _DISCORD_CHANNEL_TYPE_PROBE_CACHE[str(chat_id)] = bool(is_forum)
 
 
+async def _read_response_bytes_bounded(resp: Any, limit_bytes: int) -> bytes:
+    """Read at most *limit_bytes* from an aiohttp response, raising on overflow."""
+    data = await resp.content.read(limit_bytes + 1)
+    if len(data) > limit_bytes:
+        resp.close()
+        raise ValueError(
+            f"Response body exceeded {limit_bytes} bytes ({len(data)} bytes read)"
+        )
+    return data
+
+
 def _probe_is_forum_cached(chat_id: str) -> Optional[bool]:
     return _DISCORD_CHANNEL_TYPE_PROBE_CACHE.get(str(chat_id))
 

tests/gateway/test_discord_image_download_bounds.py

@@ -0,0 +1,73 @@
+"""Tests for bounded HTTP response reads in Discord image/download paths.
+
+Companion to #60122, #60112 (REST body bounding) — extends the same
+resource-limiting pattern to image/animation/attachment downloads
+in the Discord adapter that were left unbounded.
+"""
+
+import pytest
+from unittest.mock import AsyncMock, MagicMock, patch
+
+from plugins.platforms.discord.adapter import (
+    _read_response_bytes_bounded,
+    _DISCORD_IMAGE_DOWNLOAD_MAX_BYTES,
+    _DISCORD_ATTACHMENT_DOWNLOAD_MAX_BYTES,
+)
+
+
+class AsyncContextManagerMock:
+    """Mock that supports ``async with``."""
+
+    def __init__(self, return_value):
+        self._return_value = return_value
+
+    async def __aenter__(self):
+        return self._return_value
+
+    async def __aexit__(self, *args):
+        pass
+
+
+class TestReadResponseBytesBounded:
+    def test_reads_within_limit(self):
+        resp = MagicMock()
+        resp.content = MagicMock()
+        resp.content.read = AsyncMock(return_value=b"x" * 100)
+
+        result = pytest.run_sync(
+            _read_response_bytes_bounded(resp, 200)
+        )
+        assert result == b"x" * 100
+        resp.content.read.assert_awaited_once_with(201)
+
+    def test_raises_on_overflow(self):
+        resp = MagicMock()
+        resp.content = MagicMock()
+        resp.content.read = AsyncMock(return_value=b"x" * 101)
+        resp.close = MagicMock()
+
+        with pytest.raises(ValueError, match="exceeded 100 bytes"):
+            pytest.run_sync(
+                _read_response_bytes_bounded(resp, 100)
+            )
+        resp.close.assert_called_once()
+
+    def test_exact_limit_passes(self):
+        resp = MagicMock()
+        resp.content = MagicMock()
+        resp.content.read = AsyncMock(return_value=b"x" * 100)
+
+        result = pytest.run_sync(
+            _read_response_bytes_bounded(resp, 100)
+        )
+        assert result == b"x" * 100
+
+
+class TestImageDownloadLimits:
+    def test_constants_are_reasonable(self):
+        assert _DISCORD_IMAGE_DOWNLOAD_MAX_BYTES == 50 * 1024 * 1024
+        assert _DISCORD_ATTACHMENT_DOWNLOAD_MAX_BYTES == 100 * 1024 * 1024
+
+    def test_image_limit_greater_than_zero(self):
+        assert _DISCORD_IMAGE_DOWNLOAD_MAX_BYTES > 0
+        assert _DISCORD_ATTACHMENT_DOWNLOAD_MAX_BYTES > 0

tests/tools/test_environment_snapshot_security.py

@@ -0,0 +1,37 @@
+"""Tests for owner-only permissions on JSON snapshot stores (#60259)."""
+
+import json
+import os
+import tempfile
+from pathlib import Path
+
+import pytest
+
+from tools.environments.base import _save_json_store, _load_json_store
+
+
+class TestSaveJsonStorePermissions:
+    def test_writes_with_owner_only_perms(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            path = Path(tmp) / "test_store.json"
+            _save_json_store(path, {"key": "value"})
+            assert path.exists()
+            st = path.stat()
+            if os.name != "nt":
+                assert (st.st_mode & 0o777) == 0o600
+
+    def test_umask_is_restored_after_write(self):
+        old_umask = os.umask(0o022)
+        os.umask(old_umask)
+        with tempfile.TemporaryDirectory() as tmp:
+            path = Path(tmp) / "test_store.json"
+            _save_json_store(path, {"x": 1})
+        restored = os.umask(0o022)
+        os.umask(restored)
+        assert restored == old_umask
+
+    def test_creates_parent_directories(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            path = Path(tmp) / "sub" / "nested" / "store.json"
+            _save_json_store(path, {})
+            assert path.exists()

tools/environments/base.py

@@ -167,11 +167,13 @@ def _load_json_store(path: Path) -> dict:
 
 
 def _save_json_store(path: Path, data: dict) -> None:
-    """Write *data* as pretty-printed JSON to *path*."""
+    """Write *data* as pretty-printed JSON to *path* with owner-only perms."""
     path.parent.mkdir(parents=True, exist_ok=True)
-    path.write_text(json.dumps(data, indent=2), encoding="utf-8")
-
-
+    old_umask = os.umask(0o077)
+    try:
+        path.write_text(json.dumps(data, indent=2), encoding="utf-8")
+    finally:
+        os.umask(old_umask)
 def _file_mtime_key(host_path: str) -> tuple[float, int] | None:
     """Return ``(mtime, size)`` for cache comparison, or ``None`` if unreadable."""
     try: