✨ Transition Vulnerabilities, Permissions, CI-Tests, Dependency-Update-Tool, Code-Reviews to structured details (#889)

* move other checks togit add -u

* more checks

* fixes

checks/automatic_dependency_update.go

@@ -36,8 +36,12 @@ func AutomaticDependencyUpdate(c *checker.CheckRequest) checker.CheckResult {
 		return checker.CreateRuntimeErrorResult(CheckDependencyUpdateTool, err)
 	}
 	if !r {
-		c.Dlogger.Warn("dependabot not detected")
+		c.Dlogger.Warn3(&checker.LogMessage{
-		c.Dlogger.Warn("renovatebot not detected")
+			Text: "dependabot not detected",
+		})
+		c.Dlogger.Warn3(&checker.LogMessage{
+			Text: "renovatebot not detected",
+		})
 		return checker.CreateMinScoreResult(CheckDependencyUpdateTool, "no update tool detected")
 	}
 
@@ -51,11 +55,23 @@ func fileExists(name string, dl checker.DetailLogger, data FileCbData) (bool, er
 
 	switch strings.ToLower(name) {
 	case ".github/dependabot.yml":
-		dl.Info("dependabot detected : %s", name)
+		dl.Info3(&checker.LogMessage{
+			Path: name,
+			Type: checker.FileTypeSource,
+			// Source file must have line number > 0.
+			Offset: 1,
+			Text:   "dependabot detected",
+		})
 		// https://docs.renovatebot.com/configuration-options/
 	case ".github/renovate.json", ".github/renovate.json5", ".renovaterc.json", "renovate.json",
 		"renovate.json5", ".renovaterc":
-		dl.Info("renovate detected: %s", name)
+		dl.Info3(&checker.LogMessage{
+			Path: name,
+			Type: checker.FileTypeSource,
+			// Source file must have line number > 0.
+			Offset: 1,
+			Text:   "renovate detected",
+		})
 	default:
 		// Continue iterating.
 		return true, nil

checks/ci_tests.go

@@ -90,7 +90,9 @@ func CITests(c *checker.CheckRequest) checker.CheckResult {
 		}
 
 		if !foundCI {
-			c.Dlogger.Debug("merged PR without CI test: %d", pr.Number)
+			c.Dlogger.Debug3(&checker.LogMessage{
+				Text: fmt.Sprintf("merged PR without CI test: %d", pr.Number),
+			})
 		}
 	}
 
@@ -116,8 +118,12 @@ func prHasSuccessStatus(pr *clients.PullRequest, c *checker.CheckRequest) (bool,
 			continue
 		}
 		if isTest(status.GetContext()) {
-			c.Dlogger.Debug("CI test found: pr: %d, context: %success, url: %success", pr.Number,
+			c.Dlogger.Debug3(&checker.LogMessage{
-				status.GetContext(), status.GetURL())
+				Path: status.GetURL(),
+				Type: checker.FileTypeURL,
+				Text: fmt.Sprintf("CI test found: pr: %d, context: %s", pr.Number,
+					status.GetContext()),
+			})
 			return true, nil
 		}
 	}
@@ -145,8 +151,12 @@ func prHasSuccessfulCheck(pr *clients.PullRequest, c *checker.CheckRequest) (boo
 			continue
 		}
 		if isTest(cr.GetApp().GetSlug()) {
-			c.Dlogger.Debug("CI test found: pr: %d, context: %success, url: %success", pr.Number,
+			c.Dlogger.Debug3(&checker.LogMessage{
-				cr.GetApp().GetSlug(), cr.GetURL())
+				Path: cr.GetURL(),
+				Type: checker.FileTypeURL,
+				Text: fmt.Sprintf("CI test found: pr: %d, context: %s", pr.Number,
+					cr.GetApp().GetSlug()),
+			})
 			return true, nil
 		}
 	}

checks/code_review.go

