Repository: peopledoc/vault-cli

Issue:
[Vault_cli][SSH] ssh-add message
Bonjour à tous, 

lors de l'utilisation de vault_cli avec l'option ssh, le message de ssh-add s'affiche.

```
/usr/local/bin/vault --config-file /etc/{SERVICE}/vault_ro.yml ssh --key {PATH}:{VALUE}

Identity added: (stdin) ((stdin))
```

Est-il possible de rendre cette affichage optionnel afin de pouvoir le désactiver (notamment lors de l'utilisation via cron) ?

Discussion:
ewjoachim: Hi ! Thanks for your contribution !

Let me know if you would rather:
- Finish it (write missing tests)
- Have us finish it for you
- Finish it with us

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 988012c..e75aad1 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -7,6 +7,7 @@ Changelog
 - Verbosity can now be controlled through ``verbose:`` option in config file
 - New option ``umask: / --umask / VAULT_CLI_UMASK`` controls permissions of new files (#162)
 - Forced dependency pyyaml>=5.3.1 to avoid CVE (#156)
+- Silenced ``ssh-add`` in ``vault-cli ssh`` (#167)
 
 1.3.1 (2020-04-16)
 ------------------
diff --git a/vault_cli/cli.py b/vault_cli/cli.py
index 1e1cf03..2a00471 100644
--- a/vault_cli/cli.py
+++ b/vault_cli/cli.py
@@ -59,7 +59,14 @@ def handle_errors():
     try:
         yield
     except exceptions.VaultException as exc:
-        raise click.ClickException(str(exc))
+        messages = []
+        while True:
+            exc_str = str(exc).strip()
+            messages.append(f"{type(exc).__name__}: {exc_str}")
+            exc = exc.__cause__ or exc.__context__
+            if not exc:
+                break
+        raise click.ClickException("\n".join(messages))
 
 
 def print_version(ctx, __, value):
diff --git a/vault_cli/exceptions.py b/vault_cli/exceptions.py
index ccfce57..38e06a6 100644
--- a/vault_cli/exceptions.py
+++ b/vault_cli/exceptions.py
@@ -80,3 +80,7 @@ class VaultSealed(VaultAPIException):
 
 class VaultInvalidEnvironmentName(VaultException):
     pass
+
+
+class VaultSubprocessException(VaultException):
+    pass
diff --git a/vault_cli/ssh.py b/vault_cli/ssh.py
index 0d7a620..32a7264 100644
--- a/vault_cli/ssh.py
+++ b/vault_cli/ssh.py
@@ -4,6 +4,7 @@ import sys
 from typing import Mapping, Optional, Sequence
 
 from vault_cli import environment as env_module
+from vault_cli import exceptions
 
 SSH_PASSPHRASE_ENVVAR = "VAULT_CLI_SSH_PASSPHRASE"
 
@@ -18,11 +19,20 @@ def _launch_command(
     command: Sequence[str], stdin: str, environment: Mapping[str, str]
 ) -> str:
     environment = env_module.full_environment(environment)
-    process = subprocess.Popen(
-        command, stdin=subprocess.PIPE, stdout=subprocess.PIPE, env=environment
+    process = subprocess.run(
+        command,
+        input=stdin,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        env=environment,
+        encoding="utf-8",
     )
-    stdout, _ = process.communicate(input=stdin.encode("utf-8"))
-    return stdout.decode("utf-8")
+    try:
+        process.check_returncode()
+    except subprocess.CalledProcessError as exc:
+        raise exceptions.VaultSubprocessException(process.stderr.strip()) from exc
+
+    return process.stdout.strip()
 
 
 def add_key(key: str, passphrase: Optional[str] = None) -> None:


Test patch:
diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py
index b7fa4a8..741e03a 100644
--- a/tests/integration/test_integration.py
+++ b/tests/integration/test_integration.py
@@ -84,7 +84,10 @@ c:
     assert result.exit_code == 1
     assert (
         result.output
-        == "Error: Secret not found\nKey 'foo' not found in secret at path '/secretkvv1/c/d'\n"
+        == """Error: VaultSecretNotFound: Secret not found
+Key 'foo' not found in secret at path '/secretkvv1/c/d'
+KeyError: 'foo'
+"""
     )
 
     assert call(cli_runner, ["list"]).output == "c/\n"
@@ -190,11 +193,16 @@ Fh34DrZLZim42czNi6I+ww6+/y68rkmExwToM=
         cli_runner,
         ["set", "ssh_key", f"private={ssh_private}", f"passphrase={ssh_passphrase}"],
     )
-    identities = subprocess.check_output(
+    identities = subprocess.run(
         "vault-cli ssh --key ssh_key:private --passphrase ssh_key:passphrase "
-        "-- ssh-add -L".split()
+        "-- ssh-add -L".split(),
+        check=True,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
     )
-    assert ssh_public in identities.decode("utf-8")
+    assert ssh_public in identities.stdout.decode("utf-8")
+    assert "Identity added" not in identities.stdout.decode("utf-8")
+    assert identities.stderr.decode("utf-8") == ""
 
 
 @pytest.fixture
diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py
index dccd9b5..3196c7a 100644
--- a/tests/unit/test_cli.py
+++ b/tests/unit/test_cli.py
@@ -619,11 +619,13 @@ def test_lookup_token(cli_runner, vault_with_token):
 def test_handle_errors(cli_runner):
     @cli.handle_errors()
     def inner():
-        raise exceptions.VaultException("yay")
+        raise exceptions.VaultException("foo") from ValueError("bar")
 
-    with pytest.raises(click.ClickException):
+    with pytest.raises(click.ClickException) as exc_info:
         inner()
 
+    assert str(exc_info.value) == "VaultException: foo\nValueError: bar"
+
 
 def test_version(cli_runner):
     result = cli_runner.invoke(cli.cli, ["--version"])
diff --git a/tests/unit/test_ssh.py b/tests/unit/test_ssh.py
index 1674b39..bdd3986 100644
--- a/tests/unit/test_ssh.py
+++ b/tests/unit/test_ssh.py
@@ -1,6 +1,6 @@
 import pytest
 
-from vault_cli import ssh
+from vault_cli import exceptions, ssh
 
 
 @pytest.fixture
@@ -24,10 +24,22 @@ def test_ensure_agent_already_present(exec, environ):
 # This should rather be an integration test
 def test_launch_command(environ):
     environ.update({"A": "B"})
-    stdout = ssh._launch_command(["env"], "", {"C": "D"})
+    stdout = ssh._launch_command(command=["env"], stdin="", environment={"C": "D"})
     assert "A=B\nC=D" in stdout
 
 
+# This should rather be an integration test
+def test_launch_command_error(environ):
+    non_existing_filename = "a" * 30
+    with pytest.raises(exceptions.VaultSubprocessException) as exc_info:
+        ssh._launch_command(
+            command=["cat", non_existing_filename], stdin="", environment={}
+        )
+
+    error_message = f"cat: {non_existing_filename}: No such file or directory"
+    assert str(exc_info.value) == error_message
+
+
 @pytest.fixture
 def launch_command(mocker):
     return mocker.patch("vault_cli.ssh._launch_command")