Repository: peopledoc/vault-cli

Issue:
Simply select umask
Multiple options of ``vault-cli`` allow creating a file. It would be nice if we could select the umask used by the process for creating files, thous would allow for a simpler integration.

Discussion:

Patch:
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
index b8451b7..735cabc 100644
--- a/.github/PULL_REQUEST_TEMPLATE.md
+++ b/.github/PULL_REQUEST_TEMPLATE.md
@@ -4,4 +4,4 @@ Closes #ticket-number
 - [ ] Write documentation (we can help you doing so)
 - [ ] Write unit tests (100% coverage ?)
 - [ ] Write or edit integration tests, if applicable ?
-- [ ] Add a line in ``CHANGELOG.rst``
+- [ ] Add a line in CHANGELOG.md
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 29dd420..a6dda1f 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,6 +1,12 @@
 Changelog
 =========
 
+1.4.0 (unreleased)
+------------------
+
+- Verbosity can now be controlled through ``verbose:`` option in config file
+- New option ``umask: / --umask / VAULT_CLI_UMASK`` controls permissions of new files (#162)
+
 1.3.1 (2020-04-16)
 ------------------
 
diff --git a/docs/howto/permissions.rst b/docs/howto/permissions.rst
new file mode 100644
index 0000000..7c02abc
--- /dev/null
+++ b/docs/howto/permissions.rst
@@ -0,0 +1,21 @@
+Control permissions of newly created files
+==========================================
+
+If you want the files that ``vault-cli`` creates to have specific permissions,
+use the ``--umask`` option (a value in octal base is expected):
+
+.. code:: console
+
+    $ vault-cli --umask=066 get -o /path/to/secret mysecret
+
+See umask__ for more details on calculating a umask value
+
+.. __: https://en.wikipedia.org/wiki/Umask
+
+Quick crash course:
+
+- The value has 3 digits that can each be 0, 2, 4 or 6
+- First value controls owner permissions, second value controls group permission,
+  third value controls other users permissions
+- 0 is read-write, 2 is read only, 4 is write only, 6 is nothing
+- "Execute" permission cannot be granted through umask
diff --git a/docs/howto_index.rst b/docs/howto_index.rst
index 1004924..a016cc3 100644
--- a/docs/howto_index.rst
+++ b/docs/howto_index.rst
@@ -17,6 +17,7 @@ How-to...
     howto/systemd
     howto/organize
     howto/base_path
+    howto/permissions
     howto/safe_write
     howto/secret_with_dash
     howto/library
diff --git a/vault_cli/cli.py b/vault_cli/cli.py
index 12a740e..1e1cf03 100644
--- a/vault_cli/cli.py
+++ b/vault_cli/cli.py
@@ -43,11 +43,15 @@ def load_config(ctx: click.Context, param: click.Parameter, value: str) -> None:
     ctx.default_map = config
 
 
-def set_verbosity(ctx: click.Context, param: click.Parameter, value: int) -> int:
+def set_verbosity(value: int) -> None:
     level = settings.get_log_level(verbosity=value)
     logging.basicConfig(level=level)
     logger.info(f"Log level set to {logging.getLevelName(level)}")
-    return value
+
+
+def set_umask(umask: Optional[int]) -> None:
+    if umask is not None:
+        os.umask(umask)
 
 
 @contextlib.contextmanager
@@ -66,6 +70,21 @@ def print_version(ctx, __, value):
     ctx.exit()
 
 
+def parse_octal(value: Optional[str]) -> Optional[int]:
+    if not value:
+        return None
+    return int(value, base=8)
+
+
+def click_octal(_, __, value: Optional[str]) -> Optional[int]:
+    return parse_octal(value)
+
+
+def repr_octal(value: Optional[int]) -> Optional[str]:
+    # #05o means octal with 5 digits (0o octal prefix included)
+    return f"{value:#05o}" if value is not None else None
+
+
 @click.group(context_settings=CONTEXT_SETTINGS)
 @click.pass_context
 @click.option(
@@ -121,12 +140,10 @@ def print_version(ctx, __, value):
     help="Render templated values",
 )
 @click.option(
-    "-v",
-    "--verbose",
-    is_eager=True,
-    callback=set_verbosity,
-    count=True,
-    help="Use multiple times to increase verbosity",
+    "--umask", callback=click_octal, help="Set umask for newly created files.",
+)
+@click.option(
+    "-v", "--verbose", count=True, help="Use multiple times to increase verbosity",
 )
 @click.option(
     "--config-file",
@@ -145,7 +162,7 @@ def print_version(ctx, __, value):
     is_eager=True,
 )
 @handle_errors()
-def cli(ctx: click.Context, **kwargs) -> None:
+def cli(ctx: click.Context, verbose: int, umask: int, **kwargs) -> None:
     """
     Interact with a Vault. See subcommands for details.
 
@@ -154,7 +171,8 @@ def cli(ctx: click.Context, **kwargs) -> None:
 
     """
     kwargs.pop("config_file")
-    verbose = kwargs.pop("verbose")
+    set_verbosity(verbose)
+    set_umask(umask)
 
     assert ctx.default_map  # make mypy happy
     kwargs.update(extract_special_args(ctx.default_map, os.environ))
@@ -162,7 +180,8 @@ def cli(ctx: click.Context, **kwargs) -> None:
     # There might still be files to read, so let's do it now
     kwargs = settings.read_all_files(kwargs)
     saved_settings = kwargs.copy()
-    saved_settings.update({"verbose": verbose})
+
+    saved_settings.update({"verbose": verbose, "umask": repr_octal(umask)})
 
     ctx.obj = client.get_client_class()(**kwargs)  # type: ignore
     ctx.obj.auth()
diff --git a/vault_cli/types.py b/vault_cli/types.py
index 2d0eb74..ad7c4d1 100644
--- a/vault_cli/types.py
+++ b/vault_cli/types.py
@@ -3,7 +3,7 @@ import typing as t
 JSONValue = t.Union[str, int, float, bool, None, t.Dict[str, t.Any], t.List[t.Any]]
 JSONDict = t.Dict[str, JSONValue]
 
-Settings = t.Union[str, bool, None]
+Settings = t.Union[str, bool, int, None]
 SettingsDict = t.Dict[str, Settings]
 
 VerifyOrCABundle = t.Union[bool, str]


Test patch:
diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py
index 972a0e8..2dec598 100644
--- a/tests/integration/test_integration.py
+++ b/tests/integration/test_integration.py
@@ -154,15 +154,12 @@ def check_call(command):
 
 
 @pytest.fixture
-def set_ACD(cli_runner):
+def set_ACD(cli_runner, clean_vault):
     call(cli_runner, ["set", "A", "value=B"])
     call(cli_runner, ["set", "C/D", "username=foo", "password=bar"])
-    yield
-    call(cli_runner, ["delete", "A"])
-    call(cli_runner, ["delete", "C/D"])
 
 
-def test_boostrap_env(clean_vault, set_ACD):
+def test_boostrap_env(set_ACD):
     env = subprocess.check_output(
         "vault-cli env -p A -p C -p C/D:password=PASS -- env".split()
     )
@@ -194,3 +191,18 @@ Fh34DrZLZim42czNi6I+ww6+/y68rkmExwToM=
         "-- ssh-add -L".split()
     )
     assert ssh_public in identities.decode("utf-8")
