fix(paths): clear read-only bit before unlinking the atomic-write temp on Windows (#2622)

graphify/paths.py

@@ -76,7 +76,15 @@ def _atomic_replace(path: "str | Path", write_fn) -> None:
         try:
             os.unlink(tmp)
         except OSError:
-            pass
+            # The temp was chmod'd to match the destination above, so when the
+            # destination is read-only the temp is too — and Windows refuses to
+            # unlink a read-only file. Clear the bit and retry, or every failed
+            # write leaks a `.graph.json.*.tmp` into the output directory.
+            try:
+                os.chmod(tmp, stat.S_IWRITE)
+                os.unlink(tmp)
+            except OSError:
+                pass
         raise
 
 

tests/test_atomic_writes.py

@@ -36,6 +36,12 @@ def test_write_text_atomic_preserves_existing_on_failure(tmp_path, monkeypatch):
     assert sorted(x.name for x in tmp_path.iterdir()) == ["graph.json"]
 
 
+@pytest.mark.skipif(
+    os.name == "nt",
+    reason="Windows has no POSIX mode bits: chmod only toggles the read-only "
+           "attribute, and st_mode reports 0o666 for any writable file, so "
+           "chmod(0o644) followed by an equality check can never hold",
+)
 def test_write_text_atomic_preserves_existing_mode(tmp_path):
     # An atomic replace must not tighten a 0644 file to mkstemp's 0600 default.
     p = tmp_path / "graph.json"
@@ -45,6 +51,44 @@ def test_write_text_atomic_preserves_existing_mode(tmp_path):
     assert (os.stat(p).st_mode & 0o777) == 0o644
 
 
+@pytest.mark.skipif(os.name != "nt", reason="Windows read-only attribute semantics")
+def test_write_text_atomic_refuses_a_readonly_destination_without_leaking_a_temp(
+    tmp_path,
+):
+    """The Windows analogue of the mode-preservation contract.
+
+    There is no POSIX mode to preserve here, so the property worth pinning is
+    the one Windows actually has: a read-only destination is NOT silently
+    overwritten, the original survives intact, and — the part that used to be
+    wrong — no temp file is left behind.
+
+    `_atomic_replace` chmods the temp to match the destination, so against a
+    read-only target the temp is read-only too; Windows then refuses to unlink
+    it and the cleanup swallowed the error, dropping a `.graph.json.*.tmp` into
+    the output directory on every failed write.
+
+    Note this diverges from POSIX, where `os.replace` needs only directory write
+    permission and so happily replaces a read-only file. Documented rather than
+    "fixed": refusing to overwrite a file the user marked read-only is the
+    defensible behaviour.
+    """
+    import stat as _stat
+
+    p = tmp_path / "graph.json"
+    p.write_text("original", encoding="utf-8")
+    os.chmod(p, _stat.S_IREAD)
+    try:
+        with pytest.raises(PermissionError):
+            write_text_atomic(p, "replaced")
+
+        assert p.read_text(encoding="utf-8") == "original", "read-only file was clobbered"
+        assert [x.name for x in tmp_path.iterdir()] == ["graph.json"], (
+            f"failed write leaked a temp file: {[x.name for x in tmp_path.iterdir()]}"
+        )
+    finally:
+        os.chmod(p, _stat.S_IWRITE)  # let tmp_path cleanup remove it
+
+
 def test_write_text_atomic_new_file_respects_umask(tmp_path):
     # A brand-new file must land at the umask default (e.g. 0644), NOT mkstemp's
     # 0600 — otherwise every fresh graph.json would be owner-only.