From e2192d475c327bb9c1ea8d7c8eb603701d296250 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Fri, 5 Jun 2026 06:14:44 +0000 Subject: [PATCH] fix(rmcp): allow read-only fallback credentials --- codex-rs/rmcp-client/src/oauth.rs | 63 +++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 7 deletions(-) diff --git a/codex-rs/rmcp-client/src/oauth.rs b/codex-rs/rmcp-client/src/oauth.rs index 01799268e7..ab5db28524 100644 --- a/codex-rs/rmcp-client/src/oauth.rs +++ b/codex-rs/rmcp-client/src/oauth.rs @@ -58,7 +58,7 @@ use tokio::sync::Mutex; use codex_utils_home_dir::find_codex_home; const KEYRING_SERVICE: &str = "Codex MCP Credentials"; -const FALLBACK_WRITE_LOCK_FILENAME: &str = ".credentials.json.lock"; +const FALLBACK_LOCK_PREFIX: &str = "codex-mcp-oauth-fallback"; const FILE_OAUTH_REFRESH_LOCK_PREFIX: &str = ".credentials"; const KEYRING_OAUTH_REFRESH_LOCK_PREFIX: &str = "codex-mcp-oauth-refresh"; const MISSING_REFRESH_TOKEN_ERROR: &str = "No refresh token available"; @@ -683,9 +683,7 @@ async fn acquire_oauth_refresh_lock(path: &Path) -> Result { } fn acquire_fallback_write_lock() -> Result { - let path = find_codex_home()? - .join(FALLBACK_WRITE_LOCK_FILENAME) - .to_path_buf(); + let path = fallback_lock_path()?; let file = open_oauth_lock_file(&path)?; file.lock().with_context(|| { format!( @@ -697,9 +695,7 @@ fn acquire_fallback_write_lock() -> Result { } fn acquire_fallback_read_lock() -> Result { - let path = find_codex_home()? - .join(FALLBACK_WRITE_LOCK_FILENAME) - .to_path_buf(); + let path = fallback_lock_path()?; let file = open_oauth_lock_file(&path)?; file.lock_shared().with_context(|| { format!( @@ -710,6 +706,15 @@ fn acquire_fallback_read_lock() -> Result { Ok(file) } +fn fallback_lock_path() -> Result { + let codex_home = find_codex_home()?; + let codex_home_id = sha_256_bytes_prefix(codex_home.as_os_str().to_string_lossy().as_bytes()); + let user_namespace = os_user_namespace()?; + Ok(os_shared_temp_dir()?.join(format!( + "{FALLBACK_LOCK_PREFIX}-{user_namespace}-{codex_home_id}.lock" + ))) +} + fn open_oauth_lock_file(path: &Path) -> Result { if let Some(parent) = path.parent() { fs::create_dir_all(parent)?; @@ -1038,6 +1043,10 @@ mod tests { _dir: dir, } } + + fn path(&self) -> &Path { + self._dir.path() + } } impl Drop for TempCodexHome { @@ -1230,6 +1239,46 @@ mod tests { Ok(()) } + #[cfg(unix)] + #[test] + fn file_and_auto_reads_work_with_read_only_codex_home() -> Result<()> { + use std::os::unix::fs::PermissionsExt; + + let env = TempCodexHome::new(); + let store = MockKeyringStore::default(); + let tokens = sample_tokens(); + save_oauth_tokens_to_file(&tokens)?; + + let credentials_path = fallback_file_path()?; + fs::set_permissions(&credentials_path, fs::Permissions::from_mode(0o400))?; + fs::set_permissions(env.path(), fs::Permissions::from_mode(0o500))?; + + let result = (|| -> Result<(StoredOAuthTokens, StoredOAuthTokens)> { + let file_tokens = load_oauth_tokens_from_file(&tokens.server_name, &tokens.url)? + .expect("file credentials should load"); + let auto_tokens = load_oauth_tokens_from_keyring_with_fallback_to_file( + &store, + &tokens.server_name, + &tokens.url, + )? + .expect("auto credentials should load from fallback"); + Ok((file_tokens, auto_tokens)) + })(); + + fs::set_permissions(env.path(), fs::Permissions::from_mode(0o700))?; + fs::set_permissions(&credentials_path, fs::Permissions::from_mode(0o600))?; + + let (file_tokens, auto_tokens) = result?; + assert_tokens_match_without_expiry(&file_tokens, &tokens); + assert_tokens_match_without_expiry(&auto_tokens, &tokens); + assert!(!env.path().join(".credentials.json.lock").exists()); + assert_eq!( + fallback_lock_path()?.parent(), + Some(os_shared_temp_dir()?.as_path()) + ); + Ok(()) + } + #[test] fn refresh_reauthentication_classification_is_narrow() { assert!(refresh_requires_reauthentication(