Skip to content

fix: do not read through a symlink when rewriting a .env file - #690

Closed
Gares95 wants to merge 1 commit into
theskumar:mainfrom
Gares95:fix/do-not-read-through-symlink
Closed

fix: do not read through a symlink when rewriting a .env file#690
Gares95 wants to merge 1 commit into
theskumar:mainfrom
Gares95:fix/do-not-read-through-symlink

Conversation

@Gares95

@Gares95 Gares95 commented Aug 23, 2026

Copy link
Copy Markdown

Description

rewrite() no longer follows symlinks when it replaces a .env file, but it
still opens the path for reading with open(), which does follow them. When the
path is a symlink, the target's contents are read and carried into the regular
file that replaces the link:

>>> import os, tempfile, dotenv
>>> d = tempfile.mkdtemp()
>>> open(f"{d}/target.env", "w").write("SECRET=classified\n")
>>> os.symlink(f"{d}/target.env", f"{d}/.env")
>>> dotenv.set_key(f"{d}/.env", "a", "y")
>>> print(open(f"{d}/.env").read())
SECRET=classified
a='y'

unset_key behaves the same way.

To be clear about what this is not: the target file is never modified, so this
is not the file-overwrite problem fixed in 1.2.2, and the replacement file is
created with mode 0600, so it is not readable by another user. It is a
narrower mismatch between what the rewrite says it does and what it does. The
1.2.2 change described itself as "set_key and unset_key used to follow
symlinks in some situations. This is no longer the case", and os.replace()
replaces the link rather than its target, so the target's contents are not the
contents of the file being written.

The fix

Open the source with O_NOFOLLOW unless follow_symlinks=True, and treat the
resulting error the way a missing file is already treated: a symlink that is not
being followed has no existing content to carry over into its replacement.

O_NOFOLLOW is POSIX only. Where it is unavailable the flag is a no-op and
behaviour is unchanged, which is why the platform errno set is built defensively
rather than assumed.

This is a behaviour change in the same sense the 1.2.2 notes used: a .env that
is a symlink to a file with other keys will now be replaced by a file containing
only what was written, rather than the target's keys plus what was written.

Why the existing tests did not catch it

test_set_key_symlink_to_existing_file and
test_unset_key_symlink_to_existing_file both used a target whose only key was
the key being set or unset, so anything read through the link was overwritten by
the operation and never visible. The set_key test also asserted with in
rather than ==, which tolerated extra content.

Both now put a different key in the target and assert exact contents, so they
fail against the previous behaviour and pass with this change.

Validation

pytest: 252 passed, 1 skipped. ruff check src tests and
ruff format --check src tests clean. mypy src tests reports no issues on
each of the versions the lint environment checks, 3.10 through 3.14.

`rewrite()` no longer follows symlinks when replacing a `.env` file, but it
still opened the path for reading with `open()`, which does follow them. When
the path was a symlink, the target's contents were read and carried into the
regular file that replaced the link, so keys from the target appeared in the
new `.env`.

The target itself was never modified, so this is not the file-overwrite issue
addressed previously. It is a narrower mismatch between what the rewrite says
it does and what it does: `os.replace()` replaces the link rather than its
target, so the target's contents are not the contents of the file being
written.

Open the source with `O_NOFOLLOW` unless `follow_symlinks=True`, and treat the
resulting error the same way a missing file is treated, since a symlink we are
not following has no existing content to carry over. `O_NOFOLLOW` is POSIX
only; where it is unavailable the flag is a no-op and behaviour is unchanged.

The existing symlink tests could not catch this. Both used a target whose only
key was the key being set or unset, so anything read through the link was
overwritten by the operation, and the `set_key` test asserted with `in` rather
than `==`. Both now use a different key in the target and assert exact
contents.
@Gares95 Gares95 closed this Aug 23, 2026
@Gares95

Gares95 commented Aug 23, 2026

Copy link
Copy Markdown
Author

Withdrawing this. On further testing the change loses data, so it should not be
merged as written.

set_key reads the existing file, edits one key and writes the result back.
This patch opens with O_NOFOLLOW and treats the resulting ELOOP the same way
a missing file is treated, as an empty source. When .env is a symlink, which
is an ordinary deployment layout, the rewrite therefore emits only the new key
and every pre-existing key is dropped:

before:  DB_HOST=prod
         API_KEY=secret
         DEBUG=false

after `set_key(".env", "NEW_KEY", "v")` on a symlinked .env:
         NEW_KEY='v'

The behaviour I was trying to tighten is minor next to that, so the trade is not
worth making.

Worth flagging for anyone who picks this up later: the existing symlink test did
not catch it, and I had adjusted that test while developing the patch, which is
what let it through a fully green matrix. A variant that refuses to write through
a symlink would need to raise rather than silently fall back to an empty source,
and that is a behaviour change worth deciding on deliberately rather than folding
into a fix like this one.

Sorry for the noise, and thanks for the CI time.

@Gares95
Gares95 deleted the fix/do-not-read-through-symlink branch August 23, 2026 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant