Skip to content

Commit 0acfc22

Browse files
sgtojsgtoj
authored
Merge pull request #9 from cruxstack/dev
feat: add for gh rulesets during pr compliance checks
2 parents fbd7236 + 7e118f5 commit 0acfc22

2 files changed

Lines changed: 213 additions & 22 deletions

File tree

‎fixtures/scenarios.json‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,11 @@
153153
"method": "GET",
154154
"path": "/repos/acme-ghorg/fake-app-repo/branches/main/protection"
155155
},
156+
{
157+
"service": "github",
158+
"method": "GET",
159+
"path": "/repos/acme-ghorg/fake-app-repo/rules/branches/main"
160+
},
156161
{
157162
"service": "github",
158163
"method": "GET",
@@ -194,6 +199,14 @@
194199
"body": "{\"required_pull_request_reviews\":{\"required_approving_review_count\":2,\"dismiss_stale_reviews\":true},\"enforce_admins\":{\"enabled\":true}}",
195200
"description": "fetch branch protection rules (requires 2 approvals, enforce for admins)"
196201
},
202+
{
203+
"service": "github",
204+
"method": "GET",
205+
"path": "/repos/acme-ghorg/fake-app-repo/rules/branches/main",
206+
"status_code": 200,
207+
"body": "[]",
208+
"description": "fetch repository rulesets (none configured)"
209+
},
197210
{
198211
"service": "github",
199212
"method": "GET",
@@ -271,6 +284,11 @@
271284
"method": "GET",
272285
"path": "/repos/acme-ghorg/fake-app-repo/branches/main/protection"
273286
},
287+
{
288+
"service": "github",
289+
"method": "GET",
290+
"path": "/repos/acme-ghorg/fake-app-repo/rules/branches/main"
291+
},
274292
{
275293
"service": "github",
276294
"method": "GET",
@@ -302,6 +320,14 @@
302320
"body": "{\"required_pull_request_reviews\":{\"required_approving_review_count\":2}}",
303321
"description": "fetch branch protection rules (requires 2 approvals)"
304322
},
323+
{
324+
"service": "github",
325+
"method": "GET",
326+
"path": "/repos/acme-ghorg/fake-app-repo/rules/branches/main",
327+
"status_code": 200,
328+
"body": "[]",
329+
"description": "fetch repository rulesets (none configured)"
330+
},
305331
{
306332
"service": "github",
307333
"method": "GET",
@@ -312,6 +338,137 @@
312338
}
313339
]
314340
},
341+
{
342+
"name": "pr_webhook_ruleset_bypass",
343+
"description": "Detect bypass when repo uses rulesets instead of legacy branch protection",
344+
"event_type": "webhook",
345+
"webhook_type": "pull_request",
346+
"config_overrides": {
347+
"APP_GITHUB_WEBHOOK_SECRET": ""
348+
},
349+
"webhook_payload": {
350+
"action": "closed",
351+
"number": 101,
352+
"pull_request": {
353+
"id": 101,
354+
"number": 101,
355+
"state": "closed",
356+
"merged": true,
357+
"merged_at": "2025-11-22T12:00:00Z",
358+
"merged_by": {
359+
"login": "admin-user",
360+
"id": 2,
361+
"type": "User"
362+
},
363+
"head": {
364+
"ref": "feature-branch",
365+
"sha": "abc123def456"
366+
},
367+
"base": {
368+
"ref": "main",
369+
"sha": "def456abc123"
370+
},
371+
"html_url": "https://github.com/acme-ghorg/ruleset-repo/pull/101"
372+
},
373+
"repository": {
374+
"name": "ruleset-repo",
375+
"full_name": "acme-ghorg/ruleset-repo",
376+
"owner": {
377+
"login": "acme-ghorg"
378+
}
379+
}
380+
},
381+
"expected_calls": [
382+
{
383+
"service": "github",
384+
"method": "GET",
385+
"path": "/repos/acme-ghorg/ruleset-repo/pulls/101"
386+
},
387+
{
388+
"service": "github",
389+
"method": "GET",
390+
"path": "/repos/acme-ghorg/ruleset-repo/branches/main/protection"
391+
},
392+
{
393+
"service": "github",
394+
"method": "GET",
395+
"path": "/repos/acme-ghorg/ruleset-repo/rules/branches/main"
396+
},
397+
{
398+
"service": "github",
399+
"method": "GET",
400+
"path": "/repos/acme-ghorg/ruleset-repo/pulls/101/reviews"
401+
},
402+
{
403+
"service": "github",
404+
"method": "GET",
405+
"path": "/repos/acme-ghorg/ruleset-repo/collaborators/admin-user/permission"
406+
},
407+
{
408+
"service": "slack",
409+
"method": "POST",
410+
"path": "/chat.postMessage"
411+
}
412+
],
413+
"mock_responses": [
414+
{
415+
"service": "github",
416+
"method": "POST",
417+
"path": "/app/installations/987654/access_tokens",
418+
"status_code": 201,
419+
"body": "{\"token\":\"ghs_mock_installation_token\",\"expires_at\":\"2099-12-31T23:59:59Z\"}",
420+
"description": "github app installation token authentication"
421+
},
422+
{
423+
"service": "github",
424+
"method": "GET",
425+
"path": "/repos/acme-ghorg/ruleset-repo/pulls/101",
426+
"status_code": 200,
427+
"body": "{\"number\":101,\"state\":\"closed\",\"merged\":true,\"merged_at\":\"2025-11-22T12:00:00Z\",\"merged_by\":{\"login\":\"admin-user\"},\"base\":{\"ref\":\"main\"}}",
428+
"description": "fetch pr #101 details (merged by admin-user)"
429+
},
430+
{
431+
"service": "github",
432+
"method": "GET",
433+
"path": "/repos/acme-ghorg/ruleset-repo/branches/main/protection",
434+
"status_code": 404,
435+
"body": "{\"message\":\"Branch not protected\"}",
436+
"description": "no legacy branch protection configured"
437+
},
438+
{
439+
"service": "github",
440+
"method": "GET",
441+
"path": "/repos/acme-ghorg/ruleset-repo/rules/branches/main",
442+
"status_code": 200,
443+
"body": "[{\"type\":\"pull_request\",\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"acme-ghorg/ruleset-repo\",\"ruleset_id\":123,\"parameters\":{\"required_approving_review_count\":1,\"dismiss_stale_reviews_on_push\":false,\"require_code_owner_review\":false,\"require_last_push_approval\":false,\"required_review_thread_resolution\":false}}]",
444+
"description": "fetch repository rulesets (requires 1 approval via ruleset)"
445+
},
446+
{
447+
"service": "github",
448+
"method": "GET",
449+
"path": "/repos/acme-ghorg/ruleset-repo/pulls/101/reviews",
450+
"status_code": 200,
451+
"body": "[]",
452+
"description": "fetch pr reviews (no approvals)"
453+
},
454+
{
455+
"service": "github",
456+
"method": "GET",
457+
"path": "/repos/acme-ghorg/ruleset-repo/collaborators/admin-user/permission",
458+
"status_code": 200,
459+
"body": "{\"permission\":\"admin\",\"user\":{\"login\":\"admin-user\"}}",
460+
"description": "check merger permissions (admin-user has admin permission)"
461+
},
462+
{
463+
"service": "slack",
464+
"method": "POST",
465+
"path": "/chat.postMessage",
466+
"status_code": 200,
467+
"body": "{\"ok\":true,\"channel\":\"C01234TEST\",\"ts\":\"1234567890.123456\"}",
468+
"description": "send bypass alert notification to slack"
469+
}
470+
]
471+
},
315472
{
316473
"name": "okta_sync_inactive_users",
317474
"description": "Test that inactive Okta users are excluded from sync",

‎internal/github/pr.go‎

Lines changed: 56 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ type PRComplianceResult struct {
2323
PR *github.PullRequest
2424
BaseBranch string
2525
Protection *github.Protection
26+
BranchRules *github.BranchRules
2627
Violations []ComplianceViolation
2728
UserHasBypass bool
2829
UserBypassReason string
@@ -51,36 +52,49 @@ func (c *Client) CheckPRCompliance(ctx context.Context, owner, repo string, prNu
5152

5253
baseBranch := *pr.Base.Ref
5354

54-
protection, _, err := c.client.Repositories.GetBranchProtection(ctx, owner, repo, baseBranch)
55-
if err != nil {
56-
return &PRComplianceResult{
57-
PR: pr,
58-
BaseBranch: baseBranch,
59-
Violations: []ComplianceViolation{},
60-
}, nil
61-
}
62-
6355
result := &PRComplianceResult{
6456
PR: pr,
6557
BaseBranch: baseBranch,
66-
Protection: protection,
6758
Violations: []ComplianceViolation{},
6859
}
6960

70-
c.checkReviewRequirements(ctx, owner, repo, pr, protection, result)
71-
c.checkStatusRequirements(ctx, owner, repo, pr, protection, result)
61+
// fetch legacy branch protection rules
62+
protection, _, err := c.client.Repositories.GetBranchProtection(ctx, owner, repo, baseBranch)
63+
if err == nil {
64+
result.Protection = protection
65+
}
66+
67+
// fetch repository rulesets for the branch
68+
branchRules, _, err := c.client.Repositories.GetRulesForBranch(ctx, owner, repo, baseBranch, nil)
69+
if err == nil {
70+
result.BranchRules = branchRules
71+
}
72+
73+
c.checkReviewRequirements(ctx, owner, repo, pr, result)
74+
c.checkStatusRequirements(ctx, owner, repo, pr, result)
7275
c.checkUserBypassPermission(ctx, owner, repo, pr, result)
7376

7477
return result, nil
7578
}
7679

7780
// checkReviewRequirements validates that PR had required approving reviews.
78-
func (c *Client) checkReviewRequirements(ctx context.Context, owner, repo string, pr *github.PullRequest, protection *github.Protection, result *PRComplianceResult) {
79-
if protection.RequiredPullRequestReviews == nil {
80-
return
81+
// checks both legacy branch protection and repository rulesets.
82+
func (c *Client) checkReviewRequirements(ctx context.Context, owner, repo string, pr *github.PullRequest, result *PRComplianceResult) {
83+
requiredApprovals := 0
84+
85+
// check legacy branch protection
86+
if result.Protection != nil && result.Protection.RequiredPullRequestReviews != nil {
87+
requiredApprovals = result.Protection.RequiredPullRequestReviews.RequiredApprovingReviewCount
8188
}
8289

83-
requiredApprovals := protection.RequiredPullRequestReviews.RequiredApprovingReviewCount
90+
// check repository rulesets (use the highest requirement)
91+
if result.BranchRules != nil {
92+
for _, rule := range result.BranchRules.PullRequest {
93+
if rule.Parameters.RequiredApprovingReviewCount > requiredApprovals {
94+
requiredApprovals = rule.Parameters.RequiredApprovingReviewCount
95+
}
96+
}
97+
}
8498

8599
if requiredApprovals == 0 {
86100
return
@@ -107,16 +121,36 @@ func (c *Client) checkReviewRequirements(ctx context.Context, owner, repo string
107121
}
108122

109123
// checkStatusRequirements validates that required status checks passed.
110-
func (c *Client) checkStatusRequirements(ctx context.Context, owner, repo string, pr *github.PullRequest, protection *github.Protection, result *PRComplianceResult) {
111-
if protection.RequiredStatusChecks == nil || protection.RequiredStatusChecks.Contexts == nil || len(*protection.RequiredStatusChecks.Contexts) == 0 {
124+
// checks both legacy branch protection and repository rulesets.
125+
func (c *Client) checkStatusRequirements(ctx context.Context, owner, repo string, pr *github.PullRequest, result *PRComplianceResult) {
126+
if pr.Head == nil || pr.Head.SHA == nil {
112127
return
113128
}
114129

115-
if pr.Head == nil || pr.Head.SHA == nil {
116-
return
130+
// collect required checks from both sources
131+
requiredChecks := make(map[string]bool)
132+
133+
// check legacy branch protection
134+
if result.Protection != nil &&
135+
result.Protection.RequiredStatusChecks != nil &&
136+
result.Protection.RequiredStatusChecks.Contexts != nil {
137+
for _, check := range *result.Protection.RequiredStatusChecks.Contexts {
138+
requiredChecks[check] = true
139+
}
117140
}
118141

119-
requiredChecks := *protection.RequiredStatusChecks.Contexts
142+
// check repository rulesets
143+
if result.BranchRules != nil {
144+
for _, rule := range result.BranchRules.RequiredStatusChecks {
145+
for _, check := range rule.Parameters.RequiredStatusChecks {
146+
requiredChecks[check.Context] = true
147+
}
148+
}
149+
}
150+
151+
if len(requiredChecks) == 0 {
152+
return
153+
}
120154

121155
combinedStatus, _, err := c.client.Repositories.GetCombinedStatus(ctx, owner, repo, *pr.Head.SHA, nil)
122156
if err != nil {
@@ -130,7 +164,7 @@ func (c *Client) checkStatusRequirements(ctx context.Context, owner, repo string
130164
}
131165
}
132166

133-
for _, required := range requiredChecks {
167+
for required := range requiredChecks {
134168
if !passedChecks[required] {
135169
result.Violations = append(result.Violations, ComplianceViolation{
136170
Type: "missing_status_check",

0 commit comments

Comments
 (0)