gh-Cyan4973/xxHash

Watch

Fork 0

[security] Set minimum permissions for workflows #806

New issue

Closed

opened 2023-02-28 05:31:46 -06:00 by gabibguti · 4 comments

gabibguti commented

2023-02-28 05:31:46 -06:00

(Migrated from github.com)

This is considered good-practice and recommended by GitHub itself and by other security tools, such as Scorecards and StepSecurity.

Additional Context

I'm Gabriela and I work on behalf of Google and the OpenSSF suggesting supply-chain security changes :)

Setting minimum permissions to your workflows is important to keep your repository safe against supply-chain attacks. By default, GitHub gives higher permissions then necessary to the GITHUB_TOKEN generated for workflows. I see `ci.yml` does not need many permissions to run the tests. The actions used by `ci.yml`, `msys2/setup-msys2` and `actions/checkout`, only need access to read the repository code. Thus, we can reduce the permissions to `contents: read` and should be good to go. This change should not impact any tests for any environments :) If you agree with the changes, I can open a PR! This is considered good-practice and recommended by [GitHub itself](https://docs.github.com/en/actions/security-guides/automatic-token-authentication) and by other security tools, such as [Scorecards](https://github.com/ossf/scorecard) and [StepSecurity](https://github.com/step-security). ##### Additional Context I'm Gabriela and I work on behalf of Google and the OpenSSF suggesting supply-chain security changes :)

👍 1

t-mat commented

2023-02-28 08:18:25 -06:00

(Migrated from github.com)

Hi @gabibguti,

Thank you for letting us know. I suppose that you're suggesting the following actions to us.

Set "Read repository contents and packages permissions" to the "Workflow permission" in the Settings tab of the repo.
Reference: Setting the permissions of the GITHUB_TOKEN for your repository - GitHub Docs
As you described, add the following lines to ci.yml. Similar (same?) to above settings, but since this form is written in code, it may propagate to other forks.
Reference: Configuring the default GITHUB_TOKEN permissions

        on: [push, pull_request]

+       permissions:
+         contents: read

        jobs:

I think we need to do both of them. Because 1. helps all branches which don't have permissions: in ci.yml.
But 2. for dev branch is also good because it's explicit and may help future improvements or suggestions from other contributors.

Please let us know if there are mistakes or misunderstands.

Hi @gabibguti, Thank you for letting us know. I suppose that you're suggesting the following actions to us. 1. Set "Read repository contents and packages permissions" to the "Workflow permission" in the [Settings tab of the repo](https://github.com/Cyan4973/xxHash/settings/actions). Reference: [Setting the permissions of the GITHUB_TOKEN for your repository - GitHub Docs](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#setting-the-permissions-of-the-github_token-for-your-repository) 2. As you described, add the following lines to [`ci.yml`](https://github.com/Cyan4973/xxHash/blob/2b328a10983d232364ceda15df1d33531b5f0eb5/.github/workflows/ci.yml#L55). Similar (same?) to above settings, but since this form is written in code, it may propagate to other forks. Reference: [Configuring the default GITHUB_TOKEN permissions](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#configuring-the-default-github_token-permissions) ```YAML on: [push, pull_request] + permissions: + contents: read jobs: ``` I think we need to do both of them. Because 1. helps all branches which don't have `permissions:` in `ci.yml`. But 2. for `dev` branch is also good because it's explicit and may help future improvements or suggestions from other contributors. Please let us know if there are mistakes or misunderstands.

👍 1

Cyan4973 commented

2023-02-28 09:46:29 -06:00

(Migrated from github.com)

Thanks for information !

As suggested by @t-mat in (1),
I updated "Workflow permissions" to "Read repository content" only.
I think none of our CI tests require more than that, so it should be okay.

A (2) follow up is also possible.

Thanks for information ! As suggested by @t-mat in (1), I updated "Workflow permissions" to "Read repository content" only. I think none of our CI tests require more than that, so it should be okay. A (2) follow up is also possible.

🚀 1

gabibguti commented

2023-03-01 13:57:55 -06:00

(Migrated from github.com)

@t-mat Yes! Perfect. It's exactly as you explained.
@t-mat @Cyan4973 Let me know if you'd like me to open a PR for (2) :)

@t-mat Yes! Perfect. It's exactly as you explained. @t-mat @Cyan4973 Let me know if you'd like me to open a PR for (2) :)

Cyan4973 commented

2023-03-09 11:14:55 -06:00

(Migrated from github.com)

Permissions restricted to content: read in #807 . Thanks for the advise !

Permissions restricted to `content: read` in #807 . Thanks for the advise !

👍 1 🚀 1

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Cyan4973/xxHash#806

No description provided.

Rows
Columns