Skip to content

Commit

Permalink
✨ Pull request support for GitHub action (#1222)
Browse files Browse the repository at this point in the history 
* draft

* updates

* PR support

* comments

* comment

* category

* fix rule ID

* typo

* always populate rules

* typo
  • Loading branch information
@laurentsimon
laurentsimon authored Nov 8, 2021
1 parent 3dc507b commit 09b7b3b
Show file tree
Hide file tree
Showing 11 changed files with 700 additions and 59 deletions.
7 changes: 4 additions & 3 deletions cmd/root.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,10 @@ func getEnabledChecks(sp *spol.ScorecardPolicy, argsChecks []string,
// Populate checks to run with policy file.
for checkName := range sp.GetPolicies() {
if !isSupportedCheck(supportedChecks, checkName) {
return enabledChecks,
sce.WithMessage(sce.ErrScorecardInternal,
fmt.Sprintf("repo type %s: unsupported check: %s", repoType, checkName))
// We silently ignore the check, like we do
// for the default case when no argsChecks
// or policy are present.
continue
}

if !enableCheck(checkName, &enabledChecks) {
Expand Down
165 changes: 117 additions & 48 deletions pkg/sarif.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ type help struct {
}

type rule struct {
ID string `json:"id"`
// ID should be an opaque, stable ID.
// TODO: check if GitHub follows this.
// Last time I tried, it used ID to display to users.
ID string `json:"id"`
// Name must be readable by human.
Name string `json:"name"`
HelpURI string `json:"helpUri"`
ShortDesc text `json:"shortDescription"`
Expand All @@ -107,7 +111,7 @@ type driver struct {
Name string `json:"name"`
InformationURI string `json:"informationUri"`
SemVersion string `json:"semanticVersion"`
Rules []rule `json:"rules"`
Rules []rule `json:"rules,omitempty"`
}

type tool struct {
Expand Down Expand Up @@ -137,8 +141,9 @@ type run struct {
Tool tool `json:"tool"`
// For generated files during analysis. We leave this blank.
// See https://github.com/microsoft/sarif-tutorials/blob/main/docs/1-Introduction.md#simple-example.
Artifacts string `json:"artifacts,omitempty"`
Results []result `json:"results"`
Artifacts string `json:"artifacts,omitempty"`
// This MUST never be omitted or set as `nil`.
Results []result `json:"results"`
}

type sarif210 struct {
Expand Down Expand Up @@ -331,31 +336,50 @@ func addDefaultLocation(locs []location, policyFile string) []location {
return locs
}

func createSARIFHeader(url, category, name, version, commit string, t time.Time) sarif210 {
func createSARIFHeader() sarif210 {
return sarif210{
Schema: "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
Version: "2.1.0",
Runs: []run{
{
Tool: tool{
Driver: driver{
Name: strings.Title(name),
InformationURI: url,
SemVersion: version,
Rules: nil,
},
},
//nolint
// See https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#runautomationdetails-object.
AutomationDetails: automationDetails{
// Time formatting: https://pkg.go.dev/time#pkg-constants.
ID: fmt.Sprintf("%s/%s/%s", category, name, fmt.Sprintf("%s-%s", commit, t.Format(time.RFC822Z))),
},
},
Runs: []run{},
}
}

func createSARIFTool(url, name, version string) tool {
return tool{
Driver: driver{
Name: strings.Title(name),
InformationURI: url,
SemVersion: version,
Rules: nil,
},
}
}

func createSARIFRun(uri, toolName, version, commit string, t time.Time,
category, runName string) run {
return run{
Tool: createSARIFTool(uri, toolName, version),
Results: []result{},
//nolint
// See https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#runautomationdetails-object.
AutomationDetails: automationDetails{
// Time formatting: https://pkg.go.dev/time#pkg-constants.
ID: fmt.Sprintf("%s/%s/%s", category, runName, fmt.Sprintf("%s-%s", commit, t.Format(time.RFC822Z))),
},
}
}

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 @@ -414,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 @@ -423,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) {
// In terms of sets, 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 @@ -432,18 +489,38 @@ func (r *ScorecardResult) AsSARIF(showDetails bool, logLevel zapcore.Level,
// We only support GitHub-supported properties:
// 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("https://github.com/ossf/scorecard",
"supply-chain", "scorecard", r.Scorecard.Version, r.Scorecard.CommitSHA, r.Date)
results := []result{}
rules := []rule{}
sarif := createSARIFHeader()
runs := make(map[string]*run)

// nolint
for i, check := range r.Checks {
doc, e := checkDocs.GetCheck(check.Name)
if e != nil {
return sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("GetCheck: %v: %s", e, check.Name))
for _, check := range r.Checks {
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.
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")

// Always add rules to indicate which checks were run.
// We don't have so many rules, so this should not clobber the output too much.
// See https://github.com/github/codeql-action/issues/810.
checkID := check.Name
rule := createSARIFRule(check.Name, checkID,
doc.GetDocumentationURL(r.Scorecard.CommitSHA),
doc.GetDescription(), doc.GetShort(), doc.GetRisk(),
doc.GetRemediation(), doc.GetTags())
run.Tool.Driver.Rules = append(run.Tool.Driver.Rules, rule)

// Check the policy configuration.
minScore, enabled, err := getCheckPolicyInfo(policy, check.Name)
if err != nil {
return err
Expand All @@ -466,38 +543,30 @@ func (r *ScorecardResult) AsSARIF(showDetails bool, logLevel zapcore.Level,
// location. Then in the next run, the result would occur on a different line, the computed fingerprint
// would change, and the result management system would erroneously report it as a new result."

// Set the check ID.
checkID := check.Name

// Create a header's rule.
rule := createSARIFRule(check.Name, checkID,
doc.GetDocumentationURL(r.Scorecard.CommitSHA),
doc.GetDescription(), doc.GetShort(), doc.GetRisk(),
doc.GetRemediation(), doc.GetTags())
rules = append(rules, rule)

// Create locations.
locs := detailsToLocations(check.Details2, showDetails, minScore, check.Score)

// Add default location if no locations are present.
// Note: GitHub needs at least one location to show the results.
// RuleIndex is the position of the corresponding rule in `run.Tool.Driver.Rules`,
// so it's the last position for us.
RuleIndex := len(run.Tool.Driver.Rules) - 1
if len(locs) == 0 {
locs = addDefaultLocation(locs, policyFile)
// Use the `reason` as message.
r := createSARIFCheckResult(i, checkID, check.Reason, &locs[0])
results = append(results, r)
cr := createSARIFCheckResult(RuleIndex, checkID, check.Reason, &locs[0])
run.Results = append(run.Results, cr)
} else {
for _, loc := range locs {
// Use the location's message (check's detail's message) as message.
r := createSARIFCheckResult(i, checkID, loc.Message.Text, &loc)
results = append(results, r)
cr := createSARIFCheckResult(RuleIndex, checkID, loc.Message.Text, &loc)
run.Results = append(run.Results, cr)
}
}
}

// Set the results and rules to sarif.
sarif.Runs[0].Tool.Driver.Rules = rules
sarif.Runs[0].Results = results
// Set the sarif's runs.
sarif.Runs = createSARIFRuns(runs)

encoder := json.NewEncoder(writer)
encoder.SetIndent("", " ")
Expand Down
Loading

0 comments on commit 09b7b3b

Please sign in to comment.