Check for secrets in pull_request_target (#1634)

* checks/dangerous_workflow.go: add pull_request_target support for secrets

* missing files

* linter
This commit is contained in:
laurentsimon 2022-02-15 08:04:57 -08:00 committed by GitHub
parent e3637c9e17
commit e7fd58d9a3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 316 additions and 22 deletions

View File

@ -82,6 +82,7 @@ type triggerName string
var ( var (
triggerPullRequestTarget = triggerName("pull_request_target") triggerPullRequestTarget = triggerName("pull_request_target")
triggerPullRequest = triggerName("pull_request") triggerPullRequest = triggerName("pull_request")
checkoutUntrustedRef = "github.event.pull_request"
) )
// Holds stateful data to pass thru callbacks. // Holds stateful data to pass thru callbacks.
@ -146,22 +147,31 @@ func validateGitHubActionWorkflowPatterns(path string, content []byte, dl checke
func validateSecretsInPullRequests(workflow *actionlint.Workflow, path string, 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. // We need pull request trigger.
if !usesEventTrigger(workflow, triggerPullRequest) { usesPullRequest := usesEventTrigger(workflow, triggerPullRequest)
usesPullRequestTarget := usesEventTrigger(workflow, triggerPullRequestTarget)
if !usesPullRequest && !usesPullRequestTarget {
return nil return nil
} }
// Record the triggers.
if usesPullRequest {
triggers[triggerPullRequest] = usesPullRequest
}
if usesPullRequestTarget {
triggers[triggerPullRequestTarget] = usesPullRequestTarget
}
// Secrets used in env at the top of the wokflow. // Secrets used in env at the top of the wokflow.
if err := checkWorkflowSecretInEnv(workflow, path, dl, pdata); err != nil { if err := checkWorkflowSecretInEnv(workflow, triggers, path, dl, pdata); err != nil {
return err return err
} }
// Secrets used on jobs. // Secrets used on jobs.
for _, job := range workflow.Jobs { for _, job := range workflow.Jobs {
if !jobUsesCodeCheckout(job) { if err := checkJobForUsedSecrets(job, triggers, path, dl, pdata); err != nil {
continue
}
if err := checkJobForUsedSecrets(job, path, dl, pdata); err != nil {
return err return err
} }
} }
@ -204,8 +214,8 @@ func jobUsesEnvironment(job *actionlint.Job) bool {
job.Environment.Name.Value != "" job.Environment.Name.Value != ""
} }
func checkJobForUsedSecrets(job *actionlint.Job, path string, func checkJobForUsedSecrets(job *actionlint.Job, triggers map[triggerName]bool,
dl checker.DetailLogger, pdata *patternCbData) error { path string, dl checker.DetailLogger, pdata *patternCbData) error {
if job == nil { if job == nil {
return nil return nil
} }
@ -216,6 +226,15 @@ func checkJobForUsedSecrets(job *actionlint.Job, path string,
return nil return nil
} }
// For pull request target, we need a ref to the pull request.
_, usesPullRequest := triggers[triggerPullRequest]
_, usesPullRequestTarget := triggers[triggerPullRequestTarget]
chk, ref := jobUsesCodeCheckout(job)
if !((chk && usesPullRequest) ||
(chk && usesPullRequestTarget && strings.Contains(ref, checkoutUntrustedRef))) {
return nil
}
// https://docs.github.com/en/actions/security-guides/encrypted-secrets#naming-your-secrets // https://docs.github.com/en/actions/security-guides/encrypted-secrets#naming-your-secrets
for _, step := range job.Steps { for _, step := range job.Steps {
if step == nil { if step == nil {
@ -237,13 +256,19 @@ func checkJobForUsedSecrets(job *actionlint.Job, path string,
return nil return nil
} }
func workflowUsesCodeCheckoutAndNoEnvironment(workflow *actionlint.Workflow) bool { func workflowUsesCodeCheckoutAndNoEnvironment(workflow *actionlint.Workflow,
triggers map[triggerName]bool) bool {
if workflow == nil { if workflow == nil {
return false return false
} }
_, usesPullRequest := triggers[triggerPullRequest]
_, usesPullRequestTarget := triggers[triggerPullRequestTarget]
for _, job := range workflow.Jobs { for _, job := range workflow.Jobs {
if jobUsesCodeCheckout(job) && chk, ref := jobUsesCodeCheckout(job)
if ((chk && usesPullRequest) ||
(chk && usesPullRequestTarget && strings.Contains(ref, checkoutUntrustedRef))) &&
!jobUsesEnvironment(job) { !jobUsesEnvironment(job) {
return true return true
} }
@ -251,10 +276,12 @@ func workflowUsesCodeCheckoutAndNoEnvironment(workflow *actionlint.Workflow) boo
return false return false
} }
func jobUsesCodeCheckout(job *actionlint.Job) bool { func jobUsesCodeCheckout(job *actionlint.Job) (bool, string) {
if job == nil { if job == nil {
return false return false, ""
} }
hasCheckout := false
for _, step := range job.Steps { for _, step := range job.Steps {
if step == nil || step.Exec == nil { if step == nil || step.Exec == nil {
continue continue
@ -265,10 +292,15 @@ func jobUsesCodeCheckout(job *actionlint.Job) bool {
continue continue
} }
if strings.Contains(e.Uses.Value, "actions/checkout") { if strings.Contains(e.Uses.Value, "actions/checkout") {
return true hasCheckout = true
ref, ok := e.Inputs["ref"]
if !ok || ref.Value == nil {
continue
}
return true, ref.Value.Value
} }
} }
return false return hasCheckout, ""
} }
func checkJobForUntrustedCodeCheckout(job *actionlint.Job, path string, func checkJobForUntrustedCodeCheckout(job *actionlint.Job, path string,
@ -296,7 +328,7 @@ func checkJobForUntrustedCodeCheckout(job *actionlint.Job, path string,
if !ok || ref.Value == nil { if !ok || ref.Value == nil {
continue continue
} }
if strings.Contains(ref.Value.Value, "github.event.pull_request") { if strings.Contains(ref.Value.Value, checkoutUntrustedRef) {
line := fileparser.GetLineNumber(step.Pos) line := fileparser.GetLineNumber(step.Pos)
dl.Warn(&checker.LogMessage{ dl.Warn(&checker.LogMessage{
Path: path, Path: path,
@ -336,10 +368,10 @@ func validateScriptInjection(workflow *actionlint.Workflow, path string,
return nil return nil
} }
func checkWorkflowSecretInEnv(workflow *actionlint.Workflow, path string, func checkWorkflowSecretInEnv(workflow *actionlint.Workflow, triggers map[triggerName]bool,
dl checker.DetailLogger, pdata *patternCbData) error { path string, dl checker.DetailLogger, pdata *patternCbData) error {
// We need code checkout and not environment rule protection. // We need code checkout and not environment rule protection.
if !workflowUsesCodeCheckoutAndNoEnvironment(workflow) { if !workflowUsesCodeCheckoutAndNoEnvironment(workflow, triggers) {
return nil return nil
} }

View File

@ -49,7 +49,7 @@ func TestGithubDangerousWorkflow(t *testing.T) {
expected: scut.TestReturn{ expected: scut.TestReturn{
Error: nil, Error: nil,
Score: checker.MinResultScore, Score: checker.MinResultScore,
NumberOfWarn: 1, NumberOfWarn: 2,
NumberOfInfo: 0, NumberOfInfo: 0,
NumberOfDebug: 0, NumberOfDebug: 0,
}, },
@ -208,6 +208,61 @@ func TestGithubDangerousWorkflow(t *testing.T) {
NumberOfDebug: 0, NumberOfDebug: 0,
}, },
}, },
{
name: "secret with environment protection pull request target",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-environment-prt.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MinResultConfidence,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "secret in env pull request target",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-secret-run-prt.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MinResultConfidence,
NumberOfWarn: 2,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "secret in env pull request target",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-prt.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MinResultConfidence,
NumberOfWarn: 4,
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",
expected: scut.TestReturn{
Error: nil,
Score: checker.MaxResultConfidence,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "secret in top env checkout no ref pull request target",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-secret-env-checkout-noref-prt.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MaxResultConfidence,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
} }
for _, tt := range tests { for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below tt := tt // Re-initializing variable so it is not changed while executing the closure below
@ -225,7 +280,6 @@ func TestGithubDangerousWorkflow(t *testing.T) {
} }
dl := scut.TestDetailLogger{} dl := scut.TestDetailLogger{}
p := strings.Replace(tt.filename, "./testdata/", "", 1) p := strings.Replace(tt.filename, "./testdata/", "", 1)
r := testValidateGitHubActionDangerousWorkflow(p, content, &dl) r := testValidateGitHubActionDangerousWorkflow(p, content, &dl)
if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl) { if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl) {
t.Fail() t.Fail()

View File

@ -0,0 +1,52 @@
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 }}"

View File

@ -0,0 +1,63 @@
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

View File

@ -0,0 +1,52 @@
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 }}"

View File

@ -0,0 +1,27 @@
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"

View File

@ -0,0 +1,14 @@
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) }}"

View File

@ -44,7 +44,7 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() {
expected := scut.TestReturn{ expected := scut.TestReturn{
Error: nil, Error: nil,
Score: checker.MinResultScore, Score: checker.MinResultScore,
NumberOfWarn: 1, NumberOfWarn: 2,
NumberOfInfo: 0, NumberOfInfo: 0,
NumberOfDebug: 0, NumberOfDebug: 0,
} }
@ -73,7 +73,7 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() {
expected := scut.TestReturn{ expected := scut.TestReturn{
Error: nil, Error: nil,
Score: checker.MinResultScore, Score: checker.MinResultScore,
NumberOfWarn: 1, NumberOfWarn: 2,
NumberOfInfo: 0, NumberOfInfo: 0,
NumberOfDebug: 0, NumberOfDebug: 0,
} }