From 449a692f4380585b6d707629f72bf434c3fb18d5 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Mon, 14 Sep 2026 09:28:32 +0900 Subject: [PATCH 1/2] fix(codex): bound completed entitlement version misses per account --- src/codex/model-entitlements.ts | 80 +++++++++++++++++++ structure/catalog.md | 7 ++ .../codex-model-entitlements.test.ts | 40 ++++++++++ 3 files changed, 127 insertions(+) diff --git a/src/codex/model-entitlements.ts b/src/codex/model-entitlements.ts index 553fb501b7..6a4401db0f 100644 --- a/src/codex/model-entitlements.ts +++ b/src/codex/model-entitlements.ts @@ -317,6 +317,37 @@ const MODEL_ROSTER_VERSIONS_PER_ACCOUNT_MAX = 4; * roster. */ const MODEL_ROSTER_FLIGHTS_PER_ACCOUNT_MAX = 4; + +/** + * Distinct caller-selected roster versions admitted per account in one roster window. + * + * The cache budget and the flight budget both bound STATE, not WORK. A caller that cycles + * `client_version` and waits for each answer misses the cache by design and misses the flight + * key by design, so it can renew an authenticated upstream request under EVERY stored account + * token as often as it likes, and the gated-model checks it displaces fail closed while it does. + * + * DISTINCT VERSIONS are counted, never attempts. One legitimate client retrying a single version + * through an upstream outage comes back every 15s on the failure TTL; charging each attempt would + * spend the whole allowance on that one version and then refuse it for the rest of the 5-minute + * window, turning a recovered upstream into several more minutes without gated models. + */ +const MODEL_ROSTER_VERSION_MISSES_PER_ACCOUNT_MAX = 4; + +interface AccountVersionMissBudget { + credentialIdentity: string; + /** Version -> when this version stops occupying the allowance. */ + versions: Map; +} + +/** + * One row per ACCOUNT, not per credential identity. + * + * A Pool access-token refresh increments the generation, so an identity-keyed map would gain a + * permanent row per generation for the lifetime of the process: a protection against renewable + * work would have introduced an unbounded cache. A generation change replaces the row instead, + * which is also the right budget semantics — new credential, new allowance. + */ +const accountModelsMisses = new Map(); const DIRECT_CALLER_ACCOUNT_PREFIX = "__direct_codex__:"; export interface CodexModelEntitlementCredentialSnapshot { @@ -670,6 +701,7 @@ async function modelsForCredential( fetcher: typeof fetch, now: number, clientVersion: string, + trustedClientVersion: string, credentialMutationEpoch?: number, ): Promise { const cached = accountModelsCache.get(cacheKeyFor(credential.accountId, clientVersion)); @@ -684,6 +716,23 @@ async function modelsForCredential( const existing = accountModelsFlights.get(flightKey); if (existing) return existing; + // Joining an in-flight request costs nothing upstream, so the allowance is spent only where a + // NEW upstream request would be opened. The locally selected runtime version is exempt: it has + // one stable key, and the legitimate refresh must survive an untrusted caller spending the rest. + if ( + !credential.accountId.startsWith(DIRECT_CALLER_ACCOUNT_PREFIX) + && clientVersion !== trustedClientVersion + && !admitVersionMiss(credential, clientVersion, now) + ) { + return { + credentialIdentity: credential.credentialIdentity, + clientVersion, + expiresAt: now, + models: new Set(), + confirmed: false, + }; + } + // Bound concurrency per account before opening another upstream request. let liveForAccount = 0; for (const key of accountModelsFlights.keys()) { @@ -716,6 +765,30 @@ async function modelsForCredential( return flight; } +/** Whether this caller-selected version may open a new upstream request for the account. */ +function admitVersionMiss( + credential: CodexModelEntitlementCredentialSnapshot, + clientVersion: string, + now: number, +): boolean { + const stored = accountModelsMisses.get(credential.accountId); + const budget = stored && stored.credentialIdentity === credential.credentialIdentity + ? stored + : { credentialIdentity: credential.credentialIdentity, versions: new Map() }; + for (const [version, expiresAt] of budget.versions) { + if (expiresAt <= now) budget.versions.delete(version); + } + const alreadyCharged = budget.versions.has(clientVersion); + const admitted = alreadyCharged + || budget.versions.size < MODEL_ROSTER_VERSION_MISSES_PER_ACCOUNT_MAX; + // A repeat keeps its ORIGINAL expiry. Refreshing it here would let a caller hold one version + // open indefinitely, and it is the retry case this distinction exists to protect. + if (admitted && !alreadyCharged) budget.versions.set(clientVersion, now + MODEL_ROSTER_TTL_MS); + if (budget.versions.size === 0) accountModelsMisses.delete(credential.accountId); + else accountModelsMisses.set(credential.accountId, budget); + return admitted; +} + function candidateAccountIds(config: Pick): string[] { return [ MAIN_CODEX_ACCOUNT_ID, @@ -992,6 +1065,10 @@ export async function resolveCodexModelEntitlements( fetcher, now, clientVersion, + resolveCodexEntitlementClientVersion( + null, + options.loadPersistedRuntime ?? loadPersistedCodexRuntime, + ), options.credentialMutationEpoch, ), }))); @@ -1050,6 +1127,7 @@ export async function isDirectCallerEntitledToCodexModel( options.fetcher ?? fetch, options.now ?? Date.now(), clientVersion, + clientVersion, ); return codexModelEntitlementStateForRoster( result.models, @@ -1128,11 +1206,13 @@ export function invalidateCodexModelEntitlementsForAccount(accountId: string | n for (const key of [...accountModelsCache.keys()]) { if (accountIdOfCacheKey(key) === accountId) accountModelsCache.delete(key); } + accountModelsMisses.delete(accountId); } export function resetCodexModelEntitlementCacheForTests(): void { accountModelsCache.clear(); accountModelsFlights.clear(); + accountModelsMisses.clear(); negativeCredentialMemo.clear(); entitlementEnsureFlights.clear(); runtimeVersionMemo = null; diff --git a/structure/catalog.md b/structure/catalog.md index 0f995ec4fa..dfc1916684 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -81,6 +81,13 @@ mapped account reports it. A failed or malformed discovery is not positive evide hides the gated row until a later refresh. The same snapshot gates Pool selection, so the catalog and runtime cannot disagree by advertising through one account and dispatching through another. +`client_version` arrives on the inbound request and is part of that cache identity, so `src/codex/ +model-entitlements.ts` bounds the work as well as the state: stored versions per account, concurrent +roster flights per account, and distinct caller-selected versions admitted per account in one roster +window. Repeating a version already charged still retries on the failure TTL, and the locally +selected runtime version is never charged, so a legitimate refresh survives. Over the bound the +answer is unconfirmed, which hides the gated row rather than confirming a denial. + The app-server's model list comes from this shared catalog, not from patching the App. Codex Desktop may still apply its remote native-only allowlist after `model/list`; an explicitly configured combo `nativeAlias` is the bounded compatibility path. It replaces one supported bare native row with a diff --git a/tests/codex-integration/codex-model-entitlements.test.ts b/tests/codex-integration/codex-model-entitlements.test.ts index 219ec7dcc8..5acdbc47ed 100644 --- a/tests/codex-integration/codex-model-entitlements.test.ts +++ b/tests/codex-integration/codex-model-entitlements.test.ts @@ -1544,6 +1544,46 @@ describe("entitlement client version (#2886)", () => { expect(fetches).toBe(afterFill + 1); }); + test("completed caller-selected version misses are bounded per account", async () => { + // The cache budget bounds stored state and the flight budget bounds concurrency. Neither + // bounds COMPLETED work, so a caller cycling client_version and waiting for each answer could + // renew an authenticated upstream request under the account token as often as it liked. + let fetches = 0; + const backend = (async () => { fetches += 1; return roster(SOL); }) as typeof fetch; + const credentials = [{ + accountId: "pool-miss-budget", + accessToken: "tok-miss", + chatgptAccountId: "acct-miss", + credentialIdentity: "pool:1:acct-miss", + }]; + const ask = (clientVersion: string) => resolveCodexModelEntitlements({ codexAccounts: [] }, { + fetcher: backend, + now: 1_000, + clientVersion, + credentials, + loadPersistedRuntime: () => ({ selectedVersion: "0.300.0" }), + }); + + for (let i = 0; i < 8; i += 1) await ask(`0.${400 + i}.0`); + expect(fetches).toBe(4); + + // Over the allowance the answer is UNCONFIRMED - the same fail-closed shape a discovery + // failure produces, never a confirmed denial assembled from a roster nobody fetched. + const refused = await ask("0.499.0"); + expect(refused.confirmedAccountIds.has("pool-miss-budget")).toBe(false); + expect(fetches).toBe(4); + + // A version already charged keeps retrying. One client coming back every 15s on the failure + // TTL must not spend the allowance and lock itself out for the rest of the roster window. + await ask("0.400.0"); + expect(fetches).toBe(5); + + // The locally selected runtime version is never charged, so its legitimate refresh survives + // an untrusted caller spending everything else. + await ask("0.300.0"); + expect(fetches).toBe(6); + }); + test("the class budget counts accounts, not cached keys", async () => { // The documented budget is 64 ACCOUNTS per class. Counting keys instead would silently divide // that by the per-account version bound, so a deployment well inside the intended limit would From 1be399ae938f67bfbd823a2715c7e3f91fa0966e Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Mon, 14 Sep 2026 12:31:40 +0900 Subject: [PATCH 2/2] fix(codex): charge roster misses only after flight admission --- src/codex/model-entitlements.ts | 28 +++++++-------- structure/catalog.md | 7 ++-- .../codex-model-entitlements.test.ts | 35 +++++++++++++++++++ 3 files changed, 52 insertions(+), 18 deletions(-) diff --git a/src/codex/model-entitlements.ts b/src/codex/model-entitlements.ts index 6a4401db0f..790d598eaf 100644 --- a/src/codex/model-entitlements.ts +++ b/src/codex/model-entitlements.ts @@ -716,14 +716,12 @@ async function modelsForCredential( const existing = accountModelsFlights.get(flightKey); if (existing) return existing; - // Joining an in-flight request costs nothing upstream, so the allowance is spent only where a - // NEW upstream request would be opened. The locally selected runtime version is exempt: it has - // one stable key, and the legitimate refresh must survive an untrusted caller spending the rest. - if ( - !credential.accountId.startsWith(DIRECT_CALLER_ACCOUNT_PREFIX) - && clientVersion !== trustedClientVersion - && !admitVersionMiss(credential, clientVersion, now) - ) { + // Bound concurrency per account before opening another upstream request. + let liveForAccount = 0; + for (const key of accountModelsFlights.keys()) { + if (accountIdOfCacheKey(key) === credential.accountId) liveForAccount += 1; + } + if (liveForAccount >= MODEL_ROSTER_FLIGHTS_PER_ACCOUNT_MAX) { return { credentialIdentity: credential.credentialIdentity, clientVersion, @@ -732,13 +730,13 @@ async function modelsForCredential( confirmed: false, }; } - - // Bound concurrency per account before opening another upstream request. - let liveForAccount = 0; - for (const key of accountModelsFlights.keys()) { - if (accountIdOfCacheKey(key) === credential.accountId) liveForAccount += 1; - } - if (liveForAccount >= MODEL_ROSTER_FLIGHTS_PER_ACCOUNT_MAX) { + // Cache hits, joined flights and capacity refusals start no upstream request. Charge only + // after capacity admission; the locally selected runtime version remains exempt. + if ( + !credential.accountId.startsWith(DIRECT_CALLER_ACCOUNT_PREFIX) + && clientVersion !== trustedClientVersion + && !admitVersionMiss(credential, clientVersion, now) + ) { return { credentialIdentity: credential.credentialIdentity, clientVersion, diff --git a/structure/catalog.md b/structure/catalog.md index dfc1916684..47a827426b 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -81,12 +81,13 @@ mapped account reports it. A failed or malformed discovery is not positive evide hides the gated row until a later refresh. The same snapshot gates Pool selection, so the catalog and runtime cannot disagree by advertising through one account and dispatching through another. -`client_version` arrives on the inbound request and is part of that cache identity, so `src/codex/ -model-entitlements.ts` bounds the work as well as the state: stored versions per account, concurrent +`client_version` arrives on the inbound request and is part of that cache identity, so +`src/codex/model-entitlements.ts` bounds the work as well as the state: stored versions per account, concurrent roster flights per account, and distinct caller-selected versions admitted per account in one roster window. Repeating a version already charged still retries on the failure TTL, and the locally selected runtime version is never charged, so a legitimate refresh survives. Over the bound the -answer is unconfirmed, which hides the gated row rather than confirming a denial. +answer is unconfirmed, which hides the gated row rather than confirming a denial. Flight capacity +is checked before charging a distinct version, so a capacity refusal consumes no miss allowance. The app-server's model list comes from this shared catalog, not from patching the App. Codex Desktop may still apply its remote native-only allowlist after `model/list`; an explicitly configured combo diff --git a/tests/codex-integration/codex-model-entitlements.test.ts b/tests/codex-integration/codex-model-entitlements.test.ts index 5acdbc47ed..926f1994a7 100644 --- a/tests/codex-integration/codex-model-entitlements.test.ts +++ b/tests/codex-integration/codex-model-entitlements.test.ts @@ -1544,6 +1544,41 @@ describe("entitlement client version (#2886)", () => { expect(fetches).toBe(afterFill + 1); }); + test("capacity-rejected versions do not consume the miss allowance", async () => { + let fetches = 0; + let hold = true; + const release: Array<() => void> = []; + const backend = (async () => { + fetches += 1; + if (hold) await new Promise(resolve => release.push(resolve)); + return roster(SOL); + }) as typeof fetch; + const credentials = [credential("pool-capacity-budget")]; + const ask = (clientVersion: string) => resolveCodexModelEntitlements({ codexAccounts: [] }, { + fetcher: backend, now: 1_000, clientVersion, credentials, + loadPersistedRuntime: () => ({ selectedVersion: "0.300.0" }), + }); + const pending = ["0.300.0", "0.400.0", "0.401.0", "0.402.0"].map(ask); + try { + for (let i = 0; i < 100 && release.length < 4; i += 1) { + await new Promise(resolve => setTimeout(resolve, 0)); + } + expect(fetches).toBe(4); + const rejected = await ask("0.403.0"); + expect(rejected.confirmedAccountIds.has("pool-capacity-budget")).toBe(false); + expect(fetches).toBe(4); + } finally { + hold = false; + for (const resolve of release) resolve(); + await Promise.all(pending); + } + // Only three caller-selected versions opened a flight. A different fourth version + // must still be admitted once capacity is free; the rejected attempt spent nothing. + const admitted = await ask("0.404.0"); + expect(fetches).toBe(5); + expect(admitted.confirmedAccountIds.has("pool-capacity-budget")).toBe(true); + }); + test("completed caller-selected version misses are bounded per account", async () => { // The cache budget bounds stored state and the flight budget bounds concurrency. Neither // bounds COMPLETED work, so a caller cycling client_version and waiting for each answer could