openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-05 20:10:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(approvals): centralize execve review routing

Browse Source
This commit is contained in:
Dylan Hurd
2026-07-07 15:33:31 -07:00
parent af3fd8b849
commit b0bd67617c
5 changed files with 64 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/approval_coordinator.rs
+4
View File
@@ -104,6 +104,10 @@ impl ApprovalReviewer {
Self::routes_to_guardian(turn, reviewer).then_some(Self::Guardian)
}
pub(crate) fn automatic_for_turn(turn: &TurnContext) -> Option<Self> {
Self::automatic_for_reviewer(turn, turn.config.approvals_reviewer)
}
fn routes_to_guardian(turn: &TurnContext, reviewer: ApprovalsReviewer) -> bool {
matches!(
turn.approval_policy.value(),
codex-rs/core/src/guardian/mod.rs
+1
View File
@@ -38,6 +38,7 @@ pub(crate) use review::record_guardian_denial_for_test;
pub(crate) use review::review_approval_request;
#[cfg(test)]
pub(crate) use review::review_approval_request_with_cancel;
#[cfg(test)]
pub(crate) use review::routes_approval_to_guardian;
#[cfg(test)]
pub(crate) use review::routes_approval_to_guardian_with_reviewer;
codex-rs/core/src/guardian/review.rs
+4
View File
@@ -5,7 +5,9 @@ use codex_analytics::GuardianReviewFailureReason;
use codex_analytics::GuardianReviewTerminalStatus;
use codex_analytics::GuardianReviewTrackContext;
use codex_analytics::GuardianReviewedAction;
#[cfg(test)]
use codex_protocol::config_types::ApprovalsReviewer;
#[cfg(test)]
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::CodexErrorInfo;
use codex_protocol::protocol::EventMsg;
@@ -165,11 +167,13 @@ fn guardian_risk_level_str(level: GuardianRiskLevel) -> &'static str {
/// Whether this turn should route allowed approval prompts through the guardian
/// reviewer instead of surfacing them to the user. ARC may still block actions
/// earlier in the flow.
#[cfg(test)]
pub(crate) fn routes_approval_to_guardian(turn: &TurnContext) -> bool {
routes_approval_to_guardian_with_reviewer(turn, turn.config.approvals_reviewer)
}
/// Whether an approval with its own reviewer selection should be routed through guardian.
#[cfg(test)]
pub(crate) fn routes_approval_to_guardian_with_reviewer(
turn: &TurnContext,
approvals_reviewer: ApprovalsReviewer,
codex-rs/core/src/session_startup_prewarm.rs
+2 -2
View File
@@ -9,8 +9,8 @@ use tracing::info;
use tracing::instrument;
use tracing::warn;
use crate::approval_coordinator::ApprovalReviewer;
use crate::client::ModelClientSession;
use crate::guardian::routes_approval_to_guardian;
use crate::responses_metadata::CodexResponsesRequestKind;
use crate::session::INITIAL_SUBMIT_ID;
use crate::session::session::Session;
@@ -251,7 +251,7 @@ async fn schedule_startup_prewarm_inner(
prewarm_started_at.elapsed(),
/*status*/ None,
);
if routes_approval_to_guardian(&startup_turn_context) {
if ApprovalReviewer::automatic_for_turn(&startup_turn_context).is_some() {
let guardian_session = Arc::clone(&session);
let guardian_parent_turn = Arc::clone(&startup_turn_context);
drop(tokio::spawn(async move {
codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
+53 -91
View File
@@ -1,15 +1,16 @@
use super::ShellRequest;
use crate::approval_coordinator::ApprovalAction;
use crate::approval_coordinator::ApprovalCoordinator;
use crate::approval_coordinator::ApprovalHookRequest;
use crate::approval_coordinator::ApprovalResolution;
use crate::approval_coordinator::ApprovalResolutionSource;
use crate::approval_coordinator::ApprovalReview;
use crate::approval_coordinator::ApprovalReviewer;
use crate::exec::ExecCapturePolicy;
use crate::exec::ExecExpiration;
use crate::exec::cancel_when_either;
use crate::exec::is_likely_sandbox_denied;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::guardian_rejection_message;
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::run_permission_request_hooks;
use crate::sandboxing::ExecOptions;
use crate::sandboxing::ExecRequest;
use crate::sandboxing::SandboxPermissions;
@@ -30,7 +31,6 @@ use codex_execpolicy::MatchOptions;
use codex_execpolicy::Policy;
use codex_execpolicy::RuleMatch;
use codex_features::Feature;
use codex_hooks::PermissionRequestDecision;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::error::CodexErr;
use codex_protocol::error::SandboxErr;
@@ -374,12 +374,6 @@ enum DecisionSource {
UnmatchedCommandFallback,
}
struct PromptDecision {
decision: ReviewDecision,
guardian_review_id: Option<String>,
rejection_message: Option<String>,
}
fn execve_prompt_is_rejected_by_policy(
approval_policy: AskForApproval,
decision_source: &DecisionSource,
@@ -446,7 +440,7 @@ impl CoreShellActionProvider {
workdir: &AbsolutePathBuf,
stopwatch: &Stopwatch,
additional_permissions: Option<AdditionalPermissionProfile>,
) -> anyhow::Result<PromptDecision> {
) -> anyhow::Result<ApprovalResolution> {
let command = join_program_and_argv(program, argv);
let workdir = workdir.clone();
let session = self.session.clone();
@@ -455,85 +449,52 @@ impl CoreShellActionProvider {
let approval_id = Some(Uuid::new_v4().to_string());
let environment_id = Some(self.environment_id.clone());
let source = self.tool_name;
let guardian_review_id = routes_approval_to_guardian(&turn).then(new_guardian_review_id);
Ok(stopwatch
.pause_for(async move {
// 1) Run PermissionRequest hooks
let permission_request = PermissionRequestPayload::bash(
codex_shell_command::parse_command::shlex_join(&command),
/*description*/ None,
);
let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone());
match run_permission_request_hooks(
ApprovalCoordinator::resolve_event_cancellable(
&session,
&turn,
&effective_approval_id,
permission_request,
)
.await
{
Some(PermissionRequestDecision::Allow) => {
return PromptDecision {
decision: ReviewDecision::Approved,
guardian_review_id: None,
rejection_message: None,
};
}
Some(PermissionRequestDecision::Deny { message }) => {
return PromptDecision {
decision: ReviewDecision::Denied,
guardian_review_id: None,
rejection_message: Some(message),
};
}
None => {}
}
// 2) Route to Guardian if configured
if let Some(review_id) = guardian_review_id.clone() {
let decision = review_approval_request(
&session,
&turn,
review_id.clone(),
GuardianApprovalRequest::Execve {
ApprovalReviewer::for_turn(&turn),
Some(ApprovalHookRequest {
run_id: &effective_approval_id,
payload: permission_request,
}),
ApprovalReview::main_turn_cancellable(
ApprovalAction::Execve {
id: call_id.clone(),
source,
program: program.to_string_lossy().into_owned(),
argv: argv.to_vec(),
cwd: workdir.clone(),
additional_permissions,
additional_permissions: additional_permissions.clone(),
},
/*retry_reason*/ None,
)
.await;
return PromptDecision {
decision,
guardian_review_id,
rejection_message: None,
};
}
// 3) Fall back to regular user prompt
let decision = session
.request_command_approval(
&turn,
call_id,
approval_id,
environment_id,
command,
workdir.clone(),
/*reason*/ None,
/*network_approval_context*/ None,
/*proposed_execpolicy_amendment*/ None,
additional_permissions,
Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]),
)
.await;
PromptDecision {
decision,
guardian_review_id: None,
rejection_message: None,
}
CancellationToken::new(),
),
|| async {
session
.request_command_approval(
&turn,
call_id,
approval_id,
environment_id,
command,
workdir,
/*reason*/ None,
/*network_approval_context*/ None,
/*proposed_execpolicy_amendment*/ None,
additional_permissions,
Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]),
)
.await
},
)
.await
})
.await)
}
@@ -588,22 +549,23 @@ impl CoreShellActionProvider {
}
},
ReviewDecision::Denied => {
let message = if let Some(message) =
prompt_decision.rejection_message.clone()
{
message
} else if let Some(review_id) =
prompt_decision.guardian_review_id.as_deref()
{
guardian_rejection_message(self.session.as_ref(), review_id).await
} else {
"User denied execution".to_string()
};
let message =
if prompt_decision.source == ApprovalResolutionSource::User {
"User denied execution".to_string()
} else {
prompt_decision
.rejection
.clone()
.unwrap_or_else(|| "Execution denied by policy".to_string())
};
EscalationDecision::deny(Some(message))
}
ReviewDecision::TimedOut => {
EscalationDecision::deny(Some(guardian_timeout_message()))
}
ReviewDecision::TimedOut => EscalationDecision::deny(Some(
prompt_decision
.rejection
.clone()
.unwrap_or_else(guardian_timeout_message),
)),
ReviewDecision::Abort => {
EscalationDecision::deny(Some("User cancelled execution".to_string()))
}