Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Set permissions for GitHub actions #901

Merged
merged 4 commits into from
Dec 5, 2022

Conversation

neilnaveen
Copy link
Contributor

 Restrict the GitHub token permissions only to the required ones; this way, even if the attackers will succeed in compromising your workflow, they won’t be able to do much.

- Included permissions for the action. https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions

https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#permissions

https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs

[Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/)

Signed-off-by: neilnaveen <42328488+neilnaveen@users.noreply.github.com>
navrkald added a commit to navrkald/twine that referenced this pull request Aug 16, 2022
Comment on lines +17 to +19
permissions:
contents: read

Copy link
Member

Choose a reason for hiding this comment

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

The default permissions for our workflows is already 'read', so I don't believe this is necessary.

Copy link
Member

Choose a reason for hiding this comment

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

@di this also sets all the other (non-content) scopes to null restricting them even further. It's one of the latest best practices security-wise.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this isn't widely-adopted in the pypa org; only pipenv seems to do it for their CI workflow. @neilnaveen out of curiosity, why did you choose this to add this to Twine?

Copy link
Member

Choose a reason for hiding this comment

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

@bhrutledge I saw it being adopted by attrs and I know that Tidelift is starting to push for openSSF, so I foresee it appearing on our radars more often over time. I'm slowly adopting this too.

Comment on lines +19 to +21
permissions:
contents: read

Copy link
Member

Choose a reason for hiding this comment

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

Ditto here.

Copy link
Contributor

@bhrutledge bhrutledge left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution; not quite sure about merging it yet.

.github/workflows/codeql-analysis.yml Outdated Show resolved Hide resolved
Comment on lines +17 to +19
permissions:
contents: read

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this isn't widely-adopted in the pypa org; only pipenv seems to do it for their CI workflow. @neilnaveen out of curiosity, why did you choose this to add this to Twine?

@bhrutledge bhrutledge enabled auto-merge (squash) December 5, 2022 17:57
@bhrutledge bhrutledge merged commit 815853f into pypa:main Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants