Skip to content

fix(web): enforce GitHub installation admin proof #5477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
RSO merged 1 commit into from
Aug 25, 2026
+88 −70
Merged

fix(web): enforce GitHub installation admin proof #5477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 41 additions & 32 deletions apps/web/src/app/api/integrations/github/callback/route.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,15 @@ const OTHER_USER_ID = 'c00b91a1-6959-4b04-9ef8-e8d37b340f4a';
const GITHUB_USER_ID = '12345';
const INSTALLATION_ID = '98765';

beforeEach(() => {
mockedExchangeGitHubOAuthCode.mockResolvedValue({
id: GITHUB_USER_ID,
login: 'octocat',
accessToken: 'ghu_test-token',
});
mockedAssertUserAdministersInstallation.mockResolvedValue(true);
});

function makeRequest(pathWithQuery: string) {
return new NextRequest(`http://localhost:3000${pathWithQuery}`);
}
Expand Down Expand Up @@ -288,7 +297,7 @@ describe('GET /api/integrations/github/callback installation flow', () => {
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=update&state=user_${USER_ID}%7Creturn%3D%252Fgithub-app`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=update&state=user_${USER_ID}%7Creturn%3D%252Fgithub-app&code=abc`
) as never
);

Expand Down Expand Up @@ -360,7 +369,7 @@ describe('GET /api/integrations/github/callback database-backed install flow', (
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}&code=abc`
) as never
);

Expand Down Expand Up @@ -506,7 +515,7 @@ describe('GET /api/integrations/github/callback database-backed install flow', (
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${PREFIXED_DB_TOKEN}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${PREFIXED_DB_TOKEN}&code=abc`
) as never
);

Expand Down Expand Up @@ -541,7 +550,7 @@ describe('GET /api/integrations/github/callback database-backed install flow', (
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}&code=abc`
) as never
);

Expand Down Expand Up @@ -569,7 +578,7 @@ describe('GET /api/integrations/github/callback database-backed install flow', (
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}&code=abc`
) as never
);

Expand Down Expand Up @@ -630,7 +639,7 @@ describe('GET /api/integrations/github/callback database-backed install flow', (
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}&code=abc`
) as never
);

Expand Down Expand Up @@ -662,7 +671,7 @@ describe('GET /api/integrations/github/callback database-backed install flow', (
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}&code=abc`
) as never
);

Expand Down Expand Up @@ -723,7 +732,7 @@ describe('GET /api/integrations/github/callback database-backed install flow', (
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}&code=abc`
) as never
);

Expand Down Expand Up @@ -783,7 +792,7 @@ describe('GET /api/integrations/github/callback database-backed install flow', (
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}&code=abc`
) as never
);

Expand Down Expand Up @@ -812,7 +821,7 @@ describe('GET /api/integrations/github/callback database-backed install flow', (
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${DB_TOKEN}&code=abc`
) as never
);

Expand Down Expand Up @@ -902,7 +911,7 @@ describe('GET /api/integrations/github/callback legacy flag gating', () => {
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=org_${USER_ID}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=org_${USER_ID}&code=abc`
) as never
);

Expand Down Expand Up @@ -935,7 +944,7 @@ describe('GET /api/integrations/github/callback legacy flag gating', () => {
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=user_${USER_ID}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=user_${USER_ID}&code=abc`
) as never
);

Expand All @@ -950,7 +959,7 @@ describe('GET /api/integrations/github/callback legacy flag gating', () => {
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=user_${USER_ID}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=user_${USER_ID}&code=abc`
) as never
);

