diff --git a/agent/onboarding.py b/agent/onboarding.py index cf7e20593e2..c29ea1529fb 100644 --- a/agent/onboarding.py +++ b/agent/onboarding.py @@ -209,7 +209,7 @@ def mark_seen(config_path: Path, flag: str) -> bool: """ try: import yaml - from utils import atomic_yaml_write + from hermes_cli.config import atomic_config_write except Exception as e: # pragma: no cover — dependency issue logger.debug("onboarding: failed to import yaml/utils: %s", e) return False @@ -228,7 +228,7 @@ def mark_seen(config_path: Path, flag: str) -> bool: if seen.get(flag) is True: return True # already marked — nothing to do seen[flag] = True - atomic_yaml_write(config_path, cfg) + atomic_config_write(config_path, cfg) return True except Exception as e: logger.debug("onboarding: failed to mark flag %s: %s", flag, e) diff --git a/gateway/platforms/yuanbao.py b/gateway/platforms/yuanbao.py index c6c410a0a43..69d5b75b5aa 100644 --- a/gateway/platforms/yuanbao.py +++ b/gateway/platforms/yuanbao.py @@ -1650,7 +1650,7 @@ class AutoSetHomeMiddleware(InboundMiddleware): if _should_set: try: from hermes_constants import get_hermes_home - from utils import atomic_yaml_write + from hermes_cli.config import atomic_config_write import yaml _home = get_hermes_home() @@ -1660,7 +1660,7 @@ class AutoSetHomeMiddleware(InboundMiddleware): with open(config_path, encoding="utf-8") as f: user_config = yaml.safe_load(f) or {} user_config["YUANBAO_HOME_CHANNEL"] = ctx.chat_id - atomic_yaml_write(config_path, user_config) + atomic_config_write(config_path, user_config) os.environ["YUANBAO_HOME_CHANNEL"] = str(ctx.chat_id) logger.info( "[%s] Auto-sethome: designated %s (%s) as Yuanbao home channel", diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index 0d7be83cf9e..a41fed13384 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -38,10 +38,9 @@ from gateway.session import ( build_session_key, is_shared_multi_user_session, ) -from hermes_cli.config import cfg_get, clear_model_endpoint_credentials +from hermes_cli.config import atomic_config_write, cfg_get, clear_model_endpoint_credentials from utils import ( atomic_json_write, - atomic_yaml_write, base_url_host_matches, is_truthy_value, ) @@ -2093,7 +2092,7 @@ class GatewaySlashCommandsMixin: if "agent" not in config or not isinstance(config.get("agent"), dict): config["agent"] = {} config["agent"]["system_prompt"] = "" - atomic_yaml_write(config_path, config) + atomic_config_write(config_path, config) except Exception as e: return t("gateway.personality.save_failed", error=str(e)) self._ephemeral_system_prompt = "" @@ -2106,7 +2105,7 @@ class GatewaySlashCommandsMixin: if "agent" not in config or not isinstance(config.get("agent"), dict): config["agent"] = {} config["agent"]["system_prompt"] = new_prompt - atomic_yaml_write(config_path, config) + atomic_config_write(config_path, config) except Exception as e: return t("gateway.personality.save_failed", error=str(e)) @@ -2652,7 +2651,7 @@ class GatewaySlashCommandsMixin: current[k] = {} current = current[k] current[keys[-1]] = value - atomic_yaml_write(config_path, user_config) + atomic_config_write(config_path, user_config) return True except Exception as e: logger.error("Failed to save config key %s: %s", key_path, e) @@ -2755,7 +2754,7 @@ class GatewaySlashCommandsMixin: with open(config_path, encoding="utf-8") as f: user_config = yaml.safe_load(f) or {} user_config.setdefault("memory", {})["write_approval"] = bool(enabled) - atomic_yaml_write(config_path, user_config) + atomic_config_write(config_path, user_config) # New setting must take effect next message → drop cached agent. self._evict_cached_agent(session_key) @@ -2811,7 +2810,7 @@ class GatewaySlashCommandsMixin: with open(config_path, encoding="utf-8") as f: user_config = yaml.safe_load(f) or {} user_config.setdefault("skills", {})["write_approval"] = bool(enabled) - atomic_yaml_write(config_path, user_config) + atomic_config_write(config_path, user_config) # New setting must take effect next message → drop cached agent. self._evict_cached_agent(session_key) @@ -2863,7 +2862,7 @@ class GatewaySlashCommandsMixin: current[k] = {} current = current[k] current[keys[-1]] = value - atomic_yaml_write(config_path, user_config) + atomic_config_write(config_path, user_config) return True except Exception as e: logger.error("Failed to save config key %s: %s", key_path, e) @@ -2960,7 +2959,7 @@ class GatewaySlashCommandsMixin: if platform_key not in display["platforms"] or not isinstance(display["platforms"].get(platform_key), dict): display["platforms"][platform_key] = {} display["platforms"][platform_key]["tool_progress"] = new_mode - atomic_yaml_write(config_path, user_config) + atomic_config_write(config_path, user_config) return ( f"{descriptions[new_mode]}\n" + t("gateway.verbose.saved_suffix", platform=platform_key) @@ -3035,7 +3034,7 @@ class GatewaySlashCommandsMixin: if not isinstance(display.get("runtime_footer"), dict): display["runtime_footer"] = {} display["runtime_footer"]["enabled"] = new_state - atomic_yaml_write(config_path, user_config) + atomic_config_write(config_path, user_config) except Exception as e: logger.warning("Failed to save runtime_footer.enabled: %s", e) return t("gateway.config_save_failed", error=e) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 08ca6cadc27..fb408026867 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -6523,6 +6523,32 @@ def require_readable_config_before_write(config_path: Optional[Path] = None) -> ) from exc +def atomic_config_write(config_path: "Path", data: Any, **kwargs: Any) -> None: + """Fail-closed atomic write for ``config.yaml``. + + The single chokepoint every config-update path should use instead of + calling :func:`utils.atomic_yaml_write` directly. It runs + :func:`require_readable_config_before_write` first, so a full-file + replacement can never silently clobber an existing ``config.yaml`` that + degraded to an empty dict on read (permission error, broken mount, + transient I/O). New-file creation still works when the path is absent. + + Root cause this guards: ``read_raw_config()`` returns ``{}`` for BOTH an + absent file and an unreadable-but-present file. Callers that read then + overwrite can't tell the two apart, so an unreadable config would be + replaced with only defaults or the single edited section. Routing every + write through this helper enforces the invariant in one place rather than + relying on each of ~15 independent write sites to remember the guard. + + ``kwargs`` are forwarded verbatim to ``atomic_yaml_write`` + (``sort_keys``, ``default_flow_style``, ``extra_content``, ...). + """ + from utils import atomic_yaml_write + + require_readable_config_before_write(config_path) + atomic_yaml_write(config_path, data, **kwargs) + + def load_config() -> Dict[str, Any]: """Load configuration from ~/.hermes/config.yaml. diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index a70fa36c90a..12b688b224c 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -983,8 +983,8 @@ def run_doctor(args): model_section[k] = raw_config.pop(k) else: raw_config.pop(k) - from utils import atomic_yaml_write - atomic_yaml_write(config_path, raw_config) + from hermes_cli.config import atomic_config_write + atomic_config_write(config_path, raw_config) check_ok("Migrated stale root-level keys into model section") fixed_count += 1 else: diff --git a/plugins/platforms/telegram/adapter.py b/plugins/platforms/telegram/adapter.py index 3fa37f94aed..c673062d7b4 100644 --- a/plugins/platforms/telegram/adapter.py +++ b/plugins/platforms/telegram/adapter.py @@ -2727,9 +2727,9 @@ class TelegramAdapter(BasePlatformAdapter): changed = True if changed: - from utils import atomic_yaml_write + from hermes_cli.config import atomic_config_write - atomic_yaml_write( + atomic_config_write( config_path, config, default_flow_style=False, diff --git a/tests/hermes_cli/test_config.py b/tests/hermes_cli/test_config.py index 60d23e2a80a..b777bf393ce 100644 --- a/tests/hermes_cli/test_config.py +++ b/tests/hermes_cli/test_config.py @@ -318,6 +318,34 @@ class TestSaveAndLoadRoundtrip: assert config_path.read_text(encoding="utf-8") == original + def test_atomic_config_write_refuses_unreadable_existing_config(self, tmp_path): + """The shared chokepoint every sibling write site routes through must + fail closed on an unreadable existing config.yaml — this locks in the + whole bug class (gateway slash commands, doctor --fix, yuanbao/telegram + auto-sethome, tui_gateway _save_cfg), not just the three named paths.""" + from hermes_cli.config import atomic_config_write + + config_path = tmp_path / "config.yaml" + original = "model:\n provider: openrouter\n" + config_path.write_text(original, encoding="utf-8") + + with patch("builtins.open", side_effect=self._deny_config_reads(config_path)): + with pytest.raises(RuntimeError, match="Refusing to overwrite"): + atomic_config_write(config_path, {"model": {"provider": "openai"}}) + + assert config_path.read_text(encoding="utf-8") == original + + def test_atomic_config_write_creates_new_file(self, tmp_path): + """A genuinely absent config.yaml must still be created — the guard + only refuses to clobber an existing-but-unreadable file.""" + from hermes_cli.config import atomic_config_write + + config_path = tmp_path / "config.yaml" + assert not config_path.exists() + atomic_config_write(config_path, {"model": {"provider": "openrouter"}}) + assert config_path.exists() + assert "openrouter" in config_path.read_text(encoding="utf-8") + def test_save_config_normalizes_legacy_root_level_max_turns(self, tmp_path): with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): save_config({"model": "test/custom-model", "max_turns": 37}) diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 8b2ab195b02..7c9287aaab0 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1805,10 +1805,10 @@ def _apply_managed(cfg: dict) -> dict: def _save_cfg(cfg: dict): global _cfg_cache, _cfg_mtime, _cfg_path - from utils import atomic_yaml_write + from hermes_cli.config import atomic_config_write path = _hermes_home / "config.yaml" - atomic_yaml_write(path, cfg) + atomic_config_write(path, cfg) with _cfg_lock: _cfg_cache = copy.deepcopy(cfg) _cfg_path = path