diff --git a/changelog.d/fixes/6947-tokenhealthcheck-provider-case.md b/changelog.d/fixes/6947-tokenhealthcheck-provider-case.md new file mode 100644 index 0000000000..d9b19fa4e9 --- /dev/null +++ b/changelog.d/fixes/6947-tokenhealthcheck-provider-case.md @@ -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) diff --git a/src/lib/tokenHealthCheck.ts b/src/lib/tokenHealthCheck.ts index 1fa4f48d7f..b21b91005c 100644 --- a/src/lib/tokenHealthCheck.ts +++ b/src/lib/tokenHealthCheck.ts @@ -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; diff --git a/tests/unit/oauth-providers-error-handling.test.ts b/tests/unit/oauth-providers-error-handling.test.ts index c899866ed7..dd34d14272 100644 --- a/tests/unit/oauth-providers-error-handling.test.ts +++ b/tests/unit/oauth-providers-error-handling.test.ts @@ -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 ─────────────────────────────────────────────────