Skip to content

Commit 0d9b7a1

Browse files
qequqequ
authored
✨ Feature: Improve Dependabot detection through PRs (#2125)
* clients: Update client type to add SearchCommits function Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * checks/raw: Update Dependency Update Tool check to search for commits made by dependabot in default branch Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * clients/mockclients: Update mock for repoClient to add SearchCommits function mocks Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * checks: Update unit tests for Dependency Update tool with new feature of SearchCommits Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * clients/githubrepo: Update SearchCommitsHandler's buildQuery function & add tests Add "author:" to the query. Remove ReplaceAll unused for Author formatting. Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * clients: Add explanatory comment for SearchCommits Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * Clients: Update SearchCommits to return []Commit instead of SearchCommitsResponse Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * checks: Update dependency update tool check according to change by SearchCommits now returning []Commit Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * clients/githubrepo: Add license header Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * clients: Add exported comment & remove unused structs Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * checks/raw: Address rangeValCopy issue when iterating commits Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * clients: Address issue concerning field alignment in User struct Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> * clients/githubrepo: Addres line length linter issue Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com> Co-authored-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
1 parent 4e37e79 commit 0d9b7a1

File tree

11 files changed

+365
-47
lines changed

11 files changed

+365
-47
lines changed

‎checks/dependency_update_tool_test.go‎

Lines changed: 73 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,27 +20,35 @@ import (
2020
"github.com/golang/mock/gomock"
2121

2222
"github.com/ossf/scorecard/v4/checker"
23+
clients "github.com/ossf/scorecard/v4/clients"
2324
mockrepo "github.com/ossf/scorecard/v4/clients/mockclients"
2425
scut "github.com/ossf/scorecard/v4/utests"
2526
)
2627

28+
const (
29+
dependabotID = 49699333
30+
)
31+
2732
// TestDependencyUpdateTool tests the DependencyUpdateTool checker.
2833
func TestDependencyUpdateTool(t *testing.T) {
2934
t.Parallel()
3035
//nolint
3136
tests := []struct {
32-
name string
33-
wantErr bool
34-
files []string
35-
want checker.CheckResult
36-
expected scut.TestReturn
37+
name string
38+
wantErr bool
39+
SearchCommits []clients.Commit
40+
CallSearchCommits int
41+
files []string
42+
want checker.CheckResult
43+
expected scut.TestReturn
3744
}{
3845
{
3946
name: "dependency yml",
4047
wantErr: false,
4148
files: []string{
4249
".github/dependabot.yml",
4350
},
51+
CallSearchCommits: 0,
4452
expected: scut.TestReturn{
4553
NumberOfInfo: 1,
4654
Score: 10,
@@ -52,6 +60,7 @@ func TestDependencyUpdateTool(t *testing.T) {
5260
files: []string{
5361
".github/dependabot.yaml",
5462
},
63+
CallSearchCommits: 0,
5564
expected: scut.TestReturn{
5665
NumberOfInfo: 1,
5766
Score: 10,
@@ -63,10 +72,68 @@ func TestDependencyUpdateTool(t *testing.T) {
6372
files: []string{
6473
".github/foobar.yml",
6574
},
75+
SearchCommits: []clients.Commit{{Committer: clients.User{ID: 111111111}}},
76+
CallSearchCommits: 1,
77+
expected: scut.TestReturn{
78+
NumberOfWarn: 2,
79+
},
80+
},
81+
{
82+
name: "foo bar 2",
83+
wantErr: false,
84+
files: []string{
85+
".github/foobar.yml",
86+
},
87+
SearchCommits: []clients.Commit{},
88+
CallSearchCommits: 1,
6689
expected: scut.TestReturn{
6790
NumberOfWarn: 2,
6891
},
6992
},
93+
94+
{
95+
name: "found in commits",
96+
wantErr: false,
97+
files: []string{
98+
".github/foobar.yaml",
99+
},
100+
SearchCommits: []clients.Commit{{Committer: clients.User{ID: dependabotID}}},
101+
CallSearchCommits: 1,
102+
expected: scut.TestReturn{
103+
NumberOfInfo: 1,
104+
Score: 10,
105+
},
106+
},
107+
{
108+
name: "found in commits 2",
109+
wantErr: false,
110+
files: []string{},
111+
SearchCommits: []clients.Commit{{Committer: clients.User{ID: 111111111}},
112+
{Committer: clients.User{ID: dependabotID}},
113+
},
114+
115+
CallSearchCommits: 1,
116+
expected: scut.TestReturn{
117+
NumberOfInfo: 1,
118+
Score: 10,
119+
},
120+
},
121+
122+
{
123+
name: "many commits",
124+
wantErr: false,
125+
files: []string{
126+
".github/foobar.yml",
127+
},
128+
SearchCommits: []clients.Commit{{Committer: clients.User{ID: 111111111}},
129+
{Committer: clients.User{ID: dependabotID}},
130+
},
131+
CallSearchCommits: 1,
132+
expected: scut.TestReturn{
133+
NumberOfInfo: 1,
134+
Score: 10,
135+
},
136+
},
70137
}
71138
for _, tt := range tests {
72139
tt := tt
@@ -75,6 +142,7 @@ func TestDependencyUpdateTool(t *testing.T) {
75142
ctrl := gomock.NewController(t)
76143
mockRepo := mockrepo.NewMockRepoClient(ctrl)
77144
mockRepo.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil)
145+
mockRepo.EXPECT().SearchCommits(gomock.Any()).Return(tt.SearchCommits, nil).Times(tt.CallSearchCommits)
78146
dl := scut.TestDetailLogger{}
79147
c := &checker.CheckRequest{
80148
RepoClient: mockRepo,

‎checks/raw/dependency_update_tool.go‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import (
2323
"github.com/ossf/scorecard/v4/clients"
2424
)
2525

26+
const (
27+
dependabotID = 49699333
28+
)
29+
2630
// DependencyUpdateTool is the exported name for Depdendency-Update-Tool.
2731
func DependencyUpdateTool(c clients.RepoClient) (checker.DependencyUpdateToolData, error) {
2832
var tools []checker.Tool
@@ -31,7 +35,27 @@ func DependencyUpdateTool(c clients.RepoClient) (checker.DependencyUpdateToolDat
3135
return checker.DependencyUpdateToolData{}, fmt.Errorf("%w", err)
3236
}
3337

34-
// No error, return the tools.
38+
if len(tools) != 0 {
39+
return checker.DependencyUpdateToolData{Tools: tools}, nil
40+
}
41+
42+
commits, err := c.SearchCommits(clients.SearchCommitsOptions{Author: "dependabot[bot]"})
43+
if err != nil {
44+
return checker.DependencyUpdateToolData{}, fmt.Errorf("%w", err)
45+
}
46+
47+
for i := range commits {
48+
if commits[i].Committer.ID == dependabotID {
49+
tools = append(tools, checker.Tool{
50+
Name: "Dependabot",
51+
URL: asPointer("https://github.com/dependabot"),
52+
Desc: asPointer("Automated dependency updates built into GitHub"),
53+
Files: []checker.File{{}},
54+
})
55+
break
56+
}
57+
}
58+
3559
return checker.DependencyUpdateToolData{Tools: tools}, nil
3660
}
3761

‎checks/raw/dependency_update_tool_test.go‎

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/golang/mock/gomock"
2121

2222
"github.com/ossf/scorecard/v4/checker"
23+
clients "github.com/ossf/scorecard/v4/clients"
2324
mockrepo "github.com/ossf/scorecard/v4/clients/mockclients"
2425
)
2526

@@ -131,35 +132,59 @@ func TestDependencyUpdateTool(t *testing.T) {
131132
t.Parallel()
132133
//nolint
133134
tests := []struct {
134-
name string
135-
wantErr bool
136-
want int
137-
files []string
135+
name string
136+
wantErr bool
137+
want int
138+
SearchCommits []clients.Commit
139+
CallSearchCommits int
140+
files []string
138141
}{
139142
{
140-
name: "dependency update tool",
141-
wantErr: false,
142-
want: 1,
143+
name: "dependency update tool",
144+
wantErr: false,
145+
want: 1,
146+
CallSearchCommits: 0,
143147
files: []string{
144148
".github/dependabot.yml",
145149
},
146150
},
147151
{
148-
name: "dependency update tool",
149-
wantErr: false,
150-
want: 1,
152+
name: "dependency update tool",
153+
wantErr: false,
154+
want: 1,
155+
CallSearchCommits: 0,
151156
files: []string{
152157
".github/dependabot.yaml",
153158
},
154159
},
155160
{
156-
name: "foo bar",
157-
wantErr: false,
158-
want: 0,
161+
name: "foo bar",
162+
wantErr: false,
163+
want: 0,
164+
CallSearchCommits: 1,
165+
SearchCommits: []clients.Commit{{Committer: clients.User{ID: 111111111}}},
159166
files: []string{
160167
".github/foobar.yml",
161168
},
162169
},
170+
{
171+
name: "dependency update tool via commits",
172+
wantErr: false,
173+
want: 1,
174+
CallSearchCommits: 1,
175+
files: []string{},
176+
SearchCommits: []clients.Commit{{Committer: clients.User{ID: dependabotID}}},
177+
},
178+
{
179+
name: "dependency update tool via commits",
180+
wantErr: false,
181+
want: 1,
182+
CallSearchCommits: 1,
183+
files: []string{},
184+
SearchCommits: []clients.Commit{{Committer: clients.User{ID: 111111111}},
185+
{Committer: clients.User{ID: dependabotID}},
186+
},
187+
},
163188
}
164189
for _, tt := range tests {
165190
tt := tt
@@ -169,6 +194,7 @@ func TestDependencyUpdateTool(t *testing.T) {
169194
ctrl := gomock.NewController(t)
170195
mockRepo := mockrepo.NewMockRepoClient(ctrl)
171196
mockRepo.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil)
197+
mockRepo.EXPECT().SearchCommits(gomock.Any()).Return(tt.SearchCommits, nil).Times(tt.CallSearchCommits)
172198

173199
got, err := DependencyUpdateTool(mockRepo)
174200
if (err != nil) != tt.wantErr {

‎clients/githubrepo/client.go‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,22 @@ var (
3737

3838
// Client is GitHub-specific implementation of RepoClient.
3939
type Client struct {
40-
repourl *repoURL
41-
repo *github.Repository
42-
repoClient *github.Client
43-
graphClient *graphqlHandler
44-
contributors *contributorsHandler
45-
branches *branchesHandler
46-
releases *releasesHandler
47-
workflows *workflowsHandler
48-
checkruns *checkrunsHandler
49-
statuses *statusesHandler
50-
search *searchHandler
51-
webhook *webhookHandler
52-
languages *languagesHandler
53-
ctx context.Context
54-
tarball tarballHandler
40+
repourl *repoURL
41+
repo *github.Repository
42+
repoClient *github.Client
43+
graphClient *graphqlHandler
44+
contributors *contributorsHandler
45+
branches *branchesHandler
46+
releases *releasesHandler
47+
workflows *workflowsHandler
48+
checkruns *checkrunsHandler
49+
statuses *statusesHandler
50+
search *searchHandler
51+
searchCommits *searchCommitsHandler
52+
webhook *webhookHandler
53+
languages *languagesHandler
54+
ctx context.Context
55+
tarball tarballHandler
5556
}
5657

5758
// InitRepo sets up the GitHub repo in local storage for improving performance and GitHub token usage efficiency.
@@ -102,6 +103,9 @@ func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string) error {
102103
// Setup searchHandler.
103104
client.search.init(client.ctx, client.repourl)
104105

106+
// Setup searchCommitsHandler
107+
client.searchCommits.init(client.ctx, client.repourl)
108+
105109
// Setup webhookHandler.
106110
client.webhook.init(client.ctx, client.repourl)
107111

@@ -190,6 +194,11 @@ func (client *Client) Search(request clients.SearchRequest) (clients.SearchRespo
190194
return client.search.search(request)
191195
}
192196

197+
// SearchCommits implements RepoClient.SearchCommits
198+
func (client *Client) SearchCommits(request clients.SearchCommitsOptions) ([]clients.Commit, error) {
199+
return client.searchCommits.search(request)
200+
}
201+
193202
// Close implements RepoClient.Close.
194203
func (client *Client) Close() error {
195204
return client.tarball.cleanup()
@@ -231,6 +240,9 @@ func CreateGithubRepoClientWithTransport(ctx context.Context, rt http.RoundTripp
231240
search: &searchHandler{
232241
ghClient: client,
233242
},
243+
searchCommits: &searchCommitsHandler{
244+
ghClient: client,
245+
},
234246
webhook: &webhookHandler{
235247
ghClient: client,
236248
},

0 commit comments

Comments
 (0)