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

Report the local command working directory in PreToolUse hooks (#57198)

Browse Source 
## Why

`PreToolUse` hooks receive the host working directory even when a local
`exec_command` call specifies a different `workdir`. This prevents hooks from
seeing the directory in which the command will execute.

## What changed

- Share working-directory resolution between command execution and hook input,
  using the selected environment and the command's `workdir`.
- Report the resolved local directory in the hook input's `cwd`. Keep hook
  processes in the host directory and retain the host `cwd` for remote commands.
- Reject invalid environment selections or paths before running hooks and record
  failures in tool-result telemetry.

## Testing

Add unit and integration coverage for default, empty, relative, and absolute
working directories; Windows path conventions; remote commands; hook denial and
input rewriting; and invalid arguments that fail without running hooks.

GitOrigin-RevId: 97f71f6d3e21a5d95a7abf5384f06f4993241797
This commit is contained in:
Andrei Eternal
2026-10-26 19:11:28 +00:00
committed by copyberry
parent 60e4d10ad9
commit fe15bd38de
9 changed files with 395 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/hook_runtime.rs
+2
View File
@@ -196,6 +196,7 @@ pub(crate) async fn run_pre_tool_use_hooks(
tool_use_id: String,
tool_name: &HookToolName,
tool_input: &Value,
tool_cwd: Option<codex_utils_path_uri::PathUri>,
target_environments: &[&str],
) -> PreToolUseHookResult {
let turn_context = &step_context.turn;
@@ -204,6 +205,7 @@ pub(crate) async fn run_pre_tool_use_hooks(
turn_id: turn_context.sub_id.clone(),
subagent: thread_spawn_subagent_hook_context(sess, turn_context),
cwd: tool_hook_cwd(&step_context.environments, turn_context),
tool_cwd,
transcript_path: sess.hook_transcript_path().await,
model: step_context.settings.model_info.slug.clone(),
permission_mode: hook_permission_mode(step_context.settings.approval_policy()),
codex-rs/core/src/tools/handlers/unified_exec.rs
+16
View File
@@ -1,5 +1,6 @@
use crate::function_tool::FunctionCallError;
use crate::sandboxing::SandboxPermissions;
use crate::session::turn_context::TurnEnvironment;
use crate::shell::Shell;
use crate::shell::ShellInvocation;
use crate::shell::get_shell_by_model_provided_path;
@@ -12,6 +13,7 @@ use crate::tools::handlers::resolve_tool_environment;
use crate::tools::hook_names::HookToolName;
use crate::tools::registry::PostToolUsePayload;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_utils_path_uri::PathUri;
use futures::future::BoxFuture;
use serde::Deserialize;
use std::path::PathBuf;
@@ -93,6 +95,20 @@ struct ExecCommandWorkdirArgs {
workdir: Option<String>,
}
impl ExecCommandWorkdirArgs {
/// Share cwd resolution between hooks and execution after environment selection.
fn resolve_cwd(&self, environment: &TurnEnvironment) -> Result<PathUri, FunctionCallError> {
self.workdir
.as_deref()
.filter(|workdir| !workdir.is_empty())
.map_or_else(
|| Ok(environment.cwd().clone()),
|workdir| environment.cwd().join(workdir),
)
.map_err(|err| FunctionCallError::RespondToModel(err.to_string()))
}
}
fn default_exec_yield_time_ms() -> u64 {
10_000
}
codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs
+21 -11
View File
@@ -47,6 +47,7 @@ use codex_tools::ToolName;
use codex_tools::ToolSpec;
use codex_utils_output_truncation::approx_token_count;
use codex_utils_path_uri::PathConvention;
use codex_utils_path_uri::PathUri;
use codex_utils_string::truncate_middle_chars;
use super::super::shell_spec::CommandToolOptions;
@@ -183,16 +184,7 @@ impl ExecCommandHandler {
);
let workdir_args: ExecCommandWorkdirArgs = parse_arguments(&arguments)?;
let turn_environment = selected_executor(&environments, &step_context.environments)?;
let native_environment_cwd = turn_environment.cwd().clone();
let cwd = workdir_args
.workdir
.as_deref()
.filter(|workdir| !workdir.is_empty())
.map_or_else(
|| Ok(native_environment_cwd.clone()),
|workdir| native_environment_cwd.join(workdir),
)
.map_err(|err| FunctionCallError::RespondToModel(err.to_string()))?;
let cwd = workdir_args.resolve_cwd(turn_environment)?;
let environment = Arc::clone(&turn_environment.environment);
let fs = environment.get_filesystem();
@@ -408,7 +400,7 @@ impl ExecCommandHandler {
yield_time_ms,
max_output_tokens,
cwd,
sandbox_cwd: native_environment_cwd,
sandbox_cwd: turn_environment.cwd().clone(),
turn_environment: turn_environment.clone(),
network: context.step_context.turn.network.clone(),
tty,
@@ -522,6 +514,24 @@ impl CoreToolRuntime for ExecCommandHandler {
})
}
fn pre_tool_use_cwd(
&self,
invocation: &ToolInvocation,
) -> Result<Option<PathUri>, FunctionCallError> {
let ToolPayload::Function { arguments } = &invocation.payload else {
return Ok(None);
};
let args: ExecCommandWorkdirArgs = parse_arguments(arguments)?;
let environment = selected_executor(
&invocation.environments,
&invocation.step_context.environments,
)?;
let cwd = args.resolve_cwd(environment)?;
// Hook scripts execute on the host. Remote command paths cannot replace
// the host cwd in hook input, even when both machines use the same OS.
Ok((!environment.environment.is_remote()).then_some(cwd))
}
fn with_updated_hook_input(
&self,
mut invocation: ToolInvocation,
codex-rs/core/src/tools/handlers/unified_exec_tests.rs
+73
View File
@@ -489,6 +489,79 @@ async fn exec_command_pre_tool_use_payload_uses_raw_command() {
);
}
#[tokio::test]
async fn exec_command_pre_tool_use_cwd_resolves_selected_environment() {
let (session, mut turn) = make_session_and_context().await;
let mut selected = turn.initial_environments.environments[0].clone();
let TurnEnvironmentState::Ready(environment) = &mut selected else {
panic!("ready environment");
};
environment.selection.environment_id = "other".to_string();
environment.selection.cwd = PathUri::parse("file:///C:/repo-B").unwrap();
turn.initial_environments.environments.push(selected);
let turn = Arc::new(turn);
let mut invocation = ToolInvocation {
session: session.into(),
step_context: StepContext::for_test(Arc::clone(&turn)),
turn,
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "hook-cwd".to_string(),
tool_name: codex_tools::ToolName::plain("exec_command"),
source: ToolCallSource::Direct,
payload: ToolPayload::Function {
arguments: String::new(),
},
environments: Default::default(),
};
for (workdir, expected) in [
(None, "file:///C:/repo-B"),
(Some(""), "file:///C:/repo-B"),
(Some(r"nested\..\target"), "file:///C:/repo-B/target"),
(Some(r"D:\repo-C"), "file:///D:/repo-C"),
] {
invocation.payload = ToolPayload::Function {
arguments: serde_json::json!({
"cmd": "echo hello",
"environment_id": "other",
"workdir": workdir,
})
.to_string(),
};
let handler = ExecCommandHandler::default();
invocation.environments = handler.environments_for_input(&invocation).unwrap();
assert_eq!(
handler.pre_tool_use_cwd(&invocation).unwrap(),
Some(PathUri::parse(expected).unwrap()),
);
}
}
#[tokio::test]
async fn exec_command_pre_tool_use_cwd_rejects_invalid_environment_and_path() {
for args in [
serde_json::json!({ "cmd": "echo hello", "environment_id": "missing" }),
serde_json::json!({ "cmd": "echo hello", "workdir": "\u{0}" }),
] {
let mut invocation = invocation_for_payload(
"exec_command",
"invalid-cwd",
ToolPayload::Function {
arguments: args.to_string(),
},
)
.await;
let handler = ExecCommandHandler::default();
let cwd = handler
.environments_for_input(&invocation)
.and_then(|environments| {
invocation.environments = environments;
handler.pre_tool_use_cwd(&invocation)
});
assert!(matches!(cwd, Err(FunctionCallError::RespondToModel(_))));
}
}
#[tokio::test]
async fn exec_command_pre_tool_use_payload_skips_write_stdin() {
let payload = ToolPayload::Function {
codex-rs/core/src/tools/registry.rs
+27
View File
@@ -43,6 +43,7 @@ use codex_tools::FunctionsNamespaceFunctionPrefixes;
use codex_tools::ToolName;
use codex_tools::ToolSearchInfo;
use codex_tools::ToolSpec;
use codex_utils_path_uri::PathUri;
use futures::future::BoxFuture;
use indexmap::IndexMap;
use indexmap::map::Entry;
@@ -175,6 +176,16 @@ pub(crate) trait CoreToolRuntime: ToolExecutor<ToolInvocation> {
})
}
/// Resolves a local execution cwd exposed in `PreToolUse` stdin, when the tool
/// supports a per-call working directory. Remote commands return `None` to
/// retain the host cwd. This does not move the hook process.
fn pre_tool_use_cwd(
&self,
_invocation: &ToolInvocation,
) -> Result<Option<PathUri>, FunctionCallError> {
Ok(None)
}
/// Rebuilds a tool invocation from hook-facing `tool_input`.
///
/// Tools that opt into input-rewriting hooks should invert the same stable
@@ -775,6 +786,17 @@ impl ToolRegistry {
}
let fail_before_execution = async |invocation: &ToolInvocation, err: FunctionCallError| {
let log_payload = tool_log_payload(&invocation.payload, &invocation.source);
otel.tool_result_with_tags(
&tool_name,
&call_id_owned,
log_payload.as_ref(),
Duration::ZERO,
/*success*/ false,
&err.to_string(),
&tool_result_tags,
&extra_trace_fields,
);
if tool.is_builtin_control_tool() {
let mut analytics = ControlToolCallGuard::new(invocation);
analytics.finish(ControlToolCallStatus::Failed);
@@ -799,12 +821,17 @@ impl ToolRegistry {
Err(err) => return Err(fail_before_execution(&invocation, err).await),
};
if let Some(pre_tool_use_payload) = tool.pre_tool_use_payload(&invocation) {
let tool_cwd = match tool.pre_tool_use_cwd(&invocation) {
Ok(cwd) => cwd,
Err(err) => return Err(fail_before_execution(&invocation, err).await),
};
match run_pre_tool_use_hooks(
&invocation.session,
invocation.step_context.as_ref(),
invocation.call_id.clone(),
&pre_tool_use_payload.tool_name,
&pre_tool_use_payload.tool_input,
tool_cwd,
&invocation.environments.hook_ids(),
)
.or_cancel_tool(&cancellation_token)
codex-rs/core/tests/suite/hooks.rs
+4
View File
@@ -91,6 +91,10 @@ const BLOCKED_PROMPT_CONTEXT: &str = "Remember the blocked lighthouse note.";
const PERMISSION_REQUEST_HOOK_MATCHER: &str = "^Bash$";
const PERMISSION_REQUEST_ALLOW_REASON: &str = "should not be used for allow";
#[cfg(test)]
#[path = "hooks_cwd_tests.rs"]
mod cwd_tests;
#[tokio::test]
async fn managed_hook_discovery_failure_rejects_session_startup_when_hooks_enabled() -> Result<()> {
let server = start_mock_server().await;
codex-rs/core/tests/suite/hooks_cwd_tests.rs
+215
View File
@@ -0,0 +1,215 @@
#![cfg(test)]
//! PreToolUse reports local command cwd and retains host cwd for remote commands.
use super::*;
use codex_exec_server::CreateDirectoryOptions;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::test_codex::local;
use pretty_assertions::assert_eq;
use test_case::test_case;
#[test_case(None, "allow"; "default")]
#[test_case(Some(""), "allow"; "empty")]
#[test_case(Some("nested/../nested"), "allow"; "relative")]
#[test_case(Some("absolute"), "allow"; "absolute")]
#[test_case(Some("nested"), "json_deny"; "denied")]
#[test_case(Some("nested"), "rewrite"; "rewritten")]
#[tokio::test]
async fn pre_tool_use_cwd_matches_execution(
workdir: Option<&str>,
mode: &'static str,
) -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let test = test_codex()
.with_pre_build_hook(move |home| {
if mode == "rewrite" {
write_updating_pre_tool_use_hook(
home,
"^Bash$",
&json!({ "command": "cat rewritten-marker.txt" }),
)
.expect("write rewriting hook");
} else {
write_pre_tool_use_hook(home, Some("^Bash$"), mode, "blocked by cwd hook")
.expect("write recording hook");
}
let script_path = home.join("pre_tool_use_hook.py");
let script = fs::read_to_string(&script_path)
.expect("read hook script")
.replace(
"payload = json.load(sys.stdin)",
"payload = json.load(sys.stdin)\npayload['process_cwd'] = str(Path.cwd())",
);
fs::write(script_path, script).expect("record hook process cwd");
})
.with_config(trust_discovered_hooks)
.build_with_remote_and_local_env(&server)
.await?;
// Hooks run on the host even when commands use the remote test executor.
let local_hook_directory = TempDir::new()?;
let mut environments = vec![test.executor_environment().selection().clone()];
let hook_cwd = if test.executor_environment().environment().is_remote() {
let cwd = AbsolutePathBuf::try_from(local_hook_directory.path())?;
environments.push(local(cwd.clone()));
cwd
} else {
test.config.cwd.clone()
};
let root = test.executor_environment().selection().cwd.clone();
let nested = root.join("nested")?;
test.fs()
.create_directory(
&nested,
CreateDirectoryOptions {
recursive: true,
follow_symlinks: true,
},
/*sandbox*/ None,
)
.await?;
for (directory, marker) in [(&root, "root-marker"), (&nested, "nested-marker")] {
for filename in ["cwd-marker.txt", "rewritten-marker.txt"] {
test.fs()
.write_file(
&directory.join(filename)?,
format!("{marker}:{filename}").into_bytes(),
Default::default(),
/*sandbox*/ None,
)
.await?;
}
}
let mut args = json!({ "cmd": "cat cwd-marker.txt", "login": false });
if let Some(workdir) = workdir {
args["workdir"] = if workdir == "absolute" {
json!(nested.inferred_native_path_string())
} else {
json!(workdir)
};
}
let call_id = "hook-cwd";
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(call_id, "exec_command", &args.to_string()),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "done"),
ev_completed("resp-2"),
]),
],
)
.await;
test.submit_turn_with_environments(
"read the marker in the requested directory",
Some(environments),
)
.await?;
let inputs = read_pre_tool_use_hook_inputs(test.codex_home_path())?;
let expected_cwd = if workdir.is_none_or(str::is_empty) {
&root
} else {
&nested
};
assert_eq!(
inputs
.iter()
.map(|input| json!({
"cwd": input["cwd"],
"process_cwd": input["process_cwd"],
"tool_input": input["tool_input"],
}))
.collect::<Vec<_>>(),
vec![json!({
"cwd": if test.executor_environment().environment().is_remote() {
dunce::canonicalize(&hook_cwd)?.to_string_lossy().into_owned()
} else {
expected_cwd.inferred_native_path_string()
},
"process_cwd": dunce::canonicalize(&hook_cwd)?.to_string_lossy(),
"tool_input": { "command": "cat cwd-marker.txt" },
})],
);
let requests = responses.requests();
let output = requests[1].function_call_output(call_id);
let output = output["output"].as_str().context("command output")?;
if mode == "json_deny" {
assert!(output.contains("Command blocked by PreToolUse hook: blocked by cwd hook"));
} else {
let marker = if expected_cwd == &root {
"root-marker"
} else {
"nested-marker"
};
let filename = if mode == "rewrite" {
"rewritten-marker.txt"
} else {
"cwd-marker.txt"
};
assert!(output.contains(&format!("{marker}:{filename}")), "{output}");
}
Ok(())
}
#[tokio::test]
#[tracing_test::traced_test]
async fn invalid_execution_cwd_records_failure_without_running_hooks() -> Result<()> {
skip_if_no_network!(Ok(()));
for (field, value) in [("environment_id", "missing-environment"), ("workdir", "\0")] {
let server = start_mock_server().await;
let mut builder = test_codex()
.with_pre_build_hook(|home| {
write_pre_tool_use_hook(home, Some("^Bash$"), "allow", "")
.expect("write recording hook");
})
.with_config(trust_discovered_hooks);
let test = builder.build_with_auto_env(&server).await?;
let args = json!({ "cmd": "git version", field: value });
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(field, "exec_command", &args.to_string()),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "done"),
ev_completed("resp-2"),
]),
],
)
.await;
test.submit_turn("try the invalid shell arguments").await?;
assert!(
!test
.codex_home_path()
.join("pre_tool_use_hook_log.jsonl")
.exists()
);
let output = responses.requests()[1].function_call_output(field);
let output = output["output"].as_str().context("tool failure")?;
assert!(!output.is_empty());
logs_assert(|lines: &[&str]| {
lines
.iter()
.any(|line| {
line.contains("codex.tool_result")
&& line.contains(&format!("call_id={field}"))
&& line.contains("success=false")
})
.then_some(())
.ok_or_else(|| format!("missing failure telemetry for {field}"))
});
}
Ok(())
}
codex-rs/hooks/src/engine/mod_tests.rs
+9
View File
@@ -646,6 +646,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
turn_id: "turn-1".to_string(),
subagent: None,
cwd: cwd.clone(),
tool_cwd: None,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -666,6 +667,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
turn_id: "turn-1".to_string(),
subagent: None,
cwd,
tool_cwd: None,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -740,6 +742,7 @@ async fn requirements_managed_hooks_execute_windows_command_override() {
turn_id: "turn-1".to_string(),
subagent: None,
cwd: cwd(),
tool_cwd: None,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -1142,6 +1145,7 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() {
turn_id: "turn-1".to_string(),
subagent: None,
cwd,
tool_cwd: None,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -1575,6 +1579,7 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
turn_id: "turn-1".to_string(),
subagent: None,
cwd,
tool_cwd: None,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -1672,6 +1677,7 @@ fn profile_user_layers_load_shared_hooks_json_once() {
turn_id: "turn-1".to_string(),
subagent: None,
cwd: cwd(),
tool_cwd: None,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -2112,6 +2118,7 @@ print(json.dumps({
turn_id: "turn-1".to_string(),
subagent: None,
cwd: cwd(),
tool_cwd: None,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -2147,6 +2154,7 @@ print(json.dumps({
turn_id: "turn-1".to_string(),
subagent: None,
cwd: cwd(),
tool_cwd: None,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -2853,6 +2861,7 @@ async fn mcp_tool_hooks_expand_event_input_and_apply_pre_tool_decisions() {
turn_id: "turn-1".to_string(),
subagent: None,
cwd: cwd(),
tool_cwd: None,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
codex-rs/hooks/src/events/pre_tool_use.rs
+28 -2
View File
@@ -8,6 +8,7 @@ use codex_protocol::protocol::HookOutputEntryKind;
use codex_protocol::protocol::HookRunStatus;
use codex_protocol::protocol::HookRunSummary;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use serde_json::Value;
use super::common;
@@ -26,6 +27,9 @@ pub struct PreToolUseRequest {
pub turn_id: String,
pub subagent: Option<common::SubagentHookContext>,
pub cwd: AbsolutePathBuf,
/// Local tool cwd for hook stdin. Remote commands leave this unset to retain
/// the host cwd. The hook process still runs in `cwd`.
pub tool_cwd: Option<PathUri>,
pub transcript_path: Option<PathBuf>,
pub model: String,
pub permission_mode: String,
@@ -171,7 +175,9 @@ fn latest_updated_input(
/// Handler selection may include internal matcher aliases, but hook stdin keeps
/// the canonical `tool_name` so audit logs and downstream policy decisions stay
/// stable. Shell-like tools pass `{ "command": ... }` as `tool_input`; MCP
/// tools pass their resolved JSON arguments.
/// tools pass their resolved JSON arguments. Local commands can expose their
/// execution cwd here, independently of the directory used to launch hooks.
/// Remote commands retain the host cwd; remote cwd reporting is deferred.
fn command_input(request: &PreToolUseRequest) -> Result<Value, serde_json::Error> {
let subagent = SubagentCommandInputFields::from(request.subagent.as_ref());
serde_json::to_value(PreToolUseCommandInput {
@@ -180,7 +186,10 @@ fn command_input(request: &PreToolUseRequest) -> Result<Value, serde_json::Error
agent_id: subagent.agent_id,
agent_type: subagent.agent_type,
transcript_path: crate::schema::NullableString::from_path(request.transcript_path.clone()),
cwd: request.cwd.display().to_string(),
cwd: request.tool_cwd.as_ref().map_or_else(
|| request.cwd.display().to_string(),
PathUri::inferred_native_path_string,
),
hook_event_name: "PreToolUse".to_string(),
model: request.model.clone(),
permission_mode: request.permission_mode.clone(),
@@ -352,6 +361,22 @@ mod tests {
assert_eq!(input["tool_name"], "apply_patch");
}
#[test]
fn command_input_uses_tool_cwd_with_its_own_path_convention() {
for (uri, expected) in [
("file:///repo-B", "/repo-B"),
("file:///C:/repo-B", r"C:\repo-B"),
] {
let mut request = request_for_tool_use("call-cwd");
let original = command_input(&request).unwrap();
request.tool_cwd = Some(codex_utils_path_uri::PathUri::parse(uri).unwrap());
let actual = command_input(&request).unwrap();
let mut expected_input = original;
expected_input["cwd"] = serde_json::json!(expected);
assert_eq!(actual, expected_input);
}
}
#[test]
fn permission_decision_deny_blocks_processing() {
let parsed = parse_completed(
@@ -809,6 +834,7 @@ mod tests {
turn_id: "turn-1".to_string(),
subagent: None,
cwd: test_path_buf("/tmp").abs(),
tool_cwd: None,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),