diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 80a1997477..95fbf22b43 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -40,6 +40,7 @@ use serde_json::Value; use crate::context::ContextualUserFragment; use crate::context::HookAdditionalContext; use crate::event_mapping::parse_turn_item; +use crate::exec_env::create_env; use crate::session::TurnInput; use crate::session::session::Session; use crate::session::turn_context::TurnContext; @@ -124,6 +125,7 @@ pub(crate) async fn run_pending_session_start_hooks( source: session_start_source, }, }; + let captures_shell_env = matches!(target, StartHookTarget::SessionStart { .. }); let request = codex_hooks::SessionStartRequest { session_id: sess.session_id().into(), #[allow(deprecated)] @@ -135,15 +137,30 @@ pub(crate) async fn run_pending_session_start_hooks( }; let hooks = sess.hooks(); let preview_runs = hooks.preview_session_start(&request); - if run_context_injecting_hook( - sess, - turn_context, - preview_runs, - hooks.run_session_start(request, Some(turn_context.sub_id.clone())), - ) - .await - .record_additional_contexts(sess, turn_context) - .await + emit_hook_started_events(sess, turn_context, preview_runs).await; + let outcome = hooks + .run_session_start(request, Some(turn_context.sub_id.clone())) + .await; + if captures_shell_env { + let base_env = create_env( + &turn_context.shell_environment_policy, + Some(sess.conversation_id), + ); + let cwd = turn_context + .environments + .primary() + .map(|environment| environment.cwd.as_path()) + .unwrap_or_else(|| turn_context.config.cwd.as_path()); + if let Err(error) = sess.capture_shell_env_exports(cwd, &base_env).await { + tracing::warn!("failed to capture SessionStart shell environment: {error:#}"); + } + } + let outcome: ContextInjectingHookOutcome = outcome.into(); + emit_hook_completed_events(sess, turn_context, outcome.hook_events).await; + if outcome + .outcome + .record_additional_contexts(sess, turn_context) + .await { return true; } diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 22a95c1a93..4bdec101fd 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -3223,16 +3223,27 @@ impl Session { Arc::clone(&self.services.user_shell) } - pub(crate) fn shell_env_file_path(&self) -> Option<&Path> { - self.services - .shell_env_file - .as_ref() - .map(crate::shell_env_file::ShellEnvFile::path) + pub(crate) async fn capture_shell_env_exports( + &self, + cwd: &Path, + base_env: &HashMap, + ) -> anyhow::Result<()> { + if let Some(shell_env_file) = self.services.shell_env_file.as_ref() { + let shell = self.user_shell(); + shell_env_file + .capture_exports(shell.as_ref(), cwd, base_env) + .await?; + } + Ok(()) } - pub(crate) fn insert_shell_env_file(&self, env: &mut HashMap) { + pub(crate) fn apply_shell_env_exports( + &self, + env: &mut HashMap, + explicit_env_overrides: &HashMap, + ) { if let Some(shell_env_file) = self.services.shell_env_file.as_ref() { - shell_env_file.insert_into_env(env); + shell_env_file.apply_exports(env, explicit_env_overrides); } } @@ -3317,7 +3328,7 @@ async fn build_hooks_for_config( let plugin_hook_load_warnings = plugin_outcome.effective_plugin_hook_warnings(); let mut command_env = HashMap::new(); if let Some(shell_env_file) = shell_env_file { - shell_env_file.insert_into_env(&mut command_env); + shell_env_file.insert_path_into_env(&mut command_env); } Hooks::new(HooksConfig { legacy_notify_argv: config.notify.clone(), diff --git a/codex-rs/core/src/shell_env_file.rs b/codex-rs/core/src/shell_env_file.rs index 21b29b5b13..1d16eae7ea 100644 --- a/codex-rs/core/src/shell_env_file.rs +++ b/codex-rs/core/src/shell_env_file.rs @@ -1,19 +1,28 @@ use std::collections::HashMap; use std::path::Path; +use std::sync::Mutex; -#[cfg(not(windows))] use anyhow::Context; use anyhow::Result; +use anyhow::anyhow; +use anyhow::bail; use codex_protocol::ThreadId; use codex_protocol::shell_environment::CODEX_ENV_FILE_ENV_VAR; +use codex_protocol::shell_environment::CODEX_THREAD_ID_ENV_VAR; use tempfile::TempPath; +use tokio::process::Command; + +use crate::shell::Shell; +use crate::shell::ShellType; /// Session-owned script that hooks can populate with exported shell state. /// -/// Local shell tool commands source this file before running so lifecycle hook -/// setup remains scoped to the active session rather than persistent config. +/// Only lifecycle hooks receive the writable file path. After SessionStart +/// hooks finish, Codex captures supported exported variables and passes those +/// values to later commands without exposing the writable path. pub(crate) struct ShellEnvFile { path: TempPath, + exports: Mutex>>, } impl ShellEnvFile { @@ -40,6 +49,7 @@ impl ShellEnvFile { .context("failed to create temporary shell env file")?; Ok(Self { path: file.into_temp_path(), + exports: Mutex::new(HashMap::new()), }) } @@ -47,12 +57,184 @@ impl ShellEnvFile { self.path.as_ref() } - pub(crate) fn insert_into_env(&self, env: &mut HashMap) { + pub(crate) fn insert_path_into_env(&self, env: &mut HashMap) { env.insert( CODEX_ENV_FILE_ENV_VAR.to_string(), self.path().to_string_lossy().to_string(), ); } + + /// Sources the hook-writable env file once and stores the resulting + /// environment diff. + /// + /// The temp file remains an input channel for SessionStart hooks, but later + /// commands receive only captured variable changes. Running both a baseline + /// environment dump and a sourced dump lets shell syntax such as command + /// substitution behave naturally without keeping `CODEX_ENV_FILE` available + /// after hook execution. + pub(crate) async fn capture_exports( + &self, + shell: &Shell, + cwd: &Path, + base_env: &HashMap, + ) -> Result<()> { + if !matches!( + shell.shell_type, + ShellType::Zsh | ShellType::Bash | ShellType::Sh + ) { + return Ok(()); + } + + let mut capture_env = base_env.clone(); + capture_env.remove(CODEX_ENV_FILE_ENV_VAR); + self.insert_path_into_env(&mut capture_env); + + let baseline = capture_env_from_shell(shell, cwd, DUMP_ENV_SCRIPT, &capture_env).await?; + let captured = capture_env_from_shell( + shell, + cwd, + SOURCE_ENV_FILE_AND_DUMP_ENV_SCRIPT, + &capture_env, + ) + .await?; + let exports = diff_env(&baseline, &captured); + *self + .exports + .lock() + .map_err(|_| anyhow!("shell env exports lock poisoned"))? = exports; + Ok(()) + } + + /// Applies captured SessionStart environment changes to a command + /// environment without exposing the writable env-file path. + /// + /// Explicit shell-environment policy values are layered back on top so + /// configured overrides keep precedence, and runtime-owned values such as + /// the Codex thread id are preserved rather than accepting hook-written + /// replacements. + pub(crate) fn apply_exports( + &self, + env: &mut HashMap, + explicit_env_overrides: &HashMap, + ) { + let thread_id = env.get(CODEX_THREAD_ID_ENV_VAR).cloned(); + let exports = self + .exports + .lock() + .map(|exports| exports.clone()) + .unwrap_or_default(); + for (key, value) in exports { + if ignored_capture_key(&key) { + continue; + } + match value { + Some(value) => { + env.insert(key, value); + } + None => { + env.remove(&key); + } + } + } + for (key, value) in explicit_env_overrides { + env.insert(key.clone(), value.clone()); + } + if let Some(thread_id) = thread_id { + env.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id); + } + env.remove(CODEX_ENV_FILE_ENV_VAR); + } +} + +const DUMP_ENV_SCRIPT: &str = r#"if [ -x /usr/bin/env ]; then + /usr/bin/env -0 +else + env -0 +fi"#; + +const SOURCE_ENV_FILE_AND_DUMP_ENV_SCRIPT: &str = r#"if [ -n "${CODEX_ENV_FILE:-}" ] && [ -f "$CODEX_ENV_FILE" ]; then + if . "$CODEX_ENV_FILE" >/dev/null 2>&1; then + : + fi +fi +if [ -x /usr/bin/env ]; then + /usr/bin/env -0 +else + env -0 +fi"#; + +async fn capture_env_from_shell( + shell: &Shell, + cwd: &Path, + script: &str, + env: &HashMap, +) -> Result> { + let output = Command::new(&shell.shell_path) + .current_dir(cwd) + .arg("-c") + .arg(script) + .env_clear() + .envs(env) + .output() + .await + .with_context(|| format!("failed to run {}", shell.shell_path.display()))?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + bail!( + "failed to capture shell environment with {}: {stderr}", + shell.shell_path.display() + ); + } + parse_env_output(&output.stdout) +} + +fn parse_env_output(output: &[u8]) -> Result> { + let mut env = HashMap::new(); + for entry in output.split(|byte| *byte == 0) { + if entry.is_empty() { + continue; + } + let Some(separator) = entry.iter().position(|byte| *byte == b'=') else { + continue; + }; + let key = String::from_utf8(entry[..separator].to_vec()) + .context("captured shell environment key was not UTF-8")?; + let value = String::from_utf8(entry[separator + 1..].to_vec()) + .context("captured shell environment value was not UTF-8")?; + env.insert(key, value); + } + Ok(env) +} + +fn diff_env( + baseline: &HashMap, + captured: &HashMap, +) -> HashMap> { + let mut exports = HashMap::new(); + for (key, value) in captured { + if ignored_capture_key(key) { + continue; + } + if baseline.get(key) != Some(value) { + exports.insert(key.clone(), Some(value.clone())); + } + } + for key in baseline.keys() { + if ignored_capture_key(key) { + continue; + } + if !captured.contains_key(key) { + exports.insert(key.clone(), None); + } + } + exports +} + +fn ignored_capture_key(key: &str) -> bool { + matches!( + key, + CODEX_ENV_FILE_ENV_VAR | CODEX_THREAD_ID_ENV_VAR | "PWD" | "OLDPWD" | "SHLVL" | "_" + ) } #[cfg(all(test, not(windows)))] diff --git a/codex-rs/core/src/shell_env_file_tests.rs b/codex-rs/core/src/shell_env_file_tests.rs index 2fe0f9c026..8d42452d92 100644 --- a/codex-rs/core/src/shell_env_file_tests.rs +++ b/codex-rs/core/src/shell_env_file_tests.rs @@ -1,7 +1,13 @@ use anyhow::Result; use codex_protocol::ThreadId; +use pretty_assertions::assert_eq; +use std::collections::HashMap; +use std::path::PathBuf; use super::*; +use crate::shell::Shell; +use crate::shell::ShellType; +use crate::shell::empty_shell_snapshot_receiver; #[cfg(not(windows))] #[test] @@ -15,3 +21,103 @@ fn shell_env_file_is_removed_when_session_owner_drops() -> Result<()> { Ok(()) } + +#[cfg(not(windows))] +#[tokio::test] +async fn shell_env_file_applies_exports_without_exposing_writable_path() -> Result<()> { + let env_file = ShellEnvFile::new(ThreadId::new())?; + let base_env = HashMap::from([ + ("PATH".to_string(), "/usr/bin".to_string()), + ( + CODEX_THREAD_ID_ENV_VAR.to_string(), + "real-thread".to_string(), + ), + ]); + std::fs::write( + env_file.path(), + "\ +export CODEX_SESSION_START_TEST='from-session-start' +export PATH=\"/plugin/bin:$PATH\" +export CODEX_ENV_FILE='/tmp/poison' +export CODEX_THREAD_ID='poisoned-thread' +export EXPLICIT_OVERRIDE='from-hook' +", + )?; + let cwd = std::env::current_dir()?; + env_file + .capture_exports(&test_shell(), cwd.as_path(), &base_env) + .await?; + + let mut env = base_env; + env.insert( + CODEX_ENV_FILE_ENV_VAR.to_string(), + env_file.path().display().to_string(), + ); + let explicit_env_overrides = + HashMap::from([("EXPLICIT_OVERRIDE".to_string(), "from-policy".to_string())]); + + env_file.apply_exports(&mut env, &explicit_env_overrides); + + assert_eq!( + env, + HashMap::from([ + ("PATH".to_string(), "/plugin/bin:/usr/bin".to_string()), + ( + "CODEX_SESSION_START_TEST".to_string(), + "from-session-start".to_string(), + ), + ( + CODEX_THREAD_ID_ENV_VAR.to_string(), + "real-thread".to_string(), + ), + ("EXPLICIT_OVERRIDE".to_string(), "from-policy".to_string()), + ]) + ); + + Ok(()) +} + +#[cfg(not(windows))] +#[tokio::test] +async fn shell_env_file_sources_shell_code_once() -> Result<()> { + let env_file = ShellEnvFile::new(ThreadId::new())?; + std::fs::write( + env_file.path(), + "\ +echo hidden +export SAFE=value +export COMMAND_SUBSTITUTION=$(printf unsafe) +export FUNCTION_DEF='() { echo unsafe; }' +", + )?; + let cwd = std::env::current_dir()?; + env_file + .capture_exports(&test_shell(), cwd.as_path(), &HashMap::new()) + .await?; + + let mut env = HashMap::new(); + env_file.apply_exports(&mut env, &HashMap::new()); + + assert_eq!( + env, + HashMap::from([ + ("SAFE".to_string(), "value".to_string()), + ("COMMAND_SUBSTITUTION".to_string(), "unsafe".to_string()), + ( + "FUNCTION_DEF".to_string(), + "() { echo unsafe; }".to_string(), + ), + ]) + ); + + Ok(()) +} + +#[cfg(not(windows))] +fn test_shell() -> Shell { + Shell { + shell_type: ShellType::Sh, + shell_path: PathBuf::from("/bin/sh"), + shell_snapshot: empty_shell_snapshot_receiver(), + } +} diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 6093dbbc64..80895a9fb1 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -130,7 +130,10 @@ pub(crate) async fn execute_user_shell_command( &turn_context.shell_environment_policy, Some(session.conversation_id), ); - session.insert_shell_env_file(&mut exec_env_map); + session.apply_shell_env_exports( + &mut exec_env_map, + &turn_context.shell_environment_policy.r#set, + ); if exec_env_map.contains_key(PROXY_ACTIVE_ENV_KEY) { for key in PROXY_ENV_KEYS { exec_env_map.remove(*key); @@ -150,7 +153,6 @@ pub(crate) async fn execute_user_shell_command( session_shell.as_ref(), #[allow(deprecated)] &turn_context.cwd, - session.shell_env_file_path(), &turn_context.shell_environment_policy.r#set, &exec_env_map, ); diff --git a/codex-rs/core/src/tools/handlers/shell/shell_command.rs b/codex-rs/core/src/tools/handlers/shell/shell_command.rs index 553735fb78..f06e4a410b 100644 --- a/codex-rs/core/src/tools/handlers/shell/shell_command.rs +++ b/codex-rs/core/src/tools/handlers/shell/shell_command.rs @@ -96,7 +96,7 @@ impl ShellCommandHandler { let cwd = turn_context.resolve_path(params.workdir.clone()); let mut env = create_env(&turn_context.shell_environment_policy, Some(thread_id)); - session.insert_shell_env_file(&mut env); + session.apply_shell_env_exports(&mut env, &turn_context.shell_environment_policy.r#set); Ok(ExecParams { command, diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index 9e8a68acf0..c6007043e9 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -18,10 +18,6 @@ use codex_network_proxy::PROXY_ENV_KEYS; use codex_network_proxy::PROXY_GIT_SSH_COMMAND_ENV_KEY; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::AdditionalPermissionProfile; -use codex_protocol::permissions::FileSystemAccessMode; -use codex_protocol::permissions::FileSystemPath; -use codex_protocol::permissions::FileSystemSandboxEntry; -use codex_protocol::shell_environment::CODEX_ENV_FILE_ENV_VAR; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxType; use codex_utils_absolute_path::AbsolutePathBuf; @@ -75,29 +71,6 @@ pub(crate) fn exec_env_for_sandbox_permissions( env } -/// Adds the read required by a shell wrapper that sources `CODEX_ENV_FILE`. -/// -/// Callers apply this only while preparing execution, after requested -/// permissions have already been considered for approval. -pub(crate) fn with_shell_env_file_read_permission( - mut additional_permissions: Option, - env_file: Option<&Path>, -) -> Option { - let Some(path) = non_empty_shell_env_file_path(env_file) else { - return additional_permissions; - }; - let Ok(path) = AbsolutePathBuf::from_absolute_path(path) else { - return additional_permissions; - }; - let profile = additional_permissions.get_or_insert_with(Default::default); - let file_system = profile.file_system.get_or_insert_with(Default::default); - file_system.entries.push(FileSystemSandboxEntry { - path: FileSystemPath::Path { path }, - access: FileSystemAccessMode::Read, - }); - additional_permissions -} - #[cfg(unix)] fn prepend_path_entry(env: &mut HashMap, path_entry: &str) -> String { let updated_path = match env.get("PATH") { @@ -165,33 +138,26 @@ pub(crate) fn disable_powershell_profile_for_elevated_windows_sandbox( command } -/// POSIX-only wrapper for commands produced by `Shell::derive_exec_args` for -/// Bash/Zsh/sh when shell setup must be sourced before the user script: -/// -/// shell -c/-lc "