From fea69453a2b20b6a337579ba25b5c2f59cc3a95a Mon Sep 17 00:00:00 2001 From: Adam Korczynski Date: Tue, 29 Jul 2025 10:46:14 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=8C=B1=20Add=20tagsHandler=20to=20githubr?= =?UTF-8?q?epo=20client?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Adam Korczynski --- checker/raw_result.go | 10 +- checks/branch_protection_test.go | 50 +- checks/permissions_test.go | 2 +- checks/raw/branch_protection.go | 4 +- checks/raw/branch_protection_test.go | 28 +- checks/raw/tag_protection.go | 71 ++ checks/raw/tag_protection_test.go | 116 ++++ clients/azuredevopsrepo/branches.go | 10 +- clients/azuredevopsrepo/branches_test.go | 4 +- clients/azuredevopsrepo/client.go | 14 +- clients/branch.go | 14 +- clients/git/client.go | 16 +- clients/githubrepo/branches.go | 87 +-- clients/githubrepo/branches_test.go | 108 ++-- clients/githubrepo/client.go | 21 +- .../internal/fnmatch/fnmatch_test.go | 1 + clients/githubrepo/tags.go | 120 ++++ clients/githubrepo/tags_e2e_test.go | 59 ++ clients/githubrepo/tags_test.go | 611 ++++++++++++++++++ clients/gitlabrepo/branches.go | 18 +- clients/gitlabrepo/client.go | 14 +- clients/localdir/client.go | 14 +- clients/mockclients/repo_client.go | 38 +- clients/ossfuzz/client.go | 14 +- clients/repo_client.go | 6 +- cron/internal/format/json_raw_results.go | 20 +- go.mod | 2 + pkg/scorecard/json_raw_results.go | 20 +- pkg/scorecard/json_raw_results_test.go | 10 +- probes/blocksDeleteOnBranches/impl.go | 6 +- probes/blocksDeleteOnBranches/impl_test.go | 28 +- probes/blocksForcePushOnBranches/impl.go | 6 +- probes/blocksForcePushOnBranches/impl_test.go | 28 +- .../branchProtectionAppliesToAdmins/impl.go | 2 +- .../impl_test.go | 28 +- probes/branchesAreProtected/impl_test.go | 12 +- probes/dismissesStaleReviews/impl.go | 2 +- probes/dismissesStaleReviews/impl_test.go | 28 +- .../requiresApproversForPullRequests/impl.go | 2 +- .../impl_test.go | 28 +- probes/requiresCodeOwnersReview/impl.go | 2 +- probes/requiresCodeOwnersReview/impl_test.go | 50 +- probes/requiresLastPushApproval/impl.go | 2 +- probes/requiresLastPushApproval/impl_test.go | 28 +- probes/requiresPRsToChangeCode/impl.go | 2 +- probes/requiresPRsToChangeCode/impl_test.go | 28 +- probes/requiresUpToDateBranches/impl.go | 2 +- probes/requiresUpToDateBranches/impl_test.go | 28 +- probes/runsStatusChecksBeforeMerging/impl.go | 2 +- .../impl_test.go | 28 +- 50 files changed, 1468 insertions(+), 376 deletions(-) create mode 100644 checks/raw/tag_protection.go create mode 100644 checks/raw/tag_protection_test.go create mode 100644 clients/githubrepo/tags.go create mode 100644 clients/githubrepo/tags_e2e_test.go create mode 100644 clients/githubrepo/tags_test.go diff --git a/checker/raw_result.go b/checker/raw_result.go index bf5132b7e45..9340e347875 100644 --- a/checker/raw_result.go +++ b/checker/raw_result.go @@ -29,6 +29,7 @@ import ( type RawResults struct { BinaryArtifactResults BinaryArtifactData BranchProtectionResults BranchProtectionsData + TagProtectionResults TagProtectionsData CIIBestPracticesResults CIIBestPracticesData CITestResults CITestData CodeReviewResults CodeReviewData @@ -332,7 +333,14 @@ type WebhooksData struct { // BranchProtectionsData contains the raw results // for the Branch-Protection check. type BranchProtectionsData struct { - Branches []clients.BranchRef + Branches []clients.RepoRef + CodeownersFiles []string +} + +// TagProtectionsData contains the raw results +// for the Tag-Protection check. +type TagProtectionsData struct { + Tags []clients.RepoRef CodeownersFiles []string } diff --git a/checks/branch_protection_test.go b/checks/branch_protection_test.go index 758efe8fa17..3b375e7c65d 100644 --- a/checks/branch_protection_test.go +++ b/checks/branch_protection_test.go @@ -26,14 +26,14 @@ import ( scut "github.com/ossf/scorecard/v5/utests" ) -func getBranchName(branch *clients.BranchRef) string { +func getBranchName(branch *clients.RepoRef) string { if branch == nil || branch.Name == nil { return "" } return *branch.Name } -func getBranch(branches []*clients.BranchRef, name string, isNonAdmin bool) *clients.BranchRef { +func getBranch(branches []*clients.RepoRef, name string, isNonAdmin bool) *clients.RepoRef { for _, branch := range branches { branchName := getBranchName(branch) if branchName == name { @@ -46,9 +46,9 @@ func getBranch(branches []*clients.BranchRef, name string, isNonAdmin bool) *cli return nil } -func scrubBranch(branch *clients.BranchRef) *clients.BranchRef { +func scrubBranch(branch *clients.RepoRef) *clients.RepoRef { ret := branch - ret.BranchProtectionRule = clients.BranchProtectionRule{} + ret.ProtectionRule = clients.ProtectionRule{} return ret } @@ -67,7 +67,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { tests := []struct { name string defaultBranch string - branches []*clients.BranchRef + branches []*clients.RepoRef releases []string repoFiles []string expected scut.TestReturn @@ -83,10 +83,10 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { NumberOfDebug: 0, }, defaultBranch: main, - branches: []*clients.BranchRef{ + branches: []*clients.RepoRef{ { Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ RequiresStatusChecks: &trueVal, UpToDateBeforeMerge: &trueVal, @@ -106,7 +106,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { }, { Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ RequiresStatusChecks: &trueVal, UpToDateBeforeMerge: &falseVal, @@ -138,7 +138,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { NumberOfDebug: 0, }, defaultBranch: main, - branches: []*clients.BranchRef{ + branches: []*clients.RepoRef{ { Name: &rel1, Protected: &falseVal, @@ -146,7 +146,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { { Name: &main, Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ RequiresStatusChecks: &trueVal, UpToDateBeforeMerge: &falseVal, @@ -178,11 +178,11 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { NumberOfDebug: 0, }, defaultBranch: main, - branches: []*clients.BranchRef{ + branches: []*clients.RepoRef{ { Name: &main, Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ RequiresStatusChecks: &trueVal, UpToDateBeforeMerge: &trueVal, @@ -204,7 +204,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { { Name: &rel1, Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ RequiresStatusChecks: &trueVal, UpToDateBeforeMerge: &falseVal, @@ -236,11 +236,11 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { NumberOfDebug: 0, }, defaultBranch: main, - branches: []*clients.BranchRef{ + branches: []*clients.RepoRef{ { Name: &main, Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ RequiresStatusChecks: &trueVal, UpToDateBeforeMerge: &trueVal, @@ -262,7 +262,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { { Name: &rel1, Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ RequiresStatusChecks: &trueVal, UpToDateBeforeMerge: &trueVal, @@ -295,11 +295,11 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { }, defaultBranch: main, releases: []string{sha}, - branches: []*clients.BranchRef{ + branches: []*clients.RepoRef{ { Name: &main, Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ RequiresStatusChecks: &trueVal, UpToDateBeforeMerge: &falseVal, @@ -334,11 +334,11 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { }, defaultBranch: main, releases: []string{""}, - branches: []*clients.BranchRef{ + branches: []*clients.RepoRef{ { Name: &main, Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ RequiresStatusChecks: &trueVal, UpToDateBeforeMerge: &falseVal, @@ -371,11 +371,11 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { defaultBranch: main, // branches: []*string{&rel1, &main}, releases: []string{rel1}, - branches: []*clients.BranchRef{ + branches: []*clients.RepoRef{ { Name: &main, Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ RequiresStatusChecks: &trueVal, UpToDateBeforeMerge: &trueVal, @@ -386,7 +386,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { { Name: &rel1, Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ RequiresStatusChecks: &trueVal, UpToDateBeforeMerge: &trueVal, @@ -405,7 +405,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { ctrl := gomock.NewController(t) mockRepoClient := mockrepo.NewMockRepoClient(ctrl) mockRepoClient.EXPECT().GetDefaultBranch(). - DoAndReturn(func() (*clients.BranchRef, error) { + DoAndReturn(func() (*clients.RepoRef, error) { defaultBranch := getBranch(tt.branches, tt.defaultBranch, tt.nonadmin) return defaultBranch, nil }).AnyTimes() @@ -420,7 +420,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { return ret, nil }).AnyTimes() mockRepoClient.EXPECT().GetBranch(gomock.Any()). - DoAndReturn(func(b string) (*clients.BranchRef, error) { + DoAndReturn(func(b string) (*clients.RepoRef, error) { return getBranch(tt.branches, b, tt.nonadmin), nil }).AnyTimes() mockRepoClient.EXPECT().ListFiles(gomock.Any()).AnyTimes().Return(tt.repoFiles, nil) diff --git a/checks/permissions_test.go b/checks/permissions_test.go index e959e66e16d..c50fda41111 100644 --- a/checks/permissions_test.go +++ b/checks/permissions_test.go @@ -434,7 +434,7 @@ func TestGithubTokenPermissions(t *testing.T) { main := "main" mockRepo.EXPECT().URI().Return("github.com/ossf/scorecard").AnyTimes() - mockRepo.EXPECT().GetDefaultBranch().Return(&clients.BranchRef{Name: &main}, nil).AnyTimes() + mockRepo.EXPECT().GetDefaultBranch().Return(&clients.RepoRef{Name: &main}, nil).AnyTimes() mockRepo.EXPECT().ListFiles(gomock.Any()).DoAndReturn(func(predicate func(string) (bool, error)) ([]string, error) { files := []string{} for _, fn := range tt.filenames { diff --git a/checks/raw/branch_protection.go b/checks/raw/branch_protection.go index 8ad8dff959e..b0257ef1104 100644 --- a/checks/raw/branch_protection.go +++ b/checks/raw/branch_protection.go @@ -31,10 +31,10 @@ var commit = regexp.MustCompile("^[a-f0-9]{40}$") type branchSet struct { exists map[string]bool - set []clients.BranchRef + set []clients.RepoRef } -func (set *branchSet) add(branch *clients.BranchRef) bool { +func (set *branchSet) add(branch *clients.RepoRef) bool { if branch != nil && branch.Name != nil && *branch.Name != "" && diff --git a/checks/raw/branch_protection_test.go b/checks/raw/branch_protection_test.go index 28f516a78e8..c4c54d1a45a 100644 --- a/checks/raw/branch_protection_test.go +++ b/checks/raw/branch_protection_test.go @@ -37,13 +37,13 @@ var ( type branchArg struct { err error name string - branchRef *clients.BranchRef + branchRef *clients.RepoRef defaultBranch bool } type branchesArg []branchArg -func (ba branchesArg) getDefaultBranch() (*clients.BranchRef, error) { +func (ba branchesArg) getDefaultBranch() (*clients.RepoRef, error) { for _, branch := range ba { if branch.defaultBranch { return branch.branchRef, branch.err @@ -52,7 +52,7 @@ func (ba branchesArg) getDefaultBranch() (*clients.BranchRef, error) { return nil, nil } -func (ba branchesArg) getBranch(b string) (*clients.BranchRef, error) { +func (ba branchesArg) getBranch(b string) (*clients.RepoRef, error) { for _, branch := range ba { if branch.name == b { return branch.branchRef, branch.err @@ -104,13 +104,13 @@ func TestBranchProtection(t *testing.T) { { name: defaultBranchName, defaultBranch: true, - branchRef: &clients.BranchRef{ + branchRef: &clients.RepoRef{ Name: &defaultBranchName, }, }, }, want: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &defaultBranchName, }, @@ -180,13 +180,13 @@ func TestBranchProtection(t *testing.T) { branches: branchesArg{ { name: releaseBranchName, - branchRef: &clients.BranchRef{ + branchRef: &clients.RepoRef{ Name: &releaseBranchName, }, }, }, want: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &releaseBranchName, }, @@ -204,13 +204,13 @@ func TestBranchProtection(t *testing.T) { branches: branchesArg{ { name: mainBranchName, - branchRef: &clients.BranchRef{ + branchRef: &clients.RepoRef{ Name: &mainBranchName, }, }, }, want: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &mainBranchName, }, @@ -229,19 +229,19 @@ func TestBranchProtection(t *testing.T) { { name: defaultBranchName, defaultBranch: true, - branchRef: &clients.BranchRef{ + branchRef: &clients.RepoRef{ Name: &defaultBranchName, }, }, { name: releaseBranchName, - branchRef: &clients.BranchRef{ + branchRef: &clients.RepoRef{ Name: &releaseBranchName, }, }, }, want: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &defaultBranchName, }, @@ -260,11 +260,11 @@ func TestBranchProtection(t *testing.T) { ctrl := gomock.NewController(t) mockRepoClient := mockrepo.NewMockRepoClient(ctrl) mockRepoClient.EXPECT().GetDefaultBranch(). - AnyTimes().DoAndReturn(func() (*clients.BranchRef, error) { + AnyTimes().DoAndReturn(func() (*clients.RepoRef, error) { return tt.branches.getDefaultBranch() }) mockRepoClient.EXPECT().GetBranch(gomock.Any()).AnyTimes(). - DoAndReturn(func(branch string) (*clients.BranchRef, error) { + DoAndReturn(func(branch string) (*clients.RepoRef, error) { return tt.branches.getBranch(branch) }) mockRepoClient.EXPECT().ListReleases().AnyTimes(). diff --git a/checks/raw/tag_protection.go b/checks/raw/tag_protection.go new file mode 100644 index 00000000000..5fd178fdb6c --- /dev/null +++ b/checks/raw/tag_protection.go @@ -0,0 +1,71 @@ +// Copyright 2025 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. + +package raw + +import ( + "errors" + "fmt" + + "github.com/ossf/scorecard/v5/checker" + "github.com/ossf/scorecard/v5/clients" +) + +type tagSet struct { + exists map[string]bool + set []clients.RepoRef +} + +func (set *tagSet) add(tag *clients.RepoRef) bool { + if tag != nil && + tag.Name != nil && + *tag.Name != "" && + !set.exists[*tag.Name] { + set.set = append(set.set, *tag) + set.exists[*tag.Name] = true + return true + } + return false +} + +// TagProtection retrieves the raw data for the Tag-Protection check. +func TagProtection(cr *checker.CheckRequest) (checker.TagProtectionsData, error) { + c := cr.RepoClient + tags := tagSet{ + exists: make(map[string]bool), + } + repoTags, err := c.ListTags() + if err != nil && !errors.Is(err, clients.ErrUnsupportedFeature) { + return checker.TagProtectionsData{}, fmt.Errorf("%w", err) + } + for _, tag := range repoTags { + tagRef, err := c.GetTag(*tag.Name) + if err != nil { + return checker.TagProtectionsData{}, + fmt.Errorf("error during GetTag(): %w", err) + } + tags.add(tagRef) + } + + codeownersFiles := []string{} + if err := collectCodeownersFiles(c, &codeownersFiles); err != nil { + return checker.TagProtectionsData{}, err + } + + // No error, return the data. + return checker.TagProtectionsData{ + Tags: tags.set, + CodeownersFiles: codeownersFiles, + }, nil +} diff --git a/checks/raw/tag_protection_test.go b/checks/raw/tag_protection_test.go new file mode 100644 index 00000000000..07632836d2d --- /dev/null +++ b/checks/raw/tag_protection_test.go @@ -0,0 +1,116 @@ +// Copyright 2025 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. + +package raw + +import ( + "errors" + "testing" + + "github.com/google/go-cmp/cmp" + "go.uber.org/mock/gomock" + + "github.com/ossf/scorecard/v5/checker" + "github.com/ossf/scorecard/v5/clients" + mockrepo "github.com/ossf/scorecard/v5/clients/mockclients" +) + +var tagv01 = "v01" + +//nolint:govet +type tagArg struct { + err error + name string + tagRef *clients.RepoRef +} + +type tagsArg []tagArg + +func (ba tagsArg) getTag(b string) (*clients.RepoRef, error) { + for _, tag := range ba { + if tag.name == b { + return tag.tagRef, tag.err + } + } + return nil, nil +} + +func TestTagProtection(t *testing.T) { + t.Parallel() + //nolint:govet + tests := []struct { + name string + tags tagsArg + repoFiles []string + repoTags []*clients.RepoRef + releasesErr error + want checker.TagProtectionsData + wantErr error + }{ + { + name: "v01", + repoTags: []*clients.RepoRef{ + { + Name: &tagv01, + }, + }, + tags: tagsArg{ + { + name: "v01", + tagRef: &clients.RepoRef{ + Name: &tagv01, + }, + }, + }, + want: checker.TagProtectionsData{ + Tags: []clients.RepoRef{ + { + Name: &tagv01, + }, + }, + CodeownersFiles: []string{}, + }, + releasesErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + mockRepoClient := mockrepo.NewMockRepoClient(ctrl) + mockRepoClient.EXPECT().GetTag(gomock.Any()).AnyTimes(). + DoAndReturn(func(tag string) (*clients.RepoRef, error) { + return tt.tags.getTag(tag) + }) + mockRepoClient.EXPECT().ListTags().AnyTimes(). + DoAndReturn(func() ([]*clients.RepoRef, error) { + return tt.repoTags, tt.releasesErr + }) + mockRepoClient.EXPECT().ListFiles(gomock.Any()).AnyTimes().Return(tt.repoFiles, nil) + + c := &checker.CheckRequest{ + RepoClient: mockRepoClient, + } + rawData, err := TagProtection(c) + if !errors.Is(err, tt.wantErr) { + t.Errorf("failed. expected: %v, got: %v", tt.wantErr, err) + t.Fail() + } + if !cmp.Equal(rawData, tt.want) { + t.Errorf("failed. expected: %v, got: %v", tt.want, rawData) + t.Fail() + } + }) + } +} diff --git a/clients/azuredevopsrepo/branches.go b/clients/azuredevopsrepo/branches.go index 900831b70cb..e4ddc44c778 100644 --- a/clients/azuredevopsrepo/branches.go +++ b/clients/azuredevopsrepo/branches.go @@ -31,7 +31,7 @@ type branchesHandler struct { once *sync.Once errSetup error repourl *Repo - defaultBranchRef *clients.BranchRef + defaultBranchRef *clients.RepoRef queryBranch fnQueryBranch getPolicyConfigurations fnGetPolicyConfigurations } @@ -64,7 +64,7 @@ func (b *branchesHandler) setup() error { b.errSetup = fmt.Errorf("request for default branch failed with error %w", err) return } - b.defaultBranchRef = &clients.BranchRef{ + b.defaultBranchRef = &clients.RepoRef{ Name: branch.Name, } @@ -73,7 +73,7 @@ func (b *branchesHandler) setup() error { return b.errSetup } -func (b *branchesHandler) getDefaultBranch() (*clients.BranchRef, error) { +func (b *branchesHandler) getDefaultBranch() (*clients.RepoRef, error) { if err := b.setup(); err != nil { return nil, fmt.Errorf("error during branchesHandler.setup: %w", err) } @@ -81,7 +81,7 @@ func (b *branchesHandler) getDefaultBranch() (*clients.BranchRef, error) { return b.defaultBranchRef, nil } -func (b *branchesHandler) getBranch(branchName string) (*clients.BranchRef, error) { +func (b *branchesHandler) getBranch(branchName string) (*clients.RepoRef, error) { branch, err := b.queryBranch(b.ctx, git.GetBranchArgs{ RepositoryId: &b.repourl.id, Name: &branchName, @@ -110,7 +110,7 @@ func (b *branchesHandler) getBranch(branchName string) (*clients.BranchRef, erro } // TODO: map Azure DevOps branch protection to Scorecard branch protection - return &clients.BranchRef{ + return &clients.RepoRef{ Name: branch.Name, Protected: &isBranchProtected, }, nil diff --git a/clients/azuredevopsrepo/branches_test.go b/clients/azuredevopsrepo/branches_test.go index 5e99d2584b8..ca003ce6f15 100644 --- a/clients/azuredevopsrepo/branches_test.go +++ b/clients/azuredevopsrepo/branches_test.go @@ -81,7 +81,7 @@ func Test_getBranch(t *testing.T) { tests := []struct { getBranch fnQueryBranch getPolicyConfigurations fnGetPolicyConfigurations - expected *clients.BranchRef + expected *clients.RepoRef name string branchName string expectedError bool @@ -97,7 +97,7 @@ func Test_getBranch(t *testing.T) { PolicyConfigurations: &[]policy.PolicyConfiguration{}, }, nil }, - expected: &clients.BranchRef{ + expected: &clients.RepoRef{ Name: toPtr("main"), Protected: toPtr(false), }, diff --git a/clients/azuredevopsrepo/client.go b/clients/azuredevopsrepo/client.go index bae822d3ce6..fbd8209ffc8 100644 --- a/clients/azuredevopsrepo/client.go +++ b/clients/azuredevopsrepo/client.go @@ -145,7 +145,7 @@ func (c *Client) GetFileReader(filename string) (io.ReadCloser, error) { return c.zip.getFile(filename) } -func (c *Client) GetBranch(branch string) (*clients.BranchRef, error) { +func (c *Client) GetBranch(branch string) (*clients.RepoRef, error) { return c.branches.getBranch(branch) } @@ -170,7 +170,7 @@ func (c *Client) GetDefaultBranchName() (string, error) { return "", errDefaultBranchNotFound } -func (c *Client) GetDefaultBranch() (*clients.BranchRef, error) { +func (c *Client) GetDefaultBranch() (*clients.RepoRef, error) { return c.branches.getDefaultBranch() } @@ -233,6 +233,16 @@ func (c *Client) Close() error { return c.zip.cleanup() } +// GetBranch implements RepoClient.GetTag. +func (client *Client) GetTag(tag string) (*clients.RepoRef, error) { + return &clients.RepoRef{}, fmt.Errorf("GetTag: %w", clients.ErrUnsupportedFeature) +} + +func (client *Client) ListTags() ([]*clients.RepoRef, error) { + tags := make([]*clients.RepoRef, 0) + return tags, fmt.Errorf("ListTags: %w", clients.ErrUnsupportedFeature) +} + func CreateAzureDevOpsClient(ctx context.Context, repo clients.Repo) (*Client, error) { token := os.Getenv("AZURE_DEVOPS_AUTH_TOKEN") return CreateAzureDevOpsClientWithToken(ctx, token, repo) diff --git a/clients/branch.go b/clients/branch.go index 6d3190fe007..98d9a46639b 100644 --- a/clients/branch.go +++ b/clients/branch.go @@ -14,15 +14,15 @@ package clients -// BranchRef represents a single branch reference and its protection rules. -type BranchRef struct { - Name *string - Protected *bool - BranchProtectionRule BranchProtectionRule +// RepoRef represents either a single branch or tag reference and its protection rules. +type RepoRef struct { + Name *string + Protected *bool + ProtectionRule ProtectionRule } -// BranchProtectionRule captures the settings enabled on a branch for security. -type BranchProtectionRule struct { +// ProtectionRule captures the settings enabled on a branch for security. +type ProtectionRule struct { PullRequestRule PullRequestRule AllowDeletions *bool AllowForcePushes *bool diff --git a/clients/git/client.go b/clients/git/client.go index 4d6812a40b7..e5b9f73c15e 100644 --- a/clients/git/client.go +++ b/clients/git/client.go @@ -266,7 +266,7 @@ func (c *Client) Close() error { return nil } -func (c *Client) GetBranch(branch string) (*clients.BranchRef, error) { +func (c *Client) GetBranch(branch string) (*clients.RepoRef, error) { // Get the branch reference ref, err := c.gitRepo.Branch(branch) if err != nil { @@ -279,7 +279,7 @@ func (c *Client) GetBranch(branch string) (*clients.BranchRef, error) { } f := false // Create the BranchRef object - branchRef := &clients.BranchRef{ + branchRef := &clients.RepoRef{ Name: &ref.Name, Protected: &f, } @@ -328,7 +328,17 @@ func (c *Client) GetDefaultBranchName() (string, error) { return string(defaultBranch), nil } -func (c *Client) GetDefaultBranch() (*clients.BranchRef, error) { +// GetBranch implements RepoClient.GetTag. +func (client *Client) GetTag(tag string) (*clients.RepoRef, error) { + return &clients.RepoRef{}, clients.ErrUnsupportedFeature +} + +func (client *Client) ListTags() ([]*clients.RepoRef, error) { + tags := make([]*clients.RepoRef, 0) + return tags, clients.ErrUnsupportedFeature +} + +func (c *Client) GetDefaultBranch() (*clients.RepoRef, error) { // TODO: Implement this return nil, clients.ErrUnsupportedFeature } diff --git a/clients/githubrepo/branches.go b/clients/githubrepo/branches.go index eb5153c460c..65da3820335 100644 --- a/clients/githubrepo/branches.go +++ b/clients/githubrepo/branches.go @@ -37,7 +37,7 @@ const ( /* Example of query: query { repository(owner: "laurentsimon", name: "test3") { - branchProtectionRules(first: 100) { + ProtectionRules(first: 100) { edges { node { allowsDeletions @@ -128,7 +128,7 @@ type refUpdateRule struct { // Used for all settings, both admin and non-admin ones. // This only works with an admin token. -type branchProtectionRule struct { +type ProtectionRule struct { DismissesStaleReviews *bool IsAdminEnforced *bool RequiresStrictStatusChecks *bool @@ -141,17 +141,17 @@ type branchProtectionRule struct { RequireLastPushApproval *bool RequiredStatusCheckContexts []string // TODO: verify there is no conflicts. - // BranchProtectionRuleConflicts interface{} + // ProtectionRuleConflicts interface{} } -type branch struct { +type ref struct { Name *string RefUpdateRule *refUpdateRule - BranchProtectionRule *branchProtectionRule + BranchProtectionRule *ProtectionRule } type defaultBranchData struct { Repository struct { - DefaultBranchRef *branch + DefaultBranchRef *ref } `graphql:"repository(owner: $owner, name: $name)"` RateLimit struct { Cost *int @@ -218,7 +218,7 @@ type ruleSetData struct { type branchData struct { Repository struct { - Ref *branch `graphql:"ref(qualifiedName: $branchRefName)"` + Ref *ref `graphql:"ref(qualifiedName: $branchRefName)"` } `graphql:"repository(owner: $owner, name: $name)"` } @@ -230,7 +230,7 @@ type branchesHandler struct { ctx context.Context errSetup error repourl *Repo - defaultBranchRef *clients.BranchRef + defaultBranchRef *clients.RepoRef defaultBranchName string ruleSets []*repoRuleSet } @@ -275,7 +275,7 @@ func (handler *branchesHandler) setup() error { return } - rules, err := rulesMatchingBranch(handler.ruleSets, handler.defaultBranchName, true) + rules, err := rulesMatchingBranch(handler.ruleSets, handler.defaultBranchName, true, "BRANCH") if err != nil { handler.errSetup = sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("rulesMatchingBranch: %v", err)) return @@ -285,7 +285,7 @@ func (handler *branchesHandler) setup() error { return handler.errSetup } -func (handler *branchesHandler) query(branchName string) (*clients.BranchRef, error) { +func (handler *branchesHandler) query(branchName string) (*clients.RepoRef, error) { if !strings.EqualFold(handler.repourl.commitSHA, clients.HeadSHA) { return nil, fmt.Errorf("%w: branches only supported for HEAD queries", clients.ErrUnsupportedFeature) } @@ -302,21 +302,21 @@ func (handler *branchesHandler) query(branchName string) (*clients.BranchRef, er if err := handler.graphClient.Query(handler.ctx, queryData, vars); err != nil { return nil, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("githubv4.Query: %v", err)) } - rules, err := rulesMatchingBranch(handler.ruleSets, branchName, branchName == handler.defaultBranchName) + rules, err := rulesMatchingBranch(handler.ruleSets, branchName, branchName == handler.defaultBranchName, "BRANCH") if err != nil { return nil, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("rulesMatchingBranch: %v", err)) } return getBranchRefFrom(queryData.Repository.Ref, rules), nil } -func (handler *branchesHandler) getDefaultBranch() (*clients.BranchRef, error) { +func (handler *branchesHandler) getDefaultBranch() (*clients.RepoRef, error) { if err := handler.setup(); err != nil { return nil, fmt.Errorf("error during branchesHandler.setup: %w", err) } return handler.defaultBranchRef, nil } -func (handler *branchesHandler) getBranch(branch string) (*clients.BranchRef, error) { +func (handler *branchesHandler) getBranch(branch string) (*clients.RepoRef, error) { branchRef, err := handler.query(branch) if err != nil { return nil, fmt.Errorf("error during branchesHandler.query: %w", err) @@ -325,7 +325,7 @@ func (handler *branchesHandler) getBranch(branch string) (*clients.BranchRef, er } // TODO: Move these two functions to below the GetBranchRefFrom functions, the single place they're used. -func copyAdminSettings(src *branchProtectionRule, dst *clients.BranchProtectionRule) { +func copyAdminSettings(src *ProtectionRule, dst *clients.ProtectionRule) { copyBoolPtr(src.IsAdminEnforced, &dst.EnforceAdmins) copyBoolPtr(src.RequireLastPushApproval, &dst.RequireLastPushApproval) copyBoolPtr(src.DismissesStaleReviews, &dst.PullRequestRule.DismissStaleReviews) @@ -352,10 +352,10 @@ func copyAdminSettings(src *branchProtectionRule, dst *clients.BranchProtectionR } } -func copyNonAdminSettings(src interface{}, dst *clients.BranchProtectionRule) { +func copyNonAdminSettings(src interface{}, dst *clients.ProtectionRule) { // TODO: requiresConversationResolution, requiresSignatures, viewerAllowedToDismissReviews, viewerCanPush switch v := src.(type) { - case *branchProtectionRule: + case *ProtectionRule: copyBoolPtr(v.AllowsDeletions, &dst.AllowDeletions) copyBoolPtr(v.AllowsForcePushes, &dst.AllowForcePushes) copyBoolPtr(v.RequiresLinearHistory, &dst.RequireLinearHistory) @@ -407,11 +407,11 @@ func getActiveRuleSetsFrom(data *ruleSetData) []*repoRuleSet { return ret } -func getBranchRefFrom(data *branch, rules []*repoRuleSet) *clients.BranchRef { +func getBranchRefFrom(data *ref, rules []*repoRuleSet) *clients.RepoRef { if data == nil { return nil } - branchRef := new(clients.BranchRef) + branchRef := new(clients.RepoRef) if data.Name != nil { branchRef.Name = data.Name } @@ -428,7 +428,7 @@ func getBranchRefFrom(data *branch, rules []*repoRuleSet) *clients.BranchRef { } *branchRef.Protected = true - branchRule := &branchRef.BranchProtectionRule + branchRule := &branchRef.ProtectionRule switch { // All settings are available. This typically means @@ -460,22 +460,27 @@ func isPermissionsError(err error) bool { } const ( - ruleConditionDefaultBranch = "~DEFAULT_BRANCH" - ruleConditionAllBranches = "~ALL" - ruleDeletion = "DELETION" - ruleForcePush = "NON_FAST_FORWARD" - ruleLinear = "REQUIRED_LINEAR_HISTORY" - rulePullRequest = "PULL_REQUEST" - ruleStatusCheck = "REQUIRED_STATUS_CHECKS" + ruleConditionDefaultBranch = "~DEFAULT_BRANCH" + ruleConditionAllBranchesAndTags = "~ALL" + ruleDeletion = "DELETION" + ruleForcePush = "NON_FAST_FORWARD" + ruleLinear = "REQUIRED_LINEAR_HISTORY" + rulePullRequest = "PULL_REQUEST" + ruleStatusCheck = "REQUIRED_STATUS_CHECKS" ) -func rulesMatchingBranch(rules []*repoRuleSet, name string, defaultRef bool) ([]*repoRuleSet, error) { - refName := refPrefix + name +func rulesMatchingBranch(rules []*repoRuleSet, name string, defaultRef bool, target string) ([]*repoRuleSet, error) { + var refName string + if target == "BRANCH" { + refName = refPrefix + name + } else { + refName = name + } ret := make([]*repoRuleSet, 0) nextRule: for _, rule := range rules { // Skip rulesets that don't target branches - if rule.Target != nil && *rule.Target != "BRANCH" { + if rule.Target != nil && *rule.Target != target { continue } @@ -486,17 +491,17 @@ nextRule: continue nextRule } } - for _, cond := range rule.Conditions.RefName.Include { - if cond == ruleConditionAllBranches { + if cond == ruleConditionAllBranchesAndTags { ret = append(ret, rule) break } - if cond == ruleConditionDefaultBranch && defaultRef { - ret = append(ret, rule) - break + if target == "BRANCH" { + if cond == ruleConditionDefaultBranch && defaultRef { + ret = append(ret, rule) + break + } } - if match, err := fnmatch.Match(cond, refName); err != nil { return nil, fmt.Errorf("include match error: %w", err) } else if match { @@ -507,10 +512,10 @@ nextRule: return ret, nil } -func applyRepoRules(branchRef *clients.BranchRef, rules []*repoRuleSet) { +func applyRepoRules(branchRef *clients.RepoRef, rules []*repoRuleSet) { for _, r := range rules { // Init values of base checkbox as if they're unchecked - translated := clients.BranchProtectionRule{ + translated := clients.ProtectionRule{ AllowDeletions: asPtr(true), AllowForcePushes: asPtr(true), RequireLinearHistory: asPtr(false), @@ -535,11 +540,11 @@ func applyRepoRules(branchRef *clients.BranchRef, rules []*repoRuleSet) { translateRequiredStatusRepoRule(&translated, rule) } } - mergeBranchProtectionRules(&branchRef.BranchProtectionRule, &translated) + mergeProtectionRules(&branchRef.ProtectionRule, &translated) } } -func translatePullRequestRepoRule(base *clients.BranchProtectionRule, rule *repoRule) { +func translatePullRequestRepoRule(base *clients.ProtectionRule, rule *repoRule) { base.PullRequestRule.Required = asPtr(true) base.PullRequestRule.DismissStaleReviews = rule.Parameters.PullRequestParameters.DismissStaleReviewsOnPush base.PullRequestRule.RequireCodeOwnerReviews = rule.Parameters.PullRequestParameters.RequireCodeOwnerReview @@ -548,7 +553,7 @@ func translatePullRequestRepoRule(base *clients.BranchProtectionRule, rule *repo RequiredApprovingReviewCount } -func translateRequiredStatusRepoRule(base *clients.BranchProtectionRule, rule *repoRule) { +func translateRequiredStatusRepoRule(base *clients.ProtectionRule, rule *repoRule) { statusParams := rule.Parameters.StatusCheckParameters if len(statusParams.RequiredStatusChecks) == 0 { return @@ -566,7 +571,7 @@ func translateRequiredStatusRepoRule(base *clients.BranchProtectionRule, rule *r // Merge strategy: // - if both are nil, keep it nil // - if any of them is not nil, keep the most restrictive one -func mergeBranchProtectionRules(base, translated *clients.BranchProtectionRule) { +func mergeProtectionRules(base, translated *clients.ProtectionRule) { if base.AllowDeletions == nil || (translated.AllowDeletions != nil && !*translated.AllowDeletions) { base.AllowDeletions = translated.AllowDeletions } diff --git a/clients/githubrepo/branches_test.go b/clients/githubrepo/branches_test.go index 72c7fafebe3..16dd49cd49d 100644 --- a/clients/githubrepo/branches_test.go +++ b/clients/githubrepo/branches_test.go @@ -35,7 +35,7 @@ func Test_rulesMatchingBranch(t *testing.T) { name: "including all branches", condition: ruleSetCondition{ RefName: ruleSetConditionRefs{ - Include: []string{ruleConditionAllBranches}, + Include: []string{ruleConditionAllBranchesAndTags}, }, }, defaultBranchNames: map[string]bool{ @@ -124,7 +124,7 @@ func Test_rulesMatchingBranch(t *testing.T) { for branchName, expected := range testcase.defaultBranchNames { t.Run(fmt.Sprintf("default branch %s", branchName), func(t *testing.T) { t.Parallel() - matching, err := rulesMatchingBranch(inputRules, branchName, true) + matching, err := rulesMatchingBranch(inputRules, branchName, true, "BRANCH") if err != nil { t.Fatalf("expected - no error, got: %v", err) } @@ -136,7 +136,7 @@ func Test_rulesMatchingBranch(t *testing.T) { for branchName, expected := range testcase.nonDefaultBranchNames { t.Run(fmt.Sprintf("non-default branch %s", branchName), func(t *testing.T) { t.Parallel() - matching, err := rulesMatchingBranch(inputRules, branchName, false) + matching, err := rulesMatchingBranch(inputRules, branchName, false, "BRANCH") if err != nil { t.Fatalf("expected - no error, got: %v", err) } @@ -179,20 +179,20 @@ func Test_applyRepoRules(t *testing.T) { twoVal := int32(2) testcases := []struct { - base *clients.BranchRef - expected *clients.BranchRef + base *clients.RepoRef + expected *clients.RepoRef ruleBypass *ruleSetBypass name string ruleSets []*repoRuleSet }{ { name: "unchecked checkboxes have consistent values", - base: &clients.BranchRef{}, + base: &clients.RepoRef{}, ruleSets: []*repoRuleSet{ ruleSet(), }, - expected: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &trueVal, AllowForcePushes: &trueVal, CheckRules: clients.StatusChecksRule{ @@ -212,12 +212,12 @@ func Test_applyRepoRules(t *testing.T) { }, { name: "block deletion no bypass", - base: &clients.BranchRef{}, + base: &clients.RepoRef{}, ruleSets: []*repoRuleSet{ ruleSet(withRules(&repoRule{Type: ruleDeletion})), }, - expected: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, AllowForcePushes: &trueVal, RequireLinearHistory: &falseVal, @@ -230,12 +230,12 @@ func Test_applyRepoRules(t *testing.T) { }, { name: "block deletion with bypass", - base: &clients.BranchRef{}, + base: &clients.RepoRef{}, ruleSets: []*repoRuleSet{ ruleSet(withRules(&repoRule{Type: ruleDeletion}), withBypass()), }, - expected: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, AllowForcePushes: &trueVal, RequireLinearHistory: &falseVal, @@ -248,8 +248,8 @@ func Test_applyRepoRules(t *testing.T) { }, { name: "block deletion and force push with bypass when block force push no bypass", - base: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + base: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: &falseVal, EnforceAdmins: &trueVal, }, @@ -257,8 +257,8 @@ func Test_applyRepoRules(t *testing.T) { ruleSets: []*repoRuleSet{ ruleSet(withRules(&repoRule{Type: ruleDeletion}, &repoRule{Type: ruleForcePush}), withBypass()), }, - expected: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, AllowForcePushes: &falseVal, EnforceAdmins: &falseVal, // Downgrade: deletion does not enforce admins @@ -271,8 +271,8 @@ func Test_applyRepoRules(t *testing.T) { }, { name: "block deletion no bypass while force push is blocked with bypass", - base: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + base: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: &falseVal, EnforceAdmins: &falseVal, RequireLinearHistory: &falseVal, @@ -281,8 +281,8 @@ func Test_applyRepoRules(t *testing.T) { ruleSets: []*repoRuleSet{ ruleSet(withRules(&repoRule{Type: ruleDeletion})), }, - expected: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, AllowForcePushes: &falseVal, EnforceAdmins: &falseVal, // Maintain: deletion enforces but force-push does not @@ -295,8 +295,8 @@ func Test_applyRepoRules(t *testing.T) { }, { name: "block deletion no bypass while force push is blocked no bypass", - base: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + base: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: &falseVal, EnforceAdmins: &trueVal, }, @@ -304,8 +304,8 @@ func Test_applyRepoRules(t *testing.T) { ruleSets: []*repoRuleSet{ ruleSet(withRules(&repoRule{Type: ruleDeletion})), }, - expected: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, AllowForcePushes: &falseVal, EnforceAdmins: &trueVal, // Maintain: base and rule are equal strictness @@ -318,12 +318,12 @@ func Test_applyRepoRules(t *testing.T) { }, { name: "block force push no bypass", - base: &clients.BranchRef{}, + base: &clients.RepoRef{}, ruleSets: []*repoRuleSet{ ruleSet(withRules(&repoRule{Type: ruleForcePush})), }, - expected: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &trueVal, AllowForcePushes: &falseVal, EnforceAdmins: &trueVal, @@ -336,12 +336,12 @@ func Test_applyRepoRules(t *testing.T) { }, { name: "require linear history no bypass", - base: &clients.BranchRef{}, + base: &clients.RepoRef{}, ruleSets: []*repoRuleSet{ ruleSet(withRules(&repoRule{Type: ruleLinear})), }, - expected: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &trueVal, AllowForcePushes: &trueVal, RequireLinearHistory: &trueVal, @@ -354,7 +354,7 @@ func Test_applyRepoRules(t *testing.T) { }, { name: "require pull request but no reviewers and no bypass", - base: &clients.BranchRef{}, + base: &clients.RepoRef{}, ruleSets: []*repoRuleSet{ ruleSet(withRules(&repoRule{ Type: rulePullRequest, @@ -366,8 +366,8 @@ func Test_applyRepoRules(t *testing.T) { }, })), }, - expected: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &trueVal, AllowForcePushes: &trueVal, EnforceAdmins: &trueVal, @@ -382,7 +382,7 @@ func Test_applyRepoRules(t *testing.T) { }, { name: "require pull request with 2 reviewers no bypass", - base: &clients.BranchRef{}, + base: &clients.RepoRef{}, ruleSets: []*repoRuleSet{ ruleSet(withRules(&repoRule{ Type: rulePullRequest, @@ -397,8 +397,8 @@ func Test_applyRepoRules(t *testing.T) { }, })), }, - expected: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &trueVal, AllowForcePushes: &trueVal, EnforceAdmins: &trueVal, @@ -415,7 +415,7 @@ func Test_applyRepoRules(t *testing.T) { }, { name: "required status checks no bypass", - base: &clients.BranchRef{}, + base: &clients.RepoRef{}, ruleSets: []*repoRuleSet{ ruleSet(withRules(&repoRule{ Type: ruleStatusCheck, @@ -431,8 +431,8 @@ func Test_applyRepoRules(t *testing.T) { }, })), }, - expected: &clients.BranchRef{ - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &trueVal, AllowForcePushes: &trueVal, EnforceAdmins: &trueVal, @@ -450,7 +450,7 @@ func Test_applyRepoRules(t *testing.T) { }, { name: "Multiple rules sets impacting a branch", - base: &clients.BranchRef{}, + base: &clients.RepoRef{}, ruleSets: []*repoRuleSet{ ruleSet(withRules( // first a restrictive rule set, let's suppose it was built only for main. &repoRule{Type: ruleDeletion}, @@ -498,8 +498,8 @@ func Test_applyRepoRules(t *testing.T) { }, )), }, - expected: &clients.BranchRef{ // We expect to see dominance of restrictive rules. - BranchProtectionRule: clients.BranchProtectionRule{ + expected: &clients.RepoRef{ // We expect to see dominance of restrictive rules. + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, AllowForcePushes: &falseVal, EnforceAdmins: &trueVal, @@ -541,14 +541,14 @@ func Test_translationFromGithubAPIBranchProtectionData(t *testing.T) { zeroVal := int32(0) testcases := []struct { - branch *branch + ref *ref ruleSet *repoRuleSet - expected *clients.BranchRef + expected *clients.RepoRef name string }{ { name: "Non-admin Branch Protection rule with insufficient data about requiring PRs", - branch: &branch{ + ref: &ref{ RefUpdateRule: &refUpdateRule{ AllowsDeletions: &falseVal, AllowsForcePushes: &falseVal, @@ -560,9 +560,9 @@ func Test_translationFromGithubAPIBranchProtectionData(t *testing.T) { BranchProtectionRule: nil, }, ruleSet: nil, - expected: &clients.BranchRef{ + expected: &clients.RepoRef{ Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, AllowForcePushes: &falseVal, RequireLinearHistory: &falseVal, @@ -580,8 +580,8 @@ func Test_translationFromGithubAPIBranchProtectionData(t *testing.T) { }, { name: "Admin Branch Protection rule nothing selected", - branch: &branch{ - BranchProtectionRule: &branchProtectionRule{ + ref: &ref{ + BranchProtectionRule: &ProtectionRule{ DismissesStaleReviews: &falseVal, IsAdminEnforced: &falseVal, RequiresStrictStatusChecks: &falseVal, @@ -596,9 +596,9 @@ func Test_translationFromGithubAPIBranchProtectionData(t *testing.T) { }, }, ruleSet: nil, - expected: &clients.BranchRef{ + expected: &clients.RepoRef{ Protected: &trueVal, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, AllowForcePushes: &falseVal, EnforceAdmins: &falseVal, @@ -631,7 +631,7 @@ func Test_translationFromGithubAPIBranchProtectionData(t *testing.T) { repoRules = []*repoRuleSet{testcase.ruleSet} } - result := getBranchRefFrom(testcase.branch, repoRules) + result := getBranchRefFrom(testcase.ref, repoRules) if !cmp.Equal(result, testcase.expected) { diff := cmp.Diff(result, testcase.expected) diff --git a/clients/githubrepo/client.go b/clients/githubrepo/client.go index a2527db07a7..032ab7e60bc 100644 --- a/clients/githubrepo/client.go +++ b/clients/githubrepo/client.go @@ -51,6 +51,7 @@ type Client struct { graphClient *graphqlHandler contributors *contributorsHandler branches *branchesHandler + tags *tagsHandler releases *releasesHandler workflows *workflowsHandler checkruns *checkrunsHandler @@ -130,6 +131,9 @@ func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string, commitD // Setup branchesHandler. client.branches.init(client.ctx, client.repourl) + // Setup tagsHandler. + client.tags.init(client.ctx, client.repourl) + // Setup releasesHandler. client.releases.init(client.ctx, client.repourl) @@ -240,7 +244,7 @@ func (client *Client) IsArchived() (bool, error) { } // GetDefaultBranch implements RepoClient.GetDefaultBranch. -func (client *Client) GetDefaultBranch() (*clients.BranchRef, error) { +func (client *Client) GetDefaultBranch() (*clients.RepoRef, error) { return client.branches.getDefaultBranch() } @@ -254,10 +258,19 @@ func (client *Client) GetDefaultBranchName() (string, error) { } // GetBranch implements RepoClient.GetBranch. -func (client *Client) GetBranch(branch string) (*clients.BranchRef, error) { +func (client *Client) GetBranch(branch string) (*clients.RepoRef, error) { return client.branches.getBranch(branch) } +// GetBranch implements RepoClient.GetTag. +func (client *Client) GetTag(tag string) (*clients.RepoRef, error) { + return client.tags.getTag(tag) +} + +func (client *Client) ListTags() ([]*clients.RepoRef, error) { + return client.tags.getTags() +} + // GetCreatedAt is a getter for repo.CreatedAt. func (client *Client) GetCreatedAt() (time.Time, error) { return client.repo.CreatedAt.Time, nil @@ -395,6 +408,10 @@ func NewRepoClient(ctx context.Context, opts ...Option) (clients.RepoClient, err ghClient: client, graphClient: graphClient, }, + tags: &tagsHandler{ + ghClient: client, + graphClient: graphClient, + }, releases: &releasesHandler{ client: client, }, diff --git a/clients/githubrepo/internal/fnmatch/fnmatch_test.go b/clients/githubrepo/internal/fnmatch/fnmatch_test.go index 617506a368f..4f67954245b 100644 --- a/clients/githubrepo/internal/fnmatch/fnmatch_test.go +++ b/clients/githubrepo/internal/fnmatch/fnmatch_test.go @@ -54,6 +54,7 @@ func TestMatch(t *testing.T) { {"releases/**/**/*", "releases/v2", true}, {"releases/**/bar/**/qux", "releases/foo/bar/baz/qux", true}, {"users/**/*", "users/foo/bar", true}, + {"v1*", "v1.2.3", true}, } for _, tt := range tests { got, err := Match(tt.pattern, tt.path) diff --git a/clients/githubrepo/tags.go b/clients/githubrepo/tags.go new file mode 100644 index 00000000000..f532840a902 --- /dev/null +++ b/clients/githubrepo/tags.go @@ -0,0 +1,120 @@ +// Copyright 2025 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. + +package githubrepo + +import ( + "context" + "fmt" + "sync" + + "github.com/google/go-github/v53/github" + "github.com/shurcooL/githubv4" + + "github.com/ossf/scorecard/v5/clients" + sce "github.com/ossf/scorecard/v5/errors" +) + +// GraphQL query to load tag refs. +type tagsQuery struct { + Repository struct { + Refs struct { + Nodes []*ref + } `graphql:"refs(refPrefix: \"refs/tags/\", first: 100)"` + } `graphql:"repository(owner: $owner, name: $name)"` +} + +type tagsHandler struct { + ctx context.Context + ghClient *github.Client + graphClient *githubv4.Client + repourl *Repo + + once *sync.Once + errSetup error + + ruleSets []*repoRuleSet + tags []*ref +} + +func (h *tagsHandler) init(ctx context.Context, repourl *Repo) { + h.ctx = ctx + h.repourl = repourl + h.errSetup = nil + h.once = new(sync.Once) + h.ruleSets = nil +} + +func (h *tagsHandler) setup() error { + h.once.Do(func() { + vars := map[string]interface{}{ + "owner": githubv4.String(h.repourl.owner), + "name": githubv4.String(h.repourl.repo), + } + rulesData := new(ruleSetData) + if err := h.graphClient.Query(h.ctx, rulesData, vars); err != nil { + h.errSetup = sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("githubv4.Query: %v", err)) + return + } + h.ruleSets = getActiveRuleSetsFrom(rulesData) + + var q tagsQuery + if err := h.graphClient.Query(h.ctx, &q, vars); err != nil { + h.errSetup = sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("githubv4.Query tags: %v", err)) + return + } + h.tags = q.Repository.Refs.Nodes + }) + return h.errSetup +} + +func (h *tagsHandler) query(tagName string) (*clients.RepoRef, error) { + if err := h.setup(); err != nil { + return nil, fmt.Errorf("error during branchesHandler.setup: %w", err) + } + rules, err := rulesMatchingBranch(h.ruleSets, tagName, false, "TAG") + if err != nil { + return nil, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("rulesMatchingBranch: %v", err)) + } + + for _, r := range h.tags { + if *r.Name == tagName { + return getBranchRefFrom(r, rules), nil + } + } + return nil, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("tag not found: %s", tagName)) +} + +func (h *tagsHandler) getTags() ([]*clients.RepoRef, error) { + if err := h.setup(); err != nil { + return nil, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("error during tagsHandler.setup: %v", err)) + } + var out []*clients.RepoRef + for _, tag := range h.tags { + tag, err := h.getTag(*tag.Name) + if err != nil { + return nil, fmt.Errorf("getTag: %w", err) + } + out = append(out, tag) + } + return out, nil +} + +func (h *tagsHandler) getTag(tag string) (*clients.RepoRef, error) { + branchRef, err := h.query(tag) + if err != nil { + return nil, fmt.Errorf("error during branchesHandler.query: %w", err) + } + return branchRef, nil +} diff --git a/clients/githubrepo/tags_e2e_test.go b/clients/githubrepo/tags_e2e_test.go new file mode 100644 index 00000000000..ff8aae1a1a6 --- /dev/null +++ b/clients/githubrepo/tags_e2e_test.go @@ -0,0 +1,59 @@ +// Copyright 2025 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. + +package githubrepo + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("E2E TEST: githubrepo.tagsHandler", func() { + var tagshandler *tagsHandler + + BeforeEach(func() { + tagshandler = &tagsHandler{ + graphClient: graphClient, + } + }) + Context("E2E TEST: getTag", func() { + It("Should return a tag", func() { + skipIfTokenIsNot(patTokenType, "PAT only") + + repourl := &Repo{ + owner: "AdamKorcz", + repo: "scorecard-testing", + } + tagshandler.init(context.Background(), repourl) + err := tagshandler.setup() + Expect(err).Should(BeNil()) + tagRef, err := tagshandler.getTag("test-tag1") + Expect(err).Should(BeNil()) + Expect(tagRef).ShouldNot(BeNil()) + Expect(tagRef.ProtectionRule).ShouldNot(BeNil()) + Expect(tagRef.ProtectionRule.AllowForcePushes).ShouldNot(BeNil()) + Expect(tagRef.ProtectionRule.AllowDeletions).ShouldNot(BeNil()) + Expect(tagRef.ProtectionRule.RequireLinearHistory).ShouldNot(BeNil()) + Expect(tagRef.ProtectionRule.EnforceAdmins).ShouldNot(BeNil()) + Expect(tagRef.ProtectionRule.RequireLastPushApproval).Should(BeNil()) + Expect(tagRef.ProtectionRule.PullRequestRule.Required).To(Equal(asPtr(false))) + Expect(tagRef.ProtectionRule.PullRequestRule.DismissStaleReviews).Should(BeNil()) + Expect(tagRef.ProtectionRule.PullRequestRule.RequireCodeOwnerReviews).Should(BeNil()) + Expect(tagRef.ProtectionRule.AllowDeletions).To(Equal(asPtr(false))) + Expect(tagRef.ProtectionRule.AllowForcePushes).To(Equal(asPtr(false))) + }) + }) +}) diff --git a/clients/githubrepo/tags_test.go b/clients/githubrepo/tags_test.go new file mode 100644 index 00000000000..2b0ca1c3e5a --- /dev/null +++ b/clients/githubrepo/tags_test.go @@ -0,0 +1,611 @@ +// Copyright 2025 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. + +package githubrepo + +import ( + "bytes" + "context" + "fmt" + "io" + "net/http" + "testing" + + "github.com/shurcooL/githubv4" +) + +// RoundTripper that returns canned GraphQL responses in order. +type seqRT struct { + bodies [][]byte + i int + code int +} + +func (s *seqRT) RoundTrip(req *http.Request) (*http.Response, error) { + if s.i >= len(s.bodies) { + return nil, fmt.Errorf("seqRT: no more responses (call #%d)", s.i) + } + b := s.bodies[s.i] + s.i++ + return &http.Response{ + StatusCode: s.code, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(bytes.NewReader(b)), + Request: req, + }, nil +} + +func newGraphQLClientWith(jsonBodies ...string) *githubv4.Client { + bufs := make([][]byte, 0, len(jsonBodies)) + for _, s := range jsonBodies { + bufs = append(bufs, []byte(s)) + } + return githubv4.NewClient(&http.Client{Transport: &seqRT{bodies: bufs, code: 200}}) +} + +func TestTagsHandler_GetTag_TableDriven(t *testing.T) { + t.Parallel() + + rulesJSONCase1 := ` +{ + "data": { + "repository": { + "rulesets": { + "nodes": [ + { + "name": "Tag PR + checks", + "enforcement": "ACTIVE", + "target": "TAG", + "conditions": { "refName": { "include": ["v1.*"], "exclude": [] } }, + "bypassActors": { "nodes": [ + { "bypassMode": "ALWAYS", "organizationAdmin": true, "repositoryRoleName": "maintainer" } + ]}, + "rules": { "nodes": [ + { + "type": "PULL_REQUEST", + "parameters": { + "dismissStaleReviewsOnPush": true, + "requireCodeOwnerReview": true, + "requireLastPushApproval": true, + "requiredApprovingReviewCount": 2, + "requiredReviewThreadResolution": true + } + }, + { + "type": "REQUIRED_STATUS_CHECKS", + "parameters": { + "strictRequiredStatusChecksPolicy": true, + "requiredStatusChecks": [ + { "context": "build", "integrationID": 1 }, + { "context": "test", "integrationID": 1 } + ] + } + } + ]} + } + ] + } + } + } +}` + + // Tag refs (must ONLY include fields that `branch` selected via tagsQuery2 -> name) + tagsJSONCase1 := ` +{ + "data": { + "repository": { + "refs": { + "nodes": [ + { "name": "v1.2.3" }, + { "name": "v0.9.0" } + ] + } + } + } +}` + + // CASE 2 — Another tag with only status checks via a matching ruleset (no PR rule). + // Strict=false and single context "ci". + rulesJSONCase2 := ` +{ + "data": { + "repository": { + "rulesets": { + "nodes": [ + { + "name": "Release checks", + "enforcement": "ACTIVE", + "target": "TAG", + "conditions": { "refName": { "include": ["release-*"], "exclude": [] } }, + "bypassActors": { "nodes": [] }, + "rules": { "nodes": [ + { + "type": "REQUIRED_STATUS_CHECKS", + "parameters": { + "strictRequiredStatusChecksPolicy": false, + "requiredStatusChecks": [ + { "context": "ci", "integrationID": 1 } + ] + } + } + ]} + } + ] + } + } + } +}` + + tagsJSONCase2 := ` +{ + "data": { + "repository": { + "refs": { + "nodes": [ + { "name": "release-2024.10" } + ] + } + } + } +}` + + // CASE 3 — No ruleset matches tag "v1.9.9"; ≥3 rulesets present; each ruleset has 2 rules. + rulesJSONCase3 := ` +{ + "data": { + "repository": { + "rulesets": { + "nodes": [ + { + "name": "Only v2 tags", + "enforcement": "ACTIVE", + "target": "TAG", + "conditions": { "refName": { "include": ["v2.*"], "exclude": [] } }, + "bypassActors": { "nodes": [] }, + "rules": { "nodes": [ + { + "type": "PULL_REQUEST", + "parameters": { + "requiredApprovingReviewCount": 2 + } + }, + { + "type": "REQUIRED_STATUS_CHECKS", + "parameters": { + "strictRequiredStatusChecksPolicy": true, + "requiredStatusChecks": [ + { "context": "build", "integrationID": 1 } + ] + } + } + ] } + }, + { + "name": "Beta rules", + "enforcement": "ACTIVE", + "target": "TAG", + "conditions": { "refName": { "include": ["beta-*"], "exclude": [] } }, + "bypassActors": { "nodes": [] }, + "rules": { "nodes": [ + { + "type": "PULL_REQUEST", + "parameters": { + "requireCodeOwnerReview": true + } + }, + { + "type": "REQUIRED_STATUS_CHECKS", + "parameters": { + "strictRequiredStatusChecksPolicy": false, + "requiredStatusChecks": [ + { "context": "ci", "integrationID": 1 } + ] + } + } + ] } + }, + { + "name": "Exclude v1 series", + "enforcement": "ACTIVE", + "target": "TAG", + "conditions": { "refName": { "include": ["*"], "exclude": ["v1.*"] } }, + "bypassActors": { "nodes": [] }, + "rules": { "nodes": [ + { + "type": "PULL_REQUEST", + "parameters": { + "dismissStaleReviewsOnPush": true + } + }, + { + "type": "REQUIRED_STATUS_CHECKS", + "parameters": { + "strictRequiredStatusChecksPolicy": true, + "requiredStatusChecks": [ + { "context": "lint", "integrationID": 1 } + ] + } + } + ] } + } + ] + } + } + } +}` + + tagsJSONCase3 := ` +{ + "data": { + "repository": { + "refs": { + "nodes": [ + { "name": "v1.9.9" } + ] + } + } + } +}` + + // CASE 4 — PR-only ruleset applies to "beta-2025.01"; ≥3 rulesets total; matching ruleset has 2 PR rules. + rulesJSONCase4 := ` +{ + "data": { + "repository": { + "rulesets": { + "nodes": [ + { + "name": "Beta PR gate", + "enforcement": "ACTIVE", + "target": "TAG", + "conditions": { "refName": { "include": ["beta-*"], "exclude": [] } }, + "bypassActors": { "nodes": [] }, + "rules": { "nodes": [ + { + "type": "PULL_REQUEST", + "parameters": { + "requiredApprovingReviewCount": 1 + } + }, + { + "type": "PULL_REQUEST", + "parameters": { + "dismissStaleReviewsOnPush": true + } + } + ] } + }, + { + "name": "General checks for v2", + "enforcement": "ACTIVE", + "target": "TAG", + "conditions": { "refName": { "include": ["v2.*"], "exclude": [] } }, + "bypassActors": { "nodes": [] }, + "rules": { "nodes": [ + { + "type": "REQUIRED_STATUS_CHECKS", + "parameters": { + "strictRequiredStatusChecksPolicy": true, + "requiredStatusChecks": [ + { "context": "unit", "integrationID": 1 } + ] + } + }, + { + "type": "REQUIRED_STATUS_CHECKS", + "parameters": { + "strictRequiredStatusChecksPolicy": true, + "requiredStatusChecks": [ + { "context": "integration", "integrationID": 1 } + ] + } + } + ] } + }, + { + "name": "Exclude betas", + "enforcement": "ACTIVE", + "target": "TAG", + "conditions": { "refName": { "include": ["*"], "exclude": ["beta-*"] } }, + "bypassActors": { "nodes": [] }, + "rules": { "nodes": [ + { + "type": "PULL_REQUEST", + "parameters": { + "requireCodeOwnerReview": true + } + }, + { + "type": "REQUIRED_STATUS_CHECKS", + "parameters": { + "strictRequiredStatusChecksPolicy": true, + "requiredStatusChecks": [ + { "context": "lint", "integrationID": 1 } + ] + } + } + ] } + } + ] + } + } + } +}` + + tagsJSONCase4 := ` +{ + "data": { + "repository": { + "refs": { + "nodes": [ + { "name": "beta-2025.01" } + ] + } + } + } +}` + + tests := []struct { + wantCodeOwners *bool + wantRequireLinearHistory *bool + wantStatusChecksStrictUpToDate *bool + wantStatusChecksRequired *bool + wantRequireLastPushApproval *bool + wantEnforceAdmins *bool + wantDismissStale *bool + wantPRApprovals *int32 + wantAllowDeletions *bool + wantAllowForcePushes *bool + wantPRRequired *bool + tag string + rulesJSON string + wantName string + name string + repo string + owner string + tagsJSON string + wantStatusChecksContextsUnordered []string + wantProtected bool + }{ + { + name: "ruleset-applies-v1.2.3", + rulesJSON: rulesJSONCase1, + tagsJSON: tagsJSONCase1, + owner: "o", + repo: "r", + tag: "v1.2.3", + + wantName: "v1.2.3", + wantProtected: true, + // No BPR/RUR in tags JSON -> these toggles remain nil + wantEnforceAdmins: nil, + wantAllowDeletions: nil, + wantAllowForcePushes: nil, + wantRequireLinearHistory: nil, + + // From ruleset: + wantPRRequired: boolPtr(true), + wantPRApprovals: int32Ptr(2), + wantDismissStale: boolPtr(true), + wantCodeOwners: boolPtr(true), + wantRequireLastPushApproval: boolPtr(true), + wantStatusChecksRequired: boolPtr(true), + wantStatusChecksStrictUpToDate: boolPtr(true), + wantStatusChecksContextsUnordered: []string{"build", "test"}, + }, + { + name: "release-tag-status-checks-only", + rulesJSON: rulesJSONCase2, + tagsJSON: tagsJSONCase2, + owner: "o", + repo: "r", + tag: "release-2024.10", + + wantName: "release-2024.10", + wantProtected: true, + wantEnforceAdmins: nil, + wantAllowDeletions: nil, + wantAllowForcePushes: nil, + wantRequireLinearHistory: nil, + + // From ruleset (no PR rules): + wantPRRequired: nil, + wantPRApprovals: nil, + wantDismissStale: nil, + wantCodeOwners: nil, + wantRequireLastPushApproval: nil, + wantStatusChecksRequired: boolPtr(true), + wantStatusChecksStrictUpToDate: boolPtr(false), + wantStatusChecksContextsUnordered: []string{"ci"}, + }, + { + name: "no-ruleset-match-unprotected", + rulesJSON: rulesJSONCase3, + tagsJSON: tagsJSONCase3, + owner: "o", + repo: "r", + tag: "v1.9.9", + + wantName: "v1.9.9", + wantProtected: false, // none of the 3 rulesets apply + wantEnforceAdmins: nil, + wantAllowDeletions: nil, + wantAllowForcePushes: nil, + wantRequireLinearHistory: nil, + + wantPRRequired: nil, + wantPRApprovals: nil, + wantDismissStale: nil, + wantCodeOwners: nil, + wantRequireLastPushApproval: nil, + wantStatusChecksRequired: nil, + wantStatusChecksStrictUpToDate: nil, + wantStatusChecksContextsUnordered: nil, + }, + { + name: "pr-only-two-rules-beta", + rulesJSON: rulesJSONCase4, + tagsJSON: tagsJSONCase4, + owner: "o", + repo: "r", + tag: "beta-2025.01", + + wantName: "beta-2025.01", + wantProtected: true, + wantEnforceAdmins: nil, + wantAllowDeletions: nil, + wantAllowForcePushes: nil, + wantRequireLinearHistory: nil, + + // From matching ruleset's two PR rules: + wantPRRequired: boolPtr(true), + wantPRApprovals: nil, + wantDismissStale: boolPtr(true), + wantCodeOwners: nil, + wantRequireLastPushApproval: nil, + + wantStatusChecksRequired: nil, + wantStatusChecksStrictUpToDate: nil, + wantStatusChecksContextsUnordered: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + graph := newGraphQLClientWith(tc.rulesJSON, tc.tagsJSON) + + var h tagsHandler + h.graphClient = graph + ctx := context.Background() + h.init(ctx, &Repo{owner: tc.owner, repo: tc.repo}) + + if err := h.setup(); err != nil { + t.Fatalf("setup() error: %v", err) + } + + got, err := h.getTag(tc.tag) + if err != nil { + t.Fatalf("getTag(%q) error: %v", tc.tag, err) + } + if got == nil { + t.Fatalf("nil RepoRef") + } + + // Name / Protected + if got.Name == nil || *got.Name != tc.wantName { + t.Fatalf("Name = %v, want %q", derefStr(got.Name), tc.wantName) + } + if got.Protected == nil || *got.Protected != tc.wantProtected { + t.Fatalf("Protected = %v, want %v", derefBool(got.Protected), tc.wantProtected) + } + + // Low-level toggles (should stay nil without BPR/RUR in the JSON) + assertOptBool(t, "EnforceAdmins", got.ProtectionRule.EnforceAdmins, tc.wantEnforceAdmins) + assertOptBool(t, "AllowDeletions", got.ProtectionRule.AllowDeletions, tc.wantAllowDeletions) + assertOptBool(t, "AllowForcePushes", got.ProtectionRule.AllowForcePushes, tc.wantAllowForcePushes) + assertOptBool(t, "RequireLinearHistory", got.ProtectionRule.RequireLinearHistory, tc.wantRequireLinearHistory) + + // PR Rule expectations + assertOptBool(t, "PR.Required", got.ProtectionRule.PullRequestRule.Required, tc.wantPRRequired) + assertOptInt32(t, "PR.RequiredApprovingReviewCount", got.ProtectionRule.PullRequestRule.RequiredApprovingReviewCount, tc.wantPRApprovals) + assertOptBool(t, "PR.DismissStaleReviews", got.ProtectionRule.PullRequestRule.DismissStaleReviews, tc.wantDismissStale) + assertOptBool(t, "PR.RequireCodeOwnerReviews", got.ProtectionRule.PullRequestRule.RequireCodeOwnerReviews, tc.wantCodeOwners) + assertOptBool(t, "RequireLastPushApproval", got.ProtectionRule.RequireLastPushApproval, tc.wantRequireLastPushApproval) + + // Status checks expectations + assertOptBool(t, "Checks.RequiresStatusChecks", got.ProtectionRule.CheckRules.RequiresStatusChecks, tc.wantStatusChecksRequired) + assertOptBool(t, "Checks.UpToDateBeforeMerge", got.ProtectionRule.CheckRules.UpToDateBeforeMerge, tc.wantStatusChecksStrictUpToDate) + + // Contexts (order-insensitive) + if tc.wantStatusChecksContextsUnordered != nil { + gotCtx := append([]string(nil), got.ProtectionRule.CheckRules.Contexts...) + want := set(tc.wantStatusChecksContextsUnordered...) + if !sameSet(gotCtx, want) { + t.Fatalf("Checks.Contexts = %v, want set %v", gotCtx, tc.wantStatusChecksContextsUnordered) + } + } + }) + } +} + +/********************** + * Small helpers + **********************/ + +func boolPtr(b bool) *bool { return &b } +func int32Ptr(i int32) *int32 { return &i } + +func derefStr(p *string) string { + if p == nil { + return "" + } + return *p +} + +func derefBool(p *bool) bool { + if p == nil { + return false + } + return *p +} + +func assertOptBool(t *testing.T, field string, got, want *bool) { + t.Helper() + // If we don't care (want is nil), don't assert. + if want == nil { + return + } + if got == nil { + t.Fatalf("%s = , want %v", field, *want) + } + if *got != *want { + t.Fatalf("%s = %v, want %v", field, *got, *want) + } +} + +func assertOptInt32(t *testing.T, field string, got, want *int32) { + t.Helper() + // If we don't care (want is nil), don't assert. + if want == nil { + return + } + if got == nil { + t.Fatalf("%s = , want %v", field, *want) + } + if *got != *want { + t.Fatalf("%s = %v, want %v", field, *got, *want) + } +} + +func set(ss ...string) map[string]struct{} { + m := make(map[string]struct{}, len(ss)) + for _, s := range ss { + m[s] = struct{}{} + } + return m +} + +func sameSet(got []string, want map[string]struct{}) bool { + if len(got) != len(want) { + return false + } + for _, s := range got { + if _, ok := want[s]; !ok { + return false + } + } + return true +} diff --git a/clients/gitlabrepo/branches.go b/clients/gitlabrepo/branches.go index 9e19f1995f0..438ba5c281a 100644 --- a/clients/gitlabrepo/branches.go +++ b/clients/gitlabrepo/branches.go @@ -30,7 +30,7 @@ type branchesHandler struct { once *sync.Once errSetup error repourl *Repo - defaultBranchRef *clients.BranchRef + defaultBranchRef *clients.RepoRef queryProject fnProject queryBranch fnQueryBranch getProtectedBranch fnProtectedBranch @@ -108,7 +108,7 @@ func (handler *branchesHandler) setup() error { handler.defaultBranchRef = makeBranchRefFrom(branch, protectedBranch, projectStatusChecks, projectApprovalRule) } else { - handler.defaultBranchRef = &clients.BranchRef{ + handler.defaultBranchRef = &clients.RepoRef{ Name: &branch.Name, Protected: &branch.Protected, } @@ -118,7 +118,7 @@ func (handler *branchesHandler) setup() error { return handler.errSetup } -func (handler *branchesHandler) getDefaultBranch() (*clients.BranchRef, error) { +func (handler *branchesHandler) getDefaultBranch() (*clients.RepoRef, error) { err := handler.setup() if err != nil { return nil, fmt.Errorf("error during branchesHandler.setup: %w", err) @@ -127,11 +127,11 @@ func (handler *branchesHandler) getDefaultBranch() (*clients.BranchRef, error) { return handler.defaultBranchRef, nil } -func (handler *branchesHandler) getBranch(branch string) (*clients.BranchRef, error) { +func (handler *branchesHandler) getBranch(branch string) (*clients.RepoRef, error) { if strings.Contains(branch, "/-/commit/") { // Gitlab's release commitish contains commit and is not easily tied to specific branch p, b := true, "" - ret := &clients.BranchRef{ + ret := &clients.RepoRef{ Name: &b, Protected: &p, } @@ -162,7 +162,7 @@ func (handler *branchesHandler) getBranch(branch string) (*clients.BranchRef, er return makeBranchRefFrom(bran, protectedBranch, projectStatusChecks, projectApprovalRule), nil } else { - ret := &clients.BranchRef{ + ret := &clients.RepoRef{ Name: &bran.Name, Protected: &bran.Protected, } @@ -181,7 +181,7 @@ func makeContextsFromResp(checks []*gitlab.ProjectStatusCheck) []string { func makeBranchRefFrom(branch *gitlab.Branch, protectedBranch *gitlab.ProtectedBranch, projectStatusChecks []*gitlab.ProjectStatusCheck, projectApprovalRule *gitlab.ProjectApprovals, -) *clients.BranchRef { +) *clients.RepoRef { requiresStatusChecks := newFalse() if len(projectStatusChecks) > 0 { requiresStatusChecks = newTrue() @@ -204,10 +204,10 @@ func makeBranchRefFrom(branch *gitlab.Branch, protectedBranch *gitlab.ProtectedB pullRequestReviewRule.RequiredApprovingReviewCount = &requiredApprovalNum } - ret := &clients.BranchRef{ + ret := &clients.RepoRef{ Name: &branch.Name, Protected: &branch.Protected, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: pullRequestReviewRule, AllowDeletions: newFalse(), AllowForcePushes: &protectedBranch.AllowForcePush, diff --git a/clients/gitlabrepo/client.go b/clients/gitlabrepo/client.go index 40289969a97..53724485608 100644 --- a/clients/gitlabrepo/client.go +++ b/clients/gitlabrepo/client.go @@ -220,7 +220,7 @@ func (client *Client) IsArchived() (bool, error) { return client.project.isArchived() } -func (client *Client) GetDefaultBranch() (*clients.BranchRef, error) { +func (client *Client) GetDefaultBranch() (*clients.RepoRef, error) { return client.branches.getDefaultBranch() } @@ -228,7 +228,7 @@ func (client *Client) GetDefaultBranchName() (string, error) { return client.repourl.defaultBranch, nil } -func (client *Client) GetBranch(branch string) (*clients.BranchRef, error) { +func (client *Client) GetBranch(branch string) (*clients.RepoRef, error) { return client.branches.getBranch(branch) } @@ -277,6 +277,16 @@ func (client *Client) Close() error { return nil } +// GetBranch implements RepoClient.GetTag. +func (client *Client) GetTag(tag string) (*clients.RepoRef, error) { + return &clients.RepoRef{}, fmt.Errorf("GetTag: %w", clients.ErrUnsupportedFeature) +} + +func (client *Client) ListTags() ([]*clients.RepoRef, error) { + tags := make([]*clients.RepoRef, 0) + return tags, fmt.Errorf("ListTags: %w", clients.ErrUnsupportedFeature) +} + func CreateGitlabClient(ctx context.Context, host string) (clients.RepoClient, error) { token := os.Getenv("GITLAB_AUTH_TOKEN") return CreateGitlabClientWithToken(ctx, token, host) diff --git a/clients/localdir/client.go b/clients/localdir/client.go index e3c18eb4c15..cc1cb0f330f 100644 --- a/clients/localdir/client.go +++ b/clients/localdir/client.go @@ -180,15 +180,25 @@ func (client *Client) GetFileReader(filename string) (io.ReadCloser, error) { } // GetBranch implements RepoClient.GetBranch. -func (client *Client) GetBranch(branch string) (*clients.BranchRef, error) { +func (client *Client) GetBranch(branch string) (*clients.RepoRef, error) { return nil, fmt.Errorf("ListBranches: %w", clients.ErrUnsupportedFeature) } // GetDefaultBranch implements RepoClient.GetDefaultBranch. -func (client *Client) GetDefaultBranch() (*clients.BranchRef, error) { +func (client *Client) GetDefaultBranch() (*clients.RepoRef, error) { return nil, fmt.Errorf("GetDefaultBranch: %w", clients.ErrUnsupportedFeature) } +// GetBranch implements RepoClient.GetTag. +func (client *Client) GetTag(tag string) (*clients.RepoRef, error) { + return &clients.RepoRef{}, fmt.Errorf("GetTag: %w", clients.ErrUnsupportedFeature) +} + +func (client *Client) ListTags() ([]*clients.RepoRef, error) { + tags := make([]*clients.RepoRef, 0) + return tags, fmt.Errorf("ListTags: %w", clients.ErrUnsupportedFeature) +} + // GetDefaultBranchName implements RepoClient.GetDefaultBranchName. func (client *Client) GetDefaultBranchName() (string, error) { return "", fmt.Errorf("GetDefaultBranchName: %w", clients.ErrUnsupportedFeature) diff --git a/clients/mockclients/repo_client.go b/clients/mockclients/repo_client.go index b028ba22864..0a4d490f7d7 100644 --- a/clients/mockclients/repo_client.go +++ b/clients/mockclients/repo_client.go @@ -73,10 +73,10 @@ func (mr *MockRepoClientMockRecorder) Close() *gomock.Call { } // GetBranch mocks base method. -func (m *MockRepoClient) GetBranch(branch string) (*clients.BranchRef, error) { +func (m *MockRepoClient) GetBranch(branch string) (*clients.RepoRef, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetBranch", branch) - ret0, _ := ret[0].(*clients.BranchRef) + ret0, _ := ret[0].(*clients.RepoRef) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -103,10 +103,10 @@ func (mr *MockRepoClientMockRecorder) GetCreatedAt() *gomock.Call { } // GetDefaultBranch mocks base method. -func (m *MockRepoClient) GetDefaultBranch() (*clients.BranchRef, error) { +func (m *MockRepoClient) GetDefaultBranch() (*clients.RepoRef, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetDefaultBranch") - ret0, _ := ret[0].(*clients.BranchRef) + ret0, _ := ret[0].(*clients.RepoRef) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -162,6 +162,21 @@ func (mr *MockRepoClientMockRecorder) GetOrgRepoClient(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrgRepoClient", reflect.TypeOf((*MockRepoClient)(nil).GetOrgRepoClient), arg0) } +// GetTag mocks base method. +func (m *MockRepoClient) GetTag(tag string) (*clients.RepoRef, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetTag", tag) + ret0, _ := ret[0].(*clients.RepoRef) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetTag indicates an expected call of GetTag. +func (mr *MockRepoClientMockRecorder) GetTag(tag any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTag", reflect.TypeOf((*MockRepoClient)(nil).GetTag), tag) +} + // InitRepo mocks base method. func (m *MockRepoClient) InitRepo(repo clients.Repo, commitSHA string, commitDepth int) error { m.ctrl.T.Helper() @@ -341,6 +356,21 @@ func (mr *MockRepoClientMockRecorder) ListSuccessfulWorkflowRuns(filename any) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListSuccessfulWorkflowRuns", reflect.TypeOf((*MockRepoClient)(nil).ListSuccessfulWorkflowRuns), filename) } +// ListTags mocks base method. +func (m *MockRepoClient) ListTags() ([]*clients.RepoRef, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListTags") + ret0, _ := ret[0].([]*clients.RepoRef) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListTags indicates an expected call of ListTags. +func (mr *MockRepoClientMockRecorder) ListTags() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListTags", reflect.TypeOf((*MockRepoClient)(nil).ListTags)) +} + // ListWebhooks mocks base method. func (m *MockRepoClient) ListWebhooks() ([]clients.Webhook, error) { m.ctrl.T.Helper() diff --git a/clients/ossfuzz/client.go b/clients/ossfuzz/client.go index 916ff6021b7..0c5fcf912f1 100644 --- a/clients/ossfuzz/client.go +++ b/clients/ossfuzz/client.go @@ -162,6 +162,16 @@ func (c *client) URI() string { return c.statusURL } +// GetBranch implements RepoClient.GetTag. +func (client *client) GetTag(tag string) (*clients.RepoRef, error) { + return &clients.RepoRef{}, fmt.Errorf("GetTag: %w", clients.ErrUnsupportedFeature) +} + +func (client *client) ListTags() ([]*clients.RepoRef, error) { + tags := make([]*clients.RepoRef, 0) + return tags, fmt.Errorf("ListTags: %w", clients.ErrUnsupportedFeature) +} + // InitRepo implements RepoClient.InitRepo. func (c *client) InitRepo(inputRepo clients.Repo, commitSHA string, commitDepth int) error { return fmt.Errorf("InitRepo: %w", clients.ErrUnsupportedFeature) @@ -188,12 +198,12 @@ func (c *client) GetFileReader(filename string) (io.ReadCloser, error) { } // GetBranch implements RepoClient.GetBranch. -func (c *client) GetBranch(branch string) (*clients.BranchRef, error) { +func (c *client) GetBranch(branch string) (*clients.RepoRef, error) { return nil, fmt.Errorf("GetBranch: %w", clients.ErrUnsupportedFeature) } // GetDefaultBranch implements RepoClient.GetDefaultBranch. -func (c *client) GetDefaultBranch() (*clients.BranchRef, error) { +func (c *client) GetDefaultBranch() (*clients.RepoRef, error) { return nil, fmt.Errorf("GetDefaultBranch: %w", clients.ErrUnsupportedFeature) } diff --git a/clients/repo_client.go b/clients/repo_client.go index d51661e9976..7d3fb0f0a5f 100644 --- a/clients/repo_client.go +++ b/clients/repo_client.go @@ -34,16 +34,18 @@ type RepoClient interface { URI() string IsArchived() (bool, error) ListFiles(predicate func(string) (bool, error)) ([]string, error) + ListTags() ([]*RepoRef, error) // Returns an absolute path to the local repository // in the format that matches the local OS LocalPath() (string, error) // GetFileReader returns an io.ReadCloser corresponding to the desired file. // Callers should ensure to Close the Reader when finished. GetFileReader(filename string) (io.ReadCloser, error) - GetBranch(branch string) (*BranchRef, error) + GetBranch(branch string) (*RepoRef, error) + GetTag(tag string) (*RepoRef, error) GetCreatedAt() (time.Time, error) GetDefaultBranchName() (string, error) - GetDefaultBranch() (*BranchRef, error) + GetDefaultBranch() (*RepoRef, error) GetOrgRepoClient(context.Context) (RepoClient, error) ListCommits() ([]Commit, error) ListIssues() ([]Issue, error) diff --git a/cron/internal/format/json_raw_results.go b/cron/internal/format/json_raw_results.go index 9f018ef5acd..5d62b931de3 100644 --- a/cron/internal/format/json_raw_results.go +++ b/cron/internal/format/json_raw_results.go @@ -208,16 +208,16 @@ func addBranchProtectionRawResults(r *jsonScorecardRawResult, bp *checker.Branch var bp *jsonBranchProtectionSettings if v.Protected != nil && *v.Protected { bp = &jsonBranchProtectionSettings{ - AllowsDeletions: v.BranchProtectionRule.AllowDeletions, - AllowsForcePushes: v.BranchProtectionRule.AllowForcePushes, - RequiresCodeOwnerReviews: v.BranchProtectionRule.PullRequestRule.RequireCodeOwnerReviews, - RequiresLinearHistory: v.BranchProtectionRule.RequireLinearHistory, - DismissesStaleReviews: v.BranchProtectionRule.PullRequestRule.DismissStaleReviews, - EnforcesAdmins: v.BranchProtectionRule.EnforceAdmins, - RequiresStatusChecks: v.BranchProtectionRule.CheckRules.RequiresStatusChecks, - RequiresUpToDateBranchBeforeMerging: v.BranchProtectionRule.CheckRules.UpToDateBeforeMerge, - RequiredApprovingReviewCount: v.BranchProtectionRule.PullRequestRule.RequiredApprovingReviewCount, - StatusCheckContexts: v.BranchProtectionRule.CheckRules.Contexts, + AllowsDeletions: v.ProtectionRule.AllowDeletions, + AllowsForcePushes: v.ProtectionRule.AllowForcePushes, + RequiresCodeOwnerReviews: v.ProtectionRule.PullRequestRule.RequireCodeOwnerReviews, + RequiresLinearHistory: v.ProtectionRule.RequireLinearHistory, + DismissesStaleReviews: v.ProtectionRule.PullRequestRule.DismissStaleReviews, + EnforcesAdmins: v.ProtectionRule.EnforceAdmins, + RequiresStatusChecks: v.ProtectionRule.CheckRules.RequiresStatusChecks, + RequiresUpToDateBranchBeforeMerging: v.ProtectionRule.CheckRules.UpToDateBeforeMerge, + RequiredApprovingReviewCount: v.ProtectionRule.PullRequestRule.RequiredApprovingReviewCount, + StatusCheckContexts: v.ProtectionRule.CheckRules.Contexts, } } r.Results.BranchProtections = append(r.Results.BranchProtections, jsonBranchProtection{ diff --git a/go.mod b/go.mod index f1fef06f661..3f0e965af03 100644 --- a/go.mod +++ b/go.mod @@ -47,6 +47,7 @@ require ( github.com/mcuadros/go-jsonschema-generator v0.0.0-20200330054847-ba7a369d4303 github.com/onsi/ginkgo/v2 v2.23.4 github.com/otiai10/copy v1.14.1 + github.com/stretchr/testify v1.10.0 gitlab.com/gitlab-org/api/client-go v0.134.0 sigs.k8s.io/release-utils v0.8.4 ) @@ -116,6 +117,7 @@ require ( github.com/pierrec/lz4/v4 v4.1.21 // indirect github.com/pjbgf/sha1cd v0.3.2 // indirect github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/prometheus v0.54.0 // indirect github.com/robfig/cron/v3 v3.0.1 // indirect github.com/skeema/knownhosts v1.3.1 // indirect diff --git a/pkg/scorecard/json_raw_results.go b/pkg/scorecard/json_raw_results.go index eab4118a4e7..65c41831aee 100644 --- a/pkg/scorecard/json_raw_results.go +++ b/pkg/scorecard/json_raw_results.go @@ -710,16 +710,16 @@ func (r *jsonScorecardRawResult) addBranchProtectionRawResults(bp *checker.Branc var bp *jsonBranchProtectionSettings if v.Protected != nil && *v.Protected { bp = &jsonBranchProtectionSettings{ - AllowsDeletions: v.BranchProtectionRule.AllowDeletions, - AllowsForcePushes: v.BranchProtectionRule.AllowForcePushes, - RequiresCodeOwnerReviews: v.BranchProtectionRule.PullRequestRule.RequireCodeOwnerReviews, - RequiresLinearHistory: v.BranchProtectionRule.RequireLinearHistory, - DismissesStaleReviews: v.BranchProtectionRule.PullRequestRule.DismissStaleReviews, - EnforcesAdmins: v.BranchProtectionRule.EnforceAdmins, - RequiresStatusChecks: v.BranchProtectionRule.CheckRules.RequiresStatusChecks, - RequiresUpToDateBranchBeforeMerging: v.BranchProtectionRule.CheckRules.UpToDateBeforeMerge, - RequiredApprovingReviewCount: v.BranchProtectionRule.PullRequestRule.RequiredApprovingReviewCount, - StatusCheckContexts: v.BranchProtectionRule.CheckRules.Contexts, + AllowsDeletions: v.ProtectionRule.AllowDeletions, + AllowsForcePushes: v.ProtectionRule.AllowForcePushes, + RequiresCodeOwnerReviews: v.ProtectionRule.PullRequestRule.RequireCodeOwnerReviews, + RequiresLinearHistory: v.ProtectionRule.RequireLinearHistory, + DismissesStaleReviews: v.ProtectionRule.PullRequestRule.DismissStaleReviews, + EnforcesAdmins: v.ProtectionRule.EnforceAdmins, + RequiresStatusChecks: v.ProtectionRule.CheckRules.RequiresStatusChecks, + RequiresUpToDateBranchBeforeMerging: v.ProtectionRule.CheckRules.UpToDateBeforeMerge, + RequiredApprovingReviewCount: v.ProtectionRule.PullRequestRule.RequiredApprovingReviewCount, + StatusCheckContexts: v.ProtectionRule.CheckRules.Contexts, } } branches = append(branches, jsonBranchProtection{ diff --git a/pkg/scorecard/json_raw_results_test.go b/pkg/scorecard/json_raw_results_test.go index d757a252f51..fdd2f96c9bf 100644 --- a/pkg/scorecard/json_raw_results_test.go +++ b/pkg/scorecard/json_raw_results_test.go @@ -1097,11 +1097,11 @@ func TestJsonScorecardRawResult(t *testing.T) { }, } bp := &checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: stringPtr("main"), Protected: boolPtr(true), - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: boolPtr(true), AllowForcePushes: boolPtr(false), PullRequestRule: clients.PullRequestRule{ @@ -1298,11 +1298,11 @@ func TestAddBranchProtectionRawResults(t *testing.T) { { name: "one protected branch", input: &checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: stringPtr("main"), Protected: boolPtr(true), - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: boolPtr(false), }, }, @@ -1370,7 +1370,7 @@ func TestFillJSONRawResults(t *testing.T) { }, }, BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ {Name: stringPtr("main"), Protected: boolPtr(true)}, }, }, diff --git a/probes/blocksDeleteOnBranches/impl.go b/probes/blocksDeleteOnBranches/impl.go index c86a9298d78..4a642e853dc 100644 --- a/probes/blocksDeleteOnBranches/impl.go +++ b/probes/blocksDeleteOnBranches/impl.go @@ -61,13 +61,13 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { var text string var outcome finding.Outcome switch { - case branch.BranchProtectionRule.AllowDeletions == nil: + case branch.ProtectionRule.AllowDeletions == nil: text = "could not determine whether branch is protected against deletion" outcome = finding.OutcomeNotAvailable - case *branch.BranchProtectionRule.AllowDeletions: + case *branch.ProtectionRule.AllowDeletions: text = fmt.Sprintf("'allow deletion' enabled on branch '%s'", *branch.Name) outcome = finding.OutcomeFalse - case !*branch.BranchProtectionRule.AllowDeletions: + case !*branch.ProtectionRule.AllowDeletions: text = fmt.Sprintf("'allow deletion' disabled on branch '%s'", *branch.Name) outcome = finding.OutcomeTrue default: diff --git a/probes/blocksDeleteOnBranches/impl_test.go b/probes/blocksDeleteOnBranches/impl_test.go index c0f271ea723..6c73b5507a8 100644 --- a/probes/blocksDeleteOnBranches/impl_test.go +++ b/probes/blocksDeleteOnBranches/impl_test.go @@ -45,10 +45,10 @@ func Test_Run(t *testing.T) { name: "One branch blocks branch deletion should result in one true outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, }, }, @@ -63,16 +63,16 @@ func Test_Run(t *testing.T) { name: "Two branches that block branch deletions should result in two true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, }, }, @@ -87,16 +87,16 @@ func Test_Run(t *testing.T) { name: "Two branches in total: One blocks branch deletion and one doesn't = 1 true & 1 false", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &trueVal, }, }, @@ -111,16 +111,16 @@ func Test_Run(t *testing.T) { name: "Two branches in total: One blocks branch deletion and one doesn't = 1 false & 1 true", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &trueVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &falseVal, }, }, @@ -135,16 +135,16 @@ func Test_Run(t *testing.T) { name: "Two branches in total: One blocks branch deletion and one lacks data = 1 false & 1 unavailable", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: &trueVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: nil, }, }, diff --git a/probes/blocksForcePushOnBranches/impl.go b/probes/blocksForcePushOnBranches/impl.go index 30ea3268529..b81c01f9dbb 100644 --- a/probes/blocksForcePushOnBranches/impl.go +++ b/probes/blocksForcePushOnBranches/impl.go @@ -61,13 +61,13 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { var text string var outcome finding.Outcome switch { - case branch.BranchProtectionRule.AllowForcePushes == nil: + case branch.ProtectionRule.AllowForcePushes == nil: text = "could not determine whether for push is allowed" outcome = finding.OutcomeNotAvailable - case *branch.BranchProtectionRule.AllowForcePushes: + case *branch.ProtectionRule.AllowForcePushes: text = fmt.Sprintf("'force pushes' enabled on branch '%s'", *branch.Name) outcome = finding.OutcomeFalse - case !*branch.BranchProtectionRule.AllowForcePushes: + case !*branch.ProtectionRule.AllowForcePushes: text = fmt.Sprintf("'force pushes' disabled on branch '%s'", *branch.Name) outcome = finding.OutcomeTrue default: diff --git a/probes/blocksForcePushOnBranches/impl_test.go b/probes/blocksForcePushOnBranches/impl_test.go index f2a3881ff8f..f05be5846ad 100644 --- a/probes/blocksForcePushOnBranches/impl_test.go +++ b/probes/blocksForcePushOnBranches/impl_test.go @@ -45,10 +45,10 @@ func Test_Run(t *testing.T) { name: "Blocks Force Push on 1/1 branches = 1 true outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: &falseVal, }, }, @@ -63,16 +63,16 @@ func Test_Run(t *testing.T) { name: "Blocks Force Push on 2/2 branches = 2 true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: &falseVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: &falseVal, }, }, @@ -87,16 +87,16 @@ func Test_Run(t *testing.T) { name: "Blocks Force Push on 1/2 branches = 1 true and 1 false outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: &falseVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: &trueVal, }, }, @@ -111,16 +111,16 @@ func Test_Run(t *testing.T) { name: "Blocks Force Push on 1/2 branches = 1 false and 1 true outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: &trueVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: &falseVal, }, }, @@ -135,16 +135,16 @@ func Test_Run(t *testing.T) { name: "Blocks Force Push on 0/2 branches, 1 branch lacks data = 1 false and 1 unavailable", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: &trueVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowForcePushes: nil, }, }, diff --git a/probes/branchProtectionAppliesToAdmins/impl.go b/probes/branchProtectionAppliesToAdmins/impl.go index b8cd21c37e2..2da80d66906 100644 --- a/probes/branchProtectionAppliesToAdmins/impl.go +++ b/probes/branchProtectionAppliesToAdmins/impl.go @@ -59,7 +59,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for i := range r.Branches { branch := &r.Branches[i] - p := branch.BranchProtectionRule.EnforceAdmins + p := branch.ProtectionRule.EnforceAdmins text, outcome, err := branchprotection.GetTextOutcomeFromBool(p, "branch protection settings apply to administrators", *branch.Name) diff --git a/probes/branchProtectionAppliesToAdmins/impl_test.go b/probes/branchProtectionAppliesToAdmins/impl_test.go index ac5502fcfb8..423287ea7b6 100644 --- a/probes/branchProtectionAppliesToAdmins/impl_test.go +++ b/probes/branchProtectionAppliesToAdmins/impl_test.go @@ -45,10 +45,10 @@ func Test_Run(t *testing.T) { name: "1 branch enforces protection rules on admins = 1 true outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ EnforceAdmins: &trueVal, }, }, @@ -63,16 +63,16 @@ func Test_Run(t *testing.T) { name: "1 branch enforces protection rules on admins = 2 true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ EnforceAdmins: &trueVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ EnforceAdmins: &trueVal, }, }, @@ -87,16 +87,16 @@ func Test_Run(t *testing.T) { name: "1 branch enforces protection rules on admins and 1 doesn't = 1 true & 1 false", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ EnforceAdmins: &trueVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ EnforceAdmins: &falseVal, }, }, @@ -111,16 +111,16 @@ func Test_Run(t *testing.T) { name: "1 branch does not enforce protection rules on admins and 1 does = 1 false & 1 true", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ EnforceAdmins: &falseVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ EnforceAdmins: &trueVal, }, }, @@ -135,16 +135,16 @@ func Test_Run(t *testing.T) { name: "1 branch does not enforce protection rules on admins and 1 doesn't have data = 1 false & 1 unavailable", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ EnforceAdmins: &falseVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ EnforceAdmins: nil, }, }, diff --git a/probes/branchesAreProtected/impl_test.go b/probes/branchesAreProtected/impl_test.go index 5bbc025ba22..b4bbab00d07 100644 --- a/probes/branchesAreProtected/impl_test.go +++ b/probes/branchesAreProtected/impl_test.go @@ -44,7 +44,7 @@ func Test_Run(t *testing.T) { name: "One branch. Protection unknown", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, Protected: nil, @@ -60,7 +60,7 @@ func Test_Run(t *testing.T) { name: "Two protected branches", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, Protected: &trueVal, @@ -80,7 +80,7 @@ func Test_Run(t *testing.T) { name: "Two branches. First is protected", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, Protected: &trueVal, @@ -100,7 +100,7 @@ func Test_Run(t *testing.T) { name: "Two branches. Second is protected", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, Protected: &falseVal, @@ -120,14 +120,14 @@ func Test_Run(t *testing.T) { name: "Two branches. First one is not protected, second unknown", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, Protected: &falseVal, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ AllowDeletions: nil, }, }, diff --git a/probes/dismissesStaleReviews/impl.go b/probes/dismissesStaleReviews/impl.go index fea575cade4..eba806788e1 100644 --- a/probes/dismissesStaleReviews/impl.go +++ b/probes/dismissesStaleReviews/impl.go @@ -59,7 +59,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for i := range r.Branches { branch := &r.Branches[i] - p := branch.BranchProtectionRule.PullRequestRule.DismissStaleReviews + p := branch.ProtectionRule.PullRequestRule.DismissStaleReviews text, outcome, err := branchprotection.GetTextOutcomeFromBool(p, "stale review dismissal", *branch.Name) diff --git a/probes/dismissesStaleReviews/impl_test.go b/probes/dismissesStaleReviews/impl_test.go index 6b6a417b3ed..0d7027c2567 100644 --- a/probes/dismissesStaleReviews/impl_test.go +++ b/probes/dismissesStaleReviews/impl_test.go @@ -45,10 +45,10 @@ func Test_Run(t *testing.T) { name: "Dismisses stale reviews on 1/1 branches", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &trueVal, }, @@ -65,10 +65,10 @@ func Test_Run(t *testing.T) { name: "Dismisses stale reviews on 2/2 branches", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &trueVal, }, @@ -76,7 +76,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &trueVal, }, @@ -93,10 +93,10 @@ func Test_Run(t *testing.T) { name: "Dismisses stale reviews on 1/2 branches - 1", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &trueVal, }, @@ -104,7 +104,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &falseVal, }, @@ -121,10 +121,10 @@ func Test_Run(t *testing.T) { name: "Dismisses stale reviews on 1/2 branches - 2", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &falseVal, }, @@ -132,7 +132,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &trueVal, }, @@ -149,10 +149,10 @@ func Test_Run(t *testing.T) { name: "Dismisses stale reviews on 0/2 branches", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &falseVal, }, @@ -160,7 +160,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: nil, }, diff --git a/probes/requiresApproversForPullRequests/impl.go b/probes/requiresApproversForPullRequests/impl.go index 4d16e945dea..66c3c4b7ebd 100644 --- a/probes/requiresApproversForPullRequests/impl.go +++ b/probes/requiresApproversForPullRequests/impl.go @@ -66,7 +66,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { nilMsg := fmt.Sprintf("could not determine whether branch '%s' has required approving review count", *branch.Name) falseMsg := fmt.Sprintf("branch '%s' does not require approvers", *branch.Name) - p := branch.BranchProtectionRule.PullRequestRule.RequiredApprovingReviewCount + p := branch.ProtectionRule.PullRequestRule.RequiredApprovingReviewCount f, err := finding.NewWith(fs, Probe, "", nil, finding.OutcomeNotAvailable) if err != nil { diff --git a/probes/requiresApproversForPullRequests/impl_test.go b/probes/requiresApproversForPullRequests/impl_test.go index 1f9166d2c05..2e402c90695 100644 --- a/probes/requiresApproversForPullRequests/impl_test.go +++ b/probes/requiresApproversForPullRequests/impl_test.go @@ -45,10 +45,10 @@ func Test_Run(t *testing.T) { name: "1 branch requires 1 reviewer = 1 true outcome = 1 true outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &oneVal, }, @@ -65,10 +65,10 @@ func Test_Run(t *testing.T) { name: "2 branch require 1 reviewer each = 2 true outcomes = 2 true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &oneVal, }, @@ -76,7 +76,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &oneVal, }, @@ -93,10 +93,10 @@ func Test_Run(t *testing.T) { name: "1 branch requires 1 reviewer and 1 branch requires 0 reviewers = 1 true and 1 false", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &oneVal, }, @@ -104,7 +104,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &zeroVal, }, @@ -121,10 +121,10 @@ func Test_Run(t *testing.T) { name: "1 branch requires 0 reviewers and 1 branch requires 1 reviewer = 1 false and 1 true", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &zeroVal, }, @@ -132,7 +132,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &oneVal, }, @@ -149,10 +149,10 @@ func Test_Run(t *testing.T) { name: "1 branch requires 0 reviewers and 1 branch lacks data = 1 false and 1 unavailable", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &zeroVal, }, @@ -160,7 +160,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: nil, }, diff --git a/probes/requiresCodeOwnersReview/impl.go b/probes/requiresCodeOwnersReview/impl.go index 68e9e638e97..2e5a2166923 100644 --- a/probes/requiresCodeOwnersReview/impl.go +++ b/probes/requiresCodeOwnersReview/impl.go @@ -58,7 +58,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for i := range r.Branches { branch := &r.Branches[i] - reqOwnerReviews := branch.BranchProtectionRule.PullRequestRule.RequireCodeOwnerReviews + reqOwnerReviews := branch.ProtectionRule.PullRequestRule.RequireCodeOwnerReviews var text string var outcome finding.Outcome diff --git a/probes/requiresCodeOwnersReview/impl_test.go b/probes/requiresCodeOwnersReview/impl_test.go index 1b89dc73fdd..4151cf41188 100644 --- a/probes/requiresCodeOwnersReview/impl_test.go +++ b/probes/requiresCodeOwnersReview/impl_test.go @@ -44,10 +44,10 @@ func Test_Run(t *testing.T) { name: "1 branch requires code owner reviews with files = 1 true outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, @@ -65,10 +65,10 @@ func Test_Run(t *testing.T) { name: "1 branch requires code owner reviews without files = 1 false outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, @@ -86,10 +86,10 @@ func Test_Run(t *testing.T) { name: "2 branches require code owner reviews with files = 2 true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, @@ -97,7 +97,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, @@ -115,10 +115,10 @@ func Test_Run(t *testing.T) { name: "2 branches require code owner reviews with files = 2 true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, @@ -126,7 +126,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, @@ -144,10 +144,10 @@ func Test_Run(t *testing.T) { name: "1 branches require code owner reviews and 1 branch doesn't with files = 1 true 1 false", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, @@ -155,7 +155,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &falseVal, }, @@ -173,10 +173,10 @@ func Test_Run(t *testing.T) { name: "Requires code owner reviews on 1/2 branches - without files = 1 true and 1 false", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, @@ -184,7 +184,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &falseVal, }, @@ -202,10 +202,10 @@ func Test_Run(t *testing.T) { name: "Requires code owner reviews on 1/2 branches - with files = 1 false and 1 true", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &falseVal, }, @@ -213,7 +213,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, @@ -231,10 +231,10 @@ func Test_Run(t *testing.T) { name: "Requires code owner reviews on 1/2 branches - without files = 2 false", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &falseVal, }, @@ -242,7 +242,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, @@ -260,10 +260,10 @@ func Test_Run(t *testing.T) { name: "1 branch does not require code owner review and 1 lacks data = 1 false and 1 unavailable", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &falseVal, }, @@ -271,7 +271,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: nil, }, diff --git a/probes/requiresLastPushApproval/impl.go b/probes/requiresLastPushApproval/impl.go index 0cedbeace0e..f0ddd9a103b 100644 --- a/probes/requiresLastPushApproval/impl.go +++ b/probes/requiresLastPushApproval/impl.go @@ -59,7 +59,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for i := range r.Branches { branch := &r.Branches[i] - p := branch.BranchProtectionRule.RequireLastPushApproval + p := branch.ProtectionRule.RequireLastPushApproval text, outcome, err := branchprotection.GetTextOutcomeFromBool(p, "last push approval", *branch.Name) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) diff --git a/probes/requiresLastPushApproval/impl_test.go b/probes/requiresLastPushApproval/impl_test.go index f5c6803ae15..d7a1c60f889 100644 --- a/probes/requiresLastPushApproval/impl_test.go +++ b/probes/requiresLastPushApproval/impl_test.go @@ -44,10 +44,10 @@ func Test_Run(t *testing.T) { name: "1 branch requires last push approval = 1 true outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ RequireLastPushApproval: &trueVal, }, }, @@ -62,16 +62,16 @@ func Test_Run(t *testing.T) { name: "2 branches requires last push approval = 2 true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ RequireLastPushApproval: &trueVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ RequireLastPushApproval: &trueVal, }, }, @@ -86,16 +86,16 @@ func Test_Run(t *testing.T) { name: "Last push approval enabled on 1/2 branches = 1 true and 1 false outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ RequireLastPushApproval: &trueVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ RequireLastPushApproval: &falseVal, }, }, @@ -110,16 +110,16 @@ func Test_Run(t *testing.T) { name: "Last push approval enabled on 1/2 branches = 1 false and 1 true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ RequireLastPushApproval: &falseVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ RequireLastPushApproval: &trueVal, }, }, @@ -134,16 +134,16 @@ func Test_Run(t *testing.T) { name: "1 branch does not require last push approval and 1 lacks data = 1 false and 1 unavailable", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ RequireLastPushApproval: &falseVal, }, }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ RequireLastPushApproval: nil, }, }, diff --git a/probes/requiresPRsToChangeCode/impl.go b/probes/requiresPRsToChangeCode/impl.go index 8d7bcf1becc..b9d88ea6aef 100644 --- a/probes/requiresPRsToChangeCode/impl.go +++ b/probes/requiresPRsToChangeCode/impl.go @@ -68,7 +68,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { "If you think it might be the latter, make sure to run Scorecard with a PAT or use Repo " + "Rules (that are always public) instead of Branch Protection settings" - p := branch.BranchProtectionRule.PullRequestRule.Required + p := branch.ProtectionRule.PullRequestRule.Required f, err := finding.NewWith(fs, Probe, "", nil, finding.OutcomeNotAvailable) if err != nil { diff --git a/probes/requiresPRsToChangeCode/impl_test.go b/probes/requiresPRsToChangeCode/impl_test.go index 12d882dc020..955b5a12eeb 100644 --- a/probes/requiresPRsToChangeCode/impl_test.go +++ b/probes/requiresPRsToChangeCode/impl_test.go @@ -43,10 +43,10 @@ func Test_Run(t *testing.T) { name: "1 branch requires PRs to change code", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ Required: &trueVal, }, @@ -63,10 +63,10 @@ func Test_Run(t *testing.T) { name: "2 branches require PRs to change code = 2 true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ Required: &trueVal, }, @@ -74,7 +74,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ Required: &trueVal, }, @@ -91,10 +91,10 @@ func Test_Run(t *testing.T) { name: "1 branches require PRs to change code and 1 branch doesn't = 1 true 1 false", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ Required: &trueVal, }, @@ -102,7 +102,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, @@ -119,10 +119,10 @@ func Test_Run(t *testing.T) { name: "Requires PRs to change code on 1/2 branches = 1 false and 1 true", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, @@ -130,7 +130,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ Required: &trueVal, }, @@ -147,10 +147,10 @@ func Test_Run(t *testing.T) { name: "1 branch does not require PRs to change code and 1 lacks data = 1 false and 1 unavailable", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, @@ -158,7 +158,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ PullRequestRule: clients.PullRequestRule{ Required: nil, }, diff --git a/probes/requiresUpToDateBranches/impl.go b/probes/requiresUpToDateBranches/impl.go index 8c86839f750..1f2a2da2d1c 100644 --- a/probes/requiresUpToDateBranches/impl.go +++ b/probes/requiresUpToDateBranches/impl.go @@ -59,7 +59,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for i := range r.Branches { branch := &r.Branches[i] - p := branch.BranchProtectionRule.CheckRules.UpToDateBeforeMerge + p := branch.ProtectionRule.CheckRules.UpToDateBeforeMerge text, outcome, err := branchprotection.GetTextOutcomeFromBool(p, "up-to-date branches", *branch.Name) diff --git a/probes/requiresUpToDateBranches/impl_test.go b/probes/requiresUpToDateBranches/impl_test.go index 93b0aeaa521..c0cf160c855 100644 --- a/probes/requiresUpToDateBranches/impl_test.go +++ b/probes/requiresUpToDateBranches/impl_test.go @@ -44,10 +44,10 @@ func Test_Run(t *testing.T) { name: "1 branch requires up-to-date before merge = 1 true outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ UpToDateBeforeMerge: &trueVal, }, @@ -64,10 +64,10 @@ func Test_Run(t *testing.T) { name: "2 branches require up-to-date before merge = 2 true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ UpToDateBeforeMerge: &trueVal, }, @@ -75,7 +75,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ UpToDateBeforeMerge: &trueVal, }, @@ -92,10 +92,10 @@ func Test_Run(t *testing.T) { name: "Requires up to date branches on 1/2 branches = 1 true and 1 false outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ UpToDateBeforeMerge: &trueVal, }, @@ -103,7 +103,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ UpToDateBeforeMerge: &falseVal, }, @@ -120,10 +120,10 @@ func Test_Run(t *testing.T) { name: "Requires up to date branches on 1/2 branches = 1 false and 1 true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ UpToDateBeforeMerge: &falseVal, }, @@ -131,7 +131,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ UpToDateBeforeMerge: &trueVal, }, @@ -148,10 +148,10 @@ func Test_Run(t *testing.T) { name: "1 branch does no require up-to-date before merge and 1 branch lacks data= 1 true & 1 unavailable", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ UpToDateBeforeMerge: &falseVal, }, @@ -159,7 +159,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ UpToDateBeforeMerge: nil, }, diff --git a/probes/runsStatusChecksBeforeMerging/impl.go b/probes/runsStatusChecksBeforeMerging/impl.go index a1d4fcddc1c..21081e149a3 100644 --- a/probes/runsStatusChecksBeforeMerging/impl.go +++ b/probes/runsStatusChecksBeforeMerging/impl.go @@ -61,7 +61,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { var err error switch { - case len(branch.BranchProtectionRule.CheckRules.Contexts) > 0: + case len(branch.ProtectionRule.CheckRules.Contexts) > 0: f, err = finding.NewWith(fs, Probe, fmt.Sprintf("status check found to merge onto on branch '%s'", *branch.Name), nil, finding.OutcomeTrue) diff --git a/probes/runsStatusChecksBeforeMerging/impl_test.go b/probes/runsStatusChecksBeforeMerging/impl_test.go index 16f7ce4bc21..0a86af52927 100644 --- a/probes/runsStatusChecksBeforeMerging/impl_test.go +++ b/probes/runsStatusChecksBeforeMerging/impl_test.go @@ -43,10 +43,10 @@ func Test_Run(t *testing.T) { name: "Runs status checks on 1/1 branches with contexts = 1 true outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ Contexts: []string{"foo"}, }, @@ -63,10 +63,10 @@ func Test_Run(t *testing.T) { name: "Runs status checks on 2/2 branches with contexts = 2 true outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ Contexts: []string{"foo"}, }, @@ -74,7 +74,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ Contexts: []string{"foo"}, }, @@ -91,10 +91,10 @@ func Test_Run(t *testing.T) { name: "Runs status checks on 1/2 branches = 1 true and 1 false outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ Contexts: []string{"foo"}, }, @@ -102,7 +102,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ Contexts: []string{}, }, @@ -119,10 +119,10 @@ func Test_Run(t *testing.T) { name: "Runs status checks on 1/2 branches = 1 false and 1 true outcome", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ Contexts: []string{}, }, @@ -130,7 +130,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ Contexts: []string{"foo"}, }, @@ -147,10 +147,10 @@ func Test_Run(t *testing.T) { name: "Runs status checks on 0/2 branches = 2 false outcomes", raw: &checker.RawResults{ BranchProtectionResults: checker.BranchProtectionsData{ - Branches: []clients.BranchRef{ + Branches: []clients.RepoRef{ { Name: &branchVal1, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ Contexts: []string{}, }, @@ -158,7 +158,7 @@ func Test_Run(t *testing.T) { }, { Name: &branchVal2, - BranchProtectionRule: clients.BranchProtectionRule{ + ProtectionRule: clients.ProtectionRule{ CheckRules: clients.StatusChecksRule{ Contexts: []string{}, },