diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index e16a9d3258..527ec52a17 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -322,13 +322,37 @@ 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" ) - # 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. + # + # 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( + "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..eb3b2fee83 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -197,6 +197,45 @@ def test_non_mapping_yaml_raises_validation_error(self, temp_dir): with pytest.raises(PresetValidationError, match="YAML mapping"): PresetManifest(manifest_path) + @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) + + # 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( + 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"]