From be7cd6fd9e44ab46205a30259fd613a9bd88bc38 Mon Sep 17 00:00:00 2001 From: alexsong-oai Date: Fri, 23 Oct 2026 22:56:59 +0000 Subject: [PATCH] Add a credential file override for GCP provider authentication (#57188) ## What changed Support `model_providers.gcp.credential_file` as an absolute path to a Google Application Default Credentials (ADC) file. Pass it as `GOOGLE_APPLICATION_CREDENTIALS` in the provider auth command's environment, retaining the override for token refreshes without changing the parent process environment. Validate the path before merging GCP defaults and preserve the setting in thread configuration serialization. ## Testing Add coverage for invalid paths, configuration round trips, separate provider token caches, and credential overrides retained after a `401` refresh. Include a Windows regression test for credential paths containing command metacharacters. GitOrigin-RevId: d9a1d4b3775a9a4c4325dddaa153d92a2d1430a0 --- .../thread_processor_tests.rs | 1 + codex-rs/config/src/config_toml.rs | 22 ++++++ codex-rs/config/src/thread_config.rs | 1 + .../proto/codex.thread_config.v1.proto | 1 + .../proto/codex.thread_config.v1.rs | 2 + codex-rs/config/src/thread_config/remote.rs | 6 ++ codex-rs/core/config.schema.json | 4 ++ codex-rs/core/tests/responses_headers.rs | 3 + codex-rs/core/tests/suite/client.rs | 43 ++++++++++++ .../core/tests/suite/client_websockets.rs | 1 + codex-rs/core/tests/suite/gcp.rs | 63 +++++++++++++++++ .../suite/stream_error_allows_next_turn.rs | 1 + .../core/tests/suite/stream_no_completed.rs | 1 + codex-rs/login/src/auth/external_bearer.rs | 21 ++++-- codex-rs/login/src/auth/manager.rs | 13 +++- codex-rs/login/src/auth_env_telemetry.rs | 1 + codex-rs/model-provider-info/src/lib.rs | 29 +++++++- .../src/model_provider_info_tests.rs | 39 +++++++++++ codex-rs/model-provider/src/auth.rs | 14 +++- codex-rs/model-provider/src/gcp/gcp_tests.rs | 70 +++++++++++++++++++ codex-rs/model-provider/src/provider.rs | 1 + 21 files changed, 328 insertions(+), 9 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs index e4549bb1d3..9e510df839 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs @@ -658,6 +658,7 @@ mod thread_processor_behavior_tests { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; let config_manager = ConfigManager::new( diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index c3ac3e0b4b..98c02e6e5c 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -986,6 +986,11 @@ pub fn validate_model_providers( "model_providers.{key}: provider name must not be empty" )); } + if key == GCP_PROVIDER_ID { + provider + .validate_gcp_credential_file() + .map_err(|message| format!("model_providers.{key}: {message}"))?; + } provider .validate() .map_err(|message| format!("model_providers.{key}: {message}"))?; @@ -1159,4 +1164,21 @@ command = "print-token" (Some("https://vertex.example.com/v1"), Some("print-token")) ); } + + #[test] + fn gcp_provider_rejects_relative_credential_file() { + for credential_file in ["", "adc.json", "../adc.json"] { + let error = toml::from_str::(&format!( + r#" +[model_providers.gcp] +credential_file = "{credential_file}" +"#, + )) + .expect_err("GCP overrides must validate credential paths before merging defaults"); + + assert!(error.to_string().contains( + "model_providers.gcp: provider credential_file must be an absolute path" + )); + } + } } diff --git a/codex-rs/config/src/thread_config.rs b/codex-rs/config/src/thread_config.rs index 16a2e09ad6..a40e59436a 100644 --- a/codex-rs/config/src/thread_config.rs +++ b/codex-rs/config/src/thread_config.rs @@ -319,6 +319,7 @@ mod tests { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, } } diff --git a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto index efd43c4d20..0208d5ece3 100644 --- a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto +++ b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto @@ -52,6 +52,7 @@ message ModelProvider { optional string model_catalog_url = 19; optional string project_id = 20; optional string location = 21; + optional string credential_file = 22; } message StringMap { diff --git a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs index 2e5a3788e3..23fe7a280f 100644 --- a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs +++ b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs @@ -83,6 +83,8 @@ pub struct ModelProvider { pub project_id: ::core::option::Option<::prost::alloc::string::String>, #[prost(string, optional, tag = "21")] pub location: ::core::option::Option<::prost::alloc::string::String>, + #[prost(string, optional, tag = "22")] + pub credential_file: ::core::option::Option<::prost::alloc::string::String>, } #[derive(Clone, PartialEq, ::prost::Message)] pub struct StringMap { diff --git a/codex-rs/config/src/thread_config/remote.rs b/codex-rs/config/src/thread_config/remote.rs index b9d8127da6..85be589dc4 100644 --- a/codex-rs/config/src/thread_config/remote.rs +++ b/codex-rs/config/src/thread_config/remote.rs @@ -198,6 +198,7 @@ fn model_provider_from_proto( capabilities: None, project_id: provider.project_id, location: provider.location, + credential_file: provider.credential_file.map(Into::into), include_internal_metadata: false, }; Ok((id, info)) @@ -232,6 +233,7 @@ fn model_provider_to_proto( supports_websockets, supports_standalone_web_search, include_internal_metadata: _, + credential_file, capabilities: _, } = provider; @@ -240,6 +242,7 @@ fn model_provider_to_proto( name, base_url, model_catalog_url: model_catalog_url.map(RedactedString::into_inner), + credential_file: credential_file.map(|path| path.to_string_lossy().into_owned()), env_key, env_key_instructions, experimental_bearer_token: experimental_bearer_token.map(RedactedString::into_inner), @@ -454,6 +457,7 @@ mod tests { let mut expected = expected_provider(); expected.project_id = Some("vertex-project".to_string()); expected.location = Some("us-central1".to_string()); + expected.credential_file = Some(workspace_dir().join("adc.json").into_path_buf()); expected.auth = None; expected.experimental_bearer_token = Some("synthetic-provider-token".into()); let proto = model_provider_to_proto("local", expected.clone()); @@ -492,6 +496,7 @@ mod tests { model_catalog_url: Some( "http://127.0.0.1:8061/api/codex/models".to_string(), ), + credential_file: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, @@ -597,6 +602,7 @@ mod tests { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, } } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index bb6a528b0e..bbfed66c0d 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -3034,6 +3034,10 @@ ], "description": "Optional API capability overrides for a custom Responses-compatible provider. Unspecified capabilities retain their existing provider defaults." }, + "credential_file": { + "description": "Absolute path to a Google ADC credential file. For GCP, overrides GOOGLE_APPLICATION_CREDENTIALS in the auth command's environment only.", + "type": "string" + }, "env_http_headers": { "additionalProperties": { "type": "string" diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index 78215bf75d..ef0102393a 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -95,6 +95,7 @@ async fn responses_stream_includes_subagent_header_on_review() { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; @@ -244,6 +245,7 @@ async fn responses_stream_includes_subagent_header_on_other() { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; @@ -374,6 +376,7 @@ async fn responses_respects_model_info_overrides_from_config() { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 624d3b9b1b..2e192498cb 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -910,6 +910,7 @@ impl ProviderAuthCommandFixture { codex_utils_cargo_bin::write_executable( &script_path, r#"#!/bin/sh +printf '%s\n' "$GOOGLE_APPLICATION_CREDENTIALS" >> credential-env.txt if [ -f fail-until-401 ]; then exit 1 fi @@ -929,6 +930,9 @@ mv tokens.next tokens.txt &script_path, r#"@echo off setlocal EnableExtensions DisableDelayedExpansion +setlocal EnableDelayedExpansion +echo(!GOOGLE_APPLICATION_CREDENTIALS!>> credential-env.txt +endlocal if exist fail-until-401 exit /b 1 set "first_line=" @@ -971,6 +975,41 @@ move /y tokens.next tokens.txt >nul } } +#[cfg(windows)] +#[test] +fn provider_auth_command_logs_credential_paths_with_cmd_metacharacters() -> anyhow::Result<()> { + let auth_fixture = ProviderAuthCommandFixture::new(&["first-token", "second-token"])?; + let credential_file = auth_fixture + .tempdir + .path() + .join("ADC (team)&^!credentials!%PATH%.json"); + + for (credential_path, token) in [ + (Some(credential_file.as_os_str()), "first-token"), + (None, "second-token"), + ] { + let mut command = codex_utils_process::background_command(&auth_fixture.command); + command + .args(&auth_fixture.args) + .current_dir(auth_fixture.tempdir.path()); + if let Some(credential_path) = credential_path { + command.env("GOOGLE_APPLICATION_CREDENTIALS", credential_path); + } else { + command.env_remove("GOOGLE_APPLICATION_CREDENTIALS"); + } + let output = command.output()?; + assert!(output.status.success(), "{output:?}"); + assert_eq!(String::from_utf8(output.stdout)?, format!("{token}\r\n")); + assert_eq!(output.stderr, Vec::::new()); + } + + assert_eq!( + std::fs::read_to_string(auth_fixture.tempdir.path().join("credential-env.txt"))?, + format!("{}\r\n\r\n", credential_file.display()), + ); + Ok(()) +} + fn non_zero_u64(value: u64) -> NonZeroU64 { NonZeroU64::new(value).expect("expected non-zero value") } @@ -1746,6 +1785,7 @@ async fn send_provider_auth_request(server: &MockServer, auth: ModelProviderAuth capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; @@ -3254,6 +3294,7 @@ async fn azure_responses_request_does_not_store_and_preserves_prefixed_item_ids( capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; @@ -4104,6 +4145,7 @@ async fn azure_overrides_assign_properties_used_for_responses_url() { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; @@ -4194,6 +4236,7 @@ async fn env_var_overrides_loaded_auth() { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index 1486db4c25..c48ae76402 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -2738,6 +2738,7 @@ fn websocket_provider_with_connect_timeout( capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, } } diff --git a/codex-rs/core/tests/suite/gcp.rs b/codex-rs/core/tests/suite/gcp.rs index 098d59513f..bc99bb7614 100644 --- a/codex-rs/core/tests/suite/gcp.rs +++ b/codex-rs/core/tests/suite/gcp.rs @@ -28,9 +28,72 @@ use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; use test_case::test_case; +use wiremock::Mock; +use wiremock::ResponseTemplate; +use wiremock::matchers::header; +use wiremock::matchers::method; +use wiremock::matchers::path; use super::client::ProviderAuthCommandFixture; +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn vertex_credential_file_is_retained_when_refreshing_after_401() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let auth_fixture = ProviderAuthCommandFixture::new(&["first-token", "second-token"])?; + let auth = auth_fixture.auth(); + let credential_file = auth.cwd.join("adc.json").into_path_buf(); + let credential_log = auth.cwd.join("credential-env.txt"); + let inherited_credential_file = std::env::var_os("GOOGLE_APPLICATION_CREDENTIALS"); + + Mock::given(method("POST")) + .and(path("/v1/responses")) + .and(header("authorization", "Bearer first-token")) + .respond_with(ResponseTemplate::new(401).set_body_string("unauthorized")) + .expect(1) + .mount(&server) + .await; + let response = responses::mount_sse_once_match( + &server, + header("authorization", "Bearer second-token"), + responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_completed("resp-1"), + ]), + ) + .await; + let mut provider = ModelProviderInfo::create_gcp_provider(); + provider.base_url = Some(format!("{}/v1", server.uri())); + provider.auth = Some(auth); + provider.credential_file = Some(credential_file.clone()); + let test = test_codex() + .with_config(move |config| { + config.model_provider_id = GCP_PROVIDER_ID.to_string(); + config.model_provider = provider; + }) + .build_with_auto_env(&server) + .await?; + + test.submit_turn("say hello").await?; + + assert_eq!( + response.single_request().header("authorization").as_deref(), + Some("Bearer second-token"), + ); + let observed = std::fs::read_to_string(credential_log)?; + let expected = credential_file.to_string_lossy(); + assert_eq!( + observed.lines().collect::>(), + vec![expected.as_ref(); 2] + ); + assert_eq!( + std::env::var_os("GOOGLE_APPLICATION_CREDENTIALS"), + inherited_credential_file, + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn vertex_requests_use_command_auth_with_namespaces_and_tool_search() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs b/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs index 196e202523..033c291f6d 100644 --- a/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs +++ b/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs @@ -87,6 +87,7 @@ async fn continue_after_stream_error() { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; diff --git a/codex-rs/core/tests/suite/stream_no_completed.rs b/codex-rs/core/tests/suite/stream_no_completed.rs index 5386eb2364..2bcef8928f 100644 --- a/codex-rs/core/tests/suite/stream_no_completed.rs +++ b/codex-rs/core/tests/suite/stream_no_completed.rs @@ -73,6 +73,7 @@ async fn retries_on_early_close() { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; diff --git a/codex-rs/login/src/auth/external_bearer.rs b/codex-rs/login/src/auth/external_bearer.rs index b8ad0d0672..ed58f4ccdb 100644 --- a/codex-rs/login/src/auth/external_bearer.rs +++ b/codex-rs/login/src/auth/external_bearer.rs @@ -3,6 +3,8 @@ use super::manager::ExternalAuth; use super::manager::ExternalAuthFuture; use super::manager::ExternalAuthRefreshContext; use codex_protocol::config_types::ModelProviderAuthInfo; +use std::collections::HashMap; +use std::ffi::OsString; use std::fmt; use std::io; use std::ops::Deref; @@ -20,9 +22,9 @@ pub(crate) struct BearerTokenRefresher { } impl BearerTokenRefresher { - pub(crate) fn new(config: ModelProviderAuthInfo) -> Self { + pub(crate) fn new(config: ModelProviderAuthInfo, env: HashMap) -> Self { Self { - state: Arc::new(ExternalBearerAuthState::new(config)), + state: Arc::new(ExternalBearerAuthState::new(config, env)), } } @@ -43,7 +45,8 @@ impl BearerTokenRefresher { } } - let access_token = run_provider_auth_command(&self.state.config).await?; + let access_token = + run_provider_auth_command(&self.state.config, &self.state.env).await?; *cached = Some(CachedExternalBearerToken { access_token: access_token.clone(), fetched_at: Instant::now(), @@ -54,7 +57,7 @@ impl BearerTokenRefresher { } async fn refresh(&self, _context: ExternalAuthRefreshContext) -> io::Result { - let access_token = run_provider_auth_command(&self.state.config).await?; + let access_token = run_provider_auth_command(&self.state.config, &self.state.env).await?; let mut cached = self.state.cached_token.lock().await; *cached = Some(CachedExternalBearerToken { access_token: access_token.clone(), @@ -83,13 +86,15 @@ impl fmt::Debug for BearerTokenRefresher { struct ExternalBearerAuthState { config: ModelProviderAuthInfo, + env: HashMap, cached_token: Mutex>, } impl ExternalBearerAuthState { - fn new(config: ModelProviderAuthInfo) -> Self { + fn new(config: ModelProviderAuthInfo, env: HashMap) -> Self { Self { config, + env, cached_token: Mutex::new(None), } } @@ -100,11 +105,15 @@ struct CachedExternalBearerToken { fetched_at: Instant, } -async fn run_provider_auth_command(config: &ModelProviderAuthInfo) -> io::Result { +async fn run_provider_auth_command( + config: &ModelProviderAuthInfo, + env: &HashMap, +) -> io::Result { let program = resolve_provider_auth_program(&config.command, &config.cwd)?; let mut command = Command::from(codex_utils_process::background_command(&program)); command .args(config.args.iter().map(Deref::deref)) + .envs(env) .current_dir(config.cwd.as_path()) .stdin(Stdio::null()) .stdout(Stdio::piped()) diff --git a/codex-rs/login/src/auth/manager.rs b/codex-rs/login/src/auth/manager.rs index ec292e7fcc..7b8c7ffb5f 100644 --- a/codex-rs/login/src/auth/manager.rs +++ b/codex-rs/login/src/auth/manager.rs @@ -13,7 +13,9 @@ use http::StatusCode; use serde::Deserialize; #[cfg(test)] use serial_test::serial; +use std::collections::HashMap; use std::env; +use std::ffi::OsString; use std::fmt::Debug; use std::future::Future; use std::path::Path; @@ -2408,6 +2410,15 @@ impl AuthManager { } pub fn external_bearer_only(config: ModelProviderAuthInfo) -> Arc { + Self::external_bearer_only_with_env(config, HashMap::new()) + } + + /// Creates a provider-scoped token cache whose command environment overrides are + /// retained for both initial authentication and every subsequent refresh. + pub fn external_bearer_only_with_env( + config: ModelProviderAuthInfo, + env: HashMap, + ) -> Arc { let (auth_change_tx, _auth_change_rx) = watch::channel(0); Arc::new(Self { codex_home: PathBuf::from("non-existent"), @@ -2429,7 +2440,7 @@ impl AuthManager { refresh_lock: Arc::new(Semaphore::new(/*permits*/ 1)), agent_identity_lock: Semaphore::new(/*permits*/ 1), agent_identity_bootstrap_cooldown: Mutex::default(), - external_auth: RwLock::new(Some(Arc::new(BearerTokenRefresher::new(config)))), + external_auth: RwLock::new(Some(Arc::new(BearerTokenRefresher::new(config, env)))), workload_identity_selected: false, // External bearer auth refreshes by running the provider's command and never makes // auth-owned HTTP requests, so this route is intentionally inert. diff --git a/codex-rs/login/src/auth_env_telemetry.rs b/codex-rs/login/src/auth_env_telemetry.rs index 8f1c8ad8e1..fc40cf7fb6 100644 --- a/codex-rs/login/src/auth_env_telemetry.rs +++ b/codex-rs/login/src/auth_env_telemetry.rs @@ -82,6 +82,7 @@ mod tests { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; diff --git a/codex-rs/model-provider-info/src/lib.rs b/codex-rs/model-provider-info/src/lib.rs index 8fc8602dbc..3e7860ec0a 100644 --- a/codex-rs/model-provider-info/src/lib.rs +++ b/codex-rs/model-provider-info/src/lib.rs @@ -29,6 +29,7 @@ use std::fmt; use std::num::NonZeroU64; use std::path::Component; use std::path::Path; +use std::path::PathBuf; use std::sync::PoisonError; use std::sync::RwLock; use std::time::Duration; @@ -173,6 +174,9 @@ pub struct ModelProviderInfo { /// Google Cloud location for the built-in GCP provider. Takes precedence /// over `GOOGLE_CLOUD_LOCATION`; defaults to `global` when neither is set. pub location: Option, + /// Absolute path to a Google ADC credential file. For GCP, overrides + /// GOOGLE_APPLICATION_CREDENTIALS in the auth command's environment only. + pub credential_file: Option, /// Which wire protocol this provider expects. #[serde(default)] pub wire_api: WireApi, @@ -308,7 +312,20 @@ other non-default provider fields are not supported" Ok(()) } + /// Checks the credential-file path for a GCP provider or built-in GCP override. + pub fn validate_gcp_credential_file(&self) -> Result<(), String> { + if let Some(credential_file) = &self.credential_file + && !credential_file.is_absolute() + { + return Err("provider credential_file must be an absolute path".to_string()); + } + Ok(()) + } + pub fn validate(&self) -> std::result::Result<(), String> { + if self.is_gcp() { + self.validate_gcp_credential_file()?; + } if let Some(gateway) = &self.gateway_oauth { gateway.validate(self)?; } @@ -578,6 +595,7 @@ other non-default provider fields are not supported" capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: true, } } @@ -620,6 +638,7 @@ other non-default provider fields are not supported" capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, } } @@ -658,6 +677,7 @@ other non-default provider fields are not supported" supports_standalone_web_search: false, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, } } @@ -779,21 +799,27 @@ pub fn merge_configured_model_providers( } } } else if key == GCP_PROVIDER_ID { + provider + .validate_gcp_credential_file() + .and_then(|()| provider.validate()) + .map_err(|message| format!("model_providers.{GCP_PROVIDER_ID} {message}"))?; let base_url_override = provider.base_url.take(); let auth_override = provider.auth.take(); + let credential_file_override = provider.credential_file.take(); let http_headers_override = provider.http_headers.take(); let project_id_override = provider.project_id.take(); let location_override = provider.location.take(); if provider != ModelProviderInfo::default() { return Err(format!( "model_providers.{GCP_PROVIDER_ID} only supports changing \ -`base_url`, `auth`, `http_headers`, `project_id`, and `location`; other non-default provider fields are not supported" +`base_url`, `auth`, `http_headers`, `project_id`, `location`, and `credential_file`; other non-default provider fields are not supported" )); } if let Some(built_in_provider) = model_providers.get_mut(GCP_PROVIDER_ID) { built_in_provider.project_id = project_id_override; built_in_provider.location = location_override; + built_in_provider.credential_file = credential_file_override; if let Some(base_url_override) = base_url_override { built_in_provider.base_url = Some(base_url_override); } @@ -859,6 +885,7 @@ pub fn create_oss_provider_with_base_url(base_url: &str, wire_api: WireApi) -> M capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, } } diff --git a/codex-rs/model-provider-info/src/model_provider_info_tests.rs b/codex-rs/model-provider-info/src/model_provider_info_tests.rs index 6156d06513..ba3bd79290 100644 --- a/codex-rs/model-provider-info/src/model_provider_info_tests.rs +++ b/codex-rs/model-provider-info/src/model_provider_info_tests.rs @@ -99,6 +99,7 @@ base_url = "http://localhost:11434/v1" capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; @@ -140,6 +141,7 @@ query_params = { api-version = "2025-04-01-preview" } capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; @@ -185,6 +187,7 @@ supports_standalone_web_search = true capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, }; @@ -377,6 +380,7 @@ fn test_create_amazon_bedrock_provider() { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, } ); @@ -648,6 +652,7 @@ fn test_merge_configured_model_providers_applies_amazon_bedrock_transport_overri #[test] fn test_merge_configured_model_providers_applies_gcp_transport_overrides() { let auth = provider_auth_for_test(); + let credential_file = std::env::current_dir().unwrap().join("adc.json"); let configured_model_providers = std::collections::HashMap::from([( GCP_PROVIDER_ID.to_string(), ModelProviderInfo { @@ -655,6 +660,7 @@ fn test_merge_configured_model_providers_applies_gcp_transport_overrides() { project_id: Some("configured-project".to_string()), location: Some("us-east4".to_string()), auth: Some(auth.clone()), + credential_file: Some(credential_file.clone()), http_headers: Some(maplit::hashmap! { "x-example-header".to_string() => "value".into(), }), @@ -670,6 +676,7 @@ fn test_merge_configured_model_providers_applies_gcp_transport_overrides() { provider.project_id = Some("configured-project".to_string()); provider.location = Some("us-east4".to_string()); provider.auth = Some(auth); + provider.credential_file = Some(credential_file); provider.http_headers = Some(maplit::hashmap! { "x-example-header".to_string() => "value".into(), }); @@ -707,6 +714,38 @@ fn test_merge_configured_model_providers_applies_gcp_endpoint_override() { ); } +#[test] +fn test_merge_configured_gcp_rejects_relative_credential_file() { + for credential_file in ["", "adc.json", "../adc.json"] { + let provider = ModelProviderInfo { + credential_file: Some(credential_file.into()), + ..ModelProviderInfo::create_gcp_provider() + }; + assert_eq!( + provider.validate(), + Err("provider credential_file must be an absolute path".to_string()), + ); + + let overrides = HashMap::from([( + GCP_PROVIDER_ID.to_string(), + ModelProviderInfo { + credential_file: Some(credential_file.into()), + ..Default::default() + }, + )]); + + assert_eq!( + merge_configured_model_providers( + built_in_model_providers(/*openai_base_url*/ None), + overrides, + ), + Err( + "model_providers.gcp provider credential_file must be an absolute path".to_string() + ), + ); + } +} + #[test] fn test_merge_configured_model_providers_rejects_amazon_bedrock_non_default_fields() { let configured_model_providers = std::collections::HashMap::from([( diff --git a/codex-rs/model-provider/src/auth.rs b/codex-rs/model-provider/src/auth.rs index ba50f628bf..e542b9d70e 100644 --- a/codex-rs/model-provider/src/auth.rs +++ b/codex-rs/model-provider/src/auth.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::sync::Arc; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; @@ -240,7 +241,18 @@ pub(crate) fn auth_manager_for_provider( provider: &ModelProviderInfo, ) -> Option> { match provider.auth.clone() { - Some(config) => Some(AuthManager::external_bearer_only(config)), + Some(config) => { + let mut env = HashMap::new(); + if provider.is_gcp() + && let Some(credential_file) = &provider.credential_file + { + env.insert( + "GOOGLE_APPLICATION_CREDENTIALS".to_string(), + credential_file.as_os_str().to_os_string(), + ); + } + Some(AuthManager::external_bearer_only_with_env(config, env)) + } None => auth_manager, } } diff --git a/codex-rs/model-provider/src/gcp/gcp_tests.rs b/codex-rs/model-provider/src/gcp/gcp_tests.rs index fb3fbed094..57e1e3cc1c 100644 --- a/codex-rs/model-provider/src/gcp/gcp_tests.rs +++ b/codex-rs/model-provider/src/gcp/gcp_tests.rs @@ -164,6 +164,76 @@ async fn configured_base_url_bypasses_project_discovery() { ); } +#[tokio::test] +async fn credential_files_scope_token_caches_and_refreshes() { + let tempdir = tempfile::tempdir().unwrap(); + let first_file = tempdir.path().join("first credentials.json"); + let second_file = tempdir.path().join("second credentials.json"); + std::fs::write(&first_file, "first-token").unwrap(); + std::fs::write(&second_file, "second-token").unwrap(); + + #[cfg(unix)] + let (command, args) = ( + "sh".to_string(), + vec![ + "-c".into(), + "cat \"$GOOGLE_APPLICATION_CREDENTIALS\"".into(), + ], + ); + #[cfg(windows)] + let (command, args) = { + std::fs::write( + tempdir.path().join("read-token.cmd"), + "@type \"%GOOGLE_APPLICATION_CREDENTIALS%\"\r\n", + ) + .unwrap(); + ( + "cmd.exe".to_string(), + vec!["/d".into(), "/c".into(), r".\read-token.cmd".into()], + ) + }; + let mut first_info = ModelProviderInfo::create_gcp_provider(); + first_info.auth = Some(ModelProviderAuthInfo { + command, + args, + timeout_ms: std::num::NonZeroU64::new(5_000).unwrap(), + refresh_interval_ms: 300_000, + cwd: tempdir.path().try_into().unwrap(), + }); + first_info.credential_file = Some(first_file.clone()); + let mut second_info = first_info.clone(); + second_info.credential_file = Some(second_file); + let first = GcpModelProvider::new(first_info, /*auth_manager*/ None); + let second = GcpModelProvider::new(second_info, /*auth_manager*/ None); + + assert_eq!( + (first.auth().await, second.auth().await), + ( + Some(CodexAuth::from_api_key("first-token")), + Some(CodexAuth::from_api_key("second-token")), + ), + ); + std::fs::write(first_file, "refreshed-token").unwrap(); + assert_eq!( + first.auth().await, + Some(CodexAuth::from_api_key("first-token")) + ); + first + .auth_manager() + .unwrap() + .unauthorized_recovery() + .next() + .await + .unwrap(); + assert_eq!( + (first.auth().await, second.auth().await), + ( + Some(CodexAuth::from_api_key("refreshed-token")), + Some(CodexAuth::from_api_key("second-token")), + ), + ); +} + #[tokio::test] async fn bundled_vertex_models_filter_catalog_and_normalize_unsupported_tool_metadata() { let provider = GcpModelProvider::new( diff --git a/codex-rs/model-provider/src/provider.rs b/codex-rs/model-provider/src/provider.rs index d5cab7cc20..17250c2540 100644 --- a/codex-rs/model-provider/src/provider.rs +++ b/codex-rs/model-provider/src/provider.rs @@ -748,6 +748,7 @@ mod tests { capabilities: None, project_id: None, location: None, + credential_file: None, include_internal_metadata: false, } }