Compare commits

...

5 Commits

Author SHA1 Message Date
mikhail-oai
73f59d88f8 revision timestamp is not necessary 2026-03-16 14:47:47 -04:00
mikhail-oai
41dd142dbc windows gating 2026-03-16 14:47:47 -04:00
mikhail-oai
89486d6d59 scope json breakout to windows only 2026-03-16 14:47:46 -04:00
mikhail-oai
7ea33aad24 generalize json serialization and unify MCP/Auth codepaths. 2026-03-16 14:47:46 -04:00
mikhail-oai
89323e9ebc Use binary split-entry keyring storage for auth to avoid Windows credential size failures 2026-03-16 14:47:46 -04:00
7 changed files with 1376 additions and 123 deletions

3
codex-rs/Cargo.lock generated
View File

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

View File

@@ -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,47 +165,39 @@ 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 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))
}
}
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);
};
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}");
}
@@ -210,13 +205,12 @@ 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 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)
}

View File

@@ -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(
@@ -209,6 +296,39 @@ fn keyring_auth_storage_load_returns_deserialized_auth() -> anyhow::Result<()> {
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(())
}
#[test]
fn keyring_auth_storage_load_supports_split_json_compatibility() -> 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.clone()),
);
let expected = auth_with_prefix("split-compat");
let key = compute_store_key(codex_home.path())?;
let value = serde_json::to_value(&expected)?;
codex_keyring_store::save_split_json_to_keyring(&mock_keyring, KEYRING_SERVICE, &key, &value)?;
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");
@@ -256,17 +376,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(),
@@ -316,12 +435,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 +477,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 +497,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 +511,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(),

View File

@@ -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"] }

View File

