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

Split out PermissionRequest rewrites

Browse Source
This commit is contained in:
Abhinav Vedmala
2026-05-06 11:08:56 -07:00
parent a4176794dc
commit 2f423b129d
20 changed files with 87 additions and 594 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
codex-rs/core/src/function_tool.rs
View File

@@ -4,8 +4,6 @@ use thiserror::Error;
pub enum FunctionCallError {
#[error("{0}")]
RespondToModel(String),
#[error("tool input rewritten by hook")]
UpdatedInput(serde_json::Value),
#[error("LocalShellCall without call_id or id")]
MissingLocalShellCallId,
#[error("Fatal error: {0}")]

2
codex-rs/core/src/hook_runtime.rs
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> },

87
codex-rs/core/src/mcp_tool_call.rs
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;
@@ -34,6 +33,7 @@ use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::tools::hook_names::HookToolName;
use crate::tools::sandboxing::PermissionRequestPayload;
use crate::turn_metadata::McpTurnMetadataContext;
use codex_analytics::AppInvocation;
use codex_analytics::InvocationType;
use codex_analytics::build_track_events_context;
@@ -197,45 +197,18 @@ 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;
};
let tool_input = invocation
.arguments
.clone()
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new()));
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 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;
}
}
McpToolApprovalDecision::Accept
| McpToolApprovalDecision::AcceptForSession
| McpToolApprovalDecision::AcceptAndRemember => {
@@ -302,7 +275,8 @@ pub(crate) async fn handle_mcp_tool_call(
return HandledMcpToolCall {
result: CallToolResult::from_result(result),
tool_input,
tool_input: arguments_value
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new())),
};
}
@@ -841,7 +815,6 @@ async fn maybe_track_codex_app_used(
#[derive(Debug, Clone, PartialEq, Eq)]
enum McpToolApprovalDecision {
Accept,
AcceptWithUpdatedInput(JsonValue),
AcceptForSession,
AcceptAndRemember,
Decline { message: Option<String> },
@@ -923,7 +896,13 @@ fn build_mcp_tool_call_request_meta(
) -> Option<serde_json::Value> {
let mut request_meta = serde_json::Map::new();
if let Some(turn_metadata) = turn_context.turn_metadata_state.current_meta_value() {
if let Some(turn_metadata) = turn_context
.turn_metadata_state
.current_meta_value_for_mcp_request(McpTurnMetadataContext {
model: turn_context.model_info.slug.as_str(),
reasoning_effort: turn_context.effective_reasoning_effort(),
})
{
request_meta.insert(
crate::X_CODEX_TURN_METADATA_HEADER.to_string(),
turn_metadata,
@@ -1099,34 +1078,21 @@ async fn maybe_request_mcp_tool_approval(
return Some(McpToolApprovalDecision::Accept);
}
let permission_request_tool_input = invocation
.arguments
.clone()
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new()));
match run_permission_request_hooks(
sess,
turn_context,
call_id,
PermissionRequestPayload {
tool_name: HookToolName::new(hook_tool_name),
tool_input: permission_request_tool_input.clone(),
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),
}) => {
if updated_input == permission_request_tool_input {
return Some(McpToolApprovalDecision::Accept);
}
return Some(McpToolApprovalDecision::AcceptWithUpdatedInput(
updated_input,
));
}
Some(PermissionRequestDecision::Allow {
updated_input: None,
}) => {
Some(PermissionRequestDecision::Allow) => {
return Some(McpToolApprovalDecision::Accept);
}
Some(PermissionRequestDecision::Deny { message }) => {
@@ -1880,7 +1846,6 @@ async fn apply_mcp_tool_approval_decision(
}
}
McpToolApprovalDecision::Accept
| McpToolApprovalDecision::AcceptWithUpdatedInput(_)
| McpToolApprovalDecision::Decline { .. }
| McpToolApprovalDecision::Cancel
| McpToolApprovalDecision::BlockedBySafetyMonitor(_) => {}

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

@@ -4,6 +4,7 @@ use crate::session::tests::make_session_and_context;
use crate::session::tests::make_session_and_context_with_rx;
use crate::state::ActiveTurn;
use crate::test_support::models_manager_with_provider;
use crate::turn_metadata::McpTurnMetadataContext;
use codex_config::CONFIG_TOML_FILE;
use codex_config::config_toml::ConfigToml;
use codex_config::types::AppConfig;
@@ -71,6 +72,13 @@ fn approval_metadata(
}
}
fn mcp_turn_metadata_context(turn_context: &TurnContext) -> McpTurnMetadataContext<'_> {
McpTurnMetadataContext {
model: turn_context.model_info.slug.as_str(),
reasoning_effort: turn_context.effective_reasoning_effort(),
}
}
fn write_sample_plugin_mcp(codex_home: &std::path::Path) {
let plugin_root = codex_home.join("plugins/cache/test/sample/local");
std::fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir");
@@ -920,13 +928,10 @@ fn truncate_mcp_tool_result_for_event_bounds_large_error() {
#[tokio::test]
async fn mcp_tool_call_request_meta_includes_turn_metadata_for_custom_server() {
let (_, turn_context) = make_session_and_context().await;
let expected_turn_metadata = serde_json::from_str::<serde_json::Value>(
&turn_context
.turn_metadata_state
.current_header_value()
.expect("turn metadata header"),
)
.expect("turn metadata json");
let expected_turn_metadata = turn_context
.turn_metadata_state
.current_meta_value_for_mcp_request(mcp_turn_metadata_context(&turn_context))
.expect("turn metadata");
let meta = build_mcp_tool_call_request_meta(
&turn_context,
@@ -935,6 +940,25 @@ async fn mcp_tool_call_request_meta_includes_turn_metadata_for_custom_server() {
/*metadata*/ None,
)
.expect("custom servers should receive turn metadata");
let turn_metadata = meta
.get(crate::X_CODEX_TURN_METADATA_HEADER)
.expect("turn metadata should be present");
assert_eq!(
turn_metadata
.get("model")
.and_then(serde_json::Value::as_str),
Some(turn_context.model_info.slug.as_str())
);
assert_eq!(
turn_metadata
.get("reasoning_effort")
.and_then(serde_json::Value::as_str),
turn_context
.effective_reasoning_effort()
.map(|effort| effort.to_string())
.as_deref()
);
assert_eq!(
meta,
@@ -973,13 +997,10 @@ async fn mcp_tool_call_request_meta_includes_turn_started_at_unix_ms() {
#[tokio::test]
async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps_meta() {
let (_, turn_context) = make_session_and_context().await;
let expected_turn_metadata = serde_json::from_str::<serde_json::Value>(
&turn_context
.turn_metadata_state
.current_header_value()
.expect("turn metadata header"),
)
.expect("turn metadata json");
let expected_turn_metadata = turn_context
.turn_metadata_state
.current_meta_value_for_mcp_request(mcp_turn_metadata_context(&turn_context))
.expect("turn metadata");
let metadata = McpToolApprovalMetadata {
annotations: None,
connector_id: Some("calendar".to_string()),
@@ -1023,13 +1044,10 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps
#[tokio::test]
async fn codex_apps_tool_call_request_meta_includes_call_id_without_existing_codex_apps_meta() {
let (_, turn_context) = make_session_and_context().await;
let expected_turn_metadata = serde_json::from_str::<serde_json::Value>(
&turn_context
.turn_metadata_state
.current_header_value()
.expect("turn metadata header"),
)
.expect("turn metadata json");
let expected_turn_metadata = turn_context
.turn_metadata_state
.current_meta_value_for_mcp_request(mcp_turn_metadata_context(&turn_context))
.expect("turn metadata");
assert_eq!(
build_mcp_tool_call_request_meta(
@@ -2082,80 +2100,6 @@ async fn permission_request_hook_allows_mcp_tool_call() {
);
}
#[tokio::test]
async fn permission_request_hook_can_update_mcp_tool_input() {
let (mut session, turn_context) = make_session_and_context().await;
install_mcp_permission_request_hook(
&mut session,
&turn_context,
"mcp__memory__.*",
&serde_json::json!({
"hookSpecificOutput": {
"hookEventName": "PermissionRequest",
"decision": {
"behavior": "allow",
"updatedInput": {
"entities": [{
"name": "Grace",
"entityType": "person"
}]
}
}
}
}),
);
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let invocation = McpInvocation {
server: "memory".to_string(),
tool: "create_entities".to_string(),
arguments: Some(serde_json::json!({
"entities": [{
"name": "Ada",
"entityType": "person"
}]
})),
};
let metadata = McpToolApprovalMetadata {
annotations: Some(annotations(
Some(false),
Some(true),
/*open_world*/ None,
)),
connector_id: None,
connector_name: None,
connector_description: None,
tool_title: Some("Create entities".to_string()),
tool_description: None,
mcp_app_resource_uri: None,
codex_apps_meta: None,
openai_file_input_params: None,
};
let decision = maybe_request_mcp_tool_approval(
&session,
&turn_context,
"call-mcp-hook",
&invocation,
"mcp__memory__create_entities",
Some(&metadata),
AppToolApproval::Auto,
)
.await;
assert_eq!(
decision,
Some(McpToolApprovalDecision::AcceptWithUpdatedInput(
serde_json::json!({
"entities": [{
"name": "Grace",
"entityType": "person"
}]
})
))
);
}
#[tokio::test]
async fn permission_request_hook_uses_hook_tool_name_without_metadata() {
let (mut session, turn_context) = make_session_and_context().await;

5
codex-rs/core/src/stream_events_utils.rs
View File

@@ -336,11 +336,6 @@ pub(crate) async fn handle_output_item_done(
output.needs_follow_up = true;
}
Err(FunctionCallError::UpdatedInput(_)) => {
return Err(CodexErr::Fatal(
"updated tool input escaped dispatch".to_string(),
));
}
// A fatal error occurred; surface it back into history.
Err(FunctionCallError::Fatal(message)) => {
return Err(CodexErr::Fatal(message));

3
codex-rs/core/src/tools/events.rs
View File

@@ -353,9 +353,6 @@ impl ToolEmitter {
let result = Err(FunctionCallError::RespondToModel(normalized));
(event, result)
}
Err(ToolError::UpdatedInput(updated_input)) => {
return Err(FunctionCallError::UpdatedInput(updated_input));
}
};
self.emit(ctx, event).await;
result

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

@@ -372,9 +372,6 @@ impl ToolHandler for ExecCommandHandler {
.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);

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

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

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

@@ -398,7 +398,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,
@@ -407,24 +406,7 @@ impl ToolOrchestrator {
)
.await
{
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);
}
Some(PermissionRequestDecision::Allow {
updated_input: None,
}) => {
Some(PermissionRequestDecision::Allow) => {
let decision = ReviewDecision::Approved;
otel.tool_decision(
&tool_ctx.tool_name,

19
codex-rs/core/src/tools/registry.rs
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;
@@ -229,23 +228,7 @@ where
invocation: ToolInvocation,
) -> 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 = self.handle(invocation.clone()).await?;
let call_id = invocation.call_id.clone();
let payload = invocation.payload.clone();
let post_tool_use_payload =

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

@@ -420,21 +420,7 @@ impl CoreShellActionProvider {
)
.await
{
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::Allow {
updated_input: None,
}) => {
Some(PermissionRequestDecision::Allow) => {
return PromptDecision {
decision: ReviewDecision::Approved,
guardian_review_id: None,

9
codex-rs/core/src/tools/sandboxing.rs
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.
@@ -358,7 +350,6 @@ pub(crate) struct ToolCtx {
#[derive(Debug)]
pub(crate) enum ToolError {
Rejected(String),
UpdatedInput(serde_json::Value),
Codex(CodexErr),
}

24
codex-rs/core/src/tools/sandboxing_tests.rs
View File

@@ -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 {

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

@@ -1,5 +1,4 @@
use codex_protocol::exec_output::ExecToolCallOutput;
use serde_json::Value;
use thiserror::Error;
#[derive(Debug, Error)]
@@ -24,8 +23,6 @@ pub(crate) enum UnifiedExecError {
message: String,
output: ExecToolCallOutput,
},
#[error("tool input rewritten by hook")]
UpdatedInput(Value),
}
impl UnifiedExecError {

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

@@ -221,9 +221,6 @@ async fn finish_deferred_network_approval_for_session(
fn network_approval_error_message(err: ToolError) -> String {
match err {
ToolError::Rejected(message) => message,
ToolError::UpdatedInput(_) => {
"updatedInput is not supported for deferred network approvals".to_string()
}
ToolError::Codex(err) => err.to_string(),
}
}
@@ -1066,9 +1063,6 @@ impl UnifiedExecProcessManager {
};
UnifiedExecError::sandbox_denied(message, output)
}
ToolError::UpdatedInput(updated_input) => {
UnifiedExecError::UpdatedInput(updated_input)
}
other => UnifiedExecError::create_process(format!("{other:?}")),
})
}

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

@@ -496,56 +496,6 @@ elif mode == "exit_2":
Ok(())
}
fn write_updating_permission_request_hook(
home: &Path,
matcher: &str,
updated_input: &Value,
) -> Result<()> {
let script_path = home.join("permission_request_hook.py");
let log_path = home.join("permission_request_hook_log.jsonl");
let updated_input_json =
serde_json::to_string(updated_input).context("serialize updated permission input")?;
let script = format!(
r#"import json
from pathlib import Path
import sys
payload = json.load(sys.stdin)
with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
handle.write(json.dumps(payload) + "\n")
print(json.dumps({{
"hookSpecificOutput": {{
"hookEventName": "PermissionRequest",
"decision": {{
"behavior": "allow",
"updatedInput": {updated_input_json}
}}
}}
}}))
"#,
log_path = log_path.display(),
updated_input_json = updated_input_json,
);
let hooks = serde_json::json!({
"hooks": {
"PermissionRequest": [{
"matcher": matcher,
"hooks": [{
"type": "command",
"command": format!("python3 {}", script_path.display()),
"statusMessage": "rewriting permission request input",
}]
}]
}
});
fs::write(&script_path, script).context("write updating permission hook script")?;
fs::write(home.join("hooks.json"), hooks.to_string()).context("write hooks.json")?;
Ok(())
}
fn install_allow_permission_request_hook(home: &Path) -> Result<()> {
write_permission_request_hook(
home,
@@ -1603,92 +1553,6 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() ->
Ok(())
}
#[tokio::test]
async fn permission_request_hook_rewrites_shell_command_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");
let rewritten_marker = std::env::temp_dir().join("permissionrequest-shell-command-rewritten");
let original_command = format!("rm -f {}", original_marker.display());
let rewritten_command = format!("printf rewritten > {}", rewritten_marker.display());
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 hook rewrote it"),
ev_completed("resp-2"),
]),
],
)
.await;
let updated_input = serde_json::json!({ "command": rewritten_command.clone() });
let mut builder = test_codex()
.with_pre_build_hook(move |home| {
if let Err(error) =
write_updating_permission_request_hook(home, "^Bash$", &updated_input)
{
panic!("failed to write updating permission hook fixture: {error}");
}
})
.with_config(|config| {
trust_discovered_hooks(config);
});
let test = builder.build(&server).await?;
if original_marker.exists() {
fs::remove_file(&original_marker).context("remove stale original permission marker")?;
}
if rewritten_marker.exists() {
fs::remove_file(&rewritten_marker).context("remove stale rewritten permission marker")?;
}
fs::write(&original_marker, "seed").context("create original permission marker")?;
test.submit_turn_with_approval_and_permission_profile(
"run the rewritten shell command after hook approval",
AskForApproval::OnRequest,
PermissionProfile::Disabled,
)
.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 execute after permission rewrite"
);
assert_eq!(
fs::read_to_string(&rewritten_marker).context("read rewritten permission marker")?,
"rewritten"
);
let hook_inputs = read_permission_request_hook_inputs(test.codex_home_path())?;
assert_eq!(hook_inputs.len(), 1);
assert_permission_request_hook_input(
&hook_inputs[0],
"Bash",
&original_command,
/*description*/ None,
);
assert!(hook_inputs[0].get("tool_use_id").is_none());
Ok(())
}
#[tokio::test]
async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -1763,98 +1627,6 @@ async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result
Ok(())
}
#[tokio::test]
async fn permission_request_hook_rewrites_apply_patch_before_execution() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let call_id = "permissionrequest-apply-patch-rewrite";
let original_file = "permission_request_apply_patch_original.txt";
let rewritten_file = "permission_request_apply_patch_rewritten.txt";
let original_path = format!("../{original_file}");
let rewritten_path = format!("../{rewritten_file}");
let original_patch = format!(
r#"*** Begin Patch
*** Add File: {original_path}
+original
*** End Patch"#
);
let rewritten_patch = format!(
r#"*** Begin Patch
*** Add File: {rewritten_path}
+rewritten
*** End Patch"#
);
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_apply_patch_function_call(call_id, &original_patch),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "permission hook rewrote apply_patch"),
ev_completed("resp-2"),
]),
],
)
.await;
let updated_input = serde_json::json!({ "command": rewritten_patch.clone() });
let mut builder = test_codex()
.with_pre_build_hook(move |home| {
if let Err(error) =
write_updating_permission_request_hook(home, "^apply_patch$", &updated_input)
{
panic!("failed to write updating permission hook fixture: {error}");
}
})
.with_config(|config| {
config.include_apply_patch_tool = true;
trust_discovered_hooks(config);
});
let test = builder.build(&server).await?;
test.submit_turn_with_approval_and_permission_profile(
"apply the rewritten patch after hook approval",
AskForApproval::OnRequest,
restrictive_workspace_write_profile(),
)
.await?;
let requests = responses.requests();
assert_eq!(requests.len(), 2);
requests[1].function_call_output(call_id);
assert!(
!test.workspace_path(&original_path).exists(),
"original patch should not create its target file"
);
assert_eq!(
fs::read_to_string(test.workspace_path(&rewritten_path))
.context("read rewritten permission apply_patch file")?,
"rewritten\n"
);
let hook_inputs = read_permission_request_hook_inputs(test.codex_home_path())?;
assert_eq!(hook_inputs.len(), 2);
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(())
}
#[tokio::test]
async fn permission_request_hook_sees_raw_exec_command_input() -> 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": "Replaces the entire tool input object when `behavior` is `allow`."
"description": "Reserved for a future input-rewrite capability.\n\nPermissionRequest hooks currently fail closed if this field is present."
},
"updatedPermissions": {
"default": null,

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

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

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

@@ -47,7 +47,7 @@ pub struct PermissionRequestRequest {
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum PermissionRequestDecision {
Allow { updated_input: Option<Value> },
Allow,
Deny { message: String },
}
@@ -154,10 +154,8 @@ fn resolve_permission_request_decision<'a>(
let mut resolved_allow = None;
for decision in decisions {
match decision {
PermissionRequestDecision::Allow { updated_input } => {
resolved_allow = Some(PermissionRequestDecision::Allow {
updated_input: updated_input.clone(),
});
PermissionRequestDecision::Allow => {
resolved_allow = Some(PermissionRequestDecision::Allow);
}
PermissionRequestDecision::Deny { message } => {
return Some(PermissionRequestDecision::Deny {
@@ -221,8 +219,8 @@ fn parse_completed(
});
} else if let Some(parsed_decision) = parsed.decision {
match parsed_decision {
output_parser::PermissionRequestDecision::Allow { updated_input } => {
decision = Some(PermissionRequestDecision::Allow { updated_input });
output_parser::PermissionRequestDecision::Allow => {
decision = Some(PermissionRequestDecision::Allow);
}
output_parser::PermissionRequestDecision::Deny { message } => {
status = HookRunStatus::Blocked;
@@ -296,9 +294,7 @@ mod tests {
#[test]
fn permission_request_deny_overrides_earlier_allow() {
let decisions = [
PermissionRequestDecision::Allow {
updated_input: None,
},
PermissionRequestDecision::Allow,
PermissionRequestDecision::Deny {
message: "repo deny".to_string(),
},
@@ -315,38 +311,13 @@ mod tests {
#[test]
fn permission_request_returns_allow_when_no_handler_denies() {
let decisions = [
PermissionRequestDecision::Allow {
updated_input: None,
},
PermissionRequestDecision::Allow {
updated_input: None,
},
PermissionRequestDecision::Allow,
PermissionRequestDecision::Allow,
];
assert_eq!(
resolve_permission_request_decision(decisions.iter()),
Some(PermissionRequestDecision::Allow {
updated_input: None,
})
);
}
#[test]
fn permission_request_keeps_highest_precedence_allow_update() {
let decisions = [
PermissionRequestDecision::Allow {
updated_input: Some(serde_json::json!({ "command": "first" })),
},
PermissionRequestDecision::Allow {
updated_input: Some(serde_json::json!({ "command": "second" })),
},
];
assert_eq!(
resolve_permission_request_decision(decisions.iter()),
Some(PermissionRequestDecision::Allow {
updated_input: Some(serde_json::json!({ "command": "second" })),
})
Some(PermissionRequestDecision::Allow)
);
}

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

@@ -138,7 +138,9 @@ pub(crate) struct PermissionRequestHookSpecificOutputWire {
#[serde(deny_unknown_fields)]
pub(crate) struct PermissionRequestDecisionWire {
pub behavior: PermissionRequestBehaviorWire,
/// Replaces the entire tool input object when `behavior` is `allow`.
/// Reserved for a future input-rewrite capability.
///
/// PermissionRequest hooks currently fail closed if this field is present.
#[serde(default)]
pub updated_input: Option<Value>,
/// Reserved for a future permission-rewrite capability.