From 4ce8f3b38ceb361a5763d8b3462b95a19fc3e334 Mon Sep 17 00:00:00 2001 From: packet-mover Date: Mon, 6 Apr 2026 12:50:50 +0200 Subject: [PATCH] feat: add branch protection rules 9-11 Add HasBranchProtection, HasRequiredReviewers, HasRequiredStatusChecks. All three rules use a single GetBranchProtection API call per repo. 404 (no protection) is handled as nil, not an error. This completes all 11 rules defined in the spec. Co-Authored-By: Claude Opus 4.6 (1M context) --- client.go | 63 +++++++++++++++++++++++++++++++++++++++---- client_mock_test.go | 22 +++++++++++---- rules.go | 27 +++++++++++++++++++ rules_test.go | 66 +++++++++++++++++++++++++++++++++++++++++++++ scanner.go | 6 +++++ 5 files changed, 174 insertions(+), 10 deletions(-) diff --git a/client.go b/client.go index 90ba36e..ea9d536 100644 --- a/client.go +++ b/client.go @@ -15,13 +15,20 @@ type FileEntry struct { Type string // "blob" (file) or "tree" (directory) } +// BranchProtection holds the branch protection settings the scanner needs. +type BranchProtection struct { + RequiredReviewers int + RequiredStatusChecks []string +} + // Repo represents a GitHub repository with the fields the scanner needs. type Repo struct { - Name string - Description string - DefaultBranch string - Archived bool - Files []FileEntry // all files and directories in the repo + Name string + Description string + DefaultBranch string + Archived bool + Files []FileEntry // all files and directories in the repo + BranchProtection *BranchProtection // nil if no protection configured } // GitHubClient is the interface for all GitHub API interactions. @@ -29,6 +36,7 @@ type Repo struct { type GitHubClient interface { ListRepos(ctx context.Context, org string) ([]Repo, error) GetTree(ctx context.Context, owner, repo, branch string) ([]FileEntry, error) + GetBranchProtection(ctx context.Context, owner, repo, branch string) (*BranchProtection, error) CreateIssue(ctx context.Context, owner, repo, title, body string) error } @@ -130,6 +138,51 @@ func (c *realGitHubClient) GetTree(ctx context.Context, owner, repo, branch stri return files, nil } +func (c *realGitHubClient) GetBranchProtection(ctx context.Context, owner, repo, branch string) (*BranchProtection, error) { + url := fmt.Sprintf("https://api.github.com/repos/%s/%s/branches/%s/protection", owner, repo, branch) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, fmt.Errorf("create request for %s: %w", url, err) + } + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Accept", "application/vnd.github+json") + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("request %s: %w", url, err) + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusNotFound { + return nil, nil + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("get branch protection for %s/%s: status %d", owner, repo, resp.StatusCode) + } + + var result struct { + RequiredPullRequestReviews *struct { + RequiredApprovingReviewCount int `json:"required_approving_review_count"` + } `json:"required_pull_request_reviews"` + RequiredStatusChecks *struct { + Contexts []string `json:"contexts"` + } `json:"required_status_checks"` + } + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return nil, fmt.Errorf("decode branch protection for %s/%s: %w", owner, repo, err) + } + + bp := &BranchProtection{} + if result.RequiredPullRequestReviews != nil { + bp.RequiredReviewers = result.RequiredPullRequestReviews.RequiredApprovingReviewCount + } + if result.RequiredStatusChecks != nil { + bp.RequiredStatusChecks = result.RequiredStatusChecks.Contexts + } + return bp, nil +} + func (c *realGitHubClient) CreateIssue(ctx context.Context, owner, repo, title, body string) error { url := fmt.Sprintf("https://api.github.com/repos/%s/%s/issues", owner, repo) diff --git a/client_mock_test.go b/client_mock_test.go index 1139663..308207d 100644 --- a/client_mock_test.go +++ b/client_mock_test.go @@ -4,11 +4,13 @@ import "context" // MockGitHubClient implements GitHubClient with canned responses for testing. type MockGitHubClient struct { - Repos []Repo - Err error - Tree map[string][]FileEntry // repo name -> file entries - TreeErr error - IssueErr error + Repos []Repo + Err error + Tree map[string][]FileEntry // repo name -> file entries + TreeErr error + Protection map[string]*BranchProtection // repo name -> branch protection + ProtectionErr error + IssueErr error // CreatedIssue records the last CreateIssue call for assertions. CreatedIssue struct { Owner, Repo, Title, Body string @@ -29,6 +31,16 @@ func (m *MockGitHubClient) GetTree(ctx context.Context, owner, repo, branch stri return nil, nil } +func (m *MockGitHubClient) GetBranchProtection(ctx context.Context, owner, repo, branch string) (*BranchProtection, error) { + if m.ProtectionErr != nil { + return nil, m.ProtectionErr + } + if m.Protection != nil { + return m.Protection[repo], nil + } + return nil, nil +} + func (m *MockGitHubClient) CreateIssue(ctx context.Context, owner, repo, title, body string) error { m.CreatedIssue.Owner = owner m.CreatedIssue.Repo = repo diff --git a/rules.go b/rules.go index 3549a19..7559148 100644 --- a/rules.go +++ b/rules.go @@ -27,6 +27,9 @@ func AllRules() []Rule { HasCIWorkflow{}, HasTestDirectory{}, HasCodeowners{}, + HasBranchProtection{}, + HasRequiredReviewers{}, + HasRequiredStatusChecks{}, } } @@ -109,6 +112,30 @@ func (r HasCodeowners) Check(repo Repo) bool { hasFile(repo.Files, ".github/CODEOWNERS") } +// HasBranchProtection checks that the default branch has protection rules enabled. +type HasBranchProtection struct{} + +func (r HasBranchProtection) Name() string { return "Has branch protection" } +func (r HasBranchProtection) Check(repo Repo) bool { + return repo.BranchProtection != nil +} + +// HasRequiredReviewers checks that at least one approving review is required. +type HasRequiredReviewers struct{} + +func (r HasRequiredReviewers) Name() string { return "Has required reviewers" } +func (r HasRequiredReviewers) Check(repo Repo) bool { + return repo.BranchProtection != nil && repo.BranchProtection.RequiredReviewers >= 1 +} + +// HasRequiredStatusChecks checks that at least one status check is required before merging. +type HasRequiredStatusChecks struct{} + +func (r HasRequiredStatusChecks) Name() string { return "Requires status checks before merging" } +func (r HasRequiredStatusChecks) Check(repo Repo) bool { + return repo.BranchProtection != nil && len(repo.BranchProtection.RequiredStatusChecks) > 0 +} + func findFile(files []FileEntry, path string) (FileEntry, bool) { for _, f := range files { if f.Path == path { diff --git a/rules_test.go b/rules_test.go index 12aa44e..fc5b401 100644 --- a/rules_test.go +++ b/rules_test.go @@ -211,3 +211,69 @@ func TestHasCodeowners_Fail(t *testing.T) { t.Error("expected fail when CODEOWNERS is missing from all locations") } } + +func TestHasBranchProtection_Pass(t *testing.T) { + rule := HasBranchProtection{} + + if !rule.Check(Repo{BranchProtection: &BranchProtection{}}) { + t.Error("expected pass when branch protection is enabled") + } +} + +func TestHasBranchProtection_Fail(t *testing.T) { + rule := HasBranchProtection{} + + if rule.Check(Repo{BranchProtection: nil}) { + t.Error("expected fail when branch protection is nil") + } +} + +func TestHasRequiredReviewers_Pass(t *testing.T) { + rule := HasRequiredReviewers{} + + if !rule.Check(Repo{BranchProtection: &BranchProtection{RequiredReviewers: 1}}) { + t.Error("expected pass when required reviewers >= 1") + } +} + +func TestHasRequiredReviewers_Fail_Zero(t *testing.T) { + rule := HasRequiredReviewers{} + + if rule.Check(Repo{BranchProtection: &BranchProtection{RequiredReviewers: 0}}) { + t.Error("expected fail when required reviewers is 0") + } +} + +func TestHasRequiredReviewers_Fail_NoProtection(t *testing.T) { + rule := HasRequiredReviewers{} + + if rule.Check(Repo{BranchProtection: nil}) { + t.Error("expected fail when branch protection is nil") + } +} + +func TestHasRequiredStatusChecks_Pass(t *testing.T) { + rule := HasRequiredStatusChecks{} + + bp := &BranchProtection{RequiredStatusChecks: []string{"ci/build"}} + if !rule.Check(Repo{BranchProtection: bp}) { + t.Error("expected pass when status checks are configured") + } +} + +func TestHasRequiredStatusChecks_Fail_Empty(t *testing.T) { + rule := HasRequiredStatusChecks{} + + bp := &BranchProtection{RequiredStatusChecks: []string{}} + if rule.Check(Repo{BranchProtection: bp}) { + t.Error("expected fail when status checks list is empty") + } +} + +func TestHasRequiredStatusChecks_Fail_NoProtection(t *testing.T) { + rule := HasRequiredStatusChecks{} + + if rule.Check(Repo{BranchProtection: nil}) { + t.Error("expected fail when branch protection is nil") + } +} diff --git a/scanner.go b/scanner.go index e47ae55..438575d 100644 --- a/scanner.go +++ b/scanner.go @@ -65,6 +65,12 @@ func Scan(ctx context.Context, client GitHubClient, org string) ([]RepoResult, e } repo.Files = files + protection, err := client.GetBranchProtection(ctx, org, repo.Name, repo.DefaultBranch) + if err != nil { + return nil, fmt.Errorf("get branch protection for repo %s: %w", repo.Name, err) + } + repo.BranchProtection = protection + rr := RepoResult{RepoName: repo.Name} for _, rule := range rules { rr.Results = append(rr.Results, RuleResult{