diff --git a/gateway/platforms/api_server.py b/gateway/platforms/api_server.py index 0668896e170..1f02bde5a2a 100644 --- a/gateway/platforms/api_server.py +++ b/gateway/platforms/api_server.py @@ -35,6 +35,7 @@ import re import sqlite3 import time import uuid +from pathlib import Path from typing import Any, Dict, List, Optional try: @@ -337,10 +338,12 @@ class ResponseStore: db_path = str(get_hermes_home() / "response_store.db") except Exception: db_path = ":memory:" + self._db_path: Optional[str] = db_path if db_path != ":memory:" else None try: self._conn = sqlite3.connect(db_path, check_same_thread=False) except Exception: self._conn = sqlite3.connect(":memory:", check_same_thread=False) + self._db_path = None # Use shared WAL-fallback helper so response_store.db degrades # gracefully on NFS/SMB/FUSE-mounted HERMES_HOME (same filesystem # issue addressed for state.db/kanban.db — see @@ -361,6 +364,31 @@ class ResponseStore: )""" ) self._conn.commit() + # response_store.db contains conversation history (tool payloads, + # prompts, results). Tighten to owner-only after creation so other + # local users on a shared box can't read it. Run once at __init__ + # rather than after every commit — chmod-on-every-write is wasted + # syscalls on a hot path. + self._tighten_file_permissions() + + def _tighten_file_permissions(self) -> None: + """Force owner-only permissions on the DB and SQLite sidecars.""" + if not self._db_path: + return + for candidate in ( + Path(self._db_path), + Path(f"{self._db_path}-wal"), + Path(f"{self._db_path}-shm"), + ): + try: + if candidate.exists(): + candidate.chmod(0o600) + except OSError: + logger.debug( + "Failed to restrict response store permissions for %s", + candidate, + exc_info=True, + ) def get(self, response_id: str) -> Optional[Dict[str, Any]]: """Retrieve a stored response by ID (updates access time for LRU).""" diff --git a/hermes_cli/webhook.py b/hermes_cli/webhook.py index 621acc82e27..75470128707 100644 --- a/hermes_cli/webhook.py +++ b/hermes_cli/webhook.py @@ -11,8 +11,10 @@ hot-reloaded by the webhook adapter without a gateway restart. """ import json +import os import re import secrets +import tempfile import time from pathlib import Path from typing import Dict @@ -23,6 +25,7 @@ from hermes_cli.config import cfg_get _SUBSCRIPTIONS_FILENAME = "webhook_subscriptions.json" +_SUBSCRIPTIONS_FILE_MODE = 0o600 def _hermes_home() -> Path: @@ -48,12 +51,33 @@ def _load_subscriptions() -> Dict[str, dict]: def _save_subscriptions(subs: Dict[str, dict]) -> None: path = _subscriptions_path() path.parent.mkdir(parents=True, exist_ok=True) - tmp_path = path.with_suffix(".tmp") - tmp_path.write_text( - json.dumps(subs, indent=2, ensure_ascii=False), - encoding="utf-8", + # webhook_subscriptions.json contains per-route HMAC secrets — write + # via tempfile + chmod 0o600 before the atomic rename so a permissive + # umask cannot leave the secrets readable to other local users in the + # window between create and rename. + fd, tmp_name = tempfile.mkstemp( + prefix=f".{path.name}.", + suffix=".tmp", + dir=path.parent, + text=True, ) - atomic_replace(tmp_path, path) + tmp_path = Path(tmp_name) + try: + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump(subs, fh, indent=2, ensure_ascii=False) + fh.flush() + os.fsync(fh.fileno()) + os.chmod(tmp_path, _SUBSCRIPTIONS_FILE_MODE) + atomic_replace(tmp_path, path) + # Re-assert after rename in case the destination existed with a + # broader mode and atomic_replace preserved it. + os.chmod(path, _SUBSCRIPTIONS_FILE_MODE) + except Exception: + try: + tmp_path.unlink(missing_ok=True) + except OSError: + pass + raise def _get_webhook_config() -> dict: diff --git a/tests/gateway/test_api_server.py b/tests/gateway/test_api_server.py index aae5f550532..608385bef17 100644 --- a/tests/gateway/test_api_server.py +++ b/tests/gateway/test_api_server.py @@ -14,6 +14,8 @@ Tests cover: import asyncio import json +import os +import stat import time import uuid from unittest.mock import AsyncMock, MagicMock, patch @@ -128,6 +130,37 @@ class TestResponseStore: # resp_2 mapping should still be intact assert store.get_conversation("chat-b") == "resp_2" + @pytest.mark.skipif(os.name == "nt", reason="POSIX mode bits are platform-specific") + def test_file_store_created_owner_only_under_permissive_umask(self, tmp_path): + """response_store.db must be 0o600 on creation even under umask 022.""" + db_path = tmp_path / "response_store.db" + store = None + old_umask = os.umask(0o022) + try: + store = ResponseStore(max_size=10, db_path=str(db_path)) + store.put( + "resp_secret", + { + "response": {"id": "resp_secret"}, + "conversation_history": [{"role": "tool", "content": "dummy-marker"}], + }, + ) + finally: + os.umask(old_umask) + if store is not None: + store.close() + + assert stat.S_IMODE(db_path.stat().st_mode) == 0o600 + # WAL/SHM sidecars are owner-only too when present. WAL mode may be + # unavailable on some filesystems (NFS/SMB) — only assert when the + # sidecar files actually exist. + for sidecar in ( + db_path.with_name(db_path.name + "-wal"), + db_path.with_name(db_path.name + "-shm"), + ): + if sidecar.exists(): + assert stat.S_IMODE(sidecar.stat().st_mode) == 0o600 + # --------------------------------------------------------------------------- # _IdempotencyCache diff --git a/tests/hermes_cli/test_webhook_cli.py b/tests/hermes_cli/test_webhook_cli.py index 0094e917c54..8d3880722bb 100644 --- a/tests/hermes_cli/test_webhook_cli.py +++ b/tests/hermes_cli/test_webhook_cli.py @@ -3,6 +3,7 @@ import json import os import pytest +import stat from argparse import Namespace from pathlib import Path @@ -145,6 +146,31 @@ class TestPersistence: path.write_text("broken{{{") assert _load_subscriptions() == {} + @pytest.mark.skipif(os.name == "nt", reason="POSIX mode bits are platform-specific") + def test_save_creates_secret_file_owner_only_under_permissive_umask(self): + old_umask = os.umask(0o022) + try: + _save_subscriptions({"demo": {"secret": "TOPSECRET", "prompt": "x"}}) + finally: + os.umask(old_umask) + + path = _subscriptions_path() + assert stat.S_IMODE(path.stat().st_mode) == 0o600 + assert "TOPSECRET" in path.read_text(encoding="utf-8") + + @pytest.mark.skipif(os.name == "nt", reason="POSIX mode bits are platform-specific") + def test_save_narrows_existing_broad_secret_file_mode(self): + # Simulate a pre-existing 0o644 file from before this hardening landed. + path = _subscriptions_path() + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps({"old": {"secret": "stale", "prompt": "x"}})) + path.chmod(0o644) + + _save_subscriptions({"demo": {"secret": "FRESH", "prompt": "x"}}) + + assert stat.S_IMODE(path.stat().st_mode) == 0o600 + assert "FRESH" in path.read_text(encoding="utf-8") + class TestWebhookEnabledGate: def test_blocks_when_disabled(self, capsys, monkeypatch):