Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions gateway/platforms/api_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import sqlite3
import time
import uuid
from pathlib import Path
from typing import Any, Dict, List, Optional

try:
Expand Down Expand Up @@ -337,10 +338,12 @@ def __init__(self, max_size: int = MAX_STORED_RESPONSES, db_path: str = None):
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
Expand All @@ -361,6 +364,31 @@ def __init__(self, max_size: int = MAX_STORED_RESPONSES, db_path: str = None):
)"""
)
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)."""
Expand Down
34 changes: 29 additions & 5 deletions hermes_cli/webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
"""

import json
import os
import re
import secrets
import tempfile
import time
from pathlib import Path
from typing import Dict
Expand All @@ -23,6 +25,7 @@


_SUBSCRIPTIONS_FILENAME = "webhook_subscriptions.json"
_SUBSCRIPTIONS_FILE_MODE = 0o600


def _hermes_home() -> Path:
Expand All @@ -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:
Expand Down
33 changes: 33 additions & 0 deletions tests/gateway/test_api_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

import asyncio
import json
import os
import stat
import time
import uuid
from unittest.mock import AsyncMock, MagicMock, patch
Expand Down Expand Up @@ -128,6 +130,37 @@ def test_eviction_clears_conversation_mapping(self):
# 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
Expand Down
26 changes: 26 additions & 0 deletions tests/hermes_cli/test_webhook_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import json
import os
import pytest
import stat
from argparse import Namespace
from pathlib import Path

Expand Down Expand Up @@ -145,6 +146,31 @@ def test_corrupted_file(self):
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):
Expand Down
Loading