From ce14db1865f30c651e9caba8527d35855855f53e Mon Sep 17 00:00:00 2001 From: gitdevine <210788251+gitdevine@users.noreply.github.com> Date: Tue, 23 Jun 2026 21:29:59 +0200 Subject: [PATCH] fix(gemini-vertex): persist Vertex creds, sanitize project fallback, harden env test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 CodeRabbit findings on #1614: - provider-bootstrap.ts: carry the active credential into the saved profile. PROFILE_ENV_KEYS clears GEMINI_ACCESS_TOKEN / GOOGLE_APPLICATION_CREDENTIALS on relaunch, so an access-token profile must persist the bearer (and fail fast if absent) and an ADC profile must persist the credentials file — otherwise the saved profile relaunches unauthed (thread 3460713116). - providerProfiles.ts: sanitize each Vertex project candidate with trimOrUndefined() + ?? at the startup-profile site so a whitespace-only GEMINI_VERTEX_PROJECT no longer shadows a real GOOGLE_CLOUD_PROJECT / GCLOUD_PROJECT / GOOGLE_PROJECT_ID (thread 3460713146; the compatibility builder site was already fixed in 0501694). - security-hardening.test.ts: assert env classification via runtime isProviderManagedEnvVar() / SAFE_ENV_VARS.has() instead of slicing source text, so the policy test tracks behavior, not formatting (thread 3460713129). Validation: tsc --noEmit, type-tests, security-hardening.test.ts (13/13), knip — all green. providerProfiles getAPIProvider() timeouts are pre-existing (identical on the unmodified tree). Co-Authored-By: Claude Opus 4.8 --- scripts/provider-bootstrap.ts | 15 +++++++++++++++ src/__tests__/security-hardening.test.ts | 18 ++++++++---------- src/utils/providerProfiles.ts | 14 ++++++++------ 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/scripts/provider-bootstrap.ts b/scripts/provider-bootstrap.ts index 95a21ab92..c72deb62f 100644 --- a/scripts/provider-bootstrap.ts +++ b/scripts/provider-bootstrap.ts @@ -117,6 +117,21 @@ async function main(): Promise { authMode: vertexAuthMode, processEnv: process.env, }) + // PROFILE_ENV_KEYS clears GEMINI_ACCESS_TOKEN / GOOGLE_APPLICATION_CREDENTIALS + // on relaunch, so the saved profile must carry the active credential forward + // or it relaunches unauthed — mirrors the compatibility builder in + // providerProfile.ts / applyProviderProfileToProcessEnv. + if (vertexAuthMode === 'access-token') { + const accessToken = process.env.GEMINI_ACCESS_TOKEN?.trim() + if (!accessToken) { + console.error('Gemini Vertex access-token profiles require GEMINI_ACCESS_TOKEN.') + process.exit(1) + } + env.GEMINI_ACCESS_TOKEN = accessToken + } else { + const adcFile = process.env.GOOGLE_APPLICATION_CREDENTIALS?.trim() + if (adcFile) env.GOOGLE_APPLICATION_CREDENTIALS = adcFile + } } else if (selected === 'mistral') { const builtEnv = buildMistralProfileEnv({ model: argModel || null, diff --git a/src/__tests__/security-hardening.test.ts b/src/__tests__/security-hardening.test.ts index 71a941f8d..1e11c0b46 100644 --- a/src/__tests__/security-hardening.test.ts +++ b/src/__tests__/security-hardening.test.ts @@ -106,13 +106,11 @@ describe('SAFE_ENV_VARS excludes credentials', () => { }) test('Gemini Vertex routing env vars are managed; project ids stay unsafe', async () => { - const content = await file('utils/managedEnvConstants.ts').text() - const managedStart = content.indexOf('const PROVIDER_MANAGED_ENV_VARS') - const managedEnd = content.indexOf('])', managedStart) - const managedSection = content.slice(managedStart, managedEnd) - const safeStart = content.indexOf('export const SAFE_ENV_VARS') - const safeEnd = content.indexOf('])', safeStart) - const safeSection = content.slice(safeStart, safeEnd) + // Assert runtime classification behavior, not source formatting, so the + // test catches real regressions in env policy instead of breaking on edits. + const { isProviderManagedEnvVar, SAFE_ENV_VARS } = await import( + '../utils/managedEnvConstants.ts' + ) for (const envVar of [ 'CLAUDE_CODE_USE_GEMINI_VERTEX', @@ -124,7 +122,7 @@ describe('SAFE_ENV_VARS excludes credentials', () => { 'GCLOUD_PROJECT', 'GOOGLE_PROJECT_ID', ]) { - expect(managedSection).toContain(envVar) + expect(isProviderManagedEnvVar(envVar)).toBe(true) } for (const envVar of [ @@ -133,7 +131,7 @@ describe('SAFE_ENV_VARS excludes credentials', () => { 'GEMINI_VERTEX_MODEL', 'GEMINI_VERTEX_LOCATION', ]) { - expect(safeSection).toContain(envVar) + expect(SAFE_ENV_VARS.has(envVar)).toBe(true) } // Project ids from untrusted settings would switch Vertex traffic to an @@ -145,7 +143,7 @@ describe('SAFE_ENV_VARS excludes credentials', () => { 'GCLOUD_PROJECT', 'GOOGLE_PROJECT_ID', ]) { - expect(safeSection).not.toContain(envVar) + expect(SAFE_ENV_VARS.has(envVar)).toBe(false) } }) }) diff --git a/src/utils/providerProfiles.ts b/src/utils/providerProfiles.ts index a8c2a9eb8..e69df5d96 100644 --- a/src/utils/providerProfiles.ts +++ b/src/utils/providerProfiles.ts @@ -1392,13 +1392,15 @@ function buildStartupProfileFromActiveProfile( const env = buildGeminiVertexProfileEnv({ model: getPrimaryModel(activeProfile.model), // baseUrl carries the project id, but the preset can seed it with the - // endpoint URL — never let a URL through as a project. + // endpoint URL — never let a URL through as a project. Sanitize each + // candidate first so a whitespace-only GEMINI_VERTEX_PROJECT doesn't win + // over a real GOOGLE_CLOUD_PROJECT/GCLOUD_PROJECT/GOOGLE_PROJECT_ID. project: - geminiVertexProjectFromProfile(activeProfile.baseUrl) || - process.env.GEMINI_VERTEX_PROJECT || - process.env.GOOGLE_CLOUD_PROJECT || - process.env.GCLOUD_PROJECT || - process.env.GOOGLE_PROJECT_ID, + geminiVertexProjectFromProfile(activeProfile.baseUrl) ?? + trimOrUndefined(process.env.GEMINI_VERTEX_PROJECT) ?? + trimOrUndefined(process.env.GOOGLE_CLOUD_PROJECT) ?? + trimOrUndefined(process.env.GCLOUD_PROJECT) ?? + trimOrUndefined(process.env.GOOGLE_PROJECT_ID), authMode: vertexAuthMode, processEnv: process.env, })