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
This commit is contained in:
Manfred Riem
2026-03-31 09:27:55 -05:00
parent 868bfd06c4
commit 7ccbf6913a
3 changed files with 29 additions and 13 deletions

View File

@@ -135,8 +135,9 @@ class IntegrationBase(ABC):
for src_file in sorted(tpl_dir.iterdir()): for src_file in sorted(tpl_dir.iterdir()):
if src_file.is_file(): if src_file.is_file():
dst_file = (dest / src_file.name).resolve() dst_file = dest / src_file.name
rel = dst_file.relative_to(project_root_resolved) dst_resolved = dst_file.resolve()
rel = dst_resolved.relative_to(project_root_resolved)
shutil.copy2(src_file, dst_file) shutil.copy2(src_file, dst_file)
manifest.record_existing(rel) manifest.record_existing(rel)
created.append(dst_file) created.append(dst_file)

View File

@@ -144,21 +144,24 @@ class IntegrationManifest:
skipped: list[Path] = [] skipped: list[Path] = []
for rel, expected_hash in self._files.items(): for rel, expected_hash in self._files.items():
abs_path = (root / rel).resolve() # Use non-resolved path for deletion so symlinks themselves
# Skip paths that escape the project root # are removed, not their targets.
path = root / rel
# Validate containment via the resolved path
try: try:
abs_path.relative_to(root) resolved = path.resolve()
except ValueError: resolved.relative_to(root)
except (ValueError, OSError):
continue continue
if not abs_path.exists(): if not path.exists():
continue continue
if not force and _sha256(abs_path) != expected_hash: if not force and _sha256(path) != expected_hash:
skipped.append(abs_path) skipped.append(path)
continue continue
abs_path.unlink() path.unlink()
removed.append(abs_path) removed.append(path)
# Clean up empty parent directories up to project root # Clean up empty parent directories up to project root
parent = abs_path.parent parent = path.parent
while parent != root: while parent != root:
try: try:
parent.rmdir() # only succeeds if empty parent.rmdir() # only succeeds if empty
@@ -204,7 +207,12 @@ class IntegrationManifest:
""" """
inst = cls(key, project_root) inst = cls(key, project_root)
path = inst.manifest_path path = inst.manifest_path
try:
data = json.loads(path.read_text(encoding="utf-8")) 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): if not isinstance(data, dict):
raise ValueError( raise ValueError(

View File

@@ -411,3 +411,10 @@ class TestManifestLoadValidation:
path.write_text(json.dumps({"files": {"a.txt": 123}}), encoding="utf-8") path.write_text(json.dumps({"files": {"a.txt": 123}}), encoding="utf-8")
with pytest.raises(ValueError, match="mapping"): with pytest.raises(ValueError, match="mapping"):
IntegrationManifest.load("bad", tmp_path) 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)