feat: check if password file exists before running pass

Currently, `pass-git-helper` doesn't check if a password file exists before
running `pass`. This leads to authentication failures when the configured
password store entry (aka `target`) points to a directory instead of a file.

Example:

    $ tree -F .password-store
    .password-store/
    └── git/
        ├── github.com.gpg
        └── gitlab.com/
            ├── user1.gpg
            └── user2.gpg

If target is `git/gitlab.com`, `pass-git-helper` will get the following output
from `pass git/gitlab/com`:

    git/gitlab.com
    ├── user1
    └── user2

and will then use the first line (`git/gitlab/com`) as password.

This commit introduces the following changes:

1. To fix the problem described above, `pass-git-helper` first determines the
   `pass` password store directory as:

   - the value of `password_store_dir` if defined in `git_pass_mapping.ini` (and
     non-empty)
   - the value of the environment variable `PASSWORD_STORE_DIR` if defined (and
     non-empty)
   - the default: `~/.password-store`

   (the latter two correspond to the implementation of `pass`).
   `pass-git-helper` will then check if an actual `<target>.gpg` is present in
   the selected directory (to be clear: only one of the three alternative
   directories gets checked).

   The new checks implemented in `pass-git-helper` detect two different error
   cases:
   1. the scenario described above where `target` itself is a directory
   2. a (rather obscure) scenario where `<target>.gpg` is a directory

   In both cases, `pass-git-helper` will now exit with an error after presenting
   the user with a (hopefully) useful error message.

2. The value of `password_store_dir` in the ini file may now contain a leading
   `~` (*tilde*) which will get replaced by the users *HOME* (or, on Windows,
   *USERPROFILE*) directory. The change is documented in README.md.

3. New tests for 1. and 2.

Fixes #371

README.md

