Add support for GPG-signed commits (mystor#46)

GPG-sign commits if option -S/--gpg-sign is given, or if configurations
revise.gpgSign or commit.gpgSign are set to true.

Unlike Git, we do not support passing the key ID as argument.  Git's
way of accepting this optional argument is a bit weird.  It would be
awkward to try to support the same with argparse.  So it's probably
best to use a separate argument for the key ID in future.

For now, the key ID is taken from Git configuration (user.signingKey)
or the committer signature as fallback.

A call of `git-revise COMMIT --gpg-sign` w/o any changes in the index
causes creation of GPG commit signatures for all commits since COMMIT.

Conversely, if GPG signing is disabled, git-revise will remove
the signature, even if the commits were not modified otherwise.
So adding/removing a signature is just another modification, like
rewording/rebasing. This logic is applied to all commits in the
todo-list.

The test is a bit large, but the pieces are mostly independen.  Maybe
we should share the GPG initialization somehow?

Closes mystor#46
Closes mystor#73

Co-authored-by: Johannes Altmanninger <aclopte@gmail.com>
Signed-off-by: Nicolas Schier <nicolas@fjasle.eu>

CHANGELOG.md

@@ -6,6 +6,7 @@
 * Fix autosquash order of multiple fixup commits with the same target (#72)
 * Use `GIT_SEQUENCE_EDITOR` instead of `SEQUENCE_EDITOR` (#71)
 * Fix handling of multiline commit subjects (#86)
+* Add support for `commit.gpgSign` (#46)
 
 ## v0.6.0
 

docs/man.rst

@@ -58,6 +58,11 @@ General options
 
    Working branch to update; defaults to ``HEAD``.
 
+.. option:: -S, --gpg-sign, --no-gpg-sign
+
+   GPG-sign commits.  Overrides both the ``commit.gpgSign`` and
+   ``revise.gpgSign`` git configurations.
+
 Main modes of operation
 -----------------------
 
@@ -126,6 +131,13 @@ Configuration is managed by :manpage:`git-config(1)`.
    is specified. Overridden by :option:`--no-autosquash`. Defaults to false. If
    not set, the value of ``rebase.autoSquash`` is used instead.
 
+.. gitconfig:: revise.gpgSign
+
+   If set to true, GPG-sign new commits; defaults to false.  This setting
+   overrides the original git configuration ``commit.gpgSign`` and may be
+   overridden by the command line options ``--gpg-sign`` and
+   ``--no-gpg-sign``.
+
 
 CONFLICT RESOLUTION
 ===================

git-revise.1

@@ -79,6 +79,12 @@ Reset target commit\(aqs author to the current user.
 .B \-\-ref <gitref>
 Working branch to update; defaults to \fBHEAD\fP\&.
 .UNINDENT
+.INDENT 0.0
+.TP
+.B \-S, \-\-gpg\-sign, \-\-no\-gpg\-sign
+GPG\-sign commits.  Overrides both the \fBcommit.gpgSign\fP and
+\fBrevise.gpgSign\fP git configurations.
+.UNINDENT
 .SS Main modes of operation
 .INDENT 0.0
 .TP
@@ -147,6 +153,14 @@ If set to true, imply \fI\%\-\-autosquash\fP whenever \fI\%\-\-interactive\fP
 is specified. Overridden by \fI\%\-\-no\-autosquash\fP\&. Defaults to false. If
 not set, the value of \fBrebase.autoSquash\fP is used instead.
 .UNINDENT
+.INDENT 0.0
+.TP
+.B revise.gpgSign
+If set to true, GPG\-sign new commits; defaults to false.  This setting
+overrides the original git configuration \fBcommit.gpgSign\fP and may be
+overridden by the command line options \fB\-\-gpg\-sign\fP and
+\fB\-\-no\-gpg\-sign\fP\&.
+.UNINDENT
 .SH CONFLICT RESOLUTION
 .sp
 When a conflict is encountered, \fBgit revise\fP will attempt to resolve

gitrevise/odb.py

@@ -17,6 +17,7 @@
     Tuple,
     cast,
 )
+import sys
 from types import TracebackType
 from pathlib import Path
 from enum import Enum
@@ -32,6 +33,13 @@ def __init__(self, ref: str) -> None:
         Exception.__init__(self, f"Object {ref} does not exist")
 
 
+class GPGSignError(Exception):
+    """Exception raised when we fail to sign a commit"""
+
+    def __init__(self, stderr: str) -> None:
+        Exception.__init__(self, f"unable to sign object: {stderr}")
+
+
 T = TypeVar("T")  # pylint: disable=invalid-name
 
 
@@ -80,7 +88,10 @@ class Signature(bytes):
 
     sig_re = re.compile(
         rb"""
-        (?P<name>[^<>]+)<(?P<email>[^<>]+)>[ ]
+        (?P<signing_key>
+            (?P<name>[^<>]+)<(?P<email>[^<>]+)>
+        )
+        [ ]
         (?P<timestamp>[0-9]+)
         (?:[ ](?P<offset>[\+\-][0-9]+))?
         """,
@@ -101,6 +112,13 @@ def email(self) -> bytes:
         assert match, "invalid signature"
         return match.group("email").strip()
 
+    @property
+    def signing_key(self) -> bytes:
+        """user name <email>"""
+        match = self.sig_re.fullmatch(self)
+        assert match, "invalid signature"
+        return match.group("signing_key").strip()
+
     @property
     def timestamp(self) -> bytes:
         """unix timestamp"""
@@ -134,6 +152,12 @@ class Repository:
     index: "Index"
     """current index state"""
 
+    sign_commits: bool
+    """sign commits with gpg"""
+
+    gpg: bytes
+    """path to GnuPG binary"""
+
     _objects: Dict[int, Dict[Oid, "GitObj"]]
     _catfile: Popen
     _tempdir: Optional[TemporaryDirectory]
@@ -144,6 +168,8 @@ class Repository:
         "default_author",
         "default_committer",
         "index",
+        "sign_commits",
+        "gpg",
         "_objects",
         "_catfile",
         "_tempdir",
@@ -162,6 +188,12 @@ def __init__(self, cwd: Optional[Path] = None) -> None:
 
         self.index = Index(self)
 
+        self.sign_commits = self.bool_config(
+            "revise.gpgSign", default=self.bool_config("commit.gpgSign", default=False)
+        )
+
+        self.gpg = self.config("gpg.program", default=b"gpg")
+
         # Pylint 2.8 emits a false positive; fixed in 2.9.
         self._catfile = Popen(  # pylint: disable=consider-using-with
             ["git", "cat-file", "--batch"],
@@ -274,10 +306,43 @@ def new_commit(
             body += b"parent " + parent.oid.hex().encode() + b"\n"
         body += b"author " + author + b"\n"
         body += b"committer " + committer + b"\n"
-        body += b"\n"
-        body += message
+
+        body_tail = b"\n" + message
+        body += self.sign_buffer(body + body_tail)
+        body += body_tail
+
         return Commit(self, body)
 
+    def sign_buffer(self, buffer: bytes) -> bytes:
+        """Return the text of the signed commit object."""
+        if not self.sign_commits:
+            return b""
+
+        key_id = self.config(
+            "user.signingKey", default=self.default_committer.signing_key
+        )
+        gpg = None
+        try:
+            gpg = run(
+                (self.gpg, "--status-fd=2", "-bsau", key_id),
+                stdout=PIPE,
+                stderr=PIPE,
+                input=buffer,
+                check=True,
+            )
+        except CalledProcessError as gpg:
+            print(gpg.stderr.decode(), file=sys.stderr, end="")
+            print("gpg failed to sign commit", file=sys.stderr)
+            raise
+
+        if b"\n[GNUPG:] SIG_CREATED " not in gpg.stderr:
+            raise GPGSignError(gpg.stderr.decode())
+
+        signature = b"gpgsig"
+        for line in gpg.stdout.splitlines():
+            signature += b" " + line + b"\n"
+        return signature
+
     def new_tree(self, entries: Mapping[bytes, "Entry"]) -> "Tree":
         """Directly create an in-memory tree object, without persisting it.
         If a tree object with these entries already exists, it will be
@@ -476,10 +541,13 @@ class Commit(GitObj):
     committer: Signature
     """:class:`Signature` of this commit's committer"""
 
+    gpgsig: Optional[bytes]
+    """GPG signature of this commit"""
+
     message: bytes
     """Body of this commit's message"""
 
-    __slots__ = ("tree_oid", "parent_oids", "author", "committer", "message")
+    __slots__ = ("tree_oid", "parent_oids", "author", "committer", "gpgsig", "message")
 
     def _parse_body(self) -> None:
         # Split the header from the core commit message.
@@ -493,6 +561,7 @@ def _parse_body(self) -> None:
             key, value = hdr.split(maxsplit=1)
             value = value.replace(b"\n ", b"\n")
 
+            self.gpgsig = None
             if key == b"tree":
                 self.tree_oid = Oid.fromhex(value.decode())
             elif key == b"parent":
@@ -501,6 +570,8 @@ def _parse_body(self) -> None:
                 self.author = Signature(value)
             elif key == b"committer":
                 self.committer = Signature(value)
+            elif key == b"gpgsig":
+                self.gpgsig = value
 
     def tree(self) -> "Tree":
         """``tree`` object corresponding to this commit"""
@@ -539,6 +610,7 @@ def update(
         parents: Optional[Sequence["Commit"]] = None,
         message: Optional[bytes] = None,
         author: Optional[Signature] = None,
+        recommit: bool = False,
     ) -> "Commit":
         """Create a new commit with specific properties updated or replaced"""
         # Compute parameters used to create the new object.
@@ -551,16 +623,18 @@ def update(
         if author is None:
             author = self.author
 
-        # Check if the commit was unchanged to avoid creating a new commit if
-        # only the committer has changed.
-        unchanged = (
-            tree == self.tree()
-            and parents == self.parents()
-            and message == self.message
-            and author == self.author
-        )
-        if unchanged:
-            return self
+        if not recommit:
+            # Check if the commit was unchanged to avoid creating a new commit if
+            # only the committer has changed.
+            unchanged = (
+                tree == self.tree()
+                and parents == self.parents()
+                and message == self.message
+                and author == self.author
+            )
+            if unchanged:
+                return self
+
         return self.repo.new_commit(tree, parents, message, author)
 
     def _persist_deps(self) -> None:

gitrevise/tui.py

@@ -88,6 +88,19 @@ def build_parser() -> ArgumentParser:
         action="store_true",
         help="interactively cut a commit into two smaller commits",
     )
+
+    gpg_group = parser.add_mutually_exclusive_group()
+    gpg_group.add_argument(
+        "--gpg-sign",
+        "-S",
+        action="store_true",
+        help="GPG sign commits",
+    )
+    gpg_group.add_argument(
+        "--no-gpg-sign",
+        action="store_true",
+        help="do not GPG sign commits",
+    )
     return parser
 
 
@@ -172,11 +185,20 @@ def noninteractive(
     if args.cut:
         current = cut_commit(current)
 
-    if current != replaced:
+    # Add or remove GPG signatures.
+    if repo.sign_commits != bool(current.gpgsig):
+        current = current.update(recommit=True)
+    change_signature = any(
+        repo.sign_commits != bool(commit.gpgsig) for commit in to_rebase
+    )
+
+    if current != replaced or change_signature:
         print(f"{current.oid.short()} {current.summary()}")
 
         # Rebase commits atop the commit range.
         for commit in to_rebase:
+            if repo.sign_commits != bool(commit.gpgsig):
+                commit = commit.update(recommit=True)
             current = commit.rebase(current)
             print(f"{current.oid.short()} {current.summary()}")
 
@@ -192,6 +214,11 @@ def inner_main(args: Namespace, repo: Repository) -> None:
     if args.patch:
         repo.git("add", "-p")
 
+    if args.gpg_sign:
+        repo.sign_commits = True
+    if args.no_gpg_sign:
+        repo.sign_commits = False
+
     # Create a commit with changes from the index
     staged = None
     if not args.no_index:

tests/conftest.py

@@ -4,7 +4,9 @@
 import shutil
 import shlex
 import os
+import py.path
 import sys
+import tempfile
 import textwrap
 import subprocess
 import traceback
@@ -76,6 +78,12 @@ def repo(hermetic_seal):
         yield repo
 
 
+@pytest.fixture
+def short_tmpdir():
+    with tempfile.TemporaryDirectory() as tdir:
+        yield py.path.local(tdir)
+
+
 @contextmanager
 def in_parallel(func, *args, **kwargs):
     class HelperThread(Thread):