Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions src/dotenv/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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


Expand Down
39 changes: 39 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import io
import logging
import os
import pathlib
import stat
import subprocess
import sys
Expand Down Expand Up @@ -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")
Expand Down