expand {env: foo} in any place a string can be

`keypass: {env: keypass}` has been in use in production repos for
years.  That is not anything new. It makes it possible to maintain
_config.yml_ publicly even when it needs secrets.  This change makes
sure it is possible to use {env: foo} syntax anywhere where a string
value is valid. The "list of dicts" values can be str, list of str or
list of dicts with str.

Before the {env: keypass} syntax, the actual password was just inline
in the config file.  Before this commit, it was only possible to use
{env: key} syntax in simple, string-only configs, e.g. from
examples/config.yml:

fdroidserver/common.py

@@ -642,16 +642,43 @@ def read_config():
     return config
 
 
+def expand_env_dict(s):
+    """Expand env var dict to a string value.
+
+    {env: varName} syntax can be used to replace any string value in the
+    config with the value of an environment variable "varName".  This
+    allows for secrets management when commiting the config file to a
+    public git repo.
+
+    """
+    if not s or type(s) not in (str, dict):
+        return
+    if isinstance(s, dict):
+        if 'env' not in s or len(s) > 1:
+            raise TypeError(_('Only accepts a single key "env"'))
+        var = s['env']
+        s = os.getenv(var)
+        if not s:
+            logging.error(
+                _('Environment variable {{env: {var}}} is not set!').format(var=var)
+            )
+            return
+    return os.path.expanduser(s)
+
+
 def parse_mirrors_config(mirrors):
     """Mirrors can be specified as a string, list of strings, or dictionary map."""
     if isinstance(mirrors, str):
-        return [{"url": mirrors}]
+        return [{"url": expand_env_dict(mirrors)}]
-    elif all(isinstance(item, str) for item in mirrors):
+    if isinstance(mirrors, dict):
-        return [{'url': i} for i in mirrors]
+        return [{"url": expand_env_dict(mirrors)}]
-    elif all(isinstance(item, dict) for item in mirrors):
+    if all(isinstance(item, str) for item in mirrors):
+        return [{'url': expand_env_dict(i)} for i in mirrors]
+    if all(isinstance(item, dict) for item in mirrors):
+        for item in mirrors:
+            item['url'] = expand_env_dict(item['url'])
         return mirrors
-    else:
+    raise TypeError(_('only accepts strings, lists, and tuples'))
-        raise TypeError(_('only accepts strings, lists, and tuples'))
 
 
 def get_mirrors(url, filename=None):

tests/test_common.py

@@ -2122,6 +2122,27 @@ class CommonTest(unittest.TestCase):
         )
         fdroidserver.common.read_config()
 
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
+    def test_config_with_env_string(self):
+        """Test whether env works in keys with string values."""
+        os.chdir(self.testdir)
+        testvalue = 'this is just a test'
+        Path('config.yml').write_text('keypass: {env: foo}')
+        os.environ['foo'] = testvalue
+        self.assertEqual(testvalue, fdroidserver.common.get_config()['keypass'])
+
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
+    def test_config_with_env_path(self):
+        """Test whether env works in keys with path values."""
+        os.chdir(self.testdir)
+        path = 'user@server:/path/to/bar/'
+        os.environ['foo'] = path
+        Path('config.yml').write_text('serverwebroot: {env: foo}')
+        self.assertEqual(
+            [{'url': path}],
+            fdroidserver.common.get_config()['serverwebroot'],
+        )
+
     def test_setup_status_output(self):
         os.chdir(self.tmpdir)
         start_timestamp = time.gmtime()
@@ -2847,6 +2868,41 @@ class CommonTest(unittest.TestCase):
             fdroidserver.common.read_config()['serverwebroot'],
         )
 
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
+    def test_config_serverwebroot_list_of_dicts_env(self):
+        os.chdir(self.testdir)
+        url = 'foo@example.com:/var/www/'
+        os.environ['serverwebroot'] = url
+        fdroidserver.common.write_config_file(
+            textwrap.dedent(
+                """\
+                serverwebroot:
+                  - url: {env: serverwebroot}
+                    index_only: true
+                """
+            )
+        )
+        self.assertEqual(
+            [{'url': url, 'index_only': True}],
+            fdroidserver.common.read_config()['serverwebroot'],
+        )
+
+    def test_expand_env_dict_fake_str(self):
+        testvalue = '"{env: foo}"'
+        self.assertEqual(testvalue, fdroidserver.common.expand_env_dict(testvalue))
+
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
+    def test_expand_env_dict_good(self):
+        name = 'foo'
+        value = 'bar'
+        os.environ[name] = value
+        self.assertEqual(value, fdroidserver.common.expand_env_dict({'env': name}))
+
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
+    def test_expand_env_dict_bad_dict(self):
+        with self.assertRaises(TypeError):
+            fdroidserver.common.expand_env_dict({'env': 'foo', 'foo': 'bar'})
+
     def test_parse_mirrors_config_str(self):
         s = 'foo@example.com:/var/www'
         mirrors = yaml.load("""'%s'""" % s)
@@ -2868,6 +2924,27 @@ class CommonTest(unittest.TestCase):
             [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors)
         )
 
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH'), 'foo': 'bar'}, clear=True)
+    def test_parse_mirrors_config_env_str(self):
+        mirrors = yaml.load('{env: foo}')
+        self.assertEqual(
+            [{'url': 'bar'}], fdroidserver.common.parse_mirrors_config(mirrors)
+        )
+
+    def test_parse_mirrors_config_env_list(self):
+        s = 'foo@example.com:/var/www'
+        mirrors = yaml.load("""- '%s'""" % s)
+        self.assertEqual(
+            [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors)
+        )
+
+    def test_parse_mirrors_config_env_dict(self):
+        s = 'foo@example.com:/var/www'
+        mirrors = yaml.load("""- url: '%s'""" % s)
+        self.assertEqual(
+            [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors)
+        )
+
     def test_KnownApks_recordapk(self):
         """Test that added dates are being fetched from the index.
 

tests/test_lint.py

@@ -550,7 +550,12 @@ class ConfigYmlTest(LintTest):
         self.config_yml.write_text('sdk_path: /opt/android-sdk\n')
         self.assertTrue(fdroidserver.lint.lint_config(self.config_yml))
 
-    def test_config_yml_str_dict(self):
+    def test_config_yml_str_list_of_dicts_env(self):
+        """serverwebroot can be str, list of str, or list of dicts."""
+        self.config_yml.write_text('serverwebroot: {env: ANDROID_HOME}\n')
+        self.assertTrue(fdroidserver.lint.lint_config(self.config_yml))
+
+    def test_config_yml_str_env(self):
         self.config_yml.write_text('sdk_path: {env: ANDROID_HOME}\n')
         self.assertTrue(fdroidserver.lint.lint_config(self.config_yml))