diff --git a/checks/evaluation/license.go b/checks/evaluation/license.go index 0401e5ec..7eff4c9d 100644 --- a/checks/evaluation/license.go +++ b/checks/evaluation/license.go @@ -20,7 +20,6 @@ import ( "github.com/ossf/scorecard/v4/finding" "github.com/ossf/scorecard/v4/probes/hasFSFOrOSIApprovedLicense" "github.com/ossf/scorecard/v4/probes/hasLicenseFile" - "github.com/ossf/scorecard/v4/probes/hasLicenseFileAtTopDir" ) // License applies the score policy for the License check. @@ -32,7 +31,6 @@ func License(name string, expectedProbes := []string{ hasLicenseFile.Probe, hasFSFOrOSIApprovedLicense.Probe, - hasLicenseFileAtTopDir.Probe, } if !finding.UniqueProbesEqual(findings, expectedProbes) { @@ -45,58 +43,20 @@ func License(name string, m := make(map[string]bool) for i := range findings { f := &findings[i] - switch f.Outcome { - case finding.OutcomeNotApplicable: - dl.Info(&checker.LogMessage{ - Type: finding.FileTypeSource, - Offset: 1, - Text: f.Message, - }) - case finding.OutcomePositive: + if f.Outcome == finding.OutcomePositive { switch f.Probe { case hasFSFOrOSIApprovedLicense.Probe: - dl.Info(&checker.LogMessage{ - Type: finding.FileTypeSource, - Offset: 1, - Path: f.Message, - Text: "FSF or OSI recognized license", - }) score += scoreProbeOnce(f.Probe, m, 1) - case hasLicenseFileAtTopDir.Probe: - dl.Info(&checker.LogMessage{ - Type: finding.FileTypeSource, - Offset: 1, - Path: f.Message, - Text: "License file found in expected location", - }) - score += scoreProbeOnce(f.Probe, m, 3) case hasLicenseFile.Probe: - score += scoreProbeOnce(f.Probe, m, 6) + score += scoreProbeOnce(f.Probe, m, 9) default: e := sce.WithMessage(sce.ErrScorecardInternal, "unknown probe results") return checker.CreateRuntimeErrorResult(name, e) } - case finding.OutcomeNegative: - switch f.Probe { - case hasLicenseFileAtTopDir.Probe: - dl.Warn(&checker.LogMessage{ - Type: finding.FileTypeSource, - Offset: 1, - Path: f.Message, - Text: "License file found in unexpected location", - }) - case hasFSFOrOSIApprovedLicense.Probe: - dl.Warn(&checker.LogMessage{ - Type: finding.FileTypeSource, - Offset: 1, - Path: "", - Text: f.Message, - }) - } - default: - continue // for linting } } + checker.LogFindings(findings, dl) + _, defined := m[hasLicenseFile.Probe] if !defined { if score > 0 { diff --git a/checks/evaluation/license_test.go b/checks/evaluation/license_test.go index 66857014..8b2b4e24 100644 --- a/checks/evaluation/license_test.go +++ b/checks/evaluation/license_test.go @@ -40,10 +40,6 @@ func TestLicense(t *testing.T) { Probe: "hasFSFOrOSIApprovedLicense", Outcome: finding.OutcomePositive, }, - { - Probe: "hasLicenseFileAtTopDir", - Outcome: finding.OutcomePositive, - }, }, result: scut.TestReturn{ Score: checker.MaxResultScore, @@ -60,17 +56,13 @@ func TestLicense(t *testing.T) { Probe: "hasFSFOrOSIApprovedLicense", Outcome: finding.OutcomeNegative, }, - { - Probe: "hasLicenseFileAtTopDir", - Outcome: finding.OutcomeNegative, - }, }, result: scut.TestReturn{ Score: checker.MinResultScore, NumberOfWarn: 2, }, }, { - name: "Has license file but not a top level or in OSI/FSF format", + name: "Has license file but not OSI/FSF approved", findings: []finding.Finding{ { Probe: "hasLicenseFile", @@ -80,14 +72,11 @@ func TestLicense(t *testing.T) { Probe: "hasFSFOrOSIApprovedLicense", Outcome: finding.OutcomeNegative, }, - { - Probe: "hasLicenseFileAtTopDir", - Outcome: finding.OutcomeNegative, - }, }, result: scut.TestReturn{ - Score: 6, - NumberOfWarn: 2, + Score: 9, + NumberOfWarn: 1, + NumberOfInfo: 1, }, }, { name: "Findings missing a probe = Error", @@ -96,10 +85,6 @@ func TestLicense(t *testing.T) { Probe: "hasLicenseFile", Outcome: finding.OutcomePositive, }, - { - Probe: "hasFSFOrOSIApprovedLicense", - Outcome: finding.OutcomeNegative, - }, }, result: scut.TestReturn{ Score: -1, @@ -116,37 +101,12 @@ func TestLicense(t *testing.T) { Probe: "hasFSFOrOSIApprovedLicense", Outcome: finding.OutcomeNegative, }, - { - Probe: "hasLicenseFileAtTopDir", - Outcome: finding.OutcomePositive, - }, }, result: scut.TestReturn{ Score: 9, NumberOfInfo: 1, NumberOfWarn: 1, }, - }, { - name: "Has an OSI/FSF approved license but not at top level dir", - findings: []finding.Finding{ - { - Probe: "hasLicenseFile", - Outcome: finding.OutcomePositive, - }, - { - Probe: "hasFSFOrOSIApprovedLicense", - Outcome: finding.OutcomePositive, - }, - { - Probe: "hasLicenseFileAtTopDir", - Outcome: finding.OutcomeNegative, - }, - }, - result: scut.TestReturn{ - Score: 7, - NumberOfInfo: 1, - NumberOfWarn: 1, - }, }, } for _, tt := range tests { diff --git a/checks/license_test.go b/checks/license_test.go index 7726d1c7..bc828e45 100644 --- a/checks/license_test.go +++ b/checks/license_test.go @@ -52,10 +52,11 @@ func TestLicenseFileSubdirectory(t *testing.T) { name: "Without LICENSE", inputFolder: "testdata/licensedir/withoutlicense", expected: scut.TestReturn{ - Error: nil, - Score: checker.MinResultScore, - NumberOfWarn: 0, - NumberOfInfo: 2, + Error: nil, + Score: checker.MinResultScore, + NumberOfWarn: 1, + NumberOfInfo: 0, + NumberOfDebug: 1, }, err: nil, }, diff --git a/probes/entries.go b/probes/entries.go index d18db1cb..6a0365c9 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -32,7 +32,6 @@ import ( "github.com/ossf/scorecard/v4/probes/hasDangerousWorkflowUntrustedCheckout" "github.com/ossf/scorecard/v4/probes/hasFSFOrOSIApprovedLicense" "github.com/ossf/scorecard/v4/probes/hasLicenseFile" - "github.com/ossf/scorecard/v4/probes/hasLicenseFileAtTopDir" "github.com/ossf/scorecard/v4/probes/hasNoGitHubWorkflowPermissionUnknown" "github.com/ossf/scorecard/v4/probes/hasOSVVulnerabilities" "github.com/ossf/scorecard/v4/probes/hasOpenSSFBadge" @@ -95,7 +94,6 @@ var ( License = []ProbeImpl{ hasLicenseFile.Run, hasFSFOrOSIApprovedLicense.Run, - hasLicenseFileAtTopDir.Run, } Contributors = []ProbeImpl{ contributorsFromOrgOrCompany.Run, diff --git a/probes/hasFSFOrOSIApprovedLicense/impl.go b/probes/hasFSFOrOSIApprovedLicense/impl.go index 828d49e3..1e3c6877 100644 --- a/probes/hasFSFOrOSIApprovedLicense/impl.go +++ b/probes/hasFSFOrOSIApprovedLicense/impl.go @@ -39,7 +39,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) } - if raw.LicenseResults.LicenseFiles == nil || len(raw.LicenseResults.LicenseFiles) == 0 { + if len(raw.LicenseResults.LicenseFiles) == 0 { f, err := finding.NewWith(fs, Probe, "project does not have a license file", nil, finding.OutcomeNotApplicable) @@ -50,18 +50,16 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } for _, licenseFile := range raw.LicenseResults.LicenseFiles { - if licenseFile.LicenseInformation.Approved { - // Store the file path in the msg - msg := licenseFile.File.Path - - f, err := finding.NewWith(fs, Probe, - msg, nil, - finding.OutcomePositive) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - return []finding.Finding{*f}, Probe, nil + if !licenseFile.LicenseInformation.Approved { + continue } + licenseFile := licenseFile + loc := licenseFile.File.Location() + f, err := finding.NewPositive(fs, Probe, "FSF or OSI recognized license: "+licenseFile.LicenseInformation.Name, loc) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + return []finding.Finding{*f}, Probe, nil } f, err := finding.NewWith(fs, Probe, diff --git a/probes/hasLicenseFile/impl.go b/probes/hasLicenseFile/impl.go index 4757ea04..6e8a5cfc 100644 --- a/probes/hasLicenseFile/impl.go +++ b/probes/hasLicenseFile/impl.go @@ -40,17 +40,11 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } var findings []finding.Finding - var outcome finding.Outcome - var msg string licenseFiles := raw.LicenseResults.LicenseFiles if len(licenseFiles) == 0 { - outcome = finding.OutcomeNegative - msg = "project does not have a license file" - f, err := finding.NewWith(fs, Probe, - msg, nil, - outcome) + f, err := finding.NewNegative(fs, Probe, "project does not have a license file", nil) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } @@ -59,18 +53,8 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } else { for _, licenseFile := range licenseFiles { licenseFile := licenseFile - loc := &finding.Location{ - Type: licenseFile.File.Type, - Path: licenseFile.File.Path, - LineStart: &licenseFile.File.Offset, - LineEnd: &licenseFile.File.EndOffset, - Snippet: &licenseFile.File.Snippet, - } - msg = "project has a license file" - outcome = finding.OutcomePositive - f, err := finding.NewWith(fs, Probe, - msg, loc, - outcome) + loc := licenseFile.File.Location() + f, err := finding.NewPositive(fs, Probe, "project has a license file", loc) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } diff --git a/probes/hasLicenseFileAtTopDir/def.yml b/probes/hasLicenseFileAtTopDir/def.yml deleted file mode 100644 index 282bcdd2..00000000 --- a/probes/hasLicenseFileAtTopDir/def.yml +++ /dev/null @@ -1,34 +0,0 @@ -# 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: hasLicenseFileAtTopDir -short: Check that the project has a license file -motivation: > - A license can give users information about how the source code may or may not be used. The lack of a license will impede any kind of security review or audit and creates a legal risk for potential users. -implementation: > - This check will detect files in the top-level directory with any combination of the following names and extensions: LICENSE, LICENCE, COPYING, COPYRIGHT and having common extensions such as .html, .txt, or .md. It will also detect these files in a directory named LICENSES in the top directory. (Files in a LICENSES directory are typically named as their SPDX license identifier followed by an appropriate file extension, as described in the [REUSE Specification](https://reuse.software/spec/).) -outcome: - - If the projects license file is found at the top level, the probe returns a single OutcomePositive (1). - - If the projects license file is not found at the top level, the probe returns a single OutcomeNegative (0). -remediation: - effort: Low - text: - - Place the license file at the top level of the project source tree. -ecosystem: - languages: - - all - clients: - - github - - gitlab - - localdir \ No newline at end of file diff --git a/probes/hasLicenseFileAtTopDir/impl.go b/probes/hasLicenseFileAtTopDir/impl.go deleted file mode 100644 index 48fbfb26..00000000 --- a/probes/hasLicenseFileAtTopDir/impl.go +++ /dev/null @@ -1,88 +0,0 @@ -// 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 hasLicenseFileAtTopDir - -import ( - "embed" - "fmt" - - "github.com/ossf/scorecard/v4/checker" - "github.com/ossf/scorecard/v4/finding" - "github.com/ossf/scorecard/v4/internal/probes" - "github.com/ossf/scorecard/v4/probes/internal/utils/uerror" -) - -func init() { - probes.MustRegister(Probe, Run, []probes.CheckName{probes.License}) -} - -//go:embed *.yml -var fs embed.FS - -const Probe = "hasLicenseFileAtTopDir" - -func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { - if raw == nil { - return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) - } - - if raw.LicenseResults.LicenseFiles == nil || len(raw.LicenseResults.LicenseFiles) == 0 { - f, err := finding.NewWith(fs, Probe, - "project does not have a license file", nil, - finding.OutcomeNotApplicable) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - return []finding.Finding{*f}, Probe, nil - } - - for _, licenseFile := range raw.LicenseResults.LicenseFiles { - switch licenseFile.LicenseInformation.Attribution { - case checker.LicenseAttributionTypeAPI, checker.LicenseAttributionTypeHeuristics: - // both repoAPI and scorecard (not using the API) follow checks.md - // for a file to be found it must have been in the correct location - // award location points. - - // Store the file path in the msg - msg := licenseFile.File.Path - f, err := finding.NewWith(fs, Probe, - msg, nil, - finding.OutcomePositive) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - return []finding.Finding{*f}, Probe, nil - - case checker.LicenseAttributionTypeOther: - msg := "License file found in unexpected location" - f, err := finding.NewWith(fs, Probe, - msg, nil, - finding.OutcomeNegative) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - return []finding.Finding{*f}, Probe, nil - } - } - - f, err := finding.NewWith(fs, Probe, - "Did not find the license file at the expected location.", nil, - finding.OutcomeNegative) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - return []finding.Finding{*f}, Probe, nil -} diff --git a/probes/hasLicenseFileAtTopDir/impl_test.go b/probes/hasLicenseFileAtTopDir/impl_test.go deleted file mode 100644 index 1e73feb6..00000000 --- a/probes/hasLicenseFileAtTopDir/impl_test.go +++ /dev/null @@ -1,150 +0,0 @@ -// 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 hasLicenseFileAtTopDir - -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/finding" - "github.com/ossf/scorecard/v4/probes/internal/utils/test" -) - -func Test_Run(t *testing.T) { - t.Parallel() - //nolint:govet - tests := []struct { - name string - raw *checker.RawResults - outcomes []finding.Outcome - err error - }{ - { - name: "License file found and correct attribution 1", - raw: &checker.RawResults{ - LicenseResults: checker.LicenseData{ - LicenseFiles: []checker.LicenseFile{ - { - File: checker.File{ - Path: "LICENSE.md", - }, - LicenseInformation: checker.License{ - Attribution: checker.LicenseAttributionTypeAPI, - }, - }, - }, - }, - }, - outcomes: []finding.Outcome{ - finding.OutcomePositive, - }, - }, - { - name: "License file not found and outcome should be OutcomeNotApplicable", - raw: &checker.RawResults{ - LicenseResults: checker.LicenseData{ - LicenseFiles: []checker.LicenseFile{}, - }, - }, - outcomes: []finding.Outcome{ - finding.OutcomeNotApplicable, - }, - }, - { - name: "License file found and correct attribution 2", - raw: &checker.RawResults{ - LicenseResults: checker.LicenseData{ - LicenseFiles: []checker.LicenseFile{ - { - File: checker.File{ - Path: "LICENSE.md", - }, - LicenseInformation: checker.License{ - Attribution: checker.LicenseAttributionTypeHeuristics, - }, - }, - }, - }, - }, - outcomes: []finding.Outcome{ - finding.OutcomePositive, - }, - }, - { - name: "License file found and wrong attribution", - raw: &checker.RawResults{ - LicenseResults: checker.LicenseData{ - LicenseFiles: []checker.LicenseFile{ - { - File: checker.File{ - Path: "LICENSE.md", - }, - LicenseInformation: checker.License{ - Attribution: "wrong_attribution", - }, - }, - }, - }, - }, - outcomes: []finding.Outcome{ - finding.OutcomeNegative, - }, - }, - { - name: "nil license files and outcome should be OutcomeNotApplicable", - raw: &checker.RawResults{ - LicenseResults: checker.LicenseData{ - LicenseFiles: nil, - }, - }, - outcomes: []finding.Outcome{ - finding.OutcomeNotApplicable, - }, - }, - { - name: "0 license files and outcome should be OutcomeNotApplicable", - raw: &checker.RawResults{ - LicenseResults: checker.LicenseData{ - LicenseFiles: []checker.LicenseFile{}, - }, - }, - outcomes: []finding.Outcome{ - finding.OutcomeNotApplicable, - }, - }, - } - 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) - } - test.AssertOutcomes(t, findings, tt.outcomes) - }) - } -}