Skip to content

Commit

Permalink
Replace .Incomplete []error with .ProcessingErrors []ElementError
Browse files Browse the repository at this point in the history
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
  • Loading branch information
pnacht committed Nov 6, 2023
1 parent 01f3fe2 commit 74f9892
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 86 deletions.
4 changes: 2 additions & 2 deletions checker/raw_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ const (

// PinningDependenciesData represents pinned dependency data.
type PinningDependenciesData struct {
Dependencies []Dependency
Incomplete []error // jobs or files with errors may have incomplete results
Dependencies []Dependency
ProcessingErrors []ElementError // jobs or files with errors may have incomplete results
}

// Dependency represents a dependency.
Expand Down
12 changes: 8 additions & 4 deletions checks/evaluation/pinned_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"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/remediation"
"github.com/ossf/scorecard/v4/rule"
)
Expand Down Expand Up @@ -65,9 +66,12 @@ func PinningDependencies(name string, c *checker.CheckRequest,
//nolint:errcheck
remediationMetadata, _ := remediation.New(c)

for _, incomplete := range r.Incomplete {
for _, e := range r.ProcessingErrors {
dl.Info(&checker.LogMessage{
Text: generateTextIncompleteResults(incomplete),
Finding: &finding.Finding{
Message: generateTextIncompleteResults(e),
Location: e.Element,
},
})
}

Expand Down Expand Up @@ -193,8 +197,8 @@ func generateTextUnpinned(rr *checker.Dependency) string {
return fmt.Sprintf("%s not pinned by hash", rr.Type)
}

func generateTextIncompleteResults(e error) string {
return fmt.Sprintf("Possibly incomplete results: %s", e)
func generateTextIncompleteResults(e checker.ElementError) string {
return fmt.Sprintf("Possibly incomplete results: %s", e.Err)
}

func generateOwnerToDisplay(gitHubOwned bool) string {
Expand Down
23 changes: 12 additions & 11 deletions checks/evaluation/pinned_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"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"
)

Expand Down Expand Up @@ -239,10 +240,10 @@ func Test_PinningDependencies(t *testing.T) {
t.Parallel()

tests := []struct {
name string
dependencies []checker.Dependency
incomplete []error
expected scut.TestReturn
name string
dependencies []checker.Dependency
processingErrors []checker.ElementError
expected scut.TestReturn
}{
{
name: "all dependencies pinned",
Expand Down Expand Up @@ -811,17 +812,17 @@ func Test_PinningDependencies(t *testing.T) {
Pinned: asBoolPointer(false),
},
},
incomplete: []error{
sce.ErrorJobOSParsing,
&checker.ElementError{
Err: sce.ErrorJobOSParsing,
processingErrors: []checker.ElementError{
{
Err: sce.ErrorJobOSParsing,
Element: &finding.Location{},
},
},
expected: scut.TestReturn{
Error: nil,
Score: 0,
NumberOfWarn: 2, // unpinned deps
NumberOfInfo: 3, // 1 for npm deps, 2 for incomplete results
NumberOfInfo: 3, // 1 for npm deps, 2 for processing errors
NumberOfDebug: 0,
},
},
Expand All @@ -836,8 +837,8 @@ func Test_PinningDependencies(t *testing.T) {
c := checker.CheckRequest{Dlogger: &dl}
actual := PinningDependencies("checkname", &c,
&checker.PinningDependenciesData{
Dependencies: tt.dependencies,
Incomplete: tt.incomplete,
Dependencies: tt.dependencies,
ProcessingErrors: tt.processingErrors,
})

if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &actual, &dl) {
Expand Down
10 changes: 5 additions & 5 deletions checks/raw/pinned_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,18 +384,18 @@ var validateGitHubWorkflowIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFile
// https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idstepsrun.
shell, err := fileparser.GetShellForStep(step, job)
if err != nil {
var unknownOSError *checker.ElementError
if errors.As(err, &unknownOSError) {
var elementError *checker.ElementError
if errors.As(err, &elementError) {
// Add the workflow name and step ID to the element
lineStart := uint(step.Pos.Line)
unknownOSError.Element = &finding.Location{
elementError.Element = &finding.Location{
Path: pathfn,
Snippet: unknownOSError.Element.Snippet,
Snippet: elementError.Element.Snippet,
LineStart: &lineStart,
Type: finding.FileTypeSource,
}

pdata.Incomplete = append(pdata.Incomplete, unknownOSError)
pdata.ProcessingErrors = append(pdata.ProcessingErrors, *elementError)

// continue instead of break because other `run` steps may declare
// a valid shell we can scan
Expand Down
124 changes: 62 additions & 62 deletions checks/raw/pinned_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,22 +268,22 @@ func TestGithubWorkflowPkgManagerPinning(t *testing.T) {

//nolint
tests := []struct {
unpinned int
incomplete int
err error
name string
filename string
unpinned int
processingErrors int
err error
name string
filename string
}{
{
name: "npm packages without verification",
filename: "./testdata/.github/workflows/github-workflow-pkg-managers.yaml",
unpinned: 49,
},
{
name: "Can't identify OS but doesn't crash",
filename: "./testdata/.github/workflows/github-workflow-unknown-os.yaml",
incomplete: 1, // job with unknown OS is skipped
unpinned: 1, // only 1 in job with known OS, since other job is skipped
name: "Can't identify OS but doesn't crash",
filename: "./testdata/.github/workflows/github-workflow-unknown-os.yaml",
processingErrors: 1, // job with unknown OS is skipped
unpinned: 1, // only 1 in job with known OS, since other job is skipped
},
}
for _, tt := range tests {
Expand Down Expand Up @@ -316,8 +316,8 @@ func TestGithubWorkflowPkgManagerPinning(t *testing.T) {
t.Errorf("expected %v unpinned. Got %v", tt.unpinned, unpinned)
}

if tt.incomplete != len(r.Incomplete) {
t.Errorf("expected %v incomplete. Got %v", tt.incomplete, len(r.Incomplete))
if tt.processingErrors != len(r.ProcessingErrors) {
t.Errorf("expected %v processing errors. Got %v", tt.processingErrors, len(r.ProcessingErrors))
}
})
}
Expand Down Expand Up @@ -576,10 +576,10 @@ func TestDockerfileInsecureDownloadsLineNumber(t *testing.T) {
t.Parallel()
//nolint
tests := []struct {
name string
filename string
incomplete int
expected []struct {
name string
filename string
processingErrors int
expected []struct {
snippet string
startLine uint
endLine uint
Expand Down Expand Up @@ -707,9 +707,9 @@ func TestDockerfileInsecureDownloadsLineNumber(t *testing.T) {
},
},
{
name: "Parser error may lead to incomplete data",
filename: "./testdata/Dockerfile-not-pinned-with-parser-error",
incomplete: 1,
name: "Parser error may lead to incomplete data",
filename: "./testdata/Dockerfile-not-pinned-with-parser-error",
processingErrors: 1,
expected: []struct {
snippet string
startLine uint
Expand Down Expand Up @@ -761,8 +761,8 @@ func TestDockerfileInsecureDownloadsLineNumber(t *testing.T) {
}
}

if tt.incomplete != len(r.Incomplete) {
t.Errorf("expected %v incomplete. Got %v", tt.incomplete, len(r.Incomplete))
if tt.processingErrors != len(r.ProcessingErrors) {
t.Errorf("expected %v processing errors. Got %v", tt.processingErrors, len(r.ProcessingErrors))
}
})
}
Expand Down Expand Up @@ -1012,11 +1012,11 @@ func TestDockerfileScriptDownload(t *testing.T) {
t.Parallel()
//nolint
tests := []struct {
unpinned int
incomplete int
err error
name string
filename string
unpinned int
processingErrors int
err error
name string
filename string
}{
{
name: "curl | sh",
Expand Down Expand Up @@ -1080,10 +1080,10 @@ func TestDockerfileScriptDownload(t *testing.T) {
unpinned: 1,
},
{
name: "Parser error doesn't affect docker image pinning",
filename: "./testdata/Dockerfile-not-pinned-with-parser-error",
incomplete: 1,
unpinned: 2, // `curl bla | bash` missed due to parser error
name: "Parser error doesn't affect docker image pinning",
filename: "./testdata/Dockerfile-not-pinned-with-parser-error",
processingErrors: 1,
unpinned: 2, // `curl bla | bash` missed due to parser error
},
}
for _, tt := range tests {
Expand Down Expand Up @@ -1117,8 +1117,8 @@ func TestDockerfileScriptDownload(t *testing.T) {
t.Errorf("expected %v unpinned. Got %v", tt.unpinned, unpinned)
}

if tt.incomplete != len(r.Incomplete) {
t.Errorf("expected %v incomplete. Got %v", tt.incomplete, len(r.Incomplete))
if tt.processingErrors != len(r.ProcessingErrors) {
t.Errorf("expected %v processing errors. Got %v", tt.processingErrors, len(r.ProcessingErrors))
}
})
}
Expand All @@ -1128,20 +1128,20 @@ func TestDockerfileScriptDownloadInfo(t *testing.T) {
t.Parallel()
//nolint
tests := []struct {
name string
filename string
unpinned int
incomplete int
err error
name string
filename string
unpinned int
processingErrors int
err error
}{
{
name: "curl | sh",
filename: "./testdata/Dockerfile-no-curl-sh",
},
{
name: "Parser error doesn't affect docker image pinning",
filename: "./testdata/Dockerfile-no-curl-sh-with-parser-error",
incomplete: 1, // everything is pinned, but parser error still throws warning
name: "Parser error doesn't affect docker image pinning",
filename: "./testdata/Dockerfile-no-curl-sh-with-parser-error",
processingErrors: 1, // everything is pinned, but parser error still throws warning
},
}
for _, tt := range tests {
Expand Down Expand Up @@ -1171,8 +1171,8 @@ func TestDockerfileScriptDownloadInfo(t *testing.T) {
t.Errorf("expected %v unpinned. Got %v", tt.unpinned, unpinned)
}

if tt.incomplete != len(r.Incomplete) {
t.Errorf("expected %v incomplete. Got %v", tt.incomplete, len(r.Incomplete))
if tt.processingErrors != len(r.ProcessingErrors) {
t.Errorf("expected %v processing errors. Got %v", tt.processingErrors, len(r.ProcessingErrors))
}
})
}
Expand All @@ -1182,11 +1182,11 @@ func TestShellScriptDownload(t *testing.T) {
t.Parallel()
//nolint
tests := []struct {
name string
filename string
unpinned int
incomplete int
err error
name string
filename string
unpinned int
processingErrors int
err error
}{
{
name: "sh script",
Expand Down Expand Up @@ -1217,9 +1217,9 @@ func TestShellScriptDownload(t *testing.T) {
unpinned: 56,
},
{
name: "invalid shell script",
filename: "./testdata/script-invalid.sh",
incomplete: 1, // `curl bla | bash` not detected due to invalid script
name: "invalid shell script",
filename: "./testdata/script-invalid.sh",
processingErrors: 1, // `curl bla | bash` not detected due to invalid script
},
}
for _, tt := range tests {
Expand Down Expand Up @@ -1254,8 +1254,8 @@ func TestShellScriptDownload(t *testing.T) {
t.Errorf("expected %v unpinned. Got %v", tt.unpinned, len(r.Dependencies))
}

if tt.incomplete != len(r.Incomplete) {
t.Errorf("expected %v incomplete. Got %v", tt.incomplete, len(r.Incomplete))
if tt.processingErrors != len(r.ProcessingErrors) {
t.Errorf("expected %v processing errors. Got %v", tt.processingErrors, len(r.ProcessingErrors))
}
})
}
Expand Down Expand Up @@ -1315,11 +1315,11 @@ func TestGitHubWorflowRunDownload(t *testing.T) {
t.Parallel()
//nolint
tests := []struct {
name string
filename string
unpinned int
incomplete int
err error
name string
filename string
unpinned int
processingErrors int
err error
}{
{
name: "workflow curl default",
Expand All @@ -1337,10 +1337,10 @@ func TestGitHubWorflowRunDownload(t *testing.T) {
unpinned: 2,
},
{
name: "Can't identify OS but doesn't crash",
filename: "./testdata/.github/workflows/github-workflow-unknown-os.yaml",
incomplete: 1, // job with unknown OS has a skipped step
unpinned: 1, // only found in 1 in job with known OS
name: "Can't identify OS but doesn't crash",
filename: "./testdata/.github/workflows/github-workflow-unknown-os.yaml",
processingErrors: 1, // job with unknown OS has a skipped step
unpinned: 1, // only found in 1 in job with known OS
},
}
for _, tt := range tests {
Expand Down Expand Up @@ -1376,8 +1376,8 @@ func TestGitHubWorflowRunDownload(t *testing.T) {
t.Errorf("expected %v unpinned. Got %v", tt.unpinned, unpinned)
}

if tt.incomplete != len(r.Incomplete) {
t.Errorf("expected %v incomplete. Got %v", tt.incomplete, len(r.Incomplete))
if tt.processingErrors != len(r.ProcessingErrors) {
t.Errorf("expected %v processing errors. Got %v", tt.processingErrors, len(r.ProcessingErrors))
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion checks/raw/shell_download_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ func validateShellFileAndRecord(pathfn string, startLine, endLine uint, content
var parseError syntax.ParseError
if errors.As(err, &parseError) {
content := string(content)
r.Incomplete = append(r.Incomplete, &checker.ElementError{
r.ProcessingErrors = append(r.ProcessingErrors, checker.ElementError{
Err: sce.WithMessage(sce.ErrorShellParsing, parseError.Text),
Element: &finding.Location{
Path: pathfn,
Expand Down
2 changes: 1 addition & 1 deletion checks/raw/shell_download_validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func TestValidateShellFile(t *testing.T) {
t.Errorf("error validating shell file")
}

if r.Incomplete == nil {
if r.ProcessingErrors == nil {
t.Errorf("failed to register shell parsing error")
}
}
Expand Down

0 comments on commit 74f9892

Please sign in to comment.