From 5ee646e68e336244dd03c964c99761e12910c1a9 Mon Sep 17 00:00:00 2001 From: Ravi Tharuma <25951435+RaviTharuma@users.noreply.github.com> Date: Mon, 24 Aug 2026 06:50:30 +0200 Subject: [PATCH] fix(github): verify access tokens during health checks (#11320) Validated on a 17-PR combined board: token-health-check + token-health-no-refresh-token-expired-5326 + token-refresh-service within the board's 287/287, typecheck:core clean. GitHub access-token-only connections are now actively verified on each due health interval (via the existing Copilot token exchange); the parent credential is marked expired only on a confirmed 401, never on 403/429/5xx/network failures; response bodies and transport messages no longer enter token-refresh logs. Closes #10352. Thank you @RaviTharuma! --- .../fixes/10352-github-access-token-health.md | 1 + .../tokenRefresh/providers/copilot.ts | 8 +- src/lib/tokenHealthCheck.ts | 66 ++++++----- src/sse/services/tokenRefresh.ts | 4 +- tests/unit/token-health-check.test.ts | 93 ++++++++++++++++ ...alth-no-refresh-token-expired-5326.test.ts | 104 +++++++++++------- tests/unit/token-refresh-service.test.ts | 29 +++++ 7 files changed, 231 insertions(+), 74 deletions(-) create mode 100644 changelog.d/fixes/10352-github-access-token-health.md diff --git a/changelog.d/fixes/10352-github-access-token-health.md b/changelog.d/fixes/10352-github-access-token-health.md new file mode 100644 index 0000000000..732f6ef713 --- /dev/null +++ b/changelog.d/fixes/10352-github-access-token-health.md @@ -0,0 +1 @@ +- **fix(github):** proactive credential health now verifies GitHub access tokens through the existing Copilot token exchange, marks only a confirmed `401 Unauthorized` as expired, and leaves rate limits, permission failures, upstream failures, and network errors routable ([#10352](https://github.com/diegosouzapw/OmniRoute/issues/10352)) — thanks @RaviTharuma diff --git a/open-sse/services/tokenRefresh/providers/copilot.ts b/open-sse/services/tokenRefresh/providers/copilot.ts index 44e05b647f..f4bd65add1 100644 --- a/open-sse/services/tokenRefresh/providers/copilot.ts +++ b/open-sse/services/tokenRefresh/providers/copilot.ts @@ -28,12 +28,10 @@ export async function refreshCopilotToken( ); if (!response.ok) { - const errorText = await response.text(); log?.error?.("TOKEN_REFRESH", "Failed to refresh Copilot token", { status: response.status, - error: errorText, }); - return null; + return { status: response.status }; } const data = await response.json(); @@ -49,8 +47,8 @@ export async function refreshCopilotToken( }; } catch (error) { log?.error?.("TOKEN_REFRESH", "Error refreshing Copilot token", { - error: error.message, + errorType: error?.name || "Error", }); - return null; + return { status: null }; } } diff --git a/src/lib/tokenHealthCheck.ts b/src/lib/tokenHealthCheck.ts index 4927c4823f..dd039399c0 100644 --- a/src/lib/tokenHealthCheck.ts +++ b/src/lib/tokenHealthCheck.ts @@ -636,35 +636,45 @@ export async function checkConnection(conn) { copilotExpiresAtMs - Date.now() < TOKEN_EXPIRY_BUFFER; let refreshedProviderSpecificData: Record | null = null; - if (copilotAboutToExpire) { - const hideLogs = await shouldHideLogs(); - const proxyResolution = await resolveProxyForConnection(conn.id); - const proxyConfig = extractResolvedProxyConfig(proxyResolution); - const healthCheckLog = { - info: (tag: string, msg: string) => { - if (!hideLogs) console.log(LOG_PREFIX, `[${tag}]`, msg); - }, - warn: (tag: string, msg: string) => { - if (!hideLogs) console.warn(LOG_PREFIX, `[${tag}]`, msg); - }, - error: (tag: string, msg: string, extra?: Record) => { - if (!hideLogs) console.error(LOG_PREFIX, `[${tag}]`, msg, extra || ""); - }, - }; + const hideLogs = await shouldHideLogs(); + const proxyResolution = await resolveProxyForConnection(conn.id); + const proxyConfig = extractResolvedProxyConfig(proxyResolution); + const healthCheckLog = { + info: (tag: string, msg: string) => { + if (!hideLogs) console.log(LOG_PREFIX, `[${tag}]`, msg); + }, + warn: (tag: string, msg: string) => { + if (!hideLogs) console.warn(LOG_PREFIX, `[${tag}]`, msg); + }, + error: (tag: string, msg: string, extra?: Record) => { + if (!hideLogs) console.error(LOG_PREFIX, `[${tag}]`, msg, extra || ""); + }, + }; - const copilotResult = await refreshCopilotToken( - conn.accessToken, - healthCheckLog, - proxyConfig, - getCopilotTokenBaseUrl(conn) - ); - if (copilotResult?.token) { - refreshedProviderSpecificData = { - ...providerSpecificData, - copilotToken: copilotResult.token, - copilotTokenExpiresAt: copilotResult.expiresAt, - }; - } + const copilotResult = await refreshCopilotToken( + conn.accessToken, + healthCheckLog, + proxyConfig, + getCopilotTokenBaseUrl(conn) + ); + if (copilotResult?.status === 401) { + await updateProviderConnection(conn.id, { + testStatus: "expired", + lastHealthCheckAt: now, + lastError: "GitHub rejected the access token", + lastErrorAt: now, + lastErrorType: "github_access_token_invalid", + lastErrorSource: "oauth", + errorCode: "github_access_token_invalid", + }); + return; + } + if (copilotResult?.token && copilotAboutToExpire) { + refreshedProviderSpecificData = { + ...providerSpecificData, + copilotToken: copilotResult.token, + copilotTokenExpiresAt: copilotResult.expiresAt, + }; } if (canClearGitHubNoRefreshTokenState(conn)) { diff --git a/src/sse/services/tokenRefresh.ts b/src/sse/services/tokenRefresh.ts index 2032aaafda..b655cedc26 100755 --- a/src/sse/services/tokenRefresh.ts +++ b/src/sse/services/tokenRefresh.ts @@ -276,7 +276,7 @@ export async function checkAndRefreshToken(provider: string, credentials: any) { updatedCredentials, resolveCopilotTokenBaseUrl(provider, updatedCredentials) ); - if (copilotToken) { + if (copilotToken?.token) { await updateProviderCredentials(updatedCredentials.connectionId, { providerSpecificData: { ...updatedCredentials.providerSpecificData, @@ -304,7 +304,7 @@ export async function refreshGitHubAndCopilotTokens(credentials: any) { const newGitHubCredentials = await refreshGitHubToken(credentials.refreshToken, credentials); if (newGitHubCredentials?.accessToken) { const copilotToken = await refreshCopilotToken(newGitHubCredentials.accessToken, credentials); - if (copilotToken) { + if (copilotToken?.token) { return { ...newGitHubCredentials, providerSpecificData: { diff --git a/tests/unit/token-health-check.test.ts b/tests/unit/token-health-check.test.ts index 8606c29b5f..274be4c126 100644 --- a/tests/unit/token-health-check.test.ts +++ b/tests/unit/token-health-check.test.ts @@ -38,6 +38,99 @@ async function resetStorage() { fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); } +test("GitHub access-token health demotes only a verified 401 and stores no secrets", async () => { + for (const status of [200, 401, 403, 429, 500]) { + await resetStorage(); + const accessToken = `ghp_status_${status}_secret`; + const responseSecret = `response-${status}-secret`; + const originalFetch = globalThis.fetch; + const consoleOutput: unknown[] = []; + const originalError = console.error; + console.error = (...args: unknown[]) => consoleOutput.push(args); + globalThis.fetch = (async () => + status === 200 + ? new Response( + JSON.stringify({ + token: `copilot-${status}-secret`, + expires_at: Math.floor(Date.now() / 1000) + 1800, + }), + { status, headers: { "content-type": "application/json" } } + ) + : new Response(responseSecret, { status })) as typeof fetch; + + try { + const connection = await providersDb.createProviderConnection({ + provider: "github", + authType: "oauth", + name: `GitHub ${status}`, + accessToken, + healthCheckInterval: 60, + isActive: true, + testStatus: "active", + providerSpecificData: { + copilotToken: "existing-copilot-secret", + copilotTokenExpiresAt: Math.floor(Date.now() / 1000) + 3600, + }, + }); + + await tokenHealthCheck.checkConnection({ + ...connection, + lastHealthCheckAt: new Date(Date.now() - 61 * 60 * 1000).toISOString(), + }); + + const updated = await providersDb.getProviderConnectionById(connection.id); + assert.equal(updated?.testStatus, status === 401 ? "expired" : "active"); + assert.equal(updated?.lastHealthCheckAt !== connection.lastHealthCheckAt, true); + assert.equal(JSON.stringify(updated).includes(responseSecret), false); + assert.equal(JSON.stringify(consoleOutput).includes(accessToken), false); + assert.equal(JSON.stringify(consoleOutput).includes(responseSecret), false); + if (status === 401) { + assert.equal(updated?.errorCode, "github_access_token_invalid"); + assert.equal(updated?.lastErrorType, "github_access_token_invalid"); + assert.equal(updated?.lastErrorSource, "oauth"); + } + } finally { + globalThis.fetch = originalFetch; + console.error = originalError; + } + } +}); + +test("GitHub access-token health keeps network failures active", async () => { + await resetStorage(); + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => { + throw new Error("network down"); + }) as typeof fetch; + + try { + const connection = await providersDb.createProviderConnection({ + provider: "github", + authType: "oauth", + name: "GitHub network", + accessToken: "ghp_network_secret", + healthCheckInterval: 60, + isActive: true, + testStatus: "active", + providerSpecificData: { + copilotToken: "existing-copilot-secret", + copilotTokenExpiresAt: Math.floor(Date.now() / 1000) + 3600, + }, + }); + + await tokenHealthCheck.checkConnection({ + ...connection, + lastHealthCheckAt: new Date(Date.now() - 61 * 60 * 1000).toISOString(), + }); + + const updated = await providersDb.getProviderConnectionById(connection.id); + assert.equal(updated?.testStatus, "active"); + assert.equal(updated?.lastHealthCheckAt !== connection.lastHealthCheckAt, true); + } finally { + globalThis.fetch = originalFetch; + } +}); + async function withHttpServer(handler, fn) { const server = http.createServer(handler); diff --git a/tests/unit/token-health-no-refresh-token-expired-5326.test.ts b/tests/unit/token-health-no-refresh-token-expired-5326.test.ts index a79934bc05..204c11276e 100644 --- a/tests/unit/token-health-no-refresh-token-expired-5326.test.ts +++ b/tests/unit/token-health-no-refresh-token-expired-5326.test.ts @@ -124,55 +124,81 @@ test("checkConnection leaves a non-refresh provider with no refresh token untouc test("checkConnection keeps GitHub Copilot access-token-only connections active", async () => { await resetStorage(); + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => + new Response( + JSON.stringify({ + token: "verified-copilot-token", + expires_at: Math.floor((Date.now() + 60 * 60 * 1000) / 1000), + }), + { status: 200, headers: { "content-type": "application/json" } } + )) as typeof fetch; - const connection = await providersDb.createProviderConnection({ - provider: "github", - authType: "oauth", - name: "GitHub Access Token Account", - accessToken: "github-access-token", - refreshToken: null, - providerSpecificData: { - copilotToken: "copilot-token", - copilotTokenExpiresAt: Math.floor((Date.now() + 60 * 60 * 1000) / 1000), - }, - testStatus: "active", - isActive: true, - }); + try { + const connection = await providersDb.createProviderConnection({ + provider: "github", + authType: "oauth", + name: "GitHub Access Token Account", + accessToken: "github-access-token", + refreshToken: null, + providerSpecificData: { + copilotToken: "copilot-token", + copilotTokenExpiresAt: Math.floor((Date.now() + 60 * 60 * 1000) / 1000), + }, + testStatus: "active", + isActive: true, + }); - await tokenHealthCheck.checkConnection(connection); + await tokenHealthCheck.checkConnection(connection); - const updated = await providersDb.getProviderConnectionById(getCreatedConnectionId(connection)); - assert.equal(updated?.testStatus, "active"); - assert.notEqual(updated?.errorCode, "no_refresh_token"); - assert.ok(updated?.lastHealthCheckAt); + const updated = await providersDb.getProviderConnectionById(getCreatedConnectionId(connection)); + assert.equal(updated?.testStatus, "active"); + assert.notEqual(updated?.errorCode, "no_refresh_token"); + assert.ok(updated?.lastHealthCheckAt); + } finally { + globalThis.fetch = originalFetch; + } }); test("checkConnection clears stale no_refresh_token state for usable GitHub Copilot connections", async () => { await resetStorage(); + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => + new Response( + JSON.stringify({ + token: "verified-copilot-token", + expires_at: Math.floor((Date.now() + 60 * 60 * 1000) / 1000), + }), + { status: 200, headers: { "content-type": "application/json" } } + )) as typeof fetch; - const connection = await providersDb.createProviderConnection({ - provider: "github", - authType: "oauth", - name: "GitHub False Expired Account", - accessToken: "github-access-token", - refreshToken: null, - providerSpecificData: { - copilotToken: "copilot-token", - copilotTokenExpiresAt: Math.floor((Date.now() + 60 * 60 * 1000) / 1000), - }, - testStatus: "expired", - errorCode: "no_refresh_token", - lastError: "No refresh token available — re-authenticate this account.", - isActive: true, - }); + try { + const connection = await providersDb.createProviderConnection({ + provider: "github", + authType: "oauth", + name: "GitHub False Expired Account", + accessToken: "github-access-token", + refreshToken: null, + providerSpecificData: { + copilotToken: "copilot-token", + copilotTokenExpiresAt: Math.floor((Date.now() + 60 * 60 * 1000) / 1000), + }, + testStatus: "expired", + errorCode: "no_refresh_token", + lastError: "No refresh token available — re-authenticate this account.", + isActive: true, + }); - await tokenHealthCheck.checkConnection(connection); + await tokenHealthCheck.checkConnection(connection); - const updated = await providersDb.getProviderConnectionById(getCreatedConnectionId(connection)); - assert.equal(updated?.testStatus, "active"); - assert.equal(updated?.errorCode ?? null, null); - assert.equal(updated?.lastError ?? null, null); - assert.ok(updated?.lastHealthCheckAt); + const updated = await providersDb.getProviderConnectionById(getCreatedConnectionId(connection)); + assert.equal(updated?.testStatus, "active"); + assert.equal(updated?.errorCode ?? null, null); + assert.equal(updated?.lastError ?? null, null); + assert.ok(updated?.lastHealthCheckAt); + } finally { + globalThis.fetch = originalFetch; + } }); // Boundary regression for #8182 vs #5326: the terminal-skip guard added by #8182 diff --git a/tests/unit/token-refresh-service.test.ts b/tests/unit/token-refresh-service.test.ts index fdc7ef16c0..aa675efb2b 100644 --- a/tests/unit/token-refresh-service.test.ts +++ b/tests/unit/token-refresh-service.test.ts @@ -739,6 +739,35 @@ test("refreshCopilotToken returns the short-lived copilot token", async () => { assert.equal(calls[0].options.headers.Authorization, "token github-access-token"); }); +test("refreshCopilotToken reports HTTP outcomes without logging response bodies", async () => { + const secret = "ghp_never-log-this"; + const responseBody = `credential ${secret} rejected`; + + for (const status of [401, 403, 429, 500]) { + const log = createLog(); + const result = await withMockedFetch( + async () => textResponse(responseBody, status), + () => refreshCopilotToken(secret, log) + ); + + assert.deepEqual(result, { status }); + assert.equal(JSON.stringify(log.entries).includes(secret), false); + assert.equal(JSON.stringify(log.entries).includes(responseBody), false); + } +}); + +test("refreshCopilotToken distinguishes network failures from HTTP failures", async () => { + const log = createLog(); + const result = await withMockedFetch( + async () => { + throw new Error("socket closed"); + }, + () => refreshCopilotToken("ghp_network-test", log) + ); + + assert.deepEqual(result, { status: null }); +}); + test("supportsTokenRefresh, isUnrecoverableRefreshError and formatProviderCredentials cover provider helpers", async () => { const log = createLog();