[login] auth storage to use new API

codex-rs/Cargo.lock

@@ -2176,6 +2176,9 @@ name = "codex-keyring-store"
 version = "0.0.0"
 dependencies = [
  "keyring",
+ "pretty_assertions",
+ "serde",
+ "serde_json",
  "tracing",
 ]
 

codex-rs/keyring-store/Cargo.toml

@@ -9,8 +9,13 @@ workspace = true
 
 [dependencies]
 keyring = { workspace = true, features = ["crypto-rust"] }
+serde = { workspace = true, features = ["derive"] }
+serde_json = { workspace = true }
 tracing = { workspace = true }
 
+[dev-dependencies]
+pretty_assertions = { workspace = true }
+
 [target.'cfg(target_os = "linux")'.dependencies]
 keyring = { workspace = true, features = ["linux-native-async-persistent"] }
 

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

@@ -0,0 +1,192 @@
+use crate::CredentialStoreError;
+use crate::KeyringStore;
+use serde_json::Value;
+use std::fmt;
+
+#[derive(Debug, Clone)]
+pub struct FullJsonKeyringError {
+    message: String,
+}
+
+pub type JsonKeyringError = FullJsonKeyringError;
+
+impl FullJsonKeyringError {
+    fn new(message: impl Into<String>) -> Self {
+        Self {
+            message: message.into(),
+        }
+    }
+}
+
+impl fmt::Display for FullJsonKeyringError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.message)
+    }
+}
+
+impl std::error::Error for FullJsonKeyringError {}
+
+pub fn load_json_from_keyring<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    base_key: &str,
+) -> Result<Option<Value>, JsonKeyringError> {
+    if let Some(bytes) = load_secret_from_keyring(keyring_store, service, base_key, "JSON record")?
+    {
+        let value = serde_json::from_slice(&bytes).map_err(|err| {
+            FullJsonKeyringError::new(format!(
+                "failed to deserialize JSON record from keyring secret: {err}"
+            ))
+        })?;
+        return Ok(Some(value));
+    }
+
+    match keyring_store.load(service, base_key) {
+        Ok(Some(serialized)) => serde_json::from_str(&serialized).map(Some).map_err(|err| {
+            FullJsonKeyringError::new(format!(
+                "failed to deserialize JSON record from keyring password: {err}"
+            ))
+        }),
+        Ok(None) => Ok(None),
+        Err(error) => Err(credential_store_error("load", "JSON record", error)),
+    }
+}
+
+pub fn save_json_to_keyring<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    base_key: &str,
+    value: &Value,
+) -> Result<(), JsonKeyringError> {
+    let bytes = serde_json::to_vec(value).map_err(|err| {
+        FullJsonKeyringError::new(format!("failed to serialize JSON record: {err}"))
+    })?;
+    save_secret_to_keyring(keyring_store, service, base_key, &bytes, "JSON record")
+}
+
+pub fn delete_json_from_keyring<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    base_key: &str,
+) -> Result<bool, JsonKeyringError> {
+    delete_keyring_entry(keyring_store, service, base_key, "JSON record")
+}
+
+fn load_secret_from_keyring<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    key: &str,
+    field: &str,
+) -> Result<Option<Vec<u8>>, FullJsonKeyringError> {
+    keyring_store
+        .load_secret(service, key)
+        .map_err(|err| credential_store_error("load", field, err))
+}
+
+fn save_secret_to_keyring<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    key: &str,
+    value: &[u8],
+    field: &str,
+) -> Result<(), FullJsonKeyringError> {
+    keyring_store
+        .save_secret(service, key, value)
+        .map_err(|err| credential_store_error("write", field, err))
+}
+
+fn delete_keyring_entry<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    key: &str,
+    field: &str,
+) -> Result<bool, FullJsonKeyringError> {
+    keyring_store
+        .delete(service, key)
+        .map_err(|err| credential_store_error("delete", field, err))
+}
+
+fn credential_store_error(
+    action: &str,
+    field: &str,
+    error: CredentialStoreError,
+) -> FullJsonKeyringError {
+    FullJsonKeyringError::new(format!(
+        "failed to {action} {field} in keyring: {}",
+        error.message()
+    ))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::delete_json_from_keyring;
+    use super::load_json_from_keyring;
+    use super::save_json_to_keyring;
+    use crate::KeyringStore;
+    use crate::tests::MockKeyringStore;
+    use pretty_assertions::assert_eq;
+    use serde_json::json;
+
+    const SERVICE: &str = "Test Service";
+    const BASE_KEY: &str = "base";
+
+    #[test]
+    fn json_storage_round_trips_using_full_backend() {
+        let store = MockKeyringStore::default();
+        let expected = json!({
+            "token": "secret",
+            "nested": {"id": 7}
+        });
+
+        save_json_to_keyring(&store, SERVICE, BASE_KEY, &expected).expect("JSON should save");
+
+        let loaded = load_json_from_keyring(&store, SERVICE, BASE_KEY)
+            .expect("JSON should load")
+            .expect("JSON should exist");
+        assert_eq!(loaded, expected);
+        assert_eq!(
+            store.saved_secret(BASE_KEY),
+            Some(serde_json::to_vec(&expected).expect("JSON should serialize")),
+        );
+    }
+
+    #[test]
+    fn json_storage_loads_legacy_single_entry() {
+        let store = MockKeyringStore::default();
+        let expected = json!({
+            "token": "secret",
+            "nested": {"id": 9}
+        });
+        store
+            .save(
+                SERVICE,
+                BASE_KEY,
+                &serde_json::to_string(&expected).expect("JSON should serialize"),
+            )
+            .expect("legacy JSON should save");
+
+        let loaded = load_json_from_keyring(&store, SERVICE, BASE_KEY)
+            .expect("JSON should load")
+            .expect("JSON should exist");
+        assert_eq!(loaded, expected);
+    }
+
+    #[test]
+    fn json_storage_delete_removes_full_entry() {
+        let store = MockKeyringStore::default();
+        let expected = json!({"current": true});
+
+        save_json_to_keyring(&store, SERVICE, BASE_KEY, &expected).expect("JSON should save");
+
+        let removed = delete_json_from_keyring(&store, SERVICE, BASE_KEY)
+            .expect("JSON delete should succeed");
+
+        assert!(removed);
+        assert!(
+            load_json_from_keyring(&store, SERVICE, BASE_KEY)
+                .expect("JSON load should succeed")
+                .is_none()
+        );
+        assert!(!store.contains(BASE_KEY));
+    }
+}

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

