From 7cc80b39f1247beb9319228d9b3129c510763914 Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Fri, 12 Jun 2026 12:52:49 -0700 Subject: [PATCH] feat: add auth-specific encrypted secret namespaces (#27535) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why CLI auth and MCP OAuth credentials should use separate encrypted files while sharing the existing local-secrets implementation and OS-keyring-backed encryption key mechanism. This is the second PR in the encrypted-auth stack: 1. #27504 — feature and config selection 2. This PR — auth-specific local-secrets namespaces 3. CLI auth implementation and activation 4. MCP OAuth implementation and activation ## What Changed - Added `LocalSecretsNamespace` variants for shared secrets, CLI auth, and MCP OAuth. - Selected `local.age`, `cli_auth.age`, or `mcp_oauth.age` from the namespace. - Made atomic temporary filenames derive from the selected secrets filename. - Added namespaced `SecretsManager` construction and coverage proving the auth namespaces write separate encrypted files. - Made the default keyring store clonable for downstream namespaced auth backends. This PR does not activate either auth backend or change existing credential behavior. ## Validation - `just test -p codex-secrets` — 7 passed - `just test -p codex-keyring-store` — package has no test binaries - `just fmt` --- codex-rs/keyring-store/src/lib.rs | 2 +- codex-rs/secrets/src/lib.rs | 20 ++++++- codex-rs/secrets/src/local.rs | 90 ++++++++++++++++++++++++++++++- 3 files changed, 108 insertions(+), 4 deletions(-) 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(()) + } }