Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Update raw format for Dangerous workflows #1865

Merged
merged 5 commits into from May 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 14 additions & 11 deletions checker/raw_result.go
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
azeemshaikh38 marked this conversation as resolved.
Show resolved Hide resolved
}

// WorkflowJob reprresents a workflow job.
Expand Down
28 changes: 13 additions & 15 deletions checks/evaluation/dangerous_workflow.go
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
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
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
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
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
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