From 9d53f4c6f51b947ef830024468f642299da0a8b8 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Fri, 20 Mar 2026 02:51:51 +0100 Subject: [PATCH] Fix GPG verification runtime handling --- flake.nix | 3 + scripts/installation/arch/dependencies.sh | 1 + scripts/installation/centos/dependencies.sh | 1 + scripts/installation/debian/dependencies.sh | 1 + scripts/installation/fedora/dependencies.sh | 1 + scripts/installation/ubuntu/dependencies.sh | 1 + .../git/queries/get_latest_signing_key.py | 67 +++++++++++++++++-- src/pkgmgr/core/repository/verify.py | 9 ++- ...t_git_verification_runtime_dependencies.py | 57 ++++++++++++++++ .../queries/test_get_latest_signing_key.py | 45 ++++++++++--- .../core/repository/test_verify_repository.py | 17 +++++ 11 files changed, 187 insertions(+), 16 deletions(-) create mode 100644 tests/integration/test_git_verification_runtime_dependencies.py diff --git a/flake.nix b/flake.nix index bf27391..244c0b2 100644 --- a/flake.nix +++ b/flake.nix @@ -51,6 +51,8 @@ pyPkgs.pyyaml pyPkgs.jinja2 pyPkgs.pip + pkgs.git + pkgs.gnupg ]; doCheck = false; @@ -87,6 +89,7 @@ buildInputs = [ pythonWithDeps pkgs.git + pkgs.gnupg ansiblePkg ]; diff --git a/scripts/installation/arch/dependencies.sh b/scripts/installation/arch/dependencies.sh index 82dd38a..b98beb7 100755 --- a/scripts/installation/arch/dependencies.sh +++ b/scripts/installation/arch/dependencies.sh @@ -16,6 +16,7 @@ fi pacman -S --noconfirm --needed \ base-devel \ git \ + gnupg \ rsync \ curl \ ca-certificates \ diff --git a/scripts/installation/centos/dependencies.sh b/scripts/installation/centos/dependencies.sh index 2749280..fe339b9 100755 --- a/scripts/installation/centos/dependencies.sh +++ b/scripts/installation/centos/dependencies.sh @@ -6,6 +6,7 @@ echo "[centos/dependencies] Installing CentOS build dependencies..." dnf -y update dnf -y install \ git \ + gnupg2 \ rsync \ rpm-build \ make \ diff --git a/scripts/installation/debian/dependencies.sh b/scripts/installation/debian/dependencies.sh index 847cbea..14441e1 100755 --- a/scripts/installation/debian/dependencies.sh +++ b/scripts/installation/debian/dependencies.sh @@ -9,6 +9,7 @@ DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ debhelper \ dpkg-dev \ git \ + gnupg \ rsync \ bash \ curl \ diff --git a/scripts/installation/fedora/dependencies.sh b/scripts/installation/fedora/dependencies.sh index 0b8ee9c..e2a2ae6 100755 --- a/scripts/installation/fedora/dependencies.sh +++ b/scripts/installation/fedora/dependencies.sh @@ -6,6 +6,7 @@ echo "[fedora/dependencies] Installing Fedora build dependencies..." dnf -y update dnf -y install \ git \ + gnupg2 \ rsync \ rpm-build \ make \ diff --git a/scripts/installation/ubuntu/dependencies.sh b/scripts/installation/ubuntu/dependencies.sh index 00825ce..e988f7d 100755 --- a/scripts/installation/ubuntu/dependencies.sh +++ b/scripts/installation/ubuntu/dependencies.sh @@ -9,6 +9,7 @@ DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ debhelper \ dpkg-dev \ git \ + gnupg \ tzdata \ lsb-release \ rsync \ diff --git a/src/pkgmgr/core/git/queries/get_latest_signing_key.py b/src/pkgmgr/core/git/queries/get_latest_signing_key.py index 9b84b99..a99b7f0 100644 --- a/src/pkgmgr/core/git/queries/get_latest_signing_key.py +++ b/src/pkgmgr/core/git/queries/get_latest_signing_key.py @@ -1,13 +1,33 @@ from __future__ import annotations -from ..errors import GitQueryError, GitRunError -from ..run import run +import subprocess + +from ..errors import GitNotRepositoryError, GitQueryError class GitLatestSigningKeyQueryError(GitQueryError): """Raised when querying the latest commit signing key fails.""" +def _is_not_repository(stderr: str) -> bool: + return "not a git repository" in (stderr or "").lower() + + +def _looks_like_gpg_runtime_error(stderr: str) -> bool: + lowered = (stderr or "").lower() + markers = ( + "cannot run gpg", + "can't check signature", + "no public key", + "failed to create temporary file", + "can't connect to the keyboxd", + "error opening key db", + "gpg failed", + "no such file or directory", + ) + return any(marker in lowered for marker in markers) + + def get_latest_signing_key(*, cwd: str = ".") -> str: """ Return the GPG signing key ID of the latest commit, via: @@ -17,9 +37,46 @@ def get_latest_signing_key(*, cwd: str = ".") -> str: Returns: The key id string (may be empty if commit is not signed). """ + cmd = ["git", "log", "-1", "--format=%GK"] try: - return run(["log", "-1", "--format=%GK"], cwd=cwd).strip() - except GitRunError as exc: + result = subprocess.run( + cmd, + cwd=cwd, + check=False, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + except OSError as exc: raise GitLatestSigningKeyQueryError( - "Failed to query latest signing key.", + "Failed to query latest signing key.\n" + f"Command: {' '.join(cmd)}\n" + f"Reason: {exc}" ) from exc + + stdout = (result.stdout or "").strip() + stderr = (result.stderr or "").strip() + + if result.returncode != 0: + if _is_not_repository(stderr): + raise GitNotRepositoryError( + f"Not a git repository: {cwd!r}\n" + f"Command: {' '.join(cmd)}\n" + f"STDERR:\n{stderr}" + ) + raise GitLatestSigningKeyQueryError( + "Failed to query latest signing key.\n" + f"Command: {' '.join(cmd)}\n" + f"Exit code: {result.returncode}\n" + f"STDOUT:\n{stdout}\n" + f"STDERR:\n{stderr}" + ) + + if not stdout and stderr and _looks_like_gpg_runtime_error(stderr): + raise GitLatestSigningKeyQueryError( + "Failed to query latest signing key.\n" + f"Command: {' '.join(cmd)}\n" + f"STDERR:\n{stderr}" + ) + + return stdout diff --git a/src/pkgmgr/core/repository/verify.py b/src/pkgmgr/core/repository/verify.py index 7bcbd6a..94a33f9 100644 --- a/src/pkgmgr/core/repository/verify.py +++ b/src/pkgmgr/core/repository/verify.py @@ -16,6 +16,7 @@ def verify_repository(repo, repo_dir, mode="local", no_verification=False): commit_hash = "" signing_key = "" + signing_key_query_failed = False # best-effort info collection try: @@ -59,6 +60,7 @@ def verify_repository(repo, repo_dir, mode="local", no_verification=False): except GitLatestSigningKeyQueryError as exc: error_details.append(str(exc)) signing_key = "" + signing_key_query_failed = True commit_check_passed = True gpg_check_passed = True @@ -78,9 +80,10 @@ def verify_repository(repo, repo_dir, mode="local", no_verification=False): if expected_gpg_keys: if not signing_key: gpg_check_passed = False - error_details.append( - f"Expected one of GPG keys: {expected_gpg_keys}, but no signing key was found." - ) + if not signing_key_query_failed: + error_details.append( + f"Expected one of GPG keys: {expected_gpg_keys}, but no signing key was found." + ) elif signing_key not in expected_gpg_keys: gpg_check_passed = False error_details.append( diff --git a/tests/integration/test_git_verification_runtime_dependencies.py b/tests/integration/test_git_verification_runtime_dependencies.py new file mode 100644 index 0000000..a9bf360 --- /dev/null +++ b/tests/integration/test_git_verification_runtime_dependencies.py @@ -0,0 +1,57 @@ +from __future__ import annotations + +import re +import unittest +from pathlib import Path + + +def _find_repo_root() -> Path: + here = Path(__file__).resolve() + for parent in here.parents: + if (parent / "pyproject.toml").is_file() and ( + parent / "src" / "pkgmgr" + ).is_dir(): + return parent + raise RuntimeError( + "Could not determine repository root for pkgmgr integration test" + ) + + +class TestGitVerificationRuntimeDependencies(unittest.TestCase): + def test_flake_app_includes_git_and_gpg_runtime_tools(self) -> None: + repo_root = _find_repo_root() + flake_text = (repo_root / "flake.nix").read_text(encoding="utf-8") + + self.assertIn("pkgs.git", flake_text) + self.assertIn("pkgs.gnupg", flake_text) + + def test_distro_dependency_scripts_install_gpg_tools(self) -> None: + repo_root = _find_repo_root() + expected_packages = { + "arch": "gnupg", + "debian": "gnupg", + "ubuntu": "gnupg", + "fedora": "gnupg2", + "centos": "gnupg2", + } + + missing: list[str] = [] + for distro, package_name in expected_packages.items(): + script_path = ( + repo_root / "scripts" / "installation" / distro / "dependencies.sh" + ) + content = script_path.read_text(encoding="utf-8") + if not re.search(rf"\b{re.escape(package_name)}\b", content): + missing.append( + f"{distro}: expected package {package_name} in {script_path}" + ) + + if missing: + self.fail( + "Git signature verification runtime dependencies are incomplete:\n" + + "\n".join(f" - {item}" for item in missing) + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/pkgmgr/core/git/queries/test_get_latest_signing_key.py b/tests/unit/pkgmgr/core/git/queries/test_get_latest_signing_key.py index d7900e7..f770519 100644 --- a/tests/unit/pkgmgr/core/git/queries/test_get_latest_signing_key.py +++ b/tests/unit/pkgmgr/core/git/queries/test_get_latest_signing_key.py @@ -1,7 +1,8 @@ import unittest +import subprocess from unittest.mock import patch -from pkgmgr.core.git.errors import GitNotRepositoryError, GitRunError +from pkgmgr.core.git.errors import GitNotRepositoryError from pkgmgr.core.git.queries.get_latest_signing_key import ( GitLatestSigningKeyQueryError, get_latest_signing_key, @@ -10,25 +11,53 @@ from pkgmgr.core.git.queries.get_latest_signing_key import ( class TestGetLatestSigningKey(unittest.TestCase): @patch( - "pkgmgr.core.git.queries.get_latest_signing_key.run", - return_value="ABCDEF1234567890\n", + "pkgmgr.core.git.queries.get_latest_signing_key.subprocess.run", + return_value=subprocess.CompletedProcess( + args=["git", "log", "-1", "--format=%GK"], + returncode=0, + stdout="ABCDEF1234567890\n", + stderr="", + ), ) def test_strips_output(self, _mock_run) -> None: out = get_latest_signing_key(cwd="/tmp/repo") self.assertEqual(out, "ABCDEF1234567890") @patch( - "pkgmgr.core.git.queries.get_latest_signing_key.run", - side_effect=GitRunError("boom"), + "pkgmgr.core.git.queries.get_latest_signing_key.subprocess.run", + return_value=subprocess.CompletedProcess( + args=["git", "log", "-1", "--format=%GK"], + returncode=1, + stdout="", + stderr="boom", + ), ) def test_wraps_git_run_error(self, _mock_run) -> None: - with self.assertRaises(GitLatestSigningKeyQueryError): + with self.assertRaisesRegex(GitLatestSigningKeyQueryError, "boom"): get_latest_signing_key(cwd="/tmp/repo") @patch( - "pkgmgr.core.git.queries.get_latest_signing_key.run", - side_effect=GitNotRepositoryError("no repo"), + "pkgmgr.core.git.queries.get_latest_signing_key.subprocess.run", + return_value=subprocess.CompletedProcess( + args=["git", "log", "-1", "--format=%GK"], + returncode=128, + stdout="", + stderr="fatal: not a git repository", + ), ) def test_does_not_catch_not_repository_error(self, _mock_run) -> None: with self.assertRaises(GitNotRepositoryError): get_latest_signing_key(cwd="/tmp/no-repo") + + @patch( + "pkgmgr.core.git.queries.get_latest_signing_key.subprocess.run", + return_value=subprocess.CompletedProcess( + args=["git", "log", "-1", "--format=%GK"], + returncode=0, + stdout="", + stderr="error: cannot run gpg: No such file or directory", + ), + ) + def test_raises_when_git_reports_gpg_runtime_error(self, _mock_run) -> None: + with self.assertRaisesRegex(GitLatestSigningKeyQueryError, "cannot run gpg"): + get_latest_signing_key(cwd="/tmp/repo") diff --git a/tests/unit/pkgmgr/core/repository/test_verify_repository.py b/tests/unit/pkgmgr/core/repository/test_verify_repository.py index dc2700b..23dd774 100644 --- a/tests/unit/pkgmgr/core/repository/test_verify_repository.py +++ b/tests/unit/pkgmgr/core/repository/test_verify_repository.py @@ -77,6 +77,23 @@ class TestVerifyRepository(unittest.TestCase): self.assertEqual(commit, "") self.assertEqual(key, "") + def test_verified_gpg_query_error_does_not_add_missing_key_fallback(self) -> None: + repo = {"verified": {"commit": None, "gpg_keys": ["ABC"]}} + with ( + patch("pkgmgr.core.repository.verify.get_head_commit", return_value=""), + patch( + "pkgmgr.core.repository.verify.get_latest_signing_key", + side_effect=GitLatestSigningKeyQueryError("cannot run gpg"), + ), + ): + ok, errors, commit, key = verify_repository(repo, "/tmp/repo", mode="local") + + self.assertFalse(ok) + self.assertIn("cannot run gpg", " ".join(errors)) + self.assertFalse(any("no signing key was found" in e for e in errors)) + self.assertEqual(commit, "") + self.assertEqual(key, "") + def test_strict_pull_collects_remote_error_message(self) -> None: repo = {"verified": {"commit": "expected", "gpg_keys": None}} with (