From 10d4bca64ccde571ddd7ba5166930da42265a262 Mon Sep 17 00:00:00 2001
From: Ali jawwad <33836051+jawwad-ali@users.noreply.github.com>
Date: Wed, 8 Jul 2026 01:11:18 +0500
Subject: [PATCH] fix(integrations): guard _sha256 against unreadable managed
 files (#3376)

manifest.py::_sha256 does an unguarded open(). check_modified() and
uninstall() both call it on a readable-but-unopenable regular file
(e.g. permission denied) without catching OSError, so
'specify integration upgrade/uninstall/switch' surface a raw
PermissionError traceback. Guard both call sites: in check_modified()
treat an unreadable file as modified (consistent with the adjacent
symlink / non-regular-file handling); in uninstall() treat it as skipped
and preserve it (mirroring the existing path.unlink() OSError guard just
below). The force short-circuit is unchanged.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
---
 src/specify_cli/integrations/manifest.py | 23 ++++++++++++---
 tests/integrations/test_manifest.py      | 37 ++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/src/specify_cli/integrations/manifest.py b/src/specify_cli/integrations/manifest.py
index 58719e146..8c98243c9 100644
--- a/src/specify_cli/integrations/manifest.py
+++ b/src/specify_cli/integrations/manifest.py
@@ -309,7 +309,14 @@ class IntegrationManifest:
             if abs_path.is_symlink() or not abs_path.is_file():
                 modified.append(rel)
                 continue
-            if _sha256(abs_path) != expected_hash:
+            try:
+                changed = _sha256(abs_path) != expected_hash
+            except OSError:
+                # Unreadable regular file (e.g. permission denied): treat as
+                # modified, consistent with the symlink / non-regular-file
+                # handling above, rather than letting the OSError escape.
+                changed = True
+            if changed:
                 modified.append(rel)
         return modified
 
@@ -358,9 +365,17 @@ class IntegrationManifest:
                     skipped.append(path)
                     continue
             else:
-                if not force and _sha256(path) != expected_hash:
-                    skipped.append(path)
-                    continue
+                if not force:
+                    try:
+                        matches = _sha256(path) == expected_hash
+                    except OSError:
+                        # Unreadable: can't verify it's ours, so preserve it
+                        # (mirrors the path.unlink() OSError guard below).
+                        skipped.append(path)
+                        continue
+                    if not matches:
+                        skipped.append(path)
+                        continue
             try:
                 path.unlink()
             except OSError:
diff --git a/tests/integrations/test_manifest.py b/tests/integrations/test_manifest.py
index 32ff6efbd..06c1fd398 100644
--- a/tests/integrations/test_manifest.py
+++ b/tests/integrations/test_manifest.py
@@ -481,3 +481,40 @@ class TestRecordExistingNewGuards:
         m = IntegrationManifest("test", tmp_path)
         with pytest.raises(ValueError, match=r"canonical|'\.\.' segments"):
             m.record_existing("dir/../file.txt")
+
+
+class TestManifestUnreadableFile:
+    """A managed file that is unreadable (e.g. PermissionError) must not crash
+    check_modified()/uninstall() — the CLI handlers surfaced a raw traceback."""
+
+    def _mk(self, tmp_path):
+        m = IntegrationManifest("test", tmp_path)
+        m.record_file("sub/f.md", "content")
+        return m
+
+    def test_check_modified_treats_unreadable_as_modified(self, tmp_path, monkeypatch):
+        m = self._mk(tmp_path)
+
+        def raise_perm(_path):
+            raise PermissionError("unreadable")
+
+        monkeypatch.setattr(
+            "specify_cli.integrations.manifest._sha256", raise_perm
+        )
+        # Before the fix this raised PermissionError.
+        assert m.check_modified() == ["sub/f.md"]
+
+    def test_uninstall_preserves_unreadable_file(self, tmp_path, monkeypatch):
+        m = self._mk(tmp_path)
+
+        def raise_perm(_path):
+            raise PermissionError("unreadable")
+
+        monkeypatch.setattr(
+            "specify_cli.integrations.manifest._sha256", raise_perm
+        )
+        removed, skipped = m.uninstall(force=False)
+        # Can't verify ownership => preserve, don't crash and don't delete.
+        assert removed == []
+        assert (tmp_path / "sub" / "f.md") in skipped
+        assert (tmp_path / "sub" / "f.md").exists()