mirror of
https://github.com/openai/codex.git
synced 2026-04-24 06:35:50 +00:00
Treat keyring storage as unsupported on Android
Make credential persistence robust on Android by explicitly handling unsupported keyring backends and falling back to file-backed storage. - keyring-store: gate the keyring dependency to non-android, add an Unsupported error, and provide an Android stub store that reports Unsupported. - core auth storage: treat Unsupported as a soft failure in Auto mode so the file store is selected automatically. - secrets: persist the passphrase to a private file with atomic writes (0600) when keyring is unavailable and load it on startup. - rmcp-client OAuth: downgrade Unsupported keyring errors into file-backed storage flows and add coverage for load/save/delete. - move keyring crate usage to dev-dependencies in core/rmcp-client to avoid linking keyring into Android builds. Desktop behavior is preserved; Android now cleanly opts into the file-based path instead of failing.
This commit is contained in:
@@ -140,10 +140,10 @@ windows-sys = { version = "0.52", features = [
|
||||
|
||||
[target.'cfg(unix)'.dependencies]
|
||||
codex-shell-escalation = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
assert_cmd = { workspace = true }
|
||||
assert_matches = { workspace = true }
|
||||
keyring = { workspace = true, features = ["crypto-rust"] }
|
||||
codex-arg0 = { workspace = true }
|
||||
codex-otel = { workspace = true, features = [
|
||||
"disable-default-metrics-exporter",
|
||||
@@ -171,5 +171,17 @@ walkdir = { workspace = true }
|
||||
wiremock = { workspace = true }
|
||||
zstd = { workspace = true }
|
||||
|
||||
[target.'cfg(target_os = "linux")'.dev-dependencies]
|
||||
keyring = { workspace = true, features = ["linux-native-async-persistent"] }
|
||||
|
||||
[target.'cfg(target_os = "macos")'.dev-dependencies]
|
||||
keyring = { workspace = true, features = ["apple-native"] }
|
||||
|
||||
[target.'cfg(target_os = "windows")'.dev-dependencies]
|
||||
keyring = { workspace = true, features = ["windows-native"] }
|
||||
|
||||
[target.'cfg(any(target_os = "freebsd", target_os = "openbsd"))'.dev-dependencies]
|
||||
keyring = { workspace = true, features = ["sync-secret-service"] }
|
||||
|
||||
[package.metadata.cargo-shear]
|
||||
ignored = ["openssl-sys"]
|
||||
|
||||
@@ -8,17 +8,19 @@ license.workspace = true
|
||||
workspace = true
|
||||
|
||||
[dependencies]
|
||||
keyring = { workspace = true, features = ["crypto-rust"] }
|
||||
tracing = { workspace = true }
|
||||
|
||||
[target.'cfg(target_os = "linux")'.dependencies]
|
||||
[target.'cfg(not(target_os = "android"))'.dependencies]
|
||||
keyring = { workspace = true, features = ["crypto-rust"] }
|
||||
|
||||
[target.'cfg(all(not(target_os = "android"), target_os = "linux"))'.dependencies]
|
||||
keyring = { workspace = true, features = ["linux-native-async-persistent"] }
|
||||
|
||||
[target.'cfg(target_os = "macos")'.dependencies]
|
||||
[target.'cfg(all(not(target_os = "android"), target_os = "macos"))'.dependencies]
|
||||
keyring = { workspace = true, features = ["apple-native"] }
|
||||
|
||||
[target.'cfg(target_os = "windows")'.dependencies]
|
||||
[target.'cfg(all(not(target_os = "android"), target_os = "windows"))'.dependencies]
|
||||
keyring = { workspace = true, features = ["windows-native"] }
|
||||
|
||||
[target.'cfg(any(target_os = "freebsd", target_os = "openbsd"))'.dependencies]
|
||||
[target.'cfg(all(not(target_os = "android"), any(target_os = "freebsd", target_os = "openbsd")))'.dependencies]
|
||||
keyring = { workspace = true, features = ["sync-secret-service"] }
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
#[cfg(not(target_os = "android"))]
|
||||
use keyring::Entry;
|
||||
#[cfg(not(target_os = "android"))]
|
||||
use keyring::Error as KeyringError;
|
||||
use std::error::Error;
|
||||
use std::fmt;
|
||||
@@ -7,30 +9,35 @@ use tracing::trace;
|
||||
|
||||
#[derive(Debug)]
|
||||
pub enum CredentialStoreError {
|
||||
Unsupported,
|
||||
#[cfg(not(target_os = "android"))]
|
||||
Other(KeyringError),
|
||||
}
|
||||
|
||||
impl CredentialStoreError {
|
||||
#[cfg(not(target_os = "android"))]
|
||||
pub fn new(error: KeyringError) -> Self {
|
||||
Self::Other(error)
|
||||
}
|
||||
|
||||
pub fn message(&self) -> String {
|
||||
match self {
|
||||
Self::Unsupported => "keyring unsupported on this platform".to_string(),
|
||||
#[cfg(not(target_os = "android"))]
|
||||
Self::Other(error) => error.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn into_error(self) -> KeyringError {
|
||||
match self {
|
||||
Self::Other(error) => error,
|
||||
}
|
||||
pub fn is_unsupported(&self) -> bool {
|
||||
matches!(self, Self::Unsupported)
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for CredentialStoreError {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
Self::Unsupported => write!(f, "keyring unsupported on this platform"),
|
||||
#[cfg(not(target_os = "android"))]
|
||||
Self::Other(error) => write!(f, "{error}"),
|
||||
}
|
||||
}
|
||||
@@ -48,6 +55,27 @@ pub trait KeyringStore: Debug + Send + Sync {
|
||||
#[derive(Debug)]
|
||||
pub struct DefaultKeyringStore;
|
||||
|
||||
#[cfg(target_os = "android")]
|
||||
impl KeyringStore for DefaultKeyringStore {
|
||||
fn load(&self, _service: &str, _account: &str) -> Result<Option<String>, CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
|
||||
fn save(
|
||||
&self,
|
||||
_service: &str,
|
||||
_account: &str,
|
||||
_value: &str,
|
||||
) -> Result<(), CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
|
||||
fn delete(&self, _service: &str, _account: &str) -> Result<bool, CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "android"))]
|
||||
impl KeyringStore for DefaultKeyringStore {
|
||||
fn load(&self, service: &str, account: &str) -> Result<Option<String>, CredentialStoreError> {
|
||||
trace!("keyring.load start, service={service}, account={account}");
|
||||
@@ -106,6 +134,7 @@ impl KeyringStore for DefaultKeyringStore {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "android"))]
|
||||
pub mod tests {
|
||||
use super::CredentialStoreError;
|
||||
use super::KeyringStore;
|
||||
|
||||
@@ -170,10 +170,18 @@ impl KeyringAuthStorage {
|
||||
))
|
||||
}),
|
||||
Ok(None) => Ok(None),
|
||||
Err(error) => Err(std::io::Error::other(format!(
|
||||
"failed to load CLI auth from keyring: {}",
|
||||
error.message()
|
||||
))),
|
||||
Err(error) => {
|
||||
if error.is_unsupported() {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::Unsupported,
|
||||
"keyring unsupported on this platform",
|
||||
));
|
||||
}
|
||||
Err(std::io::Error::other(format!(
|
||||
"failed to load CLI auth from keyring: {}",
|
||||
error.message()
|
||||
)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -181,6 +189,12 @@ impl KeyringAuthStorage {
|
||||
match self.keyring_store.save(KEYRING_SERVICE, key, value) {
|
||||
Ok(()) => Ok(()),
|
||||
Err(error) => {
|
||||
if error.is_unsupported() {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::Unsupported,
|
||||
"keyring unsupported on this platform",
|
||||
));
|
||||
}
|
||||
let message = format!(
|
||||
"failed to write OAuth tokens to keyring: {}",
|
||||
error.message()
|
||||
@@ -215,6 +229,12 @@ impl AuthStorageBackend for KeyringAuthStorage {
|
||||
.keyring_store
|
||||
.delete(KEYRING_SERVICE, &key)
|
||||
.map_err(|err| {
|
||||
if err.is_unsupported() {
|
||||
return std::io::Error::new(
|
||||
std::io::ErrorKind::Unsupported,
|
||||
"keyring unsupported on this platform",
|
||||
);
|
||||
}
|
||||
std::io::Error::other(format!("failed to delete auth from keyring: {err}"))
|
||||
})?;
|
||||
let file_removed = delete_file_if_exists(&self.codex_home)?;
|
||||
@@ -243,6 +263,9 @@ impl AuthStorageBackend for AutoAuthStorage {
|
||||
Ok(Some(auth)) => Ok(Some(auth)),
|
||||
Ok(None) => self.file_storage.load(),
|
||||
Err(err) => {
|
||||
if err.kind() == std::io::ErrorKind::Unsupported {
|
||||
return self.file_storage.load();
|
||||
}
|
||||
warn!("failed to load CLI auth from keyring, falling back to file storage: {err}");
|
||||
self.file_storage.load()
|
||||
}
|
||||
@@ -253,6 +276,9 @@ impl AuthStorageBackend for AutoAuthStorage {
|
||||
match self.keyring_storage.save(auth) {
|
||||
Ok(()) => Ok(()),
|
||||
Err(err) => {
|
||||
if err.kind() == std::io::ErrorKind::Unsupported {
|
||||
return self.file_storage.save(auth);
|
||||
}
|
||||
warn!("failed to save auth to keyring, falling back to file storage: {err}");
|
||||
self.file_storage.save(auth)
|
||||
}
|
||||
@@ -261,7 +287,15 @@ impl AuthStorageBackend for AutoAuthStorage {
|
||||
|
||||
fn delete(&self) -> std::io::Result<bool> {
|
||||
// Keyring storage will delete from disk as well
|
||||
self.keyring_storage.delete()
|
||||
match self.keyring_storage.delete() {
|
||||
Ok(removed) => Ok(removed),
|
||||
Err(err) => {
|
||||
if err.kind() == std::io::ErrorKind::Unsupported {
|
||||
return self.file_storage.delete();
|
||||
}
|
||||
Err(err)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ use super::*;
|
||||
use crate::token_data::IdTokenInfo;
|
||||
use anyhow::Context;
|
||||
use base64::Engine;
|
||||
use codex_keyring_store::CredentialStoreError;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use tempfile::tempdir;
|
||||
@@ -144,6 +145,28 @@ fn assert_keyring_saved_auth_and_removed_fallback(
|
||||
);
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct UnsupportedKeyringStore;
|
||||
|
||||
impl KeyringStore for UnsupportedKeyringStore {
|
||||
fn load(&self, _service: &str, _account: &str) -> Result<Option<String>, CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
|
||||
fn save(
|
||||
&self,
|
||||
_service: &str,
|
||||
_account: &str,
|
||||
_value: &str,
|
||||
) -> Result<(), CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
|
||||
fn delete(&self, _service: &str, _account: &str) -> Result<bool, CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
}
|
||||
|
||||
fn id_token_with_prefix(prefix: &str) -> IdTokenInfo {
|
||||
#[derive(Serialize)]
|
||||
struct Header {
|
||||
@@ -413,3 +436,20 @@ fn auto_auth_storage_delete_removes_keyring_and_file() -> anyhow::Result<()> {
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auto_auth_storage_falls_back_when_keyring_unsupported() -> anyhow::Result<()> {
|
||||
let codex_home = tempdir()?;
|
||||
let storage = create_auth_storage_with_keyring_store(
|
||||
codex_home.path().to_path_buf(),
|
||||
AuthCredentialsStoreMode::Auto,
|
||||
Arc::new(UnsupportedKeyringStore),
|
||||
);
|
||||
let auth = auth_with_prefix("unsupported");
|
||||
|
||||
storage.save(&auth)?;
|
||||
let loaded = storage.load()?;
|
||||
|
||||
assert_eq!(loaded, Some(auth));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -19,7 +19,6 @@ codex-protocol = { workspace = true }
|
||||
codex-utils-pty = { workspace = true }
|
||||
codex-utils-home-dir = { workspace = true }
|
||||
futures = { workspace = true, default-features = false, features = ["std"] }
|
||||
keyring = { workspace = true, features = ["crypto-rust"] }
|
||||
oauth2 = "5"
|
||||
reqwest = { workspace = true, features = ["json", "stream"] }
|
||||
rmcp = { workspace = true, default-features = false, features = [
|
||||
@@ -56,17 +55,19 @@ which = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
codex-utils-cargo-bin = { workspace = true }
|
||||
keyring = { workspace = true, features = ["crypto-rust"] }
|
||||
pretty_assertions = { workspace = true }
|
||||
serial_test = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
[target.'cfg(target_os = "linux")'.dependencies]
|
||||
|
||||
[target.'cfg(target_os = "linux")'.dev-dependencies]
|
||||
keyring = { workspace = true, features = ["linux-native-async-persistent"] }
|
||||
|
||||
[target.'cfg(target_os = "macos")'.dependencies]
|
||||
[target.'cfg(target_os = "macos")'.dev-dependencies]
|
||||
keyring = { workspace = true, features = ["apple-native"] }
|
||||
|
||||
[target.'cfg(target_os = "windows")'.dependencies]
|
||||
[target.'cfg(target_os = "windows")'.dev-dependencies]
|
||||
keyring = { workspace = true, features = ["windows-native"] }
|
||||
|
||||
[target.'cfg(any(target_os = "freebsd", target_os = "openbsd"))'.dependencies]
|
||||
[target.'cfg(any(target_os = "freebsd", target_os = "openbsd"))'.dev-dependencies]
|
||||
keyring = { workspace = true, features = ["sync-secret-service"] }
|
||||
|
||||
@@ -43,6 +43,7 @@ use std::time::SystemTime;
|
||||
use std::time::UNIX_EPOCH;
|
||||
use tracing::warn;
|
||||
|
||||
use codex_keyring_store::CredentialStoreError;
|
||||
use codex_keyring_store::DefaultKeyringStore;
|
||||
use codex_keyring_store::KeyringStore;
|
||||
use rmcp::transport::auth::AuthorizationManager;
|
||||
@@ -142,6 +143,9 @@ fn load_oauth_tokens_from_keyring_with_fallback_to_file<K: KeyringStore>(
|
||||
Ok(Some(tokens)) => Ok(Some(tokens)),
|
||||
Ok(None) => load_oauth_tokens_from_file(server_name, url),
|
||||
Err(error) => {
|
||||
if is_keyring_unsupported(&error) {
|
||||
return load_oauth_tokens_from_file(server_name, url);
|
||||
}
|
||||
warn!("failed to read OAuth tokens from keyring: {error}");
|
||||
load_oauth_tokens_from_file(server_name, url)
|
||||
.with_context(|| format!("failed to read OAuth tokens from keyring: {error}"))
|
||||
@@ -163,7 +167,7 @@ fn load_oauth_tokens_from_keyring<K: KeyringStore>(
|
||||
Ok(Some(tokens))
|
||||
}
|
||||
Ok(None) => Ok(None),
|
||||
Err(error) => Err(Error::new(error.into_error())),
|
||||
Err(error) => Err(Error::new(error)),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -202,12 +206,15 @@ fn save_oauth_tokens_with_keyring<K: KeyringStore>(
|
||||
Ok(())
|
||||
}
|
||||
Err(error) => {
|
||||
if error.is_unsupported() {
|
||||
return Err(Error::new(error));
|
||||
}
|
||||
let message = format!(
|
||||
"failed to write OAuth tokens to keyring: {}",
|
||||
error.message()
|
||||
);
|
||||
warn!("{message}");
|
||||
Err(Error::new(error.into_error()).context(message))
|
||||
Err(Error::new(error).context(message))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -220,6 +227,9 @@ fn save_oauth_tokens_with_keyring_with_fallback_to_file<K: KeyringStore>(
|
||||
match save_oauth_tokens_with_keyring(keyring_store, server_name, tokens) {
|
||||
Ok(()) => Ok(()),
|
||||
Err(error) => {
|
||||
if is_keyring_unsupported(&error) {
|
||||
return save_oauth_tokens_to_file(tokens);
|
||||
}
|
||||
let message = error.to_string();
|
||||
warn!("falling back to file storage for OAuth tokens: {message}");
|
||||
save_oauth_tokens_to_file(tokens)
|
||||
@@ -248,11 +258,23 @@ fn delete_oauth_tokens_from_keyring_and_file<K: KeyringStore>(
|
||||
let keyring_removed = match keyring_result {
|
||||
Ok(removed) => removed,
|
||||
Err(error) => {
|
||||
if error.is_unsupported() {
|
||||
return match store_mode {
|
||||
OAuthCredentialsStoreMode::Keyring => {
|
||||
Err(Error::msg("keyring unsupported on this platform"))
|
||||
}
|
||||
OAuthCredentialsStoreMode::Auto | OAuthCredentialsStoreMode::File => {
|
||||
let file_removed = delete_oauth_tokens_from_file(&key)?;
|
||||
Ok(file_removed)
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
let message = error.message();
|
||||
warn!("failed to delete OAuth tokens from keyring: {message}");
|
||||
match store_mode {
|
||||
OAuthCredentialsStoreMode::Auto | OAuthCredentialsStoreMode::Keyring => {
|
||||
return Err(error.into_error())
|
||||
return Err(Error::new(error))
|
||||
.context("failed to delete OAuth tokens from keyring");
|
||||
}
|
||||
OAuthCredentialsStoreMode::File => false,
|
||||
@@ -264,6 +286,14 @@ fn delete_oauth_tokens_from_keyring_and_file<K: KeyringStore>(
|
||||
Ok(keyring_removed || file_removed)
|
||||
}
|
||||
|
||||
fn is_keyring_unsupported(error: &Error) -> bool {
|
||||
error.chain().any(|cause| {
|
||||
cause
|
||||
.downcast_ref::<CredentialStoreError>()
|
||||
.is_some_and(CredentialStoreError::is_unsupported)
|
||||
})
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct OAuthPersistor {
|
||||
inner: Arc<OAuthPersistorInner>,
|
||||
@@ -619,6 +649,32 @@ mod tests {
|
||||
_dir: tempfile::TempDir,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct UnsupportedKeyringStore;
|
||||
|
||||
impl KeyringStore for UnsupportedKeyringStore {
|
||||
fn load(
|
||||
&self,
|
||||
_service: &str,
|
||||
_account: &str,
|
||||
) -> Result<Option<String>, CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
|
||||
fn save(
|
||||
&self,
|
||||
_service: &str,
|
||||
_account: &str,
|
||||
_value: &str,
|
||||
) -> Result<(), CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
|
||||
fn delete(&self, _service: &str, _account: &str) -> Result<bool, CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
}
|
||||
|
||||
impl TempCodexHome {
|
||||
fn new() -> Self {
|
||||
static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
|
||||
@@ -702,6 +758,25 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_oauth_tokens_falls_back_when_keyring_unsupported() -> Result<()> {
|
||||
let _env = TempCodexHome::new();
|
||||
let store = UnsupportedKeyringStore;
|
||||
let tokens = sample_tokens();
|
||||
let expected = tokens.clone();
|
||||
|
||||
super::save_oauth_tokens_to_file(&tokens)?;
|
||||
|
||||
let loaded = super::load_oauth_tokens_from_keyring_with_fallback_to_file(
|
||||
&store,
|
||||
&tokens.server_name,
|
||||
&tokens.url,
|
||||
)?
|
||||
.expect("tokens should load from fallback");
|
||||
assert_tokens_match_without_expiry(&loaded, &expected);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn save_oauth_tokens_prefers_keyring_when_available() -> Result<()> {
|
||||
let _env = TempCodexHome::new();
|
||||
@@ -754,6 +829,23 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn save_oauth_tokens_writes_fallback_when_keyring_unsupported() -> Result<()> {
|
||||
let _env = TempCodexHome::new();
|
||||
let store = UnsupportedKeyringStore;
|
||||
let tokens = sample_tokens();
|
||||
|
||||
super::save_oauth_tokens_with_keyring_with_fallback_to_file(
|
||||
&store,
|
||||
&tokens.server_name,
|
||||
&tokens,
|
||||
)?;
|
||||
|
||||
let fallback_path = super::fallback_file_path()?;
|
||||
assert!(fallback_path.exists(), "fallback file should be created");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn delete_oauth_tokens_removes_all_storage() -> Result<()> {
|
||||
let _env = TempCodexHome::new();
|
||||
@@ -818,6 +910,24 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn delete_oauth_tokens_falls_back_when_keyring_unsupported() -> Result<()> {
|
||||
let _env = TempCodexHome::new();
|
||||
let store = UnsupportedKeyringStore;
|
||||
let tokens = sample_tokens();
|
||||
super::save_oauth_tokens_to_file(&tokens)?;
|
||||
|
||||
let removed = super::delete_oauth_tokens_from_keyring_and_file(
|
||||
&store,
|
||||
OAuthCredentialsStoreMode::Auto,
|
||||
&tokens.server_name,
|
||||
&tokens.url,
|
||||
)?;
|
||||
assert!(removed);
|
||||
assert!(!super::fallback_file_path()?.exists());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn refresh_expires_in_from_timestamp_restores_future_durations() {
|
||||
let mut tokens = sample_tokens();
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::fs;
|
||||
use std::io::Write;
|
||||
#[cfg(unix)]
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
#[cfg(unix)]
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
@@ -35,6 +39,7 @@ use super::keyring_service;
|
||||
|
||||
const SECRETS_VERSION: u8 = 1;
|
||||
const LOCAL_SECRETS_FILENAME: &str = "local.age";
|
||||
const LOCAL_PASSPHRASE_FILENAME: &str = ".passphrase";
|
||||
|
||||
#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)]
|
||||
struct SecretsFile {
|
||||
@@ -115,6 +120,10 @@ impl LocalSecretsBackend {
|
||||
self.secrets_dir().join(LOCAL_SECRETS_FILENAME)
|
||||
}
|
||||
|
||||
fn passphrase_path(&self) -> PathBuf {
|
||||
self.secrets_dir().join(LOCAL_PASSPHRASE_FILENAME)
|
||||
}
|
||||
|
||||
fn load_file(&self) -> Result<SecretsFile> {
|
||||
let path = self.secrets_path();
|
||||
if !path.exists() {
|
||||
@@ -158,26 +167,73 @@ impl LocalSecretsBackend {
|
||||
|
||||
fn load_or_create_passphrase(&self) -> Result<SecretString> {
|
||||
let account = compute_keyring_account(&self.codex_home);
|
||||
let loaded = self
|
||||
.keyring_store
|
||||
.load(keyring_service(), &account)
|
||||
.map_err(|err| anyhow::anyhow!(err.message()))
|
||||
.with_context(|| format!("failed to load secrets key from keyring for {account}"))?;
|
||||
match loaded {
|
||||
Some(existing) => Ok(SecretString::from(existing)),
|
||||
None => {
|
||||
match self.keyring_store.load(keyring_service(), &account) {
|
||||
Ok(Some(existing)) => Ok(SecretString::from(existing)),
|
||||
Ok(None) => {
|
||||
// Generate a high-entropy key and persist it in the OS keyring.
|
||||
// This keeps secrets out of plaintext config while remaining
|
||||
// fully local/offline for the MVP.
|
||||
let generated = generate_passphrase()?;
|
||||
self.keyring_store
|
||||
.save(keyring_service(), &account, generated.expose_secret())
|
||||
.map_err(|err| anyhow::anyhow!(err.message()))
|
||||
.context("failed to persist secrets key in keyring")?;
|
||||
Ok(generated)
|
||||
match self.keyring_store.save(
|
||||
keyring_service(),
|
||||
&account,
|
||||
generated.expose_secret(),
|
||||
) {
|
||||
Ok(()) => Ok(generated),
|
||||
Err(err) => {
|
||||
if err.is_unsupported() {
|
||||
self.save_passphrase_to_file(&generated)?;
|
||||
return Ok(generated);
|
||||
}
|
||||
Err(anyhow::anyhow!(err.message()))
|
||||
.context("failed to persist secrets key in keyring")
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
if err.is_unsupported() {
|
||||
return self.load_or_create_passphrase_from_file();
|
||||
}
|
||||
Err(anyhow::anyhow!(err.message())).with_context(|| {
|
||||
format!("failed to load secrets key from keyring for {account}")
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn load_or_create_passphrase_from_file(&self) -> Result<SecretString> {
|
||||
if let Some(existing) = self.load_passphrase_from_file()? {
|
||||
return Ok(existing);
|
||||
}
|
||||
let generated = generate_passphrase()?;
|
||||
self.save_passphrase_to_file(&generated)?;
|
||||
Ok(generated)
|
||||
}
|
||||
|
||||
fn load_passphrase_from_file(&self) -> Result<Option<SecretString>> {
|
||||
let path = self.passphrase_path();
|
||||
if !path.exists() {
|
||||
return Ok(None);
|
||||
}
|
||||
let raw = fs::read_to_string(&path)
|
||||
.with_context(|| format!("failed to read secrets passphrase at {}", path.display()))?;
|
||||
let trimmed = raw.trim_end();
|
||||
anyhow::ensure!(
|
||||
!trimmed.is_empty(),
|
||||
"secrets passphrase file at {} is empty",
|
||||
path.display()
|
||||
);
|
||||
Ok(Some(SecretString::from(trimmed.to_string())))
|
||||
}
|
||||
|
||||
fn save_passphrase_to_file(&self, passphrase: &SecretString) -> Result<()> {
|
||||
let dir = self.secrets_dir();
|
||||
fs::create_dir_all(&dir)
|
||||
.with_context(|| format!("failed to create secrets dir {}", dir.display()))?;
|
||||
let path = self.passphrase_path();
|
||||
write_private_file_atomically(&path, passphrase.expose_secret().as_bytes())?;
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl SecretsBackend for LocalSecretsBackend {
|
||||
@@ -270,6 +326,77 @@ fn write_file_atomically(path: &Path, contents: &[u8]) -> Result<()> {
|
||||
}
|
||||
}
|
||||
|
||||
fn write_private_file_atomically(path: &Path, contents: &[u8]) -> Result<()> {
|
||||
let dir = path.parent().with_context(|| {
|
||||
format!(
|
||||
"failed to compute parent directory for secrets file at {}",
|
||||
path.display()
|
||||
)
|
||||
})?;
|
||||
let nonce = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.map_or(0, |duration| duration.as_nanos());
|
||||
let file_name = path
|
||||
.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.unwrap_or("secrets");
|
||||
let tmp_path = dir.join(format!(".{file_name}.tmp-{}-{nonce}", std::process::id()));
|
||||
|
||||
{
|
||||
let mut options = fs::OpenOptions::new();
|
||||
options.create_new(true).write(true);
|
||||
#[cfg(unix)]
|
||||
{
|
||||
options.mode(0o600);
|
||||
}
|
||||
let mut tmp_file = options.open(&tmp_path).with_context(|| {
|
||||
format!(
|
||||
"failed to create temp secrets file at {}",
|
||||
tmp_path.display()
|
||||
)
|
||||
})?;
|
||||
tmp_file.write_all(contents).with_context(|| {
|
||||
format!(
|
||||
"failed to write temp secrets file at {}",
|
||||
tmp_path.display()
|
||||
)
|
||||
})?;
|
||||
tmp_file.sync_all().with_context(|| {
|
||||
format!("failed to sync temp secrets file at {}", tmp_path.display())
|
||||
})?;
|
||||
}
|
||||
|
||||
match fs::rename(&tmp_path, path) {
|
||||
Ok(()) => {
|
||||
set_private_permissions(path)?;
|
||||
Ok(())
|
||||
}
|
||||
Err(initial_error) => {
|
||||
let _ = fs::remove_file(&tmp_path);
|
||||
Err(initial_error).with_context(|| {
|
||||
format!(
|
||||
"failed to atomically replace secrets file at {} with {}",
|
||||
path.display(),
|
||||
tmp_path.display()
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn set_private_permissions(path: &Path) -> Result<()> {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
fs::set_permissions(path, fs::Permissions::from_mode(0o600)).with_context(|| {
|
||||
format!(
|
||||
"failed to set permissions for secrets file at {}",
|
||||
path.display()
|
||||
)
|
||||
})?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn generate_passphrase() -> Result<SecretString> {
|
||||
let mut bytes = [0_u8; 32];
|
||||
let mut rng = OsRng;
|
||||
@@ -332,6 +459,7 @@ fn parse_canonical_key(canonical_key: &str) -> Option<SecretListEntry> {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_keyring_store::CredentialStoreError;
|
||||
use codex_keyring_store::tests::MockKeyringStore;
|
||||
use keyring::Error as KeyringError;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -408,4 +536,66 @@ mod tests {
|
||||
assert_eq!(backend.get(&scope, &name)?, Some("two".to_string()));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct UnsupportedKeyringStore;
|
||||
|
||||
impl KeyringStore for UnsupportedKeyringStore {
|
||||
fn load(
|
||||
&self,
|
||||
_service: &str,
|
||||
_account: &str,
|
||||
) -> Result<Option<String>, CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
|
||||
fn save(
|
||||
&self,
|
||||
_service: &str,
|
||||
_account: &str,
|
||||
_value: &str,
|
||||
) -> Result<(), CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
|
||||
fn delete(&self, _service: &str, _account: &str) -> Result<bool, CredentialStoreError> {
|
||||
Err(CredentialStoreError::Unsupported)
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn set_falls_back_to_passphrase_file_when_keyring_unsupported() -> Result<()> {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let backend = LocalSecretsBackend::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
Arc::new(UnsupportedKeyringStore),
|
||||
);
|
||||
let scope = SecretScope::Global;
|
||||
let name = SecretName::new("TEST_SECRET")?;
|
||||
|
||||
backend.set(&scope, &name, "secret-value")?;
|
||||
|
||||
let passphrase_path = backend.passphrase_path();
|
||||
assert!(passphrase_path.exists(), "passphrase file should exist");
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let mode = passphrase_path.metadata()?.permissions().mode() & 0o777;
|
||||
assert_eq!(mode, 0o600);
|
||||
}
|
||||
assert_eq!(
|
||||
backend.get(&scope, &name)?,
|
||||
Some("secret-value".to_string())
|
||||
);
|
||||
|
||||
let reload_backend = LocalSecretsBackend::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
Arc::new(UnsupportedKeyringStore),
|
||||
);
|
||||
assert_eq!(
|
||||
reload_backend.get(&scope, &name)?,
|
||||
Some("secret-value".to_string())
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user