diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 48c3bc0d6fe8..2efdba16548e 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -2,7 +2,7 @@ name: Bug report about: Create a report for a problem you are encountering title: BUG -labels: bug +labels: kind/bug assignees: '' --- diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md index 6b7961bc61e8..af59d4db7c4b 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.md +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -2,7 +2,7 @@ name: Feature request about: Suggest an idea for this project title: Feature -labels: enhancement +labels: kind/enhancement assignees: '' --- diff --git a/.github/workflows/gitlab.yml b/.github/workflows/gitlab.yml index b20008129f50..00b836fbeeae 100644 --- a/.github/workflows/gitlab.yml +++ b/.github/workflows/gitlab.yml @@ -52,7 +52,7 @@ jobs: echo "go-mod=$(go env GOMODCACHE)" >> "$GITHUB_OUTPUT" - name: Cache builds # https://github.com/mvdan/github-actions-golang#how-do-i-set-up-caching-between-builds - uses: actions/cache@13aacd865c20de90d75de3b17ebe84f7a17d57d2 #v4.0.0 + uses: actions/cache@ab5e6d0c87105b4c9c2047343972218f562e4319 #v4.0.1 with: path: | ${{ steps.go-cache-paths.outputs.go-build }} diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 97e75b1e24f7..60cfdc15f122 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -63,7 +63,7 @@ jobs: echo "go-mod=$(go env GOMODCACHE)" >> "$GITHUB_OUTPUT" - name: Cache builds # https://github.com/mvdan/github-actions-golang#how-do-i-set-up-caching-between-builds - uses: actions/cache@13aacd865c20de90d75de3b17ebe84f7a17d57d2 #v4.0.0 + uses: actions/cache@ab5e6d0c87105b4c9c2047343972218f562e4319 #v4.0.1 with: path: | ${{ steps.go-cache-paths.outputs.go-build }} diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1c118f03639c..1d130212b2f1 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -54,7 +54,7 @@ jobs: echo "go-mod=$(go env GOMODCACHE)" >> "$GITHUB_OUTPUT" - name: Cache builds # https://github.com/mvdan/github-actions-golang#how-do-i-set-up-caching-between-builds - uses: actions/cache@13aacd865c20de90d75de3b17ebe84f7a17d57d2 #v4.0.0 + uses: actions/cache@ab5e6d0c87105b4c9c2047343972218f562e4319 #v4.0.1 with: path: | ${{ steps.go-cache-paths.outputs.go-build }} @@ -106,7 +106,7 @@ jobs: repo-token: ${{ secrets.GITHUB_TOKEN }} - name: Cache builds # https://github.com/mvdan/github-actions-golang#how-do-i-set-up-caching-between-builds - uses: actions/cache@13aacd865c20de90d75de3b17ebe84f7a17d57d2 # v4.0.0 + uses: actions/cache@ab5e6d0c87105b4c9c2047343972218f562e4319 # v4.0.1 with: path: | ~/go/pkg/mod @@ -226,7 +226,7 @@ jobs: egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs - name: Cache builds # https://github.com/mvdan/github-actions-golang#how-do-i-set-up-caching-between-builds - uses: actions/cache@13aacd865c20de90d75de3b17ebe84f7a17d57d2 # v4.0.0 + uses: actions/cache@ab5e6d0c87105b4c9c2047343972218f562e4319 # v4.0.1 with: path: | ~/go/pkg/mod @@ -266,7 +266,7 @@ jobs: - name: Cache builds # https://github.com/mvdan/github-actions-golang#how-do-i-set-up-caching-between-builds - uses: actions/cache@13aacd865c20de90d75de3b17ebe84f7a17d57d2 # v4.0.0 + uses: actions/cache@ab5e6d0c87105b4c9c2047343972218f562e4319 # v4.0.1 with: path: | ~/go/pkg/mod diff --git a/.github/workflows/slsa-goreleaser.yml b/.github/workflows/slsa-goreleaser.yml index 0d14c52106e7..8ef05204acc6 100644 --- a/.github/workflows/slsa-goreleaser.yml +++ b/.github/workflows/slsa-goreleaser.yml @@ -47,12 +47,12 @@ jobs: uses: slsa-framework/slsa-verifier/actions/installer@v2.4.1 - name: Download the artifact - uses: actions/download-artifact@eaceaf801fd36c7dee90939fad912460b18a1ffe # v4.1.2 + uses: actions/download-artifact@c850b930e6ba138125429b7e5c93fc707a7f8427 # v4.1.4 with: name: "${{ needs.build.outputs.go-binary-name }}.intoto.jsonl" - name: Download the artifact - uses: actions/download-artifact@eaceaf801fd36c7dee90939fad912460b18a1ffe # v4.1.2 + uses: actions/download-artifact@c850b930e6ba138125429b7e5c93fc707a7f8427 # v4.1.4 with: name: ${{ needs.build.outputs.go-binary-name }} diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index e0cbc5e2d04b..8529f1758617 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -34,11 +34,11 @@ jobs: - uses: actions/stale@28ca1036281a5e5922ead5184a1bbf96e5fc984e # v3.0.18 with: repo-token: ${{ secrets.GITHUB_TOKEN }} - stale-issue-message: 'This issue is stale because it has been open for 60 days with no activity.' - stale-pr-message: 'This pull request is stale because it has been open for 10 days with no activity' - exempt-issue-labels: 'priority,bug,good first issue,backlog,help wanted' + stale-issue-message: 'This issue has been marked stale because it has been open for 60 days with no activity.' + stale-pr-message: 'This pull request has been marked stale because it has been open for 10 days with no activity' + exempt-issue-labels: 'priority/must-do,kind/bug,good first issue,help wanted' exempt-issue-milestones: 'Structured results' - exempt-pr-labels: 'awaiting-approval,work-in-progress' + exempt-pr-labels: 'awaiting-approval' days-before-pr-stale: '10' days-before-pr-close: '20' days-before-issue-stale: '60' diff --git a/CONTRIBUTOR_LADDER.md b/CONTRIBUTOR_LADDER.md index e0b3c6762a1f..7dea2bf011c9 100644 --- a/CONTRIBUTOR_LADDER.md +++ b/CONTRIBUTOR_LADDER.md @@ -83,7 +83,7 @@ and software engineering principles. #### Pre-requisites -- Community Member for at least 3 months +- Community Member for at least 1 month - Helped to triage issues and pull requests - Knowledgeable about the codebase @@ -131,7 +131,7 @@ approval is focused on holistic acceptance of a contribution including: #### Pre-requisites -- Triager for at least 3 months +- Triager for at least 1 month - Reviewed at least 10 substantial PRs to the codebase - Reviewed or got at least 30 PRs merged to the codebase diff --git a/README.md b/README.md index 95a21a451e65..afa239a16c55 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ [![Go Report Card](https://goreportcard.com/badge/github.com/ossf/scorecard/v4)](https://goreportcard.com/report/github.com/ossf/scorecard/v4) [![codecov](https://codecov.io/gh/ossf/scorecard/branch/main/graph/badge.svg?token=PMJ6NAN9J3)](https://codecov.io/gh/ossf/scorecard) [![SLSA 3](https://slsa.dev/images/gh-badge-level3.svg)](https://slsa.dev) -[![Slack](https://img.shields.io/badge/slack-openssf/security_scorecards-white.svg?logo=slack)](https://slack.openssf.org/#scorecard) +[![Slack](https://img.shields.io/badge/slack-openssf/scorecard-white.svg?logo=slack)](https://slack.openssf.org/#scorecard) diff --git a/checks/branch_protection.go b/checks/branch_protection.go index bb48a863396e..33f6d60984d0 100644 --- a/checks/branch_protection.go +++ b/checks/branch_protection.go @@ -19,6 +19,8 @@ import ( "github.com/ossf/scorecard/v4/checks/evaluation" "github.com/ossf/scorecard/v4/checks/raw" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/probes" + "github.com/ossf/scorecard/v4/probes/zrunner" ) // CheckBranchProtection is the exported name for Branch-Protected check. @@ -34,17 +36,23 @@ func init() { // BranchProtection runs the Branch-Protection check. func BranchProtection(c *checker.CheckRequest) checker.CheckResult { - rawData, err := raw.BranchProtection(c.RepoClient) + rawData, err := raw.BranchProtection(c) if err != nil { e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) return checker.CreateRuntimeErrorResult(CheckBranchProtection, e) } - // Return raw results. - if c.RawResults != nil { - c.RawResults.BranchProtectionResults = rawData + // Set the raw results. + pRawResults := getRawResults(c) + pRawResults.BranchProtectionResults = rawData + + // Evaluate the probes. + findings, err := zrunner.Run(pRawResults, probes.BranchProtection) + if err != nil { + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return checker.CreateRuntimeErrorResult(CheckBranchProtection, e) } // Return the score evaluation. - return evaluation.BranchProtection(CheckBranchProtection, c.Dlogger, &rawData) + return evaluation.BranchProtection(CheckBranchProtection, findings, c.Dlogger) } diff --git a/checks/branch_protection_test.go b/checks/branch_protection_test.go index d56a4cce2bd5..3e55e834cbd3 100644 --- a/checks/branch_protection_test.go +++ b/checks/branch_protection_test.go @@ -174,7 +174,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { Error: nil, Score: 4, NumberOfWarn: 9, - NumberOfInfo: 12, + NumberOfInfo: 11, NumberOfDebug: 0, }, defaultBranch: main, @@ -232,7 +232,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { Error: nil, Score: 8, NumberOfWarn: 4, - NumberOfInfo: 18, + NumberOfInfo: 16, NumberOfDebug: 0, }, defaultBranch: main, @@ -363,9 +363,9 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { expected: scut.TestReturn{ Error: nil, Score: 0, - NumberOfWarn: 6, + NumberOfWarn: 4, NumberOfInfo: 0, - NumberOfDebug: 8, + NumberOfDebug: 10, }, nonadmin: true, defaultBranch: main, diff --git a/checks/evaluation/branch_protection.go b/checks/evaluation/branch_protection.go index f5848d521dc8..5e72958addd8 100644 --- a/checks/evaluation/branch_protection.go +++ b/checks/evaluation/branch_protection.go @@ -16,10 +16,22 @@ package evaluation import ( "fmt" + "strconv" "github.com/ossf/scorecard/v4/checker" - "github.com/ossf/scorecard/v4/clients" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/blocksDeleteOnBranches" + "github.com/ossf/scorecard/v4/probes/blocksForcePushOnBranches" + "github.com/ossf/scorecard/v4/probes/branchProtectionAppliesToAdmins" + "github.com/ossf/scorecard/v4/probes/branchesAreProtected" + "github.com/ossf/scorecard/v4/probes/dismissesStaleReviews" + "github.com/ossf/scorecard/v4/probes/requiresApproversForPullRequests" + "github.com/ossf/scorecard/v4/probes/requiresCodeOwnersReview" + "github.com/ossf/scorecard/v4/probes/requiresLastPushApproval" + "github.com/ossf/scorecard/v4/probes/requiresPRsToChangeCode" + "github.com/ossf/scorecard/v4/probes/requiresUpToDateBranches" + "github.com/ossf/scorecard/v4/probes/runsStatusChecksBeforeMerging" ) const ( @@ -60,41 +72,143 @@ const ( ) // BranchProtection runs Branch-Protection check. -func BranchProtection(name string, dl checker.DetailLogger, - r *checker.BranchProtectionsData, +func BranchProtection(name string, + findings []finding.Finding, dl checker.DetailLogger, ) checker.CheckResult { - var scores []levelScore - - // Check protections on all the branches. - for i := range r.Branches { - var score levelScore - b := r.Branches[i] - - // Protected field only indicates that the branch matches - // one `Branch protection rules`. All settings may be disabled, - // so it does not provide any guarantees. - protected := !(b.Protected != nil && !*b.Protected) - if !protected { + expectedProbes := []string{ + blocksDeleteOnBranches.Probe, + blocksForcePushOnBranches.Probe, + branchesAreProtected.Probe, + branchProtectionAppliesToAdmins.Probe, + dismissesStaleReviews.Probe, + requiresApproversForPullRequests.Probe, + requiresCodeOwnersReview.Probe, + requiresLastPushApproval.Probe, + requiresUpToDateBranches.Probe, + runsStatusChecksBeforeMerging.Probe, + requiresPRsToChangeCode.Probe, + } + + if !finding.UniqueProbesEqual(findings, expectedProbes) { + e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") + return checker.CreateRuntimeErrorResult(name, e) + } + + // Create a map branches and whether theyare protected + // Protected field only indates that the branch matches + // one `Branch protection rules`. All settings may be disabled, + // so it does not provide any guarantees. + protectedBranches := make(map[string]bool) + for i := range findings { + f := &findings[i] + if f.Outcome == finding.OutcomeNotApplicable { + return checker.CreateInconclusiveResult(name, + "unable to detect any development/release branches") + } + branchName, err := getBranchName(f) + if err != nil { + return checker.CreateRuntimeErrorResult(name, err) + } + // the order of this switch statement matters. + switch { + // Sanity check: + case f.Probe != branchesAreProtected.Probe: + continue + // Sanity check: + case branchName == "": + e := sce.WithMessage(sce.ErrScorecardInternal, "probe is missing branch name") + return checker.CreateRuntimeErrorResult(name, e) + // Now we can check whether the branch is protected: + case f.Outcome == finding.OutcomeNegative: + protectedBranches[branchName] = false dl.Warn(&checker.LogMessage{ - Text: fmt.Sprintf("branch protection not enabled for branch '%s'", *b.Name), + Text: fmt.Sprintf("branch protection not enabled for branch '%s'", branchName), }) + case f.Outcome == finding.OutcomePositive: + protectedBranches[branchName] = true + default: + continue } - score.scores.basic, score.maxes.basic = basicNonAdminProtection(&b, dl) - score.scores.review, score.maxes.review = nonAdminReviewProtection(&b) - score.scores.adminReview, score.maxes.adminReview = adminReviewProtection(&b, dl) - score.scores.context, score.maxes.context = nonAdminContextProtection(&b, dl) - score.scores.thoroughReview, score.maxes.thoroughReview = nonAdminThoroughReviewProtection(&b, dl) - // Do we want this? - score.scores.adminThoroughReview, score.maxes.adminThoroughReview = adminThoroughReviewProtection(&b, dl) - score.scores.codeownerReview, score.maxes.codeownerReview = codeownerBranchProtection(&b, r.CodeownersFiles, dl) - - scores = append(scores, score) } - if len(scores) == 0 { + branchScores := make(map[string]*levelScore) + + for i := range findings { + f := &findings[i] + if f.Outcome == finding.OutcomeNotApplicable { + return checker.CreateInconclusiveResult(name, "unable to detect any development/release branches") + } + + branchName, err := getBranchName(f) + if err != nil { + return checker.CreateRuntimeErrorResult(name, err) + } + if branchName == "" { + e := sce.WithMessage(sce.ErrScorecardInternal, "probe is missing branch name") + return checker.CreateRuntimeErrorResult(name, e) + } + + if _, ok := branchScores[branchName]; !ok { + branchScores[branchName] = &levelScore{} + } + + var score, max int + + doLogging := protectedBranches[branchName] + switch f.Probe { + case blocksDeleteOnBranches.Probe, blocksForcePushOnBranches.Probe: + score, max = deleteAndForcePushProtection(f, doLogging, dl) + branchScores[branchName].scores.basic += score + branchScores[branchName].maxes.basic += max + + case dismissesStaleReviews.Probe, branchProtectionAppliesToAdmins.Probe: + score, max = adminThoroughReviewProtection(f, doLogging, dl) + branchScores[branchName].scores.adminThoroughReview += score + branchScores[branchName].maxes.adminThoroughReview += max + + case requiresApproversForPullRequests.Probe: + // Scorecard evaluation scores twice with this probe: + // Once if the count is above 0 + // Once if the count is above 2 + score, max = nonAdminThoroughReviewProtection(f, doLogging, dl) + branchScores[branchName].scores.thoroughReview += score + branchScores[branchName].maxes.thoroughReview += max + + reviewerWeight := 2 + max = reviewerWeight + noOfRequiredReviewers, _ := strconv.Atoi(f.Values["numberOfRequiredReviewers"]) //nolint:errcheck + if f.Outcome == finding.OutcomePositive && noOfRequiredReviewers > 0 { + branchScores[branchName].scores.review += reviewerWeight + } + branchScores[branchName].maxes.review += max + + case requiresCodeOwnersReview.Probe: + score, max = codeownerBranchProtection(f, doLogging, dl) + branchScores[branchName].scores.codeownerReview += score + branchScores[branchName].maxes.codeownerReview += max + + case requiresUpToDateBranches.Probe, requiresLastPushApproval.Probe, + requiresPRsToChangeCode.Probe: + score, max = adminReviewProtection(f, doLogging, dl) + branchScores[branchName].scores.adminReview += score + branchScores[branchName].maxes.adminReview += max + + case runsStatusChecksBeforeMerging.Probe: + score, max = nonAdminContextProtection(f, doLogging, dl) + branchScores[branchName].scores.context += score + branchScores[branchName].maxes.context += max + } + } + + if len(branchScores) == 0 { return checker.CreateInconclusiveResult(name, "unable to detect any development/release branches") } + scores := make([]levelScore, 0, len(branchScores)) + for _, v := range branchScores { + scores = append(scores, *v) + } + score, err := computeFinalScore(scores) if err != nil { return checker.CreateRuntimeErrorResult(name, err) @@ -113,6 +227,14 @@ func BranchProtection(name string, dl checker.DetailLogger, } } +func getBranchName(f *finding.Finding) (string, error) { + name, ok := f.Values["branchName"] + if !ok { + return "", sce.WithMessage(sce.ErrScorecardInternal, "no branch name found") + } + return name, nil +} + func sumUpScoreForTier(t tier, scoresData []levelScore) int { sum := 0 for i := range scoresData { @@ -133,6 +255,39 @@ func sumUpScoreForTier(t tier, scoresData []levelScore) int { return sum } +func logWithDebug(f *finding.Finding, doLogging bool, dl checker.DetailLogger) { + switch f.Outcome { + case finding.OutcomeNotAvailable: + debug(dl, doLogging, f.Message) + case finding.OutcomePositive: + info(dl, doLogging, f.Message) + case finding.OutcomeNegative: + warn(dl, doLogging, f.Message) + default: + // To satisfy linter + } +} + +func logWithoutDebug(f *finding.Finding, doLogging bool, dl checker.DetailLogger) { + switch f.Outcome { + case finding.OutcomePositive: + info(dl, doLogging, f.Message) + case finding.OutcomeNegative: + warn(dl, doLogging, f.Message) + default: + // To satisfy linter + } +} + +func logInfoOrWarn(f *finding.Finding, doLogging bool, dl checker.DetailLogger) { + switch f.Outcome { + case finding.OutcomePositive: + info(dl, doLogging, f.Message) + default: + warn(dl, doLogging, f.Message) + } +} + func normalizeScore(score, max, level int) float64 { if max == 0 { return float64(level) @@ -226,208 +381,77 @@ func warn(dl checker.DetailLogger, doLogging bool, desc string, args ...interfac }) } -func basicNonAdminProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) { - score := 0 - max := 0 - // Only log information if the branch is protected. - log := branch.Protected != nil && *branch.Protected - - max++ - if branch.BranchProtectionRule.AllowForcePushes != nil { - switch *branch.BranchProtectionRule.AllowForcePushes { - case true: - warn(dl, log, "'force pushes' enabled on branch '%s'", *branch.Name) - case false: - info(dl, log, "'force pushes' disabled on branch '%s'", *branch.Name) - score++ - } +func deleteAndForcePushProtection(f *finding.Finding, doLogging bool, dl checker.DetailLogger) (int, int) { + var score, max int + logWithoutDebug(f, doLogging, dl) + if f.Outcome == finding.OutcomePositive { + score++ } - max++ - if branch.BranchProtectionRule.AllowDeletions != nil { - switch *branch.BranchProtectionRule.AllowDeletions { - case true: - warn(dl, log, "'allow deletion' enabled on branch '%s'", *branch.Name) - case false: - info(dl, log, "'allow deletion' disabled on branch '%s'", *branch.Name) - score++ - } - } return score, max } -func nonAdminContextProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) { - score := 0 - max := 0 - // Only log information if the branch is protected. - log := branch.Protected != nil && *branch.Protected - - // This means there are specific checks enabled. - // If only `Requires status check to pass before merging` is enabled - // but no specific checks are declared, it's equivalent - // to having no status check at all. - max++ - switch { - case len(branch.BranchProtectionRule.CheckRules.Contexts) > 0: - info(dl, log, "status check found to merge onto on branch '%s'", *branch.Name) +func nonAdminContextProtection(f *finding.Finding, doLogging bool, dl checker.DetailLogger) (int, int) { + var score, max int + logInfoOrWarn(f, doLogging, dl) + if f.Outcome == finding.OutcomePositive { score++ - default: - warn(dl, log, "no status checks found to merge onto branch '%s'", *branch.Name) } + max++ return score, max } -func nonAdminReviewProtection(branch *clients.BranchRef) (int, int) { - score := 0 - max := 0 - - // Having at least 1 reviewer is twice as important as the other Tier 2 requirements. - const reviewerWeight = 2 - max += reviewerWeight - if valueOrZero(branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount) > 0 { - // We do not display anything here, it's done in nonAdminThoroughReviewProtection() - score += reviewerWeight +func adminReviewProtection(f *finding.Finding, doLogging bool, dl checker.DetailLogger) (int, int) { + var score, max int + if f.Outcome == finding.OutcomePositive { + score++ + } + logWithDebug(f, doLogging, dl) + if f.Outcome != finding.OutcomeNotAvailable { + max++ } return score, max } -func adminReviewProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) { - score := 0 - max := 0 - - // Only log information if the branch is protected. - log := branch.Protected != nil && *branch.Protected +func adminThoroughReviewProtection(f *finding.Finding, doLogging bool, dl checker.DetailLogger) (int, int) { + var score, max int - // Process UpToDateBeforeMerge value. - if branch.BranchProtectionRule.CheckRules.UpToDateBeforeMerge == nil { - debug(dl, log, "unable to retrieve whether up-to-date branches are needed to merge on branch '%s'", *branch.Name) - } else { - // Note: `This setting will not take effect unless at least one status check is enabled`. - max++ - if *branch.BranchProtectionRule.CheckRules.UpToDateBeforeMerge { - info(dl, log, "status checks require up-to-date branches for '%s'", *branch.Name) - score++ - } else { - warn(dl, log, "status checks do not require up-to-date branches for '%s'", *branch.Name) - } + logWithDebug(f, doLogging, dl) + if f.Outcome == finding.OutcomePositive { + score++ } - - // Process RequireLastPushApproval value. - if branch.BranchProtectionRule.RequireLastPushApproval == nil { - debug(dl, log, "unable to retrieve whether 'last push approval' is required to merge on branch '%s'", *branch.Name) - } else { + if f.Outcome != finding.OutcomeNotAvailable { max++ - if *branch.BranchProtectionRule.RequireLastPushApproval { - info(dl, log, "'last push approval' enabled on branch '%s'", *branch.Name) - score++ - } else { - warn(dl, log, "'last push approval' disabled on branch '%s'", *branch.Name) - } - } - - max++ - if valueOrZero(branch.BranchProtectionRule.RequiredPullRequestReviews.Required) { - score++ - info(dl, log, "PRs are required in order to make changes on branch '%s'", *branch.Name) - } else { - warn(dl, log, "PRs are not required to make changes on branch '%s'; or we don't have data to detect it."+ - "If you think it might be the latter, make sure to run Scorecard with a PAT or use Repo "+ - "Rules (that are always public) instead of Branch Protection settings", *branch.Name) } - return score, max } -func adminThoroughReviewProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) { - score := 0 - max := 0 - // Only log information if the branch is protected. - log := branch.Protected != nil && *branch.Protected - - if branch.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews != nil { - // Note: we don't increase max possible score for non-admin viewers. - max++ - switch *branch.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews { - case true: - info(dl, log, "stale review dismissal enabled on branch '%s'", *branch.Name) - score++ - case false: - warn(dl, log, "stale review dismissal disabled on branch '%s'", *branch.Name) - } - } else { - debug(dl, log, "unable to retrieve review dismissal on branch '%s'", *branch.Name) - } - - // nil typically means we do not have access to the value. - if branch.BranchProtectionRule.EnforceAdmins != nil { - // Note: we don't increase max possible score for non-admin viewers. - max++ - switch *branch.BranchProtectionRule.EnforceAdmins { - case true: - info(dl, log, "settings apply to administrators on branch '%s'", *branch.Name) +func nonAdminThoroughReviewProtection(f *finding.Finding, doLogging bool, dl checker.DetailLogger) (int, int) { + var score, max int + if f.Outcome == finding.OutcomePositive { + noOfRequiredReviews, _ := strconv.Atoi(f.Values["numberOfRequiredReviewers"]) //nolint:errcheck + if noOfRequiredReviews >= minReviews { + info(dl, doLogging, f.Message) score++ - case false: - warn(dl, log, "settings do not apply to administrators on branch '%s'", *branch.Name) + } else { + warn(dl, doLogging, f.Message) } - } else { - debug(dl, log, "unable to retrieve whether or not settings apply to administrators on branch '%s'", *branch.Name) + } else if f.Outcome == finding.OutcomeNegative { + warn(dl, doLogging, f.Message) } - + max++ return score, max } -func nonAdminThoroughReviewProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) { - score := 0 - max := 0 - - // Only log information if the branch is protected. - log := branch.Protected != nil && *branch.Protected - - max++ - - reviewers := valueOrZero(branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount) - if reviewers >= minReviews { - info(dl, log, "number of required reviewers is %d on branch '%s'", reviewers, *branch.Name) +func codeownerBranchProtection(f *finding.Finding, doLogging bool, dl checker.DetailLogger) (int, int) { + var score, max int + if f.Outcome == finding.OutcomePositive { + info(dl, doLogging, f.Message) score++ } else { - warn(dl, log, "number of required reviewers is %d on branch '%s', while the ideal suggested is %d", - reviewers, *branch.Name, minReviews) + warn(dl, doLogging, f.Message) } - - return score, max -} - -func codeownerBranchProtection( - branch *clients.BranchRef, codeownersFiles []string, dl checker.DetailLogger, -) (int, int) { - score := 0 - max := 1 - - log := branch.Protected != nil && *branch.Protected - - if branch.BranchProtectionRule.RequiredPullRequestReviews.RequireCodeOwnerReviews != nil { - switch *branch.BranchProtectionRule.RequiredPullRequestReviews.RequireCodeOwnerReviews { - case true: - info(dl, log, "codeowner review is required on branch '%s'", *branch.Name) - if len(codeownersFiles) == 0 { - warn(dl, log, "codeowners branch protection is being ignored - but no codeowners file found in repo") - } else { - score++ - } - default: - warn(dl, log, "codeowner review is not required on branch '%s'", *branch.Name) - } - } - + max++ return score, max } - -// returns the pointer's value if it exists, the type's zero-value otherwise. -func valueOrZero[T any](ptr *T) T { - if ptr == nil { - var zero T - return zero - } - return *ptr -} diff --git a/checks/evaluation/branch_protection_test.go b/checks/evaluation/branch_protection_test.go index 0084e4e0a343..c0e0565b9554 100644 --- a/checks/evaluation/branch_protection_test.go +++ b/checks/evaluation/branch_protection_test.go @@ -18,562 +18,1349 @@ import ( "testing" "github.com/ossf/scorecard/v4/checker" - "github.com/ossf/scorecard/v4/clients" + sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/finding" scut "github.com/ossf/scorecard/v4/utests" ) -func testScore(branch *clients.BranchRef, codeownersFiles []string, dl checker.DetailLogger) (int, error) { - var score levelScore - score.scores.basic, score.maxes.basic = basicNonAdminProtection(branch, dl) - score.scores.review, score.maxes.review = nonAdminReviewProtection(branch) - score.scores.adminReview, score.maxes.adminReview = adminReviewProtection(branch, dl) - score.scores.context, score.maxes.context = nonAdminContextProtection(branch, dl) - score.scores.thoroughReview, score.maxes.thoroughReview = nonAdminThoroughReviewProtection(branch, dl) - score.scores.adminThoroughReview, score.maxes.adminThoroughReview = adminThoroughReviewProtection(branch, dl) - score.scores.codeownerReview, score.maxes.codeownerReview = codeownerBranchProtection(branch, codeownersFiles, dl) - - return computeFinalScore([]levelScore{score}) -} - -// TODO: order of tests to have progressive scores. -func TestIsBranchProtected(t *testing.T) { +func TestBranchProtection(t *testing.T) { t.Parallel() - trueVal := true - falseVal := false - var zeroVal int32 - var oneVal int32 = 1 - branchVal := "branch-name" tests := []struct { - name string - branch *clients.BranchRef - codeownersFiles []string - expected scut.TestReturn + name string + findings []finding.Finding + result scut.TestReturn }{ { - name: "GitHub default settings", - expected: scut.TestReturn{ - Error: nil, - Score: 3, - NumberOfWarn: 6, - NumberOfInfo: 2, - NumberOfDebug: 1, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - AllowDeletions: &falseVal, - AllowForcePushes: &falseVal, - RequireLinearHistory: &falseVal, - EnforceAdmins: &falseVal, - RequireLastPushApproval: &falseVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &falseVal, - }, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &trueVal, - Contexts: nil, - UpToDateBeforeMerge: &falseVal, + name: "Branch name is an empty string which is not allowed and will error", + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "", + "numberOfRequiredReviewers": "0", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "", }, }, }, - }, - { - name: "Nothing is enabled and values are nil", - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 3, - NumberOfInfo: 0, - NumberOfDebug: 4, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, + result: scut.TestReturn{ + Error: sce.ErrScorecardInternal, + Score: checker.InconclusiveResultScore, }, }, { name: "Required status check enabled", - expected: scut.TestReturn{ - Error: nil, - Score: 4, - NumberOfWarn: 5, - NumberOfInfo: 5, - NumberOfDebug: 0, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: &falseVal, - RequireCodeOwnerReviews: &falseVal, - RequiredApprovingReviewCount: &zeroVal, - }, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &trueVal, - UpToDateBeforeMerge: &trueVal, - Contexts: []string{"foo"}, - }, - EnforceAdmins: &falseVal, - RequireLastPushApproval: &falseVal, - RequireLinearHistory: &falseVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "0", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + }, + result: scut.TestReturn{ + Score: 4, + NumberOfInfo: 5, + NumberOfWarn: 5, }, }, { name: "Required status check enabled without checking for status string", - expected: scut.TestReturn{ - Error: nil, - Score: 4, - NumberOfWarn: 6, - NumberOfInfo: 4, - NumberOfDebug: 0, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &falseVal, - RequireLastPushApproval: &falseVal, - RequireLinearHistory: &falseVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: &falseVal, - RequireCodeOwnerReviews: &falseVal, - RequiredApprovingReviewCount: &zeroVal, - }, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &trueVal, - UpToDateBeforeMerge: &trueVal, - Contexts: nil, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "0", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", }, }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + }, + result: scut.TestReturn{ + Score: 4, + NumberOfInfo: 4, + NumberOfWarn: 6, }, }, { name: "Admin run only preventing force pushes and deletions", - expected: scut.TestReturn{ - Error: nil, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "0", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + }, + result: scut.TestReturn{ Score: 3, NumberOfWarn: 6, NumberOfInfo: 2, NumberOfDebug: 1, }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &falseVal, - RequireLastPushApproval: &falseVal, - RequireLinearHistory: &falseVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &falseVal, - UpToDateBeforeMerge: &falseVal, - Contexts: nil, - }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &falseVal, - }, - }, - }, }, { name: "Admin run with all tier 2 requirements except require PRs and reviewers", - expected: scut.TestReturn{ - Error: nil, - Score: 4, // Should be 4.2 if we allow decimal puctuation + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "0", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + }, + result: scut.TestReturn{ + Score: 4, NumberOfWarn: 2, NumberOfInfo: 6, NumberOfDebug: 1, }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &trueVal, - RequireLastPushApproval: &trueVal, - RequireLinearHistory: &trueVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &falseVal, - UpToDateBeforeMerge: &trueVal, - Contexts: []string{"foo"}, - }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &falseVal, - }, - }, - }, }, { name: "Admin run on project requiring pull requests but without approver -- best a single maintainer can do", - expected: scut.TestReturn{ - Error: nil, - Score: 4, // Should be 4.8 if we allow decimal punctuation - NumberOfWarn: 2, - NumberOfInfo: 9, - NumberOfDebug: 0, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &trueVal, - RequireLastPushApproval: &trueVal, - RequireLinearHistory: &trueVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &trueVal, - UpToDateBeforeMerge: &trueVal, - Contexts: []string{"foo"}, - }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: &trueVal, - RequireCodeOwnerReviews: &trueVal, - RequiredApprovingReviewCount: &zeroVal, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", }, }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "0", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + }, + result: scut.TestReturn{ + Score: 4, + NumberOfWarn: 1, + NumberOfInfo: 9, }, }, { name: "Admin run on project with all tier 2 requirements", - expected: scut.TestReturn{ - Error: nil, - Score: 6, - NumberOfWarn: 4, - NumberOfInfo: 6, - NumberOfDebug: 0, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &trueVal, - RequireLastPushApproval: &trueVal, - RequireLinearHistory: &trueVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &falseVal, - UpToDateBeforeMerge: &trueVal, - Contexts: nil, - }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: &falseVal, - RequireCodeOwnerReviews: &falseVal, - RequiredApprovingReviewCount: &oneVal, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", }, }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "1", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + }, + result: scut.TestReturn{ + Score: 6, + NumberOfWarn: 4, + NumberOfInfo: 6, }, }, { name: "Non-admin run on project that require zero reviewer (or don't require PRs at all, we can't differentiate it)", - expected: scut.TestReturn{ - Error: nil, - Score: 3, - NumberOfWarn: 3, - NumberOfInfo: 2, - NumberOfDebug: 4, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: nil, - RequireLastPushApproval: nil, - RequireLinearHistory: &falseVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: nil, - UpToDateBeforeMerge: nil, - Contexts: nil, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", }, }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "0", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + }, + result: scut.TestReturn{ + Score: 3, + NumberOfWarn: 2, + NumberOfInfo: 2, + NumberOfDebug: 5, }, }, { name: "Non-admin run on project that require 1 reviewer", - expected: scut.TestReturn{ - Error: nil, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "1", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomeNotAvailable, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + }, + result: scut.TestReturn{ Score: 6, NumberOfWarn: 3, NumberOfInfo: 3, NumberOfDebug: 4, }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: nil, - RequireLastPushApproval: nil, - RequireLinearHistory: &falseVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: nil, - UpToDateBeforeMerge: nil, - Contexts: nil, - }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: nil, - RequireCodeOwnerReviews: &falseVal, - RequiredApprovingReviewCount: &oneVal, - }, - }, - }, }, { name: "Required admin enforcement enabled", - expected: scut.TestReturn{ - Error: nil, - Score: 3, - NumberOfWarn: 5, - NumberOfInfo: 5, - NumberOfDebug: 0, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &trueVal, - RequireLastPushApproval: &falseVal, - RequireLinearHistory: &trueVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &falseVal, - UpToDateBeforeMerge: &falseVal, - Contexts: []string{"foo"}, - }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: &falseVal, - RequireCodeOwnerReviews: &falseVal, - RequiredApprovingReviewCount: &zeroVal, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "0", }, }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + }, + result: scut.TestReturn{ + Score: 3, + NumberOfWarn: 5, + NumberOfInfo: 5, }, }, { name: "Required linear history enabled", - expected: scut.TestReturn{ - Error: nil, - Score: 3, - NumberOfWarn: 6, - NumberOfInfo: 4, - NumberOfDebug: 0, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &falseVal, - RequireLastPushApproval: &falseVal, - RequireLinearHistory: &trueVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &falseVal, - UpToDateBeforeMerge: &falseVal, - Contexts: []string{"foo"}, - }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: &falseVal, - RequireCodeOwnerReviews: &falseVal, - RequiredApprovingReviewCount: &zeroVal, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "0", }, }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + }, + result: scut.TestReturn{ + Score: 3, + NumberOfWarn: 6, + NumberOfInfo: 4, }, }, { name: "Allow force push enabled", - expected: scut.TestReturn{ - Error: nil, - Score: 1, - NumberOfWarn: 7, - NumberOfInfo: 3, - NumberOfDebug: 0, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &falseVal, - RequireLastPushApproval: &falseVal, - RequireLinearHistory: &falseVal, - AllowForcePushes: &trueVal, - AllowDeletions: &falseVal, - - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &falseVal, - UpToDateBeforeMerge: &falseVal, - Contexts: []string{"foo"}, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "0", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: &falseVal, - RequireCodeOwnerReviews: &falseVal, - RequiredApprovingReviewCount: &zeroVal, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", }, }, }, + result: scut.TestReturn{ + Score: 1, + NumberOfWarn: 7, + NumberOfInfo: 3, + }, }, { name: "Allow deletions enabled", - expected: scut.TestReturn{ - Error: nil, - Score: 1, - NumberOfWarn: 7, - NumberOfInfo: 3, - NumberOfDebug: 0, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &falseVal, - RequireLastPushApproval: &falseVal, - RequireLinearHistory: &falseVal, - AllowForcePushes: &falseVal, - AllowDeletions: &trueVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &falseVal, - UpToDateBeforeMerge: &falseVal, - Contexts: []string{"foo"}, - }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: &falseVal, - RequireCodeOwnerReviews: &falseVal, - RequiredApprovingReviewCount: &zeroVal, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "0", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomeNegative, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", }, }, }, + result: scut.TestReturn{ + Score: 1, + NumberOfWarn: 7, + NumberOfInfo: 3, + }, }, { name: "Branches are protected", - expected: scut.TestReturn{ - Error: nil, - Score: 8, - NumberOfWarn: 2, - NumberOfInfo: 9, - NumberOfDebug: 0, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &trueVal, - RequireLinearHistory: &trueVal, - RequireLastPushApproval: &trueVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &falseVal, - UpToDateBeforeMerge: &trueVal, - Contexts: []string{"foo"}, - }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: &trueVal, - RequireCodeOwnerReviews: &trueVal, - RequiredApprovingReviewCount: &oneVal, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "1", + }, + }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", }, }, }, + result: scut.TestReturn{ + Score: 8, + NumberOfWarn: 1, + NumberOfInfo: 9, + }, }, { name: "Branches are protected and require codeowner review", - expected: scut.TestReturn{ - Error: nil, - Score: 8, - NumberOfWarn: 1, - NumberOfInfo: 9, - NumberOfDebug: 0, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &trueVal, - RequireLinearHistory: &trueVal, - RequireLastPushApproval: &trueVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &trueVal, - UpToDateBeforeMerge: &trueVal, - Contexts: []string{"foo"}, - }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: &trueVal, - RequireCodeOwnerReviews: &trueVal, - RequiredApprovingReviewCount: &oneVal, + findings: []finding.Finding{ + { + Probe: "blocksDeleteOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", }, }, - }, - codeownersFiles: []string{".github/CODEOWNERS"}, - }, - { - name: "Branches are protected and require codeowner review, but file is not present", - expected: scut.TestReturn{ - Error: nil, - Score: 5, - NumberOfWarn: 3, - NumberOfInfo: 8, - NumberOfDebug: 0, - }, - branch: &clients.BranchRef{ - Name: &branchVal, - Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &trueVal, - RequireLastPushApproval: &falseVal, - RequireLinearHistory: &trueVal, - AllowForcePushes: &falseVal, - AllowDeletions: &falseVal, - CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &falseVal, - UpToDateBeforeMerge: &trueVal, - Contexts: []string{"foo"}, - }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ - Required: &trueVal, - DismissStaleReviews: &trueVal, - RequireCodeOwnerReviews: &trueVal, - RequiredApprovingReviewCount: &oneVal, + { + Probe: "blocksForcePushOnBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchesAreProtected", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "branchProtectionAppliesToAdmins", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "dismissesStaleReviews", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresApproversForPullRequests", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + "numberOfRequiredReviewers": "1", }, }, + { + Probe: "requiresCodeOwnersReview", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresLastPushApproval", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresUpToDateBranches", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "runsStatusChecksBeforeMerging", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + { + Probe: "requiresPRsToChangeCode", + Outcome: finding.OutcomePositive, + Values: map[string]string{ + "branchName": "main", + }, + }, + }, + result: scut.TestReturn{ + Score: 8, + NumberOfWarn: 1, + NumberOfInfo: 9, }, }, } for _, tt := range tests { - tt := tt // Re-initializing variable so it is not changed while executing the closure below + tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() dl := scut.TestDetailLogger{} - score, err := testScore(tt.branch, tt.codeownersFiles, &dl) - actual := &checker.CheckResult{ - Score: score, - Error: err, - } - scut.ValidateTestReturn(t, tt.name, &tt.expected, actual, &dl) + got := BranchProtection(tt.name, tt.findings, &dl) + scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) }) } } diff --git a/checks/fileparser/listing.go b/checks/fileparser/listing.go index 3efb82089910..a51519dd12cc 100644 --- a/checks/fileparser/listing.go +++ b/checks/fileparser/listing.go @@ -17,6 +17,7 @@ package fileparser import ( "bufio" "fmt" + "io" "path" "strings" @@ -63,6 +64,21 @@ type PathMatcher struct { CaseSensitive bool } +// DoWhileTrueOnFileReader takes a filepath, its reader and +// optional variadic args. It returns a boolean indicating whether +// iterating over next files should continue. +type DoWhileTrueOnFileReader func(path string, reader io.Reader, args ...interface{}) (bool, error) + +// OnMatchingFileReaderDo matches all files listed by `repoClient` against `matchPathTo` +// and on every successful match, runs onFileReader fn on the file's reader. +// Continues iterating along the matched files until onFileReader returns +// either a false value or an error. +func OnMatchingFileReaderDo(repoClient clients.RepoClient, matchPathTo PathMatcher, + onFileReader DoWhileTrueOnFileReader, args ...interface{}, +) error { + return onMatchingFileDo(repoClient, matchPathTo, onFileReader, args...) +} + // DoWhileTrueOnFileContent takes a filepath, its content and // optional variadic args. It returns a boolean indicating whether // iterating over next files should continue. @@ -74,6 +90,12 @@ type DoWhileTrueOnFileContent func(path string, content []byte, args ...interfac // either a false value or an error. func OnMatchingFileContentDo(repoClient clients.RepoClient, matchPathTo PathMatcher, onFileContent DoWhileTrueOnFileContent, args ...interface{}, +) error { + return onMatchingFileDo(repoClient, matchPathTo, onFileContent, args...) +} + +func onMatchingFileDo(repoClient clients.RepoClient, matchPathTo PathMatcher, + onFile any, args ...interface{}, ) error { predicate := func(filepath string) (bool, error) { // Filter out test files. @@ -94,12 +116,28 @@ func OnMatchingFileContentDo(repoClient clients.RepoClient, matchPathTo PathMatc } for _, file := range matchedFiles { - content, err := repoClient.GetFileContent(file) + reader, err := repoClient.GetFileReader(file) if err != nil { - return fmt.Errorf("error during GetFileContent: %w", err) + return fmt.Errorf("error during GetFileReader: %w", err) } - continueIter, err := onFileContent(file, content, args...) + var continueIter bool + switch f := onFile.(type) { + case DoWhileTrueOnFileReader: + continueIter, err = f(file, reader, args...) + reader.Close() + case DoWhileTrueOnFileContent: + var content []byte + content, err = io.ReadAll(reader) + reader.Close() + if err != nil { + return fmt.Errorf("reading from file: %w", err) + } + continueIter, err = f(file, content, args...) + default: + msg := fmt.Sprintf("invalid type (%T) passed to onMatchingFileDo", f) + return sce.WithMessage(sce.ErrScorecardInternal, msg) + } if err != nil { return err } diff --git a/checks/fileparser/listing_test.go b/checks/fileparser/listing_test.go index 1b09c921fd74..be9ef0c0294b 100644 --- a/checks/fileparser/listing_test.go +++ b/checks/fileparser/listing_test.go @@ -16,6 +16,8 @@ package fileparser import ( "errors" + "io" + "strings" "testing" "github.com/golang/mock/gomock" @@ -526,7 +528,7 @@ func TestOnMatchingFileContent(t *testing.T) { ctrl := gomock.NewController(t) mockRepo := mockrepo.NewMockRepoClient(ctrl) mockRepo.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil).AnyTimes() - mockRepo.EXPECT().GetFileContent(gomock.Any()).Return(nil, nil).AnyTimes() + mockRepo.EXPECT().GetFileReader(gomock.Any()).Return(io.NopCloser(strings.NewReader("")), nil).AnyTimes() result := OnMatchingFileContentDo(mockRepo, PathMatcher{ Pattern: tt.shellPattern, diff --git a/checks/fuzzing_test.go b/checks/fuzzing_test.go index 460baf20f93a..848a0abd8d4f 100644 --- a/checks/fuzzing_test.go +++ b/checks/fuzzing_test.go @@ -16,6 +16,8 @@ package checks import ( "errors" + "io" + "strings" "testing" "github.com/golang/mock/gomock" @@ -144,11 +146,12 @@ func TestFuzzing(t *testing.T) { }).AnyTimes() mockFuzz.EXPECT().ListProgrammingLanguages().Return(tt.langs, nil).AnyTimes() mockFuzz.EXPECT().ListFiles(gomock.Any()).Return(tt.fileName, nil).AnyTimes() - mockFuzz.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(f string) (string, error) { + mockFuzz.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(f string) (io.ReadCloser, error) { if tt.wantErr { - return "", errors.New("error") + return nil, errors.New("error") } - return tt.fileContent, nil + rc := io.NopCloser(strings.NewReader(tt.fileContent)) + return rc, nil }).AnyTimes() dl := scut.TestDetailLogger{} raw := checker.RawResults{} diff --git a/checks/permissions_test.go b/checks/permissions_test.go index 2d80ef33f1cf..16c7b9bc4abb 100644 --- a/checks/permissions_test.go +++ b/checks/permissions_test.go @@ -15,7 +15,7 @@ package checks import ( - "fmt" + "io" "os" "strings" "testing" @@ -443,12 +443,8 @@ func TestGithubTokenPermissions(t *testing.T) { } return files, nil }).AnyTimes() - mockRepo.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { - content, err := os.ReadFile("./testdata/" + fn) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepo.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { + return os.Open("./testdata/" + fn) }).AnyTimes() dl := scut.TestDetailLogger{} c := checker.CheckRequest{ @@ -499,11 +495,6 @@ func TestGithubTokenPermissionsLineNumber(t *testing.T) { 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() - content, err := os.ReadFile(tt.filename) - if err != nil { - t.Errorf("cannot read file: %v", err) - } - p := strings.Replace(tt.filename, "./testdata/", "", 1) ctrl := gomock.NewController(t) mockRepo := mockrepo.NewMockRepoClient(ctrl) @@ -514,8 +505,8 @@ func TestGithubTokenPermissionsLineNumber(t *testing.T) { mockRepo.EXPECT().ListFiles(gomock.Any()).DoAndReturn(func(predicate func(string) (bool, error)) ([]string, error) { return []string{p}, nil }).AnyTimes() - mockRepo.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { - return content, nil + mockRepo.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { + return os.Open(tt.filename) }).AnyTimes() dl := scut.TestDetailLogger{} c := checker.CheckRequest{ diff --git a/checks/pinned_dependencies_test.go b/checks/pinned_dependencies_test.go index b22fa7fb8493..ea22bdd73027 100644 --- a/checks/pinned_dependencies_test.go +++ b/checks/pinned_dependencies_test.go @@ -15,7 +15,7 @@ package checks import ( - "fmt" + "io" "os" "testing" @@ -58,15 +58,11 @@ func TestPinningDependencies(t *testing.T) { mockRepo.EXPECT().URI().Return("github.com/ossf/scorecard").AnyTimes() mockRepo.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil).AnyTimes() - mockRepo.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { + mockRepo.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { if tt.path == "" { return nil, nil } - content, err := os.ReadFile(tt.path) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + return os.Open(tt.path) }).AnyTimes() dl := scut.TestDetailLogger{} diff --git a/checks/raw/binary_artifact.go b/checks/raw/binary_artifact.go index 1c7a82f246d7..ad963b092bc2 100644 --- a/checks/raw/binary_artifact.go +++ b/checks/raw/binary_artifact.go @@ -17,6 +17,7 @@ package raw import ( "errors" "fmt" + "io" "path/filepath" "regexp" "strings" @@ -39,6 +40,9 @@ var ( gradleWrapperValidationActionVersionConstraint = mustParseConstraint(`>= 1.0.0`) ) +// how many bytes are considered when determining if a file is text or binary. +const binaryTestLen = 1024 + // mustParseConstraint attempts parse of semver constraint, panics if fail. func mustParseConstraint(c string) *semver.Constraints { if c, err := semver.NewConstraint(c); err != nil { @@ -52,10 +56,10 @@ func mustParseConstraint(c string) *semver.Constraints { func BinaryArtifacts(req *checker.CheckRequest) (checker.BinaryArtifactData, error) { c := req.RepoClient files := []checker.File{} - err := fileparser.OnMatchingFileContentDo(c, fileparser.PathMatcher{ + err := fileparser.OnMatchingFileReaderDo(c, fileparser.PathMatcher{ Pattern: "*", CaseSensitive: false, - }, checkBinaryFileContent, &files) + }, checkBinaryFileReader, &files) if err != nil { return checker.BinaryArtifactData{}, fmt.Errorf("%w", err) } @@ -96,17 +100,17 @@ func excludeValidatedGradleWrappers(c clients.RepoClient, files []checker.File) return files, nil } -var checkBinaryFileContent fileparser.DoWhileTrueOnFileContent = func(path string, content []byte, +var checkBinaryFileReader fileparser.DoWhileTrueOnFileReader = func(path string, reader io.Reader, args ...interface{}, ) (bool, error) { if len(args) != 1 { return false, fmt.Errorf( - "checkBinaryFileContent requires exactly one argument: %w", errInvalidArgLength) + "checkBinaryFileReader requires exactly one argument: %w", errInvalidArgLength) } pfiles, ok := args[0].(*[]checker.File) if !ok { return false, fmt.Errorf( - "checkBinaryFileContent requires argument of type *[]checker.File: %w", errInvalidArgType) + "checkBinaryFileReader requires argument of type *[]checker.File: %w", errInvalidArgType) } binaryFileTypes := map[string]bool{ @@ -138,8 +142,13 @@ var checkBinaryFileContent fileparser.DoWhileTrueOnFileContent = func(path strin "wasm": true, "whl": true, } + + content, err := io.ReadAll(io.LimitReader(reader, binaryTestLen)) + if err != nil { + return false, fmt.Errorf("reading file: %w", err) + } + var t types.Type - var err error if len(content) == 0 { return true, nil } @@ -169,12 +178,12 @@ var checkBinaryFileContent fileparser.DoWhileTrueOnFileContent = func(path strin return true, nil } -// determines if the first 1024 bytes are text +// determines if the first binaryTestLen bytes are text // // A version of golang.org/x/tools/godoc/util modified to allow carriage returns // and utf8.RuneError (0xFFFD), as the file may not be utf8 encoded. func isText(s []byte) bool { - const max = 1024 // at least utf8.UTFMax + const max = binaryTestLen // at least utf8.UTFMax (4) if len(s) > max { s = s[0:max] } diff --git a/checks/raw/binary_artifact_test.go b/checks/raw/binary_artifact_test.go index 590e86a78331..8b956ab3d415 100644 --- a/checks/raw/binary_artifact_test.go +++ b/checks/raw/binary_artifact_test.go @@ -15,7 +15,7 @@ package raw import ( - "fmt" + "io" "os" "testing" @@ -227,13 +227,8 @@ func TestBinaryArtifacts(t *testing.T) { mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return(files, nil) } for i := 0; i < tt.getFileContentCount; i++ { - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { - // This will read the file and return the content - content, err := os.ReadFile(file) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open(file) }) } if tt.successfulWorkflowRuns != nil { @@ -276,19 +271,11 @@ func TestBinaryArtifacts_workflow_runs_unsupported(t *testing.T) { const verifyWorkflow = ".github/workflows/verify.yaml" files := []string{jarFile, verifyWorkflow} mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return(files, nil).AnyTimes() - mockRepoClient.EXPECT().GetFileContent(jarFile).DoAndReturn(func(file string) ([]byte, error) { - content, err := os.ReadFile("../testdata/binaryartifacts/jars/gradle-wrapper.jar") - if err != nil { - return nil, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(jarFile).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open("../testdata/binaryartifacts/jars/gradle-wrapper.jar") }).AnyTimes() - mockRepoClient.EXPECT().GetFileContent(verifyWorkflow).DoAndReturn(func(file string) ([]byte, error) { - content, err := os.ReadFile("../testdata/binaryartifacts/workflows/verify.yaml") - if err != nil { - return nil, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(verifyWorkflow).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open("../testdata/binaryartifacts/workflows/verify.yaml") }).AnyTimes() mockRepoClient.EXPECT().ListSuccessfulWorkflowRuns(gomock.Any()).Return(nil, clients.ErrUnsupportedFeature).AnyTimes() diff --git a/checks/raw/branch_protection.go b/checks/raw/branch_protection.go index 6e58bd2f2f8a..505fb608da04 100644 --- a/checks/raw/branch_protection.go +++ b/checks/raw/branch_protection.go @@ -51,7 +51,8 @@ func (set branchSet) contains(branch string) bool { } // BranchProtection retrieves the raw data for the Branch-Protection check. -func BranchProtection(c clients.RepoClient) (checker.BranchProtectionsData, error) { +func BranchProtection(cr *checker.CheckRequest) (checker.BranchProtectionsData, error) { + c := cr.RepoClient branches := branchSet{ exists: make(map[string]bool), } diff --git a/checks/raw/branch_protection_test.go b/checks/raw/branch_protection_test.go index 4db6f385f658..7a5ce78031b6 100644 --- a/checks/raw/branch_protection_test.go +++ b/checks/raw/branch_protection_test.go @@ -273,7 +273,11 @@ func TestBranchProtection(t *testing.T) { return tt.releases, tt.releasesErr }) mockRepoClient.EXPECT().ListFiles(gomock.Any()).AnyTimes().Return(tt.repoFiles, nil) - rawData, err := BranchProtection(mockRepoClient) + + c := &checker.CheckRequest{ + RepoClient: mockRepoClient, + } + rawData, err := BranchProtection(c) if !errors.Is(err, tt.wantErr) { t.Errorf("failed. expected: %v, got: %v", tt.wantErr, err) t.Fail() diff --git a/checks/raw/dangerous_workflow_test.go b/checks/raw/dangerous_workflow_test.go index 787f37f33113..753dbab2b922 100644 --- a/checks/raw/dangerous_workflow_test.go +++ b/checks/raw/dangerous_workflow_test.go @@ -17,7 +17,7 @@ package raw import ( "context" "errors" - "fmt" + "io" "os" "testing" @@ -159,13 +159,8 @@ func TestGithubDangerousWorkflow(t *testing.T) { ctrl := gomock.NewController(t) mockRepoClient := mockrepo.NewMockRepoClient(ctrl) mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return([]string{tt.filename}, nil) - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { - // This will read the file and return the content - content, err := os.ReadFile("../testdata/" + file) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open("../testdata/" + file) }) req := &checker.CheckRequest{ diff --git a/checks/raw/fuzzing_test.go b/checks/raw/fuzzing_test.go index e409b4014476..18d669740d53 100644 --- a/checks/raw/fuzzing_test.go +++ b/checks/raw/fuzzing_test.go @@ -16,8 +16,10 @@ package raw import ( "errors" + "io" "path" "regexp" + "strings" "testing" "github.com/golang/mock/gomock" @@ -135,11 +137,11 @@ func Test_checkCFLite(t *testing.T) { defer ctrl.Finish() mockFuzz := mockrepo.NewMockRepoClient(ctrl) mockFuzz.EXPECT().ListFiles(gomock.Any()).Return(tt.fileName, nil).AnyTimes() - mockFuzz.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(f string) (string, error) { + mockFuzz.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(f string) (io.ReadCloser, error) { if tt.wantErr { - return "", errors.New("error") + return nil, errors.New("error") } - return tt.fileContent, nil + return io.NopCloser(strings.NewReader(tt.fileContent)), nil }).AnyTimes() req := checker.CheckRequest{ RepoClient: mockFuzz, @@ -486,11 +488,11 @@ func Test_checkFuzzFunc(t *testing.T) { defer ctrl.Finish() mockClient := mockrepo.NewMockRepoClient(ctrl) mockClient.EXPECT().ListFiles(gomock.Any()).Return(tt.fileName, nil).AnyTimes() - mockClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(f string) ([]byte, error) { + mockClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(f string) (io.ReadCloser, error) { if tt.wantErr { return nil, errors.New("error") } - return []byte(tt.fileContent), nil + return io.NopCloser(strings.NewReader(tt.fileContent)), nil }).AnyTimes() req := checker.CheckRequest{ RepoClient: mockClient, diff --git a/checks/raw/github/packaging.go b/checks/raw/github/packaging.go index 1769dcec7ecd..291fdf36172f 100644 --- a/checks/raw/github/packaging.go +++ b/checks/raw/github/packaging.go @@ -16,6 +16,7 @@ package github import ( "fmt" + "io" "path/filepath" "github.com/rhysd/actionlint" @@ -37,9 +38,14 @@ func Packaging(c *checker.CheckRequest) (checker.PackagingData, error) { } for _, fp := range matchedFiles { - fc, err := c.RepoClient.GetFileContent(fp) + fr, err := c.RepoClient.GetFileReader(fp) if err != nil { - return data, fmt.Errorf("RepoClient.GetFileContent: %w", err) + return data, fmt.Errorf("RepoClient.GetFileReader: %w", err) + } + fc, err := io.ReadAll(fr) + fr.Close() + if err != nil { + return data, fmt.Errorf("reading file: %w", err) } workflow, errs := actionlint.Parse(fc) diff --git a/checks/raw/gitlab/packaging.go b/checks/raw/gitlab/packaging.go index d6d9277fd2c4..72de346f73da 100644 --- a/checks/raw/gitlab/packaging.go +++ b/checks/raw/gitlab/packaging.go @@ -16,6 +16,7 @@ package gitlab import ( "fmt" + "io" "strings" "github.com/ossf/scorecard/v4/checker" @@ -32,9 +33,14 @@ func Packaging(c *checker.CheckRequest) (checker.PackagingData, error) { } for _, fp := range matchedFiles { - fc, err := c.RepoClient.GetFileContent(fp) + fr, err := c.RepoClient.GetFileReader(fp) if err != nil { - return data, fmt.Errorf("RepoClient.GetFileContent: %w", err) + return data, fmt.Errorf("RepoClient.GetFileReader: %w", err) + } + fc, err := io.ReadAll(fr) + fr.Close() + if err != nil { + return data, fmt.Errorf("reading from file: %w", err) } file, found := isGitlabPackagingWorkflow(fc, fp) diff --git a/checks/raw/gitlab/packaging_test.go b/checks/raw/gitlab/packaging_test.go index 1e66f0452007..ffd3eea47c5c 100644 --- a/checks/raw/gitlab/packaging_test.go +++ b/checks/raw/gitlab/packaging_test.go @@ -15,6 +15,7 @@ package gitlab import ( + "io" "os" "testing" @@ -134,10 +135,9 @@ func TestGitlabPackagingPackager(t *testing.T) { moqRepoClient.EXPECT().ListFiles(gomock.Any()). Return([]string{tt.filename}, nil).AnyTimes() - moqRepoClient.EXPECT().GetFileContent(tt.filename). - DoAndReturn(func(b string) ([]byte, error) { - content, err := os.ReadFile(b) - return content, err + moqRepoClient.EXPECT().GetFileReader(tt.filename). + DoAndReturn(func(b string) (io.ReadCloser, error) { + return os.Open(b) }).AnyTimes() if tt.exists { diff --git a/checks/raw/pinned_dependencies_test.go b/checks/raw/pinned_dependencies_test.go index 82b0588511c4..305c075a6002 100644 --- a/checks/raw/pinned_dependencies_test.go +++ b/checks/raw/pinned_dependencies_test.go @@ -15,7 +15,7 @@ package raw import ( - "fmt" + "io" "os" "path/filepath" "strings" @@ -1895,13 +1895,8 @@ func TestCollectDockerfilePinning(t *testing.T) { mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return([]string{tt.filename}, nil).AnyTimes() mockRepoClient.EXPECT().GetDefaultBranchName().Return("main", nil).AnyTimes() mockRepoClient.EXPECT().URI().Return("github.com/ossf/scorecard").AnyTimes() - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { - // This will read the file and return the content - content, err := os.ReadFile(file) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open(file) }) req := checker.CheckRequest{ @@ -1994,13 +1989,8 @@ func TestCollectGitHubActionsWorkflowPinning(t *testing.T) { mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return([]string{tt.filename}, nil).AnyTimes() mockRepoClient.EXPECT().GetDefaultBranchName().Return("main", nil).AnyTimes() mockRepoClient.EXPECT().URI().Return("github.com/ossf/scorecard").AnyTimes() - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { - // This will read the file and return the content - content, err := os.ReadFile(filepath.Join("testdata", file)) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open(filepath.Join("testdata", file)) }) req := checker.CheckRequest{ diff --git a/checks/raw/sast.go b/checks/raw/sast.go index 0dc9ef52ff76..54ce809906cb 100644 --- a/checks/raw/sast.go +++ b/checks/raw/sast.go @@ -19,6 +19,7 @@ import ( "bytes" "errors" "fmt" + "io" "path" "regexp" "strings" @@ -230,7 +231,8 @@ type sonarConfig struct { func getSonarWorkflows(c *checker.CheckRequest) ([]checker.SASTWorkflow, error) { var config []sonarConfig var sastWorkflows []checker.SASTWorkflow - err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ + // in the future, we may want to use ListFiles instead, so we don't open every file + err := fileparser.OnMatchingFileReaderDo(c.RepoClient, fileparser.PathMatcher{ Pattern: "*", CaseSensitive: false, }, validateSonarConfig, &config) @@ -255,8 +257,8 @@ func getSonarWorkflows(c *checker.CheckRequest) ([]checker.SASTWorkflow, error) } // Check file content. -var validateSonarConfig fileparser.DoWhileTrueOnFileContent = func(pathfn string, - content []byte, +var validateSonarConfig fileparser.DoWhileTrueOnFileReader = func(pathfn string, + reader io.Reader, args ...interface{}, ) (bool, error) { if !strings.EqualFold(path.Base(pathfn), "pom.xml") { @@ -275,6 +277,10 @@ var validateSonarConfig fileparser.DoWhileTrueOnFileContent = func(pathfn string "validateSonarConfig expects arg[0] of type *[]sonarConfig]: %w", errInvalid) } + content, err := io.ReadAll(reader) + if err != nil { + return false, fmt.Errorf("read file: %w", err) + } regex := regexp.MustCompile(`\s*(\S+)\s*<\/sonar\.host\.url>`) match := regex.FindSubmatch(content) @@ -308,12 +314,11 @@ func findLine(content, data []byte) (uint, error) { r := bytes.NewReader(content) scanner := bufio.NewScanner(r) - line := 0 - // https://golang.org/pkg/bufio/#Scanner.Scan + var line uint for scanner.Scan() { line++ - if strings.Contains(scanner.Text(), string(data)) { - return uint(line), nil + if bytes.Contains(scanner.Bytes(), data) { + return line, nil } } diff --git a/checks/raw/sast_test.go b/checks/raw/sast_test.go index 8bdf4020463a..d724ec4ddfc8 100644 --- a/checks/raw/sast_test.go +++ b/checks/raw/sast_test.go @@ -15,7 +15,7 @@ package raw import ( - "fmt" + "io" "os" "testing" @@ -207,13 +207,8 @@ func TestSAST(t *testing.T) { mockRepoClient.EXPECT().ListCommits().DoAndReturn(func() ([]clients.Commit, error) { return tt.commits, nil }) - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { - // This will read the file and return the content - content, err := os.ReadFile("./testdata/" + file) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open("./testdata/" + file) }).AnyTimes() req := checker.CheckRequest{ RepoClient: mockRepoClient, diff --git a/checks/raw/security_policy_test.go b/checks/raw/security_policy_test.go index d311d85e283b..579b9576d73d 100644 --- a/checks/raw/security_policy_test.go +++ b/checks/raw/security_policy_test.go @@ -15,8 +15,9 @@ package raw import ( - "fmt" + "io" "os" + "strings" "testing" "github.com/golang/mock/gomock" @@ -142,18 +143,14 @@ func TestSecurityPolicy(t *testing.T) { // file contents once found. This test will return that // mock file, but this specific unit test is not testing // for content. As such, this test will crash without - // a mock GetFileContent, so this will return no content + // a mock GetFileReader, so this will return no content // for the existing file. content test are in overall check // - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { if tt.path == "" { - return nil, nil + return io.NopCloser(strings.NewReader("")), nil } - content, err := os.ReadFile(tt.path) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + return os.Open(tt.path) }).AnyTimes() dl := scut.TestDetailLogger{} diff --git a/checks/sast_test.go b/checks/sast_test.go index 01da79ae43bc..16b879b24bf3 100644 --- a/checks/sast_test.go +++ b/checks/sast_test.go @@ -17,7 +17,7 @@ package checks import ( "context" "errors" - "fmt" + "io" "os" "strings" "testing" @@ -320,15 +320,11 @@ func Test_SAST(t *testing.T) { } return []string{tt.path}, nil }).AnyTimes() - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { if tt.path == "" { - return nil, nil + return io.NopCloser(strings.NewReader("")), nil } - content, err := os.ReadFile("./testdata/" + tt.path) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + return os.Open("./testdata/" + tt.path) }).AnyTimes() dl := scut.TestDetailLogger{} diff --git a/checks/security_policy_test.go b/checks/security_policy_test.go index 1517c7805894..4d74406927bd 100644 --- a/checks/security_policy_test.go +++ b/checks/security_policy_test.go @@ -15,7 +15,7 @@ package checks import ( - "fmt" + "io" "os" "testing" @@ -178,15 +178,11 @@ func TestSecurityPolicy(t *testing.T) { mockRepo.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil).AnyTimes() - mockRepo.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { + mockRepo.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { if tt.path == "" { return nil, nil } - content, err := os.ReadFile(tt.path) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + return os.Open(tt.path) }).AnyTimes() dl := scut.TestDetailLogger{} diff --git a/clients/git/client.go b/clients/git/client.go index fa6d2e3fea0f..f530cc7c82a3 100644 --- a/clients/git/client.go +++ b/clients/git/client.go @@ -42,6 +42,9 @@ var ( errNilCommitFound = errors.New("nil commit found") errEmptyQuery = errors.New("query is empty") errDefaultBranch = errors.New("default branch name could not be determined") + + // ensure Client implements clients.RepoClient. + _ clients.RepoClient = (*Client)(nil) ) type Client struct { @@ -235,17 +238,17 @@ func (c *Client) ListFiles(predicate func(string) (bool, error)) ([]string, erro return files, nil } -func (c *Client) GetFileContent(filename string) ([]byte, error) { +func (c *Client) GetFileReader(filename string) (io.ReadCloser, error) { // Create the full path of the file fullPath := filepath.Join(c.tempDir, filename) // Read the file - content, err := os.ReadFile(fullPath) + f, err := os.Open(fullPath) if err != nil { - return nil, fmt.Errorf("os.ReadFile: %w", err) + return nil, fmt.Errorf("os.Open: %w", err) } - return content, nil + return f, nil } func (c *Client) IsArchived() (bool, error) { diff --git a/clients/githubrepo/client.go b/clients/githubrepo/client.go index dbef2cf91345..dc5dba42af7f 100644 --- a/clients/githubrepo/client.go +++ b/clients/githubrepo/client.go @@ -19,6 +19,7 @@ import ( "context" "errors" "fmt" + "io" "net/http" "os" "strings" @@ -147,9 +148,9 @@ func (client *Client) ListFiles(predicate func(string) (bool, error)) ([]string, return client.tarball.listFiles(predicate) } -// GetFileContent implements RepoClient.GetFileContent. -func (client *Client) GetFileContent(filename string) ([]byte, error) { - return client.tarball.getFileContent(filename) +// GetFileReader implements RepoClient.GetFileReader. +func (client *Client) GetFileReader(filename string) (io.ReadCloser, error) { + return client.tarball.getFile(filename) } // ListCommits implements RepoClient.ListCommits. diff --git a/clients/githubrepo/tarball.go b/clients/githubrepo/tarball.go index 8888e2f26b61..ae5455ce6e7c 100644 --- a/clients/githubrepo/tarball.go +++ b/clients/githubrepo/tarball.go @@ -260,15 +260,15 @@ func (handler *tarballHandler) getLocalPath() (string, error) { return absTempDir, nil } -func (handler *tarballHandler) getFileContent(filename string) ([]byte, error) { +func (handler *tarballHandler) getFile(filename string) (*os.File, error) { if err := handler.setup(); err != nil { return nil, fmt.Errorf("error during tarballHandler.setup: %w", err) } - content, err := os.ReadFile(filepath.Join(handler.tempDir, filename)) + f, err := os.Open(filepath.Join(handler.tempDir, filename)) if err != nil { - return content, fmt.Errorf("os.ReadFile: %w", err) + return nil, fmt.Errorf("open file: %w", err) } - return content, nil + return f, nil } func (handler *tarballHandler) cleanup() error { diff --git a/clients/githubrepo/tarball_test.go b/clients/githubrepo/tarball_test.go index 106877dbdf7a..89f570484bc7 100644 --- a/clients/githubrepo/tarball_test.go +++ b/clients/githubrepo/tarball_test.go @@ -161,12 +161,18 @@ func TestExtractTarball(t *testing.T) { // Test GetFileContent API. for _, getcontenttest := range testcase.getcontentTests { - content, err := handler.getFileContent(getcontenttest.filename) + f, err := handler.getFile(getcontenttest.filename) if getcontenttest.err != nil && !errors.Is(err, getcontenttest.err) { t.Errorf("test failed: expected - %v, got - %v", getcontenttest.err, err) } - if getcontenttest.err == nil && !cmp.Equal(getcontenttest.output, content) { - t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + if getcontenttest.err == nil { + content, err := io.ReadAll(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !cmp.Equal(getcontenttest.output, content) { + t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + } } } diff --git a/clients/gitlabrepo/client.go b/clients/gitlabrepo/client.go index 68644e241e8a..a53588cf3ace 100644 --- a/clients/gitlabrepo/client.go +++ b/clients/gitlabrepo/client.go @@ -19,6 +19,7 @@ import ( "context" "errors" "fmt" + "io" "log" "os" "time" @@ -173,8 +174,8 @@ func (client *Client) ListFiles(predicate func(string) (bool, error)) ([]string, return client.tarball.listFiles(predicate) } -func (client *Client) GetFileContent(filename string) ([]byte, error) { - return client.tarball.getFileContent(filename) +func (client *Client) GetFileReader(filename string) (io.ReadCloser, error) { + return client.tarball.getFile(filename) } func (client *Client) ListCommits() ([]clients.Commit, error) { diff --git a/clients/gitlabrepo/tarball.go b/clients/gitlabrepo/tarball.go index be0b42ffcaf5..75ef533f2315 100644 --- a/clients/gitlabrepo/tarball.go +++ b/clients/gitlabrepo/tarball.go @@ -292,15 +292,15 @@ func (handler *tarballHandler) listFiles(predicate func(string) (bool, error)) ( return ret, nil } -func (handler *tarballHandler) getFileContent(filename string) ([]byte, error) { +func (handler *tarballHandler) getFile(filename string) (*os.File, error) { if err := handler.setup(); err != nil { return nil, fmt.Errorf("error during tarballHandler.setup: %w", err) } - content, err := os.ReadFile(filepath.Join(handler.tempDir, filename)) + f, err := os.Open(filepath.Join(handler.tempDir, filename)) if err != nil { - return content, fmt.Errorf("os.ReadFile: %w", err) + return nil, fmt.Errorf("open file: %w", err) } - return content, nil + return f, nil } func (handler *tarballHandler) cleanup() error { diff --git a/clients/gitlabrepo/tarball_e2e_test.go b/clients/gitlabrepo/tarball_e2e_test.go index 38f938506d67..fa3d7b5a7cc1 100644 --- a/clients/gitlabrepo/tarball_e2e_test.go +++ b/clients/gitlabrepo/tarball_e2e_test.go @@ -16,6 +16,7 @@ package gitlabrepo import ( "context" + "io" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -39,7 +40,10 @@ var _ = Describe("E2E TEST: gitlabrepo.ListFiles", func() { Expect(err).Should(BeNil()) Expect(len(files)).ShouldNot(BeZero()) - data, err := client.GetFileContent("README.md") + r, err := client.GetFileReader("README.md") + Expect(err).Should(BeNil()) + defer r.Close() + data, err := io.ReadAll(r) Expect(err).Should(BeNil()) Expect(len(data)).ShouldNot(BeZero()) }) diff --git a/clients/gitlabrepo/tarball_test.go b/clients/gitlabrepo/tarball_test.go index 75ad34e87e61..3bbbc18928b1 100644 --- a/clients/gitlabrepo/tarball_test.go +++ b/clients/gitlabrepo/tarball_test.go @@ -153,12 +153,18 @@ func TestExtractTarball(t *testing.T) { // Test GetFileContent API. for _, getcontenttest := range testcase.getcontentTests { - content, err := handler.getFileContent(getcontenttest.filename) + f, err := handler.getFile(getcontenttest.filename) if getcontenttest.err != nil && !errors.Is(err, getcontenttest.err) { t.Errorf("test failed: expected - %v, got - %v", getcontenttest.err, err) } - if getcontenttest.err == nil && !cmp.Equal(getcontenttest.output, content) { - t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + if getcontenttest.err == nil { + content, err := io.ReadAll(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !cmp.Equal(getcontenttest.output, content) { + t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + } } } diff --git a/clients/localdir/client.go b/clients/localdir/client.go index 45cf09924ae5..2038108d1d2c 100644 --- a/clients/localdir/client.go +++ b/clients/localdir/client.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "io" "io/fs" "os" "path" @@ -159,19 +160,19 @@ func (client *localDirClient) ListFiles(predicate func(string) (bool, error)) ([ return applyPredicate(client.files, client.errFiles, predicate) } -func getFileContent(clientpath, filename string) ([]byte, error) { +func getFile(clientpath, filename string) (*os.File, error) { // Note: the filenames do not contain the original path - see ListFiles(). fn := path.Join(clientpath, filename) - content, err := os.ReadFile(fn) + f, err := os.Open(fn) if err != nil { - return content, fmt.Errorf("%w", err) + return nil, fmt.Errorf("open file: %w", err) } - return content, nil + return f, nil } -// GetFileContent implements RepoClient.GetFileContent. -func (client *localDirClient) GetFileContent(filename string) ([]byte, error) { - return getFileContent(client.path, filename) +// GetFileReader implements RepoClient.GetFileReader. +func (client *localDirClient) GetFileReader(filename string) (io.ReadCloser, error) { + return getFile(client.path, filename) } // GetBranch implements RepoClient.GetBranch. diff --git a/clients/localdir/client_test.go b/clients/localdir/client_test.go index c11db84c2962..6d8964eca215 100644 --- a/clients/localdir/client_test.go +++ b/clients/localdir/client_test.go @@ -17,6 +17,7 @@ package localdir import ( "context" "errors" + "io" "os" "strings" "testing" @@ -119,6 +120,7 @@ func isSortedString(x, y string) bool { return x < y } +//nolint:gocognit func TestClient_GetFileListAndContent(t *testing.T) { t.Parallel() testcases := []struct { @@ -189,12 +191,18 @@ func TestClient_GetFileListAndContent(t *testing.T) { // Test GetFileContent API. for _, getcontenttest := range testcase.getcontentTests { - content, err := getFileContent(testcase.inputFolder, getcontenttest.filename) + f, err := getFile(testcase.inputFolder, getcontenttest.filename) if getcontenttest.err != nil && !errors.Is(err, getcontenttest.err) { t.Errorf("test failed: expected - %v, got - %v", getcontenttest.err, err) } - if getcontenttest.err == nil && !cmp.Equal(getcontenttest.output, content) { - t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + if err == nil { + content, err := io.ReadAll(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !cmp.Equal(getcontenttest.output, content) { + t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + } } } }) diff --git a/clients/mockclients/repo_client.go b/clients/mockclients/repo_client.go index deb2af7ff292..6ad96ba1095b 100644 --- a/clients/mockclients/repo_client.go +++ b/clients/mockclients/repo_client.go @@ -21,6 +21,7 @@ package mockrepo import ( context "context" + io "io" reflect "reflect" time "time" @@ -125,19 +126,19 @@ func (mr *MockRepoClientMockRecorder) GetDefaultBranchName() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDefaultBranchName", reflect.TypeOf((*MockRepoClient)(nil).GetDefaultBranchName)) } -// GetFileContent mocks base method. -func (m *MockRepoClient) GetFileContent(filename string) ([]byte, error) { +// GetFileReader mocks base method. +func (m *MockRepoClient) GetFileReader(filename string) (io.ReadCloser, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetFileContent", filename) - ret0, _ := ret[0].([]byte) + ret := m.ctrl.Call(m, "GetFileReader", filename) + ret0, _ := ret[0].(io.ReadCloser) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetFileContent indicates an expected call of GetFileContent. -func (mr *MockRepoClientMockRecorder) GetFileContent(filename interface{}) *gomock.Call { +// GetFileReader indicates an expected call of GetFileReader. +func (mr *MockRepoClientMockRecorder) GetFileReader(filename interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFileContent", reflect.TypeOf((*MockRepoClient)(nil).GetFileContent), filename) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFileReader", reflect.TypeOf((*MockRepoClient)(nil).GetFileReader), filename) } // GetOrgRepoClient mocks base method. diff --git a/clients/ossfuzz/client.go b/clients/ossfuzz/client.go index 8c19d8038e64..2439244737fb 100644 --- a/clients/ossfuzz/client.go +++ b/clients/ossfuzz/client.go @@ -182,9 +182,9 @@ func (c *client) ListFiles(predicate func(string) (bool, error)) ([]string, erro return nil, fmt.Errorf("ListFiles: %w", clients.ErrUnsupportedFeature) } -// GetFileContent implements RepoClient.GetFileContent. -func (c *client) GetFileContent(filename string) ([]byte, error) { - return nil, fmt.Errorf("GetFileContent: %w", clients.ErrUnsupportedFeature) +// GetFileReader implements RepoClient.GetFileReader. +func (c *client) GetFileReader(filename string) (io.ReadCloser, error) { + return nil, fmt.Errorf("GetFileReader: %w", clients.ErrUnsupportedFeature) } // GetBranch implements RepoClient.GetBranch. diff --git a/clients/ossfuzz/client_test.go b/clients/ossfuzz/client_test.go index fd0ad92a6128..3900526170ef 100644 --- a/clients/ossfuzz/client_test.go +++ b/clients/ossfuzz/client_test.go @@ -235,9 +235,9 @@ func TestAllClientMethods(t *testing.T) { // Test GetFileContent { - _, err := c.GetFileContent("") + _, err := c.GetFileReader("") if !errors.Is(err, clients.ErrUnsupportedFeature) { - t.Errorf("GetFileContent: Expected %v, but got %v", clients.ErrUnsupportedFeature, err) + t.Errorf("GetFileReader: Expected %v, but got %v", clients.ErrUnsupportedFeature, err) } } diff --git a/clients/osv.go b/clients/osv.go index e265aa2bbea0..6015f2fd7ae4 100644 --- a/clients/osv.go +++ b/clients/osv.go @@ -66,6 +66,11 @@ func (v osvClient) ListUnfixedVulnerabilities( if errors.Is(err, osvscanner.VulnerabilitiesFoundErr) { vulns := res.Flatten() for i := range vulns { + // ignore Go stdlib vulns. The go directive from the go.mod isn't a perfect metric + // of which version of Go will be used to build a project. + if vulns[i].Package.Ecosystem == "Go" && vulns[i].Package.Name == "stdlib" { + continue + } response.Vulnerabilities = append(response.Vulnerabilities, Vulnerability{ ID: vulns[i].Vulnerability.ID, Aliases: vulns[i].Vulnerability.Aliases, diff --git a/clients/repo_client.go b/clients/repo_client.go index a5804b3f7d7e..d51661e99762 100644 --- a/clients/repo_client.go +++ b/clients/repo_client.go @@ -18,6 +18,7 @@ package clients import ( "context" "errors" + "io" "time" ) @@ -36,7 +37,9 @@ type RepoClient interface { // Returns an absolute path to the local repository // in the format that matches the local OS LocalPath() (string, error) - GetFileContent(filename string) ([]byte, error) + // GetFileReader returns an io.ReadCloser corresponding to the desired file. + // Callers should ensure to Close the Reader when finished. + GetFileReader(filename string) (io.ReadCloser, error) GetBranch(branch string) (*BranchRef, error) GetCreatedAt() (time.Time, error) GetDefaultBranchName() (string, error) diff --git a/docs/checks.md b/docs/checks.md index 4bc62dba4c42..a0e069e2324d 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -594,7 +594,7 @@ Signed releases attest to the provenance of the artifact. This check looks for the following filenames in the project's last five [release assets](https://docs.github.com/en/repositories/releasing-projects-on-github/about-releases): [*.minisig](https://github.com/jedisct1/minisign), *.asc (pgp), -*.sig, *.sign, [*.intoto.jsonl](https://slsa.dev). +*.sig, *.sign, *.sigstore, [*.intoto.jsonl](https://slsa.dev). If a signature is found in the assets for each release, a score of 8 is given. If a [SLSA provenance file](https://slsa.dev/spec/v0.1/index) is found in the assets for each release (*.intoto.jsonl), the maximum score of 10 is given. diff --git a/docs/checks/internal/checks.yaml b/docs/checks/internal/checks.yaml index 4c25be6b7146..9abc6802624e 100644 --- a/docs/checks/internal/checks.yaml +++ b/docs/checks/internal/checks.yaml @@ -626,7 +626,7 @@ checks: This check looks for the following filenames in the project's last five [release assets](https://docs.github.com/en/repositories/releasing-projects-on-github/about-releases): [*.minisig](https://github.com/jedisct1/minisign), *.asc (pgp), - *.sig, *.sign, [*.intoto.jsonl](https://slsa.dev). + *.sig, *.sign, *.sigstore, [*.intoto.jsonl](https://slsa.dev). If a signature is found in the assets for each release, a score of 8 is given. If a [SLSA provenance file](https://slsa.dev/spec/v0.1/index) is found in the assets for each release (*.intoto.jsonl), the maximum score of 10 is given. diff --git a/e2e/branch_protection_test.go b/e2e/branch_protection_test.go index 0696903de5c7..38804dc27d2e 100644 --- a/e2e/branch_protection_test.go +++ b/e2e/branch_protection_test.go @@ -48,7 +48,7 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() { Error: nil, Score: 6, NumberOfWarn: 2, - NumberOfInfo: 5, + NumberOfInfo: 4, NumberOfDebug: 4, } result := checks.BranchProtection(&req) diff --git a/go.mod b/go.mod index ede96cb6c93e..f65998a1f99b 100644 --- a/go.mod +++ b/go.mod @@ -4,9 +4,9 @@ go 1.21.5 require ( cloud.google.com/go/bigquery v1.59.1 - cloud.google.com/go/monitoring v1.17.0 // indirect - cloud.google.com/go/pubsub v1.36.1 - cloud.google.com/go/trace v1.10.4 // indirect + cloud.google.com/go/monitoring v1.18.0 // indirect + cloud.google.com/go/pubsub v1.36.2 + cloud.google.com/go/trace v1.10.5 // indirect contrib.go.opencensus.io/exporter/stackdriver v0.13.14 github.com/bombsimon/logrusr/v2 v2.0.1 github.com/bradleyfalzon/ghinstallation/v2 v2.9.0 @@ -31,7 +31,7 @@ require ( gocloud.dev v0.36.0 golang.org/x/text v0.14.0 golang.org/x/tools v0.17.0 // indirect - google.golang.org/genproto v0.0.0-20240125205218-1f4bbc51befe // indirect + google.golang.org/genproto v0.0.0-20240213162025-012b6fc9bca9 // indirect google.golang.org/protobuf v1.32.0 gopkg.in/yaml.v2 v2.4.0 gopkg.in/yaml.v3 v3.0.1 @@ -45,15 +45,15 @@ require ( github.com/google/go-github/v53 v53.2.0 github.com/google/osv-scanner v1.6.2 github.com/mcuadros/go-jsonschema-generator v0.0.0-20200330054847-ba7a369d4303 - github.com/onsi/ginkgo/v2 v2.15.0 + github.com/onsi/ginkgo/v2 v2.16.0 github.com/otiai10/copy v1.14.0 sigs.k8s.io/release-utils v0.6.0 ) require ( cloud.google.com/go/compute/metadata v0.2.3 // indirect - cloud.google.com/go/containeranalysis v0.11.3 // indirect - cloud.google.com/go/kms v1.15.5 // indirect + cloud.google.com/go/containeranalysis v0.11.4 // indirect + cloud.google.com/go/kms v1.15.7 // indirect dario.cat/mergo v1.0.0 // indirect deps.dev/api/v3alpha v0.0.0-20240109042716-00b51ef52ece // indirect github.com/BurntSushi/toml v1.3.2 // indirect @@ -101,17 +101,17 @@ require ( github.com/spdx/gordf v0.0.0-20221230105357-b735bd5aac89 // indirect github.com/spdx/tools-golang v0.5.3 // indirect github.com/zeebo/xxh3 v1.0.2 // indirect - go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.47.0 // indirect - go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.47.0 // indirect - go.opentelemetry.io/otel v1.22.0 // indirect - go.opentelemetry.io/otel/metric v1.22.0 // indirect - go.opentelemetry.io/otel/trace v1.22.0 // indirect + go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.48.0 // indirect + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.48.0 // indirect + go.opentelemetry.io/otel v1.23.0 // indirect + go.opentelemetry.io/otel/metric v1.23.0 // indirect + go.opentelemetry.io/otel/trace v1.23.0 // indirect golang.org/x/mod v0.14.0 // indirect - golang.org/x/term v0.17.0 // indirect + golang.org/x/term v0.18.0 // indirect golang.org/x/time v0.5.0 // indirect golang.org/x/vuln v1.0.1 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20240205150955-31a09d347014 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20240205150955-31a09d347014 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20240221002015-b0ce06bbee7c // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20240213162025-012b6fc9bca9 // indirect gopkg.in/inf.v0 v0.9.1 // indirect k8s.io/api v0.28.2 // indirect k8s.io/apimachinery v0.28.2 // indirect @@ -126,7 +126,7 @@ require ( require ( cloud.google.com/go v0.112.0 // indirect - cloud.google.com/go/compute v1.23.3 // indirect + cloud.google.com/go/compute v1.24.0 // indirect cloud.google.com/go/iam v1.1.6 // indirect cloud.google.com/go/storage v1.37.0 // indirect github.com/Microsoft/go-winio v0.6.1 // indirect @@ -151,7 +151,7 @@ require ( github.com/google/uuid v1.6.0 // indirect github.com/google/wire v0.5.0 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect - github.com/googleapis/gax-go/v2 v2.12.0 // indirect + github.com/googleapis/gax-go/v2 v2.12.1 // indirect github.com/imdario/mergo v0.3.16 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect @@ -169,19 +169,19 @@ require ( github.com/sergi/go-diff v1.3.1 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/vbatts/tar-split v0.11.3 // indirect - github.com/xanzy/go-gitlab v0.97.0 + github.com/xanzy/go-gitlab v0.99.0 github.com/xanzy/ssh-agent v0.3.3 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect - golang.org/x/crypto v0.19.0 // indirect + golang.org/x/crypto v0.21.0 // indirect golang.org/x/exp v0.0.0-20240119083558-1b970713d09a // indirect - golang.org/x/net v0.21.0 // indirect - golang.org/x/oauth2 v0.17.0 + golang.org/x/net v0.22.0 // indirect + golang.org/x/oauth2 v0.18.0 golang.org/x/sync v0.6.0 // indirect - golang.org/x/sys v0.17.0 // indirect + golang.org/x/sys v0.18.0 // indirect golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028 // indirect - google.golang.org/api v0.162.0 // indirect + google.golang.org/api v0.166.0 // indirect google.golang.org/appengine v1.6.8 // indirect - google.golang.org/grpc v1.61.0 // indirect + google.golang.org/grpc v1.61.1 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect ) diff --git a/go.sum b/go.sum index 69da275b8d90..ef5e7972c9ef 100644 --- a/go.sum +++ b/go.sum @@ -16,12 +16,12 @@ cloud.google.com/go/bigquery v1.3.0/go.mod h1:PjpwJnslEMmckchkHFfq+HTD2DmtT67aNF cloud.google.com/go/bigquery v1.4.0/go.mod h1:S8dzgnTigyfTmLBfrtrhyYhwRxG72rYxvftPBK2Dvzc= cloud.google.com/go/bigquery v1.59.1 h1:CpT+/njKuKT3CEmswm6IbhNu9u35zt5dO4yPDLW+nG4= cloud.google.com/go/bigquery v1.59.1/go.mod h1:VP1UJYgevyTwsV7desjzNzDND5p6hZB+Z8gZJN1GQUc= -cloud.google.com/go/compute v1.23.3 h1:6sVlXXBmbd7jNX0Ipq0trII3e4n1/MsADLK6a+aiVlk= -cloud.google.com/go/compute v1.23.3/go.mod h1:VCgBUoMnIVIR0CscqQiPJLAG25E3ZRZMzcFZeQ+h8CI= +cloud.google.com/go/compute v1.24.0 h1:phWcR2eWzRJaL/kOiJwfFsPs4BaKq1j6vnpZrc1YlVg= +cloud.google.com/go/compute v1.24.0/go.mod h1:kw1/T+h/+tK2LJK0wiPPx1intgdAM3j/g3hFDlscY40= cloud.google.com/go/compute/metadata v0.2.3 h1:mg4jlk7mCAj6xXp9UJ4fjI9VUI5rubuGBW5aJ7UnBMY= cloud.google.com/go/compute/metadata v0.2.3/go.mod h1:VAV5nSsACxMJvgaAuX6Pk2AawlZn8kiOGuCv6gTkwuA= -cloud.google.com/go/containeranalysis v0.11.3 h1:5rhYLX+3a01drpREqBZVXR9YmWH45RnML++8NsCtuD8= -cloud.google.com/go/containeranalysis v0.11.3/go.mod h1:kMeST7yWFQMGjiG9K7Eov+fPNQcGhb8mXj/UcTiWw9U= +cloud.google.com/go/containeranalysis v0.11.4 h1:doJ0M1ljS4hS0D2UbHywlHGwB7sQLNrt9vFk9Zyi7vY= +cloud.google.com/go/containeranalysis v0.11.4/go.mod h1:cVZT7rXYBS9NG1rhQbWL9pWbXCKHWJPYraE8/FTSYPE= cloud.google.com/go/datacatalog v1.19.3 h1:A0vKYCQdxQuV4Pi0LL9p39Vwvg4jH5yYveMv50gU5Tw= cloud.google.com/go/datacatalog v1.19.3/go.mod h1:ra8V3UAsciBpJKQ+z9Whkxzxv7jmQg1hfODr3N3YPJ4= cloud.google.com/go/datastore v1.0.0/go.mod h1:LXYbyblFSglQ5pkeyhO+Qmw7ukd3C+pD7TKLgZqpHYE= @@ -29,24 +29,24 @@ cloud.google.com/go/datastore v1.1.0/go.mod h1:umbIZjpQpHh4hmRpGhH4tLFup+FVzqBi1 cloud.google.com/go/firestore v1.1.0/go.mod h1:ulACoGHTpvq5r8rxGJ4ddJZBZqakUQqClKRT5SZwBmk= cloud.google.com/go/iam v1.1.6 h1:bEa06k05IO4f4uJonbB5iAgKTPpABy1ayxaIZV/GHVc= cloud.google.com/go/iam v1.1.6/go.mod h1:O0zxdPeGBoFdWW3HWmBxJsk0pfvNM/p/qa82rWOGTwI= -cloud.google.com/go/kms v1.15.5 h1:pj1sRfut2eRbD9pFRjNnPNg/CzJPuQAzUujMIM1vVeM= -cloud.google.com/go/kms v1.15.5/go.mod h1:cU2H5jnp6G2TDpUGZyqTCoy1n16fbubHZjmVXSMtwDI= +cloud.google.com/go/kms v1.15.7 h1:7caV9K3yIxvlQPAcaFffhlT7d1qpxjB1wHBtjWa13SM= +cloud.google.com/go/kms v1.15.7/go.mod h1:ub54lbsa6tDkUwnu4W7Yt1aAIFLnspgh0kPGToDukeI= cloud.google.com/go/longrunning v0.5.5 h1:GOE6pZFdSrTb4KAiKnXsJBtlE6mEyaW44oKyMILWnOg= cloud.google.com/go/longrunning v0.5.5/go.mod h1:WV2LAxD8/rg5Z1cNW6FJ/ZpX4E4VnDnoTk0yawPBB7s= -cloud.google.com/go/monitoring v1.17.0 h1:blrdvF0MkPPivSO041ihul7rFMhXdVp8Uq7F59DKXTU= -cloud.google.com/go/monitoring v1.17.0/go.mod h1:KwSsX5+8PnXv5NJnICZzW2R8pWTis8ypC4zmdRD63Tw= +cloud.google.com/go/monitoring v1.18.0 h1:NfkDLQDG2UR3WYZVQE8kwSbUIEyIqJUPl+aOQdFH1T4= +cloud.google.com/go/monitoring v1.18.0/go.mod h1:c92vVBCeq/OB4Ioyo+NbN2U7tlg5ZH41PZcdvfc+Lcg= cloud.google.com/go/pubsub v1.0.1/go.mod h1:R0Gpsv3s54REJCy4fxDixWD93lHJMoZTyQ2kNxGRt3I= 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.36.1 h1:dfEPuGCHGbWUhaMCTHUFjfroILEkx55iUmKBZTP5f+Y= -cloud.google.com/go/pubsub v1.36.1/go.mod h1:iYjCa9EzWOoBiTdd4ps7QoMtMln5NwaZQpK1hbRfBDE= +cloud.google.com/go/pubsub v1.36.2 h1:nAUD4aiWHZFYyINhRag1qOnHUk0/7QiWEa04XWnqACA= +cloud.google.com/go/pubsub v1.36.2/go.mod h1:mHCFLNG8abCrPzhuOnpBcr9DUy+l3/LWWn0qoJdbh1w= 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= cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohlUTyfDhBk= cloud.google.com/go/storage v1.37.0 h1:WI8CsaFO8Q9KjPVtsZ5Cmi0dXV25zMoX0FklT7c3Jm4= cloud.google.com/go/storage v1.37.0/go.mod h1:i34TiT2IhiNDmcj65PqwCjcoUX7Z5pLzS8DEmoiFq1k= -cloud.google.com/go/trace v1.10.4 h1:2qOAuAzNezwW3QN+t41BtkDJOG42HywL73q8x/f6fnM= -cloud.google.com/go/trace v1.10.4/go.mod h1:Nso99EDIK8Mj5/zmB+iGr9dosS/bzWCJ8wGmE6TXNWY= +cloud.google.com/go/trace v1.10.5 h1:0pr4lIKJ5XZFYD9GtxXEWr0KkVeigc3wlGpZco0X1oA= +cloud.google.com/go/trace v1.10.5/go.mod h1:9hjCV1nGBCtXbAE4YK7OqJ8pmPYSxPA0I67JwRd5s3M= contrib.go.opencensus.io/exporter/stackdriver v0.13.14 h1:zBakwHardp9Jcb8sQHcHpXy/0+JIb1M8KjigCJzx7+4= contrib.go.opencensus.io/exporter/stackdriver v0.13.14/go.mod h1:5pSSGY0Bhuk7waTHuDf4aQ8D2DrhgETRo9fy6k3Xlzc= dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk= @@ -436,8 +436,8 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.2 h1:Vie5ybvEvT75RniqhfF github.com/googleapis/enterprise-certificate-proxy v0.3.2/go.mod h1:VLSiSSBs/ksPL8kq3OBOQ6WRI2QnaFynd1DCjZ62+V0= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= -github.com/googleapis/gax-go/v2 v2.12.0 h1:A+gCJKdRfqXkr+BIRGtZLibNXf0m1f9E4HG56etFpas= -github.com/googleapis/gax-go/v2 v2.12.0/go.mod h1:y+aIqrI5eb1YGMVJfuV3185Ts/D7qKpsEkdD5+I6QGU= +github.com/googleapis/gax-go/v2 v2.12.1 h1:9F8GV9r9ztXyAi00gsMQHNoF51xPZm8uj1dpYt2ZETM= +github.com/googleapis/gax-go/v2 v2.12.1/go.mod h1:61M8vcyyXR2kqKFxKrfA22jaA8JGF7Dc8App1U3H6jc= github.com/googleapis/gnostic v0.0.0-20170729233727-0c5108395e2d/go.mod h1:sJBsCZ4ayReDTBIg8b9dl28c5xFWyhBTVRp3pOg5EKY= github.com/googleapis/gnostic v0.1.0/go.mod h1:sJBsCZ4ayReDTBIg8b9dl28c5xFWyhBTVRp3pOg5EKY= github.com/googleapis/gnostic v0.2.2/go.mod h1:sJBsCZ4ayReDTBIg8b9dl28c5xFWyhBTVRp3pOg5EKY= @@ -610,8 +610,8 @@ github.com/onsi/ginkgo v1.11.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+ github.com/onsi/ginkgo v1.12.0/go.mod h1:oUhWkIvk5aDxtKvDDuw8gItl8pKl42LzjC9KZE0HfGg= github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk= github.com/onsi/ginkgo v1.14.2/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY= -github.com/onsi/ginkgo/v2 v2.15.0 h1:79HwNRBAZHOEwrczrgSOPy+eFTTlIGELKy5as+ClttY= -github.com/onsi/ginkgo/v2 v2.15.0/go.mod h1:HlxMHtYF57y6Dpf+mc5529KKmSq9h2FpCF+/ZkwUxKM= +github.com/onsi/ginkgo/v2 v2.16.0 h1:7q1w9frJDzninhXxjZd+Y/x54XNjG/UlRLIYPZafsPM= +github.com/onsi/ginkgo/v2 v2.16.0/go.mod h1:llBI3WDLL9Z6taip6f33H76YcWtJv+7R3HigUjbIBOs= github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= @@ -772,8 +772,8 @@ github.com/vdemeester/k8s-pkg-credentialprovider v1.18.1-0.20201019120933-f1d169 github.com/vmihailenco/msgpack/v4 v4.3.12/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+NXzzngzBKDPIqw4= github.com/vmihailenco/tagparser v0.1.1/go.mod h1:OeAg3pn3UbLjkWt+rN9oFYB6u/cQgqMEUPoW2WPyhdI= github.com/vmware/govmomi v0.20.3/go.mod h1:URlwyTFZX72RmxtxuaFL2Uj3fD1JTvZdx59bHWk6aFU= -github.com/xanzy/go-gitlab v0.97.0 h1:StMqJ1Kvt00X43pYIBBjj52dFlghwSeBhRDRfzaZ7xY= -github.com/xanzy/go-gitlab v0.97.0/go.mod h1:ETg8tcj4OhrB84UEgeE8dSuV/0h4BBL1uOV/qK0vlyI= +github.com/xanzy/go-gitlab v0.99.0 h1:0W5dmFQejPlqnScZoGRXNPmx+evOxBMk50P40cxlnWU= +github.com/xanzy/go-gitlab v0.99.0/go.mod h1:ETg8tcj4OhrB84UEgeE8dSuV/0h4BBL1uOV/qK0vlyI= github.com/xanzy/ssh-agent v0.3.3 h1:+/15pJfg/RsTxqYcX6fHqOXZwwMP+2VyYWJeWM2qQFM= github.com/xanzy/ssh-agent v0.3.3/go.mod h1:6dzNDKs0J9rVPHPhaGCukekBHKqfl+L3KghI1Bc68Uw= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c= @@ -802,18 +802,18 @@ go.opencensus.io v0.22.2/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.22.3/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0= go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo= -go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.47.0 h1:UNQQKPfTDe1J81ViolILjTKPr9WetKW6uei2hFgJmFs= -go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.47.0/go.mod h1:r9vWsPS/3AQItv3OSlEJ/E4mbrhUbbw18meOjArPtKQ= -go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.47.0 h1:sv9kVfal0MK0wBMCOGr+HeJm9v803BkJxGrk2au7j08= -go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.47.0/go.mod h1:SK2UL73Zy1quvRPonmOmRDiWk1KBV3LyIeeIxcEApWw= -go.opentelemetry.io/otel v1.22.0 h1:xS7Ku+7yTFvDfDraDIJVpw7XPyuHlB9MCiqqX5mcJ6Y= -go.opentelemetry.io/otel v1.22.0/go.mod h1:eoV4iAi3Ea8LkAEI9+GFT44O6T/D0GWAVFyZVCC6pMI= -go.opentelemetry.io/otel/metric v1.22.0 h1:lypMQnGyJYeuYPhOM/bgjbFM6WE44W1/T45er4d8Hhg= -go.opentelemetry.io/otel/metric v1.22.0/go.mod h1:evJGjVpZv0mQ5QBRJoBF64yMuOf4xCWdXjK8pzFvliY= +go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.48.0 h1:P+/g8GpuJGYbOp2tAdKrIPUX9JO02q8Q0YNlHolpibA= +go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.48.0/go.mod h1:tIKj3DbO8N9Y2xo52og3irLsPI4GW02DSMtrVgNMgxg= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.48.0 h1:doUP+ExOpH3spVTLS0FcWGLnQrPct/hD/bCPbDRUEAU= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.48.0/go.mod h1:rdENBZMT2OE6Ne/KLwpiXudnAsbdrdBaqBvTN8M8BgA= +go.opentelemetry.io/otel v1.23.0 h1:Df0pqjqExIywbMCMTxkAwzjLZtRf+bBKLbUcpxO2C9E= +go.opentelemetry.io/otel v1.23.0/go.mod h1:YCycw9ZeKhcJFrb34iVSkyT0iczq/zYDtZYFufObyB0= +go.opentelemetry.io/otel/metric v1.23.0 h1:pazkx7ss4LFVVYSxYew7L5I6qvLXHA0Ap2pwV+9Cnpo= +go.opentelemetry.io/otel/metric v1.23.0/go.mod h1:MqUW2X2a6Q8RN96E2/nqNoT+z9BSms20Jb7Bbp+HiTo= go.opentelemetry.io/otel/sdk v1.21.0 h1:FTt8qirL1EysG6sTQRZ5TokkU8d0ugCj8htOgThZXQ8= go.opentelemetry.io/otel/sdk v1.21.0/go.mod h1:Nna6Yv7PWTdgJHVRD9hIYywQBRx7pbox6nwBnZIxl/E= -go.opentelemetry.io/otel/trace v1.22.0 h1:Hg6pPujv0XG9QaVbGOBVHunyuLcCC3jN7WEhPx83XD0= -go.opentelemetry.io/otel/trace v1.22.0/go.mod h1:RbbHXVqKES9QhzZq/fE5UnOSILqRt40a21sPw2He1xo= +go.opentelemetry.io/otel/trace v1.23.0 h1:37Ik5Ib7xfYVb4V1UtnT97T1jI+AoIYkJyPkuL4iJgI= +go.opentelemetry.io/otel/trace v1.23.0/go.mod h1:GSGTbIClEsuZrGIzoEHqsVfxgn5UkggkflQwDScNUsk= go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= @@ -837,8 +837,8 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.3.1-0.20221117191849-2c476679df9a/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4= golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU= -golang.org/x/crypto v0.19.0 h1:ENy+Az/9Y1vSrlrvBSyna3PITt4tiZLf7sgCjZBX7Wo= -golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= +golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA= +golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -920,15 +920,15 @@ golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= -golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= -golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= +golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc= +golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= -golang.org/x/oauth2 v0.17.0 h1:6m3ZPmLEFdVxKKWnKq4VqZ60gutO35zm+zrAHVmHyDQ= -golang.org/x/oauth2 v0.17.0/go.mod h1:OzPDGQiuQMguemayvdylqddI7qcD9lnSDb+1FiwQ5HA= +golang.org/x/oauth2 v0.18.0 h1:09qnuIAgzdx1XplqJvW6CQqMCtGZykZWcXzPMPUusvI= +golang.org/x/oauth2 v0.18.0/go.mod h1:Wf7knwG0MPoWIMMBgFlEaSUDaKskp0dCfrlJRJXbBi8= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -1000,8 +1000,8 @@ golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= +golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20201210144234-2321bbc49cbf/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= @@ -1009,8 +1009,8 @@ golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuX golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U= -golang.org/x/term v0.17.0 h1:mkTF7LCd6WGJNL3K1Ad7kwxNfYAW6a8a8QqtMblp/4U= -golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= +golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8= +golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= golang.org/x/text v0.0.0-20160726164857-2910a502d2bf/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -1105,8 +1105,8 @@ google.golang.org/api v0.15.0/go.mod h1:iLdEw5Ide6rF15KTC1Kkl0iskquN2gFfn9o9XIsb google.golang.org/api v0.17.0/go.mod h1:BwFmGc8tA3vsd7r/7kR8DY7iEEGSU04BFxCo5jP/sfE= google.golang.org/api v0.18.0/go.mod h1:BwFmGc8tA3vsd7r/7kR8DY7iEEGSU04BFxCo5jP/sfE= google.golang.org/api v0.22.0/go.mod h1:BwFmGc8tA3vsd7r/7kR8DY7iEEGSU04BFxCo5jP/sfE= -google.golang.org/api v0.162.0 h1:Vhs54HkaEpkMBdgGdOT2P6F0csGG/vxDS0hWHJzmmps= -google.golang.org/api v0.162.0/go.mod h1:6SulDkfoBIg4NFmCuZ39XeeAgSHCPecfSUuDyYlAHs0= +google.golang.org/api v0.166.0 h1:6m4NUwrZYhAaVIHZWxaKjw1L1vNAjtMwORmKRyEEo24= +google.golang.org/api v0.166.0/go.mod h1:4FcBc686KFi7QI/U51/2GKKevfZMpM17sCdibqe/bSA= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/appengine v1.5.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= @@ -1136,12 +1136,12 @@ google.golang.org/genproto v0.0.0-20200430143042-b979b6f78d84/go.mod h1:55QSHmfG google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo= google.golang.org/genproto v0.0.0-20200527145253-8367513e4ece/go.mod h1:jDfRM7FcilCzHH/e9qn6dsT145K34l5v+OpcnNgKAAA= google.golang.org/genproto v0.0.0-20201203001206-6486ece9c497/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= -google.golang.org/genproto v0.0.0-20240125205218-1f4bbc51befe h1:USL2DhxfgRchafRvt/wYyyQNzwgL7ZiURcozOE/Pkvo= -google.golang.org/genproto v0.0.0-20240125205218-1f4bbc51befe/go.mod h1:cc8bqMqtv9gMOr0zHg2Vzff5ULhhL2IXP4sbcn32Dro= -google.golang.org/genproto/googleapis/api v0.0.0-20240205150955-31a09d347014 h1:x9PwdEgd11LgK+orcck69WVRo7DezSO4VUMPI4xpc8A= -google.golang.org/genproto/googleapis/api v0.0.0-20240205150955-31a09d347014/go.mod h1:rbHMSEDyoYX62nRVLOCc4Qt1HbsdytAYoVwgjiOhF3I= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240205150955-31a09d347014 h1:FSL3lRCkhaPFxqi0s9o+V4UI2WTzAVOvkgbd4kVV4Wg= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240205150955-31a09d347014/go.mod h1:SaPjaZGWb0lPqs6Ittu0spdfrOArqji4ZdeP5IC/9N4= +google.golang.org/genproto v0.0.0-20240213162025-012b6fc9bca9 h1:9+tzLLstTlPTRyJTh+ah5wIMsBW5c4tQwGTN3thOW9Y= +google.golang.org/genproto v0.0.0-20240213162025-012b6fc9bca9/go.mod h1:mqHbVIp48Muh7Ywss/AD6I5kNVKZMmAa/QEW58Gxp2s= +google.golang.org/genproto/googleapis/api v0.0.0-20240221002015-b0ce06bbee7c h1:9g7erC9qu44ks7UK4gDNlnk4kOxZG707xKm4jVniy6o= +google.golang.org/genproto/googleapis/api v0.0.0-20240221002015-b0ce06bbee7c/go.mod h1:5iCWqnniDlqZHrd3neWVTOwvh/v6s3232omMecelax8= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240213162025-012b6fc9bca9 h1:hZB7eLIaYlW9qXRfCq/qDaPdbeY3757uARz5Vvfv+cY= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240213162025-012b6fc9bca9/go.mod h1:YUWgXUFRPfoYK1IHMuxH5K6nPEXSCzIMljnQ59lLRCk= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= google.golang.org/grpc v1.21.0/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= @@ -1154,8 +1154,8 @@ google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8 google.golang.org/grpc v1.27.1/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.29.1/go.mod h1:itym6AZVZYACWQqET3MqgPpjcuV5QH3BxFS3IjizoKk= google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc= -google.golang.org/grpc v1.61.0 h1:TOvOcuXn30kRao+gfcvsebNEa5iZIiLkisYEkf7R7o0= -google.golang.org/grpc v1.61.0/go.mod h1:VUbo7IFqmF1QtCAstipjG0GIoq49KvMe9+h1jFLBNJs= +google.golang.org/grpc v1.61.1 h1:kLAiWrZs7YeDM6MumDe7m3y4aM6wacLzM1Y/wiLP9XY= +google.golang.org/grpc v1.61.1/go.mod h1:VUbo7IFqmF1QtCAstipjG0GIoq49KvMe9+h1jFLBNJs= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/probes/blocksDeleteOnBranches/impl.go b/probes/blocksDeleteOnBranches/impl.go index bd295fa44759..a48bf1923fda 100644 --- a/probes/blocksDeleteOnBranches/impl.go +++ b/probes/blocksDeleteOnBranches/impl.go @@ -40,6 +40,15 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { r := raw.BranchProtectionResults var findings []finding.Finding + if len(r.Branches) == 0 { + f, err := finding.NewWith(fs, Probe, "no branches found", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + for i := range r.Branches { branch := &r.Branches[i] diff --git a/probes/blocksForcePushOnBranches/impl.go b/probes/blocksForcePushOnBranches/impl.go index d87921c1dd13..41871d7eae44 100644 --- a/probes/blocksForcePushOnBranches/impl.go +++ b/probes/blocksForcePushOnBranches/impl.go @@ -40,8 +40,18 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { r := raw.BranchProtectionResults var findings []finding.Finding + if len(r.Branches) == 0 { + f, err := finding.NewWith(fs, Probe, "no branches found", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + for i := range r.Branches { branch := &r.Branches[i] + var text string var outcome finding.Outcome switch { diff --git a/probes/branchProtectionAppliesToAdmins/impl.go b/probes/branchProtectionAppliesToAdmins/impl.go index 6170ff5cb80e..14fcec6973f7 100644 --- a/probes/branchProtectionAppliesToAdmins/impl.go +++ b/probes/branchProtectionAppliesToAdmins/impl.go @@ -41,6 +41,15 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { r := raw.BranchProtectionResults var findings []finding.Finding + if len(r.Branches) == 0 { + f, err := finding.NewWith(fs, Probe, "no branches found", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + for i := range r.Branches { branch := &r.Branches[i] diff --git a/probes/branchesAreProtected/def.yml b/probes/branchesAreProtected/def.yml new file mode 100644 index 000000000000..ddbb00a89d35 --- /dev/null +++ b/probes/branchesAreProtected/def.yml @@ -0,0 +1,30 @@ +# Copyright 2023 OpenSSF 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. + +id: branchesAreProtected +short: Check that the projects branches are protected. +motivation: > + Branches that are not protected may allow excessive actions that could compromise the projects security. +implementation: > + Checks the protection rules of default and release branches. +outcome: + - The probe returns one OutcomePositive for each branch that is protected, and one OutcomeNegative for branches that are not protected. Scorecard only considers default and releases branches. +remediation: + effort: Low + text: + - For Gitlab-hosted project, follow the documentation on how to protect branches, https://docs.gitlab.com/ee/user/project/protected_branches.html + - For GitHub-hosted projects, follow [the documentation on protected branches, https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches + markdown: + - For Gitlab-hosted project, follow [the documentation on how to protect branches](https://docs.gitlab.com/ee/user/project/protected_branches.html) + - For GitHub-hosted projects, follow [the documentation on protected branches](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches) diff --git a/probes/branchesAreProtected/impl.go b/probes/branchesAreProtected/impl.go new file mode 100644 index 000000000000..e35b9dd6150d --- /dev/null +++ b/probes/branchesAreProtected/impl.go @@ -0,0 +1,73 @@ +// Copyright 2023 OpenSSF 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. + +//nolint:stylecheck +package branchesAreProtected + +import ( + "embed" + "fmt" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/internal/utils/uerror" +) + +//go:embed *.yml +var fs embed.FS + +const ( + Probe = "branchesAreProtected" + BranchNameKey = "branchName" +) + +func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { + if raw == nil { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + + r := raw.BranchProtectionResults + var findings []finding.Finding + + if len(r.Branches) == 0 { + f, err := finding.NewWith(fs, Probe, "no branches found", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + + for i := range r.Branches { + branch := &r.Branches[i] + + protected := (branch.Protected != nil && *branch.Protected) + var text string + var outcome finding.Outcome + if protected { + text = fmt.Sprintf("branch '%s' is protected", *branch.Name) + outcome = finding.OutcomePositive + } else { + text = fmt.Sprintf("branch '%s' is not protected", *branch.Name) + outcome = finding.OutcomeNegative + } + f, err := finding.NewWith(fs, Probe, text, nil, outcome) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + f = f.WithValue(BranchNameKey, *branch.Name) + findings = append(findings, *f) + } + return findings, Probe, nil +} diff --git a/probes/branchesAreProtected/impl_test.go b/probes/branchesAreProtected/impl_test.go new file mode 100644 index 000000000000..823f888da65b --- /dev/null +++ b/probes/branchesAreProtected/impl_test.go @@ -0,0 +1,169 @@ +// Copyright 2023 OpenSSF 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. + +//nolint:stylecheck +package branchesAreProtected + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/clients" + "github.com/ossf/scorecard/v4/finding" +) + +func Test_Run(t *testing.T) { + t.Parallel() + trueVal := true + falseVal := false + branchVal1 := "branch-name1" + branchVal2 := "branch-name1" + + //nolint:govet + tests := []struct { + name string + raw *checker.RawResults + outcomes []finding.Outcome + err error + }{ + { + name: "One branch. Protection unknown", + raw: &checker.RawResults{ + BranchProtectionResults: checker.BranchProtectionsData{ + Branches: []clients.BranchRef{ + { + Name: &branchVal1, + Protected: nil, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + { + name: "Two protected branches", + raw: &checker.RawResults{ + BranchProtectionResults: checker.BranchProtectionsData{ + Branches: []clients.BranchRef{ + { + Name: &branchVal1, + Protected: &trueVal, + }, + { + Name: &branchVal2, + Protected: &trueVal, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, finding.OutcomePositive, + }, + }, + { + name: "Two branches. First is protected", + raw: &checker.RawResults{ + BranchProtectionResults: checker.BranchProtectionsData{ + Branches: []clients.BranchRef{ + { + Name: &branchVal1, + Protected: &trueVal, + }, + { + Name: &branchVal2, + Protected: &falseVal, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, finding.OutcomeNegative, + }, + }, + { + name: "Two branches. Second is protected", + raw: &checker.RawResults{ + BranchProtectionResults: checker.BranchProtectionsData{ + Branches: []clients.BranchRef{ + { + Name: &branchVal1, + Protected: &falseVal, + }, + { + Name: &branchVal2, + Protected: &trueVal, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, finding.OutcomePositive, + }, + }, + { + name: "Two branches. First one is not protected, second unknown", + raw: &checker.RawResults{ + BranchProtectionResults: checker.BranchProtectionsData{ + Branches: []clients.BranchRef{ + { + Name: &branchVal1, + Protected: &falseVal, + }, + { + Name: &branchVal2, + BranchProtectionRule: clients.BranchProtectionRule{ + AllowDeletions: nil, + }, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, finding.OutcomeNegative, + }, + }, + } + 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() + + findings, s, err := Run(tt.raw) + if !cmp.Equal(tt.err, err, cmpopts.EquateErrors()) { + t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(tt.err, err, cmpopts.EquateErrors())) + } + if err != nil { + return + } + if diff := cmp.Diff(Probe, s); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(len(tt.outcomes), len(findings)); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + for i := range tt.outcomes { + outcome := &tt.outcomes[i] + f := &findings[i] + if diff := cmp.Diff(*outcome, f.Outcome); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + } + }) + } +} diff --git a/probes/codeApproved/def.yml b/probes/codeApproved/def.yml index 7bd82284a046..2ba1154bb97d 100644 --- a/probes/codeApproved/def.yml +++ b/probes/codeApproved/def.yml @@ -19,10 +19,13 @@ motivation: > To ensure that the review process works, the proposed changes should have a minimum number of approvals. implementation: > - This probe looks for whether all changes over the last `--commit-depth` commits have been approved before merge. Commits are grouped by the Pull Request they were introduced in, and each Pull Request must have at least one approval. + This probe looks for whether all changes over the last `--commit-depth` commits have been approved before merge. + Commits are grouped by the changeset they were introduced in, and each changesets must have at least one approval. + Reviewed, bot authored changesets (e.g. dependabot) are not counted. outcome: - - If all commits were approved, the probe returns OutcomePositive (1) - - If any commit was not approved, the prove returns OutcomeNegative (0) + - If all commits were approved, the probe returns OutcomePositive + - If any commits were not approved, the probe returns OutcomeNegative + - If there are no changes, the probe returns OutcomeNotApplicable remediation: effort: Low text: diff --git a/probes/codeApproved/impl.go b/probes/codeApproved/impl.go index 631be68d1fb1..c2d74789a418 100644 --- a/probes/codeApproved/impl.go +++ b/probes/codeApproved/impl.go @@ -17,63 +17,69 @@ package codeApproved import ( "embed" + "errors" "fmt" + "strconv" "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/finding" - "github.com/ossf/scorecard/v4/probes/utils" + "github.com/ossf/scorecard/v4/probes/internal/utils/uerror" ) -//go:embed *.yml -var fs embed.FS +var ( + //go:embed *.yml + fs embed.FS -const probe = "codeApproved" + errNoAuthor = errors.New("could not retrieve changeset author") + errNoReviewer = errors.New("could not retrieve the changeset reviewer") +) + +const ( + Probe = "codeApproved" + NumApprovedKey = "approvedChangesets" + NumTotalKey = "totalChangesets" +) func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { + if raw == nil { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } rawReviewData := &raw.CodeReviewResults - return approvedRun(rawReviewData, fs, probe, finding.OutcomePositive, finding.OutcomeNegative) + return approvedRun(rawReviewData, fs, Probe) } // Looks through the data and validates that each changeset has been approved at least once. - -//nolint:gocognit -func approvedRun(reviewData *checker.CodeReviewData, fs embed.FS, probeID string, - positiveOutcome, negativeOutcome finding.Outcome, -) ([]finding.Finding, string, error) { +func approvedRun(reviewData *checker.CodeReviewData, fs embed.FS, probeID string) ([]finding.Finding, string, error) { changesets := reviewData.DefaultBranchChangesets var findings []finding.Finding + + if len(changesets) == 0 { + f, err := finding.NewWith(fs, Probe, "no changesets detected", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + foundHumanActivity := false nChangesets := len(changesets) nChanges := 0 - nUnapprovedChangesets := 0 - if nChangesets == 0 { - return nil, probeID, utils.ErrNoChangesets - } + nApproved := 0 + for x := range changesets { data := &changesets[x] - if data.Author.Login == "" { - f, err := finding.NewNotAvailable(fs, probeID, "Could not retrieve the author of a changeset.", nil) + approvedChangeset, err := approved(data) + if err != nil { + f, err := finding.NewWith(fs, probeID, err.Error(), nil, finding.OutcomeError) if err != nil { return nil, probeID, fmt.Errorf("create finding: %w", err) } findings = append(findings, *f) return findings, probeID, nil } - approvedChangeset := false - for y := range data.Reviews { - if data.Reviews[y].Author.Login == "" { - f, err := finding.NewNotAvailable(fs, probeID, "Could not retrieve the reviewer of a changeset.", nil) - if err != nil { - return nil, probeID, fmt.Errorf("create finding: %w", err) - } - findings = append(findings, *f) - return findings, probeID, nil - } - if data.Reviews[y].State == "APPROVED" && data.Reviews[y].Author.Login != data.Author.Login { - approvedChangeset = true - break - } - } + // skip bot authored changesets, which can skew single maintainer projects which otherwise dont code review + // https://github.com/ossf/scorecard/issues/2450 if approvedChangeset && data.Author.IsBot { continue } @@ -81,36 +87,44 @@ func approvedRun(reviewData *checker.CodeReviewData, fs embed.FS, probeID string if !data.Author.IsBot { foundHumanActivity = true } - if !approvedChangeset { - nUnapprovedChangesets += 1 + if approvedChangeset { + nApproved += 1 } } + var outcome finding.Outcome + var reason string switch { + case nApproved != nChanges: + outcome = finding.OutcomeNegative + reason = fmt.Sprintf("Found %d/%d approved changesets", nApproved, nChanges) case !foundHumanActivity: - // returns a NotAvailable outcome if all changesets were authored by bots - f, err := finding.NewNotAvailable(fs, probeID, fmt.Sprintf("Found no human activity "+ - "in the last %d changesets", nChangesets), nil) - if err != nil { - return nil, probeID, fmt.Errorf("create finding: %w", err) - } - findings = append(findings, *f) - return findings, probeID, nil - case nUnapprovedChangesets > 0: - // returns NegativeOutcome if not all changesets were approved - f, err := finding.NewWith(fs, probeID, fmt.Sprintf("Not all changesets approved. "+ - "Found %d unapproved changesets of %d.", nUnapprovedChangesets, nChanges), nil, negativeOutcome) - if err != nil { - return nil, probeID, fmt.Errorf("create finding: %w", err) - } - findings = append(findings, *f) + outcome = finding.OutcomeNotApplicable + reason = fmt.Sprintf("Found no human activity in the last %d changesets", nChangesets) default: - // returns PositiveOutcome if all changesets have been approved - f, err := finding.NewWith(fs, probeID, fmt.Sprintf("All %d changesets approved.", - nChangesets), nil, positiveOutcome) - if err != nil { - return nil, probeID, fmt.Errorf("create finding: %w", err) - } - findings = append(findings, *f) + outcome = finding.OutcomePositive + reason = "All changesets approved" + } + f, err := finding.NewWith(fs, probeID, reason, nil, outcome) + if err != nil { + return nil, probeID, fmt.Errorf("create finding: %w", err) } + f.WithValue(NumApprovedKey, strconv.Itoa(nApproved)) + f.WithValue(NumTotalKey, strconv.Itoa(nChanges)) + findings = append(findings, *f) return findings, probeID, nil } + +func approved(c *checker.Changeset) (bool, error) { + if c.Author.Login == "" { + return false, errNoAuthor + } + for _, review := range c.Reviews { + if review.Author.Login == "" { + return false, errNoReviewer + } + if review.State == "APPROVED" && review.Author.Login != c.Author.Login { + return true, nil + } + } + return false, nil +} diff --git a/probes/codeApproved/impl_test.go b/probes/codeApproved/impl_test.go index 567851f29632..7defde3747aa 100644 --- a/probes/codeApproved/impl_test.go +++ b/probes/codeApproved/impl_test.go @@ -16,23 +16,21 @@ package codeApproved import ( - "errors" "testing" "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/clients" "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/internal/utils/test" ) -var errProbeReturned = errors.New("probe run failure") - func TestProbeCodeApproved(t *testing.T) { t.Parallel() probeTests := []struct { name string rawResults *checker.RawResults err error - expectedFindings []finding.Finding + expectedOutcomes []finding.Outcome }{ { name: "no changesets", @@ -41,8 +39,9 @@ func TestProbeCodeApproved(t *testing.T) { DefaultBranchChangesets: []checker.Changeset{}, }, }, - err: errProbeReturned, - expectedFindings: nil, + expectedOutcomes: []finding.Outcome{ + finding.OutcomeNotApplicable, + }, }, { name: "no changesets no authors", @@ -60,11 +59,8 @@ func TestProbeCodeApproved(t *testing.T) { }, }, }, - expectedFindings: []finding.Finding{ - { - Probe: "codeApproved", - Outcome: finding.OutcomeNotAvailable, - }, + expectedOutcomes: []finding.Outcome{ + finding.OutcomeError, }, }, { @@ -99,11 +95,8 @@ func TestProbeCodeApproved(t *testing.T) { }, }, }, - expectedFindings: []finding.Finding{ - { - Probe: "codeApproved", - Outcome: finding.OutcomeNotAvailable, - }, + expectedOutcomes: []finding.Outcome{ + finding.OutcomeNotApplicable, }, }, { @@ -126,11 +119,8 @@ func TestProbeCodeApproved(t *testing.T) { }, }, }, - expectedFindings: []finding.Finding{ - { - Probe: "codeApproved", - Outcome: finding.OutcomeNotAvailable, - }, + expectedOutcomes: []finding.Outcome{ + finding.OutcomeError, }, }, { @@ -149,15 +139,12 @@ func TestProbeCodeApproved(t *testing.T) { }, }, }, - expectedFindings: []finding.Finding{ - { - Probe: "codeApproved", - Outcome: finding.OutcomeNegative, - }, + expectedOutcomes: []finding.Outcome{ + finding.OutcomeNegative, }, }, { - name: "all authors are bots", + name: "only approved bot PRs gives not applicable outcome", rawResults: &checker.RawResults{ CodeReviewResults: checker.CodeReviewData{ DefaultBranchChangesets: []checker.Changeset{ @@ -173,7 +160,12 @@ func TestProbeCodeApproved(t *testing.T) { Message: "Title\nPiperOrigin-RevId: 444529962", }, }, - Reviews: []clients.Review{}, + Reviews: []clients.Review{ + { + Author: &clients.User{Login: "baldur"}, + State: "APPROVED", + }, + }, Author: clients.User{ Login: "bot", IsBot: true, @@ -190,7 +182,12 @@ func TestProbeCodeApproved(t *testing.T) { }, }, }, - Reviews: []clients.Review{}, + Reviews: []clients.Review{ + { + Author: &clients.User{Login: "baldur"}, + State: "APPROVED", + }, + }, Author: clients.User{ Login: "bot", IsBot: true, @@ -199,11 +196,8 @@ func TestProbeCodeApproved(t *testing.T) { }, }, }, - expectedFindings: []finding.Finding{ - { - Probe: "codeApproved", - Outcome: finding.OutcomeNotAvailable, - }, + expectedOutcomes: []finding.Outcome{ + finding.OutcomeNotApplicable, }, }, { @@ -230,11 +224,8 @@ func TestProbeCodeApproved(t *testing.T) { }, }, }, - expectedFindings: []finding.Finding{ - { - Probe: "codeApproved", - Outcome: finding.OutcomeNegative, - }, + expectedOutcomes: []finding.Outcome{ + finding.OutcomeNegative, }, }, { @@ -274,11 +265,8 @@ func TestProbeCodeApproved(t *testing.T) { }, }, }, - expectedFindings: []finding.Finding{ - { - Probe: "codeApproved", - Outcome: finding.OutcomePositive, - }, + expectedOutcomes: []finding.Outcome{ + finding.OutcomePositive, }, }, { @@ -310,12 +298,36 @@ func TestProbeCodeApproved(t *testing.T) { }, }, }, - expectedFindings: []finding.Finding{ - { - Probe: "codeApproved", - Outcome: finding.OutcomePositive, + expectedOutcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + }, + { + name: "only unreviewed bot changesets gives negative outcome", + rawResults: &checker.RawResults{ + CodeReviewResults: checker.CodeReviewData{ + DefaultBranchChangesets: []checker.Changeset{ + { + ReviewPlatform: checker.ReviewPlatformGitHub, + Commits: []clients.Commit{ + { + SHA: "sha", + Committer: clients.User{Login: "dependabot"}, + Message: "foo", + }, + }, + Reviews: []clients.Review{}, + Author: clients.User{ + IsBot: true, + Login: "dependabot", + }, + }, + }, }, }, + expectedOutcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, }, } @@ -331,15 +343,10 @@ func TestProbeCodeApproved(t *testing.T) { t.Errorf("Expected error %v, got nil", tt.err) case res == nil && err == nil: t.Errorf("Probe returned nil for both finding and error") - case probeID != probe: + case probeID != Probe: t.Errorf("Probe returned the wrong probe ID") default: - for i := range tt.expectedFindings { - if tt.expectedFindings[i].Outcome != res[i].Outcome { - t.Errorf("Code-review probe: %v error: test name: \"%v\", wanted outcome %v, got %v", - res[i].Probe, tt.name, tt.expectedFindings[i].Outcome, res[i].Outcome) - } - } + test.AssertOutcomes(t, res, tt.expectedOutcomes) } }) } diff --git a/probes/dismissesStaleReviews/impl.go b/probes/dismissesStaleReviews/impl.go index fd63d7646da8..814122bfe1c0 100644 --- a/probes/dismissesStaleReviews/impl.go +++ b/probes/dismissesStaleReviews/impl.go @@ -41,6 +41,15 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { r := raw.BranchProtectionResults var findings []finding.Finding + if len(r.Branches) == 0 { + f, err := finding.NewWith(fs, Probe, "no branches found", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + for i := range r.Branches { branch := &r.Branches[i] diff --git a/probes/entries.go b/probes/entries.go index ecaab984c6de..14bba5ec7045 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -19,9 +19,14 @@ import ( "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/blocksDeleteOnBranches" + "github.com/ossf/scorecard/v4/probes/blocksForcePushOnBranches" + "github.com/ossf/scorecard/v4/probes/branchProtectionAppliesToAdmins" + "github.com/ossf/scorecard/v4/probes/branchesAreProtected" "github.com/ossf/scorecard/v4/probes/codeApproved" "github.com/ossf/scorecard/v4/probes/codeReviewOneReviewers" "github.com/ossf/scorecard/v4/probes/contributorsFromOrgOrCompany" + "github.com/ossf/scorecard/v4/probes/dismissesStaleReviews" "github.com/ossf/scorecard/v4/probes/freeOfUnverifiedBinaryArtifacts" "github.com/ossf/scorecard/v4/probes/fuzzed" "github.com/ossf/scorecard/v4/probes/hasDangerousWorkflowScriptInjection" @@ -39,6 +44,12 @@ import ( "github.com/ossf/scorecard/v4/probes/pinsDependencies" "github.com/ossf/scorecard/v4/probes/releasesAreSigned" "github.com/ossf/scorecard/v4/probes/releasesHaveProvenance" + "github.com/ossf/scorecard/v4/probes/requiresApproversForPullRequests" + "github.com/ossf/scorecard/v4/probes/requiresCodeOwnersReview" + "github.com/ossf/scorecard/v4/probes/requiresLastPushApproval" + "github.com/ossf/scorecard/v4/probes/requiresPRsToChangeCode" + "github.com/ossf/scorecard/v4/probes/requiresUpToDateBranches" + "github.com/ossf/scorecard/v4/probes/runsStatusChecksBeforeMerging" "github.com/ossf/scorecard/v4/probes/sastToolConfigured" "github.com/ossf/scorecard/v4/probes/sastToolRunsOnAllCommits" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsLinks" @@ -125,6 +136,19 @@ var ( releasesAreSigned.Run, releasesHaveProvenance.Run, } + BranchProtection = []ProbeImpl{ + blocksDeleteOnBranches.Run, + blocksForcePushOnBranches.Run, + branchesAreProtected.Run, + branchProtectionAppliesToAdmins.Run, + dismissesStaleReviews.Run, + requiresApproversForPullRequests.Run, + requiresCodeOwnersReview.Run, + requiresLastPushApproval.Run, + requiresUpToDateBranches.Run, + runsStatusChecksBeforeMerging.Run, + requiresPRsToChangeCode.Run, + } PinnedDependencies = []ProbeImpl{ pinsDependencies.Run, } diff --git a/probes/requiresApproversForPullRequests/impl.go b/probes/requiresApproversForPullRequests/impl.go index 14f114f2f5e8..05960a1189ec 100644 --- a/probes/requiresApproversForPullRequests/impl.go +++ b/probes/requiresApproversForPullRequests/impl.go @@ -45,10 +45,19 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { r := raw.BranchProtectionResults var findings []finding.Finding + if len(r.Branches) == 0 { + f, err := finding.NewWith(fs, Probe, "no branches found", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + for i := range r.Branches { branch := &r.Branches[i] + nilMsg := fmt.Sprintf("could not determine whether branch '%s' has required approving review count", *branch.Name) - trueMsg := fmt.Sprintf("required approving review count on branch '%s'", *branch.Name) falseMsg := fmt.Sprintf("branch '%s' does not require approvers", *branch.Name) p := branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount @@ -62,7 +71,8 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { case p == nil: f = f.WithMessage(nilMsg).WithOutcome(finding.OutcomeNotAvailable) case *p > 0: - f = f.WithMessage(trueMsg).WithOutcome(finding.OutcomePositive) + msg := fmt.Sprintf("required approving review count is %d on branch '%s'", *p, *branch.Name) + f = f.WithMessage(msg).WithOutcome(finding.OutcomePositive) f = f.WithValue(RequiredReviewersKey, strconv.Itoa(int(*p))) case *p == 0: f = f.WithMessage(falseMsg).WithOutcome(finding.OutcomeNegative) diff --git a/probes/requiresCodeOwnersReview/impl.go b/probes/requiresCodeOwnersReview/impl.go index b5ff4bc9143a..b1463d68d82e 100644 --- a/probes/requiresCodeOwnersReview/impl.go +++ b/probes/requiresCodeOwnersReview/impl.go @@ -40,8 +40,18 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { r := raw.BranchProtectionResults var findings []finding.Finding + if len(r.Branches) == 0 { + f, err := finding.NewWith(fs, Probe, "no branches found", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + for i := range r.Branches { branch := &r.Branches[i] + reqOwnerReviews := branch.BranchProtectionRule.RequiredPullRequestReviews.RequireCodeOwnerReviews var text string var outcome finding.Outcome diff --git a/probes/requiresCodeOwnersReview/impl_test.go b/probes/requiresCodeOwnersReview/impl_test.go index b58a00c716a2..7d0df964d013 100644 --- a/probes/requiresCodeOwnersReview/impl_test.go +++ b/probes/requiresCodeOwnersReview/impl_test.go @@ -104,7 +104,7 @@ func Test_Run(t *testing.T) { }, }, }, - CodeownersFiles: []string{"file"}, + CodeownersFiles: []string{"file1"}, }, }, outcomes: []finding.Outcome{ @@ -112,7 +112,7 @@ func Test_Run(t *testing.T) { }, }, { - name: "2 branches require code owner reviews with files = 2 negative outcomes", + name: "2 branches require code owner reviews with files = 2 positive outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ Branches: []clients.BranchRef{ @@ -133,11 +133,11 @@ func Test_Run(t *testing.T) { }, }, }, - CodeownersFiles: []string{}, + CodeownersFiles: []string{"file1", "file2"}, }, }, outcomes: []finding.Outcome{ - finding.OutcomeNegative, finding.OutcomeNegative, + finding.OutcomePositive, finding.OutcomePositive, }, }, { @@ -170,7 +170,7 @@ func Test_Run(t *testing.T) { }, }, { - name: "Requires code owner reviews on 1/2 branches - without files = 2 negative outcomes", + name: "Requires code owner reviews on 1/2 branches - without files = 1 positive and 1 negative", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ Branches: []clients.BranchRef{ @@ -191,11 +191,11 @@ func Test_Run(t *testing.T) { }, }, }, - CodeownersFiles: []string{}, + CodeownersFiles: []string{"file"}, }, }, outcomes: []finding.Outcome{ - finding.OutcomeNegative, finding.OutcomeNegative, + finding.OutcomePositive, finding.OutcomeNegative, }, }, { @@ -228,7 +228,7 @@ func Test_Run(t *testing.T) { }, }, { - name: "Requires code owner reviews on 1/2 branches - without files = 2 negative outcomes", + name: "Requires code owner reviews on 1/2 branches - without files = 2 negative", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ Branches: []clients.BranchRef{ diff --git a/probes/requiresLastPushApproval/impl.go b/probes/requiresLastPushApproval/impl.go index 43ef8e534426..e4ff33cfd2bf 100644 --- a/probes/requiresLastPushApproval/impl.go +++ b/probes/requiresLastPushApproval/impl.go @@ -41,6 +41,15 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { r := raw.BranchProtectionResults var findings []finding.Finding + if len(r.Branches) == 0 { + f, err := finding.NewWith(fs, Probe, "no branches found", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + for i := range r.Branches { branch := &r.Branches[i] diff --git a/probes/requiresPRsToChangeCode/def.yml b/probes/requiresPRsToChangeCode/def.yml new file mode 100644 index 000000000000..01bdeb8a741a --- /dev/null +++ b/probes/requiresPRsToChangeCode/def.yml @@ -0,0 +1,32 @@ +# Copyright 2023 OpenSSF 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. + +id: requiresPRsToChangeCode +short: Check that the project requires pull requests to change code. +motivation: > + Changing code without pull requests does not leave a traceable trail and can allow malicious actors to sneak in vulnerable code. +implementation: > + The probe checks which branches require pull requests to change the branches' code. The probe only considers default and release branches. +outcome: + - The probe returns one OutcomePositive for each branch that requires pull requests to change code, and one OutcomeNegative for branches that don't. +remediation: + effort: Low + text: + - Configure the project such that contributors must make PRs to change code. + - For GitHub-hosted projects, see [the Pull Requests documentation](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests). + - For Gitlab-hosted projects, see [the Merge Requests documentation](https://docs.gitlab.com/ee/user/project/merge_requests/). + markdown: + - Configure the project such that contributors must make PRs to change code. + - For GitHub-hosted projects, see [the Pull Requests documentation](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests). + - For Gitlab-hosted projects, see [the Merge Requests documentation](https://docs.gitlab.com/ee/user/project/merge_requests/). \ No newline at end of file diff --git a/probes/requiresPRsToChangeCode/impl.go b/probes/requiresPRsToChangeCode/impl.go new file mode 100644 index 000000000000..b895d8320b8e --- /dev/null +++ b/probes/requiresPRsToChangeCode/impl.go @@ -0,0 +1,86 @@ +// Copyright 2023 OpenSSF 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. + +//nolint:stylecheck +package requiresPRsToChangeCode + +import ( + "embed" + "errors" + "fmt" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/internal/utils/uerror" +) + +//go:embed *.yml +var fs embed.FS + +const ( + Probe = "requiresPRsToChangeCode" + BranchNameKey = "branchName" +) + +var errWrongValue = errors.New("wrong value, should not happen") + +func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { + if raw == nil { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + + r := raw.BranchProtectionResults + var findings []finding.Finding + + if len(r.Branches) == 0 { + f, err := finding.NewWith(fs, Probe, "no branches found", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + + for i := range r.Branches { + branch := &r.Branches[i] + + nilMsg := fmt.Sprintf("could not determine whether branch '%s' requires PRs to change code", *branch.Name) + trueMsg := fmt.Sprintf("PRs are required in order to make changes on branch '%s'", *branch.Name) + falseMsg := fmt.Sprintf("PRs are not required to make changes on branch '%s'; ", *branch.Name) + + "or we don't have data to detect it." + + "If you think it might be the latter, make sure to run Scorecard with a PAT or use Repo " + + "Rules (that are always public) instead of Branch Protection settings" + + p := branch.BranchProtectionRule.RequiredPullRequestReviews.Required + + f, err := finding.NewWith(fs, Probe, "", nil, finding.OutcomeNotAvailable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + + switch { + case p == nil: + f = f.WithMessage(nilMsg).WithOutcome(finding.OutcomeNotAvailable) + case *p: + f = f.WithMessage(trueMsg).WithOutcome(finding.OutcomePositive) + case !*p: + f = f.WithMessage(falseMsg).WithOutcome(finding.OutcomeNegative) + default: + return nil, Probe, fmt.Errorf("create finding: %w", errWrongValue) + } + f = f.WithValue(BranchNameKey, *branch.Name) + findings = append(findings, *f) + } + return findings, Probe, nil +} diff --git a/probes/requiresPRsToChangeCode/impl_test.go b/probes/requiresPRsToChangeCode/impl_test.go new file mode 100644 index 000000000000..9bf181d3a7db --- /dev/null +++ b/probes/requiresPRsToChangeCode/impl_test.go @@ -0,0 +1,202 @@ +// Copyright 2023 OpenSSF 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. + +//nolint:stylecheck +package requiresPRsToChangeCode + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/clients" + "github.com/ossf/scorecard/v4/finding" +) + +func Test_Run(t *testing.T) { + t.Parallel() + trueVal := true + falseVal := false + branchVal1 := "branch-name1" + branchVal2 := "branch-name1" + //nolint:govet + tests := []struct { + name string + raw *checker.RawResults + outcomes []finding.Outcome + err error + }{ + { + name: "1 branch requires PRs to change code", + raw: &checker.RawResults{ + BranchProtectionResults: checker.BranchProtectionsData{ + Branches: []clients.BranchRef{ + { + Name: &branchVal1, + BranchProtectionRule: clients.BranchProtectionRule{ + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + Required: &trueVal, + }, + }, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + }, + { + name: "2 branches require PRs to change code = 2 positive outcomes", + raw: &checker.RawResults{ + BranchProtectionResults: checker.BranchProtectionsData{ + Branches: []clients.BranchRef{ + { + Name: &branchVal1, + BranchProtectionRule: clients.BranchProtectionRule{ + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + Required: &trueVal, + }, + }, + }, + { + Name: &branchVal2, + BranchProtectionRule: clients.BranchProtectionRule{ + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + Required: &trueVal, + }, + }, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, finding.OutcomePositive, + }, + }, + { + name: "1 branches require PRs to change code and 1 branch doesn't = 1 positive 1 negative", + raw: &checker.RawResults{ + BranchProtectionResults: checker.BranchProtectionsData{ + Branches: []clients.BranchRef{ + { + Name: &branchVal1, + BranchProtectionRule: clients.BranchProtectionRule{ + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + Required: &trueVal, + }, + }, + }, + { + Name: &branchVal2, + BranchProtectionRule: clients.BranchProtectionRule{ + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + Required: &falseVal, + }, + }, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, finding.OutcomeNegative, + }, + }, + { + name: "Requires PRs to change code on 1/2 branches = 1 negative and 1 positive", + raw: &checker.RawResults{ + BranchProtectionResults: checker.BranchProtectionsData{ + Branches: []clients.BranchRef{ + { + Name: &branchVal1, + BranchProtectionRule: clients.BranchProtectionRule{ + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + Required: &falseVal, + }, + }, + }, + { + Name: &branchVal2, + BranchProtectionRule: clients.BranchProtectionRule{ + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + Required: &trueVal, + }, + }, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, finding.OutcomePositive, + }, + }, + { + name: "1 branch does not require PRs to change code and 1 lacks data = 1 negative and 1 unavailable", + raw: &checker.RawResults{ + BranchProtectionResults: checker.BranchProtectionsData{ + Branches: []clients.BranchRef{ + { + Name: &branchVal1, + BranchProtectionRule: clients.BranchProtectionRule{ + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + Required: &falseVal, + }, + }, + }, + { + Name: &branchVal2, + BranchProtectionRule: clients.BranchProtectionRule{ + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + Required: nil, + }, + }, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, finding.OutcomeNotAvailable, + }, + }, + } + 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() + + findings, s, err := Run(tt.raw) + if !cmp.Equal(tt.err, err, cmpopts.EquateErrors()) { + t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(tt.err, err, cmpopts.EquateErrors())) + } + if err != nil { + return + } + if diff := cmp.Diff(Probe, s); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(len(tt.outcomes), len(findings)); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + for i := range tt.outcomes { + outcome := &tt.outcomes[i] + f := &findings[i] + if diff := cmp.Diff(*outcome, f.Outcome); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + } + }) + } +} diff --git a/probes/requiresUpToDateBranches/impl.go b/probes/requiresUpToDateBranches/impl.go index fc083cf56568..ed9331cb71b4 100644 --- a/probes/requiresUpToDateBranches/impl.go +++ b/probes/requiresUpToDateBranches/impl.go @@ -41,6 +41,15 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { r := raw.BranchProtectionResults var findings []finding.Finding + if len(r.Branches) == 0 { + f, err := finding.NewWith(fs, Probe, "no branches found", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + for i := range r.Branches { branch := &r.Branches[i] diff --git a/probes/runsStatusChecksBeforeMerging/impl.go b/probes/runsStatusChecksBeforeMerging/impl.go index ba9c6bc691c2..6d5720f6bea3 100644 --- a/probes/runsStatusChecksBeforeMerging/impl.go +++ b/probes/runsStatusChecksBeforeMerging/impl.go @@ -40,28 +40,38 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { r := raw.BranchProtectionResults var findings []finding.Finding + if len(r.Branches) == 0 { + f, err := finding.NewWith(fs, Probe, "no branches found", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + for i := range r.Branches { branch := &r.Branches[i] + var f *finding.Finding + var err error + switch { case len(branch.BranchProtectionRule.CheckRules.Contexts) > 0: - f, err := finding.NewWith(fs, Probe, + f, err = finding.NewWith(fs, Probe, fmt.Sprintf("status check found to merge onto on branch '%s'", *branch.Name), nil, finding.OutcomePositive) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValue(BranchNameKey, *branch.Name) - findings = append(findings, *f) default: - f, err := finding.NewWith(fs, Probe, + f, err = finding.NewWith(fs, Probe, fmt.Sprintf("no status checks found to merge onto branch '%s'", *branch.Name), nil, finding.OutcomeNegative) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValue(BranchNameKey, *branch.Name) - findings = append(findings, *f) } + f = f.WithValue(BranchNameKey, *branch.Name) + findings = append(findings, *f) } return findings, Probe, nil } diff --git a/tools/go.mod b/tools/go.mod index ec24e450fbae..d3e8b27bf26d 100644 --- a/tools/go.mod +++ b/tools/go.mod @@ -8,8 +8,8 @@ require ( github.com/google/addlicense v1.1.1 github.com/google/ko v0.15.2 github.com/goreleaser/goreleaser v1.24.0 - github.com/onsi/ginkgo/v2 v2.15.0 - google.golang.org/protobuf v1.32.0 + github.com/onsi/ginkgo/v2 v2.16.0 + google.golang.org/protobuf v1.33.0 ) require ( @@ -392,7 +392,7 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20240116215550-a9fa1716bcac // indirect google.golang.org/grpc v1.61.0 // indirect gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect - gopkg.in/go-jose/go-jose.v2 v2.6.1 // indirect + gopkg.in/go-jose/go-jose.v2 v2.6.3 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/mail.v2 v2.3.1 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect diff --git a/tools/go.sum b/tools/go.sum index cfca3317ea27..582fe15cc54a 100644 --- a/tools/go.sum +++ b/tools/go.sum @@ -845,8 +845,8 @@ github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108 github.com/onsi/ginkgo v1.16.4 h1:29JGrr5oVBm5ulCWet69zQkzWipVXIol6ygQUe/EzNc= github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0= github.com/onsi/ginkgo/v2 v2.1.3/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c= -github.com/onsi/ginkgo/v2 v2.15.0 h1:79HwNRBAZHOEwrczrgSOPy+eFTTlIGELKy5as+ClttY= -github.com/onsi/ginkgo/v2 v2.15.0/go.mod h1:HlxMHtYF57y6Dpf+mc5529KKmSq9h2FpCF+/ZkwUxKM= +github.com/onsi/ginkgo/v2 v2.16.0 h1:7q1w9frJDzninhXxjZd+Y/x54XNjG/UlRLIYPZafsPM= +github.com/onsi/ginkgo/v2 v2.16.0/go.mod h1:llBI3WDLL9Z6taip6f33H76YcWtJv+7R3HigUjbIBOs= github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= @@ -1536,8 +1536,8 @@ google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGj google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= -google.golang.org/protobuf v1.32.0 h1:pPC6BG5ex8PDFnkbrGU3EixyhKcQ2aDuBS36lqK/C7I= -google.golang.org/protobuf v1.32.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= +google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI= +google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc h1:2gGKlE2+asNV9m7xrywl36YYNnBG5ZQ0r/BOOxqPpmk= gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc/go.mod h1:m7x9LTH6d71AHyAX77c9yqWCCa3UKHcVEj9y7hAtKDk= @@ -1549,8 +1549,8 @@ gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntN gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= -gopkg.in/go-jose/go-jose.v2 v2.6.1 h1:qEzJlIDmG9q5VO0M/o8tGS65QMHMS1w01TQJB1VPJ4U= -gopkg.in/go-jose/go-jose.v2 v2.6.1/go.mod h1:zzZDPkNNw/c9IE7Z9jr11mBZQhKQTMzoEEIoEdZlFBI= +gopkg.in/go-jose/go-jose.v2 v2.6.3 h1:nt80fvSDlhKWQgSWyHyy5CfmlQr+asih51R8PTWNKKs= +gopkg.in/go-jose/go-jose.v2 v2.6.3/go.mod h1:zzZDPkNNw/c9IE7Z9jr11mBZQhKQTMzoEEIoEdZlFBI= gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA= gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/mail.v2 v2.3.1 h1:WYFn/oANrAGP2C0dcV6/pbkPzv8yGzqTjPmTeO7qoXk=