diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/components/PolicyDeleteDialog.test.tsx b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/components/PolicyDeleteDialog.test.tsx index 5d303fd137..d28a0eaa02 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/components/PolicyDeleteDialog.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/components/PolicyDeleteDialog.test.tsx @@ -95,6 +95,22 @@ describe('PolicyDeleteDialog', () => { ).toBeInTheDocument(); }); + it('warns that the entire policy and all versions are deleted, not just the current version', () => { + render( + , + ); + expect( + screen.getByText(/all of its versions/i), + ).toBeInTheDocument(); + expect( + screen.getByText(/not just the version you are currently viewing/i), + ).toBeInTheDocument(); + }); + it('renders Delete button enabled for admin', () => { render( Delete Policy - Are you sure you want to delete this policy? This action cannot be undone. + Are you sure you want to delete this policy? This permanently deletes the + entire "{policy.name}" policy and all of its versions, not just the version + you are currently viewing. This action cannot be undone.
diff --git a/packages/integration-platform/src/manifests/github/checks/__tests__/branch-protection.test.ts b/packages/integration-platform/src/manifests/github/checks/__tests__/branch-protection.test.ts new file mode 100644 index 0000000000..69859463ce --- /dev/null +++ b/packages/integration-platform/src/manifests/github/checks/__tests__/branch-protection.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, it } from 'bun:test'; +import type { CheckContext } from '../../../../types'; +import type { GitHubBranchRule, GitHubRepo } from '../../types'; +import { branchProtectionCheck } from '../branch-protection'; +import { REPO_CHECK_CONCURRENCY } from '../concurrency'; + +const sleep = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)); + +type RepoState = 'protected' | 'unprotected' | 'missing'; + +interface RunResult { + passed: Array<{ resourceId: string; title: string }>; + failed: Array<{ resourceId: string; title: string }>; + /** Peak number of repositories whose top-level fetch was in flight at once. */ + maxInFlight: number; +} + +const makeRepo = (fullName: string): GitHubRepo => + ({ + id: 1, + name: fullName.split('/')[1]!, + full_name: fullName, + private: true, + html_url: `https://github.com/${fullName}`, + default_branch: 'main', + owner: { login: fullName.split('/')[0]!, type: 'Organization' }, + }) as GitHubRepo; + +async function runCheck( + repoStates: Record, + { repoFetchDelayMs = 0 }: { repoFetchDelayMs?: number } = {}, +): Promise { + const passed: RunResult['passed'] = []; + const failed: RunResult['failed'] = []; + + let inFlight = 0; + let maxInFlight = 0; + + const ctx: CheckContext = { + accessToken: 'tok', + credentials: {}, + variables: { target_repos: Object.keys(repoStates) }, + connectionId: 'conn_1', + organizationId: 'org_1', + metadata: {}, + log: () => {}, + warn: () => {}, + pass: (result) => { + passed.push({ resourceId: result.resourceId ?? '', title: result.title }); + }, + fail: (result) => { + failed.push({ resourceId: result.resourceId ?? '', title: result.title }); + }, + fetch: (async (path: string): Promise => { + // /repos// — the first call each repo makes. Instrument it to + // measure how many repositories are being processed at the same time. + const repoMatch = path.match(/^\/repos\/([^/]+\/[^/]+)$/); + if (repoMatch) { + const fullName = repoMatch[1]!; + const state = repoStates[fullName]; + inFlight += 1; + maxInFlight = Math.max(maxInFlight, inFlight); + try { + if (repoFetchDelayMs > 0) await sleep(repoFetchDelayMs); + if (!state || state === 'missing') throw new Error(`404 ${path}`); + return makeRepo(fullName) as unknown as T; + } finally { + inFlight -= 1; + } + } + + // Strategy 1: /repos///rules/branches/ + const rulesMatch = path.match(/^\/repos\/([^/]+\/[^/]+)\/rules\/branches\/.+$/); + if (rulesMatch) { + const rules: GitHubBranchRule[] = + repoStates[rulesMatch[1]!] === 'protected' ? [{ type: 'pull_request' }] : []; + return rules as unknown as T; + } + + // Strategy 2: /repos///rulesets — none configured. + if (/^\/repos\/[^/]+\/[^/]+\/rulesets$/.test(path)) { + return [] as unknown as T; + } + + // Strategy 3: /repos///branches//protection — absent. + if (/^\/repos\/[^/]+\/[^/]+\/branches\/.+\/protection$/.test(path)) { + throw new Error(`404 ${path}`); + } + + throw new Error(`Unexpected fetch: ${path}`); + }) as CheckContext['fetch'], + fetchAllPages: (async () => []) as CheckContext['fetchAllPages'], + fetchWithCursor: (async () => []) as CheckContext['fetchWithCursor'], + fetchWithLinkHeader: (async () => []) as CheckContext['fetchWithLinkHeader'], + graphql: (async () => ({})) as CheckContext['graphql'], + getState: (async () => null) as CheckContext['getState'], + setState: (async () => {}) as CheckContext['setState'], + } as CheckContext; + + await branchProtectionCheck.run(ctx); + return { passed, failed, maxInFlight }; +} + +describe('branchProtectionCheck concurrency', () => { + it('checks repositories in parallel, not one-at-a-time (regression: manual-run HTTP timeout)', async () => { + // With the old serial `for...of` loop the top-level repo fetch was only ever + // in flight for ONE repo at a time (maxInFlight === 1). For an org with many + // monitored repos that meant ~250 sequential GitHub calls, which blew past + // the synchronous manual-run HTTP timeout — the connection died with no + // result persisted and the UI fell back to "No runs yet". The bounded pool + // overlaps repos so the run finishes well under that ceiling. + const repoCount = 12; + const repoStates: Record = {}; + for (let i = 0; i < repoCount; i++) { + repoStates[`acme/repo-${i}`] = 'protected'; + } + + const { passed, failed, maxInFlight } = await runCheck(repoStates, { + repoFetchDelayMs: 15, + }); + + // Core regression assertion: repos are no longer processed strictly serially. + expect(maxInFlight).toBeGreaterThan(1); + // ...but stay bounded by the pool so we never blast GitHub's rate limits. + expect(maxInFlight).toBeLessThanOrEqual(REPO_CHECK_CONCURRENCY); + expect(maxInFlight).toBe(Math.min(REPO_CHECK_CONCURRENCY, repoCount)); + + // Correctness is preserved: every repo still produced exactly one result. + expect(passed).toHaveLength(repoCount); + expect(failed).toHaveLength(0); + }); +}); + +describe('branchProtectionCheck results', () => { + it('emits the correct pass/fail per repo regardless of interleaving', async () => { + const { passed, failed } = await runCheck({ + 'acme/protected': 'protected', + 'acme/unprotected': 'unprotected', + 'acme/missing': 'missing', + }); + + expect(passed.map((p) => p.resourceId).sort()).toEqual(['acme/protected']); + // Unprotected fails on full_name; a missing repo fails on the raw name. + expect(failed.map((f) => f.resourceId).sort()).toEqual(['acme/missing', 'acme/unprotected']); + + expect(passed.find((p) => p.resourceId === 'acme/protected')?.title).toBe( + 'All branches protected on protected', + ); + expect(failed.find((f) => f.resourceId === 'acme/unprotected')?.title).toBe( + 'No branch protection on unprotected', + ); + expect(failed.find((f) => f.resourceId === 'acme/missing')?.title).toBe( + 'Repository not found: acme/missing', + ); + }); +}); diff --git a/packages/integration-platform/src/manifests/github/checks/branch-protection.ts b/packages/integration-platform/src/manifests/github/checks/branch-protection.ts index d65216a63d..4a5e398664 100644 --- a/packages/integration-platform/src/manifests/github/checks/branch-protection.ts +++ b/packages/integration-platform/src/manifests/github/checks/branch-protection.ts @@ -19,6 +19,7 @@ import { recentPullRequestDaysVariable, targetReposVariable, } from '../variables'; +import { mapWithConcurrency, REPO_CHECK_CONCURRENCY } from './concurrency'; // ───────────────────────────────────────────────────────────────────────────── // PR History Config @@ -137,266 +138,274 @@ export const branchProtectionCheck: IntegrationCheck = { } // ─────────────────────────────────────────────────────────────────────── - // Check each repository (with all its branches) + // Check each repository (with all its branches). Repos are checked in + // parallel with a bounded pool so an org with many monitored repos finishes + // well under the synchronous manual-run HTTP timeout (see concurrency.ts). // ─────────────────────────────────────────────────────────────────────── - for (const [repoName, branchesToCheck] of repoGroups) { - // Fetch repository info - let repo: GitHubRepo; - try { - repo = await ctx.fetch(`/repos/${repoName}`); - } catch { - ctx.warn(`Could not fetch repo ${repoName}`); - ctx.fail({ - title: `Repository not found: ${repoName}`, - description: `Could not access repository "${repoName}". It may not exist or the integration lacks permission.`, - resourceType: 'repository', - resourceId: repoName, - severity: 'medium', - remediation: `Verify the repository name is correct (format: owner/repo) and that the GitHub integration has access to it.`, - }); - continue; - } - - ctx.log( - `Checking ${branchesToCheck.length} branches on ${repo.full_name}: ${branchesToCheck.join(', ')}`, - ); - - // Collect results for all branches in this repo - const branchResults: Record< - string, - { - protected: boolean; - evidence: Record; - description: string; + await mapWithConcurrency( + Array.from(repoGroups), + REPO_CHECK_CONCURRENCY, + async ([repoName, branchesToCheck]) => { + // Fetch repository info + let repo: GitHubRepo; + try { + repo = await ctx.fetch(`/repos/${repoName}`); + } catch { + ctx.warn(`Could not fetch repo ${repoName}`); + ctx.fail({ + title: `Repository not found: ${repoName}`, + description: `Could not access repository "${repoName}". It may not exist or the integration lacks permission.`, + resourceType: 'repository', + resourceId: repoName, + severity: 'medium', + remediation: `Verify the repository name is correct (format: owner/repo) and that the GitHub integration has access to it.`, + }); + return; } - > = {}; - - // Check each branch - for (const branchToCheck of branchesToCheck) { - ctx.log(`Checking branch "${branchToCheck}" on ${repo.full_name}`); - - // Fetch recent PRs in parallel while we check protection - const pullRequestsPromise = fetchRecentPullRequests({ - repoFullName: repo.full_name, - baseBranch: branchToCheck, - }); - - // Helper to check if a branch matches ruleset conditions - const branchMatchesRuleset = (ruleset: GitHubRuleset, branch: string): boolean => { - if (!ruleset.conditions?.ref_name) return true; - const includes = ruleset.conditions.ref_name.include || []; - const excludes = ruleset.conditions.ref_name.exclude || []; - - for (const pattern of excludes) { - if (pattern === `refs/heads/${branch}` || pattern === `~DEFAULT_BRANCH`) { - return false; - } - } - if (includes.length === 0) return true; - for (const pattern of includes) { - if ( - pattern === `refs/heads/${branch}` || - pattern === '~ALL' || - (pattern === '~DEFAULT_BRANCH' && branch === repo.default_branch) - ) { - return true; - } - } - return false; - }; - - let isProtected = false; - let protectionEvidence: Record = {}; - let protectionDescription = ''; + ctx.log( + `Checking ${branchesToCheck.length} branches on ${repo.full_name}: ${branchesToCheck.join(', ')}`, + ); - // Strategy 1: Try the unified rules endpoint - ctx.log(`[Strategy 1] Trying /repos/${repo.full_name}/rules/branches/${branchToCheck}`); - try { - const rules = await ctx.fetch( - `/repos/${repo.full_name}/rules/branches/${branchToCheck}`, - ); - - ctx.log( - `[Strategy 1] Got ${rules.length} rules: ${JSON.stringify(rules.map((r) => r.type))}`, - ); - - const hasPullRequestRule = rules.some((r) => r.type === 'pull_request'); - const hasNonFastForward = rules.some((r) => r.type === 'non_fast_forward'); - - if (rules.length > 0 && (hasPullRequestRule || hasNonFastForward)) { - isProtected = true; - const protectionTypes: string[] = []; - if (hasPullRequestRule) protectionTypes.push('pull request reviews'); - if (hasNonFastForward) protectionTypes.push('non-fast-forward'); - - const rulesetSources = [ - ...new Set(rules.filter((r) => r.ruleset_source).map((r) => r.ruleset_source)), - ]; - - protectionDescription = `Branch "${branchToCheck}" is protected with: ${protectionTypes.join(', ')}. ${rulesetSources.length > 0 ? `Source: ${rulesetSources.join(', ')}` : ''}`; - protectionEvidence = { - source: 'rules_endpoint', - branch: branchToCheck, - rules, - rule_types: rules.map((r) => r.type), - ruleset_sources: rulesetSources, - }; - ctx.log(`[Strategy 1] SUCCESS - Found protection via rules endpoint`); - } else { - ctx.log(`[Strategy 1] No PR or non-fast-forward rules found`); + // Collect results for all branches in this repo + const branchResults: Record< + string, + { + protected: boolean; + evidence: Record; + description: string; } - } catch (err) { - const errorMsg = err instanceof Error ? err.message : String(err); - ctx.warn(`[Strategy 1] FAILED: ${errorMsg}`); - } + > = {}; + + // Check each branch + for (const branchToCheck of branchesToCheck) { + ctx.log(`Checking branch "${branchToCheck}" on ${repo.full_name}`); + + // Fetch recent PRs in parallel while we check protection + const pullRequestsPromise = fetchRecentPullRequests({ + repoFullName: repo.full_name, + baseBranch: branchToCheck, + }); + + // Helper to check if a branch matches ruleset conditions + const branchMatchesRuleset = (ruleset: GitHubRuleset, branch: string): boolean => { + if (!ruleset.conditions?.ref_name) return true; + const includes = ruleset.conditions.ref_name.include || []; + const excludes = ruleset.conditions.ref_name.exclude || []; + + for (const pattern of excludes) { + if (pattern === `refs/heads/${branch}` || pattern === `~DEFAULT_BRANCH`) { + return false; + } + } - // Strategy 2: Check rulesets directly - if (!isProtected) { - ctx.log(`[Strategy 2] Trying /repos/${repo.full_name}/rulesets`); - try { - const rulesets = await ctx.fetch(`/repos/${repo.full_name}/rulesets`); + if (includes.length === 0) return true; + for (const pattern of includes) { + if ( + pattern === `refs/heads/${branch}` || + pattern === '~ALL' || + (pattern === '~DEFAULT_BRANCH' && branch === repo.default_branch) + ) { + return true; + } + } + return false; + }; - ctx.log(`[Strategy 2] Got ${rulesets.length} rulesets`); + let isProtected = false; + let protectionEvidence: Record = {}; + let protectionDescription = ''; - const applicableRulesets = rulesets.filter( - (rs) => - rs.enforcement === 'active' && - rs.target === 'branch' && - branchMatchesRuleset(rs, branchToCheck), + // Strategy 1: Try the unified rules endpoint + ctx.log(`[Strategy 1] Trying /repos/${repo.full_name}/rules/branches/${branchToCheck}`); + try { + const rules = await ctx.fetch( + `/repos/${repo.full_name}/rules/branches/${branchToCheck}`, ); ctx.log( - `[Strategy 2] ${applicableRulesets.length} active rulesets apply to "${branchToCheck}"`, + `[Strategy 1] Got ${rules.length} rules: ${JSON.stringify(rules.map((r) => r.type))}`, ); - for (const rs of applicableRulesets) { + const hasPullRequestRule = rules.some((r) => r.type === 'pull_request'); + const hasNonFastForward = rules.some((r) => r.type === 'non_fast_forward'); + + if (rules.length > 0 && (hasPullRequestRule || hasNonFastForward)) { + isProtected = true; + const protectionTypes: string[] = []; + if (hasPullRequestRule) protectionTypes.push('pull request reviews'); + if (hasNonFastForward) protectionTypes.push('non-fast-forward'); + + const rulesetSources = [ + ...new Set(rules.filter((r) => r.ruleset_source).map((r) => r.ruleset_source)), + ]; + + protectionDescription = `Branch "${branchToCheck}" is protected with: ${protectionTypes.join(', ')}. ${rulesetSources.length > 0 ? `Source: ${rulesetSources.join(', ')}` : ''}`; + protectionEvidence = { + source: 'rules_endpoint', + branch: branchToCheck, + rules, + rule_types: rules.map((r) => r.type), + ruleset_sources: rulesetSources, + }; + ctx.log(`[Strategy 1] SUCCESS - Found protection via rules endpoint`); + } else { + ctx.log(`[Strategy 1] No PR or non-fast-forward rules found`); + } + } catch (err) { + const errorMsg = err instanceof Error ? err.message : String(err); + ctx.warn(`[Strategy 1] FAILED: ${errorMsg}`); + } + + // Strategy 2: Check rulesets directly + if (!isProtected) { + ctx.log(`[Strategy 2] Trying /repos/${repo.full_name}/rulesets`); + try { + const rulesets = await ctx.fetch( + `/repos/${repo.full_name}/rulesets`, + ); + + ctx.log(`[Strategy 2] Got ${rulesets.length} rulesets`); + + const applicableRulesets = rulesets.filter( + (rs) => + rs.enforcement === 'active' && + rs.target === 'branch' && + branchMatchesRuleset(rs, branchToCheck), + ); + ctx.log( - `[Strategy 2] Ruleset "${rs.name}": rules=${JSON.stringify(rs.rules?.map((r) => r.type))}`, + `[Strategy 2] ${applicableRulesets.length} active rulesets apply to "${branchToCheck}"`, ); - } - for (const ruleset of applicableRulesets) { - const hasPullRequest = ruleset.rules?.some((r) => r.type === 'pull_request'); - if (hasPullRequest) { - isProtected = true; - const pullRequestRule = ruleset.rules?.find((r) => r.type === 'pull_request'); - protectionDescription = `Branch "${branchToCheck}" is protected by ruleset "${ruleset.name}" requiring pull request reviews.`; - protectionEvidence = { - source: 'rulesets_endpoint', - branch: branchToCheck, - ruleset_name: ruleset.name, - ruleset_id: ruleset.id, - enforcement: ruleset.enforcement, - rules: ruleset.rules, - pull_request_params: pullRequestRule?.parameters, - }; - break; + for (const rs of applicableRulesets) { + ctx.log( + `[Strategy 2] Ruleset "${rs.name}": rules=${JSON.stringify(rs.rules?.map((r) => r.type))}`, + ); + } + + for (const ruleset of applicableRulesets) { + const hasPullRequest = ruleset.rules?.some((r) => r.type === 'pull_request'); + if (hasPullRequest) { + isProtected = true; + const pullRequestRule = ruleset.rules?.find((r) => r.type === 'pull_request'); + protectionDescription = `Branch "${branchToCheck}" is protected by ruleset "${ruleset.name}" requiring pull request reviews.`; + protectionEvidence = { + source: 'rulesets_endpoint', + branch: branchToCheck, + ruleset_name: ruleset.name, + ruleset_id: ruleset.id, + enforcement: ruleset.enforcement, + rules: ruleset.rules, + pull_request_params: pullRequestRule?.parameters, + }; + break; + } } + } catch (err) { + const errorMsg = err instanceof Error ? err.message : String(err); + ctx.warn(`[Strategy 2] FAILED: ${errorMsg}`); } - } catch (err) { - const errorMsg = err instanceof Error ? err.message : String(err); - ctx.warn(`[Strategy 2] FAILED: ${errorMsg}`); } - } - // Strategy 3: Try legacy branch protection endpoint - if (!isProtected) { - ctx.log( - `[Strategy 3] Trying /repos/${repo.full_name}/branches/${branchToCheck}/protection`, - ); - try { - const protection = await ctx.fetch( - `/repos/${repo.full_name}/branches/${branchToCheck}/protection`, + // Strategy 3: Try legacy branch protection endpoint + if (!isProtected) { + ctx.log( + `[Strategy 3] Trying /repos/${repo.full_name}/branches/${branchToCheck}/protection`, ); + try { + const protection = await ctx.fetch( + `/repos/${repo.full_name}/branches/${branchToCheck}/protection`, + ); - isProtected = true; - protectionDescription = `Branch "${branchToCheck}" requires ${protection.required_pull_request_reviews?.required_approving_review_count || 0} approving review(s) (legacy protection).`; - protectionEvidence = { - source: 'legacy_branch_protection', - branch: branchToCheck, - protection_rules: protection, - }; - ctx.log(`[Strategy 3] SUCCESS - Found legacy branch protection`); - } catch (err) { - const errorMsg = err instanceof Error ? err.message : String(err); - ctx.warn(`[Strategy 3] FAILED: ${errorMsg}`); + isProtected = true; + protectionDescription = `Branch "${branchToCheck}" requires ${protection.required_pull_request_reviews?.required_approving_review_count || 0} approving review(s) (legacy protection).`; + protectionEvidence = { + source: 'legacy_branch_protection', + branch: branchToCheck, + protection_rules: protection, + }; + ctx.log(`[Strategy 3] SUCCESS - Found legacy branch protection`); + } catch (err) { + const errorMsg = err instanceof Error ? err.message : String(err); + ctx.warn(`[Strategy 3] FAILED: ${errorMsg}`); + } } - } - // Wait for PR fetch to complete - const pullRequests = await pullRequestsPromise; - - // Build evidence for this branch - const branchEvidence: Record = { - protected: isProtected, - ...protectionEvidence, - pull_requests: pullRequests, - pull_requests_window_days: recentWindowDays, - checked_at: new Date().toISOString(), - }; - - branchResults[branchToCheck] = { - protected: isProtected, - evidence: branchEvidence, - description: isProtected - ? protectionDescription - : `Branch "${branchToCheck}" has no protection rules configured.`, - }; - } // End of branch loop - - // Emit combined result for this repo - const protectedBranches = Object.entries(branchResults) - .filter(([, r]) => r.protected) - .map(([b]) => b); - const unprotectedBranches = Object.entries(branchResults) - .filter(([, r]) => !r.protected) - .map(([b]) => b); - - // Build combined evidence: { "owner/repo": { "branch1": {...}, "branch2": {...} } } - const combinedEvidence: Record> = {}; - for (const [branch, result] of Object.entries(branchResults)) { - combinedEvidence[branch] = result.evidence; - } + // Wait for PR fetch to complete + const pullRequests = await pullRequestsPromise; + + // Build evidence for this branch + const branchEvidence: Record = { + protected: isProtected, + ...protectionEvidence, + pull_requests: pullRequests, + pull_requests_window_days: recentWindowDays, + checked_at: new Date().toISOString(), + }; + + branchResults[branchToCheck] = { + protected: isProtected, + evidence: branchEvidence, + description: isProtected + ? protectionDescription + : `Branch "${branchToCheck}" has no protection rules configured.`, + }; + } // End of branch loop + + // Emit combined result for this repo + const protectedBranches = Object.entries(branchResults) + .filter(([, r]) => r.protected) + .map(([b]) => b); + const unprotectedBranches = Object.entries(branchResults) + .filter(([, r]) => !r.protected) + .map(([b]) => b); + + // Build combined evidence: { "owner/repo": { "branch1": {...}, "branch2": {...} } } + const combinedEvidence: Record> = {}; + for (const [branch, result] of Object.entries(branchResults)) { + combinedEvidence[branch] = result.evidence; + } - if (unprotectedBranches.length === 0) { - // All branches protected - ctx.pass({ - title: `All branches protected on ${repo.name}`, - description: `${protectedBranches.length} branch(es) have protection enabled: ${protectedBranches.join(', ')}`, - resourceType: 'repository', - resourceId: repo.full_name, - evidence: { - [repo.full_name]: combinedEvidence, - }, - }); - } else if (protectedBranches.length === 0) { - // No branches protected - ctx.fail({ - title: `No branch protection on ${repo.name}`, - description: `${unprotectedBranches.length} branch(es) have no protection: ${unprotectedBranches.join(', ')}`, - resourceType: 'repository', - resourceId: repo.full_name, - severity: 'high', - remediation: `1. Go to ${repo.html_url}/settings/rules\n2. Create rulesets for branches: ${unprotectedBranches.join(', ')}\n3. Enable "Require a pull request before merging"\n4. Set required approvals to at least 1`, - evidence: { - [repo.full_name]: combinedEvidence, - }, - }); - } else { - // Mixed: some protected, some not - ctx.fail({ - title: `Partial branch protection on ${repo.name}`, - description: `Protected: ${protectedBranches.join(', ')}. Unprotected: ${unprotectedBranches.join(', ')}`, - resourceType: 'repository', - resourceId: repo.full_name, - severity: 'high', - remediation: `1. Go to ${repo.html_url}/settings/rules\n2. Create rulesets for unprotected branches: ${unprotectedBranches.join(', ')}\n3. Enable "Require a pull request before merging"`, - evidence: { - [repo.full_name]: combinedEvidence, - }, - }); - } - } + if (unprotectedBranches.length === 0) { + // All branches protected + ctx.pass({ + title: `All branches protected on ${repo.name}`, + description: `${protectedBranches.length} branch(es) have protection enabled: ${protectedBranches.join(', ')}`, + resourceType: 'repository', + resourceId: repo.full_name, + evidence: { + [repo.full_name]: combinedEvidence, + }, + }); + } else if (protectedBranches.length === 0) { + // No branches protected + ctx.fail({ + title: `No branch protection on ${repo.name}`, + description: `${unprotectedBranches.length} branch(es) have no protection: ${unprotectedBranches.join(', ')}`, + resourceType: 'repository', + resourceId: repo.full_name, + severity: 'high', + remediation: `1. Go to ${repo.html_url}/settings/rules\n2. Create rulesets for branches: ${unprotectedBranches.join(', ')}\n3. Enable "Require a pull request before merging"\n4. Set required approvals to at least 1`, + evidence: { + [repo.full_name]: combinedEvidence, + }, + }); + } else { + // Mixed: some protected, some not + ctx.fail({ + title: `Partial branch protection on ${repo.name}`, + description: `Protected: ${protectedBranches.join(', ')}. Unprotected: ${unprotectedBranches.join(', ')}`, + resourceType: 'repository', + resourceId: repo.full_name, + severity: 'high', + remediation: `1. Go to ${repo.html_url}/settings/rules\n2. Create rulesets for unprotected branches: ${unprotectedBranches.join(', ')}\n3. Enable "Require a pull request before merging"`, + evidence: { + [repo.full_name]: combinedEvidence, + }, + }); + } + }, + ); }, }; diff --git a/packages/integration-platform/src/manifests/github/checks/concurrency.ts b/packages/integration-platform/src/manifests/github/checks/concurrency.ts index 2859478b5e..2c05acae7e 100644 --- a/packages/integration-platform/src/manifests/github/checks/concurrency.ts +++ b/packages/integration-platform/src/manifests/github/checks/concurrency.ts @@ -15,6 +15,18 @@ /** Per-repo file reads to keep in flight at once. */ export const FILE_READ_CONCURRENCY = 10; +/** + * Repositories to check in parallel within a single run. The branch-protection + * check fetches protection rules (up to 3 strategies) plus recent PRs per + * repo/branch; running repos SERIALLY meant an org with many monitored repos + * (~28) issued ~250 sequential GitHub calls — with retry backoff stacking on + * 429s — and blew past the synchronous manual-run HTTP timeout, so the run died + * with no result persisted. A bounded pool keeps the run well under that ceiling + * while staying far below GitHub's concurrent-request limit (each repo makes at + * most ~2 requests in flight, so this caps total concurrency at ~16). + */ +export const REPO_CHECK_CONCURRENCY = 8; + /** Run `fn` over `items` with at most `limit` in flight, preserving order. */ export async function mapWithConcurrency( items: T[],