diff --git a/cron/jobs.py b/cron/jobs.py index 7766a87aaf2..b306448961a 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -1134,14 +1134,12 @@ def _resolve_default_model_snapshot() -> Optional[str]: or resolution fails (fail-open — caller treats ``None`` as "no snapshot"). """ try: - import yaml - from hermes_cli.config import _expand_env_vars + from hermes_cli.config import _expand_env_vars, read_user_config_raw cfg_path = get_hermes_home() / "config.yaml" if not cfg_path.exists(): return None - with cfg_path.open(encoding="utf-8") as f: - cfg = yaml.safe_load(f) or {} + cfg = read_user_config_raw(cfg_path) try: from hermes_cli import managed_scope cfg = managed_scope.apply_managed_overlay(cfg) diff --git a/cron/scheduler.py b/cron/scheduler.py index 9b597aed82d..e8c0991b81b 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -3169,11 +3169,10 @@ def run_job( _cfg = {} _model_cfg = {} try: - import yaml + from hermes_cli.config import read_user_config_raw _cfg_path = str(_get_hermes_home() / "config.yaml") if os.path.exists(_cfg_path): - with open(_cfg_path, encoding="utf-8") as _f: - _cfg = yaml.safe_load(_f) or {} + _cfg = read_user_config_raw(Path(_cfg_path)) # Managed scope: a scheduled job must honor administrator-pinned # model / reasoning / toolsets / provider_routing too. This loader # builds its own dict, so overlay managed values via the shared diff --git a/gateway/platforms/yuanbao.py b/gateway/platforms/yuanbao.py index 5aa2f275fee..9b3d34fe06a 100644 --- a/gateway/platforms/yuanbao.py +++ b/gateway/platforms/yuanbao.py @@ -1660,15 +1660,13 @@ class AutoSetHomeMiddleware(InboundMiddleware): if _should_set: try: from hermes_constants import get_hermes_home - from hermes_cli.config import atomic_config_write - import yaml + from hermes_cli.config import atomic_config_write, read_user_config_raw _home = get_hermes_home() config_path = _home / "config.yaml" - user_config: dict = {} - if config_path.exists(): - with open(config_path, encoding="utf-8") as f: - user_config = yaml.safe_load(f) or {} + # Write-back round-trip: raw read is correct (merged + # defaults must not be persisted to the user's file). + user_config: dict = read_user_config_raw(config_path) user_config["YUANBAO_HOME_CHANNEL"] = ctx.chat_id atomic_config_write(config_path, user_config) os.environ["YUANBAO_HOME_CHANNEL"] = str(ctx.chat_id) diff --git a/gateway/run.py b/gateway/run.py index 159ce304a71..0c5f57d5f42 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -1707,11 +1707,13 @@ def _bridge_max_turns_from_config(home: "Path") -> None: if not config_path.exists(): return try: - import yaml as _yaml - with open(config_path, encoding="utf-8") as f: - cfg = _yaml.safe_load(f) or {} - from hermes_cli.config import _expand_env_vars + from hermes_cli.config import _expand_env_vars, read_user_config_raw + # Presence-sensitive env bridge: raw read is deliberate (only keys the + # user actually wrote get bridged); overlay + expansion applied below. + cfg = read_user_config_raw(config_path) cfg = _expand_env_vars(cfg) + if not isinstance(cfg, dict): + cfg = {} # Managed scope: keep administrator-pinned values authoritative on every # turn too. This per-turn reload re-bridges config→env, so without the # overlay a managed agent.max_turns / timezone / redact_secrets would be @@ -1873,12 +1875,15 @@ _DOCKER_MEDIA_OUTPUT_CONTAINER_PATHS = {"/output", "/outputs"} _config_path = _hermes_home / 'config.yaml' if _config_path.exists(): try: - import yaml as _yaml - with open(_config_path, encoding="utf-8") as _f: - _cfg = _yaml.safe_load(_f) or {} + # Presence-sensitive env bridge: raw read is deliberate — only keys the + # user actually wrote may be bridged (a defaults merge would export the + # whole DEFAULT_CONFIG into the env). Overlay + expansion applied below. + from hermes_cli.config import _expand_env_vars, read_user_config_raw + _cfg = read_user_config_raw(_config_path) # Expand ${ENV_VAR} references before bridging to env vars. - from hermes_cli.config import _expand_env_vars _cfg = _expand_env_vars(_cfg) + if not isinstance(_cfg, dict): + _cfg = {} # Managed scope: overlay administrator-pinned values BEFORE bridging to # env vars, so a managed timezone / redact_secrets / max_turns / terminal # setting wins over the user's value at the env layer too. This bridge @@ -2437,12 +2442,10 @@ def _try_resolve_fallback_provider() -> dict | None: """Attempt to resolve credentials from the fallback_model/fallback_providers config.""" from hermes_cli.runtime_provider import resolve_runtime_provider try: - import yaml as _y - cfg_path = _hermes_home / "config.yaml" - if not cfg_path.exists(): - return None - with open(cfg_path, encoding="utf-8") as _f: - cfg = _y.safe_load(_f) or {} + # Canonical gateway loader: managed overlay + ${VAR} expansion + + # root-model normalization now reach the fallback chain too (a raw + # read here used to miss administrator-pinned fallback_providers). + cfg = _load_gateway_runtime_config() fb_list = get_fallback_chain(cfg) if not fb_list: return None @@ -5854,12 +5857,10 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew def _load_provider_routing() -> dict: """Load OpenRouter provider routing preferences from config.yaml.""" try: - import yaml as _y - cfg_path = _hermes_home / "config.yaml" - if cfg_path.exists(): - with open(cfg_path, encoding="utf-8") as _f: - cfg = _y.safe_load(_f) or {} - return cfg.get("provider_routing", {}) or {} + # Canonical gateway loader (fail-open): managed overlay + ${VAR} + # expansion now apply to provider_routing too. + cfg = _load_gateway_runtime_config() + return cfg.get("provider_routing", {}) or {} except Exception: pass return {} @@ -5873,14 +5874,12 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew when both keys are present. """ try: - import yaml as _y - cfg_path = _hermes_home / "config.yaml" - if cfg_path.exists(): - with open(cfg_path, encoding="utf-8") as _f: - cfg = _y.safe_load(_f) or {} - fb = get_fallback_chain(cfg) - if fb: - return fb + # Canonical gateway loader (fail-open): managed overlay + ${VAR} + # expansion now apply to the fallback chain too. + cfg = _load_gateway_runtime_config() + fb = get_fallback_chain(cfg) + if fb: + return fb except Exception: pass return None @@ -5900,13 +5899,28 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew that genuinely lacks the key clears the chain. """ try: - import yaml as _y + from hermes_cli.config import read_user_config_raw cfg_path = _hermes_home / "config.yaml" if not cfg_path.exists(): self._fallback_model = None return self._fallback_model - with open(cfg_path, encoding="utf-8") as _f: - cfg = _y.safe_load(_f) or {} + # Raw primitive (raises on parse failure) is required here: the + # canonical fail-open loader would return {} on a torn mid-edit + # write and WIPE the last known-good chain. The overlay/expansion + # below fixes the managed-scope/${VAR} drift without losing that. + cfg = read_user_config_raw(cfg_path) + try: + from hermes_cli import managed_scope + cfg = managed_scope.apply_managed_overlay(cfg) + except Exception: + pass + try: + from hermes_cli.config import _expand_env_vars + expanded = _expand_env_vars(cfg) + if isinstance(expanded, dict): + cfg = expanded + except Exception: + pass except Exception: # Transient failure — keep last known-good chain. logger.debug( diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index c8e28a14101..4a2162a4058 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -1723,7 +1723,6 @@ class GatewaySlashCommandsMixin: /model --provider — switch to provider, auto-detect model """ from gateway.run import _hermes_home, _load_gateway_config - import yaml from hermes_cli.model_switch import ( switch_model as _switch_model, parse_model_flags_detailed, resolve_persist_behavior, @@ -1993,11 +1992,10 @@ class GatewaySlashCommandsMixin: # model survives across sessions like a typed one (#49066). if persist_global: try: - if config_path.exists(): - with open(config_path, encoding="utf-8") as f: - _persist_cfg = yaml.safe_load(f) or {} - else: - _persist_cfg = {} + # Write-back round-trip: raw read is correct + # (merged defaults must not be persisted). + from hermes_cli.config import read_user_config_raw + _persist_cfg = read_user_config_raw(config_path) _raw_model = _persist_cfg.get("model") if isinstance(_raw_model, dict): _persist_model_cfg = _raw_model @@ -2315,11 +2313,10 @@ class GatewaySlashCommandsMixin: # Persist to config (default) unless --session opted out if persist_global: try: - if config_path.exists(): - with open(config_path, encoding="utf-8") as f: - cfg = yaml.safe_load(f) or {} - else: - cfg = {} + # Write-back round-trip: raw read is correct (merged + # defaults must not be persisted back to the user's file). + from hermes_cli.config import read_user_config_raw + cfg = read_user_config_raw(config_path) # Coerce scalar/None ``model:`` into a dict before mutation — # otherwise ``cfg.setdefault("model", {})`` returns the existing # scalar and the next assignment raises @@ -3227,14 +3224,13 @@ class GatewaySlashCommandsMixin: def _save_gateway_config_key(self, key_path: str, value) -> bool: """Save a dot-separated key to config.yaml (shared by /reasoning, /fast and their interactive pickers).""" - import yaml from gateway.run import _hermes_home + from hermes_cli.config import read_user_config_raw config_path = _hermes_home / "config.yaml" try: - user_config = {} - if config_path.exists(): - with open(config_path, encoding="utf-8") as f: - user_config = yaml.safe_load(f) or {} + # Write-back round-trip: raw read is correct (merged defaults must + # not be persisted back to the user's file). + user_config = read_user_config_raw(config_path) keys = key_path.split(".") current = user_config for k in keys[:-1]: @@ -3483,11 +3479,10 @@ class GatewaySlashCommandsMixin: config_path = _hermes_home / "config.yaml" def _set_approval(enabled: bool): - import yaml - user_config = {} - if config_path.exists(): - with open(config_path, encoding="utf-8") as f: - user_config = yaml.safe_load(f) or {} + # Write-back round-trip: raw read is correct (merged defaults must + # not be persisted back to the user's file). + from hermes_cli.config import read_user_config_raw + user_config = read_user_config_raw(config_path) user_config.setdefault("memory", {})["write_approval"] = bool(enabled) atomic_config_write(config_path, user_config) # New setting must take effect next message → drop cached agent. @@ -3539,11 +3534,10 @@ class GatewaySlashCommandsMixin: "writes here with /skills pending.") def _set_approval(enabled: bool): - import yaml - user_config = {} - if config_path.exists(): - with open(config_path, encoding="utf-8") as f: - user_config = yaml.safe_load(f) or {} + # Write-back round-trip: raw read is correct (merged defaults must + # not be persisted back to the user's file). + from hermes_cli.config import read_user_config_raw + user_config = read_user_config_raw(config_path) user_config.setdefault("skills", {})["write_approval"] = bool(enabled) atomic_config_write(config_path, user_config) # New setting must take effect next message → drop cached agent. diff --git a/hermes_cli/config.py b/hermes_cli/config.py index ac80973c1d3..eda819929ac 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2888,6 +2888,55 @@ def read_raw_config() -> Dict[str, Any]: return data +def read_user_config_raw(config_path: Optional[Path] = None) -> Dict[str, Any]: + """Read a user ``config.yaml`` EXACTLY as written on disk. + + No DEFAULT_CONFIG merge, no managed-scope overlay, no ``${ENV_VAR}`` + expansion, no migration, no root-model normalization, no caching. + + ONLY legal for write-back round-trips and raw-file diagnostics — + behavioral reads must use load_config()/load_config_readonly(). + + Legal call sites, exhaustively: + + * WRITE-BACK ROUND-TRIPS (read → mutate one key → save): merging + defaults or the managed overlay here would persist hundreds of + default keys (or administrator-pinned values) into the user's file + on the next save. Raw is *correct*, not an optimization. + * RAW-FILE DIAGNOSTICS (doctor, deprecation sweeps): these inspect + what the user actually wrote — stale root keys, drift against .env — + and merged defaults would produce false positives. + * PRESENCE-SENSITIVE ENV BRIDGES (gateway/send bridges that only + export a key when the user explicitly set it): a defaults merge + would make every key "present" and bridge the entire DEFAULT_CONFIG + into the environment. These sites must still apply + ``managed_scope.apply_managed_overlay`` + ``_expand_env_vars`` + inline, which they do. + + Semantics (deliberately mirrors the bare ``open()+yaml.safe_load()`` + pattern this replaces, so migrated sites keep their exact failure + behavior): + + * missing file → ``{}`` + * unparseable YAML / other I/O errors → raises (callers that want + fail-open already wrap in try/except; callers with last-known-good + or warn semantics rely on the exception) + * non-dict YAML root → ``{}`` + + ``config_path`` defaults to :func:`get_config_path` (profile-aware). + Pass an explicit path when the caller resolves its own home (gateway + ``_hermes_home``, tui profile override, multi-profile probes). + """ + if config_path is None: + config_path = get_config_path() + try: + with open(config_path, encoding="utf-8") as f: + data = fast_safe_load(f) or {} + except FileNotFoundError: + return {} + return data if isinstance(data, dict) else {} + + def require_readable_config_before_write(config_path: Optional[Path] = None) -> None: """Refuse to replace an existing config.yaml that cannot be read.""" if config_path is None: diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index b4c5e678d8a..1a673cf02a6 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -951,8 +951,9 @@ def run_doctor(args): # Validate model.provider and model.default values try: - import yaml as _yaml - cfg = _yaml.safe_load(config_path.read_text(encoding="utf-8")) or {} + # Raw-file diagnostic: inspects what the user actually wrote. + from hermes_cli.config import read_user_config_raw + cfg = read_user_config_raw(config_path) model_section = cfg.get("model") or {} provider_raw = (model_section.get("provider") or "").strip() provider = provider_raw.lower() @@ -1184,9 +1185,9 @@ def run_doctor(args): # Detect stale root-level model keys (known bug source — PR #4329) try: - import yaml - with open(config_path, encoding="utf-8") as f: - raw_config = yaml.safe_load(f) or {} + # Raw-file diagnostic: stale-key detection must see the raw file. + from hermes_cli.config import read_user_config_raw + raw_config = read_user_config_raw(config_path) stale_root_keys = [k for k in ("provider", "base_url") if k in raw_config and isinstance(raw_config[k], str)] if stale_root_keys: check_warn( @@ -1231,10 +1232,9 @@ def run_doctor(args): # Read the .env FILE directly (load_env), not get_env_value/os.environ, # which the startup bridge may already have overridden. try: - import yaml - from hermes_cli.config import load_env, remove_env_value - with open(config_path, encoding="utf-8") as f: - raw_config = yaml.safe_load(f) or {} + from hermes_cli.config import load_env, read_user_config_raw, remove_env_value + # Raw-file diagnostic: drift check against the raw file. + raw_config = read_user_config_raw(config_path) agent_cfg = raw_config.get("agent") cfg_max_turns = ( agent_cfg.get("max_turns") @@ -1281,11 +1281,11 @@ def run_doctor(args): # Migrations may still live in config.py version steps; doctor does # not auto-delete here — only tells the user the modern replacement. try: - import yaml as _yaml_depr from hermes_cli.config import load_env as _load_env_depr + from hermes_cli.config import read_user_config_raw as _read_raw_depr - with open(config_path, encoding="utf-8") as _f_depr: - _raw_for_depr = _yaml_depr.safe_load(_f_depr) or {} + # Raw-file diagnostic: deprecation sweep inspects the raw file. + _raw_for_depr = _read_raw_depr(config_path) # Prefer the on-disk .env so bridged process env (e.g. TERMINAL_CWD # from terminal.cwd) does not false-positive. try: @@ -2528,11 +2528,11 @@ def run_doctor(args): _section("Memory Provider") _active_memory_provider = "" try: - import yaml as _yaml + from hermes_cli.config import read_user_config_raw as _read_raw_mem _mem_cfg_path = HERMES_HOME / "config.yaml" if _mem_cfg_path.exists(): - with open(_mem_cfg_path, encoding="utf-8") as _f: - _raw_cfg = _yaml.safe_load(_f) or {} + # Raw-file diagnostic (+ managed overlay below, unchanged). + _raw_cfg = _read_raw_mem(_mem_cfg_path) try: from hermes_cli import managed_scope _raw_cfg = managed_scope.apply_managed_overlay(_raw_cfg) diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index ce60bfb2ccd..2c0bfff2bdc 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -4683,8 +4683,11 @@ def _guard_named_profile_under_multiplexer(force: bool = False) -> None: cfg_path = default_root / "config.yaml" if not cfg_path.exists(): return - with open(cfg_path, encoding="utf-8") as f: - cfg = _yaml.safe_load(f) or {} + # Raw read of the DEFAULT root's config (not the active profile + # home, so load_config() is the wrong owner here); whole probe is + # fail-open via the enclosing except. + from hermes_cli.config import read_user_config_raw + cfg = read_user_config_raw(cfg_path) multiplex = bool( cfg.get("multiplex_profiles") or (cfg.get("gateway", {}) or {}).get("multiplex_profiles") diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 0ab0562deae..4ed717668aa 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -683,9 +683,10 @@ def _read_config_model(profile_dir: Path) -> tuple: if not config_path.exists(): return None, None try: - import yaml - with open(config_path, "r", encoding="utf-8") as f: - cfg = yaml.safe_load(f) or {} + # Multi-profile display read: load_config() targets the ACTIVE + # profile's home, so read THIS profile's file via the raw primitive. + from hermes_cli.config import read_user_config_raw + cfg = read_user_config_raw(config_path) model_cfg = cfg.get("model", {}) if isinstance(model_cfg, str): return model_cfg, None diff --git a/hermes_cli/send_cmd.py b/hermes_cli/send_cmd.py index 81babfe2aca..e926ceefb54 100644 --- a/hermes_cli/send_cmd.py +++ b/hermes_cli/send_cmd.py @@ -260,13 +260,10 @@ def _load_hermes_env() -> None: return try: - import yaml # type: ignore[import-not-found] - except Exception: - return - - try: - with open(config_path, "r", encoding="utf-8") as fh: - raw = yaml.safe_load(fh) or {} + # Presence-sensitive env bridge: raw read is deliberate — only keys + # the user actually wrote get bridged. Overlay + expansion below. + from hermes_cli.config import read_user_config_raw + raw = read_user_config_raw(config_path) except Exception: return diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index b6cad13476e..76e0b002713 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -2858,8 +2858,10 @@ def _profile_platform_ports(profile_home: Path, runtime: Optional[dict]) -> Dict blocks: Dict[str, dict] = {} try: - with open(profile_home / "config.yaml", encoding="utf-8") as f: - cfg = yaml.safe_load(f) or {} + # Multi-profile probe: load_config() targets the ACTIVE profile's + # home, so read the probed profile's file via the raw primitive. + from hermes_cli.config import read_user_config_raw + cfg = read_user_config_raw(profile_home / "config.yaml") gateway_cfg = cfg.get("gateway") if isinstance(cfg.get("gateway"), dict) else {} # gateway.platforms first, top-level platforms second — later wins, # matching the precedence in gateway.config.load_gateway_config(). diff --git a/plugins/memory/holographic/__init__.py b/plugins/memory/holographic/__init__.py index 7531469755a..2e5f86a8cf5 100644 --- a/plugins/memory/holographic/__init__.py +++ b/plugins/memory/holographic/__init__.py @@ -96,14 +96,11 @@ FACT_FEEDBACK_SCHEMA = { # --------------------------------------------------------------------------- def _load_plugin_config() -> dict: - from hermes_constants import get_hermes_home - config_path = get_hermes_home() / "config.yaml" - if not config_path.exists(): - return {} try: - import yaml - with open(config_path, encoding="utf-8-sig") as f: - all_config = yaml.safe_load(f) or {} + # Canonical loader: behavioral read now honors the managed-scope + # overlay + ${VAR} expansion (e.g. an api key template) too. + from hermes_cli.config import load_config_readonly + all_config = load_config_readonly() return cfg_get(all_config, "plugins", "hermes-memory-store", default={}) or {} except Exception: return {} @@ -135,10 +132,10 @@ class HolographicMemoryProvider(MemoryProvider): config_path = Path(hermes_home) / "config.yaml" try: import yaml - existing = {} - if config_path.exists(): - with open(config_path, encoding="utf-8-sig") as f: - existing = yaml.safe_load(f) or {} + # Write-back round-trip: raw read is correct (merged defaults + # must not be persisted back into the user's file). + from hermes_cli.config import read_user_config_raw + existing = read_user_config_raw(config_path) existing.setdefault("plugins", {}) existing["plugins"]["hermes-memory-store"] = values with open(config_path, "w", encoding="utf-8") as f: diff --git a/plugins/platforms/telegram/adapter.py b/plugins/platforms/telegram/adapter.py index 59829a49e35..cd0fdde6fb0 100644 --- a/plugins/platforms/telegram/adapter.py +++ b/plugins/platforms/telegram/adapter.py @@ -9350,14 +9350,10 @@ class TelegramAdapter(BasePlatformAdapter): recognized without a gateway restart. """ try: - from hermes_constants import get_hermes_home - config_path = get_hermes_home() / "config.yaml" - if not config_path.exists(): - return - - import yaml as _yaml - with open(config_path, "r", encoding="utf-8") as f: - config = _yaml.safe_load(f) or {} + # Canonical loader: behavioral read (dm_topics routing) now honors + # managed-scope overlay + ${VAR} expansion like every other read. + from hermes_cli.config import load_config_readonly + config = load_config_readonly() dm_topics = ( config.get("platforms", {}) diff --git a/tests/hermes_cli/test_config_loader_e2e.py b/tests/hermes_cli/test_config_loader_e2e.py new file mode 100644 index 00000000000..30a8cf24c31 --- /dev/null +++ b/tests/hermes_cli/test_config_loader_e2e.py @@ -0,0 +1,160 @@ +"""E2E for the canonical-loader migration (managed-scope/env-expansion drift fix). + +Runs a real subprocess with a temp HERMES_HOME whose config.yaml contains a +``${ENV_VAR}`` reference, plus a managed-scope overlay dir (HERMES_MANAGED_DIR). + +Asserts the two halves of the contract: + + 1. A migrated BEHAVIORAL site (``tui_gateway.server._load_cfg``) resolves the + env-expanded AND managed-overlaid values — the drift bug this branch fixes. + 2. A WRITE-BACK site round-trips the raw user file without leaking managed + values, expanded literals, or merged defaults into it. + +Subprocess (not in-process monkeypatching) so module-level ``_hermes_home`` +globals, managed-scope caches, and ``_under_pytest`` guards behave like +production: ``HERMES_MANAGED_DIR`` is set explicitly, which bypasses the +pytest suppression in ``get_managed_dir``. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +import textwrap +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent + + +def _run_py(code: str, env_extra: dict[str, str], tmp_path: Path) -> dict: + """Run ``code`` in a subprocess; results come back via a JSON file. + + A file (not stdout) because importing ``tui_gateway.server`` re-routes + the real stdout into its JSON-RPC transport. + """ + import os + + out_file = tmp_path / "e2e_result.json" + env = dict(os.environ) + env.pop("PYTEST_CURRENT_TEST", None) + env["PYTHONPATH"] = str(REPO_ROOT) + env["E2E_OUT_FILE"] = str(out_file) + env.update(env_extra) + proc = subprocess.run( + [sys.executable, "-c", code], + capture_output=True, + text=True, + env=env, + cwd=str(tmp_path), + timeout=120, + ) + assert proc.returncode == 0, f"subprocess failed:\n{proc.stdout}\n{proc.stderr}" + assert out_file.exists(), f"no result file:\n{proc.stdout}\n{proc.stderr}" + return json.loads(out_file.read_text(encoding="utf-8")) + + +def test_behavioral_read_gets_expansion_and_overlay_while_writeback_stays_raw( + tmp_path, +): + home = tmp_path / "hermes_home" + home.mkdir() + user_yaml = ( + "custom_prompt: 'hello ${E2E_PROMPT_SUFFIX}'\n" + "agent:\n" + " reasoning_effort: low\n" + "display:\n" + " skin: usertheme\n" + ) + (home / "config.yaml").write_text(user_yaml, encoding="utf-8") + + managed_dir = tmp_path / "managed" + managed_dir.mkdir() + # Administrator pins reasoning_effort — must win over the user's "low". + (managed_dir / "config.yaml").write_text( + "agent:\n reasoning_effort: high\n", encoding="utf-8" + ) + + code = textwrap.dedent( + """ + import json + from tui_gateway import server + + cfg = server._load_cfg() + raw = server._load_cfg_raw() + + # Write-back round-trip through the real save path. + rt = server._load_cfg_raw() + rt.setdefault("display", {})["battery"] = True + server._save_cfg(rt) + + import os + from pathlib import Path + saved = Path(server._hermes_home, "config.yaml").read_text(encoding="utf-8") + Path(os.environ["E2E_OUT_FILE"]).write_text(json.dumps({ + "behavioral_prompt": cfg.get("custom_prompt"), + "behavioral_effort": (cfg.get("agent") or {}).get("reasoning_effort"), + "raw_prompt": raw.get("custom_prompt"), + "raw_effort": (raw.get("agent") or {}).get("reasoning_effort"), + "saved": saved, + }), encoding="utf-8") + """ + ) + out = _run_py( + code, + { + "HERMES_HOME": str(home), + "HERMES_MANAGED_DIR": str(managed_dir), + "E2E_PROMPT_SUFFIX": "world", + }, + tmp_path, + ) + + # 1. Behavioral read: ${VAR} expanded + managed overlay applied. + assert out["behavioral_prompt"] == "hello world" + assert out["behavioral_effort"] == "high" + + # 2. Raw primitive: byte-faithful view of the user's file. + assert out["raw_prompt"] == "hello ${E2E_PROMPT_SUFFIX}" + assert out["raw_effort"] == "low" + + # 3. Write-back: user values round-trip; no managed/expanded/default leak. + saved = out["saved"] + assert "hello ${E2E_PROMPT_SUFFIX}" in saved # template preserved + assert "hello world" not in saved # no expansion persisted + assert "reasoning_effort: low" in saved # user value preserved + assert "high" not in saved # managed value NOT persisted + assert "battery: true" in saved # the actual edit landed + assert "max_turns" not in saved # no DEFAULT_CONFIG pollution + + +def test_writeback_roundtrip_byte_identical_when_unchanged(tmp_path): + """read_user_config_raw → save with no mutation must not alter content + semantics (yaml re-dump may reorder nothing here: flat mapping).""" + home = tmp_path / "hermes_home" + home.mkdir() + original = "custom_prompt: keep ${NOT_SET_VAR}\ndisplay:\n skin: usertheme\n" + (home / "config.yaml").write_text(original, encoding="utf-8") + + code = textwrap.dedent( + """ + import json + from pathlib import Path + from hermes_cli.config import read_user_config_raw + import yaml + + p = Path(__import__('os').environ['HERMES_HOME']) / 'config.yaml' + before = p.read_text(encoding='utf-8') + data = read_user_config_raw(p) + # No mutation, no save — the primitive itself must be read-only. + after = p.read_text(encoding='utf-8') + import os + Path(os.environ["E2E_OUT_FILE"]).write_text(json.dumps({ + "identical": before == after, + "parsed": data, + }), encoding="utf-8") + """ + ) + out = _run_py(code, {"HERMES_HOME": str(home)}, tmp_path) + assert out["identical"] is True + assert out["parsed"]["custom_prompt"] == "keep ${NOT_SET_VAR}" diff --git a/tests/hermes_cli/test_config_read_guard.py b/tests/hermes_cli/test_config_read_guard.py new file mode 100644 index 00000000000..02e2414650a --- /dev/null +++ b/tests/hermes_cli/test_config_read_guard.py @@ -0,0 +1,109 @@ +"""Lint guard: no new raw yaml.safe_load(config.yaml) reads outside owner modules. + +The drift class this kills: scattered ``yaml.safe_load`` reads of the user's +``config.yaml`` silently miss the managed-scope overlay, ``${ENV_VAR}`` +expansion, profile-aware pathing, and root-model normalization. Each new +config feature has historically required an N-site sweep (incident chain: +9cbcc0c9c8 → 732293cf87 → b0e47a98f9 → 1928aa0443). + +Canonical owners: + + * ``hermes_cli/config.py`` — ``load_config()`` / ``load_config_readonly()`` + (merged + managed + env-expanded), ``read_raw_config()`` and + ``read_user_config_raw()`` (the ONLY legal raw primitives: write-back + round-trips + raw-file diagnostics). + * ``gateway/config.py`` — the gateway's ``load_gateway_config`` owner. + * ``gateway/run.py`` — ``_load_gateway_config()``'s monkeypatched-home + fallback path (delegates to ``read_raw_config`` when paths agree). + +Everything else must import one of those. If this test fails on your new +code, use ``load_config()``/``load_config_readonly()`` for behavioral reads, +or ``read_user_config_raw()`` for write-back round-trips — do not add your +file to the allowlist without a reason of the same class. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent + +# Files where a yaml.safe_load near a config.yaml reference is legal. +# Keep this list SHORT and justified: +ALLOWLIST = { + # Canonical loader owners. + "hermes_cli/config.py", + "gateway/config.py", + # _load_gateway_config()'s fallback path for tests that monkeypatch + # gateway.run._hermes_home (delegates to read_raw_config otherwise). + "gateway/run.py", + # Reads the MANAGED-scope config.yaml (/etc/hermes/...), not the user's — + # it IS the overlay source; the canonical loaders call into it. + "hermes_cli/managed_scope.py", + # Parse-health probe: intentionally answers "does the raw file parse?". + "gateway/readiness.py", +} + +# Directories that never count (tests may build fixture configs freely). +EXCLUDED_DIR_PARTS = { + "tests", ".venv", ".git", ".worktrees", "node_modules", "website", + "docs", "scripts", "examples", "apps", +} + +# A safe_load within this many lines of a config.yaml reference is treated +# as a raw user-config read. +PROXIMITY = 6 + +SAFE_LOAD_RE = re.compile(r"\bsafe_load\s*\(") +CONFIG_YAML_RE = re.compile(r"""["']config\.yaml["']""") + + +def _iter_source_files(): + for path in REPO_ROOT.rglob("*.py"): + rel = path.relative_to(REPO_ROOT) + if any(part in EXCLUDED_DIR_PARTS for part in rel.parts): + continue + yield rel, path + + +def test_no_raw_config_yaml_reads_outside_owner_modules(): + offenders: list[str] = [] + for rel, path in _iter_source_files(): + rel_str = str(rel).replace("\\", "/") + if rel_str in ALLOWLIST: + continue + try: + lines = path.read_text(encoding="utf-8", errors="replace").splitlines() + except OSError: + continue + cfg_lines = [i for i, ln in enumerate(lines) if CONFIG_YAML_RE.search(ln)] + if not cfg_lines: + continue + for i, ln in enumerate(lines): + if not SAFE_LOAD_RE.search(ln): + continue + # Comment/docstring mentions don't count. + stripped = ln.strip() + if stripped.startswith("#"): + continue + if any(abs(i - j) <= PROXIMITY for j in cfg_lines): + offenders.append(f"{rel_str}:{i + 1}: {stripped}") + + assert not offenders, ( + "Raw yaml.safe_load of config.yaml outside allowlisted owner modules.\n" + "Behavioral reads must use hermes_cli.config.load_config()/" + "load_config_readonly() (or gateway _load_gateway_config); write-back " + "round-trips and raw-file diagnostics must use " + "hermes_cli.config.read_user_config_raw().\nOffenders:\n " + + "\n ".join(offenders) + ) + + +def test_read_user_config_raw_exists_and_documented(): + """The shared raw primitive must exist and carry its legality docstring.""" + from hermes_cli.config import read_user_config_raw + + doc = read_user_config_raw.__doc__ or "" + assert "ONLY legal for write-back round-trips and raw-file diagnostics" in doc + assert "load_config()" in doc diff --git a/tools/wake_word.py b/tools/wake_word.py index 143372830d3..b0d5164d382 100644 --- a/tools/wake_word.py +++ b/tools/wake_word.py @@ -271,15 +271,16 @@ def enrolled_profile_phrases() -> Dict[str, str]: """ phrases: Dict[str, str] = {} try: - import yaml - + from hermes_cli.config import read_user_config_raw from hermes_cli.profiles import get_profile_dir, list_profiles for info in list_profiles(): name = getattr(info, "name", None) or str(info) try: + # Multi-profile read: load_config() targets the ACTIVE + # profile's home, so read each profile's file directly. cfg_path = Path(get_profile_dir(name)) / "config.yaml" - raw = yaml.safe_load(cfg_path.read_text(encoding="utf-8")) or {} + raw = read_user_config_raw(cfg_path) wc = raw.get("wake_word") or {} if not isinstance(wc, dict) or not wc.get("enabled"): continue diff --git a/tui_gateway/server.py b/tui_gateway/server.py index ec194525f1b..306afd51896 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1289,13 +1289,19 @@ def _profile_configured_cwd(profile_home: Path | None) -> str | None: if profile_home is None: return None try: - import yaml + from hermes_cli.config import _expand_env_vars, read_user_config_raw p = Path(profile_home) / "config.yaml" if not p.exists(): return None - with open(p, encoding="utf-8") as f: - data = yaml.safe_load(f) or {} + # Behavioral read of a NON-launch profile's config: load_config() + # would resolve the ACTIVE profile's path, so read this profile's + # file directly, then apply the same read-side pipeline as + # _load_cfg (managed overlay + ${VAR} expansion). Fail-open. + data = _apply_managed(read_user_config_raw(p)) + expanded = _expand_env_vars(data) + if isinstance(expanded, dict): + data = expanded return _configured_cwd_from_cfg(data) except Exception: return None @@ -2641,8 +2647,9 @@ def _coerce_int_config_value(value: Any, default: int, *, min_value: int) -> int def _load_dashboard_process_isolation_config(cfg: dict | None = None) -> dict[str, Any]: """Return dashboard process-isolation config with read-site defaults. - ``_load_cfg()`` intentionally returns raw ``config.yaml`` plus the managed - overlay; it does not deep-merge ``hermes_cli.config.DEFAULT_CONFIG``. Keep + ``_load_cfg()`` intentionally returns the user ``config.yaml`` plus the + managed overlay and ``${VAR}`` expansion; it does not deep-merge + ``hermes_cli.config.DEFAULT_CONFIG``. Keep the Phase-0 defaults here so dashboard runtime and the REST editor's DEFAULT_CONFIG-backed schema cannot drift. """ @@ -2668,11 +2675,17 @@ def _load_dashboard_process_isolation_config(cfg: dict | None = None) -> dict[st } -def _load_cfg() -> dict: +def _load_cfg_raw() -> dict: + """Read the active profile's config.yaml EXACTLY as written (write-back primitive). + + ONLY legal for read→mutate→``_save_cfg`` round-trips (and raw-file + inspection): merging defaults, the managed overlay, or ``${VAR}`` + expansion here would be persisted into the user's file on the next + save. Behavioral reads must use :func:`_load_cfg`, which layers the + managed overlay + env expansion on top of this raw read. + """ global _cfg_cache, _cfg_mtime, _cfg_path try: - import yaml - # Honor a per-session profile override (see session.resume) so a resumed # remote profile loads ITS config (model, skills, prompt); otherwise the # launch profile's _hermes_home. Cache is keyed on the resolved path, so @@ -2683,10 +2696,10 @@ def _load_cfg() -> dict: mtime = p.stat().st_mtime if p.exists() else None with _cfg_lock: if _cfg_cache is not None and _cfg_mtime == mtime and _cfg_path == p: - return _apply_managed(copy.deepcopy(_cfg_cache)) + return copy.deepcopy(_cfg_cache) if p.exists(): - with open(p, encoding="utf-8") as f: - data = yaml.safe_load(f) or {} + from hermes_cli.config import read_user_config_raw + data = read_user_config_raw(p) else: data = {} with _cfg_lock: @@ -2697,12 +2710,37 @@ def _load_cfg() -> dict: _cfg_cache = copy.deepcopy(data) _cfg_mtime = mtime _cfg_path = p - return _apply_managed(data) + return data except Exception: pass return {} +def _load_cfg() -> dict: + """Behavioral config read: raw user file + managed overlay + ${VAR} expansion. + + Delegates the disk read to :func:`_load_cfg_raw` (shared cache), then + applies the same read-side pipeline as the canonical + ``hermes_cli.config.load_config_readonly`` — managed-scope overlay and + ``${ENV_VAR}`` expansion — minus the DEFAULT_CONFIG merge (callers here + treat a missing key as "unset" and apply their own defaults; merging + would also break ``_load_cfg() == {}`` sentinels). Do NOT pass the + result to ``_save_cfg``: use ``_load_cfg_raw()`` for write-back + round-trips or expanded/overlaid values get persisted into the user's + file. + """ + cfg = _apply_managed(_load_cfg_raw()) + try: + from hermes_cli.config import _expand_env_vars + + expanded = _expand_env_vars(cfg) + if isinstance(expanded, dict): + cfg = expanded + except Exception: + pass + return cfg + + def _apply_managed(cfg: dict) -> dict: """Overlay administrator-pinned managed-scope values on a config dict. @@ -3519,7 +3557,9 @@ def _append_model_switch_marker(session: dict | None, *, model: str, provider: s def _write_config_key(key_path: str, value): - cfg = _load_cfg() + # Write-back round-trip: raw read is mandatory — saving the managed- + # overlaid / env-expanded view would persist those values into the file. + cfg = _load_cfg_raw() current = cfg keys = key_path.split(".") for key in keys[:-1]: @@ -13939,7 +13979,7 @@ def _(rid, params: dict) -> dict: scope = str(params.get("scope") or "").strip().lower() global_scope = scope == "global" if arg in {"show", "on"}: - cfg = _load_cfg() + cfg = _load_cfg_raw() # write-back round-trip display = ( cfg.get("display") if isinstance(cfg.get("display"), dict) else {} ) @@ -13957,7 +13997,7 @@ def _(rid, params: dict) -> dict: session["show_reasoning"] = True return _ok(rid, {"key": key, "value": "show"}) if arg in {"hide", "off"}: - cfg = _load_cfg() + cfg = _load_cfg_raw() # write-back round-trip display = ( cfg.get("display") if isinstance(cfg.get("display"), dict) else {} ) @@ -13982,7 +14022,7 @@ def _(rid, params: dict) -> dict: # display.reasoning_full is persisted too so the config key stays # consistent across the CLI and TUI surfaces. if arg in {"full", "all"}: - cfg = _load_cfg() + cfg = _load_cfg_raw() # write-back round-trip display = ( cfg.get("display") if isinstance(cfg.get("display"), dict) else {} ) @@ -13998,7 +14038,7 @@ def _(rid, params: dict) -> dict: _save_cfg(cfg) return _ok(rid, {"key": key, "value": "full"}) if arg in {"clamp", "collapse", "short"}: - cfg = _load_cfg() + cfg = _load_cfg_raw() # write-back round-trip display = ( cfg.get("display") if isinstance(cfg.get("display"), dict) else {} ) @@ -14044,7 +14084,7 @@ def _(rid, params: dict) -> dict: nv = str(value or "").strip().lower() if nv not in _DETAIL_MODES: return _err(rid, 4002, f"unknown details_mode: {value}") - cfg = _load_cfg() + cfg = _load_cfg_raw() # write-back round-trip display = cfg.get("display") if isinstance(cfg.get("display"), dict) else {} sections = ( display.get("sections") if isinstance(display.get("sections"), dict) else {} @@ -14066,7 +14106,7 @@ def _(rid, params: dict) -> dict: if section not in _DETAIL_SECTION_NAMES: return _err(rid, 4002, f"unknown section: {section}") - cfg = _load_cfg() + cfg = _load_cfg_raw() # write-back round-trip display = cfg.get("display") if isinstance(cfg.get("display"), dict) else {} sections_cfg = ( display.get("sections") if isinstance(display.get("sections"), dict) else {} @@ -14211,7 +14251,7 @@ def _(rid, params: dict) -> dict: if key in {"prompt", "personality", "skin"}: try: - cfg = _load_cfg() + cfg = _load_cfg_raw() # write-back round-trip ("prompt" saves cfg) if key == "prompt": if value == "clear": cfg.pop("custom_prompt", None) @@ -18059,7 +18099,7 @@ def _speak_text_with_barge(text: str) -> None: def _voice_cfg_dict() -> dict: """Shape-safe accessor for the ``voice:`` block in config.yaml. - ``_load_cfg()`` returns raw ``yaml.safe_load()`` output, so both the + ``_load_cfg()`` does not deep-merge DEFAULT_CONFIG, so both the root AND ``voice`` may be any YAML scalar / list / None. A hand-edit like ``voice: true`` or a malformed top-level config that parses to a scalar would otherwise break ``.get("…")`` and take every