Skip to content

Adding GitHub content protection rule to CI#2052

Open
john-science wants to merge 1 commit intohgrecco:masterfrom
john-science:protect_ci
Open

Adding GitHub content protection rule to CI#2052
john-science wants to merge 1 commit intohgrecco:masterfrom
john-science:protect_ci

Conversation

@john-science
Copy link
Copy Markdown

@john-science john-science commented Sep 4, 2024

What is the change?

I have constrained the permissions of the GitHub Actions in this repo.

The permissions I set are maximally constrained so that a bad actor can't use a PR into your repo to do various things like: commit to your main branch, modify a GH "Discussion" in your repo, or author an official release.

A Little Background

GitHub added this security feature in 2021.

This is not a complete solution in itself, of course. But it pairs really well with the setting pint has where Pint team members have to "approve a workflow" from a non-team member, to run CI:

Screenshot from 2024-09-04 07-31-41

For a shorter discussion of the topic than the full GitHub docs, see this blog post.

The Checklist

  • Closes # (insert issue number)
  • Executed pre-commit run --all-files with no errors
  • The change is fully covered by automated unit tests
  • Documented in docs/ as appropriate
  • Added an entry to the CHANGES file

@dopplershift
Copy link
Copy Markdown
Contributor

I'll just note that an alternative is to set default permissions on the whole repo in the settings:

image

@john-science
Copy link
Copy Markdown
Author

john-science commented Sep 4, 2024

I'll just note that an alternative is to set default permissions on the whole repo in the settings:

Sure!

As it happens, I can tell that is not being done for this repo, so I am just trying to help.

There is a GitHub Workflow for this repo (lint-autoupdate) that does actually need the write permissions. So, if you set the repo-global permissions, that CI will fail and you will need to make a change to that YAML file to allow it to run.

So, either approach works. But either way you need a PR.

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.

2 participants