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

BUG: Branch-Protection scoring #3250

Closed
gabibguti opened this issue Jul 7, 2023 · 2 comments · Fixed by #3251
Closed

BUG: Branch-Protection scoring #3250

gabibguti opened this issue Jul 7, 2023 · 2 comments · Fixed by #3251
Labels

Comments

@gabibguti
Copy link
Contributor

gabibguti commented Jul 7, 2023

Describe the bug
The Branch-Protection score should count 1 point for each setting you enable, of course, considering the Tiers. However, the scoring is wrong for the following cases:

  • Using non-admin token, when you reach tier 3 you have 8/10 and should be 7/10.
  • Using non-admin token, when you reach tier 3 you have 8/10. When enabling "Require review from code owners" you still get an 8/10.
  • Using admin token, when you reach tier 1 you have 4/10 and should be 3/10.
  • Using admin token, when you reach tier 2 and enable "Require at least 1 reviewer for approval before merging " you have 5/10. When enabling "For administrators: Require branch to be up to date before merging" you still get a 5/10.
  • Using admin token, when you reach tier 3 you have 8/10 and should be 7/10.
  • Using admin token, when you reach tier 3 you have 8/10. When enabling "Require at least 2 reviewers for approval before merging" you still get an 8/10.
  • Using admin token, when you reach tier 3 you have 8/10. When enabling "Require review from code owners" you still get an 8/10.

Reproduction steps

To reproduce, get an non-admin token, enable the branch protection settings until you reach Tier 2, run Scorecard with non-admin token and then do it again until you reach Tier 3 and compare the scores.
Same for admin-token, but comparing the Tiers 1, 2 and 3 scores.

Expected behavior

The Branch-Protection score should count 1 point for each setting you enable but considering the Tiers.

Additional context
#2772 (comment)

@gabibguti
Copy link
Contributor Author

Spencer confirmed me that in Tier 3 you should gain 2 points for enabling one setting, "Require status checks to pass before merging > Status Checks". So, updating the wrong scoring cases:

  • Using admin token, when you reach tier 1 you have 4/10 and should be 3/10.
  • Using admin token, when you reach tier 2 and enable "Require at least 1 reviewer for approval before merging " you have 5/10. When enabling "For administrators: Require branch to be up to date before merging" you still get a 5/10.

@gabibguti
Copy link
Contributor Author

The bug about GitHub responding wrongly if "branches are required to be up to date before merge" is already reported https://github.com/orgs/community/discussions/59471

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants