diff --git a/codex-rs/keyring-store/src/lib.rs b/codex-rs/keyring-store/src/lib.rs index 10dad3a98..ee91af114 100644 --- a/codex-rs/keyring-store/src/lib.rs +++ b/codex-rs/keyring-store/src/lib.rs @@ -45,7 +45,7 @@ pub trait KeyringStore: Debug + Send + Sync { fn delete(&self, service: &str, account: &str) -> Result; } -#[derive(Debug)] +#[derive(Debug, Clone, Copy)] pub struct DefaultKeyringStore; impl KeyringStore for DefaultKeyringStore { diff --git a/codex-rs/secrets/src/lib.rs b/codex-rs/secrets/src/lib.rs index 280c723d3..c3ddc8a24 100644 --- a/codex-rs/secrets/src/lib.rs +++ b/codex-rs/secrets/src/lib.rs @@ -17,6 +17,7 @@ mod local; mod sanitizer; pub use local::LocalSecretsBackend; +pub use local::LocalSecretsNamespace; pub use sanitizer::redact_secrets; const KEYRING_SERVICE: &str = "codex"; @@ -122,6 +123,22 @@ impl SecretsManager { Self { backend } } + pub fn new_with_keyring_store_and_namespace( + codex_home: PathBuf, + backend_kind: SecretsBackendKind, + keyring_store: Arc, + namespace: LocalSecretsNamespace, + ) -> Self { + let backend: Arc = match backend_kind { + SecretsBackendKind::Local => Arc::new(LocalSecretsBackend::new_with_namespace( + codex_home, + keyring_store, + namespace, + )), + }; + Self { backend } + } + pub fn set(&self, scope: &SecretScope, name: &SecretName, value: &str) -> Result<()> { self.backend.set(scope, name, value) } @@ -162,7 +179,8 @@ pub fn environment_id_from_cwd(cwd: &Path) -> String { format!("cwd-{short}") } -pub(crate) fn compute_keyring_account(codex_home: &Path) -> String { +/// Computes the OS keyring account name used to store the local secrets passphrase. +pub fn compute_keyring_account(codex_home: &Path) -> String { let canonical = codex_home .canonicalize() .unwrap_or_else(|_| codex_home.to_path_buf()) diff --git a/codex-rs/secrets/src/local.rs b/codex-rs/secrets/src/local.rs index 127fc84c5..366be386b 100644 --- a/codex-rs/secrets/src/local.rs +++ b/codex-rs/secrets/src/local.rs @@ -35,6 +35,20 @@ use super::keyring_service; const SECRETS_VERSION: u8 = 1; const LOCAL_SECRETS_FILENAME: &str = "local.age"; +const CODEX_AUTH_SECRETS_FILENAME: &str = "codex_auth.age"; +const MCP_OAUTH_SECRETS_FILENAME: &str = "mcp_oauth.age"; + +/// Selects the local encrypted file used by a `LocalSecretsBackend`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum LocalSecretsNamespace { + /// General managed secrets stored in `local.age`. + #[default] + ManagedSecrets, + /// Codex authentication credentials used by the CLI, TUI, app server, and other clients. + CodexAuth, + /// OAuth credentials for external MCP servers. + McpOAuth, +} #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] struct SecretsFile { @@ -55,13 +69,27 @@ impl SecretsFile { pub struct LocalSecretsBackend { codex_home: PathBuf, keyring_store: Arc, + namespace: LocalSecretsNamespace, } impl LocalSecretsBackend { pub fn new(codex_home: PathBuf, keyring_store: Arc) -> Self { + Self::new_with_namespace( + codex_home, + keyring_store, + LocalSecretsNamespace::ManagedSecrets, + ) + } + + pub fn new_with_namespace( + codex_home: PathBuf, + keyring_store: Arc, + namespace: LocalSecretsNamespace, + ) -> Self { Self { codex_home, keyring_store, + namespace, } } @@ -112,7 +140,12 @@ impl LocalSecretsBackend { } fn secrets_path(&self) -> PathBuf { - self.secrets_dir().join(LOCAL_SECRETS_FILENAME) + let filename = match self.namespace { + LocalSecretsNamespace::ManagedSecrets => LOCAL_SECRETS_FILENAME, + LocalSecretsNamespace::CodexAuth => CODEX_AUTH_SECRETS_FILENAME, + LocalSecretsNamespace::McpOAuth => MCP_OAUTH_SECRETS_FILENAME, + }; + self.secrets_dir().join(filename) } fn load_file(&self) -> Result { @@ -208,8 +241,15 @@ fn write_file_atomically(path: &Path, contents: &[u8]) -> Result<()> { let nonce = SystemTime::now() .duration_since(UNIX_EPOCH) .map_or(0, |duration| duration.as_nanos()); + let filename = path.file_name().with_context(|| { + format!( + "failed to compute filename for secrets file at {}", + path.display() + ) + })?; let tmp_path = dir.join(format!( - ".{LOCAL_SECRETS_FILENAME}.tmp-{}-{nonce}", + ".{}.tmp-{}-{nonce}", + filename.to_string_lossy(), std::process::id() )); @@ -408,4 +448,50 @@ mod tests { assert_eq!(backend.get(&scope, &name)?, Some("two".to_string())); Ok(()) } + + #[test] + fn local_namespaces_write_separate_files() -> Result<()> { + let codex_home = tempfile::tempdir().expect("tempdir"); + let keyring = Arc::new(MockKeyringStore::default()); + let codex_auth_backend = LocalSecretsBackend::new_with_namespace( + codex_home.path().to_path_buf(), + keyring.clone(), + LocalSecretsNamespace::CodexAuth, + ); + let mcp_backend = LocalSecretsBackend::new_with_namespace( + codex_home.path().to_path_buf(), + keyring, + LocalSecretsNamespace::McpOAuth, + ); + let scope = SecretScope::Global; + let name = SecretName::new("TEST_SECRET")?; + + codex_auth_backend.set(&scope, &name, "codex-auth-value")?; + mcp_backend.set(&scope, &name, "mcp-value")?; + + assert_eq!( + codex_auth_backend.get(&scope, &name)?, + Some("codex-auth-value".to_string()) + ); + assert_eq!( + mcp_backend.get(&scope, &name)?, + Some("mcp-value".to_string()) + ); + assert!( + codex_home + .path() + .join("secrets") + .join("codex_auth.age") + .exists() + ); + assert!( + codex_home + .path() + .join("secrets") + .join("mcp_oauth.age") + .exists() + ); + assert!(!codex_home.path().join("secrets").join("local.age").exists()); + Ok(()) + } }