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

Revert "Simplify approval routing abstraction"

Browse Source 
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Abhinav Vedmala
2026-04-13 12:31:35 -07:00
parent e55105c6e2
commit d9d15997a4
6 changed files with 332 additions and 169 deletions
Download Patch File Download Diff File Expand all files Collapse all files

165
codex-rs/core/src/tools/approval.rs
View File

@@ -13,10 +13,15 @@ use crate::guardian::new_guardian_review_id;
use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian;
use crate::state::SessionServices;
use codex_protocol::approvals::ExecPolicyAmendment;
use codex_protocol::approvals::NetworkApprovalContext;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::FileChange;
use codex_protocol::protocol::ReviewDecision;
use futures::Future;
use serde::Serialize;
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::Arc;
#[derive(Clone, Default, Debug)]
@@ -43,6 +48,70 @@ impl ApprovalStore {
}
}
#[derive(Debug)]
pub(crate) enum ApprovalCache<K> {
None,
SessionApproveOnly {
tool_name: &'static str,
keys: Vec<K>,
},
}
#[derive(Debug)]
pub(crate) struct ApprovalOutcome {
pub decision: ReviewDecision,
pub guardian_review_id: Option<String>,
}
#[derive(Debug)]
pub(crate) struct CommandApprovalRequest {
pub call_id: String,
pub approval_id: Option<String>,
pub command: Vec<String>,
pub cwd: PathBuf,
pub reason: Option<String>,
pub network_approval_context: Option<NetworkApprovalContext>,
pub proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
pub additional_permissions: Option<PermissionProfile>,
pub available_decisions: Option<Vec<ReviewDecision>>,
}
#[derive(Debug)]
pub(crate) struct PatchApprovalRequest {
pub call_id: String,
pub changes: HashMap<PathBuf, FileChange>,
pub reason: Option<String>,
pub grant_root: Option<PathBuf>,
}
#[derive(Debug)]
pub(crate) enum UserApprovalRequest {
Command(CommandApprovalRequest),
Patch(PatchApprovalRequest),
}
#[derive(Debug)]
pub(crate) struct GuardianApproval {
pub request: GuardianApprovalRequest,
pub retry_reason: Option<String>,
}
impl GuardianApproval {
pub(crate) fn new(request: GuardianApprovalRequest, retry_reason: Option<String>) -> Self {
Self {
request,
retry_reason,
}
}
}
#[derive(Debug)]
pub(crate) struct ApprovalPlan<K> {
pub cache: ApprovalCache<K>,
pub user: UserApprovalRequest,
pub guardian: GuardianApproval,
}
pub(crate) fn guardian_review_id_for_turn(turn: &crate::codex::TurnContext) -> Option<String> {
routes_approval_to_guardian(turn).then(new_guardian_review_id)
}
@@ -99,28 +168,94 @@ where
decision
}
pub(crate) async fn route_approval<K, F, Fut>(
async fn dispatch_user_approval(
session: &Arc<Session>,
turn: &Arc<TurnContext>,
request: UserApprovalRequest,
) -> ReviewDecision {
match request {
UserApprovalRequest::Command(request) => {
session
.request_command_approval(
turn.as_ref(),
request.call_id,
request.approval_id,
request.command,
request.cwd,
request.reason,
request.network_approval_context,
request.proposed_execpolicy_amendment,
request.additional_permissions,
request.available_decisions,
)
.await
}
UserApprovalRequest::Patch(request) => {
let rx_approve = session
.request_patch_approval(
turn.as_ref(),
request.call_id,
request.changes,
request.reason,
request.grant_root,
)
.await;
rx_approve.await.unwrap_or_default()
}
}
}
pub(crate) async fn request_approval<K>(
session: &Arc<Session>,
turn: &Arc<TurnContext>,
guardian_review_id: Option<String>,
cache: Option<(&'static str, Vec<K>)>,
guardian_request: GuardianApprovalRequest,
retry_reason: Option<String>,
user: F,
) -> ReviewDecision
plan: ApprovalPlan<K>,
) -> ApprovalOutcome
where
K: Serialize,
F: FnOnce() -> Fut,
Fut: Future<Output = ReviewDecision>,
{
if let Some(review_id) = guardian_review_id {
return review_approval_request(session, turn, review_id, guardian_request, retry_reason)
.await;
let ApprovalPlan {
cache,
user,
guardian,
} = plan;
if let Some(review_id) = guardian_review_id.clone() {
return ApprovalOutcome {
decision: review_approval_request(
session,
turn,
review_id,
guardian.request,
guardian.retry_reason,
)
.await,
guardian_review_id,
};
}
if let Some((tool_name, keys)) = cache {
with_cached_approval(&session.services, tool_name, keys, user).await
} else {
user().await
let decision = match cache {
ApprovalCache::None => dispatch_user_approval(session, turn, user).await,
ApprovalCache::SessionApproveOnly { tool_name, keys } => {
with_cached_approval(&session.services, tool_name, keys, || {
dispatch_user_approval(session, turn, user)
})
.await
}
};
ApprovalOutcome {
decision,
guardian_review_id: None,
}
}
pub(crate) async fn request_approval_for_turn<K>(
session: &Arc<Session>,
turn: &Arc<TurnContext>,
plan: ApprovalPlan<K>,
) -> ApprovalOutcome
where
K: Serialize,
{
request_approval(session, turn, guardian_review_id_for_turn(turn), plan).await
}

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

@@ -3,8 +3,12 @@ use crate::guardian::GuardianApprovalRequest;
use crate::guardian::guardian_rejection_message;
use crate::guardian::guardian_timeout_message;
use crate::network_policy_decision::denied_network_policy_message;
use crate::tools::approval::guardian_review_id_for_turn;
use crate::tools::approval::route_approval;
use crate::tools::approval::ApprovalCache;
use crate::tools::approval::ApprovalPlan;
use crate::tools::approval::CommandApprovalRequest;
use crate::tools::approval::GuardianApproval;
use crate::tools::approval::UserApprovalRequest;
use crate::tools::approval::request_approval_for_turn;
use crate::tools::sandboxing::ToolError;
use codex_network_proxy::BlockedRequest;
use codex_network_proxy::BlockedRequestObserver;
@@ -369,45 +373,41 @@ impl NetworkApprovalService {
protocol,
};
let owner_call = self.resolve_single_active_call().await;
let guardian_review_id = guardian_review_id_for_turn(&turn_context);
let approval_id = Self::approval_id_for_key(&key);
let user_session = Arc::clone(&session);
let user_turn_context = Arc::clone(&turn_context);
let user_network_approval_context = network_approval_context.clone();
let approval_decision = route_approval(
let approval_outcome = request_approval_for_turn(
&session,
&turn_context,
guardian_review_id.clone(),
None::<(&'static str, Vec<String>)>,
GuardianApprovalRequest::NetworkAccess {
id: approval_id.clone(),
turn_id: owner_call
.as_ref()
.map_or_else(|| turn_context.sub_id.clone(), |call| call.turn_id.clone()),
target: target.clone(),
host: request.host,
protocol,
port: key.port,
},
Some(policy_denial_message.clone()),
|| async move {
user_session
.request_command_approval(
user_turn_context.as_ref(),
approval_id,
/*approval_id*/ None,
vec!["network-access".to_string(), target],
user_turn_context.cwd.to_path_buf(),
Some(prompt_reason),
Some(user_network_approval_context),
/*proposed_execpolicy_amendment*/ None,
/*additional_permissions*/ None,
None,
)
.await
ApprovalPlan {
cache: ApprovalCache::<String>::None,
user: UserApprovalRequest::Command(CommandApprovalRequest {
call_id: Self::approval_id_for_key(&key),
approval_id: None,
command: vec!["network-access".to_string(), target.clone()],
cwd: turn_context.cwd.to_path_buf(),
reason: Some(prompt_reason),
network_approval_context: Some(network_approval_context.clone()),
proposed_execpolicy_amendment: None,
additional_permissions: None,
available_decisions: None,
}),
guardian: GuardianApproval::new(
GuardianApprovalRequest::NetworkAccess {
id: Self::approval_id_for_key(&key),
turn_id: owner_call.as_ref().map_or_else(
|| turn_context.sub_id.clone(),
|call| call.turn_id.clone(),
),
target,
host: request.host,
protocol,
port: key.port,
},
Some(policy_denial_message.clone()),
),
},
)
.await;
let guardian_review_id = approval_outcome.guardian_review_id;
let approval_decision = approval_outcome.decision;
let mut cache_session_deny = false;
let resolved = match approval_decision {

66
codex-rs/core/src/tools/runtimes/apply_patch.rs
View File

@@ -9,7 +9,12 @@ use crate::exec::ExecCapturePolicy;
use crate::guardian::GuardianApprovalRequest;
use crate::sandboxing::ExecOptions;
use crate::sandboxing::execute_env;
use crate::tools::approval::route_approval;
use crate::tools::approval::ApprovalCache;
use crate::tools::approval::ApprovalPlan;
use crate::tools::approval::GuardianApproval;
use crate::tools::approval::PatchApprovalRequest;
use crate::tools::approval::UserApprovalRequest;
use crate::tools::approval::request_approval;
use crate::tools::sandboxing::Approvable;
use crate::tools::sandboxing::ApprovalCtx;
use crate::tools::sandboxing::ExecApprovalRequirement;
@@ -150,46 +155,51 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
return ReviewDecision::Approved;
}
if let Some(reason) = retry_reason.clone() {
return route_approval(
return request_approval(
session,
turn,
ctx.guardian_review_id.clone(),
None::<(&'static str, Vec<AbsolutePathBuf>)>,
ApplyPatchRuntime::build_guardian_review_request(req, &call_id),
retry_reason,
|| async move {
let rx_approve = session
.request_patch_approval(
turn,
call_id,
changes.clone(),
Some(reason),
/*grant_root*/ None,
)
.await;
rx_approve.await.unwrap_or_default()
ApprovalPlan {
cache: ApprovalCache::<AbsolutePathBuf>::None,
user: UserApprovalRequest::Patch(PatchApprovalRequest {
call_id: call_id.clone(),
changes: changes.clone(),
reason: Some(reason),
grant_root: None,
}),
guardian: GuardianApproval::new(
ApplyPatchRuntime::build_guardian_review_request(req, &call_id),
retry_reason,
),
},
)
.await;
.await
.decision;
}
route_approval(
request_approval(
session,
turn,
ctx.guardian_review_id.clone(),
Some(("apply_patch", approval_keys)),
ApplyPatchRuntime::build_guardian_review_request(req, ctx.call_id),
retry_reason,
|| async move {
let rx_approve = session
.request_patch_approval(
turn, call_id, changes, /*reason*/ None, /*grant_root*/ None,
)
.await;
rx_approve.await.unwrap_or_default()
ApprovalPlan {
cache: ApprovalCache::SessionApproveOnly {
tool_name: "apply_patch",
keys: approval_keys,
},
user: UserApprovalRequest::Patch(PatchApprovalRequest {
call_id: call_id.clone(),
changes,
reason: None,
grant_root: None,
}),
guardian: GuardianApproval::new(
ApplyPatchRuntime::build_guardian_review_request(req, ctx.call_id),
retry_reason,
),
},
)
.await
.decision
})
}

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

@@ -15,7 +15,12 @@ use crate::sandboxing::ExecOptions;
use crate::sandboxing::SandboxPermissions;
use crate::sandboxing::execute_env;
use crate::shell::ShellType;
use crate::tools::approval::route_approval;
use crate::tools::approval::ApprovalCache;
use crate::tools::approval::ApprovalPlan;
use crate::tools::approval::CommandApprovalRequest;
use crate::tools::approval::GuardianApproval;
use crate::tools::approval::UserApprovalRequest;
use crate::tools::approval::request_approval;
use crate::tools::network_approval::NetworkApprovalMode;
use crate::tools::network_approval::NetworkApprovalSpec;
use crate::tools::runtimes::build_sandbox_command;
@@ -151,41 +156,44 @@ impl Approvable<ShellRequest> for ShellRuntime {
let turn = ctx.turn;
let call_id = ctx.call_id.to_string();
Box::pin(async move {
let guardian_request = GuardianApprovalRequest::Shell {
id: call_id.clone(),
command: command.clone(),
cwd: cwd.clone(),
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
};
route_approval(
request_approval(
session,
turn,
ctx.guardian_review_id.clone(),
Some(("shell", keys)),
guardian_request,
retry_reason,
|| async move {
session
.request_command_approval(
turn,
call_id,
/*approval_id*/ None,
ApprovalPlan {
cache: ApprovalCache::SessionApproveOnly {
tool_name: "shell",
keys,
},
user: UserApprovalRequest::Command(CommandApprovalRequest {
call_id: call_id.clone(),
approval_id: None,
command: command.clone(),
cwd: cwd.clone(),
reason,
network_approval_context: ctx.network_approval_context.clone(),
proposed_execpolicy_amendment: req
.exec_approval_requirement
.proposed_execpolicy_amendment()
.cloned(),
additional_permissions: req.additional_permissions.clone(),
available_decisions: None,
}),
guardian: GuardianApproval::new(
GuardianApprovalRequest::Shell {
id: call_id,
command,
cwd,
reason,
ctx.network_approval_context.clone(),
req.exec_approval_requirement
.proposed_execpolicy_amendment()
.cloned(),
req.additional_permissions.clone(),
None,
)
.await
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
},
retry_reason,
),
},
)
.await
.decision
})
}

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

@@ -9,8 +9,12 @@ use crate::sandboxing::ExecOptions;
use crate::sandboxing::ExecRequest;
use crate::sandboxing::SandboxPermissions;
use crate::shell::ShellType;
use crate::tools::approval::guardian_review_id_for_turn;
use crate::tools::approval::route_approval;
use crate::tools::approval::ApprovalCache;
use crate::tools::approval::ApprovalPlan;
use crate::tools::approval::CommandApprovalRequest;
use crate::tools::approval::GuardianApproval;
use crate::tools::approval::UserApprovalRequest;
use crate::tools::approval::request_approval_for_turn;
use crate::tools::runtimes::build_sandbox_command;
use crate::tools::sandboxing::SandboxAttempt;
use crate::tools::sandboxing::ToolCtx;
@@ -390,46 +394,44 @@ impl CoreShellActionProvider {
let call_id = self.call_id.clone();
let approval_id = Some(Uuid::new_v4().to_string());
let source = self.tool_name;
let guardian_review_id = guardian_review_id_for_turn(&turn);
let user_session = session.clone();
let user_turn = turn.clone();
Ok(stopwatch
.pause_for(async move {
let decision = route_approval(
let outcome = request_approval_for_turn(
&session,
&turn,
guardian_review_id.clone(),
None::<(&'static str, Vec<String>)>,
GuardianApprovalRequest::Execve {
id: call_id.clone(),
source,
program: program.to_string_lossy().into_owned(),
argv: argv.to_vec(),
cwd: workdir.clone(),
additional_permissions: additional_permissions.clone(),
},
/*retry_reason*/ None,
|| async move {
user_session
.request_command_approval(
&user_turn,
call_id,
approval_id,
command,
workdir,
/*reason*/ None,
/*network_approval_context*/ None,
/*proposed_execpolicy_amendment*/ None,
ApprovalPlan {
cache: ApprovalCache::<String>::None,
user: UserApprovalRequest::Command(CommandApprovalRequest {
call_id: call_id.clone(),
approval_id,
command: command.clone(),
cwd: workdir.clone(),
reason: None,
network_approval_context: None,
proposed_execpolicy_amendment: None,
additional_permissions: additional_permissions.clone(),
available_decisions: Some(vec![
ReviewDecision::Approved,
ReviewDecision::Abort,
]),
}),
guardian: GuardianApproval::new(
GuardianApprovalRequest::Execve {
id: call_id,
source,
program: program.to_string_lossy().into_owned(),
argv: argv.to_vec(),
cwd: workdir,
additional_permissions,
Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]),
)
.await
},
/*retry_reason*/ None,
),
},
)
.await;
PromptDecision {
decision,
guardian_review_id,
decision: outcome.decision,
guardian_review_id: outcome.guardian_review_id,
}
})
.await)

66
codex-rs/core/src/tools/runtimes/unified_exec.rs
View File

@@ -11,7 +11,12 @@ use crate::guardian::GuardianApprovalRequest;
use crate::sandboxing::ExecOptions;
use crate::sandboxing::SandboxPermissions;
use crate::shell::ShellType;
use crate::tools::approval::route_approval;
use crate::tools::approval::ApprovalCache;
use crate::tools::approval::ApprovalPlan;
use crate::tools::approval::CommandApprovalRequest;
use crate::tools::approval::GuardianApproval;
use crate::tools::approval::UserApprovalRequest;
use crate::tools::approval::request_approval;
use crate::tools::network_approval::NetworkApprovalMode;
use crate::tools::network_approval::NetworkApprovalSpec;
use crate::tools::runtimes::build_sandbox_command;
@@ -127,42 +132,45 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
let retry_reason = ctx.retry_reason.clone();
let reason = retry_reason.clone().or_else(|| req.justification.clone());
Box::pin(async move {
let guardian_request = GuardianApprovalRequest::ExecCommand {
id: call_id.clone(),
command: command.clone(),
cwd: cwd.clone(),
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
tty: req.tty,
};
route_approval(
request_approval(
session,
turn,
ctx.guardian_review_id.clone(),
Some(("unified_exec", keys)),
guardian_request,
retry_reason,
|| async move {
session
.request_command_approval(
turn,
call_id,
/*approval_id*/ None,
ApprovalPlan {
cache: ApprovalCache::SessionApproveOnly {
tool_name: "unified_exec",
keys,
},
user: UserApprovalRequest::Command(CommandApprovalRequest {
call_id: call_id.clone(),
approval_id: None,
command: command.clone(),
cwd: cwd.clone(),
reason,
network_approval_context: ctx.network_approval_context.clone(),
proposed_execpolicy_amendment: req
.exec_approval_requirement
.proposed_execpolicy_amendment()
.cloned(),
additional_permissions: req.additional_permissions.clone(),
available_decisions: None,
}),
guardian: GuardianApproval::new(
GuardianApprovalRequest::ExecCommand {
id: call_id,
command,
cwd,
reason,
ctx.network_approval_context.clone(),
req.exec_approval_requirement
.proposed_execpolicy_amendment()
.cloned(),
req.additional_permissions.clone(),
None,
)
.await
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
tty: req.tty,
},
retry_reason,
),
},
)
.await
.decision
})
}