Skip to content

Commit

Permalink
Add GitHub committer verification and fix empty reviewers
Browse files Browse the repository at this point in the history
  • Loading branch information
laurentsimon committed Mar 2, 2022
1 parent 738b246 commit ade2929
Showing 1 changed file with 26 additions and 3 deletions.
29 changes: 26 additions & 3 deletions clients/githubrepo/graphql.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package githubrepo

import (
"context"
"errors"
"fmt"
"strings"
"sync"
Expand All @@ -36,6 +37,8 @@ const (
commitsToAnalyze = 30
)

var errorInvalidCommitterLogin = errors.New("cannot retrieve committer login")

// nolint: govet
type graphqlData struct {
Repository struct {
Expand All @@ -58,6 +61,10 @@ type graphqlData struct {
Login *string
}
}
Signature struct {
IsValid bool
WasSignedByGitHub bool
}
AssociatedPullRequests struct {
Nodes []struct {
Repository struct {
Expand Down Expand Up @@ -196,10 +203,22 @@ func commitsFrom(data *graphqlData, repoOwner, repoName string) ([]clients.Commi
ret := make([]clients.Commit, 0)
for _, commit := range data.Repository.Object.Commit.History.Nodes {
var committer string
if commit.Committer.User.Login != nil {
// Find the commit's committer.
if commit.Committer.User.Login != nil && *commit.Committer.User.Login != "" {
committer = *commit.Committer.User.Login
} else if commit.Committer.Name != nil &&
// Username "GitHub" may indicate the commit was committed by GitHub.
// We verify the signature on the commit, because the name can be spoofed.
*commit.Committer.Name == "GitHub" &&
commit.Signature.IsValid &&
commit.Signature.WasSignedByGitHub {
committer = "github"
}
// TODO(#1543): Figure out a way to safely get committer if `User.Login` is `nil`.

if committer == "" {
return ret, fmt.Errorf("commit %s: %w", commit.Oid, errorInvalidCommitterLogin)
}

var associatedPR clients.PullRequest
for i := range commit.AssociatedPullRequests.Nodes {
pr := commit.AssociatedPullRequests.Nodes[i]
Expand All @@ -224,8 +243,12 @@ func commitsFrom(data *graphqlData, repoOwner, repoName string) ([]clients.Commi
})
}
for _, review := range pr.Reviews.Nodes {
author := clients.User{
Login: string(review.Author.Login),
}
associatedPR.Reviews = append(associatedPR.Reviews, clients.Review{
State: string(review.State),
State: string(review.State),
Author: &author,
})
}
break
Expand Down

0 comments on commit ade2929

Please sign in to comment.