Handle Amazon Bedrock credentials during logout (#33175)

## Why

Amazon Bedrock can use either a Codex-managed API key or credentials managed by
AWS, so logout must not remove or misrepresent credentials that Codex does not
control.

## What changed

- Remove the managed Bedrock key on logout and clear `model_provider` only when
  its user-config value is still `amazon-bedrock`.
- Reject logout for AWS-managed Bedrock credentials without changing existing
  authentication or configuration.
- Add a `test-logout` app-server test-client command that waits for the
  resulting `account/updated` notification.

## Testing

Add coverage for managed and AWS-managed Bedrock logout, concurrent provider
changes, config reload failures, and conditional user-config cleanup.

GitOrigin-RevId: a003c0ec27ad7b36d499fe7b63ab4a0dd6369b4f
This commit is contained in:
Celia Chen
2026-07-14 21:54:24 +00:00
committed by copyberry
parent c38c30ef51
commit 6e215e0db5
8 changed files with 384 additions and 16 deletions
+12
View File
@@ -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
@@ -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<String>,
},
/// 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<LogoutAccountResponse> {
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<ModelListResponse> {
let request_id = self.request_id();
let request = ClientRequest::ModelList {
+3 -1
View File
@@ -1937,7 +1937,7 @@ Codex supports these authentication modes. The current mode is surfaced in `acco
- `account/login/start` — begin login (`apiKey`, `chatgpt`, `chatgptDeviceCode`, `amazonBedrock`).
- `account/login/completed` (notify) — emitted when a login attempt finishes (success or error).
- `account/login/cancel` — cancel a pending managed ChatGPT login by `loginId`.
- `account/logout` — sign out; triggers `account/updated`.
- `account/logout` — sign out; triggers `account/updated` on success.
- `account/updated` (notify) — emitted whenever auth mode changes (`authMode`: `apikey`, `bedrockApiKey`, `chatgpt`, `personalAccessToken`, or `null`) and includes the current ChatGPT `planType` when available.
- `account/rateLimits/read` — fetch ChatGPT rate limits, an optional effective monthly credit limit, and the earned rate-limit resets currently available, including expiry details when provided by the backend. Rate-limit updates arrive via `account/rateLimits/updated` (notify); reset-credit data is snapshot-only.
- `account/rateLimitResetCredit/consume` — consume one earned reset using a caller-provided idempotency key, optionally selecting a reset-credit ID returned by `account/rateLimits/read`.
@@ -2067,6 +2067,8 @@ Codex stores the key and region as the primary Codex auth, replacing any previou
{ "method": "account/updated", "params": { "authMode": null, "planType": null } }
```
When using a Codex-managed Bedrock key, logout removes the key and clears `model_provider` if it is still set to `"amazon-bedrock"`. When using AWS-managed credentials, manage them through AWS or switch providers before logging out.
### 7) Rate limits (ChatGPT)
```json
@@ -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,
@@ -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");
@@ -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;
@@ -832,6 +833,17 @@ impl AccountRequestProcessor {
}
async fn logout_common(&self) -> std::result::Result<Option<AuthMode>, JSONRPCErrorError> {
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 Err(invalid_request(
"cannot log out while Amazon Bedrock is using AWS-managed credentials; manage those credentials through AWS or switch model providers before logging out Codex authentication",
));
}
// Cancel any active login attempt.
{
let mut guard = self.active_login.lock().await;
@@ -847,6 +859,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,
@@ -3,6 +3,7 @@ use crate::config_manager::ConfigManager;
use crate::error_code::internal_error;
use crate::error_code::invalid_request;
use codex_app_server_protocol::ConfigValueWriteParams;
use codex_app_server_protocol::ConfigWriteErrorCode;
use codex_app_server_protocol::JSONRPCErrorError;
use codex_app_server_protocol::MergeStrategy;
use codex_config::CONFIG_TOML_FILE;
@@ -45,28 +46,41 @@ pub(super) async fn set_user_model_provider_to_bedrock(
)));
}
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<String>,
) -> 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> {
let result = config_manager
.clear_user_value_if_matches(
"model_provider",
serde_json::json!(AMAZON_BEDROCK_PROVIDER_ID),
)
.await;
if let Err(err) = &result
&& err.write_error_code() == Some(ConfigWriteErrorCode::ConfigVersionConflict)
{
tracing::warn!(
"configuration changed while clearing the managed Amazon Bedrock model provider; retrying once"
);
return config_manager
.clear_user_value_if_matches(
"model_provider",
serde_json::json!(AMAZON_BEDROCK_PROVIDER_ID),
)
.await
.map_err(map_config_error);
}
result.map_err(map_config_error)
}
@@ -166,6 +166,20 @@ fn load_file_auth(codex_home: &Path) -> Result<Option<AuthDotJson>> {
)?)
}
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<GetAccountResponse> {
let request_id = mcp
.send_get_account_request(GetAccountParams {
@@ -318,6 +332,43 @@ async fn logout_account_removes_auth_and_notifies() -> Result<()> {
Ok(())
}
#[tokio::test]
async fn logout_account_succeeds_when_config_reload_fails() -> Result<()> {
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), CreateConfigTomlParams::default())?;
login_with_api_key(
codex_home.path(),
"sk-test-key",
AuthCredentialsStoreMode::File,
AuthKeyringBackendKind::default(),
)?;
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??;
std::fs::write(codex_home.path().join("config.toml"), "invalid = [")?;
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::<LogoutAccountResponse>(response)?,
LogoutAccountResponse {}
);
assert_eq!(load_file_auth(codex_home.path())?, None);
assert_account_updated(&mut mcp, /*auth_mode*/ None).await?;
Ok(())
}
#[tokio::test]
async fn set_auth_token_updates_account_and_notifies() -> Result<()> {
let codex_home = TempDir::new()?;
@@ -1267,6 +1318,169 @@ async fn login_amazon_bedrock_allows_bedrock_provider_override() -> Result<()> {
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::<LoginAccountResponse>(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::<LogoutAccountResponse>(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_errors_without_changing_auth_or_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 error = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
)
.await??;
assert_eq!(error.error.code, -32600);
assert_eq!(
error.error.message,
"cannot log out while Amazon Bedrock is using AWS-managed credentials; manage those credentials through AWS or switch model providers before logging out Codex authentication"
);
assert_eq!(load_file_auth(codex_home.path())?, expected_auth);
assert_eq!(read_config_toml(codex_home.path())?, expected_config);
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::<LogoutAccountResponse>(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()?;