Skip to content

Commit

Permalink
Remove Version field from LogMessage (#1640)
Browse files Browse the repository at this point in the history
Co-authored-by: Azeem Shaikh <[email protected]>
  • Loading branch information
azeemshaikh38 and azeemsgoogle authored Feb 15, 2022
1 parent 3551134 commit 2b206dc
Show file tree
Hide file tree
Showing 22 changed files with 139 additions and 266 deletions.
2 changes: 0 additions & 2 deletions checker/check_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 1 addition & 4 deletions checks/ci_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})
}
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions checks/contributors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
27 changes: 12 additions & 15 deletions checks/dangerous_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions checks/evaluation/binary_artifacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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--
Expand Down
15 changes: 4 additions & 11 deletions checks/evaluation/code_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
11 changes: 4 additions & 7 deletions checks/evaluation/dependency_update_tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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.
Expand Down
7 changes: 3 additions & 4 deletions checks/evaluation/security_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 1 addition & 2 deletions checks/evaluation/vulnerabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 3 additions & 4 deletions checks/license.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 17 additions & 22 deletions checks/packaging.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Loading

0 comments on commit 2b206dc

Please sign in to comment.