From a3ceee4333f004a8faebba96f0c2be2c23544b67 Mon Sep 17 00:00:00 2001 From: EmanueleCornaggia Date: Thu, 3 Sep 2026 02:59:55 +0200 Subject: [PATCH] fix(config): refuse migration on malformed YAML Make validation and migration paths distinguish parse failures from current configs so invalid YAML cannot trigger .env or config-side effects. --- hermes_cli/config.py | 23 ++++++++++++++++------- hermes_cli/update_cmd.py | 2 +- tests/hermes_cli/test_config.py | 28 +++++++++++++++++++++++++++- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index add9141382..9fe2dfb806 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2272,7 +2272,7 @@ def _raw_config_has_explicit_version() -> bool: return isinstance(raw, dict) and "_config_version" in raw -def check_config_version() -> Tuple[int, int]: +def check_config_version(*, raise_on_parse_error: bool = False) -> Tuple[int, int]: """ Check the raw on-disk config schema version. @@ -2282,7 +2282,10 @@ def check_config_version() -> Tuple[int, int]: raw ``_config_version`` must remain visible as legacy instead of inheriting the latest default version in memory. - Returns (current_version, latest_version). + Returns (current_version, latest_version). Tolerant runtime status callers + retain the historical latest/latest fallback for malformed YAML. Mutation + and explicit validation paths can set ``raise_on_parse_error`` so a parse + failure cannot be mistaken for an up-to-date config. """ latest = _coerce_config_version(DEFAULT_CONFIG.get("_config_version", 1)) or 1 config_path = get_config_path() @@ -2296,6 +2299,10 @@ def check_config_version() -> Tuple[int, int]: # Invalid YAML needs a parse warning, not an automatic schema rewrite # that could replace the user's broken file with defaults. _warn_config_parse_failure(config_path, e) + if raise_on_parse_error: + raise InvalidUserConfigError( + f"Cannot inspect {config_path}: config.yaml is not valid YAML ({e})" + ) from e return latest, latest if not isinstance(config, dict): @@ -2659,6 +2666,11 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A """ results = {"env_added": [], "config_added": [], "warnings": []} + # Validate config.yaml before any migration side effect. In particular, + # sanitize_env_file() can rewrite .env, which must not happen when the + # migration will be refused for malformed YAML. + current_ver, latest_ver = check_config_version(raise_on_parse_error=True) + # ── Always: normalize safe .env line formatting ── try: fixes = sanitize_env_file() @@ -2667,9 +2679,6 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A except Exception: pass # best-effort; don't block migration on sanitize failure - # Check config version - current_ver, latest_ver = check_config_version() - # ── Auto-migration support floor (policy: v12, July 2026) ── # A config with an EXPLICIT on-disk ``_config_version`` below the floor is # NOT auto-migrated and NOT rewritten: we surface a clear, actionable @@ -6366,7 +6375,7 @@ def config_command(args): # Check what's missing missing_env = get_missing_env_vars(required_only=False) missing_config = get_missing_config_fields() - current_ver, latest_ver = check_config_version() + current_ver, latest_ver = check_config_version(raise_on_parse_error=True) if not missing_env and not missing_config and current_ver >= latest_ver: print(color("✓ Configuration is up to date!", Colors.GREEN)) @@ -6420,7 +6429,7 @@ def config_command(args): print(color("📋 Configuration Status", Colors.CYAN, Colors.BOLD)) print() - current_ver, latest_ver = check_config_version() + current_ver, latest_ver = check_config_version(raise_on_parse_error=True) if current_ver >= latest_ver: print(f" Config version: {current_ver} ✓") else: diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 8553ba3de4..05a4e11669 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -227,7 +227,7 @@ def _run_config_check_fresh() -> tuple: _reload_config_modules() from hermes_cli.config import check_config_version - return check_config_version() + return check_config_version(raise_on_parse_error=True) def _run_migrate_config_fresh(*, interactive: bool = False, quiet: bool = False) -> dict: diff --git a/tests/hermes_cli/test_config.py b/tests/hermes_cli/test_config.py index 0db5db5226..7944a57ab0 100644 --- a/tests/hermes_cli/test_config.py +++ b/tests/hermes_cli/test_config.py @@ -10,6 +10,7 @@ import yaml from hermes_cli.config import ( DEFAULT_CONFIG, + InvalidUserConfigError, check_config_version, get_hermes_home, ensure_hermes_home, @@ -738,7 +739,9 @@ class TestConfigMigrationSecretPrompts: saved = {} monkeypatch.setattr(cfg_mod, "sanitize_env_file", lambda: 0) - monkeypatch.setattr(cfg_mod, "check_config_version", lambda: (999, 999)) + monkeypatch.setattr( + cfg_mod, "check_config_version", lambda **_kwargs: (999, 999) + ) monkeypatch.setattr(cfg_mod, "get_missing_config_fields", lambda: []) monkeypatch.setattr(cfg_mod, "get_missing_skill_config_vars", lambda: []) monkeypatch.setattr( @@ -783,6 +786,29 @@ class TestConfigVersionDetection: assert load_config()["_config_version"] == DEFAULT_CONFIG["_config_version"] assert check_config_version() == (0, DEFAULT_CONFIG["_config_version"]) + def test_strict_check_rejects_malformed_yaml(self, tmp_path): + config_path = tmp_path / "config.yaml" + config_path.write_text("model: [unterminated\n", encoding="utf-8") + + with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): + with pytest.raises(InvalidUserConfigError, match="not valid YAML"): + check_config_version(raise_on_parse_error=True) + + def test_migration_rejects_malformed_yaml_before_sanitizing_env(self, tmp_path): + config_path = tmp_path / "config.yaml" + config_bytes = b"model: [unterminated\n" + config_path.write_bytes(config_bytes) + env_path = tmp_path / ".env" + env_bytes = b"OPENAI_API_KEY=test-without-final-newline" + env_path.write_bytes(env_bytes) + + with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): + with pytest.raises(InvalidUserConfigError, match="not valid YAML"): + migrate_config(interactive=False, quiet=True) + + assert config_path.read_bytes() == config_bytes + assert env_path.read_bytes() == env_bytes + class TestConfigSupportFloor: """Auto-migration support floor (v12).