mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-25 08:32:11 +03:00
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!
This commit is contained in:
1
changelog.d/fixes/10352-github-access-token-health.md
Normal file
1
changelog.d/fixes/10352-github-access-token-health.md
Normal file
@@ -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
|
||||
@@ -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 };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -636,35 +636,45 @@ export async function checkConnection(conn) {
|
||||
copilotExpiresAtMs - Date.now() < TOKEN_EXPIRY_BUFFER;
|
||||
|
||||
let refreshedProviderSpecificData: Record<string, unknown> | 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<string, unknown>) => {
|
||||
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<string, unknown>) => {
|
||||
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)) {
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user