Expand All @@ -965,7 +974,7 @@ describe('GET /api/integrations/github/callback legacy flag gating', () => {
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=user_${USER_ID}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=user_${USER_ID}&code=abc`
) as never
);

Expand All @@ -975,7 +984,7 @@ describe('GET /api/integrations/github/callback legacy flag gating', () => {
});
});

describe('GET /api/integrations/github/callback admin proof (report mode)', () => {
describe('GET /api/integrations/github/callback admin proof', () => {
beforeEach(() => {
jest.clearAllMocks();

Expand Down Expand Up @@ -1019,7 +1028,7 @@ describe('GET /api/integrations/github/callback admin proof (report mode)', () =
});
});

test('completes an install when code is absent (report mode)', async () => {
test('rejects an install when code is absent', async () => {
const { GET } = await import('./route');
const response = await GET(
makeRequest(
Expand All @@ -1028,14 +1037,14 @@ describe('GET /api/integrations/github/callback admin proof (report mode)', () =
);

expect(response.status).toBe(307);
expectRedirectLocation(response, `/integrations/github?success=installed`);
expect(mockedUpsertPlatformIntegrationForOwner).toHaveBeenCalled();
// Admin proof was not run.
expectRedirectLocation(response, `/integrations/github?error=not_installation_admin`);
expect(mockedUpsertPlatformIntegrationForOwner).not.toHaveBeenCalled();
expect(mockedExchangeGitHubOAuthCode).not.toHaveBeenCalled();
expect(mockedAssertUserAdministersInstallation).not.toHaveBeenCalled();
expect(mockedCreateAppAuth).not.toHaveBeenCalled();
});

test('completes an install when code is present and user is admin (report mode)', async () => {
test('completes an install when code is present and user is admin', async () => {
const { GET } = await import('./route');
const response = await GET(
makeRequest(
Expand All @@ -1053,7 +1062,7 @@ describe('GET /api/integrations/github/callback admin proof (report mode)', () =
expect(mockedUpsertPlatformIntegrationForOwner).toHaveBeenCalled();
});

test('still completes install when admin check returns false (report mode does not block non-admin)', async () => {
test('rejects an install when admin check returns false', async () => {
mockedAssertUserAdministersInstallation.mockResolvedValue(false);

const { GET } = await import('./route');
Expand All @@ -1063,15 +1072,15 @@ describe('GET /api/integrations/github/callback admin proof (report mode)', () =
) as never
);

// Report mode: non-admin is logged but the install proceeds.
expect(response.status).toBe(307);
expectRedirectLocation(response, `/integrations/github?success=installed`);
expectRedirectLocation(response, `/integrations/github?error=not_installation_admin`);
expect(mockedExchangeGitHubOAuthCode).toHaveBeenCalled();
expect(mockedAssertUserAdministersInstallation).toHaveBeenCalled();
expect(mockedUpsertPlatformIntegrationForOwner).toHaveBeenCalled();
expect(mockedUpsertPlatformIntegrationForOwner).not.toHaveBeenCalled();
expect(mockedCreateAppAuth).not.toHaveBeenCalled();
});

test('still completes install when code exchange fails (report mode)', async () => {
test('rejects an install when code exchange fails', async () => {
mockedExchangeGitHubOAuthCode.mockRejectedValue(new Error('Token exchange failed'));

const { GET } = await import('./route');
Expand All @@ -1081,10 +1090,10 @@ describe('GET /api/integrations/github/callback admin proof (report mode)', () =
) as never
);

// API failure in admin proof does not block the install.
expect(response.status).toBe(307);
expectRedirectLocation(response, `/integrations/github?success=installed`);
expect(mockedUpsertPlatformIntegrationForOwner).toHaveBeenCalled();
expectRedirectLocation(response, `/integrations/github?error=not_installation_admin`);
expect(mockedUpsertPlatformIntegrationForOwner).not.toHaveBeenCalled();
expect(mockedCreateAppAuth).not.toHaveBeenCalled();
});

test('redirects with installation_already_claimed when upsert detects cross-owner claim', async () => {
Expand All @@ -1096,15 +1105,15 @@ describe('GET /api/integrations/github/callback admin proof (report mode)', () =
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=user_${USER_ID}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=user_${USER_ID}&code=abc`
) as never
);

