From 730076fab1232ce5df0563f25dc952f80177f377 Mon Sep 17 00:00:00 2001 From: asraa Date: Fri, 19 Nov 2021 18:16:02 -0600 Subject: [PATCH] :bug: fix dangerous workflow test and workflow parsing (#1283) * fix dangerous workflow Signed-off-by: Asra Ali * check if removing label comment fixes Signed-off-by: Asra Ali Co-authored-by: Azeem Shaikh --- .github/workflows/integration.yml | 2 +- checks/dangerous_workflow.go | 18 +++++++++--------- checks/dangerous_workflow_test.go | 12 ++++-------- ...flow-dangerous-pattern-trusted-checkout.yml | 2 +- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 541cab56..198b32cc 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -67,4 +67,4 @@ jobs: body: | Integration tests ${{ job.status }} for [${{ github.event.client_payload.slash_command.args.named.sha || github.event.pull_request.head.sha }}] - (https://github.com/ossf/scorecard/actions/runs/${{ github.run_id }}) + (https://github.com/ossf/scorecard/actions/runs/${{ github.run_id }}) \ No newline at end of file diff --git a/checks/dangerous_workflow.go b/checks/dangerous_workflow.go index 68e93d23..42e461c6 100644 --- a/checks/dangerous_workflow.go +++ b/checks/dangerous_workflow.go @@ -140,15 +140,9 @@ func checkPullRequestTrigger(config map[interface{}]interface{}) (bool, error) { if strings.EqualFold(val, "pull_request_target") { isPullRequestTrigger = true } - case []string: + case []interface{}: for _, onVal := range val { - if strings.EqualFold(onVal, "pull_request_target") { - isPullRequestTrigger = true - } - } - case map[interface{}]interface{}: - for k := range val { - key, ok := k.(string) + key, ok := onVal.(string) if !ok { return false, sce.WithMessage(sce.ErrScorecardInternal, errInvalidGitHubWorkflow.Error()) } @@ -156,6 +150,12 @@ func checkPullRequestTrigger(config map[interface{}]interface{}) (bool, error) { isPullRequestTrigger = true } } + case map[string]interface{}: + for key := range val { + if strings.EqualFold(key, "pull_request_target") { + isPullRequestTrigger = true + } + } default: return false, sce.WithMessage(sce.ErrScorecardInternal, errInvalidGitHubWorkflow.Error()) } @@ -258,7 +258,7 @@ func createResultForDangerousWorkflowPatterns(result patternCbData, err error) c "no dangerous workflow patterns detected") } -func testValidateGitHubActionDangerousWOrkflow(pathfn string, +func testValidateGitHubActionDangerousWorkflow(pathfn string, content []byte, dl checker.DetailLogger) checker.CheckResult { data := patternCbData{ workflowPattern: make(map[string]bool), diff --git a/checks/dangerous_workflow_test.go b/checks/dangerous_workflow_test.go index 1caebcc5..be3ad2ca 100644 --- a/checks/dangerous_workflow_test.go +++ b/checks/dangerous_workflow_test.go @@ -57,13 +57,9 @@ func TestGithubDangerousWorkflow(t *testing.T) { name: "run trusted code checkout test", filename: "./testdata/github-workflow-dangerous-pattern-trusted-checkout.yml", expected: scut.TestReturn{ - Error: nil, - // TODO(#1294): Fix the score calculation to return MaxScore. - // Score: checker.MaxResultScore, - Score: checker.MinResultScore, - // TODO(#1294): NumberOfWarn should be 0. - // NumberOfWarn: 0, - NumberOfWarn: 1, + Error: nil, + Score: checker.MaxResultScore, + NumberOfWarn: 0, NumberOfInfo: 0, NumberOfDebug: 0, }, @@ -106,7 +102,7 @@ func TestGithubDangerousWorkflow(t *testing.T) { } } dl := scut.TestDetailLogger{} - r := testValidateGitHubActionDangerousWOrkflow(tt.filename, content, &dl) + r := testValidateGitHubActionDangerousWorkflow(tt.filename, content, &dl) if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl) { t.Fail() } diff --git a/checks/testdata/github-workflow-dangerous-pattern-trusted-checkout.yml b/checks/testdata/github-workflow-dangerous-pattern-trusted-checkout.yml index 474f218f..b2f70ace 100644 --- a/checks/testdata/github-workflow-dangerous-pattern-trusted-checkout.yml +++ b/checks/testdata/github-workflow-dangerous-pattern-trusted-checkout.yml @@ -21,7 +21,7 @@ jobs: steps: - uses: actions/checkout@v2 with: - ref: ${{ github.event.pull_request.head.sha }} + ref: main - uses: actions/setup-node@v1 - run: |