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

Preserve command cwd in permission and post-tool hooks (#57203)

Browse Source 
## Why

`PermissionRequest` and `PostToolUse` hook inputs could report the turn's working directory instead of the local command's execution directory. This also affected commands whose results were collected in a later turn after the working directory changed.

## What changed

- Report the local command's execution directory in hook input `cwd`, consistent with `PreToolUse`.
- Preserve the original directory through `write_stdin` approvals and delayed completion, including network permission requests.
- Retain the host directory for remote commands and keep hook process working directories unchanged.

## Testing

Add coverage for relative and absolute workdirs, approval denials, command rewrites, completion and stdin approval across turns, remote command fallback, network approvals, and Unix and Windows path formatting.

GitOrigin-RevId: ef5dad2a0634dda9edb2f841ef4fe49ae0d1187d
This commit is contained in:
Andrei Eternal
2026-10-26 19:21:08 +00:00
committed by copyberry
parent 536d1f5b68
commit beb34ce8d0
26 changed files with 598 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/hook_runtime.rs
+10 -13
View File
@@ -71,6 +71,7 @@ use crate::session::step_context::StepContext;
use crate::session::turn_context::TurnContext;
use crate::state::TurnState;
use crate::tools::hook_names::HookToolName;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::sandboxing::PermissionRequestPayload;
use crate::turn_metadata::ExecutionMetadata;
@@ -281,6 +282,7 @@ pub(crate) async fn run_permission_request_hooks(
turn_id: turn_context.sub_id.clone(),
subagent: thread_spawn_subagent_hook_context(sess, turn_context),
cwd: tool_hook_cwd(review_context.environments(), turn_context).to_path_buf(),
tool_cwd: payload.tool_cwd,
transcript_path: sess.hook_transcript_path().await,
model: review_context.model_info.slug.clone(),
permission_mode: hook_permission_mode(review_context.approval_policy),
@@ -309,19 +311,13 @@ pub(crate) async fn run_permission_request_hooks(
/// Runs matching `PostToolUse` hooks after a tool has produced a successful output.
///
/// The `tool_name`, matcher aliases, `tool_input`, and `tool_response` values are
/// already adapted by the tool handler into the stable hook contract. Passing
/// The payload is already adapted by the tool handler into the stable hook contract. Passing
/// raw internal tool data here would leak implementation details into user hook
/// matchers and hook logs.
#[allow(clippy::too_many_arguments)]
pub(crate) async fn run_post_tool_use_hooks(
sess: &Arc<Session>,
step_context: &StepContext,
tool_use_id: String,
tool_name: String,
matcher_aliases: Vec<String>,
tool_input: Value,
tool_response: Value,
payload: PostToolUsePayload,
target_environments: &[&str],
) -> PostToolUseOutcome {
let turn_context = &step_context.turn;
@@ -330,14 +326,15 @@ pub(crate) async fn run_post_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: payload.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()),
tool_name,
matcher_aliases,
tool_use_id,
tool_input,
tool_response,
tool_name: payload.tool_name.name().to_string(),
matcher_aliases: payload.tool_name.matcher_aliases().to_vec(),
tool_use_id: payload.tool_use_id,
tool_input: payload.tool_input,
tool_response: payload.tool_response,
};
let hooks = with_tool_environment_hooks(
sess,
codex-rs/core/src/tools/approvals.rs
+29 -2
View File
@@ -166,12 +166,17 @@ impl ApprovalAction {
}
pub(crate) fn permission_request_payload(&self) -> PermissionRequestPayload {
match self {
let mut payload = match self {
Self::ExecCommand {
hook_command,
justification,
cwd,
..
} => PermissionRequestPayload::bash(hook_command.clone(), justification.clone()),
} => PermissionRequestPayload::bash(
hook_command.clone(),
justification.clone(),
cwd.clone(),
),
Self::WriteStdin {
id,
approval_id,
@@ -185,6 +190,7 @@ impl ApprovalAction {
additional_permissions_preapproved: _,
} => PermissionRequestPayload {
tool_name: HookToolName::new("write_stdin"),
tool_cwd: Some(cwd.clone()),
tool_input: serde_json::json!({
"session_id": process_id,
"chars": input,
@@ -199,6 +205,7 @@ impl ApprovalAction {
},
Self::ApplyPatch { patch, .. } => PermissionRequestPayload {
tool_name: HookToolName::apply_patch(),
tool_cwd: None,
tool_input: serde_json::json!({ "command": patch }),
},
Self::McpToolCall {
@@ -207,6 +214,7 @@ impl ApprovalAction {
..
} => PermissionRequestPayload {
tool_name: hook_tool_name.clone(),
tool_cwd: None,
tool_input: arguments
.clone()
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())),
@@ -214,10 +222,16 @@ impl ApprovalAction {
Self::NetworkAccess {
hook_command,
target,
cwd,
trigger,
..
} => PermissionRequestPayload::bash(
hook_command.clone(),
Some(format!("network-access {target}")),
trigger.as_ref().map_or_else(
|| PathUri::from_abs_path(cwd),
|trigger| trigger.cwd.clone(),
),
),
Self::RequestPermissions {
reason,
@@ -225,12 +239,25 @@ impl ApprovalAction {
..
} => PermissionRequestPayload {
tool_name: HookToolName::new("request_permissions"),
tool_cwd: None,
tool_input: serde_json::json!({
"reason": reason,
"permissions": permissions,
}),
},
};
// Command hooks run on the host. Keep remote paths in the action's
// original tool input, but never use them as the hook's top-level cwd.
if matches!(
self,
Self::ExecCommand { environment_id, .. }
| Self::WriteStdin { environment_id, .. }
| Self::NetworkAccess { environment_id, .. }
if environment_id != codex_exec_server::LOCAL_ENVIRONMENT_ID
) {
payload.tool_cwd = None;
}
payload
}
pub(crate) fn cache_keys(&self) -> Vec<ApprovalCacheKey> {
codex-rs/core/src/tools/approvals_tests.rs
+55
View File
@@ -243,3 +243,58 @@ async fn explicit_mcp_reviewer_override_takes_precedence_over_action_context() {
}
}
}
#[test]
fn network_hook_cwd_overrides_only_local_commands() {
let local_cwd = AbsolutePathBuf::try_from(std::env::temp_dir()).unwrap();
for (environment_id, uri) in [
(codex_exec_server::LOCAL_ENVIRONMENT_ID, None),
(
codex_exec_server::LOCAL_ENVIRONMENT_ID,
Some("file:///local/repo"),
),
("remote", None),
("remote", Some("file:///remote/repo")),
("remote", Some("file:///C:/remote/repo")),
] {
let expected_cwd = uri.map_or_else(
|| PathUri::from_abs_path(&local_cwd),
|uri| PathUri::parse(uri).unwrap(),
);
let action = ApprovalAction::NetworkAccess {
id: "network-1".into(),
turn_id: "turn-1".into(),
environment_id: environment_id.into(),
target: "https://example.com:443".into(),
host: "example.com".into(),
protocol: NetworkApprovalProtocol::Https,
port: 443,
trigger: uri.map(|_| GuardianNetworkAccessTrigger {
call_id: "exec-1".into(),
tool_name: "exec_command".into(),
command: vec!["curl".into(), "https://example.com".into()],
cwd: expected_cwd.clone(),
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
justification: None,
tty: None,
}),
hook_command: "curl https://example.com".into(),
hook_run_id: "hook-1".into(),
command: vec!["network-access".into(), "https://example.com:443".into()],
cwd: local_cwd.clone(),
};
assert_eq!(
action.permission_request_payload(),
PermissionRequestPayload {
tool_name: HookToolName::bash(),
tool_input: serde_json::json!({
"command": "curl https://example.com",
"description": "network-access https://example.com:443",
}),
tool_cwd: (environment_id == codex_exec_server::LOCAL_ENVIRONMENT_ID)
.then_some(expected_cwd),
}
);
}
}
codex-rs/core/src/tools/context.rs
+8
View File
@@ -20,6 +20,7 @@ use codex_utils_audio::estimate_audio_token_count;
use codex_utils_output_truncation::TruncationPolicy;
use codex_utils_output_truncation::truncate_function_output_payload;
use codex_utils_output_truncation::with_serialization_allowance;
use codex_utils_path_uri::PathUri;
use serde::Serialize;
use serde_json::Value as JsonValue;
use std::sync::Arc;
@@ -429,6 +430,9 @@ pub struct ExecCommandToolOutput {
pub exit_code: Option<i32>,
pub original_token_count: Option<usize>,
pub hook_command: Option<String>,
/// Local execution cwd retained for PostToolUse, independent of the collecting
/// turn. Remote commands leave this unset to retain the host hook cwd.
pub tool_cwd: Option<PathUri>,
}
impl ToolOutput for ExecCommandToolOutput {
@@ -469,6 +473,10 @@ impl ToolOutput for ExecCommandToolOutput {
.map(|command| serde_json::json!({ "command": command }))
}
fn post_tool_use_cwd(&self) -> Option<PathUri> {
self.tool_cwd.clone()
}
fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option<JsonValue> {
if self.process_id.is_some() || self.hook_command.is_none() {
return None;
codex-rs/core/src/tools/context_tests.rs
+3
View File
@@ -425,6 +425,7 @@ fn exec_command_tool_output_formats_truncated_response() {
exit_code: Some(0),
original_token_count: Some(10),
hook_command: None,
tool_cwd: None,
};
assert_eq!(
output.log_output(),
@@ -480,6 +481,7 @@ fn exec_command_tool_output_reserves_metadata_budget_and_preserves_policy_units(
exit_code: Some(0),
original_token_count: Some(123),
hook_command: None,
tool_cwd: None,
}
.to_response_item("call-42", &payload);
@@ -525,6 +527,7 @@ fn exec_command_tool_output_preserves_omission_metadata_when_truncated() {
exit_code: Some(0),
original_token_count: Some(42_000),
hook_command: None,
tool_cwd: None,
};
let expected_header = "Chunk ID: abc123\nWall time: 1.2500 seconds\nProcess exited with code 0\nOriginal token count: 42000\nOutput:\n";
assert_eq!(
codex-rs/core/src/tools/handlers/apply_patch.rs
+1
View File
@@ -449,6 +449,7 @@ impl CoreToolRuntime for ApplyPatchHandler {
let tool_response =
result.post_tool_use_response(&invocation.call_id, &invocation.payload)?;
Some(PostToolUsePayload {
tool_cwd: result.post_tool_use_cwd(),
tool_name: HookToolName::apply_patch(),
tool_use_id: invocation.call_id.clone(),
tool_input: serde_json::json!({
codex-rs/core/src/tools/handlers/apply_patch_tests.rs
+1
View File
@@ -90,6 +90,7 @@ async fn post_tool_use_payload_uses_patch_input_and_tool_output() {
assert_eq!(
handler.post_tool_use_payload(&invocation, &output),
Some(PostToolUsePayload {
tool_cwd: None,
tool_name: HookToolName::apply_patch(),
tool_use_id: "call-apply-patch".to_string(),
tool_input: json!({ "command": patch }),
codex-rs/core/src/tools/handlers/extension_tools.rs
+1
View File
@@ -449,6 +449,7 @@ mod tests {
assert_eq!(
CoreToolRuntime::post_tool_use_payload(&handler, &invocation, &output),
Some(PostToolUsePayload {
tool_cwd: None,
tool_name: HookToolName::new("extension_echo"),
tool_use_id: "call-extension".to_string(),
tool_input: json!({ "message": "hello" }),
codex-rs/core/src/tools/handlers/mcp.rs
+2
View File
@@ -545,6 +545,7 @@ impl CoreToolRuntime for McpHandler {
let tool_response =
result.post_tool_use_response(&invocation.call_id, &invocation.payload)?;
Some(PostToolUsePayload {
tool_cwd: result.post_tool_use_cwd(),
tool_name: self.hook_tool_name(),
tool_use_id: invocation.call_id.clone(),
tool_input: result.post_tool_use_input(&invocation.payload)?,
@@ -834,6 +835,7 @@ mod tests {
assert_eq!(
handler.post_tool_use_payload(&invocation, &output),
Some(PostToolUsePayload {
tool_cwd: None,
tool_name: HookToolName::new("mcp__filesystem__read_file"),
tool_use_id: "call-mcp-post".to_string(),
tool_input: json!({
codex-rs/core/src/tools/handlers/unified_exec.rs
+1
View File
@@ -139,6 +139,7 @@ fn post_unified_exec_tool_use_payload(
let tool_use_id = result.post_tool_use_id(&invocation.call_id);
let tool_response = result.post_tool_use_response(&tool_use_id, &invocation.payload)?;
Some(PostToolUsePayload {
tool_cwd: result.post_tool_use_cwd(),
tool_name: HookToolName::bash(),
tool_use_id,
tool_input,
codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs
+3 -1
View File
@@ -377,6 +377,7 @@ impl ExecCommandHandler {
exit_code: None,
original_token_count: None,
hook_command: None,
tool_cwd: None,
}));
}
@@ -399,7 +400,7 @@ impl ExecCommandHandler {
process_id,
yield_time_ms,
max_output_tokens,
cwd,
cwd: cwd.clone(),
sandbox_cwd: turn_environment.cwd().clone(),
turn_environment: turn_environment.clone(),
network: context.step_context.turn.network.clone(),
@@ -445,6 +446,7 @@ impl ExecCommandHandler {
exit_code: Some(output.exit_code),
original_token_count: Some(original_token_count),
hook_command: Some(hook_command),
tool_cwd: (!turn_environment.environment.is_remote()).then_some(cwd),
}))
}
Err(err) => {
codex-rs/core/src/tools/handlers/unified_exec_tests.rs
+15
View File
@@ -604,12 +604,14 @@ async fn exec_command_post_tool_use_payload_uses_output_for_noninteractive_one_s
exit_code: Some(0),
original_token_count: None,
hook_command: Some("echo three".to_string()),
tool_cwd: Some(PathUri::parse("file:///C:/workspace").expect("valid Windows cwd")),
};
let invocation = invocation_for_payload("exec_command", "call-43", payload).await;
let handler = ExecCommandHandler::default();
assert_eq!(
handler.post_tool_use_payload(&invocation, &output),
Some(crate::tools::registry::PostToolUsePayload {
tool_cwd: Some(PathUri::parse("file:///C:/workspace").expect("valid Windows cwd")),
tool_name: HookToolName::bash(),
tool_use_id: "call-43".to_string(),
tool_input: serde_json::json!({ "command": "echo three" }),
@@ -634,6 +636,7 @@ async fn exec_command_post_tool_use_payload_uses_output_for_interactive_completi
exit_code: Some(0),
original_token_count: None,
hook_command: Some("echo three".to_string()),
tool_cwd: None,
};
let invocation = invocation_for_payload("exec_command", "call-44", payload).await;
let handler = ExecCommandHandler::default();
@@ -641,6 +644,7 @@ async fn exec_command_post_tool_use_payload_uses_output_for_interactive_completi
assert_eq!(
handler.post_tool_use_payload(&invocation, &output),
Some(crate::tools::registry::PostToolUsePayload {
tool_cwd: None,
tool_name: HookToolName::bash(),
tool_use_id: "call-44".to_string(),
tool_input: serde_json::json!({ "command": "echo three" }),
@@ -665,6 +669,7 @@ async fn exec_command_post_tool_use_payload_skips_running_sessions() {
exit_code: None,
original_token_count: None,
hook_command: Some("echo three".to_string()),
tool_cwd: None,
};
let invocation = invocation_for_payload("exec_command", "call-45", payload).await;
let handler = ExecCommandHandler::default();
@@ -691,6 +696,7 @@ async fn write_stdin_post_tool_use_payload_uses_original_exec_call_id_and_comman
exit_code: Some(0),
original_token_count: None,
hook_command: Some("sleep 1; echo finished".to_string()),
tool_cwd: None,
};
let invocation = invocation_for_payload("write_stdin", "write-stdin-call", payload).await;
let handler = WriteStdinHandler;
@@ -698,6 +704,7 @@ async fn write_stdin_post_tool_use_payload_uses_original_exec_call_id_and_comman
assert_eq!(
handler.post_tool_use_payload(&invocation, &output),
Some(crate::tools::registry::PostToolUsePayload {
tool_cwd: None,
tool_name: HookToolName::bash(),
tool_use_id: "exec-call-45".to_string(),
tool_input: serde_json::json!({ "command": "sleep 1; echo finished" }),
@@ -722,6 +729,7 @@ async fn write_stdin_post_tool_use_payload_keeps_parallel_session_metadata_separ
exit_code: Some(0),
original_token_count: None,
hook_command: Some("sleep 2; echo alpha".to_string()),
tool_cwd: Some(PathUri::parse("file:///C:/workspace/alpha").expect("valid Windows cwd")),
};
let output_b = ExecCommandToolOutput {
event_call_id: "exec-call-b".to_string(),
@@ -734,6 +742,7 @@ async fn write_stdin_post_tool_use_payload_keeps_parallel_session_metadata_separ
exit_code: Some(0),
original_token_count: None,
hook_command: Some("sleep 1; echo beta".to_string()),
tool_cwd: Some(PathUri::parse("file:///C:/workspace/beta").expect("valid Windows cwd")),
};
let invocation_b = invocation_for_payload("write_stdin", "write-call-b", payload.clone()).await;
let invocation_a = invocation_for_payload("write_stdin", "write-call-a", payload).await;
@@ -748,12 +757,18 @@ async fn write_stdin_post_tool_use_payload_keeps_parallel_session_metadata_separ
payloads,
[
Some(crate::tools::registry::PostToolUsePayload {
tool_cwd: Some(
PathUri::parse("file:///C:/workspace/beta").expect("valid Windows cwd")
),
tool_name: HookToolName::bash(),
tool_use_id: "exec-call-b".to_string(),
tool_input: serde_json::json!({ "command": "sleep 1; echo beta" }),
tool_response: serde_json::json!("beta\n"),
}),
Some(crate::tools::registry::PostToolUsePayload {
tool_cwd: Some(
PathUri::parse("file:///C:/workspace/alpha").expect("valid Windows cwd")
),
tool_name: HookToolName::bash(),
tool_use_id: "exec-call-a".to_string(),
tool_input: serde_json::json!({ "command": "sleep 2; echo alpha" }),
codex-rs/core/src/tools/registry.rs
+4 -5
View File
@@ -141,6 +141,7 @@ pub(crate) trait CoreToolRuntime: ToolExecutor<ToolInvocation> {
};
Some(PostToolUsePayload {
tool_cwd: result.post_tool_use_cwd(),
tool_name: function_hook_tool_name(invocation),
tool_use_id: result.post_tool_use_id(&invocation.call_id),
tool_input: result
@@ -375,6 +376,8 @@ pub(crate) struct PreToolUsePayload {
#[derive(Debug, Clone, PartialEq)]
pub(crate) struct PostToolUsePayload {
/// Local execution cwd used by hook input; remote tools retain the host fallback.
pub(crate) tool_cwd: Option<PathUri>,
/// Hook-facing tool name model.
///
/// The canonical name is serialized to hook stdin, while aliases are used
@@ -934,11 +937,7 @@ impl ToolRegistry {
run_post_tool_use_hooks(
&invocation.session,
invocation.step_context.as_ref(),
post_tool_use_payload.tool_use_id,
post_tool_use_payload.tool_name.name().to_string(),
post_tool_use_payload.tool_name.matcher_aliases().to_vec(),
post_tool_use_payload.tool_input,
post_tool_use_payload.tool_response,
post_tool_use_payload,
&invocation.environments.hook_ids(),
)
.or_cancel_tool(&cancellation_token)
codex-rs/core/src/tools/registry_tests.rs
+1
View File
@@ -470,6 +470,7 @@ async fn function_tools_expose_default_hook_payloads_and_rewrites() -> anyhow::R
assert_eq!(
handler.post_tool_use_payload(&invocation, &output),
Some(PostToolUsePayload {
tool_cwd: None,
tool_name: HookToolName::new("functions.echo"),
tool_use_id: "call-1".to_string(),
tool_input: serde_json::json!({ "message": "hello" }),
codex-rs/core/src/tools/sandboxing.rs
+3 -1
View File
@@ -129,10 +129,11 @@ pub(crate) use super::approvals::ApprovalAction;
pub(crate) struct PermissionRequestPayload {
pub tool_name: HookToolName,
pub tool_input: serde_json::Value,
pub tool_cwd: Option<PathUri>,
}
impl PermissionRequestPayload {
pub(crate) fn bash(command: String, description: Option<String>) -> Self {
pub(crate) fn bash(command: String, description: Option<String>, cwd: PathUri) -> Self {
let mut tool_input = serde_json::Map::new();
tool_input.insert("command".to_string(), serde_json::Value::String(command));
if let Some(description) = description {
@@ -145,6 +146,7 @@ impl PermissionRequestPayload {
Self {
tool_name: HookToolName::bash(),
tool_input: serde_json::Value::Object(tool_input),
tool_cwd: Some(cwd),
}
}
}
codex-rs/core/src/tools/sandboxing_tests.rs
+8 -1
View File
@@ -21,9 +21,14 @@ use std::collections::HashMap;
#[test]
fn bash_permission_request_payload_omits_missing_description() {
assert_eq!(
PermissionRequestPayload::bash("echo hi".to_string(), /*description*/ None),
PermissionRequestPayload::bash(
"echo hi".to_string(),
/*description*/ None,
PathUri::parse("file:///repo-B").unwrap()
),
PermissionRequestPayload {
tool_name: HookToolName::bash(),
tool_cwd: Some(PathUri::parse("file:///repo-B").unwrap()),
tool_input: json!({ "command": "echo hi" }),
}
);
@@ -35,9 +40,11 @@ fn bash_permission_request_payload_includes_description_when_present() {
PermissionRequestPayload::bash(
"echo hi".to_string(),
Some("network-access example.com".to_string()),
PathUri::parse("file:///repo-B").unwrap(),
),
PermissionRequestPayload {
tool_name: HookToolName::bash(),
tool_cwd: Some(PathUri::parse("file:///repo-B").unwrap()),
tool_input: json!({
"command": "echo hi",
"description": "network-access example.com",
codex-rs/core/src/tools/user_messaging_tests.rs
+1
View File
@@ -47,6 +47,7 @@ fn messaging_result(name: &str, input: Value, success: bool) -> AnyToolResult {
Some(success),
)),
post_tool_use_payload: Some(PostToolUsePayload {
tool_cwd: None,
tool_name: HookToolName::new(name),
tool_use_id: "message-call".to_owned(),
tool_input: input,
codex-rs/core/src/unified_exec/mod_tests.rs
+1
View File
@@ -212,6 +212,7 @@ async fn exec_command_with_tty(
exit_code,
original_token_count: Some(original_token_count),
hook_command: Some(cmd.to_string()),
tool_cwd: None,
})
}
codex-rs/core/src/unified_exec/process_manager.rs
+9
View File
@@ -266,6 +266,7 @@ struct PreparedProcessHandles {
network_approval: Option<DeferredNetworkApproval>,
call_id: String,
hook_command: String,
tool_cwd: Option<PathUri>,
process_id: i32,
tty: bool,
}
@@ -874,6 +875,8 @@ impl UnifiedExecProcessManager {
exit_code,
original_token_count: Some(original_token_count),
hook_command: Some(request.hook_command.clone()),
tool_cwd: (!request.turn_environment.environment.is_remote())
.then(|| request.cwd.clone()),
};
Ok(response)
@@ -996,6 +999,7 @@ impl UnifiedExecProcessManager {
network_approval,
call_id,
hook_command,
tool_cwd,
process_id,
tty,
..
@@ -1126,6 +1130,7 @@ impl UnifiedExecProcessManager {
exit_code,
original_token_count: Some(original_token_count),
hook_command: Some(hook_command),
tool_cwd,
};
let should_emit_interaction = !request.input.is_empty() || response.process_id.is_some();
@@ -1230,6 +1235,10 @@ impl UnifiedExecProcessManager {
network_approval: entry.network_approval.clone(),
call_id: entry.call_id.clone(),
hook_command: entry.hook_command.clone(),
// Capture the original process's locality before it can be removed
// from the store. A later turn may select a different environment.
tool_cwd: (entry.environment_id == codex_exec_server::LOCAL_ENVIRONMENT_ID)
.then(|| entry.cwd.clone()),
process_id: entry.process_id,
tty: entry.tty,
})
codex-rs/core/tests/suite/hooks.rs
+22 -2
View File
@@ -3441,7 +3441,7 @@ mode = "limited"
allow_local_binding = true
"#,
)?;
let args = serde_json::json!({ "cmd": command });
let args = serde_json::json!({ "cmd": command, "workdir": "network-workdir" });
let responses = mount_sse_sequence(
&server,
vec![
@@ -3472,6 +3472,14 @@ allow_local_binding = true
hook_reason,
)
.expect("failed to write permission request hook test fixture");
let script_path = home.join("permission_request_hook.py");
let script = fs::read_to_string(&script_path)
.expect("read permission request 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 network hook process cwd");
})
.with_cloud_config_bundle(
CloudConfigBundleFixture::loader_with_enterprise_requirement(
@@ -3495,6 +3503,8 @@ review_domains = ["codex-network-test.invalid"]
})
.build(&server)
.await?;
let execution_cwd = test.workspace_path_uri("network-workdir")?;
fs::create_dir(execution_cwd.to_abs_path()?).context("create network command workdir")?;
test.submit_turn_with_approval_and_permission_profile(
"run the shell command after the network permission hook",
@@ -3531,11 +3541,21 @@ review_domains = ["codex-network-test.invalid"]
);
}
assert_single_permission_request_hook_input(
let hook_inputs = assert_single_permission_request_hook_input(
test.codex_home_path(),
command,
Some("network-access http://codex-network-test.invalid:80"),
)?;
assert_eq!(
json!({
"cwd": hook_inputs[0]["cwd"],
"process_cwd": hook_inputs[0]["process_cwd"],
}),
json!({
"cwd": execution_cwd.inferred_native_path_string(),
"process_cwd": dunce::canonicalize(&test.config.cwd)?.to_string_lossy(),
}),
);
let requests = responses.requests();
assert_eq!(
requests
codex-rs/core/tests/suite/hooks_cwd_tests.rs
+339 -1
View File
@@ -1,6 +1,6 @@
#![cfg(test)]
//! PreToolUse reports local command cwd and retains host cwd for remote commands.
//! Tool hooks report local command cwd and retain host cwd for remote commands.
use super::*;
use codex_exec_server::CreateDirectoryOptions;
@@ -213,3 +213,341 @@ async fn invalid_execution_cwd_records_failure_without_running_hooks() -> Result
}
Ok(())
}
#[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"), "deny"; "permission_denied")]
#[test_case(Some("nested"), "rewrite"; "rewritten")]
#[test_case(Some("nested"), "delayed"; "completion_in_another_turn")]
#[test_case(Some("nested"), "stdin"; "stdin_approval_in_another_turn")]
#[tokio::test]
async fn command_hooks_keep_execution_cwd(workdir: Option<&str>, mode: &'static str) -> Result<()> {
use codex_protocol::protocol::TurnEnvironmentRequests;
skip_if_no_network!(Ok(()));
if mode == "stdin" {
core_test_support::skip_if_target_windows!(Ok(()), "uses a POSIX read/tty fixture");
}
let later_turn = matches!(mode, "delayed" | "stdin");
let server = start_mock_server().await;
let test = test_codex()
.with_pre_build_hook(move |home| {
let script = format!(
r#"import json
from pathlib import Path
import sys
payload = json.load(sys.stdin)
payload['process_cwd'] = str(Path.cwd())
with Path({log_path:?}).open('a', encoding='utf-8') as log:
log.write(json.dumps(payload) + '\n')
event = payload['hook_event_name']
if event == 'PermissionRequest':
print(json.dumps({{'hookSpecificOutput': {{'hookEventName': event, 'decision': {{
'behavior': 'deny' if {mode:?} == 'deny' else 'allow', 'message': 'cwd test denied'
}}}}}}))
elif event == 'PreToolUse' and {mode:?} == 'rewrite':
print(json.dumps({{'hookSpecificOutput': {{'hookEventName': event,
'permissionDecision': 'allow',
'updatedInput': {{'command': 'cat rewritten-marker.txt'}}
}}}}))
"#,
log_path = home.join("tool_cwd_hook_log.jsonl").to_string_lossy()
);
let script_path = home.join("tool_cwd_hook.py");
fs::write(&script_path, script).expect("write hook recorder");
let group = json!([{"matcher": "^(Bash|write_stdin)$", "hooks": [{
"type": "command", "command": format!("python3 {}", script_path.display()),
}]}]);
fs::write(
home.join("hooks.json"),
json!({"hooks": {
"PreToolUse": group, "PermissionRequest": group, "PostToolUse": group,
}})
.to_string(),
)
.expect("write hook configuration");
})
.with_config(move |config| {
trust_discovered_hooks(config);
if mode == "stdin" {
config
.features
.enable(Feature::WriteStdinApproval)
.expect("enable stdin approval");
}
})
.build_with_remote_and_local_env(&server)
.await?;
// Remote command paths never replace the host cwd in hook input.
let local_hook_directory = TempDir::new()?;
let mut environments = vec![test.executor_environment().request()];
let hook_cwd = if test.executor_environment().environment().is_remote() {
let cwd = AbsolutePathBuf::try_from(local_hook_directory.path())?;
environments.push(core_test_support::test_codex::local_request(cwd.clone()));
cwd
} else {
test.config.cwd.clone()
};
let root = test.executor_environment().selection().cwd.clone();
let nested = root.join("nested")?;
let other = root.join("other")?;
for dir in [&nested, &other] {
test.fs()
.create_directory(
dir,
CreateDirectoryOptions {
recursive: true,
follow_symlinks: true,
},
/*sandbox*/ None,
)
.await?;
}
for (dir, marker) in [(&root, "root"), (&nested, "nested")] {
for filename in ["cwd-marker.txt", "rewritten-marker.txt"] {
test.fs()
.write_file(
&dir.join(filename)?,
format!("{marker}:{filename}").into_bytes(),
Default::default(),
/*sandbox*/ None,
)
.await?;
}
}
let command = if mode == "stdin" {
"read -r line; cat cwd-marker.txt"
} else if mode == "delayed" {
match test_target_os() {
TestTargetOs::Windows => {
"while (!(Test-Path release.txt)) { Start-Sleep -Milliseconds 50 }; Get-Content cwd-marker.txt"
}
TestTargetOs::Linux | TestTargetOs::MacOs => {
"while [ ! -f release.txt ]; do sleep 0.05; done; cat cwd-marker.txt"
}
}
} else {
"cat cwd-marker.txt"
};
// A cold remote shell can take more than a second to start, especially under Wine.
// Only the cross-turn cases intentionally return before the command completes.
let yield_time_ms = if later_turn { 1000 } else { 30000 };
let mut args = json!({"cmd": command, "login": false, "yield_time_ms": yield_time_ms,
"sandbox_permissions": "require_escalated"});
if mode == "stdin" {
args["tty"] = json!(true);
}
if let Some(workdir) = workdir {
args["workdir"] = if workdir == "absolute" {
json!(nested.inferred_native_path_string())
} else {
json!(workdir)
};
}
let call_id = "command-cwd";
let mut response_events = 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"),
]),
];
if later_turn {
response_events.extend([
sse(vec![
ev_response_created("resp-3"),
ev_function_call(
"poll-cwd",
"write_stdin",
&json!({
"session_id": 1000, "chars": if mode == "stdin" { "go\n" } else { "" }, "yield_time_ms": 5000,
})
.to_string(),
),
ev_completed("resp-3"),
]),
sse(vec![
ev_response_created("resp-4"),
ev_assistant_message("msg-2", "collected"),
ev_completed("resp-4"),
]),
]);
}
let responses = mount_sse_sequence(&server, response_events).await;
let (sandbox_policy, permission_profile) =
turn_permission_fields(PermissionProfile::read_only(), test.config.cwd.as_path());
let turn_settings = ThreadSettingsOverrides {
environments: Some(TurnEnvironmentRequests::new(
test.config.cwd.clone(),
environments.clone(),
)),
approval_policy: Some(AskForApproval::OnRequest),
sandbox_policy: Some(sandbox_policy),
permission_profile,
..Default::default()
};
test.codex
.start_or_steer_turn(
TurnInputRequest::user_input(vec![UserInput::Text {
text: "run the command".into(),
text_elements: Vec::new(),
}])
.with_thread_settings(turn_settings.clone()),
)
.await?;
wait_for_event(&test.codex, |event| {
assert!(
!matches!(event, EventMsg::ExecApprovalRequest(_)),
"hook should decide approval"
);
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let log_path = test.codex_home_path().join("tool_cwd_hook_log.jsonl");
let expected_cwd = if workdir.is_none_or(str::is_empty) {
&root
} else {
&nested
};
let mut post_process_cwd = dunce::canonicalize(&hook_cwd)?
.to_string_lossy()
.into_owned();
let expected_hook_cwd = if test.executor_environment().environment().is_remote() {
post_process_cwd.clone()
} else {
expected_cwd.inferred_native_path_string()
};
if later_turn {
let first_output = responses.requests()[1].function_call_output(call_id);
assert!(
first_output["output"]
.as_str()
.context("running command output")?
.contains("Process running with session ID 1000")
);
assert_eq!(
fs::read_to_string(&log_path)?.lines().count(),
2,
"no post hook until completion"
);
if mode == "delayed" {
test.fs()
.write_file(
&expected_cwd.join("release.txt")?,
Vec::new(),
Default::default(),
/*sandbox*/ None,
)
.await?;
}
environments[0].cwd = other.clone();
if !test.executor_environment().environment().is_remote() {
post_process_cwd = dunce::canonicalize(other.to_abs_path()?.as_path())?
.to_string_lossy()
.into_owned();
}
test.codex
.start_or_steer_turn(
TurnInputRequest::user_input(vec![UserInput::Text {
text: "collect the command result".into(),
text_elements: Vec::new(),
}])
.with_thread_settings(ThreadSettingsOverrides {
environments: Some(TurnEnvironmentRequests::new(
test.config.cwd.clone(),
environments,
)),
..turn_settings
}),
)
.await?;
wait_for_event(&test.codex, |event| {
assert!(
!matches!(event, EventMsg::ExecApprovalRequest(_)),
"hook should decide stdin approval"
);
matches!(event, EventMsg::TurnComplete(_))
})
.await;
}
let inputs: Vec<Value> = fs::read_to_string(log_path)?
.lines()
.map(serde_json::from_str)
.collect::<std::result::Result<_, _>>()?;
let executed_command = if mode == "rewrite" {
"cat rewritten-marker.txt"
} else {
command
};
let mut expected = vec![];
for (event, event_command) in [
("PreToolUse", command),
("PermissionRequest", executed_command),
("PostToolUse", executed_command),
] {
if mode == "deny" && event == "PostToolUse" {
continue;
}
if mode == "stdin" && event == "PostToolUse" {
expected.push(json!({
"event": "PermissionRequest", "tool_name": "write_stdin",
"cwd": expected_hook_cwd, "process_cwd": post_process_cwd,
"tool_input": {
"session_id": 1000, "chars": "go\n", "parent_call_id": call_id,
"approval_id": "poll-cwd", "environment_id": test.executor_environment().selection().environment_id,
"cwd": expected_cwd, "tty": true, "sandbox_permissions": "require_escalated",
"additional_permissions": null,
},
}));
}
expected.push(json!({
"event": event, "tool_name": "Bash", "cwd": expected_hook_cwd,
"process_cwd": if event == "PostToolUse" { post_process_cwd.clone() } else { dunce::canonicalize(&hook_cwd)?.to_string_lossy().into_owned() },
"tool_input": {"command": event_command},
}));
}
assert_eq!(
inputs
.iter()
.map(|input| json!({
"event": input["hook_event_name"], "tool_name": input["tool_name"], "cwd": input["cwd"],
"process_cwd": input["process_cwd"], "tool_input": input["tool_input"],
}))
.collect::<Vec<_>>(),
expected
);
let requests = responses.requests();
let output = requests
.last()
.context("final model request")?
.function_call_output(if later_turn { "poll-cwd" } else { call_id });
let output = output["output"].as_str().context("command result")?;
if mode == "deny" {
assert!(output.contains("cwd test denied"), "{output}");
} else {
let filename = if mode == "rewrite" {
"rewritten-marker.txt"
} else {
"cwd-marker.txt"
};
let marker = if workdir.is_none_or(str::is_empty) {
"root"
} else {
"nested"
};
assert!(output.contains(&format!("{marker}:{filename}")), "{output}");
assert_eq!(
inputs.last().context("completion hook input")?["tool_use_id"],
call_id
);
}
Ok(())
}
codex-rs/hooks/src/events/common.rs
+10
View File
@@ -1,9 +1,12 @@
use std::path::Path;
use codex_protocol::protocol::HookCompletedEvent;
use codex_protocol::protocol::HookEventName;
use codex_protocol::protocol::HookOutputEntry;
use codex_protocol::protocol::HookOutputEntryKind;
use codex_protocol::protocol::HookRunStatus;
use codex_protocol::protocol::HookRunSummary;
use codex_utils_path_uri::PathUri;
use crate::engine::ConfiguredHandler;
use crate::engine::HandlerSourcePath;
@@ -18,6 +21,13 @@ pub struct SubagentHookContext {
pub agent_type: String,
}
pub(crate) fn command_input_cwd(cwd: &Path, tool_cwd: Option<&PathUri>) -> String {
tool_cwd.map_or_else(
|| cwd.display().to_string(),
PathUri::inferred_native_path_string,
)
}
pub(crate) fn join_text_chunks(chunks: Vec<String>) -> Option<String> {
if chunks.is_empty() {
None
codex-rs/hooks/src/events/permission_request.rs
+36 -1
View File
@@ -30,6 +30,7 @@ use codex_protocol::protocol::HookOutputEntry;
use codex_protocol::protocol::HookOutputEntryKind;
use codex_protocol::protocol::HookRunStatus;
use codex_protocol::protocol::HookRunSummary;
use codex_utils_path_uri::PathUri;
use serde_json::Value;
#[derive(Debug, Clone)]
@@ -38,6 +39,9 @@ pub struct PermissionRequestRequest {
pub turn_id: String,
pub subagent: Option<common::SubagentHookContext>,
pub cwd: PathBuf,
/// Local approval action 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,
@@ -176,7 +180,7 @@ fn build_command_input(request: &PermissionRequestRequest) -> PermissionRequestC
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: common::command_input_cwd(request.cwd.as_ref(), request.tool_cwd.as_ref()),
hook_event_name: "PermissionRequest".to_string(),
model: request.model.clone(),
permission_mode: request.permission_mode.clone(),
@@ -304,6 +308,37 @@ mod tests {
use super::PermissionRequestDecision;
use super::resolve_permission_request_decision;
#[test]
fn command_input_uses_approval_cwd_path_convention() {
let mut request = super::PermissionRequestRequest {
session_id: codex_protocol::ThreadId::new(),
turn_id: "turn-1".into(),
subagent: None,
cwd: codex_utils_absolute_path::test_support::test_path_buf("/local"),
tool_cwd: None,
transcript_path: None,
model: "test".into(),
permission_mode: "default".into(),
tool_name: "Bash".into(),
matcher_aliases: Vec::new(),
run_id_suffix: "call-cwd".into(),
tool_input: serde_json::json!({"command": "pwd"}),
};
let original: serde_json::Value =
serde_json::to_value(super::build_command_input(&request)).unwrap();
for (uri, expected) in [
("file:///repo-B", "/repo-B"),
("file:///C:/repo-B", r"C:\repo-B"),
] {
request.tool_cwd = Some(codex_utils_path_uri::PathUri::parse(uri).unwrap());
let actual: serde_json::Value =
serde_json::to_value(super::build_command_input(&request)).unwrap();
let mut expected_input = original.clone();
expected_input["cwd"] = serde_json::json!(expected);
assert_eq!(actual, expected_input);
}
}
#[test]
fn permission_request_deny_overrides_earlier_allow() {
let decisions = [
codex-rs/hooks/src/events/post_tool_use.rs
+22 -1
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 PostToolUseRequest {
pub turn_id: String,
pub subagent: Option<common::SubagentHookContext>,
pub cwd: AbsolutePathBuf,
/// Original local execution 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,
@@ -155,7 +159,7 @@ fn command_input(request: &PostToolUseRequest) -> Result<Value, serde_json::Erro
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: common::command_input_cwd(request.cwd.as_ref(), request.tool_cwd.as_ref()),
hook_event_name: "PostToolUse".to_string(),
model: request.model.clone(),
permission_mode: request.permission_mode.clone(),
@@ -338,6 +342,22 @@ mod tests {
use crate::output_spill::AdditionalContext;
use crate::output_spill::AdditionalContextLimit;
#[test]
fn command_input_uses_execution_cwd_path_convention() {
let mut request = request_for_tool_use("call-cwd");
let original = command_input(&request).unwrap();
for (uri, expected) in [
("file:///repo-B", "/repo-B"),
("file:///C:/repo-B", r"C:\repo-B"),
] {
request.tool_cwd = Some(codex_utils_path_uri::PathUri::parse(uri).unwrap());
let actual = command_input(&request).unwrap();
let mut expected_input = original.clone();
expected_input["cwd"] = serde_json::json!(expected);
assert_eq!(actual, expected_input);
}
}
#[test]
fn command_input_uses_request_tool_name() {
let mut request = request_for_tool_use("call-apply-patch");
@@ -627,6 +647,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(),
codex-rs/hooks/src/events/pre_tool_use.rs
+1 -4
View File
@@ -186,10 +186,7 @@ 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.tool_cwd.as_ref().map_or_else(
|| request.cwd.display().to_string(),
PathUri::inferred_native_path_string,
),
cwd: common::command_input_cwd(request.cwd.as_ref(), request.tool_cwd.as_ref()),
hook_event_name: "PreToolUse".to_string(),
model: request.model.clone(),
permission_mode: request.permission_mode.clone(),
codex-rs/tools/src/tool_output.rs
+12
View File
@@ -3,6 +3,7 @@ use codex_protocol::models::FunctionCallOutputContentItem;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ImageReference;
use codex_protocol::models::ResponseInputItem;
use codex_utils_path_uri::PathUri;
use serde_json::Value as JsonValue;
use crate::ToolPayload;
@@ -40,6 +41,13 @@ pub trait ToolOutput: Send {
call_id.to_string()
}
/// Returns the original local execution cwd for hook input, including delayed
/// results. Remote commands return `None` to retain the host hook cwd.
/// This does not change the local directory used to launch the hook process.
fn post_tool_use_cwd(&self) -> Option<PathUri> {
None
}
/// Returns the tool input exposed to `PostToolUse` hooks for this output.
fn post_tool_use_input(&self, _payload: &ToolPayload) -> Option<JsonValue> {
None
@@ -98,6 +106,10 @@ where
(**self).post_tool_use_id(call_id)
}
fn post_tool_use_cwd(&self) -> Option<PathUri> {
(**self).post_tool_use_cwd()
}
fn post_tool_use_input(&self, payload: &ToolPayload) -> Option<JsonValue> {
(**self).post_tool_use_input(payload)
}