From 7f7d229f83c3a513244f33297f564d66bbbc8e27 Mon Sep 17 00:00:00 2001 From: Konstantin Khlopkov Date: Wed, 16 Sep 2026 07:46:07 +0300 Subject: [PATCH] fix(utils): atomic writers refuse to resurrect a deleted named profile home Background writers that still carry a tombstoned profile as their Hermes home (reasoning-caps warm thread, models cache, models.dev ETag, gateway lifecycle ledger, MCP OAuth tokens, memory store) re-created profiles// with a bare mkdir right before an atomic write. Route the parent-dir creation through mkdir_under_hermes_home so a deleted named profile raises FileNotFoundError and stays gone, matching the tombstone contract already enforced for logging and state. --- agent/models_dev.py | 4 +- agent/secret_sources/_cache.py | 4 +- gateway/lifecycle_ledger.py | 8 +- .../test_atomic_writers_deleted_profile.py | 158 ++++++++++++++++++ tools/mcp_oauth.py | 16 +- tools/memory_tool_store.py | 12 +- 6 files changed, 191 insertions(+), 11 deletions(-) create mode 100644 tests/utils/test_atomic_writers_deleted_profile.py diff --git a/agent/models_dev.py b/agent/models_dev.py index fd40784b9b..6b6b4d1e50 100644 --- a/agent/models_dev.py +++ b/agent/models_dev.py @@ -189,7 +189,9 @@ def _load_etag() -> str: def _save_etag(etag: str) -> None: def write() -> None: etag_path = _get_etag_path() - etag_path.parent.mkdir(parents=True, exist_ok=True) + from hermes_constants import mkdir_under_hermes_home + + mkdir_under_hermes_home(etag_path.parent) atomic_write_text(etag_path, etag) _quietly("save models.dev ETag", write) diff --git a/agent/secret_sources/_cache.py b/agent/secret_sources/_cache.py index fab398b2dd..7e24f7a5c1 100644 --- a/agent/secret_sources/_cache.py +++ b/agent/secret_sources/_cache.py @@ -73,7 +73,9 @@ def atomic_write_json(path: Path, payload: dict) -> None: """Secret cache entry at 0600 from creation; the containing dir is tightened to 0700 (``secure_parent_dir`` refuses ``/``, top-level dirs and the install tree). Raises ``OSError`` on failure; callers decide whether that is best-effort.""" - path.parent.mkdir(parents=True, exist_ok=True) + from hermes_constants import mkdir_under_hermes_home + + mkdir_under_hermes_home(path.parent) secure_parent_dir(path) atomic_json_write(path, payload, indent=None, mode=0o600) diff --git a/gateway/lifecycle_ledger.py b/gateway/lifecycle_ledger.py index a92968cbd5..8eb0364ab0 100644 --- a/gateway/lifecycle_ledger.py +++ b/gateway/lifecycle_ledger.py @@ -87,7 +87,9 @@ def _write_sentinel(payload: Dict[str, Any], home: Optional[Path]) -> None: from utils import atomic_json_write path = get_lifecycle_sentinel_path(home) - path.parent.mkdir(parents=True, exist_ok=True) + from hermes_constants import mkdir_under_hermes_home + + mkdir_under_hermes_home(path.parent) atomic_json_write(path, payload, indent=None) except Exception: logger.debug("Failed to write lifecycle sentinel", exc_info=True) @@ -97,7 +99,9 @@ def _append_exit_diag(record: Dict[str, Any], home: Optional[Path]) -> None: """Append a JSON line to gateway-exit-diag.log (same format as the CLI's ``_exit_diag``).""" try: path = _home_path(home, "logs", "gateway-exit-diag.log") - path.parent.mkdir(parents=True, exist_ok=True) + from hermes_constants import mkdir_under_hermes_home + + mkdir_under_hermes_home(path.parent) with path.open("a", encoding="utf-8") as fh: fh.write(json.dumps(record, default=str) + "\n") except OSError: diff --git a/tests/utils/test_atomic_writers_deleted_profile.py b/tests/utils/test_atomic_writers_deleted_profile.py new file mode 100644 index 0000000000..95daee405d --- /dev/null +++ b/tests/utils/test_atomic_writers_deleted_profile.py @@ -0,0 +1,158 @@ +"""Atomic writers must not resurrect a deleted named profile home. + +``hermes profile delete`` removes the tree and writes a tombstone under +``profiles/.deleted/``. Background writers that still carry the dead +profile as their Hermes home (reasoning-caps warm thread, models.dev refresh, +gateway lifecycle ledger, MCP OAuth token writes, memory store mutations) used +to re-create ``profiles//`` with a bare ``mkdir(parents=True)`` right +before an atomic write — the exact resurrection class the tombstone guard +closed for logging and state. These tests lock the same contract for the +writers themselves: a tombstoned home raises ``FileNotFoundError`` and leaves +nothing on disk, while unrelated paths with a ``profiles`` path segment keep +working. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from hermes_constants import ( + mark_named_profile_deleted, + named_profile_home, + set_hermes_home_override, +) +from utils import atomic_json_write, atomic_write_text + + +def _tombstoned_profile(tmp_path: Path) -> Path: + """A real ``/profiles/`` home that has just been deleted.""" + (tmp_path / "config.yaml").write_text("{}\n", encoding="utf-8") + profile = tmp_path / "profiles" / "p1" + profile.mkdir(parents=True) + mark_named_profile_deleted(profile) + import shutil + + shutil.rmtree(profile) + assert named_profile_home(profile) is not None + assert not profile.exists() + return profile + + +class TestAtomicWritersRefuseDeletedProfileHome: + def test_atomic_json_write_does_not_recreate_home(self, tmp_path): + profile = _tombstoned_profile(tmp_path) + with pytest.raises( + FileNotFoundError, match="Named profile home does not exist" + ): + atomic_json_write(profile / "cache" / "reasoning_caps.json", {"m": {}}) + assert not profile.exists() + + def test_atomic_write_text_does_not_recreate_home(self, tmp_path): + profile = _tombstoned_profile(tmp_path) + with pytest.raises( + FileNotFoundError, match="Named profile home does not exist" + ): + atomic_write_text(profile / "cache" / "models-dev-etag", "etag") + assert not profile.exists() + + def test_late_reasoning_caps_save_after_delete(self, tmp_path): + from hermes_cli import models_reasoning_caps + + profile = _tombstoned_profile(tmp_path) + token = set_hermes_home_override(profile) + try: + models_reasoning_caps._save_reasoning_caps_disk( + "https://example/v1/models", {"m": {"supports_reasoning": True}} + ) + finally: + from hermes_constants import reset_hermes_home_override + + reset_hermes_home_override(token) + assert not (profile / "cache" / "reasoning_caps.json").exists() + assert not profile.exists() + + def test_late_models_cache_save_after_delete(self, tmp_path): + from hermes_cli.models import _write_json_cache + + profile = _tombstoned_profile(tmp_path) + token = set_hermes_home_override(profile) + try: + with pytest.raises( + FileNotFoundError, match="Named profile home does not exist" + ): + _write_json_cache( + profile / "cache" / "reasoning_caps.json", + {"m": {}}, + indent=0, + separators=(",", ":"), + ) + finally: + from hermes_constants import reset_hermes_home_override + + reset_hermes_home_override(token) + assert not profile.exists() + + def test_lifecycle_sentinel_write_after_delete(self, tmp_path): + from gateway.lifecycle_ledger import _write_sentinel + + profile = _tombstoned_profile(tmp_path) + _write_sentinel({"reason": "clean-exit"}, profile) + assert not profile.exists() + + def test_oauth_token_write_after_delete(self, tmp_path): + from tools.mcp_oauth import _write_json + + profile = _tombstoned_profile(tmp_path) + with pytest.raises( + FileNotFoundError, match="Named profile home does not exist" + ): + _write_json(profile / "mcp-oauth" / "tokens.json", {"access_token": "x"}) + assert not profile.exists() + + def test_memory_store_add_after_delete(self, tmp_path): + from tools.memory_tool_store import MemoryStore + + profile = _tombstoned_profile(tmp_path) + token = set_hermes_home_override(profile) + try: + store = MemoryStore(memory_char_limit=100, user_char_limit=100) + with pytest.raises( + FileNotFoundError, match="Named profile home does not exist" + ): + store.add("memory", "entry") + finally: + from hermes_constants import reset_hermes_home_override + + reset_hermes_home_override(token) + assert not profile.exists() + + def test_roundtrip_yaml_update_does_not_recreate_home(self, tmp_path): + from utils import atomic_roundtrip_yaml_update + + profile = _tombstoned_profile(tmp_path) + with pytest.raises( + FileNotFoundError, match="Named profile home does not exist" + ): + atomic_roundtrip_yaml_update(profile / "config.yaml", "model", "glm-5.3") + assert not profile.exists() + + +class TestUnrelatedProfilesPathsStillWrite: + def test_custom_home_with_profiles_segment_writes(self, tmp_path): + custom_home = tmp_path / "srv" / "profiles" / "buildcache" + atomic_json_write(custom_home / "cache" / "blob.json", {"a": 1}) + assert json.loads( + (custom_home / "cache" / "blob.json").read_text(encoding="utf-8") + ) == {"a": 1} + + def test_default_home_cache_write(self, tmp_path): + home = tmp_path / ".hermes" + atomic_write_text(home / "cache" / "etag", "v1") + assert (home / "cache" / "etag").read_text(encoding="utf-8") == "v1" + + def test_plain_tmp_path_write(self, tmp_path): + atomic_json_write(tmp_path / "plain" / "data.json", [1, 2]) + assert (tmp_path / "plain" / "data.json").exists() diff --git a/tools/mcp_oauth.py b/tools/mcp_oauth.py index 6e17a8e821..885ae40c48 100644 --- a/tools/mcp_oauth.py +++ b/tools/mcp_oauth.py @@ -108,7 +108,9 @@ async def acquire_refresh_fence(path: "Path", *, timeout: float = _REFRESH_FENCE """ lock_path = _refresh_lock_path(path) try: - lock_path.parent.mkdir(parents=True, exist_ok=True) + from hermes_constants import mkdir_under_hermes_home + + mkdir_under_hermes_home(lock_path.parent) secure_parent_dir(lock_path) fd = os.open(lock_path, os.O_RDWR | os.O_CREAT, 0o600) except OSError as exc: @@ -386,7 +388,9 @@ def _read_json(path: Path) -> dict | None: def _write_json(path: Path, data: dict) -> None: """OAuth tokens/client info at 0600 from creation, parent tightened to 0700 (``secure_parent_dir`` refuses ``/``, top-level dirs and the install tree — #25821, #93050).""" - path.parent.mkdir(parents=True, exist_ok=True) + from hermes_constants import mkdir_under_hermes_home + + mkdir_under_hermes_home(path.parent) secure_parent_dir(path) atomic_json_write(path, data, mode=0o600, default=str) @@ -555,7 +559,9 @@ class HermesTokenStorage: the refused client_id. Cleared by ``remove()`` so a fixed document gets a retry.""" path = self._cimd_rejected_path() try: - path.parent.mkdir(parents=True, exist_ok=True) + from hermes_constants import mkdir_under_hermes_home + + mkdir_under_hermes_home(path.parent) path.touch() except OSError as exc: # non-fatal — worst case we retry CIMD later logger.debug("Could not record CIMD rejection at %s: %s", path, exc) @@ -589,7 +595,9 @@ class HermesTokenStorage: if not snapshot: return token_dir = _get_token_dir(self._hermes_home) - token_dir.mkdir(parents=True, exist_ok=True) + from hermes_constants import mkdir_under_hermes_home + + mkdir_under_hermes_home(token_dir) for fname, data in snapshot.items(): try: fd = os.open(str(token_dir / fname), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, stat.S_IRUSR | stat.S_IWUSR) diff --git a/tools/memory_tool_store.py b/tools/memory_tool_store.py index 7098f91899..0bca155602 100644 --- a/tools/memory_tool_store.py +++ b/tools/memory_tool_store.py @@ -127,7 +127,9 @@ class MemoryStore: for target in ("memory", "user"): path = self._path_for(target) - path.parent.mkdir(parents=True, exist_ok=True) + from hermes_constants import mkdir_under_hermes_home + + mkdir_under_hermes_home(path.parent) # Deduplicate (order-preserving, first occurrence wins). entries = list(dict.fromkeys(self._read_file(path))) self._set_entries(target, entries) @@ -148,7 +150,9 @@ class MemoryStore: from tools import memory_tool as _mt # fcntl/msvcrt live (and are patched) there fcntl, msvcrt = _mt.fcntl, _mt.msvcrt lock_path = path.with_suffix(path.suffix + ".lock") - lock_path.parent.mkdir(parents=True, exist_ok=True) + from hermes_constants import mkdir_under_hermes_home + + mkdir_under_hermes_home(lock_path.parent) if fcntl is None and msvcrt is None: yield return @@ -230,7 +234,9 @@ class MemoryStore: if isinstance(result, dict): return result self._set_entries(target, result[0]) - path.parent.mkdir(parents=True, exist_ok=True) + from hermes_constants import mkdir_under_hermes_home + + mkdir_under_hermes_home(path.parent) self._write_file(path, result[0]) return self._success_response(target, result[1])