mirror of
https://github.com/openai/codex.git
synced 2026-02-01 14:44:17 +00:00
Validate CODEX_HOME before resolving (#10249)
Summary - require `CODEX_HOME` to point to an existing directory before canonicalizing and surface clear errors otherwise - share the same helper logic in both `core` and `rmcp-client` and add unit tests that cover missing, non-directory, valid, and default paths This addresses #9222
This commit is contained in:
13
codex-rs/Cargo.lock
generated
13
codex-rs/Cargo.lock
generated
@@ -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"
|
||||
|
||||
@@ -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" }
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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<PathBuf> {
|
||||
// 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
|
||||
|
||||
@@ -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" }
|
||||
|
||||
@@ -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<PathBuf> {
|
||||
// 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)
|
||||
}
|
||||
@@ -1,5 +1,4 @@
|
||||
mod auth_status;
|
||||
mod find_codex_home;
|
||||
mod logging_client_handler;
|
||||
mod oauth;
|
||||
mod perform_oauth_login;
|
||||
|
||||
@@ -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;
|
||||
|
||||
6
codex-rs/utils/home-dir/BUILD.bazel
Normal file
6
codex-rs/utils/home-dir/BUILD.bazel
Normal file
@@ -0,0 +1,6 @@
|
||||
load("//:defs.bzl", "codex_rust_crate")
|
||||
|
||||
codex_rust_crate(
|
||||
name = "home-dir",
|
||||
crate_name = "codex_utils_home_dir",
|
||||
)
|
||||
15
codex-rs/utils/home-dir/Cargo.toml
Normal file
15
codex-rs/utils/home-dir/Cargo.toml
Normal file
@@ -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 }
|
||||
128
codex-rs/utils/home-dir/src/lib.rs
Normal file
128
codex-rs/utils/home-dir/src/lib.rs
Normal file
@@ -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<PathBuf> {
|
||||
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<PathBuf> {
|
||||
// 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);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user