ci: Add GitHub token permissions for workflows #74331

varunsh-coder · 2022-07-02T18:37:57Z

Proposed change

This PR adds minimum token permissions for the GITHUB_TOKEN using https://github.com/step-security/secure-workflows.

GitHub recommends defining minimum GITHUB_TOKEN permissions for securing GitHub Actions workflows

https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/
https://docs.github.com/en/actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token
The Open Source Security Foundation (OpenSSF) Scorecards treats not setting token permissions as a high-risk issue

This project is part of the top 100 critical projects as per OpenSSF (https://github.com/ossf/wg-securing-critical-projects), so fixing the token permissions to improve security.

Signed-off-by: Varun Sharma varunsh@stepsecurity.io

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

Signed-off-by: Varun Sharma <varunsh@stepsecurity.io>

homeassistant · 2022-07-02T18:37:59Z

Hi @varunsh-coder,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

Signed-off-by: Varun Sharma <varunsh@stepsecurity.io>

ludeeus · 2022-07-06T07:55:52Z

.github/workflows/builder.yml

+permissions:
+  contents: read
+


We should set read on repository settings instead

Hi @ludeeus, I think it would be good to set read on repo settings in addition to this change. This way, since the permissions are set explicitly in the workflow, even if one forks this repo, the workflow will still be secure in the forked repo. Please let me know your thoughts.

I think that if that is required to secure forks, GitHub has an issue that we should take up with them. Basically, you are saying that the repository settings don't work?

I think that if that is required to secure forks, GitHub has an issue that we should take up with them. Basically, you are saying that the repository settings don't work?

Setting the token permission using repository setting will work for this repository. But if the repository is forked or if someone copies the same workflow and uses it in a different repository, the repository settings will not go along. This is why it is a best practice to keep the permissions along with the workflow file.

So, you are saying the repository permissions settings are not working / causing a security issue?

Since you guys are on a mission fixing / improving security for open source project, I assume you have reported/contacted GitHub about this as well? I am interested in their response actually?

So, you are saying the repository permissions settings are not working / causing a security issue?

Since you guys are on a mission fixing / improving security for open source project, I assume you have reported/contacted GitHub about this as well? I am interested in their response actually?

Hi @frenck I have not reported/ contacted GitHub about this, since they offer multiple solutions (repo setting + workflow file setting) and developers can choose to use both of them to cover all scenarios. I did work with GitHub to set token permissions in starter workflows. For starter workflow, permissions are required to be set in the workflow file. This is the issue for it: actions/starter-workflows#1299

For your repo, if you prefer to just use repo settings for cases where only contents: read is needed, I can remove the explicit workflow level permissions. The only downside is that forks of this repo will not automatically be secure, but the maintainers of those forks can implement the fix on their own. Please let me know. Thanks!

Hi @frenck, please let me know about this. Thanks!

ludeeus · 2022-07-06T07:56:19Z

.github/workflows/builder.yml

+    permissions:
+      actions: read # for dawidd6/action-download-artifact to query and download artifacts
+      contents: read # for actions/checkout to fetch code
+      pull-requests: read # for dawidd6/action-download-artifact to query commit hash


These would not do anything

To clarify, do you mean these would not do anything because these are read permissions and this is a public repo, so the token will anyways have read access? I think that makes sense.

These changes are automated using https://github.com/step-security/secure-workflows, and that program just takes a workflow as input and fixes it. It does not have context on whether the workflow is in public repo or private repo. For a private repo, these permissions would need to be set explicitly. If you prefer that I remove these from the PR, I can do that. Please let me know.

These changes are automated using step-security/secure-workflows, and that program just takes a workflow as input and fixes it. It does not have context on whether the workflow is in public repo or private repo.

That is why automated tools for things like these are not really helpful and generate tons of noise.

Yes, please address the review comment.

ludeeus · 2022-07-06T07:56:34Z

.github/workflows/ci.yaml

+permissions:
+  contents: read
+


We should set read on repository settings instead

ludeeus · 2022-07-06T07:56:41Z

.github/workflows/ci.yaml

+    permissions:
+      contents: read # for dorny/paths-filter to fetch a list of changed files
+      pull-requests: read # for dorny/paths-filter to read pull requests


These would not do anything

ludeeus · 2022-07-06T07:56:49Z

.github/workflows/stale.yml

+permissions:
+  contents: read
+


We should set read on repository settings instead

ludeeus · 2022-07-06T07:56:55Z

.github/workflows/translations.yaml

+permissions:
+  contents: read
+


We should set read on repository settings instead

ludeeus · 2022-07-06T07:57:02Z

.github/workflows/wheels.yml

+permissions:
+  contents: read
+


We should set read on repository settings instead

Signed-off-by: Varun Sharma <varunsh@stepsecurity.io>

github-actions · 2022-10-17T17:19:50Z

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

ci: Add GitHub token permissions for workflows

5484237

Signed-off-by: Varun Sharma <varunsh@stepsecurity.io>

homeassistant added the cla-needed label Jul 2, 2022

probot-home-assistant bot added code-quality small-pr PRs with less than 30 lines. labels Jul 2, 2022

varunsh-coder mentioned this pull request Jul 2, 2022

Fix token permissions for 100 critical open-source projects step-security/secure-repo#462

Open

homeassistant added cla-signed and removed cla-needed labels Jul 2, 2022

Fix formatting to comply with prettier

85f430d

Signed-off-by: Varun Sharma <varunsh@stepsecurity.io>

ludeeus requested changes Jul 6, 2022

View reviewed changes

Remove explicit job level read perms

d600ba7

Signed-off-by: Varun Sharma <varunsh@stepsecurity.io>

github-actions bot added the stale label Oct 17, 2022

github-actions bot closed this Oct 24, 2022

github-actions bot locked and limited conversation to collaborators Oct 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: Add GitHub token permissions for workflows #74331

ci: Add GitHub token permissions for workflows #74331

varunsh-coder commented Jul 2, 2022

homeassistant commented Jul 2, 2022

ludeeus Jul 6, 2022

varunsh-coder Jul 6, 2022

frenck Jul 7, 2022

varunsh-coder Jul 7, 2022

frenck Jul 8, 2022

varunsh-coder Jul 9, 2022

varunsh-coder Jul 19, 2022

ludeeus Jul 6, 2022

varunsh-coder Jul 6, 2022

frenck Jul 7, 2022

varunsh-coder Jul 7, 2022

ludeeus Jul 6, 2022

ludeeus Jul 6, 2022

varunsh-coder Jul 7, 2022

ludeeus Jul 6, 2022

ludeeus Jul 6, 2022

ludeeus Jul 6, 2022

github-actions bot commented Oct 17, 2022

ci: Add GitHub token permissions for workflows #74331

ci: Add GitHub token permissions for workflows #74331

Conversation

varunsh-coder commented Jul 2, 2022

Proposed change

Type of change

Checklist

homeassistant commented Jul 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 17, 2022