fix: don't leak or mask errors when rewrite cleanup fails on Windows

CHANGELOG.md

@@ -11,6 +11,7 @@ project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 - Fix a package build deprecation warning caused by a non-string `license` value in `pyproject.toml` by [@kurtmckee] in [#648]
 - `set_key`, `unset_key` and the `dotenv set`/`unset` commands now name the `.env` path instead of an internal temporary file when its directory is missing or not writable, and the CLI prints a short error and exits with code 2 instead of a traceback by [@jamalkamaladdin] in [#711]
+- `set_key` and `unset_key` no longer leave a `.tmp_*` file behind on Windows when writing a read-only `.env` fails, and the error raised is the one from the failed write rather than from cleaning up the temporary file by [@MohammedAlkindi] in [#686]
 
 ## [1.2.4] - 2026-10-01
 
@@ -458,6 +459,7 @@ os.PathLike]` instead of just `os.PathLike` (#347 by [@bbc2]).
 [#648]: https://github.com/theskumar/python-dotenv/pull/648
 [#663]: https://github.com/theskumar/python-dotenv/pull/663
 [#680]: https://github.com/theskumar/python-dotenv/pull/680
+[#686]: https://github.com/theskumar/python-dotenv/pull/686
 [#698]: https://github.com/theskumar/python-dotenv/pull/698
 [#700]: https://github.com/theskumar/python-dotenv/pull/700
 [#711]: https://github.com/theskumar/python-dotenv/pull/711
@@ -468,6 +470,7 @@ os.PathLike]` instead of just `os.PathLike` (#347 by [@bbc2]).
 [@23f3001135]: https://github.com/23f3001135
 [@EpicWink]: https://github.com/EpicWink
 [@Flimm]: https://github.com/Flimm
+[@MohammedAlkindi]: https://github.com/MohammedAlkindi
 [@Nicals]: https://github.com/Nicals
 [@Nougat-Waffle]: https://github.com/Nougat-Waffle
 [@Qwerty-133]: https://github.com/Qwerty-133

src/dotenv/main.py

@@ -135,6 +135,25 @@ def get_key(
     return DotEnv(dotenv_path, verbose=True, encoding=encoding).get(key_to_get)
 
 
+def _discard_temp_file(path: pathlib.Path) -> None:
+    """
+    Delete `rewrite`'s temporary file, ignoring any failure to do so.
+
+    This runs while another exception is propagating, so it must not raise:
+    that error is the one worth reporting. On Windows, a file whose mode has
+    no owner-write bit carries the read-only attribute and can't be unlinked,
+    so the mode is reset before a second attempt.
+    """
+    try:
+        path.unlink(missing_ok=True)
+    except OSError:
+        try:
+            path.chmod(stat.S_IWRITE | stat.S_IREAD)
+            path.unlink(missing_ok=True)
+        except OSError:
+            logger.warning("python-dotenv could not remove the temporary file %s", path)
+
+
 @contextmanager
 def rewrite(
     path: StrPath,
@@ -191,10 +210,10 @@ def rewrite(
 
             os.replace(dest_path, path)
         except BaseException:
-            dest_path.unlink(missing_ok=True)
+            _discard_temp_file(dest_path)
             raise
     else:
-        dest_path.unlink(missing_ok=True)
+        _discard_temp_file(dest_path)
         raise error from None
 
 

tests/test_main.py

@@ -1,6 +1,7 @@
 import io
 import logging
 import os
+import pathlib
 import stat
 import subprocess
 import sys
@@ -195,6 +196,48 @@ def test_set_key_permission_error(dotenv_path):
     assert dotenv_path.read_text() == ""
 
 
+@pytest.mark.skipif(
+    sys.platform != "win32" and os.geteuid() == 0,
+    reason="Root user can access files even with 000 permissions.",
+)
+def test_set_key_permission_error_leaves_no_temp_file(dotenv_path):
+    if sys.platform == "win32":
+        # On Windows, make file read-only
+        dotenv_path.chmod(stat.S_IREAD)
+    else:
+        # On Unix, remove all permissions
+        dotenv_path.chmod(0o000)
+
+    try:
+        with pytest.raises(PermissionError):
+            dotenv.set_key(dotenv_path, "a", "b")
+
+        assert list(dotenv_path.parent.glob(".tmp_*")) == []
+    finally:
+        # Restore permissions
+        if sys.platform == "win32":
+            dotenv_path.chmod(stat.S_IWRITE | stat.S_IREAD)
+        else:
+            dotenv_path.chmod(0o600)
+
+
+def test_rewrite_reports_original_error_when_cleanup_fails(dotenv_path, caplog):
+    replace_error = OSError("replace failed")
+
+    with mock.patch("dotenv.main.os.replace", side_effect=replace_error):
+        with mock.patch.object(
+            pathlib.Path, "unlink", side_effect=OSError("unlink failed")
+        ):
+            with pytest.raises(OSError) as excinfo:
+                dotenv.set_key(dotenv_path, "a", "b")
+
+    assert excinfo.value is replace_error
+    [temp_file] = dotenv_path.parent.glob(".tmp_*")
+    assert caplog.messages == [
+        f"python-dotenv could not remove the temporary file {temp_file}"
+    ]
+
+
 def test_set_key_missing_directory(tmp_path):
     dotenv_path = tmp_path / "nx_dir" / ".env"
 
@@ -227,6 +270,49 @@ def test_set_key_read_only_directory(tmp_path):
     assert list(directory.iterdir()) == [dotenv_path]
 
 
+def windows_read_only_semantics(path):
+    # On Windows, a file without the owner-write bit can't be replaced or deleted.
+    return path.exists() and not path.stat().st_mode & stat.S_IWUSR
+
+
+@pytest.mark.parametrize(
+    "rewrite",
+    [
+        lambda path: dotenv.set_key(path, "a", "y"),
+        lambda path: dotenv.unset_key(path, "a"),
+    ],
+    ids=["set_key", "unset_key"],
+)
+def test_rewrite_read_only_file_leaves_no_temp_file(tmp_path, rewrite):
+    dotenv_path = tmp_path / ".env"
+    dotenv_path.write_text("a=x\n")
+    dotenv_path.chmod(stat.S_IREAD)
+    real_replace = os.replace
+    real_unlink = pathlib.Path.unlink
+
+    def replace(src, dst):
+        if windows_read_only_semantics(pathlib.Path(dst)):
+            raise PermissionError(
+                13, "Access is denied", os.fspath(src), None, os.fspath(dst)
+            )
+        real_replace(src, dst)
+
+    def unlink(self, missing_ok=False):
+        if windows_read_only_semantics(self):
+            raise PermissionError(13, "Access is denied", str(self))
+        real_unlink(self, missing_ok=missing_ok)
+
+    with mock.patch("dotenv.main.os.replace", side_effect=replace):
+        with mock.patch.object(pathlib.Path, "unlink", unlink):
+            with pytest.raises(PermissionError) as exc_info:
+                rewrite(dotenv_path)
+
+    dotenv_path.chmod(stat.S_IREAD | stat.S_IWRITE)
+    assert exc_info.value.filename2 == str(dotenv_path)
+    assert dotenv_path.read_text() == "a=x\n"
+    assert list(tmp_path.iterdir()) == [dotenv_path]
+
+
 def test_get_key_no_file(tmp_path):
     nx_path = tmp_path / "nx"
     logger = logging.getLogger("dotenv.main")