Nixpkgs (better) branch protection rules? #121589
Labels
1.severity: security
Issues which raise a security issue, or PRs that fix one
Comments
primeos
added
the
1.severity: security
|
This issue has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/branch-protection-policies/6410/34 |
|
#249117 appears to have changed this somewhat. Possibly this issue can close. |
|
Also see the recently introduced GitHub Rulesets, which is a better version of the previous branch protection rules. In particular it allows anybody to view them, and they don't have any quirks about when they apply. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We currently have a lot of branches (86 to be precise, see
git ls-remote --heads https://github.com/NixOS/nixpkgs.git) and AFAIK we neither have a good naming scheme nor enough documentation on some of them. Events in the past also showed that our branch protection rules are lacking (force-pushes to production branches, accidentally created branches, etc.).Currently we e.g. have the following "interesting" branches:
Some of them should be for custom Hydra jobsets (but not all of them might still be in use) while others seem to be created on accident:IMO it would be a good idea to use branch protection rules with wildcards combined with a naming scheme (the naming scheme would be important for the wildcard rules but would also help understanding the purpose of a branch and identify "invalid" branches). Unfortunately the "protected branches" feature from GitHub is pretty limited (https://docs.github.com/en/github/administering-a-repository/about-protected-branches). E.g.:
I haven't thought much about this yet but it should be possible to do something like this (likely still incomplete):
master,release-*, andstaging-*: Allow@NixOS/nixpkgs-committersto push but no force pushes and no deletions (with "Require status checks to pass before merging" it should also be possible to prevent direct pushing but allow merging PRs)nixos-*: Only give@NixOS/channel-updatersaccess, no force pushes, and no deletionshydra-jobset-*: Create a @NixOS/ team for people who need to push to these branches and allow force pushes as well as deletions.*: Use something super strict as default for all other branches (no push access (e.g. assigning an empty team or org admins only), no force pushes, no deletions, etc.)Ideally the branch protection rules would also be documented somewhere as only org admins can see https://github.com/NixOS/nixpkgs/settings/branches.
Known limitation:
"By default, you cannot delete a protected branch. When you enable deletion of a protected branch, anyone with at least write permissions to the repository can delete the branch." - But this would luckily only affecthydra-jobset-*so far and from looking at the rest of the documentation the might've only forgotten to mention that "Restrict who can push to matching branches" also applies here because it should at least apply to "Allow force pushes" as well.Other relevant issues/PRs:
nixos-*branch which can unfortunately happen super easilymasteryears ago (an intentional test that was scripted to avoid damage) - I reported it on IRC and it shouldn't be possible anymore but since then I was too scared to test it again (I expected it wouldn't work when I first tried it...).Examples of branches that might've been created on accident (this list is likely incomplete and I've redacted the names as the purpose isn't to blame anyone)
The text was updated successfully, but these errors were encountered: