Skip to content

Reject a key that set_key cannot write faithfully - #678

Open
lenamonj wants to merge 1 commit into
theskumar:mainfrom
lenamonj:fix/set-key-validates-key
Open

Reject a key that set_key cannot write faithfully#678
lenamonj wants to merge 1 commit into
theskumar:mainfrom
lenamonj:fix/set-key-validates-key

Conversation

@lenamonj

Copy link
Copy Markdown

set_key validates quote_mode and quotes the value it writes, but nothing validates the key. The key is interpolated straight into the line, so a key the parser reads back differently is silently stored as a different variable, while the call returns (True, key, value) to report success.

>>> from dotenv import set_key, dotenv_values
>>> set_key(".env", "a=b", "x")
(True, 'a=b', 'x')
>>> open(".env").read()
"a=b='x'\n"
>>> dotenv_values(".env")
{'a': "b='x'"}

The caller asked for key a=b and got key a holding b='x'.

Other shapes are lost outright rather than altered. "a b", " a", "a\tb" and "" write lines the parser rejects, so the key silently does not exist afterwards; "a\nb" writes a second line, so a spurious extra key appears. Each returns True.

All of it is reachable from the CLI:

$ dotenv -f .env set "a=b" x
a=b=x
$ echo $?
0
$ dotenv -f .env list
a=b='x'

The change

Validate by round trip rather than by restating the parser's rules in a second place: write a probe line with a trivial value and check that the parser hands the same key back. If the parser and set_key ever disagree about what a key is, this check follows the parser automatically.

Keys that work today are unaffected. The added test pins that explicitly for a, A_B1, a.b, MY-KEY, a'b, a$b and a non-ASCII key.

ValueError matches the existing behaviour for a bad quote_mode a few lines above.

Scope is deliberately the key only. The value side of quote_mode="never" is #218, which you closed after #330, and this change does not touch it.

Verification

Against main at 751f8c14:

  • The two added tests, applied alone to unmodified main: 8 failed, 7 passed. The 8 failures are exactly the 8 rejection cases; the 7 passes are the control keys, which already round-trip correctly.
  • With the change: 235 passed, 1 skipped (the suite was 220 passed, 1 skipped before the 15 added cases).
  • ruff check ., ruff format --check src tests and mypy src all clean.

set_key validates quote_mode and the value it writes, but never the key.
The key is interpolated straight into the line, so a key the parser reads
back differently is stored as a different variable while the call returns
(True, key, value):

    set_key(p, "a=b", "x")   writes  a=b='x'
                              reads back as {"a": "b='x'"}

Other shapes are lost outright rather than altered: "a b", " a", "a\tb"
and "" write lines the parser rejects, and "a\nb" writes a second line so
a spurious extra key appears. All are reachable from the CLI, where
dotenv set "a=b" x exits 0 and echoes the key it did not write.

Validate by round trip rather than by restating the parser's rules: write
a probe line with a trivial value and check the parser hands the same key
back. Keys that already work are unaffected, including "a.b", "MY-KEY",
"a" and non-ASCII keys.
@theskumar theskumar added the inprogress Being actively reviewed/worked on label Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inprogress Being actively reviewed/worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants