diff --git a/checker/raw_result.go b/checker/raw_result.go index 9c2093d9812..e615e51f1ce 100644 --- a/checker/raw_result.go +++ b/checker/raw_result.go @@ -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 -} - -// UntrustedCheckout represents an untrusted checkout. -type UntrustedCheckout struct { - Job *WorkflowJob - File File + Workflows []DangerousWorkflow } -// 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. diff --git a/checks/evaluation/dangerous_workflow.go b/checks/evaluation/dangerous_workflow.go index 6fe443926ff..faf9ce8f28c 100644 --- a/checks/evaluation/dangerous_workflow.go +++ b/checks/evaluation/dangerous_workflow.go @@ -30,30 +30,28 @@ func DangerousWorkflow(name string, dl checker.DetailLogger, return checker.CreateRuntimeErrorResult(name, e) } - // Script injections. - for _, e := range r.ScriptInjections { - 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), - Snippet: e.File.Snippet, - }) - } + 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) + } - // 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), + Text: text, 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) diff --git a/checks/raw/dangerous_workflow.go b/checks/raw/dangerous_workflow.go index 910d455f30e..d6d47189704 100644 --- a/checks/raw/dangerous_workflow.go +++ b/checks/raw/dangerous_workflow.go @@ -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, }, ) } diff --git a/checks/raw/dangerous_workflow_test.go b/checks/raw/dangerous_workflow_test.go index c271831bb4b..7714caa6069 100644 --- a/checks/raw/dangerous_workflow_test.go +++ b/checks/raw/dangerous_workflow_test.go @@ -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)) } diff --git a/pkg/json.go b/pkg/json.go index 201b43c6b78..4806a0d704a 100644 --- a/pkg/json.go +++ b/pkg/json.go @@ -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 { diff --git a/pkg/json_raw_results.go b/pkg/json_raw_results.go index 6cfb0de38e2..e435116a9e5 100644 --- a/pkg/json_raw_results.go +++ b/pkg/json_raw_results.go @@ -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 diff --git a/pkg/scorecard_result.go b/pkg/scorecard_result.go index 3f163615908..c1a0223b9a0 100644 --- a/pkg/scorecard_result.go +++ b/pkg/scorecard_result.go @@ -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