From 7ccbf6913a8632cd2e78488bb2212e3d3c44f5b2 Mon Sep 17 00:00:00 2001
From: Manfred Riem <15701806+mnriem@users.noreply.github.com>
Date: Tue, 31 Mar 2026 09:27:55 -0500
Subject: [PATCH] fix: symlink safety in uninstall/setup, handle invalid JSON
 in load

- uninstall() now uses non-resolved path for deletion so symlinks
  themselves are removed, not their targets; resolve only for
  containment validation
- setup() keeps unresolved dst_file for copy; resolves separately
  for project-root validation
- load() catches json.JSONDecodeError and re-raises as ValueError
  with the manifest path for clearer diagnostics
- Added test for invalid JSON manifest loading
---
 src/specify_cli/integrations/base.py     |  5 ++--
 src/specify_cli/integrations/manifest.py | 30 +++++++++++++++---------
 tests/test_integrations.py               |  7 ++++++
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py
index a29668daa..5242a95e5 100644
--- a/src/specify_cli/integrations/base.py
+++ b/src/specify_cli/integrations/base.py
@@ -135,8 +135,9 @@ class IntegrationBase(ABC):
 
         for src_file in sorted(tpl_dir.iterdir()):
             if src_file.is_file():
-                dst_file = (dest / src_file.name).resolve()
-                rel = dst_file.relative_to(project_root_resolved)
+                dst_file = dest / src_file.name
+                dst_resolved = dst_file.resolve()
+                rel = dst_resolved.relative_to(project_root_resolved)
                 shutil.copy2(src_file, dst_file)
                 manifest.record_existing(rel)
                 created.append(dst_file)
diff --git a/src/specify_cli/integrations/manifest.py b/src/specify_cli/integrations/manifest.py
index 1e1e205a5..32e5e5208 100644
--- a/src/specify_cli/integrations/manifest.py
+++ b/src/specify_cli/integrations/manifest.py
@@ -144,21 +144,24 @@ class IntegrationManifest:
         skipped: list[Path] = []
 
         for rel, expected_hash in self._files.items():
-            abs_path = (root / rel).resolve()
-            # Skip paths that escape the project root
+            # Use non-resolved path for deletion so symlinks themselves
+            # are removed, not their targets.
+            path = root / rel
+            # Validate containment via the resolved path
             try:
-                abs_path.relative_to(root)
-            except ValueError:
+                resolved = path.resolve()
+                resolved.relative_to(root)
+            except (ValueError, OSError):
                 continue
-            if not abs_path.exists():
+            if not path.exists():
                 continue
-            if not force and _sha256(abs_path) != expected_hash:
-                skipped.append(abs_path)
+            if not force and _sha256(path) != expected_hash:
+                skipped.append(path)
                 continue
-            abs_path.unlink()
-            removed.append(abs_path)
+            path.unlink()
+            removed.append(path)
             # Clean up empty parent directories up to project root
-            parent = abs_path.parent
+            parent = path.parent
             while parent != root:
                 try:
                     parent.rmdir()  # only succeeds if empty
@@ -204,7 +207,12 @@ class IntegrationManifest:
         """
         inst = cls(key, project_root)
         path = inst.manifest_path
-        data = json.loads(path.read_text(encoding="utf-8"))
+        try:
+            data = json.loads(path.read_text(encoding="utf-8"))
+        except json.JSONDecodeError as exc:
+            raise ValueError(
+                f"Integration manifest at {path} contains invalid JSON"
+            ) from exc
 
         if not isinstance(data, dict):
             raise ValueError(
diff --git a/tests/test_integrations.py b/tests/test_integrations.py
index e2ce2a780..d3c99b639 100644
--- a/tests/test_integrations.py
+++ b/tests/test_integrations.py
@@ -411,3 +411,10 @@ class TestManifestLoadValidation:
         path.write_text(json.dumps({"files": {"a.txt": 123}}), encoding="utf-8")
         with pytest.raises(ValueError, match="mapping"):
             IntegrationManifest.load("bad", tmp_path)
+
+    def test_load_invalid_json_raises(self, tmp_path):
+        path = tmp_path / ".specify" / "integrations" / "bad.manifest.json"
+        path.parent.mkdir(parents=True)
+        path.write_text("{not valid json", encoding="utf-8")
+        with pytest.raises(ValueError, match="invalid JSON"):
+            IntegrationManifest.load("bad", tmp_path)