Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion src/specify_cli/extensions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
62 changes: 62 additions & 0 deletions tests/test_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 ``<id>-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."""

Expand Down