Skip to content

fix(extensions): reject non-mapping extension.yml config section - #4323

Open
Noor-ul-ain001 wants to merge 1 commit into
github:mainfrom
Noor-ul-ain001:fix/extension-config-defaults-non-mapping
Open

fix(extensions): reject non-mapping extension.yml config section#4323
Noor-ul-ain001 wants to merge 1 commit into
github:mainfrom
Noor-ul-ain001:fix/extension-config-defaults-non-mapping

Conversation

@Noor-ul-ain001

Copy link
Copy Markdown
Contributor

Summary

  • ConfigManager._get_extension_defaults() read an extension's extension.yml config.defaults via manifest_data.get("config", {}).get("defaults", {}) with no shape check on the intermediate config value. Shipped extensions (e.g. extensions/git/extension.yml) use a top-level config: { defaults: {...} } section — distinct from the already-validated provides.config list. A manifest with config: [] or config: "oops" made the chained .get() raise a bare AttributeError instead of degrading gracefully like every other malformed config source in this class.
  • That crash was silently swallowed by should_execute_hook's blanket except Exception, so a hook's config.x is set condition permanently evaluated to False for the extension with no diagnostic — a hook that should run (or should raise a clear error) instead silently never fires.
  • Mirrors the existing TestConfigManagerNonMappingYaml coverage for a non-mapping root of <id>-config.yml (already fixed), one level deeper in the manifest's own config section, which was previously unchecked. ExtensionManifest._validate() does not validate this top-level config field at all.

Test plan

  • Added TestConfigManagerNonMappingManifestConfigSection to tests/test_extensions.py (5 tests: list config:, scalar config:, non-mapping config.defaults, valid-shape regression guard, and a HookExecutor._evaluate_condition non-crash check)
  • Verified all 4 crash-reproducing tests fail without the fix (AttributeError: 'list'/'str' object has no attribute 'get') and pass with it — stashed only the source change (src/specify_cli/extensions/__init__.py), confirmed the failure, then restored it and confirmed the pass
  • Ran tests/test_extensions.py full suite — 505 passed, 20 pre-existing failures (Windows symlink-elevation and env-specific; reproduced identically on the unmodified branch), 8 skipped — no regressions

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01FW9fAYsCBCAgdKWovtSyqt

@Noor-ul-ain001
Noor-ul-ain001 requested a review from mnriem as a code owner August 25, 2026 15:12
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant