From 2b206dc3654ab96b88310625c5f0a1a3902723e7 Mon Sep 17 00:00:00 2001 From: Azeem Shaikh Date: Tue, 15 Feb 2022 10:26:06 -0800 Subject: [PATCH] Remove `Version` field from LogMessage (#1640) Co-authored-by: Azeem Shaikh --- checker/check_result.go | 2 - checks/ci_tests.go | 5 +- checks/contributors.go | 3 +- checks/dangerous_workflow.go | 27 +++-- checks/evaluation/binary_artifacts.go | 5 +- checks/evaluation/code_review.go | 15 +-- checks/evaluation/dependency_update_tool.go | 11 +- checks/evaluation/security_policy.go | 7 +- checks/evaluation/vulnerabilities.go | 3 +- checks/license.go | 7 +- checks/packaging.go | 39 +++---- checks/permissions.go | 108 +++++++++----------- checks/pinned_dependencies.go | 17 ++- checks/sast.go | 31 +++--- checks/shell_download_validate.go | 6 -- checks/signed_releases.go | 20 ++-- cron/format/json_test.go | 28 ----- pkg/common.go | 30 ++---- pkg/json_test.go | 28 ----- pkg/sarif_test.go | 8 +- pkg/testdata/check6.sarif | 2 +- utests/utlib.go | 3 - 22 files changed, 139 insertions(+), 266 deletions(-) diff --git a/checker/check_result.go b/checker/check_result.go index 2ff95667..495f60f6 100644 --- a/checker/check_result.go +++ b/checker/check_result.go @@ -110,8 +110,6 @@ type LogMessage struct { Offset uint // Offset in the file of Path (line for source/text files). EndOffset uint // End of offset in the file, e.g. if the command spans multiple lines. Snippet string // Snippet of code - // UPGRADEv3: to remove. - Version int // `3` to indicate the detail was logged using new structure. } // CreateProportionalScore creates a proportional score. diff --git a/checks/ci_tests.go b/checks/ci_tests.go index 2b87cb41..a821723f 100644 --- a/checks/ci_tests.go +++ b/checks/ci_tests.go @@ -84,8 +84,7 @@ func CITests(c *checker.CheckRequest) checker.CheckResult { if !foundCI { c.Dlogger.Debug(&checker.LogMessage{ - Text: fmt.Sprintf("merged PR without CI test: %d", pr.Number), - Version: 3, + Text: fmt.Sprintf("merged PR without CI test: %d", pr.Number), }) } } @@ -115,7 +114,6 @@ func prHasSuccessStatus(pr *clients.PullRequest, c *checker.CheckRequest) (bool, Type: checker.FileTypeURL, Text: fmt.Sprintf("CI test found: pr: %d, context: %s", pr.Number, status.Context), - Version: 3, }) return true, nil } @@ -143,7 +141,6 @@ func prHasSuccessfulCheck(pr *clients.PullRequest, c *checker.CheckRequest) (boo Type: checker.FileTypeURL, Text: fmt.Sprintf("CI test found: pr: %d, context: %s", pr.Number, cr.App.Slug), - Version: 3, }) return true, nil } diff --git a/checks/contributors.go b/checks/contributors.go index e979f3dc..3c938932 100644 --- a/checks/contributors.go +++ b/checks/contributors.go @@ -74,8 +74,7 @@ func Contributors(c *checker.CheckRequest) checker.CheckResult { } c.Dlogger.Info(&checker.LogMessage{ - Text: fmt.Sprintf("contributors work for: %v", strings.Join(names, ",")), - Version: 3, + Text: fmt.Sprintf("contributors work for: %v", strings.Join(names, ",")), }) reason := fmt.Sprintf("%d different companies found", len(companies)) diff --git a/checks/dangerous_workflow.go b/checks/dangerous_workflow.go index 5e6923d9..4731e72f 100644 --- a/checks/dangerous_workflow.go +++ b/checks/dangerous_workflow.go @@ -331,11 +331,10 @@ func checkJobForUntrustedCodeCheckout(job *actionlint.Job, path string, if strings.Contains(ref.Value.Value, checkoutUntrustedRef) { line := fileparser.GetLineNumber(step.Pos) dl.Warn(&checker.LogMessage{ - Path: path, - Type: checker.FileTypeSource, - Offset: line, - Text: fmt.Sprintf("untrusted code checkout '%v'", ref.Value.Value), - Version: 3, + Path: path, + Type: checker.FileTypeSource, + Offset: line, + Text: fmt.Sprintf("untrusted code checkout '%v'", ref.Value.Value), // TODO: set Snippet. }) // Detected untrusted checkout. @@ -445,11 +444,10 @@ func checkSecretInScript(script string, pos *actionlint.Pos, path string, if strings.Contains(variable, "secrets.") { line := fileparser.GetLineNumber(pos) dl.Warn(&checker.LogMessage{ - Path: path, - Type: checker.FileTypeSource, - Offset: line, - Text: fmt.Sprintf("secret accessible to pull requests '%v'", variable), - Version: 3, + Path: path, + Type: checker.FileTypeSource, + Offset: line, + Text: fmt.Sprintf("secret accessible to pull requests '%v'", variable), // TODO: set Snippet. }) pdata.workflowPattern[secretsViaPullRequests] = true @@ -477,11 +475,10 @@ func checkVariablesInScript(script string, pos *actionlint.Pos, path string, if containsUntrustedContextPattern(variable) { line := fileparser.GetLineNumber(pos) dl.Warn(&checker.LogMessage{ - Path: path, - Type: checker.FileTypeSource, - Offset: line, - Text: fmt.Sprintf("script injection with untrusted input '%v'", variable), - Version: 3, + Path: path, + Type: checker.FileTypeSource, + Offset: line, + Text: fmt.Sprintf("script injection with untrusted input '%v'", variable), // TODO: set Snippet. }) pdata.workflowPattern[scriptInjection] = true diff --git a/checks/evaluation/binary_artifacts.go b/checks/evaluation/binary_artifacts.go index a347f5b4..d9fd97f8 100644 --- a/checks/evaluation/binary_artifacts.go +++ b/checks/evaluation/binary_artifacts.go @@ -36,9 +36,8 @@ func BinaryArtifacts(name string, dl checker.DetailLogger, for _, f := range r.Files { dl.Warn(&checker.LogMessage{ Path: f.Path, Type: checker.FileTypeBinary, - Offset: f.Offset, - Text: "binary detected", - Version: 3, + Offset: f.Offset, + Text: "binary detected", }) // We remove one point for each binary. score-- diff --git a/checks/evaluation/code_review.go b/checks/evaluation/code_review.go index 06975002..e21bfb8b 100644 --- a/checks/evaluation/code_review.go +++ b/checks/evaluation/code_review.go @@ -53,8 +53,7 @@ func CodeReview(name string, dl checker.DetailLogger, rs := getApprovedReviewSystem(&commit, dl) if rs == "" { dl.Warn(&checker.LogMessage{ - Text: fmt.Sprintf("no reviews found for commit: %s", commit.SHA), - Version: 3, + Text: fmt.Sprintf("no reviews found for commit: %s", commit.SHA), }) continue } @@ -127,7 +126,6 @@ func isReviewedOnGitHub(c *checker.DefaultBranchCommit, dl checker.DetailLogger) dl.Debug(&checker.LogMessage{ Text: fmt.Sprintf("commit %s was reviewed through %s #%d approved merge request", c.SHA, reviewPlatformGitHub, mr.Number), - Version: 3, }) return true } @@ -141,7 +139,6 @@ func isReviewedOnGitHub(c *checker.DefaultBranchCommit, dl checker.DetailLogger) dl.Debug(&checker.LogMessage{ Text: fmt.Sprintf("commit %s was reviewed through %s #%d merge request", c.SHA, reviewPlatformGitHub, mr.Number), - Version: 3, }) return true } @@ -152,8 +149,7 @@ func isReviewedOnGitHub(c *checker.DefaultBranchCommit, dl checker.DetailLogger) func isReviewedOnProw(c *checker.DefaultBranchCommit, dl checker.DetailLogger) bool { if isBot(c.Committer.Login) { dl.Debug(&checker.LogMessage{ - Text: fmt.Sprintf("skip commit %s from bot account: %s", c.SHA, c.Committer.Login), - Version: 3, + Text: fmt.Sprintf("skip commit %s from bot account: %s", c.SHA, c.Committer.Login), }) return true } @@ -164,7 +160,6 @@ func isReviewedOnProw(c *checker.DefaultBranchCommit, dl checker.DetailLogger) b dl.Debug(&checker.LogMessage{ Text: fmt.Sprintf("commit %s review was through %s #%d approved merge request", c.SHA, reviewPlatformProw, c.MergeRequest.Number), - Version: 3, }) return true } @@ -176,8 +171,7 @@ func isReviewedOnProw(c *checker.DefaultBranchCommit, dl checker.DetailLogger) b func isReviewedOnGerrit(c *checker.DefaultBranchCommit, dl checker.DetailLogger) bool { if isBot(c.Committer.Login) { dl.Debug(&checker.LogMessage{ - Text: fmt.Sprintf("skip commit %s from bot account: %s", c.SHA, c.Committer.Login), - Version: 3, + Text: fmt.Sprintf("skip commit %s from bot account: %s", c.SHA, c.Committer.Login), }) return true } @@ -186,8 +180,7 @@ func isReviewedOnGerrit(c *checker.DefaultBranchCommit, dl checker.DetailLogger) if strings.Contains(m, "\nReviewed-on: ") && strings.Contains(m, "\nReviewed-by: ") { dl.Debug(&checker.LogMessage{ - Text: fmt.Sprintf("commit %s was approved through %s", c.SHA, reviewPlatformGerrit), - Version: 3, + Text: fmt.Sprintf("commit %s was approved through %s", c.SHA, reviewPlatformGerrit), }) return true } diff --git a/checks/evaluation/dependency_update_tool.go b/checks/evaluation/dependency_update_tool.go index 6ecb7c6c..6c81e913 100644 --- a/checks/evaluation/dependency_update_tool.go +++ b/checks/evaluation/dependency_update_tool.go @@ -34,12 +34,10 @@ func DependencyUpdateTool(name string, dl checker.DetailLogger, dl.Warn(&checker.LogMessage{ Text: `dependabot config file not detected in source location. We recommend setting this configuration in code so it can be easily verified by others.`, - Version: 3, }) dl.Warn(&checker.LogMessage{ Text: `renovatebot config file not detected in source location. We recommend setting this configuration in code so it can be easily verified by others.`, - Version: 3, }) return checker.CreateMinScoreResult(name, "no update tool detected") } @@ -59,11 +57,10 @@ func DependencyUpdateTool(name string, dl checker.DetailLogger, // Note: only one file per tool is present, // so we do not iterate thru all entries. dl.Info(&checker.LogMessage{ - Path: r.Tools[0].ConfigFiles[0].Path, - Type: r.Tools[0].ConfigFiles[0].Type, - Offset: r.Tools[0].ConfigFiles[0].Offset, - Text: fmt.Sprintf("%s detected", r.Tools[0].Name), - Version: 3, + Path: r.Tools[0].ConfigFiles[0].Path, + Type: r.Tools[0].ConfigFiles[0].Type, + Offset: r.Tools[0].ConfigFiles[0].Offset, + Text: fmt.Sprintf("%s detected", r.Tools[0].Name), }) // High score result. diff --git a/checks/evaluation/security_policy.go b/checks/evaluation/security_policy.go index 170c2bf7..15e6a8db 100644 --- a/checks/evaluation/security_policy.go +++ b/checks/evaluation/security_policy.go @@ -33,10 +33,9 @@ func SecurityPolicy(name string, dl checker.DetailLogger, r *checker.SecurityPol for _, f := range r.Files { msg := checker.LogMessage{ - Path: f.Path, - Type: f.Type, - Offset: f.Offset, - Version: 3, + Path: f.Path, + Type: f.Type, + Offset: f.Offset, } if msg.Type == checker.FileTypeURL { msg.Text = "security policy detected in org repo" diff --git a/checks/evaluation/vulnerabilities.go b/checks/evaluation/vulnerabilities.go index 231c78ed..1130686a 100644 --- a/checks/evaluation/vulnerabilities.go +++ b/checks/evaluation/vulnerabilities.go @@ -43,8 +43,7 @@ func Vulnerabilities(name string, dl checker.DetailLogger, if len(IDs) > 0 { dl.Warn(&checker.LogMessage{ - Text: fmt.Sprintf("HEAD is vulnerable to %s", strings.Join(IDs, ", ")), - Version: 3, + Text: fmt.Sprintf("HEAD is vulnerable to %s", strings.Join(IDs, ", ")), }) return checker.CreateResultWithScore(name, fmt.Sprintf("%v existing vulnerabilities detected", len(IDs)), score) diff --git a/checks/license.go b/checks/license.go index abe01108..d477be05 100644 --- a/checks/license.go +++ b/checks/license.go @@ -110,10 +110,9 @@ func LicenseCheck(c *checker.CheckRequest) checker.CheckResult { if checkLicense(name) { c.Dlogger.Info(&checker.LogMessage{ - Path: name, - Type: checker.FileTypeSource, - Offset: 1, - Version: 3, + Path: name, + Type: checker.FileTypeSource, + Offset: 1, }) *pdata = true return false, nil diff --git a/checks/packaging.go b/checks/packaging.go index 21f990b0..8fd3e1ca 100644 --- a/checks/packaging.go +++ b/checks/packaging.go @@ -67,27 +67,24 @@ func Packaging(c *checker.CheckRequest) checker.CheckResult { } if len(runs) > 0 { c.Dlogger.Info(&checker.LogMessage{ - Path: fp, - Type: checker.FileTypeSource, - Offset: checker.OffsetDefault, - Text: fmt.Sprintf("GitHub publishing workflow used in run %s", runs[0].URL), - Version: 3, + Path: fp, + Type: checker.FileTypeSource, + Offset: checker.OffsetDefault, + Text: fmt.Sprintf("GitHub publishing workflow used in run %s", runs[0].URL), }) return checker.CreateMaxScoreResult(CheckPackaging, "publishing workflow detected") } c.Dlogger.Debug(&checker.LogMessage{ - Path: fp, - Type: checker.FileTypeSource, - Offset: checker.OffsetDefault, - Text: "GitHub publishing workflow not used in runs", - Version: 3, + Path: fp, + Type: checker.FileTypeSource, + Offset: checker.OffsetDefault, + Text: "GitHub publishing workflow not used in runs", }) } c.Dlogger.Warn(&checker.LogMessage{ - Text: "no GitHub publishing workflow detected", - Version: 3, + Text: "no GitHub publishing workflow detected", }) return checker.CreateInconclusiveResult(CheckPackaging, @@ -211,22 +208,20 @@ func isPackagingWorkflow(workflow *actionlint.Workflow, fp string, dl checker.De } dl.Info(&checker.LogMessage{ - Path: fp, - Type: checker.FileTypeSource, - Offset: fileparser.GetLineNumber(job.Pos), - Text: matcher.LogText, - Version: 3, + Path: fp, + Type: checker.FileTypeSource, + Offset: fileparser.GetLineNumber(job.Pos), + Text: matcher.LogText, }) return true } } dl.Debug(&checker.LogMessage{ - Path: fp, - Type: checker.FileTypeSource, - Offset: checker.OffsetDefault, - Text: "not a publishing workflow", - Version: 3, + Path: fp, + Type: checker.FileTypeSource, + Offset: checker.OffsetDefault, + Text: "not a publishing workflow", }) return false } diff --git a/checks/permissions.go b/checks/permissions.go index 31030cd7..2bcaa944 100644 --- a/checks/permissions.go +++ b/checks/permissions.go @@ -98,11 +98,10 @@ func validatePermission(permissionKey permission, permissionValue *actionlint.Pe if strings.EqualFold(val, "write") { if isPermissionOfInterest(permissionKey, ignoredPermissions) { dl.Warn(&checker.LogMessage{ - Path: path, - Type: checker.FileTypeSource, - Offset: lineNumber, - Text: fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val), - Version: 3, + Path: path, + Type: checker.FileTypeSource, + Offset: lineNumber, + Text: fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val), // TODO: set Snippet. }) recordPermissionWrite(pPermissions, permissionKey) @@ -110,11 +109,10 @@ func validatePermission(permissionKey permission, permissionValue *actionlint.Pe // Only log for debugging, otherwise // it may confuse users. dl.Debug(&checker.LogMessage{ - Path: path, - Type: checker.FileTypeSource, - Offset: lineNumber, - Text: fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val), - Version: 3, + Path: path, + Type: checker.FileTypeSource, + Offset: lineNumber, + Text: fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val), // TODO: set Snippet. }) } @@ -122,11 +120,10 @@ func validatePermission(permissionKey permission, permissionValue *actionlint.Pe } dl.Info(&checker.LogMessage{ - Path: path, - Type: checker.FileTypeSource, - Offset: lineNumber, - Text: fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val), - Version: 3, + Path: path, + Type: checker.FileTypeSource, + Offset: lineNumber, + Text: fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val), // TODO: set Snippet. }) return nil @@ -174,11 +171,10 @@ func validatePermissions(permissions *actionlint.Permissions, permLevel, path st scopeIsSet := permissions != nil && len(permissions.Scopes) > 0 if permissions == nil || (!allIsSet && !scopeIsSet) { dl.Info(&checker.LogMessage{ - Path: path, - Type: checker.FileTypeSource, - Offset: checker.OffsetDefault, - Text: fmt.Sprintf("%s permissions set to 'none'", permLevel), - Version: 3, + Path: path, + Type: checker.FileTypeSource, + Offset: checker.OffsetDefault, + Text: fmt.Sprintf("%s permissions set to 'none'", permLevel), }) } if allIsSet { @@ -186,11 +182,10 @@ func validatePermissions(permissions *actionlint.Permissions, permLevel, path st lineNumber := fileparser.GetLineNumber(permissions.All.Pos) if !strings.EqualFold(val, "read-all") && val != "" { dl.Warn(&checker.LogMessage{ - Path: path, - Type: checker.FileTypeSource, - Offset: lineNumber, - Text: fmt.Sprintf("%s permissions set to '%v'", permLevel, val), - Version: 3, + Path: path, + Type: checker.FileTypeSource, + Offset: lineNumber, + Text: fmt.Sprintf("%s permissions set to '%v'", permLevel, val), // TODO: set Snippet. }) recordAllPermissionsWrite(pdata, permLevel, path) @@ -198,11 +193,10 @@ func validatePermissions(permissions *actionlint.Permissions, permLevel, path st } dl.Info(&checker.LogMessage{ - Path: path, - Type: checker.FileTypeSource, - Offset: lineNumber, - Text: fmt.Sprintf("%s permissions set to '%v'", permLevel, val), - Version: 3, + Path: path, + Type: checker.FileTypeSource, + Offset: lineNumber, + Text: fmt.Sprintf("%s permissions set to '%v'", permLevel, val), // TODO: set Snippet. }) } else /* scopeIsSet == true */ if err := validateMapPermissions(permissions.Scopes, @@ -217,11 +211,10 @@ func validateTopLevelPermissions(workflow *actionlint.Workflow, path string, // Check if permissions are set explicitly. if workflow.Permissions == nil { dl.Warn(&checker.LogMessage{ - Path: path, - Type: checker.FileTypeSource, - Offset: checker.OffsetDefault, - Text: fmt.Sprintf("no %s permission defined", topLevelPermission), - Version: 3, + Path: path, + Type: checker.FileTypeSource, + Offset: checker.OffsetDefault, + Text: fmt.Sprintf("no %s permission defined", topLevelPermission), }) recordAllPermissionsWrite(pdata, topLevelPermission, path) return nil @@ -240,11 +233,10 @@ func validatejobLevelPermissions(workflow *actionlint.Workflow, path string, // so only top-level read-only permissions need to be declared. if job.Permissions == nil { dl.Debug(&checker.LogMessage{ - Path: path, - Type: checker.FileTypeSource, - Offset: fileparser.GetLineNumber(job.Pos), - Text: fmt.Sprintf("no %s permission defined", jobLevelPermission), - Version: 3, + Path: path, + Type: checker.FileTypeSource, + Offset: fileparser.GetLineNumber(job.Pos), + Text: fmt.Sprintf("no %s permission defined", jobLevelPermission), }) recordAllPermissionsWrite(pdata, jobLevelPermission, path) continue @@ -481,11 +473,10 @@ func isSARIFUploadAction(workflow *actionlint.Workflow, fp string, dl checker.De } if strings.HasPrefix(uses.Value, "github/codeql-action/upload-sarif@") { dl.Debug(&checker.LogMessage{ - Path: fp, - Type: checker.FileTypeSource, - Offset: fileparser.GetLineNumber(uses.Pos), - Text: "codeql SARIF upload workflow detected", - Version: 3, + Path: fp, + Type: checker.FileTypeSource, + Offset: fileparser.GetLineNumber(uses.Pos), + Text: "codeql SARIF upload workflow detected", // TODO: set Snippet. }) return true @@ -493,11 +484,10 @@ func isSARIFUploadAction(workflow *actionlint.Workflow, fp string, dl checker.De } } dl.Debug(&checker.LogMessage{ - Path: fp, - Type: checker.FileTypeSource, - Offset: checker.OffsetDefault, - Text: "not a codeql upload SARIF workflow", - Version: 3, + Path: fp, + Type: checker.FileTypeSource, + Offset: checker.OffsetDefault, + Text: "not a codeql upload SARIF workflow", }) return false } @@ -515,11 +505,10 @@ func isCodeQlAnalysisWorkflow(workflow *actionlint.Workflow, fp string, dl check } if strings.HasPrefix(uses.Value, "github/codeql-action/analyze@") { dl.Debug(&checker.LogMessage{ - Path: fp, - Type: checker.FileTypeSource, - Offset: fileparser.GetLineNumber(uses.Pos), - Text: "codeql workflow detected", - Version: 3, + Path: fp, + Type: checker.FileTypeSource, + Offset: fileparser.GetLineNumber(uses.Pos), + Text: "codeql workflow detected", // TODO: set Snippet. }) return true @@ -527,11 +516,10 @@ func isCodeQlAnalysisWorkflow(workflow *actionlint.Workflow, fp string, dl check } } dl.Debug(&checker.LogMessage{ - Path: fp, - Type: checker.FileTypeSource, - Offset: checker.OffsetDefault, - Text: "not a codeql workflow", - Version: 3, + Path: fp, + Type: checker.FileTypeSource, + Offset: checker.OffsetDefault, + Text: "not a codeql workflow", }) return false } diff --git a/checks/pinned_dependencies.go b/checks/pinned_dependencies.go index 4aae6643..be672e44 100644 --- a/checks/pinned_dependencies.go +++ b/checks/pinned_dependencies.go @@ -162,20 +162,18 @@ func createReturnValuesForGitHubActionsWorkflowPinned(r worklowPinningResult, in if r.gitHubOwned != notPinned { score += 2 dl.Info(&checker.LogMessage{ - Type: checker.FileTypeSource, - Offset: checker.OffsetDefault, - Text: fmt.Sprintf("%s %s", "GitHub-owned", infoMsg), - Version: 3, + Type: checker.FileTypeSource, + Offset: checker.OffsetDefault, + Text: fmt.Sprintf("%s %s", "GitHub-owned", infoMsg), }) } if r.thirdParties != notPinned { score += 8 dl.Info(&checker.LogMessage{ - Type: checker.FileTypeSource, - Offset: checker.OffsetDefault, - Text: fmt.Sprintf("%s %s", "Third-party", infoMsg), - Version: 3, + Type: checker.FileTypeSource, + Offset: checker.OffsetDefault, + Text: fmt.Sprintf("%s %s", "Third-party", infoMsg), }) } @@ -440,7 +438,6 @@ func validateDockerfileIsPinned(pathfn string, content []byte, EndOffset: uint(child.EndLine), Text: "docker image not pinned by hash", Snippet: child.Original, - Version: 3, }) // FROM name. @@ -456,7 +453,6 @@ func validateDockerfileIsPinned(pathfn string, content []byte, EndOffset: uint(child.EndLine), Text: "docker image not pinned by hash", Snippet: child.Original, - Version: 3, }) } @@ -666,7 +662,6 @@ func validateGitHubActionWorkflow(pathfn string, content []byte, EndOffset: uint(execAction.Uses.Pos.Line), // `Uses` always span a single line. Snippet: execAction.Uses.Value, Text: fmt.Sprintf("%s action not pinned by hash", owner), - Version: 3, }) } diff --git a/checks/sast.go b/checks/sast.go index 35237adf..b9d8e85a 100644 --- a/checks/sast.go +++ b/checks/sast.go @@ -141,10 +141,9 @@ func sastToolInCheckRuns(c *checker.CheckRequest) (int, error) { } if sastTools[cr.App.Slug] { c.Dlogger.Debug(&checker.LogMessage{ - Path: cr.URL, - Type: checker.FileTypeURL, - Text: "tool detected", - Version: 3, + Path: cr.URL, + Type: checker.FileTypeURL, + Text: "tool detected", }) totalTested++ break @@ -153,21 +152,18 @@ func sastToolInCheckRuns(c *checker.CheckRequest) (int, error) { } if totalMerged == 0 { c.Dlogger.Warn(&checker.LogMessage{ - Text: "no pull requests merged into dev branch", - Version: 3, + Text: "no pull requests merged into dev branch", }) return checker.InconclusiveResultScore, nil } if totalTested == totalMerged { c.Dlogger.Info(&checker.LogMessage{ - Text: fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalMerged), - Version: 3, + Text: fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalMerged), }) } else { c.Dlogger.Warn(&checker.LogMessage{ - Text: fmt.Sprintf("%v commits out of %v are checked with a SAST tool", totalTested, totalMerged), - Version: 3, + Text: fmt.Sprintf("%v commits out of %v are checked with a SAST tool", totalTested, totalMerged), }) } @@ -188,11 +184,10 @@ func codeQLInCheckDefinitions(c *checker.CheckRequest) (int, error) { for _, result := range resp.Results { c.Dlogger.Debug(&checker.LogMessage{ - Path: result.Path, - Type: checker.FileTypeSource, - Offset: checker.OffsetDefault, - Text: "CodeQL detected", - Version: 3, + Path: result.Path, + Type: checker.FileTypeSource, + Offset: checker.OffsetDefault, + Text: "CodeQL detected", }) } @@ -200,15 +195,13 @@ func codeQLInCheckDefinitions(c *checker.CheckRequest) (int, error) { // TODO: check which branches it is enabled on. We should find main. if resp.Hits > 0 { c.Dlogger.Info(&checker.LogMessage{ - Text: "SAST tool detected: CodeQL", - Version: 3, + Text: "SAST tool detected: CodeQL", }) return checker.MaxResultScore, nil } c.Dlogger.Warn(&checker.LogMessage{ - Text: "CodeQL tool not detected", - Version: 3, + Text: "CodeQL tool not detected", }) return checker.MinResultScore, nil } diff --git a/checks/shell_download_validate.go b/checks/shell_download_validate.go index 83a0df9c..4d715856 100644 --- a/checks/shell_download_validate.go +++ b/checks/shell_download_validate.go @@ -331,7 +331,6 @@ func isFetchPipeExecute(startLine, endLine uint, node syntax.Node, cmd, pathfn s EndOffset: endLine, Snippet: cmd, Text: "insecure (not pinned by hash) download detected", - Version: 3, }) return true } @@ -380,7 +379,6 @@ func isExecuteFiles(startLine, endLine uint, node syntax.Node, cmd, pathfn strin EndOffset: endLine, Snippet: cmd, Text: "insecure (not pinned by hash) download-then-run", - Version: 3, }) ok = true } @@ -598,7 +596,6 @@ func isUnpinnedPakageManagerDownload(startLine, endLine uint, node syntax.Node, EndOffset: endLine, Snippet: cmd, Text: "go installation not pinned by hash", - Version: 3, }) return true } @@ -612,7 +609,6 @@ func isUnpinnedPakageManagerDownload(startLine, endLine uint, node syntax.Node, EndOffset: endLine, Snippet: cmd, Text: "pip installation not pinned by hash", - Version: 3, }) return true } @@ -626,7 +622,6 @@ func isUnpinnedPakageManagerDownload(startLine, endLine uint, node syntax.Node, EndOffset: endLine, Snippet: cmd, Text: "npm installation not pinned by hash", - Version: 3, }) return true } @@ -719,7 +714,6 @@ func isFetchProcSubsExecute(startLine, endLine uint, node syntax.Node, cmd, path EndOffset: endLine, Snippet: cmd, Text: "insecure (not pinned by hash) download-then-run", - Version: 3, }) return true } diff --git a/checks/signed_releases.go b/checks/signed_releases.go index 0bd04579..1f759673 100644 --- a/checks/signed_releases.go +++ b/checks/signed_releases.go @@ -53,8 +53,7 @@ func SignedReleases(c *checker.CheckRequest) checker.CheckResult { continue } c.Dlogger.Debug(&checker.LogMessage{ - Text: fmt.Sprintf("GitHub release found: %s", r.TagName), - Version: 3, + Text: fmt.Sprintf("GitHub release found: %s", r.TagName), }) totalReleases++ signed := false @@ -62,10 +61,9 @@ func SignedReleases(c *checker.CheckRequest) checker.CheckResult { for _, suffix := range artifactExtensions { if strings.HasSuffix(asset.Name, suffix) { c.Dlogger.Info(&checker.LogMessage{ - Path: asset.URL, - Type: checker.FileTypeURL, - Text: fmt.Sprintf("signed release artifact: %s", asset.Name), - Version: 3, + Path: asset.URL, + Type: checker.FileTypeURL, + Text: fmt.Sprintf("signed release artifact: %s", asset.Name), }) signed = true break @@ -78,10 +76,9 @@ func SignedReleases(c *checker.CheckRequest) checker.CheckResult { } if !signed { c.Dlogger.Warn(&checker.LogMessage{ - Path: r.URL, - Type: checker.FileTypeURL, - Text: fmt.Sprintf("release artifact %s not signed", r.TagName), - Version: 3, + Path: r.URL, + Type: checker.FileTypeURL, + Text: fmt.Sprintf("release artifact %s not signed", r.TagName), }) } if totalReleases >= releaseLookBack { @@ -91,8 +88,7 @@ func SignedReleases(c *checker.CheckRequest) checker.CheckResult { if totalReleases == 0 { c.Dlogger.Warn(&checker.LogMessage{ - Text: "no GitHub releases found", - Version: 3, + Text: "no GitHub releases found", }) // Generic summary. return checker.CreateInconclusiveResult(CheckSignedReleases, "no releases found") diff --git a/cron/format/json_test.go b/cron/format/json_test.go index 6f6f05a7..c3650d24 100644 --- a/cron/format/json_test.go +++ b/cron/format/json_test.go @@ -113,8 +113,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 5, Snippet: "if (bad) {BUG();}", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -151,8 +149,6 @@ func TestJSONOutput(t *testing.T) { Path: "bin/binary.elf", Type: checker.FileTypeBinary, Offset: 0, - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -189,8 +185,6 @@ func TestJSONOutput(t *testing.T) { Path: "bin/binary.elf", Type: checker.FileTypeBinary, Offset: 0, - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -208,8 +202,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeText, Offset: 3, Snippet: "some text", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -227,8 +219,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG();}", - // UPGRADEv3: to remove. - Version: 3, }, }, { @@ -239,8 +229,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG2();}", - // UPGRADEv3: to remove. - Version: 3, }, }, { @@ -251,8 +239,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG5();}", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -289,8 +275,6 @@ func TestJSONOutput(t *testing.T) { Path: "bin/binary.elf", Type: checker.FileTypeBinary, Offset: 0, - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -308,8 +292,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeText, Offset: 3, Snippet: "some text", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -327,8 +309,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG();}", - // UPGRADEv3: to remove. - Version: 3, }, }, { @@ -339,8 +319,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG2();}", - // UPGRADEv3: to remove. - Version: 3, }, }, { @@ -351,8 +329,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG5();}", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -390,8 +366,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 5, Snippet: "if (bad) {BUG();}", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -427,8 +401,6 @@ func TestJSONOutput(t *testing.T) { Text: "warn message", Path: "https://domain.com/something", Type: checker.FileTypeURL, - // UPGRADEv3: to remove. - Version: 3, }, }, }, diff --git a/pkg/common.go b/pkg/common.go index 8cfc8c6f..0b45fd4a 100644 --- a/pkg/common.go +++ b/pkg/common.go @@ -28,26 +28,18 @@ func textToMarkdown(s string) string { // DetailToString turns a detail information into a string. func DetailToString(d *checker.CheckDetail, logLevel log.Level) string { - // TODO(#1393): remove switch statement. - switch d.Msg.Version { - case 3: - if d.Type == checker.DetailDebug && logLevel != log.DebugLevel { - return "" - } - switch { - case d.Msg.Path != "" && d.Msg.Offset != 0 && d.Msg.EndOffset != 0 && d.Msg.Offset < d.Msg.EndOffset: - return fmt.Sprintf("%s: %s: %s:%d-%d", typeToString(d.Type), d.Msg.Text, d.Msg.Path, d.Msg.Offset, d.Msg.EndOffset) - case d.Msg.Path != "" && d.Msg.Offset != 0: - return fmt.Sprintf("%s: %s: %s:%d", typeToString(d.Type), d.Msg.Text, d.Msg.Path, d.Msg.Offset) - case d.Msg.Path != "" && d.Msg.Offset == 0: - return fmt.Sprintf("%s: %s: %s", typeToString(d.Type), d.Msg.Text, d.Msg.Path) - default: - return fmt.Sprintf("%s: %s", typeToString(d.Type), d.Msg.Text) - } + if d.Type == checker.DetailDebug && logLevel != log.DebugLevel { + return "" + } + + switch { + case d.Msg.Path != "" && d.Msg.Offset != 0 && d.Msg.EndOffset != 0 && d.Msg.Offset < d.Msg.EndOffset: + return fmt.Sprintf("%s: %s: %s:%d-%d", typeToString(d.Type), d.Msg.Text, d.Msg.Path, d.Msg.Offset, d.Msg.EndOffset) + case d.Msg.Path != "" && d.Msg.Offset != 0: + return fmt.Sprintf("%s: %s: %s:%d", typeToString(d.Type), d.Msg.Text, d.Msg.Path, d.Msg.Offset) + case d.Msg.Path != "" && d.Msg.Offset == 0: + return fmt.Sprintf("%s: %s: %s", typeToString(d.Type), d.Msg.Text, d.Msg.Path) default: - if d.Type == checker.DetailDebug && logLevel != log.DebugLevel { - return "" - } return fmt.Sprintf("%s: %s", typeToString(d.Type), d.Msg.Text) } } diff --git a/pkg/json_test.go b/pkg/json_test.go index 5c3b01c5..a26684eb 100644 --- a/pkg/json_test.go +++ b/pkg/json_test.go @@ -112,8 +112,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 5, Snippet: "if (bad) {BUG();}", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -150,8 +148,6 @@ func TestJSONOutput(t *testing.T) { Path: "bin/binary.elf", Type: checker.FileTypeBinary, Offset: 0, - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -188,8 +184,6 @@ func TestJSONOutput(t *testing.T) { Path: "bin/binary.elf", Type: checker.FileTypeBinary, Offset: 0, - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -207,8 +201,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeText, Offset: 3, Snippet: "some text", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -226,8 +218,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG();}", - // UPGRADEv3: to remove. - Version: 3, }, }, { @@ -238,8 +228,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG2();}", - // UPGRADEv3: to remove. - Version: 3, }, }, { @@ -250,8 +238,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG5();}", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -288,8 +274,6 @@ func TestJSONOutput(t *testing.T) { Path: "bin/binary.elf", Type: checker.FileTypeBinary, Offset: 0, - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -307,8 +291,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeText, Offset: 3, Snippet: "some text", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -326,8 +308,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG();}", - // UPGRADEv3: to remove. - Version: 3, }, }, { @@ -338,8 +318,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG2();}", - // UPGRADEv3: to remove. - Version: 3, }, }, { @@ -350,8 +328,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 3, Snippet: "if (bad) {BUG5();}", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -389,8 +365,6 @@ func TestJSONOutput(t *testing.T) { Type: checker.FileTypeSource, Offset: 5, Snippet: "if (bad) {BUG();}", - // UPGRADEv3: to remove. - Version: 3, }, }, }, @@ -426,8 +400,6 @@ func TestJSONOutput(t *testing.T) { Text: "warn message", Path: "https://domain.com/something", Type: checker.FileTypeURL, - // UPGRADEv3: to remove. - Version: 3, }, }, }, diff --git a/pkg/sarif_test.go b/pkg/sarif_test.go index 73f7ec8c..9bbeb97b 100644 --- a/pkg/sarif_test.go +++ b/pkg/sarif_test.go @@ -21,6 +21,8 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/log" spol "github.com/ossf/scorecard/v4/policy" @@ -767,8 +769,8 @@ func TestSARIFOutput(t *testing.T) { }, }, } - for _, tt := range tests { - tt := tt // Re-initializing variable so it is not changed while executing the closure below + for i := range tests { + tt := &tests[i] // Re-initializing variable so it is not changed while executing the closure below t.Run(tt.name, func(t *testing.T) { t.Parallel() var content []byte @@ -796,7 +798,7 @@ func TestSARIFOutput(t *testing.T) { r := bytes.Compare(expected.Bytes(), result.Bytes()) if r != 0 { - t.Fatalf("%s: invalid result: %d", tt.name, r) + t.Fatalf("%s: invalid result: %d, %s", tt.name, r, cmp.Diff(expected.Bytes(), result.Bytes())) } }) } diff --git a/pkg/testdata/check6.sarif b/pkg/testdata/check6.sarif index aaa8a245..bcaebda1 100644 --- a/pkg/testdata/check6.sarif +++ b/pkg/testdata/check6.sarif @@ -47,7 +47,7 @@ "ruleId": "CheckNameID", "ruleIndex": 0, "message": { - "text": "score is 6: six score reason:\nWarn: warn message\nClick Remediation section below to solve this issue" + "text": "score is 6: six score reason:\nWarn: warn message: https://domain.com/something\nClick Remediation section below to solve this issue" }, "locations": [ { diff --git a/utests/utlib.go b/utests/utlib.go index 4f4d825f..3dc1dc35 100644 --- a/utests/utlib.go +++ b/utests/utlib.go @@ -46,7 +46,6 @@ func (l *TestDetailLogger) Info(msg *checker.LogMessage) { Type: checker.DetailInfo, Msg: *msg, } - cd.Msg.Version = 3 l.messages = append(l.messages, cd) } @@ -56,7 +55,6 @@ func (l *TestDetailLogger) Warn(msg *checker.LogMessage) { Type: checker.DetailWarn, Msg: *msg, } - cd.Msg.Version = 3 l.messages = append(l.messages, cd) } @@ -66,7 +64,6 @@ func (l *TestDetailLogger) Debug(msg *checker.LogMessage) { Type: checker.DetailDebug, Msg: *msg, } - cd.Msg.Version = 3 l.messages = append(l.messages, cd) }