Implement GPG signing and verification support for commits and tags (#1909)

This implements support for GPG signing and verification as requested in
#1790. Changes include:

- Fix bug in verify() methods where key.subkeys was incorrectly iterated
- Add support for commit.gpgsign and tag.gpgsign config options
- Add support for user.signingkey config option
- Add 'sign' parameter to porcelain.commit() for GPG signing commits
- Update porcelain.tag_create() to properly handle GPG signing with
config
- Add verify_commit() and verify_tag() functions to porcelain module
- Update worktree.commit() to support config-based signing
- Clarify that 'signoff' is for DCO Signed-off-by, separate from GPG
signing

.github/workflows/pythontest.yml

@@ -46,7 +46,7 @@ jobs:
           # Install build-time dependencies
           python -m pip install --upgrade "setuptools>=77"
           python -m pip install --upgrade pip
-          pip install --upgrade ".[merge,fastimport,paramiko,https]"  setuptools-rust
+          pip install --upgrade ".[merge,fastimport,paramiko,https,patiencediff,colordiff]"  setuptools-rust
       - name: Install gpg on supported platforms
         run: pip install --upgrade ".[pgp]"
         if: "matrix.os != 'windows-latest' && matrix.python-version != 'pypy3'"

dulwich/objects.py

@@ -1172,7 +1172,7 @@ class Tag(ShaFile):
             if keyids:
                 keys = [ctx.get_key(key) for key in keyids]
                 for key in keys:
-                    for subkey in keys:
+                    for subkey in key.subkeys:
                         for sig in result.signatures:
                             if subkey.can_sign and subkey.fpr == sig.fpr:
                                 return
@@ -1919,7 +1919,7 @@ class Commit(ShaFile):
             if keyids:
                 keys = [ctx.get_key(key) for key in keyids]
                 for key in keys:
-                    for subkey in keys:
+                    for subkey in key.subkeys:
                         for sig in result.signatures:
                             if subkey.can_sign and subkey.fpr == sig.fpr:
                                 return

dulwich/porcelain.py

@@ -575,9 +575,10 @@ def commit(
     commit_timezone: Optional[int] = None,
     encoding: Optional[bytes] = None,
     no_verify: bool = False,
-    signoff: bool = False,
+    signoff: Optional[bool] = None,
     all: bool = False,
     amend: bool = False,
+    sign: Optional[bool] = None,
 ) -> bytes:
     """Create a new commit.
 
@@ -591,11 +592,11 @@ def commit(
       commit_timezone: Commit timestamp timezone
       encoding: Encoding to use for commit message
       no_verify: Skip pre-commit and commit-msg hooks
-      signoff: GPG Sign the commit (bool, defaults to False,
-        pass True to use default GPG key,
-        pass a str containing Key ID to use a specific GPG key)
+      signoff: Add Signed-off-by line to commit message. If None, uses format.signoff config.
       all: Automatically stage all tracked files that have been modified
       amend: Replace the tip of the current branch by creating a new commit
+      sign: GPG sign the commit. If None, uses commit.gpgsign config.
+        If True, signs with default GPG key. If False, does not sign.
     Returns: SHA1 of the new commit
     """
     encoding_str = encoding.decode("ascii") if encoding else DEFAULT_ENCODING
@@ -676,7 +677,7 @@ def commit(
                 commit_timezone=commit_timezone,
                 encoding=encoding,
                 no_verify=no_verify,
-                sign=bool(signoff),
+                sign=sign,
                 merge_heads=merge_heads,
                 ref=None,
             )
@@ -692,7 +693,7 @@ def commit(
                 commit_timezone=commit_timezone,
                 encoding=encoding,
                 no_verify=no_verify,
-                sign=bool(signoff),
+                sign=sign,
                 merge_heads=merge_heads,
             )
 
@@ -1960,7 +1961,7 @@ def tag_create(
     objectish: Union[str, bytes] = "HEAD",
     tag_time: Optional[int] = None,
     tag_timezone: Optional[int] = None,
-    sign: bool = False,
+    sign: Optional[bool] = None,
     encoding: str = DEFAULT_ENCODING,
 ) -> None:
     """Creates a tag in git via dulwich calls.
@@ -2014,24 +2015,27 @@ def tag_create(
             tag_obj.tag_timezone = tag_timezone
 
             # Check if we should sign the tag
-            should_sign = sign
+            config = r.get_config_stack()
+
             if sign is None:
                 # Check tag.gpgSign configuration when sign is not explicitly set
-                config = r.get_config_stack()
                 try:
-                    should_sign = config.get_boolean((b"tag",), b"gpgSign")
+                    should_sign = config.get_boolean(
+                        (b"tag",), b"gpgsign", default=False
+                    )
                 except KeyError:
                     should_sign = False  # Default to not signing if no config
+            else:
+                should_sign = sign
+
+            # Get the signing key from config if signing is enabled
+            keyid = None
             if should_sign:
-                keyid = sign if isinstance(sign, str) else None
-                # If sign is True but no keyid specified, check user.signingKey config
-                if should_sign is True and keyid is None:
-                    config = r.get_config_stack()
-                    try:
-                        keyid = config.get((b"user",), b"signingKey").decode("ascii")
-                    except KeyError:
-                        # No user.signingKey configured, will use default GPG key
-                        pass
+                try:
+                    keyid_bytes = config.get((b"user",), b"signingkey")
+                    keyid = keyid_bytes.decode() if keyid_bytes else None
+                except KeyError:
+                    keyid = None
                 tag_obj.sign(keyid)
 
             r.object_store.add_object(tag_obj)
@@ -2042,6 +2046,60 @@ def tag_create(
         r.refs[_make_tag_ref(tag)] = tag_id
 
 
+def verify_commit(
+    repo: RepoPath,
+    committish: Union[str, bytes] = "HEAD",
+    keyids: Optional[list[str]] = None,
+) -> None:
+    """Verify GPG signature on a commit.
+
+    Args:
+      repo: Path to repository
+      committish: Commit to verify (defaults to HEAD)
+      keyids: Optional list of trusted key IDs. If provided, the commit
+        must be signed by one of these keys. If not provided, just verifies
+        that the commit has a valid signature.
+
+    Raises:
+      gpg.errors.BadSignatures: if GPG signature verification fails
+      gpg.errors.MissingSignatures: if commit was not signed by a key
+        specified in keyids
+    """
+    with open_repo_closing(repo) as r:
+        commit = parse_commit(r, committish)
+        commit.verify(keyids)
+
+
+def verify_tag(
+    repo: RepoPath,
+    tagname: Union[str, bytes],
+    keyids: Optional[list[str]] = None,
+) -> None:
+    """Verify GPG signature on a tag.
+
+    Args:
+      repo: Path to repository
+      tagname: Name of tag to verify
+      keyids: Optional list of trusted key IDs. If provided, the tag
+        must be signed by one of these keys. If not provided, just verifies
+        that the tag has a valid signature.
+
+    Raises:
+      gpg.errors.BadSignatures: if GPG signature verification fails
+      gpg.errors.MissingSignatures: if tag was not signed by a key
+        specified in keyids
+    """
+    with open_repo_closing(repo) as r:
+        if isinstance(tagname, str):
+            tagname = tagname.encode()
+        tag_ref = _make_tag_ref(tagname)
+        tag_id = r.refs[tag_ref]
+        tag_obj = r[tag_id]
+        if not isinstance(tag_obj, Tag):
+            raise Error(f"{tagname!r} does not point to a tag object")
+        tag_obj.verify(keyids)
+
+
 def tag_list(repo: RepoPath, outstream: TextIO = sys.stdout) -> list[bytes]:
     """List all tags.
 

dulwich/worktree.py

@@ -413,7 +413,7 @@ class WorkTree:
         ref: Ref | None = b"HEAD",
         merge_heads: Sequence[ObjectID] | None = None,
         no_verify: bool = False,
-        sign: bool = False,
+        sign: bool | None = None,
     ) -> ObjectID:
         """Create a new commit.
 
@@ -501,15 +501,25 @@ class WorkTree:
         message = None  # Will be set later after parents are set
 
         # Check if we should sign the commit
-        should_sign = sign
         if sign is None:
             # Check commit.gpgSign configuration when sign is not explicitly set
-            config = self._repo.get_config_stack()
             try:
-                should_sign = config.get_boolean((b"commit",), b"gpgSign")
+                should_sign = config.get_boolean(
+                    (b"commit",), b"gpgsign", default=False
+                )
             except KeyError:
                 should_sign = False  # Default to not signing if no config
-        keyid = sign if isinstance(sign, str) else None
+        else:
+            should_sign = sign
+
+        # Get the signing key from config if signing is enabled
+        keyid = None
+        if should_sign:
+            try:
+                keyid_bytes = config.get((b"user",), b"signingkey")
+                keyid = keyid_bytes.decode() if keyid_bytes else None
+            except KeyError:
+                keyid = None
 
         if ref is None:
             # Create a dangling commit

tests/compat/test_porcelain.py

@@ -109,7 +109,7 @@ class CommitCreateSignTestCase(PorcelainGpgTestCase, CompatTestCase):
             self.repo.path,
             b"messy message messiah",
             b"foo <foo@b.ar>",
-            signoff=True,
+            sign=True,
         )
 
         run_git_or_fail(

tests/test_porcelain.py

@@ -625,7 +625,7 @@ class CommitSignTests(PorcelainGpgTestCase):
             message="Some message",
             author="Joe <joe@example.com>",
             committer="Bob <bob@example.com>",
-            signoff=True,
+            sign=True,
         )
         self.assertIsInstance(sha, bytes)
         self.assertEqual(len(sha), 40)
@@ -694,7 +694,7 @@ class CommitSignTests(PorcelainGpgTestCase):
             message="Signed with configured key",
             author="Joe <joe@example.com>",
             committer="Bob <bob@example.com>",
-            signoff=True,  # This should read user.signingKey from config
+            sign=True,  # This should read user.signingKey from config
         )
 
         self.assertIsInstance(sha, bytes)
@@ -809,7 +809,7 @@ class CommitSignTests(PorcelainGpgTestCase):
         )
         self.repo.refs[b"HEAD"] = c3.id
 
-        # Set up commit.gpgSign=false but explicitly pass signoff=True
+        # Set up commit.gpgSign=false but explicitly pass sign=True
         cfg = self.repo.get_config()
         cfg.set(("user",), "signingKey", PorcelainGpgTestCase.DEFAULT_KEY_ID)
         cfg.set(("commit",), "gpgSign", False)
@@ -817,13 +817,13 @@ class CommitSignTests(PorcelainGpgTestCase):
 
         self.import_default_key()
 
-        # Create commit with explicit signoff=True (should override config)
+        # Create commit with explicit sign=True (should override config)
         sha = porcelain.commit(
             self.repo.path,
             message="Explicitly signed commit",
             author="Joe <joe@example.com>",
             committer="Bob <bob@example.com>",
-            signoff=True,  # This should override commit.gpgSign=false
+            sign=True,  # This should override commit.gpgSign=false
         )
 
         self.assertIsInstance(sha, bytes)
@@ -842,7 +842,7 @@ class CommitSignTests(PorcelainGpgTestCase):
         )
         self.repo.refs[b"HEAD"] = c3.id
 
-        # Set up commit.gpgSign=true but explicitly pass signoff=False
+        # Set up commit.gpgSign=true but explicitly pass sign=False
         cfg = self.repo.get_config()
         cfg.set(("user",), "signingKey", PorcelainGpgTestCase.DEFAULT_KEY_ID)
         cfg.set(("commit",), "gpgSign", True)
@@ -850,13 +850,13 @@ class CommitSignTests(PorcelainGpgTestCase):
 
         self.import_default_key()
 
-        # Create commit with explicit signoff=False (should disable signing)
+        # Create commit with explicit sign=False (should disable signing)
         sha = porcelain.commit(
             self.repo.path,
             message="Explicitly unsigned commit",
             author="Joe <joe@example.com>",
             committer="Bob <bob@example.com>",
-            signoff=False,  # This should override commit.gpgSign=true
+            sign=False,  # This should override commit.gpgSign=true
         )
 
         self.assertIsInstance(sha, bytes)
@@ -868,6 +868,165 @@ class CommitSignTests(PorcelainGpgTestCase):
         self.assertIsNone(commit._gpgsig)
 
 
+@skipIf(
+    platform.python_implementation() == "PyPy" or sys.platform == "win32",
+    "gpgme not easily available or supported on Windows and PyPy",
+)
+class VerifyCommitTests(PorcelainGpgTestCase):
+    def test_verify_commit_valid_signature(self) -> None:
+        """Test verifying a commit with a valid GPG signature."""
+        _c1, _c2, c3 = build_commit_graph(
+            self.repo.object_store, [[1], [2, 1], [3, 1, 2]]
+        )
+        self.repo.refs[b"HEAD"] = c3.id
+        cfg = self.repo.get_config()
+        cfg.set(("user",), "signingKey", PorcelainGpgTestCase.DEFAULT_KEY_ID)
+        cfg.write_to_path()
+        self.import_default_key()
+
+        # Create a signed commit
+        sha = porcelain.commit(
+            self.repo.path,
+            message="Signed commit",
+            author="Joe <joe@example.com>",
+            committer="Bob <bob@example.com>",
+            sign=True,
+        )
+
+        # Verify should not raise
+        porcelain.verify_commit(self.repo.path, sha)
+        porcelain.verify_commit(
+            self.repo.path, sha, keyids=[PorcelainGpgTestCase.DEFAULT_KEY_ID]
+        )
+
+    def test_verify_commit_with_wrong_key(self) -> None:
+        """Test that verifying with wrong keyid raises MissingSignatures."""
+        _c1, _c2, c3 = build_commit_graph(
+            self.repo.object_store, [[1], [2, 1], [3, 1, 2]]
+        )
+        self.repo.refs[b"HEAD"] = c3.id
+        cfg = self.repo.get_config()
+        cfg.set(("user",), "signingKey", PorcelainGpgTestCase.DEFAULT_KEY_ID)
+        cfg.write_to_path()
+        self.import_default_key()
+
+        sha = porcelain.commit(
+            self.repo.path,
+            message="Signed commit",
+            author="Joe <joe@example.com>",
+            committer="Bob <bob@example.com>",
+            sign=True,
+        )
+
+        self.import_non_default_key()
+        self.assertRaises(
+            gpg.errors.MissingSignatures,
+            porcelain.verify_commit,
+            self.repo.path,
+            sha,
+            keyids=[PorcelainGpgTestCase.NON_DEFAULT_KEY_ID],
+        )
+
+    def test_verify_commit_unsigned(self) -> None:
+        """Test that verifying an unsigned commit succeeds (no signature to verify)."""
+        _c1, _c2, c3 = build_commit_graph(
+            self.repo.object_store, [[1], [2, 1], [3, 1, 2]]
+        )
+        self.repo.refs[b"HEAD"] = c3.id
+
+        sha = porcelain.commit(
+            self.repo.path,
+            message="Unsigned commit",
+            author="Joe <joe@example.com>",
+            committer="Bob <bob@example.com>",
+            sign=False,
+        )
+
+        # Verify should not raise for unsigned commits
+        porcelain.verify_commit(self.repo.path, sha)
+
+
+@skipIf(
+    platform.python_implementation() == "PyPy" or sys.platform == "win32",
+    "gpgme not easily available or supported on Windows and PyPy",
+)
+class VerifyTagTests(PorcelainGpgTestCase):
+    def test_verify_tag_valid_signature(self) -> None:
+        """Test verifying a tag with a valid GPG signature."""
+        _c1, _c2, c3 = build_commit_graph(
+            self.repo.object_store, [[1], [2, 1], [3, 1, 2]]
+        )
+        self.repo.refs[b"HEAD"] = c3.id
+        cfg = self.repo.get_config()
+        cfg.set(("user",), "signingKey", PorcelainGpgTestCase.DEFAULT_KEY_ID)
+        cfg.write_to_path()
+        self.import_default_key()
+
+        # Create a signed tag
+        porcelain.tag_create(
+            self.repo.path,
+            b"signed-tag",
+            b"Tagger <tagger@example.com>",
+            b"Signed tag message",
+            annotated=True,
+            sign=True,
+        )
+
+        # Verify should not raise
+        porcelain.verify_tag(self.repo.path, b"signed-tag")
+        porcelain.verify_tag(
+            self.repo.path, b"signed-tag", keyids=[PorcelainGpgTestCase.DEFAULT_KEY_ID]
+        )
+
+    def test_verify_tag_with_wrong_key(self) -> None:
+        """Test that verifying with wrong keyid raises MissingSignatures."""
+        _c1, _c2, c3 = build_commit_graph(
+            self.repo.object_store, [[1], [2, 1], [3, 1, 2]]
+        )
+        self.repo.refs[b"HEAD"] = c3.id
+        cfg = self.repo.get_config()
+        cfg.set(("user",), "signingKey", PorcelainGpgTestCase.DEFAULT_KEY_ID)
+        cfg.write_to_path()
+        self.import_default_key()
+
+        porcelain.tag_create(
+            self.repo.path,
+            b"signed-tag",
+            b"Tagger <tagger@example.com>",
+            b"Signed tag message",
+            annotated=True,
+            sign=True,
+        )
+
+        self.import_non_default_key()
+        self.assertRaises(
+            gpg.errors.MissingSignatures,
+            porcelain.verify_tag,
+            self.repo.path,
+            b"signed-tag",
+            keyids=[PorcelainGpgTestCase.NON_DEFAULT_KEY_ID],
+        )
+
+    def test_verify_tag_unsigned(self) -> None:
+        """Test that verifying an unsigned tag succeeds (no signature to verify)."""
+        _c1, _c2, c3 = build_commit_graph(
+            self.repo.object_store, [[1], [2, 1], [3, 1, 2]]
+        )
+        self.repo.refs[b"HEAD"] = c3.id
+
+        porcelain.tag_create(
+            self.repo.path,
+            b"unsigned-tag",
+            b"Tagger <tagger@example.com>",
+            b"Unsigned tag message",
+            annotated=True,
+            sign=False,
+        )
+
+        # Verify should not raise for unsigned tags
+        porcelain.verify_tag(self.repo.path, b"unsigned-tag")
+
+
 class TimezoneTests(PorcelainTestCase):
     def put_envs(self, value) -> None:
         self.overrideEnv("GIT_AUTHOR_DATE", value)