@@ -171,13 +171,13 @@ See also the official [documentation](https://git-scm.com/docs/git-config#_inclu
 ### Switching Password Stores per Mapping
 
 To select a different password store for certain entries, the `password_store_dir` configuration key can be set.
-If set, `pass` is directed to a different data directory by defining the `PASSWORD_STORE_DIR` environment variable when calling `pass`.
+If set to a non-empty value, `pass` is directed to a different data directory by defining the `PASSWORD_STORE_DIR` environment variable when calling `pass`. If the `password_store_dir` value starts with a tilde (`~`) it will be replaced with the users *HOME* directory (i.e. with the value of the `HOME` or, on Windows, `USERPROFILE` environment variable, see [os.path.expanduser()](https://docs.python.org/3/library/os.path.html#os.path.expanduser) for details).
 
 The following config demonstrates this practices
 
 ```init
 [github.com/mycompany]
-password_store_dir=/home/me/.work-passwords
+password_store_dir=~/.work-passwords
 ```
 
 ## Password Store Layout and Data Extraction

passgithelper.py

@@ -17,7 +17,7 @@
 import re
 import subprocess
 import sys
-from typing import Dict, IO, Mapping, Optional, Pattern, Sequence, Text
+from typing import Dict, IO, Mapping, Optional, Pattern, Sequence, Text, Tuple
 
 import xdg.BaseDirectory
 
@@ -357,13 +357,68 @@ def define_pass_target(
     return pass_target
 
 
-def compute_pass_environment(section: configparser.SectionProxy) -> Mapping[str, str]:
+def compute_pass_environment(
+    section: configparser.SectionProxy,
+) -> Tuple[Mapping[str, str], Path]:
+    """Returns the environment variables needed to start the ``pass`` subprocess.
+
+    The main task of this function is to determine the password store directory
+    to be used by ``pass``. It does this by:
+
+    1. using the value of ``password_store_dir`` in ``section`` (if defined and
+       non-empty),
+    2. using the value of the ``PASSWORD_STORE_DIR`` environment variable (if
+       defined and non-empty),
+    3. falling back to the default: ``~/password-store``.
+
+    In the next step, a leading ``~`` (tilde) in the resulting path gets
+    replaced by the users ``$HOME`` (on Windows: ``%USERPROFILE%``) directory.
+    See ``os.path.expanduser()`` for more details.
+
+    Finally, the result is used to add or update ``PASSWORD_STORE_DIR`` to/in a
+    copy of the current process environment.
+
+    Args:
+        section:
+            Ini file section which applies to the current password target.
+
+    Returns:
+        A tuple (env, dir) where ``env`` is a dictionary comprising a copy of
+        the current process environment wth updated ``PASSWORD_STORE_DIR`` value
+        and ``dir`` is the value of ``PASSWORD_STORE_DIR`` as a ``Path``
+        instance (for the callers convenience).
+
+    """
     environment = os.environ.copy()
-    password_store_dir = section.get("password_store_dir")
-    if password_store_dir:
-        LOGGER.debug('Setting PASSWORD_STORE_DIR to "%s"', password_store_dir)
-        environment["PASSWORD_STORE_DIR"] = password_store_dir
-    return environment
+    password_store_dir = Path(
+        section.get("password_store_dir")
+        or environment.get("PASSWORD_STORE_DIR")
+        or "~/.password-store"
+    ).expanduser()
+    LOGGER.debug('Setting PASSWORD_STORE_DIR to "%s"', password_store_dir)
+    environment["PASSWORD_STORE_DIR"] = str(password_store_dir)
+    return environment, password_store_dir
+
+
+def check_password_file(password_store_dir: Path, pass_target: str) -> None:
+    """Check that the password file exists and that it is a regular file.
+
+    Args:
+        password_store_dir:
+            Directory which contains the password to be checked.
+        pass_target:
+            Path to the actual password file within ``password_store_dir``
+            (without ``.gpg`` extension).
+
+    Raises:
+        - FileNotFoundError if the password file does not exist,
+        - TypeError if if the password file is not a real file.
+    """
+    pass_target_file = Path(password_store_dir / (pass_target + ".gpg"))
+    if not pass_target_file.exists():
+        raise FileNotFoundError(f"'{pass_target_file}' does not exist")
+    if not pass_target_file.is_file():
+        raise TypeError(f"'{pass_target_file}' is not a file")
 
 
 def get_password(
@@ -399,7 +454,8 @@ def get_password(
         )
     username_extractor.configure(section)
 
-    environment = compute_pass_environment(section)
+    environment, password_store_dir = compute_pass_environment(section)
+    check_password_file(password_store_dir, pass_target)
 
     LOGGER.debug('Requesting entry "%s" from pass', pass_target)
     # silence the subprocess injection warnings as it is the user's

test_passgithelper.py

@@ -1,6 +1,7 @@
 import configparser
 from dataclasses import dataclass
 import io
+from pathlib import Path
 from subprocess import CalledProcessError
 from typing import Any, Iterable, Optional, Sequence, Text
 from unittest.mock import ANY
@@ -28,6 +29,9 @@ def helper_config(mocker: MockerFixture, request: Any) -> Iterable[Any]:
         request.param.request
     )
 
+    if request.param.entry_data != b"check-password-file":
+        mocker.patch("passgithelper.check_password_file")
+
     subprocess_mock = mocker.patch("subprocess.check_output")
     if request.param.entry_data:
         subprocess_mock.return_value = request.param.entry_data
@@ -55,6 +59,85 @@ def test_handle_skip_exits(monkeypatch: Any) -> None:
         passgithelper.handle_skip()
 
 
+class TestPasswordStoreDirSelection:
+    """Test password store directory selection.
+
+    The password store directory used by ``pass`` can either be set via the
+    ``PASSWORD_STORE_DIR`` environment variable or via the
+    ``password_store_dir`` option in the ini file. In case both are present, the
+    ini file option overrides the environment variable.
+
+    Empty values are ignored (as if they have not been set).
+
+    The different variants are tested here.
+
+    """
+
+    def test_uses_default_value(self, monkeypatch: Any) -> None:
+        ini = configparser.ConfigParser()
+
+        expected = str(Path("~/.password-store").expanduser())
+        # no password store configured (neither per env. variable nor ini file
+        # option) -> default value (`~/.password-store`)
+        monkeypatch.delenv("PASSWORD_STORE_DIR", raising=False)
+        ini["example.com"] = {}
+        env, pwd_store_dir = passgithelper.compute_pass_environment(ini["example.com"])
+        assert str(pwd_store_dir) == env.get("PASSWORD_STORE_DIR")
+        assert pwd_store_dir == Path(expected)
+        # also: check `~` expansion
+        assert pwd_store_dir.is_absolute()
+
+    def test_uses_env_var_value(self, monkeypatch: Any) -> None:
+        ini = configparser.ConfigParser()
+
+        expected = "/tmp/password-store-from-env"
+        # `PASSWORD_STORE_DIR` in environment, empty ini file section -> value of
+        # env. variable overrides default
+        monkeypatch.setenv("PASSWORD_STORE_DIR", expected)
+        ini["example.com"] = {}
+        env, pwd_store_dir = passgithelper.compute_pass_environment(ini["example.com"])
+        assert str(pwd_store_dir) == env.get("PASSWORD_STORE_DIR")
+        assert pwd_store_dir == Path(expected)
+
+    def test_ini_file_option_overrides_environment(self, monkeypatch: Any) -> None:
+        ini = configparser.ConfigParser()
+
+        expected = "/tmp/password-store-from-ini"
+        # `PASSWORD_STORE_DIR` in environemnt, `password_store_dir` in ini file
+        # section -> value from ini file overrides env. variable
+        monkeypatch.setenv("PASSWORD_STORE_DIR", "/tmp/password-store-from-env")
+        ini["example.com"] = {"password_store_dir": expected}
+        env, pwd_store_dir = passgithelper.compute_pass_environment(ini["example.com"])
+        assert str(pwd_store_dir) == env.get("PASSWORD_STORE_DIR")
+        assert pwd_store_dir == Path(expected)
+
+    def test_ignores_empty_ini_file_option(self, monkeypatch: Any) -> None:
+        ini = configparser.ConfigParser()
+
+        expected = "/tmp/password-store-from-env"
+        # `PASSWORD_STORE_DIR` in environment, empty `password_store_dir` in ini
+        # file section -> empty ini value ignored, fall back to env. variable
+        monkeypatch.setenv("PASSWORD_STORE_DIR", expected)
+        ini["example.com"] = {"password_store_dir": ""}
+        env, pwd_store_dir = passgithelper.compute_pass_environment(ini["example.com"])
+        assert str(pwd_store_dir) == env.get("PASSWORD_STORE_DIR")
+        assert pwd_store_dir == Path(expected)
+
+    def test_ignores_empty_env_var_value(self, monkeypatch: Any) -> None:
+        ini = configparser.ConfigParser()
+
+        expected = str(Path("~/.password-store").expanduser())
+        # empty `PASSWORD_STORE_DIR` in environment, empty ini file section -> empty
+        # env. var value ignored, fall back to default
+        monkeypatch.setenv("PASSWORD_STORE_DIR", "")
+        ini["example.com"] = {}
+        env, pwd_store_dir = passgithelper.compute_pass_environment(ini["example.com"])
+        assert str(pwd_store_dir) == env.get("PASSWORD_STORE_DIR")
+        assert pwd_store_dir == Path(expected)
+        # also: check `~` expansion
+        assert pwd_store_dir.is_absolute()
+
+
 class TestSkippingDataExtractor:
     class ExtractorImplementation(passgithelper.SkippingDataExtractor):
         def configure(self, config: configparser.SectionProxy) -> None:
@@ -529,3 +612,76 @@ def test_supports_switching_password_store_dirs(
             helper_config.mock_calls[-1].kwargs["env"]["PASSWORD_STORE_DIR"]
             == "/some/dir"
         )
+
+    @pytest.mark.parametrize(
+        "helper_config",
+        [
+            HelperConfig(
+                None,
+                "\nhost=example.com",
+                b"ignored",
+            ),
+        ],
+        indirect=True,
+    )
+    def test_uses_password_store_dir_relative_to_home(
+        self, mocker: MockerFixture, helper_config: Any
+    ) -> None:
+        config = configparser.ConfigParser()
+        config["example.com"] = {
+            "password_store_dir": "~/some/dir",
+            "target": "dev/mytest",
+        }
+        mocker.patch("passgithelper.parse_mapping").return_value = config
+
+        passgithelper.main(["get"])
+        password_store_dir = Path(
+            helper_config.mock_calls[-1].kwargs["env"]["PASSWORD_STORE_DIR"]
+        )
+        assert password_store_dir.is_absolute()
+        assert password_store_dir == Path("~/some/dir").expanduser()
+
+    @pytest.mark.parametrize(
+        "helper_config",
+        [
+            HelperConfig(
+                None,
+                "",
+                b"check-password-file",
+            ),
+        ],
+        indirect=True,
+    )
+    @pytest.mark.usefixtures("helper_config")
+    def test_verifies_check_password_file_function(
+        self, monkeypatch: Any, mocker: MockerFixture, capsys: Any
+    ) -> None:
+        monkeypatch.setenv(
+            "PASSWORD_STORE_DIR", str(Path.cwd() / "test_data/dummy-password-store")
+        )
+        config = configparser.ConfigParser()
+        config["example.com"] = {"target": ""}
+        mocker.patch("passgithelper.parse_mapping").return_value = config
+        mocker.patch("passgithelper.parse_request").return_value = {
+            "host": "example.com"
+        }
+
+        config["example.com"]["target"] = "example.com"
+        with pytest.raises(SystemExit):
+            passgithelper.main(["get"])
+        out, err = capsys.readouterr()
+        assert err.endswith("example.com.gpg' is not a file\n")
+        assert not out
+
+        config["example.com"]["target"] = "example.com/doesnotexist"
+        with pytest.raises(SystemExit):
+            passgithelper.main(["get"])
+        _, err = capsys.readouterr()
+        assert err.endswith("example.com/doesnotexist.gpg' does not exist\n")
+        assert not out
+
+        config["example.com"]["target"] = "example.com/fakepwd"
+        passgithelper.main(["get"])
+        out, err = capsys.readouterr()
+        assert out == "password=check-password-file\n"
+        assert not err