From 1f085694f16a65f758f128ef9307a2bd5e003906 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 25 Apr 2022 23:33:56 +0000 Subject: [PATCH 1/4] Remove pull_request --- checks/raw/dangerous_workflow.go | 188 +------------------------------ 1 file changed, 1 insertion(+), 187 deletions(-) diff --git a/checks/raw/dangerous_workflow.go b/checks/raw/dangerous_workflow.go index 15755c9bb8d..9cc28945d29 100644 --- a/checks/raw/dangerous_workflow.go +++ b/checks/raw/dangerous_workflow.go @@ -60,7 +60,6 @@ type triggerName string var ( triggerPullRequestTarget = triggerName("pull_request_target") triggerWorkflowRun = triggerName("workflow_run") - triggerPullRequest = triggerName("pull_request") checkoutUntrustedPullRequestRef = "github.event.pull_request" checkoutUntrustedWorkflowRunRef = "github.event.workflow_run" ) @@ -117,55 +116,10 @@ var validateGitHubActionWorkflowPatterns fileparser.DoWhileTrueOnFileContent = f return false, err } - // 3. Check for secrets used in workflows triggered by pull requests. - if err := validateSecretsInPullRequests(workflow, path, pdata); err != nil { - return false, err - } - // TODO: Check other dangerous patterns. return true, nil } -func validateSecretsInPullRequests(workflow *actionlint.Workflow, path string, - pdata *checker.DangerousWorkflowData, -) error { - triggers := make(map[triggerName]bool) - - // We need pull request trigger. - usesPullRequest := usesEventTrigger(workflow, triggerPullRequest) - usesPullRequestTarget := usesEventTrigger(workflow, triggerPullRequestTarget) - usesWorkflowRun := usesEventTrigger(workflow, triggerWorkflowRun) - - if !usesPullRequest && !usesPullRequestTarget && !usesWorkflowRun { - return nil - } - - // Record the triggers. - if usesPullRequest { - triggers[triggerPullRequest] = usesPullRequest - } - if usesPullRequestTarget { - triggers[triggerPullRequestTarget] = usesPullRequestTarget - } - if usesWorkflowRun { - triggers[triggerWorkflowRun] = usesWorkflowRun - } - - // Secrets used in env at the top of the wokflow. - if err := checkWorkflowSecretInEnv(workflow, triggers, path, pdata); err != nil { - return err - } - - // Secrets used on jobs. - for _, job := range workflow.Jobs { - if err := checkJobForUsedSecrets(job, triggers, path, pdata); err != nil { - return err - } - } - - return nil -} - func validateUntrustedCodeCheckout(workflow *actionlint.Workflow, path string, pdata *checker.DangerousWorkflowData, ) error { @@ -202,40 +156,6 @@ func jobUsesEnvironment(job *actionlint.Job) bool { job.Environment.Name.Value != "" } -func checkJobForUsedSecrets(job *actionlint.Job, triggers map[triggerName]bool, - path string, pdata *checker.DangerousWorkflowData, -) error { - if job == nil { - return nil - } - - // If the job has an environment, assume it's an env secret gated by - // some approval and don't alert. - if !jobUsesCodeCheckoutAndNoEnvironment(job, triggers) { - return nil - } - - // https://docs.github.com/en/actions/security-guides/encrypted-secrets#naming-your-secrets - for _, step := range job.Steps { - if step == nil { - continue - } - - if err := checkSecretInActionArgs(step, job, path, pdata); err != nil { - return err - } - - if err := checkSecretInRun(step, job, path, pdata); err != nil { - return err - } - - if err := checkSecretInEnv(step.Env, job, path, pdata); err != nil { - return err - } - } - return nil -} - func workflowUsesCodeCheckoutAndNoEnvironment(workflow *actionlint.Workflow, triggers map[triggerName]bool, ) bool { @@ -256,14 +176,12 @@ func jobUsesCodeCheckoutAndNoEnvironment(job *actionlint.Job, triggers map[trigg if job == nil { return false } - _, usesPullRequest := triggers[triggerPullRequest] _, usesPullRequestTarget := triggers[triggerPullRequestTarget] _, usesWorkflowRun := triggers[triggerWorkflowRun] chk, ref := jobUsesCodeCheckout(job) if !jobUsesEnvironment(job) { - if (chk && usesPullRequest) || - (chk && usesPullRequestTarget && strings.Contains(ref, checkoutUntrustedPullRequestRef)) || + if (chk && usesPullRequestTarget && strings.Contains(ref, checkoutUntrustedPullRequestRef)) || (chk && usesWorkflowRun && strings.Contains(ref, checkoutUntrustedWorkflowRunRef)) { return true } @@ -383,110 +301,6 @@ func validateScriptInjection(workflow *actionlint.Workflow, path string, return nil } -func checkWorkflowSecretInEnv(workflow *actionlint.Workflow, triggers map[triggerName]bool, - path string, pdata *checker.DangerousWorkflowData, -) error { - // We need code checkout and not environment rule protection. - if !workflowUsesCodeCheckoutAndNoEnvironment(workflow, triggers) { - return nil - } - - return checkSecretInEnv(workflow.Env, nil, path, pdata) -} - -func checkSecretInEnv(env *actionlint.Env, job *actionlint.Job, path string, - pdata *checker.DangerousWorkflowData, -) error { - if env == nil { - return nil - } - - for _, v := range env.Vars { - if err := checkSecretInScript(v.Value.Value, v.Value.Pos, job, path, pdata); err != nil { - return err - } - } - return nil -} - -func checkSecretInRun(step *actionlint.Step, job *actionlint.Job, path string, - pdata *checker.DangerousWorkflowData, -) error { - if step == nil || step.Exec == nil { - return nil - } - - run, ok := step.Exec.(*actionlint.ExecRun) - if ok && run.Run != nil { - if err := checkSecretInScript(run.Run.Value, run.Run.Pos, job, path, pdata); err != nil { - return err - } - } - return nil -} - -func checkSecretInActionArgs(step *actionlint.Step, job *actionlint.Job, path string, - pdata *checker.DangerousWorkflowData, -) error { - if step == nil || step.Exec == nil { - return nil - } - - e, ok := step.Exec.(*actionlint.ExecAction) - if ok && e.Uses != nil { - // Check for reference. If not defined for a pull_request_target event, this defaults to - // the base branch of the pull request. - for _, v := range e.Inputs { - if v.Value != nil { - if err := checkSecretInScript(v.Value.Value, v.Value.Pos, job, path, pdata); err != nil { - return err - } - } - } - } - return nil -} - -func checkSecretInScript(script string, pos *actionlint.Pos, - job *actionlint.Job, path string, - pdata *checker.DangerousWorkflowData, -) error { - for { - s := strings.Index(script, "${{") - if s == -1 { - break - } - - e := strings.Index(script[s:], "}}") - if e == -1 { - 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.GITHUB_TOKEN") && - strings.Contains(variable, "secrets.") { - line := fileparser.GetLineNumber(pos) - pdata.SecretInPullRequests = append(pdata.SecretInPullRequests, - checker.EncryptedSecret{ - File: checker.File{ - Path: path, - Type: checker.FileTypeSource, - Offset: line, - Snippet: variable, - }, - Job: createJob(job), - }, - ) - } - script = script[s+e:] - } - return nil -} - func checkVariablesInScript(script string, pos *actionlint.Pos, job *actionlint.Job, path string, pdata *checker.DangerousWorkflowData, From 3f49d4f71733fc2ff46087bb4fc845860b34b06b Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 25 Apr 2022 23:36:04 +0000 Subject: [PATCH 2/4] updates --- checker/raw_result.go | 11 ++--------- pkg/json_raw_results.go | 35 +++++------------------------------ 2 files changed, 7 insertions(+), 39 deletions(-) diff --git a/checker/raw_result.go b/checker/raw_result.go index ee16274cd63..9c2093d9812 100644 --- a/checker/raw_result.go +++ b/checker/raw_result.go @@ -286,9 +286,8 @@ type CIIBestPracticesData struct { // DangerousWorkflowData contains raw results // for dangerous workflow check. type DangerousWorkflowData struct { - ScriptInjections []ScriptInjection - SecretInPullRequests []EncryptedSecret - UntrustedCheckouts []UntrustedCheckout + ScriptInjections []ScriptInjection + UntrustedCheckouts []UntrustedCheckout // TODO: other } @@ -304,12 +303,6 @@ type ScriptInjection struct { File File } -// EncryptedSecret represents an encrypted secret. -type EncryptedSecret struct { - Job *WorkflowJob - File File -} - // WorkflowJob reprresents a workflow job. type WorkflowJob struct { Name *string diff --git a/pkg/json_raw_results.go b/pkg/json_raw_results.go index 525258acdf6..6cfb0de38e2 100644 --- a/pkg/json_raw_results.go +++ b/pkg/json_raw_results.go @@ -143,9 +143,8 @@ type jsonLicense struct { } type jsonWorkflows struct { - UntrustedCheckouts []jsonUntrustedCheckout `json:"untrustedCheckouts"` - ScriptInjections []jsonScriptInjection `json:"scriptInjections"` - SecretInPullRequests []jsonEncryptedSecret `json:"secretInPullRequests"` + UntrustedCheckouts []jsonUntrustedCheckout `json:"untrustedCheckouts"` + ScriptInjections []jsonScriptInjection `json:"scriptInjections"` } type jsonUntrustedCheckout struct { @@ -158,11 +157,6 @@ type jsonScriptInjection struct { File jsonFile `json:"file"` } -type jsonEncryptedSecret struct { - Job *jsonWorkflowJob `json:"job"` - File jsonFile `json:"file"` -} - type jsonWorkflowJob struct { Name *string `json:"name"` ID *string `json:"id"` @@ -241,25 +235,6 @@ func (r *jsonScorecardRawResult) addDangerousWorkflowRawResults(df *checker.Dang r.Results.Workflows.ScriptInjections = append(r.Results.Workflows.ScriptInjections, v) } - // Secrets in pull requests. - for _, e := range df.SecretInPullRequests { - v := jsonEncryptedSecret{ - File: jsonFile{ - Path: e.File.Path, - Offset: int(e.File.Offset), - }, - } - 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.SecretInPullRequests = append(r.Results.Workflows.SecretInPullRequests, v) - } return nil } @@ -540,10 +515,10 @@ func (r *jsonScorecardRawResult) fillJSONRawResults(raw *checker.RawResults) err // CII-Best-Practices. if err := r.addOssfBestPracticesRawResults(&raw.CIIBestPracticesResults); err != nil { - return sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return sce.WithMessage(sce.ErrScorecardInternal, err.Error()) } - - // Dangerous workflow. + + // Dangerous workflow. if err := r.addDangerousWorkflowRawResults(&raw.DangerousWorkflowResults); err != nil { return sce.WithMessage(sce.ErrScorecardInternal, err.Error()) } From 753370ad6f725b1bfd753a0eac1e635c413e888e Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 26 Apr 2022 00:07:48 +0000 Subject: [PATCH 3/4] updates --- checks/evaluation/dangerous_workflow.go | 14 +--- checks/raw/dangerous_workflow_test.go | 76 +------------------ ...ow-dangerous-pattern-default-secret-pr.yml | 36 --------- ...w-dangerous-pattern-default-secret-prt.yml | 36 --------- ...w-dangerous-pattern-secret-action-args.yml | 17 ----- ...-dangerous-pattern-secret-all-checkout.yml | 60 --------------- ...-pattern-secret-env-checkout-noref-prt.yml | 52 ------------- ...ous-pattern-secret-env-environment-prt.yml | 63 --------------- ...ngerous-pattern-secret-env-environment.yml | 61 --------------- ...ous-pattern-secret-env-no-checkout-prt.yml | 52 ------------- ...ngerous-pattern-secret-env-no-checkout.yml | 52 ------------- ...kflow-dangerous-pattern-secret-env-prt.yml | 27 ------- ...-workflow-dangerous-pattern-secret-env.yml | 22 ------ ...ngerous-pattern-secret-no-pull-request.yml | 60 --------------- ...kflow-dangerous-pattern-secret-run-prt.yml | 14 ---- ...-workflow-dangerous-pattern-secret-run.yml | 12 --- 16 files changed, 4 insertions(+), 650 deletions(-) delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-default-secret-pr.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-default-secret-prt.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-action-args.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-all-checkout.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-checkout-noref-prt.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-environment-prt.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-environment.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-no-checkout-prt.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-no-checkout.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-prt.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-no-pull-request.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-run-prt.yml delete mode 100644 checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-run.yml diff --git a/checks/evaluation/dangerous_workflow.go b/checks/evaluation/dangerous_workflow.go index a4ebc963e31..6fe443926ff 100644 --- a/checks/evaluation/dangerous_workflow.go +++ b/checks/evaluation/dangerous_workflow.go @@ -52,20 +52,8 @@ func DangerousWorkflow(name string, dl checker.DetailLogger, }) } - // Secrets in pull requests. - for _, e := range r.SecretInPullRequests { - dl.Warn(&checker.LogMessage{ - Path: e.File.Path, - Type: e.File.Type, - Offset: e.File.Offset, - Text: fmt.Sprintf("secret accessible to pull requests '%v'", e.File.Snippet), - Snippet: e.File.Snippet, - }) - } - if len(r.ScriptInjections) > 0 || - len(r.UntrustedCheckouts) > 0 || - len(r.SecretInPullRequests) > 0 { + len(r.UntrustedCheckouts) > 0 { return createResult(name, checker.MinResultScore) } return createResult(name, checker.MaxResultScore) diff --git a/checks/raw/dangerous_workflow_test.go b/checks/raw/dangerous_workflow_test.go index d25e734e9dd..c271831bb4b 100644 --- a/checks/raw/dangerous_workflow_test.go +++ b/checks/raw/dangerous_workflow_test.go @@ -106,12 +106,12 @@ func TestGithubDangerousWorkflow(t *testing.T) { { name: "run untrusted code checkout test - workflow_run", filename: ".github/workflows/github-workflow-dangerous-pattern-untrusted-checkout-workflow_run.yml", - expected: ret{nb: 2}, + expected: ret{nb: 1}, }, { name: "run untrusted code checkout test", filename: ".github/workflows/github-workflow-dangerous-pattern-untrusted-checkout.yml", - expected: ret{nb: 2}, + expected: ret{nb: 1}, }, { name: "run trusted code checkout test", @@ -148,76 +148,6 @@ func TestGithubDangerousWorkflow(t *testing.T) { filename: ".github/workflows/github-workflow-dangerous-pattern-untrusted-script-injection-wildcard.yml", expected: ret{nb: 1}, }, - { - name: "secret in top env no checkout", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-env-no-checkout.yml", - expected: ret{nb: 0}, - }, - { - name: "secret in action args", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-action-args.yml", - expected: ret{nb: 1}, - }, - { - name: "secret in all places", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-all-checkout.yml", - expected: ret{nb: 7}, - }, - { - name: "secret in env", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-env.yml", - expected: ret{nb: 2}, - }, - { - name: "secret in env", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-no-pull-request.yml", - expected: ret{nb: 0}, - }, - { - name: "secret in env", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-run.yml", - expected: ret{nb: 1}, - }, - { - name: "secret with environment protection", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-env-environment.yml", - expected: ret{nb: 0}, - }, - { - name: "secret with environment protection pull request target", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-env-environment-prt.yml", - expected: ret{nb: 1}, - }, - { - name: "secret in env pull request target", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-run-prt.yml", - expected: ret{nb: 2}, - }, - { - name: "secret in env pull request target", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-env-prt.yml", - expected: ret{nb: 4}, - }, - { - name: "default secret in pull request", - filename: ".github/workflows/github-workflow-dangerous-pattern-default-secret-pr.yml", - expected: ret{nb: 0}, - }, - { - name: "default secret in pull request target", - filename: ".github/workflows/github-workflow-dangerous-pattern-default-secret-prt.yml", - expected: ret{nb: 1}, - }, - { - name: "secret in top env no checkout pull request target", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-env-no-checkout-prt.yml", - expected: ret{nb: 0}, - }, - { - name: "secret in top env checkout no ref pull request target", - filename: ".github/workflows/github-workflow-dangerous-pattern-secret-env-checkout-noref-prt.yml", - expected: ret{nb: 0}, - }, } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below @@ -245,7 +175,7 @@ func TestGithubDangerousWorkflow(t *testing.T) { return } - nb := len(dw.ScriptInjections) + len(dw.SecretInPullRequests) + len(dw.UntrustedCheckouts) + nb := len(dw.ScriptInjections) + len(dw.UntrustedCheckouts) if nb != tt.expected.nb { t.Errorf(cmp.Diff(nb, tt.expected.nb)) } diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-default-secret-pr.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-default-secret-pr.yml deleted file mode 100644 index 9e927e8a00c..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-default-secret-pr.yml +++ /dev/null @@ -1,36 +0,0 @@ -name: Close issue on Jira - -on: - pull_request - -env: - BLA: ${{ secrets.GITHUB_TOKEN }} - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1.2.3 - with: - ref: ${{ github.event.pull_request.head.sha }} - name: Use in env toJson - - - uses: some/action@v1.2.3 - with: - option: ${{ secrets.GITHUB_TOKEN }} - name: Use secret in args - - - name: Use in with toJson - env: - GITHUB_CONTEXT: ${{ secrets.GITHUB_TOKEN }} - run: | - echo "$GITHUB_CONTEXT" - echo "${{ secrets.GITHUB_TOKEN }}" - - - name: Use in with toJson - uses: some/action@v1.2.3 - env: - GITHUB_CONTEXT: ${{ secrets.GITHUB_TOKEN }} - run: | - echo "$GITHUB_CONTEXT" - echo "${{ secrets.GITHUB_TOKEN }}" diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-default-secret-prt.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-default-secret-prt.yml deleted file mode 100644 index 28e185b57cb..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-default-secret-prt.yml +++ /dev/null @@ -1,36 +0,0 @@ -name: Close issue on Jira - -on: - pull_request_target - -env: - BLA: ${{ secrets.GITHUB_TOKEN }} - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1.2.3 - with: - ref: ${{ github.event.pull_request.head.sha }} - name: Use in env toJson - - - uses: some/action@v1.2.3 - with: - option: ${{ secrets.GITHUB_TOKEN }} - name: Use secret in args - - - name: Use in with toJson - env: - GITHUB_CONTEXT: ${{ secrets.GITHUB_TOKEN }} - run: | - echo "$GITHUB_CONTEXT" - echo "${{ secrets.GITHUB_TOKEN }}" - - - name: Use in with toJson - uses: some/action@v1.2.3 - env: - GITHUB_CONTEXT: ${{ secrets.GITHUB_TOKEN }} - run: | - echo "$GITHUB_CONTEXT" - echo "${{ secrets.GITHUB_TOKEN }}" diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-action-args.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-action-args.yml deleted file mode 100644 index f25295764ea..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-action-args.yml +++ /dev/null @@ -1,17 +0,0 @@ -name: Close issue on Jira - -on: - pull_request - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1.2.3 - name: Use in env toJson - - - name: Use in with toJson - uses: some/action@main - with: - some-args: ${{ toJson(secrets.SE12) }} - \ No newline at end of file diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-all-checkout.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-all-checkout.yml deleted file mode 100644 index 96cd9be974b..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-all-checkout.yml +++ /dev/null @@ -1,60 +0,0 @@ -name: Close issue on Jira - -on: - pull_request - -env: - BLA: ${{ secrets.SE00 }} - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1.2.3 - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ toJson(secrets.SE11) }} - #run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@main - with: - some-args: ${{ toJson(secrets.SE12) }} - #run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ toJson(secrets.SE13) }}" - test2: - runs-on: ubuntu-latest - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE21 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE22 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE23 }}" - test3: - runs-on: ubuntu-latest - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE31 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE32 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE33 }}" - - - uses: actions/checkout@v1.2.3 \ No newline at end of file diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-checkout-noref-prt.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-checkout-noref-prt.yml deleted file mode 100644 index 6cdfa84f7c0..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-checkout-noref-prt.yml +++ /dev/null @@ -1,52 +0,0 @@ -name: Close issue on Jira - -on: - pull_request_target - -env: - BLA: ${{ secrets.SE00 }} - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - name: Use in with toJson - uses: some/action@main - with: - some-args: ${{ toJson(secrets.SE12) }} - - - name: Use in run toJson - run: echo "${{ toJson(secrets.SE13) }}" - test2: - runs-on: ubuntu-latest - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE21 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE22 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE23 }}" - test3: - runs-on: ubuntu-latest - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE31 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE32 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE33 }}" - \ No newline at end of file diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-environment-prt.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-environment-prt.yml deleted file mode 100644 index 6e7cbdff553..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-environment-prt.yml +++ /dev/null @@ -1,63 +0,0 @@ -name: Close issue on Jira - -on: - pull_request_target - -env: - BLA: ${{ secrets.SE00 }} - -jobs: - test1: - runs-on: ubuntu-latest - environment: protected - steps: - - name: Use in with toJson - uses: some/action@main - with: - some-args: ${{ toJson(secrets.SE12) }} - - - name: Use in run toJson - run: echo "${{ toJson(secrets.SE13) }}" - - - uses: actions/checkout@v1.2.3 - with: - ref: ${{ github.event.pull_request.head.sha }} - test2: - runs-on: ubuntu-latest - environment: protected - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE21 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE22 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE23 }}" - - - uses: actions/checkout@v1.2.3 - test3: - runs-on: ubuntu-latest - environment: protected - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE31 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE32 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE33 }}" - - - uses: actions/checkout@v1.2.3 - \ No newline at end of file diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-environment.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-environment.yml deleted file mode 100644 index 1f86e327d69..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-environment.yml +++ /dev/null @@ -1,61 +0,0 @@ -name: Close issue on Jira - -on: - pull_request - -env: - BLA: ${{ secrets.SE00 }} - -jobs: - test1: - runs-on: ubuntu-latest - environment: protected - steps: - - name: Use in with toJson - uses: some/action@main - with: - some-args: ${{ toJson(secrets.SE12) }} - - - name: Use in run toJson - run: echo "${{ toJson(secrets.SE13) }}" - - - uses: actions/checkout@v1.2.3 - test2: - runs-on: ubuntu-latest - environment: protected - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE21 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE22 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE23 }}" - - - uses: actions/checkout@v1.2.3 - test3: - runs-on: ubuntu-latest - environment: protected - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE31 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE32 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE33 }}" - - - uses: actions/checkout@v1.2.3 - \ No newline at end of file diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-no-checkout-prt.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-no-checkout-prt.yml deleted file mode 100644 index 354742a2ab8..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-no-checkout-prt.yml +++ /dev/null @@ -1,52 +0,0 @@ -name: Close issue on Jira - -on: - pull_request - -env: - BLA: ${{ secrets.SE00 }} - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - name: Use in with toJson - uses: some/action@main - with: - some-args: ${{ toJson(secrets.SE12) }} - - - name: Use in run toJson - run: echo "${{ toJson(secrets.SE13) }}" - test2: - runs-on: ubuntu-latest - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE21 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE22 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE23 }}" - test3: - runs-on: ubuntu-latest - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE31 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE32 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE33 }}" - \ No newline at end of file diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-no-checkout.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-no-checkout.yml deleted file mode 100644 index 354742a2ab8..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-no-checkout.yml +++ /dev/null @@ -1,52 +0,0 @@ -name: Close issue on Jira - -on: - pull_request - -env: - BLA: ${{ secrets.SE00 }} - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - name: Use in with toJson - uses: some/action@main - with: - some-args: ${{ toJson(secrets.SE12) }} - - - name: Use in run toJson - run: echo "${{ toJson(secrets.SE13) }}" - test2: - runs-on: ubuntu-latest - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE21 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE22 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE23 }}" - test3: - runs-on: ubuntu-latest - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE31 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE32 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE33 }}" - \ No newline at end of file diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-prt.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-prt.yml deleted file mode 100644 index 8e39566beca..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-prt.yml +++ /dev/null @@ -1,27 +0,0 @@ -name: Close issue on Jira - -on: - pull_request_target - -env: - BLA: ${{ secrets.SE00 }} - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1.2.3 - with: - ref: ${{ github.event.pull_request.head.sha }} - name: Use in env toJson - - - name: Use in with toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE21 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - env: - GITHUB_CONTEXT: ${{ secrets.SE22 }} - run: echo "$GITHUB_CONTEXT" diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env.yml deleted file mode 100644 index 443e9fcbc6b..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env.yml +++ /dev/null @@ -1,22 +0,0 @@ -name: Close issue on Jira - -on: - pull_request - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1.2.3 - name: Use in env toJson - - - name: Use in with toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE21 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - env: - GITHUB_CONTEXT: ${{ secrets.SE22 }} - run: echo "$GITHUB_CONTEXT" diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-no-pull-request.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-no-pull-request.yml deleted file mode 100644 index c2e7ec5848e..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-no-pull-request.yml +++ /dev/null @@ -1,60 +0,0 @@ -name: Close issue on Jira - -on: - push - -env: - BLA: ${{ secrets.SE00 }} - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1.2.3 - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ toJson(secrets.SE11) }} - #run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@main - with: - some-args: ${{ toJson(secrets.SE12) }} - #run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ toJson(secrets.SE13) }}" - test2: - runs-on: ubuntu-latest - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE21 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE22 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE23 }}" - test3: - runs-on: ubuntu-latest - steps: - - name: Use in env toJson - env: - GITHUB_CONTEXT: ${{ secrets.SE31 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in with toJson - uses: some/action@v1.2.3 - with: - some-args: ${{ secrets.SE32 }} - run: echo "$GITHUB_CONTEXT" - - - name: Use in run toJson - run: echo "${{ secrets.SE33 }}" - - - uses: actions/checkout@v1.2.3 \ No newline at end of file diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-run-prt.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-run-prt.yml deleted file mode 100644 index fdaaea25328..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-run-prt.yml +++ /dev/null @@ -1,14 +0,0 @@ -name: Close issue on Jira - -on: - pull_request_target - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1.2.3 - with: - ref: ${{ github.event.pull_request.head.sha }} - - name: Use in run toJson - run: echo "${{ toJson(secrets.SE13) }}" \ No newline at end of file diff --git a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-run.yml b/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-run.yml deleted file mode 100644 index 0adf8680bbe..00000000000 --- a/checks/testdata/.github/workflows/github-workflow-dangerous-pattern-secret-run.yml +++ /dev/null @@ -1,12 +0,0 @@ -name: Close issue on Jira - -on: - pull_request - -jobs: - test1: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1.2.3 - - name: Use in run toJson - run: echo "${{ toJson(secrets.SE13) }}" \ No newline at end of file From a912c894210b5b6da071c051e6335b688dac356e Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 26 Apr 2022 15:15:49 +0000 Subject: [PATCH 4/4] linter and e2e --- checks/raw/dangerous_workflow.go | 70 -------------------------------- e2e/dangerous_workflow_test.go | 6 +-- 2 files changed, 3 insertions(+), 73 deletions(-) diff --git a/checks/raw/dangerous_workflow.go b/checks/raw/dangerous_workflow.go index 9cc28945d29..910d455f30e 100644 --- a/checks/raw/dangerous_workflow.go +++ b/checks/raw/dangerous_workflow.go @@ -147,76 +147,6 @@ func usesEventTrigger(workflow *actionlint.Workflow, name triggerName) bool { return false } -func jobUsesEnvironment(job *actionlint.Job) bool { - if job.Environment == nil { - return false - } - - return job.Environment.Name != nil && - job.Environment.Name.Value != "" -} - -func workflowUsesCodeCheckoutAndNoEnvironment(workflow *actionlint.Workflow, - triggers map[triggerName]bool, -) bool { - if workflow == nil { - return false - } - - for _, job := range workflow.Jobs { - if jobUsesCodeCheckoutAndNoEnvironment(job, triggers) { - return true - } - } - return false -} - -func jobUsesCodeCheckoutAndNoEnvironment(job *actionlint.Job, triggers map[triggerName]bool, -) bool { - if job == nil { - return false - } - _, usesPullRequestTarget := triggers[triggerPullRequestTarget] - _, usesWorkflowRun := triggers[triggerWorkflowRun] - - chk, ref := jobUsesCodeCheckout(job) - if !jobUsesEnvironment(job) { - if (chk && usesPullRequestTarget && strings.Contains(ref, checkoutUntrustedPullRequestRef)) || - (chk && usesWorkflowRun && strings.Contains(ref, checkoutUntrustedWorkflowRunRef)) { - return true - } - } - - return false -} - -func jobUsesCodeCheckout(job *actionlint.Job) (bool, string) { - if job == nil { - return false, "" - } - - hasCheckout := false - for _, step := range job.Steps { - if step == nil || step.Exec == nil { - continue - } - // Check for a step that uses actions/checkout - e, ok := step.Exec.(*actionlint.ExecAction) - if !ok || e.Uses == nil { - continue - } - if strings.Contains(e.Uses.Value, "actions/checkout") { - hasCheckout = true - ref, ok := e.Inputs["ref"] - if !ok || ref.Value == nil { - continue - } - return true, ref.Value.Value - } - } - return hasCheckout, "" -} - func createJob(job *actionlint.Job) *checker.WorkflowJob { if job == nil { return nil diff --git a/e2e/dangerous_workflow_test.go b/e2e/dangerous_workflow_test.go index 84024463279..9acaaf0af5c 100644 --- a/e2e/dangerous_workflow_test.go +++ b/e2e/dangerous_workflow_test.go @@ -48,7 +48,7 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() { expected := scut.TestReturn{ Error: nil, Score: checker.MinResultScore, - NumberOfWarn: 2, + NumberOfWarn: 1, NumberOfInfo: 0, NumberOfDebug: 0, } @@ -76,7 +76,7 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() { expected := scut.TestReturn{ Error: nil, Score: checker.MinResultScore, - NumberOfWarn: 2, + NumberOfWarn: 1, NumberOfInfo: 0, NumberOfDebug: 0, } @@ -116,7 +116,7 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() { expected := scut.TestReturn{ Error: nil, Score: checker.MinResultScore, - NumberOfWarn: 2, + NumberOfWarn: 1, NumberOfInfo: 0, NumberOfDebug: 0, }