@@ -0,0 +1,736 @@
+use crate::CredentialStoreError;
+use crate::KeyringStore;
+use serde::Deserialize;
+use serde::Serialize;
+use serde_json::Map;
+use serde_json::Value;
+use std::fmt;
+use std::fmt::Write as _;
+use tracing::warn;
+
+const LAYOUT_VERSION: &str = "v1";
+const MANIFEST_ENTRY: &str = "manifest";
+const VALUE_ENTRY_PREFIX: &str = "value";
+const ROOT_PATH_SENTINEL: &str = "root";
+
+#[derive(Debug, Clone)]
+pub struct SplitJsonKeyringError {
+    message: String,
+}
+
+pub type JsonKeyringError = SplitJsonKeyringError;
+
+impl SplitJsonKeyringError {
+    fn new(message: impl Into<String>) -> Self {
+        Self {
+            message: message.into(),
+        }
+    }
+}
+
+impl fmt::Display for SplitJsonKeyringError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.message)
+    }
+}
+
+impl std::error::Error for SplitJsonKeyringError {}
+
+#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq)]
+#[serde(rename_all = "lowercase")]
+enum JsonNodeKind {
+    Null,
+    Bool,
+    Number,
+    String,
+    Object,
+    Array,
+}
+
+impl JsonNodeKind {
+    fn from_value(value: &Value) -> Self {
+        match value {
+            Value::Null => Self::Null,
+            Value::Bool(_) => Self::Bool,
+            Value::Number(_) => Self::Number,
+            Value::String(_) => Self::String,
+            Value::Object(_) => Self::Object,
+            Value::Array(_) => Self::Array,
+        }
+    }
+
+    fn is_container(self) -> bool {
+        matches!(self, Self::Object | Self::Array)
+    }
+
+    fn empty_value(self) -> Option<Value> {
+        match self {
+            Self::Object => Some(Value::Object(Map::new())),
+            Self::Array => Some(Value::Array(Vec::new())),
+            Self::Null | Self::Bool | Self::Number | Self::String => None,
+        }
+    }
+}
+
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
+struct SplitJsonNode {
+    path: String,
+    kind: JsonNodeKind,
+}
+
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
+struct SplitJsonManifest {
+    nodes: Vec<SplitJsonNode>,
+}
+
+type SplitJsonLeafValues = Vec<(String, Vec<u8>)>;
+
+pub fn load_json_from_keyring<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    base_key: &str,
+) -> Result<Option<Value>, JsonKeyringError> {
+    let Some(manifest) = load_manifest(keyring_store, service, base_key)? else {
+        return Ok(None);
+    };
+    inflate_split_json(keyring_store, service, base_key, &manifest).map(Some)
+}
+
+pub fn save_json_to_keyring<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    base_key: &str,
+    value: &Value,
+) -> Result<(), JsonKeyringError> {
+    let previous_manifest = match load_manifest(keyring_store, service, base_key) {
+        Ok(manifest) => manifest,
+        Err(err) => {
+            warn!("failed to read previous split JSON manifest from keyring: {err}");
+            None
+        }
+    };
+    let (manifest, leaf_values) = flatten_split_json(value)?;
+    let current_scalar_paths = manifest
+        .nodes
+        .iter()
+        .filter(|node| !node.kind.is_container())
+        .map(|node| node.path.as_str())
+        .collect::<std::collections::HashSet<_>>();
+
+    for (path, bytes) in leaf_values {
+        let key = value_key(base_key, &path);
+        save_secret_to_keyring(
+            keyring_store,
+            service,
+            &key,
+            &bytes,
+            &format!("JSON value at {path}"),
+        )?;
+    }
+
+    let manifest_key = layout_key(base_key, MANIFEST_ENTRY);
+    let manifest_bytes = serde_json::to_vec(&manifest).map_err(|err| {
+        SplitJsonKeyringError::new(format!("failed to serialize JSON manifest: {err}"))
+    })?;
+    save_secret_to_keyring(
+        keyring_store,
+        service,
+        &manifest_key,
+        &manifest_bytes,
+        "JSON manifest",
+    )?;
+
+    if let Some(previous_manifest) = previous_manifest {
+        for node in previous_manifest.nodes {
+            if node.kind.is_container() || current_scalar_paths.contains(node.path.as_str()) {
+                continue;
+            }
+            let key = value_key(base_key, &node.path);
+            if let Err(err) = delete_keyring_entry(
+                keyring_store,
+                service,
+                &key,
+                &format!("stale JSON value at {}", node.path),
+            ) {
+                warn!("failed to remove stale split JSON value from keyring: {err}");
+            }
+        }
+    }
+
+    Ok(())
+}
+
+pub fn delete_json_from_keyring<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    base_key: &str,
+) -> Result<bool, JsonKeyringError> {
+    let Some(manifest) = load_manifest(keyring_store, service, base_key)? else {
+        return Ok(false);
+    };
+
+    let mut removed = false;
+    for node in manifest.nodes {
+        if node.kind.is_container() {
+            continue;
+        }
+        let key = value_key(base_key, &node.path);
+        removed |= delete_keyring_entry(
+            keyring_store,
+            service,
+            &key,
+            &format!("JSON value at {}", node.path),
+        )?;
+    }
+
+    let manifest_key = layout_key(base_key, MANIFEST_ENTRY);
+    removed |= delete_keyring_entry(keyring_store, service, &manifest_key, "JSON manifest")?;
+    Ok(removed)
+}
+
+fn flatten_split_json(
+    value: &Value,
+) -> Result<(SplitJsonManifest, SplitJsonLeafValues), SplitJsonKeyringError> {
+    let mut nodes = Vec::new();
+    let mut leaf_values = Vec::new();
+    collect_nodes("", value, &mut nodes, &mut leaf_values)?;
+    nodes.sort_by(|left, right| {
+        path_depth(&left.path)
+            .cmp(&path_depth(&right.path))
+            .then_with(|| left.path.cmp(&right.path))
+    });
+    leaf_values.sort_by(|left, right| left.0.cmp(&right.0));
+    Ok((SplitJsonManifest { nodes }, leaf_values))
+}
+
+fn collect_nodes(
+    path: &str,
+    value: &Value,
+    nodes: &mut Vec<SplitJsonNode>,
+    leaf_values: &mut SplitJsonLeafValues,
+) -> Result<(), SplitJsonKeyringError> {
+    let kind = JsonNodeKind::from_value(value);
+    nodes.push(SplitJsonNode {
+        path: path.to_string(),
+        kind,
+    });
+
+    match value {
+        Value::Object(map) => {
+            let mut keys = map.keys().cloned().collect::<Vec<_>>();
+            keys.sort();
+            for key in keys {
+                let child_path = append_json_pointer_token(path, &key);
+                let child_value = map.get(&key).ok_or_else(|| {
+                    SplitJsonKeyringError::new(format!(
+                        "missing object value for path {child_path}"
+                    ))
+                })?;
+                collect_nodes(&child_path, child_value, nodes, leaf_values)?;
+            }
+        }
+        Value::Array(items) => {
+            for (index, item) in items.iter().enumerate() {
+                let child_path = append_json_pointer_token(path, &index.to_string());
+                collect_nodes(&child_path, item, nodes, leaf_values)?;
+            }
+        }
+        Value::Null | Value::Bool(_) | Value::Number(_) | Value::String(_) => {
+            let bytes = serde_json::to_vec(value).map_err(|err| {
+                SplitJsonKeyringError::new(format!(
+                    "failed to serialize JSON value at {path}: {err}"
+                ))
+            })?;
+            leaf_values.push((path.to_string(), bytes));
+        }
+    }
+
+    Ok(())
+}
+
+fn inflate_split_json<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    base_key: &str,
+    manifest: &SplitJsonManifest,
+) -> Result<Value, SplitJsonKeyringError> {
+    let root_node = manifest
+        .nodes
+        .iter()
+        .find(|node| node.path.is_empty())
+        .ok_or_else(|| SplitJsonKeyringError::new("missing root JSON node in keyring manifest"))?;
+
+    let mut result = if let Some(value) = root_node.kind.empty_value() {
+        value
+    } else {
+        load_value(keyring_store, service, base_key, "")?
+    };
+
+    let mut nodes = manifest.nodes.clone();
+    nodes.sort_by(|left, right| {
+        path_depth(&left.path)
+            .cmp(&path_depth(&right.path))
+            .then_with(|| left.path.cmp(&right.path))
+    });
+
+    for node in nodes.into_iter().filter(|node| !node.path.is_empty()) {
+        let value = if let Some(value) = node.kind.empty_value() {
+            value
+        } else {
+            load_value(keyring_store, service, base_key, &node.path)?
+        };
+        insert_value_at_pointer(&mut result, &node.path, value)?;
+    }
+
+    Ok(result)
+}
+
+fn load_value<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    base_key: &str,
+    path: &str,
+) -> Result<Value, SplitJsonKeyringError> {
+    let key = value_key(base_key, path);
+    let bytes = load_secret_from_keyring(
+        keyring_store,
+        service,
+        &key,
+        &format!("JSON value at {path}"),
+    )?
+    .ok_or_else(|| {
+        SplitJsonKeyringError::new(format!("missing JSON value at {path} in keyring"))
+    })?;
+    serde_json::from_slice(&bytes).map_err(|err| {
+        SplitJsonKeyringError::new(format!("failed to deserialize JSON value at {path}: {err}"))
+    })
+}
+
+fn insert_value_at_pointer(
+    root: &mut Value,
+    pointer: &str,
+    value: Value,
+) -> Result<(), SplitJsonKeyringError> {
+    if pointer.is_empty() {
+        *root = value;
+        return Ok(());
+    }
+
+    let tokens = decode_json_pointer(pointer)?;
+    let Some((last, parents)) = tokens.split_last() else {
+        return Err(SplitJsonKeyringError::new(
+            "missing JSON pointer path tokens",
+        ));
+    };
+
+    let mut current = root;
+    for token in parents {
+        current = match current {
+            Value::Object(map) => map.get_mut(token).ok_or_else(|| {
+                SplitJsonKeyringError::new(format!(
+                    "missing parent object entry for JSON pointer {pointer}"
+                ))
+            })?,
+            Value::Array(items) => {
+                let index = parse_array_index(token, pointer)?;
+                items.get_mut(index).ok_or_else(|| {
+                    SplitJsonKeyringError::new(format!(
+                        "missing parent array entry for JSON pointer {pointer}"
+                    ))
+                })?
+            }
+            Value::Null | Value::Bool(_) | Value::Number(_) | Value::String(_) => {
+                return Err(SplitJsonKeyringError::new(format!(
+                    "encountered scalar while walking JSON pointer {pointer}"
+                )));
+            }
+        };
+    }
+
+    match current {
+        Value::Object(map) => {
+            map.insert(last.to_string(), value);
+            Ok(())
+        }
+        Value::Array(items) => {
+            let index = parse_array_index(last, pointer)?;
+            if index >= items.len() {
+                items.resize(index + 1, Value::Null);
+            }
+            items[index] = value;
+            Ok(())
+        }
+        Value::Null | Value::Bool(_) | Value::Number(_) | Value::String(_) => {
+            Err(SplitJsonKeyringError::new(format!(
+                "encountered scalar while assigning JSON pointer {pointer}"
+            )))
+        }
+    }
+}
+
+fn load_manifest<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    base_key: &str,
+) -> Result<Option<SplitJsonManifest>, SplitJsonKeyringError> {
+    let manifest_key = layout_key(base_key, MANIFEST_ENTRY);
+    let Some(bytes) =
+        load_secret_from_keyring(keyring_store, service, &manifest_key, "JSON manifest")?
+    else {
+        return Ok(None);
+    };
+    let manifest: SplitJsonManifest = serde_json::from_slice(&bytes).map_err(|err| {
+        SplitJsonKeyringError::new(format!("failed to deserialize JSON manifest: {err}"))
+    })?;
+    if manifest.nodes.is_empty() {
+        return Err(SplitJsonKeyringError::new("JSON manifest is empty"));
+    }
+    Ok(Some(manifest))
+}
+
+fn load_secret_from_keyring<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    key: &str,
+    field: &str,
+) -> Result<Option<Vec<u8>>, SplitJsonKeyringError> {
+    keyring_store
+        .load_secret(service, key)
+        .map_err(|err| credential_store_error("load", field, err))
+}
+
+fn save_secret_to_keyring<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    key: &str,
+    value: &[u8],
+    field: &str,
+) -> Result<(), SplitJsonKeyringError> {
+    keyring_store
+        .save_secret(service, key, value)
+        .map_err(|err| credential_store_error("write", field, err))
+}
+
+fn delete_keyring_entry<K: KeyringStore + ?Sized>(
+    keyring_store: &K,
+    service: &str,
+    key: &str,
+    field: &str,
+) -> Result<bool, SplitJsonKeyringError> {
+    keyring_store
+        .delete(service, key)
+        .map_err(|err| credential_store_error("delete", field, err))
+}
+
+fn credential_store_error(
+    action: &str,
+    field: &str,
+    error: CredentialStoreError,
+) -> SplitJsonKeyringError {
+    SplitJsonKeyringError::new(format!(
+        "failed to {action} {field} in keyring: {}",
+        error.message()
+    ))
+}
+
+fn layout_key(base_key: &str, suffix: &str) -> String {
+    format!("{base_key}|{LAYOUT_VERSION}|{suffix}")
+}
+
+fn value_key(base_key: &str, path: &str) -> String {
+    let encoded_path = encode_path(path);
+    layout_key(base_key, &format!("{VALUE_ENTRY_PREFIX}|{encoded_path}"))
+}
+
+fn encode_path(path: &str) -> String {
+    if path.is_empty() {
+        return ROOT_PATH_SENTINEL.to_string();
+    }
+
+    let mut encoded = String::with_capacity(path.len() * 2);
+    for byte in path.as_bytes() {
+        let _ = write!(&mut encoded, "{byte:02x}");
+    }
+    encoded
+}
+
+fn append_json_pointer_token(path: &str, token: &str) -> String {
+    let escaped = token.replace('~', "~0").replace('/', "~1");
+    if path.is_empty() {
+        format!("/{escaped}")
+    } else {
+        format!("{path}/{escaped}")
+    }
+}
+
+fn decode_json_pointer(pointer: &str) -> Result<Vec<String>, SplitJsonKeyringError> {
+    if pointer.is_empty() {
+        return Ok(Vec::new());
+    }
+    if !pointer.starts_with('/') {
+        return Err(SplitJsonKeyringError::new(format!(
+            "invalid JSON pointer {pointer}: expected leading slash"
+        )));
+    }
+
+    pointer[1..]
+        .split('/')
+        .map(unescape_json_pointer_token)
+        .collect()
+}
+
+fn unescape_json_pointer_token(token: &str) -> Result<String, SplitJsonKeyringError> {
+    let mut result = String::with_capacity(token.len());
+    let mut chars = token.chars();
+
+    while let Some(ch) = chars.next() {
+        if ch != '~' {
+            result.push(ch);
+            continue;
+        }
+
+        match chars.next() {
+            Some('0') => result.push('~'),
+            Some('1') => result.push('/'),
+            Some(other) => {
+                return Err(SplitJsonKeyringError::new(format!(
+                    "invalid JSON pointer escape sequence ~{other}"
+                )));
+            }
+            None => {
+                return Err(SplitJsonKeyringError::new(
+                    "invalid JSON pointer escape sequence at end of token",
+                ));
+            }
+        }
+    }
+
+    Ok(result)
+}
+
+fn parse_array_index(token: &str, pointer: &str) -> Result<usize, SplitJsonKeyringError> {
+    token.parse::<usize>().map_err(|err| {
+        SplitJsonKeyringError::new(format!(
+            "invalid array index '{token}' in JSON pointer {pointer}: {err}"
+        ))
+    })
+}
+
+fn path_depth(path: &str) -> usize {
+    path.chars().filter(|ch| *ch == '/').count()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::LAYOUT_VERSION;
+    use super::MANIFEST_ENTRY;
+    use super::delete_json_from_keyring;
+    use super::layout_key;
+    use super::load_json_from_keyring;
+    use super::save_json_to_keyring;
+    use super::value_key;
+    use crate::KeyringStore;
+    use crate::tests::MockKeyringStore;
+    use pretty_assertions::assert_eq;
+    use serde_json::json;
+
+    const SERVICE: &str = "Test Service";
+    const BASE_KEY: &str = "base";
+
+    #[test]
+    fn json_storage_round_trips_using_split_backend() {
+        let store = MockKeyringStore::default();
+        let expected = json!({
+            "token": "secret",
+            "nested": {"id": 7}
+        });
+
+        save_json_to_keyring(&store, SERVICE, BASE_KEY, &expected).expect("JSON should save");
+
+        let loaded = load_json_from_keyring(&store, SERVICE, BASE_KEY)
+            .expect("JSON should load")
+            .expect("JSON should exist");
+        assert_eq!(loaded, expected);
+        assert!(
+            store.saved_secret(BASE_KEY).is_none(),
+            "split storage should not write the full record under the base key"
+        );
+        assert!(
+            store.contains(&layout_key(BASE_KEY, MANIFEST_ENTRY)),
+            "split storage should write manifest metadata"
+        );
+    }
+
+    #[test]
+    fn json_storage_does_not_load_legacy_single_entry() {
+        let store = MockKeyringStore::default();
+        let expected = json!({
+            "token": "secret",
+            "nested": {"id": 9}
+        });
+        store
+            .save(
+                SERVICE,
+                BASE_KEY,
+                &serde_json::to_string(&expected).expect("JSON should serialize"),
+            )
+            .expect("legacy JSON should save");
+
+        let loaded = load_json_from_keyring(&store, SERVICE, BASE_KEY).expect("JSON should load");
+        assert_eq!(loaded, None);
+    }
+
+    #[test]
+    fn json_storage_save_preserves_legacy_single_entry() {
+        let store = MockKeyringStore::default();
+        let current = json!({"current": true});
+        let legacy = json!({"legacy": true});
+        store
+            .save(
+                SERVICE,
+                BASE_KEY,
+                &serde_json::to_string(&legacy).expect("JSON should serialize"),
+            )
+            .expect("legacy JSON should save");
+
+        save_json_to_keyring(&store, SERVICE, BASE_KEY, &current).expect("JSON should save");
+
+        let loaded = load_json_from_keyring(&store, SERVICE, BASE_KEY)
+            .expect("JSON should load")
+            .expect("JSON should exist");
+        assert_eq!(loaded, current);
+        assert_eq!(
+            store.saved_value(BASE_KEY),
+            Some(serde_json::to_string(&legacy).expect("JSON should serialize"))
+        );
+    }
+
+    #[test]
+    fn json_storage_delete_removes_only_split_entries() {
+        let store = MockKeyringStore::default();
+        let current = json!({"current": true});
+        let legacy = json!({"legacy": true});
+        store
+            .save(
+                SERVICE,
+                BASE_KEY,
+                &serde_json::to_string(&legacy).expect("JSON should serialize"),
+            )
+            .expect("legacy JSON should save");
+        save_json_to_keyring(&store, SERVICE, BASE_KEY, &current).expect("JSON should save");
+
+        let removed = delete_json_from_keyring(&store, SERVICE, BASE_KEY)
+            .expect("JSON delete should succeed");
+
+        assert!(removed);
+        assert!(
+            load_json_from_keyring(&store, SERVICE, BASE_KEY)
+                .expect("JSON load should succeed")
+                .is_none()
+        );
+        assert!(store.contains(BASE_KEY));
+        assert!(!store.contains(&layout_key(BASE_KEY, MANIFEST_ENTRY)));
+    }
+
+    #[test]
+    fn split_json_round_trips_nested_values() {
+        let store = MockKeyringStore::default();
+        let expected = json!({
+            "name": "codex",
+            "enabled": true,
+            "count": 3,
+            "nested": {
+                "items": [null, {"hello": "world"}],
+                "slash/key": "~value~",
+            },
+        });
+
+        save_json_to_keyring(&store, SERVICE, BASE_KEY, &expected).expect("split JSON should save");
+
+        let loaded = load_json_from_keyring(&store, SERVICE, BASE_KEY)
+            .expect("split JSON should load")
+            .expect("split JSON should exist");
+        assert_eq!(loaded, expected);
+    }
+
+    #[test]
+    fn split_json_supports_scalar_root_values() {
+        let store = MockKeyringStore::default();
+        let expected = json!("value");
+
+        save_json_to_keyring(&store, SERVICE, BASE_KEY, &expected).expect("split JSON should save");
+
+        let root_value_key = value_key(BASE_KEY, "");
+        assert_eq!(
+            store.saved_secret_utf8(&root_value_key),
+            Some("\"value\"".to_string())
+        );
+
+        let loaded = load_json_from_keyring(&store, SERVICE, BASE_KEY)
+            .expect("split JSON should load")
+            .expect("split JSON should exist");
+        assert_eq!(loaded, expected);
+    }
+
+    #[test]
+    fn split_json_delete_removes_saved_entries() {
+        let store = MockKeyringStore::default();
+        let expected = json!({
+            "token": "secret",
+            "nested": {
+                "id": 123,
+            },
+        });
+
+        save_json_to_keyring(&store, SERVICE, BASE_KEY, &expected).expect("split JSON should save");
+
+        let manifest_key = layout_key(BASE_KEY, MANIFEST_ENTRY);
+        let token_key = value_key(BASE_KEY, "/token");
+        let nested_id_key = value_key(BASE_KEY, "/nested/id");
+
+        let removed = delete_json_from_keyring(&store, SERVICE, BASE_KEY)
+            .expect("split JSON delete should succeed");
+
+        assert!(removed);
+        assert!(!store.contains(&manifest_key));
+        assert!(!store.contains(&token_key));
+        assert!(!store.contains(&nested_id_key));
+    }
+
+    #[test]
+    fn split_json_save_replaces_previous_values() {
+        let store = MockKeyringStore::default();
+        let first = json!({"value": "first", "stale": true});
+        let second = json!({"value": "second", "extra": 1});
+
+        save_json_to_keyring(&store, SERVICE, BASE_KEY, &first)
+            .expect("first split JSON save should succeed");
+        let manifest_key = layout_key(BASE_KEY, MANIFEST_ENTRY);
+        let stale_value_key = value_key(BASE_KEY, "/stale");
+        assert!(store.contains(&manifest_key));
+        assert!(store.contains(&stale_value_key));
+
+        save_json_to_keyring(&store, SERVICE, BASE_KEY, &second)
+            .expect("second split JSON save should succeed");
+        assert!(!store.contains(&stale_value_key));
+        assert!(store.contains(&manifest_key));
+        assert_eq!(
+            store.saved_secret_utf8(&value_key(BASE_KEY, "/value")),
+            Some("\"second\"".to_string())
+        );
+        assert_eq!(
+            store.saved_secret_utf8(&value_key(BASE_KEY, "/extra")),
+            Some("1".to_string())
+        );
+
+        let loaded = load_json_from_keyring(&store, SERVICE, BASE_KEY)
+            .expect("split JSON should load")
+            .expect("split JSON should exist");
+        assert_eq!(loaded, second);
+    }
+
+    #[test]
+    fn split_json_uses_distinct_layout_version() {
+        assert_eq!(LAYOUT_VERSION, "v1");
+    }
+}

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

