clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-23 22:24:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

Store keyring auth in encrypted local secrets

Browse Source
This commit is contained in:
mikhail-oai
2026-04-15 08:38:29 -04:00
parent 86bd0bc95c
commit b8064342bd
4 changed files with 95 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
codex-rs/Cargo.lock generated
View File

@@ -2313,6 +2313,7 @@ dependencies = [
"codex-model-provider-info",
"codex-otel",
"codex-protocol",
"codex-secrets",
"codex-terminal-detection",
"codex-utils-template",
"core_test_support",

1
codex-rs/login/Cargo.toml
View File

@@ -19,6 +19,7 @@ codex-keyring-store = { workspace = true }
codex-model-provider-info = { workspace = true }
codex-otel = { workspace = true }
codex-protocol = { workspace = true }
codex-secrets = { workspace = true }
codex-terminal-detection = { workspace = true }
codex-utils-template = { workspace = true }
once_cell = { workspace = true }

69
codex-rs/login/src/auth/storage.rs
View File

@@ -23,6 +23,9 @@ use codex_app_server_protocol::AuthMode;
use codex_config::types::AuthCredentialsStoreMode;
use codex_keyring_store::DefaultKeyringStore;
use codex_keyring_store::KeyringStore;
use codex_secrets::LocalSecretsBackend;
use codex_secrets::SecretName;
use codex_secrets::SecretScope;
use once_cell::sync::Lazy;
/// Expected structure for $CODEX_HOME/auth.json.
@@ -117,7 +120,8 @@ impl AuthStorageBackend for FileAuthStorage {
}
}
const KEYRING_SERVICE: &str = "Codex Auth";
static CLI_AUTH_SECRET_NAME: Lazy<SecretName> =
Lazy::new(|| SecretName::new("CLI_AUTH").expect("CLI_AUTH should be valid"));
// turns codex_home path into a stable, short key string
fn compute_store_key(codex_home: &Path) -> std::io::Result<String> {
@@ -136,58 +140,40 @@ fn compute_store_key(codex_home: &Path) -> std::io::Result<String> {
#[derive(Clone, Debug)]
struct KeyringAuthStorage {
codex_home: PathBuf,
keyring_store: Arc<dyn KeyringStore>,
secrets_backend: LocalSecretsBackend,
}
impl KeyringAuthStorage {
fn new(codex_home: PathBuf, keyring_store: Arc<dyn KeyringStore>) -> Self {
let secrets_backend = LocalSecretsBackend::new(codex_home.clone(), keyring_store);
Self {
codex_home,
keyring_store,
}
}
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))
}
secrets_backend,
}
}
}
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)
match self
.secrets_backend
.get(&SecretScope::Global, &CLI_AUTH_SECRET_NAME)
.map_err(std::io::Error::other)?
{
Some(serialized) => serde_json::from_str(&serialized).map(Some).map_err(|err| {
std::io::Error::other(format!(
"failed to deserialize CLI auth from encrypted auth storage: {err}"
))
}),
None => Ok(None),
}
}
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)?;
self.secrets_backend
.set(&SecretScope::Global, &CLI_AUTH_SECRET_NAME, &serialized)
.map_err(std::io::Error::other)?;
if let Err(err) = delete_file_if_exists(&self.codex_home) {
warn!("failed to remove CLI auth fallback file: {err}");
}
@@ -195,13 +181,10 @@ 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}"))
})?;
.secrets_backend
.delete(&SecretScope::Global, &CLI_AUTH_SECRET_NAME)
.map_err(std::io::Error::other)?;
let file_removed = delete_file_if_exists(&self.codex_home)?;
Ok(keyring_removed || file_removed)
}

129
codex-rs/login/src/auth/storage_tests.rs
View File

