Skip to content

Commit 52480ee

Browse files
jawwad-aliclaude
andauthored
fix(extensions): coerce non-mapping YAML config roots to {} in ConfigManager (#3345)
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 <noreply@anthropic.com>
1 parent d3e7b06 commit 52480ee

2 files changed

Lines changed: 56 additions & 1 deletion

File tree

src/specify_cli/extensions/__init__.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2688,7 +2688,12 @@ def _load_yaml_config(self, file_path: Path) -> Dict[str, Any]:
26882688
return {}
26892689

26902690
try:
2691-
return yaml.safe_load(file_path.read_text(encoding="utf-8")) or {}
2691+
data = yaml.safe_load(file_path.read_text(encoding="utf-8"))
2692+
# Coerce a non-mapping root (list/scalar, or None for an empty
2693+
# file) to {} so callers that iterate/merge the result — e.g.
2694+
# _merge_configs' .items() — never crash. Mirrors the same
2695+
# non-dict-root guard in get_project_config().
2696+
return data if isinstance(data, dict) else {}
26922697
except (yaml.YAMLError, OSError, UnicodeError):
26932698
return {}
26942699

tests/test_extensions.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
ExtensionRegistry,
3232
ExtensionManager,
3333
CommandRegistrar,
34+
ConfigManager,
3435
HookExecutor,
3536
ExtensionCatalog,
3637
ExtensionError,
@@ -7492,3 +7493,52 @@ def fake_open(url, timeout=None, extra_headers=None):
74927493
)
74937494
assert resolved == "https://ghes.example/api/v3/repos/o/r/releases/assets/7"
74947495
assert captured == ["https://ghes.example/api/v3/repos/o/r/releases/tags/v1"]
7496+
7497+
7498+
class TestConfigManagerNonMappingYaml:
7499+
"""A non-mapping YAML config root must not crash config/hook resolution."""
7500+
7501+
def _make(self, tmp_path, body: str):
7502+
ext_dir = tmp_path / ".specify" / "extensions" / "jira"
7503+
ext_dir.mkdir(parents=True)
7504+
(ext_dir / "jira-config.yml").write_text(body, encoding="utf-8")
7505+
return ConfigManager(tmp_path, "jira")
7506+
7507+
def test_get_config_coerces_list_root(self, tmp_path):
7508+
"""A YAML list root previously raised AttributeError in _merge_configs."""
7509+
cm = self._make(tmp_path, "- foo\n- bar\n")
7510+
assert cm.get_config() == {}
7511+
7512+
def test_get_config_coerces_scalar_root(self, tmp_path):
7513+
cm = self._make(tmp_path, "just a string\n")
7514+
assert cm.get_config() == {}
7515+
7516+
def test_has_value_and_get_value_do_not_raise(self, tmp_path):
7517+
cm = self._make(tmp_path, "- foo\n")
7518+
assert cm.has_value("anything") is False
7519+
assert cm.get_value("anything") is None
7520+
7521+
def test_valid_local_config_layers_over_list_root_project_config(self, tmp_path):
7522+
"""A malformed project config must not block a valid local config."""
7523+
ext_dir = tmp_path / ".specify" / "extensions" / "jira"
7524+
ext_dir.mkdir(parents=True)
7525+
(ext_dir / "jira-config.yml").write_text("- foo\n- bar\n", encoding="utf-8")
7526+
(ext_dir / "local-config.yml").write_text(
7527+
"notifications:\n enabled: true\n", encoding="utf-8"
7528+
)
7529+
cm = ConfigManager(tmp_path, "jira")
7530+
assert cm.get_value("notifications.enabled") is True
7531+
7532+
def test_hook_condition_returns_false_without_raising(self, tmp_path):
7533+
"""`config.x is set` on a scalar-root config must evaluate cleanly.
7534+
7535+
Before the fix, _merge_configs raised AttributeError and the
7536+
exception was swallowed by should_execute_hook, silently disabling
7537+
every config-based hook for the extension. Assert on
7538+
_evaluate_condition directly so the crash isn't masked.
7539+
"""
7540+
ext_dir = tmp_path / ".specify" / "extensions" / "jira"
7541+
ext_dir.mkdir(parents=True)
7542+
(ext_dir / "jira-config.yml").write_text("just a string\n", encoding="utf-8")
7543+
executor = HookExecutor(tmp_path)
7544+
assert executor._evaluate_condition("config.x is set", "jira") is False

0 commit comments

Comments
 (0)