Template injection in vault-cli

@@ -124,8 +124,3 @@ even if it will be missing some secrets.
 .. code:: console
 
     $ vault-cli env --envvar myapp --force -- myapp
-
-.. warning::
-
-    Even if just a single key for a secret produces an error (e.g. a template rendering
-    error), the whole secret will be missing.

@@ -12,7 +12,6 @@ How-to...
     howto/write
     howto/environment
     howto/template
-    howto/templated_secrets
     howto/ssh
     howto/systemd
     howto/organize

@@ -18,14 +18,6 @@ Vault-cli aims at helping you launch your application with the secrets
 it needs without writing them on disk. This page lists a few scenarios
 that may be useful.
 
-If the value you need to pass is directly a secret that is stored in the
-vault, perfect. Otherwise, you may want to create a `templated
-value`__
-to recreate your secret value by combining static strings and other
-secrets.
-
-.. __: https://github.com/peopledoc/vault-cli/#create-a-templated-value
-
 Let’s assume the value you need to pass is the value you get with:
 
 .. code:: console

@@ -1,84 +0,0 @@
-Make a secret point to dynamic content
-======================================
-
-.. warning::
-
-    This feature will be removed from Vault-CLI in the next major version.
-
-With ``vault-cli``, it's possible to have secret values be Jinja2_ templates. This is
-useful if you have multiple related secrets that you would like to retrieve as a single
-string.
-
-.. _Jinja2: https://jinja.palletsprojects.com/en/2.11.x/
-
-.. note::
-
-    This is a pure ``vault-cli`` feature, built on top of vault. Do not expect this
-    to be interoperable with other vault clients.
-
-.. warning::
-
-    ``vault-cli`` takes the assumption that untrusted parties cannot store arbitrary
-    secrets in your vault, and access those secrets, otherwise it would be trivial to
-    use a templated value that would return other secrets. If you really want to use the
-    vault this way, please make sure to use either the ``--no-render`` flag, the
-    ``render: no`` configuration file option or the ``VAULT_CLI_RENDER=false``
-    environment variable.
-
-.. note::
-
-    Templated secrets rendering work like ``vault-cli template``. See :ref:`template`.
-
-Create and read a templated secret
-----------------------------------
-
-Templated secrets start with the special prefix ``!template!``. Vault-cli recongizes
-this and will evaluate the rest of the value as a Jinja2 template.
-
-The template context includes a ``vault(path: str)`` function that returns the secret
-object stored at path. Individual values can be accessed by simply reading attributes on
-the secret object:
-
-.. code:: console
-
-   $ vault-cli set service username=foo password=bar host=example.com
-   $ vault-cli set shortcut dsn='!template!proto://{{ vault("service").username }}:{{ vault("service").password }}@{{ vault("service").host }}/'
-   $ vault-cli get shortcut dsn
-   proto://foo:bar@example.com/
-   $ vault-cli --no-render get shortcut dsn
-   !template!proto://{{ vault("service").username }}:{{ vault("service").password }}@{{ vault("service").host }}/
-
-Variable rendering can be recursive as long as there is no loop (a uses b, b uses a)
-
-Including a templated secret in an Ansible YAML file
-----------------------------------------------------
-
-The following is relevant only if your setup includes Ansible_.
-
-Ansible is based on running Jinja2 on YAML files. Consider a YAML object looking like:
-
-.. code:: yaml
-
-    ---
-    template: !template!{{ vault("path").key }}
-
-The ``!template`` part will be interpreted as an unknown YAML directive. The
-``{{ vault("path").key }}`` will be interpreted by Ansible's Jinja2, which will crash
-because Ansible doesn't have a ``vault`` function in its context.
-
-Using ``!unsafe``, a real Ansible YAML directive this time, we can instruct Ansible
-to leave the rest of the value as-is, and not run Jinja2 on it:
-
-.. code:: yaml
-
-    - name: Add templated secrets
-      command: vault-cli set {{ item.path }} '{{ item.key }}={{ item.template }}'
-      loop:
-        - path: path/one
-          key: mykey
-          template: !unsafe '!template!{{ vault("path").key }}
-        - path: path/two
-          key: otherkey
-          template: !unsafe '!template!{{ vault("/otherpath").somekey }}'
-
-.. _Ansible: https://www.ansible.com/

