From 080c78a3215a257240b378fe43c64ab2adda545f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yasin=20O=CC=88zmen?= Date: Wed, 26 Aug 2026 08:05:09 +0300 Subject: [PATCH 1/3] fix(codex): honor effective CODEX_HOME during launch --- src/hooks/codex.rs | 153 +++++++++++++++++++++++-------- src/launcher.rs | 57 ++++++++---- src/tools/codex_preprocessing.rs | 53 ++++++++++- 3 files changed, 205 insertions(+), 58 deletions(-) diff --git a/src/hooks/codex.rs b/src/hooks/codex.rs index b02341dd..87f1ea56 100644 --- a/src/hooks/codex.rs +++ b/src/hooks/codex.rs @@ -595,17 +595,29 @@ fn codex_config_dir() -> PathBuf { /// Get path to Codex config.toml. pub fn get_codex_config_path() -> PathBuf { - codex_config_dir().join("config.toml") + codex_config_path_at(&codex_config_dir()) } /// Get path to Codex hooks.json. pub fn get_codex_hooks_path() -> PathBuf { - codex_config_dir().join("hooks.json") + codex_hooks_path_at(&codex_config_dir()) } /// Get path to Codex execpolicy rules directory. pub fn get_codex_rules_path() -> PathBuf { - codex_config_dir().join("rules") + codex_rules_path_at(&codex_config_dir()) +} + +fn codex_config_path_at(codex_home: &Path) -> PathBuf { + codex_home.join("config.toml") +} + +fn codex_hooks_path_at(codex_home: &Path) -> PathBuf { + codex_home.join("hooks.json") +} + +fn codex_rules_path_at(codex_home: &Path) -> PathBuf { + codex_home.join("rules") } /// Strip a Windows verbatim prefix and collapse `.`/`..` components. @@ -1209,7 +1221,7 @@ fn test_hook_list_from_hooks_json(hooks_path: &Path) -> Result Result, String> { +fn fetch_codex_hook_list(cwd: &Path, codex_home: &Path) -> Result, String> { #[cfg(test)] { let _ = cwd; @@ -1220,13 +1232,14 @@ fn fetch_codex_hook_list(cwd: &Path) -> Result, String> let json: Value = serde_json::from_str(&value).map_err(|e| e.to_string())?; return parse_codex_hook_list_entries(&json); } - test_hook_list_from_hooks_json(&get_codex_hooks_path()) + test_hook_list_from_hooks_json(&codex_hooks_path_at(codex_home)) } #[cfg(not(test))] { let mut child = crate::terminal::executable_command("codex") .args(["app-server", "--listen", "stdio://"]) + .env("CODEX_HOME", codex_home) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -1293,9 +1306,12 @@ fn fetch_codex_hook_list(cwd: &Path) -> Result, String> } } -fn fetch_codex_hcom_hook_entries(cwd: &Path) -> Result, String> { - let entries = fetch_codex_hook_list(cwd)?; - hcom_trust_entries_from_hook_list(&entries, &get_codex_hooks_path()) +fn fetch_codex_hcom_hook_entries( + cwd: &Path, + codex_home: &Path, +) -> Result, String> { + let entries = fetch_codex_hook_list(cwd, codex_home)?; + hcom_trust_entries_from_hook_list(&entries, &codex_hooks_path_at(codex_home)) } #[cfg(not(test))] @@ -1561,13 +1577,14 @@ fn write_hcom_hook_trust_state( fn write_hcom_trust_state_from_hook_list( hook_list: &[CodexHookListEntry], codex_cli_version: &str, + codex_home: &Path, ) -> Result<(), String> { - let hooks_path = get_codex_hooks_path(); + let hooks_path = codex_hooks_path_at(codex_home); let entries = hcom_trust_entries_from_hook_list(hook_list, &hooks_path)?; let definition_hashes = hcom_hook_definition_hashes_from_hooks_path(&hooks_path).map_err(|e| e.to_string())?; write_hcom_hook_trust_state( - &get_codex_config_path(), + &codex_config_path_at(codex_home), &hooks_path, &entries, &HashSet::new(), @@ -1582,7 +1599,10 @@ fn write_hcom_trust_state_from_hook_list( /// Exact trust state is always preferred; the invocation-wide bypass flag is a /// last resort and is only permitted when hcom can show that nothing but its own /// hooks would be unlocked by it. -pub(crate) fn resolve_codex_hook_trust_state(launch_dir: &Path) -> CodexHookTrustState { +pub(crate) fn resolve_codex_hook_trust_state_at( + launch_dir: &Path, + codex_home: &Path, +) -> CodexHookTrustState { let codex_cli_version = match codex_hook_trust_version() { // Codex predates the trust gate — nothing is holding hcom's hooks back. Ok(None) => return CodexHookTrustState::Trusted, @@ -1603,10 +1623,19 @@ pub(crate) fn resolve_codex_hook_trust_state(launch_dir: &Path) -> CodexHookTrus // This is the launch-time guardrail. Cheap status/verify paths only // inspect local metadata, but before opening Codex we ask Codex for // authoritative currentHash values and rewrite hcom's trust entries. - match fetch_codex_hook_list(launch_dir) { + match fetch_codex_hook_list(launch_dir, codex_home) { Ok(hook_list) => { - match write_hcom_trust_state_from_hook_list(&hook_list, &codex_cli_version) { - Ok(()) if codex_hcom_hooks_trusted_locally_for_version(&codex_cli_version) => { + match write_hcom_trust_state_from_hook_list( + &hook_list, + &codex_cli_version, + codex_home, + ) { + Ok(()) + if codex_hcom_hooks_trusted_locally_for_version( + &codex_cli_version, + codex_home, + ) => + { return CodexHookTrustState::Trusted; } Ok(()) => log::log_warn( @@ -1624,7 +1653,8 @@ pub(crate) fn resolve_codex_hook_trust_state(launch_dir: &Path) -> CodexHookTrus // Self-heal did not land, but Codex just reported every hook it // can see along with its trust status, so the bypass can be // judged precisely instead of guessed at. - let foreign = foreign_hooks_unlocked_by_bypass(&hook_list, &get_codex_hooks_path()); + let foreign = + foreign_hooks_unlocked_by_bypass(&hook_list, &codex_hooks_path_at(codex_home)); return if foreign.is_empty() { CodexHookTrustState::BypassSafeFromHooksList } else { @@ -1645,7 +1675,7 @@ pub(crate) fn resolve_codex_hook_trust_state(launch_dir: &Path) -> CodexHookTrus // Worth its own step because a flaky or slow app-server is the // ordinary failure here, and it must not turn every launch into // a false alarm. - if codex_hcom_hooks_trusted_locally_for_version(&codex_cli_version) { + if codex_hcom_hooks_trusted_locally_for_version(&codex_cli_version, codex_home) { log::log_warn( "codex", "codex.hook_list_unavailable_state_exact", @@ -1668,7 +1698,7 @@ pub(crate) fn resolve_codex_hook_trust_state(launch_dir: &Path) -> CodexHookTrus // Blind mode: no authoritative inventory. Only bypass when a purely local // scan proves that nothing but hcom's own hooks could be in scope. - match scan_local_codex_hook_definitions(launch_dir) { + match scan_local_codex_hook_definitions(launch_dir, codex_home) { Ok(foreign) if foreign.is_empty() => CodexHookTrustState::BypassSafeFromLocalScan, Ok(foreign) => CodexHookTrustState::BypassUnsafe { reason: format!( @@ -1697,9 +1727,11 @@ pub(crate) fn resolve_codex_hook_trust_state(launch_dir: &Path) -> CodexHookTrus /// hcom writes exactly one hooks file, so only handlers in that file with a /// command hcom installs are hcom's; everything found anywhere else is foreign. /// `Err` means the scan could not be completed and the caller must fail closed. -fn scan_local_codex_hook_definitions(launch_dir: &Path) -> Result, String> { - let codex_home = codex_config_dir(); - let hcom_hooks_path = get_codex_hooks_path(); +fn scan_local_codex_hook_definitions( + launch_dir: &Path, + codex_home: &Path, +) -> Result, String> { + let hcom_hooks_path = codex_hooks_path_at(codex_home); let expected = expected_hcom_hook_commands(); let mut foreign = Vec::new(); @@ -1710,14 +1742,14 @@ fn scan_local_codex_hook_definitions(launch_dir: &Path) -> Result, S collect_foreign_hooks_from_config_toml(&user_config_path, config, &expected, &mut foreign)?; note_declared_plugins(&user_config_path, config, &mut foreign); } - note_possible_plugin_hooks(&codex_home, &mut foreign); + note_possible_plugin_hooks(codex_home, &mut foreign); let markers = codex_project_root_markers(user_config.as_ref())?; for dir in codex_project_layer_dirs(launch_dir, &markers)? { let dot_codex = dir.join(".codex"); // Codex skips a project `.codex` that resolves to CODEX_HOME itself // (codex-rs/config/src/loader/mod.rs:1256-1259). - if paths_equivalent(&dot_codex, &codex_home) || !dot_codex.is_dir() { + if paths_equivalent(&dot_codex, codex_home) || !dot_codex.is_dir() { continue; } collect_foreign_hooks_from_hooks_json( @@ -1916,8 +1948,11 @@ fn note_possible_plugin_hooks(codex_home: &Path, out: &mut Vec) { } } -fn codex_hcom_hooks_trusted_locally_for_version(codex_cli_version: &str) -> bool { - let hooks_path = get_codex_hooks_path(); +fn codex_hcom_hooks_trusted_locally_for_version( + codex_cli_version: &str, + codex_home: &Path, +) -> bool { + let hooks_path = codex_hooks_path_at(codex_home); let hooks_content = match std::fs::read_to_string(&hooks_path) { Ok(content) => content, Err(_) => return false, @@ -1940,7 +1975,7 @@ fn codex_hcom_hooks_trusted_locally_for_version(codex_cli_version: &str) -> bool let keys: HashSet = entries.into_iter().map(|entry| entry.key).collect(); codex_hcom_hook_keys_trusted_for_version( - &get_codex_config_path(), + &codex_config_path_at(codex_home), &keys, codex_cli_version, &definition_hashes, @@ -2212,7 +2247,11 @@ fn codex_feature_enabled(config_path: &Path, feature_key: CodexHooksFeatureKey) /// Codex warns if the deprecated key is present at all, even when `hooks` is /// also enabled, so treat that mixed state as not current. pub(crate) fn codex_current_feature_enabled() -> bool { - let config_path = get_codex_config_path(); + codex_current_feature_enabled_at(&codex_config_dir()) +} + +pub(crate) fn codex_current_feature_enabled_at(codex_home: &Path) -> bool { + let config_path = codex_config_path_at(codex_home); let feature_key = detect_codex_hooks_feature_key(); codex_selected_feature_enabled(&config_path, feature_key) && !codex_deprecated_feature_present(&config_path, feature_key) @@ -2366,7 +2405,11 @@ fn build_codex_rules() -> String { /// Set up Codex execpolicy rules for auto-approval. pub fn setup_codex_execpolicy() -> bool { - let rules_dir = get_codex_rules_path(); + setup_codex_execpolicy_at(&codex_config_dir()) +} + +fn setup_codex_execpolicy_at(codex_home: &Path) -> bool { + let rules_dir = codex_rules_path_at(codex_home); let rules_file = rules_dir.join("hcom.rules"); let rule_content = build_codex_rules(); @@ -2382,7 +2425,11 @@ pub fn setup_codex_execpolicy() -> bool { /// Remove hcom execpolicy rule. pub fn remove_codex_execpolicy() -> bool { - let rules_file = get_codex_rules_path().join("hcom.rules"); + remove_codex_execpolicy_at(&codex_config_dir()) +} + +fn remove_codex_execpolicy_at(codex_home: &Path) -> bool { + let rules_file = codex_rules_path_at(codex_home).join("hcom.rules"); if rules_file.exists() { std::fs::remove_file(&rules_file).is_ok() } else { @@ -2470,8 +2517,15 @@ pub enum SetupError { } pub fn try_setup_codex_hooks(include_permissions: bool) -> Result<(), SetupError> { - let config_path = get_codex_config_path(); - let hooks_path = get_codex_hooks_path(); + try_setup_codex_hooks_at(include_permissions, &codex_config_dir()) +} + +pub(crate) fn try_setup_codex_hooks_at( + include_permissions: bool, + codex_home: &Path, +) -> Result<(), SetupError> { + let config_path = codex_config_path_at(codex_home); + let hooks_path = codex_hooks_path_at(codex_home); let feature_key = detect_codex_hooks_feature_key(); ensure_codex_feature_enabled(&config_path, feature_key).map_err(|e| { @@ -2525,7 +2579,7 @@ pub fn try_setup_codex_hooks(include_permissions: bool) -> Result<(), SetupError let definition_hashes = hcom_hook_definition_hashes_from_hooks_json(&hooks_json, &hooks_path); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); - match fetch_codex_hcom_hook_entries(&cwd).and_then(|entries| { + match fetch_codex_hcom_hook_entries(&cwd, codex_home).and_then(|entries| { let current_keys: HashSet = entries.iter().map(|entry| entry.key.clone()).collect(); let stale_keys: HashSet = old_hcom_hook_keys @@ -2556,9 +2610,9 @@ pub fn try_setup_codex_hooks(include_permissions: bool) -> Result<(), SetupError } let ep_ok = if include_permissions { - setup_codex_execpolicy() + setup_codex_execpolicy_at(codex_home) } else { - remove_codex_execpolicy() + remove_codex_execpolicy_at(codex_home) }; if !ep_ok { log::log_warn( @@ -2575,12 +2629,19 @@ pub fn setup_codex_hooks(include_permissions: bool) -> bool { } pub fn verify_codex_hooks_installed(check_permissions: bool) -> bool { - verify_codex_hooks_inner(check_permissions).is_ok() + verify_codex_hooks_installed_at(check_permissions, &codex_config_dir()) } -pub(crate) fn verify_codex_hooks_inner(check_permissions: bool) -> Result<(), VerifyFailReason> { - let config_path = get_codex_config_path(); - let hooks_path = get_codex_hooks_path(); +pub(crate) fn verify_codex_hooks_installed_at(check_permissions: bool, codex_home: &Path) -> bool { + verify_codex_hooks_inner_at(check_permissions, codex_home).is_ok() +} + +fn verify_codex_hooks_inner_at( + check_permissions: bool, + codex_home: &Path, +) -> Result<(), VerifyFailReason> { + let config_path = codex_config_path_at(codex_home); + let hooks_path = codex_hooks_path_at(codex_home); if !config_path.exists() { return Err(VerifyFailReason::ConfigPathMissing(config_path)); @@ -2594,7 +2655,7 @@ pub(crate) fn verify_codex_hooks_inner(check_permissions: bool) -> Result<(), Ve verify_hooks_json_at(&hooks_path)?; verify_hcom_hook_trust_state(&config_path, &hooks_path)?; if check_permissions { - let rules_file = get_codex_rules_path().join("hcom.rules"); + let rules_file = codex_rules_path_at(codex_home).join("hcom.rules"); if !rules_file.exists() { return Err(VerifyFailReason::PermissionsRulesMissing(rules_file)); } @@ -2769,6 +2830,22 @@ mod tests { assert!(!verify_codex_hooks_installed(false)); } + #[test] + #[serial] + fn test_setup_codex_hooks_targets_effective_child_home() { + let (tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + unsafe { std::env::set_var("HCOM_TEST_CODEX_CLI_VERSION", "codex-cli 0.130.0") }; + let ambient_config = get_codex_config_path(); + let child_home = tmp.path().join("child-codex-home"); + + try_setup_codex_hooks_at(false, &child_home).unwrap(); + + assert!(child_home.join("config.toml").exists()); + assert!(child_home.join("hooks.json").exists()); + assert!(verify_codex_hooks_installed_at(false, &child_home)); + assert!(!ambient_config.exists()); + } + #[test] #[serial] fn test_setup_codex_hooks_trusts_hcom_hooks_for_modern_codex() { diff --git a/src/launcher.rs b/src/launcher.rs index 47b0c383..06eb5911 100644 --- a/src/launcher.rs +++ b/src/launcher.rs @@ -550,7 +550,11 @@ fn format_plugin_install_error( /// /// Uses verify-first pattern: read-only check first, only write if needed. /// Strict gate: refuses to launch if hooks can't be installed. -fn ensure_hooks_installed(tool: &LaunchTool, include_permissions: bool) -> Result<()> { +fn ensure_hooks_installed( + tool: &LaunchTool, + include_permissions: bool, + codex_home: Option<&std::path::Path>, +) -> Result<()> { match tool { LaunchTool::Claude | LaunchTool::ClaudePty => { if crate::hooks::claude::verify_claude_hooks_installed(None, include_permissions) { @@ -605,12 +609,15 @@ fn ensure_hooks_installed(tool: &LaunchTool, include_permissions: bool) -> Resul Ok(()) } LaunchTool::Codex => { - if crate::hooks::codex::verify_codex_hooks_installed(include_permissions) - && crate::hooks::codex::codex_current_feature_enabled() + let codex_home = codex_home.expect("Codex launch must resolve CODEX_HOME"); + if crate::hooks::codex::verify_codex_hooks_installed_at(include_permissions, codex_home) + && crate::hooks::codex::codex_current_feature_enabled_at(codex_home) { return Ok(()); } - if let Err(e) = crate::hooks::codex::try_setup_codex_hooks(include_permissions) { + if let Err(e) = + crate::hooks::codex::try_setup_codex_hooks_at(include_permissions, codex_home) + { if matches!(e, crate::hooks::codex::SetupError::HookTrustFailed { .. }) { crate::log::log_warn( "codex", @@ -623,8 +630,8 @@ fn ensure_hooks_installed(tool: &LaunchTool, include_permissions: bool) -> Resul let diag = install_diag_context( tool, &[ - ("config_path", crate::hooks::codex::get_codex_config_path()), - ("hooks_path", crate::hooks::codex::get_codex_hooks_path()), + ("config_path", codex_home.join("config.toml")), + ("hooks_path", codex_home.join("hooks.json")), ], ); bail!( @@ -1700,14 +1707,6 @@ pub fn launch(db: &HcomDb, mut params: LaunchParams) -> Result { c }); - // For Codex: probe CODEX_HOME writability synchronously. Sandboxed parent - // codex would otherwise spawn a child that hangs on the readonly-state-DB - // repair prompt. Failing here lets the parent's sandbox-escalation flow - // surface the denial to the user. - if matches!(normalized, LaunchTool::Codex) { - crate::tools::codex_preprocessing::ensure_codex_home_writable()?; - } - let inside_ai_tool = crate::shared::context::HcomContext::from_os().is_inside_ai_tool(); let terminal_mode = params .terminal @@ -1721,9 +1720,6 @@ pub fn launch(db: &HcomDb, mut params: LaunchParams) -> Result { inside_ai_tool, ); - // Ensure hooks are installed (strict: refuse to launch without hooks) - ensure_hooks_installed(&normalized, hcom_config.auto_approve)?; - // Build base environment for the current launch regime, then overlay // config.toml + ~/.hcom/env which win. let mut base_env = build_launch_env( @@ -1748,6 +1744,24 @@ pub fn launch(db: &HcomDb, mut params: LaunchParams) -> Result { ); } + // Codex preflight and hook setup must use the same effective CODEX_HOME as + // the child, including overrides from ~/.hcom/env and caller-provided env. + let codex_home = if matches!(normalized, LaunchTool::Codex) { + crate::tools::codex_preprocessing::resolve_codex_home_from_env(&base_env) + } else { + None + }; + if let Some((ref path, explicit_env)) = codex_home { + crate::tools::codex_preprocessing::ensure_codex_home_writable_at(path, explicit_env)?; + } + + // Ensure hooks are installed (strict: refuse to launch without hooks) + ensure_hooks_installed( + &normalized, + hcom_config.auto_approve, + codex_home.as_ref().map(|(path, _)| path.as_path()), + )?; + // Tag resolution let effective_tag = if let Some(ref tag) = params.tag { base_env.insert("HCOM_TAG".to_string(), tag.clone()); @@ -1832,7 +1846,14 @@ pub fn launch(db: &HcomDb, mut params: LaunchParams) -> Result { // paired with a project layer that hcom itself just marked trusted — that // layer could contribute a hook source the scan never saw. let codex_hook_trust = if matches!(normalized, LaunchTool::Codex) { - codex_preprocessing::resolve_codex_hook_trust(¶ms.args, &canonical_dir) + codex_preprocessing::resolve_codex_hook_trust_at( + ¶ms.args, + &canonical_dir, + codex_home + .as_ref() + .map(|(path, _)| path.as_path()) + .expect("Codex launch must resolve CODEX_HOME"), + ) } else { codex_preprocessing::CodexHookTrustOutcome::NoActionNeeded }; diff --git a/src/tools/codex_preprocessing.rs b/src/tools/codex_preprocessing.rs index 0406a5e9..0d61808f 100644 --- a/src/tools/codex_preprocessing.rs +++ b/src/tools/codex_preprocessing.rs @@ -1,5 +1,6 @@ //! Codex launch preprocessing — sandbox flags, DB access, bootstrap injection. +use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::OnceLock; @@ -195,6 +196,23 @@ fn resolve_codex_home() -> Option<(PathBuf, bool)> { dirs::home_dir().map(|h| (h.join(".codex"), false)) } +/// Resolve the Codex state directory from the effective child launch +/// environment, including values supplied through `~/.hcom/env` or `--env`. +pub(crate) fn resolve_codex_home_from_env( + env: &HashMap, +) -> Option<(PathBuf, bool)> { + if let Some(val) = env.get("CODEX_HOME").filter(|val| !val.is_empty()) { + return Some((PathBuf::from(val), true)); + } + env.get("HOME") + .or_else(|| env.get("USERPROFILE")) + .filter(|val| !val.is_empty()) + .map(PathBuf::from) + .or_else(dirs::home_dir) + .or_else(|| Some(crate::runtime_env::tool_config_root())) + .map(|home| (home.join(".codex"), false)) +} + /// Probe whether `CODEX_HOME` is writable before launching codex. /// /// When hcom is invoked from inside a sandboxed parent codex (e.g. @@ -211,8 +229,12 @@ pub fn ensure_codex_home_writable() -> Result<()> { let Some((codex_home, explicit_env)) = resolve_codex_home() else { return Ok(()); }; + ensure_codex_home_writable_at(&codex_home, explicit_env) +} + +pub(crate) fn ensure_codex_home_writable_at(codex_home: &Path, explicit_env: bool) -> Result<()> { let probe_dir = if codex_home.exists() { - codex_home.as_path() + codex_home } else if explicit_env { return Ok(()); } else { @@ -301,6 +323,17 @@ impl CodexHookTrustOutcome { /// Split from `preprocess_codex_args` because the outcome also governs /// workspace-trust injection, which happens earlier in the launch sequence. pub fn resolve_codex_hook_trust(codex_args: &[String], launch_dir: &Path) -> CodexHookTrustOutcome { + let Some((codex_home, _)) = resolve_codex_home() else { + return CodexHookTrustOutcome::NoActionNeeded; + }; + resolve_codex_hook_trust_at(codex_args, launch_dir, &codex_home) +} + +pub(crate) fn resolve_codex_hook_trust_at( + codex_args: &[String], + launch_dir: &Path, + codex_home: &Path, +) -> CodexHookTrustOutcome { if !codex_supports_bypass_hook_trust() { return CodexHookTrustOutcome::NoActionNeeded; } @@ -310,7 +343,7 @@ pub fn resolve_codex_hook_trust(codex_args: &[String], launch_dir: &Path) -> Cod return CodexHookTrustOutcome::NoActionNeeded; } - match crate::hooks::codex::resolve_codex_hook_trust_state(launch_dir) { + match crate::hooks::codex::resolve_codex_hook_trust_state_at(launch_dir, codex_home) { crate::hooks::codex::CodexHookTrustState::Trusted => CodexHookTrustOutcome::NoActionNeeded, crate::hooks::codex::CodexHookTrustState::BypassSafeFromHooksList => { warn_bypass_granted("Codex's own hook list"); @@ -818,6 +851,22 @@ mod tests { assert!(!home.join(".hcom_writable_probe").exists()); } + #[test] + fn test_resolve_codex_home_uses_effective_child_env_override() { + let env = HashMap::from([ + ("HOME".to_string(), "/readonly-parent-home".to_string()), + ( + "CODEX_HOME".to_string(), + "/writable-child-codex-home".to_string(), + ), + ]); + + let resolved = resolve_codex_home_from_env(&env).unwrap(); + + assert_eq!(resolved.0, PathBuf::from("/writable-child-codex-home")); + assert!(resolved.1); + } + /// Resolve the hook-trust decision and apply it, the way the launcher does /// across its two call sites. fn bypass_args(args: &[String], launch_dir: &std::path::Path) -> Vec { From 40e6ad99bad379c286ed5c32546295e322bdbf49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yasin=20O=CC=88zmen?= Date: Wed, 26 Aug 2026 08:15:56 +0300 Subject: [PATCH 2/3] fix(launcher): preserve ambient tool config overrides --- src/launcher.rs | 54 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/src/launcher.rs b/src/launcher.rs index 06eb5911..5c39ec05 100644 --- a/src/launcher.rs +++ b/src/launcher.rs @@ -453,6 +453,31 @@ fn isolated_tool_config_dir(tool: &LaunchTool) -> Option { Some(root.join(dirname)) } +/// Make the tool config directory explicit in the child environment. +/// +/// Some launch backends clear the inherited environment while others do not. +/// Copying an ambient override into the effective launch map keeps preflight, +/// hook setup, and the child process on the same directory in both cases. +fn ensure_tool_config_env(tool: &LaunchTool, env: &mut HashMap) { + let Some(env_var) = tool.spec().launch.config_dir_env else { + return; + }; + if env.contains_key(env_var) { + return; + } + if let Some(value) = std::env::var(env_var) + .ok() + .filter(|value| !value.is_empty()) + { + env.insert(env_var.to_string(), value); + } else if let Some(config_dir) = isolated_tool_config_dir(tool) { + env.insert( + env_var.to_string(), + config_dir.to_string_lossy().to_string(), + ); + } +} + /// Get system prompt file path for Gemini/Codex. fn get_system_prompt_path(tool: &str) -> std::path::PathBuf { let prompts_dir = paths::hcom_path(&["system-prompts"]); @@ -1730,19 +1755,7 @@ pub fn launch(db: &HcomDb, mut params: LaunchParams) -> Result { base_env.extend(caller_env.clone()); } base_env.remove("HCOM_TERMINAL"); - if let Some(env_var) = normalized.spec().launch.config_dir_env - && !base_env.contains_key(env_var) - && std::env::var(env_var) - .ok() - .filter(|v| !v.is_empty()) - .is_none() - && let Some(config_dir) = isolated_tool_config_dir(&normalized) - { - base_env.insert( - env_var.to_string(), - config_dir.to_string_lossy().to_string(), - ); - } + ensure_tool_config_env(&normalized, &mut base_env); // Codex preflight and hook setup must use the same effective CODEX_HOME as // the child, including overrides from ~/.hcom/env and caller-provided env. @@ -3070,6 +3083,21 @@ mod tests { ); } + #[test] + #[serial] + fn test_tool_config_env_copies_ambient_override_into_clean_child_env() { + let _guard = EnvVarGuard::remove(vec!["CODEX_HOME".to_string()]); + unsafe { std::env::set_var("CODEX_HOME", "/isolated/codex-home") }; + let mut env = HashMap::from([("HOME".to_string(), "/clean-shell-home".to_string())]); + + ensure_tool_config_env(&LaunchTool::Codex, &mut env); + + assert_eq!( + env.get("CODEX_HOME").map(String::as_str), + Some("/isolated/codex-home") + ); + } + #[test] #[serial] fn test_build_launch_env_strips_closed_categories() { From 02fa3cc1860b78b68de94afb8811bdb6504fd227 Mon Sep 17 00:00:00 2001 From: aannoo Date: Sun, 13 Sep 2026 17:08:11 +0100 Subject: [PATCH 3/3] fix(codex): unify effective home resolution --- src/launcher.rs | 84 +++++++++++++++++++++++++---- src/tools/codex_preprocessing.rs | 93 ++++++++++++++++++++++++++++---- 2 files changed, 157 insertions(+), 20 deletions(-) diff --git a/src/launcher.rs b/src/launcher.rs index 5c39ec05..eb64b29e 100644 --- a/src/launcher.rs +++ b/src/launcher.rs @@ -383,7 +383,7 @@ where // HCOM_* settings from config.toml for (key, value) in hcom_config.to_env_dict() { if !value.is_empty() { - env.insert(key, value); + insert_effective_env(&mut env, key, value, cfg!(windows)); } } @@ -391,7 +391,7 @@ where let env_path = paths::hcom_path(&["env"]); for (key, value) in config::load_env_extras(&env_path) { if !value.is_empty() { - env.insert(key, value); + insert_effective_env(&mut env, key, value, cfg!(windows)); } } @@ -453,6 +453,36 @@ fn isolated_tool_config_dir(tool: &LaunchTool) -> Option { Some(root.join(dirname)) } +/// Insert an environment override using the target platform's key semantics. +/// Windows environment names are case-insensitive, while `HashMap` keys are +/// not; remove an earlier spelling so the child receives one authoritative +/// value instead of an order-dependent pair. +fn insert_effective_env( + env: &mut HashMap, + key: String, + value: String, + case_insensitive: bool, +) { + if case_insensitive { + env.retain(|existing, _| !existing.eq_ignore_ascii_case(&key)); + } + env.insert(key, value); +} + +fn effective_env_value<'a>( + env: &'a HashMap, + key: &str, + case_insensitive: bool, +) -> Option<&'a str> { + if case_insensitive { + env.iter() + .find(|(existing, _)| existing.eq_ignore_ascii_case(key)) + .map(|(_, value)| value.as_str()) + } else { + env.get(key).map(String::as_str) + } +} + /// Make the tool config directory explicit in the child environment. /// /// Some launch backends clear the inherited environment while others do not. @@ -462,18 +492,22 @@ fn ensure_tool_config_env(tool: &LaunchTool, env: &mut HashMap) let Some(env_var) = tool.spec().launch.config_dir_env else { return; }; - if env.contains_key(env_var) { + let case_insensitive = cfg!(windows); + if let Some(value) = effective_env_value(env, env_var, case_insensitive).map(str::to_owned) { + insert_effective_env(env, env_var.to_string(), value, case_insensitive); return; } if let Some(value) = std::env::var(env_var) .ok() .filter(|value| !value.is_empty()) { - env.insert(env_var.to_string(), value); + insert_effective_env(env, env_var.to_string(), value, case_insensitive); } else if let Some(config_dir) = isolated_tool_config_dir(tool) { - env.insert( + insert_effective_env( + env, env_var.to_string(), config_dir.to_string_lossy().to_string(), + case_insensitive, ); } } @@ -1752,19 +1786,31 @@ pub fn launch(db: &HcomDb, mut params: LaunchParams) -> Result { launch_env_regime(base_env_run_here, inside_ai_tool), ); if let Some(ref caller_env) = params.env { - base_env.extend(caller_env.clone()); + for (key, value) in caller_env { + insert_effective_env(&mut base_env, key.clone(), value.clone(), cfg!(windows)); + } } base_env.remove("HCOM_TERMINAL"); ensure_tool_config_env(&normalized, &mut base_env); + let working_dir = params.cwd.as_deref().unwrap_or("."); + let canonical_dir = std::fs::canonicalize(working_dir) + .unwrap_or_else(|_| std::path::PathBuf::from(working_dir)); + // Codex preflight and hook setup must use the same effective CODEX_HOME as // the child, including overrides from ~/.hcom/env and caller-provided env. let codex_home = if matches!(normalized, LaunchTool::Codex) { - crate::tools::codex_preprocessing::resolve_codex_home_from_env(&base_env) + crate::tools::codex_preprocessing::resolve_codex_home_from_env(&base_env, &canonical_dir) } else { None }; if let Some((ref path, explicit_env)) = codex_home { + insert_effective_env( + &mut base_env, + "CODEX_HOME".to_string(), + path.to_string_lossy().into_owned(), + cfg!(windows), + ); crate::tools::codex_preprocessing::ensure_codex_home_writable_at(path, explicit_env)?; } @@ -1808,9 +1854,6 @@ pub fn launch(db: &HcomDb, mut params: LaunchParams) -> Result { base_env.insert("GEMINI_SYSTEM_MD".to_string(), path); } - let working_dir = params.cwd.as_deref().unwrap_or("."); - let canonical_dir = std::fs::canonicalize(working_dir) - .unwrap_or_else(|_| std::path::PathBuf::from(working_dir)); // Folder trust: on first run each tool shows a "do you trust this folder?" // prompt — the user accepts to continue or declines and it exits. When an // agent launches another agent via hcom, auto-approve the prompt for the @@ -3098,6 +3141,27 @@ mod tests { ); } + #[test] + fn test_windows_env_override_replaces_different_key_casing() { + let mut env = HashMap::from([( + "CODEX_HOME".to_string(), + r"C:\ambient-codex-home".to_string(), + )]); + + insert_effective_env( + &mut env, + "Codex_Home".to_string(), + r"C:\caller-codex-home".to_string(), + true, + ); + + assert_eq!(env.len(), 1); + assert_eq!( + effective_env_value(&env, "CODEX_HOME", true), + Some(r"C:\caller-codex-home") + ); + } + #[test] #[serial] fn test_build_launch_env_strips_closed_categories() { diff --git a/src/tools/codex_preprocessing.rs b/src/tools/codex_preprocessing.rs index 0d61808f..a8da6360 100644 --- a/src/tools/codex_preprocessing.rs +++ b/src/tools/codex_preprocessing.rs @@ -200,17 +200,51 @@ fn resolve_codex_home() -> Option<(PathBuf, bool)> { /// environment, including values supplied through `~/.hcom/env` or `--env`. pub(crate) fn resolve_codex_home_from_env( env: &HashMap, + launch_dir: &Path, ) -> Option<(PathBuf, bool)> { - if let Some(val) = env.get("CODEX_HOME").filter(|val| !val.is_empty()) { - return Some((PathBuf::from(val), true)); - } - env.get("HOME") - .or_else(|| env.get("USERPROFILE")) - .filter(|val| !val.is_empty()) + // `dirs::home_dir()` reads HOME on Unix but uses the platform profile API + // on Windows. Reproduce that distinction from the child's effective env. + #[cfg(windows)] + let default_home = dirs::home_dir(); + #[cfg(not(windows))] + let default_home = env + .get("HOME") + .filter(|value| !value.is_empty()) .map(PathBuf::from) - .or_else(dirs::home_dir) - .or_else(|| Some(crate::runtime_env::tool_config_root())) - .map(|home| (home.join(".codex"), false)) + .or_else(dirs::home_dir); + resolve_codex_home_from_env_with( + env, + launch_dir, + default_home.or_else(|| Some(crate::runtime_env::tool_config_root())), + cfg!(windows), + ) +} + +fn resolve_codex_home_from_env_with( + env: &HashMap, + launch_dir: &Path, + default_home: Option, + case_insensitive: bool, +) -> Option<(PathBuf, bool)> { + let configured = if case_insensitive { + env.iter() + .find(|(key, _)| key.eq_ignore_ascii_case("CODEX_HOME")) + .map(|(_, value)| value.as_str()) + } else { + env.get("CODEX_HOME").map(String::as_str) + }; + if let Some(value) = configured.filter(|value| !value.is_empty()) { + let path = PathBuf::from(value); + return Some(( + if path.is_absolute() { + path + } else { + launch_dir.join(path) + }, + true, + )); + } + default_home.map(|home| (home.join(".codex"), false)) } /// Probe whether `CODEX_HOME` is writable before launching codex. @@ -861,12 +895,51 @@ mod tests { ), ]); - let resolved = resolve_codex_home_from_env(&env).unwrap(); + let resolved = resolve_codex_home_from_env(&env, Path::new("/workspace")).unwrap(); assert_eq!(resolved.0, PathBuf::from("/writable-child-codex-home")); assert!(resolved.1); } + #[test] + fn test_resolve_codex_home_uses_platform_home_not_child_home_env() { + let env = HashMap::from([ + ("HOME".to_string(), "/different-child-home".to_string()), + ( + "USERPROFILE".to_string(), + r"C:\different-child-home".to_string(), + ), + ]); + + let resolved = resolve_codex_home_from_env_with( + &env, + Path::new("/workspace"), + Some(PathBuf::from("/platform-home")), + true, + ) + .unwrap(); + + assert_eq!(resolved, (PathBuf::from("/platform-home/.codex"), false)); + } + + #[test] + fn test_resolve_codex_home_handles_windows_key_casing_and_child_cwd() { + let env = HashMap::from([("Codex_Home".to_string(), "relative-home".to_string())]); + + let resolved = resolve_codex_home_from_env_with( + &env, + Path::new("/child-workspace"), + Some(PathBuf::from("/platform-home")), + true, + ) + .unwrap(); + + assert_eq!( + resolved, + (PathBuf::from("/child-workspace/relative-home"), true) + ); + } + /// Resolve the hook-trust decision and apply it, the way the launcher does /// across its two call sites. fn bypass_args(args: &[String], launch_dir: &std::path::Path) -> Vec {