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

🌱 Add probes for Branch Protection #3691

Merged
merged 30 commits into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
5ad0848
:seedling: Add probes for Branch Protection
AdamKorcz Nov 21, 2023
17fb059
specify that Scorecard only considers default and releases branches
AdamKorcz Dec 8, 2023
b99a6b3
reduce duplication in blocksDeleteOnBranches
AdamKorcz Dec 8, 2023
cbe9922
use helper to test for boolean values
AdamKorcz Dec 8, 2023
b330f57
Fix typo, mention OutcomeNotAvailable
AdamKorcz Dec 8, 2023
7453581
fix typo and elaborate on effort
AdamKorcz Dec 8, 2023
8a63586
fix typo. Specify which branches the probe considers
AdamKorcz Dec 8, 2023
5087d5f
Fix copy paste typo
AdamKorcz Dec 8, 2023
c280afd
remove '/en' from url
AdamKorcz Dec 8, 2023
8d22094
change effort from 'High' to 'Low' in the blocksForcePushOnBranches p…
AdamKorcz Dec 8, 2023
90c0b3a
fix remediation level
AdamKorcz Dec 8, 2023
ed7d61b
Change probe package name
AdamKorcz Dec 8, 2023
4864abd
improve probe definitions
AdamKorcz Dec 11, 2023
73d5a2c
refactor test names
AdamKorcz Dec 11, 2023
58cd406
Change motivation of two probes
AdamKorcz Dec 12, 2023
c63a064
downgrade effort of runsStatusChecksBeforeMerging
AdamKorcz Dec 13, 2023
010c045
reduce complexity of blocksForcePushOnBranches
AdamKorcz Dec 13, 2023
31eb14d
simplify requiresCodeOwnersReview logic
AdamKorcz Dec 13, 2023
4bf3d9d
fix linter issues
AdamKorcz Dec 13, 2023
07737c8
fix copy paste error
AdamKorcz Dec 19, 2023
3063a0f
differentiate trueMsg and falseMsg in requiresApproversForPullRequests
AdamKorcz Dec 19, 2023
ddf9569
fix text in requiresCodeOwnersReview
AdamKorcz Dec 19, 2023
a9fbd3c
change outcome in utils
AdamKorcz Dec 19, 2023
4bd1ec4
fix lint issues
AdamKorcz Dec 19, 2023
f4b3962
fix nit in text
AdamKorcz Dec 19, 2023
8bad034
use standardized messages
AdamKorcz Dec 19, 2023
87ecafb
remove 'Uint32LargerThan0'
AdamKorcz Dec 19, 2023
e8e09ec
Add number of required reviewers to values. Refactor to avoid nil-der…
AdamKorcz Dec 19, 2023
2f2ec9e
fix nit log message
AdamKorcz Dec 20, 2023
24f9b94
Merge branch 'main' into branch-protection
spencerschrock Dec 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions probes/blocksDeleteOnBranches/def.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Copyright 2023 OpenSSF Scorecard Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

id: blocksDeleteOnBranches
short: Check that the project blocks non-admins from deleting branches.
motivation: >
Allowing non-admins to delete project branches has a similar effect to performing force pushes.
implementation: >
Checks the protection rules of default and release branches.
outcome:
- The probe returns one OutcomePositive for each branch that is disallowed from users deleting it, and one OutcomeNegative for branches where users are able to delete it. Scorecard only considers default and releases branches.
remediation:
effort: Low
text:
- Disallow deletion of branches in your project to remove negative outcomes.
- GitHub and Gitlab by default disable deleting a protected branch.
67 changes: 67 additions & 0 deletions probes/blocksDeleteOnBranches/impl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright 2023 OpenSSF Scorecard Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//nolint:stylecheck
package blocksDeleteOnBranches

import (
"embed"
"fmt"

"github.com/ossf/scorecard/v4/checker"
"github.com/ossf/scorecard/v4/finding"
"github.com/ossf/scorecard/v4/probes/internal/utils/uerror"
)

//go:embed *.yml
var fs embed.FS

const Probe = "blocksDeleteOnBranches"

func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
if raw == nil {
return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil)
}

Check warning on line 35 in probes/blocksDeleteOnBranches/impl.go

View check run for this annotation

Codecov / codecov/patch

probes/blocksDeleteOnBranches/impl.go#L34-L35

Added lines #L34 - L35 were not covered by tests

r := raw.BranchProtectionResults
var findings []finding.Finding