@@ -5,6 +5,19 @@ use std::fmt;
 use std::fmt::Debug;
 use tracing::trace;
 
+#[cfg(not(windows))]
+#[path = "json_store_full.rs"]
+mod json_store;
+
+#[cfg(windows)]
+#[path = "json_store_split.rs"]
+mod json_store;
+
+pub use json_store::JsonKeyringError;
+pub use json_store::delete_json_from_keyring;
+pub use json_store::load_json_from_keyring;
+pub use json_store::save_json_to_keyring;
+
 #[derive(Debug)]
 pub enum CredentialStoreError {
     Other(KeyringError),
@@ -41,7 +54,20 @@ 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 +94,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 +137,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 +221,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 +276,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 +312,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

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

@@ -23,6 +23,9 @@ use crate::token_data::TokenData;
 use codex_app_server_protocol::AuthMode;
 use codex_keyring_store::DefaultKeyringStore;
 use codex_keyring_store::KeyringStore;
+use codex_keyring_store::delete_json_from_keyring;
+use codex_keyring_store::load_json_from_keyring;
+use codex_keyring_store::save_json_to_keyring;
 use once_cell::sync::Lazy;
 
 /// Determine where Codex should store CLI auth credentials.
@@ -162,62 +165,58 @@ impl KeyringAuthStorage {
         }
     }
 
