Skip to content

Commit

Permalink
chore: restrict the permissions granted to the workflows. (#136)
Browse files Browse the repository at this point in the history
This ensures untrusted code by external contributors cannot access
highly-priviledged GitHub tokens, even when triggered by a PR comment.

The touchstone actions are split into two parts, one "receive" action
which does the actual benchmarking and one "comment" action that
publishes the results. This is designed so that the receive may safely
run untrusted code by external contributors, while running with weaker
permission. Only the comment actions needs write permissions to the
repository.

The permission drop is normally performed automatically by GitHub when a
pull_request event is triggered from a fork repository. However, when
the workflow is triggered by the issue_comment, by default it runs with
a permissive token, with full access to the repository, even if PR comes
from a fork.

Because the library encourages one to set a condition on who can trigger
the workflow, the impact of this was limited. An owner/collaborator
would have had to issue the benchmark command before the untrusted code
can run. Nevertheless, reducing the permission set is safer as it
protects the repository even if the owner issues initiates the benchmark
before fully reviewing the PR code.

[GitHub has actually changed the default token permission][0] for the
issue_comment trigger recently, from write-all to contents read only,
similar to what this change tries to accomplish. However that is a
global per-repository setting that users may want to change, and it only
applies to repositories that were created after Februrary 2023.

Conversly, the comment workflow needs permission to set statuses and
comment on issues. Whether or not the workflow has the permission by
default depends on the aforementioned repo-wide setting, and thus on the
date the repo was created. By being explicit about it in the workflow
definition we can make sure it is reliably granted. This also reduces
the extent of the write permissions the workflow could have gotten by
default, which is always a good thing even if the workflow is relatively
simple.

[0]: https://github.blog/changelog/2023-02-02-github-actions-updating-the-default-github_token-permissions-to-read-only/
  • Loading branch information
plietar committed Jun 9, 2024
1 parent 1486f44 commit cef300d
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 0 deletions.
5 changes: 5 additions & 0 deletions inst/touchstone-comment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ on:
types:
- completed

permissions:
contents: read
statuses: write
pull-requests: write

jobs:
upload:
runs-on: ubuntu-latest
Expand Down
3 changes: 3 additions & 0 deletions inst/touchstone-receive.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ concurrency:

on: #- trigger

permissions:
contents: read

jobs:
prepare:
runs-on: ubuntu-latest #- ward
Expand Down
3 changes: 3 additions & 0 deletions tests/testthat/_snaps/use/receive_all.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ on:
issue_comment:
types: ['created', 'edited']

permissions:
contents: read

jobs:
prepare:
runs-on: ubuntu-latest
Expand Down
3 changes: 3 additions & 0 deletions tests/testthat/_snaps/use/receive_command.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ on:
issue_comment:
types: ['created', 'edited']

permissions:
contents: read

jobs:
prepare:
runs-on: ubuntu-latest
Expand Down
3 changes: 3 additions & 0 deletions tests/testthat/_snaps/use/receive_default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ concurrency:
on:
pull_request:

permissions:
contents: read

jobs:
prepare:
runs-on: ubuntu-latest
Expand Down
3 changes: 3 additions & 0 deletions tests/testthat/_snaps/use/receive_limit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ concurrency:
on:
pull_request:

permissions:
contents: read

jobs:
prepare:
runs-on: ubuntu-latest
Expand Down

0 comments on commit cef300d

Please sign in to comment.