for i := range r.Branches {
branch := &r.Branches[i]

var text string
var outcome finding.Outcome
switch {
case branch.BranchProtectionRule.AllowDeletions == nil:
text = "could not determine whether branch is protected against deletion"
outcome = finding.OutcomeNotAvailable
case *branch.BranchProtectionRule.AllowDeletions:
text = fmt.Sprintf("'allow deletion' enabled on branch '%s'", *branch.Name)
outcome = finding.OutcomeNegative
case !*branch.BranchProtectionRule.AllowDeletions:
text = fmt.Sprintf("'allow deletion' disabled on branch '%s'", *branch.Name)
outcome = finding.OutcomePositive
default:

Check warning on line 55 in probes/blocksDeleteOnBranches/impl.go

View check run for this annotation

Codecov / codecov/patch

probes/blocksDeleteOnBranches/impl.go#L55

Added line #L55 was not covered by tests
}
f, err := finding.NewWith(fs, Probe, text, nil, outcome)
if err != nil {
return nil, Probe, fmt.Errorf("create finding: %w", err)
}

Check warning on line 60 in probes/blocksDeleteOnBranches/impl.go

View check run for this annotation

Codecov / codecov/patch

probes/blocksDeleteOnBranches/impl.go#L59-L60

Added lines #L59 - L60 were not covered by tests
f = f.WithValues(map[string]int{
*branch.Name: 1,
})
findings = append(findings, *f)
}
return findings, Probe, nil
}
185 changes: 185 additions & 0 deletions probes/blocksDeleteOnBranches/impl_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
// Copyright 2023 OpenSSF Scorecard Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//nolint:stylecheck
package blocksDeleteOnBranches

import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"

"github.com/ossf/scorecard/v4/checker"
"github.com/ossf/scorecard/v4/clients"
"github.com/ossf/scorecard/v4/finding"
)

func Test_Run(t *testing.T) {
t.Parallel()
trueVal := true
falseVal := false
branchVal1 := "branch-name1"
branchVal2 := "branch-name1"

//nolint:govet
tests := []struct {
name string
raw *checker.RawResults
outcomes []finding.Outcome
err error
}{
{
name: "One branch blocks branch deletion should result in one positive outcome",
raw: &checker.RawResults{
BranchProtectionResults: checker.BranchProtectionsData{
Branches: []clients.BranchRef{
{
Name: &branchVal1,
BranchProtectionRule: clients.BranchProtectionRule{
AllowDeletions: &falseVal,
},
},
},
},
},
outcomes: []finding.Outcome{
finding.OutcomePositive,
},
},
{
name: "Two branches that block branch deletions should result in two positive outcomes",
raw: &checker.RawResults{
BranchProtectionResults: checker.BranchProtectionsData{
Branches: []clients.BranchRef{
{
Name: &branchVal1,
BranchProtectionRule: clients.BranchProtectionRule{
AllowDeletions: &falseVal,
},
},
{
Name: &branchVal2,
BranchProtectionRule: clients.BranchProtectionRule{
AllowDeletions: &falseVal,
},
},
},
},
},
outcomes: []finding.Outcome{
finding.OutcomePositive, finding.OutcomePositive,
},
},
{
name: "Two branches in total: One blocks branch deletion and one doesn't = 1 positive & 1 negative",
raw: &checker.RawResults{
BranchProtectionResults: checker.BranchProtectionsData{
Branches: []clients.BranchRef{
{
Name: &branchVal1,
BranchProtectionRule: clients.BranchProtectionRule{
AllowDeletions: &falseVal,
},
},
{
Name: &branchVal2,
BranchProtectionRule: clients.BranchProtectionRule{
AllowDeletions: &trueVal,
},
},
},
},
},
outcomes: []finding.Outcome{
finding.OutcomePositive, finding.OutcomeNegative,
},
},
{
name: "Two branches in total: One blocks branch deletion and one doesn't = 1 negative & 1 positive",
raw: &checker.RawResults{
BranchProtectionResults: checker.BranchProtectionsData{
Branches: []clients.BranchRef{
{
Name: &branchVal1,
BranchProtectionRule: clients.BranchProtectionRule{
AllowDeletions: &trueVal,
},
},
{
Name: &branchVal2,
BranchProtectionRule: clients.BranchProtectionRule{
AllowDeletions: &falseVal,
},
},
},
},
},
outcomes: []finding.Outcome{
finding.OutcomeNegative, finding.OutcomePositive,
},
},
{
name: "Two branches in total: One blocks branch deletion and one lacks data = 1 negative & 1 unavailable",
raw: &checker.RawResults{
BranchProtectionResults: checker.BranchProtectionsData{
Branches: []clients.BranchRef{
{
Name: &branchVal1,
BranchProtectionRule: clients.BranchProtectionRule{
AllowDeletions: &trueVal,
},
},
{
Name: &branchVal2,
BranchProtectionRule: clients.BranchProtectionRule{
AllowDeletions: nil,
},
},
},
},
},
outcomes: []finding.Outcome{
finding.OutcomeNegative, finding.OutcomeNotAvailable,
},
},
}
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

findings, s, err := Run(tt.raw)
if !cmp.Equal(tt.err, err, cmpopts.EquateErrors()) {
t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(tt.err, err, cmpopts.EquateErrors()))
}
if err != nil {
return
}
if diff := cmp.Diff(Probe, s); diff != "" {
t.Errorf("mismatch (-want +got):\n%s", diff)
}
if diff := cmp.Diff(len(tt.outcomes), len(findings)); diff != "" {
t.Errorf("mismatch (-want +got):\n%s", diff)
}
for i := range tt.outcomes {
outcome := &tt.outcomes[i]
f := &findings[i]
if diff := cmp.Diff(*outcome, f.Outcome); diff != "" {
t.Errorf("mismatch (-want +got):\n%s", diff)
}
}
})
}
}
33 changes: 33 additions & 0 deletions probes/blocksForcePushOnBranches/def.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Copyright 2023 OpenSSF Scorecard Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

