fix(gemini-vertex): address CodeRabbit review — drift, malformed-history and credential hardening

Verified each CodeRabbit finding against current code; applied the valid ones,
skipped two with documented reasons.

Provider-flag drift / registry adoption:
- auth.ts: isAnthropicAuthEnabled's is3P gate now derives from
  hasAnyTruthyProviderSelectionFlag() instead of a hand-maintained 8-flag list.
- routeMetadata.ts: reuse the shared hasConflictingProviderFlag() helper instead
  of an inline PROVIDER_SELECTION_FLAGS scan.
- withRetry/promptCacheBreakDetection tests: derive env scrub lists from
  PROVIDER_SELECTION_FLAGS so new flags are covered automatically.
- bugfixes drift guard updated to assert the registry derivation rather than the
  now-removed literal in auth.ts.

Credential / managed-env correctness:
- managedEnvConstants.ts: treat GOOGLE_APPLICATION_CREDENTIALS as provider-managed
  so host-managed sessions can't have ADC creds swapped via settings.
- providerStartupOverrides.ts: clear the Google ADC/project aliases
  (GOOGLE_APPLICATION_CREDENTIALS, GOOGLE_CLOUD_PROJECT, GCLOUD_PROJECT,
  GOOGLE_PROJECT_ID) so a cleared Vertex override can't stay partially active.
- agentRouting.ts: clear the Vertex-specific selectors before forcing an
  OpenAI-compatible override, mirroring the existing GEMINI_MODEL/BASE_URL clears.
- providerProfile.ts: treat GEMINI_VERTEX_AUTH_MODE as a concrete Vertex signal in
  hasConcreteProviderSelection (mirrors hasCompleteProviderSelection).
- client.ts: reuse resolveGeminiVertexAuthMode() instead of an inline auth-mode
  computation (trims whitespace tokens, validates the mode).
- system-check.ts: use getGeminiVertexLocation() so doctor output matches runtime.
- openaiShim.ts: pick the first non-blank credential so a whitespace-only
  OPENAI_API_KEY no longer suppresses a real GITHUB_TOKEN/GH_TOKEN (+regression).

Malformed-history hardening:
- messages.ts: validate the first content block before reading .type/.text;
  sanitize non-string tool_result.tool_use_id to mirror the assistant id path.
- normalizeMessages malformed test: cover falsy slots and unknown message types.
- providerConfig.envDiagnostics test: clear CLAUDE_CODE_USE_GITHUB in Vertex tests.

Skipped (documented):
- log.ts saved-profile gate: importing isGeminiVertexEffectiveProvider would create
  a log -> providerProfiles -> config -> log cycle (config.ts imports log.ts).
- providerProfiles.ts hasCompleteProviderSelection access-token gate: CodeRabbit
  asked to require a project, which contradicts the maintainer's explicit P2
  request to treat a bare access-token Vertex selection as complete.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gitdevine