@@ -2,6 +2,8 @@ use super::*;
use crate::token_data::IdTokenInfo;
use anyhow::Context;
use base64::Engine;
use codex_secrets::LocalSecretsBackend;
use codex_secrets::SecretScope;
use pretty_assertions::assert_eq;
use serde_json::json;
use tempfile::tempdir;
@@ -97,51 +99,75 @@ fn ephemeral_storage_save_load_delete_is_in_memory_only() -> anyhow::Result<()>
Ok(())
}
fn seed_keyring_and_fallback_auth_file_for_delete<F>(
fn seed_keyring_and_fallback_auth_file_for_delete(
mock_keyring: &MockKeyringStore,
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<PathBuf> {
let backend =
LocalSecretsBackend::new(codex_home.to_path_buf(), Arc::new(mock_keyring.clone()));
backend.set(
&SecretScope::Global,
&CLI_AUTH_SECRET_NAME,
&serde_json::to_string(auth)?,
)?;
let auth_file = get_auth_file(codex_home);
std::fs::write(&auth_file, "stale")?;
Ok((key, auth_file))
Ok(auth_file)
}
fn seed_keyring_with_auth<F>(
fn seed_keyring_with_auth(
mock_keyring: &MockKeyringStore,
compute_key: F,
codex_home: &Path,
auth: &AuthDotJson,
) -> anyhow::Result<()>
where
F: FnOnce() -> std::io::Result<String>,
{
let key = compute_key()?;
let serialized = serde_json::to_string(auth)?;
mock_keyring.save(KEYRING_SERVICE, &key, &serialized)?;
) -> anyhow::Result<()> {
let backend =
LocalSecretsBackend::new(codex_home.to_path_buf(), Arc::new(mock_keyring.clone()));
backend.set(
&SecretScope::Global,
&CLI_AUTH_SECRET_NAME,
&serde_json::to_string(auth)?,
)?;
Ok(())
}
fn assert_keyring_saved_auth_and_removed_fallback(
mock_keyring: &MockKeyringStore,
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");
) -> anyhow::Result<()> {
let backend =
LocalSecretsBackend::new(codex_home.to_path_buf(), Arc::new(mock_keyring.clone()));
let saved_value = backend
.get(&SecretScope::Global, &CLI_AUTH_SECRET_NAME)?
.context("encrypted auth entry should exist")?;
let expected_serialized = serde_json::to_string(expected)?;
assert_eq!(saved_value, expected_serialized);
let old_key = compute_store_key(codex_home)?;
assert!(
mock_keyring.saved_value(&old_key).is_none(),
"legacy keyring auth entry should not be used"
);
let secrets_key = compute_secrets_keyring_account(codex_home)?;
assert!(
mock_keyring.saved_value(&secrets_key).is_some(),
"secrets backend should persist an encryption passphrase in the keyring"
);
assert!(encrypted_auth_file(codex_home).exists());
let auth_file = get_auth_file(codex_home);
assert!(
!auth_file.exists(),
"fallback auth.json should be removed after keyring save"
);
Ok(())
}
fn encrypted_auth_file(codex_home: &Path) -> PathBuf {
codex_home.join("secrets").join("local.age")
}
fn compute_secrets_keyring_account(codex_home: &Path) -> anyhow::Result<String> {
Ok(compute_store_key(codex_home)?.replacen("cli|", "secrets|", 1))
}
fn id_token_with_prefix(prefix: &str) -> IdTokenInfo {
@@ -198,11 +224,7 @@ fn keyring_auth_storage_load_returns_deserialized_auth() -> anyhow::Result<()> {
tokens: None,
last_refresh: None,
};
seed_keyring_with_auth(
&mock_keyring,
|| compute_store_key(codex_home.path()),
&expected,
)?;
seed_keyring_with_auth(&mock_keyring, codex_home.path(), &expected)?;
let loaded = storage.load()?;
assert_eq!(Some(expected), loaded);
@@ -243,8 +265,7 @@ fn keyring_auth_storage_save_persists_and_removes_fallback_file() -> anyhow::Res
storage.save(&auth)?;
let key = compute_store_key(codex_home.path())?;
assert_keyring_saved_auth_and_removed_fallback(&mock_keyring, &key, codex_home.path(), &auth);
assert_keyring_saved_auth_and_removed_fallback(&mock_keyring, codex_home.path(), &auth)?;
Ok(())
}
@@ -256,18 +277,14 @@ 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("to-delete");
let auth_file =
seed_keyring_and_fallback_auth_file_for_delete(&mock_keyring, codex_home.path(), &auth)?;
let removed = storage.delete()?;
assert!(removed, "delete should report removal");
assert!(
!mock_keyring.contains(&key),
"keyring entry should be removed"
);
assert_eq!(storage.load()?, None, "encrypted auth should be removed");
assert!(
!auth_file.exists(),
"fallback auth.json should be removed after keyring delete"
@@ -284,11 +301,7 @@ fn auto_auth_storage_load_prefers_keyring_value() -> anyhow::Result<()> {
Arc::new(mock_keyring.clone()),
);
let keyring_auth = auth_with_prefix("keyring");
seed_keyring_with_auth(
&mock_keyring,
|| compute_store_key(codex_home.path()),
&keyring_auth,
)?;
seed_keyring_with_auth(&mock_keyring, codex_home.path(), &keyring_auth)?;
let file_auth = auth_with_prefix("file");
storage.file_storage.save(&file_auth)?;
@@ -320,7 +333,10 @@ fn auto_auth_storage_load_falls_back_when_keyring_errors() -> anyhow::Result<()>
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
);
let key = compute_store_key(codex_home.path())?;
let key = compute_secrets_keyring_account(codex_home.path())?;
let encrypted = auth_with_prefix("encrypted");
seed_keyring_with_auth(&mock_keyring, codex_home.path(), &encrypted)?;
mock_keyring.set_error(&key, KeyringError::Invalid("error".into(), "load".into()));
let expected = auth_with_prefix("fallback");
@@ -339,20 +355,13 @@ fn auto_auth_storage_save_prefers_keyring() -> anyhow::Result<()> {
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
);
let key = compute_store_key(codex_home.path())?;
let stale = auth_with_prefix("stale");
storage.file_storage.save(&stale)?;
let expected = auth_with_prefix("to-save");
storage.save(&expected)?;
assert_keyring_saved_auth_and_removed_fallback(
&mock_keyring,
&key,
codex_home.path(),
&expected,
);
assert_keyring_saved_auth_and_removed_fallback(&mock_keyring, codex_home.path(), &expected)?;
Ok(())
}
@@ -364,7 +373,7 @@ fn auto_auth_storage_save_falls_back_when_keyring_errors() -> anyhow::Result<()>
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
);
let key = compute_store_key(codex_home.path())?;
let key = compute_secrets_keyring_account(codex_home.path())?;
mock_keyring.set_error(&key, KeyringError::Invalid("error".into(), "save".into()));
let auth = auth_with_prefix("fallback");
@@ -395,18 +404,14 @@ 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("to-delete");
let auth_file =
seed_keyring_and_fallback_auth_file_for_delete(&mock_keyring, codex_home.path(), &auth)?;
let removed = storage.delete()?;
assert!(removed, "delete should report removal");
assert!(
!mock_keyring.contains(&key),
"keyring entry should be removed"
);
assert_eq!(storage.load()?, None, "encrypted auth should be removed");
assert!(
!auth_file.exists(),
"fallback auth.json should be removed after delete"