Skip to content

Commit

Permalink
feat: update permissions in distributed template
Browse files Browse the repository at this point in the history
  • Loading branch information
@galargh
galargh committed Nov 30, 2023
1 parent f35932b commit 31914bf
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions templates/.github/workflows/js-test-and-release.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ on:

permissions:
contents: write
id-token: write
packages: write
pull-requests: write

concurrency:
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event_name == 'push' && github.sha || github.ref }}
Expand Down

7 comments on commit 31914bf

@achingbrain
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgive me, but does this not give write permissions to every job in the build?

They get run on PRs as well as main/master - is it possible to only grant write perms when running on the default branch?

@galargh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Permissions for PRs initiated from forks are capped at read as per https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs#changing-the-permissions-in-a-forked-repository

@achingbrain
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I know, but they are not for PRs from branches in the same repo, right?

@galargh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's true. One way to differentiate them would be to have to workflow files. One that's triggered only on pull_request and the other only triggered on push. Both with different permissions.

However, if someone has push access to repo, then they can change permissions anyway. Do you have any specific scenario in mind that we'd like to prevent?

BTW, this is will not be distributed everywhere automatically until we decide to (not even with the next uCI release) so we can take our time figuring it out.

@achingbrain
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone submits a PR from a branch in the repo that then creates a release.

I can turn on branch protection for main, and add rules to prevent a PR being merged without approval, or only with approval by certain people, but it they can just rewrite the actions file to make a release (whether on purpose or by accident) it sidesteps these other protections.

@galargh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the reusable workflow, the release job is hidden behind a check for trigger. So that would prevent accidental release.

And if someone has write access to the repo, then they would be able to rewrite the workflow file or add new ones.

@achingbrain
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A fair point, I guess there's no way round it.

Please sign in to comment.