Improve quoting of values in set_key

The value of `quote_mode` must now be one of `auto`, `never` or
`always`, to ensure that users aren't accidentally relying on any other
value for their scripts to work.

Surrounding quotes are no longer stripped.  This makes it possible for
the user to control exactly what goes in the .env file.  Note that when
doing `dotenv set foo 'bar'` in Bash, the shell will have already
removed the quotes.

Single quotes are used instead of double quotes.  This avoids
accidentally having values interpreted by the parser or Bash (e.g. if
you set a password with `dotenv set password 'af$rb0'`.

Previously, the `auto` mode of quoting had the same effect as `always`.
This commit restores the functionality of `auto` by not quoting
alphanumeric values (which don't need quotes).  Plenty of other kinds of
values also don't need quotes but it's hard to know which ones without
actually parsing them, so we just omit quotes for alphanumeric values,
at least for now.

Author: bbc2

CHANGELOG.md

@@ -5,6 +5,19 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this
 project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Changed
+
+- Raise `ValueError` if `quote_mode` isn't one of `always`, `auto` or `never` in
+  `set_key` (#330 by [@bbc2]).
+- When writing a value to a .env file with `set_key` or `dotenv set <key> <value>` (#330
+  by [@bbc2]):
+  - Use single quotes instead of double quotes.
+  - Don't strip surrounding quotes.
+  - In `auto` mode, don't add quotes if the value is only made of alphanumeric characters
+    (as determined by `string.isalnum`).
+
 ## [0.17.1] - 2021-04-29
 
 ### Fixed

src/dotenv/main.py

@@ -151,13 +151,16 @@ def set_key(dotenv_path, key_to_set, value_to_set, quote_mode="always", export=F
     If the .env path given doesn't exist, fails instead of risking creating
     an orphan .env somewhere in the filesystem
     """
-    value_to_set = value_to_set.strip("'").strip('"')
+    if quote_mode not in ("always", "auto", "never"):
+        raise ValueError("Unknown quote_mode: {}".format(quote_mode))
 
-    if " " in value_to_set:
-        quote_mode = "always"
+    quote = (
+        quote_mode == "always"
+        or (quote_mode == "auto" and not value_to_set.isalnum())
+    )
 
-    if quote_mode == "always":
-        value_out = '"{}"'.format(value_to_set.replace('"', '\\"'))
+    if quote:
+        value_out = "'{}'".format(value_to_set.replace("'", "\\'"))
     else:
         value_out = value_to_set
     if export:

tests/test_cli.py

@@ -73,10 +73,11 @@ def test_unset_non_existent_value(cli, dotenv_file):
 @pytest.mark.parametrize(
     "quote_mode,variable,value,expected",
     (
-        ("always", "HELLO", "WORLD", 'HELLO="WORLD"\n'),
-        ("never", "HELLO", "WORLD", 'HELLO=WORLD\n'),
-        ("auto", "HELLO", "WORLD", 'HELLO=WORLD\n'),
-        ("auto", "HELLO", "HELLO WORLD", 'HELLO="HELLO WORLD"\n'),
+        ("always", "a", "x", "a='x'\n"),
+        ("never", "a", "x", 'a=x\n'),
+        ("auto", "a", "x", "a=x\n"),
+        ("auto", "a", "x y", "a='x y'\n"),
+        ("auto", "a", "$", "a='$'\n"),
     )
 )
 def test_set_quote_options(cli, dotenv_file, quote_mode, variable, value, expected):
@@ -92,8 +93,8 @@ def test_set_quote_options(cli, dotenv_file, quote_mode, variable, value, expect
 @pytest.mark.parametrize(
     "dotenv_file,export_mode,variable,value,expected",
     (
-        (".nx_file", "true", "HELLO", "WORLD", "export HELLO=\"WORLD\"\n"),
-        (".nx_file", "false", "HELLO", "WORLD", "HELLO=\"WORLD\"\n"),
+        (".nx_file", "true", "a", "x", "export a='x'\n"),
+        (".nx_file", "false", "a", "x", "a='x'\n"),
     )
 )
 def test_set_export(cli, dotenv_file, export_mode, variable, value, expected):

tests/test_main.py

@@ -28,18 +28,18 @@ def test_set_key_no_file(tmp_path):
 @pytest.mark.parametrize(
     "before,key,value,expected,after",
     [
-        ("", "a", "", (True, "a", ""), 'a=""\n'),
-        ("", "a", "b", (True, "a", "b"), 'a="b"\n'),
-        ("", "a", "'b'", (True, "a", "b"), 'a="b"\n'),
-        ("", "a", "\"b\"", (True, "a", "b"), 'a="b"\n'),
-        ("", "a", "b'c", (True, "a", "b'c"), 'a="b\'c"\n'),
-        ("", "a", "b\"c", (True, "a", "b\"c"), 'a="b\\\"c"\n'),
-        ("a=b", "a", "c", (True, "a", "c"), 'a="c"\n'),
-        ("a=b\n", "a", "c", (True, "a", "c"), 'a="c"\n'),
-        ("a=b\n\n", "a", "c", (True, "a", "c"), 'a="c"\n\n'),
-        ("a=b\nc=d", "a", "e", (True, "a", "e"), 'a="e"\nc=d'),
-        ("a=b\nc=d\ne=f", "c", "g", (True, "c", "g"), 'a=b\nc="g"\ne=f'),
-        ("a=b\n", "c", "d", (True, "c", "d"), 'a=b\nc="d"\n'),
+        ("", "a", "", (True, "a", ""), "a=''\n"),
+        ("", "a", "b", (True, "a", "b"), "a='b'\n"),
+        ("", "a", "'b'", (True, "a", "'b'"), "a='\\'b\\''\n"),
+        ("", "a", "\"b\"", (True, "a", '"b"'), "a='\"b\"'\n"),
+        ("", "a", "b'c", (True, "a", "b'c"), "a='b\\'c'\n"),
+        ("", "a", "b\"c", (True, "a", "b\"c"), "a='b\"c'\n"),
+        ("a=b", "a", "c", (True, "a", "c"), "a='c'\n"),
+        ("a=b\n", "a", "c", (True, "a", "c"), "a='c'\n"),
+        ("a=b\n\n", "a", "c", (True, "a", "c"), "a='c'\n\n"),
+        ("a=b\nc=d", "a", "e", (True, "a", "e"), "a='e'\nc=d"),
+        ("a=b\nc=d\ne=f", "c", "g", (True, "c", "g"), "a=b\nc='g'\ne=f"),
+        ("a=b\n", "c", "d", (True, "c", "d"), "a=b\nc='d'\n"),
     ],
 )
 def test_set_key(dotenv_file, before, key, value, expected, after):