@@ -5,6 +5,17 @@ use std::fmt;
use std::fmt::Debug;
use tracing::trace;
mod split_json;
pub use split_json::JsonKeyringError;
pub use split_json::SplitJsonKeyringError;
pub use split_json::delete_json_from_keyring;
pub use split_json::delete_split_json_from_keyring;
pub use split_json::load_json_from_keyring;
pub use split_json::load_split_json_from_keyring;
pub use split_json::save_json_to_keyring;
pub use split_json::save_split_json_to_keyring;
#[derive(Debug)]
pub enum CredentialStoreError {
Other(KeyringError),
@@ -41,7 +52,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 +90,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 +133,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 +217,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 +272,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 +308,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

View File

@@ -0,0 +1,856 @@
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>)>;
#[cfg(windows)]
pub fn load_json_from_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
) -> Result<Option<Value>, JsonKeyringError> {
if let Some(value) = load_split_json_from_keyring(keyring_store, service, base_key)? {
return Ok(Some(value));
}
load_full_json_from_keyring(keyring_store, service, base_key)
}
#[cfg(not(windows))]
pub fn load_json_from_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
) -> Result<Option<Value>, JsonKeyringError> {
if let Some(value) = load_full_json_from_keyring(keyring_store, service, base_key)? {
return Ok(Some(value));
}
load_split_json_from_keyring(keyring_store, service, base_key)
}
#[cfg(windows)]
pub fn save_json_to_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
value: &Value,
) -> Result<(), JsonKeyringError> {
save_split_json_to_keyring(keyring_store, service, base_key, value)?;
if let Err(err) = delete_full_json_from_keyring(keyring_store, service, base_key) {
warn!("failed to remove stale full JSON record from keyring: {err}");
}
Ok(())
}
#[cfg(not(windows))]
pub fn save_json_to_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
value: &Value,
) -> Result<(), JsonKeyringError> {
save_full_json_to_keyring(keyring_store, service, base_key, value)?;
if let Err(err) = delete_split_json_from_keyring(keyring_store, service, base_key) {
warn!("failed to remove stale split JSON record from keyring: {err}");
}
Ok(())
}
#[cfg(windows)]
pub fn delete_json_from_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
) -> Result<bool, JsonKeyringError> {
let split_removed = delete_split_json_from_keyring(keyring_store, service, base_key)?;
let full_removed = delete_full_json_from_keyring(keyring_store, service, base_key)?;
Ok(split_removed || full_removed)
}
#[cfg(not(windows))]
pub fn delete_json_from_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
) -> Result<bool, JsonKeyringError> {
let full_removed = delete_full_json_from_keyring(keyring_store, service, base_key)?;
let split_removed = delete_split_json_from_keyring(keyring_store, service, base_key)?;
Ok(full_removed || split_removed)
}
pub fn load_split_json_from_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
) -> Result<Option<Value>, SplitJsonKeyringError> {
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_split_json_to_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
value: &Value,
) -> Result<(), SplitJsonKeyringError> {
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_split_json_from_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
) -> Result<bool, SplitJsonKeyringError> {
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 load_full_json_from_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
) -> Result<Option<Value>, SplitJsonKeyringError> {
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| {
SplitJsonKeyringError::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| {
SplitJsonKeyringError::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)),
}
}
#[cfg(not(windows))]
fn save_full_json_to_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
value: &Value,
) -> Result<(), SplitJsonKeyringError> {
let bytes = serde_json::to_vec(value).map_err(|err| {
SplitJsonKeyringError::new(format!("failed to serialize JSON record: {err}"))
})?;
save_secret_to_keyring(keyring_store, service, base_key, &bytes, "JSON record")
}
fn delete_full_json_from_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
) -> Result<bool, SplitJsonKeyringError> {
delete_keyring_entry(keyring_store, service, base_key, "JSON record")
}
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::delete_split_json_from_keyring;
use super::layout_key;
use super::load_json_from_keyring;
use super::load_split_json_from_keyring;
use super::save_json_to_keyring;
use super::save_split_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_platform_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);
#[cfg(windows)]
{
assert!(
store.saved_secret(BASE_KEY).is_none(),
"windows should not store the full JSON record under the base key"
);
assert!(
store.contains(&layout_key(BASE_KEY, MANIFEST_ENTRY)),
"windows should store split JSON manifest metadata"
);
}
#[cfg(not(windows))]
{
assert_eq!(
store.saved_secret(BASE_KEY),
Some(serde_json::to_vec(&expected).expect("JSON should serialize")),
);
assert!(
!store.contains(&layout_key(BASE_KEY, MANIFEST_ENTRY)),
"non-windows should not create split JSON manifest metadata"
);
}
}
#[cfg(not(windows))]
#[test]
fn json_storage_loads_split_json_compatibility_on_non_windows() {
let store = MockKeyringStore::default();
let expected = json!({
"token": "secret",
"nested": {"id": 9}
});
save_split_json_to_keyring(&store, SERVICE, BASE_KEY, &expected)
.expect("split 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_platform_and_compat_entries() {
let store = MockKeyringStore::default();
let current = json!({"current": true});
let split = json!({"split": true});
save_json_to_keyring(&store, SERVICE, BASE_KEY, &current).expect("JSON should save");
save_split_json_to_keyring(&store, SERVICE, BASE_KEY, &split)
.expect("split JSON should save");
store
.save(
SERVICE,
BASE_KEY,
&serde_json::to_string(&current).expect("JSON should serialize"),
)
.expect("legacy 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_split_json_to_keyring(&store, SERVICE, BASE_KEY, &expected)
.expect("split JSON should save");
let loaded = load_split_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_split_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_split_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_split_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_split_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_split_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_split_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_split_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");
}
}

View File

@@ -45,6 +45,9 @@ use tracing::warn;
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 rmcp::transport::auth::AuthorizationManager;
use tokio::sync::Mutex;
@@ -155,16 +158,15 @@ fn load_oauth_tokens_from_keyring<K: KeyringStore>(
url: &str,
) -> Result<Option<StoredOAuthTokens>> {
let key = compute_store_key(server_name, url)?;
match keyring_store.load(KEYRING_SERVICE, &key) {
Ok(Some(serialized)) => {
let mut tokens: StoredOAuthTokens = serde_json::from_str(&serialized)
.context("failed to deserialize OAuth tokens from keyring")?;
refresh_expires_in_from_timestamp(&mut tokens);
Ok(Some(tokens))
}
Ok(None) => Ok(None),
Err(error) => Err(Error::new(error.into_error())),
}
let Some(value) = load_json_from_keyring(keyring_store, KEYRING_SERVICE, &key)
.map_err(|err| Error::msg(err.to_string()))?
else {
return Ok(None);
};
let mut tokens: StoredOAuthTokens =
serde_json::from_value(value).context("failed to deserialize OAuth tokens from keyring")?;
refresh_expires_in_from_timestamp(&mut tokens);
Ok(Some(tokens))
}
pub fn save_oauth_tokens(
@@ -191,10 +193,9 @@ fn save_oauth_tokens_with_keyring<K: KeyringStore>(
server_name: &str,
tokens: &StoredOAuthTokens,
) -> Result<()> {
let serialized = serde_json::to_string(tokens).context("failed to serialize OAuth tokens")?;
let value = serde_json::to_value(tokens).context("failed to serialize OAuth tokens")?;
let key = compute_store_key(server_name, &tokens.url)?;
match keyring_store.save(KEYRING_SERVICE, &key, &serialized) {
match save_json_to_keyring(keyring_store, KEYRING_SERVICE, &key, &value) {
Ok(()) => {
if let Err(error) = delete_oauth_tokens_from_file(&key) {
warn!("failed to remove OAuth tokens from fallback storage: {error:?}");
@@ -202,12 +203,9 @@ fn save_oauth_tokens_with_keyring<K: KeyringStore>(
Ok(())
}
Err(error) => {
let message = format!(
"failed to write OAuth tokens to keyring: {}",
error.message()
);
let message = format!("failed to write OAuth tokens to keyring: {error}");
warn!("{message}");
Err(Error::new(error.into_error()).context(message))
Err(Error::msg(message))
}
}
}
@@ -244,22 +242,20 @@ fn delete_oauth_tokens_from_keyring_and_file<K: KeyringStore>(
url: &str,
) -> Result<bool> {
let key = compute_store_key(server_name, url)?;
let keyring_result = keyring_store.delete(KEYRING_SERVICE, &key);
let keyring_removed = match keyring_result {
let keyring_removed = match delete_json_from_keyring(keyring_store, KEYRING_SERVICE, &key) {
Ok(removed) => removed,
Err(error) => {
let message = error.message();
let message = error.to_string();
warn!("failed to delete OAuth tokens from keyring: {message}");
match store_mode {
OAuthCredentialsStoreMode::Auto | OAuthCredentialsStoreMode::Keyring => {
return Err(error.into_error())
return Err(Error::msg(message))
.context("failed to delete OAuth tokens from keyring");
}
OAuthCredentialsStoreMode::File => false,
}
}
};
let file_removed = delete_oauth_tokens_from_file(&key)?;
Ok(keyring_removed || file_removed)
}
@@ -604,6 +600,10 @@ fn sha_256_prefix(value: &Value) -> Result<String> {
mod tests {
use super::*;
use anyhow::Result;
use codex_keyring_store::CredentialStoreError;
use codex_keyring_store::load_json_from_keyring;
use codex_keyring_store::save_json_to_keyring;
use codex_keyring_store::save_split_json_to_keyring;
use keyring::Error as KeyringError;
use pretty_assertions::assert_eq;
use std::sync::Mutex;
@@ -614,6 +614,101 @@ mod tests {
use codex_keyring_store::tests::MockKeyringStore;
#[derive(Clone, Debug)]
struct KeyringStoreWithError {
inner: MockKeyringStore,
fail_delete: bool,
fail_load_secret: bool,
fail_save_secret: bool,
}
impl KeyringStoreWithError {
fn fail_delete(inner: MockKeyringStore) -> Self {
Self {
inner,
fail_delete: true,
fail_load_secret: false,
fail_save_secret: false,
}
}
fn fail_load_secret(inner: MockKeyringStore) -> Self {
Self {
inner,
fail_delete: false,
fail_load_secret: true,
fail_save_secret: false,
}
}
fn fail_save_secret(inner: MockKeyringStore) -> Self {
Self {
inner,
fail_delete: false,
fail_load_secret: false,
fail_save_secret: true,
}
}
}
impl KeyringStore for KeyringStoreWithError {
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> {
if self.fail_load_secret {
return Err(CredentialStoreError::new(KeyringError::Invalid(
"error".into(),
"load".into(),
)));
}
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> {
if self.fail_save_secret {
return Err(CredentialStoreError::new(KeyringError::Invalid(
"error".into(),
"save".into(),
)));
}
self.inner.save_secret(service, account, value)
}
fn delete(&self, service: &str, account: &str) -> Result<bool, CredentialStoreError> {
if self.fail_delete {
return Err(CredentialStoreError::new(KeyringError::Invalid(
"error".into(),
"delete".into(),
)));
}
self.inner.delete(service, account)
}
}
struct TempCodexHome {
_guard: MutexGuard<'static, ()>,
_dir: tempfile::TempDir,
@@ -651,6 +746,38 @@ mod tests {
let store = MockKeyringStore::default();
let tokens = sample_tokens();
let expected = tokens.clone();
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
let value = serde_json::to_value(&tokens)?;
save_json_to_keyring(&store, KEYRING_SERVICE, &key, &value)?;
let loaded =
super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)?
.expect("tokens should load from keyring");
assert_tokens_match_without_expiry(&loaded, &expected);
Ok(())
}
#[test]
fn load_oauth_tokens_supports_split_json_compatibility() -> Result<()> {
let _env = TempCodexHome::new();
let store = MockKeyringStore::default();
let tokens = sample_tokens();
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
let value = serde_json::to_value(&tokens)?;
save_split_json_to_keyring(&store, KEYRING_SERVICE, &key, &value)?;
let loaded =
super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)?
.expect("tokens should load from split-json compatibility format");
assert_tokens_match_without_expiry(&loaded, &tokens);
Ok(())
}
#[test]
fn load_oauth_tokens_supports_legacy_single_entry() -> Result<()> {
let _env = TempCodexHome::new();
let store = MockKeyringStore::default();
let tokens = sample_tokens();
let serialized = serde_json::to_string(&tokens)?;
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
store.save(KEYRING_SERVICE, &key, &serialized)?;
@@ -658,7 +785,7 @@ mod tests {
let loaded =
super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)?
.expect("tokens should load from keyring");
assert_tokens_match_without_expiry(&loaded, &expected);
assert_tokens_match_without_expiry(&loaded, &tokens);
Ok(())
}
@@ -684,11 +811,9 @@ mod tests {
#[test]
fn load_oauth_tokens_falls_back_when_keyring_errors() -> Result<()> {
let _env = TempCodexHome::new();
let store = MockKeyringStore::default();
let store = KeyringStoreWithError::fail_load_secret(MockKeyringStore::default());
let tokens = sample_tokens();
let expected = tokens.clone();
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
store.set_error(&key, KeyringError::Invalid("error".into(), "load".into()));
super::save_oauth_tokens_to_file(&tokens)?;
@@ -719,18 +844,29 @@ mod tests {
let fallback_path = super::fallback_file_path()?;
assert!(!fallback_path.exists(), "fallback file should be removed");
let stored = store.saved_value(&key).expect("value saved to keyring");
assert_eq!(serde_json::from_str::<StoredOAuthTokens>(&stored)?, tokens);
#[cfg(windows)]
assert!(
store.saved_secret(&key).is_none(),
"windows should not store the full JSON record under the base key"
);
#[cfg(not(windows))]
assert!(
store.saved_secret(&key).is_some(),
"non-windows should store the full JSON record as one secret"
);
let stored =
super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)?
.expect("value saved to keyring");
assert_tokens_match_without_expiry(&stored, &tokens);
Ok(())
}
#[test]
fn save_oauth_tokens_writes_fallback_when_keyring_fails() -> Result<()> {
let _env = TempCodexHome::new();
let store = MockKeyringStore::default();
let mock_keyring = MockKeyringStore::default();
let store = KeyringStoreWithError::fail_save_secret(mock_keyring.clone());
let tokens = sample_tokens();
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
store.set_error(&key, KeyringError::Invalid("error".into(), "save".into()));
super::save_oauth_tokens_with_keyring_with_fallback_to_file(
&store,
@@ -750,7 +886,11 @@ mod tests {
entry.access_token,
tokens.token_response.0.access_token().secret().as_str()
);
assert!(store.saved_value(&key).is_none());
assert!(mock_keyring.saved_value(&key).is_none());
assert!(
load_json_from_keyring(&mock_keyring, KEYRING_SERVICE, &key)?.is_none(),
"keyring should not point at saved OAuth tokens when save fails"
);
Ok(())
}
@@ -759,8 +899,10 @@ mod tests {
let _env = TempCodexHome::new();
let store = MockKeyringStore::default();
let tokens = sample_tokens();
let serialized = serde_json::to_string(&tokens)?;
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
let value = serde_json::to_value(&tokens)?;
save_split_json_to_keyring(&store, KEYRING_SERVICE, &key, &value)?;
let serialized = serde_json::to_string(&tokens)?;
store.save(KEYRING_SERVICE, &key, &serialized)?;
super::save_oauth_tokens_to_file(&tokens)?;
@@ -771,7 +913,10 @@ mod tests {
&tokens.url,
)?;
assert!(removed);
assert!(!store.contains(&key));
assert!(
load_json_from_keyring(&store, KEYRING_SERVICE, &key)?.is_none(),
"keyring entry should be removed"
);
assert!(!super::fallback_file_path()?.exists());
Ok(())
}
@@ -781,10 +926,13 @@ mod tests {
let _env = TempCodexHome::new();
let store = MockKeyringStore::default();
let tokens = sample_tokens();
let serialized = serde_json::to_string(&tokens)?;
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
store.save(KEYRING_SERVICE, &key, &serialized)?;
assert!(store.contains(&key));
let value = serde_json::to_value(&tokens)?;
save_split_json_to_keyring(&store, KEYRING_SERVICE, &key, &value)?;
assert!(
super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)?
.is_some()
);
let removed = super::delete_oauth_tokens_from_keyring_and_file(
&store,
@@ -793,7 +941,14 @@ mod tests {
&tokens.url,
)?;
assert!(removed);
assert!(!store.contains(&key));
assert!(
load_json_from_keyring(&store, KEYRING_SERVICE, &key)?.is_none(),
"keyring entry should be removed"
);
assert!(
super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)?
.is_none()
);
assert!(!super::fallback_file_path()?.exists());
Ok(())
}
@@ -801,10 +956,8 @@ mod tests {
#[test]
fn delete_oauth_tokens_propagates_keyring_errors() -> Result<()> {
let _env = TempCodexHome::new();
let store = MockKeyringStore::default();
let store = KeyringStoreWithError::fail_delete(MockKeyringStore::default());
let tokens = sample_tokens();
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
store.set_error(&key, KeyringError::Invalid("error".into(), "delete".into()));
super::save_oauth_tokens_to_file(&tokens).unwrap();
let result = super::delete_oauth_tokens_from_keyring_and_file(