openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-21 17:48:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

Simplify session start env file handling

Browse Source
This commit is contained in:
Abhinav Vedmala
2026-05-30 16:53:21 -07:00
parent df54a06344
commit fb5acdbaae
13 changed files with 195 additions and 368 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/session/mod.rs
+2 -5
View File
@@ -3337,10 +3337,6 @@ async fn build_hooks_for_config(
let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await;
let plugin_hook_sources = plugin_outcome.effective_plugin_hook_sources();
let plugin_hook_load_warnings = plugin_outcome.effective_plugin_hook_warnings();
let mut session_start_env = HashMap::new();
if let Some(shell_env_file) = shell_env_file {
shell_env_file.insert_path_into_env(&mut session_start_env);
}
Hooks::new(HooksConfig {
legacy_notify_argv: config.notify.clone(),
feature_enabled: config.features.enabled(Feature::CodexHooks),
@@ -3350,7 +3346,8 @@ async fn build_hooks_for_config(
plugin_hook_load_warnings,
shell_program: Some(hook_shell_program),
shell_args: hook_shell_argv,
session_start_env,
session_start_env_file: shell_env_file
.map(|shell_env_file| shell_env_file.path().to_string_lossy().to_string()),
})
}
codex-rs/core/src/session/session.rs
+4 -2
View File
@@ -865,8 +865,10 @@ impl Session {
default_shell.shell_snapshot = rx;
tx
};
let shell_env_file =
crate::shell_env_file::ShellEnvFile::for_session(thread_id, &default_shell)?;
let shell_env_file = Some(crate::shell_env_file::ShellEnvFile::for_session(
thread_id,
&default_shell,
)?);
let thread_name =
thread_title_from_thread_store(live_thread_init.as_ref(), &thread_store, thread_id)
.instrument(info_span!(
codex-rs/core/src/shell_env_file.rs
+15 -48
View File
@@ -7,9 +7,9 @@ use anyhow::Result;
use anyhow::anyhow;
use anyhow::bail;
use codex_protocol::ThreadId;
use codex_protocol::config_types::EnvironmentVariablePattern;
use codex_protocol::config_types::ShellEnvironmentPolicy;
use codex_protocol::exec_output::bytes_to_string_smart;
use codex_protocol::shell_environment;
use codex_protocol::shell_environment::CLAUDE_ENV_FILE_ENV_VAR;
use codex_protocol::shell_environment::CODEX_ENV_FILE_ENV_VAR;
use codex_protocol::shell_environment::CODEX_THREAD_ID_ENV_VAR;
@@ -31,11 +31,13 @@ pub(crate) struct ShellEnvFile {
}
impl ShellEnvFile {
pub(crate) fn for_session(thread_id: ThreadId, shell: &Shell) -> Result<Option<Self>> {
let Some(capture) = ShellEnvCapture::for_shell_type(&shell.shell_type) else {
return Ok(None);
pub(crate) fn for_session(thread_id: ThreadId, shell: &Shell) -> Result<Self> {
let capture = match shell.shell_type {
ShellType::Zsh | ShellType::Bash | ShellType::Sh => ShellEnvCapture::Posix,
ShellType::PowerShell => ShellEnvCapture::PowerShell,
ShellType::Cmd => ShellEnvCapture::Cmd,
};
Self::new(thread_id, capture).map(Some)
Self::new(thread_id, capture)
}
fn new(thread_id: ThreadId, capture: ShellEnvCapture) -> Result<Self> {
@@ -55,12 +57,6 @@ impl ShellEnvFile {
self.path.as_ref()
}
pub(crate) fn insert_path_into_env(&self, env: &mut HashMap<String, String>) {
let path = self.path().to_string_lossy().to_string();
insert_env_var(env, CODEX_ENV_FILE_ENV_VAR.to_string(), path.clone());
insert_env_var(env, CLAUDE_ENV_FILE_ENV_VAR.to_string(), path);
}
/// Sources the hook-writable env file once and stores the resulting
/// environment diff.
///
@@ -75,14 +71,16 @@ impl ShellEnvFile {
cwd: &Path,
base_env: &HashMap<String, String>,
) -> Result<()> {
if ShellEnvCapture::for_shell_type(&shell.shell_type) != Some(self.capture) {
return Ok(());
}
let mut capture_env = base_env.clone();
remove_env_var(&mut capture_env, CODEX_ENV_FILE_ENV_VAR);
remove_env_var(&mut capture_env, CLAUDE_ENV_FILE_ENV_VAR);
self.insert_path_into_env(&mut capture_env);
let path = self.path().to_string_lossy().to_string();
insert_env_var(
&mut capture_env,
CODEX_ENV_FILE_ENV_VAR.to_string(),
path.clone(),
);
insert_env_var(&mut capture_env, CLAUDE_ENV_FILE_ENV_VAR.to_string(), path);
let (baseline_script, source_script) = self.capture.scripts();
let baseline = self
@@ -168,14 +166,6 @@ enum ShellEnvCapture {
}
impl ShellEnvCapture {
fn for_shell_type(shell_type: &ShellType) -> Option<Self> {
match shell_type {
ShellType::Zsh | ShellType::Bash | ShellType::Sh => Some(Self::Posix),
ShellType::PowerShell => Some(Self::PowerShell),
ShellType::Cmd => Some(Self::Cmd),
}
}
fn file_suffix(self) -> &'static str {
match self {
Self::Posix => ".sh",
@@ -359,30 +349,7 @@ fn ignored_capture_key(key: &str) -> bool {
}
fn should_apply_captured_export(key: &str, policy: &ShellEnvironmentPolicy) -> bool {
!ignored_capture_key(key) && captured_export_allowed_by_policy(key, policy)
}
fn captured_export_allowed_by_policy(key: &str, policy: &ShellEnvironmentPolicy) -> bool {
if !policy.ignore_default_excludes {
let default_excludes = [
EnvironmentVariablePattern::new_case_insensitive("*KEY*"),
EnvironmentVariablePattern::new_case_insensitive("*SECRET*"),
EnvironmentVariablePattern::new_case_insensitive("*TOKEN*"),
];
if matches_any_pattern(key, &default_excludes) {
return false;
}
}
if matches_any_pattern(key, &policy.exclude) {
return false;
}
policy.include_only.is_empty() || matches_any_pattern(key, &policy.include_only)
}
fn matches_any_pattern(key: &str, patterns: &[EnvironmentVariablePattern]) -> bool {
patterns.iter().any(|pattern| pattern.matches(key))
!ignored_capture_key(key) && shell_environment::inherited_env_var_allowed_by_policy(key, policy)
}
fn get_env_var<'a>(env: &'a HashMap<String, String>, key: &str) -> Option<&'a String> {
codex-rs/core/src/shell_env_file_tests.rs
+65 -111
View File
@@ -57,18 +57,8 @@ unset REMOVED_BY_HOOK
.await?;
let mut env = base_env;
env.insert(
CODEX_ENV_FILE_ENV_VAR.to_string(),
env_file.path().display().to_string(),
);
env.insert(
CLAUDE_ENV_FILE_ENV_VAR.to_string(),
env_file.path().display().to_string(),
);
let policy = ShellEnvironmentPolicy {
r#set: HashMap::from([("EXPLICIT_OVERRIDE".to_string(), "from-policy".to_string())]),
..Default::default()
};
insert_env_file_paths(&env_file, &mut env);
let policy = explicit_override_policy();
env_file.apply_exports(&mut env, &policy);
@@ -175,29 +165,14 @@ fn test_shell() -> Shell {
}
#[test]
fn shell_env_file_for_session_uses_powershell_script_suffix() -> Result<()> {
let shell = test_powershell_shell();
let env_file = ShellEnvFile::for_session(ThreadId::new(), &shell)?
.expect("PowerShell should support a session env file");
assert_eq!(
env_file.path().extension().and_then(|ext| ext.to_str()),
Some("ps1")
);
Ok(())
}
#[test]
fn shell_env_file_for_session_uses_cmd_script_suffix() -> Result<()> {
let shell = test_cmd_shell();
let env_file = ShellEnvFile::for_session(ThreadId::new(), &shell)?
.expect("cmd should support a session env file");
assert_eq!(
env_file.path().extension().and_then(|ext| ext.to_str()),
Some("cmd")
);
fn shell_env_file_for_session_uses_shell_script_suffix() -> Result<()> {
for (shell, suffix) in [(test_powershell_shell(), "ps1"), (test_cmd_shell(), "cmd")] {
let env_file = ShellEnvFile::for_session(ThreadId::new(), &shell)?;
assert_eq!(
env_file.path().extension().and_then(|ext| ext.to_str()),
Some(suffix)
);
}
Ok(())
}
@@ -230,17 +205,9 @@ fn cmd_env_output_parser_accepts_set_output() {
#[cfg(windows)]
#[tokio::test]
async fn powershell_env_file_applies_exports_without_exposing_writable_path() -> Result<()> {
let env_file = ShellEnvFile::new(ThreadId::new(), ShellEnvCapture::PowerShell)?;
let base_env = HashMap::from([
("BASE".to_string(), "keep".to_string()),
(
CODEX_THREAD_ID_ENV_VAR.to_string(),
"real-thread".to_string(),
),
("REMOVED_BY_HOOK".to_string(), "remove-me".to_string()),
]);
std::fs::write(
env_file.path(),
assert_windows_env_file_applies_exports(
ShellEnvCapture::PowerShell,
test_powershell_shell(),
r#"
Write-Output "hidden"
$env:CODEX_SESSION_START_TEST = "from-session-start"
@@ -251,30 +218,6 @@ $env:CODEX_THREAD_ID = "poisoned-thread"
$env:EXPLICIT_OVERRIDE = "from-hook"
Remove-Item Env:REMOVED_BY_HOOK -ErrorAction SilentlyContinue
"#,
)?;
let cwd = std::env::current_dir()?;
env_file
.capture_exports(&test_powershell_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(),
);
env.insert(
CLAUDE_ENV_FILE_ENV_VAR.to_string(),
env_file.path().display().to_string(),
);
let policy = ShellEnvironmentPolicy {
r#set: HashMap::from([("EXPLICIT_OVERRIDE".to_string(), "from-policy".to_string())]),
..Default::default()
};
env_file.apply_exports(&mut env, &policy);
assert_eq!(
env,
HashMap::from([
("BASE".to_string(), "keep".to_string()),
(
@@ -287,26 +230,17 @@ Remove-Item Env:REMOVED_BY_HOOK -ErrorAction SilentlyContinue
"real-thread".to_string(),
),
("EXPLICIT_OVERRIDE".to_string(), "from-policy".to_string()),
])
);
Ok(())
]),
)
.await
}
#[cfg(windows)]
#[tokio::test]
async fn cmd_env_file_applies_exports_without_exposing_writable_path() -> Result<()> {
let env_file = ShellEnvFile::new(ThreadId::new(), ShellEnvCapture::Cmd)?;
let base_env = HashMap::from([
("BASE".to_string(), "keep".to_string()),
(
CODEX_THREAD_ID_ENV_VAR.to_string(),
"real-thread".to_string(),
),
("REMOVED_BY_HOOK".to_string(), "remove-me".to_string()),
]);
std::fs::write(
env_file.path(),
assert_windows_env_file_applies_exports(
ShellEnvCapture::Cmd,
test_cmd_shell(),
"\
@echo off\r
set CODEX_SESSION_START_TEST=from-session-start\r
@@ -316,30 +250,6 @@ set CODEX_THREAD_ID=poisoned-thread\r
set EXPLICIT_OVERRIDE=from-hook\r
set REMOVED_BY_HOOK=\r
",
)?;
let cwd = std::env::current_dir()?;
env_file
.capture_exports(&test_cmd_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(),
);
env.insert(
CLAUDE_ENV_FILE_ENV_VAR.to_string(),
env_file.path().display().to_string(),
);
let policy = ShellEnvironmentPolicy {
r#set: HashMap::from([("EXPLICIT_OVERRIDE".to_string(), "from-policy".to_string())]),
..Default::default()
};
env_file.apply_exports(&mut env, &policy);
assert_eq!(
env,
HashMap::from([
("BASE".to_string(), "keep".to_string()),
(
@@ -351,12 +261,56 @@ set REMOVED_BY_HOOK=\r
"real-thread".to_string(),
),
("EXPLICIT_OVERRIDE".to_string(), "from-policy".to_string()),
])
);
]),
)
.await
}
#[cfg(windows)]
async fn assert_windows_env_file_applies_exports(
capture: ShellEnvCapture,
shell: Shell,
script: &str,
expected: HashMap<String, String>,
) -> Result<()> {
let env_file = ShellEnvFile::new(ThreadId::new(), capture)?;
let base_env = HashMap::from([
("BASE".to_string(), "keep".to_string()),
(
CODEX_THREAD_ID_ENV_VAR.to_string(),
"real-thread".to_string(),
),
("REMOVED_BY_HOOK".to_string(), "remove-me".to_string()),
]);
std::fs::write(env_file.path(), script)?;
let cwd = std::env::current_dir()?;
env_file
.capture_exports(&shell, cwd.as_path(), &base_env)
.await?;
let mut env = base_env;
insert_env_file_paths(&env_file, &mut env);
let policy = explicit_override_policy();
env_file.apply_exports(&mut env, &policy);
assert_eq!(env, expected);
Ok(())
}
fn insert_env_file_paths(env_file: &ShellEnvFile, env: &mut HashMap<String, String>) {
let path = env_file.path().display().to_string();
env.insert(CODEX_ENV_FILE_ENV_VAR.to_string(), path.clone());
env.insert(CLAUDE_ENV_FILE_ENV_VAR.to_string(), path);
}
fn explicit_override_policy() -> ShellEnvironmentPolicy {
ShellEnvironmentPolicy {
r#set: HashMap::from([("EXPLICIT_OVERRIDE".to_string(), "from-policy".to_string())]),
..Default::default()
}
}
fn test_powershell_shell() -> Shell {
Shell {
shell_type: ShellType::PowerShell,
codex-rs/core/tests/suite/hooks.rs
-4
View File
@@ -2749,10 +2749,6 @@ async fn assert_session_start_env_file_reaches_bash_surface(
let requests = responses.requests();
assert_eq!(requests.len(), 2);
let output = requests[1].function_call_output(&call_id).to_string();
assert!(
output.contains("from-session-start"),
"expected hook-exported value in {surface:?} output: {output}"
);
assert!(
output.contains("from-session-start||"),
"expected env-file paths to stay hidden from {surface:?} command: {output}"
codex-rs/hooks/Cargo.toml
+1 -1
View File
@@ -25,11 +25,11 @@ regex = { workspace = true }
schemars = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
tempfile = { workspace = true }
tokio = { workspace = true, features = ["fs", "io-util", "process", "time"] }
tracing = { workspace = true }
uuid = { workspace = true, features = ["v4"] }
[dev-dependencies]
pretty_assertions = { workspace = true }
tempfile = { workspace = true }
tokio = { workspace = true, features = ["macros", "rt-multi-thread", "time"] }
codex-rs/hooks/src/engine/command_runner.rs
+6 -12
View File
@@ -120,8 +120,11 @@ fn build_command(shell: &CommandShell, handler: &ConfiguredHandler) -> Command {
// Only SessionStart hooks receive session env-file paths.
command.env_remove(CODEX_ENV_FILE_ENV_VAR);
command.env_remove(CLAUDE_ENV_FILE_ENV_VAR);
if handler.event_name == HookEventName::SessionStart {
command.envs(&shell.session_start_env);
if handler.event_name == HookEventName::SessionStart
&& let Some(path) = &shell.session_start_env_file
{
command.env(CODEX_ENV_FILE_ENV_VAR, path);
command.env(CLAUDE_ENV_FILE_ENV_VAR, path);
}
command
}
@@ -165,16 +168,7 @@ mod tests {
CommandShell {
program: "hook-shell".to_string(),
args: Vec::new(),
session_start_env: HashMap::from([
(
CODEX_ENV_FILE_ENV_VAR.to_string(),
"session-owned-env-file".to_string(),
),
(
CLAUDE_ENV_FILE_ENV_VAR.to_string(),
"session-owned-env-file".to_string(),
),
]),
session_start_env_file: Some("session-owned-env-file".to_string()),
}
}
codex-rs/hooks/src/engine/mod.rs
+1 -1
View File
@@ -36,7 +36,7 @@ use std::collections::HashMap;
pub(crate) struct CommandShell {
pub program: String,
pub args: Vec<String>,
pub session_start_env: HashMap<String, String>,
pub session_start_env_file: Option<String>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
codex-rs/hooks/src/engine/mod_tests.rs
+17 -17
View File
@@ -203,7 +203,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -219,7 +219,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
plugin_hook_load_warnings: Vec::new(),
shell_program: None,
shell_args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
});
assert!(listed.hooks[0].is_managed);
let cwd = cwd();
@@ -308,7 +308,7 @@ async fn requirements_managed_hooks_execute_windows_command_override() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -388,7 +388,7 @@ fn unknown_requirement_source_hooks_stay_managed() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -471,7 +471,7 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -535,7 +535,7 @@ fn user_disablement_does_not_filter_managed_layer_hooks() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -697,7 +697,7 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -754,7 +754,7 @@ fn allow_managed_hooks_only_false_keeps_unmanaged_hooks() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -809,7 +809,7 @@ fn allow_managed_hooks_only_in_config_toml_does_not_enable_policy() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -880,7 +880,7 @@ fn allow_managed_hooks_only_skips_unmanaged_json_and_toml_hooks() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -920,7 +920,7 @@ fn allow_managed_hooks_only_skips_unmanaged_plugin_hooks() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -993,7 +993,7 @@ fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -1104,7 +1104,7 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -1198,7 +1198,7 @@ print(json.dumps({
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -1227,7 +1227,7 @@ print(json.dumps({
plugin_hook_load_warnings: Vec::new(),
shell_program: None,
shell_args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
});
assert_eq!(
listed.hooks[0].plugin_id.as_deref(),
@@ -1314,7 +1314,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
@@ -1359,7 +1359,7 @@ fn plugin_hook_load_warnings_are_startup_warnings() {
CommandShell {
program: String::new(),
args: Vec::new(),
session_start_env: HashMap::new(),
session_start_env_file: None,
},
);
codex-rs/hooks/src/events/session_start.rs
+40 -77
View File
@@ -365,7 +365,13 @@ mod tests {
use pretty_assertions::assert_eq;
use super::SessionStartHandlerData;
use super::SessionStartOutcome;
use super::SessionStartRequest;
use super::SessionStartSource;
use super::StartHookTarget;
use super::parse_completed;
use super::run;
use crate::engine::CommandShell;
use crate::engine::ConfiguredHandler;
use crate::engine::command_runner::CommandRunResult;
@@ -522,36 +528,11 @@ mod tests {
#[cfg(not(windows))]
#[tokio::test]
async fn session_start_hooks_merge_env_file_in_configured_order_after_parallel_runs() {
use std::collections::HashMap;
use std::fs;
use codex_protocol::ThreadId;
use codex_protocol::shell_environment::CLAUDE_ENV_FILE_ENV_VAR;
use codex_protocol::shell_environment::CODEX_ENV_FILE_ENV_VAR;
use tempfile::NamedTempFile;
use tempfile::tempdir;
use super::SessionStartRequest;
use super::SessionStartSource;
use super::StartHookTarget;
use super::run;
use crate::engine::CommandShell;
let env_file = NamedTempFile::new().expect("create env file");
let shell = CommandShell {
program: "sh".to_string(),
args: vec!["-c".to_string()],
session_start_env: HashMap::from([
(
CODEX_ENV_FILE_ENV_VAR.to_string(),
env_file.path().display().to_string(),
),
(
CLAUDE_ENV_FILE_ENV_VAR.to_string(),
env_file.path().display().to_string(),
),
]),
};
let env_file = tempfile::NamedTempFile::new().expect("create env file");
let cwd = tempdir().expect("create cwd");
let marker = cwd.path().join("second-started");
let mut first = handler();
@@ -577,22 +558,8 @@ printf 'export FIRST=1\n' >> "$CLAUDE_ENV_FILE""#
r#"printf 'export SECOND=1\n' >> "$CODEX_ENV_FILE"; touch "$MARKER""#.to_string();
second.display_order = 1;
let outcome = run(
&[first, second],
&shell,
SessionStartRequest {
session_id: ThreadId::new(),
cwd: cwd.path().to_path_buf().abs(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
target: StartHookTarget::SessionStart {
source: SessionStartSource::Startup,
},
},
/*turn_id*/ None,
)
.await;
let outcome =
run_session_start_env_file_hooks(&[first, second], &env_file, cwd.path()).await;
assert_eq!(
outcome
@@ -611,29 +578,11 @@ printf 'export FIRST=1\n' >> "$CLAUDE_ENV_FILE""#
#[cfg(not(windows))]
#[tokio::test]
async fn session_start_hooks_merge_only_successful_env_files_with_separator() {
use std::collections::HashMap;
use std::fs;
use codex_protocol::ThreadId;
use codex_protocol::shell_environment::CODEX_ENV_FILE_ENV_VAR;
use tempfile::NamedTempFile;
use tempfile::tempdir;
use super::SessionStartRequest;
use super::SessionStartSource;
use super::StartHookTarget;
use super::run;
use crate::engine::CommandShell;
let env_file = NamedTempFile::new().expect("create env file");
let shell = CommandShell {
program: "sh".to_string(),
args: vec!["-c".to_string()],
session_start_env: HashMap::from([(
CODEX_ENV_FILE_ENV_VAR.to_string(),
env_file.path().display().to_string(),
)]),
};
let env_file = tempfile::NamedTempFile::new().expect("create env file");
let mut first = handler();
first.command = r#"printf 'export FIRST=1' >> "$CODEX_ENV_FILE""#.to_string();
let mut failed = handler();
@@ -644,22 +593,8 @@ printf 'export FIRST=1\n' >> "$CLAUDE_ENV_FILE""#
third.display_order = 2;
let cwd = tempdir().expect("create cwd");
let outcome = run(
&[first, failed, third],
&shell,
SessionStartRequest {
session_id: ThreadId::new(),
cwd: cwd.path().to_path_buf().abs(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
target: StartHookTarget::SessionStart {
source: SessionStartSource::Startup,
},
},
/*turn_id*/ None,
)
.await;
let outcome =
run_session_start_env_file_hooks(&[first, failed, third], &env_file, cwd.path()).await;
assert_eq!(
outcome
@@ -679,6 +614,34 @@ printf 'export FIRST=1\n' >> "$CLAUDE_ENV_FILE""#
);
}
#[cfg(not(windows))]
async fn run_session_start_env_file_hooks(
handlers: &[ConfiguredHandler],
env_file: &tempfile::NamedTempFile,
cwd: &std::path::Path,
) -> SessionStartOutcome {
run(
handlers,
&CommandShell {
program: "sh".to_string(),
args: vec!["-c".to_string()],
session_start_env_file: Some(env_file.path().display().to_string()),
},
SessionStartRequest {
session_id: codex_protocol::ThreadId::new(),
cwd: cwd.to_path_buf().abs(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
target: StartHookTarget::SessionStart {
source: SessionStartSource::Startup,
},
},
/*turn_id*/ None,
)
.await
}
fn handler() -> ConfiguredHandler {
handler_for(HookEventName::SessionStart)
}
codex-rs/hooks/src/events/session_start_env_file.rs
+18 -68
View File
@@ -1,19 +1,13 @@
use std::io::ErrorKind;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use std::path::PathBuf;
use anyhow::Context;
use anyhow::Result;
use futures::StreamExt;
use futures::stream::FuturesUnordered;
use tempfile::TempPath;
use tokio::fs;
use tokio::io::AsyncWriteExt;
use uuid::Uuid;
use codex_protocol::shell_environment::CLAUDE_ENV_FILE_ENV_VAR;
use codex_protocol::shell_environment::CODEX_ENV_FILE_ENV_VAR;
use crate::engine::CommandShell;
use crate::engine::ConfiguredHandler;
@@ -38,63 +32,28 @@ pub(super) async fn execute_handlers<T>(
) -> Vec<dispatcher::ParsedHandler<T>> {
// No env file means there is nothing special to isolate; use the standard
// parallel dispatcher.
let Some(env_file_path) = shell
.session_start_env
.get(CODEX_ENV_FILE_ENV_VAR)
.or_else(|| shell.session_start_env.get(CLAUDE_ENV_FILE_ENV_VAR))
.map(PathBuf::from)
else {
let Some(env_file_path) = shell.session_start_env_file.as_deref() else {
return dispatcher::execute_handlers(shell, handlers, input_json, cwd, turn_id, parse)
.await;
};
let file_name = env_file_path
.file_name()
.map(|name| name.to_string_lossy())
.unwrap_or_else(|| "env".into());
let env_file_path = Path::new(env_file_path);
let scratch_dir = env_file_path.parent().unwrap_or_else(|| Path::new("."));
let mut pending = FuturesUnordered::new();
for (configured_order, handler) in handlers.into_iter().enumerate() {
// Each handler writes to an isolated env file, but still sees both
// compatibility aliases when the original shell had them.
let scratch_path = env_file_path.with_file_name(format!(
".{file_name}.{configured_order}.{}.tmp",
Uuid::new_v4()
));
let scratch_result = async {
let mut options = std::fs::OpenOptions::new();
options.write(true).create_new(true);
#[cfg(unix)]
options.mode(0o600);
options.open(&scratch_path).with_context(|| {
format!(
"failed to create SessionStart scratch env file {}",
scratch_path.display()
)
})?;
let scratch_result = tempfile::NamedTempFile::new_in(scratch_dir)
.context("failed to create SessionStart scratch env file")
.map(|scratch_file| {
let scratch_path = scratch_file.into_temp_path();
let mut scratch_shell = shell.clone();
let scratch_path_env = scratch_path.to_string_lossy().to_string();
if scratch_shell
.session_start_env
.contains_key(CODEX_ENV_FILE_ENV_VAR)
{
scratch_shell
.session_start_env
.insert(CODEX_ENV_FILE_ENV_VAR.to_string(), scratch_path_env.clone());
}
if scratch_shell
.session_start_env
.contains_key(CLAUDE_ENV_FILE_ENV_VAR)
{
scratch_shell
.session_start_env
.insert(CLAUDE_ENV_FILE_ENV_VAR.to_string(), scratch_path_env);
}
let mut scratch_shell = shell.clone();
scratch_shell.session_start_env_file =
Some(scratch_path.to_string_lossy().to_string());
Ok((scratch_path, scratch_shell))
}
.await;
(scratch_path, scratch_shell)
});
let input_json = input_json.clone();
let turn_id = turn_id.clone();
@@ -133,7 +92,7 @@ pub(super) async fn execute_handlers<T>(
}
completed.sort_by_key(|execution| execution.configured_order);
let mut env_file_ends_with_newline = match fs::read(&env_file_path).await {
let mut env_file_ends_with_newline = match fs::read(env_file_path).await {
Ok(contents) => contents.last().is_none_or(|byte| *byte == b'\n'),
Err(error) if error.kind() == ErrorKind::NotFound => true,
Err(error) => {
@@ -149,7 +108,7 @@ pub(super) async fn execute_handlers<T>(
// core session layer will source this single canonical file after hooks
// finish.
for execution in &mut completed {
let Some(scratch_path) = execution.scratch_path.take() else {
let Some(scratch_path) = execution.scratch_path.as_ref() else {
continue;
};
let result: Result<bool> = async {
@@ -157,7 +116,7 @@ pub(super) async fn execute_handlers<T>(
return Ok(env_file_ends_with_newline);
}
let contents = fs::read(&scratch_path).await.with_context(|| {
let contents = fs::read(scratch_path).await.with_context(|| {
format!(
"failed to read SessionStart scratch env file {}",
scratch_path.display()
@@ -170,7 +129,7 @@ pub(super) async fn execute_handlers<T>(
let mut file = fs::OpenOptions::new()
.append(true)
.create(true)
.open(&env_file_path)
.open(env_file_path)
.await
.with_context(|| format!("failed to open {}", env_file_path.display()))?;
if !env_file_ends_with_newline && contents.first() != Some(&b'\n') {
@@ -188,15 +147,6 @@ pub(super) async fn execute_handlers<T>(
}
.await;
let cleanup_result = fs::remove_file(&scratch_path).await;
if let Err(error) = cleanup_result
&& error.kind() != ErrorKind::NotFound
{
tracing::warn!(
"failed to remove SessionStart scratch env file {}: {error}",
scratch_path.display()
);
}
match result {
Ok(ends_with_newline) => {
env_file_ends_with_newline = ends_with_newline;
@@ -222,7 +172,7 @@ struct HandlerExecution {
completion_order: usize,
handler: ConfiguredHandler,
run_result: CommandRunResult,
scratch_path: Option<PathBuf>,
scratch_path: Option<TempPath>,
turn_id: Option<String>,
}
codex-rs/hooks/src/registry.rs
+2 -3
View File
@@ -1,6 +1,5 @@
use codex_config::ConfigLayerStack;
use codex_plugin::PluginHookSource;
use std::collections::HashMap;
use tokio::process::Command;
use crate::engine::ClaudeHooksEngine;
@@ -37,7 +36,7 @@ pub struct HooksConfig {
pub plugin_hook_load_warnings: Vec<String>,
pub shell_program: Option<String>,
pub shell_args: Vec<String>,
pub session_start_env: HashMap<String, String>,
pub session_start_env_file: Option<String>,
}
#[derive(Debug, Clone, Default, PartialEq, Eq)]
@@ -75,7 +74,7 @@ impl Hooks {
CommandShell {
program: config.shell_program.unwrap_or_default(),
args: config.shell_args,
session_start_env: config.session_start_env,
session_start_env_file: config.session_start_env_file,
},
);
Self {
codex-rs/protocol/src/shell_environment.rs
+24 -19
View File
@@ -2,6 +2,15 @@ use crate::config_types::EnvironmentVariablePattern;
use crate::config_types::ShellEnvironmentPolicy;
use crate::config_types::ShellEnvironmentPolicyInherit;
use std::collections::HashMap;
use std::sync::LazyLock;
static DEFAULT_EXCLUDES: LazyLock<[EnvironmentVariablePattern; 3]> = LazyLock::new(|| {
[
EnvironmentVariablePattern::new_case_insensitive("*KEY*"),
EnvironmentVariablePattern::new_case_insensitive("*SECRET*"),
EnvironmentVariablePattern::new_case_insensitive("*TOKEN*"),
]
});
pub const CODEX_ENV_FILE_ENV_VAR: &str = "CODEX_ENV_FILE";
pub const CLAUDE_ENV_FILE_ENV_VAR: &str = "CLAUDE_ENV_FILE";
@@ -74,24 +83,8 @@ where
}
};
let matches_any = |name: &str, patterns: &[EnvironmentVariablePattern]| -> bool {
patterns.iter().any(|pattern| pattern.matches(name))
};
// Step 2 - Apply the default exclude if not disabled.
if !policy.ignore_default_excludes {
let default_excludes = vec![
EnvironmentVariablePattern::new_case_insensitive("*KEY*"),
EnvironmentVariablePattern::new_case_insensitive("*SECRET*"),
EnvironmentVariablePattern::new_case_insensitive("*TOKEN*"),
];
env_map.retain(|k, _| !matches_any(k, &default_excludes));
}
// Step 3 - Apply custom excludes.
if !policy.exclude.is_empty() {
env_map.retain(|k, _| !matches_any(k, &policy.exclude));
}
// Steps 2, 3, and 5 - Filter inherited variables.
env_map.retain(|key, _| inherited_env_var_allowed_by_policy(key, policy));
// Step 4 - Apply user-provided overrides.
for (key, val) in &policy.r#set {
@@ -100,7 +93,7 @@ where
// Step 5 - If include_only is non-empty, keep only the matching vars.
if !policy.include_only.is_empty() {
env_map.retain(|k, _| matches_any(k, &policy.include_only));
env_map.retain(|key, _| matches_any_pattern(key, &policy.include_only));
}
// Step 6 - Populate the thread ID environment variable when provided.
@@ -111,6 +104,18 @@ where
env_map
}
/// Returns whether an inherited variable is permitted by the filtering parts
/// of a shell-environment policy.
pub fn inherited_env_var_allowed_by_policy(name: &str, policy: &ShellEnvironmentPolicy) -> bool {
(policy.ignore_default_excludes || !matches_any_pattern(name, DEFAULT_EXCLUDES.as_slice()))
&& !matches_any_pattern(name, &policy.exclude)
&& (policy.include_only.is_empty() || matches_any_pattern(name, &policy.include_only))
}
fn matches_any_pattern(name: &str, patterns: &[EnvironmentVariablePattern]) -> bool {
patterns.iter().any(|pattern| pattern.matches(name))
}
#[cfg(not(target_os = "windows"))]
const UNIX_CORE_ENV_VARS: &[&str] = &[
"PATH", "SHELL", "TMPDIR", "TEMP", "TMP", "HOME", "LANG", "LC_ALL", "LC_CTYPE", "LOGNAME",