Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: check if password file exists before running pass #372

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
72 changes: 64 additions & 8 deletions passgithelper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down
Empty file.
Empty file.
156 changes: 156 additions & 0 deletions test_passgithelper.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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