Use binary split-entry keyring storage for auth to avoid Windows credential size failures

codex-rs/core/src/auth/storage.rs

@@ -133,6 +133,21 @@ impl AuthStorageBackend for FileAuthStorage {
 }
 
 const KEYRING_SERVICE: &str = "Codex Auth";
+const KEYRING_LAYOUT_VERSION: &str = "v2";
+const KEYRING_ACTIVE_REVISION_ENTRY: &str = "active";
+const KEYRING_MANIFEST_ENTRY: &str = "manifest";
+const KEYRING_OPENAI_API_KEY_ENTRY: &str = "OPENAI_API_KEY";
+const KEYRING_ID_TOKEN_ENTRY: &str = "tokens.id_token";
+const KEYRING_ACCESS_TOKEN_ENTRY: &str = "tokens.access_token";
+const KEYRING_REFRESH_TOKEN_ENTRY: &str = "tokens.refresh_token";
+const KEYRING_ACCOUNT_ID_ENTRY: &str = "tokens.account_id";
+const KEYRING_RECORD_ENTRIES: [&str; 5] = [
+    KEYRING_MANIFEST_ENTRY,
+    KEYRING_OPENAI_API_KEY_ENTRY,
+    KEYRING_ID_TOKEN_ENTRY,
+    KEYRING_ACCESS_TOKEN_ENTRY,
+    KEYRING_REFRESH_TOKEN_ENTRY,
+];
 
 // turns codex_home path into a stable, short key string
 fn compute_store_key(codex_home: &Path) -> std::io::Result<String> {
@@ -148,6 +163,47 @@ fn compute_store_key(codex_home: &Path) -> std::io::Result<String> {
     Ok(format!("cli|{truncated}"))
 }
 
+fn keyring_layout_key(base_key: &str, suffix: &str) -> String {
+    format!("{base_key}|{KEYRING_LAYOUT_VERSION}|{suffix}")
+}
+
+fn keyring_revision_key(base_key: &str, revision: &str, suffix: &str) -> String {
+    format!("{base_key}|{KEYRING_LAYOUT_VERSION}|{revision}|{suffix}")
+}
+
+fn next_keyring_revision() -> String {
+    Utc::now()
+        .timestamp_nanos_opt()
+        .unwrap_or_else(|| Utc::now().timestamp_micros() * 1_000)
+        .to_string()
+}
+
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
+struct KeyringAuthManifest {
+    auth_mode: Option<AuthMode>,
+    has_openai_api_key: bool,
+    has_tokens: bool,
+    has_account_id: bool,
+    last_refresh: Option<DateTime<Utc>>,
+}
+
+impl From<&AuthDotJson> for KeyringAuthManifest {
+    fn from(auth: &AuthDotJson) -> Self {
+        let has_account_id = auth
+            .tokens
+            .as_ref()
+            .and_then(|tokens| tokens.account_id.as_ref())
+            .is_some();
+        Self {
+            auth_mode: auth.auth_mode,
+            has_openai_api_key: auth.openai_api_key.is_some(),
+            has_tokens: auth.tokens.is_some(),
+            has_account_id,
+            last_refresh: auth.last_refresh,
+        }
+    }
+}
+
 #[derive(Clone, Debug)]
 struct KeyringAuthStorage {
     codex_home: PathBuf,
@@ -162,7 +218,7 @@ impl KeyringAuthStorage {
         }
     }
 
-    fn load_from_keyring(&self, key: &str) -> std::io::Result<Option<AuthDotJson>> {
+    fn load_legacy_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!(
@@ -177,19 +233,231 @@ impl KeyringAuthStorage {
         }
     }
 
-    fn save_to_keyring(&self, key: &str, value: &str) -> std::io::Result<()> {
-        match self.keyring_store.save(KEYRING_SERVICE, key, value) {
+    fn load_secret_from_keyring(&self, key: &str, field: &str) -> std::io::Result<Option<Vec<u8>>> {
+        match self.keyring_store.load_secret(KEYRING_SERVICE, key) {
+            Ok(secret) => Ok(secret),
+            Err(error) => Err(std::io::Error::other(format!(
+                "failed to load {field} from keyring: {}",
+                error.message()
+            ))),
+        }
+    }
+
+    fn load_utf8_secret_from_keyring(
+        &self,
+        key: &str,
+        field: &str,
+    ) -> std::io::Result<Option<String>> {
+        let Some(secret) = self.load_secret_from_keyring(key, field)? else {
+            return Ok(None);
+        };
+        String::from_utf8(secret).map(Some).map_err(|err| {
+            std::io::Error::other(format!(
+                "failed to decode {field} from keyring as UTF-8: {err}"
+            ))
+        })
+    }
+
+    fn save_secret_to_keyring(&self, key: &str, value: &[u8], field: &str) -> std::io::Result<()> {
+        match self.keyring_store.save_secret(KEYRING_SERVICE, key, value) {
             Ok(()) => Ok(()),
             Err(error) => {
-                let message = format!(
-                    "failed to write OAuth tokens to keyring: {}",
-                    error.message()
-                );
+                let message = format!("failed to write {field} to keyring: {}", error.message());
                 warn!("{message}");
                 Err(std::io::Error::other(message))
             }
         }
     }
+
+    fn load_active_revision(&self, base_key: &str) -> std::io::Result<Option<String>> {
+        let active_key = keyring_layout_key(base_key, KEYRING_ACTIVE_REVISION_ENTRY);
+        self.load_utf8_secret_from_keyring(&active_key, "active auth revision")
+    }
+
+    fn load_required_utf8_secret(&self, key: &str, field: &str) -> std::io::Result<String> {
+        self.load_utf8_secret_from_keyring(key, field)?
+            .ok_or_else(|| std::io::Error::other(format!("missing {field} in keyring")))
+    }
+
+    fn load_manifest(
+        &self,
+        base_key: &str,
+        revision: &str,
+    ) -> std::io::Result<KeyringAuthManifest> {
+        let manifest_key = keyring_revision_key(base_key, revision, KEYRING_MANIFEST_ENTRY);
+        let manifest = self
+            .load_secret_from_keyring(&manifest_key, "auth manifest")?
+            .ok_or_else(|| std::io::Error::other("missing auth manifest in keyring"))?;
+        serde_json::from_slice(&manifest).map_err(|err| {
+            std::io::Error::other(format!(
+                "failed to deserialize auth manifest from keyring: {err}"
+            ))
+        })
+    }
+
+    fn load_v2_from_keyring(&self, base_key: &str, revision: &str) -> std::io::Result<AuthDotJson> {
+        let manifest = self.load_manifest(base_key, revision)?;
+        let openai_api_key = if manifest.has_openai_api_key {
+            let key = keyring_revision_key(base_key, revision, KEYRING_OPENAI_API_KEY_ENTRY);
+            Some(self.load_required_utf8_secret(&key, "OPENAI_API_KEY")?)
+        } else {
+            None
+        };
+        let tokens = if manifest.has_tokens {
+            let id_token_key = keyring_revision_key(base_key, revision, KEYRING_ID_TOKEN_ENTRY);
+            let id_token = self.load_required_utf8_secret(&id_token_key, "ID token")?;
+            let access_token_key =
+                keyring_revision_key(base_key, revision, KEYRING_ACCESS_TOKEN_ENTRY);
+            let access_token = self.load_required_utf8_secret(&access_token_key, "access token")?;
+            let refresh_token_key =
+                keyring_revision_key(base_key, revision, KEYRING_REFRESH_TOKEN_ENTRY);
+            let refresh_token =
+                self.load_required_utf8_secret(&refresh_token_key, "refresh token")?;
+            let account_id = if manifest.has_account_id {
+                let account_id_key =
+                    keyring_revision_key(base_key, revision, KEYRING_ACCOUNT_ID_ENTRY);
+                Some(self.load_required_utf8_secret(&account_id_key, "account ID")?)
+            } else {
+                None
+            };
+            Some(TokenData {
+                id_token: crate::token_data::parse_chatgpt_jwt_claims(&id_token)
+                    .map_err(std::io::Error::other)?,
+                access_token,
+                refresh_token,
+                account_id,
+            })
+        } else {
+            None
+        };
+        Ok(AuthDotJson {
+            auth_mode: manifest.auth_mode,
+            openai_api_key,
+            tokens,
+            last_refresh: manifest.last_refresh,
+        })
+    }
+
+    fn load_from_keyring(&self, base_key: &str) -> std::io::Result<Option<AuthDotJson>> {
+        if let Some(revision) = self.load_active_revision(base_key)? {
+            return self.load_v2_from_keyring(base_key, &revision).map(Some);
+        }
+        self.load_legacy_from_keyring(base_key)
+    }
+
+    fn write_optional_secret(
+        &self,
+        base_key: &str,
+        revision: &str,
+        entry: &str,
+        value: Option<&str>,
+        field: &str,
+    ) -> std::io::Result<()> {
+        if let Some(value) = value {
+            let key = keyring_revision_key(base_key, revision, entry);
+            self.save_secret_to_keyring(&key, value.as_bytes(), field)?;
+        }
+        Ok(())
+    }
+
+    fn delete_keyring_entry(&self, key: &str) -> std::io::Result<bool> {
+        self.keyring_store
+            .delete(KEYRING_SERVICE, key)
+            .map_err(|err| {
+                std::io::Error::other(format!("failed to delete auth from keyring: {err}"))
+            })
+    }
+
+    fn delete_v2_revision(&self, base_key: &str, revision: &str) -> std::io::Result<bool> {
+        let mut removed = false;
+        for entry in KEYRING_RECORD_ENTRIES {
+            let key = keyring_revision_key(base_key, revision, entry);
+            removed |= self.delete_keyring_entry(&key)?;
+        }
+        let account_id_key = keyring_revision_key(base_key, revision, KEYRING_ACCOUNT_ID_ENTRY);
+        removed |= self.delete_keyring_entry(&account_id_key)?;
+        Ok(removed)
+    }
+
+    fn delete_from_keyring_only(&self) -> std::io::Result<bool> {
+        let base_key = compute_store_key(&self.codex_home)?;
+        let mut removed = false;
+        if let Some(revision) = self.load_active_revision(&base_key)? {
+            removed |= self.delete_v2_revision(&base_key, &revision)?;
+            let active_key = keyring_layout_key(&base_key, KEYRING_ACTIVE_REVISION_ENTRY);
+            removed |= self.delete_keyring_entry(&active_key)?;
+        }
+        removed |= self.delete_keyring_entry(&base_key)?;
+        Ok(removed)
+    }
+
+    fn save_v2_to_keyring(&self, base_key: &str, auth: &AuthDotJson) -> std::io::Result<()> {
+        let previous_revision = match self.load_active_revision(base_key) {
+            Ok(revision) => revision,
+            Err(err) => {
+                warn!("failed to read previous auth revision from keyring: {err}");
+                None
+            }
+        };
+        let revision = next_keyring_revision();
+        let manifest = KeyringAuthManifest::from(auth);
+
+        self.write_optional_secret(
+            base_key,
+            &revision,
+            KEYRING_OPENAI_API_KEY_ENTRY,
+            auth.openai_api_key.as_deref(),
+            "OPENAI_API_KEY",
+        )?;
+        if let Some(tokens) = auth.tokens.as_ref() {
+            self.write_optional_secret(
+                base_key,
+                &revision,
+                KEYRING_ID_TOKEN_ENTRY,
+                Some(&tokens.id_token.raw_jwt),
+                "ID token",
+            )?;
+            self.write_optional_secret(
+                base_key,
+                &revision,
+                KEYRING_ACCESS_TOKEN_ENTRY,
+                Some(&tokens.access_token),
+                "access token",
+            )?;
+            self.write_optional_secret(
+                base_key,
+                &revision,
+                KEYRING_REFRESH_TOKEN_ENTRY,
+                Some(&tokens.refresh_token),
+                "refresh token",
+            )?;
+            self.write_optional_secret(
+                base_key,
+                &revision,
+                KEYRING_ACCOUNT_ID_ENTRY,
+                tokens.account_id.as_deref(),
+                "account ID",
+            )?;
+        }
+
+        let manifest_key = keyring_revision_key(base_key, &revision, KEYRING_MANIFEST_ENTRY);
+        let manifest_bytes = serde_json::to_vec(&manifest).map_err(std::io::Error::other)?;
+        self.save_secret_to_keyring(&manifest_key, &manifest_bytes, "auth manifest")?;
+
+        let active_key = keyring_layout_key(base_key, KEYRING_ACTIVE_REVISION_ENTRY);
+        self.save_secret_to_keyring(&active_key, revision.as_bytes(), "active auth revision")?;
+
+        if let Some(previous_revision) = previous_revision
+            && previous_revision != revision
+            && let Err(err) = self.delete_v2_revision(base_key, &previous_revision)
+        {
+            warn!("failed to remove stale auth revision from keyring: {err}");
+        }
+        if let Err(err) = self.delete_keyring_entry(base_key) {
+            warn!("failed to remove legacy auth entry from keyring: {err}");
+        }
+        Ok(())
+    }
 }
 
 impl AuthStorageBackend for KeyringAuthStorage {
@@ -199,10 +467,8 @@ impl AuthStorageBackend for KeyringAuthStorage {
     }
 
     fn save(&self, auth: &AuthDotJson) -> std::io::Result<()> {
-        let key = compute_store_key(&self.codex_home)?;
-        // 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)?;
+        let base_key = compute_store_key(&self.codex_home)?;
+        self.save_v2_to_keyring(&base_key, auth)?;
         if let Err(err) = delete_file_if_exists(&self.codex_home) {
             warn!("failed to remove CLI auth fallback file: {err}");
         }
@@ -210,13 +476,7 @@ impl AuthStorageBackend for KeyringAuthStorage {
     }
 
     fn delete(&self) -> std::io::Result<bool> {
-        let key = compute_store_key(&self.codex_home)?;
-        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}"))
-            })?;
+        let keyring_removed = self.delete_from_keyring_only()?;
         let file_removed = delete_file_if_exists(&self.codex_home)?;
         Ok(keyring_removed || file_removed)
     }
@@ -341,9 +601,58 @@ mod tests {
     use serde_json::json;
     use tempfile::tempdir;
 
+    use codex_keyring_store::CredentialStoreError;
     use codex_keyring_store::tests::MockKeyringStore;
     use keyring::Error as KeyringError;
 
+    #[derive(Clone, Debug)]
+    struct SaveSecretErrorKeyringStore {
+        inner: MockKeyringStore,
+    }
+
+    impl KeyringStore for SaveSecretErrorKeyringStore {
+        fn load(
+            &self,
+            service: &str,
+            account: &str,
+        ) -> Result<Option<String>, CredentialStoreError> {
+            self.inner.load(service, account)
+        }
+
+        fn load_secret(
+            &self,
+            service: &str,
+            account: &str,
+        ) -> Result<Option<Vec<u8>>, CredentialStoreError> {
+            self.inner.load_secret(service, account)
+        }
+
+        fn save(
+            &self,
+            service: &str,
+            account: &str,
+            value: &str,
+        ) -> Result<(), CredentialStoreError> {
+            self.inner.save(service, account, value)
+        }
+
+        fn save_secret(
+            &self,
+            _service: &str,
+            _account: &str,
+            _value: &[u8],
+        ) -> Result<(), CredentialStoreError> {
+            Err(CredentialStoreError::new(KeyringError::Invalid(
+                "error".into(),
+                "save".into(),
+            )))
+        }
+
+        fn delete(&self, service: &str, account: &str) -> Result<bool, CredentialStoreError> {
+            self.inner.delete(service, account)
+        }
+    }
+
     #[tokio::test]
     async fn file_storage_load_returns_auth_dot_json() -> anyhow::Result<()> {
         let codex_home = tempdir()?;
@@ -432,19 +741,19 @@ mod tests {
         Ok(())
     }
 
-    fn seed_keyring_and_fallback_auth_file_for_delete<F>(
-        mock_keyring: &MockKeyringStore,
+    fn seed_keyring_and_fallback_auth_file_for_delete(
+        storage: &KeyringAuthStorage,
         codex_home: &Path,
-        compute_key: F,
-    ) -> anyhow::Result<(String, PathBuf)>
-    where
-        F: FnOnce() -> std::io::Result<String>,
-    {
-        let key = compute_key()?;
-        mock_keyring.save(KEYRING_SERVICE, &key, "{}")?;
+        auth: &AuthDotJson,
+    ) -> anyhow::Result<(String, String, PathBuf)> {
+        storage.save(auth)?;
+        let base_key = compute_store_key(codex_home)?;
+        let revision = storage
+            .load_active_revision(&base_key)?
+            .context("active auth revision should exist")?;
         let auth_file = get_auth_file(codex_home);
         std::fs::write(&auth_file, "stale")?;
-        Ok((key, auth_file))
+        Ok((base_key, revision, auth_file))
     }
 
     fn seed_keyring_with_auth<F>(
@@ -463,15 +772,58 @@ mod tests {
 
     fn assert_keyring_saved_auth_and_removed_fallback(
         mock_keyring: &MockKeyringStore,
-        key: &str,
+        base_key: &str,
         codex_home: &Path,
         expected: &AuthDotJson,
     ) {
-        let saved_value = mock_keyring
-            .saved_value(key)
-            .expect("keyring entry should exist");
-        let expected_serialized = serde_json::to_string(expected).expect("serialize expected auth");
-        assert_eq!(saved_value, expected_serialized);
+        let active_key = keyring_layout_key(base_key, KEYRING_ACTIVE_REVISION_ENTRY);
+        let revision = mock_keyring
+            .saved_secret_utf8(&active_key)
+            .expect("active auth revision should exist");
+        assert!(
+            mock_keyring.saved_value(base_key).is_none(),
+            "legacy keyring entry should not be used for split auth storage"
+        );
+        let manifest_key = keyring_revision_key(base_key, &revision, KEYRING_MANIFEST_ENTRY);
+        let manifest_bytes = mock_keyring
+            .saved_secret(&manifest_key)
+            .expect("auth manifest should exist");
+        let manifest: KeyringAuthManifest =
+            serde_json::from_slice(&manifest_bytes).expect("manifest should deserialize");
+        assert_eq!(manifest, KeyringAuthManifest::from(expected));
+
+        let openai_api_key_key =
+            keyring_revision_key(base_key, &revision, KEYRING_OPENAI_API_KEY_ENTRY);
+        assert_eq!(
+            mock_keyring.saved_secret_utf8(&openai_api_key_key),
+            expected.openai_api_key
+        );
+
+        if let Some(tokens) = expected.tokens.as_ref() {
+            let id_token_key = keyring_revision_key(base_key, &revision, KEYRING_ID_TOKEN_ENTRY);
+            assert_eq!(
+                mock_keyring.saved_secret_utf8(&id_token_key),
+                Some(tokens.id_token.raw_jwt.clone())
+            );
+            let access_token_key =
+                keyring_revision_key(base_key, &revision, KEYRING_ACCESS_TOKEN_ENTRY);
+            assert_eq!(
+                mock_keyring.saved_secret_utf8(&access_token_key),
+                Some(tokens.access_token.clone())
+            );
+            let refresh_token_key =
+                keyring_revision_key(base_key, &revision, KEYRING_REFRESH_TOKEN_ENTRY);
+            assert_eq!(
+                mock_keyring.saved_secret_utf8(&refresh_token_key),
+                Some(tokens.refresh_token.clone())
+            );
+            let account_id_key =
+                keyring_revision_key(base_key, &revision, KEYRING_ACCOUNT_ID_ENTRY);
+            assert_eq!(
+                mock_keyring.saved_secret_utf8(&account_id_key),
+                tokens.account_id.clone()
+            );
+        }
         let auth_file = get_auth_file(codex_home);
         assert!(
             !auth_file.exists(),
@@ -520,7 +872,7 @@ mod tests {
     }
 
     #[test]
-    fn keyring_auth_storage_load_returns_deserialized_auth() -> anyhow::Result<()> {
+    fn keyring_auth_storage_load_supports_legacy_single_entry() -> anyhow::Result<()> {
         let codex_home = tempdir()?;
         let mock_keyring = MockKeyringStore::default();
         let storage = KeyringAuthStorage::new(
@@ -544,6 +896,21 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn keyring_auth_storage_load_returns_deserialized_v2_auth() -> anyhow::Result<()> {
+        let codex_home = tempdir()?;
+        let mock_keyring = MockKeyringStore::default();
+        let storage =
+            KeyringAuthStorage::new(codex_home.path().to_path_buf(), Arc::new(mock_keyring));
+        let expected = auth_with_prefix("split");
+
+        storage.save(&expected)?;
+
+        let loaded = storage.load()?;
+        assert_eq!(Some(expected), loaded);
+        Ok(())
+    }
+
     #[test]
     fn keyring_auth_storage_compute_store_key_for_home_directory() -> anyhow::Result<()> {
         let codex_home = PathBuf::from("~/.codex");
@@ -596,18 +963,29 @@ mod tests {
             codex_home.path().to_path_buf(),
             Arc::new(mock_keyring.clone()),
         );
-        let (key, auth_file) = seed_keyring_and_fallback_auth_file_for_delete(
-            &mock_keyring,
-            codex_home.path(),
-            || compute_store_key(codex_home.path()),
-        )?;
+        let auth = auth_with_prefix("delete");
+        let (base_key, revision, auth_file) =
+            seed_keyring_and_fallback_auth_file_for_delete(&storage, codex_home.path(), &auth)?;
 
         let removed = storage.delete()?;
 
         assert!(removed, "delete should report removal");
+        let active_key = keyring_layout_key(&base_key, KEYRING_ACTIVE_REVISION_ENTRY);
         assert!(
-            !mock_keyring.contains(&key),
-            "keyring entry should be removed"
+            !mock_keyring.contains(&active_key),
+            "active revision should be removed"
+        );
+        for entry in KEYRING_RECORD_ENTRIES {
+            let key = keyring_revision_key(&base_key, &revision, entry);
+            assert!(
+                !mock_keyring.contains(&key),
+                "keyring entry should be removed"
+            );
+        }
+        let account_id_key = keyring_revision_key(&base_key, &revision, KEYRING_ACCOUNT_ID_ENTRY);
+        assert!(
+            !mock_keyring.contains(&account_id_key),
+            "account id entry should be removed"
         );
         assert!(
             !auth_file.exists(),
@@ -662,7 +1040,11 @@ mod tests {
             Arc::new(mock_keyring.clone()),
         );
         let key = compute_store_key(codex_home.path())?;
-        mock_keyring.set_error(&key, KeyringError::Invalid("error".into(), "load".into()));
+        let active_key = keyring_layout_key(&key, KEYRING_ACTIVE_REVISION_ENTRY);
+        mock_keyring.set_error(
+            &active_key,
+            KeyringError::Invalid("error".into(), "load".into()),
+        );
 
         let expected = auth_with_prefix("fallback");
         storage.file_storage.save(&expected)?;
@@ -701,12 +1083,13 @@ mod tests {
     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()),
-        );
+        let failing_keyring = SaveSecretErrorKeyringStore {
+            inner: mock_keyring.clone(),
+        };
+        let storage =
+            AutoAuthStorage::new(codex_home.path().to_path_buf(), Arc::new(failing_keyring));
         let key = compute_store_key(codex_home.path())?;
-        mock_keyring.set_error(&key, KeyringError::Invalid("error".into(), "save".into()));
+        let active_key = keyring_layout_key(&key, KEYRING_ACTIVE_REVISION_ENTRY);
 
         let auth = auth_with_prefix("fallback");
         storage.save(&auth)?;
@@ -722,8 +1105,8 @@ mod tests {
             .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"
+            mock_keyring.saved_secret_utf8(&active_key).is_none(),
+            "keyring should not point to a saved auth revision when save fails"
         );
         Ok(())
     }
@@ -736,18 +1119,37 @@ mod tests {
             codex_home.path().to_path_buf(),
             Arc::new(mock_keyring.clone()),
         );
-        let (key, auth_file) = seed_keyring_and_fallback_auth_file_for_delete(
-            &mock_keyring,
+        let auth = auth_with_prefix("auto-delete");
+        let (base_key, revision, auth_file) = seed_keyring_and_fallback_auth_file_for_delete(
+            storage.keyring_storage.as_ref(),
             codex_home.path(),
-            || compute_store_key(codex_home.path()),
+            &auth,
         )?;
 
         let removed = storage.delete()?;
 
         assert!(removed, "delete should report removal");
         assert!(
-            !mock_keyring.contains(&key),
-            "keyring entry should be removed"
+            !mock_keyring.contains(&keyring_layout_key(
+                &base_key,
+                KEYRING_ACTIVE_REVISION_ENTRY
+            )),
+            "active revision should be removed"
+        );
+        for entry in KEYRING_RECORD_ENTRIES {
+            let key = keyring_revision_key(&base_key, &revision, entry);
+            assert!(
+                !mock_keyring.contains(&key),
+                "keyring entry should be removed"
+            );
+        }
+        assert!(
+            !mock_keyring.contains(&keyring_revision_key(
+                &base_key,
+                &revision,
+                KEYRING_ACCOUNT_ID_ENTRY
+            )),
+            "account id entry should be removed"
         );
         assert!(
             !auth_file.exists(),

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

@@ -41,7 +41,18 @@ impl Error for CredentialStoreError {}
 /// Shared credential store abstraction for keyring-backed implementations.
 pub trait KeyringStore: Debug + Send + Sync {
     fn load(&self, service: &str, account: &str) -> Result<Option<String>, CredentialStoreError>;
+    fn load_secret(
+        &self,
+        service: &str,
+        account: &str,
+    ) -> Result<Option<Vec<u8>>, CredentialStoreError>;
     fn save(&self, service: &str, account: &str, value: &str) -> Result<(), CredentialStoreError>;
+    fn save_secret(
+        &self,
+        service: &str,
+        account: &str,
+        value: &[u8],
+    ) -> Result<(), CredentialStoreError>;
     fn delete(&self, service: &str, account: &str) -> Result<bool, CredentialStoreError>;
 }
 
@@ -68,6 +79,31 @@ impl KeyringStore for DefaultKeyringStore {
         }
     }
 
+    fn load_secret(
+        &self,
+        service: &str,
+        account: &str,
+    ) -> Result<Option<Vec<u8>>, CredentialStoreError> {
+        trace!("keyring.load_secret start, service={service}, account={account}");
+        let entry = Entry::new(service, account).map_err(CredentialStoreError::new)?;
+        match entry.get_secret() {
+            Ok(secret) => {
+                trace!("keyring.load_secret success, service={service}, account={account}");
+                Ok(Some(secret))
+            }
+            Err(keyring::Error::NoEntry) => {
+                trace!("keyring.load_secret no entry, service={service}, account={account}");
+                Ok(None)
+            }
+            Err(error) => {
+                trace!(
+                    "keyring.load_secret error, service={service}, account={account}, error={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={}",
@@ -86,6 +122,31 @@ impl KeyringStore for DefaultKeyringStore {
         }
     }
 
+    fn save_secret(
+        &self,
+        service: &str,
+        account: &str,
+        value: &[u8],
+    ) -> Result<(), CredentialStoreError> {
+        trace!(
+            "keyring.save_secret start, service={service}, account={account}, value_len={}",
+            value.len()
+        );
+        let entry = Entry::new(service, account).map_err(CredentialStoreError::new)?;
+        match entry.set_secret(value) {
+            Ok(()) => {
+                trace!("keyring.save_secret success, service={service}, account={account}");
+                Ok(())
+            }
+            Err(error) => {
+                trace!(
+                    "keyring.save_secret error, service={service}, account={account}, error={error}"
+                );
+                Err(CredentialStoreError::new(error))
+            }
+        }
+    }
+
     fn delete(&self, service: &str, account: &str) -> Result<bool, CredentialStoreError> {
         trace!("keyring.delete start, service={service}, account={account}");
         let entry = Entry::new(service, account).map_err(CredentialStoreError::new)?;
@@ -145,6 +206,22 @@ pub mod tests {
             credential.get_password().ok()
         }
 
+        pub fn saved_secret(&self, account: &str) -> Option<Vec<u8>> {
+            let credential = {
+                let guard = self
+                    .credentials
+                    .lock()
+                    .unwrap_or_else(PoisonError::into_inner);
+                guard.get(account).cloned()
+            }?;
+            credential.get_secret().ok()
+        }
+
+        pub fn saved_secret_utf8(&self, account: &str) -> Option<String> {
+            let secret = self.saved_secret(account)?;
+            String::from_utf8(secret).ok()
+        }
+
         pub fn set_error(&self, account: &str, error: KeyringError) {
             let credential = self.credential(account);
             credential.set_error(error);
@@ -184,6 +261,30 @@ pub mod tests {
             }
         }
 
+        fn load_secret(
+            &self,
+            _service: &str,
+            account: &str,
+        ) -> Result<Option<Vec<u8>>, CredentialStoreError> {
+            let credential = {
+                let guard = self
+                    .credentials
+                    .lock()
+                    .unwrap_or_else(PoisonError::into_inner);
+                guard.get(account).cloned()
+            };
+
+            let Some(credential) = credential else {
+                return Ok(None);
+            };
+
+            match credential.get_secret() {
+                Ok(secret) => Ok(Some(secret)),
+                Err(KeyringError::NoEntry) => Ok(None),
+                Err(error) => Err(CredentialStoreError::new(error)),
+            }
+        }
+
         fn save(
             &self,
             _service: &str,
@@ -196,6 +297,18 @@ pub mod tests {
                 .map_err(CredentialStoreError::new)
         }
 
+        fn save_secret(
+            &self,
+            _service: &str,
+            account: &str,
+            value: &[u8],
+        ) -> Result<(), CredentialStoreError> {
+            let credential = self.credential(account);
+            credential
+                .set_secret(value)
+                .map_err(CredentialStoreError::new)
+        }
+
         fn delete(&self, _service: &str, account: &str) -> Result<bool, CredentialStoreError> {
             let credential = {
                 let guard = self