openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-09 15:58:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

Support decision.updatedInput in PermissionRequest hook output

Browse Source
This commit is contained in:
Abhinav Vedmala
2026-04-14 17:22:07 -07:00
parent f7d4874a0e
commit eba705f448
17 changed files with 572 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
codex-rs/core/src/hook_runtime.rs
View File

@@ -165,8 +165,9 @@ pub(crate) async fn run_pre_tool_use_hooks(
}
// PermissionRequest hooks share the same preview/start/completed event flow as
// other hook types, but they return an optional decision instead of mutating
// tool input or post-run state.
// other hook types, but they return an optional approval decision plus any
// selected permission updates, and allow decisions may rewrite the hook-visible
// command input before execution.
pub(crate) async fn run_permission_request_hooks(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
@@ -197,6 +198,7 @@ pub(crate) async fn run_permission_request_hooks(
if let Some(PermissionRequestDecision::Allow {
updated_permissions,
updated_input: _,
}) = &decision
{
apply_permission_updates_from_hook(sess, turn_context, updated_permissions).await;

9
codex-rs/core/src/tools/handlers/shell.rs
View File

@@ -31,6 +31,7 @@ use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
use crate::tools::runtimes::shell::ShellRequest;
use crate::tools::runtimes::shell::ShellRequestCommandInputKind;
use crate::tools::runtimes::shell::ShellRuntime;
use crate::tools::runtimes::shell::ShellRuntimeBackend;
use crate::tools::sandboxing::ToolCtx;
@@ -79,6 +80,7 @@ struct RunExecLikeArgs {
tool_name: String,
exec_params: ExecParams,
hook_command: String,
command_input_kind: ShellRequestCommandInputKind,
additional_permissions: Option<PermissionProfile>,
prefix_rule: Option<Vec<String>>,
session: Arc<crate::codex::Session>,
@@ -244,6 +246,7 @@ impl ToolHandler for ShellHandler {
tool_name: tool_name.display(),
exec_params,
hook_command: codex_shell_command::parse_command::shlex_join(&params.command),
command_input_kind: ShellRequestCommandInputKind::Argv,
additional_permissions: params.additional_permissions.clone(),
prefix_rule,
session,
@@ -262,6 +265,7 @@ impl ToolHandler for ShellHandler {
tool_name: tool_name.display(),
exec_params,
hook_command: codex_shell_command::parse_command::shlex_join(&params.command),
command_input_kind: ShellRequestCommandInputKind::Argv,
additional_permissions: None,
prefix_rule: None,
session,
@@ -351,6 +355,8 @@ impl ToolHandler for ShellCommandHandler {
let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?;
let params: ShellCommandToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?;
let use_login_shell =
Self::resolve_use_login_shell(params.login, turn.tools_config.allow_login_shell)?;
let workdir = turn.resolve_path(params.workdir.clone());
maybe_emit_implicit_skill_invocation(
session.as_ref(),
@@ -371,6 +377,7 @@ impl ToolHandler for ShellCommandHandler {
tool_name: tool_name.display(),
exec_params,
hook_command: params.command,
command_input_kind: ShellRequestCommandInputKind::ShellString { use_login_shell },
additional_permissions: params.additional_permissions.clone(),
prefix_rule,
session,
@@ -390,6 +397,7 @@ impl ShellHandler {
tool_name,
exec_params,
hook_command,
command_input_kind,
additional_permissions,
prefix_rule,
session,
@@ -530,6 +538,7 @@ impl ShellHandler {
let req = ShellRequest {
command: exec_params.command.clone(),
command_input_kind,
hook_command,
cwd: exec_params.cwd.clone(),
timeout_ms: exec_params.expiration.timeout_ms(),

66
codex-rs/core/src/tools/handlers/shell_tests.rs
View File

@@ -16,11 +16,19 @@ use crate::tools::context::ToolPayload;
use crate::tools::handlers::ShellCommandHandler;
use crate::tools::handlers::ShellHandler;
use crate::tools::registry::ToolHandler;
use crate::tools::runtimes::shell::ShellRequest;
use crate::tools::runtimes::shell::ShellRequestCommandInputKind;
use crate::tools::runtimes::shell::ShellRuntime;
use crate::tools::sandboxing::Approvable;
use crate::tools::sandboxing::ApprovalCtx;
use crate::tools::sandboxing::ExecApprovalRequirement;
use crate::turn_diff_tracker::TurnDiffTracker;
use codex_hooks::PermissionRequestToolInput;
use codex_shell_command::is_safe_command::is_known_safe_command;
use codex_shell_command::powershell::try_find_powershell_executable_blocking;
use codex_shell_command::powershell::try_find_pwsh_executable_blocking;
use serde_json::json;
use std::collections::HashMap;
use tokio::sync::Mutex;
use tokio::sync::watch;
@@ -199,6 +207,64 @@ fn shell_command_handler_rejects_login_when_disallowed() {
);
}
#[tokio::test]
async fn shell_runtime_rewrite_updates_command_and_justification() {
let (session, turn) = make_session_and_context().await;
let session = Arc::new(session);
let turn = Arc::new(turn);
let req = ShellRequest {
command: session
.user_shell()
.derive_exec_args("echo original", /*use_login_shell*/ true),
command_input_kind: ShellRequestCommandInputKind::ShellString {
use_login_shell: true,
},
hook_command: "echo original".to_string(),
cwd: turn.cwd.clone(),
timeout_ms: None,
env: HashMap::new(),
explicit_env_overrides: HashMap::new(),
network: None,
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
#[cfg(unix)]
additional_permissions_preapproved: false,
justification: Some("old justification".to_string()),
exec_approval_requirement: ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: None,
},
};
let runtime = ShellRuntime::for_shell_command(super::ShellRuntimeBackend::ShellCommandClassic);
let updated = runtime
.updated_request_from_permission_request(
&req,
&PermissionRequestToolInput {
command: "echo rewritten".to_string(),
description: Some("new justification".to_string()),
},
&ApprovalCtx {
session: &session,
turn: &turn,
call_id: "call-1",
guardian_review_id: None,
retry_reason: None,
network_approval_context: None,
},
)
.expect("rewrite should succeed");
assert_eq!(
updated.command,
session
.user_shell()
.derive_exec_args("echo rewritten", /*use_login_shell*/ true)
);
assert_eq!(updated.hook_command, "echo rewritten");
assert_eq!(updated.justification, Some("new justification".to_string()));
}
#[tokio::test]
async fn shell_pre_tool_use_payload_uses_joined_command() {
let payload = ToolPayload::LocalShell {

32
codex-rs/core/src/tools/handlers/unified_exec.rs
View File

@@ -208,10 +208,15 @@ impl ToolHandler for UnifiedExecHandler {
turn.tools_config.allow_login_shell,
)
.map_err(FunctionCallError::RespondToModel)?;
let use_login_shell =
resolve_use_login_shell(args.login, turn.tools_config.allow_login_shell)
.map_err(FunctionCallError::RespondToModel)?;
let command_for_display = codex_shell_command::parse_command::shlex_join(&command);
let hook_command = args.cmd.clone();
let ExecCommandArgs {
workdir,
shell,
tty,
yield_time_ms,
max_output_tokens,
@@ -314,7 +319,9 @@ impl ToolHandler for UnifiedExecHandler {
.exec_command(
ExecCommandRequest {
command,
hook_command: args.cmd,
hook_command,
shell,
use_login_shell,
process_id,
yield_time_ms,
max_output_tokens,
@@ -388,15 +395,7 @@ pub(crate) fn get_command(
shell_mode: &UnifiedExecShellMode,
allow_login_shell: bool,
) -> Result<Vec<String>, String> {
let use_login_shell = match args.login {
Some(true) if !allow_login_shell => {
return Err(
"login shell is disabled by config; omit `login` or set it to false.".to_string(),
);
}
Some(use_login_shell) => use_login_shell,
None => allow_login_shell,
};
let use_login_shell = resolve_use_login_shell(args.login, allow_login_shell)?;
match shell_mode {
UnifiedExecShellMode::Direct => {
@@ -416,6 +415,19 @@ pub(crate) fn get_command(
}
}
pub(crate) fn resolve_use_login_shell(
login: Option<bool>,
allow_login_shell: bool,
) -> Result<bool, String> {
match login {
Some(true) if !allow_login_shell => {
Err("login shell is disabled by config; omit `login` or set it to false.".to_string())
}
Some(use_login_shell) => Ok(use_login_shell),
None => Ok(allow_login_shell),
}
}
#[cfg(test)]
#[path = "unified_exec_tests.rs"]
mod tests;

82
codex-rs/core/src/tools/handlers/unified_exec_tests.rs
View File

@@ -2,6 +2,12 @@ use super::*;
use crate::shell::default_user_shell;
use crate::tools::handlers::parse_arguments_with_base_path;
use crate::tools::handlers::resolve_workdir_base_path;
use crate::tools::runtimes::unified_exec::UnifiedExecRequest;
use crate::tools::runtimes::unified_exec::UnifiedExecRuntime;
use crate::tools::sandboxing::Approvable;
use crate::tools::sandboxing::ApprovalCtx;
use crate::tools::sandboxing::ExecApprovalRequirement;
use codex_hooks::PermissionRequestToolInput;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_tools::UnifiedExecShellMode;
@@ -9,6 +15,7 @@ use codex_tools::ZshForkConfig;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::PathExt;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::fs;
use std::sync::Arc;
use tempfile::tempdir;
@@ -198,6 +205,81 @@ fn exec_command_args_resolve_relative_additional_permissions_against_workdir() -
Ok(())
}
#[tokio::test]
async fn unified_exec_runtime_rewrite_updates_command_and_justification() {
let (session, turn) = make_session_and_context().await;
let session = Arc::new(session);
let turn = Arc::new(turn);
let manager = UnifiedExecProcessManager::default();
let runtime = UnifiedExecRuntime::new(&manager, UnifiedExecShellMode::Direct);
let req = UnifiedExecRequest {
command: session
.user_shell()
.derive_exec_args("echo original", /*use_login_shell*/ true),
hook_command: "echo original".to_string(),
shell: Some("/bin/bash".to_string()),
use_login_shell: true,
process_id: 7,
cwd: turn.cwd.clone(),
env: HashMap::new(),
explicit_env_overrides: HashMap::new(),
network: None,
tty: false,
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
#[cfg(unix)]
additional_permissions_preapproved: false,
justification: Some("old justification".to_string()),
exec_approval_requirement: ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: None,
},
};
let updated = runtime
.updated_request_from_permission_request(
&req,
&PermissionRequestToolInput {
command: "echo rewritten".to_string(),
description: Some("new justification".to_string()),
},
&ApprovalCtx {
session: &session,
turn: &turn,
call_id: "call-1",
guardian_review_id: None,
retry_reason: None,
network_approval_context: None,
},
)
.expect("rewrite should succeed");
assert_eq!(
updated.command,
get_command(
&ExecCommandArgs {
cmd: "echo rewritten".to_string(),
workdir: None,
shell: Some("/bin/bash".to_string()),
login: Some(true),
tty: false,
yield_time_ms: 10_000,
max_output_tokens: None,
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
justification: None,
prefix_rule: None,
},
session.user_shell(),
&UnifiedExecShellMode::Direct,
/*allow_login_shell*/ true,
)
.expect("command should build")
);
assert_eq!(updated.hook_command, "echo rewritten");
assert_eq!(updated.justification, Some("new justification".to_string()));
}
#[tokio::test]
async fn exec_command_pre_tool_use_payload_uses_raw_command() {
let payload = ToolPayload::Function {

73
codex-rs/core/src/tools/orchestrator.rs
View File

@@ -46,6 +46,11 @@ pub(crate) struct OrchestratorRunResult<Out> {
pub deferred_network_approval: Option<DeferredNetworkApproval>,
}
struct ApprovalResult<Rq> {
decision: ReviewDecision,
updated_req: Option<Rq>,
}
struct ApprovalTelemetry<'a> {
otel: &'a SessionTelemetry,
tool_name: &'a str,
@@ -124,6 +129,7 @@ impl ToolOrchestrator {
let otel_tn = &tool_ctx.tool_name;
let otel_ci = &tool_ctx.call_id;
let use_guardian = routes_approval_to_guardian(turn_ctx);
let mut effective_req = None;
// 1) Approval
let mut already_approved = false;
@@ -153,9 +159,9 @@ impl ToolOrchestrator {
retry_reason: reason,
network_approval_context: None,
};
let decision = Self::request_approval(
let approval = Self::request_approval(
tool,
req,
effective_req.as_ref().unwrap_or(req),
tool_ctx.call_id.as_str(),
approval_ctx,
turn_ctx,
@@ -166,10 +172,11 @@ impl ToolOrchestrator {
},
)
.await?;
effective_req = approval.updated_req.or(effective_req);
Self::enforce_approval_decision(
tool_ctx.session.as_ref(),
guardian_review_id.as_deref(),
decision,
approval.decision,
)
.await?;
already_approved = true;
@@ -183,16 +190,17 @@ impl ToolOrchestrator {
.requirements_toml()
.network
.is_some();
let initial_sandbox = match tool.sandbox_mode_for_first_attempt(req) {
SandboxOverride::BypassSandboxFirstAttempt => SandboxType::None,
SandboxOverride::NoOverride => self.sandbox.select_initial(
&turn_ctx.file_system_sandbox_policy,
turn_ctx.network_sandbox_policy,
tool.sandbox_preference(),
turn_ctx.windows_sandbox_level,
has_managed_network_requirements,
),
};
let initial_sandbox =
match tool.sandbox_mode_for_first_attempt(effective_req.as_ref().unwrap_or(req)) {
SandboxOverride::BypassSandboxFirstAttempt => SandboxType::None,
SandboxOverride::NoOverride => self.sandbox.select_initial(
&turn_ctx.file_system_sandbox_policy,
turn_ctx.network_sandbox_policy,
tool.sandbox_preference(),
turn_ctx.windows_sandbox_level,
has_managed_network_requirements,
),
};
// Platform-specific flag gating is handled by SandboxManager::select_initial.
let use_legacy_landlock = turn_ctx.features.use_legacy_landlock();
@@ -215,7 +223,7 @@ impl ToolOrchestrator {
let (first_result, first_deferred_network_approval) = Self::run_attempt(
tool,
req,
effective_req.as_ref().unwrap_or(req),
tool_ctx,
&initial_attempt,
has_managed_network_requirements,
@@ -298,9 +306,9 @@ impl ToolOrchestrator {
network_approval_context: network_approval_context.clone(),
};
let decision = Self::request_approval(
let approval = Self::request_approval(
tool,
req,
effective_req.as_ref().unwrap_or(req),
&format!("{}:retry", tool_ctx.call_id),
approval_ctx,
turn_ctx,
@@ -311,14 +319,16 @@ impl ToolOrchestrator {
},
)
.await?;
effective_req = approval.updated_req.or(effective_req);
Self::enforce_approval_decision(
tool_ctx.session.as_ref(),
guardian_review_id.as_deref(),
decision,
approval.decision,
)
.await?;
}
let req = effective_req.as_ref().unwrap_or(req);
let escalated_attempt = SandboxAttempt {
sandbox: SandboxType::None,
policy: &turn_ctx.sandbox_policy,
@@ -364,7 +374,7 @@ impl ToolOrchestrator {
approval_ctx: ApprovalCtx<'_>,
turn_ctx: &crate::codex::TurnContext,
telemetry: ApprovalTelemetry<'_>,
) -> Result<ReviewDecision, ToolError>
) -> Result<ApprovalResult<Rq>, ToolError>
where
T: ToolRuntime<Rq, Out>,
{
@@ -377,14 +387,32 @@ impl ToolOrchestrator {
)
.await
{
Some(PermissionRequestDecision::Allow { .. }) => {
Some(PermissionRequestDecision::Allow { updated_input, .. }) => {
let updated_req = updated_input
.as_ref()
.map(|updated_input| {
tool.updated_request_from_permission_request(
req,
updated_input,
&approval_ctx,
)
})
.transpose()
.map_err(|err| {
ToolError::Rejected(format!(
"PermissionRequest hook returned invalid updatedInput: {err}"
))
})?;
telemetry.otel.tool_decision(
telemetry.tool_name,
telemetry.call_id,
&ReviewDecision::Approved,
ToolDecisionSource::Config,
);
return Ok(ReviewDecision::Approved);
return Ok(ApprovalResult {
decision: ReviewDecision::Approved,
updated_req,
});
}
Some(PermissionRequestDecision::Deny { message }) => {
telemetry.otel.tool_decision(
@@ -411,7 +439,10 @@ impl ToolOrchestrator {
&decision,
otel_source,
);
Ok(decision)
Ok(ApprovalResult {
decision,
updated_req: None,
})
}
// Normalizes approval outcomes from hooks, guardian, and user prompts into

42
codex-rs/core/src/tools/runtimes/shell.rs
View File

@@ -33,6 +33,7 @@ use crate::tools::sandboxing::ToolRuntime;
use crate::tools::sandboxing::approval_permission_suggestions;
use crate::tools::sandboxing::sandbox_override_for_first_attempt;
use crate::tools::sandboxing::with_cached_approval;
use codex_hooks::PermissionRequestToolInput;
use codex_hooks::PermissionUpdateDestination;
use codex_network_proxy::NetworkProxy;
use codex_protocol::exec_output::ExecToolCallOutput;
@@ -42,11 +43,13 @@ use codex_sandboxing::SandboxablePreference;
use codex_shell_command::powershell::prefix_powershell_script_with_utf8;
use codex_utils_absolute_path::AbsolutePathBuf;
use futures::future::BoxFuture;
use shlex::split as shlex_split;
use std::collections::HashMap;
#[derive(Clone, Debug)]
pub struct ShellRequest {
pub command: Vec<String>,
pub command_input_kind: ShellRequestCommandInputKind,
pub hook_command: String,
pub cwd: AbsolutePathBuf,
pub timeout_ms: Option<u64>,
@@ -61,6 +64,12 @@ pub struct ShellRequest {
pub exec_approval_requirement: ExecApprovalRequirement,
}
#[derive(Clone, Debug)]
pub enum ShellRequestCommandInputKind {
Argv,
ShellString { use_login_shell: bool },
}
/// Selects `ShellRuntime` behavior for different callers.
///
/// Note: `Generic` is not the same as `ShellCommandClassic`.
@@ -221,11 +230,44 @@ impl Approvable<ShellRequest> for ShellRuntime {
})
}
fn updated_request_from_permission_request(
&self,
req: &ShellRequest,
updated_input: &PermissionRequestToolInput,
approval_ctx: &ApprovalCtx<'_>,
) -> Result<ShellRequest, String> {
let mut updated_req = req.clone();
updated_req.command = match req.command_input_kind {
ShellRequestCommandInputKind::Argv => parse_command_argv(&updated_input.command)?,
ShellRequestCommandInputKind::ShellString { use_login_shell } => approval_ctx
.session
.user_shell()
.derive_exec_args(&updated_input.command, use_login_shell),
};
updated_req.hook_command = updated_input.command.clone();
updated_req.justification = updated_input.description.clone();
Ok(updated_req)
}
fn sandbox_mode_for_first_attempt(&self, req: &ShellRequest) -> SandboxOverride {
sandbox_override_for_first_attempt(req.sandbox_permissions, &req.exec_approval_requirement)
}
}
fn parse_command_argv(command: &str) -> Result<Vec<String>, String> {
let parsed = shlex_split(command).unwrap_or_else(|| {
command
.split_whitespace()
.map(ToString::to_string)
.collect()
});
if parsed.is_empty() {
Err("command cannot be empty".to_string())
} else {
Ok(parsed)
}
}
impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
fn network_approval_spec(
&self,

51
codex-rs/core/src/tools/runtimes/unified_exec.rs
View File

@@ -11,7 +11,9 @@ use crate::guardian::GuardianApprovalRequest;
use crate::guardian::review_approval_request;
use crate::sandboxing::ExecOptions;
use crate::sandboxing::SandboxPermissions;
use crate::shell::Shell;
use crate::shell::ShellType;
use crate::shell::get_shell_by_model_provided_path;
use crate::tools::network_approval::NetworkApprovalMode;
use crate::tools::network_approval::NetworkApprovalSpec;
use crate::tools::runtimes::build_sandbox_command;
@@ -34,6 +36,7 @@ use crate::unified_exec::NoopSpawnLifecycle;
use crate::unified_exec::UnifiedExecError;
use crate::unified_exec::UnifiedExecProcess;
use crate::unified_exec::UnifiedExecProcessManager;
use codex_hooks::PermissionRequestToolInput;
use codex_hooks::PermissionUpdateDestination;
use codex_network_proxy::NetworkProxy;
use codex_protocol::error::CodexErr;
@@ -46,6 +49,8 @@ use codex_tools::UnifiedExecShellMode;
use codex_utils_absolute_path::AbsolutePathBuf;
use futures::future::BoxFuture;
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::Arc;
/// Request payload used by the unified-exec runtime after approvals and
/// sandbox preferences have been resolved for the current turn.
@@ -53,6 +58,8 @@ use std::collections::HashMap;
pub struct UnifiedExecRequest {
pub command: Vec<String>,
pub hook_command: String,
pub shell: Option<String>,
pub use_login_shell: bool,
pub process_id: i32,
pub cwd: AbsolutePathBuf,
pub env: HashMap<String, String>,
@@ -201,6 +208,25 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
})
}
fn updated_request_from_permission_request(
&self,
req: &UnifiedExecRequest,
updated_input: &PermissionRequestToolInput,
approval_ctx: &ApprovalCtx<'_>,
) -> Result<UnifiedExecRequest, String> {
let mut updated_req = req.clone();
updated_req.command = build_command(
&updated_input.command,
req.shell.as_deref(),
req.use_login_shell,
approval_ctx.session.user_shell(),
&self.shell_mode,
);
updated_req.hook_command = updated_input.command.clone();
updated_req.justification = updated_input.description.clone();
Ok(updated_req)
}
fn sandbox_mode_for_first_attempt(&self, req: &UnifiedExecRequest) -> SandboxOverride {
sandbox_override_for_first_attempt(req.sandbox_permissions, &req.exec_approval_requirement)
}
@@ -347,3 +373,28 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
})
}
}
fn build_command(
cmd: &str,
shell: Option<&str>,
use_login_shell: bool,
session_shell: Arc<Shell>,
shell_mode: &UnifiedExecShellMode,
) -> Vec<String> {
match shell_mode {
UnifiedExecShellMode::Direct => {
let model_shell = shell.map(|shell_str| {
let mut shell = get_shell_by_model_provided_path(&PathBuf::from(shell_str));
shell.shell_snapshot = crate::shell::empty_shell_snapshot_receiver();
shell
});
let shell = model_shell.as_ref().unwrap_or(session_shell.as_ref());
shell.derive_exec_args(cmd, use_login_shell)
}
UnifiedExecShellMode::ZshFork(zsh_fork_config) => vec![
zsh_fork_config.shell_zsh_path.to_string_lossy().to_string(),
if use_login_shell { "-lc" } else { "-c" }.to_string(),
cmd.to_string(),
],
}
}

10
codex-rs/core/src/tools/sandboxing.rs
View File

@@ -10,6 +10,7 @@ use crate::sandboxing::ExecOptions;
use crate::sandboxing::SandboxPermissions;
use crate::state::SessionServices;
use crate::tools::network_approval::NetworkApprovalSpec;
use codex_hooks::PermissionRequestToolInput;
use codex_hooks::PermissionUpdate;
use codex_hooks::PermissionUpdateBehavior;
use codex_hooks::PermissionUpdateDestination;
@@ -341,6 +342,15 @@ pub(crate) trait Approvable<Req> {
None
}
fn updated_request_from_permission_request(
&self,
_req: &Req,
_updated_input: &PermissionRequestToolInput,
_approval_ctx: &ApprovalCtx<'_>,
) -> Result<Req, String> {
Err("PermissionRequest hook returned unsupported updatedInput".to_string())
}
/// Decide we can request an approval for no-sandbox execution.
fn wants_no_sandbox_approval(&self, policy: AskForApproval) -> bool {
match policy {

2
codex-rs/core/src/unified_exec/mod.rs
View File

@@ -89,6 +89,8 @@ impl UnifiedExecContext {
pub(crate) struct ExecCommandRequest {
pub command: Vec<String>,
pub hook_command: String,
pub shell: Option<String>,
pub use_login_shell: bool,
pub process_id: i32,
pub yield_time_ms: u64,
pub max_output_tokens: Option<usize>,

2
codex-rs/core/src/unified_exec/process_manager.rs
View File

@@ -688,6 +688,8 @@ impl UnifiedExecProcessManager {
let req = UnifiedExecToolRequest {
command: request.command.clone(),
hook_command: request.hook_command.clone(),
shell: request.shell.clone(),
use_login_shell: request.use_login_shell,
process_id: request.process_id,
cwd,
env,

94
codex-rs/core/tests/suite/hooks.rs
View File

@@ -284,6 +284,17 @@ if mode == "allow":
"decision": {{"behavior": "allow"}}
}}
}}))
elif mode == "rewrite":
updated = json.loads(reason)
print(json.dumps({{
"hookSpecificOutput": {{
"hookEventName": "PermissionRequest",
"decision": {{
"behavior": "allow",
"updatedInput": updated
}}
}}
}}))
elif mode == "allow_selected_session":
print(json.dumps({{
"hookSpecificOutput": {{
@@ -1346,6 +1357,89 @@ 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_rewrites_shell_command_input_before_execution() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let call_id = "permissionrequest-shell-command-rewrite";
let original_marker =
std::env::temp_dir().join("permissionrequest-shell-command-original-marker");
let rewritten_marker =
std::env::temp_dir().join("permissionrequest-shell-command-rewritten-marker");
let original_command = format!("rm -f {}", original_marker.display());
let rewritten_command = format!("rm -f {}", rewritten_marker.display());
let rewritten_description = "rewrite the command before execution";
let args = serde_json::json!({ "command": original_command });
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
core_test_support::responses::ev_function_call(
call_id,
"shell_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 rewrote it"),
ev_completed("resp-2"),
]),
],
)
.await;
let rewritten_input = serde_json::json!({
"command": rewritten_command,
"description": rewritten_description,
});
let mut builder = test_codex()
.with_pre_build_hook(move |home| {
if let Err(error) = write_permission_request_hook(
home,
Some(PERMISSION_REQUEST_HOOK_MATCHER),
"rewrite",
&rewritten_input.to_string(),
) {
panic!("failed to write permission request hook test fixture: {error}");
}
})
.with_config(|config| {
config
.features
.enable(Feature::CodexHooks)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
fs::write(&original_marker, "seed").context("create original marker")?;
fs::write(&rewritten_marker, "seed").context("create rewritten marker")?;
test.submit_turn_with_policies(
"run the rewritten shell 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!(
original_marker.exists(),
"original command should not run after hook rewrite"
);
assert!(
!rewritten_marker.exists(),
"rewritten command should remove rewritten marker"
);
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(()));

23
codex-rs/hooks/schema/generated/permission-request.command.output.schema.json
View File

@@ -36,8 +36,12 @@
"type": "string"
},
"updatedInput": {
"default": null,
"description": "Reserved for a future input-rewrite capability.\n\nPermissionRequest hooks currently fail closed if this field is present."
"allOf": [
{
"$ref": "#/definitions/PermissionRequestToolInput"
}
],
"default": null
},
"updatedPermissions": {
"default": null,
@@ -72,6 +76,21 @@
],
"type": "object"
},
"PermissionRequestToolInput": {
"additionalProperties": false,
"properties": {
"command": {
"type": "string"
},
"description": {
"type": "string"
}
},
"required": [
"command"
],
"type": "object"
},
"PermissionUpdate": {
"oneOf": [
{

53
codex-rs/hooks/src/engine/output_parser.rs
View File

@@ -23,6 +23,7 @@ pub(crate) struct PreToolUseOutput {
pub(crate) enum PermissionRequestDecision {
Allow {
updated_permissions: Vec<crate::events::permission_request::PermissionUpdate>,
updated_input: Option<PermissionRequestToolInput>,
},
Deny {
message: String,
@@ -63,6 +64,7 @@ pub(crate) struct StopOutput {
pub invalid_block_reason: Option<String>,
}
use crate::events::permission_request::PermissionRequestToolInput;
use crate::schema::BlockDecisionWire;
use crate::schema::HookUniversalOutputWire;
use crate::schema::PermissionRequestBehaviorWire;
@@ -302,8 +304,10 @@ fn unsupported_permission_request_hook_specific_output(
decision: Option<&PermissionRequestDecisionWire>,
) -> Option<String> {
let decision = decision?;
if decision.updated_input.is_some() {
Some("PermissionRequest hook returned unsupported updatedInput".to_string())
if matches!(decision.behavior, PermissionRequestBehaviorWire::Deny)
&& decision.updated_input.is_some()
{
Some("PermissionRequest hook returned updatedInput for deny decision".to_string())
} else if matches!(decision.behavior, PermissionRequestBehaviorWire::Deny)
&& decision.updated_permissions.is_some()
{
@@ -321,6 +325,7 @@ fn permission_request_decision(
match decision.behavior {
PermissionRequestBehaviorWire::Allow => PermissionRequestDecision::Allow {
updated_permissions: decision.updated_permissions.clone().unwrap_or_default(),
updated_input: decision.updated_input.clone(),
},
PermissionRequestBehaviorWire::Deny => PermissionRequestDecision::Deny {
message: decision
@@ -431,9 +436,10 @@ mod tests {
use super::PermissionRequestDecision;
use super::parse_permission_request;
use crate::events::permission_request::PermissionRequestToolInput;
#[test]
fn permission_request_rejects_reserved_updated_input_field() {
fn permission_request_accepts_updated_input_for_allow_decision() {
let parsed = parse_permission_request(
&json!({
"continue": true,
@@ -441,7 +447,9 @@ mod tests {
"hookEventName": "PermissionRequest",
"decision": {
"behavior": "allow",
"updatedInput": {}
"updatedInput": {
"command": "echo hi"
}
}
}
})
@@ -450,8 +458,14 @@ mod tests {
.expect("permission request hook output should parse");
assert_eq!(
parsed.invalid_reason,
Some("PermissionRequest hook returned unsupported updatedInput".to_string())
parsed.decision,
Some(PermissionRequestDecision::Allow {
updated_permissions: vec![],
updated_input: Some(PermissionRequestToolInput {
command: "echo hi".to_string(),
description: None,
}),
})
);
}
@@ -493,6 +507,7 @@ mod tests {
crate::events::permission_request::PermissionUpdateBehavior::Allow,
destination: crate::events::permission_request::PermissionUpdateDestination::UserSettings,
}],
updated_input: None,
})
);
}
@@ -528,10 +543,36 @@ mod tests {
crate::events::permission_request::PermissionUpdateDestination::Session,
},
],
updated_input: None,
})
);
}
#[test]
fn permission_request_rejects_updated_input_for_deny_decision() {
let parsed = parse_permission_request(
&json!({
"continue": true,
"hookSpecificOutput": {
"hookEventName": "PermissionRequest",
"decision": {
"behavior": "deny",
"updatedInput": {
"command": "echo nope"
}
}
}
})
.to_string(),
)
.expect("permission request hook output should parse");
assert_eq!(
parsed.invalid_reason,
Some("PermissionRequest hook returned updatedInput for deny decision".to_string())
);
}
#[test]
fn permission_request_rejects_updated_permissions_for_deny_decision() {
let parsed = parse_permission_request(

69
codex-rs/hooks/src/events/permission_request.rs
View File

@@ -1,9 +1,9 @@
//! Permission-request hook execution.
//!
//! This event runs in the approval path, before guardian or user approval UI is
//! shown. Unlike `pre_tool_use`, handlers do not rewrite tool input or block by
//! stopping execution outright; instead they can return a concrete allow/deny
//! decision, or decline to decide and let the normal approval flow continue.
//! shown. Handlers can return a concrete allow/deny decision, optionally
//! rewriting the hook-visible tool input for allow decisions, or decline to
//! decide and let the normal approval flow continue.
//!
//! The event also mirrors the rest of the hook system's lifecycle:
//!
@@ -23,7 +23,6 @@ use crate::engine::command_runner::CommandRunResult;
use crate::engine::dispatcher;
use crate::engine::output_parser;
use crate::schema::PermissionRequestCommandInput;
use crate::schema::PermissionRequestToolInput;
use codex_protocol::ThreadId;
use codex_protocol::protocol::HookCompletedEvent;
use codex_protocol::protocol::HookEventName;
@@ -88,10 +87,20 @@ pub struct PermissionRequestRequest {
pub permission_suggestions: Vec<PermissionUpdate>,
}
#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Eq)]
#[serde(rename_all = "camelCase")]
#[serde(deny_unknown_fields)]
pub struct PermissionRequestToolInput {
pub command: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub description: Option<String>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum PermissionRequestDecision {
Allow {
updated_permissions: Vec<PermissionUpdate>,
updated_input: Option<PermissionRequestToolInput>,
},
Deny {
message: String,
@@ -212,10 +221,12 @@ fn resolve_permission_request_decision<'a>(
) -> Option<PermissionRequestDecision> {
let mut saw_allow = false;
let mut updated_permissions = Vec::new();
let mut updated_input = None;
for decision in decisions {
match decision {
PermissionRequestDecision::Allow {
updated_permissions: selected_permissions,
updated_input: selected_input,
} => {
saw_allow = true;
for permission in selected_permissions {
@@ -223,6 +234,9 @@ fn resolve_permission_request_decision<'a>(
updated_permissions.push(permission.clone());
}
}
if let Some(selected_input) = selected_input {
updated_input = Some(selected_input.clone());
}
}
PermissionRequestDecision::Deny { message } => {
return Some(PermissionRequestDecision::Deny {
@@ -233,6 +247,7 @@ fn resolve_permission_request_decision<'a>(
}
saw_allow.then_some(PermissionRequestDecision::Allow {
updated_permissions,
updated_input,
})
}
@@ -242,6 +257,7 @@ fn invalid_permission_updates(
) -> Option<String> {
let PermissionRequestDecision::Allow {
updated_permissions,
updated_input: _,
} = decision?
else {
return None;
@@ -315,9 +331,11 @@ fn parse_completed(
match parsed_decision {
output_parser::PermissionRequestDecision::Allow {
updated_permissions,
updated_input,
} => {
decision = Some(PermissionRequestDecision::Allow {
updated_permissions,
updated_input,
});
}
output_parser::PermissionRequestDecision::Deny { message } => {
@@ -387,6 +405,7 @@ mod tests {
use pretty_assertions::assert_eq;
use super::PermissionRequestDecision;
use super::PermissionRequestToolInput;
use super::PermissionUpdate;
use super::PermissionUpdateBehavior;
use super::PermissionUpdateDestination;
@@ -399,6 +418,7 @@ mod tests {
let decisions = [
PermissionRequestDecision::Allow {
updated_permissions: vec![],
updated_input: None,
},
PermissionRequestDecision::Deny {
message: "repo deny".to_string(),
@@ -418,9 +438,11 @@ mod tests {
let decisions = [
PermissionRequestDecision::Allow {
updated_permissions: vec![],
updated_input: None,
},
PermissionRequestDecision::Allow {
updated_permissions: vec![],
updated_input: None,
},
];
@@ -428,6 +450,7 @@ mod tests {
resolve_permission_request_decision(decisions.iter()),
Some(PermissionRequestDecision::Allow {
updated_permissions: vec![],
updated_input: None,
})
);
}
@@ -451,9 +474,11 @@ mod tests {
let decisions = [
PermissionRequestDecision::Allow {
updated_permissions: vec![permission.clone()],
updated_input: None,
},
PermissionRequestDecision::Allow {
updated_permissions: vec![permission.clone()],
updated_input: None,
},
];
@@ -461,6 +486,7 @@ mod tests {
resolve_permission_request_decision(decisions.iter()),
Some(PermissionRequestDecision::Allow {
updated_permissions: vec![permission],
updated_input: None,
})
);
}
@@ -474,9 +500,11 @@ mod tests {
let decisions = [
PermissionRequestDecision::Allow {
updated_permissions: vec![directory_update.clone()],
updated_input: None,
},
PermissionRequestDecision::Allow {
updated_permissions: vec![directory_update.clone()],
updated_input: None,
},
];
@@ -484,6 +512,38 @@ mod tests {
resolve_permission_request_decision(decisions.iter()),
Some(PermissionRequestDecision::Allow {
updated_permissions: vec![directory_update],
updated_input: None,
})
);
}
#[test]
fn permission_request_uses_last_updated_input_from_allows() {
let decisions = [
PermissionRequestDecision::Allow {
updated_permissions: vec![],
updated_input: Some(PermissionRequestToolInput {
command: "echo first".to_string(),
description: Some("first".to_string()),
}),
},
PermissionRequestDecision::Allow {
updated_permissions: vec![],
updated_input: Some(PermissionRequestToolInput {
command: "echo second".to_string(),
description: Some("second".to_string()),
}),
},
];
assert_eq!(
resolve_permission_request_decision(decisions.iter()),
Some(PermissionRequestDecision::Allow {
updated_permissions: vec![],
updated_input: Some(PermissionRequestToolInput {
command: "echo second".to_string(),
description: Some("second".to_string()),
}),
})
);
}
@@ -505,6 +565,7 @@ mod tests {
behavior: PermissionUpdateBehavior::Allow,
destination: PermissionUpdateDestination::UserSettings,
}],
updated_input: None,
};
assert_eq!(

1
codex-rs/hooks/src/lib.rs
View File

@@ -8,6 +8,7 @@ mod types;
pub use events::permission_request::PermissionRequestDecision;
pub use events::permission_request::PermissionRequestOutcome;
pub use events::permission_request::PermissionRequestRequest;
pub use events::permission_request::PermissionRequestToolInput;
pub use events::permission_request::PermissionUpdate;
pub use events::permission_request::PermissionUpdateBehavior;
pub use events::permission_request::PermissionUpdateDestination;

15
codex-rs/hooks/src/schema.rs
View File

@@ -12,6 +12,7 @@ use serde_json::Value;
use std::path::Path;
use std::path::PathBuf;
use crate::events::permission_request::PermissionRequestToolInput;
use crate::events::permission_request::PermissionUpdate;
const GENERATED_DIR: &str = "generated";
@@ -140,11 +141,8 @@ pub(crate) struct PermissionRequestHookSpecificOutputWire {
#[serde(deny_unknown_fields)]
pub(crate) struct PermissionRequestDecisionWire {
pub behavior: PermissionRequestBehaviorWire,
/// Reserved for a future input-rewrite capability.
///
/// PermissionRequest hooks currently fail closed if this field is present.
#[serde(default)]
pub updated_input: Option<Value>,
pub updated_input: Option<PermissionRequestToolInput>,
#[serde(default)]
pub updated_permissions: Option<Vec<PermissionUpdate>>,
#[serde(default)]
@@ -236,15 +234,6 @@ pub(crate) struct PreToolUseCommandInput {
pub tool_use_id: String,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
#[serde(deny_unknown_fields)]
pub(crate) struct PermissionRequestToolInput {
pub command: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub description: Option<String>,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
#[serde(deny_unknown_fields)]
#[schemars(rename = "permission-request.command.input")]