missing files

ossf · Mar 22, 2022 · 79c4923 · 79c4923
1 parent 7d359c6
commit 79c4923
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 15 deletions.
diff --git a/checks/dangerous_workflow.go b/checks/dangerous_workflow.go
@@ -109,7 +109,8 @@ func DangerousWorkflow(c *checker.CheckRequest) checker.CheckResult {
 // Check file content.
 var validateGitHubActionWorkflowPatterns fileparser.DoWhileTrueOnFileContent = func(path string,
 	content []byte,
-	args ...interface{}) (bool, error) {
+	args ...interface{},
+) (bool, error) {
 	if !fileparser.IsWorkflowFile(path) {
 		return true, nil
 	}
@@ -160,7 +161,8 @@ var validateGitHubActionWorkflowPatterns fileparser.DoWhileTrueOnFileContent = f
 }
 
 func validateSecretsInPullRequests(workflow *actionlint.Workflow, path string,
-	dl checker.DetailLogger, pdata *patternCbData) error {
+	dl checker.DetailLogger, pdata *patternCbData,
+) error {
 	triggers := make(map[triggerName]bool)
 
 	// We need pull request trigger.
@@ -194,7 +196,8 @@ func validateSecretsInPullRequests(workflow *actionlint.Workflow, path string,
 }
 
 func validateUntrustedCodeCheckout(workflow *actionlint.Workflow, path string,
-	dl checker.DetailLogger, pdata *patternCbData) error {
+	dl checker.DetailLogger, pdata *patternCbData,
+) error {
 	if !usesEventTrigger(workflow, triggerPullRequestTarget) {
 		return nil
 	}
@@ -229,7 +232,8 @@ func jobUsesEnvironment(job *actionlint.Job) bool {
 }
 
 func checkJobForUsedSecrets(job *actionlint.Job, triggers map[triggerName]bool,
-	path string, dl checker.DetailLogger, pdata *patternCbData) error {
+	path string, dl checker.DetailLogger, pdata *patternCbData,
+) error {
 	if job == nil {
 		return nil
 	}
@@ -271,7 +275,8 @@ func checkJobForUsedSecrets(job *actionlint.Job, triggers map[triggerName]bool,
 }
 
 func workflowUsesCodeCheckoutAndNoEnvironment(workflow *actionlint.Workflow,
-	triggers map[triggerName]bool) bool {
+	triggers map[triggerName]bool,
+) bool {
 	if workflow == nil {
 		return false
 	}
@@ -318,7 +323,8 @@ func jobUsesCodeCheckout(job *actionlint.Job) (bool, string) {
 }
 
 func checkJobForUntrustedCodeCheckout(job *actionlint.Job, path string,
-	dl checker.DetailLogger, pdata *patternCbData) error {
+	dl checker.DetailLogger, pdata *patternCbData,
+) error {
 	if job == nil {
 		return nil
 	}
@@ -359,7 +365,8 @@ func checkJobForUntrustedCodeCheckout(job *actionlint.Job, path string,
 }
 
 func validateScriptInjection(workflow *actionlint.Workflow, path string,
-	dl checker.DetailLogger, pdata *patternCbData) error {
+	dl checker.DetailLogger, pdata *patternCbData,
+) error {
 	for _, job := range workflow.Jobs {
 		if job == nil {
 			continue
@@ -382,7 +389,8 @@ func validateScriptInjection(workflow *actionlint.Workflow, path string,
 }
 
 func checkWorkflowSecretInEnv(workflow *actionlint.Workflow, triggers map[triggerName]bool,
-	path string, dl checker.DetailLogger, pdata *patternCbData) error {
+	path string, dl checker.DetailLogger, pdata *patternCbData,
+) error {
 	// We need code checkout and not environment rule protection.
 	if !workflowUsesCodeCheckoutAndNoEnvironment(workflow, triggers) {
 		return nil
@@ -392,7 +400,8 @@ func checkWorkflowSecretInEnv(workflow *actionlint.Workflow, triggers map[trigge
 }
 
 func checkSecretInEnv(env *actionlint.Env, path string,
-	dl checker.DetailLogger, pdata *patternCbData) error {
+	dl checker.DetailLogger, pdata *patternCbData,
+) error {
 	if env == nil {
 		return nil
 	}
@@ -406,7 +415,8 @@ func checkSecretInEnv(env *actionlint.Env, path string,
 }
 
 func checkSecretInRun(step *actionlint.Step, path string,
-	dl checker.DetailLogger, pdata *patternCbData) error {
+	dl checker.DetailLogger, pdata *patternCbData,
+) error {
 	if step == nil || step.Exec == nil {
 		return nil
 	}
@@ -421,7 +431,8 @@ func checkSecretInRun(step *actionlint.Step, path string,
 }
 
 func checkSecretInActionArgs(step *actionlint.Step, path string,
-	dl checker.DetailLogger, pdata *patternCbData) error {
+	dl checker.DetailLogger, pdata *patternCbData,
+) error {
 	if step == nil || step.Exec == nil {
 		return nil
 	}
@@ -442,7 +453,8 @@ func checkSecretInActionArgs(step *actionlint.Step, path string,
 }
 
 func checkSecretInScript(script string, pos *actionlint.Pos, path string,
-	dl checker.DetailLogger, pdata *patternCbData) error {
+	dl checker.DetailLogger, pdata *patternCbData,
+) error {
 	for {
 		s := strings.Index(script, "${{")
 		if s == -1 {
@@ -454,8 +466,13 @@ func checkSecretInScript(script string, pos *actionlint.Pos, path string,
 			return sce.WithMessage(sce.ErrScorecardInternal, errInvalidGitHubWorkflow.Error())
 		}
 
+		// Note: The default GitHub token is allowed, as it has
+		// only read permission for `pull_request`.
+		// For `pull_request_event`, we use other signals such as
+		// whether checkout action is used.
 		variable := strings.Trim(script[s:s+e+2], " ")
-		if strings.Contains(variable, "secrets.") {
+		if !strings.Contains(variable, "secrets.GITHUB_TOKEN") &&
+			strings.Contains(variable, "secrets.") {
 			line := fileparser.GetLineNumber(pos)
 			dl.Warn(&checker.LogMessage{
 				Path:   path,
@@ -472,7 +489,8 @@ func checkSecretInScript(script string, pos *actionlint.Pos, path string,
 }
 
 func checkVariablesInScript(script string, pos *actionlint.Pos, path string,
-	dl checker.DetailLogger, pdata *patternCbData) error {
+	dl checker.DetailLogger, pdata *patternCbData,
+) error {
 	for {
 		s := strings.Index(script, "${{")
 		if s == -1 {
@@ -548,7 +566,8 @@ func createResultForDangerousWorkflowPatterns(result patternCbData, err error) c
 }
 
 func testValidateGitHubActionDangerousWorkflow(pathfn string,
-	content []byte, dl checker.DetailLogger) checker.CheckResult {
+	content []byte, dl checker.DetailLogger,
+) checker.CheckResult {
 	data := patternCbData{
 		workflowPattern: make(map[dangerousResults]bool),
 	}

diff --git a/checks/dangerous_workflow_test.go b/checks/dangerous_workflow_test.go
@@ -241,6 +241,28 @@ func TestGithubDangerousWorkflow(t *testing.T) {
 				NumberOfDebug: 0,
 			},
 		},
+		{
+			name:     "default secret in pull request",
+			filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-default-secret-pr.yml",
+			expected: scut.TestReturn{
+				Error:         nil,
+				Score:         checker.MaxResultConfidence,
+				NumberOfWarn:  0,
+				NumberOfInfo:  0,
+				NumberOfDebug: 0,
+			},
+		},
+		{
+			name:     "default secret in pull request target",
+			filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-default-secret-prt.yml",
+			expected: scut.TestReturn{
+				Error:         nil,
+				Score:         checker.MinResultConfidence,
+				NumberOfWarn:  1,
+				NumberOfInfo:  0,
+				NumberOfDebug: 0,
+			},
+		},
 		{
 			name:     "secret in top env no checkout pull request target",
 			filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-no-checkout-prt.yml",