From 880dec3c8ffb2f84f5b7f1984095d6ceab42b97f Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Tue, 25 Aug 2026 20:13:10 +0500 Subject: [PATCH] fix(extensions): reject non-mapping extension.yml `config` section ConfigManager._get_extension_defaults() read extension.yml's config.defaults via manifest_data.get("config", {}).get("defaults", {}) with no shape check on the intermediate "config" value. A manifest with `config: []` or `config: "oops"` (the top-level config.defaults field used by shipped extensions like extensions/git/extension.yml, distinct from the already- validated provides.config list) made the chained .get() raise a bare AttributeError instead of degrading like every other malformed config source in this class. The crash was silently swallowed by should_execute_hook's blanket except, so a hook's `config.x is set` condition permanently evaluated to False for the extension with no diagnostic. Mirrors TestConfigManagerNonMappingYaml's existing coverage for a non-mapping *root* of -config.yml, one level deeper in the manifest's own `config` section, which was previously unchecked. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01FW9fAYsCBCAgdKWovtSyqt --- src/specify_cli/extensions/__init__.py | 20 ++++++++- tests/test_extensions.py | 62 ++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 3968e4fcbe..005f78feeb 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -4502,7 +4502,25 @@ def _get_extension_defaults(self) -> Dict[str, Any]: return {} manifest_data = self._load_yaml_config(manifest_path) - return manifest_data.get("config", {}).get("defaults", {}) + # _load_yaml_config already coerces a non-mapping *root* to {}, but + # extension.yml's top-level 'config' key is unvalidated by + # ExtensionManifest (only 'provides.config' is checked there -- a + # different field). A manifest author's ``config: []`` or + # ``config: "oops"`` therefore reaches here as a dict whose 'config' + # value is a list/str, and the unguarded chained .get() raised a bare + # AttributeError ('list'/'str' object has no attribute 'get') instead + # of degrading like every other malformed-shape config source in this + # class. That crash was swallowed by should_execute_hook's blanket + # except, so a hook's 'config.x is set' condition silently and + # permanently evaluated to False for the extension -- mirroring the + # 'jira-config.yml' non-mapping-root case TestConfigManagerNonMappingYaml + # already covers for _get_project_config/_get_local_config, one level + # deeper in the manifest's own 'config' section. + config_section = manifest_data.get("config", {}) + if not isinstance(config_section, dict): + return {} + defaults = config_section.get("defaults", {}) + return defaults if isinstance(defaults, dict) else {} def _get_project_config(self) -> Dict[str, Any]: """Get project-level configuration. diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 6642da2b09..f0ef4d5e93 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -11060,6 +11060,68 @@ def test_hook_condition_returns_false_without_raising(self, tmp_path): assert executor._evaluate_condition("config.x is set", "jira") is False +class TestConfigManagerNonMappingManifestConfigSection: + """A non-mapping `config:` section in extension.yml must not crash. + + Distinct from TestConfigManagerNonMappingYaml above: that class covers a + malformed *root* of the project ``-config.yml`` file, which + ``_load_yaml_config`` already coerces to ``{}``. Here the YAML root of + ``extension.yml`` is a well-formed mapping, but its own ``config:`` key + (read by ``_get_extension_defaults`` for ``config.defaults``) is given + the wrong shape -- e.g. a list instead of a mapping. That is one level + deeper than ``_load_yaml_config``'s guard and was previously unchecked. + """ + + def _make(self, tmp_path, config_yaml_body: str): + ext_dir = tmp_path / ".specify" / "extensions" / "jira" + ext_dir.mkdir(parents=True) + (ext_dir / "extension.yml").write_text(config_yaml_body, encoding="utf-8") + return ConfigManager(tmp_path, "jira") + + def test_get_config_coerces_list_config_section(self, tmp_path): + """A list `config:` section previously raised AttributeError. + + ``manifest_data.get("config", {}).get("defaults", {})`` assumed the + 'config' value was already a mapping; a list value made the chained + `.get()` raise ``AttributeError: 'list' object has no attribute + 'get'`` instead of degrading like every other malformed config + source in this class. + """ + cm = self._make(tmp_path, "config:\n - foo\n - bar\n") + assert cm.get_config() == {} + + def test_get_config_coerces_scalar_config_section(self, tmp_path): + cm = self._make(tmp_path, "config: just-a-string\n") + assert cm.get_config() == {} + + def test_get_config_coerces_non_mapping_defaults(self, tmp_path): + """A non-mapping `config.defaults` value degrades to {} as well.""" + cm = self._make(tmp_path, "config:\n defaults:\n - foo\n") + assert cm.get_config() == {} + + def test_valid_defaults_still_load(self, tmp_path): + """The fix must not regress the well-formed shape.""" + cm = self._make( + tmp_path, + "config:\n defaults:\n feature:\n enabled: true\n", + ) + assert cm.get_value("feature.enabled") is True + + def test_hook_condition_returns_false_without_raising(self, tmp_path): + """`config.x is set` against a malformed manifest config must not raise. + + Before the fix, _get_extension_defaults 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 / "extension.yml").write_text("config:\n - foo\n", encoding="utf-8") + executor = HookExecutor(tmp_path) + assert executor._evaluate_condition("config.x is set", "jira") is False + + class TestConfigManagerEnvPrefixCollision: """Prefix-colliding env vars must not crash or clobber nested config."""