From 99458d2929298809dff88a5aac7d7862d2be2ae4 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Sun, 12 Apr 2026 12:40:24 -0700 Subject: [PATCH] Include approval context in PermissionRequest hooks Co-authored-by: Codex --- codex-rs/core/src/hook_runtime.rs | 11 ++- codex-rs/core/src/tools/orchestrator.rs | 63 +++++++++---- codex-rs/core/src/tools/runtimes/shell.rs | 7 +- .../core/src/tools/runtimes/unified_exec.rs | 7 +- codex-rs/core/src/tools/sandboxing.rs | 14 ++- codex-rs/core/tests/suite/hooks.rs | 28 +++--- ...rmission-request.command.input.schema.json | 88 +++++++++++++++++++ .../hooks/src/events/permission_request.rs | 41 ++++++--- codex-rs/hooks/src/schema.rs | 12 +++ 9 files changed, 217 insertions(+), 54 deletions(-) diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 6ce74467e5..4393bf4039 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -26,6 +26,7 @@ use serde_json::Value; use crate::codex::Session; use crate::codex::TurnContext; use crate::event_mapping::parse_turn_item; +use crate::tools::sandboxing::PermissionRequestPayload; pub(crate) struct HookRuntimeOutcome { pub should_stop: bool, @@ -152,8 +153,7 @@ pub(crate) async fn run_permission_request_hooks( sess: &Arc, turn_context: &Arc, run_id_suffix: String, - tool_name: String, - command: String, + payload: PermissionRequestPayload, ) -> Option { let request = PermissionRequestRequest { session_id: sess.conversation_id, @@ -162,9 +162,12 @@ pub(crate) async fn run_permission_request_hooks( transcript_path: sess.hook_transcript_path().await, model: turn_context.model_info.slug.clone(), permission_mode: hook_permission_mode(turn_context), - tool_name, + tool_name: payload.tool_name, run_id_suffix, - command, + command: payload.command, + sandbox_permissions: payload.sandbox_permissions, + additional_permissions: payload.additional_permissions, + justification: payload.justification, }; let preview_runs = sess.hooks().preview_permission_request(&request); emit_hook_started_events(sess, turn_context, preview_runs).await; diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index b2322245e5..eb3005411a 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -46,6 +46,18 @@ pub(crate) struct OrchestratorRunResult { pub deferred_network_approval: Option, } +#[derive(Clone, Copy)] +enum ApprovalAttempt { + Initial, + Retry, +} + +struct ApprovalTelemetry<'a> { + otel: &'a SessionTelemetry, + tool_name: &'a str, + call_id: &'a str, +} + impl ToolOrchestrator { pub fn new() -> Self { Self { @@ -150,11 +162,14 @@ impl ToolOrchestrator { let decision = Self::request_approval( tool, req, + ApprovalAttempt::Initial, approval_ctx, turn_ctx, - &otel, - otel_tn, - otel_ci, + ApprovalTelemetry { + otel: &otel, + tool_name: otel_tn, + call_id: otel_ci, + }, ) .await?; @@ -311,11 +326,14 @@ impl ToolOrchestrator { let decision = Self::request_approval( tool, req, + ApprovalAttempt::Retry, approval_ctx, turn_ctx, - &otel, - otel_tn, - otel_ci, + ApprovalTelemetry { + otel: &otel, + tool_name: otel_tn, + call_id: otel_ci, + }, ) .await?; @@ -388,38 +406,40 @@ impl ToolOrchestrator { async fn request_approval( tool: &mut T, req: &Rq, + approval_attempt: ApprovalAttempt, approval_ctx: ApprovalCtx<'_>, turn_ctx: &crate::codex::TurnContext, - otel: &SessionTelemetry, - otel_tn: &str, - otel_ci: &str, + telemetry: ApprovalTelemetry<'_>, ) -> Result where T: ToolRuntime, { if let Some(permission_request) = tool.permission_request_payload(req) { + let run_id_suffix = match approval_attempt { + ApprovalAttempt::Initial => format!("{}:initial", approval_ctx.call_id), + ApprovalAttempt::Retry => format!("{}:retry", approval_ctx.call_id), + }; match run_permission_request_hooks( approval_ctx.session, approval_ctx.turn, - approval_ctx.call_id.to_string(), - permission_request.tool_name, - permission_request.command, + run_id_suffix, + permission_request, ) .await { Some(PermissionRequestDecision::Allow) => { - otel.tool_decision( - otel_tn, - otel_ci, + telemetry.otel.tool_decision( + telemetry.tool_name, + telemetry.call_id, &ReviewDecision::Approved, ToolDecisionSource::Config, ); return Ok(ReviewDecision::Approved); } Some(PermissionRequestDecision::Deny { message }) => { - otel.tool_decision( - otel_tn, - otel_ci, + telemetry.otel.tool_decision( + telemetry.tool_name, + telemetry.call_id, &ReviewDecision::Denied, ToolDecisionSource::Config, ); @@ -435,7 +455,12 @@ impl ToolOrchestrator { } else { ToolDecisionSource::User }; - otel.tool_decision(otel_tn, otel_ci, &decision, otel_source); + telemetry.otel.tool_decision( + telemetry.tool_name, + telemetry.call_id, + &decision, + otel_source, + ); Ok(decision) } } diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index d3cfe75189..735c83385a 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -200,7 +200,12 @@ impl Approvable for ShellRuntime { } fn permission_request_payload(&self, req: &ShellRequest) -> Option { - Some(PermissionRequestPayload::bash(req.hook_command.clone())) + Some(PermissionRequestPayload::bash( + req.hook_command.clone(), + req.sandbox_permissions, + req.additional_permissions.clone(), + req.justification.clone(), + )) } fn sandbox_mode_for_first_attempt(&self, req: &ShellRequest) -> SandboxOverride { diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 2d3d4a67c2..c5998c6e75 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -183,7 +183,12 @@ impl Approvable for UnifiedExecRuntime<'_> { &self, req: &UnifiedExecRequest, ) -> Option { - Some(PermissionRequestPayload::bash(req.hook_command.clone())) + Some(PermissionRequestPayload::bash( + req.hook_command.clone(), + req.sandbox_permissions, + req.additional_permissions.clone(), + req.justification.clone(), + )) } fn sandbox_mode_for_first_attempt(&self, req: &UnifiedExecRequest) -> SandboxOverride { diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index ce32faa079..c5d0392219 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -14,6 +14,7 @@ use codex_network_proxy::NetworkProxy; use codex_protocol::approvals::ExecPolicyAmendment; use codex_protocol::approvals::NetworkApprovalContext; use codex_protocol::error::CodexErr; +use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemSandboxKind; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; @@ -135,13 +136,24 @@ pub(crate) struct ApprovalCtx<'a> { pub(crate) struct PermissionRequestPayload { pub tool_name: String, pub command: String, + pub sandbox_permissions: SandboxPermissions, + pub additional_permissions: Option, + pub justification: Option, } impl PermissionRequestPayload { - pub(crate) fn bash(command: String) -> Self { + pub(crate) fn bash( + command: String, + sandbox_permissions: SandboxPermissions, + additional_permissions: Option, + justification: Option, + ) -> Self { Self { tool_name: "Bash".to_string(), command, + sandbox_permissions, + additional_permissions, + justification, } } } diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 1825a44699..b09c3db98e 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -1095,7 +1095,7 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() -> let server = start_mock_server().await; let call_id = "permissionrequest-shell-command"; let marker = std::env::temp_dir().join("permissionrequest-shell-command-marker"); - let command = format!("printf allowed > {}", marker.display()); + let command = format!("rm -f {}", marker.display()); let args = serde_json::json!({ "command": command }); let responses = mount_sse_sequence( &server, @@ -1137,9 +1137,7 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() -> }); let test = builder.build(&server).await?; - if marker.exists() { - fs::remove_file(&marker).context("remove leftover permission request marker")?; - } + fs::write(&marker, "seed").context("create permission request marker")?; test.submit_turn_with_policies( "run the shell command after hook approval", @@ -1150,18 +1148,10 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() -> 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"); + requests[1].function_call_output(call_id); assert!( - output.contains("allowed"), - "shell command output should reach the model after hook approval", - ); - assert!( - marker.exists(), - "approved command should create marker file" + !marker.exists(), + "approved command should remove marker file" ); let hook_inputs = read_permission_request_hook_inputs(test.codex_home_path())?; @@ -1169,6 +1159,14 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() -> assert_eq!(hook_inputs[0]["hook_event_name"], "PermissionRequest"); assert_eq!(hook_inputs[0]["tool_name"], "Bash"); assert_eq!(hook_inputs[0]["tool_input"]["command"], command); + assert_eq!( + hook_inputs[0]["approval_context"], + serde_json::json!({ + "sandbox_permissions": "use_default", + "additional_permissions": null, + "justification": null, + }) + ); assert!( hook_inputs[0].get("tool_use_id").is_none(), "PermissionRequest input should not include a tool_use_id", diff --git a/codex-rs/hooks/schema/generated/permission-request.command.input.schema.json b/codex-rs/hooks/schema/generated/permission-request.command.input.schema.json index 9bdcacf0cb..0e4e1f58c0 100644 --- a/codex-rs/hooks/schema/generated/permission-request.command.input.schema.json +++ b/codex-rs/hooks/schema/generated/permission-request.command.input.schema.json @@ -2,12 +2,70 @@ "$schema": "http://json-schema.org/draft-07/schema#", "additionalProperties": false, "definitions": { + "AbsolutePathBuf": { + "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", + "type": "string" + }, + "FileSystemPermissions": { + "properties": { + "read": { + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "write": { + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + } + }, + "type": "object" + }, + "NetworkPermissions": { + "properties": { + "enabled": { + "type": "boolean" + } + }, + "type": "object" + }, "NullableString": { "type": [ "string", "null" ] }, + "PermissionProfile": { + "properties": { + "file_system": { + "$ref": "#/definitions/FileSystemPermissions" + }, + "network": { + "$ref": "#/definitions/NetworkPermissions" + } + }, + "type": "object" + }, + "PermissionRequestApprovalContext": { + "additionalProperties": false, + "properties": { + "additional_permissions": { + "$ref": "#/definitions/PermissionProfile" + }, + "justification": { + "type": "string" + }, + "sandbox_permissions": { + "$ref": "#/definitions/SandboxPermissions" + } + }, + "required": [ + "sandbox_permissions" + ], + "type": "object" + }, "PermissionRequestToolInput": { "additionalProperties": false, "properties": { @@ -19,9 +77,38 @@ "command" ], "type": "object" + }, + "SandboxPermissions": { + "description": "Controls the per-command sandbox override requested by a shell-like tool call.", + "oneOf": [ + { + "description": "Run with the turn's configured sandbox policy unchanged.", + "enum": [ + "use_default" + ], + "type": "string" + }, + { + "description": "Request to run outside the sandbox.", + "enum": [ + "require_escalated" + ], + "type": "string" + }, + { + "description": "Request to stay in the sandbox while widening permissions for this command only.", + "enum": [ + "with_additional_permissions" + ], + "type": "string" + } + ] } }, "properties": { + "approval_context": { + "$ref": "#/definitions/PermissionRequestApprovalContext" + }, "cwd": { "type": "string" }, @@ -61,6 +148,7 @@ } }, "required": [ + "approval_context", "cwd", "hook_event_name", "model", diff --git a/codex-rs/hooks/src/events/permission_request.rs b/codex-rs/hooks/src/events/permission_request.rs index 62a3265f7c..c7d5deec42 100644 --- a/codex-rs/hooks/src/events/permission_request.rs +++ b/codex-rs/hooks/src/events/permission_request.rs @@ -1,6 +1,8 @@ use std::path::PathBuf; use codex_protocol::ThreadId; +use codex_protocol::models::PermissionProfile; +use codex_protocol::models::SandboxPermissions; use codex_protocol::protocol::HookCompletedEvent; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::HookOutputEntry; @@ -14,6 +16,7 @@ use crate::engine::ConfiguredHandler; use crate::engine::command_runner::CommandRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; +use crate::schema::PermissionRequestApprovalContext; use crate::schema::PermissionRequestCommandInput; use crate::schema::PermissionRequestToolInput; @@ -28,6 +31,9 @@ pub struct PermissionRequestRequest { pub tool_name: String, pub run_id_suffix: String, pub command: String, + pub sandbox_permissions: SandboxPermissions, + pub additional_permissions: Option, + pub justification: Option, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -83,19 +89,7 @@ pub(crate) async fn run( }; } - let input_json = match serde_json::to_string(&PermissionRequestCommandInput { - session_id: request.session_id.to_string(), - turn_id: request.turn_id.clone(), - transcript_path: crate::schema::NullableString::from_path(request.transcript_path.clone()), - cwd: request.cwd.display().to_string(), - hook_event_name: "PermissionRequest".to_string(), - model: request.model.clone(), - permission_mode: request.permission_mode.clone(), - tool_name: "Bash".to_string(), - tool_input: PermissionRequestToolInput { - command: request.command.clone(), - }, - }) { + let input_json = match serde_json::to_string(&build_command_input(&request)) { Ok(input_json) => input_json, Err(error) => { let hook_events = common::serialization_failure_hook_events_for_tool_use( @@ -136,6 +130,27 @@ pub(crate) async fn run( } } +fn build_command_input(request: &PermissionRequestRequest) -> PermissionRequestCommandInput { + PermissionRequestCommandInput { + session_id: request.session_id.to_string(), + turn_id: request.turn_id.clone(), + transcript_path: crate::schema::NullableString::from_path(request.transcript_path.clone()), + cwd: request.cwd.display().to_string(), + hook_event_name: "PermissionRequest".to_string(), + model: request.model.clone(), + permission_mode: request.permission_mode.clone(), + tool_name: "Bash".to_string(), + tool_input: PermissionRequestToolInput { + command: request.command.clone(), + }, + approval_context: PermissionRequestApprovalContext { + sandbox_permissions: request.sandbox_permissions, + additional_permissions: request.additional_permissions.clone(), + justification: request.justification.clone(), + }, + } +} + fn parse_completed( handler: &ConfiguredHandler, run_result: CommandRunResult, diff --git a/codex-rs/hooks/src/schema.rs b/codex-rs/hooks/src/schema.rs index 51a635f761..27c1e2cc91 100644 --- a/codex-rs/hooks/src/schema.rs +++ b/codex-rs/hooks/src/schema.rs @@ -12,6 +12,9 @@ use serde_json::Value; use std::path::Path; use std::path::PathBuf; +use codex_protocol::models::PermissionProfile; +use codex_protocol::models::SandboxPermissions; + const GENERATED_DIR: &str = "generated"; const POST_TOOL_USE_INPUT_FIXTURE: &str = "post-tool-use.command.input.schema.json"; const POST_TOOL_USE_OUTPUT_FIXTURE: &str = "post-tool-use.command.output.schema.json"; @@ -235,6 +238,14 @@ pub(crate) struct PermissionRequestToolInput { pub command: String, } +#[derive(Debug, Clone, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] +pub(crate) struct PermissionRequestApprovalContext { + pub sandbox_permissions: SandboxPermissions, + pub additional_permissions: Option, + pub justification: Option, +} + #[derive(Debug, Clone, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] #[schemars(rename = "permission-request.command.input")] @@ -252,6 +263,7 @@ pub(crate) struct PermissionRequestCommandInput { #[schemars(schema_with = "permission_request_tool_name_schema")] pub tool_name: String, pub tool_input: PermissionRequestToolInput, + pub approval_context: PermissionRequestApprovalContext, } #[derive(Debug, Clone, Serialize, JsonSchema)]