@@ -1,6 +1,14 @@
 Upgrade ``vault-cli`` from previous version
 ===========================================
 
+From 2.x to 3.x
+~~~~~~~~~~~~~~~
+
+Templated values (``!template!{{ vault("a").b }}``) aren't supported anymore.
+If you were using those, you will get a warning starting on ``vault-cli~=2.2.1``.
+
+On 3.x, a ``--render/--no-render`` flag is still available but it does nothing.
+
 From 1.x to 2.x
 ~~~~~~~~~~~~~~~
 

@@ -42,7 +42,6 @@ Some features
 - Launch processes with your secrets as environment variables
 - Launch processes with ``ssh-agent`` configured from your vault
 - Write templated files with secrets inside
-- Combine multiple secrets into a single one (e.g. a DSN string from components)
 
 ``vault-cli`` tries to make accessing secrets both secure and painless.
 

@@ -1,8 +1,6 @@
-import itertools
-
 import pytest
 
-from vault_cli import client, exceptions, testing
+from vault_cli import client, exceptions
 
 
 def test_get_client(mocker):
@@ -476,44 +474,6 @@ def test_vault_client_base_render_template_path_not_found(vault, template):
         ({"a": {"value": "b"}}, {"value": "b"}),
         # Secret not a string
         ({"a": {"value": ["yay"]}}, {"value": ["yay"]}),
-        # Secret is a template without variable expansion
-        ({"a": {"value": "!template!b"}, "b": {"value": "c"}}, {"value": "b"}),
-        # Secret is a template
-        (
-            {"a": {"value": "!template!{{ vault('b').value }}"}, "b": {"value": "c"}},
-            {"value": "c"},
-        ),
-        # Secret is a dict with containing a template
-        (
-            {
-                "a": {"x": "!template!{{ vault('b').value }}", "y": "yay"},
-                "b": {"value": "c"},
-            },
-            {"x": "c", "y": "yay"},
-        ),
-        # Finite recursion
-        (
-            {
-                "a": {"value": "!template!{{ vault('b').value }}"},
-                "b": {"value": "!template!{{ vault('c').value }}"},
-                "c": {"value": "d"},
-            },
-            {"value": "d"},
-        ),
-        # Infinite Recursion
-        (
-            {
-                "a": {"value": "!template!{{ vault('b').value }}"},
-                "b": {"value": "!template!{{ vault('c').value }}"},
-                "c": {"value": "!template!{{ vault('a').value }}"},
-            },
-            {"value": '<recursive value "a">'},
-        ),
-        # Direct Recursion
-        (
-            {"a": {"value": "!template!{{ vault('a').value }}"}},
-            {"value": '<recursive value "a">'},
-        ),
     ],
 )
 def test_vault_client_base_get_secret(vault, vault_contents, expected):
@@ -522,23 +482,6 @@ def test_vault_client_base_get_secret(vault, vault_contents, expected):
     assert vault.get_secret("a") == expected
 
 
-def test_vault_client_base_get_secret_deprecation_warning(vault, caplog):
-    vault.db = {"a": {"value": "!template!b"}}
-    caplog.set_level("WARNING")
-
-    vault.get_secret("a")
-    assert "Templated values are deprecated" in caplog.records[0].message
-
-
-def test_vault_client_base_get_secret_template_root(vault):
-    vault.base_path = "base"
-    vault.db = {"/base/a": {"value": '!template!{{ vault("a").value }} yay'}}
-
-    # In case of erroneous caching, e.g. a different cache entry
-    # for /base/a and base/a, we would find '<recursive value "a"> yay yay'
-    assert vault.get_secret("/base/a") == {"value": '<recursive value "a"> yay'}
-
-
 def test_vault_client_base_get_secret_multiple_keys(vault):
     vault.db = {"rabbitmq/creds/role": {"username": "foo", "password": "bar"}}
     assert vault.get_secret("rabbitmq/creds/role") == {
@@ -547,22 +490,11 @@ def test_vault_client_base_get_secret_multiple_keys(vault):
     }
 
 
