Skip to content

GitHub Workflows security hardening#14388

Merged
Jellyfrog merged 3 commits intolibrenms:masterfrom
sashashura:patch-1
Sep 27, 2022
Merged

GitHub Workflows security hardening#14388
Jellyfrog merged 3 commits intolibrenms:masterfrom
sashashura:patch-1

Conversation

@sashashura
Copy link
Copy Markdown
Contributor

This PR adds explicit permissions section to workflows. This is a security best practice because by default workflows run with extended set of permissions (except from on: pull_request from external forks). By specifying any permission explicitly all others are set to none. By using the principle of least privilege the damage a compromised workflow can do (because of an injection or compromised third party tool or action) is restricted.
It is recommended to have most strict permissions on the top level and grant write permissions on job level case by case.

Signed-off-by: Alex <aleksandrosansan@gmail.com>
Signed-off-by: Alex <aleksandrosansan@gmail.com>
Signed-off-by: Alex <aleksandrosansan@gmail.com>
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Sep 25, 2022

CLA assistant check
All committers have signed the CLA.

super-linter:
permissions:
contents: read # to fetch code (actions/checkout)
checks: write # to mark the status of each individual linter run
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Phpstan also needs write permissions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have added checks: write according to the comment in super-linter action. I think phpstan is not an action and doesn't know how to call github api to make use of the permission, although GitHub should show the overall check for the job as a whole - failed or succeed. Please, let me know if you still think it is better to grant checks: write to the phpstan job.

@Jellyfrog Jellyfrog merged commit b1d25a9 into librenms:master Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants