From 6f087f7bad443a514d9def94cf6fe92552b9f2ea Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 00:39:59 -0400 Subject: [PATCH 01/13] fix(github): resume installation permission upgrades --- docs/github-design.md | 16 +++ specs/github-adapter.feature | 10 ++ .../github/external-authorization.ts | 37 +++++- .../github-external-authorization.test.ts | 118 ++++++++++++++++++ 4 files changed, 177 insertions(+), 4 deletions(-) diff --git a/docs/github-design.md b/docs/github-design.md index b7a05c2..404348f 100644 --- a/docs/github-design.md +++ b/docs/github-design.md @@ -38,6 +38,22 @@ human-facing display value: the GitHub account login is the Context label and the installation ID remains stable metadata. Realmroot and Toolbox never use the display label as authorization input. +## Installation permission upgrades + +The GitHub App Setup URL is +`https://adapters.realmroot.dev/github/account-connection-installations`, with +GitHub's **Redirect on update** option enabled. When an existing installation +lacks a newly requested App permission, the Adapter keeps the original OAuth +intent and redirects the owner through GitHub's installation update flow. The +Setup URL accepts only the installation selected by the authorization detail, +then resumes the same Realmroot authorization transaction. The Adapter retries +the permission check once and fails closed if the owner did not approve the +update. + +Incomplete permission upgrades are not persisted as connected authority and do +not surface an intermediate Adapter error page during the normal approval +flow. + ## Permission translation The adapter reads the permissions configured on the GitHub App and exposes diff --git a/specs/github-adapter.feature b/specs/github-adapter.feature index 4dc1335..792ff94 100644 --- a/specs/github-adapter.feature +++ b/specs/github-adapter.feature @@ -24,6 +24,16 @@ Feature: GitHub App adapter Then the adapter keeps one stable external subject for the owner And replaces the installation authorization details with the newly authorized set + @journey:github-installation-permission-upgrade @entrypoint:http + Scenario: Authorization resumes after a GitHub installation permission upgrade + Given an existing GitHub App installation lacks a newly requested permission + When the owner authorizes that permission through Realmroot + Then the adapter preserves the original authorization transaction + And redirects through the GitHub installation update flow + And accepts only the target installation + And resumes the same Realmroot authorization after GitHub returns + But it does not show an adapter insufficient-scope page + @journey:github-context-catalog @entrypoint:http Scenario: GitHub describes installation Contexts without exposing credentials Given the Adapter holds an active GitHub external authorization diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index 0b00969..accbefc 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -117,7 +117,8 @@ export function createGitHubExternalAuthorization(input: { input.connection.getUser(userToken), input.connection.listUserInstallations(userToken), ]) - const expectedInstallationId = numberValue(intent.providerData.expectedInstallationId) + const expectedInstallationId = + numberValue(intent.providerData.expectedInstallationId) ?? requestedInstallationId(intent.authorizationDetails) if (expectedInstallationId && !installations.some((installation) => installation.id === expectedInstallationId)) { throw forbidden('The GitHub user cannot manage the selected App installation.') } @@ -134,14 +135,28 @@ export function createGitHubExternalAuthorization(input: { const selected = expectedInstallationId ? installations.filter((installation) => installation.id === expectedInstallationId) : installations - const contexts = await input.connections.upsertExternalAuthorization(user, selected) const providerScopes = new Set(selected.flatMap((installation) => permissionsToScopes(installation.permissions))) const requestedProviderScopes = intent.scopes.filter( (scope) => !['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope), ) if (requestedProviderScopes.some((scope) => !providerScopes.has(scope))) { - throw forbidden('The selected GitHub installation does not grant every requested scope.') + const selectedInstallation = selected.length === 1 ? selected[0] : undefined + if (intent.providerData.permissionUpdateAttempted === true || !selectedInstallation) { + throw forbidden('The selected GitHub installation permission update was not approved.') + } + const providerState = nextProviderState() + return { + type: 'continue', + providerState, + stage: 'install', + data: { + expectedInstallationId: selectedInstallation.id, + permissionUpdateAttempted: true, + }, + url: await input.connection.newInstallationUrl(providerState), + } } + const contexts = await input.connections.upsertExternalAuthorization(user, selected) return { type: 'complete', grant: { @@ -169,13 +184,17 @@ export function createGitHubExternalAuthorization(input: { if (intent.providerId !== 'github' || intent.providerStage !== 'install') { throw badRequest('GitHub installation authorization state is invalid.') } + const expectedInstallationId = numberValue(intent.providerData.expectedInstallationId) + if (expectedInstallationId && expectedInstallationId !== installationId) { + throw forbidden('GitHub returned a different App installation than the authorization requested.') + } const providerState = randomToken() await input.oauthStore.advanceIntent({ id: intent.id, expectedStage: 'install', providerStateHash: await sha256(providerState), providerStage: 'oauth-selected', - providerData: { expectedInstallationId: installationId }, + providerData: { ...intent.providerData, expectedInstallationId: installationId }, }) return c.redirect(input.connection.authorizationUrl(providerState)) }) @@ -188,6 +207,16 @@ function numberValue(value: unknown) { return typeof value === 'number' && Number.isSafeInteger(value) && value > 0 ? value : null } +function requestedInstallationId(authorizationDetails: Array>) { + const ids = authorizationDetails.flatMap((detail) => { + if (detail.type !== GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE) return [] + if (typeof detail.installation_id !== 'string' || !/^\d+$/.test(detail.installation_id)) return [] + const id = Number(detail.installation_id) + return Number.isSafeInteger(id) && id > 0 ? [id] : [] + }) + return ids.length === 1 ? ids[0] : null +} + function required(value: string | null | undefined, name: string) { if (!value) throw badRequest(`${name} is required.`) return value diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index 4bacf0d..eb1cf8d 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -1,3 +1,4 @@ +import { Hono } from 'hono' import { describe, expect, it, vi } from 'vitest' import type { D1ExternalOAuthStore, ExternalOAuthIntent } from '../../src/core/external-oauth-store.js' import { GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE } from '../../src/providers/github/authorization-details.js' @@ -100,8 +101,125 @@ describe('GitHub external authorization', () => { grant: { scopes: ['metadata:read', 'openid', 'pull_requests:read'] }, }) }) + + it('[spec: github-adapter/github-installation-permission-upgrade] continues through a target installation permission update', async () => { + const installation = githubInstallation({ administration: 'read' }) + const connections = { upsertExternalAuthorization: vi.fn() } + const connection = githubConnection([installation]) + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection, + connections: connections as unknown as D1GitHubConnections, + oauthStore: {} as D1ExternalOAuthStore, + scopes: ['administration:read', 'administration:write'], + }) + + await expect( + external.authorization.complete({ + callbackUrl: 'https://adapter.example/github/oauth/callback?code=provider-code', + intent: { + ...intent(), + scopes: ['administration:write', 'openid'], + authorizationDetails: [{ type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: '701' }], + }, + nextProviderState: () => 'permission-update-state', + }), + ).resolves.toEqual({ + type: 'continue', + providerState: 'permission-update-state', + stage: 'install', + data: { expectedInstallationId: 701, permissionUpdateAttempted: true }, + url: 'https://github.com/apps/example/installations/new?state=permission-update-state', + }) + expect(connection.newInstallationUrl).toHaveBeenCalledWith('permission-update-state') + expect(connections.upsertExternalAuthorization).not.toHaveBeenCalled() + }) + + it('[spec: github-adapter/github-installation-permission-upgrade] stops after one rejected permission update', async () => { + const connections = { upsertExternalAuthorization: vi.fn() } + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection: githubConnection([githubInstallation({ administration: 'read' })]), + connections: connections as unknown as D1GitHubConnections, + oauthStore: {} as D1ExternalOAuthStore, + scopes: ['administration:read', 'administration:write'], + }) + + await expect( + external.authorization.complete({ + callbackUrl: 'https://adapter.example/github/oauth/callback?code=provider-code', + intent: { + ...intent(), + scopes: ['administration:write', 'openid'], + providerStage: 'oauth-selected', + providerData: { expectedInstallationId: 701, permissionUpdateAttempted: true }, + }, + nextProviderState: () => 'must-not-loop', + }), + ).rejects.toMatchObject({ status: 403 }) + expect(connections.upsertExternalAuthorization).not.toHaveBeenCalled() + }) + + it('preserves the permission update intent when GitHub returns through the Setup URL', async () => { + const oauthStore = { + intentByProviderState: vi.fn(async () => ({ + ...intent(), + providerStage: 'install', + providerData: { expectedInstallationId: 701, permissionUpdateAttempted: true }, + })), + advanceIntent: vi.fn(async () => undefined), + } + const connection = githubConnection([]) + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection, + connections: {} as D1GitHubConnections, + oauthStore: oauthStore as unknown as D1ExternalOAuthStore, + scopes: ['administration:write'], + }) + const app = new Hono() + external.installationCallback.register(app as never) + + const response = await app.request( + 'https://adapter.example/github/account-connection-installations?state=permission-update-state&installation_id=701', + ) + + expect(response.status).toBe(302) + expect(response.headers.get('location')).toMatch(/^https:\/\/github\.com\/login\/oauth\/authorize\?state=/) + expect(oauthStore.advanceIntent).toHaveBeenCalledWith({ + id: 'intent-1', + expectedStage: 'install', + providerStateHash: expect.any(String), + providerStage: 'oauth-selected', + providerData: { expectedInstallationId: 701, permissionUpdateAttempted: true }, + }) + }) }) +function githubInstallation(permissions: Record) { + return { + id: 701, + accountLogin: 'realmroot', + targetType: 'Organization', + permissions: { metadata: 'read' as const, ...permissions }, + repositorySelection: 'all' as const, + repositories: [], + updatedAt: '2027-01-15T07:00:00.000Z', + } +} + +function githubConnection(installations: ReturnType[]) { + return { + authorizationUrl: vi.fn((state: string) => `https://github.com/login/oauth/authorize?state=${state}`), + exchangeUserCode: vi.fn(async () => 'user-token'), + getUser: vi.fn(async () => ({ id: 70, login: 'controller', name: 'Controller' })), + listUserInstallations: vi.fn(async () => installations), + newInstallationUrl: vi.fn( + async (state: string) => `https://github.com/apps/example/installations/new?state=${state}`, + ), + } +} + function intent(): ExternalOAuthIntent { return { id: 'intent-1', From 7ad2a10775d5551d825f233518eb5b139b07eb29 Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 00:50:44 -0400 Subject: [PATCH 02/13] fix(github): target installation permission review --- docs/github-design.md | 10 +++---- src/providers/github/client.ts | 14 +++++++++ .../github/external-authorization.ts | 2 +- src/providers/github/types.ts | 1 + test/providers/github-client.test.ts | 30 +++++++++++++++++++ .../github-external-authorization.test.ts | 11 +++++-- 6 files changed, 60 insertions(+), 8 deletions(-) diff --git a/docs/github-design.md b/docs/github-design.md index 404348f..cd5453e 100644 --- a/docs/github-design.md +++ b/docs/github-design.md @@ -44,11 +44,11 @@ The GitHub App Setup URL is `https://adapters.realmroot.dev/github/account-connection-installations`, with GitHub's **Redirect on update** option enabled. When an existing installation lacks a newly requested App permission, the Adapter keeps the original OAuth -intent and redirects the owner through GitHub's installation update flow. The -Setup URL accepts only the installation selected by the authorization detail, -then resumes the same Realmroot authorization transaction. The Adapter retries -the permission check once and fails closed if the owner did not approve the -update. +intent and redirects the owner to that installation's dedicated +`/permissions/update` page. The Setup URL accepts only the installation +selected by the authorization detail, then resumes the same Realmroot +authorization transaction. The Adapter retries the permission check once and +fails closed if the owner did not approve the update. Incomplete permission upgrades are not persisted as connected authority and do not surface an intermediate Adapter error page during the normal approval diff --git a/src/providers/github/client.ts b/src/providers/github/client.ts index 4f776f8..6512b0f 100644 --- a/src/providers/github/client.ts +++ b/src/providers/github/client.ts @@ -166,6 +166,20 @@ export function createGitHubConnectionProvider( url.searchParams.set('state', state) return url.toString() }, + installationPermissionUpdateUrl(installation, state) { + const url = + installation.targetType === 'Organization' + ? new URL( + `/organizations/${encodeURIComponent(installation.accountLogin)}/settings/installations/${installation.id}/permissions/update`, + 'https://github.com', + ) + : installation.targetType === 'User' + ? new URL(`/settings/installations/${installation.id}/permissions/update`, 'https://github.com') + : null + if (!url) throw failedDependency(`GitHub installation target type ${installation.targetType} is unsupported.`) + url.searchParams.set('state', state) + return url.toString() + }, } async function listInstallationRepositories(token: string, installationId: number) { diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index accbefc..eb6f9cd 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -153,7 +153,7 @@ export function createGitHubExternalAuthorization(input: { expectedInstallationId: selectedInstallation.id, permissionUpdateAttempted: true, }, - url: await input.connection.newInstallationUrl(providerState), + url: input.connection.installationPermissionUpdateUrl(selectedInstallation, providerState), } } const contexts = await input.connections.upsertExternalAuthorization(user, selected) diff --git a/src/providers/github/types.ts b/src/providers/github/types.ts index 9169d87..2ed177e 100644 --- a/src/providers/github/types.ts +++ b/src/providers/github/types.ts @@ -33,4 +33,5 @@ export interface GitHubConnectionProvider { getUser(token: string): Promise listUserInstallations(token: string): Promise newInstallationUrl(state: string): Promise + installationPermissionUpdateUrl(installation: GitHubInstallation, state: string): string } diff --git a/test/providers/github-client.test.ts b/test/providers/github-client.test.ts index 16a8e31..5e8b515 100644 --- a/test/providers/github-client.test.ts +++ b/test/providers/github-client.test.ts @@ -76,6 +76,36 @@ describe('GitHub provider HTTP boundary', () => { }) describe('GitHub account connection OAuth boundary', () => { + it('targets the selected installation permission review page', () => { + const provider = createGitHubConnectionProvider({ + appId: '123', + privateKey: privateKey('pkcs8'), + clientId: 'client-id', + clientSecret: 'client-secret', + redirectUri: 'https://adapters.realmroot.dev/github/oauth/callback', + apiOrigin: 'https://api.github.test', + }) + const installation = { + id: 42, + accountLogin: 'realmroot', + targetType: 'Organization', + permissions: { metadata: 'read' as const }, + repositorySelection: 'all' as const, + repositories: [], + updatedAt: '2027-01-15T08:00:00+00:00', + } + + expect(provider.installationPermissionUpdateUrl(installation, 'provider-state')).toBe( + 'https://github.com/organizations/realmroot/settings/installations/42/permissions/update?state=provider-state', + ) + expect( + provider.installationPermissionUpdateUrl( + { ...installation, accountLogin: 'controller', targetType: 'User' }, + 'provider-state', + ), + ).toBe('https://github.com/settings/installations/42/permissions/update?state=provider-state') + }) + it('selects the adapter callback when the GitHub App has multiple callback URLs', async () => { const requests: Array<{ url: string; body: unknown }> = [] const provider = createGitHubConnectionProvider({ diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index eb1cf8d..356f195 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -84,6 +84,9 @@ describe('GitHub external authorization', () => { getUser: vi.fn(async () => ({ id: 70, login: 'controller', name: 'Controller' })), listUserInstallations: vi.fn(async () => [installation]), newInstallationUrl: vi.fn(async () => 'https://github.com/apps/example/installations/new'), + installationPermissionUpdateUrl: vi.fn( + () => 'https://github.com/organizations/realmroot/settings/installations/701/permissions/update', + ), }, connections: connections as unknown as D1GitHubConnections, oauthStore: {} as D1ExternalOAuthStore, @@ -129,9 +132,9 @@ describe('GitHub external authorization', () => { providerState: 'permission-update-state', stage: 'install', data: { expectedInstallationId: 701, permissionUpdateAttempted: true }, - url: 'https://github.com/apps/example/installations/new?state=permission-update-state', + url: 'https://github.com/organizations/realmroot/settings/installations/701/permissions/update?state=permission-update-state', }) - expect(connection.newInstallationUrl).toHaveBeenCalledWith('permission-update-state') + expect(connection.installationPermissionUpdateUrl).toHaveBeenCalledWith(installation, 'permission-update-state') expect(connections.upsertExternalAuthorization).not.toHaveBeenCalled() }) @@ -217,6 +220,10 @@ function githubConnection(installations: ReturnType[] newInstallationUrl: vi.fn( async (state: string) => `https://github.com/apps/example/installations/new?state=${state}`, ), + installationPermissionUpdateUrl: vi.fn( + (installation: ReturnType, state: string) => + `https://github.com/organizations/${installation.accountLogin}/settings/installations/${installation.id}/permissions/update?state=${state}`, + ), } } From adebfc7453f51734cabccf1fa7788592dc5e0183 Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 01:49:58 -0400 Subject: [PATCH 03/13] fix(github): return declined permission updates --- docs/github-design.md | 5 ++- specs/github-adapter.feature | 3 +- src/core/external-authorization-server.ts | 30 ++++++++++++++ src/core/external-oauth-store.ts | 8 ++++ .../github/external-authorization.ts | 6 ++- .../external-authorization-server.test.ts | 40 +++++++++++++++++++ test/integration/external-oauth-store.test.ts | 37 +++++++++++++++++ .../github-external-authorization.test.ts | 6 ++- 8 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 test/integration/external-oauth-store.test.ts diff --git a/docs/github-design.md b/docs/github-design.md index cd5453e..823ba74 100644 --- a/docs/github-design.md +++ b/docs/github-design.md @@ -45,10 +45,11 @@ The GitHub App Setup URL is GitHub's **Redirect on update** option enabled. When an existing installation lacks a newly requested App permission, the Adapter keeps the original OAuth intent and redirects the owner to that installation's dedicated -`/permissions/update` page. The Setup URL accepts only the installation +`/permissions/update` page with an opaque state. GitHub returns that state to +the Setup URL after the update. The Setup URL accepts only the installation selected by the authorization detail, then resumes the same Realmroot authorization transaction. The Adapter retries the permission check once and -fails closed if the owner did not approve the update. +returns an OAuth denial to Realmroot if the owner did not approve the update. Incomplete permission upgrades are not persisted as connected authority and do not surface an intermediate Adapter error page during the normal approval diff --git a/specs/github-adapter.feature b/specs/github-adapter.feature index 792ff94..d8e70fe 100644 --- a/specs/github-adapter.feature +++ b/specs/github-adapter.feature @@ -29,9 +29,10 @@ Feature: GitHub App adapter Given an existing GitHub App installation lacks a newly requested permission When the owner authorizes that permission through Realmroot Then the adapter preserves the original authorization transaction - And redirects through the GitHub installation update flow + And redirects through the GitHub installation update flow with an opaque state And accepts only the target installation And resumes the same Realmroot authorization after GitHub returns + And returns a declined permission update to Realmroot as an OAuth denial But it does not show an adapter insufficient-scope page @journey:github-context-catalog @entrypoint:http diff --git a/src/core/external-authorization-server.ts b/src/core/external-authorization-server.ts index c93d333..16119a5 100644 --- a/src/core/external-authorization-server.ts +++ b/src/core/external-authorization-server.ts @@ -64,6 +64,7 @@ export type ExternalProviderAuthorization = { }): Promise< | { type: 'continue'; url: string; stage: string; data: Record; providerState: string } | { type: 'complete'; grant: Omit } + | { type: 'error'; error: 'access_denied' | 'server_error'; description: string } > } @@ -170,6 +171,19 @@ export async function createExternalAuthorizationServer(input: { app.get(providerCallbackPath, async (c) => { const state = required(c.req.query('state'), 'state') const intent = await input.store.intentByProviderState(await sha256(state)) + const providerError = c.req.query('error') + if (providerError) { + await input.store.cancelIntent(intent) + return c.redirect( + authorizationErrorRedirect( + intent, + providerError === 'access_denied' ? 'access_denied' : 'server_error', + providerError === 'access_denied' + ? 'Provider authorization was denied.' + : 'Provider authorization failed.', + ), + ) + } const result = await input.provider.complete({ callbackUrl: c.req.url, intent, @@ -185,6 +199,10 @@ export async function createExternalAuthorizationServer(input: { }) return c.redirect(result.url) } + if (result.type === 'error') { + await input.store.cancelIntent(intent) + return c.redirect(authorizationErrorRedirect(intent, result.error, result.description)) + } const code = opaque('code') await input.store.completeIntent( intent, @@ -472,6 +490,18 @@ export async function createExternalAuthorizationServer(input: { } } +function authorizationErrorRedirect( + intent: ExternalOAuthIntent, + error: 'access_denied' | 'server_error', + description: string, +) { + const callback = new URL(intent.redirectUri) + callback.searchParams.set('error', error) + callback.searchParams.set('error_description', description) + callback.searchParams.set('state', intent.realmrootState) + return callback.toString() +} + async function authenticateClient(store: D1ExternalOAuthStore, providerId: string, request: Request) { const header = request.headers.get('authorization') if (!header?.startsWith('Basic ')) throw oauthError('invalid_client', 'Client authentication is required.', 401) diff --git a/src/core/external-oauth-store.ts b/src/core/external-oauth-store.ts index 11f44cc..38e0bdf 100644 --- a/src/core/external-oauth-store.ts +++ b/src/core/external-oauth-store.ts @@ -173,6 +173,14 @@ export class D1ExternalOAuthStore { if (results[1]?.meta.changes !== 1) throw unauthorized('External OAuth authorization was already completed.') } + async cancelIntent(intent: ExternalOAuthIntent) { + const result = await this.db + .prepare("DELETE FROM external_oauth_intent WHERE id = ? AND status = 'pending'") + .bind(intent.id) + .run() + if (result.meta.changes !== 1) throw unauthorized('External OAuth authorization was already completed.') + } + async consumeCode(input: { code: string; clientId: string; redirectUri: string; verifier: string }) { const codeHash = await sha256(input.code) const row = await this.db diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index eb6f9cd..fa5f5d4 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -142,7 +142,11 @@ export function createGitHubExternalAuthorization(input: { if (requestedProviderScopes.some((scope) => !providerScopes.has(scope))) { const selectedInstallation = selected.length === 1 ? selected[0] : undefined if (intent.providerData.permissionUpdateAttempted === true || !selectedInstallation) { - throw forbidden('The selected GitHub installation permission update was not approved.') + return { + type: 'error', + error: 'access_denied', + description: 'The selected GitHub installation permission update was not approved.', + } } const providerState = nextProviderState() return { diff --git a/test/core/external-authorization-server.test.ts b/test/core/external-authorization-server.test.ts index ab73132..a374227 100644 --- a/test/core/external-authorization-server.test.ts +++ b/test/core/external-authorization-server.test.ts @@ -171,6 +171,45 @@ describe('external authorization server', () => { ) }) + it('returns provider denial to Realmroot without requiring an authorization code', async () => { + const intent = exampleIntent() + const { app, provider, store } = await testServer({ intent }) + + const response = await app.request( + '/oauth/example/provider/callback?state=provider-state&error=access_denied&error_description=The+user+cancelled', + ) + + expect(response.status).toBe(302) + const callback = new URL(response.headers.get('location') ?? '') + expect(callback.origin + callback.pathname).toBe('https://id.realmroot.dev/callback') + expect(callback.searchParams.get('state')).toBe('realmroot-state') + expect(callback.searchParams.get('error')).toBe('access_denied') + expect(callback.searchParams.get('error_description')).toBe('Provider authorization was denied.') + expect(store.cancelIntent).toHaveBeenCalledWith(intent) + expect(provider.complete).not.toHaveBeenCalled() + }) + + it('returns a provider completion denial to Realmroot instead of an Adapter error page', async () => { + const intent = exampleIntent() + const { app, provider, store } = await testServer({ intent }) + vi.mocked(provider.complete).mockResolvedValueOnce({ + type: 'error', + error: 'access_denied', + description: 'The requested provider permission was not approved.', + }) + + const response = await app.request('/oauth/example/provider/callback?state=provider-state&code=provider-code') + + expect(response.status).toBe(302) + const callback = new URL(response.headers.get('location') ?? '') + expect(callback.origin + callback.pathname).toBe('https://id.realmroot.dev/callback') + expect(callback.searchParams.get('state')).toBe('realmroot-state') + expect(callback.searchParams.get('error')).toBe('access_denied') + expect(callback.searchParams.get('error_description')).toBe('The requested provider permission was not approved.') + expect(store.cancelIntent).toHaveBeenCalledWith(intent) + expect(store.completeIntent).not.toHaveBeenCalled() + }) + it('supports a provider callback path already registered with the upstream OAuth application', async () => { const intent = exampleIntent() const { app, store } = await testServer({ intent, providerCallbackPath: '/example/oauth/callback' }) @@ -198,6 +237,7 @@ async function testServer(options: { intent?: ExternalOAuthIntent; providerCallb intentByProviderState: vi.fn(async () => options.intent ?? exampleIntent()), advanceIntent: vi.fn(async () => undefined), completeIntent: vi.fn(async () => undefined), + cancelIntent: vi.fn(async () => undefined), consumeCode: vi.fn(async () => ({ providerId: 'example', clientId: 'client-1', diff --git a/test/integration/external-oauth-store.test.ts b/test/integration/external-oauth-store.test.ts new file mode 100644 index 0000000..bc50da1 --- /dev/null +++ b/test/integration/external-oauth-store.test.ts @@ -0,0 +1,37 @@ +import { env } from 'cloudflare:test' +import { describe, expect, it } from 'vitest' +import { D1ExternalOAuthStore, type ExternalOAuthIntent, sha256 } from '../../src/core/external-oauth-store.js' + +describe('External OAuth intent persistence', () => { + it('[spec: github-adapter/github-installation-permission-upgrade] consumes a denied authorization intent once', async () => { + const store = new D1ExternalOAuthStore(env.DB) + const intent: ExternalOAuthIntent = { + id: 'github-permission-update-denied', + providerId: 'github', + clientId: 'github-permission-update-client', + redirectUri: 'https://id.realmroot.dev/oauth/account-connection/callback', + realmrootState: 'realmroot-state', + scopes: ['administration:write', 'openid'], + authorizationDetails: [], + codeChallenge: 'challenge', + providerStage: 'oauth-selected', + providerData: { expectedInstallationId: 701, permissionUpdateAttempted: true }, + expiresAt: Date.now() + 60_000, + } + await store.registerClient({ + clientId: intent.clientId, + providerId: 'github', + clientSecretHash: await sha256('secret'), + redirectUris: [intent.redirectUri], + jwksUri: 'https://id.realmroot.dev/api/auth/jwks', + }) + await store.createIntent(intent, await sha256('provider-state')) + + await store.cancelIntent(intent) + + await expect(store.intentByProviderState(await sha256('provider-state'))).rejects.toThrow( + 'External OAuth state is invalid or expired.', + ) + await expect(store.cancelIntent(intent)).rejects.toThrow('External OAuth authorization was already completed.') + }) +}) diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index 356f195..67fb884 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -159,7 +159,11 @@ describe('GitHub external authorization', () => { }, nextProviderState: () => 'must-not-loop', }), - ).rejects.toMatchObject({ status: 403 }) + ).resolves.toEqual({ + type: 'error', + error: 'access_denied', + description: 'The selected GitHub installation permission update was not approved.', + }) expect(connections.upsertExternalAuthorization).not.toHaveBeenCalled() }) From e91d1c26576d914dcd4b38dd70d939c59d2ad09b Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 02:08:20 -0400 Subject: [PATCH 04/13] fix(github): preserve installation catalog on targeted update --- src/providers/github/external-authorization.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index fa5f5d4..04e3376 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -160,7 +160,7 @@ export function createGitHubExternalAuthorization(input: { url: input.connection.installationPermissionUpdateUrl(selectedInstallation, providerState), } } - const contexts = await input.connections.upsertExternalAuthorization(user, selected) + const contexts = await input.connections.upsertExternalAuthorization(user, installations) return { type: 'complete', grant: { From a4a183fbac6e8f09796d268adce45168e9067f74 Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 09:02:46 -0400 Subject: [PATCH 05/13] fix(github): publish context-specific scope coverage --- specs/github-adapter.feature | 1 + src/core/external-authorization-server.ts | 1 + .../github/external-authorization.ts | 14 ++++++ .../github-external-authorization.test.ts | 44 +++++++++++++++++++ 4 files changed, 60 insertions(+) diff --git a/specs/github-adapter.feature b/specs/github-adapter.feature index d8e70fe..6018909 100644 --- a/specs/github-adapter.feature +++ b/specs/github-adapter.feature @@ -40,6 +40,7 @@ Feature: GitHub App adapter Given the Adapter holds an active GitHub external authorization When Realmroot reads the advertised authorization-detail catalog with the connected subject token Then the adapter returns each active installation with its account name, stable installation ID, account type, and repository selection + And reports the exact scopes currently granted by each installation And the response uses the shared authorization-detail catalog representation And the authorization detail type is a stable URI owned by the Adapter But it does not expose installation credentials diff --git a/src/core/external-authorization-server.ts b/src/core/external-authorization-server.ts index 16119a5..a134a53 100644 --- a/src/core/external-authorization-server.ts +++ b/src/core/external-authorization-server.ts @@ -35,6 +35,7 @@ export type ExternalProviderAuthorization = { list(input: { subject: string; limit: number; offset: number }): Promise<{ items: Array<{ authorizationDetail: Record + grantedScopes?: string[] display: { label: string; description?: string; metadata?: Record } }> pagination: { limit: number; offset: number; total: number; hasMore: boolean; nextOffset: number | null } diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index 04e3376..db9f856 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -71,6 +71,7 @@ export function createGitHubExternalAuthorization(input: { const contexts = (await input.connections.externalAuthorization(subject)).contexts const items = contexts.slice(offset, offset + limit).map((context) => ({ authorizationDetail: githubInstallationAuthorizationDetail(context), + grantedScopes: context.scopes, display: githubInstallationAuthorizationDetailDisplay(context), })) const nextOffset = offset + limit < contexts.length ? offset + limit : null @@ -89,12 +90,25 @@ export function createGitHubExternalAuthorization(input: { authorizationDetailsSubset, async validateGrant({ subject, scopes, authorizationDetails }) { const active = await input.connections.externalAuthorization(subject) + const requestedScopes = scopes.filter( + (scope) => !['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope), + ) + const selectedContexts = authorizationDetails.map((detail) => + active.contexts.find( + (context) => + detail.type === GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE && + detail.installation_id === String(context.installationId), + ), + ) return ( scopes.every( (scope) => ['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope) || active.scopes.includes(scope), ) && + selectedContexts.every( + (context) => context !== undefined && requestedScopes.every((scope) => context.scopes.includes(scope)), + ) && authorizationDetailsSubset({ requested: authorizationDetails, granted: active.contexts.map(githubInstallationAuthorizationDetail), diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index 67fb884..323a640 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -39,6 +39,7 @@ describe('GitHub external authorization', () => { target_type: 'Organization', repository_selection: 'all', }, + grantedScopes: ['metadata:read'], display: { label: 'realmroot', description: 'Organization GitHub App installation', @@ -54,6 +55,49 @@ describe('GitHub external authorization', () => { }) }) + it('rejects a scope that another installation grants but the selected installation does not', async () => { + const contexts = [ + { + installationId: 701, + accountLogin: 'saltbo', + targetType: 'User', + scopes: ['metadata:read'], + repositorySelection: 'all' as const, + repositories: [], + }, + { + installationId: 702, + accountLogin: 'realmroot', + targetType: 'Organization', + scopes: ['administration:write', 'metadata:read'], + repositorySelection: 'all' as const, + repositories: [], + }, + ] + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection: {} as never, + connections: { + externalAuthorization: vi.fn(async () => ({ + scopes: ['administration:write', 'metadata:read'], + contexts, + })), + } as unknown as D1GitHubConnections, + oauthStore: {} as D1ExternalOAuthStore, + scopes: ['administration:write', 'metadata:read'], + }) + + await expect( + external.authorization.validateGrant?.({ + subject: '70', + scopes: ['administration:write'], + authorizationDetails: [ + { type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: '701' }, + ], + }), + ).resolves.toBe(false) + }) + it('accepts read scopes implied by a write installation permission', async () => { const installation = { id: 701, From 165e1f4a53b96705092cdf5ff1e12a82445530ca Mon Sep 17 00:00:00 2001 From: saltbo Date: Fri, 14 Aug 2026 09:42:29 -0400 Subject: [PATCH 06/13] fix(github): preserve permission upgrade continuation --- docs/github-design.md | 13 ++++---- specs/github-adapter.feature | 2 +- src/providers/github/client.ts | 14 --------- .../github/external-authorization.ts | 2 +- src/providers/github/types.ts | 1 - test/providers/github-client.test.ts | 30 ------------------- .../github-external-authorization.test.ts | 11 ++----- 7 files changed, 11 insertions(+), 62 deletions(-) diff --git a/docs/github-design.md b/docs/github-design.md index 823ba74..daa6c72 100644 --- a/docs/github-design.md +++ b/docs/github-design.md @@ -44,12 +44,13 @@ The GitHub App Setup URL is `https://adapters.realmroot.dev/github/account-connection-installations`, with GitHub's **Redirect on update** option enabled. When an existing installation lacks a newly requested App permission, the Adapter keeps the original OAuth -intent and redirects the owner to that installation's dedicated -`/permissions/update` page with an opaque state. GitHub returns that state to -the Setup URL after the update. The Setup URL accepts only the installation -selected by the authorization detail, then resumes the same Realmroot -authorization transaction. The Adapter retries the permission check once and -returns an OAuth denial to Realmroot if the owner did not approve the update. +intent and redirects the owner through the App's documented +`/apps/{app}/installations/new` entry point with an opaque state. GitHub +preserves that state when the owner accepts the update and returns it to the +Setup URL. The Setup URL accepts only the installation selected by the +authorization detail, then resumes the same Realmroot authorization +transaction. The Adapter retries the permission check once and returns an +OAuth denial to Realmroot if the owner did not approve the update. Incomplete permission upgrades are not persisted as connected authority and do not surface an intermediate Adapter error page during the normal approval diff --git a/specs/github-adapter.feature b/specs/github-adapter.feature index 6018909..844fe0c 100644 --- a/specs/github-adapter.feature +++ b/specs/github-adapter.feature @@ -29,7 +29,7 @@ Feature: GitHub App adapter Given an existing GitHub App installation lacks a newly requested permission When the owner authorizes that permission through Realmroot Then the adapter preserves the original authorization transaction - And redirects through the GitHub installation update flow with an opaque state + And redirects through GitHub's state-preserving App installation entry point And accepts only the target installation And resumes the same Realmroot authorization after GitHub returns And returns a declined permission update to Realmroot as an OAuth denial diff --git a/src/providers/github/client.ts b/src/providers/github/client.ts index 6512b0f..4f776f8 100644 --- a/src/providers/github/client.ts +++ b/src/providers/github/client.ts @@ -166,20 +166,6 @@ export function createGitHubConnectionProvider( url.searchParams.set('state', state) return url.toString() }, - installationPermissionUpdateUrl(installation, state) { - const url = - installation.targetType === 'Organization' - ? new URL( - `/organizations/${encodeURIComponent(installation.accountLogin)}/settings/installations/${installation.id}/permissions/update`, - 'https://github.com', - ) - : installation.targetType === 'User' - ? new URL(`/settings/installations/${installation.id}/permissions/update`, 'https://github.com') - : null - if (!url) throw failedDependency(`GitHub installation target type ${installation.targetType} is unsupported.`) - url.searchParams.set('state', state) - return url.toString() - }, } async function listInstallationRepositories(token: string, installationId: number) { diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index db9f856..45cb14c 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -171,7 +171,7 @@ export function createGitHubExternalAuthorization(input: { expectedInstallationId: selectedInstallation.id, permissionUpdateAttempted: true, }, - url: input.connection.installationPermissionUpdateUrl(selectedInstallation, providerState), + url: await input.connection.newInstallationUrl(providerState), } } const contexts = await input.connections.upsertExternalAuthorization(user, installations) diff --git a/src/providers/github/types.ts b/src/providers/github/types.ts index 2ed177e..9169d87 100644 --- a/src/providers/github/types.ts +++ b/src/providers/github/types.ts @@ -33,5 +33,4 @@ export interface GitHubConnectionProvider { getUser(token: string): Promise listUserInstallations(token: string): Promise newInstallationUrl(state: string): Promise - installationPermissionUpdateUrl(installation: GitHubInstallation, state: string): string } diff --git a/test/providers/github-client.test.ts b/test/providers/github-client.test.ts index 5e8b515..16a8e31 100644 --- a/test/providers/github-client.test.ts +++ b/test/providers/github-client.test.ts @@ -76,36 +76,6 @@ describe('GitHub provider HTTP boundary', () => { }) describe('GitHub account connection OAuth boundary', () => { - it('targets the selected installation permission review page', () => { - const provider = createGitHubConnectionProvider({ - appId: '123', - privateKey: privateKey('pkcs8'), - clientId: 'client-id', - clientSecret: 'client-secret', - redirectUri: 'https://adapters.realmroot.dev/github/oauth/callback', - apiOrigin: 'https://api.github.test', - }) - const installation = { - id: 42, - accountLogin: 'realmroot', - targetType: 'Organization', - permissions: { metadata: 'read' as const }, - repositorySelection: 'all' as const, - repositories: [], - updatedAt: '2027-01-15T08:00:00+00:00', - } - - expect(provider.installationPermissionUpdateUrl(installation, 'provider-state')).toBe( - 'https://github.com/organizations/realmroot/settings/installations/42/permissions/update?state=provider-state', - ) - expect( - provider.installationPermissionUpdateUrl( - { ...installation, accountLogin: 'controller', targetType: 'User' }, - 'provider-state', - ), - ).toBe('https://github.com/settings/installations/42/permissions/update?state=provider-state') - }) - it('selects the adapter callback when the GitHub App has multiple callback URLs', async () => { const requests: Array<{ url: string; body: unknown }> = [] const provider = createGitHubConnectionProvider({ diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index 323a640..7f50519 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -128,9 +128,6 @@ describe('GitHub external authorization', () => { getUser: vi.fn(async () => ({ id: 70, login: 'controller', name: 'Controller' })), listUserInstallations: vi.fn(async () => [installation]), newInstallationUrl: vi.fn(async () => 'https://github.com/apps/example/installations/new'), - installationPermissionUpdateUrl: vi.fn( - () => 'https://github.com/organizations/realmroot/settings/installations/701/permissions/update', - ), }, connections: connections as unknown as D1GitHubConnections, oauthStore: {} as D1ExternalOAuthStore, @@ -176,9 +173,9 @@ describe('GitHub external authorization', () => { providerState: 'permission-update-state', stage: 'install', data: { expectedInstallationId: 701, permissionUpdateAttempted: true }, - url: 'https://github.com/organizations/realmroot/settings/installations/701/permissions/update?state=permission-update-state', + url: 'https://github.com/apps/example/installations/new?state=permission-update-state', }) - expect(connection.installationPermissionUpdateUrl).toHaveBeenCalledWith(installation, 'permission-update-state') + expect(connection.newInstallationUrl).toHaveBeenCalledWith('permission-update-state') expect(connections.upsertExternalAuthorization).not.toHaveBeenCalled() }) @@ -268,10 +265,6 @@ function githubConnection(installations: ReturnType[] newInstallationUrl: vi.fn( async (state: string) => `https://github.com/apps/example/installations/new?state=${state}`, ), - installationPermissionUpdateUrl: vi.fn( - (installation: ReturnType, state: string) => - `https://github.com/organizations/${installation.accountLogin}/settings/installations/${installation.id}/permissions/update?state=${state}`, - ), } } From 695c854bfc7fcfe9011dd8dfed8fc445ef0e2ba4 Mon Sep 17 00:00:00 2001 From: saltbo Date: Fri, 14 Aug 2026 09:44:26 -0400 Subject: [PATCH 07/13] style(github): format authorization test --- test/providers/github-external-authorization.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index 7f50519..ddd772f 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -91,9 +91,7 @@ describe('GitHub external authorization', () => { external.authorization.validateGrant?.({ subject: '70', scopes: ['administration:write'], - authorizationDetails: [ - { type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: '701' }, - ], + authorizationDetails: [{ type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: '701' }], }), ).resolves.toBe(false) }) From 78f8e5cf8c26a7ef4bc22345f03b4bbcd81e50db Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 11:04:24 -0400 Subject: [PATCH 08/13] fix(github): validate scopes against selected installation --- src/core/external-authorization-server.ts | 16 +++++++- .../github/external-authorization.ts | 30 ++++++++++++--- .../github-external-authorization.test.ts | 38 ++++++++++++++++++- 3 files changed, 76 insertions(+), 8 deletions(-) diff --git a/src/core/external-authorization-server.ts b/src/core/external-authorization-server.ts index a134a53..2ad6639 100644 --- a/src/core/external-authorization-server.ts +++ b/src/core/external-authorization-server.ts @@ -45,6 +45,11 @@ export type ExternalProviderAuthorization = { requested: Array> granted: Array> }): boolean + validateScopes?(input: { + subject: string + scopes: string[] + authorizationDetails: Array> + }): Promise validateGrant?(input: { subject: string scopes: string[] @@ -281,10 +286,17 @@ export async function createExternalAuthorizationServer(input: { } const scopes = normalizeScopes(requiredForm(form, 'scope')) const subjectScopes = normalizeScopes(String(subject.payload.scope ?? '')) - if (scopes.some((scope) => !subjectScopes.includes(scope))) { + const authorizationDetails = parseAuthorizationDetails(form.get('authorization_details')) + const scopesGranted = input.provider.validateScopes + ? await input.provider.validateScopes({ + subject: String(subject.payload.sub), + scopes, + authorizationDetails, + }) + : scopes.every((scope) => subjectScopes.includes(scope)) + if (!scopesGranted) { throw oauthError('invalid_scope', 'Requested scope exceeds the connected account.') } - const authorizationDetails = parseAuthorizationDetails(form.get('authorization_details')) const subjectDetails = authorizationDetailsValue(subject.payload.authorization_details) if ( !(input.provider.authorizationDetailsSubset ?? authorizationDetailsSubset)({ diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index 45cb14c..a9ee698 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -88,11 +88,14 @@ export function createGitHubExternalAuthorization(input: { }, }, authorizationDetailsSubset, - async validateGrant({ subject, scopes, authorizationDetails }) { + async validateScopes({ subject, scopes, authorizationDetails }) { const active = await input.connections.externalAuthorization(subject) const requestedScopes = scopes.filter( (scope) => !['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope), ) + if (authorizationDetails.length === 0) { + return requestedScopes.every((scope) => active.scopes.includes(scope)) + } const selectedContexts = authorizationDetails.map((detail) => active.contexts.find( (context) => @@ -101,11 +104,28 @@ export function createGitHubExternalAuthorization(input: { ), ) return ( - scopes.every( - (scope) => - ['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope) || - active.scopes.includes(scope), + selectedContexts.every( + (context) => context !== undefined && requestedScopes.every((scope) => context.scopes.includes(scope)), ) && + authorizationDetailsSubset({ + requested: authorizationDetails, + granted: active.contexts.map(githubInstallationAuthorizationDetail), + }) + ) + }, + async validateGrant({ subject, scopes, authorizationDetails }) { + const active = await input.connections.externalAuthorization(subject) + const requestedScopes = scopes.filter( + (scope) => !['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope), + ) + const selectedContexts = authorizationDetails.map((detail) => + active.contexts.find( + (context) => + detail.type === GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE && + detail.installation_id === String(context.installationId), + ), + ) + return ( selectedContexts.every( (context) => context !== undefined && requestedScopes.every((scope) => context.scopes.includes(scope)), ) && diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index ddd772f..5fbda61 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -88,7 +88,7 @@ describe('GitHub external authorization', () => { }) await expect( - external.authorization.validateGrant?.({ + external.authorization.validateScopes?.({ subject: '70', scopes: ['administration:write'], authorizationDetails: [{ type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: '701' }], @@ -96,6 +96,42 @@ describe('GitHub external authorization', () => { ).resolves.toBe(false) }) + it('accepts a scope granted by the selected installation after the subject token was issued', async () => { + const authorizationDetail = { + type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, + installation_id: '701', + } + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection: {} as never, + connections: { + externalAuthorization: vi.fn(async () => ({ + scopes: ['metadata:read'], + contexts: [ + { + installationId: 701, + accountLogin: 'saltbo', + targetType: 'User', + scopes: ['actions:read', 'metadata:read'], + repositorySelection: 'all' as const, + repositories: [], + }, + ], + })), + } as unknown as D1GitHubConnections, + oauthStore: {} as D1ExternalOAuthStore, + scopes: ['actions:read', 'metadata:read'], + }) + + await expect( + external.authorization.validateScopes?.({ + subject: '70', + scopes: ['actions:read'], + authorizationDetails: [authorizationDetail], + }), + ).resolves.toBe(true) + }) + it('accepts read scopes implied by a write installation permission', async () => { const installation = { id: 701, From f6657561e3738055e73db9c4b5b151f8db5aa17c Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 11:07:41 -0400 Subject: [PATCH 09/13] fix(github): surface provider token errors --- src/providers/github/client.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/providers/github/client.ts b/src/providers/github/client.ts index 4f776f8..10d99c7 100644 --- a/src/providers/github/client.ts +++ b/src/providers/github/client.ts @@ -99,7 +99,7 @@ export function createGitHubProvider(input: GitHubClientInput): GitHubProvider { signal: AbortSignal.timeout(mode === 'git' ? 10 * 60_000 : 30_000), }), ) - if (requireSuccess && !response.ok) throw providerFailure(response, request.url) + if (requireSuccess && !response.ok) throw await providerFailure(response, request.url) return response } } @@ -224,10 +224,17 @@ function forwardedHeaders(input: Headers) { return headers } -function providerFailure(response: Response, target: string) { +async function providerFailure(response: Response, target: string) { const requestId = response.headers.get('x-github-request-id') + const body = z.object({ message: z.string().min(1).max(500) }).safeParse( + await response + .clone() + .json() + .catch(() => null), + ) + const message = body.success ? `: ${body.data.message}` : '' return failedDependency( - `GitHub rejected ${new URL(target, 'https://api.github.com').pathname} with ${response.status}${requestId ? ` (${requestId})` : ''}.`, + `GitHub rejected ${new URL(target, 'https://api.github.com').pathname} with ${response.status}${requestId ? ` (${requestId})` : ''}${message}.`, ) } From 8570235f76710e3d6ebf7a8effba3255437e97ea Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 11:10:57 -0400 Subject: [PATCH 10/13] fix(github): respect all-repository installations --- src/providers/github/adapter.ts | 14 +++++++++----- test/app.test.ts | 6 ------ 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/providers/github/adapter.ts b/src/providers/github/adapter.ts index 01badef..b7d76b8 100644 --- a/src/providers/github/adapter.ts +++ b/src/providers/github/adapter.ts @@ -152,7 +152,7 @@ export function createGitHubAdapter( const createToken = await provider.installationToken({ installationId: installation.installationId, permissions: scopesToPermissions(new Set(['pull_requests:write']), available), - repositories: [repository as string], + ...repositoryRestriction(installation, repository as string), }) const response = await createGitHubPullRequestWithRest({ provider, @@ -198,7 +198,7 @@ export function createGitHubAdapter( const commentToken = await provider.installationToken({ installationId: installation.installationId, permissions: scopesToPermissions(new Set([commentScope]), available), - repositories: [repository as string], + ...repositoryRestriction(installation, repository as string), }) const response = await createGitHubCommentWithRest({ provider, @@ -241,7 +241,7 @@ export function createGitHubAdapter( const mergeToken = await provider.installationToken({ installationId: installation.installationId, permissions: scopesToPermissions(new Set(['contents:write']), available), - repositories: [repository as string], + ...repositoryRestriction(installation, repository as string), }) const response = await mergeGitHubPullRequestWithRest({ provider, @@ -297,7 +297,7 @@ export function createGitHubAdapter( const token = await provider.installationToken({ installationId: installation.installationId, permissions, - repositories: [repository as string], + ...repositoryRestriction(installation, repository as string), }) const response = await provider.request( new Request(target.url, { @@ -341,7 +341,7 @@ export function createGitHubAdapter( const token = await provider.installationToken({ installationId: installation.installationId, permissions, - ...(repository ? { repositories: [repository] } : {}), + ...(repository ? repositoryRestriction(installation, repository) : {}), }) const upstreamRequest = new Request(upstream, { method: c.req.method, @@ -489,6 +489,10 @@ function repositoryTarget(path: string, installation: GitHubAuthorizationContext return repository } +function repositoryRestriction(installation: GitHubAuthorizationContext, repository: string) { + return installation.repositorySelection === 'selected' ? { repositories: [repository] } : {} +} + function installationId(value: string) { const parsed = Number(value) if (!Number.isSafeInteger(parsed) || parsed <= 0) throw badRequest('The GitHub installation ID is invalid.') diff --git a/test/app.test.ts b/test/app.test.ts index 857a616..f7bd701 100644 --- a/test/app.test.ts +++ b/test/app.test.ts @@ -196,7 +196,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenNthCalledWith(2, { installationId: 42, permissions: { pull_requests: 'write' }, - repositories: ['example'], }) }) @@ -255,7 +254,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenNthCalledWith(2, { installationId: 42, permissions: { issues: 'write' }, - repositories: ['example'], }) }) @@ -300,7 +298,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenNthCalledWith(2, { installationId: 42, permissions: { contents: 'write' }, - repositories: ['example'], }) }) @@ -336,7 +333,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenLastCalledWith({ installationId: 42, permissions: { contents: 'read' }, - repositories: ['example'], }) const pushResponse = await app.request('/github/git/realmroot/example.git/git-receive-pack', { method: 'POST', @@ -346,7 +342,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenLastCalledWith({ installationId: 42, permissions: { contents: 'write', workflows: 'write' }, - repositories: ['example'], }) }) @@ -435,7 +430,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenCalledWith({ installationId: 42, permissions: { issues: 'write' }, - repositories: ['example'], }) }) From e14ef5172bddce98c3ba7cb095382d440cfe0d91 Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 11:58:35 -0400 Subject: [PATCH 11/13] fix(github): resume installation permission upgrades --- docs/github-design.md | 24 ++-- specs/github-adapter.feature | 8 +- src/core/external-authorization-server.ts | 51 +++++-- src/providers/github/client.ts | 5 + .../github/external-authorization.ts | 125 +++++++++++++++++- src/providers/github/types.ts | 2 + .../external-authorization-server.test.ts | 41 +++++- test/integration/worker.test.ts | 6 + test/providers/github-client.test.ts | 5 + .../github-external-authorization.test.ts | 121 +++++++++++------ 10 files changed, 318 insertions(+), 70 deletions(-) diff --git a/docs/github-design.md b/docs/github-design.md index daa6c72..ded899d 100644 --- a/docs/github-design.md +++ b/docs/github-design.md @@ -40,17 +40,19 @@ the display label as authorization input. ## Installation permission upgrades -The GitHub App Setup URL is -`https://adapters.realmroot.dev/github/account-connection-installations`, with -GitHub's **Redirect on update** option enabled. When an existing installation -lacks a newly requested App permission, the Adapter keeps the original OAuth -intent and redirects the owner through the App's documented -`/apps/{app}/installations/new` entry point with an opaque state. GitHub -preserves that state when the owner accepts the update and returns it to the -Setup URL. The Setup URL accepts only the installation selected by the -authorization detail, then resumes the same Realmroot authorization -transaction. The Adapter retries the permission check once and returns an -OAuth denial to Realmroot if the owner did not approve the update. +GitHub does not return permission-update acceptance to the App's Setup URL. +The Setup URL remains responsible for new installations and repository-access +changes, but it is not the completion signal for a newly added App permission. + +When an existing installation lacks a newly requested permission, the Adapter +keeps the original OAuth intent and opens the exact installation review URL at +`/settings/installations/{installation_id}/permissions/update`. An +Adapter-owned waiting page polls the preserved transaction while GitHub opens +the review in a separate window. The signed `installation` webhook with action +`new_permissions_accepted` updates Adapter-owned authority. Only after that +target installation covers every requested scope does the Adapter complete the +same OAuth transaction, close the GitHub window, and return the controller to +Realmroot automatically. Incomplete permission upgrades are not persisted as connected authority and do not surface an intermediate Adapter error page during the normal approval diff --git a/specs/github-adapter.feature b/specs/github-adapter.feature index 844fe0c..4f96821 100644 --- a/specs/github-adapter.feature +++ b/specs/github-adapter.feature @@ -29,10 +29,10 @@ Feature: GitHub App adapter Given an existing GitHub App installation lacks a newly requested permission When the owner authorizes that permission through Realmroot Then the adapter preserves the original authorization transaction - And redirects through GitHub's state-preserving App installation entry point - And accepts only the target installation - And resumes the same Realmroot authorization after GitHub returns - And returns a declined permission update to Realmroot as an OAuth denial + And opens the exact target installation's permission review + And waits for GitHub's signed lifecycle event instead of expecting an unsupported browser callback + And resumes the same Realmroot authorization when that target installation accepts the permission + And returns automatically to the pending Realmroot approval But it does not show an adapter insufficient-scope page @journey:github-context-catalog @entrypoint:http diff --git a/src/core/external-authorization-server.ts b/src/core/external-authorization-server.ts index 2ad6639..e912689 100644 --- a/src/core/external-authorization-server.ts +++ b/src/core/external-authorization-server.ts @@ -72,6 +72,13 @@ export type ExternalProviderAuthorization = { | { type: 'complete'; grant: Omit } | { type: 'error'; error: 'access_denied' | 'server_error'; description: string } > + resume?(input: { + intent: ExternalOAuthIntent + }): Promise< + | { type: 'pending' } + | { type: 'complete'; grant: Omit } + | { type: 'error'; error: 'access_denied' | 'server_error'; description: string } + > } export type ExternalAuthorizationServer = { @@ -209,17 +216,28 @@ export async function createExternalAuthorizationServer(input: { await input.store.cancelIntent(intent) return c.redirect(authorizationErrorRedirect(intent, result.error, result.description)) } - const code = opaque('code') - await input.store.completeIntent( - intent, - { ...result.grant, providerId: input.provider.id, clientId: intent.clientId }, - code, - ) - const callback = new URL(intent.redirectUri) - callback.searchParams.set('code', code) - callback.searchParams.set('state', intent.realmrootState) - return c.redirect(callback.toString()) + return c.redirect(await completeAuthorization(input, intent, result.grant)) }) + const resume = input.provider.resume + if (resume) { + app.get(`/oauth/${input.provider.id}/continue`, async (c) => { + c.header('Cache-Control', 'no-store') + const intent = await input.store.intentByProviderState(await sha256(required(c.req.query('state'), 'state'))) + const result = await resume({ intent }) + if (result.type === 'pending') return c.json({ status: 'pending' }, 202) + if (result.type === 'error') { + await input.store.cancelIntent(intent) + return c.json({ + status: 'failed', + redirectUrl: authorizationErrorRedirect(intent, result.error, result.description), + }) + } + return c.json({ + status: 'complete', + redirectUrl: await completeAuthorization(input, intent, result.grant), + }) + }) + } app.post(`/oauth/${input.provider.id}/token`, async (c) => { const client = await authenticateClient(input.store, input.provider.id, c.req.raw) const form = await c.req.formData() @@ -503,6 +521,19 @@ export async function createExternalAuthorizationServer(input: { } } +async function completeAuthorization( + input: Pick[0], 'provider' | 'store'>, + intent: ExternalOAuthIntent, + grant: Omit, +) { + const code = opaque('code') + await input.store.completeIntent(intent, { ...grant, providerId: input.provider.id, clientId: intent.clientId }, code) + const callback = new URL(intent.redirectUri) + callback.searchParams.set('code', code) + callback.searchParams.set('state', intent.realmrootState) + return callback.toString() +} + function authorizationErrorRedirect( intent: ExternalOAuthIntent, error: 'access_denied' | 'server_error', diff --git a/src/providers/github/client.ts b/src/providers/github/client.ts index 10d99c7..bd785ee 100644 --- a/src/providers/github/client.ts +++ b/src/providers/github/client.ts @@ -14,6 +14,7 @@ const userInstallationsSchema = z.object({ installations: z.array( z.object({ id: z.number().int().positive(), + html_url: z.url().refine((value) => new URL(value).origin === 'https://github.com'), account: z.object({ login: z.string().min(1) }), target_type: z.string().min(1), permissions: permissionsSchema, @@ -147,6 +148,7 @@ export function createGitHubConnectionProvider( return Promise.all( parsed.installations.map(async (installation) => ({ id: installation.id, + htmlUrl: installation.html_url, accountLogin: installation.account.login, targetType: installation.target_type, permissions: installation.permissions, @@ -166,6 +168,9 @@ export function createGitHubConnectionProvider( url.searchParams.set('state', state) return url.toString() }, + permissionUpdateUrl(installation) { + return `${installation.htmlUrl}/permissions/update` + }, } async function listInstallationRepositories(token: string, installationId: number) { diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index a9ee698..d21c7ff 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -175,7 +175,7 @@ export function createGitHubExternalAuthorization(input: { ) if (requestedProviderScopes.some((scope) => !providerScopes.has(scope))) { const selectedInstallation = selected.length === 1 ? selected[0] : undefined - if (intent.providerData.permissionUpdateAttempted === true || !selectedInstallation) { + if (!selectedInstallation) { return { type: 'error', error: 'access_denied', @@ -186,12 +186,14 @@ export function createGitHubExternalAuthorization(input: { return { type: 'continue', providerState, - stage: 'install', + stage: 'permission-update', data: { expectedInstallationId: selectedInstallation.id, - permissionUpdateAttempted: true, + permissionUpdateUrl: input.connection.permissionUpdateUrl(selectedInstallation), + subject: String(user.id), + displayName: user.name ?? user.login, }, - url: await input.connection.newInstallationUrl(providerState), + url: `${input.origin}/github/permission-update?state=${encodeURIComponent(providerState)}`, } } const contexts = await input.connections.upsertExternalAuthorization(user, installations) @@ -205,6 +207,34 @@ export function createGitHubExternalAuthorization(input: { }, } }, + async resume({ intent }) { + if (intent.providerStage !== 'permission-update') { + return { type: 'error', error: 'server_error', description: 'GitHub permission update state is invalid.' } + } + const subject = stringValue(intent.providerData.subject) + const displayName = stringValue(intent.providerData.displayName) + const expectedInstallationId = numberValue(intent.providerData.expectedInstallationId) + if (!subject || !displayName || !expectedInstallationId) { + return { type: 'error', error: 'server_error', description: 'GitHub permission update state is incomplete.' } + } + const active = await input.connections.externalAuthorization(subject) + const selected = active.contexts.find((context) => context.installationId === expectedInstallationId) + const requestedProviderScopes = intent.scopes.filter( + (scope) => !['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope), + ) + if (!selected || requestedProviderScopes.some((scope) => !selected.scopes.includes(scope))) { + return { type: 'pending' } + } + return { + type: 'complete', + grant: { + subject, + displayName, + scopes: intent.scopes, + authorizationDetails: active.contexts.map(githubInstallationAuthorizationDetail), + }, + } + }, } return { @@ -212,6 +242,30 @@ export function createGitHubExternalAuthorization(input: { installationCallback: { id: 'github-installation-callback', register(app: Hono) { + app.get('/github/permission-update', async (c) => { + const state = required(c.req.query('state'), 'state') + const intent = await input.oauthStore.intentByProviderState(await sha256(state)) + if (intent.providerId !== 'github' || intent.providerStage !== 'permission-update') { + throw badRequest('GitHub permission update state is invalid.') + } + const installationId = numberValue(intent.providerData.expectedInstallationId) + if (!installationId) throw badRequest('GitHub permission update installation is invalid.') + const updateUrl = stringValue(intent.providerData.permissionUpdateUrl) + if (!updateUrl) throw badRequest('GitHub permission update URL is invalid.') + const nonce = randomToken() + c.header('Cache-Control', 'no-store') + c.header( + 'Content-Security-Policy', + `default-src 'none'; connect-src 'self'; script-src 'nonce-${nonce}'; style-src 'nonce-${nonce}'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'`, + ) + return c.html( + permissionUpdatePage({ + continueUrl: `${input.origin}/oauth/github/continue?state=${encodeURIComponent(state)}`, + nonce, + updateUrl, + }), + ) + }) app.get('/github/account-connection-installations', async (c) => { const state = required(c.req.query('state'), 'state') const installationId = Number(required(c.req.query('installation_id'), 'installation_id')) @@ -245,6 +299,69 @@ function numberValue(value: unknown) { return typeof value === 'number' && Number.isSafeInteger(value) && value > 0 ? value : null } +function stringValue(value: unknown) { + return typeof value === 'string' && value.length > 0 ? value : null +} + +function permissionUpdatePage(input: { continueUrl: string; nonce: string; updateUrl: string }) { + const continueUrl = JSON.stringify(input.continueUrl) + const updateUrl = JSON.stringify(input.updateUrl) + return ` + + + + + Update GitHub permissions + + + +
+

