From 3e546f76012c2e52064db8b7f563980d783ad7db Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Fri, 8 Dec 2023 10:52:27 +1300 Subject: [PATCH] ci: explicitly define permissions for each job and as the first property in each job (#513) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In my mind there's a number of advantages to this: * you can easily see all the permissions a job requires * the block always exists making it easier to pick up on when you need to add new permissions * because of this I've also put the `permissions` property as the first one in every job for consistency * we're leading by example, instead of doing something we're telling people not to do (using a global permission) Note that our reusable workflows do still have a workflow-level `permissions` property - I've kept this because reusable workflows can only _narrow_ permissions meaning the caller job should still always have a `permissions` block, and so in my mind for reusable workflows `permissions` is more "here's what you need to copy into the caller job" which would be more work if we were to have a reusable workflow with multiple jobs that had different permissions as you'd have to manually do the work of merging. On the other hand they arguably muddy our "don't use global permissions" message so happy to refactor them to be per-job as well if people don't think it'll be a problem. Also note that while we can and should configure the default permissions at the repository level, we still want to explicitly set the `permissions` in each workflow because we can't be sure the permissions are tightened at the repository level and this only costs us a couple of bytes in lines of code so there's no real downside 🤷 --- .github/workflows/ci.yml | 18 ++++++---- .github/workflows/codeql-analysis.yml | 9 +++-- .../github_actions_ci/workflows/ci.yml.tt | 36 ++++++++++--------- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8c3f6437..a2b5ce6a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,16 +10,14 @@ on: # * At 05:00 UTC every Monday, run the latest commit on the default or base branch - cron: '0 5 * * MON' -# Restrict jobs in this workflow to only be allowed to read this repo by default. -# -# If you are wanting to introduce a job/tool that requires more permissions (such -# as posting comments or commits to the repository), then you should grant just -# that job the necessarily permissions by giving it a dedicated `permissions` block. -permissions: - contents: read # to fetch code (actions/checkout) +# Restrict jobs in this workflow to have no permissions by default; permissions +# should be granted per job as needed using a dedicated `permissions` block +permissions: {} jobs: audit_dependencies: + permissions: + contents: read runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -28,6 +26,8 @@ jobs: - name: Audit dependencies for security vulnerabilities uses: g-rath/check-with-osv-detector@main test: + permissions: + contents: read runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -41,6 +41,8 @@ jobs: - run: yarn run format-check rubocop: + permissions: + contents: read runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -56,6 +58,8 @@ jobs: run: bundle exec rubocop test_generated_apps: + permissions: + contents: read runs-on: ubuntu-latest strategy: # don't stop all variants if one of them fails (we usually want to know diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 3451df64..c6cf0cf5 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -14,15 +14,20 @@ on: schedule: - cron: '0 0 * * 0' +# Restrict jobs in this workflow to have no permissions by default; permissions +# should be granted per job as needed using a dedicated `permissions` block +permissions: {} + jobs: analyze: - name: Analyze - runs-on: ubuntu-latest permissions: actions: read contents: read security-events: write + name: Analyze + runs-on: ubuntu-latest + strategy: fail-fast: false matrix: diff --git a/variants/github_actions_ci/workflows/ci.yml.tt b/variants/github_actions_ci/workflows/ci.yml.tt index aaaa92aa..de3ca46c 100644 --- a/variants/github_actions_ci/workflows/ci.yml.tt +++ b/variants/github_actions_ci/workflows/ci.yml.tt @@ -19,16 +19,14 @@ env: SIDEKIQ_WEB_PASSWORD: password <%- end -%> -# Restrict jobs in this workflow to only be allowed to read this repo by default. -# -# If you are wanting to introduce a job/tool that requires more permissions (such -# as posting comments or commits to the repository), then you should grant just -# that job the necessarily permissions by giving it a dedicated `permissions` block. -permissions: - contents: read # to fetch code (actions/checkout) +# Restrict jobs in this workflow to have no permissions by default; permissions +# should be granted per job as needed using a dedicated `permissions` block +permissions: {} jobs: audit_dependencies: + permissions: + contents: read runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -37,6 +35,8 @@ jobs: - name: Audit dependencies for security vulnerabilities uses: g-rath/check-with-osv-detector@main js_based_checks: + permissions: + contents: read runs-on: ubuntu-latest timeout-minutes: 15 steps: @@ -54,6 +54,8 @@ jobs: - run: yarn run js-lint - run: yarn run format-check ruby_based_checks: + permissions: + contents: read runs-on: ubuntu-latest timeout-minutes: 20 services: @@ -127,15 +129,15 @@ jobs: # ######################################################################### # # deploy_to_ec2_staging: + # permissions: + # id-token: write # to use OIDC (aws-actions/configure-aws-credentials) + # contents: read # to fetch code (actions/checkout) # if: github.event_name == 'push' && github.ref == 'refs/heads/main' # needs: # - audit_dependencies # - ruby_based_checks # - js_based_checks # uses: ./.github/workflows/deploy_to_ec2.yml - # permissions: - # id-token: write # to use OIDC (aws-actions/configure-aws-credentials) - # contents: read # to fetch code (actions/checkout) # with: # environment: staging # environment_url: '<%= "https://#{TEMPLATE_CONFIG.staging_hostname}" %>' @@ -145,15 +147,15 @@ jobs: # ssh_private_key: ${{ secrets.STAGING_SSH_PRIVATE_KEY }} # slack_webhook: ${{ secrets.SLACK_WEBHOOK }} # deploy_to_ec2_production: + # permissions: + # id-token: write # to use OIDC (aws-actions/configure-aws-credentials) + # contents: read # to fetch code (actions/checkout) # if: github.event_name == 'push' && github.ref == 'refs/heads/production' # needs: # - audit_dependencies # - ruby_based_checks # - js_based_checks # uses: ./.github/workflows/deploy_to_ec2.yml - # permissions: - # id-token: write # to use OIDC (aws-actions/configure-aws-credentials) - # contents: read # to fetch code (actions/checkout) # with: # environment: production # environment_url: '<%= "https://#{TEMPLATE_CONFIG.production_hostname}" %>' @@ -168,14 +170,14 @@ jobs: # ######################################################################### # # deploy_to_heroku_staging: + # permissions: + # contents: read # to fetch code (actions/checkout) # if: github.event_name == 'push' && github.ref == 'refs/heads/main' # needs: # - audit_dependencies # - ruby_based_checks # - js_based_checks # uses: ./.github/workflows/deploy_to_heroku.yml - # permissions: - # contents: read # to fetch code (actions/checkout) # with: # environment: staging # environment_url: '<%= "https://#{TEMPLATE_CONFIG.staging_hostname}" %>' @@ -185,14 +187,14 @@ jobs: # heroku_app_name: ${{ secrets.HEROKU_APP_NAME_STAGING }} # slack_webhook: ${{ secrets.SLACK_WEBHOOK }} # deploy_to_heroku_production: + # permissions: + # contents: read # to fetch code (actions/checkout) # if: github.event_name == 'push' && github.ref == 'refs/heads/production' # needs: # - audit_dependencies # - ruby_based_checks # - js_based_checks # uses: ./.github/workflows/deploy_to_heroku.yml - # permissions: - # contents: read # to fetch code (actions/checkout) # with: # environment: production # environment_url: '<%= "https://#{TEMPLATE_CONFIG.production_hostname}" %>'