From ce75fa668660338297477209a058aec87fb24782 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 22 Sep 2023 20:34:31 +0100 Subject: [PATCH 01/14] :seedling: convert binary artifact check to probe Signed-off-by: AdamKorcz --- checks/binary_artifact.go | 19 +- checks/evaluation/binary_artifacts.go | 45 ++- checks/evaluation/binary_artifacts_test.go | 356 ++++++++------------- checks/raw/binary_artifact.go | 3 +- checks/raw/binary_artifact_test.go | 21 +- probes/entries.go | 4 + probes/hasBinaryArtifacts/def.yml | 27 ++ probes/hasBinaryArtifacts/impl.go | 79 +++++ probes/hasBinaryArtifacts/impl_test.go | 300 +++++++++++++++++ 9 files changed, 598 insertions(+), 256 deletions(-) create mode 100644 probes/hasBinaryArtifacts/def.yml create mode 100644 probes/hasBinaryArtifacts/impl.go create mode 100644 probes/hasBinaryArtifacts/impl_test.go diff --git a/checks/binary_artifact.go b/checks/binary_artifact.go index 63a7ec98794..9d64bf2a123 100644 --- a/checks/binary_artifact.go +++ b/checks/binary_artifact.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" ) // CheckBinaryArtifacts is the exported name for Binary-Artifacts check. @@ -38,17 +40,22 @@ func init() { // BinaryArtifacts will check the repository contains binary artifacts. func BinaryArtifacts(c *checker.CheckRequest) checker.CheckResult { - rawData, err := raw.BinaryArtifacts(c.RepoClient) + rawData, err := raw.BinaryArtifacts(c) if err != nil { e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) return checker.CreateRuntimeErrorResult(CheckBinaryArtifacts, e) } - // Return raw results. - if c.RawResults != nil { - c.RawResults.BinaryArtifactResults = rawData + // Set the raw results. + pRawResults := getRawResults(c) + pRawResults.BinaryArtifactResults = rawData + + // Evaluate the probes. + findings, err := zrunner.Run(pRawResults, probes.BinaryArtifacts) + if err != nil { + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return checker.CreateRuntimeErrorResult(CheckVulnerabilities, e) } - // Return the score evaluation. - return evaluation.BinaryArtifacts(CheckBinaryArtifacts, c.Dlogger, &rawData) + return evaluation.BinaryArtifacts(CheckBinaryArtifacts, findings, c.Dlogger) } diff --git a/checks/evaluation/binary_artifacts.go b/checks/evaluation/binary_artifacts.go index 59075912844..c74fb7f7ed3 100644 --- a/checks/evaluation/binary_artifacts.go +++ b/checks/evaluation/binary_artifacts.go @@ -18,36 +18,47 @@ import ( "github.com/ossf/scorecard/v4/checker" sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/hasBinaryArtifacts" ) // BinaryArtifacts applies the score policy for the Binary-Artifacts check. -func BinaryArtifacts(name string, dl checker.DetailLogger, - r *checker.BinaryArtifactData, +func BinaryArtifacts(name string, + findings []finding.Finding, + dl checker.DetailLogger, ) checker.CheckResult { - if r == nil { - e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data") - return checker.CreateRuntimeErrorResult(name, e) + expectedProbes := []string{ + hasBinaryArtifacts.Probe, } - // Apply the policy evaluation. - if r.Files == nil || len(r.Files) == 0 { - return checker.CreateMaxScoreResult(name, "no binaries found in the repo") + err := validateFindings(findings, expectedProbes) + if err != nil { + return checker.CreateRuntimeErrorResult(name, err) } - score := checker.MaxResultScore - for _, f := range r.Files { - dl.Warn(&checker.LogMessage{ - Path: f.Path, Type: finding.FileTypeBinary, - Offset: f.Offset, - Text: "binary detected", - }) - // We remove one point for each binary. - score-- + if findings[0].Outcome == finding.OutcomePositive { + return checker.CreateMaxScoreResult(name, "no binaries found in the repo") } + // There are only negative findings. + // Deduct the number of findings from max score + numberOfBinaryFilesFound := len(findings) + + score := checker.MaxResultScore - numberOfBinaryFilesFound + if score < checker.MinResultScore { score = checker.MinResultScore } return checker.CreateResultWithScore(name, "binaries present in source code", score) } + +func validateFindings(findings []finding.Finding, expectedProbes []string) error { + if !finding.UniqueProbesEqual(findings, expectedProbes) { + return sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") + } + + if len(findings) == 0 { + return sce.WithMessage(sce.ErrScorecardInternal, "found 0 findings. Should not happen") + } + return nil +} diff --git a/checks/evaluation/binary_artifacts_test.go b/checks/evaluation/binary_artifacts_test.go index ed7a0fa95a7..77976e7d914 100644 --- a/checks/evaluation/binary_artifacts_test.go +++ b/checks/evaluation/binary_artifacts_test.go @@ -17,7 +17,8 @@ package evaluation import ( "testing" - "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" ) @@ -31,259 +32,154 @@ func TestBinaryArtifacts(t *testing.T) { r *checker.BinaryArtifactData } tests := []struct { - name string - args args - want checker.CheckResult - wantErr bool + name string + findings []finding.Finding + result scut.TestReturn + expected []struct { + lineNumber uint + } }{ { - name: "r nil", - args: args{ - name: "test_binary_artifacts_check_pass", - dl: &scut.TestDetailLogger{}, + name: "no binary artifacts", + findings: []finding.Finding { + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomePositive, + }, + }, + result: scut.TestReturn{ + Score: 10, }, - wantErr: true, }, { - name: "no binary artifacts", - args: args{ - name: "no binary artifacts", - dl: &scut.TestDetailLogger{}, - r: &checker.BinaryArtifactData{}, + name: "one binary artifact", + findings: []finding.Finding { + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, }, - want: checker.CheckResult{ - Score: checker.MaxResultScore, + result: scut.TestReturn{ + Score: 9, }, }, { - name: "1 binary artifact", - args: args{ - name: "no binary artifacts", - dl: &scut.TestDetailLogger{}, - r: &checker.BinaryArtifactData{ - Files: []checker.File{ - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - }, + name: "two binary artifact", + findings: []finding.Finding { + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: 9, + result: scut.TestReturn{ + Score: 8, }, }, { - name: "many binary artifact", - args: args{ - name: "no binary artifacts", - dl: &scut.TestDetailLogger{}, - r: &checker.BinaryArtifactData{ - Files: []checker.File{ - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - }, + name: "five binary artifact", + findings: []finding.Finding { + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ + result: scut.TestReturn{ + Score: 5, + }, + }, + { + name: "twelve binary artifact", + findings: []finding.Finding { + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "hasBinaryArtifacts", + Outcome: finding.OutcomeNegative, + }, + }, + result: scut.TestReturn{ Score: 0, }, }, + { + name: "invalid findings", + findings: []finding.Finding {}, + result: scut.TestReturn{ + Score: -1, + Error: sce.ErrScorecardInternal, + }, + }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - got := BinaryArtifacts(tt.args.name, tt.args.dl, tt.args.r) - if tt.wantErr { - if got.Error == nil { - t.Errorf("BinaryArtifacts() error = %v, wantErr %v", got.Error, tt.wantErr) - } - } else { - if got.Score != tt.want.Score { - t.Errorf("BinaryArtifacts() = %v, want %v", got.Score, tt.want.Score) - } + dl := scut.TestDetailLogger{} + got := BinaryArtifacts(tt.name, tt.findings, &dl) + if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) { + t.Errorf("got %v, expected %v", got, tt.result) } }) } diff --git a/checks/raw/binary_artifact.go b/checks/raw/binary_artifact.go index 03c129b5727..fa87e6fe203 100644 --- a/checks/raw/binary_artifact.go +++ b/checks/raw/binary_artifact.go @@ -49,7 +49,8 @@ func mustParseConstraint(c string) *semver.Constraints { } // BinaryArtifacts retrieves the raw data for the Binary-Artifacts check. -func BinaryArtifacts(c clients.RepoClient) (checker.BinaryArtifactData, error) { +func BinaryArtifacts(req *checker.CheckRequest) (checker.BinaryArtifactData, error) { + c := req.RepoClient files := []checker.File{} err := fileparser.OnMatchingFileContentDo(c, fileparser.PathMatcher{ Pattern: "*", diff --git a/checks/raw/binary_artifact_test.go b/checks/raw/binary_artifact_test.go index ae9970bc500..8cde90496c8 100644 --- a/checks/raw/binary_artifact_test.go +++ b/checks/raw/binary_artifact_test.go @@ -21,8 +21,10 @@ import ( "github.com/golang/mock/gomock" + "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/clients" mockrepo "github.com/ossf/scorecard/v4/clients/mockclients" + scut "github.com/ossf/scorecard/v4/utests" ) func strptr(s string) *string { @@ -220,6 +222,7 @@ func TestBinaryArtifacts(t *testing.T) { ctrl := gomock.NewController(t) mockRepoClient := mockrepo.NewMockRepoClient(ctrl) + mockRepo := mockrepo.NewMockRepo(ctrl) for _, files := range tt.files { mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return(files, nil) } @@ -240,7 +243,14 @@ func TestBinaryArtifacts(t *testing.T) { mockRepoClient.EXPECT().ListCommits().Return(tt.commits, nil) } - f, err := BinaryArtifacts(mockRepoClient) + dl := scut.TestDetailLogger{} + c := &checker.CheckRequest{ + RepoClient: mockRepoClient, + Repo: mockRepo, + Dlogger: &dl, + } + + f, err := BinaryArtifacts(c) if tt.err != nil { // If we expect an error, make sure it is the same @@ -260,6 +270,7 @@ func TestBinaryArtifacts(t *testing.T) { func TestBinaryArtifacts_workflow_runs_unsupported(t *testing.T) { ctrl := gomock.NewController(t) mockRepoClient := mockrepo.NewMockRepoClient(ctrl) + mockRepo := mockrepo.NewMockRepo(ctrl) const jarFile = "gradle-wrapper.jar" const verifyWorkflow = ".github/workflows/verify.yaml" files := []string{jarFile, verifyWorkflow} @@ -280,7 +291,13 @@ func TestBinaryArtifacts_workflow_runs_unsupported(t *testing.T) { }).AnyTimes() mockRepoClient.EXPECT().ListSuccessfulWorkflowRuns(gomock.Any()).Return(nil, clients.ErrUnsupportedFeature).AnyTimes() - got, err := BinaryArtifacts(mockRepoClient) + dl := scut.TestDetailLogger{} + c := &checker.CheckRequest{ + RepoClient: mockRepoClient, + Repo: mockRepo, + Dlogger: &dl, + } + got, err := BinaryArtifacts(c) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/probes/entries.go b/probes/entries.go index b5600ebe7e2..c5fbaac6b6b 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -45,6 +45,7 @@ import ( "github.com/ossf/scorecard/v4/probes/sastToolCodeQLInstalled" "github.com/ossf/scorecard/v4/probes/sastToolRunsOnAllCommits" "github.com/ossf/scorecard/v4/probes/sastToolSonarInstalled" + "github.com/ossf/scorecard/v4/probes/hasBinaryArtifacts" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsLinks" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsText" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsVulnerabilityDisclosure" @@ -121,6 +122,9 @@ var ( CIIBestPractices = []ProbeImpl{ hasOpenSSFBadge.Run, } + BinaryArtifacts = []ProbeImpl{ + hasBinaryArtifacts.Run, + } ) //nolint:gochecknoinits diff --git a/probes/hasBinaryArtifacts/def.yml b/probes/hasBinaryArtifacts/def.yml new file mode 100644 index 00000000000..80bb4020fbc --- /dev/null +++ b/probes/hasBinaryArtifacts/def.yml @@ -0,0 +1,27 @@ +# Copyright 2023 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +id: hasBinaryArtifacts +short: Checks if the project has binary files in its source tree. +motivation: > + Including generated executables in the source repository increases user risk. Many programming language systems can generate executables from source code (e.g., C/C++ generated machine code, Java .class files, Python .pyc files, and minified JavaScript). Users will often directly use executables if they are included in the source repository, leading to many dangerous behaviors. +implementation: > + The implementation looks for the presence of binary files. +outcome: + - If the probe finds binary files, it returns a number of negative outcomes equal to the number of binary files found. +remediation: + effort: Medium + text: + - Remove the generated executable artifacts from the repository. + - Build from source. \ No newline at end of file diff --git a/probes/hasBinaryArtifacts/impl.go b/probes/hasBinaryArtifacts/impl.go new file mode 100644 index 00000000000..c6a1b896aa1 --- /dev/null +++ b/probes/hasBinaryArtifacts/impl.go @@ -0,0 +1,79 @@ +// 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 hasBinaryArtifacts + +import ( + "embed" + "fmt" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/internal/utils/uerror" +) + +//go:embed *.yml +var fs embed.FS + +const Probe = "hasBinaryArtifacts" + +func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { + if raw == nil { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + + r := raw.BinaryArtifactResults + + // Apply the policy evaluation. + if r.Files == nil || len(r.Files) == 0 { + return positiveOutcome() + } + + var findings []finding.Finding + + for range r.Files { + f, err := finding.NewWith(fs, Probe, "binary artifact detected", + nil, finding.OutcomeNegative) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + } + + if len(findings) == 0 { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + + // Check if there are both positive and negative outcomes. + // This should not happen. + for i := range findings { + f := &findings[i] + if f.Outcome == finding.OutcomePositive { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + } + + return findings, Probe, nil +} + +func positiveOutcome() ([]finding.Finding, string, error) { + f, err := finding.NewWith(fs, Probe, + "Repository does not have binary artifacts.", nil, + finding.OutcomePositive) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + return []finding.Finding{*f}, Probe, nil +} diff --git a/probes/hasBinaryArtifacts/impl_test.go b/probes/hasBinaryArtifacts/impl_test.go new file mode 100644 index 00000000000..71f23628b4f --- /dev/null +++ b/probes/hasBinaryArtifacts/impl_test.go @@ -0,0 +1,300 @@ +// 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 hasBinaryArtifacts + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/finding" +) + +func Test_Run(t *testing.T) { + t.Parallel() + // nolint:govet + tests := []struct { + name string + raw *checker.RawResults + outcomes []finding.Outcome + err error + }{ + { + name: "1 binary artifact", + raw: &checker.RawResults{ + BinaryArtifactResults: checker.BinaryArtifactData{ + Files: []checker.File{ + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + { + name: "No binary artifacts", + raw: &checker.RawResults{ + BinaryArtifactResults: checker.BinaryArtifactData{}, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + }, + { + name: "many binary artifact", + raw: &checker.RawResults{ + BinaryArtifactResults: checker.BinaryArtifactData{ + Files: []checker.File{ + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + { + Path: "test_binary_artifacts_check_pass", + Snippet: ` + package main + import "fmt" + func main() { + fmt.Println("Hello, world!") + }i`, + Offset: 0, + Type: 0, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + }, + }, + } + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + findings, s, err := Run(tt.raw) + if !cmp.Equal(tt.err, err, cmpopts.EquateErrors()) { + t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(tt.err, err, cmpopts.EquateErrors())) + } + if err != nil { + return + } + if diff := cmp.Diff(Probe, s); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(len(tt.outcomes), len(findings)); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + for i := range tt.outcomes { + outcome := &tt.outcomes[i] + f := &findings[i] + if diff := cmp.Diff(*outcome, f.Outcome); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + } + }) + } +} From 5c2059ba64ea84b19b89e1c1e3fda77c8c33a233 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Tue, 3 Oct 2023 14:45:48 +0100 Subject: [PATCH 02/14] Reword motivation Signed-off-by: AdamKorcz --- probes/hasBinaryArtifacts/def.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probes/hasBinaryArtifacts/def.yml b/probes/hasBinaryArtifacts/def.yml index 80bb4020fbc..809863d2390 100644 --- a/probes/hasBinaryArtifacts/def.yml +++ b/probes/hasBinaryArtifacts/def.yml @@ -15,7 +15,7 @@ id: hasBinaryArtifacts short: Checks if the project has binary files in its source tree. motivation: > - Including generated executables in the source repository increases user risk. Many programming language systems can generate executables from source code (e.g., C/C++ generated machine code, Java .class files, Python .pyc files, and minified JavaScript). Users will often directly use executables if they are included in the source repository, leading to many dangerous behaviors. + Binary files are not readable so users can't see what they do. Many programming language systems can generate executables from source code (e.g., C/C++ generated machine code, Java .class files, Python .pyc files, and minified JavaScript). Users will often directly use executables if they are included in the source repository, leading to many dangerous behaviors. implementation: > The implementation looks for the presence of binary files. outcome: From 5142ce4d30d4540ae892cb44c7a3fd6696f3044b Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Tue, 3 Oct 2023 14:48:23 +0100 Subject: [PATCH 03/14] remove unused variable in test Signed-off-by: AdamKorcz --- checks/evaluation/binary_artifacts_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/checks/evaluation/binary_artifacts_test.go b/checks/evaluation/binary_artifacts_test.go index 77976e7d914..c477e9ea4ac 100644 --- a/checks/evaluation/binary_artifacts_test.go +++ b/checks/evaluation/binary_artifacts_test.go @@ -35,9 +35,6 @@ func TestBinaryArtifacts(t *testing.T) { name string findings []finding.Finding result scut.TestReturn - expected []struct { - lineNumber uint - } }{ { name: "no binary artifacts", From 28728ead7bcf967526645957e71679aececd11ca Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Tue, 3 Oct 2023 14:53:19 +0100 Subject: [PATCH 04/14] remove positiveOutcome() and length check Signed-off-by: AdamKorcz --- checks/evaluation/binary_artifacts.go | 19 ++++--------------- probes/hasBinaryArtifacts/impl.go | 20 ++++++++------------ 2 files changed, 12 insertions(+), 27 deletions(-) diff --git a/checks/evaluation/binary_artifacts.go b/checks/evaluation/binary_artifacts.go index c74fb7f7ed3..cfeea1e7d70 100644 --- a/checks/evaluation/binary_artifacts.go +++ b/checks/evaluation/binary_artifacts.go @@ -30,9 +30,9 @@ func BinaryArtifacts(name string, hasBinaryArtifacts.Probe, } - err := validateFindings(findings, expectedProbes) - if err != nil { - return checker.CreateRuntimeErrorResult(name, err) + if !finding.UniqueProbesEqual(findings, expectedProbes) { + e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") + return checker.CreateRuntimeErrorResult(name, e) } if findings[0].Outcome == finding.OutcomePositive { @@ -50,15 +50,4 @@ func BinaryArtifacts(name string, } return checker.CreateResultWithScore(name, "binaries present in source code", score) -} - -func validateFindings(findings []finding.Finding, expectedProbes []string) error { - if !finding.UniqueProbesEqual(findings, expectedProbes) { - return sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") - } - - if len(findings) == 0 { - return sce.WithMessage(sce.ErrScorecardInternal, "found 0 findings. Should not happen") - } - return nil -} +} \ No newline at end of file diff --git a/probes/hasBinaryArtifacts/impl.go b/probes/hasBinaryArtifacts/impl.go index c6a1b896aa1..7c332cdcf28 100644 --- a/probes/hasBinaryArtifacts/impl.go +++ b/probes/hasBinaryArtifacts/impl.go @@ -38,7 +38,13 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { // Apply the policy evaluation. if r.Files == nil || len(r.Files) == 0 { - return positiveOutcome() + f, err := finding.NewWith(fs, Probe, + "Repository does not have binary artifacts.", nil, + finding.OutcomePositive) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + return []finding.Finding{*f}, Probe, nil } var findings []finding.Finding @@ -66,14 +72,4 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } return findings, Probe, nil -} - -func positiveOutcome() ([]finding.Finding, string, error) { - f, err := finding.NewWith(fs, Probe, - "Repository does not have binary artifacts.", nil, - finding.OutcomePositive) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - return []finding.Finding{*f}, Probe, nil -} +} \ No newline at end of file From cbf8b385a820f22a4150a461e588424ba1f13820 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Thu, 5 Oct 2023 15:53:28 +0100 Subject: [PATCH 05/14] fix wrong check name Signed-off-by: AdamKorcz --- checks/binary_artifact.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks/binary_artifact.go b/checks/binary_artifact.go index 9d64bf2a123..f57d154524f 100644 --- a/checks/binary_artifact.go +++ b/checks/binary_artifact.go @@ -54,7 +54,7 @@ func BinaryArtifacts(c *checker.CheckRequest) checker.CheckResult { findings, err := zrunner.Run(pRawResults, probes.BinaryArtifacts) if err != nil { e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) - return checker.CreateRuntimeErrorResult(CheckVulnerabilities, e) + return checker.CreateRuntimeErrorResult(CheckBinaryArtifacts, e) } return evaluation.BinaryArtifacts(CheckBinaryArtifacts, findings, c.Dlogger) From 71bfeb478cdcf665ee01f6d5ff6b766a69a1b115 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Thu, 23 Nov 2023 13:49:00 +0000 Subject: [PATCH 06/14] Split into two probes: One with and one without gradle-wrappers Signed-off-by: AdamKorcz --- checks/evaluation/binary_artifacts.go | 16 +- checks/evaluation/binary_artifacts_test.go | 152 ++++----- checks/raw/binary_artifact.go | 5 +- checks/raw/binary_artifact_test.go | 4 +- finding/finding.go | 2 + probes/entries.go | 4 +- .../def.yml | 9 +- .../impl.go | 39 +-- probes/freeOfAnyBinaryArtifacts/impl_test.go | 158 +++++++++ probes/freeOfUntrustedBinaryArtifacts/def.yml | 28 ++ probes/freeOfUntrustedBinaryArtifacts/impl.go | 69 ++++ .../impl_test.go | 123 +++++++ probes/hasBinaryArtifacts/impl_test.go | 300 ------------------ 13 files changed, 479 insertions(+), 430 deletions(-) rename probes/{hasBinaryArtifacts => freeOfAnyBinaryArtifacts}/def.yml (72%) rename probes/{hasBinaryArtifacts => freeOfAnyBinaryArtifacts}/impl.go (72%) create mode 100644 probes/freeOfAnyBinaryArtifacts/impl_test.go create mode 100644 probes/freeOfUntrustedBinaryArtifacts/def.yml create mode 100644 probes/freeOfUntrustedBinaryArtifacts/impl.go create mode 100644 probes/freeOfUntrustedBinaryArtifacts/impl_test.go delete mode 100644 probes/hasBinaryArtifacts/impl_test.go diff --git a/checks/evaluation/binary_artifacts.go b/checks/evaluation/binary_artifacts.go index cfeea1e7d70..99f519435ad 100644 --- a/checks/evaluation/binary_artifacts.go +++ b/checks/evaluation/binary_artifacts.go @@ -18,7 +18,7 @@ import ( "github.com/ossf/scorecard/v4/checker" sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/finding" - "github.com/ossf/scorecard/v4/probes/hasBinaryArtifacts" + "github.com/ossf/scorecard/v4/probes/freeOfUntrustedBinaryArtifacts" ) // BinaryArtifacts applies the score policy for the Binary-Artifacts check. @@ -27,7 +27,7 @@ func BinaryArtifacts(name string, dl checker.DetailLogger, ) checker.CheckResult { expectedProbes := []string{ - hasBinaryArtifacts.Probe, + freeOfUntrustedBinaryArtifacts.Probe, } if !finding.UniqueProbesEqual(findings, expectedProbes) { @@ -39,6 +39,16 @@ func BinaryArtifacts(name string, return checker.CreateMaxScoreResult(name, "no binaries found in the repo") } + for i := range findings { + f := &findings[i] + dl.Warn(&checker.LogMessage{ + Path: f.Location.Path, + Type: f.Location.Type, + Offset: *f.Location.LineStart, + Text: "binary detected", + }) + } + // There are only negative findings. // Deduct the number of findings from max score numberOfBinaryFilesFound := len(findings) @@ -50,4 +60,4 @@ func BinaryArtifacts(name string, } return checker.CreateResultWithScore(name, "binaries present in source code", score) -} \ No newline at end of file +} diff --git a/checks/evaluation/binary_artifacts_test.go b/checks/evaluation/binary_artifacts_test.go index c477e9ea4ac..6ace7f0c346 100644 --- a/checks/evaluation/binary_artifacts_test.go +++ b/checks/evaluation/binary_artifacts_test.go @@ -25,22 +25,28 @@ import ( // TestBinaryArtifacts tests the binary artifacts check. func TestBinaryArtifacts(t *testing.T) { t.Parallel() - //nolint:govet - type args struct { - name string - dl checker.DetailLogger - r *checker.BinaryArtifactData + lineStart := uint(123) + negativeFinding := finding.Finding{ + Probe: "freeOfUntrustedBinaryArtifacts", + Outcome: finding.OutcomeNegative, + + Location: &finding.Location{ + Path: "path", + Type: finding.FileTypeBinary, + LineStart: &lineStart, + }, } + tests := []struct { name string - findings []finding.Finding - result scut.TestReturn + findings []finding.Finding + result scut.TestReturn }{ { name: "no binary artifacts", - findings: []finding.Finding { + findings: []finding.Finding{ { - Probe: "hasBinaryArtifacts", + Probe: "freeOfUntrustedBinaryArtifacts", Outcome: finding.OutcomePositive, }, }, @@ -50,119 +56,79 @@ func TestBinaryArtifacts(t *testing.T) { }, { name: "one binary artifact", - findings: []finding.Finding { - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, + findings: []finding.Finding{ + negativeFinding, }, result: scut.TestReturn{ - Score: 9, + Score: 9, + NumberOfWarn: 1, }, }, { name: "two binary artifact", - findings: []finding.Finding { + findings: []finding.Finding{ { - Probe: "hasBinaryArtifacts", + Probe: "freeOfUntrustedBinaryArtifacts", Outcome: finding.OutcomeNegative, + Location: &finding.Location{ + Path: "path", + Type: finding.FileTypeBinary, + LineStart: &lineStart, + }, }, { - Probe: "hasBinaryArtifacts", + Probe: "freeOfUntrustedBinaryArtifacts", Outcome: finding.OutcomeNegative, + Location: &finding.Location{ + Path: "path", + Type: finding.FileTypeBinary, + LineStart: &lineStart, + }, }, }, result: scut.TestReturn{ - Score: 8, + Score: 8, + NumberOfWarn: 2, }, }, { name: "five binary artifact", - findings: []finding.Finding { - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, + findings: []finding.Finding{ + negativeFinding, + negativeFinding, + negativeFinding, + negativeFinding, + negativeFinding, }, result: scut.TestReturn{ - Score: 5, + Score: 5, + NumberOfWarn: 5, }, }, { name: "twelve binary artifact", - findings: []finding.Finding { - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "hasBinaryArtifacts", - Outcome: finding.OutcomeNegative, - }, + findings: []finding.Finding{ + negativeFinding, + negativeFinding, + negativeFinding, + negativeFinding, + negativeFinding, + negativeFinding, + negativeFinding, + negativeFinding, + negativeFinding, + negativeFinding, + negativeFinding, + negativeFinding, }, result: scut.TestReturn{ - Score: 0, + Score: 0, + NumberOfWarn: 12, }, }, { - name: "invalid findings", - findings: []finding.Finding {}, + name: "invalid findings", + findings: []finding.Finding{}, result: scut.TestReturn{ Score: -1, Error: sce.ErrScorecardInternal, diff --git a/checks/raw/binary_artifact.go b/checks/raw/binary_artifact.go index fa87e6fe203..a2e2f33f8ee 100644 --- a/checks/raw/binary_artifact.go +++ b/checks/raw/binary_artifact.go @@ -90,9 +90,10 @@ func excludeValidatedGradleWrappers(c clients.RepoClient, files []checker.File) // Remove Gradle wrapper JARs from files. filterFiles := []checker.File{} for _, f := range files { - if filepath.Base(f.Path) != "gradle-wrapper.jar" { - filterFiles = append(filterFiles, f) + if filepath.Base(f.Path) == "gradle-wrapper.jar" { + f.Type = finding.FileTypeGradleWrapper } + filterFiles = append(filterFiles, f) } files = filterFiles return files, nil diff --git a/checks/raw/binary_artifact_test.go b/checks/raw/binary_artifact_test.go index 8cde90496c8..6329cb7cf4b 100644 --- a/checks/raw/binary_artifact_test.go +++ b/checks/raw/binary_artifact_test.go @@ -128,7 +128,7 @@ func TestBinaryArtifacts(t *testing.T) { }, }, getFileContentCount: 3, - expect: 0, + expect: 1, }, { name: "gradle-wrapper.jar with non-verification action", @@ -212,7 +212,7 @@ func TestBinaryArtifacts(t *testing.T) { }, }, getFileContentCount: 3, - expect: 0, + expect: 1, }, } for _, tt := range tests { diff --git a/finding/finding.go b/finding/finding.go index 39f06175cee..635dbfa0b69 100644 --- a/finding/finding.go +++ b/finding/finding.go @@ -40,6 +40,8 @@ const ( FileTypeText // FileTypeURL for URLs. FileTypeURL + // FileTypeGradleWrapper for gradle-wrapper.jar files. + FileTypeGradleWrapper ) // Location represents the location of a finding. diff --git a/probes/entries.go b/probes/entries.go index c5fbaac6b6b..94f19586a81 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -18,6 +18,7 @@ import ( "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/finding" "github.com/ossf/scorecard/v4/probes/contributorsFromOrgOrCompany" + "github.com/ossf/scorecard/v4/probes/freeOfUntrustedBinaryArtifacts" "github.com/ossf/scorecard/v4/probes/fuzzedWithCLibFuzzer" "github.com/ossf/scorecard/v4/probes/fuzzedWithClusterFuzzLite" "github.com/ossf/scorecard/v4/probes/fuzzedWithCppLibFuzzer" @@ -45,7 +46,6 @@ import ( "github.com/ossf/scorecard/v4/probes/sastToolCodeQLInstalled" "github.com/ossf/scorecard/v4/probes/sastToolRunsOnAllCommits" "github.com/ossf/scorecard/v4/probes/sastToolSonarInstalled" - "github.com/ossf/scorecard/v4/probes/hasBinaryArtifacts" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsLinks" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsText" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsVulnerabilityDisclosure" @@ -123,7 +123,7 @@ var ( hasOpenSSFBadge.Run, } BinaryArtifacts = []ProbeImpl{ - hasBinaryArtifacts.Run, + freeOfUntrustedBinaryArtifacts.Run, } ) diff --git a/probes/hasBinaryArtifacts/def.yml b/probes/freeOfAnyBinaryArtifacts/def.yml similarity index 72% rename from probes/hasBinaryArtifacts/def.yml rename to probes/freeOfAnyBinaryArtifacts/def.yml index 809863d2390..dc1d36fd845 100644 --- a/probes/hasBinaryArtifacts/def.yml +++ b/probes/freeOfAnyBinaryArtifacts/def.yml @@ -12,14 +12,15 @@ # See the License for the specific language governing permissions and # limitations under the License. -id: hasBinaryArtifacts -short: Checks if the project has binary files in its source tree. +id: freeOfAnyBinaryArtifacts +short: Checks if the project has any binary files in its source tree. motivation: > Binary files are not readable so users can't see what they do. Many programming language systems can generate executables from source code (e.g., C/C++ generated machine code, Java .class files, Python .pyc files, and minified JavaScript). Users will often directly use executables if they are included in the source repository, leading to many dangerous behaviors. implementation: > - The implementation looks for the presence of binary files. + The implementation looks for the presence of binary files. This is a more restrictive probe than "freeOfUntrustedBinaryArtifacts" which excludes trusted binary files. outcome: - - If the probe finds binary files, it returns a number of negative outcomes equal to the number of binary files found. + - If the probe finds binary files, it returns a number of negative outcomes equal to the number of binary files found. Each outcome includes a location of the file. + - If the probe finds no untrusted binary files, it returns a single positive outcome. remediation: effort: Medium text: diff --git a/probes/hasBinaryArtifacts/impl.go b/probes/freeOfAnyBinaryArtifacts/impl.go similarity index 72% rename from probes/hasBinaryArtifacts/impl.go rename to probes/freeOfAnyBinaryArtifacts/impl.go index 7c332cdcf28..c1588b2d291 100644 --- a/probes/hasBinaryArtifacts/impl.go +++ b/probes/freeOfAnyBinaryArtifacts/impl.go @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -// nolint:stylecheck -package hasBinaryArtifacts +//nolint:stylecheck +package freeOfAnyBinaryArtifacts import ( "embed" @@ -27,7 +27,7 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "hasBinaryArtifacts" +const Probe = "freeOfAnyBinaryArtifacts" func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { @@ -35,41 +35,32 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } r := raw.BinaryArtifactResults + var findings []finding.Finding // Apply the policy evaluation. if r.Files == nil || len(r.Files) == 0 { f, err := finding.NewWith(fs, Probe, - "Repository does not have binary artifacts.", nil, - finding.OutcomePositive) + "Repository does not have any binary artifacts.", nil, + finding.OutcomePositive) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - return []finding.Finding{*f}, Probe, nil + findings = append(findings, *f) } - - var findings []finding.Finding - - for range r.Files { + for i := range r.Files { + file := &r.Files[i] f, err := finding.NewWith(fs, Probe, "binary artifact detected", nil, finding.OutcomeNegative) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } + f = f.WithLocation(&finding.Location{ + Path: file.Path, + LineStart: &file.Offset, + Type: file.Type, + }) findings = append(findings, *f) } - if len(findings) == 0 { - return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) - } - - // Check if there are both positive and negative outcomes. - // This should not happen. - for i := range findings { - f := &findings[i] - if f.Outcome == finding.OutcomePositive { - return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) - } - } - return findings, Probe, nil -} \ No newline at end of file +} diff --git a/probes/freeOfAnyBinaryArtifacts/impl_test.go b/probes/freeOfAnyBinaryArtifacts/impl_test.go new file mode 100644 index 00000000000..64b5c95e1ef --- /dev/null +++ b/probes/freeOfAnyBinaryArtifacts/impl_test.go @@ -0,0 +1,158 @@ +// 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 freeOfAnyBinaryArtifacts + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/finding" +) + +func Test_Run(t *testing.T) { + t.Parallel() + //nolint:govet + tests := []struct { + name string + raw *checker.RawResults + outcomes []finding.Outcome + err error + }{ + { + name: "1 binary artifact", + raw: &checker.RawResults{ + BinaryArtifactResults: checker.BinaryArtifactData{ + Files: []checker.File{ + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeBinary, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + { + name: "No binary artifacts", + raw: &checker.RawResults{ + BinaryArtifactResults: checker.BinaryArtifactData{}, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + }, + { + name: "many binary artifact", + raw: &checker.RawResults{ + BinaryArtifactResults: checker.BinaryArtifactData{ + Files: []checker.File{ + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeBinary, + }, + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeBinary, + }, + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeBinary, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + }, + }, + { + name: "many binary artifact including gradle wrappers", + raw: &checker.RawResults{ + BinaryArtifactResults: checker.BinaryArtifactData{ + Files: []checker.File{ + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeBinary, + }, + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeBinary, + }, + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeBinary, + }, + { + Path: "gradle-wrapper.jar", + Type: finding.FileTypeGradleWrapper, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + finding.OutcomeNegative, + }, + }, + { + name: "Is free of any binary artifacts", + raw: &checker.RawResults{ + BinaryArtifactResults: checker.BinaryArtifactData{ + Files: []checker.File{}, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + }, + } + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + findings, s, err := Run(tt.raw) + if !cmp.Equal(tt.err, err, cmpopts.EquateErrors()) { + t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(tt.err, err, cmpopts.EquateErrors())) + } + if err != nil { + return + } + if diff := cmp.Diff(Probe, s); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(len(tt.outcomes), len(findings)); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + for i := range tt.outcomes { + outcome := &tt.outcomes[i] + f := &findings[i] + if diff := cmp.Diff(*outcome, f.Outcome); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + } + }) + } +} diff --git a/probes/freeOfUntrustedBinaryArtifacts/def.yml b/probes/freeOfUntrustedBinaryArtifacts/def.yml new file mode 100644 index 00000000000..d4ec65a8a23 --- /dev/null +++ b/probes/freeOfUntrustedBinaryArtifacts/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: freeOfUntrustedBinaryArtifacts +short: Checks if the project has binary files in its source tree. The probe skips trusted binary files which currently are gradle-wrappers. +motivation: > + Binary files are not readable so users can't see what they do. Many programming language systems can generate executables from source code (e.g., C/C++ generated machine code, Java .class files, Python .pyc files, and minified JavaScript). Users will often directly use executables if they are included in the source repository, leading to many dangerous behaviors. +implementation: > + The implementation looks for the presence of binary files. This is a more permissive probe than "freeOfAnyBinaryArtifacts" which does not trust any binary files. +outcome: + - If the probe finds untrusted binary files, it returns a number of negative outcomes equal to the number of untrusted binary files found. Each outcome includes a location of the file. + - If the probe finds no untrusted binary files, it returns a single positive outcome. +remediation: + effort: Medium + text: + - Remove the generated executable artifacts from the repository. + - Build from source. \ No newline at end of file diff --git a/probes/freeOfUntrustedBinaryArtifacts/impl.go b/probes/freeOfUntrustedBinaryArtifacts/impl.go new file mode 100644 index 00000000000..70bcef3fc8b --- /dev/null +++ b/probes/freeOfUntrustedBinaryArtifacts/impl.go @@ -0,0 +1,69 @@ +// 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 freeOfUntrustedBinaryArtifacts + +import ( + "embed" + "fmt" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/internal/utils/uerror" +) + +//go:embed *.yml +var fs embed.FS + +const Probe = "freeOfUntrustedBinaryArtifacts" + +func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { + if raw == nil { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + + r := raw.BinaryArtifactResults + + var findings []finding.Finding + + for i := range r.Files { + file := &r.Files[i] + if file.Type == finding.FileTypeGradleWrapper { + continue + } + f, err := finding.NewWith(fs, Probe, "binary artifact detected", + nil, finding.OutcomeNegative) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + f = f.WithLocation(&finding.Location{ + Path: file.Path, + LineStart: &file.Offset, + Type: file.Type, + }) + findings = append(findings, *f) + } + + if len(findings) == 0 { + f, err := finding.NewWith(fs, Probe, + "Repository does not have binary artifacts.", nil, + finding.OutcomePositive) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + } + return findings, Probe, nil +} diff --git a/probes/freeOfUntrustedBinaryArtifacts/impl_test.go b/probes/freeOfUntrustedBinaryArtifacts/impl_test.go new file mode 100644 index 00000000000..90c598cc37a --- /dev/null +++ b/probes/freeOfUntrustedBinaryArtifacts/impl_test.go @@ -0,0 +1,123 @@ +// 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 freeOfUntrustedBinaryArtifacts + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/finding" +) + +func Test_Run(t *testing.T) { + t.Parallel() + + //nolint:govet + tests := []struct { + name string + raw *checker.RawResults + outcomes []finding.Outcome + err error + }{ + { + name: "1 binary artifact", + raw: &checker.RawResults{ + BinaryArtifactResults: checker.BinaryArtifactData{ + Files: []checker.File{ + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeBinary, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + { + name: "Two binary artifacts and one gradle wrapper (which is trusted)", + raw: &checker.RawResults{ + BinaryArtifactResults: checker.BinaryArtifactData{ + Files: []checker.File{ + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeBinary, + }, + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeBinary, + }, + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeGradleWrapper, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + finding.OutcomeNegative, + }, + }, + { + name: "One gradle wrapper (which is trusted)", + raw: &checker.RawResults{ + BinaryArtifactResults: checker.BinaryArtifactData{ + Files: []checker.File{ + { + Path: "test_binary_artifacts_check_pass", + Type: finding.FileTypeGradleWrapper, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + }, + } + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + findings, s, err := Run(tt.raw) + if !cmp.Equal(tt.err, err, cmpopts.EquateErrors()) { + t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(tt.err, err, cmpopts.EquateErrors())) + } + if err != nil { + return + } + if diff := cmp.Diff(Probe, s); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(len(tt.outcomes), len(findings)); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + for i := range tt.outcomes { + outcome := &tt.outcomes[i] + f := &findings[i] + if diff := cmp.Diff(*outcome, f.Outcome); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + } + }) + } +} diff --git a/probes/hasBinaryArtifacts/impl_test.go b/probes/hasBinaryArtifacts/impl_test.go deleted file mode 100644 index 71f23628b4f..00000000000 --- a/probes/hasBinaryArtifacts/impl_test.go +++ /dev/null @@ -1,300 +0,0 @@ -// Copyright 2023 OpenSSF Scorecard Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// nolint:stylecheck -package hasBinaryArtifacts - -import ( - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - - "github.com/ossf/scorecard/v4/checker" - "github.com/ossf/scorecard/v4/finding" -) - -func Test_Run(t *testing.T) { - t.Parallel() - // nolint:govet - tests := []struct { - name string - raw *checker.RawResults - outcomes []finding.Outcome - err error - }{ - { - name: "1 binary artifact", - raw: &checker.RawResults{ - BinaryArtifactResults: checker.BinaryArtifactData{ - Files: []checker.File{ - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - }, - }, - }, - outcomes: []finding.Outcome{ - finding.OutcomeNegative, - }, - }, - { - name: "No binary artifacts", - raw: &checker.RawResults{ - BinaryArtifactResults: checker.BinaryArtifactData{}, - }, - outcomes: []finding.Outcome{ - finding.OutcomePositive, - }, - }, - { - name: "many binary artifact", - raw: &checker.RawResults{ - BinaryArtifactResults: checker.BinaryArtifactData{ - Files: []checker.File{ - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - { - Path: "test_binary_artifacts_check_pass", - Snippet: ` - package main - import "fmt" - func main() { - fmt.Println("Hello, world!") - }i`, - Offset: 0, - Type: 0, - }, - }, - }, - }, - outcomes: []finding.Outcome{ - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - finding.OutcomeNegative, - }, - }, - } - for _, tt := range tests { - tt := tt // Re-initializing variable so it is not changed while executing the closure below - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - findings, s, err := Run(tt.raw) - if !cmp.Equal(tt.err, err, cmpopts.EquateErrors()) { - t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(tt.err, err, cmpopts.EquateErrors())) - } - if err != nil { - return - } - if diff := cmp.Diff(Probe, s); diff != "" { - t.Errorf("mismatch (-want +got):\n%s", diff) - } - if diff := cmp.Diff(len(tt.outcomes), len(findings)); diff != "" { - t.Errorf("mismatch (-want +got):\n%s", diff) - } - for i := range tt.outcomes { - outcome := &tt.outcomes[i] - f := &findings[i] - if diff := cmp.Diff(*outcome, f.Outcome); diff != "" { - t.Errorf("mismatch (-want +got):\n%s", diff) - } - } - }) - } -} From c7b8915f14f4db04664e220b9cb3f3ec2affa4e3 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Fri, 1 Dec 2023 17:30:25 +0000 Subject: [PATCH 07/14] Add description about what Scorecard considers a verified binary Signed-off-by: Adam Korczynski --- probes/freeOfUntrustedBinaryArtifacts/def.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/probes/freeOfUntrustedBinaryArtifacts/def.yml b/probes/freeOfUntrustedBinaryArtifacts/def.yml index d4ec65a8a23..ea8f3f0d0a3 100644 --- a/probes/freeOfUntrustedBinaryArtifacts/def.yml +++ b/probes/freeOfUntrustedBinaryArtifacts/def.yml @@ -17,7 +17,7 @@ short: Checks if the project has binary files in its source tree. The probe skip motivation: > Binary files are not readable so users can't see what they do. Many programming language systems can generate executables from source code (e.g., C/C++ generated machine code, Java .class files, Python .pyc files, and minified JavaScript). Users will often directly use executables if they are included in the source repository, leading to many dangerous behaviors. implementation: > - The implementation looks for the presence of binary files. This is a more permissive probe than "freeOfAnyBinaryArtifacts" which does not trust any binary files. + The implementation looks for the presence of binary files that are not "verified". A verified binary is one that Scorecard considers valid for building and/or releasing the project. This is a more permissive probe than "freeOfAnyBinaryArtifacts" which does not trust any binary files. outcome: - If the probe finds untrusted binary files, it returns a number of negative outcomes equal to the number of untrusted binary files found. Each outcome includes a location of the file. - If the probe finds no untrusted binary files, it returns a single positive outcome. @@ -25,4 +25,4 @@ remediation: effort: Medium text: - Remove the generated executable artifacts from the repository. - - Build from source. \ No newline at end of file + - Build from source. From 37bfaa6153e09bdae323220dd2825a78f021d82e Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Fri, 1 Dec 2023 17:39:23 +0000 Subject: [PATCH 08/14] change 'trusted' to 'verified' Signed-off-by: Adam Korczynski --- checks/evaluation/binary_artifacts.go | 4 ++-- checks/evaluation/binary_artifacts_test.go | 8 ++++---- probes/entries.go | 4 ++-- probes/freeOfAnyBinaryArtifacts/def.yml | 6 +++--- .../def.yml | 10 +++++----- .../impl.go | 4 ++-- .../impl_test.go | 2 +- 7 files changed, 19 insertions(+), 19 deletions(-) rename probes/{freeOfUntrustedBinaryArtifacts => freeOfUnverifiedBinaryArtifacts}/def.yml (77%) rename probes/{freeOfUntrustedBinaryArtifacts => freeOfUnverifiedBinaryArtifacts}/impl.go (95%) rename probes/{freeOfUntrustedBinaryArtifacts => freeOfUnverifiedBinaryArtifacts}/impl_test.go (98%) diff --git a/checks/evaluation/binary_artifacts.go b/checks/evaluation/binary_artifacts.go index 99f519435ad..20599b941e2 100644 --- a/checks/evaluation/binary_artifacts.go +++ b/checks/evaluation/binary_artifacts.go @@ -18,7 +18,7 @@ import ( "github.com/ossf/scorecard/v4/checker" sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/finding" - "github.com/ossf/scorecard/v4/probes/freeOfUntrustedBinaryArtifacts" + "github.com/ossf/scorecard/v4/probes/freeOfUnverifiedBinaryArtifacts" ) // BinaryArtifacts applies the score policy for the Binary-Artifacts check. @@ -27,7 +27,7 @@ func BinaryArtifacts(name string, dl checker.DetailLogger, ) checker.CheckResult { expectedProbes := []string{ - freeOfUntrustedBinaryArtifacts.Probe, + freeOfUnverifiedBinaryArtifacts.Probe, } if !finding.UniqueProbesEqual(findings, expectedProbes) { diff --git a/checks/evaluation/binary_artifacts_test.go b/checks/evaluation/binary_artifacts_test.go index 6ace7f0c346..12bb01b3a1e 100644 --- a/checks/evaluation/binary_artifacts_test.go +++ b/checks/evaluation/binary_artifacts_test.go @@ -27,7 +27,7 @@ func TestBinaryArtifacts(t *testing.T) { t.Parallel() lineStart := uint(123) negativeFinding := finding.Finding{ - Probe: "freeOfUntrustedBinaryArtifacts", + Probe: "freeOfUnverifiedBinaryArtifacts", Outcome: finding.OutcomeNegative, Location: &finding.Location{ @@ -46,7 +46,7 @@ func TestBinaryArtifacts(t *testing.T) { name: "no binary artifacts", findings: []finding.Finding{ { - Probe: "freeOfUntrustedBinaryArtifacts", + Probe: "freeOfUnverifiedBinaryArtifacts", Outcome: finding.OutcomePositive, }, }, @@ -68,7 +68,7 @@ func TestBinaryArtifacts(t *testing.T) { name: "two binary artifact", findings: []finding.Finding{ { - Probe: "freeOfUntrustedBinaryArtifacts", + Probe: "freeOfUnverifiedBinaryArtifacts", Outcome: finding.OutcomeNegative, Location: &finding.Location{ Path: "path", @@ -77,7 +77,7 @@ func TestBinaryArtifacts(t *testing.T) { }, }, { - Probe: "freeOfUntrustedBinaryArtifacts", + Probe: "freeOfUnverifiedBinaryArtifacts", Outcome: finding.OutcomeNegative, Location: &finding.Location{ Path: "path", diff --git a/probes/entries.go b/probes/entries.go index 94f19586a81..c9d125f81e5 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -18,7 +18,7 @@ import ( "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/finding" "github.com/ossf/scorecard/v4/probes/contributorsFromOrgOrCompany" - "github.com/ossf/scorecard/v4/probes/freeOfUntrustedBinaryArtifacts" + "github.com/ossf/scorecard/v4/probes/freeOfUnverifiedBinaryArtifacts" "github.com/ossf/scorecard/v4/probes/fuzzedWithCLibFuzzer" "github.com/ossf/scorecard/v4/probes/fuzzedWithClusterFuzzLite" "github.com/ossf/scorecard/v4/probes/fuzzedWithCppLibFuzzer" @@ -123,7 +123,7 @@ var ( hasOpenSSFBadge.Run, } BinaryArtifacts = []ProbeImpl{ - freeOfUntrustedBinaryArtifacts.Run, + freeOfUnverifiedBinaryArtifacts.Run, } ) diff --git a/probes/freeOfAnyBinaryArtifacts/def.yml b/probes/freeOfAnyBinaryArtifacts/def.yml index dc1d36fd845..858a882f45c 100644 --- a/probes/freeOfAnyBinaryArtifacts/def.yml +++ b/probes/freeOfAnyBinaryArtifacts/def.yml @@ -17,12 +17,12 @@ short: Checks if the project has any binary files in its source tree. motivation: > Binary files are not readable so users can't see what they do. Many programming language systems can generate executables from source code (e.g., C/C++ generated machine code, Java .class files, Python .pyc files, and minified JavaScript). Users will often directly use executables if they are included in the source repository, leading to many dangerous behaviors. implementation: > - The implementation looks for the presence of binary files. This is a more restrictive probe than "freeOfUntrustedBinaryArtifacts" which excludes trusted binary files. + The implementation looks for the presence of binary files. This is a more restrictive probe than "freeOfUnverifiededBinaryArtifacts" which excludes verified binary files. outcome: - If the probe finds binary files, it returns a number of negative outcomes equal to the number of binary files found. Each outcome includes a location of the file. - - If the probe finds no untrusted binary files, it returns a single positive outcome. + - If the probe finds no verified binary files, it returns a single positive outcome. remediation: effort: Medium text: - Remove the generated executable artifacts from the repository. - - Build from source. \ No newline at end of file + - Build from source. diff --git a/probes/freeOfUntrustedBinaryArtifacts/def.yml b/probes/freeOfUnverifiedBinaryArtifacts/def.yml similarity index 77% rename from probes/freeOfUntrustedBinaryArtifacts/def.yml rename to probes/freeOfUnverifiedBinaryArtifacts/def.yml index ea8f3f0d0a3..871475ea7be 100644 --- a/probes/freeOfUntrustedBinaryArtifacts/def.yml +++ b/probes/freeOfUnverifiedBinaryArtifacts/def.yml @@ -12,15 +12,15 @@ # See the License for the specific language governing permissions and # limitations under the License. -id: freeOfUntrustedBinaryArtifacts -short: Checks if the project has binary files in its source tree. The probe skips trusted binary files which currently are gradle-wrappers. +id: freeOfUnverifiedBinaryArtifacts +short: Checks if the project has binary files in its source tree. The probe skips verified binary files which currently are gradle-wrappers. motivation: > Binary files are not readable so users can't see what they do. Many programming language systems can generate executables from source code (e.g., C/C++ generated machine code, Java .class files, Python .pyc files, and minified JavaScript). Users will often directly use executables if they are included in the source repository, leading to many dangerous behaviors. implementation: > - The implementation looks for the presence of binary files that are not "verified". A verified binary is one that Scorecard considers valid for building and/or releasing the project. This is a more permissive probe than "freeOfAnyBinaryArtifacts" which does not trust any binary files. + The implementation looks for the presence of binary files that are not "verified". A verified binary is one that Scorecard considers valid for building and/or releasing the project. This is a more permissive probe than "freeOfAnyBinaryArtifacts" which does not skip verified binary files. outcome: - - If the probe finds untrusted binary files, it returns a number of negative outcomes equal to the number of untrusted binary files found. Each outcome includes a location of the file. - - If the probe finds no untrusted binary files, it returns a single positive outcome. + - If the probe finds unverified binary files, it returns a number of negative outcomes equal to the number of unverified binary files found. Each outcome includes a location of the file. + - If the probe finds no unverified binary files, it returns a single positive outcome. remediation: effort: Medium text: diff --git a/probes/freeOfUntrustedBinaryArtifacts/impl.go b/probes/freeOfUnverifiedBinaryArtifacts/impl.go similarity index 95% rename from probes/freeOfUntrustedBinaryArtifacts/impl.go rename to probes/freeOfUnverifiedBinaryArtifacts/impl.go index 70bcef3fc8b..2a6ca21c4ed 100644 --- a/probes/freeOfUntrustedBinaryArtifacts/impl.go +++ b/probes/freeOfUnverifiedBinaryArtifacts/impl.go @@ -13,7 +13,7 @@ // limitations under the License. //nolint:stylecheck -package freeOfUntrustedBinaryArtifacts +package freeOfUnverifiedBinaryArtifacts import ( "embed" @@ -27,7 +27,7 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "freeOfUntrustedBinaryArtifacts" +const Probe = "freeOfUnverifiedBinaryArtifacts" func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { diff --git a/probes/freeOfUntrustedBinaryArtifacts/impl_test.go b/probes/freeOfUnverifiedBinaryArtifacts/impl_test.go similarity index 98% rename from probes/freeOfUntrustedBinaryArtifacts/impl_test.go rename to probes/freeOfUnverifiedBinaryArtifacts/impl_test.go index 90c598cc37a..0b5f52a65aa 100644 --- a/probes/freeOfUntrustedBinaryArtifacts/impl_test.go +++ b/probes/freeOfUnverifiedBinaryArtifacts/impl_test.go @@ -13,7 +13,7 @@ // limitations under the License. //nolint:stylecheck -package freeOfUntrustedBinaryArtifacts +package freeOfUnverifiedBinaryArtifacts import ( "testing" From 7a63302e1646be71a4d58f82cd3c575f6af0ad42 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Mon, 4 Dec 2023 13:41:31 +0000 Subject: [PATCH 09/14] remove nil check Signed-off-by: Adam Korczynski --- probes/freeOfAnyBinaryArtifacts/impl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probes/freeOfAnyBinaryArtifacts/impl.go b/probes/freeOfAnyBinaryArtifacts/impl.go index c1588b2d291..19df5a131c4 100644 --- a/probes/freeOfAnyBinaryArtifacts/impl.go +++ b/probes/freeOfAnyBinaryArtifacts/impl.go @@ -38,7 +38,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { var findings []finding.Finding // Apply the policy evaluation. - if r.Files == nil || len(r.Files) == 0 { + if len(r.Files) == 0 { f, err := finding.NewWith(fs, Probe, "Repository does not have any binary artifacts.", nil, finding.OutcomePositive) From 7cb09fcc6543fdebc8ea57e337aa79c0dd326cc6 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Mon, 4 Dec 2023 13:44:54 +0000 Subject: [PATCH 10/14] remove filtering Signed-off-by: Adam Korczynski --- checks/raw/binary_artifact.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/checks/raw/binary_artifact.go b/checks/raw/binary_artifact.go index a2e2f33f8ee..3c2461e3985 100644 --- a/checks/raw/binary_artifact.go +++ b/checks/raw/binary_artifact.go @@ -88,14 +88,11 @@ func excludeValidatedGradleWrappers(c clients.RepoClient, files []checker.File) } // It has been confirmed that latest commit has validated JARs! // Remove Gradle wrapper JARs from files. - filterFiles := []checker.File{} - for _, f := range files { - if filepath.Base(f.Path) == "gradle-wrapper.jar" { - f.Type = finding.FileTypeGradleWrapper + for i := range files { + if filepath.Base(files[i].Path) == "gradle-wrapper.jar" { + files[i].Type = finding.FileTypeGradleWrapper } - filterFiles = append(filterFiles, f) } - files = filterFiles return files, nil } From 4b91026d414dc24b82279676ca6a794d038d6c58 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Mon, 4 Dec 2023 13:47:44 +0000 Subject: [PATCH 11/14] use const scores in tests Signed-off-by: Adam Korczynski --- checks/evaluation/binary_artifacts_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/checks/evaluation/binary_artifacts_test.go b/checks/evaluation/binary_artifacts_test.go index 12bb01b3a1e..011eae6f812 100644 --- a/checks/evaluation/binary_artifacts_test.go +++ b/checks/evaluation/binary_artifacts_test.go @@ -17,6 +17,7 @@ package evaluation import ( "testing" + "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" @@ -51,7 +52,7 @@ func TestBinaryArtifacts(t *testing.T) { }, }, result: scut.TestReturn{ - Score: 10, + Score: checker.MaxResultScore, }, }, { @@ -122,7 +123,7 @@ func TestBinaryArtifacts(t *testing.T) { negativeFinding, }, result: scut.TestReturn{ - Score: 0, + Score: checker.MinResultScore, NumberOfWarn: 12, }, }, @@ -130,7 +131,7 @@ func TestBinaryArtifacts(t *testing.T) { name: "invalid findings", findings: []finding.Finding{}, result: scut.TestReturn{ - Score: -1, + Score: checker.InconclusiveResultScore, Error: sce.ErrScorecardInternal, }, }, From 4eb17db4033db1fc24939efa834bc4d21cf9f1eb Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Mon, 4 Dec 2023 13:49:02 +0000 Subject: [PATCH 12/14] rename test Signed-off-by: Adam Korczynski --- checks/evaluation/binary_artifacts_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks/evaluation/binary_artifacts_test.go b/checks/evaluation/binary_artifacts_test.go index 011eae6f812..d2f9c4f8727 100644 --- a/checks/evaluation/binary_artifacts_test.go +++ b/checks/evaluation/binary_artifacts_test.go @@ -107,7 +107,7 @@ func TestBinaryArtifacts(t *testing.T) { }, }, { - name: "twelve binary artifact", + name: "twelve binary artifact - ensure score doesn't drop below min", findings: []finding.Finding{ negativeFinding, negativeFinding, From 07e38c74a4e884241723bbc949e00513b14301d3 Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Mon, 4 Dec 2023 13:52:32 +0000 Subject: [PATCH 13/14] add sanity check in loop Signed-off-by: Adam Korczynski --- checks/evaluation/binary_artifacts.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/checks/evaluation/binary_artifacts.go b/checks/evaluation/binary_artifacts.go index 20599b941e2..fff943fb2a1 100644 --- a/checks/evaluation/binary_artifacts.go +++ b/checks/evaluation/binary_artifacts.go @@ -41,6 +41,9 @@ func BinaryArtifacts(name string, for i := range findings { f := &findings[i] + if f.Outcome != finding.OutcomeNegative { + continue + } dl.Warn(&checker.LogMessage{ Path: f.Location.Path, Type: f.Location.Type, From 3e5274ca88cc2261c5adb4cf3abc6b83707037ad Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Mon, 4 Dec 2023 14:04:52 +0000 Subject: [PATCH 14/14] rename binary file const Signed-off-by: Adam Korczynski --- checks/raw/binary_artifact.go | 2 +- finding/finding.go | 4 ++-- probes/freeOfAnyBinaryArtifacts/impl_test.go | 2 +- probes/freeOfUnverifiedBinaryArtifacts/impl.go | 2 +- probes/freeOfUnverifiedBinaryArtifacts/impl_test.go | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/checks/raw/binary_artifact.go b/checks/raw/binary_artifact.go index 3c2461e3985..c2fc925c097 100644 --- a/checks/raw/binary_artifact.go +++ b/checks/raw/binary_artifact.go @@ -90,7 +90,7 @@ func excludeValidatedGradleWrappers(c clients.RepoClient, files []checker.File) // Remove Gradle wrapper JARs from files. for i := range files { if filepath.Base(files[i].Path) == "gradle-wrapper.jar" { - files[i].Type = finding.FileTypeGradleWrapper + files[i].Type = finding.FileTypeBinaryVerified } } return files, nil diff --git a/finding/finding.go b/finding/finding.go index 635dbfa0b69..761fb876f16 100644 --- a/finding/finding.go +++ b/finding/finding.go @@ -40,8 +40,8 @@ const ( FileTypeText // FileTypeURL for URLs. FileTypeURL - // FileTypeGradleWrapper for gradle-wrapper.jar files. - FileTypeGradleWrapper + // FileTypeBinaryVerified for verified binary files. + FileTypeBinaryVerified ) // Location represents the location of a finding. diff --git a/probes/freeOfAnyBinaryArtifacts/impl_test.go b/probes/freeOfAnyBinaryArtifacts/impl_test.go index 64b5c95e1ef..94046cb5fb7 100644 --- a/probes/freeOfAnyBinaryArtifacts/impl_test.go +++ b/probes/freeOfAnyBinaryArtifacts/impl_test.go @@ -104,7 +104,7 @@ func Test_Run(t *testing.T) { }, { Path: "gradle-wrapper.jar", - Type: finding.FileTypeGradleWrapper, + Type: finding.FileTypeBinaryVerified, }, }, }, diff --git a/probes/freeOfUnverifiedBinaryArtifacts/impl.go b/probes/freeOfUnverifiedBinaryArtifacts/impl.go index 2a6ca21c4ed..ad934e50498 100644 --- a/probes/freeOfUnverifiedBinaryArtifacts/impl.go +++ b/probes/freeOfUnverifiedBinaryArtifacts/impl.go @@ -40,7 +40,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for i := range r.Files { file := &r.Files[i] - if file.Type == finding.FileTypeGradleWrapper { + if file.Type == finding.FileTypeBinaryVerified { continue } f, err := finding.NewWith(fs, Probe, "binary artifact detected", diff --git a/probes/freeOfUnverifiedBinaryArtifacts/impl_test.go b/probes/freeOfUnverifiedBinaryArtifacts/impl_test.go index 0b5f52a65aa..ce03d7400bc 100644 --- a/probes/freeOfUnverifiedBinaryArtifacts/impl_test.go +++ b/probes/freeOfUnverifiedBinaryArtifacts/impl_test.go @@ -66,7 +66,7 @@ func Test_Run(t *testing.T) { }, { Path: "test_binary_artifacts_check_pass", - Type: finding.FileTypeGradleWrapper, + Type: finding.FileTypeBinaryVerified, }, }, }, @@ -83,7 +83,7 @@ func Test_Run(t *testing.T) { Files: []checker.File{ { Path: "test_binary_artifacts_check_pass", - Type: finding.FileTypeGradleWrapper, + Type: finding.FileTypeBinaryVerified, }, }, },