From fb162d516f87c914dadee92613a2624a143f8819 Mon Sep 17 00:00:00 2001
From: Mohammed Alkindi <alkndymhmd692@gmail.com>
Date: Wed, 19 Aug 2026 00:13:49 +0400
Subject: [PATCH] fix: don't leak or mask errors when rewrite cleanup fails on
 Windows

When set_key or unset_key fails after rewrite() has restored the original
file mode, the temporary file has already been chmod'ed to that mode. On
Windows a mode without the owner-write bit sets the read-only attribute,
so the os.unlink in the cleanup path fails too. The temporary file is left
next to the .env file and the cleanup error replaces the real one in the
traceback.

Reset the mode and retry before giving up, and never let a cleanup failure
propagate in place of the error that caused the rewrite to be abandoned.
---
 src/dotenv/main.py | 23 +++++++++++++++++++++--
 tests/test_main.py | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/src/dotenv/main.py b/src/dotenv/main.py
index 3123690a..f60d4d3b 100644
--- a/src/dotenv/main.py
+++ b/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,
@@ -183,10 +202,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
 
 
diff --git a/tests/test_main.py b/tests/test_main.py
index 6f9d4c5c..ce83a657 100644
--- a/tests/test_main.py
+++ b/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,44 @@ 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):
+    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
+
+
 def test_get_key_no_file(tmp_path):
     nx_path = tmp_path / "nx"
     logger = logging.getLogger("dotenv.main")