Simplify PermissionRequest input rewrites

This commit is contained in:
Abhinav Vedmala
2026-05-06 14:01:45 -07:00
parent 412d7d5b26
commit cb3b6fadef
15 changed files with 678 additions and 601 deletions
-2
View File
@@ -48,8 +48,6 @@ pub(crate) enum PreToolUseHookResult {
Blocked(String),
}
pub(crate) const MAX_HOOK_INPUT_REWRITES: usize = 8;
pub(crate) enum PendingInputHookDisposition {
Accepted(Box<PendingInputRecord>),
Blocked { additional_contexts: Vec<String> },
+22 -32
View File
@@ -25,7 +25,6 @@ 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::MAX_HOOK_INPUT_REWRITES;
use crate::hook_runtime::run_permission_request_hooks;
use crate::mcp_openai_file::rewrite_mcp_tool_arguments_for_openai_files;
use crate::mcp_tool_approval_templates::RenderedMcpToolApprovalParam;
@@ -203,43 +202,34 @@ pub(crate) async fn handle_mcp_tool_call(
.await;
let mut invocation = invocation;
let mut input_rewrites = 0;
loop {
let Some(decision) = maybe_request_mcp_tool_approval(
&sess,
turn_context,
&call_id,
&invocation,
&hook_tool_name,
metadata.as_ref(),
approval_mode,
)
.await
else {
break;
};
if let Some(decision) = maybe_request_mcp_tool_approval(
&sess,
turn_context,
&call_id,
&invocation,
&hook_tool_name,
metadata.as_ref(),
approval_mode,
)
.await
{
let tool_input = invocation
.arguments
.clone()
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new()));
let result = match decision {
McpToolApprovalDecision::AcceptWithUpdatedInput(updated_input) => {
input_rewrites += 1;
if input_rewrites > MAX_HOOK_INPUT_REWRITES {
notify_mcp_tool_call_skip(
sess.as_ref(),
turn_context.as_ref(),
&call_id,
invocation,
mcp_app_resource_uri.clone(),
"hook input rewrite limit exceeded".to_string(),
/*already_started*/ true,
)
.await
} else {
invocation.arguments = Some(updated_input);
continue;
}
invocation.arguments = Some(updated_input);
return handle_approved_mcp_tool_call(
sess.as_ref(),
turn_context.as_ref(),
&call_id,
invocation,
metadata.as_ref(),
request_meta,
mcp_app_resource_uri,
)
.await;
}
McpToolApprovalDecision::Accept
| McpToolApprovalDecision::AcceptForSession
+1
View File
@@ -1035,6 +1035,7 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an
&tool_ctx,
turn.as_ref(),
AskForApproval::Never,
crate::tools::sandboxing::InitialApprovalState::Evaluate,
)
.await
.expect("probe runtime should succeed");
+130 -114
View File
@@ -33,6 +33,7 @@ use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
use crate::tools::runtimes::apply_patch::ApplyPatchRequest;
use crate::tools::runtimes::apply_patch::ApplyPatchRuntime;
use crate::tools::sandboxing::InitialApprovalState;
use crate::tools::sandboxing::ToolCtx;
use codex_apply_patch::ApplyPatchAction;
use codex_apply_patch::ApplyPatchFileChange;
@@ -369,129 +370,143 @@ impl ToolHandler for ApplyPatchHandler {
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
tool_name,
payload,
..
} = invocation;
handle_apply_patch_invocation(invocation, InitialApprovalState::Evaluate).await
}
let patch_input = match payload {
ToolPayload::Function { arguments } => {
let args: ApplyPatchToolArgs = parse_arguments(&arguments)?;
args.input
}
ToolPayload::Custom { input } => input,
_ => {
return Err(FunctionCallError::RespondToModel(
"apply_patch handler received unsupported payload".to_string(),
));
}
};
async fn handle_after_permission_request_rewrite(
&self,
invocation: ToolInvocation,
) -> Result<Self::Output, FunctionCallError> {
handle_apply_patch_invocation(invocation, InitialApprovalState::AlreadyApproved).await
}
}
// Re-parse and verify the patch so we can compute changes and approval.
// Avoid building temporary ExecParams/command vectors; derive directly from inputs.
let cwd = turn.cwd.clone();
let command = vec!["apply_patch".to_string(), patch_input.clone()];
let Some(turn_environment) = turn.environments.primary() else {
async fn handle_apply_patch_invocation(
invocation: ToolInvocation,
initial_approval_state: InitialApprovalState,
) -> Result<ApplyPatchToolOutput, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
tool_name,
payload,
..
} = invocation;
let patch_input = match payload {
ToolPayload::Function { arguments } => {
let args: ApplyPatchToolArgs = parse_arguments(&arguments)?;
args.input
}
ToolPayload::Custom { input } => input,
_ => {
return Err(FunctionCallError::RespondToModel(
"apply_patch is unavailable in this session".to_string(),
"apply_patch handler received unsupported payload".to_string(),
));
};
let fs = turn_environment.environment.get_filesystem();
let sandbox = turn_environment
.environment
.is_remote()
.then(|| turn.file_system_sandbox_context(/*additional_permissions*/ None));
match codex_apply_patch::maybe_parse_apply_patch_verified(
&command,
&cwd,
fs.as_ref(),
sandbox.as_ref(),
)
.await
{
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
let (file_paths, effective_additional_permissions, file_system_sandbox_policy) =
effective_patch_permissions(session.as_ref(), turn.as_ref(), &changes).await;
match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes)
.await
{
InternalApplyPatchInvocation::Output(item) => {
let content = item?;
Ok(ApplyPatchToolOutput::from_text(content))
}
InternalApplyPatchInvocation::DelegateToRuntime(apply) => {
let changes = convert_apply_patch_to_protocol(&apply.action);
let emitter =
ToolEmitter::apply_patch(changes.clone(), apply.auto_approved);
let event_ctx = ToolEventCtx::new(
session.as_ref(),
turn.as_ref(),
&call_id,
Some(&tracker),
);
emitter.begin(event_ctx).await;
}
};
let req = ApplyPatchRequest {
action: apply.action,
file_paths,
changes,
exec_approval_requirement: apply.exec_approval_requirement,
additional_permissions: effective_additional_permissions
.additional_permissions,
permissions_preapproved: effective_additional_permissions
.permissions_preapproved,
};
// Re-parse and verify the patch so we can compute changes and approval.
// Avoid building temporary ExecParams/command vectors; derive directly from inputs.
let cwd = turn.cwd.clone();
let command = vec!["apply_patch".to_string(), patch_input.clone()];
let Some(turn_environment) = turn.environments.primary() else {
return Err(FunctionCallError::RespondToModel(
"apply_patch is unavailable in this session".to_string(),
));
};
let fs = turn_environment.environment.get_filesystem();
let sandbox = turn_environment
.environment
.is_remote()
.then(|| turn.file_system_sandbox_context(/*additional_permissions*/ None));
match codex_apply_patch::maybe_parse_apply_patch_verified(
&command,
&cwd,
fs.as_ref(),
sandbox.as_ref(),
)
.await
{
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
let (file_paths, effective_additional_permissions, file_system_sandbox_policy) =
effective_patch_permissions(session.as_ref(), turn.as_ref(), &changes).await;
match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes)
.await
{
InternalApplyPatchInvocation::Output(item) => {
let content = item?;
Ok(ApplyPatchToolOutput::from_text(content))
}
InternalApplyPatchInvocation::DelegateToRuntime(apply) => {
let changes = convert_apply_patch_to_protocol(&apply.action);
let emitter = ToolEmitter::apply_patch(changes.clone(), apply.auto_approved);
let event_ctx = ToolEventCtx::new(
session.as_ref(),
turn.as_ref(),
&call_id,
Some(&tracker),
);
emitter.begin(event_ctx).await;
let mut orchestrator = ToolOrchestrator::new();
let mut runtime = ApplyPatchRuntime::new();
let tool_ctx = ToolCtx {
session: session.clone(),
turn: turn.clone(),
call_id: call_id.clone(),
tool_name: tool_name.display(),
};
let out = orchestrator
.run(
&mut runtime,
&req,
&tool_ctx,
turn.as_ref(),
turn.approval_policy.value(),
)
.await
.map(|result| result.output);
let event_ctx = ToolEventCtx::new(
session.as_ref(),
let req = ApplyPatchRequest {
action: apply.action,
file_paths,
changes,
exec_approval_requirement: apply.exec_approval_requirement,
additional_permissions: effective_additional_permissions
.additional_permissions,
permissions_preapproved: effective_additional_permissions
.permissions_preapproved,
};
let mut orchestrator = ToolOrchestrator::new();
let mut runtime = ApplyPatchRuntime::new();
let tool_ctx = ToolCtx {
session: session.clone(),
turn: turn.clone(),
call_id: call_id.clone(),
tool_name: tool_name.display(),
};
let out = orchestrator
.run(
&mut runtime,
&req,
&tool_ctx,
turn.as_ref(),
&call_id,
Some(&tracker),
);
let content = emitter.finish(event_ctx, out).await?;
Ok(ApplyPatchToolOutput::from_text(content))
}
turn.approval_policy.value(),
initial_approval_state,
)
.await
.map(|result| result.output);
let event_ctx = ToolEventCtx::new(
session.as_ref(),
turn.as_ref(),
&call_id,
Some(&tracker),
);
let content = emitter.finish(event_ctx, out).await?;
Ok(ApplyPatchToolOutput::from_text(content))
}
}
codex_apply_patch::MaybeApplyPatchVerified::CorrectnessError(parse_error) => {
Err(FunctionCallError::RespondToModel(format!(
"apply_patch verification failed: {parse_error}"
)))
}
codex_apply_patch::MaybeApplyPatchVerified::ShellParseError(error) => {
tracing::trace!("Failed to parse apply_patch input, {error:?}");
Err(FunctionCallError::RespondToModel(
"apply_patch handler received invalid patch input".to_string(),
))
}
codex_apply_patch::MaybeApplyPatchVerified::NotApplyPatch => {
Err(FunctionCallError::RespondToModel(
"apply_patch handler received non-apply_patch input".to_string(),
))
}
}
codex_apply_patch::MaybeApplyPatchVerified::CorrectnessError(parse_error) => {
Err(FunctionCallError::RespondToModel(format!(
"apply_patch verification failed: {parse_error}"
)))
}
codex_apply_patch::MaybeApplyPatchVerified::ShellParseError(error) => {
tracing::trace!("Failed to parse apply_patch input, {error:?}");
Err(FunctionCallError::RespondToModel(
"apply_patch handler received invalid patch input".to_string(),
))
}
codex_apply_patch::MaybeApplyPatchVerified::NotApplyPatch => {
Err(FunctionCallError::RespondToModel(
"apply_patch handler received non-apply_patch input".to_string(),
))
}
}
}
@@ -570,6 +585,7 @@ pub(crate) async fn intercept_apply_patch(
&tool_ctx,
turn.as_ref(),
turn.approval_policy.value(),
InitialApprovalState::Evaluate,
)
.await
.map(|result| result.output);
+229 -149
View File
@@ -37,6 +37,7 @@ use crate::tools::registry::ToolKind;
use crate::tools::runtimes::shell::ShellRequest;
use crate::tools::runtimes::shell::ShellRuntime;
use crate::tools::runtimes::shell::ShellRuntimeBackend;
use crate::tools::sandboxing::InitialApprovalState;
use crate::tools::sandboxing::ToolCtx;
use codex_features::Feature;
use codex_protocol::models::AdditionalPermissionProfile;
@@ -130,6 +131,7 @@ struct RunExecLikeArgs {
call_id: String,
freeform: bool,
shell_runtime_backend: ShellRuntimeBackend,
initial_approval_state: InitialApprovalState,
}
impl ShellHandler {
@@ -155,6 +157,50 @@ impl ShellHandler {
arg0: None,
}
}
async fn handle_with_permission_request_hooks(
invocation: ToolInvocation,
initial_approval_state: InitialApprovalState,
) -> Result<FunctionToolOutput, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
payload,
..
} = invocation;
let arguments = match payload {
ToolPayload::Function { arguments } => arguments,
_ => {
return Err(FunctionCallError::RespondToModel(
"unsupported payload for shell handler".to_string(),
));
}
};
let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?;
let params: ShellToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?;
let prefix_rule = params.prefix_rule.clone();
let exec_params =
ShellHandler::to_exec_params(&params, turn.as_ref(), session.conversation_id);
ShellHandler::run_exec_like(RunExecLikeArgs {
tool_name: "shell".to_string(),
exec_params,
hook_command: codex_shell_command::parse_command::shlex_join(&params.command),
additional_permissions: params.additional_permissions.clone(),
prefix_rule,
session,
turn,
tracker,
call_id,
freeform: false,
shell_runtime_backend: ShellRuntimeBackend::Generic,
initial_approval_state,
})
.await
}
}
impl ShellCommandHandler {
@@ -210,6 +256,62 @@ impl ShellCommandHandler {
arg0: None,
})
}
async fn handle_with_permission_request_hooks(
&self,
invocation: ToolInvocation,
initial_approval_state: InitialApprovalState,
) -> Result<FunctionToolOutput, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
payload,
..
} = invocation;
let ToolPayload::Function { arguments } = payload else {
return Err(FunctionCallError::RespondToModel(format!(
"unsupported payload for shell_command handler: {}",
self.tool_name().display()
)));
};
let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?;
let params: ShellCommandToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?;
let workdir = turn.resolve_path(params.workdir.clone());
maybe_emit_implicit_skill_invocation(
session.as_ref(),
turn.as_ref(),
&params.command,
&workdir,
)
.await;
let prefix_rule = params.prefix_rule.clone();
let exec_params = Self::to_exec_params(
&params,
session.as_ref(),
turn.as_ref(),
session.conversation_id,
turn.tools_config.allow_login_shell,
)?;
ShellHandler::run_exec_like(RunExecLikeArgs {
tool_name: self.tool_name().display(),
exec_params,
hook_command: params.command,
additional_permissions: params.additional_permissions.clone(),
prefix_rule,
session,
turn,
tracker,
call_id,
freeform: true,
shell_runtime_backend: self.shell_runtime_backend(),
initial_approval_state,
})
.await
}
}
impl From<ShellCommandBackendConfig> for ShellCommandHandler {
@@ -268,42 +370,17 @@ impl ToolHandler for ShellHandler {
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
payload,
..
} = invocation;
Self::handle_with_permission_request_hooks(invocation, InitialApprovalState::Evaluate).await
}
let arguments = match payload {
ToolPayload::Function { arguments } => arguments,
_ => {
return Err(FunctionCallError::RespondToModel(
"unsupported payload for shell handler".to_string(),
));
}
};
let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?;
let params: ShellToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?;
let prefix_rule = params.prefix_rule.clone();
let exec_params =
ShellHandler::to_exec_params(&params, turn.as_ref(), session.conversation_id);
ShellHandler::run_exec_like(RunExecLikeArgs {
tool_name: "shell".to_string(),
exec_params,
hook_command: codex_shell_command::parse_command::shlex_join(&params.command),
additional_permissions: params.additional_permissions.clone(),
prefix_rule,
session,
turn,
tracker,
call_id,
freeform: false,
shell_runtime_backend: ShellRuntimeBackend::Generic,
})
async fn handle_after_permission_request_rewrite(
&self,
invocation: ToolInvocation,
) -> Result<Self::Output, FunctionCallError> {
Self::handle_with_permission_request_hooks(
invocation,
InitialApprovalState::AlreadyApproved,
)
.await
}
}
@@ -354,44 +431,69 @@ impl ToolHandler for ContainerExecHandler {
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
payload,
..
} = invocation;
let arguments = match payload {
ToolPayload::Function { arguments } => arguments,
_ => {
return Err(FunctionCallError::RespondToModel(
"unsupported payload for container.exec handler".to_string(),
));
}
};
let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?;
let params: ShellToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?;
let prefix_rule = params.prefix_rule.clone();
let exec_params =
ShellHandler::to_exec_params(&params, turn.as_ref(), session.conversation_id);
ShellHandler::run_exec_like(RunExecLikeArgs {
tool_name: "container.exec".to_string(),
exec_params,
hook_command: codex_shell_command::parse_command::shlex_join(&params.command),
additional_permissions: params.additional_permissions.clone(),
prefix_rule,
session,
turn,
tracker,
call_id,
freeform: false,
shell_runtime_backend: ShellRuntimeBackend::Generic,
})
handle_shell_function_invocation(
invocation,
"container.exec",
InitialApprovalState::Evaluate,
)
.await
}
async fn handle_after_permission_request_rewrite(
&self,
invocation: ToolInvocation,
) -> Result<Self::Output, FunctionCallError> {
handle_shell_function_invocation(
invocation,
"container.exec",
InitialApprovalState::AlreadyApproved,
)
.await
}
}
async fn handle_shell_function_invocation(
invocation: ToolInvocation,
tool_name: &str,
initial_approval_state: InitialApprovalState,
) -> Result<FunctionToolOutput, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
payload,
..
} = invocation;
let arguments = match payload {
ToolPayload::Function { arguments } => arguments,
_ => {
return Err(FunctionCallError::RespondToModel(format!(
"unsupported payload for {tool_name} handler"
)));
}
};
let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?;
let params: ShellToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?;
let prefix_rule = params.prefix_rule.clone();
let exec_params = ShellHandler::to_exec_params(&params, turn.as_ref(), session.conversation_id);
ShellHandler::run_exec_like(RunExecLikeArgs {
tool_name: tool_name.to_string(),
exec_params,
hook_command: codex_shell_command::parse_command::shlex_join(&params.command),
additional_permissions: params.additional_permissions.clone(),
prefix_rule,
session,
turn,
tracker,
call_id,
freeform: false,
shell_runtime_backend: ShellRuntimeBackend::Generic,
initial_approval_state,
})
.await
}
impl ToolHandler for LocalShellHandler {
@@ -478,38 +580,52 @@ impl ToolHandler for LocalShellHandler {
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
payload,
..
} = invocation;
let ToolPayload::LocalShell { params } = payload else {
return Err(FunctionCallError::RespondToModel(
"unsupported payload for local_shell handler".to_string(),
));
};
let exec_params =
ShellHandler::to_exec_params(&params, turn.as_ref(), session.conversation_id);
ShellHandler::run_exec_like(RunExecLikeArgs {
tool_name: "local_shell".to_string(),
exec_params,
hook_command: codex_shell_command::parse_command::shlex_join(&params.command),
additional_permissions: None,
prefix_rule: None,
session,
turn,
tracker,
call_id,
freeform: false,
shell_runtime_backend: ShellRuntimeBackend::Generic,
})
.await
handle_local_shell_invocation(invocation, InitialApprovalState::Evaluate).await
}
async fn handle_after_permission_request_rewrite(
&self,
invocation: ToolInvocation,
) -> Result<Self::Output, FunctionCallError> {
handle_local_shell_invocation(invocation, InitialApprovalState::AlreadyApproved).await
}
}
async fn handle_local_shell_invocation(
invocation: ToolInvocation,
initial_approval_state: InitialApprovalState,
) -> Result<FunctionToolOutput, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
payload,
..
} = invocation;
let ToolPayload::LocalShell { params } = payload else {
return Err(FunctionCallError::RespondToModel(
"unsupported payload for local_shell handler".to_string(),
));
};
let exec_params = ShellHandler::to_exec_params(&params, turn.as_ref(), session.conversation_id);
ShellHandler::run_exec_like(RunExecLikeArgs {
tool_name: "local_shell".to_string(),
exec_params,
hook_command: codex_shell_command::parse_command::shlex_join(&params.command),
additional_permissions: None,
prefix_rule: None,
session,
turn,
tracker,
call_id,
freeform: false,
shell_runtime_backend: ShellRuntimeBackend::Generic,
initial_approval_state,
})
.await
}
fn shell_function_pre_tool_use_payload(invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
@@ -614,54 +730,16 @@ impl ToolHandler for ShellCommandHandler {
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
payload,
..
} = invocation;
self.handle_with_permission_request_hooks(invocation, InitialApprovalState::Evaluate)
.await
}
let ToolPayload::Function { arguments } = payload else {
return Err(FunctionCallError::RespondToModel(format!(
"unsupported payload for shell_command handler: {}",
self.tool_name().display()
)));
};
let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?;
let params: ShellCommandToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?;
let workdir = turn.resolve_path(params.workdir.clone());
maybe_emit_implicit_skill_invocation(
session.as_ref(),
turn.as_ref(),
&params.command,
&workdir,
)
.await;
let prefix_rule = params.prefix_rule.clone();
let exec_params = Self::to_exec_params(
&params,
session.as_ref(),
turn.as_ref(),
session.conversation_id,
turn.tools_config.allow_login_shell,
)?;
ShellHandler::run_exec_like(RunExecLikeArgs {
tool_name: self.tool_name().display(),
exec_params,
hook_command: params.command,
additional_permissions: params.additional_permissions.clone(),
prefix_rule,
session,
turn,
tracker,
call_id,
freeform: true,
shell_runtime_backend: self.shell_runtime_backend(),
})
.await
async fn handle_after_permission_request_rewrite(
&self,
invocation: ToolInvocation,
) -> Result<Self::Output, FunctionCallError> {
self.handle_with_permission_request_hooks(invocation, InitialApprovalState::AlreadyApproved)
.await
}
}
@@ -679,6 +757,7 @@ impl ShellHandler {
call_id,
freeform,
shell_runtime_backend,
initial_approval_state,
} = args;
let mut exec_params = exec_params;
@@ -841,6 +920,7 @@ impl ShellHandler {
&tool_ctx,
&turn,
turn.approval_policy.value(),
initial_approval_state,
)
.await
.map(|result| result.output);
+205 -190
View File
@@ -21,6 +21,7 @@ use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
use crate::tools::sandboxing::InitialApprovalState;
use crate::unified_exec::ExecCommandRequest;
use crate::unified_exec::UnifiedExecContext;
use crate::unified_exec::UnifiedExecError;
@@ -196,208 +197,222 @@ impl ToolHandler for ExecCommandHandler {
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
payload,
..
} = invocation;
handle_exec_command_invocation(invocation, InitialApprovalState::Evaluate).await
}
let arguments = match payload {
ToolPayload::Function { arguments } => arguments,
_ => {
return Err(FunctionCallError::RespondToModel(
"exec_command handler received unsupported payload".to_string(),
));
}
};
async fn handle_after_permission_request_rewrite(
&self,
invocation: ToolInvocation,
) -> Result<Self::Output, FunctionCallError> {
handle_exec_command_invocation(invocation, InitialApprovalState::AlreadyApproved).await
}
}
let manager: &UnifiedExecProcessManager = &session.services.unified_exec_manager;
let context = UnifiedExecContext::new(session.clone(), turn.clone(), call_id.clone());
let environment_args: ExecCommandEnvironmentArgs = parse_arguments(&arguments)?;
let Some(turn_environment) =
resolve_tool_environment(turn.as_ref(), environment_args.environment_id.as_deref())?
else {
async fn handle_exec_command_invocation(
invocation: ToolInvocation,
initial_approval_state: InitialApprovalState,
) -> Result<ExecCommandToolOutput, FunctionCallError> {
let ToolInvocation {
session,
turn,
tracker,
call_id,
payload,
..
} = invocation;
let arguments = match payload {
ToolPayload::Function { arguments } => arguments,
_ => {
return Err(FunctionCallError::RespondToModel(
"unified exec is unavailable in this session".to_string(),
"exec_command handler received unsupported payload".to_string(),
));
};
let cwd = environment_args
.workdir
.as_deref()
.filter(|workdir| !workdir.is_empty())
.map_or_else(
|| turn_environment.cwd.clone(),
|workdir| turn_environment.cwd.join(workdir),
);
let environment = Arc::clone(&turn_environment.environment);
let fs = environment.get_filesystem();
let args: ExecCommandArgs = parse_arguments_with_base_path(&arguments, &cwd)?;
let hook_command = args.cmd.clone();
maybe_emit_implicit_skill_invocation(
session.as_ref(),
context.turn.as_ref(),
&hook_command,
&cwd,
)
.await;
let process_id = manager.allocate_process_id().await;
let command = get_command(
&args,
session.user_shell(),
&turn.tools_config.unified_exec_shell_mode,
turn.tools_config.allow_login_shell,
)
.map_err(FunctionCallError::RespondToModel)?;
let command_for_display = codex_shell_command::parse_command::shlex_join(&command);
let ExecCommandArgs {
tty,
yield_time_ms,
max_output_tokens,
sandbox_permissions,
additional_permissions,
justification,
prefix_rule,
..
} = args;
let max_output_tokens =
effective_max_output_tokens(max_output_tokens, turn.truncation_policy);
let exec_permission_approvals_enabled =
session.features().enabled(Feature::ExecPermissionApprovals);
let requested_additional_permissions = additional_permissions.clone();
let effective_additional_permissions = apply_granted_turn_permissions(
context.session.as_ref(),
cwd.as_path(),
sandbox_permissions,
additional_permissions,
)
.await;
let additional_permissions_allowed = exec_permission_approvals_enabled
|| (session.features().enabled(Feature::RequestPermissionsTool)
&& effective_additional_permissions.permissions_preapproved);
// Sticky turn permissions have already been approved, so they should
// continue through the normal exec approval flow for the command.
if effective_additional_permissions
.sandbox_permissions
.requests_sandbox_override()
&& !effective_additional_permissions.permissions_preapproved
&& !matches!(
context.turn.approval_policy.value(),
codex_protocol::protocol::AskForApproval::OnRequest
)
{
let approval_policy = context.turn.approval_policy.value();
manager.release_process_id(process_id).await;
return Err(FunctionCallError::RespondToModel(format!(
"approval policy is {approval_policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {approval_policy:?}"
)));
}
};
let normalized_additional_permissions = match implicit_granted_permissions(
sandbox_permissions,
requested_additional_permissions.as_ref(),
&effective_additional_permissions,
)
let manager: &UnifiedExecProcessManager = &session.services.unified_exec_manager;
let context = UnifiedExecContext::new(session.clone(), turn.clone(), call_id.clone());
let environment_args: ExecCommandEnvironmentArgs = parse_arguments(&arguments)?;
let Some(turn_environment) =
resolve_tool_environment(turn.as_ref(), environment_args.environment_id.as_deref())?
else {
return Err(FunctionCallError::RespondToModel(
"unified exec is unavailable in this session".to_string(),
));
};
let cwd = environment_args
.workdir
.as_deref()
.filter(|workdir| !workdir.is_empty())
.map_or_else(
|| {
normalize_and_validate_additional_permissions(
additional_permissions_allowed,
context.turn.approval_policy.value(),
effective_additional_permissions.sandbox_permissions,
effective_additional_permissions.additional_permissions,
effective_additional_permissions.permissions_preapproved,
&cwd,
)
},
|permissions| Ok(Some(permissions)),
) {
Ok(normalized) => normalized,
Err(err) => {
manager.release_process_id(process_id).await;
return Err(FunctionCallError::RespondToModel(err));
}
};
|| turn_environment.cwd.clone(),
|workdir| turn_environment.cwd.join(workdir),
);
let environment = Arc::clone(&turn_environment.environment);
let fs = environment.get_filesystem();
let args: ExecCommandArgs = parse_arguments_with_base_path(&arguments, &cwd)?;
let hook_command = args.cmd.clone();
maybe_emit_implicit_skill_invocation(
session.as_ref(),
context.turn.as_ref(),
&hook_command,
&cwd,
)
.await;
let process_id = manager.allocate_process_id().await;
let command = get_command(
&args,
session.user_shell(),
&turn.tools_config.unified_exec_shell_mode,
turn.tools_config.allow_login_shell,
)
.map_err(FunctionCallError::RespondToModel)?;
let command_for_display = codex_shell_command::parse_command::shlex_join(&command);
if let Some(output) = intercept_apply_patch(
&command,
&cwd,
fs.as_ref(),
context.session.clone(),
context.turn.clone(),
Some(&tracker),
&context.call_id,
"exec_command",
let ExecCommandArgs {
tty,
yield_time_ms,
max_output_tokens,
sandbox_permissions,
additional_permissions,
justification,
prefix_rule,
..
} = args;
let max_output_tokens = effective_max_output_tokens(max_output_tokens, turn.truncation_policy);
let exec_permission_approvals_enabled =
session.features().enabled(Feature::ExecPermissionApprovals);
let requested_additional_permissions = additional_permissions.clone();
let effective_additional_permissions = apply_granted_turn_permissions(
context.session.as_ref(),
cwd.as_path(),
sandbox_permissions,
additional_permissions,
)
.await;
let additional_permissions_allowed = exec_permission_approvals_enabled
|| (session.features().enabled(Feature::RequestPermissionsTool)
&& effective_additional_permissions.permissions_preapproved);
// Sticky turn permissions have already been approved, so they should
// continue through the normal exec approval flow for the command.
if effective_additional_permissions
.sandbox_permissions
.requests_sandbox_override()
&& !effective_additional_permissions.permissions_preapproved
&& !matches!(
context.turn.approval_policy.value(),
codex_protocol::protocol::AskForApproval::OnRequest
)
.await?
{
manager.release_process_id(process_id).await;
return Ok(ExecCommandToolOutput {
event_call_id: String::new(),
chunk_id: String::new(),
wall_time: std::time::Duration::ZERO,
raw_output: output.into_text().into_bytes(),
max_output_tokens: Some(max_output_tokens),
process_id: None,
exit_code: None,
original_token_count: None,
hook_command: None,
});
}
{
let approval_policy = context.turn.approval_policy.value();
manager.release_process_id(process_id).await;
return Err(FunctionCallError::RespondToModel(format!(
"approval policy is {approval_policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {approval_policy:?}"
)));
}
emit_unified_exec_tty_metric(&turn.session_telemetry, tty);
match manager
.exec_command(
ExecCommandRequest {
command,
hook_command: hook_command.clone(),
process_id,
yield_time_ms,
max_output_tokens: Some(max_output_tokens),
cwd,
environment,
network: context.turn.network.clone(),
tty,
sandbox_permissions: effective_additional_permissions.sandbox_permissions,
additional_permissions: normalized_additional_permissions,
additional_permissions_preapproved: effective_additional_permissions
.permissions_preapproved,
justification,
prefix_rule,
},
&context,
let normalized_additional_permissions = match implicit_granted_permissions(
sandbox_permissions,
requested_additional_permissions.as_ref(),
&effective_additional_permissions,
)
.map_or_else(
|| {
normalize_and_validate_additional_permissions(
additional_permissions_allowed,
context.turn.approval_policy.value(),
effective_additional_permissions.sandbox_permissions,
effective_additional_permissions.additional_permissions,
effective_additional_permissions.permissions_preapproved,
&cwd,
)
.await
{
Ok(response) => Ok(response),
Err(UnifiedExecError::UpdatedInput(updated_input)) => {
Err(FunctionCallError::UpdatedInput(updated_input))
}
Err(UnifiedExecError::SandboxDenied { output, .. }) => {
let output_text = output.aggregated_output.text;
let original_token_count = approx_token_count(&output_text);
Ok(ExecCommandToolOutput {
event_call_id: context.call_id.clone(),
chunk_id: generate_chunk_id(),
wall_time: output.duration,
raw_output: output_text.into_bytes(),
max_output_tokens: Some(max_output_tokens),
// Sandbox denial is terminal, so there is no live
// process for write_stdin to resume.
process_id: None,
exit_code: Some(output.exit_code),
original_token_count: Some(original_token_count),
hook_command: Some(hook_command),
})
}
Err(err) => Err(FunctionCallError::RespondToModel(format!(
"exec_command failed for `{command_for_display}`: {err:?}"
))),
},
|permissions| Ok(Some(permissions)),
) {
Ok(normalized) => normalized,
Err(err) => {
manager.release_process_id(process_id).await;
return Err(FunctionCallError::RespondToModel(err));
}
};
if let Some(output) = intercept_apply_patch(
&command,
&cwd,
fs.as_ref(),
context.session.clone(),
context.turn.clone(),
Some(&tracker),
&context.call_id,
"exec_command",
)
.await?
{
manager.release_process_id(process_id).await;
return Ok(ExecCommandToolOutput {
event_call_id: String::new(),
chunk_id: String::new(),
wall_time: std::time::Duration::ZERO,
raw_output: output.into_text().into_bytes(),
max_output_tokens: Some(max_output_tokens),
process_id: None,
exit_code: None,
original_token_count: None,
hook_command: None,
});
}
emit_unified_exec_tty_metric(&turn.session_telemetry, tty);
match manager
.exec_command(
ExecCommandRequest {
command,
hook_command: hook_command.clone(),
process_id,
yield_time_ms,
max_output_tokens: Some(max_output_tokens),
cwd,
environment,
network: context.turn.network.clone(),
tty,
sandbox_permissions: effective_additional_permissions.sandbox_permissions,
additional_permissions: normalized_additional_permissions,
additional_permissions_preapproved: effective_additional_permissions
.permissions_preapproved,
justification,
prefix_rule,
initial_approval_state,
},
&context,
)
.await
{
Ok(response) => Ok(response),
Err(UnifiedExecError::UpdatedInput(updated_input)) => {
Err(FunctionCallError::UpdatedInput(updated_input))
}
Err(UnifiedExecError::SandboxDenied { output, .. }) => {
let output_text = output.aggregated_output.text;
let original_token_count = approx_token_count(&output_text);
Ok(ExecCommandToolOutput {
event_call_id: context.call_id.clone(),
chunk_id: generate_chunk_id(),
wall_time: output.duration,
raw_output: output_text.into_bytes(),
max_output_tokens: Some(max_output_tokens),
// Sandbox denial is terminal, so there is no live
// process for write_stdin to resume.
process_id: None,
exit_code: Some(output.exit_code),
original_token_count: Some(original_token_count),
hook_command: Some(hook_command),
})
}
Err(err) => Err(FunctionCallError::RespondToModel(format!(
"exec_command failed for `{command_for_display}`: {err:?}"
))),
}
}
+57 -57
View File
@@ -20,6 +20,7 @@ use crate::tools::network_approval::finish_deferred_network_approval;
use crate::tools::network_approval::finish_immediate_network_approval;
use crate::tools::sandboxing::ApprovalCtx;
use crate::tools::sandboxing::ExecApprovalRequirement;
use crate::tools::sandboxing::InitialApprovalState;
use crate::tools::sandboxing::SandboxAttempt;
use crate::tools::sandboxing::SandboxOverride;
use crate::tools::sandboxing::ToolCtx;
@@ -130,6 +131,7 @@ impl ToolOrchestrator {
tool_ctx: &ToolCtx,
turn_ctx: &crate::session::turn_context::TurnContext,
approval_policy: AskForApproval,
initial_approval_state: InitialApprovalState,
) -> Result<OrchestratorRunResult<Out>, ToolError>
where
T: ToolRuntime<Rq, Out>,
@@ -141,23 +143,66 @@ impl ToolOrchestrator {
let use_guardian = routes_approval_to_guardian(turn_ctx) || strict_auto_review;
// 1) Approval
let mut already_approved = false;
let mut already_approved = matches!(
initial_approval_state,
InitialApprovalState::AlreadyApproved
);
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());
if !already_approved {
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 decision = Self::request_approval(
tool,
req,
tool_ctx.call_id.as_str(),
approval_ctx,
tool_ctx,
/*evaluate_permission_request_hooks*/ false,
&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,
);
}
}
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(
@@ -166,50 +211,16 @@ impl ToolOrchestrator {
tool_ctx.call_id.as_str(),
approval_ctx,
tool_ctx,
/*evaluate_permission_request_hooks*/ false,
/*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;
} else {
otel.tool_decision(
otel_tn,
otel_ci,
&ReviewDecision::Approved,
ToolDecisionSource::Config,
);
}
}
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.
@@ -398,7 +409,6 @@ impl ToolOrchestrator {
if evaluate_permission_request_hooks
&& let Some(permission_request) = tool.permission_request_payload(req)
{
let permission_request_for_noop_check = permission_request.clone();
match run_permission_request_hooks(
approval_ctx.session,
approval_ctx.turn,
@@ -410,17 +420,7 @@ impl ToolOrchestrator {
Some(PermissionRequestDecision::Allow {
updated_input: Some(updated_input),
}) => {
if !permission_request_for_noop_check.updated_input_is_noop(&updated_input) {
return Err(ToolError::UpdatedInput(updated_input));
}
let decision = ReviewDecision::Approved;
otel.tool_decision(
&tool_ctx.tool_name,
&tool_ctx.call_id,
&decision,
ToolDecisionSource::Config,
);
return Ok(decision);
return Err(ToolError::UpdatedInput(updated_input));
}
Some(PermissionRequestDecision::Allow {
updated_input: None,
+19 -15
View File
@@ -5,7 +5,6 @@ use std::time::Instant;
use crate::function_tool::FunctionCallError;
use crate::goals::GoalRuntimeEvent;
use crate::hook_runtime::MAX_HOOK_INPUT_REWRITES;
use crate::hook_runtime::record_additional_contexts;
use crate::hook_runtime::run_post_tool_use_hooks;
use crate::hook_runtime::run_pre_tool_use_hooks;
@@ -107,6 +106,18 @@ pub trait ToolHandler: Send + Sync {
&self,
invocation: ToolInvocation,
) -> impl std::future::Future<Output = Result<Self::Output, FunctionCallError>> + Send;
/// Continue execution after a `PermissionRequest` hook rewrites input.
///
/// Implementations that route through approval-time hooks should override
/// this to reuse the hook verdict instead of evaluating `PermissionRequest`
/// again for the rewritten payload.
fn handle_after_permission_request_rewrite(
&self,
invocation: ToolInvocation,
) -> impl std::future::Future<Output = Result<Self::Output, FunctionCallError>> + Send {
self.handle(invocation)
}
}
/// Consumes streamed argument diffs for a tool call and emits protocol events
@@ -230,21 +241,14 @@ where
) -> BoxFuture<'a, Result<AnyToolResult, FunctionCallError>> {
Box::pin(async move {
let mut invocation = invocation;
let mut rewrites = 0;
let output = loop {
match self.handle(invocation.clone()).await {
Err(FunctionCallError::UpdatedInput(updated_input)) => {
rewrites += 1;
if rewrites > MAX_HOOK_INPUT_REWRITES {
return Err(FunctionCallError::RespondToModel(
"hook input rewrite limit exceeded".to_string(),
));
}
invocation =
ToolHandler::with_updated_hook_input(self, invocation, updated_input)?;
}
result => break result?,
let output = match self.handle(invocation.clone()).await {
Err(FunctionCallError::UpdatedInput(updated_input)) => {
invocation =
ToolHandler::with_updated_hook_input(self, invocation, updated_input)?;
ToolHandler::handle_after_permission_request_rewrite(self, invocation.clone())
.await?
}
result => result?,
};
let call_id = invocation.call_id.clone();
let payload = invocation.payload.clone();
+6 -8
View File
@@ -154,14 +154,6 @@ impl PermissionRequestPayload {
tool_input: serde_json::Value::Object(tool_input),
}
}
pub(crate) fn updated_input_is_noop(&self, updated_input: &serde_json::Value) -> bool {
if self.tool_name.name() == "Bash" {
return self.tool_input.get("command") == updated_input.get("command");
}
self.tool_input == *updated_input
}
}
// Specifies what tool orchestrator should do with a given tool call.
@@ -355,6 +347,12 @@ pub(crate) struct ToolCtx {
pub tool_name: String,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum InitialApprovalState {
Evaluate,
AlreadyApproved,
}
#[derive(Debug)]
pub(crate) enum ToolError {
Rejected(String),
@@ -34,30 +34,6 @@ fn bash_permission_request_payload_includes_description_when_present() {
);
}
#[test]
fn bash_updated_input_noop_ignores_description() {
let payload = PermissionRequestPayload::bash(
"echo hi".to_string(),
Some("network-access example.com".to_string()),
);
assert!(payload.updated_input_is_noop(&json!({ "command": "echo hi" })));
}
#[test]
fn non_bash_updated_input_noop_requires_full_equality() {
let payload = PermissionRequestPayload {
tool_name: HookToolName::apply_patch(),
tool_input: json!({ "command": "patch" }),
};
assert!(payload.updated_input_is_noop(&json!({ "command": "patch" })));
assert!(!payload.updated_input_is_noop(&json!({
"command": "patch",
"description": "ignored"
})));
}
#[test]
fn external_sandbox_skips_exec_approval_on_request() {
let sandbox_policy = SandboxPolicy::ExternalSandbox {
+2
View File
@@ -39,6 +39,7 @@ use crate::sandboxing::SandboxPermissions;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::tools::network_approval::DeferredNetworkApproval;
use crate::tools::sandboxing::InitialApprovalState;
mod async_watcher;
mod errors;
@@ -103,6 +104,7 @@ pub(crate) struct ExecCommandRequest {
pub additional_permissions_preapproved: bool,
pub justification: Option<String>,
pub prefix_rule: Option<Vec<String>>,
pub initial_approval_state: InitialApprovalState,
}
#[derive(Debug)]
@@ -1052,6 +1052,7 @@ impl UnifiedExecProcessManager {
&tool_ctx,
&context.turn,
context.turn.approval_policy.value(),
request.initial_approval_state,
)
.await
.map(|result| (result.output, result.deferred_network_approval))
@@ -1,4 +1,5 @@
use super::*;
use crate::tools::sandboxing::InitialApprovalState;
use pretty_assertions::assert_eq;
use tokio::time::Duration;
use tokio::time::Instant;
@@ -187,6 +188,7 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() {
additional_permissions_preapproved: false,
justification: None,
prefix_rule: None,
initial_approval_state: InitialApprovalState::Evaluate,
};
let transcript = Arc::new(tokio::sync::Mutex::new(HeadTailBuffer::default()));
+1 -7
View File
@@ -1838,19 +1838,13 @@ async fn permission_request_hook_rewrites_apply_patch_before_execution() -> Resu
);
let hook_inputs = read_permission_request_hook_inputs(test.codex_home_path())?;
assert_eq!(hook_inputs.len(), 2);
assert_eq!(hook_inputs.len(), 1);
assert_permission_request_hook_input(
&hook_inputs[0],
"apply_patch",
&original_patch,
/*description*/ None,
);
assert_permission_request_hook_input(
&hook_inputs[1],
"apply_patch",
&rewritten_patch,
/*description*/ None,
);
Ok(())
}
@@ -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
//! replacing the full tool input object on allow, or decline to decide and let
//! the normal approval flow continue.
//!
//! The event also mirrors the rest of the hook system's lifecycle:
//!