github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 00:18:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Track credential storage outcomes and redact sensitive errors (#49384)

Browse Source 
## Why

Successful file fallback can hide a secure-store failure. Storage diagnostics need to distinguish that fallback from explicit file storage while keeping credential values out of error output.

## What changed

- Record credential load, save, delete, and fallback-file cleanup outcomes, including secure-store failures and fallback reasons.
- Preserve the initiating client's attribution across app-server requests, login tasks, sessions, turns, and blocking token persistence.
- Preserve storage error context and typed causes while redacting credential JSON data errors and payload-bearing keyring errors.
- Include sanitized fallback completion warnings in the default `codex login` file logs.

## Testing

Add coverage for storage policy and metric attribution, credential redaction, login diagnostics, concurrent app-server clients, device-code login, and token refresh during a turn.

GitOrigin-RevId: 6d5651c1718d184016fa77befa02aa528750b2ae
This commit is contained in:
Celia Chen
2026-09-29 22:24:09 +00:00
committed by copyberry
parent bd4204efc2
commit a44afa527c
24 changed files with 1465 additions and 353 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/Cargo.lock
Generated
+1
View File
@@ -3814,6 +3814,7 @@ dependencies = [
"jsonwebtoken",
"keyring",
"once_cell",
"opentelemetry_sdk",
"os_info",
"pretty_assertions",
"rand 0.9.3",
codex-rs/app-server/src/message_processor.rs
+15 -8
View File
@@ -83,6 +83,7 @@ use codex_feedback::CodexFeedback;
use codex_goal_extension::GoalService;
use codex_home::CodexHomeUserInstructionsProvider;
use codex_login::AuthManager;
use codex_otel::auth_storage::AuthStorageOriginator;
use codex_protocol::ThreadId;
use codex_protocol::mcp::ClientMcpExtensions;
use codex_protocol::protocol::SessionSource;
@@ -1031,15 +1032,21 @@ impl MessageProcessor {
return;
}
let processor_for_request = Arc::clone(&processor);
let originator = AuthStorageOriginator::from_client_name(
session.app_server_client_name().unwrap_or("none"),
);
// Keep queued requests small to avoid large stack temporaries during construction.
let result = Box::pin(processor_for_request.handle_initialized_client_request(
connection_request_id,
codex_request,
request_context,
session,
event_stream_ready,
))
.await;
let result = originator
.scope(Box::pin(
processor_for_request.handle_initialized_client_request(
connection_request_id,
codex_request,
request_context,
session,
event_stream_ready,
),
))
.await;
if let Err(error) = result {
processor.outgoing.send_error(error_request_id, error).await;
}
codex-rs/app-server/src/request_processors/account_processor.rs
+7 -4
View File
@@ -13,6 +13,7 @@ use codex_login::LoginOnboardingEntrypoint;
use codex_login::login_with_bedrock_access_keys;
use codex_mcp::ema_auth_scope;
use codex_model_provider::is_supported_amazon_bedrock_region;
use codex_otel::auth_storage::AuthStorageOriginator;
use codex_rmcp_client::EnterpriseOAuthCredentialGuard;
mod bedrock_setup;
@@ -678,7 +679,7 @@ impl AccountRequestProcessor {
let processor = self.clone();
let active_login = self.active_login.clone();
let auth_url = server.auth_url.clone();
tokio::spawn(async move {
let task = async move {
let (success, error_msg, onboarding_entrypoint) = match tokio::time::timeout(
LOGIN_CHATGPT_TIMEOUT,
server.block_until_done_with_callback_result(),
@@ -715,7 +716,8 @@ impl AccountRequestProcessor {
if guard.as_ref().map(ActiveLogin::login_id) == Some(login_id) {
*guard = None;
}
});
};
tokio::spawn(AuthStorageOriginator::current().scope(task));
Ok(LoginAccountResponse::Chatgpt {
login_id: login_id.to_string(),
@@ -759,7 +761,7 @@ impl AccountRequestProcessor {
let processor = self.clone();
let active_login = self.active_login.clone();
tokio::spawn(async move {
let task = async move {
let (success, error_msg) = tokio::select! {
_ = cancel.cancelled() => {
(false, Some("Login was not completed".to_string()))
@@ -785,7 +787,8 @@ impl AccountRequestProcessor {
if guard.as_ref().map(ActiveLogin::login_id) == Some(login_id) {
*guard = None;
}
});
};
tokio::spawn(AuthStorageOriginator::current().scope(task));
Ok(LoginAccountResponse::ChatgptDeviceCode {
login_id: login_id.to_string(),
codex-rs/app-server/tests/suite/v2/account.rs
+10 -1
View File
@@ -2166,6 +2166,7 @@ async fn login_account_chatgpt_device_code_succeeds_and_notifies() -> Result<()>
},
)?;
write_models_cache(codex_home.path()).await?;
let collector = super::auth_storage_originator::configure_collector(codex_home.path()).await?;
mock_device_code_usercode(&mock_server, /*interval_seconds*/ 0).await;
mock_device_code_token_success(&mock_server).await;
@@ -2184,10 +2185,17 @@ async fn login_account_chatgpt_device_code_succeeds_and_notifies() -> Result<()>
.without_auto_env()
.with_env_overrides(&[
("OPENAI_API_KEY", None),
("OTEL_METRIC_EXPORT_INTERVAL", Some("200")),
(LOGIN_ISSUER_ENV_VAR, Some(issuer.as_str())),
])
.build_initialized_with_timeout(DEFAULT_READ_TIMEOUT)
.build()
.await?;
mcp.initialize_with_client_info(ClientInfo {
name: "codex_vscode".into(),
title: None,
version: "1".into(),
})
.await?;
let request_id = mcp.send_login_account_chatgpt_device_code_request().await?;
let login: LoginAccountResponse =
@@ -2231,6 +2239,7 @@ async fn login_account_chatgpt_device_code_succeeds_and_notifies() -> Result<()>
codex_home.path().join("auth.json").exists(),
"auth.json should be created when device code login succeeds"
);
super::auth_storage_originator::assert_saved_originators(&collector, &["codex_vscode"]).await?;
Ok(())
}
codex-rs/app-server/tests/suite/v2/auth_storage_originator_tests.rs
+252
View File
@@ -0,0 +1,252 @@
//! Checks client attribution on exported storage metrics through the public RPC API.
use super::connection_handling_websocket::connect_websocket;
use super::connection_handling_websocket::read_response_for_id;
use super::connection_handling_websocket::send_initialize_request;
use super::connection_handling_websocket::send_request;
use super::connection_handling_websocket::spawn_websocket_server;
use anyhow::Context;
use anyhow::Result;
use app_test_support::ChatGptAuthFixture;
use app_test_support::MockResponsesConfig;
use app_test_support::TestAppServer;
use app_test_support::write_chatgpt_auth;
use app_test_support::write_models_cache;
use codex_app_server_protocol::ClientInfo;
use codex_app_server_protocol::ThreadStartParams;
use codex_app_server_protocol::ThreadStartResponse;
use codex_app_server_protocol::TurnCompletedNotification;
use codex_app_server_protocol::TurnStartParams;
use codex_app_server_protocol::TurnStartResponse;
use codex_app_server_protocol::TurnStatus;
use codex_app_server_protocol::UserInput;
use codex_login::AuthCredentialsStoreMode;
use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
use core_test_support::responses;
use pretty_assertions::assert_eq;
use serde_json::Value;
use serde_json::json;
use std::collections::BTreeMap;
use std::io::Write;
use std::path::Path;
use std::time::Duration;
use tempfile::TempDir;
use wiremock::Mock;
use wiremock::MockServer;
use wiremock::ResponseTemplate;
use wiremock::matchers::method;
use wiremock::matchers::path;
pub(super) async fn configure_collector(codex_home: &Path) -> Result<MockServer> {
let collector = MockServer::start().await;
Mock::given(method("POST"))
.and(path("/v1/metrics"))
.respond_with(ResponseTemplate::new(/*s*/ 200))
.mount(&collector)
.await;
let endpoint = format!("{}/v1/metrics", collector.uri());
let mut config = std::fs::OpenOptions::new()
.create(true)
.append(true)
.open(codex_home.join("config.toml"))?;
writeln!(
config,
"\n[analytics]\nenabled = true\n[otel.metrics_exporter.otlp-http]\nendpoint = {endpoint:?}\nprotocol = \"json\""
)?;
Ok(collector)
}
pub(super) async fn assert_saved_originators(
collector: &MockServer,
expected: &[&str],
) -> Result<()> {
let expected: BTreeMap<_, _> = expected
.iter()
.map(|value| (value.to_string(), 1_u64))
.collect();
let actual = tokio::time::timeout(Duration::from_secs(/*secs*/ 60), async {
loop {
let mut actual = BTreeMap::<String, u64>::new();
for request in collector.received_requests().await.unwrap_or_default() {
let body: Value = serde_json::from_slice(&request.body)?;
for resource in body["resourceMetrics"].as_array().into_iter().flatten() {
for scope in resource["scopeMetrics"].as_array().into_iter().flatten() {
for metric in scope["metrics"].as_array().into_iter().flatten() {
if metric["name"] != "codex.auth_storage.operation" {
continue;
}
for point in
metric["sum"]["dataPoints"].as_array().into_iter().flatten()
{
let tags: BTreeMap<_, _> = point["attributes"]
.as_array()
.into_iter()
.flatten()
.filter_map(|tag| {
Some((
tag["key"].as_str()?,
tag["value"]["stringValue"].as_str()?,
))
})
.collect();
if tags.get("credential_kind") == Some(&"codex")
&& tags.get("operation") == Some(&"save")
{
let count = point["asInt"]
.as_u64()
.or_else(|| point["asInt"].as_str()?.parse().ok())
.context("storage count must be an integer")?;
*actual
.entry(
tags.get("originator")
.unwrap_or(&"missing")
.to_string(),
)
.or_default() += count;
}
}
}
}
}
}
if actual.values().sum::<u64>() >= expected.len() as u64 {
break Ok::<_, anyhow::Error>(actual);
}
tokio::time::sleep(Duration::from_millis(/*millis*/ 25)).await;
}
})
.await??;
assert_eq!(actual, expected);
Ok(())
}
#[tokio::test]
async fn concurrent_connections_export_their_own_storage_originator() -> Result<()> {
let codex_home = TempDir::new()?;
std::fs::write(
codex_home.path().join("config.toml"),
"cli_auth_credentials_store = \"file\"\n",
)?;
let collector = configure_collector(codex_home.path()).await?;
let (_process, address) = spawn_websocket_server(codex_home.path()).await?;
let mut clients = Vec::new();
for name in ["Codex Desktop", "codex_vscode", "private-client-name"] {
let mut client = connect_websocket(address).await?;
send_initialize_request(&mut client, /*id*/ 1, name).await?;
read_response_for_id(&mut client, /*id*/ 1).await?;
clients.push(client);
}
// All clients initialize before any storage request; a process-global identity fails this test.
for client in &mut clients {
send_request(
client,
"account/login/start",
/*id*/ 2,
Some(json!({"type": "apiKey", "apiKey": "sk-test-key"})),
)
.await?;
}
for client in &mut clients {
let response = read_response_for_id(client, /*id*/ 2).await?;
assert_eq!(response.result, json!({"type": "apiKey"}));
}
assert_saved_originators(&collector, &["codex_desktop", "codex_vscode", "other"]).await
}
#[tokio::test]
async fn turn_token_refresh_exports_thread_storage_originator() -> Result<()> {
let codex_home = TempDir::new()?;
let backend = MockServer::start().await;
MockResponsesConfig::new(&backend.uri())
.with_root_config("cli_auth_credentials_store = \"file\"")
.with_provider_config("requires_openai_auth = true")
.write(codex_home.path())?;
write_models_cache(codex_home.path()).await?;
write_chatgpt_auth(
codex_home.path(),
ChatGptAuthFixture::new("initial-access-token")
.account_id("account-123")
.chatgpt_user_id("user-123"),
AuthCredentialsStoreMode::File,
)?;
let collector = configure_collector(codex_home.path()).await?;
Mock::given(method("POST"))
.and(path("/oauth/token"))
.respond_with(ResponseTemplate::new(/*s*/ 200).set_body_json(json!({
"access_token": "refreshed-access-token",
"refresh_token": "refreshed-refresh-token",
})))
.expect(/*r*/ 1)
.mount(&backend)
.await;
// Fresh credentials prevent startup refresh. The turn first reloads storage on 401,
// then refreshes and persists the tokens when the retry is also unauthorized.
let responses_mock = responses::mount_response_sequence(
&backend,
vec![
ResponseTemplate::new(/*s*/ 401),
ResponseTemplate::new(/*s*/ 401),
responses::sse_response(responses::sse(vec![
responses::ev_response_created("resp-turn"),
responses::ev_completed("resp-turn"),
])),
],
)
.await;
let refresh_url = format!("{}/oauth/token", backend.uri());
let mut server = TestAppServer::builder()
.with_codex_home(codex_home.path())
.with_env_overrides(&[
("OPENAI_API_KEY", None),
(
REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR,
Some(refresh_url.as_str()),
),
])
.build()
.await?;
server
.initialize_with_client_info(ClientInfo {
name: "codex_vscode".to_string(),
title: None,
version: "0.1.0".to_string(),
})
.await?;
let request = server
.send_thread_start_request_with_auto_env(ThreadStartParams::default())
.await?;
let thread: ThreadStartResponse = server.read_response(request).await?;
let request = server
.send_turn_start_request(TurnStartParams {
thread_id: thread.thread.id,
input: vec![UserInput::Text {
text: "Hello".to_string(),
text_elements: Vec::new(),
}],
..Default::default()
})
.await?;
let _: TurnStartResponse = server.read_response(request).await?;
let notification = server
.read_stream_until_notification_message("turn/completed")
.await?;
let completed: TurnCompletedNotification =
serde_json::from_value(notification.params.context("turn/completed params")?)?;
assert_eq!(completed.turn.status, TurnStatus::Completed);
assert_eq!(
responses_mock
.requests()
.iter()
.map(|request| request.header("authorization"))
.collect::<Vec<_>>(),
vec![
Some("Bearer initial-access-token".to_string()),
Some("Bearer initial-access-token".to_string()),
Some("Bearer refreshed-access-token".to_string()),
]
);
assert_saved_originators(&collector, &["codex_vscode"]).await?;
backend.verify().await;
Ok(())
}
codex-rs/app-server/tests/suite/v2/connection_handling_websocket.rs
+2 -1
View File
@@ -507,7 +507,8 @@ pub(super) async fn spawn_websocket_server_with_args(
.stdout(Stdio::null())
.stderr(Stdio::piped())
.env("CODEX_HOME", codex_home)
.env("RUST_LOG", "warn");
.env("RUST_LOG", "warn")
.env("OTEL_METRIC_EXPORT_INTERVAL", "200");
let mut process = cmd
.kill_on_drop(true)
codex-rs/app-server/tests/suite/v2/mod.rs
+2
View File
@@ -9,6 +9,8 @@ mod app_list;
mod app_read;
mod application_network;
mod attestation;
#[path = "auth_storage_originator_tests.rs"]
mod auth_storage_originator;
mod auto_env;
mod bedrock_setup;
mod client_metadata;
codex-rs/cli/src/login.rs
+7 -1
View File
@@ -46,6 +46,8 @@ const API_KEY_LOGIN_DISABLED_MESSAGE: &str =
const ACCESS_TOKEN_LOGIN_DISABLED_MESSAGE: &str =
"Access token login is disabled. Use API key login instead.";
const LOGIN_SUCCESS_MESSAGE: &str = "Successfully logged in";
const DEFAULT_LOGIN_LOG_FILTER: &str =
"codex_cli=info,codex_core=info,codex_login=info,codex_otel::auth_storage=warn";
/// Installs a small file-backed tracing layer for direct `codex login` flows.
///
@@ -94,7 +96,7 @@ fn init_login_file_logging(config: &Config) -> Option<WorkerGuard> {
let (non_blocking, guard) = non_blocking(log_file);
let env_filter = EnvFilter::try_from_default_env()
.unwrap_or_else(|_| EnvFilter::new("codex_cli=info,codex_core=info,codex_login=info"));
.unwrap_or_else(|_| EnvFilter::new(DEFAULT_LOGIN_LOG_FILTER));
let file_layer = tracing_subscriber::fmt::layer()
.with_writer(non_blocking)
.with_target(true)
@@ -619,6 +621,10 @@ fn safe_format_key(key: &str) -> String {
format!("{prefix}***{suffix}")
}
#[cfg(test)]
#[path = "login_storage_tests.rs"]
mod storage_tests;
#[cfg(test)]
mod tests {
use codex_config::types::AuthCredentialsStoreMode;
codex-rs/cli/src/login_storage_tests.rs
+87
View File
@@ -0,0 +1,87 @@
//! Verifies storage failures remain visible in direct login diagnostics.
use super::DEFAULT_LOGIN_LOG_FILTER;
use codex_config::types::AuthCredentialsStoreMode;
use codex_login::AuthKeyringBackendKind;
use codex_login::login_with_api_key;
use pretty_assertions::assert_eq;
use tempfile::tempdir;
use tracing_subscriber::EnvFilter;
#[test]
fn login_default_filter_records_sanitized_storage_fallback() -> anyhow::Result<()> {
let home = tempdir()?;
// Fail before accessing the real keyring, then allow the plaintext fallback to succeed.
std::fs::write(home.path().join("secrets"), "not a directory")?;
let log_path = home.path().join("login.log");
let subscriber = tracing_subscriber::fmt()
.without_time()
.with_ansi(false)
.with_env_filter(EnvFilter::new(DEFAULT_LOGIN_LOG_FILTER))
.with_writer(std::fs::File::create(&log_path)?)
.finish();
tracing::subscriber::with_default(subscriber, || {
login_with_api_key(
home.path(),
"private-api-key",
AuthCredentialsStoreMode::Auto,
AuthKeyringBackendKind::Secrets,
)
})?;
let logs = std::fs::read_to_string(log_path)?;
assert_eq!(logs.lines().count(), 2);
let failure = logs.lines().next().unwrap();
assert!(
failure.contains("failed to save auth to keyring, falling back to file storage"),
"{logs}"
);
assert!(failure.contains("failed to create secrets dir"), "{logs}");
assert!(
logs.contains("credential storage file fallback completed"),
"{logs}"
);
assert!(logs.contains("fallback_reason=\"secure_error\""), "{logs}");
assert!(logs.contains("outcome=\"success\""), "{logs}");
assert!(!logs.contains("private-api-key"));
let completion = logs.lines().last().unwrap();
assert!(!completion.contains(home.path().to_str().unwrap()));
let cause = std::fs::create_dir_all(home.path().join("secrets"))
.unwrap_err()
.to_string();
insta::assert_snapshot!(
logs.replace(
home.path().join("secrets").to_str().unwrap(),
"[SECRETS_PATH]"
)
.replace(&cause, "[OS_ERROR]")
);
Ok(())
}
#[test]
fn login_storage_error_displays_context_and_cause() -> anyhow::Result<()> {
let home = tempdir()?;
let secrets_path = home.path().join("secrets");
std::fs::write(&secrets_path, "not a directory")?;
let cause = std::fs::create_dir_all(&secrets_path)
.unwrap_err()
.to_string();
let error = login_with_api_key(
home.path(),
"private-api-key",
AuthCredentialsStoreMode::Keyring,
AuthKeyringBackendKind::Secrets,
)
.unwrap_err();
let message = error.to_string();
assert!(message.contains(&cause), "{message}");
assert!(!message.contains("private-api-key"));
// The temporary path and native filesystem error differ across platforms.
insta::assert_snapshot!(
message
.replace(secrets_path.to_str().unwrap(), "[SECRETS_PATH]")
.replace(&cause, "[OS_ERROR]")
);
Ok(())
}
codex-rs/cli/src/snapshots/codex_cli__login__storage_tests__login_default_filter_records_sanitized_storage_fallback.snap
+7
View File
@@ -0,0 +1,7 @@
---
source: cli/src/login_storage_tests.rs
assertion_line: 47
expression: "logs.replace(home.path().join(\"secrets\").to_str().unwrap(),\n\"[SECRETS_PATH]\").replace(&cause, \"[OS_ERROR]\")"
---
WARN codex_login::auth::storage: failed to save auth to keyring, falling back to file storage: failed to write OAuth tokens to encrypted auth storage: failed to create secrets dir [SECRETS_PATH]: [OS_ERROR]
WARN codex_otel::auth_storage: credential storage file fallback completed credential_kind="codex" store_mode="auto" selected_store="secrets" actual_store="file" operation="save" secure_outcome="error" outcome="success" fallback_reason="secure_error" secure_error="other" storage_phase="policy" originator="none"
codex-rs/cli/src/snapshots/codex_cli__login__storage_tests__login_storage_error_displays_context_and_cause.snap
+6
View File
@@ -0,0 +1,6 @@
---
source: cli/src/login_storage_tests.rs
assertion_line: 58
expression: "message.replace(secrets_path.to_str().unwrap(),\n\"[SECRETS_PATH]\").replace(&cause, \"[OS_ERROR]\")"
---
failed to write OAuth tokens to encrypted auth storage: failed to create secrets dir [SECRETS_PATH]: [OS_ERROR]
codex-rs/cli/tests/daemon_telemetry.rs
+8 -2
View File
@@ -53,8 +53,14 @@ async fn foreground_update_respects_consent_and_reports_unconfirmed() -> anyhow:
continue;
}
let body: Value = serde_json::from_slice(&requests[0].body)?;
let metric = &body["resourceMetrics"][0]["scopeMetrics"][0]["metrics"][0];
assert_eq!(metric["name"], "codex.daemon.update");
let metric = body["resourceMetrics"]
.as_array()
.into_iter()
.flatten()
.flat_map(|resource| resource["scopeMetrics"].as_array().into_iter().flatten())
.flat_map(|scope| scope["metrics"].as_array().into_iter().flatten())
.find(|metric| metric["name"] == "codex.daemon.update")
.context("daemon update metric")?;
assert!(
metric["sum"]["dataPoints"][0]["attributes"]
.as_array()
codex-rs/core/src/session/mod.rs
+4 -2
View File
@@ -91,6 +91,7 @@ use codex_models_manager::manager::SharedModelsManager;
use codex_network_proxy::NetworkProxy;
use codex_network_proxy::NetworkProxyAuditMetadata;
use codex_network_proxy::normalize_host;
use codex_otel::auth_storage::AuthStorageOriginator;
use codex_otel::current_span_trace_id;
use codex_otel::current_span_w3c_trace_context;
use codex_otel::set_parent_from_w3c_trace_context;
@@ -822,6 +823,7 @@ impl Session {
);
let service_tier =
get_service_tier(config.service_tier.clone(), fast_mode_enabled, &model_info);
let storage_originator = AuthStorageOriginator::from_client_name(&originator);
let session_configuration = SessionConfiguration {
provider: create_model_provider(
config.model_provider.clone(),
@@ -930,11 +932,11 @@ impl Session {
// This task will run until Op::Shutdown is received.
let session_for_loop = Arc::clone(&session);
let session_loop_handle = tokio::spawn(async move {
let session_loop_handle = tokio::spawn(storage_originator.scope(async move {
submission_loop(session_for_loop, configured_config, rx_sub)
.instrument(info_span!("session_loop", thread_id = %thread_id))
.await;
});
}));
let io = SessionIo {
tx_sub,
rx_event,
codex-rs/core/src/tasks/mod.rs
+40 -39
View File
@@ -46,6 +46,7 @@ use codex_otel::TURN_MEMORY_METRIC;
use codex_otel::TURN_NETWORK_PROXY_METRIC;
use codex_otel::TURN_TOOL_CALL_METRIC;
use codex_otel::TURN_UNIFIED_EXEC_RUNNING_PROCESSES_METRIC;
use codex_otel::auth_storage::AuthStorageOriginator;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::ErrorEvent;
use codex_protocol::protocol::EventMsg;
@@ -361,46 +362,46 @@ impl Session {
codex.turn.token_usage.reasoning_output_tokens = field::Empty,
codex.turn.token_usage.total_tokens = field::Empty,
);
let handle = tokio::spawn(
async move {
let ctx_for_finish = Arc::clone(&ctx);
let task_result = task_for_run
.run(
Arc::clone(&session),
ctx,
task_input,
task_cancellation_token.child_token(),
)
.instrument(trace_span!("session_task.run"))
.await;
let sess = Arc::clone(&session);
// Private reviewers save their transcript together with the terminal event.
// Errors and cancellation retain their existing save path.
if (!sess.is_private_guardian_reviewer().await
|| task_cancellation_token.is_cancelled()
|| task_result.is_err())
&& let Err(err) = sess.flush_rollout().await
{
warn!("failed to flush rollout before completing turn: {err}");
sess.send_event(
ctx_for_finish.as_ref(),
EventMsg::Warning(WarningEvent {
message: format!(
"Failed to save the conversation transcript; Codex will continue retrying. Error: {err}"
),
}),
)
.await;
}
if !task_cancellation_token.is_cancelled() {
// Finish uniformly from the spawn site so all tasks share the same lifecycle.
sess.on_task_finished(Arc::clone(&ctx_for_finish), task_result)
.await;
}
done_clone.notify_waiters();
let storage_originator = AuthStorageOriginator::from_client_name(&turn_context.originator);
let task_future = async move {
let ctx_for_finish = Arc::clone(&ctx);
let task_result = task_for_run
.run(
Arc::clone(&session),
ctx,
task_input,
task_cancellation_token.child_token(),
)
.instrument(trace_span!("session_task.run"))
.await;
let sess = Arc::clone(&session);
// Private reviewers save their transcript together with the terminal event.
// Errors and cancellation retain their existing save path.
if (!sess.is_private_guardian_reviewer().await
|| task_cancellation_token.is_cancelled()
|| task_result.is_err())
&& let Err(err) = sess.flush_rollout().await
{
warn!("failed to flush rollout before completing turn: {err}");
sess.send_event(
ctx_for_finish.as_ref(),
EventMsg::Warning(WarningEvent {
message: format!(
"Failed to save the conversation transcript; Codex will continue retrying. Error: {err}"
),
}),
)
.await;
}
.instrument(task_span),
);
if !task_cancellation_token.is_cancelled() {
// Finish uniformly from the spawn site so all tasks share the same lifecycle.
sess.on_task_finished(Arc::clone(&ctx_for_finish), task_result)
.await;
}
done_clone.notify_waiters();
}
.instrument(task_span);
let handle = tokio::spawn(storage_originator.scope(task_future));
let timer = turn_context
.session_telemetry
.start_timer(TURN_E2E_DURATION_METRIC, &[])
codex-rs/keyring-store/src/error_display_tests.rs
+51
View File
@@ -0,0 +1,51 @@
//! Ensures credential payloads cannot escape through diagnostic error formatting.
use super::*;
use keyring::credential::CredentialApi;
use pretty_assertions::assert_eq;
#[test]
fn payload_bearing_errors_redact_credentials_and_end_the_source_chain() {
let credential = keyring::mock::MockCredential::default();
credential.set_password("private-credential-value").unwrap();
for (error, message) in [
(
KeyringError::Ambiguous(vec![Box::new(credential) as Box<keyring::Credential>]),
"Entry is matched by 1 credentials",
),
(
KeyringError::BadEncoding(b"private-credential-value".to_vec()),
"Data is not UTF-8 encoded",
),
] {
let error = CredentialStoreError::new(error);
assert_eq!(
(
error.to_string(),
error.message(),
format!("{error:?}"),
error.source().is_none()
),
(
message.to_string(),
message.to_string(),
message.to_string(),
true
),
);
}
}
#[test]
fn debug_preserves_native_error_details() {
let error = CredentialStoreError::new(KeyringError::PlatformFailure(Box::new(
std::io::Error::new(
std::io::ErrorKind::PermissionDenied,
"permission denied by keyring",
),
)));
assert_eq!(
format!("{error:?}"),
"Platform secure storage failure: permission denied by keyring",
);
}
codex-rs/keyring-store/src/lib.rs
+16 -4
View File
@@ -5,7 +5,6 @@ use std::fmt;
use std::fmt::Debug;
use tracing::trace;
#[derive(Debug)]
pub enum CredentialStoreError {
Other(KeyringError),
}
@@ -16,9 +15,7 @@ impl CredentialStoreError {
}
pub fn message(&self) -> String {
match self {
Self::Other(error) => error.to_string(),
}
self.to_string()
}
pub fn into_error(self) -> KeyringError {
@@ -31,19 +28,34 @@ impl CredentialStoreError {
impl fmt::Display for CredentialStoreError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Other(KeyringError::Ambiguous(items)) => {
write!(f, "Entry is matched by {} credentials", items.len())
}
Self::Other(error) => write!(f, "{error}"),
}
}
}
impl fmt::Debug for CredentialStoreError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(self, f)
}
}
impl Error for CredentialStoreError {
fn source(&self) -> Option<&(dyn Error + 'static)> {
match self {
// Do not expose payload-bearing variants to error-chain formatters.
Self::Other(KeyringError::Ambiguous(_) | KeyringError::BadEncoding(_)) => None,
Self::Other(error) => Some(error),
}
}
}
#[cfg(test)]
#[path = "error_display_tests.rs"]
mod error_display_tests;
mod error_kind;
impl From<CredentialStoreError> for std::io::Error {
codex-rs/login/Cargo.toml
+2 -1
View File
@@ -9,6 +9,7 @@ workspace = true
[dependencies]
codex-utils-process = { workspace = true }
anyhow = { workspace = true }
base64 = { workspace = true }
chrono = { workspace = true, features = ["serde"] }
codex-agent-identity = { workspace = true }
@@ -44,7 +45,7 @@ urlencoding = { workspace = true }
webbrowser = { workspace = true }
[dev-dependencies]
anyhow = { workspace = true }
opentelemetry_sdk = { workspace = true, features = ["testing"] }
core_test_support = { workspace = true }
jsonwebtoken = { workspace = true }
keyring = { workspace = true }
codex-rs/login/src/auth/storage.rs
+149 -59
View File
@@ -1,5 +1,7 @@
use chrono::DateTime;
use chrono::Utc;
use codex_otel::auth_storage::Operation;
use codex_otel::auth_storage::Store;
use serde::Deserialize;
use serde::Serialize;
use sha2::Digest;
@@ -18,6 +20,12 @@ use std::sync::Arc;
use std::sync::Mutex;
use tracing::warn;
#[path = "storage_error.rs"]
mod storage_error;
#[path = "storage_telemetry.rs"]
mod storage_telemetry;
use super::BedrockAccessKeysAuth;
use super::BedrockApiKeyAuth;
use crate::token_data::TokenData;
@@ -251,44 +259,50 @@ fn compute_store_key(codex_home: &Path) -> std::io::Result<String> {
#[derive(Clone, Debug)]
struct DirectKeyringAuthStorage {
codex_home: PathBuf,
mode: AuthCredentialsStoreMode,
keyring_store: Arc<dyn KeyringStore>,
}
impl DirectKeyringAuthStorage {
fn new(codex_home: PathBuf, keyring_store: Arc<dyn KeyringStore>) -> Self {
fn new(
codex_home: PathBuf,
keyring_store: Arc<dyn KeyringStore>,
mode: AuthCredentialsStoreMode,
) -> Self {
Self {
codex_home,
keyring_store,
mode,
}
}
fn load_from_keyring(&self, key: &str) -> std::io::Result<Option<AuthDotJson>> {
match self.keyring_store.load(KEYRING_SERVICE, key) {
Ok(Some(serialized)) => serde_json::from_str(&serialized).map(Some).map_err(|err| {
std::io::Error::other(format!(
"failed to deserialize CLI auth from keyring: {err}"
))
match self
.keyring_store
.load(KEYRING_SERVICE, key)
.map_err(std::io::Error::from)
.map_err(|error| {
storage_error::with_context("failed to load CLI auth from keyring", error)
})? {
Some(serialized) => serde_json::from_str(&serialized).map(Some).map_err(|err| {
storage_error::with_context("failed to deserialize CLI auth from keyring", err)
}),
Ok(None) => Ok(None),
Err(error) => Err(std::io::Error::other(format!(
"failed to load CLI auth from keyring: {}",
error.message()
))),
None => Ok(None),
}
}
fn save_to_keyring(&self, key: &str, value: &str) -> std::io::Result<()> {
match self.keyring_store.save(KEYRING_SERVICE, key, value) {
Ok(()) => Ok(()),
Err(error) => {
let message = format!(
"failed to write OAuth tokens to keyring: {}",
error.message()
);
warn!("{message}");
Err(std::io::Error::other(message))
}
}
self.keyring_store
.save(KEYRING_SERVICE, key, value)
.map_err(std::io::Error::from)
.map_err(|error| {
let error =
storage_error::with_context("failed to write OAuth tokens to keyring", error);
if self.mode == AuthCredentialsStoreMode::Keyring {
warn!("{error}");
}
error
})
}
}
@@ -303,9 +317,17 @@ impl AuthStorageBackend for DirectKeyringAuthStorage {
// Simpler error mapping per style: prefer method reference over closure
let serialized = serde_json::to_string(auth).map_err(std::io::Error::other)?;
self.save_to_keyring(&key, &serialized)?;
if let Err(err) = delete_file_if_exists(&self.codex_home) {
let mut telemetry = storage_telemetry::telemetry(
self.mode,
AuthKeyringBackendKind::Direct,
Operation::Cleanup,
);
let result = delete_file_if_exists(&self.codex_home);
telemetry.record_delete_attempt(Store::File, &result);
if let Err(err) = &result {
warn!("failed to remove CLI auth fallback file: {err}");
}
drop(telemetry);
Ok(())
}
@@ -314,8 +336,9 @@ impl AuthStorageBackend for DirectKeyringAuthStorage {
let keyring_removed = self
.keyring_store
.delete(KEYRING_SERVICE, &key)
.map_err(|err| {
std::io::Error::other(format!("failed to delete auth from keyring: {err}"))
.map_err(std::io::Error::from)
.map_err(|error| {
storage_error::with_context("failed to delete auth from keyring", error)
})?;
let file_removed = delete_file_if_exists(&self.codex_home)?;
Ok(keyring_removed || file_removed)
@@ -325,6 +348,7 @@ impl AuthStorageBackend for DirectKeyringAuthStorage {
#[derive(Clone)]
struct SecretsKeyringAuthStorage {
codex_home: PathBuf,
mode: AuthCredentialsStoreMode,
direct_storage: DirectKeyringAuthStorage,
secrets_manager: SecretsManager,
}
@@ -338,9 +362,13 @@ impl Debug for SecretsKeyringAuthStorage {
}
impl SecretsKeyringAuthStorage {
fn new(codex_home: PathBuf, keyring_store: Arc<dyn KeyringStore>) -> Self {
fn new(
codex_home: PathBuf,
keyring_store: Arc<dyn KeyringStore>,
mode: AuthCredentialsStoreMode,
) -> Self {
let direct_storage =
DirectKeyringAuthStorage::new(codex_home.clone(), Arc::clone(&keyring_store));
DirectKeyringAuthStorage::new(codex_home.clone(), Arc::clone(&keyring_store), mode);
let secrets_manager = SecretsManager::new_with_keyring_store_and_namespace(
codex_home.clone(),
SecretsBackendKind::Local,
@@ -349,6 +377,7 @@ impl SecretsKeyringAuthStorage {
);
Self {
codex_home,
mode,
direct_storage,
secrets_manager,
}
@@ -360,15 +389,17 @@ impl AuthStorageBackend for SecretsKeyringAuthStorage {
match self
.secrets_manager
.get(&SecretScope::Global, &CODEX_AUTH_SECRET_NAME)
.map_err(|err| {
std::io::Error::other(format!(
"failed to load CLI auth from encrypted auth storage: {err}"
))
.map_err(|error| {
storage_error::with_context(
"failed to load CLI auth from encrypted auth storage",
error,
)
})? {
Some(serialized) => serde_json::from_str(&serialized).map(Some).map_err(|err| {
std::io::Error::other(format!(
"failed to deserialize CLI auth from encrypted auth storage: {err}"
))
storage_error::with_context(
"failed to deserialize CLI auth from encrypted auth storage",
err,
)
}),
None => Ok(None),
}
@@ -378,15 +409,27 @@ impl AuthStorageBackend for SecretsKeyringAuthStorage {
let serialized = serde_json::to_string(auth).map_err(std::io::Error::other)?;
self.secrets_manager
.set(&SecretScope::Global, &CODEX_AUTH_SECRET_NAME, &serialized)
.map_err(|err| {
let message =
format!("failed to write OAuth tokens to encrypted auth storage: {err}");
warn!("{message}");
std::io::Error::other(message)
.map_err(|error| {
let error = storage_error::with_context(
"failed to write OAuth tokens to encrypted auth storage",
error,
);
if self.mode == AuthCredentialsStoreMode::Keyring {
warn!("{error}");
}
error
})?;
if let Err(err) = delete_file_if_exists(&self.codex_home) {
let mut telemetry = storage_telemetry::telemetry(
self.mode,
AuthKeyringBackendKind::Secrets,
Operation::Cleanup,
);
let result = delete_file_if_exists(&self.codex_home);
telemetry.record_delete_attempt(Store::File, &result);
if let Err(err) = &result {
warn!("failed to remove CLI auth fallback file: {err}");
}
drop(telemetry);
Ok(())
}
@@ -394,10 +437,11 @@ impl AuthStorageBackend for SecretsKeyringAuthStorage {
let keyring_removed = self
.secrets_manager
.delete(&SecretScope::Global, &CODEX_AUTH_SECRET_NAME)
.map_err(|err| {
std::io::Error::other(format!(
"failed to delete auth from encrypted auth storage: {err}"
))
.map_err(|error| {
storage_error::with_context(
"failed to delete auth from encrypted auth storage",
error,
)
})?;
let file_removed = delete_file_if_exists(&self.codex_home)?;
let direct_removed = self.direct_storage.delete()?;
@@ -409,6 +453,7 @@ impl AuthStorageBackend for SecretsKeyringAuthStorage {
struct AutoAuthStorage {
keyring_storage: Arc<dyn AuthStorageBackend>,
file_storage: Arc<FileAuthStorage>,
keyring_backend_kind: AuthKeyringBackendKind,
}
impl AutoAuthStorage {
@@ -422,37 +467,76 @@ impl AutoAuthStorage {
codex_home.clone(),
keyring_store,
keyring_backend_kind,
AuthCredentialsStoreMode::Auto,
),
file_storage: Arc::new(FileAuthStorage::new(codex_home)),
keyring_backend_kind,
}
}
}
impl AuthStorageBackend for AutoAuthStorage {
fn load(&self) -> std::io::Result<Option<AuthDotJson>> {
match self.keyring_storage.load() {
let mut telemetry = storage_telemetry::telemetry(
AuthCredentialsStoreMode::Auto,
self.keyring_backend_kind,
Operation::Load,
);
let result = self.keyring_storage.load();
telemetry.record_load_attempt(
storage_telemetry::keyring_store(self.keyring_backend_kind),
&result,
);
match result {
Ok(Some(auth)) => Ok(Some(auth)),
Ok(None) => self.file_storage.load(),
Ok(None) => {
let result = self.file_storage.load();
telemetry.record_load_attempt(Store::File, &result);
result
}
Err(err) => {
warn!("failed to load CLI auth from keyring, falling back to file storage: {err}");
self.file_storage.load()
telemetry.record_secure_error(&err);
let result = self.file_storage.load();
telemetry.record_load_attempt(Store::File, &result);
result
}
}
}
fn save(&self, auth: &AuthDotJson) -> std::io::Result<()> {
match self.keyring_storage.save(auth) {
let mut telemetry = storage_telemetry::telemetry(
AuthCredentialsStoreMode::Auto,
self.keyring_backend_kind,
Operation::Save,
);
let result = self.keyring_storage.save(auth);
telemetry.record_save_attempt(
storage_telemetry::keyring_store(self.keyring_backend_kind),
&result,
);
match result {
Ok(()) => Ok(()),
Err(err) => {
warn!("failed to save auth to keyring, falling back to file storage: {err}");
self.file_storage.save(auth)
telemetry.record_secure_error(&err);
let result = self.file_storage.save(auth);
telemetry.record_save_attempt(Store::File, &result);
result
}
}
}
fn delete(&self) -> std::io::Result<bool> {
// Keyring storage will delete from disk as well
self.keyring_storage.delete()
let mut telemetry = storage_telemetry::telemetry(
AuthCredentialsStoreMode::Auto,
self.keyring_backend_kind,
Operation::Delete,
);
let result = self.keyring_storage.delete();
telemetry.record_delete_attempt(Store::Multiple, &result);
result
}
}
@@ -514,10 +598,10 @@ fn create_auth_storage_with_store(
keyring_store: Arc<dyn KeyringStore>,
keyring_backend_kind: AuthKeyringBackendKind,
) -> Arc<dyn AuthStorageBackend> {
match mode {
let storage: Arc<dyn AuthStorageBackend> = match mode {
AuthCredentialsStoreMode::File => Arc::new(FileAuthStorage::new(codex_home)),
AuthCredentialsStoreMode::Keyring => {
create_keyring_auth_storage(codex_home, keyring_store, keyring_backend_kind)
create_keyring_auth_storage(codex_home, keyring_store, keyring_backend_kind, mode)
}
AuthCredentialsStoreMode::Auto => Arc::new(AutoAuthStorage::new(
codex_home,
@@ -525,21 +609,27 @@ fn create_auth_storage_with_store(
keyring_backend_kind,
)),
AuthCredentialsStoreMode::Ephemeral => Arc::new(EphemeralAuthStorage::new(codex_home)),
}
};
storage_telemetry::observe(storage, mode, keyring_backend_kind)
}
fn create_keyring_auth_storage(
codex_home: PathBuf,
keyring_store: Arc<dyn KeyringStore>,
keyring_backend_kind: AuthKeyringBackendKind,
mode: AuthCredentialsStoreMode,
) -> Arc<dyn AuthStorageBackend> {
match keyring_backend_kind {
AuthKeyringBackendKind::Direct => {
Arc::new(DirectKeyringAuthStorage::new(codex_home, keyring_store))
}
AuthKeyringBackendKind::Secrets => {
Arc::new(SecretsKeyringAuthStorage::new(codex_home, keyring_store))
}
AuthKeyringBackendKind::Direct => Arc::new(DirectKeyringAuthStorage::new(
codex_home,
keyring_store,
mode,
)),
AuthKeyringBackendKind::Secrets => Arc::new(SecretsKeyringAuthStorage::new(
codex_home,
keyring_store,
mode,
)),
}
}
codex-rs/login/src/auth/storage_error.rs
+49
View File
@@ -0,0 +1,49 @@
//! Removes credential JSON data errors while preserving native storage diagnostics and typed causes.
#[derive(thiserror::Error)]
struct CredentialStorageError {
context: &'static str,
#[source]
source: anyhow::Error,
}
impl std::fmt::Display for CredentialStorageError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}: {:#}", self.context, self.source)
}
}
impl std::fmt::Debug for CredentialStorageError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Display::fmt(self, f)
}
}
pub(super) fn with_context(
context: &'static str,
source: impl Into<anyhow::Error>,
) -> std::io::Error {
let mut source = source.into();
let mut cause: Option<&(dyn std::error::Error + 'static)> = Some(source.as_ref());
while let Some(error) = cause {
if let Some(error) = error.downcast_ref::<serde_json::Error>()
&& error.is_data()
{
// Discard the entire chain: serde errors and their context may contain credentials.
source = anyhow::anyhow!(
"invalid credential JSON ({:?} at line {} column {})",
error.classify(),
error.line(),
error.column(),
);
break;
}
cause = if let Some(error) = error.downcast_ref::<std::io::Error>() {
// io::Error::source() skips its contained error.
error.get_ref().map(|error| error as &dyn std::error::Error)
} else {
error.source()
};
}
std::io::Error::other(CredentialStorageError { context, source })
}
codex-rs/login/src/auth/storage_error_tests.rs
+146
View File
@@ -0,0 +1,146 @@
//! Checks that storage diagnostics preserve useful causes without exposing credential values.
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn secure_storage_errors_display_the_underlying_cause() -> anyhow::Result<()> {
for kind in [
AuthKeyringBackendKind::Direct,
AuthKeyringBackendKind::Secrets,
] {
for (error_kind, cause) in [
(
std::io::ErrorKind::PermissionDenied,
"permission denied by keyring",
),
(std::io::ErrorKind::NotConnected, "keyring unavailable"),
] {
let home = tempdir()?;
let keyring = MockKeyringStore::default();
let storage = create_auth_storage_with_store(
home.path().to_path_buf(),
AuthCredentialsStoreMode::Keyring,
Arc::new(keyring.clone()),
kind,
);
let auth = auth_with_prefix("private credential value");
storage.save(&auth)?;
let key = match kind {
AuthKeyringBackendKind::Direct => compute_store_key(home.path())?,
AuthKeyringBackendKind::Secrets => {
compute_keyring_account(home.path(), LocalSecretsNamespace::CodexAuth)
}
};
for operation in [Operation::Load, Operation::Save, Operation::Delete] {
keyring.set_error(
&key,
KeyringError::PlatformFailure(Box::new(std::io::Error::new(error_kind, cause))),
);
let error = match operation {
Operation::Load => storage.load().unwrap_err(),
Operation::Save => storage.save(&auth).unwrap_err(),
Operation::Delete => storage.delete().unwrap_err(),
Operation::Cleanup | Operation::RefreshPersist => unreachable!(),
};
let message = error.to_string();
assert!(message.starts_with("failed to "), "{message}");
assert!(message.contains(cause), "{message}");
assert!(!message.contains("private credential value"));
let debug = format!("{error:?}");
assert!(debug.contains(cause), "{debug}");
assert!(!debug.contains("private credential value"));
}
}
}
Ok(())
}
#[test]
fn json_syntax_and_eof_errors_preserve_diagnostics() {
for (input, category) in [
(
r#"{"OPENAI_API_KEY" "private-credential-value"}"#,
serde_json::error::Category::Syntax,
),
(
r#"{"OPENAI_API_KEY":"private-credential-value""#,
serde_json::error::Category::Eof,
),
] {
let source = serde_json::from_str::<AuthDotJson>(input).unwrap_err();
assert_eq!(source.classify(), category);
let expected = format!("failed to load auth: decrypted auth JSON: {source}");
let error = storage_error::with_context(
"failed to load auth",
anyhow::Error::new(source).context("decrypted auth JSON"),
);
let inner = error.get_ref().unwrap();
assert_eq!(
(error.to_string(), format!("{inner:?}")),
(expected.clone(), expected)
);
assert!(!format!("{error:?}").contains("private-credential-value"));
}
}
#[test]
fn json_data_errors_redact_credential_values() {
enum Wrapper {
Direct,
AnyhowContext,
Io,
Nested,
}
for wrapper in [
Wrapper::Direct,
Wrapper::AnyhowContext,
Wrapper::Io,
Wrapper::Nested,
] {
let source =
serde_json::from_str::<AuthDotJson>(r#"{"auth_mode":"private-credential-value"}"#)
.unwrap_err();
assert!(source.to_string().contains("private-credential-value"));
let expected = format!(
"failed to load auth: invalid credential JSON (Data at line {} column {})",
source.line(),
source.column(),
);
let source = match wrapper {
Wrapper::Direct => anyhow::Error::new(source),
Wrapper::AnyhowContext => anyhow::Error::new(source).context("decrypted auth JSON"),
Wrapper::Io => anyhow::Error::new(std::io::Error::other(source)),
Wrapper::Nested => anyhow::Error::new(std::io::Error::other(
anyhow::Error::new(std::io::Error::other(source))
.context("private-credential-value"),
))
.context("decrypted auth JSON"),
};
let error = storage_error::with_context("failed to load auth", source);
let inner = error.get_ref().unwrap();
assert_eq!(
(error.to_string(), format!("{inner:?}")),
(expected.clone(), expected)
);
assert!(!format!("{error:?}").contains("private-credential-value"));
let error = anyhow::Error::new(error).context("credential storage failed");
for formatted in [format!("{error:#}"), format!("{error:?}")] {
assert!(
!formatted.contains("private-credential-value"),
"{formatted}"
);
}
for cause in error.chain() {
assert!(cause.downcast_ref::<serde_json::Error>().is_none());
for formatted in [cause.to_string(), format!("{cause:?}")] {
assert!(
!formatted.contains("private-credential-value"),
"{formatted}"
);
}
}
}
}
codex-rs/login/src/auth/storage_policy_tests.rs
+443
View File
@@ -0,0 +1,443 @@
//! Exercises configured storage policy and its emitted metadata.
use super::*;
use opentelemetry_sdk::metrics::data::AggregatedMetrics;
use opentelemetry_sdk::metrics::data::MetricData;
use pretty_assertions::assert_eq;
#[test]
fn storage_metrics_distinguish_explicit_file_from_secure_failure_fallback() -> anyhow::Result<()> {
if std::env::var_os("CODEX_AUTH_STORAGE_TEST_CHILD").is_none() {
let output = std::process::Command::new(std::env::current_exe()?)
.args([
"--exact",
std::thread::current().name().unwrap(),
"--nocapture",
])
.env("CODEX_AUTH_STORAGE_TEST_CHILD", "1")
.output()?;
assert!(
output.status.success(),
"{}{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
return Ok(());
}
use std::collections::BTreeMap;
let metrics = codex_otel::install_global_metrics(codex_otel::MetricsClient::new(
codex_otel::MetricsConfig::in_memory("test", "test", "1", Default::default())
.with_runtime_reader(),
)?);
let mut expected = BTreeMap::new();
for kind in [
AuthKeyringBackendKind::Direct,
AuthKeyringBackendKind::Secrets,
] {
for (scenario, mode, operation, actual, secure, outcome, reason, error) in [
(
"success",
AuthCredentialsStoreMode::Auto,
"save",
storage_telemetry::keyring_store(kind).as_ref(),
"success",
"success",
"none",
"none",
),
(
"cleanup_failed",
AuthCredentialsStoreMode::Auto,
"save",
storage_telemetry::keyring_store(kind).as_ref(),
"success",
"success",
"none",
"none",
),
(
"success",
AuthCredentialsStoreMode::Keyring,
"save",
storage_telemetry::keyring_store(kind).as_ref(),
"success",
"success",
"none",
"none",
),
(
"cleanup_failed",
AuthCredentialsStoreMode::Keyring,
"save",
storage_telemetry::keyring_store(kind).as_ref(),
"success",
"success",
"none",
"none",
),
(
"denied",
AuthCredentialsStoreMode::Auto,
"save",
"file",
"error",
"success",
"secure_error",
"access_denied",
),
(
"explicit",
AuthCredentialsStoreMode::File,
"save",
"file",
"not_attempted",
"success",
"none",
"none",
),
(
"missing",
AuthCredentialsStoreMode::Auto,
"load",
"file",
"not_found",
"success",
"secure_entry_missing",
"none",
),
(
"failed_file",
AuthCredentialsStoreMode::Auto,
"load",
"file",
"error",
"error",
"secure_error",
"access_denied",
),
] {
let home = tempdir()?;
let keyring = MockKeyringStore::default();
let key = match kind {
AuthKeyringBackendKind::Direct => compute_store_key(home.path())?,
AuthKeyringBackendKind::Secrets => {
compute_keyring_account(home.path(), LocalSecretsNamespace::CodexAuth)
}
};
let auth = auth_with_prefix("private credential value");
if operation == "load" || scenario == "success" {
FileAuthStorage::new(home.path().to_path_buf()).save(&auth)?;
}
if scenario == "cleanup_failed" {
std::fs::create_dir(get_auth_file(home.path()))?;
}
if scenario == "failed_file" {
std::fs::write(get_auth_file(home.path()), "invalid private file contents")?;
}
if error == "access_denied" {
keyring.set_error(
&key,
KeyringError::PlatformFailure(Box::new(std::io::Error::new(
std::io::ErrorKind::PermissionDenied,
"private account path error",
))),
);
// Secrets loads consult the keyring only when an encrypted file exists.
if operation == "load" && kind == AuthKeyringBackendKind::Secrets {
let manager = SecretsManager::new_with_keyring_store_and_namespace(
home.path().to_path_buf(),
SecretsBackendKind::Local,
Arc::new(MockKeyringStore::default()),
LocalSecretsNamespace::CodexAuth,
);
manager.set(&SecretScope::Global, &CODEX_AUTH_SECRET_NAME, "{}")?;
}
}
let storage = create_auth_storage_with_store(
home.path().to_path_buf(),
mode,
Arc::new(keyring),
kind,
);
if operation == "load" {
let result = storage.load();
if outcome == "error" {
assert!(result.is_err());
} else {
assert_eq!(result?, Some(auth.clone()));
}
} else {
storage.save(&auth)?;
}
let store_mode = match mode {
AuthCredentialsStoreMode::Auto => "auto",
AuthCredentialsStoreMode::Keyring => "keyring",
AuthCredentialsStoreMode::File => "file",
AuthCredentialsStoreMode::Ephemeral => "ephemeral",
};
let tags: BTreeMap<String, String> = BTreeMap::from([
("credential_kind".into(), "codex".into()),
("store_mode".into(), store_mode.into()),
(
"selected_store".into(),
storage_telemetry::keyring_store(kind).as_ref().into(),
),
("actual_store".into(), actual.into()),
("operation".into(), operation.into()),
("secure_outcome".into(), secure.into()),
("outcome".into(), outcome.into()),
("fallback_reason".into(), reason.into()),
("secure_error".into(), error.into()),
("storage_phase".into(), "policy".into()),
("originator".into(), "none".into()),
]);
if secure == "success" {
let mut cleanup_tags = tags.clone();
cleanup_tags.extend([
("actual_store".into(), "file".into()),
("operation".into(), "cleanup".into()),
("secure_outcome".into(), "not_attempted".into()),
(
"outcome".into(),
if scenario == "cleanup_failed" {
"error"
} else {
"success"
}
.into(),
),
]);
*expected.entry(cleanup_tags).or_insert(0) += 1;
}
*expected.entry(tags).or_insert(0) += 1;
}
}
let snapshot = metrics.snapshot()?;
let mut actual = Vec::new();
for metric in snapshot
.scope_metrics()
.flat_map(opentelemetry_sdk::metrics::data::ScopeMetrics::metrics)
{
if metric.name() == "codex.auth_storage.operation"
&& let AggregatedMetrics::U64(MetricData::Sum(sum)) = metric.data()
{
for point in sum.data_points() {
let tags: BTreeMap<String, String> = point
.attributes()
.map(|a| (a.key.to_string(), a.value.to_string()))
.collect();
actual.push((point.value(), tags));
}
}
}
let mut expected: Vec<_> = expected
.into_iter()
.map(|(tags, count)| (count, tags))
.collect();
actual.sort();
expected.sort();
assert_eq!(actual, expected);
metrics.shutdown()?;
Ok(())
}
#[test]
fn auto_auth_storage_load_prefers_keyring_value() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthKeyringBackendKind::Secrets,
);
let keyring_auth = auth_with_prefix("keyring");
seed_secrets_backend_with_auth(&mock_keyring, codex_home.path(), &keyring_auth)?;
let file_auth = auth_with_prefix("file");
storage.file_storage.save(&file_auth)?;
let loaded = storage.load()?;
assert_eq!(loaded, Some(keyring_auth));
Ok(())
}
#[test]
fn auto_auth_storage_load_uses_file_when_keyring_empty() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring),
AuthKeyringBackendKind::Secrets,
);
let expected = auth_with_prefix("file-only");
storage.file_storage.save(&expected)?;
let loaded = storage.load()?;
assert_eq!(loaded, Some(expected));
Ok(())
}
#[test]
fn auto_auth_storage_load_falls_back_when_keyring_errors() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthKeyringBackendKind::Secrets,
);
let key = compute_keyring_account(codex_home.path(), LocalSecretsNamespace::CodexAuth);
let encrypted = auth_with_prefix("encrypted");
seed_secrets_backend_with_auth(&mock_keyring, codex_home.path(), &encrypted)?;
mock_keyring.set_error(&key, KeyringError::Invalid("error".into(), "load".into()));
let expected = auth_with_prefix("fallback");
storage.file_storage.save(&expected)?;
let loaded = storage.load()?;
assert_eq!(loaded, Some(expected));
Ok(())
}
#[test]
fn auto_auth_storage_save_prefers_keyring() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthKeyringBackendKind::Secrets,
);
let stale = auth_with_prefix("stale");
storage.file_storage.save(&stale)?;
let expected = auth_with_prefix("to-save");
storage.save(&expected)?;
assert_keyring_saved_auth_and_removed_fallback(&mock_keyring, codex_home.path(), &expected)?;
Ok(())
}
#[test]
fn auto_auth_storage_save_falls_back_when_keyring_errors() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthKeyringBackendKind::Secrets,
);
let key = compute_keyring_account(codex_home.path(), LocalSecretsNamespace::CodexAuth);
mock_keyring.set_error(&key, KeyringError::Invalid("error".into(), "save".into()));
let auth = auth_with_prefix("fallback");
storage.save(&auth)?;
let auth_file = get_auth_file(codex_home.path());
assert!(
auth_file.exists(),
"fallback auth.json should be created when keyring save fails"
);
let saved = storage
.file_storage
.load()?
.context("fallback auth should exist")?;
assert_eq!(saved, auth);
assert!(
mock_keyring.saved_value(&key).is_none(),
"keyring should not contain value when save fails"
);
Ok(())
}
#[test]
fn auto_auth_storage_delete_removes_keyring_and_file() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthKeyringBackendKind::Secrets,
);
let auth = auth_with_prefix("to-delete");
let auth_file = seed_secrets_backend_and_fallback_auth_file_for_delete(
&mock_keyring,
codex_home.path(),
&auth,
)?;
let removed = storage.delete()?;
assert!(removed, "delete should report removal");
assert_eq!(storage.load()?, None, "encrypted auth should be removed");
assert!(
!auth_file.exists(),
"fallback auth.json should be removed after delete"
);
Ok(())
}
#[test]
fn direct_keyring_auth_storage_saves_legacy_keyring_entry() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = DirectKeyringAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthCredentialsStoreMode::Keyring,
);
let auth_file = get_auth_file(codex_home.path());
std::fs::write(&auth_file, "stale")?;
let auth = auth_with_prefix("direct");
storage.save(&auth)?;
let legacy_key = compute_store_key(codex_home.path())?;
let saved_value = mock_keyring
.saved_value(&legacy_key)
.context("direct keyring auth entry should exist")?;
assert_eq!(saved_value, serde_json::to_string(&auth)?);
assert!(!encrypted_auth_file(codex_home.path()).exists());
assert!(
!auth_file.exists(),
"fallback auth.json should be removed after keyring save"
);
assert_eq!(storage.load()?, Some(auth));
Ok(())
}
#[test]
fn secrets_keyring_auth_storage_save_persists_and_removes_fallback_file() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = SecretsKeyringAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthCredentialsStoreMode::Keyring,
);
let auth_file = get_auth_file(codex_home.path());
std::fs::write(&auth_file, "stale")?;
let auth = AuthDotJson {
auth_mode: Some(AuthMode::Chatgpt),
openai_api_key: None,
tokens: Some(TokenData {
id_token: Default::default(),
access_token: "access".to_string(),
refresh_token: "refresh".to_string(),
account_id: Some("account".to_string()),
}),
last_refresh: Some(Utc::now()),
agent_identity: None,
personal_access_token: None,
bedrock_api_key: None,
bedrock_access_keys: None,
};
storage.save(&auth)?;
assert_keyring_saved_auth_and_removed_fallback(&mock_keyring, codex_home.path(), &auth)?;
Ok(())
}
codex-rs/login/src/auth/storage_telemetry.rs
+108
View File
@@ -0,0 +1,108 @@
//! Observes logical Codex credential operations without inspecting authentication material.
use super::AuthDotJson;
use super::AuthStorageBackend;
use codex_config::types::AuthCredentialsStoreMode;
use codex_config::types::AuthKeyringBackendKind;
use codex_otel::auth_storage::AuthStorageOriginator;
use codex_otel::auth_storage::CredentialKind;
use codex_otel::auth_storage::Operation;
use codex_otel::auth_storage::StorageTelemetry;
use codex_otel::auth_storage::Store;
use codex_otel::auth_storage::StoreMode;
use std::sync::Arc;
pub(super) fn keyring_store(kind: AuthKeyringBackendKind) -> Store {
match kind {
AuthKeyringBackendKind::Direct => Store::DirectKeyring,
AuthKeyringBackendKind::Secrets => Store::Secrets,
}
}
pub(super) fn telemetry(
mode: AuthCredentialsStoreMode,
kind: AuthKeyringBackendKind,
operation: Operation,
) -> StorageTelemetry {
let mode = match mode {
AuthCredentialsStoreMode::File => StoreMode::File,
AuthCredentialsStoreMode::Auto => StoreMode::Auto,
AuthCredentialsStoreMode::Keyring => StoreMode::Keyring,
AuthCredentialsStoreMode::Ephemeral => StoreMode::Ephemeral,
};
StorageTelemetry::new(
CredentialKind::Codex,
mode,
keyring_store(kind),
operation,
AuthStorageOriginator::current(),
)
}
pub(super) fn observe(
inner: Arc<dyn AuthStorageBackend>,
mode: AuthCredentialsStoreMode,
kind: AuthKeyringBackendKind,
) -> Arc<dyn AuthStorageBackend> {
// Auto records both attempts together so file fallback cannot hide a secure-store failure.
if mode == AuthCredentialsStoreMode::Auto {
return inner;
}
Arc::new(ObservedStorage { inner, mode, kind })
}
#[derive(Debug)]
struct ObservedStorage {
inner: Arc<dyn AuthStorageBackend>,
mode: AuthCredentialsStoreMode,
kind: AuthKeyringBackendKind,
}
impl ObservedStorage {
fn store(&self) -> Store {
match self.mode {
AuthCredentialsStoreMode::File => Store::File,
AuthCredentialsStoreMode::Ephemeral => Store::Ephemeral,
AuthCredentialsStoreMode::Keyring | AuthCredentialsStoreMode::Auto => {
keyring_store(self.kind)
}
}
}
}
impl AuthStorageBackend for ObservedStorage {
fn load(&self) -> std::io::Result<Option<AuthDotJson>> {
let mut telemetry = telemetry(self.mode, self.kind, Operation::Load);
let result = self.inner.load();
telemetry.record_load_attempt(self.store(), &result);
if self.mode == AuthCredentialsStoreMode::Keyring
&& let Err(error) = &result
{
telemetry.record_secure_error(error);
}
result
}
fn save(&self, auth: &AuthDotJson) -> std::io::Result<()> {
let mut telemetry = telemetry(self.mode, self.kind, Operation::Save);
let result = self.inner.save(auth);
telemetry.record_save_attempt(self.store(), &result);
if self.mode == AuthCredentialsStoreMode::Keyring
&& let Err(error) = &result
{
telemetry.record_secure_error(error);
}
result
}
fn delete(&self) -> std::io::Result<bool> {
let store = match self.mode {
AuthCredentialsStoreMode::File | AuthCredentialsStoreMode::Ephemeral => self.store(),
AuthCredentialsStoreMode::Auto | AuthCredentialsStoreMode::Keyring => Store::Multiple,
};
let mut telemetry = telemetry(self.mode, self.kind, Operation::Delete);
let result = self.inner.delete();
telemetry.record_delete_attempt(store, &result);
result
}
}
codex-rs/login/src/auth/storage_tests.rs
+9 -197
View File
@@ -458,6 +458,7 @@ fn secrets_keyring_auth_storage_load_returns_deserialized_auth() -> anyhow::Resu
let storage = SecretsKeyringAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthCredentialsStoreMode::Keyring,
);
let expected = AuthDotJson {
auth_mode: Some(AuthMode::ApiKey),
@@ -486,34 +487,6 @@ fn keyring_auth_storage_compute_store_key_for_home_directory() -> anyhow::Result
Ok(())
}
#[test]
fn direct_keyring_auth_storage_saves_legacy_keyring_entry() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = DirectKeyringAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
);
let auth_file = get_auth_file(codex_home.path());
std::fs::write(&auth_file, "stale")?;
let auth = auth_with_prefix("direct");
storage.save(&auth)?;
let legacy_key = compute_store_key(codex_home.path())?;
let saved_value = mock_keyring
.saved_value(&legacy_key)
.context("direct keyring auth entry should exist")?;
assert_eq!(saved_value, serde_json::to_string(&auth)?);
assert!(!encrypted_auth_file(codex_home.path()).exists());
assert!(
!auth_file.exists(),
"fallback auth.json should be removed after keyring save"
);
assert_eq!(storage.load()?, Some(auth));
Ok(())
}
#[test]
fn direct_keyring_auth_storage_delete_removes_keyring_and_file() -> anyhow::Result<()> {
let codex_home = tempdir()?;
@@ -521,6 +494,7 @@ fn direct_keyring_auth_storage_delete_removes_keyring_and_file() -> anyhow::Resu
let storage = DirectKeyringAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthCredentialsStoreMode::Keyring,
);
let auth = auth_with_prefix("direct-delete");
storage.save(&auth)?;
@@ -586,38 +560,6 @@ fn factory_uses_secrets_backend_only_when_requested() -> anyhow::Result<()> {
Ok(())
}
#[test]
fn secrets_keyring_auth_storage_save_persists_and_removes_fallback_file() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = SecretsKeyringAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
);
let auth_file = get_auth_file(codex_home.path());
std::fs::write(&auth_file, "stale")?;
let auth = AuthDotJson {
auth_mode: Some(AuthMode::Chatgpt),
openai_api_key: None,
tokens: Some(TokenData {
id_token: Default::default(),
access_token: "access".to_string(),
refresh_token: "refresh".to_string(),
account_id: Some("account".to_string()),
}),
last_refresh: Some(Utc::now()),
agent_identity: None,
personal_access_token: None,
bedrock_api_key: None,
bedrock_access_keys: None,
};
storage.save(&auth)?;
assert_keyring_saved_auth_and_removed_fallback(&mock_keyring, codex_home.path(), &auth)?;
Ok(())
}
#[test]
fn secrets_keyring_auth_storage_delete_removes_keyring_and_file() -> anyhow::Result<()> {
let codex_home = tempdir()?;
@@ -625,6 +567,7 @@ fn secrets_keyring_auth_storage_delete_removes_keyring_and_file() -> anyhow::Res
let storage = SecretsKeyringAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthCredentialsStoreMode::Keyring,
);
let auth = auth_with_prefix("to-delete");
let auth_file = seed_secrets_backend_and_fallback_auth_file_for_delete(
@@ -651,11 +594,13 @@ fn secrets_keyring_auth_storage_delete_removes_legacy_direct_keyring_entry() ->
let direct_storage = DirectKeyringAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthCredentialsStoreMode::Keyring,
);
direct_storage.save(&auth_with_prefix("legacy-direct"))?;
let storage = SecretsKeyringAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthCredentialsStoreMode::Keyring,
);
let auth = auth_with_prefix("to-delete");
let auth_file = seed_secrets_backend_and_fallback_auth_file_for_delete(
@@ -680,141 +625,8 @@ fn secrets_keyring_auth_storage_delete_removes_legacy_direct_keyring_entry() ->
Ok(())
}
#[test]
fn auto_auth_storage_load_prefers_keyring_value() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthKeyringBackendKind::Secrets,
);
let keyring_auth = auth_with_prefix("keyring");
seed_secrets_backend_with_auth(&mock_keyring, codex_home.path(), &keyring_auth)?;
#[path = "storage_policy_tests.rs"]
mod policy;
let file_auth = auth_with_prefix("file");
storage.file_storage.save(&file_auth)?;
let loaded = storage.load()?;
assert_eq!(loaded, Some(keyring_auth));
Ok(())
}
#[test]
fn auto_auth_storage_load_uses_file_when_keyring_empty() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring),
AuthKeyringBackendKind::Secrets,
);
let expected = auth_with_prefix("file-only");
storage.file_storage.save(&expected)?;
let loaded = storage.load()?;
assert_eq!(loaded, Some(expected));
Ok(())
}
#[test]
fn auto_auth_storage_load_falls_back_when_keyring_errors() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthKeyringBackendKind::Secrets,
);
let key = compute_keyring_account(codex_home.path(), LocalSecretsNamespace::CodexAuth);
let encrypted = auth_with_prefix("encrypted");
seed_secrets_backend_with_auth(&mock_keyring, codex_home.path(), &encrypted)?;
mock_keyring.set_error(&key, KeyringError::Invalid("error".into(), "load".into()));
let expected = auth_with_prefix("fallback");
storage.file_storage.save(&expected)?;
let loaded = storage.load()?;
assert_eq!(loaded, Some(expected));
Ok(())
}
#[test]
fn auto_auth_storage_save_prefers_keyring() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthKeyringBackendKind::Secrets,
);
let stale = auth_with_prefix("stale");
storage.file_storage.save(&stale)?;
let expected = auth_with_prefix("to-save");
storage.save(&expected)?;
assert_keyring_saved_auth_and_removed_fallback(&mock_keyring, codex_home.path(), &expected)?;
Ok(())
}
#[test]
fn auto_auth_storage_save_falls_back_when_keyring_errors() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthKeyringBackendKind::Secrets,
);
let key = compute_keyring_account(codex_home.path(), LocalSecretsNamespace::CodexAuth);
mock_keyring.set_error(&key, KeyringError::Invalid("error".into(), "save".into()));
let auth = auth_with_prefix("fallback");
storage.save(&auth)?;
let auth_file = get_auth_file(codex_home.path());
assert!(
auth_file.exists(),
"fallback auth.json should be created when keyring save fails"
);
let saved = storage
.file_storage
.load()?
.context("fallback auth should exist")?;
assert_eq!(saved, auth);
assert!(
mock_keyring.saved_value(&key).is_none(),
"keyring should not contain value when save fails"
);
Ok(())
}
#[test]
fn auto_auth_storage_delete_removes_keyring_and_file() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = AutoAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
AuthKeyringBackendKind::Secrets,
);
let auth = auth_with_prefix("to-delete");
let auth_file = seed_secrets_backend_and_fallback_auth_file_for_delete(
&mock_keyring,
codex_home.path(),
&auth,
)?;
let removed = storage.delete()?;
assert!(removed, "delete should report removal");
assert_eq!(storage.load()?, None, "encrypted auth should be removed");
assert!(
!auth_file.exists(),
"fallback auth.json should be removed after delete"
);
Ok(())
}
#[path = "storage_error_tests.rs"]
mod errors;
codex-rs/login/src/server.rs
+44 -34
View File
@@ -61,6 +61,7 @@ use codex_http_client::HttpClient;
use codex_http_client::HttpClientBuilder;
use codex_http_client::HttpClientFactory;
use codex_http_client::OutboundProxyPolicy;
use codex_otel::auth_storage::AuthStorageOriginator;
use codex_protocol::auth::AuthMode;
use codex_utils_template::Template;
use serde_json::Value as JsonValue;
@@ -226,7 +227,7 @@ pub fn run_login_server(opts: ServerOptions) -> io::Result<LoginServer> {
let server_handle = {
let shutdown_notify = shutdown_notify.clone();
let server = server;
tokio::spawn(async move {
let task = async move {
let mut callback_result = LoginCallbackResult::default();
let result = loop {
tokio::select! {
@@ -311,7 +312,8 @@ pub fn run_login_server(opts: ServerOptions) -> io::Result<LoginServer> {
// running `server.recv()` in a loop exits cleanly.
server.unblock();
result
})
};
tokio::spawn(AuthStorageOriginator::current().scope(task))
};
Ok(LoginServer {
@@ -839,38 +841,46 @@ pub(crate) async fn persist_tokens_async(
) -> io::Result<()> {
// Reuse existing synchronous logic but run it off the async runtime.
let codex_home = codex_home.to_path_buf();
tokio::task::spawn_blocking(move || {
let mut tokens = TokenData {
id_token: parse_chatgpt_jwt_claims(&id_token).map_err(io::Error::other)?,
access_token,
refresh_token,
account_id: None,
};
if let Some(acc) = jwt_auth_claims(&id_token)
.get("chatgpt_account_id")
.and_then(|v| v.as_str())
{
tokens.account_id = Some(acc.to_string());
}
let auth = AuthDotJson {
auth_mode: Some(AuthMode::Chatgpt),
openai_api_key: api_key,
tokens: Some(tokens),
last_refresh: Some(Utc::now()),
agent_identity: None,
personal_access_token: None,
bedrock_api_key: None,
bedrock_access_keys: None,
};
save_auth(
&codex_home,
&auth,
auth_credentials_store_mode,
keyring_backend_kind,
)
})
.await
.map_err(|e| io::Error::other(format!("persist task failed: {e}")))?
// Blocking tasks don't inherit task-local context.
// Preserve the initiating client for credential-storage metrics.
let originator = AuthStorageOriginator::current();
let persist_task = tokio::task::spawn_blocking(move || {
originator.sync_scope(|| {
let mut tokens = TokenData {
id_token: parse_chatgpt_jwt_claims(&id_token).map_err(io::Error::other)?,
access_token,
refresh_token,
account_id: None,
};
if let Some(acc) = jwt_auth_claims(&id_token)
.get("chatgpt_account_id")
.and_then(|v| v.as_str())
{
tokens.account_id = Some(acc.to_string());
}
let auth = AuthDotJson {
auth_mode: Some(AuthMode::Chatgpt),
openai_api_key: api_key,
tokens: Some(tokens),
last_refresh: Some(Utc::now()),
agent_identity: None,
personal_access_token: None,
bedrock_api_key: None,
bedrock_access_keys: None,
};
save_auth(
&codex_home,
&auth,
auth_credentials_store_mode,
keyring_backend_kind,
)
})
});
persist_task
.await
.map_err(|e| io::Error::other(format!("persist task failed: {e}")))?
}
/// Validates the ID token against an optional workspace restriction.