From 084fca42bfa746a2d54e28f73e443b8c963f3082 Mon Sep 17 00:00:00 2001 From: Paijo <14921983+oyi77@users.noreply.github.com> Date: Sun, 12 Jul 2026 20:29:48 +0700 Subject: [PATCH] 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 Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- .../6947-tokenhealthcheck-provider-case.md | 1 + src/lib/tokenHealthCheck.ts | 6 ++- .../oauth-providers-error-handling.test.ts | 46 ++++++++++++++++++- 3 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 changelog.d/fixes/6947-tokenhealthcheck-provider-case.md 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 ─────────────────────────────────────────────────