fix(extensions): reject non-mapping extension.yml config section - #4323
Open
Noor-ul-ain001 wants to merge 1 commit into
Open
fix(extensions): reject non-mapping extension.yml config section#4323Noor-ul-ain001 wants to merge 1 commit into
Noor-ul-ain001 wants to merge 1 commit into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ConfigManager._get_extension_defaults()read an extension'sextension.ymlconfig.defaultsviamanifest_data.get("config", {}).get("defaults", {})with no shape check on the intermediateconfigvalue. Shipped extensions (e.g.extensions/git/extension.yml) use a top-levelconfig: { defaults: {...} }section — distinct from the already-validatedprovides.configlist. A manifest withconfig: []orconfig: "oops"made the chained.get()raise a bareAttributeErrorinstead of degrading gracefully like every other malformed config source in this class.should_execute_hook's blanketexcept Exception, so a hook'sconfig.x is setcondition permanently evaluated toFalsefor the extension with no diagnostic — a hook that should run (or should raise a clear error) instead silently never fires.TestConfigManagerNonMappingYamlcoverage for a non-mapping root of<id>-config.yml(already fixed), one level deeper in the manifest's ownconfigsection, which was previously unchecked.ExtensionManifest._validate()does not validate this top-levelconfigfield at all.Test plan
TestConfigManagerNonMappingManifestConfigSectiontotests/test_extensions.py(5 tests: listconfig:, scalarconfig:, non-mappingconfig.defaults, valid-shape regression guard, and aHookExecutor._evaluate_conditionnon-crash check)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 passtests/test_extensions.pyfull suite — 505 passed, 20 pre-existing failures (Windows symlink-elevation and env-specific; reproduced identically on the unmodified branch), 8 skipped — no regressionsCo-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01FW9fAYsCBCAgdKWovtSyqt