mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: add auth-specific encrypted secret namespaces (#27535)
## 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`
This commit is contained in:
committed by
GitHub
Unverified
parent
d1aaf789ad
commit
7cc80b39f1
@@ -45,7 +45,7 @@ pub trait KeyringStore: Debug + Send + Sync {
|
||||
fn delete(&self, service: &str, account: &str) -> Result<bool, CredentialStoreError>;
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
pub struct DefaultKeyringStore;
|
||||
|
||||
impl KeyringStore for DefaultKeyringStore {
|
||||
|
||||
@@ -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<dyn KeyringStore>,
|
||||
namespace: LocalSecretsNamespace,
|
||||
) -> Self {
|
||||
let backend: Arc<dyn SecretsBackend> = 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())
|
||||
|
||||
@@ -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<dyn KeyringStore>,
|
||||
namespace: LocalSecretsNamespace,
|
||||
}
|
||||
|
||||
impl LocalSecretsBackend {
|
||||
pub fn new(codex_home: PathBuf, keyring_store: Arc<dyn KeyringStore>) -> Self {
|
||||
Self::new_with_namespace(
|
||||
codex_home,
|
||||
keyring_store,
|
||||
LocalSecretsNamespace::ManagedSecrets,
|
||||
)
|
||||
}
|
||||
|
||||
pub fn new_with_namespace(
|
||||
codex_home: PathBuf,
|
||||
keyring_store: Arc<dyn KeyringStore>,
|
||||
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<SecretsFile> {
|
||||
@@ -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(())
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user