From 7a01f3fe0992bc3cdbce97663fb5d08d2b910934 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 14 Sep 2026 12:48:10 +0900 Subject: [PATCH 1/3] fix(providers): close the Devin rekey host window and diagnose Antigravity quota blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The devin-cli -> devin merge saves the config row synchronously but rekeys the credential slot detached, so resolveDevinApiServer read an empty "devin" slot and sent EU/FedStart keys to the US default until the rekey landed — or forever when it failed or refused on collision. The literal slot is still consulted first; only when it yields no usable apiBaseUrl are the slots linked by DEPRECATED_OAUTH_PROVIDER_ALIASES consulted, each through the same allowlist. The Antigravity quota probe discarded the summary attempt's classified failure, so a Fake-IP destination block was reported as whatever vaguer diagnosis the models fallback produced. The summary's network-policy diagnosis now outranks a non-policy fallback diagnosis; a successful fallback still clears it entirely. Closes #4503 Closes #3781 --- .../000_plan.md | 35 +++++ .../010_wp1_account_lifecycle_edges.md | 54 +++++++ src/oauth/devin.ts | 57 +++++-- src/providers/quota.ts | 43 +++++- tests/providers/devin-adapter.test.ts | 127 ++++++++++++++- tests/providers/devin-login.test.ts | 69 +++++++++ .../providers/provider-account-quota.test.ts | 146 ++++++++++++++++++ .../chat-completions-endpoint.test.ts | 38 +++++ 8 files changed, 549 insertions(+), 20 deletions(-) create mode 100644 devlog/_plan/260914_l5_provider_account_edges/000_plan.md create mode 100644 devlog/_plan/260914_l5_provider_account_edges/010_wp1_account_lifecycle_edges.md diff --git a/devlog/_plan/260914_l5_provider_account_edges/000_plan.md b/devlog/_plan/260914_l5_provider_account_edges/000_plan.md new file mode 100644 index 00000000000..c7dc7f872ea --- /dev/null +++ b/devlog/_plan/260914_l5_provider_account_edges/000_plan.md @@ -0,0 +1,35 @@ +# 260914 L5 — provider account lifecycle edges (#4503, #3781) + +R1 라운드의 L5 레인. 계정 수명주기 경계에서 생긴 두 건을 한 PR로 닫는다. +분기: `codex/260914-l5-provider-account-edges`, 타깃 `dev`. + +## 다루는 것 + +- **#4503** Devin 프로바이더 병합 마이그레이션이 남긴 host-selection 창. + config 저장은 동기인데 credential rekey는 detached라, 그 사이(그리고 rekey가 + 실패하거나 collision으로 거부되면 그 프로세스 내내) EU/FedStart 테넌트가 US + 기본 호스트로 키를 보낸다. +- **#4503 부록** 같은 감사에서 함께 기록된 커버리지 공백. Pi-shape 이미지 파트의 + tool 경로가 합성으로만 덮여 있어, tool 분기 한정 회귀는 잡히지 않는다. +- **#3781** Antigravity 할당량 갱신 실패. canonical Fake-IP 처리 가설을 실제 + 소스에서 확인하고, 남은 구멍과 커버리지를 메운다. + +## 레인 경계 + +쓰기 가능: `src/oauth/devin.ts`, `src/providers/quota.ts`의 Antigravity 블록, +그리고 위 서브시스템의 테스트. + +쓰면 안 되는 것: account pool 커널, `src/codex/routing.ts`, +`src/server/responses/*`, `src/codex/catalog/*`, `src/adapters/cursor/*`, `gui/`. +같은 라운드의 다른 레인이 별도 워크트리에서 그 경로들을 소유한다. + +## 검증 방침 + +로컬 스위트/타입체크/설치는 레인 제약으로 **실행하지 않는다**. `node_modules`도 +없다. 증거는 최종 head의 hosted CI 하나뿐이다. 그래서 구현은 타입체커 대신 +기존 파일의 import 경로/타입 이름/strict null 처리를 그대로 맞추는 방식으로 간다. + +## 작업 단위 + +- `010_wp1_account_lifecycle_edges.md` — 단일 work-phase. 여섯 개 서브에이전트에 + 서로 겹치지 않는 write scope를 배정해 병렬로 구현한다. diff --git a/devlog/_plan/260914_l5_provider_account_edges/010_wp1_account_lifecycle_edges.md b/devlog/_plan/260914_l5_provider_account_edges/010_wp1_account_lifecycle_edges.md new file mode 100644 index 00000000000..4ce068936d5 --- /dev/null +++ b/devlog/_plan/260914_l5_provider_account_edges/010_wp1_account_lifecycle_edges.md @@ -0,0 +1,54 @@ +# 010 — wp1: 계정 수명주기 경계 (#4503, #3781) + +## MODIFY: `src/oauth/devin.ts` — `resolveDevinApiServer` + +현재는 `getCredential(providerId)`를 **리터럴 슬롯 키**로 읽는다. 병합 +마이그레이션(`runDevinProviderMergeStartupMigration`)은 `providers["devin"]`을 +동기로 저장한 뒤 `void rekeyProviderCredentials("devin-cli","devin")`을 detached로 +던진다. 그래서 config 행은 이미 `devin`인데 credential은 아직 `devin-cli` 슬롯에 +있는 창이 생기고, rekey가 실패하거나 collision으로 거부되면 그 상태가 그 프로세스 +동안 계속된다. 그 사이 `getCredential("devin")`은 undefined라 EU/FedStart 테넌트가 +configured baseUrl 또는 `DEVIN_DEFAULT_API_SERVER`(US)로 떨어진다. + +계약: + +1. 요청받은 providerId의 **리터럴 슬롯을 먼저** 본다. 아직 `devin-cli`로 남아 있는 + config 행은 자기 슬롯을 읽어야 하므로, 앞단에서 id를 정규화하면 오히려 틀린 + 슬롯을 읽는다. 기존 주석의 그 논거는 유지하고 확장한다. +2. 리터럴 슬롯에 쓸 만한 `apiBaseUrl`이 없을 때만 `DEPRECATED_OAUTH_PROVIDER_ALIASES`가 + 묶어 둔 슬롯을 **양방향**으로 더 본다 (`devin` → `devin-cli`, `devin-cli` → `devin`). + 두 번째 문자열 리터럴을 박지 않고 alias 맵에서 유도해, 맵이 단일 출처로 남게 한다. +3. 후보는 모두 `validateDevinApiBaseUrl`을 통과해야 한다. alias 슬롯을 리터럴보다 + 더 신뢰하지 않는다. +4. 이후 순서는 그대로: configured baseUrl → `DEVIN_DEFAULT_API_SERVER`. +5. 시그니처와 기존 호출부는 불변. + +## MODIFY: `src/providers/quota.ts` — Antigravity 블록만 + +`probeAntigravityUsageQuota`의 summary 프로브는 바인딩도 본문도 없는 빈 catch로 받아서 +분류된 진단을 통째로 버린다. summary가 outbound 정책(`destination_blocked`)이나 DNS(`dns_failed`)로 +막히는 건 정확히 이 이슈가 말하는 Fake-IP 증상인데, fallback까지 실패하면 사용자에게는 +더 두루뭉술한 쪽(`upstream_error`, `response_unusable`)만 보인다. + +summary의 분류 결과를 기억해 두고, fallback도 unavailable로 끝났을 때 summary 쪽이 +네트워크 정책 진단이고 fallback 쪽이 아니면 summary 진단을 택한다. 보존 조건: +fallback이 성공하면 첫 실패는 완전히 지워진다, summary의 즉시 반환 +(`redirect_blocked`/`access_denied`)은 그대로, `legacy` 채널의 모양과 +`rejects.toBe(error)` 동일성은 건드리지 않는다, 진단 값은 닫힌 +`QUOTA_FAILURE_CODES` 밖으로 나가지 않는다. + +## 회귀 테스트 + +새 테스트 파일은 만들지 않는다 (test-layout 게이트와 그 fixture를 건드리지 않기 위해). + +- `tests/providers/devin-login.test.ts` — 마이그레이션 창에서 `devin` 요청이 + `devin-cli` 슬롯의 테넌트 호스트를 읽는지, 리터럴 슬롯 우선순위가 유지되는지, + 잘못된 alias `apiBaseUrl`이 신뢰받지 않는지. +- `tests/providers/devin-adapter.test.ts` — 같은 보장을 어댑터가 실제로 디스패치하는 + 호스트 수준에서. +- `tests/providers/provider-account-quota.test.ts` — 프록시 없이 Fake-IP DNS 응답이 + canonical 할당량 URL 두 개에 대해 허용되는지, 예외가 lookalike 호스트/다른 경로/쿼리 + 추가/다른 프로바이더 이름으로 넓어지지 않는지, 무관한 private·metadata 응답은 여전히 + 거부되고 안전한 `destination_blocked`로 보고되는지. +- `tests/responses/chat-completions-endpoint.test.ts` — #4503 부록. 직접 + `role:"tool"` 봉투에 실린 Pi-shape 이미지 파트 fixture. diff --git a/src/oauth/devin.ts b/src/oauth/devin.ts index d661b9e0f22..fe8150abcb3 100644 --- a/src/oauth/devin.ts +++ b/src/oauth/devin.ts @@ -20,9 +20,28 @@ import { registerUser } from "./devin/register-user"; import { DEVIN_DEFAULT_API_SERVER, resolveDevinApiBaseUrl, validateDevinApiBaseUrl } from "./devin/api-base"; import { readDevinCliCredentialOutcome } from "./devin/cli-import"; import { getCredential } from "./store"; +import { DEPRECATED_OAUTH_PROVIDER_ALIASES } from "./index"; export { DEVIN_DEFAULT_API_SERVER } from "./devin/api-base"; +/** + * Credential slots the deprecated-alias map ties to `providerId`, in both + * directions: a deprecated id also reads its destination's slot, and a merge + * destination also reads every deprecated source slot pointing at it. Derived + * from DEPRECATED_OAUTH_PROVIDER_ALIASES rather than a second "devin-cli" + * literal so the map stays the single source of truth — a hard-coded pair here + * would drift the day another alias is added. + */ +function devinAliasCredentialSlots(providerId: string): string[] { + const slots: string[] = []; + const destination = DEPRECATED_OAUTH_PROVIDER_ALIASES[providerId]; + if (destination !== undefined) slots.push(destination); + for (const [alias, target] of Object.entries(DEPRECATED_OAUTH_PROVIDER_ALIASES)) { + if (target === providerId && alias !== providerId) slots.push(alias); + } + return slots; +} + /** * The api-server host this account must talk to. * @@ -33,18 +52,32 @@ export { DEVIN_DEFAULT_API_SERVER } from "./devin/api-base"; * network value. */ export function resolveDevinApiServer(configuredBaseUrl?: string, providerId = "devin"): string { - return ( - // Provider-scoped, keyed by the configured provider id verbatim. `devin-cli` - // is a deprecated alias for `devin`, but an unmigrated config row still owns - // its old credential slot until the startup migration rekeys the row and the - // slot together — normalizing the id here would read the wrong slot for that - // window. An EU or FedStart tenant is recorded on the credential rather than - // in the registry, so a fixed "devin" slot would send the key to the wrong - // host either way. - validateDevinApiBaseUrl(getCredential(providerId)?.apiBaseUrl) ?? - validateDevinApiBaseUrl(configuredBaseUrl) ?? - DEVIN_DEFAULT_API_SERVER - ); + // Provider-scoped, keyed by the configured provider id verbatim and consulted + // FIRST. `devin-cli` is a deprecated alias for `devin`, but an unmigrated + // config row still owns its old credential slot until the startup migration + // rekeys the row and the slot together — normalizing the id here would read + // the wrong slot for that window. An EU or FedStart tenant is recorded on the + // credential rather than in the registry, so a fixed "devin" slot would send + // the key to the wrong host either way. + const literal = validateDevinApiBaseUrl(getCredential(providerId)?.apiBaseUrl); + if (literal !== undefined) return literal; + + // The startup merge saves providers["devin"] synchronously but fires the + // credential rekey detached — runDevinProviderMergeStartupMigration cannot + // await inside the synchronous startServer window — so the row can already + // say "devin" while the credential still sits in the "devin-cli" slot, and it + // stays that way for the whole process when the rekey fails or refuses on an + // occupied destination slot. Reading the alias-linked slots in both + // directions closes that window: "devin" finds the not-yet-rekeyed + // "devin-cli" credential, and a lingering "devin-cli" row finds a credential + // already rekeyed to "devin". Every candidate passes the same allowlist — an + // alias slot is not trusted more than the literal one. + for (const slot of devinAliasCredentialSlots(providerId)) { + const host = validateDevinApiBaseUrl(getCredential(slot)?.apiBaseUrl); + if (host !== undefined) return host; + } + + return validateDevinApiBaseUrl(configuredBaseUrl) ?? DEVIN_DEFAULT_API_SERVER; } function decodeJwtPayload(token: string): Record | undefined { diff --git a/src/providers/quota.ts b/src/providers/quota.ts index 1ab17a36034..386e9213918 100644 --- a/src/providers/quota.ts +++ b/src/providers/quota.ts @@ -2951,7 +2951,26 @@ function unavailableAntigravityQuota(failure: QuotaFailureCode): AntigravityQuot return { kind: "unavailable", failure, legacy: { kind: "null" } }; } -/** Final attempt determines the safe diagnosis; a successful fallback clears the first failure. */ +/** + * Prefer a summary network-policy diagnosis over a vaguer fallback. A blocked + * destination is an actionable local-network fact, while "upstream_error" tells + * the operator to go look at Google. A successful models probe still clears + * the first failure completely. + */ +function antigravityUnavailableFailure( + summaryFailure: QuotaFailureCode | undefined, + fallbackFailure: QuotaFailureCode, +): QuotaFailureCode { + if ( + (summaryFailure === "destination_blocked" || summaryFailure === "dns_failed") + && fallbackFailure !== "destination_blocked" + && fallbackFailure !== "dns_failed" + ) { + return summaryFailure; + } + return fallbackFailure; +} + async function probeAntigravityUsageQuota(accessToken: string, projectId: string): Promise { const fetchQuota = (url: string) => providerOutboundPost("google-antigravity", { baseUrl: ANTIGRAVITY_ACCOUNT_QUOTA_BASE }, url, { headers: { @@ -2960,6 +2979,7 @@ async function probeAntigravityUsageQuota(accessToken: string, projectId: string }, body: JSON.stringify({ project: projectId }), signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), }, antigravityOutboundDependencies); + let summaryFailure: QuotaFailureCode | undefined; try { const response = await fetchQuota(ANTIGRAVITY_QUOTA_SUMMARY_URL); if (await providerRedirectError(response, ANTIGRAVITY_QUOTA_SUMMARY_URL)) return unavailableAntigravityQuota("redirect_blocked"); @@ -2968,19 +2988,30 @@ async function probeAntigravityUsageQuota(accessToken: string, projectId: string const quota = parseAntigravityQuotaSummary(asRecord(await readQuotaJson(response))); if (quota) return { kind: "available", quota, source: "google-antigravity:retrieveUserQuotaSummary" }; } - } catch { + } catch (error) { // Existing behavior: summary transport/parse failure may recover through the models probe. + summaryFailure = quotaTransportFailure(error); } try { const response = await fetchQuota(ANTIGRAVITY_QUOTA_MODELS_URL); - if (await providerRedirectError(response, ANTIGRAVITY_QUOTA_MODELS_URL)) return unavailableAntigravityQuota("redirect_blocked"); - if (!response.ok) return unavailableAntigravityQuota(quotaHttpFailure(response.status)); + if (await providerRedirectError(response, ANTIGRAVITY_QUOTA_MODELS_URL)) { + return unavailableAntigravityQuota(antigravityUnavailableFailure(summaryFailure, "redirect_blocked")); + } + if (!response.ok) { + return unavailableAntigravityQuota(antigravityUnavailableFailure(summaryFailure, quotaHttpFailure(response.status))); + } const customWindows = antigravityWindowsFromModels(asRecord(await readQuotaJson(response))); - if (!customWindows.length) return unavailableAntigravityQuota("response_unusable"); + if (!customWindows.length) { + return unavailableAntigravityQuota(antigravityUnavailableFailure(summaryFailure, "response_unusable")); + } return { kind: "available", quota: { customWindows, updatedAt: Date.now() }, source: "google-antigravity:fetchAvailableModels" }; } catch (error) { // The public compatibility wrapper still rejects this exact fallback error; it never enters a DTO. - return { kind: "unavailable", failure: quotaTransportFailure(error), legacy: { kind: "throw", error } }; + return { + kind: "unavailable", + failure: antigravityUnavailableFailure(summaryFailure, quotaTransportFailure(error)), + legacy: { kind: "throw", error }, + }; } } diff --git a/tests/providers/devin-adapter.test.ts b/tests/providers/devin-adapter.test.ts index 743887f9dcd..a50a3d96e4d 100644 --- a/tests/providers/devin-adapter.test.ts +++ b/tests/providers/devin-adapter.test.ts @@ -1,12 +1,19 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { createDevinAdapter, mapDevinToolCallStartForTests, mapOcxMessagesToDevin, mapOcxToolsToDevin, resolveWireModelUidForTests } from "../../src/adapters/devin"; import { sanitizeToolDescriptionForCognitionForTests } from "../../src/adapters/devin/cloud-direct/chat"; import { DEVIN_MODEL_CONTEXT_WINDOWS, DEVIN_STATIC_MODELS, collapseDevinModelUid } from "../../src/adapters/devin/live-models"; import { parseCatalogBuffer } from "../../src/adapters/devin/cloud-direct/catalog"; import { encodeMessage, encodeString, encodeVarintField } from "../../src/adapters/devin/cloud-direct/wire"; import { DEPRECATED_OAUTH_PROVIDER_ALIASES, OAUTH_PROVIDERS, resolveRefreshPolicy } from "../../src/oauth"; +import { DEVIN_DEFAULT_API_SERVER } from "../../src/oauth/devin"; +import { saveCredential } from "../../src/oauth/store"; +import { createTranslatorBudget } from "../../src/lib/translator-budget"; import { PROVIDER_REGISTRY } from "../../src/providers/registry"; -import type { OcxParsedRequest } from "../../src/types"; +import type { AdapterEvent, OcxParsedRequest } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; describe("devin adapter", () => { test("is registered as an oauth provider and adapter", () => { @@ -449,3 +456,119 @@ describe("effort suffix detection and caller effort values are different sets", } }); }); + +describe("devin adapter api-server host resolution (#4503)", () => { + // The `devin-cli` -> `devin` merge rekeys a config row and its credential + // slot together at startup, so until that migration runs a row already named + // `devin` can have its only credential — and the tenant apiBaseUrl recorded + // on it — still sitting under the `devin-cli` slot. runTurn resolves the + // dispatch host through resolveDevinApiServer(provider.baseUrl, + // credentialProviderId), which must follow the DEPRECATED_OAUTH_PROVIDER_ALIASES + // link to that slot before falling back to the configured baseUrl and then + // the US default. Without it an EU/FedStart tenant's traffic — api_key + // included — is sent to a host the account is not provisioned on. + const EU_TENANT_HOST = "https://eu.windsurf.com/_route/api_server"; + const FEDSTART_TENANT_HOST = "https://windsurf.fedstart.com/_route/api_server"; + // A valid, non-default configured baseUrl. If the credential slots were + // skipped the adapter would dispatch here; if baseUrl were also skipped it + // would land on DEVIN_DEFAULT_API_SERVER. The assertions below reject both. + const CONFIGURED_BASE_URL = "https://server-staging.codeium.com"; + + const previousHome = process.env.OPENCODEX_HOME; + const previousFetch = globalThis.fetch; + let home = ""; + let seenUrls: string[] = []; + + beforeEach(() => { + home = mkdtempSync(join(tmpdir(), "ocx-devin-host-")); + process.env.OPENCODEX_HOME = home; + seenUrls = []; + // This adapter's transport fetches through the global fetch — it does not + // consume IncomingMeta.providerFetch — so the stub observes every upstream + // URL the turn dispatches to. + globalThis.fetch = (async (input: RequestInfo | URL) => { + seenUrls.push(String(input)); + return new Response("down", { status: 500 }); + }) as typeof fetch; + }); + + afterEach(() => { + globalThis.fetch = previousFetch; + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(home); + }); + + // Drive one real runTurn. The stubbed 500 ends the turn in an upstream error + // only after every outbound URL has been recorded. + async function runOneTurn(apiKey: string): Promise { + const adapter = createDevinAdapter( + { adapter: "devin", baseUrl: CONFIGURED_BASE_URL, apiKey }, + { providerId: "devin" }, + ); + const parsed: OcxParsedRequest = { + modelId: "swe-2-high", + stream: true, + context: { messages: [{ role: "user", content: "hi", timestamp: 1 }] }, + options: {}, + }; + const events: AdapterEvent[] = []; + await adapter.runTurn!( + parsed, + { headers: new Headers(), translatorBudget: createTranslatorBudget() }, + (event) => events.push(event), + ); + return events; + } + + function expectDispatchedTo(host: string): void { + expect(seenUrls.length).toBeGreaterThan(0); + for (const url of seenUrls) expect(url).toStartWith(host); + } + + test("a devin row adopts the tenant host from an un-rekeyed devin-cli credential", async () => { + await saveCredential("devin-cli", { + access: "devin-cli-session", + refresh: "devin-cli-session", + expires: Number.MAX_SAFE_INTEGER, + source: "local-cli", + apiBaseUrl: EU_TENANT_HOST, + }); + + const events = await runOneTurn("ocx-test-alias-slot-key"); + + expectDispatchedTo(EU_TENANT_HOST); + expect(seenUrls.some((url) => url.startsWith(DEVIN_DEFAULT_API_SERVER))).toBe(false); + expect(seenUrls.some((url) => url.startsWith(CONFIGURED_BASE_URL))).toBe(false); + // The turn reached the transport and failed there on the stubbed 500 — + // proof the recorded URLs came from a real dispatch, not an early return. + expect(events.some((event) => event.type === "error")).toBe(true); + }); + + test("a usable literal devin slot still wins over the aliased devin-cli slot", async () => { + await saveCredential("devin", { + access: "devin-session", + refresh: "devin-session", + expires: Number.MAX_SAFE_INTEGER, + source: "oauth", + apiBaseUrl: FEDSTART_TENANT_HOST, + }); + await saveCredential("devin-cli", { + access: "devin-cli-session", + refresh: "devin-cli-session", + expires: Number.MAX_SAFE_INTEGER, + source: "local-cli", + apiBaseUrl: EU_TENANT_HOST, + }); + + await runOneTurn("ocx-test-literal-slot-key"); + + expectDispatchedTo(FEDSTART_TENANT_HOST); + }); + + test("with neither credential slot populated the configured baseUrl still applies", async () => { + await runOneTurn("ocx-test-no-credential-key"); + + expectDispatchedTo(CONFIGURED_BASE_URL); + }); +}); diff --git a/tests/providers/devin-login.test.ts b/tests/providers/devin-login.test.ts index 6f9286a7b68..db7922366b8 100644 --- a/tests/providers/devin-login.test.ts +++ b/tests/providers/devin-login.test.ts @@ -11,6 +11,7 @@ import { type DevinCliLoginDeps, } from "../../src/oauth/devin/cli-import"; import { loginDevin, refreshDevinToken, resolveDevinApiServer } from "../../src/oauth/devin"; +import { saveCredential } from "../../src/oauth/store"; import type { OAuthController } from "../../src/oauth/types"; /** @@ -207,6 +208,38 @@ describe("devin merged login is import-first", () => { }); describe("devin tenant selection is provider-scoped", () => { + // resolveDevinApiServer reads auth.json through getCredential. Isolate the + // home so these cases cannot pick up a live Devin login, and so seeding a + // slot cannot write the operator's real store. + const tmp = mkdtempSync(join(tmpdir(), "ocx-devin-tenant-")); + let savedHome: string | undefined; + + const EU_HOST = "https://eu.windsurf.com/_route/api_server"; + const FEDSTART_HOST = "https://windsurf.fedstart.com/_route/api_server"; + const US_HOST = "https://server.codeium.com"; + + async function seedSlot(provider: string, apiBaseUrl: string) { + await saveCredential(provider, { + access: KEY, + refresh: KEY, + expires: Number.MAX_SAFE_INTEGER, + source: "local-cli", + apiBaseUrl, + }); + } + + beforeEach(() => { + savedHome = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = tmp; + rmSync(join(tmp, "auth.json"), { force: true }); + }); + + afterEach(() => { + rmSync(join(tmp, "auth.json"), { force: true }); + if (savedHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = savedHome; + }); + test("the default still reads the devin slot", () => { // Every existing one-argument caller must keep its behaviour. expect(resolveDevinApiServer("https://server.codeium.com")).toBe("https://server.codeium.com"); @@ -219,6 +252,42 @@ describe("devin tenant selection is provider-scoped", () => { // read the same place rather than an orphaned slot. expect(resolveDevinApiServer(undefined, "devin-cli")).toBe("https://server.codeium.com"); }); + + test("an unmigrated EU tenant on the alias slot is used after the config row is rewritten", async () => { + // The config rewriter can land providers["devin"] while rekeyProviderCredentials + // has not yet moved the auth slot. Asking for "devin" must still find the + // tenant host sitting on "devin-cli"; otherwise the key is sent to the US + // default and Cognition answers permission_denied. + await seedSlot("devin-cli", EU_HOST); + expect(resolveDevinApiServer(undefined, "devin")).toBe(EU_HOST); + }); + + test("the signed-in alias tenant wins over a configured baseUrl", async () => { + // RegisterUser recorded the tenant on the credential. A leftover US + // baseUrl on the rewritten config row must not override that account. + await seedSlot("devin-cli", EU_HOST); + expect(resolveDevinApiServer(US_HOST, "devin")).toBe(EU_HOST); + }); + + test("the literal slot wins when both alias ids hold a tenant", async () => { + // An unmigrated "devin-cli" row must keep reading its own slot even if a + // "devin" credential already exists; swapping them would send each key + // to the other account's host. + await seedSlot("devin", EU_HOST); + await seedSlot("devin-cli", FEDSTART_HOST); + expect(resolveDevinApiServer(undefined, "devin")).toBe(EU_HOST); + expect(resolveDevinApiServer(undefined, "devin-cli")).toBe(FEDSTART_HOST); + }); + + test("an alias slot with a non-Devin apiBaseUrl is not trusted", async () => { + // The store allowlists Copilot and Devin together, so a Copilot origin is + // the host that survives persist and still fails validateDevinApiBaseUrl. + // Without that check on the alias candidate, the merge window would send + // a Devin key to GitHub. + await seedSlot("devin-cli", "https://api.githubcopilot.com"); + expect(resolveDevinApiServer(EU_HOST, "devin")).toBe(EU_HOST); + expect(resolveDevinApiServer(undefined, "devin")).toBe(US_HOST); + }); }); describe("devin-cli credential path and read bounds", () => { diff --git a/tests/providers/provider-account-quota.test.ts b/tests/providers/provider-account-quota.test.ts index bf4855caa51..2ac6489e839 100644 --- a/tests/providers/provider-account-quota.test.ts +++ b/tests/providers/provider-account-quota.test.ts @@ -963,3 +963,149 @@ describe("google-antigravity per-account quota (#1082)", () => { } }); }); + +describe("google-antigravity Fake-IP TUN quota probes without HTTP proxy (#3781)", () => { + // Clash/Surge/Mihomo TUN intercepts 198.18/15 (and Mihomo IPv6 fake-IP) without an + // outbound HTTP(S) proxy env. Quota probing has to arm the same canonical-URL proof + // model discovery already had; without it the resolver rejects the fake-IP answer and + // the Accounts page surfaces "quota refresh failed". The exception is URL-exact and + // per-answer: a lookalike host, a different path, a query string, or a differently + // named provider never arms it, and loopback/RFC1918/link-local/metadata companions + // still fail closed as destination_blocked. + const { setAntigravityAccountQuotaTransportForTests, isCanonicalAntigravityQuotaUrl } = require("../../src/providers/quota") as typeof import("../../src/providers/quota"); + const idFor = (email: string) => getAccountSet("google-antigravity")!.accounts.find(a => a.credential.email === email)!.id; + + function antigravityBody(gemRemaining: number, claRemaining: number): string { + return JSON.stringify({ + models: { + "gemini-3.7-flash": { displayName: "Gemini 3.7 Flash", quotaInfo: { remainingFraction: gemRemaining, resetTime: "2026-09-02T12:00:00Z" } }, + "claude-opus-5": { displayName: "Claude Opus 5", quotaInfo: { remainingFraction: claRemaining, resetTime: "2026-09-02T18:00:00Z" } }, + }, + }); + } + + function antigravitySummaryBody(gemRemaining: number, claRemaining: number): string { + return JSON.stringify({ + groups: [ + { + displayName: "Gemini Models", + buckets: [ + { bucketId: "gemini-weekly", window: "weekly", remainingFraction: gemRemaining, resetTime: "2026-09-09T12:00:00Z" }, + { bucketId: "gemini-5h", window: "5h", remainingFraction: gemRemaining, resetTime: "2026-09-02T12:00:00Z" }, + ], + }, + { + displayName: "Claude and GPT models", + buckets: [ + { bucketId: "3p-weekly", window: "weekly", remainingFraction: claRemaining, resetTime: "2026-09-09T18:00:00Z" }, + { bucketId: "3p-5h", window: "5h", remainingFraction: claRemaining, resetTime: "2026-09-02T18:00:00Z" }, + ], + }, + ], + }); + } + + const proxyKeys = PROXY_ENV_KEYS.flatMap(key => [key, key.toLowerCase()]); + const originalProxyEnv = Object.fromEntries(proxyKeys.map(key => [key, process.env[key]])); + const summaryUrl = "https://daily-cloudcode-pa.googleapis.com/v1internal:retrieveUserQuotaSummary"; + const modelsUrl = "https://daily-cloudcode-pa.googleapis.com/v1internal:fetchAvailableModels"; + const fakeIp = "198.18.56.214"; + + beforeEach(() => { + for (const key of proxyKeys) delete process.env[key]; + }); + afterEach(() => { + setAntigravityAccountQuotaTransportForTests(null); + for (const key of proxyKeys) { + if (originalProxyEnv[key] === undefined) delete process.env[key]; + else process.env[key] = originalProxyEnv[key]; + } + }); + + for (const fallback of [false, true]) { + test(`canonical ${fallback ? "fetchAvailableModels fallback" : "retrieveUserQuotaSummary"} admits Fake-IP DNS with no HTTP proxy`, async () => { + await saveCredential("google-antigravity", { access: "agy-first", refresh: "r1", expires: Date.now() + 3600_000, projectId: "proj-first", accountId: "agy-a", email: "a@example.com" }); + for (const key of proxyKeys) expect(process.env[key]).toBeUndefined(); + let plainFetchCalls = 0; + globalThis.fetch = (async () => { plainFetchCalls += 1; throw new Error("unexpected raw quota fetch"); }) as typeof fetch; + const resolved: Array<{ url: string; benchmark?: boolean; private?: boolean; mihomo?: boolean }> = []; + const posted: Array<{ url: string; address: string; tls?: boolean; auth: string | null; project: string }> = []; + // A caller that forgets the canonical-URL seam must fail closed. The test seam + // overwrites isCanonicalUrl with the production proof, so injecting `() => false` + // still has to admit the fake-IP answer for the two accounting URLs. + setAntigravityAccountQuotaTransportForTests({ isCanonicalUrl: () => false }); + setAntigravityAccountQuotaTransportForTests({ + resolveAddresses: async (url, options) => { + const policy = typeof options === "object" ? options : undefined; + resolved.push({ url, benchmark: policy?.allowBenchmarkAddresses, private: policy?.allowPrivateNetwork, mihomo: policy?.allowMihomoIpv6FakeIp }); + if (!policy?.allowBenchmarkAddresses) throw new Error("benchmark address rejected"); + return { hostname: "daily-cloudcode-pa.googleapis.com", addresses: [{ address: fakeIp, family: 4 }], privateNetwork: false }; + }, + pinnedPost: async (url, pinned, body, _signal, options) => { + posted.push({ url, address: pinned.address, tls: options?.rejectUnauthorized, auth: new Headers(options?.headers).get("authorization"), project: String(JSON.parse(String(body)).project) }); + if (url === summaryUrl && fallback) return new Response(null, { status: 404 }); + return new Response(url === summaryUrl ? antigravitySummaryBody(0.86, 0.38) : antigravityBody(0.86, 0.38)); + }, + }); + const rows = await fetchProviderAccountQuotas("google-antigravity"); + const urls = fallback ? [summaryUrl, modelsUrl] : [summaryUrl]; + expect(resolved).toEqual(urls.map(url => ({ url, benchmark: true, private: false, mihomo: true }))); + expect(posted).toEqual(urls.map(url => ({ url, address: fakeIp, tls: true, auth: "Bearer agy-first", project: "proj-first" }))); + expect(rows).toHaveLength(1); + expect(rows[0]!.accountId).toBe(idFor("a@example.com")); + expect(rows[0]!.unavailable).toBeUndefined(); + expect(rows[0]!.quotaFailure).toBeUndefined(); + expect(rows[0]!.quota?.customWindows?.map(w => `${w.label}=${w.percent}`)).toEqual(fallback ? ["Gem=14", "Cla=62"] : ["Gem=14", "Gem (Weekly)=14", "Cla=62", "Cla (Weekly)=62"]); + expect(plainFetchCalls).toBe(0); + }); + } + + test("canonical proof rejects a lookalike host, a different path, a query string, and a differently named provider", () => { + const lookalikeHost = summaryUrl.replace(".googleapis.com", ".googleapis.com.evil.example"); + const differentPath = "https://daily-cloudcode-pa.googleapis.com/v1internal:other"; + const summaryQuery = `${summaryUrl}?token=secret`; + const modelsQuery = `${modelsUrl}?alt=json`; + for (const url of [summaryUrl, modelsUrl]) { + expect(isCanonicalAntigravityQuotaUrl("google-antigravity", url)).toBe(true); + // A differently named provider never inherits the TUN exception, even for the exact Google accounting URL. + expect(isCanonicalAntigravityQuotaUrl("openai", url)).toBe(false); + expect(isCanonicalAntigravityQuotaUrl("google-antigravity-custom", url)).toBe(false); + } + expect(isCanonicalAntigravityQuotaUrl("google-antigravity", lookalikeHost)).toBe(false); + expect(isCanonicalAntigravityQuotaUrl("google-antigravity", differentPath)).toBe(false); + expect(isCanonicalAntigravityQuotaUrl("google-antigravity", summaryQuery)).toBe(false); + expect(isCanonicalAntigravityQuotaUrl("google-antigravity", modelsQuery)).toBe(false); + }); + + test.each([ + ["127.0.0.1", "loopback address"], + ["10.0.0.5", "private-network address"], + ["169.254.1.1", "link-local address"], + ["169.254.169.254", "blocked metadata endpoint"], + ] as const)("canonical Fake-IP exception still rejects %s (%s)", async (address, detail) => { + await saveCredential("google-antigravity", { access: "agy-first", refresh: "r1", expires: Date.now() + 3600_000, projectId: "proj-first", accountId: "agy-a", email: "a@example.com" }); + for (const key of proxyKeys) expect(process.env[key]).toBeUndefined(); + const admitted: Array<{ url: string; benchmark?: boolean; mihomo?: boolean }> = []; + let posted = 0; + let plainFetchCalls = 0; + globalThis.fetch = (async () => { plainFetchCalls += 1; throw new Error("unexpected raw quota fetch"); }) as typeof fetch; + setAntigravityAccountQuotaTransportForTests({ + resolveAddresses: async (url, options) => { + const policy = typeof options === "object" ? options : undefined; + admitted.push({ url, benchmark: policy?.allowBenchmarkAddresses, mihomo: policy?.allowMihomoIpv6FakeIp }); + // The TUN exception arms benchmark admission for the canonical URL, but the + // resolver still classifies each answer. Loopback/RFC1918/link-local/metadata + // never piggy-back on 198.18/15. + throw new Error(`provider URL hostname daily-cloudcode-pa.googleapis.com resolves to ${detail} (${address})`); + }, + pinnedPost: async () => { posted += 1; return new Response(antigravitySummaryBody(0.5, 0.5)); }, + }); + expect(await fetchProviderAccountQuotas("google-antigravity")).toEqual([{ accountId: idFor("a@example.com"), quota: null, unavailable: true, quotaFailure: "destination_blocked" }]); + expect(admitted).toEqual([ + { url: summaryUrl, benchmark: true, mihomo: true }, + { url: modelsUrl, benchmark: true, mihomo: true }, + ]); + expect(posted).toBe(0); + expect(plainFetchCalls).toBe(0); + }); +}); diff --git a/tests/responses/chat-completions-endpoint.test.ts b/tests/responses/chat-completions-endpoint.test.ts index 3780c33c6c5..82317d89bd8 100644 --- a/tests/responses/chat-completions-endpoint.test.ts +++ b/tests/responses/chat-completions-endpoint.test.ts @@ -3606,3 +3606,41 @@ test("/v1/chat/completions status:failed replay preserves structured model_not_f globalThis.fetch = originalFetch; } }); + +// Issue #4503 coverage note: the Pi/Anthropic rows in the image-parts describe above +// reach the shared userContentToBlocks helper through the user branch only, so a +// regression confined to the role:"tool" branch — the tool_call_id gate, the +// input_image presence check, or the input_text/input_image filter that decides +// between structured output and a flattened string — would survive the suite. Feed +// the same foreign shapes through a direct tool envelope so the branch itself is +// under test rather than covered by composition. +describe("chatCompletionsToResponsesBody tool-result image parts", () => { + test.each([ + { part: { type: "image", data: "aGVsbG8=", mimeType: "image/png" }, expected: { type: "input_image", image_url: "data:image/png;base64,aGVsbG8=" } }, + { part: { type: "image", data: "aGVsbG8=", mediaType: "image/jpeg" }, expected: { type: "input_image", image_url: "data:image/jpeg;base64,aGVsbG8=" } }, + { part: { type: "image", data: "data:image/webp;base64,aGVsbG8=", mimeType: "image/png" }, expected: { type: "input_image", image_url: "data:image/webp;base64,aGVsbG8=" } }, + { part: { type: "image", source: { type: "base64", media_type: "image/jpeg", data: "aGVsbG8=" } }, expected: { type: "input_image", image_url: "data:image/jpeg;base64,aGVsbG8=" } }, + { part: { type: "image", source: { type: "url", url: "https://example.com/claude.png" } }, expected: { type: "input_image", image_url: "https://example.com/claude.png" } }, + ])("normalizes a non-OpenAI image part inside a direct tool result: %j", ({ part, expected }) => { + const body = chatCompletionsToResponsesBody({ + model: "mock/test-model", + messages: [ + { role: "assistant", tool_calls: [{ id: "call_shot", type: "function", function: { name: "screenshot", arguments: "{}" } }] }, + { role: "tool", tool_call_id: "call_shot", content: [ + { type: "text", text: "captured" }, + part, + ] }, + ], + }); + expect(body.input).toEqual([ + { type: "function_call", call_id: "call_shot", name: "screenshot", arguments: "{}" }, + { type: "function_call_output", call_id: "call_shot", output: [ + { type: "input_text", text: "captured" }, + expected, + ] }, + ]); + // The endpoint replays this body verbatim, so a shape parseRequest rejects + // would surface as a 500 on a well-formed client request. + expect(() => parseRequest(body)).not.toThrow(); + }); +}); From 99cdd9dc63e896efc7898395d9ccf18e7f9ef6aa Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 14 Sep 2026 13:25:45 +0900 Subject: [PATCH 2/3] fix(devin): borrow an alias tenant only when the id owns no credential Security review of this branch found the alias host fallback keyed off the wrong condition. It fired whenever the literal credential had no usable apiBaseUrl, which includes a credential that exists and merely carries a missing or off-allowlist host. That case is not the migration window this was written for. The rekey refuses an occupied destination slot, so both the literal and the alias slot can hold credentials belonging to two different accounts. In that state the request kept account A's bearer and took account B's EU or FedStart host. The fallback now runs only when the literal slot holds no credential at all, which is exactly the unmigrated case: the config row already says devin while the credential still sits under devin-cli. A credential that exists with an unusable host falls through to the configured base URL and then the default, as it did before. Adds the regression the existing tests were missing: both slots occupied, the literal host unusable, and the alias holding a different tenant. --- src/oauth/devin.ts | 20 ++++++++++++++++---- tests/providers/devin-login.test.ts | 12 ++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/oauth/devin.ts b/src/oauth/devin.ts index fe8150abcb3..2f5183522a8 100644 --- a/src/oauth/devin.ts +++ b/src/oauth/devin.ts @@ -59,7 +59,8 @@ export function resolveDevinApiServer(configuredBaseUrl?: string, providerId = " // the wrong slot for that window. An EU or FedStart tenant is recorded on the // credential rather than in the registry, so a fixed "devin" slot would send // the key to the wrong host either way. - const literal = validateDevinApiBaseUrl(getCredential(providerId)?.apiBaseUrl); + const literalCredential = getCredential(providerId); + const literal = validateDevinApiBaseUrl(literalCredential?.apiBaseUrl); if (literal !== undefined) return literal; // The startup merge saves providers["devin"] synchronously but fires the @@ -72,9 +73,20 @@ export function resolveDevinApiServer(configuredBaseUrl?: string, providerId = " // "devin-cli" credential, and a lingering "devin-cli" row finds a credential // already rekeyed to "devin". Every candidate passes the same allowlist — an // alias slot is not trusted more than the literal one. - for (const slot of devinAliasCredentialSlots(providerId)) { - const host = validateDevinApiBaseUrl(getCredential(slot)?.apiBaseUrl); - if (host !== undefined) return host; + // Only when this id owns no credential at all. A present credential whose + // apiBaseUrl is missing or off-allowlist is a different situation: the rekey + // refuses an occupied destination slot, so both ids can hold credentials that + // belong to two different accounts. Borrowing a tenant across that pair would + // send this account's key to the other account's EU or FedStart host, which + // is the exact misdirection the provider-scoped lookup exists to prevent. An + // unusable host on a credential that does exist falls through to the + // configured base URL and then the default, as it did before this window was + // closed. + if (literalCredential === undefined) { + for (const slot of devinAliasCredentialSlots(providerId)) { + const host = validateDevinApiBaseUrl(getCredential(slot)?.apiBaseUrl); + if (host !== undefined) return host; + } } return validateDevinApiBaseUrl(configuredBaseUrl) ?? DEVIN_DEFAULT_API_SERVER; diff --git a/tests/providers/devin-login.test.ts b/tests/providers/devin-login.test.ts index db7922366b8..0b4b938728b 100644 --- a/tests/providers/devin-login.test.ts +++ b/tests/providers/devin-login.test.ts @@ -279,6 +279,18 @@ describe("devin tenant selection is provider-scoped", () => { expect(resolveDevinApiServer(undefined, "devin-cli")).toBe(FEDSTART_HOST); }); + test("a credential that exists but has no usable tenant does not borrow the alias tenant", async () => { + // rekeyProviderCredentials refuses when both slots are occupied, so this + // pair can be two different accounts. If the alias host were consulted + // whenever the literal host is merely unusable — rather than when the + // literal slot is empty — this account's key would go to the other + // account's FedStart tenant. + await seedSlot("devin", "https://api.githubcopilot.com"); + await seedSlot("devin-cli", FEDSTART_HOST); + expect(resolveDevinApiServer(undefined, "devin")).toBe(US_HOST); + expect(resolveDevinApiServer(EU_HOST, "devin")).toBe(EU_HOST); + }); + test("an alias slot with a non-Devin apiBaseUrl is not trusted", async () => { // The store allowlists Copilot and Devin together, so a Copilot origin is // the host that survives persist and still fails validateDevinApiBaseUrl. From 7d223ee7bd05256f22b8eeed9a67773788c88d8d Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 14 Sep 2026 13:37:42 +0900 Subject: [PATCH 3/3] fix(devin): treat an absent credential slot as null, not undefined getCredential returns null for a slot that holds no credential, so guarding the alias-tenant fallback on `=== undefined` disabled it outright and the un-rekeyed devin-cli case fell through to the configured base URL. The guard now accepts either absent form, which restores the migration-window fix while keeping the narrower trigger the security review asked for: a credential that exists with an unusable host still does not borrow another account's tenant. --- src/oauth/devin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oauth/devin.ts b/src/oauth/devin.ts index 2f5183522a8..47bac42f9d3 100644 --- a/src/oauth/devin.ts +++ b/src/oauth/devin.ts @@ -82,7 +82,7 @@ export function resolveDevinApiServer(configuredBaseUrl?: string, providerId = " // unusable host on a credential that does exist falls through to the // configured base URL and then the default, as it did before this window was // closed. - if (literalCredential === undefined) { + if (literalCredential === null || literalCredential === undefined) { for (const slot of devinAliasCredentialSlots(providerId)) { const host = validateDevinApiBaseUrl(getCredential(slot)?.apiBaseUrl); if (host !== undefined) return host;