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

1.1.16 and 1.1.17 producing false positives #123

Open
chuglo opened this issue Mar 17, 2023 · 1 comment
Open

1.1.16 and 1.1.17 producing false positives #123

chuglo opened this issue Mar 17, 2023 · 1 comment

Comments

@chuglo
Copy link

chuglo commented Mar 17, 2023

Description

1.1.16 states that for each repository in use, we must validate that no one can “force push” code.

1.1.17 states that for each repository that is being used, we must verify that protected branches cannot be deleted.

The rule logic for these two benchmarks appears to be written in such a way that it produces false positives. When Allow force pushes and Allow deletions are checked, thus permitting the ability to force pushes and/or delete branches, Chain-Bench outputs a Passed where a Failed would be expected.

Screenshot 2023-03-17 at 10 42 29 AM

Screenshot 2023-03-17 at 10 43 18 AM

The opposite will happen if you have them unchecked - you'll get a Failed result.

Looking at the rule logic in question

#Looking for default branch protection that restrict force push to branch
CbPolicy[msg] {
	not is_no_branch_protection
	is_branch_protection_restrict_force_push
	msg := {"ids": ["1.1.16"], "status": constsLib.status.Failed}
}

#Looking for default branch protection that restrict who can delete protected branch
CbPolicy[msg] {
	not is_no_branch_protection
	is_branch_protection_restrict_delete_branch
	msg := {"ids": ["1.1.17"], "status": constsLib.status.Failed}
}

this reads to say "when the branch is protected and disallows force pushes or deletions (in other words, if AllowForcePushes and AllowDeletions == false), produce a Failed result. In my mind, this should read as "when the branch is protected and allows force pushes or deletes, produce a Failed result.

Prepending not to both L226 and L233 causes Chain-Bench to produce an expected result.

@ad-mcas
Copy link

ad-mcas commented Jul 8, 2024

I don't know for 1.1.17, but for 1.1.16 the program has wrong.

Actual Result (if "allow_force_push": true in protected branch of gitlab):

1.1.16   Ensure force pushes code to branches is denied                   Failed

Excepted Result:

1.1.16   Ensure force pushes code to branches is denied                    Passed

And
Actual Result (if "allow_force_push": false in protected branch of gitlab):

1.1.16   Ensure force pushes code to branches is denied                     Passed

Excepted Result:

1.1.16   Ensure force pushes code to branches is denied                     Failed

If we look others rules (for instance 1.1.12), the method to check the rule is always negative.

So for 1.1.16, the rule must be:

 CbPolicy[msg] {
	not is_no_branch_protection
	is_branch_protection_not_restrict_force_push
	msg := {"ids": ["1.1.16"], "status": constsLib.status.Failed}
}

with is_branch_protection_not_restrict_force_push:

is_branch_protection_not_restrict_force_push {
	input.BranchProtections.AllowForcePushes == true
}

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

No branches or pull requests

2 participants