diff --git a/src/specify_cli/workflows/catalog.py b/src/specify_cli/workflows/catalog.py index 61f490631c..f944f95bfd 100644 --- a/src/specify_cli/workflows/catalog.py +++ b/src/specify_cli/workflows/catalog.py @@ -784,12 +784,17 @@ def remove_catalog(self, index: int) -> str: raise WorkflowValidationError("No catalog config file found.") try: - data = yaml.safe_load(config_path.read_text(encoding="utf-8")) or {} + data = yaml.safe_load(config_path.read_text(encoding="utf-8")) except (yaml.YAMLError, OSError, UnicodeDecodeError) as exc: raise WorkflowValidationError( f"Catalog config file is unreadable or malformed: {exc}" ) from exc - if not isinstance(data, dict): + # Do NOT coerce with ``or {}`` here: that also turns a FALSY non-mapping + # (top-level ``[]``, ``false``, ``0``, ``''``) into ``{}`` and silently + # swallows it, matching _load_catalog_config's guard above. + if data is None: + data = {} + elif not isinstance(data, dict): raise WorkflowValidationError( "Catalog config file is corrupted (expected a mapping)." ) @@ -1394,11 +1399,16 @@ def add_catalog(self, url: str, name: str | None = None) -> None: data: dict[str, Any] = {"catalogs": []} if config_path.exists(): try: - raw = yaml.safe_load(config_path.read_text(encoding="utf-8")) or {} + raw = yaml.safe_load(config_path.read_text(encoding="utf-8")) except (yaml.YAMLError, OSError, UnicodeDecodeError) as exc: raise StepValidationError( f"Catalog config file is unreadable or malformed: {exc}" ) from exc + # Do NOT coerce with ``or {}`` here: that also turns a FALSY + # non-mapping (top-level ``[]``, ``false``, ``0``, ``''``) into + # ``{}`` and silently swallows it. + if raw is None: + raw = {"catalogs": []} if not isinstance(raw, dict): raise StepValidationError( "Catalog config file is corrupted (expected a mapping)." @@ -1463,12 +1473,17 @@ def remove_catalog(self, index: int) -> str: raise StepValidationError("No step catalog config file found.") try: - data = yaml.safe_load(config_path.read_text(encoding="utf-8")) or {} + data = yaml.safe_load(config_path.read_text(encoding="utf-8")) except (yaml.YAMLError, OSError, UnicodeDecodeError) as exc: raise StepValidationError( f"Catalog config file is unreadable or malformed: {exc}" ) from exc - if not isinstance(data, dict): + # Do NOT coerce with ``or {}`` here: that also turns a FALSY non-mapping + # (top-level ``[]``, ``false``, ``0``, ``''``) into ``{}`` and silently + # swallows it. + if data is None: + data = {} + elif not isinstance(data, dict): raise StepValidationError( "Catalog config file is corrupted (expected a mapping)." ) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index d599f3c6a4..9e9a896438 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -8453,6 +8453,20 @@ def test_remove_catalog_malformed_yaml_raises(self, project_dir): with pytest.raises(WorkflowValidationError, match="unreadable or malformed"): catalog.remove_catalog(0) + @pytest.mark.parametrize("body", ["[]\n", "false\n", "0\n", "''\n"]) + def test_remove_catalog_rejects_falsy_non_mapping_config(self, project_dir, body): + """A FALSY non-mapping top-level config ([], false, 0, '') must raise + 'corrupted (expected a mapping)', not be silently coerced to {} by + ``or {}`` and then fail as a misleading 'out of range' error.""" + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError + + config_path = project_dir / ".specify" / "workflow-catalogs.yml" + config_path.write_text(body, encoding="utf-8") + + catalog = WorkflowCatalog(project_dir) + with pytest.raises(WorkflowValidationError, match="expected a mapping"): + catalog.remove_catalog(0) + def test_add_catalog_wraps_write_oserror(self, project_dir, monkeypatch): """An OSError on write must be wrapped as WorkflowValidationError.""" from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError @@ -9099,6 +9113,21 @@ def test_add_catalog_duplicate_rejected(self, project_dir): with pytest.raises(StepValidationError, match="already configured"): catalog.add_catalog("https://example.com/steps.json") + @pytest.mark.parametrize("body", ["[]\n", "false\n", "0\n", "''\n"]) + def test_add_catalog_rejects_falsy_non_mapping_config(self, project_dir, body): + """A FALSY non-mapping top-level config ([], false, 0, '') must raise + 'corrupted (expected a mapping)', not be silently coerced to {} by + ``or {}`` — matching the empty-document case above, which correctly + treats only a real absence of a document (None) as empty.""" + from specify_cli.workflows.catalog import StepCatalog, StepValidationError + + config_path = project_dir / ".specify" / "step-catalogs.yml" + config_path.write_text(body, encoding="utf-8") + + catalog = StepCatalog(project_dir) + with pytest.raises(StepValidationError, match="expected a mapping"): + catalog.add_catalog("https://example.com/steps.json") + def test_remove_catalog(self, project_dir): from specify_cli.workflows.catalog import StepCatalog @@ -9122,6 +9151,20 @@ def test_remove_catalog_invalid_index(self, project_dir): with pytest.raises(StepValidationError, match="out of range"): catalog.remove_catalog(5) + @pytest.mark.parametrize("body", ["[]\n", "false\n", "0\n", "''\n"]) + def test_remove_catalog_rejects_falsy_non_mapping_config(self, project_dir, body): + """A FALSY non-mapping top-level config ([], false, 0, '') must raise + 'corrupted (expected a mapping)', not be silently coerced to {} by + ``or {}`` and then fail as a misleading 'out of range' error.""" + from specify_cli.workflows.catalog import StepCatalog, StepValidationError + + config_path = project_dir / ".specify" / "step-catalogs.yml" + config_path.write_text(body, encoding="utf-8") + + catalog = StepCatalog(project_dir) + with pytest.raises(StepValidationError, match="expected a mapping"): + catalog.remove_catalog(0) + def test_remove_catalog_no_config(self, project_dir): from specify_cli.workflows.catalog import StepCatalog, StepValidationError