mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 12:22:14 +03:00
fix(quota): never flag rotating-refresh providers expired from the quota sync
The quota-sync path deliberately reuses a rotating-refresh provider's (Codex/ OpenAI/Claude — see refreshSerializer ROTATION_LOCK_GROUP) access_token WITHOUT proactively refreshing it (#3019, to avoid the Auth0 family-revocation cascade). When that token is expired the codex usage fetch returns "token expired", and syncExpiredStatusIfNeeded then flagged the connection testStatus="expired" — a false-negative: the credential is still valid (expires_at in the future) and the reactive serialized 401 path refreshes the access_token on next use. Symptom: freshly-added Codex accounts showed "expired" with no quota on the quota page, while a providers-page refresh turned them green. They never lost access — only the quota sync mislabeled them. Fix: extract the decision into the pure, exported `quotaPathShouldMarkExpired()` and skip rotating providers (rotationGroupFor !== null). Their status is owned by the reactive path / connection test, never the quota sync. Adds unit coverage.
This commit is contained in:
@@ -213,15 +213,43 @@ function shouldFailClosedForProviderLimitsProxy(
|
||||
return connection.authType === "oauth" && isAccountScopedProxyResolution(proxyInfo);
|
||||
}
|
||||
|
||||
async function syncExpiredStatusIfNeeded(connection: ProviderConnectionLike, usage: JsonRecord) {
|
||||
const errorMessage = typeof usage.message === "string" ? usage.message.toLowerCase() : "";
|
||||
const isAuthError =
|
||||
errorMessage.includes("token expired") ||
|
||||
errorMessage.includes("access denied") ||
|
||||
errorMessage.includes("re-authenticate") ||
|
||||
errorMessage.includes("unauthorized");
|
||||
/**
|
||||
* Decide whether the quota-sync path should flag a connection `expired` from an
|
||||
* auth-style usage error. Exported for unit testing.
|
||||
*
|
||||
* Rotating-refresh providers (Codex/OpenAI/Claude/etc. — see refreshSerializer's
|
||||
* ROTATION_LOCK_GROUP) have their access_token deliberately NOT proactively
|
||||
* refreshed in this quota path (#3019, to avoid the Auth0 family-revocation
|
||||
* cascade). So a "token expired" from the quota fetch is a recoverable
|
||||
* false-negative: the credential is still valid (its `expires_at` is in the
|
||||
* future) and the reactive, serialized 401 path refreshes the access_token on
|
||||
* next use. Flagging it `expired` hides a healthy account from the quota page
|
||||
* (observed: freshly-added Codex accounts flagged expired while a providers-page
|
||||
* refresh turns them green). So never mark a rotating provider expired from the
|
||||
* quota sync — leave its status to the reactive path / connection test.
|
||||
*/
|
||||
export function quotaPathShouldMarkExpired(
|
||||
provider: string,
|
||||
usageMessage: unknown,
|
||||
currentTestStatus: string | null | undefined
|
||||
): boolean {
|
||||
if (currentTestStatus === "expired") return false;
|
||||
|
||||
if (!isAuthError || connection.testStatus === "expired") {
|
||||
const message = typeof usageMessage === "string" ? usageMessage.toLowerCase() : "";
|
||||
const isAuthError =
|
||||
message.includes("token expired") ||
|
||||
message.includes("access denied") ||
|
||||
message.includes("re-authenticate") ||
|
||||
message.includes("unauthorized");
|
||||
if (!isAuthError) return false;
|
||||
|
||||
if (rotationGroupFor(provider) !== null) return false;
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
async function syncExpiredStatusIfNeeded(connection: ProviderConnectionLike, usage: JsonRecord) {
|
||||
if (!quotaPathShouldMarkExpired(connection.provider, usage.message, connection.testStatus)) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
49
tests/unit/provider-limits-rotating-expired-guard.test.ts
Normal file
49
tests/unit/provider-limits-rotating-expired-guard.test.ts
Normal file
@@ -0,0 +1,49 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
// providerLimits.ts touches the DB singleton at import time; give it a scratch dir.
|
||||
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-rotating-expired-guard-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "rotating-expired-guard-secret";
|
||||
|
||||
const { quotaPathShouldMarkExpired } = await import("../../src/lib/usage/providerLimits.ts");
|
||||
|
||||
test.after(() => {
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// Regression: the quota sync reuses a rotating provider's (possibly expired)
|
||||
// access_token without refreshing it (#3019, to avoid the Auth0 family-revocation
|
||||
// cascade). A "token expired" from that fetch is recoverable, NOT a real expiry —
|
||||
// flagging it expired hid freshly-added Codex accounts from the quota page even
|
||||
// though a providers-page refresh turned them green.
|
||||
test("rotating providers are NEVER flagged expired from the quota path", () => {
|
||||
for (const provider of ["codex", "openai", "claude", "kiro", "qwen", "gitlab-duo"]) {
|
||||
assert.equal(
|
||||
quotaPathShouldMarkExpired(provider, "Token expired, please re-authenticate", "active"),
|
||||
false,
|
||||
`${provider} (rotating) must not be marked expired by the quota sync`
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("non-rotating OAuth providers are still flagged expired on a genuine auth error", () => {
|
||||
assert.equal(quotaPathShouldMarkExpired("github", "token expired", "active"), true);
|
||||
assert.equal(quotaPathShouldMarkExpired("github", "Unauthorized", "active"), true);
|
||||
assert.equal(quotaPathShouldMarkExpired("cursor", "Access denied", "active"), true);
|
||||
});
|
||||
|
||||
test("non-auth usage messages never trigger an expired flag", () => {
|
||||
assert.equal(quotaPathShouldMarkExpired("github", "rate limit exceeded", "active"), false);
|
||||
assert.equal(quotaPathShouldMarkExpired("github", "", "active"), false);
|
||||
assert.equal(quotaPathShouldMarkExpired("github", undefined, "active"), false);
|
||||
assert.equal(quotaPathShouldMarkExpired("github", { nested: true }, "active"), false);
|
||||
});
|
||||
|
||||
test("an already-expired connection is left untouched (no redundant write)", () => {
|
||||
assert.equal(quotaPathShouldMarkExpired("github", "token expired", "expired"), false);
|
||||
assert.equal(quotaPathShouldMarkExpired("codex", "token expired", "expired"), false);
|
||||
});
|
||||
Reference in New Issue
Block a user