Skip to content

Commit

Permalink
preserve logging from original evaluation
Browse files Browse the repository at this point in the history 
Signed-off-by: AdamKorcz <[email protected]>
  • Loading branch information
@AdamKorcz
AdamKorcz committed Oct 20, 2023
1 parent 713d267 commit e0b7800
Show file tree
Hide file tree
Showing 4 changed files with 204 additions and 20 deletions.
18 changes: 18 additions & 0 deletions checks/evaluation/dangerous_workflow.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,24 @@ func DangerousWorkflow(name string,
return checker.CreateInconclusiveResult(name, "no workflows found")
}

// Log all detected dangerous workflows
for i := range findings {
f := &findings[i]
if f.Outcome == finding.OutcomeNegative {
if f.Location == nil {
e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results")
return checker.CreateRuntimeErrorResult(name, e)
}

Check warning on line 50 in checks/evaluation/dangerous_workflow.go

View check run for this annotation

Codecov / codecov/patch

checks/evaluation/dangerous_workflow.go#L48-L50 

Added lines #L48 - L50 were not covered by tests
dl.Warn(&checker.LogMessage{
Path: f.Location.Path,
Type: f.Location.Type,
Offset: *f.Location.LineStart,
Text: f.Message,
Snippet: *f.Location.Snippet,
})
}
}

if hasDWWithUntrustedCheckout(findings) || hasDWWithScriptInjection(findings) {
return checker.CreateMinScoreResult(name,
"dangerous workflow patterns detected")
Expand Down
148 changes: 145 additions & 3 deletions checks/evaluation/dangerous_workflow_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ import (
scut "github.com/ossf/scorecard/v4/utests"
)

var (
testSnippet = "other/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675"
testLineStart = uint(123)
)

func TestDangerousWorkflow(t *testing.T) {
t.Parallel()
tests := []struct {
Expand All @@ -37,10 +42,17 @@ func TestDangerousWorkflow(t *testing.T) {
}, {
Probe: "hasDangerousWorkflowUntrustedCheckout",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
},
},
result: scut.TestReturn{
Score: 0,
Score: 0,
NumberOfWarn: 1,
},
},
{
Expand Down Expand Up @@ -82,10 +94,17 @@ func TestDangerousWorkflow(t *testing.T) {
}, {
Probe: "hasDangerousWorkflowUntrustedCheckout",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
},
},
result: scut.TestReturn{
Score: 0,
Score: 0,
NumberOfWarn: 1,
},
},
{
Expand All @@ -94,13 +113,136 @@ func TestDangerousWorkflow(t *testing.T) {
{
Probe: "hasDangerousWorkflowScriptInjection",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
}, {
Probe: "hasDangerousWorkflowUntrustedCheckout",
Outcome: finding.OutcomePositive,
},
},
result: scut.TestReturn{
Score: 0,
NumberOfWarn: 1,
},
},
{
name: "DangerousWorkflow - 3 script injection workflows detected",
findings: []finding.Finding{
{
Probe: "hasDangerousWorkflowScriptInjection",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
}, {
Probe: "hasDangerousWorkflowScriptInjection",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow2.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
}, {
Probe: "hasDangerousWorkflowUntrustedCheckout",
Outcome: finding.OutcomePositive,
},
},
result: scut.TestReturn{
Score: 0,
NumberOfWarn: 2,
},
},
{
name: "DangerousWorkflow - 8 script injection workflows detected",
findings: []finding.Finding{
{
Probe: "hasDangerousWorkflowScriptInjection",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
}, {
Probe: "hasDangerousWorkflowScriptInjection",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow2.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
}, {
Probe: "hasDangerousWorkflowScriptInjection",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow3.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
}, {
Probe: "hasDangerousWorkflowScriptInjection",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow4.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
}, {
Probe: "hasDangerousWorkflowScriptInjection",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow5.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
}, {
Probe: "hasDangerousWorkflowScriptInjection",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow6.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
}, {
Probe: "hasDangerousWorkflowScriptInjection",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow7.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
}, {
Probe: "hasDangerousWorkflowScriptInjection",
Outcome: finding.OutcomeNegative,
Location: &finding.Location{
Type: finding.FileTypeText,
Path: "./github/workflows/dangerous-workflow8.yml",
LineStart: &testLineStart,
Snippet: &testSnippet,
},
}, {
Probe: "hasDangerousWorkflowUntrustedCheckout",
Outcome: finding.OutcomePositive,
},
},
result: scut.TestReturn{
Score: 0,
Score: 0,
NumberOfWarn: 8,
},
},
}
Expand Down
32 changes: 20 additions & 12 deletions probes/hasDangerousWorkflowScriptInjection/impl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,31 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
return []finding.Finding{*f}, Probe, nil

