diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst index 5e3d6b67ce..708b89be3b 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst differ diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 8d5d2be81d..1afa3c6b42 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1338,6 +1338,13 @@ client_request_definitions! { response: v2::BedrockSetupResponse, }, + #[experimental("account/gcp/setup")] + GcpSetup => "account/gcp/setup" { + params: v2::GcpSetupParams, + serialization: global("account-auth"), + response: v2::GcpSetupResponse, + }, + #[experimental("account/bedrock/checkGovCloudRequirements")] BedrockCheckGovCloudRequirements => "account/bedrock/checkGovCloudRequirements" { params: v2::BedrockCheckGovCloudRequirementsParams, diff --git a/codex-rs/app-server-protocol/src/protocol/v2/gcp.rs b/codex-rs/app-server-protocol/src/protocol/v2/gcp.rs new file mode 100644 index 0000000000..97b18ef66c --- /dev/null +++ b/codex-rs/app-server-protocol/src/protocol/v2/gcp.rs @@ -0,0 +1,27 @@ +//! Shared setup parameters for the built-in Google Cloud Vertex AI provider. + +use crate::JsonSchema; +use crate::TS; +use serde::Deserialize; +use serde::Serialize; + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct GcpSetupParams { + pub project_id: String, + /// Vertex AI location. Omission or null selects `global`. + #[ts(optional = nullable)] + pub location: Option, + /// Absolute path to a credential file on the app-server host. + /// Omit or pass `null` to clear the active user configuration's override and use ADC. + /// If another configuration layer would still supply a credential file, setup fails + /// without saving changes. + #[ts(optional = nullable)] + pub credential_file: Option, +} + +#[derive(Serialize, Deserialize, Debug, Default, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct GcpSetupResponse {} diff --git a/codex-rs/app-server-protocol/src/protocol/v2/mod.rs b/codex-rs/app-server-protocol/src/protocol/v2/mod.rs index f2f157fe7f..37b6f5ad20 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/mod.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/mod.rs @@ -16,6 +16,7 @@ mod environment; mod experimental_feature; mod feedback; mod fs; +mod gcp; mod hook; mod item; mod mcp; @@ -57,6 +58,7 @@ pub use environment::*; pub use experimental_feature::*; pub use feedback::*; pub use fs::*; +pub use gcp::*; pub use hook::*; pub use item::*; pub use mcp::*; diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 93533fce60..26a385cec5 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -646,3 +646,36 @@ and out-of-scope item IDs return invalid params (`-32602`) with Omitted or null cursors preserve normal first-page behavior. Continue anchored pages with the returned opaque string `nextCursor`; response fields and `backwardsCursor` semantics are unchanged. + +# GCP provider account + +The experimental `account/gcp/setup` method configures the built-in Vertex AI provider. + +- `projectId` (required) — Google Cloud project ID. +- `location` (optional) — Defaults to `global`. +- `credentialFile` (optional) — Absolute path to a credential file on the app-server + host. Omit it or pass `null` to clear the saved path in the active user + configuration and use Application Default Credentials (ADC), including + `GOOGLE_APPLICATION_CREDENTIALS`. + +If the active profile inherits `model_providers.gcp.credential_file` from the base +`config.toml`, clearing the profile’s own setting would expose the base path again. +Setup returns an error without saving changes. To use ADC, remove the inherited +setting from the file named in the error and retry. + +Setup rejects a GCP `base_url` override or higher-priority settings that conflict +with the request. It fetches an access token before saving the settings and +selecting GCP, then returns `{}`. The next `model/list` reloads the catalog. +`account/read` reports `{ "account": { "type": "gcp" }, "requiresOpenaiAuth": false }`. +Use a model request to verify Vertex model access. + +Credential failures include `error.data.reason`. + +- `gcloudNotInstalled` — The app-server could not find `gcloud` on its PATH. +- `credentialFileInvalid` — The path is invalid or the file is unreadable. For + default `gcloud` auth, setup also checks for a JSON object with a nonempty string `type`. +- `authCommandFailed` — The token command failed, including when ADC is missing + or a token refresh fails. + +For personal sign-in, users can run `gcloud auth application-default login` on +the app-server host and retry setup. diff --git a/codex-rs/app-server/src/config_manager_service.rs b/codex-rs/app-server/src/config_manager_service.rs index df807ba4b5..ebb3b5efc5 100644 --- a/codex-rs/app-server/src/config_manager_service.rs +++ b/codex-rs/app-server/src/config_manager_service.rs @@ -3,6 +3,7 @@ use crate::config_layer::config_layer_to_api; use crate::config_manager::ConfigManager; use codex_app_server_protocol::Config as ApiConfig; use codex_app_server_protocol::ConfigBatchWriteParams; +use codex_app_server_protocol::ConfigEdit as ApiConfigEdit; use codex_app_server_protocol::ConfigReadParams; use codex_app_server_protocol::ConfigReadResponse; use codex_app_server_protocol::ConfigValueWriteParams; @@ -31,6 +32,7 @@ use codex_core::path_utils; use codex_core::path_utils::SymlinkWritePaths; use codex_core::path_utils::resolve_symlink_write_paths; use codex_core::path_utils::write_atomically; +use codex_model_provider_info::ModelProviderInfo; use codex_protocol::protocol::AskForApproval; use codex_utils_absolute_path::AbsolutePathBuf; use serde_json::Value as JsonValue; @@ -44,6 +46,8 @@ use toml_edit::Item as TomlItem; #[path = "config_manager_service_credential_edits.rs"] mod credential_edits; +#[path = "config_manager_service_provider_setup.rs"] +mod provider_setup; use credential_edits::CredentialProviderEdits; @@ -116,6 +120,11 @@ impl ConfigManagerError { } } +pub(crate) struct PreparedProviderWrite { + edits: Vec, + expected_version: String, +} + impl ConfigManager { pub(crate) async fn read( &self, @@ -202,6 +211,63 @@ impl ConfigManager { .await } + pub(crate) fn prepare_provider_config<'a>( + &self, + layers: &ConfigLayerStack, + provider_id: &'a str, + settings: impl IntoIterator)>, + ) -> Result<(PreparedProviderWrite, ModelProviderInfo), ConfigManagerError> { + let selection = match layers.required_model_provider() { + Some(required) if required == provider_id => None, + Some(required) => { + return Err(ConfigManagerError::write( + ConfigWriteErrorCode::ConfigRequirementReadonly, + format!( + "Cannot select `{provider_id}` because requirements set `model_provider` to `{required}`." + ), + )); + } + None => Some(("model_provider".to_owned(), Some(provider_id))), + }; + let settings = settings + .into_iter() + .map(|(key, value)| (format!("model_providers.{provider_id}.{key}"), value)); + let edits = selection + .into_iter() + .chain(settings) + .map(|(key_path, value)| ApiConfigEdit { + key_path, + value: value.into(), + merge_strategy: MergeStrategy::Replace, + }) + .collect(); + let user_path = self + .user_config_path() + .map_err(|err| ConfigManagerError::io("failed to resolve user config path", err))?; + provider_setup::preview(layers, &user_path, provider_id, edits) + } + + pub(crate) async fn commit_provider_config( + &self, + write: PreparedProviderWrite, + ) -> Result<(), ConfigManagerError> { + let response = self + .batch_write(ConfigBatchWriteParams { + edits: write.edits, + file_path: None, + expected_version: Some(write.expected_version), + reload_user_config: false, + }) + .await?; + if let Some(overridden) = response.overridden_metadata { + return Err(ConfigManagerError::write( + ConfigWriteErrorCode::ConfigLayerReadonly, + overridden.message, + )); + } + Ok(()) + } + pub(crate) async fn batch_write( &self, params: ConfigBatchWriteParams, @@ -262,18 +328,7 @@ impl ConfigManager { let mut config_edits = Vec::new(); for (key_path, value, strategy) in edits.into_iter() { - let mut segments = parse_key_path(&key_path).map_err(|message| { - ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, message) - })?; - if let Some(field) = layers - .requirements_toml() - .exact_requirement_for_config_path(&segments) - { - return Err(ConfigManagerError::write( - ConfigWriteErrorCode::ConfigRequirementReadonly, - format!("`{field}` is managed by requirements and cannot be changed"), - )); - } + let mut segments = writable_key_path(&layers, &key_path)?; if (value.is_null() || matches!(strategy, MergeStrategy::Upsert)) && let Some(pattern) = shell_environment_filter_entry(&user_config, &segments) .map(|(pattern, _)| pattern.clone()) @@ -318,9 +373,7 @@ impl ConfigManager { let parsed_value = if preserves_network_proxy_settings { Some(TomlValue::Boolean(false)) } else { - parse_value(value).map_err(|message| { - ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, message) - })? + parse_value(value)? }; if matches!(strategy, MergeStrategy::Upsert) && let Some(value) = parsed_value.as_ref() @@ -526,14 +579,38 @@ async fn write_empty_user_config(write_path: PathBuf) -> Result<(), ConfigManage .map_err(|err| ConfigManagerError::io("failed to create empty user config.toml", err)) } -fn parse_value(value: JsonValue) -> Result, String> { +fn parse_value(value: JsonValue) -> Result, ConfigManagerError> { if value.is_null() { return Ok(None); } serde_json::from_value::(value) .map(Some) - .map_err(|err| format!("invalid value: {err}")) + .map_err(|err| { + ConfigManagerError::write( + ConfigWriteErrorCode::ConfigValidationError, + format!("invalid value: {err}"), + ) + }) +} + +fn writable_key_path( + layers: &ConfigLayerStack, + key_path: &str, +) -> Result, ConfigManagerError> { + let segments = parse_key_path(key_path).map_err(|message| { + ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, message) + })?; + if let Some(field) = layers + .requirements_toml() + .exact_requirement_for_config_path(&segments) + { + return Err(ConfigManagerError::write( + ConfigWriteErrorCode::ConfigRequirementReadonly, + format!("`{field}` is managed by requirements and cannot be changed"), + )); + } + Ok(segments) } fn parse_key_path(path: &str) -> Result, String> { diff --git a/codex-rs/app-server/src/config_manager_service_provider_setup.rs b/codex-rs/app-server/src/config_manager_service_provider_setup.rs new file mode 100644 index 0000000000..d66114663f --- /dev/null +++ b/codex-rs/app-server/src/config_manager_service_provider_setup.rs @@ -0,0 +1,87 @@ +//! Previews the effective provider configuration and captures the user layer version for commit. + +use super::ConfigManagerError; +use super::MergeError; +use super::PreparedProviderWrite; +use super::apply_merge; +use super::find_effective_layer; +use super::override_message; +use super::parse_value; +use super::value_at_path; +use super::writable_key_path; +use codex_app_server_protocol::ConfigEdit; +use codex_app_server_protocol::ConfigWriteErrorCode; +use codex_config::ConfigLayerStack; +use codex_config::version_for_toml; +use codex_model_provider_info::ModelProviderInfo; +use codex_utils_absolute_path::AbsolutePathBuf; +use toml::Value as TomlValue; + +pub(super) fn preview( + layers: &ConfigLayerStack, + user_path: &AbsolutePathBuf, + provider_id: &str, + edits: Vec, +) -> Result<(PreparedProviderWrite, ModelProviderInfo), ConfigManagerError> { + let (mut user_config, expected_version) = match layers.get_active_user_layer() { + Some(layer) => (layer.config.clone(), layer.version.clone()), + None => { + let empty = TomlValue::Table(Default::default()); + let version = version_for_toml(&empty); + (empty, version) + } + }; + let mut fields = Vec::with_capacity(edits.len()); + for edit in &edits { + let path = writable_key_path(layers, &edit.key_path)?; + let value = parse_value(edit.value.clone())?; + apply_merge( + &mut user_config, + &path, + value.as_ref(), + edit.merge_strategy.clone(), + ) + .map_err(|MergeError::Validation(message)| { + ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, message) + })?; + fields.push((&edit.key_path, path, value)); + } + + let preview = layers + .with_user_config(user_path, user_config) + .map_err(|err| { + ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, err.to_string()) + })?; + let effective = preview.effective_config(); + for (key, path, expected) in fields { + if value_at_path(&effective, &path) != expected.as_ref() { + let source = find_effective_layer(&preview, &path) + .map(|layer| override_message(&layer.name)) + .unwrap_or_else(|| "Overridden by another configuration layer".to_owned()); + return Err(ConfigManagerError::write( + ConfigWriteErrorCode::ConfigLayerReadonly, + format!("Cannot configure `{key}`: {source}"), + )); + } + } + let provider = effective + .get("model_providers") + .and_then(|providers| providers.get(provider_id)) + .cloned() + .map(TomlValue::try_into) + .transpose() + .map_err(|_| { + ConfigManagerError::write( + ConfigWriteErrorCode::ConfigValidationError, + format!("Invalid `{provider_id}` provider configuration."), + ) + })? + .unwrap_or_default(); + Ok(( + PreparedProviderWrite { + edits, + expected_version, + }, + provider, + )) +} diff --git a/codex-rs/app-server/src/config_manager_service_tests.rs b/codex-rs/app-server/src/config_manager_service_tests.rs index dad7956ee1..2948434c95 100644 --- a/codex-rs/app-server/src/config_manager_service_tests.rs +++ b/codex-rs/app-server/src/config_manager_service_tests.rs @@ -1187,6 +1187,29 @@ async fn write_value_reports_override() { assert!(result.overridden_metadata.is_none()); } +#[tokio::test] +async fn provider_setup_rejects_config_changed_after_preview() -> Result<()> { + let tmp = tempdir()?; + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); + let layers = service.load_config_layers(/*cwd*/ None).await?; + let (write, _) = service.prepare_provider_config( + &layers, + codex_model_provider_info::AMAZON_BEDROCK_PROVIDER_ID, + [("aws.region", Some("us-west-2"))], + )?; + + let user_path = tmp.path().join(CONFIG_TOML_FILE); + let newer_config = "model = 'newer'"; + std::fs::write(&user_path, newer_config)?; + let error = service.commit_provider_config(write).await.unwrap_err(); + assert_eq!( + error.write_error_code(), + Some(ConfigWriteErrorCode::ConfigVersionConflict) + ); + assert_eq!(std::fs::read_to_string(user_path)?, newer_config); + Ok(()) +} + #[tokio::test] async fn version_conflict_rejected() { let tmp = tempdir().expect("tempdir"); diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index a1bb6d1901..c664fac4b2 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -50,6 +50,7 @@ use crate::request_processors::ThreadRequestProcessor; use crate::request_processors::ThreadResumeTarget; use crate::request_processors::TurnRequestProcessor; use crate::request_processors::WindowsSandboxRequestProcessor; +use crate::request_processors::account_processor::handle_gcp_setup; use crate::request_processors::read_server_diagnostics; use crate::request_serialization::QueuedInitializedRequest; use crate::request_serialization::RequestSerializationQueueKey; @@ -1806,6 +1807,9 @@ impl MessageProcessor { ClientRequest::BedrockSetup { params, .. } => { self.account_processor.bedrock_setup(params).await } + ClientRequest::GcpSetup { params, .. } => { + handle_gcp_setup(&self.account_processor, params).await + } ClientRequest::BedrockCheckGovCloudRequirements { .. } => { self.account_processor .bedrock_check_gov_cloud_requirements() diff --git a/codex-rs/app-server/src/model_catalog.rs b/codex-rs/app-server/src/model_catalog.rs index ff9704fa30..8413c3d530 100644 --- a/codex-rs/app-server/src/model_catalog.rs +++ b/codex-rs/app-server/src/model_catalog.rs @@ -3,12 +3,14 @@ use std::path::PathBuf; use std::sync::Arc; +use codex_config::ConfigLayerStack; use codex_core::config::Config; use codex_features::Feature; use codex_login::AuthManager; use codex_models_manager::manager::RefreshStrategy; use codex_models_manager::manager::SharedModelsManager; use codex_protocol::openai_models::ModelPreset; +use codex_utils_absolute_path::AbsolutePathBuf; use tokio::sync::Mutex; use crate::config_manager::ConfigManager; @@ -48,6 +50,16 @@ impl ModelCatalog { } } + /// Loads the same startup context used to reload the catalog after account setup. + pub(crate) async fn load_startup_config_layers(&self) -> std::io::Result { + let cwd = self + .startup_cwd + .as_deref() + .map(AbsolutePathBuf::from_absolute_path) + .transpose()?; + self.config_manager.load_config_layers(cwd).await + } + /// Marks catalog configuration stale after an account transition. pub(crate) async fn invalidate(&self) { self.state.lock().await.needs_reload = true; diff --git a/codex-rs/app-server/src/request_processors/account_processor.rs b/codex-rs/app-server/src/request_processors/account_processor.rs index 43946dc1ed..2aaa283f78 100644 --- a/codex-rs/app-server/src/request_processors/account_processor.rs +++ b/codex-rs/app-server/src/request_processors/account_processor.rs @@ -1,7 +1,7 @@ use super::bedrock_auth::BedrockProviderConfig; use super::bedrock_auth::clear_user_model_provider_if_bedrock; -use super::bedrock_auth::configure_bedrock_provider; -use super::bedrock_auth::ensure_user_model_provider_can_be_bedrock; +use super::bedrock_auth::prepare_bedrock_provider; +use super::config_processor::map_error as map_config_error; use super::*; use crate::auth_mode::auth_mode_to_api; use crate::external_auth::ExternalAuthBridge; @@ -21,11 +21,13 @@ mod bedrock_gov_cloud; mod bedrock_setup; mod enterprise_login; mod gateway_oauth; +mod gcp_setup; mod rate_limit_resets; mod workspace_routing; pub(super) use enterprise_login::EnterpriseLoginCompletion; pub(super) use enterprise_login::EnterpriseLoginTarget; +pub(crate) use gcp_setup::handle_gcp_setup; // Duration before a browser ChatGPT login attempt is abandoned. const LOGIN_CHATGPT_TIMEOUT: Duration = Duration::from_secs(10 * 60); @@ -80,6 +82,11 @@ enum RefreshTokenRequestOutcome { FailedPermanently, } +enum CloudLoginProvider { + AmazonBedrock, + Gcp, +} + enum BedrockLoginCredentials { ApiKey(String), AccessKeys { @@ -426,7 +433,10 @@ impl AccountRequestProcessor { ) } - fn ensure_bedrock_login_allowed(&self) -> Result<(), JSONRPCErrorError> { + fn ensure_provider_login_allowed( + &self, + provider: CloudLoginProvider, + ) -> Result<(), JSONRPCErrorError> { if self.auth_manager.is_workload_identity_selected() { return Err(self.configured_auth_owned_by_host_error()); } @@ -437,9 +447,13 @@ impl AccountRequestProcessor { .auth_manager .is_login_method_allowed(ForcedLoginMethod::Api) { - return Err(invalid_request( - "Amazon Bedrock login is disabled. Use ChatGPT login instead.", - )); + let operation = match provider { + CloudLoginProvider::AmazonBedrock => "Amazon Bedrock login", + CloudLoginProvider::Gcp => "Google Cloud setup", + }; + return Err(invalid_request(format!( + "{operation} is disabled. Use ChatGPT login instead." + ))); } Ok(()) } @@ -508,7 +522,7 @@ impl AccountRequestProcessor { region: String, ) { let result = async { - self.ensure_bedrock_login_allowed()?; + self.ensure_provider_login_allowed(CloudLoginProvider::AmazonBedrock)?; match &credentials { BedrockLoginCredentials::ApiKey(api_key) => { @@ -536,8 +550,7 @@ impl AccountRequestProcessor { } self.cancel_active_login().await; - ensure_user_model_provider_can_be_bedrock(&self.config_manager).await?; - configure_bedrock_provider( + let write = prepare_bedrock_provider( &self.config_manager, BedrockProviderConfig { region: matches!(&credentials, BedrockLoginCredentials::AccessKeys { .. }) @@ -546,6 +559,10 @@ impl AccountRequestProcessor { }, ) .await?; + self.config_manager + .commit_provider_config(write) + .await + .map_err(map_config_error)?; match credentials { BedrockLoginCredentials::ApiKey(api_key) => login_with_bedrock_api_key( diff --git a/codex-rs/app-server/src/request_processors/account_processor/bedrock_setup.rs b/codex-rs/app-server/src/request_processors/account_processor/bedrock_setup.rs index a3fe8f9108..aa609e73dd 100644 --- a/codex-rs/app-server/src/request_processors/account_processor/bedrock_setup.rs +++ b/codex-rs/app-server/src/request_processors/account_processor/bedrock_setup.rs @@ -1,7 +1,8 @@ use super::super::bedrock_auth::BedrockProviderConfig; -use super::super::bedrock_auth::configure_bedrock_provider; -use super::super::bedrock_auth::ensure_user_model_provider_can_be_bedrock; +use super::super::bedrock_auth::prepare_bedrock_provider; +use super::super::config_processor::map_error as map_config_error; use super::AccountRequestProcessor; +use super::CloudLoginProvider; use crate::error_code::internal_error; use crate::error_code::invalid_request; use codex_app_server_protocol::AwsCredentialType; @@ -25,7 +26,7 @@ impl AccountRequestProcessor { &self, _params: BedrockDiscoverParams, ) -> Result, JSONRPCErrorError> { - self.ensure_bedrock_login_allowed()?; + self.ensure_provider_login_allowed(CloudLoginProvider::AmazonBedrock)?; let profiles = codex_aws_auth::discover_aws_profiles() .await .map_err(|err| internal_error(format!("failed to discover AWS profiles: {err}")))? @@ -69,9 +70,22 @@ impl AccountRequestProcessor { &self, params: BedrockSetupParams, ) -> Result, JSONRPCErrorError> { - self.ensure_bedrock_login_allowed()?; - ensure_user_model_provider_can_be_bedrock(&self.config_manager).await?; - if matches!(¶ms, BedrockSetupParams::Environment { .. }) + self.ensure_provider_login_allowed(CloudLoginProvider::AmazonBedrock)?; + let (profile, region) = match ¶ms { + BedrockSetupParams::Profile { profile, region } => { + (Some(profile.trim()), region.trim()) + } + BedrockSetupParams::Environment { region } => (None, region.trim()), + }; + let write = prepare_bedrock_provider( + &self.config_manager, + BedrockProviderConfig { + region: Some(region), + profile, + }, + ) + .await?; + if profile.is_none() && matches!( self.auth_manager.auth_cached(), Some(CodexAuth::BedrockApiKey(_) | CodexAuth::BedrockAccessKeys(_)) @@ -82,58 +96,44 @@ impl AccountRequestProcessor { )); } - let region = match ¶ms { - BedrockSetupParams::Profile { region, .. } - | BedrockSetupParams::Environment { region, .. } => region.trim(), - }; if !is_supported_amazon_bedrock_region(region) { return Err(invalid_request(format!( "Amazon Bedrock does not support region `{region}`" ))); } - let profile = match ¶ms { - BedrockSetupParams::Profile { profile, .. } => { - let profile = profile.trim(); - if profile.is_empty() { - return Err(invalid_request("AWS profile name must not be empty.")); - } - codex_aws_auth::validate_aws_profile( - profile, - region, - self.config.http_client_factory(), - ) - .await - .map_err(|err| { - invalid_request(format!( - "failed to load credentials for AWS profile `{profile}`: {err}" - )) - })?; - Some(profile.to_string()) + if let Some(profile) = profile { + if profile.is_empty() { + return Err(invalid_request("AWS profile name must not be empty.")); } - BedrockSetupParams::Environment { .. } => { - let has_bedrock_api_key = non_empty_env_var(AWS_BEARER_TOKEN_BEDROCK).is_some(); - let has_access_keys = non_empty_env_var(AWS_ACCESS_KEY_ID).is_some() - && non_empty_env_var(AWS_SECRET_ACCESS_KEY).is_some(); - if !has_bedrock_api_key && !has_access_keys { - return Err(invalid_request( - "No AWS credentials found. Please Configure AWS credentials or complete AWS sign-in, then try again.", - )); - } - None + codex_aws_auth::validate_aws_profile( + profile, + region, + self.config.http_client_factory(), + ) + .await + .map_err(|err| { + invalid_request(format!( + "failed to load credentials for AWS profile `{profile}`: {err}" + )) + })?; + } else { + let has_bedrock_api_key = non_empty_env_var(AWS_BEARER_TOKEN_BEDROCK).is_some(); + let has_access_keys = non_empty_env_var(AWS_ACCESS_KEY_ID).is_some() + && non_empty_env_var(AWS_SECRET_ACCESS_KEY).is_some(); + if !has_bedrock_api_key && !has_access_keys { + return Err(invalid_request( + "No AWS credentials found. Please Configure AWS credentials or complete AWS sign-in, then try again.", + )); } - }; + } // TODO: Validate the selected credentials against Bedrock ListModels once supported. self.cancel_active_login().await; - configure_bedrock_provider( - &self.config_manager, - BedrockProviderConfig { - region: Some(region), - profile: profile.as_deref(), - }, - ) - .await?; + self.config_manager + .commit_provider_config(write) + .await + .map_err(map_config_error)?; Ok(Some(BedrockSetupResponse {}.into())) } diff --git a/codex-rs/app-server/src/request_processors/account_processor/gcp_setup.rs b/codex-rs/app-server/src/request_processors/account_processor/gcp_setup.rs new file mode 100644 index 0000000000..c7989eb5a1 --- /dev/null +++ b/codex-rs/app-server/src/request_processors/account_processor/gcp_setup.rs @@ -0,0 +1,207 @@ +//! Validates and persists GCP setup without changing saved login credentials or existing threads. + +use super::super::config_processor::map_error as map_config_error; +use super::AccountRequestProcessor; +use super::CloudLoginProvider; +use crate::error_code::internal_error; +use crate::error_code::invalid_request; +use codex_app_server_protocol::ClientResponsePayload; +use codex_app_server_protocol::GcpSetupParams; +use codex_app_server_protocol::GcpSetupResponse; +use codex_app_server_protocol::JSONRPCErrorError; +use codex_config::apply_default_gcp_auth; +use codex_login::RefreshTokenError; +use codex_model_provider::create_model_provider; +use codex_model_provider_info::GCP_PROVIDER_ID; +use codex_model_provider_info::ModelProviderInfo; +use codex_model_provider_info::merge_gcp_provider_overrides; +use codex_utils_absolute_path::AbsolutePathBuf; +use serde::Deserialize; +use serde::Deserializer; +use serde::Serialize; +use serde::de::MapAccess; +use serde::de::Visitor; +use serde::de::value::MapAccessDeserializer; +use serde_json::json; +use std::fmt; +use std::io::BufReader; +use std::io::ErrorKind; +use std::path::Path; +use std::path::PathBuf; +use tokio::fs::File; + +pub(crate) async fn handle_gcp_setup( + processor: &AccountRequestProcessor, + params: GcpSetupParams, +) -> Result, JSONRPCErrorError> { + processor.ensure_provider_login_allowed(CloudLoginProvider::Gcp)?; + let network_policy = processor + .auth_manager + .application_network_policy() + .for_current_account(); + let layers = processor + .model_catalog + .load_startup_config_layers() + .await + .map_err(|err| internal_error(format!("failed to load configuration layers: {err}")))?; + let (write, provider) = processor + .config_manager + .prepare_provider_config( + &layers, + GCP_PROVIDER_ID, + [ + ("project_id", Some(params.project_id.trim())), + ( + "location", + Some(params.location.as_deref().unwrap_or("global").trim()), + ), + ("credential_file", params.credential_file.as_deref()), + ], + ) + .map_err(map_config_error)?; + if provider.base_url.is_some() { + return Err(invalid_request( + "Google Cloud has a custom `base_url`. Clear `model_providers.gcp.base_url` before configuring a project and location.", + )); + } + // Even checking a Windows UNC credential path can contact the remote host. + let permit = network_policy + .acquire_for_unsupported_sdk() + .map_err(|err| invalid_request(err.to_string()))?; + permit + .run(verify_credentials(provider, &processor.config.cwd)) + .await + .map_err(|err| invalid_request(err.to_string()))??; + + processor + .config_manager + .commit_provider_config(write) + .await + .map_err(map_config_error)?; + processor.cancel_active_login().await; + processor.model_catalog.invalidate().await; + Ok(Some(GcpSetupResponse {}.into())) +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +enum AuthErrorReason { + GcloudNotInstalled, + CredentialFileInvalid, + AuthCommandFailed, +} + +impl AuthErrorReason { + fn error(self, message: impl Into) -> JSONRPCErrorError { + let mut error = invalid_request(message); + error.data = Some(json!({ "reason": self })); + error + } +} + +async fn open_credential_file(path: &Path) -> std::io::Result { + if !tokio::fs::metadata(path).await?.is_file() { + return Err(std::io::Error::new( + ErrorKind::InvalidInput, + "expected a regular file", + )); + } + File::open(path).await +} + +// Require a JSON object. Serde also accepts positional arrays when deserializing a struct. +struct AdcObject; + +impl<'de> Visitor<'de> for AdcObject { + type Value = (); + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a Google Cloud credential object") + } + + fn visit_map>(self, map: A) -> Result<(), A::Error> { + #[derive(Deserialize)] + struct Adc { + #[serde(rename = "type")] + kind: String, + } + let adc = Adc::deserialize(MapAccessDeserializer::new(map))?; + if adc.kind.trim().is_empty() { + return Err(serde::de::Error::custom( + "expected a nonempty credential type", + )); + } + Ok(()) + } +} + +async fn validate_adc_file(file: File) -> std::io::Result<()> { + let file = file.into_std().await; + tokio::task::spawn_blocking(move || { + // Stream past type-specific fields without retaining the credential contents. + let mut json = serde_json::Deserializer::from_reader(BufReader::new(file)); + json.deserialize_map(AdcObject)?; + json.end()?; + Ok(()) + }) + .await? +} + +async fn verify_credentials( + configured: ModelProviderInfo, + cwd: &AbsolutePathBuf, +) -> Result<(), JSONRPCErrorError> { + configured + .validate_gcp_credential_file() + .map_err(|err| AuthErrorReason::CredentialFileInvalid.error(err))?; + let default_gcloud = configured.auth.is_none(); + let mut candidate = ModelProviderInfo::create_gcp_provider(); + merge_gcp_provider_overrides(Some(&mut candidate), configured).map_err(invalid_request)?; + apply_default_gcp_auth(&mut candidate, cwd); + candidate.validate().map_err(invalid_request)?; + let auth_file = candidate.credential_file.clone().or_else(|| { + if default_gcloud { + std::env::var_os("GOOGLE_APPLICATION_CREDENTIALS").map(PathBuf::from) + } else { + None + } + }); + if let Some(path) = auth_file { + let path = cwd.as_path().join(path); + let file = open_credential_file(&path).await.map_err(|err| { + AuthErrorReason::CredentialFileInvalid.error(format!( + "Cannot read the Google Cloud credential file: {err}" + )) + })?; + if default_gcloud { + validate_adc_file(file).await.map_err(|_| AuthErrorReason::CredentialFileInvalid.error( + "Expected a Google Cloud credential file containing a JSON object with a nonempty string type.", + ))?; + } + } + let provider = create_model_provider(candidate, /*auth_manager*/ None); + provider + .runtime_base_url() + .await + .map_err(|err| invalid_request(err.to_string()))?; + // This fresh provider owns a command-backed manager. A refresh runs that command + // once and preserves errors that the inference auth path reduces to missing auth. + let auth_manager = provider + .auth_manager() + .ok_or_else(|| internal_error("Missing GCP auth manager."))?; + if let Err(err) = auth_manager.refresh_token().await { + if default_gcloud + && let RefreshTokenError::Transient(err) = err + && err.kind() == ErrorKind::NotFound + && cwd.as_path().is_dir() + { + return Err(AuthErrorReason::GcloudNotInstalled.error( + "Cannot find gcloud on the app-server host. Check its installation and PATH.", + )); + } + return Err(AuthErrorReason::AuthCommandFailed + .error("The Google Cloud auth command could not obtain an access token.")); + } + + Ok(()) +} diff --git a/codex-rs/app-server/src/request_processors/bedrock_auth.rs b/codex-rs/app-server/src/request_processors/bedrock_auth.rs index 9aba8f9c52..ce2183e718 100644 --- a/codex-rs/app-server/src/request_processors/bedrock_auth.rs +++ b/codex-rs/app-server/src/request_processors/bedrock_auth.rs @@ -1,5 +1,6 @@ use super::config_processor::map_error as map_config_error; use crate::config_manager::ConfigManager; +use crate::config_manager_service::PreparedProviderWrite; use crate::error_code::internal_error; use crate::error_code::invalid_request; use codex_app_server_protocol::ConfigBatchWriteParams; @@ -7,9 +8,6 @@ use codex_app_server_protocol::ConfigEdit; use codex_app_server_protocol::ConfigWriteErrorCode; use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::MergeStrategy; -use codex_config::CONFIG_TOML_FILE; -use codex_config::ConfigLayerSource; -use codex_config::format_config_layer_source; use codex_core::config::Config; use codex_core::config::edit::ConfigEditsBuilder; use codex_model_provider::AMAZON_BEDROCK_PROVIDER_ID; @@ -19,100 +17,30 @@ pub(super) struct BedrockProviderConfig<'a> { pub(super) profile: Option<&'a str>, } -pub(super) async fn configure_bedrock_provider( +pub(super) async fn prepare_bedrock_provider( config_manager: &ConfigManager, config: BedrockProviderConfig<'_>, -) -> Result<(), JSONRPCErrorError> { - let mut edits = vec![ - ( - "model_provider", - serde_json::json!(AMAZON_BEDROCK_PROVIDER_ID), - ), - ( - "model_providers.amazon-bedrock.aws.profile", - serde_json::json!(config.profile), - ), - ]; - if let Some(region) = config.region { - edits.push(( - "model_providers.amazon-bedrock.aws.region", - serde_json::json!(region), - )); - } - - let response = config_manager - .batch_write(ConfigBatchWriteParams { - edits: edits - .into_iter() - .map(|(key_path, value)| ConfigEdit { - key_path: key_path.to_string(), - value, - merge_strategy: MergeStrategy::Replace, - }) - .collect(), - file_path: None, - expected_version: None, - reload_user_config: false, - }) - .await - .map_err(map_config_error)?; - if let Some(overridden) = response.overridden_metadata { - let message = overridden.message; - return Err(invalid_request(format!( - "Amazon Bedrock configuration cannot take effect: {message}" - ))); - } - Ok(()) -} - -pub(super) async fn ensure_user_model_provider_can_be_bedrock( - config_manager: &ConfigManager, -) -> Result<(), JSONRPCErrorError> { +) -> Result { let layers = config_manager .load_config_layers(/*cwd*/ None) .await .map_err(|err| internal_error(format!("failed to load configuration layers: {err}")))?; - let user_precedence = match layers.get_active_user_layer() { - Some(layer) => layer.name.precedence(), - None => ConfigLayerSource::User { - file: config_manager.user_config_path().map_err(|err| { - internal_error(format!("failed to resolve user config path: {err}")) - })?, - profile: None, - } - .precedence(), - }; - if let Some((overriding_layer, effective_provider)) = layers - .layers_high_to_low() - .filter(|layer| layer.name.precedence() > user_precedence) - .find_map(|layer| { - layer - .config - .get("model_provider") - .map(|value| (layer, value)) - }) - && effective_provider.as_str() != Some(AMAZON_BEDROCK_PROVIDER_ID) - { - let source = format_config_layer_source(&overriding_layer.name, CONFIG_TOML_FILE); - return Err(invalid_request(format!( - "Amazon Bedrock login cannot select `{AMAZON_BEDROCK_PROVIDER_ID}` because {source} sets `model_provider` to {effective_provider}" - ))); + let mut settings = vec![("aws.profile", config.profile)]; + if let Some(region) = config.region { + settings.push(("aws.region", Some(region))); } - - if layers - .effective_config() - .get("model_providers") - .and_then(|providers| providers.get(AMAZON_BEDROCK_PROVIDER_ID)) - .and_then(|provider| provider.get("aws")) - .and_then(|aws| aws.get("credential_export")) - .is_some() + let (write, provider) = config_manager + .prepare_provider_config(&layers, AMAZON_BEDROCK_PROVIDER_ID, settings) + .map_err(map_config_error)?; + if provider + .aws + .is_some_and(|aws| aws.credential_export.is_some()) { return Err(invalid_request( "Amazon Bedrock is configured to use `aws.credential_export`. Please clear this setting to use another sign-in method.", )); } - - Ok(()) + Ok(write) } pub(super) async fn clear_user_model_provider_if_bedrock( diff --git a/codex-rs/app-server/tests/suite/v2/account.rs b/codex-rs/app-server/tests/suite/v2/account.rs index 3ffcf0d2f7..259438bee8 100644 --- a/codex-rs/app-server/tests/suite/v2/account.rs +++ b/codex-rs/app-server/tests/suite/v2/account.rs @@ -1441,7 +1441,7 @@ async fn login_amazon_bedrock_rejects_non_bedrock_provider_override_without_chan .await??; assert_eq!( error.error.message, - "Amazon Bedrock login cannot select `amazon-bedrock` because session-flags sets `model_provider` to \"mock_provider\"" + "Cannot configure `model_provider`: Overridden by session flags" ); assert_eq!(load_file_auth(codex_home.path())?, expected_auth); assert_eq!(read_config_toml(codex_home.path())?, expected_config); @@ -1470,10 +1470,9 @@ async fn login_amazon_bedrock_rejects_non_bedrock_provider_override_without_chan #[tokio::test] async fn login_amazon_bedrock_access_keys_rejects_overridden_aws_configuration() -> Result<()> { - for config_override in [ - r#"model_providers.amazon-bedrock.aws.profile="other-account""#, - r#"model_providers.amazon-bedrock.aws.region="eu-west-1""#, - ] { + for (field, value) in [("profile", "other-account"), ("region", "eu-west-1")] { + let key = format!("model_providers.amazon-bedrock.aws.{field}"); + let config_override = format!(r#"{key}="{value}""#); let codex_home = TempDir::new()?; create_config_toml(codex_home.path(), CreateConfigTomlParams::default())?; login_with_api_key( @@ -1488,9 +1487,11 @@ async fn login_amazon_bedrock_access_keys_rejects_overridden_aws_configuration() .with_codex_home(codex_home.path()) .without_auto_env() .with_env_overrides(&[("OPENAI_API_KEY", None)]) - .with_args(&["-c", config_override]) + .with_args(&["-c", &config_override]) .build_initialized_with_timeout(DEFAULT_READ_TIMEOUT) .await?; + let config_path = codex_home.path().join("config.toml"); + let expected_config = std::fs::read_to_string(&config_path)?; let request_id = mcp .send_login_account_request(json!({ @@ -1508,9 +1509,10 @@ async fn login_amazon_bedrock_access_keys_rejects_overridden_aws_configuration() assert_eq!( error.error.message, - "Amazon Bedrock configuration cannot take effect: Overridden by session flags" + format!("Cannot configure `{key}`: Overridden by session flags") ); assert_eq!(load_file_auth(codex_home.path())?, expected_auth); + assert_eq!(std::fs::read_to_string(&config_path)?, expected_config); } Ok(()) diff --git a/codex-rs/app-server/tests/suite/v2/bedrock_setup.rs b/codex-rs/app-server/tests/suite/v2/bedrock_setup.rs index 21ce43474b..3b0695c366 100644 --- a/codex-rs/app-server/tests/suite/v2/bedrock_setup.rs +++ b/codex-rs/app-server/tests/suite/v2/bedrock_setup.rs @@ -199,7 +199,11 @@ async fn setup_bedrock_profile_and_environment() -> Result<()> { let managed_home = TempDir::new()?; let managed_config_path = managed_home.path().join("config.toml"); - std::fs::write(&managed_config_path, "")?; + std::fs::write(&managed_config_path, "model_provider = 'openai'")?; + std::fs::write( + managed_home.path().join("requirements.toml"), + "model_provider = 'amazon-bedrock'", + )?; login_with_bedrock_api_key( managed_home.path(), "managed-bedrock-api-key", @@ -223,7 +227,6 @@ async fn setup_bedrock_profile_and_environment() -> Result<()> { .as_table_mut() .expect("config should be a table") .extend(toml::toml! { - model_provider = "amazon-bedrock" [model_providers.amazon-bedrock.aws] profile = "engineering" region = "us-west-2" diff --git a/codex-rs/app-server/tests/suite/v2/gcp_setup_tests.rs b/codex-rs/app-server/tests/suite/v2/gcp_setup_tests.rs new file mode 100644 index 0000000000..6be43f95c7 --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/gcp_setup_tests.rs @@ -0,0 +1,350 @@ +//! GCP setup validates before saving and shares provider configuration with subsequent requests. + +use anyhow::Context; +use anyhow::Result; +use app_test_support::TestAppServer; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::GcpSetupParams; +use codex_app_server_protocol::GcpSetupResponse; +use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::ModelListParams; +use codex_app_server_protocol::ModelListResponse; +use codex_app_server_protocol::RequestId; +use codex_protocol::openai_models::ModelVisibility; +use pretty_assertions::assert_eq; +use serde_json::Value; +use serde_json::json; +use std::path::Path; +use std::path::PathBuf; +use std::time::Duration; +use tempfile::TempDir; +use tokio::time::timeout; + +const CLEARED_AUTH_ENV: &[(&str, Option<&str>)] = &[ + ("OPENAI_API_KEY", None), + ("CODEX_API_KEY", None), + ("OPENAI_FEDERATION_RULE_ID", None), + ("OPENAI_IDENTITY_TOKEN_FILE", None), + ("OPENAI_WORKLOAD_IDENTITY_CONTEXT", None), +]; + +const READ_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 60); + +fn write_fake_gcloud(home: &Path) -> Result { + #[cfg(unix)] + let script = home.join("gcloud"); + #[cfg(unix)] + codex_utils_cargo_bin::write_executable( + &script, + r#"#!/bin/sh +if [ -f fail ]; then exit 1; fi +printf '%s\n' "$GOOGLE_APPLICATION_CREDENTIALS" >> credential-log +printf '%s\n' setup-token +"#, + )?; + #[cfg(windows)] + let script = home.join("gcloud.cmd"); + #[cfg(windows)] + std::fs::write( + &script, + "@echo off\r\nif exist fail exit /b 1\r\nsetlocal EnableDelayedExpansion\r\necho(!GOOGLE_APPLICATION_CREDENTIALS!>>credential-log\r\nendlocal\r\necho setup-token\r\n", + )?; + Ok(script) +} + +async fn gcp_app_server( + home: &Path, + env_overrides: &[(&str, Option<&str>)], +) -> Result { + let existing_path = std::env::var_os("PATH").unwrap_or_default(); + let child_path = std::env::join_paths( + std::iter::once(home.to_path_buf()).chain(std::env::split_paths(&existing_path)), + )?; + TestAppServer::builder() + .with_codex_home(home) + .with_env_overrides(&[ + ( + "PATH", + Some(child_path.to_str().context("child PATH must be UTF-8")?), + ), + ("GOOGLE_APPLICATION_CREDENTIALS", None), + ("GOOGLE_CLOUD_PROJECT", Some("ambient-project")), + ("GOOGLE_CLOUD_LOCATION", Some("ambient-location")), + ]) + .with_env_overrides(CLEARED_AUTH_ENV) + .with_env_overrides(env_overrides) + .build_initialized_with_timeout(READ_TIMEOUT) + .await +} + +async fn setup_error(app_server: &mut TestAppServer, params: Value) -> Result { + let id = app_server + .send_raw_request("account/gcp/setup", Some(params)) + .await?; + let error = timeout( + READ_TIMEOUT, + app_server.read_stream_until_error_message(RequestId::Integer(id)), + ) + .await??; + assert_eq!(error.error.code, -32600); + Ok(error.error) +} + +#[tokio::test] +async fn setup_gcp_saves_overrides_restores_adc_and_refreshes_catalog() -> Result<()> { + let home = TempDir::new()?; + let home = home.path(); + write_fake_gcloud(home)?; + let mut catalog = codex_models_manager::bundled_models_response()?; + catalog.models.retain(|model| model.slug == "gpt-6.1-sol"); + let model = catalog + .models + .first_mut() + .context("bundled catalog must include a supported Vertex model")?; + model.visibility = ModelVisibility::List; + model.supported_in_api = true; + let mut unsupported_model = model.clone(); + unsupported_model.slug = "test-non-vertex-model".to_string(); + catalog.models.push(unsupported_model); + let catalog_path = home.join("models.json"); + std::fs::write(&catalog_path, serde_json::to_vec(&catalog)?)?; + let config_path = home.join("config.toml"); + let mut expected_config = json!({ + "model_catalog_json": catalog_path, + "model_providers": {"gcp": {"http_headers": {"X-Existing": "preserved"}}} + }); + std::fs::write(&config_path, toml::to_string(&expected_config)?)?; + let selected = home.join("selected.json"); + let ambient = home.join("ambient.json"); + for file in [&selected, &ambient] { + std::fs::write(file, r#"{"type":"external_account"}"#)?; + } + let mut app_server = gcp_app_server( + home, + &[( + "GOOGLE_APPLICATION_CREDENTIALS", + Some(ambient.to_str().context("non-UTF-8 ADC path")?), + )], + ) + .await?; + let before: ModelListResponse = app_server + .request(|request_id| ClientRequest::ModelList { + request_id, + params: ModelListParams::default(), + }) + .await?; + assert_eq!(before.data.len(), 2); + + let _: GcpSetupResponse = app_server + .request(|request_id| ClientRequest::GcpSetup { + request_id, + params: GcpSetupParams { + project_id: " selected-project ".to_string(), + location: Some(" us-central1 ".to_string()), + credential_file: Some(selected.to_string_lossy().into_owned()), + }, + }) + .await?; + expected_config["model_provider"] = json!("gcp"); + expected_config["model_providers"]["gcp"]["project_id"] = json!("selected-project"); + expected_config["model_providers"]["gcp"]["location"] = json!("us-central1"); + expected_config["model_providers"]["gcp"]["credential_file"] = json!(selected); + assert_eq!( + toml::from_str::(&std::fs::read_to_string(&config_path)?)?, + expected_config + ); + + let after: ModelListResponse = app_server + .request(|request_id| ClientRequest::ModelList { + request_id, + params: ModelListParams::default(), + }) + .await?; + assert_eq!(after.data.len(), 1); + assert_eq!(after.data[0].model, "gpt-6.1-sol"); + + // A managed GCP selection takes precedence without replacing the user's preference. + expected_config["model_provider"] = json!("openai"); + std::fs::write(&config_path, toml::to_string(&expected_config)?)?; + std::fs::write(home.join("requirements.toml"), "model_provider = 'gcp'")?; + let request_id = app_server + .send_raw_request( + "account/gcp/setup", + Some(json!({"projectId": "other-project"})), + ) + .await?; + let _: GcpSetupResponse = app_server.read_response(request_id).await?; + expected_config["model_providers"]["gcp"]["project_id"] = json!("other-project"); + expected_config["model_providers"]["gcp"]["location"] = json!("global"); + expected_config["model_providers"]["gcp"] + .as_object_mut() + .unwrap() + .remove("credential_file"); + assert_eq!( + toml::from_str::(&std::fs::read_to_string(&config_path)?)?, + expected_config + ); + assert_eq!( + std::fs::read_to_string(home.join("credential-log"))? + .lines() + .map(Path::new) + .collect::>(), + [selected.as_path(), ambient.as_path()] + ); + Ok(()) +} + +#[tokio::test] +async fn setup_gcp_rejects_invalid_inputs_and_credentials_before_saving() -> Result<()> { + let home = TempDir::new()?; + let home = home.path(); + let config_path = home.join("config.toml"); + std::fs::write(&config_path, "")?; + let invalid = home.join("invalid.json"); + std::fs::write(&invalid, r#"["authorized_user"]"#)?; + let mut server = gcp_app_server( + home, + &[("PATH", Some(home.to_str().context("non-UTF-8 PATH")?))], + ) + .await?; + let params = json!({"projectId": "project"}); + for (request, reason) in [ + (json!({"projectId": " "}), None), + (params.clone(), Some("gcloudNotInstalled")), + ( + json!({"projectId": "project", "credentialFile": invalid}), + Some("credentialFileInvalid"), + ), + ] { + let error = setup_error(&mut server, request).await?; + assert_eq!(error.data, reason.map(|reason| json!({ "reason": reason }))); + } + + write_fake_gcloud(home)?; + std::fs::write(home.join("fail"), "")?; + let error = setup_error(&mut server, params).await?; + assert_eq!(error.data, Some(json!({"reason": "authCommandFailed"}))); + assert_eq!(std::fs::read_to_string(config_path)?, ""); + Ok(()) +} + +#[tokio::test] +async fn setup_gcp_validates_the_auth_override_relative_to_the_startup_directory() -> Result<()> { + let home = TempDir::new()?; + let startup = TempDir::new()?; + let script = write_fake_gcloud(home.path())?; + let auth = toml::Value::try_from(json!({"command": script, "cwd": "."}))?; + let config_path = home.path().join("config.toml"); + std::fs::write(&config_path, "")?; + // The same command succeeds in CODEX_HOME but fails in the catalog's startup context. + std::fs::write(startup.path().join("fail"), "")?; + let mut server = TestAppServer::builder() + .with_codex_home(startup.path()) + .with_env_overrides(CLEARED_AUTH_ENV) + .with_env_overrides(&[( + "CODEX_HOME", + Some(home.path().to_str().context("non-UTF-8 CODEX_HOME")?), + )]) + .with_args(&["-c", &format!("model_providers.gcp.auth={auth}")]) + .build_initialized_with_timeout(READ_TIMEOUT) + .await?; + + let error = setup_error(&mut server, json!({"projectId": "project"})).await?; + assert_eq!(error.data, Some(json!({"reason": "authCommandFailed"}))); + assert_eq!(std::fs::read_to_string(config_path)?, ""); + Ok(()) +} + +#[tokio::test] +async fn setup_gcp_rejects_policy_and_endpoint_conflicts_before_saving() -> Result<()> { + for (file, policy, expected_message) in [ + ( + "managed_config.toml", + "forced_login_method = 'chatgpt'", + "Google Cloud setup is disabled. Use ChatGPT login instead.", + ), + ( + "managed_config.toml", + "[model_providers.gcp]\nproject_id = 'managed-project'", + "Cannot configure `model_providers.gcp.project_id`: Overridden by legacy managed_config.toml", + ), + ( + "managed_config.toml", + "[model_providers.gcp]\nbase_url = 'https://vertex.example.com/v1'", + "Google Cloud has a custom `base_url`", + ), + ( + "requirements.toml", + "model_provider = 'openai'", + "requirements set `model_provider` to `openai`", + ), + ( + "requirements.toml", + "[application.network.domains]\n'oauth2.googleapis.com' = 'allow'", + "disabled by application network restrictions", + ), + ] { + let home = TempDir::new()?; + let home = home.path(); + write_fake_gcloud(home)?; + std::fs::write(home.join(file), policy)?; + let config_path = home.join("config.toml"); + std::fs::write(&config_path, "")?; + let mut app_server = gcp_app_server(home, &[]).await?; + let error = setup_error( + &mut app_server, + json!({"projectId": "project", "credentialFile": home.join("missing.json")}), + ) + .await?; + assert!( + error.message.contains(expected_message), + "expected {expected_message:?}, got {:?}", + error.message + ); + assert_eq!(std::fs::read_to_string(config_path)?, ""); + assert!(!home.join("credential-log").exists()); + } + Ok(()) +} + +#[cfg(unix)] +#[tokio::test] +async fn setup_gcp_stops_the_credential_command_when_network_permission_is_revoked() -> Result<()> { + use core_test_support::process::wait_for_pid_file; + use core_test_support::process::wait_for_process_exit; + + let home = TempDir::new()?; + let config_path = home.path().join("config.toml"); + let initial_config = r#" +[model_providers.gcp.auth] +command = "sh" +args = ["-c", "echo $$ > refresh.pid; exec sleep 120"] +timeout_ms = 120000 +"#; + std::fs::write(&config_path, initial_config)?; + let mut app_server = gcp_app_server(home.path(), &[]).await?; + let request_id = app_server + .send_raw_request("account/gcp/setup", Some(json!({"projectId": "project"}))) + .await?; + let pid = wait_for_pid_file(&home.path().join("refresh.pid")).await?; + + std::fs::write( + home.path().join("requirements.toml"), + "[application.network]", + )?; + app_server + .send_raw_request("config/read", Some(json!({}))) + .await?; + let error = timeout( + READ_TIMEOUT, + app_server.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + assert_eq!( + error.error.message, + "application network permission was revoked" + ); + wait_for_process_exit(&pid).await?; + assert_eq!(std::fs::read_to_string(config_path)?, initial_config); + Ok(()) +} diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index 42c6ecb14f..032f5e5a32 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -76,6 +76,8 @@ mod frozen_cloud_skills; mod fs; #[path = "gateway_oauth_tests.rs"] mod gateway_oauth; +#[path = "gcp_setup_tests.rs"] +mod gcp_setup; mod git_attribution; mod guardian_v2; mod history_notes_extension; diff --git a/codex-rs/login/src/auth/external_bearer.rs b/codex-rs/login/src/auth/external_bearer.rs index ed58f4ccdb..4fcea04f0e 100644 --- a/codex-rs/login/src/auth/external_bearer.rs +++ b/codex-rs/login/src/auth/external_bearer.rs @@ -130,10 +130,13 @@ async fn run_provider_auth_command( )) })? .map_err(|err| { - io::Error::other(format!( - "provider auth command `{}` failed to start: {err}", - config.command - )) + io::Error::new( + err.kind(), + format!( + "provider auth command `{}` failed to start: {err}", + config.command + ), + ) })?; if !output.status.success() { diff --git a/codex-rs/model-provider-info/src/lib.rs b/codex-rs/model-provider-info/src/lib.rs index 9678d7056b..78fc793c35 100644 --- a/codex-rs/model-provider-info/src/lib.rs +++ b/codex-rs/model-provider-info/src/lib.rs @@ -800,40 +800,7 @@ 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`, `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); - } - if let Some(auth_override) = auth_override { - built_in_provider.auth = Some(auth_override); - } - if let Some(http_headers_override) = http_headers_override { - built_in_provider - .http_headers - .get_or_insert_default() - .extend(http_headers_override); - } - } + merge_gcp_provider_overrides(model_providers.get_mut(&key), provider)?; } else { model_providers.entry(key).or_insert(provider); } @@ -842,6 +809,48 @@ pub fn merge_configured_model_providers( Ok(model_providers) } +/// Validates GCP overrides and applies them when the built-in provider is present. +pub fn merge_gcp_provider_overrides( + built_in: Option<&mut ModelProviderInfo>, + mut overrides: ModelProviderInfo, +) -> Result<(), String> { + overrides + .validate_gcp_credential_file() + .and_then(|()| overrides.validate()) + .map_err(|message| format!("model_providers.{GCP_PROVIDER_ID} {message}"))?; + let base_url = overrides.base_url.take(); + let auth = overrides.auth.take(); + let credential_file = overrides.credential_file.take(); + let http_headers = overrides.http_headers.take(); + let project_id = overrides.project_id.take(); + let location = overrides.location.take(); + if overrides != ModelProviderInfo::default() { + return Err(format!( + "model_providers.{GCP_PROVIDER_ID} only supports changing \ +`base_url`, `auth`, `http_headers`, `project_id`, `location`, and `credential_file`; other non-default provider fields are not supported" + )); + } + + if let Some(provider) = built_in { + provider.project_id = project_id; + provider.location = location; + provider.credential_file = credential_file; + if let Some(base_url) = base_url { + provider.base_url = Some(base_url); + } + if let Some(auth) = auth { + provider.auth = Some(auth); + } + if let Some(http_headers) = http_headers { + provider + .http_headers + .get_or_insert_default() + .extend(http_headers); + } + } + Ok(()) +} + pub fn create_oss_provider(default_provider_port: u16, wire_api: WireApi) -> ModelProviderInfo { // These CODEX_OSS_ environment variables are experimental: we may // switch to reading values from config.toml instead.