Repository: libgit2/git2go
Pull request #626 (2020-08-12 08:59:47)

Add support for creating signed commits and signing commits during a rebase
This all works but I have a few questions about submitting it:

- I'm targeting this at maser but due to package versions available in other distros I'm guessing I will still have to maintain a fork if I want this to be able to use with v29 and below (eg, pre-1.0 which a lot of distros don't have)?
- There's no dependencies in the go.mod file, but I obviously had to add a dependency on the x/crypto module to add unit tests, note sure if that's going to be a problem?
- In libgit2 `git_commit_create_with_signature` is in the 'repository' block of functions in the documentation, but practically it's very difficult to use like that (you would need to look up the commit you were using to get the commit content anyway) so I added it to the `Commit` struct instead. The other sensible way would possibly be to add it as a method on the `Repository` struct, make it take a commit ID, and make it looks up the commit to read the content, but then it's just a very roundabout way of doing it
- The `RebaseOptions.fromC` does not load the function pointer for `git_commit_signing_cb` (there is a note about it), but I don't know if that use case would ever come up. If it was, there would need to be another bridge function in C that called that function pointer.
- I wasn't really sure on how to actually use the commit that you get from `git_commit_create_with_signature` and I can't find any examples, so what I did in the tests (and what I would probably do if I was using this in the real world) is to create the signed commit, then just re-point head at that signed commit.
- I added a utility method `WithSignatureUsing` to the `Commit` to allow passing a callback to create the signature for the commit, this isn't a 1:1 mapping with the libgit2 api obviously but I figure it's what 99% of people using this library will write themselves anyway

mbfr: It looks like the tests are failing because of the module dependencies, which I suppose is why there were no dependencies there already. 1.9 hasn't had any releases for over 2 years and 1.10 hasn't had a release for a year and a half, is there any reason they're still in the CI? If they do have to be supported then I'm open to suggestions on how to write tests for this functionality without adding any dependencies

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 88e6c57..42636bc 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -35,7 +35,7 @@ jobs:
       run: |
         git submodule update --init
         make build-libgit2-static
-        go get -tags static github.com/${{ github.repository }}/...
+        go get -tags static -t github.com/${{ github.repository }}/...
         go build -tags static github.com/${{ github.repository }}/...
     - name: Test
       env:
diff --git a/commit.go b/commit.go
index 4262060..1c546b3 100644
--- a/commit.go
+++ b/commit.go
@@ -40,6 +40,69 @@ func (c *Commit) RawMessage() string {
 	return ret
 }
 
+// RawHeader gets the full raw text of the commit header.
+func (c *Commit) RawHeader() string {
+	ret := C.GoString(C.git_commit_raw_header(c.cast_ptr))
+	runtime.KeepAlive(c)
+	return ret
+}
+
+// ContentToSign returns the content that will be passed to a signing function for this commit
+func (c *Commit) ContentToSign() string {
+	return c.RawHeader() + "\n" + c.RawMessage()
+}
+
+// CommitSigningCallback defines a function type that takes some data to sign and returns (signature, signature_field, error)
+type CommitSigningCallback func(string) (signature, signatureField string, err error)
+
+// WithSignatureUsing creates a new signed commit from this one using the given signing callback
+func (c *Commit) WithSignatureUsing(f CommitSigningCallback) (*Oid, error) {
+	signature, signatureField, err := f(c.ContentToSign())
+	if err != nil {
+		return nil, err
+	}
+
+	return c.WithSignature(signature, signatureField)
+}
+
+// WithSignature creates a new signed commit from the given signature and signature field
+func (c *Commit) WithSignature(signature string, signatureField string) (*Oid, error) {
+	totalCommit := c.ContentToSign()
+
+	oid := new(Oid)
+
+	var csf *C.char = nil
+	if signatureField != "" {
+		csf = C.CString(signatureField)
+		defer C.free(unsafe.Pointer(csf))
+	}
+
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	cTotalCommit := C.CString(totalCommit)
+	cSignature := C.CString(signature)
+	defer C.free(unsafe.Pointer(cTotalCommit))
+	defer C.free(unsafe.Pointer(cSignature))
+
+	ret := C.git_commit_create_with_signature(
+		oid.toC(),
+		c.Owner().ptr,
+		cTotalCommit,
+		cSignature,
+		csf,
+	)
+
+	runtime.KeepAlive(c)
+	runtime.KeepAlive(oid)
+
+	if ret < 0 {
+		return nil, MakeGitError(ret)
+	}
+
+	return oid, nil
+}
+
 func (c *Commit) ExtractSignature() (string, string, error) {
 
 	var c_signed C.git_buf
diff --git a/go.mod b/go.mod
index ee2b6e7..c190305 100644
--- a/go.mod
+++ b/go.mod
@@ -1,3 +1,5 @@
 module github.com/libgit2/git2go/v30
 
 go 1.13
+
+require golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de
diff --git a/go.sum b/go.sum
new file mode 100644
index 0000000..1769e6b
--- /dev/null
+++ b/go.sum
@@ -0,0 +1,7 @@
+golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
+golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de h1:ikNHVSjEfnvz6sxdSPCaPt572qowuyMDMJLLm3Db3ig=
+golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
+golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
+golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
diff --git a/rebase.go b/rebase.go
index d29e183..d685f25 100644
--- a/rebase.go
+++ b/rebase.go
@@ -2,6 +2,8 @@ package git
 
 /*
 #include <git2.h>
+
+extern void _go_git_populate_commit_sign_cb(git_rebase_options *opts);
 */
 import "C"
 import (
@@ -69,14 +71,66 @@ func newRebaseOperationFromC(c *C.git_rebase_operation) *RebaseOperation {
 	return operation
 }
 
+//export commitSignCallback
+func commitSignCallback(_signature *C.git_buf, _signature_field *C.git_buf, _commit_content *C.char, _payload unsafe.Pointer) C.int {
+	opts, ok := pointerHandles.Get(_payload).(*RebaseOptions)
+	if !ok {
+		panic("invalid sign payload")
+	}
+
+	if opts.CommitSigningCallback == nil {
+		return C.GIT_PASSTHROUGH
+	}
+
+	commitContent := C.GoString(_commit_content)
+
+	signature, signatureField, err := opts.CommitSigningCallback(commitContent)
+	if err != nil {
+		if gitError, ok := err.(*GitError); ok {
+			return C.int(gitError.Code)
+		}
+		return C.int(-1)
+	}
+
+	fillBuf := func(bufData string, buf *C.git_buf) error {
+		clen := C.size_t(len(bufData))
+		cstr := unsafe.Pointer(C.CString(bufData))
+		defer C.free(cstr)
+
+		// libgit2 requires the contents of the buffer to be NULL-terminated.
+		// C.CString() guarantees that the returned buffer will be
+		// NULL-terminated, so we can safely copy the terminator.
+		if int(C.git_buf_set(buf, cstr, clen+1)) != 0 {
+			return errors.New("could not set buffer")
+		}
+
+		return nil
+	}
+
+	if signatureField != "" {
+		err := fillBuf(signatureField, _signature_field)
+		if err != nil {
+			return C.int(-1)
+		}
+	}
+
+	err = fillBuf(signature, _signature)
+	if err != nil {
+		return C.int(-1)
+	}
+
+	return C.GIT_OK
+}
+
 // RebaseOptions are used to tell the rebase machinery how to operate
 type RebaseOptions struct {
-	Version         uint
-	Quiet           int
-	InMemory        int
-	RewriteNotesRef string
-	MergeOptions    MergeOptions
-	CheckoutOptions CheckoutOpts
+	Version               uint
+	Quiet                 int
+	InMemory              int
+	RewriteNotesRef       string
+	MergeOptions          MergeOptions
+	CheckoutOptions       CheckoutOpts
+	CommitSigningCallback CommitSigningCallback
 }
 
 // DefaultRebaseOptions returns a RebaseOptions with default values.
@@ -108,7 +162,7 @@ func (ro *RebaseOptions) toC() *C.git_rebase_options {
 	if ro == nil {
 		return nil
 	}
-	return &C.git_rebase_options{
+	opts := &C.git_rebase_options{
 		version:           C.uint(ro.Version),
 		quiet:             C.int(ro.Quiet),
 		inmemory:          C.int(ro.InMemory),
@@ -116,6 +170,13 @@ func (ro *RebaseOptions) toC() *C.git_rebase_options {
 		merge_options:     *ro.MergeOptions.toC(),
 		checkout_options:  *ro.CheckoutOptions.toC(),
 	}
+
+	if ro.CommitSigningCallback != nil {
+		C._go_git_populate_commit_sign_cb(opts)
+		opts.payload = pointerHandles.Track(ro)
+	}
+
+	return opts
 }
 
 func mapEmptyStringToNull(ref string) *C.char {
diff --git a/wrapper.c b/wrapper.c
index 4308ae4..90b0e1e 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -12,6 +12,11 @@ void _go_git_populate_apply_cb(git_apply_options *options)
 	options->hunk_cb = (git_apply_hunk_cb)hunkApplyCallback;
 }
 
+void _go_git_populate_commit_sign_cb(git_rebase_options *opts)
+{
+	opts->signing_cb = (git_commit_signing_cb)commitSignCallback;
+}
+
 void _go_git_populate_remote_cb(git_clone_options *opts)
 {
 	opts->remote_cb = (git_remote_create_cb)remoteCreateCallback;


Test patch:
diff --git a/git_test.go b/git_test.go
index 807dcc2..91ade73 100644
--- a/git_test.go
+++ b/git_test.go
@@ -45,7 +45,16 @@ func createBareTestRepo(t *testing.T) *Repository {
 	return repo
 }
 
+// commitOpts contains any extra options for creating commits in the seed repo
+type commitOpts struct {
+	CommitSigningCallback
+}
+
 func seedTestRepo(t *testing.T, repo *Repository) (*Oid, *Oid) {
+	return seedTestRepoOpt(t, repo, commitOpts{})
+}
+
+func seedTestRepoOpt(t *testing.T, repo *Repository, opts commitOpts) (*Oid, *Oid) {
 	loc, err := time.LoadLocation("Europe/Berlin")
 	checkFatal(t, err)
 	sig := &Signature{
@@ -69,6 +78,28 @@ func seedTestRepo(t *testing.T, repo *Repository) (*Oid, *Oid) {
 	commitId, err := repo.CreateCommit("HEAD", sig, sig, message, tree)
 	checkFatal(t, err)
 
+	if opts.CommitSigningCallback != nil {
+		commit, err := repo.LookupCommit(commitId)
+		checkFatal(t, err)
+
+		signature, signatureField, err := opts.CommitSigningCallback(commit.ContentToSign())
+		checkFatal(t, err)
+
+		oid, err := commit.WithSignature(signature, signatureField)
+		checkFatal(t, err)
+		newCommit, err := repo.LookupCommit(oid)
+		checkFatal(t, err)
+		head, err := repo.Head()
+		checkFatal(t, err)
+		_, err = repo.References.Create(
+			head.Name(),
+			newCommit.Id(),
+			true,
+			"repoint to signed commit",
+		)
+		checkFatal(t, err)
+	}
+
 	return commitId, treeId
 }
 
diff --git a/rebase_test.go b/rebase_test.go
index ef4f920..a78e6c7 100644
--- a/rebase_test.go
+++ b/rebase_test.go
@@ -1,10 +1,15 @@
 package git
 
 import (
+	"bytes"
 	"errors"
 	"strconv"
+	"strings"
 	"testing"
 	"time"
+
+	"golang.org/x/crypto/openpgp"
+	"golang.org/x/crypto/openpgp/packet"
 )
 
 // Tests
@@ -33,12 +38,12 @@ func TestRebaseAbort(t *testing.T) {
 	seedTestRepo(t, repo)
 
 	// Setup a repo with 2 branches and a different tree
-	err := setupRepoForRebase(repo, masterCommit, branchName)
+	err := setupRepoForRebase(repo, masterCommit, branchName, commitOpts{})
 	checkFatal(t, err)
 
 	// Create several commits in emile
 	for _, commit := range emileCommits {
-		_, err = commitSomething(repo, commit, commit)
+		_, err = commitSomething(repo, commit, commit, commitOpts{})
 		checkFatal(t, err)
 	}
 
@@ -48,7 +53,7 @@ func TestRebaseAbort(t *testing.T) {
 	assertStringList(t, expectedHistory, actualHistory)
 
 	// Rebase onto master
-	rebase, err := performRebaseOnto(repo, "master")
+	rebase, err := performRebaseOnto(repo, "master", nil)
 	checkFatal(t, err)
 	defer rebase.Free()
 
@@ -94,17 +99,17 @@ func TestRebaseNoConflicts(t *testing.T) {
 	}
 
 	// Setup a repo with 2 branches and a different tree
-	err = setupRepoForRebase(repo, masterCommit, branchName)
+	err = setupRepoForRebase(repo, masterCommit, branchName, commitOpts{})
 	checkFatal(t, err)
 
 	// Create several commits in emile
 	for _, commit := range emileCommits {
-		_, err = commitSomething(repo, commit, commit)
+		_, err = commitSomething(repo, commit, commit, commitOpts{})
 		checkFatal(t, err)
 	}
 
 	// Rebase onto master
-	rebase, err := performRebaseOnto(repo, "master")
+	rebase, err := performRebaseOnto(repo, "master", nil)
 	checkFatal(t, err)
 	defer rebase.Free()
 
@@ -130,11 +135,127 @@ func TestRebaseNoConflicts(t *testing.T) {
 	actualHistory, err := commitMsgsList(repo)
 	checkFatal(t, err)
 	assertStringList(t, expectedHistory, actualHistory)
+}
+
+func TestRebaseGpgSigned(t *testing.T) {
+	// TEST DATA
+
+	entity, err := openpgp.NewEntity("Namey mcnameface", "test comment", "test@example.com", nil)
+	checkFatal(t, err)
+
+	opts, err := DefaultRebaseOptions()
+	checkFatal(t, err)
+
+	signCommitContent := func(commitContent string) (string, string, error) {
+		cipherText := new(bytes.Buffer)
+		err := openpgp.ArmoredDetachSignText(cipherText, entity, strings.NewReader(commitContent), &packet.Config{})
+		if err != nil {
+			return "", "", errors.New("error signing payload")
+		}
+
+		return cipherText.String(), "", nil
+	}
+	opts.CommitSigningCallback = signCommitContent
+
+	commitOpts := commitOpts{
+		CommitSigningCallback: signCommitContent,
+	}
+
+	// Inputs
+	branchName := "emile"
+	masterCommit := "something"
+	emileCommits := []string{
+		"fou",
+		"barre",
+		"ouich",
+	}
+
+	// Outputs
+	expectedHistory := []string{
+		"Test rebase, Baby! " + emileCommits[2],
+		"Test rebase, Baby! " + emileCommits[1],
+		"Test rebase, Baby! " + emileCommits[0],
+		"Test rebase, Baby! " + masterCommit,
+		"This is a commit\n",
+	}
+
+	// TEST
+	repo := createTestRepo(t)
+	defer cleanupTestRepo(t, repo)
+	seedTestRepoOpt(t, repo, commitOpts)
+
+	// Try to open existing rebase
+	_, err = repo.OpenRebase(nil)
+	if err == nil {
+		t.Fatal("Did not expect to find a rebase in progress")
+	}
 
+	// Setup a repo with 2 branches and a different tree
+	err = setupRepoForRebase(repo, masterCommit, branchName, commitOpts)
+	checkFatal(t, err)
+
+	// Create several commits in emile
+	for _, commit := range emileCommits {
+		_, err = commitSomething(repo, commit, commit, commitOpts)
+		checkFatal(t, err)
+	}
+
+	// Rebase onto master
+	rebase, err := performRebaseOnto(repo, "master", &opts)
+	checkFatal(t, err)
+	defer rebase.Free()
+
+	// Finish the rebase properly
+	err = rebase.Finish()
+	checkFatal(t, err)
+
+	// Check history is in correct order
+	actualHistory, err := commitMsgsList(repo)
+	checkFatal(t, err)
+	assertStringList(t, expectedHistory, actualHistory)
+
+	checkAllCommitsSigned(t, entity, repo)
+}
+
+func checkAllCommitsSigned(t *testing.T, entity *openpgp.Entity, repo *Repository) {
+	head, err := headCommit(repo)
+	checkFatal(t, err)
+	defer head.Free()
+
+	parent := head
+
+	err = checkCommitSigned(t, entity, parent)
+	checkFatal(t, err)
+
+	for parent.ParentCount() != 0 {
+		parent = parent.Parent(0)
+		defer parent.Free()
+
+		err = checkCommitSigned(t, entity, parent)
+		checkFatal(t, err)
+	}
+}
+
+func checkCommitSigned(t *testing.T, entity *openpgp.Entity, commit *Commit) error {
+	t.Helper()
+
+	signature, signedData, err := commit.ExtractSignature()
+	if err != nil {
+		t.Logf("No signature on commit\n%s", commit.ContentToSign())
+		return err
+	}
+
+	_, err = openpgp.CheckArmoredDetachedSignature(openpgp.EntityList{entity}, strings.NewReader(signedData), bytes.NewBufferString(signature))
+	if err != nil {
+		t.Logf("Commit is not signed correctly\n%s", commit.ContentToSign())
+		return err
+	}
+
+	return nil
 }
 
 // Utils
-func setupRepoForRebase(repo *Repository, masterCommit, branchName string) error {
+func setupRepoForRebase(repo *Repository, masterCommit, branchName string, opts commitOpts) error {
 	// Create a new branch from master
 	err := createBranch(repo, branchName)
 	if err != nil {
@@ -142,7 +263,7 @@ func setupRepoForRebase(repo *Repository, masterCommit, branchName string) error
 	}
 
 	// Create a commit in master
-	_, err = commitSomething(repo, masterCommit, masterCommit)
+	_, err = commitSomething(repo, masterCommit, masterCommit, opts)
 	if err != nil {
 		return err
 	}
@@ -161,7 +282,7 @@ func setupRepoForRebase(repo *Repository, masterCommit, branchName string) error
 	return nil
 }
 
-func performRebaseOnto(repo *Repository, branch string) (*Rebase, error) {
+func performRebaseOnto(repo *Repository, branch string, opts *RebaseOptions) (*Rebase, error) {
 	master, err := repo.LookupBranch(branch, BranchLocal)
 	if err != nil {
 		return nil, err
@@ -175,7 +296,7 @@ func performRebaseOnto(repo *Repository, branch string) (*Rebase, error) {
 	defer onto.Free()
 
 	// Init rebase
-	rebase, err := repo.InitRebase(nil, nil, onto, nil)
+	rebase, err := repo.InitRebase(nil, nil, onto, opts)
 	if err != nil {
 		return nil, err
 	}
@@ -276,7 +397,7 @@ func headTree(repo *Repository) (*Tree, error) {
 	return tree, nil
 }
 
-func commitSomething(repo *Repository, something, content string) (*Oid, error) {
+func commitSomething(repo *Repository, something, content string, commitOpts commitOpts) (*Oid, error) {
 	headCommit, err := headCommit(repo)
 	if err != nil {
 		return nil, err
@@ -315,14 +436,40 @@ func commitSomething(repo *Repository, something, content string) (*Oid, error)
 	}
 	defer newTree.Free()
 
-	if err != nil {
-		return nil, err
-	}
 	commit, err := repo.CreateCommit("HEAD", signature(), signature(), "Test rebase, Baby! "+something, newTree, headCommit)
 	if err != nil {
 		return nil, err
 	}
 
+	if commitOpts.CommitSigningCallback != nil {
+		commit, err := repo.LookupCommit(commit)
+		if err != nil {
+			return nil, err
+		}
+
+		oid, err := commit.WithSignatureUsing(commitOpts.CommitSigningCallback)
+		if err != nil {
+			return nil, err
+		}
+		newCommit, err := repo.LookupCommit(oid)
+		if err != nil {
+			return nil, err
+		}
+		head, err := repo.Head()
+		if err != nil {
+			return nil, err
+		}
+		_, err = repo.References.Create(
+			head.Name(),
+			newCommit.Id(),
+			true,
+			"repoint to signed commit",
+		)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	opts := &CheckoutOpts{
 		Strategy: CheckoutRemoveUntracked | CheckoutForce,
 	}