Update raw format for Dangerous workflows (#1865)

* updates

* e2e fix

* comments
This commit is contained in:
laurentsimon 2022-05-13 19:10:57 -07:00 committed by GitHub
parent cd0470403b
commit b1ab7eb9bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 66 additions and 72 deletions

View File

@ -283,24 +283,27 @@ type CIIBestPracticesData struct {
Badge CIIBadge
}
// DangerousWorkflowType represents a type of dangerous workflow.
type DangerousWorkflowType int
const (
// DangerousWorkflowScriptInjection represents a script injection.
DangerousWorkflowScriptInjection DangerousWorkflowType = iota
// DangerousWorkflowUntrustedCheckout represents an untrusted checkout.
DangerousWorkflowUntrustedCheckout
)
// DangerousWorkflowData contains raw results
// for dangerous workflow check.
type DangerousWorkflowData struct {
ScriptInjections []ScriptInjection
UntrustedCheckouts []UntrustedCheckout
// TODO: other
Workflows []DangerousWorkflow
}
// UntrustedCheckout represents an untrusted checkout.
type UntrustedCheckout struct {
Job *WorkflowJob
File File
}
// ScriptInjection represents a script injection.
type ScriptInjection struct {
// DangerousWorkflow represents a dangerous workflow.
type DangerousWorkflow struct {
Job *WorkflowJob
File File
Type DangerousWorkflowType
}
// WorkflowJob reprresents a workflow job.

View File

@ -30,30 +30,28 @@ func DangerousWorkflow(name string, dl checker.DetailLogger,
return checker.CreateRuntimeErrorResult(name, e)
}
// Script injections.
for _, e := range r.ScriptInjections {
for _, e := range r.Workflows {
var text string
switch e.Type {
case checker.DangerousWorkflowUntrustedCheckout:
text = fmt.Sprintf("untrusted code checkout '%v'", e.File.Snippet)
case checker.DangerousWorkflowScriptInjection:
text = fmt.Sprintf("script injection with untrusted input '%v'", e.File.Snippet)
default:
err := sce.WithMessage(sce.ErrScorecardInternal, "invalid type")
return checker.CreateRuntimeErrorResult(name, err)
}
dl.Warn(&checker.LogMessage{
Path: e.File.Path,
Type: e.File.Type,
Offset: e.File.Offset,
Text: fmt.Sprintf("script injection with untrusted input '%v'", e.File.Snippet),
Text: text,
Snippet: e.File.Snippet,
})
}
// Untrusted checkouts.
for _, e := range r.UntrustedCheckouts {
dl.Warn(&checker.LogMessage{
Path: e.File.Path,
Type: e.File.Type,
Offset: e.File.Offset,
Text: fmt.Sprintf("untrusted code checkout '%v'", e.File.Snippet),
Snippet: e.File.Snippet,
})
}
if len(r.ScriptInjections) > 0 ||
len(r.UntrustedCheckouts) > 0 {
if len(r.Workflows) > 0 {
return createResult(name, checker.MinResultScore)
}
return createResult(name, checker.MaxResultScore)

View File

@ -191,8 +191,9 @@ func checkJobForUntrustedCodeCheckout(job *actionlint.Job, path string,
if strings.Contains(ref.Value.Value, checkoutUntrustedPullRequestRef) ||
strings.Contains(ref.Value.Value, checkoutUntrustedWorkflowRunRef) {
line := fileparser.GetLineNumber(step.Pos)
pdata.UntrustedCheckouts = append(pdata.UntrustedCheckouts,
checker.UntrustedCheckout{
pdata.Workflows = append(pdata.Workflows,
checker.DangerousWorkflow{
Type: checker.DangerousWorkflowUntrustedCheckout,
File: checker.File{
Path: path,
Type: checker.FileTypeSource,
@ -250,15 +251,16 @@ func checkVariablesInScript(script string, pos *actionlint.Pos,
variable := script[s+3 : s+e]
if containsUntrustedContextPattern(variable) {
line := fileparser.GetLineNumber(pos)
pdata.ScriptInjections = append(pdata.ScriptInjections,
checker.ScriptInjection{
pdata.Workflows = append(pdata.Workflows,
checker.DangerousWorkflow{
File: checker.File{
Path: path,
Type: checker.FileTypeSource,
Offset: line,
Snippet: variable,
},
Job: createJob(job),
Job: createJob(job),
Type: checker.DangerousWorkflowScriptInjection,
},
)
}

View File

@ -175,7 +175,7 @@ func TestGithubDangerousWorkflow(t *testing.T) {
return
}
nb := len(dw.ScriptInjections) + len(dw.UntrustedCheckouts)
nb := len(dw.Workflows)
if nb != tt.expected.nb {
t.Errorf(cmp.Diff(nb, tt.expected.nb))
}

View File

@ -92,7 +92,6 @@ func (r *ScorecardResult) AsJSON(showDetails bool, logLevel log.Level, writer io
Metadata: r.Metadata,
}
//nolint
for _, checkResult := range r.Checks {
tmpResult := jsonCheckResult{
Name: checkResult.Name,
@ -139,7 +138,6 @@ func (r *ScorecardResult) AsJSON2(showDetails bool,
AggregateScore: jsonFloatScore(score),
}
//nolint
for _, checkResult := range r.Checks {
doc, e := checkDocs.GetCheck(checkResult.Name)
if e != nil {

View File

@ -16,6 +16,7 @@ package pkg
import (
"encoding/json"
"errors"
"fmt"
"io"
"time"
@ -24,6 +25,11 @@ import (
sce "github.com/ossf/scorecard/v4/errors"
)
// TODO: add a "check" field to all results so that they can be linked to a check.
// TODO(#1874): Add a severity field in all results.
var errorInvalidType = errors.New("invalid type")
// Flat JSON structure to hold raw results.
type jsonScorecardRawResult struct {
Date string `json:"date"`
@ -142,19 +148,18 @@ type jsonLicense struct {
// TODO: add fields, like type of license, etc.
}
type jsonWorkflows struct {
UntrustedCheckouts []jsonUntrustedCheckout `json:"untrustedCheckouts"`
ScriptInjections []jsonScriptInjection `json:"scriptInjections"`
}
type dangerousPatternType string
type jsonUntrustedCheckout struct {
Job *jsonWorkflowJob `json:"job"`
File jsonFile `json:"file"`
}
const (
patternUntrustedCheckout dangerousPatternType = "untrustedCheckout"
patternScriptInjection dangerousPatternType = "scriptInjection"
)
type jsonScriptInjection struct {
type jsonWorkflow struct {
Job *jsonWorkflowJob `json:"job"`
File jsonFile `json:"file"`
File *jsonFile `json:"file"`
// Type is a string to allow different types for permissions, unpinned dependencies, etc.
Type string `json:"type"`
}
type jsonWorkflowJob struct {
@ -165,7 +170,7 @@ type jsonWorkflowJob struct {
//nolint
type jsonRawResults struct {
// Workflow results.
Workflows jsonWorkflows `json:"workflows"`
Workflows []jsonWorkflow `json:"workflows"`
// License.
Licenses []jsonLicense `json:"licenses"`
// List of recent issues.
@ -192,13 +197,11 @@ type jsonRawResults struct {
Releases []jsonRelease `json:"releases"`
}
//nolint:unparam
func (r *jsonScorecardRawResult) addDangerousWorkflowRawResults(df *checker.DangerousWorkflowData) error {
r.Results.Workflows = jsonWorkflows{}
// Untrusted checkouts.
for _, e := range df.UntrustedCheckouts {
v := jsonUntrustedCheckout{
File: jsonFile{
r.Results.Workflows = []jsonWorkflow{}
for _, e := range df.Workflows {
v := jsonWorkflow{
File: &jsonFile{
Path: e.File.Path,
Offset: int(e.File.Offset),
},
@ -212,27 +215,17 @@ func (r *jsonScorecardRawResult) addDangerousWorkflowRawResults(df *checker.Dang
ID: e.Job.ID,
}
}
r.Results.Workflows.UntrustedCheckouts = append(r.Results.Workflows.UntrustedCheckouts, v)
}
// Script injections
for _, e := range df.ScriptInjections {
v := jsonScriptInjection{
File: jsonFile{
Path: e.File.Path,
Offset: int(e.File.Offset),
},
switch e.Type {
case checker.DangerousWorkflowUntrustedCheckout:
v.Type = string(patternUntrustedCheckout)
case checker.DangerousWorkflowScriptInjection:
v.Type = string(patternScriptInjection)
default:
return fmt.Errorf("%w: %d", errorInvalidType, e.Type)
}
if e.File.Snippet != "" {
v.File.Snippet = &e.File.Snippet
}
if e.Job != nil {
v.Job = &jsonWorkflowJob{
Name: e.Job.Name,
ID: e.Job.ID,
}
}
r.Results.Workflows.ScriptInjections = append(r.Results.Workflows.ScriptInjections, v)
r.Results.Workflows = append(r.Results.Workflows, v)
}
return nil

View File

@ -141,7 +141,7 @@ func (r *ScorecardResult) AsString(showDetails bool, logLevel log.Level,
checkDocs checks.Doc, writer io.Writer,
) error {
data := make([][]string, len(r.Checks))
//nolint
for i, row := range r.Checks {
const withdetails = 5
const withoutdetails = 4