-    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}"
-                ))
-            }),
-            Ok(None) => Ok(None),
-            Err(error) => Err(std::io::Error::other(format!(
-                "failed to load CLI auth from keyring: {}",
-                error.message()
-            ))),
-        }
-    }
+    fn load_auth_from_keyring(&self, base_key: &str) -> std::io::Result<Option<AuthDotJson>> {
+        let Some(value) =
+            load_json_from_keyring(self.keyring_store.as_ref(), KEYRING_SERVICE, base_key)
+                .map_err(|err| {
+                    std::io::Error::other(format!("failed to load CLI auth from keyring: {err}"))
+                })?
+        else {
+            return 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))
-            }
-        }
+        serde_json::from_value(value).map(Some).map_err(|err| {
+            std::io::Error::other(format!(
+                "failed to deserialize CLI auth from keyring: {err}"
+            ))
+        })
     }
 }
 
 impl AuthStorageBackend for KeyringAuthStorage {
     fn load(&self) -> std::io::Result<Option<AuthDotJson>> {
         let key = compute_store_key(&self.codex_home)?;
-        self.load_from_keyring(&key)
+        self.load_auth_from_keyring(&key)
     }
 
     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)?;
+        let value = serde_json::to_value(auth).map_err(std::io::Error::other)?;
+
+        save_json_to_keyring(
+            self.keyring_store.as_ref(),
+            KEYRING_SERVICE,
+            &base_key,
+            &value,
+        )
+        .map_err(|err| std::io::Error::other(format!("failed to write auth to keyring: {err}")))?;
+
         if let Err(err) = delete_file_if_exists(&self.codex_home) {
             warn!("failed to remove CLI auth fallback file: {err}");
         }
