From 7fb0a0e5269fdea32fbcaca3b91ac6fad00cc3a0 Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Sat, 11 Apr 2026 17:49:27 -0400 Subject: [PATCH] Harden GitHub auth context and cancel abandoned requests - Catch uncaught token refresh errors in getGitHubAppUserInstallations and getGitHubUserContextForOwner so a failed refresh falls back to the OAuth client instead of crashing the Worker - Eagerly verify installation auth in getGitHubContextForOwner since @octokit/auth-app authenticates lazily and the existing try/catch could never catch auth failures - Skip suspended GitHub App installations - Cache resolved GitHub contexts per-request (WeakMap) so parallel server functions reuse one session/installation lookup - Propagate the incoming HTTP request abort signal to GitHub API calls via AbortSignal.any so navigating away cancels in-flight work - Add debug() logging to the auth and context resolution flow --- apps/dashboard/src/lib/github-app.server.ts | 42 +++- .../src/lib/github-request-policy.ts | 29 ++- apps/dashboard/src/lib/github.functions.ts | 230 ++++++++++++------ 3 files changed, 217 insertions(+), 84 deletions(-) diff --git a/apps/dashboard/src/lib/github-app.server.ts b/apps/dashboard/src/lib/github-app.server.ts index d0e3629..4a78863 100644 --- a/apps/dashboard/src/lib/github-app.server.ts +++ b/apps/dashboard/src/lib/github-app.server.ts @@ -3,6 +3,7 @@ import { env } from "cloudflare:workers"; import { and, eq } from "drizzle-orm"; import { getDb } from "../db"; import { account } from "../db/schema"; +import { debug } from "./debug"; import { normalizeGitHubAppPrivateKey } from "./github-private-key"; import { GITHUB_REQUEST_TIMEOUT_MS } from "./github-request-policy"; @@ -196,19 +197,25 @@ async function refreshGitHubAppUserToken({ userId: string; }) { const githubApp = getGitHubAppAuthConfig(); - const payload = await requestGitHubAppUserToken({ - client_id: githubApp.clientId, - client_secret: githubApp.clientSecret, - grant_type: "refresh_token", - refresh_token: refreshToken, - }); - - await saveGitHubAppUserToken({ - userId, - payload, - }); - - return payload.access_token; + try { + const payload = await requestGitHubAppUserToken({ + client_id: githubApp.clientId, + client_secret: githubApp.clientSecret, + grant_type: "refresh_token", + refresh_token: refreshToken, + }); + + await saveGitHubAppUserToken({ + userId, + payload, + }); + + debug("github-auth", "app user token refreshed", { userId }); + return payload.access_token; + } catch (error) { + console.error("[github-auth] app user token refresh failed", userId, error); + throw error; + } } async function saveGitHubAppUserToken({ @@ -258,10 +265,12 @@ async function saveGitHubAppUserToken({ export async function getGitHubAppUserAccessTokenByUserId(userId: string) { const githubAccount = await getGitHubAppUserAccountByUserId(userId); if (!githubAccount?.accessToken) { + debug("github-auth", "no app user account", { userId }); return null; } if (isUsableAccessTokenExpiresAt(githubAccount.accessTokenExpiresAt)) { + debug("github-auth", "app user token is fresh", { userId }); return githubAccount.accessToken; } @@ -269,9 +278,16 @@ export async function getGitHubAppUserAccessTokenByUserId(userId: string) { !githubAccount.refreshToken || !isUsableAccessTokenExpiresAt(githubAccount.refreshTokenExpiresAt) ) { + debug("github-auth", "app user token expired, refresh unavailable", { + userId, + hasRefreshToken: Boolean(githubAccount.refreshToken), + refreshTokenExpiresAt: + githubAccount.refreshTokenExpiresAt?.toISOString() ?? null, + }); return null; } + debug("github-auth", "app user token expired, refreshing", { userId }); return refreshGitHubAppUserToken({ refreshToken: githubAccount.refreshToken, userId, diff --git a/apps/dashboard/src/lib/github-request-policy.ts b/apps/dashboard/src/lib/github-request-policy.ts index f792d0d..30d0a1a 100644 --- a/apps/dashboard/src/lib/github-request-policy.ts +++ b/apps/dashboard/src/lib/github-request-policy.ts @@ -1,3 +1,4 @@ +import { getRequest } from "@tanstack/react-start/server"; import type { Octokit as OctokitType } from "octokit"; const GITHUB_READ_RETRY_COUNT = 1; @@ -19,8 +20,28 @@ function isSafeGitHubRetryMethod(method: string | undefined) { return method === "GET" || method === "HEAD" || method === "OPTIONS"; } -function createGitHubRequestTimeoutSignal() { - return AbortSignal.timeout(GITHUB_REQUEST_TIMEOUT_MS); +function createGitHubRequestTimeoutSignal( + requestSignal: AbortSignal | undefined, +) { + const timeoutSignal = AbortSignal.timeout(GITHUB_REQUEST_TIMEOUT_MS); + if (!requestSignal) { + return timeoutSignal; + } + + return AbortSignal.any([requestSignal, timeoutSignal]); +} + +/** + * Returns the incoming HTTP request's abort signal when available. + * When the client navigates away the signal fires, letting us cancel + * in-flight GitHub requests instead of running them to completion. + */ +function getIncomingRequestSignal(): AbortSignal | undefined { + try { + return getRequest().signal; + } catch { + return undefined; + } } export function configureGitHubRequestPolicies(octokit: OctokitType) { @@ -30,6 +51,8 @@ export function configureGitHubRequestPolicies(octokit: OctokitType) { requestOptions.retries = isSafeGitHubRetryMethod(options.method) ? GITHUB_READ_RETRY_COUNT : 0; - requestOptions.signal ??= createGitHubRequestTimeoutSignal(); + requestOptions.signal ??= createGitHubRequestTimeoutSignal( + getIncomingRequestSignal(), + ); }); } diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index ccf8629..1832af5 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -1,5 +1,6 @@ import { createServerFn } from "@tanstack/react-start"; import { type Octokit as OctokitType, RequestError } from "octokit"; +import { debug } from "./debug"; import type { CommandPaletteSearchResult, CreateLabelInput, @@ -693,19 +694,59 @@ async function safeCommandPaletteSearch({ } } -async function getGitHubContext(): Promise { - const { getGitHubClientByUserId, getRequestSession } = await import( - "./auth-runtime" - ); - const session = await getRequestSession(); - if (!session) { +const requestScopedGitHubContextCache = new WeakMap< + Request, + Map> +>(); + +async function getRequestScopedContextCache() { + try { + const { getRequest } = await import("@tanstack/react-start/server"); + const request = getRequest(); + let cache = requestScopedGitHubContextCache.get(request); + if (!cache) { + cache = new Map(); + requestScopedGitHubContextCache.set(request, cache); + } + return cache; + } catch { return null; } +} + +async function getOrCreateCachedContext( + cacheKey: string, + factory: () => Promise, +): Promise { + const cache = await getRequestScopedContextCache(); + const existing = cache?.get(cacheKey); + if (existing) { + debug("github-access", "reusing cached context", { cacheKey }); + return existing; + } - return { - session, - octokit: await getGitHubClientByUserId(session.user.id), - }; + const promise = factory(); + cache?.set(cacheKey, promise); + return promise; +} + +async function getGitHubContext(): Promise { + return getOrCreateCachedContext("base", async () => { + const { getGitHubClientByUserId, getRequestSession } = await import( + "./auth-runtime" + ); + const session = await getRequestSession(); + if (!session) { + debug("github-access", "no session, unauthenticated request"); + return null; + } + + debug("github-access", "session found", { userId: session.user.id }); + return { + session, + octokit: await getGitHubClientByUserId(session.user.id), + }; + }); } function toRepositorySelection(value: string | undefined) { @@ -747,23 +788,29 @@ async function getGitHubAppUserInstallations(userId: string): Promise<{ installations: GitHubAppInstallation[]; installationsAvailable: boolean; }> { - const { getGitHubAppUserClientByUserId } = await import("./auth-runtime"); - const appUserOctokit = await getGitHubAppUserClientByUserId(userId); - if (!appUserOctokit) { - return { installations: [], installationsAvailable: false }; - } - try { + const { getGitHubAppUserClientByUserId } = await import("./auth-runtime"); + const appUserOctokit = await getGitHubAppUserClientByUserId(userId); + if (!appUserOctokit) { + debug("github-access", "no app user client, skipping installations"); + return { installations: [], installationsAvailable: false }; + } + const installationsResponse = await appUserOctokit.request( "GET /user/installations", { per_page: 100, }, ); + const installations = mapGitHubAppInstallations( + installationsResponse.data as GitHubUserInstallationsPayload, + ); + debug("github-access", "loaded app installations", { + count: installations.length, + owners: installations.map((i) => i.account.login), + }); return { - installations: mapGitHubAppInstallations( - installationsResponse.data as GitHubUserInstallationsPayload, - ), + installations, installationsAvailable: true, }; } catch (error) { @@ -773,32 +820,53 @@ async function getGitHubAppUserInstallations(userId: string): Promise<{ } async function getGitHubContextForOwner(owner: string) { - const context = await getGitHubContext(); - if (!context) { - return null; - } - - const { installations } = await getGitHubAppUserInstallations( - context.session.user.id, - ); - const installation = findGitHubAppInstallationForOwner(installations, owner); - if (!installation) { - return context; - } + return getOrCreateCachedContext(`owner:${owner}`, async () => { + const context = await getGitHubContext(); + if (!context) { + return null; + } - try { - const { getGitHubInstallationClient } = await import("./github.server"); - return { - ...context, - octokit: await getGitHubInstallationClient(installation.id), - }; - } catch (error) { - console.error( - "[github-access] failed to create installation client", - error, + const { installations } = await getGitHubAppUserInstallations( + context.session.user.id, ); - return context; - } + const installation = findGitHubAppInstallationForOwner( + installations, + owner, + ); + if (!installation) { + debug("github-access", "no installation for owner, using OAuth token", { + owner, + }); + return context; + } + + try { + debug("github-access", "creating installation client", { + owner, + installationId: installation.id, + }); + const { getGitHubInstallationClient } = await import("./github.server"); + const installationOctokit = await getGitHubInstallationClient( + installation.id, + ); + // Eagerly authenticate to verify the installation token is valid. + // auth-app authenticates lazily by default, so without this the + // try/catch cannot catch auth failures. + await installationOctokit.auth({ type: "installation" }); + debug("github-access", "installation client ready", { owner }); + return { + ...context, + octokit: installationOctokit, + }; + } catch (error) { + console.error( + "[github-access] installation client failed, falling back to OAuth token", + owner, + error, + ); + return context; + } + }); } async function getGitHubContextForRepository(input: { @@ -815,38 +883,64 @@ function findGitHubAppInstallationForOwner( const normalizedOwner = owner.toLowerCase(); return installations.find( (candidate) => - candidate.account.login.toLowerCase() === normalizedOwner || - (candidate.targetType === "User" && - candidate.account.login.toLowerCase() === normalizedOwner), + !candidate.suspendedAt && + (candidate.account.login.toLowerCase() === normalizedOwner || + (candidate.targetType === "User" && + candidate.account.login.toLowerCase() === normalizedOwner)), ); } async function getGitHubUserContextForOwner(owner: string) { - const context = await getGitHubContext(); - if (!context) { - return null; - } + return getOrCreateCachedContext(`user-owner:${owner}`, async () => { + const context = await getGitHubContext(); + if (!context) { + return null; + } - const { getGitHubAppUserClientByUserId } = await import("./auth-runtime"); - const appUserOctokit = await getGitHubAppUserClientByUserId( - context.session.user.id, - ); - if (!appUserOctokit) { - return context; - } + try { + const { getGitHubAppUserClientByUserId } = await import("./auth-runtime"); + const appUserOctokit = await getGitHubAppUserClientByUserId( + context.session.user.id, + ); + if (!appUserOctokit) { + debug( + "github-access", + "no app user client for writes, using OAuth token", + { owner }, + ); + return context; + } - const { installations } = await getGitHubAppUserInstallations( - context.session.user.id, - ); - const installation = findGitHubAppInstallationForOwner(installations, owner); - if (!installation) { - return context; - } + const { installations } = await getGitHubAppUserInstallations( + context.session.user.id, + ); + const installation = findGitHubAppInstallationForOwner( + installations, + owner, + ); + if (!installation) { + debug( + "github-access", + "no installation for owner, using OAuth token for writes", + { owner }, + ); + return context; + } - return { - ...context, - octokit: appUserOctokit, - }; + debug("github-access", "using app user token for writes", { owner }); + return { + ...context, + octokit: appUserOctokit, + }; + } catch (error) { + console.error( + "[github-access] failed to resolve user context, falling back to OAuth token", + owner, + error, + ); + return context; + } + }); } async function getGitHubUserContextForRepository(input: {