From eaac4c8545c2471c137437ac2400d27e9e7c3c81 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Mon, 18 Sep 2023 12:00:49 +0100 Subject: [PATCH 1/8] :seedling: convert packaging check to probe Signed-off-by: AdamKorcz --- checks/evaluation/packaging.go | 84 +++------ checks/evaluation/packaging_test.go | 169 +++++------------- checks/packaging.go | 14 +- probes/entries.go | 4 + probes/packagedWithGithubActions/def.yml | 27 +++ probes/packagedWithGithubActions/impl.go | 59 ++++++ probes/packagedWithGithubActions/impl_test.go | 94 ++++++++++ 7 files changed, 266 insertions(+), 185 deletions(-) create mode 100644 probes/packagedWithGithubActions/def.yml create mode 100644 probes/packagedWithGithubActions/impl.go create mode 100644 probes/packagedWithGithubActions/impl_test.go diff --git a/checks/evaluation/packaging.go b/checks/evaluation/packaging.go index a3c13fd3507..ce86bfb1487 100644 --- a/checks/evaluation/packaging.go +++ b/checks/evaluation/packaging.go @@ -15,75 +15,49 @@ package evaluation import ( - "fmt" - "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/packagedWithGithubActions" ) // Packaging applies the score policy for the Packaging check. -func Packaging(name string, dl checker.DetailLogger, r *checker.PackagingData) checker.CheckResult { - if r == nil { - e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data") - return checker.CreateRuntimeErrorResult(name, e) +func Packaging(name string, + findings []finding.Finding, + dl checker.DetailLogger, +) checker.CheckResult { + expectedProbes := []string{ + packagedWithGithubActions.Probe, } - pass := false - for _, p := range r.Packages { - if p.Msg != nil { - // This is a debug message. Let's just replay the message. - dl.Debug(&checker.LogMessage{ - Text: *p.Msg, - }) - continue - } - - // Presence of a single non-debug message means the - // check passes. - pass = true - - msg, err := createLogMessage(p) - if err != nil { - return checker.CreateRuntimeErrorResult(name, err) - } - dl.Info(&msg) + err := validateFindings(findings, expectedProbes) + if err != nil { + return checker.CreateRuntimeErrorResult(name, err) } - if pass { - return checker.CreateMaxScoreResult(name, - "publishing workflow detected") + // Currently there is only a single packaging probe that returns + // a single positive or negative outcome. As such, in this evaluation, + // we return max score if the outcome is positive and lowest score if + // the outcome is negative. + for _, f := range findings { + if f.Outcome == finding.OutcomePositive { + // Log all findings except the negative ones. + checker.LogFindings(nonNegativeFindings(findings), dl) + return checker.CreateMaxScoreResult(name, "project is published as package") + } } - dl.Warn(&checker.LogMessage{ - Text: "no GitHub/GitLab publishing workflow detected", - }) - - return checker.CreateInconclusiveResult(name, - "no published package detected") + checker.LogFindings(nonNegativeFindings(findings), dl) + return checker.CreateMinScoreResult(name, "project is not published as package") } -func createLogMessage(p checker.Package) (checker.LogMessage, error) { - var msg checker.LogMessage - - if p.Msg != nil { - return msg, sce.WithMessage(sce.ErrScorecardInternal, "Msg should be nil") - } - - if p.File == nil { - return msg, sce.WithMessage(sce.ErrScorecardInternal, "File field is nil") +func validateFindings(findings []finding.Finding, expectedProbes []string) error { + if !finding.UniqueProbesEqual(findings, expectedProbes) { + return sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") } - if p.File != nil { - msg.Path = p.File.Path - msg.Type = p.File.Type - msg.Offset = p.File.Offset + if len(findings) == 0 { + return sce.WithMessage(sce.ErrScorecardInternal, "found 0 findings. Should not happen") } - - if len(p.Runs) == 0 { - return msg, sce.WithMessage(sce.ErrScorecardInternal, "no run data") - } - - msg.Text = fmt.Sprintf("GitHub/GitLab publishing workflow used in run %s", p.Runs[0].URL) - - return msg, nil + return nil } diff --git a/checks/evaluation/packaging_test.go b/checks/evaluation/packaging_test.go index eaf6a623a2e..18aeb055635 100644 --- a/checks/evaluation/packaging_test.go +++ b/checks/evaluation/packaging_test.go @@ -16,153 +16,68 @@ package evaluation import ( "testing" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/finding" scut "github.com/ossf/scorecard/v4/utests" ) -func Test_createLogMessage(t *testing.T) { - msg := "msg" +func TestPackaging(t *testing.T) { t.Parallel() - tests := []struct { //nolint:govet - name string - args checker.Package - want checker.LogMessage - wantErr bool + tests := []struct { + name string + findings []finding.Finding + result scut.TestReturn }{ { - name: "nil package", - args: checker.Package{}, - want: checker.LogMessage{}, - wantErr: true, - }, - { - name: "nil file", - args: checker.Package{ - File: nil, - }, - want: checker.LogMessage{}, - wantErr: true, - }, - { - name: "msg is not nil", - args: checker.Package{ - File: &checker.File{}, - Msg: &msg, - }, - want: checker.LogMessage{ - Text: "", - }, - wantErr: true, - }, - { - name: "file is not nil", - args: checker.Package{ - File: &checker.File{ - Path: "path", + name: "test positive outcome", + findings: []finding.Finding{ + { + Probe: "packagedWithGithubActions", + Outcome: finding.OutcomePositive, }, }, - want: checker.LogMessage{ - Path: "path", + result: scut.TestReturn{ + Score: checker.MaxResultScore, + NumberOfInfo: 1, }, - wantErr: true, }, { - name: "runs are not zero", - args: checker.Package{ - File: &checker.File{ - Path: "path", - }, - Runs: []checker.Run{ - {}, + name: "test positive outcome with wrong probes", + findings: []finding.Finding{ + { + Probe: "wrongProbe", + Outcome: finding.OutcomePositive, }, }, - want: checker.LogMessage{ - Text: "GitHub/GitLab publishing workflow used in run ", - Path: "path", - }, - }, - } - for _, tt := range tests { - tt := tt // Parallel testing - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - got, err := createLogMessage(tt.args) - if (err != nil) != tt.wantErr { - t.Errorf("createLogMessage() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !cmp.Equal(got, tt.want) { - t.Errorf("createLogMessage() got = %v, want %v", got, cmp.Diff(got, tt.want)) - } - }) - } -} - -func TestPackaging(t *testing.T) { - t.Parallel() - type args struct { //nolint:govet - name string - dl checker.DetailLogger - r *checker.PackagingData - } - tests := []struct { - name string - args args - want checker.CheckResult - }{ - { - name: "nil packaging data", - args: args{ - name: "name", - dl: nil, - r: nil, - }, - want: checker.CheckResult{ - Name: "name", - Version: 2, - Score: -1, - Reason: "internal error: empty raw data", + result: scut.TestReturn{ + Score: -1, + Error: sce.ErrScorecardInternal, }, }, { - name: "empty packaging data", - args: args{ - name: "name", - dl: &scut.TestDetailLogger{}, - r: &checker.PackagingData{}, + name: "test negative outcome", + findings: []finding.Finding{ + { + Probe: "packagedWithGithubActions", + Outcome: finding.OutcomeNegative, + }, }, - want: checker.CheckResult{ - Name: "name", - Version: 2, - Score: -1, - Reason: "no published package detected", + result: scut.TestReturn{ + Score: checker.MinResultScore, }, }, { - name: "runs are not zero", - args: args{ - dl: &scut.TestDetailLogger{}, - r: &checker.PackagingData{ - Packages: []checker.Package{ - { - File: &checker.File{ - Path: "path", - }, - Runs: []checker.Run{ - {}, - }, - }, - }, + name: "test negative outcome with wrong probes", + findings: []finding.Finding{ + { + Probe: "wrongProbe", + Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Name: "", - Version: 2, - Score: 10, - Reason: "publishing workflow detected", + result: scut.TestReturn{ + Score: -1, + Error: sce.ErrScorecardInternal, }, }, } @@ -170,8 +85,10 @@ func TestPackaging(t *testing.T) { tt := tt // Parallel testing t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := Packaging(tt.args.name, tt.args.dl, tt.args.r); !cmp.Equal(got, tt.want, cmpopts.IgnoreFields(checker.CheckResult{}, "Error")) { //nolint:lll - t.Errorf("Packaging() = %v, want %v", got, cmp.Diff(got, tt.want, cmpopts.IgnoreFields(checker.CheckResult{}, "Error"))) //nolint:lll + dl := scut.TestDetailLogger{} + got := Packaging(tt.name, tt.findings, &dl) + if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) { + t.Errorf("got %v, expected %v", got, tt.result) } }) } diff --git a/checks/packaging.go b/checks/packaging.go index 1ae19bdaada..cfa7878229b 100644 --- a/checks/packaging.go +++ b/checks/packaging.go @@ -22,6 +22,8 @@ import ( "github.com/ossf/scorecard/v4/clients/githubrepo" "github.com/ossf/scorecard/v4/clients/gitlabrepo" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/probes" + "github.com/ossf/scorecard/v4/probes/zrunner" ) // CheckPackaging is the registered name for Packaging. @@ -54,10 +56,14 @@ func Packaging(c *checker.CheckRequest) checker.CheckResult { return checker.CreateRuntimeErrorResult(CheckPackaging, e) } - // Set the raw results. - if c.RawResults != nil { - c.RawResults.PackagingResults = rawData + pRawResults := getRawResults(c) + pRawResults.PackagingResults = rawData + + findings, err := zrunner.Run(pRawResults, probes.Packaging) + if err != nil { + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return checker.CreateRuntimeErrorResult(CheckPackaging, e) } - return evaluation.Packaging(CheckPackaging, c.Dlogger, &rawData) + return evaluation.Packaging(CheckPackaging, findings, c.Dlogger) } diff --git a/probes/entries.go b/probes/entries.go index 4d96d051f65..b24d6b799c9 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -30,6 +30,7 @@ import ( "github.com/ossf/scorecard/v4/probes/fuzzedWithPythonAtheris" "github.com/ossf/scorecard/v4/probes/fuzzedWithRustCargofuzz" "github.com/ossf/scorecard/v4/probes/fuzzedWithSwiftLibFuzzer" + "github.com/ossf/scorecard/v4/probes/packagedWithGithubActions" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsLinks" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsText" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsVulnerabilityDisclosure" @@ -77,6 +78,9 @@ var ( fuzzedWithPropertyBasedTypescript.Run, fuzzedWithPropertyBasedJavascript.Run, } + Packaging = []ProbeImpl{ + packagedWithGithubActions.Run, + } ) //nolint:gochecknoinits diff --git a/probes/packagedWithGithubActions/def.yml b/probes/packagedWithGithubActions/def.yml new file mode 100644 index 00000000000..8ace205660c --- /dev/null +++ b/probes/packagedWithGithubActions/def.yml @@ -0,0 +1,27 @@ +# 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: packagedWithGithubActions +short: Check that the project is fuzzed using OSS-Fuzz +motivation: > + Packages give users of a project an easy way to download, install, update, and uninstall the software by a package manager. In particular, they make it easy for users to receive security patches as updates. +implementation: > + The implementation checks all the packages of the project. If one of these packages do not have a debug message, then it is considered a release package and the probe returns a positive outcome. +outcome: + - If the project has a package without a debug message, the outcome is positive. + - If the project has a package with a debug message, the outcome is negative. +remediation: + effort: Low + text: + - Use a GitHub action to release your package to language-specific hubs. \ No newline at end of file diff --git a/probes/packagedWithGithubActions/impl.go b/probes/packagedWithGithubActions/impl.go new file mode 100644 index 00000000000..658ccea33e1 --- /dev/null +++ b/probes/packagedWithGithubActions/impl.go @@ -0,0 +1,59 @@ +// 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 packagedWithGithubActions + +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 = "packagedWithGithubActions" + +func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { + if raw == nil { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + + r := raw.PackagingResults + for _, p := range r.Packages { + if p.Msg == nil { + // Presence of a single non-debug message means the + // check passes. + f, err := finding.NewWith(fs, Probe, + "Project packages its releases by way of Github Actions.", nil, + finding.OutcomePositive) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + return []finding.Finding{*f}, Probe, nil + } + } + + f, err := finding.NewWith(fs, Probe, + "Project does not package its releases by way of Github Actions.", 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/packagedWithGithubActions/impl_test.go b/probes/packagedWithGithubActions/impl_test.go new file mode 100644 index 00000000000..5d4e5e2b13a --- /dev/null +++ b/probes/packagedWithGithubActions/impl_test.go @@ -0,0 +1,94 @@ +// 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 packagedWithGithubActions + +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" +) + +func Test_Run(t *testing.T) { + msg := "msg" + t.Parallel() + // nolint:govet + tests := []struct { + name string + raw *checker.RawResults + outcomes []finding.Outcome + err error + }{ + { + name: "debug msg is nil", + raw: &checker.RawResults{ + PackagingResults: checker.PackagingData{ + Packages: []checker.Package{ + {}, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + }, + { + name: "debug msg is not nil", + raw: &checker.RawResults{ + PackagingResults: checker.PackagingData{ + Packages: []checker.Package{ + { + Msg: &msg, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + 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) + } + } + }) + } +} From e7124fe3ac5d6ce0a61e453e9b7e99330580b424 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Thu, 28 Sep 2023 21:22:24 +0100 Subject: [PATCH 2/8] amend text in def.yml Signed-off-by: AdamKorcz --- checks/evaluation/packaging.go | 21 +++++-------------- checks/evaluation/packaging_test.go | 4 ++-- probes/entries.go | 3 ++- .../def.yml | 4 ++-- .../impl.go | 4 ++-- .../impl_test.go | 2 +- 6 files changed, 14 insertions(+), 24 deletions(-) rename probes/{packagedWithGithubActions => packagedNpmWithGitHubWorkflow}/def.yml (80%) rename probes/{packagedWithGithubActions => packagedNpmWithGitHubWorkflow}/impl.go (95%) rename probes/{packagedWithGithubActions => packagedNpmWithGitHubWorkflow}/impl_test.go (98%) diff --git a/checks/evaluation/packaging.go b/checks/evaluation/packaging.go index ce86bfb1487..b560a2d605b 100644 --- a/checks/evaluation/packaging.go +++ b/checks/evaluation/packaging.go @@ -18,7 +18,7 @@ import ( "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/packagedWithGithubActions" + "github.com/ossf/scorecard/v4/probes/packagedNpmWithGitHubWorkflow" ) // Packaging applies the score policy for the Packaging check. @@ -27,12 +27,12 @@ func Packaging(name string, dl checker.DetailLogger, ) checker.CheckResult { expectedProbes := []string{ - packagedWithGithubActions.Probe, + packagedNpmWithGitHubWorkflow.Probe, } - err := validateFindings(findings, expectedProbes) - if err != nil { - return checker.CreateRuntimeErrorResult(name, err) + if !finding.UniqueProbesEqual(findings, expectedProbes) { + e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") + return checker.CreateRuntimeErrorResult(name, e) } // Currently there is only a single packaging probe that returns @@ -50,14 +50,3 @@ func Packaging(name string, checker.LogFindings(nonNegativeFindings(findings), dl) return checker.CreateMinScoreResult(name, "project is not published as package") } - -func validateFindings(findings []finding.Finding, expectedProbes []string) error { - if !finding.UniqueProbesEqual(findings, expectedProbes) { - return sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") - } - - if len(findings) == 0 { - return sce.WithMessage(sce.ErrScorecardInternal, "found 0 findings. Should not happen") - } - return nil -} diff --git a/checks/evaluation/packaging_test.go b/checks/evaluation/packaging_test.go index 18aeb055635..fd89909ee27 100644 --- a/checks/evaluation/packaging_test.go +++ b/checks/evaluation/packaging_test.go @@ -33,7 +33,7 @@ func TestPackaging(t *testing.T) { name: "test positive outcome", findings: []finding.Finding{ { - Probe: "packagedWithGithubActions", + Probe: "packagedNpmWithGitHubWorkflow", Outcome: finding.OutcomePositive, }, }, @@ -59,7 +59,7 @@ func TestPackaging(t *testing.T) { name: "test negative outcome", findings: []finding.Finding{ { - Probe: "packagedWithGithubActions", + Probe: "packagedNpmWithGitHubWorkflow", Outcome: finding.OutcomeNegative, }, }, diff --git a/probes/entries.go b/probes/entries.go index b24d6b799c9..06ccc570dd9 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -31,6 +31,7 @@ import ( "github.com/ossf/scorecard/v4/probes/fuzzedWithRustCargofuzz" "github.com/ossf/scorecard/v4/probes/fuzzedWithSwiftLibFuzzer" "github.com/ossf/scorecard/v4/probes/packagedWithGithubActions" + "github.com/ossf/scorecard/v4/probes/packagedNpmWithGitHubWorkflow" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsLinks" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsText" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsVulnerabilityDisclosure" @@ -79,7 +80,7 @@ var ( fuzzedWithPropertyBasedJavascript.Run, } Packaging = []ProbeImpl{ - packagedWithGithubActions.Run, + packagedNpmWithGitHubWorkflow.Run, } ) diff --git a/probes/packagedWithGithubActions/def.yml b/probes/packagedNpmWithGitHubWorkflow/def.yml similarity index 80% rename from probes/packagedWithGithubActions/def.yml rename to probes/packagedNpmWithGitHubWorkflow/def.yml index 8ace205660c..dfa72d5746c 100644 --- a/probes/packagedWithGithubActions/def.yml +++ b/probes/packagedNpmWithGitHubWorkflow/def.yml @@ -12,12 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -id: packagedWithGithubActions +id: packagedNpmWithGitHubWorkflow short: Check that the project is fuzzed using OSS-Fuzz motivation: > Packages give users of a project an easy way to download, install, update, and uninstall the software by a package manager. In particular, they make it easy for users to receive security patches as updates. implementation: > - The implementation checks all the packages of the project. If one of these packages do not have a debug message, then it is considered a release package and the probe returns a positive outcome. + The implementation checks whether a project uses common patterns for packaging across multiple ecosystems. Scorecard gets this by checking the projects workflows for specific uses of actions and build commands such as `docker push` or `mvn deploy`. outcome: - If the project has a package without a debug message, the outcome is positive. - If the project has a package with a debug message, the outcome is negative. diff --git a/probes/packagedWithGithubActions/impl.go b/probes/packagedNpmWithGitHubWorkflow/impl.go similarity index 95% rename from probes/packagedWithGithubActions/impl.go rename to probes/packagedNpmWithGitHubWorkflow/impl.go index 658ccea33e1..85e59bc21fe 100644 --- a/probes/packagedWithGithubActions/impl.go +++ b/probes/packagedNpmWithGitHubWorkflow/impl.go @@ -13,7 +13,7 @@ // limitations under the License. // nolint:stylecheck -package packagedWithGithubActions +package packagedNpmWithGitHubWorkflow import ( "embed" @@ -27,7 +27,7 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "packagedWithGithubActions" +const Probe = "packagedNpmWithGitHubWorkflow" func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { diff --git a/probes/packagedWithGithubActions/impl_test.go b/probes/packagedNpmWithGitHubWorkflow/impl_test.go similarity index 98% rename from probes/packagedWithGithubActions/impl_test.go rename to probes/packagedNpmWithGitHubWorkflow/impl_test.go index 5d4e5e2b13a..d156424bd7d 100644 --- a/probes/packagedWithGithubActions/impl_test.go +++ b/probes/packagedNpmWithGitHubWorkflow/impl_test.go @@ -13,7 +13,7 @@ // limitations under the License. // nolint:stylecheck -package packagedWithGithubActions +package packagedNpmWithGitHubWorkflow import ( "testing" From 099185cd87281c3f21acb43306eba8b5b9c10a9a Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Wed, 11 Oct 2023 15:03:49 +0100 Subject: [PATCH 3/8] Correct short description in def.yml Signed-off-by: AdamKorcz --- probes/packagedNpmWithGitHubWorkflow/def.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probes/packagedNpmWithGitHubWorkflow/def.yml b/probes/packagedNpmWithGitHubWorkflow/def.yml index dfa72d5746c..01171a45ba7 100644 --- a/probes/packagedNpmWithGitHubWorkflow/def.yml +++ b/probes/packagedNpmWithGitHubWorkflow/def.yml @@ -13,7 +13,7 @@ # limitations under the License. id: packagedNpmWithGitHubWorkflow -short: Check that the project is fuzzed using OSS-Fuzz +short: Checks whether the project uses automated packaging. motivation: > Packages give users of a project an easy way to download, install, update, and uninstall the software by a package manager. In particular, they make it easy for users to receive security patches as updates. implementation: > From 148801e5b24c381a0b3549caa46f75597a42b690 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Wed, 11 Oct 2023 18:59:53 +0100 Subject: [PATCH 4/8] log negative findings Signed-off-by: AdamKorcz --- checks/evaluation/finding.go | 11 +++++++++++ checks/evaluation/packaging.go | 2 +- checks/evaluation/packaging_test.go | 3 ++- probes/entries.go | 1 - 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/checks/evaluation/finding.go b/checks/evaluation/finding.go index f11c8fad34c..11d1e6f9fc4 100644 --- a/checks/evaluation/finding.go +++ b/checks/evaluation/finding.go @@ -29,3 +29,14 @@ func nonNegativeFindings(findings []finding.Finding) []finding.Finding { } return ff } + +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) + } + } + return ff +} diff --git a/checks/evaluation/packaging.go b/checks/evaluation/packaging.go index b560a2d605b..733fd6a51fd 100644 --- a/checks/evaluation/packaging.go +++ b/checks/evaluation/packaging.go @@ -47,6 +47,6 @@ func Packaging(name string, } } - checker.LogFindings(nonNegativeFindings(findings), dl) + checker.LogFindings(negativeFindings(findings), dl) return checker.CreateMinScoreResult(name, "project is not published as package") } diff --git a/checks/evaluation/packaging_test.go b/checks/evaluation/packaging_test.go index fd89909ee27..4a7a5bf1113 100644 --- a/checks/evaluation/packaging_test.go +++ b/checks/evaluation/packaging_test.go @@ -64,7 +64,8 @@ func TestPackaging(t *testing.T) { }, }, result: scut.TestReturn{ - Score: checker.MinResultScore, + Score: checker.MinResultScore, + NumberOfWarn: 1, }, }, { diff --git a/probes/entries.go b/probes/entries.go index 06ccc570dd9..44632ff4d5d 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -30,7 +30,6 @@ import ( "github.com/ossf/scorecard/v4/probes/fuzzedWithPythonAtheris" "github.com/ossf/scorecard/v4/probes/fuzzedWithRustCargofuzz" "github.com/ossf/scorecard/v4/probes/fuzzedWithSwiftLibFuzzer" - "github.com/ossf/scorecard/v4/probes/packagedWithGithubActions" "github.com/ossf/scorecard/v4/probes/packagedNpmWithGitHubWorkflow" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsLinks" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsText" From ddae0cafc633a0b6fdb0aa7fe299d855c4d26d5a Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 20 Oct 2023 09:25:37 +0100 Subject: [PATCH 5/8] rename probe Signed-off-by: AdamKorcz --- checks/evaluation/packaging.go | 4 ++-- checks/evaluation/packaging_test.go | 4 ++-- probes/entries.go | 4 ++-- .../def.yml | 2 +- .../impl.go | 4 ++-- .../impl_test.go | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) rename probes/{packagedNpmWithGitHubWorkflow => packagedWithAutomatedWorkflow}/def.yml (97%) rename probes/{packagedNpmWithGitHubWorkflow => packagedWithAutomatedWorkflow}/impl.go (95%) rename probes/{packagedNpmWithGitHubWorkflow => packagedWithAutomatedWorkflow}/impl_test.go (98%) diff --git a/checks/evaluation/packaging.go b/checks/evaluation/packaging.go index 733fd6a51fd..4fa34a32a6e 100644 --- a/checks/evaluation/packaging.go +++ b/checks/evaluation/packaging.go @@ -18,7 +18,7 @@ import ( "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/packagedNpmWithGitHubWorkflow" + "github.com/ossf/scorecard/v4/probes/packagedWithAutomatedWorkflow" ) // Packaging applies the score policy for the Packaging check. @@ -27,7 +27,7 @@ func Packaging(name string, dl checker.DetailLogger, ) checker.CheckResult { expectedProbes := []string{ - packagedNpmWithGitHubWorkflow.Probe, + packagedWithAutomatedWorkflow.Probe, } if !finding.UniqueProbesEqual(findings, expectedProbes) { diff --git a/checks/evaluation/packaging_test.go b/checks/evaluation/packaging_test.go index 4a7a5bf1113..9af18702640 100644 --- a/checks/evaluation/packaging_test.go +++ b/checks/evaluation/packaging_test.go @@ -33,7 +33,7 @@ func TestPackaging(t *testing.T) { name: "test positive outcome", findings: []finding.Finding{ { - Probe: "packagedNpmWithGitHubWorkflow", + Probe: "packagedWithAutomatedWorkflow", Outcome: finding.OutcomePositive, }, }, @@ -59,7 +59,7 @@ func TestPackaging(t *testing.T) { name: "test negative outcome", findings: []finding.Finding{ { - Probe: "packagedNpmWithGitHubWorkflow", + Probe: "packagedWithAutomatedWorkflow", Outcome: finding.OutcomeNegative, }, }, diff --git a/probes/entries.go b/probes/entries.go index 44632ff4d5d..6a750cbc794 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -30,7 +30,7 @@ import ( "github.com/ossf/scorecard/v4/probes/fuzzedWithPythonAtheris" "github.com/ossf/scorecard/v4/probes/fuzzedWithRustCargofuzz" "github.com/ossf/scorecard/v4/probes/fuzzedWithSwiftLibFuzzer" - "github.com/ossf/scorecard/v4/probes/packagedNpmWithGitHubWorkflow" + "github.com/ossf/scorecard/v4/probes/packagedWithAutomatedWorkflow" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsLinks" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsText" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsVulnerabilityDisclosure" @@ -79,7 +79,7 @@ var ( fuzzedWithPropertyBasedJavascript.Run, } Packaging = []ProbeImpl{ - packagedNpmWithGitHubWorkflow.Run, + packagedWithAutomatedWorkflow.Run, } ) diff --git a/probes/packagedNpmWithGitHubWorkflow/def.yml b/probes/packagedWithAutomatedWorkflow/def.yml similarity index 97% rename from probes/packagedNpmWithGitHubWorkflow/def.yml rename to probes/packagedWithAutomatedWorkflow/def.yml index 01171a45ba7..a7df0433ec3 100644 --- a/probes/packagedNpmWithGitHubWorkflow/def.yml +++ b/probes/packagedWithAutomatedWorkflow/def.yml @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -id: packagedNpmWithGitHubWorkflow +id: packagedWithAutomatedWorkflow short: Checks whether the project uses automated packaging. motivation: > Packages give users of a project an easy way to download, install, update, and uninstall the software by a package manager. In particular, they make it easy for users to receive security patches as updates. diff --git a/probes/packagedNpmWithGitHubWorkflow/impl.go b/probes/packagedWithAutomatedWorkflow/impl.go similarity index 95% rename from probes/packagedNpmWithGitHubWorkflow/impl.go rename to probes/packagedWithAutomatedWorkflow/impl.go index 85e59bc21fe..d3e9732827f 100644 --- a/probes/packagedNpmWithGitHubWorkflow/impl.go +++ b/probes/packagedWithAutomatedWorkflow/impl.go @@ -13,7 +13,7 @@ // limitations under the License. // nolint:stylecheck -package packagedNpmWithGitHubWorkflow +package packagedWithAutomatedWorkflow import ( "embed" @@ -27,7 +27,7 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "packagedNpmWithGitHubWorkflow" +const Probe = "packagedWithAutomatedWorkflow" func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { diff --git a/probes/packagedNpmWithGitHubWorkflow/impl_test.go b/probes/packagedWithAutomatedWorkflow/impl_test.go similarity index 98% rename from probes/packagedNpmWithGitHubWorkflow/impl_test.go rename to probes/packagedWithAutomatedWorkflow/impl_test.go index d156424bd7d..0d9e6b38bc2 100644 --- a/probes/packagedNpmWithGitHubWorkflow/impl_test.go +++ b/probes/packagedWithAutomatedWorkflow/impl_test.go @@ -13,7 +13,7 @@ // limitations under the License. // nolint:stylecheck -package packagedNpmWithGitHubWorkflow +package packagedWithAutomatedWorkflow import ( "testing" From cebda784cc03152d36253a4a728a98f769b0b711 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 20 Oct 2023 12:01:04 +0100 Subject: [PATCH 6/8] Fix the broken e2e test: The probe returned minimum score instead of inconclusive score which was not consistent with the previous scoring. This commit also removes the debug statements Signed-off-by: AdamKorcz --- checks/evaluation/finding.go | 11 +++++++++++ checks/evaluation/packaging.go | 5 +++-- checks/evaluation/packaging_test.go | 4 ++-- e2e/packaging_test.go | 2 +- probes/packagedWithAutomatedWorkflow/impl.go | 2 +- 5 files changed, 18 insertions(+), 6 deletions(-) diff --git a/checks/evaluation/finding.go b/checks/evaluation/finding.go index 11d1e6f9fc4..cecc9608c1f 100644 --- a/checks/evaluation/finding.go +++ b/checks/evaluation/finding.go @@ -40,3 +40,14 @@ func negativeFindings(findings []finding.Finding) []finding.Finding { } return ff } + +func positiveFindings(findings []finding.Finding) []finding.Finding { + var ff []finding.Finding + for i := range findings { + f := &findings[i] + if f.Outcome == finding.OutcomePositive { + ff = append(ff, *f) + } + } + return ff +} diff --git a/checks/evaluation/packaging.go b/checks/evaluation/packaging.go index 4fa34a32a6e..739ac30756a 100644 --- a/checks/evaluation/packaging.go +++ b/checks/evaluation/packaging.go @@ -42,11 +42,12 @@ func Packaging(name string, for _, f := range findings { if f.Outcome == finding.OutcomePositive { // Log all findings except the negative ones. - checker.LogFindings(nonNegativeFindings(findings), dl) + checker.LogFindings(positiveFindings(findings), dl) return checker.CreateMaxScoreResult(name, "project is published as package") } } checker.LogFindings(negativeFindings(findings), dl) - return checker.CreateMinScoreResult(name, "project is not published as package") + return checker.CreateInconclusiveResult(name, + "no published package detected") } diff --git a/checks/evaluation/packaging_test.go b/checks/evaluation/packaging_test.go index 9af18702640..459552e35be 100644 --- a/checks/evaluation/packaging_test.go +++ b/checks/evaluation/packaging_test.go @@ -56,7 +56,7 @@ func TestPackaging(t *testing.T) { }, }, { - name: "test negative outcome", + name: "test inconclusive outcome", findings: []finding.Finding{ { Probe: "packagedWithAutomatedWorkflow", @@ -64,7 +64,7 @@ func TestPackaging(t *testing.T) { }, }, result: scut.TestReturn{ - Score: checker.MinResultScore, + Score: checker.InconclusiveResultScore, NumberOfWarn: 1, }, }, diff --git a/e2e/packaging_test.go b/e2e/packaging_test.go index 197505660da..9c6eb42b269 100644 --- a/e2e/packaging_test.go +++ b/e2e/packaging_test.go @@ -47,7 +47,7 @@ var _ = Describe("E2E TEST:"+checks.CheckPackaging, func() { Score: checker.InconclusiveResultScore, NumberOfWarn: 1, NumberOfInfo: 0, - NumberOfDebug: 4, + NumberOfDebug: 0, } result := checks.Packaging(&req) Expect(scut.ValidateTestReturn(nil, "use packaging", &expected, &result, &dl)).Should(BeTrue()) diff --git a/probes/packagedWithAutomatedWorkflow/impl.go b/probes/packagedWithAutomatedWorkflow/impl.go index d3e9732827f..b52c609f055 100644 --- a/probes/packagedWithAutomatedWorkflow/impl.go +++ b/probes/packagedWithAutomatedWorkflow/impl.go @@ -50,7 +50,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } f, err := finding.NewWith(fs, Probe, - "Project does not package its releases by way of Github Actions.", nil, + "no GitHub/GitLab publishing workflow detected.", nil, finding.OutcomeNegative) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) From 3c08b5d0e928e50998792be0990b1fcf9168f32c Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Tue, 24 Oct 2023 18:37:42 +0100 Subject: [PATCH 7/8] change score text Signed-off-by: AdamKorcz --- checks/evaluation/packaging.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/checks/evaluation/packaging.go b/checks/evaluation/packaging.go index 739ac30756a..4ec97759e75 100644 --- a/checks/evaluation/packaging.go +++ b/checks/evaluation/packaging.go @@ -43,11 +43,11 @@ func Packaging(name string, if f.Outcome == finding.OutcomePositive { // Log all findings except the negative ones. checker.LogFindings(positiveFindings(findings), dl) - return checker.CreateMaxScoreResult(name, "project is published as package") + return checker.CreateMaxScoreResult(name, "packaging workflow detected") } } checker.LogFindings(negativeFindings(findings), dl) return checker.CreateInconclusiveResult(name, - "no published package detected") + "packaging workflow not detected") } From 5e460a33d0adee393a6c58c9e8355b71c858303d Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Tue, 24 Oct 2023 18:58:55 +0100 Subject: [PATCH 8/8] include file details. process all packaging workflows Signed-off-by: AdamKorcz --- checks/evaluation/finding.go | 11 ------- checks/evaluation/packaging.go | 11 +++++-- probes/packagedWithAutomatedWorkflow/impl.go | 34 ++++++++++++++------ 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/checks/evaluation/finding.go b/checks/evaluation/finding.go index cecc9608c1f..11d1e6f9fc4 100644 --- a/checks/evaluation/finding.go +++ b/checks/evaluation/finding.go @@ -40,14 +40,3 @@ func negativeFindings(findings []finding.Finding) []finding.Finding { } return ff } - -func positiveFindings(findings []finding.Finding) []finding.Finding { - var ff []finding.Finding - for i := range findings { - f := &findings[i] - if f.Outcome == finding.OutcomePositive { - ff = append(ff, *f) - } - } - return ff -} diff --git a/checks/evaluation/packaging.go b/checks/evaluation/packaging.go index 4ec97759e75..da69d8c196f 100644 --- a/checks/evaluation/packaging.go +++ b/checks/evaluation/packaging.go @@ -39,13 +39,20 @@ func Packaging(name string, // a single positive or negative outcome. As such, in this evaluation, // we return max score if the outcome is positive and lowest score if // the outcome is negative. + maxScore := false for _, f := range findings { + f := f if f.Outcome == finding.OutcomePositive { + maxScore = true // Log all findings except the negative ones. - checker.LogFindings(positiveFindings(findings), dl) - return checker.CreateMaxScoreResult(name, "packaging workflow detected") + dl.Info(&checker.LogMessage{ + Finding: &f, + }) } } + if maxScore { + return checker.CreateMaxScoreResult(name, "packaging workflow detected") + } checker.LogFindings(negativeFindings(findings), dl) return checker.CreateInconclusiveResult(name, diff --git a/probes/packagedWithAutomatedWorkflow/impl.go b/probes/packagedWithAutomatedWorkflow/impl.go index b52c609f055..bf261d94f7b 100644 --- a/probes/packagedWithAutomatedWorkflow/impl.go +++ b/probes/packagedWithAutomatedWorkflow/impl.go @@ -35,18 +35,32 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } r := raw.PackagingResults + var findings []finding.Finding for _, p := range r.Packages { - if p.Msg == nil { - // Presence of a single non-debug message means the - // check passes. - f, err := finding.NewWith(fs, Probe, - "Project packages its releases by way of Github Actions.", nil, - finding.OutcomePositive) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - return []finding.Finding{*f}, Probe, nil + p := p + if p.Msg != nil { + continue } + // Presence of a single non-debug message means the + // check passes. + f, err := finding.NewWith(fs, Probe, + "Project packages its releases by way of Github Actions.", nil, + finding.OutcomePositive) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + loc := &finding.Location{} + if p.File != nil { + loc.Path = p.File.Path + loc.Type = p.File.Type + loc.LineStart = &p.File.Offset + } + f = f.WithLocation(loc) + findings = append(findings, *f) + } + + if len(findings) > 0 { + return findings, Probe, nil } f, err := finding.NewWith(fs, Probe,