From f2241bac2fe7d7f00f636168887c724b0e01025d Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 25 Apr 2022 23:59:42 +0000 Subject: [PATCH] updates --- checker/raw_result.go | 25 ++++++----- checks/evaluation/dangerous_workflow.go | 28 ++++++------ checks/raw/dangerous_workflow.go | 14 +++--- pkg/json_raw_results.go | 57 +++++++++++-------------- 4 files changed, 60 insertions(+), 64 deletions(-) diff --git a/checker/raw_result.go b/checker/raw_result.go index 9c2093d98129..af285ecf1d0b 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 []Workflow } -// ScriptInjection represents a script injection. -type ScriptInjection struct { +// Workflow represents a result for a workflow. +type Workflow 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 6fe443926ff3..faf9ce8f28cb 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 910d455f30e0..ff6a3f0ab354 100644 --- a/checks/raw/dangerous_workflow.go +++ b/checks/raw/dangerous_workflow.go @@ -191,15 +191,16 @@ 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.Workflow{ File: checker.File{ Path: path, Type: checker.FileTypeSource, Offset: line, Snippet: ref.Value.Value, }, - Job: createJob(job), + Job: createJob(job), + Type: checker.DangerousWorkflowUntrustedCheckout, }, ) } @@ -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.Workflow{ File: checker.File{ Path: path, Type: checker.FileTypeSource, Offset: line, Snippet: variable, }, - Job: createJob(job), + Job: createJob(job), + Type: checker.DangerousWorkflowScriptInjection, }, ) } diff --git a/pkg/json_raw_results.go b/pkg/json_raw_results.go index 6cfb0de38e2f..85c9f380bbf7 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,8 @@ import ( sce "github.com/ossf/scorecard/v4/errors" ) +var errorInvalidType = errors.New("invalid type") + // Flat JSON structure to hold raw results. type jsonScorecardRawResult struct { Date string `json:"date"` @@ -143,18 +146,20 @@ type jsonLicense struct { } type jsonWorkflows struct { - UntrustedCheckouts []jsonUntrustedCheckout `json:"untrustedCheckouts"` - ScriptInjections []jsonScriptInjection `json:"scriptInjections"` + DangerousPatterns []jsonDangerousPattern `json:"dangerousPatterns"` } -type jsonUntrustedCheckout struct { - Job *jsonWorkflowJob `json:"job"` - File jsonFile `json:"file"` -} +type dangerousPatternType string -type jsonScriptInjection struct { - Job *jsonWorkflowJob `json:"job"` - File jsonFile `json:"file"` +const ( + patternUntrustedCheckout dangerousPatternType = "untrustedCheckout" + patternScriptInjection dangerousPatternType = "scriptInjection" +) + +type jsonDangerousPattern struct { + Job *jsonWorkflowJob `json:"job"` + File jsonFile `json:"file"` + Type dangerousPatternType `json:"type"` } type jsonWorkflowJob struct { @@ -192,12 +197,10 @@ 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{ + for _, e := range df.Workflows { + v := jsonDangerousPattern{ 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 = patternUntrustedCheckout + case checker.DangerousWorkflowScriptInjection: + v.Type = 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.DangerousPatterns = append(r.Results.Workflows.DangerousPatterns, v) } return nil