+
         Ok(())
     }
 
     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 base_key = compute_store_key(&self.codex_home)?;
+        let keyring_removed =
+            delete_json_from_keyring(self.keyring_store.as_ref(), KEYRING_SERVICE, &base_key)
+                .map_err(|err| {
+                    std::io::Error::other(format!("failed to delete auth from keyring: {err}"))
+                })?;
         let file_removed = delete_file_if_exists(&self.codex_home)?;
+
         Ok(keyring_removed || file_removed)
     }
 }

codex-rs/login/src/auth/storage_tests.rs

@@ -6,9 +6,88 @@ use pretty_assertions::assert_eq;
 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)
+    }
+}
+
+#[derive(Clone, Debug)]
+struct LoadSecretErrorKeyringStore {
+    inner: MockKeyringStore,
+}
+
+impl KeyringStore for LoadSecretErrorKeyringStore {
+    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> {
+        Err(CredentialStoreError::new(KeyringError::Invalid(
+            "error".into(),
+            "load".into(),
+        )))
+    }
+
+    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> {
+        self.inner.save_secret(service, account, value)
+    }
+
+    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()?;
@@ -97,19 +176,16 @@ fn ephemeral_storage_save_load_delete_is_in_memory_only() -> anyhow::Result<()>
     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, PathBuf)> {
+    storage.save(auth)?;
+    let base_key = compute_store_key(codex_home)?;
     let auth_file = get_auth_file(codex_home);
     std::fs::write(&auth_file, "stale")?;
-    Ok((key, auth_file))
+    Ok((base_key, auth_file))
 }
 
 fn seed_keyring_with_auth<F>(
@@ -128,15 +204,26 @@ where
 
 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 expected_json = serde_json::to_value(expected).expect("auth should serialize");
+    let loaded = load_json_from_keyring(mock_keyring, KEYRING_SERVICE, base_key)
+        .expect("auth should load from keyring")
+        .expect("auth should exist");
+    assert_eq!(loaded, expected_json);
+    #[cfg(windows)]
+    assert!(
+        mock_keyring.saved_secret(base_key).is_none(),
+        "windows should store auth using split keyring entries"
+    );
+    #[cfg(not(windows))]
+    assert_eq!(
+        mock_keyring.saved_secret(base_key),
+        Some(serde_json::to_vec(&expected_json).expect("auth should serialize")),
+        "non-windows should store auth as one JSON secret"
+    );
     let auth_file = get_auth_file(codex_home);
     assert!(
         !auth_file.exists(),
@@ -185,7 +272,7 @@ fn auth_with_prefix(prefix: &str) -> AuthDotJson {
 }
 
 #[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(
@@ -204,6 +291,29 @@ fn keyring_auth_storage_load_returns_deserialized_auth() -> anyhow::Result<()> {
         &expected,
     )?;
 
+    let loaded = storage.load()?;
+
+    #[cfg(not(windows))]
+    {
+        assert_eq!(Some(expected), loaded);
+    }
+
+    #[cfg(windows)]
+    {
+        assert_eq!(None, loaded);
+    }
+    Ok(())
+}
+
+#[test]
+fn keyring_auth_storage_load_returns_deserialized_keyring_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("keyring");
+
+    storage.save(&expected)?;
+
     let loaded = storage.load()?;
     assert_eq!(Some(expected), loaded);
     Ok(())
@@ -256,17 +366,16 @@ fn keyring_auth_storage_delete_removes_keyring_and_file() -> anyhow::Result<()>
         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, 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");
     assert!(
-        !mock_keyring.contains(&key),
-        "keyring entry should be removed"
+        load_json_from_keyring(&mock_keyring, KEYRING_SERVICE, &base_key)?.is_none(),
+        "keyring auth should be removed"
     );
     assert!(
         !auth_file.exists(),
@@ -279,16 +388,9 @@ fn keyring_auth_storage_delete_removes_keyring_and_file() -> anyhow::Result<()>
 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()),
-    );
+    let storage = AutoAuthStorage::new(codex_home.path().to_path_buf(), Arc::new(mock_keyring));
     let keyring_auth = auth_with_prefix("keyring");
