Skip to content

Commit 880dec3

Browse files
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 <id>-config.yml, one level deeper in the manifest's own `config` section, which was previously unchecked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FW9fAYsCBCAgdKWovtSyqt
1 parent c6b0d3f commit 880dec3

2 files changed

Lines changed: 81 additions & 1 deletion

File tree

‎src/specify_cli/extensions/__init__.py‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4502,7 +4502,25 @@ def _get_extension_defaults(self) -> Dict[str, Any]:
45024502
return {}
45034503

45044504
manifest_data = self._load_yaml_config(manifest_path)
4505-
return manifest_data.get("config", {}).get("defaults", {})
4505+
# _load_yaml_config already coerces a non-mapping *root* to {}, but
4506+
# extension.yml's top-level 'config' key is unvalidated by
4507+
# ExtensionManifest (only 'provides.config' is checked there -- a
4508+
# different field). A manifest author's ``config: []`` or
4509+
# ``config: "oops"`` therefore reaches here as a dict whose 'config'
4510+
# value is a list/str, and the unguarded chained .get() raised a bare
4511+
# AttributeError ('list'/'str' object has no attribute 'get') instead
4512+
# of degrading like every other malformed-shape config source in this
4513+
# class. That crash was swallowed by should_execute_hook's blanket
4514+
# except, so a hook's 'config.x is set' condition silently and
4515+
# permanently evaluated to False for the extension -- mirroring the
4516+
# 'jira-config.yml' non-mapping-root case TestConfigManagerNonMappingYaml
4517+
# already covers for _get_project_config/_get_local_config, one level
4518+
# deeper in the manifest's own 'config' section.
4519+
config_section = manifest_data.get("config", {})
4520+
if not isinstance(config_section, dict):
4521+
return {}
4522+
defaults = config_section.get("defaults", {})
4523+
return defaults if isinstance(defaults, dict) else {}
45064524

45074525
def _get_project_config(self) -> Dict[str, Any]:
45084526
"""Get project-level configuration.

‎tests/test_extensions.py‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11060,6 +11060,68 @@ def test_hook_condition_returns_false_without_raising(self, tmp_path):
1106011060
assert executor._evaluate_condition("config.x is set", "jira") is False
1106111061

1106211062

11063+
class TestConfigManagerNonMappingManifestConfigSection:
11064+
"""A non-mapping `config:` section in extension.yml must not crash.
11065+
11066+
Distinct from TestConfigManagerNonMappingYaml above: that class covers a
11067+
malformed *root* of the project ``<id>-config.yml`` file, which
11068+
``_load_yaml_config`` already coerces to ``{}``. Here the YAML root of
11069+
``extension.yml`` is a well-formed mapping, but its own ``config:`` key
11070+
(read by ``_get_extension_defaults`` for ``config.defaults``) is given
11071+
the wrong shape -- e.g. a list instead of a mapping. That is one level
11072+
deeper than ``_load_yaml_config``'s guard and was previously unchecked.
11073+
"""
11074+
11075+
def _make(self, tmp_path, config_yaml_body: str):
11076+
ext_dir = tmp_path / ".specify" / "extensions" / "jira"
11077+
ext_dir.mkdir(parents=True)
11078+
(ext_dir / "extension.yml").write_text(config_yaml_body, encoding="utf-8")
11079+
return ConfigManager(tmp_path, "jira")
11080+
11081+
def test_get_config_coerces_list_config_section(self, tmp_path):
11082+
"""A list `config:` section previously raised AttributeError.
11083+
11084+
``manifest_data.get("config", {}).get("defaults", {})`` assumed the
11085+
'config' value was already a mapping; a list value made the chained
11086+
`.get()` raise ``AttributeError: 'list' object has no attribute
11087+
'get'`` instead of degrading like every other malformed config
11088+
source in this class.
11089+
"""
11090+
cm = self._make(tmp_path, "config:\n - foo\n - bar\n")
11091+
assert cm.get_config() == {}
11092+
11093+
def test_get_config_coerces_scalar_config_section(self, tmp_path):
11094+
cm = self._make(tmp_path, "config: just-a-string\n")
11095+
assert cm.get_config() == {}
11096+
11097+
def test_get_config_coerces_non_mapping_defaults(self, tmp_path):
11098+
"""A non-mapping `config.defaults` value degrades to {} as well."""
11099+
cm = self._make(tmp_path, "config:\n defaults:\n - foo\n")
11100+
assert cm.get_config() == {}
11101+
11102+
def test_valid_defaults_still_load(self, tmp_path):
11103+
"""The fix must not regress the well-formed shape."""
11104+
cm = self._make(
11105+
tmp_path,
11106+
"config:\n defaults:\n feature:\n enabled: true\n",
11107+
)
11108+
assert cm.get_value("feature.enabled") is True
11109+
11110+
def test_hook_condition_returns_false_without_raising(self, tmp_path):
11111+
"""`config.x is set` against a malformed manifest config must not raise.
11112+
11113+
Before the fix, _get_extension_defaults raised AttributeError and the
11114+
exception was swallowed by should_execute_hook, silently disabling
11115+
every config-based hook for the extension. Assert on
11116+
_evaluate_condition directly so the crash isn't masked.
11117+
"""
11118+
ext_dir = tmp_path / ".specify" / "extensions" / "jira"
11119+
ext_dir.mkdir(parents=True)
11120+
(ext_dir / "extension.yml").write_text("config:\n - foo\n", encoding="utf-8")
11121+
executor = HookExecutor(tmp_path)
11122+
assert executor._evaluate_condition("config.x is set", "jira") is False
11123+
11124+
1106311125
class TestConfigManagerEnvPrefixCollision:
1106411126
"""Prefix-colliding env vars must not crash or clobber nested config."""
1106511127

0 commit comments

Comments
 (0)