diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index f97903b31..9c12272d9 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -1,13 +1,14 @@ use crate::config::CONFIG_TOML_FILE; use crate::config::types::McpServerConfig; use crate::config::types::Notice; +use crate::path_utils::resolve_symlink_write_paths; +use crate::path_utils::write_atomically; use anyhow::Context; use codex_protocol::config_types::TrustLevel; use codex_protocol::openai_models::ReasoningEffort; use std::collections::BTreeMap; use std::path::Path; use std::path::PathBuf; -use tempfile::NamedTempFile; use tokio::task; use toml_edit::ArrayOfTables; use toml_edit::DocumentMut; @@ -625,10 +626,14 @@ pub fn apply_blocking( } let config_path = codex_home.join(CONFIG_TOML_FILE); - let serialized = match std::fs::read_to_string(&config_path) { - Ok(contents) => contents, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => String::new(), - Err(err) => return Err(err.into()), + let write_paths = resolve_symlink_write_paths(&config_path)?; + let serialized = match write_paths.read_path { + Some(path) => match std::fs::read_to_string(&path) { + Ok(contents) => contents, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(err) => return Err(err.into()), + }, + None => String::new(), }; let doc = if serialized.is_empty() { @@ -654,22 +659,13 @@ pub fn apply_blocking( return Ok(()); } - std::fs::create_dir_all(codex_home).with_context(|| { + write_atomically(&write_paths.write_path, &document.doc.to_string()).with_context(|| { format!( - "failed to create Codex home directory at {}", - codex_home.display() + "failed to persist config.toml at {}", + write_paths.write_path.display() ) })?; - let tmp = NamedTempFile::new_in(codex_home)?; - std::fs::write(tmp.path(), document.doc.to_string()).with_context(|| { - format!( - "failed to write temporary config file at {}", - tmp.path().display() - ) - })?; - tmp.persist(config_path)?; - Ok(()) } @@ -813,6 +809,8 @@ mod tests { use crate::config::types::McpServerTransportConfig; use codex_protocol::openai_models::ReasoningEffort; use pretty_assertions::assert_eq; + #[cfg(unix)] + use std::os::unix::fs::symlink; use tempfile::tempdir; use toml::Value as TomlValue; @@ -952,6 +950,71 @@ profiles = { fast = { model = "gpt-4o", sandbox_mode = "strict" } } ); } + #[cfg(unix)] + #[test] + fn blocking_set_model_writes_through_symlink_chain() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + let target_dir = tempdir().expect("target dir"); + let target_path = target_dir.path().join(CONFIG_TOML_FILE); + let link_path = codex_home.join("config-link.toml"); + let config_path = codex_home.join(CONFIG_TOML_FILE); + + symlink(&target_path, &link_path).expect("symlink link"); + symlink("config-link.toml", &config_path).expect("symlink config"); + + apply_blocking( + codex_home, + None, + &[ConfigEdit::SetModel { + model: Some("gpt-5.1-codex".to_string()), + effort: Some(ReasoningEffort::High), + }], + ) + .expect("persist"); + + let meta = std::fs::symlink_metadata(&config_path).expect("config metadata"); + assert!(meta.file_type().is_symlink()); + + let contents = std::fs::read_to_string(&target_path).expect("read target"); + let expected = r#"model = "gpt-5.1-codex" +model_reasoning_effort = "high" +"#; + assert_eq!(contents, expected); + } + + #[cfg(unix)] + #[test] + fn blocking_set_model_replaces_symlink_on_cycle() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + let link_a = codex_home.join("a.toml"); + let link_b = codex_home.join("b.toml"); + let config_path = codex_home.join(CONFIG_TOML_FILE); + + symlink("b.toml", &link_a).expect("symlink a"); + symlink("a.toml", &link_b).expect("symlink b"); + symlink("a.toml", &config_path).expect("symlink config"); + + apply_blocking( + codex_home, + None, + &[ConfigEdit::SetModel { + model: Some("gpt-5.1-codex".to_string()), + effort: None, + }], + ) + .expect("persist"); + + let meta = std::fs::symlink_metadata(&config_path).expect("config metadata"); + assert!(!meta.file_type().is_symlink()); + + let contents = std::fs::read_to_string(&config_path).expect("read config"); + let expected = r#"model = "gpt-5.1-codex" +"#; + assert_eq!(contents, expected); + } + #[test] fn batch_write_table_upsert_preserves_inline_comments() { let tmp = tempdir().expect("tmpdir"); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 956df12e9..970ec7c4d 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1,4 +1,6 @@ use crate::auth::AuthCredentialsStoreMode; +use crate::config::edit::ConfigEdit; +use crate::config::edit::ConfigEditsBuilder; use crate::config::types::DEFAULT_OTEL_ENVIRONMENT; use crate::config::types::History; use crate::config::types::McpServerConfig; @@ -751,30 +753,17 @@ pub fn set_default_oss_provider(codex_home: &Path, provider: &str) -> std::io::R )); } } - let config_path = codex_home.join(CONFIG_TOML_FILE); - - // Read existing config or create empty string if file doesn't exist - let content = match std::fs::read_to_string(&config_path) { - Ok(content) => content, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => String::new(), - Err(e) => return Err(e), - }; - - // Parse as DocumentMut for editing while preserving structure - let mut doc = content.parse::().map_err(|e| { - std::io::Error::new( - std::io::ErrorKind::InvalidData, - format!("failed to parse config.toml: {e}"), - ) - })?; - - // Set the default_oss_provider at root level use toml_edit::value; - doc["oss_provider"] = value(provider); - // Write the modified document back - std::fs::write(&config_path, doc.to_string())?; - Ok(()) + let edits = [ConfigEdit::SetPath { + segments: vec!["oss_provider".to_string()], + value: value(provider), + }]; + + ConfigEditsBuilder::new(codex_home) + .with_edits(edits) + .apply_blocking() + .map_err(|err| std::io::Error::other(format!("failed to persist config.toml: {err}"))) } /// Base config deserialized from ~/.codex/config.toml. diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/core/src/config/service.rs index 913c02df1..b6ed9f617 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/core/src/config/service.rs @@ -9,6 +9,9 @@ use crate::config_loader::LoaderOverrides; use crate::config_loader::load_config_layers_state; use crate::config_loader::merge_toml_values; use crate::path_utils; +use crate::path_utils::SymlinkWritePaths; +use crate::path_utils::resolve_symlink_write_paths; +use crate::path_utils::write_atomically; use codex_app_server_protocol::Config as ApiConfig; use codex_app_server_protocol::ConfigBatchWriteParams; use codex_app_server_protocol::ConfigLayerMetadata; @@ -27,6 +30,7 @@ use std::borrow::Cow; use std::path::Path; use std::path::PathBuf; use thiserror::Error; +use tokio::task; use toml::Value as TomlValue; use toml_edit::Item as TomlItem; @@ -362,19 +366,30 @@ impl ConfigService { async fn create_empty_user_layer( config_toml: &AbsolutePathBuf, ) -> Result { - let toml_value = match tokio::fs::read_to_string(config_toml).await { - Ok(contents) => toml::from_str(&contents).map_err(|e| { - ConfigServiceError::toml("failed to parse existing user config.toml", e) - })?, - Err(e) => { - if e.kind() == std::io::ErrorKind::NotFound { - tokio::fs::write(config_toml, "").await.map_err(|e| { - ConfigServiceError::io("failed to create empty user config.toml", e) - })?; + let SymlinkWritePaths { + read_path, + write_path, + } = resolve_symlink_write_paths(config_toml.as_path()) + .map_err(|err| ConfigServiceError::io("failed to resolve user config path", err))?; + let toml_value = match read_path { + Some(path) => match tokio::fs::read_to_string(&path).await { + Ok(contents) => toml::from_str(&contents).map_err(|e| { + ConfigServiceError::toml("failed to parse existing user config.toml", e) + })?, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + write_empty_user_config(write_path.clone()).await?; TomlValue::Table(toml::map::Map::new()) - } else { - return Err(ConfigServiceError::io("failed to read user config.toml", e)); } + Err(err) => { + return Err(ConfigServiceError::io( + "failed to read user config.toml", + err, + )); + } + }, + None => { + write_empty_user_config(write_path).await?; + TomlValue::Table(toml::map::Map::new()) } }; Ok(ConfigLayerEntry::new( @@ -385,6 +400,13 @@ async fn create_empty_user_layer( )) } +async fn write_empty_user_config(write_path: PathBuf) -> Result<(), ConfigServiceError> { + task::spawn_blocking(move || write_atomically(&write_path, "")) + .await + .map_err(|err| ConfigServiceError::anyhow("config persistence task panicked", err.into()))? + .map_err(|err| ConfigServiceError::io("failed to create empty user config.toml", err)) +} + fn parse_value(value: JsonValue) -> Result, String> { if value.is_null() { return Ok(None); diff --git a/codex-rs/core/src/path_utils.rs b/codex-rs/core/src/path_utils.rs index 65b3db0f6..4885679e5 100644 --- a/codex-rs/core/src/path_utils.rs +++ b/codex-rs/core/src/path_utils.rs @@ -1,5 +1,9 @@ +use codex_utils_absolute_path::AbsolutePathBuf; +use std::collections::HashSet; +use std::io; use std::path::Path; use std::path::PathBuf; +use tempfile::NamedTempFile; use crate::env; @@ -8,6 +12,106 @@ pub fn normalize_for_path_comparison(path: impl AsRef) -> std::io::Result< Ok(normalize_for_wsl(canonical)) } +pub struct SymlinkWritePaths { + pub read_path: Option, + pub write_path: PathBuf, +} + +/// Resolve the final filesystem target for `path` while retaining a safe write path. +/// +/// This follows symlink chains (including relative symlink targets) until it reaches a +/// non-symlink path. If the chain cycles or any metadata/link resolution fails, it +/// returns `read_path: None` and uses the original absolute path as `write_path`. +/// There is no fixed max-resolution count; cycles are detected via a visited set. +pub fn resolve_symlink_write_paths(path: &Path) -> io::Result { + let root = AbsolutePathBuf::from_absolute_path(path) + .map(AbsolutePathBuf::into_path_buf) + .unwrap_or_else(|_| path.to_path_buf()); + let mut current = root.clone(); + let mut visited = HashSet::new(); + + // Follow symlink chains while guarding against cycles. + loop { + let meta = match std::fs::symlink_metadata(¤t) { + Ok(meta) => meta, + Err(err) if err.kind() == io::ErrorKind::NotFound => { + return Ok(SymlinkWritePaths { + read_path: Some(current.clone()), + write_path: current, + }); + } + Err(_) => { + return Ok(SymlinkWritePaths { + read_path: None, + write_path: root, + }); + } + }; + + if !meta.file_type().is_symlink() { + return Ok(SymlinkWritePaths { + read_path: Some(current.clone()), + write_path: current, + }); + } + + // If we've already seen this path, the chain cycles. + if !visited.insert(current.clone()) { + return Ok(SymlinkWritePaths { + read_path: None, + write_path: root, + }); + } + + let target = match std::fs::read_link(¤t) { + Ok(target) => target, + Err(_) => { + return Ok(SymlinkWritePaths { + read_path: None, + write_path: root, + }); + } + }; + + let next = if target.is_absolute() { + AbsolutePathBuf::from_absolute_path(&target) + } else if let Some(parent) = current.parent() { + AbsolutePathBuf::resolve_path_against_base(&target, parent) + } else { + return Ok(SymlinkWritePaths { + read_path: None, + write_path: root, + }); + }; + + let next = match next { + Ok(path) => path.into_path_buf(), + Err(_) => { + return Ok(SymlinkWritePaths { + read_path: None, + write_path: root, + }); + } + }; + + current = next; + } +} + +pub fn write_atomically(write_path: &Path, contents: &str) -> io::Result<()> { + let parent = write_path.parent().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + format!("path {} has no parent directory", write_path.display()), + ) + })?; + std::fs::create_dir_all(parent)?; + let tmp = NamedTempFile::new_in(parent)?; + std::fs::write(tmp.path(), contents)?; + tmp.persist(write_path)?; + Ok(()) +} + fn normalize_for_wsl(path: PathBuf) -> PathBuf { normalize_for_wsl_with_flag(path, env::is_wsl()) } @@ -84,6 +188,29 @@ fn lower_ascii_path(path: PathBuf) -> PathBuf { #[cfg(test)] mod tests { + #[cfg(unix)] + mod symlinks { + use super::super::resolve_symlink_write_paths; + use pretty_assertions::assert_eq; + use std::os::unix::fs::symlink; + + #[test] + fn symlink_cycles_fall_back_to_root_write_path() -> std::io::Result<()> { + let dir = tempfile::tempdir()?; + let a = dir.path().join("a"); + let b = dir.path().join("b"); + + symlink(&b, &a)?; + symlink(&a, &b)?; + + let resolved = resolve_symlink_write_paths(&a)?; + + assert_eq!(resolved.read_path, None); + assert_eq!(resolved.write_path, a); + Ok(()) + } + } + #[cfg(target_os = "linux")] mod wsl { use super::super::normalize_for_wsl_with_flag;