From 4e00ca89a589efadddcf467c7acde22e849f3757 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Wed, 20 Mar 2024 10:22:53 -0700 Subject: [PATCH 1/5] switch no dependencies to OutcomeNotApplicable OutcomeNotApplicable is what we've been using for cases where there are no occurences of X. Previously this outcome was used for this probe to handle some error cases, but OutcomeError is currently being used. Existing callers were moved to OutcomeNotSupported. Signed-off-by: Spencer Schrock --- checks/evaluation/pinned_dependencies.go | 4 ++-- checks/evaluation/pinned_dependencies_test.go | 2 +- probes/pinsDependencies/impl.go | 12 +++++------- probes/pinsDependencies/impl_test.go | 8 ++++---- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/checks/evaluation/pinned_dependencies.go b/checks/evaluation/pinned_dependencies.go index f1526c5007c0..fe02baf1e268 100644 --- a/checks/evaluation/pinned_dependencies.go +++ b/checks/evaluation/pinned_dependencies.go @@ -81,9 +81,9 @@ func PinningDependencies(name string, for i := range findings { f := findings[i] switch f.Outcome { - case finding.OutcomeNotAvailable: - return checker.CreateInconclusiveResult(name, "no dependencies found") case finding.OutcomeNotApplicable: + return checker.CreateInconclusiveResult(name, "no dependencies found") + case finding.OutcomeNotSupported: if f.Location != nil { dl.Debug(&checker.LogMessage{ Path: f.Location.Path, diff --git a/checks/evaluation/pinned_dependencies_test.go b/checks/evaluation/pinned_dependencies_test.go index 5af1bead9db6..f8b5adc2ed59 100644 --- a/checks/evaluation/pinned_dependencies_test.go +++ b/checks/evaluation/pinned_dependencies_test.go @@ -288,7 +288,7 @@ func Test_PinningDependencies(t *testing.T) { findings: []finding.Finding{ { Probe: "pinsDependencies", - Outcome: finding.OutcomeNotApplicable, + Outcome: finding.OutcomeNotSupported, Location: &finding.Location{ Type: finding.FileTypeText, Path: "test-file", diff --git a/probes/pinsDependencies/impl.go b/probes/pinsDependencies/impl.go index 1cfd239e08d9..359c0e35b73c 100644 --- a/probes/pinsDependencies/impl.go +++ b/probes/pinsDependencies/impl.go @@ -62,7 +62,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for i := range r.Dependencies { rr := r.Dependencies[i] - f, err := finding.NewWith(fs, Probe, "", nil, finding.OutcomeNotApplicable) + f, err := finding.NewWith(fs, Probe, "", nil, finding.OutcomeNotSupported) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } @@ -71,7 +71,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { e := sce.WithMessage(sce.ErrScorecardInternal, "empty File field") return findings, Probe, e } - f = f.WithMessage(*rr.Msg).WithOutcome(finding.OutcomeNotApplicable) + f = f.WithMessage(*rr.Msg).WithOutcome(finding.OutcomeNotSupported) findings = append(findings, *f) continue } @@ -83,7 +83,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { LineEnd: &rr.Location.EndOffset, Snippet: &rr.Location.Snippet, } - f = f.WithMessage(*rr.Msg).WithLocation(loc).WithOutcome(finding.OutcomeNotApplicable) + f = f.WithMessage(*rr.Msg).WithLocation(loc).WithOutcome(finding.OutcomeNotSupported) findings = append(findings, *f) continue } @@ -97,7 +97,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } f = f.WithMessage(fmt.Sprintf("%s has empty Pinned field", rr.Type)). WithLocation(loc). - WithOutcome(finding.OutcomeNotApplicable) + WithOutcome(finding.OutcomeNotSupported) findings = append(findings, *f) continue } @@ -136,9 +136,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } if len(findings) == 0 { - f, err := finding.NewWith(fs, Probe, - "no dependencies found", nil, - finding.OutcomeNotAvailable) + f, err := finding.NewWith(fs, Probe, "no dependencies found", nil, finding.OutcomeNotApplicable) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } diff --git a/probes/pinsDependencies/impl_test.go b/probes/pinsDependencies/impl_test.go index c0ae087def29..2f30650124c4 100644 --- a/probes/pinsDependencies/impl_test.go +++ b/probes/pinsDependencies/impl_test.go @@ -206,7 +206,7 @@ func Test_Run(t *testing.T) { }, }, outcomes: []finding.Outcome{ - finding.OutcomeNotAvailable, + finding.OutcomeNotApplicable, }, }, { @@ -509,7 +509,7 @@ func Test_Run(t *testing.T) { }, }, outcomes: []finding.Outcome{ - finding.OutcomeNotApplicable, + finding.OutcomeNotSupported, }, }, { @@ -527,7 +527,7 @@ func Test_Run(t *testing.T) { }, }, outcomes: []finding.Outcome{ - finding.OutcomeNotApplicable, + finding.OutcomeNotSupported, }, }, { @@ -545,7 +545,7 @@ func Test_Run(t *testing.T) { }, }, outcomes: []finding.Outcome{ - finding.OutcomeNotApplicable, + finding.OutcomeNotSupported, }, }, { From 6439b056c306bed2429acf5304fa21338f8dfd52 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Wed, 20 Mar 2024 10:30:34 -0700 Subject: [PATCH 2/5] deduplicate location setting checker.File.Location() is nil safe, so this should work when we have a location or not Signed-off-by: Spencer Schrock --- probes/pinsDependencies/impl.go | 37 ++++----------------------------- 1 file changed, 4 insertions(+), 33 deletions(-) diff --git a/probes/pinsDependencies/impl.go b/probes/pinsDependencies/impl.go index 359c0e35b73c..d000a4412fae 100644 --- a/probes/pinsDependencies/impl.go +++ b/probes/pinsDependencies/impl.go @@ -62,7 +62,8 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for i := range r.Dependencies { rr := r.Dependencies[i] - f, err := finding.NewWith(fs, Probe, "", nil, finding.OutcomeNotSupported) + loc := rr.Location.Location() + f, err := finding.NewWith(fs, Probe, "", loc, finding.OutcomeNotSupported) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } @@ -76,41 +77,18 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { continue } if rr.Msg != nil { - loc := &finding.Location{ - Type: rr.Location.Type, - Path: rr.Location.Path, - LineStart: &rr.Location.Offset, - LineEnd: &rr.Location.EndOffset, - Snippet: &rr.Location.Snippet, - } - f = f.WithMessage(*rr.Msg).WithLocation(loc).WithOutcome(finding.OutcomeNotSupported) + f = f.WithMessage(*rr.Msg).WithOutcome(finding.OutcomeNotSupported) findings = append(findings, *f) continue } if rr.Pinned == nil { - loc := &finding.Location{ - Type: rr.Location.Type, - Path: rr.Location.Path, - LineStart: &rr.Location.Offset, - LineEnd: &rr.Location.EndOffset, - Snippet: &rr.Location.Snippet, - } f = f.WithMessage(fmt.Sprintf("%s has empty Pinned field", rr.Type)). - WithLocation(loc). WithOutcome(finding.OutcomeNotSupported) findings = append(findings, *f) continue } if !*rr.Pinned { - loc := &finding.Location{ - Type: rr.Location.Type, - Path: rr.Location.Path, - LineStart: &rr.Location.Offset, - LineEnd: &rr.Location.EndOffset, - Snippet: &rr.Location.Snippet, - } f = f.WithMessage(generateTextUnpinned(&rr)). - WithLocation(loc). WithOutcome(finding.OutcomeNegative) if rr.Remediation != nil { f.Remediation = ruleRemToProbeRem(rr.Remediation) @@ -120,14 +98,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { }) findings = append(findings, *f) } else { - loc := &finding.Location{ - Type: rr.Location.Type, - Path: rr.Location.Path, - LineStart: &rr.Location.Offset, - LineEnd: &rr.Location.EndOffset, - Snippet: &rr.Location.Snippet, - } - f = f.WithMessage("").WithLocation(loc).WithOutcome(finding.OutcomePositive) + f = f.WithMessage("").WithOutcome(finding.OutcomePositive) f = f.WithValues(map[string]string{ DepTypeKey: string(rr.Type), }) From 4dece778b5f8d2ffc5859d56ee9baa839a142f40 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Wed, 20 Mar 2024 10:39:06 -0700 Subject: [PATCH 3/5] update outcome descriptions Signed-off-by: Spencer Schrock --- probes/pinsDependencies/def.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/probes/pinsDependencies/def.yml b/probes/pinsDependencies/def.yml index 42d410a7880e..55e95233ee2d 100644 --- a/probes/pinsDependencies/def.yml +++ b/probes/pinsDependencies/def.yml @@ -19,9 +19,9 @@ motivation: > implementation: > The probe works by looking for unpinned dependencies in Dockerfiles, shell scripts, and GitHub workflows which are used during the build and release process of a project. Special considerations for Go modules treat full semantic versions as pinned due to how the Go tool verifies downloaded content against the hashes when anyone first downloaded the module. outcome: - - For each of the last 5 releases, the probe returns OutcomePositive, if the release has a signature file in the release assets. - - For each of the last 5 releases, the probe returns OutcomeNegative, if the release does not have a signature file in the release assets. - - If the project has no releases, the probe returns OutcomeNotApplicable. + - For supported ecosystem, the probe returns OutcomePositive per pinned dependency. + - For supported ecosystem, the probe returns OutcomeNegative per unpinned dependency. + - If the project has no supported dependencies, the probe returns OutcomeNotApplicable. remediation: effort: Medium text: From b1391f3f24f279fc72ca4149dd1a34ff1dd5f0b5 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Wed, 20 Mar 2024 10:54:41 -0700 Subject: [PATCH 4/5] simplify OutcomeNotSupported logging path Signed-off-by: Spencer Schrock --- checks/evaluation/pinned_dependencies.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/checks/evaluation/pinned_dependencies.go b/checks/evaluation/pinned_dependencies.go index fe02baf1e268..2aaddff3cda3 100644 --- a/checks/evaluation/pinned_dependencies.go +++ b/checks/evaluation/pinned_dependencies.go @@ -84,22 +84,13 @@ func PinningDependencies(name string, case finding.OutcomeNotApplicable: return checker.CreateInconclusiveResult(name, "no dependencies found") case finding.OutcomeNotSupported: - if f.Location != nil { - dl.Debug(&checker.LogMessage{ - Path: f.Location.Path, - Type: f.Location.Type, - Offset: *f.Location.LineStart, - EndOffset: *f.Location.LineEnd, - Text: f.Message, - Snippet: *f.Location.Snippet, - }) - } else { - dl.Debug(&checker.LogMessage{ - Text: f.Message, - }) - } + dl.Debug(&checker.LogMessage{ + Finding: &f, + }) continue case finding.OutcomeNegative: + // we cant use the finding if we want the remediation to show + // finding.Remediation are currently suppressed (#3349) lm := &checker.LogMessage{ Path: f.Location.Path, Type: f.Location.Type, From 954c3ea92c9b091774e97a3f3d693c88c28ddb4a Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Wed, 20 Mar 2024 11:04:00 -0700 Subject: [PATCH 5/5] add tests for no deps and processing errors Signed-off-by: Spencer Schrock --- checks/evaluation/pinned_dependencies_test.go | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/checks/evaluation/pinned_dependencies_test.go b/checks/evaluation/pinned_dependencies_test.go index f8b5adc2ed59..cd9259a1e348 100644 --- a/checks/evaluation/pinned_dependencies_test.go +++ b/checks/evaluation/pinned_dependencies_test.go @@ -21,6 +21,7 @@ import ( "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/pinsDependencies" scut "github.com/ossf/scorecard/v4/utests" ) @@ -405,6 +406,57 @@ func Test_PinningDependencies(t *testing.T) { NumberOfInfo: 1, }, }, + { + name: "no dependencies leads to an inconclusive score", + findings: []finding.Finding{ + { + Probe: pinsDependencies.Probe, + Outcome: finding.OutcomeNotApplicable, + }, + }, + result: scut.TestReturn{ + Score: checker.InconclusiveResultScore, + }, + }, + { + name: "processing errors are logged as info", + findings: []finding.Finding{ + { + Probe: pinsDependencies.Probe, + Outcome: finding.OutcomeError, + }, + }, + result: scut.TestReturn{ + Score: checker.InconclusiveResultScore, + NumberOfInfo: 1, + }, + }, + { + name: "processing errors dont affect other dependencies", + findings: []finding.Finding{ + { + Probe: pinsDependencies.Probe, + Outcome: finding.OutcomeError, + }, + { + Probe: pinsDependencies.Probe, + Outcome: finding.OutcomePositive, + Location: &finding.Location{ + Type: finding.FileTypeText, + Path: "test-file", + LineStart: &testLineStart, + Snippet: &testSnippet, + }, + Values: map[string]string{ + "dependencyType": string(checker.DependencyUseTypePipCommand), + }, + }, + }, + result: scut.TestReturn{ + Score: checker.MaxResultScore, + NumberOfInfo: 2, // 1 for processing error, 1 for pinned pip ecosystem + }, + }, } for _, tt := range tests {