From 0f30f4eec723dfb4ed02007dbe32a0044363b9f2 Mon Sep 17 00:00:00 2001 From: laurentsimon <64505099+laurentsimon@users.noreply.github.com> Date: Wed, 11 May 2022 18:41:37 -0700 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Make=20permission=20check=20aware?= =?UTF-8?q?=20of=20GH=20Pages=20Action=20(#1902)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * update * update * update --- checks/permissions.go | 17 ++++- checks/permissions_test.go | 63 +++++++++++-------- .../github-workflow-permissions-gh-pages.yaml | 26 ++++++++ e2e/permissions_test.go | 6 +- 4 files changed, 83 insertions(+), 29 deletions(-) create mode 100644 checks/testdata/.github/workflows/github-workflow-permissions-gh-pages.yaml diff --git a/checks/permissions.go b/checks/permissions.go index 2e999219..f1ee6729 100644 --- a/checks/permissions.go +++ b/checks/permissions.go @@ -566,7 +566,22 @@ func requiresPackagesPermissions(workflow *actionlint.Workflow, fp string, dl ch // requiresContentsPermissions returns true if the workflow requires the `contents: write` permission. func requiresContentsPermissions(workflow *actionlint.Workflow, fp string, dl checker.DetailLogger) bool { - return isReleasingWorkflow(workflow, fp, dl) + return isReleasingWorkflow(workflow, fp, dl) || isGitHubPagesDeploymentWorkflow(workflow, fp, dl) +} + +// isGitHubPagesDeploymentWorkflow returns true if the workflow involves pushing static pages to GitHub pages. +func isGitHubPagesDeploymentWorkflow(workflow *actionlint.Workflow, fp string, dl checker.DetailLogger) bool { + jobMatchers := []fileparser.JobMatcher{ + { + Steps: []*fileparser.JobMatcherStep{ + { + Uses: "peaceiris/actions-gh-pages", + }, + }, + LogText: "candidate GitHub page deployment workflow using peaceiris/actions-gh-pages", + }, + } + return fileparser.AnyJobsMatch(workflow, jobMatchers, fp, dl, "not a GitHub Pages deployment workflow") } // isReleasingWorkflow returns true if the workflow involves creating a release on GitHub. diff --git a/checks/permissions_test.go b/checks/permissions_test.go index 8377f7c2..3ba1f4f9 100644 --- a/checks/permissions_test.go +++ b/checks/permissions_test.go @@ -63,7 +63,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 1, - NumberOfDebug: 4, + NumberOfDebug: 5, }, }, { @@ -74,7 +74,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore - 1, NumberOfWarn: 1, NumberOfInfo: 1, - NumberOfDebug: 4, + NumberOfDebug: 5, }, }, { @@ -85,7 +85,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 3, NumberOfInfo: 2, - NumberOfDebug: 4, + NumberOfDebug: 5, }, }, { @@ -96,7 +96,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 2, - NumberOfDebug: 4, + NumberOfDebug: 5, }, }, { @@ -107,7 +107,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 1, - NumberOfDebug: 4, + NumberOfDebug: 5, }, }, { @@ -118,7 +118,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 1, - NumberOfDebug: 5, + NumberOfDebug: 6, }, }, { @@ -129,7 +129,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 0, - NumberOfDebug: 5, + NumberOfDebug: 6, }, }, { @@ -140,7 +140,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 1, - NumberOfDebug: 5, + NumberOfDebug: 6, }, }, { @@ -151,7 +151,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 0, - NumberOfDebug: 5, + NumberOfDebug: 6, }, }, { @@ -162,7 +162,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 1, - NumberOfDebug: 6, + NumberOfDebug: 7, }, }, { @@ -173,7 +173,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 10, - NumberOfDebug: 5, + NumberOfDebug: 6, }, }, { @@ -184,7 +184,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 10, - NumberOfDebug: 5, + NumberOfDebug: 6, }, }, { @@ -195,7 +195,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 1, - NumberOfDebug: 5, + NumberOfDebug: 6, }, }, { @@ -206,7 +206,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore - 1, NumberOfWarn: 2, NumberOfInfo: 2, - NumberOfDebug: 6, + NumberOfDebug: 7, }, }, { @@ -217,7 +217,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore - 2, NumberOfWarn: 2, NumberOfInfo: 3, - NumberOfDebug: 5, + NumberOfDebug: 6, }, }, { @@ -228,7 +228,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 2, - NumberOfDebug: 5, + NumberOfDebug: 6, }, }, { @@ -239,7 +239,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 2, - NumberOfDebug: 5, + NumberOfDebug: 6, }, }, { @@ -250,7 +250,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 1, - NumberOfDebug: 5, + NumberOfDebug: 6, }, }, { @@ -272,7 +272,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 2, - NumberOfDebug: 3, + NumberOfDebug: 4, }, }, { @@ -294,7 +294,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 2, - NumberOfDebug: 4, + NumberOfDebug: 5, }, }, { @@ -305,7 +305,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: 9, NumberOfWarn: 1, NumberOfInfo: 3, - NumberOfDebug: 4, + NumberOfDebug: 5, }, }, { @@ -316,7 +316,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore - 1, NumberOfWarn: 1, NumberOfInfo: 1, - NumberOfDebug: 4, + NumberOfDebug: 5, }, }, { @@ -330,7 +330,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore - 1, NumberOfWarn: 1, NumberOfInfo: 2, - NumberOfDebug: 9, + NumberOfDebug: 11, }, }, { @@ -344,7 +344,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 2, NumberOfInfo: 1, - NumberOfDebug: 9, + NumberOfDebug: 11, }, }, { @@ -358,7 +358,20 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 1, - NumberOfDebug: 10, + NumberOfDebug: 12, + }, + }, + { + name: "read permission with GitHub pages write", + filenames: []string{ + "./testdata/.github/workflows/github-workflow-permissions-gh-pages.yaml", + }, + expected: scut.TestReturn{ + Error: nil, + Score: checker.MaxResultScore, + NumberOfWarn: 0, + NumberOfInfo: 2, + NumberOfDebug: 5, }, }, } diff --git a/checks/testdata/.github/workflows/github-workflow-permissions-gh-pages.yaml b/checks/testdata/.github/workflows/github-workflow-permissions-gh-pages.yaml new file mode 100644 index 00000000..95b9069f --- /dev/null +++ b/checks/testdata/.github/workflows/github-workflow-permissions-gh-pages.yaml @@ -0,0 +1,26 @@ +# Copyright 2021 Security Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: write-and-read workflow +on: [push] + +permissions: read-all + +jobs: + Explore-GitHub-Actions: + runs-on: ubuntu-latest + permissions: + contents: write + steps: + - run: echo "write-and-read workflow" + - uses: peaceiris/actions-gh-pages@v3 \ No newline at end of file diff --git a/e2e/permissions_test.go b/e2e/permissions_test.go index 806914a0..0a4c2a51 100644 --- a/e2e/permissions_test.go +++ b/e2e/permissions_test.go @@ -50,7 +50,7 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 2, - NumberOfDebug: 5, + NumberOfDebug: 6, } result := checks.TokenPermissions(&req) // New version. @@ -75,7 +75,7 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 2, - NumberOfDebug: 5, + NumberOfDebug: 6, } result := checks.TokenPermissions(&req) // New version. @@ -112,7 +112,7 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 2, - NumberOfDebug: 5, + NumberOfDebug: 6, } result := checks.TokenPermissions(&req) // New version.