Skip to content

Commit

Permalink
ci: explicitly define permissions for each job and as the first prope…
Browse files Browse the repository at this point in the history
…rty in each job (#513)

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 🤷
  • Loading branch information
G-Rath authored Dec 7, 2023
1 parent 68be65c commit 3e546f7
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 26 deletions.
18 changes: 11 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -41,6 +41,8 @@ jobs:
- run: yarn run format-check

rubocop:
permissions:
contents: read
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
Expand All @@ -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
Expand Down
9 changes: 7 additions & 2 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
36 changes: 19 additions & 17 deletions variants/github_actions_ci/workflows/ci.yml.tt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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}" %>'
Expand All @@ -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}" %>'
Expand All @@ -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}" %>'
Expand All @@ -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}" %>'
Expand Down

0 comments on commit 3e546f7

Please sign in to comment.