clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-17 17:53:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

Support PermissionRequest updatedInput rewrites

Browse Source
This commit is contained in:
Abhinav Vedmala
2026-05-06 15:41:15 -07:00
parent 5ecbcf609b
commit 8a89e05de5
17 changed files with 545 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
codex-rs/core/src/mcp_tool_call.rs
View File

@@ -215,6 +215,7 @@ pub(crate) async fn handle_mcp_tool_call(
)
.await;
let mut invocation = invocation;
if let Some(decision) = maybe_request_mcp_tool_approval(
&sess,
turn_context,
@@ -223,9 +224,28 @@ pub(crate) async fn handle_mcp_tool_call(
&hook_tool_name,
metadata.as_ref(),
approval_mode,
/*evaluate_permission_request_hooks*/ true,
)
.await
{
let decision = match decision {
McpToolApprovalDecision::AcceptWithUpdatedInput(updated_input) => {
invocation.arguments = Some(updated_input);
maybe_request_mcp_tool_approval(
&sess,
turn_context,
&call_id,
&invocation,
&hook_tool_name,
metadata.as_ref(),
approval_mode,
/*evaluate_permission_request_hooks*/ false,
)
.await
.unwrap_or(McpToolApprovalDecision::Accept)
}
decision => decision,
};
let result = match decision {
McpToolApprovalDecision::Accept
| McpToolApprovalDecision::AcceptForSession
@@ -279,6 +299,9 @@ pub(crate) async fn handle_mcp_tool_call(
)
.await
}
McpToolApprovalDecision::AcceptWithUpdatedInput(_) => {
unreachable!("updated MCP tool input should be re-approved before dispatch")
}
};
let status = if result.is_ok() { "ok" } else { "error" };
@@ -951,6 +974,7 @@ async fn maybe_track_codex_app_used(
#[derive(Debug, Clone, PartialEq, Eq)]
enum McpToolApprovalDecision {
Accept,
AcceptWithUpdatedInput(JsonValue),
AcceptForSession,
AcceptAndRemember,
Decline { message: Option<String> },
@@ -1139,6 +1163,7 @@ fn mcp_tool_approval_prompt_options(
}
}
#[allow(clippy::too_many_arguments)]
async fn maybe_request_mcp_tool_approval(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
@@ -1147,6 +1172,7 @@ async fn maybe_request_mcp_tool_approval(
hook_tool_name: &str,
metadata: Option<&McpToolApprovalMetadata>,
approval_mode: AppToolApproval,
evaluate_permission_request_hooks: bool,
) -> Option<McpToolApprovalDecision> {
if mcp_permission_prompt_is_auto_approved(
turn_context.approval_policy.value(),
@@ -1199,29 +1225,40 @@ async fn maybe_request_mcp_tool_approval(
return Some(McpToolApprovalDecision::Accept);
}
match run_permission_request_hooks(
sess,
turn_context,
call_id,
PermissionRequestPayload {
tool_name: HookToolName::new(hook_tool_name),
tool_input: invocation
.arguments
.clone()
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())),
},
)
.await
{
Some(PermissionRequestDecision::Allow) => {
return Some(McpToolApprovalDecision::Accept);
if evaluate_permission_request_hooks {
match run_permission_request_hooks(
sess,
turn_context,
call_id,
PermissionRequestPayload {
tool_name: HookToolName::new(hook_tool_name),
tool_input: invocation
.arguments
.clone()
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())),
},
)
.await
{
Some(PermissionRequestDecision::Allow {
updated_input: Some(updated_input),
}) => {
return Some(McpToolApprovalDecision::AcceptWithUpdatedInput(
updated_input,
));
}
Some(PermissionRequestDecision::Allow {
updated_input: None,
}) => {
return Some(McpToolApprovalDecision::Accept);
}
Some(PermissionRequestDecision::Deny { message }) => {
return Some(McpToolApprovalDecision::Decline {
message: Some(message),
});
}
None => {}
}
Some(PermissionRequestDecision::Deny { message }) => {
return Some(McpToolApprovalDecision::Decline {
message: Some(message),
});
}
None => {}
}
let tool_call_mcp_elicitation_enabled = turn_context
@@ -1967,6 +2004,7 @@ async fn apply_mcp_tool_approval_decision(
}
}
McpToolApprovalDecision::Accept
| McpToolApprovalDecision::AcceptWithUpdatedInput(_)
| McpToolApprovalDecision::Decline { .. }
| McpToolApprovalDecision::Cancel
| McpToolApprovalDecision::BlockedBySafetyMonitor(_) => {}

12
codex-rs/core/src/mcp_tool_call_tests.rs
View File

@@ -2189,6 +2189,7 @@ async fn approve_mode_skips_when_annotations_do_not_require_approval() {
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Approve,
/*evaluate_permission_request_hooks*/ true,
)
.await;
@@ -2262,6 +2263,7 @@ async fn guardian_mode_skips_auto_when_annotations_do_not_require_approval() {
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Auto,
/*evaluate_permission_request_hooks*/ true,
)
.await;
@@ -2318,6 +2320,7 @@ async fn permission_request_hook_allows_mcp_tool_call() {
"mcp__memory__create_entities",
Some(&metadata),
AppToolApproval::Auto,
/*evaluate_permission_request_hooks*/ true,
)
.await;
@@ -2378,6 +2381,7 @@ async fn permission_request_hook_uses_hook_tool_name_without_metadata() {
"mcp__memory__create_entities",
/*metadata*/ None,
AppToolApproval::Auto,
/*evaluate_permission_request_hooks*/ true,
)
.await;
@@ -2455,6 +2459,7 @@ async fn permission_request_hook_runs_after_remembered_mcp_approval() {
"mcp__memory__create_entities",
Some(&metadata),
AppToolApproval::Auto,
/*evaluate_permission_request_hooks*/ true,
)
.await;
@@ -2535,6 +2540,7 @@ async fn guardian_mode_mcp_denial_returns_rationale_message() {
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Auto,
/*evaluate_permission_request_hooks*/ true,
)
.await;
@@ -2592,6 +2598,7 @@ async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Prompt,
/*evaluate_permission_request_hooks*/ true,
)
.await
})
@@ -2667,6 +2674,7 @@ async fn approve_mode_skips_arc_interrupt_for_model() {
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Approve,
/*evaluate_permission_request_hooks*/ true,
)
.await;
@@ -2734,6 +2742,7 @@ async fn custom_approve_mode_skips_arc_interrupt_for_model() {
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Approve,
/*evaluate_permission_request_hooks*/ true,
)
.await;
@@ -2801,6 +2810,7 @@ async fn approve_mode_skips_arc_interrupt_without_annotations() {
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Approve,
/*evaluate_permission_request_hooks*/ true,
)
.await;
@@ -2878,6 +2888,7 @@ async fn full_access_mode_skips_arc_monitor_for_all_approval_modes() {
"mcp__test__tool",
Some(&metadata),
approval_mode,
/*evaluate_permission_request_hooks*/ true,
)
.await;
@@ -2981,6 +2992,7 @@ async fn approve_mode_skips_arc_and_guardian_in_every_permission_mode() {
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Approve,
/*evaluate_permission_request_hooks*/ true,
)
.await;

