From 52480ee50f76341ecf57241edafd5bf30918f4e7 Mon Sep 17 00:00:00 2001 From: Ali jawwad <33836051+jawwad-ali@users.noreply.github.com> Date: Tue, 7 Jul 2026 03:36:35 +0500 Subject: [PATCH] fix(extensions): coerce non-mapping YAML config roots to {} in ConfigManager (#3345) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ConfigManager._load_yaml_config returned yaml.safe_load(...) or {}, which only guards falsy roots — a truthy non-mapping root (a YAML list or scalar) flows straight into _merge_configs, whose .items() raises AttributeError. get_config()/has_value()/get_value() then crash, and via should_execute_hook's blanket 'except Exception: return False' every config-based hook condition for that extension is silently disabled. Coerce a non-dict root to {}, mirroring the existing non-dict-root guard in get_project_config(). Hardens all three call sites in one place. Co-authored-by: Claude Fable 5 --- src/specify_cli/extensions/__init__.py | 7 +++- tests/test_extensions.py | 50 ++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 8dd1b1055..5eea80c77 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -2688,7 +2688,12 @@ class ConfigManager: return {} try: - return yaml.safe_load(file_path.read_text(encoding="utf-8")) or {} + data = yaml.safe_load(file_path.read_text(encoding="utf-8")) + # Coerce a non-mapping root (list/scalar, or None for an empty + # file) to {} so callers that iterate/merge the result — e.g. + # _merge_configs' .items() — never crash. Mirrors the same + # non-dict-root guard in get_project_config(). + return data if isinstance(data, dict) else {} except (yaml.YAMLError, OSError, UnicodeError): return {} diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 2a4b2aa66..2a3d02987 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -31,6 +31,7 @@ from specify_cli.extensions import ( ExtensionRegistry, ExtensionManager, CommandRegistrar, + ConfigManager, HookExecutor, ExtensionCatalog, ExtensionError, @@ -7492,3 +7493,52 @@ def test_extension_wrapper_resolves_ghes_asset_when_host_configured(tmp_path, mo ) assert resolved == "https://ghes.example/api/v3/repos/o/r/releases/assets/7" assert captured == ["https://ghes.example/api/v3/repos/o/r/releases/tags/v1"] + + +class TestConfigManagerNonMappingYaml: + """A non-mapping YAML config root must not crash config/hook resolution.""" + + def _make(self, tmp_path, body: str): + ext_dir = tmp_path / ".specify" / "extensions" / "jira" + ext_dir.mkdir(parents=True) + (ext_dir / "jira-config.yml").write_text(body, encoding="utf-8") + return ConfigManager(tmp_path, "jira") + + def test_get_config_coerces_list_root(self, tmp_path): + """A YAML list root previously raised AttributeError in _merge_configs.""" + cm = self._make(tmp_path, "- foo\n- bar\n") + assert cm.get_config() == {} + + def test_get_config_coerces_scalar_root(self, tmp_path): + cm = self._make(tmp_path, "just a string\n") + assert cm.get_config() == {} + + def test_has_value_and_get_value_do_not_raise(self, tmp_path): + cm = self._make(tmp_path, "- foo\n") + assert cm.has_value("anything") is False + assert cm.get_value("anything") is None + + def test_valid_local_config_layers_over_list_root_project_config(self, tmp_path): + """A malformed project config must not block a valid local config.""" + ext_dir = tmp_path / ".specify" / "extensions" / "jira" + ext_dir.mkdir(parents=True) + (ext_dir / "jira-config.yml").write_text("- foo\n- bar\n", encoding="utf-8") + (ext_dir / "local-config.yml").write_text( + "notifications:\n enabled: true\n", encoding="utf-8" + ) + cm = ConfigManager(tmp_path, "jira") + assert cm.get_value("notifications.enabled") is True + + def test_hook_condition_returns_false_without_raising(self, tmp_path): + """`config.x is set` on a scalar-root config must evaluate cleanly. + + Before the fix, _merge_configs raised AttributeError and the + exception was swallowed by should_execute_hook, silently disabling + every config-based hook for the extension. Assert on + _evaluate_condition directly so the crash isn't masked. + """ + ext_dir = tmp_path / ".specify" / "extensions" / "jira" + ext_dir.mkdir(parents=True) + (ext_dir / "jira-config.yml").write_text("just a string\n", encoding="utf-8") + executor = HookExecutor(tmp_path) + assert executor._evaluate_condition("config.x is set", "jira") is False