From ca944e8169ea43deca0fa2f057054841b59b61c4 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Wed, 7 Feb 2024 10:55:16 -0800 Subject: [PATCH] :seedling: Change finding Values to map[string]string (#3837) * make values map string -> string Signed-off-by: Spencer Schrock * fixup branch protection probes Signed-off-by: Spencer Schrock * fix sast probe Signed-off-by: Spencer Schrock * fix signed-releases probes Signed-off-by: Spencer Schrock * fix maintained probes Signed-off-by: Spencer Schrock * fix cii-best-practices probes Signed-off-by: Spencer Schrock * fix cii-best-practices eval Signed-off-by: Spencer Schrock * fix signed-releases eval Signed-off-by: Spencer Schrock * fix sast eval Signed-off-by: Spencer Schrock * fix maintained eval Signed-off-by: Spencer Schrock * fix permissions eval Signed-off-by: Spencer Schrock * appease the linter Signed-off-by: Spencer Schrock * standardize maintained key names Signed-off-by: Spencer Schrock * set lookback days value regardless of outcome Signed-off-by: Spencer Schrock --------- Signed-off-by: Spencer Schrock --- checks/evaluation/cii_best_practices.go | 25 +++---- checks/evaluation/cii_best_practices_test.go | 28 +++---- checks/evaluation/maintained.go | 16 ++-- checks/evaluation/maintained_test.go | 42 ++++++----- checks/evaluation/permissions/permissions.go | 74 +++++++++---------- checks/evaluation/pinned_dependencies.go | 33 +-------- checks/evaluation/sast.go | 25 +++++-- checks/evaluation/sast_test.go | 30 ++++---- checks/evaluation/signed_releases.go | 21 ++---- checks/evaluation/signed_releases_test.go | 24 +++--- finding/finding.go | 8 +- probes/blocksDeleteOnBranches/impl.go | 9 ++- probes/blocksForcePushOnBranches/impl.go | 9 ++- .../branchProtectionAppliesToAdmins/impl.go | 9 ++- probes/dismissesStaleReviews/impl.go | 9 ++- probes/hasOpenSSFBadge/impl.go | 5 +- probes/hasRecentCommits/impl.go | 43 +++++------ probes/hasRecentCommits/impl_test.go | 15 ++-- probes/issueActivityByProjectMember/impl.go | 42 +++++------ .../issueActivityByProjectMember/impl_test.go | 18 +++-- probes/notCreatedRecently/impl.go | 38 ++++------ probes/releasesAreSigned/impl.go | 22 ++---- probes/releasesHaveProvenance/impl.go | 22 ++---- .../requiresApproversForPullRequests/impl.go | 22 +++--- probes/requiresCodeOwnersReview/impl.go | 9 ++- probes/requiresLastPushApproval/impl.go | 9 ++- probes/requiresUpToDateBranches/impl.go | 9 ++- probes/runsStatusChecksBeforeMerging/impl.go | 13 ++-- probes/sastToolRunsOnAllCommits/impl.go | 5 +- probes/sastToolRunsOnAllCommits/impl_test.go | 12 +-- 30 files changed, 302 insertions(+), 344 deletions(-) diff --git a/checks/evaluation/cii_best_practices.go b/checks/evaluation/cii_best_practices.go index 0dd8511c..31672d5e 100644 --- a/checks/evaluation/cii_best_practices.go +++ b/checks/evaluation/cii_best_practices.go @@ -56,27 +56,26 @@ func CIIBestPractices(name string, text = "no effort to earn an OpenSSF best practices badge detected" return checker.CreateMinScoreResult(name, text) } - //nolint:nestif - if _, hasKey := f.Values[hasOpenSSFBadge.GoldLevel]; hasKey { + + level, ok := f.Values[hasOpenSSFBadge.LevelKey] + if !ok { + return checker.CreateRuntimeErrorResult(name, sce.WithMessage(sce.ErrScorecardInternal, "no badge level present")) + } + switch level { + case hasOpenSSFBadge.GoldLevel: score = checker.MaxResultScore text = "badge detected: Gold" - } else if _, hasKey := f.Values[hasOpenSSFBadge.SilverLevel]; hasKey { + case hasOpenSSFBadge.SilverLevel: score = silverScore text = "badge detected: Silver" - } else if _, hasKey := f.Values[hasOpenSSFBadge.PassingLevel]; hasKey { + case hasOpenSSFBadge.PassingLevel: score = passingScore text = "badge detected: Passing" - } else if _, hasKey := f.Values[hasOpenSSFBadge.InProgressLevel]; hasKey { + case hasOpenSSFBadge.InProgressLevel: score = inProgressScore text = "badge detected: InProgress" - } else if _, hasKey := f.Values[hasOpenSSFBadge.UnknownLevel]; hasKey { - text = "unknown badge detected" - e := sce.WithMessage(sce.ErrScorecardInternal, text) - return checker.CreateRuntimeErrorResult(name, e) - } else { - text = "unsupported badge detected" - e := sce.WithMessage(sce.ErrScorecardInternal, text) - return checker.CreateRuntimeErrorResult(name, e) + default: + return checker.CreateRuntimeErrorResult(name, sce.WithMessage(sce.ErrScorecardInternal, "unsupported badge detected")) } return checker.CreateResultWithScore(name, text, score) diff --git a/checks/evaluation/cii_best_practices_test.go b/checks/evaluation/cii_best_practices_test.go index 5cfb08c2..ca7eb518 100644 --- a/checks/evaluation/cii_best_practices_test.go +++ b/checks/evaluation/cii_best_practices_test.go @@ -35,8 +35,8 @@ func TestCIIBestPractices(t *testing.T) { { Probe: "hasOpenSSFBadge", Outcome: finding.OutcomeNegative, - Values: map[string]int{ - "Unsupported": 1, + Values: map[string]string{ + hasOpenSSFBadge.LevelKey: "Unsupported", }, }, }, @@ -50,8 +50,8 @@ func TestCIIBestPractices(t *testing.T) { { Probe: "hasOpenSSFBadge", Outcome: finding.OutcomePositive, - Values: map[string]int{ - "Unsupported": 1, + Values: map[string]string{ + hasOpenSSFBadge.LevelKey: "Unsupported", }, }, }, @@ -66,8 +66,8 @@ func TestCIIBestPractices(t *testing.T) { { Probe: "hasOpenSSFBadge", Outcome: finding.OutcomePositive, - Values: map[string]int{ - hasOpenSSFBadge.InProgressLevel: 1, + Values: map[string]string{ + hasOpenSSFBadge.LevelKey: hasOpenSSFBadge.InProgressLevel, }, }, }, @@ -81,8 +81,8 @@ func TestCIIBestPractices(t *testing.T) { { Probe: "hasOpenSSFBadge", Outcome: finding.OutcomePositive, - Values: map[string]int{ - hasOpenSSFBadge.PassingLevel: 1, + Values: map[string]string{ + hasOpenSSFBadge.LevelKey: hasOpenSSFBadge.PassingLevel, }, }, }, @@ -96,8 +96,8 @@ func TestCIIBestPractices(t *testing.T) { { Probe: "hasOpenSSFBadge", Outcome: finding.OutcomePositive, - Values: map[string]int{ - hasOpenSSFBadge.SilverLevel: 1, + Values: map[string]string{ + hasOpenSSFBadge.LevelKey: hasOpenSSFBadge.SilverLevel, }, }, }, @@ -111,8 +111,8 @@ func TestCIIBestPractices(t *testing.T) { { Probe: "hasOpenSSFBadge", Outcome: finding.OutcomePositive, - Values: map[string]int{ - hasOpenSSFBadge.GoldLevel: 1, + Values: map[string]string{ + hasOpenSSFBadge.LevelKey: hasOpenSSFBadge.GoldLevel, }, }, }, @@ -126,8 +126,8 @@ func TestCIIBestPractices(t *testing.T) { { Probe: "hasOpenSSFBadge", Outcome: finding.OutcomePositive, - Values: map[string]int{ - "Unknown": 1, + Values: map[string]string{ + hasOpenSSFBadge.LevelKey: hasOpenSSFBadge.UnknownLevel, }, }, }, diff --git a/checks/evaluation/maintained.go b/checks/evaluation/maintained.go index 3be51d57..5ccdb0ed 100644 --- a/checks/evaluation/maintained.go +++ b/checks/evaluation/maintained.go @@ -16,6 +16,7 @@ package evaluation import ( "fmt" + "strconv" "github.com/ossf/scorecard/v4/checker" sce "github.com/ossf/scorecard/v4/errors" @@ -59,17 +60,22 @@ func Maintained(name string, return checker.CreateMinScoreResult(name, "project was created in last 90 days. please review its contents carefully") } - commitsWithinThreshold := 0 - numberOfIssuesUpdatedWithinThreshold := 0 - + var commitsWithinThreshold, numberOfIssuesUpdatedWithinThreshold int + var err error for i := range findings { f := &findings[i] if f.Outcome == finding.OutcomePositive { switch f.Probe { case issueActivityByProjectMember.Probe: - numberOfIssuesUpdatedWithinThreshold = f.Values[issueActivityByProjectMember.NoOfIssuesValue] + numberOfIssuesUpdatedWithinThreshold, err = strconv.Atoi(f.Values[issueActivityByProjectMember.NumIssuesKey]) + if err != nil { + return checker.CreateRuntimeErrorResult(name, sce.WithMessage(sce.ErrScorecardInternal, err.Error())) + } case hasRecentCommits.Probe: - commitsWithinThreshold = f.Values[hasRecentCommits.CommitsValue] + commitsWithinThreshold, err = strconv.Atoi(f.Values[hasRecentCommits.NumCommitsKey]) + if err != nil { + return checker.CreateRuntimeErrorResult(name, sce.WithMessage(sce.ErrScorecardInternal, err.Error())) + } } } } diff --git a/checks/evaluation/maintained_test.go b/checks/evaluation/maintained_test.go index d504a0c0..84680deb 100644 --- a/checks/evaluation/maintained_test.go +++ b/checks/evaluation/maintained_test.go @@ -18,6 +18,10 @@ import ( sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/hasRecentCommits" + "github.com/ossf/scorecard/v4/probes/issueActivityByProjectMember" + "github.com/ossf/scorecard/v4/probes/notArchived" + "github.com/ossf/scorecard/v4/probes/notCreatedRecently" scut "github.com/ossf/scorecard/v4/utests" ) @@ -32,22 +36,22 @@ func TestMaintained(t *testing.T) { name: "Two commits in last 90 days", findings: []finding.Finding{ { - Probe: "hasRecentCommits", + Probe: hasRecentCommits.Probe, Outcome: finding.OutcomePositive, - Values: map[string]int{ - "commitsWithinThreshold": 2, + Values: map[string]string{ + hasRecentCommits.NumCommitsKey: "2", }, }, { - Probe: "issueActivityByProjectMember", + Probe: issueActivityByProjectMember.Probe, Outcome: finding.OutcomePositive, - Values: map[string]int{ - "numberOfIssuesUpdatedWithinThreshold": 1, + Values: map[string]string{ + issueActivityByProjectMember.NumIssuesKey: "1", }, }, { - Probe: "notArchived", + Probe: notArchived.Probe, Outcome: finding.OutcomePositive, }, { - Probe: "notCreatedRecently", + Probe: notCreatedRecently.Probe, Outcome: finding.OutcomePositive, }, }, @@ -59,16 +63,16 @@ func TestMaintained(t *testing.T) { name: "No issues, no commits and not archived", findings: []finding.Finding{ { - Probe: "hasRecentCommits", + Probe: hasRecentCommits.Probe, Outcome: finding.OutcomeNegative, }, { - Probe: "issueActivityByProjectMember", + Probe: issueActivityByProjectMember.Probe, Outcome: finding.OutcomeNegative, }, { - Probe: "notArchived", + Probe: notArchived.Probe, Outcome: finding.OutcomePositive, }, { - Probe: "notCreatedRecently", + Probe: notCreatedRecently.Probe, Outcome: finding.OutcomePositive, }, }, @@ -80,16 +84,16 @@ func TestMaintained(t *testing.T) { name: "Wrong probe name", findings: []finding.Finding{ { - Probe: "hasRecentCommits", + Probe: hasRecentCommits.Probe, Outcome: finding.OutcomeNegative, }, { - Probe: "issueActivityByProjectMember", + Probe: issueActivityByProjectMember.Probe, Outcome: finding.OutcomeNegative, }, { Probe: "archvied", /*misspelling*/ Outcome: finding.OutcomePositive, }, { - Probe: "notCreatedRecently", + Probe: notCreatedRecently.Probe, Outcome: finding.OutcomePositive, }, }, @@ -102,16 +106,16 @@ func TestMaintained(t *testing.T) { name: "Project is archived", findings: []finding.Finding{ { - Probe: "hasRecentCommits", + Probe: hasRecentCommits.Probe, Outcome: finding.OutcomeNegative, }, { - Probe: "issueActivityByProjectMember", + Probe: issueActivityByProjectMember.Probe, Outcome: finding.OutcomeNegative, }, { - Probe: "notArchived", + Probe: notArchived.Probe, Outcome: finding.OutcomeNegative, }, { - Probe: "notCreatedRecently", + Probe: notCreatedRecently.Probe, Outcome: finding.OutcomePositive, }, }, diff --git a/checks/evaluation/permissions/permissions.go b/checks/evaluation/permissions/permissions.go index 3aebd06c..23ee6c2a 100644 --- a/checks/evaluation/permissions/permissions.go +++ b/checks/evaluation/permissions/permissions.go @@ -38,56 +38,56 @@ var ( topNoWriteID = "gitHubWorkflowPermissionsTopNoWrite" ) -type permissionLevel int +type permissionLevel string const ( // permissionLevelNone is a permission set to `none`. - permissionLevelNone permissionLevel = iota + permissionLevelNone permissionLevel = "none" // permissionLevelRead is a permission set to `read`. - permissionLevelRead + permissionLevelRead permissionLevel = "read" // permissionLevelUnknown is for other kinds of alerts, mostly to support debug messages. // TODO: remove it once we have implemented severity (#1874). - permissionLevelUnknown + permissionLevelUnknown permissionLevel = "unknown" // permissionLevelUndeclared is an undeclared permission. - permissionLevelUndeclared + permissionLevelUndeclared permissionLevel = "undeclared" // permissionLevelWrite is a permission set to `write` for a permission we consider potentially dangerous. - permissionLevelWrite + permissionLevelWrite permissionLevel = "write" ) // permissionLocation represents a declaration type. -type permissionLocationType int +type permissionLocationType string const ( // permissionLocationNil is in case the permission is nil. - permissionLocationNil permissionLocationType = iota + permissionLocationNil permissionLocationType = "nil" // permissionLocationNotDeclared is for undeclared permission. - permissionLocationNotDeclared + permissionLocationNotDeclared permissionLocationType = "not declared" // permissionLocationTop is top-level workflow permission. - permissionLocationTop + permissionLocationTop permissionLocationType = "top" // permissionLocationJob is job-level workflow permission. - permissionLocationJob + permissionLocationJob permissionLocationType = "job" ) // permissionType represents a permission type. -type permissionType int +type permissionType string const ( // permissionTypeNone represents none permission type. - permissionTypeNone permissionType = iota + permissionTypeNone permissionType = "none" // permissionTypeNone is the "all" github permission type. - permissionTypeAll + permissionTypeAll permissionType = "all" // permissionTypeNone is the "statuses" github permission type. - permissionTypeStatuses + permissionTypeStatuses permissionType = "statuses" // permissionTypeNone is the "checks" github permission type. - permissionTypeChecks + permissionTypeChecks permissionType = "checks" // permissionTypeNone is the "security-events" github permission type. - permissionTypeSecurityEvents + permissionTypeSecurityEvents permissionType = "security-events" // permissionTypeNone is the "deployments" github permission type. - permissionTypeDeployments + permissionTypeDeployments permissionType = "deployments" // permissionTypeNone is the "packages" github permission type. - permissionTypePackages + permissionTypePackages permissionType = "packages" // permissionTypeNone is the "actions" github permission type. - permissionTypeActions + permissionTypeActions permissionType = "actions" ) // TokenPermissions applies the score policy for the Token-Permissions check. @@ -152,19 +152,13 @@ func rawToFindings(results *checker.TokenPermissionsData) ([]finding.Finding, er switch r.Type { case checker.PermissionLevelNone: f = f.WithOutcome(finding.OutcomePositive) - f = f.WithValues(map[string]int{ - "PermissionLevel": int(permissionLevelNone), - }) + f = f.WithValue("PermissionLevel", string(permissionLevelNone)) case checker.PermissionLevelRead: f = f.WithOutcome(finding.OutcomePositive) - f = f.WithValues(map[string]int{ - "PermissionLevel": int(permissionLevelRead), - }) - + f = f.WithValue("PermissionLevel", string(permissionLevelRead)) case checker.PermissionLevelUnknown: - f = f.WithValues(map[string]int{ - "PermissionLevel": int(permissionLevelUnknown), - }).WithOutcome(finding.OutcomeError) + f = f.WithValue("PermissionLevel", string(permissionLevelUnknown)) + f = f.WithOutcome(finding.OutcomeError) case checker.PermissionLevelUndeclared: var locationType permissionLocationType //nolint:gocritic @@ -176,10 +170,10 @@ func rawToFindings(results *checker.TokenPermissionsData) ([]finding.Finding, er locationType = permissionLocationNotDeclared } permType := permTypeToEnum(r.Name) - f = f.WithValues(map[string]int{ - "PermissionLevel": int(permissionLevelUndeclared), - "LocationType": int(locationType), - "PermissionType": int(permType), + f = f.WithValues(map[string]string{ + "PermissionLevel": string(permissionLevelUndeclared), + "LocationType": string(locationType), + "PermissionType": string(permType), }) case checker.PermissionLevelWrite: var locationType permissionLocationType @@ -192,10 +186,10 @@ func rawToFindings(results *checker.TokenPermissionsData) ([]finding.Finding, er locationType = permissionLocationNotDeclared } permType := permTypeToEnum(r.Name) - f = f.WithValues(map[string]int{ - "PermissionLevel": int(permissionLevelWrite), - "LocationType": int(locationType), - "PermissionType": int(permType), + f = f.WithValues(map[string]string{ + "PermissionLevel": string(permissionLevelWrite), + "LocationType": string(locationType), + "PermissionType": string(permType), }) f = f.WithOutcome(finding.OutcomeNegative) } @@ -229,7 +223,7 @@ func permTypeToEnum(tokenName *string) permissionType { } } -func permTypeToName(permType int) *string { +func permTypeToName(permType string) *string { var permName string switch permissionType(permType) { case permissionTypeAll: @@ -401,7 +395,7 @@ func warnWithRemediation(logger checker.DetailLogger, } func recordPermissionWrite(hm map[string]permissions, path string, - locType permissionLocationType, permType int, + locType permissionLocationType, permType string, ) { if _, exists := hm[path]; !exists { hm[path] = permissions{ diff --git a/checks/evaluation/pinned_dependencies.go b/checks/evaluation/pinned_dependencies.go index 6344c9bf..368a8587 100644 --- a/checks/evaluation/pinned_dependencies.go +++ b/checks/evaluation/pinned_dependencies.go @@ -54,29 +54,6 @@ const ( depTypeKey = "dependencyType" ) -var ( - dependencyTypes = map[checker.DependencyUseType]int{ - checker.DependencyUseTypeGHAction: 0, - checker.DependencyUseTypeDockerfileContainerImage: 1, - checker.DependencyUseTypeDownloadThenRun: 2, - checker.DependencyUseTypeGoCommand: 3, - checker.DependencyUseTypeChocoCommand: 4, - checker.DependencyUseTypeNpmCommand: 5, - checker.DependencyUseTypePipCommand: 6, - checker.DependencyUseTypeNugetCommand: 7, - } - intToDepType = map[int]checker.DependencyUseType{ - 0: checker.DependencyUseTypeGHAction, - 1: checker.DependencyUseTypeDockerfileContainerImage, - 2: checker.DependencyUseTypeDownloadThenRun, - 3: checker.DependencyUseTypeGoCommand, - 4: checker.DependencyUseTypeChocoCommand, - 5: checker.DependencyUseTypeNpmCommand, - 6: checker.DependencyUseTypePipCommand, - 7: checker.DependencyUseTypeNugetCommand, - } -) - func ruleRemToProbeRem(rem *rule.Remediation) *probe.Remediation { return &probe.Remediation{ Patch: rem.Patch, @@ -174,9 +151,7 @@ func dependenciesToFindings(r *checker.PinningDependenciesData) ([]finding.Findi if rr.Remediation != nil { f.Remediation = ruleRemToProbeRem(rr.Remediation) } - f = f.WithValues(map[string]int{ - depTypeKey: dependencyTypes[rr.Type], - }) + f = f.WithValue(depTypeKey, string(rr.Type)) findings = append(findings, *f) } else { loc := &finding.Location{ @@ -191,9 +166,7 @@ func dependenciesToFindings(r *checker.PinningDependenciesData) ([]finding.Findi Outcome: finding.OutcomePositive, Location: loc, } - f = f.WithValues(map[string]int{ - depTypeKey: dependencyTypes[rr.Type], - }) + f = f.WithValue(depTypeKey, string(rr.Type)) findings = append(findings, *f) } } @@ -259,7 +232,7 @@ func PinningDependencies(name string, c *checker.CheckRequest, default: // ignore } - updatePinningResults(intToDepType[f.Values[depTypeKey]], + updatePinningResults(checker.DependencyUseType(f.Values[depTypeKey]), f.Outcome, f.Location.Snippet, &wp, pr) } diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go index f6a6897c..a01ecf95 100644 --- a/checks/evaluation/sast.go +++ b/checks/evaluation/sast.go @@ -15,6 +15,9 @@ package evaluation import ( + "fmt" + "strconv" + "github.com/ossf/scorecard/v4/checker" sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/finding" @@ -46,12 +49,16 @@ func SAST(name string, } var sastScore, codeQlScore, pysaScore, qodanaScore, snykScore, sonarScore int + var err error // Assign sastScore, codeQlScore and sonarScore for i := range findings { f := &findings[i] switch f.Probe { case sastToolRunsOnAllCommits.Probe: - sastScore = getSASTScore(f, dl) + sastScore, err = getSASTScore(f, dl) + if err != nil { + return checker.CreateRuntimeErrorResult(name, sce.WithMessage(sce.ErrScorecardInternal, err.Error())) + } case sastToolCodeQLInstalled.Probe: codeQlScore = getSastToolScore(f, dl) case sastToolSnykInstalled.Probe: @@ -147,13 +154,13 @@ func SAST(name string, // getSASTScore returns the proportional score of how many commits // run SAST tools. -func getSASTScore(f *finding.Finding, dl checker.DetailLogger) int { +func getSASTScore(f *finding.Finding, dl checker.DetailLogger) (int, error) { switch f.Outcome { case finding.OutcomeNotApplicable: dl.Warn(&checker.LogMessage{ Text: f.Message, }) - return checker.InconclusiveResultScore + return checker.InconclusiveResultScore, nil case finding.OutcomePositive: dl.Info(&checker.LogMessage{ Text: f.Message, @@ -164,9 +171,15 @@ func getSASTScore(f *finding.Finding, dl checker.DetailLogger) int { }) default: } - analyzed := f.Values[sastToolRunsOnAllCommits.AnalyzedPRsKey] - total := f.Values[sastToolRunsOnAllCommits.TotalPRsKey] - return checker.CreateProportionalScore(analyzed, total) + analyzed, err := strconv.Atoi(f.Values[sastToolRunsOnAllCommits.AnalyzedPRsKey]) + if err != nil { + return 0, fmt.Errorf("parsing analyzed PR count: %w", err) + } + total, err := strconv.Atoi(f.Values[sastToolRunsOnAllCommits.TotalPRsKey]) + if err != nil { + return 0, fmt.Errorf("parsing total PR count: %w", err) + } + return checker.CreateProportionalScore(analyzed, total), nil } // getSastToolScore returns positive if the project runs the Sast tool diff --git a/checks/evaluation/sast_test.go b/checks/evaluation/sast_test.go index 656ebe9a..350f8ec5 100644 --- a/checks/evaluation/sast_test.go +++ b/checks/evaluation/sast_test.go @@ -76,9 +76,9 @@ func TestSAST(t *testing.T) { { Probe: sastToolRunsOnAllCommits.Probe, Outcome: finding.OutcomePositive, - Values: map[string]int{ - sastToolRunsOnAllCommits.AnalyzedPRsKey: 1, - sastToolRunsOnAllCommits.TotalPRsKey: 2, + Values: map[string]string{ + sastToolRunsOnAllCommits.AnalyzedPRsKey: "1", + sastToolRunsOnAllCommits.TotalPRsKey: "2", }, }, { @@ -121,9 +121,9 @@ func TestSAST(t *testing.T) { { Probe: sastToolRunsOnAllCommits.Probe, Outcome: finding.OutcomePositive, - Values: map[string]int{ - sastToolRunsOnAllCommits.AnalyzedPRsKey: 1, - sastToolRunsOnAllCommits.TotalPRsKey: 2, + Values: map[string]string{ + sastToolRunsOnAllCommits.AnalyzedPRsKey: "1", + sastToolRunsOnAllCommits.TotalPRsKey: "2", }, }, { @@ -202,9 +202,9 @@ func TestSAST(t *testing.T) { { Probe: sastToolRunsOnAllCommits.Probe, Outcome: finding.OutcomeNegative, - Values: map[string]int{ - sastToolRunsOnAllCommits.AnalyzedPRsKey: 1, - sastToolRunsOnAllCommits.TotalPRsKey: 3, + Values: map[string]string{ + sastToolRunsOnAllCommits.AnalyzedPRsKey: "1", + sastToolRunsOnAllCommits.TotalPRsKey: "3", }, }, { @@ -232,9 +232,9 @@ func TestSAST(t *testing.T) { { Probe: sastToolRunsOnAllCommits.Probe, Outcome: finding.OutcomePositive, - Values: map[string]int{ - sastToolRunsOnAllCommits.AnalyzedPRsKey: 1, - sastToolRunsOnAllCommits.TotalPRsKey: 3, + Values: map[string]string{ + sastToolRunsOnAllCommits.AnalyzedPRsKey: "1", + sastToolRunsOnAllCommits.TotalPRsKey: "3", }, }, { @@ -270,9 +270,9 @@ func TestSAST(t *testing.T) { { Probe: sastToolRunsOnAllCommits.Probe, Outcome: finding.OutcomePositive, - Values: map[string]int{ - sastToolRunsOnAllCommits.AnalyzedPRsKey: 1, - sastToolRunsOnAllCommits.TotalPRsKey: 3, + Values: map[string]string{ + sastToolRunsOnAllCommits.AnalyzedPRsKey: "1", + sastToolRunsOnAllCommits.TotalPRsKey: "3", }, }, { diff --git a/checks/evaluation/signed_releases.go b/checks/evaluation/signed_releases.go index 97b07b51..c580a561 100644 --- a/checks/evaluation/signed_releases.go +++ b/checks/evaluation/signed_releases.go @@ -130,20 +130,15 @@ func SignedReleases(name string, } func getReleaseName(f *finding.Finding) string { - m := f.Values - for k, v := range m { - var value int - switch f.Probe { - case releasesAreSigned.Probe: - value = int(releasesAreSigned.ValueTypeRelease) - case releasesHaveProvenance.Probe: - value = int(releasesHaveProvenance.ValueTypeRelease) - } - if v == value { - return k - } + var key string + // these keys should be the same, but might as handle situations when they're not + switch f.Probe { + case releasesAreSigned.Probe: + key = releasesAreSigned.ReleaseNameKey + case releasesHaveProvenance.Probe: + key = releasesHaveProvenance.ReleaseNameKey } - return "" + return f.Values[key] } func contains(releases []string, release string) bool { diff --git a/checks/evaluation/signed_releases_test.go b/checks/evaluation/signed_releases_test.go index e02516f4..e4d359c1 100644 --- a/checks/evaluation/signed_releases_test.go +++ b/checks/evaluation/signed_releases_test.go @@ -46,9 +46,9 @@ func signedProbe(release, asset int, outcome finding.Outcome) finding.Finding { return finding.Finding{ Probe: releasesAreSigned.Probe, Outcome: outcome, - Values: map[string]int{ - fmt.Sprintf("v%d", release): int(releasesAreSigned.ValueTypeRelease), - fmt.Sprintf("artifact-%d", asset): int(releasesAreSigned.ValueTypeReleaseAsset), + Values: map[string]string{ + releasesAreSigned.ReleaseNameKey: fmt.Sprintf("v%d", release), + releasesAreSigned.AssetNameKey: fmt.Sprintf("artifact-%d", asset), }, } } @@ -57,9 +57,9 @@ func provenanceProbe(release, asset int, outcome finding.Outcome) finding.Findin return finding.Finding{ Probe: releasesHaveProvenance.Probe, Outcome: outcome, - Values: map[string]int{ - fmt.Sprintf("v%d", release): int(releasesHaveProvenance.ValueTypeRelease), - fmt.Sprintf("artifact-%d", asset): int(releasesHaveProvenance.ValueTypeReleaseAsset), + Values: map[string]string{ + releasesHaveProvenance.ReleaseNameKey: fmt.Sprintf("v%d", release), + releasesHaveProvenance.AssetNameKey: fmt.Sprintf("artifact-%d", asset), }, } } @@ -322,7 +322,7 @@ func Test_getReleaseName(t *testing.T) { name: "no release", args: args{ f: &finding.Finding{ - Values: map[string]int{}, + Values: map[string]string{}, }, }, want: "", @@ -331,8 +331,8 @@ func Test_getReleaseName(t *testing.T) { name: "release", args: args{ f: &finding.Finding{ - Values: map[string]int{ - "v1": int(releasesAreSigned.ValueTypeRelease), + Values: map[string]string{ + releasesAreSigned.ReleaseNameKey: "v1", }, Probe: releasesAreSigned.Probe, }, @@ -343,9 +343,9 @@ func Test_getReleaseName(t *testing.T) { name: "release and asset", args: args{ f: &finding.Finding{ - Values: map[string]int{ - "v1": int(releasesAreSigned.ValueTypeRelease), - "artifact-1": int(releasesAreSigned.ValueTypeReleaseAsset), + Values: map[string]string{ + releasesAreSigned.ReleaseNameKey: "v1", + releasesAreSigned.AssetNameKey: "artifact-1", }, Probe: releasesAreSigned.Probe, }, diff --git a/finding/finding.go b/finding/finding.go index 451a4b8f..cb7c38c0 100644 --- a/finding/finding.go +++ b/finding/finding.go @@ -97,7 +97,7 @@ const ( type Finding struct { Location *Location `json:"location,omitempty"` Remediation *probe.Remediation `json:"remediation,omitempty"` - Values map[string]int `json:"values,omitempty"` + Values map[string]string `json:"values,omitempty"` Probe string `json:"probe"` Message string `json:"message"` Outcome Outcome `json:"outcome"` @@ -221,7 +221,7 @@ func (f *Finding) WithLocation(loc *Location) *Finding { // WithValues sets the values to an existing finding. // No copy is made. -func (f *Finding) WithValues(values map[string]int) *Finding { +func (f *Finding) WithValues(values map[string]string) *Finding { f.Values = values return f } @@ -266,9 +266,9 @@ func (f *Finding) WithRemediationMetadata(values map[string]string) *Finding { // WithValue adds a value to f.Values. // No copy is made. -func (f *Finding) WithValue(k string, v int) *Finding { +func (f *Finding) WithValue(k, v string) *Finding { if f.Values == nil { - f.Values = make(map[string]int) + f.Values = make(map[string]string) } f.Values[k] = v return f diff --git a/probes/blocksDeleteOnBranches/impl.go b/probes/blocksDeleteOnBranches/impl.go index d8c67ebc..bd295fa4 100644 --- a/probes/blocksDeleteOnBranches/impl.go +++ b/probes/blocksDeleteOnBranches/impl.go @@ -27,7 +27,10 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "blocksDeleteOnBranches" +const ( + Probe = "blocksDeleteOnBranches" + BranchNameKey = "branchName" +) func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { @@ -58,9 +61,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValues(map[string]int{ - *branch.Name: 1, - }) + f = f.WithValue(BranchNameKey, *branch.Name) findings = append(findings, *f) } return findings, Probe, nil diff --git a/probes/blocksForcePushOnBranches/impl.go b/probes/blocksForcePushOnBranches/impl.go index 72bee662..d87921c1 100644 --- a/probes/blocksForcePushOnBranches/impl.go +++ b/probes/blocksForcePushOnBranches/impl.go @@ -27,7 +27,10 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "blocksForcePushOnBranches" +const ( + Probe = "blocksForcePushOnBranches" + BranchNameKey = "branchName" +) func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { @@ -57,9 +60,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValues(map[string]int{ - *branch.Name: 1, - }) + f = f.WithValue(BranchNameKey, *branch.Name) findings = append(findings, *f) } return findings, Probe, nil diff --git a/probes/branchProtectionAppliesToAdmins/impl.go b/probes/branchProtectionAppliesToAdmins/impl.go index 7c8099aa..6170ff5c 100644 --- a/probes/branchProtectionAppliesToAdmins/impl.go +++ b/probes/branchProtectionAppliesToAdmins/impl.go @@ -28,7 +28,10 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "branchProtectionAppliesToAdmins" +const ( + Probe = "branchProtectionAppliesToAdmins" + BranchNameKey = "branchName" +) func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { @@ -52,9 +55,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValues(map[string]int{ - *branch.Name: 1, - }) + f = f.WithValue(BranchNameKey, *branch.Name) findings = append(findings, *f) } return findings, Probe, nil diff --git a/probes/dismissesStaleReviews/impl.go b/probes/dismissesStaleReviews/impl.go index bb21e1c3..fd63d764 100644 --- a/probes/dismissesStaleReviews/impl.go +++ b/probes/dismissesStaleReviews/impl.go @@ -28,7 +28,10 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "dismissesStaleReviews" +const ( + Probe = "dismissesStaleReviews" + BranchNameKey = "branchName" +) func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { @@ -52,9 +55,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValues(map[string]int{ - *branch.Name: 1, - }) + f = f.WithValue(BranchNameKey, *branch.Name) findings = append(findings, *f) } return findings, Probe, nil diff --git a/probes/hasOpenSSFBadge/impl.go b/probes/hasOpenSSFBadge/impl.go index 009326da..e254d31a 100644 --- a/probes/hasOpenSSFBadge/impl.go +++ b/probes/hasOpenSSFBadge/impl.go @@ -30,6 +30,7 @@ var fs embed.FS const ( Probe = "hasOpenSSFBadge" + LevelKey = "badgeLevel" GoldLevel = "Gold" SilverLevel = "Silver" PassingLevel = "Passing" @@ -73,8 +74,6 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValues(map[string]int{ - badgeLevel: 1, - }) + f = f.WithValue(LevelKey, badgeLevel) return []finding.Finding{*f}, Probe, nil } diff --git a/probes/hasRecentCommits/impl.go b/probes/hasRecentCommits/impl.go index 8b4fb866..26620a45 100644 --- a/probes/hasRecentCommits/impl.go +++ b/probes/hasRecentCommits/impl.go @@ -18,6 +18,7 @@ package hasRecentCommits import ( "embed" "fmt" + "strconv" "time" "github.com/ossf/scorecard/v4/checker" @@ -29,13 +30,12 @@ import ( var fs embed.FS const ( - lookBackDays = 90 - CommitsValue = "commitsWithinThreshold" - LookBackValue = "lookBackDays" + Probe = "hasRecentCommits" + NumCommitsKey = "commitsWithinThreshold" + LookbackDayKey = "lookBackDays" + lookBackDays = 90 ) -const Probe = "hasRecentCommits" - func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) @@ -54,27 +54,24 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } } + var text string + var outcome finding.Outcome if commitsWithinThreshold > 0 { - f, err := finding.NewWith(fs, Probe, - "Found a contribution within the threshold.", nil, - finding.OutcomePositive) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - f = f.WithValues(map[string]int{ - CommitsValue: commitsWithinThreshold, - LookBackValue: lookBackDays, - }) - findings = append(findings, *f) + text = "Found a contribution within the threshold." + outcome = finding.OutcomePositive } else { - f, err := finding.NewWith(fs, Probe, - "Did not find contribution within the threshold.", nil, - finding.OutcomeNegative) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - findings = append(findings, *f) + text = "Did not find contribution within the threshold." + outcome = finding.OutcomeNegative } + f, err := finding.NewWith(fs, Probe, text, nil, outcome) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + f = f.WithValues(map[string]string{ + NumCommitsKey: strconv.Itoa(commitsWithinThreshold), + LookbackDayKey: strconv.Itoa(lookBackDays), + }) + findings = append(findings, *f) return findings, Probe, nil } diff --git a/probes/hasRecentCommits/impl_test.go b/probes/hasRecentCommits/impl_test.go index 383c7f37..8d7d72a1 100644 --- a/probes/hasRecentCommits/impl_test.go +++ b/probes/hasRecentCommits/impl_test.go @@ -16,6 +16,7 @@ package hasRecentCommits import ( + "strconv" "testing" "time" @@ -56,7 +57,7 @@ func Test_Run(t *testing.T) { name string raw *checker.RawResults outcomes []finding.Outcome - values map[string]int + values map[string]string err error }{ { @@ -75,9 +76,9 @@ func Test_Run(t *testing.T) { DefaultBranchCommits: fiveCommitsInThreshold(), }, }, - values: map[string]int{ - "commitsWithinThreshold": 5, - "lookBackDays": 90, + values: map[string]string{ + NumCommitsKey: "5", + LookbackDayKey: strconv.Itoa(lookBackDays), }, outcomes: []finding.Outcome{finding.OutcomePositive}, }, @@ -88,9 +89,9 @@ func Test_Run(t *testing.T) { DefaultBranchCommits: twentyCommitsInThresholdAndTwentyNot(), }, }, - values: map[string]int{ - "commitsWithinThreshold": 20, - "lookBackDays": 90, + values: map[string]string{ + NumCommitsKey: "20", + LookbackDayKey: strconv.Itoa(lookBackDays), }, outcomes: []finding.Outcome{finding.OutcomePositive}, }, diff --git a/probes/issueActivityByProjectMember/impl.go b/probes/issueActivityByProjectMember/impl.go index 04d000cd..6402c9aa 100644 --- a/probes/issueActivityByProjectMember/impl.go +++ b/probes/issueActivityByProjectMember/impl.go @@ -18,6 +18,7 @@ package issueActivityByProjectMember import ( "embed" "fmt" + "strconv" "time" "github.com/ossf/scorecard/v4/checker" @@ -30,12 +31,12 @@ import ( var fs embed.FS const ( - lookBackDays = 90 - NoOfIssuesValue = "numberOfIssuesUpdatedWithinThreshold" + Probe = "issueActivityByProjectMember" + NumIssuesKey = "numberOfIssuesUpdatedWithinThreshold" + LookbackDayKey = "lookBackDays" + lookBackDays = 90 ) -const Probe = "issueActivityByProjectMember" - func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) @@ -53,27 +54,26 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } } + var text string + var outcome finding.Outcome if numberOfIssuesUpdatedWithinThreshold > 0 { - f, err := finding.NewWith(fs, Probe, - "Found a issue within the threshold.", nil, - finding.OutcomePositive) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - f = f.WithValues(map[string]int{ - NoOfIssuesValue: numberOfIssuesUpdatedWithinThreshold, - }) - findings = append(findings, *f) + text = "Found a issue within the threshold." + outcome = finding.OutcomePositive } else { - f, err := finding.NewWith(fs, Probe, - "Did not find issues within the threshold.", nil, - finding.OutcomeNegative) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - findings = append(findings, *f) + text = "Did not find issues within the threshold." + outcome = finding.OutcomeNegative } + f, err := finding.NewWith(fs, Probe, text, nil, outcome) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + f = f.WithValues(map[string]string{ + NumIssuesKey: strconv.Itoa(numberOfIssuesUpdatedWithinThreshold), + LookbackDayKey: strconv.Itoa(lookBackDays), + }) + findings = append(findings, *f) + return findings, Probe, nil } diff --git a/probes/issueActivityByProjectMember/impl_test.go b/probes/issueActivityByProjectMember/impl_test.go index 0b762fdb..d43dd704 100644 --- a/probes/issueActivityByProjectMember/impl_test.go +++ b/probes/issueActivityByProjectMember/impl_test.go @@ -16,6 +16,7 @@ package issueActivityByProjectMember import ( + "strconv" "testing" "time" @@ -80,7 +81,7 @@ func Test_Run(t *testing.T) { //nolint:govet tests := []struct { name string - values map[string]int + values map[string]string raw *checker.RawResults outcomes []finding.Outcome err error @@ -101,8 +102,9 @@ func Test_Run(t *testing.T) { Issues: fiveIssuesInThreshold(), }, }, - values: map[string]int{ - "numberOfIssuesUpdatedWithinThreshold": 5, + values: map[string]string{ + LookbackDayKey: strconv.Itoa(lookBackDays), + NumIssuesKey: "5", }, outcomes: []finding.Outcome{finding.OutcomePositive}, }, @@ -113,8 +115,9 @@ func Test_Run(t *testing.T) { Issues: twentyIssuesInThresholdAndTwentyNot(), }, }, - values: map[string]int{ - "numberOfIssuesUpdatedWithinThreshold": 20, + values: map[string]string{ + LookbackDayKey: strconv.Itoa(lookBackDays), + NumIssuesKey: "20", }, outcomes: []finding.Outcome{finding.OutcomePositive}, }, @@ -125,8 +128,9 @@ func Test_Run(t *testing.T) { Issues: fiveInThresholdByCollabAndFiveByFirstTimeUser(), }, }, - values: map[string]int{ - "numberOfIssuesUpdatedWithinThreshold": 5, + values: map[string]string{ + LookbackDayKey: strconv.Itoa(lookBackDays), + NumIssuesKey: "5", }, outcomes: []finding.Outcome{finding.OutcomePositive}, }, diff --git a/probes/notCreatedRecently/impl.go b/probes/notCreatedRecently/impl.go index 70919300..fa2bf1ac 100644 --- a/probes/notCreatedRecently/impl.go +++ b/probes/notCreatedRecently/impl.go @@ -18,6 +18,7 @@ package notCreatedRecently import ( "embed" "fmt" + "strconv" "time" "github.com/ossf/scorecard/v4/checker" @@ -29,10 +30,11 @@ import ( var fs embed.FS const ( - lookBackDays = 90 -) + Probe = "notCreatedRecently" -const Probe = "notCreatedRecently" + LookbackDayKey = "lookBackDays" + lookBackDays = 90 +) func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { @@ -43,31 +45,19 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { recencyThreshold := time.Now().AddDate(0 /*years*/, 0 /*months*/, -1*lookBackDays /*days*/) + var text string + var outcome finding.Outcome if r.CreatedAt.After(recencyThreshold) { - return negativeOutcome() + text = fmt.Sprintf("Repository was created in last %d days.", lookBackDays) + outcome = finding.OutcomeNegative + } else { + text = fmt.Sprintf("Repository was not created in last %d days.", lookBackDays) + outcome = finding.OutcomePositive } - return positiveOutcome() -} - -func negativeOutcome() ([]finding.Finding, string, error) { - f, err := finding.NewWith(fs, Probe, - "Repository was created in last 90 days.", nil, - finding.OutcomeNegative) + f, err := finding.NewWith(fs, Probe, text, nil, outcome) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - return []finding.Finding{*f}, Probe, nil -} - -func positiveOutcome() ([]finding.Finding, string, error) { - f, err := finding.NewWith(fs, Probe, - "Repository was not created in last 90 days.", nil, - finding.OutcomePositive) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - f = f.WithValues(map[string]int{ - "lookBackDays": 90, - }) + f = f.WithValue(LookbackDayKey, strconv.Itoa(lookBackDays)) return []finding.Finding{*f}, Probe, nil } diff --git a/probes/releasesAreSigned/impl.go b/probes/releasesAreSigned/impl.go index 21898a7f..8b7b8ea2 100644 --- a/probes/releasesAreSigned/impl.go +++ b/probes/releasesAreSigned/impl.go @@ -30,19 +30,11 @@ var fs embed.FS const ( Probe = "releasesAreSigned" + ReleaseNameKey = "releaseName" + AssetNameKey = "assetName" releaseLookBack = 5 ) -// ValueType is the type of a value in the finding values. -type ValueType int - -const ( - // ValueTypeRelease represents a release name. - ValueTypeRelease ValueType = iota - // ValueTypeReleaseAsset represents a release asset name. - ValueTypeReleaseAsset -) - var signatureExtensions = []string{".asc", ".minisig", ".sig", ".sign", ".sigstore"} func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { @@ -85,9 +77,9 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f.Values = map[string]int{ - release.TagName: int(ValueTypeRelease), - asset.Name: int(ValueTypeReleaseAsset), + f.Values = map[string]string{ + ReleaseNameKey: release.TagName, + AssetNameKey: asset.Name, } findings = append(findings, *f) signed = true @@ -113,9 +105,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f.Values = map[string]int{ - release.TagName: int(ValueTypeRelease), - } + f = f.WithValue(ReleaseNameKey, release.TagName) findings = append(findings, *f) } diff --git a/probes/releasesHaveProvenance/impl.go b/probes/releasesHaveProvenance/impl.go index 1433ab1f..703d994c 100644 --- a/probes/releasesHaveProvenance/impl.go +++ b/probes/releasesHaveProvenance/impl.go @@ -30,19 +30,11 @@ var fs embed.FS const ( Probe = "releasesHaveProvenance" + ReleaseNameKey = "releaseName" + AssetNameKey = "assetName" releaseLookBack = 5 ) -// ValueType is the type of a value in the finding values. -type ValueType int - -const ( - // ValueTypeRelease represents a release name. - ValueTypeRelease ValueType = iota - // ValueTypeReleaseAsset represents a release asset name. - ValueTypeReleaseAsset -) - var provenanceExtensions = []string{".intoto.jsonl"} //nolint:gocognit // bug hotfix @@ -86,9 +78,9 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f.Values = map[string]int{ - release.TagName: int(ValueTypeRelease), - asset.Name: int(ValueTypeReleaseAsset), + f.Values = map[string]string{ + ReleaseNameKey: release.TagName, + AssetNameKey: asset.Name, } findings = append(findings, *f) hasProvenance = true @@ -114,9 +106,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f.Values = map[string]int{ - release.TagName: int(ValueTypeRelease), - } + f = f.WithValue(ReleaseNameKey, release.TagName) findings = append(findings, *f) if totalReleases >= releaseLookBack { diff --git a/probes/requiresApproversForPullRequests/impl.go b/probes/requiresApproversForPullRequests/impl.go index fc9c526b..14f114f2 100644 --- a/probes/requiresApproversForPullRequests/impl.go +++ b/probes/requiresApproversForPullRequests/impl.go @@ -19,6 +19,7 @@ import ( "embed" "errors" "fmt" + "strconv" "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/finding" @@ -28,7 +29,11 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "requiresApproversForPullRequests" +const ( + Probe = "requiresApproversForPullRequests" + BranchNameKey = "branchName" + RequiredReviewersKey = "numberOfRequiredReviewers" +) var errWrongValue = errors.New("wrong value, should not happen") @@ -52,25 +57,16 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - + f = f.WithValue(BranchNameKey, *branch.Name) switch { case p == nil: f = f.WithMessage(nilMsg).WithOutcome(finding.OutcomeNotAvailable) - f = f.WithValues(map[string]int{ - *branch.Name: 1, - }) case *p > 0: f = f.WithMessage(trueMsg).WithOutcome(finding.OutcomePositive) - f = f.WithValues(map[string]int{ - *branch.Name: 1, - "numberOfRequiredReviewers": int(*p), - }) + f = f.WithValue(RequiredReviewersKey, strconv.Itoa(int(*p))) case *p == 0: f = f.WithMessage(falseMsg).WithOutcome(finding.OutcomeNegative) - f = f.WithValues(map[string]int{ - *branch.Name: 1, - "numberOfRequiredReviewers": int(*p), - }) + f = f.WithValue(RequiredReviewersKey, strconv.Itoa(int(*p))) default: return nil, Probe, fmt.Errorf("create finding: %w", errWrongValue) } diff --git a/probes/requiresCodeOwnersReview/impl.go b/probes/requiresCodeOwnersReview/impl.go index b7692bf6..b5ff4bc9 100644 --- a/probes/requiresCodeOwnersReview/impl.go +++ b/probes/requiresCodeOwnersReview/impl.go @@ -27,7 +27,10 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "requiresCodeOwnersReview" +const ( + Probe = "requiresCodeOwnersReview" + BranchNameKey = "branchName" +) func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { @@ -61,9 +64,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValues(map[string]int{ - *branch.Name: 1, - }) + f = f.WithValue(BranchNameKey, *branch.Name) findings = append(findings, *f) } return findings, Probe, nil diff --git a/probes/requiresLastPushApproval/impl.go b/probes/requiresLastPushApproval/impl.go index c457a71d..43ef8e53 100644 --- a/probes/requiresLastPushApproval/impl.go +++ b/probes/requiresLastPushApproval/impl.go @@ -28,7 +28,10 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "requiresLastPushApproval" +const ( + Probe = "requiresLastPushApproval" + BranchNameKey = "branchName" +) func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { @@ -50,9 +53,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValues(map[string]int{ - *branch.Name: 1, - }) + f = f.WithValue(BranchNameKey, *branch.Name) findings = append(findings, *f) } return findings, Probe, nil diff --git a/probes/requiresUpToDateBranches/impl.go b/probes/requiresUpToDateBranches/impl.go index 4e230297..fc083cf5 100644 --- a/probes/requiresUpToDateBranches/impl.go +++ b/probes/requiresUpToDateBranches/impl.go @@ -28,7 +28,10 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "requiresUpToDateBranches" +const ( + Probe = "requiresUpToDateBranches" + BranchNameKey = "branchName" +) func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { @@ -52,9 +55,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValues(map[string]int{ - *branch.Name: 1, - }) + f = f.WithValue(BranchNameKey, *branch.Name) findings = append(findings, *f) } return findings, Probe, nil diff --git a/probes/runsStatusChecksBeforeMerging/impl.go b/probes/runsStatusChecksBeforeMerging/impl.go index 940b5356..ba9c6bc6 100644 --- a/probes/runsStatusChecksBeforeMerging/impl.go +++ b/probes/runsStatusChecksBeforeMerging/impl.go @@ -27,7 +27,10 @@ import ( //go:embed *.yml var fs embed.FS -const Probe = "runsStatusChecksBeforeMerging" +const ( + Probe = "runsStatusChecksBeforeMerging" + BranchNameKey = "branchName" +) func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { @@ -47,9 +50,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValues(map[string]int{ - *branch.Name: 1, - }) + f = f.WithValue(BranchNameKey, *branch.Name) findings = append(findings, *f) default: f, err := finding.NewWith(fs, Probe, @@ -58,9 +59,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) } - f = f.WithValues(map[string]int{ - *branch.Name: 1, - }) + f = f.WithValue(BranchNameKey, *branch.Name) findings = append(findings, *f) } } diff --git a/probes/sastToolRunsOnAllCommits/impl.go b/probes/sastToolRunsOnAllCommits/impl.go index ab9daaf8..3ef9c711 100644 --- a/probes/sastToolRunsOnAllCommits/impl.go +++ b/probes/sastToolRunsOnAllCommits/impl.go @@ -18,6 +18,7 @@ package sastToolRunsOnAllCommits import ( "embed" "fmt" + "strconv" "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/finding" @@ -65,8 +66,8 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { return []finding.Finding{*f}, Probe, nil } - f = f.WithValue(AnalyzedPRsKey, totalPullRequestsAnalyzed) - f = f.WithValue(TotalPRsKey, totalPullRequestsMerged) + f = f.WithValue(AnalyzedPRsKey, strconv.Itoa(totalPullRequestsAnalyzed)) + f = f.WithValue(TotalPRsKey, strconv.Itoa(totalPullRequestsMerged)) if totalPullRequestsAnalyzed == totalPullRequestsMerged { msg := fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalPullRequestsMerged) diff --git a/probes/sastToolRunsOnAllCommits/impl_test.go b/probes/sastToolRunsOnAllCommits/impl_test.go index 329bbde8..46e023b7 100644 --- a/probes/sastToolRunsOnAllCommits/impl_test.go +++ b/probes/sastToolRunsOnAllCommits/impl_test.go @@ -57,9 +57,9 @@ func Test_Run(t *testing.T) { { Probe: Probe, Message: "1 commits out of 2 are checked with a SAST tool", - Values: map[string]int{ - AnalyzedPRsKey: 1, - TotalPRsKey: 2, + Values: map[string]string{ + AnalyzedPRsKey: "1", + TotalPRsKey: "2", }, }, }, @@ -87,9 +87,9 @@ func Test_Run(t *testing.T) { Probe: Probe, Message: "all commits (2) are checked with a SAST tool", Outcome: finding.OutcomePositive, - Values: map[string]int{ - AnalyzedPRsKey: 2, - TotalPRsKey: 2, + Values: map[string]string{ + AnalyzedPRsKey: "2", + TotalPRsKey: "2", }, }, },