Replace checker.Webhook with clients.Webhook (#1948)

Co-authored-by: Azeem Shaikh <azeems@google.com>

checker/raw_result.go

@@ -107,15 +107,7 @@ type DependencyUpdateToolData struct {
 // WebhooksData contains the raw results
 // for the Webhook check.
 type WebhooksData struct {
-	Webhook []WebhookData
+	Webhooks []clients.Webhook
-}
-
-// WebhookData contains the raw results
-// for webhook check.
-type WebhookData struct {
-	Path           string
-	ID             int64
-	UsesAuthSecret bool
 }
 
 // BranchProtectionsData contains the raw results

checks/evaluation/webhooks.go

@@ -30,12 +30,12 @@ func Webhooks(name string, dl checker.DetailLogger,
 		return checker.CreateRuntimeErrorResult(name, e)
 	}
 
-	if len(r.Webhook) < 1 {
+	if len(r.Webhooks) < 1 {
 		return checker.CreateMaxScoreResult(name, "no webhooks defined")
 	}
 
 	hasNoSecretCount := 0
-	for _, hook := range r.Webhook {
+	for _, hook := range r.Webhooks {
 		if !hook.UsesAuthSecret {
 			dl.Warn(&checker.LogMessage{
 				Path: hook.Path,
@@ -47,14 +47,14 @@ func Webhooks(name string, dl checker.DetailLogger,
 	}
 
 	if hasNoSecretCount == 0 {
-		return checker.CreateMaxScoreResult(name, fmt.Sprintf("all %d hook(s) have a secret configured", len(r.Webhook)))
+		return checker.CreateMaxScoreResult(name, fmt.Sprintf("all %d hook(s) have a secret configured", len(r.Webhooks)))
 	}
 
-	if len(r.Webhook) == hasNoSecretCount {
+	if len(r.Webhooks) == hasNoSecretCount {
-		return checker.CreateMinScoreResult(name, fmt.Sprintf("%d hook(s) do not have a secret configured", len(r.Webhook)))
+		return checker.CreateMinScoreResult(name, fmt.Sprintf("%d hook(s) do not have a secret configured", len(r.Webhooks)))
 	}
 
 	return checker.CreateProportionalScoreResult(name,
 		fmt.Sprintf("%d/%d hook(s) with no secrets configured detected",
-			hasNoSecretCount, len(r.Webhook)), hasNoSecretCount, len(r.Webhook))
+			hasNoSecretCount, len(r.Webhooks)), hasNoSecretCount, len(r.Webhooks))
 }

checks/evaluation/webhooks_test.go

@@ -18,6 +18,7 @@ import (
 	"testing"
 
 	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/clients"
 	scut "github.com/ossf/scorecard/v4/utests"
 )
 
@@ -61,7 +62,7 @@ func TestWebhooks(t *testing.T) {
 				name: "1 webhook with secret",
 				dl:   &scut.TestDetailLogger{},
 				r: &checker.WebhooksData{
-					Webhook: []checker.WebhookData{
+					Webhooks: []clients.Webhook{
 						{
 							Path:           "https://github.com/owner/repo/settings/hooks/1234",
 							ID:             1234,
@@ -80,7 +81,7 @@ func TestWebhooks(t *testing.T) {
 				name: "1 webhook with no secret",
 				dl:   &scut.TestDetailLogger{},
 				r: &checker.WebhooksData{
-					Webhook: []checker.WebhookData{
+					Webhooks: []clients.Webhook{
 						{
 							Path:           "https://github.com/owner/repo/settings/hooks/1234",
 							ID:             1234,
@@ -99,7 +100,7 @@ func TestWebhooks(t *testing.T) {
 				name: "many webhooks with no secret and with secret",
 				dl:   &scut.TestDetailLogger{},
 				r: &checker.WebhooksData{
-					Webhook: []checker.WebhookData{
+					Webhooks: []clients.Webhook{
 						{
 							Path:           "https://github.com/owner/repo/settings/hooks/1234",
 							ID:             1234,

checks/raw/webhook.go

@@ -15,8 +15,6 @@
 package raw
 
 import (
-	"fmt"
-
 	"github.com/ossf/scorecard/v4/checker"
 	sce "github.com/ossf/scorecard/v4/errors"
 )
@@ -28,21 +26,7 @@ func WebHook(c *checker.CheckRequest) (checker.WebhooksData, error) {
 		return checker.WebhooksData{},
 			sce.WithMessage(sce.ErrScorecardInternal, "Client.Repositories.ListWebhooks")
 	}
-
+	return checker.WebhooksData{
-	if len(hooksResp) < 1 {
+		Webhooks: hooksResp,
-		return checker.WebhooksData{}, nil
+	}, nil
-	}
-
-	hooks := []checker.WebhookData{}
-	for _, hook := range hooksResp {
-		v := checker.WebhookData{
-			ID:             hook.ID,
-			UsesAuthSecret: hook.UsesAuthSecret,
-			Path:           fmt.Sprintf("https://%s/settings/hooks/%d", c.RepoClient.URI(), hook.ID),
-			// Note: add fields if needed.
-		}
-		hooks = append(hooks, v)
-	}
-
-	return checker.WebhooksData{Webhook: hooks}, nil
 }

checks/raw/webhooks_test.go

@@ -37,12 +37,12 @@ func TestWebhooks(t *testing.T) {
 		wantErr                bool
 		expectedUsesAuthSecret int
 		expected               scut.TestReturn
-		webhookResponse        []*clients.Webhook
+		webhookResponse        []clients.Webhook
 	}{
 		{
 			name:            "No Webhooks",
 			wantErr:         false,
-			webhookResponse: []*clients.Webhook{},
+			webhookResponse: []clients.Webhook{},
 		},
 		{
 			name:    "Error getting webhook",
@@ -53,7 +53,7 @@ func TestWebhooks(t *testing.T) {
 			name:                   "Webhook with no secret",
 			wantErr:                false,
 			expectedUsesAuthSecret: 0,
-			webhookResponse: []*clients.Webhook{
+			webhookResponse: []clients.Webhook{
 				{
 					UsesAuthSecret: false,
 				},
@@ -63,7 +63,7 @@ func TestWebhooks(t *testing.T) {
 			name:                   "Webhook with secrets",
 			wantErr:                false,
 			expectedUsesAuthSecret: 2,
-			webhookResponse: []*clients.Webhook{
+			webhookResponse: []clients.Webhook{
 				{
 					UsesAuthSecret: true,
 				},
@@ -76,7 +76,7 @@ func TestWebhooks(t *testing.T) {
 			name:                   "Webhook with secrets and some without defined secrets",
 			wantErr:                false,
 			expectedUsesAuthSecret: 1,
-			webhookResponse: []*clients.Webhook{
+			webhookResponse: []clients.Webhook{
 				{
 					UsesAuthSecret: true,
 				},
@@ -99,7 +99,7 @@ func TestWebhooks(t *testing.T) {
 
 			mockRepo.EXPECT().URI().Return(tt.uri).AnyTimes()
 
-			mockRepo.EXPECT().ListWebhooks().DoAndReturn(func() ([]*clients.Webhook, error) {
+			mockRepo.EXPECT().ListWebhooks().DoAndReturn(func() ([]clients.Webhook, error) {
 				if tt.err != nil {
 					return nil, tt.err
 				}
@@ -119,7 +119,7 @@ func TestWebhooks(t *testing.T) {
 			}
 			if !tt.wantErr {
 				gotHasSecret := 0
-				for _, gotHook := range got.Webhook {
+				for _, gotHook := range got.Webhooks {
 					if gotHook.UsesAuthSecret {
 						gotHasSecret++
 					}

checks/webhook_test.go

@@ -34,7 +34,7 @@ func TestWebhooks(t *testing.T) {
 		uri      string
 		err      error
 		name     string
-		webhooks []*clients.Webhook
+		webhooks []clients.Webhook
 	}{
 		{
 			name: "No Webhooks",
@@ -43,7 +43,7 @@ func TestWebhooks(t *testing.T) {
 				Score: 10,
 			},
 			err:      nil,
-			webhooks: []*clients.Webhook{},
+			webhooks: []clients.Webhook{},
 		},
 		{
 			name: "With Webhooks and secret set",
@@ -52,7 +52,7 @@ func TestWebhooks(t *testing.T) {
 				Score: 10,
 			},
 			err: nil,
-			webhooks: []*clients.Webhook{
+			webhooks: []clients.Webhook{
 				{
 					ID:             12345,
 					UsesAuthSecret: true,
@@ -66,7 +66,7 @@ func TestWebhooks(t *testing.T) {
 				Score: 0,
 			},
 			err: nil,
-			webhooks: []*clients.Webhook{
+			webhooks: []clients.Webhook{
 				{
 					ID:             12345,
 					UsesAuthSecret: false,
@@ -80,7 +80,7 @@ func TestWebhooks(t *testing.T) {
 				Score: 5,
 			},
 			err: nil,
-			webhooks: []*clients.Webhook{
+			webhooks: []clients.Webhook{
 				{
 					ID:             12345,
 					UsesAuthSecret: false,
@@ -102,7 +102,7 @@ func TestWebhooks(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			mockRepo := mockrepo.NewMockRepoClient(ctrl)
 
-			mockRepo.EXPECT().ListWebhooks().DoAndReturn(func() ([]*clients.Webhook, error) {
+			mockRepo.EXPECT().ListWebhooks().DoAndReturn(func() ([]clients.Webhook, error) {
 				if tt.err != nil {
 					return nil, tt.err
 				}

clients/githubrepo/client.go

@@ -155,7 +155,7 @@ func (client *Client) ListBranches() ([]*clients.BranchRef, error) {
 }
 
 // ListWebhooks implements RepoClient.ListWebhooks.
-func (client *Client) ListWebhooks() ([]*clients.Webhook, error) {
+func (client *Client) ListWebhooks() ([]clients.Webhook, error) {
 	return client.webhook.listWebhooks()
 }
 

clients/githubrepo/webhook.go

@@ -31,7 +31,7 @@ type webhookHandler struct {
 	ctx      context.Context
 	errSetup error
 	repourl  *repoURL
-	webhook  []*clients.Webhook
+	webhook  []clients.Webhook
 }
 
 func (handler *webhookHandler) init(ctx context.Context, repourl *repoURL) {
@@ -55,7 +55,7 @@ func (handler *webhookHandler) setup() error {
 		}
 
 		for _, hook := range hooks {
-			repoHook := &clients.Webhook{
+			repoHook := clients.Webhook{
 				ID:             hook.GetID(),
 				UsesAuthSecret: getAuthSecret(hook.Config),
 			}
@@ -76,7 +76,7 @@ func getAuthSecret(config map[string]interface{}) bool {
 	return false
 }
 
-func (handler *webhookHandler) listWebhooks() ([]*clients.Webhook, error) {
+func (handler *webhookHandler) listWebhooks() ([]clients.Webhook, error) {
 	if err := handler.setup(); err != nil {
 		return nil, fmt.Errorf("error during webhookHandler.setup: %w", err)
 	}

clients/localdir/client.go

@@ -202,7 +202,7 @@ func (client *localDirClient) ListStatuses(ref string) ([]clients.Status, error)
 }
 
 // ListWebhooks implements RepoClient.ListWebhooks.
-func (client *localDirClient) ListWebhooks() ([]*clients.Webhook, error) {
+func (client *localDirClient) ListWebhooks() ([]clients.Webhook, error) {
 	return nil, fmt.Errorf("ListWebhooks: %w", clients.ErrUnsupportedFeature)
 }
 

clients/mockclients/repo_client.go

@@ -258,10 +258,10 @@ func (mr *MockRepoClientMockRecorder) ListSuccessfulWorkflowRuns(filename interf
 }
 
 // ListWebhooks mocks base method.
-func (m *MockRepoClient) ListWebhooks() ([]*clients.Webhook, error) {
+func (m *MockRepoClient) ListWebhooks() ([]clients.Webhook, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "ListWebhooks")
-	ret0, _ := ret[0].([]*clients.Webhook)
+	ret0, _ := ret[0].([]clients.Webhook)
 	ret1, _ := ret[1].(error)
 	return ret0, ret1
 }

clients/repo_client.go

@@ -41,7 +41,7 @@ type RepoClient interface {
 	ListSuccessfulWorkflowRuns(filename string) ([]WorkflowRun, error)
 	ListCheckRunsForRef(ref string) ([]CheckRun, error)
 	ListStatuses(ref string) ([]Status, error)
-	ListWebhooks() ([]*Webhook, error)
+	ListWebhooks() ([]Webhook, error)
 	Search(request SearchRequest) (SearchResponse, error)
 	Close() error
 }