+
+
+@pytest.fixture
+def umask():
+    current = os.umask(0)
+    os.umask(current)
+    yield
+    os.umask(current)
+
+
+def test_umask(set_ACD, umask, tmp_path):
+    path = tmp_path / "test_boostrap_env"
+    # umask = 0o066 => permissions = 0o666 - 0o066 = 0o600
+    subprocess.check_output(f"vault-cli --umask=066 get A -o {path}".split())
+    assert oct(path.stat().st_mode & 0o777) == "0o600"
diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py
index 0bd0d33..dccd9b5 100644
--- a/tests/unit/test_cli.py
+++ b/tests/unit/test_cli.py
@@ -435,7 +435,7 @@ def test_extract_special_args(config, environ, expected):
 def test_set_verbosity(mocker):
     basic_config = mocker.patch("logging.basicConfig")
 
-    cli.set_verbosity(None, None, 1)
+    cli.set_verbosity(1)
 
     basic_config.assert_called_with(level=logging.INFO)
 
@@ -454,7 +454,7 @@ def test_dump_config(cli_runner):
 
     expected_settings = settings.DEFAULTS._as_dict()
     expected_settings.update(
-        {"base_path": "mybase/", "token": "some-token", "verbose": 0}
+        {"base_path": "mybase/", "token": "some-token", "verbose": 0, "umask": None}
     )
 
     output = yaml.safe_load(result.output)
@@ -690,3 +690,24 @@ def test_ssh_wrong_format_passphrase(cli_runner, vault_with_token, mocker):
     )
 
     assert result.exit_code > 0
+
+
+@pytest.mark.parametrize(
+    "input, output", [(None, None), ("022", 0o22), ("0o123", 0o123)]
+)
+def test_parse_octal(input, output):
+    assert cli.parse_octal(input) == output
+
+
+@pytest.mark.parametrize(
+    "input, output",
+    [
+        (None, None),
+        (0o22, "0o022"),
+        (0o123, "0o123"),
+        (0o12345, "0o12345"),
+        (0, "0o000"),
+    ],
+)
+def test_repr_octal(input, output):
+    assert cli.repr_octal(input) == output