🐛 fix dangerous workflow test and workflow parsing (#1283)

* fix dangerous workflow Signed-off-by: Asra Ali <[email protected]> * check if removing label comment fixes Signed-off-by: Asra Ali <[email protected]> Co-authored-by: Azeem Shaikh <[email protected]>
ossf · Nov 20, 2021 · 730076f · 730076f
1 parent 10ee2c0
commit 730076f
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 19 deletions.
diff --git 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 }})
diff --git a/checks/dangerous_workflow.go b/checks/dangerous_workflow.go
@@ -140,22 +140,22 @@ 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())
 			}
 			if strings.EqualFold(key, "pull_request_target") {
 				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
@@ -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
@@ -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: |