Merge pull request #30 from languitar/feature/cleanup

Project cleanup

.github/workflows/ci.yml

@@ -7,7 +7,7 @@ on:
     - cron: '0 0 * * 0'
 
 jobs:
-  lint:
+  lint-code:
     runs-on: ubuntu-latest
 
     steps:
@@ -24,6 +24,17 @@ jobs:
       - name: Lint with tox
         run: tox -e check
 
+  lint-readme:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Clone repo
+        uses: actions/checkout@v2
+      - name: Lint README.md
+        uses: docker://avtodev/markdown-lint:v1
+        with:
+          args: './README.md'
+
   test:
     runs-on: ubuntu-latest
 

.markdownlintrc

@@ -0,0 +1,5 @@
+{
+    "default": true,
+    "line_length": false,
+    "MD041": false
+}

README.md

@@ -2,7 +2,7 @@
 
 # pass-git-helper
 
-[![Debian CI](https://badges.debian.net/badges/debian/testing/pass-git-helper/version.svg)](https://buildd.debian.org/pass-git-helper) 
+[![Debian CI](https://badges.debian.net/badges/debian/testing/pass-git-helper/version.svg)](https://buildd.debian.org/pass-git-helper)
 [![AUR](https://img.shields.io/aur/version/pass-git-helper.svg)](https://aur.archlinux.org/packages/pass-git-helper/)
 [![Homebrew](https://img.shields.io/badge/dynamic/json.svg?url=https://formulae.brew.sh/api/formula/pass-git-helper.json&query=$.versions.stable&label=homebrew)
 
@@ -15,10 +15,12 @@ It is recommended to configure GPG to use a graphical pinentry program.
 That way, you can also use this helper when [git] is invoked via GUI programs such as your IDE.
 For a configuration example, refer to the [ArchWiki](https://wiki.archlinux.org/index.php/GnuPG#pinentry).
 In case you really want to use the terminal for pinentry (via `pinentry-curses`), be sure to [appropriately configure the environment variable `GPG_TTY`](https://www.gnupg.org/documentation/manuals/gnupg/Invoking-GPG_002dAGENT.html), most likely by adding the following lines to your shell initialization:
+
 ```sh
 GPG_TTY=$(tty)
 export GPG_TTY
 ```
+
 If you use this setup for remote work via SSH, also consider the alternative of [GPG agent forwarding](https://wiki.gnupg.org/AgentForwarding).
 
 ## Installation
@@ -35,11 +37,13 @@ sudo pip install .
 
 This might potentially install Python packages without the knowledge of your system's package manager.
 If all package preconditions are already met, you can also copy the script file to to your system to avoid this problem:
+
 ```sh
 sudo cp passgithelper.py /usr/local/bin/pass-git-helper
 ```
 
 Another option is to install the script in an isolated [virtualenv](https://virtualenv.pypa.io/en/latest/):
+
 ```sh
 virtualenv /your/env
 /your/env/pip install .
@@ -54,6 +58,7 @@ Matching supports wildcards (using the python [fnmatch module](https://docs.pyth
 Each section needs to contain a `target` entry pointing to the entry in the password store with the password (and optionally username) to use.
 
 Example:
+
 ```ini
 [github.com*]
 target=dev/github
@@ -62,59 +67,65 @@ target=dev/github
 target=dev/fooo-bar
 ```
 
-To instruct git to use the helper, set the `credential.helper` configuration option of git to:
-```
-/full/path/to/pass-git-helper
-```
-In case you do not want to include a full path, a workaround using a shell fragment needs to be used, i.e.:
-```
-!pass-git-helper $@
-```
+To instruct git to use the helper, set the `credential.helper` configuration option of git to `/full/path/to/pass-git-helper`.
+In case you do not want to include a full path, a workaround using a shell fragment needs to be used, i.e. `!pass-git-helper $@` must be the option value.
+The option can be set using the CLI with:
 
-The option can be set e.g. via:
 ```sh
 git config credential.helper '!pass-git-helper $@'
 ```
 
 If you want to match entries not only based on the host, but also based on the path on a host, set `credential.useHttpPath` to `true` in your git config, e.g. via:
+
 ```sh
 git config credential.useHttpPath true
 ```
+
 Afterwards, entries can be matched against `host.com/path/to/repo` in the mapping.
-This means that in order to use a specific account for a certain github project, you can then use the following mapping pattern:
+This means that in order to use a specific account for a certain Github project, you can then use the following mapping pattern:
+
 ```ini
 [github.com/username/project*]
 target=dev/github
 ```
+
 Please note that when including the path in the mapping, the mapping expressions need to match against the whole path.
-As a consequence, in case you want to use the same account for all github projects, you need to make sure that a wildcard covers the path of the URL, as shown here:
+As a consequence, in case you want to use the same account for all Github projects, you need to make sure that a wildcard covers the path of the URL, as shown here:
+
 ```ini
 [github.com*]
 target=dev/github
 ```
+
 The host can be used as a variable to address a pass entry.
 This is especially helpful for wildcard matches:
+
 ```ini
 [*]
 target=git-logins/${host}
 ```
+
 The above configuration directive will lead to any host that did not match any previous section in the ini file to being looked up under the `git-logins` directory in your passwordstore.
 
 Using the `includeIf` directive available in git >= 2.13, it is also possible to perform matching based on the current working directory by invoking `pass-git-helper` with a conditional `MAPPING-FILE`.
 To achieve this, edit your `.gitconfig`, e.g. like this:
+
 ```ini
 [includeIf "gitdir:~/src/user1/"]
     path=~/.config/git/gitconfig_user1
 [includeIf "gitdir:~/src/user2/"]
     path=~/.config/git/gitconfig_user2
 ```
+
 With the following contents of `gitconfig_user1` (and `gitconfig_user2` repspectively), `mapping_user1.ini`, which could contain a `target` entry to e.g. `github.com/user1` would always be invoked in `~/src/user1`:
+
 ```ini
 [user]
     name = user1
 [credential]
     helper=/full/path/to/pass-git-helper -m /full/path/to/mapping_user1.ini
 ```
+
 See also the offical [documentation](https://git-scm.com/docs/git-config#_includes) for `.gitconfig`.  
 
 ### DEFAULT section
@@ -165,14 +176,16 @@ Extracts the data from a line indexed by its line number.
 Optionally a fixed-length prefix can be stripped before returning the line contents.
 
 Configuration:
+
 * `line_username`: Line number containing the username, **0-based**. Default: 1 (second line)
-* `skip_username`: Number of characters to skip at the beginning of the line, for instance to skip a `user: ` prefix. Similar to `skip_password`. Default: 0.
+* `skip_username`: Number of characters to skip at the beginning of the line, for instance to skip a `user:` prefix. Similar to `skip_password`. Default: 0.
 
 #### Strategy "regex_search"
 
 Searches for the first line that matches a provided regular expressions and returns the contents of that line that are captured in a regular expression capture group.
 
 Configuration:
+
 * `regex_username`: The regular expression to apply. Has to contain a single capture group for indicating the data to extract. Default: `^username: +(.*)$`.
 
 #### Strategy "entry_name"

passgithelper.py

@@ -17,7 +17,7 @@
 import re
 import subprocess
 import sys
-from typing import Dict, Optional, Pattern, Sequence, Text
+from typing import Dict, IO, Mapping, Optional, Pattern, Sequence, Text
 
 import xdg.BaseDirectory
 
@@ -70,12 +70,10 @@ def parse_arguments(argv: Optional[Sequence[str]] = None) -> argparse.Namespace:
         help="Action to preform as specified in the git credential API",
     )
 
-    args = parser.parse_args(argv)
+    return parser.parse_args(argv)
 
-    return args
 
-
-def parse_mapping(mapping_file: Optional[str]) -> configparser.ConfigParser:
+def parse_mapping(mapping_file: Optional[IO]) -> configparser.ConfigParser:
     """
     Parse the file containing the mappings from hosts to pass entries.
 
@@ -86,7 +84,7 @@ def parse_mapping(mapping_file: Optional[str]) -> configparser.ConfigParser:
     """
     LOGGER.debug("Parsing mapping file. Command line: %s", mapping_file)
 
-    def parse(mapping_file):
+    def parse(mapping_file: IO) -> configparser.ConfigParser:
         config = configparser.ConfigParser()
         config.read_file(mapping_file)
         return config
@@ -103,9 +101,9 @@ def parse(mapping_file):
             "No mapping configured so far at any XDG config location. "
             "Please create {config_file}".format(config_file=DEFAULT_CONFIG_FILE)
         )
-    mapping_file = os.path.join(xdg_config_dir, CONFIG_FILE_NAME)
+    default_file = os.path.join(xdg_config_dir, CONFIG_FILE_NAME)
     LOGGER.debug("Parsing mapping file %s", mapping_file)
-    with open(mapping_file, "r") as file_handle:
+    with open(default_file, "r") as file_handle:
         return parse(file_handle)
 
 
@@ -196,7 +194,7 @@ def __init__(self, prefix_length: int, option_suffix: Text = "") -> None:
         self._prefix_length = prefix_length
 
     @abc.abstractmethod
-    def configure(self, config: configparser.SectionProxy):
+    def configure(self, config: configparser.SectionProxy) -> None:
         """Configure the amount of characters to skip."""
         self._prefix_length = config.getint(
             "skip{suffix}".format(suffix=self._option_suffix),
@@ -325,7 +323,51 @@ def get_value(
 }
 
 
-def get_password(request, mapping) -> None:
+def find_mapping_section(
+    mapping: configparser.ConfigParser, request_header: str
+) -> configparser.SectionProxy:
+    """Select the mapping entry matching the request header."""
+
+    LOGGER.debug('Searching mapping to match against header "%s"', request_header)
+    for section in mapping.sections():
+        if fnmatch.fnmatch(request_header, section):
+            LOGGER.debug(
+                'Section "%s" matches requested header "%s"', section, request_header
+            )
+            return mapping[section]
+
+    raise ValueError(
+        f"No mapping section in {mapping.sections()} matches request {request_header}"
+    )
+
+
+def get_request_section_header(request: Mapping[str, str]) -> str:
+    """Return the canonical host + optional path for section header matching."""
+
+    if "host" not in request:
+        LOGGER.error("host= entry missing in request. Cannot query without a host")
+        raise ValueError("Request lacks host entry")
+
+    host = request["host"]
+    if "path" in request:
+        host = "/".join([host, request["path"]])
+    return host
+
+
+def define_pass_target(
+    section: configparser.SectionProxy, request: Mapping[str, str]
+) -> str:
+    """Determine the pass target by filling in potentially used variables."""
+
+    pass_target = section.get("target").replace("${host}", request["host"])
+    if "username" in request:
+        pass_target = pass_target.replace("${username}", request["username"])
+    return pass_target
+
+
+def get_password(
+    request: Mapping[str, str], mapping: configparser.ConfigParser
+) -> None:
     """
     Resolve the given credential request in the provided mapping definition.
 
@@ -337,54 +379,35 @@ def get_password(request, mapping) -> None:
         mapping:
             The mapping configuration as a ConfigParser instance.
     """
-    LOGGER.debug('Received request "%s"', request)
-    if "host" not in request:
-        LOGGER.error("host= entry missing in request. Cannot query without a host")
-        return
 
-    host = request["host"]
-    if "path" in request:
-        host = "/".join([host, request["path"]])
-
-    def skip(line, skip):
-        return line[skip:]
+    LOGGER.debug('Received request "%s"', request)
 
-    LOGGER.debug('Iterating mapping to match against host "%s"', host)
-    for section in mapping.sections():
-        if fnmatch.fnmatch(host, section):
-            LOGGER.debug('Section "%s" matches requested host "%s"', section, host)
-            # TODO handle exceptions
-            pass_target = mapping.get(section, "target").replace(
-                "${host}", request["host"]
-            )
-            if "username" in request:
-                pass_target = pass_target.replace("${username}", request["username"])
-
-            password_extractor = SpecificLineExtractor(0, 0, option_suffix="_password")
-            password_extractor.configure(mapping[section])
-            username_extractor = _username_extractors[
-                mapping[section].get(
-                    "username_extractor", fallback=_line_extractor_name
-                )
-            ]
-            username_extractor.configure(mapping[section])
-
-            LOGGER.debug('Requesting entry "%s" from pass', pass_target)
-            output = subprocess.check_output(["pass", "show", pass_target]).decode(
-                mapping[section].get("encoding", "UTF-8")
-            )
-            lines = output.splitlines()
-
-            password = password_extractor.get_value(pass_target, lines)
-            username = username_extractor.get_value(pass_target, lines)
-            if password:
-                print("password={password}".format(password=password))  # noqa: T001
-            if "username" not in request and username:
-                print("username={username}".format(username=username))  # noqa: T001
-            return
-
-    LOGGER.warning("No mapping matched")
-    sys.exit(1)
+    header = get_request_section_header(request)
+    section = find_mapping_section(mapping, header)
+
+    pass_target = define_pass_target(section, request)
+
+    password_extractor = SpecificLineExtractor(0, 0, option_suffix="_password")
+    password_extractor.configure(section)
+    username_extractor = _username_extractors[
+        section.get("username_extractor", fallback=_line_extractor_name)
+    ]
+    username_extractor.configure(section)
+
+    LOGGER.debug('Requesting entry "%s" from pass', pass_target)
+    # silence the subprocess injection warnings as it is the user's
+    # responsibility to provide a safe mapping and execution environment
+    output = subprocess.check_output(  # noqa: S603, S607
+        ["pass", "show", pass_target]
+    ).decode(section.get("encoding", "UTF-8"))
+    lines = output.splitlines()
+
+    password = password_extractor.get_value(pass_target, lines)
+    username = username_extractor.get_value(pass_target, lines)
+    if password:
+        print("password={password}".format(password=password))  # noqa: T001
+    if "username" not in request and username:
+        print("username={username}".format(username=username))  # noqa: T001
 
 
 def handle_skip() -> None:

setup.cfg

@@ -14,9 +14,21 @@ exclude =
     .eggs,
     env,
     .mypy_cache
-ignore = D413, E203, W503
+ignore =
+    ANN101,
+    ANN102,
+    D202,
+    D413,
+    E203,
+    S101,
+    S404,
+    TYP101,
+    TYP102,
+    TYP002,
+    TYP003,
+    W503,
 per-file-ignores =
-    test_*: D1
+    test_*: D1, S105
     setup.py: D1
 application-import-names = passgithelper
 import-order-style = google
@@ -40,3 +52,8 @@ exclude_lines =
 
 [mypy]
 ignore_missing_imports=True
+disallow_untyped_defs = True
+check_untyped_defs = True
+no_implicit_optional = True
+warn_unused_configs = True
+warn_unused_ignores = True