Skip to content

Commit

Permalink
add evaluation code
Browse files Browse the repository at this point in the history
Signed-off-by: cpanato <[email protected]>
  • Loading branch information
cpanato committed Mar 30, 2022
1 parent 84a58de commit b3be53e
Show file tree
Hide file tree
Showing 10 changed files with 228 additions and 54 deletions.
2 changes: 2 additions & 0 deletions checker/raw_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type RawResults struct {
BranchProtectionResults BranchProtectionsData
CodeReviewResults CodeReviewData
MaintainedResults MaintainedData
WebhookResults WebhooksData
}

// MaintainedData contains the raw results
Expand Down Expand Up @@ -79,6 +80,7 @@ type WebhooksData struct {
// WebhookData contains the raw results
// for webhook check.
type WebhookData struct {
Path string
ID int64
UsesAuthSecret bool
}
Expand Down
59 changes: 59 additions & 0 deletions checks/evaluation/webhooks.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// 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))
}
152 changes: 152 additions & 0 deletions checks/evaluation/webhooks_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
})
}
}
4 changes: 3 additions & 1 deletion checks/raw/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package raw

import (
"fmt"

"github.com/ossf/scorecard/v4/checker"
sce "github.com/ossf/scorecard/v4/errors"
)
Expand All @@ -33,10 +35,10 @@ func WebHook(c *checker.CheckRequest) (checker.WebhooksData, error) {

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)
Expand Down
3 changes: 3 additions & 0 deletions checks/raw/webhooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func TestWebhooks(t *testing.T) {
tests := []struct {
name string
err error
uri string
wantErr bool
expectedUsesAuthSecret int
expected scut.TestReturn
Expand Down Expand Up @@ -96,6 +97,8 @@ func TestWebhooks(t *testing.T) {
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
Expand Down
56 changes: 6 additions & 50 deletions checks/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,8 @@
package checks

import (
"fmt"
"strconv"
"strings"

"github.com/olekukonko/tablewriter"

"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"
)
Expand All @@ -47,50 +42,11 @@ func WebHooks(c *checker.CheckRequest) checker.CheckResult {
return checker.CreateRuntimeErrorResult(CheckWebHooks, e)
}

if len(rawData.Webhook) < 1 {
return checker.CreateMaxScoreResult(CheckWebHooks, "no webhooks defined")
}

hasNoSecretCount := 0
for _, hook := range rawData.Webhook {
if !hook.UsesAuthSecret {
c.Dlogger.Warn(&checker.LogMessage{
Path: fmt.Sprintf("https://%s/settings/hooks/%d", c.RepoClient.URI(), hook.ID),
Type: checker.FileTypeURL,
Text: "Webhook with no secret configured",
})
hasNoSecretCount++
}
// Set the raw results.
if c.RawResults != nil {
c.RawResults.WebhookResults = rawData
}

generateWebhookTable(c.RepoClient.URI(), rawData.Webhook)

if hasNoSecretCount == 0 {
return checker.CreateMaxScoreResult(CheckWebHooks, fmt.Sprintf("all %d hook(s) have a secret configured", len(rawData.Webhook)))
}

if len(rawData.Webhook) == hasNoSecretCount {
return checker.CreateMinScoreResult(CheckWebHooks, fmt.Sprintf("%d hook(s) do not have a secret configured", len(rawData.Webhook)))
}

return checker.CreateProportionalScoreResult(CheckWebHooks,
fmt.Sprintf("%d out of %d hook(s) with no secrets configured detected", hasNoSecretCount, len(rawData.Webhook)), hasNoSecretCount, len(rawData.Webhook))
}

func generateWebhookTable(repo string, data []checker.WebhookData) {
tableString := &strings.Builder{}
table := tablewriter.NewWriter(tableString)
table.SetAutoWrapText(false)
table.SetHeader([]string{"GitHub Webhook", "Uses Auth Secret"})

for _, hook := range data {
table.Append([]string{fmt.Sprintf("https://%s/settings/hooks/%d", repo, hook.ID), strconv.FormatBool(hook.UsesAuthSecret)})
// https: //github.com/cpanato/testing-ci-providers/settings/hooks/289347313
}

table.SetBorders(tablewriter.Border{Left: true, Top: true, Right: true, Bottom: true})
table.SetCenterSeparator("|")
table.Render()

fmt.Println(tableString.String())
// Return the score evaluation.
return evaluation.Webhooks(CheckWebHooks, c.Dlogger, &rawData)
}
1 change: 0 additions & 1 deletion clients/githubrepo/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ func (handler *webhookHandler) setup() error {
}

for _, hook := range hooks {

repoHook := &clients.Webhook{
ID: hook.GetID(),
UsesAuthSecret: getAuthSecret(hook.Config),
Expand Down
1 change: 1 addition & 0 deletions clients/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package clients

// Webhook represents VCS Webhook.
type Webhook struct {
Path string
ID int64
UsesAuthSecret bool
}
2 changes: 1 addition & 1 deletion docs/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ possible.

## Webhooks

Risk: `High` (service possibly accessible to third parties)
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.

Expand Down
2 changes: 1 addition & 1 deletion docs/checks/internal/checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ checks:
repos: GitHub
short: This check validate if the webhook defined in the repository have a token configured.
description: |
Risk: `High` (service possibly accessible to third parties)
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:
Expand Down

0 comments on commit b3be53e

Please sign in to comment.