diff --git a/codex-rs/app-server-test-client/README.md b/codex-rs/app-server-test-client/README.md index 9f73b1db6f5..9d559d80b4a 100644 --- a/codex-rs/app-server-test-client/README.md +++ b/codex-rs/app-server-test-client/README.md @@ -46,6 +46,18 @@ cargo run -p codex-app-server-test-client -- \ The test client redacts `apiKey` from its outbound request log. After login, start a fresh Codex process with the same `CODEX_HOME` to verify that it uses the persisted managed credential. +## Testing logout + +`test-logout` initializes the app-server, sends an `account/logout` request, and waits for the +resulting `account/updated` notification. It uses the active `CODEX_HOME`, so point it at an +isolated directory when testing credential cleanup. + +```bash +cargo run -p codex-app-server-test-client -- \ + --codex-bin ./target/debug/codex \ + test-logout +``` + ## Testing Plugin Analytics The `plugin-analytics-smoke` command exercises `plugin/installed`, plugin diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index 10d66dbc01d..21becb302fb 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -46,6 +46,7 @@ use codex_app_server_protocol::JSONRPCNotification; use codex_app_server_protocol::JSONRPCRequest; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::LoginAccountResponse; +use codex_app_server_protocol::LogoutAccountResponse; use codex_app_server_protocol::ModelListParams; use codex_app_server_protocol::ModelListResponse; use codex_app_server_protocol::RequestId; @@ -245,6 +246,8 @@ enum CliCommand { #[arg(long, value_name = "REGION")] region: Option, }, + /// Log out of the current account and wait for the account update. + TestLogout, /// Fetch the current account rate limits from the Codex app-server. GetAccountRateLimits, /// List the available models from the Codex app-server. @@ -449,6 +452,11 @@ pub async fn run() -> Result<()> { }; test_login(&endpoint, &config_overrides, mode).await } + CliCommand::TestLogout => { + ensure_dynamic_tools_unused(&dynamic_tools, "test-logout")?; + let endpoint = resolve_endpoint(codex_bin, url)?; + test_logout(&endpoint, &config_overrides).await + } CliCommand::GetAccountRateLimits => { ensure_dynamic_tools_unused(&dynamic_tools, "get-account-rate-limits")?; let endpoint = resolve_endpoint(codex_bin, url)?; @@ -1253,6 +1261,27 @@ async fn get_account_rate_limits(endpoint: &Endpoint, config_overrides: &[String .await } +async fn test_logout(endpoint: &Endpoint, config_overrides: &[String]) -> Result<()> { + with_client("test-logout", endpoint, config_overrides, |client| { + let initialize = client.initialize()?; + println!("< initialize response: {initialize:?}"); + + let response = client.logout_account()?; + println!("< account/logout response: {response:?}"); + + loop { + let notification = client.next_notification()?; + if let Ok(ServerNotification::AccountUpdated(account_updated)) = + ServerNotification::try_from(notification) + { + println!("< account/updated notification: {account_updated:?}"); + return Ok(()); + } + } + }) + .await +} + async fn model_list(endpoint: &Endpoint, config_overrides: &[String]) -> Result<()> { with_client("model-list", endpoint, config_overrides, |client| { let initialize = client.initialize()?; @@ -1809,6 +1838,16 @@ impl CodexClient { self.send_request(request, request_id, "account/rateLimits/read") } + fn logout_account(&mut self) -> Result { + let request_id = self.request_id(); + let request = ClientRequest::LogoutAccount { + request_id: request_id.clone(), + params: None, + }; + + self.send_request(request, request_id, "account/logout") + } + fn model_list(&mut self, params: ModelListParams) -> Result { let request_id = self.request_id(); let request = ClientRequest::ModelList { diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 6280cf52040..a4f75d1541e 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -2060,6 +2060,8 @@ Codex stores the key and region as the primary Codex auth, replacing any previou { "method": "account/updated", "params": { "authMode": null, "planType": null } } ``` +When Codex-managed Amazon Bedrock auth is stored, logout removes that credential and clears the user-level `model_provider` only when it is still `"amazon-bedrock"`. A concurrent user config change is preserved. For AWS-managed Bedrock auth, logout is a no-op because the AWS SDK credential chain is managed outside Codex. + ### 7) Rate limits (ChatGPT) ```json diff --git a/codex-rs/app-server/src/config_manager_service.rs b/codex-rs/app-server/src/config_manager_service.rs index 94980bf8e01..5640c9cb740 100644 --- a/codex-rs/app-server/src/config_manager_service.rs +++ b/codex-rs/app-server/src/config_manager_service.rs @@ -181,6 +181,43 @@ impl ConfigManager { .await } + /// Clears a value from the active user config only when its current raw value matches. + pub(crate) async fn clear_user_value_if_matches( + &self, + key_path: &str, + expected_value: JsonValue, + ) -> Result<(), ConfigManagerError> { + let layers = self + .load_thread_agnostic_config() + .await + .map_err(|err| ConfigManagerError::io("failed to load configuration", err))?; + let Some(user_layer) = layers.get_active_user_layer() else { + return Ok(()); + }; + let segments = parse_key_path(key_path).map_err(|message| { + ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, message) + })?; + let expected_value = parse_value(expected_value).map_err(|message| { + ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, message) + })?; + if value_at_path(&user_layer.config, &segments) != expected_value.as_ref() { + return Ok(()); + } + let expected_version = Some(user_layer.version.clone()); + + self.apply_edits( + /*file_path*/ None, + expected_version, + vec![( + key_path.to_string(), + JsonValue::Null, + MergeStrategy::Replace, + )], + ) + .await?; + Ok(()) + } + pub(crate) async fn batch_write( &self, params: ConfigBatchWriteParams, 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 1ec79a749b3..3407c1b423f 100644 --- a/codex-rs/app-server/src/config_manager_service_tests.rs +++ b/codex-rs/app-server/src/config_manager_service_tests.rs @@ -130,6 +130,40 @@ async fn clear_missing_nested_config_is_noop() -> Result<()> { Ok(()) } +#[tokio::test] +async fn clear_user_value_if_matches_clears_matching_value() -> Result<()> { + let tmp = tempdir().expect("tempdir"); + let path = tmp.path().join(CONFIG_TOML_FILE); + std::fs::write(&path, "model = \"gpt-5.2\"\napproval_policy = \"never\"\n")?; + + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); + service + .clear_user_value_if_matches("model", serde_json::json!("gpt-5.2")) + .await?; + + assert_eq!( + std::fs::read_to_string(&path)?, + "approval_policy = \"never\"\n" + ); + Ok(()) +} + +#[tokio::test] +async fn clear_user_value_if_matches_preserves_non_matching_value() -> Result<()> { + let tmp = tempdir().expect("tempdir"); + let path = tmp.path().join(CONFIG_TOML_FILE); + let original = "model = \"gpt-5.2\"\napproval_policy = \"never\"\n"; + std::fs::write(&path, original)?; + + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); + service + .clear_user_value_if_matches("model", serde_json::json!("gpt-5.3")) + .await?; + + assert_eq!(std::fs::read_to_string(&path)?, original); + Ok(()) +} + #[tokio::test] async fn write_value_rejects_legacy_profile_selector() -> Result<()> { let tmp = tempdir().expect("tempdir"); 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 8a7243ffdd5..584aef36bbc 100644 --- a/codex-rs/app-server/src/request_processors/account_processor.rs +++ b/codex-rs/app-server/src/request_processors/account_processor.rs @@ -1,3 +1,4 @@ +use super::bedrock_auth::clear_user_model_provider_if_bedrock; use super::bedrock_auth::set_user_model_provider_to_bedrock; use super::*; use crate::auth_mode::auth_mode_to_api; @@ -833,6 +834,20 @@ impl AccountRequestProcessor { } } + let managed_bedrock_auth = matches!( + self.auth_manager.auth_cached(), + Some(CodexAuth::BedrockApiKey(_)) + ); + let config = self.load_latest_config().await?; + if config.model_provider.is_amazon_bedrock() && !managed_bedrock_auth { + return Ok(self + .auth_manager + .auth_cached() + .as_ref() + .map(CodexAuth::api_auth_mode) + .map(auth_mode_to_api)); + } + match self.auth_manager.logout_with_revoke().await { Ok(_) => {} Err(err) => { @@ -840,6 +855,10 @@ impl AccountRequestProcessor { } } + if managed_bedrock_auth { + clear_user_model_provider_if_bedrock(&self.config_manager).await?; + } + Self::maybe_refresh_plugin_caches_for_current_config( &self.config_manager, &self.thread_manager, 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 88576e5ffe9..4a023d989a6 100644 --- a/codex-rs/app-server/src/request_processors/bedrock_auth.rs +++ b/codex-rs/app-server/src/request_processors/bedrock_auth.rs @@ -7,29 +7,28 @@ use codex_model_provider::AMAZON_BEDROCK_PROVIDER_ID; pub(super) async fn set_user_model_provider_to_bedrock( config_manager: &ConfigManager, -) -> Result<(), JSONRPCErrorError> { - write_user_model_provider( - config_manager, - serde_json::json!(AMAZON_BEDROCK_PROVIDER_ID), - /*expected_version*/ None, - ) - .await -} - -async fn write_user_model_provider( - config_manager: &ConfigManager, - value: serde_json::Value, - expected_version: Option, ) -> Result<(), JSONRPCErrorError> { config_manager .write_value(ConfigValueWriteParams { key_path: "model_provider".to_string(), - value, + value: serde_json::json!(AMAZON_BEDROCK_PROVIDER_ID), merge_strategy: MergeStrategy::Replace, file_path: None, - expected_version, + expected_version: None, }) .await .map(|_| ()) .map_err(map_config_error) } + +pub(super) async fn clear_user_model_provider_if_bedrock( + config_manager: &ConfigManager, +) -> Result<(), JSONRPCErrorError> { + config_manager + .clear_user_value_if_matches( + "model_provider", + serde_json::json!(AMAZON_BEDROCK_PROVIDER_ID), + ) + .await + .map_err(map_config_error) +} diff --git a/codex-rs/app-server/tests/suite/v2/account.rs b/codex-rs/app-server/tests/suite/v2/account.rs index 2ffec32bb9b..79a5a640bbf 100644 --- a/codex-rs/app-server/tests/suite/v2/account.rs +++ b/codex-rs/app-server/tests/suite/v2/account.rs @@ -166,6 +166,20 @@ fn load_file_auth(codex_home: &Path) -> Result> { )?) } +fn aws_managed_bedrock_config() -> CreateConfigTomlParams { + CreateConfigTomlParams { + model_provider_id: Some("amazon-bedrock".to_string()), + extra_provider_config: Some( + r#"[model_providers.amazon-bedrock.aws] +profile = "codex-bedrock" +region = "us-west-2" +"# + .to_string(), + ), + ..Default::default() + } +} + async fn read_account(mcp: &mut TestAppServer) -> Result { let request_id = mcp .send_get_account_request(GetAccountParams { @@ -1147,6 +1161,169 @@ async fn login_amazon_bedrock_replaces_primary_auth_and_persists_provider() -> R Ok(()) } +#[tokio::test] +async fn logout_managed_bedrock_restores_default_account() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), CreateConfigTomlParams::default())?; + let mut expected_config = read_config_toml(codex_home.path())?; + expected_config + .as_table_mut() + .expect("config should be a table") + .remove("model_provider"); + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .with_env_overrides(&[("OPENAI_API_KEY", None)]) + .build() + .await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + let request_id = mcp + .send_login_account_amazon_bedrock_request("managed-bedrock-api-key", "us-west-2") + .await?; + let response = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + assert_eq!( + to_response::(response)?, + LoginAccountResponse::AmazonBedrock {} + ); + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("account/login/completed"), + ) + .await??; + assert_account_updated(&mut mcp, Some(AuthMode::BedrockApiKey)).await?; + assert_eq!( + read_account(&mut mcp).await?, + GetAccountResponse { + account: Some(Account::AmazonBedrock { + credential_source: AmazonBedrockCredentialSource::CodexManaged, + }), + requires_openai_auth: false, + } + ); + + let request_id = mcp.send_logout_account_request().await?; + let response = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + assert_eq!( + to_response::(response)?, + LogoutAccountResponse {} + ); + assert_eq!(load_file_auth(codex_home.path())?, None); + assert_eq!(read_config_toml(codex_home.path())?, expected_config); + assert_account_updated(&mut mcp, /*auth_mode*/ None).await?; + assert_eq!( + read_account(&mut mcp).await?, + GetAccountResponse { + account: None, + requires_openai_auth: true, + } + ); + Ok(()) +} + +#[tokio::test] +async fn logout_aws_managed_bedrock_preserves_openai_auth_and_config() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), aws_managed_bedrock_config())?; + login_with_api_key( + codex_home.path(), + "sk-test-key", + AuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), + )?; + let expected_auth = load_file_auth(codex_home.path())?; + let expected_config = read_config_toml(codex_home.path())?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .with_env_overrides(&[("OPENAI_API_KEY", None)]) + .build() + .await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + let request_id = mcp.send_logout_account_request().await?; + let response = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + assert_eq!( + to_response::(response)?, + LogoutAccountResponse {} + ); + assert_eq!(load_file_auth(codex_home.path())?, expected_auth); + assert_eq!(read_config_toml(codex_home.path())?, expected_config); + assert_account_updated(&mut mcp, Some(AuthMode::ApiKey)).await?; + Ok(()) +} + +#[tokio::test] +async fn logout_managed_bedrock_preserves_changed_provider_without_experimental_api() -> Result<()> +{ + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), aws_managed_bedrock_config())?; + login_with_bedrock_api_key( + codex_home.path(), + "managed-bedrock-api-key", + "us-west-2", + AuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), + )?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build() + .await?; + let initialized = mcp + .initialize_with_capabilities( + ClientInfo { + name: DEFAULT_CLIENT_NAME.to_string(), + title: None, + version: "0.1.0".to_string(), + }, + Some(InitializeCapabilities { + experimental_api: false, + ..Default::default() + }), + ) + .await?; + assert!(matches!(initialized, JSONRPCMessage::Response(_))); + + create_config_toml(codex_home.path(), CreateConfigTomlParams::default())?; + let expected_config = read_config_toml(codex_home.path())?; + + let request_id = mcp.send_logout_account_request().await?; + let response = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + assert_eq!( + to_response::(response)?, + LogoutAccountResponse {} + ); + assert_eq!(load_file_auth(codex_home.path())?, None); + assert_eq!(read_config_toml(codex_home.path())?, expected_config); + assert_account_updated(&mut mcp, /*auth_mode*/ None).await?; + assert_eq!( + read_account(&mut mcp).await?, + GetAccountResponse { + account: None, + requires_openai_auth: false, + } + ); + Ok(()) +} + #[tokio::test] async fn managed_bedrock_login_requires_experimental_api() -> Result<()> { let codex_home = TempDir::new()?;