Add hook env file persistence

This commit is contained in:
Abhinav Vedmala
2026-05-25 12:09:46 -07:00
parent 613e5149a4
commit 5bccb54bdf
13 changed files with 678 additions and 8 deletions
+252
View File
@@ -0,0 +1,252 @@
use std::collections::HashMap;
use std::fs;
use codex_protocol::ThreadId;
use codex_utils_absolute_path::AbsolutePathBuf;
use tracing::warn;
const HOOK_ENV_DIR: &str = "env";
#[derive(Debug)]
pub(crate) struct HookEnvFile {
path: AbsolutePathBuf,
}
impl HookEnvFile {
/// Creates the per-thread env file handle and makes its parent directory appendable for hooks.
pub(crate) fn new(codex_home: &AbsolutePathBuf, thread_id: ThreadId) -> Self {
let env_file = Self {
path: codex_home
.join(HOOK_ENV_DIR)
.join(format!("{thread_id}.sh")),
};
env_file.ensure_parent();
env_file
}
/// Returns the path exposed to hook commands via CODEX_ENV_FILE and CLAUDE_ENV_FILE.
pub(crate) fn path(&self) -> &AbsolutePathBuf {
&self.path
}
/// Best-effort directory setup so a hook can append to the advertised env file path.
fn ensure_parent(&self) {
let Some(parent) = self.path.as_path().parent() else {
return;
};
if let Err(err) = fs::create_dir_all(parent) {
warn!(
path = %parent.display(),
"failed to create hook env file directory: {err}"
);
}
}
/// Applies persisted hook env updates to the command environment used by later local tools.
pub(crate) fn apply_to_env(&self, env: &mut HashMap<String, String>) {
match fs::read_to_string(self.path.as_path()) {
Ok(contents) => {
for line in contents.lines() {
apply_env_file_line(env, line);
}
}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
Err(err) => {
warn!(
path = %self.path.display(),
"failed to read hook env file: {err}"
);
}
}
}
}
/// Applies one supported shell-style env update line.
///
/// We intentionally support the common hook outputs here rather than sourcing
/// the file in a hidden shell wrapper: `export NAME=value`, plus Bash's
/// `declare -x NAME=value` form emitted by `export -p`.
fn apply_env_file_line(env: &mut HashMap<String, String>, line: &str) {
let line = line.trim();
if line.is_empty() || line.starts_with('#') {
return;
}
let Some(assignment) = line
.strip_prefix("export ")
.or_else(|| line.strip_prefix("declare -x "))
else {
return;
};
let assignment = assignment.trim_start();
let Some((name, value)) = assignment.split_once('=') else {
return;
};
let name = name.trim();
if !is_env_name(name) {
return;
}
let value = parse_env_value(value.trim_start(), env);
env.insert(name.to_string(), value);
}
/// Parses the value side of an env assignment, preserving single-quoted values literally.
fn parse_env_value(value: &str, env: &HashMap<String, String>) -> String {
if let Some(value) = value
.strip_prefix('\'')
.and_then(|value| value.strip_suffix('\''))
{
return value.to_string();
}
if let Some(value) = value
.strip_prefix('"')
.and_then(|value| value.strip_suffix('"'))
{
return expand_env_vars(&unescape_double_quoted(value), env);
}
expand_env_vars(value.trim_end(), env)
}
/// Handles the small set of backslash escapes expected inside double-quoted env values.
fn unescape_double_quoted(value: &str) -> String {
let mut out = String::with_capacity(value.len());
let mut chars = value.chars();
while let Some(ch) = chars.next() {
if ch == '\\'
&& let Some(next) = chars.next()
{
match next {
'$' | '"' | '\\' => out.push(next),
other => {
out.push(ch);
out.push(other);
}
}
continue;
}
out.push(ch);
}
out
}
/// Expands `$NAME` and `${NAME}` using the environment accumulated for this command.
fn expand_env_vars(value: &str, env: &HashMap<String, String>) -> String {
let mut out = String::with_capacity(value.len());
let mut chars = value.chars().peekable();
while let Some(ch) = chars.next() {
if ch != '$' {
out.push(ch);
continue;
}
if chars.peek() == Some(&'{') {
chars.next();
let mut name = String::new();
while let Some(&next) = chars.peek() {
chars.next();
if next == '}' {
break;
}
name.push(next);
}
if is_env_name(&name) {
out.push_str(env.get(&name).map_or("", String::as_str));
} else {
out.push_str("${");
out.push_str(&name);
out.push('}');
}
continue;
}
let mut name = String::new();
while let Some(&next) = chars.peek() {
if !is_env_name_char(next) {
break;
}
chars.next();
name.push(next);
}
if name.is_empty() {
out.push('$');
} else if is_env_name(&name) {
out.push_str(env.get(&name).map_or("", String::as_str));
} else {
out.push('$');
out.push_str(&name);
}
}
out
}
/// Returns whether a string is a valid shell environment variable name.
fn is_env_name(name: &str) -> bool {
let mut chars = name.chars();
let Some(first) = chars.next() else {
return false;
};
(first == '_' || first.is_ascii_alphabetic()) && chars.all(is_env_name_char)
}
/// Returns whether a character is valid after the first character in an env var name.
fn is_env_name_char(ch: char) -> bool {
ch == '_' || ch.is_ascii_alphanumeric()
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
fn apply_env_file_lines(env: &mut HashMap<String, String>, contents: &str) {
for line in contents.lines() {
apply_env_file_line(env, line);
}
}
#[test]
fn env_file_applies_exports_and_expands_path() {
let mut env = HashMap::from([("PATH".to_string(), "/usr/bin".to_string())]);
apply_env_file_lines(
&mut env,
r#"
# ignored
export FOO=bar
export PATH="/plugin/bin:$PATH"
export LITERAL='$FOO'
"#,
);
assert_eq!(
env,
HashMap::from([
("FOO".to_string(), "bar".to_string()),
("LITERAL".to_string(), "$FOO".to_string()),
("PATH".to_string(), "/plugin/bin:/usr/bin".to_string()),
])
);
}
#[test]
fn env_file_supports_braced_references_and_declare_exports() {
let mut env = HashMap::from([("BASE".to_string(), "base".to_string())]);
apply_env_file_lines(
&mut env,
r#"
declare -x FROM_DECLARE="${BASE}/declare"
export FROM_BRACES=${FROM_DECLARE}/braces
"#,
);
assert_eq!(
env,
HashMap::from([
("BASE".to_string(), "base".to_string()),
("FROM_DECLARE".to_string(), "base/declare".to_string()),
("FROM_BRACES".to_string(), "base/declare/braces".to_string()),
])
);
}
}
+1
View File
@@ -40,6 +40,7 @@ mod goals;
pub use goals::ExternalGoalPreviousStatus;
pub use goals::ExternalGoalSet;
mod guardian;
mod hook_env;
mod hook_runtime;
mod installation_id;
pub(crate) mod landlock;
+7
View File
@@ -1476,6 +1476,7 @@ impl Session {
config.as_ref(),
self.services.plugins_manager.as_ref(),
self.services.user_shell.as_ref(),
&self.services.hook_env_file,
)
.await;
@@ -3243,6 +3244,10 @@ impl Session {
self.services.hooks.load_full()
}
pub(crate) fn apply_hook_env_file(&self, env: &mut HashMap<String, String>) {
self.services.hook_env_file.apply_to_env(env);
}
pub(crate) fn user_shell(&self) -> Arc<shell::Shell> {
Arc::clone(&self.services.user_shell)
}
@@ -3315,6 +3320,7 @@ async fn build_hooks_for_config(
config: &Config,
plugins_manager: &PluginsManager,
user_shell: &crate::shell::Shell,
hook_env_file: &crate::hook_env::HookEnvFile,
) -> Hooks {
let mut hook_shell_argv = user_shell.derive_exec_args("", /*use_login_shell*/ false);
let hook_shell_program = hook_shell_argv.remove(0);
@@ -3332,6 +3338,7 @@ async fn build_hooks_for_config(
plugin_hook_load_warnings,
shell_program: Some(hook_shell_program),
shell_args: hook_shell_argv,
env_file_path: Some(hook_env_file.path().clone()),
})
}
+9 -2
View File
@@ -930,8 +930,14 @@ impl Session {
(None, None)
};
let hooks =
build_hooks_for_config(&config, plugins_manager.as_ref(), &default_shell).await;
let hook_env_file = crate::hook_env::HookEnvFile::new(&config.codex_home, thread_id);
let hooks = build_hooks_for_config(
&config,
plugins_manager.as_ref(),
&default_shell,
&hook_env_file,
)
.await;
for warning in hooks.startup_warnings() {
post_session_configured_events.push(Event {
id: INITIAL_SUBMIT_ID.to_owned(),
@@ -988,6 +994,7 @@ impl Session {
main_execve_wrapper_exe: config.main_execve_wrapper_exe.clone(),
analytics_events_client,
hooks: arc_swap::ArcSwap::from_pointee(hooks),
hook_env_file,
rollout_thread_trace,
user_shell: Arc::new(default_shell),
shell_snapshot_tx,
+2
View File
@@ -4536,6 +4536,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
legacy_notify_argv: config.notify.clone(),
..HooksConfig::default()
})),
hook_env_file: crate::hook_env::HookEnvFile::new(&config.codex_home, thread_id),
rollout_thread_trace: codex_rollout_trace::ThreadTraceContext::disabled(),
user_shell: Arc::new(default_user_shell()),
shell_snapshot_tx: watch::channel(None).0,
@@ -6365,6 +6366,7 @@ where
legacy_notify_argv: config.notify.clone(),
..HooksConfig::default()
})),
hook_env_file: crate::hook_env::HookEnvFile::new(&config.codex_home, thread_id),
rollout_thread_trace: codex_rollout_trace::ThreadTraceContext::disabled(),
user_shell: Arc::new(default_user_shell()),
shell_snapshot_tx: watch::channel(None).0,
+2
View File
@@ -10,6 +10,7 @@ use crate::config::StartedNetworkProxy;
use crate::exec_policy::ExecPolicyManager;
use crate::guardian::GuardianRejection;
use crate::guardian::GuardianRejectionCircuitBreaker;
use crate::hook_env::HookEnvFile;
use crate::mcp::McpManager;
use crate::tools::code_mode::CodeModeService;
use crate::tools::network_approval::NetworkApprovalService;
@@ -48,6 +49,7 @@ pub(crate) struct SessionServices {
pub(crate) main_execve_wrapper_exe: Option<PathBuf>,
pub(crate) analytics_events_client: AnalyticsEventsClient,
pub(crate) hooks: ArcSwap<Hooks>,
pub(crate) hook_env_file: HookEnvFile,
pub(crate) rollout_thread_trace: ThreadTraceContext,
pub(crate) user_shell: Arc<crate::shell::Shell>,
pub(crate) shell_snapshot_tx: watch::Sender<Option<Arc<crate::shell_snapshot::ShellSnapshot>>>,
@@ -95,12 +95,15 @@ impl ShellCommandHandler {
#[allow(deprecated)]
let cwd = turn_context.resolve_path(params.workdir.clone());
let mut env = create_env(&turn_context.shell_environment_policy, Some(thread_id));
session.apply_hook_env_file(&mut env);
Ok(ExecParams {
command,
cwd,
expiration: params.timeout_ms.into(),
capture_policy: ExecCapturePolicy::ShellTool,
env: create_env(&turn_context.shell_environment_policy, Some(thread_id)),
env,
network: turn_context.network.clone(),
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
windows_sandbox_level: turn_context.windows_sandbox_level,
@@ -1006,6 +1006,13 @@ impl UnifiedExecProcessManager {
/*thread_id*/ None,
);
let mut env = local_policy_env.clone();
if request.environment.is_remote() {
tracing::debug!(
"CODEX_ENV_FILE overlays are local-only; skipping persisted hook env for remote exec",
);
} else {
context.session.apply_hook_env_file(&mut env);
}
env.insert(
CODEX_THREAD_ID_ENV_VAR.to_string(),
context.session.conversation_id.to_string(),
+166
View File
@@ -104,6 +104,79 @@ fn trust_plugin_hooks(config: &mut Config, plugin_hook_sources: Vec<PluginHookSo
trust_hooks(config, listed.hooks);
}
fn write_path_persisting_plugin(
home: &Path,
) -> Result<(Vec<PluginHookSource>, std::path::PathBuf)> {
let plugin_root = home.join("plugins/cache/test/sample/local");
let hooks_dir = plugin_root.join("hooks");
let plugin_data = home.join("plugins/data/sample-test");
fs::create_dir_all(plugin_root.join(".codex-plugin"))
.context("create plugin manifest directory")?;
fs::create_dir_all(&hooks_dir).context("create plugin hooks directory")?;
fs::write(
plugin_root.join(".codex-plugin/plugin.json"),
r#"{"name":"sample"}"#,
)
.context("write plugin manifest")?;
fs::write(
home.join("config.toml"),
r#"[plugins."sample@test"]
enabled = true
"#,
)
.context("write plugin config")?;
let script_path = hooks_dir.join("session_start_hook.py");
fs::write(
&script_path,
r##"import os
from pathlib import Path
env_file = os.environ.get("CODEX_ENV_FILE")
if not env_file or env_file != os.environ.get("CLAUDE_ENV_FILE"):
raise SystemExit("missing CODEX_ENV_FILE/CLAUDE_ENV_FILE")
bin_dir = Path(os.environ["PLUGIN_DATA"]) / "bin"
bin_dir.mkdir(parents=True, exist_ok=True)
shim = bin_dir / "adam-shim"
shim.write_text("#!/bin/sh\nprintf shim-ok\n", encoding="utf-8")
shim.chmod(0o755)
with open(env_file, "a", encoding="utf-8") as handle:
handle.write(f'export PATH="{bin_dir}:$PATH"\n')
"##,
)
.context("write plugin session start hook script")?;
let plugin_hooks_json = r#"{
"hooks": {
"SessionStart": [{
"matcher": "startup",
"hooks": [{
"type": "command",
"command": "python3 ${PLUGIN_ROOT}/hooks/session_start_hook.py"
}]
}]
}
}"#;
let plugin_hooks_path = hooks_dir.join("hooks.json");
fs::write(&plugin_hooks_path, plugin_hooks_json).context("write plugin hooks config")?;
let plugin_hook_sources = vec![PluginHookSource {
plugin_id: PluginId::parse("sample@test").context("plugin id")?,
plugin_root: AbsolutePathBuf::try_from(plugin_root).context("absolute plugin root")?,
plugin_data_root: AbsolutePathBuf::try_from(plugin_data.clone())
.context("absolute plugin data root")?,
source_path: AbsolutePathBuf::try_from(plugin_hooks_path)
.context("absolute plugin hooks path")?,
source_relative_path: "hooks/hooks.json".to_string(),
hooks: serde_json::from_str::<codex_config::HooksFile>(plugin_hooks_json)
.context("parse plugin hooks")?
.hooks,
}];
Ok((plugin_hook_sources, plugin_data))
}
fn write_stop_hook(home: &Path, block_prompts: &[&str]) -> Result<()> {
let script_path = home.join("stop_hook.py");
let log_path = home.join("stop_hook_log.jsonl");
@@ -2973,6 +3046,99 @@ print(json.dumps({{
Ok(())
}
#[tokio::test]
async fn plugin_session_start_env_file_path_persists_for_shell_and_exec_command() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let shell_call_id = "plugin-env-file-shell-command";
let exec_call_id = "plugin-env-file-exec-command";
let shell_args = serde_json::json!({ "command": "adam-shim" });
let exec_args = serde_json::json!({ "cmd": "adam-shim" });
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(
shell_call_id,
"shell_command",
&serde_json::to_string(&shell_args)?,
),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_function_call(
exec_call_id,
"exec_command",
&serde_json::to_string(&exec_args)?,
),
ev_completed("resp-2"),
]),
sse(vec![
ev_response_created("resp-3"),
ev_assistant_message("msg-1", "both shim calls worked"),
ev_completed("resp-3"),
]),
],
)
.await;
let home = Arc::new(TempDir::new()?);
let (plugin_hook_sources, plugin_data) = write_path_persisting_plugin(home.path())?;
let mut builder = test_codex()
.with_home(Arc::clone(&home))
.with_config(move |config| {
config.use_experimental_unified_exec_tool = true;
config
.features
.enable(Feature::Plugins)
.expect("test config should allow feature update");
config
.features
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
trust_plugin_hooks(config, plugin_hook_sources);
});
let test = builder.build(&server).await?;
test.submit_turn_with_permission_profile(
"run the plugin shim by name with shell and exec",
PermissionProfile::Disabled,
)
.await?;
assert!(
plugin_data.join("bin/adam-shim").exists(),
"session start hook should create the shim under plugin data"
);
let requests = responses.requests();
assert_eq!(requests.len(), 3);
let shell_output_item = requests[1].function_call_output(shell_call_id);
let shell_output = shell_output_item
.get("output")
.and_then(Value::as_str)
.expect("shell command output string");
assert!(
shell_output.contains("shim-ok"),
"shell command should find the shim through CODEX_ENV_FILE PATH"
);
let exec_output_item = requests[2].function_call_output(exec_call_id);
let exec_output = exec_output_item
.get("output")
.and_then(Value::as_str)
.expect("exec command output string");
assert!(
exec_output.contains("shim-ok"),
"exec command should find the shim through CODEX_ENV_FILE PATH"
);
Ok(())
}
#[tokio::test]
async fn pre_tool_use_blocks_shell_when_defined_in_config_toml() -> Result<()> {
skip_if_no_network!(Ok(()));