Merge pull request #196 from peopledoc/umask-0600

Default umask to restrict permissions on file

docs/howto/permissions.rst

@@ -6,9 +6,11 @@ use the ``--umask`` option (a value in octal base is expected):
 
 .. code:: console
 
-    $ vault-cli --umask=066 get -o /path/to/secret mysecret
+    $ vault-cli --umask=006 get -o /path/to/secret mysecret
 
-See umask__ for more details on calculating a umask value
+See umask__ for more details on calculating a ``umask`` value. The default
+``umask`` will be ``066``, meaning the file is readable (and writable) by the
+owner only.
 
 .. __: https://en.wikipedia.org/wiki/Umask
 
@@ -18,4 +20,4 @@ Quick crash course:
 - 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
+- "Execute" permission cannot be granted through ``umask``

tests/integration/test_integration.py

@@ -213,8 +213,15 @@ def umask():
     os.umask(current)
 
 
-def test_umask(set_ACD, umask, tmp_path):
+@pytest.mark.parametrize(
+    "flag, expected",
+    [
+        ("", "0o600"),
+        ("--umask=000 ", "0o666"),
+    ],
+)
+def test_umask(set_ACD, umask, tmp_path, flag, expected):
     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"
+    # umask = 0o000 => permissions = 0o666 - 0o000 = 0o666
+    subprocess.check_output(f"vault-cli {flag}get A -o {path}".split())
+    assert oct(path.stat().st_mode & 0o777) == expected

tests/unit/test_cli.py

@@ -571,7 +571,7 @@ def test_dump_config(cli_runner):
 
     expected_settings = settings.DEFAULTS._as_dict()
     expected_settings.update(
-        {"base_path": "mybase/", "token": "some-token", "verbose": 0, "umask": None}
+        {"base_path": "mybase/", "token": "some-token", "verbose": 0, "umask": "0o066"}
     )
 
     output = yaml.safe_load(result.output)
@@ -870,9 +870,7 @@ 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)]
-)
+@pytest.mark.parametrize("input, output", [("022", 0o22), ("0o123", 0o123)])
 def test_parse_octal(input, output):
     assert cli.parse_octal(input) == output
 

vault_cli/cli.py

@@ -49,9 +49,8 @@ def set_verbosity(value: int) -> None:
     logger.info(f"Log level set to {logging.getLevelName(level)}")
 
 
-def set_umask(umask: Optional[int]) -> None:
-    if umask is not None:
-        os.umask(umask)
+def set_umask(umask: int) -> None:
+    os.umask(umask)
 
 
 @contextlib.contextmanager
@@ -70,13 +69,11 @@ def print_version(ctx, __, value):
     ctx.exit()
 
 
-def parse_octal(value: Optional[str]) -> Optional[int]:
-    if not value:
-        return None
+def parse_octal(value: str) -> int:
     return int(value, base=8)
 
 
-def click_octal(_, __, value: Optional[str]) -> Optional[int]:
+def click_octal(_, __, value: str) -> int:
     return parse_octal(value)
 
 
@@ -142,7 +139,9 @@ def repr_octal(value: Optional[int]) -> Optional[str]:
 @click.option(
     "--umask",
     callback=click_octal,
-    help="Set umask for newly created files.",
+    default="066",
+    help="Set umask for newly created files. Defaults to files with read-write "
+    "for owner and nothing for group & others",
 )
 @click.option(
     "-v",