diff --git a/knoe/ui/screens/cfg.py b/knoe/ui/screens/cfg.py index f8589cd..b62c6d9 100644 --- a/knoe/ui/screens/cfg.py +++ b/knoe/ui/screens/cfg.py @@ -34,6 +34,22 @@ from knoe.core.env import ( ) +def _validate_cfg_values(name: str, mapping: dict) -> None: + """Raise TypeError if any value in *mapping* is not a plain string. + + Prevents MagicMock (or other non-string) reprs from being silently + coerced and written into conf/.cfg by _save_knoe_cfg. + """ + for k, v in mapping.items(): + if not isinstance(v, str): + raise TypeError( + f"cfg {name!r}: field {k!r} has non-string value " + f"{type(v).__name__} ({v!r}). Refusing to serialize. " + "This usually means a Tk widget var wasn't properly initialized " + "(headless env, partial mocking, etc.)." + ) + + class ConfigMixin: """knoe.cfg persistence, input snapshot collection and port-forward management.""" @@ -189,6 +205,8 @@ class ConfigMixin: if explicit_service_namespace: explicit_global_keys.add("SERVICE_NAMESPACE") + _validate_cfg_values("globals", globals_to_save) + globals_to_save = _filter_cfg_values_for_persistence( "Global", globals_to_save, @@ -278,6 +296,8 @@ class ConfigMixin: cfg_path.write_text(cfg_text) print(f"[DEBUG] knoe.cfg saved to {cfg_path}") + except (TypeError, ValueError): + raise except Exception as e: print(f"[ERROR] Failed to save knoe.cfg: {e}") diff --git a/tests/installer/test_cfg_save_refuses_mock_values.py b/tests/installer/test_cfg_save_refuses_mock_values.py new file mode 100644 index 0000000..ec35a2e --- /dev/null +++ b/tests/installer/test_cfg_save_refuses_mock_values.py @@ -0,0 +1,177 @@ +from __future__ import annotations + +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from knoe.ui.screens.cfg import ConfigMixin + + +class _DummyCfgAppWithMockVar(ConfigMixin): + """Worst-case input: every widget var is a bare MagicMock. + + This is what happens when an interactive `./install.py -c conf/k3d.cfg` + run launches the TUI in a context where Tk variables aren't real + StringVars (headless env, partial mock setup, etc.). + """ + + def __init__(self, conf_dir: Path): + self._conf_dir = conf_dir + self._cfg_path_override = conf_dir / "knoe.cfg" + self._mode = "k3d" + self._resolved_service_namespace = "knoe-system" + # Every var is a MagicMock — `var.get()` returns MagicMock, + # `.strip()` returns MagicMock, `str(...)` returns a repr. + for name in ( + "cluster_env", + "selected_kubectx", + "db_username", + "db_password", + "db_namespace", + "cnpg_cluster_name", + "db_host_port", + "supabase_pv_node", + "supabase_pv_base_dir", + "gitops_node_selector", + "argocd_node_selector", + "k3s_server_url", + "k3s_token", + "prod_artifacts_path", + ): + setattr(self, name, MagicMock()) + + self.knoe_cfg_data = { + "Global": {}, + "Initialize Cluster": {}, + "Dev Cluster (k3d)": {}, + "Service Cluster (k3s)": {}, + "Prod Cluster (k8s)": {}, + } + + def _deployment_mode(self) -> str: + return self._mode + + def _get_service_namespace(self) -> str: + return self._resolved_service_namespace + + def _secret_cfg_value(self, *_args, **_kwargs): + return "" + + def _save_ansible_knoe_vault(self, *_args, **_kwargs): + return None + + def _sanitize_sections_for_cfg(self, sections: dict) -> dict: + return sections + + def _sync_port_forward_mappings(self): + return None + + def _collect_input_snapshot(self) -> dict: + return {} + + +def test_save_knoe_cfg_refuses_non_string_widget_values(tmp_path, monkeypatch): + """The save path must not serialize MagicMock (or any non-str) values + into the cfg file. Either raise a clear TypeError, or skip the + offending field. Silent str()-coercion-to-repr is forbidden. + """ + import knoe.ui.screens.cfg as cfg_mod + + monkeypatch.setattr(cfg_mod.knoe_conf, "activate_environment", lambda *_a, **_k: None) + + app = _DummyCfgAppWithMockVar(tmp_path) + cfg_path = tmp_path / "knoe.cfg" + + with pytest.raises((TypeError, ValueError)) as excinfo: + app._save_knoe_cfg() + + # The error must name the offending field/var so the engineer can + # diagnose without grepping mock-reprs out of files. + assert "MagicMock" in str(excinfo.value) or "non-string" in str(excinfo.value).lower() + + # The cfg file must NOT contain any MagicMock string-reprs even if + # save partially succeeded before raising. + if cfg_path.exists(): + text = cfg_path.read_text() + assert " str: + return self._value + + +class _DummyCfgApp(ConfigMixin): + """Real-string variant — save must succeed and produce a clean cfg.""" + + def __init__(self, conf_dir: Path): + self._conf_dir = conf_dir + self._cfg_path_override = conf_dir / "knoe.cfg" + self._mode = "k3d" + self._resolved_service_namespace = "knoe-system" + + self.cluster_env = _Var("dev") + self.selected_kubectx = _Var("k3d-knoe-system") + self.db_username = _Var("knoe-db") + self.db_password = _Var("secret") + self.db_namespace = _Var("knoe-system") + self.cnpg_cluster_name = _Var("knoe-db") + self.db_host_port = _Var("5432") + self.supabase_pv_node = _Var("") + self.supabase_pv_base_dir = _Var("") + self.gitops_node_selector = _Var("") + self.argocd_node_selector = _Var("") + self.k3s_server_url = _Var("") + self.k3s_token = _Var("") + self.prod_artifacts_path = _Var(str(conf_dir)) + self.service_namespace = _Var("knoe-system") + + self.knoe_cfg_data = { + "Global": {}, + "Initialize Cluster": {}, + "Dev Cluster (k3d)": {}, + "Service Cluster (k3s)": {}, + "Prod Cluster (k8s)": {}, + } + + def _deployment_mode(self) -> str: + return self._mode + + def _get_service_namespace(self) -> str: + return self._resolved_service_namespace + + def _secret_cfg_value(self, _section, _key, value, *_args) -> str: + return value + + def _save_ansible_knoe_vault(self, *_args, **_kwargs): + return None + + def _sanitize_sections_for_cfg(self, sections: dict) -> dict: + return sections + + def _sync_port_forward_mappings(self): + return None + + def _collect_input_snapshot(self) -> dict: + return {} + + +def test_save_knoe_cfg_real_strings_produce_clean_cfg(tmp_path, monkeypatch): + """When all widget vars return real strings, save must succeed without + any MagicMock reprs in the output file.""" + import knoe.ui.screens.cfg as cfg_mod + + monkeypatch.setattr(cfg_mod.knoe_conf, "activate_environment", lambda *_a, **_k: None) + + app = _DummyCfgApp(tmp_path) + app._save_knoe_cfg() + + cfg_path = tmp_path / "knoe.cfg" + assert cfg_path.exists(), "cfg file was not written" + text = cfg_path.read_text() + assert "