diff --git a/checks/contributors.go b/checks/contributors.go index c8e000a8c5d..15db76f2d9d 100644 --- a/checks/contributors.go +++ b/checks/contributors.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" ) // CheckContributors is the registered name for Contributors. @@ -34,17 +36,23 @@ func init() { // Contributors run Contributors check. func Contributors(c *checker.CheckRequest) checker.CheckResult { - rawData, err := raw.Contributors(c.RepoClient) + rawData, err := raw.Contributors(c) if err != nil { e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) return checker.CreateRuntimeErrorResult(CheckContributors, e) } - // Return raw results. - if c.RawResults != nil { - c.RawResults.ContributorsResults = rawData + // Set the raw results. + pRawResults := getRawResults(c) + pRawResults.ContributorsResults = rawData + + // Evaluate the probes. + findings, err := zrunner.Run(pRawResults, probes.Contributors) + if err != nil { + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return checker.CreateRuntimeErrorResult(CheckContributors, e) } // Return the score evaluation. - return evaluation.Contributors(CheckContributors, c.Dlogger, &rawData) + return evaluation.Contributors(CheckContributors, findings, c.Dlogger) } diff --git a/checks/evaluation/contributors.go b/checks/evaluation/contributors.go index d419e6a1ea4..d58653e6278 100644 --- a/checks/evaluation/contributors.go +++ b/checks/evaluation/contributors.go @@ -16,60 +16,67 @@ package evaluation import ( "fmt" - "sort" "strings" "github.com/ossf/scorecard/v4/checker" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/contributorsFromOrgOrCompany" ) const ( - minContributionsPerUser = 5 numberCompaniesForTopScore = 3 ) // Contributors applies the score policy for the Contributors check. -func Contributors(name string, dl checker.DetailLogger, - r *checker.ContributorsData, +func Contributors(name string, + findings []finding.Finding, + dl checker.DetailLogger, ) checker.CheckResult { - if r == nil { - e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data") - return checker.CreateRuntimeErrorResult(name, e) + expectedProbes := []string{ + contributorsFromOrgOrCompany.Probe, } - entities := make(map[string]bool) - - for _, user := range r.Users { - if user.NumContributions < minContributionsPerUser { - continue - } + if !finding.UniqueProbesEqual(findings, expectedProbes) { + e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") + return checker.CreateRuntimeErrorResult(name, e) + } - for _, org := range user.Organizations { - entities[org.Login] = true - } + numberOfPositives := getNumberOfPositives(findings) + reason := fmt.Sprintf("project has %d contributing companies or organizations", numberOfPositives) - for _, comp := range user.Companies { - entities[comp] = true - } + if numberOfPositives > 0 { + logFindings(findings, dl) } - - names := []string{} - for c := range entities { - names = append(names, c) + if numberOfPositives > numberCompaniesForTopScore { + return checker.CreateMaxScoreResult(name, reason) } - sort.Strings(names) + return checker.CreateProportionalScoreResult(name, reason, numberOfPositives, numberCompaniesForTopScore) +} - if len(names) > 0 { - dl.Info(&checker.LogMessage{ - Text: fmt.Sprintf("contributors work for %v", strings.Join(names, ",")), - }) - } else { - dl.Warn(&checker.LogMessage{ - Text: "no contributors have an org or company", - }) +func getNumberOfPositives(findings []finding.Finding) int { + var numberOfPositives int + for i := range findings { + f := &findings[i] + if f.Outcome == finding.OutcomePositive { + if f.Probe == contributorsFromOrgOrCompany.Probe { + numberOfPositives++ + } + } } + return numberOfPositives +} - reason := fmt.Sprintf("%d different organizations found", len(entities)) - return checker.CreateProportionalScoreResult(name, reason, len(entities), numberCompaniesForTopScore) +func logFindings(findings []finding.Finding, dl checker.DetailLogger) { + var sb strings.Builder + for i := range findings { + f := &findings[i] + if f.Outcome == finding.OutcomePositive { + sb.WriteString(fmt.Sprintf("%s, ", f.Message)) + } + } + dl.Info(&checker.LogMessage{ + Text: sb.String(), + }) } diff --git a/checks/evaluation/contributors_test.go b/checks/evaluation/contributors_test.go index 87b2c622df7..d5962e1328e 100644 --- a/checks/evaluation/contributors_test.go +++ b/checks/evaluation/contributors_test.go @@ -16,138 +16,75 @@ package evaluation 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/utests" + "github.com/ossf/scorecard/v4/finding" + scut "github.com/ossf/scorecard/v4/utests" ) func TestContributors(t *testing.T) { t.Parallel() - testCases := []struct { + tests := []struct { name string - raw *checker.ContributorsData - expected checker.CheckResult + findings []finding.Finding + result scut.TestReturn }{ { - name: "No data", - raw: nil, - expected: checker.CheckResult{ - Version: 2, - Score: -1, - Reason: "internal error: empty raw data", - }, - }, - { - name: "No contributors", - raw: &checker.ContributorsData{ - Users: []clients.User{}, + name: "Only has two positive outcomes", + findings: []finding.Finding{ + { + Probe: "contributorsFromOrgOrCompany", + Outcome: finding.OutcomePositive, + }, + { + Probe: "contributorsFromOrgOrCompany", + Outcome: finding.OutcomePositive, + }, }, - expected: checker.CheckResult{ - Version: 2, - Score: 0, - Reason: "0 different organizations found -- score normalized to 0", + result: scut.TestReturn{ + Score: 6, + NumberOfInfo: 1, }, - }, - { - name: "Contributors with orgs and number of contributions is greater than 5 with companies", - raw: &checker.ContributorsData{ - Users: []clients.User{ - { - NumContributions: 10, - Organizations: []clients.User{ - { - Login: "org1", - }, - }, - Companies: []string{"company1"}, - }, - { - NumContributions: 10, - Organizations: []clients.User{ - { - Login: "org2", - }, - }, - }, - { - NumContributions: 10, - Organizations: []clients.User{ - { - Login: "org3", - }, - }, - }, - { - NumContributions: 1, - Organizations: []clients.User{ - { - Login: "org2", - }, - }, - }, + }, { + name: "No contributors", + findings: []finding.Finding{ + { + Probe: "contributorsFromOrgOrCompany", + Outcome: finding.OutcomeNegative, }, }, - expected: checker.CheckResult{ - Version: 2, - Score: 10, - Reason: "4 different organizations found -- score normalized to 10", + result: scut.TestReturn{ + Score: 0, }, - }, - { - name: "Contributors with orgs and number of contributions is greater than 5 without companies", - raw: &checker.ContributorsData{ - Users: []clients.User{ - { - NumContributions: 10, - Organizations: []clients.User{ - { - Login: "org1", - }, - }, - }, - { - NumContributions: 10, - Organizations: []clients.User{ - { - Login: "org2", - }, - }, - }, - { - NumContributions: 10, - Organizations: []clients.User{ - { - Login: "org3", - }, - }, - }, - { - NumContributions: 1, - Organizations: []clients.User{ - { - Login: "org10", - }, - }, - }, + }, { + name: "Has three positive outcomes", + findings: []finding.Finding{ + { + Probe: "contributorsFromOrgOrCompany", + Outcome: finding.OutcomePositive, + }, + { + Probe: "contributorsFromOrgOrCompany", + Outcome: finding.OutcomePositive, + }, + { + Probe: "contributorsFromOrgOrCompany", + Outcome: finding.OutcomePositive, }, }, - expected: checker.CheckResult{ - Version: 2, - Score: 10, - Reason: "3 different organizations found -- score normalized to 10", + result: scut.TestReturn{ + Score: checker.MaxResultScore, + NumberOfInfo: 1, }, }, } - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { t.Parallel() - result := Contributors("", &utests.TestDetailLogger{}, tc.raw) - if !cmp.Equal(result, tc.expected, cmpopts.IgnoreFields(checker.CheckResult{}, "Error")) { //nolint:govet - t.Errorf("expected %v, got %v", tc.expected, cmp.Diff(tc.expected, result, cmpopts.IgnoreFields(checker.CheckResult{}, "Error"))) //nolint:lll + dl := scut.TestDetailLogger{} + got := Contributors(tt.name, tt.findings, &dl) + if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) { + t.Error(tt.name) } }) } diff --git a/checks/evaluation/finding.go b/checks/evaluation/finding.go index 11d1e6f9fc4..2861318e984 100644 --- a/checks/evaluation/finding.go +++ b/checks/evaluation/finding.go @@ -34,9 +34,10 @@ func negativeFindings(findings []finding.Finding) []finding.Finding { var ff []finding.Finding for i := range findings { f := &findings[i] - if f.Outcome == finding.OutcomeNegative { - ff = append(ff, *f) + if f.Outcome == finding.OutcomePositive { + continue } + ff = append(ff, *f) } return ff } diff --git a/checks/raw/contributors.go b/checks/raw/contributors.go index e8a9d56b327..309d21b7d5f 100644 --- a/checks/raw/contributors.go +++ b/checks/raw/contributors.go @@ -23,7 +23,8 @@ import ( ) // Contributors retrieves the raw data for the Contributors check. -func Contributors(c clients.RepoClient) (checker.ContributorsData, error) { +func Contributors(cr *checker.CheckRequest) (checker.ContributorsData, error) { + c := cr.RepoClient var users []clients.User contribs, err := c.ListContributors() diff --git a/checks/raw/contributors_test.go b/checks/raw/contributors_test.go index 6d7814246fc..7b2da17c915 100644 --- a/checks/raw/contributors_test.go +++ b/checks/raw/contributors_test.go @@ -19,6 +19,7 @@ import ( "github.com/golang/mock/gomock" "github.com/google/go-cmp/cmp" + "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/clients" mockrepo "github.com/ossf/scorecard/v4/clients/mockclients" ) @@ -130,8 +131,10 @@ func TestContributors(t *testing.T) { } mockRepoClient.EXPECT().ListContributors().Return(contributors, nil) - - data, err := Contributors(mockRepoClient) + req := &checker.CheckRequest{ + RepoClient: mockRepoClient, + } + data, err := Contributors(req) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/probes/contributorsFromOrgOrCompany/def.yml b/probes/contributorsFromOrgOrCompany/def.yml new file mode 100644 index 00000000000..630fcc46c9f --- /dev/null +++ b/probes/contributorsFromOrgOrCompany/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: contributorsFromOrgOrCompany +short: Checks whether a project has a contributions from users associated with a company or organization. +motivation: > + This probe tries to determine if the project has recent contributors from multiple organizations (e.g., companies). + + Some projects cannot meet this requirement, such as small projects with only one active participant, or projects with a narrow scope that cannot attract the interest of multiple organizations. See Code Reviews for more information about evaluating projects with a small number of participants. + +implementation: > + The probe looks at the Company field on the user profile for authors of recent commits. To receive the highest score, the project must have had contributors from at least 3 different companies in the last 30 commits. +outcome: + - If the project has no contributing organizations or companies, the probe returns 1 OutcomeNegative + - If the project has contributing organizations or companies, the probe returns 1 Outcome per org or company. + - If the probe fails to process the raw results, it returns nil instead of the findings slice as well as an error. +remediation: + effort: High + text: + - Encourage community-driven contributions to your project. + - Ask contributors to join their respective organizations, if they have not already. Otherwise, there is no remediation for this probe; it simply provides insight into how many organizations have contributed so that you can make a trust-based decision based on that information. \ No newline at end of file diff --git a/probes/contributorsFromOrgOrCompany/impl.go b/probes/contributorsFromOrgOrCompany/impl.go new file mode 100644 index 00000000000..25cab7b652f --- /dev/null +++ b/probes/contributorsFromOrgOrCompany/impl.go @@ -0,0 +1,97 @@ +// 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 contributorsFromOrgOrCompany + +import ( + "embed" + "fmt" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/internal/utils/uerror" +) + +const ( + minContributionsPerUser = 5 +) + +//go:embed *.yml +var fs embed.FS + +const Probe = "contributorsFromOrgOrCompany" + +func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { + if raw == nil { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + var findings []finding.Finding + + users := raw.ContributorsResults.Users + + if len(users) == 0 { + f, err := finding.NewWith(fs, Probe, + "Project does not have contributors.", nil, + finding.OutcomeNegative) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + + findings = append(findings, *f) + return findings, Probe, nil + } + + entities := make(map[string]bool) + + for _, user := range users { + if user.NumContributions < minContributionsPerUser { + continue + } + + for _, org := range user.Organizations { + entities[org.Login] = true + } + + for _, comp := range user.Companies { + entities[comp] = true + } + } + + if len(entities) == 0 { + f, err := finding.NewWith(fs, Probe, + "No companies/organizations have contributed to the project.", nil, + finding.OutcomeNegative) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + + findings = append(findings, *f) + return findings, Probe, nil + } + + // Convert entities map to findings slice + for e := range entities { + f, err := finding.NewWith(fs, Probe, + fmt.Sprintf("%s contributor org/company found", e), nil, + finding.OutcomePositive) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + + findings = append(findings, *f) + } + + return findings, Probe, nil +} diff --git a/probes/contributorsFromOrgOrCompany/impl_test.go b/probes/contributorsFromOrgOrCompany/impl_test.go new file mode 100644 index 00000000000..006436b4b0f --- /dev/null +++ b/probes/contributorsFromOrgOrCompany/impl_test.go @@ -0,0 +1,171 @@ +// 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 contributorsFromOrgOrCompany + +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" +) + +type User struct { + Login string + Companies []string + Organizations []User + NumContributions int + ID int64 + IsBot bool +} + +func Test_Run(t *testing.T) { + t.Parallel() + // nolint:govet + tests := []struct { + name string + raw *checker.RawResults + outcomes []finding.Outcome + err error + }{ + { + name: "Test that both User.Companies and User.Organizations are included", + raw: &checker.RawResults{ + ContributorsResults: checker.ContributorsData{ + Users: []clients.User{ + { + Companies: []string{"comp1", "comp2"}, + Organizations: []clients.User{ + { + Login: "Login", + Companies: []string{"comp3", "comp4"}, // These should not be included + NumContributions: 10, + }, + }, + NumContributions: 10, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + }, + }, { + name: "Test multiple users", + raw: &checker.RawResults{ + ContributorsResults: checker.ContributorsData{ + Users: []clients.User{ + { + Companies: []string{"comp1", "comp2"}, + Organizations: []clients.User{ + { + Login: "Login1", + NumContributions: 10, + }, + }, + NumContributions: 10, + }, + { + Companies: []string{"comp3", "comp4"}, + Organizations: []clients.User{ + { + Login: "Login2", + NumContributions: 10, + }, + }, + NumContributions: 10, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + }, + }, { + name: "Test multiple users where one user has insufficient contributions.", + raw: &checker.RawResults{ + ContributorsResults: checker.ContributorsData{ + Users: []clients.User{ + { + Companies: []string{"comp1", "comp2"}, + Organizations: []clients.User{ + { + Login: "Login1", + NumContributions: 10, + }, + }, + NumContributions: 10, + }, + { + Companies: []string{"comp3", "comp4"}, + Organizations: []clients.User{ + { + Login: "Login2", + NumContributions: 10, + }, + }, + NumContributions: 2, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + }, + }, + } + 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() + + // TODO(#https://github.com/ossf/scorecard/issues/3472) Use common validation function. + 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 { + t.Error(err) + } + + 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/entries.go b/probes/entries.go index 17caef3d374..3b4275c49c5 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -17,6 +17,7 @@ package probes import ( "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/contributorsFromOrgOrCompany" "github.com/ossf/scorecard/v4/probes/fuzzedWithCLibFuzzer" "github.com/ossf/scorecard/v4/probes/fuzzedWithClusterFuzzLite" "github.com/ossf/scorecard/v4/probes/fuzzedWithCppLibFuzzer" @@ -89,6 +90,9 @@ var ( hasFSFOrOSIApprovedLicense.Run, hasLicenseFileAtTopDir.Run, } + Contributors = []ProbeImpl{ + contributorsFromOrgOrCompany.Run, + } ) //nolint:gochecknoinits @@ -98,6 +102,7 @@ func init() { SecurityPolicy, Fuzzing, License, + Contributors, }) }