chore(config): allow simple commands without external script

klorenz · nejch · commit 91ffb8e97e21 · 2021-04-18T11:59:45.000+02:00
diff --git a/docs/cli-usage.rst b/docs/cli-usage.rst
@@ -93,6 +93,8 @@ Only one of ``private_token``, ``oauth_token`` or ``job_token`` should be
 defined. If neither are defined an anonymous request will be sent to the Gitlab
 server, with very limited permissions.
 
+We recommend that you use `Credential helpers`_ to securely store your tokens.
+
 .. list-table:: GitLab server options
    :header-rows: 1
 
@@ -119,22 +121,50 @@ server, with very limited permissions.
    * - ``http_password``
      - Password for optional HTTP authentication
 
-For all settings, which contain secrets (``http_password``, 
+
+Credential helpers
+------------------
+
+For all configuration options that contain secrets (``http_password``,
 ``personal_token``, ``oauth_token``, ``job_token``), you can specify
-a helper program to retrieve the secret indicated by ``helper:`` 
-prefix.  You can only specify a path to a program without any 
-parameters.  You may use ``~`` for expanding your homedir in helper
-program's path.  It is expected, that the program prints the secret 
-to standard output.
+a helper program to retrieve the secret indicated by a ``helper:``
+prefix. This allows you to fetch values from a local keyring store
+or cloud-hosted vaults such as Bitwarden. Environment variables are
+expanded if they exist and ``~`` expands to your home directory.
+
+It is expected that the helper program prints the secret to standard output.
+To use shell features such as piping to retrieve the value, you will need
+to use a wrapper script; see below.
 
 Example for a `keyring <https://github.com/jaraco/keyring>`_ helper:
 
-.. code-block:: bash
+.. code-block:: ini
 
-    #!/bin/bash
-    keyring get Service Username
+   [global]
+   default = somewhere
+   ssl_verify = true
+   timeout = 5
+
+   [somewhere]
+   url = http://somewhe.re
+   private_token = helper: keyring get Service Username
+   timeout = 1
+
+Example for a `pass <https://www.passwordstore.org>`_ helper with a wrapper script:
+
+.. code-block:: ini
+
+   [global]
+   default = somewhere
+   ssl_verify = true
+   timeout = 5
+
+   [somewhere]
+   url = http://somewhe.re
+   private_token = helper: /path/to/helper.sh
+   timeout = 1
 
-Example for a `pass <https://www.passwordstore.org>`_ helper:
+In `/path/to/helper.sh`:
 
 .. code-block:: bash
 
diff --git a/gitlab/config.py b/gitlab/config.py
@@ -17,9 +17,10 @@
 
 import os
 import configparser
+import shlex
 import subprocess
 from typing import List, Optional, Union
-from os.path import expanduser
+from os.path import expanduser, expandvars
 
 from gitlab.const import USER_AGENT
 
@@ -56,6 +57,10 @@ class GitlabConfigMissingError(ConfigError):
     pass
 
 
+class GitlabConfigHelperError(ConfigError):
+    pass
+
+
 class GitlabConfigParser(object):
     def __init__(
         self, gitlab_id: Optional[str] = None, config_files: Optional[List[str]] = None
@@ -202,13 +207,29 @@ def __init__(
             pass
 
     def _get_values_from_helper(self):
-        """Update attributes, which may get values from an external helper program"""
+        """Update attributes that may get values from an external helper program"""
         for attr in HELPER_ATTRIBUTES:
             value = getattr(self, attr)
             if not isinstance(value, str):
                 continue
 
-            if value.lower().strip().startswith(HELPER_PREFIX):
-                helper = expanduser(value[len(HELPER_PREFIX) :].strip())
-                value = subprocess.check_output([helper]).decode("utf-8").strip()
-                setattr(self, attr, value)
+            if not value.lower().strip().startswith(HELPER_PREFIX):
+                continue
+
+            helper = value[len(HELPER_PREFIX) :].strip()
+            commmand = [expanduser(expandvars(token)) for token in shlex.split(helper)]
+
+            try:
+                value = (
+                    subprocess.check_output(commmand, stderr=subprocess.PIPE)
+                    .decode("utf-8")
+                    .strip()
+                )
+            except subprocess.CalledProcessError as e:
+                stderr = e.stderr.decode().strip()
+                raise GitlabConfigHelperError(
+                    f"Failed to read {attr} value from helper "
+                    f"for {self.gitlab_id}:\n{stderr}"
+                ) from e
+
+            setattr(self, attr, value)