-def test_vault_client_base_get_secret_with_dict(vault):
-    vault.db = {
-        "credentials": {"value": {"username": "foo", "password": "bar"}},
-        "dsn": {
-            "value": "!template!proto://{{ vault('credentials')['value']['username'] }}:{{ vault('credentials').value.password }}@host"
-        },
-    }
-
-    assert vault.get_secret("dsn") == {"value": "proto://foo:bar@host"}
-
-
 def test_vault_client_base_get_secret_not_found(vault):
     vault.db = {}
 
     with pytest.raises(exceptions.VaultSecretNotFound):
-        vault.get_secret("not-exiting")
+        vault.get_secret("not-existing")
 
 
 def test_vault_client_base_get_secret_missing_key(vault):
@@ -572,20 +504,6 @@ def test_vault_client_base_get_secret_missing_key(vault):
         vault.get_secret("a", key="username")
 
 
-def test_vault_client_base_get_secret_template_error(vault, caplog):
-    vault.db = {"a": {"key": "!template!{{"}}
-
-    with pytest.raises(exceptions.VaultRenderTemplateError) as exc_info:
-        vault.get_secret("a")
-
-    assert str(exc_info.value) == 'Error while rendering secret at path "a"'
-    assert (
-        str(exc_info.value.__cause__)
-        == 'Error while rendering secret value for key "key"'
-    )
-    assert str(exc_info.value.__cause__.__cause__) == "Jinja2 template syntax error"
-
-
 def test_vault_client_base_lookup_token(vault):
     assert vault.lookup_token() == {"data": {"expire_time": "2100-01-01T00:00:00"}}
 
@@ -672,56 +590,6 @@ def test_vault_client_base_get_secret_implicit_cache(vault):
     assert vault.get_secret("a") == {"value": "b"}
 
 
-class RaceConditionTestVaultClient(testing.TestVaultClient):
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self.counter = itertools.count()
-
-    def _get_secret(self, path):
-        if path == "a":
-            val = next(self.counter)
-            return {"b": f"b{val}", "c": f"c{val}"}
-        return super()._get_secret(path)
-
-
-def test_vault_client_base_get_secret_implicit_cache_no_race_condition():
-    # In this test we check that if a value is read several times by
-    # a template, implicit caching makes sure we have the same value
-    # every time.
-
-    # Values returned by this client keep changing
-
-    vault = RaceConditionTestVaultClient()
-
-    with vault:
-        assert vault.get_secret("a") == {"b": "b0", "c": "c0"}
-    with vault:
-        assert vault.get_secret("a") == {"b": "b1", "c": "c1"}
-
-    vault.db = {"d": {"value": """!template!{{ vault("a").b }}-{{ vault("a").c }}"""}}
-
-    # b2-c3 would be the value if caching didn't work.
-    with vault:
-        assert vault.get_secret("d") == {"value": "b2-c2"}
-
-
-def test_vault_client_base_get_secrets_implicit_cache_no_race_condition():
-    # In this test, the same value is read twice by get-all and template
-    # We check that 2 values are consistent
-
-    vault = RaceConditionTestVaultClient()
-
-    vault.db = {
-        "a": {},
-        "d": {"value": """!template!{{ vault("a").b }}-{{ vault("a").c }}"""},
-    }
-
-    assert vault.get_secrets("") == {
-        "a": {"b": "b0", "c": "c0"},
-        "d": {"value": "b0-c0"},
-    }
-
-
 def test_vault_client_base_get_secret_explicit_cache(vault):
     vault.db = {"a": {"value": "b"}}
     with vault:

@@ -24,7 +24,6 @@ def test_options(cli_runner, mocker):
             "bla",
             "--ca-bundle",
             "yay",
-            "--no-render",
             "--login-cert",
             "puc",
             "--login-cert-key",