Check warning on line 46 in probes/hasDangerousWorkflowScriptInjection/impl.go

View check run for this annotation

Codecov / codecov/patch

probes/hasDangerousWorkflowScriptInjection/impl.go#L40-L46 

Added lines #L40 - L46 were not covered by tests
}

var findings []finding.Finding
for _, e := range r.Workflows {
e := e
if e.Type == checker.DangerousWorkflowScriptInjection {
return negativeOutcome()
f, err := finding.NewWith(fs, Probe,
fmt.Sprintf("script injection with untrusted input '%v'", e.File.Snippet),
nil, finding.OutcomeNegative)
if err != nil {
return nil, Probe, fmt.Errorf("create finding: %w", err)
}

Check warning on line 58 in probes/hasDangerousWorkflowScriptInjection/impl.go

View check run for this annotation

Codecov / codecov/patch

probes/hasDangerousWorkflowScriptInjection/impl.go#L57-L58 

Added lines #L57 - L58 were not covered by tests
f = f.WithLocation(&finding.Location{
Path: e.File.Path,
Type: e.File.Type,
LineStart: &e.File.Offset,
Snippet: &e.File.Snippet,
})
findings = append(findings, *f)
}
}

return positiveOutcome()
if len(findings) == 0 {
return positiveOutcome()
}

return findings, Probe, nil
}

func positiveOutcome() ([]finding.Finding, string, error) {
Expand All @@ -64,13 +82,3 @@ func positiveOutcome() ([]finding.Finding, string, error) {
}

Check warning on line 82 in probes/hasDangerousWorkflowScriptInjection/impl.go

View check run for this annotation

Codecov / codecov/patch

probes/hasDangerousWorkflowScriptInjection/impl.go#L81-L82 

Added lines #L81 - L82 were not covered by tests
return []finding.Finding{*f}, Probe, nil
}

func negativeOutcome() ([]finding.Finding, string, error) {
f, err := finding.NewWith(fs, Probe,
"Project has dangerous workflow(s) with possibility of script injection.", nil,
finding.OutcomeNegative)
if err != nil {
return nil, Probe, fmt.Errorf("create finding: %w", err)
}
return []finding.Finding{*f}, Probe, nil
}
26 changes: 21 additions & 5 deletions probes/hasDangerousWorkflowUntrustedCheckout/impl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,29 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
return []finding.Finding{*f}, Probe, nil

Check warning on line 46 in probes/hasDangerousWorkflowUntrustedCheckout/impl.go

View check run for this annotation

Codecov / codecov/patch

probes/hasDangerousWorkflowUntrustedCheckout/impl.go#L40-L46 

Added lines #L40 - L46 were not covered by tests
}

var findings []finding.Finding
for _, e := range r.Workflows {
e := e
if e.Type == checker.DangerousWorkflowUntrustedCheckout {
return negativeOutcome()
f, err := finding.NewWith(fs, Probe,
fmt.Sprintf("untrusted code checkout '%v'", e.File.Snippet),
nil, finding.OutcomeNegative)
if err != nil {
return nil, Probe, fmt.Errorf("create finding: %w", err)
}

Check warning on line 58 in probes/hasDangerousWorkflowUntrustedCheckout/impl.go

View check run for this annotation

Codecov / codecov/patch

probes/hasDangerousWorkflowUntrustedCheckout/impl.go#L57-L58 

Added lines #L57 - L58 were not covered by tests
f = f.WithLocation(&finding.Location{
Path: e.File.Path,
Type: e.File.Type,
LineStart: &e.File.Offset,
Snippet: &e.File.Snippet,
})
findings = append(findings, *f)
}
}

return positiveOutcome()
if len(findings) == 0 {
return positiveOutcome()
}
return findings, Probe, nil
}

func positiveOutcome() ([]finding.Finding, string, error) {
Expand All @@ -65,12 +81,12 @@ func positiveOutcome() ([]finding.Finding, string, error) {
return []finding.Finding{*f}, Probe, nil
}

func negativeOutcome() ([]finding.Finding, string, error) {
/*func negativeOutcome() ([]finding.Finding, string, error) {
f, err := finding.NewWith(fs, Probe,
"Project has workflow(s) with untrusted checkout.", nil,
finding.OutcomeNegative)
if err != nil {
return nil, Probe, fmt.Errorf("create finding: %w", err)
}
return []finding.Finding{*f}, Probe, nil
}
}*/

0 comments on commit e0b7800

Please sign in to comment.