expect(response.status).toBe(307);
expectRedirectLocation(response, `/integrations/github?error=installation_already_claimed`);
});

test('logs distinct messages for code-absent vs non-admin (report mode distinguishes)', async () => {
test('logs distinct messages for code-absent vs non-admin', async () => {
const logSpy = jest.spyOn(console, 'log').mockImplementation(() => {});

// Case 1: code absent — should log code_absent.
Expand Down Expand Up @@ -1190,7 +1199,7 @@ describe('GET /api/integrations/github/callback Sentry redaction', () => {
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${RAW_TOKEN}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${RAW_TOKEN}&code=abc`
) as never
);

Expand Down Expand Up @@ -1234,7 +1243,7 @@ describe('GET /api/integrations/github/callback Sentry redaction', () => {
const { GET } = await import('./route');
const response = await GET(
makeRequest(
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${RAW_TOKEN}`
`/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${RAW_TOKEN}&code=abc`
) as never
);

Expand Down
85 changes: 47 additions & 38 deletions apps/web/src/app/api/integrations/github/callback/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,53 +528,62 @@ async function handleCoreInstallFlow(params: {
);
}

// Admin proof — report mode. The GitHub App does not yet request OAuth
// authorization during installation. When `code` is present we verify
// administration and log the outcome; when absent we log and proceed.
// A follow-up commit will hard-require `code` after the App setting is
// enabled in the GitHub App dashboard.
// Require proof that the OAuth-authorized GitHub user administers the
// installation before using app credentials to fetch or persist it.
if (setupAction === 'install' || setupAction === 'update') {
const code = searchParams.get('code');
const rejectUnauthorizedInstallation = () =>
NextResponse.redirect(
new URL(
isAppInitiated
? appFallbackPath('error=not_installation_admin')
: appendQueryParam(redirectPath, 'error=not_installation_admin'),
APP_URL
)
);

if (code) {
try {
const exchangeResult = await exchangeGitHubOAuthCode(code, githubAppType);
const isAdmin = await assertUserAdministersInstallation({
accessToken: exchangeResult.accessToken,
installationId,
});
if (!code) {
console.log('[github_admin_proof:code_absent]', {
installation_id: installationId,
setup_action: setupAction,
});
return rejectUnauthorizedInstallation();
}

if (isAdmin) {
console.log('[github_admin_proof:pass]', {
github_user_id: exchangeResult.id,
github_user_login: exchangeResult.login,
installation_id: installationId,
});
} else {
console.log('[github_admin_proof:fail_non_admin]', {
github_user_id: exchangeResult.id,
github_user_login: exchangeResult.login,
installation_id: installationId,
});
}
} catch (error) {
console.error('[github_admin_proof:error]', {
try {
const exchangeResult = await exchangeGitHubOAuthCode(code, githubAppType);
const isAdmin = await assertUserAdministersInstallation({
accessToken: exchangeResult.accessToken,
installationId,
});

if (!isAdmin) {
console.log('[github_admin_proof:fail_non_admin]', {
github_user_id: exchangeResult.id,
github_user_login: exchangeResult.login,
installation_id: installationId,
error: (error as Error).message,
});
captureException(error, {
tags: {
endpoint: 'github/callback',
source: 'github_admin_proof',
},
extra: { installationId },
});
return rejectUnauthorizedInstallation();
}
} else {
console.log('[github_admin_proof:code_absent]', {

console.log('[github_admin_proof:pass]', {
github_user_id: exchangeResult.id,
github_user_login: exchangeResult.login,
installation_id: installationId,
setup_action: setupAction,
});
} catch (error) {
console.error('[github_admin_proof:error]', {
installation_id: installationId,
error: error instanceof Error ? error.message : String(error),
});
captureException(error, {
tags: {
endpoint: 'github/callback',
source: 'github_admin_proof',
},
extra: { installationId },
});
return rejectUnauthorizedInstallation();
}
}

Expand Down