From 65f41bd218a2e35297263e25067975d13e698edf Mon Sep 17 00:00:00 2001 From: Nicolas Schier Date: Thu, 27 Aug 2020 14:43:50 +0200 Subject: [PATCH] [gpgsign-at-last] Add support for GPG-signed commits (#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 that 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 considered a modification, which seems consistent. This logic is applied to all commits in the todo-list for more consistency. The test is a bit large, but the pieces are mostly independenty. I'm not sure if it's worth to have a separate GPG secret key initialization for each test, and I don't know how to share that. Closes # 46 Closes # 73 Co-authored-by: Johannes Altmanninger Signed-off-by: Nicolas Schier --- CHANGELOG.md | 1 + docs/man.rst | 12 +++++ git-revise.1 | 14 ++++++ gitrevise/odb.py | 102 ++++++++++++++++++++++++++++++++++++------ gitrevise/tui.py | 29 +++++++++++- tests/test_gpgsign.py | 77 +++++++++++++++++++++++++++++++ 6 files changed, 220 insertions(+), 15 deletions(-) create mode 100644 tests/test_gpgsign.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dd787f..b8f1932 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/docs/man.rst b/docs/man.rst index e47e285..ca9cfb6 100644 --- a/docs/man.rst +++ b/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 =================== diff --git a/git-revise.1 b/git-revise.1 index fede168..ea6f82e 100644 --- a/git-revise.1 +++ b/git-revise.1 @@ -79,6 +79,12 @@ Reset target commit\(aqs author to the current user. .B \-\-ref 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 diff --git a/gitrevise/odb.py b/gitrevise/odb.py index 8e7b315..c0b68e9 100644 --- a/gitrevise/odb.py +++ b/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[^<>]+)<(?P[^<>]+)>[ ] + (?P + (?P[^<>]+)<(?P[^<>]+)> + ) + [ ] (?P[0-9]+) (?:[ ](?P[\+\-][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 """ + 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: diff --git a/gitrevise/tui.py b/gitrevise/tui.py index 8734522..fc21d89 100644 --- a/gitrevise/tui.py +++ b/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: diff --git a/tests/test_gpgsign.py b/tests/test_gpgsign.py new file mode 100644 index 0000000..beeff7e --- /dev/null +++ b/tests/test_gpgsign.py @@ -0,0 +1,77 @@ +# pylint: skip-file + +from conftest import * +from subprocess import CalledProcessError + + +def test_gpgsign(repo): + bash("git commit --allow-empty -m 'commit 1'") + assert repo.get_commit("HEAD").gpgsig is None + + bash( + """ + mkdir ~/.gnupg/ + cat > ~/.gnupg/gpg.conf << EOF + pinentry-mode loopback + EOF + + name=$(git config user.name) + email=$(git config user.email) + gpg --batch --passphrase "" --quick-gen-key "$name <$email>" >/dev/null 2>&1 + """ + ) + + bash("git config commit.gpgSign true") + main(["HEAD"]) + assert ( + repo.get_commit("HEAD").gpgsig is not None + ), "git config commit.gpgSign activates GPG signing" + + bash("git config revise.gpgSign false") + main(["HEAD"]) + assert ( + repo.get_commit("HEAD").gpgsig is None + ), "git config revise.gpgSign overrides commit.gpgSign" + + main(["HEAD", "--gpg-sign"]) + assert ( + repo.get_commit("HEAD").gpgsig is not None + ), "commandline option overrides configuration" + + main(["HEAD", "--no-gpg-sign"]) + assert repo.get_commit("HEAD").gpgsig is None, "long option" + + main(["HEAD", "-S"]) + assert repo.get_commit("HEAD").gpgsig is not None, "short option" + + bash("git config gpg.program false") + try: + main(["HEAD", "--gpg-sign"]) + assert False, "Overridden gpg.program should fail" + except CalledProcessError: + pass + bash("git config --unset gpg.program") + + # Check that we can sign multiple commits. + bash( + """ + git -c commit.gpgSign=false commit --allow-empty -m 'commit 2' + git -c commit.gpgSign=false commit --allow-empty -m 'commit 3' + git -c commit.gpgSign=false commit --allow-empty -m 'commit 4' + """ + ) + main(["HEAD~~", "--gpg-sign"]) + assert repo.get_commit("HEAD~~").gpgsig is not None + assert repo.get_commit("HEAD~").gpgsig is not None + assert repo.get_commit("HEAD").gpgsig is not None + + # Check that we can remove signatures from multiple commits. + main(["HEAD~", "--no-gpg-sign"]) + assert repo.get_commit("HEAD~").gpgsig is None + assert repo.get_commit("HEAD").gpgsig is None + + # Check that we add signatures, even if the target commit already has one. + assert repo.get_commit("HEAD~~").gpgsig is not None + main(["HEAD~~", "--gpg-sign"]) + assert repo.get_commit("HEAD~").gpgsig is not None + assert repo.get_commit("HEAD").gpgsig is not None