Add bounded credential-storage telemetry helpers (#47901)

## What changed

- Add `StorageTelemetry` to record operation counts, durations, secure-store outcomes, and file fallback reasons using bounded labels. Emit fallback warnings without credential payloads or error text.
- Add task-local client attribution with explicit propagation across task boundaries.
- Preserve typed keyring error causes through `std::io::Error` and the local secrets backend, classifying supported native failures without parsing error messages.
- Remove service names, account names, value lengths, and error text from keyring trace logs.

## Testing

Add tests for buffered metric emission, fallback outcomes, exclusion of sensitive error contents, client attribution isolation, and typed native error classification.

GitOrigin-RevId: 18e7201566a3b09061a6014fd6bd90eae96e3fc2

codex-rs/Cargo.lock

@@ -3706,6 +3706,9 @@ name = "codex-keyring-store"
 version = "0.0.0"
 dependencies = [
  "keyring",
+ "pretty_assertions",
+ "secret-service",
+ "security-framework 3.5.1",
  "tracing",
 ]
 

codex-rs/keyring-store/Cargo.toml

@@ -12,9 +12,11 @@ keyring = { workspace = true, features = ["crypto-rust"] }
 tracing = { workspace = true }
 
 [target.'cfg(target_os = "linux")'.dependencies]
+secret-service = "4"
 keyring = { workspace = true, features = ["linux-native-async-persistent"] }
 
 [target.'cfg(target_os = "macos")'.dependencies]
+security-framework = "3"
 keyring = { workspace = true, features = ["apple-native"] }
 
 [target.'cfg(target_os = "windows")'.dependencies]
@@ -23,6 +25,8 @@ keyring = { workspace = true, features = ["windows-native"] }
 [target.'cfg(any(target_os = "freebsd", target_os = "openbsd"))'.dependencies]
 keyring = { workspace = true, features = ["sync-secret-service"] }
 
+[dev-dependencies]
+pretty_assertions = { workspace = true }
+
 [lib]
-test = false
 doctest = false

codex-rs/keyring-store/src/error_kind.rs

@@ -0,0 +1,51 @@
+//! Maps reliable native failures to portable IO kinds without examining error messages.
+//! Ambiguous keyring errors stay Other: NoStorageAccess alone does not prove a locked store.
+
+use crate::CredentialStoreError;
+use std::error::Error;
+use std::io::ErrorKind;
+
+pub(super) fn classify(error: &CredentialStoreError) -> ErrorKind {
+    let mut cause: Option<&(dyn Error + 'static)> = Some(error);
+    while let Some(error) = cause {
+        if let Some(error) = error.downcast_ref::<std::io::Error>() {
+            return error.kind();
+        }
+        if let Some(error) = error.downcast_ref::<std::sync::Arc<std::io::Error>>() {
+            return error.kind();
+        }
+        #[cfg(target_os = "macos")]
+        if let Some(error) = error.downcast_ref::<security_framework::base::Error>() {
+            return match error.code() {
+                -25291 => ErrorKind::NotConnected, // errSecNotAvailable
+                -25292 | -25293 => ErrorKind::PermissionDenied, // errSecReadOnly / errSecAuthFailed
+                -4 => ErrorKind::Unsupported,      // errSecUnimplemented
+                _ => ErrorKind::Other,
+            };
+        }
+        #[cfg(target_os = "linux")]
+        if let Some(error) = error.downcast_ref::<secret_service::Error>() {
+            match error {
+                secret_service::Error::Locked => return ErrorKind::WouldBlock,
+                secret_service::Error::Unavailable => return ErrorKind::NotConnected,
+                _ => {}
+            }
+        }
+        #[cfg(target_os = "windows")]
+        if let Some(error) = error.downcast_ref::<keyring::windows::Error>() {
+            return match error.0 {
+                1312 => ErrorKind::NotConnected,  // ERROR_NO_SUCH_LOGON_SESSION
+                5 => ErrorKind::PermissionDenied, // ERROR_ACCESS_DENIED
+                50 => ErrorKind::Unsupported,     // ERROR_NOT_SUPPORTED
+                1460 => ErrorKind::TimedOut,      // ERROR_TIMEOUT
+                _ => ErrorKind::Other,
+            };
+        }
+        cause = error.source();
+    }
+    ErrorKind::Other
+}
+
+#[cfg(test)]
+#[path = "error_kind_tests.rs"]
+mod tests;

codex-rs/keyring-store/src/error_kind_tests.rs

@@ -0,0 +1,102 @@
+//! Tests native cause classification without touching the real keyring.
+
+use super::*;
+use pretty_assertions::assert_eq;
+
+#[test]
+fn preserves_typed_causes_without_guessing_from_messages() {
+    let cases = [
+        (
+            keyring::Error::NoStorageAccess(Box::new(std::io::Error::other(
+                "locked timeout access denied",
+            ))),
+            ErrorKind::Other,
+        ),
+        (
+            keyring::Error::PlatformFailure(Box::new(std::io::Error::new(
+                ErrorKind::TimedOut,
+                "private account",
+            ))),
+            ErrorKind::TimedOut,
+        ),
+        (
+            keyring::Error::PlatformFailure(Box::new(std::sync::Arc::new(std::io::Error::new(
+                ErrorKind::PermissionDenied,
+                "private D-Bus socket",
+            )))),
+            ErrorKind::PermissionDenied,
+        ),
+        (
+            keyring::Error::Invalid("private account".into(), "private reason".into()),
+            ErrorKind::Other,
+        ),
+    ];
+    for (error, expected) in cases {
+        let error = std::io::Error::from(CredentialStoreError::new(error));
+        assert_eq!(error.kind(), expected);
+        assert!(
+            error
+                .get_ref()
+                .unwrap()
+                .downcast_ref::<CredentialStoreError>()
+                .is_some()
+        );
+    }
+}
+
+#[test]
+fn classifies_native_backend_errors() {
+    #[cfg(target_os = "macos")]
+    let native: Box<dyn Error + Send + Sync> = Box::new(
+        security_framework::base::Error::from_code(/*code*/ -25291),
+    );
+    #[cfg(target_os = "linux")]
+    let native: Box<dyn Error + Send + Sync> = Box::new(secret_service::Error::Locked);
+    #[cfg(target_os = "windows")]
+    let native: Box<dyn Error + Send + Sync> = Box::new(keyring::windows::Error(1312));
+    #[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))]
+    assert_eq!(
+        classify(&CredentialStoreError::new(keyring::Error::NoStorageAccess(
+            native
+        ))),
+        if cfg!(target_os = "linux") {
+            ErrorKind::WouldBlock
+        } else {
+            ErrorKind::NotConnected
+        },
+    );
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn unavailable_secret_service_is_not_connected() {
+    let error = CredentialStoreError::new(keyring::Error::PlatformFailure(Box::new(
+        secret_service::Error::Unavailable,
+    )));
+    assert_eq!(std::io::Error::from(error).kind(), ErrorKind::NotConnected);
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn secret_service_dbus_io_error_preserves_its_kind() {
+    let error = secret_service::Error::Zbus(
+        std::io::Error::new(ErrorKind::PermissionDenied, "private D-Bus socket").into(),
+    );
+    let error = CredentialStoreError::new(keyring::Error::PlatformFailure(Box::new(error)));
+    assert_eq!(
+        std::io::Error::from(error).kind(),
+        ErrorKind::PermissionDenied
+    );
+}
+
+#[cfg(target_os = "macos")]
+#[test]
+fn read_only_keychain_is_permission_denied() {
+    let error = CredentialStoreError::new(keyring::Error::NoStorageAccess(Box::new(
+        security_framework::base::Error::from_code(/*code*/ -25292),
+    )));
+    assert_eq!(
+        std::io::Error::from(error).kind(),
+        ErrorKind::PermissionDenied
+    );
+}

codex-rs/keyring-store/src/lib.rs

@@ -36,7 +36,21 @@ impl fmt::Display for CredentialStoreError {
     }
 }
 
-impl Error for CredentialStoreError {}
+impl Error for CredentialStoreError {
+    fn source(&self) -> Option<&(dyn Error + 'static)> {
+        match self {
+            Self::Other(error) => Some(error),
+        }
+    }
+}
+
+mod error_kind;
+
+impl From<CredentialStoreError> for std::io::Error {
+    fn from(error: CredentialStoreError) -> Self {
+        Self::new(error_kind::classify(&error), error)
+    }
+}
 
 /// Shared credential store abstraction for keyring-backed implementations.
 pub trait KeyringStore: Debug + Send + Sync {
@@ -50,56 +64,53 @@ pub struct DefaultKeyringStore;
 
 impl KeyringStore for DefaultKeyringStore {
     fn load(&self, service: &str, account: &str) -> Result<Option<String>, CredentialStoreError> {
-        trace!("keyring.load start, service={service}, account={account}");
+        trace!("keyring.load start");
         let entry = Entry::new(service, account).map_err(CredentialStoreError::new)?;
         match entry.get_password() {
             Ok(password) => {
-                trace!("keyring.load success, service={service}, account={account}");
+                trace!("keyring.load success");
                 Ok(Some(password))
             }
             Err(keyring::Error::NoEntry) => {
-                trace!("keyring.load no entry, service={service}, account={account}");
+                trace!("keyring.load no entry");
                 Ok(None)
             }
             Err(error) => {
-                trace!("keyring.load error, service={service}, account={account}, error={error}");
+                trace!("keyring.load error");
                 Err(CredentialStoreError::new(error))
             }
         }
     }
 
     fn save(&self, service: &str, account: &str, value: &str) -> Result<(), CredentialStoreError> {
-        trace!(
-            "keyring.save start, service={service}, account={account}, value_len={}",
-            value.len()
-        );
+        trace!("keyring.save start");
         let entry = Entry::new(service, account).map_err(CredentialStoreError::new)?;
         match entry.set_password(value) {
             Ok(()) => {
-                trace!("keyring.save success, service={service}, account={account}");
+                trace!("keyring.save success");
                 Ok(())
             }
             Err(error) => {
-                trace!("keyring.save error, service={service}, account={account}, error={error}");
+                trace!("keyring.save error");
                 Err(CredentialStoreError::new(error))
             }
         }
     }
 
     fn delete(&self, service: &str, account: &str) -> Result<bool, CredentialStoreError> {
-        trace!("keyring.delete start, service={service}, account={account}");
+        trace!("keyring.delete start");
         let entry = Entry::new(service, account).map_err(CredentialStoreError::new)?;
         match entry.delete_credential() {
             Ok(()) => {
-                trace!("keyring.delete success, service={service}, account={account}");
+                trace!("keyring.delete success");
                 Ok(true)
             }
             Err(keyring::Error::NoEntry) => {
-                trace!("keyring.delete no entry, service={service}, account={account}");
+                trace!("keyring.delete no entry");
                 Ok(false)
             }
             Err(error) => {
-                trace!("keyring.delete error, service={service}, account={account}, error={error}");
+                trace!("keyring.delete error");
                 Err(CredentialStoreError::new(error))
             }
         }

codex-rs/otel/src/auth_storage.rs

@@ -0,0 +1,257 @@
+//! Credential-storage metrics contain only bounded classifications, never credentials or paths.
+//! Shared metrics infrastructure supplies OS/version metadata and startup buffering.
+
+mod originator;
+
+pub use originator::AuthStorageOriginator;
+use std::error::Error;
+use std::time::Duration;
+use std::time::Instant;
+use strum_macros::AsRefStr;
+
+#[derive(Clone, Copy, Debug, AsRefStr)]
+#[strum(serialize_all = "snake_case")]
+pub enum CredentialKind {
+    Codex,
+    Mcp,
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq, AsRefStr)]
+#[strum(serialize_all = "snake_case")]
+pub enum StoreMode {
+    File,
+    Auto,
+    Keyring,
+    Ephemeral,
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq, AsRefStr)]
+#[strum(serialize_all = "snake_case")]
+pub enum Store {
+    File,
+    DirectKeyring,
+    Secrets,
+    Ephemeral,
+    /// A deletion spanning the selected backend, plaintext fallback, and legacy entries.
+    Multiple,
+}
+
+#[derive(Clone, Copy, Debug, AsRefStr)]
+#[strum(serialize_all = "snake_case")]
+pub enum Operation {
+    Load,
+    Save,
+    Delete,
+    Cleanup,
+    RefreshPersist,
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq, AsRefStr)]
+#[strum(serialize_all = "snake_case")]
+enum Outcome {
+    NotAttempted,
+    Success,
+    NotFound,
+    Error,
+}
+
+#[derive(Clone, Copy, Debug)]
+struct Observation {
+    credential_kind: CredentialKind,
+    mode: StoreMode,
+    selected_store: Store,
+    actual_store: Store,
+    operation: Operation,
+    secure_outcome: Outcome,
+    outcome: Outcome,
+    duration: Duration,
+    phase: StoragePhase,
+    secure_error: &'static str,
+    originator: AuthStorageOriginator,
+}
+
+impl Observation {
+    fn record(self) {
+        let fallback = match (
+            self.mode,
+            self.phase,
+            self.actual_store,
+            self.secure_outcome,
+        ) {
+            (StoreMode::Auto, StoragePhase::Policy, Store::File, Outcome::Error) => "secure_error",
+            (StoreMode::Auto, StoragePhase::Policy, Store::File, Outcome::NotFound) => {
+                "secure_entry_missing"
+            }
+            _ => "none",
+        };
+        let tags = &[
+            ("credential_kind", self.credential_kind.as_ref()),
+            ("store_mode", self.mode.as_ref()),
+            ("selected_store", self.selected_store.as_ref()),
+            ("actual_store", self.actual_store.as_ref()),
+            ("operation", self.operation.as_ref()),
+            ("secure_outcome", self.secure_outcome.as_ref()),
+            ("outcome", self.outcome.as_ref()),
+            ("fallback_reason", fallback),
+            ("secure_error", self.secure_error),
+            ("storage_phase", self.phase.as_ref()),
+            (crate::ORIGINATOR_TAG, self.originator.as_str()),
+        ];
+        if fallback != "none" {
+            tracing::warn!(
+                credential_kind = self.credential_kind.as_ref(),
+                store_mode = self.mode.as_ref(),
+                selected_store = self.selected_store.as_ref(),
+                actual_store = self.actual_store.as_ref(),
+                operation = self.operation.as_ref(),
+                secure_outcome = self.secure_outcome.as_ref(),
+                outcome = self.outcome.as_ref(),
+                fallback_reason = fallback,
+                secure_error = self.secure_error,
+                storage_phase = self.phase.as_ref(),
+                originator = self.originator.as_str(),
+                "credential storage file fallback completed"
+            );
+        }
+        let (count, duration) = match self.operation {
+            Operation::RefreshPersist => (
+                "codex.auth_storage.refresh_persist",
+                "codex.auth_storage.refresh_persist.duration",
+            ),
+            Operation::Load | Operation::Save | Operation::Delete | Operation::Cleanup => (
+                "codex.auth_storage.operation",
+                "codex.auth_storage.duration",
+            ),
+        };
+        let _ = crate::record_global_operation(count, duration, self.duration, tags);
+    }
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq, AsRefStr)]
+#[strum(serialize_all = "snake_case")]
+pub enum StoragePhase {
+    Policy,
+    Pinned,
+}
+
+/// Records one logical storage operation, including the secure attempt and any file fallback.
+/// Callers execute each attempt and pass its result by reference; error messages and payloads
+/// are not read. The combined outcome and elapsed time are recorded when this guard is dropped.
+/// Client attribution is supplied at construction and retained until emission.
+#[derive(Debug)]
+pub struct StorageTelemetry {
+    observation: Observation,
+    started: Instant,
+}
+
+impl StorageTelemetry {
+    pub fn new(
+        credential_kind: CredentialKind,
+        mode: StoreMode,
+        selected_store: Store,
+        operation: Operation,
+        originator: AuthStorageOriginator,
+    ) -> Self {
+        Self {
+            observation: Observation {
+                credential_kind,
+                mode,
+                selected_store,
+                actual_store: selected_store,
+                operation,
+                secure_outcome: Outcome::NotAttempted,
+                outcome: Outcome::NotAttempted,
+                duration: Duration::ZERO,
+                phase: StoragePhase::Policy,
+                secure_error: "none",
+                originator,
+            },
+            started: Instant::now(),
+        }
+    }
+
+    /// Pinned operations retain their configured mode but never re-evaluate fallback policy.
+    pub fn with_phase(mut self, phase: StoragePhase) -> Self {
+        self.observation.phase = phase;
+        self
+    }
+
+    /// Classifies typed causes only; never formats errors or parses their messages.
+    /// May be called before or after recording the corresponding failed secure attempt.
+    pub fn record_secure_error(&mut self, error: &(dyn Error + 'static)) {
+        let mut cause = Some(error);
+        while let Some(error) = cause {
+            if let Some(error) = error.downcast_ref::<std::io::Error>() {
+                let category = match error.kind() {
+                    std::io::ErrorKind::PermissionDenied => "access_denied",
+                    std::io::ErrorKind::NotConnected => "unavailable",
+                    std::io::ErrorKind::WouldBlock => "locked",
+                    std::io::ErrorKind::Unsupported => "unsupported",
+                    std::io::ErrorKind::TimedOut => "timeout",
+                    _ => "other",
+                };
+                if category != "other" {
+                    self.observation.secure_error = category;
+                    return;
+                }
+                cause = error.get_ref().map(|error| error as &dyn Error);
+            } else {
+                cause = error.source();
+            }
+        }
+        self.observation.secure_error = "other";
+    }
+
+    pub fn record_load_attempt<T, E>(&mut self, store: Store, result: &Result<Option<T>, E>) {
+        let outcome = match result {
+            Ok(Some(_)) => Outcome::Success,
+            Ok(None) => Outcome::NotFound,
+            Err(_) => Outcome::Error,
+        };
+        self.record_attempt(store, outcome);
+    }
+
+    pub fn record_save_attempt<E>(&mut self, store: Store, result: &Result<(), E>) {
+        let outcome = match result {
+            Ok(()) => Outcome::Success,
+            Err(_) => Outcome::Error,
+        };
+        self.record_attempt(store, outcome);
+    }
+
+    pub fn record_delete_attempt<E>(&mut self, store: Store, result: &Result<bool, E>) {
+        let outcome = match result {
+            Ok(true) => Outcome::Success,
+            Ok(false) => Outcome::NotFound,
+            Err(_) => Outcome::Error,
+        };
+        self.record_attempt(store, outcome);
+    }
+
+    fn record_attempt(&mut self, store: Store, outcome: Outcome) {
+        self.observation.actual_store = store;
+        self.observation.outcome = outcome;
+        if matches!(store, Store::DirectKeyring | Store::Secrets) {
+            self.observation.secure_outcome = outcome;
+            if outcome != Outcome::Error {
+                self.observation.secure_error = "none";
+            } else if self.observation.secure_error == "none" {
+                self.observation.secure_error = "other";
+            }
+        }
+    }
+}
+
+impl Drop for StorageTelemetry {
+    fn drop(&mut self) {
+        if self.observation.outcome == Outcome::NotAttempted {
+            return;
+        }
+        self.observation.duration = self.started.elapsed();
+        self.observation.record();
+    }
+}
+
+#[cfg(test)]
+#[path = "auth_storage_tests.rs"]
+mod tests;

codex-rs/otel/src/auth_storage/originator.rs

@@ -0,0 +1,45 @@
+//! Bounded attribution for auth-storage observations, scoped to one task.
+//! Spawned tasks must explicitly carry this context across the task boundary.
+
+use std::future::Future;
+
+tokio::task_local! {
+    static CURRENT: AuthStorageOriginator;
+}
+
+/// A bounded client identity used only by credential-storage telemetry.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub struct AuthStorageOriginator(&'static str);
+
+impl AuthStorageOriginator {
+    pub fn from_client_name(name: &str) -> Self {
+        let name = match name {
+            "Codex Desktop" => "codex_desktop",
+            _ => name,
+        };
+        Self(crate::bounded_originator_tag_value(name))
+    }
+
+    /// Unattributed work must not borrow another connection's identity.
+    pub fn current() -> Self {
+        CURRENT
+            .try_with(|originator| *originator)
+            .unwrap_or(Self("none"))
+    }
+
+    pub fn as_str(self) -> &'static str {
+        self.0
+    }
+
+    pub async fn scope<F: Future>(self, future: F) -> F::Output {
+        CURRENT.scope(self, future).await
+    }
+
+    pub fn sync_scope<R>(self, operation: impl FnOnce() -> R) -> R {
+        CURRENT.sync_scope(self, operation)
+    }
+}
+
+#[cfg(test)]
+#[path = "originator_tests.rs"]
+mod tests;

codex-rs/otel/src/auth_storage/originator_tests.rs

@@ -0,0 +1,49 @@
+//! Verifies attribution isolation and explicit propagation across task boundaries.
+
+use super::AuthStorageOriginator;
+use pretty_assertions::assert_eq;
+use std::sync::Arc;
+use tokio::sync::Barrier;
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn concurrent_clients_keep_their_originator_across_awaits_and_blocking_work() {
+    let barrier = Arc::new(Barrier::new(/*n*/ 3));
+    let mut tasks = Vec::new();
+    for name in ["Codex Desktop", "codex_vscode", "private-client-name"] {
+        let barrier = Arc::clone(&barrier);
+        let originator = AuthStorageOriginator::from_client_name(name);
+        tasks.push(tokio::spawn(originator.scope(async move {
+            barrier.wait().await;
+            tokio::task::yield_now().await;
+            let before = AuthStorageOriginator::current();
+            let nested = AuthStorageOriginator::from_client_name("codex_cli_rs")
+                .scope(async {
+                    tokio::task::yield_now().await;
+                    AuthStorageOriginator::current().as_str()
+                })
+                .await;
+            let blocking = tokio::task::spawn_blocking(move || {
+                before.sync_scope(AuthStorageOriginator::current)
+            })
+            .await
+            .unwrap();
+            (
+                before.as_str(),
+                nested,
+                blocking.as_str(),
+                AuthStorageOriginator::current().as_str(),
+            )
+        })));
+    }
+    let mut actual = Vec::new();
+    for task in tasks {
+        actual.push(task.await.unwrap());
+    }
+    assert_eq!(
+        actual,
+        ["codex_desktop", "codex_vscode", "other"]
+            .map(|originator| (originator, "codex_cli_rs", originator, originator))
+            .to_vec()
+    );
+    assert_eq!(AuthStorageOriginator::current().as_str(), "none");
+}

codex-rs/otel/src/auth_storage_tests.rs

@@ -0,0 +1,248 @@
+//! Exercises buffered storage observations and bounded diagnostics.
+
+use super::*;
+use crate::MetricsClient;
+use crate::MetricsConfig;
+use opentelemetry_sdk::metrics::InMemoryMetricExporter;
+use opentelemetry_sdk::metrics::data::AggregatedMetrics;
+use opentelemetry_sdk::metrics::data::MetricData;
+use pretty_assertions::assert_eq;
+use std::collections::BTreeMap;
+
+// Provider installation is process-global. Isolate these cases even under Bazel/libtest,
+// which otherwise runs all unit tests in a shared process.
+fn run_in_subprocess() -> bool {
+    if std::env::var_os("CODEX_AUTH_STORAGE_TEST_CHILD").is_some() {
+        return false;
+    }
+    let thread = std::thread::current();
+    let output = std::process::Command::new(std::env::current_exe().unwrap())
+        .args(["--exact", thread.name().unwrap(), "--nocapture"])
+        .env("CODEX_AUTH_STORAGE_TEST_CHILD", "1")
+        .output()
+        .unwrap();
+    assert!(
+        output.status.success(),
+        "{}{}",
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr)
+    );
+    true
+}
+
+#[test]
+fn startup_fallback_exports_one_logical_operation_without_error_contents() -> crate::Result<()> {
+    if run_in_subprocess() {
+        return Ok(());
+    }
+    let mut telemetry = AuthStorageOriginator::from_client_name("Codex Desktop").sync_scope(|| {
+        StorageTelemetry::new(
+            CredentialKind::Codex,
+            StoreMode::Auto,
+            Store::Secrets,
+            Operation::Save,
+            AuthStorageOriginator::current(),
+        )
+    });
+    let logs = std::sync::Arc::new(std::sync::Mutex::new(Vec::new()));
+    let writer = LogWriter(logs.clone());
+    let subscriber = tracing_subscriber::fmt()
+        .without_time()
+        .with_ansi(false)
+        .with_target(false)
+        .with_writer(move || writer.clone())
+        .finish();
+    let failure: Result<(), &str> = Err("sensitive error contents");
+    telemetry.record_save_attempt(Store::Secrets, &failure);
+    let success: Result<(), &str> = Ok(());
+    telemetry.record_save_attempt(Store::File, &success);
+    telemetry.record_secure_error(&std::io::Error::new(
+        std::io::ErrorKind::PermissionDenied,
+        "private account",
+    ));
+    tracing::subscriber::with_default(subscriber, || drop(telemetry));
+    let logs = String::from_utf8(logs.lock().unwrap().clone()).unwrap();
+    assert_eq!(logs.lines().count(), 1);
+    assert!(logs.contains("secure_error=\"access_denied\""), "{logs}");
+    assert!(logs.contains("originator=\"codex_desktop\""), "{logs}");
+    assert!(logs.contains("outcome=\"success\""), "{logs}");
+    assert!(!logs.contains("private account") && !logs.contains("sensitive error contents"));
+
+    let metrics = crate::install_global_metrics(MetricsClient::new(
+        MetricsConfig::in_memory("test", "test", "1", InMemoryMetricExporter::default())
+            .with_runtime_reader(),
+    )?);
+    let snapshot = metrics.snapshot()?;
+    let mut counts = Vec::new();
+    let mut durations = Vec::new();
+    for metric in snapshot
+        .scope_metrics()
+        .flat_map(opentelemetry_sdk::metrics::data::ScopeMetrics::metrics)
+    {
+        match metric.data() {
+            AggregatedMetrics::U64(MetricData::Sum(sum)) => {
+                assert_eq!(metric.name(), "codex.auth_storage.operation");
+                for point in sum.data_points() {
+                    counts.push((
+                        point.value(),
+                        point
+                            .attributes()
+                            .map(|attribute| {
+                                (attribute.key.to_string(), attribute.value.to_string())
+                            })
+                            .collect::<BTreeMap<_, _>>(),
+                    ));
+                }
+            }
+            AggregatedMetrics::F64(MetricData::Histogram(histogram)) => {
+                assert_eq!(metric.name(), "codex.auth_storage.duration");
+                durations.extend(
+                    histogram
+                        .data_points()
+                        .map(opentelemetry_sdk::metrics::data::HistogramDataPoint::count),
+                );
+            }
+            _ => panic!("unexpected metric"),
+        }
+    }
+    assert_eq!(
+        counts,
+        vec![(
+            1,
+            BTreeMap::from([
+                ("credential_kind".into(), "codex".into()),
+                ("store_mode".into(), "auto".into()),
+                ("selected_store".into(), "secrets".into()),
+                ("actual_store".into(), "file".into()),
+                ("operation".into(), "save".into()),
+                ("secure_outcome".into(), "error".into()),
+                ("outcome".into(), "success".into()),
+                ("fallback_reason".into(), "secure_error".into()),
+                ("secure_error".into(), "access_denied".into()),
+                ("storage_phase".into(), "policy".into()),
+                ("originator".into(), "codex_desktop".into()),
+            ])
+        )]
+    );
+    assert_eq!(durations, vec![1]);
+    drop(StorageTelemetry::new(
+        CredentialKind::Codex,
+        StoreMode::Auto,
+        Store::Secrets,
+        Operation::Save,
+        AuthStorageOriginator::current(),
+    ));
+    assert!(
+        metrics
+            .snapshot()?
+            .scope_metrics()
+            .all(|scope| scope.metrics().next().is_none())
+    );
+    StorageTelemetry::new(
+        CredentialKind::Mcp,
+        StoreMode::Auto,
+        Store::Secrets,
+        Operation::RefreshPersist,
+        AuthStorageOriginator::current(),
+    )
+    .record_save_attempt(Store::Secrets, &Ok::<(), ()>(()));
+    let snapshot = metrics.snapshot()?;
+    let mut names: Vec<_> = snapshot
+        .scope_metrics()
+        .flat_map(opentelemetry_sdk::metrics::data::ScopeMetrics::metrics)
+        .map(opentelemetry_sdk::metrics::data::Metric::name)
+        .collect();
+    names.sort();
+    assert_eq!(
+        names,
+        vec![
+            "codex.auth_storage.refresh_persist",
+            "codex.auth_storage.refresh_persist.duration"
+        ]
+    );
+    metrics.shutdown()
+}
+
+#[test]
+fn missing_credentials_and_failed_fallback_keep_distinct_outcomes() {
+    if run_in_subprocess() {
+        return;
+    }
+    let mut telemetry = StorageTelemetry::new(
+        CredentialKind::Mcp,
+        StoreMode::Auto,
+        Store::DirectKeyring,
+        Operation::Load,
+        AuthStorageOriginator::current(),
+    );
+    let missing: Result<Option<String>, &str> = Ok(None);
+    telemetry.record_load_attempt(Store::DirectKeyring, &missing);
+    let failure: Result<Option<String>, &str> = Err("file unavailable");
+    telemetry.record_load_attempt(Store::File, &failure);
+    assert_eq!(
+        (
+            telemetry.observation.secure_outcome,
+            telemetry.observation.outcome,
+            telemetry.observation.actual_store
+        ),
+        (Outcome::NotFound, Outcome::Error, Store::File)
+    );
+}
+
+#[test]
+fn secure_error_classification_is_independent_of_attempt_recording_order() {
+    if run_in_subprocess() {
+        return;
+    }
+    for store in [Store::DirectKeyring, Store::Secrets] {
+        for classify_first in [true, false] {
+            let mut telemetry = StorageTelemetry::new(
+                CredentialKind::Codex,
+                StoreMode::Auto,
+                store,
+                Operation::Save,
+                AuthStorageOriginator::current(),
+            );
+            let failure = Err(std::io::Error::new(
+                std::io::ErrorKind::PermissionDenied,
+                "private account",
+            ));
+            if classify_first {
+                telemetry.record_secure_error(failure.as_ref().unwrap_err());
+            }
+            telemetry.record_save_attempt(store, &failure);
+            if !classify_first {
+                telemetry.record_secure_error(failure.as_ref().unwrap_err());
+            }
+            telemetry.record_save_attempt(Store::File, &Ok::<(), ()>(()));
+            assert_eq!(
+                (
+                    telemetry.observation.secure_outcome,
+                    telemetry.observation.outcome,
+                    telemetry.observation.secure_error,
+                ),
+                (Outcome::Error, Outcome::Success, "access_denied")
+            );
+            telemetry.record_save_attempt(store, &Ok::<(), ()>(()));
+            assert_eq!(
+                (
+                    telemetry.observation.secure_outcome,
+                    telemetry.observation.secure_error,
+                ),
+                (Outcome::Success, "none")
+            );
+        }
+    }
+}
+
+#[derive(Clone)]
+struct LogWriter(std::sync::Arc<std::sync::Mutex<Vec<u8>>>);
+impl std::io::Write for LogWriter {
+    fn write(&mut self, bytes: &[u8]) -> std::io::Result<usize> {
+        self.0.lock().unwrap().extend_from_slice(bytes);
+        Ok(bytes.len())
+    }
+    fn flush(&mut self) -> std::io::Result<()> {
+        Ok(())
+    }
+}

codex-rs/otel/src/lib.rs

@@ -1,3 +1,4 @@
+pub mod auth_storage;
 pub(crate) mod config;
 mod events;
 pub(crate) mod metrics;

codex-rs/secrets/src/local.rs

@@ -261,7 +261,7 @@ impl LocalSecretsBackend {
         let loaded = self
             .keyring_store
             .load(keyring_service(), &account)
-            .map_err(|err| anyhow::anyhow!(err.message()))
+            .map_err(std::io::Error::from)
             .with_context(|| format!("failed to load secrets key from keyring for {account}"))?;
         match loaded {
             Some(existing) => Ok(SecretString::from(existing)),
@@ -272,7 +272,7 @@ impl LocalSecretsBackend {
                 let generated = generate_passphrase()?;
                 self.keyring_store
                     .save(keyring_service(), &account, generated.expose_secret())
-                    .map_err(|err| anyhow::anyhow!(err.message()))
+                    .map_err(std::io::Error::from)
                     .context("failed to persist secrets key in keyring")?;
                 Ok(generated)
             }