diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 241e121aa9..a808f0d026 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -157,7 +157,7 @@ pub(crate) async fn run_pre_tool_use_hooks( pub(crate) async fn run_permission_request_hooks( sess: &Arc, turn_context: &Arc, - run_id_suffix: String, + request_id: &str, payload: PermissionRequestPayload, approval_attempt: PermissionRequestApprovalAttempt, retry_reason: Option, @@ -171,7 +171,7 @@ pub(crate) async fn run_permission_request_hooks( model: turn_context.model_info.slug.clone(), permission_mode: hook_permission_mode(turn_context), tool_name: payload.tool_name, - run_id_suffix, + run_id_suffix: format!("{request_id}:{}", approval_attempt.as_wire_value()), command: payload.command, sandbox_permissions: payload.sandbox_permissions, additional_permissions: payload.additional_permissions, diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index a604e9762c..9c8649cb64 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -314,6 +314,7 @@ impl ToolHandler for UnifiedExecHandler { .exec_command( ExecCommandRequest { command, + hook_command: args.cmd, process_id, yield_time_ms, max_output_tokens, diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index fa421e3eaa..2534e4c02c 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -373,11 +373,7 @@ impl ToolOrchestrator { match run_permission_request_hooks( approval_ctx.session, approval_ctx.turn, - format!( - "{}:{}", - approval_ctx.call_id, - approval_attempt.as_wire_value() - ), + approval_ctx.call_id, permission_request, approval_attempt, approval_ctx.retry_reason.clone(), diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 07c78d2e79..2cdaf37b27 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -8,11 +8,13 @@ use crate::guardian::guardian_timeout_message; use crate::guardian::new_guardian_review_id; use crate::guardian::review_approval_request; use crate::guardian::routes_approval_to_guardian; +use crate::hook_runtime::run_permission_request_hooks; use crate::sandboxing::ExecOptions; use crate::sandboxing::ExecRequest; use crate::sandboxing::SandboxPermissions; use crate::shell::ShellType; use crate::tools::runtimes::build_sandbox_command; +use crate::tools::sandboxing::PermissionRequestPayload; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; @@ -22,6 +24,8 @@ use codex_execpolicy::MatchOptions; use codex_execpolicy::Policy; use codex_execpolicy::RuleMatch; use codex_features::Feature; +use codex_hooks::PermissionRequestApprovalAttempt; +use codex_hooks::PermissionRequestDecision; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::error::CodexErr; use codex_protocol::error::SandboxErr; @@ -320,6 +324,7 @@ enum DecisionSource { struct PromptDecision { decision: ReviewDecision, guardian_review_id: Option, + rejection_message: Option, } fn execve_prompt_is_rejected_by_policy( @@ -394,11 +399,49 @@ impl CoreShellActionProvider { let guardian_review_id = routes_approval_to_guardian(&turn).then(new_guardian_review_id); Ok(stopwatch .pause_for(async move { + // 1) Run PermissionRequest hooks + let permission_request = PermissionRequestPayload { + tool_name: "Bash".to_string(), + command: codex_shell_command::parse_command::shlex_join(&command), + sandbox_permissions: self.approval_sandbox_permissions, + additional_permissions: additional_permissions.clone(), + justification: None, + }; + let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone()); + match run_permission_request_hooks( + &session, + &turn, + &effective_approval_id, + permission_request, + PermissionRequestApprovalAttempt::Initial, + /*retry_reason*/ None, + /*network_approval_context*/ None, + ) + .await + { + Some(PermissionRequestDecision::Allow) => { + return PromptDecision { + decision: ReviewDecision::Approved, + guardian_review_id: None, + rejection_message: None, + }; + } + Some(PermissionRequestDecision::Deny { message }) => { + return PromptDecision { + decision: ReviewDecision::Denied, + guardian_review_id: None, + rejection_message: Some(message), + }; + } + None => {} + } + + // 2) Route to Guardian if configured if let Some(review_id) = guardian_review_id.clone() { let decision = review_approval_request( &session, &turn, - review_id, + review_id.clone(), GuardianApprovalRequest::Execve { id: call_id.clone(), source, @@ -413,8 +456,11 @@ impl CoreShellActionProvider { return PromptDecision { decision, guardian_review_id, + rejection_message: None, }; } + + // 3) Fall back to regular user prompt let decision = session .request_command_approval( &turn, @@ -432,6 +478,7 @@ impl CoreShellActionProvider { PromptDecision { decision, guardian_review_id: None, + rejection_message: None, } }) .await) @@ -487,7 +534,11 @@ impl CoreShellActionProvider { } }, ReviewDecision::Denied => { - let message = if let Some(review_id) = + let message = if let Some(message) = + prompt_decision.rejection_message.clone() + { + message + } else if let Some(review_id) = prompt_decision.guardian_review_id.as_deref() { guardian_rejection_message(self.session.as_ref(), review_id).await diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index f11050f72b..46f0c6412f 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -6,11 +6,16 @@ use super::evaluate_intercepted_exec_policy; use super::extract_shell_script; use super::join_program_and_argv; use super::map_exec_result; +use crate::codex::make_session_and_context; +use crate::config::Constrained; use crate::sandboxing::SandboxPermissions; +use anyhow::Context; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; use codex_execpolicy::PolicyParser; use codex_execpolicy::RuleMatch; +use codex_hooks::Hooks; +use codex_hooks::HooksConfig; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; @@ -21,6 +26,7 @@ use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::GranularApprovalConfig; +use codex_protocol::protocol::GuardianCommandSource; use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::SandboxType; @@ -30,8 +36,10 @@ use codex_shell_escalation::ExecResult; use codex_shell_escalation::Permissions as EscalatedPermissions; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; +use serde_json::Value; use std::path::PathBuf; use std::time::Duration; +use tokio::sync::RwLock; fn host_absolute_path(segments: &[&str]) -> String { let mut path = if cfg!(windows) { @@ -319,6 +327,134 @@ fn shell_request_escalation_execution_is_explicit() { ); } +#[tokio::test(flavor = "current_thread")] +async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Result<()> { + let (mut session, mut turn_context) = make_session_and_context().await; + std::fs::create_dir_all(&turn_context.config.codex_home) + .context("recreate codex home for hook fixtures")?; + let script_path = turn_context + .config + .codex_home + .join("permission_request_hook.py"); + let log_path = turn_context + .config + .codex_home + .join("permission_request_hook_log.jsonl"); + std::fs::write( + &script_path, + format!( + "#!/bin/sh\ncat > {log_path}\nprintf '%s\\n' '{response}'\n", + log_path = shlex::try_quote(log_path.to_string_lossy().as_ref())?, + response = "{\"hookSpecificOutput\":{\"hookEventName\":\"PermissionRequest\",\"decision\":{\"behavior\":\"allow\"}}}", + ), + ) + .with_context(|| format!("write hook script to {}", script_path.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + let mut permissions = std::fs::metadata(&script_path) + .with_context(|| format!("read hook script metadata from {}", script_path.display()))? + .permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&script_path, permissions) + .with_context(|| format!("set hook script permissions on {}", script_path.display()))?; + } + std::fs::write( + turn_context.config.codex_home.join("hooks.json"), + serde_json::json!({ + "hooks": { + "PermissionRequest": [{ + "hooks": [{ + "type": "command", + "command": script_path.display().to_string(), + }] + }] + } + }) + .to_string(), + ) + .context("write hooks.json")?; + + let mut hook_shell_argv = session + .user_shell() + .derive_exec_args("", /*use_login_shell*/ false); + let hook_shell_program = hook_shell_argv.remove(0); + let _ = hook_shell_argv.pop(); + session.services.hooks = Hooks::new(HooksConfig { + feature_enabled: true, + config_layer_stack: Some(turn_context.config.config_layer_stack.clone()), + shell_program: Some(hook_shell_program), + shell_args: hook_shell_argv, + ..HooksConfig::default() + }); + + let sandbox_policy = SandboxPolicy::new_read_only_policy(); + turn_context.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + turn_context.sandbox_policy = Constrained::allow_any(sandbox_policy.clone()); + turn_context.file_system_sandbox_policy = read_only_file_system_sandbox_policy(); + turn_context.network_sandbox_policy = NetworkSandboxPolicy::Restricted; + + let workdir = AbsolutePathBuf::try_from(std::env::current_dir()?)?; + let target = std::env::temp_dir().join("execve-hook-short-circuit.txt"); + let target_str = target.display().to_string(); + let command = vec!["touch".to_string(), target_str.clone()]; + let expected_hook_command = + codex_shell_command::parse_command::shlex_join(&["/usr/bin/touch".to_string(), target_str]); + let provider = CoreShellActionProvider { + policy: std::sync::Arc::new(RwLock::new(codex_execpolicy::Policy::empty())), + session: std::sync::Arc::new(session), + turn: std::sync::Arc::new(turn_context), + call_id: "execve-hook-call".to_string(), + tool_name: GuardianCommandSource::Shell, + approval_policy: AskForApproval::OnRequest, + sandbox_policy, + file_system_sandbox_policy: read_only_file_system_sandbox_policy(), + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox_permissions: SandboxPermissions::RequireEscalated, + approval_sandbox_permissions: SandboxPermissions::RequireEscalated, + prompt_permissions: None, + stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), + }; + + let action = tokio::time::timeout( + Duration::from_secs(5), + codex_shell_escalation::EscalationPolicy::determine_action( + &provider, + &AbsolutePathBuf::from_absolute_path("/usr/bin/touch") + .context("build touch absolute path")?, + &command, + &workdir, + ), + ) + .await + .context("timed out waiting for execve permission hook decision")??; + assert!(matches!( + action, + codex_shell_escalation::EscalationDecision::Escalate( + codex_shell_escalation::EscalationExecution::Unsandboxed + ) + )); + + let hook_inputs: Vec = std::fs::read_to_string(&log_path) + .with_context(|| format!("read hook log at {}", log_path.display()))? + .lines() + .map(serde_json::from_str) + .collect::>() + .context("parse hook log")?; + assert_eq!(hook_inputs.len(), 1); + assert_eq!( + hook_inputs[0]["tool_input"]["command"], + expected_hook_command + ); + assert_eq!( + hook_inputs[0]["approval_context"]["approval_attempt"], + "initial" + ); + + Ok(()) +} + #[test] fn evaluate_intercepted_exec_policy_uses_wrapper_command_when_shell_wrapper_parsing_disabled() { let policy_src = r#"prefix_rule(pattern = ["npm", "publish"], decision = "prompt")"#; diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 3f3b018df4..4285ef2e51 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -88,6 +88,7 @@ impl UnifiedExecContext { #[derive(Debug)] pub(crate) struct ExecCommandRequest { pub command: Vec, + pub hook_command: String, pub process_id: i32, pub yield_time_ms: u64, pub max_output_tokens: Option, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 6710abe0ab..b3705ca07e 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -677,7 +677,7 @@ impl UnifiedExecProcessManager { .await; let req = UnifiedExecToolRequest { command: request.command.clone(), - hook_command: codex_shell_command::parse_command::shlex_join(&request.command), + hook_command: request.hook_command.clone(), process_id: request.process_id, cwd, env, diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index cda757f9f2..3425bbf313 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -1183,6 +1183,100 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() -> Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn permission_request_hook_sees_raw_exec_command_input() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "permissionrequest-exec-command"; + let marker = std::env::temp_dir().join("permissionrequest-exec-command-marker"); + let command = format!("rm -f {}", marker.display()); + let args = serde_json::json!({ + "cmd": command, + "login": true, + }); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + core_test_support::responses::ev_function_call( + call_id, + "exec_command", + &serde_json::to_string(&args)?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "permission request hook allowed exec_command"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let mut builder = test_codex() + .with_pre_build_hook(|home| { + if let Err(error) = write_permission_request_hook( + home, + Some("^Bash$"), + "allow", + "should not be used for allow", + ) { + panic!("failed to write permission request hook test fixture: {error}"); + } + }) + .with_config(|config| { + config.use_experimental_unified_exec_tool = true; + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + fs::write(&marker, "seed").context("create exec command permission request marker")?; + + test.submit_turn_with_policies( + "run the exec command after hook approval", + AskForApproval::OnRequest, + codex_protocol::protocol::SandboxPolicy::DangerFullAccess, + ) + .await?; + + let requests = responses.requests(); + assert_eq!(requests.len(), 2); + requests[1].function_call_output(call_id); + assert!( + !marker.exists(), + "approved exec command should remove marker file" + ); + + let hook_inputs = read_permission_request_hook_inputs(test.codex_home_path())?; + assert_eq!(hook_inputs.len(), 1); + assert_eq!(hook_inputs[0]["hook_event_name"], "PermissionRequest"); + assert_eq!(hook_inputs[0]["tool_name"], "Bash"); + assert_eq!(hook_inputs[0]["tool_input"]["command"], command); + assert_eq!( + hook_inputs[0]["approval_context"], + serde_json::json!({ + "sandbox_permissions": "use_default", + "additional_permissions": null, + "justification": null, + "approval_attempt": "initial", + "retry_reason": null, + "network_approval_context": null, + }) + ); + + Ok(()) +} + #[cfg(not(target_os = "linux"))] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn permission_request_hook_sees_retry_context_after_sandbox_denial() -> Result<()> { diff --git a/codex-rs/hooks/src/events/permission_request.rs b/codex-rs/hooks/src/events/permission_request.rs index 24123d5083..10b00c4642 100644 --- a/codex-rs/hooks/src/events/permission_request.rs +++ b/codex-rs/hooks/src/events/permission_request.rs @@ -211,7 +211,7 @@ fn build_command_input(request: &PermissionRequestRequest) -> PermissionRequestC sandbox_permissions: request.sandbox_permissions, additional_permissions: request.additional_permissions.clone(), justification: request.justification.clone(), - approval_attempt: request.approval_attempt.clone(), + approval_attempt: request.approval_attempt, retry_reason: request.retry_reason.clone(), network_approval_context: request.network_approval_context.clone(), },