ossf scorecard: Branch-Protection

[pjbgf]

Description:

This issue is to drive discussion around the ossf scorecard results for the Branch-Protection section. This is part of improving the project's security posture #653.

The Branch-Protection section of the ossf scorecard highlights the hardening recommendations for GitHub branches. Some of them may conflict with parts of our CI/CD automation, which may mean we won't be able to mitigate.

Result: branch protection is not maximal on development and all release branches
Details:

1. • Linear history disabled on branch 'main'
2. • Status checks for merging disabled on branch 'main'
3. • Stale review dismissal disabled on branch 'main'
4. • Number of required reviewers is only 0 on branch 'main'
5. • Owner review not required on branch 'main'
6. • 'administrator' PRs are exempt from reviews on branch 'main'

[pjbgf]

@saschagrunert here's my views/questions on the 6 points:

1. I can't think of any issues enabling linear history for the master branch. What do you think?
2. This is a false positive, as status checks are enabled for master. No action required here.
3. The state dismissal can be a problem for us whilst we have some long running/flake tests. I think we could keep as is and review it after further improvements on test suite.
4. / 5. / 6. This may be due to the fact that "Include Administrators" is disabled - probably the same is for number 2. Do you think doing the below may break our merge automation?
   • Enable Include Administrators
   • Enable Require a pull request before merging
   • Add k8s-ci-robot to Restrict who can push to matching branches

[saschagrunert]

1. I can't think of any issues enabling linear history for the master branch. What do you think?

We already have it enabled, right? :)

2. This is a false positive, as status checks are enabled for master. No action required here.

Agree 👍

3. The state dismissal can be a problem for us whilst we have some long running/flake tests. I think we could keep as is and review it after further improvements on test suite.

About which kind of dismissal are we speaking about here? Something like the integration of a stale bot?

Edit: Ah the review dismissal. I'm not sure if the bot removes the lgtm / approve labels when the review gets dismissed.

4. / 5. / 6. This may be due to the fact that "Include Administrators" is disabled - probably the same is for number 2. Do you think doing the below may break our merge automation?

   • Enable Include Administrators
   • Enable Require a pull request before merging
   • Add k8s-ci-robot to Restrict who can push to matching branches

We could give it a try! :)

[saschagrunert]

@pjbgf changed the branch protection rule for master, let's see what it breaks.

[pjbgf]

@saschagrunert nice one, let's hope it all works out.

By the way, the linear history for master branch still shows as disabled for me:

So we are probably not refering to the same thing. :)

[jhrozek]

On Thu, Nov 11, 2021 at 02:24:22AM -0800, Paulo Gomes wrote: @saschagrunert nice one, let's hope it all works out. By the way, the linear history for master branch still shows as disabled for me:  So we are probably not refering to the same thing. :)

But we don't have those horrible merge commits that GH uses by default for whatever silly reason. Isn't that it?

[saschagrunert]

@pjbgf I changed it to require a linear history.

It looks like that the other options from the branch protection were gone, so I hope that there is nothing like a nightly sync which enforces a certain branch protection ruleset within this org.

[pjbgf]

Will keep open until the desired state can be manually confirmed.

[pjbgf]

But we don't have those horrible merge commits that GH uses by default for whatever silly reason. Isn't that it?

@jhrozek sorry missed your message here. But I also did not understand exactly what do you mean here, can you clarify please?

[pjbgf]

After a delay the changes were applied:

Let's keep an eye in the comming days as to the impact this may have.
Closing it for the time being.

[jhrozek]

But we don't have those horrible merge commits that GH uses by default for whatever silly reason. Isn't that it?

@jhrozek sorry missed your message here. But I also did not understand exactly what do you mean here, can you clarify please?

No problem, I probably meant something else than you, Sascha and the scorecard meant. I thought we already did have linear history because I don't see merge commits in SPO like I see e.g. in selinuxd (https://github.com/containers/selinuxd/commits/main)