mirror of
https://github.com/openai/codex.git
synced 2026-11-11 21:10:39 +00:00
Add execve Guardian timeout fallback coverage
This commit is contained in:
@@ -348,6 +348,55 @@ struct PromptDecision {
|
||||
rejection_message: Option<String>,
|
||||
}
|
||||
|
||||
struct ExecvePromptRequest {
|
||||
session: Arc<crate::session::session::Session>,
|
||||
turn: Arc<crate::session::turn_context::TurnContext>,
|
||||
call_id: String,
|
||||
approval_id: Option<String>,
|
||||
command: Vec<String>,
|
||||
workdir: AbsolutePathBuf,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
}
|
||||
|
||||
impl ExecvePromptRequest {
|
||||
async fn resolve_guardian_decision(
|
||||
self,
|
||||
guardian_review_id: Option<String>,
|
||||
decision: ReviewDecision,
|
||||
) -> PromptDecision {
|
||||
if !matches!(decision, ReviewDecision::TimedOut)
|
||||
|| !self.turn.manual_approval_fallback_enabled
|
||||
{
|
||||
return PromptDecision {
|
||||
decision,
|
||||
guardian_review_id,
|
||||
rejection_message: None,
|
||||
};
|
||||
}
|
||||
|
||||
let decision = self
|
||||
.session
|
||||
.request_command_approval(
|
||||
&self.turn,
|
||||
self.call_id,
|
||||
self.approval_id,
|
||||
self.command,
|
||||
self.workdir,
|
||||
Some(guardian_timeout_message()),
|
||||
/*network_approval_context*/ None,
|
||||
/*proposed_execpolicy_amendment*/ None,
|
||||
self.additional_permissions,
|
||||
Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]),
|
||||
)
|
||||
.await;
|
||||
PromptDecision {
|
||||
decision,
|
||||
guardian_review_id: None,
|
||||
rejection_message: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn execve_prompt_is_rejected_by_policy(
|
||||
approval_policy: AskForApproval,
|
||||
decision_source: &DecisionSource,
|
||||
@@ -416,11 +465,11 @@ impl CoreShellActionProvider {
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
) -> anyhow::Result<PromptDecision> {
|
||||
let command = join_program_and_argv(program, argv);
|
||||
let approval_id = Some(Uuid::new_v4().to_string());
|
||||
let workdir = workdir.clone();
|
||||
let session = self.session.clone();
|
||||
let turn = self.turn.clone();
|
||||
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 = routes_approval_to_guardian(&turn).then(new_guardian_review_id);
|
||||
Ok(stopwatch
|
||||
@@ -473,16 +522,18 @@ impl CoreShellActionProvider {
|
||||
/*retry_reason*/ None,
|
||||
)
|
||||
.await;
|
||||
if !matches!(decision, ReviewDecision::TimedOut)
|
||||
|| !turn.manual_approval_fallback_enabled
|
||||
{
|
||||
return PromptDecision {
|
||||
decision,
|
||||
guardian_review_id,
|
||||
rejection_message: None,
|
||||
};
|
||||
}
|
||||
Some(guardian_timeout_message())
|
||||
let prompt_request = ExecvePromptRequest {
|
||||
session: session.clone(),
|
||||
turn: turn.clone(),
|
||||
call_id: call_id.clone(),
|
||||
approval_id: approval_id.clone(),
|
||||
command: command.clone(),
|
||||
workdir: workdir.clone(),
|
||||
additional_permissions: additional_permissions.clone(),
|
||||
};
|
||||
return prompt_request
|
||||
.resolve_guardian_decision(guardian_review_id, decision)
|
||||
.await;
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
@@ -9,8 +9,11 @@ use super::join_program_and_argv;
|
||||
use super::map_exec_result;
|
||||
use crate::config::Constrained;
|
||||
use crate::guardian::can_fallback_to_manual_after_guardian_timeout;
|
||||
use crate::guardian::guardian_timeout_message;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::session::tests::make_session_and_context;
|
||||
use crate::session::tests::make_session_and_context_with_rx;
|
||||
use crate::state::ActiveTurn;
|
||||
use anyhow::Context;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Evaluation;
|
||||
@@ -31,8 +34,10 @@ use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::GuardianCommandSource;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::policy_transforms::effective_permission_profile;
|
||||
@@ -143,6 +148,71 @@ fn execve_guardian_timeout_manual_fallback_is_limited_to_interactive_sources() {
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn execve_guardian_timeout_falls_back_to_manual_approval() -> anyhow::Result<()> {
|
||||
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
|
||||
*session.active_turn.lock().await = Some(ActiveTurn::default());
|
||||
Arc::get_mut(&mut turn_context)
|
||||
.expect("turn context should be unique")
|
||||
.manual_approval_fallback_enabled = true;
|
||||
|
||||
let approval_id = "execve-approval".to_string();
|
||||
let command = vec![
|
||||
"/usr/bin/touch".to_string(),
|
||||
"/tmp/execve-guardian-timeout-fallback".to_string(),
|
||||
];
|
||||
let workdir = AbsolutePathBuf::try_from(std::env::current_dir()?)?;
|
||||
let prompt_request = super::ExecvePromptRequest {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
call_id: "execve-call".to_string(),
|
||||
approval_id: Some(approval_id.clone()),
|
||||
command: command.clone(),
|
||||
workdir: workdir.clone(),
|
||||
additional_permissions: None,
|
||||
};
|
||||
|
||||
let prompt_handle = tokio::spawn(async move {
|
||||
prompt_request
|
||||
.resolve_guardian_decision(
|
||||
Some("guardian-review-id".to_string()),
|
||||
ReviewDecision::TimedOut,
|
||||
)
|
||||
.await
|
||||
});
|
||||
|
||||
let event = tokio::time::timeout(Duration::from_secs(1), rx.recv())
|
||||
.await
|
||||
.context("timed out waiting for execve approval request")??;
|
||||
let EventMsg::ExecApprovalRequest(approval) = event.msg else {
|
||||
panic!("expected exec approval request event");
|
||||
};
|
||||
assert_eq!(approval.effective_approval_id(), approval_id);
|
||||
assert_eq!(approval.command, command);
|
||||
assert_eq!(approval.cwd, workdir);
|
||||
assert_eq!(approval.reason, Some(guardian_timeout_message()));
|
||||
assert_eq!(
|
||||
approval.available_decisions,
|
||||
Some(vec![ReviewDecision::Approved, ReviewDecision::Abort])
|
||||
);
|
||||
|
||||
session
|
||||
.notify_approval(&approval.effective_approval_id(), ReviewDecision::Approved)
|
||||
.await;
|
||||
let prompt_decision = tokio::time::timeout(Duration::from_secs(1), prompt_handle)
|
||||
.await
|
||||
.context("timed out waiting for manual approval decision")??;
|
||||
assert_eq!(
|
||||
prompt_decision.decision,
|
||||
ReviewDecision::Approved,
|
||||
"manual approval should replace the Guardian timeout decision"
|
||||
);
|
||||
assert_eq!(prompt_decision.guardian_review_id, None);
|
||||
assert_eq!(prompt_decision.rejection_message, None);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_sandbox_permissions_only_downgrades_preapproved_additional_permissions() {
|
||||
assert_eq!(
|
||||
|
||||
Reference in New Issue
Block a user