From 894e076114d86500e5200ee1c4c98bb2245d1a38 Mon Sep 17 00:00:00 2001 From: saltbo Date: Fri, 14 Aug 2026 16:48:19 -0400 Subject: [PATCH 1/5] fix(github): support cross-fork pull request creation --- .dev.vars.example | 1 + README.md | 15 +- docs/architecture.md | 9 ++ migrations/0010_github_user_credentials.sql | 23 +++ providers/github/README.md | 20 +-- specs/github-adapter.feature | 10 ++ src/providers/github/adapter.ts | 91 +++++++++--- src/providers/github/client.ts | 63 ++++++--- src/providers/github/config.ts | 5 + src/providers/github/credentials.ts | 102 ++++++++++++++ .../github/external-authorization.ts | 11 +- src/providers/github/graphql.ts | 2 +- src/providers/github/types.ts | 16 ++- src/worker.ts | 26 ++-- test/app.test.ts | 133 +++++++++++++++++- .../github-user-credentials.test.ts | 49 +++++++ .../provider-connection-migration.test.ts | 39 +++++ test/providers/github-client.test.ts | 38 ++++- .../github-external-authorization.test.ts | 36 ++++- worker-configuration.d.ts | 5 +- wrangler.jsonc | 1 + 21 files changed, 621 insertions(+), 74 deletions(-) create mode 100644 migrations/0010_github_user_credentials.sql create mode 100644 src/providers/github/credentials.ts create mode 100644 test/integration/github-user-credentials.test.ts diff --git a/.dev.vars.example b/.dev.vars.example index b099c51..4b4b833 100644 --- a/.dev.vars.example +++ b/.dev.vars.example @@ -8,6 +8,7 @@ GITHUB_APP_ID=replace-with-github-app-id GITHUB_PRIVATE_KEY=replace-with-pkcs1-or-pkcs8-private-key GITHUB_CLIENT_ID=replace-with-github-app-client-id GITHUB_CLIENT_SECRET=replace-with-github-app-client-secret +GITHUB_CREDENTIAL_ENCRYPTION_KEY=replace-with-base64-encoded-32-byte-key GITHUB_WEBHOOK_SECRET=replace-with-github-app-webhook-secret CLOUDFLARE_API_ORIGIN=https://api.cloudflare.com/client/v4 CLOUDFLARE_AUTHORIZATION_ORIGIN=https://dash.cloudflare.com diff --git a/README.md b/README.md index f5811dd..bbd035b 100644 --- a/README.md +++ b/README.md @@ -210,9 +210,10 @@ Resource Servers and never appear in the audience URL: } ``` -Set `GITHUB_APP_ID`, `GITHUB_PRIVATE_KEY`, `GITHUB_CLIENT_ID`, and -`GITHUB_CLIENT_SECRET` in the ignored `.dev.vars` file. Both GitHub-downloaded -PKCS#1 keys and unencrypted PKCS#8 PEM keys are accepted. +Set `GITHUB_APP_ID`, `GITHUB_PRIVATE_KEY`, `GITHUB_CLIENT_ID`, +`GITHUB_CLIENT_SECRET`, and a base64-encoded 32-byte +`GITHUB_CREDENTIAL_ENCRYPTION_KEY` in the ignored `.dev.vars` file. Both +GitHub-downloaded PKCS#1 keys and unencrypted PKCS#8 PEM keys are accepted. Configure the GitHub App callbacks as: @@ -251,6 +252,7 @@ pnpm exec wrangler secret put GITHUB_APP_ID pnpm exec wrangler secret put GITHUB_PRIVATE_KEY < github-app.private-key.pem pnpm exec wrangler secret put GITHUB_CLIENT_ID pnpm exec wrangler secret put GITHUB_CLIENT_SECRET +pnpm exec wrangler secret put GITHUB_CREDENTIAL_ENCRYPTION_KEY pnpm exec wrangler secret put GITHUB_WEBHOOK_SECRET ``` @@ -276,6 +278,13 @@ discovery publishes the subset GitHub documents for installation access tokens, preserving alternative permission sets as OR and each set's required permissions as AND. For every request, the adapter resolves the original method and path and mints only one least-privileged permission set satisfied by the Realmroot token. +Installation credentials remain the default for Git and API operations. A +pull-request creation uses the connected user's credential to resolve GitHub's +opaque repository ID. The actual write still uses the installation credential +for an installed target. A cross-fork write is the narrow exception: after +verifying that its head belongs to the selected installation account and +repository boundary, the adapter uses the encrypted delegated credential to +create the pull request against the external upstream. GitHub requires both `contents:write` and `workflows:write` when the Contents API writes under `.github/workflows`; the adapter enforces that condition from the diff --git a/docs/architecture.md b/docs/architecture.md index 5ff9d2d..c6c6705 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -98,6 +98,15 @@ connection identity, and the resource authorization record. Provider credentials never cross the Agent boundary. The Worker runtime uses Web Crypto and Fetch APIs without a Node process or filesystem. +Provider credential selection is operation-specific and least-privileged. +GitHub uses installation credentials for repository reads, ordinary writes, +Git transport, comments, and merges. Pull-request creation uses an encrypted +delegated-user credential to resolve GitHub's opaque target repository ID. The +write keeps installation authority for an installed target, and uses delegated +user authority only when GitHub requires it for an approved installed fork to +an external upstream; the Adapter verifies the head installation boundary +before that credential is selected. + ## Identity model Every operation records two identities: diff --git a/migrations/0010_github_user_credentials.sql b/migrations/0010_github_user_credentials.sql new file mode 100644 index 0000000..46c1dc3 --- /dev/null +++ b/migrations/0010_github_user_credentials.sql @@ -0,0 +1,23 @@ +CREATE TABLE github_user_credential ( + subject TEXT PRIMARY KEY NOT NULL, + access_token_ciphertext TEXT NOT NULL, + refresh_token_ciphertext TEXT, + access_token_expires_at INTEGER, + refresh_token_expires_at INTEGER, + credential_version INTEGER NOT NULL DEFAULT 1, + updated_at INTEGER NOT NULL +); + +UPDATE external_oauth_refresh +SET revoked_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER), + updated_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER) +WHERE provider_id = 'github' AND revoked_at IS NULL; + +UPDATE github_connection_binding +SET status = 'revoked', updated_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER) +WHERE status = 'active'; + +DELETE FROM github_connection_context +WHERE broker_reference IN ( + SELECT broker_reference FROM github_connection_binding WHERE status = 'revoked' +); diff --git a/providers/github/README.md b/providers/github/README.md index c3b8341..e0685b1 100644 --- a/providers/github/README.md +++ b/providers/github/README.md @@ -37,10 +37,10 @@ tokens, not a Resource Server URL or caller-selected path parameter. - discover connected installations and selected repositories; - create an issue with Agent attribution. -The current executable slice supports installation repository discovery, issue -creation, and webhook-driven installation lifecycle invalidation. Pull -requests, comments, reviews, and delegated user-token operations remain roadmap -work. Standard OAuth revocation is implemented. +The current executable slice supports installation repository discovery, +attributed issue and pull-request operations, cross-fork pull-request creation, +and webhook-driven installation lifecycle invalidation. Standard OAuth +revocation is implemented. GitHub sends lifecycle deliveries to `/github/webhooks`. The adapter verifies `X-Hub-Signature-256`, durably deduplicates `X-GitHub-Delivery`, and updates @@ -51,10 +51,14 @@ future exchanges fail as soon as the Adapter observes removed authority. ## Initial credential modes - installation access token for App-attributed automation; -- GitHub App user access token only while authorizing and verifying a - Connection; it is not returned to Realmroot or retained as Agent authority. - -Provider credentials remain adapter-owned and are never returned to the Agent. +- encrypted GitHub App user access and refresh credentials for resolving the + target of a pull-request creation and for the cross-fork write that GitHub + cannot perform with the selected installation. + +The delegated-user credential is selected only after the Adapter verifies that +the pull-request head belongs to the approved installation account and +repository boundary. Provider credentials remain adapter-owned and are never +returned to Realmroot or the Agent. ## Acceptance outcome diff --git a/specs/github-adapter.feature b/specs/github-adapter.feature index 4f96821..3f2c0df 100644 --- a/specs/github-adapter.feature +++ b/specs/github-adapter.feature @@ -140,6 +140,16 @@ Feature: GitHub App adapter And unsupported createPullRequest, addComment, and mergePullRequest mutations use GitHub's installation-compatible REST operations And the GitHub credential is never returned + @journey:github-cross-fork-pull-request @entrypoint:http + Scenario: An Agent opens a pull request from an installed fork to an external upstream + Given the selected GitHub installation owns the pull request head fork + And the connected GitHub user can access the upstream repository + When GitHub CLI creates a pull request against that external upstream + Then the adapter verifies the head belongs to the selected installation + And creates the pull request with the connected user's delegated credential + And repository pushes continue to use a repository-constrained installation credential + And the delegated credential is encrypted at rest and never returned + @journey:github-git-transport @entrypoint:http Scenario: Native Git uses the GitHub installation through the adapter Given the Agent has approved repository contents authority diff --git a/src/providers/github/adapter.ts b/src/providers/github/adapter.ts index b7d76b8..40af11b 100644 --- a/src/providers/github/adapter.ts +++ b/src/providers/github/adapter.ts @@ -7,6 +7,7 @@ import { GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE } from './authorization-d import { createGitHubProvider } from './client.js' import type { GitHubAdapterConfig } from './config.js' import type { GitHubAuthorizationContext, GitHubConnectionStore } from './connections.js' +import type { GitHubUserCredentialStore } from './credentials.js' import { createGitHubCommentWithRest, createGitHubPullRequestWithRest, @@ -23,7 +24,7 @@ import { githubOpenApi } from './openapi.js' import { resolveGitHubOperationPermissions } from './operation-permissions.js' import { permissionsToScopes, scopesToPermissions } from './permissions.js' import { transformGitHubRequest } from './transformers.js' -import type { GitHubProvider } from './types.js' +import type { GitHubConnectionProvider, GitHubProvider } from './types.js' import { handleGitHubWebhook } from './webhooks.js' export type GitHubAdapterDependencies = { @@ -32,6 +33,8 @@ export type GitHubAdapterDependencies = { agentInfo?: AgentInfoResolver provider?: GitHubProvider connections?: GitHubConnectionStore + connectionProvider?: GitHubConnectionProvider + userCredentials?: GitHubUserCredentialStore } export function createGitHubAdapter( @@ -131,29 +134,25 @@ export function createGitHubAdapter( [...requiredScopes], ]) } - const lookupToken = await provider.installationToken({ - installationId: installation.installationId, - permissions: scopesToPermissions(new Set(['metadata:read']), available), - ...(installation.repositorySelection === 'selected' - ? { - repositories: installation.repositories.map( - (repository) => repository.fullName.split('/').at(-1) as string, - ), - } - : {}), - }) + const delegatedToken = await delegatedUserToken(principal.subject) const nameWithOwner = await resolveGitHubRepositoryName({ provider, - token: lookupToken, + token: delegatedToken, apiOrigin: config.githubApiOrigin, repositoryId: createPullRequest.repositoryId, }) - const repository = repositoryTarget(`/repos/${nameWithOwner}`, installation) - const createToken = await provider.installationToken({ - installationId: installation.installationId, - permissions: scopesToPermissions(new Set(['pull_requests:write']), available), - ...repositoryRestriction(installation, repository as string), - }) + const [baseOwner, baseRepository] = nameWithOwner.split('/') as [string, string] + const sameInstallation = baseOwner.toLowerCase() === installation.accountLogin.toLowerCase() + const createToken = sameInstallation + ? await provider.installationToken({ + installationId: installation.installationId, + permissions: scopesToPermissions(new Set(['pull_requests:write']), available), + ...repositoryRestriction( + installation, + repositoryTarget(`/repos/${nameWithOwner}`, installation) as string, + ), + }) + : delegatedCrossForkToken(installation, baseRepository, createPullRequest.headRefName, delegatedToken) const response = await createGitHubPullRequestWithRest({ provider, token: createToken, @@ -161,7 +160,14 @@ export function createGitHubAdapter( nameWithOwner, pullRequest: createPullRequest, }) - await auditNative(c, principal, installation, 'graphql-create-pull-request-rest-compatibility', response.status) + await auditNative( + c, + principal, + installation, + 'graphql-create-pull-request-rest-compatibility', + response.status, + sameInstallation ? undefined : { type: 'github_user', id: principal.subject }, + ) return response } const addComment = parseGitHubAddComment(body) @@ -391,6 +397,7 @@ export function createGitHubAdapter( installation: GitHubAuthorizationContext, operation: string, status: number, + providerActor: { type: 'github_user'; id: string } | undefined = undefined, ) { await dependencies.audit({ event: 'provider.operation', @@ -399,12 +406,54 @@ export function createGitHubAdapter( operation, installationId: installation.installationId, originatingPrincipal: { issuer: principal.actor.issuer, subject: principal.actor.subject }, - providerActor: { type: 'github_app', id: config.githubAppId ?? 'injected-test-provider' }, + providerActor: providerActor ?? { type: 'github_app', id: config.githubAppId ?? 'injected-test-provider' }, identityLevel: 'provider-delegated', result: { status }, occurredAt: new Date().toISOString(), }) } + + async function delegatedUserToken(subject: string) { + if (!dependencies.userCredentials || !dependencies.connectionProvider) { + throw forbidden('GitHub delegated-user operations are not configured.') + } + const credential = await dependencies.userCredentials.credential(subject) + if (credential.expiresAt === null || credential.expiresAt > Date.now() + 30_000) return credential.accessToken + if ( + !credential.refreshToken || + (credential.refreshTokenExpiresAt !== null && credential.refreshTokenExpiresAt <= Date.now()) + ) { + throw forbidden('Reconnect the GitHub account before creating cross-account pull requests.') + } + const refreshed = await dependencies.connectionProvider.refreshUserToken(credential.refreshToken) + if (!(await dependencies.userCredentials.replace(credential, refreshed))) { + return (await dependencies.userCredentials.credential(subject)).accessToken + } + return refreshed.accessToken + } +} + +function delegatedCrossForkToken( + installation: GitHubAuthorizationContext, + baseRepository: string, + headRefName: string, + delegatedToken: string, +) { + const separator = headRefName.indexOf(':') + const headOwner = separator > 0 ? headRefName.slice(0, separator) : '' + if (headOwner.toLowerCase() !== installation.accountLogin.toLowerCase()) { + throw forbidden('The pull request head must belong to the selected GitHub installation account.') + } + if ( + installation.repositorySelection === 'selected' && + !installation.repositories.some( + (repository) => + repository.fullName.toLowerCase() === `${installation.accountLogin}/${baseRepository}`.toLowerCase(), + ) + ) { + throw forbidden('The pull request head repository is outside the selected GitHub installation authority.') + } + return delegatedToken } function gitTransportTarget(requestUrl: string, method: string, origin: string) { diff --git a/src/providers/github/client.ts b/src/providers/github/client.ts index bd785ee..d5c8623 100644 --- a/src/providers/github/client.ts +++ b/src/providers/github/client.ts @@ -8,7 +8,24 @@ const githubApiVersion = '2026-03-10' const userAgent = 'realmroot-adapters/0.1' const permissionsSchema = z.record(z.string(), z.enum(['read', 'write', 'admin'])) const installationTokenSchema = z.object({ token: z.string().min(1), expires_at: z.iso.datetime() }) -const oauthTokenSchema = z.object({ access_token: z.string().min(1) }) +const oauthTokenSchema = z + .object({ + access_token: z.string().min(1), + expires_in: z.number().int().positive().optional(), + refresh_token: z.string().min(1).optional(), + refresh_token_expires_in: z.number().int().positive().optional(), + }) + .superRefine((value, context) => { + const expiring = value.expires_in !== undefined || value.refresh_token !== undefined + if ( + expiring && + (value.expires_in === undefined || + value.refresh_token === undefined || + value.refresh_token_expires_in === undefined) + ) { + context.addIssue({ code: 'custom', message: 'GitHub returned an incomplete expiring user credential.' }) + } + }) const userSchema = z.object({ id: z.number().int().positive(), login: z.string().min(1), name: z.string().nullable() }) const userInstallationsSchema = z.object({ installations: z.array( @@ -75,8 +92,8 @@ export function createGitHubProvider(input: GitHubClientInput): GitHubProvider { return installationTokenSchema.parse(await response.json()).token }, - request(request, installationToken, mode = 'api') { - return githubRequest(request, installationToken, false, mode) + request(request, token, mode = 'api') { + return githubRequest(request, token, false, mode) }, } @@ -114,6 +131,7 @@ export function createGitHubConnectionProvider( ): GitHubConnectionProvider { const fetcher = input.fetcher ?? fetch const appJwt = createAppJwt(input) + const now = input.now ?? Date.now return { authorizationUrl(state) { @@ -124,19 +142,10 @@ export function createGitHubConnectionProvider( return url.toString() }, async exchangeUserCode(code) { - const response = await fetcher('https://github.com/login/oauth/access_token', { - method: 'POST', - headers: { accept: 'application/json', 'content-type': 'application/json', 'user-agent': userAgent }, - body: JSON.stringify({ - client_id: input.clientId, - client_secret: input.clientSecret, - code, - redirect_uri: input.redirectUri, - }), - signal: AbortSignal.timeout(10_000), - }) - if (!response.ok) throw failedDependency(`GitHub rejected OAuth authorization with ${response.status}.`) - return oauthTokenSchema.parse(await response.json()).access_token + return tokenRequest({ code, redirect_uri: input.redirectUri }) + }, + async refreshUserToken(refreshToken) { + return tokenRequest({ grant_type: 'refresh_token', refresh_token: refreshToken }) }, async getUser(token) { const response = await userRequest('/user', token) @@ -173,6 +182,28 @@ export function createGitHubConnectionProvider( }, } + async function tokenRequest(parameters: Record) { + const response = await fetcher('https://github.com/login/oauth/access_token', { + method: 'POST', + headers: { accept: 'application/json', 'content-type': 'application/json', 'user-agent': userAgent }, + body: JSON.stringify({ + client_id: input.clientId, + client_secret: input.clientSecret, + ...parameters, + }), + signal: AbortSignal.timeout(10_000), + }) + if (!response.ok) throw failedDependency(`GitHub rejected OAuth authorization with ${response.status}.`) + const token = oauthTokenSchema.parse(await response.json()) + return { + accessToken: token.access_token, + refreshToken: token.refresh_token ?? null, + expiresAt: token.expires_in === undefined ? null : now() + token.expires_in * 1000, + refreshTokenExpiresAt: + token.refresh_token_expires_in === undefined ? null : now() + token.refresh_token_expires_in * 1000, + } + } + async function listInstallationRepositories(token: string, installationId: number) { const repositories: Array<{ id: number; fullName: string }> = [] let page = 1 diff --git a/src/providers/github/config.ts b/src/providers/github/config.ts index ff4d571..2cf9271 100644 --- a/src/providers/github/config.ts +++ b/src/providers/github/config.ts @@ -9,6 +9,7 @@ const githubEnvironmentSchema = z.object({ GITHUB_PRIVATE_KEY: z.string().trim().min(1).optional(), GITHUB_CLIENT_ID: z.string().trim().min(1).optional(), GITHUB_CLIENT_SECRET: z.string().trim().min(1).optional(), + GITHUB_CREDENTIAL_ENCRYPTION_KEY: z.string().trim().min(1).optional(), GITHUB_WEBHOOK_SECRET: z.string().min(32).optional(), }) @@ -20,6 +21,7 @@ export type GitHubAdapterConfig = AppConfig & { githubPrivateKey?: string githubClientId?: string githubClientSecret?: string + githubCredentialEncryptionKey?: string githubWebhookSecret?: string } @@ -34,6 +36,9 @@ export function loadGitHubConfig(environment: unknown, config: AppConfig): GitHu ...(parsed.GITHUB_PRIVATE_KEY ? { githubPrivateKey: parsed.GITHUB_PRIVATE_KEY } : {}), ...(parsed.GITHUB_CLIENT_ID ? { githubClientId: parsed.GITHUB_CLIENT_ID } : {}), ...(parsed.GITHUB_CLIENT_SECRET ? { githubClientSecret: parsed.GITHUB_CLIENT_SECRET } : {}), + ...(parsed.GITHUB_CREDENTIAL_ENCRYPTION_KEY + ? { githubCredentialEncryptionKey: parsed.GITHUB_CREDENTIAL_ENCRYPTION_KEY } + : {}), ...(parsed.GITHUB_WEBHOOK_SECRET ? { githubWebhookSecret: parsed.GITHUB_WEBHOOK_SECRET } : {}), } } diff --git a/src/providers/github/credentials.ts b/src/providers/github/credentials.ts new file mode 100644 index 0000000..3cfadc7 --- /dev/null +++ b/src/providers/github/credentials.ts @@ -0,0 +1,102 @@ +import type { CredentialCipher } from '../../core/credential-cipher.js' +import { forbidden } from '../../core/problem.js' +import type { GitHubUserCredential, GitHubUserToken } from './types.js' + +export interface GitHubUserCredentialStore { + upsert(subject: string, token: GitHubUserToken): Promise + credential(subject: string): Promise + replace(credential: GitHubUserCredential, token: GitHubUserToken): Promise + revoke(subject: string): Promise +} + +export class D1GitHubUserCredentials implements GitHubUserCredentialStore { + constructor( + private readonly db: D1Database, + private readonly cipher: CredentialCipher, + ) {} + + async upsert(subject: string, token: GitHubUserToken) { + const context = credentialContext(subject) + const [accessToken, refreshToken] = await Promise.all([ + this.cipher.seal(token.accessToken, `${context}:access`), + token.refreshToken ? this.cipher.seal(token.refreshToken, `${context}:refresh`) : null, + ]) + await this.db + .prepare( + `INSERT INTO github_user_credential + (subject, access_token_ciphertext, refresh_token_ciphertext, access_token_expires_at, + refresh_token_expires_at, credential_version, updated_at) + VALUES (?, ?, ?, ?, ?, 1, ?) + ON CONFLICT(subject) DO UPDATE SET + access_token_ciphertext = excluded.access_token_ciphertext, + refresh_token_ciphertext = excluded.refresh_token_ciphertext, + access_token_expires_at = excluded.access_token_expires_at, + refresh_token_expires_at = excluded.refresh_token_expires_at, + credential_version = github_user_credential.credential_version + 1, + updated_at = excluded.updated_at`, + ) + .bind(subject, accessToken, refreshToken, token.expiresAt, token.refreshTokenExpiresAt, Date.now()) + .run() + } + + async credential(subject: string): Promise { + const row = await this.db + .prepare( + `SELECT subject, access_token_ciphertext AS accessToken, + refresh_token_ciphertext AS refreshToken, access_token_expires_at AS expiresAt, + refresh_token_expires_at AS refreshTokenExpiresAt, + credential_version AS credentialVersion + FROM github_user_credential WHERE subject = ?`, + ) + .bind(subject) + .first<{ + subject: string + accessToken: string + refreshToken: string | null + expiresAt: number | null + refreshTokenExpiresAt: number | null + credentialVersion: number + }>() + if (!row) throw forbidden('Reconnect the GitHub account before creating cross-account pull requests.') + const context = credentialContext(subject) + const [accessToken, refreshToken] = await Promise.all([ + this.cipher.open(row.accessToken, `${context}:access`), + row.refreshToken ? this.cipher.open(row.refreshToken, `${context}:refresh`) : null, + ]) + return { ...row, accessToken, refreshToken } + } + + async replace(credential: GitHubUserCredential, token: GitHubUserToken) { + const context = credentialContext(credential.subject) + const [accessToken, refreshToken] = await Promise.all([ + this.cipher.seal(token.accessToken, `${context}:access`), + token.refreshToken ? this.cipher.seal(token.refreshToken, `${context}:refresh`) : null, + ]) + const result = await this.db + .prepare( + `UPDATE github_user_credential SET access_token_ciphertext = ?, refresh_token_ciphertext = ?, + access_token_expires_at = ?, refresh_token_expires_at = ?, + credential_version = credential_version + 1, updated_at = ? + WHERE subject = ? AND credential_version = ?`, + ) + .bind( + accessToken, + refreshToken, + token.expiresAt, + token.refreshTokenExpiresAt, + Date.now(), + credential.subject, + credential.credentialVersion, + ) + .run() + return result.meta.changes === 1 + } + + async revoke(subject: string) { + await this.db.prepare('DELETE FROM github_user_credential WHERE subject = ?').bind(subject).run() + } +} + +function credentialContext(subject: string) { + return `github:${subject}:delegated-user` +} diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index d21c7ff..40c5615 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -9,6 +9,7 @@ import { githubInstallationAuthorizationDetailDisplay, } from './authorization-details.js' import type { D1GitHubConnections } from './connections.js' +import type { GitHubUserCredentialStore } from './credentials.js' import { permissionsToScopes } from './permissions.js' import type { GitHubConnectionProvider } from './types.js' @@ -18,6 +19,7 @@ export function createGitHubExternalAuthorization(input: { origin: string connection: GitHubConnectionProvider connections: D1GitHubConnections + credentials: GitHubUserCredentialStore oauthStore: D1ExternalOAuthStore scopes: readonly string[] }): { authorization: ExternalProviderAuthorization; installationCallback: AdapterModule } { @@ -135,8 +137,8 @@ export function createGitHubExternalAuthorization(input: { }) ) }, - revoke(subject) { - return input.connections.revokeExternalAuthorization(subject) + async revoke(subject) { + await Promise.all([input.connections.revokeExternalAuthorization(subject), input.credentials.revoke(subject)]) }, begin({ providerState }) { return { url: input.connection.authorizationUrl(providerState), stage: 'oauth' } @@ -148,14 +150,15 @@ export function createGitHubExternalAuthorization(input: { const callback = new URL(callbackUrl) const userToken = await input.connection.exchangeUserCode(required(callback.searchParams.get('code'), 'code')) const [user, installations] = await Promise.all([ - input.connection.getUser(userToken), - input.connection.listUserInstallations(userToken), + input.connection.getUser(userToken.accessToken), + input.connection.listUserInstallations(userToken.accessToken), ]) 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.') } + await input.credentials.upsert(String(user.id), userToken) if (installations.length === 0) { const providerState = nextProviderState() return { diff --git a/src/providers/github/graphql.ts b/src/providers/github/graphql.ts index f30d044..4eb551c 100644 --- a/src/providers/github/graphql.ts +++ b/src/providers/github/graphql.ts @@ -18,7 +18,7 @@ const createInputSchema = z.object({ maintainerCanModify: z.boolean().optional(), }) const repositoryLookupSchema = z.object({ - data: z.object({ node: z.object({ nameWithOwner: z.string().min(3) }) }), + data: z.object({ node: z.object({ nameWithOwner: z.string().regex(/^[^/]+\/[^/]+$/) }) }), }) const pullRequestSchema = z.object({ node_id: z.string().min(1), html_url: z.url() }) const mergeInputSchema = z.object({ diff --git a/src/providers/github/types.ts b/src/providers/github/types.ts index 1746880..8151ba3 100644 --- a/src/providers/github/types.ts +++ b/src/providers/github/types.ts @@ -11,10 +11,21 @@ export interface GitHubProvider { appPermissions(): Promise openApiDocument(): Promise installationToken(input: GitHubInstallationTokenRequest): Promise - request(request: Request, installationToken: string, mode?: 'api' | 'git'): Promise + request(request: Request, token: string, mode?: 'api' | 'git'): Promise } export type GitHubUser = Readonly<{ id: number; login: string; name: string | null }> +export type GitHubUserToken = Readonly<{ + accessToken: string + refreshToken: string | null + expiresAt: number | null + refreshTokenExpiresAt: number | null +}> +export type GitHubUserCredential = GitHubUserToken & + Readonly<{ + subject: string + credentialVersion: number + }> export type GitHubInstallation = Readonly<{ id: number htmlUrl: string @@ -30,7 +41,8 @@ export type GitHubRepository = Readonly<{ id: number; fullName: string }> export interface GitHubConnectionProvider { authorizationUrl(state: string): string - exchangeUserCode(code: string): Promise + exchangeUserCode(code: string): Promise + refreshUserToken(refreshToken: string): Promise getUser(token: string): Promise listUserInstallations(token: string): Promise newInstallationUrl(state: string): Promise diff --git a/src/worker.ts b/src/worker.ts index ef58cda..017dd0e 100644 --- a/src/worker.ts +++ b/src/worker.ts @@ -16,6 +16,7 @@ import { createGitHubAdapter } from './providers/github/adapter.js' import { createGitHubConnectionProvider, createGitHubProvider } from './providers/github/client.js' import { loadGitHubConfig } from './providers/github/config.js' import { D1GitHubConnections } from './providers/github/connections.js' +import { D1GitHubUserCredentials } from './providers/github/credentials.js' import { createGitHubExternalAuthorization } from './providers/github/external-authorization.js' import { permissionsToScopes } from './providers/github/permissions.js' import { createLinearAdapter } from './providers/linear/adapter.js' @@ -43,25 +44,32 @@ export default { githubConfig.githubPrivateKey && githubConfig.githubClientId && githubConfig.githubClientSecret && + githubConfig.githubCredentialEncryptionKey && signingPrivateJwk ) { const githubConnections = new D1GitHubConnections(env.DB, state) + const githubUserCredentials = new D1GitHubUserCredentials( + env.DB, + createCredentialCipher(githubConfig.githubCredentialEncryptionKey), + ) const githubProvider = createGitHubProvider({ appId: githubConfig.githubAppId, privateKey: githubConfig.githubPrivateKey, apiOrigin: githubConfig.githubApiOrigin, }) + const githubConnectionProvider = createGitHubConnectionProvider({ + appId: githubConfig.githubAppId, + privateKey: githubConfig.githubPrivateKey, + clientId: githubConfig.githubClientId, + clientSecret: githubConfig.githubClientSecret, + redirectUri: `${config.origin}/github/oauth/callback`, + apiOrigin: githubConfig.githubApiOrigin, + }) const githubExternal = createGitHubExternalAuthorization({ origin: config.origin, - connection: createGitHubConnectionProvider({ - appId: githubConfig.githubAppId, - privateKey: githubConfig.githubPrivateKey, - clientId: githubConfig.githubClientId, - clientSecret: githubConfig.githubClientSecret, - redirectUri: `${config.origin}/github/oauth/callback`, - apiOrigin: githubConfig.githubApiOrigin, - }), + connection: githubConnectionProvider, connections: githubConnections, + credentials: githubUserCredentials, oauthStore, scopes: permissionsToScopes(await githubProvider.appPermissions()), }) @@ -81,6 +89,8 @@ export default { provider: githubProvider, audit: (record) => state.recordAudit(record), connections: githubConnections, + connectionProvider: githubConnectionProvider, + userCredentials: githubUserCredentials, }), ) } diff --git a/test/app.test.ts b/test/app.test.ts index f7bd701..821a0a3 100644 --- a/test/app.test.ts +++ b/test/app.test.ts @@ -140,7 +140,7 @@ describe('GitHub adapter contract', () => { .fn() .mockImplementationOnce(async (request: Request, token: string) => { expect(request.url).toBe('https://api.github.com/graphql') - expect(token).toBe('installation-secret') + expect(token).toBe('delegated-user-token') await expect(request.json()).resolves.toMatchObject({ variables: { id: 'repository-1' } }) return Response.json({ data: { node: { nameWithOwner: 'realmroot/example' } } }) }) @@ -189,16 +189,98 @@ describe('GitHub adapter contract', () => { await expect(response.json()).resolves.toMatchObject({ data: { createPullRequest: { pullRequest: { url: 'https://github.test/pull/1' } } }, }) - expect(provider.installationToken).toHaveBeenNthCalledWith(1, { - installationId: 42, - permissions: { metadata: 'read' }, - }) - expect(provider.installationToken).toHaveBeenNthCalledWith(2, { + expect(provider.installationToken).toHaveBeenCalledTimes(1) + expect(provider.installationToken).toHaveBeenCalledWith({ installationId: 42, permissions: { pull_requests: 'write' }, }) }) + it('[spec: github-adapter/github-cross-fork-pull-request] creates a cross-fork pull request as the connected user', async () => { + const provider = fakeProvider() + provider.request = vi + .fn() + .mockImplementationOnce(async (request: Request, token: string) => { + expect(request.url).toBe('https://api.github.com/graphql') + expect(token).toBe('delegated-user-token') + return Response.json({ data: { node: { nameWithOwner: 'upstream/example' } } }) + }) + .mockImplementationOnce(async (request: Request, token: string) => { + expect(request.url).toBe('https://api.github.com/repos/upstream/example/pulls') + expect(token).toBe('delegated-user-token') + await expect(request.json()).resolves.toMatchObject({ head: 'realmroot:codex/fix', base: 'main' }) + return Response.json({ node_id: 'pull-request-2', html_url: 'https://github.test/pull/2' }, { status: 201 }) + }) + const audit = vi.fn(async () => {}) + const response = await testApp({ + provider, + audit, + authenticator: { + authenticate: vi.fn(async () => ({ + ...principal, + scopes: new Set(['metadata:read', 'pull_requests:write']), + })), + }, + }).request('/github/graphql', { + method: 'POST', + headers: { 'Content-Type': 'application/json; charset=utf-8' }, + body: JSON.stringify({ + query: + 'mutation PullRequestCreate($input: CreatePullRequestInput!) { createPullRequest(input: $input) { pullRequest { id url } } }', + variables: { + input: { + repositoryId: 'upstream-repository', + baseRefName: 'main', + headRefName: 'realmroot:codex/fix', + title: 'Fix adapter', + }, + }, + }), + }) + + expect(response.status).toBe(200) + expect(provider.installationToken).not.toHaveBeenCalled() + expect(audit).toHaveBeenCalledWith( + expect.objectContaining({ providerActor: { type: 'github_user', id: principal.subject } }), + ) + }) + + it('[spec: github-adapter/github-cross-fork-pull-request] rejects a head outside the selected installation boundary', async () => { + const provider = fakeProvider() + provider.request = vi.fn(async () => Response.json({ data: { node: { nameWithOwner: 'upstream/example' } } })) + const authenticate = vi.fn(async () => ({ + ...principal, + scopes: new Set(['metadata:read', 'pull_requests:write']), + })) + + const wrongOwner = await testApp({ provider, authenticator: { authenticate } }).request( + '/github/graphql', + createPullRequestRequest('attacker:codex/fix'), + ) + expect(wrongOwner.status).toBe(403) + + const selectedRepositoryConnection = fakeConnections() + selectedRepositoryConnection.externalAuthorization = vi.fn(async () => + authorizationWithContexts([ + { + installationId: 42, + accountLogin: 'realmroot', + targetType: 'Organization', + scopes: ['metadata:read', 'pull_requests:write'], + repositorySelection: 'selected', + repositories: [{ id: 1, fullName: 'realmroot/another-repository' }], + }, + ]), + ) + const unselectedRepository = await testApp({ + provider, + connections: selectedRepositoryConnection, + authenticator: { authenticate }, + }).request('/github/graphql', createPullRequestRequest('realmroot:codex/fix')) + expect(unselectedRepository.status).toBe(403) + expect(provider.request).toHaveBeenCalledTimes(2) + }) + it('[spec: github-adapter/github-graphql-proxy] preserves the GitHub CLI addComment mutation', async () => { const provider = fakeProvider() provider.request = vi @@ -641,6 +723,26 @@ function testApp( }, provider: fakeProvider(), connections: fakeConnections(), + connectionProvider: { + authorizationUrl: vi.fn(), + exchangeUserCode: vi.fn(), + refreshUserToken: vi.fn(), + getUser: vi.fn(), + listUserInstallations: vi.fn(), + newInstallationUrl: vi.fn(), + permissionUpdateUrl: vi.fn(), + }, + userCredentials: { + credential: vi.fn(async () => ({ + subject: principal.subject, + accessToken: 'delegated-user-token', + refreshToken: 'delegated-refresh-token', + expiresAt: Date.now() + 60_000, + refreshTokenExpiresAt: Date.now() + 120_000, + credentialVersion: 1, + })), + replace: vi.fn(), + } as never, audit: vi.fn(async () => {}), ...overrides, } @@ -770,3 +872,22 @@ function authorizationWithContexts( contexts, } } + +function createPullRequestRequest(headRefName: string) { + return { + method: 'POST', + headers: { 'Content-Type': 'application/json; charset=utf-8' }, + body: JSON.stringify({ + query: + 'mutation PullRequestCreate($input: CreatePullRequestInput!) { createPullRequest(input: $input) { pullRequest { id url } } }', + variables: { + input: { + repositoryId: 'upstream-repository', + baseRefName: 'main', + headRefName, + title: 'Fix adapter', + }, + }, + }), + } +} diff --git a/test/integration/github-user-credentials.test.ts b/test/integration/github-user-credentials.test.ts new file mode 100644 index 0000000..44ca562 --- /dev/null +++ b/test/integration/github-user-credentials.test.ts @@ -0,0 +1,49 @@ +import { env } from 'cloudflare:test' +import { describe, expect, it } from 'vitest' +import { createCredentialCipher } from '../../src/core/credential-cipher.js' +import { D1GitHubUserCredentials } from '../../src/providers/github/credentials.js' + +describe('GitHub delegated-user credential persistence', () => { + it('[spec: github-adapter/github-cross-fork-pull-request] encrypts, rotates, and revokes the provider credential', async () => { + const store = new D1GitHubUserCredentials( + env.DB, + createCredentialCipher('AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'), + ) + await store.upsert('70', { + accessToken: 'user-token', + refreshToken: 'refresh-token', + expiresAt: 1_800_000_000_000, + refreshTokenExpiresAt: 1_900_000_000_000, + }) + + const stored = await env.DB.prepare( + 'SELECT access_token_ciphertext AS accessToken, refresh_token_ciphertext AS refreshToken FROM github_user_credential WHERE subject = ?', + ) + .bind('70') + .first<{ accessToken: string; refreshToken: string }>() + expect(stored?.accessToken).not.toContain('user-token') + expect(stored?.refreshToken).not.toContain('refresh-token') + + const credential = await store.credential('70') + expect(credential).toMatchObject({ accessToken: 'user-token', refreshToken: 'refresh-token', credentialVersion: 1 }) + await expect( + store.replace(credential, { + accessToken: 'fresh-user-token', + refreshToken: 'fresh-refresh-token', + expiresAt: 1_810_000_000_000, + refreshTokenExpiresAt: 1_910_000_000_000, + }), + ).resolves.toBe(true) + await expect(store.replace(credential, credential)).resolves.toBe(false) + await expect(store.credential('70')).resolves.toMatchObject({ + accessToken: 'fresh-user-token', + refreshToken: 'fresh-refresh-token', + credentialVersion: 2, + }) + + await store.revoke('70') + await expect(store.credential('70')).rejects.toThrow( + 'Reconnect the GitHub account before creating cross-account pull requests.', + ) + }) +}) diff --git a/test/integration/provider-connection-migration.test.ts b/test/integration/provider-connection-migration.test.ts index e2b8346..90dfbfa 100644 --- a/test/integration/provider-connection-migration.test.ts +++ b/test/integration/provider-connection-migration.test.ts @@ -10,6 +10,8 @@ describe('Provider connection migration', () => { const linearConnections = env.TEST_MIGRATIONS.slice(5, 6) const webhookLifecycle = env.TEST_MIGRATIONS.slice(6, 7) const externalAuthorizationServer = env.TEST_MIGRATIONS.slice(7, 8) + const linearAuthorizationCleanup = env.TEST_MIGRATIONS.slice(8, 9) + const githubDelegatedCredentials = env.TEST_MIGRATIONS.slice(9, 10) expect(legacy).toHaveLength(2) expect(lifecycle).toHaveLength(1) expect(installationOwnership).toHaveLength(1) @@ -17,6 +19,8 @@ describe('Provider connection migration', () => { expect(linearConnections).toHaveLength(1) expect(webhookLifecycle).toHaveLength(1) expect(externalAuthorizationServer).toHaveLength(1) + expect(linearAuthorizationCleanup).toHaveLength(1) + expect(githubDelegatedCredentials).toHaveLength(1) await applyD1Migrations(env.MIGRATION_DB, legacy) const now = Date.now() await env.MIGRATION_DB.batch([ @@ -177,5 +181,40 @@ describe('Provider connection migration', () => { "SELECT COUNT(*) AS count FROM sqlite_master WHERE type = 'table' AND name IN ('external_oauth_client', 'external_oauth_intent', 'external_oauth_code', 'external_oauth_refresh', 'external_oauth_access', 'cloudflare_external_credential')", ).first(), ).resolves.toEqual({ count: 6 }) + + await env.MIGRATION_DB.batch([ + env.MIGRATION_DB.prepare( + `INSERT INTO external_oauth_client + (client_id, provider_id, client_secret_hash, redirect_uris_json, jwks_uri, created_at) + VALUES (?, 'github', ?, '[]', ?, ?)`, + ).bind('github-client', 'hash', 'https://id.example/jwks', now), + env.MIGRATION_DB.prepare( + `INSERT INTO external_oauth_refresh + (token_hash, provider_id, client_id, subject, display_name, scope_json, + authorization_details_json, created_at, updated_at) + VALUES (?, 'github', ?, ?, ?, '[]', '[]', ?, ?)`, + ).bind('refresh-hash', 'github-client', '7', 'Controller', now, now), + ]) + await applyD1Migrations(env.MIGRATION_DB, linearAuthorizationCleanup) + await applyD1Migrations(env.MIGRATION_DB, githubDelegatedCredentials) + + await expect( + env.MIGRATION_DB.prepare( + "SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'github_user_credential'", + ).first(), + ).resolves.toEqual({ name: 'github_user_credential' }) + await expect( + env.MIGRATION_DB.prepare( + "SELECT revoked_at IS NOT NULL AS revoked FROM external_oauth_refresh WHERE provider_id = 'github'", + ).first(), + ).resolves.toEqual({ revoked: 1 }) + await expect( + env.MIGRATION_DB.prepare( + "SELECT COUNT(*) AS count FROM github_connection_binding WHERE status = 'active'", + ).first(), + ).resolves.toEqual({ count: 0 }) + await expect( + env.MIGRATION_DB.prepare('SELECT COUNT(*) AS count FROM github_connection_context').first(), + ).resolves.toEqual({ count: 0 }) }) }) diff --git a/test/providers/github-client.test.ts b/test/providers/github-client.test.ts index e0ef60c..4288e6e 100644 --- a/test/providers/github-client.test.ts +++ b/test/providers/github-client.test.ts @@ -93,7 +93,12 @@ describe('GitHub account connection OAuth boundary', () => { const authorization = new URL(provider.authorizationUrl('provider-state')) expect(authorization.searchParams.get('redirect_uri')).toBe('https://adapters.realmroot.dev/github/oauth/callback') - await expect(provider.exchangeUserCode('authorization-code')).resolves.toBe('user-token') + await expect(provider.exchangeUserCode('authorization-code')).resolves.toEqual({ + accessToken: 'user-token', + refreshToken: null, + expiresAt: null, + refreshTokenExpiresAt: null, + }) expect(requests[0]).toEqual({ url: 'https://github.com/login/oauth/access_token', body: { @@ -108,6 +113,37 @@ describe('GitHub account connection OAuth boundary', () => { ) }) + it('refreshes an expiring delegated-user credential with the GitHub App client', async () => { + 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', + now: () => 1_800_000_000_000, + fetcher: async (_input, init) => { + expect(JSON.parse(String(init?.body))).toMatchObject({ + grant_type: 'refresh_token', + refresh_token: 'old-refresh-token', + }) + return Response.json({ + access_token: 'fresh-user-token', + expires_in: 28_800, + refresh_token: 'fresh-refresh-token', + refresh_token_expires_in: 15_897_600, + }) + }, + }) + + await expect(provider.refreshUserToken('old-refresh-token')).resolves.toEqual({ + accessToken: 'fresh-user-token', + refreshToken: 'fresh-refresh-token', + expiresAt: 1_800_028_800_000, + refreshTokenExpiresAt: 1_815_897_600_000, + }) + }) + it('loads selected repository membership with the installation context', async () => { const requests: string[] = [] const provider = createGitHubConnectionProvider({ diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index 1ae68da..2c50abc 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -3,6 +3,7 @@ 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' import type { D1GitHubConnections } from '../../src/providers/github/connections.js' +import type { GitHubUserCredentialStore } from '../../src/providers/github/credentials.js' import { createGitHubExternalAuthorization } from '../../src/providers/github/external-authorization.js' import type { GitHubInstallation } from '../../src/providers/github/types.js' @@ -24,6 +25,7 @@ describe('GitHub external authorization', () => { connections: { externalAuthorization: vi.fn(async () => ({ scopes: ['metadata:read'], contexts })), } as unknown as D1GitHubConnections, + credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['metadata:read'], }) @@ -84,6 +86,7 @@ describe('GitHub external authorization', () => { contexts, })), } as unknown as D1GitHubConnections, + credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['administration:write', 'metadata:read'], }) @@ -120,6 +123,7 @@ describe('GitHub external authorization', () => { ], })), } as unknown as D1GitHubConnections, + credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['actions:read', 'metadata:read'], }) @@ -160,13 +164,15 @@ describe('GitHub external authorization', () => { origin: 'https://adapter.example', connection: { authorizationUrl: vi.fn(() => 'https://github.com/login/oauth/authorize'), - exchangeUserCode: vi.fn(async () => 'user-token'), + exchangeUserCode: vi.fn(async () => userToken()), + refreshUserToken: vi.fn(async () => userToken()), 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, + credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['metadata:read', 'pull_requests:read', 'pull_requests:write'], }) @@ -198,6 +204,7 @@ describe('GitHub external authorization', () => { connections: { upsertExternalAuthorization: vi.fn(async () => [connectionContext(selected), connectionContext(other)]), } as unknown as D1GitHubConnections, + credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['administration:read'], }) @@ -235,10 +242,15 @@ describe('GitHub external authorization', () => { const installation = githubInstallation({ administration: 'read' }) const connections = { upsertExternalAuthorization: vi.fn() } const connection = githubConnection([installation]) + const upsertCredential = vi.fn() const external = createGitHubExternalAuthorization({ origin: 'https://adapter.example', connection, connections: connections as unknown as D1GitHubConnections, + credentials: { + upsert: upsertCredential, + revoke: vi.fn(), + } as unknown as GitHubUserCredentialStore, oauthStore: {} as D1ExternalOAuthStore, scopes: ['administration:read', 'administration:write'], }) @@ -267,6 +279,7 @@ describe('GitHub external authorization', () => { }) expect(connection.newInstallationUrl).not.toHaveBeenCalled() expect(connections.upsertExternalAuthorization).not.toHaveBeenCalled() + expect(upsertCredential).toHaveBeenCalledWith('70', expect.objectContaining({ accessToken: 'user-token' })) }) it('[spec: github-adapter/github-installation-permission-upgrade] resumes only after the target installation accepts the permission', async () => { @@ -290,6 +303,7 @@ describe('GitHub external authorization', () => { origin: 'https://adapter.example', connection: githubConnection([before]), connections: { externalAuthorization } as unknown as D1GitHubConnections, + credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['administration:read', 'administration:write'], }) @@ -343,6 +357,7 @@ describe('GitHub external authorization', () => { origin: 'https://adapter.example', connection, connections: {} as D1GitHubConnections, + credentials: githubCredentials(), oauthStore: oauthStore as unknown as D1ExternalOAuthStore, scopes: ['administration:write'], }) @@ -375,7 +390,8 @@ function githubInstallation(permissions: Record) { function githubConnection(installations: ReturnType[]) { return { authorizationUrl: vi.fn((state: string) => `https://github.com/login/oauth/authorize?state=${state}`), - exchangeUserCode: vi.fn(async () => 'user-token'), + exchangeUserCode: vi.fn(async () => userToken()), + refreshUserToken: vi.fn(async () => userToken()), getUser: vi.fn(async () => ({ id: 70, login: 'controller', name: 'Controller' })), listUserInstallations: vi.fn(async () => installations), newInstallationUrl: vi.fn( @@ -387,6 +403,22 @@ function githubConnection(installations: ReturnType[] } } +function githubCredentials() { + return { + upsert: vi.fn(), + revoke: vi.fn(), + } as unknown as GitHubUserCredentialStore +} + +function userToken() { + return { + accessToken: 'user-token', + refreshToken: 'refresh-token', + expiresAt: Date.now() + 60_000, + refreshTokenExpiresAt: Date.now() + 120_000, + } +} + function connectionContext(installation: ReturnType) { const permissions = installation.permissions as Record return { diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 9c0a40c..2de96e0 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,5 +1,5 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types --env-file=.dev.vars.example --strict-vars=false` (hash: b7a9d5af36095ca8d444ae9520538651) +// Generated by Wrangler by running `wrangler types --env-file=.dev.vars.example --strict-vars=false` (hash: 070d295b2af2928fdff0fd9e0a61e120) // Runtime types generated with workerd@1.20260801.1 2026-08-08 nodejs_compat interface __BaseEnv_Env { DB: D1Database; @@ -16,6 +16,7 @@ interface __BaseEnv_Env { GITHUB_PRIVATE_KEY: string; GITHUB_CLIENT_ID: string; GITHUB_CLIENT_SECRET: string; + GITHUB_CREDENTIAL_ENCRYPTION_KEY: string; GITHUB_WEBHOOK_SECRET: string; LINEAR_CLIENT_ID: string; LINEAR_CLIENT_SECRET: string; @@ -37,7 +38,7 @@ type StringifyValues> = { [Binding in keyof EnvType]: EnvType[Binding] extends string ? EnvType[Binding] : string; }; declare namespace NodeJS { - interface ProcessEnv extends StringifyValues> {} + interface ProcessEnv extends StringifyValues> {} } // Begin runtime types diff --git a/wrangler.jsonc b/wrangler.jsonc index 2cf5def..f65cbc6 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -25,6 +25,7 @@ "GITHUB_PRIVATE_KEY", "GITHUB_CLIENT_ID", "GITHUB_CLIENT_SECRET", + "GITHUB_CREDENTIAL_ENCRYPTION_KEY", "GITHUB_WEBHOOK_SECRET", "LINEAR_CLIENT_ID", "LINEAR_CLIENT_SECRET", From be1df4a9dc762aca6078a25a45ed0258b465c013 Mon Sep 17 00:00:00 2001 From: saltbo Date: Fri, 14 Aug 2026 16:59:46 -0400 Subject: [PATCH 2/5] fix(github): keep migrations free of authorization cleanup --- migrations/0010_github_user_credentials.sql | 14 -------------- .../provider-connection-migration.test.ts | 6 +++--- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/migrations/0010_github_user_credentials.sql b/migrations/0010_github_user_credentials.sql index 46c1dc3..99d2de3 100644 --- a/migrations/0010_github_user_credentials.sql +++ b/migrations/0010_github_user_credentials.sql @@ -7,17 +7,3 @@ CREATE TABLE github_user_credential ( credential_version INTEGER NOT NULL DEFAULT 1, updated_at INTEGER NOT NULL ); - -UPDATE external_oauth_refresh -SET revoked_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER), - updated_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER) -WHERE provider_id = 'github' AND revoked_at IS NULL; - -UPDATE github_connection_binding -SET status = 'revoked', updated_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER) -WHERE status = 'active'; - -DELETE FROM github_connection_context -WHERE broker_reference IN ( - SELECT broker_reference FROM github_connection_binding WHERE status = 'revoked' -); diff --git a/test/integration/provider-connection-migration.test.ts b/test/integration/provider-connection-migration.test.ts index 90dfbfa..c5a542c 100644 --- a/test/integration/provider-connection-migration.test.ts +++ b/test/integration/provider-connection-migration.test.ts @@ -207,14 +207,14 @@ describe('Provider connection migration', () => { env.MIGRATION_DB.prepare( "SELECT revoked_at IS NOT NULL AS revoked FROM external_oauth_refresh WHERE provider_id = 'github'", ).first(), - ).resolves.toEqual({ revoked: 1 }) + ).resolves.toEqual({ revoked: 0 }) await expect( env.MIGRATION_DB.prepare( "SELECT COUNT(*) AS count FROM github_connection_binding WHERE status = 'active'", ).first(), - ).resolves.toEqual({ count: 0 }) + ).resolves.toEqual({ count: 2 }) await expect( env.MIGRATION_DB.prepare('SELECT COUNT(*) AS count FROM github_connection_context').first(), - ).resolves.toEqual({ count: 0 }) + ).resolves.toEqual({ count: 1 }) }) }) From 5c1bf7f6c53fe8dcb39ec37322c7aab83b851225 Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 17:31:35 -0400 Subject: [PATCH 3/5] Revert "fix(github): keep migrations free of authorization cleanup" This reverts commit be1df4a9dc762aca6078a25a45ed0258b465c013. --- migrations/0010_github_user_credentials.sql | 14 ++++++++++++++ .../provider-connection-migration.test.ts | 6 +++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/migrations/0010_github_user_credentials.sql b/migrations/0010_github_user_credentials.sql index 99d2de3..46c1dc3 100644 --- a/migrations/0010_github_user_credentials.sql +++ b/migrations/0010_github_user_credentials.sql @@ -7,3 +7,17 @@ CREATE TABLE github_user_credential ( credential_version INTEGER NOT NULL DEFAULT 1, updated_at INTEGER NOT NULL ); + +UPDATE external_oauth_refresh +SET revoked_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER), + updated_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER) +WHERE provider_id = 'github' AND revoked_at IS NULL; + +UPDATE github_connection_binding +SET status = 'revoked', updated_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER) +WHERE status = 'active'; + +DELETE FROM github_connection_context +WHERE broker_reference IN ( + SELECT broker_reference FROM github_connection_binding WHERE status = 'revoked' +); diff --git a/test/integration/provider-connection-migration.test.ts b/test/integration/provider-connection-migration.test.ts index c5a542c..90dfbfa 100644 --- a/test/integration/provider-connection-migration.test.ts +++ b/test/integration/provider-connection-migration.test.ts @@ -207,14 +207,14 @@ describe('Provider connection migration', () => { env.MIGRATION_DB.prepare( "SELECT revoked_at IS NOT NULL AS revoked FROM external_oauth_refresh WHERE provider_id = 'github'", ).first(), - ).resolves.toEqual({ revoked: 0 }) + ).resolves.toEqual({ revoked: 1 }) await expect( env.MIGRATION_DB.prepare( "SELECT COUNT(*) AS count FROM github_connection_binding WHERE status = 'active'", ).first(), - ).resolves.toEqual({ count: 2 }) + ).resolves.toEqual({ count: 0 }) await expect( env.MIGRATION_DB.prepare('SELECT COUNT(*) AS count FROM github_connection_context').first(), - ).resolves.toEqual({ count: 1 }) + ).resolves.toEqual({ count: 0 }) }) }) From 4d099721cfe3ce86ee1b60a6b547e1a7c58e8dde Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 17:31:35 -0400 Subject: [PATCH 4/5] Revert "fix(github): support cross-fork pull request creation" This reverts commit 894e076114d86500e5200ee1c4c98bb2245d1a38. --- .dev.vars.example | 1 - README.md | 15 +- docs/architecture.md | 9 -- migrations/0010_github_user_credentials.sql | 23 --- providers/github/README.md | 20 ++- specs/github-adapter.feature | 10 -- src/providers/github/adapter.ts | 91 +++--------- src/providers/github/client.ts | 63 +++------ src/providers/github/config.ts | 5 - src/providers/github/credentials.ts | 102 -------------- .../github/external-authorization.ts | 11 +- src/providers/github/graphql.ts | 2 +- src/providers/github/types.ts | 16 +-- src/worker.ts | 26 ++-- test/app.test.ts | 133 +----------------- .../github-user-credentials.test.ts | 49 ------- .../provider-connection-migration.test.ts | 39 ----- test/providers/github-client.test.ts | 38 +---- .../github-external-authorization.test.ts | 36 +---- worker-configuration.d.ts | 5 +- wrangler.jsonc | 1 - 21 files changed, 74 insertions(+), 621 deletions(-) delete mode 100644 migrations/0010_github_user_credentials.sql delete mode 100644 src/providers/github/credentials.ts delete mode 100644 test/integration/github-user-credentials.test.ts diff --git a/.dev.vars.example b/.dev.vars.example index 4b4b833..b099c51 100644 --- a/.dev.vars.example +++ b/.dev.vars.example @@ -8,7 +8,6 @@ GITHUB_APP_ID=replace-with-github-app-id GITHUB_PRIVATE_KEY=replace-with-pkcs1-or-pkcs8-private-key GITHUB_CLIENT_ID=replace-with-github-app-client-id GITHUB_CLIENT_SECRET=replace-with-github-app-client-secret -GITHUB_CREDENTIAL_ENCRYPTION_KEY=replace-with-base64-encoded-32-byte-key GITHUB_WEBHOOK_SECRET=replace-with-github-app-webhook-secret CLOUDFLARE_API_ORIGIN=https://api.cloudflare.com/client/v4 CLOUDFLARE_AUTHORIZATION_ORIGIN=https://dash.cloudflare.com diff --git a/README.md b/README.md index bbd035b..f5811dd 100644 --- a/README.md +++ b/README.md @@ -210,10 +210,9 @@ Resource Servers and never appear in the audience URL: } ``` -Set `GITHUB_APP_ID`, `GITHUB_PRIVATE_KEY`, `GITHUB_CLIENT_ID`, -`GITHUB_CLIENT_SECRET`, and a base64-encoded 32-byte -`GITHUB_CREDENTIAL_ENCRYPTION_KEY` in the ignored `.dev.vars` file. Both -GitHub-downloaded PKCS#1 keys and unencrypted PKCS#8 PEM keys are accepted. +Set `GITHUB_APP_ID`, `GITHUB_PRIVATE_KEY`, `GITHUB_CLIENT_ID`, and +`GITHUB_CLIENT_SECRET` in the ignored `.dev.vars` file. Both GitHub-downloaded +PKCS#1 keys and unencrypted PKCS#8 PEM keys are accepted. Configure the GitHub App callbacks as: @@ -252,7 +251,6 @@ pnpm exec wrangler secret put GITHUB_APP_ID pnpm exec wrangler secret put GITHUB_PRIVATE_KEY < github-app.private-key.pem pnpm exec wrangler secret put GITHUB_CLIENT_ID pnpm exec wrangler secret put GITHUB_CLIENT_SECRET -pnpm exec wrangler secret put GITHUB_CREDENTIAL_ENCRYPTION_KEY pnpm exec wrangler secret put GITHUB_WEBHOOK_SECRET ``` @@ -278,13 +276,6 @@ discovery publishes the subset GitHub documents for installation access tokens, preserving alternative permission sets as OR and each set's required permissions as AND. For every request, the adapter resolves the original method and path and mints only one least-privileged permission set satisfied by the Realmroot token. -Installation credentials remain the default for Git and API operations. A -pull-request creation uses the connected user's credential to resolve GitHub's -opaque repository ID. The actual write still uses the installation credential -for an installed target. A cross-fork write is the narrow exception: after -verifying that its head belongs to the selected installation account and -repository boundary, the adapter uses the encrypted delegated credential to -create the pull request against the external upstream. GitHub requires both `contents:write` and `workflows:write` when the Contents API writes under `.github/workflows`; the adapter enforces that condition from the diff --git a/docs/architecture.md b/docs/architecture.md index c6c6705..5ff9d2d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -98,15 +98,6 @@ connection identity, and the resource authorization record. Provider credentials never cross the Agent boundary. The Worker runtime uses Web Crypto and Fetch APIs without a Node process or filesystem. -Provider credential selection is operation-specific and least-privileged. -GitHub uses installation credentials for repository reads, ordinary writes, -Git transport, comments, and merges. Pull-request creation uses an encrypted -delegated-user credential to resolve GitHub's opaque target repository ID. The -write keeps installation authority for an installed target, and uses delegated -user authority only when GitHub requires it for an approved installed fork to -an external upstream; the Adapter verifies the head installation boundary -before that credential is selected. - ## Identity model Every operation records two identities: diff --git a/migrations/0010_github_user_credentials.sql b/migrations/0010_github_user_credentials.sql deleted file mode 100644 index 46c1dc3..0000000 --- a/migrations/0010_github_user_credentials.sql +++ /dev/null @@ -1,23 +0,0 @@ -CREATE TABLE github_user_credential ( - subject TEXT PRIMARY KEY NOT NULL, - access_token_ciphertext TEXT NOT NULL, - refresh_token_ciphertext TEXT, - access_token_expires_at INTEGER, - refresh_token_expires_at INTEGER, - credential_version INTEGER NOT NULL DEFAULT 1, - updated_at INTEGER NOT NULL -); - -UPDATE external_oauth_refresh -SET revoked_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER), - updated_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER) -WHERE provider_id = 'github' AND revoked_at IS NULL; - -UPDATE github_connection_binding -SET status = 'revoked', updated_at = CAST(unixepoch('subsecond') * 1000 AS INTEGER) -WHERE status = 'active'; - -DELETE FROM github_connection_context -WHERE broker_reference IN ( - SELECT broker_reference FROM github_connection_binding WHERE status = 'revoked' -); diff --git a/providers/github/README.md b/providers/github/README.md index e0685b1..c3b8341 100644 --- a/providers/github/README.md +++ b/providers/github/README.md @@ -37,10 +37,10 @@ tokens, not a Resource Server URL or caller-selected path parameter. - discover connected installations and selected repositories; - create an issue with Agent attribution. -The current executable slice supports installation repository discovery, -attributed issue and pull-request operations, cross-fork pull-request creation, -and webhook-driven installation lifecycle invalidation. Standard OAuth -revocation is implemented. +The current executable slice supports installation repository discovery, issue +creation, and webhook-driven installation lifecycle invalidation. Pull +requests, comments, reviews, and delegated user-token operations remain roadmap +work. Standard OAuth revocation is implemented. GitHub sends lifecycle deliveries to `/github/webhooks`. The adapter verifies `X-Hub-Signature-256`, durably deduplicates `X-GitHub-Delivery`, and updates @@ -51,14 +51,10 @@ future exchanges fail as soon as the Adapter observes removed authority. ## Initial credential modes - installation access token for App-attributed automation; -- encrypted GitHub App user access and refresh credentials for resolving the - target of a pull-request creation and for the cross-fork write that GitHub - cannot perform with the selected installation. - -The delegated-user credential is selected only after the Adapter verifies that -the pull-request head belongs to the approved installation account and -repository boundary. Provider credentials remain adapter-owned and are never -returned to Realmroot or the Agent. +- GitHub App user access token only while authorizing and verifying a + Connection; it is not returned to Realmroot or retained as Agent authority. + +Provider credentials remain adapter-owned and are never returned to the Agent. ## Acceptance outcome diff --git a/specs/github-adapter.feature b/specs/github-adapter.feature index 3f2c0df..4f96821 100644 --- a/specs/github-adapter.feature +++ b/specs/github-adapter.feature @@ -140,16 +140,6 @@ Feature: GitHub App adapter And unsupported createPullRequest, addComment, and mergePullRequest mutations use GitHub's installation-compatible REST operations And the GitHub credential is never returned - @journey:github-cross-fork-pull-request @entrypoint:http - Scenario: An Agent opens a pull request from an installed fork to an external upstream - Given the selected GitHub installation owns the pull request head fork - And the connected GitHub user can access the upstream repository - When GitHub CLI creates a pull request against that external upstream - Then the adapter verifies the head belongs to the selected installation - And creates the pull request with the connected user's delegated credential - And repository pushes continue to use a repository-constrained installation credential - And the delegated credential is encrypted at rest and never returned - @journey:github-git-transport @entrypoint:http Scenario: Native Git uses the GitHub installation through the adapter Given the Agent has approved repository contents authority diff --git a/src/providers/github/adapter.ts b/src/providers/github/adapter.ts index 40af11b..b7d76b8 100644 --- a/src/providers/github/adapter.ts +++ b/src/providers/github/adapter.ts @@ -7,7 +7,6 @@ import { GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE } from './authorization-d import { createGitHubProvider } from './client.js' import type { GitHubAdapterConfig } from './config.js' import type { GitHubAuthorizationContext, GitHubConnectionStore } from './connections.js' -import type { GitHubUserCredentialStore } from './credentials.js' import { createGitHubCommentWithRest, createGitHubPullRequestWithRest, @@ -24,7 +23,7 @@ import { githubOpenApi } from './openapi.js' import { resolveGitHubOperationPermissions } from './operation-permissions.js' import { permissionsToScopes, scopesToPermissions } from './permissions.js' import { transformGitHubRequest } from './transformers.js' -import type { GitHubConnectionProvider, GitHubProvider } from './types.js' +import type { GitHubProvider } from './types.js' import { handleGitHubWebhook } from './webhooks.js' export type GitHubAdapterDependencies = { @@ -33,8 +32,6 @@ export type GitHubAdapterDependencies = { agentInfo?: AgentInfoResolver provider?: GitHubProvider connections?: GitHubConnectionStore - connectionProvider?: GitHubConnectionProvider - userCredentials?: GitHubUserCredentialStore } export function createGitHubAdapter( @@ -134,25 +131,29 @@ export function createGitHubAdapter( [...requiredScopes], ]) } - const delegatedToken = await delegatedUserToken(principal.subject) + const lookupToken = await provider.installationToken({ + installationId: installation.installationId, + permissions: scopesToPermissions(new Set(['metadata:read']), available), + ...(installation.repositorySelection === 'selected' + ? { + repositories: installation.repositories.map( + (repository) => repository.fullName.split('/').at(-1) as string, + ), + } + : {}), + }) const nameWithOwner = await resolveGitHubRepositoryName({ provider, - token: delegatedToken, + token: lookupToken, apiOrigin: config.githubApiOrigin, repositoryId: createPullRequest.repositoryId, }) - const [baseOwner, baseRepository] = nameWithOwner.split('/') as [string, string] - const sameInstallation = baseOwner.toLowerCase() === installation.accountLogin.toLowerCase() - const createToken = sameInstallation - ? await provider.installationToken({ - installationId: installation.installationId, - permissions: scopesToPermissions(new Set(['pull_requests:write']), available), - ...repositoryRestriction( - installation, - repositoryTarget(`/repos/${nameWithOwner}`, installation) as string, - ), - }) - : delegatedCrossForkToken(installation, baseRepository, createPullRequest.headRefName, delegatedToken) + const repository = repositoryTarget(`/repos/${nameWithOwner}`, installation) + const createToken = await provider.installationToken({ + installationId: installation.installationId, + permissions: scopesToPermissions(new Set(['pull_requests:write']), available), + ...repositoryRestriction(installation, repository as string), + }) const response = await createGitHubPullRequestWithRest({ provider, token: createToken, @@ -160,14 +161,7 @@ export function createGitHubAdapter( nameWithOwner, pullRequest: createPullRequest, }) - await auditNative( - c, - principal, - installation, - 'graphql-create-pull-request-rest-compatibility', - response.status, - sameInstallation ? undefined : { type: 'github_user', id: principal.subject }, - ) + await auditNative(c, principal, installation, 'graphql-create-pull-request-rest-compatibility', response.status) return response } const addComment = parseGitHubAddComment(body) @@ -397,7 +391,6 @@ export function createGitHubAdapter( installation: GitHubAuthorizationContext, operation: string, status: number, - providerActor: { type: 'github_user'; id: string } | undefined = undefined, ) { await dependencies.audit({ event: 'provider.operation', @@ -406,54 +399,12 @@ export function createGitHubAdapter( operation, installationId: installation.installationId, originatingPrincipal: { issuer: principal.actor.issuer, subject: principal.actor.subject }, - providerActor: providerActor ?? { type: 'github_app', id: config.githubAppId ?? 'injected-test-provider' }, + providerActor: { type: 'github_app', id: config.githubAppId ?? 'injected-test-provider' }, identityLevel: 'provider-delegated', result: { status }, occurredAt: new Date().toISOString(), }) } - - async function delegatedUserToken(subject: string) { - if (!dependencies.userCredentials || !dependencies.connectionProvider) { - throw forbidden('GitHub delegated-user operations are not configured.') - } - const credential = await dependencies.userCredentials.credential(subject) - if (credential.expiresAt === null || credential.expiresAt > Date.now() + 30_000) return credential.accessToken - if ( - !credential.refreshToken || - (credential.refreshTokenExpiresAt !== null && credential.refreshTokenExpiresAt <= Date.now()) - ) { - throw forbidden('Reconnect the GitHub account before creating cross-account pull requests.') - } - const refreshed = await dependencies.connectionProvider.refreshUserToken(credential.refreshToken) - if (!(await dependencies.userCredentials.replace(credential, refreshed))) { - return (await dependencies.userCredentials.credential(subject)).accessToken - } - return refreshed.accessToken - } -} - -function delegatedCrossForkToken( - installation: GitHubAuthorizationContext, - baseRepository: string, - headRefName: string, - delegatedToken: string, -) { - const separator = headRefName.indexOf(':') - const headOwner = separator > 0 ? headRefName.slice(0, separator) : '' - if (headOwner.toLowerCase() !== installation.accountLogin.toLowerCase()) { - throw forbidden('The pull request head must belong to the selected GitHub installation account.') - } - if ( - installation.repositorySelection === 'selected' && - !installation.repositories.some( - (repository) => - repository.fullName.toLowerCase() === `${installation.accountLogin}/${baseRepository}`.toLowerCase(), - ) - ) { - throw forbidden('The pull request head repository is outside the selected GitHub installation authority.') - } - return delegatedToken } function gitTransportTarget(requestUrl: string, method: string, origin: string) { diff --git a/src/providers/github/client.ts b/src/providers/github/client.ts index d5c8623..bd785ee 100644 --- a/src/providers/github/client.ts +++ b/src/providers/github/client.ts @@ -8,24 +8,7 @@ const githubApiVersion = '2026-03-10' const userAgent = 'realmroot-adapters/0.1' const permissionsSchema = z.record(z.string(), z.enum(['read', 'write', 'admin'])) const installationTokenSchema = z.object({ token: z.string().min(1), expires_at: z.iso.datetime() }) -const oauthTokenSchema = z - .object({ - access_token: z.string().min(1), - expires_in: z.number().int().positive().optional(), - refresh_token: z.string().min(1).optional(), - refresh_token_expires_in: z.number().int().positive().optional(), - }) - .superRefine((value, context) => { - const expiring = value.expires_in !== undefined || value.refresh_token !== undefined - if ( - expiring && - (value.expires_in === undefined || - value.refresh_token === undefined || - value.refresh_token_expires_in === undefined) - ) { - context.addIssue({ code: 'custom', message: 'GitHub returned an incomplete expiring user credential.' }) - } - }) +const oauthTokenSchema = z.object({ access_token: z.string().min(1) }) const userSchema = z.object({ id: z.number().int().positive(), login: z.string().min(1), name: z.string().nullable() }) const userInstallationsSchema = z.object({ installations: z.array( @@ -92,8 +75,8 @@ export function createGitHubProvider(input: GitHubClientInput): GitHubProvider { return installationTokenSchema.parse(await response.json()).token }, - request(request, token, mode = 'api') { - return githubRequest(request, token, false, mode) + request(request, installationToken, mode = 'api') { + return githubRequest(request, installationToken, false, mode) }, } @@ -131,7 +114,6 @@ export function createGitHubConnectionProvider( ): GitHubConnectionProvider { const fetcher = input.fetcher ?? fetch const appJwt = createAppJwt(input) - const now = input.now ?? Date.now return { authorizationUrl(state) { @@ -142,10 +124,19 @@ export function createGitHubConnectionProvider( return url.toString() }, async exchangeUserCode(code) { - return tokenRequest({ code, redirect_uri: input.redirectUri }) - }, - async refreshUserToken(refreshToken) { - return tokenRequest({ grant_type: 'refresh_token', refresh_token: refreshToken }) + const response = await fetcher('https://github.com/login/oauth/access_token', { + method: 'POST', + headers: { accept: 'application/json', 'content-type': 'application/json', 'user-agent': userAgent }, + body: JSON.stringify({ + client_id: input.clientId, + client_secret: input.clientSecret, + code, + redirect_uri: input.redirectUri, + }), + signal: AbortSignal.timeout(10_000), + }) + if (!response.ok) throw failedDependency(`GitHub rejected OAuth authorization with ${response.status}.`) + return oauthTokenSchema.parse(await response.json()).access_token }, async getUser(token) { const response = await userRequest('/user', token) @@ -182,28 +173,6 @@ export function createGitHubConnectionProvider( }, } - async function tokenRequest(parameters: Record) { - const response = await fetcher('https://github.com/login/oauth/access_token', { - method: 'POST', - headers: { accept: 'application/json', 'content-type': 'application/json', 'user-agent': userAgent }, - body: JSON.stringify({ - client_id: input.clientId, - client_secret: input.clientSecret, - ...parameters, - }), - signal: AbortSignal.timeout(10_000), - }) - if (!response.ok) throw failedDependency(`GitHub rejected OAuth authorization with ${response.status}.`) - const token = oauthTokenSchema.parse(await response.json()) - return { - accessToken: token.access_token, - refreshToken: token.refresh_token ?? null, - expiresAt: token.expires_in === undefined ? null : now() + token.expires_in * 1000, - refreshTokenExpiresAt: - token.refresh_token_expires_in === undefined ? null : now() + token.refresh_token_expires_in * 1000, - } - } - async function listInstallationRepositories(token: string, installationId: number) { const repositories: Array<{ id: number; fullName: string }> = [] let page = 1 diff --git a/src/providers/github/config.ts b/src/providers/github/config.ts index 2cf9271..ff4d571 100644 --- a/src/providers/github/config.ts +++ b/src/providers/github/config.ts @@ -9,7 +9,6 @@ const githubEnvironmentSchema = z.object({ GITHUB_PRIVATE_KEY: z.string().trim().min(1).optional(), GITHUB_CLIENT_ID: z.string().trim().min(1).optional(), GITHUB_CLIENT_SECRET: z.string().trim().min(1).optional(), - GITHUB_CREDENTIAL_ENCRYPTION_KEY: z.string().trim().min(1).optional(), GITHUB_WEBHOOK_SECRET: z.string().min(32).optional(), }) @@ -21,7 +20,6 @@ export type GitHubAdapterConfig = AppConfig & { githubPrivateKey?: string githubClientId?: string githubClientSecret?: string - githubCredentialEncryptionKey?: string githubWebhookSecret?: string } @@ -36,9 +34,6 @@ export function loadGitHubConfig(environment: unknown, config: AppConfig): GitHu ...(parsed.GITHUB_PRIVATE_KEY ? { githubPrivateKey: parsed.GITHUB_PRIVATE_KEY } : {}), ...(parsed.GITHUB_CLIENT_ID ? { githubClientId: parsed.GITHUB_CLIENT_ID } : {}), ...(parsed.GITHUB_CLIENT_SECRET ? { githubClientSecret: parsed.GITHUB_CLIENT_SECRET } : {}), - ...(parsed.GITHUB_CREDENTIAL_ENCRYPTION_KEY - ? { githubCredentialEncryptionKey: parsed.GITHUB_CREDENTIAL_ENCRYPTION_KEY } - : {}), ...(parsed.GITHUB_WEBHOOK_SECRET ? { githubWebhookSecret: parsed.GITHUB_WEBHOOK_SECRET } : {}), } } diff --git a/src/providers/github/credentials.ts b/src/providers/github/credentials.ts deleted file mode 100644 index 3cfadc7..0000000 --- a/src/providers/github/credentials.ts +++ /dev/null @@ -1,102 +0,0 @@ -import type { CredentialCipher } from '../../core/credential-cipher.js' -import { forbidden } from '../../core/problem.js' -import type { GitHubUserCredential, GitHubUserToken } from './types.js' - -export interface GitHubUserCredentialStore { - upsert(subject: string, token: GitHubUserToken): Promise - credential(subject: string): Promise - replace(credential: GitHubUserCredential, token: GitHubUserToken): Promise - revoke(subject: string): Promise -} - -export class D1GitHubUserCredentials implements GitHubUserCredentialStore { - constructor( - private readonly db: D1Database, - private readonly cipher: CredentialCipher, - ) {} - - async upsert(subject: string, token: GitHubUserToken) { - const context = credentialContext(subject) - const [accessToken, refreshToken] = await Promise.all([ - this.cipher.seal(token.accessToken, `${context}:access`), - token.refreshToken ? this.cipher.seal(token.refreshToken, `${context}:refresh`) : null, - ]) - await this.db - .prepare( - `INSERT INTO github_user_credential - (subject, access_token_ciphertext, refresh_token_ciphertext, access_token_expires_at, - refresh_token_expires_at, credential_version, updated_at) - VALUES (?, ?, ?, ?, ?, 1, ?) - ON CONFLICT(subject) DO UPDATE SET - access_token_ciphertext = excluded.access_token_ciphertext, - refresh_token_ciphertext = excluded.refresh_token_ciphertext, - access_token_expires_at = excluded.access_token_expires_at, - refresh_token_expires_at = excluded.refresh_token_expires_at, - credential_version = github_user_credential.credential_version + 1, - updated_at = excluded.updated_at`, - ) - .bind(subject, accessToken, refreshToken, token.expiresAt, token.refreshTokenExpiresAt, Date.now()) - .run() - } - - async credential(subject: string): Promise { - const row = await this.db - .prepare( - `SELECT subject, access_token_ciphertext AS accessToken, - refresh_token_ciphertext AS refreshToken, access_token_expires_at AS expiresAt, - refresh_token_expires_at AS refreshTokenExpiresAt, - credential_version AS credentialVersion - FROM github_user_credential WHERE subject = ?`, - ) - .bind(subject) - .first<{ - subject: string - accessToken: string - refreshToken: string | null - expiresAt: number | null - refreshTokenExpiresAt: number | null - credentialVersion: number - }>() - if (!row) throw forbidden('Reconnect the GitHub account before creating cross-account pull requests.') - const context = credentialContext(subject) - const [accessToken, refreshToken] = await Promise.all([ - this.cipher.open(row.accessToken, `${context}:access`), - row.refreshToken ? this.cipher.open(row.refreshToken, `${context}:refresh`) : null, - ]) - return { ...row, accessToken, refreshToken } - } - - async replace(credential: GitHubUserCredential, token: GitHubUserToken) { - const context = credentialContext(credential.subject) - const [accessToken, refreshToken] = await Promise.all([ - this.cipher.seal(token.accessToken, `${context}:access`), - token.refreshToken ? this.cipher.seal(token.refreshToken, `${context}:refresh`) : null, - ]) - const result = await this.db - .prepare( - `UPDATE github_user_credential SET access_token_ciphertext = ?, refresh_token_ciphertext = ?, - access_token_expires_at = ?, refresh_token_expires_at = ?, - credential_version = credential_version + 1, updated_at = ? - WHERE subject = ? AND credential_version = ?`, - ) - .bind( - accessToken, - refreshToken, - token.expiresAt, - token.refreshTokenExpiresAt, - Date.now(), - credential.subject, - credential.credentialVersion, - ) - .run() - return result.meta.changes === 1 - } - - async revoke(subject: string) { - await this.db.prepare('DELETE FROM github_user_credential WHERE subject = ?').bind(subject).run() - } -} - -function credentialContext(subject: string) { - return `github:${subject}:delegated-user` -} diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index 40c5615..d21c7ff 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -9,7 +9,6 @@ import { githubInstallationAuthorizationDetailDisplay, } from './authorization-details.js' import type { D1GitHubConnections } from './connections.js' -import type { GitHubUserCredentialStore } from './credentials.js' import { permissionsToScopes } from './permissions.js' import type { GitHubConnectionProvider } from './types.js' @@ -19,7 +18,6 @@ export function createGitHubExternalAuthorization(input: { origin: string connection: GitHubConnectionProvider connections: D1GitHubConnections - credentials: GitHubUserCredentialStore oauthStore: D1ExternalOAuthStore scopes: readonly string[] }): { authorization: ExternalProviderAuthorization; installationCallback: AdapterModule } { @@ -137,8 +135,8 @@ export function createGitHubExternalAuthorization(input: { }) ) }, - async revoke(subject) { - await Promise.all([input.connections.revokeExternalAuthorization(subject), input.credentials.revoke(subject)]) + revoke(subject) { + return input.connections.revokeExternalAuthorization(subject) }, begin({ providerState }) { return { url: input.connection.authorizationUrl(providerState), stage: 'oauth' } @@ -150,15 +148,14 @@ export function createGitHubExternalAuthorization(input: { const callback = new URL(callbackUrl) const userToken = await input.connection.exchangeUserCode(required(callback.searchParams.get('code'), 'code')) const [user, installations] = await Promise.all([ - input.connection.getUser(userToken.accessToken), - input.connection.listUserInstallations(userToken.accessToken), + input.connection.getUser(userToken), + input.connection.listUserInstallations(userToken), ]) 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.') } - await input.credentials.upsert(String(user.id), userToken) if (installations.length === 0) { const providerState = nextProviderState() return { diff --git a/src/providers/github/graphql.ts b/src/providers/github/graphql.ts index 4eb551c..f30d044 100644 --- a/src/providers/github/graphql.ts +++ b/src/providers/github/graphql.ts @@ -18,7 +18,7 @@ const createInputSchema = z.object({ maintainerCanModify: z.boolean().optional(), }) const repositoryLookupSchema = z.object({ - data: z.object({ node: z.object({ nameWithOwner: z.string().regex(/^[^/]+\/[^/]+$/) }) }), + data: z.object({ node: z.object({ nameWithOwner: z.string().min(3) }) }), }) const pullRequestSchema = z.object({ node_id: z.string().min(1), html_url: z.url() }) const mergeInputSchema = z.object({ diff --git a/src/providers/github/types.ts b/src/providers/github/types.ts index 8151ba3..1746880 100644 --- a/src/providers/github/types.ts +++ b/src/providers/github/types.ts @@ -11,21 +11,10 @@ export interface GitHubProvider { appPermissions(): Promise openApiDocument(): Promise installationToken(input: GitHubInstallationTokenRequest): Promise - request(request: Request, token: string, mode?: 'api' | 'git'): Promise + request(request: Request, installationToken: string, mode?: 'api' | 'git'): Promise } export type GitHubUser = Readonly<{ id: number; login: string; name: string | null }> -export type GitHubUserToken = Readonly<{ - accessToken: string - refreshToken: string | null - expiresAt: number | null - refreshTokenExpiresAt: number | null -}> -export type GitHubUserCredential = GitHubUserToken & - Readonly<{ - subject: string - credentialVersion: number - }> export type GitHubInstallation = Readonly<{ id: number htmlUrl: string @@ -41,8 +30,7 @@ export type GitHubRepository = Readonly<{ id: number; fullName: string }> export interface GitHubConnectionProvider { authorizationUrl(state: string): string - exchangeUserCode(code: string): Promise - refreshUserToken(refreshToken: string): Promise + exchangeUserCode(code: string): Promise getUser(token: string): Promise listUserInstallations(token: string): Promise newInstallationUrl(state: string): Promise diff --git a/src/worker.ts b/src/worker.ts index 017dd0e..ef58cda 100644 --- a/src/worker.ts +++ b/src/worker.ts @@ -16,7 +16,6 @@ import { createGitHubAdapter } from './providers/github/adapter.js' import { createGitHubConnectionProvider, createGitHubProvider } from './providers/github/client.js' import { loadGitHubConfig } from './providers/github/config.js' import { D1GitHubConnections } from './providers/github/connections.js' -import { D1GitHubUserCredentials } from './providers/github/credentials.js' import { createGitHubExternalAuthorization } from './providers/github/external-authorization.js' import { permissionsToScopes } from './providers/github/permissions.js' import { createLinearAdapter } from './providers/linear/adapter.js' @@ -44,32 +43,25 @@ export default { githubConfig.githubPrivateKey && githubConfig.githubClientId && githubConfig.githubClientSecret && - githubConfig.githubCredentialEncryptionKey && signingPrivateJwk ) { const githubConnections = new D1GitHubConnections(env.DB, state) - const githubUserCredentials = new D1GitHubUserCredentials( - env.DB, - createCredentialCipher(githubConfig.githubCredentialEncryptionKey), - ) const githubProvider = createGitHubProvider({ appId: githubConfig.githubAppId, privateKey: githubConfig.githubPrivateKey, apiOrigin: githubConfig.githubApiOrigin, }) - const githubConnectionProvider = createGitHubConnectionProvider({ - appId: githubConfig.githubAppId, - privateKey: githubConfig.githubPrivateKey, - clientId: githubConfig.githubClientId, - clientSecret: githubConfig.githubClientSecret, - redirectUri: `${config.origin}/github/oauth/callback`, - apiOrigin: githubConfig.githubApiOrigin, - }) const githubExternal = createGitHubExternalAuthorization({ origin: config.origin, - connection: githubConnectionProvider, + connection: createGitHubConnectionProvider({ + appId: githubConfig.githubAppId, + privateKey: githubConfig.githubPrivateKey, + clientId: githubConfig.githubClientId, + clientSecret: githubConfig.githubClientSecret, + redirectUri: `${config.origin}/github/oauth/callback`, + apiOrigin: githubConfig.githubApiOrigin, + }), connections: githubConnections, - credentials: githubUserCredentials, oauthStore, scopes: permissionsToScopes(await githubProvider.appPermissions()), }) @@ -89,8 +81,6 @@ export default { provider: githubProvider, audit: (record) => state.recordAudit(record), connections: githubConnections, - connectionProvider: githubConnectionProvider, - userCredentials: githubUserCredentials, }), ) } diff --git a/test/app.test.ts b/test/app.test.ts index 821a0a3..f7bd701 100644 --- a/test/app.test.ts +++ b/test/app.test.ts @@ -140,7 +140,7 @@ describe('GitHub adapter contract', () => { .fn() .mockImplementationOnce(async (request: Request, token: string) => { expect(request.url).toBe('https://api.github.com/graphql') - expect(token).toBe('delegated-user-token') + expect(token).toBe('installation-secret') await expect(request.json()).resolves.toMatchObject({ variables: { id: 'repository-1' } }) return Response.json({ data: { node: { nameWithOwner: 'realmroot/example' } } }) }) @@ -189,96 +189,14 @@ describe('GitHub adapter contract', () => { await expect(response.json()).resolves.toMatchObject({ data: { createPullRequest: { pullRequest: { url: 'https://github.test/pull/1' } } }, }) - expect(provider.installationToken).toHaveBeenCalledTimes(1) - expect(provider.installationToken).toHaveBeenCalledWith({ + expect(provider.installationToken).toHaveBeenNthCalledWith(1, { installationId: 42, - permissions: { pull_requests: 'write' }, + permissions: { metadata: 'read' }, }) - }) - - it('[spec: github-adapter/github-cross-fork-pull-request] creates a cross-fork pull request as the connected user', async () => { - const provider = fakeProvider() - provider.request = vi - .fn() - .mockImplementationOnce(async (request: Request, token: string) => { - expect(request.url).toBe('https://api.github.com/graphql') - expect(token).toBe('delegated-user-token') - return Response.json({ data: { node: { nameWithOwner: 'upstream/example' } } }) - }) - .mockImplementationOnce(async (request: Request, token: string) => { - expect(request.url).toBe('https://api.github.com/repos/upstream/example/pulls') - expect(token).toBe('delegated-user-token') - await expect(request.json()).resolves.toMatchObject({ head: 'realmroot:codex/fix', base: 'main' }) - return Response.json({ node_id: 'pull-request-2', html_url: 'https://github.test/pull/2' }, { status: 201 }) - }) - const audit = vi.fn(async () => {}) - const response = await testApp({ - provider, - audit, - authenticator: { - authenticate: vi.fn(async () => ({ - ...principal, - scopes: new Set(['metadata:read', 'pull_requests:write']), - })), - }, - }).request('/github/graphql', { - method: 'POST', - headers: { 'Content-Type': 'application/json; charset=utf-8' }, - body: JSON.stringify({ - query: - 'mutation PullRequestCreate($input: CreatePullRequestInput!) { createPullRequest(input: $input) { pullRequest { id url } } }', - variables: { - input: { - repositoryId: 'upstream-repository', - baseRefName: 'main', - headRefName: 'realmroot:codex/fix', - title: 'Fix adapter', - }, - }, - }), + expect(provider.installationToken).toHaveBeenNthCalledWith(2, { + installationId: 42, + permissions: { pull_requests: 'write' }, }) - - expect(response.status).toBe(200) - expect(provider.installationToken).not.toHaveBeenCalled() - expect(audit).toHaveBeenCalledWith( - expect.objectContaining({ providerActor: { type: 'github_user', id: principal.subject } }), - ) - }) - - it('[spec: github-adapter/github-cross-fork-pull-request] rejects a head outside the selected installation boundary', async () => { - const provider = fakeProvider() - provider.request = vi.fn(async () => Response.json({ data: { node: { nameWithOwner: 'upstream/example' } } })) - const authenticate = vi.fn(async () => ({ - ...principal, - scopes: new Set(['metadata:read', 'pull_requests:write']), - })) - - const wrongOwner = await testApp({ provider, authenticator: { authenticate } }).request( - '/github/graphql', - createPullRequestRequest('attacker:codex/fix'), - ) - expect(wrongOwner.status).toBe(403) - - const selectedRepositoryConnection = fakeConnections() - selectedRepositoryConnection.externalAuthorization = vi.fn(async () => - authorizationWithContexts([ - { - installationId: 42, - accountLogin: 'realmroot', - targetType: 'Organization', - scopes: ['metadata:read', 'pull_requests:write'], - repositorySelection: 'selected', - repositories: [{ id: 1, fullName: 'realmroot/another-repository' }], - }, - ]), - ) - const unselectedRepository = await testApp({ - provider, - connections: selectedRepositoryConnection, - authenticator: { authenticate }, - }).request('/github/graphql', createPullRequestRequest('realmroot:codex/fix')) - expect(unselectedRepository.status).toBe(403) - expect(provider.request).toHaveBeenCalledTimes(2) }) it('[spec: github-adapter/github-graphql-proxy] preserves the GitHub CLI addComment mutation', async () => { @@ -723,26 +641,6 @@ function testApp( }, provider: fakeProvider(), connections: fakeConnections(), - connectionProvider: { - authorizationUrl: vi.fn(), - exchangeUserCode: vi.fn(), - refreshUserToken: vi.fn(), - getUser: vi.fn(), - listUserInstallations: vi.fn(), - newInstallationUrl: vi.fn(), - permissionUpdateUrl: vi.fn(), - }, - userCredentials: { - credential: vi.fn(async () => ({ - subject: principal.subject, - accessToken: 'delegated-user-token', - refreshToken: 'delegated-refresh-token', - expiresAt: Date.now() + 60_000, - refreshTokenExpiresAt: Date.now() + 120_000, - credentialVersion: 1, - })), - replace: vi.fn(), - } as never, audit: vi.fn(async () => {}), ...overrides, } @@ -872,22 +770,3 @@ function authorizationWithContexts( contexts, } } - -function createPullRequestRequest(headRefName: string) { - return { - method: 'POST', - headers: { 'Content-Type': 'application/json; charset=utf-8' }, - body: JSON.stringify({ - query: - 'mutation PullRequestCreate($input: CreatePullRequestInput!) { createPullRequest(input: $input) { pullRequest { id url } } }', - variables: { - input: { - repositoryId: 'upstream-repository', - baseRefName: 'main', - headRefName, - title: 'Fix adapter', - }, - }, - }), - } -} diff --git a/test/integration/github-user-credentials.test.ts b/test/integration/github-user-credentials.test.ts deleted file mode 100644 index 44ca562..0000000 --- a/test/integration/github-user-credentials.test.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { env } from 'cloudflare:test' -import { describe, expect, it } from 'vitest' -import { createCredentialCipher } from '../../src/core/credential-cipher.js' -import { D1GitHubUserCredentials } from '../../src/providers/github/credentials.js' - -describe('GitHub delegated-user credential persistence', () => { - it('[spec: github-adapter/github-cross-fork-pull-request] encrypts, rotates, and revokes the provider credential', async () => { - const store = new D1GitHubUserCredentials( - env.DB, - createCredentialCipher('AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'), - ) - await store.upsert('70', { - accessToken: 'user-token', - refreshToken: 'refresh-token', - expiresAt: 1_800_000_000_000, - refreshTokenExpiresAt: 1_900_000_000_000, - }) - - const stored = await env.DB.prepare( - 'SELECT access_token_ciphertext AS accessToken, refresh_token_ciphertext AS refreshToken FROM github_user_credential WHERE subject = ?', - ) - .bind('70') - .first<{ accessToken: string; refreshToken: string }>() - expect(stored?.accessToken).not.toContain('user-token') - expect(stored?.refreshToken).not.toContain('refresh-token') - - const credential = await store.credential('70') - expect(credential).toMatchObject({ accessToken: 'user-token', refreshToken: 'refresh-token', credentialVersion: 1 }) - await expect( - store.replace(credential, { - accessToken: 'fresh-user-token', - refreshToken: 'fresh-refresh-token', - expiresAt: 1_810_000_000_000, - refreshTokenExpiresAt: 1_910_000_000_000, - }), - ).resolves.toBe(true) - await expect(store.replace(credential, credential)).resolves.toBe(false) - await expect(store.credential('70')).resolves.toMatchObject({ - accessToken: 'fresh-user-token', - refreshToken: 'fresh-refresh-token', - credentialVersion: 2, - }) - - await store.revoke('70') - await expect(store.credential('70')).rejects.toThrow( - 'Reconnect the GitHub account before creating cross-account pull requests.', - ) - }) -}) diff --git a/test/integration/provider-connection-migration.test.ts b/test/integration/provider-connection-migration.test.ts index 90dfbfa..e2b8346 100644 --- a/test/integration/provider-connection-migration.test.ts +++ b/test/integration/provider-connection-migration.test.ts @@ -10,8 +10,6 @@ describe('Provider connection migration', () => { const linearConnections = env.TEST_MIGRATIONS.slice(5, 6) const webhookLifecycle = env.TEST_MIGRATIONS.slice(6, 7) const externalAuthorizationServer = env.TEST_MIGRATIONS.slice(7, 8) - const linearAuthorizationCleanup = env.TEST_MIGRATIONS.slice(8, 9) - const githubDelegatedCredentials = env.TEST_MIGRATIONS.slice(9, 10) expect(legacy).toHaveLength(2) expect(lifecycle).toHaveLength(1) expect(installationOwnership).toHaveLength(1) @@ -19,8 +17,6 @@ describe('Provider connection migration', () => { expect(linearConnections).toHaveLength(1) expect(webhookLifecycle).toHaveLength(1) expect(externalAuthorizationServer).toHaveLength(1) - expect(linearAuthorizationCleanup).toHaveLength(1) - expect(githubDelegatedCredentials).toHaveLength(1) await applyD1Migrations(env.MIGRATION_DB, legacy) const now = Date.now() await env.MIGRATION_DB.batch([ @@ -181,40 +177,5 @@ describe('Provider connection migration', () => { "SELECT COUNT(*) AS count FROM sqlite_master WHERE type = 'table' AND name IN ('external_oauth_client', 'external_oauth_intent', 'external_oauth_code', 'external_oauth_refresh', 'external_oauth_access', 'cloudflare_external_credential')", ).first(), ).resolves.toEqual({ count: 6 }) - - await env.MIGRATION_DB.batch([ - env.MIGRATION_DB.prepare( - `INSERT INTO external_oauth_client - (client_id, provider_id, client_secret_hash, redirect_uris_json, jwks_uri, created_at) - VALUES (?, 'github', ?, '[]', ?, ?)`, - ).bind('github-client', 'hash', 'https://id.example/jwks', now), - env.MIGRATION_DB.prepare( - `INSERT INTO external_oauth_refresh - (token_hash, provider_id, client_id, subject, display_name, scope_json, - authorization_details_json, created_at, updated_at) - VALUES (?, 'github', ?, ?, ?, '[]', '[]', ?, ?)`, - ).bind('refresh-hash', 'github-client', '7', 'Controller', now, now), - ]) - await applyD1Migrations(env.MIGRATION_DB, linearAuthorizationCleanup) - await applyD1Migrations(env.MIGRATION_DB, githubDelegatedCredentials) - - await expect( - env.MIGRATION_DB.prepare( - "SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'github_user_credential'", - ).first(), - ).resolves.toEqual({ name: 'github_user_credential' }) - await expect( - env.MIGRATION_DB.prepare( - "SELECT revoked_at IS NOT NULL AS revoked FROM external_oauth_refresh WHERE provider_id = 'github'", - ).first(), - ).resolves.toEqual({ revoked: 1 }) - await expect( - env.MIGRATION_DB.prepare( - "SELECT COUNT(*) AS count FROM github_connection_binding WHERE status = 'active'", - ).first(), - ).resolves.toEqual({ count: 0 }) - await expect( - env.MIGRATION_DB.prepare('SELECT COUNT(*) AS count FROM github_connection_context').first(), - ).resolves.toEqual({ count: 0 }) }) }) diff --git a/test/providers/github-client.test.ts b/test/providers/github-client.test.ts index 4288e6e..e0ef60c 100644 --- a/test/providers/github-client.test.ts +++ b/test/providers/github-client.test.ts @@ -93,12 +93,7 @@ describe('GitHub account connection OAuth boundary', () => { const authorization = new URL(provider.authorizationUrl('provider-state')) expect(authorization.searchParams.get('redirect_uri')).toBe('https://adapters.realmroot.dev/github/oauth/callback') - await expect(provider.exchangeUserCode('authorization-code')).resolves.toEqual({ - accessToken: 'user-token', - refreshToken: null, - expiresAt: null, - refreshTokenExpiresAt: null, - }) + await expect(provider.exchangeUserCode('authorization-code')).resolves.toBe('user-token') expect(requests[0]).toEqual({ url: 'https://github.com/login/oauth/access_token', body: { @@ -113,37 +108,6 @@ describe('GitHub account connection OAuth boundary', () => { ) }) - it('refreshes an expiring delegated-user credential with the GitHub App client', async () => { - 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', - now: () => 1_800_000_000_000, - fetcher: async (_input, init) => { - expect(JSON.parse(String(init?.body))).toMatchObject({ - grant_type: 'refresh_token', - refresh_token: 'old-refresh-token', - }) - return Response.json({ - access_token: 'fresh-user-token', - expires_in: 28_800, - refresh_token: 'fresh-refresh-token', - refresh_token_expires_in: 15_897_600, - }) - }, - }) - - await expect(provider.refreshUserToken('old-refresh-token')).resolves.toEqual({ - accessToken: 'fresh-user-token', - refreshToken: 'fresh-refresh-token', - expiresAt: 1_800_028_800_000, - refreshTokenExpiresAt: 1_815_897_600_000, - }) - }) - it('loads selected repository membership with the installation context', async () => { const requests: string[] = [] const provider = createGitHubConnectionProvider({ diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index 2c50abc..1ae68da 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -3,7 +3,6 @@ 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' import type { D1GitHubConnections } from '../../src/providers/github/connections.js' -import type { GitHubUserCredentialStore } from '../../src/providers/github/credentials.js' import { createGitHubExternalAuthorization } from '../../src/providers/github/external-authorization.js' import type { GitHubInstallation } from '../../src/providers/github/types.js' @@ -25,7 +24,6 @@ describe('GitHub external authorization', () => { connections: { externalAuthorization: vi.fn(async () => ({ scopes: ['metadata:read'], contexts })), } as unknown as D1GitHubConnections, - credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['metadata:read'], }) @@ -86,7 +84,6 @@ describe('GitHub external authorization', () => { contexts, })), } as unknown as D1GitHubConnections, - credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['administration:write', 'metadata:read'], }) @@ -123,7 +120,6 @@ describe('GitHub external authorization', () => { ], })), } as unknown as D1GitHubConnections, - credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['actions:read', 'metadata:read'], }) @@ -164,15 +160,13 @@ describe('GitHub external authorization', () => { origin: 'https://adapter.example', connection: { authorizationUrl: vi.fn(() => 'https://github.com/login/oauth/authorize'), - exchangeUserCode: vi.fn(async () => userToken()), - refreshUserToken: vi.fn(async () => userToken()), + exchangeUserCode: vi.fn(async () => 'user-token'), 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, - credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['metadata:read', 'pull_requests:read', 'pull_requests:write'], }) @@ -204,7 +198,6 @@ describe('GitHub external authorization', () => { connections: { upsertExternalAuthorization: vi.fn(async () => [connectionContext(selected), connectionContext(other)]), } as unknown as D1GitHubConnections, - credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['administration:read'], }) @@ -242,15 +235,10 @@ describe('GitHub external authorization', () => { const installation = githubInstallation({ administration: 'read' }) const connections = { upsertExternalAuthorization: vi.fn() } const connection = githubConnection([installation]) - const upsertCredential = vi.fn() const external = createGitHubExternalAuthorization({ origin: 'https://adapter.example', connection, connections: connections as unknown as D1GitHubConnections, - credentials: { - upsert: upsertCredential, - revoke: vi.fn(), - } as unknown as GitHubUserCredentialStore, oauthStore: {} as D1ExternalOAuthStore, scopes: ['administration:read', 'administration:write'], }) @@ -279,7 +267,6 @@ describe('GitHub external authorization', () => { }) expect(connection.newInstallationUrl).not.toHaveBeenCalled() expect(connections.upsertExternalAuthorization).not.toHaveBeenCalled() - expect(upsertCredential).toHaveBeenCalledWith('70', expect.objectContaining({ accessToken: 'user-token' })) }) it('[spec: github-adapter/github-installation-permission-upgrade] resumes only after the target installation accepts the permission', async () => { @@ -303,7 +290,6 @@ describe('GitHub external authorization', () => { origin: 'https://adapter.example', connection: githubConnection([before]), connections: { externalAuthorization } as unknown as D1GitHubConnections, - credentials: githubCredentials(), oauthStore: {} as D1ExternalOAuthStore, scopes: ['administration:read', 'administration:write'], }) @@ -357,7 +343,6 @@ describe('GitHub external authorization', () => { origin: 'https://adapter.example', connection, connections: {} as D1GitHubConnections, - credentials: githubCredentials(), oauthStore: oauthStore as unknown as D1ExternalOAuthStore, scopes: ['administration:write'], }) @@ -390,8 +375,7 @@ function githubInstallation(permissions: Record) { function githubConnection(installations: ReturnType[]) { return { authorizationUrl: vi.fn((state: string) => `https://github.com/login/oauth/authorize?state=${state}`), - exchangeUserCode: vi.fn(async () => userToken()), - refreshUserToken: vi.fn(async () => userToken()), + exchangeUserCode: vi.fn(async () => 'user-token'), getUser: vi.fn(async () => ({ id: 70, login: 'controller', name: 'Controller' })), listUserInstallations: vi.fn(async () => installations), newInstallationUrl: vi.fn( @@ -403,22 +387,6 @@ function githubConnection(installations: ReturnType[] } } -function githubCredentials() { - return { - upsert: vi.fn(), - revoke: vi.fn(), - } as unknown as GitHubUserCredentialStore -} - -function userToken() { - return { - accessToken: 'user-token', - refreshToken: 'refresh-token', - expiresAt: Date.now() + 60_000, - refreshTokenExpiresAt: Date.now() + 120_000, - } -} - function connectionContext(installation: ReturnType) { const permissions = installation.permissions as Record return { diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 2de96e0..9c0a40c 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,5 +1,5 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types --env-file=.dev.vars.example --strict-vars=false` (hash: 070d295b2af2928fdff0fd9e0a61e120) +// Generated by Wrangler by running `wrangler types --env-file=.dev.vars.example --strict-vars=false` (hash: b7a9d5af36095ca8d444ae9520538651) // Runtime types generated with workerd@1.20260801.1 2026-08-08 nodejs_compat interface __BaseEnv_Env { DB: D1Database; @@ -16,7 +16,6 @@ interface __BaseEnv_Env { GITHUB_PRIVATE_KEY: string; GITHUB_CLIENT_ID: string; GITHUB_CLIENT_SECRET: string; - GITHUB_CREDENTIAL_ENCRYPTION_KEY: string; GITHUB_WEBHOOK_SECRET: string; LINEAR_CLIENT_ID: string; LINEAR_CLIENT_SECRET: string; @@ -38,7 +37,7 @@ type StringifyValues> = { [Binding in keyof EnvType]: EnvType[Binding] extends string ? EnvType[Binding] : string; }; declare namespace NodeJS { - interface ProcessEnv extends StringifyValues> {} + interface ProcessEnv extends StringifyValues> {} } // Begin runtime types diff --git a/wrangler.jsonc b/wrangler.jsonc index f65cbc6..2cf5def 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -25,7 +25,6 @@ "GITHUB_PRIVATE_KEY", "GITHUB_CLIENT_ID", "GITHUB_CLIENT_SECRET", - "GITHUB_CREDENTIAL_ENCRYPTION_KEY", "GITHUB_WEBHOOK_SECRET", "LINEAR_CLIENT_ID", "LINEAR_CLIENT_SECRET", From 1b4134369a364d836f8db9e5fe6d5aa2faf00109 Mon Sep 17 00:00:00 2001 From: jarvis Date: Fri, 14 Aug 2026 17:33:00 -0400 Subject: [PATCH 5/5] fix(github): enforce installation boundary --- specs/github-adapter.feature | 8 ++++++++ src/providers/github/adapter.ts | 2 +- test/app.test.ts | 35 +++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/specs/github-adapter.feature b/specs/github-adapter.feature index 4f96821..79b50b0 100644 --- a/specs/github-adapter.feature +++ b/specs/github-adapter.feature @@ -140,6 +140,14 @@ Feature: GitHub App adapter And unsupported createPullRequest, addComment, and mergePullRequest mutations use GitHub's installation-compatible REST operations And the GitHub credential is never returned + @journey:github-cross-installation-boundary @entrypoint:http + Scenario: GitHub operations stay inside one selected App installation + Given an Agent selected a GitHub App installation + When GitHub CLI tries to create a pull request in a repository outside that installation + Then the adapter rejects the operation before requesting a write credential + And explains that the target repository must belong to the selected App installation + But it does not request broader user credentials or another OAuth application + @journey:github-git-transport @entrypoint:http Scenario: Native Git uses the GitHub installation through the adapter Given the Agent has approved repository contents authority diff --git a/src/providers/github/adapter.ts b/src/providers/github/adapter.ts index b7d76b8..f6937e4 100644 --- a/src/providers/github/adapter.ts +++ b/src/providers/github/adapter.ts @@ -475,7 +475,7 @@ function repositoryTarget(path: string, installation: GitHubAuthorizationContext if (!match) return const owner = decodeURIComponent(match[1] as string) if (owner.toLowerCase() !== installation.accountLogin.toLowerCase()) { - throw forbidden('The repository owner is outside the selected GitHub installation.') + throw forbidden('The target repository must belong to the selected GitHub App installation.') } const repository = decodeURIComponent(match[2] as string) if ( diff --git a/test/app.test.ts b/test/app.test.ts index f7bd701..7444cd2 100644 --- a/test/app.test.ts +++ b/test/app.test.ts @@ -199,6 +199,41 @@ describe('GitHub adapter contract', () => { }) }) + it('[spec: github-adapter/github-cross-installation-boundary] rejects pull requests outside the selected installation', async () => { + const provider = fakeProvider() + provider.request = vi.fn(async () => Response.json({ data: { node: { nameWithOwner: 'upstream/example' } } })) + const response = await testApp({ + provider, + authenticator: { + authenticate: vi.fn(async () => ({ + ...principal, + scopes: new Set(['metadata:read', 'pull_requests:write']), + })), + }, + }).request('/github/graphql', { + method: 'POST', + headers: { 'Content-Type': 'application/json; charset=utf-8' }, + body: JSON.stringify({ + query: + 'mutation PullRequestCreate($input: CreatePullRequestInput!) { createPullRequest(input: $input) { pullRequest { id url } } }', + variables: { + input: { + repositoryId: 'upstream-repository', + baseRefName: 'main', + headRefName: 'realmroot:codex/fix', + title: 'Fix adapter', + }, + }, + }), + }) + + expect(response.status).toBe(403) + await expect(response.json()).resolves.toMatchObject({ + detail: 'The target repository must belong to the selected GitHub App installation.', + }) + expect(provider.installationToken).toHaveBeenCalledTimes(1) + }) + it('[spec: github-adapter/github-graphql-proxy] preserves the GitHub CLI addComment mutation', async () => { const provider = fakeProvider() provider.request = vi