From 611451d3b438b159a60c343043da896aa910c3e5 Mon Sep 17 00:00:00 2001
From: kartsan03 <kartsan03@gmail.com>
Date: Tue, 20 Oct 2026 13:21:02 +0000
Subject: [PATCH] fix(extensions): reject malformed catalog entries before
 removal

`specify extension catalog remove` filtered out every non-mapping entry
in `.specify/extension-catalogs.yml` while removing the named catalog.
Removing a name that isn't there then reported success and rewrote the
file without the malformed entry, and removing the last valid catalog
deleted the file with the user's other keys. Validate the entries first
and leave the file untouched, as #4992 does for preset catalogs.

Assisted-by: Codex CLI (model: gpt-6.1-sol, autonomous)
Assisted-by: Claude Code (model: claude-opus-5-5, autonomous)
---
 .../extensions/catalog/command_remove.py      |  9 ++-
 .../extensions/catalog/test_command_remove.py | 70 +++++++++++++++++++
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/src/specify_cli/extensions/catalog/command_remove.py b/src/specify_cli/extensions/catalog/command_remove.py
index 23eb6d1214..80cc8cb33b 100644
--- a/src/specify_cli/extensions/catalog/command_remove.py
+++ b/src/specify_cli/extensions/catalog/command_remove.py
@@ -38,12 +38,19 @@ def catalog_remove(
             "[red]Error:[/red] Invalid catalog config: 'catalogs' must be a list."
         )
         raise typer.Exit(1)
+    for index, entry in enumerate(catalogs):
+        if not isinstance(entry, dict):
+            _commands.console.print(
+                f"[red]Error:[/red] Invalid catalog entry at index {index}: "
+                f"expected a mapping, got {type(entry).__name__}"
+            )
+            raise typer.Exit(1)
     safe_name = _escape_markup(name)
     original_count = len(catalogs)
     catalogs = [
         catalog
         for catalog in catalogs
-        if isinstance(catalog, dict) and catalog.get("name") != name
+        if catalog.get("name") != name
     ]
 
     if len(catalogs) == original_count:
diff --git a/tests/specify_cli/extensions/catalog/test_command_remove.py b/tests/specify_cli/extensions/catalog/test_command_remove.py
index 2892b09d20..d0c5f9e9ad 100644
--- a/tests/specify_cli/extensions/catalog/test_command_remove.py
+++ b/tests/specify_cli/extensions/catalog/test_command_remove.py
@@ -8,11 +8,47 @@
 import os
 from pathlib import Path
 
+import pytest
+import typer
 import yaml
 
 from tests.integrations.test_cli import _normalize_cli_output
 
 
+class TestExtensionCatalogRemoveValidation:
+    """Reject malformed entries before changing the user's catalog config."""
+
+    @pytest.mark.parametrize("invalid_entry", ["unfinished-entry", 0, False, None, []])
+    @pytest.mark.parametrize("invalid_index", [0, 1])
+    @pytest.mark.parametrize("target", ["keep", "does-not-exist"])
+    def test_malformed_entries_preserve_config(
+        self, tmp_path, monkeypatch, capsys, invalid_entry, invalid_index, target
+    ):
+        from specify_cli.extensions import _commands
+        from specify_cli.extensions.catalog.command_remove import catalog_remove
+
+        (tmp_path / ".specify").mkdir()
+        config_path = tmp_path / ".specify" / "extension-catalogs.yml"
+        entries = [{"name": "keep", "url": "https://example.com/team.json"}]
+        entries.insert(invalid_index, invalid_entry)
+        original = (
+            "# Local settings: café\n"
+            + yaml.safe_dump({"catalogs": entries, "metadata": "keep me"})
+        ).replace("\n", "\r\n").encode("utf-8")
+        config_path.write_bytes(original)
+        monkeypatch.setattr(_commands, "_require_specify_project", lambda: tmp_path)
+
+        with pytest.raises(typer.Exit) as exc_info:
+            catalog_remove(target)
+
+        assert exc_info.value.exit_code == 1
+        assert config_path.read_bytes() == original
+        output = _normalize_cli_output(capsys.readouterr().out)
+        assert f"Invalid catalog entry at index {invalid_index}" in output
+        assert f"expected a mapping, got {type(invalid_entry).__name__}" in output
+        assert "Removed catalog" not in output
+
+
 class TestExtensionCatalogRemoveCLI:
     """Integration coverage for the extension catalog command."""
 
@@ -34,6 +70,40 @@ def _invoke(self, argv, cwd):
         finally:
             os.chdir(old)
 
+    @pytest.mark.parametrize("target", ["keep", "does-not-exist"])
+    def test_remove_malformed_config_in_initialized_project(
+        self, tmp_path, target
+    ):
+        """A real initialized project keeps all config bytes on rejection."""
+        project = tmp_path / "initialized"
+        project.mkdir()
+        initialized = self._invoke(
+            [
+                "init", "--here", "--integration", "codex",
+                "--ignore-agent-tools", "--non-interactive", "--script", "py",
+            ],
+            project,
+        )
+        assert initialized.exit_code == 0, initialized.output
+        config_path = project / ".specify" / "extension-catalogs.yml"
+        original = (
+            "# Local settings: café\r\n"
+            "catalogs:\r\n"
+            "  - name: keep\r\n"
+            "    url: https://example.com/team.json\r\n"
+            "  - https://example.com/private.json\r\n"
+            "metadata: keep me\r\n"
+        ).encode("utf-8")
+        config_path.write_bytes(original)
+
+        result = self._invoke(["extension", "catalog", "remove", target], project)
+
+        assert result.exit_code == 1, result.output
+        assert config_path.read_bytes() == original
+        output = _normalize_cli_output(result.output)
+        assert "Invalid catalog entry at index 1: expected a mapping, got str" in output
+        assert "Removed catalog" not in output
+
     def test_extension_catalog_remove_rejects_non_mapping_config_root(self, tmp_path):
         project = self._make_project(tmp_path)
         cfg_path = project / ".specify" / "extension-catalogs.yml"