Skip to content

Commit 75630e2

Browse files
tofikwesttofikwest
authored
Merge pull request #2569 from trycompai/fix/github-2fa-skip-unowned-orgs
fix(github): scope 2FA check to selected repos' orgs only
2 parents 54415ec + 6c49228 commit 75630e2

File tree

1 file changed

+56
-57
lines changed

1 file changed

+56
-57
lines changed

‎packages/integration-platform/src/manifests/github/checks/two-factor-auth.ts‎

Lines changed: 56 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
import { TASK_TEMPLATES } from '../../../task-mappings';
1212
import type { IntegrationCheck } from '../../../types';
13-
import type { GitHubOrg } from '../types';
13+
import { parseRepoBranches, targetReposVariable } from '../variables';
1414

1515
interface GitHubOrgMember {
1616
login: string;
@@ -81,46 +81,43 @@ export const twoFactorAuthCheck: IntegrationCheck = {
8181
taskMapping: TASK_TEMPLATES.twoFactorAuth,
8282
defaultSeverity: 'high',
8383

84-
run: async (ctx) => {
85-
// Step 1: Get all orgs the authenticated user belongs to
86-
let orgs: GitHubOrg[];
87-
try {
88-
orgs = await ctx.fetchAllPages<GitHubOrg>('/user/orgs');
89-
} catch (error) {
90-
const errorMsg = error instanceof Error ? error.message : String(error);
91-
ctx.error(`Failed to fetch organizations: ${errorMsg}`);
92-
ctx.fail({
93-
title: 'Cannot fetch GitHub organizations',
94-
description: `Failed to list organizations: ${errorMsg}`,
95-
resourceType: 'organization',
96-
resourceId: 'github',
97-
severity: 'medium',
98-
remediation:
99-
'Ensure the GitHub integration has the read:org scope. You may need to reconnect the integration.',
100-
});
101-
return;
102-
}
84+
variables: [targetReposVariable],
10385

104-
if (orgs.length === 0) {
86+
run: async (ctx) => {
87+
// Derive the orgs to check from the user-selected repositories.
88+
// We intentionally do NOT call /user/orgs — checking orgs the user happens to
89+
// belong to but did not select would surface findings for unrelated orgs
90+
// (e.g. personal side-project orgs) and confuse customers.
91+
const targetRepos = ctx.variables.target_repos as string[] | undefined;
92+
const orgsToCheck = Array.from(
93+
new Set(
94+
(targetRepos ?? [])
95+
.map((value) => parseRepoBranches(value).repo.split('/')[0])
96+
.filter((owner): owner is string => Boolean(owner)),
97+
),
98+
);
99+
100+
if (orgsToCheck.length === 0) {
105101
ctx.fail({
106-
title: 'No GitHub organizations found',
102+
title: 'No repositories configured',
107103
description:
108-
'The connected GitHub account is not a member of any organizations. 2FA enforcement is an organization-level setting.',
109-
resourceType: 'organization',
104+
'No repositories are configured for 2FA enforcement checking. Please select at least one repository.',
105+
resourceType: 'integration',
110106
resourceId: 'github',
111107
severity: 'low',
112-
remediation:
113-
'Connect a GitHub account that belongs to at least one organization.',
108+
remediation: 'Open the integration settings and select repositories to monitor.',
114109
});
115110
return;
116111
}
117112

118-
ctx.log(`Found ${orgs.length} organization(s). Checking 2FA status...`);
113+
ctx.log(
114+
`Checking 2FA for ${orgsToCheck.length} organization(s) derived from selected repos: ${orgsToCheck.join(', ')}`,
115+
);
119116

120117
// Step 2: For each org, check for members without 2FA
121-
for (const org of orgs) {
122-
ctx.log(`Checking 2FA for organization: ${org.login}`);
123-
const orgSlug = encodeURIComponent(org.login);
118+
for (const orgLogin of orgsToCheck) {
119+
ctx.log(`Checking 2FA for organization: ${orgLogin}`);
120+
const orgSlug = encodeURIComponent(orgLogin);
124121
const checkedAt = new Date().toISOString();
125122

126123
let membersWithout2FA: GitHubOrgMember[];
@@ -132,13 +129,13 @@ export const twoFactorAuthCheck: IntegrationCheck = {
132129
const errorMsg = error instanceof Error ? error.message : String(error);
133130

134131
if (isSamlSsoError(errorMsg)) {
135-
ctx.warn(`Cannot check 2FA for ${org.login}: SSO authorization is required.`);
132+
ctx.warn(`Cannot check 2FA for ${orgLogin}: SSO authorization is required.`);
136133
ctx.fail({
137-
title: `Cannot verify 2FA for ${org.login}`,
134+
title: `Cannot verify 2FA for ${orgLogin}`,
138135
description:
139136
'GitHub organization SSO authorization is required to access organization members.',
140137
resourceType: 'organization',
141-
resourceId: org.login,
138+
resourceId: orgLogin,
142139
severity: 'medium',
143140
remediation:
144141
'Authorize this OAuth app for your organization SSO, then rerun the check.',
@@ -147,43 +144,45 @@ export const twoFactorAuthCheck: IntegrationCheck = {
147144
}
148145

149146
if (isRateLimitError(error, errorMsg)) {
150-
ctx.warn(`Rate limit reached while checking 2FA for ${org.login}.`);
147+
ctx.warn(`Rate limit reached while checking 2FA for ${orgLogin}.`);
151148
ctx.fail({
152-
title: `Rate limited while checking ${org.login}`,
149+
title: `Rate limited while checking ${orgLogin}`,
153150
description:
154151
'GitHub rate limits prevented completion of this 2FA check for the organization.',
155152
resourceType: 'organization',
156-
resourceId: org.login,
153+
resourceId: orgLogin,
157154
severity: 'low',
158155
remediation: 'Wait for the GitHub rate limit to reset, then rerun the check.',
159156
});
160157
continue;
161158
}
162159

163-
// GitHub returns 422 when the caller is not an org owner for 2fa_* filters.
160+
// The user explicitly selected a repo in this org but isn't an owner.
161+
// Surface as a finding so they know to either reconnect with an owner
162+
// account or remove the repo from the selection.
164163
if (isOwnerPermissionError(error, errorMsg)) {
165164
ctx.warn(
166-
`Cannot check 2FA for ${org.login}: the account must be an organization owner to use the 2FA filter.`,
165+
`Cannot check 2FA for ${orgLogin}: the account must be an organization owner to use the 2FA filter.`,
167166
);
168167
ctx.fail({
169-
title: `Cannot verify 2FA for ${org.login}`,
168+
title: `Cannot verify 2FA for ${orgLogin}`,
170169
description:
171170
'Insufficient permissions to check 2FA status. The `filter=2fa_disabled` parameter is only available to organization owners on GitHub.',
172171
resourceType: 'organization',
173-
resourceId: org.login,
172+
resourceId: orgLogin,
174173
severity: 'medium',
175174
remediation:
176-
'Reconnect the GitHub integration with an account that is an owner of this organization.',
175+
'Reconnect the GitHub integration with an account that is an owner of this organization, or remove the org\'s repositories from the selection.',
177176
});
178177
continue;
179178
}
180179

181-
ctx.error(`Failed to check 2FA for ${org.login}: ${errorMsg}`);
180+
ctx.error(`Failed to check 2FA for ${orgLogin}: ${errorMsg}`);
182181
ctx.fail({
183-
title: `Error checking 2FA for ${org.login}`,
182+
title: `Error checking 2FA for ${orgLogin}`,
184183
description: `Failed to query members without 2FA: ${errorMsg}`,
185184
resourceType: 'organization',
186-
resourceId: org.login,
185+
resourceId: orgLogin,
187186
severity: 'medium',
188187
remediation: 'Check the integration connection and try again.',
189188
});
@@ -194,12 +193,12 @@ export const twoFactorAuthCheck: IntegrationCheck = {
194193

195194
if (without2FACount === 0) {
196195
ctx.pass({
197-
title: `All members have 2FA enabled in ${org.login}`,
198-
description: `No members without 2FA were returned for ${org.login}.`,
196+
title: `All members have 2FA enabled in ${orgLogin}`,
197+
description: `No members without 2FA were returned for ${orgLogin}.`,
199198
resourceType: 'organization',
200-
resourceId: org.login,
199+
resourceId: orgLogin,
201200
evidence: {
202-
organization: org.login,
201+
organization: orgLogin,
203202
membersWithout2FA: 0,
204203
checkedAt,
205204
},
@@ -209,13 +208,13 @@ export const twoFactorAuthCheck: IntegrationCheck = {
209208
for (const member of membersWithout2FA) {
210209
ctx.fail({
211210
title: `2FA not enabled: ${member.login}`,
212-
description: `GitHub user @${member.login} in the ${org.login} organization does not have two-factor authentication enabled.`,
211+
description: `GitHub user @${member.login} in the ${orgLogin} organization does not have two-factor authentication enabled.`,
213212
resourceType: 'user',
214-
resourceId: `${org.login}/${member.login}`,
213+
resourceId: `${orgLogin}/${member.login}`,
215214
severity: 'high',
216-
remediation: `Ask @${member.login} to enable 2FA in their GitHub account settings (Settings > Password and authentication > Two-factor authentication). Alternatively, enforce 2FA at the organization level in ${org.login}'s settings.`,
215+
remediation: `Ask @${member.login} to enable 2FA in their GitHub account settings (Settings > Password and authentication > Two-factor authentication). Alternatively, enforce 2FA at the organization level in ${orgLogin}'s settings.`,
217216
evidence: {
218-
organization: org.login,
217+
organization: orgLogin,
219218
username: member.login,
220219
userId: member.id,
221220
profileUrl: member.html_url,
@@ -226,14 +225,14 @@ export const twoFactorAuthCheck: IntegrationCheck = {
226225

227226
// Also emit a summary
228227
ctx.fail({
229-
title: `${without2FACount} member(s) without 2FA in ${org.login}`,
230-
description: `${without2FACount} member(s) in the ${org.login} organization do not have two-factor authentication enabled: ${formatUsernames(membersWithout2FA)}`,
228+
title: `${without2FACount} member(s) without 2FA in ${orgLogin}`,
229+
description: `${without2FACount} member(s) in the ${orgLogin} organization do not have two-factor authentication enabled: ${formatUsernames(membersWithout2FA)}`,
231230
resourceType: 'organization',
232-
resourceId: `${org.login}/2fa-summary`,
231+
resourceId: `${orgLogin}/2fa-summary`,
233232
severity: 'high',
234-
remediation: `1. Go to https://github.com/organizations/${org.login}/settings/security\n2. Under "Authentication security", check "Require two-factor authentication for everyone"\n3. This will require all existing and future members to enable 2FA`,
233+
remediation: `1. Go to https://github.com/organizations/${orgLogin}/settings/security\n2. Under "Authentication security", check "Require two-factor authentication for everyone"\n3. This will require all existing and future members to enable 2FA`,
235234
evidence: {
236-
organization: org.login,
235+
organization: orgLogin,
237236
membersWithout2FA: without2FACount,
238237
usernames: membersWithout2FA.map((member) => member.login),
239238
checkedAt,

0 commit comments

Comments
 (0)