fix(tokenHealthCheck): case-sensitive provider comparisons break rotating/gh checks (#6947)

ROTATING_REFRESH_PROVIDERS.has(conn.provider) fails for 'OpenAI' or 'Github'
- the set is all lowercase. Same issue for the GitHub Copilot sub-token
refresh guard. Both now normalize to lowercase before comparison, matching
the established pattern from getHealthCheckSkipProviders() (line 201) and
isGitHubAccessTokenOnlyConnection() (line 94).

Replaces the whole-file regex assertion in oauth-providers-error-handling
(which passed even on unfixed code) with two statement-scoped regression
tests that fail against origin/release/v3.8.47's unfixed source and pass
only once both call sites are normalized.

Co-authored-by: oyi77 <oyi77@users.noreply.github.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
Paijo
2026-07-12 20:29:48 +07:00
committed by GitHub
parent 19d1a5d391
commit 084fca42bf
3 changed files with 50 additions and 3 deletions

View File

@@ -0,0 +1 @@
- fix(oauth): tokenHealthCheck now lowercase-normalizes `conn.provider` before checking `ROTATING_REFRESH_PROVIDERS.has()` and the GitHub Copilot sub-token refresh guard, so mixed-case provider values (e.g. "OpenAI", "Github") no longer bypass the rotating-refresh-token skip or the Copilot sub-token refresh (#6947)

View File

@@ -532,7 +532,9 @@ export async function checkConnection(conn) {
"gitlab-duo",
"claude",
]);
const isRotatingProvider = ROTATING_REFRESH_PROVIDERS.has(conn.provider);
const isRotatingProvider = ROTATING_REFRESH_PROVIDERS.has(
String(conn.provider || "").toLowerCase()
);
const shouldRefreshByInterval =
!hasKnownExpiry && !isRotatingProvider && Date.now() - lastCheck >= intervalMs;
@@ -755,7 +757,7 @@ export async function checkConnection(conn) {
// GitHub OAuth token. The health check must also refresh this sub-token before
// it expires mid-session. The Copilot token expiry is stored in
// providerSpecificData.copilotTokenExpiresAt (Unix seconds).
if (conn.provider === "github") {
if (String(conn.provider || "").toLowerCase() === "github") {
// Re-read the latest connection after the OAuth refresh (onPersist may have updated it).
const latestConn = (await getProviderConnectionById(conn.id).catch(() => null)) || conn;
const accessTokenForCopilot = result.accessToken || latestConn.accessToken;

View File

@@ -109,7 +109,51 @@ test("P1: GitHub Copilot sub-token is refreshed by tokenHealthCheck", async () =
test("P1: tokenHealthCheck checks copilotTokenExpiresAt before refreshing", async () => {
const src = await read("src/lib/tokenHealthCheck.ts");
assert.match(src, /copilotTokenExpiresAt/, "must check copilotTokenExpiresAt");
assert.match(src, /conn\.provider\s*===\s*["']github["']/, "must be gated on github provider");
assert.match(src, /toLowerCase\(\)\s*===\s*["']github["']/, "must be gated on github provider");
});
// ─── P1: case-insensitive provider comparisons (regression for #6947) ────────
//
// tokenHealthCheck.checkConnection() gates two decisions on `conn.provider`:
// 1. ROTATING_REFRESH_PROVIDERS.has(conn.provider) — skips the fixed-interval
// refresh sweep for single-use-refresh-token providers (codex/openai/etc).
// 2. conn.provider === "github" — gates the Copilot sub-token refresh.
// Both membership tests were case-sensitive while `conn.provider` can be stored
// in mixed case (e.g. "OpenAI", "Github"), silently disabling the guard. These
// assertions are scoped to the exact statement (not a whole-file scan), so they
// fail against the unfixed source — verified against
// `git show origin/release/v3.8.47:src/lib/tokenHealthCheck.ts` (lines 535/758).
test("P1: ROTATING_REFRESH_PROVIDERS.has() normalizes conn.provider case before lookup", async () => {
const src = await read("src/lib/tokenHealthCheck.ts");
const assignMatch = src.match(
/const\s+isRotatingProvider\s*=\s*ROTATING_REFRESH_PROVIDERS\.has\(\s*([\s\S]{0,80}?)\s*\);/
);
assert.ok(assignMatch, "isRotatingProvider assignment not found");
const arg = assignMatch[1];
assert.match(
arg,
/String\(\s*conn\.provider\s*\|\|\s*["']["']\s*\)\.toLowerCase\(\)/,
"ROTATING_REFRESH_PROVIDERS.has() must lowercase-normalize conn.provider before the lookup " +
"(bare `conn.provider` fails for 'OpenAI'/'Github' since the Set is all-lowercase)"
);
});
test("P1: GitHub Copilot sub-token guard normalizes conn.provider case", async () => {
const src = await read("src/lib/tokenHealthCheck.ts");
// Scope the match to the Copilot sub-token refresh block via its own comment,
// not an arbitrary occurrence elsewhere in the file.
const blockMatch = src.match(
/GitHub Copilot sub-token refresh[\s\S]{0,600}?if\s*\(([\s\S]{0,80}?)\)\s*\{/
);
assert.ok(blockMatch, "Copilot sub-token refresh block not found");
const condition = blockMatch[1];
assert.match(
condition,
/String\(\s*conn\.provider\s*\|\|\s*["']["']\s*\)\.toLowerCase\(\)\s*===\s*["']github["']/,
"the Copilot sub-token refresh guard must lowercase-normalize conn.provider before comparing " +
"to 'github' (bare `conn.provider === \"github\"` fails for mixed-case values like 'Github')"
);
});
// ─── P2: Google invalid_grant ─────────────────────────────────────────────────