Skip to content

Commit

Permalink
fix: A check should know if it's exempted or not
Browse files Browse the repository at this point in the history
Moving the verification "IsCheckExempted" from maintainers_annotation package to checker package. This way a check result will define, consulting maintainers annotation, if it is exempted or not.

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>
  • Loading branch information
gabibguti committed Mar 4, 2024
1 parent b40859a commit c5253b7
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 25 deletions.
21 changes: 21 additions & 0 deletions checker/check_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ import (
"errors"
"fmt"
"math"
"strings"

sce "github.com/ossf/scorecard/v4/errors"
"github.com/ossf/scorecard/v4/finding"
ma "github.com/ossf/scorecard/v4/maintainers_annotation"
"github.com/ossf/scorecard/v4/rule"
)

Expand Down Expand Up @@ -262,3 +264,22 @@ func LogFindings(findings []finding.Finding, dl DetailLogger) {
}
}
}

// IsExempted verifies if a given check in the results is exempted in maintainers annotation.
func (check *CheckResult) IsExempted(maintainersAnnotation ma.MaintainersAnnotation) (bool, []string) {
// If check has a maximum score, then there it doesn't make sense anymore to reason the check
// This may happen if the check score was once low but then the problem was fixed on Scorecard side
// or on the maintainers side
if check.Score == MaxResultScore {
return false, nil
}

for _, exemption := range maintainersAnnotation.Exemptions {
for _, checkName := range exemption.Checks {
if strings.EqualFold(checkName, strings.ToLower(check.Name)) {
return true, ma.GetAnnotations(exemption.Annotations)
}
}
}
return false, nil
}
23 changes: 1 addition & 22 deletions maintainers_annotation/maintainers_annotation.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ package maintainers_annotation
import (
"errors"
"fmt"
"strings"

Check failure on line 20 in maintainers_annotation/maintainers_annotation.go

View workflow job for this annotation

GitHub Actions / check-linter

File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/ossf/scorecard) (gci)
"github.com/ossf/scorecard/v4/checker"
"github.com/ossf/scorecard/v4/clients"
sce "github.com/ossf/scorecard/v4/errors"
"gopkg.in/yaml.v3"

Check failure on line 23 in maintainers_annotation/maintainers_annotation.go

View workflow job for this annotation

GitHub Actions / check-linter

File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/ossf/scorecard) (gci)
Expand Down Expand Up @@ -129,27 +127,8 @@ func GetMaintainersAnnotation(repoClient clients.RepoClient) (MaintainersAnnotat
return ma, nil
}

// IsCheckExempted verifies if a given check in the results is exempted in maintainers annotation.
func (ma *MaintainersAnnotation) IsCheckExempted(check checker.CheckResult) (bool, []string) {
// If check has a maximum score, then there it doesn't make sense anymore to reason the check
// This may happen if the check score was once low but then the problem was fixed on Scorecard side
// or on the maintainers side
if check.Score == checker.MaxResultScore {
return false, nil
}

for _, exemption := range ma.Exemptions {
for _, checkName := range exemption.Checks {
if strings.EqualFold(checkName, strings.ToLower(check.Name)) {
return true, getAnnotations(exemption.Annotations)
}
}
}
return false, nil
}

// getAnnotations parses a group of annotations into annotation reasons.
func getAnnotations(a []Annotation) []string {
func GetAnnotations(a []Annotation) []string {
var annotationReasons []string
for _, annotation := range a {
annotationReasons = append(annotationReasons, AnnotationReasonDoc[annotation.AnnotationReason])
Expand Down
2 changes: 1 addition & 1 deletion pkg/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (r *ScorecardResult) AsJSON2(showDetails bool,
}
}
if showMaintainersAnnotation {
exempted, reasons := r.MaintainersAnnotation.IsCheckExempted(checkResult)
exempted, reasons := checkResult.IsExempted(r.MaintainersAnnotation)
if exempted {
tmpResult.MaintainersAnnotation = reasons
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sarif.go
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ func (r *ScorecardResult) AsSARIF(showDetails bool, logLevel log.Level,
check := check

// If check is exempted, skip
exempted, _ := r.MaintainersAnnotation.IsCheckExempted(check)
exempted, _ := check.IsExempted(r.MaintainersAnnotation)
if exempted {
continue
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/scorecard_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func (r *ScorecardResult) AsString(showDetails bool,
}
x = append(x, doc)
if showMaintainersAnnotation {
_, reasons := r.MaintainersAnnotation.IsCheckExempted(row)
_, reasons := row.IsExempted(r.MaintainersAnnotation)
x = append(x, strings.Join(reasons, "\n"))
}
data[i] = x
Expand Down

0 comments on commit c5253b7

Please sign in to comment.