From a0a5762763706ad11a6caf25a6903d1d1a3e9424 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Mon, 29 Jan 2024 18:46:01 +0000 Subject: [PATCH 01/13] :seedling: Convert pinned dependencies to probe Signed-off-by: Adam Korczynski --- checks/evaluation/pinned_dependencies.go | 149 +--- checks/evaluation/pinned_dependencies_test.go | 738 +++--------------- checks/pinned_dependencies.go | 15 +- probes/entries.go | 4 + probes/pinsDependencies/def.yml | 28 + probes/pinsDependencies/impl.go | 201 +++++ probes/pinsDependencies/impl_test.go | 588 ++++++++++++++ 7 files changed, 966 insertions(+), 757 deletions(-) create mode 100644 probes/pinsDependencies/def.yml create mode 100644 probes/pinsDependencies/impl.go create mode 100644 probes/pinsDependencies/impl_test.go diff --git a/checks/evaluation/pinned_dependencies.go b/checks/evaluation/pinned_dependencies.go index 368a8587adf..087bdea0791 100644 --- a/checks/evaluation/pinned_dependencies.go +++ b/checks/evaluation/pinned_dependencies.go @@ -22,6 +22,7 @@ import ( sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/finding" "github.com/ossf/scorecard/v4/finding/probe" + "github.com/ossf/scorecard/v4/probes/pinsDependencies" "github.com/ossf/scorecard/v4/rule" ) @@ -54,15 +55,6 @@ const ( depTypeKey = "dependencyType" ) -func ruleRemToProbeRem(rem *rule.Remediation) *probe.Remediation { - return &probe.Remediation{ - Patch: rem.Patch, - Text: rem.Text, - Markdown: rem.Markdown, - Effort: probe.RemediationEffort(rem.Effort), - } -} - func probeRemToRuleRem(rem *probe.Remediation) *rule.Remediation { return &rule.Remediation{ Patch: rem.Patch, @@ -72,128 +64,28 @@ func probeRemToRuleRem(rem *probe.Remediation) *rule.Remediation { } } -func dependenciesToFindings(r *checker.PinningDependenciesData) ([]finding.Finding, error) { - findings := make([]finding.Finding, 0) - - for i := range r.ProcessingErrors { - e := r.ProcessingErrors[i] - f := finding.Finding{ - Message: generateTextIncompleteResults(e), - Location: &e.Location, - Outcome: finding.OutcomeNotAvailable, - } - findings = append(findings, f) - } - - for i := range r.Dependencies { - rr := r.Dependencies[i] - if rr.Location == nil { - if rr.Msg == nil { - e := sce.WithMessage(sce.ErrScorecardInternal, "empty File field") - return findings, e - } - f := &finding.Finding{ - Probe: "", - Outcome: finding.OutcomeNotApplicable, - Message: *rr.Msg, - } - findings = append(findings, *f) - 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 := &finding.Finding{ - Probe: "", - Outcome: finding.OutcomeNotApplicable, - Message: *rr.Msg, - Location: loc, - } - 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 := &finding.Finding{ - Probe: "", - Outcome: finding.OutcomeNotApplicable, - Message: fmt.Sprintf("%s has empty Pinned field", rr.Type), - Location: loc, - } - 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 := &finding.Finding{ - Probe: "", - Outcome: finding.OutcomeNegative, - Message: generateTextUnpinned(&rr), - Location: loc, - } - if rr.Remediation != nil { - f.Remediation = ruleRemToProbeRem(rr.Remediation) - } - f = f.WithValue(depTypeKey, string(rr.Type)) - 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 := &finding.Finding{ - Probe: "", - Outcome: finding.OutcomePositive, - Location: loc, - } - f = f.WithValue(depTypeKey, string(rr.Type)) - findings = append(findings, *f) - } - } - return findings, nil -} - // PinningDependencies applies the score policy for the Pinned-Dependencies check. -func PinningDependencies(name string, c *checker.CheckRequest, - r *checker.PinningDependenciesData, +func PinningDependencies(name string, + findings []finding.Finding, + dl checker.DetailLogger, ) checker.CheckResult { - if r == nil { - e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data") + expectedProbes := []string{ + pinsDependencies.Probe, + } + + if !finding.UniqueProbesEqual(findings, expectedProbes) { + e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") return checker.CreateRuntimeErrorResult(name, e) } var wp workflowPinningResult pr := make(map[checker.DependencyUseType]pinnedResult) - dl := c.Dlogger - - findings, err := dependenciesToFindings(r) - if err != nil { - return checker.CreateRuntimeErrorResult(name, err) - } for i := range findings { f := findings[i] switch f.Outcome { + case finding.OutcomeNotAvailable: + return checker.CreateInconclusiveResult(name, "no dependencies found") case finding.OutcomeNotApplicable: if f.Location != nil { dl.Debug(&checker.LogMessage{ @@ -224,7 +116,7 @@ func PinningDependencies(name string, c *checker.CheckRequest, lm.Remediation = probeRemToRuleRem(f.Remediation) } dl.Warn(lm) - case finding.OutcomeNotAvailable: + case finding.OutcomeError: dl.Info(&checker.LogMessage{ Finding: &f, }) @@ -289,21 +181,6 @@ func updatePinningResults(dependencyType checker.DependencyUseType, pr[dependencyType] = p } -func generateTextUnpinned(rr *checker.Dependency) string { - if rr.Type == checker.DependencyUseTypeGHAction { - // Check if we are dealing with a GitHub action or a third-party one. - gitHubOwned := fileparser.IsGitHubOwnedAction(rr.Location.Snippet) - owner := generateOwnerToDisplay(gitHubOwned) - return fmt.Sprintf("%s not pinned by hash", owner) - } - - return fmt.Sprintf("%s not pinned by hash", rr.Type) -} - -func generateTextIncompleteResults(e checker.ElementError) string { - return fmt.Sprintf("Possibly incomplete results: %s", e.Err) -} - func generateOwnerToDisplay(gitHubOwned bool) string { if gitHubOwned { return fmt.Sprintf("GitHub-owned %s", checker.DependencyUseTypeGHAction) diff --git a/checks/evaluation/pinned_dependencies_test.go b/checks/evaluation/pinned_dependencies_test.go index ef3fefb7edd..b65f0743454 100644 --- a/checks/evaluation/pinned_dependencies_test.go +++ b/checks/evaluation/pinned_dependencies_test.go @@ -20,11 +20,12 @@ import ( "github.com/google/go-cmp/cmp" "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" ) +var testLineEnd = uint(124) + func Test_createScoreForGitHubActionsWorkflow(t *testing.T) { t.Parallel() //nolint:govet @@ -228,602 +229,180 @@ func Test_createScoreForGitHubActionsWorkflow(t *testing.T) { } } -func asPointer(s string) *string { - return &s -} - -func asBoolPointer(b bool) *bool { - return &b -} - func Test_PinningDependencies(t *testing.T) { t.Parallel() tests := []struct { - name string - dependencies []checker.Dependency - processingErrors []checker.ElementError - expected scut.TestReturn + name string + findings []finding.Finding + result scut.TestReturn }{ { - name: "all dependencies pinned", - dependencies: []checker.Dependency{ + name: "pinned pip dependency scores 10 and shows no warn message", + findings: []finding.Finding{ { - Location: &checker.File{ - Snippet: "actions/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + Probe: "pinsDependencies", + Outcome: finding.OutcomePositive, + Location: &finding.Location{ + Type: finding.FileTypeText, + Path: "test-file", + LineStart: &testLineStart, + Snippet: &testSnippet, }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(true), - }, - { - Location: &checker.File{ - Snippet: "other/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + Values: map[string]int{ + "dependencyType": 6, // pip type }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(true), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeDockerfileContainerImage, - Pinned: asBoolPointer(true), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeDownloadThenRun, - Pinned: asBoolPointer(true), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeGoCommand, - Pinned: asBoolPointer(true), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeNpmCommand, - Pinned: asBoolPointer(true), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, - Pinned: asBoolPointer(true), }, }, - expected: scut.TestReturn{ - Error: nil, - Score: 10, - NumberOfWarn: 0, - NumberOfInfo: 7, - NumberOfDebug: 0, + result: scut.TestReturn{ + Score: 10, + NumberOfInfo: 1, }, }, { - name: "all dependencies unpinned", - dependencies: []checker.Dependency{ + name: "unpinned pip dependency scores 0 and shows warn message", + findings: []finding.Finding{ { - Location: &checker.File{ - Snippet: "actions/checkout@v2", + Probe: "pinsDependencies", + Outcome: finding.OutcomeNegative, + Location: &finding.Location{ + Type: finding.FileTypeText, + Path: "test-file", + LineStart: &testLineStart, + LineEnd: &testLineEnd, + Snippet: &testSnippet, }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{ - Snippet: "other/checkout@v2", + Values: map[string]int{ + "dependencyType": 6, // pip type }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeDockerfileContainerImage, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeDownloadThenRun, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeGoCommand, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeNpmCommand, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, - Pinned: asBoolPointer(false), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 7, - NumberOfInfo: 7, - NumberOfDebug: 0, - }, - }, - { - name: "1 ecosystem pinned and 1 ecosystem unpinned", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeGoCommand, - Pinned: asBoolPointer(true), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 5, - NumberOfWarn: 1, - NumberOfInfo: 2, - NumberOfDebug: 0, - }, - }, - { - name: "1 ecosystem partially pinned", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, - Pinned: asBoolPointer(true), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 5, - NumberOfWarn: 1, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "no dependencies found", - dependencies: []checker.Dependency{}, - expected: scut.TestReturn{ - Error: nil, - Score: -1, - NumberOfWarn: 0, - NumberOfInfo: 0, - NumberOfDebug: 0, - }, - }, - { - name: "pinned dependency shows no warn message", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, - Pinned: asBoolPointer(true), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 10, - NumberOfWarn: 0, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "unpinned dependency shows warn message", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, - Pinned: asBoolPointer(false), }, }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 1, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "dependency with parsing error does not count for score and shows debug message", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Msg: asPointer("some message"), - Type: checker.DependencyUseTypePipCommand, - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: -1, - NumberOfWarn: 0, - NumberOfInfo: 0, - NumberOfDebug: 1, + result: scut.TestReturn{ + Score: 0, + NumberOfInfo: 1, + NumberOfWarn: 1, }, }, { name: "dependency missing Pinned info does not count for score and shows debug message", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, + findings: []finding.Finding{ + { + Probe: "pinsDependencies", + Outcome: finding.OutcomeNotApplicable, + Location: &finding.Location{ + Type: finding.FileTypeText, + Path: "test-file", + LineStart: &testLineStart, + LineEnd: &testLineEnd, + Snippet: &testSnippet, + }, + Values: map[string]int{ + "dependencyType": 6, // pip type + }, }, }, - expected: scut.TestReturn{ - Error: nil, - Score: -1, - NumberOfWarn: 0, - NumberOfInfo: 0, - NumberOfDebug: 1, - }, - }, - { - name: "dependency missing Location info and no error message throws error", - dependencies: []checker.Dependency{{}}, - expected: scut.TestReturn{ - Error: sce.ErrScorecardInternal, + result: scut.TestReturn{ Score: -1, - NumberOfWarn: 0, - NumberOfInfo: 0, - NumberOfDebug: 0, - }, - }, - { - name: "dependency missing Location info with error message shows debug message", - dependencies: []checker.Dependency{{ - Msg: asPointer("some message"), - }}, - expected: scut.TestReturn{ - Error: nil, - Score: -1, - NumberOfWarn: 0, - NumberOfInfo: 0, NumberOfDebug: 1, }, }, - { - name: "unpinned choco install", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeChocoCommand, - Pinned: asBoolPointer(false), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 1, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "unpinned Dockerfile container image", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeDockerfileContainerImage, - Pinned: asBoolPointer(false), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 1, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "unpinned download then run", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeDownloadThenRun, - Pinned: asBoolPointer(false), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 1, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "unpinned go install", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeGoCommand, - Pinned: asBoolPointer(false), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 1, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "unpinned npm install", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeNpmCommand, - Pinned: asBoolPointer(false), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 1, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "unpinned nuget install", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeNugetCommand, - Pinned: asBoolPointer(false), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 1, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "unpinned pip install", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, - Pinned: asBoolPointer(false), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 1, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, { name: "2 unpinned dependencies for 1 ecosystem shows 2 warn messages", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, - Pinned: asBoolPointer(false), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 2, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "2 unpinned dependencies for 2 ecosystems shows 2 warn messages", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypePipCommand, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeGoCommand, - Pinned: asBoolPointer(false), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 2, - NumberOfInfo: 2, - NumberOfDebug: 0, - }, - }, - { - name: "GitHub Actions ecosystem with GitHub-owned pinned", - dependencies: []checker.Dependency{ - { - Location: &checker.File{ - Snippet: "actions/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + findings: []finding.Finding{ + { + Probe: "pinsDependencies", + Outcome: finding.OutcomeNegative, + Location: &finding.Location{ + Type: finding.FileTypeText, + Path: "test-file", + LineStart: &testLineStart, + LineEnd: &testLineEnd, + Snippet: &testSnippet, }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(true), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 10, - NumberOfWarn: 0, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "GitHub Actions ecosystem with third-party pinned", - dependencies: []checker.Dependency{ - { - Location: &checker.File{ - Snippet: "other/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + Values: map[string]int{ + "dependencyType": 6, // pip type }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(true), }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 10, - NumberOfWarn: 0, - NumberOfInfo: 1, - NumberOfDebug: 0, - }, - }, - { - name: "GitHub Actions ecosystem with GitHub-owned and third-party pinned", - dependencies: []checker.Dependency{ { - Location: &checker.File{ - Snippet: "actions/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + Probe: "pinsDependencies", + Outcome: finding.OutcomeNegative, + Location: &finding.Location{ + Type: finding.FileTypeText, + Path: "test-file", + LineStart: &testLineStart, + LineEnd: &testLineEnd, + Snippet: &testSnippet, }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(true), - }, - { - Location: &checker.File{ - Snippet: "other/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + Values: map[string]int{ + "dependencyType": 6, // pip type }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(true), }, }, - expected: scut.TestReturn{ - Error: nil, - Score: 10, - NumberOfWarn: 0, - NumberOfInfo: 2, - NumberOfDebug: 0, + result: scut.TestReturn{ + Score: 0, + NumberOfWarn: 2, + NumberOfInfo: 1, }, }, { - name: "GitHub Actions ecosystem with GitHub-owned and third-party unpinned", - dependencies: []checker.Dependency{ - { - Location: &checker.File{ - Snippet: "actions/checkout@v2", + name: "2 unpinned dependencies for 1 ecosystem shows 2 warn messages", + findings: []finding.Finding{ + { + Probe: "pinsDependencies", + Outcome: finding.OutcomeNegative, + Location: &finding.Location{ + Type: finding.FileTypeText, + Path: "test-file", + LineStart: &testLineStart, + LineEnd: &testLineEnd, + Snippet: &testSnippet, }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{ - Snippet: "other/checkout@v2", + Values: map[string]int{ + "dependencyType": 6, // pip type }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(false), }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 2, - NumberOfInfo: 2, - NumberOfDebug: 0, - }, - }, - { - name: "GitHub Actions ecosystem with GitHub-owned pinned and third-party unpinned", - dependencies: []checker.Dependency{ { - Location: &checker.File{ - Snippet: "actions/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + Probe: "pinsDependencies", + Outcome: finding.OutcomeNegative, + Location: &finding.Location{ + Type: finding.FileTypeText, + Path: "test-file", + LineStart: &testLineStart, + LineEnd: &testLineEnd, + Snippet: &testSnippet, }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(true), - }, - { - Location: &checker.File{ - Snippet: "other/checkout@v2", + Values: map[string]int{ + "dependencyType": 3, // go type }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(false), }, }, - expected: scut.TestReturn{ - Error: nil, - Score: 2, - NumberOfWarn: 1, - NumberOfInfo: 2, - NumberOfDebug: 0, + result: scut.TestReturn{ + Score: 0, + NumberOfWarn: 2, + NumberOfInfo: 2, }, }, { - name: "GitHub Actions ecosystem with GitHub-owned unpinned and third-party pinned", - dependencies: []checker.Dependency{ - { - Location: &checker.File{ - Snippet: "actions/checkout@v2", + name: "GitHub Actions ecosystem with GitHub-owned pinned", + findings: []finding.Finding{ + { + Probe: "pinsDependencies", + Outcome: finding.OutcomePositive, + Location: &finding.Location{ + Type: finding.FileTypeText, + Path: "test-file", + LineStart: &testLineStart, + LineEnd: &testLineEnd, + Snippet: &testSnippet, }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{ - Snippet: "other/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + Values: map[string]int{ + "dependencyType": 0, // GH Action type }, - Type: checker.DependencyUseTypeGHAction, - Pinned: asBoolPointer(true), - }, - }, - expected: scut.TestReturn{ - Error: nil, - Score: 8, - NumberOfWarn: 1, - NumberOfInfo: 2, - NumberOfDebug: 0, - }, - }, - { - name: "Skipped objects and dependencies", - dependencies: []checker.Dependency{ - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeNpmCommand, - Pinned: asBoolPointer(false), - }, - { - Location: &checker.File{}, - Type: checker.DependencyUseTypeNpmCommand, - Pinned: asBoolPointer(false), - }, - }, - processingErrors: []checker.ElementError{ - { - Err: sce.ErrJobOSParsing, - Location: finding.Location{}, }, }, - expected: scut.TestReturn{ - Error: nil, - Score: 0, - NumberOfWarn: 2, // unpinned deps - NumberOfInfo: 2, // 1 for npm deps, 1 for processing error - NumberOfDebug: 0, + result: scut.TestReturn{ + Score: 10, + NumberOfInfo: 1, }, }, } @@ -832,15 +411,8 @@ func Test_PinningDependencies(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - dl := scut.TestDetailLogger{} - c := checker.CheckRequest{Dlogger: &dl} - actual := PinningDependencies("checkname", &c, - &checker.PinningDependenciesData{ - Dependencies: tt.dependencies, - ProcessingErrors: tt.processingErrors, - }) - + got := PinningDependencies(tt.name, tt.findings, &dl) scut.ValidateTestReturn(t, tt.name, &tt.expected, &actual, &dl) }) } @@ -850,35 +422,6 @@ func stringAsPointer(s string) *string { return &s } -func Test_generateOwnerToDisplay(t *testing.T) { - t.Parallel() - tests := []struct { //nolint:govet - name string - gitHubOwned bool - want string - }{ - { - name: "returns GitHub if gitHubOwned is true", - gitHubOwned: true, - want: "GitHub-owned GitHubAction", - }, - { - name: "returns GitHub if gitHubOwned is false", - gitHubOwned: false, - want: "third-party GitHubAction", - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - if got := generateOwnerToDisplay(tt.gitHubOwned); got != tt.want { - t.Errorf("generateOwnerToDisplay() = %v, want %v", got, tt.want) - } - }) - } -} - func Test_addWorkflowPinnedResult(t *testing.T) { t.Parallel() type args struct { @@ -985,47 +528,6 @@ func Test_addWorkflowPinnedResult(t *testing.T) { } } -func TestGenerateText(t *testing.T) { - t.Parallel() - tests := []struct { - name string - dependency *checker.Dependency - expectedText string - }{ - { - name: "GitHub action not pinned by hash", - dependency: &checker.Dependency{ - Type: checker.DependencyUseTypeGHAction, - Location: &checker.File{ - Snippet: "actions/checkout@v2", - }, - }, - expectedText: "GitHub-owned GitHubAction not pinned by hash", - }, - { - name: "Third-party action not pinned by hash", - dependency: &checker.Dependency{ - Type: checker.DependencyUseTypeGHAction, - Location: &checker.File{ - Snippet: "third-party/action@v1", - }, - }, - expectedText: "third-party GitHubAction not pinned by hash", - }, - } - - for _, tc := range tests { - tc := tc - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - result := generateTextUnpinned(tc.dependency) - if !cmp.Equal(tc.expectedText, result) { - t.Errorf("generateText mismatch (-want +got):\n%s", cmp.Diff(tc.expectedText, result)) - } - }) - } -} - func TestUpdatePinningResults(t *testing.T) { t.Parallel() type args struct { diff --git a/checks/pinned_dependencies.go b/checks/pinned_dependencies.go index 3a0cbe9170e..04f8af51312 100644 --- a/checks/pinned_dependencies.go +++ b/checks/pinned_dependencies.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" ) // CheckPinnedDependencies is the registered name for FrozenDeps. @@ -45,9 +47,16 @@ func PinningDependencies(c *checker.CheckRequest) checker.CheckResult { } // Set the raw results. - if c.RawResults != nil { - c.RawResults.PinningDependenciesResults = rawData + pRawResults := getRawResults(c) + pRawResults.PinningDependenciesResults = rawData + + // Evaluate the probes. + findings, err := zrunner.Run(pRawResults, probes.PinnedDependencies) + if err != nil { + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return checker.CreateRuntimeErrorResult(CheckPinnedDependencies, e) } - return evaluation.PinningDependencies(CheckPinnedDependencies, c, &rawData) + // Return the score evaluation. + return evaluation.PinningDependencies(CheckPinnedDependencies, findings, c.Dlogger) } diff --git a/probes/entries.go b/probes/entries.go index 7e7aabfcf5e..62c0dc2d3cb 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -47,6 +47,7 @@ import ( "github.com/ossf/scorecard/v4/probes/notArchived" "github.com/ossf/scorecard/v4/probes/notCreatedRecently" "github.com/ossf/scorecard/v4/probes/packagedWithAutomatedWorkflow" + "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/sastToolConfigured" @@ -146,6 +147,9 @@ var ( releasesAreSigned.Run, releasesHaveProvenance.Run, } + PinnedDependencies = []ProbeImpl{ + pinsDependencies.Run, + } probeRunners = map[string]func(*checker.RawResults) ([]finding.Finding, string, error){ securityPolicyPresent.Probe: securityPolicyPresent.Run, diff --git a/probes/pinsDependencies/def.yml b/probes/pinsDependencies/def.yml new file mode 100644 index 00000000000..38266db942b --- /dev/null +++ b/probes/pinsDependencies/def.yml @@ -0,0 +1,28 @@ +# 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: pinsDependencies +short: Check that the project pins dependencies to a specific digest. +motivation: > + Pinned dependencies ensure that checking and deployment are all done with the same software, reducing deployment risks, simplifying debugging, and enabling reproducibility. They can help mitigate compromised dependencies from undermining the security of the project (in the case where you've evaluated the pinned dependency, you are confident it's not compromised, and a later version is released that is compromised). +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. +remediation: + effort: Medium + text: + - Pin dependencies by hash. diff --git a/probes/pinsDependencies/impl.go b/probes/pinsDependencies/impl.go new file mode 100644 index 00000000000..8a2b025416d --- /dev/null +++ b/probes/pinsDependencies/impl.go @@ -0,0 +1,201 @@ +// 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 pinsDependencies + +import ( + "embed" + "fmt" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/checks/fileparser" + sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/finding/probe" + "github.com/ossf/scorecard/v4/probes/internal/utils/uerror" + "github.com/ossf/scorecard/v4/rule" +) + +//go:embed *.yml +var fs embed.FS + +const ( + Probe = "pinsDependencies" + depTypeKey = "dependencyType" +) + +var dependencyTypes = map[checker.DependencyUseType]int{ + checker.DependencyUseTypeGHAction: 0, + checker.DependencyUseTypeDockerfileContainerImage: 1, + checker.DependencyUseTypeDownloadThenRun: 2, + checker.DependencyUseTypeGoCommand: 3, + checker.DependencyUseTypeChocoCommand: 4, + checker.DependencyUseTypeNpmCommand: 5, + checker.DependencyUseTypePipCommand: 6, + checker.DependencyUseTypeNugetCommand: 7, +} + +func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { + if raw == nil { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + + var findings []finding.Finding + + r := raw.PinningDependenciesResults + + for i := range r.ProcessingErrors { + e := r.ProcessingErrors[i] + f := finding.Finding{ + Message: generateTextIncompleteResults(e), + Location: &e.Location, + Outcome: finding.OutcomeError, + } + findings = append(findings, f) + } + + for i := range r.Dependencies { + rr := r.Dependencies[i] + if rr.Location == nil { + if rr.Msg == nil { + e := sce.WithMessage(sce.ErrScorecardInternal, "empty File field") + return findings, Probe, e + } + f := &finding.Finding{ + Probe: "", + Outcome: finding.OutcomeNotApplicable, + Message: *rr.Msg, + } + findings = append(findings, *f) + 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 := &finding.Finding{ + Probe: "", + Outcome: finding.OutcomeNotApplicable, + Message: *rr.Msg, + Location: loc, + } + 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 := &finding.Finding{ + Probe: "", + Outcome: finding.OutcomeNotApplicable, + Message: fmt.Sprintf("%s has empty Pinned field", rr.Type), + Location: loc, + } + 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 := &finding.Finding{ + Probe: "", + Outcome: finding.OutcomeNegative, + Message: generateTextUnpinned(&rr), + Location: loc, + } + if rr.Remediation != nil { + f.Remediation = ruleRemToProbeRem(rr.Remediation) + } + f = f.WithValues(map[string]int{ + depTypeKey: dependencyTypes[rr.Type], + }) + 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 := &finding.Finding{ + Probe: "", + Outcome: finding.OutcomePositive, + Location: loc, + } + f = f.WithValues(map[string]int{ + depTypeKey: dependencyTypes[rr.Type], + }) + findings = append(findings, *f) + } + } + + if len(findings) == 0 { + f, err := finding.NewWith(fs, Probe, + "no dependencies found", nil, + finding.OutcomeNotAvailable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + return []finding.Finding{*f}, Probe, nil + } + + return findings, Probe, nil +} + +func generateTextIncompleteResults(e checker.ElementError) string { + return fmt.Sprintf("Possibly incomplete results: %s", e.Err) +} + +func ruleRemToProbeRem(rem *rule.Remediation) *probe.Remediation { + return &probe.Remediation{ + Patch: rem.Patch, + Text: rem.Text, + Markdown: rem.Markdown, + Effort: probe.RemediationEffort(rem.Effort), + } +} + +func generateTextUnpinned(rr *checker.Dependency) string { + if rr.Type == checker.DependencyUseTypeGHAction { + // Check if we are dealing with a GitHub action or a third-party one. + gitHubOwned := fileparser.IsGitHubOwnedAction(rr.Location.Snippet) + owner := generateOwnerToDisplay(gitHubOwned) + return fmt.Sprintf("%s not pinned by hash", owner) + } + + return fmt.Sprintf("%s not pinned by hash", rr.Type) +} + +func generateOwnerToDisplay(gitHubOwned bool) string { + if gitHubOwned { + return fmt.Sprintf("GitHub-owned %s", checker.DependencyUseTypeGHAction) + } + return fmt.Sprintf("third-party %s", checker.DependencyUseTypeGHAction) +} diff --git a/probes/pinsDependencies/impl_test.go b/probes/pinsDependencies/impl_test.go new file mode 100644 index 00000000000..afbaffdad3d --- /dev/null +++ b/probes/pinsDependencies/impl_test.go @@ -0,0 +1,588 @@ +// 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 pinsDependencies + +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" + "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: "All dependencies pinned", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{ + Snippet: "actions/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(true), + }, + { + Location: &checker.File{ + Snippet: "other/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(true), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeDockerfileContainerImage, + Pinned: asBoolPointer(true), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeDownloadThenRun, + Pinned: asBoolPointer(true), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeGoCommand, + Pinned: asBoolPointer(true), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeNpmCommand, + Pinned: asBoolPointer(true), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypePipCommand, + Pinned: asBoolPointer(true), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + }, + }, + { + name: "All dependencies unpinned", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{ + Snippet: "actions/checkout@v2", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(false), + }, + { + Location: &checker.File{ + Snippet: "other/checkout@v2", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(false), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeDockerfileContainerImage, + Pinned: asBoolPointer(false), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeDownloadThenRun, + Pinned: asBoolPointer(false), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeGoCommand, + Pinned: asBoolPointer(false), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeNpmCommand, + Pinned: asBoolPointer(false), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypePipCommand, + Pinned: asBoolPointer(false), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + }, + }, + { + name: "1 ecosystem pinned and 1 ecosystem unpinned", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Type: checker.DependencyUseTypePipCommand, + Pinned: asBoolPointer(false), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeGoCommand, + Pinned: asBoolPointer(true), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + finding.OutcomePositive, + }, + }, + { + name: "1 ecosystem partially pinned", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Type: checker.DependencyUseTypePipCommand, + Pinned: asBoolPointer(false), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypePipCommand, + Pinned: asBoolPointer(true), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + finding.OutcomePositive, + }, + }, + { + name: "no dependencies found", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{}, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNotAvailable, + }, + }, + { + name: "unpinned choco install", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeChocoCommand, + Pinned: asBoolPointer(false), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + { + name: "unpinned Dockerfile container image", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeDockerfileContainerImage, + Pinned: asBoolPointer(false), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + { + name: "unpinned download then run", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeDownloadThenRun, + Pinned: asBoolPointer(false), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + { + name: "unpinned go install", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeGoCommand, + Pinned: asBoolPointer(false), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + { + name: "unpinned npm install", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeNpmCommand, + Pinned: asBoolPointer(false), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + { + name: "unpinned nuget install", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeNugetCommand, + Pinned: asBoolPointer(false), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + { + name: "unpinned pip install", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Type: checker.DependencyUseTypePipCommand, + Pinned: asBoolPointer(false), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + { + name: "GitHub Actions ecosystem with third-party pinned", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{ + Snippet: "other/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(true), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + }, + { + name: "GitHub Actions ecosystem with GitHub-owned and third-party pinned", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{ + Snippet: "actions/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(true), + }, + { + Location: &checker.File{ + Snippet: "other/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(true), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + finding.OutcomePositive, + }, + }, + { + name: "GitHub Actions ecosystem with GitHub-owned and third-party pinned", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{ + Snippet: "actions/checkout@v2", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(false), + }, + { + Location: &checker.File{ + Snippet: "other/checkout@v2", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(false), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + finding.OutcomeNegative, + }, + }, + { + name: "GitHub Actions ecosystem with GitHub-owned pinned and third-party unpinned", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{ + Snippet: "actions/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(true), + }, + { + Location: &checker.File{ + Snippet: "other/checkout@v2", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(false), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + finding.OutcomeNegative, + }, + }, + { + name: "GitHub Actions ecosystem with GitHub-owned unpinned and third-party pinned", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{ + Snippet: "actions/checkout@v2", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(false), + }, + { + Location: &checker.File{ + Snippet: "other/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675", + }, + Type: checker.DependencyUseTypeGHAction, + Pinned: asBoolPointer(true), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + finding.OutcomePositive, + }, + }, + { + name: "Skipped objects and dependencies", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeNpmCommand, + Pinned: asBoolPointer(false), + }, + { + Location: &checker.File{}, + Type: checker.DependencyUseTypeNpmCommand, + Pinned: asBoolPointer(false), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + finding.OutcomeNegative, + }, + }, + { + name: "dependency missing Location info and no error message throws error", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: nil, + Msg: nil, + Type: checker.DependencyUseTypeNpmCommand, + Pinned: asBoolPointer(true), + }, + }, + }, + }, + err: sce.ErrScorecardInternal, + }, + } + 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) + }) + } +} + +func asBoolPointer(b bool) *bool { + return &b +} + +func Test_generateOwnerToDisplay(t *testing.T) { + t.Parallel() + tests := []struct { //nolint:govet + name string + gitHubOwned bool + want string + }{ + { + name: "returns GitHub if gitHubOwned is true", + gitHubOwned: true, + want: "GitHub-owned GitHubAction", + }, + { + name: "returns GitHub if gitHubOwned is false", + gitHubOwned: false, + want: "third-party GitHubAction", + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := generateOwnerToDisplay(tt.gitHubOwned); got != tt.want { + t.Errorf("generateOwnerToDisplay() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestGenerateText(t *testing.T) { + t.Parallel() + tests := []struct { + name string + dependency *checker.Dependency + expectedText string + }{ + { + name: "GitHub action not pinned by hash", + dependency: &checker.Dependency{ + Type: checker.DependencyUseTypeGHAction, + Location: &checker.File{ + Snippet: "actions/checkout@v2", + }, + }, + expectedText: "GitHub-owned GitHubAction not pinned by hash", + }, + { + name: "Third-party action not pinned by hash", + dependency: &checker.Dependency{ + Type: checker.DependencyUseTypeGHAction, + Location: &checker.File{ + Snippet: "third-party/action@v1", + }, + }, + expectedText: "third-party GitHubAction not pinned by hash", + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + result := generateTextUnpinned(tc.dependency) + if !cmp.Equal(tc.expectedText, result) { + t.Errorf("generateText mismatch (-want +got):\n%s", cmp.Diff(tc.expectedText, result)) + } + }) + } +} From 5474687812198816346663c86e1f724596249042 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Tue, 30 Jan 2024 15:09:55 +0000 Subject: [PATCH 02/13] add more tests Signed-off-by: Adam Korczynski --- probes/pinsDependencies/impl_test.go | 81 ++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/probes/pinsDependencies/impl_test.go b/probes/pinsDependencies/impl_test.go index afbaffdad3d..98490925e5b 100644 --- a/probes/pinsDependencies/impl_test.go +++ b/probes/pinsDependencies/impl_test.go @@ -28,6 +28,8 @@ import ( ) func Test_Run(t *testing.T) { + jobName := "jobName" + msg := "msg" t.Parallel() //nolint:govet tests := []struct { @@ -492,6 +494,85 @@ func Test_Run(t *testing.T) { }, err: sce.ErrScorecardInternal, }, + { + name: "dependency missing Location info", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: nil, + Msg: &msg, + Type: checker.DependencyUseTypeNpmCommand, + Pinned: asBoolPointer(true), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNotApplicable, + }, + }, + { + name: "neither location nor msg is nil", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Msg: &msg, + Type: checker.DependencyUseTypeNpmCommand, + Pinned: asBoolPointer(true), + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNotApplicable, + }, + }, + { + name: "pinned = nil", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Location: &checker.File{}, + Msg: nil, + Type: checker.DependencyUseTypeNpmCommand, + Pinned: nil, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNotApplicable, + }, + }, + { + name: "2 processing errors", + raw: &checker.RawResults{ + PinningDependenciesResults: checker.PinningDependenciesData{ + ProcessingErrors: []checker.ElementError{ + { + Location: finding.Location{ + Snippet: &jobName, + }, + Err: sce.ErrJobOSParsing, + }, + { + Location: finding.Location{ + Snippet: &jobName, + }, + Err: sce.ErrJobOSParsing, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeError, + finding.OutcomeError, + }, + }, } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below From fb45482e6523d66b220c5d7b8dc06a0638e0adc7 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Tue, 30 Jan 2024 16:31:47 +0000 Subject: [PATCH 03/13] add checks unit test Signed-off-by: Adam Korczynski --- checks/pinned_dependencies_test.go | 85 +++++++++++++++++++ .../pinneddependencies/Dockerfile-script-ok | 71 ++++++++++++++++ probes/pinsDependencies/impl.go | 10 +-- 3 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 checks/pinned_dependencies_test.go create mode 100644 checks/testdata/pinneddependencies/Dockerfile-script-ok diff --git a/checks/pinned_dependencies_test.go b/checks/pinned_dependencies_test.go new file mode 100644 index 00000000000..a1374f4054c --- /dev/null +++ b/checks/pinned_dependencies_test.go @@ -0,0 +1,85 @@ +// Copyright 2024 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. + +package checks + +import ( + "fmt" + "os" + "testing" + + "github.com/golang/mock/gomock" + + "github.com/ossf/scorecard/v4/checker" + mockrepo "github.com/ossf/scorecard/v4/clients/mockclients" + scut "github.com/ossf/scorecard/v4/utests" +) + +func TestPinningDependencies(t *testing.T) { + t.Parallel() + tests := []struct { + name string + path string + files []string + want scut.TestReturn + wantErr bool + }{ + { + name: "Dockerfile", + path: "./testdata/pinneddependencies/Dockerfile-script-ok", + files: []string{ + "Dockerfile-script-ok", + }, + want: scut.TestReturn{ + Score: 10, + NumberOfInfo: 1, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockRepo := mockrepo.NewMockRepoClient(ctrl) + mockRepo.EXPECT().GetDefaultBranchName().Return("main", nil).AnyTimes() + 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) { + if tt.path == "" { + return nil, nil + } + content, err := os.ReadFile(tt.path) + if err != nil { + return content, fmt.Errorf("%w", err) + } + return content, nil + }).AnyTimes() + + dl := scut.TestDetailLogger{} + c := &checker.CheckRequest{ + RepoClient: mockRepo, + Dlogger: &dl, + } + + res := PinningDependencies(c) + + if !scut.ValidateTestReturn(t, tt.name, &tt.want, &res, &dl) { + t.Errorf("test failed: log message not present: %+v on %+v", tt.want, res) + } + }) + } +} diff --git a/checks/testdata/pinneddependencies/Dockerfile-script-ok b/checks/testdata/pinneddependencies/Dockerfile-script-ok new file mode 100644 index 00000000000..1d493959526 --- /dev/null +++ b/checks/testdata/pinneddependencies/Dockerfile-script-ok @@ -0,0 +1,71 @@ +# Copyright 2021 Security 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. + +FROM python:3.7@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2 + +# 如果在中国，apt使用163源, ifconfig.co/json, http://ip-api.com + +RUN wget program +RUN curl | echo + +# aws +RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script . + +RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script /tmp/file1 && bash /tmp/fileXXX +RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script /tmp/file2 ; bash /tmp/file1234 + +RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script . && bash scriptxxx +RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script /path/to/ && bash /path/to/scriptxxx +RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script2 /path/to/ +RUN bash /path/to/script2xxx + +# curl +RUN curl http://file 2>&1 > /tmp/file1 && sh /tmp/filex +RUN curl http://file2 2>&1 > /tmp/file2 ; sh /tmp/filex +RUN curl http://file2 2>&1 > /tmp/file2 ; sh /tmp/filex +RUN curl http://file2 2>&1 > /tmp/file4 ; \ + bash /tmp/file5 + +RUN echo hello && curl -s http://etc/file | echo +RUN echo hello && curl -s http://file-with-sudo2 | sudo echo + +# gsutil +RUN gsutil gs://file /tmp/file +RUN bash /tmp/filezx + +RUN gsutil gs://file /tmp/file1 && bash /tmp/fileqw +RUN gsutil gs://file /tmp/file2 ; bash /tmp/file122 + +RUN gsutil gs://bucket/file . && bash ./file2222 +RUN gsutil gs://bucet/file /path/to/ && bash /path/to/fileqqq +RUN gsutil gs://bucet/file2 /path/to/ && bash /path/to/file2234 +RUN bash /path/to/fileshsj + +RUN bash somescript.sh + +RUN sudo su -c "bash blabl.sh" root + +RUN echo hello && sudo curl -s file-with-sudo | echo | bla + +RUN echo hello && wget -0 - ifconfig.co/json | echo + +RUN wget http://file -O /tmp/file +RUN bash /tmp/filegshek + +RUN wget http://file -O /tmp/file1 && bash /tmp/file1xxxx +RUN wget http://file -O /tmp/file2 ; bash /tmp/file2xxxxx +RUN wget http://domain.com/file . && bash ./fileccc + +FROM scratch +FROM python@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2 \ No newline at end of file diff --git a/probes/pinsDependencies/impl.go b/probes/pinsDependencies/impl.go index 8a2b025416d..ec42a582024 100644 --- a/probes/pinsDependencies/impl.go +++ b/probes/pinsDependencies/impl.go @@ -74,7 +74,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { return findings, Probe, e } f := &finding.Finding{ - Probe: "", + Probe: Probe, Outcome: finding.OutcomeNotApplicable, Message: *rr.Msg, } @@ -90,7 +90,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { Snippet: &rr.Location.Snippet, } f := &finding.Finding{ - Probe: "", + Probe: Probe, Outcome: finding.OutcomeNotApplicable, Message: *rr.Msg, Location: loc, @@ -107,7 +107,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { Snippet: &rr.Location.Snippet, } f := &finding.Finding{ - Probe: "", + Probe: Probe, Outcome: finding.OutcomeNotApplicable, Message: fmt.Sprintf("%s has empty Pinned field", rr.Type), Location: loc, @@ -124,7 +124,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { Snippet: &rr.Location.Snippet, } f := &finding.Finding{ - Probe: "", + Probe: Probe, Outcome: finding.OutcomeNegative, Message: generateTextUnpinned(&rr), Location: loc, @@ -145,7 +145,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { Snippet: &rr.Location.Snippet, } f := &finding.Finding{ - Probe: "", + Probe: Probe, Outcome: finding.OutcomePositive, Location: loc, } From 5a3f503198277776f82aa8b8dc142de7493f0da1 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Tue, 30 Jan 2024 16:33:03 +0000 Subject: [PATCH 04/13] fix year in probe header and add mising test file Signed-off-by: Adam Korczynski --- checks/pinned_dependencies_test.go | 2 +- probes/pinsDependencies/def.yml | 2 +- probes/pinsDependencies/impl.go | 2 +- probes/pinsDependencies/impl_test.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/checks/pinned_dependencies_test.go b/checks/pinned_dependencies_test.go index a1374f4054c..63fef636760 100644 --- a/checks/pinned_dependencies_test.go +++ b/checks/pinned_dependencies_test.go @@ -55,7 +55,7 @@ func TestPinningDependencies(t *testing.T) { ctrl := gomock.NewController(t) mockRepo := mockrepo.NewMockRepoClient(ctrl) mockRepo.EXPECT().GetDefaultBranchName().Return("main", nil).AnyTimes() - mockRepo.EXPECT().URI().Return("github.com/ossf/scorecard").AnyTimes() + 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) { diff --git a/probes/pinsDependencies/def.yml b/probes/pinsDependencies/def.yml index 38266db942b..42d410a7880 100644 --- a/probes/pinsDependencies/def.yml +++ b/probes/pinsDependencies/def.yml @@ -1,4 +1,4 @@ -# Copyright 2023 OpenSSF Scorecard Authors +# Copyright 2024 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. diff --git a/probes/pinsDependencies/impl.go b/probes/pinsDependencies/impl.go index ec42a582024..bf5ffda1bfc 100644 --- a/probes/pinsDependencies/impl.go +++ b/probes/pinsDependencies/impl.go @@ -1,4 +1,4 @@ -// Copyright 2023 OpenSSF Scorecard Authors +// Copyright 2024 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. diff --git a/probes/pinsDependencies/impl_test.go b/probes/pinsDependencies/impl_test.go index 98490925e5b..19d777e1f14 100644 --- a/probes/pinsDependencies/impl_test.go +++ b/probes/pinsDependencies/impl_test.go @@ -1,4 +1,4 @@ -// Copyright 2023 OpenSSF Scorecard Authors +// Copyright 2024 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. From 012359e7f2a38f28984c177d072a386b3385b77b Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Tue, 6 Feb 2024 14:24:16 +0000 Subject: [PATCH 05/13] Change usage of ValidateTestReturn Signed-off-by: Adam Korczynski --- checks/evaluation/pinned_dependencies_test.go | 2 +- checks/pinned_dependencies_test.go | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/checks/evaluation/pinned_dependencies_test.go b/checks/evaluation/pinned_dependencies_test.go index b65f0743454..24d712bcc32 100644 --- a/checks/evaluation/pinned_dependencies_test.go +++ b/checks/evaluation/pinned_dependencies_test.go @@ -413,7 +413,7 @@ func Test_PinningDependencies(t *testing.T) { t.Parallel() dl := scut.TestDetailLogger{} got := PinningDependencies(tt.name, tt.findings, &dl) - scut.ValidateTestReturn(t, tt.name, &tt.expected, &actual, &dl) + scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) }) } } diff --git a/checks/pinned_dependencies_test.go b/checks/pinned_dependencies_test.go index 63fef636760..ec7acfb175f 100644 --- a/checks/pinned_dependencies_test.go +++ b/checks/pinned_dependencies_test.go @@ -76,10 +76,7 @@ func TestPinningDependencies(t *testing.T) { } res := PinningDependencies(c) - - if !scut.ValidateTestReturn(t, tt.name, &tt.want, &res, &dl) { - t.Errorf("test failed: log message not present: %+v on %+v", tt.want, res) - } + scut.ValidateTestReturn(t, tt.name, &tt.want, &res, &dl) }) } } From b0f0b09ed56939544029351de3f634103009f686 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Tue, 6 Feb 2024 14:25:32 +0000 Subject: [PATCH 06/13] rename test Signed-off-by: Adam Korczynski --- probes/pinsDependencies/impl_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probes/pinsDependencies/impl_test.go b/probes/pinsDependencies/impl_test.go index 19d777e1f14..4dd0b3df68a 100644 --- a/probes/pinsDependencies/impl_test.go +++ b/probes/pinsDependencies/impl_test.go @@ -549,7 +549,7 @@ func Test_Run(t *testing.T) { }, }, { - name: "2 processing errors", + name: "processing errors result in OutcomeError", raw: &checker.RawResults{ PinningDependenciesResults: checker.PinningDependenciesData{ ProcessingErrors: []checker.ElementError{ From 203dada0986b37bee0c702b584aa030842f26360 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Tue, 6 Feb 2024 14:26:31 +0000 Subject: [PATCH 07/13] change 'pinned' to 'unpinned' in test name Signed-off-by: Adam Korczynski --- probes/pinsDependencies/impl_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probes/pinsDependencies/impl_test.go b/probes/pinsDependencies/impl_test.go index 4dd0b3df68a..c0ae087def2 100644 --- a/probes/pinsDependencies/impl_test.go +++ b/probes/pinsDependencies/impl_test.go @@ -375,7 +375,7 @@ func Test_Run(t *testing.T) { }, }, { - name: "GitHub Actions ecosystem with GitHub-owned and third-party pinned", + name: "GitHub Actions ecosystem with GitHub-owned and third-party unpinned", raw: &checker.RawResults{ PinningDependenciesResults: checker.PinningDependenciesData{ Dependencies: []checker.Dependency{ From de6b3811b41e314c3a2eb478b17a668aadadfd26 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Tue, 6 Feb 2024 14:37:04 +0000 Subject: [PATCH 08/13] export 'depTypeKey' Signed-off-by: Adam Korczynski --- checks/evaluation/pinned_dependencies.go | 5 +---- probes/pinsDependencies/impl.go | 6 +++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/checks/evaluation/pinned_dependencies.go b/checks/evaluation/pinned_dependencies.go index 087bdea0791..f1526c5007c 100644 --- a/checks/evaluation/pinned_dependencies.go +++ b/checks/evaluation/pinned_dependencies.go @@ -50,9 +50,6 @@ const ( gitHubOwnedActionWeight int = 2 thirdPartyActionWeight int = 8 normalWeight int = gitHubOwnedActionWeight + thirdPartyActionWeight - - // depTypeKey is the Values map key used to fetch the dependency type. - depTypeKey = "dependencyType" ) func probeRemToRuleRem(rem *probe.Remediation) *rule.Remediation { @@ -124,7 +121,7 @@ func PinningDependencies(name string, default: // ignore } - updatePinningResults(checker.DependencyUseType(f.Values[depTypeKey]), + updatePinningResults(checker.DependencyUseType(f.Values[pinsDependencies.DepTypeKey]), f.Outcome, f.Location.Snippet, &wp, pr) } diff --git a/probes/pinsDependencies/impl.go b/probes/pinsDependencies/impl.go index bf5ffda1bfc..1b16ce27a4e 100644 --- a/probes/pinsDependencies/impl.go +++ b/probes/pinsDependencies/impl.go @@ -33,7 +33,7 @@ var fs embed.FS const ( Probe = "pinsDependencies" - depTypeKey = "dependencyType" + DepTypeKey = "dependencyType" ) var dependencyTypes = map[checker.DependencyUseType]int{ @@ -133,7 +133,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { f.Remediation = ruleRemToProbeRem(rr.Remediation) } f = f.WithValues(map[string]int{ - depTypeKey: dependencyTypes[rr.Type], + DepTypeKey: dependencyTypes[rr.Type], }) findings = append(findings, *f) } else { @@ -150,7 +150,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { Location: loc, } f = f.WithValues(map[string]int{ - depTypeKey: dependencyTypes[rr.Type], + DepTypeKey: dependencyTypes[rr.Type], }) findings = append(findings, *f) } From 3df7656db55b1232f5ce3bfa14b7b8a38804b1de Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Tue, 6 Feb 2024 15:15:01 +0000 Subject: [PATCH 09/13] Do not copy test Dockerfile Signed-off-by: Adam Korczynski --- checks/pinned_dependencies_test.go | 2 +- .../pinneddependencies/Dockerfile-script-ok | 71 ------------------- 2 files changed, 1 insertion(+), 72 deletions(-) delete mode 100644 checks/testdata/pinneddependencies/Dockerfile-script-ok diff --git a/checks/pinned_dependencies_test.go b/checks/pinned_dependencies_test.go index ec7acfb175f..b22fa7fb849 100644 --- a/checks/pinned_dependencies_test.go +++ b/checks/pinned_dependencies_test.go @@ -37,7 +37,7 @@ func TestPinningDependencies(t *testing.T) { }{ { name: "Dockerfile", - path: "./testdata/pinneddependencies/Dockerfile-script-ok", + path: "./raw/testdata/Dockerfile-script-ok", files: []string{ "Dockerfile-script-ok", }, diff --git a/checks/testdata/pinneddependencies/Dockerfile-script-ok b/checks/testdata/pinneddependencies/Dockerfile-script-ok deleted file mode 100644 index 1d493959526..00000000000 --- a/checks/testdata/pinneddependencies/Dockerfile-script-ok +++ /dev/null @@ -1,71 +0,0 @@ -# Copyright 2021 Security 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. - -FROM python:3.7@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2 - -# 如果在中国，apt使用163源, ifconfig.co/json, http://ip-api.com - -RUN wget program -RUN curl | echo - -# aws -RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script . - -RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script /tmp/file1 && bash /tmp/fileXXX -RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script /tmp/file2 ; bash /tmp/file1234 - -RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script . && bash scriptxxx -RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script /path/to/ && bash /path/to/scriptxxx -RUN aws s3api get-object --bucket DOC-EXAMPLE-BUCKET1 --key dir/script2 /path/to/ -RUN bash /path/to/script2xxx - -# curl -RUN curl http://file 2>&1 > /tmp/file1 && sh /tmp/filex -RUN curl http://file2 2>&1 > /tmp/file2 ; sh /tmp/filex -RUN curl http://file2 2>&1 > /tmp/file2 ; sh /tmp/filex -RUN curl http://file2 2>&1 > /tmp/file4 ; \ - bash /tmp/file5 - -RUN echo hello && curl -s http://etc/file | echo -RUN echo hello && curl -s http://file-with-sudo2 | sudo echo - -# gsutil -RUN gsutil gs://file /tmp/file -RUN bash /tmp/filezx - -RUN gsutil gs://file /tmp/file1 && bash /tmp/fileqw -RUN gsutil gs://file /tmp/file2 ; bash /tmp/file122 - -RUN gsutil gs://bucket/file . && bash ./file2222 -RUN gsutil gs://bucet/file /path/to/ && bash /path/to/fileqqq -RUN gsutil gs://bucet/file2 /path/to/ && bash /path/to/file2234 -RUN bash /path/to/fileshsj - -RUN bash somescript.sh - -RUN sudo su -c "bash blabl.sh" root - -RUN echo hello && sudo curl -s file-with-sudo | echo | bla - -RUN echo hello && wget -0 - ifconfig.co/json | echo - -RUN wget http://file -O /tmp/file -RUN bash /tmp/filegshek - -RUN wget http://file -O /tmp/file1 && bash /tmp/file1xxxx -RUN wget http://file -O /tmp/file2 ; bash /tmp/file2xxxxx -RUN wget http://domain.com/file . && bash ./fileccc - -FROM scratch -FROM python@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2 \ No newline at end of file From 0e273c6da2d50bb9c7193b1c773d5b2839e9fa38 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Tue, 6 Feb 2024 15:20:37 +0000 Subject: [PATCH 10/13] rename test Signed-off-by: Adam Korczynski --- checks/evaluation/pinned_dependencies_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks/evaluation/pinned_dependencies_test.go b/checks/evaluation/pinned_dependencies_test.go index 24d712bcc32..a855cec2e2d 100644 --- a/checks/evaluation/pinned_dependencies_test.go +++ b/checks/evaluation/pinned_dependencies_test.go @@ -345,7 +345,7 @@ func Test_PinningDependencies(t *testing.T) { }, }, { - name: "2 unpinned dependencies for 1 ecosystem shows 2 warn messages", + name: "2 unpinned dependencies for 2 ecosystems shows 2 warn messages", findings: []finding.Finding{ { Probe: "pinsDependencies", From aefdc236664d1d77cf5074f6322bf7ac4dc21963 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Thu, 8 Feb 2024 11:03:36 +0000 Subject: [PATCH 11/13] Rebase and bring back 'Test_generateOwnerToDisplay' Signed-off-by: Adam Korczynski --- checks/evaluation/pinned_dependencies_test.go | 61 ++++++++++++++----- probes/pinsDependencies/impl.go | 19 ++---- 2 files changed, 49 insertions(+), 31 deletions(-) diff --git a/checks/evaluation/pinned_dependencies_test.go b/checks/evaluation/pinned_dependencies_test.go index a855cec2e2d..5af1bead9db 100644 --- a/checks/evaluation/pinned_dependencies_test.go +++ b/checks/evaluation/pinned_dependencies_test.go @@ -249,8 +249,8 @@ func Test_PinningDependencies(t *testing.T) { LineStart: &testLineStart, Snippet: &testSnippet, }, - Values: map[string]int{ - "dependencyType": 6, // pip type + Values: map[string]string{ + "dependencyType": string(checker.DependencyUseTypePipCommand), }, }, }, @@ -272,8 +272,8 @@ func Test_PinningDependencies(t *testing.T) { LineEnd: &testLineEnd, Snippet: &testSnippet, }, - Values: map[string]int{ - "dependencyType": 6, // pip type + Values: map[string]string{ + "dependencyType": string(checker.DependencyUseTypePipCommand), }, }, }, @@ -296,8 +296,8 @@ func Test_PinningDependencies(t *testing.T) { LineEnd: &testLineEnd, Snippet: &testSnippet, }, - Values: map[string]int{ - "dependencyType": 6, // pip type + Values: map[string]string{ + "dependencyType": string(checker.DependencyUseTypePipCommand), }, }, }, @@ -319,8 +319,8 @@ func Test_PinningDependencies(t *testing.T) { LineEnd: &testLineEnd, Snippet: &testSnippet, }, - Values: map[string]int{ - "dependencyType": 6, // pip type + Values: map[string]string{ + "dependencyType": string(checker.DependencyUseTypePipCommand), }, }, { @@ -333,8 +333,8 @@ func Test_PinningDependencies(t *testing.T) { LineEnd: &testLineEnd, Snippet: &testSnippet, }, - Values: map[string]int{ - "dependencyType": 6, // pip type + Values: map[string]string{ + "dependencyType": string(checker.DependencyUseTypePipCommand), }, }, }, @@ -357,8 +357,8 @@ func Test_PinningDependencies(t *testing.T) { LineEnd: &testLineEnd, Snippet: &testSnippet, }, - Values: map[string]int{ - "dependencyType": 6, // pip type + Values: map[string]string{ + "dependencyType": string(checker.DependencyUseTypePipCommand), }, }, { @@ -371,8 +371,8 @@ func Test_PinningDependencies(t *testing.T) { LineEnd: &testLineEnd, Snippet: &testSnippet, }, - Values: map[string]int{ - "dependencyType": 3, // go type + Values: map[string]string{ + "dependencyType": string(checker.DependencyUseTypeGoCommand), }, }, }, @@ -395,8 +395,8 @@ func Test_PinningDependencies(t *testing.T) { LineEnd: &testLineEnd, Snippet: &testSnippet, }, - Values: map[string]int{ - "dependencyType": 0, // GH Action type + Values: map[string]string{ + "dependencyType": string(checker.DependencyUseTypeGHAction), }, }, }, @@ -703,3 +703,32 @@ func TestUpdatePinningResults(t *testing.T) { }) } } + +func Test_generateOwnerToDisplay(t *testing.T) { + t.Parallel() + tests := []struct { //nolint:govet + name string + gitHubOwned bool + want string + }{ + { + name: "returns GitHub if gitHubOwned is true", + gitHubOwned: true, + want: "GitHub-owned GitHubAction", + }, + { + name: "returns GitHub if gitHubOwned is false", + gitHubOwned: false, + want: "third-party GitHubAction", + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := generateOwnerToDisplay(tt.gitHubOwned); got != tt.want { + t.Errorf("generateOwnerToDisplay() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/probes/pinsDependencies/impl.go b/probes/pinsDependencies/impl.go index 1b16ce27a4e..8404f42e25f 100644 --- a/probes/pinsDependencies/impl.go +++ b/probes/pinsDependencies/impl.go @@ -36,17 +36,6 @@ const ( DepTypeKey = "dependencyType" ) -var dependencyTypes = map[checker.DependencyUseType]int{ - checker.DependencyUseTypeGHAction: 0, - checker.DependencyUseTypeDockerfileContainerImage: 1, - checker.DependencyUseTypeDownloadThenRun: 2, - checker.DependencyUseTypeGoCommand: 3, - checker.DependencyUseTypeChocoCommand: 4, - checker.DependencyUseTypeNpmCommand: 5, - checker.DependencyUseTypePipCommand: 6, - checker.DependencyUseTypeNugetCommand: 7, -} - func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) @@ -132,8 +121,8 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if rr.Remediation != nil { f.Remediation = ruleRemToProbeRem(rr.Remediation) } - f = f.WithValues(map[string]int{ - DepTypeKey: dependencyTypes[rr.Type], + f = f.WithValues(map[string]string{ + DepTypeKey: string(rr.Type), }) findings = append(findings, *f) } else { @@ -149,8 +138,8 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { Outcome: finding.OutcomePositive, Location: loc, } - f = f.WithValues(map[string]int{ - DepTypeKey: dependencyTypes[rr.Type], + f = f.WithValues(map[string]string{ + DepTypeKey: string(rr.Type), }) findings = append(findings, *f) } From 1d58fbcca7933ce6f488fcb48e9178671ca79a9b Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Thu, 22 Feb 2024 21:10:13 +0000 Subject: [PATCH 12/13] Use API to create finding Signed-off-by: AdamKorcz --- probes/pinsDependencies/impl.go | 42 ++++++++++++--------------------- 1 file changed, 15 insertions(+), 27 deletions(-) diff --git a/probes/pinsDependencies/impl.go b/probes/pinsDependencies/impl.go index 8404f42e25f..dc56531feba 100644 --- a/probes/pinsDependencies/impl.go +++ b/probes/pinsDependencies/impl.go @@ -47,26 +47,26 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for i := range r.ProcessingErrors { e := r.ProcessingErrors[i] - f := finding.Finding{ - Message: generateTextIncompleteResults(e), - Location: &e.Location, - Outcome: finding.OutcomeError, + f, err := finding.NewWith(fs, Probe, generateTextIncompleteResults(e), + &e.Location, finding.OutcomeError) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) } - findings = append(findings, f) + findings = append(findings, *f) } for i := range r.Dependencies { rr := r.Dependencies[i] + f, err := finding.NewWith(fs, Probe, "", nil, finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } if rr.Location == nil { if rr.Msg == nil { e := sce.WithMessage(sce.ErrScorecardInternal, "empty File field") return findings, Probe, e } - f := &finding.Finding{ - Probe: Probe, - Outcome: finding.OutcomeNotApplicable, - Message: *rr.Msg, - } + f = f.WithMessage(*rr.Msg).WithOutcome(finding.OutcomeNotApplicable) findings = append(findings, *f) continue } @@ -78,12 +78,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { LineEnd: &rr.Location.EndOffset, Snippet: &rr.Location.Snippet, } - f := &finding.Finding{ - Probe: Probe, - Outcome: finding.OutcomeNotApplicable, - Message: *rr.Msg, - Location: loc, - } + f = f.WithMessage(*rr.Msg).WithLocation(loc).WithOutcome(finding.OutcomeNotApplicable) findings = append(findings, *f) continue } @@ -112,12 +107,9 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { LineEnd: &rr.Location.EndOffset, Snippet: &rr.Location.Snippet, } - f := &finding.Finding{ - Probe: Probe, - Outcome: finding.OutcomeNegative, - Message: generateTextUnpinned(&rr), - Location: loc, - } + f = f.WithMessage(generateTextUnpinned(&rr)). + WithLocation(loc). + WithOutcome(finding.OutcomeNegative) if rr.Remediation != nil { f.Remediation = ruleRemToProbeRem(rr.Remediation) } @@ -133,11 +125,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { LineEnd: &rr.Location.EndOffset, Snippet: &rr.Location.Snippet, } - f := &finding.Finding{ - Probe: Probe, - Outcome: finding.OutcomePositive, - Location: loc, - } + f = f.WithMessage("").WithLocation(loc).WithOutcome(finding.OutcomePositive) f = f.WithValues(map[string]string{ DepTypeKey: string(rr.Type), }) From a8bfdb5364e676d37ba5567df332b68d24c99c67 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Thu, 22 Feb 2024 21:12:56 +0000 Subject: [PATCH 13/13] one more change to how the probe creates a finding Signed-off-by: AdamKorcz --- probes/pinsDependencies/impl.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/probes/pinsDependencies/impl.go b/probes/pinsDependencies/impl.go index dc56531feba..92f9de4acfa 100644 --- a/probes/pinsDependencies/impl.go +++ b/probes/pinsDependencies/impl.go @@ -90,12 +90,9 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { LineEnd: &rr.Location.EndOffset, Snippet: &rr.Location.Snippet, } - f := &finding.Finding{ - Probe: Probe, - Outcome: finding.OutcomeNotApplicable, - Message: fmt.Sprintf("%s has empty Pinned field", rr.Type), - Location: loc, - } + f = f.WithMessage(fmt.Sprintf("%s has empty Pinned field", rr.Type)). + WithLocation(loc). + WithOutcome(finding.OutcomeNotApplicable) findings = append(findings, *f) continue }