diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 676f1193d7..c9e05e3e88 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -12,6 +12,7 @@ This module provides: - hermes config wizard - Re-run setup wizard """ +import copy import os import platform import re @@ -26,6 +27,7 @@ from typing import Dict, Any, Optional, List, Tuple _IS_WINDOWS = platform.system() == "Windows" _ENV_VAR_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") +_LAST_EXPANDED_CONFIG_BY_PATH: Dict[str, Any] = {} # Env var names written to .env that aren't in OPTIONAL_ENV_VARS # (managed by setup/provider flows directly). _EXTRA_ENV_KEYS = frozenset({ @@ -2635,6 +2637,85 @@ def _expand_env_vars(obj): return obj +def _items_by_unique_name(items): + """Return a name-indexed dict only when all items have unique string names.""" + if not isinstance(items, list): + return None + indexed = {} + for item in items: + if not isinstance(item, dict) or not isinstance(item.get("name"), str): + return None + name = item["name"] + if name in indexed: + return None + indexed[name] = item + return indexed + + +def _preserve_env_ref_templates(current, raw, loaded_expanded=None): + """Restore raw ``${VAR}`` templates when a value is otherwise unchanged. + + ``load_config()`` expands env refs for runtime use. When a caller later + persists that config after modifying some unrelated setting, keep the + original on-disk template instead of writing the expanded plaintext + secret back to ``config.yaml``. + + Prefer preserving the raw template when ``current`` still matches either + the value previously returned by ``load_config()`` for this config path or + the current environment expansion of ``raw``. This handles env-var + rotation between load and save while still treating mixed literal/template + string edits as caller-owned once their rendered value diverges. + """ + if isinstance(current, str) and isinstance(raw, str) and re.search(r"\${[^}]+}", raw): + if current == raw: + return raw + if isinstance(loaded_expanded, str) and current == loaded_expanded: + return raw + if _expand_env_vars(raw) == current: + return raw + return current + + if isinstance(current, dict) and isinstance(raw, dict): + return { + key: _preserve_env_ref_templates( + value, + raw.get(key), + loaded_expanded.get(key) if isinstance(loaded_expanded, dict) else None, + ) + for key, value in current.items() + } + + if isinstance(current, list) and isinstance(raw, list): + # Prefer matching named config objects (e.g. custom_providers) by name + # so harmless reordering doesn't drop the original template. If names + # are duplicated, fall back to positional matching instead of silently + # shadowing one entry. + current_by_name = _items_by_unique_name(current) + raw_by_name = _items_by_unique_name(raw) + loaded_by_name = _items_by_unique_name(loaded_expanded) + if current_by_name is not None and raw_by_name is not None: + return [ + _preserve_env_ref_templates( + item, + raw_by_name.get(item.get("name")), + loaded_by_name.get(item.get("name")) if loaded_by_name is not None else None, + ) + for item in current + ] + return [ + _preserve_env_ref_templates( + item, + raw[index] if index < len(raw) else None, + loaded_expanded[index] + if isinstance(loaded_expanded, list) and index < len(loaded_expanded) + else None, + ) + for index, item in enumerate(current) + ] + + return current + + def _normalize_root_model_keys(config: Dict[str, Any]) -> Dict[str, Any]: """Move stale root-level provider/base_url into model section. @@ -2702,7 +2783,6 @@ def read_raw_config() -> Dict[str, Any]: def load_config() -> Dict[str, Any]: """Load configuration from ~/.hermes/config.yaml.""" - import copy ensure_hermes_home() config_path = get_config_path() @@ -2723,8 +2803,11 @@ def load_config() -> Dict[str, Any]: config = _deep_merge(config, user_config) except Exception as e: print(f"Warning: Failed to load config: {e}") - - return _expand_env_vars(_normalize_root_model_keys(_normalize_max_turns_config(config))) + + normalized = _normalize_root_model_keys(_normalize_max_turns_config(config)) + expanded = _expand_env_vars(normalized) + _LAST_EXPANDED_CONFIG_BY_PATH[str(config_path)] = copy.deepcopy(expanded) + return expanded _SECURITY_COMMENT = """ @@ -2833,7 +2916,15 @@ def save_config(config: Dict[str, Any]): ensure_hermes_home() config_path = get_config_path() - normalized = _normalize_root_model_keys(_normalize_max_turns_config(config)) + current_normalized = _normalize_root_model_keys(_normalize_max_turns_config(config)) + normalized = current_normalized + raw_existing = _normalize_root_model_keys(_normalize_max_turns_config(read_raw_config())) + if raw_existing: + normalized = _preserve_env_ref_templates( + normalized, + raw_existing, + _LAST_EXPANDED_CONFIG_BY_PATH.get(str(config_path)), + ) # Build optional commented-out sections for features that are off by # default or only relevant when explicitly configured. @@ -2851,6 +2942,7 @@ def save_config(config: Dict[str, Any]): extra_content="".join(parts) if parts else None, ) _secure_file(config_path) + _LAST_EXPANDED_CONFIG_BY_PATH[str(config_path)] = copy.deepcopy(current_normalized) def load_env() -> Dict[str, str]: diff --git a/tests/cli/test_cli_save_config_value.py b/tests/cli/test_cli_save_config_value.py index e481194146..5933038648 100644 --- a/tests/cli/test_cli_save_config_value.py +++ b/tests/cli/test_cli_save_config_value.py @@ -64,6 +64,24 @@ class TestSaveConfigValueAtomic: result = yaml.safe_load(config_env.read_text()) assert result["display"]["skin"] == "ares" + def test_preserves_env_ref_templates_in_unrelated_fields(self, config_env): + """The /model --global persistence path must not inline env-backed secrets.""" + config_env.write_text(yaml.dump({ + "custom_providers": [{ + "name": "tuzi", + "api_key": "${TU_ZI_API_KEY}", + "model": "claude-opus-4-6", + }], + "model": {"default": "test-model", "provider": "openrouter"}, + })) + + from cli import save_config_value + save_config_value("model.default", "doubao-pro") + + result = yaml.safe_load(config_env.read_text()) + assert result["model"]["default"] == "doubao-pro" + assert result["custom_providers"][0]["api_key"] == "${TU_ZI_API_KEY}" + def test_file_not_truncated_on_error(self, config_env, monkeypatch): """If atomic_yaml_write raises, the original file is untouched.""" original_content = config_env.read_text() diff --git a/tests/hermes_cli/test_config_env_refs.py b/tests/hermes_cli/test_config_env_refs.py new file mode 100644 index 0000000000..854668a2b7 --- /dev/null +++ b/tests/hermes_cli/test_config_env_refs.py @@ -0,0 +1,169 @@ +import textwrap + +from hermes_cli.config import load_config, save_config + + +def _write_config(tmp_path, body: str): + (tmp_path / "config.yaml").write_text(textwrap.dedent(body), encoding="utf-8") + + +def _read_config(tmp_path) -> str: + return (tmp_path / "config.yaml").read_text(encoding="utf-8") + + +def test_save_config_preserves_env_refs_on_unrelated_change(monkeypatch, tmp_path): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("TU_ZI_API_KEY", "sk-realsecret") + monkeypatch.setenv("ALT_SECRET", "alt-secret") + _write_config( + tmp_path, + """\ + custom_providers: + - name: tuzi + base_url: https://api.tu-zi.com + api_key: ${TU_ZI_API_KEY} + headers: + Authorization: Bearer ${ALT_SECRET} + model: claude-opus-4-6 + model: + default: claude-opus-4-6 + """, + ) + + config = load_config() + config["model"]["default"] = "doubao-pro" + save_config(config) + + saved = _read_config(tmp_path) + assert "api_key: ${TU_ZI_API_KEY}" in saved + assert "Authorization: Bearer ${ALT_SECRET}" in saved + assert "sk-realsecret" not in saved + assert "alt-secret" not in saved + + +def test_save_config_preserves_unresolved_env_refs(monkeypatch, tmp_path): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.delenv("MISSING_SECRET", raising=False) + _write_config( + tmp_path, + """\ + custom_providers: + - name: unresolved + api_key: ${MISSING_SECRET} + model: claude-opus-4-6 + model: + default: claude-opus-4-6 + """, + ) + + config = load_config() + config["display"]["compact"] = True + save_config(config) + + assert "api_key: ${MISSING_SECRET}" in _read_config(tmp_path) + + +def test_save_config_allows_intentional_secret_value_change(monkeypatch, tmp_path): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("TU_ZI_API_KEY", "sk-old-secret") + _write_config( + tmp_path, + """\ + custom_providers: + - name: tuzi + api_key: ${TU_ZI_API_KEY} + model: claude-opus-4-6 + model: + default: claude-opus-4-6 + """, + ) + + config = load_config() + config["custom_providers"][0]["api_key"] = "sk-new-secret" + save_config(config) + + saved = _read_config(tmp_path) + assert "api_key: sk-new-secret" in saved + assert "${TU_ZI_API_KEY}" not in saved + + +def test_save_config_preserves_template_when_env_rotates_after_load(monkeypatch, tmp_path): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("TU_ZI_API_KEY", "sk-old-secret") + _write_config( + tmp_path, + """\ + custom_providers: + - name: tuzi + api_key: ${TU_ZI_API_KEY} + model: claude-opus-4-6 + model: + default: claude-opus-4-6 + """, + ) + + config = load_config() + monkeypatch.setenv("TU_ZI_API_KEY", "sk-rotated-secret") + config["model"]["default"] = "doubao-pro" + save_config(config) + + saved = _read_config(tmp_path) + assert "api_key: ${TU_ZI_API_KEY}" in saved + assert "sk-old-secret" not in saved + assert "sk-rotated-secret" not in saved + + +def test_save_config_keeps_edited_partial_template_strings_literal(monkeypatch, tmp_path): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("ALT_SECRET", "alt-secret") + _write_config( + tmp_path, + """\ + custom_providers: + - name: tuzi + headers: + Authorization: Bearer ${ALT_SECRET} + model: claude-opus-4-6 + model: + default: claude-opus-4-6 + """, + ) + + config = load_config() + config["custom_providers"][0]["headers"]["Authorization"] = "Token alt-secret" + save_config(config) + + saved = _read_config(tmp_path) + assert "Authorization: Token alt-secret" in saved + assert "Authorization: Bearer ${ALT_SECRET}" not in saved + + +def test_save_config_falls_back_to_positional_matching_for_duplicate_names(monkeypatch, tmp_path): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("FIRST_SECRET", "first-secret") + monkeypatch.setenv("SECOND_SECRET", "second-secret") + _write_config( + tmp_path, + """\ + custom_providers: + - name: duplicate + api_key: ${FIRST_SECRET} + model: claude-opus-4-6 + - name: duplicate + api_key: ${SECOND_SECRET} + model: doubao-pro + model: + default: claude-opus-4-6 + """, + ) + + config = load_config() + config["display"]["compact"] = True + save_config(config) + + saved = _read_config(tmp_path) + assert saved.count("name: duplicate") == 2 + assert "api_key: ${FIRST_SECRET}" in saved + assert "api_key: ${SECOND_SECRET}" in saved + assert "first-secret" not in saved + assert "second-secret" not in saved