Skip to content

Commit

Permalink
category
Browse files Browse the repository at this point in the history
  • Loading branch information
@laurentsimon
laurentsimon committed Nov 8, 2021
1 parent 7c3ed48 commit de8b0b0
Show file tree
Hide file tree
Showing 10 changed files with 510 additions and 223 deletions.
85 changes: 61 additions & 24 deletions pkg/sarif.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,17 @@ func createSARIFRun(uri, toolName, version, commit string, t time.Time,
}
}

func getOrCreateSARIFRun(runs map[string]*run, runName string,
uri, toolName, version, commit string, t time.Time,
category string) *run {
if prun, exists := runs[runName]; exists {
return prun
}
run := createSARIFRun(uri, toolName, version, commit, t, category, runName)
runs[runName] = &run
return &run
}

func generateRemediationMarkdown(remediation []string) string {
r := ""
for _, s := range remediation {
Expand Down Expand Up @@ -427,7 +438,7 @@ func getCheckPolicyInfo(policy *spol.ScorecardPolicy, name string) (minScore int
policies := policy.GetPolicies()
if _, exists := policies[name]; !exists {
return 0, false,
sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("Missing policy for check: %s", name))
sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("missing policy for check: %s", name))
}
cp := policies[name]
if cp.GetMode() == spol.CheckPolicy_DISABLED {
Expand All @@ -436,6 +447,39 @@ func getCheckPolicyInfo(policy *spol.ScorecardPolicy, name string) (minScore int
return int(cp.GetScore()), true, nil
}

func contains(l []string, elt string) bool {
for _, v := range l {
if v == elt {
return true
}
}
return false
}

func computeCategory(repos []string) (string, error) {
// local < Git-local < GitHub
switch {
default:
return "", sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("repo types not supported: %v", repos))
case contains(repos, "local"):
return "local", nil
// Note: Git-local is not supported by any checks yet.
case contains(repos, "Git-local"):
return "local-scm", nil
case contains(repos, "GitHub"),
contains(repos, "GitLab"):
return "online-scm", nil
}
}

func createSARIFRuns(runs map[string]*run) []run {
res := []run{}
for _, v := range runs {
res = append(res, *v)
}
return res
}

// AsSARIF outputs ScorecardResult in SARIF 2.1.0 format.
func (r *ScorecardResult) AsSARIF(showDetails bool, logLevel zapcore.Level,
writer io.Writer, checkDocs docs.Doc, policy *spol.ScorecardPolicy,
Expand All @@ -446,26 +490,25 @@ func (r *ScorecardResult) AsSARIF(showDetails bool, logLevel zapcore.Level,
// see https://docs.github.com/en/code-security/secure-coding/integrating-with-code-scanning/sarif-support-for-code-scanning#supported-sarif-output-file-properties,
// https://github.com/microsoft/sarif-tutorials.
sarif := createSARIFHeader()
runs := []run{}
runs := make(map[string]*run)

// nolint
for _, check := range r.Checks {
results := []result{}
rules := []rule{}

doc, e := checkDocs.GetCheck(check.Name)
if e != nil {
return sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("GetCheck: %v: %s", e, check.Name))
doc, err := checkDocs.GetCheck(check.Name)
if err != nil {
return sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("GetCheck: %v: %s", err, check.Name))
}

// We need to create a run entry even if the check is disabled or the policy is satisfied.
// The reason is the following: if a check has findings and is later fixed by a user,
// the absence of run for the check will indicate that the check was *not* run,
// so GitHub would keep the findings in the dahsboard. We don't want that.
run := createSARIFRun("https://github.com/ossf/scorecard", "scorecard",
r.Scorecard.Version, r.Scorecard.CommitSHA, r.Date,
"supply-chain", check.Name)
runs = append(runs, run)
category, err := computeCategory(doc.GetSupportedRepoTypes())
if err != nil {
return sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("computeCategory: %v: %s", err, check.Name))
}
run := getOrCreateSARIFRun(runs, category, "https://github.com/ossf/scorecard", "scorecard",
r.Scorecard.Version, r.Scorecard.CommitSHA, r.Date, "supply-chain")

// Check the policy configuration.
minScore, enabled, err := getCheckPolicyInfo(policy, check.Name)
Expand All @@ -481,18 +524,15 @@ func (r *ScorecardResult) AsSARIF(showDetails bool, logLevel zapcore.Level,
continue
}

// We only set rules if we have findings
// to avoid clobbering the results. It would be fine to add
// rules regardless of findings.
// We only set rules if we have findings to avoid clobbering the results.
// It would be fine to add rules regardless of findings.
// See https://github.com/github/codeql-action/issues/810#issuecomment-962006930.
// Note: we use a single rule, i.e., one check per run.
checkID := check.Name
rule := createSARIFRule(check.Name, checkID,
doc.GetDocumentationURL(r.Scorecard.CommitSHA),
doc.GetDescription(), doc.GetShort(), doc.GetRisk(),
doc.GetRemediation(), doc.GetTags())
rules = append(rules, rule)
runs[len(runs)-1].Tool.Driver.Rules = rules
run.Tool.Driver.Rules = append(run.Tool.Driver.Rules, rule)

// Unclear what to use for PartialFingerprints.
// GitHub only uses `primaryLocationLineHash`, which is not properly defined
Expand All @@ -513,22 +553,19 @@ func (r *ScorecardResult) AsSARIF(showDetails bool, logLevel zapcore.Level,
// Use the `reason` as message.
// Since we have a single rule per run, the indexId is 0.
cr := createSARIFCheckResult(0, checkID, check.Reason, &locs[0])
results = append(results, cr)
run.Results = append(run.Results, cr)
} else {
for _, loc := range locs {
// Use the location's message (check's detail's message) as message.
// Since we have a single rule per run, the indexId is 0.
cr := createSARIFCheckResult(0, checkID, loc.Message.Text, &loc)
results = append(results, cr)
run.Results = append(run.Results, cr)
}
}

// Set the results for the run.
runs[len(runs)-1].Results = results
}

// Set the sarif's runs.
sarif.Runs = runs
sarif.Runs = createSARIFRuns(runs)

encoder := json.NewEncoder(writer)
encoder.SetIndent("", " ")
Expand Down
121 changes: 121 additions & 0 deletions pkg/sarif_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func sarifMockDocRead() *mockDoc {
description: "long description\n other line",
url: "https://github.com/ossf/scorecard/blob/main/docs/checks.md#check-name",
tags: []string{"tag1", "tag2"},
repos: []string{"GitHub", "local"},
remediation: []string{"not-used1", "not-used2"},
},
"Check-Name2": {
Expand All @@ -45,6 +46,7 @@ func sarifMockDocRead() *mockDoc {
description: "long description\n other line 2",
url: "https://github.com/ossf/scorecard/blob/main/docs/checks.md#check-name2",
tags: []string{" tag1 ", " tag2 ", "tag3"},
repos: []string{"GitHub", "local"},
remediation: []string{"not-used1", "not-used2"},
},
"Check-Name3": {
Expand All @@ -54,6 +56,37 @@ func sarifMockDocRead() *mockDoc {
description: "long description\n other line 3",
url: "https://github.com/ossf/scorecard/blob/main/docs/checks.md#check-name3",
tags: []string{" tag1", " tag2", "tag3", "tag 4 "},
repos: []string{"GitHub", "local"},
remediation: []string{"not-used1", "not-used2"},
},
"Check-Name4": {
name: "Check-Name4",
risk: "Low",
short: "short description 4",
description: "long description\n other line 4",
url: "https://github.com/ossf/scorecard/blob/main/docs/checks.md#check-name4",
tags: []string{" tag1", " tag2", "tag3", "tag 4 "},
repos: []string{"GitHub"},
remediation: []string{"not-used1", "not-used2"},
},
"Check-Name5": {
name: "Check-Name5",
risk: "Low",
short: "short description 5",
description: "long description\n other line 5",
url: "https://github.com/ossf/scorecard/blob/main/docs/checks.md#check-name5",
tags: []string{" tag1", " tag2", "tag3", "tag 4 "},
repos: []string{"local"},
remediation: []string{"not-used1", "not-used2"},
},
"Check-Name6": {
name: "Check-Name6",
risk: "Low",
short: "short description 6",
description: "long description\n other line 6",
url: "https://github.com/ossf/scorecard/blob/main/docs/checks.md#check-name6",
tags: []string{" tag1", " tag2", "tag3", "tag 4 "},
repos: []string{"Git-local"},
remediation: []string{"not-used1", "not-used2"},
},
}
Expand Down Expand Up @@ -605,6 +638,94 @@ func TestSARIFOutput(t *testing.T) {
Metadata: []string{},
},
},
{
name: "check-8",
showDetails: true,
expected: "./testdata/check8.sarif",
logLevel: zapcore.DebugLevel,
policy: spol.ScorecardPolicy{
Version: 1,
Policies: map[string]*spol.CheckPolicy{
"Check-Name4": &spol.CheckPolicy{
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name5": &spol.CheckPolicy{
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name6": &spol.CheckPolicy{
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
},
},
result: ScorecardResult{
Repo: RepoInfo{
Name: repoName,
CommitSHA: repoCommit,
},
Scorecard: ScorecardInfo{
Version: scorecardVersion,
CommitSHA: scorecardCommit,
},
Date: date,
Checks: []checker.CheckResult{
{
Details2: []checker.CheckDetail{
{
Type: checker.DetailWarn,
Msg: checker.LogMessage{
Text: "warn message",
Path: "src/file1.cpp",
Type: checker.FileTypeSource,
Offset: 5,
Snippet: "if (bad) {BUG();}",
},
},
},
Score: 5,
Reason: "half score reason",
Name: "Check-Name4",
},
{
Details2: []checker.CheckDetail{
{
Type: checker.DetailWarn,
Msg: checker.LogMessage{
Text: "warn message",
Path: "src/file1.cpp",
Type: checker.FileTypeSource,
Offset: 5,
Snippet: "if (bad) {BUG();}",
},
},
},
Score: 8,
Reason: "half score reason",
Name: "Check-Name5",
},
{
Details2: []checker.CheckDetail{
{
Type: checker.DetailWarn,
Msg: checker.LogMessage{
Text: "warn message",
Path: "src/file1.cpp",
Type: checker.FileTypeSource,
Offset: 5,
Snippet: "if (bad) {BUG();}",
},
},
},
Score: 9,
Reason: "half score reason",
Name: "Check-Name6",
},
},
Metadata: []string{},
},
},
}
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
Expand Down
2 changes: 1 addition & 1 deletion pkg/testdata/check1.sarif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"runs": [
{
"automationDetails": {
"id": "supply-chain/Check-Name/ccbc59901773ab4c051dfcea0cc4201a1567abdd-17 Aug 21 18:57 +0000"
"id": "supply-chain/local/ccbc59901773ab4c051dfcea0cc4201a1567abdd-17 Aug 21 18:57 +0000"
},
"tool": {
"driver": {
Expand Down
2 changes: 1 addition & 1 deletion pkg/testdata/check2.sarif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"runs": [
{
"automationDetails": {
"id": "supply-chain/Check-Name/ccbc59901773ab4c051dfcea0cc4201a1567abdd-17 Aug 21 18:57 +0000"
"id": "supply-chain/local/ccbc59901773ab4c051dfcea0cc4201a1567abdd-17 Aug 21 18:57 +0000"
},
"tool": {
"driver": {
Expand Down
Loading

0 comments on commit de8b0b0

Please sign in to comment.