Skip to content

Commit

Permalink
✨ Update raw format for Dangerous workflows (#1865)
Browse files Browse the repository at this point in the history
* updates

* e2e fix

* comments
  • Loading branch information
laurentsimon authored May 14, 2022
1 parent cd04704 commit b1ab7eb
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 71 deletions.
25 changes: 14 additions & 11 deletions checker/raw_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
28 changes: 13 additions & 15 deletions checks/evaluation/dangerous_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 7 additions & 5 deletions checks/raw/dangerous_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
},
)
}
Expand Down
2 changes: 1 addition & 1 deletion checks/raw/dangerous_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
65 changes: 29 additions & 36 deletions pkg/json_raw_results.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package pkg

import (
"encoding/json"
"errors"
"fmt"
"io"
"time"
Expand All @@ -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"`
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand All @@ -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),
},
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/scorecard_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b1ab7eb

Please sign in to comment.