From a46313ca8a9cb9a966d4747ec81fe0bd7a9f5326 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 12 Apr 2022 11:45:38 +0000 Subject: [PATCH 01/10] :seedling: Bump cloud.google.com/go/pubsub from 1.19.0 to 1.20.0 Bumps [cloud.google.com/go/pubsub](https://github.com/googleapis/google-cloud-go) from 1.19.0 to 1.20.0. - [Release notes](https://github.com/googleapis/google-cloud-go/releases) - [Changelog](https://github.com/googleapis/google-cloud-go/blob/main/CHANGES.md) - [Commits](https://github.com/googleapis/google-cloud-go/compare/pubsub/v1.19.0...pubsub/v1.20.0) --- updated-dependencies: - dependency-name: cloud.google.com/go/pubsub dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 54110a8d95c..60defabcd67 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( require ( cloud.google.com/go/bigquery v1.30.2 cloud.google.com/go/monitoring v1.4.0 // indirect - cloud.google.com/go/pubsub v1.19.0 + cloud.google.com/go/pubsub v1.20.0 cloud.google.com/go/trace v1.2.0 // indirect contrib.go.opencensus.io/exporter/stackdriver v0.13.11 github.com/bombsimon/logrusr/v2 v2.0.1 diff --git a/go.sum b/go.sum index f137376abe1..521acabd0a1 100644 --- a/go.sum +++ b/go.sum @@ -71,8 +71,9 @@ cloud.google.com/go/pubsub v1.0.1/go.mod h1:R0Gpsv3s54REJCy4fxDixWD93lHJMoZTyQ2k cloud.google.com/go/pubsub v1.1.0/go.mod h1:EwwdRX2sKPjnvnqCa270oGRyludottCI76h+R3AArQw= cloud.google.com/go/pubsub v1.2.0/go.mod h1:jhfEVHT8odbXTkndysNHCcx0awwzvfOlguIAii9o8iA= cloud.google.com/go/pubsub v1.3.1/go.mod h1:i+ucay31+CNRpDW4Lu78I4xXG+O1r/MAHgjpRVR+TSU= -cloud.google.com/go/pubsub v1.19.0 h1:WZy66ga6/tqmZiwv1jwKVgqV8FuEuAmPR5CEJHNVCZk= cloud.google.com/go/pubsub v1.19.0/go.mod h1:/O9kmSe9bb9KRnIAWkzmqhPjHo6LtzGOBYd/kr06XSs= +cloud.google.com/go/pubsub v1.20.0 h1:QuwfH5BpEIE9T3n0YK3PKlsXi/TmHMu593UpOVlWygI= +cloud.google.com/go/pubsub v1.20.0/go.mod h1:IbztU2hgrVroK+PUU6H42CcL0ISZuyOOlrnReZj6Uv8= cloud.google.com/go/secretmanager v1.3.0/go.mod h1:+oLTkouyiYiabAQNugCeTS3PAArGiMJuBqvJnJsyH+U= cloud.google.com/go/storage v1.0.0/go.mod h1:IhtSnM/ZTZV8YYJWCY8RULGVqBDmpoyjwiyrjsg+URw= cloud.google.com/go/storage v1.5.0/go.mod h1:tpKbwo567HUNpVclU5sGELwQWBDZ8gh0ZeosJ0Rtdos= From 2873c0d58dfee453e436bf5c2dce2f9ee1c9d87c Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 12 Apr 2022 00:35:44 +0000 Subject: [PATCH 02/10] e2e for GITHUB_TOKEN --- .github/workflows/integration.yml | 78 ++++++++++++++++++++++++++++++- Makefile | 13 ++++-- e2e/branch_protection_test.go | 4 +- e2e/e2e_suite_test.go | 31 ++++++++++++ 4 files changed, 119 insertions(+), 7 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 1e487099351..c3e8710f74c 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -74,7 +74,7 @@ jobs: run: | go mod download - - name: Run E2E #using retry because the GitHub token is being throttled. + - name: Run PAT E2E #using retry because the GitHub token is being throttled. uses: nick-invision/retry@7f8f3d9f0f62fe5925341be21c2e8314fd4f7c7c env: GITHUB_AUTH_TOKEN: ${{ secrets.GH_AUTH_TOKEN }} @@ -82,7 +82,81 @@ jobs: max_attempts: 3 retry_on: error timeout_minutes: 30 - command: make e2e + command: make e2e-pat + + - name: codecov + uses: codecov/codecov-action@e3c560433a6cc60aec8812599b7844a7b4fa0d71 # 2.1.0 + with: + files: ./e2e-coverage.out + verbose: true + + - name: find comment + uses: peter-evans/find-comment@1769778a0c5bd330272d749d12c036d65e70d39d # v1.2.0 + id: fc + with: + issue-number: ${{ github.event.pull_request.number || github.event.client_payload.pull_request.number }} + comment-author: 'github-actions[bot]' + body-includes: Integration tests ran for + + - name: create or update comment + uses: peter-evans/create-or-update-comment@c9fcb64660bc90ec1cc535646af190c992007c32 # v1.4.5 + with: + issue-number: ${{ github.event.pull_request.number || github.event.client_payload.pull_request.number }} + comment-id: ${{ steps.fc.outputs.comment-id }} + body: | + Integration tests ${{ job.status }} for + [${{ github.event.client_payload.slash_command.args.named.sha || github.event.pull_request.head.sha }}] + (https://github.com/ossf/scorecard/actions/runs/${{ github.run_id }}) + + integration-gh-token: + runs-on: ubuntu-latest + steps: + - name: Harden Runner + uses: step-security/harden-runner@bdb12b622a910dfdc99a31fdfe6f45a16bc287a4 # v1 + with: + egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + + - name: pull_request actions/checkout + uses: actions/checkout@a12a3943b4bdde767164f792f33f40b04645d846 # v2.3.4 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: setup-go + uses: actions/setup-go@f6164bd8c8acb4a71fb2791a8b6c4024ff038dab # v2.2.0 + with: + go-version: '1.17' + + - name: Cache builds + # https://github.com/mvdan/github-actions-golang#how-do-i-set-up-caching-between-builds + uses: actions/cache@48af2dc4a9e8278b89d7fa154b955c30c6aaab09 #v2.1.7 + with: + # In order: + # * Module download cache + # * Build cache (Linux) + # * Build cache (Mac) + # * Build cache (Windows) + path: | + ~/go/pkg/mod + ~/.cache/go-build + ~/Library/Caches/go-build + %LocalAppData%\go-build + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + + - name: Prepare test env + run: | + go mod download + + - name: Run GITHUB_TOKEN E2E #using retry because the GitHub token is being throttled. + uses: nick-invision/retry@7f8f3d9f0f62fe5925341be21c2e8314fd4f7c7c + env: + GITHUB_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + max_attempts: 3 + retry_on: error + timeout_minutes: 30 + command: make e2e-gh-token - name: codecov uses: codecov/codecov-action@e3c560433a6cc60aec8812599b7844a7b4fa0d71 # 2.1.0 diff --git a/Makefile b/Makefile index 9fdfd19641a..cba83c4877e 100644 --- a/Makefile +++ b/Makefile @@ -277,7 +277,7 @@ cron-github-server-docker: ##@ Tests ################################# make test ################################### -test-targets = unit-test e2e ci-e2e +test-targets = unit-test e2e-pat e2e-gh-token ci-e2e .PHONY: test $(test-targets) test: $(test-targets) @@ -293,8 +293,13 @@ ifndef GITHUB_AUTH_TOKEN $(error GITHUB_AUTH_TOKEN is undefined) endif -e2e: ## Runs e2e tests. Requires GITHUB_AUTH_TOKEN env var to be set to GitHub personal access token -e2e: build-scorecard check-env | $(GINKGO) +e2e-pat: ## Runs e2e tests. Requires GITHUB_AUTH_TOKEN env var to be set to GitHub personal access token +e2e-pat: build-scorecard check-env | $(GINKGO) # Run e2e tests. GITHUB_AUTH_TOKEN with personal access token must be exported to run this - $(GINKGO) --race -p -v -cover -coverprofile=e2e-coverage.out ./... + TOKEN_TYPE="PAT" $(GINKGO) --race -p -v -cover -coverprofile=e2e-coverage.out ./... + +e2e-gh-token: ## Runs e2e tests. Requires GITHUB_AUTH_TOKEN env var to be set to default GITHUB_TOKEN +e2e-gh-token: build-scorecard check-env | $(GINKGO) + # Run e2e tests. GITHUB_AUTH_TOKEN set to secrets.GITHUB_TOKEN must be used to run this. + TOKEN_TYPE="GITHUB_TOKEN" $(GINKGO) --race -p -v -cover -coverprofile=e2e-coverage.out ./... ############################################################################### diff --git a/e2e/branch_protection_test.go b/e2e/branch_protection_test.go index 06e733db60f..fb56d749e6e 100644 --- a/e2e/branch_protection_test.go +++ b/e2e/branch_protection_test.go @@ -27,8 +27,10 @@ import ( scut "github.com/ossf/scorecard/v4/utests" ) -var _ = Describe("E2E TEST:"+checks.CheckBranchProtection, func() { +var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() { Context("E2E TEST:Validating branch protection", func() { + skipIfTokenIs(githubWorkflowDefaultTokenType, checks.CheckBranchProtection+" does not support GITHUB_TOKEN") + It("Should get non-admin branch protection on other repositories", func() { dl := scut.TestDetailLogger{} repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-branch-protection-e2e") diff --git a/e2e/e2e_suite_test.go b/e2e/e2e_suite_test.go index 0e587652ff4..5f81dfbc35e 100644 --- a/e2e/e2e_suite_test.go +++ b/e2e/e2e_suite_test.go @@ -15,6 +15,7 @@ package e2e import ( + "fmt" "os" "testing" @@ -35,6 +36,36 @@ func TestE2e(t *testing.T) { var logger *log.Logger +type tokenType int + +const ( + patTokenType tokenType = iota + githubWorkflowDefaultTokenType +) + +var tokType tokenType + +func skipIfTokenIs(t tokenType, msg string) { + if tokType == t { + Skip(msg) + } +} + +func skipIfTokenIsNot(t tokenType, msg string) { + if tokType != t { + Skip(msg) + } +} + var _ = BeforeSuite(func() { logger = log.NewLogger(log.DebugLevel) + tt := os.Getenv("TOKEN_TYPE") + switch tt { + case "PAT": + tokType = patTokenType + case "GITHUB_TOKEN": + tokType = githubWorkflowDefaultTokenType + default: + panic(fmt.Sprintf("invald TOKEN_TYPE: %s", tt)) + } }) From 4b2c67718554c53a0ef46590b625c347e561bb03 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 12 Apr 2022 00:39:32 +0000 Subject: [PATCH 03/10] fix --- e2e/branch_protection_test.go | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/e2e/branch_protection_test.go b/e2e/branch_protection_test.go index fb56d749e6e..3c1465fd808 100644 --- a/e2e/branch_protection_test.go +++ b/e2e/branch_protection_test.go @@ -29,7 +29,7 @@ import ( var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() { Context("E2E TEST:Validating branch protection", func() { - skipIfTokenIs(githubWorkflowDefaultTokenType, checks.CheckBranchProtection+" does not support GITHUB_TOKEN") + skipIfTokenIsNot(patTokenType, "PAT only") It("Should get non-admin branch protection on other repositories", func() { dl := scut.TestDetailLogger{} @@ -123,3 +123,28 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() { }) }) }) + +var _ = Describe("E2E TEST GITHUB_TOKEN:"+checks.CheckBranchProtection, func() { + Context("E2E TEST:Validating branch protection", func() { + skipIfTokenIsNot(githubWorkflowDefaultTokenType, "GITHUB_TOKEN only") + + It("Should get errors with GITHUB_TOKEN", func() { + dl := scut.TestDetailLogger{} + repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-branch-protection-e2e") + Expect(err).Should(BeNil()) + repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger) + err = repoClient.InitRepo(repo, clients.HeadSHA) + Expect(err).Should(BeNil()) + req := checker.CheckRequest{ + Ctx: context.Background(), + RepoClient: repoClient, + Repo: repo, + Dlogger: &dl, + } + + result := checks.BranchProtection(&req) + Expect(result.Error).ShouldNot(BeNil()) + Expect(repoClient.Close()).Should(BeNil()) + }) + }) +}) From 6a48f174ce582a60c40c4c3d31d57fed1bccfd67 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 12 Apr 2022 00:42:46 +0000 Subject: [PATCH 04/10] fix --- .github/workflows/integration.yml | 76 +++---------------------------- 1 file changed, 6 insertions(+), 70 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index c3e8710f74c..dd83bd6f855 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -74,89 +74,25 @@ jobs: run: | go mod download - - name: Run PAT E2E #using retry because the GitHub token is being throttled. + - name: Run GITHUB_TOKEN E2E #using retry because the GitHub token is being throttled. uses: nick-invision/retry@7f8f3d9f0f62fe5925341be21c2e8314fd4f7c7c env: - GITHUB_AUTH_TOKEN: ${{ secrets.GH_AUTH_TOKEN }} + GITHUB_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: max_attempts: 3 retry_on: error timeout_minutes: 30 - command: make e2e-pat - - - name: codecov - uses: codecov/codecov-action@e3c560433a6cc60aec8812599b7844a7b4fa0d71 # 2.1.0 - with: - files: ./e2e-coverage.out - verbose: true - - - name: find comment - uses: peter-evans/find-comment@1769778a0c5bd330272d749d12c036d65e70d39d # v1.2.0 - id: fc - with: - issue-number: ${{ github.event.pull_request.number || github.event.client_payload.pull_request.number }} - comment-author: 'github-actions[bot]' - body-includes: Integration tests ran for - - - name: create or update comment - uses: peter-evans/create-or-update-comment@c9fcb64660bc90ec1cc535646af190c992007c32 # v1.4.5 - with: - issue-number: ${{ github.event.pull_request.number || github.event.client_payload.pull_request.number }} - comment-id: ${{ steps.fc.outputs.comment-id }} - body: | - Integration tests ${{ job.status }} for - [${{ github.event.client_payload.slash_command.args.named.sha || github.event.pull_request.head.sha }}] - (https://github.com/ossf/scorecard/actions/runs/${{ github.run_id }}) - - integration-gh-token: - runs-on: ubuntu-latest - steps: - - name: Harden Runner - uses: step-security/harden-runner@bdb12b622a910dfdc99a31fdfe6f45a16bc287a4 # v1 - with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs - - - name: pull_request actions/checkout - uses: actions/checkout@a12a3943b4bdde767164f792f33f40b04645d846 # v2.3.4 - with: - ref: ${{ github.event.pull_request.head.sha }} - - - name: setup-go - uses: actions/setup-go@f6164bd8c8acb4a71fb2791a8b6c4024ff038dab # v2.2.0 - with: - go-version: '1.17' - - - name: Cache builds - # https://github.com/mvdan/github-actions-golang#how-do-i-set-up-caching-between-builds - uses: actions/cache@48af2dc4a9e8278b89d7fa154b955c30c6aaab09 #v2.1.7 - with: - # In order: - # * Module download cache - # * Build cache (Linux) - # * Build cache (Mac) - # * Build cache (Windows) - path: | - ~/go/pkg/mod - ~/.cache/go-build - ~/Library/Caches/go-build - %LocalAppData%\go-build - key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} - restore-keys: | - ${{ runner.os }}-go- - - - name: Prepare test env - run: | - go mod download + command: make e2e-gh-token - - name: Run GITHUB_TOKEN E2E #using retry because the GitHub token is being throttled. + - name: Run PAT E2E #using retry because the GitHub token is being throttled. uses: nick-invision/retry@7f8f3d9f0f62fe5925341be21c2e8314fd4f7c7c env: - GITHUB_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_AUTH_TOKEN: ${{ secrets.GH_AUTH_TOKEN }} with: max_attempts: 3 retry_on: error timeout_minutes: 30 - command: make e2e-gh-token + command: make e2e-pat - name: codecov uses: codecov/codecov-action@e3c560433a6cc60aec8812599b7844a7b4fa0d71 # 2.1.0 From eedd16d5be62a0dfdb48410a44d67d367263e027 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 12 Apr 2022 14:14:49 +0000 Subject: [PATCH 05/10] linter --- e2e/branch_protection_test.go | 2 ++ e2e/e2e_suite_test.go | 6 ------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/e2e/branch_protection_test.go b/e2e/branch_protection_test.go index 3c1465fd808..d8fa9362b4c 100644 --- a/e2e/branch_protection_test.go +++ b/e2e/branch_protection_test.go @@ -143,6 +143,8 @@ var _ = Describe("E2E TEST GITHUB_TOKEN:"+checks.CheckBranchProtection, func() { } result := checks.BranchProtection(&req) + // There should be an error with the GITHUB_TOKEN, until it's supported + // byt GitHub. Expect(result.Error).ShouldNot(BeNil()) Expect(repoClient.Close()).Should(BeNil()) }) diff --git a/e2e/e2e_suite_test.go b/e2e/e2e_suite_test.go index 5f81dfbc35e..2581e274b40 100644 --- a/e2e/e2e_suite_test.go +++ b/e2e/e2e_suite_test.go @@ -45,12 +45,6 @@ const ( var tokType tokenType -func skipIfTokenIs(t tokenType, msg string) { - if tokType == t { - Skip(msg) - } -} - func skipIfTokenIsNot(t tokenType, msg string) { if tokType != t { Skip(msg) From 91202855fd762d451557b005f524b3cf0a7b747a Mon Sep 17 00:00:00 2001 From: laurentsimon <64505099+laurentsimon@users.noreply.github.com> Date: Wed, 13 Apr 2022 09:16:38 -0700 Subject: [PATCH 06/10] Fix e2e branch (#1835) --- clients/githubrepo/branches_e2e_test.go | 2 ++ clients/githubrepo/githubrepo_suite_test.go | 26 +++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/clients/githubrepo/branches_e2e_test.go b/clients/githubrepo/branches_e2e_test.go index e905dd8083c..fa6178179e0 100644 --- a/clients/githubrepo/branches_e2e_test.go +++ b/clients/githubrepo/branches_e2e_test.go @@ -33,6 +33,8 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() { }) Context("E2E TEST: Validate query cost", func() { + skipIfTokenIsNot(githubWorkflowDefaultTokenType, "GITHUB_TOKEN only") + It("Should not have increased for HEAD query", func() { repourl := &repoURL{ owner: "ossf", diff --git a/clients/githubrepo/githubrepo_suite_test.go b/clients/githubrepo/githubrepo_suite_test.go index 13ecb8fb5f6..1fbb72b42f2 100644 --- a/clients/githubrepo/githubrepo_suite_test.go +++ b/clients/githubrepo/githubrepo_suite_test.go @@ -16,6 +16,7 @@ package githubrepo import ( "context" + "fmt" "net/http" "os" "testing" @@ -39,6 +40,21 @@ func TestGithubrepo(t *testing.T) { var graphClient *githubv4.Client +type tokenType int + +const ( + patTokenType tokenType = iota + githubWorkflowDefaultTokenType +) + +var tokType tokenType + +func skipIfTokenIsNot(t tokenType, msg string) { + if tokType != t { + Skip(msg) + } +} + var _ = BeforeSuite(func() { ctx := context.Background() logger := log.NewLogger(log.DebugLevel) @@ -47,4 +63,14 @@ var _ = BeforeSuite(func() { Transport: rt, } graphClient = githubv4.NewClient(httpClient) + + tt := os.Getenv("TOKEN_TYPE") + switch tt { + case "PAT": + tokType = patTokenType + case "GITHUB_TOKEN": + tokType = githubWorkflowDefaultTokenType + default: + panic(fmt.Sprintf("invald TOKEN_TYPE: %s", tt)) + } }) From b00b31646ab420019db9bdc87f79322863b9c9fb Mon Sep 17 00:00:00 2001 From: Caleb Brown Date: Wed, 13 Apr 2022 11:18:53 +1000 Subject: [PATCH 07/10] Split NewLogger into two so we can use a custom logrus instance. --- log/log.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/log/log.go b/log/log.go index 4dd71ae4cba..aff02625d0c 100644 --- a/log/log.go +++ b/log/log.go @@ -38,11 +38,16 @@ func NewLogger(logLevel Level) *Logger { logrusLevel := parseLogrusLevel(logLevel) logrusLog.SetLevel(logrusLevel) + return NewLogrusLogger(logrusLog) +} + +// NewLogrusLogger creates an instance of *Logger backed by the supplied +// logrusLog instance. +func NewLogrusLogger(logrusLog *logrus.Logger) *Logger { logrLogger := logrusr.New(logrusLog) logger := &Logger{ &logrLogger, } - return logger } From 410a145db2c79896a1572c96256d35e82eda7e2d Mon Sep 17 00:00:00 2001 From: laurentsimon <64505099+laurentsimon@users.noreply.github.com> Date: Wed, 13 Apr 2022 16:00:19 -0700 Subject: [PATCH 08/10] fix (#1837) --- clients/githubrepo/branches_e2e_test.go | 6 ++++-- e2e/branch_protection_test.go | 11 +++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/clients/githubrepo/branches_e2e_test.go b/clients/githubrepo/branches_e2e_test.go index fa6178179e0..626d1b14133 100644 --- a/clients/githubrepo/branches_e2e_test.go +++ b/clients/githubrepo/branches_e2e_test.go @@ -33,9 +33,9 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() { }) Context("E2E TEST: Validate query cost", func() { - skipIfTokenIsNot(githubWorkflowDefaultTokenType, "GITHUB_TOKEN only") - It("Should not have increased for HEAD query", func() { + skipIfTokenIsNot(githubWorkflowDefaultTokenType, "GITHUB_TOKEN only") + repourl := &repoURL{ owner: "ossf", repo: "scorecard", @@ -48,6 +48,8 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() { Expect(*brancheshandler.data.RateLimit.Cost).Should(BeNumerically("<=", 1)) }) It("Should fail for non-HEAD query", func() { + skipIfTokenIsNot(githubWorkflowDefaultTokenType, "GITHUB_TOKEN only") + repourl := &repoURL{ owner: "ossf", repo: "scorecard", diff --git a/e2e/branch_protection_test.go b/e2e/branch_protection_test.go index d8fa9362b4c..39fbb11550a 100644 --- a/e2e/branch_protection_test.go +++ b/e2e/branch_protection_test.go @@ -29,9 +29,9 @@ import ( var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() { Context("E2E TEST:Validating branch protection", func() { - skipIfTokenIsNot(patTokenType, "PAT only") - It("Should get non-admin branch protection on other repositories", func() { + skipIfTokenIsNot(patTokenType, "PAT only") + dl := scut.TestDetailLogger{} repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-branch-protection-e2e") Expect(err).Should(BeNil()) @@ -62,6 +62,8 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() { Expect(repoClient.Close()).Should(BeNil()) }) It("Should fail to return branch protection on other repositories", func() { + skipIfTokenIsNot(patTokenType, "PAT only") + dl := scut.TestDetailLogger{} repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-branch-protection-e2e-none") Expect(err).Should(BeNil()) @@ -92,6 +94,8 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() { Expect(repoClient.Close()).Should(BeNil()) }) It("Should fail to return branch protection on other repositories", func() { + skipIfTokenIsNot(patTokenType, "PAT only") + dl := scut.TestDetailLogger{} repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-branch-protection-e2e-patch-1") Expect(err).Should(BeNil()) @@ -126,9 +130,8 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() { var _ = Describe("E2E TEST GITHUB_TOKEN:"+checks.CheckBranchProtection, func() { Context("E2E TEST:Validating branch protection", func() { - skipIfTokenIsNot(githubWorkflowDefaultTokenType, "GITHUB_TOKEN only") - It("Should get errors with GITHUB_TOKEN", func() { + skipIfTokenIsNot(githubWorkflowDefaultTokenType, "GITHUB_TOKEN only") dl := scut.TestDetailLogger{} repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-branch-protection-e2e") Expect(err).Should(BeNil()) From c0e41f3a54ddd3ee7f0b273f4be5e7240e4c0ac9 Mon Sep 17 00:00:00 2001 From: laurentsimon <64505099+laurentsimon@users.noreply.github.com> Date: Wed, 13 Apr 2022 16:45:07 -0700 Subject: [PATCH 09/10] Update branches_e2e_test.go (#1838) --- clients/githubrepo/branches_e2e_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clients/githubrepo/branches_e2e_test.go b/clients/githubrepo/branches_e2e_test.go index 626d1b14133..0396e750f2c 100644 --- a/clients/githubrepo/branches_e2e_test.go +++ b/clients/githubrepo/branches_e2e_test.go @@ -34,7 +34,7 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() { Context("E2E TEST: Validate query cost", func() { It("Should not have increased for HEAD query", func() { - skipIfTokenIsNot(githubWorkflowDefaultTokenType, "GITHUB_TOKEN only") + skipIfTokenIsNot(patTokenType, "GITHUB_TOKEN only") repourl := &repoURL{ owner: "ossf", @@ -48,7 +48,7 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() { Expect(*brancheshandler.data.RateLimit.Cost).Should(BeNumerically("<=", 1)) }) It("Should fail for non-HEAD query", func() { - skipIfTokenIsNot(githubWorkflowDefaultTokenType, "GITHUB_TOKEN only") + skipIfTokenIsNot(patTokenType, "GITHUB_TOKEN only") repourl := &repoURL{ owner: "ossf", From 4d1c5316907e496cb98bcb397c17b994f5c4f296 Mon Sep 17 00:00:00 2001 From: laurentsimon <64505099+laurentsimon@users.noreply.github.com> Date: Wed, 13 Apr 2022 18:20:05 -0700 Subject: [PATCH 10/10] =?UTF-8?q?=E2=9C=A8=20Raw=20results=20for=20license?= =?UTF-8?q?=20(#1790)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Raw results for license * tests * tests * e2e fix * comment * fix * linter --- checker/raw_result.go | 7 ++ checks/evaluation/license.go | 45 +++++++++++ checks/license.go | 135 +++---------------------------- checks/license_test.go | 82 +------------------ checks/raw/license.go | 149 +++++++++++++++++++++++++++++++++++ checks/raw/license_test.go | 99 +++++++++++++++++++++++ e2e/license_test.go | 6 +- pkg/json_raw_results.go | 30 ++++++- 8 files changed, 346 insertions(+), 207 deletions(-) create mode 100644 checks/evaluation/license.go create mode 100644 checks/raw/license.go create mode 100644 checks/raw/license_test.go diff --git a/checker/raw_result.go b/checker/raw_result.go index c27b6a18d81..3e7cd484719 100644 --- a/checker/raw_result.go +++ b/checker/raw_result.go @@ -29,6 +29,7 @@ type RawResults struct { WebhookResults WebhooksData MaintainedResults MaintainedData SignedReleasesResults SignedReleasesData + LicenseResults LicenseData } // MaintainedData contains the raw results @@ -39,6 +40,12 @@ type MaintainedData struct { ArchivedStatus ArchivedStatus } +// LicenseData contains the raw results +// for the License check. +type LicenseData struct { + Files []File +} + // CodeReviewData contains the raw results // for the Code-Review check. type CodeReviewData struct { diff --git a/checks/evaluation/license.go b/checks/evaluation/license.go new file mode 100644 index 00000000000..d5ad18ee189 --- /dev/null +++ b/checks/evaluation/license.go @@ -0,0 +1,45 @@ +// 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 ( + "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" +) + +// License applies the score policy for the License check. +func License(name string, dl checker.DetailLogger, + r *checker.LicenseData, +) checker.CheckResult { + if r == nil { + e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data") + return checker.CreateRuntimeErrorResult(name, e) + } + + // Apply the policy evaluation. + if r.Files == nil || len(r.Files) == 0 { + return checker.CreateMinScoreResult(name, "license file not detected") + } + + for _, f := range r.Files { + dl.Info(&checker.LogMessage{ + Path: f.Path, + Type: checker.FileTypeSource, + Offset: 1, + }) + } + + return checker.CreateMaxScoreResult(name, "license file detected") +} diff --git a/checks/license.go b/checks/license.go index a3b242f27c8..6778b92fdb1 100644 --- a/checks/license.go +++ b/checks/license.go @@ -15,22 +15,12 @@ package checks import ( - "fmt" - "regexp" - "strings" - "github.com/ossf/scorecard/v4/checker" - "github.com/ossf/scorecard/v4/checks/fileparser" + "github.com/ossf/scorecard/v4/checks/evaluation" + "github.com/ossf/scorecard/v4/checks/raw" + sce "github.com/ossf/scorecard/v4/errors" ) -type check func(str string, extCheck []string) bool - -type checks struct { - rstr string // regex string - f check - p []string -} - // CheckLicense is the registered name for License. const CheckLicense = "License" @@ -40,123 +30,24 @@ func init() { checker.FileBased, checker.CommitBased, } - if err := registerCheck(CheckLicense, LicenseCheck, supportedRequestTypes); err != nil { + if err := registerCheck(CheckLicense, License, supportedRequestTypes); err != nil { // this should never happen panic(err) } } -const ( - copying = "copy(ing|right)" - license = "(un)?licen[sc]e" - preferredExt = "*\\.(md|markdown|html)$" - anyExt = ".[^./]" - ofl = "ofl" - patents = "patents" -) - -// Regex converted from -// https://github.com/licensee/licensee/blob/master/lib/licensee/project_files/license_file.rb -var ( - extensions = []string{"xml", "go", "gemspec"} - regexChecks = []checks{ - {rstr: copying, f: nil}, - {rstr: license, f: nil}, - {rstr: license + preferredExt, f: nil}, - {rstr: copying + preferredExt, f: nil}, - {rstr: copying + anyExt, f: nil}, - {rstr: ofl, f: nil}, - {rstr: ofl + preferredExt, f: nil}, - {rstr: patents, f: nil}, - {rstr: license, f: extensionMatch, p: []string{"spdx", "header"}}, - {rstr: license + "[-_][^.]*", f: extensionMatch, p: extensions}, - {rstr: copying + "[-_][^.]*", f: extensionMatch, p: extensions}, - {rstr: "\\w+[-_]" + license + "[^.]*", f: extensionMatch, p: extensions}, - {rstr: "\\w+[-_]" + copying + "[^.]*", f: extensionMatch, p: extensions}, - {rstr: ofl, f: extensionMatch, p: extensions}, - } -) - -// ExtensionMatch to check for matching extension. -func extensionMatch(f string, exts []string) bool { - s := strings.Split(f, ".") - - if len(s) <= 1 { - return false - } - - fext := s[len(s)-1] - - found := false - for _, ext := range exts { - if ext == fext { - found = true - break - } - } - - return found -} - -// TestLicenseCheck used for testing purposes. -func testLicenseCheck(name string) bool { - return checkLicense(name) -} - -// LicenseCheck runs LicenseCheck check. -func LicenseCheck(c *checker.CheckRequest) checker.CheckResult { - var s string - - err := fileparser.OnAllFilesDo(c.RepoClient, isLicenseFile, &s) +// License runs License check. +func License(c *checker.CheckRequest) checker.CheckResult { + rawData, err := raw.License(c) if err != nil { - return checker.CreateRuntimeErrorResult(CheckLicense, err) - } - if s != "" { - c.Dlogger.Info(&checker.LogMessage{ - Path: s, - Type: checker.FileTypeSource, - Offset: 1, - }) - return checker.CreateMaxScoreResult(CheckLicense, "license file detected") + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return checker.CreateRuntimeErrorResult(CheckLicense, e) } - return checker.CreateMinScoreResult(CheckLicense, "license file not detected") -} -var isLicenseFile fileparser.DoWhileTrueOnFilename = func(name string, args ...interface{}) (bool, error) { - if len(args) != 1 { - return false, fmt.Errorf("isLicenseFile requires exactly one argument: %w", errInvalidArgLength) + // Set the raw results. + if c.RawResults != nil { + c.RawResults.LicenseResults = rawData } - s, ok := args[0].(*string) - if !ok { - return false, fmt.Errorf("isLicenseFile requires argument of type: *string: %w", errInvalidArgType) - } - if checkLicense(name) { - if s != nil { - *s = name - } - return false, nil - } - return true, nil -} - -// CheckLicense to check whether the name parameter fulfill license file criteria. -func checkLicense(name string) bool { - for _, check := range regexChecks { - rg := regexp.MustCompile(check.rstr) - nameLower := strings.ToLower(name) - t := rg.MatchString(nameLower) - if t { - extFound := true - - // check extension calling f function. - // f function will always be func extensionMatch(..) - if check.f != nil { - extFound = check.f(nameLower, check.p) - } - - return extFound - } - } - return false + return evaluation.License(CheckLicense, c.Dlogger, &rawData) } diff --git a/checks/license_test.go b/checks/license_test.go index d778bf15398..450b8626251 100644 --- a/checks/license_test.go +++ b/checks/license_test.go @@ -28,86 +28,6 @@ import ( scut "github.com/ossf/scorecard/v4/utests" ) -func TestLicenseFileCheck(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - filename string - }{ - { - name: "LICENSE.md", - filename: "LICENSE.md", - }, - { - name: "LICENSE", - filename: "LICENSE", - }, - { - name: "COPYING", - filename: "COPYING", - }, - { - name: "COPYING.md", - filename: "COPYING.md", - }, - { - name: "LICENSE.textile", - filename: "LICENSE.textile", - }, - { - name: "COPYING.textile", - filename: "COPYING.textile", - }, - { - name: "LICENSE-MIT", - filename: "LICENSE-MIT", - }, - { - name: "COPYING-MIT", - filename: "COPYING-MIT", - }, - { - name: "MIT-LICENSE-MIT", - filename: "MIT-LICENSE-MIT", - }, - { - name: "MIT-COPYING", - filename: "MIT-COPYING", - }, - { - name: "OFL.md", - filename: "OFL.md", - }, - { - name: "OFL.textile", - filename: "OFL.textile", - }, - { - name: "OFL", - filename: "OFL", - }, - { - name: "PATENTS", - filename: "PATENTS", - }, - { - name: "PATENTS.txt", - filename: "PATENTS.txt", - }, - } - 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() - s := testLicenseCheck(tt.filename) - if !s { - t.Fail() - } - }) - } -} - func TestLicenseFileSubdirectory(t *testing.T) { t.Parallel() @@ -167,7 +87,7 @@ func TestLicenseFileSubdirectory(t *testing.T) { Dlogger: &dl, } - res := LicenseCheck(&req) + res := License(&req) if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &res, &dl) { t.Fail() diff --git a/checks/raw/license.go b/checks/raw/license.go new file mode 100644 index 00000000000..61f66ee7573 --- /dev/null +++ b/checks/raw/license.go @@ -0,0 +1,149 @@ +// 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 raw + +import ( + "fmt" + "regexp" + "strings" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/checks/fileparser" +) + +type check func(str string, extCheck []string) bool + +type checks struct { + rstr string // regex string + f check + p []string +} + +const ( + copying = "copy(ing|right)" + license = "(un)?licen[sc]e" + preferredExt = "*\\.(md|markdown|html)$" + anyExt = ".[^./]" + ofl = "ofl" + patents = "patents" +) + +// Regex converted from +// https://github.com/licensee/licensee/blob/master/lib/licensee/project_files/license_file.rb +var ( + extensions = []string{"xml", "go", "gemspec"} + regexChecks = []checks{ + {rstr: copying, f: nil}, + {rstr: license, f: nil}, + {rstr: license + preferredExt, f: nil}, + {rstr: copying + preferredExt, f: nil}, + {rstr: copying + anyExt, f: nil}, + {rstr: ofl, f: nil}, + {rstr: ofl + preferredExt, f: nil}, + {rstr: patents, f: nil}, + {rstr: license, f: extensionMatch, p: []string{"spdx", "header"}}, + {rstr: license + "[-_][^.]*", f: extensionMatch, p: extensions}, + {rstr: copying + "[-_][^.]*", f: extensionMatch, p: extensions}, + {rstr: "\\w+[-_]" + license + "[^.]*", f: extensionMatch, p: extensions}, + {rstr: "\\w+[-_]" + copying + "[^.]*", f: extensionMatch, p: extensions}, + {rstr: ofl, f: extensionMatch, p: extensions}, + } +) + +// License retrieves the raw data for the License check. +func License(c *checker.CheckRequest) (checker.LicenseData, error) { + var results checker.LicenseData + var path string + + err := fileparser.OnAllFilesDo(c.RepoClient, isLicenseFile, &path) + if err != nil { + return results, fmt.Errorf("fileparser.OnAllFilesDo: %w", err) + } + + if path != "" { + results.Files = append(results.Files, + checker.File{ + Path: path, + Type: checker.FileTypeSource, + }) + } + + return results, nil +} + +// ExtensionMatch to check for matching extension. +func extensionMatch(f string, exts []string) bool { + s := strings.Split(f, ".") + + if len(s) <= 1 { + return false + } + + fext := s[len(s)-1] + + found := false + for _, ext := range exts { + if ext == fext { + found = true + break + } + } + + return found +} + +// TestLicense used for testing purposes. +func TestLicense(name string) bool { + return checkLicense(name) +} + +var isLicenseFile fileparser.DoWhileTrueOnFilename = func(name string, args ...interface{}) (bool, error) { + if len(args) != 1 { + return false, fmt.Errorf("isLicenseFile requires exactly one argument: %w", errInvalidArgLength) + } + s, ok := args[0].(*string) + if !ok { + return false, fmt.Errorf("isLicenseFile requires argument of type: *string: %w", errInvalidArgType) + } + if checkLicense(name) { + if s != nil { + *s = name + } + return false, nil + } + return true, nil +} + +// CheckLicense to check whether the name parameter fulfill license file criteria. +func checkLicense(name string) bool { + for _, check := range regexChecks { + rg := regexp.MustCompile(check.rstr) + + nameLower := strings.ToLower(name) + t := rg.MatchString(nameLower) + if t { + extFound := true + + // check extension calling f function. + // f function will always be func extensionMatch(..) + if check.f != nil { + extFound = check.f(nameLower, check.p) + } + + return extFound + } + } + return false +} diff --git a/checks/raw/license_test.go b/checks/raw/license_test.go new file mode 100644 index 00000000000..1f0d5920104 --- /dev/null +++ b/checks/raw/license_test.go @@ -0,0 +1,99 @@ +// Copyright 2020 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 ( + "testing" +) + +func TestLicenseFileCheck(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + filename string + }{ + { + name: "LICENSE.md", + filename: "LICENSE.md", + }, + { + name: "LICENSE", + filename: "LICENSE", + }, + { + name: "COPYING", + filename: "COPYING", + }, + { + name: "COPYING.md", + filename: "COPYING.md", + }, + { + name: "LICENSE.textile", + filename: "LICENSE.textile", + }, + { + name: "COPYING.textile", + filename: "COPYING.textile", + }, + { + name: "LICENSE-MIT", + filename: "LICENSE-MIT", + }, + { + name: "COPYING-MIT", + filename: "COPYING-MIT", + }, + { + name: "MIT-LICENSE-MIT", + filename: "MIT-LICENSE-MIT", + }, + { + name: "MIT-COPYING", + filename: "MIT-COPYING", + }, + { + name: "OFL.md", + filename: "OFL.md", + }, + { + name: "OFL.textile", + filename: "OFL.textile", + }, + { + name: "OFL", + filename: "OFL", + }, + { + name: "PATENTS", + filename: "PATENTS", + }, + { + name: "PATENTS.txt", + filename: "PATENTS.txt", + }, + } + 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() + s := TestLicense(tt.filename) + if !s { + t.Fail() + } + }) + } +} diff --git a/e2e/license_test.go b/e2e/license_test.go index fdcd45c9156..8efa1f53003 100644 --- a/e2e/license_test.go +++ b/e2e/license_test.go @@ -52,7 +52,7 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() { NumberOfInfo: 1, NumberOfDebug: 0, } - result := checks.LicenseCheck(&req) + result := checks.License(&req) Expect(result.Error).Should(BeNil()) Expect(result.Pass).Should(BeTrue()) @@ -80,7 +80,7 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() { NumberOfInfo: 1, NumberOfDebug: 0, } - result := checks.LicenseCheck(&req) + result := checks.License(&req) Expect(result.Error).Should(BeNil()) Expect(result.Pass).Should(BeTrue()) @@ -120,7 +120,7 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() { NumberOfInfo: 1, NumberOfDebug: 0, } - result := checks.LicenseCheck(&req) + result := checks.License(&req) Expect(result.Error).Should(BeNil()) Expect(result.Pass).Should(BeTrue()) diff --git a/pkg/json_raw_results.go b/pkg/json_raw_results.go index c75eeab2172..3f150ee4082 100644 --- a/pkg/json_raw_results.go +++ b/pkg/json_raw_results.go @@ -131,8 +131,16 @@ type jsonReleaseAsset struct { URL string `json:"url"` } +//nolint +type jsonLicense struct { + File jsonFile `json:"file"` + // TODO: add fields, like type of license, etc. +} + //nolint type jsonRawResults struct { + // License. + Licenses []jsonLicense `json:"licenses"` // List of recent issues. RecentIssues []jsonIssue `json:"issues"` // List of vulnerabilities. @@ -279,6 +287,21 @@ func (r *jsonScorecardRawResult) addCodeReviewRawResults(cr *checker.CodeReviewD return r.setDefaultCommitData(cr.DefaultBranchCommits) } +//nolint:unparam +func (r *jsonScorecardRawResult) addLicenseRawResults(ld *checker.LicenseData) error { + r.Results.Licenses = []jsonLicense{} + for _, file := range ld.Files { + r.Results.Licenses = append(r.Results.Licenses, + jsonLicense{ + File: jsonFile{ + Path: file.Path, + }, + }, + ) + } + return nil +} + //nolint:unparam func (r *jsonScorecardRawResult) addVulnerbilitiesRawResults(vd *checker.VulnerabilitiesData) error { r.Results.DatabaseVulnerabilities = []jsonDatabaseVulnerability{} @@ -364,7 +387,12 @@ func (r *jsonScorecardRawResult) addBranchProtectionRawResults(bp *checker.Branc } func (r *jsonScorecardRawResult) fillJSONRawResults(raw *checker.RawResults) error { - // Vulnerabiliries. + // Licenses. + if err := r.addLicenseRawResults(&raw.LicenseResults); err != nil { + return sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + } + + // Vulnerabilities. if err := r.addVulnerbilitiesRawResults(&raw.VulnerabilitiesResults); err != nil { return sce.WithMessage(sce.ErrScorecardInternal, err.Error()) }