diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 5cb338be1d..c46c24fb31 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1400,6 +1400,7 @@ dependencies = [ "codex-state", "codex-utils-absolute-path", "codex-utils-cargo-bin", + "codex-utils-home-dir", "codex-utils-pty", "codex-utils-readiness", "codex-utils-string", @@ -1407,7 +1408,6 @@ dependencies = [ "core-foundation 0.9.4", "core_test_support", "ctor 0.6.3", - "dirs", "dunce", "encoding_rs", "env-flags", @@ -1842,7 +1842,7 @@ dependencies = [ "codex-keyring-store", "codex-protocol", "codex-utils-cargo-bin", - "dirs", + "codex-utils-home-dir", "futures", "keyring", "mcp-types", @@ -2007,6 +2007,15 @@ dependencies = [ "thiserror 2.0.17", ] +[[package]] +name = "codex-utils-home-dir" +version = "0.0.0" +dependencies = [ + "dirs", + "pretty_assertions", + "tempfile", +] + [[package]] name = "codex-utils-image" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 327e6beeb0..84210838fc 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -43,6 +43,7 @@ members = [ "utils/cache", "utils/image", "utils/json-to-toml", + "utils/home-dir", "utils/pty", "utils/readiness", "utils/string", @@ -102,6 +103,7 @@ codex-utils-cache = { path = "utils/cache" } codex-utils-cargo-bin = { path = "utils/cargo-bin" } codex-utils-image = { path = "utils/image" } codex-utils-json-to-toml = { path = "utils/json-to-toml" } +codex-utils-home-dir = { path = "utils/home-dir" } codex-utils-pty = { path = "utils/pty" } codex-utils-readiness = { path = "utils/readiness" } codex-utils-string = { path = "utils/string" } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index b336b296c6..1715a04142 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -39,11 +39,11 @@ codex-protocol = { workspace = true } codex-rmcp-client = { workspace = true } codex-state = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-home-dir = { workspace = true } codex-utils-pty = { workspace = true } codex-utils-readiness = { workspace = true } codex-utils-string = { workspace = true } codex-windows-sandbox = { package = "codex-windows-sandbox", path = "../windows-sandbox-rs" } -dirs = { workspace = true } dunce = { workspace = true } encoding_rs = { workspace = true } env-flags = { workspace = true } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index bb84cd6755..580f5de9f2 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -57,7 +57,6 @@ use codex_protocol::openai_models::ReasoningEffort; use codex_rmcp_client::OAuthCredentialsStoreMode; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; -use dirs::home_dir; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -1753,27 +1752,12 @@ fn toml_uses_deprecated_instructions_file(value: &TomlValue) -> bool { /// specified by the `CODEX_HOME` environment variable. If not set, defaults to /// `~/.codex`. /// -/// - If `CODEX_HOME` is set, the value will be canonicalized and this -/// function will Err if the path does not exist. +/// - If `CODEX_HOME` is set, the value must exist and be a directory. The +/// value will be canonicalized and this function will Err otherwise. /// - If `CODEX_HOME` is not set, this function does not verify that the /// directory exists. pub fn find_codex_home() -> std::io::Result { - // Honor the `CODEX_HOME` environment variable when it is set to allow users - // (and tests) to override the default location. - if let Ok(val) = std::env::var("CODEX_HOME") - && !val.is_empty() - { - return PathBuf::from(val).canonicalize(); - } - - let mut p = home_dir().ok_or_else(|| { - std::io::Error::new( - std::io::ErrorKind::NotFound, - "Could not find home directory", - ) - })?; - p.push(".codex"); - Ok(p) + codex_utils_home_dir::find_codex_home() } /// Returns the path to the folder where Codex logs are stored. Does not verify diff --git a/codex-rs/rmcp-client/Cargo.toml b/codex-rs/rmcp-client/Cargo.toml index 8aa7512faf..4b2d6964f4 100644 --- a/codex-rs/rmcp-client/Cargo.toml +++ b/codex-rs/rmcp-client/Cargo.toml @@ -15,7 +15,7 @@ axum = { workspace = true, default-features = false, features = [ ] } codex-keyring-store = { workspace = true } codex-protocol = { workspace = true } -dirs = { workspace = true } +codex-utils-home-dir = { workspace = true } futures = { workspace = true, default-features = false, features = ["std"] } keyring = { workspace = true, features = ["crypto-rust"] } mcp-types = { path = "../mcp-types" } diff --git a/codex-rs/rmcp-client/src/find_codex_home.rs b/codex-rs/rmcp-client/src/find_codex_home.rs deleted file mode 100644 index d683ba9d16..0000000000 --- a/codex-rs/rmcp-client/src/find_codex_home.rs +++ /dev/null @@ -1,33 +0,0 @@ -use dirs::home_dir; -use std::path::PathBuf; - -/// This was copied from codex-core but codex-core depends on this crate. -/// TODO: move this to a shared crate lower in the dependency tree. -/// -/// -/// Returns the path to the Codex configuration directory, which can be -/// specified by the `CODEX_HOME` environment variable. If not set, defaults to -/// `~/.codex`. -/// -/// - If `CODEX_HOME` is set, the value will be canonicalized and this -/// function will Err if the path does not exist. -/// - If `CODEX_HOME` is not set, this function does not verify that the -/// directory exists. -pub(crate) fn find_codex_home() -> std::io::Result { - // Honor the `CODEX_HOME` environment variable when it is set to allow users - // (and tests) to override the default location. - if let Ok(val) = std::env::var("CODEX_HOME") - && !val.is_empty() - { - return PathBuf::from(val).canonicalize(); - } - - let mut p = home_dir().ok_or_else(|| { - std::io::Error::new( - std::io::ErrorKind::NotFound, - "Could not find home directory", - ) - })?; - p.push(".codex"); - Ok(p) -} diff --git a/codex-rs/rmcp-client/src/lib.rs b/codex-rs/rmcp-client/src/lib.rs index e4d1f3b9f0..a10d3b29ae 100644 --- a/codex-rs/rmcp-client/src/lib.rs +++ b/codex-rs/rmcp-client/src/lib.rs @@ -1,5 +1,4 @@ mod auth_status; -mod find_codex_home; mod logging_client_handler; mod oauth; mod perform_oauth_login; diff --git a/codex-rs/rmcp-client/src/oauth.rs b/codex-rs/rmcp-client/src/oauth.rs index a3a256374b..cdb64ff151 100644 --- a/codex-rs/rmcp-client/src/oauth.rs +++ b/codex-rs/rmcp-client/src/oauth.rs @@ -48,7 +48,7 @@ use codex_keyring_store::KeyringStore; use rmcp::transport::auth::AuthorizationManager; use tokio::sync::Mutex; -use crate::find_codex_home::find_codex_home; +use codex_utils_home_dir::find_codex_home; const KEYRING_SERVICE: &str = "Codex MCP Credentials"; const REFRESH_SKEW_MILLIS: u64 = 30_000; diff --git a/codex-rs/utils/home-dir/BUILD.bazel b/codex-rs/utils/home-dir/BUILD.bazel new file mode 100644 index 0000000000..8cae2e9239 --- /dev/null +++ b/codex-rs/utils/home-dir/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "home-dir", + crate_name = "codex_utils_home_dir", +) diff --git a/codex-rs/utils/home-dir/Cargo.toml b/codex-rs/utils/home-dir/Cargo.toml new file mode 100644 index 0000000000..ff263fbed4 --- /dev/null +++ b/codex-rs/utils/home-dir/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "codex-utils-home-dir" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lints] +workspace = true + +[dependencies] +dirs = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/utils/home-dir/src/lib.rs b/codex-rs/utils/home-dir/src/lib.rs new file mode 100644 index 0000000000..17be887a83 --- /dev/null +++ b/codex-rs/utils/home-dir/src/lib.rs @@ -0,0 +1,128 @@ +use dirs::home_dir; +use std::path::PathBuf; + +/// Returns the path to the Codex configuration directory, which can be +/// specified by the `CODEX_HOME` environment variable. If not set, defaults to +/// `~/.codex`. +/// +/// - If `CODEX_HOME` is set, the value must exist and be a directory. The +/// value will be canonicalized and this function will Err otherwise. +/// - If `CODEX_HOME` is not set, this function does not verify that the +/// directory exists. +pub fn find_codex_home() -> std::io::Result { + let codex_home_env = std::env::var("CODEX_HOME") + .ok() + .filter(|val| !val.is_empty()); + find_codex_home_from_env(codex_home_env.as_deref()) +} + +fn find_codex_home_from_env(codex_home_env: Option<&str>) -> std::io::Result { + // Honor the `CODEX_HOME` environment variable when it is set to allow users + // (and tests) to override the default location. + match codex_home_env { + Some(val) => { + let path = PathBuf::from(val); + let metadata = std::fs::metadata(&path).map_err(|err| match err.kind() { + std::io::ErrorKind::NotFound => std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("CODEX_HOME points to {val:?}, but that path does not exist"), + ), + _ => std::io::Error::new( + err.kind(), + format!("failed to read CODEX_HOME {val:?}: {err}"), + ), + })?; + + if !metadata.is_dir() { + Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("CODEX_HOME points to {val:?}, but that path is not a directory"), + )) + } else { + path.canonicalize().map_err(|err| { + std::io::Error::new( + err.kind(), + format!("failed to canonicalize CODEX_HOME {val:?}: {err}"), + ) + }) + } + } + None => { + let mut p = home_dir().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::NotFound, + "Could not find home directory", + ) + })?; + p.push(".codex"); + Ok(p) + } + } +} + +#[cfg(test)] +mod tests { + use super::find_codex_home_from_env; + use dirs::home_dir; + use pretty_assertions::assert_eq; + use std::fs; + use std::io::ErrorKind; + use tempfile::TempDir; + + #[test] + fn find_codex_home_env_missing_path_is_fatal() { + let temp_home = TempDir::new().expect("temp home"); + let missing = temp_home.path().join("missing-codex-home"); + let missing_str = missing + .to_str() + .expect("missing codex home path should be valid utf-8"); + + let err = find_codex_home_from_env(Some(missing_str)).expect_err("missing CODEX_HOME"); + assert_eq!(err.kind(), ErrorKind::NotFound); + assert!( + err.to_string().contains("CODEX_HOME"), + "unexpected error: {err}" + ); + } + + #[test] + fn find_codex_home_env_file_path_is_fatal() { + let temp_home = TempDir::new().expect("temp home"); + let file_path = temp_home.path().join("codex-home.txt"); + fs::write(&file_path, "not a directory").expect("write temp file"); + let file_str = file_path + .to_str() + .expect("file codex home path should be valid utf-8"); + + let err = find_codex_home_from_env(Some(file_str)).expect_err("file CODEX_HOME"); + assert_eq!(err.kind(), ErrorKind::InvalidInput); + assert!( + err.to_string().contains("not a directory"), + "unexpected error: {err}" + ); + } + + #[test] + fn find_codex_home_env_valid_directory_canonicalizes() { + let temp_home = TempDir::new().expect("temp home"); + let temp_str = temp_home + .path() + .to_str() + .expect("temp codex home path should be valid utf-8"); + + let resolved = find_codex_home_from_env(Some(temp_str)).expect("valid CODEX_HOME"); + let expected = temp_home + .path() + .canonicalize() + .expect("canonicalize temp home"); + assert_eq!(resolved, expected); + } + + #[test] + fn find_codex_home_without_env_uses_default_home_dir() { + let resolved = find_codex_home_from_env(None).expect("default CODEX_HOME"); + let mut expected = home_dir().expect("home dir"); + expected.push(".codex"); + assert_eq!(resolved, expected); + } +}