Merge pull request #39 from languitar/feature/graceful-failures

fix: don't leak Python exceptions to the user

Author: languitar

passgithelper.py

@@ -448,7 +448,13 @@ def main(argv: Optional[Sequence[str]] = None) -> None:
         sys.exit(1)
 
     if action == "get":
-        get_password(request, mapping)
+        try:
+            get_password(request, mapping)
+        except Exception as error:
+            print(  # noqa: T001
+                "Unable to retrieve entry: {error}".format(error=error), file=sys.stderr
+            )
+            sys.exit(1)
     else:
         LOGGER.info("Action %s is currently not supported", action)
         sys.exit(1)

test_passgithelper.py

@@ -1,6 +1,7 @@
 import configparser
 from dataclasses import dataclass
 import io
+from subprocess import CalledProcessError
 from typing import Any, Iterable, Optional, Sequence, Text
 
 import pytest
@@ -13,7 +14,7 @@
 class HelperConfig:
     xdg_dir: Optional[str]
     request: str
-    entry_data: bytes
+    entry_data: Optional[bytes]
     entry_name: Optional[str] = None
 
 
@@ -27,7 +28,10 @@ def _helper_config(mocker: MockFixture, request: Any) -> Iterable[None]:
     )
 
     subprocess_mock = mocker.patch("subprocess.check_output")
-    subprocess_mock.return_value = request.param.entry_data
+    if request.param.entry_data:
+        subprocess_mock.return_value = request.param.entry_data
+    else:
+        subprocess_mock.side_effect = CalledProcessError(1, ["pass"], "pass failed")
 
     yield
 
@@ -216,10 +220,13 @@ def test_smoke_resolve(self, capsys: Any) -> None:
         indirect=True,
     )
     @pytest.mark.usefixtures("_helper_config")
-    def test_path_used_if_present_fails(self) -> None:
-        with pytest.raises(ValueError, match="No mapping section"):
+    def test_path_used_if_present_fails(self, capsys: Any) -> None:
+        with pytest.raises(SystemExit):
             passgithelper.main(["get"])
 
+        _, err = capsys.readouterr()
+        assert "No mapping section" in err
+
     @pytest.mark.parametrize(
         "_helper_config",
         [
@@ -364,7 +371,7 @@ def test_prefix_skipping(self, capsys: Any) -> None:
     )
     @pytest.mark.usefixtures("_helper_config")
     def test_select_unknown_extractor(self) -> None:
-        with pytest.raises(KeyError):
+        with pytest.raises(SystemExit):
             passgithelper.main(["get"])
 
     @pytest.mark.parametrize(
@@ -450,3 +457,46 @@ def test_uses_utf8_by_default(self, capsys: Any) -> None:
 
         out, _ = capsys.readouterr()
         assert out == "password=täßt\n"
+
+    @pytest.mark.parametrize(
+        "_helper_config",
+        [
+            HelperConfig(
+                "test_data/smoke",
+                """
+protocol=https
+host=mytest.com""",
+                None,
+                "dev/mytest",
+            ),
+        ],
+        indirect=True,
+    )
+    @pytest.mark.usefixtures("_helper_config")
+    def test_fails_gracefully_on_pass_errors(self, capsys: Any) -> None:
+        with pytest.raises(SystemExit):
+            passgithelper.main(["get"])
+
+        _, err = capsys.readouterr()
+        assert "Unable to retrieve" in err
+
+    @pytest.mark.parametrize(
+        "_helper_config",
+        [
+            HelperConfig(
+                "test_data/smoke",
+                """
+protocol=https
+host=unknown""",
+                "ignored".encode("UTF-8"),
+            ),
+        ],
+        indirect=True,
+    )
+    @pytest.mark.usefixtures("_helper_config")
+    def test_fails_gracefully_on_missing_entries(self, capsys: Any) -> None:
+        with pytest.raises(SystemExit):
+            passgithelper.main(["get"])
+
+        _, err = capsys.readouterr()
+        assert "Unable to retrieve" in err