From 10e60060d98b1f215c84ed4d997bbc365a68b148 Mon Sep 17 00:00:00 2001 From: Erosika Date: Tue, 30 Jun 2026 19:26:04 +0000 Subject: [PATCH] fix(profile): resolve import-time path globals per-call to honor profile override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In single-process multi-profile runtimes (desktop tui_gateway), profile scoping is a context-local ContextVar override, not a process env var. Three subsystems froze their HERMES_HOME-derived paths at import time (or read os.environ directly), pinning every later profile to whichever profile first imported the module — a cross-profile data leak. - tools/skills_hub.py: SKILLS_DIR/HUB_DIR/LOCK_FILE/etc. were module constants frozen at import. Replace with per-call resolver functions; add a PEP 562 module __getattr__ so external 'from tools.skills_hub import SKILLS_DIR' callers (all function-local) resolve dynamically with no call-site changes. Convert default-arg bindings (HubLockFile/TapsManager) and the derived HERMES_INDEX_CACHE_FILE constant too. - gateway/platforms/base.py: image/audio/video/document cache-dir getters now re-resolve via get_hermes_dir() per call, falling back to the module constant when a test has monkeypatched it (preserves the existing test seam). Media-delivery safe-roots already enumerate all profiles' cache dirs (#31733), so per-profile resolution does not break delivery. - gateway/rich_sent_store.py: _store_path() read os.environ['HERMES_HOME'] directly, bypassing the override entirely; route through get_hermes_home(). --- gateway/platforms/base.py | 62 ++++++++++++-- gateway/rich_sent_store.py | 11 ++- tools/skills_hub.py | 163 +++++++++++++++++++++++++++---------- 3 files changed, 181 insertions(+), 55 deletions(-) diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index f7ce60477f8..6fd0cb52e59 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -565,8 +565,37 @@ async def _ssrf_redirect_guard(response): # --------------------------------------------------------------------------- # Default location: {HERMES_HOME}/cache/images/ (legacy: image_cache/) +# +# NOTE: These module-level constants are the import-time DEFAULTS. They exist +# for two reasons: (1) backward-compatible references elsewhere, and (2) tests +# monkeypatch them (e.g. ``monkeypatch.setattr("...IMAGE_CACHE_DIR", tmp)``). +# The ``get_*_cache_dir()`` getters below re-resolve through ``get_hermes_dir()`` +# on every call so the context-local profile override +# (``set_hermes_home_override``) is honored — freezing the resolved path at +# import pinned every profile to whichever one first imported this module +# (cross-profile leak in single-process multi-profile desktop runtime). When a +# test has monkeypatched the constant away from its import-time default, that +# override wins (preserves the existing test seam). IMAGE_CACHE_DIR = get_hermes_dir("cache/images", "image_cache") + +def _resolve_cache_dir(constant_name: str, new_subpath: str, old_name: str) -> Path: + """Resolve a cache dir, honoring profile override and test monkeypatches. + + Precedence: + 1. If the module constant ``constant_name`` was monkeypatched away from + its import-time default, return the patched value (test seam). + 2. Otherwise resolve fresh via ``get_hermes_dir`` so the active profile's + ``set_hermes_home_override`` is reflected per-call. + """ + fresh = get_hermes_dir(new_subpath, old_name) + current = globals().get(constant_name) + default = _CACHE_DIR_IMPORT_DEFAULTS.get(constant_name) + if current is not None and default is not None and current != default: + # A test (or caller) replaced the module constant — respect it. + return Path(current) + return fresh + # --------------------------------------------------------------------------- # Inbound media size cap (#13145) # @@ -660,8 +689,9 @@ async def _read_httpx_body_with_limit(response, *, media_type: str) -> bytes: def get_image_cache_dir() -> Path: """Return the image cache directory, creating it if it doesn't exist.""" - IMAGE_CACHE_DIR.mkdir(parents=True, exist_ok=True) - return IMAGE_CACHE_DIR + d = _resolve_cache_dir("IMAGE_CACHE_DIR", "cache/images", "image_cache") + d.mkdir(parents=True, exist_ok=True) + return d def _looks_like_image(data: bytes) -> bool: @@ -806,8 +836,9 @@ AUDIO_CACHE_DIR = get_hermes_dir("cache/audio", "audio_cache") def get_audio_cache_dir() -> Path: """Return the audio cache directory, creating it if it doesn't exist.""" - AUDIO_CACHE_DIR.mkdir(parents=True, exist_ok=True) - return AUDIO_CACHE_DIR + d = _resolve_cache_dir("AUDIO_CACHE_DIR", "cache/audio", "audio_cache") + d.mkdir(parents=True, exist_ok=True) + return d def cache_audio_from_bytes(data: bytes, ext: str = ".ogg") -> str: @@ -912,8 +943,9 @@ SUPPORTED_VIDEO_TYPES = { def get_video_cache_dir() -> Path: """Return the video cache directory, creating it if it doesn't exist.""" - VIDEO_CACHE_DIR.mkdir(parents=True, exist_ok=True) - return VIDEO_CACHE_DIR + d = _resolve_cache_dir("VIDEO_CACHE_DIR", "cache/videos", "video_cache") + d.mkdir(parents=True, exist_ok=True) + return d def cache_video_from_bytes(data: bytes, ext: str = ".mp4") -> str: @@ -935,6 +967,19 @@ def cache_video_from_bytes(data: bytes, ext: str = ".mp4") -> str: DOCUMENT_CACHE_DIR = get_hermes_dir("cache/documents", "document_cache") SCREENSHOT_CACHE_DIR = get_hermes_dir("cache/screenshots", "browser_screenshots") + +# Import-time defaults for the cache-dir constants. ``_resolve_cache_dir`` +# compares the live module value against these to detect a test monkeypatch +# (in which case the patched value wins) vs. an unmodified constant (in which +# case it re-resolves through the active profile override). +_CACHE_DIR_IMPORT_DEFAULTS = { + "IMAGE_CACHE_DIR": IMAGE_CACHE_DIR, + "AUDIO_CACHE_DIR": AUDIO_CACHE_DIR, + "VIDEO_CACHE_DIR": VIDEO_CACHE_DIR, + "DOCUMENT_CACHE_DIR": DOCUMENT_CACHE_DIR, + "SCREENSHOT_CACHE_DIR": SCREENSHOT_CACHE_DIR, +} + _HERMES_HOME = get_hermes_home() _HERMES_ROOT = get_default_hermes_root() MEDIA_DELIVERY_ALLOW_DIRS_ENV = "HERMES_MEDIA_ALLOW_DIRS" @@ -1491,8 +1536,9 @@ def _strip_media_tag_directives(text: str) -> str: def get_document_cache_dir() -> Path: """Return the document cache directory, creating it if it doesn't exist.""" - DOCUMENT_CACHE_DIR.mkdir(parents=True, exist_ok=True) - return DOCUMENT_CACHE_DIR + d = _resolve_cache_dir("DOCUMENT_CACHE_DIR", "cache/documents", "document_cache") + d.mkdir(parents=True, exist_ok=True) + return d def cache_document_from_bytes(data: bytes, filename: str) -> str: diff --git a/gateway/rich_sent_store.py b/gateway/rich_sent_store.py index fee63602bd0..414e48736d3 100644 --- a/gateway/rich_sent_store.py +++ b/gateway/rich_sent_store.py @@ -25,8 +25,15 @@ _MAX_TEXT_CHARS = 2000 def _store_path() -> str: - home = os.environ.get("HERMES_HOME") or os.path.expanduser("~/.hermes") - return os.path.join(home, "state", "rich_sent_index.json") + # Resolve through get_hermes_home() so the context-local profile override + # (set_hermes_home_override) is honored. Reading os.environ["HERMES_HOME"] + # directly bypassed the override and leaked every profile's rich-sent index + # into the launch/default profile in single-process multi-profile runtimes + # (desktop tui_gateway). + from hermes_constants import get_hermes_home + + home = get_hermes_home() + return os.path.join(str(home), "state", "rich_sent_index.json") def _key(chat_id, message_id) -> str: diff --git a/tools/skills_hub.py b/tools/skills_hub.py index 0827545ae72..3863cbc977a 100644 --- a/tools/skills_hub.py +++ b/tools/skills_hub.py @@ -46,18 +46,78 @@ logger = logging.getLogger(__name__) # --------------------------------------------------------------------------- # Paths # --------------------------------------------------------------------------- +# +# These directories MUST resolve through ``get_hermes_home()`` on every access +# so the context-local profile override (``set_hermes_home_override``) is +# honored. Freezing them as module-level constants at import time pinned every +# later profile to whichever profile first imported this module — a cross- +# profile data leak in single-process multi-profile runtimes (desktop +# ``tui_gateway``). See the profile-isolation fix. +# +# Backward compatibility: external callers do ``from tools.skills_hub import +# SKILLS_DIR`` (always inside a function body, re-evaluated per call). The +# module-level ``__getattr__`` below makes those names resolve dynamically, so +# no external call site needs to change. -HERMES_HOME = get_hermes_home() -SKILLS_DIR = HERMES_HOME / "skills" -HUB_DIR = SKILLS_DIR / ".hub" -LOCK_FILE = HUB_DIR / "lock.json" -QUARANTINE_DIR = HUB_DIR / "quarantine" -AUDIT_LOG = HUB_DIR / "audit.log" -TAPS_FILE = HUB_DIR / "taps.json" -INDEX_CACHE_DIR = HUB_DIR / "index-cache" +INDEX_CACHE_TTL = 3600 # 1 hour (defined early; referenced below) -# Cache duration for remote index fetches -INDEX_CACHE_TTL = 3600 # 1 hour + +def _hermes_home() -> Path: + return get_hermes_home() + + +def _skills_dir() -> Path: + return _hermes_home() / "skills" + + +def _hub_dir() -> Path: + return _skills_dir() / ".hub" + + +def _lock_file() -> Path: + return _hub_dir() / "lock.json" + + +def _quarantine_dir() -> Path: + return _hub_dir() / "quarantine" + + +def _audit_log() -> Path: + return _hub_dir() / "audit.log" + + +def _taps_file() -> Path: + return _hub_dir() / "taps.json" + + +def _index_cache_dir() -> Path: + return _hub_dir() / "index-cache" + + +_DYNAMIC_PATH_RESOLVERS = { + "HERMES_HOME": _hermes_home, + "SKILLS_DIR": _skills_dir, + "HUB_DIR": _hub_dir, + "LOCK_FILE": _lock_file, + "QUARANTINE_DIR": _quarantine_dir, + "AUDIT_LOG": _audit_log, + "TAPS_FILE": _taps_file, + "INDEX_CACHE_DIR": _index_cache_dir, +} + + +def __getattr__(name: str): + """Resolve the legacy path constants dynamically per access. + + PEP 562 module ``__getattr__``: only called for names NOT found as real + module attributes, so it does not slow down ordinary lookups. This lets + ``tools.skills_hub.SKILLS_DIR`` (and the rest) reflect the active profile + override instead of an import-time snapshot. + """ + resolver = _DYNAMIC_PATH_RESOLVERS.get(name) + if resolver is not None: + return resolver() + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") _REDIRECT_STATUS_CODES = {301, 302, 303, 307, 308} _MAX_SKILL_FETCH_REDIRECTS = 5 @@ -176,9 +236,10 @@ def _resolve_lock_install_path(install_path: str, skill_name: str) -> Path: and ``rmtree(SKILLS_DIR)`` would wipe every installed skill. """ normalized = _normalize_lock_install_path(install_path, skill_name) - skills_root = SKILLS_DIR.resolve() + skills_dir = _skills_dir() + skills_root = skills_dir.resolve() - target = SKILLS_DIR + target = skills_dir for part in normalized.split("/"): target = target / part if _is_path_redirect(target): @@ -973,7 +1034,7 @@ class GitHubSource(SkillSource): def _read_cache(self, key: str) -> Optional[list]: """Read cached index if not expired.""" - cache_file = INDEX_CACHE_DIR / f"{key}.json" + cache_file = _index_cache_dir() / f"{key}.json" if not cache_file.exists(): return None try: @@ -986,8 +1047,9 @@ class GitHubSource(SkillSource): def _write_cache(self, key: str, data: list) -> None: """Write index data to cache.""" - INDEX_CACHE_DIR.mkdir(parents=True, exist_ok=True) - cache_file = INDEX_CACHE_DIR / f"{key}.json" + index_cache_dir = _index_cache_dir() + index_cache_dir.mkdir(parents=True, exist_ok=True) + cache_file = index_cache_dir / f"{key}.json" try: cache_file.write_text(json.dumps(data, ensure_ascii=False)) except OSError as e: @@ -3157,7 +3219,7 @@ class OptionalSkillSource(SkillSource): def _read_index_cache(key: str) -> Optional[Any]: """Read cached data if not expired.""" - cache_file = INDEX_CACHE_DIR / f"{key}.json" + cache_file = _index_cache_dir() / f"{key}.json" if not cache_file.exists(): return None try: @@ -3171,17 +3233,18 @@ def _read_index_cache(key: str) -> Optional[Any]: def _write_index_cache(key: str, data: Any) -> None: """Write data to cache.""" - INDEX_CACHE_DIR.mkdir(parents=True, exist_ok=True) + index_cache_dir = _index_cache_dir() + index_cache_dir.mkdir(parents=True, exist_ok=True) # Ensure .ignore exists so ripgrep (and tools respecting .ignore) skip # this directory. Cache files contain unvetted community content that # could include adversarial text (prompt injection via catalog entries). - ignore_file = HUB_DIR / ".ignore" + ignore_file = _hub_dir() / ".ignore" if not ignore_file.exists(): try: ignore_file.write_text("# Exclude hub internals from search tools\n*\n") except OSError: pass - cache_file = INDEX_CACHE_DIR / f"{key}.json" + cache_file = index_cache_dir / f"{key}.json" try: cache_file.write_text(json.dumps(data, ensure_ascii=False, default=str)) except OSError as e: @@ -3210,8 +3273,8 @@ def _skill_meta_to_dict(meta: SkillMeta) -> dict: class HubLockFile: """Manages skills/.hub/lock.json — tracks provenance of installed hub skills.""" - def __init__(self, path: Path = LOCK_FILE): - self.path = path + def __init__(self, path: Optional[Path] = None): + self.path = path if path is not None else _lock_file() def load(self) -> dict: if not self.path.exists(): @@ -3282,8 +3345,8 @@ class HubLockFile: class TapsManager: """Manages the taps.json file — custom GitHub repo sources.""" - def __init__(self, path: Path = TAPS_FILE): - self.path = path + def __init__(self, path: Optional[Path] = None): + self.path = path if path is not None else _taps_file() def load(self) -> List[dict]: if not self.path.exists(): @@ -3327,14 +3390,15 @@ class TapsManager: def append_audit_log(action: str, skill_name: str, source: str, trust_level: str, verdict: str, extra: str = "") -> None: """Append a line to the audit log.""" - AUDIT_LOG.parent.mkdir(parents=True, exist_ok=True) + audit_log = _audit_log() + audit_log.parent.mkdir(parents=True, exist_ok=True) timestamp = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") parts = [timestamp, action, skill_name, f"{source}:{trust_level}", verdict] if extra: parts.append(extra) line = " ".join(parts) + "\n" try: - with open(AUDIT_LOG, "a", encoding="utf-8") as f: + with open(audit_log, "a", encoding="utf-8") as f: f.write(line) except OSError as e: logger.debug("Could not write audit log: %s", e) @@ -3346,15 +3410,19 @@ def append_audit_log(action: str, skill_name: str, source: str, def ensure_hub_dirs() -> None: """Create the .hub directory structure if it doesn't exist.""" - HUB_DIR.mkdir(parents=True, exist_ok=True) - QUARANTINE_DIR.mkdir(exist_ok=True) - INDEX_CACHE_DIR.mkdir(exist_ok=True) - if not LOCK_FILE.exists(): - LOCK_FILE.write_text('{"version": 1, "installed": {}}\n') - if not AUDIT_LOG.exists(): - AUDIT_LOG.touch() - if not TAPS_FILE.exists(): - TAPS_FILE.write_text('{"taps": []}\n') + hub_dir = _hub_dir() + lock_file = _lock_file() + audit_log = _audit_log() + taps_file = _taps_file() + hub_dir.mkdir(parents=True, exist_ok=True) + _quarantine_dir().mkdir(exist_ok=True) + _index_cache_dir().mkdir(exist_ok=True) + if not lock_file.exists(): + lock_file.write_text('{"version": 1, "installed": {}}\n') + if not audit_log.exists(): + audit_log.touch() + if not taps_file.exists(): + taps_file.write_text('{"taps": []}\n') def quarantine_bundle(bundle: SkillBundle) -> Path: @@ -3366,7 +3434,7 @@ def quarantine_bundle(bundle: SkillBundle) -> Path: safe_rel_path = _validate_bundle_rel_path(rel_path) validated_files.append((safe_rel_path, file_content)) - dest = QUARANTINE_DIR / skill_name + dest = _quarantine_dir() / skill_name if dest.exists(): shutil.rmtree(dest) dest.mkdir(parents=True) @@ -3393,7 +3461,7 @@ def install_from_quarantine( safe_skill_name = _validate_skill_name(skill_name) safe_category = _validate_install_parent_path(category) if category else "" quarantine_resolved = quarantine_path.resolve() - quarantine_root = QUARANTINE_DIR.resolve() + quarantine_root = _quarantine_dir().resolve() if not quarantine_resolved.is_relative_to(quarantine_root): raise ValueError(f"Unsafe quarantine path: {quarantine_path}") @@ -3453,7 +3521,7 @@ def install_from_quarantine( trust_level=bundle.trust_level, scan_verdict=scan_result.verdict, skill_hash=content_hash(install_dir), - install_path=str(install_dir.relative_to(SKILLS_DIR)), + install_path=str(install_dir.relative_to(_skills_dir())), files=list(bundle.files.keys()), metadata=bundle.metadata, ) @@ -3582,10 +3650,13 @@ def check_for_skill_updates( # --------------------------------------------------------------------------- HERMES_INDEX_URL = "https://hermes-agent.nousresearch.com/docs/api/skills-index.json" -HERMES_INDEX_CACHE_FILE = INDEX_CACHE_DIR / "hermes-index.json" HERMES_INDEX_TTL = 6 * 3600 # 6 hours +def _hermes_index_cache_file() -> Path: + return _index_cache_dir() / "hermes-index.json" + + def _load_hermes_index() -> Optional[dict]: """Fetch the centralized skills index, with local cache. @@ -3594,11 +3665,12 @@ def _load_hermes_index() -> Optional[dict]: downloads within a session. """ # Check local cache - if HERMES_INDEX_CACHE_FILE.exists(): + hermes_index_cache_file = _hermes_index_cache_file() + if hermes_index_cache_file.exists(): try: - age = time.time() - HERMES_INDEX_CACHE_FILE.stat().st_mtime + age = time.time() - hermes_index_cache_file.stat().st_mtime if age < HERMES_INDEX_TTL: - return json.loads(HERMES_INDEX_CACHE_FILE.read_text()) + return json.loads(hermes_index_cache_file.read_text()) except (OSError, json.JSONDecodeError): pass @@ -3619,8 +3691,8 @@ def _load_hermes_index() -> Optional[dict]: # Cache locally try: - HERMES_INDEX_CACHE_FILE.parent.mkdir(parents=True, exist_ok=True) - HERMES_INDEX_CACHE_FILE.write_text(json.dumps(data)) + hermes_index_cache_file.parent.mkdir(parents=True, exist_ok=True) + hermes_index_cache_file.write_text(json.dumps(data)) except OSError: pass @@ -3629,9 +3701,10 @@ def _load_hermes_index() -> Optional[dict]: def _load_stale_index_cache() -> Optional[dict]: """Fall back to stale cache when the network fetch fails.""" - if HERMES_INDEX_CACHE_FILE.exists(): + hermes_index_cache_file = _hermes_index_cache_file() + if hermes_index_cache_file.exists(): try: - return json.loads(HERMES_INDEX_CACHE_FILE.read_text()) + return json.loads(hermes_index_cache_file.read_text()) except (OSError, json.JSONDecodeError): pass return None