Update GitHub permissions

+

Accept the requested permission in GitHub. This page will return to Realmroot automatically.

+ +
+ + +` +} + function requestedInstallationId(authorizationDetails: Array>) { const ids = authorizationDetails.flatMap((detail) => { if (detail.type !== GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE) return [] diff --git a/src/providers/github/types.ts b/src/providers/github/types.ts index 9169d87..1746880 100644 --- a/src/providers/github/types.ts +++ b/src/providers/github/types.ts @@ -17,6 +17,7 @@ export interface GitHubProvider { export type GitHubUser = Readonly<{ id: number; login: string; name: string | null }> export type GitHubInstallation = Readonly<{ id: number + htmlUrl: string accountLogin: string targetType: string permissions: GitHubPermissions @@ -33,4 +34,5 @@ export interface GitHubConnectionProvider { getUser(token: string): Promise listUserInstallations(token: string): Promise newInstallationUrl(state: string): Promise + permissionUpdateUrl(installation: GitHubInstallation): string } diff --git a/test/core/external-authorization-server.test.ts b/test/core/external-authorization-server.test.ts index a374227..588d302 100644 --- a/test/core/external-authorization-server.test.ts +++ b/test/core/external-authorization-server.test.ts @@ -171,6 +171,38 @@ describe('external authorization server', () => { ) }) + it('resumes a preserved provider authorization after an asynchronous provider decision', async () => { + const intent = { ...exampleIntent(), providerStage: 'permission-update' } + const { app, provider, store } = await testServer({ intent, resume: true }) + if (!provider.resume) throw new Error('Expected resumable provider authorization.') + vi.mocked(provider.resume) + .mockResolvedValueOnce({ type: 'pending' }) + .mockResolvedValueOnce({ + type: 'complete', + grant: { + subject: 'provider-user-1', + displayName: 'Provider User', + scopes: intent.scopes, + authorizationDetails: intent.authorizationDetails, + }, + }) + + const pending = await app.request('/oauth/example/continue?state=provider-state') + expect(pending.status).toBe(202) + expect(pending.headers.get('cache-control')).toBe('no-store') + await expect(pending.json()).resolves.toEqual({ status: 'pending' }) + + const completed = await app.request('/oauth/example/continue?state=provider-state') + expect(completed.status).toBe(200) + const body = (await completed.json()) as { status: string; redirectUrl: string } + expect(body.status).toBe('complete') + const callback = new URL(body.redirectUrl) + expect(callback.origin + callback.pathname).toBe('https://id.realmroot.dev/callback') + expect(callback.searchParams.get('state')).toBe('realmroot-state') + expect(callback.searchParams.get('code')).toMatch(/^code_/) + expect(store.completeIntent).toHaveBeenCalledWith(intent, expect.any(Object), callback.searchParams.get('code')) + }) + it('returns provider denial to Realmroot without requiring an authorization code', async () => { const intent = exampleIntent() const { app, provider, store } = await testServer({ intent }) @@ -223,7 +255,9 @@ describe('external authorization server', () => { }) }) -async function testServer(options: { intent?: ExternalOAuthIntent; providerCallbackPath?: string } = {}) { +async function testServer( + options: { intent?: ExternalOAuthIntent; providerCallbackPath?: string; resume?: boolean } = {}, +) { const store = { registerClient: vi.fn(async () => undefined), client: vi.fn(async (_providerId: string, clientId: string) => ({ @@ -278,6 +312,11 @@ async function testServer(options: { intent?: ExternalOAuthIntent; providerCallb authorizationDetails: intent.authorizationDetails, }, })), + ...(options.resume + ? { + resume: vi.fn(async () => ({ type: 'pending' as const })), + } + : {}), } const { privateKey } = await generateKeyPair('ES256', { extractable: true }) const signingPrivateJwk = await exportJWK(privateKey) diff --git a/test/integration/worker.test.ts b/test/integration/worker.test.ts index b2b12e5..a8268d3 100644 --- a/test/integration/worker.test.ts +++ b/test/integration/worker.test.ts @@ -53,6 +53,7 @@ describe('Cloudflare Worker runtime', () => { [ { id: 101, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/101', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read', issues: 'write' }, @@ -62,6 +63,7 @@ describe('Cloudflare Worker runtime', () => { }, { id: 102, + htmlUrl: 'https://github.com/settings/installations/102', accountLogin: 'controller', targetType: 'User', permissions: { metadata: 'read', issues: 'write' }, @@ -114,6 +116,7 @@ describe('Cloudflare Worker runtime', () => { [ { id: 103, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/103', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read', issues: 'write' }, @@ -177,6 +180,7 @@ describe('Cloudflare Worker runtime', () => { [ { id: 701, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/701', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read', pull_requests: 'read' }, @@ -223,6 +227,7 @@ describe('Cloudflare Worker runtime', () => { [ { id: 201, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/201', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read' }, @@ -261,6 +266,7 @@ describe('Cloudflare Worker runtime', () => { [ { id: 202, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/202', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read' }, diff --git a/test/providers/github-client.test.ts b/test/providers/github-client.test.ts index 16a8e31..e0ef60c 100644 --- a/test/providers/github-client.test.ts +++ b/test/providers/github-client.test.ts @@ -103,6 +103,9 @@ describe('GitHub account connection OAuth boundary', () => { redirect_uri: 'https://adapters.realmroot.dev/github/oauth/callback', }, }) + expect(provider.permissionUpdateUrl({ htmlUrl: 'https://github.com/settings/installations/42' } as never)).toBe( + 'https://github.com/settings/installations/42/permissions/update', + ) }) it('loads selected repository membership with the installation context', async () => { @@ -122,6 +125,7 @@ describe('GitHub account connection OAuth boundary', () => { installations: [ { id: 42, + html_url: 'https://github.com/organizations/realmroot/settings/installations/42', account: { login: 'realmroot' }, target_type: 'Organization', permissions: { metadata: 'read' }, @@ -154,6 +158,7 @@ describe('GitHub account connection OAuth boundary', () => { expect(installations).toHaveLength(1) expect(installations[0]).toMatchObject({ id: 42, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/42', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read' }, diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index 5fbda61..c79c625 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -4,6 +4,7 @@ import type { D1ExternalOAuthStore, ExternalOAuthIntent } from '../../src/core/e import { GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE } from '../../src/providers/github/authorization-details.js' import type { D1GitHubConnections } from '../../src/providers/github/connections.js' import { createGitHubExternalAuthorization } from '../../src/providers/github/external-authorization.js' +import type { GitHubInstallation } from '../../src/providers/github/types.js' describe('GitHub external authorization', () => { it('[spec: github-adapter/github-context-catalog] exposes stable installation details with human labels', async () => { @@ -135,6 +136,7 @@ describe('GitHub external authorization', () => { it('accepts read scopes implied by a write installation permission', async () => { const installation = { id: 701, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/701', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read', pull_requests: 'write' } as const, @@ -162,6 +164,7 @@ describe('GitHub external authorization', () => { getUser: vi.fn(async () => ({ id: 70, login: 'controller', name: 'Controller' })), listUserInstallations: vi.fn(async () => [installation]), newInstallationUrl: vi.fn(async () => 'https://github.com/apps/example/installations/new'), + permissionUpdateUrl: vi.fn((installation: GitHubInstallation) => `${installation.htmlUrl}/permissions/update`), }, connections: connections as unknown as D1GitHubConnections, oauthStore: {} as D1ExternalOAuthStore, @@ -205,51 +208,76 @@ describe('GitHub external authorization', () => { ).resolves.toEqual({ type: 'continue', providerState: 'permission-update-state', - stage: 'install', - data: { expectedInstallationId: 701, permissionUpdateAttempted: true }, - url: 'https://github.com/apps/example/installations/new?state=permission-update-state', + stage: 'permission-update', + data: { + expectedInstallationId: 701, + permissionUpdateUrl: 'https://github.com/organizations/realmroot/settings/installations/701/permissions/update', + subject: '70', + displayName: 'Controller', + }, + url: 'https://adapter.example/github/permission-update?state=permission-update-state', }) - expect(connection.newInstallationUrl).toHaveBeenCalledWith('permission-update-state') + expect(connection.newInstallationUrl).not.toHaveBeenCalled() expect(connections.upsertExternalAuthorization).not.toHaveBeenCalled() }) - it('[spec: github-adapter/github-installation-permission-upgrade] stops after one rejected permission update', async () => { - const connections = { upsertExternalAuthorization: vi.fn() } + it('[spec: github-adapter/github-installation-permission-upgrade] resumes only after the target installation accepts the permission', async () => { + const before = githubInstallation({ administration: 'read' }) + const after = githubInstallation({ administration: 'write' }) + const externalAuthorization = vi + .fn() + .mockResolvedValueOnce({ scopes: ['administration:read'], contexts: [connectionContext(before)] }) + .mockResolvedValueOnce({ + scopes: ['administration:read', 'administration:write'], + contexts: [connectionContext(after)], + }) const external = createGitHubExternalAuthorization({ origin: 'https://adapter.example', - connection: githubConnection([githubInstallation({ administration: 'read' })]), - connections: connections as unknown as D1GitHubConnections, + connection: githubConnection([before]), + connections: { externalAuthorization } as unknown as D1GitHubConnections, oauthStore: {} as D1ExternalOAuthStore, scopes: ['administration:read', 'administration:write'], }) + const permissionUpdateIntent = { + ...intent(), + scopes: ['administration:write', 'openid'], + authorizationDetails: [{ type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: '701' }], + providerStage: 'permission-update', + providerData: { expectedInstallationId: 701, subject: '70', displayName: 'Controller' }, + } - await expect( - external.authorization.complete({ - callbackUrl: 'https://adapter.example/github/oauth/callback?code=provider-code', - intent: { - ...intent(), - scopes: ['administration:write', 'openid'], - providerStage: 'oauth-selected', - providerData: { expectedInstallationId: 701, permissionUpdateAttempted: true }, - }, - nextProviderState: () => 'must-not-loop', - }), - ).resolves.toEqual({ - type: 'error', - error: 'access_denied', - description: 'The selected GitHub installation permission update was not approved.', + await expect(external.authorization.resume?.({ intent: permissionUpdateIntent })).resolves.toEqual({ + type: 'pending', + }) + await expect(external.authorization.resume?.({ intent: permissionUpdateIntent })).resolves.toEqual({ + type: 'complete', + grant: { + subject: '70', + displayName: 'Controller', + scopes: ['administration:write', 'openid'], + authorizationDetails: [ + expect.objectContaining({ + type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, + installation_id: '701', + }), + ], + }, }) - expect(connections.upsertExternalAuthorization).not.toHaveBeenCalled() }) - it('preserves the permission update intent when GitHub returns through the Setup URL', async () => { + it('[spec: github-adapter/github-installation-permission-upgrade] opens the exact installation permission review and polls the preserved intent', async () => { const oauthStore = { intentByProviderState: vi.fn(async () => ({ ...intent(), - providerStage: 'install', - providerData: { expectedInstallationId: 701, permissionUpdateAttempted: true }, + providerStage: 'permission-update', + providerData: { + expectedInstallationId: 701, + permissionUpdateUrl: + 'https://github.com/organizations/realmroot/settings/installations/701/permissions/update', + subject: '70', + displayName: 'Controller', + }, })), - advanceIntent: vi.fn(async () => undefined), } const connection = githubConnection([]) const external = createGitHubExternalAuthorization({ @@ -262,25 +290,20 @@ describe('GitHub external authorization', () => { const app = new Hono() external.installationCallback.register(app as never) - const response = await app.request( - 'https://adapter.example/github/account-connection-installations?state=permission-update-state&installation_id=701', - ) + const response = await app.request('https://adapter.example/github/permission-update?state=permission-update-state') - expect(response.status).toBe(302) - expect(response.headers.get('location')).toMatch(/^https:\/\/github\.com\/login\/oauth\/authorize\?state=/) - expect(oauthStore.advanceIntent).toHaveBeenCalledWith({ - id: 'intent-1', - expectedStage: 'install', - providerStateHash: expect.any(String), - providerStage: 'oauth-selected', - providerData: { expectedInstallationId: 701, permissionUpdateAttempted: true }, - }) + expect(response.status).toBe(200) + expect(response.headers.get('cache-control')).toBe('no-store') + await expect(response.text()).resolves.toContain( + 'https://github.com/organizations/realmroot/settings/installations/701/permissions/update', + ) }) }) function githubInstallation(permissions: Record) { return { id: 701, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/701', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read' as const, ...permissions }, @@ -299,6 +322,24 @@ function githubConnection(installations: ReturnType[] newInstallationUrl: vi.fn( async (state: string) => `https://github.com/apps/example/installations/new?state=${state}`, ), + permissionUpdateUrl: vi.fn( + (installation: ReturnType) => `${installation.htmlUrl}/permissions/update`, + ), + } +} + +function connectionContext(installation: ReturnType) { + const permissions = installation.permissions as Record + return { + installationId: installation.id, + accountLogin: installation.accountLogin, + targetType: installation.targetType, + scopes: + permissions.administration === 'write' + ? ['administration:read', 'administration:write', 'metadata:read'] + : ['administration:read', 'metadata:read'], + repositorySelection: installation.repositorySelection, + repositories: [], } } From c8ac07c19c00e5cc6505267a04145a3021f1d4d3 Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 13:24:05 -0400 Subject: [PATCH 12/13] fix(github): constrain connection details to context --- .../github/external-authorization.ts | 7 ++- .../github-external-authorization.test.ts | 53 ++++++++++++++++++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index d21c7ff..c0c37f7 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -197,13 +197,16 @@ export function createGitHubExternalAuthorization(input: { } } const contexts = await input.connections.upsertExternalAuthorization(user, installations) + const grantedContexts = expectedInstallationId + ? contexts.filter((context) => context.installationId === expectedInstallationId) + : contexts return { type: 'complete', grant: { subject: String(user.id), displayName: user.name ?? user.login, scopes: intent.scopes, - authorizationDetails: contexts.map(githubInstallationAuthorizationDetail), + authorizationDetails: grantedContexts.map(githubInstallationAuthorizationDetail), }, } }, @@ -231,7 +234,7 @@ export function createGitHubExternalAuthorization(input: { subject, displayName, scopes: intent.scopes, - authorizationDetails: active.contexts.map(githubInstallationAuthorizationDetail), + authorizationDetails: [githubInstallationAuthorizationDetail(selected)], }, } }, diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index c79c625..0546df3 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -183,6 +183,50 @@ describe('GitHub external authorization', () => { }) }) + it('grants only the concrete installation requested by the provider connection', async () => { + const selected = githubInstallation({ administration: 'write' }) + const other = { + ...selected, + id: 702, + htmlUrl: 'https://github.com/settings/installations/702', + accountLogin: 'controller', + targetType: 'User', + } + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection: githubConnection([selected, other]), + connections: { + upsertExternalAuthorization: vi.fn(async () => [connectionContext(selected), connectionContext(other)]), + } as unknown as D1GitHubConnections, + oauthStore: {} as D1ExternalOAuthStore, + scopes: ['administration:read'], + }) + + await expect( + external.authorization.complete({ + callbackUrl: 'https://adapter.example/github/oauth/callback?code=provider-code', + intent: { + ...intent(), + scopes: ['administration:read', 'openid'], + authorizationDetails: [ + { type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: String(selected.id) }, + ], + }, + nextProviderState: () => 'next-state', + }), + ).resolves.toMatchObject({ + type: 'complete', + grant: { + authorizationDetails: [ + expect.objectContaining({ + type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, + installation_id: String(selected.id), + }), + ], + }, + }) + }) + it('[spec: github-adapter/github-installation-permission-upgrade] continues through a target installation permission update', async () => { const installation = githubInstallation({ administration: 'read' }) const connections = { upsertExternalAuthorization: vi.fn() } @@ -224,12 +268,19 @@ describe('GitHub external authorization', () => { it('[spec: github-adapter/github-installation-permission-upgrade] resumes only after the target installation accepts the permission', async () => { const before = githubInstallation({ administration: 'read' }) const after = githubInstallation({ administration: 'write' }) + const other = { + ...after, + id: 702, + htmlUrl: 'https://github.com/settings/installations/702', + accountLogin: 'controller', + targetType: 'User', + } const externalAuthorization = vi .fn() .mockResolvedValueOnce({ scopes: ['administration:read'], contexts: [connectionContext(before)] }) .mockResolvedValueOnce({ scopes: ['administration:read', 'administration:write'], - contexts: [connectionContext(after)], + contexts: [connectionContext(after), connectionContext(other)], }) const external = createGitHubExternalAuthorization({ origin: 'https://adapter.example', From 823190a64ecc207720b41956a84a4e5b025c7000 Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 13:48:05 -0400 Subject: [PATCH 13/13] fix(github): return complete authorization snapshot --- src/providers/github/external-authorization.ts | 7 ++----- test/providers/github-external-authorization.test.ts | 10 +++++++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index c0c37f7..d21c7ff 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -197,16 +197,13 @@ export function createGitHubExternalAuthorization(input: { } } const contexts = await input.connections.upsertExternalAuthorization(user, installations) - const grantedContexts = expectedInstallationId - ? contexts.filter((context) => context.installationId === expectedInstallationId) - : contexts return { type: 'complete', grant: { subject: String(user.id), displayName: user.name ?? user.login, scopes: intent.scopes, - authorizationDetails: grantedContexts.map(githubInstallationAuthorizationDetail), + authorizationDetails: contexts.map(githubInstallationAuthorizationDetail), }, } }, @@ -234,7 +231,7 @@ export function createGitHubExternalAuthorization(input: { subject, displayName, scopes: intent.scopes, - authorizationDetails: [githubInstallationAuthorizationDetail(selected)], + authorizationDetails: active.contexts.map(githubInstallationAuthorizationDetail), }, } }, diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index 0546df3..1ae68da 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -183,7 +183,7 @@ describe('GitHub external authorization', () => { }) }) - it('grants only the concrete installation requested by the provider connection', async () => { + it('returns the complete installation snapshot after a concrete connection request', async () => { const selected = githubInstallation({ administration: 'write' }) const other = { ...selected, @@ -222,6 +222,10 @@ describe('GitHub external authorization', () => { type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: String(selected.id), }), + expect.objectContaining({ + type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, + installation_id: String(other.id), + }), ], }, }) @@ -311,6 +315,10 @@ describe('GitHub external authorization', () => { type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: '701', }), + expect.objectContaining({ + type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, + installation_id: '702', + }), ], }, })