From 76f2fdb794520f0356215d81cc528fd111488fea Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 27 Oct 2026 11:08:49 -0700 Subject: [PATCH] fix(cache): a rotated key, token or same-size edit is never served stale from a file cache Every cache keyed on a file's stat signature had the bug the config caches had: a same-size in-place rewrite in one coarse timestamp tick keeps mtime, ctime, size and inode, so the older parse was served for the rest of the process. Move the helper to utils.file_cache_version (file_signature + content digest while the file is under 2 s old; always_digest for signatures compared much later, where the racy digest going empty would read as a change) and key every cache that can hold a secret, token, pairing/allowlist, route or config on it: - auth.py: global auth-store memo, Nous auth-status memo (was mtime only) - gateway/pairing.py approved-users cache (a revocation kept access) - run_profile_reconcile: served-profile signature (always_digest: a false change reconnects adapters) and the hook-consent signature - webhook dynamic routes (route HMAC secrets), Feishu comment pairing/rules cache (mtime only) - MCP OAuth tokens-file reload check (mtime only; always_digest: a reload tears the session down) - config parse-failure record (always_digest), named-custom-provider memo, model aliases, gateway.standalone memo, profile model read, MoA preset, agent raw config, tui_gateway raw config cache, onboarding due stamp, free-tier inventory stamp, provider model-list credential fingerprint, git safe.directory key, skills prompt manifest config_cache_version takes its stat through config_version, so its errors and seams are unchanged. Left alone: the tool-definition cache key and the platform manifest stamp (a safe worker must never open config.yaml or a plugin file; the MoA preset cache is skipped in one instead) and the CLI MCP watcher seed (pinned to file_signature by a main test). --- agent/moa_loop.py | 6 +- agent/prompt_builder.py | 4 +- agent/skill_utils.py | 8 +- gateway/pairing.py | 5 +- gateway/platforms/webhook.py | 10 ++- gateway/run_profile_reconcile.py | 12 +-- hermes_cli/_subprocess_compat.py | 5 +- hermes_cli/auth.py | 18 ++--- hermes_cli/config.py | 4 +- hermes_cli/config_backend.py | 27 +++---- hermes_cli/config_read_errors.py | 19 ++++- hermes_cli/free_tier_bootstrap.py | 4 +- hermes_cli/managed_scope.py | 6 +- hermes_cli/model_switch.py | 4 +- hermes_cli/models.py | 4 +- hermes_cli/profiles.py | 6 +- hermes_cli/stale_modules.py | 2 +- .../platforms/feishu/feishu_comment_rules.py | 13 ++-- providers/__init__.py | 6 +- .../test_file_cache_same_tick_rewrite.py | 76 +++++++++++++++++++ tools/mcp_oauth_manager.py | 14 +++- tui_gateway/methods_onboarding.py | 9 +-- tui_gateway/server.py | 8 +- utils.py | 35 ++++++++- .../docs/developer-guide/config-backend.md | 8 +- 25 files changed, 222 insertions(+), 91 deletions(-) create mode 100644 tests/hermes_cli/test_file_cache_same_tick_rewrite.py diff --git a/agent/moa_loop.py b/agent/moa_loop.py index cdfaa30c0e..406c2a2e64 100644 --- a/agent/moa_loop.py +++ b/agent/moa_loop.py @@ -126,8 +126,10 @@ def _resolve_preset_cached(preset_name: str) -> tuple[dict[str, Any], Any]: (skips resolve_moa_preset's full validation of the moa block on every create()).""" from hermes_cli.config import get_config_path, load_config from hermes_cli.moa_config import resolve_moa_preset - from hermes_cli.config_backend import config_version_or_none - cfg_stamp = config_version_or_none(get_config_path()) + from hermes_cli.config_backend import config_cache_version_or_none + from agent.safe_worker_policy import worker_config_snapshot + # A safe worker's config is its frozen snapshot: never open the profile file (uncached there). + cfg_stamp = None if worker_config_snapshot() is not None else config_cache_version_or_none(get_config_path()) moa_raw = load_config().get("moa") or {} key = (cfg_stamp, preset_name) with _preset_cache_lock: diff --git a/agent/prompt_builder.py b/agent/prompt_builder.py index f85a291127..517b07132b 100644 --- a/agent/prompt_builder.py +++ b/agent/prompt_builder.py @@ -29,7 +29,7 @@ from agent.skill_utils import ( skill_matches_platform, skill_matches_platform_list, ) from tools.threat_patterns import scan_for_threats as _scan_for_threats -from utils import atomic_json_write, file_signature +from utils import atomic_json_write, file_cache_version logger = logging.getLogger(__name__) @@ -1110,7 +1110,7 @@ def _build_skills_manifest(skills_dir: Path) -> dict[str, list[int]]: try: if filename in files: st = os.stat(path) - manifest[path[prefix_len:]] = list(file_signature(st)) + manifest[path[prefix_len:]] = list(file_cache_version(path, st)) except OSError: pass return manifest diff --git a/agent/skill_utils.py b/agent/skill_utils.py index 03c087c4ed..b62103add3 100644 --- a/agent/skill_utils.py +++ b/agent/skill_utils.py @@ -208,10 +208,10 @@ def _raw_config_cache_clear() -> None: _RAW_CONFIG_CACHE.clear() -def _config_cache_key(config_path: Path) -> Optional[tuple[str, int, int, int, int]]: - """``(path, *backend version)`` identity of config.yaml, or None when unreadable/absent.""" - from hermes_cli.config_backend import config_version_or_none - version = config_version_or_none(config_path) +def _config_cache_key(config_path: Path) -> Optional[tuple[Any, ...]]: + """``(path, *backend cache version)`` identity of config.yaml, or None when unreadable/absent.""" + from hermes_cli.config_backend import config_cache_version_or_none + version = config_cache_version_or_none(config_path) return None if version is None else (str(config_path), *version) diff --git a/gateway/pairing.py b/gateway/pairing.py index d4a7737fe1..e7103d3d13 100644 --- a/gateway/pairing.py +++ b/gateway/pairing.py @@ -20,7 +20,7 @@ from typing import Optional from gateway.whatsapp_identity import expand_whatsapp_aliases, normalize_whatsapp_identifier from hermes_constants import get_default_hermes_root, get_hermes_dir, get_hermes_home -from utils import atomic_json_write, file_signature +from utils import atomic_json_write, file_cache_version logger = logging.getLogger(__name__) @@ -402,7 +402,8 @@ class PairingStore: # fstat identifies the actual opened file even during atomic replacement. with path.open("rb") as stream: st = os.fstat(stream.fileno()) - key = (st.st_dev, *file_signature(st)) + # Keyed before the parse: a write in between costs one re-parse, never a stale hit. + key = (st.st_dev, *file_cache_version(path, st)) cached = self._approved_cache.get(platform) if cached is not None and cached[0] == key: return cached[1] diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index aa1cffb394..1982cee745 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -439,19 +439,23 @@ class WebhookAdapter(BasePlatformAdapter): """Reload agent-created subscriptions when the file's stat identity changes. Runs on every POST before auth, so it never takes the CLI writer lock: writers publish via - atomic rename, which also gives the file a new inode, so a restored mtime cannot hide a change. + atomic rename, which also gives the file a new inode, so a restored mtime cannot hide a change; + ``file_cache_version`` also catches a same-tick same-size edit made in place (a hand edit). """ from hermes_constants import get_hermes_home + from utils import file_cache_version subs_path = get_hermes_home() / _DYNAMIC_ROUTES_FILENAME try: - st = subs_path.stat() + stat_key = file_cache_version(subs_path) except FileNotFoundError: if self._dynamic_routes: self._dynamic_routes, self._routes = {}, dict(self._static_routes) logger.debug("[webhook] Dynamic subscriptions file removed, cleared dynamic routes") self._dynamic_routes_stat = None return - stat_key = (st.st_mtime_ns, st.st_size, st.st_ino) + except OSError as e: + logger.error("[webhook] Failed to read dynamic routes: %s", e) + return if stat_key == self._dynamic_routes_stat: return # No change try: diff --git a/gateway/run_profile_reconcile.py b/gateway/run_profile_reconcile.py index 20bbfce5a2..ccb37e9c62 100644 --- a/gateway/run_profile_reconcile.py +++ b/gateway/run_profile_reconcile.py @@ -22,13 +22,12 @@ from __future__ import annotations import asyncio import logging -import os from functools import partial from pathlib import Path from typing import Any, Dict, Optional from gateway.run_shutdown import _log_suppressed -from utils import file_signature +from utils import file_cache_version logger = logging.getLogger(__name__) @@ -42,18 +41,19 @@ _OWN_GATEWAY_PROBE_TIMEOUT_SECS = 5.0 def _stat_signature(path: Path) -> Optional[tuple]: try: - return file_signature(os.stat(path)) + return file_cache_version(path) except OSError: return None def profile_serve_signature(home: Path) -> tuple: - """Cheap change detector for a served profile's credentials/config: file signature per file.""" + """Change detector for a served profile's credentials/config: ``file_cache_version`` per file, + hashed every time (``always_digest``): it is compared long after it was taken, and a racy digest + going empty would read as a change and reconnect the profile's adapters.""" sig = [] for name in _PROFILE_SIGNATURE_FILES: try: - st = os.stat(Path(home) / name) - sig.append(file_signature(st)) + sig.append(file_cache_version(Path(home) / name, always_digest=True)) except OSError: sig.append(None) return tuple(sig) diff --git a/hermes_cli/_subprocess_compat.py b/hermes_cli/_subprocess_compat.py index 596279d1fc..6da70a551f 100644 --- a/hermes_cli/_subprocess_compat.py +++ b/hermes_cli/_subprocess_compat.py @@ -305,7 +305,7 @@ _GIT_CONFIG_OVERRIDES = { def _safe_directory_cache_key(env: Mapping[str, str]) -> tuple: """Everything that decides which files ``git config --system/--global`` reads, plus the - global candidates' mtimes so an edit to ``~/.gitconfig`` is picked up without a restart.""" + global candidates' ``file_cache_version`` so an edit to ``~/.gitconfig`` is picked up without a restart.""" home = env.get("HOME", "") xdg = env.get("XDG_CONFIG_HOME") or os.path.join(home, ".config") candidates = ( @@ -313,10 +313,11 @@ def _safe_directory_cache_key(env: Mapping[str, str]) -> tuple: env.get("GIT_CONFIG_GLOBAL") or os.path.join(home, ".gitconfig"), os.path.join(xdg, "git", "config"), ) + from utils import file_cache_version stamps = [] for path in candidates: try: - stamps.append(os.stat(path).st_mtime_ns) + stamps.append(file_cache_version(path)) except OSError: stamps.append(None) return ( diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index ab25405f98..aeacdf3b23 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -32,7 +32,7 @@ from urllib.parse import urlparse from hermes_constants import OPENROUTER_BASE_URL, hermes_home_key, secure_parent_dir from agent.credential_persistence import sanitize_borrowed_credential_payload -from utils import atomic_json_write, env_float, file_signature, is_truthy_value +from utils import atomic_json_write, env_float, file_cache_version, is_truthy_value from hermes_cli.auth_zai_kimi import ( KIMI_CODE_BASE_URL, ZAI_ENDPOINTS, _normalize_lmstudio_runtime_base_url, _resolve_kimi_base_url, _resolve_zai_base_url, detect_zai_endpoint) @@ -510,8 +510,8 @@ def _load_global_auth_store() -> dict[str, Any]: _global_auth_store_cache = None return {} try: - cache_key: Optional[tuple[str, tuple[int, int, int, int]]] = ( - str(global_path.resolve(strict=False)), file_signature(global_path.stat())) + cache_key: Optional[tuple[str, tuple[Any, ...]]] = ( + str(global_path.resolve(strict=False)), file_cache_version(global_path)) except Exception: cache_key = None cached = _global_auth_store_cache @@ -1882,19 +1882,19 @@ def _resolve_nous_access_token( # Process-level memo for get_nous_auth_status(): it validates via a synchronous refresh POST # (~350ms) and read-only UI surfaces call it many times per render (~31x per menu paint), burning -# single-use refresh tokens. Keyed on auth.json path + mtime so profile switches don't share a memo +# single-use refresh tokens. Keyed on auth.json path + file_cache_version so profile switches don't share a memo # and login/logout/add/remove invalidate naturally. _NOUS_AUTH_STATUS_CACHE_TTL = 15.0 # seconds -_nous_auth_status_cache: Optional[tuple[float, str, Optional[float], dict[str, Any]]] = None +_nous_auth_status_cache: Optional[tuple[float, str, Optional[tuple[Any, ...]], dict[str, Any]]] = None -# mtime-keyed memo for _load_global_auth_store(): (path, mtime_ns, store); same invalidation rule. -_global_auth_store_cache: Optional[tuple[str, int, dict[str, Any]]] = None +# Memo for _load_global_auth_store(): (path, file_cache_version, store); same invalidation rule. +_global_auth_store_cache: Optional[tuple[str, tuple[Any, ...], dict[str, Any]]] = None -def _auth_file_cache_key() -> tuple[str, Optional[float]]: +def _auth_file_cache_key() -> tuple[str, Optional[tuple[Any, ...]]]: auth_file = _auth_file_path() try: - return _resolved_key(auth_file), auth_file.stat().st_mtime + return _resolved_key(auth_file), file_cache_version(auth_file) except Exception: # missing file included: key without an mtime return _resolved_key(auth_file), None diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 8c7e8f8937..d6c2bc5c48 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -44,10 +44,10 @@ from hermes_constants import ( apply_secure_dir_policy, get_managed_system) # Re-export from hermes_constants — canonical definition lives there. from hermes_constants import get_hermes_home, get_process_hermes_home -from utils import atomic_replace, fast_safe_load, file_signature, mkstemp_beside +from utils import atomic_replace, fast_safe_load, file_cache_version, file_signature, mkstemp_beside from hermes_cli.config_issues import ConfigIssue, _issue from hermes_cli.config_backend import ( - config_cache_version, config_exists, config_version, file_cache_version, probe_config_readable, read_config_doc, + config_cache_version, config_exists, config_version, probe_config_readable, read_config_doc, write_config_document) from hermes_cli.config_read_errors import ( _CONFIG_PARSE_FAILURES, _FIX_PERMS, _FIX_YAML, FailedConfigRead, _backups_dir_display, diff --git a/hermes_cli/config_backend.py b/hermes_cli/config_backend.py index 9136fe99a3..ddd68d09ad 100644 --- a/hermes_cli/config_backend.py +++ b/hermes_cli/config_backend.py @@ -57,8 +57,8 @@ class ConfigBackend(Protocol): """Cheap change signature; changes whenever the user layer does.""" ... - def cache_version(self, home: Path) -> tuple[Any, ...]: - """``version`` plus whatever a cache needs to tell apart writes ``version`` cannot see.""" + def cache_version(self, home: Path, version: tuple[Any, ...]) -> tuple[Any, ...]: + """``version`` (just taken) plus whatever a cache needs to tell apart writes it cannot see.""" ... def exists(self, home: Path) -> bool: ... @@ -83,12 +83,6 @@ class FileBackend: from utils import file_signature return file_signature(st) - @classmethod - def cache_version_path(cls, path: Path) -> tuple[Any, ...]: - version = cls.version_path(path) - from utils import racy_content_digest - return (*version, racy_content_digest(path, version[0])) # version[0] is st_mtime_ns - @staticmethod def read_path(path: Path) -> Any: from utils import fast_safe_load @@ -122,9 +116,8 @@ class FileBackend: def version(self, home: Path) -> tuple[int, int, int, int]: return self.version_path(self.config_path(home)) - def cache_version(self, home: Path) -> tuple[Any, ...]: - version = self.version(home) - from utils import racy_content_digest + def cache_version(self, home: Path, version: tuple[Any, ...]) -> tuple[Any, ...]: + from utils import racy_content_digest # utils.file_cache_version's rule, on this stat return (*version, racy_content_digest(self.config_path(home), version[0])) def exists(self, home: Path) -> bool: @@ -191,13 +184,17 @@ def config_version(config_path: PathLike) -> tuple[Any, ...]: def config_cache_version(config_path: PathLike) -> tuple[Any, ...]: """Signature for a cache of a config file's parse: :func:`config_version` can stay equal across a same-tick same-size rewrite, which would pin the older parse for the process lifetime.""" + version = config_version(config_path) # the one stat; its errors (and test seams) stay config_version's backend, home = _route(config_path) - return backend.cache_version(home) + return backend.cache_version(home, version) -def file_cache_version(path: PathLike) -> tuple[Any, ...]: - """:func:`config_cache_version` of a file that is not a home's user layer (the managed overlay).""" - return FileBackend.cache_version_path(Path(path)) +def config_cache_version_or_none(config_path: PathLike) -> Optional[tuple[Any, ...]]: + """:func:`config_cache_version`, or None when the config is absent or cannot be read.""" + try: + return config_cache_version(config_path) + except OSError: + return None def config_version_or_none(config_path: PathLike) -> Optional[tuple[Any, ...]]: diff --git a/hermes_cli/config_read_errors.py b/hermes_cli/config_read_errors.py index afad4defaf..11666bb4a4 100644 --- a/hermes_cli/config_read_errors.py +++ b/hermes_cli/config_read_errors.py @@ -78,7 +78,7 @@ def _warn_config_parse_failure( from hermes_cli.config_backend import config_version sig = config_version(config_path) key = (str(config_path), *sig) - _CONFIG_PARSE_FAILURES[str(config_path)] = (*sig, str(exc)) + _CONFIG_PARSE_FAILURES[str(config_path)] = (*_failure_version(config_path), str(exc)) except OSError: key = (str(config_path), 0, 0, 0, 0) if key in _CONFIG_PARSE_WARNED: @@ -98,14 +98,25 @@ def _warn_config_parse_failure( pass +def _failure_version(path: Path) -> tuple: + """What a parse-failure record is matched on: ``file_cache_version`` hashed even once settled, + because the record is compared long after it was taken and a same-tick same-size fix must clear + it. An unreadable file (the OSError failures) keeps its stat signature and a ``None`` digest.""" + from utils import file_cache_version + try: + return file_cache_version(path, always_digest=True) + except OSError: + from hermes_cli.config_backend import config_version + return (*config_version(path), None) + + def get_active_config_parse_failure() -> Optional[str]: """Return the recorded parse error while the ACTIVE config.yaml is still byte-identical - (mtime_ns + size + ino + ctime_ns) to the file that failed to parse; else None.""" + (``file_cache_version`` with its content digest) to the file that failed to parse; else None.""" from hermes_cli.config import get_config_path try: - from hermes_cli.config_backend import config_version record = _CONFIG_PARSE_FAILURES[str(path := get_config_path())] - sig = config_version(path) + sig = _failure_version(path) return record[len(sig)] if sig == record[:len(sig)] else None except Exception: return None diff --git a/hermes_cli/free_tier_bootstrap.py b/hermes_cli/free_tier_bootstrap.py index 8d214acd92..539ff70e51 100644 --- a/hermes_cli/free_tier_bootstrap.py +++ b/hermes_cli/free_tier_bootstrap.py @@ -130,12 +130,12 @@ def reset_for_tests() -> None: def _config_stamp() -> tuple: from hermes_cli.config import get_hermes_home + from utils import file_cache_version home = get_hermes_home() stamp = [] for name in _INVENTORY_FILES: try: - st = (home / name).stat() - stamp.append((st.st_mtime_ns, st.st_size)) + stamp.append(file_cache_version(home / name)) except OSError: stamp.append(None) return tuple(stamp) diff --git a/hermes_cli/managed_scope.py b/hermes_cli/managed_scope.py index 3cc274f1e4..e47edea25f 100644 --- a/hermes_cli/managed_scope.py +++ b/hermes_cli/managed_scope.py @@ -23,7 +23,7 @@ from hermes_cli.stale_modules import drop_stale_root_modules drop_stale_root_modules() -from utils import fast_safe_load, file_signature, racy_content_digest +from utils import fast_safe_load, file_cache_version, file_signature # file_signature: bound for the bridge above logger = logging.getLogger(__name__) @@ -31,7 +31,7 @@ logger = logging.getLogger(__name__) _DEFAULT_MANAGED_DIR = Path("/etc/hermes") _CACHE_LOCK = threading.Lock() -# path_key -> (*file_signature, racy digest, parsed) +# path_key -> (*file_cache_version, parsed) _CONFIG_CACHE: dict[str, tuple] = {} _ENV_CACHE: dict[str, tuple] = {} @@ -77,7 +77,7 @@ def _cached_read(path: Path, cache: dict[str, tuple], parse): st = path.stat() except OSError: return None # absent - key = (*file_signature(st), racy_content_digest(path, st.st_mtime_ns)) + key = file_cache_version(path, st) path_key = str(path) with _CACHE_LOCK: hit = cache.get(path_key) diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 077f3e62fd..17a0c701e6 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -270,9 +270,9 @@ def _direct_alias_source_identity() -> Optional[tuple]: path = get_config_path() except Exception: return None - from hermes_cli.config_backend import config_version + from hermes_cli.config_backend import config_cache_version try: - version = config_version(path) + version = config_cache_version(path) except OSError: # A missing config is still a definite identity for this profile. return (str(path), None) diff --git a/hermes_cli/models.py b/hermes_cli/models.py index c9dc1b4776..065e82c8a7 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -1930,8 +1930,9 @@ def _credential_fingerprint(provider: str) -> str: ] def _mtime_part(label: str, path) -> None: + from utils import file_cache_version # hashed every time: keys a disk cache later processes read try: - parts.append(f"{label}@{os.stat(path).st_mtime_ns}") + parts.append(f"{label}@{file_cache_version(path, always_digest=True)}") except FileNotFoundError: parts.append(f"{label}@missing") except Exception: @@ -1939,7 +1940,6 @@ def _credential_fingerprint(provider: str) -> str: if provider == "openai-codex": from hermes_cli.codex_models import codex_catalog_credential_identity - parts.append(f"codex_identity={codex_catalog_credential_identity()}") else: try: diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 7be9481a41..f0bd12c6c9 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -24,7 +24,7 @@ from hermes_constants import ( LOCAL_RUNTIME_ROOT_DIRS, PROFILE_ID_RE, clear_named_profile_deleted, mark_named_profile_deleted, named_profile_has_identity, named_profile_is_deleted, named_profile_is_live, ) -from hermes_cli.config_backend import config_exists, config_version, config_version_or_none +from hermes_cli.config_backend import config_cache_version, config_cache_version_or_none, config_exists from hermes_cli.profiles_cache import _PROFILE_FILE_CACHE, _cached_profile_read logger = logging.getLogger(__name__) @@ -738,7 +738,7 @@ def _read_config_model(profile_dir: Path) -> tuple: return None, None # The user layer changes through the config backend, not necessarily on local disk. - return _cached_profile_read(config_path, "config-model", _read, config_version_or_none) + return _cached_profile_read(config_path, "config-model", _read, config_cache_version_or_none) def _seed_model_config(profile_dir: Path) -> None: @@ -1039,7 +1039,7 @@ def profile_is_standalone(home: Path) -> bool: cfg_path = home / "config.yaml" key = str(home) try: - signature = config_version(cfg_path) + signature = config_cache_version(cfg_path) except FileNotFoundError: signature = None except OSError as exc: diff --git a/hermes_cli/stale_modules.py b/hermes_cli/stale_modules.py index a5971a4d8f..41dbdb3464 100644 --- a/hermes_cli/stale_modules.py +++ b/hermes_cli/stale_modules.py @@ -25,7 +25,7 @@ from typing import Mapping, Sequence # exist on the on-disk copy after this release. Extend when a new root-level # symbol would otherwise break the pre-handoff upgrade path. _ROOT_MODULE_REQUIRED_ATTRS: dict[str, tuple[str, ...]] = { - "utils": ("file_signature", "racy_content_digest"), + "utils": ("file_signature", "file_cache_version"), } diff --git a/plugins/platforms/feishu/feishu_comment_rules.py b/plugins/platforms/feishu/feishu_comment_rules.py index 15b745e659..2b10cbfc46 100644 --- a/plugins/platforms/feishu/feishu_comment_rules.py +++ b/plugins/platforms/feishu/feishu_comment_rules.py @@ -61,26 +61,29 @@ class ResolvedCommentRule: class _MtimeCache: - """Mtime-based JSON file cache: ``stat()`` per access, re-read only on change. ``path`` is a + """JSON file cache: ``utils.file_cache_version`` per access, re-read only on change. ``path`` is a ``Path`` or a zero-arg callable resolving one; state is keyed per resolved path so profiles routed through one multiplexed process never share a slot.""" def __init__(self, path: Path | Callable[[], Path]): self._resolve = path if callable(path) else (lambda: path) - self._entries: dict[Path, tuple[float, dict]] = {} + self._entries: dict[Path, tuple[tuple, dict]] = {} def invalidate(self) -> None: self._entries.pop(self._resolve(), None) def load(self) -> dict: + from utils import file_cache_version path = self._resolve() try: - mtime = path.stat().st_mtime + version = file_cache_version(path) except FileNotFoundError: self._entries.pop(path, None) return {} + except OSError: + version = None # unreadable: the read below logs it and serves {}; never a cache hit cached = self._entries.get(path) - if cached is not None and cached[0] == mtime: + if cached is not None and version is not None and cached[0] == version: return cached[1] try: with open(path, "r", encoding="utf-8-sig") as f: @@ -89,7 +92,7 @@ class _MtimeCache: logger.warning("[Feishu-Rules] Failed to read %s, using empty config", path) data = {} data = data if isinstance(data, dict) else {} - self._entries[path] = (mtime, data) + self._entries[path] = (version, data) return data diff --git a/providers/__init__.py b/providers/__init__.py index 5c992d7f04..75921076e6 100644 --- a/providers/__init__.py +++ b/providers/__init__.py @@ -185,11 +185,11 @@ _NAMED_CUSTOM_MEMO_NO_SIG = object() def _has_named_custom_provider(name: str, home: Path | None, hkey: str) -> bool: - """``has_named_custom_provider`` memoized on the home's config file signature.""" - from utils import file_signature + """``has_named_custom_provider`` memoized on the home's config cache version.""" + from hermes_cli.config_backend import config_cache_version try: - sig = file_signature((home / "config.yaml").stat()) if home is not None else None + sig = config_cache_version(home / "config.yaml") if home is not None else None except OSError: sig = None # no readable config: nothing configured, stable until one appears memo = _NAMED_CUSTOM_MEMO.get(hkey) diff --git a/tests/hermes_cli/test_file_cache_same_tick_rewrite.py b/tests/hermes_cli/test_file_cache_same_tick_rewrite.py new file mode 100644 index 0000000000..f052a6c388 --- /dev/null +++ b/tests/hermes_cli/test_file_cache_same_tick_rewrite.py @@ -0,0 +1,76 @@ +"""Credential and allowlist file caches see a same-size in-place rewrite made in the tick of the cached read. + +Kernels without multigrain timestamps (CI runners, older hosts) stamp mtime/ctime from a coarse clock, +so a file rewritten in place to a same-length value in the tick of an earlier read keeps mtime, ctime, +size and inode. A cache keyed on that stat signature served the older content for the rest of the +process: a revoked pairing kept its access, and a rotated key in the shared auth store never reached +the profiles reading it. ``coarse_clock`` reports both writes in one frozen tick, which is what such a +kernel returns, on this host too.""" +import json +import sys +import time + +import pytest + + +@pytest.fixture +def coarse_clock(monkeypatch): + """Every stat signature reports one frozen mtime/ctime tick, like a coarse filesystem clock.""" + import utils + real, tick = utils.file_signature, time.time_ns() + + def frozen(st): + return (tick, *real(st)[1:3], tick) + + monkeypatch.setattr(utils, "file_signature", frozen) + for module in list(sys.modules.values()): # modules that bound the name at import + if getattr(module, "file_signature", None) is real: + monkeypatch.setattr(module, "file_signature", frozen) + + +def test_an_in_place_revocation_in_one_tick_is_not_served_from_the_pairing_cache(coarse_clock): + from gateway.pairing import PairingStore + store = PairingStore() + path = store._approved_path("telegram") + path.write_text(json.dumps({"111111": {"user_name": "a"}})) + assert store.is_approved("telegram", "111111") + path.write_text(json.dumps({"222222": {"user_name": "a"}})) # same size, same inode, same tick + assert not store.is_approved("telegram", "111111") + assert store.is_approved("telegram", "222222") + + +def test_a_key_rotated_in_place_in_one_tick_reaches_global_auth_store_readers(coarse_clock, tmp_path, monkeypatch): + import hermes_cli.auth as auth_mod + path = tmp_path / "global-hermes" / "auth.json" + path.parent.mkdir(parents=True) + monkeypatch.setattr(auth_mod, "_global_auth_file_path", lambda: path) + monkeypatch.setattr(auth_mod, "_global_auth_store_cache", None) + + def write(key): + path.write_text(json.dumps({"version": 1, "providers": {"openrouter": {"api_key": key}}})) + + write("sk-old") + assert auth_mod._load_global_auth_store()["providers"]["openrouter"]["api_key"] == "sk-old" + write("sk-new") + assert auth_mod._load_global_auth_store()["providers"]["openrouter"]["api_key"] == "sk-new" + + +def test_a_same_size_env_edit_in_one_tick_changes_the_served_profile_signature(coarse_clock, tmp_path): + from gateway.run_profile_reconcile import profile_serve_signature + (tmp_path / ".env").write_text("OPENROUTER_API_KEY=sk-aaaa\n") + before = profile_serve_signature(tmp_path) + (tmp_path / ".env").write_text("OPENROUTER_API_KEY=sk-bbbb\n") + assert profile_serve_signature(tmp_path) != before + + +def test_a_long_lived_signature_does_not_change_when_the_file_settles(tmp_path, monkeypatch): + """``always_digest`` is for signatures compared much later (profile reconnects, parse-failure + records): one taken while the file was fresh still matches the unchanged file once it settles.""" + import utils + path = tmp_path / "config.yaml" + path.write_text("model: a\n") + fresh = utils.file_cache_version(path, always_digest=True) + monkeypatch.setattr(utils, "mtime_is_racy", lambda _mtime_ns: False) + assert utils.file_cache_version(path, always_digest=True) == fresh + path.write_text("model: b\n") + assert utils.file_cache_version(path, always_digest=True) != fresh diff --git a/tools/mcp_oauth_manager.py b/tools/mcp_oauth_manager.py index 3399d06c21..f78199f1f9 100644 --- a/tools/mcp_oauth_manager.py +++ b/tools/mcp_oauth_manager.py @@ -29,14 +29,15 @@ except ImportError: # pragma: no cover — SDK required in CI; module must stil @dataclass class _ProviderEntry: - """Per-server OAuth state. ``last_mtime_ns``: last-seen tokens-file mtime (0 = never read) - for external-refresh detection; ``lock`` binds to the first asyncio loop awaiting it (the MCP + """Per-server OAuth state. ``last_mtime_ns``: last-seen tokens-file mtime (0 = never read), + ``last_version`` its ``file_cache_version``, for external-refresh detection; ``lock`` binds to the first asyncio loop awaiting it (the MCP loop); ``pending_401`` dedupes thundering-herd 401s by failed access_token.""" server_url: str oauth_config: Optional[dict] provider: Optional[Any] = None last_mtime_ns: int = 0 + last_version: tuple = () lock: asyncio.Lock = field(default_factory=asyncio.Lock) pending_401: dict[str, asyncio.Future[bool]] = field(default_factory=dict) @@ -382,16 +383,21 @@ class MCPOAuthManager: async def invalidate_if_disk_changed(self, server_name: str, *, hermes_home: str | Path | None = None) -> bool: """Force the SDK provider to reload when the tokens file mtime changed (e.g. a cron refresh); True if so.""" from tools.mcp_oauth import _get_token_dir, _safe_filename + from utils import file_cache_version entry = self._entries.get(self._key(server_name, hermes_home)) if entry is None or entry.provider is None: return False async with entry.lock: try: - mtime_ns = (_get_token_dir(hermes_home) / f"{_safe_filename(server_name)}.json").stat().st_mtime_ns + # Hashed every time: a refresh atomically replaced in the same tick keeps the mtime, + # and a racy digest going empty later would read as a change and tear the session down. + version = file_cache_version(_get_token_dir(hermes_home) / f"{_safe_filename(server_name)}.json", + always_digest=True) except OSError: return False - if mtime_ns == entry.last_mtime_ns: + if version == entry.last_version: return False + mtime_ns, entry.last_version = version[0], version old, entry.last_mtime_ns = entry.last_mtime_ns, mtime_ns if old == 0 and getattr(getattr(entry.provider, "context", None), "current_tokens", None) is not None: # First observation with tokens already in memory only seeds the diff --git a/tui_gateway/methods_onboarding.py b/tui_gateway/methods_onboarding.py index c45f5c8814..f3333120e9 100644 --- a/tui_gateway/methods_onboarding.py +++ b/tui_gateway/methods_onboarding.py @@ -60,13 +60,11 @@ def _due_stamp() -> tuple: layer's ``config.yaml`` and ``.env`` (an admin's onboarding or provider policy).""" from hermes_cli import managed_scope from hermes_constants import get_default_hermes_root - from utils import file_signature - home = Path(_profile_home("default") or get_default_hermes_root()) managed = managed_scope.get_managed_dir() paths = (get_default_hermes_root() / "config.yaml", home / "config.yaml", home / ".env", home / "auth.json", *_shared_nous_store(), *((managed / "config.yaml", managed / ".env") if managed else ())) - return tuple(file_signature(st) if (st := _stat(path)) else None for path in paths) + return tuple(_version(path) for path in paths) def _shared_nous_store() -> tuple[Path, ...]: @@ -78,9 +76,10 @@ def _shared_nous_store() -> tuple[Path, ...]: return () -def _stat(path): +def _version(path): + from utils import file_cache_version try: - return path.stat() + return file_cache_version(path) except OSError: return None diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 6fd63b676b..00849ea49e 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -998,13 +998,13 @@ def _load_cfg_raw() -> dict: expansion applied here would be persisted on the next save). Behavioral reads use :func:`_load_cfg`. Cache keyed on the resolved path so profiles don't clobber.""" global _cfg_cache, _cfg_sig, _cfg_path - from hermes_cli.config_backend import config_exists, config_version + from hermes_cli.config_backend import config_cache_version, config_exists from hermes_cli.config import read_user_config_raw from hermes_cli.config_read_errors import FailedConfigRead try: p = _active_config_path() try: - sig = config_version(p) + sig = config_cache_version(p) except FileNotFoundError: # absent; any other stat error is a failed read, never "no config" sig = None with _cfg_lock: @@ -1032,13 +1032,13 @@ def _load_cfg() -> dict: def _save_cfg(cfg: dict): global _cfg_cache, _cfg_sig, _cfg_path from hermes_cli.config import atomic_config_replace - from hermes_cli.config_backend import config_version + from hermes_cli.config_backend import config_cache_version path = _active_config_path() atomic_config_replace(path, cfg) with _cfg_lock: _cfg_cache, _cfg_path = copy.deepcopy(cfg), path try: - _cfg_sig = config_version(path) + _cfg_sig = config_cache_version(path) except Exception: _cfg_sig = None diff --git a/utils.py b/utils.py index 6538220418..42c5661819 100644 --- a/utils.py +++ b/utils.py @@ -61,6 +61,11 @@ def mtime_is_racy(mtime_ns: int) -> bool: return time.time_ns() - mtime_ns < RACY_MTIME_WINDOW_NS +def content_digest(data: bytes) -> str: + """Short digest of ``data`` for cache keys (not a security primitive).""" + return hashlib.blake2b(data, digest_size=16).hexdigest() + + def racy_content_digest(path: str | Path, mtime_ns: int) -> str: """``""`` once ``mtime_ns`` has settled; while it is racy, a digest of the file's bytes. A cache keyed on a stat signature appends it, because two same-size in-place writes in one timestamp tick @@ -69,7 +74,30 @@ def racy_content_digest(path: str | Path, mtime_ns: int) -> str: if not mtime_is_racy(mtime_ns): return "" with open(path, "rb") as f: - return hashlib.blake2b(f.read(), digest_size=16).hexdigest() + return content_digest(f.read()) + + +def file_cache_version(path: str | Path, st: os.stat_result | None = None, *, + always_digest: bool = False) -> tuple[Any, ...]: + """Key for anything derived from a file's content: :func:`file_signature` plus a content digest. + + The digest is :func:`racy_content_digest` (empty once the file is 2 s old), which suits a cache + re-validated on every read: a racily-stored entry costs one re-parse when the file settles. + ``always_digest`` hashes on every call, for a signature stored and compared much later (a + change detector whose false "changed" restarts something, or a "still the broken file" record), + where the racy digest turning empty would read as a change. Raises ``OSError`` like ``os.stat``; + given the caller's ``st`` it does not: a file gone or unreadable since that stat gets the error's + name as its digest, which no readable version matches.""" + sig = file_signature(Path(path).stat() if st is None else st) + try: + if always_digest: + with open(path, "rb") as f: + return (*sig, content_digest(f.read())) + return (*sig, racy_content_digest(path, sig[0])) # sig[0] is st_mtime_ns + except OSError as exc: + if st is None: + raise + return (*sig, type(exc).__name__) def _preserve_file_mode(path: Path) -> int | None: @@ -647,13 +675,12 @@ _YAML_FILE_CACHE_LOCK = threading.Lock() def load_yaml_file_readonly(path: str | Path) -> Any: - """``fast_safe_load`` of a file, re-parsed only when its :func:`file_signature` changes. + """``fast_safe_load`` of a file, re-parsed only when its :func:`file_cache_version` changes. Returns the cached object itself — callers must never mutate it. Parse errors propagate and are not cached; a missing file raises ``FileNotFoundError`` like ``open`` does.""" path = Path(path) - st = path.stat() - sig = (*file_signature(st), racy_content_digest(path, st.st_mtime_ns)) + sig = file_cache_version(path) key = str(path) with _YAML_FILE_CACHE_LOCK: cached = _YAML_FILE_CACHE.get(key) diff --git a/website/docs/developer-guide/config-backend.md b/website/docs/developer-guide/config-backend.md index 218563a23f..a3c692f8ec 100644 --- a/website/docs/developer-guide/config-backend.md +++ b/website/docs/developer-guide/config-backend.md @@ -20,7 +20,7 @@ already reaches it. | `read_config_doc(path)` | Parsed user layer (raises like `open` + `fast_safe_load`) | | `read_config_doc_readonly(path)` | Signature-cached read; never mutate the result | | `config_version(path)` | Change signature (the file's stat tuple) | -| `config_cache_version(path)` / `file_cache_version(path)` | Signature for a cache of the parse: the stat tuple plus a content digest while the mtime is younger than `utils.RACY_MTIME_WINDOW_NS` | +| `config_cache_version(path)` (`config_cache_version_or_none`) | Signature for a cache of the parse: the stat tuple plus a content digest while the mtime is younger than `utils.RACY_MTIME_WINDOW_NS` (`utils.file_cache_version` on the file backend) | | `config_exists(path)` | Whether the layer exists | | `probe_config_readable(path)` | Raise `OSError` while the layer cannot be read (no parse) | | `write_config_document(path, doc)` / `write_config_key(path, key, value)` | Writes; callers normally use `atomic_config_write` / `atomic_config_replace` | @@ -29,7 +29,11 @@ Only `/config.yaml` is a user layer. Any other file passed to these helper source, a fixture) is read and written where it is. A cache of a parse keys on `config_cache_version`, not `config_version`: on kernels without multigrain timestamps a same-size in-place rewrite in the tick of a read keeps mtime, ctime, size and inode, so the stat tuple alone -pins the older parse for the life of the process (a rotated `api_key` is the usual casualty). The managed scope (`/etc/hermes`) stays an overlay on top +pins the older parse for the life of the process (a rotated `api_key` is the usual casualty). +Any other file a cache derives from (`auth.json`, pairing lists, `.env`, token files, the managed +overlay) keys on `utils.file_cache_version(path)`; a signature stored and compared much later (a +change detector whose false positive reconnects something, a parse-failure record) passes +`always_digest=True`, because the racy digest turns empty once the file settles. The managed scope (`/etc/hermes`) stays an overlay on top of whatever user layer the backend returns. ## Lints