diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index 50c3520b0df..15580c03d97 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -1462,6 +1462,25 @@ model key. ## Storing keys in the OS keychain +For Vertex, the dedicated API-key pool input (`POST /api/providers/keys`) accepts complete GCP +`service_account` or `authorized_user` credential JSON after the provider has been created. +The provider create/edit form and `ocx provider add --api-key` refuse credential JSON before +saving: create the provider without a key, then add the JSON through its key pool. Paste the +JSON contents, not a Windows, Unix, or relative file path. Multiple +JSON objects separated by commas or whitespace are stored together as separate pool entries; +the first becomes active. Configuration contains only `gcp-sa:` references, never the +credential JSON. Mixed JSON/literal-key bulk pastes are rejected as a whole; add literal keys +separately. Storage failures leave the configuration unchanged and report a fixed error without +echoing credential material. Empty marker accounts are invalid and do not fall back to host credentials. +Removing a key or provider, or replacing a provider with `ocx provider add --force`, deletes its unreferenced GCP credentials after the configuration is +saved; a credential referenced by another provider is retained. If OS-store cleanup fails, a +warning requests manual inspection without undoing the committed configuration removal. +Ordinary keychain store/restore actions leave GCP markers and JSON in the OS store. Restoring +credential JSON through a `keychain:` alias is refused, and cleanup preserves accounts still +referenced through either marker spelling. +Credential writes and marker persistence share the cleanup transaction lock, so concurrent +removal cannot delete the stored JSON between read-back and the marker commit. + By default a provider's `apiKey` and `apiKeyPool` sit in `config.json` (mode 0600, atomic writes). If you would rather keep the key material out of the file, move it into the OS credential store: diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 0c289a8fdfc..1ac23fa3128 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -2,8 +2,8 @@ "version": 1, "root": "tests", "explicitShards": ["layout-clients.json"], - "explicit": { "service-log.test.ts": "service", - "codex-history-pending-wal.test.ts": "codex-integration", "responses-anthropic-billing-send-budget.test.ts": "responses", "anthropic-billing-ownership.test.ts": "adapters/anthropic", "server-anthropic-billing.test.ts": "server", "server-anthropic-billing-boundaries.test.ts": "server", "router-anthropic-billing-failover.test.ts": "routing", "anthropic-billing-refusal.test.ts": "adapters/anthropic", "messages-request-id-headers.test.ts": "claude-integration", "bounded-file-read.test.ts": "lib", "responses-undeclared-tool-guard-inbound.test.ts": "responses", + "explicit": { "gcp-sa-marker.test.ts": "adapters/google", "gcp-sa-adapter.test.ts": "adapters/google", "management-gcp-credential-input.test.ts": "server", "service-log.test.ts": "service", + "codex-history-pending-wal.test.ts": "codex-integration", "responses-anthropic-billing-send-budget.test.ts": "responses", "anthropic-billing-ownership.test.ts": "adapters/anthropic", "server-anthropic-billing.test.ts": "server", "server-anthropic-billing-boundaries.test.ts": "server", "router-anthropic-billing-failover.test.ts": "routing", "anthropic-billing-refusal.test.ts": "adapters/anthropic", "messages-request-id-headers.test.ts": "claude-integration", "bounded-file-read.test.ts": "lib", "responses-undeclared-tool-guard-inbound.test.ts": "responses", "messages-request-id-endpoint.test.ts": "claude-integration", "anthropic-revoked-token.test.ts": "adapters/anthropic", "messages-revoked-token.test.ts": "claude-integration", diff --git a/src/adapters/google.ts b/src/adapters/google.ts index df34d14a493..b7f72d61d91 100644 --- a/src/adapters/google.ts +++ b/src/adapters/google.ts @@ -17,7 +17,7 @@ import type { import { isAllowedToolChoice, namespacedToolName, resolveToolChoiceWireName, toolChoiceToolPredicate } from "../types"; import type { OcxTool } from "../types"; import { contentPartsToText, parseDataUrl } from "./image"; -import { getVertexAccessToken } from "../lib/gcp-adc"; +import { GCP_CREDENTIAL_MARKER_PREFIX, gcpCredentialMarkerAccount, getVertexAccessToken } from "../lib/gcp-adc"; import { fetchAntigravityWithRetry, fetchVertexWithRetry } from "./google-http"; import { safeAntigravityHttpErrorMessage, safeVertexHttpErrorMessage } from "./google-errors"; import { isVertexTruncatedTurn, vertexTruncationErrorEvent } from "./google-truncation"; @@ -174,7 +174,15 @@ function resolveDirectGeminiWireModelId(modelId: string, applyRenames: boolean): /** Vertex API key: provider.apiKey if it looks real (not a sentinel), else GOOGLE_CLOUD_API_KEY env. */ function resolveVertexApiKey(optKey?: string): string | undefined { - const realKey = optKey && !optKey.startsWith("<") && optKey !== "N/A" ? optKey : undefined; + // A `gcp-sa:` marker is the keychain-backed credential reference (see gcp-adc): it is not + // API-key material, so it must not be sent as x-goog-api-key — the ADC branch below handles it. + // AND the marker means the operator named one credential explicitly: the GOOGLE_CLOUD_API_KEY + // env fallback must NOT fire (it would silently route this provider's inference through the + // ambient env key instead of the configured service account). + if (optKey && optKey.startsWith(GCP_CREDENTIAL_MARKER_PREFIX)) return undefined; + const realKey = optKey && !optKey.startsWith("<") && optKey !== "N/A" + ? optKey + : undefined; return realKey || process.env.GOOGLE_CLOUD_API_KEY; } @@ -1197,7 +1205,11 @@ export function createGoogleAdapter(provider: OcxProviderConfig): ProviderAdapte if (locationError) throw new Error(locationError); const host = location === "global" ? "aiplatform.googleapis.com" : `${location}-aiplatform.googleapis.com`; const url = `https://${host}/v1/projects/${project}/locations/${location}/publishers/google/models/${parsed.modelId}:${method}${streamParam}`; - const token = await getVertexAccessToken(); + // The marker account travels per-request as an argument (never process-global state): a + // module-level "active marker" written at routing time would race across concurrent + // requests for different providers and could resolve one call with another's credential. + const markerAccount = gcpCredentialMarkerAccount(provider.apiKey); + const token = await getVertexAccessToken({ markerAccount }); headers["Authorization"] = `Bearer ${token}`; return { url, method: "POST", headers, body: JSON.stringify(compiled.body) }; } diff --git a/src/cli/provider.ts b/src/cli/provider.ts index a013f2d915e..69c04994e6d 100644 --- a/src/cli/provider.ts +++ b/src/cli/provider.ts @@ -8,13 +8,15 @@ * show Show provider config details (secrets masked) * set-default Change the default provider */ -import { hasOwnProvider, isValidProviderName, loadConfig, sanitizeModelCostsForDisplay, saveConfig, validateConfigCandidate, withConfigMutationLockSync } from "../config"; +import { hasOwnProvider, isValidProviderName, loadConfig, readConfigAdmissionSnapshot, sanitizeModelCostsForDisplay, saveConfig, validateConfigCandidate, withConfigMutationLockSync } from "../config"; import { apiKeyTransportConfigError, modelCapabilitiesConfigError, mergeModelCapabilities } from "../config/provider-validation"; import { hasHelpFlag, printSubcommandUsage } from "./help"; import { getProviderRegistryEntry, PROVIDER_REGISTRY } from "../providers/registry"; import { providerConfigSeed } from "../providers/derive"; import { assertAnthropicInstanceLoginConfig } from "../oauth/store-anthropic-instance"; import { dropProviderCustomModels } from "../providers/provider-id-rewrite"; +import { cleanupRemovedGcpCredentials, providerGcpCredentialMarkers } from "../providers/gcp-credential-cleanup"; +import { providerCredentialInputError } from "../providers/api-keys"; import type { OcxProviderConfig } from "../types"; import { findLiveProxy } from "../server/proxy-liveness"; import { syncModelsToCodex } from "../codex/sync"; @@ -223,6 +225,12 @@ async function handleAdd(args: string[], deps: ProviderCommandDeps): Promise { if (provConfig.anthropicOAuthInstance) { try { assertAnthropicInstanceLoginConfig(config, name); } catch { markerCollision = true; return; } } initializeProviderModelSelection(name, provConfig, existingProvider, config); + const durable = readConfigAdmissionSnapshot(); + const durableProvider = durable.kind === "read" && durable.diagnostics.source === "file" + && durable.diagnostics.error === null ? durable.diagnostics.config.providers[name] : undefined; + removedCredentials = [config.providers[name], durableProvider].flatMap(provider => provider ? providerGcpCredentialMarkers(provider) : []); config.providers[name] = provConfig; if (setDefault) config.defaultProvider = name; validateAndSave(config); @@ -344,6 +363,7 @@ async function handleAdd(args: string[], deps: ProviderCommandDeps): Promise` reference (the provider's active key resolving to one) → the Service + * Account / authorized_user JSON stored in the OS credential store under that account + * (`keychain:` secret, written by the key-store save path when the user pastes a + * credential JSON directly into the API-key slot). See resolveGcpCredentialMarker. * 1. `GOOGLE_APPLICATION_CREDENTIALS` env → file with `type: "service_account"` (RS256 JWT * exchange) or `type: "authorized_user"` (refresh-token exchange). * 2. gcloud user ADC (`CLOUDSDK_CONFIG`, else `%APPDATA%\gcloud` on Windows, else @@ -18,6 +22,7 @@ import { Buffer } from "node:buffer"; import * as os from "node:os"; import * as path from "node:path"; import { readFileSync, existsSync, statSync } from "node:fs"; +import { GCP_CREDENTIAL_MARKER_PREFIX, providerKeychainEntry } from "../providers/api-key-resolve"; import { captureConfigGeneration, sweepExpiredOnWrite, @@ -36,6 +41,50 @@ const TOKEN_TIMEOUT_MS = 15_000; const TOKEN_ATTEMPTS = 3; const TOKEN_RETRY_BASE_MS = 300; +/** + * Reference prefix stored in `provider.apiKey` (and `apiKeyPool` entries) when the user pastes a + * GCP credential JSON directly into an API-key slot. The secret JSON lives in the OS credential + * store under the same service the `keychain:` references use; only the marker is persisted to + * `config.json`. The leaf reference prefix also identifies already-stored material for migration. + */ +export { GCP_CREDENTIAL_MARKER_PREFIX } from "../providers/api-key-resolve"; + +/** Extract a marker account; malformed explicit markers never opt into host ADC. */ +export function gcpCredentialMarkerAccount(value: string | undefined): string | undefined { + if (typeof value !== "string" || !value.startsWith(GCP_CREDENTIAL_MARKER_PREFIX)) return undefined; + const account = value.slice(GCP_CREDENTIAL_MARKER_PREFIX.length); + if (!account.trim()) throw new Error("GCP credential marker requires a non-empty account"); + return account; +} + +/** + * Whether a pasted API-key value is in fact a GCP credential JSON. Used by the save path to + * divert JSON material out of the literal-key flow before it reaches `config.json`. Deliberately + * strict: only a `type` field with a known ADC credential type counts, so a user's literal API + * key that happens to start with `{` is never eaten. + */ +export function parseGcpCredentialJson(value: string): AdcFileCredentials | undefined { + const trimmed = (value ?? "").trim(); + if (!trimmed.startsWith("{") || !trimmed.endsWith("}")) return undefined; + try { + const parsed = JSON.parse(trimmed) as Record; + if (parsed?.type === "service_account") { + if (typeof parsed.client_email !== "string" || !parsed.client_email.trim()) return undefined; + if (typeof parsed.private_key !== "string" || !parsed.private_key.trim()) return undefined; + return parsed as unknown as AdcFileCredentials; + } + if (parsed?.type === "authorized_user") { + if (typeof parsed.client_id !== "string" || !parsed.client_id.trim()) return undefined; + if (typeof parsed.client_secret !== "string" || !parsed.client_secret.trim()) return undefined; + if (typeof parsed.refresh_token !== "string" || !parsed.refresh_token.trim()) return undefined; + return parsed as unknown as AdcFileCredentials; + } + return undefined; + } catch { + return undefined; + } +} + interface CachedToken { token: string; expiresAtMs: number; @@ -81,7 +130,10 @@ export function sweepExpiredGcpAdcTokens(now = Date.now()): number { export function reconcileGcpAdcTokens(context: GenerationContext): number { if (context.generation <= lastReconciledGeneration) return 0; - const currentSource = currentAdcSourceKey(); + // The periodic sweep registration has no request context, so no marker is active here: markers + // travel per-request via getVertexAccessToken(options.markerAccount), and a marker-token cache + // entry whose account no longer resolves is simply pruned by this sweep when it expires. + const currentSource = currentAdcSourceKey(undefined); let removed = 0; for (const source of tokenCache.keys()) { if (source === currentSource) continue; @@ -131,7 +183,29 @@ function readJsonFile(filePath: string): T | undefined { return JSON.parse(readFileSync(filePath, "utf8")) as T; } -function loadAdcCredentials(): { source: string; creds: AdcFileCredentials } | undefined { +/** + * Read the credential JSON for a `gcp-sa:` marker from the OS credential store. Uses the + * same service name and the same injectable entry factory as the `keychain:` API-key references + * (api-key-resolve.providerKeychainEntry), so one probe surface and one test seam cover both. + * Mirrors `readKeychain`'s fail-closed policy: an unreadable entry yields no credential (the + * caller surfaces the failure instead of silently falling through to the next ADC source). + */ +function readMarkerCredential(account: string): AdcFileCredentials | undefined { + try { + const raw = providerKeychainEntry(account).getPassword(); + if (typeof raw !== "string" || !raw.trim()) return undefined; + return parseGcpCredentialJson(raw); + } catch { + return undefined; + } +} + +function loadAdcCredentials(markerAccount: string | undefined): { source: string; creds: AdcFileCredentials } | undefined { + if (markerAccount) { + const creds = readMarkerCredential(markerAccount); + if (!creds) throw new Error(`gcp-sa:${markerAccount} marker is set but its credential could not be read from the OS credential store`); + return { source: `marker:${markerAccount}`, creds }; + } const gacPath = process.env.GOOGLE_APPLICATION_CREDENTIALS; if (gacPath) { const creds = readJsonFile(gacPath); @@ -148,9 +222,10 @@ function loadAdcCredentials(): { source: string; creds: AdcFileCredentials } | u * The cache key for the source the NEXT resolve would use, computed cheaply (no network). Lets the * cache return a token only when it still matches the active credential source, so an in-process * change to GOOGLE_APPLICATION_CREDENTIALS (or the user ADC file) does not keep serving a stale - * token from a different source. Falls back to "metadata" when no file/env ADC is present. + * token from a different source. Falls back to "metadata" when no marker/file/env ADC is present. */ -function currentAdcSourceKey(): string { +function currentAdcSourceKey(markerAccount: string | undefined): string { + if (markerAccount) return `marker:${markerAccount}`; const gacPath = process.env.GOOGLE_APPLICATION_CREDENTIALS; if (gacPath) return fileSourceTag("gac", gacPath); const userPath = userAdcPath(); @@ -279,8 +354,8 @@ async function fetchMetadataToken(signal: AbortSignal | undefined, fetchImpl: Fe } } -async function resolveAccessTokenUncached(signal: AbortSignal | undefined, fetchImpl: FetchImpl): Promise<{ source: string; token: TokenResponse }> { - const adc = loadAdcCredentials(); +async function resolveAccessTokenUncached(markerAccount: string | undefined, signal: AbortSignal | undefined, fetchImpl: FetchImpl): Promise<{ source: string; token: TokenResponse }> { + const adc = loadAdcCredentials(markerAccount); if (adc) { const token = adc.creds.type === "service_account" ? await exchangeJwtForToken(adc.creds, signal, fetchImpl) @@ -295,14 +370,19 @@ async function resolveAccessTokenUncached(signal: AbortSignal | undefined, fetch } /** Returns a Bearer access token for the `Authorization` header on Vertex AI calls (cached + refreshed). */ -export async function getVertexAccessToken(options?: { signal?: AbortSignal; fetch?: FetchImpl }): Promise { +export async function getVertexAccessToken(options?: { signal?: AbortSignal; fetch?: FetchImpl; markerAccount?: string }): Promise { + // The marker travels as an argument, never as process-global state: a module-level "active + // marker" written by the router would race across concurrent requests for different providers + // and could resolve one provider's Vertex call with another provider's service account. The + // adapter passes `gcpCredentialMarkerAccount(provider.apiKey)` per request instead. + const markerAccount = options?.markerAccount; const fetchImpl = options?.fetch ?? globalThis.fetch.bind(globalThis); const skew = getRefreshSkewMs(); const now = Date.now(); // Only serve a cached token that matches the source the next resolve would actually use; prune // expired or now-stale (different-source) entries so a credential-source change is honored. - const expectedSource = currentAdcSourceKey(); + const expectedSource = currentAdcSourceKey(markerAccount); const writerGeneration = captureConfigGeneration(); for (const [source, cached] of tokenCache) { if (source === expectedSource && cached.expiresAtMs - skew > now) return cached.token; @@ -317,7 +397,7 @@ export async function getVertexAccessToken(options?: { signal?: AbortSignal; fet const promise = (async () => { try { - const { source, token } = await resolveAccessTokenUncached(options?.signal, fetchImpl); + const { source, token } = await resolveAccessTokenUncached(markerAccount, options?.signal, fetchImpl); const expiresAtMs = Date.now() + Math.max(0, token.expires_in * 1000); if (writerGeneration >= lastReconciledGeneration || liveSources.has(source)) { tokenCache.set(source, { token: token.access_token, expiresAtMs }); diff --git a/src/providers/api-key-resolve.ts b/src/providers/api-key-resolve.ts index eeaa0f4d028..00a787d284e 100644 --- a/src/providers/api-key-resolve.ts +++ b/src/providers/api-key-resolve.ts @@ -21,6 +21,8 @@ import { loadKeyringBinding } from "../lib/keyring-native"; import type { OcxProviderConfig } from "../types"; export const KEYCHAIN_REFERENCE_PREFIX = "keychain:"; +/** Stored GCP JSON uses the same OS service, but must never be restored as a literal API key. */ +export const GCP_CREDENTIAL_MARKER_PREFIX = "gcp-sa:"; export const PROVIDER_KEYCHAIN_SERVICE = "opencodex.provider-api-key.v1"; export interface ProviderKeychainEntry { @@ -125,7 +127,7 @@ export type ProviderKeyStoreKind = "keychain" | "env" | "file" | "none"; export function providerKeyStoreKind(provider: Pick | undefined): ProviderKeyStoreKind { const key = provider?.apiKey; if (!key) return "none"; - if (isKeychainReference(key)) return "keychain"; + if (key.startsWith(GCP_CREDENTIAL_MARKER_PREFIX) || isKeychainReference(key)) return "keychain"; if (/^\$\{?\w+\}?$/.test(key)) return "env"; return "file"; } diff --git a/src/providers/api-keys.ts b/src/providers/api-keys.ts index ceb5522df29..9ad35d5ae32 100644 --- a/src/providers/api-keys.ts +++ b/src/providers/api-keys.ts @@ -9,8 +9,11 @@ import { createHash } from "node:crypto"; import { saveConfigPreservingClaudeCode } from "../config"; import type { OcxConfig, OcxProviderConfig } from "../types"; +import { GCP_CREDENTIAL_MARKER_PREFIX, gcpCredentialMarkerAccount, parseGcpCredentialJson } from "../lib/gcp-adc"; +import { invalidateResolvedProviderKeyCache, probeProviderKeychain, providerKeychainEntry } from "./api-key-resolve"; import type { AccountQuotaFields } from "./quota-types"; import { commitProviderApiKeySelection } from "./api-key-selection"; +import { cleanupRemovedGcpCredentials } from "./gcp-credential-cleanup"; export interface ProviderApiKeyInfo extends AccountQuotaFields { id: string; @@ -28,11 +31,156 @@ function isEnvReference(value: string): boolean { export function maskApiKey(value: string): string { // Env and keychain references carry no secret material; show them verbatim so an operator // can tell where the key lives. - if (isEnvReference(value) || value.startsWith("keychain:")) return value; + if (isEnvReference(value) || value.startsWith("keychain:") || value.startsWith(GCP_CREDENTIAL_MARKER_PREFIX)) return value; if (value.length <= 8) return "****"; return `${value.slice(0, 4)}****${value.slice(-4)}`; } +/** + * Split a pasted key value into candidate credentials. A plain API key is one row; a pasted GCP + * credential JSON (or several, comma/space-separated — the same convention as newline-separated + * API keys) is one row per JSON object. File paths are rejected with guidance rather + * than stored, because they are the most common paste mistake and silently break at runtime. + * + * Honors the parseGcpCredentialJson contract "a literal key that starts with `{` is never eaten": + * a brace-leading value that is NOT recognizable credential JSON falls back to a plain literal + * row (preserving the old single-line behavior), and only a multi-line or multi-object paste that + * fails credential validation rejects with guidance. The brace scanner tracks JSON string and + * escape state so a `}` inside a string field does not split the object early; any non-separator + * text between objects is rejected rather than silently discarded. + */ +export function splitCredentialPaste(value: string): Array { + const trimmed = (value ?? "").trim(); + if (!trimmed) return []; + // Fast path: a plain key (no JSON braces) — also covers env/keychain/gcp-sa references. + if (!trimmed.startsWith("{")) { + if (/(?:^|[\s,])\{/.test(trimmed)) { + throw new Error("Mixing credential JSON and literal API keys in one paste is not supported. Add them separately."); + } + if (/^(?:[A-Za-z]:[\\/]|[\\/]|~[\\/]|\.{1,2}[\\/])/.test(trimmed) || /^[^\s{}]+\.json$/i.test(trimmed)) { + throw new Error("This looks like a file path. Paste the full JSON body of the credential, not a file path."); + } + return [trimmed]; + } + // Brace-leading scans: split adjacent JSON objects (comma/newline separated). The scanner + // tracks string/escape state so braces inside JSON string fields do not count, and records + // each object's [start, end] so the gap check below can reject non-separator junk between + // objects in one pass. + const spans: Array<{ start: number; end: number }> = []; + const objects: string[] = []; + let depth = 0; + let start = -1; + let inString = false; + let escaped = false; + for (let i = 0; i < trimmed.length; i++) { + const ch = trimmed[i]; + if (inString) { + if (escaped) escaped = false; + else if (ch === "\\") escaped = true; + else if (ch === '"') inString = false; + continue; + } + if (ch === '"') { inString = true; continue; } + if (ch === "{" && depth === 0) { + start = i; + } + if (ch === "{") depth += 1; + if (ch === "}") { + depth -= 1; + if (depth === 0 && start >= 0) { + objects.push(trimmed.slice(start, i + 1)); + spans.push({ start, end: i }); + start = -1; + } + if (depth < 0) throw new Error("Invalid credential JSON: unbalanced braces."); + } + } + if (depth !== 0 || start >= 0) throw new Error("Invalid credential JSON: the pasted text ends inside a JSON object."); + // Single unrecognized object on a single line = a plain key starting with `{` (never eaten) — + // BUT only when it is NOT real JSON: a parseable JSON object with an unrecognized `type` + // (e.g. `external_account`) is a credential the user believes is valid, and saving it as a + // plaintext literal key would send it to the wire. Distinguish by parse success, not shape. + const singleLine = !/[\r\n]/.test(trimmed); + const isRealJson = (() => { + try { JSON.parse(objects[0]!); return true; } catch { return false; } + })(); + if (objects.length === 1 && singleLine && spans[0]!.end === trimmed.length - 1 && !parseGcpCredentialJson(objects[0]!) && !isRealJson) { + return [trimmed]; + } + // Non-separator content BETWEEN and AFTER objects is rejected, not silently discarded: + // everything between/after one object's close must be a separator character. + for (let i = 0; i < spans.length; i++) { + if (i === 0) { + if (!/^[\s,]*$/.test(trimmed.slice(0, spans[0]!.start))) { + throw new Error("Unrecognized credential JSON: unexpected content before the first JSON object. Paste credential JSON(s) separated by commas or newlines, or a plain API key."); + } + continue; + } + const gap = trimmed.slice(spans[i - 1]!.end + 1, spans[i]!.start); + if (!/^[\s,]*$/.test(gap)) { + throw new Error("Unrecognized credential JSON: unexpected content between JSON objects. Paste credential JSON(s) separated by commas or newlines, or a plain API key."); + } + } + // Tail gap: content after the LAST object's close up to EOF must also be separators — + // trailing garbage (` garbage`) is rejected rather than silently kept. + const tailGap = trimmed.slice(spans[spans.length - 1]!.end + 1); + if (!/^[\s,]*$/.test(tailGap)) { + throw new Error("Unrecognized credential JSON: unexpected content after the last JSON object. Mixing JSON and literal keys is not supported; add them separately."); + } + return objects.map((json) => { + if (!parseGcpCredentialJson(json)) { + throw new Error("Unrecognized credential JSON: expected a GCP Service Account or authorized_user JSON with a \"type\" field."); + } + return { credentialJson: json }; + }); +} + +/** Generic provider writes do not own the keychain transaction. Refuse before persistence. */ +export function providerCredentialInputError(provider: { apiKey?: unknown; apiKeyPool?: unknown }): string | undefined { + const values = [provider.apiKey, ...(Array.isArray(provider.apiKeyPool) + ? provider.apiKeyPool.map(entry => entry && typeof entry === "object" ? entry.key : undefined) : [])]; + try { + for (const value of values) { + if (typeof value !== "string") continue; + if (splitCredentialPaste(value).some(part => typeof part !== "string")) { + return "Credential JSON must be added through the provider API-key pool endpoint (/api/providers/keys), after creating the provider without a key. Nothing was saved."; + } + } + } catch (error) { + return error instanceof Error ? error.message : "Invalid credential paste"; + } + return undefined; +} + +/** + * Store a pasted GCP credential JSON in the OS keychain and return the `gcp-sa:` marker for it. + * Verified write (read-back check) mirrors storeProviderKeyInKeychain; on any failure the caller + * receives an error instead of half-stored state. + */ +function storeGcpCredentialJson(config: OcxConfig, name: string, credentialJson: string, existedBefore: boolean): string { + const probe = probeProviderKeychain(); + if (!probe.available) throw new Error(`OS keychain unavailable: ${probe.reason}`); + const id = apiKeyPoolEntryId(credentialJson); + const account = gcpCredentialStoreAccount(name, id); + const entry = providerKeychainEntry(account); + entry.setPassword(credentialJson); + if (entry.getPassword() !== credentialJson) { + // Delete on mismatch ONLY when this write created the entry: a re-paste targets an account an + // existing `gcp-sa:` marker already names, and deleting it would destroy the live credential + // the config still references. + if (!existedBefore) { + try { entry.deletePassword(); } catch { /* best effort */ } + } + throw new Error(`keychain read-back mismatch for ${account}`); + } + return `${GCP_CREDENTIAL_MARKER_PREFIX}${account}`; +} + +/** The credential-store account a marker names for `name`/`id` (write + cleanup share it). */ +function gcpCredentialStoreAccount(name: string, id: string): string { + return `${name}/${id}`; +} + /** Content-derived id: re-adding the same key upserts instead of duplicating. */ export function apiKeyPoolEntryId(key: string): string { return createHash("sha256").update(key).digest("hex").slice(0, 8); @@ -84,6 +232,60 @@ export function addProviderApiKey(config: OcxConfig, name: string, key: string, const provider = config.providers[name]; if (!provider || !isKeyAuthProvider(provider)) return { error: "provider does not use API-key auth" }; if (typeof key !== "string" || !key.trim()) return { error: "key is required" }; + // GCP credential JSON never enters the literal-key path: it is diverted to the OS keychain and + // only the `gcp-sa:` marker is stored. splitCredentialPaste runs FIRST (before the CRLF + // rejection) because pretty-printed JSON contains line breaks, and it also catches the + // file-path paste mistake with its own guidance before anything is stored. + try { + const parts = splitCredentialPaste(key); + const first = parts[0]; + const isJsonPaste = first !== undefined && typeof first !== "string"; + if (!isJsonPaste) { + // Not a credential paste — fall through to the plain API-key path below. splitCredentialPaste + // has already thrown for file paths; a plain key lands here as a single literal row. + } else { + const stored: Array<{ id: string; marker: string }> = []; + const created: string[] = []; + try { + const committed = commitProviderApiKeySelection(config, name, fresh => { + // Hold the same mutation lock as cleanup from the verified OS write through marker + // persistence. Re-run idempotent writes on rebase so a removed payload is restored. + stored.length = 0; + for (const part of parts) { + const credentialJson = (part as { credentialJson: string }).credentialJson; + const id = apiKeyPoolEntryId(credentialJson); + const account = gcpCredentialStoreAccount(name, id); + const existedBefore = providerKeychainEntry(account).getPassword() !== null; + if (!existedBefore) created.push(`${GCP_CREDENTIAL_MARKER_PREFIX}${account}`); + stored.push({ id, marker: storeGcpCredentialJson(config, name, credentialJson, existedBefore) }); + } + const pool = ensurePool(fresh); + for (const { id, marker } of stored) { + const existing = pool.find(e => e.id === id); + if (existing) { + existing.key = marker; + if (label?.trim()) existing.label = label.trim(); + } else { + pool.push({ id, key: marker, ...(label?.trim() ? { label: label.trim() } : {}), addedAt: Date.now() }); + } + } + fresh.apiKey = stored[0]!.marker; + return { changed: true, selectionChanged: true, value: stored[0]!.id }; + }); + if (committed.status === "committed") { + invalidateResolvedProviderKeyCache(); + return { id: committed.value }; + } + cleanupRemovedGcpCredentials(created); + return { error: "provider selection unavailable" }; + } catch (error) { + cleanupRemovedGcpCredentials(created); + return { error: "credential storage failed; configuration was not changed" }; + } + } + } catch (error) { + return { error: error instanceof Error ? error.message : "credential parse failed" }; + } const trimmed = sanitizeApiKeyValue(key); if (!trimmed) return { error: "key must not include line breaks" }; const id = apiKeyPoolEntryId(trimmed); @@ -98,7 +300,8 @@ export function addProviderApiKey(config: OcxConfig, name: string, key: string, fresh.apiKey = trimmed; return { changed: true, selectionChanged: true, value: id }; }); - return committed.status === "committed" ? { id } : { error: "provider selection unavailable" }; + if (committed.status !== "committed") return { error: "provider selection unavailable" }; + return { id }; } /** Switch the ACTIVE key (mirrors into `provider.apiKey`). Persists config. */ @@ -129,11 +332,13 @@ export function setProviderApiKeyLabel(config: OcxConfig, name: string, id: stri /** Remove one key; removing the active one promotes the first remaining. Persists config. */ export function removeProviderApiKey(config: OcxConfig, name: string, id: string): boolean { + let removed: string | undefined; const committed = commitProviderApiKeySelection(config, name, provider => { const pool = provider.apiKeyPool?.length ? provider.apiKeyPool : provider.apiKey ? [{ id: apiKeyPoolEntryId(provider.apiKey), key: provider.apiKey }] : []; const entry = pool.find(e => e.id === id); if (!entry) return { changed: false, value: false }; + removed = entry.key; provider.apiKeyPool = pool.filter(e => e.id !== id); if (provider.apiKey === entry.key) { const next = provider.apiKeyPool[0]; @@ -143,5 +348,8 @@ export function removeProviderApiKey(config: OcxConfig, name: string, id: string if (provider.apiKeyPool.length === 0) delete provider.apiKeyPool; return { changed: true, value: true }; }); - return committed.status === "committed" && committed.value; + if (committed.status !== "committed" || !committed.value) return false; + if (removed?.startsWith(GCP_CREDENTIAL_MARKER_PREFIX)) cleanupRemovedGcpCredentials([removed]); + invalidateResolvedProviderKeyCache(); + return true; } diff --git a/src/providers/gcp-credential-cleanup.ts b/src/providers/gcp-credential-cleanup.ts new file mode 100644 index 00000000000..97cae4e15ce --- /dev/null +++ b/src/providers/gcp-credential-cleanup.ts @@ -0,0 +1,48 @@ +import { mutatePersistedConfig, readConfigAdmissionSnapshot } from "../config"; +import { GCP_CREDENTIAL_MARKER_PREFIX, gcpCredentialMarkerAccount } from "../lib/gcp-adc"; +import type { OcxProviderConfig } from "../types"; +import { isKeychainReference, keychainAccount, providerKeychainEntry } from "./api-key-resolve"; + +/** Snapshot credential references before removing a key or provider. */ +export function providerGcpCredentialMarkers(provider: OcxProviderConfig): string[] { + return [...new Set([provider.apiKey, ...(provider.apiKeyPool ?? []).map(entry => entry.key)] + .filter((key): key is string => typeof key === "string" && key.startsWith(GCP_CREDENTIAL_MARKER_PREFIX)))]; +} + +/** Capture both the caller's row and the valid durable row before a provider mutation. */ +export function snapshotProviderGcpCredentialMarkers(name: string, provider: OcxProviderConfig | undefined): string[] { + const durable = readConfigAdmissionSnapshot(); + const durableProvider = durable.kind === "read" && durable.diagnostics.source === "file" + && durable.diagnostics.error === null ? durable.diagnostics.config.providers[name] : undefined; + return [provider, durableProvider].flatMap(row => row ? providerGcpCredentialMarkers(row) : []); +} + +function cleanupGcpCredentialAccount(key: string | undefined): string | undefined { + if (key?.startsWith(GCP_CREDENTIAL_MARKER_PREFIX) && !key.slice(GCP_CREDENTIAL_MARKER_PREFIX.length).trim()) return undefined; + return gcpCredentialMarkerAccount(key); +} + +/** After a successful config commit, remove only credentials no persisted provider references. */ +export function cleanupRemovedGcpCredentials(markers: readonly string[]): void { + if (!markers.length) return; + try { + // Re-read under the config mutation lock: another provider may share a marker, or a newer + // commit may have reintroduced it. Never decide deletion from the caller's stale snapshot. + const outcome = mutatePersistedConfig(fresh => { + const retained = new Set(Object.values(fresh.providers).flatMap(provider => + [provider.apiKey, ...(provider.apiKeyPool ?? []).map(entry => entry.key)] + .flatMap(key => { + const account = isKeychainReference(key) ? keychainAccount(key) : cleanupGcpCredentialAccount(key); + return account === undefined ? [] : [account]; + }))); + for (const marker of new Set(markers)) { + const account = cleanupGcpCredentialAccount(marker); + if (account && !retained.has(account)) providerKeychainEntry(account).deletePassword(); + } + return { changed: false, value: true }; + }); + if (outcome.status === "unavailable") throw new Error("configuration unavailable"); + } catch { + console.warn("GCP credential cleanup incomplete; inspect the OS credential store. Configuration removal remains committed."); + } +} diff --git a/src/providers/key-store.ts b/src/providers/key-store.ts index 428b018213f..20272343319 100644 --- a/src/providers/key-store.ts +++ b/src/providers/key-store.ts @@ -1,8 +1,10 @@ import { resolveEnvValue, saveConfigPreservingClaudeCode } from "../config"; import type { OcxConfig, OcxProviderConfig } from "../types"; import type { ProviderRegistryEntry } from "./registry"; +import { parseGcpCredentialJson } from "../lib/gcp-adc"; import { KEYCHAIN_REFERENCE_PREFIX, + GCP_CREDENTIAL_MARKER_PREFIX, invalidateResolvedProviderKeyCache, isKeychainReference, keychainAccount, @@ -79,7 +81,7 @@ export function storeProviderKeyInKeychain(config: OcxConfig, name: string): { o const pool = provider.apiKeyPool ?? []; try { for (const entry of pool) { - if (isKeychainReference(entry.key)) continue; + if (entry.key.startsWith(GCP_CREDENTIAL_MARKER_PREFIX) || isKeychainReference(entry.key)) continue; const secret = resolveEnvValue(entry.key); if (!secret) continue; // unresolved env reference stays as-is const account = `${name}/${entry.id}`; @@ -87,7 +89,7 @@ export function storeProviderKeyInKeychain(config: OcxConfig, name: string): { o written.push(account); planned.push(() => { entry.key = `${KEYCHAIN_REFERENCE_PREFIX}${account}`; }); } - if (provider.apiKey && !isKeychainReference(provider.apiKey)) { + if (provider.apiKey && !provider.apiKey.startsWith(GCP_CREDENTIAL_MARKER_PREFIX) && !isKeychainReference(provider.apiKey)) { const active = pool.find(e => e.key === provider.apiKey || (isKeychainReference(e.key) && false)); const secret = resolveEnvValue(provider.apiKey); if (secret) { @@ -138,6 +140,9 @@ export function restoreProviderKeyFromKeychain(config: OcxConfig, name: string): let value: string | null = null; try { value = providerKeychainEntry(account).getPassword(); } catch { value = null; } if (!value) return { ok: false, error: `OS keychain has no readable secret for ${ref}; config left unchanged`, status: 503 }; + if (parseGcpCredentialJson(value)) { + return { ok: false, error: "GCP credentials must remain in the OS store; use a gcp-sa reference. Config left unchanged", status: 400 }; + } resolved.set(account, value); } for (const entry of pool) { diff --git a/src/router.ts b/src/router.ts index 0ea69b230a3..b707d9e235a 100644 --- a/src/router.ts +++ b/src/router.ts @@ -15,6 +15,7 @@ import type { NormalizedComboConfig } from "./combos/types"; import { hasOwnProvider } from "./config/provider-name"; import { anthropicInstanceRowShapeMatches } from "./providers/anthropic-instance-id"; import { providerUsesKeyAuthOverride, resolveProviderApiKey } from "./providers/key-store"; +import { GCP_CREDENTIAL_MARKER_PREFIX } from "./lib/gcp-adc"; import { captureProviderApiKeySelection } from "./providers/api-key-selection-capture"; import { assertProviderDestinationAllowed } from "./lib/destination-policy"; import { redactSecretString, redactUrlForLog } from "./lib/redact"; @@ -298,6 +299,12 @@ export function resetCompactionFallbackWarningsForTests(): void { } function usableResolvedApiKey(apiKey: string | undefined): string | undefined { + // A `gcp-sa:` marker must pass through UNCHANGED: it resolves to `undefined` here (it is not a + // keychain/env ref, and the marker itself is not a literal), and replacing it would silently + // drop the marker from the routed config — the vertex adapter would then read + // `gcpCredentialMarkerAccount(undefined)` and fall back to host ADC instead of the configured + // service account. Return the marker verbatim so the adapter passes its account per request. + if (typeof apiKey === "string" && apiKey.startsWith(GCP_CREDENTIAL_MARKER_PREFIX)) return apiKey; const resolved = resolveProviderApiKey(apiKey); return typeof resolved === "string" && resolved.trim().length > 0 ? resolved : undefined; } diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index fd0c1e68df0..643214cd6de 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -1,6 +1,8 @@ import { contextTierRecordConfigError, modelCapabilitiesConfigError, mergeModelCapabilities } from "../../config/provider-validation"; import { DECLARABLE_HOSTED_TOOL_TYPES } from "../../responses/hosted-tool-policy"; import { randomUUID } from "node:crypto"; +import { cleanupRemovedGcpCredentials, providerGcpCredentialMarkers, snapshotProviderGcpCredentialMarkers } from "../../providers/gcp-credential-cleanup"; +import { providerCredentialInputError } from "../../providers/api-keys"; import { readFileSync } from "node:fs"; import { isDeepStrictEqual } from "node:util"; import type { CatalogModel } from "../../codex/catalog"; @@ -1073,6 +1075,7 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise(persisted => { if (!isDeepStrictEqual(providerEditorConfigDTO(persisted), baselineResult.value)) { return { @@ -1095,6 +1098,7 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise { validationError = anthropicInstancePublicationError(name, candidate, config.providers[name]); if (validationError) return; @@ -1375,6 +1379,7 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise` references in one pool transaction in `src/providers/api-keys.ts`; post-commit deletion follows [Vertex credential markers](providers/google.md#vertex-credential-markers). Quota activation reuses the existing next-reset fields without adding a polling configuration key. See the [quota activation contract](providers/openai-tiers.md#public-provider-contract). `src/types/config.ts` defines `anthropicAccountPool.routes` as ordered model rules. `src/oauth/anthropic-model-routes.ts` validates bounded names, full case-sensitive globs and stored account IDs; `src/config/diagnostics.ts` rejects malformed candidate writes. Load retains a malformed hand edit so unrelated providers survive, while the enabled Anthropic selector refuses it locally before dispatch. Saved rules remain inert when the pool is disabled; affinity is process-local. Request logs use only the rule’s 1-based `route:#` position, never its configured name. `src/cli/hub-disable.ts` removes interdependent hub/loopback/role/hostname settings through `mutatePersistedConfig` and `validateConfigCandidate` in one commit. Top-level deletion intent uses `src/config/rebase-provenance.ts` and authoritative disk provenance in `src/config/live-reconcile.ts` to preserve removed hub bindings through later live saves, `src/config/live-topology.ts` keeps an independent active-topology baseline and merges desired next-start settings into a separate persistence projection, retaining actual socket identity and live hub/client topology during guarded saves and OAuth disk adoption. Client connections and runtimeRole remain one topology unit through connect/disconnect, including fresh disk-only connections. Disk additions/value updates and deletions all survive repeated saves without changing active topology. `src/config/api-key-reconcile.ts` merges key rows by stable ID under the existing save lock; either side's revocation defeats stale edits while disjoint names and rotations merge; unchanged live key membership/order adopts persisted ordering. `tests/config/hub-disable-live-save.test.ts` exercises disable followed by live key creation, rename, rotation, unrelated settings and repeated saves. Saved API keys have no immutable enrollment marker; explicit exact-ID revocation preserves all unselected keys. Native function-result injection follows [the separate opt-in control contract](transports/streaming-health.md#experimental-native-function-result-injection); this surface does not infer upstream support or alter its defaults. diff --git a/structure/gui-and-management-api.md b/structure/gui-and-management-api.md index 961a55f07ed..a2cd9e93f1d 100644 --- a/structure/gui-and-management-api.md +++ b/structure/gui-and-management-api.md @@ -1,5 +1,5 @@ # GUI And Management API - +Provider PUT, POST and DELETE in `src/server/management/provider-routes.ts` run [GCP credential cleanup](providers/google.md#vertex-credential-markers) only after saving; overwrite and deletion snapshot live and valid durable row markers through the cleanup module before mutation. Rejected mutations and failed saves preserve secrets and other persisted references retain them. Anthropic OAuth account DTOs include `autoSwitchThresholdOverride` (integer or null), `autoSwitchThreshold` (pool default) and `effectiveAutoSwitchThreshold`. The dedicated `PUT /api/oauth/accounts/auto-switch` accepts `{ provider: "anthropic", accountId, threshold }`; diff --git a/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index c7c9665547a..343710362b5 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -1,5 +1,5 @@ # Providers And Adapters - +GCP JSON storage in `src/providers/api-keys.ts` and durable-row snapshot/post-commit cleanup in `src/providers/gcp-credential-cleanup.ts` follow [Vertex credential markers](providers/google.md#vertex-credential-markers); malformed blank marker accounts do not block cleanup of valid accounts. Configured spend ceilings bind adapter inference executors to the [seed and physical-start contract](transports/responses-spend.md#historical-pool-continuity-and-rollback). Zed completion and auth-recovery replays claim separately; token acquisition stays outside inference accounting. Claude CLI, CodeBuddy and Qoder each count one invocation toward the frozen request limit, seed before spawn and retain a producer through completion. Child-internal retries/tool turns are not separately counted; reported actual usage settles in full, including amounts above the seed estimate. Devin combines only [consecutive same-ID tool results](adapters/registry.md#devin-consecutive-tool-results), using linear accumulation while preserving intervening message slots, image parts and error markers without mutating the parsed request. Anthropic account pause, model routes, quota labels, and the separate `anthropic2` pool (its helpers, quota, reset grants and management surfaces) follow the [Anthropic account-pool contract](providers/anthropic-account-pool.md). Devin Messages follows the [per-turn output ordering contract](clients/claude-desktop.md#devin-messages-output-ordering), preserving late signatures before text/tools without changing Responses or Chat ordering. diff --git a/structure/providers/google.md b/structure/providers/google.md index 7e81d31afc2..aade9992443 100644 --- a/structure/providers/google.md +++ b/structure/providers/google.md @@ -1,5 +1,26 @@ # Google Provider +## Vertex credential markers + +`src/lib/gcp-adc.ts` resolves an explicit `gcp-sa:` through the OS credential store. +An empty or whitespace-only account is rejected before token resolution; it never selects host ADC. +Post-commit cleanup skips empty or whitespace-only GCP marker accounts in both persisted references and captured removal candidates, so one malformed marker cannot block valid-account retention or cleanup. Request-time marker validation remains strict. +`src/providers/api-keys.ts` stores pasted credential JSON as markers, including multiple JSON objects +in one atomic pool update. Verified OS writes and marker persistence share the cleanup mutation +lock. Rebased callbacks repeat idempotent writes before committing; failed batches remove only +newly created, unreferenced secrets. +The dedicated key-pool endpoint owns JSON ingestion. Provider POST/create/overwrite and CLI add +reject raw JSON in active keys or submitted pool entries before saving; mixed JSON/literal-key +bulk pastes are rejected before any store writes. OS-store failures return a fixed error without +credential material. `src/server/management/provider-routes.ts` and `src/cli/provider.ts` enforce +this admission through `providerCredentialInputError` in `src/providers/api-keys.ts`; CLI registry input is checked before forward/OAuth branches can discard `--api-key`. +`src/providers/gcp-credential-cleanup.ts` cleans removed markers after config persistence, including CLI `provider add --force` overwrites, under the + config mutation lock, retaining credentials still referenced by any persisted provider. CLI overwrite captures both live and latest durable rows under that lock. Management overwrite and deletion reuse `snapshotProviderGcpCredentialMarkers` to capture the caller's row and a valid durable row before mutation. Failed saves and refused CLI additions do not clean credentials. +Reference retention compares OS account identities across both `gcp-sa:` and `keychain:` +spellings. `src/providers/key-store.ts` leaves GCP markers stored during ordinary keychain +store/restore and refuses to restore GCP JSON through a keychain alias; the shared classification +in `src/providers/api-key-resolve.ts` reports those markers as OS-stored credentials. + ## Google thought-text visibility boundary Google-family parts with `thought: true` stay separate from assistant output. After a CCA diff --git a/tests/adapters/google/gcp-sa-adapter.test.ts b/tests/adapters/google/gcp-sa-adapter.test.ts new file mode 100644 index 00000000000..b2eb1e08ffe --- /dev/null +++ b/tests/adapters/google/gcp-sa-adapter.test.ts @@ -0,0 +1,249 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { Buffer } from "node:buffer"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { __resetVertexTokenCache, gcpCredentialMarkerAccount } from "../../../src/lib/gcp-adc"; +import { createGoogleAdapter } from "../../../src/adapters/google"; +import { routedProviderConfig } from "../../../src/router"; +import { setProviderKeychainEntryFactoryForTests } from "../../../src/providers/key-store"; +import { saveConfig } from "../../../src/config"; +import type { OcxConfig, OcxParsedRequest, OcxProviderConfig } from "../../../src/types"; +import { removeTreeWithRetry } from "../../helpers/remove-tree"; + +function parsed(modelId = "gemini-3-pro"): OcxParsedRequest { + return { + modelId, + stream: true, + context: { messages: [{ role: "user", content: "hi" }], systemPrompt: [], tools: [] }, + options: {}, + } as unknown as OcxParsedRequest; +} + +afterEach(() => { + __resetVertexTokenCache(); + setProviderKeychainEntryFactoryForTests(null); +}); + +describe("vertex adapter with a gcp-sa marker key", () => { + test("an empty marker fails before host ADC or any token fetch", async () => { + const realFetch = globalThis.fetch; + const previousKey = process.env.GOOGLE_CLOUD_API_KEY; + let calls = 0; + process.env.GOOGLE_CLOUD_API_KEY = "fixture-env-key"; + globalThis.fetch = (async () => { calls++; return new Response("fixture"); }) as typeof fetch; + try { + for (const apiKey of ["gcp-sa:", "gcp-sa: "]) { + const provider = { adapter: "google", googleMode: "vertex", baseUrl: "https://aiplatform.googleapis.com", project: "fixture", location: "global", apiKey } as OcxProviderConfig; + await expect(createGoogleAdapter(provider).buildRequest(parsed())).rejects.toThrow(/non-empty account/); + } + expect(calls).toBe(0); + } finally { + globalThis.fetch = realFetch; + if (previousKey === undefined) delete process.env.GOOGLE_CLOUD_API_KEY; + else process.env.GOOGLE_CLOUD_API_KEY = previousKey; + } + }); + + test("marker resolves through the ADC branch (never the x-goog-api-key fast path)", async () => { + // A real, signable RSA key so buildRequest reaches the actual signing step of the ADC branch. + const kp = await generateKeyPair(); + const pkcs8 = await globalThis.crypto.subtle.exportKey("pkcs8", kp.privateKey); + const b64 = Buffer.from(pkcs8).toString("base64").match(/.{1,64}/g)!.join("\n"); + const pem = `-----BEGIN PRIVATE KEY-----\n${b64}\n-----END PRIVATE KEY-----\n`; + const serviceAccount = JSON.stringify({ type: "service_account", client_email: "svc@example.test", private_key: pem, private_key_id: "k1" }); + + // Stub fetch so the JWT exchange succeeds and NO network call carries x-goog-api-key. + const realFetch = globalThis.fetch; + globalThis.fetch = (async () => + new Response(JSON.stringify({ access_token: "tok", expires_in: 3600 }), { status: 200 })) as typeof fetch; + + // The marker in a keychain-backed store; the adapter passes its account to the ADC branch per + // request (gcpCredentialMarkerAccount(provider.apiKey)) — no process-global registration. + setProviderKeychainEntryFactoryForTests((service, account) => ({ + getPassword: () => (account === "p/mk1" ? serviceAccount : null), + setPassword: () => {}, + deletePassword: () => true, + })); + const provider = { + adapter: "google", + baseUrl: "https://x", + googleMode: "vertex", + apiKey: "gcp-sa:p/mk1", + project: "proj-1", + location: "global", + } as OcxProviderConfig; + + try { + const req = await createGoogleAdapter(provider).buildRequest(parsed()); + // The produced request must NOT be the API-key fast path (which would carry the marker + // itself as x-goog-api-key); it must be the ADC Bearer request. + expect(req.url).toContain("/v1/projects/proj-1/locations/global/publishers/google/models/"); + expect(req.headers["x-goog-api-key"]).toBeUndefined(); + expect((req.headers["Authorization"] ?? "").startsWith("Bearer tok")).toBe(true); + } finally { + globalThis.fetch = realFetch; + } + }); +}); + +describe("router preserves the gcp-sa marker", () => { + let home: string; + const previousHome = process.env.OPENCODEX_HOME; + + // routedProviderConfig reads the committed config through mutatePersistedConfig-dependent + // helpers, so tests run against a throwaway OPENCODEX_HOME with a real config file. + beforeEach(() => { + home = mkdtempSync(join(tmpdir(), "ocx-gcp-sa-routing-")); + process.env.OPENCODEX_HOME = home; + saveConfig({ + port: 10199, + defaultProvider: "google-vertex", + providers: { + "google-vertex": { adapter: "google", baseUrl: "https://aiplatform.googleapis.com", googleMode: "vertex" } as OcxProviderConfig, + }, + } as unknown as OcxConfig); + }); + + afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(home); + }); + + test("routedProviderConfig preserves a gcp-sa marker in apiKey (usableResolvedApiKey pass-through)", () => { + const provider = { adapter: "google", baseUrl: "https://aiplatform.googleapis.com", googleMode: "vertex", apiKey: "gcp-sa:p/abcd1234" } as OcxProviderConfig; + const routed = routedProviderConfig("google-vertex", provider); + expect(routed.apiKey).toBe("gcp-sa:p/abcd1234"); + expect(gcpCredentialMarkerAccount(routed.apiKey)).toBe("p/abcd1234"); + }); + + test("a non-marker key still resolves through the normal path (no regression)", () => { + const provider = { adapter: "google", baseUrl: "https://aiplatform.googleapis.com", googleMode: "vertex", apiKey: "${OCX_ROUTER_TEST_KEY}" } as OcxProviderConfig; + process.env.OCX_ROUTER_TEST_KEY = "resolved-literal"; + try { + const routed = routedProviderConfig("google-vertex", provider); + expect(routed.apiKey).toBe("resolved-literal"); + } finally { + delete process.env.OCX_ROUTER_TEST_KEY; + } + }); +}); + +describe("routed marker reaches the ADC request path (router + adapter, end to end)", () => { + let home: string; + const previousHome = process.env.OPENCODEX_HOME; + + beforeEach(() => { + home = mkdtempSync(join(tmpdir(), "ocx-gcp-sa-e2e-")); + process.env.OPENCODEX_HOME = home; + saveConfig({ + port: 10199, + defaultProvider: "google-vertex", + providers: { + "google-vertex": { adapter: "google", baseUrl: "https://aiplatform.googleapis.com", googleMode: "vertex" } as OcxProviderConfig, + }, + } as unknown as OcxConfig); + }); + + afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(home); + __resetVertexTokenCache(); + setProviderKeychainEntryFactoryForTests(null); + }); + + test("routedProviderConfig output feeds buildRequest and the ADC branch wins", async () => { + const kp = await generateKeyPair(); + const pkcs8 = await globalThis.crypto.subtle.exportKey("pkcs8", kp.privateKey); + const b64 = Buffer.from(pkcs8).toString("base64").match(/.{1,64}/g)!.join("\n"); + const pem = `-----BEGIN PRIVATE KEY-----\n${b64}\n-----END PRIVATE KEY-----\n`; + const serviceAccount = JSON.stringify({ type: "service_account", client_email: "svc@example.test", private_key: pem, private_key_id: "k1" }); + + const realFetch = globalThis.fetch; + globalThis.fetch = (async () => + new Response(JSON.stringify({ access_token: "routed-tok", expires_in: 3600 }), { status: 200 })) as typeof fetch; + setProviderKeychainEntryFactoryForTests((service, account) => ({ + getPassword: () => (account === "p/mk1" ? serviceAccount : null), + setPassword: () => {}, + deletePassword: () => true, + })); + + try { + const provider = { adapter: "google", baseUrl: "https://aiplatform.googleapis.com", googleMode: "vertex", apiKey: "gcp-sa:p/mk1", project: "proj-1", location: "global" } as OcxProviderConfig; + const routed = routedProviderConfig("google-vertex", provider); + const req = await createGoogleAdapter(routed).buildRequest(parsed()); + expect(req.url).toContain("/v1/projects/proj-1/locations/global/publishers/google/models/"); + expect(req.headers["x-goog-api-key"]).toBeUndefined(); + expect((req.headers["Authorization"] ?? "").startsWith("Bearer routed-tok")).toBe(true); + } finally { + globalThis.fetch = realFetch; + } + }); + + test("marker suppresses the GOOGLE_CLOUD_API_KEY env fallback (explicit credential wins)", async () => { + // A marker names ONE credential explicitly — the ambient env fallback must not silently + // route this provider's inference through the env key instead of the named service account. + // Without this guard, a marker + env key would send the ENV key as x-goog-api-key (review + // finding: fallback fires after the marker skips the fast path). + const prevEnvKey = process.env.GOOGLE_CLOUD_API_KEY; + process.env.GOOGLE_CLOUD_API_KEY = "ambient-env-key-should-not-be-used"; + const realFetch = globalThis.fetch; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input instanceof URL ? input.href : (input as Request).url; + if (url === "https://oauth2.googleapis.com/token") { + return new Response(JSON.stringify({ access_token: "env-fallback-tok", expires_in: 3600 }), { status: 200 }); + } + return new Response("nope", { status: 404 }); + }) as typeof fetch; + const sawEnvKey = { value: false }; + const spyFetch = globalThis.fetch; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input instanceof URL ? input.href : (input as Request).url; + if (url.includes("oauth2")) { + return new Response(JSON.stringify({ access_token: "adc-tok", expires_in: 3600 }), { status: 200 }); + } + const headers = (init?.headers ?? {}) as Record; + if (headers["x-goog-api-key"] === "ambient-env-key-should-not-be-used") sawEnvKey.value = true; + return new Response("nope", { status: 404 }); + }) as typeof fetch; + + try { + // Keychain serves the marker credential; env key is set. The adapter must use ADC + // (marker-named credential), never the env key. + const kp = await globalThis.crypto.subtle.generateKey( + { name: "RSASSA-PKCS1-v1_5", modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: "SHA-256" }, + true, ["sign", "verify"], + ); + const pkcs8 = await globalThis.crypto.subtle.exportKey("pkcs8", kp.privateKey); + const b64 = Buffer.from(pkcs8).toString("base64").match(/.{1,64}/g)!.join("\n"); + const pem = `-----BEGIN PRIVATE KEY-----\n${b64}\n-----END PRIVATE KEY-----\n`; + const serviceAccount = JSON.stringify({ type: "service_account", client_email: "svc@example.test", private_key: pem, private_key_id: "k1" }); + setProviderKeychainEntryFactoryForTests((service, account) => ({ + getPassword: () => (account === "p/mk1" ? serviceAccount : null), + setPassword: () => {}, + deletePassword: () => true, + })); + + const provider = { adapter: "google", baseUrl: "https://x", googleMode: "vertex", apiKey: "gcp-sa:p/mk1", project: "proj-1", location: "global" } as OcxProviderConfig; + const req = await createGoogleAdapter(provider).buildRequest(parsed()); + // ADC branch: Bearer + no env key on the fast path + expect(req.headers["x-goog-api-key"]).toBeUndefined(); + expect((req.headers["Authorization"] ?? "").startsWith("Bearer adc-tok")).toBe(true); + expect(sawEnvKey.value).toBe(false); + } finally { + globalThis.fetch = realFetch; + if (prevEnvKey === undefined) delete process.env.GOOGLE_CLOUD_API_KEY; + else process.env.GOOGLE_CLOUD_API_KEY = prevEnvKey; + } + }); +}); + +async function generateKeyPair(): Promise { + return globalThis.crypto.subtle.generateKey( + { name: "RSASSA-PKCS1-v1_5", modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: "SHA-256" }, + true, + ["sign", "verify"], + ) as Promise; +} diff --git a/tests/adapters/google/gcp-sa-marker.test.ts b/tests/adapters/google/gcp-sa-marker.test.ts new file mode 100644 index 00000000000..3afe46d0679 --- /dev/null +++ b/tests/adapters/google/gcp-sa-marker.test.ts @@ -0,0 +1,522 @@ +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { Buffer } from "node:buffer"; +import { mkdtempSync, mkdirSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + GCP_CREDENTIAL_MARKER_PREFIX, + gcpCredentialMarkerAccount, + getVertexAccessToken, + parseGcpCredentialJson, + __resetVertexTokenCache, +} from "../../../src/lib/gcp-adc"; +import { splitCredentialPaste, addProviderApiKey, removeProviderApiKey } from "../../../src/providers/api-keys"; +import { cleanupRemovedGcpCredentials, providerGcpCredentialMarkers } from "../../../src/providers/gcp-credential-cleanup"; +import { setProviderKeychainEntryFactoryForTests } from "../../../src/providers/key-store"; +import { loadConfig, saveConfig } from "../../../src/config"; +import type { OcxConfig, OcxProviderConfig } from "../../../src/types"; +import { removeTreeWithRetry } from "../../helpers/remove-tree"; +import * as destinationPolicy from "../../../src/lib/destination-policy"; +import { providerEditorConfigDTO } from "../../../src/server/auth-cors"; + +const SERVICE_ACCOUNT_JSON = JSON.stringify({ + type: "service_account", + client_email: "svc@example.test", + private_key: "-----BEGIN PRIVATE KEY-----\nAAAA\n-----END PRIVATE KEY-----\n", + private_key_id: "k1", +}); +const SERVICE_ACCOUNT_JSON_2 = JSON.stringify({ + type: "service_account", + client_email: "svc2@example.test", + private_key: "-----BEGIN PRIVATE KEY-----\nBBBB\n-----END PRIVATE KEY-----\n", + private_key_id: "k2", +}); + +afterEach(() => { + __resetVertexTokenCache(); + setProviderKeychainEntryFactoryForTests(null); +}); + +describe("parseGcpCredentialJson", () => { + test("accepts a service_account JSON body", () => { + const creds = parseGcpCredentialJson(SERVICE_ACCOUNT_JSON); + expect(creds?.type).toBe("service_account"); + }); + + test("accepts an authorized_user JSON body", () => { + const creds = parseGcpCredentialJson(JSON.stringify({ type: "authorized_user", client_id: "c", client_secret: "s", refresh_token: "r" })); + expect(creds?.type).toBe("authorized_user"); + }); + + test("rejects unknown types, non-JSON, and plain keys", () => { + expect(parseGcpCredentialJson(JSON.stringify({ type: "external_account" }))).toBeUndefined(); + expect(parseGcpCredentialJson("not json")).toBeUndefined(); + expect(parseGcpCredentialJson("AIza-real-key-123456")).toBeUndefined(); + expect(parseGcpCredentialJson("{}")).toBeUndefined(); + }); + + test("rejects incomplete service_account missing required fields", () => { + expect(parseGcpCredentialJson(JSON.stringify({ type: "service_account" }))).toBeUndefined(); + expect(parseGcpCredentialJson(JSON.stringify({ type: "service_account", client_email: "svc@test" }))).toBeUndefined(); + expect(parseGcpCredentialJson(JSON.stringify({ type: "service_account", private_key: "k" }))).toBeUndefined(); + expect(parseGcpCredentialJson(JSON.stringify({ type: "service_account", client_email: " ", private_key: "k" }))).toBeUndefined(); + }); + + test("rejects incomplete authorized_user missing required fields", () => { + expect(parseGcpCredentialJson(JSON.stringify({ type: "authorized_user" }))).toBeUndefined(); + expect(parseGcpCredentialJson(JSON.stringify({ type: "authorized_user", client_id: "c", client_secret: "s" }))).toBeUndefined(); + expect(parseGcpCredentialJson(JSON.stringify({ type: "authorized_user", client_id: "c", refresh_token: "r" }))).toBeUndefined(); + expect(parseGcpCredentialJson(JSON.stringify({ type: "authorized_user", client_id: "c", client_secret: "s", refresh_token: " " }))).toBeUndefined(); + }); +}); + +describe("gcpCredentialMarkerAccount", () => { + test("extracts the account from a marker", () => { + expect(gcpCredentialMarkerAccount(`${GCP_CREDENTIAL_MARKER_PREFIX}p/abc123`)).toBe("p/abc123"); + expect(() => gcpCredentialMarkerAccount("gcp-sa:")).toThrow(/non-empty account/); + expect(() => gcpCredentialMarkerAccount("gcp-sa: ")).toThrow(/non-empty account/); + expect(gcpCredentialMarkerAccount("keychain:p/abc")).toBeUndefined(); + expect(gcpCredentialMarkerAccount(undefined)).toBeUndefined(); + }); +}); + +describe("splitCredentialPaste", () => { + test("a plain API key is one literal row", () => { + expect(splitCredentialPaste("sk-real-key")).toEqual(["sk-real-key"]); + }); + + test("one credential JSON parses to one row", () => { + const parts = splitCredentialPaste(SERVICE_ACCOUNT_JSON); + expect(parts).toHaveLength(1); + expect(parts[0]).toEqual({ credentialJson: SERVICE_ACCOUNT_JSON }); + }); + + test("pretty-printed JSON (with newlines) parses", () => { + const pretty = JSON.stringify(JSON.parse(SERVICE_ACCOUNT_JSON), null, 2); + const parts = splitCredentialPaste(pretty); + expect(parts).toHaveLength(1); + expect(parts[0]).toEqual({ credentialJson: pretty }); + }); + + test("two adjacent credential JSONs split into two rows (rotation pool)", () => { + const parts = splitCredentialPaste(`${SERVICE_ACCOUNT_JSON}, ${SERVICE_ACCOUNT_JSON_2}`); + expect(parts).toHaveLength(2); + expect(parts[0]).toEqual({ credentialJson: SERVICE_ACCOUNT_JSON }); + expect(parts[1]).toEqual({ credentialJson: SERVICE_ACCOUNT_JSON_2 }); + }); + + test("a file path is rejected with paste guidance", () => { + for (const path of ["C:\\Users\\me\\sa.json", "/tmp/sa.json", "./sa.json", "../sa.json", "~/sa.json", "credentials/sa.json", "sa.json", "\\\\host\\share\\sa.json"]) { + expect(() => splitCredentialPaste(path)).toThrow(/file path/i); + } + }); + + test("a single-line brace-leading NON-JSON literal falls back to a plain key (never eaten)", () => { + // parseGcpCredentialJson contract: a literal key starting with `{` is never eaten — + // but only for NON-JSON values. Prana review: a parseable JSON with an unrecognized type + // must be rejected, not saved as plaintext (it would go to the wire as a key). + expect(splitCredentialPaste("{abc:def}")).toEqual(["{abc:def}"]); + }); + + test("a single-line PARSEABLE JSON with an unrecognized type is REJECTED (prana finding 2)", () => { + // `external_account` is a real JSON object — saving it as a plaintext literal key would + // send it verbatim to the wire. Reject with guidance instead. + expect(() => splitCredentialPaste('{"type": "external_account"}')).toThrow(); + }); + + test("a MULTILINE JSON without a recognized type field is rejected with guidance", () => { + // Multi-line braces are not a plausible literal key — guidance error. + const multiline = JSON.stringify({ type: "external_account", token_url: "https://sts.example" }, null, 2); + expect(() => splitCredentialPaste(multiline)).toThrow(/type/i); + }); + + test("text between JSON objects is rejected, not silently discarded", () => { + const sa1 = JSON.parse(SERVICE_ACCOUNT_JSON); + expect(() => splitCredentialPaste(`${JSON.stringify(sa1)} garbage ${JSON.stringify(sa1)}`)).toThrow(/unexpected content|Paste credential/i); + }); + + test("text after the last credential JSON is rejected", () => { + for (const input of [SERVICE_ACCOUNT_JSON, `${SERVICE_ACCOUNT_JSON}, ${SERVICE_ACCOUNT_JSON_2}`]) { + expect(() => splitCredentialPaste(`${input} trailing-garbage`)).toThrow(/after the last JSON object/i); + } + }); + + test("separators after the last credential JSON remain valid", () => { + expect(splitCredentialPaste(`${SERVICE_ACCOUNT_JSON}, \n\t`)).toEqual([{ credentialJson: SERVICE_ACCOUNT_JSON }]); + }); + + test("truncated JSON is rejected", () => { + expect(() => splitCredentialPaste('{"type": "service_account", "client_email"')).toThrow(); + }); +}); + +describe("addProviderApiKey with a pasted credential JSON", () => { + let home: string; + const previousHome = process.env.OPENCODEX_HOME; + + // commitProviderApiKeySelection persists through mutatePersistedConfig, which refuses to + // mutate unless a real config file exists — so each test runs against a throwaway home dir. + beforeEach(() => { + home = mkdtempSync(join(tmpdir(), "ocx-gcp-sa-marker-")); + process.env.OPENCODEX_HOME = home; + }); + + afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(home); + }); + + function makeConfig(provider: Partial): OcxConfig { + const config = { + port: 10199, + defaultProvider: "p", + providers: { + p: { adapter: "openai-chat", baseUrl: "https://api.example.com/v1", ...provider } as OcxProviderConfig, + }, + } as unknown as OcxConfig; + saveConfig(config); + return config; + } + + const keychainStore = new Map(); + + function installFakeKeychain(): void { + keychainStore.clear(); + setProviderKeychainEntryFactoryForTests((service, account) => ({ + getPassword: () => keychainStore.get(`${service}:${account}`) ?? null, + setPassword: (password: string) => { keychainStore.set(`${service}:${account}`, password); }, + deletePassword: () => keychainStore.delete(`${service}:${account}`), + })); + } + + test("JSON is diverted to the keychain and only the marker lands in config", () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + const result = addProviderApiKey(config, "p", SERVICE_ACCOUNT_JSON); + expect(result).toEqual({ id: expect.any(String) }); + + const provider = config.providers.p!; + // config holds ONLY the marker — no key material anywhere. + expect(provider.apiKey?.startsWith(GCP_CREDENTIAL_MARKER_PREFIX)).toBe(true); + expect(JSON.stringify(config).includes("private_key")).toBe(false); + // The secret itself lives in the credential store under the marker's account. + const account = gcpCredentialMarkerAccount(provider.apiKey)!; + expect(account.startsWith("p/")).toBe(true); + expect(keychainStore.get(`opencodex.provider-api-key.v1:${account}`)).toBe(SERVICE_ACCOUNT_JSON); + }); + + test("re-adding the same JSON upserts instead of duplicating", () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + const first = addProviderApiKey(config, "p", SERVICE_ACCOUNT_JSON); + const second = addProviderApiKey(config, "p", SERVICE_ACCOUNT_JSON); + expect(first).toEqual(second); + const pool = config.providers.p!.apiKeyPool!; + expect(pool).toHaveLength(2); // legacy seed + the single marker entry + }); + + test("a file path paste returns an error and stores nothing", () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + const result = addProviderApiKey(config, "p", "C:\\Users\\me\\sa.json"); + expect("error" in result && /file path/i.test(result.error)).toBe(true); + expect(JSON.stringify(config).includes(GCP_CREDENTIAL_MARKER_PREFIX)).toBe(false); + }); + + test("keychain-unavailable fails closed with an error", () => { + // No fake factory installed: the real keyring may exist in CI, so probe through the seam + // with a factory that always throws instead. + setProviderKeychainEntryFactoryForTests(() => { + throw new Error("keychain unavailable in test"); + }); + const config = makeConfig({ apiKey: "legacy-key" }); + const result = addProviderApiKey(config, "p", SERVICE_ACCOUNT_JSON); + expect("error" in result).toBe(true); + expect(JSON.stringify(config).includes(GCP_CREDENTIAL_MARKER_PREFIX)).toBe(false); + }); + + test("multiple JSON credentials commit together, deduplicate and select the first", () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + const pasted = `${SERVICE_ACCOUNT_JSON}, ${SERVICE_ACCOUNT_JSON_2}, ${SERVICE_ACCOUNT_JSON}`; + expect(addProviderApiKey(config, "p", pasted)).toEqual({ id: expect.any(String) }); + const pool = config.providers.p!.apiKeyPool!; + expect(pool).toHaveLength(3); + expect(config.providers.p!.apiKey).toBe(pool[1]!.key); + expect([...keychainStore.values()].sort()).toEqual([SERVICE_ACCOUNT_JSON, SERVICE_ACCOUNT_JSON_2].sort()); + expect(JSON.stringify(config)).not.toContain("private_key"); + expect(addProviderApiKey(config, "p", pasted)).toEqual({ id: pool[1]!.id }); + expect(config.providers.p!.apiKeyPool).toHaveLength(3); + }); + + test("a failed second write rolls back new credentials but preserves existing upserts", () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + addProviderApiKey(config, "p", SERVICE_ACCOUNT_JSON); + const before = JSON.stringify(config); + setProviderKeychainEntryFactoryForTests((service, account) => ({ + getPassword: () => keychainStore.get(`${service}:${account}`) ?? null, + setPassword: value => { if (value === SERVICE_ACCOUNT_JSON_2) throw new Error("fixture write failed"); keychainStore.set(`${service}:${account}`, value); }, + deletePassword: () => keychainStore.delete(`${service}:${account}`), + })); + expect(addProviderApiKey(config, "p", `${SERVICE_ACCOUNT_JSON}, ${SERVICE_ACCOUNT_JSON_2}`)).toHaveProperty("error"); + expect(JSON.stringify(config)).toBe(before); + expect([...keychainStore.values()]).toEqual([SERVICE_ACCOUNT_JSON]); + }); + + test("a failed batch config commit removes new secrets and preserves existing credentials", () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + addProviderApiKey(config, "p", SERVICE_ACCOUNT_JSON); + const removed = config.providers.p!; + config.providers.fallback = { adapter: "openai-chat", baseUrl: "https://api.example.test/v1", apiKey: "fixture" }; + config.defaultProvider = "fallback"; + delete config.providers.p; + saveConfig(config); + config.providers.p = removed; + expect(addProviderApiKey(config, "p", `${SERVICE_ACCOUNT_JSON}, ${SERVICE_ACCOUNT_JSON_2}`)).toHaveProperty("error"); + expect([...keychainStore.values()]).toEqual([SERVICE_ACCOUNT_JSON]); + }); + + test("a later write failure removes every newly created secret without a partial pool", () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + const before = JSON.stringify(config); + setProviderKeychainEntryFactoryForTests((service, account) => ({ + getPassword: () => keychainStore.get(`${service}:${account}`) ?? null, + setPassword: value => { if (value === SERVICE_ACCOUNT_JSON_2) throw new Error("fixture write failed"); keychainStore.set(`${service}:${account}`, value); }, + deletePassword: () => keychainStore.delete(`${service}:${account}`), + })); + expect(addProviderApiKey(config, "p", `${SERVICE_ACCOUNT_JSON}, ${SERVICE_ACCOUNT_JSON_2}`)).toHaveProperty("error"); + expect(JSON.stringify(config)).toBe(before); + expect(keychainStore.size).toBe(0); + }); + + test("a refused removal leaves the credential intact", () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + const result = addProviderApiKey(config, "p", SERVICE_ACCOUNT_JSON); + if ("error" in result) throw new Error(result.error); + const provider = config.providers.p!; + config.providers.fallback = { adapter: "openai-chat", baseUrl: "https://api.example.test/v1", apiKey: "fixture" }; + config.defaultProvider = "fallback"; + delete config.providers.p; + saveConfig(config); + config.providers.p = provider; + expect(removeProviderApiKey(config, "p", result.id)).toBe(false); + expect([...keychainStore.values()]).toEqual([SERVICE_ACCOUNT_JSON]); + }); + + test("key removal deletes its secret only after successful config removal", () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + const result = addProviderApiKey(config, "p", SERVICE_ACCOUNT_JSON); + if ("error" in result) throw new Error(result.error); + expect(removeProviderApiKey(config, "p", "missing")).toBe(false); + expect([...keychainStore.values()]).toEqual([SERVICE_ACCOUNT_JSON]); + expect(removeProviderApiKey(config, "p", result.id)).toBe(true); + expect(keychainStore.size).toBe(0); + expect(config.providers.p!.apiKey).toBe("legacy-key"); + }); + + test("provider cleanup preserves shared markers until their last reference is removed", () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + addProviderApiKey(config, "p", SERVICE_ACCOUNT_JSON); + const markers = providerGcpCredentialMarkers(config.providers.p!); + config.providers.other = { ...config.providers.p!, apiKeyPool: undefined }; + config.defaultProvider = "other"; + delete config.providers.p; + saveConfig(config); + cleanupRemovedGcpCredentials(markers); + expect([...keychainStore.values()]).toEqual([SERVICE_ACCOUNT_JSON]); + config.providers.fallback = { adapter: "openai-chat", baseUrl: "https://api.example.test/v1", apiKey: "fixture" }; + config.defaultProvider = "fallback"; + delete config.providers.other; + saveConfig(config); + cleanupRemovedGcpCredentials(markers); + expect(keychainStore.size).toBe(0); + }); + + test("CLI provider removal cleans all removed GCP credentials", async () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + addProviderApiKey(config, "p", `${SERVICE_ACCOUNT_JSON}, ${SERVICE_ACCOUNT_JSON_2}`); + config.providers.fallback = { adapter: "openai-chat", baseUrl: "https://api.example.test/v1", apiKey: "fixture" }; + config.defaultProvider = "fallback"; + saveConfig(config); + const { handleProviderCommand } = await import("../../../src/cli/provider"); + await handleProviderCommand(["remove", "p", "--json"]); + expect(loadConfig().providers.p).toBeUndefined(); + expect(keychainStore.size).toBe(0); + }); + + test.each(["PUT", "POST"])("management %s cleans replaced markers only after commit", async method => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + addProviderApiKey(config, "p", `${SERVICE_ACCOUNT_JSON}, ${SERVICE_ACCOUNT_JSON_2}`); + config.providers.fallback = { adapter: "openai-chat", baseUrl: "https://api.example.test/v1", apiKey: "fixture" }; + saveConfig(config); + const resolver = spyOn(destinationPolicy, "providerDestinationResolvedError").mockResolvedValue(null); + try { + const { handleProviderRoutes } = await import("../../../src/server/management/provider-routes"); + const request = async (body: unknown, failSave = false) => { + const req = new Request("http://localhost/api/providers", { + method, headers: { "Content-Type": "application/json" }, body: JSON.stringify(body), + }); + return handleProviderRoutes({ req, url: new URL(req.url), config, + deps: { saveConfigPreservingClaudeCode: (next: OcxConfig) => { + if (failSave) throw new Error("fixture save refused"); + saveConfig(next); + } }, + convergeCodexCatalog: async () => ({ converged: true }), + syncClaudeAgentDefsBestEffort: async () => {}, + } as unknown as import("../../../src/server/management/context").ManagementContext); + }; + const replacement = { adapter: "openai-chat", baseUrl: "https://replacement.example.test/v1", apiKey: "replacement" }; + const baseline = providerEditorConfigDTO(loadConfig()); + const next = structuredClone(baseline); + delete next.providers.p; + next.defaultProvider = "fallback"; + const body = method === "PUT" ? { baseline, next } : { name: "p", provider: replacement }; + if (method === "PUT") { + const refused = await request({ baseline: { ...baseline, defaultProvider: "fallback" }, next }); + expect(refused?.status).toBe(409); + } else { + await expect(request(body, true)).rejects.toThrow("fixture save refused"); + } + expect(keychainStore.size).toBe(2); + expect((await request(body))?.status).toBe(200); + expect(keychainStore.size).toBe(0); + if (method === "PUT") expect(loadConfig().providers.p).toBeUndefined(); + else expect(loadConfig().providers.p!.apiKey).toBe("replacement"); + } finally { resolver.mockRestore(); } + }); + + test("management provider removal cleans all removed GCP credentials", async () => { + installFakeKeychain(); + const config = makeConfig({ apiKey: "legacy-key" }); + addProviderApiKey(config, "p", `${SERVICE_ACCOUNT_JSON}, ${SERVICE_ACCOUNT_JSON_2}`); + config.providers.fallback = { adapter: "openai-chat", baseUrl: "https://api.example.test/v1", apiKey: "fixture" }; + saveConfig(config); + const { handleProviderRoutes } = await import("../../../src/server/management/provider-routes"); + const req = new Request("http://localhost/api/providers?name=p", { method: "DELETE" }); + const response = await handleProviderRoutes({ + req, url: new URL(req.url), config, deps: {}, + convergeCodexCatalog: async () => ({ converged: true }), + } as unknown as import("../../../src/server/management/context").ManagementContext); + expect(response?.status).toBe(200); + expect(loadConfig().providers.p).toBeUndefined(); + expect(keychainStore.size).toBe(0); + }); +}); + +describe("gcp-adc marker source", () => { + let oauthCalls = 0; + const realFetch = globalThis.fetch; + const prevEnv: Record = {}; + + afterEach(() => { + globalThis.fetch = realFetch; + for (const [k, v] of Object.entries(prevEnv)) { + if (v === undefined) delete process.env[k]; + else process.env[k] = v; + delete prevEnv[k]; + } + }); + + // The marker-account tests pass the account per request (options.markerAccount) — no + // process-global registration anywhere. Host ADC state is fully isolated: an empty + // CLOUDSDK_CONFIG dir, no GOOGLE_APPLICATION_CREDENTIALS, and a 404 fetching stub, so + // no test can make a real network call or depend on the developer's gcloud login. + function isolateHostAdc(): void { + prevEnv.GOOGLE_APPLICATION_CREDENTIALS ??= process.env.GOOGLE_APPLICATION_CREDENTIALS; + delete process.env.GOOGLE_APPLICATION_CREDENTIALS; + prevEnv.CLOUDSDK_CONFIG ??= process.env.CLOUDSDK_CONFIG; + process.env.CLOUDSDK_CONFIG = join(tmpdir(), `ocx-gcp-sa-isolated-${Date.now()}`); + mkdirSync(process.env.CLOUDSDK_CONFIG, { recursive: true }); + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input instanceof URL ? input.href : (input as Request).url; + if (url === "https://oauth2.googleapis.com/token") { + oauthCalls++; + return new Response(JSON.stringify({ access_token: "marker-tok", expires_in: 3600 }), { status: 200 }); + } + return new Response("nope", { status: 404 }); + }) as typeof fetch; + } + + test("a marker account resolves its keychain credential through the JWT exchange", async () => { + // A real, signable RSA key (generated) so the RS256 JWT exchange inside the resolver works. + const kp = await globalThis.crypto.subtle.generateKey( + { name: "RSASSA-PKCS1-v1_5", modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: "SHA-256" }, + true, + ["sign", "verify"], + ); + const pkcs8 = await globalThis.crypto.subtle.exportKey("pkcs8", kp.privateKey); + const b64 = Buffer.from(pkcs8).toString("base64").match(/.{1,64}/g)!.join("\n"); + const pem = `-----BEGIN PRIVATE KEY-----\n${b64}\n-----END PRIVATE KEY-----\n`; + const serviceAccount = JSON.stringify({ + type: "service_account", + client_email: "svc@example.test", + private_key: pem, + private_key_id: "k1", + }); + setProviderKeychainEntryFactoryForTests((service, account) => ({ + getPassword: () => (account === "p/abc123" ? serviceAccount : null), + setPassword: () => {}, + deletePassword: () => true, + })); + isolateHostAdc(); + + const token = await getVertexAccessToken({ markerAccount: "p/abc123" }); + expect(token).toBe("marker-tok"); + expect(oauthCalls).toBe(1); + }); + + test("an unreadable marker credential fails closed (no token, no silent fallback to metadata)", async () => { + setProviderKeychainEntryFactoryForTests((service, account) => ({ + getPassword: () => null, + setPassword: () => {}, + deletePassword: () => true, + })); + isolateHostAdc(); + await expect(getVertexAccessToken({ markerAccount: "p/missing" })).rejects.toThrow(/gcp-sa:p\/missing/); + }); + + test("no marker account falls through to the normal source priority (isolated: fails with ADC guidance)", async () => { + isolateHostAdc(); + // With no marker and no env/file ADC, the resolver reaches the metadata-server attempt and + // fails with the standard ADC guidance — proving the no-marker path is unchanged. + await expect(getVertexAccessToken()).rejects.toThrow(/Application Default Credentials/); + }); + + test("a marker does NOT fall back to GOOGLE_CLOUD_API_KEY when ADC resolution succeeds via keychain", async () => { + // Prana review: with a marker set, the vertex adapter must suppress the env fast path — + // the keychain credential wins over the ambient env key (request-per-request ownership). + const kp = await globalThis.crypto.subtle.generateKey( + { name: "RSASSA-PKCS1-v1_5", modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: "SHA-256" }, + true, + ["sign", "verify"], + ); + const pkcs8 = await globalThis.crypto.subtle.exportKey("pkcs8", kp.privateKey); + const b64 = Buffer.from(pkcs8).toString("base64").match(/.{1,64}/g)!.join("\n"); + const pem = `-----BEGIN PRIVATE KEY-----\n${b64}\n-----END PRIVATE KEY-----\n`; + const serviceAccount = JSON.stringify({ + type: "service_account", + client_email: "svc@example.test", + private_key: pem, + private_key_id: "k1", + }); + setProviderKeychainEntryFactoryForTests((service, account) => ({ + getPassword: () => (account === "p/own" ? serviceAccount : null), + setPassword: () => {}, + deletePassword: () => true, + })); + isolateHostAdc(); + oauthCalls = 0; // this describe's counter is shared across tests — reset for this probe + const tok = await getVertexAccessToken({ markerAccount: "p/own" }); + expect(tok).toBe("marker-tok"); + expect(oauthCalls).toBe(1); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 67e3147d48d..a7b4f6e432b 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1,4 +1,5 @@ { + "gcp-sa-marker.test.ts": "adapters/google", "gcp-sa-adapter.test.ts": "adapters/google", "management-gcp-credential-input.test.ts": "server", "codex-history-pending-wal.test.ts": "codex-integration", "service-log.test.ts": "service", "service-health-sentinel-policy.test.ts": "service", diff --git a/tests/server/management-gcp-credential-input.test.ts b/tests/server/management-gcp-credential-input.test.ts new file mode 100644 index 00000000000..12018a08bef --- /dev/null +++ b/tests/server/management-gcp-credential-input.test.ts @@ -0,0 +1,397 @@ +import { afterEach, beforeEach, expect, mock, spyOn, test } from "bun:test"; +import { mkdtempSync, readFileSync, readdirSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveConfig, loadConfig } from "../../src/config"; +import * as configModule from "../../src/config"; +import { addProviderApiKey, removeProviderApiKey } from "../../src/providers/api-keys"; +import { cleanupRemovedGcpCredentials, snapshotProviderGcpCredentialMarkers } from "../../src/providers/gcp-credential-cleanup"; +import { setProviderKeychainEntryFactoryForTests, storeProviderKeyInKeychain, restoreProviderKeyFromKeychain, providerKeyStoreKind } from "../../src/providers/key-store"; +import { handleProviderRoutes } from "../../src/server/management/provider-routes"; +import { handleProviderCommand } from "../../src/cli/provider"; +import * as destinationPolicy from "../../src/lib/destination-policy"; +import type { OcxConfig } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const credential = JSON.stringify({ type: "authorized_user", client_id: "test-client", client_secret: "fixture-secret", refresh_token: "fixture-refresh" }); +let home: string; +let previousHome: string | undefined; +let config: OcxConfig; +const secrets = new Map(); + +beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + home = mkdtempSync(join(tmpdir(), "ocx-gcp-input-")); + process.env.OPENCODEX_HOME = home; + secrets.clear(); + setProviderKeychainEntryFactoryForTests((service, account) => ({ + getPassword: () => secrets.get(`${service}:${account}`) ?? null, + setPassword: value => { secrets.set(`${service}:${account}`, value); }, + deletePassword: () => secrets.delete(`${service}:${account}`), + })); + spyOn(destinationPolicy, "providerDestinationResolvedError").mockResolvedValue(null); + config = { port: 10199, defaultProvider: "relay", providers: { + relay: { adapter: "openai-chat", baseUrl: "https://relay.example/v1", apiKey: "old-key" }, + } }; + saveConfig(config); +}); + +afterEach(() => { + mock.restore(); + setProviderKeychainEntryFactoryForTests(null); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(home); +}); + +function assertNoPlaintext() { + for (const file of readdirSync(home).filter(file => file.startsWith("config.json"))) { + const content = readFileSync(join(home, file), "utf8"); + expect(content).not.toContain("fixture-secret"); + expect(content).not.toContain("fixture-refresh"); + } +} + +test("management credential snapshot captures both live and valid durable provider rows", () => { + const live = { ...config.providers.relay!, apiKey: "gcp-sa:live" }; + config.providers.relay!.apiKey = "gcp-sa:durable"; + saveConfig(config); + expect(snapshotProviderGcpCredentialMarkers("relay", live)).toEqual(["gcp-sa:live", "gcp-sa:durable"]); + expect(snapshotProviderGcpCredentialMarkers("relay", undefined)).toEqual(["gcp-sa:durable"]); + expect(secrets.size).toBe(0); +}); + +test.each(["", "{not-json"])("management credential snapshot ignores an invalid durable config: %j", raw => { + const live = { ...config.providers.relay!, apiKey: "gcp-sa:live" }; + writeFileSync(join(home, "config.json"), raw); + expect(snapshotProviderGcpCredentialMarkers("relay", live)).toEqual(["gcp-sa:live"]); + expect(readFileSync(join(home, "config.json"), "utf8")).toBe(raw); + expect(secrets.size).toBe(0); +}); + +test.each(["gcp-sa:", "gcp-sa: "])("cleanup skips malformed persisted markers without blocking valid cleanup: %j", malformed => { + expect(addProviderApiKey(config, "relay", credential)).toHaveProperty("id"); + const retained = config.providers.relay!.apiKey!; + config.providers.removed = { adapter: "openai-chat", baseUrl: "https://relay.example/v1" }; + saveConfig(config); + expect(addProviderApiKey(config, "removed", JSON.stringify({ ...JSON.parse(credential), client_id: "removed-client" }))).toHaveProperty("id"); + const removed = config.providers.removed!.apiKey!; + delete config.providers.removed; + config.providers.unrelated = { adapter: "openai-chat", baseUrl: "https://relay.example/v1", apiKey: malformed, + apiKeyPool: [{ id: "bad", key: malformed }] }; + saveConfig(config); + const warn = spyOn(console, "warn").mockImplementation(() => {}); + cleanupRemovedGcpCredentials([removed, retained]); + expect(secrets.size).toBe(1); + expect([...secrets.values()]).toEqual([credential]); + expect(loadConfig().providers.unrelated!.apiKey).toBe(malformed); + expect(warn).not.toHaveBeenCalled(); +}); + +test.each(["gcp-sa:", "gcp-sa: "])("cleanup skips malformed captured markers and preserves keychain aliases: %j", malformed => { + expect(addProviderApiKey(config, "relay", credential)).toHaveProperty("id"); + const retained = config.providers.relay!.apiKey!; + config.providers.removed = { adapter: "openai-chat", baseUrl: "https://relay.example/v1" }; + saveConfig(config); + expect(addProviderApiKey(config, "removed", JSON.stringify({ ...JSON.parse(credential), client_id: "removed-client" }))).toHaveProperty("id"); + const removed = config.providers.removed!.apiKey!; + delete config.providers.removed; + config.providers.relay!.apiKey = `keychain:${retained.slice("gcp-sa:".length)}`; + delete config.providers.relay!.apiKeyPool; + saveConfig(config); + const warn = spyOn(console, "warn").mockImplementation(() => {}); + cleanupRemovedGcpCredentials([malformed, removed, retained]); + expect(secrets.size).toBe(1); + expect([...secrets.values()]).toEqual([credential]); + expect(warn).not.toHaveBeenCalled(); +}); + +test.each([ + { name: "new-provider", values: { apiKey: credential } }, + { name: "relay", values: { apiKey: credential } }, + { name: "relay", values: { apiKey: credential, apiKeyPool: [{ id: "raw", key: credential }] } }, + { name: "relay", values: { apiKey: "plain-key", apiKeyPool: [{ id: "raw", key: credential }] } }, + { name: "relay", values: { apiKey: `plain-key, ${credential}` } }, +])("provider create/overwrite refuses raw JSON before saving: %j", async ({ name, values }) => { + const before = structuredClone(config); + const url = new URL("http://127.0.0.1/api/providers"); + const response = await handleProviderRoutes({ + req: new Request(url, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ + name, provider: { adapter: "openai-chat", baseUrl: "https://relay.example/v1", ...values }, + }) }), url, config, deps: {}, version: "test", trustedLoopbackIngress: true, guiSessionIssuance: null, + convergeCodexCatalog: async () => ({ status: "committed", changed: false, degraded: false, notices: [] }), + syncClaudeAgentDefsBestEffort: async () => {}, + }); + expect(response?.status).toBe(400); + expect(await response!.json()).toMatchObject({ error: expect.stringMatching(/API-key|credential|JSON/i) }); + expect(config).toEqual(before); + expect(loadConfig().providers).toEqual(before.providers); + expect(secrets.size).toBe(0); + assertNoPlaintext(); +}); + +test.each([`plain-key, ${credential}`, `${credential}, plain-key`])("mixed key/JSON pool paste is entirely rejected", paste => { + const before = structuredClone(config); + const result = addProviderApiKey(config, "relay", paste); + expect(result).toMatchObject({ error: expect.stringMatching(/Mixing|separate/i) }); + expect(config).toEqual(before); + expect(secrets.size).toBe(0); + assertNoPlaintext(); +}); + +test("dedicated pool paste persists markers only, including subsequent backups", () => { + expect(addProviderApiKey(config, "relay", credential)).toHaveProperty("id"); + expect(config.providers.relay!.apiKey).toStartWith("gcp-sa:"); + saveConfig(config); + expect([...secrets.values()]).toEqual([credential]); + assertNoPlaintext(); +}); + +test("CLI provider add refuses credential JSON before config or backups change", async () => { + const before = readFileSync(join(home, "config.json"), "utf8"); + const previousExitCode = process.exitCode; + const output = spyOn(console, "error").mockImplementation(() => {}); + try { + await handleProviderCommand(["add", "relay", "--force", "--adapter", "openai-chat", "--base-url", "https://relay.example/v1", "--api-key", credential]); + expect(process.exitCode).toBe(2); + expect(output.mock.calls.flat().join(" ")).toContain("API-key pool endpoint"); + expect(output.mock.calls.flat().join(" ")).not.toContain("fixture-secret"); + expect(readFileSync(join(home, "config.json"), "utf8")).toBe(before); + expect(secrets.size).toBe(0); + assertNoPlaintext(); + } finally { process.exitCode = previousExitCode ?? 0; } +}); + +test("OS-store failures propagate without echoing credential material or committing a pool", () => { + const before = structuredClone(config); + setProviderKeychainEntryFactoryForTests(() => ({ + getPassword: () => null, + setPassword: () => { throw new Error(`store failed: ${credential}`); }, + deletePassword: () => {}, + })); + const result = addProviderApiKey(config, "relay", credential); + expect(result).toMatchObject({ error: expect.stringContaining("credential storage failed") }); + expect(JSON.stringify(result)).not.toContain("fixture-secret"); + expect(config).toEqual(before); + assertNoPlaintext(); +}); + +test.each(["openai", "cursor"].flatMap(name => [credential, + JSON.stringify({ type: "service_account", client_email: "fixture@example.test", private_key: "synthetic-test-key" }), + JSON.stringify({ type: "external_account", client_secret: "fixture-secret" }), +].map(input => ({ name, input }))))("CLI refuses credential JSON before ignoring registry key (%j)", async ({ name, input }) => { + const before = readFileSync(join(home, "config.json"), "utf8"); + const files = readdirSync(home).filter(file => file.startsWith("config.json")); + const previousExitCode = process.exitCode; + const error = spyOn(console, "error").mockImplementation(() => {}); + const warning = spyOn(console, "warn").mockImplementation(() => {}); + try { + process.exitCode = 0; + await handleProviderCommand(["add", name, "--api-key", input]); + expect(process.exitCode).toBe(2); + expect(error).toHaveBeenCalled(); + expect(error.mock.calls.flat().join(" ")).not.toContain("fixture-secret"); + expect(error.mock.calls.flat().join(" ")).not.toContain("synthetic-test-key"); + expect(warning).not.toHaveBeenCalled(); + expect(readFileSync(join(home, "config.json"), "utf8")).toBe(before); + expect(readdirSync(home).filter(file => file.startsWith("config.json"))).toEqual(files); + expect(secrets.size).toBe(0); + } finally { process.exitCode = previousExitCode ?? 0; } +}); + +test.each(["openai", "cursor"])("CLI still ignores ordinary keys for registry provider %s", async name => { + const warning = spyOn(console, "warn").mockImplementation(() => {}); + spyOn(console, "log").mockImplementation(() => {}); + await handleProviderCommand(["add", name, "--api-key", "synthetic-unused-key"]); + expect(warning.mock.calls.flat().join(" ")).toContain("--api-key is ignored"); + expect(loadConfig().providers[name]).toBeDefined(); + expect(loadConfig().providers[name]!.apiKey).not.toBe("synthetic-unused-key"); + expect(secrets.size).toBe(0); +}); + +const overwriteArgs = ["add", "relay", "--force", "--adapter", "openai-chat", "--base-url", "https://relay.example/v1", "--api-key", "replacement-key"]; + +test.each(["none", "gcp-sa", "keychain"])("CLI overwrite includes a newer durable row, retaining %s shared references", async shared => { + expect(addProviderApiKey(config, "relay", credential)).toHaveProperty("id"); + const lateCredential = JSON.stringify({ ...JSON.parse(credential), client_id: "late-client" }); + const lock = configModule.withConfigMutationLockSync; + let wrote = false, lateAccount = ""; + if (shared !== "none") { + const originalRow = structuredClone(config.providers.relay!); + const added = addProviderApiKey(config, "relay", lateCredential); + if ("error" in added) throw new Error(added.error); + lateAccount = config.providers.relay!.apiKeyPool!.find(entry => entry.id === added.id)!.key.slice("gcp-sa:".length); + config.providers.relay = originalRow; + config.providers.shared = { adapter: "openai-chat", baseUrl: "https://shared.example/v1", apiKey: `${shared}:${lateAccount}` }; + saveConfig(config); + } + spyOn(configModule, "withConfigMutationLockSync").mockImplementation((fn: () => T, dir?: string): T => { + if (!wrote) { + wrote = true; + const fresh = loadConfig(); + fresh.providers.relay!.apiKey = undefined; + fresh.providers.relay!.apiKeyPool = undefined; + expect(addProviderApiKey(fresh, "relay", lateCredential)).toHaveProperty("id"); + lateAccount = fresh.providers.relay!.apiKey!.slice("gcp-sa:".length); + saveConfig(fresh); + } + return lock(fn, dir); + }); + spyOn(console, "log").mockImplementation(() => {}); + await handleProviderCommand(overwriteArgs); + expect(wrote).toBe(true); + const saved = loadConfig(); + expect(saved.providers.relay!.apiKey).toBe("replacement-key"); + expect(saved.providers.relay!.apiKeyPool).toBeUndefined(); + if (shared !== "none") expect(saved.providers.shared!.apiKey).toBe(`${shared}:${lateAccount}`); + expect(secrets.size).toBe(shared === "none" ? 0 : 1); + if (shared !== "none") expect(secrets.get(`opencodex.provider-api-key.v1:${lateAccount}`)).toBe(lateCredential); + assertNoPlaintext(); +}); + +test.each(["none", "gcp-sa", "keychain"])("CLI overwrite cleans old pool credentials after commit, preserving %s shared references", async shared => { + expect(addProviderApiKey(config, "relay", credential)).toHaveProperty("id"); + const retainedMarker = config.providers.relay!.apiKey!; + const account = retainedMarker.slice("gcp-sa:".length); + expect(addProviderApiKey(config, "relay", JSON.stringify({ ...JSON.parse(credential), client_id: "second-client" }))).toHaveProperty("id"); + if (shared !== "none") config.providers.shared = { + adapter: "openai-chat", baseUrl: "https://shared.example/v1", apiKey: `${shared}:${account}`, + }; + saveConfig(config); + expect(secrets.size).toBe(2); + let deletions = 0; + setProviderKeychainEntryFactoryForTests((service, key) => ({ + getPassword: () => secrets.get(`${service}:${key}`) ?? null, + setPassword: value => { secrets.set(`${service}:${key}`, value); }, + deletePassword: () => { + expect(loadConfig().providers.relay!.apiKey).toBe("replacement-key"); + deletions += 1; + return secrets.delete(`${service}:${key}`); + }, + })); + spyOn(console, "log").mockImplementation(() => {}); + await handleProviderCommand(overwriteArgs); + expect(loadConfig().providers.relay!.apiKey).toBe("replacement-key"); + expect(deletions).toBe(shared === "none" ? 2 : 1); + expect(secrets.size).toBe(shared === "none" ? 0 : 1); + if (shared !== "none") expect(secrets.get(`opencodex.provider-api-key.v1:${account}`)).toBe(credential); + assertNoPlaintext(); +}); + +test.each(["save failure", "refused paste"])("CLI overwrite preserves credentials on %s", async failure => { + expect(addProviderApiKey(config, "relay", credential)).toHaveProperty("id"); + const before = readFileSync(join(home, "config.json"), "utf8"); + const previousExitCode = process.exitCode; + try { + if (failure === "save failure") { + spyOn(configModule, "saveConfig").mockImplementation(() => { throw new Error("synthetic save failure"); }); + await expect(handleProviderCommand(overwriteArgs)).rejects.toThrow("synthetic save failure"); + } else { + spyOn(console, "error").mockImplementation(() => {}); + await handleProviderCommand([...overwriteArgs.slice(0, -1), credential]); + expect(process.exitCode).toBe(2); + } + expect(readFileSync(join(home, "config.json"), "utf8")).toBe(before); + expect([...secrets.values()]).toEqual([credential]); + assertNoPlaintext(); + } finally { process.exitCode = previousExitCode ?? 0; } +}); + +test("keychain store/restore leaves GCP markers and their stored JSON intact in mixed pools", () => { + expect(addProviderApiKey(config, "relay", credential)).toHaveProperty("id"); + const marker = config.providers.relay!.apiKey!; + const account = marker.slice("gcp-sa:".length); + expect(providerKeyStoreKind(config.providers.relay)).toBe("keychain"); + expect(storeProviderKeyInKeychain(config, "relay")).toEqual({ ok: true, moved: 1 }); + expect(config.providers.relay!.apiKey).toBe(marker); + expect(secrets.get(`opencodex.provider-api-key.v1:${account}`)).toBe(credential); + expect(restoreProviderKeyFromKeychain(config, "relay")).toEqual({ ok: true, restored: 1 }); + expect(config.providers.relay!.apiKey).toBe(marker); + expect(config.providers.relay!.apiKeyPool!.some(row => row.key === marker)).toBe(true); + expect(secrets.get(`opencodex.provider-api-key.v1:${account}`)).toBe(credential); + assertNoPlaintext(); +}); + +test("cleanup retains the same OS account referenced by another provider through keychain syntax", () => { + const result = addProviderApiKey(config, "relay", credential); + if ("error" in result) throw new Error(result.error); + const account = config.providers.relay!.apiKey!.slice("gcp-sa:".length); + config.providers.shared = { adapter: "openai-chat", baseUrl: "https://shared.example/v1", apiKey: `keychain:${account}` }; + saveConfig(config); + expect(removeProviderApiKey(config, "relay", result.id)).toBe(true); + expect(secrets.get(`opencodex.provider-api-key.v1:${account}`)).toBe(credential); + expect(loadConfig().providers.shared!.apiKey).toBe(`keychain:${account}`); + assertNoPlaintext(); +}); + +test("restore never publishes stored credential JSON through a keychain alias", () => { + expect(addProviderApiKey(config, "relay", credential)).toHaveProperty("id"); + const marker = config.providers.relay!.apiKey!; + const account = marker.slice("gcp-sa:".length); + config.providers.relay!.apiKey = `keychain:${account}`; + saveConfig(config); + const before = structuredClone(config); + expect(restoreProviderKeyFromKeychain(config, "relay")).toMatchObject({ ok: false, status: 400 }); + expect(config).toEqual(before); + expect(secrets.get(`opencodex.provider-api-key.v1:${account}`)).toBe(credential); + assertNoPlaintext(); +}); + +test("credential re-add rebases and restores its payload after an overlapping last-reference removal", () => { + const original = addProviderApiKey(config, "relay", credential); + if ("error" in original) throw new Error(original.error); + const marker = config.providers.relay!.apiKey!; + const account = marker.slice("gcp-sa:".length); + let written = false; + let injectRemoval = true; + let removed = false; + setProviderKeychainEntryFactoryForTests((service, key) => ({ + getPassword: () => { + const value = secrets.get(`${service}:${key}`) ?? null; + if (key === account && written && injectRemoval) { + injectRemoval = false; + removed = removeProviderApiKey(loadConfig(), "relay", original.id); + } + return value; + }, + setPassword: value => { secrets.set(`${service}:${key}`, value); if (key === account) written = true; }, + deletePassword: () => secrets.delete(`${service}:${key}`), + })); + expect(addProviderApiKey(config, "relay", credential)).toEqual(original); + expect(removed).toBe(true); + expect(loadConfig().providers.relay!.apiKey).toBe(marker); + expect(secrets.get(`opencodex.provider-api-key.v1:${account}`)).toBe(credential); + assertNoPlaintext(); +}); + +test("a separate process cannot acquire the cleanup transaction during credential write/read-back", () => { + const original = addProviderApiKey(config, "relay", credential); + if ("error" in original) throw new Error(original.error); + const account = config.providers.relay!.apiKey!.slice("gcp-sa:".length); + let checked = false; + let blocked = false; + setProviderKeychainEntryFactoryForTests((service, key) => ({ + getPassword: () => secrets.get(`${service}:${key}`) ?? null, + setPassword: value => { + secrets.set(`${service}:${key}`, value); + if (key !== account || checked) return; + checked = true; + const child = Bun.spawnSync([process.execPath, "-e", ` + import { Database } from "bun:sqlite"; + const db = new Database(process.argv[1]); + let blocked = false; + try { db.exec("PRAGMA busy_timeout = 0; BEGIN IMMEDIATE"); db.exec("ROLLBACK"); } + catch (error) { blocked = error.code === "SQLITE_BUSY"; } + finally { db.close(); } + console.log(JSON.stringify({ blocked })); + `, join(home, "config-mutation.sqlite")], { stdout: "pipe", stderr: "pipe" }); + expect(child.exitCode).toBe(0); + blocked = JSON.parse(child.stdout.toString()).blocked; + }, + deletePassword: () => secrets.delete(`${service}:${key}`), + })); + expect(addProviderApiKey(config, "relay", credential)).toEqual(original); + expect(checked).toBe(true); + expect(blocked).toBe(true); + assertNoPlaintext(); +});