2
codex-rs/core/src/session/tests.rs
View File

@@ -1031,7 +1031,7 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an
orchestrator
.run(
&mut tool,
&(),
(),
&tool_ctx,
turn.as_ref(),
AskForApproval::Never,

4
codex-rs/core/src/tools/handlers/apply_patch.rs
View File

@@ -422,7 +422,7 @@ impl ToolHandler for ApplyPatchHandler {
let out = orchestrator
.run(
&mut runtime,
&req,
req,
&tool_ctx,
turn.as_ref(),
turn.approval_policy.value(),
@@ -530,7 +530,7 @@ pub(crate) async fn intercept_apply_patch(
let out = orchestrator
.run(
&mut runtime,
&req,
req,
&tool_ctx,
turn.as_ref(),
turn.approval_policy.value(),

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

@@ -229,7 +229,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
} else {
effective_additional_permissions.sandbox_permissions
},
prefix_rule,
prefix_rule: prefix_rule.clone(),
})
.await;
@@ -247,6 +247,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
additional_permissions_preapproved: effective_additional_permissions
.permissions_preapproved,
justification: exec_params.justification.clone(),
prefix_rule,
exec_approval_requirement,
};
let mut orchestrator = ToolOrchestrator::new();
@@ -268,7 +269,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
let out = orchestrator
.run(
&mut runtime,
&req,
req,
&tool_ctx,
&turn,
turn.approval_policy.value(),

20
codex-rs/core/src/tools/network_approval.rs
View File

@@ -473,7 +473,9 @@ impl NetworkApprovalService {
.await
{
match permission_request_decision {
PermissionRequestDecision::Allow => {
PermissionRequestDecision::Allow {
updated_input: None,
} => {
pending
.set_decision(PendingApprovalDecision::AllowOnce)
.await;
@@ -481,6 +483,22 @@ impl NetworkApprovalService {
pending_approvals.remove(&key);
return NetworkDecision::Allow;
}
PermissionRequestDecision::Allow {
updated_input: Some(_),
} => {
let message = "updatedInput is not supported for network approvals".to_string();
if let Some(owner_call) = owner_call.as_ref() {
self.record_call_outcome(
&owner_call.registration_id,
NetworkApprovalOutcome::DeniedByPolicy(message),
)
.await;
}
pending.set_decision(PendingApprovalDecision::Deny).await;
let mut pending_approvals = self.pending_host_approvals.lock().await;
pending_approvals.remove(&key);
return NetworkDecision::deny(REASON_NOT_ALLOWED);
}
PermissionRequestDecision::Deny { message } => {
if let Some(owner_call) = owner_call.as_ref() {
self.record_call_outcome(

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

@@ -46,6 +46,11 @@ pub(crate) struct OrchestratorRunResult<Out> {
pub deferred_network_approval: Option<DeferredNetworkApproval>,
}
enum ApprovalResult {
Decision(ReviewDecision),
UpdatedInput(serde_json::Value),
}
impl ToolOrchestrator {
pub fn new() -> Self {
Self {
@@ -126,7 +131,7 @@ impl ToolOrchestrator {
pub async fn run<Rq, Out, T>(
&mut self,
tool: &mut T,
req: &Rq,
mut req: Rq,
tool_ctx: &ToolCtx,
turn_ctx: &crate::session::turn_context::TurnContext,
approval_policy: AskForApproval,
@@ -142,79 +147,109 @@ impl ToolOrchestrator {
// 1) Approval
let mut already_approved = false;
let mut permission_request_hooks_available = !strict_auto_review;
let file_system_sandbox_policy = turn_ctx.file_system_sandbox_policy();
let network_sandbox_policy = turn_ctx.network_sandbox_policy();
let requirement = tool.exec_approval_requirement(req).unwrap_or_else(|| {
default_exec_approval_requirement(approval_policy, &file_system_sandbox_policy)
});
match requirement {
ExecApprovalRequirement::Skip { .. } => {
if strict_auto_review {
let guardian_review_id = Some(new_guardian_review_id());
loop {
let requirement = tool.exec_approval_requirement(&req).unwrap_or_else(|| {
default_exec_approval_requirement(approval_policy, &file_system_sandbox_policy)
});
match requirement {
ExecApprovalRequirement::Skip { .. } => {
if strict_auto_review {
let guardian_review_id = Some(new_guardian_review_id());
let approval_ctx = ApprovalCtx {
session: &tool_ctx.session,
turn: &tool_ctx.turn,
call_id: &tool_ctx.call_id,
guardian_review_id: guardian_review_id.clone(),
retry_reason: None,
network_approval_context: None,
};
let ApprovalResult::Decision(decision) = Self::request_approval(
tool,
&req,
tool_ctx.call_id.as_str(),
approval_ctx,
tool_ctx,
/*evaluate_permission_request_hooks*/ false,
&otel,
)
.await?
else {
unreachable!("permission hooks are disabled under strict auto review");
};
Self::reject_if_not_approved(
tool_ctx,
guardian_review_id.as_deref(),
decision,
)
.await?;
already_approved = true;
} else {
otel.tool_decision(
otel_tn,
otel_ci,
&ReviewDecision::Approved,
ToolDecisionSource::Config,
);
}
break;
}
ExecApprovalRequirement::Forbidden { reason } => {
return Err(ToolError::Rejected(reason));
}
ExecApprovalRequirement::NeedsApproval { reason, .. } => {
let guardian_review_id = use_guardian.then(new_guardian_review_id);
let approval_ctx = ApprovalCtx {
session: &tool_ctx.session,
turn: &tool_ctx.turn,
call_id: &tool_ctx.call_id,
guardian_review_id: guardian_review_id.clone(),
retry_reason: None,
retry_reason: reason,
network_approval_context: None,
};
let decision = Self::request_approval(
match Self::request_approval(
tool,
req,
&req,
tool_ctx.call_id.as_str(),
approval_ctx,
tool_ctx,
/*evaluate_permission_request_hooks*/ false,
permission_request_hooks_available,
&otel,
)
.await?;
Self::reject_if_not_approved(tool_ctx, guardian_review_id.as_deref(), decision)
.await?;
already_approved = true;
} else {
otel.tool_decision(
otel_tn,
otel_ci,
&ReviewDecision::Approved,
ToolDecisionSource::Config,
);
.await?
{
ApprovalResult::Decision(decision) => {
Self::reject_if_not_approved(
tool_ctx,
guardian_review_id.as_deref(),
decision,
)
.await?;
already_approved = true;
break;
}
ApprovalResult::UpdatedInput(updated_input) => {
req = tool
.with_updated_permission_request_input(
&req,
updated_input,
tool_ctx,
approval_policy,
)
.await?;
permission_request_hooks_available = false;
}
}
}
}
ExecApprovalRequirement::Forbidden { reason } => {
return Err(ToolError::Rejected(reason));
}
ExecApprovalRequirement::NeedsApproval { reason, .. } => {
let guardian_review_id = use_guardian.then(new_guardian_review_id);
let approval_ctx = ApprovalCtx {
session: &tool_ctx.session,
turn: &tool_ctx.turn,
call_id: &tool_ctx.call_id,
guardian_review_id: guardian_review_id.clone(),
retry_reason: reason,
network_approval_context: None,
};
let decision = Self::request_approval(
tool,
req,
tool_ctx.call_id.as_str(),
approval_ctx,
tool_ctx,
/*evaluate_permission_request_hooks*/ !strict_auto_review,
&otel,
)
.await?;
Self::reject_if_not_approved(tool_ctx, guardian_review_id.as_deref(), decision)
.await?;
already_approved = true;
}
}
// 2) First attempt under the selected sandbox.
let managed_network_active = turn_ctx.network.is_some();
let initial_sandbox = match tool.sandbox_mode_for_first_attempt(req) {
let initial_sandbox = match tool.sandbox_mode_for_first_attempt(&req) {
SandboxOverride::BypassSandboxFirstAttempt => SandboxType::None,
SandboxOverride::NoOverride => self.sandbox.select_initial(
&file_system_sandbox_policy,
@@ -227,7 +262,7 @@ impl ToolOrchestrator {
// Platform-specific flag gating is handled by SandboxManager::select_initial.
let use_legacy_landlock = turn_ctx.features.use_legacy_landlock();
let sandbox_cwd = tool.sandbox_cwd(req).unwrap_or(&turn_ctx.cwd);
let sandbox_cwd = tool.sandbox_cwd(&req).unwrap_or(&turn_ctx.cwd);
let initial_attempt = SandboxAttempt {
sandbox: initial_sandbox,
permissions: &turn_ctx.permission_profile,
@@ -246,7 +281,7 @@ impl ToolOrchestrator {
let (first_result, first_deferred_network_approval) = Self::run_attempt(
tool,
req,
&req,
tool_ctx,
&initial_attempt,
managed_network_active,
@@ -333,15 +368,20 @@ impl ToolOrchestrator {
let permission_request_run_id = format!("{}:retry", tool_ctx.call_id);
let decision = Self::request_approval(
tool,
req,
&req,
&permission_request_run_id,
approval_ctx,
tool_ctx,
/*evaluate_permission_request_hooks*/ !strict_auto_review,
permission_request_hooks_available,
&otel,
)
.await?;
let ApprovalResult::Decision(decision) = decision else {
return Err(ToolError::Rejected(
"updatedInput is not supported during retry approval".to_string(),
));
};
Self::reject_if_not_approved(tool_ctx, guardian_review_id.as_deref(), decision)
.await?;
}
@@ -365,7 +405,7 @@ impl ToolOrchestrator {
// Second attempt.
let (retry_result, retry_deferred_network_approval) = Self::run_attempt(
tool,
req,
&req,
tool_ctx,
&escalated_attempt,
managed_network_active,
@@ -391,7 +431,7 @@ impl ToolOrchestrator {
tool_ctx: &ToolCtx,
evaluate_permission_request_hooks: bool,
otel: &codex_otel::SessionTelemetry,
) -> Result<ReviewDecision, ToolError>
) -> Result<ApprovalResult, ToolError>
where
T: ToolRuntime<Rq, Out>,
{
@@ -406,7 +446,14 @@ impl ToolOrchestrator {
)
.await
{
Some(PermissionRequestDecision::Allow) => {
Some(PermissionRequestDecision::Allow {
updated_input: Some(updated_input),
}) => {
return Ok(ApprovalResult::UpdatedInput(updated_input));
}
Some(PermissionRequestDecision::Allow {
updated_input: None,
}) => {
let decision = ReviewDecision::Approved;
otel.tool_decision(
&tool_ctx.tool_name,
@@ -414,7 +461,7 @@ impl ToolOrchestrator {
&decision,
ToolDecisionSource::Config,
);
return Ok(decision);
return Ok(ApprovalResult::Decision(decision));
}
Some(PermissionRequestDecision::Deny { message }) => {
let decision = ReviewDecision::Denied;
@@ -442,7 +489,7 @@ impl ToolOrchestrator {
&decision,
otel_source,
);
Ok(decision)
Ok(ApprovalResult::Decision(decision))
}
async fn reject_if_not_approved(

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

@@ -38,11 +38,13 @@ use crate::tools::sandboxing::with_cached_approval;
use codex_network_proxy::NetworkProxy;
use codex_protocol::exec_output::ExecToolCallOutput;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
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 serde_json::Value;
use std::collections::HashMap;
#[derive(Clone, Debug)]
@@ -59,6 +61,7 @@ pub struct ShellRequest {
#[cfg(unix)]
pub additional_permissions_preapproved: bool,
pub justification: Option<String>,
pub prefix_rule: Option<Vec<String>>,
pub exec_approval_requirement: ExecApprovalRequirement,
}
@@ -215,6 +218,75 @@ impl Approvable<ShellRequest> for ShellRuntime {
}
impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
fn with_updated_permission_request_input<'a>(
&'a self,
req: &'a ShellRequest,
updated_input: Value,
ctx: &'a ToolCtx,
approval_policy: AskForApproval,
) -> BoxFuture<'a, Result<ShellRequest, ToolError>> {
Box::pin(async move {
let command = updated_input
.get("command")
.and_then(Value::as_str)
.ok_or_else(|| {
ToolError::Rejected(
"PermissionRequest updatedInput must include string field `command`"
.to_string(),
)
})?;
let mut updated = req.clone();
updated.hook_command = command.to_string();
updated.justification = match updated_input.get("description") {
Some(Value::String(description)) => Some(description.clone()),
Some(_) => {
return Err(ToolError::Rejected(
"PermissionRequest updatedInput field `description` must be a string"
.to_string(),
));
}
None => None,
};
updated.command = match self.backend {
ShellRuntimeBackend::Generic => shlex::split(command).ok_or_else(|| {
ToolError::Rejected(
"PermissionRequest updatedInput contained an invalid shell command"
.to_string(),
)
})?,
ShellRuntimeBackend::ShellCommandClassic
| ShellRuntimeBackend::ShellCommandZshFork => {
let mut command_argv = updated.command;
let Some(script) = command_argv.last_mut() else {
return Err(ToolError::Rejected(
"shell command args are empty".to_string(),
));
};
*script = command.to_string();
command_argv
}
};
let file_system_sandbox_policy = ctx.turn.file_system_sandbox_policy();
updated.exec_approval_requirement = ctx
.session
.services
.exec_policy
.create_exec_approval_requirement_for_command(
crate::exec_policy::ExecApprovalRequest {
command: &updated.command,
approval_policy,
permission_profile: ctx.turn.permission_profile(),
file_system_sandbox_policy: &file_system_sandbox_policy,
sandbox_cwd: updated.cwd.as_path(),
sandbox_permissions: updated_exec_policy_sandbox_permissions(&updated),
prefix_rule: updated.prefix_rule.clone(),
},
)
.await;
Ok(updated)
})
}
fn network_approval_spec(
&self,
req: &ShellRequest,
@@ -292,3 +364,12 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
Ok(out)
}
}
fn updated_exec_policy_sandbox_permissions(req: &ShellRequest) -> SandboxPermissions {
#[cfg(unix)]
if req.additional_permissions_preapproved {
return SandboxPermissions::UseDefault;
}
req.sandbox_permissions
}

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

@@ -420,13 +420,27 @@ impl CoreShellActionProvider {
)
.await
{
Some(PermissionRequestDecision::Allow) => {
Some(PermissionRequestDecision::Allow {
updated_input: None,
}) => {
return PromptDecision {
decision: ReviewDecision::Approved,
guardian_review_id: None,
rejection_message: None,
};
}
Some(PermissionRequestDecision::Allow {
updated_input: Some(_),
}) => {
return PromptDecision {
decision: ReviewDecision::Denied,
guardian_review_id: None,
rejection_message: Some(
"updatedInput is not supported for intercepted exec approvals"
.to_string(),
),
};
}
Some(PermissionRequestDecision::Deny { message }) => {
return PromptDecision {
decision: ReviewDecision::Denied,

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

@@ -42,12 +42,14 @@ use codex_network_proxy::NetworkProxy;
use codex_protocol::error::CodexErr;
use codex_protocol::error::SandboxErr;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use codex_sandboxing::SandboxablePreference;
use codex_shell_command::powershell::prefix_powershell_script_with_utf8;
use codex_tools::UnifiedExecShellMode;
use codex_utils_absolute_path::AbsolutePathBuf;
use futures::future::BoxFuture;
use serde_json::Value;
use std::collections::HashMap;
use std::sync::Arc;
use tokio_util::sync::CancellationToken;
@@ -71,6 +73,7 @@ pub struct UnifiedExecRequest {
#[cfg(unix)]
pub additional_permissions_preapproved: bool,
pub justification: Option<String>,
pub prefix_rule: Option<Vec<String>>,
pub exec_approval_requirement: ExecApprovalRequirement,
}
@@ -217,6 +220,62 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
}
impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRuntime<'a> {
fn with_updated_permission_request_input<'b>(
&'b self,
req: &'b UnifiedExecRequest,
updated_input: Value,
ctx: &'b ToolCtx,
approval_policy: AskForApproval,
) -> BoxFuture<'b, Result<UnifiedExecRequest, ToolError>> {
Box::pin(async move {
let command = updated_input
.get("command")
.and_then(Value::as_str)
.ok_or_else(|| {
ToolError::Rejected(
"PermissionRequest updatedInput must include string field `command`"
.to_string(),
)
})?;
let mut updated = req.clone();
updated.hook_command = command.to_string();
updated.justification = match updated_input.get("description") {
Some(Value::String(description)) => Some(description.clone()),
Some(_) => {
return Err(ToolError::Rejected(
"PermissionRequest updatedInput field `description` must be a string"
.to_string(),
));
}
None => None,
};
let Some(script) = updated.command.last_mut() else {
return Err(ToolError::Rejected(
"unified exec command args are empty".to_string(),
));
};
*script = command.to_string();
let file_system_sandbox_policy = ctx.turn.file_system_sandbox_policy();
updated.exec_approval_requirement = ctx
.session
.services
.exec_policy
.create_exec_approval_requirement_for_command(
crate::exec_policy::ExecApprovalRequest {
command: &updated.command,
approval_policy,
permission_profile: ctx.turn.permission_profile(),
file_system_sandbox_policy: &file_system_sandbox_policy,
sandbox_cwd: updated.cwd.as_path(),
sandbox_permissions: updated_exec_policy_sandbox_permissions(&updated),
prefix_rule: updated.prefix_rule.clone(),
},
)
.await;
Ok(updated)
})
}
fn sandbox_cwd<'b>(&self, req: &'b UnifiedExecRequest) -> Option<&'b AbsolutePathBuf> {
Some(&req.cwd)
}
@@ -358,6 +417,15 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
}
}
fn updated_exec_policy_sandbox_permissions(req: &UnifiedExecRequest) -> SandboxPermissions {
#[cfg(unix)]
if req.additional_permissions_preapproved {
return SandboxPermissions::UseDefault;
}
req.sandbox_permissions
}
#[cfg(test)]
mod tests {
use super::*;

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

@@ -31,6 +31,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use futures::Future;
use futures::future::BoxFuture;
use serde::Serialize;
use serde_json::Value;
use std::collections::HashMap;
use std::fmt::Debug;
use std::hash::Hash;
@@ -362,6 +363,26 @@ pub(crate) trait ToolRuntime<Req, Out>: Approvable<Req> + Sandboxable {
None
}
/// Rebuilds a typed request after a `PermissionRequest` hook replaces the
/// full hook-facing input object.
///
/// Implementations should only support this when they can faithfully
/// reconstruct the request that will later be evaluated by the normal
/// policy / guardian / user approval path.
fn with_updated_permission_request_input<'a>(
&'a self,
_req: &'a Req,
_updated_input: Value,
_ctx: &'a ToolCtx,
_approval_policy: AskForApproval,
) -> BoxFuture<'a, Result<Req, ToolError>> {
Box::pin(async {
Err(ToolError::Rejected(
"updatedInput is not supported for this PermissionRequest target".to_string(),
))
})
}
async fn run(
&mut self,
req: &Req,

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

@@ -1034,6 +1034,7 @@ impl UnifiedExecProcessManager {
#[cfg(unix)]
additional_permissions_preapproved: request.additional_permissions_preapproved,
justification: request.justification.clone(),
prefix_rule: request.prefix_rule.clone(),
exec_approval_requirement,
};
let tool_ctx = ToolCtx {
@@ -1045,7 +1046,7 @@ impl UnifiedExecProcessManager {
orchestrator
.run(
&mut runtime,
&req,
req,
&tool_ctx,
&context.turn,
context.turn.approval_policy.value(),

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

@@ -456,6 +456,16 @@ if mode == "allow":
"decision": {{"behavior": "allow"}}
}}
}}))
elif mode == "allow_update":
print(json.dumps({{
"hookSpecificOutput": {{
"hookEventName": "PermissionRequest",
"decision": {{
"behavior": "allow",
"updatedInput": {{"command": reason}}
}}
}}
}}))
elif mode == "deny":
print(json.dumps({{
"hookSpecificOutput": {{
@@ -1554,6 +1564,82 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() ->
Ok(())
}
#[tokio::test]
async fn permission_request_hook_rewrites_shell_command_before_normal_approval() -> 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-original-marker");
let original_command = format!("rm -f {}", original_marker.display());
let rewritten_command = "echo rewritten-by-permission-request".to_string();
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_command_for_hook = rewritten_command.clone();
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),
"allow_update",
&rewritten_command_for_hook,
) {
panic!("failed to write permission request hook test fixture: {error}");
}
})
.with_config(trust_discovered_hooks);
let test = builder.build(&server).await?;
fs::write(&original_marker, "seed").context("create original permission request marker")?;
test.submit_turn_with_approval_and_permission_profile(
"run the shell command after hook rewrite",
AskForApproval::OnRequest,
PermissionProfile::Disabled,
)
.await?;
let requests = responses.requests();
assert_eq!(requests.len(), 2);
let output_item = requests[1].function_call_output(call_id);
let output = output_item
.get("output")
.and_then(Value::as_str)
.expect("shell command output string");
assert!(
original_marker.exists(),
"original command should not execute after hook rewrite"
);
assert!(output.contains("rewritten-by-permission-request"));
assert_single_permission_request_hook_input(
test.codex_home_path(),
&original_command,
/*description*/ None,
)?;
Ok(())
}
#[tokio::test]
async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result<()> {
skip_if_no_network!(Ok(()));

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

@@ -37,7 +37,7 @@
},
"updatedInput": {
"default": null,
"description": "Reserved for a future input-rewrite capability.\n\nPermissionRequest hooks currently fail closed if this field is present."
"description": "Replacement input for the pending tool call when `behavior` is `allow`.\n\nThis replaces the full input object rather than merging into it."
},
"updatedPermissions": {
"default": null,

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

@@ -23,8 +23,12 @@ pub(crate) struct PreToolUseOutput {
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum PermissionRequestDecision {
Allow,
Deny { message: String },
Allow {
updated_input: Option<serde_json::Value>,
},
Deny {
message: String,
},
}
#[derive(Debug, Clone)]
@@ -315,8 +319,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 decision.updated_input.is_some()
&& !matches!(decision.behavior, PermissionRequestBehaviorWire::Allow)
{
Some("PermissionRequest hook returned updatedInput without behavior:allow".to_string())
} else if decision.updated_permissions.is_some() {
Some("PermissionRequest hook returned unsupported updatedPermissions".to_string())
} else if decision.interrupt {
@@ -330,7 +336,9 @@ fn permission_request_decision(
decision: &PermissionRequestDecisionWire,
) -> PermissionRequestDecision {
match decision.behavior {
PermissionRequestBehaviorWire::Allow => PermissionRequestDecision::Allow,
PermissionRequestBehaviorWire::Allow => PermissionRequestDecision::Allow {
updated_input: decision.updated_input.clone(),
},
PermissionRequestBehaviorWire::Deny => PermissionRequestDecision::Deny {
message: decision
.message
@@ -437,7 +445,7 @@ mod tests {
use super::parse_permission_request;
#[test]
fn permission_request_rejects_reserved_updated_input_field() {
fn permission_request_accepts_updated_input_for_allow() {
let parsed = parse_permission_request(
&json!({
"continue": true,
@@ -453,9 +461,36 @@ mod tests {
)
.expect("permission request hook output should parse");
assert_eq!(
parsed.decision,
Some(super::PermissionRequestDecision::Allow {
updated_input: Some(json!({})),
})
);
assert_eq!(parsed.invalid_reason, None);
}
#[test]
fn permission_request_rejects_updated_input_for_deny() {
let parsed = parse_permission_request(
&json!({
"continue": true,
"hookSpecificOutput": {
"hookEventName": "PermissionRequest",
"decision": {
"behavior": "deny",
"message": "blocked",
"updatedInput": {}
}
}
})
.to_string(),
)
.expect("permission request hook output should parse");
assert_eq!(
parsed.invalid_reason,
Some("PermissionRequest hook returned unsupported updatedInput".to_string())
Some("PermissionRequest hook returned updatedInput without behavior:allow".to_string())
);
}

34
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 pair an
//! allow decision with a one-shot input rewrite, or decline to decide and let
//! the normal approval flow continue.
//!
//! The event also mirrors the rest of the hook system's lifecycle:
//!
@@ -47,7 +47,7 @@ pub struct PermissionRequestRequest {
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum PermissionRequestDecision {
Allow,
Allow { updated_input: Option<Value> },
Deny { message: String },
}
@@ -154,8 +154,10 @@ fn resolve_permission_request_decision<'a>(
let mut resolved_allow = None;
for decision in decisions {
match decision {
PermissionRequestDecision::Allow => {
resolved_allow = Some(PermissionRequestDecision::Allow);
PermissionRequestDecision::Allow { updated_input } => {
resolved_allow = Some(PermissionRequestDecision::Allow {
updated_input: updated_input.clone(),
});
}
PermissionRequestDecision::Deny { message } => {
return Some(PermissionRequestDecision::Deny {
@@ -219,8 +221,8 @@ fn parse_completed(
});
} else if let Some(parsed_decision) = parsed.decision {
match parsed_decision {
output_parser::PermissionRequestDecision::Allow => {
decision = Some(PermissionRequestDecision::Allow);
output_parser::PermissionRequestDecision::Allow { updated_input } => {
decision = Some(PermissionRequestDecision::Allow { updated_input });
}
output_parser::PermissionRequestDecision::Deny { message } => {
status = HookRunStatus::Blocked;
@@ -294,7 +296,9 @@ mod tests {
#[test]
fn permission_request_deny_overrides_earlier_allow() {
let decisions = [
PermissionRequestDecision::Allow,
PermissionRequestDecision::Allow {
updated_input: None,
},
PermissionRequestDecision::Deny {
message: "repo deny".to_string(),
},
@@ -311,13 +315,19 @@ mod tests {
#[test]
fn permission_request_returns_allow_when_no_handler_denies() {
let decisions = [
PermissionRequestDecision::Allow,
PermissionRequestDecision::Allow,
PermissionRequestDecision::Allow {
updated_input: None,
},
PermissionRequestDecision::Allow {
updated_input: Some(serde_json::json!({"command": "echo rewritten"})),
},
];
assert_eq!(
resolve_permission_request_decision(decisions.iter()),
Some(PermissionRequestDecision::Allow)
Some(PermissionRequestDecision::Allow {
updated_input: Some(serde_json::json!({"command": "echo rewritten"})),
})
);
}

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

@@ -138,9 +138,9 @@ pub(crate) struct PermissionRequestHookSpecificOutputWire {
#[serde(deny_unknown_fields)]
pub(crate) struct PermissionRequestDecisionWire {
pub behavior: PermissionRequestBehaviorWire,
/// Reserved for a future input-rewrite capability.
/// Replacement input for the pending tool call when `behavior` is `allow`.
///
/// PermissionRequest hooks currently fail closed if this field is present.
/// This replaces the full input object rather than merging into it.
#[serde(default)]
pub updated_input: Option<Value>,
/// Reserved for a future permission-rewrite capability.