-    seed_keyring_with_auth(
-        &mock_keyring,
-        || compute_store_key(codex_home.path()),
-        &keyring_auth,
-    )?;
+    storage.keyring_storage.save(&keyring_auth)?;
 
     let file_auth = auth_with_prefix("file");
     storage.file_storage.save(&file_auth)?;
@@ -316,12 +418,10 @@ fn auto_auth_storage_load_uses_file_when_keyring_empty() -> anyhow::Result<()> {
 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()),
-    );
-    let key = compute_store_key(codex_home.path())?;
-    mock_keyring.set_error(&key, KeyringError::Invalid("error".into(), "load".into()));
+    let failing_keyring = LoadSecretErrorKeyringStore {
+        inner: mock_keyring,
+    };
+    let storage = AutoAuthStorage::new(codex_home.path().to_path_buf(), Arc::new(failing_keyring));
 
     let expected = auth_with_prefix("fallback");
     storage.file_storage.save(&expected)?;
@@ -360,12 +460,11 @@ fn auto_auth_storage_save_prefers_keyring() -> anyhow::Result<()> {
 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 auth = auth_with_prefix("fallback");
     storage.save(&auth)?;
@@ -381,8 +480,8 @@ fn auto_auth_storage_save_falls_back_when_keyring_errors() -> anyhow::Result<()>
         .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"
+        load_json_from_keyring(&mock_keyring, KEYRING_SERVICE, &key)?.is_none(),
+        "keyring should not point to saved auth when save fails"
     );
     Ok(())
 }
@@ -395,17 +494,19 @@ fn auto_auth_storage_delete_removes_keyring_and_file() -> anyhow::Result<()> {
         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("auto-delete");
+    let (base_key, auth_file) = seed_keyring_and_fallback_auth_file_for_delete(
+        storage.keyring_storage.as_ref(),
+        codex_home.path(),
+        &auth,
+    )?;
 
     let removed = storage.delete()?;
 
     assert!(removed, "delete should report removal");
     assert!(
-        !mock_keyring.contains(&key),
-        "keyring entry should be removed"
+        load_json_from_keyring(&mock_keyring, KEYRING_SERVICE, &base_key)?.is_none(),
+        "keyring auth should be removed"
     );
     assert!(
         !auth_file.exists(),