2026-06-26 13:05:08 +02:00
co-authored by Claude Opus 4.8
parent 5e975fff48
commit ac896eede2
16 changed files with 133 additions and 56 deletions
+2 -1
View File
@@ -27,6 +27,7 @@ import {
type SecretValueSource,
} from '../src/utils/providerSecrets.js'
import {
getGeminiVertexLocation,
getGeminiVertexProjectId,
resolveGeminiCredential,
resolveGeminiVertexAuthMode,
@@ -345,7 +346,7 @@ function hasPlaceholderCredential(value: string | undefined): boolean {
function currentBaseUrl(): string {
if (isTruthy(process.env.CLAUDE_CODE_USE_GEMINI_VERTEX)) {
const location = process.env.GEMINI_VERTEX_LOCATION ?? 'global'
const location = getGeminiVertexLocation(process.env)
const project =
getGeminiVertexProjectId(process.env) ?? '<set GEMINI_VERTEX_PROJECT>'
return `https://aiplatform.googleapis.com/v1/projects/${project}/locations/${location}`
+12 -1
View File
@@ -584,6 +584,12 @@ describe('Gemini Vertex provider drift guards', () => {
})
test('auth treats Gemini Vertex as a third-party provider', async () => {
// isAnthropicAuthEnabled's is3P gate now derives from the
// PROVIDER_SELECTION_FLAGS registry instead of hand-enumerating
// CLAUDE_CODE_USE_* flags, so the guarantee is two-fold: (1) the gate routes
// through the registry helper, and (2) the Vertex flag is registered in that
// single source of truth. Asserting the literal in the function body would
// regress to the exact drift the registry refactor eliminated.
const content = await file('utils/auth.ts').text()
const is3PSection = sectionBetween(
content,
@@ -591,7 +597,12 @@ describe('Gemini Vertex provider drift guards', () => {
'// Check if user has configured',
)
expect(is3PSection).toContain('CLAUDE_CODE_USE_GEMINI_VERTEX')
expect(is3PSection).toContain('hasAnyTruthyProviderSelectionFlag')
const { PROVIDER_SELECTION_FLAGS } = await import(
'../utils/providerSelectionFlags.js'
)
expect(PROVIDER_SELECTION_FLAGS).toContain('CLAUDE_CODE_USE_GEMINI_VERTEX')
})
test('isUsing3PServices() includes Gemini Vertex flag', async () => {
+5 -8
View File
@@ -14,8 +14,10 @@ import {
} from './index.js'
import { hasUsableOpenAICredential } from '../services/api/credentialPool.js'
import { isEnvTruthy } from '../utils/envUtils.js'
import { hasAnyTruthyProviderSelectionFlag } from '../utils/providerSelectionFlags.js'
import { PROVIDER_SELECTION_FLAGS } from '../utils/providerSelectionFlags.js'
import {
hasAnyTruthyProviderSelectionFlag,
hasConflictingProviderFlag,
} from '../utils/providerSelectionFlags.js'
export type RouteDescriptor = GatewayDescriptor | VendorDescriptor
@@ -816,14 +818,9 @@ export function resolveActiveRouteIdFromEnv(
// Mirror shouldRouteToGeminiVertexFromProfile exactly — any *defined* (not
// just truthy) provider flag other than the Vertex one is explicit intent, so
// CLAUDE_CODE_USE_OPENAI=0 must block the saved-profile route here too.
const hasConflictingProviderFlag = PROVIDER_SELECTION_FLAGS.some(
flag =>
flag !== 'CLAUDE_CODE_USE_GEMINI_VERTEX' &&
processEnv[flag] !== undefined,
)
if (
options?.activeProfileProvider === 'gemini-vertex' &&
!hasConflictingProviderFlag
!hasConflictingProviderFlag(processEnv, 'CLAUDE_CODE_USE_GEMINI_VERTEX')
) {
return 'gemini-vertex'
}
+9
View File
@@ -44,6 +44,15 @@ const PROVIDER_ENV_VARS_TO_CLEAR_FOR_OVERRIDE = [
'ANTHROPIC_BASE_URL',
'GEMINI_MODEL',
'GEMINI_BASE_URL',
'GEMINI_ACCESS_TOKEN',
'GEMINI_VERTEX_AUTH_MODE',
'GEMINI_VERTEX_MODEL',
'GEMINI_VERTEX_PROJECT',
'GEMINI_VERTEX_LOCATION',
'GOOGLE_APPLICATION_CREDENTIALS',
'GOOGLE_CLOUD_PROJECT',
'GCLOUD_PROJECT',
'GOOGLE_PROJECT_ID',
'MISTRAL_MODEL',
'MISTRAL_BASE_URL',
'OPENAI_API_BASE',
+2 -3
View File
@@ -54,6 +54,7 @@ import {
getGeminiVertexModel,
getGeminiVertexProjectId,
resolveGeminiCredential,
resolveGeminiVertexAuthMode,
} from '../../utils/geminiAuth.js'
import {
shouldUseFirstPartyAnthropicAuth,
@@ -576,9 +577,7 @@ export async function getAnthropicClient({
const resolveVertexCredential = () =>
resolveGeminiCredential({
...process.env,
GEMINI_AUTH_MODE:
process.env.GEMINI_VERTEX_AUTH_MODE ??
(process.env.GEMINI_ACCESS_TOKEN ? 'access-token' : 'adc'),
GEMINI_AUTH_MODE: resolveGeminiVertexAuthMode(process.env),
GEMINI_API_KEY: undefined,
GOOGLE_API_KEY: undefined,
} as NodeJS.ProcessEnv)
+39
View File
@@ -700,6 +700,45 @@ test('GitHub mode falls back to GITHUB_TOKEN when OPENAI_API_KEY is blank', asyn
expect(capturedHeaders?.get('authorization')).toBe('Bearer gho_test_token')
})
test('GitHub mode falls back to GH_TOKEN when OPENAI_API_KEY is whitespace-only', async () => {
process.env.CLAUDE_CODE_USE_GITHUB = '1'
// Whitespace-only (not just empty) OPENAI_API_KEY is truthy under `||`, so it
// would suppress a real token and emit a blank Authorization header unless the
// fallback chain trims candidates.
process.env.OPENAI_API_KEY = ' '
process.env.GH_TOKEN = 'gho_ws_token'
delete process.env.GITHUB_COPILOT_KEY
delete process.env.GITHUB_TOKEN
let capturedHeaders: Headers | undefined
globalThis.fetch = (async (_input, init) => {
capturedHeaders = new Headers(init?.headers as HeadersInit)
return new Response(
JSON.stringify({
id: 'chatcmpl-1',
choices: [{ message: { role: 'assistant', content: 'ok' } }],
}),
{
headers: {
'Content-Type': 'application/json',
},
},
)
}) as unknown as FetchType
const client = createOpenAIShimClient({ defaultHeaders: {} }) as OpenAIShimClient
await client.beta.messages.create({
model: 'gpt-4o',
messages: [{ role: 'user', content: 'hello' }],
max_tokens: 64,
stream: false,
})
expect(capturedHeaders?.get('authorization')).toBe('Bearer gho_ws_token')
})
test('uses Hicap api-key auth header for the Hicap route', async () => {
process.env.OPENAI_API_KEY = 'hicap-live-key'
process.env.OPENAI_BASE_URL = 'https://api.hicap.ai/v1'
+24 -10
View File
@@ -491,6 +491,19 @@ function isGeminiMode(): boolean {
)
}
/**
* First candidate whose trimmed value is non-empty, else ''. Used for
* credential fallback chains where a whitespace-only env value must not
* suppress a real token further down the chain (`||` alone treats ' ' as
* truthy and would leave the request unauthenticated).
*/
function firstNonBlankCredential(...values: (string | undefined)[]): string {
for (const value of values) {
if (value && value.trim().length > 0) return value
}
return ''
}
function hydrateOpenAIShimCompatibilityEnv(
processEnv: NodeJS.ProcessEnv = process.env,
): void {
@@ -514,16 +527,17 @@ function hydrateOpenAIShimCompatibilityEnv(
}
if (isEnvTruthy(processEnv.CLAUDE_CODE_USE_GITHUB)) {
// Use || (not ??) so a blank exported OPENAI_API_KEY does not win over a
// real GITHUB_TOKEN/GH_TOKEN and leave GitHub requests unauthenticated —
// the shim auth path reads OPENAI_API_KEY ?? '', so an empty string here
// means no Authorization header. Matches the Gemini/Mistral branches.
processEnv.OPENAI_API_KEY =
processEnv.GITHUB_COPILOT_KEY ||
processEnv.OPENAI_API_KEY ||
processEnv.GITHUB_TOKEN ||
processEnv.GH_TOKEN ||
''
// Pick the first non-blank candidate so a blank or whitespace-only exported
// OPENAI_API_KEY does not win over a real GITHUB_TOKEN/GH_TOKEN and leave
// GitHub requests unauthenticated — the shim auth path reads
// OPENAI_API_KEY ?? '', so a blank-like value here means no Authorization
// header. Matches the Gemini/Mistral branches.
processEnv.OPENAI_API_KEY = firstNonBlankCredential(
processEnv.GITHUB_COPILOT_KEY,
processEnv.OPENAI_API_KEY,
processEnv.GITHUB_TOKEN,
processEnv.GH_TOKEN,
)
return
}
@@ -8,6 +8,7 @@ import {
acquireSharedMutationLock,
releaseSharedMutationLock,
} from '../../test/sharedMutationLock.js'
import { PROVIDER_SELECTION_FLAGS } from '../../utils/providerSelectionFlags.js'
type PromptCacheBreakModule = typeof import('./promptCacheBreakDetection.js')
type EventCall = {
@@ -47,14 +48,7 @@ mock.module('../../utils/debug.js', () => ({
}))
const PROVIDER_ENV_KEYS = [
'CLAUDE_CODE_USE_OPENAI',
'CLAUDE_CODE_USE_GITHUB',
'CLAUDE_CODE_USE_GEMINI',
'CLAUDE_CODE_USE_GEMINI_VERTEX',
'CLAUDE_CODE_USE_MISTRAL',
'CLAUDE_CODE_USE_BEDROCK',
'CLAUDE_CODE_USE_VERTEX',
'CLAUDE_CODE_USE_FOUNDRY',
...PROVIDER_SELECTION_FLAGS,
'OPENAI_BASE_URL',
'OPENAI_API_BASE',
'OPENAI_MODEL',
@@ -108,6 +108,7 @@ test('does not warn for OPENAI_API_BASE when OPENAI_BASE_URL is active', async (
process.env.CLAUDE_CODE_USE_OPENAI = '1'
delete process.env.CLAUDE_CODE_USE_MISTRAL
delete process.env.CLAUDE_CODE_USE_GITHUB
process.env.OPENAI_BASE_URL = 'http://127.0.0.1:11434/v1'
process.env.OPENAI_MODEL = 'qwen2.5-coder:7b'
process.env.OPENAI_API_BASE = 'undefined'
@@ -150,6 +151,7 @@ test('uses descriptor-backed Gemini default model when GEMINI_MODEL is unset', a
delete process.env.CLAUDE_CODE_USE_OPENAI
delete process.env.CLAUDE_CODE_USE_MISTRAL
delete process.env.CLAUDE_CODE_USE_GITHUB
process.env.CLAUDE_CODE_USE_GEMINI = '1'
delete process.env.GEMINI_MODEL
delete process.env.GEMINI_BASE_URL
@@ -171,6 +173,7 @@ test('uses GEMINI_VERTEX_MODEL instead of OPENAI_MODEL in Gemini Vertex mode', a
delete process.env.CLAUDE_CODE_USE_OPENAI
delete process.env.CLAUDE_CODE_USE_GEMINI
delete process.env.CLAUDE_CODE_USE_MISTRAL
delete process.env.CLAUDE_CODE_USE_GITHUB
process.env.CLAUDE_CODE_USE_GEMINI_VERTEX = '1'
process.env.OPENAI_MODEL = 'gpt-5.5'
process.env.GEMINI_VERTEX_MODEL = 'gemini-vertex-custom'
@@ -191,6 +194,7 @@ test('uses current Gemini Vertex fallback model instead of OPENAI_MODEL when ver
delete process.env.CLAUDE_CODE_USE_OPENAI
delete process.env.CLAUDE_CODE_USE_GEMINI
delete process.env.CLAUDE_CODE_USE_MISTRAL
delete process.env.CLAUDE_CODE_USE_GITHUB
process.env.CLAUDE_CODE_USE_GEMINI_VERTEX = '1'
process.env.OPENAI_MODEL = 'gpt-5.5'
delete process.env.GEMINI_VERTEX_MODEL
@@ -211,6 +215,7 @@ test('exposes Gemini Vertex project and location from dedicated env vars', async
delete process.env.CLAUDE_CODE_USE_OPENAI
delete process.env.CLAUDE_CODE_USE_GEMINI
delete process.env.CLAUDE_CODE_USE_MISTRAL
delete process.env.CLAUDE_CODE_USE_GITHUB
process.env.CLAUDE_CODE_USE_GEMINI_VERTEX = '1'
process.env.GEMINI_VERTEX_PROJECT = 'vertex-project'
process.env.GEMINI_VERTEX_LOCATION = 'europe-west4'
@@ -231,6 +236,7 @@ test('falls back to Google project env vars for Gemini Vertex project', async ()
delete process.env.CLAUDE_CODE_USE_OPENAI
delete process.env.CLAUDE_CODE_USE_GEMINI
delete process.env.CLAUDE_CODE_USE_MISTRAL
delete process.env.CLAUDE_CODE_USE_GITHUB
process.env.CLAUDE_CODE_USE_GEMINI_VERTEX = '1'
process.env.GOOGLE_CLOUD_PROJECT = 'project-test-1234'
process.env.GEMINI_VERTEX_LOCATION = 'us-central1'
@@ -254,6 +260,7 @@ test('defaults Gemini Vertex location to global when unset', async () => {
delete process.env.CLAUDE_CODE_USE_OPENAI
delete process.env.CLAUDE_CODE_USE_GEMINI
delete process.env.CLAUDE_CODE_USE_MISTRAL
delete process.env.CLAUDE_CODE_USE_GITHUB
process.env.CLAUDE_CODE_USE_GEMINI_VERTEX = '1'
process.env.GOOGLE_CLOUD_PROJECT = 'project-test-1234'
delete process.env.GEMINI_VERTEX_PROJECT
+2 -7
View File
@@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test'
import type Anthropic from '@anthropic-ai/sdk'
import { APIError } from '@anthropic-ai/sdk'
import { acquireSharedMutationLock, releaseSharedMutationLock } from '../../test/sharedMutationLock.js'
import { PROVIDER_SELECTION_FLAGS } from '../../utils/providerSelectionFlags.js'
type ProvidersModule = typeof import('../../utils/model/providers.js')
// Helper to build a mock APIError with specific headers
@@ -20,13 +21,7 @@ const originalEnv = { ...process.env }
let originalProvidersModule: ProvidersModule | undefined
const envKeys = [
'CLAUDE_CODE_USE_OPENAI',
'CLAUDE_CODE_USE_GEMINI',
'CLAUDE_CODE_USE_GEMINI_VERTEX',
'CLAUDE_CODE_USE_GITHUB',
'CLAUDE_CODE_USE_BEDROCK',
'CLAUDE_CODE_USE_VERTEX',
'CLAUDE_CODE_USE_FOUNDRY',
...PROVIDER_SELECTION_FLAGS,
'CLAUDE_CODE_UNATTENDED_RETRY',
'CLAUDE_CODE_MAX_RETRIES',
'OPENCLAUDE_MAX_RETRIES',
+1 -9
View File
@@ -113,15 +113,7 @@ export function isAnthropicAuthEnabled(): boolean {
return !!process.env.CLAUDE_CODE_OAUTH_TOKEN
}
const is3P =
isEnvTruthy(process.env.CLAUDE_CODE_USE_BEDROCK) ||
isEnvTruthy(process.env.CLAUDE_CODE_USE_VERTEX) ||
isEnvTruthy(process.env.CLAUDE_CODE_USE_GEMINI_VERTEX) ||
isEnvTruthy(process.env.CLAUDE_CODE_USE_FOUNDRY) ||
isEnvTruthy(process.env.CLAUDE_CODE_USE_OPENAI) ||
isEnvTruthy(process.env.CLAUDE_CODE_USE_GEMINI) ||
isEnvTruthy(process.env.CLAUDE_CODE_USE_MISTRAL) ||
isEnvTruthy(process.env.CLAUDE_CODE_USE_GITHUB)
const is3P = hasAnyTruthyProviderSelectionFlag()
// Check if user has configured an external API key source
// This allows externally-provided API keys to work (without requiring proxy configuration)
+1
View File
@@ -41,6 +41,7 @@ const PROVIDER_MANAGED_ENV_VARS = new Set([
'AWS_BEARER_TOKEN_BEDROCK',
'ANTHROPIC_FOUNDRY_API_KEY',
'GEMINI_VERTEX_AUTH_MODE',
'GOOGLE_APPLICATION_CREDENTIALS',
'CLAUDE_CODE_SKIP_BEDROCK_AUTH',
'CLAUDE_CODE_SKIP_VERTEX_AUTH',
'CLAUDE_CODE_SKIP_FOUNDRY_AUTH',
@@ -5,6 +5,11 @@ import type { Message } from '../types/message.js'
describe('normalizeMessages malformed-payload hardening', () => {
test('drops malformed assistant/user slots instead of throwing', () => {
const messages = [
// Falsy top-level slots must be dropped, not throw.
null,
undefined,
// Unknown message type must be dropped.
{ type: 'totally-bogus', message: { content: 'x' } },
{ type: 'user', message: {} },
{ type: 'assistant' },
{ type: 'user', message: { content: null } },
+16 -8
View File
@@ -745,14 +745,21 @@ export function isNotEmptyMessage(message: Message): boolean {
return true
}
if (message.message.content[0]!.type !== 'text') {
const firstBlock = message.message.content[0]
if (!firstBlock || typeof firstBlock !== 'object') {
return false
}
if (firstBlock.type !== 'text') {
return true
}
if (typeof firstBlock.text !== 'string') {
return false
}
return (
message.message.content[0]!.text.trim().length > 0 &&
message.message.content[0]!.text !== NO_CONTENT_MESSAGE &&
message.message.content[0]!.text !== INTERRUPT_MESSAGE_FOR_TOOL_USE
firstBlock.text.trim().length > 0 &&
firstBlock.text !== NO_CONTENT_MESSAGE &&
firstBlock.text !== INTERRUPT_MESSAGE_FOR_TOOL_USE
)
}
@@ -2270,10 +2277,11 @@ function sanitizeUserMessageToolUseIds(message: UserMessage): UserMessage {
if (!Array.isArray(content)) return message
let changed = false
const next = content.map(block => {
if (
block.type === 'tool_result' &&
typeof block.tool_use_id === 'string'
) {
// Mirror the assistant tool_use.id path (which sanitizes any present id via
// the unknown-safe helper): a non-string persisted tool_use_id would
// otherwise stay un-sanitized while its matching tool_use.id is rewritten,
// breaking call/result pairing on the wire.
if (block.type === 'tool_result' && block.tool_use_id != null) {
const sanitized = sanitizeToolUseIdForWire(block.tool_use_id)
if (sanitized !== block.tool_use_id) {
changed = true
+2 -1
View File
@@ -1322,7 +1322,8 @@ function hasConcreteProviderSelection(
sanitizeProviderConfigValue(processEnv.GEMINI_VERTEX_MODEL),
) !== undefined ||
sanitizeApiKey(processEnv.GEMINI_ACCESS_TOKEN) !== undefined ||
sanitizeProviderConfigValue(processEnv.GOOGLE_APPLICATION_CREDENTIALS) !== undefined
sanitizeProviderConfigValue(processEnv.GOOGLE_APPLICATION_CREDENTIALS) !== undefined ||
sanitizeProviderConfigValue(processEnv.GEMINI_VERTEX_AUTH_MODE) !== undefined
)
}
+4
View File
@@ -26,6 +26,10 @@ export const STARTUP_PROVIDER_OVERRIDE_ENV_KEYS = [
'GEMINI_VERTEX_MODEL',
'GEMINI_VERTEX_PROJECT',
'GEMINI_VERTEX_LOCATION',
'GOOGLE_APPLICATION_CREDENTIALS',
'GOOGLE_CLOUD_PROJECT',
'GCLOUD_PROJECT',
'GOOGLE_PROJECT_ID',
'MISTRAL_BASE_URL',
'MISTRAL_MODEL',
'MISTRAL_API_KEY',