From 26fddb3c3cad79d890a4045155d5085235b19c36 Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Tue, 2 Apr 2019 00:20:01 -0400 Subject: [PATCH 01/12] add SignKey gpg entity param to allow easier pgp signing of commits --- github/git_commits.go | 53 +++++++++ github/git_commits_test.go | 229 +++++++++++++++++++++++++++++++++++++ go.mod | 1 + go.sum | 2 + 4 files changed, 285 insertions(+) diff --git a/github/git_commits.go b/github/git_commits.go index 9dfd3afb3b5..50033590db0 100644 --- a/github/git_commits.go +++ b/github/git_commits.go @@ -6,9 +6,12 @@ package github import ( + "bytes" "context" "fmt" "time" + + "golang.org/x/crypto/openpgp" ) // SignatureVerification represents GPG signature verification. @@ -37,6 +40,11 @@ type Commit struct { // is only populated for requests that fetch GitHub data like // Pulls.ListCommits, Repositories.ListCommits, etc. CommentCount *int `json:"comment_count,omitempty"` + + // SigningKey denotes a key to sign the commit with. If not nil this key will + // be used to sign the commit. The private key must be present and already + // decrypted. Ignored if Verification.Signature is defined. + SigningKey *openpgp.Entity `json:"-"` } func (c Commit) String() string { @@ -116,6 +124,13 @@ func (s *GitService) CreateCommit(ctx context.Context, owner string, repo string if commit.Tree != nil { body.Tree = commit.Tree.SHA } + if commit.SigningKey != nil { + signature, err := createSignature(commit.SigningKey, body) + if err != nil { + return nil, nil, err + } + body.Signature = &signature + } if commit.Verification != nil { body.Signature = commit.Verification.Signature } @@ -133,3 +148,41 @@ func (s *GitService) CreateCommit(ctx context.Context, owner string, repo string return c, resp, nil } + +func createSignature(SigningKey *openpgp.Entity, commit *createCommit) (string, error) { + if commit.Author == nil { + return "", fmt.Errorf("Commit Author is required to sign a commit") + } + message := createSignatureMessage(commit) + + writer := new(bytes.Buffer) + reader := bytes.NewReader([]byte(message)) + err := openpgp.ArmoredDetachSign(writer, SigningKey, reader, nil) + if err != nil { + return "", err + } + + return writer.String(), nil +} + +func createSignatureMessage(commit *createCommit) string { + message := "" + + if commit.Tree != nil { + message = fmt.Sprintf("tree %s\n", *commit.Tree) + } + + for _, parent := range commit.Parents { + message += fmt.Sprintf("parent %s\n", parent) + } + + message += fmt.Sprintf("author %s <%s> %d %s\n", *commit.Author.Name, *commit.Author.Email, commit.Author.Date.Unix(), commit.Author.Date.Format("-0700")) + commiter := commit.Committer + if commiter == nil { + commiter = commit.Author + } + // There needs to be a double newline after committer + message += fmt.Sprintf("committer %s <%s> %d %s\n\n", *commiter.Name, *commiter.Email, commiter.Date.Unix(), commiter.Date.Format("-0700")) + message += fmt.Sprintf("%s", *commit.Message) + return message +} diff --git a/github/git_commits_test.go b/github/git_commits_test.go index 83f56407295..29205ec794b 100644 --- a/github/git_commits_test.go +++ b/github/git_commits_test.go @@ -11,7 +11,11 @@ import ( "fmt" "net/http" "reflect" + "strings" "testing" + "time" + + "golang.org/x/crypto/openpgp" ) func TestGitService_GetCommit(t *testing.T) { @@ -123,6 +127,172 @@ func TestGitService_CreateSignedCommit(t *testing.T) { t.Errorf("Git.CreateCommit returned %+v, want %+v", commit, want) } } +func TestGitService_CreateSignedCommitWithInvalidParams(t *testing.T) { + client, _, _, teardown := setup() + defer teardown() + + input := &Commit{ + SigningKey: &openpgp.Entity{}, + } + + _, _, err := client.Git.CreateCommit(context.Background(), "o", "r", input) + if err == nil { + t.Errorf("Expected error to be returned because invalid params was passed") + } +} + +func TestGitService_CreateSignedCommitWithKey(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + s := strings.NewReader(testKey) + keyring, err := openpgp.ReadArmoredKeyRing(s) + + date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") + author := CommitAuthor{ + Name: String("go-github"), + Email: String("go-github@github.com"), + Date: &date, + } + input := &Commit{ + Message: String("m"), + Tree: &Tree{SHA: String("t")}, + Parents: []Commit{{SHA: String("p")}}, + SigningKey: keyring[0], + Author: &author, + } + + messageReader := strings.NewReader(`tree t +parent p +author go-github 1493849023 +0200 +committer go-github 1493849023 +0200 + +m`) + + mux.HandleFunc("/repos/o/r/git/commits", func(w http.ResponseWriter, r *http.Request) { + v := new(createCommit) + json.NewDecoder(r.Body).Decode(v) + + testMethod(t, r, "POST") + + want := &createCommit{ + Message: input.Message, + Tree: String("t"), + Parents: []string{"p"}, + Author: &author, + } + + sigReader := strings.NewReader(*v.Signature) + signer, err := openpgp.CheckArmoredDetachedSignature(keyring, messageReader, sigReader) + if err != nil { + t.Errorf("Error verifying signature: %+v", err) + } + if signer.Identities["go-github "].Name != "go-github " { + t.Errorf("Signer is incorrect. got: %+v, want %+v", signer.Identities["go-github "].Name, "go-github ") + } + // Nullify Signature since we checked it above + v.Signature = nil + if !reflect.DeepEqual(v, want) { + t.Errorf("Request body = %+v, want %+v", v, want) + } + fmt.Fprint(w, `{"sha":"s"}`) + }) + + commit, _, err := client.Git.CreateCommit(context.Background(), "o", "r", input) + if err != nil { + t.Errorf("Git.CreateCommit returned error: %v", err) + } + + want := &Commit{SHA: String("s")} + if !reflect.DeepEqual(commit, want) { + t.Errorf("Git.CreateCommit returned %+v, want %+v", commit, want) + } +} + +func TestGitService_createSignature_noAuthor(t *testing.T) { + a := &createCommit{ + Message: String("m"), + Tree: String("t"), + Parents: []string{"p"}, + } + + _, err := createSignature(nil, a) + + if err == nil { + t.Errorf("Expected error to be returned because no author was passed") + } + expectedString := "Commit Author is required to sign a commit" + if !strings.Contains(err.Error(), expectedString) { + t.Errorf("Returned incorrect error. returned %s, want %s", err.Error(), expectedString) + } +} + +func TestGitService_createSignature_invalidKey(t *testing.T) { + date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") + + _, err := createSignature(&openpgp.Entity{}, &createCommit{ + Message: String("m"), + Tree: String("t"), + Parents: []string{"p"}, + Author: &CommitAuthor{ + Name: String("go-github"), + Email: String("go-github@github.com"), + Date: &date, + }, + }) + + if err == nil { + t.Errorf("Expected error to be returned due to invalid key") + } +} + +func TestGitService_createSignatureMessage_withoutTree(t *testing.T) { + date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") + + msg := createSignatureMessage(&createCommit{ + Message: String("m"), + Parents: []string{"p"}, + Author: &CommitAuthor{ + Name: String("go-github"), + Email: String("go-github@github.com"), + Date: &date, + }, + }) + expected := `parent p +author go-github 1493849023 +0200 +committer go-github 1493849023 +0200 + +m` + if msg != expected { + t.Errorf("Returned message incorrect. returned %s, want %s", msg, expected) + } +} + +func TestGitService_createSignatureMessage_withoutCommitter(t *testing.T) { + date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") + + msg := createSignatureMessage(&createCommit{ + Message: String("m"), + Parents: []string{"p"}, + Author: &CommitAuthor{ + Name: String("go-github"), + Email: String("go-github@github.com"), + Date: &date, + }, + Committer: &CommitAuthor{ + Name: String("foo"), + Email: String("foo@bar.com"), + Date: &date, + }, + }) + expected := `parent p +author go-github 1493849023 +0200 +committer foo 1493849023 +0200 + +m` + if msg != expected { + t.Errorf("Returned message incorrect. returned %s, want %s", msg, expected) + } +} func TestGitService_CreateCommit_invalidOwner(t *testing.T) { client, _, _, teardown := setup() @@ -131,3 +301,62 @@ func TestGitService_CreateCommit_invalidOwner(t *testing.T) { _, _, err := client.Git.CreateCommit(context.Background(), "%", "%", &Commit{}) testURLParseError(t, err) } + +const testKey = ` +-----BEGIN PGP PRIVATE KEY BLOCK----- + +lQOYBFyi1qYBCAD3EPfLJzIt4qkAceUKkhdvfaIvOsBwXbfr5sSu/lkMqL0Wq47+ +iv+SRwOC7zvN8SlB8nPUgs5dbTRCJJfG5MAqTRR7KZRbyq2jBpi4BtmO30Ul/qId +3A18cVUfgVbxH85K9bdnyOxep/Q2NjLjTKmWLkzgmgkfbUmSLuWW9HRXPjYy9B7i +dOFD6GdkN/HwPAaId8ym0TE1mIuSpw8UQHyxusAkK52Pn4h/PgJhLTzbSi1X2eDt +OgzjhbdxTPzKFQfs97dY8y9C7Bt+CqH6Bvr3785LeKdxiUnCjfUJ+WAoJy780ec+ +IVwSpPp1CaEtzu73w6GH5945GELHE8HRe25FABEBAAEAB/9dtx72/VAoXZCTbaBe +iRnAnZwWZCe4t6PbJHa4lhv7FEpdPggIf3r/5lXrpYk+zdpDfI75LgDPKWwoJq83 +r29A3GoHabcvtkp0yzzEmTyO2BvnlJWz09N9v5N1Vt8+qTzb7CZ8hJc8NGMK6TYW +R+8P21In4+XP+OluPMGzp9g1etHScLhQUtF/xcN3JQGkeq4CPX6jUSYlJNeEtuLm +xjBTLBdg8zK5mJ3tolvnS/VhSTdiBeUaYtVt/qxq+fPqdFGHrO5H9ORbt56ahU+f +Ne86sOjQfJZPsx9z8ffP+XhLZPT1ZUGJMI/Vysx9gwDiEnaxrCJ02fO0Dnqsj/o2 +T14lBAD55+KtaS0C0OpHpA/F+XhL3IDcYQOYgu8idBTshr4vv7M+jdZqpECOn72Q +8SZJ+gYMcA9Z07Afnin1DVdtxiMN/tbyOu7e1BE7y77eA+zQw4PjLJPZJMbco7z+ +q9ZnZF3GyRyil6HkKUTfrao8AMtb0allZnqXwpPb5Mza32VqtwQA/RdbG6OIS6og +OpP7zKu4GP4guBk8NrVpVuV5Xz4r8JlL+POt0TadlT93coW/SajLrN/eeUwk6jQw +wrabmIGMarG5mrC4tnXLze5LICJTpOuqCACyFwL6w/ag+c7Qt9t9hvLMDFifcZW/ +mylqY7Z1eVcnbOcFsQG+0LzJBU0qouMEAKkXmJcQ3lJM8yoJuYOvbwexVR+5Y+5v +FNEGPlp3H/fq6ETYWHjMxPOE5dvGbQL8oKWZgkkHEGAKAavEGebM/y/qIPOCAluT +tn1sfx//n6kTMhswpg/3+BciUaJFjwYbIwUH5XD0vFbe9O2VOfTVdo1p19wegVs5 +LMf8rWFWYXtqUgG0IGdvLWdpdGh1YiA8Z28tZ2l0aHViQGdpdGh1Yi5jb20+iQFU +BBMBCAA+FiEELZ6AMqOpBMVblK0uiKTQXVy+MAsFAlyi1qYCGwMFCQPCZwAFCwkI +BwIGFQoJCAsCBBYCAwECHgECF4AACgkQiKTQXVy+MAtEYggA0LRecz71HUjEKXJj +C5Wgds1hZ0q+g3ew7zms4fuascd/2PqT5lItHU3oezdzMOHetSPvPzJILjl7RYcY +pWvoyzEBC5MutlmuzfwUa7qYCiuRDkYRjke8a4o8ijsxc8ANXwulXcI3udjAZdV0 +CKjrjPTyrHFUnPyZyaZp8p2eX62iPYhaXkoBnEiarf0xKtJuT/8IlP5n/redlKYz +GIHG5Svg3uDq9E09BOjFsgemhPyqbf7yrh5aRwDOIdHtn9mNevFPfQ1jO8lI/wbe +4kC6zXM7te0/ZkM06DYRhcaeoYdeyY/gvE+w7wU/+f7Wzqt+LxOMIjKk0oDxZIv9 +praEM50DmARcotamAQgAsiO75WZvjt7BEAzdTvWekWXqBo4NOes2UgzSYToVs6xW +8iXnE+mpDS7GHtNQLU6oeC0vizUjCwBfU+qGqw1JjI3I1pwv7xRqBIlA6f5ancVK +KiMx+/HxasbBrbav8DmZT8E8VaJhYM614Kav91W8YoqK5YXmP/A+OwwhkVEGo8v3 +Iy7mnJPMSjNiNTpiDgc5wvRiTan+uf+AtNPUS0k0fbrTZWosbrSmBymhrEy8stMj +rG2wZX5aRY7AXrQXoIXedqvP3kW/nqd0wvuiD11ZZWvoawjZRRVsT27DED0x2+o6 +aAEKrSLj8LlWvGVkD/jP9lSkC81uwGgD5VIMeXv6EQARAQABAAf7BHef8SdJ+ee9 +KLVh4WaIdPX80fBDBaZP5OvcZMLLo4dZYNYxfs7XxfRb1I8RDinQUL81V4TcHZ0D +Rvv1J5n8M7GkjTk6fIDjDb0RayzNQfKeIwNh8AMHvllApyYTMG+JWDYs2KrrTT2x +0vHrLMUyJbh6tjnO5eCU9u8dcmL5Syc6DzGUvDl6ZdJxlHEEJOwMlVCwQn5LQDVI +t0KEXigqs7eDCpTduJeAI7oA96s/8LwdlG5t6q9vbkEjl1XpR5FfKvJcZbd7Kmk9 +6R0EdbH6Ffe8qAp8lGmjx+91gqeL7jyl500H4gK/ybzlxQczIsbQ7WcZTPEnROIX +tCFWh6puvwQAyV6ygcatz+1BfCfgxWNYFXyowwOGSP9Nma+/aDVdeRCjZ69Is0lz +GV0NNqh7hpaoVbXS9Vc3sFOwBr5ZyKQaf07BoCDW+XJtvPyyZNLb004smtB5uHCf +uWDBpQ9erlrpSkOLgifbzfkYHdSvhc2ws9Tgab7Mk7P/ExOZjnUJPOcEAOJ3q/2/ +0wqRnkSelgkWwUmZ+hFIBz6lgWS3KTJs6Qc5WBnXono+EOoqhFxsiRM4lewExxHM +kPIcxb+0hiNz8hJkWOHEdgkXNim9Q08J0HPz6owtlD/rtmOi2+7d5BukbY/3JEXs +r2bjqbXXIE7heytIn/dQv7aEDyDqexiJKnpHBACQItjuYlewLt94NMNdGcwxmKdJ +bfaoIQz1h8fX5uSGKU+hXatI6sltD9PrhwwhdqJNcQ0K1dRkm24olO4I/sJwactI +G3r1UTq6BMV94eIyS/zZH5xChlOUavy9PrgU3kAK21bdmAFuNwbHnN34BBUk9J6f +IIxEZUOxw2CrKhsubUOuiQE8BBgBCAAmFiEELZ6AMqOpBMVblK0uiKTQXVy+MAsF +Alyi1qYCGwwFCQPCZwAACgkQiKTQXVy+MAstJAf/Tm2hfagVjzgJ5pFHmpP+fYxp +8dIPZLonP5HW12iaSOXThtvWBY578Cb9RmU+WkHyPXg8SyshW7aco4HrUDk+Qmyi +f9BvHS5RsLbyPlhgCqNkn+3QS62fZiIlbHLrQ/6iHXkgLV04Fnj+F4v8YYpOI9nY +NFc5iWm0zZRcLiRKZk1up8SCngyolcjVuTuCXDKyAUX1jRqDu7tlN0qVH0CYDGch +BqTKXNkzAvV+CKOyaUILSBBWdef+cxVrDCJuuC3894x3G1FjJycOy0m9PArvGtSG +g7/0Bp9oLXwiHzFoUMDvx+WlPnPHQNcufmQXUNdZvg+Ad4/unEU81EGDBDz3Eg== +=VFSn +-----END PGP PRIVATE KEY BLOCK-----` diff --git a/go.mod b/go.mod index ac4c2dab09c..f1ae2d664d5 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/google/go-github/v26 require ( github.com/golang/protobuf v1.2.0 // indirect github.com/google/go-querystring v1.0.0 + github.com/jchavannes/go-pgp v0.0.0-20170517064339-cb4b0b0ac8cf golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 golang.org/x/net v0.0.0-20190311183353-d8887717615a // indirect golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be diff --git a/go.sum b/go.sum index 927256d3d86..f372ca69390 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,8 @@ github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk= github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck= +github.com/jchavannes/go-pgp v0.0.0-20170517064339-cb4b0b0ac8cf h1:y8QyJtluNQtCj0K70OUtGUxlpGiKDB74iJArA9h4eaE= +github.com/jchavannes/go-pgp v0.0.0-20170517064339-cb4b0b0ac8cf/go.mod h1:dtFptCZ3M/9AWU38htm1xFvWqaJr5ZvkiOiozne99Ps= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628= From fb87aabbae831d6df40fa96b039588ec6149f3c2 Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Fri, 14 Jun 2019 14:17:38 -0400 Subject: [PATCH 02/12] move to using getters when possible. Fix error message. --- github/git_commits.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/github/git_commits.go b/github/git_commits.go index 50033590db0..a5ad375d5f2 100644 --- a/github/git_commits.go +++ b/github/git_commits.go @@ -8,6 +8,7 @@ package github import ( "bytes" "context" + "errors" "fmt" "time" @@ -151,14 +152,13 @@ func (s *GitService) CreateCommit(ctx context.Context, owner string, repo string func createSignature(SigningKey *openpgp.Entity, commit *createCommit) (string, error) { if commit.Author == nil { - return "", fmt.Errorf("Commit Author is required to sign a commit") + return "", errors.New("createSignature: commit.Author=nil") } message := createSignatureMessage(commit) writer := new(bytes.Buffer) reader := bytes.NewReader([]byte(message)) - err := openpgp.ArmoredDetachSign(writer, SigningKey, reader, nil) - if err != nil { + if err := openpgp.ArmoredDetachSign(writer, SigningKey, reader, nil); err != nil { return "", err } @@ -176,13 +176,14 @@ func createSignatureMessage(commit *createCommit) string { message += fmt.Sprintf("parent %s\n", parent) } - message += fmt.Sprintf("author %s <%s> %d %s\n", *commit.Author.Name, *commit.Author.Email, commit.Author.Date.Unix(), commit.Author.Date.Format("-0700")) - commiter := commit.Committer - if commiter == nil { - commiter = commit.Author + message += fmt.Sprintf("author %s <%s> %d %s\n", commit.Author.GetName(), commit.Author.GetEmail(), commit.Author.GetDate().Unix(), commit.Author.GetDate().Format("-0700")) + committer := commit.Committer + if committer == nil { + committer = commit.Author } + // There needs to be a double newline after committer - message += fmt.Sprintf("committer %s <%s> %d %s\n\n", *commiter.Name, *commiter.Email, commiter.Date.Unix(), commiter.Date.Format("-0700")) + message += fmt.Sprintf("committer %s <%s> %d %s\n\n", committer.GetName(), committer.GetEmail(), committer.GetDate().Unix(), committer.GetDate().Format("-0700")) message += fmt.Sprintf("%s", *commit.Message) return message } From e035001c28e10b49fd1955d4b3ce510ec5247ff7 Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Fri, 14 Jun 2019 14:17:48 -0400 Subject: [PATCH 03/12] make test easier to understand --- github/git_commits_test.go | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/github/git_commits_test.go b/github/git_commits_test.go index 29205ec794b..89a533606e0 100644 --- a/github/git_commits_test.go +++ b/github/git_commits_test.go @@ -144,7 +144,7 @@ func TestGitService_CreateSignedCommitWithInvalidParams(t *testing.T) { func TestGitService_CreateSignedCommitWithKey(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - s := strings.NewReader(testKey) + s := strings.NewReader(testGPGKey) keyring, err := openpgp.ReadArmoredKeyRing(s) date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") @@ -220,19 +220,15 @@ func TestGitService_createSignature_noAuthor(t *testing.T) { if err == nil { t.Errorf("Expected error to be returned because no author was passed") } - expectedString := "Commit Author is required to sign a commit" - if !strings.Contains(err.Error(), expectedString) { - t.Errorf("Returned incorrect error. returned %s, want %s", err.Error(), expectedString) - } } func TestGitService_createSignature_invalidKey(t *testing.T) { date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") _, err := createSignature(&openpgp.Entity{}, &createCommit{ - Message: String("m"), - Tree: String("t"), - Parents: []string{"p"}, + Message: String("Commit message."), + Tree: String("commitTree"), + Parents: []string{"commitParent"}, Author: &CommitAuthor{ Name: String("go-github"), Email: String("go-github@github.com"), @@ -249,19 +245,19 @@ func TestGitService_createSignatureMessage_withoutTree(t *testing.T) { date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") msg := createSignatureMessage(&createCommit{ - Message: String("m"), - Parents: []string{"p"}, + Message: String("Commit message."), + Parents: []string{"commitParent"}, Author: &CommitAuthor{ Name: String("go-github"), Email: String("go-github@github.com"), Date: &date, }, }) - expected := `parent p + expected := `parent commitParent author go-github 1493849023 +0200 committer go-github 1493849023 +0200 -m` +Commit message.` if msg != expected { t.Errorf("Returned message incorrect. returned %s, want %s", msg, expected) } @@ -271,8 +267,8 @@ func TestGitService_createSignatureMessage_withoutCommitter(t *testing.T) { date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") msg := createSignatureMessage(&createCommit{ - Message: String("m"), - Parents: []string{"p"}, + Message: String("Commit Message."), + Parents: []string{"commitParent"}, Author: &CommitAuthor{ Name: String("go-github"), Email: String("go-github@github.com"), @@ -284,11 +280,11 @@ func TestGitService_createSignatureMessage_withoutCommitter(t *testing.T) { Date: &date, }, }) - expected := `parent p + expected := `parent commitParent author go-github 1493849023 +0200 committer foo 1493849023 +0200 -m` +Commit Message.` if msg != expected { t.Errorf("Returned message incorrect. returned %s, want %s", msg, expected) } @@ -302,7 +298,7 @@ func TestGitService_CreateCommit_invalidOwner(t *testing.T) { testURLParseError(t, err) } -const testKey = ` +const testGPGKey = ` -----BEGIN PGP PRIVATE KEY BLOCK----- lQOYBFyi1qYBCAD3EPfLJzIt4qkAceUKkhdvfaIvOsBwXbfr5sSu/lkMqL0Wq47+ From 4d7876c0b56cbc2618e86ca8da09543bd06bc659 Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Fri, 14 Jun 2019 14:28:13 -0400 Subject: [PATCH 04/12] make git commit test variables more verbose --- github/git_commits_test.go | 76 +++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/github/git_commits_test.go b/github/git_commits_test.go index 89a533606e0..bb535594c7f 100644 --- a/github/git_commits_test.go +++ b/github/git_commits_test.go @@ -22,17 +22,17 @@ func TestGitService_GetCommit(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - mux.HandleFunc("/repos/o/r/git/commits/s", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/org/repo/git/commits/commitSHA", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - fmt.Fprint(w, `{"sha":"s","message":"m","author":{"name":"n"}}`) + fmt.Fprint(w, `{"sha":"commitSHA","message":"Commit message.","author":{"name":"commitAuthorName"}}`) }) - commit, _, err := client.Git.GetCommit(context.Background(), "o", "r", "s") + commit, _, err := client.Git.GetCommit(context.Background(), "org", "repo", "commitSHA") if err != nil { t.Errorf("Git.GetCommit returned error: %v", err) } - want := &Commit{SHA: String("s"), Message: String("m"), Author: &CommitAuthor{Name: String("n")}} + want := &Commit{SHA: String("commitSHA"), Message: String("Commit message."), Author: &CommitAuthor{Name: String("commitAuthorName")}} if !reflect.DeepEqual(commit, want) { t.Errorf("Git.GetCommit returned %+v, want %+v", commit, want) } @@ -51,12 +51,12 @@ func TestGitService_CreateCommit(t *testing.T) { defer teardown() input := &Commit{ - Message: String("m"), - Tree: &Tree{SHA: String("t")}, - Parents: []Commit{{SHA: String("p")}}, + Message: String("Commit message."), + Tree: &Tree{SHA: String("commitTree")}, + Parents: []Commit{{SHA: String("commitParent")}}, } - mux.HandleFunc("/repos/o/r/git/commits", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/org/repo/git/commits", func(w http.ResponseWriter, r *http.Request) { v := new(createCommit) json.NewDecoder(r.Body).Decode(v) @@ -64,21 +64,21 @@ func TestGitService_CreateCommit(t *testing.T) { want := &createCommit{ Message: input.Message, - Tree: String("t"), - Parents: []string{"p"}, + Tree: String("commitTree"), + Parents: []string{"commitParent"}, } if !reflect.DeepEqual(v, want) { t.Errorf("Request body = %+v, want %+v", v, want) } - fmt.Fprint(w, `{"sha":"s"}`) + fmt.Fprint(w, `{"sha":"commitSHA"}`) }) - commit, _, err := client.Git.CreateCommit(context.Background(), "o", "r", input) + commit, _, err := client.Git.CreateCommit(context.Background(), "org", "repo", input) if err != nil { t.Errorf("Git.CreateCommit returned error: %v", err) } - want := &Commit{SHA: String("s")} + want := &Commit{SHA: String("commitSHA")} if !reflect.DeepEqual(commit, want) { t.Errorf("Git.CreateCommit returned %+v, want %+v", commit, want) } @@ -91,15 +91,15 @@ func TestGitService_CreateSignedCommit(t *testing.T) { signature := "----- BEGIN PGP SIGNATURE -----\n\naaaa\naaaa\n----- END PGP SIGNATURE -----" input := &Commit{ - Message: String("m"), - Tree: &Tree{SHA: String("t")}, - Parents: []Commit{{SHA: String("p")}}, + Message: String("Commit message."), + Tree: &Tree{SHA: String("commitTree")}, + Parents: []Commit{{SHA: String("commitParent")}}, Verification: &SignatureVerification{ Signature: String(signature), }, } - mux.HandleFunc("/repos/o/r/git/commits", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/org/repo/git/commits", func(w http.ResponseWriter, r *http.Request) { v := new(createCommit) json.NewDecoder(r.Body).Decode(v) @@ -107,22 +107,22 @@ func TestGitService_CreateSignedCommit(t *testing.T) { want := &createCommit{ Message: input.Message, - Tree: String("t"), - Parents: []string{"p"}, + Tree: String("commitTree"), + Parents: []string{"commitParent"}, Signature: String(signature), } if !reflect.DeepEqual(v, want) { t.Errorf("Request body = %+v, want %+v", v, want) } - fmt.Fprint(w, `{"sha":"s"}`) + fmt.Fprint(w, `{"sha":"commitSha"}`) }) - commit, _, err := client.Git.CreateCommit(context.Background(), "o", "r", input) + commit, _, err := client.Git.CreateCommit(context.Background(), "org", "repo", input) if err != nil { t.Errorf("Git.CreateCommit returned error: %v", err) } - want := &Commit{SHA: String("s")} + want := &Commit{SHA: String("commitSha")} if !reflect.DeepEqual(commit, want) { t.Errorf("Git.CreateCommit returned %+v, want %+v", commit, want) } @@ -135,7 +135,7 @@ func TestGitService_CreateSignedCommitWithInvalidParams(t *testing.T) { SigningKey: &openpgp.Entity{}, } - _, _, err := client.Git.CreateCommit(context.Background(), "o", "r", input) + _, _, err := client.Git.CreateCommit(context.Background(), "org", "repo", input) if err == nil { t.Errorf("Expected error to be returned because invalid params was passed") } @@ -154,21 +154,21 @@ func TestGitService_CreateSignedCommitWithKey(t *testing.T) { Date: &date, } input := &Commit{ - Message: String("m"), - Tree: &Tree{SHA: String("t")}, - Parents: []Commit{{SHA: String("p")}}, + Message: String("Commit message."), + Tree: &Tree{SHA: String("commitTree")}, + Parents: []Commit{{SHA: String("commitParent")}}, SigningKey: keyring[0], Author: &author, } - messageReader := strings.NewReader(`tree t -parent p + messageReader := strings.NewReader(`tree commitTree +parent commitParent author go-github 1493849023 +0200 committer go-github 1493849023 +0200 -m`) +Commit message.`) - mux.HandleFunc("/repos/o/r/git/commits", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/org/repo/git/commits", func(w http.ResponseWriter, r *http.Request) { v := new(createCommit) json.NewDecoder(r.Body).Decode(v) @@ -176,8 +176,8 @@ m`) want := &createCommit{ Message: input.Message, - Tree: String("t"), - Parents: []string{"p"}, + Tree: String("commitTree"), + Parents: []string{"commitParent"}, Author: &author, } @@ -194,15 +194,15 @@ m`) if !reflect.DeepEqual(v, want) { t.Errorf("Request body = %+v, want %+v", v, want) } - fmt.Fprint(w, `{"sha":"s"}`) + fmt.Fprint(w, `{"sha":"commitSha"}`) }) - commit, _, err := client.Git.CreateCommit(context.Background(), "o", "r", input) + commit, _, err := client.Git.CreateCommit(context.Background(), "org", "repo", input) if err != nil { t.Errorf("Git.CreateCommit returned error: %v", err) } - want := &Commit{SHA: String("s")} + want := &Commit{SHA: String("commitSha")} if !reflect.DeepEqual(commit, want) { t.Errorf("Git.CreateCommit returned %+v, want %+v", commit, want) } @@ -210,9 +210,9 @@ m`) func TestGitService_createSignature_noAuthor(t *testing.T) { a := &createCommit{ - Message: String("m"), - Tree: String("t"), - Parents: []string{"p"}, + Message: String("Commit message."), + Tree: String("commitTree"), + Parents: []string{"commitParent"}, } _, err := createSignature(nil, a) From b95b3735a38083a60afc0a83b7b6a9f6358e01b7 Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Fri, 14 Jun 2019 16:19:47 -0400 Subject: [PATCH 05/12] revert verbose name to keep names consistent --- github/git_commits_test.go | 86 +++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/github/git_commits_test.go b/github/git_commits_test.go index bb535594c7f..38418c9e885 100644 --- a/github/git_commits_test.go +++ b/github/git_commits_test.go @@ -22,17 +22,17 @@ func TestGitService_GetCommit(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - mux.HandleFunc("/repos/org/repo/git/commits/commitSHA", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/o/r/git/commits/s", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - fmt.Fprint(w, `{"sha":"commitSHA","message":"Commit message.","author":{"name":"commitAuthorName"}}`) + fmt.Fprint(w, `{"sha":"s","message":"Commit Message.","author":{"name":"n"}}`) }) - commit, _, err := client.Git.GetCommit(context.Background(), "org", "repo", "commitSHA") + commit, _, err := client.Git.GetCommit(context.Background(), "o", "r", "s") if err != nil { t.Errorf("Git.GetCommit returned error: %v", err) } - want := &Commit{SHA: String("commitSHA"), Message: String("Commit message."), Author: &CommitAuthor{Name: String("commitAuthorName")}} + want := &Commit{SHA: String("s"), Message: String("Commit Message."), Author: &CommitAuthor{Name: String("n")}} if !reflect.DeepEqual(commit, want) { t.Errorf("Git.GetCommit returned %+v, want %+v", commit, want) } @@ -51,12 +51,12 @@ func TestGitService_CreateCommit(t *testing.T) { defer teardown() input := &Commit{ - Message: String("Commit message."), - Tree: &Tree{SHA: String("commitTree")}, - Parents: []Commit{{SHA: String("commitParent")}}, + Message: String("Commit Message."), + Tree: &Tree{SHA: String("t")}, + Parents: []Commit{{SHA: String("p")}}, } - mux.HandleFunc("/repos/org/repo/git/commits", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/o/r/git/commits", func(w http.ResponseWriter, r *http.Request) { v := new(createCommit) json.NewDecoder(r.Body).Decode(v) @@ -64,21 +64,21 @@ func TestGitService_CreateCommit(t *testing.T) { want := &createCommit{ Message: input.Message, - Tree: String("commitTree"), - Parents: []string{"commitParent"}, + Tree: String("t"), + Parents: []string{"p"}, } if !reflect.DeepEqual(v, want) { t.Errorf("Request body = %+v, want %+v", v, want) } - fmt.Fprint(w, `{"sha":"commitSHA"}`) + fmt.Fprint(w, `{"sha":"s"}`) }) - commit, _, err := client.Git.CreateCommit(context.Background(), "org", "repo", input) + commit, _, err := client.Git.CreateCommit(context.Background(), "o", "r", input) if err != nil { t.Errorf("Git.CreateCommit returned error: %v", err) } - want := &Commit{SHA: String("commitSHA")} + want := &Commit{SHA: String("s")} if !reflect.DeepEqual(commit, want) { t.Errorf("Git.CreateCommit returned %+v, want %+v", commit, want) } @@ -91,15 +91,15 @@ func TestGitService_CreateSignedCommit(t *testing.T) { signature := "----- BEGIN PGP SIGNATURE -----\n\naaaa\naaaa\n----- END PGP SIGNATURE -----" input := &Commit{ - Message: String("Commit message."), - Tree: &Tree{SHA: String("commitTree")}, - Parents: []Commit{{SHA: String("commitParent")}}, + Message: String("Commit Message."), + Tree: &Tree{SHA: String("t")}, + Parents: []Commit{{SHA: String("p")}}, Verification: &SignatureVerification{ Signature: String(signature), }, } - mux.HandleFunc("/repos/org/repo/git/commits", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/o/r/git/commits", func(w http.ResponseWriter, r *http.Request) { v := new(createCommit) json.NewDecoder(r.Body).Decode(v) @@ -107,8 +107,8 @@ func TestGitService_CreateSignedCommit(t *testing.T) { want := &createCommit{ Message: input.Message, - Tree: String("commitTree"), - Parents: []string{"commitParent"}, + Tree: String("t"), + Parents: []string{"p"}, Signature: String(signature), } if !reflect.DeepEqual(v, want) { @@ -117,7 +117,7 @@ func TestGitService_CreateSignedCommit(t *testing.T) { fmt.Fprint(w, `{"sha":"commitSha"}`) }) - commit, _, err := client.Git.CreateCommit(context.Background(), "org", "repo", input) + commit, _, err := client.Git.CreateCommit(context.Background(), "o", "r", input) if err != nil { t.Errorf("Git.CreateCommit returned error: %v", err) } @@ -135,7 +135,7 @@ func TestGitService_CreateSignedCommitWithInvalidParams(t *testing.T) { SigningKey: &openpgp.Entity{}, } - _, _, err := client.Git.CreateCommit(context.Background(), "org", "repo", input) + _, _, err := client.Git.CreateCommit(context.Background(), "o", "r", input) if err == nil { t.Errorf("Expected error to be returned because invalid params was passed") } @@ -154,21 +154,21 @@ func TestGitService_CreateSignedCommitWithKey(t *testing.T) { Date: &date, } input := &Commit{ - Message: String("Commit message."), - Tree: &Tree{SHA: String("commitTree")}, - Parents: []Commit{{SHA: String("commitParent")}}, + Message: String("Commit Message."), + Tree: &Tree{SHA: String("t")}, + Parents: []Commit{{SHA: String("p")}}, SigningKey: keyring[0], Author: &author, } - messageReader := strings.NewReader(`tree commitTree -parent commitParent + messageReader := strings.NewReader(`tree t +parent p author go-github 1493849023 +0200 committer go-github 1493849023 +0200 -Commit message.`) +Commit Message.`) - mux.HandleFunc("/repos/org/repo/git/commits", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/o/r/git/commits", func(w http.ResponseWriter, r *http.Request) { v := new(createCommit) json.NewDecoder(r.Body).Decode(v) @@ -176,8 +176,8 @@ Commit message.`) want := &createCommit{ Message: input.Message, - Tree: String("commitTree"), - Parents: []string{"commitParent"}, + Tree: String("t"), + Parents: []string{"p"}, Author: &author, } @@ -197,7 +197,7 @@ Commit message.`) fmt.Fprint(w, `{"sha":"commitSha"}`) }) - commit, _, err := client.Git.CreateCommit(context.Background(), "org", "repo", input) + commit, _, err := client.Git.CreateCommit(context.Background(), "o", "r", input) if err != nil { t.Errorf("Git.CreateCommit returned error: %v", err) } @@ -210,9 +210,9 @@ Commit message.`) func TestGitService_createSignature_noAuthor(t *testing.T) { a := &createCommit{ - Message: String("Commit message."), - Tree: String("commitTree"), - Parents: []string{"commitParent"}, + Message: String("Commit Message."), + Tree: String("t"), + Parents: []string{"p"}, } _, err := createSignature(nil, a) @@ -226,9 +226,9 @@ func TestGitService_createSignature_invalidKey(t *testing.T) { date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") _, err := createSignature(&openpgp.Entity{}, &createCommit{ - Message: String("Commit message."), - Tree: String("commitTree"), - Parents: []string{"commitParent"}, + Message: String("Commit Message."), + Tree: String("t"), + Parents: []string{"p"}, Author: &CommitAuthor{ Name: String("go-github"), Email: String("go-github@github.com"), @@ -245,19 +245,19 @@ func TestGitService_createSignatureMessage_withoutTree(t *testing.T) { date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") msg := createSignatureMessage(&createCommit{ - Message: String("Commit message."), - Parents: []string{"commitParent"}, + Message: String("Commit Message."), + Parents: []string{"p"}, Author: &CommitAuthor{ Name: String("go-github"), Email: String("go-github@github.com"), Date: &date, }, }) - expected := `parent commitParent + expected := `parent p author go-github 1493849023 +0200 committer go-github 1493849023 +0200 -Commit message.` +Commit Message.` if msg != expected { t.Errorf("Returned message incorrect. returned %s, want %s", msg, expected) } @@ -268,7 +268,7 @@ func TestGitService_createSignatureMessage_withoutCommitter(t *testing.T) { msg := createSignatureMessage(&createCommit{ Message: String("Commit Message."), - Parents: []string{"commitParent"}, + Parents: []string{"p"}, Author: &CommitAuthor{ Name: String("go-github"), Email: String("go-github@github.com"), @@ -280,7 +280,7 @@ func TestGitService_createSignatureMessage_withoutCommitter(t *testing.T) { Date: &date, }, }) - expected := `parent commitParent + expected := `parent p author go-github 1493849023 +0200 committer foo 1493849023 +0200 From 720c6aa5558f5295c4bf874496822cd2bc561fc1 Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Fri, 14 Jun 2019 16:20:47 -0400 Subject: [PATCH 06/12] check message nil --- github/git_commits.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/github/git_commits.go b/github/git_commits.go index a5ad375d5f2..4c225440451 100644 --- a/github/git_commits.go +++ b/github/git_commits.go @@ -154,6 +154,11 @@ func createSignature(SigningKey *openpgp.Entity, commit *createCommit) (string, if commit.Author == nil { return "", errors.New("createSignature: commit.Author=nil") } + + if commit.Message == nil { + return "", errors.New("createSignature: commit.Message=nil") + } + message := createSignatureMessage(commit) writer := new(bytes.Buffer) From ae5f43ccbd27867de9f15e379c585780d4103359 Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Fri, 14 Jun 2019 16:35:04 -0400 Subject: [PATCH 07/12] add nil checks and respective tests --- github/git_commits.go | 19 +++++++----- github/git_commits_test.go | 63 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 11 deletions(-) diff --git a/github/git_commits.go b/github/git_commits.go index 4c225440451..a7435e8c5a1 100644 --- a/github/git_commits.go +++ b/github/git_commits.go @@ -151,16 +151,15 @@ func (s *GitService) CreateCommit(ctx context.Context, owner string, repo string } func createSignature(SigningKey *openpgp.Entity, commit *createCommit) (string, error) { - if commit.Author == nil { - return "", errors.New("createSignature: commit.Author=nil") + if SigningKey == nil || commit == nil { + return "", errors.New("createSignature: invalid parameters") } - if commit.Message == nil { - return "", errors.New("createSignature: commit.Message=nil") + message, err := createSignatureMessage(commit) + if err != nil { + return "", fmt.Errorf("createSignature: createSignatureMessage error: %s", err.Error()) } - message := createSignatureMessage(commit) - writer := new(bytes.Buffer) reader := bytes.NewReader([]byte(message)) if err := openpgp.ArmoredDetachSign(writer, SigningKey, reader, nil); err != nil { @@ -170,7 +169,11 @@ func createSignature(SigningKey *openpgp.Entity, commit *createCommit) (string, return writer.String(), nil } -func createSignatureMessage(commit *createCommit) string { +func createSignatureMessage(commit *createCommit) (string, error) { + if commit == nil || commit.Message == nil || commit.Author == nil { + return "", errors.New("createSignatureMessage: invalid parameters") + } + message := "" if commit.Tree != nil { @@ -190,5 +193,5 @@ func createSignatureMessage(commit *createCommit) string { // There needs to be a double newline after committer message += fmt.Sprintf("committer %s <%s> %d %s\n\n", committer.GetName(), committer.GetEmail(), committer.GetDate().Unix(), committer.GetDate().Format("-0700")) message += fmt.Sprintf("%s", *commit.Message) - return message + return message, nil } diff --git a/github/git_commits_test.go b/github/git_commits_test.go index 38418c9e885..05232599200 100644 --- a/github/git_commits_test.go +++ b/github/git_commits_test.go @@ -208,7 +208,7 @@ Commit Message.`) } } -func TestGitService_createSignature_noAuthor(t *testing.T) { +func TestGitService_createSignature_nilSigningKey(t *testing.T) { a := &createCommit{ Message: String("Commit Message."), Tree: String("t"), @@ -222,6 +222,28 @@ func TestGitService_createSignature_noAuthor(t *testing.T) { } } +func TestGitService_createSignature_nilCommit(t *testing.T) { + _, err := createSignature(&openpgp.Entity{}, nil) + + if err == nil { + t.Errorf("Expected error to be returned because no author was passed") + } +} + +func TestGitService_createSignature_noAuthor(t *testing.T) { + a := &createCommit{ + Message: String("Commit Message."), + Tree: String("t"), + Parents: []string{"p"}, + } + + _, err := createSignature(&openpgp.Entity{}, a) + + if err == nil { + t.Errorf("Expected error to be returned because no author was passed") + } +} + func TestGitService_createSignature_invalidKey(t *testing.T) { date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") @@ -241,10 +263,45 @@ func TestGitService_createSignature_invalidKey(t *testing.T) { } } +func TestGitService_createSignatureMessage_nilCommit(t *testing.T) { + _, err := createSignatureMessage(nil) + if err == nil { + t.Errorf("Expected error to be returned due to nil key") + } +} + +func TestGitService_createSignatureMessage_nilMessage(t *testing.T) { + date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") + + _, err := createSignatureMessage(&createCommit{ + Message: nil, + Parents: []string{"p"}, + Author: &CommitAuthor{ + Name: String("go-github"), + Email: String("go-github@github.com"), + Date: &date, + }, + }) + if err == nil { + t.Errorf("Expected error to be returned due to nil key") + } +} + +func TestGitService_createSignatureMessage_nilAuthor(t *testing.T) { + _, err := createSignatureMessage(&createCommit{ + Message: String("Commit Message."), + Parents: []string{"p"}, + Author: nil, + }) + if err == nil { + t.Errorf("Expected error to be returned due to nil key") + } +} + func TestGitService_createSignatureMessage_withoutTree(t *testing.T) { date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") - msg := createSignatureMessage(&createCommit{ + msg, _ := createSignatureMessage(&createCommit{ Message: String("Commit Message."), Parents: []string{"p"}, Author: &CommitAuthor{ @@ -266,7 +323,7 @@ Commit Message.` func TestGitService_createSignatureMessage_withoutCommitter(t *testing.T) { date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") - msg := createSignatureMessage(&createCommit{ + msg, _ := createSignatureMessage(&createCommit{ Message: String("Commit Message."), Parents: []string{"p"}, Author: &CommitAuthor{ From b874928534765a0111942dd5989fadfd7b9ee461 Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Fri, 14 Jun 2019 16:35:46 -0400 Subject: [PATCH 08/12] make signingKey lowercase --- github/git_commits.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/github/git_commits.go b/github/git_commits.go index a7435e8c5a1..9191a290d3a 100644 --- a/github/git_commits.go +++ b/github/git_commits.go @@ -150,8 +150,8 @@ func (s *GitService) CreateCommit(ctx context.Context, owner string, repo string return c, resp, nil } -func createSignature(SigningKey *openpgp.Entity, commit *createCommit) (string, error) { - if SigningKey == nil || commit == nil { +func createSignature(signingKey *openpgp.Entity, commit *createCommit) (string, error) { + if signingKey == nil || commit == nil { return "", errors.New("createSignature: invalid parameters") } @@ -162,7 +162,7 @@ func createSignature(SigningKey *openpgp.Entity, commit *createCommit) (string, writer := new(bytes.Buffer) reader := bytes.NewReader([]byte(message)) - if err := openpgp.ArmoredDetachSign(writer, SigningKey, reader, nil); err != nil { + if err := openpgp.ArmoredDetachSign(writer, signingKey, reader, nil); err != nil { return "", err } From 4c3dd29303f7ad4cb8c85fd807a39c75db600d96 Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Fri, 14 Jun 2019 19:35:16 -0400 Subject: [PATCH 09/12] do not wrap error --- github/git_commits.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/git_commits.go b/github/git_commits.go index 9191a290d3a..65251103c1e 100644 --- a/github/git_commits.go +++ b/github/git_commits.go @@ -157,7 +157,7 @@ func createSignature(signingKey *openpgp.Entity, commit *createCommit) (string, message, err := createSignatureMessage(commit) if err != nil { - return "", fmt.Errorf("createSignature: createSignatureMessage error: %s", err.Error()) + return "", err } writer := new(bytes.Buffer) From 3e950144b5acc1334070b22dc5d00e45e8b42534 Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Fri, 14 Jun 2019 19:40:09 -0400 Subject: [PATCH 10/12] use string array vs hardcoded strings --- github/git_commits.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/github/git_commits.go b/github/git_commits.go index 65251103c1e..1ba418cbfe8 100644 --- a/github/git_commits.go +++ b/github/git_commits.go @@ -10,6 +10,7 @@ import ( "context" "errors" "fmt" + "strings" "time" "golang.org/x/crypto/openpgp" @@ -174,24 +175,26 @@ func createSignatureMessage(commit *createCommit) (string, error) { return "", errors.New("createSignatureMessage: invalid parameters") } - message := "" + var message []string if commit.Tree != nil { - message = fmt.Sprintf("tree %s\n", *commit.Tree) + message = append(message, fmt.Sprintf("tree %s", *commit.Tree)) } for _, parent := range commit.Parents { - message += fmt.Sprintf("parent %s\n", parent) + message = append(message, fmt.Sprintf("parent %s", parent)) } - message += fmt.Sprintf("author %s <%s> %d %s\n", commit.Author.GetName(), commit.Author.GetEmail(), commit.Author.GetDate().Unix(), commit.Author.GetDate().Format("-0700")) + message = append(message, fmt.Sprintf("author %s <%s> %d %s", commit.Author.GetName(), commit.Author.GetEmail(), commit.Author.GetDate().Unix(), commit.Author.GetDate().Format("-0700"))) + committer := commit.Committer if committer == nil { committer = commit.Author } // There needs to be a double newline after committer - message += fmt.Sprintf("committer %s <%s> %d %s\n\n", committer.GetName(), committer.GetEmail(), committer.GetDate().Unix(), committer.GetDate().Format("-0700")) - message += fmt.Sprintf("%s", *commit.Message) - return message, nil + message = append(message, fmt.Sprintf("committer %s <%s> %d %s\n", committer.GetName(), committer.GetEmail(), committer.GetDate().Unix(), committer.GetDate().Format("-0700"))) + message = append(message, fmt.Sprintf("%s", *commit.Message)) + + return strings.Join(message, "\n"), nil } From 179d11c571954c1a9c5237ddb70848eb7636e762 Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Sat, 15 Jun 2019 07:48:16 -0400 Subject: [PATCH 11/12] add empty message check --- github/git_commits.go | 4 ++-- github/git_commits_test.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/github/git_commits.go b/github/git_commits.go index 1ba418cbfe8..741961980a1 100644 --- a/github/git_commits.go +++ b/github/git_commits.go @@ -171,7 +171,7 @@ func createSignature(signingKey *openpgp.Entity, commit *createCommit) (string, } func createSignatureMessage(commit *createCommit) (string, error) { - if commit == nil || commit.Message == nil || commit.Author == nil { + if commit == nil || commit.Message == nil || *commit.Message == "" || commit.Author == nil { return "", errors.New("createSignatureMessage: invalid parameters") } @@ -194,7 +194,7 @@ func createSignatureMessage(commit *createCommit) (string, error) { // There needs to be a double newline after committer message = append(message, fmt.Sprintf("committer %s <%s> %d %s\n", committer.GetName(), committer.GetEmail(), committer.GetDate().Unix(), committer.GetDate().Format("-0700"))) - message = append(message, fmt.Sprintf("%s", *commit.Message)) + message = append(message, *commit.Message) return strings.Join(message, "\n"), nil } diff --git a/github/git_commits_test.go b/github/git_commits_test.go index 05232599200..580e6b05515 100644 --- a/github/git_commits_test.go +++ b/github/git_commits_test.go @@ -287,6 +287,23 @@ func TestGitService_createSignatureMessage_nilMessage(t *testing.T) { } } +func TestGitService_createSignatureMessage_emptyMessage(t *testing.T) { + date, _ := time.Parse("Mon Jan 02 15:04:05 2006 -0700", "Thu May 04 00:03:43 2017 +0200") + emptyString := "" + _, err := createSignatureMessage(&createCommit{ + Message: &emptyString, + Parents: []string{"p"}, + Author: &CommitAuthor{ + Name: String("go-github"), + Email: String("go-github@github.com"), + Date: &date, + }, + }) + if err == nil { + t.Errorf("Expected error to be returned due to nil key") + } +} + func TestGitService_createSignatureMessage_nilAuthor(t *testing.T) { _, err := createSignatureMessage(&createCommit{ Message: String("Commit Message."), From 27bf573be40ba437de9f783335838a3c379505aa Mon Sep 17 00:00:00 2001 From: Anand Patel Date: Sat, 22 Jun 2019 12:27:46 -0400 Subject: [PATCH 12/12] remove unused dep --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index f1ae2d664d5..ac4c2dab09c 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module github.com/google/go-github/v26 require ( github.com/golang/protobuf v1.2.0 // indirect github.com/google/go-querystring v1.0.0 - github.com/jchavannes/go-pgp v0.0.0-20170517064339-cb4b0b0ac8cf golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 golang.org/x/net v0.0.0-20190311183353-d8887717615a // indirect golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be diff --git a/go.sum b/go.sum index f372ca69390..927256d3d86 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,6 @@ github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk= github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck= -github.com/jchavannes/go-pgp v0.0.0-20170517064339-cb4b0b0ac8cf h1:y8QyJtluNQtCj0K70OUtGUxlpGiKDB74iJArA9h4eaE= -github.com/jchavannes/go-pgp v0.0.0-20170517064339-cb4b0b0ac8cf/go.mod h1:dtFptCZ3M/9AWU38htm1xFvWqaJr5ZvkiOiozne99Ps= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628=