id: blocksForcePushOnBranches
short: Check that the project blocks force push on its branches.
motivation: >
Allowing non-admins to force push to branches could allow untrusted users to make insecure changes to the behavior of the project.
implementation: >
Checks the protection rules of default and release branches.
outcome:
- The probe returns one OutcomePositive for each branch that is blocked from force pushes, and one OutcomeNegative for branches that allows force push.
- Returns OutcomeNotAvailable if Scorecard cannot fetch the data from the repository.
remediation:
effort: Low
text:
- Disallow force pushes branches in your project to remove negative outcomes.
- For GitHub-hosted projects, force pushes are disabled by default. To make sure it has not been enabled, see ["Allow force pushes"](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches#allow-force-pushes).
- For Gitlab-hosted projects, follow the ["Protected branches"](https://docs.gitlab.com/ee/user/project/protected_branches.html) documentation to see who can force push to the project.
markdown:
- Disallow force pushes branches in your project to remove negative outcomes.
- For GitHub-hosted projects, force pushes are disabled by default. To make sure it has not been enabled, see ["Allow force pushes"](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches#allow-force-pushes).
- For Gitlab-hosted projects, follow the ["Protected branches"](https://docs.gitlab.com/ee/user/project/protected_branches.html) documentation to see who can force push to the project.
66 changes: 66 additions & 0 deletions probes/blocksForcePushOnBranches/impl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright 2023 OpenSSF Scorecard Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//nolint:stylecheck
package blocksForcePushOnBranches

import (
"embed"
"fmt"

"github.com/ossf/scorecard/v4/checker"
"github.com/ossf/scorecard/v4/finding"
"github.com/ossf/scorecard/v4/probes/internal/utils/uerror"
)

//go:embed *.yml
var fs embed.FS

const Probe = "blocksForcePushOnBranches"

func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
if raw == nil {
return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil)
}

Check warning on line 35 in probes/blocksForcePushOnBranches/impl.go

View check run for this annotation

Codecov / codecov/patch

probes/blocksForcePushOnBranches/impl.go#L34-L35

Added lines #L34 - L35 were not covered by tests

r := raw.BranchProtectionResults
var findings []finding.Finding

for i := range r.Branches {
branch := &r.Branches[i]
var text string
var outcome finding.Outcome
switch {
case branch.BranchProtectionRule.AllowForcePushes == nil:
text = "could not determine whether for push is allowed"
outcome = finding.OutcomeNotAvailable
case *branch.BranchProtectionRule.AllowForcePushes:
text = fmt.Sprintf("'force pushes' enabled on branch '%s'", *branch.Name)
outcome = finding.OutcomeNegative
case !*branch.BranchProtectionRule.AllowForcePushes:
text = fmt.Sprintf("'force pushes' disabled on branch '%s'", *branch.Name)
outcome = finding.OutcomePositive
default:

Check warning on line 54 in probes/blocksForcePushOnBranches/impl.go

View check run for this annotation

Codecov / codecov/patch

probes/blocksForcePushOnBranches/impl.go#L54

Added line #L54 was not covered by tests
}
f, err := finding.NewWith(fs, Probe, text, nil, outcome)
if err != nil {
return nil, Probe, fmt.Errorf("create finding: %w", err)
}

Check warning on line 59 in probes/blocksForcePushOnBranches/impl.go

View check run for this annotation

Codecov / codecov/patch

probes/blocksForcePushOnBranches/impl.go#L58-L59

Added lines #L58 - L59 were not covered by tests
f = f.WithValues(map[string]int{
*branch.Name: 1,
})
findings = append(findings, *f)
}
return findings, Probe, nil
}
Loading
Loading