From 02b2d5a5bceba45665db860a50998f2645fa7187 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Fri, 24 Jul 2026 14:29:01 +0500 Subject: [PATCH 1/3] fix(presets): guard non-list/non-mapping provides.templates in PresetManifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PresetManifest._validate iterated provides["templates"] with no shape guards, unlike the sibling ExtensionManifest. A malformed third-party preset.yml crashed with a raw TypeError that escapes the install handler's PresetValidationError/PresetError catch and dumps an unhandled traceback: templates: 5 -> "'int' object is not iterable" templates: [null] -> "argument of type 'NoneType' is not iterable" templates: [5] -> "argument of type 'int' is not iterable" (and a string/list entry raised the misleading "Template missing 'type', 'name', or 'file'"). Add a container list-guard and a per-entry mapping-guard that raise a clean PresetValidationError, mirroring ExtensionManifest's provides.commands guards. Valid manifests (list of mappings) are unaffected. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) --- src/specify_cli/presets/__init__.py | 18 ++++++++++++++++-- tests/test_presets.py | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index e16a9d3258..3384c5a3d3 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -327,8 +327,22 @@ def _validate(self): "Preset must provide at least one template" ) - # Validate templates - for tmpl in provides["templates"]: + # Validate templates. Guard the container and each entry's shape so a + # malformed third-party preset.yml (e.g. ``templates: 5`` or + # ``templates: [null]``) raises a clean PresetValidationError the + # install handler already catches, instead of a raw TypeError + # ('int'/'NoneType' object is not iterable) that escapes to an + # unhandled traceback. Mirrors the sibling ExtensionManifest guards. + templates = provides["templates"] + if not isinstance(templates, list): + raise PresetValidationError( + "Invalid provides.templates: expected a list" + ) + for tmpl in templates: + if not isinstance(tmpl, dict): + raise PresetValidationError( + "Each template entry in 'provides.templates' must be a mapping" + ) if "type" not in tmpl or "name" not in tmpl or "file" not in tmpl: raise PresetValidationError( "Template missing 'type', 'name', or 'file'" diff --git a/tests/test_presets.py b/tests/test_presets.py index 5edd8ac712..17fef3507e 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -197,6 +197,28 @@ def test_non_mapping_yaml_raises_validation_error(self, temp_dir): with pytest.raises(PresetValidationError, match="YAML mapping"): PresetManifest(manifest_path) + def test_non_list_templates_raises_validation_error(self, temp_dir, valid_pack_data): + """A non-list provides.templates (e.g. a scalar) raises PresetValidationError, + not a raw 'int object is not iterable' TypeError — mirrors ExtensionManifest.""" + valid_pack_data["provides"]["templates"] = 5 + manifest_path = temp_dir / "preset.yml" + manifest_path.write_text(yaml.dump(valid_pack_data), encoding="utf-8") + with pytest.raises(PresetValidationError, match="templates.*expected a list"): + PresetManifest(manifest_path) + + @pytest.mark.parametrize("bad_entry", [None, 5, "oops", ["nested"]]) + def test_non_mapping_template_entry_raises_validation_error( + self, temp_dir, valid_pack_data, bad_entry + ): + """A non-mapping template entry (null/scalar/list) raises PresetValidationError, + not a raw 'argument of type ... is not iterable' TypeError from the + `"type" not in tmpl` membership test — mirrors ExtensionManifest.""" + valid_pack_data["provides"]["templates"] = [bad_entry] + manifest_path = temp_dir / "preset.yml" + manifest_path.write_text(yaml.dump(valid_pack_data), encoding="utf-8") + with pytest.raises(PresetValidationError, match="must be a mapping"): + PresetManifest(manifest_path) + def test_missing_schema_version(self, temp_dir, valid_pack_data): """Test missing schema_version field.""" del valid_pack_data["schema_version"] From 699dbf37d32c50484d6d33629d506eee3c2eff56 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Sat, 25 Jul 2026 09:19:42 +0500 Subject: [PATCH 2/3] fix(presets): check provides.templates type before emptiness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback: the new shape guard sat behind the existing truthiness check, so a FALSY non-list (templates: 0/false/null/''/{}) still reported the misleading "Preset must provide at least one template" instead of the type error. Only truthy non-lists (5, "oops", {"a": 1}) reached the guard, which is why the original test (templates: 5) passed. Split the checks: presence -> container type -> emptiness. A falsy non-list now reports "expected a list"; an EMPTY LIST keeps the "at least one template" message, since that genuinely is a well-typed container with no templates. Parametrize the non-list test over truthy AND falsy values, and add a regression guard for the empty-list message. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) --- src/specify_cli/presets/__init__.py | 12 ++++++++++- tests/test_presets.py | 32 +++++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 3384c5a3d3..527ec52a17 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -322,7 +322,7 @@ def _validate(self): # Validate provides section provides = self.data["provides"] - if "templates" not in provides or not provides["templates"]: + if "templates" not in provides: raise PresetValidationError( "Preset must provide at least one template" ) @@ -333,11 +333,21 @@ def _validate(self): # install handler already catches, instead of a raw TypeError # ('int'/'NoneType' object is not iterable) that escapes to an # unhandled traceback. Mirrors the sibling ExtensionManifest guards. + # + # Order matters: the container's TYPE is checked before its emptiness, + # so a FALSY non-list (``templates: 0``/``false``/``null``/``''``/``{}``) + # reports the accurate type error rather than the misleading "must + # provide at least one template". An empty list still reports the + # latter, since that genuinely is a list with no templates. templates = provides["templates"] if not isinstance(templates, list): raise PresetValidationError( "Invalid provides.templates: expected a list" ) + if not templates: + raise PresetValidationError( + "Preset must provide at least one template" + ) for tmpl in templates: if not isinstance(tmpl, dict): raise PresetValidationError( diff --git a/tests/test_presets.py b/tests/test_presets.py index 17fef3507e..4d56a116d7 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -197,15 +197,39 @@ def test_non_mapping_yaml_raises_validation_error(self, temp_dir): with pytest.raises(PresetValidationError, match="YAML mapping"): PresetManifest(manifest_path) - def test_non_list_templates_raises_validation_error(self, temp_dir, valid_pack_data): - """A non-list provides.templates (e.g. a scalar) raises PresetValidationError, - not a raw 'int object is not iterable' TypeError — mirrors ExtensionManifest.""" - valid_pack_data["provides"]["templates"] = 5 + @pytest.mark.parametrize( + "bad", + [ + 5, "oops", {"a": 1}, # truthy non-lists + 0, False, None, "", {}, # FALSY non-lists: must not fall through to + # the misleading "at least one template" + ], + ) + def test_non_list_templates_raises_validation_error( + self, temp_dir, valid_pack_data, bad + ): + """A non-list provides.templates raises the accurate type error, not a raw + 'int object is not iterable' TypeError and not the misleading "must provide + at least one template" (which a falsy non-list hit while the type check + sat behind the emptiness check) — mirrors ExtensionManifest.""" + valid_pack_data["provides"]["templates"] = bad manifest_path = temp_dir / "preset.yml" manifest_path.write_text(yaml.dump(valid_pack_data), encoding="utf-8") with pytest.raises(PresetValidationError, match="templates.*expected a list"): PresetManifest(manifest_path) + def test_empty_list_templates_still_reports_no_templates( + self, temp_dir, valid_pack_data + ): + """An EMPTY LIST is a well-typed container with no templates, so it keeps + the "must provide at least one template" message (regression guard for the + type-before-emptiness ordering).""" + valid_pack_data["provides"]["templates"] = [] + manifest_path = temp_dir / "preset.yml" + manifest_path.write_text(yaml.dump(valid_pack_data), encoding="utf-8") + with pytest.raises(PresetValidationError, match="at least one template"): + PresetManifest(manifest_path) + @pytest.mark.parametrize("bad_entry", [None, 5, "oops", ["nested"]]) def test_non_mapping_template_entry_raises_validation_error( self, temp_dir, valid_pack_data, bad_entry From 62facd3547ece4b56a6d3d9bf05f6912b5e2d6b2 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Tue, 28 Jul 2026 10:34:56 +0500 Subject: [PATCH 3/3] test(presets): drop the redundant empty-list templates test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback: the added test duplicated the pre-existing test_no_templates_provided -- both set provides.templates to [] and assert the same "must provide at least one template" error. That test already guards the empty-list result of the type-before-emptiness ordering, so keeping mine only added maintenance. Left a pointer comment where it was. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_presets.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/tests/test_presets.py b/tests/test_presets.py index 4d56a116d7..eb3b2fee83 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -218,17 +218,10 @@ def test_non_list_templates_raises_validation_error( with pytest.raises(PresetValidationError, match="templates.*expected a list"): PresetManifest(manifest_path) - def test_empty_list_templates_still_reports_no_templates( - self, temp_dir, valid_pack_data - ): - """An EMPTY LIST is a well-typed container with no templates, so it keeps - the "must provide at least one template" message (regression guard for the - type-before-emptiness ordering).""" - valid_pack_data["provides"]["templates"] = [] - manifest_path = temp_dir / "preset.yml" - manifest_path.write_text(yaml.dump(valid_pack_data), encoding="utf-8") - with pytest.raises(PresetValidationError, match="at least one template"): - PresetManifest(manifest_path) + # NOTE: the empty-list case (a well-typed container with no templates, which + # must keep the "must provide at least one template" message after the + # type-before-emptiness reordering) is already covered by + # test_no_templates_provided below. @pytest.mark.parametrize("bad_entry", [None, 5, "oops", ["nested"]]) def test_non_mapping_template_entry_raises_validation_error(