@@ -74,11 +74,15 @@ func DoesCodeReview(c *checker.CheckRequest) checker.CheckResult {
 func selectBestScoreAndReason(s1, s2 int, r1, r2 string,
 	dl checker.DetailLogger) (int, string) {
 	if s1 > s2 {
-		dl.Info(r2)
+		dl.Info3(&checker.LogMessage{
+			Text: r2,
+		})
 		return s1, r1
 	}
 
-	dl.Info(r1)
+	dl.Info3(&checker.LogMessage{
+		Text: r1,
+	})
 	return s2, r2
 }
 
@@ -101,7 +105,9 @@ func githubCodeReview(c *checker.CheckRequest) (int, string, error) {
 		foundApprovedReview := false
 		for _, r := range pr.Reviews {
 			if r.State == "APPROVED" {
-				c.Dlogger.Debug("found review approved pr: %d", pr.Number)
+				c.Dlogger.Debug3(&checker.LogMessage{
+					Text: fmt.Sprintf("found review approved pr: %d", pr.Number),
+				})
 				totalReviewed++
 				foundApprovedReview = true
 				break
@@ -113,7 +119,9 @@ func githubCodeReview(c *checker.CheckRequest) (int, string, error) {
 		// time on clicking the approve button.
 		if !foundApprovedReview {
 			if !pr.MergeCommit.AuthoredByCommitter {
-				c.Dlogger.Debug("found pr with committer different than author: %d", pr.Number)
+				c.Dlogger.Debug3(&checker.LogMessage{
+					Text: fmt.Sprintf("found pr with committer different than author: %d", pr.Number),
+				})
 				totalReviewed++
 			}
 		}
@@ -184,7 +192,9 @@ func commitMessageHints(c *checker.CheckRequest) (int, string, error) {
 			}
 		}
 		if isBot {
-			c.Dlogger.Debug("skip commit from bot account: %s", committer)
+			c.Dlogger.Debug3(&checker.LogMessage{
+				Text: fmt.Sprintf("skip commit from bot account: %s", committer),
+			})
 			continue
 		}
 
@@ -194,7 +204,9 @@ func commitMessageHints(c *checker.CheckRequest) (int, string, error) {
 		commitMessage := commit.Message
 		if strings.Contains(commitMessage, "\nReviewed-on: ") &&
 			strings.Contains(commitMessage, "\nReviewed-by: ") {
-			c.Dlogger.Debug("Gerrit review found for commit '%s'", commit.SHA)
+			c.Dlogger.Debug3(&checker.LogMessage{
+				Text: fmt.Sprintf("Gerrit review found for commit '%s'", commit.SHA),
+			})
 			totalReviewed++
 			continue
 		}

checks/contributors.go

@@ -70,7 +70,9 @@ func Contributors(c *checker.CheckRequest) checker.CheckResult {
 		names = append(names, c)
 	}
 
-	c.Dlogger.Info("contributors work for: %v", strings.Join(names, ","))
+	c.Dlogger.Info3(&checker.LogMessage{
+		Text: fmt.Sprintf("contributors work for: %v", strings.Join(names, ",")),
+	})
 
 	reason := fmt.Sprintf("%d different companies found", len(companies))
 	return checker.CreateProportionalScoreResult(CheckContributors, reason, len(companies), numberCompaniesForTopScore)

checks/permissions.go

@@ -63,17 +63,38 @@ func validatePermission(key string, value interface{}, path string,
 
 	if strings.EqualFold(val, "write") {
 		if isPermissionOfInterest(key, ignoredPermissions) {
-			dl.Warn("'%v' permission set to '%v' in %v", key, val, path)
+			dl.Warn3(&checker.LogMessage{
+				Path: path,
+				Type: checker.FileTypeSource,
+				// TODO: set line.
+				Offset: 1,
+				Text:   fmt.Sprintf("'%v' permission set to '%v'", key, val),
+				// TODO: set Snippet.
+			})
 			recordPermissionWrite(key, pPermissions)
 		} else {
 			// Only log for debugging, otherwise
 			// it may confuse users.
-			dl.Debug("'%v' permission set to '%v' in %v", key, val, path)
+			dl.Debug3(&checker.LogMessage{
+				Path: path,
+				Type: checker.FileTypeSource,
+				// TODO: set line.
+				Offset: 1,
+				Text:   fmt.Sprintf("'%v' permission set to '%v'", key, val),
+				// TODO: set Snippet.
+			})
 		}
 		return nil
 	}
 
-	dl.Info("'%v' permission set to '%v' in %v", key, val, path)
+	dl.Info3(&checker.LogMessage{
+		Path: path,
+		Type: checker.FileTypeSource,
+		// TODO: set line correctly.
+		Offset: 1,
+		Text:   fmt.Sprintf("'%v' permission set to '%v'", key, val),
+		// TODO: set Snippet.
+	})
 	return nil
 }
 
@@ -113,15 +134,37 @@ func validatePermissions(permissions interface{}, path string,
 	// Empty string is nil type.
 	// It defaults to 'none'
 	case nil:
-		dl.Info("permissions set to 'none' in %v", path)
+		dl.Info3(&checker.LogMessage{
+			Path: path,
+			Type: checker.FileTypeSource,
+			// TODO: set line correctly.
+			Offset: 1,
+			Text:   "permissions set to 'none'",
+			// TODO: set Snippet.
+		})
 	// String type.
 	case string:
 		if !strings.EqualFold(val, "read-all") && val != "" {
-			dl.Warn("permissions set to '%v' in %v", val, path)
+			dl.Warn3(&checker.LogMessage{
+				Path: path,
+				Type: checker.FileTypeSource,
+				// TODO: set line correctly.
+				Offset: 1,
+				Text:   fmt.Sprintf("permissions set to '%v'", val),
+				// TODO: set Snippet.
+			})
 			recordAllPermissionsWrite(pPermissions)
 			return nil
 		}
-		dl.Info("permission set to '%v' in %v", val, path)
+
+		dl.Info3(&checker.LogMessage{
+			Path: path,
+			Type: checker.FileTypeSource,
+			// TODO: set line correctly.
+			Offset: 1,
+			Text:   fmt.Sprintf("permissions set to '%v'", val),
+			// TODO: set Snippet.
+		})
 
 	// Map type.
 	case map[interface{}]interface{}:
@@ -142,7 +185,12 @@ func validateTopLevelPermissions(config map[interface{}]interface{}, path string
 	// Check if permissions are set explicitly.
 	permissions, ok := config["permissions"]
 	if !ok {
-		dl.Warn("no permission defined in %v", path)
+		dl.Warn3(&checker.LogMessage{
+			Path:   path,
+			Type:   checker.FileTypeSource,
+			Offset: 1,
+			Text:   "no permission defined",
+		})
 		recordAllPermissionsWrite(pdata.topLevelWritePermissions)
 		return nil
 	}
@@ -178,7 +226,12 @@ func validateRunLevelPermissions(config map[interface{}]interface{}, path string
 		// so only top-level read-only permissions need to be declared.
 		permissions, ok := job["permissions"]
 		if !ok {
-			dl.Debug("no permission defined in %v", path)
+			dl.Debug3(&checker.LogMessage{
+				Path:   path,
+				Type:   checker.FileTypeSource,
+				Offset: 1,
+				Text:   "no permission defined",
+			})
 			continue
 		}
 		err := validatePermissions(permissions, path, dl,
@@ -396,10 +449,22 @@ func isSARIFUploadWorkflow(s, fp string, dl checker.DetailLogger) bool {
 // CodeQl run externally and SARIF file uploaded.
 func isSARIFUploadAction(s, fp string, dl checker.DetailLogger) bool {
 	if strings.Contains(s, "github/codeql-action/upload-sarif@") {
-		dl.Debug("codeql SARIF upload workflow detected: %v", fp)
+		dl.Debug3(&checker.LogMessage{
+			Path: fp,
+			Type: checker.FileTypeSource,
+			// TODO: set line.
+			Offset: 1,
+			Text:   "codeql SARIF upload workflow detected",
+			// TODO: set Snippet.
+		})
 		return true
 	}
-	dl.Debug("not a codeql upload SARIF workflow: %v", fp)
+	dl.Debug3(&checker.LogMessage{
+		Path:   fp,
+		Type:   checker.FileTypeSource,
+		Offset: 1,
+		Text:   "not a codeql upload SARIF workflow",
+	})
 	return false
 }
 
@@ -409,10 +474,22 @@ func isSARIFUploadAction(s, fp string, dl checker.DetailLogger) bool {
 // https://docs.github.com/en/code-security/secure-coding/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning.
 func isCodeQlAnalysisWorkflow(s, fp string, dl checker.DetailLogger) bool {
 	if strings.Contains(s, "github/codeql-action/analyze@") {
-		dl.Debug("codeql workflow detected: %v", fp)
+		dl.Debug3(&checker.LogMessage{
+			Path: fp,
+			Type: checker.FileTypeSource,
+			// TODO: set line.
+			Offset: 1,
+			Text:   "codeql workflow detected",
+			// TODO: set Snippet.
+		})
 		return true
 	}
-	dl.Debug("not a codeql workflow: %v", fp)
+	dl.Debug3(&checker.LogMessage{
+		Path:   fp,
+		Type:   checker.FileTypeSource,
+		Offset: 1,
+		Text:   "not a codeql workflow",
+	})
 	return false
 }
 

checks/vulnerabilities.go

@@ -99,7 +99,9 @@ func HasUnfixedVulnerabilities(c *checker.CheckRequest) checker.CheckResult {
 	// TODO: take severity into account.
 	vulnIDs := osvResp.getVulnerabilities()
 	if len(vulnIDs) > 0 {
-		c.Dlogger.Warn("HEAD is vulnerable to %s", strings.Join(vulnIDs, ", "))
+		c.Dlogger.Warn3(&checker.LogMessage{
+			Text: fmt.Sprintf("HEAD is vulnerable to %s", strings.Join(vulnIDs, ", ")),
+		})
 		return checker.CreateMinScoreResult(CheckVulnerabilities, "existing vulnerabilities detected")
 	}