Add exec approval profile persistence core support

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Dylan Hurd
2026-03-22 14:35:19 -07:00
co-authored by Codex
parent 993274057e
commit c3df4c6a0a
17 changed files with 101 additions and 3 deletions
+34 -1
View File
@@ -2887,12 +2887,17 @@ impl Session {
},
]
});
let permissions_profile_persistence =
additional_permissions.as_ref().and_then(|permissions| {
persistence_target_for_permissions(turn_context.config.as_ref(), permissions)
});
let available_decisions = available_decisions.unwrap_or_else(|| {
ExecApprovalRequestEvent::default_available_decisions(
network_approval_context.as_ref(),
proposed_execpolicy_amendment.as_ref(),
proposed_network_policy_amendments.as_deref(),
additional_permissions.as_ref(),
permissions_profile_persistence.as_ref(),
)
});
let event = EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
@@ -2906,6 +2911,7 @@ impl Session {
proposed_execpolicy_amendment,
proposed_network_policy_amendments,
additional_permissions,
permissions_profile_persistence,
skill_metadata,
available_decisions: Some(available_decisions),
parsed_cmd,
@@ -4246,8 +4252,16 @@ async fn submission_loop(sess: Arc<Session>, config: Arc<Config>, rx_sub: Receiv
id: approval_id,
turn_id,
decision,
persist_permissions,
} => {
handlers::exec_approval(&sess, approval_id, turn_id, decision).await;
handlers::exec_approval(
&sess,
approval_id,
turn_id,
decision,
persist_permissions,
)
.await;
false
}
Op::PatchApproval { id, decision } => {
@@ -4640,6 +4654,7 @@ mod handlers {
approval_id: String,
turn_id: Option<String>,
decision: ReviewDecision,
persist_permissions: Option<PersistPermissionProfileAction>,
) {
let event_turn_id = turn_id.unwrap_or_else(|| approval_id.clone());
if let ReviewDecision::ApprovedExecpolicyAmendment {
@@ -4669,10 +4684,28 @@ mod handlers {
}
}
}
if matches!(
decision,
ReviewDecision::ApprovedPersistToProfile | ReviewDecision::Approved
) && let Some(action) = persist_permissions.as_ref()
&& let Err(err) = persist_permissions_for_profile(sess.as_ref(), action).await
{
let message = format!("Failed to update permissions profile: {err}");
tracing::warn!("{message}");
sess.send_event_raw(Event {
id: event_turn_id.clone(),
msg: EventMsg::Warning(WarningEvent { message }),
})
.await;
}
match decision {
ReviewDecision::Abort => {
sess.interrupt_task().await;
}
ReviewDecision::ApprovedPersistToProfile => {
sess.notify_approval(&approval_id, ReviewDecision::Approved)
.await;
}
other => sess.notify_approval(&approval_id, other).await,
}
}
+2
View File
@@ -493,6 +493,7 @@ async fn handle_exec_approval(
id: approval_id_for_op,
turn_id: Some(turn_id),
decision,
persist_permissions: None,
})
.await;
}
@@ -700,6 +701,7 @@ async fn maybe_auto_review_mcp_request_user_input(
.map(|option| option.label.clone())
.unwrap_or_else(|| MCP_TOOL_APPROVAL_ACCEPT.to_string()),
ReviewDecision::Approved
| ReviewDecision::ApprovedPersistToProfile
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::NetworkPolicyAmendment { .. } => MCP_TOOL_APPROVAL_ACCEPT.to_string(),
ReviewDecision::Denied | ReviewDecision::Abort => {
@@ -288,6 +288,7 @@ async fn handle_exec_approval_uses_call_id_for_guardian_review_and_approval_id_f
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
additional_permissions: None,
permissions_profile_persistence: None,
skill_metadata: None,
available_decisions: Some(vec![
ReviewDecision::Approved,
@@ -345,6 +346,7 @@ async fn handle_exec_approval_uses_call_id_for_guardian_review_and_approval_id_f
id: "callback-approval-1".to_string(),
turn_id: Some("child-turn-1".to_string()),
decision: ReviewDecision::Abort,
persist_permissions: None,
}
);
}
+1
View File
@@ -747,6 +747,7 @@ pub(crate) fn build_guardian_mcp_tool_review_request(
fn mcp_tool_approval_decision_from_guardian(decision: ReviewDecision) -> McpToolApprovalDecision {
match decision {
ReviewDecision::Approved
| ReviewDecision::ApprovedPersistToProfile
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::NetworkPolicyAmendment { .. } => McpToolApprovalDecision::Accept,
ReviewDecision::ApprovedForSession => McpToolApprovalDecision::AcceptForSession,
+3 -1
View File
@@ -392,7 +392,9 @@ impl NetworkApprovalService {
let mut cache_session_deny = false;
let resolved = match approval_decision {
ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } => {
ReviewDecision::Approved
| ReviewDecision::ApprovedPersistToProfile
| ReviewDecision::ApprovedExecpolicyAmendment { .. } => {
PendingApprovalDecision::AllowOnce
}
ReviewDecision::ApprovedForSession => PendingApprovalDecision::AllowForSession,
+2
View File
@@ -155,6 +155,7 @@ impl ToolOrchestrator {
return Err(ToolError::Rejected(reason));
}
ReviewDecision::Approved
| ReviewDecision::ApprovedPersistToProfile
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::ApprovedForSession => {}
ReviewDecision::NetworkPolicyAmendment {
@@ -309,6 +310,7 @@ impl ToolOrchestrator {
return Err(ToolError::Rejected(reason));
}
ReviewDecision::Approved
| ReviewDecision::ApprovedPersistToProfile
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::ApprovedForSession => {}
ReviewDecision::NetworkPolicyAmendment {
@@ -542,6 +542,7 @@ impl CoreShellActionProvider {
.await?
{
ReviewDecision::Approved
| ReviewDecision::ApprovedPersistToProfile
| ReviewDecision::ApprovedExecpolicyAmendment { .. } => {
if needs_escalation {
EscalationDecision::escalate(escalation_execution.clone())
+8
View File
@@ -1715,6 +1715,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> {
id: approval.effective_approval_id(),
turn_id: None,
decision: decision.clone(),
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -1941,6 +1942,7 @@ async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts
decision: ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: expected_execpolicy_amendment.clone(),
},
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -2180,6 +2182,7 @@ async fn spawned_subagent_execpolicy_amendment_propagates_to_parent_session() ->
decision: ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: expected_execpolicy_amendment,
},
persist_permissions: None,
})
.await?;
@@ -2405,6 +2408,7 @@ async fn approving_fallback_rule_for_compound_command_works() -> Result<()> {
decision: ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: amendment.clone(),
},
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -2599,6 +2603,7 @@ allow_local_binding = true
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
}
@@ -2639,6 +2644,7 @@ allow_local_binding = true
decision: ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment: deny_network_amendment.clone(),
},
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -2742,6 +2748,7 @@ allow_local_binding = true
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
}
@@ -2826,6 +2833,7 @@ async fn compound_command_with_one_safe_command_still_requires_approval() -> Res
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -103,6 +103,7 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await
.expect("submit exec approval");
+6
View File
@@ -1159,6 +1159,7 @@ async fn handle_container_exec_user_approved_records_tool_decision() {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await
.unwrap();
@@ -1225,6 +1226,7 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision()
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::ApprovedForSession,
persist_permissions: None,
})
.await
.unwrap();
@@ -1291,6 +1293,7 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await
.unwrap();
@@ -1357,6 +1360,7 @@ async fn handle_container_exec_user_denies_records_tool_decision() {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
persist_permissions: None,
})
.await
.unwrap();
@@ -1423,6 +1427,7 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision()
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::ApprovedForSession,
persist_permissions: None,
})
.await
.unwrap();
@@ -1490,6 +1495,7 @@ async fn handle_sandbox_error_user_denies_records_tool_decision() {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
persist_permissions: None,
})
.await
.unwrap();
@@ -378,6 +378,7 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -566,6 +567,7 @@ async fn relative_additional_permissions_resolve_against_tool_workdir() -> Resul
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -660,6 +662,7 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_cwd
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -761,6 +764,7 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_tmp
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -869,6 +873,7 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() ->
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -971,6 +976,7 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -1101,6 +1107,7 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -1212,6 +1219,7 @@ async fn request_permissions_preapprove_explicit_exec_permissions_outside_on_req
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -1326,6 +1334,7 @@ async fn request_permissions_grants_apply_to_later_shell_command_calls() -> Resu
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -1436,6 +1445,7 @@ async fn request_permissions_grants_apply_to_later_shell_command_calls_without_i
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -1606,6 +1616,7 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions()
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -1853,6 +1864,7 @@ async fn request_permissions_session_grants_carry_across_turns() -> Result<()> {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_completion(&test).await;
@@ -278,6 +278,7 @@ async fn approved_folder_write_request_permissions_unblocks_later_exec_without_s
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.await?;
wait_for_event(&test.codex, |event| {
@@ -257,6 +257,7 @@ permissions:
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
persist_permissions: None,
})
.await?;
@@ -351,6 +352,7 @@ permissions:
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
persist_permissions: None,
})
.await?;
@@ -446,6 +448,7 @@ permissions:
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
persist_permissions: None,
})
.await?;
@@ -895,6 +898,7 @@ async fn shell_zsh_fork_skill_session_approval_enforces_skill_permissions() -> R
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::ApprovedForSession,
persist_permissions: None,
})
.await?;
@@ -230,6 +230,7 @@ async fn run_codex_tool_session_inner(
additional_permissions: _,
skill_metadata: _,
available_decisions: _,
permissions_profile_persistence: _,
} = ev;
handle_exec_approval_request(
command,
+1
View File
@@ -139,6 +139,7 @@ async fn on_exec_approval_response(
id: approval_id,
turn_id: Some(event_id),
decision: response.decision,
persist_permissions: None,
})
.await
{
+14 -1
View File
@@ -10,6 +10,7 @@ use crate::permissions::NetworkSandboxPolicy;
use crate::protocol::FileChange;
use crate::protocol::ReviewDecision;
use crate::protocol::SandboxPolicy;
use crate::request_permissions::PermissionProfilePersistence;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;
@@ -181,6 +182,11 @@ pub struct ExecApprovalRequestEvent {
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub additional_permissions: Option<PermissionProfile>,
/// Optional named permissions profile that can absorb the requested
/// filesystem permissions for future approvals.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub permissions_profile_persistence: Option<PermissionProfilePersistence>,
/// Optional skill metadata when the approval was triggered by a skill script.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
@@ -212,6 +218,7 @@ impl ExecApprovalRequestEvent {
self.proposed_execpolicy_amendment.as_ref(),
self.proposed_network_policy_amendments.as_deref(),
self.additional_permissions.as_ref(),
self.permissions_profile_persistence.as_ref(),
),
}
}
@@ -221,6 +228,7 @@ impl ExecApprovalRequestEvent {
proposed_execpolicy_amendment: Option<&ExecPolicyAmendment>,
proposed_network_policy_amendments: Option<&[NetworkPolicyAmendment]>,
additional_permissions: Option<&PermissionProfile>,
permissions_profile_persistence: Option<&PermissionProfilePersistence>,
) -> Vec<ReviewDecision> {
if network_approval_context.is_some() {
let mut decisions = vec![ReviewDecision::Approved, ReviewDecision::ApprovedForSession];
@@ -238,7 +246,12 @@ impl ExecApprovalRequestEvent {
}
if additional_permissions.is_some() {
return vec![ReviewDecision::Approved, ReviewDecision::Abort];
let mut decisions = vec![ReviewDecision::Approved];
if permissions_profile_persistence.is_some() {
decisions.push(ReviewDecision::ApprovedPersistToProfile);
}
decisions.push(ReviewDecision::Abort);
return decisions;
}
let mut decisions = vec![ReviewDecision::Approved];
+8
View File
@@ -362,6 +362,9 @@ pub enum Op {
turn_id: Option<String>,
/// The user's decision in response to the request.
decision: ReviewDecision,
/// Optional permission-profile mutation to persist alongside approval.
#[serde(default, skip_serializing_if = "Option::is_none")]
persist_permissions: Option<PersistPermissionProfileAction>,
},
/// Approve a code patch
@@ -3178,6 +3181,10 @@ pub enum ReviewDecision {
/// remainder of the session.
ApprovedForSession,
/// User has approved this request and wants the filesystem permissions to
/// be persisted into the active named permissions profile.
ApprovedPersistToProfile,
/// User chose to persist a network policy rule (allow/deny) for future
/// requests to the same host.
NetworkPolicyAmendment {
@@ -3202,6 +3209,7 @@ impl ReviewDecision {
ReviewDecision::Approved => "approved",
ReviewDecision::ApprovedExecpolicyAmendment { .. } => "approved_with_amendment",
ReviewDecision::ApprovedForSession => "approved_for_session",
ReviewDecision::ApprovedPersistToProfile => "approved_persist_to_profile",
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
} => match network_policy_amendment.action {