@@ -47,7 +46,6 @@ def test_options(cli_runner, mocker):
     assert set(kwargs) == {
         "base_path",
         "ca_bundle",
-        "render",
         "login_cert",
         "login_cert_key",
         "password",
@@ -66,7 +64,6 @@ def test_options(cli_runner, mocker):
     assert kwargs["url"] == "https://foo"
     assert kwargs["username"] == "user"
     assert kwargs["verify"] is True
-    assert kwargs["render"] is False
 
 
 @pytest.fixture

@@ -2,7 +2,7 @@
 import json
 import logging
 import pathlib
-from typing import Dict, Iterable, List, Optional, Set, Tuple, Type, Union, cast
+from typing import Dict, Iterable, List, Optional, Tuple, Type, cast
 
 import hvac  # type: ignore
 import jinja2
@@ -12,9 +12,6 @@
 
 logger = logging.getLogger(__name__)
 
-JSONRecursive = Union[types.JSONValue, utils.RecursiveValue]
-JSONDictRecursive = Dict[str, JSONRecursive]
-
 
 def get_client(**kwargs) -> "VaultClientBase":
     """
@@ -49,8 +46,6 @@ def get_client(**kwargs) -> "VaultClientBase":
         Path to your config file, instead of the default ones
     safe_write : bool
         If set to True, will keep you from overwriting secrets without force=True
-    render : bool
-        If set to False, templated secrets will not be rendered
 
     Returns
     -------
@@ -82,7 +77,6 @@ def __init__(
         username: Optional[str] = settings.DEFAULTS.username,
         password: Optional[str] = settings.DEFAULTS.password,
         safe_write: bool = settings.DEFAULTS.safe_write,
-        render: bool = settings.DEFAULTS.render,
     ):
         self.url = url
         self.verify: types.VerifyOrCABundle = verify
@@ -94,10 +88,8 @@ def __init__(
         self.username = username
         self.password = password
         self.safe_write = safe_write
-        self.render = render
         self.cache: Dict[str, types.JSONDict] = {}
         self.errors: List[str] = []
-        self._currently_fetching: Set[str] = set()
 
     @property
     def base_path(self):
@@ -166,9 +158,7 @@ def _build_full_path(self, path: str) -> str:
             # path relative to base_path
             return self.base_path + path
 
-    def _browse_recursive_secrets(
-        self, path: str, render: bool = True
-    ) -> Iterable[str]:
+    def _browse_recursive_secrets(self, path: str) -> Iterable[str]:
         """
         Given a secret or folder path, return the path of all secrets
         under it (or the path itself)
@@ -194,12 +184,10 @@ def _browse_recursive_secrets(
                 yield key_url
                 continue
 
-            for sub_path in self._browse_recursive_secrets(key_url, render=render):
+            for sub_path in self._browse_recursive_secrets(key_url):
                 yield sub_path
 
-    def get_all_secrets(
-        self, *paths: str, render: bool = True, flat: bool = False
-    ) -> JSONDictRecursive:
+    def get_all_secrets(self, *paths: str, flat: bool = False) -> types.JSONDict:
         """
         Takes several paths, return the nested dict of all secrets below
         those paths
@@ -208,8 +196,6 @@ def get_all_secrets(
         ----------
         *paths : str
             Paths to read recursively
-        render : bool, optional
-            Whether templated secrets should be rendered, by default True
         flat : bool, optional
             Whether to return flat structure with full path as keys or nested
             structure that looks like a tree
@@ -220,10 +206,10 @@ def get_all_secrets(
             {"folder": {"subfolder": {"secret_key": "secret_value"}}}
         """
 
-        result: JSONDictRecursive = {}
+        result: types.JSONDict = {}
 
         for path in paths:
-            path_dict = self.get_secrets(path, render=render)
+            path_dict = self.get_secrets(path)
             if flat:
                 result.update(path_dict)
             else:
@@ -232,17 +218,15 @@ def get_all_secrets(
         return result
 
     def get_secrets(
-        self, path: str, render: bool = True, relative: bool = False
-    ) -> Dict[str, JSONDictRecursive]:
+        self, path: str, relative: bool = False
+    ) -> Dict[str, types.JSONDict]:
         """
         Takes a path, return all secrets below this path
 
         Parameters
         ----------
         path : str
             Path to read recursively
-        render : bool, optional
-            Whether templated secrets should be rendered, by default True
         relative: bool, optional
             When false (default), the keys of the returned dict are the paths of the secrets
             When true, the keys are the relative paths of the secret to `path` (`""` if the secret is directly at path `path`)
@@ -254,16 +238,14 @@ def get_secrets(
         """
         path = path.rstrip("/")
         try:
-            secrets_paths = list(
-                self._browse_recursive_secrets(path=path, render=render)
-            )
+            secrets_paths = list(self._browse_recursive_secrets(path=path))
         except exceptions.VaultAPIException:
             # If we cannot list secrets, we can't browse them, but there's still
             # a chance that the provided path is a single secret that we can
             # read
             secrets_paths = [path]
 
-        result: Dict[str, JSONDictRecursive] = {}
+        result: Dict[str, types.JSONDict] = {}
         path_obj = pathlib.Path(path)
         for subpath in secrets_paths:
             if relative:
@@ -275,13 +257,10 @@ def get_secrets(
                 key = subpath
 
             try:
-                secret = self.get_secret(path=subpath, render=render)
-                secret = cast(JSONDictRecursive, secret)
+                secret = self.get_secret(path=subpath)
+                secret = cast(types.JSONDict, secret)
                 result[key] = secret
-            except (
-                exceptions.VaultAPIException,
-                exceptions.VaultRenderTemplateError,
-            ) as exc:
+            except exceptions.VaultAPIException as exc:
                 for message in utils.extract_error_messages(exc):
                     logger.error(message)
                     self.errors.append(message)
@@ -305,9 +284,7 @@ def list_secrets(self, path: str) -> Iterable[str]:
         """
         return self._list_secrets(path=self._build_full_path(path))
 
-    def get_secret(
-        self, path: str, key: Optional[str] = None, render: bool = True
-    ) -> Union[types.JSONValue, utils.RecursiveValue]:
+    def get_secret(self, path: str, key: Optional[str] = None) -> types.JSONValue:
         """
         Retrieve the value of a single secret
 
@@ -319,35 +296,18 @@ def get_secret(
         key : str, optional
             If set, return only this key
 
-        render : bool, optional
-            Whether to render templated secret or not, by default True
-
         Returns
         -------
         types.JSONValue
             Secret value
         """
         full_path = self._build_full_path(path)
-        if full_path in self._currently_fetching:
-            return utils.RecursiveValue(path)
 
-        self._currently_fetching.add(full_path)
+        assert self.cache is not None
         try:
-            assert self.cache is not None
-            try:
-                mapping = self.cache[full_path]
-            except KeyError:
-                mapping = self.cache[full_path] = self._get_secret(path=full_path)
-
-            if mapping and render and self.render:
-                try:
-                    mapping = self._render_template_dict(mapping)
-                except exceptions.VaultRenderTemplateError as exc:
-                    message = f'Error while rendering secret at path "{path}"'
-                    raise exceptions.VaultRenderTemplateError(message) from exc
-
-        finally:
-            self._currently_fetching.remove(full_path)
+            mapping = self.cache[full_path]
+        except KeyError:
+            mapping = self.cache[full_path] = self._get_secret(path=full_path)
 
         if key is not None:
             try:
@@ -379,7 +339,7 @@ def delete_secret(self, path: str, key: Optional[str] = None) -> None:
         else:
             # Delete only one attribute
             try:
-                secret = self.get_secret(path, render=False)
+                secret = self.get_secret(path)
             except exceptions.VaultSecretNotFound:
                 # secret does not exist
                 return
@@ -401,7 +361,7 @@ def delete_secret(self, path: str, key: Optional[str] = None) -> None:
     def delete_all_secrets_iter(self, *paths: str) -> Iterable[str]:
         for path in paths:
             path = path.rstrip("/")
-            secrets_paths = self._browse_recursive_secrets(path=path, render=False)
+            secrets_paths = self._browse_recursive_secrets(path=path)
             for secret_path in secrets_paths:
                 yield secret_path
                 self.delete_secret(secret_path)
@@ -435,7 +395,7 @@ def copy_secrets_iter(
         delete_source: Optional[bool] = False,
     ) -> Iterable[Tuple[str, str]]:
 
-        source_secrets = self.get_secrets(path=source, render=False)
+        source_secrets = self.get_secrets(path=source)
 
         for old_path, secret in source_secrets.items():
             new_path = dest + old_path[len(source) :]
@@ -517,41 +477,9 @@ def copy_secrets(
             return iterator
         return list(iterator)
 
-    template_prefix = "!template!"
-
-    def _render_template_value(self, secret: types.JSONValue) -> types.JSONValue:
-
-        if isinstance(secret, dict):
-            return {k: self._render_template_value(v) for k, v in secret.items()}
-        if not isinstance(secret, str):
-            return secret
-
-        if not secret.startswith(self.template_prefix):
-            return secret
-
-        logger.warn(
-            "Templated values are deprecated and will be removed in the "
-            "following major versions.",
-        )
-        return self.render_template(secret[len(self.template_prefix) :])
-
-    def _render_template_dict(
-        self, secrets: Dict[str, types.JSONValue]
-    ) -> Dict[str, types.JSONValue]:
-        result = {}
-        for key, value in secrets.items():
-            try:
-                result[key] = self._render_template_value(value)
-            except exceptions.VaultRenderTemplateError as exc:
-                message = f'Error while rendering secret value for key "{key}"'
-                raise exceptions.VaultRenderTemplateError(message) from exc
-
-        return result
-
     def render_template(
         self,
         template: str,
-        render: bool = True,
         search_path: pathlib.Path = pathlib.Path("."),
     ) -> str:
         """
@@ -581,7 +509,7 @@ def render_template(
 
         def vault(path):
             try:
-                return self.get_secret(path, render=render)
+                return self.get_secret(path)
             except exceptions.VaultException as exc:
                 raise exceptions.VaultRenderTemplateError(
                     "Error while rendering template"
@@ -631,7 +559,7 @@ def set_secret(
         force = self.get_force(force)
 
         try:
-            existing_value = self.get_secret(path=path, render=False)
+            existing_value = self.get_secret(path=path)
             assert isinstance(existing_value, dict)
         except exceptions.VaultSecretNotFound:
             pass
@@ -675,7 +603,7 @@ def set_secret(
         path = path.rstrip("/")
         for parent in list(pathlib.PurePath(path).parents)[:-1]:
             try:
-                self.get_secret(str(parent), render=False)
+                self.get_secret(str(parent))
             except exceptions.VaultSecretNotFound:
                 pass
             except exceptions.VaultForbidden:
@@ -691,7 +619,7 @@ def set_secret(
 
     def set_secrets(
         self,
-        secrets: Dict[str, JSONDictRecursive],
+        secrets: Dict[str, types.JSONDict],
         force: Optional[bool] = None,
         update: Optional[bool] = None,
     ) -> None:

@@ -133,8 +133,8 @@ def repr_octal(value: Optional[int]) -> Optional[str]:
 )
 @click.option(
     "--render/--no-render",
-    default=settings.DEFAULTS.render,
-    help="Render templated values",
+    default=False,
+    help="Deprecated / unused",
 )
 @click.option(
     "--umask",
@@ -174,6 +174,7 @@ def cli(ctx: click.Context, verbose: int, umask: int, **kwargs) -> None:
     (including VAULT_CLI_PASSWORD and VAULT_CLI_TOKEN).
 
     """
+    kwargs.pop("render")
     kwargs.pop("config_file")
     set_verbosity(verbose)
     set_umask(umask)

@@ -5,7 +5,7 @@
 import re
 from typing import Dict, Mapping, NoReturn, Optional, Sequence
 
-from vault_cli import client, exceptions
+from vault_cli import client, exceptions, types
 
 logger = logging.getLogger(__name__)
 
@@ -25,7 +25,7 @@ def _normalize(name: str) -> str:
     return envvar_name
 
 
-def _make_env_value(value: client.JSONRecursive) -> str:
+def _make_env_value(value: types.JSONValue) -> str:
     if isinstance(value, str):
         return value
     return json.dumps(value)
@@ -45,7 +45,7 @@ def exec_command(
 
 
 def get_envvars_for_secrets(
-    secrets: Dict[str, client.JSONDictRecursive],
+    secrets: Dict[str, types.JSONDict],
     path: str,
     prefix: str,
     omit_single_key: bool = False,

@@ -27,7 +27,6 @@ class DEFAULTS:
     verify = True
     ca_bundle = None
     safe_write = False
-    render = True
 
     @staticmethod
     def _as_dict():

@@ -32,17 +32,6 @@ def path_to_nested(dict_obj: Dict) -> Dict:
     return dict_obj
 
 
-class RecursiveValue:
-    def __init__(self, name: str):
-        self.name = name
-
-    def __str__(self) -> str:
-        return f'<recursive value "{self.name}">'
-
-    def __getitem__(self, key: str) -> str:
-        return str(self)
-
-
 def extract_error_messages(exc: BaseException) -> Iterable[str]:
     while True:
         exc_str = str(exc).strip()