From 7dcb3cb3e212a0c06f784e6f9da4d5d680e96bd3 Mon Sep 17 00:00:00 2001 From: Carlos Tadeu Panato Junior Date: Thu, 31 Mar 2022 16:29:59 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20checks:=20add=20GitHub=20Webhook=20?= =?UTF-8?q?check=20(#1675)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * checks: add GitHub Webhook check Signed-off-by: Carlos Panato * update per feedback Signed-off-by: cpanato * add evaluation code Signed-off-by: cpanato * add feature gate check Signed-off-by: cpanato * fix lint Signed-off-by: cpanato --- checker/raw_result.go | 15 +++ checks/evaluation/webhooks.go | 60 ++++++++++++ checks/evaluation/webhooks_test.go | 152 +++++++++++++++++++++++++++++ checks/raw/webhook.go | 48 +++++++++ checks/raw/webhooks_test.go | 138 ++++++++++++++++++++++++++ checks/webhook.go | 65 ++++++++++++ checks/webhook_test.go | 142 +++++++++++++++++++++++++++ clients/githubrepo/client.go | 12 +++ clients/githubrepo/webhook.go | 84 ++++++++++++++++ clients/localdir/client.go | 5 + clients/mockclients/repo_client.go | 20 +++- clients/repo_client.go | 1 + clients/webhook.go | 22 +++++ docs/checks.md | 10 ++ docs/checks/internal/checks.yaml | 17 ++++ 15 files changed, 786 insertions(+), 5 deletions(-) create mode 100644 checks/evaluation/webhooks.go create mode 100644 checks/evaluation/webhooks_test.go create mode 100644 checks/raw/webhook.go create mode 100644 checks/raw/webhooks_test.go create mode 100644 checks/webhook.go create mode 100644 checks/webhook_test.go create mode 100644 clients/githubrepo/webhook.go create mode 100644 clients/webhook.go diff --git a/checker/raw_result.go b/checker/raw_result.go index 41d4aea2..2fd8836a 100644 --- a/checker/raw_result.go +++ b/checker/raw_result.go @@ -25,6 +25,7 @@ type RawResults struct { DependencyUpdateToolResults DependencyUpdateToolData BranchProtectionResults BranchProtectionsData CodeReviewResults CodeReviewData + WebhookResults WebhooksData MaintainedResults MaintainedData } @@ -70,6 +71,20 @@ type DependencyUpdateToolData struct { Tools []Tool } +// WebhooksData contains the raw results +// for the Webhook check. +type WebhooksData struct { + Webhook []WebhookData +} + +// WebhookData contains the raw results +// for webhook check. +type WebhookData struct { + Path string + ID int64 + UsesAuthSecret bool +} + // BranchProtectionsData contains the raw results // for the Branch-Protection check. type BranchProtectionsData struct { diff --git a/checks/evaluation/webhooks.go b/checks/evaluation/webhooks.go new file mode 100644 index 00000000..038fcac1 --- /dev/null +++ b/checks/evaluation/webhooks.go @@ -0,0 +1,60 @@ +// Copyright 2021 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package evaluation + +import ( + "fmt" + + "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" +) + +// Webhooks applies the score policy for the Webhooks check. +func Webhooks(name string, dl checker.DetailLogger, + r *checker.WebhooksData, +) checker.CheckResult { + if r == nil { + e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data") + return checker.CreateRuntimeErrorResult(name, e) + } + + if len(r.Webhook) < 1 { + return checker.CreateMaxScoreResult(name, "no webhooks defined") + } + + hasNoSecretCount := 0 + for _, hook := range r.Webhook { + if !hook.UsesAuthSecret { + dl.Warn(&checker.LogMessage{ + Path: hook.Path, + Type: checker.FileTypeURL, + Text: "Webhook with no secret configured", + }) + hasNoSecretCount++ + } + } + + if hasNoSecretCount == 0 { + return checker.CreateMaxScoreResult(name, fmt.Sprintf("all %d hook(s) have a secret configured", len(r.Webhook))) + } + + if len(r.Webhook) == hasNoSecretCount { + return checker.CreateMinScoreResult(name, fmt.Sprintf("%d hook(s) do not have a secret configured", len(r.Webhook))) + } + + return checker.CreateProportionalScoreResult(name, + fmt.Sprintf("%d/%d hook(s) with no secrets configured detected", + hasNoSecretCount, len(r.Webhook)), hasNoSecretCount, len(r.Webhook)) +} diff --git a/checks/evaluation/webhooks_test.go b/checks/evaluation/webhooks_test.go new file mode 100644 index 00000000..340e68ee --- /dev/null +++ b/checks/evaluation/webhooks_test.go @@ -0,0 +1,152 @@ +// Copyright 2022 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package evaluation + +import ( + "testing" + + "github.com/ossf/scorecard/v4/checker" + scut "github.com/ossf/scorecard/v4/utests" +) + +// TestWebhooks tests the webhooks check. +func TestWebhooks(t *testing.T) { + t.Parallel() + //nolint + type args struct { + name string + dl checker.DetailLogger + r *checker.WebhooksData + } + tests := []struct { + name string + args args + want checker.CheckResult + wantErr bool + }{ + { + name: "r nil", + args: args{ + name: "test_webhook_check_pass", + dl: &scut.TestDetailLogger{}, + }, + wantErr: true, + }, + { + name: "no webhooks", + args: args{ + name: "no webhooks", + dl: &scut.TestDetailLogger{}, + r: &checker.WebhooksData{}, + }, + want: checker.CheckResult{ + Score: checker.MaxResultScore, + }, + }, + { + name: "1 webhook with secret", + args: args{ + name: "1 webhook with secret", + dl: &scut.TestDetailLogger{}, + r: &checker.WebhooksData{ + Webhook: []checker.WebhookData{ + { + Path: "https://github.com/owner/repo/settings/hooks/1234", + ID: 1234, + UsesAuthSecret: true, + }, + }, + }, + }, + want: checker.CheckResult{ + Score: 10, + }, + }, + { + name: "1 webhook with no secret", + args: args{ + name: "1 webhook with no secret", + dl: &scut.TestDetailLogger{}, + r: &checker.WebhooksData{ + Webhook: []checker.WebhookData{ + { + Path: "https://github.com/owner/repo/settings/hooks/1234", + ID: 1234, + UsesAuthSecret: false, + }, + }, + }, + }, + want: checker.CheckResult{ + Score: 0, + }, + }, + { + name: "many webhooks with no secret and with secret", + args: args{ + name: "many webhooks with no secret and with secret", + dl: &scut.TestDetailLogger{}, + r: &checker.WebhooksData{ + Webhook: []checker.WebhookData{ + { + Path: "https://github.com/owner/repo/settings/hooks/1234", + ID: 1234, + UsesAuthSecret: false, + }, + { + Path: "https://github.com/owner/repo/settings/hooks/1111", + ID: 1111, + UsesAuthSecret: true, + }, + { + Path: "https://github.com/owner/repo/settings/hooks/4444", + ID: 4444, + UsesAuthSecret: true, + }, + { + Path: "https://github.com/owner/repo/settings/hooks/3333", + ID: 3333, + UsesAuthSecret: false, + }, + { + Path: "https://github.com/owner/repo/settings/hooks/2222", + ID: 2222, + UsesAuthSecret: false, + }, + }, + }, + }, + want: checker.CheckResult{ + Score: 6, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := Webhooks(tt.args.name, tt.args.dl, tt.args.r) + if tt.wantErr { + if got.Error2 == nil { + t.Errorf("Webhooks() error = %v, wantErr %v", got.Error2, tt.wantErr) + } + } else { + if got.Score != tt.want.Score { + t.Errorf("Webhooks() = %v, want %v", got.Score, tt.want.Score) + } + } + }) + } +} diff --git a/checks/raw/webhook.go b/checks/raw/webhook.go new file mode 100644 index 00000000..31a93c20 --- /dev/null +++ b/checks/raw/webhook.go @@ -0,0 +1,48 @@ +// Copyright 2022 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package raw + +import ( + "fmt" + + "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" +) + +// WebHook retrieves the raw data for the WebHooks check. +func WebHook(c *checker.CheckRequest) (checker.WebhooksData, error) { + hooksResp, err := c.RepoClient.ListWebhooks() + if err != nil { + return checker.WebhooksData{}, + sce.WithMessage(sce.ErrScorecardInternal, "Client.Repositories.ListWebhooks") + } + + if len(hooksResp) < 1 { + return checker.WebhooksData{}, 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 +} diff --git a/checks/raw/webhooks_test.go b/checks/raw/webhooks_test.go new file mode 100644 index 00000000..25eb6c1d --- /dev/null +++ b/checks/raw/webhooks_test.go @@ -0,0 +1,138 @@ +// Copyright 2022 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package raw + +import ( + "context" + "testing" + + "github.com/golang/mock/gomock" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/clients" + mockrepo "github.com/ossf/scorecard/v4/clients/mockclients" + sce "github.com/ossf/scorecard/v4/errors" + scut "github.com/ossf/scorecard/v4/utests" +) + +func TestWebhooks(t *testing.T) { + t.Parallel() + //nolint + tests := []struct { + name string + err error + uri string + wantErr bool + expectedUsesAuthSecret int + expected scut.TestReturn + webhookResponse []*clients.Webhook + }{ + { + name: "No Webhooks", + wantErr: false, + webhookResponse: []*clients.Webhook{}, + }, + { + name: "Error getting webhook", + wantErr: true, + err: sce.ErrScorecardInternal, + }, + { + name: "Webhook with no secret", + wantErr: false, + expectedUsesAuthSecret: 0, + webhookResponse: []*clients.Webhook{ + { + UsesAuthSecret: false, + }, + }, + }, + { + name: "Webhook with secrets", + wantErr: false, + expectedUsesAuthSecret: 2, + webhookResponse: []*clients.Webhook{ + { + UsesAuthSecret: true, + }, + { + UsesAuthSecret: true, + }, + }, + }, + { + name: "Webhook with secrets and some without defined secrets", + wantErr: false, + expectedUsesAuthSecret: 1, + webhookResponse: []*clients.Webhook{ + { + UsesAuthSecret: true, + }, + { + UsesAuthSecret: false, + }, + { + UsesAuthSecret: false, + }, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockRepo := mockrepo.NewMockRepoClient(ctrl) + + mockRepo.EXPECT().URI().Return(tt.uri).AnyTimes() + + mockRepo.EXPECT().ListWebhooks().DoAndReturn(func() ([]*clients.Webhook, error) { + if tt.err != nil { + return nil, tt.err + } + + return tt.webhookResponse, nil + }).AnyTimes() + + dl := scut.TestDetailLogger{} + req := checker.CheckRequest{ + RepoClient: mockRepo, + Ctx: context.TODO(), + Dlogger: &dl, + } + got, err := WebHook(&req) + if (err != nil) != tt.wantErr { + t.Errorf("Webhooks() error = %v, wantErr %v", err, tt.wantErr) + } + if !tt.wantErr { + gotHasSecret := 0 + for _, gotHook := range got.Webhook { + if gotHook.UsesAuthSecret { + gotHasSecret++ + } + } + + if gotHasSecret != tt.expectedUsesAuthSecret { + t.Errorf("Webhooks() got = %v, want %v", gotHasSecret, tt.expectedUsesAuthSecret) + } + } + + if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &checker.CheckResult{}, &dl) { + t.Fatalf("Test %s failed", tt.name) + } + }) + } +} diff --git a/checks/webhook.go b/checks/webhook.go new file mode 100644 index 00000000..10ae1bf8 --- /dev/null +++ b/checks/webhook.go @@ -0,0 +1,65 @@ +// Copyright 2022 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package checks + +import ( + "os" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/checks/evaluation" + "github.com/ossf/scorecard/v4/checks/raw" + sce "github.com/ossf/scorecard/v4/errors" +) + +const ( + // CheckWebHooks is the registered name for WebHooks. + CheckWebHooks = "Webhooks" +) + +//nolint:gochecknoinits +func init() { + if err := registerCheck(CheckWebHooks, WebHooks, nil); err != nil { + // this should never happen + panic(err) + } +} + +// WebHooks run Webhooks check. +func WebHooks(c *checker.CheckRequest) checker.CheckResult { + // TODO: remove this check when v6 is released + _, enabled := os.LookupEnv("SCORECARD_V6") + if !enabled { + c.Dlogger.Warn(&checker.LogMessage{ + Text: "SCORECARD_V6 is not set, not running the Webhook check", + }) + + e := sce.WithMessage(sce.ErrorUnsupportedCheck, "SCORECARD_V6 is not set, not running the Webhook check") + return checker.CreateRuntimeErrorResult(CheckWebHooks, e) + } + + rawData, err := raw.WebHook(c) + if err != nil { + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return checker.CreateRuntimeErrorResult(CheckWebHooks, e) + } + + // Set the raw results. + if c.RawResults != nil { + c.RawResults.WebhookResults = rawData + } + + // Return the score evaluation. + return evaluation.Webhooks(CheckWebHooks, c.Dlogger, &rawData) +} diff --git a/checks/webhook_test.go b/checks/webhook_test.go new file mode 100644 index 00000000..4c601c83 --- /dev/null +++ b/checks/webhook_test.go @@ -0,0 +1,142 @@ +// Copyright 2022 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package checks + +import ( + "context" + "os" + "testing" + + "github.com/golang/mock/gomock" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/clients" + mockrepo "github.com/ossf/scorecard/v4/clients/mockclients" + scut "github.com/ossf/scorecard/v4/utests" +) + +func TestWebhooks(t *testing.T) { + t.Parallel() + tests := []struct { + expected checker.CheckResult + uri string + err error + name string + webhooks []*clients.Webhook + }{ + { + name: "No Webhooks", + uri: "github.com/owner/repo", + expected: checker.CheckResult{ + Pass: true, + Score: 10, + }, + err: nil, + webhooks: []*clients.Webhook{}, + }, + { + name: "With Webhooks and secret set", + uri: "github.com/owner/repo", + expected: checker.CheckResult{ + Pass: true, + Score: 10, + }, + err: nil, + webhooks: []*clients.Webhook{ + { + ID: 12345, + UsesAuthSecret: true, + }, + }, + }, + { + name: "With Webhooks and no secret set", + uri: "github.com/owner/repo", + expected: checker.CheckResult{ + Pass: false, + Score: 0, + }, + err: nil, + webhooks: []*clients.Webhook{ + { + ID: 12345, + UsesAuthSecret: false, + }, + }, + }, + { + name: "With 2 Webhooks with and whitout secrets configured", + uri: "github.com/owner/repo", + expected: checker.CheckResult{ + Pass: false, + Score: 5, + }, + err: nil, + webhooks: []*clients.Webhook{ + { + ID: 12345, + UsesAuthSecret: false, + }, + { + ID: 54321, + UsesAuthSecret: true, + }, + }, + }, + } + + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + os.Setenv("SCORECARD_V6", "true") + ctrl := gomock.NewController(t) + mockRepo := mockrepo.NewMockRepoClient(ctrl) + + mockRepo.EXPECT().ListWebhooks().DoAndReturn(func() ([]*clients.Webhook, error) { + if tt.err != nil { + return nil, tt.err + } + return tt.webhooks, tt.err + }).MaxTimes(1) + + mockRepo.EXPECT().URI().Return(tt.uri).AnyTimes() + + dl := scut.TestDetailLogger{} + req := checker.CheckRequest{ + RepoClient: mockRepo, + Ctx: context.TODO(), + Dlogger: &dl, + } + res := WebHooks(&req) + if tt.err != nil { + if res.Error2 == nil { + t.Errorf("Expected error %v, got nil", tt.err) + } + // return as we don't need to check the rest of the fields. + return + } + + if res.Score != tt.expected.Score { + t.Errorf("Expected score %d, got %d for %v", tt.expected.Score, res.Score, tt.name) + } + if res.Pass != tt.expected.Pass { + t.Errorf("Expected pass %t, got %t for %v", tt.expected.Pass, res.Pass, tt.name) + } + ctrl.Finish() + }) + } +} diff --git a/clients/githubrepo/client.go b/clients/githubrepo/client.go index e821408d..cb7485f7 100644 --- a/clients/githubrepo/client.go +++ b/clients/githubrepo/client.go @@ -45,6 +45,7 @@ type Client struct { checkruns *checkrunsHandler statuses *statusesHandler search *searchHandler + webhook *webhookHandler ctx context.Context tarball tarballHandler } @@ -97,6 +98,9 @@ func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string) error { // Setup searchHandler. client.search.init(client.ctx, client.repourl) + // Setup webhookHandler. + client.webhook.init(client.ctx, client.repourl) + return nil } @@ -150,6 +154,11 @@ func (client *Client) ListBranches() ([]*clients.BranchRef, error) { return client.branches.listBranches() } +// ListWebhooks implements RepoClient.ListWebhooks. +func (client *Client) ListWebhooks() ([]*clients.Webhook, error) { + return client.webhook.listWebhooks() +} + // ListSuccessfulWorkflowRuns implements RepoClient.WorkflowRunsByFilename. func (client *Client) ListSuccessfulWorkflowRuns(filename string) ([]clients.WorkflowRun, error) { return client.workflows.listSuccessfulWorkflowRuns(filename) @@ -211,6 +220,9 @@ func CreateGithubRepoClientWithTransport(ctx context.Context, rt http.RoundTripp search: &searchHandler{ ghClient: client, }, + webhook: &webhookHandler{ + ghClient: client, + }, } } diff --git a/clients/githubrepo/webhook.go b/clients/githubrepo/webhook.go new file mode 100644 index 00000000..54d78aa2 --- /dev/null +++ b/clients/githubrepo/webhook.go @@ -0,0 +1,84 @@ +// Copyright 2022 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package githubrepo + +import ( + "context" + "fmt" + "strings" + "sync" + + "github.com/google/go-github/v38/github" + + "github.com/ossf/scorecard/v4/clients" +) + +type webhookHandler struct { + ghClient *github.Client + once *sync.Once + ctx context.Context + errSetup error + repourl *repoURL + webhook []*clients.Webhook +} + +func (handler *webhookHandler) init(ctx context.Context, repourl *repoURL) { + handler.ctx = ctx + handler.repourl = repourl + handler.errSetup = nil + handler.once = new(sync.Once) +} + +func (handler *webhookHandler) setup() error { + handler.once.Do(func() { + if !strings.EqualFold(handler.repourl.commitSHA, clients.HeadSHA) { + handler.errSetup = fmt.Errorf("%w: ListWebHooks only supported for HEAD queries", clients.ErrUnsupportedFeature) + return + } + hooks, _, err := handler.ghClient.Repositories.ListHooks( + handler.ctx, handler.repourl.owner, handler.repourl.repo, &github.ListOptions{}) + if err != nil { + handler.errSetup = fmt.Errorf("error during ListHooks: %w", err) + return + } + + for _, hook := range hooks { + repoHook := &clients.Webhook{ + ID: hook.GetID(), + UsesAuthSecret: getAuthSecret(hook.Config), + } + handler.webhook = append(handler.webhook, repoHook) + } + handler.errSetup = nil + }) + return handler.errSetup +} + +func getAuthSecret(config map[string]interface{}) bool { + if val, ok := config["secret"]; ok { + if val != nil { + return true + } + } + + return false +} + +func (handler *webhookHandler) listWebhooks() ([]*clients.Webhook, error) { + if err := handler.setup(); err != nil { + return nil, fmt.Errorf("error during webhookHandler.setup: %w", err) + } + return handler.webhook, nil +} diff --git a/clients/localdir/client.go b/clients/localdir/client.go index 3ff7c8d7..88fd3451 100644 --- a/clients/localdir/client.go +++ b/clients/localdir/client.go @@ -201,6 +201,11 @@ func (client *localDirClient) ListStatuses(ref string) ([]clients.Status, error) return nil, fmt.Errorf("ListStatuses: %w", clients.ErrUnsupportedFeature) } +// ListWebhooks implements RepoClient.ListWebhooks. +func (client *localDirClient) ListWebhooks() ([]*clients.Webhook, error) { + return nil, fmt.Errorf("ListWebhooks: %w", clients.ErrUnsupportedFeature) +} + // Search implements RepoClient.Search. func (client *localDirClient) Search(request clients.SearchRequest) (clients.SearchResponse, error) { return clients.SearchResponse{}, fmt.Errorf("Search: %w", clients.ErrUnsupportedFeature) diff --git a/clients/mockclients/repo_client.go b/clients/mockclients/repo_client.go index ee2979e9..354e7233 100644 --- a/clients/mockclients/repo_client.go +++ b/clients/mockclients/repo_client.go @@ -20,7 +20,6 @@ package mockrepo import ( - "fmt" reflect "reflect" gomock "github.com/golang/mock/gomock" @@ -155,11 +154,8 @@ func (mr *MockRepoClientMockRecorder) ListCheckRunsForRef(ref interface{}) *gomo // ListCommits mocks base method. func (m *MockRepoClient) ListCommits() ([]clients.Commit, error) { - fmt.Println("mock.ListCommits") m.ctrl.T.Helper() - fmt.Println("call mock.ListCommits") ret := m.ctrl.Call(m, "ListCommits") - fmt.Println("ret mock.ListCommits") ret0, _ := ret[0].([]clients.Commit) ret1, _ := ret[1].(error) return ret0, ret1 @@ -167,7 +163,6 @@ func (m *MockRepoClient) ListCommits() ([]clients.Commit, error) { // ListCommits indicates an expected call of ListCommits. func (mr *MockRepoClientMockRecorder) ListCommits() *gomock.Call { - fmt.Println("recorder.ListCommits") mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListCommits", reflect.TypeOf((*MockRepoClient)(nil).ListCommits)) } @@ -262,6 +257,21 @@ func (mr *MockRepoClientMockRecorder) ListSuccessfulWorkflowRuns(filename interf return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListSuccessfulWorkflowRuns", reflect.TypeOf((*MockRepoClient)(nil).ListSuccessfulWorkflowRuns), filename) } +// ListWebhooks mocks base method. +func (m *MockRepoClient) ListWebhooks() ([]*clients.Webhook, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListWebhooks") + ret0, _ := ret[0].([]*clients.Webhook) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListWebhooks indicates an expected call of ListWebhooks. +func (mr *MockRepoClientMockRecorder) ListWebhooks() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListWebhooks", reflect.TypeOf((*MockRepoClient)(nil).ListWebhooks)) +} + // Search mocks base method. func (m *MockRepoClient) Search(request clients.SearchRequest) (clients.SearchResponse, error) { m.ctrl.T.Helper() diff --git a/clients/repo_client.go b/clients/repo_client.go index 8024aada..8473c453 100644 --- a/clients/repo_client.go +++ b/clients/repo_client.go @@ -41,6 +41,7 @@ type RepoClient interface { ListSuccessfulWorkflowRuns(filename string) ([]WorkflowRun, error) ListCheckRunsForRef(ref string) ([]CheckRun, error) ListStatuses(ref string) ([]Status, error) + ListWebhooks() ([]*Webhook, error) Search(request SearchRequest) (SearchResponse, error) Close() error } diff --git a/clients/webhook.go b/clients/webhook.go new file mode 100644 index 00000000..0b3f464d --- /dev/null +++ b/clients/webhook.go @@ -0,0 +1,22 @@ +// Copyright 2022 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package clients + +// Webhook represents VCS Webhook. +type Webhook struct { + Path string + ID int64 + UsesAuthSecret bool +} diff --git a/docs/checks.md b/docs/checks.md index 73978634..997c0e61 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -601,3 +601,13 @@ possible. **Remediation steps** - Fix the vulnerabilities. The details of each vulnerability can be found on . +## Webhooks + +Risk: `Critical` (service possibly accessible to third parties) + +This check determines whether the webhook defined in the repository has a token configured to authenticate the origins of requests. + +**Remediation steps** +- Check whether your service supports token authentication. +- If there is support for token authentication, set the secret in the webhook configuration. See [Setting up a webhook](https://docs.github.com/en/developers/webhooks-and-events/webhooks/creating-webhooks#setting-up-a-webhook) +- If there is no support for token authentication, consider implementing it by following [these directions](https://docs.github.com/en/developers/webhooks-and-events/webhooks/securing-your-webhooks). diff --git a/docs/checks/internal/checks.yaml b/docs/checks/internal/checks.yaml index 354b4b52..37310f4d 100644 --- a/docs/checks/internal/checks.yaml +++ b/docs/checks/internal/checks.yaml @@ -731,3 +731,20 @@ checks: - >- Alternately, create a `LICENSE` directory and add license files with a name that matches your [SPDX license identifier](https://spdx.dev/ids/). + + Webhooks: + risk: High + tags: security, infrastructure + repos: GitHub + short: This check validate if the webhook defined in the repository have a token configured. + description: | + Risk: `Critical` (service possibly accessible to third parties) + + This check determines whether the webhook defined in the repository has a token configured to authenticate the origins of requests. + remediation: + - >- + Check whether your service supports token authentication. + - >- + If there is support for token authentication, set the secret in the webhook configuration. See [Setting up a webhook](https://docs.github.com/en/developers/webhooks-and-events/webhooks/creating-webhooks#setting-up-a-webhook) + - >- + If there is no support for token authentication, consider implementing it by following [these directions](https://docs.github.com/en/developers/webhooks-and-events/webhooks/securing-your-webhooks).